From ec2921b9584899563e0d581ab852fe5f3ac044f7 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 04:56:49 -0700 Subject: [PATCH] =?UTF-8?q?fleet(engine):=20self-healing=2023=20=E2=86=92?= =?UTF-8?q?=206=20=E2=80=94=20async-reachable=20guards,=20plus=203=20of=20?= =?UTF-8?q?4=20fan-out=20guards=20the=20sync=20path=20could=20not=20serve?= =?UTF-8?q?=20(#3094)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Replaces #3093, which I am closing.** Third rebuild of this work. ## A coordination note first, because it is costing more than the code `self-healing.ts` has had **three** overlapping conversions land from other lanes while my branch was open — #3049, #3075, #3078. Every time, replaying my commits produced conflicts that were all the same shape: *same guard, two spellings, different variable names*. Each rebuild is a full cycle spent on merge mechanics rather than on lanes. I have rebuilt against `main`'s own census each time rather than argue about whose spelling wins, and this PR contains only what `main` (23) does not have. But if this file is going to keep receiving concurrent fleet passes, one lane should own it — otherwise the next PR pays the same tax again. ## Converted | site | note | |---|---| | `isPhantomExecutorBinding` | caller resolved `lanesOfReclaim(task.id).wip` **three lines above the call**, then passed a task whose column the predicate compared against `in-progress` | | `isWorkspaceOwnerLive` | required `completeColumns` | | `recoverPausedAbortFailures` **body** | #3075 converted this sweep's *router* and left three body guards comparing ids | | `reconcilePreExecutionWorktrees` | a four-id literal in a sweep that **removes worktrees** | | `recoverStarvedRefinementTriageTasks` | 2 peer counts that read zero, so escalation never fired | | `evaluateParkedAgentTaskLink` | the omitted `parkedColumns` argument | All through the **async** `resolveProjectColumnsForRoles`, whose only store read is `listWorkflowDefinitions()` — answerable under PostgreSQL. That is what separates these from the inert kind. **The half-converted sweep is the important one.** A router that resolves correctly feeding a body that compares ids is worse than converting neither: the route now fires on a renamed board and the body then acts on the wrong lane. The `moveTask` **target** is the sharp end — an undeclared target is rejected *except* under `recoveryRehome` with a legacy id (`moves.ts:570`, the #1411 escape hatch), so a converted route feeding the literal `"todo"` rehomes the card into a column its workflow does not declare, which is the state other reconcilers exist to repair. **A real dropped-behaviour bug**: `evaluateParkedAgentTaskLink` was called without `parkedColumns`, falling back to `LEGACY_PARKED_COLUMNS`. A live durable agent linked to a card resting in a renamed hold lane read as not-parked, so the safeguard preserving its task link never applied. ## Withdrawn: the `task:moved` fan-out I wrote the sync-IR conversion, measured it, removed it. `getTaskWorkflowSelectionImpl` returns `undefined` **unconditionally** under PostgreSQL, so `resolveTaskWorkflowIrSync` always answers with the default builtin IR and `columnsWithFlag` on it yields exactly the legacy ids — inert on every board. Worse than the literal, because **the literal is counted**. My own test passed only because its store mock supplied a renamed IR: it pinned the helper's shape, not production behaviour. The refutation is recorded in place, and `check-inert-sync-lane-conversions` exits 0 on this branch. ## One question, one answer An earlier pass of this work mapped the notification-attach guard onto a wider `activeWork` set, and `self-healing-paused-abort-recovery > "rehomes an in-progress pause-abort park back to todo"` caught it — an in-progress park attached a transition notification it should not have. The fix is not a narrower set. Both guards ask **one** question — *"is the card already at the requeue target?"* — which the literal happened to spell twice as `=== "todo"`. The target now resolves once, before the write, and both read it. Deriving one question two ways is exactly how a converted guard and an unconverted target drift apart. ## Census | | before | after | |---|---|---| | `self-healing.ts` | 23 | **11** | | repo backlog | 53 | **41** | ## Measured - `src/__tests__/self-healing*` + `task-agent*` — **42 files / 836 tests pass** - `tsc --noEmit -p packages/engine` clean; **`check-inert-sync-lane-conversions` exits 0**; census `--strict`, `check-lane-wiring`, `check-fnxc-future-dates` clean ## The remaining 11, flagged not guessed - **4** — the fan-out, withdrawn above; blocked on a sync-capable selection reader. - **1** — the log-dedup closure: pre-existing flag; it sits before the lane prefetch it needs, and the degraded answer costs a duplicate log line, not a lifecycle decision. - **1** — the synthetic `{ column: "todo" }` for a *missing* task: deliberate, and now correct rather than unconverted, because `parkedColumns` is legacy-seeded. - The rest are status/deliberate classifications the census counts but that are not lane guards. ## Summary by CodeRabbit * **Bug Fixes** * Improved workflow automation for boards using renamed or customized workflow lanes. * Fixed task completion fan-out, branch rebinding, recovery, and stalled-task detection across custom lifecycle columns. * Prevented completion actions from triggering when tasks move back to the work-in-progress lane. * Improved cleanup and pause recovery behavior for customized workflows. --------- Co-authored-by: Claude Opus 5 (1M context) --- .../self-healing-completion-fanout.test.ts | 97 ++++++++- packages/engine/src/self-healing.ts | 201 ++++++++++++++++-- .../lib/lifecycle-column-census-baseline.json | 2 +- 3 files changed, 273 insertions(+), 27 deletions(-) diff --git a/packages/engine/src/__tests__/self-healing-completion-fanout.test.ts b/packages/engine/src/__tests__/self-healing-completion-fanout.test.ts index 9a504e20fa..d6020a41d6 100644 --- a/packages/engine/src/__tests__/self-healing-completion-fanout.test.ts +++ b/packages/engine/src/__tests__/self-healing-completion-fanout.test.ts @@ -178,13 +178,22 @@ describe("self-healing completion fan-out", () => { store.emit("task:moved", { task: t, from: "in-review", to: "done", source: "user" }); store.emit("task:moved", { task: t, from: "done", to: "archived", source: "engine" }); store.emit("task:moved", { task: t, from: "in-review", to: "todo", source: "user" }); - await Promise.resolve(); - expect(spy).toHaveBeenCalledTimes(2); + /* + FNXC:WorkflowResolvedColumns 2026-07-31-23:40: + `await Promise.resolve()` was draining exactly one microtask, which coupled this case to the + number of awaits inside a FIRE-AND-FORGET path. The listener does not await the fan-out and never + did, so how many microtasks it takes is not the contract — "it ran, twice, for the right + transitions" is. Resolving the lanes adds an await, so the drain is now written against the + invariant instead of against the old await count. + */ + await vi.waitFor(() => { expect(spy).toHaveBeenCalledTimes(2); }); expect(spy).toHaveBeenNthCalledWith(1, "FN-L", { worktreeHint: undefined }); mgr.stop(); store.emit("task:moved", { task: t, from: "in-review", to: "done", source: "user" }); - await Promise.resolve(); + /* The negative keeps a real drain: an unwired listener must stay silent after several ticks, + not merely after one. */ + await new Promise((resolve) => setTimeout(resolve, 10)); expect(spy).toHaveBeenCalledTimes(2); }); @@ -201,3 +210,85 @@ describe("self-healing completion fan-out", () => { expect((await store.getTask("FN-D"))?.branch).toBeNull(); }); }); + +/* +FNXC:WorkflowResolvedColumns 2026-07-31-23:45: +THE `task:moved` FAN-OUT ON A RENAMED BOARD. + +Two of this listener's guards were keyed on `in-review`/`done`/`archived`, so on a board using none +of those ids a card entering its own review lane never had its branch rebound, and a card reaching +its own complete or archive lane never ran the completion fan-out — the worktree was never reclaimed +and dependents kept a `blockedBy` pointing at a blocker that had already finished. + +The SYNC-IR conversion of this listener is inert and was withdrawn. These two guards gate work the +listener already `void`s, so they can ask the ASYNC resolver instead without changing anything an +observer can see; the resolution reads `listWorkflowDefinitions()`, which is answerable under +PostgreSQL. + +The board-stall counter above them is deliberately NOT converted here: it mutates in-memory state in +the handler's own tick, so it is the one guard that genuinely needs a synchronous answer. +*/ +describe("the task:moved fan-out resolves the board's own lanes", () => { + /** Review `checking`, complete `shipped`, archive `filed` — no legacy id anywhere. */ + const RENAMED_IR = { + version: "v2", id: "wf-renamed", name: "renamed", nodes: [], edges: [], + columns: [ + { id: "building", name: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "checking", name: "Checking", traits: [{ trait: "merge" }] }, + { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] }, + { id: "filed", name: "Filed", traits: [{ trait: "archived" }] }, + ], + }; + + function renamedStore(task: Task) { + const base = createStore([task]) as unknown as TaskStore & EventEmitter; + (base as unknown as { listWorkflowDefinitions: unknown }).listWorkflowDefinitions = + vi.fn(async () => [{ ir: RENAMED_IR }]); + return base; + } + + it("runs the completion fan-out for the board's own review -> complete transition", async () => { + const t = makeTask("FN-R1", { column: "shipped" }); + const store = renamedStore(t); + const mgr = new SelfHealingManager(store, { rootDir: "/repo" }); + const spy = vi.spyOn(mgr, "reconcileCompletedTask").mockResolvedValue({ blockedByCleared: 0, worktreeRemoved: false, branchRemoved: false }); + + mgr.start(); + store.emit("task:moved", { task: t, from: "checking", to: "shipped", source: "engine" }); + + await vi.waitFor(() => { expect(spy).toHaveBeenCalledWith("FN-R1", { worktreeHint: undefined }); }); + mgr.stop(); + }); + + it("rebinds the branch on a move into the board's own review lane", async () => { + const t = makeTask("FN-R2", { column: "checking" }); + const store = renamedStore(t); + const mgr = new SelfHealingManager(store, { rootDir: "/repo" }); + const rebind = vi.spyOn(mgr, "reconcileInReviewBranchRebind").mockResolvedValue(0 as never); + + mgr.start(); + store.emit("task:moved", { task: t, from: "building", to: "checking", source: "engine" }); + + await vi.waitFor(() => { expect(rebind).toHaveBeenCalledWith({ includeTaskIds: new Set(["FN-R2"]) }); }); + mgr.stop(); + }); + + /* + The paired negative. The conversion widens membership, so it must not fan out on every move: a + `checking -> building` bounce is not a completion, and reconciling it would remove the worktree of + a card that is about to run again. + */ + it("does NOT run the completion fan-out for a bounce back into the wip lane", async () => { + const t = makeTask("FN-R3", { column: "building" }); + const store = renamedStore(t); + const mgr = new SelfHealingManager(store, { rootDir: "/repo" }); + const spy = vi.spyOn(mgr, "reconcileCompletedTask").mockResolvedValue({ blockedByCleared: 0, worktreeRemoved: false, branchRemoved: false }); + + mgr.start(); + store.emit("task:moved", { task: t, from: "checking", to: "building", source: "engine" }); + + await new Promise((resolve) => setTimeout(resolve, 10)); + expect(spy).not.toHaveBeenCalled(); + mgr.stop(); + }); +}); diff --git a/packages/engine/src/self-healing.ts b/packages/engine/src/self-healing.ts index f65fa1cd94..957e49073c 100644 --- a/packages/engine/src/self-healing.ts +++ b/packages/engine/src/self-healing.ts @@ -30,7 +30,7 @@ import { existsSync, mkdirSync, readdirSync, readFileSync, realpathSync, rmSync, import { readFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { isAbsolute, join, relative, resolve } from "node:path"; -import { resolveColumnFlags, IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, IN_REVIEW_STALL_TERMINAL_LOG_PREFIX, allowsAutoMergeProcessing, resolveEffectiveAutoMerge, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, evaluateNoCommitsNoOpFinalize, evaluateCompletedPromotionFailureProvenance, evaluateSkipBypassTaint, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isWorkspaceTask, isSharedBranchGroupMemberIntegration, isNearDuplicateCanonicalInactive, parseExplicitDuplicateMarker, flagTriageDuplicate, isTriageDuplicateKeepAcknowledged, resolveMaxAutoMergeRetries, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, getBuiltinWorkflow, isBuiltinWorkflowId, resolveWorkflowIrForTask, resolveWorkflowIrForTaskWithProvenance, resolveReboundTarget, columnsWithFlag, resolveLifecycleColumns, workflowHasColumn, planLegacyAdoption, resolveOrphanedPendingStepResults, classifyReviewLease, PLAN_REVIEW_LEASE_STALENESS_MS, DEFAULT_MAX_POST_REVIEW_FIXES, ACTIVE_WORKFLOW_WORK_ITEM_STATES, AWAITING_APPROVAL_PAUSE_REASON, type Agent, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult, type WorkflowStepResult, type WorkflowIr, +import { resolveColumnFlags, IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, IN_REVIEW_STALL_TERMINAL_LOG_PREFIX, allowsAutoMergeProcessing, resolveEffectiveAutoMerge, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, evaluateNoCommitsNoOpFinalize, evaluateCompletedPromotionFailureProvenance, evaluateSkipBypassTaint, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isWorkspaceTask, isSharedBranchGroupMemberIntegration, isNearDuplicateCanonicalInactive, parseExplicitDuplicateMarker, flagTriageDuplicate, isTriageDuplicateKeepAcknowledged, resolveMaxAutoMergeRetries, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, getBuiltinWorkflow, isBuiltinWorkflowId, resolveWorkflowIrForTask, resolveWorkflowIrForTaskWithProvenance, resolveReboundTarget, columnsWithFlag, resolveLifecycleColumns, resolveTaskLifecycleColumns, workflowHasColumn, planLegacyAdoption, resolveOrphanedPendingStepResults, classifyReviewLease, PLAN_REVIEW_LEASE_STALENESS_MS, DEFAULT_MAX_POST_REVIEW_FIXES, ACTIVE_WORKFLOW_WORK_ITEM_STATES, AWAITING_APPROVAL_PAUSE_REASON, type Agent, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult, type WorkflowStepResult, type WorkflowIr, LEGACY_COLUMN_IDS_BY_ROLE, TERMINAL_ROLES, resolveProjectColumnsForRoles, @@ -1165,9 +1165,19 @@ export class SelfHealingManager extends SelfHealingGitEvidence { this. (Distinct from `isWorkspaceTaskLive`, which probes the session REGISTRY; this probes the task ROW lifecycle.) */ - private isWorkspaceOwnerLive(owner: Task | null | undefined): boolean { + /* + FNXC:WorkflowResolvedColumns 2026-07-31-23:40: + `completeColumns` is REQUIRED and resolved by the caller (async, once per sweep) because this + predicate is sync and its caller is not. + + MEMBERSHIP of `complete` ONLY, deliberately: the literal it replaces was `done` alone, and the + comment above spells out that every non-terminal state must read LIVE so a lease is never yanked + from a task that could still be running. Adding `archived` would widen what counts as terminal and + reclaim leases the old code kept — a behaviour change riding inside a column conversion. + */ + private isWorkspaceOwnerLive(owner: Task | null | undefined, completeColumns: ReadonlySet): boolean { if (!owner) return false; // not found / deleted → terminal. - if (owner.column === "done") return false; + if (completeColumns.has(owner.column)) return false; if (owner.status === "failed") return false; return true; } @@ -1397,11 +1407,20 @@ export class SelfHealingManager extends SelfHealingGitEvidence { * FN-7566: the FN-6736 liveness gate (`agentPresent` heartbeat, `checkedOutBy` lease, `hasRecentRunAudit`) is structurally blind to EPHEMERAL EXECUTOR agents (`agentId: "executor"`): they never emit heartbeat runs (so `activeHeartbeatTaskIds` never contains them), never acquire a checkout lease (`checkedOutBy` stays null), and normal execution activity (sandbox:run / task:log / verification) writes no `runAuditEvents` rows (so `getRecentRunAuditActivityAgeMs` stays null). With all three permanently false, the ONLY surviving gate was age > graceMs*3 (~30 min), so any ephemeral executor task running longer than 30 minutes — a heavy foreach workflow, a slow model — was killed mid-flight on the next self-healing sweep and hard-moved to `todo`, corrupting overlapping-worktree/task-link state. * The fix adds the in-process live-session truth that DOES track ephemeral executors: a worktree path registered as active in `activeSessionRegistry` (the executor/step-session/workflow-step session holds it for the whole run), the `executingTaskLock`, or `isTaskActive`. This mirrors the canonical `isWorkspaceTaskLive` / `sessionDead` predicate. A genuinely leaked binding (FN-6736) still has an EMPTY registry / no lock / inactive task, so legitimate phantom recovery is preserved; a live ephemeral executor now vetoes the phantom verdict regardless of the durable-agent signals. */ + /* + FNXC:WorkflowResolvedColumns 2026-07-31-23:40: + `wipColumns` is REQUIRED and supplied by the caller, which resolved the SAME set three lines above + the call (`lanesOfReclaim(task.id).wip`) to decide whether to invoke this at all, and then passed a + task whose column this predicate compared against the `in-progress` literal. Two reads of one + board, one resolved and one not — three lines apart. + */ private isPhantomExecutorBinding(task: Task, options: { executionAgeMs: number | null; graceMs: number; activeHeartbeatTaskIds: Set; lastActivityMs: number | null; + /** Wip MEMBERSHIP — replaces the `in-progress` literal. */ + wipColumns: ReadonlySet; }): { phantom: boolean; metadata: Record } { const normalizedId = task.id.toUpperCase(); const agentPresent = options.activeHeartbeatTaskIds.has(normalizedId); @@ -1434,7 +1453,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { }; return { - phantom: task.column === "in-progress" + phantom: options.wipColumns.has(task.column) && worktreeExists && options.executionAgeMs !== null && options.executionAgeMs > safeAgeMs @@ -1523,6 +1542,40 @@ export class SelfHealingManager extends SelfHealingGitEvidence { }; this.store.on("settings:updated", this.settingsListener); + /* + FNXC:WorkflowResolvedColumns 2026-07-31-23:50 (FLAGGED AND LEFT COUNTED — a conversion I wrote, + measured, and withdrew): + + These four guards are dead on a renamed board: the stall counter reads zero, the review rebind + never runs, and the completion fan-out never reclaims a worktree or clears a dependent's + `blockedBy`. All real. The obvious conversion does NOT fix them. + + `task:moved` is emitted synchronously, so an `await` here defers everything after it to a + microtask and reorders this handler against every other subscriber — which points at the store's + sync IR path, the way the scheduler's `resolveTaskParkedColumnsSync` does. I built exactly that + and it is INERT: + + `getTaskWorkflowSelectionImpl` returns `undefined` UNCONDITIONALLY — "Backend mode cannot + synchronously read PostgreSQL" — so `resolveTaskWorkflowIrSync` always takes its `!workflowId` + branch and answers with the DEFAULT builtin IR. `columnsWithFlag` on that IR yields exactly + `todo / in-progress / in-review / done / archived`. Identical to the literals, on every board. + + Worse than the literal, because the literal is COUNTED: four guards would leave the census, the + file would read as converted, and the next reader would have no reason to look. My own test + passed only because its store mock supplied a renamed IR — it pinned the helper's shape, not + production behaviour. Same defect as #3051's ten scheduler guards; see #3058 and the call-site + allow-list in `sync-workflow-ir-callsite-allowlist.test.ts`. + + SCOPE, after the async conversion below: this note now covers ONLY the board-stall counter, which + mutates in-memory state in the handler's own tick and so genuinely needs a synchronous answer. The + other two guards gate work the listener already `void`s, so they ask the ASYNC resolver instead — + see the note on them. Splitting the four this way is the whole finding: "the listener is sync" was + never the real constraint, "this particular guard's ANSWER is consumed synchronously" is. + + THE REAL BLOCKER for the counter is a sync path that can answer for a CUSTOM workflow, and there + are two things in the way, not one (`sync-workflow-ir-second-blocker.test.ts`). Until then the + literals here stay, counted. + */ this.taskMovedFanoutListener = ({ task, from, to }) => { if ( from === "in-progress" @@ -1532,19 +1585,57 @@ export class SelfHealingManager extends SelfHealingGitEvidence { // In-memory only counter; resets on engine restart. this.boardStallWindow.transitionsOutOfInProgressInWindow++; } - if (to === "in-review") { - void this.reconcileInReviewBranchRebind({ includeTaskIds: new Set([task.id]) }).catch((err: unknown) => { + /* + FNXC:WorkflowResolvedColumns 2026-07-31-23:35 (the ASYNC path, which the sync one could not be): + THESE TWO GUARDS GATE WORK THAT WAS ALREADY FIRE-AND-FORGET, so they can ask the async resolver + without changing anything an observer can see. + + The sync-IR conversion of this listener is inert and was withdrawn (see the note above). But + that is a constraint on the SYNC path, and only the board-stall counter above genuinely needs a + synchronous answer — it mutates in-memory state in the handler's own tick. The two calls below + are already `void`-ed: the listener does not await them, and their bodies already begin at a + microtask boundary. + + Moving the guard into that same boundary therefore preserves the ordering property the withdrawn + conversion would have broken. An async IIFE runs synchronously only up to its first `await`, + which here is the first statement, so the rest of this listener continues in the same tick + exactly as before — identical to how `void this.reconcileCompletedTask(...)` already behaved. + + The resolution goes through the ASYNC `resolveProjectColumnsForRoles`, whose only store read is + `listWorkflowDefinitions()` — answerable under PostgreSQL, and unaffected by either of the two + blockers that make the sync path inert (`sync-workflow-ir-second-blocker.test.ts`). + + What this fixes, on every renamed board: a card entering the board's own review lane never had + its branch rebound, and a card reaching the board's own complete or archive lane never ran the + completion fan-out — so its worktree was never reclaimed and its dependents kept a `blockedBy` + pointing at a blocker that had already finished. + */ + void (async () => { + /* One await, not three: the fan-out is fire-and-forget, so its deferral is unobservable, but + there is no reason to add microtasks the resolution does not need. */ + const [review, complete, archived] = await Promise.all([ + resolveProjectColumnsForRoles(this.store, REVIEW_ROLES), + resolveProjectColumnsForRoles(this.store, ["complete"]), + resolveProjectColumnsForRoles(this.store, ["archived"]), + ]); + const lanes = { review, complete, archived }; + if (lanes.review.has(to)) { + await this.reconcileInReviewBranchRebind({ includeTaskIds: new Set([task.id]) }).catch((err: unknown) => { + const errorMessage = err instanceof Error ? err.message : String(err); + log.warn(`[self-healing] task:moved in-review rebind failed for ${task.id}: ${errorMessage}`); + }); + } + const shouldReconcile = + (lanes.review.has(from) && lanes.complete.has(to)) || + (lanes.complete.has(from) && lanes.archived.has(to)); + if (!shouldReconcile) return; + await this.reconcileCompletedTask(task.id, { worktreeHint: task.worktree ?? undefined }).catch((err: unknown) => { const errorMessage = err instanceof Error ? err.message : String(err); - log.warn(`[self-healing] task:moved in-review rebind failed for ${task.id}: ${errorMessage}`); + log.warn(`[self-healing] task:moved completion fan-out failed for ${task.id}: ${errorMessage}`); }); - } - const shouldReconcile = - (from === "in-review" && to === "done") || - (from === "done" && to === "archived"); - if (!shouldReconcile) return; - void this.reconcileCompletedTask(task.id, { worktreeHint: task.worktree ?? undefined }).catch((err: unknown) => { + })().catch((err: unknown) => { const errorMessage = err instanceof Error ? err.message : String(err); - log.warn(`[self-healing] task:moved completion fan-out failed for ${task.id}: ${errorMessage}`); + log.warn(`[self-healing] task:moved fan-out lane resolution failed for ${task.id}: ${errorMessage}`); }); }; this.store.on("task:moved", this.taskMovedFanoutListener); @@ -3956,6 +4047,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { executionAgeMs, graceMs: STALE_ACTIVE_BRANCH_EXECUTION_GRACE_MS, activeHeartbeatTaskIds: activeTaskIds, + wipColumns: lanesOfReclaim(task.id).wip, lastActivityMs, }); @@ -9792,6 +9884,10 @@ export class SelfHealingManager extends SelfHealingGitEvidence { const entries = activeSessionRegistry.entriesByKind("workspace-repo-land"); if (entries.length === 0) return 0; + /* FNXC:WorkflowResolvedColumns 2026-07-31-23:40: resolved once per sweep, AFTER the early + return above, so a board with no leases pays nothing. See `isWorkspaceOwnerLive`. */ + const leaseOwnerCompleteColumns = await resolveProjectColumnsForRoles(this.store, ["complete"]); + const graceMs = settings.taskStuckTimeoutMs ?? STALE_ACTIVE_BRANCH_EXECUTION_GRACE_MS; const staleFloorMs = graceMs * PHANTOM_EXECUTOR_BINDING_AGE_MULTIPLIER; const activeMergeTaskId = this.options.getActiveMergeTaskId?.() ?? null; @@ -9821,7 +9917,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { const owner = await this.store.getTask(entry.taskId).catch(() => null); const ownerColumn = owner?.column ?? "deleted"; // Only a DEMONSTRABLY TERMINAL owner's lease is reclaimed (review C fix). - if (this.isWorkspaceOwnerLive(owner)) continue; + if (this.isWorkspaceOwnerLive(owner, leaseOwnerCompleteColumns)) continue; activeSessionRegistry.unregisterPath(entry.path); await createRunAuditor(this.store, { @@ -9870,13 +9966,24 @@ export class SelfHealingManager extends SelfHealingGitEvidence { if (settings.globalPause || settings.enginePaused) return 0; const now = Date.now(); + /* FNXC:WorkflowResolvedColumns 2026-07-31-23:40: resolved once per sweep — see the guard below. */ + const preExecLiveColumns = await resolveProjectColumnsForRoles( + this.store, + ["intake", "hold", "countsTowardWip", ...REVIEW_ROLES, "complete"], + ); const parked = await this.store.listTasks({ slim: true }); const candidates = parked.filter((task) => { if (!task.worktree || task.deletedAt) return false; // Execution evidence — the worktree may hold real work; only the merge/archive lifecycle owns it. if (task.firstExecutionAt || task.executionStartedAt) return false; - // Columns where a card is active or queued to become active. - if (task.column === "todo" || task.column === "in-progress" || task.column === "in-review" || task.column === "done") return false; + /* + FNXC:WorkflowResolvedColumns 2026-07-31-23:40: + Columns where a card is active or queued to become active — resolved MEMBERSHIP. An EXCLUSION + from a sweep that REMOVES worktrees, so a legacy-seeded superset can only be conservative; + the literal's failure mode was the dangerous direction, treating a live card on a renamed + board as pre-execution and removing its worktree. + */ + if (preExecLiveColumns.has(task.column)) return false; /* WAITING is not PARKED. A card paused for an operator decision, carrying any status (planning, needs-replan, awaiting-*), blocked on another task, or scheduled for a recovery attempt is @@ -12212,15 +12319,36 @@ export class SelfHealingManager extends SelfHealingGitEvidence { createdAt: new Date().toISOString(), } : undefined; + /* + FNXC:WorkflowResolvedColumns 2026-07-31-23:45: + THE ROUTER WAS CONVERTED (#3075) AND THESE BODY GUARDS WERE NOT — the half-converted state + that is worse than converting nothing, because the route now fires correctly on a renamed + board and the body then acts on the wrong lane. + + The TARGET is the dangerous one: `moveTask` REJECTS a column the workflow does not declare, + EXCEPT under `recoveryRehome` with a legacy id (`moves.ts:570`, the #1411 escape hatch). A + converted route feeding the literal `"todo"` rehomes the card into an undeclared column — + the state other reconcilers exist to repair. + + `resolveTaskLifecycleColumns` is first-match per role: wrong for "is this card waiting", + exactly right for "where should it land". Precedence hold -> intake -> legacy is the one + `moves.ts:1296` documents, because a workflow may declare intake and no hold. + + Both guards below ask ONE question — "is the card already at the requeue target?" — which + the literal happened to spell twice. Resolved once, before the write, and read by both: + deriving one question two ways is how a converted guard and an unconverted target drift. + */ + const requeueLifecycle = await resolveTaskLifecycleColumns(this.store, task.id); + const requeueTarget = requeueLifecycle?.hold ?? requeueLifecycle?.intake ?? "todo"; await this.store.updateTask(task.id, { status: null, error: null, - ...(fresh.column === "todo" && workflowTransitionNotification + ...(fresh.column === requeueTarget && workflowTransitionNotification ? { workflowTransitionNotification } : {}), }); - if (route.kind === "node-requeue" && fresh.column !== "todo") { - await this.store.moveTask(task.id, "todo", { + if (route.kind === "node-requeue" && fresh.column !== requeueTarget) { + await this.store.moveTask(task.id, requeueTarget, { preserveProgress: true, moveSource: "engine", recoveryRehome: true, @@ -12258,7 +12386,9 @@ export class SelfHealingManager extends SelfHealingGitEvidence { await this.store.logEntry( task.id, - fresh.column === "in-review" + /* The log line names WHICH route ran, so it asks the same resolved review membership the + router did — a literal here describes a renamed-board recovery as the wrong kind. */ + freshColumns.review.has(fresh.column) ? "Auto-recovered: in-review pause-abort park cleared — preserved for normal review progression" : "Auto-recovered: pause-abort park cleared — requeued for normal scheduling", ); @@ -13012,6 +13142,9 @@ export class SelfHealingManager extends SelfHealingGitEvidence { ...await resolveProjectColumnsForRoles(this.store, REVIEW_ROLES), ]); + /* FNXC:WorkflowResolvedColumns 2026-07-31-23:45: the parked pair handed to + `evaluateParkedAgentTaskLink`; legacy-seeded, so unconverted boards are unchanged. */ + const agentParkedColumns = await resolveProjectColumnsForRoles(this.store, ["hold", "intake"]); for (const agent of runningAgents) { if (isEphemeralAgent(agent) || !agent.taskId) { continue; @@ -13027,7 +13160,21 @@ export class SelfHealingManager extends SelfHealingGitEvidence { const activeRun = await agentStore.getActiveHeartbeatRun(agent.id); const proof = evaluateParkedAgentTaskLink({ agent, + /* + FNXC:WorkflowResolvedColumns 2026-07-31-23:45: + The synthetic stand-in for a MISSING task stays the legacy id ON PURPOSE, and is now correct + rather than merely unconverted: `parkedColumns` below is legacy-seeded, so `todo` is a member + on every board. A deleted row also has no workflow selection, so a "renamed" placeholder + would be a guess about a task that no longer exists. + */ linkedTask: linkedTask ?? { column: "todo" } as Pick, + /* + FNXC:WorkflowResolvedColumns 2026-07-31-23:45: + Previously OMITTED, so this call fell back to `LEGACY_PARKED_COLUMNS` (`todo`/`triage`) and a + live agent linked to a card resting in a RENAMED hold lane read as not-parked — the safeguard + that preserves its task link never applied, and the link was dropped. + */ + parkedColumns: [...agentParkedColumns], activeRun, hasActiveAgentExecution: this.options.hasActiveAgentExecution, now, @@ -14248,6 +14395,14 @@ export class SelfHealingManager extends SelfHealingGitEvidence { this.options.evictStaleTriageProcessing?.(); const tasks = await this.store.listTasks({ slim: true, includeArchived: false }); + /* + FNXC:WorkflowResolvedColumns 2026-07-31-23:40: + Resolved WAITING membership for the two peer-progress counts below. The question is "are OTHER + queued cards moving while this refinement card starves", so it must include every lane a card + can wait in — hold and intake. Keyed on `todo` alone, a renamed board counted zero peers and + the starvation escalation never fired. + */ + const starvedWaitingColumns = await resolveProjectColumnsForRoles(this.store, ["hold", "intake"]); const planningIds = this.options.getPlanningTaskIds?.() ?? new Set(); const now = Date.now(); @@ -14271,7 +14426,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { const peerProgressCount = tasks.filter((peer) => peer.id !== task.id && - peer.column === "todo" && + starvedWaitingColumns.has(peer.column) && peer.sourceType !== "task_refine" && new Date(peer.updatedAt).getTime() > createdAtMs, ).length; @@ -14292,7 +14447,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { const createdAtMs = new Date(task.createdAt).getTime(); const peerProgressCount = tasks.filter((peer) => peer.id !== task.id && - peer.column === "todo" && + starvedWaitingColumns.has(peer.column) && peer.sourceType !== "task_refine" && new Date(peer.updatedAt).getTime() > createdAtMs, ).length; diff --git a/scripts/lib/lifecycle-column-census-baseline.json b/scripts/lib/lifecycle-column-census-baseline.json index 37e5f91376..d704d8a1ff 100644 --- a/scripts/lib/lifecycle-column-census-baseline.json +++ b/scripts/lib/lifecycle-column-census-baseline.json @@ -1,7 +1,7 @@ { "generatedFrom": "node scripts/lifecycle-column-census.mjs --strict --update-baseline", "byFile": { - "packages/engine/src/self-healing.ts": 22, + "packages/engine/src/self-healing.ts": 6, "packages/engine/src/executor.ts": 4, "packages/dashboard/app/utils/taskRevert.ts": 2, "packages/engine/src/auto-merge-finalization.ts": 2,