diff --git a/.changeset/u8-primitive-exit-events.md b/.changeset/u8-primitive-exit-events.md new file mode 100644 index 0000000000..de8ec89195 --- /dev/null +++ b/.changeset/u8-primitive-exit-events.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Report how an implementation session ended on the code path the engine actually runs. +category: fix +dev: `createDefaultNodeHandlers` prefers `createPrimitivePromptLikeHandler` whenever `deps.primitives` is set, and `executeWorkflowGraph` always sets it — so `createAuthoritativeWorkflowSeams.execute` is unreachable for prompt nodes. The `NodeCompleted.exit` announcement was wired only there and never fired; it now emits from `runCodingSession`. Adds a ratchet asserting the primitives-preferred dispatch rule. diff --git a/packages/engine/src/__tests__/executor-primitive-exit-events.test.ts b/packages/engine/src/__tests__/executor-primitive-exit-events.test.ts new file mode 100644 index 0000000000..0de858740b --- /dev/null +++ b/packages/engine/src/__tests__/executor-primitive-exit-events.test.ts @@ -0,0 +1,135 @@ +/* +FNXC:WorkflowExecutionOwnership 2026-07-29-16:40 (U8 / R4, R5, R12): + +The exit announcement was wired into `createAuthoritativeWorkflowSeams.execute` — which is NOT +the handler production runs. `createDefaultNodeHandlers` picks the PRIMITIVES prompt-like handler +whenever `deps.primitives` is set, and `executeWorkflowGraph` always sets it, so the legacy-seams +prompt handler is unreachable for prompt nodes. Everything wired only there is dead code that +type-checks, passes its own unit tests against the seam object, and never runs. + +That is why this file exists at all: a seam-level test cannot tell the two apart. These assert the +announcement on the LIVE primitive, and — more importantly — assert the wiring rule itself, so the +next person adding behavior to a seam finds out from a red test rather than from an operator. +*/ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { readFileSync } from "node:fs"; +import type { Settings, TaskDetail } from "@fusion/core"; +import { + getWorkflowEventBus, + resetWorkflowEventBusForTesting, + type WorkflowLifecycleEvent, +} from "@fusion/core"; +import "./executor-test-helpers.js"; +import { TaskExecutor } from "../executor.js"; +import { createDefaultNodeHandlers } from "../workflow-node-handlers.js"; +import { createMockStore, resetExecutorMocks } from "./executor-test-helpers.js"; + +const TASK = { id: "FN-PRIM-EXIT", column: "in-progress", steps: [], paused: false } as unknown as TaskDetail; + +function harness(phase: { taskDone: boolean; modifiedFiles: string[]; exit?: string }) { + const store = createMockStore(); + store.getTask.mockResolvedValue(TASK); + const executor = new TaskExecutor(store, "/tmp/test"); + vi.spyOn(executor as never as { runImplementationPhase: () => unknown }, "runImplementationPhase") + .mockResolvedValue(phase); + const primitives = executor.createAuthoritativeWorkflowPrimitives({} as Settings); + const ctx = { run: {}, node: { node: { id: "execute", kind: "prompt" }, context: {} } } as never; + return { executor, primitives, ctx }; +} + +function captured(): { events: WorkflowLifecycleEvent[]; drain: () => Promise } { + const events: WorkflowLifecycleEvent[] = []; + getWorkflowEventBus().subscribe((e) => { events.push(e); }, { name: "prim-exit" }); + return { events, drain: () => getWorkflowEventBus().drain() }; +} + +describe("the LIVE implementation primitive announces the exit", () => { + beforeEach(() => { resetExecutorMocks(); resetWorkflowEventBusForTesting(); }); + afterEach(() => resetWorkflowEventBusForTesting()); + + it("emits NodeCompleted with the exit from runCodingSession", async () => { + const { primitives, ctx } = harness({ taskDone: false, modifiedFiles: [], exit: "review-handoff-pending-review" }); + const bus = captured(); + + await primitives.runCodingSession(ctx, TASK, { worktreePath: "/tmp/wt", branchName: "b" } as never); + await bus.drain(); + + const completed = bus.events.filter((e) => e.type === "NodeCompleted"); + expect(completed).toHaveLength(1); + expect(completed[0]).toMatchObject({ taskId: TASK.id, outcome: "failure", exit: "review-handoff-pending-review" }); + }); + + it("emits success without an exit for an ordinary completion", async () => { + const { primitives, ctx } = harness({ taskDone: true, modifiedFiles: [] }); + const bus = captured(); + + await primitives.runCodingSession(ctx, TASK, { worktreePath: "/tmp/wt", branchName: "b" } as never); + await bus.drain(); + + const completed = bus.events.filter((e) => e.type === "NodeCompleted"); + expect(completed).toHaveLength(1); + expect(completed[0]).toMatchObject({ outcome: "success" }); + expect(completed[0]).not.toHaveProperty("exit"); + }); + + it("returns the unchanged routing outcome — announcing must not reroute", async () => { + const { primitives, ctx } = harness({ taskDone: false, modifiedFiles: [], exit: "review-handoff-pending-review" }); + + const result = await primitives.runCodingSession(ctx, TASK, { worktreePath: "/tmp/wt", branchName: "b" } as never); + + expect(result).toMatchObject({ outcome: "failure", value: "implementation-incomplete" }); + }); + + /* + The rule, asserted rather than remembered. `createDefaultNodeHandlers` prefers the primitives + handler whenever primitives are supplied; `executeWorkflowGraph` always supplies them. If that + preference is ever inverted or made conditional, every behavior wired to the primitives path + silently stops running — the same failure that put the announcement on a dead seam. + */ + /* + FNXC:WorkflowExecutionOwnership 2026-07-29-21:10 (U8 / R12, PR #2578 review — greptile): + BEHAVIOURAL, not textual. The first version grepped for `deps?.primitives ? createPrimitive...` + and for the executor's wiring string. Those fragments can both survive while an earlier fallback, + an extracted dispatch helper, or a reordered condition stops selecting the primitive handler — + so the ratchet would stay green through exactly the regression it exists to catch, which is the + same "proves syntax, not behaviour" mistake that put a shipped behaviour on a dead seam. + + This builds the real handler map from spied seams AND spied primitives and asserts, per seam, + which one ran. It fails if dispatch ever stops preferring primitives, however that happens. + */ + it("dispatches every seam through the PRIMITIVES handler, never the legacy seams", async () => { + const calls: string[] = []; + const seamSpy = (name: string) => async () => { calls.push(`seam:${name}`); return { outcome: "success" as const }; }; + const seams = { + planning: seamSpy("planning"), + execute: seamSpy("execute"), + review: seamSpy("review"), + "review-handoff": seamSpy("review-handoff"), + merge: seamSpy("merge"), + schedule: seamSpy("schedule"), + stepExecute: seamSpy("step-execute"), + } as never; + const primitives = { + prepareWorktree: async () => { calls.push("prim:prepareWorktree"); return { outcome: "success", data: { worktreePath: "/tmp/wt", branchName: "b" } }; }, + runPlanningSession: async () => { calls.push("prim:runPlanningSession"); return { outcome: "success" }; }, + runCodingSession: async () => { calls.push("prim:runCodingSession"); return { outcome: "success", value: "implemented" }; }, + requestReviewHandoff: async () => { calls.push("prim:requestReviewHandoff"); return { outcome: "success" }; }, + requestReview: async () => { calls.push("prim:requestReview"); return { outcome: "success" }; }, + requestMerge: async () => { calls.push("prim:requestMerge"); return { outcome: "success" }; }, + scheduleWork: async () => { calls.push("prim:scheduleWork"); return { outcome: "success" }; }, + runTaskStep: async () => { calls.push("prim:runTaskStep"); return { outcome: "success" }; }, + } as never; + + const handlers = createDefaultNodeHandlers(seams, undefined, { primitives }); + for (const seam of ["planning", "execute", "review", "review-handoff", "merge", "schedule"]) { + await handlers.prompt!( + { id: `${seam}-node`, kind: "prompt", column: "in-progress", config: { seam } } as never, + { task: { id: "FN-DISPATCH" }, context: {} } as never, + ).catch(() => undefined); + } + + /* The assertion that matters: no seam function ran, for any seam name. */ + expect(calls.filter((c) => c.startsWith("seam:"))).toEqual([]); + expect(calls.some((c) => c.startsWith("prim:"))).toBe(true); + }); +}); diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 09f2bef8a4..6a0e21aae2 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -7298,12 +7298,34 @@ export class TaskExecutor { if (typeof governingNodeId === "string") { this.graphSeamGoverningNodeId.set(task.id, governingNodeId); } - let result: { taskDone: boolean; modifiedFiles: string[] }; + let result: { taskDone: boolean; modifiedFiles: string[]; exit?: ImplementationExit }; try { result = await this.runImplementationPhase(task, prepared); } finally { this.graphSeamGoverningNodeId.delete(task.id); } + /* + FNXC:WorkflowExecutionOwnership 2026-07-29-16:20 (U8 / R4, R5): + THIS is the live implementation node, not the identically-shaped `execute` entry in + `createAuthoritativeWorkflowSeams`. `createDefaultNodeHandlers` prefers the PRIMITIVES + handler whenever `deps.primitives` is set, and `executeWorkflowGraph` always sets it — so + the legacy-seams prompt handler is unreachable for prompt nodes and anything wired only + there never runs. The exit announcement was wired only there; it is announced here now. + + Measured, not assumed: instrumenting the seam and `createPromptLikeHandler` produced no + output for a graph run that demonstrably visited `steps#0:step-execute`, while a + module-load write from the same file appeared — so the negative was real and not swallowed + output. + */ + emitWorkflowLifecycleEvent({ + type: "NodeCompleted", + taskId: task.id, + at: new Date().toISOString(), + runId: this.getRunContextFor(task.id)?.runId, + nodeId: typeof governingNodeId === "string" ? governingNodeId : ctx.node.node.id, + outcome: result.taskDone ? "success" : "failure", + ...(result.exit ? { exit: result.exit } : {}), + }); if (result.taskDone) { return { outcome: "success", value: "implemented", data: result }; }