diff --git a/.changeset/graceful-rams-roar.md b/.changeset/graceful-rams-roar.md new file mode 100644 index 000000000..51a879472 --- /dev/null +++ b/.changeset/graceful-rams-roar.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4054 +--- +the phase-taking init.* queries (execute-phase, plan-phase, verify-work, code-review, phase-op, review, discuss-phase-assumptions, todos) accept --phase as an alias for the positional form, matching phase list-plans; a valueless --phase is now a usage error instead of silently answering phase_found:false for a phase that has plans (#3865) diff --git a/src/command-arg-projection.cts b/src/command-arg-projection.cts index 9f14ddb1a..c74854e71 100644 --- a/src/command-arg-projection.cts +++ b/src/command-arg-projection.cts @@ -108,7 +108,10 @@ function assertValidSpec(spec: unknown): asserts spec is NamedArgSpec { // Single predicate reused for both extraction (a value beginning with a // single `-` is a value, not a flag) and validation (negative space N5). -function isFlagToken(tok: string): boolean { +// Exported for init-command-router's #3865 --phase alias normalization, +// which needs the same flag-shape test before deciding whether args[2] is a +// phase positional or a flag token. +export function isFlagToken(tok: string): boolean { return tok.startsWith('--'); } diff --git a/src/init-command-router.cts b/src/init-command-router.cts index 786a6a3b8..ffd4a9cc3 100644 --- a/src/init-command-router.cts +++ b/src/init-command-router.cts @@ -19,7 +19,7 @@ import { INIT_SUBCOMMANDS } from './command-aliases.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports import cjsCommandRouterAdapter = require('./cjs-command-router-adapter.cjs'); const { routeCjsCommandFamily } = cjsCommandRouterAdapter; -import { parseNamedArgsOrExit } from './command-arg-projection.cjs'; +import { parseNamedArgsOrExit, isFlagToken } from './command-arg-projection.cjs'; // ─── Types ──────────────────────────────────────────────────────────────────── @@ -63,6 +63,53 @@ interface RouteInitCommandOptions { // ─── Implementation ─────────────────────────────────────────────────────────── +/** + * #3865: `--phase ` / `--phase=` alias for the positional phase token + * the phase-taking init.* queries read at args[2]. Normalizes the pair into + * that caller-owned slot so (a) the handler's own args[2] read sees the + * value, and (b) the strict flag validation it runs sees exactly the argv + * the positional form produces (a bare `--phase 60` at index 2-3 would + * otherwise leave `60` as a rejected stray positional — or, pre-ADR-3473 + * §8.4, silently answer `phase_found:false, plan_count:0` for a phase with + * plans on disk). A valueless `--phase` is a usage error naming the flag. + * Any other flag-shaped args[2] resolves to `undefined` — the commands' + * no-position-given input (execute-phase/plan-phase/verify-work usage-error + * "phase required"; the find-based queries answer phase_found:false; todos + * drops its area filter) — instead of passing the flag text down as a phase + * name. + */ +function normalizePhaseAlias( + args: string[], + error: (message: string) => void, +): { args: string[]; phase: string | undefined } { + const tok = args[2]; + if (tok === undefined) return { args, phase: undefined }; + if (tok === '--phase=') { + error('--phase requires a value: use --phase (or the positional form )'); + // Fail-closed backstop, mirroring parseNamedArgsOrExit: the wired error() + // exits, but a returning fail() must not fall through to the splices below. + throw new Error('normalizePhaseAlias: error() returned instead of exiting'); + } + if (tok.startsWith('--phase=')) { + const value = tok.slice('--phase='.length); + const out = args.slice(); + out.splice(2, 1, value); + return { args: out, phase: value }; + } + if (tok === '--phase') { + const next = args[3]; + if (next === undefined || isFlagToken(next)) { + error('--phase requires a value: use --phase (or the positional form )'); + throw new Error('normalizePhaseAlias: error() returned instead of exiting'); + } + const out = args.slice(); + out.splice(2, 2, next); + return { args: out, phase: next }; + } + if (isFlagToken(tok)) return { args, phase: undefined }; + return { args, phase: tok }; +} + function routeInitCommand({ init, args, cwd, raw, error }: RouteInitCommandOptions): void { routeCjsCommandFamily({ args, @@ -76,22 +123,33 @@ function routeInitCommand({ init, args, cwd, raw, error }: RouteInitCommandOptio // `buildSectionManifestField`'s flags-Set builder (src/init.cts) is the // single source of truth for flag ABSENCE and gates on value truthiness, // so `namedArgs` is passed through here uncoerced. + // + // #3865: the phase-taking init.* queries accept `--phase ` / + // `--phase=` as an alias for the positional form (matching + // `phase list-plans`, which accepts both). Pre-normalizing here moves + // the value into the caller-owned args[2] slot every handler below + // already reads, so the strict flag validation those handlers run sees + // exactly the argv the positional form produces. A valueless --phase is + // a usage error naming the flag — never a silent `phase_found:false` + // for a phase that has plans (the reported incident: 7 plans read as 0). 'execute-phase': () => { + const norm = normalizePhaseAlias(args, error); // `wave` is an optionalValueFlags entry, not a booleanFlags entry: // `--wave N` is a documented, shipped form (commands/gsd/execute-phase.md:4,48) // whose value is consumed by the workflow layer // (gsd-core/workflows/execute-phase.md:84), not by this CLI seam — see // NamedArgSpec.optionalValueFlags in command-arg-projection.cts. - const namedArgs = parseNamedArgsOrExit(args, { booleanFlags: ['validate', 'tdd'], optionalValueFlags: ['wave'], positionals: 3 }, error); - init.cmdInitExecutePhase(cwd, args[2], raw, { + const namedArgs = parseNamedArgsOrExit(norm.args, { booleanFlags: ['validate', 'tdd'], optionalValueFlags: ['wave'], positionals: 3 }, error); + init.cmdInitExecutePhase(cwd, norm.phase, raw, { validate: namedArgs['validate'], tdd: namedArgs['tdd'], wave: namedArgs['wave'], }); }, 'plan-phase': () => { + const norm = normalizePhaseAlias(args, error); const namedArgs = parseNamedArgsOrExit( - args, + norm.args, { valueFlags: ['granularity', 'prd', 'ingest', 'research-phase'], booleanFlags: ['validate', 'tdd', 'reviews', 'chunked'], @@ -99,7 +157,7 @@ function routeInitCommand({ init, args, cwd, raw, error }: RouteInitCommandOptio }, error, ); - init.cmdInitPlanPhase(cwd, args[2], raw, { + init.cmdInitPlanPhase(cwd, norm.phase, raw, { validate: namedArgs['validate'], tdd: namedArgs['tdd'], granularity: namedArgs['granularity'], @@ -158,28 +216,34 @@ function routeInitCommand({ init, args, cwd, raw, error }: RouteInitCommandOptio // separate `query init.verify-work` seam and is stripped before // reaching `init verify-work` (gsd-core/workflows/verify-work.md:42-45). 'verify-work': () => { - parseNamedArgsOrExit(args, { positionals: 3 }, error); - init.cmdInitVerifyWork(cwd, args[2], raw); + const norm = normalizePhaseAlias(args, error); + parseNamedArgsOrExit(norm.args, { positionals: 3 }, error); + init.cmdInitVerifyWork(cwd, norm.phase, raw); }, 'phase-op': () => { - parseNamedArgsOrExit(args, { positionals: 3 }, error); - init.cmdInitPhaseOp(cwd, args[2], raw); + const norm = normalizePhaseAlias(args, error); + parseNamedArgsOrExit(norm.args, { positionals: 3 }, error); + init.cmdInitPhaseOp(cwd, norm.phase, raw); }, 'code-review': () => { - const namedArgs = parseNamedArgsOrExit(args, { booleanFlags: ['fix'], positionals: 3 }, error); - init.cmdInitCodeReview(cwd, args[2], raw, { fix: namedArgs['fix'] }); + const norm = normalizePhaseAlias(args, error); + const namedArgs = parseNamedArgsOrExit(norm.args, { booleanFlags: ['fix'], positionals: 3 }, error); + init.cmdInitCodeReview(cwd, norm.phase, raw, { fix: namedArgs['fix'] }); }, review: () => { - parseNamedArgsOrExit(args, { positionals: 3 }, error); - init.cmdInitReview(cwd, args[2], raw, {}); + const norm = normalizePhaseAlias(args, error); + parseNamedArgsOrExit(norm.args, { positionals: 3 }, error); + init.cmdInitReview(cwd, norm.phase, raw, {}); }, 'discuss-phase-assumptions': () => { - const namedArgs = parseNamedArgsOrExit(args, { booleanFlags: ['auto'], positionals: 3 }, error); - init.cmdInitDiscussPhaseAssumptions(cwd, args[2], raw, { auto: namedArgs['auto'] }); + const norm = normalizePhaseAlias(args, error); + const namedArgs = parseNamedArgsOrExit(norm.args, { booleanFlags: ['auto'], positionals: 3 }, error); + init.cmdInitDiscussPhaseAssumptions(cwd, norm.phase, raw, { auto: namedArgs['auto'] }); }, todos: () => { - parseNamedArgsOrExit(args, { positionals: 3 }, error); - init.cmdInitTodos(cwd, args[2], raw); + const norm = normalizePhaseAlias(args, error); + parseNamedArgsOrExit(norm.args, { positionals: 3 }, error); + init.cmdInitTodos(cwd, norm.phase, raw); }, 'milestone-op': () => init.cmdInitMilestoneOp(cwd, raw), 'map-codebase': () => init.cmdInitMapCodebase(cwd, raw), diff --git a/tests/init.test.cjs b/tests/init.test.cjs index 0a197d451..e219e8046 100644 --- a/tests/init.test.cjs +++ b/tests/init.test.cjs @@ -918,6 +918,69 @@ describe('init commands', () => { const output = JSON.parse(result.output); assert.strictEqual(output.phase_found, true); assert.strictEqual(output.phase_name, 'Details Block Regression'); + + }); + // ─── #3865: --phase alias for the positional phase token ────────────────── + + test('#3865: init execute-phase accepts --phase as the positional alias', () => { + seedPhase(tmpDir, '03-api', { '03-01-PLAN.md': '# Plan' }); + writePlanningDocs(tmpDir); + const result = runGsdTools(['init', 'execute-phase', '--phase', '03'], tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.phase_found, true, '--phase 03 must resolve the phase, not answer phase_found:false'); + assert.strictEqual(output.plan_count, 1, 'the on-disk plan must be counted — the reported incident had 7 plans read as 0'); + }); + + test('#3865: init execute-phase accepts the --phase=N form', () => { + seedPhase(tmpDir, '03-api', { '03-01-PLAN.md': '# Plan' }); + writePlanningDocs(tmpDir); + const result = runGsdTools(['init', 'execute-phase', '--phase=03'], tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + assert.strictEqual(JSON.parse(result.output).phase_found, true); + }); + + test('#3865: the positional form still works (control)', () => { + seedPhase(tmpDir, '03-api', { '03-01-PLAN.md': '# Plan' }); + writePlanningDocs(tmpDir); + const result = runGsdTools(['init', 'execute-phase', '03'], tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + assert.strictEqual(JSON.parse(result.output).phase_found, true); + }); + + test('#3865: init plan-phase accepts --phase ', () => { + seedPhase(tmpDir, '03-api', { '03-01-PLAN.md': '# Plan' }); + writePlanningDocs(tmpDir); + const result = runGsdTools(['init', 'plan-phase', '--phase', '03'], tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + assert.strictEqual(JSON.parse(result.output).phase_found, true); + }); + + test('#3865: init verify-work accepts --phase ', () => { + seedPhase(tmpDir, '03-api', { '03-01-PLAN.md': '# Plan' }); + writePlanningDocs(tmpDir); + const result = runGsdTools(['init', 'verify-work', '--phase', '03'], tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + assert.strictEqual(JSON.parse(result.output).phase_found, true); + }); + + test('#3865: init code-review accepts --phase ', () => { + seedPhase(tmpDir, '03-api', { '03-01-PLAN.md': '# Plan' }); + writePlanningDocs(tmpDir); + const result = runGsdTools(['init', 'code-review', '--phase', '03'], tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + assert.strictEqual(JSON.parse(result.output).phase_found, true); + }); + + test('#3865: --phase with no value is a usage error, never a silent phase_found:false', () => { + seedPhase(tmpDir, '03-api', { '03-01-PLAN.md': '# Plan' }); + writePlanningDocs(tmpDir); + const result = runGsdTools(['init', 'execute-phase', '--phase'], tmpDir); + assert.strictEqual(result.success, false, 'a valueless --phase must exit non-zero with a diagnostic'); + assert.ok( + (result.error || '').includes('--phase'), + `the diagnostic must name the flag; got: ${result.error}` + ); }); }); @@ -4877,3 +4940,5 @@ describe('init — GSD_PROJECT scoping (#3964)', () => { assert.equal(out['codebase_dir_exists'], true, 'unscoped probe of the root codebase dir'); }); }); + +