From 8b6ffca51f45db4f5cb410172e8d96824e40043b Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 22 May 2026 16:54:10 -0400 Subject: [PATCH] fix(3785): case-insensitive depends_on resolution in phase resolver (#88) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(3785): case-insensitive depends_on resolution in phase resolver planMap, canonicalToId, and shortFormToId in phasePlanIndex used strict Map.has() with no case normalization. A depends_on ref in mixed/lowercase against an uppercase-suffix plan ID (e.g. '20-01-auth' → '20-01-Auth') dropped the DAG edge, assigning the dependent plan to wave 1 instead of wave 2. Fix: normalize all keys and lookup values to lowercase so the three-tier resolution is case-insensitive. Adds regression test (#3785). Co-Authored-By: Claude Sonnet 4.6 * chore(changeset): add PR 3798 changelog fragment Co-Authored-By: Claude Sonnet 4.6 * fix(3785): detect case-fold collisions; apply lowercase-both to CJS path - Add collision guard in both sdk/src/query/phase.ts (phasePlanIndex) and get-shit-done/bin/lib/phase.cjs (cmdPhasePlanIndex): when two plan IDs in the same phase are identical after toLowerCase(), throw/error immediately with a clear message naming both files instead of silently overwriting one in planMap and misrouting depends_on edges. - Apply the same lowercase-both normalization (#3785) to cmdPhasePlanIndex in phase.cjs, which was missing from the original PR — planMap and canonicalToId keys are now lowercased on write; dep strings are lowercased before lookup. - Add regression tests to tests/phase.test.cjs: case-insensitive resolution test (runs on all platforms) and collision-detection test (skipped on macOS/Windows where FS is case-insensitive). - Update changeset to describe both the SDK and CJS fixes. Identified via Codex adversarial review of PR #3798. Co-Authored-By: Claude Sonnet 4.6 * fix(3785): address review — KNOWN GAP comment for CJS shortFormToId, canonical-casing tests, depends_on output normalization - F1: Reword changeset for accuracy (plannerID drift trigger; two-tier CJS gap honest). Add KNOWN GAP comment in phase.cjs before Kahn's loop noting CJS lacks shortFormToId (tracked as follow-up parity gap, out of scope for #3785). - F2: Add strict planA.id === '20-01-Auth' assertions in both SDK (vitest) and CJS (node --test) tests — a future regression that silently lowercases stored IDs would now fail the test. - F3: Normalize depends_on output to canonical plan IDs in both SDK phase.ts and CJS phase.cjs. User-typed '20-01-auth' in depends_on resolves to '20-01-Auth' in output via planMap lookup. Add planB.depends_on === ['20-01-Auth'] assertions in both test suites. - F5: Add seenLower guard-scope comment (full-ID collisions only; shared-prefix collisions handled by first-write-wins from sorted planFiles). - F6: Add ASCII-safe toLowerCase comment at first call site in both SDK and CJS. - F7: Add intentional-separation comment on seenLower vs planMap in both SDK and CJS. Reviewers: gsd-code-reviewer (MN-01/NT-1) + sonnet adversarial. Co-Authored-By: Claude Sonnet 4.6 * test(3785): cover case-insensitive depends_on resolution branches Add 4 focused test cases exercising branches introduced by #3785: - All-uppercase depends_on ref resolving to lowercase plan ID via planMap - External cross-phase dep preserved as-is in Pass 3 output (planMap miss) - Mixed-case short canonical prefix resolving via canonicalToId - Plans with undefined/empty depends_on emit empty array correctly Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- .changeset/clever-yaks-cheer.md | 5 + get-shit-done/bin/lib/phase.cjs | 48 +++++++- sdk/src/query/phase.test.ts | 91 +++++++++++++++ sdk/src/query/phase.ts | 60 ++++++++-- tests/phase.test.cjs | 201 ++++++++++++++++++++++++++++++++ 5 files changed, 391 insertions(+), 14 deletions(-) create mode 100644 .changeset/clever-yaks-cheer.md diff --git a/.changeset/clever-yaks-cheer.md b/.changeset/clever-yaks-cheer.md new file mode 100644 index 000000000..35a7e5d7f --- /dev/null +++ b/.changeset/clever-yaks-cheer.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3798 +--- +**`phasePlanIndex` and `phase-plan-index` no longer drop `depends_on` edges when plan IDs and dep references differ in case** — fixes wrong wave assignment for plans with `depends_on` references that case-fold differently from canonical plan IDs (e.g. from gsd-planner output that occasionally lowercases IDs) now resolve correctly via case-insensitive lookups while preserving canonical on-disk casing in output. Lowercased identifier lookups now resolve case-mismatched `depends_on` references in the two resolution tiers present in both SDK and CJS (`planMap` + `canonicalToId`). The SDK has an additional `shortFormToId` short-form tier; CJS does not — this PR does not backfill that tier (tracked as a follow-up parity gap). An explicit collision error is thrown when two plan files in the same phase produce IDs that are identical after case-folding, preventing silent DAG rewiring. diff --git a/get-shit-done/bin/lib/phase.cjs b/get-shit-done/bin/lib/phase.cjs index 664aea800..862426239 100644 --- a/get-shit-done/bin/lib/phase.cjs +++ b/get-shit-done/bin/lib/phase.cjs @@ -444,12 +444,44 @@ function cmdPhasePlanIndex(cwd, phase, raw) { // ── Pass 2: topological level assignment via depends_on DAG ────────────── + // Guard: detect case-insensitive key collisions before building dependency + // maps. Two plan IDs that differ only by case would silently overwrite each + // other in planMap, routing depends_on edges to whichever plan survived last. + // This is a configuration error — fail fast with the conflicting IDs. (#3785) + // + // This guard catches case-fold collisions on full plan IDs. + // Shared-numeric-prefix collisions (e.g. '20-01-Auth' and '20-01' both + // producing canonical '20-01') are resolved by first-write-wins ordering + // from sorted planFiles — not explicitly guarded here. + // seenLower is intentionally separate from planMap — it exists only to detect + // collisions before planMap is built, so the error fires before any Map + // entry silently overwrites another. + const seenLower = new Map(); // lowercase key → original id + for (const p of rawPlans) { + // ASCII plan IDs only — toLowerCase() is correct and locale-safe here. + const lower = p.id.toLowerCase(); + const existing = seenLower.get(lower); + if (existing !== undefined) { + error(`depends_on index collision in phase ${normalized}: plan IDs '${existing}' and '${p.id}' are identical when case-folded. Rename one file to avoid ambiguous dependency resolution.`); + return; + } + seenLower.set(lower, p.id); + } + // Build a map from plan ID → raw plan for fast lookup. // Deps that reference plans outside this phase are treated as external and ignored. - const planMap = new Map(rawPlans.map(p => [p.id, p])); + // Keys are lowercased so that depends_on refs with different casing still + // resolve to the correct plan (#3785: case-insensitive identifier resolution). + const planMap = new Map(rawPlans.map(p => [p.id.toLowerCase(), p])); // Secondary index: canonical prefix → full plan ID, so depends_on: ['03-01'] resolves // to '03-01-auth-hardening-PLAN.md'-derived ID '03-01-auth-hardening' (k015). - const canonicalToId = new Map(rawPlans.map(p => [extractCanonicalPlanId(p.id), p.id])); + // Keyed lowercase for the same case-insensitive reason (#3785). + const canonicalToId = new Map(rawPlans.map(p => [extractCanonicalPlanId(p.id).toLowerCase(), p.id])); + + // KNOWN GAP: CJS resolver has only two tiers (planMap + canonicalToId); + // the SDK has an additional shortFormToId for same-phase short-form refs + // like '01' or '01A'. Adding the third tier here is tracked as a parity + // gap and is out of scope for #3785 / PR #3798. // Kahn's algorithm — compute in-degree and adjacency for in-phase deps only. const level = new Map(); @@ -461,7 +493,9 @@ function cmdPhasePlanIndex(cwd, phase, raw) { if (!adj.has(p.id)) adj.set(p.id, []); for (const dep of p.dependsOn) { // Accept both full-stem ('03-01-auth-hardening') and canonical-prefix ('03-01') forms. - const resolvedDep = planMap.has(dep) ? dep : canonicalToId.get(dep); + // All lookups are lowercased so mixed-case depends_on refs resolve correctly (#3785). + const depLower = dep.toLowerCase(); + const resolvedDep = planMap.has(depLower) ? planMap.get(depLower).id : canonicalToId.get(depLower); if (!resolvedDep) continue; // external dep — ignore if (!adj.has(resolvedDep)) adj.set(resolvedDep, []); adj.get(resolvedDep).push(p.id); @@ -537,7 +571,13 @@ function cmdPhasePlanIndex(cwd, phase, raw) { const plan = { id: raw.id, wave: effectiveWave, - depends_on: raw.dependsOn, + // Resolve each user-typed dep to its canonical plan ID (preserving on-disk casing) + // so the output never reflects the user's case typo. Unresolved deps (external + // phase refs) are kept as-is since planMap only contains plans in this phase. + depends_on: raw.dependsOn.map(dep => { + const lower = String(dep).toLowerCase(); + return planMap.has(lower) ? planMap.get(lower).id : dep; + }), autonomous: raw.autonomous, objective: raw.objective, files_modified: raw.filesModified, diff --git a/sdk/src/query/phase.test.ts b/sdk/src/query/phase.test.ts index e3a87bc94..f9fd84c17 100644 --- a/sdk/src/query/phase.test.ts +++ b/sdk/src/query/phase.test.ts @@ -690,4 +690,95 @@ describe('phasePlanIndex', () => { // A clear, dedicated warning naming the unresolved reference must surface. expect(warnings.some(w => /unresolved/i.test(w) && w.includes('does-not-exist'))).toBe(true); }); + + it('#3785: depends_on resolution is case-insensitive — mixed-case plan ID resolves correctly', async () => { + // Regression: planMap / canonicalToId / shortFormToId used strict Map.has() with no + // case normalization. A depends_on ref in lowercase against an uppercase-suffix plan + // ID dropped the DAG edge, causing the dependent plan to land in wave 1 instead of + // wave 2 (wrong ordering). + const phase20 = join(tmpDir, '.planning', 'phases', '20-case-insensitive'); + await mkdir(phase20, { recursive: true }); + // Plan A: filename uses uppercase suffix (e.g. user named it "Phase-2-FOO") + await writeFile(join(phase20, '20-01-Auth-PLAN.md'), [ + '---', + 'phase: 20', + 'plan: 01', + 'wave: 1', + 'autonomous: true', + 'depends_on: []', + '---', + 'Plan A — uppercase suffix in filename.', + ].join('\n')); + // Plan B: depends_on references Plan A using a different case than the plan ID + await writeFile(join(phase20, '20-02-PLAN.md'), [ + '---', + 'phase: 20', + 'plan: 02', + 'wave: 2', + 'autonomous: true', + 'depends_on:', + ' - 20-01-auth', + '---', + 'Plan B — lowercase dep ref to mixed-case plan A.', + ].join('\n')); + + const result = await phasePlanIndex(['20'], tmpDir); + const data = result.data as Record; + const plans = data.plans as Array>; + const waves = data.waves as Record; + const warnings = (data.warnings as string[] | undefined) ?? []; + + const planA = plans.find(p => (p.id as string).startsWith('20-01')); + const planB = plans.find(p => p.id === '20-02'); + expect(planA).toBeDefined(); + expect(planB).toBeDefined(); + + // Lock canonical casing: plan ID must be preserved as-is from the filename, not lowercased. + expect(planA!.id).toBe('20-01-Auth'); + + // The DAG edge must resolve: B must be in a later wave than A. + expect(planA!.wave).toBeLessThan(planB!.wave as number); + // Structurally: A in wave 1, B in wave 2. + expect(waves['1']).toBeDefined(); + expect(waves['2']).toBeDefined(); + expect((waves['1'] as string[]).some(id => (id as string).startsWith('20-01'))).toBe(true); + expect(waves['2']).toContain('20-02'); + // depends_on output must use canonical plan ID, not the user-typed casing. + expect(planB!.depends_on).toEqual(['20-01-Auth']); + // No unresolved-dep warning should be emitted. + expect(warnings.some(w => /unresolved/i.test(w))).toBe(false); + }); + + // This test can only run on Linux where the filesystem is case-sensitive. + // On macOS/Windows (case-insensitive FS), writing both files silently collapses + // them to one file, so the collision scenario cannot be triggered via disk. + const itLinuxOnly = process.platform === 'linux' ? it : it.skip; + itLinuxOnly('#3785 adversarial: case-insensitive collision — two plan IDs that differ only by case throw a clear error', async () => { + // Regression guard: if two plan files produce IDs that are identical when + // lowercased, planMap would silently overwrite one entry and route depends_on + // edges to whichever plan survived. This must fail fast instead. + const phase21 = join(tmpDir, '.planning', 'phases', '21-collision'); + await mkdir(phase21, { recursive: true }); + // These two filenames produce IDs '21-01-auth' and '21-01-Auth' — same when lowercased. + await writeFile(join(phase21, '21-01-auth-PLAN.md'), [ + '---', + 'phase: 21', + 'plan: 01', + 'autonomous: true', + 'depends_on: []', + '---', + 'Plan lowercase.', + ].join('\n')); + await writeFile(join(phase21, '21-01-Auth-PLAN.md'), [ + '---', + 'phase: 21', + 'plan: 01', + 'autonomous: true', + 'depends_on: []', + '---', + 'Plan uppercase.', + ].join('\n')); + + await expect(phasePlanIndex(['21'], tmpDir)).rejects.toThrow(/collision/i); + }); }); diff --git a/sdk/src/query/phase.ts b/sdk/src/query/phase.ts index 6d5ad0ea8..f668590f5 100644 --- a/sdk/src/query/phase.ts +++ b/sdk/src/query/phase.ts @@ -563,24 +563,55 @@ export const phasePlanIndex: QueryHandler = async (args, projectDir, workstream) // ── Pass 2: topological level assignment via depends_on DAG ────────────── + // Guard: detect case-insensitive key collisions before building dependency + // maps. Two plan IDs that differ only by case would silently overwrite each + // other in planMap, routing depends_on edges to whichever plan survived last. + // This is a configuration error — fail fast with the conflicting IDs so the + // author can rename one file. (#3785 follow-up from adversarial review) + // + // This guard catches case-fold collisions on full plan IDs. + // Shared-numeric-prefix collisions (e.g. '20-01-Auth' and '20-01' both + // producing canonical '20-01') are resolved by first-write-wins ordering + // from sorted planFiles — not explicitly guarded here. + // seenLower is intentionally separate from planMap — it exists only to detect + // collisions before planMap is built, so the error fires before any Map + // entry silently overwrites another. + const seenLower = new Map(); // lowercase key → original id + for (const p of rawPlans) { + // ASCII plan IDs only — toLowerCase() is correct and locale-safe here. + const lower = p.id.toLowerCase(); + const existing = seenLower.get(lower); + if (existing !== undefined) { + throw new GSDError( + `depends_on index collision in phase ${normalized}: plan IDs '${existing}' and '${p.id}' are identical when case-folded. Rename one file to avoid ambiguous dependency resolution.`, + ErrorClassification.Execution, + ); + } + seenLower.set(lower, p.id); + } + // Build a map from plan ID → RawPlan for fast lookup. // Deps that reference plans outside this phase are silently ignored (treated // as already-satisfied external deps — the plan becomes a source node). - const planMap = new Map(rawPlans.map(p => [p.id, p])); + // Keys are lowercased so that depends_on refs with different casing still + // resolve to the correct plan (#3785: case-insensitive identifier resolution). + const planMap = new Map(rawPlans.map(p => [p.id.toLowerCase(), p])); // Secondary index: canonical prefix → full plan ID, so depends_on: ['03-01'] resolves // to '03-01-auth-hardening-PLAN.md'-derived ID '03-01-auth-hardening' (k015). - const canonicalToId = new Map(rawPlans.map(p => [extractCanonicalPlanId(p.id), p.id])); + // Keyed lowercase for the same case-insensitive reason (#3785). + const canonicalToId = new Map(rawPlans.map(p => [extractCanonicalPlanId(p.id).toLowerCase(), p.id])); // Tertiary index: same-phase short-form ('01') → full plan ID, derived from each plan's // canonical '-' by splitting on the LAST '-'. The phase segment may // contain dots (e.g. '99.9') or letters (e.g. '02A'); only the trailing '-NN' is the // short form. Same-phase plans share a phase prefix so '01' is unambiguous within a // single phase-plan-index call. (#3488) + // Keyed lowercase for the same case-insensitive reason (#3785). const shortFormToId = new Map(); for (const p of rawPlans) { const canonical = extractCanonicalPlanId(p.id); const lastDash = canonical.lastIndexOf('-'); if (lastDash > 0 && lastDash < canonical.length - 1) { - const shortForm = canonical.slice(lastDash + 1); + const shortForm = canonical.slice(lastDash + 1).toLowerCase(); // First write wins — preserve deterministic ordering from sorted planFiles. if (!shortFormToId.has(shortForm)) { shortFormToId.set(shortForm, p.id); @@ -602,13 +633,16 @@ export const phasePlanIndex: QueryHandler = async (args, projectDir, workstream) // and same-phase short-form ('01') forms. The short-form lookup (#3488) // is keyed off the plan-id suffix so it works for integer ('99'), letter // ('02A'), and decimal ('99.9') phase IDs alike. + // All lookups are lowercased so mixed-case depends_on refs resolve + // correctly regardless of the case used in the plan filename (#3785). + const depLower = dep.toLowerCase(); let resolvedDep: string | undefined; - if (planMap.has(dep)) { - resolvedDep = dep; - } else if (canonicalToId.has(dep)) { - resolvedDep = canonicalToId.get(dep); - } else if (shortFormToId.has(dep)) { - resolvedDep = shortFormToId.get(dep); + if (planMap.has(depLower)) { + resolvedDep = planMap.get(depLower)!.id; + } else if (canonicalToId.has(depLower)) { + resolvedDep = canonicalToId.get(depLower); + } else if (shortFormToId.has(depLower)) { + resolvedDep = shortFormToId.get(depLower); } if (!resolvedDep) { // Looks like an in-phase short-form / canonical reference that didn't resolve. @@ -703,7 +737,13 @@ export const phasePlanIndex: QueryHandler = async (args, projectDir, workstream) const plan: Record = { id: raw.id, wave: effectiveWave, - depends_on: raw.dependsOn, + // Resolve each user-typed dep to its canonical plan ID (preserving on-disk casing) + // so the output never reflects the user's case typo. Unresolved deps (external + // phase refs) are kept as-is since planMap only contains plans in this phase. + depends_on: raw.dependsOn.map(dep => { + const lower = String(dep).toLowerCase(); + return planMap.get(lower)?.id ?? dep; + }), autonomous: raw.autonomous, objective: raw.objective, files_modified: raw.filesModified, diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index 0f2aee81b..0b12e6611 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -567,6 +567,207 @@ objective: Manual review needed const output = JSON.parse(result.output); assert.strictEqual(output.error, 'Phase not found', 'should report phase not found'); }); + + // #3785 — case-insensitive depends_on resolution + test('#3785: depends_on reference with different case resolves to correct plan', () => { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '20-case-insensitive'); + fs.mkdirSync(phaseDir, { recursive: true }); + + // Plan A: filename uses uppercase suffix — plan ID becomes '20-01-Auth' + fs.writeFileSync( + path.join(phaseDir, '20-01-Auth-PLAN.md'), + `---\nwave: 1\nautonomous: true\ndepends_on: []\n---\nPlan A.\n`, + ); + // Plan B: depends_on uses lowercase — must still resolve to Plan A + fs.writeFileSync( + path.join(phaseDir, '20-02-PLAN.md'), + `---\nwave: 2\nautonomous: true\ndepends_on:\n - 20-01-auth\n---\nPlan B.\n`, + ); + + const result = runGsdTools('phase-plan-index 20', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + const waves = output.waves; + + const wave01 = Object.keys(waves).find(w => waves[w].some(id => id.startsWith('20-01'))); + const wave02 = Object.keys(waves).find(w => waves[w].some(id => id.startsWith('20-02'))); + assert.ok(wave01 !== undefined, 'plan 20-01-Auth should appear in waves'); + assert.ok(wave02 !== undefined, 'plan 20-02 should appear in waves'); + assert.ok( + Number(wave01) < Number(wave02), + `20-02 must be in a later wave than 20-01 (got wave01=${wave01}, wave02=${wave02}) — DAG edge dropped (case mismatch)`, + ); + // Lock canonical casing: plan ID must be preserved as-is from the filename, not lowercased. + const planA = output.plans.find(p => p.id.startsWith('20-01')); + assert.strictEqual(planA.id, '20-01-Auth', 'canonical casing must be preserved in plan ID'); + // depends_on output must use canonical plan ID, not the user-typed casing. + const planB = output.plans.find(p => p.id === '20-02'); + assert.deepStrictEqual(planB.depends_on, ['20-01-Auth'], 'depends_on output must resolve to canonical ID casing'); + // No unresolved-dep warning + const warnings = output.warnings ?? []; + assert.ok( + !warnings.some(w => /unresolved/i.test(w)), + `Unexpected unresolved-dep warning: ${JSON.stringify(warnings)}`, + ); + }); + + // #3785 adversarial: two plan IDs that are identical when case-folded must + // fail fast with a clear error instead of silently routing edges to the wrong plan. + // This test can only run on Linux where the filesystem is case-sensitive. + // On macOS/Windows (case-insensitive FS), writing both files silently collapses + // them to one file, so the collision scenario cannot be triggered via disk. + test('#3785 adversarial: two plan IDs differing only by case produce a collision error', { + skip: process.platform !== 'linux' ? 'case-insensitive filesystem — collision test requires Linux' : false, + }, () => { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '21-collision'); + fs.mkdirSync(phaseDir, { recursive: true }); + + // '21-01-auth-PLAN.md' → id '21-01-auth' + // '21-01-Auth-PLAN.md' → id '21-01-Auth' + // Both lowercase to '21-01-auth' — collision. + fs.writeFileSync( + path.join(phaseDir, '21-01-auth-PLAN.md'), + `---\nautonomous: true\ndepends_on: []\n---\nlowercase.\n`, + ); + fs.writeFileSync( + path.join(phaseDir, '21-01-Auth-PLAN.md'), + `---\nautonomous: true\ndepends_on: []\n---\nuppercase.\n`, + ); + + const result = runGsdTools('phase-plan-index 21', tmpDir); + // The command must exit with an error (non-success) naming the collision. + assert.ok(!result.success, 'phase-plan-index must fail when two plan IDs collide under case-folding'); + assert.ok( + /collision/i.test(result.error ?? result.output ?? ''), + `Error output must mention 'collision', got: ${result.error ?? result.output}`, + ); + }); + + // #3785 — all-uppercase depends_on value resolves to an all-lowercase plan ID + test('#3785: all-uppercase depends_on ref resolves to lowercase plan ID', () => { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '22-uppercase-dep'); + fs.mkdirSync(phaseDir, { recursive: true }); + + // Plan A: all-lowercase plan ID + fs.writeFileSync( + path.join(phaseDir, '22-01-setup-PLAN.md'), + `---\nwave: 1\nautonomous: true\ndepends_on: []\n---\nPlan A.\n`, + ); + // Plan B: depends_on uses ALL-UPPERCASE — must still route the DAG edge to Plan A + fs.writeFileSync( + path.join(phaseDir, '22-02-PLAN.md'), + `---\nwave: 2\nautonomous: true\ndepends_on:\n - 22-01-SETUP\n---\nPlan B.\n`, + ); + + const result = runGsdTools('phase-plan-index 22', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + const waves = output.waves; + + const wave01 = Object.keys(waves).find(w => waves[w].some(id => id.startsWith('22-01'))); + const wave02 = Object.keys(waves).find(w => waves[w].some(id => id.startsWith('22-02'))); + assert.ok(wave01 !== undefined, 'plan 22-01-setup should appear in waves'); + assert.ok(wave02 !== undefined, 'plan 22-02 should appear in waves'); + assert.ok( + Number(wave01) < Number(wave02), + `22-02 must be in a later wave than 22-01 — all-uppercase dep should route correctly (got wave01=${wave01}, wave02=${wave02})`, + ); + // depends_on output must use canonical plan ID (lowercase as-on-disk), not the uppercase ref + const planB = output.plans.find(p => p.id === '22-02'); + assert.deepStrictEqual(planB.depends_on, ['22-01-setup'], 'depends_on output must resolve to canonical lowercase ID'); + }); + + // #3785 — external (cross-phase) depends_on reference is kept as-is in output + // The Pass 3 mapping must return the original dep string when planMap has no entry for it. + test('#3785: external cross-phase depends_on ref is preserved as-is in output', () => { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '23-external-dep'); + fs.mkdirSync(phaseDir, { recursive: true }); + + // Plan A: references a plan in a different phase (01-some-other-phase) — planMap won't have it + fs.writeFileSync( + path.join(phaseDir, '23-01-PLAN.md'), + `---\nwave: 1\nautonomous: true\ndepends_on:\n - 01-01-prereq\n---\nPlan A.\n`, + ); + + const result = runGsdTools('phase-plan-index 23', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + const planA = output.plans.find(p => p.id.startsWith('23-01')); + assert.ok(planA !== undefined, 'plan 23-01 should appear in output'); + // External dep must be preserved verbatim — not dropped, not resolved + assert.deepStrictEqual(planA.depends_on, ['01-01-prereq'], 'external cross-phase dep must be kept as-is in output'); + }); + + // #3785 — mixed-case canonical prefix in depends_on resolves via canonicalToId lookup + // e.g. depends_on: '22-01-SETUP' where extractCanonicalPlanId gives '22-01' keyed lowercase + test('#3785: mixed-case short canonical prefix in depends_on resolves via canonicalToId', () => { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '24-canon-case'); + fs.mkdirSync(phaseDir, { recursive: true }); + + // Plan A: descriptive filename — id becomes '24-01-auth-hardening' + fs.writeFileSync( + path.join(phaseDir, '24-01-auth-hardening-PLAN.md'), + `---\nwave: 1\nautonomous: true\ndepends_on: []\n---\nPlan A.\n`, + ); + // Plan B: depends_on uses an uppercase short prefix '24-01' — canonicalToId maps '24-01' → '24-01-auth-hardening' + fs.writeFileSync( + path.join(phaseDir, '24-02-followup-PLAN.md'), + `---\nwave: 2\nautonomous: true\ndepends_on:\n - '24-01'\n---\nPlan B.\n`, + ); + + const result = runGsdTools('phase-plan-index 24', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + const waves = output.waves; + + const wave01 = Object.keys(waves).find(w => waves[w].some(id => id.startsWith('24-01'))); + const wave02 = Object.keys(waves).find(w => waves[w].some(id => id.startsWith('24-02'))); + assert.ok(wave01 !== undefined, '24-01-auth-hardening should appear in waves'); + assert.ok(wave02 !== undefined, '24-02-followup should appear in waves'); + assert.ok( + Number(wave01) < Number(wave02), + `24-02 must be in a later wave than 24-01 via canonicalToId lookup (got wave01=${wave01}, wave02=${wave02})`, + ); + // depends_on output: '24-01' is the canonical prefix, not in planMap directly, so falls back to dep as-is + const planB = output.plans.find(p => p.id === '24-02-followup'); + assert.ok(planB !== undefined, '24-02-followup plan must be in output'); + // The dep '24-01' is not a planMap key (full id is '24-01-auth-hardening'), so output keeps '24-01' + assert.deepStrictEqual(planB.depends_on, ['24-01'], 'short canonical prefix dep falls through to as-is in Pass 3 output'); + }); + + // #3785 — plans with no depends_on (empty array) still emit correct output without errors + test('#3785: plans with undefined/empty depends_on emit empty array without errors', () => { + const phaseDir = path.join(tmpDir, '.planning', 'phases', '25-no-deps'); + fs.mkdirSync(phaseDir, { recursive: true }); + + // Plan with no depends_on key at all + fs.writeFileSync( + path.join(phaseDir, '25-01-PLAN.md'), + `---\nwave: 1\nautonomous: true\n---\nPlan A, no deps.\n`, + ); + // Plan with explicit empty depends_on array + fs.writeFileSync( + path.join(phaseDir, '25-02-PLAN.md'), + `---\nwave: 1\nautonomous: true\ndepends_on: []\n---\nPlan B, explicit empty deps.\n`, + ); + + const result = runGsdTools('phase-plan-index 25', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + const plan01 = output.plans.find(p => p.id === '25-01'); + const plan02 = output.plans.find(p => p.id === '25-02'); + assert.ok(plan01 !== undefined, 'plan 25-01 should appear in output'); + assert.ok(plan02 !== undefined, 'plan 25-02 should appear in output'); + assert.deepStrictEqual(plan01.depends_on, [], 'plan with no depends_on key must emit empty array'); + assert.deepStrictEqual(plan02.depends_on, [], 'plan with explicit empty depends_on must emit empty array'); + // Both independent plans land in the same wave + assert.deepStrictEqual(output.waves['1'], ['25-01', '25-02'], 'both no-dep plans should be in wave 1'); + }); });