golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(#3865): init.* phase queries accept --phase <N> as the positional alias (#4054)

Browse Source 
* test(#3865): init.* phase queries accept --phase as positional alias (failing first)

* fix(#3865): init.* phase queries accept --phase <N> as the positional alias

The phase-taking init.* queries read their phase token at args[2]
blindly: '--phase 60' made the literal '--phase' the phase, which the
locator can never resolve — the reported incident answered a well-formed
phase_found:false, plan_count:0 with exit 0 for a phase holding seven
committed plans (ADR-3473 §8.4's later strict validation turned the
paired form into a usage error instead, still not the alias).

normalizePhaseAlias (shared by all eight phase-taking handlers — the
issue's four plus phase-op, review, discuss-phase-assumptions, todos,
the same class) rewrites '--phase N'/'--phase=N' into the caller-owned
positional slot before flag parsing, so handlers and strict validation
see exactly the argv the positional form produces. A valueless --phase
is a usage error naming the flag. Any other flag-shaped args[2] now
resolves to undefined (the commands' designed use-the-current-phase
input) instead of passing flag text down as a phase name.

isFlagToken exported from command-arg-projection (single owner of the
flag-shape predicate).

* fix(#3865): review fold-ins — honest no-position-given comment, fail-closed throws

The helper's comment claimed flag-shaped args[2] resolves to 'the
commands' designed use-the-current-phase input' — no such cross-module
behavior exists: execute-phase/plan-phase/verify-work usage-error
'phase required', the find-based queries answer phase_found:false, and
todos drops its area filter. Reworded to what actually happens, so the
contract-grade comment cannot mislead a future edit. Adds the
parseNamedArgsOrExit-style throw after each error() call as a
fail-closed backstop against a returning fail().

* chore(#3865): changeset fragment (pr number backfilled after PR creation)

* chore(#3865): backfill changeset PR number (4054)

---------

Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
2026-08-29 15:22:34 -04:00
committed by GitHub
parent d9e906a744
commit 44ddc6dc46
4 changed files with 155 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/graceful-rams-roar.md Normal file
View File

@@ -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 <N> 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)

5
src/command-arg-projection.cts
View File

@@ -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('--');
}

98
src/init-command-router.cts
View File

@@ -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 <N>` / `--phase=<N>` 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 <N> (or the positional form <N>)');
// 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 <N> (or the positional form <N>)');
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 <N>` /
// `--phase=<N>` 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),

65
tests/init.test.cjs
View File

@@ -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 <N> 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 <N>', () => {
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 <N>', () => {
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 <N>', () => {
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');
});
});