From a3b72d90718a10f031cd3621b0188a4a830b8fdb Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 13 Aug 2026 23:49:23 -0400 Subject: [PATCH] fix(#3171): init execute-phase emits display name, not directory slug (#3429) * fix(#3171): init execute-phase emits display name, not directory slug When a phase directory already exists on disk, the disk-lookup path (searchPhaseInDir) derived phase_name from the directory-name remainder -- itself an already-slugified value (phase.add writes ${num}-${slug} dirs) -- so phase_name and phase_slug came out byte-identical. The execute-phase workflow forwards phase_name into 'state begin-phase --name', which wrote that raw slug into STATE.md's current_phase_name on every phase start. cmdInitExecutePhase now prefers the ROADMAP's curated display name ('### Phase N: ') for phase_name, matching the no-disk fallback path that already did this correctly. phase_slug is unchanged (it feeds branch-name construction). The state.begin-phase authoritativeFm override (#2821/#2736) is untouched; the correction is in the value fed into --name. The milestone_name half of #3171 was subsumed by #3216 / PR #3226; this fixes the remaining current_phase_name half. * docs(changeset): backfill pr 3429 for #3171 --------- Co-authored-by: sim --- .changeset/bold-jaguars-run.md | 5 ++ src/init.cts | 12 +++- tests/init.test.cjs | 107 ++++++++++++++++++++++++++++++++- 3 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 .changeset/bold-jaguars-run.md diff --git a/.changeset/bold-jaguars-run.md b/.changeset/bold-jaguars-run.md new file mode 100644 index 000000000..649ddefa3 --- /dev/null +++ b/.changeset/bold-jaguars-run.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3429 +--- +**`init execute-phase` no longer hands a directory slug to the phase-start flow as the phase display name.** When a phase's working directory already exists on disk, the disk-lookup path derived `phase_name` from the directory-name remainder — itself an already-slugified value (`phase.add` writes `${num}-${slug}` dirs) — so `phase_name` and `phase_slug` came out byte-identical. The execute-phase workflow forwards `phase_name` into `state begin-phase --name`, which wrote that raw slug into STATE.md's `current_phase_name` on every phase start (`loop-termination-and-baseline-correctness` instead of `Loop-Termination and Baseline Correctness`). `init execute-phase` now prefers the ROADMAP's curated display name (`### Phase N: `) for `phase_name`, matching the no-disk fallback path that already did this correctly; `phase_slug` is unchanged so branch-name construction is unaffected. The `state begin-phase` override mechanism (#2821/#2736) is untouched. (#3171) diff --git a/src/init.cts b/src/init.cts index 2112cb629..ee7682823 100644 --- a/src/init.cts +++ b/src/init.cts @@ -910,7 +910,17 @@ function cmdInitExecutePhase( ? toPosixPath(path.join(cwd, phaseInfo['directory'] as string)) : null, phase_number: phaseInfo?.['phase_number'] || null, - phase_name: phaseInfo?.['phase_name'] || null, + // #3171: prefer the ROADMAP's curated display name for `phase_name`. When + // the phase directory already exists on disk, the disk-lookup path + // (searchPhaseInDir) derives phase_name from the directory-name remainder + // — itself an already-slugified value (`phase.add` writes `${num}-${slug}` + // dirs), so phase_name and phase_slug come out byte-identical. An + // orchestrator wiring this field into `state begin-phase --name` then + // lands a raw slug in STATE.md's current_phase_name. The ROADMAP carries + // the human-curated display name (`### Phase N: `); prefer it, + // matching the no-disk fallback above. phase_slug stays disk-derived — it + // correctly feeds branch-name construction below and is unchanged here. + phase_name: (roadmapPhase?.['phase_name']) || (phaseInfo?.['phase_name']) || null, phase_slug: phaseInfo?.['phase_slug'] || null, phase_req_ids, diff --git a/tests/init.test.cjs b/tests/init.test.cjs index be00977e5..3bb2334f9 100644 --- a/tests/init.test.cjs +++ b/tests/init.test.cjs @@ -7,7 +7,7 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); const { spawnSync } = require('node:child_process'); -const { runGsdTools, cleanup, absPlanningPath, TOOLS_PATH } = require('./helpers.cjs'); +const { runGsdTools, cleanup, absPlanningPath, TOOLS_PATH, parseFrontmatter } = require('./helpers.cjs'); const { createFixture, seedPhase } = require('./fixtures/index.cjs'); const { createTempProject, createTempDir } = require('./helpers.cjs'); const { executionContextRefs } = require('../scripts/command-contract-helpers.cjs'); @@ -4066,3 +4066,108 @@ describe('init section manifest', () => { }); }); }); + +// ───────────────────────────────────────────────────────────────────────────── +// #3171 (Claim 3): the phase-start flow must not land a directory slug in +// STATE.md's `current_phase_name`. When a phase directory already exists on +// disk, `init execute-phase`'s disk-lookup path derived `phase_name` from the +// directory-name remainder — itself an already-slugified value (`phase.add` +// writes `${num}-${slug}` dirs) — so `phase_name` and `phase_slug` came out +// byte-identical, and the execute-phase workflow forwarded that slug into +// `state begin-phase --name`. The milestone-name half of #3171 was subsumed +// by #3216 / PR #3226; these tests cover the remaining current_phase_name half. +// ───────────────────────────────────────────────────────────────────────────── + +describe('#3171: init execute-phase emits the display name, not the directory slug', () => { + const DISPLAY_NAME = 'Loop-Termination and Baseline Correctness'; + const PHASE_SLUG_DIR = '35-loop-termination-and-baseline-correctness'; + const ROADMAP_3171 = [ + '# Roadmap', + '', + `### Phase 35: ${DISPLAY_NAME}`, + '**Goal:** Fix loop termination', + '**Plans:** 1 plans', + '', + ].join('\n'); + let tmpDir; + + beforeEach(() => { + tmpDir = fs.realpathSync(createFixture()); + seedPhase(tmpDir, PHASE_SLUG_DIR, { '35-01-PLAN.md': '# Plan' }); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), ROADMAP_3171); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('phase_name is the ROADMAP display name when the phase directory exists', () => { + const result = runGsdTools('init execute-phase 35 --raw', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.phase_found, true, 'phase must be found on disk'); + assert.ok( + typeof output.phase_dir === 'string' && output.phase_dir.includes(PHASE_SLUG_DIR), + `phase_dir must point at the on-disk directory; got ${JSON.stringify(output.phase_dir)}`, + ); + assert.strictEqual( + output.phase_name, + DISPLAY_NAME, + `phase_name must be the ROADMAP display name, not the directory slug; got ${JSON.stringify(output.phase_name)}`, + ); + assert.strictEqual(output.phase_slug, 'loop-termination-and-baseline-correctness'); + assert.notStrictEqual(output.phase_name, output.phase_slug, + 'phase_name must differ from phase_slug — a byte-identical pair is the #3171 defect signature'); + }); + + test('the phase-start flow does not land a slug in current_phase_name', () => { + // 1. init execute-phase → the value the execute-phase workflow forwards to begin-phase. + const initResult = runGsdTools('init execute-phase 35 --raw', tmpDir); + assert.ok(initResult.success, `init execute-phase failed: ${initResult.error}`); + const initOutput = JSON.parse(initResult.output); + assert.strictEqual(initOutput.phase_name, DISPLAY_NAME); + + // 2. Seed a STATE.md the transition module can rewrite (frontmatter + body). + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + [ + '---', + 'gsd_state_version: 1.0', + 'current_phase: 34', + 'current_phase_name: Prior Phase', + 'status: planning', + '---', + '', + '# Project State', + '', + '## Current Position', + '', + 'Phase: 34 — Prior Phase', + 'Plan: Not started', + 'Status: Ready to execute', + '', + ].join('\n'), + ); + + // 3. The orchestrator wiring: feed init's phase_name into begin-phase --name. + const beginResult = runGsdTools( + ['state', 'begin-phase', '--phase', '35', '--name', initOutput.phase_name, '--plans', '1'], + tmpDir, + ); + assert.ok(beginResult.success, `state begin-phase failed: ${beginResult.error}`); + + // 4. current_phase_name in STATE.md must be the display name, never the slug. + const stateContent = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + const fm = parseFrontmatter(stateContent); + assert.strictEqual( + fm.current_phase_name, + DISPLAY_NAME, + `current_phase_name must hold the display name, not a directory slug; got ${JSON.stringify(fm.current_phase_name)}`, + ); + assert.ok( + !/^[a-z0-9]+(-[a-z0-9]+)+$/.test(String(fm.current_phase_name)), + `current_phase_name must not be slug-shaped; got ${JSON.stringify(fm.current_phase_name)}`, + ); + }); +});