fix: prevent auto-mode from dispatching deferred slices (#3309)
* fix: prevent auto-mode from dispatching deferred slices (#2661) When a decision deferred a slice via gsd_decision_save, the deferral was recorded in DECISIONS.md but the slice status in the DB remained "active". The dispatcher reads slice status (not DECISIONS.md) to choose work, so it kept dispatching the deferred slice — burning tokens on the wrong work. Three changes fix the split-brain: 1. status-guards.ts: add isDeferredStatus() and isInactiveStatus() predicates 2. state.ts: skip slices with "deferred" status in active-slice selection 3. db-writer.ts: when saveDecisionToDb detects a deferral decision referencing a slice (M###/S## pattern in scope/choice/decision), update that slice's DB status to "deferred" so the state machine sees it immediately Closes #2661 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(db-writer): consistent deferral regex + add extractDeferredSliceRef tests The scope regex was missing "ring" and "s" variants (deferring, defers) while choice/decision had the complete pattern. Unified all three to use /\bdefer(?:ral|red|ring|s)?\b/i. Added 8 unit tests for extractDeferredSliceRef covering: scope/choice/ decision deferral detection, missing M###/S## patterns, "deferring" and "defers" variants, multiple pattern matches, and non-deferral keywords. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: retrigger CI --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: trek-e <trek-e@users.noreply.github.com>
This commit is contained in:
parent
e0a5f6866e
commit
fb63c15bc9
5 changed files with 340 additions and 0 deletions
|
|
@ -469,6 +469,23 @@ export async function saveDecisionToDb(
|
|||
adapter?.prepare('DELETE FROM decisions WHERE id = :id').run({ ':id': id });
|
||||
throw diskErr;
|
||||
}
|
||||
// #2661: When a decision defers a slice, update the slice status in the DB
|
||||
// so the dispatcher skips it. Without this, STATE.md and DECISIONS.md are
|
||||
// in split-brain: the decision says "deferred" but the state still says
|
||||
// "active", causing auto-mode to keep dispatching the deferred work.
|
||||
try {
|
||||
const sliceRef = extractDeferredSliceRef(fields);
|
||||
if (sliceRef) {
|
||||
db.updateSliceStatus(sliceRef.milestoneId, sliceRef.sliceId, 'deferred');
|
||||
}
|
||||
} catch (deferErr) {
|
||||
// Non-fatal — log but don't fail the decision save
|
||||
logError('manifest', 'failed to update deferred slice status', {
|
||||
fn: 'saveDecisionToDb',
|
||||
error: String((deferErr as Error).message),
|
||||
});
|
||||
}
|
||||
|
||||
// Invalidate file-read caches so deriveState() sees the updated markdown.
|
||||
// Do NOT clear the artifacts table — we just wrote to it intentionally.
|
||||
invalidateStateCache();
|
||||
|
|
@ -482,6 +499,39 @@ export async function saveDecisionToDb(
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract a milestone/slice reference from a deferral decision.
|
||||
*
|
||||
* Detects deferrals by checking:
|
||||
* - scope contains "defer" (e.g., "deferral", "defer")
|
||||
* - choice or decision contains "defer" + an M###/S## pattern
|
||||
*
|
||||
* Returns { milestoneId, sliceId } if found, null otherwise.
|
||||
*/
|
||||
export function extractDeferredSliceRef(
|
||||
fields: Pick<SaveDecisionFields, 'scope' | 'decision' | 'choice'>,
|
||||
): { milestoneId: string; sliceId: string } | null {
|
||||
const isDeferral =
|
||||
/\bdefer(?:ral|red|ring|s)?\b/i.test(fields.scope) ||
|
||||
/\bdefer(?:ral|red|ring|s)?\b/i.test(fields.choice) ||
|
||||
/\bdefer(?:ral|red|ring|s)?\b/i.test(fields.decision);
|
||||
|
||||
if (!isDeferral) return null;
|
||||
|
||||
// Look for M###/S## pattern in choice first, then decision
|
||||
const slicePattern = /\b(M\d{3,4})\/(S\d{2,3})\b/;
|
||||
const choiceMatch = fields.choice.match(slicePattern);
|
||||
if (choiceMatch) {
|
||||
return { milestoneId: choiceMatch[1], sliceId: choiceMatch[2] };
|
||||
}
|
||||
const decisionMatch = fields.decision.match(slicePattern);
|
||||
if (decisionMatch) {
|
||||
return { milestoneId: decisionMatch[1], sliceId: decisionMatch[2] };
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
// ─── Update Requirement in DB + Regenerate Markdown ───────────────────────
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -36,6 +36,7 @@ import {
|
|||
|
||||
import { findMilestoneIds } from './milestone-ids.js';
|
||||
import { loadQueueOrder, sortByQueueOrder } from './queue-order.js';
|
||||
import { isClosedStatus, isDeferredStatus } from './status-guards.js';
|
||||
import { nativeBatchParseGsdFiles, type BatchParsedFile } from './native-parser-bridge.js';
|
||||
|
||||
import { join, resolve } from 'path';
|
||||
|
|
@ -676,6 +677,10 @@ export async function deriveStateFromDb(basePath: string): Promise<GSDState> {
|
|||
|
||||
for (const s of activeMilestoneSlices) {
|
||||
if (isStatusDone(s.status)) continue;
|
||||
// #2661: Skip deferred slices — a decision explicitly deferred this work.
|
||||
// Without this guard the dispatcher would keep dispatching deferred slices
|
||||
// because DECISIONS.md is only contextual, not authoritative for dispatch.
|
||||
if (isDeferredStatus(s.status)) continue;
|
||||
if (s.depends.every(dep => doneSliceIds.has(dep))) {
|
||||
activeSlice = { id: s.id, title: s.title };
|
||||
activeSliceRow = s;
|
||||
|
|
|
|||
|
|
@ -12,3 +12,16 @@
|
|||
export function isClosedStatus(status: string): boolean {
|
||||
return status === "complete" || status === "done" || status === "skipped";
|
||||
}
|
||||
|
||||
/** Returns true when a slice status indicates it was deferred by a decision. */
|
||||
export function isDeferredStatus(status: string): boolean {
|
||||
return status === "deferred";
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns true when a slice should be skipped during active-slice selection.
|
||||
* This includes both closed (complete/done) and deferred slices.
|
||||
*/
|
||||
export function isInactiveStatus(status: string): boolean {
|
||||
return isClosedStatus(status) || isDeferredStatus(status);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -24,6 +24,7 @@ import {
|
|||
saveDecisionToDb,
|
||||
updateRequirementInDb,
|
||||
saveArtifactToDb,
|
||||
extractDeferredSliceRef,
|
||||
} from '../db-writer.ts';
|
||||
import type { Decision, Requirement } from '../types.ts';
|
||||
|
||||
|
|
@ -694,4 +695,72 @@ describe('db-writer', () => {
|
|||
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
// extractDeferredSliceRef
|
||||
// ═══════════════════════════════════════════════════════════════════════════
|
||||
|
||||
describe('extractDeferredSliceRef', () => {
|
||||
const fields = (scope: string, choice: string, decision: string) => ({
|
||||
scope,
|
||||
choice,
|
||||
decision,
|
||||
});
|
||||
|
||||
test('detects deferral in scope with M###/S## pattern in choice', () => {
|
||||
const result = extractDeferredSliceRef(
|
||||
fields('deferral of low-priority work', 'Move M001/S03 to backlog', ''),
|
||||
);
|
||||
assert.deepStrictEqual(result, { milestoneId: 'M001', sliceId: 'S03' });
|
||||
});
|
||||
|
||||
test('detects deferral in choice field', () => {
|
||||
const result = extractDeferredSliceRef(
|
||||
fields('slice prioritization', 'defer M002/S01 until next sprint', ''),
|
||||
);
|
||||
assert.deepStrictEqual(result, { milestoneId: 'M002', sliceId: 'S01' });
|
||||
});
|
||||
|
||||
test('detects deferral in decision field', () => {
|
||||
const result = extractDeferredSliceRef(
|
||||
fields('resource constraints', '', 'deferred M010/S12 pending review'),
|
||||
);
|
||||
assert.deepStrictEqual(result, { milestoneId: 'M010', sliceId: 'S12' });
|
||||
});
|
||||
|
||||
test('returns null when no M###/S## pattern is present', () => {
|
||||
const result = extractDeferredSliceRef(
|
||||
fields('deferral of work', 'will revisit later', 'deferred indefinitely'),
|
||||
);
|
||||
assert.strictEqual(result, null);
|
||||
});
|
||||
|
||||
test('recognises "deferring" variant', () => {
|
||||
const result = extractDeferredSliceRef(
|
||||
fields('deferring this slice', 'M005/S02 can wait', ''),
|
||||
);
|
||||
assert.deepStrictEqual(result, { milestoneId: 'M005', sliceId: 'S02' });
|
||||
});
|
||||
|
||||
test('recognises "defers" variant', () => {
|
||||
const result = extractDeferredSliceRef(
|
||||
fields('team defers slice', 'M100/S10 not urgent', ''),
|
||||
);
|
||||
assert.deepStrictEqual(result, { milestoneId: 'M100', sliceId: 'S10' });
|
||||
});
|
||||
|
||||
test('returns first M###/S## match when multiple patterns exist', () => {
|
||||
const result = extractDeferredSliceRef(
|
||||
fields('', 'defer M003/S01 and M003/S02', ''),
|
||||
);
|
||||
assert.deepStrictEqual(result, { milestoneId: 'M003', sliceId: 'S01' });
|
||||
});
|
||||
|
||||
test('returns null when no deferral keyword is present', () => {
|
||||
const result = extractDeferredSliceRef(
|
||||
fields('approved work', 'M001/S01 is ready', 'proceed with M001/S01'),
|
||||
);
|
||||
assert.strictEqual(result, null);
|
||||
});
|
||||
});
|
||||
|
||||
});
|
||||
|
|
|
|||
|
|
@ -0,0 +1,203 @@
|
|||
/**
|
||||
* Regression test for #2661: Auto-mode dispatches deferred slices.
|
||||
*
|
||||
* When a decision defers a slice, the dispatcher must skip it and advance
|
||||
* to the next eligible slice. This tests both:
|
||||
* 1. deriveStateFromDb skips slices with status "deferred"
|
||||
* 2. saveDecisionToDb updates the slice status when the decision is a deferral
|
||||
*/
|
||||
|
||||
import { describe, test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
|
||||
import { deriveStateFromDb, invalidateStateCache } from "../state.ts";
|
||||
import {
|
||||
openDatabase,
|
||||
closeDatabase,
|
||||
isDbAvailable,
|
||||
insertMilestone,
|
||||
insertSlice,
|
||||
insertTask,
|
||||
insertArtifact,
|
||||
updateSliceStatus,
|
||||
} from "../gsd-db.ts";
|
||||
import { isDeferredStatus } from "../status-guards.ts";
|
||||
|
||||
// ─── Helpers ──────────────────────────────────────────────────────────────
|
||||
|
||||
function createFixtureBase(): string {
|
||||
const base = mkdtempSync(join(tmpdir(), "gsd-deferred-dispatch-"));
|
||||
mkdirSync(join(base, ".gsd", "milestones"), { recursive: true });
|
||||
return base;
|
||||
}
|
||||
|
||||
function writeFile(base: string, relativePath: string, content: string): void {
|
||||
const full = join(base, ".gsd", relativePath);
|
||||
mkdirSync(join(full, ".."), { recursive: true });
|
||||
writeFileSync(full, content);
|
||||
}
|
||||
|
||||
function cleanup(base: string): void {
|
||||
rmSync(base, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
// ─── Tests ────────────────────────────────────────────────────────────────
|
||||
|
||||
describe("deferred-slice-dispatch (#2661)", () => {
|
||||
test("isDeferredStatus returns true for 'deferred'", () => {
|
||||
assert.ok(isDeferredStatus("deferred"), "should recognize 'deferred'");
|
||||
assert.ok(!isDeferredStatus("active"), "should not match 'active'");
|
||||
assert.ok(!isDeferredStatus("complete"), "should not match 'complete'");
|
||||
assert.ok(!isDeferredStatus("pending"), "should not match 'pending'");
|
||||
});
|
||||
|
||||
test("deriveStateFromDb skips deferred slice and picks next eligible", async () => {
|
||||
const base = createFixtureBase();
|
||||
try {
|
||||
openDatabase(":memory:");
|
||||
assert.ok(isDbAvailable());
|
||||
|
||||
// M001 with three slices: S01 complete, S02 deferred, S03 pending
|
||||
insertMilestone({ id: "M001", title: "Test Milestone", status: "active" });
|
||||
|
||||
insertSlice({ id: "S01", milestoneId: "M001", title: "Done Slice", status: "complete", risk: "low", depends: [] });
|
||||
insertSlice({ id: "S02", milestoneId: "M001", title: "Deferred Slice", status: "deferred", risk: "low", depends: [] });
|
||||
insertSlice({ id: "S03", milestoneId: "M001", title: "Next Slice", status: "pending", risk: "low", depends: [] });
|
||||
|
||||
// S01 needs a SUMMARY file to count as complete for milestone-level checks
|
||||
writeFile(base, "milestones/M001/M001-ROADMAP.md", `# M001: Test Milestone
|
||||
|
||||
**Vision:** Test deferred slices.
|
||||
|
||||
## Slices
|
||||
|
||||
- [x] **S01: Done Slice** \`risk:low\` \`depends:[]\`
|
||||
> Done.
|
||||
|
||||
- [ ] **S02: Deferred Slice** \`risk:low\` \`depends:[]\`
|
||||
> Deferred.
|
||||
|
||||
- [ ] **S03: Next Slice** \`risk:low\` \`depends:[]\`
|
||||
> Next.
|
||||
`);
|
||||
writeFile(base, "milestones/M001/slices/S01/S01-SUMMARY.md", "# S01 Summary\nDone.");
|
||||
|
||||
invalidateStateCache();
|
||||
const state = await deriveStateFromDb(base);
|
||||
|
||||
// The active slice must be S03, NOT S02 (which is deferred)
|
||||
assert.equal(state.activeMilestone?.id, "M001", "active milestone is M001");
|
||||
assert.equal(state.activeSlice?.id, "S03", "active slice should skip deferred S02 and land on S03");
|
||||
assert.notEqual(state.activeSlice?.id, "S02", "active slice must NOT be the deferred S02");
|
||||
|
||||
closeDatabase();
|
||||
} finally {
|
||||
closeDatabase();
|
||||
cleanup(base);
|
||||
}
|
||||
});
|
||||
|
||||
test("deriveStateFromDb does not count deferred slices as done for progress", async () => {
|
||||
const base = createFixtureBase();
|
||||
try {
|
||||
openDatabase(":memory:");
|
||||
|
||||
insertMilestone({ id: "M001", title: "Test", status: "active" });
|
||||
insertSlice({ id: "S01", milestoneId: "M001", title: "Complete", status: "complete", risk: "low", depends: [] });
|
||||
insertSlice({ id: "S02", milestoneId: "M001", title: "Deferred", status: "deferred", risk: "low", depends: [] });
|
||||
insertSlice({ id: "S03", milestoneId: "M001", title: "Pending", status: "pending", risk: "low", depends: [] });
|
||||
|
||||
writeFile(base, "milestones/M001/M001-ROADMAP.md", `# M001
|
||||
## Slices
|
||||
- [x] **S01: Complete** \`risk:low\` \`depends:[]\`
|
||||
- [ ] **S02: Deferred** \`risk:low\` \`depends:[]\`
|
||||
- [ ] **S03: Pending** \`risk:low\` \`depends:[]\`
|
||||
`);
|
||||
writeFile(base, "milestones/M001/slices/S01/S01-SUMMARY.md", "# Done");
|
||||
|
||||
invalidateStateCache();
|
||||
const state = await deriveStateFromDb(base);
|
||||
|
||||
// Deferred slices should not count as "done" in progress
|
||||
// Only S01 (complete) counts as done
|
||||
assert.equal(state.progress?.slices?.done, 1, "only 1 slice (S01) should be done");
|
||||
// Total should still be 3 (deferred slices are still part of the milestone)
|
||||
assert.equal(state.progress?.slices?.total, 3, "all 3 slices counted in total");
|
||||
|
||||
closeDatabase();
|
||||
} finally {
|
||||
closeDatabase();
|
||||
cleanup(base);
|
||||
}
|
||||
});
|
||||
|
||||
test("all slices deferred results in blocked state", async () => {
|
||||
const base = createFixtureBase();
|
||||
try {
|
||||
openDatabase(":memory:");
|
||||
|
||||
insertMilestone({ id: "M001", title: "Test", status: "active" });
|
||||
insertSlice({ id: "S01", milestoneId: "M001", title: "Deferred A", status: "deferred", risk: "low", depends: [] });
|
||||
insertSlice({ id: "S02", milestoneId: "M001", title: "Deferred B", status: "deferred", risk: "low", depends: [] });
|
||||
|
||||
writeFile(base, "milestones/M001/M001-ROADMAP.md", `# M001
|
||||
## Slices
|
||||
- [ ] **S01: Deferred A** \`risk:low\` \`depends:[]\`
|
||||
- [ ] **S02: Deferred B** \`risk:low\` \`depends:[]\`
|
||||
`);
|
||||
|
||||
invalidateStateCache();
|
||||
const state = await deriveStateFromDb(base);
|
||||
|
||||
// No eligible slice — should be blocked
|
||||
assert.equal(state.activeSlice, null, "no active slice when all deferred");
|
||||
assert.equal(state.phase, "blocked", "phase should be blocked when all slices deferred");
|
||||
|
||||
closeDatabase();
|
||||
} finally {
|
||||
closeDatabase();
|
||||
cleanup(base);
|
||||
}
|
||||
});
|
||||
|
||||
test("saveDecisionToDb marks slice as deferred when decision is a deferral", async () => {
|
||||
const base = createFixtureBase();
|
||||
try {
|
||||
openDatabase(":memory:");
|
||||
|
||||
insertMilestone({ id: "M001", title: "Test", status: "active" });
|
||||
insertSlice({ id: "S03", milestoneId: "M001", title: "Target Slice", status: "active", risk: "low", depends: [] });
|
||||
|
||||
writeFile(base, "milestones/M001/M001-ROADMAP.md", `# M001
|
||||
## Slices
|
||||
- [ ] **S03: Target Slice** \`risk:low\` \`depends:[]\`
|
||||
`);
|
||||
|
||||
const { saveDecisionToDb } = await import("../db-writer.ts");
|
||||
const { getSlice } = await import("../gsd-db.ts");
|
||||
|
||||
// Save a deferral decision that references M001/S03
|
||||
await saveDecisionToDb(
|
||||
{
|
||||
scope: "deferral",
|
||||
decision: "Defer S03 to focus on higher priority work",
|
||||
choice: "defer M001/S03",
|
||||
rationale: "Not ready yet",
|
||||
},
|
||||
base,
|
||||
);
|
||||
|
||||
// The slice status should now be "deferred"
|
||||
const slice = getSlice("M001", "S03");
|
||||
assert.equal(slice?.status, "deferred", "slice status should be updated to 'deferred' after deferral decision");
|
||||
|
||||
closeDatabase();
|
||||
} finally {
|
||||
closeDatabase();
|
||||
cleanup(base);
|
||||
}
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue