diff --git a/.changeset/u8-implementation-exit-events.md b/.changeset/u8-implementation-exit-events.md new file mode 100644 index 0000000000..a96d316242 --- /dev/null +++ b/.changeset/u8-implementation-exit-events.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Surface how an implementation session actually ended, including when the executor moved the card itself. +category: internal +dev: Adds a closed `ImplementationExit` enum (`engine/executor/implementation-exit.ts`) reported from six completion-adjacent exits in `runImplementation` and announced by the execute seam as `NodeCompleted.exit` on the U3 lifecycle bus. Routing is byte-identical for every exit; nothing branches on an exit id (R5 — reactions only). diff --git a/packages/core/src/__tests__/workflow-events.test.ts b/packages/core/src/__tests__/workflow-events.test.ts index f1191ba472..3624a4fa64 100644 --- a/packages/core/src/__tests__/workflow-events.test.ts +++ b/packages/core/src/__tests__/workflow-events.test.ts @@ -19,11 +19,12 @@ in `workflow-events-outbox.pg.test.ts` — a hand-written fake of the lease predicate would only prove the fake redelivers. */ import { describe, expect, it, vi } from "vitest"; -import { createWorkflowEventBus } from "../workflow-events.js"; +import { createWorkflowEventBus, emitWorkflowLifecycleEvent, getWorkflowEventBus, resetWorkflowEventBusForTesting } from "../workflow-events.js"; import { findWorkflowEventShapeViolations, isIdsOnlyWorkflowEvent, MAX_ID_VALUE_LENGTH, + IMPLEMENTATION_EXITS, type WorkflowLifecycleEvent, } from "../types/workflow-events.js"; @@ -296,3 +297,45 @@ describe("workflow event bus — reactions are non-authoritative (R5, KTD-3)", ( expect(survivor).toHaveBeenCalledTimes(1); }); }); + +/* +FNXC:WorkflowEvents 2026-07-28-22:30 (U8, PR #2507 review — greptile): +The `exit` key carries a CLOSED vocabulary, and the closed-ness is enforced at the emit boundary +rather than only in the type. The type protects TypeScript producers; the boundary protects the +ones that can actually cause the silent failure — a JS caller, a plugin, or a future seam +emitting an id nobody routes, where the symptom is a card that quietly does not advance. +*/ +describe("closed-vocabulary values (exit)", () => { + const base = { type: "NodeCompleted", taskId: "FN-1", at: "2026-07-28T00:00:00.000Z", nodeId: "execute", outcome: "success" }; + + it("accepts every declared exit id", () => { + for (const exit of IMPLEMENTATION_EXITS) { + expect(findWorkflowEventShapeViolations({ ...base, exit })).toEqual([]); + } + }); + + it("refuses an exit id that is not in the vocabulary", () => { + /* A perfectly good scalar — which is exactly why value-shape checking alone is not enough. */ + expect(findWorkflowEventShapeViolations({ ...base, exit: "review-handoff-invented" })).toEqual([ + { path: "exit", reason: "unknown-enum-value" }, + ]); + }); + + it("still accepts NodeCompleted with no exit at all", () => { + expect(findWorkflowEventShapeViolations(base)).toEqual([]); + }); + + it("drops an event carrying an unrouted exit rather than delivering it", () => { + /* End to end through the bus: a violating payload must never reach a subscriber. */ + resetWorkflowEventBusForTesting(); + const seen: unknown[] = []; + getWorkflowEventBus().subscribe((e) => { seen.push(e); }, { name: "closed-vocab" }); + emitWorkflowLifecycleEvent({ ...base, exit: "not-a-real-exit" } as never); + emitWorkflowLifecycleEvent({ ...base, exit: "complete" } as never); + return getWorkflowEventBus().drain().then(() => { + expect(seen).toHaveLength(1); + expect(seen[0]).toMatchObject({ exit: "complete" }); + resetWorkflowEventBusForTesting(); + }); + }); +}); diff --git a/packages/core/src/index.gate.ts b/packages/core/src/index.gate.ts index e226a70954..4f10777211 100644 --- a/packages/core/src/index.gate.ts +++ b/packages/core/src/index.gate.ts @@ -2235,8 +2235,8 @@ export { resolveCreationColumn } from "./workflow-ir.js"; export { resolveWipBudgetColumns } from "./workflow-capacity.js"; export { createWorkflowEventBus, getWorkflowEventBus, emitWorkflowLifecycleEvent, resetWorkflowEventBusForTesting } from "./workflow-events.js"; export type { WorkflowEventBus, WorkflowEventSubscriber, WorkflowEventSubscription } from "./workflow-events.js"; -export { findWorkflowEventShapeViolations, isIdsOnlyWorkflowEvent, MAX_ID_VALUE_LENGTH } from "./types/workflow-events.js"; -export type { WorkflowLifecycleEvent, WorkflowLifecycleEventType, WorkflowLifecycleEventBase, TaskTransitionedEvent, NodeEnteredEvent, NodeCompletedEvent, RunSuspendedEvent, RunResumedEvent, WorkflowEventShapeViolation } from "./types/workflow-events.js"; +export { findWorkflowEventShapeViolations, isIdsOnlyWorkflowEvent, MAX_ID_VALUE_LENGTH, IMPLEMENTATION_EXITS } from "./types/workflow-events.js"; +export type { WorkflowLifecycleEvent, WorkflowLifecycleEventType, WorkflowLifecycleEventBase, TaskTransitionedEvent, NodeEnteredEvent, NodeCompletedEvent, RunSuspendedEvent, RunResumedEvent, WorkflowEventShapeViolation, ImplementationExit } from "./types/workflow-events.js"; export { columnsWithFlag, columnHasFlag, resolveReboundTarget, resolveCompleteColumn, resolveMergeOrchestrationColumn, resolveLifecycleColumns, resolveTaskLifecycleColumns } from "./workflow-lifecycle-traits.js"; export type { LifecycleColumns } from "./workflow-lifecycle-traits.js"; export { resolveReviewLevelSteps, applyReviewLevelPreset } from "./review-level-preset.js"; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 8abfe3f004..1d16e82d3a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -450,8 +450,8 @@ export type { PluginGateVerdict, ColumnPluginGate } from "./plugin-gate-verdict. export { resolveColumnCapacity, resolveWipBudgetColumns, DEFAULT_WORKFLOW_POOL_ID, resolveCapacityPoolId } from "./workflow-capacity.js"; export { createWorkflowEventBus, getWorkflowEventBus, emitWorkflowLifecycleEvent, resetWorkflowEventBusForTesting } from "./workflow-events.js"; export type { WorkflowEventBus, WorkflowEventSubscriber, WorkflowEventSubscription } from "./workflow-events.js"; -export { findWorkflowEventShapeViolations, isIdsOnlyWorkflowEvent, MAX_ID_VALUE_LENGTH } from "./types/workflow-events.js"; -export type { WorkflowLifecycleEvent, WorkflowLifecycleEventType, WorkflowLifecycleEventBase, TaskTransitionedEvent, NodeEnteredEvent, NodeCompletedEvent, RunSuspendedEvent, RunResumedEvent, WorkflowEventShapeViolation } from "./types/workflow-events.js"; +export { findWorkflowEventShapeViolations, isIdsOnlyWorkflowEvent, MAX_ID_VALUE_LENGTH, IMPLEMENTATION_EXITS } from "./types/workflow-events.js"; +export type { WorkflowLifecycleEvent, WorkflowLifecycleEventType, WorkflowLifecycleEventBase, TaskTransitionedEvent, NodeEnteredEvent, NodeCompletedEvent, RunSuspendedEvent, RunResumedEvent, WorkflowEventShapeViolation, ImplementationExit } from "./types/workflow-events.js"; export { columnsWithFlag, columnHasFlag, resolveReboundTarget, resolveCompleteColumn, resolveMergeOrchestrationColumn, resolveLifecycleColumns, resolveTaskLifecycleColumns } from "./workflow-lifecycle-traits.js"; export type { LifecycleColumns } from "./workflow-lifecycle-traits.js"; export { resolveReviewLevelSteps, applyReviewLevelPreset } from "./review-level-preset.js"; diff --git a/packages/core/src/types/workflow-events.ts b/packages/core/src/types/workflow-events.ts index 55b88f8ea5..a89c69fb5e 100644 --- a/packages/core/src/types/workflow-events.ts +++ b/packages/core/src/types/workflow-events.ts @@ -68,10 +68,53 @@ export interface NodeEnteredEvent extends WorkflowLifecycleEventBase { } /** A node finished with a routing outcome ("success" / "failure" / …). */ +/* +FNXC:WorkflowEvents 2026-07-28-22:10 (U8 / R4, R5, PR #2507 review — greptile): +THE CLOSED EXIT VOCABULARY, AND WHY IT LIVES HERE. + +It was first declared in `@fusion/engine` with the public event field typed `exit?: string`, so +the contract permitted values no consumer handles. That is a small typing gap today, with one +producer — and an expensive one later, because this bus is becoming the lifecycle backbone +("node transitions should emit events and event subscribers handle moving things through the +lifecycle"). A producer emitting an exit nobody routes fails SILENTLY: the card simply does not +advance, and nothing anywhere reports a problem. Close it while there is one producer. + +The union therefore lives with the contract, in core, not with its first producer in the engine +— core cannot import from the engine, and more to the point a public contract that defers its +vocabulary to a consumer is not a contract. `engine/executor/implementation-exit.ts` re-exports +it and keeps the engine-side POLICY (which exits are executor-performed) where policy belongs. + +Adding a value means editing this list, which is the same deliberate act the key allow-list +demands — and `IMPLEMENTATION_EXITS` is checked at the EMIT BOUNDARY too, so a JS producer or a +plugin cannot slip an unrouted id past the type system. +*/ +export const IMPLEMENTATION_EXITS = [ + /** fn_task_done (or implicit completion): handed back to the graph, which owns what follows. */ + "complete", + /** Completion reached on a retry session after the agent first failed to signal done. */ + "complete-after-retry", + /** Completion proven from live modified files when the session ended without a done signal. */ + "complete-from-live-files", + /** OUT OF BAND: paused after the work was complete; the executor finalized to review itself. */ + "review-handoff-paused-after-completion", + /** OUT OF BAND: stopped on a pending-review block; the executor parked it in review itself. */ + "review-handoff-pending-review", +] as const; + +/** How a node's work actually ended, when the routing outcome is coarser than the endings. */ +export type ImplementationExit = (typeof IMPLEMENTATION_EXITS)[number]; + export interface NodeCompletedEvent extends WorkflowLifecycleEventBase { type: "NodeCompleted"; nodeId: string; outcome: string; + /* + FNXC:WorkflowEvents 2026-07-28-20:20 (U8 / R4, R5): + Optional finer-grained ending, for nodes whose routing outcome is coarser than the ways they + can actually end. The execute seam is the motivating case: `success | failure` cannot express + "the executor finalized this card to review itself", so that ending was invisible. + */ + exit?: ImplementationExit; } /** A run parked at a seam it cannot cross yet (capacity, manual hold). */ @@ -146,7 +189,7 @@ const COMMON_REQUIRED_EVENT_KEYS = ["type", "taskId", "at"] as const; const ALLOWED_EVENT_KEYS: Record = { TaskTransitioned: [...COMMON_EVENT_KEYS, "from", "to", "nodeId", "moveSource"], NodeEntered: [...COMMON_EVENT_KEYS, "nodeId", "column"], - NodeCompleted: [...COMMON_EVENT_KEYS, "nodeId", "outcome"], + NodeCompleted: [...COMMON_EVENT_KEYS, "nodeId", "outcome", "exit"], RunSuspended: [...COMMON_EVENT_KEYS, "nodeId", "reason", "fromColumn", "toColumn"], RunResumed: [...COMMON_EVENT_KEYS, "nodeId", "releasedBy"], }; @@ -166,6 +209,11 @@ const REQUIRED_EVENT_KEYS: Record RunResumed: [...COMMON_REQUIRED_EVENT_KEYS, "nodeId"], }; +/** Keys whose values are a closed vocabulary rather than a free id. */ +const CLOSED_VALUE_SETS: Record = { + exit: IMPLEMENTATION_EXITS, +}; + /** A single ids-only rule violation. `path` locates it for the failure message. */ export interface WorkflowEventShapeViolation { path: string; @@ -175,7 +223,8 @@ export interface WorkflowEventShapeViolation { | "unsupported-type" | "unknown-key" | "unknown-type" - | "missing-required-key"; + | "missing-required-key" + | "unknown-enum-value"; } function checkScalar(path: string, value: unknown, out: WorkflowEventShapeViolation[]): void { @@ -222,6 +271,18 @@ export function findWorkflowEventShapeViolations(event: unknown): WorkflowEventS value.forEach((entry, i) => checkScalar(`${key}[${i}]`, entry, violations)); continue; } + /* + FNXC:WorkflowEvents 2026-07-28-22:15 (U8, PR #2507 review — greptile): + The closed-set keys are checked for MEMBERSHIP, not merely scalar-ness. The type alone + protects TypeScript producers; this protects the ones that matter — a JS caller, a plugin, + and a future seam emitting an id nobody routes. Refusing it here turns a silent + card-does-not-advance into a logged drop at the boundary. + */ + const closedSet = CLOSED_VALUE_SETS[key]; + if (closedSet && value !== undefined && !closedSet.includes(value as never)) { + violations.push({ path: key, reason: "unknown-enum-value" }); + continue; + } checkScalar(key, value, violations); } /* diff --git a/packages/engine/src/__tests__/executor-implementation-exit-events.test.ts b/packages/engine/src/__tests__/executor-implementation-exit-events.test.ts new file mode 100644 index 0000000000..dd971845ed --- /dev/null +++ b/packages/engine/src/__tests__/executor-implementation-exit-events.test.ts @@ -0,0 +1,196 @@ +/* +FNXC:WorkflowExecutionOwnership 2026-07-28-20:40 (U8 / R4, R5, R12 — workflow-owned lifecycle): + +The execute seam tells the graph one bit: `result.taskDone`. The endings that bit cannot express +are exactly the ones the implementation phase transitions ITSELF — a session that paused after +the work was complete, and a session that stopped on a pending-review block. Both hand the card +to review inline; the graph then sees `taskDone === false`, reports `implementation-incomplete`, +and `handleGraphFailure` compensates with `alreadyFinalizedToReview`. Until now nothing anywhere +recorded which of those happened: an out-of-band transition and a genuine implementation failure +were indistinguishable in logs, in events, and in tests. + +These tests pin the properties that make the exit signal safe to build the routing move on: + + 1. Every exit the phase reports is forwarded with its own id, AND every id in the enum has a + real call site in `runImplementation`. The second half is not pedantry: these tests stub + `runImplementationPhase`, so without it deleting a `reportImplementationExit(...)` call + leaves all of them green — verified by deleting one. A stubbed seam can only prove the seam. + 2. ROUTING IS UNCHANGED. For every exit the seam returns byte-identically what it returned + before, so this PR cannot move a card. That is the property that lets the reporting and the + routing land as separate, independently revertable changes. + 3. DROPPING EVERY SUBSCRIBER CHANGES NO OUTCOME (R5, and a named U8 scenario). An exit id is a + reaction; if anything downstream ever depends on one arriving, the bus has quietly become a + second source of truth, which is the failure mode this whole program exists to remove. +*/ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { readFileSync } from "node:fs"; +import type { Settings, Task } from "@fusion/core"; +import { + getWorkflowEventBus, + resetWorkflowEventBusForTesting, + type WorkflowLifecycleEvent, +} from "@fusion/core"; +import "./executor-test-helpers.js"; +import { TaskExecutor } from "../executor.js"; +import { createMockStore, resetExecutorMocks } from "./executor-test-helpers.js"; +import { + OUT_OF_BAND_IMPLEMENTATION_EXITS, + isOutOfBandImplementationExit, + type ImplementationExit, +} from "../executor/implementation-exit.js"; + +const SEAM_TASK = { id: "FN-U8-EXIT", title: "exit vocabulary", column: "in-progress" } as Task; + +/** + * Drive the execute seam with a stubbed implementation phase. Stubbing the phase rather than + * running a real agent session is the only way to exercise all six exits deterministically; the + * exits themselves are wired at their real call sites in `runImplementation` and covered by the + * completion/handoff suites. + */ +function seamHarness(phaseResult: { taskDone: boolean; modifiedFiles: string[]; exit?: ImplementationExit }) { + const store = createMockStore(); + store.getTask.mockResolvedValue({ ...SEAM_TASK, paused: false }); + const executor = new TaskExecutor(store, "/tmp/test"); + const runImplementationPhase = vi + .spyOn(executor as never as { runImplementationPhase: () => unknown }, "runImplementationPhase") + .mockResolvedValue(phaseResult); + const seams = executor.createAuthoritativeWorkflowSeams({} as Settings); + return { executor, store, seams, runImplementationPhase }; +} + +function captureEvents(): { events: WorkflowLifecycleEvent[]; drain: () => Promise } { + const events: WorkflowLifecycleEvent[] = []; + getWorkflowEventBus().subscribe((event) => { events.push(event); }, { name: "exit-test" }); + return { events, drain: () => getWorkflowEventBus().drain() }; +} + +/** Every exit, with the outcome/value the seam returned for it BEFORE this change. */ +const EXITS: Array<{ + exit: ImplementationExit; + taskDone: boolean; + expected: { outcome: string; value: string }; +}> = [ + { exit: "complete", taskDone: true, expected: { outcome: "success", value: "implemented" } }, + { exit: "complete-after-retry", taskDone: true, expected: { outcome: "success", value: "implemented" } }, + { exit: "complete-from-live-files", taskDone: true, expected: { outcome: "success", value: "implemented" } }, + { exit: "review-handoff-paused-after-completion", taskDone: false, expected: { outcome: "failure", value: "implementation-incomplete" } }, + { exit: "review-handoff-pending-review", taskDone: false, expected: { outcome: "failure", value: "implementation-incomplete" } }, +]; + +describe("execute seam announces the implementation phase's exit", () => { + beforeEach(() => { + resetExecutorMocks(); + resetWorkflowEventBusForTesting(); + }); + afterEach(() => resetWorkflowEventBusForTesting()); + + it.each(EXITS)("reports $exit on the lifecycle bus", async ({ exit, taskDone }) => { + const { seams } = seamHarness({ taskDone, modifiedFiles: [], exit }); + const bus = captureEvents(); + + await seams.execute!(SEAM_TASK, undefined); + await bus.drain(); + + const completed = bus.events.filter((e) => e.type === "NodeCompleted"); + expect(completed).toHaveLength(1); + expect(completed[0]).toMatchObject({ + type: "NodeCompleted", + taskId: SEAM_TASK.id, + nodeId: "execute", + outcome: taskDone ? "success" : "failure", + exit, + }); + }); + + /* + The property that makes this PR safe to land ahead of the routing move: naming an exit must not + reroute anything. If any of these drift, a card is moving somewhere new and that belongs in the + PR that adds the IR edge, not in this one. + */ + it.each(EXITS)("returns the pre-existing routing outcome for $exit", async ({ exit, taskDone, expected }) => { + const { seams } = seamHarness({ taskDone, modifiedFiles: [], exit }); + + await expect(seams.execute!(SEAM_TASK, undefined)).resolves.toEqual(expected); + }); + + it("returns the same outcome when the phase reports no exit at all", async () => { + /* The ~22 uninstrumented dispositions still report nothing; they must be unaffected. */ + const { seams } = seamHarness({ taskDone: false, modifiedFiles: [] }); + const bus = captureEvents(); + + const outcome = await seams.execute!(SEAM_TASK, undefined); + await bus.drain(); + + expect(outcome).toEqual({ outcome: "failure", value: "implementation-incomplete" }); + const completed = bus.events.filter((e) => e.type === "NodeCompleted"); + expect(completed).toHaveLength(1); + expect(completed[0]).not.toHaveProperty("exit"); + }); + + /* + R5, and a named U8 test scenario: "A dropped event subscriber changes no execution outcome + (proves reactions are non-authoritative)." + */ + it("produces identical outcomes with NO subscribers at all", async () => { + for (const { exit, taskDone, expected } of EXITS) { + resetWorkflowEventBusForTesting(); + const { seams } = seamHarness({ taskDone, modifiedFiles: [], exit }); + expect(getWorkflowEventBus().subscriberCount()).toBe(0); + + await expect(seams.execute!(SEAM_TASK, undefined)).resolves.toEqual(expected); + } + }); + + it("is not derailed by a throwing subscriber", async () => { + const { seams } = seamHarness({ taskDone: true, modifiedFiles: [], exit: "complete" }); + getWorkflowEventBus().subscribe(() => { throw new Error("subscriber blew up"); }, { name: "boom" }); + + await expect(seams.execute!(SEAM_TASK, undefined)).resolves.toEqual({ + outcome: "success", + value: "implemented", + }); + await getWorkflowEventBus().drain(); + }); + + /* + FNXC:WorkflowExecutionOwnership 2026-07-28-21:05 (U8 / R12): + The wiring ratchet. Everything above drives a STUBBED implementation phase, which is the only + way to reach all six exits deterministically — but it means the real call sites are not + exercised. Deleting `reportImplementationExit?.("review-handoff-pending-review")` from + `runImplementation` left this whole file green (measured, not assumed), so the enum would have + drifted into a vocabulary that describes endings nothing actually reports. This asserts each id + is wired, and that the two out-of-band ids sit with the handoff they describe. + */ + it("every exit id has a real call site in runImplementation", () => { + const source = readFileSync(new URL("../executor.ts", import.meta.url), "utf8") + .replace(/\/\*[\s\S]*?\*\//g, " ") + .replace(/(^|[^:])\/\/[^\n]*/g, "$1 "); + const ALL_EXITS: ImplementationExit[] = [ + "complete", + "complete-after-retry", + "complete-from-live-files", + "review-handoff-paused-after-completion", + "review-handoff-pending-review", + ]; + const missing = ALL_EXITS.filter((exit) => !source.includes(`reportImplementationExit?.("${exit}")`)); + expect(missing).toEqual([]); + /* Each out-of-band id must accompany an inline review handoff — that pairing IS its meaning. */ + for (const exit of OUT_OF_BAND_IMPLEMENTATION_EXITS) { + const idx = source.indexOf(`reportImplementationExit?.("${exit}")`); + expect(source.slice(idx, idx + 400)).toContain("handoffTaskToReview("); + } + }); + + it("classifies exactly the two executor-performed transitions as out-of-band", () => { + /* + The ledger this unit closes: an out-of-band exit is one where the EXECUTOR moved the card. + If a third appears without a routing move, U8 has gone backwards. + */ + expect([...OUT_OF_BAND_IMPLEMENTATION_EXITS]).toEqual([ + "review-handoff-paused-after-completion", + "review-handoff-pending-review", + ]); + expect(isOutOfBandImplementationExit("complete")).toBe(false); + expect(isOutOfBandImplementationExit(undefined)).toBe(false); + }); +}); diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index df6e926f30..b9fc6db8e6 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -14,6 +14,8 @@ import { existsSync, lstatSync, realpathSync } from "node:fs"; import { readFile, rm, writeFile } from "node:fs/promises"; import type { TaskStore, Task, TaskDetail, TaskTokenUsage, StepStatus, Settings, WorkflowStep, MissionStore, AsyncMissionStore, Slice, AgentState, AgentCapability, RunMutationContext, AgentHeartbeatConfig, Agent, AgentMemoryInclusionMode, ProjectSettings, MergeResult, WorkflowIrNode, WorkflowIrNodeKind, WorkflowStepResult as CoreWorkflowStepResult, ThinkingLevel } from "@fusion/core"; import { getUnmetSchedulingDependencies } from "./scheduler.js"; +import type { ImplementationExit, ImplementationExitReporter } from "./executor/implementation-exit.js"; +import { emitWorkflowLifecycleEvent } from "@fusion/core"; import { RetryStormError, serializeRetryStormError, evaluateCompletedPromotionFailureProvenance, evaluateSkipBypassTaint, resolveWorkflowIrForTask, evaluateForeachMergeProof, resolveCompleteColumn, resolveMergeOrchestrationColumn, resolveReboundTarget, resolveLifecycleColumns, resolveColumnAgentBinding, resolveEffectiveAgent, instanceNodeId, getWorkflowExtensionRegistry, getBuiltinWorkflow, parseNoOpCompletionMarker, allowsAutoMergeProcessing, resolveEffectiveAutoMerge, isLiveSharedBranchGroupMemberIntegration, resolveMaxAutoMergeRetries, resolveMaxConsecutiveToolFailureRetries, resolveConsecutiveToolFailureRetryBackoffMs, resolveConsecutiveToolFailureThreshold, resolveExecutorEscalationTarget, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, DEFAULT_MAX_POST_REVIEW_FIXES, COMPLETION_SUMMARY_NODE_ID, upsertWorkflowStepResult, AWAITING_APPROVAL_PAUSE_REASON, THINKING_LEVELS, ACTIVE_WORKFLOW_WORK_ITEM_STATES, AgentStore, resolveExecutorFallbackModel } from "@fusion/core"; import { finalizeProvenAutoMergeTask } from "./auto-merge-finalization.js"; import { mergeEffectiveSettings } from "./effective-settings.js"; @@ -6916,10 +6918,14 @@ export class TaskExecutor { private async runImplementationPhase( task: Task, prepared?: PreparedWorktree, - ): Promise<{ taskDone: boolean; modifiedFiles: string[] }> { - let captured: { taskDone: boolean; modifiedFiles: string[] } = { taskDone: false, modifiedFiles: [] }; + ): Promise<{ taskDone: boolean; modifiedFiles: string[]; exit?: ImplementationExit }> { + let captured: { taskDone: boolean; modifiedFiles: string[]; exit?: ImplementationExit } = { taskDone: false, modifiedFiles: [] }; const graphCompletion: GraphCompletionCallback = (info) => { - captured = { taskDone: true, modifiedFiles: info.modifiedFiles }; + captured = { ...captured, taskDone: true, modifiedFiles: info.modifiedFiles }; + }; + /* Recorded independently of `graphCompletion`: the out-of-band exits never call it. */ + const reportExit: ImplementationExitReporter = (exit) => { + captured = { ...captured, exit }; }; const executionTask = prepared ? { @@ -6928,7 +6934,7 @@ export class TaskExecutor { branch: prepared.branchName || task.branch, } : task; - await this.runImplementation(executionTask, graphCompletion); + await this.runImplementation(executionTask, graphCompletion, reportExit); return captured; } @@ -7753,7 +7759,7 @@ export class TaskExecutor { if (typeof seamThinkingLevel === "string" && WORKFLOW_THINKING_LEVEL_SET.has(seamThinkingLevel)) { this.graphSeamThinkingLevel.set(seamTask.id, seamThinkingLevel as ThinkingLevel); } - let result: { taskDone: boolean; modifiedFiles: string[] }; + let result: { taskDone: boolean; modifiedFiles: string[]; exit?: ImplementationExit }; try { result = await this.runImplementationPhase(seamTask); } finally { @@ -7780,6 +7786,29 @@ export class TaskExecutor { compensating classifiers are the acceptance test — they become unreachable, and then deletable, exactly when the last out-of-band transition is gone. */ + /* + FNXC:WorkflowExecutionOwnership 2026-07-28-20:25 (U8 / R4, R5): + Announce the exit on the U3 lifecycle bus. Until this, the two out-of-band review + handoffs left NO trace anywhere that the executor — not the graph — moved the card; + they surfaced as an ordinary `implementation-incomplete` failure that + `handleGraphFailure` then quietly compensated for. An operator could not tell the two + apart, and neither could a test. + + Emission is deliberately AFTER the phase and BEFORE the return, and it changes nothing: + the outcome/value below are byte-identical to what this seam returned before, for every + exit, which `executor-implementation-exit-events.test.ts` pins by driving each exit and + asserting the seam's return. Per R5 an exit id is a REACTION — dropping every subscriber + must change no execution outcome, and that is asserted too. + */ + emitWorkflowLifecycleEvent({ + type: "NodeCompleted", + taskId: seamTask.id, + at: new Date().toISOString(), + runId: this.getRunContextFor(seamTask.id)?.runId, + nodeId: typeof governingNodeId === "string" ? governingNodeId : "execute", + outcome: result.taskDone ? "success" : "failure", + ...(result.exit ? { exit: result.exit } : {}), + }); if (result.taskDone) { return { outcome: "success", value: "implemented" }; } @@ -11602,6 +11631,16 @@ export class TaskExecutor { an implementation pass whose completion nothing owns can no longer be constructed. */ graphCompletion: GraphCompletionCallback, + /* + FNXC:WorkflowExecutionOwnership 2026-07-28-20:15 (U8 / R4, R5): + Optional exit reporter. `graphCompletion` can only say "done"; the endings it cannot express + are the ones the executor transitions itself (see `executor/implementation-exit.ts`). This + names them so they are OBSERVABLE before they are moved — it changes no routing and nothing + branches on it, by R5: an exit id is a reaction, and a dropped reaction must never cost a + state change. Optional so the ~22 uninstrumented dispositions stay silent rather than + forcing a 3k-line diff; the ownership ledger is the record of that gap, not this callback. + */ + reportImplementationExit?: ImplementationExitReporter, ): Promise { // FN-4811 follow-up (FN-4814/FN-4809/FN-4811 production failure): claim a @@ -12552,6 +12591,7 @@ export class TaskExecutor { this.clearCompletedTaskWatchdog(task.id); executorLog.log(`✓ ${task.id} implementation complete — graph interpreter owns the remaining lifecycle`); const liveModified = (await this.store.getTask(task.id).catch(() => task)).modifiedFiles ?? []; + reportImplementationExit?.("complete-from-live-files"); graphCompletion({ modifiedFiles: liveModified }); return; } else { @@ -13405,6 +13445,7 @@ export class TaskExecutor { FN-6644/FN-6641: the graceful-session-exit handoff must also record durable completed-finalize state because a later teardown can re-mark the abort as `hard-cancel`. The classifier uses that durable handoff marker, not the volatile provenance alone, to keep completed no-commit tasks from being re-parked failed. */ this.markCompletionFinalized(task.id); + reportImplementationExit?.("review-handoff-paused-after-completion"); await this.handoffTaskToReview(task, "paused-after-completion"); this.clearCompletedTaskWatchdog(task.id); this.signalTaskComplete(task); @@ -13472,6 +13513,7 @@ export class TaskExecutor { // at the implementation-complete boundary and hand control back. this.clearCompletedTaskWatchdog(task.id); executorLog.log(`✓ ${task.id} implementation complete — graph interpreter owns the remaining lifecycle`); + reportImplementationExit?.("complete"); graphCompletion({ modifiedFiles }); return; } else { @@ -13536,6 +13578,7 @@ export class TaskExecutor { // the task in review without setting status=failed; otherwise the // merge/review queue deadlocks on a task that is both in-review and // failed. + reportImplementationExit?.("review-handoff-pending-review"); await this.handoffTaskToReview(task, "executor-exit-while-review-pending"); pendingReviewParked = true; break; @@ -13754,6 +13797,7 @@ export class TaskExecutor { // executeWorkflowGraph, KTD-5) — nothing to gate before handoff. this.clearCompletedTaskWatchdog(task.id); executorLog.log(`✓ ${task.id} implementation complete (retry) — graph interpreter owns the remaining lifecycle`); + reportImplementationExit?.("complete-after-retry"); graphCompletion({ modifiedFiles }); return; } else if (terminallyParked) { @@ -13987,7 +14031,8 @@ export class TaskExecutor { FN-6644/FN-6641: the finally-block handoff must record durable completed-finalize state because a later teardown can overwrite provenance to `hard-cancel`. The classifier must still resolve that completed no-commit tail failure benignly without weakening genuine pause or active hard-cancel behavior. */ this.markCompletionFinalized(task.id); - await this.handoffTaskToReview(task, "paused-after-completion"); + reportImplementationExit?.("review-handoff-paused-after-completion"); + await this.handoffTaskToReview(task, "paused-after-completion"); this.signalTaskComplete(task); } else if (finalizationDecision === "blocked") { await this.persistTokenUsage(task.id); diff --git a/packages/engine/src/executor/implementation-exit.ts b/packages/engine/src/executor/implementation-exit.ts new file mode 100644 index 0000000000..2294f69777 --- /dev/null +++ b/packages/engine/src/executor/implementation-exit.ts @@ -0,0 +1,64 @@ +/* +FNXC:WorkflowExecutionOwnership 2026-07-28-20:10 (U8 / R4, R5 — workflow-owned lifecycle): + +THE IMPLEMENTATION PHASE'S EXIT VOCABULARY. + +`runImplementation` can end in many ways, and the graph is told about exactly one bit of it: +`result.taskDone`. That is the whole language the execute seam has (`executor.ts` — the seam +maps it to `"implemented"` or `"implementation-incomplete"`), and it is why the implementation +phase transitions cards ITSELF for the endings the boolean cannot express: + + - a session that paused AFTER the work was already complete finalizes to review inline; + - a session that stopped because a step is blocked on a pending review hands off to review + inline, because it cannot continue and review is not an error bucket. + +The graph then sees `taskDone === false`, reports `implementation-incomplete`, and +`handleGraphFailure` compensates with `alreadyFinalizedToReview` / `completionFinalized` — +classifiers whose entire job is recognising a move the graph did not make. Dual ownership, and +today it is INVISIBLE: nothing anywhere records that the executor, not the graph, moved the card. + +This module names those endings so they can be observed before they are moved. Each id is a +closed enum value, never prose — these ids travel on the U3 lifecycle bus to plugin subscribers +under its ids-only rule. + +WHAT THIS DELIBERATELY DOES NOT DO. It does not change routing. The execute seam returns exactly +the outcome and value it returned before, for every exit, and `executor-implementation-exit- +events.test.ts` pins that. An exit id is a REACTION under R5 — a dropped event must cost a +notification and never a state change — so nothing downstream may branch on one until the +routing move lands with its own IR edges. Reporting first, moving second, is what keeps the two +changes independently revertable. + +COVERAGE IS PARTIAL ON PURPOSE. `runImplementation` has ~28 lifecycle dispositions (measured by +`executor-lifecycle-ownership-ledger.test.ts`) and this instruments the six completion-adjacent +ones — the three graph handbacks and the three inline review handoffs. Those are the exits U8's +routing move needs; the rest report nothing yet and the ledger, not this enum, is the record of +that gap. +*/ + +/* +FNXC:WorkflowExecutionOwnership 2026-07-28-22:20 (U8, PR #2507 review — greptile): +THE UNION MOVED TO CORE. It was declared here and the public `NodeCompletedEvent.exit` was typed +`string`, so the contract permitted ids no consumer routes — and that failure is silent (the card +does not advance; nothing reports anything). A public contract cannot defer its vocabulary to one +of its producers, so `ImplementationExit` now lives beside the event that carries it, is checked +at the emit boundary against `IMPLEMENTATION_EXITS`, and is re-exported here for the call sites. + +What stays in the engine is POLICY, not contract: which of those endings are ones the EXECUTOR +performed rather than the graph. That is a statement about this engine's current ownership split, +it changes as U8 lands its routing moves, and core has no business knowing it. +*/ +import type { ImplementationExit as CoreImplementationExit } from "@fusion/core"; +export type { ImplementationExit } from "@fusion/core"; + +/** The exits where the EXECUTOR performs the lifecycle transition instead of the graph. */ +export const OUT_OF_BAND_IMPLEMENTATION_EXITS: readonly CoreImplementationExit[] = [ + "review-handoff-paused-after-completion", + "review-handoff-pending-review", +]; + +export function isOutOfBandImplementationExit(exit: CoreImplementationExit | undefined): boolean { + return exit !== undefined && OUT_OF_BAND_IMPLEMENTATION_EXITS.includes(exit); +} + +/** Reporter threaded into `runImplementation`; each instrumented exit calls it exactly once. */ +export type ImplementationExitReporter = (exit: CoreImplementationExit) => void;