fix(ship): stop agent-mode Test/Code fabrication and undetected stale turns

.github/workflows/ci-gates.yml

@@ -19,7 +19,7 @@ permissions:
   contents: read
 
 env:
-  GO_VERSION: '1.25'
+  GO_VERSION: '1.26'
   CGO_ENABLED: '0'
 
 jobs:
@@ -73,13 +73,25 @@ jobs:
           CHANGED=$(git diff --name-only "origin/$BASE"...HEAD 2>/dev/null || git diff --name-only HEAD~1...HEAD)
           # For each changed _test.go, find the corresponding non-test file
           # and check that the test commit is not strictly *after* the production commit.
+          #
+          # Both `git log` calls are scoped to this PR's own commit range
+          # ("origin/$BASE..HEAD") and take the *first* commit in that range
+          # (oldest, via `tail -1` — `git log` lists newest-first) rather than
+          # the most recent commit in the file's entire history. A prior
+          # version compared each file's all-time-latest-touch timestamp,
+          # which meant ANY test-only PR against a mature file (no production
+          # change at all) was flagged as a TDD violation purely because the
+          # test's last historical edit postdated the production file's last
+          # historical edit — true for almost every established file. Scoping
+          # to this PR's range means an untouched production file naturally
+          # produces no commits (PROD_DATE empty) and is correctly skipped.
           VIOLATIONS=0
           while IFS= read -r f; do
             if [[ "$f" != *_test.go ]]; then continue; fi
             PROD="${f%_test.go}.go"
             if [[ ! -f "$PROD" ]]; then continue; fi  # No paired prod file — skip
-            TEST_DATE=$(git log --follow --format='%ct' -- "$f" | head -1)
-            PROD_DATE=$(git log --follow --format='%ct' -- "$PROD" | head -1)
+            TEST_DATE=$(git log --follow --format='%ct' "origin/$BASE..HEAD" -- "$f" | tail -1)
+            PROD_DATE=$(git log --follow --format='%ct' "origin/$BASE..HEAD" -- "$PROD" | tail -1)
             if [[ -n "$TEST_DATE" && -n "$PROD_DATE" && "$TEST_DATE" -gt "$PROD_DATE" ]]; then
               echo "::warning file=$f::Test file $f was committed AFTER production file $PROD (TDD violation)"
               VIOLATIONS=$((VIOLATIONS + 1))

CHANGELOG.md

