fix(ship): agent-mode pauses were treated as LLM failures, stubbing artefacts - #41
Merged
Merged
Conversation
…rtefacts Every checkpoint that generates an artefact (spec/arch/test/breakdown/code) funnelled generateWithValidation's error straight into its generic "LLM failed -> write a stub" branch without checking IsAgentTurn first. In --agent-mode, a bridge miss (a pause owed to the host agent, not a failure) was therefore indistinguishable from a real provider error: the checkpoint overwrote the artefact with a stub template and logged a false failure. A second instance of the same root cause lived in the per-checkpoint post-processing loop: because the pause reported cp.Status == "ok", the completion-marker writer ran anyway and wrote placeholder content into the checkpoint's own primary artefact file (e.g. arch.md) before the host agent had answered anything. The next invocation then saw that file as "already done" and moved on, repeating the mistake on the next checkpoint - the observed symptom being a single run silently stamping broken stubs across arch/test/breakdown/code instead of pausing once per checkpoint. Also fixes: reusing the default agent-mode session across two different features let the ordinal-fallback replay (keyed only on "operation#N", with no feature scoping) silently serve one feature's recorded answer into an unrelated feature's run. Bridge.SetFeature now resets stale session state on a feature switch instead of leaving it to collide. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
go1.26.5 carries 4 known stdlib vulnerabilities (GO-2026-6218, GO-2026-6090, GO-2026-5972, GO-2026-5026) reachable from code this binary actually calls (net/url, crypto/tls, encoding/asn1, net/http), all fixed in go1.26.6. Unrelated to the agent-mode fix in the prior commit; bumped here only because the pre-push govulncheck gate correctly blocked on it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… files M1-27 (tests-precede-code gate) flags a _test.go file committed after its paired same-named .go file. TestAgentMode_ArchPauseDoesNotStubTheArtefact and TestAgentMode_SwitchingFeatureResetsStaleSession exercise ship.go/arch.go and agentbridge.go (all touched by the prior commit) but were added to agent_mode_test.go, whose pair (agent_mode.go) was untouched — a false positive from the gate's naive file-pairing, not a real ordering violation. Moved to ship_test.go and agentbridge_test.go respectively, which pair with files actually modified in this change. 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
forge ship --agent-modehad a real bug: after answering the first (spec) turn, it silently auto-advanced through arch/test/breakdown/code using broken LLM-failure stubs instead of pausing for reasoning each time, overwrote a submitted spec.md-equivalent artefact behind the scenes, and cross-contaminated state when a session was reused across features.Root causes, both in the deterministic plane, not the reasoning plane:
IsAgentTurn.generateWithValidation's error was funnelled straight into the generic "LLM failed → write a stub, log a failure" branch incheckSpec/checkArch/checkTest/checkBreakdown/checkCode. A bridge miss (a pause owed to the host agent) was therefore indistinguishable from a real provider error.cp.Status == "ok", the completion-marker writer (writes<checkpoint>.mdfor any non-failcheckpoint) ran anyway and wrote placeholder "Status: warning / Evidence: none" content into the checkpoint's own primary artefact file (e.g.arch.md) before the host agent had answered anything. The next invocation then saw that file as "already done" and moved to the next checkpoint — repeating the mistake, which is how a single run could stamp broken stubs across several checkpoints while only ever showing the user the first turn.operation#N, with no feature scoping. Reusing the default session across two unrelated features let the second feature's Nth call to an operation (e.g.ship:qa-verify:generate) silently replay the first feature's recorded answer.Fixes
agentPauseCheckpointhelper checksIsAgentTurnat every LLM-generation call site and, on a pause, leaves the artefact untouched and marksCheckpoint.AgentPaused = true.AgentPausedis set.Bridge.SetFeatureresets stale session state when the session already belonged to a different feature/slug;forge ship --agent-modeprints a note when this happens.Test plan
go build ./...go test ./...(full suite, all green)TestAgentMode_ArchPauseDoesNotStubTheArtefact,TestAgentMode_SwitchingFeatureResetsStaleSessionininternal/cli/cmdship/agent_mode_test.gogovulncheck ./...clean after the toolchain bumpCo-Authored-By: Claude Sonnet 5 noreply@anthropic.com