FN-6607: align step tools with zero-based prompt steps

Align executor step tools and review bookkeeping with the 0-based Step N labels agents see in PROMPT.md. - Treat fn_task_update and fn_review_step step parameters as 0-indexed values, including validation, logs, checkpoints, and review verdict maps. - Update executor/reviewer/step-runner guidance and generated tool docs to describe Step 0 semantics consistently. - Adjust affected executor and reliability tests and add coverage proving Step 0 progress, review, and revise handling work without off-by-one shifts. - Add a patch changeset for the published Fusion CLI package. Files changed: .changeset/fn-6607-step-numbering.md | 5 + .../cli/skill/fusion/references/engine-tools.md | 4 +- .../engine/src/__tests__/executor-pause.test.ts | 2 +- .../executor-review-step-indexing.test.ts | 18 +- .../src/__tests__/executor-review-verdicts.test.ts | 18 +- .../executor-step-numbering-zero-based.test.ts | 196 +++++++++++++++++++++ .../src/__tests__/executor-step-session.test.ts | 24 ++- ...executor-task-done-revise-verdict-guard.test.ts | 4 +- .../executor-pending-review-skip-retry.test.ts | 8 +- .../task-done-refusal-x-invariant.test.ts | 2 +- packages/engine/src/__tests__/step-runner.test.ts | 4 +- packages/engine/src/executor.ts | 57 +++--- packages/engine/src/reviewer.ts | 3 + packages/engine/src/step-runner.ts | 6 +- 14 files changed, 284 insertions(+), 67 deletions(-) Fusion-Task-Id: FN-6607 Fusion-Task-Lineage: 1b1fb1d8-07ca-4a33-84f5-1dab83388c01
2026-06-17 19:52:20 -07:00
parent 4c3186d0d1
commit a013bc0309
14 changed files with 284 additions and 67 deletions
--- a/.changeset/fn-6607-step-numbering.md
+++ b/.changeset/fn-6607-step-numbering.md
@@ -0,0 +1,5 @@
+---
+"@runfusion/fusion": patch
+---
+
+Fix the perpetual step off-by-one: `fn_task_update` and `fn_review_step` now treat `step` as 0-based, matching the `### Step N:` numbering in PROMPT.md (Step 0 = Preflight) and `TaskStore.updateStep`. Previously the tools were 1-indexed while everything agent-facing was 0-based, so agents could not mark Step 0 done and reviews/progress landed one step early.
--- a/packages/cli/skill/fusion/references/engine-tools.md
+++ b/packages/cli/skill/fusion/references/engine-tools.md
@@ -68,10 +68,10 @@ Note: step-session execution (`step-session-executor.ts`) reuses executor coordi

 | Tool | Purpose | Parameters |
 |---|---|---|
-| `fn_task_update` | Update a spec step status (`pending`/`in-progress`/`done`/`skipped`), task dependencies, and/or workflow-defined custom field values | `step?` (number, 1-indexed), `status?` (enum), `dependencies?` (string[]), `custom_fields?` (object keyed by field id; validated against the workflow field schema, `null` clears a field) |
+| `fn_task_update` | Update a spec step status (`pending`/`in-progress`/`done`/`skipped`), task dependencies, and/or workflow-defined custom field values | `step?` (number, 0-indexed; matches `### Step N:` in PROMPT.md, Step 0 = Preflight), `status?` (enum), `dependencies?` (string[]), `custom_fields?` (object keyed by field id; validated against the workflow field schema, `null` clears a field) |
 | `fn_task_add_dep` | Add a dependency to current task (confirmation-gated) | `task_id` (string), `confirm?` (boolean) |
 | `fn_task_done` | Mark task complete and optionally store summary | `summary?` (string) |
-| `fn_review_step` | Spawn step plan/code reviewer | `step` (number), `type` (`plan` \| `code`), `step_name` (string), `baseline?` (string) |
+| `fn_review_step` | Spawn step plan/code reviewer | `step` (number, 0-indexed; matches `### Step N:` in PROMPT.md), `type` (`plan` \| `code`), `step_name` (string), `baseline?` (string) |
 | `fn_spawn_agent` | Spawn child agent in separate worktree | `name` (string), `role` (enum), `task` (string) |

 ## Merger-only runtime tools (`merger.ts`)
