fix(engine): trust explicit task completion summaries

Stop inferring incomplete work from summary wording so scoped future-work notes cannot requeue completed tasks. Keep structural review and bulk-step completion guards intact.
This commit is contained in:
gsxdsm
2026-07-19 17:52:45 -07:00
parent 365874f7f9
commit fc24e66f63
6 changed files with 51 additions and 100 deletions

View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Accept task completion regardless of wording in the completion summary.
category: fix
dev: Keeps structural review and bulk-step completion guards while removing prose-based refusals.

View File

@@ -44,7 +44,7 @@ async function setup(overrides: Record<string, unknown> = {}) {
return { store, doneTool };
}
describe("FN-4851 dissent guard", () => {
describe("fn_task_done summary prose", () => {
beforeEach(() => {
resetExecutorMocks();
vi.spyOn(worktreePool, "isUsableTaskWorktree").mockResolvedValue(true);
@@ -57,31 +57,46 @@ describe("FN-4851 dissent guard", () => {
});
});
it("refuses summary that directly claims incompletion", async () => {
it("accepts completion when the summary documents future work as not implemented in this task", async () => {
const { store, doneTool } = await setup();
store.moveTask.mockClear();
const result = await doneTool.execute("id", {
summary: "KB-015 complete: Implemented semantic design tokens and migrated all components. All verification passes. Note: User steering comments requested migration to a shadcn-svelte pattern — this is documented as a future direction in project memory and task docs, not implemented in this task per the DO NOT redo completed steps instruction.",
});
expect(result.details.refusalClass).toBeUndefined();
expect(result.content[0].text).toContain("Task marked complete");
expect(store.moveTask).not.toHaveBeenCalledWith("FN-4851", "todo", { preserveProgress: true });
});
it("accepts a completion call even when its summary claims incompletion", async () => {
const { store, doneTool } = await setup();
store.moveTask.mockClear();
const result = await doneTool.execute("id", { summary: "Task is not complete. I'm blocked from safely finishing this." });
expect(result.details.refusalClass).toBe("summary-claims-incomplete");
expect(result.content[0].text).toContain("fn_task_done refused (summary-claims-incomplete)");
expect(store.moveTask).toHaveBeenCalledWith("FN-4851", "todo", { preserveProgress: true });
expect(store.updateTask).toHaveBeenCalledWith("FN-4851", expect.objectContaining({ taskDoneRetryCount: 1 }));
expect(result.details.refusalClass).toBeUndefined();
expect(result.content[0].text).toContain("Task marked complete");
expect(store.moveTask).not.toHaveBeenCalledWith("FN-4851", "todo", { preserveProgress: true });
});
it("refuses 'To unblock' summary", async () => {
it("accepts a completion call whose summary contains 'To unblock'", async () => {
const { doneTool } = await setup();
const result = await doneTool.execute("id", { summary: "To unblock, sync/land FN-4789 before I can finish." });
expect(result.details.refusalClass).toBe("summary-claims-incomplete");
expect(result.details.refusalClass).toBeUndefined();
expect(result.content[0].text).toContain("Task marked complete");
});
it("refuses summary that says it needs another FN task", async () => {
it("accepts a completion call whose summary says it needs another FN task", async () => {
const { doneTool } = await setup();
const result = await doneTool.execute("id", { summary: "This needs FN-1234 before completion." });
expect(result.details.refusalClass).toBe("summary-claims-incomplete");
expect(result.details.refusalClass).toBeUndefined();
expect(result.content[0].text).toContain("Task marked complete");
});
it("allows bare 'incomplete' without first-person/task context", async () => {

View File

@@ -15,11 +15,8 @@ function createTask(stepStatuses: Array<"done" | "skipped" | "pending" | "in-pro
} as any;
}
describe("preflight PREMISE STALE: escape hatch", () => {
it("allows fn_task_done when summary starts with PREMISE STALE: even if it contains 'done' near 'the task'", () => {
// Without the bypass, the scoped-incomplete regex matches 'done' and the
// 40-char window contains 'the task' → would refuse with
// summary-claims-incomplete. The bypass must let this through.
describe("fn_task_done summary-independent refusal checks", () => {
it("allows fn_task_done when summary starts with PREMISE STALE:", () => {
const task = createTask(["done", "skipped", "skipped", "skipped", "skipped"]);
const result = evaluateTaskDoneRefusal(
task,
@@ -29,10 +26,7 @@ describe("preflight PREMISE STALE: escape hatch", () => {
expect(result).toEqual({ ok: true });
});
it("allows fn_task_done when summary starts with PREMISE STALE: and contains 'I'm blocked' style dissent phrasing", () => {
// Natural premise-stale phrasing may accidentally include a dissent-pattern
// word ("blocked from", "to unblock", "requires follow-up"). The bypass
// must not refuse on the dissent regex when the sentinel is present.
it("allows fn_task_done when summary contains blocked-work phrasing", () => {
const task = createTask(["done", "skipped", "skipped"]);
const result = evaluateTaskDoneRefusal(
task,
@@ -42,7 +36,7 @@ describe("preflight PREMISE STALE: escape hatch", () => {
expect(result).toEqual({ ok: true });
});
it("is case-insensitive on the sentinel", () => {
it("allows a lowercase PREMISE STALE sentinel", () => {
const task = createTask(["done", "skipped"]);
const result = evaluateTaskDoneRefusal(
task,
@@ -52,19 +46,14 @@ describe("preflight PREMISE STALE: escape hatch", () => {
expect(result).toEqual({ ok: true });
});
it("does NOT bypass when sentinel appears later in the summary (must be at start)", () => {
// Defends against agents tacking the sentinel into the middle to dodge a
// genuine incomplete-work refusal.
const task = createTask(["done", "pending"]);
it("does not reject incomplete-work prose when PREMISE STALE appears later", () => {
const task = createTask(["done"]);
const result = evaluateTaskDoneRefusal(
task,
{ summary: "The task is not done yet, but PREMISE STALE: I think it's stale anyway" },
new Map(),
);
expect(result.ok).toBe(false);
if (!result.ok) {
expect(result.refusalClass).toBe("summary-claims-incomplete");
}
expect(result).toEqual({ ok: true });
});
it("still enforces pending-code-review-revise even with the sentinel", () => {

View File

@@ -7,11 +7,9 @@ describe("FN-4946 shared task_done refusal helper invariant", () => {
const source = readFileSync(new URL("../executor.ts", import.meta.url), "utf8");
const invocations = source.match(/evaluateTaskDoneRefusal\(/g) ?? [];
const helperDecl = source.match(/\bfunction evaluateTaskDoneRefusal\b/g) ?? [];
const dissentDecl = source.match(/\bconst DISSENT_PATTERNS\b/g) ?? [];
expect(invocations.length).toBeGreaterThanOrEqual(3);
expect(helperDecl).toHaveLength(1);
expect(dissentDecl).toHaveLength(1);
});
it("returns pending-code-review-revise for a pending step with REVISE and no summary", () => {

View File

@@ -79,20 +79,20 @@ describe("FN-4851 reliability interactions: task-done refusals x invariant", ()
invariantSpy.mockRestore();
});
it("runs dissent refusal before scope-leak guard when invariants pass", async () => {
it("does not let summary prose prevent the scope-leak guard from running", async () => {
const invariantSpy = vi.spyOn(TaskExecutor.prototype as any, "verifyWorktreeInvariants").mockResolvedValue({ ok: true });
const scopeSpy = vi.spyOn(TaskExecutor.prototype as any, "evaluateTaskDoneScopeLeak");
const { doneTool } = await setup();
const result = await doneTool.execute("done", { summary: "To unblock, land FN-4789 first." });
expect(result.details.refusalClass).toBe("summary-claims-incomplete");
expect(scopeSpy).not.toHaveBeenCalled();
expect(result.details.refusalClass).toBeUndefined();
expect(scopeSpy).toHaveBeenCalled();
scopeSpy.mockRestore();
invariantSpy.mockRestore();
});
it("shares one retry budget across mixed refusal classes", async () => {
it("shares one retry budget across repeated structural refusals", async () => {
const { doneTool, store, getTask } = await setup({ steps: [{ name: "S1", status: "in-progress" }, { name: "S2", status: "pending" }] });
// FNXC:WorkflowLifecycle 2026-07-01-21:20: setup()'s execute() drives a bare mock agent through the
@@ -115,28 +115,22 @@ describe("FN-4851 reliability interactions: task-done refusals x invariant", ()
steps: [{ name: "S1", status: "in-progress" }, { name: "S2", status: "pending" }],
});
await doneTool.execute("1", { summary: "Task is not complete." });
stageIncompleteSteps();
await doneTool.execute("1", { summary: "Completed implementation and tests." });
expect(getTask().taskDoneRetryCount).toBe(1);
stageIncompleteSteps();
await doneTool.execute("2", { summary: "Completed implementation and tests." });
expect(getTask().taskDoneRetryCount).toBe(2);
/*
FNXC:WorkflowReviewGates 2026-07-19-02:45:
U10 (R9): refusals 3 and 4 used to be driven by `fn_review_step` producing a REVISE verdict
(`pending-code-review-revise`). That tool is deleted and the verdict map has no writer, so the
class is unreachable from the executor. The invariant under test is the SHARED budget across
MIXED classes — not which specific classes — so the sequence now mixes bulk-step-completion
with summary-claims-incomplete, keeping three distinct classes on one budget.
*/
stageIncompleteSteps();
const third = await doneTool.execute("3", { summary: "Completed implementation and tests." });
expect(third.details.refusalClass).toBe("bulk-step-completion-without-review");
expect(getTask().taskDoneRetryCount).toBe(3);
const fourth = await doneTool.execute("4", { summary: "Task is not complete." });
expect(fourth.details.refusalClass).toBe("summary-claims-incomplete");
stageIncompleteSteps();
const fourth = await doneTool.execute("4", { summary: "Completed implementation and tests." });
expect(fourth.details.refusalClass).toBe("bulk-step-completion-without-review");
// FNXC:WorkflowLifecycle 2026-07-01-21:20: The shared retry budget is exhausted on the 4th refusal
// (count already 3). Exhaustion is terminal and now parks the task `status: "failed"` in place under
// the workflow-graph failure model (superseding FN-1284's move-to-in-review escalation); the invariant

View File

@@ -477,17 +477,7 @@ const LOOP_COMPACTION_TIMEOUT_MS = 60_000;
const TASK_DONE_REFUSAL_SUFFIX = "Either finish the work and resubmit, or do not call fn_task_done — exit the session and the engine will requeue.";
export const DISSENT_PATTERNS: RegExp[] = [
/\btask (is|was)(?: not|n['’]?t) complete\b/i,
/\b(?:i (?:could|can)(?:not|n['’]?t)|unable to|failed to) (?:complete|finish|implement)\b/i,
/\b(?:partially|not fully) (?:complete|implemented|done|finished)\b/i,
/\b(?:i['’]?m blocked|blocked from|blocking issue prevents)\b/i,
/\bto unblock\b/i,
/\b(?:needs|requires) (?:FN-\d+|further work|additional work|follow[- ]?up)\b/i,
];
type TaskDoneRefusalClass =
| "summary-claims-incomplete"
| "bulk-step-completion-without-review"
| "pending-code-review-revise";
@@ -581,7 +571,7 @@ function formatTaskDoneRefusal(refusalClass: TaskDoneRefusalClass, reason: strin
export function evaluateTaskDoneRefusal(
task: Task,
params: { summary?: string },
_params: { summary?: string },
codeReviewVerdicts: Map<number, ReviewVerdict>,
): TaskDoneRefusalResult {
const pendingSteps: number[] = [];
@@ -602,48 +592,6 @@ export function evaluateTaskDoneRefusal(
}
}
const summary = params.summary?.trim();
// Preflight escape hatch: when the agent's preflight finds PROMPT.md is out
// of sync with HEAD (work already done on the base), it marks remaining
// steps `skipped` and calls fn_task_done with a `PREMISE STALE:` summary.
// Skip the summary-text refusals (dissent + scoped-incomplete) for this
// sentinel so a natural premise-stale explanation like "...the work is
// already done on HEAD" cannot deadlock the executor. The pending-review
// and bulk-step-completion guards above/below still apply.
const isPremiseStale = !!summary && /^premise stale:/i.test(summary);
if (summary && !isPremiseStale) {
const dissentMatch = DISSENT_PATTERNS.find((pattern) => pattern.test(summary));
if (dissentMatch) {
const matchText = summary.match(dissentMatch)?.[0] ?? dissentMatch.source;
const reason = `summary indicates incomplete work (${JSON.stringify(matchText)})`;
return {
ok: false,
refusalClass: "summary-claims-incomplete",
reason,
message: formatTaskDoneRefusal("summary-claims-incomplete", reason),
};
}
const scopedPattern = /\b(incomplete|not implemented|not done|not finished)\b/i;
const scopedMatch = scopedPattern.exec(summary);
if (scopedMatch) {
const start = Math.max(0, scopedMatch.index - 40);
const end = Math.min(summary.length, scopedMatch.index + scopedMatch[0].length + 40);
const scopedWindow = summary.slice(start, end);
const hasFirstPersonContext = /\b(i|i['’]?m|i['’]?ve|my|we)\b/i.test(scopedWindow)
|| /\b(the task|this task)\b/i.test(scopedWindow);
if (hasFirstPersonContext) {
const reason = `summary indicates incomplete work (${JSON.stringify(scopedMatch[0])})`;
return {
ok: false,
refusalClass: "summary-claims-incomplete",
reason,
message: formatTaskDoneRefusal("summary-claims-incomplete", reason),
};
}
}
}
if (pendingSteps.length >= 2) {
const allPendingApproved = pendingSteps.every((stepIndex) => codeReviewVerdicts.get(stepIndex) === "APPROVE");
if (!allPendingApproved) {
@@ -12386,7 +12334,7 @@ export class TaskExecutor {
const implicitCheck = await this.store.getTask(task.id);
if (implicitCheck.steps.length > 0 &&
implicitCheck.steps.every((s) => s.status === "done" || s.status === "skipped")) {
// Implicit path has no summary; evaluateTaskDoneRefusal will skip summary-claims-incomplete and only enforce pending-code-review-revise / bulk-step-completion-without-review.
// Implicit and explicit paths share the same structural pending-review and bulk-step-completion guards.
const refusal = this.evaluateImplicitCompletionRefusal(implicitCheck, codeReviewVerdicts);
if (!refusal.ok) {
await this.handleImplicitTaskDoneRefusal(implicitCheck, refusal);
@@ -12654,7 +12602,7 @@ export class TaskExecutor {
const implicitCheck = await this.store.getTask(task.id);
if (implicitCheck.steps.length > 0 &&
implicitCheck.steps.every((s) => s.status === "done" || s.status === "skipped")) {
// Implicit path has no summary; evaluateTaskDoneRefusal will skip summary-claims-incomplete and only enforce pending-code-review-revise / bulk-step-completion-without-review.
// Implicit and explicit paths share the same structural pending-review and bulk-step-completion guards.
const refusal = this.evaluateImplicitCompletionRefusal(implicitCheck, codeReviewVerdicts);
if (!refusal.ok) {
await this.handleImplicitTaskDoneRefusal(implicitCheck, refusal);