From fe63ccad107c6fe19e6e54d71740f1370ef95b03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=82CHES?= Date: Sat, 21 Mar 2026 09:32:55 -0600 Subject: [PATCH] fix: dispatch guard uses dependency declarations instead of positional ordering (#1638) (#1770) The dispatch guard checked slices linearly by position, creating deadlocks when a positionally-earlier slice depended on a positionally-later one (e.g. S05 depends_on S06). Now checks declared dependencies for slices that have them, falling back to positional ordering for backward compat. Closes #1638 Co-authored-by: Claude Opus 4.6 (1M context) --- .../extensions/gsd/dispatch-guard.ts | 34 ++++++-- .../gsd/tests/dispatch-guard.test.ts | 87 ++++++++++++++++++- 2 files changed, 113 insertions(+), 8 deletions(-) diff --git a/src/resources/extensions/gsd/dispatch-guard.ts b/src/resources/extensions/gsd/dispatch-guard.ts index 717b711f9..9efcc378c 100644 --- a/src/resources/extensions/gsd/dispatch-guard.ts +++ b/src/resources/extensions/gsd/dispatch-guard.ts @@ -70,14 +70,34 @@ export function getPriorSliceCompletionBlocker( continue; } - const targetIndex = slices.findIndex((slice) => slice.id === targetSid); - if (targetIndex === -1) return null; + const targetSlice = slices.find((slice) => slice.id === targetSid); + if (!targetSlice) return null; - const incomplete = slices - .slice(0, targetIndex) - .find((slice) => !slice.done); - if (incomplete) { - return `Cannot dispatch ${unitType} ${unitId}: earlier slice ${targetMid}/${incomplete.id} is not complete.`; + // Dependency-aware ordering: if the target slice declares dependencies, + // only require those specific slices to be complete — not all positionally + // earlier slices. This prevents deadlocks when a positionally-earlier + // slice depends on a positionally-later one (e.g. S05 depends_on S06). + // + // When the target has NO declared dependencies, fall back to the original + // positional ordering for backward compatibility. + if (targetSlice.depends.length > 0) { + const sliceMap = new Map(slices.map((s) => [s.id, s])); + for (const depId of targetSlice.depends) { + const dep = sliceMap.get(depId); + if (dep && !dep.done) { + return `Cannot dispatch ${unitType} ${unitId}: dependency slice ${targetMid}/${depId} is not complete.`; + } + // If dep is not found in this milestone's slices, ignore it — + // it may be a cross-milestone reference handled elsewhere. + } + } else { + const targetIndex = slices.findIndex((slice) => slice.id === targetSid); + const incomplete = slices + .slice(0, targetIndex) + .find((slice) => !slice.done); + if (incomplete) { + return `Cannot dispatch ${unitType} ${unitId}: earlier slice ${targetMid}/${incomplete.id} is not complete.`; + } } } diff --git a/src/resources/extensions/gsd/tests/dispatch-guard.test.ts b/src/resources/extensions/gsd/tests/dispatch-guard.test.ts index 5d40b0e21..f60a5a857 100644 --- a/src/resources/extensions/gsd/tests/dispatch-guard.test.ts +++ b/src/resources/extensions/gsd/tests/dispatch-guard.test.ts @@ -38,7 +38,7 @@ test("dispatch guard blocks later slice in same milestone when earlier incomplet assert.equal( getPriorSliceCompletionBlocker(repo, "main", "execute-task", "M003/S02/T01"), - "Cannot dispatch execute-task M003/S02/T01: earlier slice M003/S01 is not complete.", + "Cannot dispatch execute-task M003/S02/T01: dependency slice M003/S01 is not complete.", ); } finally { rmSync(repo, { recursive: true, force: true }); @@ -59,6 +59,91 @@ test("dispatch guard allows dispatch when all earlier slices complete", () => { } }); +test("dispatch guard unblocks slice when positionally-earlier slice depends on it (#1638)", () => { + // S05 depends on S06, but S05 appears first positionally. + // Old behavior: S06 blocked because S05 (positionally earlier) is incomplete. + // Fixed behavior: S06 has no unmet dependencies, so it can dispatch. + const repo = mkdtempSync(join(tmpdir(), "gsd-dispatch-guard-")); + try { + mkdirSync(join(repo, ".gsd", "milestones", "M001"), { recursive: true }); + writeFileSync(join(repo, ".gsd", "milestones", "M001", "M001-ROADMAP.md"), + "# M001: Test\n\n## Slices\n" + + "- [x] **S01: Setup** `risk:low` `depends:[]`\n" + + "- [x] **S02: Core** `risk:low` `depends:[S01]`\n" + + "- [x] **S03: API** `risk:low` `depends:[S02]`\n" + + "- [x] **S04: Auth** `risk:low` `depends:[S03]`\n" + + "- [ ] **S05: Integration** `risk:high` `depends:[S04,S06]`\n" + + "- [ ] **S06: Data Layer** `risk:medium` `depends:[S04]`\n"); + + // S06 depends only on S04 (complete) — should be unblocked + assert.equal( + getPriorSliceCompletionBlocker(repo, "main", "plan-slice", "M001/S06"), + null, + ); + + // S05 depends on S04 (complete) and S06 (incomplete) — should be blocked + assert.equal( + getPriorSliceCompletionBlocker(repo, "main", "plan-slice", "M001/S05"), + "Cannot dispatch plan-slice M001/S05: dependency slice M001/S06 is not complete.", + ); + } finally { + rmSync(repo, { recursive: true, force: true }); + } +}); + +test("dispatch guard falls back to positional ordering when no dependencies declared", () => { + const repo = mkdtempSync(join(tmpdir(), "gsd-dispatch-guard-")); + try { + mkdirSync(join(repo, ".gsd", "milestones", "M001"), { recursive: true }); + writeFileSync(join(repo, ".gsd", "milestones", "M001", "M001-ROADMAP.md"), + "# M001: Test\n\n## Slices\n" + + "- [x] **S01: First** `risk:low` `depends:[]`\n" + + "- [ ] **S02: Second** `risk:low` `depends:[]`\n" + + "- [ ] **S03: Third** `risk:low` `depends:[]`\n"); + + // S03 has no dependencies — positional fallback blocks on S02 + assert.equal( + getPriorSliceCompletionBlocker(repo, "main", "plan-slice", "M001/S03"), + "Cannot dispatch plan-slice M001/S03: earlier slice M001/S02 is not complete.", + ); + + // S02 has no dependencies — positional fallback: S01 is done, so unblocked + assert.equal( + getPriorSliceCompletionBlocker(repo, "main", "plan-slice", "M001/S02"), + null, + ); + } finally { + rmSync(repo, { recursive: true, force: true }); + } +}); + +test("dispatch guard allows slice with all declared dependencies complete", () => { + const repo = mkdtempSync(join(tmpdir(), "gsd-dispatch-guard-")); + try { + mkdirSync(join(repo, ".gsd", "milestones", "M001"), { recursive: true }); + writeFileSync(join(repo, ".gsd", "milestones", "M001", "M001-ROADMAP.md"), + "# M001: Test\n\n## Slices\n" + + "- [x] **S01: Setup** `risk:low` `depends:[]`\n" + + "- [x] **S02: Core** `risk:low` `depends:[S01]`\n" + + "- [ ] **S03: Feature A** `risk:low` `depends:[S01,S02]`\n" + + "- [ ] **S04: Feature B** `risk:low` `depends:[S01]`\n"); + + // S03 depends on S01 (done) and S02 (done) — unblocked + assert.equal( + getPriorSliceCompletionBlocker(repo, "main", "plan-slice", "M001/S03"), + null, + ); + + // S04 depends only on S01 (done) — unblocked even though S03 is incomplete + assert.equal( + getPriorSliceCompletionBlocker(repo, "main", "plan-slice", "M001/S04"), + null, + ); + } finally { + rmSync(repo, { recursive: true, force: true }); + } +}); + test("dispatch guard works without git repo", () => { const repo = mkdtempSync(join(tmpdir(), "gsd-dispatch-guard-nogit-")); try {