--- a/packages/engine/src/tests/executor-pause.test.ts
+++ b/packages/engine/src/tests/executor-pause.test.ts
@@ -1402,7 +1402,7 @@ describe("TaskExecutor agent execution flow (FN-978)", () => {
      stuckDetector,
    );

-    const result = await tool.execute("call-1", { step: 1, status: "in-progress" });
+    const result = await tool.execute("call-1", { step: 0, status: "in-progress" });

    expect(stuckDetector.recordIgnoredStepUpdate).toHaveBeenCalledWith("FN-001");
    expect(result.content[0].text).toContain("already done");
--- a/packages/engine/src/tests/executor-review-step-indexing.test.ts
+++ b/packages/engine/src/tests/executor-review-step-indexing.test.ts
@@ -80,11 +80,11 @@ describe("fn_review_step indexing", () => {
    resetExecutorMocks();
  });

-  it("uses step=2 to update internal step index 1", async () => {
+  it("uses step=1 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" });
+    await tools.fn_review_step("call-1", { step: 1, type: "code", step_name: "Implement", baseline: "abc" });

    expect(stepStates[1].status).toBe("done");
    expect(store.updateStep).toHaveBeenCalledWith("FN-TEST", 1, "in-progress");
@@ -95,28 +95,28 @@ describe("fn_review_step indexing", () => {
    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" });
+    await tools.fn_task_update("set-cp", { step: 1, status: "in-progress" });
+    await tools.fn_review_step("call-1", { step: 1, 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 () => {
+  it("REVISE verdict for step=1 blocks fn_task_update step=1 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" });
+    await tools.fn_review_step("call-1", { step: 1, type: "code", step_name: "Implement", baseline: "abc" });
+    const result = await tools.fn_task_update("call-2", { step: 1, status: "done" });

-    expect(result.content[0].text).toContain("Cannot mark Step 2 as done");
+    expect(result.content[0].text).toContain("Cannot mark Step 1 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];
+    const invalids = [-1, 3, 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");
--- a/packages/engine/src/tests/executor-review-verdicts.test.ts
+++ b/packages/engine/src/tests/executor-review-verdicts.test.ts
@@ -732,7 +732,7 @@ describe("Code review verdict enforcement - fn_task_update blocking", () => {
    // the same step does not produce the "Cannot mark … as done" block.
    await tools.fn_review_step("c1", { step: 2, type: "code", step_name: "Testing", baseline: "a" });

-    const result = await tools.fn_task_update("c2", { step: 3, status: "in-progress" });
+    const result = await tools.fn_task_update("c2", { step: 2, status: "in-progress" });
    expect(result.content[0].text).not.toContain("Cannot mark");
    expect(result.content[0].text).toContain("→ in-progress");
  });
@@ -897,7 +897,7 @@ describe("RETHINK verdict handling", () => {
    });

    // updateStep should be called: once for in-progress, once for pending (reset)
-    expect(store.updateStep).toHaveBeenCalledWith("FN-040", 0, "pending");
+    expect(store.updateStep).toHaveBeenCalledWith("FN-040", 1, "pending");
  });

  it("RETHINK re-prompt includes reviewer feedback", async () => {
@@ -1012,7 +1012,7 @@ describe("RETHINK verdict handling", () => {
    expect(mockSessionManager.getLeafId).toHaveBeenCalled();
  });

-  it("uses step-1 checkpoint key when step 3 enters in-progress and step index 2 is reviewed", async () => {
+  it("uses zero-based checkpoint key when step 2 enters in-progress and is reviewed", async () => {
    const store = createMockStore();
    store.updateStep.mockImplementation(async (_id: string, step: number, status: string) =>
      makeStepResult(step, status),
@@ -1020,10 +1020,10 @@ describe("RETHINK verdict handling", () => {
    mockedReviewStep.mockResolvedValue({ verdict: "RETHINK", review: "Bad", summary: "Redo" });

    const { toolMap, mockNavigateTree } = await captureRethinkTools(store);
-    await toolMap.get("fn_task_update").execute("call-1", { step: 3, status: "in-progress" });
+    await toolMap.get("fn_task_update").execute("call-1", { step: 2, status: "in-progress" });

    await toolMap.get("fn_review_step").execute("call-2", {
-      step: 3,
+      step: 2,
      type: "code",
      step_name: "Testing",
      baseline: "abc123",
@@ -1239,7 +1239,7 @@ describe("Plan RETHINK verdict handling", () => {
    });

    // updateStep should be called with "pending" to reset the step
-    expect(store.updateStep).toHaveBeenCalledWith("FN-050", 0, "pending");
+    expect(store.updateStep).toHaveBeenCalledWith("FN-050", 1, "pending");
  });

  it("plan RETHINK re-prompt includes reviewer feedback and plan-specific language", async () => {
@@ -1435,10 +1435,10 @@ describe("E2E review pipeline — multi-verdict sequence", () => {
    }));

    const { tools } = await captureE2ETools(store);
-    const result = await tools.fn_task_update("u-warn", { step: 2, status: "in-progress" });
+    const result = await tools.fn_task_update("u-warn", { step: 1, status: "in-progress" });

    expect(store.updateStep).toHaveBeenCalledWith("FN-E2E", 1, "in-progress");
-    expect(result.content[0].text).toContain("Step 2 (Implement) → in-progress");
+    expect(result.content[0].text).toContain("Step 1 (Implement) → in-progress");
  });

  it("full sequence: plan APPROVE → code REVISE (blocked) → code APPROVE (unblocked) → done", async () => {
@@ -1513,7 +1513,7 @@ describe("E2E review pipeline — multi-verdict sequence", () => {
      expect.objectContaining({ cwd: expect.any(String) }),
    );
    expect(mockNavigateTree).toHaveBeenCalledWith("e2e-checkpoint", { summarize: false });
-    expect(store.updateStep).toHaveBeenCalledWith("FN-E2E", 0, "pending");
+    expect(store.updateStep).toHaveBeenCalledWith("FN-E2E", 1, "pending");

    // Step 3: Restart the step (new approach)
    await tools.fn_task_update("u2", { step: 1, status: "in-progress" });
--- a/packages/engine/src/tests/executor-step-numbering-zero-based.test.ts
+++ b/packages/engine/src/tests/executor-step-numbering-zero-based.test.ts
@@ -0,0 +1,196 @@
+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);
+
+describe("executor tool step numbering is 0-based", () => {
+  beforeEach(() => {
+    resetExecutorMocks();
+    mockedExistsSync.mockReturnValue(true);
+  });
+
+  async function captureTools(stepStates = [
+    { name: "Preflight", status: "pending" },
+    { name: "First", status: "pending" },
+    { name: "Second", status: "pending" },
+  ]) {
+    const store = createMockStore();
+    store.getTask.mockImplementation(async () => ({
+      id: "FN-6607-T",
+      title: "Zero based steps",
+      description: "",
+      column: "in-progress",
+      dependencies: [],
+      steps: stepStates.map((step) => ({ ...step })),
+      currentStep: 0,
+      log: [],
+      prompt: "# test\n## Steps\n### Step 0: Preflight\n### Step 1: First\n### Step 2: Second",
+      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((step) => ({ ...step })) };
+    });
+
+    let customTools: any[] = [];
+    mockedCreateFnAgent.mockImplementation(async (opts: any) => {
+      customTools = opts.customTools || [];
+      return {
+        session: {
+          prompt: vi.fn().mockResolvedValue(undefined),
+          dispose: vi.fn(),
+          subscribe: vi.fn(),
+          on: vi.fn(),
+          navigateTree: vi.fn(),
+          sessionManager: {
+            getLeafId: vi.fn().mockReturnValue("leaf-step"),
+            branchWithSummary: vi.fn(),
+          },
+          state: {},
+        },
+      } as any;
+    });
+
+    const executor = new TaskExecutor(store, "/tmp/test");
+    await executor.execute({
+      id: "FN-6607-T",
+      title: "Zero based steps",
+      description: "",
+      column: "in-progress",
+      dependencies: [],
+      steps: [],
+      currentStep: 0,
+      log: [],
+      createdAt: new Date().toISOString(),
+      updatedAt: new Date().toISOString(),
+    } as any);
+
+    const tools: Record<string, any> = {};
+    for (const tool of customTools) tools[tool.name] = tool.execute;
+    return { tools, store, stepStates };
+  }
+
+  it("maps fn_task_update and fn_review_step step directly to task.steps index", async () => {
+    mockedReviewStep.mockResolvedValue({ verdict: "APPROVE", review: "ok", summary: "ok" } as any);
+    const { tools, store, stepStates } = await captureTools();
+
+    const preflightDone = await tools.fn_task_update("update-0", { step: 0, status: "done" });
+    expect(preflightDone.content[0].text).toContain("Step 0 (Preflight) → done");
+    expect(store.updateStep).toHaveBeenCalledWith("FN-6607-T", 0, "done");
+    expect(stepStates[0].status).toBe("done");
+
+    const firstStarted = await tools.fn_task_update("update-1", { step: 1, status: "in-progress" });
+    expect(firstStarted.content[0].text).toContain("Step 1 (First) → in-progress");
+    expect(store.updateStep).toHaveBeenCalledWith("FN-6607-T", 1, "in-progress");
+    expect(stepStates[1].status).toBe("in-progress");
+
+    const review = await tools.fn_review_step("review-1", {
+      step: 1,
+      type: "code",
+      step_name: "First",
+      baseline: "abc123",
+    });
+    expect(review.content[0].text).toBe("APPROVE");
+    expect(mockedReviewStep).toHaveBeenCalledWith(
+      expect.any(String),
+      "FN-6607-T",
+      1,
+      "First",
+      "code",
+      expect.any(String),
+      "abc123",
+      expect.any(Object),
+    );
+    expect(store.logEntry).toHaveBeenCalledWith("FN-6607-T", "code review Step 1: APPROVE", "ok");
+    expect(store.updateStep).toHaveBeenCalledWith("FN-6607-T", 1, "done");
+
+    const invalidNegative = await tools.fn_task_update("bad-update-negative", { step: -1, status: "done" });
+    expect(invalidNegative.content[0].text).toContain("0-indexed");
+    const invalidReview = await tools.fn_review_step("bad-review", { step: 3, type: "code", step_name: "Missing", baseline: "abc" });
+    expect(invalidReview.details.error).toBe("invalid_step");
+  });
+
+  it("resume recovery reads the same 0-based review log written by fn_review_step", async () => {
+    const store = createMockStore();
+    store.getTask.mockResolvedValue({
+      id: "FN-6607-R",
+      title: "Resume",
+      description: "",
+      column: "in-progress",
+      dependencies: [],
+      steps: [
+        { name: "Preflight", status: "done" },
+        { name: "First", status: "in-progress" },
+        { name: "Second", status: "pending" },
+      ],
+      currentStep: 1,
+      log: [
+        { timestamp: "2026-06-17T00:00:00.000Z", action: "Step 1 (First) → in-progress" },
+        { timestamp: "2026-06-17T00:00:01.000Z", action: "code review Step 1: APPROVE" },
+      ],
+      createdAt: new Date().toISOString(),
+      updatedAt: new Date().toISOString(),
+    } as any);
+
+    const executor = new TaskExecutor(store as any, "/tmp/test");
+    await (executor as any).recoverApprovedStepsOnResume("FN-6607-R");
+
+    expect(store.updateStep).toHaveBeenCalledWith("FN-6607-R", 1, "done");
+    expect(store.logEntry).toHaveBeenCalledWith(
+      "FN-6607-R",
+      expect.stringContaining("Step 1 (First) recovered as done on resume"),
+    );
+  });
+
+  it("pending-review loop detection matches 0-based writer strings", async () => {
+    const store = createMockStore();
+    const task = {
+      id: "FN-6607-P",
+      title: "Pending review",
+      description: "",
+      column: "in-progress",
+      dependencies: [],
+      taskDoneRetryCount: 2,
+      steps: [
+        { name: "Preflight", status: "done" },
+        { name: "First", status: "in-progress" },
+      ],
+      currentStep: 1,
+      log: [{ timestamp: new Date().toISOString(), action: "code review requested for Step 1 (First)" }],
+      prompt: "# test\n## Steps\n### Step 0: Preflight\n### Step 1: First",
+      createdAt: new Date().toISOString(),
+      updatedAt: new Date().toISOString(),
+    } as any;
+    store.getTask.mockResolvedValue(task);
+    mockedCreateFnAgent.mockResolvedValue({
+      session: {
+        prompt: vi.fn().mockResolvedValue(undefined),
+        dispose: vi.fn(),
+        subscribe: vi.fn(),
+        on: vi.fn(),
+        sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") },
+        state: {},
+      },
+    } as any);
+
+    const executor = new TaskExecutor(store as any, "/tmp/test");
+    await executor.execute(task);
+
+    expect(store.logEntry).toHaveBeenCalledWith(
+      "FN-6607-P",
+      expect.stringContaining("Step 1 is blocked on pending review"),
+      undefined,
+      expect.objectContaining({ agentId: "executor" }),
+    );
+    expect(store.moveTask).toHaveBeenCalledWith("FN-6607-P", "in-review");
+  });
+});
--- a/packages/engine/src/tests/executor-step-session.test.ts
+++ b/packages/engine/src/tests/executor-step-session.test.ts
@@ -224,7 +224,7 @@ describe("Workflow Steps Execution", () => {
            prompt: vi.fn().mockImplementation(async () => {
              const reviewTool = tools.find((t: any) => t.name === "fn_review_step");
              if (reviewTool) {
-                await reviewTool.execute("tool-review", { step: 1, type: "code", step_name: "Implement" });
+                await reviewTool.execute("tool-review", { step: 0, type: "code", step_name: "Implement" });
              }
            }),
            dispose: vi.fn(),
@@ -269,7 +269,7 @@ describe("Workflow Steps Execution", () => {
        dependencies: [],
        steps: [{ name: "Implement", status: "in-progress" }],
        currentStep: 0,
-        log: [{ action: "code review requested for Step 1 (Implement)", timestamp: new Date().toISOString() }],
+        log: [{ action: "code review requested for Step 0 (Implement)", timestamp: new Date().toISOString() }],
        prompt: "# test\n## Steps\n### Step 1: Implement\n- [ ] implement",
        createdAt: new Date().toISOString(),
        updatedAt: new Date().toISOString(),
@@ -314,7 +314,7 @@ describe("Workflow Steps Execution", () => {
        dependencies: [],
        steps: [{ name: "Implement", status: "in-progress" }],
        currentStep: 0,
-        log: [{ action: "code review Step 1: APPROVE", timestamp: new Date().toISOString() }],
+        log: [{ action: "code review Step 0: APPROVE", timestamp: new Date().toISOString() }],
        prompt: "# test\n## Steps\n### Step 1: Implement\n- [ ] implement",
        createdAt: new Date().toISOString(),
        updatedAt: new Date().toISOString(),
@@ -2866,12 +2866,18 @@ describe("Workflow Steps Execution", () => {
        };
        return { session };
      } else {
-        // Workflow step agent that passes (no REQUEST REVISION)
+        // Workflow step agent that passes with an explicit parseable verdict.
+        let subscribeHandler: any;
        return {
          session: {
-            prompt: vi.fn().mockResolvedValue(undefined),
+            prompt: vi.fn().mockImplementation(async () => {
+              subscribeHandler?.({
+                type: "message_update",
+                assistantMessageEvent: { type: "text_delta", delta: "Verdict: APPROVE\n\nWorkflow step passed." },
+              });
+            }),
            dispose: vi.fn(),
-            subscribe: vi.fn(),
+            subscribe: vi.fn((handler: any) => { subscribeHandler = handler; }),
            state: {},
          },
        };
@@ -3553,14 +3559,14 @@ describe("U2: fn_review_step RETHINK delegates to resetStepToBaseline (character
            const updateTool = tools.find((t: any) => t.name === "fn_task_update");
            if (updateTool) {
              try {
-                await updateTool.execute("tool-update", { step: 1, status: "in-progress" });
+                await updateTool.execute("tool-update", { step: 0, status: "in-progress" });
              } catch { /* tool param shape varies; ignore */ }
            }
            const reviewTool = tools.find((t: any) => t.name === "fn_review_step");
            if (reviewTool) {
              try {
                await reviewTool.execute("tool-review", {
-                  step: 1,
+                  step: 0,
                  type: reviewType,
                  step_name: "Implement",
                  baseline: reviewType === "code" ? "agentBaselineSHA" : undefined,
@@ -3624,7 +3630,7 @@ describe("U2: fn_review_step RETHINK delegates to resetStepToBaseline (character
    expect(store.updateStep).toHaveBeenCalledWith("FN-RT-1", 0, "pending");
    expect(store.logEntry).toHaveBeenCalledWith(
      "FN-RT-1",
-      expect.stringContaining("Step 1 plan rewound"),
+      expect.stringContaining("Step 0 plan rewound"),
      "rejected approach",
    );
  });
--- a/packages/engine/src/tests/executor-task-done-revise-verdict-guard.test.ts
+++ b/packages/engine/src/tests/executor-task-done-revise-verdict-guard.test.ts
@@ -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: 1, type: "code", step_name: "Step 1", baseline: "abc123" });
+    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");
@@ -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: 1, type: "code", step_name: "Step 1", baseline: "abc123" });
+    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");
--- a/packages/engine/src/tests/reliability-interactions/executor-pending-review-skip-retry.test.ts
+++ b/packages/engine/src/tests/reliability-interactions/executor-pending-review-skip-retry.test.ts
@@ -44,7 +44,7 @@ describe("reliability interactions: FN-5436 executor pending-review skip", () =>
    const task = makeTask({
      id: "FN-5436-RI-A",
      steps: [{ name: "Step 1", status: "done" }],
-      log: [{ action: "code review Step 1: REVISE", timestamp: new Date().toISOString() }],
+      log: [{ action: "code review Step 0: REVISE", timestamp: new Date().toISOString() }],
    });
    store.getTask.mockResolvedValue(task);

@@ -78,7 +78,7 @@ describe("reliability interactions: FN-5436 executor pending-review skip", () =>
    const task = makeTask({
      id: "FN-5436-RI-C",
      taskDoneRetryCount: 2,
-      log: [{ action: "code review requested for Step 1 (Step 1)", timestamp: new Date().toISOString() }],
+      log: [{ action: "code review requested for Step 0 (Step 1)", timestamp: new Date().toISOString() }],
    });
    store.getTask.mockResolvedValue(task);

@@ -98,7 +98,7 @@ describe("reliability interactions: FN-5436 executor pending-review skip", () =>
    const task = makeTask({
      id: "FN-5436-RI-D",
      steps: [{ name: "Step 1", status: "done" }],
-      log: [{ action: "code review Step 1: APPROVE", timestamp: new Date().toISOString() }],
+      log: [{ action: "code review Step 0: APPROVE", timestamp: new Date().toISOString() }],
    });
    store.getTask.mockResolvedValue(task);

@@ -117,7 +117,7 @@ describe("reliability interactions: FN-5436 executor pending-review skip", () =>
    const store = createMockStore();
    const task = makeTask({
      id: "FN-5436-RI-E",
-      log: [{ action: "plan review Step 1: UNAVAILABLE — proceeding advisory after fallback retry exhausted", timestamp: new Date().toISOString() }],
+      log: [{ action: "plan review Step 0: UNAVAILABLE — proceeding advisory after fallback retry exhausted", timestamp: new Date().toISOString() }],
    });
    store.getTask.mockResolvedValue(task);

--- a/packages/engine/src/tests/reliability-interactions/task-done-refusal-x-invariant.test.ts
+++ b/packages/engine/src/tests/reliability-interactions/task-done-refusal-x-invariant.test.ts
@@ -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: 1, type: "code", step_name: "S1", baseline: "abc" });
+    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);
--- a/packages/engine/src/tests/step-runner.test.ts
+++ b/packages/engine/src/tests/step-runner.test.ts
@@ -220,8 +220,8 @@ describe("resetStepToBaseline", () => {
    expect(store.updateStep).toHaveBeenCalledWith("FN-001", 2, "pending");
    expect(store.logEntry).toHaveBeenCalledWith(
      "FN-001",
-      // 0-indexed step 2 → 1-indexed "Step 3"
-      expect.stringContaining("Step 3 plan rewound"),
+      // 0-indexed step 2 is displayed as "Step 2" to match PROMPT.md headings.
+      expect.stringContaining("Step 2 plan rewound"),
      "plan rejected",
    );
  });
--- a/packages/engine/src/executor.ts
+++ b/packages/engine/src/executor.ts
@@ -434,7 +434,7 @@ function detectPendingReviewBlock(
    .filter((action): action is string => Boolean(action));

  for (const stepIndex of inProgressStepIndices) {
-    const stepDisplay = stepIndex + 1;
+    const stepDisplay = stepIndex;
    const codeRequest = `code review requested for Step ${stepDisplay}`;
    const planRequest = `plan review requested for Step ${stepDisplay}`;
    const codeVerdictPrefix = `code review Step ${stepDisplay}:`;
@@ -485,7 +485,7 @@ export function evaluateTaskDoneRefusal(
    }
    pendingSteps.push(stepIndex);
    if (codeReviewVerdicts.get(stepIndex) === "REVISE") {
-      const reason = `Step ${stepIndex + 1} (${step.name}) has a pending code review verdict of REVISE`;
+      const reason = `Step ${stepIndex} (${step.name}) has a pending code review verdict of REVISE`;
      return {
        ok: false,
        refusalClass: "pending-code-review-revise",
@@ -918,7 +918,7 @@ export async function __runConfiguredCommandForTests(
 // ── Tool parameter schemas (module-level for reuse in ToolDefinition generics) ──

 const taskUpdateParams = Type.Object({
-  step: Type.Optional(Type.Number({ description: "Step number (1-indexed). Omit when updating only custom_fields/dependencies." })),
+  step: Type.Optional(Type.Number({ description: "Step number (0-indexed; matches the `### Step N:` numbers in PROMPT.md — Step 0 is Preflight). Omit when updating only custom_fields/dependencies." })),
  status: Type.Optional(Type.Union(
    STEP_STATUSES.map((s) => Type.Literal(s)),
    { description: "New status: pending, in-progress, done, or skipped. Required when step is set." },
@@ -1093,7 +1093,7 @@ export function parseWorkflowStepOutput(rawOutput: string): {
 }

 const reviewStepParams = Type.Object({
-  step: Type.Number({ description: "Step number to review" }),
+  step: Type.Number({ description: "Step number to review (0-indexed; matches the `### Step N:` numbers in PROMPT.md — Step 0 is Preflight)." }),
  type: Type.Union(
    [Type.Literal("plan"), Type.Literal("code")],
    { description: 'Review type: "plan" or "code"' },
@@ -1150,6 +1150,7 @@ If you genuinely cannot proceed (blocked on a dependency, missing information, o
 You have tools to report progress. The board updates in real-time.

 **Step lifecycle:**
+The \`step\` argument is 0-based and equals the literal \`### Step N:\` number in PROMPT.md (Step 0 is Preflight).
 - Before starting a step: \`fn_task_update(step=N, status="in-progress")\`
 - After completing a step: \`fn_task_update(step=N, status="done")\`
 - If skipping a step: \`fn_task_update(step=N, status="skipped")\`
@@ -5476,7 +5477,7 @@ export class TaskExecutor {
        const detail = await this.store.getTask(seamTask.id);
        // Worktree isolation (KTD-11): review the instance's OWN worktree when set.
        const worktreePath = active.worktreePath || detail.worktree || this.rootDir;
-        const stepName = detail.steps[stepIndex]?.name ?? `Step ${stepIndex + 1}`;
+        const stepName = detail.steps[stepIndex]?.name ?? `Step ${stepIndex}`;
        const promptContent = detail.prompt ?? "";
        // Merge per-task effective workflow settings (U3, KTD-3) so the validator
        // model-lane reads below pick up workflow values. Behavior-inert by default.
@@ -5487,7 +5488,7 @@ export class TaskExecutor {
          reviewStep(
            worktreePath,
            seamTask.id,
-            stepIndex + 1, // reviewStep is 1-indexed (matches fn_review_step)
+            stepIndex,
            stepName,
            config.type,
            promptContent,
@@ -5533,7 +5534,7 @@ export class TaskExecutor {

        await this.store.logEntry(
          seamTask.id,
-          `${config.type} step-review Step ${stepIndex + 1}: ${review.verdict}${config.advisory ? " (advisory)" : ""}`,
+          `${config.type} step-review Step ${stepIndex}: ${review.verdict}${config.advisory ? " (advisory)" : ""}`,
          review.summary,
        );

@@ -5548,12 +5549,12 @@ export class TaskExecutor {
              await this.updateStepGraph(seamTask.id, stepIndex, "done");
              await this.store.logEntry(
                seamTask.id,
-                `Step ${stepIndex + 1} (${stepName}) marked done by step-review APPROVE (graph)`,
+                `Step ${stepIndex} (${stepName}) marked done by step-review APPROVE (graph)`,
              );
            }
          } catch (err) {
            reviewerLog.warn(
-              `${seamTask.id}: failed to mark Step ${stepIndex + 1} done after APPROVE: ${err instanceof Error ? err.message : String(err)}`,
+              `${seamTask.id}: failed to mark Step ${stepIndex} done after APPROVE: ${err instanceof Error ? err.message : String(err)}`,
            );
          }
        }
@@ -7668,8 +7669,7 @@ export class TaskExecutor {
      // Build custom tools for the worker
      // Track the last code review verdict per step so we can enforce REVISE
      // (block fn_task_update status="done" until the agent re-reviews and gets APPROVE).
-      // Keyed by 0-indexed step (stepIndex). fn_task_update translates from
-      // its 1-indexed `step` parameter via `stepIndex = step - 1` (FN-3757).
+      // Keyed by the canonical 0-indexed step number used by PROMPT.md headings.
      const codeReviewVerdicts = new Map<number, ReviewVerdict>();

      let wasPaused = false;
@@ -8294,7 +8294,7 @@ export class TaskExecutor {
                );
                await this.store.logEntry(
                  task.id,
-                  `Agent finished without calling fn_task_done but Step ${pendingReviewBlock.stepIndex + 1} is blocked on pending review (${pendingReviewBlock.reason}) — skipping retry session`,
+                  `Agent finished without calling fn_task_done but Step ${pendingReviewBlock.stepIndex} is blocked on pending review (${pendingReviewBlock.reason}) — skipping retry session`,
                  undefined,
                  this.getRunContextFor(task.id),
                );
@@ -9488,17 +9488,21 @@ export class TaskExecutor {
          };
        }

-        if (!Number.isInteger(step) || step < 1) {
+        if (!Number.isInteger(step) || step < 0) {
          return {
            content: [{
              type: "text" as const,
-              text: `Invalid step number: ${step}. Steps are 1-indexed.`,
+              text: `Invalid step number: ${step}. Steps are 0-indexed; Step 0 is Preflight.`,
            }],
            details: {},
          };
        }

-        const stepIndex = step - 1;
+        /*
+         * FNXC:StepNumbering 2026-06-17-00:00:
+         * FN-6607 makes fn_task_update.step the same 0-based number agents see in PROMPT.md (`### Step N:`) and TaskStore.updateStep uses internally. The prior `step - 1` conversion made Step 0 impossible to mark done and shifted every review/progress update one array slot early.
+         */
+        const stepIndex = step;

        if (status === "in-progress") {
          try {
@@ -9508,7 +9512,7 @@ export class TaskExecutor {
            );
            if (otherInProgressStepIndex !== -1) {
              executorLog.warn(
-                `${taskId}: fn_task_update marking step ${step} in-progress while step ${otherInProgressStepIndex + 1} is already in-progress`,
+                `${taskId}: fn_task_update marking step ${step} in-progress while step ${otherInProgressStepIndex} is already in-progress`,
              );
            }
          } catch (err) {
@@ -9519,7 +9523,7 @@ export class TaskExecutor {
        // Enforce code review REVISE: block advancing to "done" when the last
        // code review for this step returned REVISE. The agent must fix the
        // issues and call fn_review_step(type="code") again before proceeding.
-        // FN-3757: verdict/checkpoint maps are keyed by 0-indexed stepIndex.
+        // FN-6607: verdict/checkpoint maps are keyed directly by the 0-indexed tool step.
        if (status === "done" && codeReviewVerdicts.get(stepIndex) === "REVISE") {
          return {
            content: [{
@@ -9576,7 +9580,7 @@ export class TaskExecutor {
          return {
            content: [{
              type: "text" as const,
-              text: `Invalid step number: ${step}. This task has ${task.steps.length} step(s) (1-indexed).`,
+              text: `Invalid step number: ${step}. This task has ${task.steps.length} step(s) (0-indexed; valid range 0-${Math.max(0, task.steps.length - 1)}).`,
            }],
            details: {},
          };
@@ -9596,7 +9600,7 @@ export class TaskExecutor {
        ) {
          const leafId = sessionRef.current.sessionManager.getLeafId();
          if (leafId) {
-            // FN-3757: verdict/checkpoint maps are keyed by 0-indexed stepIndex.
+            // FN-6607: verdict/checkpoint maps are keyed directly by the 0-indexed tool step.
            stepCheckpoints.set(stepIndex, leafId);
          }
        }
@@ -10487,18 +10491,16 @@ 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 stepIndex = step;
        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) {
+        if (!Number.isInteger(step) || step < 0 || step >= 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.` }],
+            content: [{ type: "text" as const, text: `Invalid step ${step}. Task has ${taskSteps.length} step(s) and fn_review_step is 0-indexed; Step 0 is Preflight.` }],
            details: {
              error: "invalid_step",
              step,
-              maxStep: taskSteps.length,
+              maxStep: taskSteps.length > 0 ? taskSteps.length - 1 : -1,
            },
          };
        }
@@ -10590,7 +10592,8 @@ export class TaskExecutor {
          stuckDetector?.recordProgress(taskId);

          // Track code review verdicts for enforcement. Plan reviews remain
-          // advisory — only code reviews write to the verdict map.
+          // advisory — only code reviews write to the verdict map. FN-6607 keeps
+          // the map keyed by the same 0-indexed `step` value the tool receives.
          if (reviewType === "code") {
            if (result.verdict === "REVISE") {
              codeReviewVerdicts.set(stepIndex, "REVISE");
@@ -15103,7 +15106,7 @@ You are running in an **isolated git worktree**. This means:
 ${hasProgress
    ? `Resume from Step ${task.currentStep}. Do NOT redo completed steps.`
    : "Start with Step 0 (Preflight). Work through each step in order."}
-Use \`fn_task_update\` to report progress on every step transition.
+Use \`fn_task_update\` to report progress on every step transition; its \`step\` value is 0-based and equals the \`### Step N:\` number in PROMPT.md.
 Use \`fn_task_log\` for important actions and decisions.
 Use \`fn_task_create\` for truly separate follow-up work, including unrelated/pre-existing broad-suite failures.
 Commit at step boundaries: \`git commit -m "feat(${task.id}): complete Step N — <short summary>"${sourceIssueRef ? ` -m "Ref: ${sourceIssueRef}"` : ""}${authorArg}\`
--- a/packages/engine/src/reviewer.ts
+++ b/packages/engine/src/reviewer.ts
@@ -115,6 +115,9 @@ export interface ReviewOptions {

 /**
 * Spawn a reviewer agent to evaluate a worker's plan or code for a step.
+ *
+ * FNXC:StepNumbering 2026-06-17-00:00:
+ * `stepNumber` is display-only and must remain the same 0-based number shown in PROMPT.md (`### Step N:`). Review prompts, task logs, resume reconciliation, and loop-detection all compare this literal Step N string.
 */
 export async function reviewStep(
  cwd: string,
--- a/packages/engine/src/step-runner.ts
+++ b/packages/engine/src/step-runner.ts
@@ -238,7 +238,11 @@ export async function resetStepToBaseline(
  const { store, worktreePath, sessionRef } = deps;
  const reviewType = deps.reviewType ?? "code";
  const taskId = task.id;
-  const step = stepIndex + 1; // legacy log lines are 1-indexed
+  const step = stepIndex;
+  /*
+   * FNXC:StepReset 2026-06-17-00:00:
+   * RETHINK reset logs use the same 0-based Step N as fn_review_step and PROMPT.md so recovery tooling can correlate review verdicts, checkpoints, and reset events without off-by-one translation.
+   */

  // ── KTD-2 blast-radius guard — assert BEFORE mutating anything. ──────────
  if (deps.blastRadiusGuard) {