feat(engine): attribute review-gate leases to a node so dead local leases reclaim fast

Groundwork for FN-8603's remaining ~14-minute wait. Liveness for a pending
review gate is judged purely by a 15-minute staleness floor because a lease
records WHO took it (`leaseOwner` = run id) but not WHERE, and under multi-node
every engine sees every other engine's leases. A fresh-but-unknown lease might
be running on a peer, so the floor was the only safe test -- and a lease left by
this node's own crashed process is indistinguishable from it.

Adds `WorkflowStepResult.leaseNodeId` plus an optional `LocalNodeLeaseIdentity`
argument to `classifyReviewLease`. One narrow new case: a lease stamped with the
caller's OWN node id whose `startedAt` predates the caller's process boot is
provably dead -- the process that could have owned it is gone -- so it
classifies as `reclaim` immediately rather than aging out. Deliberately narrow,
because widening it is a double-dispatch risk: absent (legacy) or peer node ids
keep the floor, and a lease taken by this process after boot is still adopted.

InProcessRuntime.start() resolves the local node id from CentralCore (fail-soft;
on error it stays undefined and floor-only semantics apply) and passes it to
SelfHealingManager. The graph executor stamps the field when deps.localNodeId is
set.

NOT YET WIRED, so this is inert in production and behavior is unchanged end to
end: `localNodeId` is not threaded from WorkflowGraphTaskRunner /
WorkflowTaskRuntime down into the executor deps, so no lease actually carries a
`leaseNodeId` yet. The reader is ready; the writer needs that pass-through
(WorkflowGraphTaskRunnerDeps gains the field, the runner forwards it, and the
runtime supplies this.localNodeId). Stopping here rather than half-threading it.

Verified: tsc clean on core and engine, pnpm lint clean, pnpm test:gate green
(299 + 70), core workflow-step-results suite green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-07-26 12:24:29 -07:00
parent fde3b76a8c
commit 3b83282273
6 changed files with 124 additions and 1 deletions

View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Review-gate leases now record which node holds them, so a restarted engine can tell its own dead leases from a peer's.
category: internal
dev: Adds `WorkflowStepResult.leaseNodeId` and an optional `LocalNodeLeaseIdentity` argument to `classifyReviewLease`. A pending lease stamped with the caller's own node id whose `startedAt` predates the current process boot now classifies as `reclaim` immediately instead of waiting out `PLAN_REVIEW_LEASE_STALENESS_MS`; peer-owned and legacy unattributed leases are unchanged. `InProcessRuntime.start()` resolves the local node id from CentralCore and passes it to SelfHealingManager. The graph executor stamps the field when `deps.localNodeId` is set — that dep is not yet threaded from the runners, so the field is not written in production yet and behavior is unchanged end to end.

View File

@@ -265,6 +265,23 @@ export interface WorkflowStepResult {
* on every terminal (passed/failed/…) record — a lease only exists while pending.
*/
leaseOwner?: string;
/*
* FNXC:PlanReviewLease 2026-07-26-20:05:
* Node that owns this lease. `leaseOwner` alone identifies a run but not WHERE it runs, and in a
* multi-node deployment (several engines on one central database) every node's self-healing sweep
* sees every other node's leases. Without attribution the only safe liveness test is the
* 15-minute staleness floor, because a lease that is fresh-but-unknown might be running on a peer.
*
* With it, a node can prove one specific case: a lease stamped with ITS OWN id whose `startedAt`
* predates its current process boot cannot have a live owner — the process that took it is gone.
* That is the restart-orphan case (FN-8603: an engine restart killed a Code Review session 34s in
* and the card then waited out the full floor before anything re-ran it). Peer-owned and
* unattributed (legacy) leases keep the floor.
*
* Absent on legacy rows written before this field existed; treat absence as "unknown node", never
* as "this node".
*/
leaseNodeId?: string;
/*
* FNXC:ReviewLaneBypass 2026-07-09-00:00:
* A privileged operator can bypass a `status:"failed"` pre-merge review step

View File

@@ -117,6 +117,17 @@ graph executor and unit tests share one lease implementation.
* FN-6736 staleness-floor standard for durable single-owner leases. */
export const PLAN_REVIEW_LEASE_STALENESS_MS = 15 * 60 * 1000;
/**
* Identity a caller supplies so {@link classifyReviewLease} can recognize leases left behind by a
* PREVIOUS process on the SAME node. `nodeId` must be the cluster node id stamped into
* `WorkflowStepResult.leaseNodeId`; `processBootAt` is this process's start time (epoch ms).
* Omit it entirely to keep pure staleness-floor semantics.
*/
export interface LocalNodeLeaseIdentity {
nodeId: string;
processBootAt: number;
}
/** Classification of a review-gate's current lease state for a re-entering run. */
export type ReviewLeaseDisposition =
/** No prior result — this run should claim the lease and dispatch the reviewer. */
@@ -158,6 +169,7 @@ export function classifyReviewLease(
stepId: string,
now: number,
stalenessMs: number = PLAN_REVIEW_LEASE_STALENESS_MS,
localNode?: LocalNodeLeaseIdentity,
): ReviewLeaseDisposition {
const existing = results?.find((r) => r.workflowStepId === stepId);
if (!existing) return { kind: "claim" };
@@ -165,6 +177,29 @@ export function classifyReviewLease(
// existing.status === "pending": it is a lease.
const startedMs = existing.startedAt ? Date.parse(existing.startedAt) : Number.NaN;
const ageMs = Number.isFinite(startedMs) ? now - startedMs : Number.POSITIVE_INFINITY;
/*
FNXC:PlanReviewLease 2026-07-26-20:12:
Pre-boot reclaim. A lease stamped with THIS node's id whose `startedAt` predates this process's
boot is provably dead: the process that could have owned it no longer exists. Reclaim it
immediately instead of waiting out the staleness floor — the floor exists to protect leases we
cannot attribute, and this one we can.
Deliberately narrow, because every widening is a double-dispatch risk:
- `leaseNodeId` must be PRESENT and EQUAL to ours. Absent (legacy rows) or a peer's id both keep
the floor — under multi-node, a fresh peer lease is very likely genuinely running.
- `startedAt` must parse and be STRICTLY before boot. A lease taken by this process after boot is
a live in-process claim and must still be adopted.
Motivating incident FN-8603: an engine restart killed a Code Review session 34s in; the lease then
read "fresh" for the remaining ~14 minutes of the floor, so nothing re-ran the gate until it aged
out and was marked failed.
*/
const ownedByDeadLocalProcess =
localNode !== undefined &&
existing.leaseNodeId !== undefined &&
existing.leaseNodeId === localNode.nodeId &&
Number.isFinite(startedMs) &&
startedMs < localNode.processBootAt;
if (ownedByDeadLocalProcess) return { kind: "reclaim", priorOwner: existing.leaseOwner };
const stale = !existing.leaseOwner || !Number.isFinite(startedMs) || ageMs >= stalenessMs;
if (stale) return { kind: "reclaim", priorOwner: existing.leaseOwner };
// Not stale ⇒ `leaseOwner` is guaranteed set (the stale check requires it).

View File

@@ -325,6 +325,8 @@ export class InProcessRuntime
*/
private cliAgentRuntime?: BootstrappedCliAgentRuntime;
private usageLimitPauser?: UsageLimitPauser;
/** FNXC:PlanReviewLease 2026-07-26-20:42: cluster node id stamped onto review-gate leases; undefined until start() resolves it, or if resolution fails. */
private localNodeId?: string;
private selfHealingManager?: SelfHealingManager;
private leaseManager?: MeshLeaseManager;
private leaseCentralClaimStore?: AsyncCentralClaimStore;
@@ -1201,8 +1203,26 @@ export class InProcessRuntime
if (!chatLayer2) throw new Error("Self-healing ChatStore requires the project PostgreSQL AsyncDataLayer");
this.chatStore ??= new ChatStore(chatLayer2);
}
/*
FNXC:PlanReviewLease 2026-07-26-20:40:
Resolve this engine's cluster node id once at start so review-gate leases can be attributed.
Attribution is what lets self-healing tell "a lease my own dead process left behind" from "a
peer node's lease that is genuinely running" — the former is reclaimed immediately, the latter
keeps the 15-minute staleness floor. Fail-soft: on any error the id stays undefined, leases are
written unattributed, and floor-only semantics (the pre-existing behavior) apply.
*/
let localNodeId: string | undefined;
try {
const registeredNodes = await this.centralCore.listNodes();
localNodeId = registeredNodes.find((node) => node.type === "local")?.id;
} catch (error) {
runtimeLog.warn(`Could not resolve local node id for review-gate lease attribution: ${error instanceof Error ? error.message : String(error)}`);
}
this.localNodeId = localNodeId;
this.selfHealingManager = new SelfHealingManager(this.taskStore, {
rootDir: this.config.workingDirectory,
localNodeId,
agentStore: this.agentStore,
isWorktreeResumeReserved: this.cliAgentRuntime?.isWorktreeResumeReserved,
recoverCompletedTask: (task) => this.executor.recoverCompletedTask(task),

View File

@@ -272,6 +272,14 @@ const PRE_EXECUTION_WORKTREE_MAX_IDLE_MS = 30 * 24 * 60 * 60 * 1000;
export interface SelfHealingOptions {
/** Project root directory (parent of .worktrees/) */
rootDir: string;
/*
* FNXC:PlanReviewLease 2026-07-26-20:30:
* This engine's cluster node id, matching what the graph stamps into
* `WorkflowStepResult.leaseNodeId`. Only used to recognize review-gate leases left by a PREVIOUS
* process on this same node so they can be reclaimed immediately rather than waiting out the
* 15-minute staleness floor. Unset (single-node, tests) keeps floor-only semantics.
*/
localNodeId?: string;
/** Optional callback to release TaskExecutor in-memory worktree ownership for a task. */
releaseExecutorWorktreeOwnership?: (taskId: string) => void;
/**
@@ -7161,9 +7169,28 @@ export class SelfHealingManager {
re-attaches an in-review graph run after a restart, so those leases simply age out and
are then marked failed as before.
*/
/*
FNXC:PlanReviewLease 2026-07-26-20:26:
FN-8603 follow-up. The paragraph above accepts that "restart-orphaned gates simply age
out" — that acceptance is what cost FN-8603 ~14 minutes of dead wait after an engine
restart killed its Code Review session 34s in. Passing this node's identity lets
classifyReviewLease reclaim a lease THIS node's previous process took (proven dead: it
predates our boot) without touching the floor that protects peer-owned and legacy
unattributed leases. When localNodeId is unset the argument is undefined and behavior is
exactly as before.
*/
const localNodeLeaseIdentity = this.options.localNodeId
? { nodeId: this.options.localNodeId, processBootAt: this.processBootStartedAt }
: undefined;
const hasLiveReviewLease = (result: WorkflowStepResult): boolean => {
if (!result.leaseOwner || !result.startedAt) return false;
return classifyReviewLease([result], result.workflowStepId, Date.now(), PLAN_REVIEW_LEASE_STALENESS_MS).kind === "adopt";
return classifyReviewLease(
[result],
result.workflowStepId,
Date.now(),
PLAN_REVIEW_LEASE_STALENESS_MS,
localNodeLeaseIdentity,
).kind === "adopt";
};
const { results, orphanedCount } = resolveOrphanedPendingStepResults<WorkflowStepResult>(
fresh.workflowStepResults,

View File

@@ -127,6 +127,15 @@ export interface WorkflowNodePreparationRequirement {
}
export interface WorkflowGraphExecutorDeps {
/*
* FNXC:PlanReviewLease 2026-07-26-20:18:
* Cluster node id stamped onto review-gate leases (`WorkflowStepResult.leaseNodeId`). It lets a
* node later recognize a lease its OWN previous process left behind and reclaim it without
* waiting out the staleness floor; peer-owned leases stay protected by the floor. Optional —
* when unset the lease is written unattributed and keeps pure floor semantics, which is the
* pre-existing behavior.
*/
localNodeId?: string;
handlers?: Partial<Record<WorkflowIrNode["kind"], WorkflowNodeHandler>>;
/*
* FNXC:WorkflowNodeRunners 2026-07-01-00:00:
@@ -818,6 +827,14 @@ export class WorkflowGraphExecutor {
// U3/KTD-4: stamp the lease owner so a concurrent/crashed re-entry
// adopts this pending gate instead of dispatching a second reviewer.
leaseOwner: runId,
/*
FNXC:PlanReviewLease 2026-07-26-20:20:
Stamp WHERE the lease runs, not just which run holds it. `runId` cannot distinguish
"this node's dead previous process" from "a peer node running right now", so without
this every restart-orphaned gate had to wait out the full staleness floor. Omitted when
the node id is unknown, which preserves the previous floor-only behavior.
*/
...(this.deps.localNodeId ? { leaseNodeId: this.deps.localNodeId } : {}),
});
this.deps.logTaskEntry?.(`${logPrefix} Starting workflow step: ${groupName}`);