From b0f1722662eb5f51a31b09908291eb01048d4ec3 Mon Sep 17 00:00:00 2001
From: Tom Boucher <trekkie@nomorestars.com>
Date: Wed, 5 Aug 2026 16:52:37 -0400
Subject: [PATCH 1/6] fix(#2969): ratchet completed_plans up for gap-closure
 plans under deriveProgressKeys (#3091)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* test(#2969): completed_plans must ratchet up for gap-closure plans

Failing-first regression: when deriveProgressKeys=true (cmdStatePlannedPhase's
opt-in), applyStatePreservation restores completed_plans to its pre-growth
curated value, so gap-closure plans that complete never increment it —
STATE.md shows completed_plans < total_plans forever even though every PLAN
has a SUMMARY. Three cases: the ratchet-up (derived 54 > curated 50), the
ratchet-down protection (derived 47 < curated 50 keeps curated), and the
body-only write protection (no deriveProgressKeys → wholesale restore).

The existing #2440 test covers the case where derived < curated (ratchet
holds); this adds the missing case where derived > curated (ratchet must
release upward).

* fix(#2969): ratchet completed_plans/plases up under deriveProgressKeys

applyStatePreservation's deriveProgressKeys path (cmdStatePlannedPhase's
opt-in) let total_plans/total_phases take the derived value but restored
completed_plans/completed_phases to their pre-growth curated value — so
gap-closure plans that completed after the plan count grew never incremented
them, leaving STATE.md at completed_plans < total_plans forever (every PLAN
had a SUMMARY).

Extend the deriveProgressKeys exclusion to also let completed_plans and
completed_phases take the derived value, but ratcheted UP only (never derive
downward past curated) — preserving the #3242 curated-progress protection
for cases unrelated to plan-count growth (e.g. a deleted SUMMARY). percent
takes the derived value (the resync already recomputed it from disk counts).

Scoped to deriveProgressKeys (plan-phase only); body-only writes
(state.update/patch without the flag) keep the full #3242 wholesale restore.

* fix(#2969): also take derived percent under deriveProgressKeys

Isolated-review blocker: percent fell into the else branch and was
overwritten with the stale curated value, contradicting the inline comment
and leaving STATE.md incoherent (e.g. completed_plans:54/total_plans:54 at
percent:93). Skip percent in the ratchet loop so the derived (resync-
recomputed) value survives.

* chore(#2969): add changeset fragment

* chore(#2969): backfill changeset PR number 3091

---------

Co-authored-by: sim <sim@local>
---
 .changeset/lucky-tunas-leap.md  |  5 +++
 src/state-transition.cts        | 19 +++++++++++-
 tests/state-transition.test.cjs | 55 +++++++++++++++++++++++++++++++++
 3 files changed, 78 insertions(+), 1 deletion(-)
 create mode 100644 .changeset/lucky-tunas-leap.md

diff --git a/.changeset/lucky-tunas-leap.md b/.changeset/lucky-tunas-leap.md
new file mode 100644
index 000000000..c548f2d6d
--- /dev/null
+++ b/.changeset/lucky-tunas-leap.md
@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 3091
+---
+**`progress.completed_plans` no longer stays pinned after a gap-closure cycle** — when plan-phase re-planned a phase and added gap-closure plans, `total_plans` corrected upward but `completed_plans` was restored to its pre-growth value, so STATE.md showed `completed_plans < total_plans` permanently even after every plan (including the gap-closure ones) was summarized. `completed_plans` and `completed_phases` now ratchet up to the disk-derived count under the plan-phase progress opt-in (never deriving downward, preserving the curated-progress ratchet for unrelated edits). (#2969)
diff --git a/src/state-transition.cts b/src/state-transition.cts
index 9db1f96f3..362180719 100644
--- a/src/state-transition.cts
+++ b/src/state-transition.cts
@@ -195,8 +195,25 @@ export function applyStatePreservation(input: StatePreservationInput): StatePres
       const derived = (postFm['progress'] ?? {}) as Record<string, unknown>;
       const merged: Record<string, unknown> = { ...derived };
       if (curated) {
+        // #2440: total_plans and total_phases always take the derived value.
+        // #2969: completed_plans and completed_phases take the derived value
+        // when it is GREATER than the curated value (gap-closure plans that
+        // completed after the plan count grew) — ratcheting UP only, never
+        // deriving downward (preserves the #3242 curated-progress protection
+        // for cases unrelated to plan-count growth, e.g. a deleted SUMMARY).
+        // percent also takes the derived value — the resync recomputed it from
+        // disk counts, and a stale curated percent would be incoherent against
+        // the ratcheted-up completed counts (e.g. 54/54 at 93%).
+        const ratchetUpKeys = new Set(['completed_plans', 'completed_phases']);
         for (const [key, value] of Object.entries(curated)) {
-          if (key !== 'total_plans' && key !== 'total_phases') {
+          if (key === 'total_plans' || key === 'total_phases' || key === 'percent') continue;
+          if (ratchetUpKeys.has(key)) {
+            const derivedNum = typeof derived[key] === 'number' ? derived[key] : -Infinity;
+            const curatedNum = typeof value === 'number' ? value : -Infinity;
+            // Take the derived value only when it ratchets up; else keep curated.
+            if (derivedNum > curatedNum) continue;
+            merged[key] = value;
+          } else {
             merged[key] = value;
           }
         }
diff --git a/tests/state-transition.test.cjs b/tests/state-transition.test.cjs
index 6b7409a2d..89b95e3b4 100644
--- a/tests/state-transition.test.cjs
+++ b/tests/state-transition.test.cjs
@@ -1340,6 +1340,61 @@ describe('ADR-1769 #1796: applyStatePreservation — table-driven post-sync cons
       'total_plans equality → derived value (identity)');
   });
 
+  test('#2969: deriveProgressKeys=true — completed_plans ratchets UP when disk count exceeds curated (gap-closure plans completed)', () => {
+    // Gap-closure scenario: a phase had 50 plans all summarized (completed_plans: 50),
+    // then 4 gap-closure plans were added (total_plans -> 54) and all 4 got SUMMARYs.
+    // Disk scan now counts 54 summaries. The curated completed_plans (50) must
+    // ratchet UP to the derived value (54), not stay pinned at 50 — otherwise
+    // completed_plans < total_plans forever even though every plan is summarized.
+    const curated = { progress: { total_plans: 54, completed_plans: 50, total_phases: 2, completed_phases: 1, percent: 93 } };
+    const r = applyStatePreservation({
+      preFm: curated,
+      preFmSnapshot: curated,
+      postFm: { progress: { total_plans: 54, completed_plans: 54, total_phases: 2, completed_phases: 1, percent: 100 } },
+      resync: false,
+      deriveProgressKeys: true,
+      ...untouched,
+    });
+    assert.equal(r.postFm.progress.total_plans, 54, 'total_plans takes derived value');
+    assert.equal(r.postFm.progress.completed_plans, 54,
+      'completed_plans must ratchet UP to derived (54 > curated 50 — gap-closure plans completed) (#2969)');
+    assert.equal(r.postFm.progress.percent, 100,
+      'percent must reflect the true completion fraction (54/54) (#2969)');
+  });
+
+  test('#2969 ratchet-down protection: deriveProgressKeys=true keeps curated when disk count < curated', () => {
+    // The ratchet must only go UP. If the disk count is somehow LOWER than
+    // curated (e.g. a SUMMARY was deleted), keep the curated value — do not
+    // derive downward. (#3242 curated-progress protection, scoped to deriveProgressKeys.)
+    const curated = { progress: { total_plans: 54, completed_plans: 50, percent: 93 } };
+    const r = applyStatePreservation({
+      preFm: curated,
+      preFmSnapshot: curated,
+      postFm: { progress: { total_plans: 54, completed_plans: 47, percent: 87 } },
+      resync: false,
+      deriveProgressKeys: true,
+      ...untouched,
+    });
+    assert.equal(r.postFm.progress.completed_plans, 50,
+      'completed_plans must NOT derive downward (47 < curated 50) — ratchet-up only (#2969/#3242)');
+  });
+
+  test('#2969 body-only write protection: deriveProgressKeys absent keeps wholesale restore', () => {
+    // state.update/patch (no deriveProgressKeys flag) must keep the full #3242
+    // wholesale curated restore — completed_plans never moves for a body-only edit.
+    const curated = { progress: { total_plans: 54, completed_plans: 50, percent: 93 } };
+    const r = applyStatePreservation({
+      preFm: curated,
+      preFmSnapshot: curated,
+      postFm: { progress: { total_plans: 54, completed_plans: 54, percent: 100 } },
+      resync: false,
+      // deriveProgressKeys NOT set — body-only write path
+      ...untouched,
+    });
+    assert.equal(r.postFm.progress.completed_plans, 50,
+      'body-only write must keep curated completed_plans (no deriveProgressKeys) (#2969/#3242)');
+  });
+
   test('progress: NOT restored when transition re-derives from disk (resync=true) — sync/advancePlan/completePhase path', () => {
     const recomputed = { progress: { total_phases: 5, completed_phases: 1, percent: 20 } };
     const r = applyStatePreservation({

From 2979f2a994b157b003b231fe41a6db4f28794dce Mon Sep 17 00:00:00 2001
From: Tom Boucher <trekkie@nomorestars.com>
Date: Wed, 5 Aug 2026 18:22:14 -0400
Subject: [PATCH 2/6] fix(#2978): add structural validation to roadmap validate
 (#3092)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* test(#2978): roadmap validate must perform structural validation

Failing-first: roadmap validate returns {"warnings":[]} (exit 0) for every
input — empty file, garbage, missing file, truncated frontmatter — because it
performs no structural validation and its one opt-in check (W021 milestone-
prefix) is off by default. Six cases: empty, garbage, missing, truncated
frontmatter, well-formed (no false positive), BOM-prefixed (not corruption).

* fix(#2978): add structural validation to roadmap validate

roadmap validate returned {"warnings":[]} (exit 0) for every input — empty
file, garbage, missing file, truncated frontmatter — because it performed no
structural validation and its one opt-in check (W021 milestone-prefix) is
off by default. A verb named validate that cannot produce a negative result
provides false assurance.

Add four structural checks, each producing a coded warning {code, message}:
- V001: file missing/unreadable (was silent success)
- V002: empty/whitespace-only
- V003: malformed frontmatter (unterminated --- fence; BOM-tolerant per #3057)
- V004: no recognizable phase entries (no ### Phase N: heading)
Keep the existing W021 milestone-prefix check as-is. Exit non-zero via
ExitError(1) when warnings are non-empty, per the documented contract
('exits non-zero on any error or warning'). Well-formed roadmaps (incl.
BOM-prefixed, CRLF) still validate cleanly with warnings: [] and exit 0.

* test(#2978): update W021 tests for non-zero exit on warnings

Two existing W021 tests asserted roadmap validate exits 0 even with warnings
('roadmap validate should exit 0 even with warnings') — that was the bug.
#2978 made validate exit non-zero on any warning (per its documented
contract). Updated both mismatch-case tests to expect success===false and
parse the JSON output from the failure path (stdout is written before the
ExitError throw).

* chore(#2978): add changeset fragment

* chore(#2978): backfill changeset PR number 3092

---------

Co-authored-by: sim <sim@local>
---
 .changeset/silly-moles-rally.md              |  5 ++
 src/roadmap-command-router.cts               | 53 ++++++++++++--
 tests/milestone-prefixed-convention.test.cjs |  6 +-
 tests/roadmap.test.cjs                       | 76 ++++++++++++++++++++
 4 files changed, 131 insertions(+), 9 deletions(-)
 create mode 100644 .changeset/silly-moles-rally.md

diff --git a/.changeset/silly-moles-rally.md b/.changeset/silly-moles-rally.md
new file mode 100644
index 000000000..f1a88560b
--- /dev/null
+++ b/.changeset/silly-moles-rally.md
@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 3092
+---
+**`roadmap validate` now performs real structural validation** — it previously returned `{"warnings":[]}` (exit 0) for every input including empty files, garbage text, and missing files, providing false assurance. It now checks file existence/readability, emptiness, frontmatter well-formedness, and the presence of at least one phase entry, exiting non-zero on any warning (per its documented contract). The existing opt-in milestone-prefix consistency check is preserved. (#2978)
diff --git a/src/roadmap-command-router.cts b/src/roadmap-command-router.cts
index 7ef26d5a1..7b54d8cf4 100644
--- a/src/roadmap-command-router.cts
+++ b/src/roadmap-command-router.cts
@@ -21,6 +21,9 @@ const { planningDir } = planningWorkspace;
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import configLoaderMod = require('./config-loader.cjs');
 const { loadConfig } = configLoaderMod;
+// eslint-disable-next-line @typescript-eslint/no-require-imports
+import cliExitMod = require('./cli-exit.cjs');
+const { ExitError } = cliExitMod;
 
 // ─── Types ────────────────────────────────────────────────────────────────────
 
@@ -143,11 +146,43 @@ function routeRoadmapCommand({ roadmap, args, cwd, raw, error }: RouteRoadmapCom
       'annotate-dependencies': () => roadmap.cmdRoadmapAnnotateDependencies(cwd, args[2], raw),
       'validate': () => {
         const roadmapPath = path.join(planningDir(cwd), 'ROADMAP.md');
-        let roadmapContent = '';
+        const warnings: Array<{ code: string; message: string }> = [];
+
+        // #2978: structural validation. A verb named "validate" that cannot
+        // produce a negative result provides false assurance. Before the
+        // opt-in milestone-prefix check, verify the file is structurally a
+        // roadmap at all.
+        let roadmapContent: string;
         try {
           roadmapContent = fs.readFileSync(roadmapPath, 'utf8');
         } catch {
-          // ROADMAP.md missing — return empty warnings
+          // ROADMAP.md missing — not silent success.
+          warnings.push({ code: 'V001', message: 'ROADMAP.md not found or unreadable' });
+          const result = { warnings };
+          process.stdout.write(raw ? JSON.stringify(result) : JSON.stringify(result, null, 2));
+          throw new ExitError(1);
+        }
+
+        // Empty or whitespace-only.
+        if (roadmapContent.trim() === '') {
+          warnings.push({ code: 'V002', message: 'ROADMAP.md is empty' });
+        }
+
+        // Malformed frontmatter — a `---` opener with no matching closer.
+        // Tolerate a leading BOM (#3057) before the fence.
+        const contentAfterBom = roadmapContent.replace(/^\uFEFF/, '');
+        if (contentAfterBom.startsWith('---')) {
+          const closeMatch = contentAfterBom.slice(3).match(/\r?\n---\s*(\r?\n|$)/);
+          if (!closeMatch) {
+            warnings.push({ code: 'V003', message: 'ROADMAP.md frontmatter is malformed (unterminated --- fence)' });
+          }
+        }
+
+        // No recognizable phase structure — at least one `### Phase N:` heading.
+        // Mirrors the phase-heading pattern used across roadmap-parser.cts.
+        const hasPhaseEntry = /^#{2,4}\s*Phase\s+\S/im.test(roadmapContent);
+        if (!hasPhaseEntry && !warnings.some((w) => w.code === 'V002')) {
+          warnings.push({ code: 'V004', message: 'ROADMAP.md contains no recognizable phase entries (no "### Phase N:" headings)' });
         }
 
         // W021 only fires when phase_id_convention is explicitly 'milestone-prefixed'.
@@ -173,13 +208,17 @@ function routeRoadmapCommand({ roadmap, args, cwd, raw, error }: RouteRoadmapCom
             }
           }
         }
-        const warnings = (convention === 'milestone-prefixed')
-          ? checkW021(roadmapContent)
-          : [];
+        if (convention === 'milestone-prefixed') {
+          warnings.push(...checkW021(roadmapContent));
+        }
 
         const result = { warnings };
-        if (raw) process.stdout.write(JSON.stringify(result));
-        else process.stdout.write(JSON.stringify(result, null, 2));
+        process.stdout.write(raw ? JSON.stringify(result) : JSON.stringify(result, null, 2));
+        // #2978: exit non-zero on any warning, per the documented contract
+        // ("exits non-zero on any error or warning").
+        if (warnings.length > 0) {
+          throw new ExitError(1);
+        }
       },
       'upgrade': () => {
         const dryRun = !args.includes('--apply');
diff --git a/tests/milestone-prefixed-convention.test.cjs b/tests/milestone-prefixed-convention.test.cjs
index f54e42a1a..455b42632 100644
--- a/tests/milestone-prefixed-convention.test.cjs
+++ b/tests/milestone-prefixed-convention.test.cjs
@@ -89,7 +89,8 @@ describe('W021 — milestone-prefixed phase ID convention', () => {
     ]);
 
     const result = runGsdTools(['roadmap', 'validate'], tmpDir);
-    assert.ok(result.success, `roadmap validate should exit 0 even with warnings: ${result.error}`);
+    // #2978: validate now exits non-zero on any warning (per its documented contract).
+    assert.strictEqual(result.success, false, `roadmap validate must exit non-zero on W021 warnings: ${result.error}`);
 
     const out = JSON.parse(result.output);
     assert.ok(Array.isArray(out.warnings), 'output.warnings should be an array');
@@ -219,7 +220,8 @@ describe('W021 — milestone-prefixed phase ID convention', () => {
     ]);
 
     const result = runGsdTools(['roadmap', 'validate'], tmpDir);
-    assert.ok(result.success, `roadmap validate failed: ${result.error}`);
+    // #2978: validate now exits non-zero on any warning (per its documented contract).
+    assert.strictEqual(result.success, false, `roadmap validate must exit non-zero on W021 warnings: ${result.error}`);
 
     const out = JSON.parse(result.output);
     const w021 = (out.warnings || []).filter(w => w.code === 'W021');
diff --git a/tests/roadmap.test.cjs b/tests/roadmap.test.cjs
index 8b9e60d6f..9b518f39b 100644
--- a/tests/roadmap.test.cjs
+++ b/tests/roadmap.test.cjs
@@ -3612,3 +3612,79 @@ describe('bug #1103 — annotate-dependencies preserves newline before Plans: he
 });
   });
 }
+
+// ─────────────────────────────────────────────────────────────────────────────
+// bug #2978: roadmap validate returns {"warnings":[]} for every input
+// ─────────────────────────────────────────────────────────────────────────────
+
+describe('bug #2978: roadmap validate performs structural validation', () => {
+  test('empty (zero-byte) ROADMAP.md → non-empty warnings, non-zero exit', () => {
+    const tmpDir = createTempProject('gsd-2978-empty-');
+    try {
+      fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), '');
+      const result = runGsdTools(['roadmap', 'validate', '--raw'], tmpDir);
+      assert.strictEqual(result.success, false, 'empty file must exit non-zero');
+      const payload = JSON.parse(result.output);
+      assert.ok(Array.isArray(payload.warnings) && payload.warnings.length > 0,
+        `empty file must produce non-empty warnings; got: ${JSON.stringify(payload)}`);
+    } finally { cleanup(tmpDir); }
+  });
+
+  test('garbage/non-roadmap text → non-empty warnings, non-zero exit', () => {
+    const tmpDir = createTempProject('gsd-2978-garbage-');
+    try {
+      fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'not a roadmap at all');
+      const result = runGsdTools(['roadmap', 'validate', '--raw'], tmpDir);
+      assert.strictEqual(result.success, false, 'garbage must exit non-zero');
+      const payload = JSON.parse(result.output);
+      assert.ok(payload.warnings.length > 0, 'garbage must produce warnings');
+    } finally { cleanup(tmpDir); }
+  });
+
+  test('missing ROADMAP.md → non-empty warnings, non-zero exit', () => {
+    const tmpDir = createTempProject('gsd-2978-missing-');
+    try {
+      // createTempProject creates .planning/phases but no ROADMAP.md — don't write one
+      const result = runGsdTools(['roadmap', 'validate', '--raw'], tmpDir);
+      assert.strictEqual(result.success, false, 'missing file must exit non-zero');
+      const payload = JSON.parse(result.output);
+      assert.ok(payload.warnings.length > 0, 'missing file must produce warnings');
+    } finally { cleanup(tmpDir); }
+  });
+
+  test('truncated frontmatter (unterminated ---) → non-empty warnings', () => {
+    const tmpDir = createTempProject('gsd-2978-trunc-');
+    try {
+      fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'),
+        '---\nmilestone: v1.0\n### Phase 1: Setup\n');
+      const result = runGsdTools(['roadmap', 'validate', '--raw'], tmpDir);
+      assert.strictEqual(result.success, false, 'truncated frontmatter must exit non-zero');
+      const payload = JSON.parse(result.output);
+      assert.ok(payload.warnings.length > 0, 'truncated frontmatter must produce warnings');
+    } finally { cleanup(tmpDir); }
+  });
+
+  test('well-formed roadmap → warnings: [], exit 0 (no false positive)', () => {
+    const tmpDir = createTempProject('gsd-2978-good-');
+    try {
+      fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'),
+        '# Roadmap\n\n## v1.0\n\n### Phase 1: Foundation\n**Goal:** setup\n');
+      const result = runGsdTools(['roadmap', 'validate', '--raw'], tmpDir);
+      assert.ok(result.success, `well-formed roadmap must exit 0; got: ${result.error}`);
+      const payload = JSON.parse(result.output);
+      assert.deepStrictEqual(payload.warnings, [], 'well-formed roadmap must have no warnings');
+    } finally { cleanup(tmpDir); }
+  });
+
+  test('BOM-prefixed well-formed roadmap → warnings: [], exit 0 (not corruption)', () => {
+    const tmpDir = createTempProject('gsd-2978-bom-');
+    try {
+      fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'),
+        '\uFEFF# Roadmap\n\n### Phase 1: Foundation\n**Goal:** setup\n');
+      const result = runGsdTools(['roadmap', 'validate', '--raw'], tmpDir);
+      assert.ok(result.success, `BOM-prefixed roadmap must exit 0; got: ${result.error}`);
+      const payload = JSON.parse(result.output);
+      assert.deepStrictEqual(payload.warnings, [], 'BOM is not corruption');
+    } finally { cleanup(tmpDir); }
+  });
+});

From 955655407c3986886ce55131d10776005dbee145 Mon Sep 17 00:00:00 2001
From: Tom Boucher <trekkie@nomorestars.com>
Date: Wed, 5 Aug 2026 18:57:10 -0400
Subject: [PATCH 3/6] fix(#2979): document ExitError plain-text carve-out in
 json-errors.md (#3093)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fix(#2979): document ExitError plain-text carve-out in json-errors.md

The JSON-errors doc claimed every error emits a structured JSON envelope,
but usage errors (ExitError) intentionally emit plain text with their own
exit code (src/cli-exit.cts:36-39 catches ExitError before the envelope
branch). Anyone following the doc's 'always parse stderr as JSON' guidance
against a usage error got a parse failure.

Amended the Wire format + Overview + Writing tests sections to scope the
structured envelope to non-ExitError failures, stated the carve-out with a
pointer to cli-exit.cts, and scoped the JSON-parse instruction to the
envelope branch. Added a characterization test pinning both paths together
(ExitError -> plain text + own code; non-ExitError -> JSON envelope) so the
code cannot drift toward the doc's prior overstated claim. No runtime
change — the test passes before and after the doc edit.

Re-scoped per maintainer triage: the smart-entry --json part is already
satisfied (shipped payload exposes the command token); only the doc
correction + characterization test remain.

* chore(#2979): backfill changeset PR number 3093

---------

Co-authored-by: sim <sim@local>
---
 .changeset/kind-sloths-climb.md |  5 ++++
 docs/json-errors.md             | 24 +++++++++++++++----
 tests/cli-exit.test.cjs         | 41 +++++++++++++++++++++++++++++++--
 3 files changed, 64 insertions(+), 6 deletions(-)
 create mode 100644 .changeset/kind-sloths-climb.md

diff --git a/.changeset/kind-sloths-climb.md b/.changeset/kind-sloths-climb.md
new file mode 100644
index 000000000..5269c7130
--- /dev/null
+++ b/.changeset/kind-sloths-climb.md
@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 3093
+---
+**`docs/json-errors.md` now documents the ExitError plain-text carve-out** — the page previously claimed every CLI error emits a structured JSON envelope on stderr, but usage errors (ExitError) intentionally emit plain text with their own exit code. The structured-envelope guidance is now scoped to non-usage failures, with the carve-out stated explicitly and a characterization test pinning both paths. (#2979)
diff --git a/docs/json-errors.md b/docs/json-errors.md
index c12c058c6..2da5a9869 100644
--- a/docs/json-errors.md
+++ b/docs/json-errors.md
@@ -2,11 +2,12 @@
 
 ## Overview
 
-`gsd-tools` supports a **JSON error mode** that emits all errors as structured
+`gsd-tools` supports a **JSON error mode** that emits most errors as structured
 JSON objects on stderr instead of free-form text.  This is the recommended
 surface for tests and tooling that need to assert on error types without
 grepping raw text (see `CONTRIBUTING.md` — "Prohibited: Raw Text Matching on
-Test Outputs").
+Test Outputs"). Usage errors are an intentional exception — see the
+`ExitError` carve-out below.
 
 ## Activating
 
@@ -37,6 +38,20 @@ Fields:
 | `reason`  | string  | Typed reason code from the taxonomy below. |
 | `message` | string  | Human-readable description (may change; do not assert on it). |
 
+### `ExitError` carve-out (plain text, not JSON)
+
+Usage errors and explicit exit-code signals take a **different path**: they
+throw `ExitError` (`src/cli-exit.cts`), which `runMain` catches *before* the
+JSON-envelope branch. An `ExitError` writes its `message` as **plain text**
+to stderr (not a JSON object) and exits with the error's own `code` (which
+may differ from 1). This is intentional — usage messages are operator-facing
+prose, not structured failures.
+
+If you are testing a usage/flag error, do **not** parse stderr as JSON;
+assert on the exit code and (if needed) the plain-text message. The
+"parse stderr as JSON" guidance below applies only to the structured-envelope
+branch (non-`ExitError` failures).
+
 ## Error code taxonomy
 
 Codes are frozen constants in `gsd-core/bin/lib/core.cjs` under
@@ -99,8 +114,9 @@ text (unstable).
 
 ## Writing tests
 
-Always parse stderr with `JSON.parse` and assert on typed fields.  Never use
-`.includes()`, `.match()`, or regex on the raw error string.
+For **non-usage** errors (the structured-envelope branch), parse stderr with
+`JSON.parse` and assert on typed fields.  Never use `.includes()`, `.match()`,
+or regex on the raw error string.
 
 ```js
 // CORRECT: parse then assert on typed field
diff --git a/tests/cli-exit.test.cjs b/tests/cli-exit.test.cjs
index 03a1de939..21823bd14 100644
--- a/tests/cli-exit.test.cjs
+++ b/tests/cli-exit.test.cjs
@@ -183,11 +183,18 @@ describe('runMain', () => {
 describe('regressions', () => {
   /** Spawn a one-shot script that sets json-error mode and calls runMain with a throwing handler. */
   function spawnJsonErrorRun({ jsonMode, errorType = 'TypeError', message = 'unexpected boom' } = {}) {
+    // ExitError lives in the same module as runMain; import it when the test
+    // wants to exercise the ExitError carve-out path. ExitError takes (code, message).
+    const isExitError = errorType === 'ExitError';
+    const destructure = isExitError ? '{ runMain, ExitError }' : '{ runMain }';
+    const throwExpr = isExitError
+      ? `new ExitError(1, ${JSON.stringify(message)})`
+      : `new ${errorType}(${JSON.stringify(message)})`;
     const script = `
       const io = require(${JSON.stringify(IO_PATH)});
-      const { runMain } = require(${JSON.stringify(BUILT_CLI_EXIT_PATH)});
+      const ${destructure} = require(${JSON.stringify(BUILT_CLI_EXIT_PATH)});
       io.setJsonErrorMode(${jsonMode ? 'true' : 'false'});
-      runMain(() => { throw new ${errorType}(${JSON.stringify(message)}); });
+      runMain(() => { throw ${throwExpr}; });
       setImmediate(() => {});
     `;
     return spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8' });
@@ -245,5 +252,35 @@ describe('regressions', () => {
         `expected "unexpected boom" in stderr, got: ${stderrTrimmed.slice(0, 200)}`
       );
     });
+
+    // #2979: characterization test pinning the two error paths under json-errors
+    // mode. The structured envelope covers non-ExitError failures; ExitError
+    // (usage errors) intentionally emits plain text with its own exit code.
+    // Both halves asserted together so the code cannot drift toward the doc's
+    // prior overstated claim that EVERY error emits JSON.
+    test('#2979: ExitError emits plain text (not JSON) even under --json-errors; non-ExitError emits the envelope', () => {
+      // ExitError path: plain text, own exit code, NOT a JSON object.
+      const exitResult = spawnJsonErrorRun({
+        jsonMode: true,
+        errorType: 'ExitError',
+        message: 'Usage: gsd-tools <command> [args]',
+      });
+      assert.strictEqual(exitResult.status, 1, 'ExitError exits with its code');
+      const exitStderr = exitResult.stderr.trim();
+      let exitParsed = null;
+      try { exitParsed = JSON.parse(exitStderr); } catch { /* expected — plain text */ }
+      assert.strictEqual(exitParsed, null,
+        `ExitError must emit plain text, not JSON; got: ${exitStderr.slice(0, 200)}`);
+      assert.ok(exitStderr.includes('Usage'),
+        `ExitError plain-text message must reach stderr; got: ${exitStderr.slice(0, 200)}`);
+
+      // Non-ExitError path: structured JSON envelope.
+      const envResult = spawnJsonErrorRun({ jsonMode: true });
+      assert.strictEqual(envResult.status, 1);
+      const envParsed = JSON.parse(envResult.stderr.trim());
+      assert.strictEqual(envParsed.ok, false);
+      assert.strictEqual(envParsed.reason, 'sdk_fail_fast');
+      assert.ok(envParsed.message, 'envelope must carry a message');
+    });
   });
 });

From 2843e25bf36cb80d5edd26f39e8cb61fabdd68f7 Mon Sep 17 00:00:00 2001
From: Tom Boucher <trekkie@nomorestars.com>
Date: Wed, 5 Aug 2026 19:27:07 -0400
Subject: [PATCH 4/6] fix(#2988): local changeset/docs lint falls back to next,
 not main (#3095)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fix(#2988): local changeset/docs lint falls back to next, not main

Both scripts/changeset/lint.cjs and scripts/lint-docs-required.cjs resolved
their diff base as GITHUB_BASE_REF || 'main'. GITHUB_BASE_REF is set only in
GitHub Actions; locally it falls back to 'main' (the release branch), which
lags far behind 'next' (the integration branch every PR targets). The
oversized diff range swept in every changeset fragment merged since the last
release, so the lint passed on the first fragment it saw regardless of
whether the current PR authored it — structurally vacuous.

Changed the fallback to 'next' (DEFAULT_BASE constant, exported from both
scripts for parity). CI behavior unchanged (GITHUB_BASE_REF is set there).
Added a parity test asserting both lints resolve the same base.

* chore(#2988): backfill changeset PR number 3095

---------

Co-authored-by: sim <sim@local>
---
 .changeset/witty-yaks-munch.md |  5 +++++
 scripts/changeset/lint.cjs     | 11 +++++++++--
 scripts/lint-docs-required.cjs | 10 +++++++++-
 tests/changeset-lint.test.cjs  | 19 ++++++++++++++++++-
 4 files changed, 41 insertions(+), 4 deletions(-)
 create mode 100644 .changeset/witty-yaks-munch.md

diff --git a/.changeset/witty-yaks-munch.md b/.changeset/witty-yaks-munch.md
new file mode 100644
index 000000000..458bf18eb
--- /dev/null
+++ b/.changeset/witty-yaks-munch.md
@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 3095
+---
+**Local `lint:changeset` and `lint:docs-required` now diff against `next` instead of `main`** — the local fallback was the release branch (`main`), which lags far behind the integration branch (`next`), so the lint always passed by finding fragments from other already-merged PRs in the oversized diff range. The local invocation now matches the base CI uses. (#2988)
diff --git a/scripts/changeset/lint.cjs b/scripts/changeset/lint.cjs
index 9fe975093..933d5fe55 100755
--- a/scripts/changeset/lint.cjs
+++ b/scripts/changeset/lint.cjs
@@ -12,6 +12,10 @@
  * Tests assert on the typed verdict, never on free text.
  */
 
+// #2988: the repo's integration/default branch — the base every PR targets.
+// Used as the local fallback when GITHUB_BASE_REF is unset (CI sets it).
+const DEFAULT_BASE = 'next';
+
 const LINT_REASON = Object.freeze({
   OK_FRAGMENT_PRESENT: 'ok_fragment_present',
   OK_OPT_OUT_LABEL: 'ok_opt_out_label',
@@ -80,7 +84,10 @@ function main() {
       labels = (event.pull_request?.labels || []).map((l) => l.name);
     } catch { /* fall through */ }
   }
-  const base = process.env.GITHUB_BASE_REF || 'main';
+  // #2988: local fallback must match the repo's integration branch (`next`),
+  // not the release branch (`main`). CI sets GITHUB_BASE_REF explicitly; the
+  // fallback only fires locally, where `next` is the base every PR targets.
+  const base = process.env.GITHUB_BASE_REF || DEFAULT_BASE;
   let changedFiles = [];
   try {
     // Use execFileSync with an argv array — the base ref is interpolated
@@ -145,4 +152,4 @@ function main() {
 
 if (require.main === module) runMain(main);
 
-module.exports = { evaluateLint, LINT_REASON, OPT_OUT_LABEL, isUserFacing, isFragment };
+module.exports = { evaluateLint, LINT_REASON, OPT_OUT_LABEL, isUserFacing, isFragment, DEFAULT_BASE };
diff --git a/scripts/lint-docs-required.cjs b/scripts/lint-docs-required.cjs
index a6580cb4a..3bcce28ca 100755
--- a/scripts/lint-docs-required.cjs
+++ b/scripts/lint-docs-required.cjs
@@ -16,6 +16,10 @@
 const { parseFragment, FRAGMENT_ERROR } = require('./changeset/parse.cjs');
 const { ExitError, runMain } = require('./lib/cli-exit.cjs');
 
+// #2988: the repo's integration/default branch — the base every PR targets.
+// Used as the local fallback when GITHUB_BASE_REF is unset (CI sets it).
+const DEFAULT_BASE = 'next';
+
 const LINT_REASON = Object.freeze({
   OK_NO_TRIGGERING_FRAGMENTS: 'ok_no_triggering_fragments',
   OK_DOCS_UPDATED: 'ok_docs_updated',
@@ -149,7 +153,10 @@ function main() {
     } catch { /* fall through */ }
   }
 
-  const base = process.env.GITHUB_BASE_REF || 'main';
+  // #2988: local fallback must match the repo's integration branch (`next`),
+  // not the release branch (`main`). CI sets GITHUB_BASE_REF explicitly; the
+  // fallback only fires locally, where `next` is the base every PR targets.
+  const base = process.env.GITHUB_BASE_REF || DEFAULT_BASE;
   let changedFiles = [];
   try {
     // execFileSync with argv — no shell, so a malicious GITHUB_BASE_REF
@@ -216,6 +223,7 @@ module.exports = {
   OPT_OUT_LABEL,
   TRIGGERING_TYPES,
   FRAGMENT_ERROR,
+  DEFAULT_BASE,
   isFragmentPath,
   isDocsFile,
   isExemptFragment,
diff --git a/tests/changeset-lint.test.cjs b/tests/changeset-lint.test.cjs
index dff7ab16a..6775d142d 100644
--- a/tests/changeset-lint.test.cjs
+++ b/tests/changeset-lint.test.cjs
@@ -8,7 +8,8 @@ const fs = require('node:fs');
 const os = require('node:os');
 const cp = require('node:child_process');
 
-const { evaluateLint, LINT_REASON } = require(path.join(__dirname, '..', 'scripts', 'changeset', 'lint.cjs'));
+const { evaluateLint, LINT_REASON, DEFAULT_BASE: CHANGESET_DEFAULT_BASE } = require(path.join(__dirname, '..', 'scripts', 'changeset', 'lint.cjs'));
+const { DEFAULT_BASE: DOCS_DEFAULT_BASE } = require(path.join(__dirname, '..', 'scripts', 'lint-docs-required.cjs'));
 
 const ROOT = path.join(__dirname, '..');
 const LINT_SCRIPT = path.join(ROOT, 'scripts', 'changeset', 'lint.cjs');
@@ -297,3 +298,19 @@ describe('changeset lint: main() end-to-end wiring (#1006)', () => {
     assert.ok(!deletedEntry, `deleted fragment must not appear in failures, got: ${JSON.stringify(failures)}`);
   });
 });
+
+// ─── #2988: local base fallback parity ──────────────────────────────────────
+
+describe('#2988: changeset + docs lints resolve the same local base fallback', () => {
+  test('both lints default to `next` (the integration branch), not `main`', () => {
+    assert.strictEqual(CHANGESET_DEFAULT_BASE, 'next',
+      `changeset lint DEFAULT_BASE must be 'next', got '${CHANGESET_DEFAULT_BASE}'`);
+    assert.strictEqual(DOCS_DEFAULT_BASE, 'next',
+      `docs lint DEFAULT_BASE must be 'next', got '${DOCS_DEFAULT_BASE}'`);
+  });
+
+  test('both lints resolve the same base given the same environment (parity)', () => {
+    assert.strictEqual(CHANGESET_DEFAULT_BASE, DOCS_DEFAULT_BASE,
+      `the two lints must not diverge on base resolution: changeset='${CHANGESET_DEFAULT_BASE}' docs='${DOCS_DEFAULT_BASE}'`);
+  });
+});

From 2a77e50dafd5439d590ddc5ea44a9f47235c97a2 Mon Sep 17 00:00:00 2001
From: Tom Boucher <trekkie@nomorestars.com>
Date: Wed, 5 Aug 2026 20:03:03 -0400
Subject: [PATCH 5/6] fix(#2989): anchor code-review diff-base grep to
 phase-mention convention (#3096)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fix(#2989): anchor code-review diff-base grep to phase-mention convention

The diff-base fallback in code-review.md used git log --grep with a bare
phase number (unanchored substring), matching version strings, dates, issue
refs, and other phases' numbers. tail -1 took the oldest match — routinely
a commit from months or years before the phase existed. The fail-closed
branch was dead code because a bare digit almost always matches something.

Changed --grep to '[Pp]hase N\b' with --extended-regexp, anchoring to the
phase-mention convention. When no commit genuinely references the phase,
the derivation yields empty and the fail-closed warning fires (now
reachable). All three consumers (Tier 3 file scope, fallow pre-pass, agent
context) use the same corrected value.

* chore(#2989): backfill changeset PR number 3096

---------

Co-authored-by: sim <sim@local>
---
 .changeset/proud-pumas-bark.md                           | 5 +++++
 gsd-core/workflows/code-review.md                        | 9 +++++++--
 .../2989-code-review-anchored-diff-base.json             | 6 ++++++
 3 files changed, 18 insertions(+), 2 deletions(-)
 create mode 100644 .changeset/proud-pumas-bark.md
 create mode 100644 tests/emitted-drift-acks/2989-code-review-anchored-diff-base.json

diff --git a/.changeset/proud-pumas-bark.md b/.changeset/proud-pumas-bark.md
new file mode 100644
index 000000000..1cbbe9665
--- /dev/null
+++ b/.changeset/proud-pumas-bark.md
@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 3096
+---
+**`/gsd-code-review` no longer picks a wrong diff base from unanchored commit-message grep** — the diff-base fallback searched all commit messages for the bare phase number as a substring, matching version strings, dates, and issue refs, then took the oldest match. The grep is now anchored to the phase-mention convention (`Phase N` with a word boundary), so the fail-closed branch is reachable when no commit genuinely references the phase. (#2989)
diff --git a/gsd-core/workflows/code-review.md b/gsd-core/workflows/code-review.md
index b64264738..9adc0f639 100644
--- a/gsd-core/workflows/code-review.md
+++ b/gsd-core/workflows/code-review.md
@@ -236,8 +236,13 @@ Additionally, whenever a reliable diff base is available, cross-check the SUMMAR
 against the diff and warn about (then add) any changed files the SUMMARY extractor did not
 surface — so a partial SUMMARY result can no longer silently mask the rest of the phase.
 ```bash
-# Compute diff base from phase commits — fail closed if no reliable base found
-PHASE_COMMITS=$(git log --oneline --all --grep="${PADDED_PHASE}" --format="%H" 2>/dev/null)
+# Compute diff base from phase commits — fail closed if no reliable base found.
+# #2989: anchor the grep to the phase-mention convention ("Phase N" / "phase N"
+# with a word boundary) so a bare digit substring doesn't match version strings,
+# dates, issue refs, or other phases' numbers. With --extended-regexp, \b is
+# a word boundary. When no commit genuinely references the phase, this yields
+# empty and the fail-closed warning below actually fires.
+PHASE_COMMITS=$(git log --oneline --all --grep="[Pp]hase ${PADDED_PHASE}\b" --extended-regexp --format="%H" 2>/dev/null)
 DIFF_BASE=""
 if [ -n "$PHASE_COMMITS" ]; then
   DIFF_BASE=$(echo "$PHASE_COMMITS" | tail -1)^
diff --git a/tests/emitted-drift-acks/2989-code-review-anchored-diff-base.json b/tests/emitted-drift-acks/2989-code-review-anchored-diff-base.json
new file mode 100644
index 000000000..3452f97c2
--- /dev/null
+++ b/tests/emitted-drift-acks/2989-code-review-anchored-diff-base.json
@@ -0,0 +1,6 @@
+{
+  "version": 1,
+  "paths": {
+    "code-review.md": "#2989: the diff-base fallback's git log --grep was changed from an unanchored bare phase number (matching version strings, dates, issue refs) to an anchored '[Pp]hase N\\b' with --extended-regexp, plus a 5-line comment explaining the anchor. Makes the fail-closed branch reachable when no commit genuinely references the phase."
+  }
+}

From 5628eddddae659f2b82242057dff72f792e5112f Mon Sep 17 00:00:00 2001
From: Tom Boucher <trekkie@nomorestars.com>
Date: Wed, 5 Aug 2026 20:48:46 -0400
Subject: [PATCH 6/6] fix(#2991): route /gsd command output through Pi's
 display shape (#3097)

* fix(#2991): route /gsd command output through Pi's display shape

The registerCommand('gsd') handler returned a bare string, which Pi's
ExtensionAPI does not display. Changed all return paths to Pi's structured
{ content: [{ type: 'text', text }] } shape, matching the gsd_invoke tool's
proven output contract. Updated 2 reachability tests that asserted the old
bare-string return shape.

* chore(#2991): backfill changeset PR number 3097

---------

Co-authored-by: sim <sim@local>
---
 .changeset/proud-eagles-chatter.md       |  5 +++++
 pi/gsd.cjs                               | 11 ++++++++---
 tests/pi-extension-reachability.test.cjs | 17 +++++++++++------
 3 files changed, 24 insertions(+), 9 deletions(-)
 create mode 100644 .changeset/proud-eagles-chatter.md

diff --git a/.changeset/proud-eagles-chatter.md b/.changeset/proud-eagles-chatter.md
new file mode 100644
index 000000000..49f9b3016
--- /dev/null
+++ b/.changeset/proud-eagles-chatter.md
@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 3097
+---
+**`/gsd` commands in Pi now display their output** — the command handler returned output as a bare string, which Pi's ExtensionAPI silently dropped. It now returns Pi's structured `{ content: [{ type: 'text', text }] }` display shape (matching the `gsd_invoke` tool's proven contract), so success output and error messages are visible. (#2991)
diff --git a/pi/gsd.cjs b/pi/gsd.cjs
index c30bd738a..9a3fdc702 100644
--- a/pi/gsd.cjs
+++ b/pi/gsd.cjs
@@ -287,11 +287,16 @@ module.exports = function gsdPiExtension(pi) {
       try {
         ({ dispatchGsdCommand } = require(path.join(GSD_CORE, 'bin', 'lib', 'shell-command-projection.cjs')));
       } catch (e) {
-        return `GSD engine unavailable: ${e && e.message ? e.message : String(e)}`;
+        // #2991: return the structured { content } shape Pi's ExtensionAPI
+        // displays, not a bare string (which Pi silently drops).
+        return { content: [{ type: 'text', text: `GSD engine unavailable: ${e && e.message ? e.message : String(e)}` }] };
       }
       const result = dispatchGsdCommand({ family, subcommand, args: rest, cwd });
-      if (result.ok) return result.stdout;
-      return `GSD error: ${result.stderr || result.stdout || `dispatch failed (exit ${result.code})`}`;
+      // #2991: match gsd_invoke's proven output shape so Pi actually displays it.
+      const text = result.ok
+        ? result.stdout
+        : `GSD error: ${result.stderr || result.stdout || `dispatch failed (exit ${result.code})`}`;
+      return { content: [{ type: 'text', text }] };
     },
   });
 
diff --git a/tests/pi-extension-reachability.test.cjs b/tests/pi-extension-reachability.test.cjs
index 8d51730cb..c2208f6b5 100644
--- a/tests/pi-extension-reachability.test.cjs
+++ b/tests/pi-extension-reachability.test.cjs
@@ -63,23 +63,28 @@ test('REACHABILITY: the /gsd handler dispatches a real family through gsd-tools.
   const dir = createTempDir();
   try {
     const result = await pi._recorded.commands['gsd'].handler('progress json', { cwd: dir });
-    assert.equal(typeof result, 'string', '/gsd handler returns a string result');
-    const parsed = JSON.parse(result);
+    // #2991: handler returns { content: [{ type: 'text', text }] } (Pi's display shape), not a bare string.
+    assert.ok(result && Array.isArray(result.content) && result.content[0].type === 'text',
+      `/gsd handler must return Pi's display shape { content: [{ type: 'text', text }] }; got: ${JSON.stringify(result).slice(0, 200)}`);
+    const parsed = JSON.parse(result.content[0].text);
     assert.equal(typeof parsed.percent, 'number', '/gsd dispatch reached gsd-tools.cjs for real (the engine was reached)');
   } finally {
     cleanup(dir);
   }
 });
 
-test('REACHABILITY: an unknown family surfaces a clear GSD error string, not a throw', async () => {
+test('REACHABILITY: an unknown family surfaces a clear GSD error, not a throw', async () => {
   const pi = mockPi();
   gsdPiExtension(pi);
   const dir = createTempDir();
   try {
     const result = await pi._recorded.commands['gsd'].handler('no-such-family-8675309', { cwd: dir });
-    assert.equal(typeof result, 'string');
-    assert.match(result, /GSD error:/);
-    assert.match(result, /no-such-family-8675309|Unknown command/);
+    // #2991: handler returns { content: [{ type: 'text', text }] } (Pi's display shape).
+    assert.ok(result && Array.isArray(result.content) && result.content[0].type === 'text',
+      `error result must carry Pi's display shape; got: ${JSON.stringify(result).slice(0, 200)}`);
+    const text = result.content[0].text;
+    assert.match(text, /GSD error:/);
+    assert.match(text, /no-such-family-8675309|Unknown command/);
   } finally {
     cleanup(dir);
   }