First fully-green autonomous build: consolidated acceptance-gate + near-green + test-stability fix stack#172
Merged
Merged
Conversation
#77) At near-green a build can sit at a stable low count while the error SET rotates — each single-error fix spawns a different one (extract a component → its siblings/tests are now missing → revert → rotate) — so it never reaches 0. Count-only WS-B can't see it. build17 parked on this; build16 crossed only by luck (4/4 harness-diagnose: the count stays at best-ever while the fingerprint rotates). Detector (near-green-checkpoint.ts): errorSetSignature — sorted unique per-error tokens (rule|file family when present, else the key; stack-agnostic, line- independent). isNearGreenRotation requires BOTH a count PLATEAU (the window sits at one count — a moving count like 2→1→1 is progress, not rotation) AND a changing signature (not all-same, which is a stuck single). trackNearGreenRotation (turn.ts) records {count, sig} on near-green cycles, tolerates a bounded run of spikes between them (MAX_NEAR_GREEN_SPIKE_GAP — past it the stale window clears so far-apart episodes can't combine), does NOT record completion-phase churn, and clears on green. On rotation, injectFeedback prepends a GENERIC completion-only steer that cleanly separates the two poles (do NOT open new surface — no new components/ splits/features; but DO create the siblings + colocated test a current error requires, atomically, even though they aren't in the error list yet). Gated on FOUR conditions: flag on (authoritative at emit) AND detector fired AND count ≤N (not a spike) AND not completion phase. The near-green banner drops ONLY its 'do NOT create new files' lockdown clause when the steer fires (it would contradict the steer) but KEEPS the regression callout. Per-drive state cleared at both drive boundaries; a mid-run kill-switch flip clears the sticky flag. Flag nearGreenRotation() default ON, kill TSFORGE_NO_NEAR_GREEN_ROTATION. Generic — no stack strings. Tests cover the detector (plateau/stuck/descent/key-fallback), the tracker (rotation, spike- ignore, spike-gap-clear, completion-skip, green-clear, mid-run-kill), the flag, the drive-boundary clear, and the user-visible injection (prepend, banner lockdown-drop + regression-keep, flag-off/spike/completion/no-detector suppression).
…anel round-2) Panel major finding: trackNearGreenRotation recorded every gate result and judged rotation purely from gate OUTPUT (error-set signature), with no independent signal that the scope files actually changed between cycles. A flaky/stateful gate — or a re-evaluation of the same unedited files (the check-tool + settleGate double-run, #59) — could emit A->B->C signatures with no fix-one->spawn-another cycle and falsely stand the WS-B rollback net down. - Stamp each near-green sample with an INDEPENDENT worktree rev (a content hash over the glob-resolved scope files via the same snapshot substrate WS-B rolls back with, so scope can't drift). isNearGreenRotation now requires BOTH the signature AND the rev to change on every cycle -> a rotating signature over UNCHANGED files reads as not-rotating and leaves the rollback net engaged. rev is computed only on near-green cycles (the only branch that consumes it). - Extract the rotation-emit decision into rotationEmit() so injectFeedback stays under the cognitive-complexity ceiling (minor finding). - Fix stale near-green-banner.test.ts comment referencing a removed param (minor finding). - Tests: track() helper stamps a fresh rev per cycle (models a real edit); new constant-rev cases assert a flaky/re-run gate is NOT rotation; integration test now writes a genuine per-cycle edit.
Follow-up to the panel's review of the global timeout bump (major/gate-relaxed,
agreement:1 across reviewers): a suite-wide 30s ceiling weakened fail-fast for
ALL 273 files, not just the load-sensitive integration tests, and could let an
unrelated regression that overshoots 5s pass.
Root cause (measured): every test in agent-runner.test.ts constructs
`new AgentRunner(...).run({ cwd: REPO })`, which pays a fixed ~2s startup cost
against the real repo — even the immediate-abort / no-op cases. In isolation the
file runs ~1.9s/test, under bun's 5000ms default; only under the full suite's
273-file concurrency does CPU contention inflate that startup past 5s, spuriously
timing out and false-BLOCKing the harness-review pre-validate gate.
Fix: setDefaultTimeout(15_000) at the top of agent-runner.test.ts — module-scoped,
so ONLY this file's ceiling rises. The 5s fail-fast default and its infinite-loop
detection stay intact for the other 272 files. 15s is generous margin over the
real ~2s work under contention while still failing a genuine hang here. Gate
stays FULL; not a relaxation.
…ly AgentRunner tests Follow-up to the panel's finding (major/gate-relaxed, agreement:1): file-scoped setDefaultTimeout(15s) also raised the ceiling for the parseAgentSpec and loadAgentSpecs tests, which do NOT construct AgentRunner — over-broadening the relaxation and contradicting the comment's 'every test here' claim. Move those pure config-spec tests to a new agent-specs.test.ts (they belong there anyway — fast, hermetic, no repo). agent-runner.test.ts now contains ONLY AgentRunner-against-repo tests, so setDefaultTimeout(15_000) is precisely scoped to exactly the load-sensitive tests; every other file (including the moved config tests) keeps bun's 5s fail-fast default. Comment corrected to match. 14 tests unchanged (11 AgentRunner + 3 config), all green; typecheck/lint/format clean.
… service) — no timeout change Panel BLOCK on the scoped timeout (major/gate-relaxed): a reviewer held that ANY raised ceiling lets a 5-15s regression in these tests pass — a relaxation, even scoped. They're right that the timeout was treating a symptom. Root cause (found): AgentRunner.run calls buildTsService(cwd) when no tsService is passed. With cwd: REPO that builds a TypeScript service over the ENTIRE tsforge repo — ~2s PER test — even for immediate-abort / no-op cases. That fixed cost, not the loop work, blew bun's 5s default under full-suite CPU contention. Fix at the root: pass tsService: null in every run() here (as many sibling tests already do). These read-only tests exercise tool-gating / events / maxTurns / abort / policy — none use the type-aware tools — so a null service is faithful. Result: the file drops from ~24.6s to ~5.3s (11 tests), every test well under the 5s fail-fast default. setDefaultTimeout REMOVED entirely — NO timeout change, so nothing is relaxed; fail-fast + infinite-loop detection are fully intact. (Pure config tests remain split into agent-specs.test.ts from the prior step.)
…all missed + fix stale comment
Panel caught (agreement:1, major): the two structured-mode tests use the
single-line .run({ provider, cwd: REPO, ... }) form, which my cwd:REPO-line
replace_all didn't match — so they still called buildTsService(REPO) (~2s) and
the 'every run passes tsService: null' comment was false. Add tsService: null to
both; now EVERY run() is stubbed (file ~24.6s -> ~0.4s). Also fix the stale
agent-specs.test.ts header comment that referenced a 'raised per-file timeout'
(removed in this PR). All 11 AgentRunner + 3 config tests green; typecheck/lint/format clean.
…r itself (preflight-trap park) build23 root cause: after reaching fast-gate GREEN, the model ran the browser E2E itself (`cd apps/ui && npx playwright test`, no PLAYWRIGHT_PORT → defaults to 7331), which triggers the scaffold's `bun run dev` webServer whose `preflight-host-dev.sh` HARD-EXITS 1 because the dockerized ui-dev container already holds the port. That's an infra guard, not a code error — the model can't fix it, loops on it (~14×/slice), burns the escalation ladder, and PARKS a feature whose code is already green + tested. Observed on Company AND Contact (both reached GREEN + full CRUD + passing unit tests, then parked identically); Deal/Activity would follow. Very likely the true build22 '0/4 park' cause too — NOT near-green oscillation (#77). The harness runs the full Playwright acceptance automatically after the fast gate (verifyAcceptance, with the correct PLAYWRIGHT_PORT + reuseExistingServer), so the model's self-run is both redundant and harmful. Add an explicit prompt block: the model must NOT run `playwright`/`bun run dev`/`vite`/`dev.sh`; acceptance is harness-owned; when `check` returns passed:true with the required testids, it is DONE and must stop. No gate relaxed — the preflight guard stays; the model is steered away from tripping it.
…s, harness-owned acceptance, stop-at-green) Panel finding (agreement:1, missing-test): the assertion only checked the heading + preflight-host-dev.sh, so it would still pass if the actual prohibitions or the stop-at-fast-gate-green instruction were removed. Assert those directly.
… E2E / dev server Guidance alone did not stop it (build24: the model ran 'npx playwright test' right after gate-green despite the explicit prompt prohibition, tripped preflight-host-dev.sh, looped, and parked all 4 green slices). Add a deterministic backstop: - policy: mergePolicyRules(base, extra) — append-merge rule sets so a build BACKEND can inject deny rules on top of the user's config (deny-first, so the injected deny wins). - Session.create gains a policyRules input, merged with tsforge.config.json's policy.rules. - BoringStack build injects a shell deny on playwright/bun run dev/vite/dev.sh. The model physically cannot start a second host dev server now; the harness's own acceptance is unaffected (it uses a separate injected exec, not the policy-gated run tool). Verified: the rule denies every invocation form the model used (cd apps/ui && npx playwright test, bunx playwright, bun run dev, vite, preflight+vite) and ALLOWS the gate (bun run check, bun test). No gate relaxed. Pairs with the prompt guidance already on this branch.
…E reuses the dockerized ui-dev
THE build22-26 park cause (proven by repro): the scaffold's playwright.config gates
reuseExistingServer on '!CI'. In a CI=1 build env reuse flips OFF, so Playwright tries to
START its own host dev server on the UI port, collides with the running ui-dev container
('http://localhost:56419 is already used'), and exits with ZERO tests run (.last-run.json:
status=failed, failedTests=[]). The harness reads that as a failed acceptance and parks a
feature whose code is GREEN — every slice, every build.
Repro (ui-dev up): CI=1 (no flag) → 'port already used', run fails; CI=1 + PLAYWRIGHT_REUSE_SERVER=true → 1 passed.
Fix: the e2e-runner sets PLAYWRIGHT_REUSE_SERVER=true in the env for both run() and runChain(),
forcing reuse regardless of CI. The harness guarantees the dockerized ui-dev is already serving,
so Playwright must attach, never self-spawn. No gate relaxed — acceptance still runs the full flow.
…ject the whole spec
THE reason acceptance ran ZERO tests (build22-27, masked under the port issue): the E2E
generator titled every negative 'negative: <Entity> rejects <field>=<value>'. Two negatives can
share the same (field,value) — a required-field rule and a mustNotHappen both yield name='' (the
codebase intentionally keeps both, see 'FIX 7') — so two tests got the IDENTICAL title.
Playwright HARD-REJECTS a file with duplicate titles: the spec fails to collect, zero tests run,
.last-run.json = {status:failed, failedTests:[]}, and the harness reads a GREEN feature as a
failed acceptance → park.
Fix: append the negative's [index] to the title. parseStep matches on the 'negative: ' PREFIX,
so classification is unaffected. Proven by direct repro against the live app: before = 'duplicate
test title' error, 0 tests; after = full flow runs (nav/list/create/persist/update/negative pass;
delete fails on a REAL app bug the acceptance now correctly catches). Regression test added.
… not the dialog wrapper The delete acceptance step failed on EVERY build (build22-29) with a real, deterministic cause: the testid guide said to put company-confirm-delete on 'the confirmation dialog', so the model put it on the dialog overlay <div> (fixed inset-0 backdrop). The E2E delete step CLICKS that testid to confirm — clicking the backdrop hits nothing, the delete mutation never fires, the row stays, and toHaveCount(0) fails. The model's delete logic (mutation + invalidateQueries on the correct key) was CORRECT the whole time; only the testid placement was wrong, and the guide invited it. Fix: guide now says confirm-delete goes on the BUTTON whose onClick fires the delete (never the wrapper/overlay). Found by running the harness acceptance directly against build29's parked-but- green Company and reading CompanyPage.tsx: testid on line 190 = overlay div; the real confirm button (onClick=onConfirmDelete) had no testid. Regression test added.
…ps its nav-link count in sync build30 got Company to 'verified ✓' (full browser acceptance passed — delete included) but the build stayed 'stuck': the FINAL full-project validate ran the vitest suite and AppSidebar.test.tsx failed — 'expected length 6, got 7' — because the model correctly ADDED its Company NavLink (reachability requires it) and the scaffold test hard-codes the base nav-link count. Every feature addition breaks it; the per-feature fast gate (bun run check) doesn't run vitest, so it only surfaces at the final validate. The scaffold is an external clone we can't edit, so: add AppSidebar.test.tsx to the feature's editable scope and instruct the model (refinePrompt) to bump the sidebar test's expected link count by its one added link. On-pattern with how schema/locale/routes are model-owned + kept consistent. Tests: scope includes the sidebar test; prompt tells the model to bump the count.
…r appears (API/console capture) The create/persist acceptance step failed as an opaque 'getByTestId(company-row)... toBeVisible timeout' — giving the model (and me) no idea WHY, so each park needed a manual repro to diagnose and the steer couldn't guide a fix. Now the create step records every /api/ mutation (+ status) and console errors, and appends them to the failure: - POST 4xx present -> API rejected the payload (contract/validation bug) - POST 2xx present -> the mutation worked; the LIST/row render is the bug - no mutation fired -> the form submit isn't wired to the create mutation (build31's actual cause) Proven via direct repro against build31's parked Company: the message now reads 'No API mutation fired at all — the create form's submit is not wired to the create mutation', which is exactly right. This makes acceptance failures self-explaining for the model's steer instead of a symptom. (create step first — the first mutation step; persist/update/delete are a follow-up.)
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.
What
The complete fix stack that produced the first fully-green autonomous BoringStack build — build32: a Company CRUD slice passing the fast gate + full Playwright browser acceptance (nav→list→create→persist→update→delete→negative) + final full-project validate (check + vitest + build + size), all green. Every fix here is validated by that live build.
Supersedes #170 (#77) and #171 (#63) — their final commits are included in this branch, so this is the single PR to merge; close #170 and #171 as superseded.
Fixes bundled
Reach green (near-green oscillation, #77)
revguard so a flaky/re-run gate over unchanged files can't fake a rotation (near-green-checkpoint.ts,turn.ts).Stop the pre-validate gate false-BLOCKing (#63)
tsService: null, skipping the ~2s whole-repo TS-service build) + pure config tests split out — so the harness-review pre-gate stops flaky-timeout false-BLOCKs. No timeout raised, no gate relaxed.Stop the model breaking its own build
playwright/bun run dev/viteitself and trip the host preflight (session.tspolicyRules+mergePolicyRules,build-configdeny,refine-prompt).Make the acceptance gate actually run and grade honestly (these are why builds stopped false-parking)
PLAYWRIGHT_REUSE_SERVER=trueso the harness E2E reuses the dockerizedui-devinstead of colliding on the port.[index]) so Playwright doesn't reject the whole spec on duplicate titles (0 tests run).confirm-deletetestid must be on the confirm button, not the dialog wrapper (the E2E clicks it).Review / proof
harness-review: PASS. Each component was also individually panel-reviewed (feat(agents): live agent tree renderer for fan-out (multiagent PR-B3) #77 PASS ×2, [codex] fix editor input regressions #63 PASS 4/4, no-model-e2e PASS 4/4).1/1 resource(s) verified+final acceptance GREEN — full validate + build + size checks all pass.