feat(FN-4851): merge fusion/fn-4851
This commit is contained in:
gsxdsm
5
.changeset/fn-4851-task-done-guards.md
Normal file
5
.changeset/fn-4851-task-done-guards.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
Guard `fn_task_done` against agent-dissent summaries, bulk auto-marking of unreviewed pending steps, and pending REVISE verdicts. These refusals share the existing requeue budget and escalate tasks to in-review when retries are exhausted.
|
||||
@@ -543,6 +543,7 @@ When debugging agent execution issues (agents stuck on "starting"), check these
|
||||
13. **`[wake-trigger-diagnostics] agent=<id> run=<id> triggerDetail=<wake-on-message*> source=<source> messageId=<id|none> from=<type:id|none> forced=<bool> createdAt=<iso|none> inboxUnreadCount=<n> wakeMessageStillUnread=<true|false|unknown> pendingRoomMessages=<n>`** — Correlates wake-on-message triggers with inbox snapshot state to diagnose empty-inbox false-positive wakes
|
||||
14. **`[retry-burned] retry-burned { taskId, agentId, role, category, attempt, total, breakdown }`** — Unified retry-burn telemetry and retry-cap circuit-breaker context
|
||||
15. **`Worktree init command failed (first test run will likely fail): ...` task log entries** — FN-4834: when `worktreeInitCommand` fails, diagnostic stderr is now written to the entry `outcome` (stdout/spawnError fallback), so dashboard logs preserve init failure details without rerunning.
|
||||
16. **`[executor] FN-XXX: fn_task_done refused (<class>) — <reason>`** — FN-4851 refusal diagnostics for `summary-claims-incomplete`, `bulk-step-completion-without-review`, and `pending-code-review-revise`; all three consume the shared `taskDoneRetryCount` budget and escalate to `in-review` once retries are exhausted.
|
||||
|
||||
### Semaphore Resilience
|
||||
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { evaluateTaskDoneRefusal } from "../executor.js";
|
||||
|
||||
function createTask(stepStatuses: Array<"done" | "skipped" | "pending" | "in-progress">) {
|
||||
return {
|
||||
id: "FN-4851",
|
||||
title: "Bulk guard",
|
||||
description: "",
|
||||
column: "in-progress",
|
||||
dependencies: [],
|
||||
steps: stepStatuses.map((status, index) => ({ name: `Step ${index + 1}`, status })),
|
||||
currentStep: 0,
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
} as any;
|
||||
}
|
||||
|
||||
describe("FN-4851 bulk-step-completion guard", () => {
|
||||
it("refuses when 2+ steps would be auto-completed without full APPROVE evidence", () => {
|
||||
const task = createTask(["done", "done", "pending", "pending", "pending", "pending", "pending"]);
|
||||
const result = evaluateTaskDoneRefusal(task, { summary: "Implemented all requested work and verified." }, new Map());
|
||||
|
||||
expect(result.ok).toBe(false);
|
||||
if (!result.ok) {
|
||||
expect(result.refusalClass).toBe("bulk-step-completion-without-review");
|
||||
}
|
||||
});
|
||||
|
||||
it("allows a single pending step without review evidence", () => {
|
||||
const task = createTask(["done", "done", "done", "done", "done", "done", "pending"]);
|
||||
const result = evaluateTaskDoneRefusal(task, { summary: "All tasks complete." }, new Map());
|
||||
|
||||
expect(result).toEqual({ ok: true });
|
||||
});
|
||||
|
||||
it("allows bulk completion when all pending steps are APPROVE", () => {
|
||||
const task = createTask(["done", "done", "done", "done", "pending", "pending", "pending"]);
|
||||
const verdicts = new Map<number, "APPROVE">([
|
||||
[4, "APPROVE"],
|
||||
[5, "APPROVE"],
|
||||
[6, "APPROVE"],
|
||||
]);
|
||||
|
||||
const result = evaluateTaskDoneRefusal(task, { summary: "All tasks complete." }, verdicts as any);
|
||||
|
||||
expect(result).toEqual({ ok: true });
|
||||
});
|
||||
|
||||
it("refuses when even one pending step lacks APPROVE", () => {
|
||||
const task = createTask(["done", "done", "done", "done", "done", "pending", "pending"]);
|
||||
const verdicts = new Map<number, "APPROVE">([[5, "APPROVE"]]);
|
||||
|
||||
const result = evaluateTaskDoneRefusal(task, { summary: "All tasks complete." }, verdicts as any);
|
||||
|
||||
expect(result.ok).toBe(false);
|
||||
if (!result.ok) {
|
||||
expect(result.refusalClass).toBe("bulk-step-completion-without-review");
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,104 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import "./executor-test-helpers.js";
|
||||
import { TaskExecutor } from "../executor.js";
|
||||
import * as worktreePool from "../worktree-pool.js";
|
||||
import { createMockStore, mockedCreateFnAgent, mockedExecSync, resetExecutorMocks } from "./executor-test-helpers.js";
|
||||
|
||||
function createTask(overrides: Record<string, unknown> = {}) {
|
||||
return {
|
||||
id: "FN-4851",
|
||||
title: "Task done refusal test",
|
||||
description: "",
|
||||
column: "in-progress",
|
||||
worktree: "/repo/.worktrees/swift-falcon",
|
||||
branch: "fusion/fn-4851",
|
||||
baseCommitSha: "abc123",
|
||||
taskDoneRetryCount: 0,
|
||||
dependencies: [],
|
||||
steps: [{ name: "Implementation", status: "in-progress" as const }],
|
||||
currentStep: 0,
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
async function setup(overrides: Record<string, unknown> = {}) {
|
||||
const store = createMockStore();
|
||||
let task: any = createTask(overrides);
|
||||
let doneTool: any;
|
||||
|
||||
store.getTask.mockImplementation(async () => ({ ...task, steps: task.steps.map((s: any) => ({ ...s })) }));
|
||||
store.moveTask.mockImplementation(async (_id: string, column: string) => {
|
||||
task = { ...task, column };
|
||||
});
|
||||
|
||||
mockedCreateFnAgent.mockImplementation(async ({ customTools }: any) => {
|
||||
doneTool = customTools.find((tool: any) => tool.name === "fn_task_done");
|
||||
return { session: { prompt: vi.fn().mockResolvedValue(undefined), dispose: vi.fn() } } as any;
|
||||
});
|
||||
|
||||
const executor = new TaskExecutor(store as any, "/repo");
|
||||
await executor.execute(createTask() as any);
|
||||
|
||||
return { store, doneTool };
|
||||
}
|
||||
|
||||
describe("FN-4851 dissent guard", () => {
|
||||
beforeEach(() => {
|
||||
resetExecutorMocks();
|
||||
vi.spyOn(worktreePool, "isUsableTaskWorktree").mockResolvedValue(true);
|
||||
mockedExecSync.mockImplementation((cmd: string) => {
|
||||
if (cmd.includes("rev-parse --show-toplevel")) return Buffer.from("/repo/.worktrees/swift-falcon\n");
|
||||
if (cmd.includes("rev-parse --abbrev-ref HEAD")) return Buffer.from("fusion/fn-4851\n");
|
||||
if (cmd.includes("rev-list --count")) return Buffer.from("1\n");
|
||||
if (cmd.includes("rev-parse HEAD")) return Buffer.from("def456\n");
|
||||
return Buffer.from("");
|
||||
});
|
||||
});
|
||||
|
||||
it("refuses summary that directly claims incompletion", async () => {
|
||||
const { store, doneTool } = await setup();
|
||||
|
||||
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 }));
|
||||
});
|
||||
|
||||
it("refuses 'To unblock' summary", 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");
|
||||
});
|
||||
|
||||
it("refuses summary that 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");
|
||||
});
|
||||
|
||||
it("allows bare 'incomplete' without first-person/task context", async () => {
|
||||
const { doneTool } = await setup();
|
||||
|
||||
const result = await doneTool.execute("id", { summary: "Fixed incomplete dependency declaration in package.json" });
|
||||
|
||||
expect(result.details.refusalClass).toBeUndefined();
|
||||
expect(result.content[0].text).toContain("Task marked complete");
|
||||
});
|
||||
|
||||
it("does not trigger dissent guard for empty summary", async () => {
|
||||
const { doneTool } = await setup();
|
||||
|
||||
const result = await doneTool.execute("id", {});
|
||||
|
||||
expect(result.details.refusalClass).toBeUndefined();
|
||||
expect(result.content[0].text).toContain("Task marked complete");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,100 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import "./executor-test-helpers.js";
|
||||
import { TaskExecutor } from "../executor.js";
|
||||
import { reviewStep } from "../reviewer.js";
|
||||
import * as worktreePool from "../worktree-pool.js";
|
||||
import { createMockStore, mockedCreateFnAgent, mockedExecSync, resetExecutorMocks } from "./executor-test-helpers.js";
|
||||
|
||||
function createTask(overrides: Record<string, unknown> = {}) {
|
||||
return {
|
||||
id: "FN-4851",
|
||||
title: "REVISE guard",
|
||||
description: "",
|
||||
column: "in-progress",
|
||||
worktree: "/repo/.worktrees/swift-falcon",
|
||||
branch: "fusion/fn-4851",
|
||||
baseCommitSha: "abc123",
|
||||
taskDoneRetryCount: 0,
|
||||
dependencies: [],
|
||||
steps: [{ name: "Step 1", status: "in-progress" as const }],
|
||||
currentStep: 0,
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
async function setup(overrides: Record<string, unknown> = {}) {
|
||||
const store = createMockStore();
|
||||
let task: any = createTask(overrides);
|
||||
let doneTool: any;
|
||||
let reviewTool: any;
|
||||
|
||||
store.getTask.mockImplementation(async () => ({ ...task, steps: task.steps.map((s: any) => ({ ...s })) }));
|
||||
store.moveTask.mockImplementation(async (_id: string, column: string) => {
|
||||
task = { ...task, column };
|
||||
});
|
||||
|
||||
mockedCreateFnAgent.mockImplementation(async ({ customTools }: any) => {
|
||||
doneTool = customTools.find((tool: any) => tool.name === "fn_task_done");
|
||||
reviewTool = customTools.find((tool: any) => tool.name === "fn_review_step");
|
||||
return { session: { prompt: vi.fn().mockResolvedValue(undefined), dispose: vi.fn() } } as any;
|
||||
});
|
||||
|
||||
const executor = new TaskExecutor(store as any, "/repo");
|
||||
await executor.execute(createTask() as any);
|
||||
|
||||
return { store, doneTool, reviewTool };
|
||||
}
|
||||
|
||||
describe("FN-4851 REVISE verdict task-done guard", () => {
|
||||
beforeEach(() => {
|
||||
resetExecutorMocks();
|
||||
vi.spyOn(worktreePool, "isUsableTaskWorktree").mockResolvedValue(true);
|
||||
mockedExecSync.mockImplementation((cmd: string) => {
|
||||
if (cmd.includes("rev-parse --show-toplevel")) return Buffer.from("/repo/.worktrees/swift-falcon\n");
|
||||
if (cmd.includes("rev-parse --abbrev-ref HEAD")) return Buffer.from("fusion/fn-4851\n");
|
||||
if (cmd.includes("rev-list --count")) return Buffer.from("1\n");
|
||||
if (cmd.includes("rev-parse HEAD")) return Buffer.from("def456\n");
|
||||
return Buffer.from("");
|
||||
});
|
||||
vi.mocked(reviewStep).mockResolvedValue({
|
||||
verdict: "REVISE",
|
||||
summary: "Needs fixes",
|
||||
review: "Please fix issues",
|
||||
} as any);
|
||||
});
|
||||
|
||||
it("refuses fn_task_done when a pending step has REVISE verdict", async () => {
|
||||
const { store, reviewTool, doneTool } = await setup();
|
||||
|
||||
await reviewTool.execute("rev", { step: 0, type: "code", step_name: "Step 1", baseline: "abc123" });
|
||||
const result = await doneTool.execute("done", { summary: "Implemented all requested changes." });
|
||||
|
||||
expect(result.details.refusalClass).toBe("pending-code-review-revise");
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-4851", "todo", { preserveProgress: true });
|
||||
});
|
||||
|
||||
it("escalates to in-review when retry budget is exhausted", async () => {
|
||||
const { store, reviewTool, doneTool } = await setup({ taskDoneRetryCount: 3 });
|
||||
|
||||
await reviewTool.execute("rev", { step: 0, type: "code", step_name: "Step 1", baseline: "abc123" });
|
||||
const result = await doneTool.execute("done", { summary: "Implemented all requested changes." });
|
||||
|
||||
expect(result.details.refusalClass).toBe("pending-code-review-revise");
|
||||
expect(result.details.error).toContain("pending-code-review-revise");
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-4851", "in-review");
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-4851", expect.objectContaining({ status: "failed" }));
|
||||
});
|
||||
|
||||
it("ignores REVISE verdict on already done or skipped steps", async () => {
|
||||
const { doneTool } = await setup({
|
||||
steps: [{ name: "Step 1", status: "done" }, { name: "Step 2", status: "skipped" }],
|
||||
});
|
||||
|
||||
const result = await doneTool.execute("done", { summary: "Implemented all requested changes." });
|
||||
|
||||
expect(result.details.refusalClass).toBeUndefined();
|
||||
expect(result.content[0].text).toContain("Task marked complete");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,116 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import "../executor-test-helpers.js";
|
||||
import { TaskExecutor } from "../../executor.js";
|
||||
import { reviewStep } from "../../reviewer.js";
|
||||
import * as worktreePool from "../../worktree-pool.js";
|
||||
import { createMockStore, mockedCreateFnAgent, mockedExecSync, resetExecutorMocks } from "../executor-test-helpers.js";
|
||||
|
||||
function createTask(overrides: Record<string, unknown> = {}) {
|
||||
return {
|
||||
id: "FN-4851",
|
||||
title: "Reliability ordering",
|
||||
description: "",
|
||||
column: "in-progress",
|
||||
worktree: "/repo/.worktrees/swift-falcon",
|
||||
branch: "fusion/fn-4851",
|
||||
baseCommitSha: "abc123",
|
||||
taskDoneRetryCount: 0,
|
||||
dependencies: [],
|
||||
steps: [{ name: "Step 1", status: "in-progress" as const }, { name: "Step 2", status: "pending" as const }],
|
||||
currentStep: 0,
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
async function setup(overrides: Record<string, unknown> = {}) {
|
||||
const store = createMockStore();
|
||||
let task: any = createTask(overrides);
|
||||
let doneTool: any;
|
||||
let reviewTool: any;
|
||||
|
||||
store.getTask.mockImplementation(async () => ({ ...task, steps: task.steps.map((s: any) => ({ ...s })) }));
|
||||
store.updateTask.mockImplementation(async (_id: string, patch: any) => {
|
||||
task = { ...task, ...patch };
|
||||
return { ...task, steps: task.steps.map((s: any) => ({ ...s })) };
|
||||
});
|
||||
store.moveTask.mockImplementation(async (_id: string, column: string) => {
|
||||
task = { ...task, column };
|
||||
});
|
||||
|
||||
mockedCreateFnAgent.mockImplementation(async ({ customTools }: any) => {
|
||||
doneTool = customTools.find((tool: any) => tool.name === "fn_task_done");
|
||||
reviewTool = customTools.find((tool: any) => tool.name === "fn_review_step");
|
||||
return { session: { prompt: vi.fn().mockResolvedValue(undefined), dispose: vi.fn() } } as any;
|
||||
});
|
||||
|
||||
const executor = new TaskExecutor(store as any, "/repo");
|
||||
await executor.execute(createTask() as any);
|
||||
|
||||
return { store, doneTool, reviewTool, getTask: () => task };
|
||||
}
|
||||
|
||||
describe("FN-4851 reliability interactions: task-done refusals x invariant", () => {
|
||||
beforeEach(() => {
|
||||
resetExecutorMocks();
|
||||
vi.spyOn(worktreePool, "isUsableTaskWorktree").mockResolvedValue(true);
|
||||
mockedExecSync.mockImplementation((cmd: string) => {
|
||||
if (cmd.includes("rev-parse --show-toplevel")) return Buffer.from("/repo/.worktrees/swift-falcon\n");
|
||||
if (cmd.includes("rev-parse --abbrev-ref HEAD")) return Buffer.from("fusion/fn-4851\n");
|
||||
if (cmd.includes("rev-list --count")) return Buffer.from("1\n");
|
||||
if (cmd.includes("rev-parse HEAD")) return Buffer.from("def456\n");
|
||||
return Buffer.from("");
|
||||
});
|
||||
vi.mocked(reviewStep).mockResolvedValue({ verdict: "REVISE", summary: "needs work", review: "fix" } as any);
|
||||
});
|
||||
|
||||
it("lets invariant refusal win over summary dissent", async () => {
|
||||
const invariantSpy = vi.spyOn(TaskExecutor.prototype as any, "verifyWorktreeInvariants").mockResolvedValue({
|
||||
ok: false,
|
||||
reason: "wrong_branch",
|
||||
observed: "main",
|
||||
expected: "fusion/fn-4851",
|
||||
});
|
||||
const { doneTool } = await setup();
|
||||
|
||||
const result = await doneTool.execute("done", { summary: "Task is not complete and I am blocked." });
|
||||
|
||||
expect(result.content[0].text).toContain("fn_task_done refused: wrong_branch");
|
||||
expect(result.details.refusalClass).toBeUndefined();
|
||||
invariantSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("runs dissent refusal before scope-leak guard when invariants pass", 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();
|
||||
scopeSpy.mockRestore();
|
||||
invariantSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("shares one retry budget across mixed refusal classes", async () => {
|
||||
const { doneTool, reviewTool, store, getTask } = await setup({ steps: [{ name: "S1", status: "in-progress" }, { name: "S2", status: "pending" }] });
|
||||
|
||||
await doneTool.execute("1", { summary: "Task is not complete." });
|
||||
expect(getTask().taskDoneRetryCount).toBe(1);
|
||||
|
||||
await doneTool.execute("2", { summary: "Completed implementation and tests." });
|
||||
expect(getTask().taskDoneRetryCount).toBe(2);
|
||||
|
||||
getTask().steps = [{ name: "S1", status: "in-progress" }];
|
||||
await reviewTool.execute("rev", { step: 0, type: "code", step_name: "S1", baseline: "abc" });
|
||||
const third = await doneTool.execute("3", { summary: "Completed implementation and tests." });
|
||||
expect(third.details.refusalClass).toBe("pending-code-review-revise");
|
||||
expect(getTask().taskDoneRetryCount).toBe(3);
|
||||
|
||||
const fourth = await doneTool.execute("4", { summary: "Task is not complete." });
|
||||
expect(fourth.details.refusalClass).toBe("pending-code-review-revise");
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-4851", "in-review");
|
||||
});
|
||||
});
|
||||
@@ -171,6 +171,108 @@ const COMPLETED_TASK_WATCHDOG_MS = 60_000;
|
||||
/** How long to wait before retrying a workflow rerun handoff that never reached in-progress. */
|
||||
const WORKFLOW_RERUN_WATCHDOG_MS = 15_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";
|
||||
|
||||
type TaskDoneRefusalResult =
|
||||
| { ok: true }
|
||||
| {
|
||||
ok: false;
|
||||
refusalClass: TaskDoneRefusalClass;
|
||||
message: string;
|
||||
reason: string;
|
||||
};
|
||||
|
||||
function formatTaskDoneRefusal(refusalClass: TaskDoneRefusalClass, reason: string): string {
|
||||
return `fn_task_done refused (${refusalClass}): ${reason}. ${TASK_DONE_REFUSAL_SUFFIX}`;
|
||||
}
|
||||
|
||||
export function evaluateTaskDoneRefusal(
|
||||
task: Task,
|
||||
params: { summary?: string },
|
||||
codeReviewVerdicts: Map<number, ReviewVerdict>,
|
||||
): TaskDoneRefusalResult {
|
||||
const pendingSteps: number[] = [];
|
||||
for (let stepIndex = 0; stepIndex < task.steps.length; stepIndex++) {
|
||||
const step = task.steps[stepIndex];
|
||||
if (!step || step.status === "done" || step.status === "skipped") {
|
||||
continue;
|
||||
}
|
||||
pendingSteps.push(stepIndex);
|
||||
if (codeReviewVerdicts.get(stepIndex) === "REVISE") {
|
||||
const reason = `Step ${stepIndex + 1} (${step.name}) has a pending code review verdict of REVISE`;
|
||||
return {
|
||||
ok: false,
|
||||
refusalClass: "pending-code-review-revise",
|
||||
reason,
|
||||
message: formatTaskDoneRefusal("pending-code-review-revise", reason),
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
const summary = params.summary?.trim();
|
||||
if (summary) {
|
||||
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) {
|
||||
const reason = `attempted to auto-complete ${pendingSteps.length} pending steps without APPROVE verdicts on all of them`;
|
||||
return {
|
||||
ok: false,
|
||||
refusalClass: "bulk-step-completion-without-review",
|
||||
reason,
|
||||
message: formatTaskDoneRefusal("bulk-step-completion-without-review", reason),
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
return { ok: true };
|
||||
}
|
||||
|
||||
/**
|
||||
* Determines the step index from which revision should restart given a set of
|
||||
* completed steps and user feedback. Exported for unit tests; no longer called
|
||||
@@ -3424,7 +3526,7 @@ export class TaskExecutor {
|
||||
this.createTaskLogTool(task.id),
|
||||
this.createTaskCreateTool(),
|
||||
this.createTaskAddDepTool(task.id),
|
||||
this.createTaskDoneTool(task.id, worktreePath, detail.prompt ?? "", () => { taskDone = true; }),
|
||||
this.createTaskDoneTool(task.id, worktreePath, detail.prompt ?? "", codeReviewVerdicts, () => { taskDone = true; }),
|
||||
createRunVerificationTool({
|
||||
worktreePath,
|
||||
rootDir: this.rootDir,
|
||||
@@ -5256,7 +5358,13 @@ export class TaskExecutor {
|
||||
return { blocked: false };
|
||||
}
|
||||
|
||||
private createTaskDoneTool(taskId: string, worktreePath: string, promptContent: string, onDone: () => void): ToolDefinition {
|
||||
private createTaskDoneTool(
|
||||
taskId: string,
|
||||
worktreePath: string,
|
||||
promptContent: string,
|
||||
codeReviewVerdicts: Map<number, ReviewVerdict>,
|
||||
onDone: () => void,
|
||||
): ToolDefinition {
|
||||
const store = this.store;
|
||||
return {
|
||||
name: "fn_task_done",
|
||||
@@ -5335,6 +5443,58 @@ export class TaskExecutor {
|
||||
};
|
||||
}
|
||||
|
||||
const taskDoneRefusal = evaluateTaskDoneRefusal(task, params, codeReviewVerdicts);
|
||||
if (!taskDoneRefusal.ok) {
|
||||
const refusalMessage = taskDoneRefusal.message;
|
||||
await store.logEntry(taskId, refusalMessage, undefined, this.currentRunContext);
|
||||
executorLog.error(`${taskId}: fn_task_done refused (${taskDoneRefusal.refusalClass}) — ${taskDoneRefusal.reason}`);
|
||||
|
||||
const priorRequeues = task.taskDoneRetryCount ?? 0;
|
||||
const nextRequeueCount = priorRequeues + 1;
|
||||
if (priorRequeues < MAX_TASK_DONE_REQUEUE_RETRIES) {
|
||||
await store.updateTask(taskId, {
|
||||
status: "failed",
|
||||
error: refusalMessage,
|
||||
taskDoneRetryCount: nextRequeueCount,
|
||||
paused: false,
|
||||
pausedByAgentId: null,
|
||||
worktree: null,
|
||||
branch: null,
|
||||
sessionFile: null,
|
||||
});
|
||||
await store.logEntry(
|
||||
taskId,
|
||||
`${refusalMessage} — requeued to todo immediately (${nextRequeueCount}/${MAX_TASK_DONE_REQUEUE_RETRIES})`,
|
||||
undefined,
|
||||
this.currentRunContext,
|
||||
);
|
||||
await store.moveTask(taskId, "todo", { preserveProgress: true });
|
||||
executorLog.log(`✗ ${taskId} fn_task_done refusal (${taskDoneRefusal.refusalClass}) — requeued to todo (${nextRequeueCount}/${MAX_TASK_DONE_REQUEUE_RETRIES})`);
|
||||
} else {
|
||||
await store.updateTask(taskId, {
|
||||
status: "failed",
|
||||
error: refusalMessage,
|
||||
paused: false,
|
||||
pausedByAgentId: null,
|
||||
worktree: null,
|
||||
branch: null,
|
||||
sessionFile: null,
|
||||
});
|
||||
await store.logEntry(taskId, `${refusalMessage} — moved to in-review for inspection`, undefined, this.currentRunContext);
|
||||
await this.persistTokenUsage(taskId);
|
||||
await store.moveTask(taskId, "in-review");
|
||||
executorLog.log(`✗ ${taskId} fn_task_done refusal (${taskDoneRefusal.refusalClass}) — moved to in-review for inspection`);
|
||||
}
|
||||
|
||||
return {
|
||||
content: [{ type: "text" as const, text: refusalMessage }],
|
||||
details: {
|
||||
error: refusalMessage,
|
||||
refusalClass: taskDoneRefusal.refusalClass,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
const settings = await store.getSettings();
|
||||
const scopeLeakCheck = await this.evaluateTaskDoneScopeLeak(task, worktreePath, promptContent, settings)
|
||||
.catch((error: unknown) => {
|
||||
|
||||
Reference in New Issue
Block a user