fix(ship): stop agent-mode Test/Code fabrication and undetected stale turns - #42
Merged
Merged
Conversation
… turns
Four confirmed root causes behind the 1.10.2 agent-mode reliability gaps
(reproduced live against a real binary in a disposable worktree):
1. writeTestArtifactsWithContext wrote the static RED placeholder stub to
tests/*.test.ts whenever an LLM/bridge call returned ANY error, including
ErrAgentTurn (a pause, not a failure). Once written, that placeholder
permanently satisfied allTestArtifactsExist, so checkTest never called
writeTestArtifacts again — a host agent's real, later-submitted answer for
the same operation sat unused in the bridge's response store forever, and
the checkpoint reported "N test file(s) found; all 4 named artifacts
present" against content nobody had reviewed. Fixed by returning the
agent-turn error before writing anything for the branch that paused, so
the exists-guard stays false and the real answer lands on the next run.
2. agentbridge.Bridge.loadPending restored a still-pending turn from disk
without latching `paused`, deferring that to a Lookup call that would only
happen if some checkpoint this run coincidentally re-asked the same
operation. A checkpoint that never calls Lookup at all (an "artefact
already exists" skip, a language branch with no LLM call) left `paused`
false for the whole process, letting `forge ship --agent-mode` run
straight through later checkpoints — and skip the final "render the
pending turn" check, which also gates on Paused() — while an earlier
turn (e.g. an arch-parallel-debate role) sat unanswered on disk the whole
time. Fixed by latching paused immediately on load; replay is unaffected
since Lookup checks byHash/byOrdinal before it ever checks paused.
3. checkSpec treated "no description/--name and 2+ existing spec dirs" as a
soft "ok, pass a description" and let the pipeline continue — every later
checkpoint then ran against an empty/undefined slug. Now hard-fails and
names the candidates, demanding --name. A single unambiguous spec dir is
unaffected.
4. The managed .gitignore block (three copies: codemod.canonicalGitignoreBlock,
codemod.defaultMarkerBody, cmddoctor.canonicalGiSnippet) never listed
.forge/agent/, .forge/.snapshots/, .forge/learned/, .forge/trash/, or
.forge/token-ledger.jsonl. Confirmed live: an agent-mode run left `git
status --porcelain` reporting the bridge's own bookkeeping as untracked,
which checkCode's countChangedFiles then counted as evidence of real code
changes ("N modified file(s)" for a run where the only real source file
was untouched).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
M1-27 compared each file's all-time-latest-touch commit timestamp across the entire repo history, not just this PR's commits. That means ANY test-only PR touching a mature file — no production change at all — was flagged as a TDD violation purely because the test file's last historical edit (this PR's own commit) postdated the paired production file's last historical edit (from whenever it was last touched, possibly months ago). That's true for nearly every established file, and hit this branch's own PR (agent_mode_test.go touched, agent_mode.go untouched). Scope both `git log` calls to "origin/$BASE..HEAD" and take the oldest commit within that range (`tail -1`) instead of the newest across all history (`head -1`). An untouched production file now correctly produces no commits in range (empty PROD_DATE) and is skipped, while a genuine same-PR "production code first, test added later" violation is still caught — verified both cases locally against this fix before pushing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
….mod ci-gates.yml was still pinned to GO_VERSION '1.25' while ci.yml and nightly.yml were already on '1.26', and go.mod's toolchain directive has required go1.26.x since 2026-07-10 (bumped for stdlib CVE fixes, most recently go1.26.6 on 2026-08-20). The drift meant ci-gates' M2-17 perf benchmark gate installed benchstat against Go 1.25 and failed outright once golang.org/x/perf's @latest started requiring go >= 1.26.0 — a pre-existing, repo-wide break confirmed on an unrelated dependabot PR from 4 days before this fix, not something introduced by this branch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Version Scope Decision:
- Chosen bump: PATCH
- Why not smaller: agent-mode fabricating checkpoint success while
discarding real host-agent answers, and a stale-turn blind spot that let
it happen even when a prior turn was still unanswered, are bug fixes to
behaviour that was already wrong — both need to reach users.
- Breaking impact: none. No CLI flag, output key, or exported symbol
changed. writeTestArtifacts(WithContext)/llmGoStub gained a return value
but are unexported (package cmdship internals); agentbridge.Bridge's
paused-latch timing changed but Paused()'s signature and contract
("a turn has been requested and not yet answered") are unchanged.
No BREAKING.md entry: nothing in this release breaks a contract.
Co-Authored-By: Claude Sonnet 5 <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
2 participants
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.
Summary
Reproduced and fixed four of the seven reliability gaps reported against
forge ship --agent-modein 1.10.2 — each confirmed live against a realbuilt binary in a disposable scratch repo, not just from reading the code.
writeTestArtifactsWithContextwrote the static RED placeholder to
tests/*.test.tswhenever the LLM/bridge call errored for any reason, including
ErrAgentTurn(a pause,not a failure). Once written, the placeholder permanently satisfied
allTestArtifactsExist, so the checkpoint never called the generatoragain — a host agent's real, later-submitted answer silently never
reached the file, while the checkpoint still reported "N test file(s)
found; all 4 named artifacts present". Fixed by propagating the pause
before writing anything for the branch that paused, so the exists-guard
stays false and the real answer lands correctly on the next run.
agentbridge.Bridgeonly latchedpausedinsideLookup, never when a pending turn was merely reloadedfrom disk. A checkpoint that skipped the bridge entirely this run (an
"artefact already exists" short-circuit, a language branch with no LLM
call) left
pausedfalse for the whole process, so both theper-checkpoint gate and the final "render the pending turn" check
(which also key off
Paused()) silently failed to fire while an earlierturn sat unanswered. Fixed by latching
pausedimmediately on load;replay is unaffected since hash/ordinal hits are checked before the
pausedcheck inLookup.checkSpectreated "no description/--name+ 2+ existing spec dirs" as a soft "ok, pass a description" andlet the whole pipeline run against an undefined slug. Now hard-fails,
names the ambiguous candidates, and demands
--name. A singleunambiguous spec dir is unaffected.
.gitignore(bug chore(deps): bump actions/download-artifact from 4 to 8 #5, directly causing bug chore(deps): bump actions/checkout from 4 to 6 #2's symptom):all three copies of the managed block (
codemod.canonicalGitignoreBlock,codemod.defaultMarkerBody,cmddoctor.canonicalGiSnippet) were missing.forge/agent/,.forge/.snapshots/,.forge/learned/,.forge/trash/,.forge/token-ledger.jsonl. Confirmed live: this let the bridge's ownbookkeeping show up in
git status, which the Code checkpoint'scountChangedFilesthen counted as "N modified file(s)" for a run wherethe only real source file was untouched.
Bug #7 (
forge ship --agent-mode -j | headhanging) could not bereproduced despite repeated attempts (small/large payloads, byte- and
line-truncated pipes) on macOS and is intentionally left out of this PR
rather than guess-fixed.
Test plan
go build ./...go vet ./...go test ./...(full suite, unmodified tests all still pass)against the pre-fix code and pass against the fix:
-
TestOpen_RestoredPendingTurnLatchesPausedImmediately(agentbridge)-
TestWriteTestArtifacts_PausedTurnDoesNotFabricatePlaceholder(cmdship)-
TestCheckSpec_NoDescription_MultipleSpecs_HardFails+TestCheckSpec_NoDescription_SingleSpec_StillOK(cmdship)-
TestCanonicalGitignoreBlock_CoversForgeScratchState(codemod) +matching tests in
cmddoctorand a secondcodemodliteralforge ship --agent-modethrough spec → arch → debate → test,submitting real answers via
forge agent submit, confirming thereal content (not a placeholder) landed in
tests/*.test.tsand that.forge/agent/no longer appears ingit statusafterforge init🤖 Generated with Claude Code