feat: Phase 1 foundation — tests, CI, content accuracy, security - #2
Closed
Steel-tech wants to merge 7 commits into
Closed
feat: Phase 1 foundation — tests, CI, content accuracy, security#2Steel-tech wants to merge 7 commits into
Steel-tech wants to merge 7 commits into
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>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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: 90f38da1e2
ℹ️ 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".
| with: | ||
| node-version: 22 | ||
| cache: npm | ||
| - run: npm ci |
There was a problem hiding this comment.
In the new CI workflow, this step fails on a clean checkout: npm ci is npm's clean-install command, and running it in this repo exits with EUSAGE because package-lock.json is not in sync (Missing: @swc/helpers@ from lock file, caused by the new @vitejs/plugin-react-swc/@swc/core peer). As written, every PR/push hits this line and stops before lint/typecheck/tests; please regenerate and commit the lockfile so npm ci can install.
Useful? React with 👍 / 👎.
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>
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). Stacked on the U1 model-fix PR (#1) — base auto-retargets tomainonce #1 merges.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