diff --git a/.changeset/review-lease-node-attribution.md b/.changeset/review-lease-node-attribution.md index 98d25f2abe..f75fa8957c 100644 --- a/.changeset/review-lease-node-attribution.md +++ b/.changeset/review-lease-node-attribution.md @@ -4,4 +4,4 @@ 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. +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 dep is threaded runtime -> TaskExecutor (`getLocalNodeId`, a getter because the runtime resolves the id asynchronously during start()) -> WorkflowGraphTaskRunner -> WorkflowGraphExecutor, which stamps it on the lease. diff --git a/packages/engine/src/__tests__/plan-review-lease.test.ts b/packages/engine/src/__tests__/plan-review-lease.test.ts index 67fae09b63..ea950a1a5a 100644 --- a/packages/engine/src/__tests__/plan-review-lease.test.ts +++ b/packages/engine/src/__tests__/plan-review-lease.test.ts @@ -99,3 +99,50 @@ describe("makeReviewLeaseRecord", () => { expect(classifyReviewLease([rec], STEP, T0 + 1).kind).toBe("adopt"); }); }); + +/* +FNXC:PlanReviewLease 2026-07-26-21:25: +Node-attributed lease reclaim (FN-8603 follow-up). Liveness used to be judged purely by the staleness +floor, so a lease left behind by THIS node's crashed process was indistinguishable from a peer's +running one and had to age out — ~14 minutes of dead wait after an engine restart. These pin the +narrow widening: only an own-node lease predating our boot reclaims early. Every other shape must +keep the floor, because reclaiming a live lease double-dispatches a reviewer. +*/ +describe("classifyReviewLease — node-attributed pre-boot reclaim", () => { + const BOOT = Date.parse("2026-07-26T18:20:00.000Z"); + const NOW = BOOT + 60_000; // one minute after boot; well inside the 15-minute floor + const lease = (over: Partial = {}): WorkflowStepResult[] => ([{ + workflowStepId: "code-review", + workflowStepName: "Code Review", + status: "pending", + startedAt: new Date(BOOT - 34_000).toISOString(), // 34s BEFORE boot — the FN-8603 shape + leaseOwner: "run-1", + ...over, + }]); + const local = { nodeId: "node-a", processBootAt: BOOT }; + + it("reclaims an own-node lease that predates this process boot", () => { + const d = classifyReviewLease(lease({ leaseNodeId: "node-a" }), "code-review", NOW, undefined, local); + expect(d.kind).toBe("reclaim"); + }); + + it("adopts a peer-node lease of the same age (we cannot prove a peer is dead)", () => { + const d = classifyReviewLease(lease({ leaseNodeId: "node-b" }), "code-review", NOW, undefined, local); + expect(d.kind).toBe("adopt"); + }); + + it("adopts an unattributed legacy lease of the same age", () => { + const d = classifyReviewLease(lease(), "code-review", NOW, undefined, local); + expect(d.kind).toBe("adopt"); + }); + + it("adopts an own-node lease taken AFTER boot — that is a live in-process claim", () => { + const results = lease({ leaseNodeId: "node-a", startedAt: new Date(BOOT + 5_000).toISOString() }); + expect(classifyReviewLease(results, "code-review", NOW, undefined, local).kind).toBe("adopt"); + }); + + it("keeps pure floor semantics when no local identity is supplied", () => { + const d = classifyReviewLease(lease({ leaseNodeId: "node-a" }), "code-review", NOW); + expect(d.kind).toBe("adopt"); + }); +}); diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 547dad7e57..bdad63a116 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -1648,6 +1648,14 @@ export function getExecutorSystemPrompt( export interface TaskExecutorOptions { + /* + * FNXC:PlanReviewLease 2026-07-26-21:07: + * Resolves this engine's cluster node id for review-gate lease attribution. A GETTER, not a + * value: the runtime resolves the id asynchronously during start(), which can complete after + * the executor is constructed, so a snapshot taken at construction would be permanently + * undefined. Read at runner-construction time instead. + */ + getLocalNodeId?: () => string | undefined; semaphore?: AgentSemaphore; /** Worktree pool for recycling idle worktrees across tasks. */ pool?: WorktreePool; @@ -5964,6 +5972,7 @@ export class TaskExecutor { resolveColumnBinding: resolveBindingForNode, }); const runner = new WorkflowGraphTaskRunner({ + localNodeId: this.options.getLocalNodeId?.(), store: { ...this.store, /* diff --git a/packages/engine/src/runtimes/in-process-runtime.ts b/packages/engine/src/runtimes/in-process-runtime.ts index 9e00c3fd85..e15c7ece81 100644 --- a/packages/engine/src/runtimes/in-process-runtime.ts +++ b/packages/engine/src/runtimes/in-process-runtime.ts @@ -867,6 +867,13 @@ export class InProcessRuntime const prNodeGithubOps = this.config.prNodeGithubOps; const executorOptions: TaskExecutorOptions = { + /* + FNXC:PlanReviewLease 2026-07-26-21:12: + Getter, not a value: `this.localNodeId` is resolved later in start() (it needs an async + CentralCore read), so capturing it here would freeze `undefined` and silently disable lease + attribution. Reading it lazily at runner-construction time picks up the resolved id. + */ + getLocalNodeId: () => this.localNodeId, semaphore: this.projectSemaphore, pool: this.worktreePool, usageLimitPauser: this.usageLimitPauser, diff --git a/packages/engine/src/workflow-graph-task-runner.ts b/packages/engine/src/workflow-graph-task-runner.ts index cefdb4c602..283c739767 100644 --- a/packages/engine/src/workflow-graph-task-runner.ts +++ b/packages/engine/src/workflow-graph-task-runner.ts @@ -76,6 +76,13 @@ export interface WorkflowGraphRunnerStore { } export interface WorkflowGraphTaskRunnerDeps { + /* + * FNXC:PlanReviewLease 2026-07-26-21:05: + * Cluster node id forwarded to the graph executor so review-gate leases are stamped with WHERE + * they run. Without it a lease is unattributed and self-healing can only use the 15-minute + * staleness floor, which is what made a restart-orphaned gate wait the floor out (FN-8603). + */ + localNodeId?: string; store: WorkflowGraphRunnerStore; seams: WorkflowLegacySeams; primitives?: WorkflowRuntimePrimitives; @@ -363,6 +370,7 @@ export class WorkflowGraphTaskRunner { : undefined; const executor = new WorkflowGraphExecutor({ + localNodeId: this.deps.localNodeId, seams: wrappedSeams, primitives: wrappedPrimitives, runCustomNode: wrappedRunCustomNode,