@@ -4,6 +4,17 @@ All notable changes to forge will be documented in this file. Format follows [Ke
 
 ## [Unreleased]
 
+## [1.10.3] — 2026-08-26 — Agent mode stopped fabricating Test/Code success behind a real pause
+
+### Fixed
+
+- **`forge ship --agent-mode` could fabricate a passing Test checkpoint while silently discarding the host agent's real answer.** `writeTestArtifactsWithContext` wrote the static RED placeholder stub to `tests/*.test.ts` whenever an LLM/bridge call returned *any* error — including `ErrAgentTurn`, a pause rather than a failure. Once written, that placeholder permanently satisfied `allTestArtifactsExist`, so `checkTest` never called the generator again on a later run: a host agent's real, later-submitted answer for the same operation sat unused in the bridge's response store forever, while the checkpoint reported "N test file(s) found; all 4 named artifacts present" against content nobody had reviewed. Confirmed live against a real binary: submitting `expect(ping()).toBe('pong')` for `ship:test:unit` never reached `tests/*.test.ts` because the placeholder from the paused attempt had already tripped the exists-guard. `writeTestArtifactsWithContext` (and its `writeTestArtifacts`/`llmGoStub` callers) now return 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.
+- **A still-pending host-agent turn from an earlier run could go undetected, letting the pipeline fabricate results past it.** `agentbridge.Bridge.loadPending` restored a 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 called `Lookup` at all this run (an "artefact already exists" skip, a language branch with no LLM call) left `paused` false for the whole process — which meant both the run loop's per-checkpoint gate and the final "render the pending turn" check in `forge ship` (both keyed on `Paused()`, not `Pending()` alone) silently failed to fire while an earlier turn (e.g. an `arch-parallel-debate` role) sat unanswered on disk the entire time. `loadPending` now latches `paused` immediately; replay is unaffected since `Lookup` checks its hash/ordinal indexes before it ever checks `paused`.
+- **No description/`--name` with more than one existing spec directory silently ran the whole pipeline against an undefined slug.** `checkSpec` treated this as a soft "ok, pass a description" and let every later checkpoint fall back to its own "no description" branch independently, producing a full pipeline's worth of fabricated, spec-less output instead of one clear error. Now hard-fails, names every ambiguous candidate, and demands `--name`. A single unambiguous spec directory is unaffected.
+- **The managed `.gitignore` block never listed forge's own agent-mode/scratch state**, so `.forge/agent/` (bridge session/pending/response files), `.forge/.snapshots/`, `.forge/learned/`, `.forge/trash/`, and `.forge/token-ledger.jsonl` all showed up as untracked in `git status`. Confirmed live: an agent-mode run left `git status --porcelain` reporting the bridge's own bookkeeping as changes, which the Code checkpoint'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. Fixed in all three copies of the managed block (`codemod.canonicalGitignoreBlock`, `codemod.defaultMarkerBody`, `cmddoctor.canonicalGiSnippet`).
+- **CI: the `M1-27` tests-precede-code gate flagged any test-only PR touching a mature file as a TDD violation.** It compared each file's all-time-latest-touch commit timestamp across the entire repo history rather than this PR's own commits — true for nearly every established file the moment its test gets a maintenance update with no accompanying production change. Both `git log` calls are now scoped to `origin/$BASE..HEAD` and take the oldest commit in that range, so an untouched production file correctly produces no commits (skipped) while a genuine same-PR "production code first, test added later" violation is still caught.
+- **CI: the `M2-17` perf benchmark gate's `benchstat` install failed on `ci-gates.yml`'s Go 1.25 pin** once `golang.org/x/perf@latest` started requiring Go ≥ 1.26. `ci.yml` and `nightly.yml` were already on Go 1.26, matching `go.mod`'s `toolchain go1.26.6` (bumped 2026-08-20 for stdlib CVE fixes) — `ci-gates.yml` alone had drifted. Bumped to match.
+
 ## [1.10.2] — 2026-08-21 — Agent mode stopped pausing: a bridge miss was treated as an LLM failure
 
 ### Fixed

internal/agentbridge/agentbridge.go

@@ -163,17 +163,15 @@ type Bridge struct {
 	seen map[string]int
 
 	pending *Turn
-	// paused latches once a turn has been requested. Every later Lookup in the
-	// same process returns ErrTurnRequired without overwriting the pending
-	// turn, so one `forge ship --agent-mode` invocation always yields exactly
-	// one question for the host agent to answer.
+	// paused latches once a turn has been requested — either freshly, on a
+	// miss inside Lookup, or immediately at Open time when pending.json
+	// already held an unanswered turn from a previous run (see loadPending).
+	// Every later Lookup in the same process returns ErrTurnRequired without
+	// overwriting the pending turn, so one `forge ship --agent-mode`
+	// invocation always yields exactly one question for the host agent to
+	// answer — and never silently proceeds past a turn it already owes.
 	paused bool
 
-	// restored records that pending was loaded from disk rather than created
-	// in this process — i.e. a previous run already showed the host agent a
-	// question it has not answered yet. See Lookup for why that matters.
-	restored bool
-
 	// StrictReplay disables the ordinal fallback: a prompt whose hash is not
 	// recorded is always re-asked, even when an ordinal-keyed answer exists.
 	StrictReplay bool
@@ -293,24 +291,21 @@ func (b *Bridge) Lookup(operation, checkpoint, model, system, user string, maxTo
 			return resp.Content, nil
 		}
 	}
-	if b.paused {
-		return "", ErrTurnRequired
-	}
 	// An unanswered turn from a previous run outranks whatever this run would
-	// have asked. Re-running while paused is normal — a driver that loses its
-	// place just runs `forge ship --agent-mode` again — and the pipeline can
-	// legitimately arrive at a *different* prompt the second time, because a
-	// checkpoint that could not generate its artefact may still have
-	// scaffolded a stub, moving the next run onto the review path instead of
-	// the generate path.
-	//
-	// Letting that overwrite pending.json would misfile the answer: the host
-	// agent is holding a prompt it was shown, and `forge agent submit`
+	// have asked. loadPending latches paused as soon as a pending turn is
+	// read from disk (before Lookup is ever called), so a restored turn is
+	// already caught by the b.paused check above — this is why: re-running
+	// while paused is normal — a driver that loses its place just runs
+	// `forge ship --agent-mode` again — and the pipeline can legitimately
+	// arrive at a *different* prompt the second time, because a checkpoint
+	// that could not generate its artefact may still have scaffolded a stub,
+	// moving the next run onto the review path instead of the generate path.
+	// Overwriting pending.json in that case would misfile the answer: the
+	// host agent is holding a prompt it was shown, and `forge agent submit`
 	// records against whatever is pending at submit time. It would file a
 	// generated spec as if it were a review. So the original question stands
 	// until it is answered or the session is reset.
-	if b.restored && b.pending != nil {
-		b.paused = true
+	if b.paused {
 		return "", ErrTurnRequired
 	}
 	b.pending = &Turn{
@@ -357,7 +352,6 @@ func (b *Bridge) Fulfil(content string) (Turn, error) {
 	b.byOrdinal[resp.Ordinal] = resp
 	b.pending = nil
 	b.paused = false
-	b.restored = false
 	b.nextSeq = t.Seq + 1
 	b.session.TurnsFilled++
 	if err := b.clearPending(); err != nil {
@@ -383,7 +377,6 @@ func (b *Bridge) Reset() error {
 	b.seen = map[string]int{}
 	b.pending = nil
 	b.paused = false
-	b.restored = false
 	b.drifted = 0
 	b.nextSeq = 0
 	b.session = Session{Name: b.session.Name, CreatedAt: time.Now().UTC()}
@@ -513,12 +506,20 @@ func (b *Bridge) loadPending() error {
 		return fmt.Errorf("parse pending turn: %w", err)
 	}
 	b.pending = &t
-	// Loading does not latch paused on its own: replay must still work, so
-	// the run has to be allowed to proceed through every already-answered
-	// prompt before it reaches the unanswered one. The latch happens in
-	// Lookup, on the first miss, and preserves this turn rather than
-	// replacing it.
-	b.restored = true
+	// Latching paused here (rather than waiting for Lookup's first miss) does
+	// not break replay: Lookup checks byHash/byOrdinal before it ever checks
+	// paused, so every already-answered prompt still replays normally even
+	// though paused is already true. What this closes is a fabrication path —
+	// a checkpoint that never calls Lookup at all this run (a stub short-
+	// circuit, a language branch with no LLM call, an "artefact already
+	// exists" skip) used to leave paused false for the entire process, so
+	// forge ship --agent-mode could run straight through Test/Breakdown/Code
+	// to a real failure or success while a turn from an earlier run — e.g. a
+	// still-unanswered arch-parallel-debate role — sat unanswered on disk the
+	// whole time, because both the run loop's per-checkpoint gate and the
+	// final "render the pending turn" check in the ship command key off this
+	// same paused flag, not off Pending() alone.
+	b.paused = true
 	return nil
 }
 

internal/agentbridge/agentbridge_test.go

@@ -485,3 +485,65 @@ func TestSetFeature_SwitchingFeatureResetsStaleSession(t *testing.T) {
 		t.Fatalf("expected a fresh pause for the new feature, got content=%q err=%v", content, lookupErr)
 	}
 }
+
+// ── Regression: a restored pending turn must latch paused immediately ────────
+//
+// Root cause this guards: loadPending used to leave b.paused false until some
+// later Lookup call happened to re-encounter the exact same hash/ordinal as
+// the restored turn. A checkpoint that never calls Lookup at all in a given
+// run — a static-stub short-circuit, a language branch with no LLM call, an
+// "artefact already exists" skip — left paused false for the whole process
+// even though a real, unanswered turn was sitting on disk. That let
+// `forge ship --agent-mode` run straight through later checkpoints (and the
+// final "render the pending turn" check in the ship command, which also
+// gates on Paused()) while a genuinely-owed turn from an earlier run went
+// unanswered the entire time. Confirmed live: an arch-parallel-debate turn
+// left pending, then Test/Breakdown/Code fabricated results in the same run.
+func TestOpen_RestoredPendingTurnLatchesPausedImmediately(t *testing.T) {
+	t.Parallel()
+	root := t.TempDir()
+
+	b := mustOpen(t, root, DefaultSession)
+	if b.Paused() {
+		t.Fatal("a fresh bridge with nothing pending must not start paused")
+	}
+	if _, err := b.Lookup("ship:arch:generate", "arch", "", "sys", "usr", 100); !errors.Is(err, ErrTurnRequired) {
+		t.Fatalf("expected a pause, got %v", err)
+	}
+	if !b.Paused() {
+		t.Fatal("a fresh miss must latch paused in the same process")
+	}
+
+	// Simulate a brand-new process (e.g. a second `forge ship --agent-mode`
+	// invocation) that has not yet called Lookup at all this run.
+	b2 := mustOpen(t, root, DefaultSession)
+	if !b2.Paused() {
+		t.Fatal("Open must latch paused immediately when pending.json already holds an unanswered turn — " +
+			"otherwise a checkpoint that never calls Lookup this run can sail past the still-owed turn")
+	}
+	turn, ok := b2.Pending()
+	if !ok {
+		t.Fatal("the restored turn must still be reported by Pending()")
+	}
+	if turn.Operation != "ship:arch:generate" {
+		t.Fatalf("restored the wrong turn: %q", turn.Operation)
+	}
+
+	// Replay through an already-answered prompt must still work even though
+	// paused is true from the moment of Open — hash/ordinal hits are checked
+	// before the paused branch in Lookup.
+	if _, err := b2.Fulfil("the answer"); err != nil {
+		t.Fatalf("Fulfil: %v", err)
+	}
+	b3 := mustOpen(t, root, DefaultSession)
+	if b3.Paused() {
+		t.Fatal("a bridge with no pending turn on disk must not start paused")
+	}
+	content, err := b3.Lookup("ship:arch:generate", "arch", "", "sys", "usr", 100)
+	if err != nil {
+		t.Fatalf("replay of the answered turn must succeed, got err=%v", err)
+	}
+	if content != "the answer" {
+		t.Fatalf("replay returned %q, want the recorded answer", content)
+	}
+}

internal/cli/cmddoctor/doctor.go

@@ -199,6 +199,11 @@ const canonicalGiSnippet = `# forge:gitignore:start
 # forge-version: managed
 .forge/scratch/
 .forge/cache/
+.forge/.snapshots/
+.forge/agent/
+.forge/learned/
+.forge/trash/
+.forge/token-ledger.jsonl
 *.tmp
 *.bak
 __pycache__/

internal/cli/cmddoctor/doctor_test.go

@@ -227,3 +227,25 @@ func TestCheckLLMProviderLive_NoProviderDetected(t *testing.T) {
 		t.Error("Hint must not be empty when no provider is detected")
 	}
 }
+
+// TestCanonicalGiSnippet_CoversForgeScratchState guards the same content this
+// duplicate must stay in sync with (internal/codemod.canonicalGitignoreBlock)
+// — see that package's TestCanonicalGitignoreBlock_CoversForgeScratchState
+// for the root cause (untracked .forge/agent, .forge/.snapshots, etc. being
+// miscounted by ship's countChangedFiles as real code changes).
+func TestCanonicalGiSnippet_CoversForgeScratchState(t *testing.T) {
+	t.Parallel()
+	for _, pattern := range []string{
+		".forge/scratch/",
+		".forge/cache/",
+		".forge/.snapshots/",
+		".forge/agent/",
+		".forge/learned/",
+		".forge/trash/",
+		".forge/token-ledger.jsonl",
+	} {
+		if !strings.Contains(canonicalGiSnippet, pattern) {
+			t.Errorf("canonicalGiSnippet missing %q", pattern)
+		}
+	}
+}

internal/cli/cmdship/agent_mode_test.go

@@ -31,6 +31,8 @@ package cmdship
 
 import (
 	"bytes"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -207,3 +209,75 @@ func TestNoBridge_PipelineUnaffected(t *testing.T) {
 		t.Fatal("a run without agent mode must behave exactly as before")
 	}
 }
+
+// ── Regression: a paused Test checkpoint must not fabricate placeholder
+// artefacts that later shadow the real host-agent answer ─────────────────────
+//
+// Root cause this guards: writeTestArtifactsWithContext used to write the
+// static RED placeholder stub to tests/*.test.ts unconditionally whenever the
+// LLM call returned any error — including ErrAgentTurn, a pause rather than a
+// failure. Once written, that placeholder satisfied allTestArtifactsExist
+// forever, so checkTest never called writeTestArtifacts again on a later run
+// — the host agent's real, later-submitted answer for the same operation sat
+// unused in the bridge's response store, and the checkpoint reported
+// "N test file(s) found; all 4 named artifacts present" against content
+// nobody had actually reviewed. Confirmed live against a real binary: a
+// submitted `expect(ping()).toBe('pong')` answer for ship:test:unit never
+// reached tests/*.test.ts because the placeholder from the paused attempt
+// had already tripped the exists-guard.
+func TestWriteTestArtifacts_PausedTurnDoesNotFabricatePlaceholder(t *testing.T) {
+	t.Parallel()
+	root := t.TempDir()
+	bridge, err := agentbridge.Open(root, agentbridge.DefaultSession)
+	if err != nil {
+		t.Fatalf("open bridge: %v", err)
+	}
+	pipe := newLLMPipeAgent(root, bridge)
+	slug := "add-ping"
+	unitPath := filepath.Join(root, "tests", slug+".test.ts")
+
+	// First attempt: ship:test:unit is unanswered, so this must pause —
+	// and, critically, must not write ANY of the 4 named artifacts.
+	paths, err := writeTestArtifactsWithContext(root, slug, "add ping function returning pong", "", TestFrameworkContext{}, pipe)
+	if !IsAgentTurn(err) {
+		t.Fatalf("expected an agent-turn pause on the first attempt, got err=%v paths=%+v", err, paths)
+	}
+	if _, statErr := os.Stat(unitPath); statErr == nil {
+		t.Fatalf("paused generation must not write a placeholder to %s — doing so permanently shadows the real answer", unitPath)
+	}
+
+	// Answer ship:test:unit, ship:test:integration, ship:test:rls in turn —
+	// each is a distinct operation, so each pauses once before the next
+	// becomes reachable.
+	realUnit := "```typescript\nexpect(ping()).toBe('pong');\n```"
+	for _, answer := range []string{realUnit, "integration answer", "rls answer"} {
+		turn, ok := bridge.Pending()
+		if !ok {
+			t.Fatalf("expected a pending turn before submitting %q", answer)
+		}
+		if _, ferr := bridge.Fulfil(answer); ferr != nil {
+			t.Fatalf("Fulfil(%q): %v", answer, ferr)
+		}
+		paths, err = writeTestArtifactsWithContext(root, slug, "add ping function returning pong", "", TestFrameworkContext{}, pipe)
+		if turn.Operation != "ship:test:rls" && !IsAgentTurn(err) {
+			t.Fatalf("expected the next operation to pause after answering %q, got err=%v", answer, err)
+		}
+	}
+
+	if err != nil {
+		t.Fatalf("all three sub-generations were answered; expected no error, got %v", err)
+	}
+	if paths.UnitTest == "" {
+		t.Fatal("UnitTest path must be set once generation completes")
+	}
+	content, readErr := os.ReadFile(paths.UnitTest)
+	if readErr != nil {
+		t.Fatalf("read %s: %v", paths.UnitTest, readErr)
+	}
+	if !strings.Contains(string(content), "ping()") {
+		t.Fatalf("tests/%s.test.ts must contain the host agent's real answer, got:\n%s", slug, content)
+	}
+	if strings.Contains(string(content), "intentionally failing — complete implementation") {
+		t.Fatalf("tests/%s.test.ts still holds the static placeholder instead of the real answer:\n%s", slug, content)
+	}
+}