From 41466e8e88ac5c0be593385654e0164cc00431c3 Mon Sep 17 00:00:00 2001 From: BeeHiggs Date: Tue, 1 Sep 2026 11:58:20 -0600 Subject: [PATCH] fix(#4023): preserve decimal phase ids in init progress ordering and smart-entry output (#4110) * test(#4023): reproduce decimal phase-id coercions * fix(#4023): preserve decimal phase ids in progress signals * test(#4023): align phase token contract expectations * chore(#4023): point the changeset at PR #4110 Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: Tom Boucher --- .changeset/calm-herons-sort.md | 5 +++ src/init.cts | 6 +-- src/smart-entry.cts | 17 ++++----- tests/init.test.cjs | 65 ++++++++++++++++++++++++++++++++- tests/smart-entry.unit.test.cjs | 15 +++++++- tests/state.test.cjs | 4 +- 6 files changed, 95 insertions(+), 17 deletions(-) create mode 100644 .changeset/calm-herons-sort.md diff --git a/.changeset/calm-herons-sort.md b/.changeset/calm-herons-sort.md new file mode 100644 index 000000000..b062506df --- /dev/null +++ b/.changeset/calm-herons-sort.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4110 +--- +**Progress routing preserves decimal phase IDs** — `init progress` now orders parent and inserted phases canonically, and `smart-entry --json` returns the complete current phase token instead of truncating it to an integer. diff --git a/src/init.cts b/src/init.cts index d236c55ca..ed9e75cd3 100644 --- a/src/init.cts +++ b/src/init.cts @@ -92,7 +92,7 @@ const { extractCurrentMilestone, } = roadmapParser; const { pathExistsInternal, generateSlugInternal, toPosixPath } = coreUtils; -const { normalizePhaseName, matchPhaseDirs, stripProjectCodePrefix, PHASE_NUMBER_TOKEN_SOURCE, isForeignPrefixedPhaseQuery, isSentinelPhaseId, extractPhaseToken, scopeToPhase } = phaseId; +const { comparePhaseNum, normalizePhaseName, matchPhaseDirs, stripProjectCodePrefix, PHASE_NUMBER_TOKEN_SOURCE, isForeignPrefixedPhaseQuery, isSentinelPhaseId, extractPhaseToken, scopeToPhase } = phaseId; const { pruneOrphanedWorktrees } = worktreeSafety; const { @@ -3175,9 +3175,7 @@ function cmdInitProgress(cwd: string, raw: boolean, options: Record parseInt(a['number'] as string, 10) - parseInt(b['number'] as string, 10), - ); + phases.sort((a, b) => comparePhaseNum(a['number'], b['number'])); // #3581: the frontier is ROADMAP ORDER, not artifact presence. The disk loop // above could claim nextPhase from a stray out-of-order artifact directory diff --git a/src/smart-entry.cts b/src/smart-entry.cts index 76405c120..ab81ff6e1 100644 --- a/src/smart-entry.cts +++ b/src/smart-entry.cts @@ -54,7 +54,7 @@ import stateDocument = require('./state-document.cjs'); const { stateFieldValue } = stateDocument; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseId = require('./phase-id.cjs'); -const { comparePhaseNum, extractPhaseToken, matchPhaseDirs, normalizePhaseName, stripProjectCodePrefix } = phaseId; +const { comparePhaseNum, extractPhaseToken, matchPhaseDirs, normalizePhaseName, parsePhaseFromProse, stripProjectCodePrefix } = phaseId; // eslint-disable-next-line @typescript-eslint/no-require-imports import stateMod = require('./state.cjs'); const { readStateHeadFreshness } = stateMod; @@ -87,7 +87,7 @@ export interface SmartEntryAction { } export interface SmartEntrySignals { - current_phase: number | null; + current_phase: string | null; total_phases: number | null; status: string; progress: number | null; @@ -306,9 +306,7 @@ function readGitSignals(cwd: string): GitSignals { /** Leading numeric phase token from a STATE.md scalar or body `Phase:` value. */ function phaseTokenFromState(raw: string | null): string | null { - if (!raw?.trim()) return null; - const match = raw.trim().match(/^(\d+(?:[A-Z])?(?:\.\d+)*)/i); - return match ? match[1] : null; + return parsePhaseFromProse(raw).phase; } /** @@ -513,7 +511,7 @@ export function detectSignals(cwd: string, now: () => number = Date.now): SmartE const freshness = readStateHeadFreshness(cwd, stateHeadRaw); return { - current_phase: parseIntOrNull(currentPhaseRaw), + current_phase: phaseTokenFromState(currentPhaseRaw), total_phases: parseIntOrNull(totalPhasesRaw), status: (statusRaw || '').toLowerCase(), progress: parseIntOrNull(progressRaw), @@ -555,10 +553,11 @@ function isComplete(s: SmartEntrySignals): boolean { if (s.roadmap_total_phases === 0) return false; if (s.roadmap_completed_phases < s.roadmap_total_phases) return false; } else { - // Legacy path: STATE.md comparison. Still subject to the two-scale bug, - // but only fires when ROADMAP.md is absent or has no Progress table. + // Legacy path: STATE.md comparison. It only fires when ROADMAP.md is + // absent or has no Progress table, and uses canonical phase-id ordering + // so dotted phase ids are never coerced to JavaScript numbers. if (s.total_phases === null || s.current_phase === null) return false; - if (s.current_phase < s.total_phases) return false; + if (comparePhaseNum(s.current_phase, s.total_phases) < 0) return false; } // Status regex: require milestone-level completion language. The pre-fix // regex matched any "shipped" / "done" substring (per-phase language). diff --git a/tests/init.test.cjs b/tests/init.test.cjs index e219e8046..90211000b 100644 --- a/tests/init.test.cjs +++ b/tests/init.test.cjs @@ -4744,6 +4744,70 @@ describe('#3581: init.progress next_phase prefers the roadmap frontier', () => { assert.equal(eight.directory, null, 'Phase 8 has no directory (corroborating the stray-only-disk shape)'); }); + test('#4023: init.progress sorts decimal phase ids before choosing the roadmap frontier', (t) => { + const tmpDir = createTempProject('gsd-4023-init-'); + t.after(() => cleanup(tmpDir)); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), [ + '# Roadmap', + '', + '## Milestone v1.1.0', + '', + '### Phase 12: Parent', + '**Goal:** g', + '', + '### Phase 12.1: Inserted fix', + '**Goal:** g', + '', + '### Phase 12.2: Second insert', + '**Goal:** g', + '', + '### Phase 12.10: Tenth insert', + '**Goal:** g', + '', + '### Phase 13: Follow-up', + '**Goal:** g', + '', + ].join('\n')); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), [ + '---', + 'gsd_state_version: 1.0', + 'milestone: v1.1.0', + 'milestone_name: Active', + 'status: executing', + 'current_phase: 12.1', + 'progress:', + ' total_phases: 13', + ' completed_phases: 11', + ' percent: 85', + '---', + '', + '# Project State', + '', + '## Current Position', + '', + 'Phase: 12.1', + 'Status: Executing', + '', + ].join('\n')); + for (const dir of ['12.1-inserted-fix', '12.10-tenth-insert']) { + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', dir), { recursive: true }); + } + + const result = runGsdTools(['init', 'progress', '--raw'], tmpDir); + assert.ok(result.success, `init progress failed: ${result.error}`); + const out = JSON.parse(result.output); + assert.deepEqual( + out.phases.map((phase) => String(phase.number).replace(/^0+(?=\d)/, '')), + ['12', '12.1', '12.2', '12.10', '13'], + 'the disk/roadmap union follows component-wise phase-id order (12.2 before 12.10)', + ); + assert.equal( + String(out.next_phase.number).replace(/^0+(?=\d)/, ''), + '12', + 'the pending parent remains the frontier when an inserted decimal directory exists first', + ); + }); + test('#3581 (control): a pending roadmap-only phase outranks a later pending directory', (t) => { writeProgressFixture(t, { strayNine: false }); // pure ordering property, no stray artifacts: roadmap-only pending 8 vs a @@ -4941,4 +5005,3 @@ describe('init — GSD_PROJECT scoping (#3964)', () => { }); }); - diff --git a/tests/smart-entry.unit.test.cjs b/tests/smart-entry.unit.test.cjs index 4fd13cb7b..33c26aac7 100644 --- a/tests/smart-entry.unit.test.cjs +++ b/tests/smart-entry.unit.test.cjs @@ -327,7 +327,7 @@ describe('smart-entry: real STATE.md schema (nested progress YAML + body Phase f roadmap: true, })); const signals = detectSignals(dir); - assert.equal(signals.current_phase, 3, 'current_phase from body Phase: field'); + assert.equal(signals.current_phase, '3', 'current_phase from body Phase: field'); assert.equal(signals.total_phases, 5, 'total_phases from nested progress.total_phases'); assert.equal(signals.progress, 40, 'percent from nested progress.percent'); assert.equal(signals.status, 'verifying'); @@ -457,6 +457,19 @@ describe('smart-entry: JSON shape invariants', () => { describe('smart-entry: CLI dispatch (gsd-tools smart-entry)', () => { afterEach(removeAll); + test('#4023: --json preserves decimal current_phase as a phase-id string', () => { + for (const phase of ['12.1', '12.10']) { + const dir = track(makeProject({ + state: state({ status: 'executing', total_phases: 13, current_phase: phase }), + roadmap: true, + })); + const r = runNode([TOOLS, 'smart-entry', '--json', '--cwd', dir], { timeoutMs: PROBE_TIMEOUT_MS }); + throwIfFailed(r, 'gsd-tools smart-entry --json'); + const out = JSON.parse(r.stdout); + assert.equal(out.signals.current_phase, phase); + } + }); + test('--json in an empty dir returns no-project machine JSON', () => { // A bare tmpdir with no .planning is a true no-project. const bare = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-se-bare-')); diff --git a/tests/state.test.cjs b/tests/state.test.cjs index 967819b1f..fc9b4d87c 100644 --- a/tests/state.test.cjs +++ b/tests/state.test.cjs @@ -6157,7 +6157,7 @@ describe('#3187 chain-owner identity — every consumer agrees with stateFieldVa stateDocument.stateFieldValue(fm, body, null, 'Phase').value; const output = JSON.parse(runGsdTools('smart-entry --json', tmpDir).output); - assert.strictEqual(output.signals.current_phase, parseInt(ownerPhaseRaw, 10)); + assert.strictEqual(output.signals.current_phase, ownerPhaseRaw); }); test('C5: workstream projection matches the owner', () => { @@ -6258,7 +6258,7 @@ describe('#3187 chain-owner identity — every consumer agrees with stateFieldVa // C4: smart-entry const smartEntry = JSON.parse(runGsdTools('smart-entry --json', tmpDir).output); - assert.strictEqual(smartEntry.signals.current_phase, Number(ownerPhase)); + assert.strictEqual(smartEntry.signals.current_phase, ownerPhase); // C6: complete-phase idempotency guard (no frontmatter in this fixture, // so this row does not exercise the frontmatter tier — see C6's own test