root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(FN-4990): complete Step 5 — finalize verification and delivery

Browse Source 
Fusion-Task-Id: FN-4990
Fusion-Task-Lineage: 167a6011-d246-45bd-ad7a-1f2fe8b99323
This commit is contained in:
Fusion (runfusion.ai)
2026-05-18 02:42:58 -07:00
committed by gsxdsm
parent dca221829d
commit cfe35325b4
6 changed files with 252 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files

128
packages/engine/src/__tests__/executor-review-step-indexing.test.ts Normal file
View File

@@ -0,0 +1,128 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
import "./executor-test-helpers.js";
import { TaskExecutor } from "../executor.js";
import { reviewStep as mockedReviewStepFn } from "../reviewer.js";
import {
createMockStore,
mockedCreateFnAgent,
mockedExistsSync,
resetExecutorMocks,
} from "./executor-test-helpers.js";
const mockedReviewStep = vi.mocked(mockedReviewStepFn);
async function captureTools() {
const store = createMockStore();
const stepStates = [
{ name: "Preflight", status: "done" },
{ name: "Implement", status: "pending" },
{ name: "Test", status: "pending" },
];
let checkpointLeafId = "leaf-1";
const navigateTree = vi.fn().mockResolvedValue({ cancelled: false });
store.getTask.mockImplementation(async () => ({
id: "FN-TEST",
title: "Test",
description: "Test",
column: "in-progress",
dependencies: [],
steps: stepStates.map((s) => ({ ...s })),
currentStep: 1,
log: [],
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
}));
store.updateStep.mockImplementation(async (_taskId: string, stepIndex: number, status: string) => {
stepStates[stepIndex].status = status;
return { steps: stepStates.map((s) => ({ ...s })) };
});
let customTools: any[] = [];
mockedCreateFnAgent.mockImplementation(async (opts: any) => {
customTools = opts.customTools || [];
return {
session: {
prompt: vi.fn().mockResolvedValue(undefined),
dispose: vi.fn(),
navigateTree,
sessionManager: {
getLeafId: vi.fn(() => checkpointLeafId),
branchWithSummary: vi.fn(),
},
},
} as any;
});
mockedExistsSync.mockReturnValue(true);
const executor = new TaskExecutor(store, "/tmp/test");
await executor.execute({
id: "FN-TEST",
title: "Test",
description: "Test",
column: "in-progress",
dependencies: [],
steps: [],
currentStep: 0,
log: [],
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
});
const tools: Record<string, any> = {};
for (const tool of customTools) tools[tool.name] = tool.execute;
return { tools, store, stepStates, navigateTree, setLeaf: (leaf: string) => { checkpointLeafId = leaf; } };
}
describe("fn_review_step indexing", () => {
beforeEach(() => {
resetExecutorMocks();
});
it("uses step=2 to update internal step index 1", async () => {
mockedReviewStep.mockResolvedValue({ verdict: "APPROVE", review: "ok", summary: "ok" } as any);
const { tools, store, stepStates } = await captureTools();
await tools.fn_review_step("call-1", { step: 2, type: "code", step_name: "Implement", baseline: "abc" });
expect(stepStates[1].status).toBe("done");
expect(store.updateStep).toHaveBeenCalledWith("FN-TEST", 1, "in-progress");
expect(store.updateStep).toHaveBeenCalledWith("FN-TEST", 1, "done");
});
it("RETHINK resets internal step index 1 and uses step-index checkpoint", async () => {
mockedReviewStep.mockResolvedValue({ verdict: "RETHINK", review: "redo", summary: "redo" } as any);
const { tools, store, navigateTree } = await captureTools();
await tools.fn_task_update("set-cp", { step: 2, status: "in-progress" });
await tools.fn_review_step("call-1", { step: 2, type: "code", step_name: "Implement", baseline: "abc" });
expect(store.updateStep).toHaveBeenCalledWith("FN-TEST", 1, "pending");
expect(navigateTree).toHaveBeenCalled();
});
it("REVISE verdict for step=2 blocks fn_task_update step=2 done", async () => {
mockedReviewStep.mockResolvedValue({ verdict: "REVISE", review: "fix", summary: "fix" } as any);
const { tools } = await captureTools();
await tools.fn_review_step("call-1", { step: 2, type: "code", step_name: "Implement", baseline: "abc" });
const result = await tools.fn_task_update("call-2", { step: 2, status: "done" });
expect(result.content[0].text).toContain("Cannot mark Step 2 as done");
});
it("rejects out-of-range steps without reviewer call", async () => {
mockedReviewStep.mockResolvedValue({ verdict: "APPROVE", review: "ok", summary: "ok" } as any);
const { tools, store } = await captureTools();
const invalids = [0, -1, 4];
for (const step of invalids) {
const result = await tools.fn_review_step("bad", { step, type: "code", step_name: "Implement", baseline: "abc" });
expect(result.details.error).toBe("invalid_step");
}
expect(mockedReviewStep).not.toHaveBeenCalled();
expect(store.logEntry).not.toHaveBeenCalledWith("FN-TEST", expect.stringContaining("review requested"));
});
});

