fix(#4383): require phase before planned-phase writes (#4534)

* fix(#4383): require phase before planned-phase writes

* chore: add changeset for #4534

---------

Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
This commit is contained in:
Michel Moreira
2026-09-09 00:38:23 -03:00
committed by GitHub
parent c9d3e66631
commit 6c5e11049b
3 changed files with 48 additions and 2 deletions

View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 4534
---
**`state planned-phase` now requires a present `--phase` before writing** — missing, empty, and flag-shaped values exit non-zero with STATE.md byte-identical, while phase zero remains valid. (#4383)

View File

@@ -5390,7 +5390,13 @@ function updatePerformanceMetricsSection(content: string, cwd: string, phaseNum:
* Gate 3a: Record state after plan-phase completes.
* Updates Status to "Ready to execute", Total Plans, Last Activity.
*/
function cmdStatePlannedPhase(cwd: string, phaseNumber: string | number, phaseName: string | null | undefined, planCount: number | null | undefined, raw: boolean): void {
function cmdStatePlannedPhase(cwd: string, phaseNumber: string | number | null | undefined, phaseName: string | null | undefined, planCount: number | null | undefined, raw: boolean): void {
// #4383: mirror begin-phase's command-boundary guard. A missing phase must
// fail before even looking up STATE.md so no invalid invocation can enter
// the read-modify-write path and serialize a null/blank phase identity.
if (phaseNumber == null || String(phaseNumber).trim() === '') {
error('phase required (--phase <N>)');
}
const statePath = planningPaths(cwd).state;
if (!fs.existsSync(statePath)) {
output({ error: 'STATE.md not found' }, raw, undefined);
@@ -5407,7 +5413,7 @@ function cmdStatePlannedPhase(cwd: string, phaseNumber: string | number, phaseNa
// still owns the lock, the #1230 preservation, and the no-op write guard.
const intent: StateTransitionIntent = {
kind: 'plannedPhase',
phaseNumber,
phaseNumber: phaseNumber as string | number,
phaseName: phaseName ?? null,
planCount: planCount ?? null,
};

View File

@@ -4923,6 +4923,13 @@ describe('updatePerformanceMetricsSection', () => {
describe('state planned-phase command', () => {
let tmpDir;
function seedState() {
fs.writeFileSync(
path.join(tmpDir, '.planning', 'STATE.md'),
'# Project State\n\n**Status:** Planning\n**Total Plans in Phase:** 0\n**Last Activity:** 2024-01-01\n**Current Phase:** 1\n',
);
}
beforeEach(() => {
tmpDir = createFixture();
});
@@ -4931,6 +4938,34 @@ describe('state planned-phase command', () => {
cleanup(tmpDir);
});
for (const { label, args } of [
{ label: 'missing', args: ['state', 'planned-phase'] },
{ label: 'flag-shaped', args: ['state', 'planned-phase', '--phase', '--name', 'API'] },
{ label: 'empty', args: ['state', 'planned-phase', '--phase', ''] },
{ label: 'whitespace-only', args: ['state', 'planned-phase', '--phase', ' '] },
]) {
test(`#4383: ${label} --phase fails before writing STATE.md`, () => {
seedState();
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
const before = fs.readFileSync(statePath, 'utf8');
const result = runGsdTools(args, tmpDir);
assert.strictEqual(result.success, false, `${label} --phase must fail the command`);
assert.notStrictEqual(result.exitCode, 0, 'usage error must exit non-zero');
assert.match(result.error, /--phase/, `usage message must name --phase; got: ${result.error}`);
assert.strictEqual(fs.readFileSync(statePath, 'utf8'), before, 'STATE.md must stay byte-identical');
});
}
test('#4383: phase zero remains a valid present value', () => {
seedState();
const result = runGsdTools(['state', 'planned-phase', '--phase', '0', '--plans', '1'], tmpDir);
assert.ok(result.success, `phase zero must not be treated as missing: ${result.error}`);
assert.strictEqual(JSON.parse(result.output).phase, '0');
});
test('after call: Status is "Ready to execute"', () => {
fs.writeFileSync(
path.join(tmpDir, '.planning', 'STATE.md'),