fix(workflows): address workflow node PR feedback
This commit is contained in:
@@ -16,7 +16,7 @@ execution: code
|
||||
| Field | Value |
|
||||
|---|---|
|
||||
| Objective | Refactor workflow lifecycle logic out of monolithic triage, executor, and reviewer files into well-defined workflow node runner modules/classes that call runtime primitives through explicit dependencies. |
|
||||
| Authority | User request in this session: assess and plan whether workflow logic can be moved from monolithic triage/executor/reviewer files into well-defined node classes. Existing product direction says workflow policy belongs in workflow nodes and runtime primitives while engine substrate keeps hard safety invariants. |
|
||||
| Authority | Architecture decision: decouple workflow node behavior from monolithic triage/executor/reviewer files into workflow-owned node runners and runtime primitive/service boundaries while engine substrate keeps hard safety invariants. |
|
||||
| Execution profile | Deep architecture refactor across `@fusion/engine` workflow dispatch, runtime primitive adapters, custom node execution, reviewer invocation, triage planning review, and focused parity tests. |
|
||||
| Stop conditions | Do not change workflow behavior while extracting boundaries. Do not weaken worktree, file-scope, merge-proof, pause/abort, semaphore, active-session, or self-healing invariants. Do not add arbitrary workflow-definition code execution. |
|
||||
| Tail ownership | Ship in small slices with characterization tests first. Add a changeset only when behavior of published `@runfusion/fusion` changes; pure internal refactor slices do not need one. |
|
||||
|
||||
@@ -202,6 +202,98 @@ describe("fast mode workflow/runtime invariants", () => {
|
||||
expect(seams.merge).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("raw fast mode skips skill executor nodes when primitives are unavailable", async () => {
|
||||
const runCustomNode = vi.fn(async () => ({ outcome: "success", value: "ran-skill" }));
|
||||
const runner = new WorkflowGraphTaskRunner({
|
||||
store: {
|
||||
getTaskWorkflowSelection: () => ({ workflowId: "WF-fast-skill", stepIds: [] }),
|
||||
getWorkflowDefinition: vi.fn(async () => ({
|
||||
id: "WF-fast-skill",
|
||||
name: "Fast skill",
|
||||
description: "custom skill workflow",
|
||||
kind: "workflow",
|
||||
layout: {},
|
||||
createdAt: now,
|
||||
updatedAt: now,
|
||||
ir: {
|
||||
version: "v1",
|
||||
name: "Fast skill",
|
||||
nodes: [
|
||||
{ id: "start", kind: "start" },
|
||||
{ id: "skill-review", kind: "prompt", config: { executor: "skill", skillName: "compound-engineering:ce-code-review" } },
|
||||
{ id: "end", kind: "end" },
|
||||
],
|
||||
edges: [
|
||||
{ from: "start", to: "skill-review" },
|
||||
{ from: "skill-review", to: "end", condition: "success" },
|
||||
],
|
||||
},
|
||||
})),
|
||||
},
|
||||
seams: {
|
||||
planning: vi.fn(async () => ({ outcome: "success", value: "planned" })),
|
||||
execute: vi.fn(async () => ({ outcome: "success", value: "implemented" })),
|
||||
review: vi.fn(async () => ({ outcome: "success", value: "approved" })),
|
||||
merge: vi.fn(async () => ({ outcome: "success", value: "merged" })),
|
||||
schedule: vi.fn(async () => ({ outcome: "success", value: "scheduled" })),
|
||||
},
|
||||
runCustomNode,
|
||||
});
|
||||
|
||||
const result = await runner.run(task({ executionMode: "fast" }), { experimentalFeatures: { workflowGraphExecutor: true } });
|
||||
|
||||
expect(result.disposition).toBe("completed");
|
||||
expect(result.visitedNodeIds).toEqual(["start", "skill-review"]);
|
||||
expect(runCustomNode).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("raw fast mode still invokes non-executable review seam nodes", async () => {
|
||||
const review = vi.fn(async () => ({ outcome: "success" as const, value: "approved" }));
|
||||
const runCustomNode = vi.fn(async () => ({ outcome: "failure" as const, value: "unexpected-custom-node" }));
|
||||
const runner = new WorkflowGraphTaskRunner({
|
||||
store: {
|
||||
getTaskWorkflowSelection: () => ({ workflowId: "WF-fast-review-seam", stepIds: [] }),
|
||||
getWorkflowDefinition: vi.fn(async () => ({
|
||||
id: "WF-fast-review-seam",
|
||||
name: "Fast review seam",
|
||||
description: "custom review seam workflow",
|
||||
kind: "workflow",
|
||||
layout: {},
|
||||
createdAt: now,
|
||||
updatedAt: now,
|
||||
ir: {
|
||||
version: "v1",
|
||||
name: "Fast review seam",
|
||||
nodes: [
|
||||
{ id: "start", kind: "start" },
|
||||
{ id: "review", kind: "prompt", config: { seam: "review" } },
|
||||
{ id: "end", kind: "end" },
|
||||
],
|
||||
edges: [
|
||||
{ from: "start", to: "review" },
|
||||
{ from: "review", to: "end", condition: "success" },
|
||||
],
|
||||
},
|
||||
})),
|
||||
},
|
||||
seams: {
|
||||
planning: vi.fn(async () => ({ outcome: "success", value: "planned" })),
|
||||
execute: vi.fn(async () => ({ outcome: "success", value: "implemented" })),
|
||||
review,
|
||||
merge: vi.fn(async () => ({ outcome: "success", value: "merged" })),
|
||||
schedule: vi.fn(async () => ({ outcome: "success", value: "scheduled" })),
|
||||
},
|
||||
runCustomNode,
|
||||
});
|
||||
|
||||
const result = await runner.run(task({ executionMode: "fast" }), { experimentalFeatures: { workflowGraphExecutor: true } });
|
||||
|
||||
expect(result.disposition).toBe("completed");
|
||||
expect(result.visitedNodeIds).toEqual(["start", "review"]);
|
||||
expect(review).toHaveBeenCalledTimes(1);
|
||||
expect(runCustomNode).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("fast builtin:coding executes explicitly selected optional-group template nodes", async () => {
|
||||
const calls: string[] = [];
|
||||
const prompt = "# Task\n\n## Steps\n\n### Step 1: Do the work\n- [ ] edit files";
|
||||
|
||||
@@ -7,6 +7,7 @@ import {
|
||||
handlerBackedRunner,
|
||||
type WorkflowNodeRunner,
|
||||
} from "../workflow-node-runner.js";
|
||||
import { createMergeAttemptHandler } from "../workflow-node-runners/merge-runner.js";
|
||||
|
||||
const task = { id: "FN-7300" } as TaskDetail;
|
||||
|
||||
@@ -102,4 +103,23 @@ describe("WorkflowNodeRunnerRegistry", () => {
|
||||
expect(result.context["node:script:value"]).toBe("handler-backed");
|
||||
expect(handler).toHaveBeenCalledWith(scriptNode, expect.objectContaining({ task }));
|
||||
});
|
||||
|
||||
it("delegates merge-attempt to the legacy merge seam when primitives are unwired", async () => {
|
||||
const merge = vi.fn(async () => ({ outcome: "success" as const, value: "legacy-merged" }));
|
||||
const handler = createMergeAttemptHandler({
|
||||
seams: { merge },
|
||||
buildPrimitiveContext: vi.fn(),
|
||||
});
|
||||
const node: WorkflowIrNode = { id: "merge-attempt", kind: "merge-attempt" };
|
||||
const context = { branch: "main" };
|
||||
|
||||
const result = await handler(node, {
|
||||
task,
|
||||
settings: settingsOn(),
|
||||
context,
|
||||
});
|
||||
|
||||
expect(result).toEqual({ outcome: "success", value: "legacy-merged" });
|
||||
expect(merge).toHaveBeenCalledWith(task, context);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -257,11 +257,14 @@ export class WorkflowGraphTaskRunner {
|
||||
|
||||
FNXC:WorkflowFastMode 2026-07-01-00:00:
|
||||
Explicitly selected optional-group bodies carry workflow:optionalGroupActive and must still execute in fast mode because selecting the optional workflow step is operator intent, not default built-in review behavior.
|
||||
|
||||
FNXC:WorkflowFastMode 2026-07-01-00:00:
|
||||
Skill executor nodes use `config.skillName`, not `config.skill`; include that field in executable-node detection so raw fast-mode compatibility skips skill prompts the same way it skips prompt/script nodes.
|
||||
*/
|
||||
const hasExecutableConfig =
|
||||
typeof node.config?.prompt === "string" ||
|
||||
typeof node.config?.scriptName === "string" ||
|
||||
typeof node.config?.skill === "string";
|
||||
typeof node.config?.skillName === "string";
|
||||
const isExplicitOptionalGroupNode = typeof c[WORKFLOW_OPTIONAL_GROUP_CONTEXT_KEY] === "string";
|
||||
if (hasExecutableConfig && !isExplicitOptionalGroupNode) {
|
||||
return Promise.resolve({ outcome: "success", value: "fast-mode-skipped" });
|
||||
@@ -295,10 +298,13 @@ export class WorkflowGraphTaskRunner {
|
||||
/*
|
||||
FNXC:WorkflowFastMode 2026-07-01-00:00:
|
||||
Raw legacy-seam graph runs do not have TaskExecutor's parse-steps dependencies. For fast-mode compatibility, parse the task prompt from memory and project the parsed steps back onto the runner task so the stepwise built-in can reach the seam-backed lifecycle suffix without a store-backed projection.
|
||||
|
||||
FNXC:WorkflowFastMode 2026-07-01-00:00:
|
||||
Parse-step projection must write through the task object supplied by the graph node context, not a closed-over outer run argument. Raw fallback callers usually pass the same object today, but the dependency contract belongs to ParseStepsNodeRunner's write target.
|
||||
*/
|
||||
readArtifact: async (_task, key) => (key === "PROMPT.md" ? task.prompt : undefined),
|
||||
writeSteps: async (_task, steps: TaskStep[]) => {
|
||||
task.steps = steps;
|
||||
writeSteps: async (target, steps: TaskStep[]) => {
|
||||
target.steps = steps;
|
||||
},
|
||||
}
|
||||
: undefined;
|
||||
|
||||
Reference in New Issue
Block a user