* 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: <Name>') 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 <sim@local>
This commit is contained in:
5
.changeset/bold-jaguars-run.md
Normal file
5
.changeset/bold-jaguars-run.md
Normal file
@@ -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: <Name>`) 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)
|
||||
12
src/init.cts
12
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: <Name>`); 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,
|
||||
|
||||
|
||||
@@ -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)}`,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user