From 038f802ba451c37015bb7dfb49b1a0f3eee5e60f Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sun, 23 Aug 2026 03:29:32 -0700 Subject: [PATCH] fix: make the workflow graph the only merge authority MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ProjectEngine's in-review auto-merge sweep was a second merge authority. It judged eligibility from column, status, steps and retry budget alone, with no idea where the card sat in its workflow graph, so it merged work the graph had never authorized: FN-9191 merged ~2s after fn_task_done, before Code Review had ever started, and FN-9193 merged while Code Review was re-running — the gate then requested revision and reset the steps, but the in-flight merge landed the pre-remediation branch anyway and left the card mergeConfirmed WITH incomplete steps, unfinalizable for five hours. - classifyMergeSweepAdmission (core) admits only merge-confirmed finalization, a card parked at a merge-region node, an interrupted attempt, or a fenced quiescent stall. Every initiation is fenced on satisfied pre-merge gates. - All four doors prove authority: the sweep, the 300ms column-entry handoff (which matches FN-9191's timing better than any sweep tick), the unpause re-enqueue, and a position-only pre-dispatch re-check for cards the graph moved out of the merge lane while they were queued. - workflow-merge-region.ts holds the canonical merge-region predicate; INTERPRETER_ENTRY_NODE_KINDS now aliases it so the two cannot drift. - Multi-repo: branch-group integration/promotion are merge-region nodes, an in-flight sub-repo land reads as foreign liveness, and a cross-node merge-dispatch lease defers. - Sweep reads are batched, so admission costs O(1) queries per poll. Co-Authored-By: Claude Opus 5 --- .changeset/merge-graph-authority.md | 7 + .../__tests__/merge-sweep-admission.test.ts | 192 ++++++++++ .../required-pre-merge-steps.test.ts | 45 +-- packages/core/src/index.ts | 17 +- .../core/src/merge/merge-sweep-admission.ts | 191 ++++++++++ .../src/merge/required-pre-merge-steps.ts | 21 -- packages/core/src/store.ts | 17 +- .../core/src/task-store/task-store-helpers.ts | 25 +- .../task-store/workflow-task-create-ops.ts | 31 ++ packages/core/src/workflows/workflow-ir.ts | 16 +- .../src/workflows/workflow-merge-region.ts | 56 +++ .../merge-sweep-graph-authority.test.ts | 254 +++++++++++++ ...ect-engine-auto-heal-lane-resolved.test.ts | 10 + .../src/__tests__/project-engine.test.ts | 98 ++--- packages/engine/src/project-engine.ts | 334 ++++++++++++++++-- 15 files changed, 1166 insertions(+), 148 deletions(-) create mode 100644 .changeset/merge-graph-authority.md create mode 100644 packages/core/src/__tests__/merge-sweep-admission.test.ts create mode 100644 packages/core/src/merge/merge-sweep-admission.ts create mode 100644 packages/core/src/workflows/workflow-merge-region.ts create mode 100644 packages/engine/src/__tests__/merge-sweep-graph-authority.test.ts diff --git a/.changeset/merge-graph-authority.md b/.changeset/merge-graph-authority.md new file mode 100644 index 0000000000..2f693faf8c --- /dev/null +++ b/.changeset/merge-graph-authority.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Auto-merge no longer merges a task before its workflow's code review has finished. +category: fix +dev: Every merge door — the in-review sweep, the 300ms column-entry handoff, the unpause re-enqueue, and the pre-dispatch check — is demoted from merge initiator to recovery servicer. `classifyMergeSweepAdmission` (core) admits a card only when it is merge-confirmed, parked at a merge-region node, recovering an interrupted attempt, or long-quiescent; a foreign live session always defers, and every initiation is fenced on satisfied pre-merge gates. Sweep reads are batched (`listWorkflowWorkItemsForTasks`, `getMergeRequestRecordsAsync`) so admission costs O(1) queries per poll rather than O(cards). Workspace and shared-branch-group cards resolve through the same rules — `branch-group-*` nodes are merge-region, and an in-flight sub-repo land reads as live. diff --git a/packages/core/src/__tests__/merge-sweep-admission.test.ts b/packages/core/src/__tests__/merge-sweep-admission.test.ts new file mode 100644 index 0000000000..45f88f8955 --- /dev/null +++ b/packages/core/src/__tests__/merge-sweep-admission.test.ts @@ -0,0 +1,192 @@ +import { describe, expect, it } from "vitest"; +import { + classifyMergeSweepAdmission, + DEFAULT_MERGE_SWEEP_QUIESCENCE_MS, + type MergeSweepAdmissionInput, +} from "../merge/merge-sweep-admission.js"; +import { BUILTIN_CODING_WORKFLOW_IR } from "../workflows/builtin-coding-workflow-ir.js"; +import { classifyWorkflowNodeMergeRegion, isMergeRegionNode } from "../workflows/workflow-merge-region.js"; + +/** A card the graph is holding mid-execution: nothing here may merge. */ +function input(overrides: Partial = {}): MergeSweepAdmissionInput { + return { + irTrust: "cards-own", + continuationPositions: ["outside-merge-region"], + continuationsReadable: true, + mergeConfirmed: false, + hasLiveSession: false, + interruptedMergeAttempt: false, + quiescentMs: 10 * 60_000, + gatesSatisfied: true, + ...overrides, + }; +} + +describe("classifyWorkflowNodeMergeRegion", () => { + it("separates the built-in coding workflow's merge region from its review lane", () => { + for (const nodeId of ["merge-gate", "merge-attempt", "merge-retry", "merge-manual-hold", "recovery-router"]) { + expect(classifyWorkflowNodeMergeRegion(BUILTIN_CODING_WORKFLOW_IR, nodeId)).toBe("merge-region"); + } + for (const nodeId of ["execute", "review", "planning"]) { + expect(classifyWorkflowNodeMergeRegion(BUILTIN_CODING_WORKFLOW_IR, nodeId)).toBe("outside-merge-region"); + } + }); + + /* Multi-repo: a shared-branch member's integration/promotion nodes ARE its merge lane. Excluding + them would freeze branch-group auto-merge recovery entirely. */ + it("treats branch-group integration and promotion as merge region", () => { + expect(classifyWorkflowNodeMergeRegion(BUILTIN_CODING_WORKFLOW_IR, "branch-group-member-integration")) + .toBe("merge-region"); + expect(classifyWorkflowNodeMergeRegion(BUILTIN_CODING_WORKFLOW_IR, "branch-group-promotion")) + .toBe("merge-region"); + }); + + it("reports an unknown node id rather than guessing", () => { + expect(classifyWorkflowNodeMergeRegion(BUILTIN_CODING_WORKFLOW_IR, "node-from-a-newer-ir")).toBe("unknown"); + }); + + /* Linear/seam workflows express merge as a prompt node; the kind set alone would miss them. */ + it("counts a legacy merge seam prompt node as merge region", () => { + expect(isMergeRegionNode({ kind: "prompt", config: { seam: "merge" } })).toBe(true); + expect(isMergeRegionNode({ kind: "prompt", config: { seam: "review" } })).toBe(false); + }); +}); + +describe("classifyMergeSweepAdmission", () => { + /* + FN-9193 SYMPTOM: the sweep merged a card whose Code Review was mid-re-run. Code Review then + requested revision and reset the steps, and the already-approved merge landed anyway — leaving + `mergeConfirmed` + incomplete steps, which nothing could finalize. + */ + it("refuses a card the graph is holding outside its merge region", () => { + expect(classifyMergeSweepAdmission(input({ continuationPositions: ["outside-merge-region"] }))) + .toEqual({ admit: false, reason: "not-at-merge-region-node" }); + }); + + it("refuses while any session is live, whatever the graph position", () => { + expect(classifyMergeSweepAdmission(input({ hasLiveSession: true, continuationPositions: ["merge-region"] }))) + .toEqual({ admit: false, reason: "live-session" }); + expect(classifyMergeSweepAdmission(input({ hasLiveSession: true, mergeConfirmed: true }))) + .toEqual({ admit: false, reason: "live-session" }); + }); + + it("admits a card parked at a merge-region node", () => { + expect(classifyMergeSweepAdmission(input({ continuationPositions: ["merge-region"] }))) + .toEqual({ admit: true, reason: "at-merge-region-node" }); + }); + + /* FN-9193's aftermath: the branch landed, so only finalization is left. */ + it("admits a confirmed merge for finalization even with the graph elsewhere", () => { + expect(classifyMergeSweepAdmission(input({ mergeConfirmed: true, continuationPositions: ["outside-merge-region"] }))) + .toEqual({ admit: true, reason: "merge-confirmed-finalization" }); + }); + + it("admits an interrupted merge attempt only when no continuation contradicts it", () => { + expect(classifyMergeSweepAdmission(input({ continuationPositions: [], interruptedMergeAttempt: true }))) + .toEqual({ admit: true, reason: "interrupted-merge-attempt" }); + // Stale merge residue must NOT override where the card is now — this is FN-9193's second pass. + expect(classifyMergeSweepAdmission(input({ + continuationPositions: ["outside-merge-region"], + interruptedMergeAttempt: true, + }))).toEqual({ admit: false, reason: "not-at-merge-region-node" }); + }); + + it("fails open for a drifted continuation node", () => { + expect(classifyMergeSweepAdmission(input({ continuationPositions: ["unknown"] }))) + .toEqual({ admit: true, reason: "drifted-continuation-node" }); + }); + + /* + A card with no stored selection runs on the PROJECT DEFAULT workflow, so that graph is its real + graph and its positions must still be honoured. Reviving a "no selection -> admit anything" + fail-open here would reopen the whole hole this classifier closes. + */ + it("honours graph position for a card running on the project default workflow", () => { + expect(classifyMergeSweepAdmission(input({ + irTrust: "effective-default", + continuationPositions: ["outside-merge-region"], + }))).toEqual({ admit: false, reason: "not-at-merge-region-node" }); + expect(classifyMergeSweepAdmission(input({ + irTrust: "effective-default", + continuationPositions: ["merge-region"], + }))).toEqual({ admit: true, reason: "at-merge-region-node" }); + }); + + describe("quiescent-stall fallback", () => { + const stalled = { continuationPositions: [] as const }; + + it("rescues a quiescent card whose gates are all satisfied", () => { + expect(classifyMergeSweepAdmission(input({ ...stalled, quiescentMs: DEFAULT_MERGE_SWEEP_QUIESCENCE_MS }))) + .toEqual({ admit: true, reason: "quiescent-stall-recovery" }); + }); + + it("refuses inside the quiescence floor — the FN-9193 race window", () => { + expect(classifyMergeSweepAdmission(input({ ...stalled, quiescentMs: 17_000 }))) + .toEqual({ admit: false, reason: "too-recent" }); + expect(classifyMergeSweepAdmission(input({ ...stalled, quiescentMs: DEFAULT_MERGE_SWEEP_QUIESCENCE_MS - 1 }))) + .toEqual({ admit: false, reason: "too-recent" }); + }); + + it("refuses when a pre-merge gate is unrun, pending, or failed — the FN-9191 race window", () => { + expect(classifyMergeSweepAdmission(input({ ...stalled, gatesSatisfied: false }))) + .toEqual({ admit: false, reason: "gates-unsatisfied" }); + }); + + it("treats an unparseable quiescence as maximally quiescent, not as zero", () => { + expect(classifyMergeSweepAdmission(input({ ...stalled, quiescentMs: Number.POSITIVE_INFINITY }))) + .toEqual({ admit: true, reason: "quiescent-stall-recovery" }); + }); + }); + + /* + FNXC:MergeAuthority 2026-08-23-20:05 — review findings #3-#6, pinned as behaviour. + */ + describe("review hardening", () => { + it("refuses to initiate when the continuation read failed (#4)", () => { + // Unreadable must not read as "nothing scheduled": that was the precondition for both + // remaining admit paths, so a transient DB error could admit a mid-execution card. + expect(classifyMergeSweepAdmission(input({ + continuationsReadable: false, + continuationPositions: [], + interruptedMergeAttempt: true, + }))).toEqual({ admit: false, reason: "continuations-unreadable" }); + }); + + it("still finalizes an already-landed merge when continuations are unreadable (#4)", () => { + // The branch is on the target already; refusing here re-creates FN-9193's unfinalizable card. + expect(classifyMergeSweepAdmission(input({ continuationsReadable: false, mergeConfirmed: true }))) + .toEqual({ admit: true, reason: "merge-confirmed-finalization" }); + }); + + it("fences EVERY initiation on satisfied gates, not just the quiescent path (#6)", () => { + // Without this the merge door's deferral re-enqueued the same card every sweep forever. + for (const overrides of [ + { continuationPositions: ["merge-region"] as const }, + { continuationPositions: ["unknown"] as const }, + { continuationPositions: [] as const, interruptedMergeAttempt: true }, + { irTrust: "effective-default" as const, continuationPositions: [] as const }, + ]) { + expect(classifyMergeSweepAdmission(input({ ...overrides, gatesSatisfied: false }))) + .toEqual({ admit: false, reason: "gates-unsatisfied" }); + } + // A landed merge is finalization, not initiation, so it is exempt. + expect(classifyMergeSweepAdmission(input({ mergeConfirmed: true, gatesSatisfied: false }))) + .toEqual({ admit: true, reason: "merge-confirmed-finalization" }); + }); + + it("does not trust an unresolved selection's node ids (#5)", () => { + // A named-but-unresolved workflow returns a DIFFERENT graph, so position is unusable — only + // the fenced quiescent path remains, and it needs quiescence, not a node classification. + expect(classifyMergeSweepAdmission(input({ + irTrust: "unresolved-selection", + continuationPositions: ["unknown"], + quiescentMs: 10 * 60_000, + }))).toEqual({ admit: true, reason: "unresolved-workflow-ir" }); + expect(classifyMergeSweepAdmission(input({ + irTrust: "unresolved-selection", + continuationPositions: ["unknown"], + quiescentMs: 5_000, + }))).toEqual({ admit: false, reason: "too-recent" }); + }); + }); +}); diff --git a/packages/core/src/__tests__/required-pre-merge-steps.test.ts b/packages/core/src/__tests__/required-pre-merge-steps.test.ts index 4e3b35f9b0..120cc033d2 100644 --- a/packages/core/src/__tests__/required-pre-merge-steps.test.ts +++ b/packages/core/src/__tests__/required-pre-merge-steps.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import { BUILTIN_CODING_WORKFLOW_IR } from "../workflows/builtin-coding-workflow-ir.js"; -import { findUnrunRequiredPreMergeStepIds, resolveRequiredPreMergeStepIds } from "../merge/required-pre-merge-steps.js"; +import { resolveRequiredPreMergeStepIds } from "../merge/required-pre-merge-steps.js"; describe("resolveRequiredPreMergeStepIds", () => { it("includes default-on pre-merge groups when no explicit selection exists", () => { @@ -23,46 +23,3 @@ describe("resolveRequiredPreMergeStepIds", () => { )).toEqual(new Set(["browser-verification"])); }); }); - -/* -FNXC:RequiredPreMergeSteps 2026-08-22-22:40 (FN-9191 wedge): -The auto-merge sweep's admission uses this to hold a card out of the merge queue until every -enabled pre-merge gate has reported. FN-9191's exact shape — both gates enabled, Plan Review -already passed, Code Review not yet started — must read as "unrun", and the same task after -Code Review lands must read as ready. -*/ -describe("findUnrunRequiredPreMergeStepIds", () => { - const planReviewPassed = { - workflowStepId: "plan-review", - workflowStepName: "Plan Review", - status: "passed" as const, - phase: "pre-merge" as const, - }; - const codeReviewPassed = { - workflowStepId: "code-review", - workflowStepName: "Code Review", - status: "passed" as const, - phase: "pre-merge" as const, - }; - - it("reports the FN-9191 window: Code Review enabled but not yet started", () => { - expect(findUnrunRequiredPreMergeStepIds(BUILTIN_CODING_WORKFLOW_IR, { - enabledWorkflowSteps: ["plan-review", "code-review"], - workflowStepResults: [planReviewPassed], - })).toEqual(["code-review"]); - }); - - it("reports nothing once every enabled gate has a result", () => { - expect(findUnrunRequiredPreMergeStepIds(BUILTIN_CODING_WORKFLOW_IR, { - enabledWorkflowSteps: ["plan-review", "code-review"], - workflowStepResults: [planReviewPassed, codeReviewPassed], - })).toEqual([]); - }); - - it("reports nothing when the gates are disabled for the task", () => { - expect(findUnrunRequiredPreMergeStepIds(BUILTIN_CODING_WORKFLOW_IR, { - enabledWorkflowSteps: [], - workflowStepResults: [], - })).toEqual([]); - }); -}); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 8cfce7f820..a4e00bc4dd 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -350,7 +350,22 @@ export { isWorkflowOptionalGroupEnabled, } from "./workflows/workflow-optional-steps.js"; export type { ResolvedWorkflowOptionalStep } from "./workflows/workflow-optional-steps.js"; -export { resolveRequiredPreMergeStepIds, findUnrunRequiredPreMergeStepIds } from "./merge/required-pre-merge-steps.js"; +export { resolveRequiredPreMergeStepIds } from "./merge/required-pre-merge-steps.js"; +export { + classifyMergeSweepAdmission, + DEFAULT_MERGE_SWEEP_QUIESCENCE_MS, +} from "./merge/merge-sweep-admission.js"; +export type { + MergeRegionPosition, + MergeSweepAdmission, + MergeSweepAdmissionInput, + MergeSweepAdmissionReason, +} from "./merge/merge-sweep-admission.js"; +export { + classifyWorkflowNodeMergeRegion, + isMergeRegionNode, + MERGE_REGION_ENTRY_NODE_KINDS, +} from "./workflows/workflow-merge-region.js"; export { applyPromptOverridesToIr, enumeratePromptBearingWorkflowNodes, diff --git a/packages/core/src/merge/merge-sweep-admission.ts b/packages/core/src/merge/merge-sweep-admission.ts new file mode 100644 index 0000000000..6fea2cd8d4 --- /dev/null +++ b/packages/core/src/merge/merge-sweep-admission.ts @@ -0,0 +1,191 @@ +/* +FNXC:MergeAuthority 2026-08-23-18:05 (FN-9191 + FN-9193 wedges): +ONE MERGE AUTHORITY. The workflow graph decides when a card merges: `code-review -> review -> +merge-gate -> ... -> merge-attempt`, and `merge-attempt` calls `requestInterpreterMerge`. The +engine's in-review auto-merge sweep is a SECOND caller that pushes ids into the same queue on its +own initiative, judging from column + status + steps + retry budget with no idea where the card sits +in its graph. Both recent wedges are that second authority firing early: + + FN-9191 — sweep merged ~2s after `fn_task_done`, before Code Review had ever started. + FN-9193 — sweep merged while Code Review was RE-running; the gate then requested revision, reset + the steps, and the in-flight merge landed the pre-remediation branch on main anyway. The + card was left `mergeConfirmed` WITH incomplete steps — a state nothing can finalize, so + it sat failed for five hours re-reading its own contradiction. + +This classifier demotes the sweep to a RECOVERY servicer: it may only re-drive a merge the graph +already authorized, or finalize one that already landed. It never initiates. A card that has not +reached its merge region is left to its graph, whatever its column says. + +Deliberately fails OPEN in three places, because a sweep that refuses everything strands cards with +no other driver: an unresolvable workflow (legacy/pre-graph rows), a continuation naming a node the +current IR no longer has (drifted IR), and a quiescent card whose gates are all satisfied. +*/ + +/** Where a card's active continuations sit relative to its workflow's merge region. */ +export type MergeRegionPosition = "merge-region" | "outside-merge-region" | "unknown"; + +export interface MergeSweepAdmissionInput { + /* + FNXC:MergeAuthority 2026-08-23-20:05 (review finding #5): + CAN WE TRUST THIS GRAPH'S NODE IDS FOR THIS CARD? `resolveWorkflowIrForTask` never returns null — + it degrades to `builtin:coding` — so the original `hasWorkflowIr: !!ir` was always true and the + legacy fail-open it guarded was dead code. Deleting that branch rather than reviving it is + deliberate: a card with no stored selection runs on the PROJECT DEFAULT workflow, so the resolved + graph is still its real graph and its positions are meaningful. Reviving the fail-open for that + case would admit every such card unconditionally — the exact hole this change closes. + + Only one case is genuinely untrustworthy: a selection that NAMES a workflow the store could not + resolve (missing or malformed). There the returned graph belongs to a different workflow, so every + node id classifies `"unknown"` and would ride the drifted-node fail-open straight to admit. + - `"cards-own"` — resolved from the card's own selection. + - `"effective-default"` — no selection stored; the default workflow IS this card's workflow. + - `"unresolved-selection"`— a named selection that did not resolve; positions are unusable. + */ + irTrust: "cards-own" | "effective-default" | "unresolved-selection"; + /** One entry per ACTIVE `kind:"task"` continuation, classified against the task's own IR. */ + continuationPositions: readonly MergeRegionPosition[]; + /* + FNXC:MergeAuthority 2026-08-23-20:05 (review finding #4): + UNREADABLE IS NOT EMPTY. A failed continuation read used to collapse to `[]`, which reads as + "nothing is scheduled" — the precondition for both remaining admit paths. A transient database + error could therefore admit a card whose graph was mid-execution. False here refuses initiation + outright; only an already-landed merge (finalization) outranks it. + */ + continuationsReadable: boolean; + /** `mergeDetails.mergeConfirmed` — the branch already landed; only finalization remains. */ + mergeConfirmed: boolean; + /** Any live session surface for this task: executor, workflow-step, AI merge, workspace repo lease. */ + hasLiveSession: boolean; + /* + FNXC:MergeAuthority 2026-08-23-20:05 (review finding #3): + Durable proof the graph started a merge for this card and was interrupted: an active/crash-left + merging status, or a live merge-request record. A consumed `mergeRetries` counter is deliberately + NOT proof — it survives a conflict bounce back to in-progress and a full re-implementation, so a + card that has since moved backward still carried it and was re-authorized by residue alone. + */ + interruptedMergeAttempt: boolean; + /** Milliseconds since the task last changed. Guards the quiescent-stall fallback. */ + quiescentMs: number; + /** False when the IR-aware merge door would refuse this card (unrun/pending/failed pre-merge gates). */ + gatesSatisfied: boolean; + /** Override for tests; production uses `DEFAULT_MERGE_SWEEP_QUIESCENCE_MS`. */ + quiescenceFloorMs?: number; +} + +export type MergeSweepAdmissionReason = + | "merge-confirmed-finalization" + | "continuations-unreadable" + | "unresolved-workflow-ir" + | "at-merge-region-node" + | "drifted-continuation-node" + | "interrupted-merge-attempt" + | "quiescent-stall-recovery" + | "live-session" + | "not-at-merge-region-node" + | "gates-unsatisfied" + | "too-recent"; + +export interface MergeSweepAdmission { + admit: boolean; + reason: MergeSweepAdmissionReason; +} + +/* +FNXC:MergeAuthority 2026-08-23-18:05: +Two minutes. The quiescent-stall fallback is the ONLY path by which the sweep may still start a +merge the graph did not ask for, so its floor has to sit well above the gap between two graph nodes +or it re-opens FN-9193. Measured on that task: `fn_task_done` -> Code Review start was 30s and 36s +on its two passes. Two minutes clears both by 3x while keeping recovery inside a few 15s sweeps. +It is a floor, not the whole guard — the fallback also requires no live session AND satisfied gates. +*/ +export const DEFAULT_MERGE_SWEEP_QUIESCENCE_MS = 2 * 60_000; + +/** + * Decide whether the in-review auto-merge sweep may enqueue this card. + * + * Pure and total: every input combination returns a reason, so the caller can log exactly why a card + * was held back. Order matters — see the inline notes. + */ +export function classifyMergeSweepAdmission(input: MergeSweepAdmissionInput): MergeSweepAdmission { + /* + Liveness first, above even a confirmed merge. The sweep has no deadline: if anything is actively + holding the card — an executor remediating, a review step running, an AI merge in flight, a + workspace sub-repo land — the right move is to let it finish and pick the card up next sweep. + FN-9193 died in exactly this window, with a Completion-summary session live. + */ + if (input.hasLiveSession) return { admit: false, reason: "live-session" }; + + /* + A landed branch outranks graph position: the merge ALREADY happened, so this is finalization, not + initiation. This is the one admission that repairs FN-9193's aftermath rather than preventing it, + and the only one that outranks an unreadable continuation read — refusing to finalize a merge that + is already on the target branch leaves the card in the unfinalizable state this change exists to + end. + */ + if (input.mergeConfirmed) return { admit: true, reason: "merge-confirmed-finalization" }; + + /* + FNXC:MergeAuthority 2026-08-23-20:05 (review finding #6): + UNIVERSAL GATE FENCE. Every remaining admission is an INITIATION, and no initiation may proceed + while an enabled pre-merge gate is unrun, pending, or failed — regardless of graph position. This + was previously fenced only on the quiescent path, which left the deferral loop open: a card + admitted for another reason reached the merge door, the door correctly refused with + `PreMergeStepsNotRunError`, the deferral wrote no status and burned no retry, and the next sweep + admitted it again — every 15s, indefinitely. Fencing here is what makes the deferral's "this + cannot spin" claim true. + */ + if (!input.gatesSatisfied) return { admit: false, reason: "gates-unsatisfied" }; + + // Unreadable continuations: we cannot prove where the graph is, so we do not initiate. (#4) + if (!input.continuationsReadable) return { admit: false, reason: "continuations-unreadable" }; + + /* + A named-but-unresolved selection means the resolved graph is NOT this card's workflow, so its node + ids cannot classify these continuations (#5). Fall through to the fenced quiescent path, which + needs no graph knowledge; refusing outright would strand every card whose workflow failed to load. + */ + if (input.irTrust === "unresolved-selection") { + return quiescentStallDecision(input, "unresolved-workflow-ir"); + } + + if (input.continuationPositions.length > 0) { + if (input.continuationPositions.includes("merge-region")) { + return { admit: true, reason: "at-merge-region-node" }; + } + // Drifted IR: a continuation names a node this workflow no longer has. Fail open. + if (input.continuationPositions.includes("unknown")) { + return { admit: true, reason: "drifted-continuation-node" }; + } + // The graph is holding this card somewhere that is not a merge node. Not ours to merge. + return { admit: false, reason: "not-at-merge-region-node" }; + } + + /* + An interrupted merge is graph-authorized work: the graph reached its merge node, the attempt was + cut short (engine restart, pause, transient failure), and the durable residue proves it. Reachable + only with NO active continuation, so a card that has since moved back out of the merge region — a + revision request, a bounce to in-progress — is judged by where it is NOW, not by the stale residue + of the attempt that preceded it. That ordering is what keeps FN-9193's second pass refused. + */ + if (input.interruptedMergeAttempt) { + return { admit: true, reason: "interrupted-merge-attempt" }; + } + + return quiescentStallDecision(input, "quiescent-stall-recovery"); +} + +/* +Quiescent-stall fallback. No continuation at all means nothing is scheduled to drive this card; +before this change the sweep rescued those, and dropping the rescue outright would trade a +premature-merge bug for a stalled-card bug. Kept, but fenced by all three conditions: nothing live +and gates satisfied (both checked by the caller above), plus a quiescence floor far longer than any +normal inter-node gap. +*/ +function quiescentStallDecision( + input: MergeSweepAdmissionInput, + reason: "quiescent-stall-recovery" | "unresolved-workflow-ir", +): MergeSweepAdmission { + const floorMs = input.quiescenceFloorMs ?? DEFAULT_MERGE_SWEEP_QUIESCENCE_MS; + if (!(input.quiescentMs >= floorMs)) return { admit: false, reason: "too-recent" }; + return { admit: true, reason }; +} diff --git a/packages/core/src/merge/required-pre-merge-steps.ts b/packages/core/src/merge/required-pre-merge-steps.ts index 329d5edfa2..a9ab28d1fd 100644 --- a/packages/core/src/merge/required-pre-merge-steps.ts +++ b/packages/core/src/merge/required-pre-merge-steps.ts @@ -24,24 +24,3 @@ export function resolveRequiredPreMergeStepIds( .map((step) => step.templateId), ); } - -/* -FNXC:RequiredPreMergeSteps 2026-08-22-22:40 (FN-9191 wedge): -Admission-side twin of the door's check. The in-review auto-merge sweep must be able to ask -"has every enabled pre-merge gate reported yet?" BEFORE it queues a card, because its sync -`canMergeTask` admission sees result rows only — and a gate that has not started has no row. -FN-9191 was queued ~2s after `fn_task_done` and ~18s before its own Code Review node started. -Shared with the door so the two can never answer differently. -*/ -/** Enabled pre-merge group ids that have produced no result row on this task yet. */ -export function findUnrunRequiredPreMergeStepIds( - ir: WorkflowIr, - task: { - enabledWorkflowSteps?: readonly string[]; - workflowStepResults?: ReadonlyArray<{ workflowStepId?: string }>; - }, -): string[] { - const results = task.workflowStepResults ?? []; - return [...resolveRequiredPreMergeStepIds(ir, task.enabledWorkflowSteps)] - .filter((workflowStepId) => !results.some((result) => result.workflowStepId === workflowStepId)); -} diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index d28d629170..9831b7e3e3 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -116,14 +116,14 @@ import { TASK_JSONB_COLUMNS, type TaskRow, type TaskPersistSerializationContext, import { pgRowToTaskRow as pgRowToTaskRowExternal, rowToTask as rowToTaskExternal, rowToBranchGroup as rowToBranchGroupExternal, generateBranchGroupId as generateBranchGroupIdExternal, computeTimedExecutionMs as computeTimedExecutionMsExternal, archiveEntryToTask as archiveEntryToTaskExternal, summarizeAgentLog as summarizeAgentLogExternal, rowToTaskDocument as rowToTaskDocumentExternal, rowToArtifact as rowToArtifactExternal, rowToTaskDocumentRevision as rowToTaskDocumentRevisionExternal, rowToGoalCitation as rowToGoalCitationExternal } from "./task-store/serialization.js"; import { moveTaskImpl, moveTaskIfImpl, handoffToReviewImpl, moveTaskInternalImpl, TerminalFailureApplyRejected, type MoveTaskIfResult } from "./task-store/moves.js"; import { resetTaskPublicationImpl } from "./task-store/reset-lifecycle.js"; -import { recordGoalCitationsImpl, insertTaskWithFtsRecoveryImpl2, assertTaskIdAvailableImpl, atomicWriteTaskJsonImpl2, createTaskWithDistributedReservationImpl, toStoredWorkflowStepImpl, ensureWorkflowStepForTemplateImpl, resolveEnabledWorkflowStepsImpl, setTaskBranchGroupImpl, getTaskColumnsImpl, prepareWorkflowMovePolicyPreflightImpl, updateTaskCustomFieldsImpl, listWorkflowPromptOverridesForProjectImpl, listWorkflowWorkItemsForTaskImpl, listDueWorkflowWorkItemsImpl, rewriteBlockedByResidueDependentsForRemovalImpl, getAllDocumentsImpl, deleteWorkflowStepImpl, toWorkflowDefinitionImpl, materializeDefaultWorkflowStepsImpl, reconcileTaskCustomFieldsForSchemaImpl, getTaskMovedCountsByDayImpl, getGoalStoreImpl, upsertTaskCommitAssociationImpl } from "./task-store/workflow-task-create-ops.js"; +import { recordGoalCitationsImpl, insertTaskWithFtsRecoveryImpl2, assertTaskIdAvailableImpl, atomicWriteTaskJsonImpl2, createTaskWithDistributedReservationImpl, toStoredWorkflowStepImpl, ensureWorkflowStepForTemplateImpl, resolveEnabledWorkflowStepsImpl, setTaskBranchGroupImpl, getTaskColumnsImpl, prepareWorkflowMovePolicyPreflightImpl, updateTaskCustomFieldsImpl, listWorkflowPromptOverridesForProjectImpl, listWorkflowWorkItemsForTaskImpl, listWorkflowWorkItemsForTasksImpl, listDueWorkflowWorkItemsImpl, rewriteBlockedByResidueDependentsForRemovalImpl, getAllDocumentsImpl, deleteWorkflowStepImpl, toWorkflowDefinitionImpl, materializeDefaultWorkflowStepsImpl, reconcileTaskCustomFieldsForSchemaImpl, getTaskMovedCountsByDayImpl, getGoalStoreImpl, upsertTaskCommitAssociationImpl } from "./task-store/workflow-task-create-ops.js"; import { applyLegacyWorkflowStepOverridesImpl, archiveDbImpl, assertNoDependencyCycleImpl, atomicCreateTaskJsonImpl, buildActiveTaskDependencyLookupImpl, buildArchivedAgentLogFieldsImpl, buildTaskIdIntegrityFallbackReportImpl, createBranchGroupImpl, dbImpl, detectAndCacheTaskIdIntegrityReportImpl, findLiveDependentsImpl, findLiveLineageChildrenImpl, getLegacyWorkflowStepSnapshotImpl, getMalformedTaskMetadataReasonImpl, getMergeQueuedTaskIdsAsyncImpl, insertRunAuditEventRowImpl, insertTaskImpl, invokeTaskCreatedHookImpl, isTaskArchivedAsyncImpl, isTaskArchivedImpl, isTaskIdPresentInArchivedTasksTableAsyncImpl, isTaskIdPresentInArchivedTasksTableImpl, logTaskCreateConflictImpl, maybeResolveTombstonedTaskIdImpl, mergeTaskIdIntegrityReportsImpl, optionalGroupIdSetImpl, patchTaskRowInTransactionImpl, readConfigFastImpl, readConfigImpl, readPromptForArchiveImpl, readTaskFromDbImpl, reconcileDistributedTaskIdStateOnOpenImpl, recordActivityFromListenerImpl, recordDependencyCycleRejectedAuditImpl, refreshTaskIdIntegrityReportImpl, resolveLocalNodeIdForTaskAllocationImpl, runTaskFtsWriteWithRecoveryImpl, scanAndRecordCitationsImpl, taskIdExistsAnywhereImpl, throwSoftDeletedWriteBlockedImpl, toBuiltInWorkflowStepImpl, trackDeferredTaskCreatedWorkImpl, upsertTaskImpl, withConfigLockImpl, withTaskLockImpl, withWorktreeAllocationLockImpl } from "./task-store/task-id-integrity.js"; import { claimNextToolFailureRetryImpl, createTaskVerificationRequestImpl, claimTaskVerificationRequestImpl, finishTaskVerificationRequestImpl, clearNearDuplicateReferencesToFailSoftImpl, clearWorkflowRunStepInstancesAsyncImpl, clearWorkflowRunStepInstancesImpl, computeMovedSettingsTargetWorkflowIdsImpl, ensureBranchGroupForSourceImpl, ensurePrEntityForSourceImpl, findRecentTasksByContentFingerprintImpl, getActiveMergingTaskImpl, getActivePrEntityBySourceImpl, getBranchGroupByBranchNameImpl, getBranchGroupBySourceImpl, getBranchGroupImpl, getBranchProgressByTaskImpl, getMutationsForRunImpl, getPrEntityByNumberImpl, getPrEntityImpl, getPrThreadStateImpl, getTasksByAssignedAgentImpl, getWorkflowPromptOverridesAsyncImpl, getWorkflowSettingValuesAsyncImpl, getWorkflowSettingValuesImpl, getWorkflowSettingsProjectIdImpl, getWorkflowWorkItemImpl, insertCompletionHandoffWorkflowWorkAuditImpl, listActivePrEntitiesImpl, listBranchGroupsImpl, listPrThreadStatesImpl, listTasksByBranchGroupImpl, listWorkflowSettingValuesForProjectImpl, loadWorkflowRunBranchesImpl, hasWorkflowRunStepInstancesForTaskImpl, loadWorkflowRunStepInstancesAsyncImpl, loadWorkflowRunStepInstancesImpl, markToolFailureRetryExhaustedAuditImpl, mergeCustomFieldPatchImpl, normalizeMergeRequestStateImpl, normalizeWorkflowWorkItemKindImpl, normalizeWorkflowWorkItemStateImpl, parseWorkflowPromptOverrideJsonImpl, recordPrThreadOutcomeImpl, resetAllStepsToPendingImpl, resetPromptCheckboxesImpl, resolveWorkflowMoveActorImpl, resolveWorkflowSettingDeclarationsImpl, saveWorkflowRunStepInstanceAsyncImpl, saveWorkflowRunStepInstanceImpl, transitionMergeRequestStateImpl, transitionWorkflowWorkItemSyncImpl, updateTaskImpl, updateWorkflowPromptOverridesImpl, upsertMergeRequestRecordImpl, workflowStateForMergeRequestStateImpl } from "./task-store/branch-and-pr-entities.js"; import { addPrInfoImpl, addSteeringCommentImpl, archiveAllDoneImpl, cleanupStaleMergeQueueRowsImpl, clearCompletionHandoffAcceptedMarkerImpl, clearDoneTransientFieldsImpl, clearStaleExecutionStartBranchReferencesImpl, deleteTaskCommentImpl, deleteTaskDocumentImpl, emitUsageEventImpl, enqueueMergeQueueImpl, getAgentLogCountImpl, getAgentLogsImpl, getArtifactImpl, getArtifactsImpl, getAttachmentImpl, getCompletionHandoffAcceptedMarkerImpl, getTaskDocumentImpl, getTaskDocumentRevisionsImpl, getTaskDocumentsImpl, insertArtifactRowImpl, linkGithubIssueImpl, listWorkflowWorkItemsForTaskSyncImpl, moveToDoneImpl, parseDependenciesFromPromptImpl, parseFileScopeFromPromptImpl, parseStepsFromPromptImpl, peekMergeQueueHeadImpl, peekMergeQueueImpl, readPreArchiveColumnFromTaskFileImpl, recordPluginActivationImpl, recordRunAuditEventBackendImpl, removePrInfoByNumberImpl, resolvePrimaryPrInfoImpl, resolveUnarchiveTargetColumnImpl, rewriteLineageChildrenForRemovalImpl, runGitCommandImpl, stopWatchingImpl, syncAgentTaskLinkOnReassignmentImpl, updateArtifactImpl, updateGithubTrackingImpl, updatePrInfoByNumberImpl, updateTaskCommentImpl, upsertPrInfoByNumberImpl, writeArtifactDataImpl } from "./task-store/task-artifacts-ops.js"; import { approveCliAutonomyImpl, approveWorkflowCliCommandImpl, cleanupOrphanedMaterializedStepsImpl, consumePluginGateVerdictsImpl, getAgentLogsByTimeRangeImpl, getDatabaseHealthImpl, getDistributedTaskIdAllocatorImpl, getExperimentSessionStoreImpl, getInReviewDurationEventsImpl, getMissionStoreImpl, getIdeationStoreImpl, getPluginStoreImpl, getSecretsStoreImpl, getSettingsSyncImpl, getTaskMergedTaskIdsImpl, getTaskWorkflowSelectionImpl, getImportTranslationImpl, recordImportTranslationImpl, pruneImportTranslationsImpl, type ImportTranslationCacheKey, type ImportTranslationCacheEntry, getVerificationCacheHitImpl, getWorkflowDefinitionImpl, healthCheckImpl, importLegacyAgentLogsOnceImpl, insertWorkflowDefinitionSyncImpl, isCliAutonomyApprovedImpl, isPluginInstalledImpl, isWorkflowCliCommandApprovedImpl, listWorkflowDefinitionsImpl, materializeExplicitWorkflowStepsImpl, materializeWorkflowStepsImpl, migrateActiveArchivedTasksToArchiveDbImpl, migrateLegacyArchiveEntriesToArchiveDbImpl, nextWorkflowDefinitionIdImpl, occupantsByColumnForWorkflowImpl, parseWorkflowLayoutImpl, pruneAgentLogFilesImpl, purgeTaskWorkflowSelectionRowsImpl, readAllWorkflowDefinitionsImpl, readRawProjectSettingsImpl, recordPluginGateVerdictImpl, recordVerificationCachePassImpl, removeMaterializedSelectionImpl, resolvePluginWorkflowStepImpl, resolveTaskWorkflowIrSyncImpl, revokeCliAutonomyImpl, selectTaskWorkflowAndReconcileImpl, writeTaskWorkflowSelectionImpl, getTaskWorkflowSelectionAsyncImpl, getTaskWorkflowSelectionsAsyncImpl, } from "./task-store/workflow-definitions.js"; import { getTaskCommitAssociationsByLineageIdImpl, replaceLegacyTaskCommitAssociationsImpl } from "./task-store/task-commit-associations.js"; import { findRecentTasksBySourceParentTaskIdImpl } from "./task-store/branch-and-pr-entities.js"; -import { addTaskCommentImpl, applyBuiltInPromptOverridesAsyncImpl, applyBuiltInPromptOverridesSyncImpl, areAllDependenciesDoneImpl, artifactStoredNameImpl, assertWorkflowIrTraitsValidImpl, clearActivityLogImpl, clearTaskWorkflowSelectionImpl, deleteTaskByIdImpl, getDefaultWorkflowIdImpl, resolveOriginWorkflowOverrideIdImpl, type TaskOriginWorkflowKind, getInsightStoreImpl, getMergeQueuedTaskIdsImpl, getMergeRequestRecordImpl, getMergeRequestRecordAsyncImpl, getResearchStoreImpl, getTaskIdFromDirImpl, getTodoStoreImpl, getWorkflowWorkItemByIdentityImpl, hasActiveTaskImpl, invalidateConfigCacheAfterMigrationImpl, isTaskIdConflictErrorImpl, listLegacyAutoMergeStampCandidatesImpl, readTaskRowFromDbImpl, recordBranchGroupMemberLandedImpl, refreshDatabaseHealthAsyncImpl, refreshDatabaseHealthImpl, resolveTaskCustomFieldDefsSyncImpl, resolveWorkflowBypassGuardsImpl, serializeConfigForDiskImpl, setPluginWorkflowStepTemplatesImpl, shouldSkipWorkflowMovePoliciesImpl, suppressWatcherImpl, upsertTaskWithFtsRecoveryImpl } from "./task-store/task-store-helpers.js"; +import { addTaskCommentImpl, applyBuiltInPromptOverridesAsyncImpl, applyBuiltInPromptOverridesSyncImpl, areAllDependenciesDoneImpl, artifactStoredNameImpl, assertWorkflowIrTraitsValidImpl, clearActivityLogImpl, clearTaskWorkflowSelectionImpl, deleteTaskByIdImpl, getDefaultWorkflowIdImpl, resolveOriginWorkflowOverrideIdImpl, type TaskOriginWorkflowKind, getInsightStoreImpl, getMergeQueuedTaskIdsImpl, getMergeRequestRecordImpl, getMergeRequestRecordAsyncImpl, getMergeRequestRecordsAsyncImpl, getResearchStoreImpl, getTaskIdFromDirImpl, getTodoStoreImpl, getWorkflowWorkItemByIdentityImpl, hasActiveTaskImpl, invalidateConfigCacheAfterMigrationImpl, isTaskIdConflictErrorImpl, listLegacyAutoMergeStampCandidatesImpl, readTaskRowFromDbImpl, recordBranchGroupMemberLandedImpl, refreshDatabaseHealthAsyncImpl, refreshDatabaseHealthImpl, resolveTaskCustomFieldDefsSyncImpl, resolveWorkflowBypassGuardsImpl, serializeConfigForDiskImpl, setPluginWorkflowStepTemplatesImpl, shouldSkipWorkflowMovePoliciesImpl, suppressWatcherImpl, upsertTaskWithFtsRecoveryImpl } from "./task-store/task-store-helpers.js"; import { getTaskSelectClauseImpl2, createTaskPersistSerializationContextImpl, getTaskPersistValuesImpl, getTaskPatchDescriptorsImpl, normalizeTaskFromDiskImpl, writeTaskJsonFileImpl, rowToPrEntityImpl, generatePrEntityIdImpl, readTaskForMoveImpl, rowToMergeQueueEntryImpl, rowToMergeRequestRecordImpl, rowToCompletionHandoffMarkerImpl, rowToWorkflowWorkItemImpl, rowToRunAuditEventImpl } from "./task-store/task-row-mappers.js"; import { getTaskSelectClauseWithActivityLogLimitImpl, getChangedTaskColumnsImpl, getSoftDeletedWriteConflictImpl, readTaskJsonImpl, writeConfigImpl, _maybeAutoArchiveSameAgentDuplicateBackendImpl, updateBranchGroupImpl, updatePrEntityImpl, listTasksForGithubTrackingReconcileImpl, listTasksForGitlabTrackingReconcileImpl, renewCheckoutLeaseImpl, updateTaskAtomicImpl, linkTaskRecommendationImpl, normalizeWorkspaceTaskWorktreeMetadataImpl, mergeWorkspaceWorktreeEntryImpl, updateTaskRepositoryScopeImpl, updateWorkspaceReviewStateImpl, resolveTaskWedgeNotificationEpisodeImpl, getWorkflowPromptOverridesImpl, updateWorkflowSettingValuesImpl, rollbackConfigurationImpl, cancelActiveWorkflowWorkItemsForTaskImpl, setCompletionHandoffAcceptedMarkerImpl, reconcileLegacyAutoMergeStampsImpl, recoverExpiredMergeQueueLeasesImpl, rewriteDependentsForRemovalImpl, cleanupBranchForTaskImpl, addAttachmentImpl, deleteAttachmentImpl, registerArtifactImpl, updatePrInfoImpl, unlinkGithubIssueImpl, cleanupArchivedTasksImpl, generatePromptFromArchiveEntryImpl, listWorkflowOccupantTaskIdsImpl, listApprovedCliAutonomyAdaptersImpl, closeImpl, getActivityLogImpl } from "./task-store/task-mutation-ops.js"; import { getOrCreateForProjectImpl, listGoalCitationsImpl, atomicWriteTaskJsonWithAuditImpl, type PlanningDependencyInvalidation, duplicateTaskImpl, listStrandedRefinementsImpl, tryClaimCheckoutImpl, evaluateWorkflowMovePoliciesImpl, recordRunAuditEventImpl, getRunAuditEventsImpl, dequeueMergeQueueOnColumnExitImpl, updateIssueInfoImpl, listWorkflowStepsImpl, getWorkflowStepImpl, createWorkflowDefinitionImpl, countActiveInCapacitySlotSyncImpl, countActiveInCapacitySlotAsyncImpl, generateSpecifiedPromptImpl, recordActivityImpl, getEvalStoreImpl } from "./task-store/project-store-ops.js"; @@ -2584,6 +2584,11 @@ export class TaskStore extends EventEmitter { async getMergeRequestRecordAsync(taskId: string): Promise { return getMergeRequestRecordAsyncImpl(this, taskId); } + + /** Batched `getMergeRequestRecordAsync` — one query for many tasks (FNXC:MergeAuthority 2026-08-23-20:05). */ + async getMergeRequestRecordsAsync(taskIds: readonly string[]): Promise> { + return getMergeRequestRecordsAsyncImpl(this, taskIds); + } async projectMergeRequestToWorkflowWorkItem( taskId: string, opts: MergeRequestWorkflowProjectionOptions = {}, ): Promise { return projectMergeRequestToWorkflowWorkItemImpl(this, taskId, opts); } @@ -2629,6 +2634,14 @@ export class TaskStore extends EventEmitter { return listWorkflowWorkItemsForTaskImpl(this, taskId, opts); } + /** Batched `listWorkflowWorkItemsForTask` — one query for many tasks (FNXC:MergeAuthority 2026-08-23-20:05). */ + async listWorkflowWorkItemsForTasks( + taskIds: readonly string[], + opts: { kinds?: WorkflowWorkItemKind[] } = {}, + ): Promise> { + return listWorkflowWorkItemsForTasksImpl(this, taskIds, opts); + } + /** * FNXC:RuntimeWorkflowAsync 2026-06-24-17:12: */ diff --git a/packages/core/src/task-store/task-store-helpers.ts b/packages/core/src/task-store/task-store-helpers.ts index 5f38a2aa6d..f87f8ee41c 100644 --- a/packages/core/src/task-store/task-store-helpers.ts +++ b/packages/core/src/task-store/task-store-helpers.ts @@ -22,7 +22,7 @@ import { ResearchStore } from "../research/research-store.js"; import { parseWorkflowIr } from "../workflows/workflow-ir.js"; import { columnsWithFlag } from "../workflows/workflow-lifecycle-traits.js"; import { type TaskRow } from "./persistence.js"; -import { eq } from "drizzle-orm"; +import { eq, inArray } from "drizzle-orm"; import * as schema from "../postgres/schema/index.js"; import { MergeRequestRow, WorkflowWorkItemRow } from "./row-types.js"; import { TodoStore } from "../stores/todo-store.js"; @@ -222,6 +222,29 @@ export async function getMergeRequestRecordAsyncImpl(store: TaskStore, taskId: s return row ? store.rowToMergeRequestRecord(row) : null; } +/* +FNXC:MergeAuthority 2026-08-23-20:05 (review finding #10): +BATCHED sibling of `getMergeRequestRecordAsyncImpl` for the in-review merge sweep, which needs one +merge-request state per review-lane card per poll. One `inArray` replaces N single-row selects. +*/ +export async function getMergeRequestRecordsAsyncImpl( + store: TaskStore, + taskIds: readonly string[], +): Promise> { + const byTask = new Map(); + if (taskIds.length === 0) return byTask; + const layer = store.asyncLayer!; + const rows = await layer.db + .select() + .from(schema.project.mergeRequests) + .where(inArray(schema.project.mergeRequests.taskId, [...taskIds])); + for (const row of rows as MergeRequestRow[]) { + const record = store.rowToMergeRequestRecord(row); + byTask.set(record.taskId, record); + } + return byTask; +} + export function getWorkflowWorkItemByIdentityImpl(store: TaskStore, runId: string, taskId: string, diff --git a/packages/core/src/task-store/workflow-task-create-ops.ts b/packages/core/src/task-store/workflow-task-create-ops.ts index 510ce20595..037cdf58ab 100644 --- a/packages/core/src/task-store/workflow-task-create-ops.ts +++ b/packages/core/src/task-store/workflow-task-create-ops.ts @@ -449,6 +449,37 @@ export async function listWorkflowWorkItemsForTaskImpl(store: TaskStore, taskId: return (rows as WorkflowWorkItemRow[]).map((row) => store.rowToWorkflowWorkItem(row)); } +/* +FNXC:MergeAuthority 2026-08-23-20:05 (FN-9191/FN-9193 follow-up, review finding #10): +BATCHED sibling of `listWorkflowWorkItemsForTaskImpl`. The in-review merge sweep asks "where is this +card's graph?" for EVERY review-lane card on every 15s poll; per-task reads made that 1 query per +card per poll. One `inArray` over the candidate ids answers the whole sweep in a single round trip. +Returns a Map keyed by taskId so a caller can look up a card with no result (empty array) without +distinguishing it from a card that was never asked about. +*/ +export async function listWorkflowWorkItemsForTasksImpl( + store: TaskStore, + taskIds: readonly string[], + opts: { kinds?: WorkflowWorkItemKind[] } = {}, +): Promise> { + const byTask = new Map(); + for (const taskId of taskIds) byTask.set(taskId, []); + if (taskIds.length === 0) return byTask; + const layer = store.asyncLayer!; + const scope = projectScopeFor(schema.project.workflowWorkItems.projectId, layer.projectId); + const conditions = [scope, inArray(schema.project.workflowWorkItems.taskId, [...taskIds])]; + if (opts.kinds?.length) conditions.push(inArray(schema.project.workflowWorkItems.kind, opts.kinds)); + const rows = await layer.db + .select() + .from(schema.project.workflowWorkItems) + .where(and(...conditions)); + for (const row of rows as WorkflowWorkItemRow[]) { + const item = store.rowToWorkflowWorkItem(row); + byTask.get(item.taskId)?.push(item); + } + return byTask; +} + export async function listDueWorkflowWorkItemsImpl(store: TaskStore, filter: WorkflowWorkItemDueFilter = {}): Promise { const layer = store.asyncLayer!; return listDueWorkflowWorkItemsAsync(layer.db, filter); diff --git a/packages/core/src/workflows/workflow-ir.ts b/packages/core/src/workflows/workflow-ir.ts index 5aa6457ad9..d121e1c308 100644 --- a/packages/core/src/workflows/workflow-ir.ts +++ b/packages/core/src/workflows/workflow-ir.ts @@ -16,6 +16,7 @@ import type { WorkflowSettingType, } from "./workflow-ir-types.js"; import { classifyWorkflowAgentNode } from "./workflow-ir-types.js"; +import { MERGE_REGION_ENTRY_NODE_KINDS } from "./workflow-merge-region.js"; import { getWorkflowExtensionRegistry } from "./workflow-extension-registry.js"; import type { WorkflowExtensionConfigField } from "./workflow-extension-types.js"; import { THINKING_LEVELS } from "../types.js"; @@ -317,18 +318,9 @@ function reachableFrom( return seen; } -const INTERPRETER_ENTRY_NODE_KINDS: ReadonlySet = new Set([ - "merge-gate", - "merge-attempt", - "manual-merge-hold", - "retry-backoff", - "recovery-router", - "branch-group-member-integration", - "branch-group-promotion", - "pr-create", - "pr-respond", - "pr-merge", -]); +/* FNXC:MergeAuthority 2026-08-23-18:05: same membership as the canonical merge-region set — one + spelling, so interpreter entry and merge-sweep admission can never drift apart. */ +const INTERPRETER_ENTRY_NODE_KINDS: ReadonlySet = MERGE_REGION_ENTRY_NODE_KINDS; /* FNXC:WorkflowValidation 2026-07-18-22:10: diff --git a/packages/core/src/workflows/workflow-merge-region.ts b/packages/core/src/workflows/workflow-merge-region.ts new file mode 100644 index 0000000000..2f04f01088 --- /dev/null +++ b/packages/core/src/workflows/workflow-merge-region.ts @@ -0,0 +1,56 @@ +import type { WorkflowIr, WorkflowIrNode, WorkflowIrNodeKind } from "./workflow-ir-types.js"; + +/* +FNXC:MergeAuthority 2026-08-23-18:05 (FN-9193 wedge): +THE MERGE REGION IS THE GRAPH'S MERGE AUTHORITY. A card whose active continuation sits on one of +these nodes has been authorized by its workflow to merge; a card anywhere else has not, no matter +what its column, steps, or status say. + +This set is the canonical spelling. `INTERPRETER_ENTRY_NODE_KINDS` (workflow-ir.ts) is the same +membership and now re-exports it. Two NEARBY sets are deliberately different and must not be folded +in: `MERGE_CLASS_NODE_KINDS` (save-time merge-blocker reachability) drops `pr-create`/`pr-respond` +because neither clears a merge-blocker gate, and `MERGE_REGION_NODE_KINDS` +(workflow-lifecycle-validation.ts) drops every `pr-*` kind because it describes the engine-owned +policy region only. Admission needs the widest reading — a card parked at `pr-respond` IS inside its +workflow's merge lane and its merge may legitimately be re-driven. +*/ +export const MERGE_REGION_ENTRY_NODE_KINDS: ReadonlySet = new Set([ + "merge-gate", + "merge-attempt", + "manual-merge-hold", + "retry-backoff", + "recovery-router", + "branch-group-member-integration", + "branch-group-promotion", + "pr-create", + "pr-respond", + "pr-merge", +]); + +/** + * True when a node belongs to its workflow's merge region. + * + * `config.seam === "merge"` is included because linear built-ins and custom seam workflows express + * their merge as a `prompt` node rather than a `merge-attempt` kind; omitting it would classify + * every seam-workflow card as "not at a merge node" and freeze their auto-merge recovery. + */ +export function isMergeRegionNode(node: Pick): boolean { + return MERGE_REGION_ENTRY_NODE_KINDS.has(node.kind) || node.config?.seam === "merge"; +} + +/** + * Resolve whether `nodeId` names a merge-region node in `ir`. + * + * Returns `"unknown"` when the id is absent from the graph — an IR that drifted under a live + * continuation. Callers must fail OPEN on `"unknown"`: refusing a card whose node we cannot even + * find would strand it with no other driver. + */ +export function classifyWorkflowNodeMergeRegion( + ir: WorkflowIr, + nodeId: string, +): "merge-region" | "outside-merge-region" | "unknown" { + if (ir.version !== "v2" || !Array.isArray(ir.nodes)) return "unknown"; + const node = ir.nodes.find((candidate) => candidate.id === nodeId); + if (!node) return "unknown"; + return isMergeRegionNode(node) ? "merge-region" : "outside-merge-region"; +} diff --git a/packages/engine/src/__tests__/merge-sweep-graph-authority.test.ts b/packages/engine/src/__tests__/merge-sweep-graph-authority.test.ts new file mode 100644 index 0000000000..f517abd5bf --- /dev/null +++ b/packages/engine/src/__tests__/merge-sweep-graph-authority.test.ts @@ -0,0 +1,254 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const testState = vi.hoisted(() => ({ currentStore: null as any })); + +vi.mock("../runtimes/in-process-runtime.js", () => ({ + InProcessRuntime: vi.fn().mockImplementation(function () { + return { + start: vi.fn(async () => undefined), + stop: vi.fn(async () => undefined), + getTaskStore: () => testState.currentStore, + getAgentStore: vi.fn(), + getMessageStore: vi.fn(), + getRoutineStore: vi.fn(), + getRoutineRunner: vi.fn(), + getHeartbeatMonitor: vi.fn(), + getTriggerScheduler: vi.fn(), + configurePrMonitoring: vi.fn(), + setActiveMergeTaskIdProvider: vi.fn(), + setActiveMergeStartedAtMsProvider: vi.fn(), + setActiveMergeAborter: vi.fn(), + setMergeEnqueuer: vi.fn(), + setMergeActiveClearer: vi.fn(), + setMergePendingProvider: vi.fn(), + setMergeRequester: vi.fn(), + resumeAfterUnpause: vi.fn(async () => undefined), + getPluginRunner: vi.fn(() => undefined), + }; + }), +})); + +import type { Task } from "@fusion/core"; +import { activeSessionRegistry, executingTaskLock } from "../agents/active-session-registry.js"; +import { ProjectEngine } from "../project-engine.js"; + +/* +FNXC:MergeAuthority 2026-08-23-18:05 (FN-9191 + FN-9193 wedges): +SYMPTOM these pin: the in-review auto-merge sweep merged cards its workflow had not authorized. +FN-9191 — merged ~2s after `fn_task_done`, before Code Review had ever started. +FN-9193 — merged while Code Review was RE-running; the gate then requested revision and reset the +steps, the in-flight merge landed the pre-remediation branch on main, and the card was left +`mergeConfirmed` WITH incomplete steps, which nothing could finalize for five hours. + +ASSERTION: the sweep enqueues only cards whose graph reached the merge region, or that are +recovering an already-authorized merge. It never initiates one. +*/ + +const AN_HOUR_AGO = () => new Date(Date.now() - 60 * 60_000).toISOString(); + +function inReview(id: string, overrides: Partial = {}): Task { + return { + id, + column: "in-review", + paused: false, + mergeRetries: 0, + status: null, + steps: [], + enabledWorkflowSteps: [], + updatedAt: AN_HOUR_AGO(), + ...overrides, + } as unknown as Task; +} + +/** `nodeId` names an ACTIVE `kind:"task"` continuation — where the graph is holding the card. */ +function makeStore(continuationsByTask: Record = {}) { + return { + getSettings: vi.fn(async () => ({ autoMerge: true, globalPause: false, enginePaused: false })), + getTask: vi.fn(async () => null), + listWorkflowWorkItemsForTask: vi.fn(async (taskId: string) => + (continuationsByTask[taskId] ?? []).map((nodeId, i) => ({ + id: `${taskId}-wi-${i}`, + taskId, + nodeId, + kind: "task", + state: "held", + })), + ), + getBranchGroup: vi.fn(async () => ({ status: "open", branchName: "main" })), + getTaskWorkflowSelection: () => undefined, + getTaskWorkflowSelectionAsync: async () => undefined, + on: vi.fn(), + off: vi.fn(), + emit: vi.fn(), + }; +} + +function createEngine(store: ReturnType) { + testState.currentStore = store; + const engine = new ProjectEngine( + { + projectId: "proj_test", + workingDirectory: "/tmp/proj_test", + isolationMode: "in-process", + maxConcurrent: 2, + maxWorktrees: 2, + }, + {} as never, + { skipNotifier: true }, + ) as any; + const enqueueSpy = vi.spyOn(engine, "internalEnqueueMerge").mockImplementation(() => true); + const sweep = (tasks: Task[]): Promise => + engine.enqueueEligibleInReviewTasks(tasks, { autoMerge: true }); + return { engine, enqueueSpy, sweep }; +} + +describe("auto-merge sweep respects the graph's merge authority", () => { + beforeEach(() => { + vi.clearAllMocks(); + executingTaskLock._clearForTest(); + for (const path of activeSessionRegistry.pathsForTask("FN-workspace")) { + activeSessionRegistry.unregisterPath(path); + } + }); + + it("holds a card the graph is holding at Code Review (FN-9193)", async () => { + const store = makeStore({ "FN-9193": ["code-review"] }); + const { enqueueSpy, sweep } = createEngine(store); + + expect(await sweep([inReview("FN-9193")])).toBe(0); + expect(enqueueSpy).not.toHaveBeenCalled(); + }); + + it("admits a card parked at its merge-attempt node", async () => { + const store = makeStore({ "FN-merge": ["merge-attempt"] }); + const { enqueueSpy, sweep } = createEngine(store); + + expect(await sweep([inReview("FN-merge")])).toBe(1); + expect(enqueueSpy).toHaveBeenCalledWith("FN-merge"); + }); + + /* MULTI-REPO: a shared-branch member's integration node is its merge lane. */ + it("admits a shared-branch member parked at branch-group-member-integration", async () => { + const store = makeStore({ "FN-member": ["branch-group-member-integration"] }); + const { enqueueSpy, sweep } = createEngine(store); + const member = inReview("FN-member", { + branchContext: { assignmentMode: "shared", groupId: "BG-1", source: "mission" }, + } as Partial); + + expect(await sweep([member])).toBe(1); + expect(enqueueSpy).toHaveBeenCalledWith("FN-member"); + }); + + /* MULTI-REPO: a workspace task mid sub-repo land registers its repo path; never dispatch over it. */ + it("holds a workspace task while a sub-repo land session is live", async () => { + const store = makeStore({ "FN-workspace": ["merge-attempt"] }); + const { enqueueSpy, sweep } = createEngine(store); + activeSessionRegistry.registerPath("/tmp/proj_test/repos/api", { + taskId: "FN-workspace", + kind: "workspace-repo-land", + } as never); + + try { + expect(await sweep([inReview("FN-workspace")])).toBe(0); + expect(enqueueSpy).not.toHaveBeenCalled(); + } finally { + activeSessionRegistry.unregisterPath("/tmp/proj_test/repos/api"); + } + }); + + /* FN-9193's aftermath: the branch landed, so finalization must still be reachable. */ + it("admits a confirmed merge for finalization even with incomplete steps", async () => { + const store = makeStore({ "FN-landed": ["execute"] }); + const { enqueueSpy, sweep } = createEngine(store); + const landed = inReview("FN-landed", { + steps: [{ status: "pending" }], + mergeDetails: { mergeConfirmed: true, commitSha: "eaa1d47c" }, + } as unknown as Partial); + + expect(await sweep([landed])).toBe(1); + expect(enqueueSpy).toHaveBeenCalledWith("FN-landed"); + }); + + /* FN-9191: marked done seconds ago, gates never ran, nothing scheduled yet. */ + it("holds a just-completed card whose pre-merge gates have not run", async () => { + const store = makeStore({}); + const { enqueueSpy, sweep } = createEngine(store); + const justDone = inReview("FN-9191", { + enabledWorkflowSteps: ["plan-review", "code-review"], + updatedAt: new Date().toISOString(), + }); + + expect(await sweep([justDone])).toBe(0); + expect(enqueueSpy).not.toHaveBeenCalled(); + }); + + it("still rescues a long-quiescent card with nothing scheduled and gates satisfied", async () => { + const store = makeStore({}); + const { enqueueSpy, sweep } = createEngine(store); + + expect(await sweep([inReview("FN-stalled")])).toBe(1); + expect(enqueueSpy).toHaveBeenCalledWith("FN-stalled"); + }); + + /* + FNXC:MergeAuthority 2026-08-23-20:05 (review findings #1, #2, #9): + THE SWEEP WAS NEVER THE ONLY DOOR. Three other paths reach `internalEnqueueMerge` or dispatch a + queued merge; gating only the sweep left the fastest one (a 300ms column-entry handoff, which + matches FN-9191's observed timing better than any 15s sweep tick) wide open. + */ + describe("every merge door proves graph authority", () => { + it("holds the 300ms column-entry handoff for a card at Code Review (#1)", async () => { + const store = makeStore({ "FN-handoff": ["code-review"] }); + const { engine, enqueueSpy } = createEngine(store); + const task = inReview("FN-handoff"); + store.getTask.mockResolvedValue(task); + + await (engine as any).classifyMergeSweepCandidate(task, new Map(), undefined, { ignoreOwnMergePipeline: true }) + .then((admission: { admit: boolean; reason: string }) => { + expect(admission).toEqual({ admit: false, reason: "not-at-merge-region-node" }); + }); + expect(enqueueSpy).not.toHaveBeenCalled(); + }); + + it("admits the handoff once the graph reaches its merge region (#1)", async () => { + const store = makeStore({ "FN-handoff": ["merge-attempt"] }); + const { engine } = createEngine(store); + const admission = await (engine as any).classifyMergeSweepCandidate( + inReview("FN-handoff"), new Map(), undefined, { ignoreOwnMergePipeline: true }, + ); + expect(admission).toEqual({ admit: true, reason: "at-merge-region-node" }); + }); + + /* FN-3900: a card whose own merge is already queued/running must still be (re-)enqueued by the + leaked-mergeActive rescue — that is dedupe, not someone else's work. */ + it("does not treat this card's own in-flight merge as foreign liveness (#1)", async () => { + const store = makeStore({ "FN-busy": ["merge-attempt"] }); + const { engine } = createEngine(store); + (engine as any).activeMergeTaskId = "FN-busy"; + + expect(await (engine as any).classifyMergeSweepCandidate(inReview("FN-busy"), new Map())) + .toEqual({ admit: false, reason: "live-session" }); + expect(await (engine as any).classifyMergeSweepCandidate( + inReview("FN-busy"), new Map(), undefined, { ignoreOwnMergePipeline: true }, + )).toEqual({ admit: true, reason: "at-merge-region-node" }); + }); + + it("refuses to dispatch a queued card the graph moved out of the merge region (#2)", async () => { + const store = makeStore({ "FN-bounced": ["code-review"] }); + const { engine } = createEngine(store); + expect(await (engine as any).isDispatchStillGraphAuthorized(inReview("FN-bounced"))).toBe(false); + }); + + it("fails the dispatch guard OPEN when position is unknowable (#2)", async () => { + // No continuation at all is the interrupted-merge / quiescent-recovery shape, and an + // unreadable read is the door's problem, not this guard's. + const { engine: noContinuation } = createEngine(makeStore({})); + expect(await (noContinuation as any).isDispatchStillGraphAuthorized(inReview("FN-none"))).toBe(true); + + const throwing = makeStore({}); + throwing.listWorkflowWorkItemsForTask = vi.fn(async () => { throw new Error("db down"); }); + const { engine: unreadable } = createEngine(throwing); + expect(await (unreadable as any).isDispatchStillGraphAuthorized(inReview("FN-unreadable"))).toBe(true); + }); + }); +}); diff --git a/packages/engine/src/__tests__/project-engine-auto-heal-lane-resolved.test.ts b/packages/engine/src/__tests__/project-engine-auto-heal-lane-resolved.test.ts index 2de82fb301..5dd4a4e0fa 100644 --- a/packages/engine/src/__tests__/project-engine-auto-heal-lane-resolved.test.ts +++ b/packages/engine/src/__tests__/project-engine-auto-heal-lane-resolved.test.ts @@ -52,6 +52,8 @@ function healableTask(column: string, id = "FN-1"): Task { id, column, mergeRetries: 5, + enabledWorkflowSteps: [], + updatedAt: new Date(Date.now() - 60 * 60_000).toISOString(), error: "Deterministic test verification failed", log: [{ action: "[verification] test command failed (exit 0) — output exceeded buffer" }], dependencies: [], @@ -129,6 +131,14 @@ describe("the in-review enqueue sweep resolves each card's own review lane", () allowInReviewMergeProcessing: vi.fn(async () => true), internalEnqueueMerge: (id: string) => enqueued.push(id), canMergeTask: ProjectEngine.prototype["canMergeTask" as keyof ProjectEngine], + /* FNXC:MergeAuthority 2026-08-23-18:05: the sweep now also proves graph authority per card. + These retry-exhausted cards are admitted as `interrupted-merge-attempt` (mergeRetries > 0), + and the classifier reuses the SAME IR cache, so the one-read contract below still holds. */ + classifyMergeSweepCandidate: + ProjectEngine.prototype["classifyMergeSweepCandidate" as keyof ProjectEngine], + loadMergeSweepBatch: ProjectEngine.prototype["loadMergeSweepBatch" as keyof ProjectEngine], + isMergePending: async () => false, + mergeSweepHoldReasons: new Map(), hasAutoHealableVerificationBufferFailure: ProjectEngine.prototype["hasAutoHealableVerificationBufferFailure" as keyof ProjectEngine], isRetryCooldownElapsed: () => false, diff --git a/packages/engine/src/__tests__/project-engine.test.ts b/packages/engine/src/__tests__/project-engine.test.ts index 221ff15037..f4e77cd416 100644 --- a/packages/engine/src/__tests__/project-engine.test.ts +++ b/packages/engine/src/__tests__/project-engine.test.ts @@ -2401,34 +2401,22 @@ describe("ProjectEngine merge queue priority ordering", () => { // Tasks returned in createdAt ASC order (matches store.listTasks contract). // Priority order is interleaved so a naive iteration would merge FN-low // first; priority-aware sorting must reorder to urgent → normal → low. + /* FNXC:MergeAuthority 2026-08-23-18:05: the sweep now also proves graph authority per card, so a + fixture expected to reach the queue must look mergeable — no outstanding optional gates, and + quiet long enough for stall recovery. Priority ordering is what this test is about. */ + const mergeable = { + column: "in-review", + paused: false, + mergeRetries: 0, + status: null, + steps: [], + enabledWorkflowSteps: [], + updatedAt: new Date(Date.now() - 60 * 60_000).toISOString(), + }; const sweptTasks = [ - { - id: "FN-low", - column: "in-review", - paused: false, - mergeRetries: 0, - status: null, - priority: "low", - createdAt: "2026-04-01T00:00:00.000Z", - }, - { - id: "FN-urgent", - column: "in-review", - paused: false, - mergeRetries: 0, - status: null, - priority: "urgent", - createdAt: "2026-04-02T00:00:00.000Z", - }, - { - id: "FN-normal", - column: "in-review", - paused: false, - mergeRetries: 0, - status: null, - priority: "normal", - createdAt: "2026-04-03T00:00:00.000Z", - }, + { ...mergeable, id: "FN-low", priority: "low", createdAt: "2026-04-01T00:00:00.000Z" }, + { ...mergeable, id: "FN-urgent", priority: "urgent", createdAt: "2026-04-02T00:00:00.000Z" }, + { ...mergeable, id: "FN-normal", priority: "normal", createdAt: "2026-04-03T00:00:00.000Z" }, ]; const tasksById: Record> = Object.fromEntries( sweptTasks.map((t) => [t.id, t]), @@ -2462,9 +2450,9 @@ describe("ProjectEngine merge queue priority ordering", () => { it("picker falls back to next-priority task when the chosen one is removed from the queue during getTask awaits", async () => { const mockStore = createMockStore({ ...baseSettings, autoMerge: true }); const tasksById: Record> = { - "FN-urgent": { id: "FN-urgent", column: "in-review", paused: false, mergeRetries: 0, status: null, priority: "urgent", createdAt: "2026-04-01T00:00:00.000Z" }, - "FN-normal-a": { id: "FN-normal-a", column: "in-review", paused: false, mergeRetries: 0, status: null, priority: "normal", createdAt: "2026-04-02T00:00:00.000Z" }, - "FN-normal-b": { id: "FN-normal-b", column: "in-review", paused: false, mergeRetries: 0, status: null, priority: "normal", createdAt: "2026-04-03T00:00:00.000Z" }, + "FN-urgent": { id: "FN-urgent", column: "in-review", paused: false, enabledWorkflowSteps: [], steps: [], mergeRetries: 0, status: null, priority: "urgent", createdAt: "2026-04-01T00:00:00.000Z", updatedAt: new Date(Date.now() - 60 * 60_000).toISOString() }, + "FN-normal-a": { id: "FN-normal-a", column: "in-review", paused: false, enabledWorkflowSteps: [], steps: [], mergeRetries: 0, status: null, priority: "normal", createdAt: "2026-04-02T00:00:00.000Z", updatedAt: new Date(Date.now() - 60 * 60_000).toISOString() }, + "FN-normal-b": { id: "FN-normal-b", column: "in-review", paused: false, enabledWorkflowSteps: [], steps: [], mergeRetries: 0, status: null, priority: "normal", createdAt: "2026-04-03T00:00:00.000Z", updatedAt: new Date(Date.now() - 60 * 60_000).toISOString() }, }; let releaseUrgent: (() => void) = () => {}; @@ -2516,8 +2504,8 @@ describe("ProjectEngine merge queue priority ordering", () => { it("picker returns undefined when shutdown lands during getTask awaits", async () => { const mockStore = createMockStore({ ...baseSettings, autoMerge: true }); const tasksById: Record> = { - "FN-a": { id: "FN-a", column: "in-review", paused: false, mergeRetries: 0, status: null, priority: "high", createdAt: "2026-04-01T00:00:00.000Z" }, - "FN-b": { id: "FN-b", column: "in-review", paused: false, mergeRetries: 0, status: null, priority: "normal", createdAt: "2026-04-02T00:00:00.000Z" }, + "FN-a": { id: "FN-a", column: "in-review", paused: false, enabledWorkflowSteps: [], steps: [], mergeRetries: 0, status: null, priority: "high", createdAt: "2026-04-01T00:00:00.000Z", updatedAt: new Date(Date.now() - 60 * 60_000).toISOString() }, + "FN-b": { id: "FN-b", column: "in-review", paused: false, enabledWorkflowSteps: [], steps: [], mergeRetries: 0, status: null, priority: "normal", createdAt: "2026-04-02T00:00:00.000Z", updatedAt: new Date(Date.now() - 60 * 60_000).toISOString() }, }; let release: (() => void) = () => {}; @@ -2618,7 +2606,7 @@ describe("ProjectEngine paused in-review auto-merge behavior", () => { if (!taskUpdatedHandler) throw new Error("task:updated handler was not registered"); await taskUpdatedHandler({ id: "FN-unpause", column: "in-review", paused: true, status: "paused" }); - await taskUpdatedHandler({ id: "FN-unpause", column: "in-review", paused: false, status: null }); + await taskUpdatedHandler({ id: "FN-unpause", column: "in-review", paused: false, status: null, enabledWorkflowSteps: [], steps: [], updatedAt: new Date(Date.now() - 60 * 60_000).toISOString() }); expect(enqueueSpy).toHaveBeenCalledWith("FN-unpause"); @@ -3196,7 +3184,7 @@ describe("ProjectEngine paused in-review auto-merge behavior", () => { const mockStore = createMockStore({ ...baseSettings, autoMerge: true }); const inReviewTasks = [ { id: "FN-paused", column: "in-review", paused: true, mergeRetries: 0, status: null }, - { id: "FN-ready", column: "in-review", paused: false, mergeRetries: 0, status: null }, + { id: "FN-ready", column: "in-review", paused: false, enabledWorkflowSteps: [], steps: [], mergeRetries: 0, status: null, updatedAt: new Date(Date.now() - 60 * 60_000).toISOString() }, ]; /* FNXC:EngineTests 2026-08-10-10:34: @@ -3239,8 +3227,8 @@ describe("ProjectEngine paused in-review auto-merge behavior", () => { status: null, branchContext: { assignmentMode: "shared", groupId: "BG-5819", source: "planning" }, }, - { id: "FN-opted-in", column: "in-review", paused: false, mergeRetries: 0, status: null, autoMerge: true, branchContext: { assignmentMode: "shared", groupId: "BG-5819", source: "planning" } }, - { id: "FN-plain", column: "in-review", paused: false, mergeRetries: 0, status: null }, + { id: "FN-opted-in", column: "in-review", paused: false, enabledWorkflowSteps: [], steps: [], mergeRetries: 0, status: null, autoMerge: true, branchContext: { assignmentMode: "shared", groupId: "BG-5819", source: "planning" }, updatedAt: new Date(Date.now() - 60 * 60_000).toISOString() }, + { id: "FN-plain", column: "in-review", paused: false, enabledWorkflowSteps: [], steps: [], mergeRetries: 0, status: null, updatedAt: new Date(Date.now() - 60 * 60_000).toISOString() }, ]; /* FNXC:EngineTests 2026-08-10-10:34: @@ -3313,7 +3301,7 @@ describe("ProjectEngine paused in-review auto-merge behavior", () => { enqueueSpy.mockClear(); mockStore.store.listTasks.mockResolvedValueOnce([ { id: "FN-paused", column: "in-review", paused: true, mergeRetries: 0, status: null }, - { id: "FN-ready", column: "in-review", paused: false, mergeRetries: 0, status: null }, + { id: "FN-ready", column: "in-review", paused: false, enabledWorkflowSteps: [], steps: [], mergeRetries: 0, status: null, updatedAt: new Date(Date.now() - 60 * 60_000).toISOString() }, ]); await mockStore.emitSettingsUpdated( @@ -3340,10 +3328,10 @@ describe("ProjectEngine paused in-review auto-merge behavior", () => { mockStore.store.listTasks.mockResolvedValueOnce([ // Retry exhausted + failed (FN-2997 observed state after merge error) - { id: "FN-failed", column: "in-review", paused: false, mergeRetries: 3, status: "failed", updatedAt: new Date(Date.now() - 60 * 60_000).toISOString() }, + { id: "FN-failed", column: "in-review", paused: false, enabledWorkflowSteps: [], steps: [], mergeRetries: 3, status: "failed", updatedAt: new Date(Date.now() - 60 * 60_000).toISOString() }, // Failed status must block even when retries are below the cap. - { id: "FN-failed-low-retries", column: "in-review", paused: false, mergeRetries: 0, status: "failed", updatedAt: new Date().toISOString() }, - { id: "FN-ready", column: "in-review", paused: false, mergeRetries: 0, status: null, updatedAt: new Date().toISOString() }, + { id: "FN-failed-low-retries", column: "in-review", paused: false, enabledWorkflowSteps: [], steps: [], mergeRetries: 0, status: "failed", updatedAt: new Date(Date.now() - 60 * 60_000).toISOString() }, + { id: "FN-ready", column: "in-review", paused: false, enabledWorkflowSteps: [], steps: [], mergeRetries: 0, status: null, updatedAt: new Date(Date.now() - 60 * 60_000).toISOString() }, ]); await vi.advanceTimersByTimeAsync(15_000); @@ -3369,9 +3357,9 @@ describe("ProjectEngine paused in-review auto-merge behavior", () => { enqueueSpy.mockClear(); mockStore.store.listTasks.mockResolvedValueOnce([ { id: "FN-paused", column: "in-review", paused: true, mergeRetries: 0, status: null }, - { id: "FN-failed", column: "in-review", paused: false, mergeRetries: 0, status: "failed" }, - { id: "FN-blocked", column: "in-review", paused: false, mergeRetries: 0, status: null }, - { id: "FN-ready", column: "in-review", paused: false, mergeRetries: 0, status: null }, + { id: "FN-failed", column: "in-review", paused: false, enabledWorkflowSteps: [], steps: [], mergeRetries: 0, status: "failed", updatedAt: new Date(Date.now() - 60 * 60_000).toISOString() }, + { id: "FN-blocked", column: "in-review", paused: false, enabledWorkflowSteps: [], steps: [], mergeRetries: 0, status: null, updatedAt: new Date(Date.now() - 60 * 60_000).toISOString() }, + { id: "FN-ready", column: "in-review", paused: false, enabledWorkflowSteps: [], steps: [], mergeRetries: 0, status: null, updatedAt: new Date(Date.now() - 60 * 60_000).toISOString() }, ]); await mockStore.emitSettingsUpdated( @@ -3999,12 +3987,18 @@ describe("ProjectEngine stale mergeActive rescue (FN-3900)", () => { vi.useFakeTimers(); const mockStore = createMockStore({ ...baseSettings, autoMerge: true }); + /* FNXC:MergeAuthority 2026-08-23-20:05: the column-entry handoff now proves graph authority + before enqueueing, so a fixture expected to reach the queue must look mergeable — no + outstanding optional gates, and quiet long enough for stall recovery. */ mockStore.store.getTask.mockResolvedValue({ id: "FN-leaked", column: "in-review", paused: false, status: null, mergeRetries: 0, + steps: [], + enabledWorkflowSteps: [], + updatedAt: new Date(Date.now() - 60 * 60_000).toISOString(), }); mocks.currentStore = mockStore.store; @@ -4034,7 +4028,7 @@ describe("ProjectEngine stale mergeActive rescue (FN-3900)", () => { if (!taskMovedHandler) throw new Error("task:moved handler was not registered"); await taskMovedHandler({ - task: { id: "FN-leaked", column: "in-review", paused: false }, + task: { id: "FN-leaked", column: "in-review", paused: false, enabledWorkflowSteps: [], steps: [], updatedAt: new Date(Date.now() - 60 * 60_000).toISOString() }, to: "in-review", }); @@ -4121,6 +4115,9 @@ describe("ProjectEngine stale mergeActive rescue (FN-3900)", () => { paused: false, status: null, mergeRetries: 0, + steps: [], + enabledWorkflowSteps: [], + updatedAt: new Date(Date.now() - 60 * 60_000).toISOString(), }); mocks.currentStore = mockStore.store; @@ -4146,7 +4143,7 @@ describe("ProjectEngine stale mergeActive rescue (FN-3900)", () => { if (!taskMovedHandler) throw new Error("task:moved handler was not registered"); await taskMovedHandler({ - task: { id: "FN-busy", column: "in-review", paused: false }, + task: { id: "FN-busy", column: "in-review", paused: false, updatedAt: new Date(Date.now() - 60 * 60_000).toISOString() }, to: "in-review", }); @@ -4302,6 +4299,15 @@ describe("allowInReviewMergeProcessing per-task autoMerge override", () => { // tests above. describe("enqueueEligibleInReviewTasks honors per-task autoMerge override (shared sweep funnel)", () => { + /* + FNXC:MergeAuthority 2026-08-23-18:05: + These fixtures test the autoMerge-override contract, so they must clear the sweep's graph-authority + gate for an unrelated reason. `enabledWorkflowSteps: []` (no optional gates to wait on) plus an old + `updatedAt` puts them on the quiescent-stall recovery path — the one admission that does not + require a merge-region continuation. Without this they are refused as `gates-unsatisfied`, which is + correct behaviour for a card whose default-on Plan/Code Review have not run, but says nothing about + the override being tested here. + */ const inReview = (id: string, overrides: Partial = {}): Task => ({ id, @@ -4309,6 +4315,8 @@ describe("enqueueEligibleInReviewTasks honors per-task autoMerge override (share paused: false, mergeRetries: 0, status: null, + enabledWorkflowSteps: [], + updatedAt: new Date(Date.now() - 60 * 60_000).toISOString(), ...overrides, }) as unknown as Task; diff --git a/packages/engine/src/project-engine.ts b/packages/engine/src/project-engine.ts index 6a4cc6faa6..c413aec7d3 100644 --- a/packages/engine/src/project-engine.ts +++ b/packages/engine/src/project-engine.ts @@ -21,6 +21,7 @@ import { resolveProjectColumnsForRoles, REVIEW_ROLES, resolveWorkflowIrForTask, + resolveWorkflowIrForTaskWithProvenance, resolveColumnFlags, type TraitFlags, allowsAutoMergeProcessing, @@ -35,7 +36,14 @@ import { getTaskHardMergeBlocker, PreMergeStepsNotRunError, PRE_MERGE_STEPS_NOT_RUN_BLOCKER, - findUnrunRequiredPreMergeStepIds, + classifyMergeSweepAdmission, + classifyWorkflowNodeMergeRegion, + isActiveMergeStatus, + resolveRequiredPreMergeStepIds, + getTaskMergeBlocker, + ACTIVE_WORKFLOW_WORK_ITEM_STATES, + type MergeRegionPosition, + type MergeSweepAdmission, isLiveSharedBranchGroupMemberIntegration, isSharedBranchGroupMemberIntegration, isWorkspaceTask, @@ -74,6 +82,7 @@ import { } from "./overseer/overseer-advisor-service.js"; import { extractAdvisorAssistantText } from "./overseer/overseer-advise-tool.js"; import { createResolvedAgentSession } from "./agents/agent-session-helpers.js"; +import { activeSessionRegistry, executingTaskLock } from "./agents/active-session-registry.js"; import type { PrNodeGithubOps } from "./merge/pr-nodes.js"; import { PrReconciler, type PrReconcileGithubOps } from "./merge/pr-reconcile.js"; import { PrCommentHandler } from "./merge/pr-comment-handler.js"; @@ -195,6 +204,19 @@ const execFileAsync = promisify(execFile); */ const MERGE_HANDOFF_GRACE_MS = 300; +/* FNXC:MergeAuthority 2026-08-23-20:05: sweep-wide batched reads for merge-sweep admission. */ +interface MergeSweepBatch { + continuations?: Map>; + mergeRequests?: Map; + /** False when the batched continuation read failed — treated as unreadable, never as empty. */ + continuationsReadable: boolean; +} + +/** A merge request the graph is actively driving; either state proves an interrupted attempt. */ +function isActiveMergeRequestState(state: string | undefined): boolean { + return state === "running" || state === "retrying"; +} + const PR_MERGE_RETRY_BACKOFF_BASE_MS = 5_000; /** @@ -552,6 +574,12 @@ export class ProjectEngine { // ── Auto-merge state ── private mergeQueue: string[] = []; + /* FNXC:MergeAuthority 2026-08-23-18:05: last logged sweep-hold reason per task, so a held card is + reported once per reason change instead of every 15s poll. In-memory by design — it is a log + de-duplicator, not state anything reads back. Pruned each sweep to the current candidate set + (review finding #11), so a card that leaves review by ANY route drops out; admission and + soft-delete also clear their entry directly. */ + private readonly mergeSweepHoldReasons = new Map(); private mergeActive = new Set(); /** Capacity-deferred ids stay out of the runnable queue until their retry timer fires. */ private readonly capacityDeferredMergeTaskIds = new Set(); @@ -3436,37 +3464,253 @@ export class ProjectEngine { }) as Task[]; const allowFlags = await Promise.all(candidates.map((t) => this.allowInReviewMergeProcessing(t, settings, this.runtime.getTaskStore()))); /* - FNXC:RequiredPreMergeSteps 2026-08-22-22:40 (FN-9191 wedge): - Admission cannot see unrun pre-merge gates: `canMergeTask` is sync and the injected - `getTaskMergeBlocker` has no workflow IR, so it answers on RESULT ROWS only — and a gate that - has not started yet has no row. FN-9191 was enqueued ~2s after `fn_task_done`, ~18s before its - Code Review node started, and the door then had to refuse it. + FNXC:MergeAuthority 2026-08-23-18:05 (FN-9191 + FN-9193 wedges): + THIS SWEEP NO LONGER INITIATES MERGES. The workflow graph owns the decision — its `merge-attempt` + node calls `requestInterpreterMerge` directly — and this sweep is demoted to servicing merges the + graph already authorized (see `classifyMergeSweepAdmission`). Everything it used to judge on + (column, steps, status, retry budget) is still applied above by `canMergeTask`; this gate is the + additional proof of graph authority that column/step state cannot supply. - This sweep already resolves each card's IR, so ask the same question the door asks and hold the - card out of the queue until every enabled pre-merge group has a result. Failure to resolve the - IR admits the card (the door remains the authority); this filter exists to stop the race, not - to become a second gate. + Refusals are logged per card, once per (task, reason), because a card silently held out of the + merge queue is the exact failure mode this whole area keeps producing: no error, no event, just a + card that never merges. `mergeSweepHoldReasons` is in-memory and cleared when the card is admitted + or leaves the sweep, so a genuine long hold does not re-log every 15s. */ - const unrunGateFlags = await Promise.all( - candidates.map(async (t) => { - try { - const ir = await resolveWorkflowIrForTask(this.runtime.getTaskStore(), t.id, reviewLaneIrCache); - if (!ir) return false; - return findUnrunRequiredPreMergeStepIds(ir, t).length > 0; - } catch { + const sweepBatch = await this.loadMergeSweepBatch(candidates); + const admissions = await Promise.all( + candidates.map((t) => this.classifyMergeSweepCandidate(t, reviewLaneIrCache, sweepBatch)), + ); + /* Prune the log de-duplicator to this sweep's candidates (review finding #11): a card that left + review by ANY route — merged, rebounded, paused, replanned — drops its entry here, so the + map cannot accumulate ids for cards this sweep no longer sees. */ + const candidateIds = new Set(candidates.map((t) => t.id)); + for (const heldTaskId of [...this.mergeSweepHoldReasons.keys()]) { + if (!candidateIds.has(heldTaskId)) this.mergeSweepHoldReasons.delete(heldTaskId); + } + const eligible = sortTasksByPriorityThenAgeAndId( + candidates.filter((t, i) => { + if (!allowFlags[i]) return false; + const admission = admissions[i]!; + if (!admission.admit) { + const previous = this.mergeSweepHoldReasons.get(t.id); + if (previous !== admission.reason) { + this.mergeSweepHoldReasons.set(t.id, admission.reason); + runtimeLog.log(`Auto-merge sweep holding ${t.id}: ${admission.reason} (graph owns the merge decision)`); + } return false; } + this.mergeSweepHoldReasons.delete(t.id); + return true; }), ); - const eligible = sortTasksByPriorityThenAgeAndId( - candidates.filter((_, i) => allowFlags[i] && !unrunGateFlags[i]), - ); for (const t of eligible) { this.internalEnqueueMerge(t.id); } return eligible.length; } + /* + FNXC:MergeAuthority 2026-08-23-18:05 (FN-9191 + FN-9193 wedges): + Gather the sweep-admission inputs for one candidate. Probes are best-effort, but they do NOT all + degrade in the same direction — the direction is chosen per probe by what a wrong answer costs: + + - liveness, continuations: a throw must not wedge the whole lane, and the classifier's other + fences still hold, so they degrade toward "not live" / "unreadable" (the latter refuses + initiation on its own — see `continuationsReadable`). + - `gatesSatisfied`: the ONE deliberate fail-CLOSED probe (review finding #8). If we cannot prove + the pre-merge gates are satisfied we must not start a merge; withholding costs a delay, while + admitting costs an unreviewed branch on the target. The real door re-checks authoritatively. + + MULTI-REPO / WORKSPACE (and shared-branch groups) resolve through the SAME rules, deliberately: + - `branch-group-member-integration` and `branch-group-promotion` are merge-region kinds, so a + shared-branch member parked at either is admitted like any other merge-region card. + - A workspace sub-repo land registers `workspace-repo-land` / `workspace-repo-acquire` paths in + `activeSessionRegistry`, so a task mid-land reads as live and is held — this is what stops a + second dispatch racing a partial land. + - A workspace partial land re-enqueues through `internalEnqueueMerge` directly (the error path + below), which never consults this gate; the merging status it leaves also satisfies + `interruptedMergeAttempt` so a restarted engine re-drives the same land. + - `isMergePending` covers the cross-node case: another engine holding a durable merge-dispatch + lease reads as live here, so two nodes on one central DB cannot both dispatch the same land. + NOTE it answers `true` from its OWN catch on an unreadable lease probe (see its FNXC note), so + a persistently broken lease read holds every card in this lane rather than racing another + node. That is the intended conservative direction; it is not a silent no-op. + + `batch` carries the sweep-wide batched reads (review finding #10) so this runs at O(1) queries per + poll instead of O(cards). It is optional: single-card callers (the column-entry handoff, the + unpause re-enqueue, the pre-dispatch re-check) pass nothing and fall back to per-task reads. + */ + private async classifyMergeSweepCandidate( + task: Task, + irCache: Map, + batch?: MergeSweepBatch, + opts: { ignoreOwnMergePipeline?: boolean } = {}, + ): Promise { + const store = this.runtime.getTaskStore(); + + /* + WHOSE GRAPH IS THIS? (review finding #5) `resolveWorkflowIrForTask` never returns null — it + degrades to `builtin:coding` — so `!!ir` was always true and a card on a missing/malformed + workflow was judged against a graph that is not its own. `selectionAbsent` separates "no + selection stored, so the project default IS this card's workflow" (trustworthy) from "a named + selection that failed to resolve" (not this card's graph; positions unusable). + */ + let irTrust: "cards-own" | "effective-default" | "unresolved-selection" = "unresolved-selection"; + let ir: WorkflowIr | null = null; + try { + const resolved = await resolveWorkflowIrForTaskWithProvenance(store, task.id, irCache); + ir = resolved.ir; + irTrust = resolved.source === "selection" + ? "cards-own" + : resolved.selectionAbsent === true ? "effective-default" : "unresolved-selection"; + } catch { + ir = null; + irTrust = "unresolved-selection"; + } + + /* Any live surface owned by this task, plus the pump's own in-flight window and the durable + cross-node merge-dispatch lease. Probe failures read as NOT live: a broken probe must not + wedge every card, and the graph-position checks below still carry the invariant. */ + let hasLiveSession = false; + try { + /* + FNXC:MergeAuthority 2026-08-23-20:05 (FN-3900 interaction): + TWO DIFFERENT QUESTIONS wear the word "live". Session liveness — an executor remediating, a + review step running, a workspace sub-repo land — means SOMEONE ELSE owns this card, and that + is the FN-9193 window; it always defers. This engine's own merge pipeline state + (`activeMergeTaskId` / `isMergePending`) means only "this card's merge is already queued or + running", which is queue DEDUPE — and `internalEnqueueMerge` already reconciles that. Callers + whose whole job is to (re-)enqueue a card the pump may already hold, like the FN-3900 leaked + `mergeActive` rescue on column entry, pass `ignoreOwnMergePipeline` so pipeline state does not + read as someone else's work. Authority and session liveness still apply to them. + */ + hasLiveSession = executingTaskLock.has(task.id) + || activeSessionRegistry.pathsForTask(task.id).length > 0 + || (!opts.ignoreOwnMergePipeline + && (this.activeMergeTaskId === task.id || await this.isMergePending(task.id))); + } catch { + hasLiveSession = false; + } + + /* ACTIVE `kind:"task"` continuations only. A cancelled/exhausted row is finished work, not a + live wait — the same reading `onSuspend` uses when it decides whether to seed a successor. + An unreadable read is reported as such (review finding #4), never as "none scheduled". */ + let continuationPositions: MergeRegionPosition[] = []; + let continuationsReadable = true; + try { + /* A store with no continuation API cannot HAVE continuations — that is an empty answer, not + an unreadable one. Only a throw from a method that exists means "we could not find out". */ + const readContinuations = (store as Partial).listWorkflowWorkItemsForTask; + const items = batch?.continuations + ? batch.continuations.get(task.id) ?? [] + : typeof readContinuations === "function" + ? await readContinuations.call(store, task.id, { kinds: ["task"] }) + : []; + continuationPositions = items + .filter((item) => (ACTIVE_WORKFLOW_WORK_ITEM_STATES as readonly string[]).includes(item.state)) + .map((item) => (ir ? classifyWorkflowNodeMergeRegion(ir, item.nodeId) : "unknown")); + } catch { + continuationsReadable = false; + continuationPositions = []; + } + if (batch && !batch.continuationsReadable) continuationsReadable = false; + + /* Only consulted when the task's own status is not already merge-active, since either proves + the same thing and the status needs no query. */ + let mergeRequestActive = false; + if (!isActiveMergeStatus(task.status)) { + try { + mergeRequestActive = batch?.mergeRequests + ? isActiveMergeRequestState(batch.mergeRequests.get(task.id)?.state) + : isActiveMergeRequestState((await store.getMergeRequestRecordAsync?.(task.id))?.state); + } catch { + mergeRequestActive = false; + } + } + + let gatesSatisfied = true; + try { + const reviewColumns = new Set([task.column]); + /* `steps` is optional on partially-hydrated rows; the door dereferences it unconditionally. */ + gatesSatisfied = !getTaskMergeBlocker({ ...task, steps: task.steps ?? [] }, { + reviewColumns, + requiredPreMergeStepIds: ir ? resolveRequiredPreMergeStepIds(ir, task.enabledWorkflowSteps) : undefined, + }); + } catch { + gatesSatisfied = false; + } + + const updatedAtMs = Date.parse(task.updatedAt ?? ""); + const quiescentMs = Number.isFinite(updatedAtMs) ? Math.max(0, Date.now() - updatedAtMs) : Number.POSITIVE_INFINITY; + + return classifyMergeSweepAdmission({ + irTrust, + continuationPositions, + continuationsReadable, + mergeConfirmed: task.mergeDetails?.mergeConfirmed === true, + hasLiveSession, + interruptedMergeAttempt: isActiveMergeStatus(task.status) || mergeRequestActive, + quiescentMs, + gatesSatisfied, + }); + } + + /* + FNXC:MergeAuthority 2026-08-23-20:05 (review finding #10): + One batched read per sweep instead of one per card. The sweep asks the same two questions of every + review-lane card on every 15s poll; per-task reads made that 2N queries. A failed batch read is + reported as unreadable rather than as empty, so it refuses initiation exactly like the per-task + failure path does. + */ + /* + FNXC:MergeAuthority 2026-08-23-20:05 (review finding #2): + Position-only dispatch guard. Answers ONE question — "do this card's active continuations still + place it inside its workflow's merge region?" — and fails OPEN on every uncertainty (unreadable + continuations, an unresolvable workflow, no continuation at all), because the merge door behind it + is the authority on everything else. A card with NO active continuation is authorized here by + design: that is the shape of the interrupted-merge and quiescent-stall recoveries. + */ + private async isDispatchStillGraphAuthorized(task: Task): Promise { + const store = this.runtime.getTaskStore() as Partial; + try { + const readContinuations = store.listWorkflowWorkItemsForTask; + if (typeof readContinuations !== "function") return true; + const items = await readContinuations.call(this.runtime.getTaskStore(), task.id, { kinds: ["task"] }); + const active = items.filter((item) => (ACTIVE_WORKFLOW_WORK_ITEM_STATES as readonly string[]).includes(item.state)); + if (active.length === 0) return true; + const resolved = await resolveWorkflowIrForTaskWithProvenance(this.runtime.getTaskStore(), task.id); + if (resolved.source !== "selection" && resolved.selectionAbsent !== true) return true; + return active.some((item) => classifyWorkflowNodeMergeRegion(resolved.ir, item.nodeId) !== "outside-merge-region"); + } catch { + return true; + } + } + + private async loadMergeSweepBatch(candidates: readonly Task[]): Promise { + const store = this.runtime.getTaskStore() as Partial; + const ids = candidates.map((t) => t.id); + const batch: MergeSweepBatch = { continuationsReadable: true }; + if (ids.length === 0) return batch; + try { + batch.continuations = typeof store.listWorkflowWorkItemsForTasks === "function" + ? await store.listWorkflowWorkItemsForTasks.call(this.runtime.getTaskStore(), ids, { kinds: ["task"] }) + : undefined; + } catch { + batch.continuations = undefined; + batch.continuationsReadable = false; + } + /* A merge-request read failure only costs the `interrupted-merge-attempt` reason, which the + merging status also proves, so it degrades to "no record" rather than to unreadable. */ + try { + batch.mergeRequests = typeof store.getMergeRequestRecordsAsync === "function" + ? await store.getMergeRequestRecordsAsync.call(this.runtime.getTaskStore(), ids) + : undefined; + } catch { + batch.mergeRequests = undefined; + } + return batch; + } + private reconcileStaleMergeActive(): number { let cleared = 0; for (const taskId of [...this.mergeActive]) { @@ -3912,6 +4156,26 @@ export class ProjectEngine { continue; } + /* + FNXC:MergeAuthority 2026-08-23-20:05 (review finding #2 — TOCTOU AT DISPATCH): + Graph POSITION is re-proved here, not only at enqueue. The queue is single-flight and can + be minutes deep, so a card admitted while parked at `merge-gate` can be bounced back to a + revision node before its turn arrives — same column, same null status, so the + `canMergeTask` re-check above cannot see the difference. That is FN-9193's shape: an + authorized merge that stops being authorized while it waits. + + Deliberately NARROWER than the sweep's admission: position is the only question the merge + door itself cannot answer. The door re-reads the task and re-checks gates, blockers, and + lane identity authoritatively (returning the FN-9191 typed deferral for an unrun gate), so + re-imposing those here would only duplicate it — and would wrongly refuse the interrupted + and merge-confirmed recoveries that legitimately have no continuation at all. + */ + if (!task.mergeDetails?.mergeConfirmed && !(await this.isDispatchStillGraphAuthorized(task as Task))) { + runtimeLog.log(`Auto-merge dispatch skipped for ${taskId}: graph moved the card out of its merge region while it was queued`); + this.clearMergeActive(taskId); + continue; + } + // Fast path: merge already confirmed (e.g. task was moved back to // in-review by auto-recovery after a successful merge) — just // complete the task without re-running the merge process. @@ -5129,8 +5393,11 @@ export class ProjectEngine { merge — including the graph's own merge node — died on `task is marked 'failed'`. Deferral semantics: no status write, no `mergeRetries` burn, no operator handoff. The - card stays merge-eligible and the admission filter in `enqueueEligibleInReviewTasks` - holds it out of the queue until the gate reports, so this cannot spin. + card stays merge-eligible, and it cannot spin because `classifyMergeSweepAdmission` fences + EVERY initiation on `gatesSatisfied` (2026-08-23-20:05, review finding #6) — the same + unrun-gate condition that produced this deferral also refuses the next admission, at every + door, until the gate actually reports. An earlier version fenced only the quiescent path, + which left this re-enqueueing every sweep forever. */ if (err instanceof PreMergeStepsNotRunError) { await store @@ -5876,6 +6143,20 @@ export class ProjectEngine { runtimeLog.warn(`Auto-merge handoff (${task.id}): clearing stale mergeActive before enqueue`); this.clearMergeActive(task.id); } + /* + FNXC:MergeAuthority 2026-08-23-20:05 (review finding #1 — THE FASTER DOOR): + This handoff, not the 15s sweep, is the door FN-9191 actually came through: it fires + MERGE_HANDOFF_GRACE_MS (300ms) after the card enters the review column, which matches that + task's "merge attempted ~2s after fn_task_done" far better than any sweep tick. Gating only + `enqueueEligibleInReviewTasks` would have left the faster path wide open — the graph would + still not be the single merge authority. Its pre-existing `options.getTaskMergeBlocker` + check is the RESULT-ROWS-ONLY blocker that cannot see a gate which has not started. + */ + const handoffAdmission = await this.classifyMergeSweepCandidate(latestTask, new Map(), undefined, { ignoreOwnMergePipeline: true }); + if (!handoffAdmission.admit) { + runtimeLog.log(`Auto-merge handoff (${task.id}) skipped: ${handoffAdmission.reason} (graph owns the merge decision)`); + return; + } this.internalEnqueueMerge(task.id); } catch (err: unknown) { runtimeLog.warn( @@ -5999,6 +6280,14 @@ export class ProjectEngine { return; } + /* FNXC:MergeAuthority 2026-08-23-20:05 (review finding #9): same authority proof as the + sweep and the column-entry handoff — unpausing resumes a card, it does not authorize a + merge the graph never reached. The periodic sweep picks it up once the graph does. */ + const unpauseAdmission = await this.classifyMergeSweepCandidate(task, new Map(), undefined, { ignoreOwnMergePipeline: true }); + if (!unpauseAdmission.admit) { + runtimeLog.log(`In-review unpause: ${task.id} not re-enqueued: ${unpauseAdmission.reason} (graph owns the merge decision)`); + return; + } runtimeLog.log(`Unpaused in-review task re-enqueued for auto-merge: ${task.id}`); this.internalEnqueueMerge(task.id); } catch (err: unknown) { @@ -6010,6 +6299,7 @@ export class ProjectEngine { this.taskDeletedHandler = (task: Task) => { this.pausedReviewTaskIds.delete(task.id); + this.mergeSweepHoldReasons.delete(task.id); const queueLengthBefore = this.mergeQueue.length; this.mergeQueue = this.mergeQueue.filter((queuedTaskId) => queuedTaskId !== task.id);