test(FN-4946): expand implicit completion guard scenarios

Fusion-Task-Id: FN-4946
Fusion-Task-Lineage: 9a48f72f-d2cc-4950-8a00-f69053dd1163

packages/engine/src/__tests__/executor-implicit-task-done-budget.test.ts

@@ -2,7 +2,8 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
 import "./executor-test-helpers.js";
 import { TaskExecutor } from "../executor.js";
 import { executorLog } from "../logger.js";
-import { createMockStore, resetExecutorMocks } from "./executor-test-helpers.js";
+import { reviewStep } from "../reviewer.js";
+import { createMockStore, mockedCreateFnAgent, resetExecutorMocks } from "./executor-test-helpers.js";
 
 function refusal() {
   return {
@@ -58,4 +59,66 @@ describe("FN-4946 implicit refusal budget handling", () => {
     expect(store.moveTask).toHaveBeenCalledWith("FN-4946-B", "in-review");
     expect(persistSpy).toHaveBeenCalledWith("FN-4946-B");
   });
+
+  it("shares retry budget with explicit fn_task_done refusals", async () => {
+    const store = createMockStore();
+    let currentTask: any = {
+      ...task(2),
+      id: "FN-4946-B2",
+      steps: [{ name: "Step 1", status: "in-progress" }],
+    };
+    let doneTool: any;
+    let reviewTool: any;
+
+    store.getTask.mockImplementation(async () => ({ ...currentTask, steps: currentTask.steps.map((s: any) => ({ ...s })) }));
+    store.updateTask.mockImplementation(async (_id: string, patch: any) => {
+      currentTask = { ...currentTask, ...patch };
+    });
+    vi.mocked(reviewStep).mockResolvedValue({ verdict: "REVISE", summary: "fix", review: "fix" } as any);
+
+    mockedCreateFnAgent.mockImplementation(async ({ customTools }: any) => {
+      doneTool = customTools.find((t: any) => t.name === "fn_task_done");
+      reviewTool = customTools.find((t: any) => t.name === "fn_review_step");
+      return { session: { prompt: vi.fn().mockResolvedValue(undefined), dispose: vi.fn(), subscribe: vi.fn(), on: vi.fn(), state: {} } } as any;
+    });
+
+    const executor = new TaskExecutor(store as any, "/repo");
+    await executor.execute(currentTask);
+
+    await reviewTool.execute("r", { step: 0, type: "code", step_name: "Step 1", baseline: "abc123" });
+    await doneTool.execute("d", { summary: "done" });
+
+    expect(store.moveTask).toHaveBeenCalledWith("FN-4946-B2", "in-review");
+  });
+
+  it("resets taskDoneRetryCount after later clean completion", async () => {
+    const store = createMockStore();
+    let currentTask: any = { ...task(1), id: "FN-4946-B3", steps: [{ name: "Step 1", status: "in-progress" }] };
+    store.getTask.mockImplementation(async () => ({ ...currentTask, steps: currentTask.steps.map((s: any) => ({ ...s })) }));
+    store.updateTask.mockImplementation(async (_id: string, patch: any) => {
+      currentTask = { ...currentTask, ...patch };
+    });
+
+    mockedCreateFnAgent.mockImplementation(async ({ customTools }: any) => {
+      const doneTool = customTools.find((t: any) => t.name === "fn_task_done");
+      return {
+        session: {
+          prompt: vi.fn().mockImplementation(async () => {
+            await doneTool.execute("done", { summary: "complete" });
+          }),
+          dispose: vi.fn(),
+          subscribe: vi.fn(),
+          on: vi.fn(),
+          state: {},
+        },
+      } as any;
+    });
+
+    const executor = new TaskExecutor(store as any, "/repo");
+    await executor.execute(currentTask);
+
+    expect(store.moveTask).toHaveBeenCalledWith("FN-4946-B3", "in-review");
+    const retryBumpCalls = store.updateTask.mock.calls.filter(([, patch]: [string, Record<string, unknown>]) => typeof patch.taskDoneRetryCount === "number");
+    expect(retryBumpCalls).toHaveLength(0);
+  });
 });

packages/engine/src/__tests__/executor-implicit-task-done-revise-guard.test.ts

@@ -1,6 +1,7 @@
 import { beforeEach, describe, expect, it, vi } from "vitest";
 import "./executor-test-helpers.js";
 import { TaskExecutor } from "../executor.js";
+import { executorLog } from "../logger.js";
 import { reviewStep } from "../reviewer.js";
 import { createMockStore, mockedCreateFnAgent, mockedExecSync, resetExecutorMocks } from "./executor-test-helpers.js";
 
@@ -23,6 +24,15 @@ function makeTask(overrides: Record<string, unknown> = {}) {
   };
 }
 
+function refusal() {
+  return {
+    ok: false as const,
+    refusalClass: "pending-code-review-revise" as const,
+    reason: "Step 1 has pending REVISE",
+    message: "fn_task_done refused (pending-code-review-revise): Step 1 has pending REVISE",
+  };
+}
+
 describe("FN-4946 implicit completion + REVISE verdict interaction", () => {
   beforeEach(() => {
     resetExecutorMocks();
@@ -35,6 +45,21 @@ describe("FN-4946 implicit completion + REVISE verdict interaction", () => {
     });
   });
 
+  it("requeues with implicit-completion refusal shape and retry count", async () => {
+    const store = createMockStore();
+    const executor = new TaskExecutor(store as any, "/repo");
+
+    await (executor as any).handleImplicitTaskDoneRefusal(makeTask({ id: "FN-4946-R1" }), "/repo/.worktrees/swift-falcon", refusal());
+
+    expect(store.updateTask).toHaveBeenCalledWith("FN-4946-R1", expect.objectContaining({ taskDoneRetryCount: 1, status: "failed" }));
+    expect(store.moveTask).toHaveBeenCalledWith("FN-4946-R1", "todo", { preserveProgress: true });
+    const refusalLogCall = store.logEntry.mock.calls.find(
+      ([id, message]: [string, string]) => id === "FN-4946-R1" && message.includes("pending-code-review-revise"),
+    );
+    expect(refusalLogCall).toBeTruthy();
+    expect(executorLog.error).toHaveBeenCalledWith(expect.stringContaining("(implicit completion)"));
+  });
+
   it("does not refuse implicit completion when REVISE is on an already done step", async () => {
     const store = createMockStore();
     let task: any = makeTask();
@@ -69,4 +94,17 @@ describe("FN-4946 implicit completion + REVISE verdict interaction", () => {
     expect(store.moveTask).toHaveBeenCalledWith("FN-4946-R", "in-review");
     expect(store.moveTask).not.toHaveBeenCalledWith("FN-4946-R", "todo", { preserveProgress: true });
   });
+
+  it("does not double-burn retry count when refusal is already handled", async () => {
+    const store = createMockStore();
+    const executor = new TaskExecutor(store as any, "/repo");
+
+    await (executor as any).handleImplicitTaskDoneRefusal(makeTask({ id: "FN-4946-R2" }), "/repo/.worktrees/swift-falcon", refusal());
+
+    const retryCountUpdates = store.updateTask.mock.calls.filter(
+      ([id, patch]: [string, Record<string, unknown>]) => id === "FN-4946-R2" && "taskDoneRetryCount" in patch,
+    );
+    expect(retryCountUpdates).toHaveLength(1);
+    expect(retryCountUpdates[0]?.[1]).toEqual(expect.objectContaining({ taskDoneRetryCount: 1 }));
+  });
 });