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 <noreply@runfusion.ai>
This commit is contained in:
7
.changeset/fn-142-prompt-write-read-back.md
Normal file
7
.changeset/fn-142-prompt-write-read-back.md
Normal file
@@ -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.
|
||||
@@ -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.
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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<typeof import("@fusion/core")>());
|
||||
return createEngineCoreMock(() => importOriginal<typeof import("@fusion/core")>(), { 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", () => {
|
||||
|
||||
@@ -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<any> }) {
|
||||
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");
|
||||
});
|
||||
});
|
||||
@@ -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<ReturnType<TaskStore["getTask"]>>;
|
||||
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");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user