U8: the exit announcement was on a dead code path — move it to the handler the engine actually runs (#2578)
A merged behavior of mine has never executed. This fixes it and adds the ratchet that would have caught it. ## The finding `createDefaultNodeHandlers` chooses the prompt-node handler like this: ```ts const promptLike = deps?.primitives ? createPrimitivePromptLikeHandler(deps.primitives, runCustomNode) : createPromptLikeHandler(seams, runCustomNode); ``` `executeWorkflowGraph` always passes `primitives: this.createAuthoritativeWorkflowPrimitives(settings)` (`executor.ts:6051`). **So `createPromptLikeHandler` — and with it every `execute` / `step-execute` function in `createAuthoritativeWorkflowSeams` — is unreachable for prompt nodes.** Both objects are passed to the graph executor and only one is consulted. The `NodeCompleted.exit` announcement added in #2507 was wired into that seam. It type-checks, its tests pass (they call the seam object directly), and it has never run in production. `runCodingSession` in the primitives is the live twin, and that is where it emits now. ## How it was found — and why the negative is trustworthy Instrumenting `createAuthoritativeWorkflowSeams.stepExecute` produced no output for a run that demonstrably visits `steps#0:step-execute`. So did instrumenting `createPromptLikeHandler`'s dispatch. A negative result from instrumentation is worthless until the instrumentation is shown to be observable, so: a `process.stderr.write` at module load of the same file **did** appear, exactly once, in the same run. The two negatives were real, not swallowed output. This is also the answer to the open question I left in #2546 — the pending-review routing move kept failing because the seam value it depends on is never produced. **That move is still not landed here.** This commit only relocates the announcement, so it stays small and separately revertable; the routing move follows once its value originates on the live path. ## The ratchet A source assertion pins the dispatch rule: `deps?.primitives ? createPrimitivePromptLikeHandler` and the executor's wiring of `primitives`. Inverting or conditionalising that preference would silently disable every behavior attached to the primitives path — the same failure in the other direction — and **a seam-level unit test cannot tell the two apart**, which is precisely how this survived review twice. ## Red-green Removing the emit fails 2 of the 4 new tests (`Tests 2 failed | 2 passed (4)`). The other two are the regression floor: an ordinary completion emits `success` with no `exit`, and the returned routing outcome is unchanged — announcing must not reroute. ## Scope note I did **not** delete the now-known-dead seam wiring in this PR. `createAuthoritativeWorkflowSeams` is still passed to the graph executor and its non-prompt entries (`stepReview`, `merge`) are reached through other handlers, so deciding what is genuinely dead there is a deletion audit of its own — and this program's rule is that deletions never ride along with behavior changes. Filed as the next slice. ## Verification - 4 new tests + exit-events + step-session + triage audit + ownership ledger — **54 tests green** - `pnpm test:gate` green (10 / 414 / 71); `pnpm lint` clean; `tsc --noEmit` clean - Changeset included (`patch`, `fix`) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
7
.changeset/u8-primitive-exit-events.md
Normal file
7
.changeset/u8-primitive-exit-events.md
Normal file
@@ -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.
|
||||
@@ -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<void> } {
|
||||
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);
|
||||
});
|
||||
});
|
||||
@@ -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 };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user