feat: Phase 1 foundation — tests, CI, content accuracy, security - #4
Merged
Conversation
Set up the project's first test infrastructure: Vitest (jsdom + Testing Library) for unit/component tests, Playwright for E2E, and a GitHub Actions workflow gating PRs on lint + typecheck + test. Replace the no-op ESLint config with eslint-config-next's native flat core-web-vitals rules (no FlatCompat). Pre-existing react-hooks violations are downgraded to warnings so the gate is green from day one, to be burned down in the depth/polish pass. Seed one passing test per layer (unit, component, e2e); e2e runs locally and is not in the CI gate. Implements U2 of docs/plans/2026-06-17-001-feat-maximize-ironforge-plan.md Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tests Move the inline rule-of-thumb figures (burden %, $/ton, profit, bonding, etc.) into lib/estimator/constants.ts; clamp negative/NaN numeric inputs at the calculation boundary so a bypassed HTML min hint can't poison a bid; derive the bonding markup from PROFIT_PCT/CONTINGENCY_PCT so it can't drift from the profit/contingency constants. Add 13 tests: golden total, per-ton + compounding-order locks, monopolistic-WC and prevailing-wage behavior, and clamping edge cases. Implements U3 of docs/plans/2026-06-17-001-feat-maximize-ironforge-plan.md Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Make data provenance a first-class field: StateVerification (lastVerified / sourceUrl / effectiveDate) on StateData, backfilled for the hand-crafted WA and OR states. getStateVerification() resolves a well-formed view for every state — verified states carry their date, machine-generated states report unverified rather than a misleading one. A persistent, non-dismissable LegalDisclaimer renders on every wizard step: "not legal advice — verify with your state agency", plus the last-verified date for verified states or a "not independently verified" notice for the machine-generated ones. Implements U5 of docs/plans/2026-06-17-001-feat-maximize-ironforge-plan.md Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
getPhaseContent fell back to Washington's content for any state missing from the registry — silently presenting one state's regulatory facts as another's. Replace all six fallbacks with an explicit, labeled "data unavailable for <state>" phase that points the user at their own agency. Add a contract test asserting all 50 states x 7 phases resolve to valid non-empty steps, that a non-WA state gets generated (not Washington) content, that an unknown code gets the labeled phase (never Washington), and that the chat-route step lookup finds a phase's steps. Implements U4 of docs/plans/2026-06-17-001-feat-maximize-ironforge-plan.md Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The proxy enforced same-origin only on /api/chat. Generalize it to every state-changing API request (non-GET/HEAD on /api/*), so /api/bid-review and /api/onboarding are no longer open to cross-site calls. Move the full security-header set (CSP, HSTS, frame/permissions policy) into next.config.ts as the single source of truth — it covers static assets the middleware matcher excludes — and drop the duplicate weaker set. Narrow the proxy matcher to /api/*. Rate limiting stays in the route handlers (in-memory, per-instance; a durable limiter is plan U10). Implements U6 of docs/plans/2026-06-17-001-feat-maximize-ironforge-plan.md Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
From the foundation code review (no P0/P1; these are the high-value P2/P3s): - Clamp estimator inputs to MAX_FIELD_VALUE (1e9) so an absurd or tampered value (e.g. 1e307) can't overflow the bid to Infinity/NaN — the clamp's stated invariant was false for large finite values. - Add the flagged missing tests: overflow finiteness; estimator cost-line flags in the "on" direction, non-union burden, and cost-per-sqft; wizard navigation (getNextStep/getPrevStep across phase boundaries); and the unavailable / legal-federal / unknown-phase content paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CI's node 22 npm rejected the lockfile (Missing @swc/helpers@0.5.23) while node 24 (local dev + lockfile generator) accepts it via `npm ci`. Align CI to node 24 so the clean install matches the committed lockfile. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds a state verification metadata system that drives a new ChangesState Verification, Legal Disclaimer, and Phase Content
Estimator Constants Refactor
Proxy Middleware and Security Headers
Test Infrastructure
Sequence Diagram(s)sequenceDiagram
participant StepContent
participant LegalDisclaimer
participant getStateVerification
participant StateRegistry as STATE_REGISTRY
StepContent->>LegalDisclaimer: render stateCode (from profile?.state)
LegalDisclaimer->>getStateVerification: getStateVerification(stateCode)
getStateVerification->>StateRegistry: lookup state entry
StateRegistry-->>getStateVerification: StateData (with optional verification)
getStateVerification-->>LegalDisclaimer: StateVerificationView { verified, lastVerified, sourceUrl, stateName }
alt verified === true
LegalDisclaimer-->>StepContent: render "Last verified [date]" with source link
else verified === false
LegalDisclaimer-->>StepContent: render "Auto-generated – not independently verified"
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
proxy.test.ts (1)
43-57: ⚡ Quick winAdd test coverage for PUT, PATCH, DELETE, and HEAD methods.
The proxy gates all non-GET/HEAD methods, but only POST is tested. Consider adding explicit tests for:
- PUT/PATCH/DELETE blocked when cross-origin
- HEAD bypasses the gate (like GET)
This ensures future refactors don't accidentally break handling for these methods.
🧪 Suggested additional test cases
+ it("blocks cross-origin PUT/PATCH/DELETE requests", () => { + for (const method of ["PUT", "PATCH", "DELETE"]) { + const res = proxy( + request("/api/chat", method, { origin: "https://evil.example", host: HOST }), + ); + expect(res.status).toBe(403); + } + }); + + it("does not gate safe HEAD requests", () => { + const res = proxy( + request("/api/chat", "HEAD", { origin: "https://evil.example", host: HOST }), + ); + expect(res.status).not.toBe(403); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@proxy.test.ts` around lines 43 - 57, The test file currently only covers POST and GET methods but the proxy gates all non-GET/HEAD methods. Add new test cases using the same pattern as the existing tests: create separate test cases for PUT, PATCH, DELETE, and HEAD methods. For PUT, PATCH, and DELETE, verify they are blocked when cross-origin by checking that res.status equals 403 (opposite of the current POST test logic). For HEAD, verify it bypasses the gate like GET does by checking that res.status is not 403. Use the proxy function with the request helper, passing the appropriate HTTP method and cross-origin headers (origin: "https://evil.example", host: HOST) to ensure the methods are properly gated or allowed as expected.components/wizard/legal-disclaimer.tsx (1)
34-85: ⚡ Quick winAdopt shadcn/ui base primitives for the disclaimer container.
This new component is implemented with raw HTML nodes only, which drifts from the repository’s required component baseline. Please wrap this in the project’s shadcn/ui primitives (e.g., alert/card composition) and keep the cyberpunk variants on top.
As per coding guidelines, "Use shadcn/ui components as the base component library and extend with custom cyberpunk variants (e.g.,
.btn-neon-solid,.btn-neon-cyan,.cyber-card) in globals.css".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/wizard/legal-disclaimer.tsx` around lines 34 - 85, Replace the raw HTML div container (the one with role="note" and aria-label="Legal disclaimer") with a shadcn/ui primitive component such as Alert or Card that provides semantic structure and accessibility out of the box. Maintain the cyberpunk visual styling by applying the custom className variants (neon-amber, neon-blue colors and border/background patterns) on top of the shadcn/ui component. Update all nested div and p elements to use appropriate shadcn/ui sub-components or typography primitives where available, ensuring the component follows the repository's pattern of using shadcn/ui as the base component library with cyberpunk customizations applied via globals.css variants.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 12-15: The GitHub Actions workflow uses mutable version tags (`@v4`)
for both actions/checkout and actions/setup-node actions instead of pinning to
specific commit SHAs, which weakens supply-chain security. Additionally, the
actions/checkout action retains default credential persistence which increases
token exposure risk. Replace the `@v4` mutable tags with specific commit SHAs for
both the actions/checkout and actions/setup-node actions, and add a token
persistence configuration to the actions/checkout action to explicitly disable
credential persistence after checkout completes.
In `@lib/estimator/constants.ts`:
- Around line 22-24: The MONOPOLISTIC_WC_STATES constant duplicates regulatory
classification data that already exists in the STATE_REGISTRY from
content/state-registry.ts using the wcType property. Remove the hard-coded
MONOPOLISTIC_WC_STATES set and instead derive the monopolistic states
dynamically by filtering the STATE_REGISTRY for states where the wcType field
matches the monopolistic regime classification. Update any references to
MONOPOLISTIC_WC_STATES in lib/estimator/calculate.ts and elsewhere to use this
derived filter from STATE_REGISTRY instead, ensuring the single source of truth
pattern is maintained.
---
Nitpick comments:
In `@components/wizard/legal-disclaimer.tsx`:
- Around line 34-85: Replace the raw HTML div container (the one with
role="note" and aria-label="Legal disclaimer") with a shadcn/ui primitive
component such as Alert or Card that provides semantic structure and
accessibility out of the box. Maintain the cyberpunk visual styling by applying
the custom className variants (neon-amber, neon-blue colors and
border/background patterns) on top of the shadcn/ui component. Update all nested
div and p elements to use appropriate shadcn/ui sub-components or typography
primitives where available, ensuring the component follows the repository's
pattern of using shadcn/ui as the base component library with cyberpunk
customizations applied via globals.css variants.
In `@proxy.test.ts`:
- Around line 43-57: The test file currently only covers POST and GET methods
but the proxy gates all non-GET/HEAD methods. Add new test cases using the same
pattern as the existing tests: create separate test cases for PUT, PATCH,
DELETE, and HEAD methods. For PUT, PATCH, and DELETE, verify they are blocked
when cross-origin by checking that res.status equals 403 (opposite of the
current POST test logic). For HEAD, verify it bypasses the gate like GET does by
checking that res.status is not 403. Use the proxy function with the request
helper, passing the appropriate HTTP method and cross-origin headers (origin:
"https://evil.example", host: HOST) to ensure the methods are properly gated or
allowed as expected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 338bcdb6-fe2a-4cd7-8b53-357cd096fb8d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (25)
.github/workflows/ci.yml.gitignorecomponents/ui/badge.test.tsxcomponents/wizard/legal-disclaimer.test.tsxcomponents/wizard/legal-disclaimer.tsxcomponents/wizard/step-content.tsxcontent/phases.test.tscontent/phases.tscontent/state-registry.test.tscontent/state-registry.tseslint.config.mjslib/ai/models.test.tslib/estimator/calculate.test.tslib/estimator/calculate.tslib/estimator/constants.tslib/types/content.tslib/utils.test.tsnext.config.tspackage.jsonplaywright.config.tsproxy.test.tsproxy.tstests/e2e/home.spec.tsvitest.config.tsvitest.setup.ts
Comment on lines
+12
to
+15
| - uses: actions/checkout@v4 | ||
| - uses: actions/setup-node@v4 | ||
| with: | ||
| # Match the local dev / lockfile npm version — node 22's older npm |
There was a problem hiding this comment.
Harden workflow actions and disable credential persistence
Line 12 and Line 13 use mutable action tags (@v4) instead of commit SHAs, and Line 12 keeps default credential persistence. This weakens CI supply-chain guarantees and increases token exposure risk.
Suggested patch
- - uses: actions/checkout@v4
+ - uses: actions/checkout@<FULL_COMMIT_SHA>
+ with:
+ persist-credentials: false
- - uses: actions/setup-node@v4
+ - uses: actions/setup-node@<FULL_COMMIT_SHA>🧰 Tools
🪛 zizmor (1.25.2)
[warning] 12-12: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 12-12: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 13-13: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 12 - 15, The GitHub Actions workflow
uses mutable version tags (`@v4`) for both actions/checkout and actions/setup-node
actions instead of pinning to specific commit SHAs, which weakens supply-chain
security. Additionally, the actions/checkout action retains default credential
persistence which increases token exposure risk. Replace the `@v4` mutable tags
with specific commit SHAs for both the actions/checkout and actions/setup-node
actions, and add a token persistence configuration to the actions/checkout
action to explicitly disable credential persistence after checkout completes.
Source: Linters/SAST tools
Comment on lines
+22
to
+24
| /** Monopolistic workers'-comp states (state-run fund, higher WC burden). */ | ||
| export const MONOPOLISTIC_WC_STATES = new Set(["WA", "OH", "ND", "WY"]); | ||
|
|
There was a problem hiding this comment.
Use STATE_REGISTRY as the single source of truth for WC regime detection.
This hard-coded state set duplicates regulatory classification data already modeled in content/state-registry.ts (wcType), so future registry updates can silently desync burden/insurance math in lib/estimator/calculate.ts.
Suggested direction
- export const MONOPOLISTIC_WC_STATES = new Set(["WA", "OH", "ND", "WY"]);
+ // Remove duplicated classification set.
+ // In calculate.ts, derive from registry metadata:
+ // const isMonopolistic = STATE_REGISTRY[stateCode]?.wcType === "monopolistic";As per coding guidelines, content/state-registry.ts should be used as the “central data source for all 50 states regulatory information”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/estimator/constants.ts` around lines 22 - 24, The MONOPOLISTIC_WC_STATES
constant duplicates regulatory classification data that already exists in the
STATE_REGISTRY from content/state-registry.ts using the wcType property. Remove
the hard-coded MONOPOLISTIC_WC_STATES set and instead derive the monopolistic
states dynamically by filtering the STATE_REGISTRY for states where the wcType
field matches the monopolistic regime classification. Update any references to
MONOPOLISTIC_WC_STATES in lib/estimator/calculate.ts and elsewhere to use this
derived filter from STATE_REGISTRY instead, ensuring the single source of truth
pattern is maintained.
Source: Coding guidelines
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c318a4d134
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // === Security headers on all responses === | ||
| const response = NextResponse.next(); | ||
| return addSecurityHeaders(response); | ||
| return NextResponse.next(); |
There was a problem hiding this comment.
When /api/chat is deployed behind a proxy that preserves client-supplied X-Forwarded-For, this now just passes the request through after the CSRF check. The only remaining chat limiter keys on x-forwarded-for before x-real-ip (app/api/chat/route.ts:27-31), so an abusive client can rotate that header and avoid the 20/min cap that the removed proxy limiter previously enforced using x-real-ip.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 1 (Foundation) of the IronForge maximization roadmap (
docs/plans/2026-06-17-001-feat-maximize-ironforge-plan.md). U1 (the model fix) already merged via #1; this PR carries U2–U6 on top, now targetingmaindirectly.Units
.github/workflows/ci.ymlgating lint + typecheck + test. ESLint was a no-op; now useseslint-config-nextnative flatcore-web-vitals(21 pre-existing react-hooks violations downgraded to warnings, flagged for burndown).lib/estimator/constants.ts; inputs clamped at the calc boundary (and capped at 1e9 so absurd values can't overflow the bid); bonding markup derived from the profit/contingency constants.StateVerificationmetadata onStateData(WA/OR backfilled); persistent "not legal advice — verify with your state agency" disclaimer on every wizard step, with a last-verified date for verified states and a "not independently verified" notice for the 48 machine-generated ones.getPhaseContentno longer serves Washington's content for an unknown state — it returns a labeled "data unavailable" phase. Contract test covers all 50 states × 7 phases./api/chatto all API routes; security headers (CSP/HSTS/…) centralized innext.config.ts.Verification
tsc --noEmitclean · lint clean · production build succeeds.Deferred / surfaced (not in this PR)
x-real-ip-first IP keying → plan U10 (Growth).'unsafe-inline'on script-src (Next App Router constraint) → future nonce-based CSP.floors,craneType) and the 21 react-hooks lint warnings → depth/polish pass (U11/U12).🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests
Chores