fix: block complete-milestone dispatch when VALIDATION is needs-remediation (#2682)
When VALIDATION.md has verdict needs-remediation and all slices appear done in the DB, the state machine enters completing-milestone. The complete-milestone dispatch rule had no verdict check, so it dispatched the unit — the agent correctly refused (validation failed), no SUMMARY was written, and the unit was re-dispatched up to MAX_LIFETIME_DISPATCHES times before stuck detection fired. The fix adds a verdict check in the completing-milestone dispatch rule that returns action: stop with level: warning when the verdict is needs-remediation. Using warning level ensures the session pauses (resumable) rather than hard-stopping, matching the pattern from #2474. Closes #2675
This commit is contained in:
parent
c6c194b7e9
commit
12713a547c
2 changed files with 129 additions and 0 deletions
|
|
@ -626,6 +626,25 @@ export const DISPATCH_RULES: DispatchRule[] = [
|
|||
match: async ({ state, mid, midTitle, basePath }) => {
|
||||
if (state.phase !== "completing-milestone") return null;
|
||||
|
||||
// Safety guard (#2675): block completion when VALIDATION verdict is
|
||||
// needs-remediation. The state machine treats needs-remediation as
|
||||
// terminal (to prevent validate-milestone loops per #832), but
|
||||
// completing-milestone should NOT proceed — remediation work is needed.
|
||||
const validationFile = resolveMilestoneFile(basePath, mid, "VALIDATION");
|
||||
if (validationFile) {
|
||||
const validationContent = await loadFile(validationFile);
|
||||
if (validationContent) {
|
||||
const verdict = extractVerdict(validationContent);
|
||||
if (verdict === "needs-remediation") {
|
||||
return {
|
||||
action: "stop",
|
||||
reason: `Cannot complete milestone ${mid}: VALIDATION verdict is "needs-remediation". Address the remediation findings and re-run validation, or update the verdict manually.`,
|
||||
level: "warning",
|
||||
};
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Safety guard (#1368): verify all roadmap slices have SUMMARY files.
|
||||
const missingSlices = findMissingSummaries(basePath, mid);
|
||||
if (missingSlices.length > 0) {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,110 @@
|
|||
/**
|
||||
* Regression test for #2675: completing-milestone dispatch rule must
|
||||
* block completion when VALIDATION verdict is "needs-remediation".
|
||||
*
|
||||
* Without this guard, needs-remediation + allSlicesDone causes a loop:
|
||||
* complete-milestone dispatched → agent refuses (correct) → no SUMMARY
|
||||
* → re-dispatch → repeat until stuck detection fires.
|
||||
*/
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
|
||||
import { DISPATCH_RULES } from "../auto-dispatch.ts";
|
||||
|
||||
/** Find the completing-milestone dispatch rule */
|
||||
const completingRule = DISPATCH_RULES.find(r => r.name === "completing-milestone → complete-milestone");
|
||||
|
||||
test("completing-milestone dispatch rule exists", () => {
|
||||
assert.ok(completingRule, "rule should exist in DISPATCH_RULES");
|
||||
});
|
||||
|
||||
test("completing-milestone blocks when VALIDATION verdict is needs-remediation (#2675)", async () => {
|
||||
const base = mkdtempSync(join(tmpdir(), "gsd-remediation-"));
|
||||
mkdirSync(join(base, ".gsd", "milestones", "M001"), { recursive: true });
|
||||
|
||||
try {
|
||||
// Write a VALIDATION file with needs-remediation verdict
|
||||
writeFileSync(
|
||||
join(base, ".gsd", "milestones", "M001", "M001-VALIDATION.md"),
|
||||
[
|
||||
"---",
|
||||
"verdict: needs-remediation",
|
||||
"remediation_round: 0",
|
||||
"---",
|
||||
"",
|
||||
"# Validation Report",
|
||||
"",
|
||||
"3 success criteria failed. Remediation required.",
|
||||
].join("\n"),
|
||||
);
|
||||
|
||||
const ctx = {
|
||||
mid: "M001",
|
||||
midTitle: "Test Milestone",
|
||||
basePath: base,
|
||||
state: { phase: "completing-milestone" } as any,
|
||||
prefs: {} as any,
|
||||
session: undefined,
|
||||
};
|
||||
|
||||
const result = await completingRule!.match(ctx);
|
||||
|
||||
assert.ok(result !== null, "rule should match");
|
||||
assert.equal(result!.action, "stop", "should return stop action");
|
||||
if (result!.action === "stop") {
|
||||
assert.equal(result!.level, "warning", "should be warning level (pausable)");
|
||||
assert.ok(
|
||||
result!.reason.includes("needs-remediation"),
|
||||
"reason should mention needs-remediation",
|
||||
);
|
||||
}
|
||||
} finally {
|
||||
rmSync(base, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("completing-milestone proceeds normally when VALIDATION verdict is pass (#2675 guard)", async () => {
|
||||
const base = mkdtempSync(join(tmpdir(), "gsd-remediation-"));
|
||||
mkdirSync(join(base, ".gsd", "milestones", "M001"), { recursive: true });
|
||||
|
||||
try {
|
||||
// Write a VALIDATION file with pass verdict
|
||||
writeFileSync(
|
||||
join(base, ".gsd", "milestones", "M001", "M001-VALIDATION.md"),
|
||||
[
|
||||
"---",
|
||||
"verdict: pass",
|
||||
"---",
|
||||
"",
|
||||
"# Validation Report",
|
||||
"",
|
||||
"All criteria met.",
|
||||
].join("\n"),
|
||||
);
|
||||
|
||||
const ctx = {
|
||||
mid: "M001",
|
||||
midTitle: "Test Milestone",
|
||||
basePath: base,
|
||||
state: { phase: "completing-milestone" } as any,
|
||||
prefs: {} as any,
|
||||
session: undefined,
|
||||
};
|
||||
|
||||
const result = await completingRule!.match(ctx);
|
||||
|
||||
// Should NOT return a stop — should either dispatch or return stop for
|
||||
// a different reason (e.g. missing SUMMARY files, no implementation)
|
||||
if (result && result.action === "stop") {
|
||||
assert.ok(
|
||||
!result.reason.includes("needs-remediation"),
|
||||
"pass verdict should NOT trigger the remediation guard",
|
||||
);
|
||||
}
|
||||
} finally {
|
||||
rmSync(base, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue