From d42a60ea0607973cd9fc0a1eb2b04c03b26c3dde Mon Sep 17 00:00:00 2001 From: Fusion Agent Date: Sat, 22 Aug 2026 03:57:19 +0000 Subject: [PATCH] FN-142: fix plan confirmation read-back and no-op routing Make plan persistence confirmations observe the filesystem-backed prompt and let valid no-op reviews terminate cleanly.\n\n- Verify PROMPT.md by querying the task after writes across planning and review surfaces.\n- Add coverage for workspace scope, duplicate plans, failed read-backs, and mirror failures.\n- Route CLOSE_NO_OP Plan Review outcomes through an explicit terminal workflow action.\n- Document the persistence failure mode and publish a patch changeset.\n\nFiles changed:\n .changeset/fn-142-prompt-write-read-back.md | 7 ++\n .../prompt-write-read-back-queries-the-task-row.md | 51 +++++++++++\n .../core/src/__tests__/builtin-workflows.test.ts | 22 +++++\n packages/core/src/workflows/builtin-workflows.ts | 12 +++\n .../src/__tests__/agent-document-tools.test.ts | 101 ++++++++++++++++++++-\n .../__tests__/plan-prompt-write-surfaces.test.ts | 77 +++++++++++++++\n packages/engine/src/agent-tools.ts | 15 ++-\n 7 files changed, 283 insertions(+), 2 deletions(-) Fusion-Task-Id: FN-142 Fusion-Task-Lineage: adddffb3-8786-4372-996a-eefef176f31b Co-authored-by: Fusion --- .changeset/fn-142-prompt-write-read-back.md | 7 ++ ...pt-write-read-back-queries-the-task-row.md | 51 +++++++++ .../src/__tests__/builtin-workflows.test.ts | 22 ++++ .../core/src/workflows/builtin-workflows.ts | 12 +++ .../__tests__/agent-document-tools.test.ts | 101 +++++++++++++++++- .../plan-prompt-write-surfaces.test.ts | 77 +++++++++++++ packages/engine/src/agent-tools.ts | 15 ++- 7 files changed, 283 insertions(+), 2 deletions(-) create mode 100644 .changeset/fn-142-prompt-write-read-back.md create mode 100644 docs/solutions/logic-errors/prompt-write-read-back-queries-the-task-row.md create mode 100644 packages/engine/src/__tests__/plan-prompt-write-surfaces.test.ts diff --git a/.changeset/fn-142-prompt-write-read-back.md b/.changeset/fn-142-prompt-write-read-back.md new file mode 100644 index 0000000000..b96ae02bcb --- /dev/null +++ b/.changeset/fn-142-prompt-write-read-back.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Restore reliable plan-save confirmations so planning no longer loops. +category: fix +dev: createTaskPromptWriteTool now verifies PROMPT.md through a post-write getTask read-back. diff --git a/docs/solutions/logic-errors/prompt-write-read-back-queries-the-task-row.md b/docs/solutions/logic-errors/prompt-write-read-back-queries-the-task-row.md new file mode 100644 index 0000000000..7d23977ef9 --- /dev/null +++ b/docs/solutions/logic-errors/prompt-write-read-back-queries-the-task-row.md @@ -0,0 +1,51 @@ +--- +title: Prompt write read-back queries the task row +date: 2026-08-22 +category: logic-errors +module: packages/engine/src/agent-tools.ts +problem_type: persistence_verification +symptoms: [prompt-write-false-failure, infinite-replanning] +applies_when: "A fail-closed mutation check compares a field stored outside the returned database row." +--- + +# Prompt write read-back queries the task row + +## Symptom + +Every `fn_task_prompt_write` call reported that the authoritative `PROMPT.md` +read-back could not be verified, even though the plan was immediately readable. +Planners correctly refuse to finish until the tool confirms persistence, so each +failed session was recovered as `needs-replan` and started another planning pass. + +## Root cause + +FN-094 added workspace repository-scope publication to the prompt-write path. +It retained the pre-write `getTask` needed for scope validation, then changed the +post-write check to inspect `updateTask`'s return value. That value is a +`project.tasks` row. `prompt` is filesystem-only: it is written to `PROMPT.md` +and only hydrated by `getTaskImpl`. The returned row therefore cannot prove the +write and has no `prompt` value to compare. + +## Fix + +After `updateTask` completes its single prompt-plus-scope mutation, read the +task again with `store.getTask(taskId)` and compare the hydrated prompt byte for +byte with the requested content. Missing, empty, changed, or unreadable +read-backs remain errors. Mirroring the verified plan to the project database +continues to be best-effort and occurs only afterwards. + +## Rule + +A mutation return value is not a read-back when the mutated field lives outside +that row. Repair fail-closed gates by correcting what they observe, never by +weakening their strictness. A false failure at a planning confirmation boundary +can amplify into an infinite replanning loop because the planner must treat an +unverified artifact as unsafe. + +## Verification + +- `packages/engine/src/__tests__/agent-document-tools.test.ts` covers exact, + duplicate, workspace, missing, empty, altered, rejecting, and mirror-failure + states. +- `packages/engine/src/__tests__/plan-prompt-write-surfaces.test.ts` covers + triage/replanning, Plan Review, and reviewer-inline registration reachability. diff --git a/packages/core/src/__tests__/builtin-workflows.test.ts b/packages/core/src/__tests__/builtin-workflows.test.ts index ea79d3db22..d3a6ae57a3 100644 --- a/packages/core/src/__tests__/builtin-workflows.test.ts +++ b/packages/core/src/__tests__/builtin-workflows.test.ts @@ -62,6 +62,26 @@ describe("built-in workflows", () => { } }); + /* + FNXC:PlanReviewNoOp 2026-08-22-03:37: + Every built-in that offers Plan Review must route CLOSE_NO_OP to the terminal no-op action. + A custom workflow without this route fails closed by holding at Plan Review; it never restarts planning. + */ + it("routes Plan Review CLOSE_NO_OP verdicts to the terminal no-op action", () => { + for (const workflow of BUILTIN_WORKFLOWS) { + const planReview = workflow.ir.nodes.find((node) => node.id === "plan-review"); + if (!planReview) continue; + expect( + workflow.ir.edges.some((edge) => + edge.from === planReview.id + && edge.condition === "outcome:close-no-op" + && workflow.ir.nodes.find((node) => node.id === edge.to)?.config?.workflowAction === "plan-review-no-op", + ), + workflow.id, + ).toBe(true); + } + }); + it("all built-ins expose workflow-native review revision cap settings", () => { for (const workflow of BUILTIN_WORKFLOWS) { if (workflow.kind === "fragment") continue; @@ -816,6 +836,7 @@ describe("built-in workflows", () => { "plan-replan", "browser-verification-remediation", "code-review-remediation", + "plan-review-no-op", ]); const execute = design!.ir.nodes.find((node) => node.id === "execute"); @@ -1066,6 +1087,7 @@ describe("built-in workflows", () => { "plan-replan", "browser-verification-remediation", "code-review-remediation", + "plan-review-no-op", ]); expect(ce.ir.nodes.some((node) => node.config?.seam === "review")).toBe(false); diff --git a/packages/core/src/workflows/builtin-workflows.ts b/packages/core/src/workflows/builtin-workflows.ts index 8f39fb0847..d410a2699a 100644 --- a/packages/core/src/workflows/builtin-workflows.ts +++ b/packages/core/src/workflows/builtin-workflows.ts @@ -357,10 +357,20 @@ function linear(spec: BuiltinSpec): WorkflowDefinition { ...(hasCodeReview ? [codeReviewRemediationNode("in-progress")] : []), ] : []; + /* + * FNXC:PlanReviewNoOp 2026-08-22-03:37: + * Optional Plan Review is available on linear built-ins too. CLOSE_NO_OP needs an explicit + * terminal action rather than falling through the ordinary success path, or duplicate plans + * hold indefinitely despite a valid reviewer verdict. + */ + const noOpTerminalNode: WorkflowIrNode | undefined = hasPlanReview + ? { id: "plan-review-no-op", kind: "gate", column: "todo", config: { workflowAction: "plan-review-no-op" } } + : undefined; const nodes: WorkflowIr["nodes"] = [ { id: "start", kind: "start" }, ...workflowNodes, ...remediationNodes, + ...(noOpTerminalNode ? [noOpTerminalNode] : []), { id: "end", kind: "end" }, ]; const edges: WorkflowIr["edges"] = []; @@ -388,6 +398,8 @@ function linear(spec: BuiltinSpec): WorkflowDefinition { */ if (hasPlanReview) { edges.push({ from: "plan-replan", to: "plan-review", condition: "success", kind: "rework" }); + edges.push({ from: "plan-review", to: "plan-review-no-op", condition: "outcome:close-no-op" }); + edges.push({ from: "plan-review-no-op", to: "end", condition: "success" }); } if (hasBrowserVerification) { edges.push({ diff --git a/packages/engine/src/__tests__/agent-document-tools.test.ts b/packages/engine/src/__tests__/agent-document-tools.test.ts index 0a192a63c3..0c2f79fea0 100644 --- a/packages/engine/src/__tests__/agent-document-tools.test.ts +++ b/packages/engine/src/__tests__/agent-document-tools.test.ts @@ -1,5 +1,9 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { TaskDocumentPreconditionFailedError, type TaskDocument, type TaskStore } from "@fusion/core"; + +const { loadWorkspaceConfig } = vi.hoisted(() => ({ + loadWorkspaceConfig: vi.fn(), +})); import { createChatTaskDocumentTools, createTaskDocumentReadTool, @@ -9,7 +13,7 @@ import { vi.mock("@fusion/core", async (importOriginal) => { const { createEngineCoreMock } = await import("../test/mockCore.js"); - return createEngineCoreMock(() => importOriginal()); + return createEngineCoreMock(() => importOriginal(), { loadWorkspaceConfig }); }); const TASK_ID = "FN-1272"; @@ -184,6 +188,11 @@ describe("task_document_write tool", () => { }); describe("task_prompt_write tool", () => { + beforeEach(() => { + vi.clearAllMocks(); + loadWorkspaceConfig.mockResolvedValue(undefined); + }); + it("reports success only after the authoritative store reads back the exact prompt", async () => { const updateTask = vi.fn().mockResolvedValue({}); const getTask = vi.fn().mockResolvedValue({ id: TASK_ID, prompt: "# Verified plan" }); @@ -210,6 +219,96 @@ describe("task_prompt_write tool", () => { expect(getText(result)).toContain("ERROR:"); expect(getText(result)).toContain("could not be verified"); }); + + it("reads the authoritative prompt only after the promptless row write resolves", async () => { + const calls: string[] = []; + let written = false; + const updateTask = vi.fn().mockImplementation(async () => { + calls.push("updateTask"); + written = true; + return { id: TASK_ID }; + }); + const getTask = vi.fn().mockImplementation(async () => { + calls.push("getTask"); + return written ? { id: TASK_ID, prompt: "# Verified plan" } : { id: TASK_ID }; + }); + const store = { updateTask, getTask } as unknown as TaskStore; + + const result = await runTool(createTaskPromptWriteTool(store, TASK_ID), "call-order", { content: "# Verified plan" }); + + expect(calls).toEqual(["getTask", "updateTask", "getTask"]); + expect(getText(result)).toBe(`Updated PROMPT.md for ${TASK_ID}.`); + }); + + it("confirms a duplicate verdict prompt when the artifact reads back exactly", async () => { + const content = "DUPLICATE: FN-1672"; + const store = { + updateTask: vi.fn().mockResolvedValue({ id: TASK_ID }), + getTask: vi.fn().mockResolvedValue({ id: TASK_ID, prompt: content }), + } as unknown as TaskStore; + + const result = await runTool(createTaskPromptWriteTool(store, TASK_ID), "call-duplicate", { content }); + + expect(getText(result)).toBe(`Updated PROMPT.md for ${TASK_ID}.`); + }); + + it.each([ + ["missing", null], + ["empty", { id: TASK_ID, prompt: "" }], + ["altered", { id: TASK_ID, prompt: "# Truncated" }], + ])("fails closed when the post-write artifact is %s", async (_state, readBack) => { + const store = { + updateTask: vi.fn().mockResolvedValue({ id: TASK_ID }), + getTask: vi.fn().mockResolvedValue(readBack), + } as unknown as TaskStore; + + const result = await runTool(createTaskPromptWriteTool(store, TASK_ID), "call-unverified", { content: "# Complete plan" }); + + expect(getText(result)).toContain("ERROR:"); + expect(getText(result)).toContain("could not be verified"); + expect(getText(result)).not.toContain(`Updated PROMPT.md for ${TASK_ID}.`); + }); + + it("fails closed when the authoritative read-back rejects", async () => { + const getTask = vi.fn() + .mockResolvedValueOnce({ id: TASK_ID }) + .mockRejectedValueOnce(new Error(`Task ${TASK_ID} not found`)); + const store = { updateTask: vi.fn().mockResolvedValue({ id: TASK_ID }), getTask } as unknown as TaskStore; + + const result = await runTool(createTaskPromptWriteTool(store, TASK_ID), "call-reject", { content: "# Complete plan" }); + + expect(getText(result)).toContain("ERROR:"); + expect(getText(result)).toContain("could not be verified"); + }); + + it("confirms a workspace prompt after atomically publishing its validated repository scope", async () => { + const content = "## Repository Scope\n- `packages/engine`\n\n# Workspace plan"; + const updateTask = vi.fn().mockResolvedValue({ id: TASK_ID }); + const getTask = vi.fn().mockResolvedValue({ id: TASK_ID, prompt: content }); + loadWorkspaceConfig.mockResolvedValue({ repos: ["packages/engine"] }); + const store = { updateTask, getTask, getRootDir: () => "/workspace" } as unknown as TaskStore; + + const result = await runTool(createTaskPromptWriteTool(store, TASK_ID), "call-workspace", { content }); + + expect(updateTask).toHaveBeenCalledWith(TASK_ID, expect.objectContaining({ + prompt: content, + repositoryScope: expect.objectContaining({ repositories: ["packages/engine"], state: "confirmed" }), + }), undefined); + expect(getText(result)).toBe(`Updated PROMPT.md for ${TASK_ID}.`); + }); + + it("keeps a verified prompt write successful when the plan mirror fails", async () => { + const content = "# Verified plan"; + const store = { + updateTask: vi.fn().mockResolvedValue({ id: TASK_ID }), + getTask: vi.fn().mockResolvedValue({ id: TASK_ID, prompt: content }), + upsertTaskDocument: vi.fn().mockRejectedValue(new Error("database unavailable")), + } as unknown as TaskStore; + + const result = await runTool(createTaskPromptWriteTool(store, TASK_ID), "call-mirror-failure", { content }); + + expect(getText(result)).toBe(`Updated PROMPT.md for ${TASK_ID}.`); + }); }); describe("task_document_read tool", () => { diff --git a/packages/engine/src/__tests__/plan-prompt-write-surfaces.test.ts b/packages/engine/src/__tests__/plan-prompt-write-surfaces.test.ts new file mode 100644 index 0000000000..3374fb47d6 --- /dev/null +++ b/packages/engine/src/__tests__/plan-prompt-write-surfaces.test.ts @@ -0,0 +1,77 @@ +import { readFile } from "node:fs/promises"; + +import { type RunMutationContext, type TaskStore } from "@fusion/core"; +import { describe, expect, it, vi } from "vitest"; +import { createTaskPromptWriteTool as createTriagePromptWriteTool } from "../agent-tools.js"; +import { createTaskPromptWriteTool as createPlanReviewPromptWriteTool } from "../executor/shared-worker-tools.js"; + +const TASK_ID = "FN-142"; +const CONTENT = "# Verified plan"; + +async function runTool(tool: { execute: (...args: any[]) => Promise }) { + return tool.execute("call-prompt", { content: CONTENT }, undefined, undefined, undefined); +} + +function getText(result: any): string { + const first = result?.content?.[0]; + return first?.type === "text" ? first.text : ""; +} + +function createProductionShapedStore() { + const updateTask = vi.fn().mockResolvedValue({ id: TASK_ID }); + const getTask = vi.fn().mockResolvedValue({ id: TASK_ID, prompt: CONTENT }); + return { + store: { updateTask, getTask } as unknown as TaskStore, + updateTask, + getTask, + }; +} + +describe("planning prompt-write surfaces", () => { + /* + FNXC:PlanArtifactPersistence 2026-08-22-03:37: + Initial planning, replanning, Plan Review repair, and reviewer inline repair must all retain the + same fail-closed PROMPT.md read-back. Their task-row mutation result intentionally has no prompt. + */ + it("confirms initial triage and replanning writes through the production factory with its run context", async () => { + const { store, updateTask } = createProductionShapedStore(); + const runContext = { agentId: "triage-agent", runId: "run-142" } as RunMutationContext; + + const result = await runTool(createTriagePromptWriteTool(store, TASK_ID, runContext)); + + expect(updateTask).toHaveBeenCalledWith(TASK_ID, { prompt: CONTENT }, runContext); + expect(getText(result)).toBe(`Updated PROMPT.md for ${TASK_ID}.`); + }); + + it("confirms Plan Review repair writes through the shared worker registration", async () => { + const { store, updateTask } = createProductionShapedStore(); + const runContext = { agentId: "review-agent", runId: "run-143" } as RunMutationContext; + const deps = { store, getRunContextFor: vi.fn().mockReturnValue(runContext) } as any; + + const result = await runTool(createPlanReviewPromptWriteTool(deps, TASK_ID)); + + expect(deps.getRunContextFor).toHaveBeenCalledWith(TASK_ID); + expect(updateTask).toHaveBeenCalledWith(TASK_ID, { prompt: CONTENT }, runContext); + expect(getText(result)).toBe(`Updated PROMPT.md for ${TASK_ID}.`); + }); + + it("keeps reviewer inline repair wired to the same production prompt-write factory", async () => { + const reviewerSource = await readFile(new URL("../execution/reviewer.ts", import.meta.url), "utf8"); + const { store } = createProductionShapedStore(); + + expect(reviewerSource).toContain("createTaskPromptWriteTool(options.store, options.taskId)"); + expect(getText(await runTool(createTriagePromptWriteTool(store, TASK_ID)))).toBe(`Updated PROMPT.md for ${TASK_ID}.`); + }); + + it("never treats a promptless updateTask row as verification evidence", async () => { + const updateTask = vi.fn().mockResolvedValue({ id: TASK_ID }); + const getTask = vi.fn().mockResolvedValue(null); + const store = { updateTask, getTask } as unknown as TaskStore; + + const result = await runTool(createTriagePromptWriteTool(store, TASK_ID)); + + expect(updateTask).toHaveBeenCalledTimes(1); + expect(getTask).toHaveBeenCalledTimes(2); + expect(getText(result)).toContain("could not be verified"); + }); +}); diff --git a/packages/engine/src/agent-tools.ts b/packages/engine/src/agent-tools.ts index 8c8af1bf87..a05dc7cd85 100644 --- a/packages/engine/src/agent-tools.ts +++ b/packages/engine/src/agent-tools.ts @@ -2202,10 +2202,23 @@ export function createTaskPromptWriteTool(store: TaskStore, taskId: string, runC extensions: current?.repositoryScope?.extensions, } : undefined; - const persisted = await store.updateTask(taskId, { + await store.updateTask(taskId, { prompt: params.content, ...(repositoryScope ? { repositoryScope } : {}), }, runContext); + /* + FNXC:PlanArtifactPersistence 2026-08-22-03:37: + FN-094 repointed this fail-closed check at updateTask's task-row return value after adding + workspace-scope publication. The row has no prompt column, so it can never verify PROMPT.md. + Re-read through getTask after the write because it hydrates the filesystem-backed artifact; + missing, unreadable, or changed content must still reject publication. + */ + let persisted: Awaited>; + try { + persisted = await store.getTask(taskId); + } catch { + throw new Error("authoritative PROMPT.md read-back did not match the requested content; persistence could not be verified"); + } if (persisted?.prompt !== params.content) { throw new Error("authoritative PROMPT.md read-back did not match the requested content; persistence could not be verified"); }