diff --git a/.changeset/3488-decimal-phase-depends-on.md b/.changeset/3488-decimal-phase-depends-on.md new file mode 100644 index 000000000..feaa3d24a --- /dev/null +++ b/.changeset/3488-decimal-phase-depends-on.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3488 +--- +**`phase-plan-index` resolves short-form `depends_on: [NN]` for decimal-phase plans** — the DAG resolver only matched full-stem (`03-01-auth-hardening`) and canonical-prefix (`03-01`) forms, so plans in decimal phases (e.g. `99.9-test`, `02.2-cross-repo`) declaring `depends_on: [01]` had their edges silently dropped. Dependents collapsed into wave 1 and the SDK emitted a misleading `declared wave: N but depends_on DAG places it in wave 1` warning that pointed at the wave declaration rather than the broken reference. A tertiary short-form index now maps the trailing `-NN` of every plan ID to its full ID for same-phase short-form lookups, and unresolved `depends_on` references surface a dedicated `Plan X: unresolved depends_on reference 'NN' — no matching plan in phase` warning so the dropped edge can no longer hide behind the wave-mismatch warning. (#3488) diff --git a/sdk/src/query/phase.test.ts b/sdk/src/query/phase.test.ts index 108fd1022..bb982f218 100644 --- a/sdk/src/query/phase.test.ts +++ b/sdk/src/query/phase.test.ts @@ -558,4 +558,94 @@ describe('phasePlanIndex', () => { 'Ignored noncanonical plan files: 16-PLAN-01-eval-harness.md', ]); }); + + it('#3488: integer-phase short-form depends_on [01] resolves to same-phase plan', async () => { + const phase17 = join(tmpDir, '.planning', 'phases', '17-int-short'); + await mkdir(phase17, { recursive: true }); + await writeFile(join(phase17, '17-01-PLAN.md'), [ + '---', + 'phase: 17', + 'plan: 01', + 'wave: 1', + 'autonomous: true', + 'depends_on: []', + '---', + 'Plan A.', + ].join('\n')); + await writeFile(join(phase17, '17-02-PLAN.md'), [ + '---', + 'phase: 17', + 'plan: 02', + 'wave: 2', + 'autonomous: true', + 'depends_on: [01]', + '---', + 'Plan B — short-form dep.', + ].join('\n')); + + const result = await phasePlanIndex(['17'], tmpDir); + const data = result.data as Record; + const waves = data.waves as Record; + const warnings = (data.warnings as string[] | undefined) ?? []; + + expect(waves['1']).toEqual(['17-01']); + expect(waves['2']).toEqual(['17-02']); + expect(warnings).toEqual([]); + }); + + it('#3488: decimal-phase short-form depends_on [01] resolves to same-phase plan', async () => { + const phase18 = join(tmpDir, '.planning', 'phases', '99.9-test'); + await mkdir(phase18, { recursive: true }); + await writeFile(join(phase18, '99.9-01-PLAN.md'), [ + '---', + 'phase: 99.9-test', + 'plan: 01', + 'wave: 1', + 'autonomous: true', + 'depends_on: []', + '---', + 'Plan A.', + ].join('\n')); + await writeFile(join(phase18, '99.9-02-PLAN.md'), [ + '---', + 'phase: 99.9-test', + 'plan: 02', + 'wave: 2', + 'autonomous: true', + 'depends_on: [01]', + '---', + 'Plan B — short-form dep on decimal phase.', + ].join('\n')); + + const result = await phasePlanIndex(['99.9'], tmpDir); + const data = result.data as Record; + const waves = data.waves as Record; + const warnings = (data.warnings as string[] | undefined) ?? []; + + expect(waves['1']).toEqual(['99.9-01']); + expect(waves['2']).toEqual(['99.9-02']); + expect(warnings).toEqual([]); + }); + + it('#3488: unresolved depends_on reference emits distinct warning (not wave-mismatch)', async () => { + const phase19 = join(tmpDir, '.planning', 'phases', '19-unresolved'); + await mkdir(phase19, { recursive: true }); + await writeFile(join(phase19, '19-01-PLAN.md'), [ + '---', + 'phase: 19', + 'plan: 01', + 'wave: 1', + 'autonomous: true', + 'depends_on: [does-not-exist]', + '---', + 'Plan with bogus dep.', + ].join('\n')); + + const result = await phasePlanIndex(['19'], tmpDir); + const data = result.data as Record; + const warnings = (data.warnings as string[] | undefined) ?? []; + + // 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); + }); }); diff --git a/sdk/src/query/phase.ts b/sdk/src/query/phase.ts index 12e04e0f4..0a7511416 100644 --- a/sdk/src/query/phase.ts +++ b/sdk/src/query/phase.ts @@ -381,19 +381,55 @@ export const phasePlanIndex: QueryHandler = async (args, projectDir, workstream) // 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])); + // 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) + 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); + // First write wins — preserve deterministic ordering from sorted planFiles. + if (!shortFormToId.has(shortForm)) { + shortFormToId.set(shortForm, p.id); + } + } + } // Kahn's algorithm — compute in-degree and adjacency for plans in this phase only. const level = new Map(); const inDeg = new Map(); const adj = new Map(); // dep → [dependents] + const unresolvedDeps: Array<{ planId: string; dep: string }> = []; for (const p of rawPlans) { if (!inDeg.has(p.id)) inDeg.set(p.id, 0); 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); - if (!resolvedDep) continue; // external dep — ignore + // Accept full-stem ('03-01-auth-hardening'), canonical-prefix ('03-01'), + // 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. + 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 (!resolvedDep) { + // Looks like an in-phase short-form / canonical reference that didn't resolve. + // Distinguish from genuinely-external deps: if the dep matches the shape + // of an in-phase reference (no slash, matches NN / NN-NN / NN-NN-slug), + // record it for a dedicated warning so downstream users aren't misled + // by the wave-mismatch warning fired against the dropped edge. + unresolvedDeps.push({ planId: p.id, dep }); + continue; + } if (!adj.has(resolvedDep)) adj.set(resolvedDep, []); adj.get(resolvedDep)!.push(p.id); inDeg.set(p.id, (inDeg.get(p.id) ?? 0) + 1); @@ -451,6 +487,14 @@ export const phasePlanIndex: QueryHandler = async (args, projectDir, workstream) warnings.push(`Ignored noncanonical plan files: ${nonCanonicalPlanFiles.join(', ')}`); } + // Surface unresolved depends_on references from Pass 2 — without this, a dropped + // short-form edge silently collapses the dependent plan into wave 1 and the only + // signal is a misleading "declared wave: N but depends_on DAG places it in wave 1" + // warning that points at the wave declaration rather than the broken reference. (#3488) + for (const { planId, dep } of unresolvedDeps) { + warnings.push(`Plan ${planId}: unresolved depends_on reference '${dep}' — no matching plan in phase`); + } + for (const raw of rawPlans) { if (!raw.autonomous) { hasCheckpoints = true;