fix(boringstack): persist pristine baseline + reuse on resume (kills the resume false-green)#198
Open
agjs wants to merge 4 commits into
Open
fix(boringstack): persist pristine baseline + reuse on resume (kills the resume false-green)#198agjs wants to merge 4 commits into
agjs wants to merge 4 commits into
Conversation
…lls the resume false-green) Grounded in build55: a fresh Company build reached "1/1 verified" but the full validate found 5 real lint errors the fast gate never caught — a FALSE-GREEN. Root-caused it: the differential gate grades features against a BASELINE captured by running the gate on the "pristine scaffold" at build start, and runBoringstackBuild RE-CAPTURES that baseline on EVERY invocation, including a RESUME. On resume the tree is NON-pristine (it holds the in-progress feature's code), so the re-capture sweeps the feature's OWN failures into the baseline and EXCLUDES them from grading → the feature "verifies" while still failing its own gate. Log proof: cycle-1 (pristine) "baseline scaffold is GREEN"; resumes "baseline scaffold is RED: 8 pre-existing failures EXCLUDED". Fix: capture the baseline ONCE and PERSIST it to `.tsforge/greenfield/baseline.json` (beside the checklist, so a fresh build's reset drops it but a resume keeps it); on any run, REUSE the persisted baseline instead of re-capturing. A GREEN pristine scaffold persists as an empty set (present, not null) so a resume reuses it rather than re-capturing. Persist only when infra is healthy (an infra-failed gate isn't a baseline). Tests: save→load round-trip, absent→null (fresh build captures), GREEN pristine → empty set NOT null (the load-bearing resume case), corrupt JSON → null (re-capture, never crash). Ruled out (verified): eslint --cache, scope, autofix, extractFailures parse (extracted all 5 sigs correctly) — the baseline INPUT was contaminated, everything downstream was correct.
…h-vs-resume, integration tests
The panel BLOCKed the first cut (correctly). Fixes:
- PERSIST THE PASSED BIT, never infer it from set size. A RED pristine gate whose output
doesn't parse is ALSO signatures:[]; the old `size===0 ⇒ GREEN` re-announced it green on
resume (a false-green). Persisted format is now `{ passed, signatures }`.
- KEEP THE FAIL-CLOSED needs-infra CHECK on every entry: the baseline gate STILL runs each
invocation (it's the infra probe); on a resume its captured signatures are DISCARDED in
favour of the persisted ones. Previously reuse skipped the gate → resume could proceed
with a dead API.
- FRESH vs RESUME is now detected via the greenfield checklist (loadState), not "no file".
A resume with no persisted baseline (pre-change build / lost file) is graded STRICTLY
(empty baseline — only ever over-strict, never a false-green) and warned; it does NOT
re-capture from — nor freeze in — the contaminated tree.
- loadBaseline rejects the wrong shape (bare array / missing passed) → null → re-capture.
Tests: round-trip passed+signatures; GREEN → passed:true+empty (present); RED-unparseable →
passed:FALSE+empty (never re-inferred green); wrong-shape → null; corrupt JSON → null; and an
INTEGRATION test — runBoringstackBuild persists on a fresh build then REUSES on resume (gate
flipped to failing; baseline stays passed:true = reused, not re-captured). Full validate green
(3119 tests).
…empty-checklist hole, stop the false RED-unparseable report Round-1 v2 was BLOCKed on five findings; all addressed: - [major, 4 reviewers] The resume-without-persisted-baseline STRICT-FALLBACK branch had no test. Added one: a resume whose baseline.json was lost, with a now-FAILING (contaminated) gate, must run WITHOUT crashing and must NOT persist the contaminated capture (loadBaseline stays null) — the exact safety-critical path the panel flagged. - [major, gate-relaxed] isResume required `features.length > 0`, so a loaded-but-empty checklist was graded FRESH and could persist a contaminated capture. Dropped the length check: a resume is ANY prior checklist on disk (`loadState !== null`). build.ts never writes the checklist before this point, so a genuine fresh build has none here; erring toward "resume" is only ever over-strict, never a false-green. - [major+minor, false diagnostic] On the strict fallback the code still called describeBaseline(false, 0), emitting the "RED-but-unparseable pristine scaffold" message — false here (the baseline is MISSING, not unparseable) and contradicting the warning just emitted. Guarded it with hasPristineReport: the fallback now emits ONLY its own warning. - [minor] loadBaseline JSDoc said null ⇒ "the caller re-captures". Corrected: a fresh build re-captures; a resume grades STRICT and does NOT re-capture. Verify: typecheck clean, lint clean, boringstack-build.test.ts 50 pass (0 fail).
…d resume detection, stronger tests Round-2 drew three correct findings; all fixed: - [critical/scope-bypass] host.captureMetaBaseline() ran on EVERY resume, capturing the META baseline (workflow perms, lockfile, root-drift) from the CONTAMINATED resume tree and subtracting it from grading — the same contaminated-resume false-green, for the meta gate, and it made the "STRICT (nothing excluded)" warning false. Now captured ONLY on a fresh capture (isFreshCapture); on a resume the session's meta-baseline stays unset ⇒ STRICT (nothing subtracted), the safe direction. A clean scaffold has an empty meta baseline anyway, so the common case is unchanged. - [major/gate-relaxed] isResume was `loadState(cwd) !== null`, but loadState returns null for a missing AND a present-but-CORRUPT features.json — so a corrupt-state resume (tree already built into) looked fresh and could persist a CONTAMINATED baseline. New `hasState(cwd)` keys off FILE PRESENCE, so a corrupt/empty checklist is correctly a resume (→ strict). Presence, not parse-success, is the "tree may already be built" signal. - [major/missing-test] strengthened coverage: a FRESH positive control (meta captured once + baseline persisted); a RESUME asserting meta is NOT re-captured AND the false "RED … did NOT parse" report is suppressed (only the strict warning emitted); a CORRUPT features.json treated as a resume (no contaminated persist, no meta capture); and a hasState unit test proving it diverges from loadState exactly on a corrupt file. Verify: typecheck + lint clean; full suite 3124 tests, 0 fail.
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.
Grounded in build55: a fresh Company build reached
1/1 verifiedbut the fullvalidatefound 5 real lint errors the fast gate never caught — a false-green (the exact #50 gate-parity failure).Root cause (fully confirmed): the differential gate grades features against a baseline captured on the pristine scaffold at build start, but
runBoringstackBuildre-captures it on every invocation, including a resume. On resume the tree is non-pristine (holds the in-progress feature's code), so the re-capture sweeps the feature's own failures into the baseline and excludes them from grading → verified-while-failing. Log proof: cycle-1 "baseline scaffold is GREEN"; resumes "baseline scaffold is RED: 8 pre-existing failures EXCLUDED". Ruled out (verified): eslint--cache, scope, autofix, andextractFailures(extracted all 5 sigs correctly).Fix: capture once, persist to
.tsforge/greenfield/baseline.json(a fresh build's reset drops it; a resume keeps it), and reuse it instead of re-capturing. GREEN pristine → empty set (present, not null) so a resume reuses it. Persist only when infra is healthy. Tests cover round-trip / absent / GREEN-empty / corrupt. Fullbun run validategreen (3116 tests).