diff --git a/.changeset/rapid-cats-glide.md b/.changeset/rapid-cats-glide.md new file mode 100644 index 000000000..98cd94c32 --- /dev/null +++ b/.changeset/rapid-cats-glide.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2700 +--- +**Verification-status next-step commands now use the command surface each runtime actually installs** — on a Codex project, a phase blocked on verification suggested `/gsd:execute-phase`, which Codex does not install; the correct form is `$gsd-execute-phase`. The routing table stored hard-coded, deprecated colon-form strings with no runtime context, so `phase complete` and `query verification.status` relayed them verbatim to every runtime. All four routed states (missing, unknown, gaps_found, stale) now project through the shared runtime formatter. (#2617) diff --git a/src/init.cts b/src/init.cts index 6d0a0d6b7..cf8d27206 100644 --- a/src/init.cts +++ b/src/init.cts @@ -165,22 +165,6 @@ interface PhaseCompletionProjection { verification_next_command: string; } -function verificationNextCommand( - status: string, - phaseNumber: string, - slashRuntime: string, -): string { - if (status === 'gaps_found') { - return `${formatGsdSlash('plan-phase', slashRuntime) as string} ${phaseNumber} --gaps`; - } - if (status === 'human_needed' || status === 'stale') { - return `${formatGsdSlash('verify-work', slashRuntime) as string} ${phaseNumber}`; - } - if (status === 'missing' || status === 'unknown') { - return `${formatGsdSlash('execute-phase', slashRuntime) as string} ${phaseNumber}`; - } - return ''; -} function projectCompletionStatus( implementationComplete: boolean, @@ -201,8 +185,14 @@ function buildPhaseCompletionProjection( ): PhaseCompletionProjection { const implementationComplete = planCount > 0 && summaryCount >= planCount; const phaseFullDir = phaseDir ? path.join(cwd, phaseDir) : ''; + // #2617: ONE verification-routing seam. init used to re-derive next_command + // from the status with its own projector, which had drifted from the router's + // table — it appended the phase number and answered `human_needed`; the table + // did neither. The router now owns both the content and the runtime + // projection, and init passes the phase number it already knows (its phaseDir + // is unresolved in some branches, where the router could not derive one). const verificationStatus = implementationComplete - ? readVerificationStatus(phaseFullDir) + ? readVerificationStatus(phaseFullDir, { runtime: slashRuntime, phaseNumber }) : { status: 'not_required', next_action: '', next_command: '' }; const projectedVerificationStatus = verificationStatus.status; const projectedVerificationAction = verificationStatus.next_action; @@ -216,11 +206,7 @@ function buildPhaseCompletionProjection( phase_complete: phaseComplete, completion_status: projectCompletionStatus(implementationComplete, verificationPassed), verification_next_action: projectedVerificationAction, - verification_next_command: verificationNextCommand( - projectedVerificationStatus, - phaseNumber, - slashRuntime, - ), + verification_next_command: verificationStatus.next_command, }; } diff --git a/src/phase.cts b/src/phase.cts index 08dacdbaa..9537df3c8 100644 --- a/src/phase.cts +++ b/src/phase.cts @@ -1760,7 +1760,10 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { let isLastPhase = true; const verificationBlocked = withPlanningLock(cwd, () => { - const verificationStatus = readVerificationStatus(phaseFullDir); + // #2617: pass the project's runtime so the blocked-completion error below + // suggests the command surface this runtime actually installs + // ($gsd-… on Codex) rather than a hard-coded Claude-style string. + const verificationStatus = readVerificationStatus(phaseFullDir, { runtime: resolveRuntime(cwd) }); if (verificationStatus.status !== 'passed') { return verificationStatus; } diff --git a/src/verification.cts b/src/verification.cts index 4d89b51ce..9a769e452 100644 --- a/src/verification.cts +++ b/src/verification.cts @@ -38,6 +38,7 @@ import frontmatterMod = require('./frontmatter.cjs'); // eslint-disable-next-line @typescript-eslint/no-require-imports -- plan-scan.cjs is an export= CommonJS module import scanPhasePlans = require('./plan-scan.cjs'); import { execGit } from './shell-command-projection.cjs'; +import { formatGsdSlash, resolveRuntime } from './runtime-slash.cjs'; const { output, error } = io; const { extractPhaseToken } = phaseId; @@ -70,6 +71,12 @@ interface VerificationRoute { * * For 'gaps_found', next_command is built at call time in readVerificationStatus * by substituting the phase number — it is NOT stored as a function in the table. + * + * #2617: `next_command` here holds a BARE command name (`execute-phase`), never a + * prefixed one. Every return path projects it through `formatGsdSlash` with the + * caller's runtime, so Codex sees `$gsd-execute-phase` and slash-hyphen runtimes + * see `/gsd-execute-phase`. Storing a prefixed literal is what leaked the + * hard-coded (and deprecated) `/gsd:` colon form to every runtime. */ const VERIFICATION_ROUTING_TABLE: Record = { passed: { @@ -86,7 +93,12 @@ const VERIFICATION_ROUTING_TABLE: Record = { human_needed: { status: 'human_needed', next_action: "Human verification required. Complete the manual tests in the phase's *-UAT.md, then re-run the verify step until status is passed.", - next_command: '', + // #2617: was '' — next_action told the user to "re-run the verify step" but + // named no command, while init.cts's parallel projector emitted + // `verify-work ` for this same state. The two surfaces disagreed on + // whether a next command existed at all; init's answer was the useful one, + // and init now delegates here rather than re-deriving it. + next_command: 'verify-work', }, stale: { status: 'stale', @@ -98,17 +110,31 @@ const VERIFICATION_ROUTING_TABLE: Record = { missing: { status: 'missing', next_action: 'No verification report found — the verify step never completed. Re-run execute-phase.', - next_command: '/gsd:execute-phase', + next_command: 'execute-phase', }, // INTERNAL SENTINEL: constructed when the file has a status value not in // VERIFIER_STATUSES. Never emitted by the verifier. unknown: { status: 'unknown', next_action: '', // filled in dynamically with the raw value - next_command: '/gsd:execute-phase', + next_command: 'execute-phase', }, }; +/** + * Project a BARE command name (plus optional argument tail) into the surface the + * given runtime actually installs (#2617). + * + * `formatGsdSlash` owns the per-runtime shape (`$gsd-` for shell-var + * runtimes like Codex, `/gsd-` otherwise) and is idempotent, so passing an + * already-prefixed string is safe. An empty command stays empty — "no next + * command" must not become a bare prefix. + */ +function projectNextCommand(bare: string, runtime: string, tail = ''): string { + if (!bare) return ''; + return `${formatGsdSlash(bare, runtime) as string}${tail}`; +} + // ─── Helpers ───────────────────────────────────────────────────────────────── interface FsLike { @@ -235,12 +261,12 @@ function defaultPhaseCleanCommitTimesMs( * Used for two early-return paths: no *-VERIFICATION.md file found, and * file present but no parseable frontmatter status. */ -function missingResult(): VerificationStatusResult { +function missingResult(runtime: string, phaseArg: string): VerificationStatusResult { const route = VERIFICATION_ROUTING_TABLE['missing']; return { status: route.status, next_action: route.next_action, - next_command: route.next_command, + next_command: projectNextCommand(route.next_command, runtime, phaseArg), }; } @@ -250,6 +276,20 @@ interface ReadVerificationStatusOptions { fs?: FsLike; /** Injectable per-phase clean-commit-time resolver for the staleness clock (#2348). */ phaseCleanCommitTimesMs?: PhaseCleanCommitTimesFn; + /** + * Runtime whose command surface `next_command` is projected into (#2617). + * Callers that have a cwd should pass `resolveRuntime(cwd)`. Defaults to + * `'claude'`, which yields the canonical `/gsd-` hyphen form — never the + * deprecated `/gsd:` colon form this field used to hard-code. + */ + runtime?: string; + /** + * Phase number appended to the routed command (#2617). Defaults to the token + * parsed from `phaseDir`, but only when that token is unambiguously numeric. + * Callers that already know the number pass it explicitly — `init` reaches + * this with `phaseDir` unresolved in some branches. + */ + phaseNumber?: string; } interface VerificationStatusResult { @@ -321,6 +361,8 @@ function findStaleVerificationSummary( * * @param phaseDir - Absolute path to the phase directory. * @param opts - Options. `opts.fs` allows test injection (defaults to node:fs). + * `opts.runtime` selects the command surface `next_command` is + * projected into (#2617). */ function readVerificationStatus( phaseDir: string, @@ -329,11 +371,20 @@ function readVerificationStatus( const fsImpl: FsLike = opts.fs ?? fs; const phaseCleanCommitTimesMs: PhaseCleanCommitTimesFn = opts.phaseCleanCommitTimesMs ?? defaultPhaseCleanCommitTimesMs; + const runtime = opts.runtime ?? 'claude'; // Phase token for the gaps_found command const baseName = path.basename(phaseDir); const phaseToken = extractPhaseToken(baseName); - const phaseNumber = phaseToken.length > 0 ? phaseToken : baseName; + const derivedPhaseNumber = phaseToken.length > 0 ? phaseToken : baseName; + // #2617: the phase number becomes a COMMAND ARGUMENT, so it is appended only + // when it is unambiguously one. extractPhaseToken also returns project-code + // forms (`PROJ-07`), which are indistinguishable by shape from an ordinary + // directory name — `gsd-651-parent` yields `gsd-651` — and emitting + // `execute-phase gsd-651` is worse than emitting no argument at all. Callers + // that already know the number (init) pass it explicitly and always get it. + const phaseArgSource = opts.phaseNumber ?? (/^\d+(\.\d+)*$/.test(derivedPhaseNumber) ? derivedPhaseNumber : ''); + const phaseArg = phaseArgSource ? ` ${phaseArgSource}` : ''; // 1. Find *-VERIFICATION.md let verificationFile: string | null = null; @@ -347,7 +398,7 @@ function readVerificationStatus( } if (!verificationFile) { - return missingResult(); + return missingResult(runtime, phaseArg); } // 2. Read and parse frontmatter using the shared parser. @@ -369,7 +420,7 @@ function readVerificationStatus( } if (!rawStatus) { - return missingResult(); + return missingResult(runtime, phaseArg); } // gaps_found takes priority over stale — gap closure is the correct next @@ -379,7 +430,7 @@ function readVerificationStatus( return { status: entry.status, next_action: entry.next_action, - next_command: `/gsd:plan-phase ${phaseNumber} --gaps`, + next_command: projectNextCommand('plan-phase', runtime, `${phaseArg} --gaps`), }; } @@ -389,7 +440,7 @@ function readVerificationStatus( return { status: entry.status, next_action: entry.next_action, - next_command: `/gsd:verify-work ${phaseNumber}`, + next_command: projectNextCommand('verify-work', runtime, phaseArg), }; } @@ -406,7 +457,7 @@ function readVerificationStatus( return { status: entry.status, next_action: entry.next_action, - next_command: entry.next_command, + next_command: projectNextCommand(entry.next_command, runtime, phaseArg), }; } @@ -415,7 +466,7 @@ function readVerificationStatus( return { status: unknownRoute.status, next_action: `Unexpected verification status '${rawStatus}'. Re-run execute-phase verification.`, - next_command: unknownRoute.next_command, + next_command: projectNextCommand(unknownRoute.next_command, runtime, phaseArg), }; } @@ -433,7 +484,7 @@ function cmdVerificationStatus(cwd: string, phaseDirArg: string | undefined, raw return; } const phaseDir = path.resolve(cwd, phaseDirArg); - const result = readVerificationStatus(phaseDir); + const result = readVerificationStatus(phaseDir, { runtime: resolveRuntime(cwd) }); output(result, raw); } diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index cb000a1d1..185defae8 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -2832,7 +2832,11 @@ describe('phase complete canonical verification gate (#1522)', () => { const errorPayload = JSON.parse(result.error); assert.equal(errorPayload.reason, 'phase_verification_incomplete'); assert.match(errorPayload.message, /stale/i); - assert.match(errorPayload.message, /\/gsd:verify-work 0?1/); + // #2617: the blocked-completion message now projects next_command onto the + // runtime's installed surface. This project has no runtime configured, so it + // takes the `claude` default — the canonical `/gsd-` hyphen form. The colon + // form this previously asserted is the deprecated shape #2617 removed. + assert.match(errorPayload.message, /\/gsd-verify-work 0?1/); assert.equal(fs.readFileSync(roadmapPath, 'utf-8'), beforeRoadmap); assert.equal(fs.readFileSync(statePath, 'utf-8'), beforeState); }); diff --git a/tests/verification-status.test.cjs b/tests/verification-status.test.cjs index 3f49c9445..389df8851 100644 --- a/tests/verification-status.test.cjs +++ b/tests/verification-status.test.cjs @@ -21,7 +21,7 @@ * Cross-platform (passes on Windows). Ref: DEFECT.TEST-SHELL-PIPELINE-NONPORTABLE. */ -const { describe, test } = require('node:test'); +const { describe, test, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); @@ -101,7 +101,7 @@ describe('verification-status', () => { result.next_command.includes('--gaps'), `next_command should include --gaps; got: ${result.next_command}`, ); - assert.equal(result.next_command, '/gsd:plan-phase 03 --gaps'); + assert.equal(result.next_command, '/gsd-plan-phase 03 --gaps'); } finally { cleanup(baseDir); } @@ -114,7 +114,9 @@ describe('verification-status', () => { writeVerificationMd(dir, '01-hn-VERIFICATION.md', 'human_needed'); const result = readVerificationStatus(dir); assert.equal(result.status, 'human_needed'); - assert.equal(result.next_command, ''); + // #2617: human_needed now names the command the next_action describes. + // This fixture's dir is not phase-shaped, so no number is appended. + assert.equal(result.next_command, '/gsd-verify-work'); assert.ok(result.next_action.length > 0); } finally { cleanup(dir); @@ -129,7 +131,7 @@ describe('verification-status', () => { fs.writeFileSync(path.join(dir, 'README.md'), '# phase'); const result = readVerificationStatus(dir); assert.equal(result.status, 'missing'); - assert.equal(result.next_command, '/gsd:execute-phase'); + assert.equal(result.next_command, '/gsd-execute-phase'); assert.ok(result.next_action.includes('verify step never completed')); } finally { cleanup(dir); @@ -143,7 +145,7 @@ describe('verification-status', () => { writeVerificationMd(dir, '01-u-VERIFICATION.md', 'bogus'); const result = readVerificationStatus(dir); assert.equal(result.status, 'unknown'); - assert.equal(result.next_command, '/gsd:execute-phase'); + assert.equal(result.next_command, '/gsd-execute-phase'); assert.ok( result.next_action.includes('bogus'), `next_action should mention the raw value; got: ${result.next_action}`, @@ -295,7 +297,7 @@ describe('verification-status', () => { const nonexistent = path.join(os.tmpdir(), 'gsd-651-nonexistent-' + Date.now()); const result = readVerificationStatus(nonexistent); assert.equal(result.status, 'missing', 'unreadable/nonexistent dir must return missing'); - assert.equal(result.next_command, '/gsd:execute-phase'); + assert.equal(result.next_command, '/gsd-execute-phase'); }); // Multiple *-VERIFICATION.md files → deterministic pick (first by sort) @@ -335,7 +337,7 @@ describe('verification-status', () => { const result = readVerificationStatus(dir, { phaseCleanCommitTimesMs: () => new Map() }); assert.equal(result.status, 'stale'); assert.match(result.next_action, /stale/i); - assert.equal(result.next_command, '/gsd:verify-work 01'); + assert.equal(result.next_command, '/gsd-verify-work 01'); } finally { cleanup(baseDir); } @@ -355,7 +357,7 @@ describe('verification-status', () => { const result = readVerificationStatus(dir); assert.equal(result.status, 'gaps_found'); - assert.equal(result.next_command, '/gsd:plan-phase 01 --gaps'); + assert.equal(result.next_command, '/gsd-plan-phase 01 --gaps'); } finally { cleanup(baseDir); } @@ -378,7 +380,7 @@ describe('verification-status', () => { // git times unavailable → mtime-fallback path (#2348). const result = readVerificationStatus(dir, { phaseCleanCommitTimesMs: () => new Map() }); assert.equal(result.status, 'stale'); - assert.equal(result.next_command, '/gsd:verify-work 01'); + assert.equal(result.next_command, '/gsd-verify-work 01'); } finally { cleanup(baseDir); } @@ -462,7 +464,7 @@ describe('verification-status', () => { const result = readVerificationStatus(dir, { phaseCleanCommitTimesMs }); assert.equal(result.status, 'stale'); - assert.equal(result.next_command, '/gsd:verify-work 02'); + assert.equal(result.next_command, '/gsd-verify-work 02'); } finally { cleanup(baseDir); } @@ -527,7 +529,7 @@ describe('verification-status', () => { 'stale', 'a dirty summary edited after the verification must stale it via mtime, not be shadowed by an equal/earlier commit time', ); - assert.equal(result.next_command, '/gsd:verify-work 02'); + assert.equal(result.next_command, '/gsd-verify-work 02'); } finally { cleanup(baseDir); } @@ -645,7 +647,7 @@ describe('verification-status', () => { 'stale', 'summary committed after the verification must read stale on the real git clock, and the dash-named file must resolve through the `--` pathspec guard', ); - assert.equal(result.next_command, '/gsd:verify-work 01'); + assert.equal(result.next_command, '/gsd-verify-work 01'); } finally { cleanup(repo); } @@ -694,7 +696,7 @@ describe('verification-status', () => { 'stale', 'a committed-then-edited (dirty) summary must read stale via mtime, not be shadowed by its now-stale commit time', ); - assert.equal(result.next_command, '/gsd:verify-work 01'); + assert.equal(result.next_command, '/gsd-verify-work 01'); } finally { cleanup(repo); } @@ -799,3 +801,252 @@ describe('verification-status', () => { }); }); + +// ─── #2617: next_command runtime projection ────────────────────────────────── +// +// Regression tests for #2617 — verification-status `next_command` bypassed the +// runtime command-surface projection. +// +// `src/verification.cts` stored and synthesized hard-coded `/gsd:…` strings with +// no runtime context, and `phase complete` relayed that raw field straight into +// its verification-blocked error. On a Codex project the suggested next step was +// `/gsd:execute-phase`, which Codex does not install — the surface there is +// `$gsd-execute-phase`. The colon form is doubly wrong: `runtime-slash.cts` +// documents that "the colon form is never emitted", so every runtime was getting +// a deprecated shape. (The 11 `/gsd-…` assertions above were `/gsd:…` before this +// fix — they are the failing-first record.) +// +// The fix keeps ONE routing seam and makes its emitted command runtime-aware: +// the table stores bare command names and every return path projects through +// `formatGsdSlash`, with callers passing `resolveRuntime(cwd)`. +// +// Coverage is the matrix the issue asked for — missing, unknown, gaps_found and +// stale, against Codex (`$gsd-…`) and a slash-hyphen runtime (`/gsd-…`) — plus +// the `phase complete` error path, not merely the router's return object. + +/** Codex installs `$gsd-`; every other shipped runtime installs `/gsd-`. */ +const RUNTIMES = [ + { id: 'codex', prefix: '$gsd-' }, + { id: 'cursor', prefix: '/gsd-' }, +]; + +// NOTE: deliberately NOT file-scope beforeEach/afterEach. node:test applies +// module-scope hooks to EVERY test in the file, so hooks added here for the +// #2617 suites would also wrap the ~40 pre-existing tests above — making this +// block a single point of failure for suites it has nothing to do with. Each +// test allocates and releases its own phase dir instead. +let projBaseDir; +let projPhaseDir; + +/** + * Install the #2617 temp-phase-dir lifecycle INSIDE the calling describe. + * node:test scopes hooks to their enclosing describe, so this keeps them off the + * ~40 pre-existing tests in this file. + */ +function useProjectionPhaseDir() { + beforeEach(() => { + projBaseDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2617-')); + projPhaseDir = path.join(projBaseDir, '01-example'); + fs.mkdirSync(projPhaseDir, { recursive: true }); + }); + afterEach(() => cleanup(projBaseDir)); +} + +const verificationPath = () => path.join(projPhaseDir, '01-VERIFICATION.md'); + +function writeStatus(status) { + fs.writeFileSync(verificationPath(), `---\nstatus: ${status}\n---\n\n# Verification\n`); +} + +function removeVerification() { + try { fs.unlinkSync(verificationPath()); } catch { /* already absent */ } +} + +/** Make the verification file older than a summary → the stale branch. */ +function makeStale() { + const summaryPath = path.join(projPhaseDir, '01-01-SUMMARY.md'); + fs.writeFileSync(summaryPath, '# Summary\n'); + fs.utimesSync(verificationPath(), new Date('2026-01-01T00:00:00Z'), new Date('2026-01-01T00:00:00Z')); + fs.utimesSync(summaryPath, new Date('2026-01-01T00:01:00Z'), new Date('2026-01-01T00:01:00Z')); +} + +// git times unavailable → mtime-fallback path (#2348). Injected so the staleness +// clock stays hermetic regardless of the tmpdir's repo state. +const NO_GIT = { phaseCleanCommitTimesMs: () => new Map() }; + +function read(runtime, extra = {}) { + return readVerificationStatus(projPhaseDir, { runtime, ...extra }); +} + +for (const { id, prefix } of RUNTIMES) { + describe(`#2617: next_command uses the ${id} command surface`, () => { + useProjectionPhaseDir(); + + test('missing verification', () => { + removeVerification(); + assert.equal(read(id).next_command, `${prefix}execute-phase 01`); + }); + + test('unparseable/absent frontmatter status is also "missing"', () => { + fs.writeFileSync(verificationPath(), '# Verification\n\nNo frontmatter here.\n'); + assert.equal(read(id).next_command, `${prefix}execute-phase 01`); + }); + + test('unknown status value', () => { + writeStatus('not-a-real-status'); + const result = read(id); + assert.equal(result.status, 'unknown'); + assert.equal(result.next_command, `${prefix}execute-phase 01`); + }); + + test('gaps_found carries the phase number and --gaps flag through the projection', () => { + writeStatus('gaps_found'); + const result = read(id); + assert.equal(result.status, 'gaps_found'); + assert.equal(result.next_command, `${prefix}plan-phase 01 --gaps`); + }); + + test('stale carries the phase number through the projection', () => { + writeStatus('passed'); + makeStale(); + const result = read(id, NO_GIT); + assert.equal(result.status, 'stale'); + assert.equal(result.next_command, `${prefix}verify-work 01`); + }); + + test('passed has no next step and stays empty, not a bare prefix', () => { + // Boundary: projecting an empty command must not emit `$gsd-` / `/gsd-`. + writeStatus('passed'); + assert.equal(read(id).next_command, '', + 'passed has no next command and must project to the empty string'); + }); + + test('human_needed names the verify-work command its next_action describes', () => { + // #2617 unification: the table used to return '' here while init.cts's + // parallel projector returned `verify-work ` for the same state — the + // two surfaces disagreed on whether a next command existed at all. + writeStatus('human_needed'); + assert.equal(read(id).next_command, `${prefix}verify-work 01`); + }); + }); +} + +describe('#2617: no verification output suggests the deprecated colon form', () => { + useProjectionPhaseDir(); + + test('across every state and runtime, and for the default runtime', () => { + const runtimeIds = [...RUNTIMES.map((r) => r.id), undefined]; + let checked = 0; + + for (const runtime of runtimeIds) { + const opts = runtime === undefined ? { ...NO_GIT } : { runtime, ...NO_GIT }; + + removeVerification(); + const cases = [readVerificationStatus(projPhaseDir, opts)]; + + for (const status of ['not-a-real-status', 'gaps_found', 'passed', 'human_needed']) { + writeStatus(status); + cases.push(readVerificationStatus(projPhaseDir, opts)); + } + writeStatus('passed'); + makeStale(); + cases.push(readVerificationStatus(projPhaseDir, opts)); + + for (const result of cases) { + assert.ok( + !result.next_command.includes('/gsd:'), + `deprecated colon form leaked for runtime=${String(runtime)}: ${result.next_command}`, + ); + checked++; + } + } + + // Non-vacuity: 3 runtimes x 6 states. + assert.equal(checked, 18, 'expected every runtime x state combination to be checked'); + }); + + test('the default runtime yields the canonical hyphen form, not the colon form', () => { + removeVerification(); + // No `runtime` option at all — the pre-fix default emitted `/gsd:execute-phase`. + assert.equal(readVerificationStatus(projPhaseDir).next_command, '/gsd-execute-phase 01'); + }); +}); + +describe('#2617: the phase-complete error path projects too', () => { + // The issue is explicit that fixing only the router is insufficient: the + // user-visible surface is `phase complete`, which relays next_command into its + // blocked-completion error. Driven through the real CLI so the assertion is on + // what a user actually sees. + const { runGsdTools, createTempGitProject } = require('./helpers.cjs'); + + for (const { id, prefix } of RUNTIMES) { + test(`phase complete on ${id} suggests ${prefix}execute-phase`, () => { + const projectDir = createTempGitProject(); + try { + fs.writeFileSync( + path.join(projectDir, '.planning', 'config.json'), + JSON.stringify({ runtime: id }, null, 2), + ); + const phase = path.join(projectDir, '.planning', 'phases', '01-example'); + fs.mkdirSync(phase, { recursive: true }); + // No *-VERIFICATION.md → the completion gate blocks with reason "missing". + + const res = runGsdTools(['phase', 'complete', '01'], projectDir); + // The blocked-completion message goes to stderr, which runGsdTools + // surfaces as `error` (NOT `stderr`) on a clean non-zero exit. Reading + // the wrong field yields '' and makes every assertion below vacuous. + const text = `${res.output || ''}${res.error || ''}`; + + assert.equal(res.success, false, 'completion must be blocked with no verification report'); + assert.match( + text, + /verification is incomplete/i, + `expected the blocked-completion error, got: ${text}`, + ); + // Unconditional — a conditional check here passes when the command is + // absent entirely, which is exactly how this path stayed untested. + assert.ok( + text.includes(`${prefix}execute-phase`), + `phase complete must suggest ${prefix}execute-phase on ${id}, got: ${text}`, + ); + assert.ok( + !text.includes('/gsd:'), + `phase complete must not surface the deprecated colon form: ${text}`, + ); + } finally { + cleanup(projectDir); + } + }); + + test(`phase complete on ${id} projects the gaps_found command too`, () => { + // Finding from review: the live-CLI check previously exercised only the + // `missing` state, so a regression in any other routed branch would show + // up in the router's return object but not in what a user actually reads. + const projectDir = createTempGitProject(); + try { + fs.writeFileSync( + path.join(projectDir, '.planning', 'config.json'), + JSON.stringify({ runtime: id }, null, 2), + ); + const phase = path.join(projectDir, '.planning', 'phases', '01-example'); + fs.mkdirSync(phase, { recursive: true }); + fs.writeFileSync( + path.join(phase, '01-VERIFICATION.md'), + '---\nstatus: gaps_found\n---\n\n# Verification\n', + ); + + const res = runGsdTools(['phase', 'complete', '01'], projectDir); + const text = `${res.output || ''}${res.error || ''}`; + + assert.equal(res.success, false, 'gaps_found must block completion'); + assert.ok( + text.includes(`${prefix}plan-phase 01 --gaps`), + `phase complete must suggest ${prefix}plan-phase 01 --gaps on ${id}, got: ${text}`, + ); + assert.ok(!text.includes('/gsd:'), `deprecated colon form leaked: ${text}`); + } finally { + cleanup(projectDir); + } + }); + } +});