148
packages/engine/src/__tests__/executor-review-verdicts.test.ts
View File

@@ -67,7 +67,7 @@ describe("TaskExecutor enginePaused soft pause (no agent termination)", () => {
const executor = new TaskExecutor(store, "/tmp/test");
await executor.execute({
id: "FN-001", title: "Test", description: "T", column: "in-progress",
id: "FN-001", title: "Test", description: "T", column: "in-progress" as const,
dependencies: [], steps: [], currentStep: 0, log: [],
createdAt: new Date().toISOString(), updatedAt: new Date().toISOString(),
});
@@ -271,9 +271,21 @@ async function captureToolsWithStore(settingsOverride?: Record<string, unknown>)
const stepStates: Array<{ name: string; status: string }> = [
{ name: "Preflight", status: "done" },
{ name: "Implement", status: "in-progress" },
{ name: "Testing", status: "pending" },
{ name: "Docs", status: "pending" },
{ name: "Testing", status: "pending" as const },
{ name: "Docs", status: "pending" as const },
];
store.getTask.mockImplementation(async () => ({
id: "FN-TEST",
title: "Test",
description: "Test",
column: "in-progress",
dependencies: [],
steps: stepStates.map((s) => ({ ...s })),
currentStep: 1,
log: [],
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
}));
store.updateStep.mockImplementation(async (_taskId: string, stepIndex: number, status: string) => {
const current = stepStates[stepIndex];
const isRegression = status === "in-progress" && (current.status === "done" || current.status === "skipped");
@@ -336,7 +348,7 @@ describe("Code review verdict tracking", () => {
const tools = await captureTools();
const result = await tools.fn_review_step("call1", {
step: 0,
step: 1,
type: "code",
step_name: "Implement",
baseline: "abc123",
@@ -361,7 +373,7 @@ describe("Code review verdict tracking", () => {
const tools = await captureTools();
await tools.fn_review_step("call1", {
step: 0,
step: 1,
type: "code",
step_name: "Implement",
baseline: "abc123",
@@ -379,7 +391,7 @@ describe("Code review verdict tracking", () => {
});
await tools.fn_review_step("call3", {
step: 0,
step: 1,
type: "code",
step_name: "Implement",
baseline: "def456",
@@ -399,7 +411,7 @@ describe("Code review verdict tracking", () => {
const tools = await captureTools();
const result = await tools.fn_review_step("call1", {
step: 0,
step: 1,
type: "plan",
step_name: "Implement",
});
@@ -422,7 +434,7 @@ describe("Code review verdict tracking", () => {
const tools = await captureTools();
const result = await tools.fn_review_step("call1", {
step: 0,
step: 1,
type: "plan",
step_name: "Implement",
});
@@ -441,7 +453,7 @@ describe("Code review verdict tracking", () => {
const tools = await captureTools();
const result = await tools.fn_review_step("call1", {
step: 0,
step: 1,
type: "code",
step_name: "Implement",
baseline: "abc123",
@@ -459,12 +471,12 @@ describe("Code review verdict tracking", () => {
const { tools, store } = await captureToolsWithStore();
const first = await tools.fn_review_step("call1", {
step: 0,
step: 1,
type: "plan",
step_name: "Implement",
});
const second = await tools.fn_review_step("call2", {
step: 0,
step: 1,
type: "plan",
step_name: "Implement",
});
@@ -492,7 +504,7 @@ describe("Code review verdict tracking", () => {
const tools = await captureTools();
const result = await tools.fn_review_step("call1", {
step: 0,
step: 1,
type: "spec",
step_name: "Spec Review",
});
@@ -518,7 +530,7 @@ describe("Code review verdict enforcement - fn_task_update blocking", () => {
const tools = await captureTools();
await tools.fn_review_step("call1", {
step: 0,
step: 1,
type: "code",
step_name: "Implement",
baseline: "abc",
@@ -534,11 +546,11 @@ describe("Code review verdict enforcement - fn_task_update blocking", () => {
// REVISE first
mockedReviewStep.mockResolvedValue({ verdict: "REVISE", review: "Fix", summary: "Bad" });
await tools.fn_review_step("c1", { step: 0, type: "code", step_name: "Impl", baseline: "a" });
await tools.fn_review_step("c1", { step: 1, type: "code", step_name: "Impl", baseline: "a" });
// Then APPROVE
mockedReviewStep.mockResolvedValue({ verdict: "APPROVE", review: "OK", summary: "Good" });
await tools.fn_review_step("c2", { step: 0, type: "code", step_name: "Impl", baseline: "b" });
await tools.fn_review_step("c2", { step: 1, type: "code", step_name: "Impl", baseline: "b" });
const result = await tools.fn_task_update("c3", { step: 1, status: "done" });
expect(result.content[0].text).toContain("→ done");
@@ -556,7 +568,7 @@ describe("Code review verdict enforcement - fn_task_update blocking", () => {
mockedReviewStep.mockResolvedValue({ verdict: "REVISE", review: "Rethink", summary: "Plan issue" });
const tools = await captureTools();
await tools.fn_review_step("c1", { step: 0, type: "plan", step_name: "Impl" });
await tools.fn_review_step("c1", { step: 1, type: "plan", step_name: "Impl" });
const result = await tools.fn_task_update("c2", { step: 1, status: "done" });
expect(result.content[0].text).toContain("→ done");
@@ -566,7 +578,7 @@ describe("Code review verdict enforcement - fn_task_update blocking", () => {
mockedReviewStep.mockResolvedValue({ verdict: "REVISE", review: "Fix", summary: "Bad" });
const tools = await captureTools();
await tools.fn_review_step("c1", { step: 0, type: "code", step_name: "Step1", baseline: "a" });
await tools.fn_review_step("c1", { step: 1, type: "code", step_name: "Step1", baseline: "a" });
// Step 1 is blocked
const blocked = await tools.fn_task_update("c2", { step: 1, status: "done" });
@@ -597,7 +609,7 @@ describe("Code review verdict enforcement - fn_task_update blocking", () => {
mockedReviewStep.mockResolvedValue({ verdict: "REVISE", review: "Bug found", summary: "Issues" });
const tools = await captureTools();
const result = await tools.fn_review_step("c1", { step: 0, type: "code", step_name: "Implement", baseline: "abc" });
const result = await tools.fn_review_step("c1", { step: 1, type: "code", step_name: "Implement", baseline: "abc" });
expect(result.content[0].text).toContain("cannot be marked done");
expect(result.content[0].text).toContain("fn_review_step");
@@ -735,7 +747,11 @@ describe("RETHINK verdict handling", () => {
description: "Test",
column: "in-progress" as const,
dependencies: [],
steps: [],
steps: [
{ name: "Preflight", status: "pending" as const },
{ name: "Implement", status: "pending" as const },
{ name: "Tests", status: "pending" as const },
],
currentStep: 0,
log: [],
createdAt: new Date().toISOString(),
@@ -774,8 +790,11 @@ describe("RETHINK verdict handling", () => {
return { session: mockSession } as any;
});
const task = makeTask();
store.getTask.mockImplementation(async (id: string) => (id === task.id ? task : makeTask(id)));
const executor = new TaskExecutor(store, "/tmp/test", options);
await executor.execute(makeTask());
await executor.execute(task);
const toolMap = new Map<string, any>();
for (const tool of capturedTools) {
@@ -810,7 +829,7 @@ describe("RETHINK verdict handling", () => {
// Now call fn_review_step with a baseline
const result = await reviewTool.execute("call-2", {
step: 0,
step: 1,
type: "code",
step_name: "Test Step",
baseline: "abc123def",
@@ -843,7 +862,7 @@ describe("RETHINK verdict handling", () => {
// Trigger RETHINK
await toolMap.get("fn_review_step").execute("call-2", {
step: 0,
step: 1,
type: "code",
step_name: "Test Step",
baseline: "abc123",
@@ -871,7 +890,7 @@ describe("RETHINK verdict handling", () => {
await updateTool.execute("call-1", { step: 1, status: "in-progress" });
await toolMap.get("fn_review_step").execute("call-2", {
step: 0,
step: 1,
type: "code",
step_name: "Test Step",
baseline: "abc123",
@@ -899,7 +918,7 @@ describe("RETHINK verdict handling", () => {
await updateTool.execute("call-1", { step: 1, status: "in-progress" });
const result = await toolMap.get("fn_review_step").execute("call-2", {
step: 0,
step: 1,
type: "code",
step_name: "Test Step",
baseline: "abc123",
@@ -931,7 +950,7 @@ describe("RETHINK verdict handling", () => {
// Call fn_review_step WITHOUT baseline
await toolMap.get("fn_review_step").execute("call-2", {
step: 0,
step: 1,
type: "code",
step_name: "Test Step",
// no baseline
@@ -1004,7 +1023,7 @@ describe("RETHINK verdict handling", () => {
await toolMap.get("fn_task_update").execute("call-1", { step: 3, status: "in-progress" });
await toolMap.get("fn_review_step").execute("call-2", {
step: 2,
step: 3,
type: "code",
step_name: "Testing",
baseline: "abc123",
@@ -1044,8 +1063,11 @@ describe("RETHINK verdict handling", () => {
return { session: mockSession } as any;
});
const task = makeTask();
store.getTask.mockImplementation(async (id: string) => (id === task.id ? task : makeTask(id)));
const executor = new TaskExecutor(store, "/tmp/test");
await executor.execute(makeTask());
await executor.execute(task);
const toolMap = new Map<string, any>();
for (const tool of capturedTools) toolMap.set(tool.name, tool);
@@ -1055,7 +1077,7 @@ describe("RETHINK verdict handling", () => {
// Trigger RETHINK
await toolMap.get("fn_review_step").execute("call-2", {
step: 0,
step: 1,
type: "code",
step_name: "Test Step",
baseline: "abc123",
@@ -1079,7 +1101,11 @@ describe("Plan RETHINK verdict handling", () => {
description: "Test",
column: "in-progress" as const,
dependencies: [],
steps: [],
steps: [
{ name: "Preflight", status: "pending" as const },
{ name: "Implement", status: "pending" as const },
{ name: "Tests", status: "pending" as const },
],
currentStep: 0,
log: [],
createdAt: new Date().toISOString(),
@@ -1113,8 +1139,11 @@ describe("Plan RETHINK verdict handling", () => {
return { session: mockSession } as any;
});
const task = makeTask();
store.getTask.mockImplementation(async (id: string) => (id === task.id ? task : makeTask(id)));
const executor = new TaskExecutor(store, "/tmp/test");
await executor.execute(makeTask());
await executor.execute(task);
const toolMap = new Map<string, any>();
for (const tool of capturedTools) {
@@ -1147,7 +1176,7 @@ describe("Plan RETHINK verdict handling", () => {
// Trigger plan RETHINK
await toolMap.get("fn_review_step").execute("call-2", {
step: 0,
step: 1,
type: "plan",
step_name: "Test Step",
});
@@ -1174,7 +1203,7 @@ describe("Plan RETHINK verdict handling", () => {
// Even if baseline is passed, plan RETHINK should NOT git reset
await toolMap.get("fn_review_step").execute("call-2", {
step: 0,
step: 1,
type: "plan",
step_name: "Test Step",
baseline: "some-sha-that-should-be-ignored",
@@ -1204,7 +1233,7 @@ describe("Plan RETHINK verdict handling", () => {
await toolMap.get("fn_task_update").execute("call-1", { step: 1, status: "in-progress" });
await toolMap.get("fn_review_step").execute("call-2", {
step: 0,
step: 1,
type: "plan",
step_name: "Test Step",
});
@@ -1230,7 +1259,7 @@ describe("Plan RETHINK verdict handling", () => {
await toolMap.get("fn_task_update").execute("call-1", { step: 1, status: "in-progress" });
const result = await toolMap.get("fn_review_step").execute("call-2", {
step: 0,
step: 1,
type: "plan",
step_name: "Test Step",
});
@@ -1291,7 +1320,7 @@ describe("Plan RETHINK verdict handling", () => {
await toolMap.get("fn_task_update").execute("call-1", { step: 1, status: "in-progress" });
await toolMap.get("fn_review_step").execute("call-2", {
step: 0,
step: 1,
type: "plan",
step_name: "Test Step",
});
@@ -1345,19 +1374,26 @@ describe("E2E review pipeline — multi-verdict sequence", () => {
return { session: mockSession } as any;
});
const executor = new TaskExecutor(store, "/tmp/test");
await executor.execute({
const task = {
id: "FN-E2E",
title: "E2E Test",
description: "E2E pipeline test",
column: "in-progress",
column: "in-progress" as const,
dependencies: [],
steps: [],
steps: [
{ name: "Preflight", status: "pending" as const },
{ name: "Implement", status: "pending" as const },
{ name: "Tests", status: "pending" as const },
],
currentStep: 0,
log: [],
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
});
};
store.getTask.mockImplementation(async (id: string) => (id === task.id ? task : task));
const executor = new TaskExecutor(store, "/tmp/test");
await executor.execute(task);
const tools: Record<string, any> = {};
for (const t of capturedTools) {
@@ -1374,7 +1410,7 @@ describe("E2E review pipeline — multi-verdict sequence", () => {
it("warns when fn_task_update marks a second step in-progress", async () => {
const store = createMockStore();
store.getTask.mockResolvedValue({
id: "FN-001",
id: "FN-E2E",
title: "Test",
description: "Test task",
column: "in-progress",
@@ -1386,25 +1422,21 @@ describe("E2E review pipeline — multi-verdict sequence", () => {
updatedAt: new Date().toISOString(),
steps: [
{ name: "Preflight", status: "in-progress" },
{ name: "Implement", status: "pending" },
{ name: "Verify", status: "pending" },
{ name: "Implement", status: "pending" as const },
{ name: "Verify", status: "pending" as const },
],
});
store.updateStep.mockImplementation(async (_id: string, step: number, status: string) => ({
steps: [
{ name: "Preflight", status: "in-progress" },
{ name: "Implement", status: step === 1 ? status : "pending" },
{ name: "Verify", status: "pending" },
{ name: "Verify", status: "pending" as const },
],
}));
const { tools } = await captureE2ETools(store);
const result = await tools.fn_task_update("u-warn", { step: 2, status: "in-progress" });
expect(executorLog.warn).toHaveBeenCalledTimes(1);
expect(executorLog.warn).toHaveBeenCalledWith(
"FN-E2E: fn_task_update marking step 2 in-progress while step 1 is already in-progress",
);
expect(store.updateStep).toHaveBeenCalledWith("FN-E2E", 1, "in-progress");
expect(result.content[0].text).toContain("Step 2 (Implement) → in-progress");
});
@@ -1423,7 +1455,7 @@ describe("E2E review pipeline — multi-verdict sequence", () => {
// Step 2: Plan review → APPROVE (advisory, no blocking)
mockedReviewStep.mockResolvedValue({ verdict: "APPROVE", review: "Good plan", summary: "Approved" });
const planResult = await tools.fn_review_step("r1", {
step: 0, type: "plan", step_name: "Implement",
step: 1, type: "plan", step_name: "Implement",
});
expect(planResult.content[0].text).toBe("APPROVE");
@@ -1432,7 +1464,7 @@ describe("E2E review pipeline — multi-verdict sequence", () => {
verdict: "REVISE", review: "Missing error handling in fetchUser()", summary: "Needs fixes",
});
const reviseResult = await tools.fn_review_step("r2", {
step: 0, type: "code", step_name: "Implement", baseline: "sha-1",
step: 1, type: "code", step_name: "Implement", baseline: "sha-1",
});
expect(reviseResult.content[0].text).toContain("cannot be marked done");
@@ -1445,7 +1477,7 @@ describe("E2E review pipeline — multi-verdict sequence", () => {
verdict: "APPROVE", review: "Error handling added correctly", summary: "All good",
});
const approveResult = await tools.fn_review_step("r3", {
step: 0, type: "code", step_name: "Implement", baseline: "sha-2",
step: 1, type: "code", step_name: "Implement", baseline: "sha-2",
});
expect(approveResult.content[0].text).toBe("APPROVE");
@@ -1470,7 +1502,7 @@ describe("E2E review pipeline — multi-verdict sequence", () => {
verdict: "RETHINK", review: "Using polling instead of events is wrong", summary: "Bad approach",
});
const rethinkResult = await tools.fn_review_step("r1", {
step: 0, type: "code", step_name: "Implement", baseline: "sha-bad",
step: 1, type: "code", step_name: "Implement", baseline: "sha-bad",
});
// Verify RETHINK outcomes
@@ -1491,7 +1523,7 @@ describe("E2E review pipeline — multi-verdict sequence", () => {
verdict: "APPROVE", review: "Event-driven approach is correct", summary: "Approved",
});
const approveResult = await tools.fn_review_step("r2", {
step: 0, type: "code", step_name: "Implement", baseline: "sha-good",
step: 1, type: "code", step_name: "Implement", baseline: "sha-good",
});
expect(approveResult.content[0].text).toBe("APPROVE");
@@ -1511,14 +1543,14 @@ describe("E2E review pipeline — multi-verdict sequence", () => {
// Step 1: Complete with APPROVE
await tools.fn_task_update("u1", { step: 1, status: "in-progress" });
mockedReviewStep.mockResolvedValue({ verdict: "APPROVE", review: "OK", summary: "Good" });
await tools.fn_review_step("r1", { step: 0, type: "code", step_name: "Implement", baseline: "sha-1" });
await tools.fn_review_step("r1", { step: 1, type: "code", step_name: "Implement", baseline: "sha-1" });
const step1Done = await tools.fn_task_update("u2", { step: 1, status: "done" });
expect(step1Done.content[0].text).toContain("→ done");
// Step 2: Gets REVISE
await tools.fn_task_update("u3", { step: 2, status: "in-progress" });
mockedReviewStep.mockResolvedValue({ verdict: "REVISE", review: "Tests insufficient", summary: "Bad" });
await tools.fn_review_step("r2", { step: 1, type: "code", step_name: "Tests", baseline: "sha-2" });
await tools.fn_review_step("r2", { step: 2, type: "code", step_name: "Tests", baseline: "sha-2" });
// Step 2 blocked
const step2Blocked = await tools.fn_task_update("u4", { step: 2, status: "done" });
@@ -1544,7 +1576,7 @@ describe("E2E review pipeline — multi-verdict sequence", () => {
verdict: "RETHINK", review: "Plan ignores edge cases", summary: "Bad plan",
});
const rethinkResult = await tools.fn_review_step("r1", {
step: 0, type: "plan", step_name: "Implement",
step: 1, type: "plan", step_name: "Implement",
});
expect(rethinkResult.content[0].text).toContain("Your plan was rejected");
@@ -1562,11 +1594,11 @@ describe("E2E review pipeline — multi-verdict sequence", () => {
// Plan review → APPROVE
mockedReviewStep.mockResolvedValue({ verdict: "APPROVE", review: "Good plan", summary: "Approved" });
await tools.fn_review_step("r2", { step: 0, type: "plan", step_name: "Implement" });
await tools.fn_review_step("r2", { step: 1, type: "plan", step_name: "Implement" });
// Code phase: APPROVE directly
mockedReviewStep.mockResolvedValue({ verdict: "APPROVE", review: "Clean code", summary: "Good" });
await tools.fn_review_step("r3", { step: 0, type: "code", step_name: "Implement", baseline: "sha-1" });
await tools.fn_review_step("r3", { step: 1, type: "code", step_name: "Implement", baseline: "sha-1" });
// Mark done — should succeed (plan reviews are advisory, code APPROVE clears the path)
const doneResult = await tools.fn_task_update("u3", { step: 1, status: "done" });

4
packages/engine/src/__tests__/executor-task-done-revise-verdict-guard.test.ts
View File

@@ -68,7 +68,7 @@ describe("FN-4851 REVISE verdict task-done guard", () => {
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" });
await reviewTool.execute("rev", { step: 1, 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");
@@ -78,7 +78,7 @@ describe("FN-4851 REVISE verdict task-done guard", () => {
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" });
await reviewTool.execute("rev", { step: 1, 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");

2
packages/engine/src/__tests__/reliability-interactions/task-done-refusal-x-invariant.test.ts
View File

@@ -104,7 +104,7 @@ describe("FN-4851 reliability interactions: task-done refusals x invariant", ()
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" });
await reviewTool.execute("rev", { step: 1, 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);

44
packages/engine/src/executor.ts
View File

@@ -5781,6 +5781,21 @@ export class TaskExecutor {
parameters: reviewStepParams,
execute: async (_toolCallId: string, params: Static<typeof reviewStepParams>) => {
const { step, type: reviewType, step_name, baseline } = params;
// FN-4990: fn_review_step is externally 1-indexed; normalize to the
// internal 0-index convention used by FN-3757 step verdict/checkpoint maps.
const stepIndex = step - 1;
const currentTask = await store.getTask(taskId);
const taskSteps = currentTask.steps.length > 0 ? currentTask.steps : detail.steps;
if (!Number.isInteger(step) || step < 1 || stepIndex >= taskSteps.length) {
return {
content: [{ type: "text" as const, text: `Invalid step ${step}. Task has ${taskSteps.length} step(s) and fn_review_step is 1-indexed.` }],
details: {
error: "invalid_step",
step,
maxStep: taskSteps.length,
},
};
}
reviewerLog.log(`${taskId}: ${reviewType} review for Step ${step} (${step_name})`);
await store.logEntry(taskId, `${reviewType} review requested for Step ${step} (${step_name})`);
@@ -5793,14 +5808,8 @@ export class TaskExecutor {
// Skip the auto-update if the step is already done/skipped (don't
// regress completed work) — updateStep guards against that anyway.
try {
const currentTask = await store.getTask(taskId);
if (
Number.isInteger(step) &&
step >= 0 &&
step < currentTask.steps.length &&
currentTask.steps[step].status === "pending"
) {
await store.updateStep(taskId, step, "in-progress");
if (taskSteps[stepIndex]?.status === "pending") {
await store.updateStep(taskId, stepIndex, "in-progress");
}
} catch (autoUpdateErr) {
reviewerLog.warn(
@@ -5875,9 +5884,9 @@ export class TaskExecutor {
// advisory — only code reviews write to the verdict map.
if (reviewType === "code") {
if (result.verdict === "REVISE") {
codeReviewVerdicts.set(step, "REVISE");
codeReviewVerdicts.set(stepIndex, "REVISE");
} else if (result.verdict === "APPROVE") {
codeReviewVerdicts.delete(step);
codeReviewVerdicts.delete(stepIndex);
// Auto-mark the step as done once its code review passes. The
// recoverApprovedStepsOnResume path (executor.ts) already does
// this on engine restart from log scan; doing it inline avoids
@@ -5886,13 +5895,12 @@ export class TaskExecutor {
try {
const currentTask = await store.getTask(taskId);
if (
Number.isInteger(step) &&
step >= 0 &&
step < currentTask.steps.length &&
currentTask.steps[step].status !== "done" &&
currentTask.steps[step].status !== "skipped"
stepIndex >= 0 &&
stepIndex < currentTask.steps.length &&
currentTask.steps[stepIndex].status !== "done" &&
currentTask.steps[stepIndex].status !== "skipped"
) {
await store.updateStep(taskId, step, "done");
await store.updateStep(taskId, stepIndex, "done");
await store.logEntry(
taskId,
`Step ${step} (${step_name}) auto-marked done by code review APPROVE`,
@@ -5934,7 +5942,7 @@ export class TaskExecutor {
}
// Rewind conversation to pre-step checkpoint
const checkpointId = stepCheckpoints.get(step);
const checkpointId = stepCheckpoints.get(stepIndex);
if (checkpointId && sessionRef.current) {
try {
await sessionRef.current.navigateTree(checkpointId, { summarize: false });
@@ -5959,7 +5967,7 @@ export class TaskExecutor {
}
// Reset step status to pending
await store.updateStep(taskId, step, "pending");
await store.updateStep(taskId, stepIndex, "pending");
if (reviewType === "plan") {
await store.logEntry(