fix(3774): treat 999 as exact sentinel in phase-lifecycle-policy (#93)
* fix(3774): treat 999 as exact sentinel, not lower bound, in phase-lifecycle-policy scanSequentialMaxPhaseFromMilestone and scanSequentialMaxPhaseFromDirs used `num >= 999` to skip the backlog lane, but this incorrectly excluded every phase ≥ 1000, causing computeNextSequentialPhaseId to return 1 for projects using canonical phase IDs in the 1000+ range. Change both guards to `num === 999` so only the backlog sentinel is skipped. Adds regression test: project with phases 1000–1500 must produce 1501, not 1. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: add changeset for fix #3792 (phase.add returns 1 on 1000+ projects) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(3774): address review — fix 4 CJS scanner twins + tighten regression test Addresses gsd-code-reviewer BLOCKER (4 CJS scanner twins in phase.cjs:610,624,688,698 still carried >= 999, reachable via GSD_WORKSTREAM / absent SDK build) and MAJOR (regression test couldn't distinguish === 999 from === 1000 — added [999, 1000] fixture asserting result === 1001). Decrement helpers at :893, :922, :930, :936 left unchanged — intentional 999-lane protection per dual-review analysis. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
5
.changeset/bold-orcas-howl.md
Normal file
5
.changeset/bold-orcas-howl.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3792
|
||||
---
|
||||
phase.add no longer returns 1 on projects using canonical phase IDs 1000 or higher.
|
||||
@@ -607,7 +607,7 @@ function cmdPhaseAdd(cwd, description, raw, customId) {
|
||||
let m;
|
||||
while ((m = phasePattern.exec(content)) !== null) {
|
||||
const num = parseInt(m[1], 10);
|
||||
if (num >= 999) continue; // backlog phases use 999.x numbering
|
||||
if (num === 999) continue; // backlog phases use 999.x numbering
|
||||
if (num > maxPhase) maxPhase = num;
|
||||
}
|
||||
|
||||
@@ -621,7 +621,7 @@ function cmdPhaseAdd(cwd, description, raw, customId) {
|
||||
const match = entry.match(dirNumPattern);
|
||||
if (!match) continue;
|
||||
const num = parseInt(match[1], 10);
|
||||
if (num >= 999) continue; // skip backlog orphans
|
||||
if (num === 999) continue; // skip backlog orphans
|
||||
if (num > maxPhase) maxPhase = num;
|
||||
}
|
||||
}
|
||||
@@ -685,7 +685,7 @@ function cmdPhaseAddBatch(cwd, descriptions, raw) {
|
||||
let m;
|
||||
while ((m = phasePattern.exec(content)) !== null) {
|
||||
const num = parseInt(m[1], 10);
|
||||
if (num >= 999) continue;
|
||||
if (num === 999) continue;
|
||||
if (num > maxPhase) maxPhase = num;
|
||||
}
|
||||
const phasesOnDisk = path.join(planningDir(cwd), 'phases');
|
||||
@@ -695,7 +695,7 @@ function cmdPhaseAddBatch(cwd, descriptions, raw) {
|
||||
const match = entry.match(dirNumPattern);
|
||||
if (!match) continue;
|
||||
const num = parseInt(match[1], 10);
|
||||
if (num >= 999) continue;
|
||||
if (num === 999) continue;
|
||||
if (num > maxPhase) maxPhase = num;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -60,7 +60,7 @@ export function extractOneLinerFromBody(content: string): string | null {
|
||||
|
||||
/**
|
||||
* Scan highest sequential phase number in milestone content.
|
||||
* Skips backlog lanes (`999.x`).
|
||||
* Skips exactly the backlog sentinel (`999`); phases 1000+ are valid canonical IDs.
|
||||
*/
|
||||
export function scanSequentialMaxPhaseFromMilestone(milestoneContent: string): number {
|
||||
const phasePattern = /(?:^|\n)\s*(?:[-*]\s*(?:\[[x ]\]\s*)?|#{2,4}\s*|\*{1,2}\s*)Phase\s+(\d+)[A-Z]?(?:\.\d+)*:/gi;
|
||||
@@ -68,7 +68,7 @@ export function scanSequentialMaxPhaseFromMilestone(milestoneContent: string): n
|
||||
let m: RegExpExecArray | null;
|
||||
while ((m = phasePattern.exec(milestoneContent)) !== null) {
|
||||
const num = parseInt(m[1], 10);
|
||||
if (num >= 999) continue;
|
||||
if (num === 999) continue;
|
||||
if (num > maxPhase) maxPhase = num;
|
||||
}
|
||||
return maxPhase;
|
||||
@@ -85,7 +85,7 @@ export function scanSequentialMaxPhaseFromDirs(dirNames: string[]): number {
|
||||
const match = dirNumPattern.exec(dirName);
|
||||
if (!match) continue;
|
||||
const num = parseInt(match[1], 10);
|
||||
if (num >= 999) continue;
|
||||
if (num === 999) continue;
|
||||
if (num > maxPhase) maxPhase = num;
|
||||
}
|
||||
return maxPhase;
|
||||
|
||||
@@ -325,7 +325,7 @@ describe('phaseAdd', () => {
|
||||
expect(roadmap).toContain('**Goal:** [To be planned]');
|
||||
});
|
||||
|
||||
it('skips phases >= 999 when calculating next number (backlog exclusion)', async () => {
|
||||
it('skips exactly phase 999 (backlog sentinel) when calculating next number', async () => {
|
||||
const { phaseAdd } = await import('./phase-lifecycle.js');
|
||||
const roadmapWith999 = MINIMAL_ROADMAP.replace(
|
||||
'---\n*Last updated',
|
||||
@@ -339,6 +339,105 @@ describe('phaseAdd', () => {
|
||||
expect(data.phase_number).toBe(11);
|
||||
});
|
||||
|
||||
it('returns correct next phase id for projects using 1000+ canonical phase numbers (regression #3774)', async () => {
|
||||
// Bug: scanSequentialMaxPhaseFromMilestone and scanSequentialMaxPhaseFromDirs
|
||||
// used `num >= 999` instead of `num === 999`, causing every phase ≥ 1000 to be
|
||||
// excluded from the max-scan. computeNextSequentialPhaseId returned 1 (0+1)
|
||||
// instead of 1501 for a project whose highest phase is 1500.
|
||||
const { phaseAdd } = await import('./phase-lifecycle.js');
|
||||
|
||||
const roadmapWith1000Plus = [
|
||||
'# Roadmap',
|
||||
'',
|
||||
'## Current Milestone: v10.0 Large Project',
|
||||
'',
|
||||
'### Phase 1000: Foundation',
|
||||
'',
|
||||
'**Goal:** Foundation',
|
||||
'**Requirements**: TBD',
|
||||
'**Plans:** 1 plans',
|
||||
'',
|
||||
'Plans:',
|
||||
'- [x] 1000-01 (Foundation setup)',
|
||||
'',
|
||||
'### Phase 1500: Latest',
|
||||
'',
|
||||
'**Goal:** Latest',
|
||||
'**Requirements**: TBD',
|
||||
'**Depends on:** Phase 1499',
|
||||
'**Plans:** 1 plans',
|
||||
'',
|
||||
'Plans:',
|
||||
'- [x] 1500-01 (Latest step)',
|
||||
'',
|
||||
'---',
|
||||
'*Last updated: 2026-05-20*',
|
||||
'',
|
||||
].join('\n');
|
||||
|
||||
const phases = [
|
||||
'1000-foundation',
|
||||
'1100-alpha',
|
||||
'1200-beta',
|
||||
'1300-gamma',
|
||||
'1400-delta',
|
||||
'1500-latest',
|
||||
];
|
||||
|
||||
await setupTestProject(tmpDir, { roadmap: roadmapWith1000Plus, phases });
|
||||
|
||||
const result = await phaseAdd(['Next After 1500'], tmpDir);
|
||||
const data = result.data as Record<string, unknown>;
|
||||
// Must be 1501, not 1 (the pre-fix bug value)
|
||||
expect(data.phase_number).toBe(1501);
|
||||
});
|
||||
|
||||
it('distinguishes === 999 guard from === 1000 off-by-one: [999, 1000] fixture must yield 1001 (regression #3774)', async () => {
|
||||
// Distinguishing fixture: phases [999, 1000] on disk + in ROADMAP.
|
||||
// With correct guard (=== 999): skips 999, keeps 1000 as max → next = 1001 ✓
|
||||
// With off-by-one guard (=== 1000): skips 1000, keeps 999 → max = 999 but
|
||||
// 999 is itself skipped by the equality guard (999 === 999 is true when
|
||||
// the guard fires), so actually keeps nothing → max = 0 → next = 1 ✗.
|
||||
// Either way, the result diverges from 1001, catching the regression.
|
||||
const { phaseAdd } = await import('./phase-lifecycle.js');
|
||||
|
||||
const roadmapWith999and1000 = [
|
||||
'# Roadmap',
|
||||
'',
|
||||
'## Current Milestone: v1.0',
|
||||
'',
|
||||
'### Phase 999: Backlog',
|
||||
'',
|
||||
'**Goal:** Backlog sentinel',
|
||||
'**Plans:** 0 plans',
|
||||
'',
|
||||
'### Phase 1000: First Four-Digit Phase',
|
||||
'',
|
||||
'**Goal:** First canonical phase above backlog sentinel',
|
||||
'**Requirements**: TBD',
|
||||
'**Plans:** 1 plans',
|
||||
'',
|
||||
'Plans:',
|
||||
'- [x] 1000-01 (initial work)',
|
||||
'',
|
||||
'---',
|
||||
'*Last updated: 2026-05-21*',
|
||||
'',
|
||||
].join('\n');
|
||||
|
||||
const phases = [
|
||||
'999-backlog',
|
||||
'1000-first-four-digit',
|
||||
];
|
||||
|
||||
await setupTestProject(tmpDir, { roadmap: roadmapWith999and1000, phases });
|
||||
|
||||
const result = await phaseAdd(['After One Thousand'], tmpDir);
|
||||
const data = result.data as Record<string, unknown>;
|
||||
// Must be 1001: skips 999 (backlog sentinel), keeps 1000 as max, adds 1
|
||||
expect(data.phase_number).toBe(1001);
|
||||
});
|
||||
|
||||
it('throws GSDError with Validation for empty description', async () => {
|
||||
const { phaseAdd } = await import('./phase-lifecycle.js');
|
||||
await setupTestProject(tmpDir);
|
||||
|
||||
@@ -932,6 +932,62 @@ describe('phase add command', () => {
|
||||
'directory should be 04-dashboard, not 1000-dashboard'
|
||||
);
|
||||
});
|
||||
|
||||
test('CJS scanner [999, 1000] fixture: skips exactly 999 and returns 1001 (regression #3774)', () => {
|
||||
// Locks the BLOCKER fix in phase.cjs: guards at :610, :624, :688, :698 must
|
||||
// use === 999 (not >= 999). With >= 999, phase 1000 is excluded from the
|
||||
// max-scan and the result collapses back toward 1 instead of 1001.
|
||||
//
|
||||
// GSD_WORKSTREAM=ws1 forces the CJS fallback in phase-command-router.cjs.
|
||||
// When GSD_WORKSTREAM is set, planningDir resolves to
|
||||
// .planning/workstreams/<ws>/ — so ROADMAP.md and phases/ live there.
|
||||
const ws = 'ws1';
|
||||
const planningBase = path.join(tmpDir, '.planning', 'workstreams', ws);
|
||||
fs.mkdirSync(path.join(planningBase, 'phases'), { recursive: true });
|
||||
|
||||
fs.writeFileSync(
|
||||
path.join(planningBase, 'ROADMAP.md'),
|
||||
[
|
||||
'# Roadmap',
|
||||
'',
|
||||
'## Current Milestone: v1.0',
|
||||
'',
|
||||
'### Phase 999: Backlog',
|
||||
'',
|
||||
'**Goal:** Backlog sentinel',
|
||||
'**Plans:** 0 plans',
|
||||
'',
|
||||
'### Phase 1000: First Four-Digit Phase',
|
||||
'',
|
||||
'**Goal:** First canonical phase above backlog sentinel',
|
||||
'**Requirements**: TBD',
|
||||
'**Plans:** 1 plans',
|
||||
'',
|
||||
'Plans:',
|
||||
'- [x] 1000-01 (initial work)',
|
||||
'',
|
||||
'---',
|
||||
'*Last updated: 2026-05-21*',
|
||||
'',
|
||||
].join('\n')
|
||||
);
|
||||
|
||||
// Create matching phase directories on disk (inside the workstream planning dir)
|
||||
fs.mkdirSync(path.join(planningBase, 'phases', '999-backlog'), { recursive: true });
|
||||
fs.mkdirSync(path.join(planningBase, 'phases', '1000-first-four-digit'), { recursive: true });
|
||||
|
||||
const result = runGsdTools('phase add After One Thousand', tmpDir, { GSD_WORKSTREAM: ws });
|
||||
assert.ok(result.success, `CJS phase add failed: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
// Must be 1001: skips 999 (backlog sentinel), keeps 1000, adds 1.
|
||||
// With the old >= 999 guard: phase 1000 is excluded → max stays 0 → result = 1.
|
||||
assert.strictEqual(output.phase_number, 1001, 'CJS scanner must return 1001, not 1 (regression #3774)');
|
||||
assert.ok(
|
||||
fs.existsSync(path.join(planningBase, 'phases', '1001-after-one-thousand')),
|
||||
'directory should be 1001-after-one-thousand'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user