From f9f06a4fb73524f6d68bc211a25331b2c6275c7a Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 11:28:54 -0700 Subject: [PATCH] =?UTF-8?q?engine=20tail:=20nine=20files=20to=20zero=20?= =?UTF-8?q?=E2=80=94=20incl.=20a=20census-INVISIBLE=20requeue=20into=20a?= =?UTF-8?q?=20lane=20that=20does=20not=20exist=20(=E2=88=9213)=20(#2797)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #2785. Nine engine files to **zero**, all one question — *"is this task finished?"* — asked in nine places, wrong in every one on a renamed board. ## Census, per file (measured, `--strict` verified) | file | main | here | | --- | ---: | ---: | | `agent-reflection.ts` | 2 | **0** | | `merger-scope-auto-widen.ts` | 2 | **0** | | `worktree-pool.ts` | 2 | **0** | | `cli-agent/state-machine.ts` | 2 | **0** (reclassified — see below) | | `auto-recovery-handlers/branch-worktree.ts` | 1 | **0** | | `cli-agent/task-session.ts` | 1 | **0** (reclassified) | | `merger-integration-worktree.ts` | 1 | **0** | | `merger-orphan-rehome.ts` | 1 | **0** | | `plugin-runner.ts` | 1 | **0** | | **net** | | **−13** | ## The one worth reading: `branch-worktree` had TWO defects, and the census could only see one ```ts if (task.column === "in-progress") { …clear branch… } // counted await this.deps.taskStore.moveTask(task.id, "todo", { … }); // INVISIBLE ``` The census scores **comparisons**. The requeue *destination* is a call argument, so nothing in the backlog ever pointed at it — and it is the worse of the two: a board with no `todo` column was requeued into a lane **that does not exist**. The counted literal is the smaller half (a renamed wip lane meant the stale branch was never cleared, so the card carried a dead branch back into execution). Converting the comparison alone would have dropped a census count and left the board requeuing into nowhere. Destination now resolves through `resolveReboundTarget` (KTD-10 ordering: hold → intake → first column). Reverted **independently**: destination restored → 2 fail (`moveTask` called with `"todo"`, not `"backlog"`); wip test restored → 1 fail (`updateTask` never called). ## The rest - **`plugin-runner`** — `onTaskCompleted` never fired on a renamed board. Every plugin that closes an issue, posts a notification, or records a metric on completion **silently stopped**, with nothing logged. Resolved *asynchronously* inside the existing fire-and-forget seam, not via `resolveTaskWorkflowIrSync` — per `sync-workflow-ir-callsite-allowlist` that reader returns the DEFAULT workflow for every task in production, so a sync guard here would read as converted and still be wrong. The listener is already `void`-dispatched, so awaiting inside it changes no observable ordering (the shape `NotificationService` already uses). - **`merger-orphan-rehome`** — a renamed complete lane made every source task read as unfinished, so orphaned commits were never rehomed and stayed stranded off the integration branch. Resolves by the **trailer id**, not `sourceTask.id`, which the fake store does not populate. - **`agent-reflection`** — `classifyOutcome` returned `null` for every finished task, so both callers treated completed work as nothing to reflect on: one recorded `reflection:skipped` with reason `"not-completed"`, the other silently `continue`d. Reflection captured **nothing at all** on a custom board. - **`worktree-pool`** — shipped tasks' worktrees stayed in the ACTIVE set, so the reclaim pass never returned them and the board walks into worktree exhaustion — a stall whose cause is invisible from the symptom. - **`merger-scope-auto-widen`** — finished cards counted as active claimants, so a merge was blocked by a task that no longer exists in any meaningful sense. - **`merger-integration-worktree`** — a shipped task still counted as a live worktree user, so the integration worktree could never be reused and the merge path took the slower rebuild every time. ## The census OVERSTATED the engine backlog by 3 `cli-agent/state-machine.ts` and `cli-agent/task-session.ts` compare against `done` — but that is a **`CliMachineState`** (`ready`/`busy`/`waitingOnInput`/`done`/`resuming`/`idle`) tracking one CLI agent process. It never reads a board column. The census matches the bare string. Marked `DELIBERATE-LITERAL` rather than left for a later sweep to "convert" a process state into a workflow role. Worth flagging fleet-wide: the backlog total includes at least these three non-columns. ## Revert results (measured, each run) | conversion | reverted → | | --- | --- | | `plugin-runner` complete gate | RENAMED case fails — `onTaskCompleted` never invoked | | `merger-orphan-rehome` source gate | RENAMED case fails — `orphan:false, reason:"source-task-not-done"` | | `branch-worktree` destination | 2 fail — `moveTask` called with `"todo"` | | `branch-worktree` wip test | 1 fail — `updateTask` never called | Each has a **non-vacuous companion** (renamed board, non-complete lane / mid-flight source / non-wip column) so a guard that fired unconditionally would not pass. **Four are NOT revert-proven, and I am not claiming otherwise:** `agent-reflection`, `merger-scope-auto-widen`, `merger-integration-worktree`, `worktree-pool`. Their suites omit a workflow and therefore assert the legacy fallback — they pass before and after. `merger-scope-auto-widen` has no test file at all; `scanIdleWorktrees` is mocked in every suite that touches it and driving it for real needs git worktrees on disk. All four strictly **widen** the finished set (resolved roles ∪ the legacy ids), so default boards are byte-identical. That is the argument for shipping them, not a substitute for coverage. ## Examined and deliberately NOT converted - **`backlog-pressure-reporter:173`** — fed by `listTasks({ column: "todo" })`, a hardcoded **query** filter. On a renamed board `todoFull` is empty and the predicate never runs. Converting it drops a census count and changes nothing observable; the fix belongs at the query layer. - **`auto-merge-finalization:28`** — the catch-arm legacy fallback, which must stay for the same reason `columnRoles.ts` keeps its id fallback. - **`auto-merge-finalization:84`** — only selects between two diagnostic reason strings that are **both** `ok: false`, on a pure validator with no store in scope. Converting it would thread a store through a pure function to change a label. ## Merge resolution note Merging main brought conflicts in `agent-assignment.ts` and `ephemeral-worker-manager.ts`. **Main's versions won both** and mine are dropped: main threads an optional `activeColumns` from `scheduler.ts:2340` (a cleaner seam than widening the store type to resolve internally), and its `isAgentIdle` carries a greptile P1 fix mine lacked — `columnsWithFlag` membership rather than first-per-role, so a workflow declaring two wip lanes has both recognised. That is the fourth time in this program main's version of a contested file was the better one. ## Verification - `pnpm test:gate` — 161 + 487 + 13 + 71, green - `npx tsc -p packages/engine/tsconfig.json --noEmit` — clean - `pnpm lint` — clean - `--strict` exits 0 ## Summary by CodeRabbit * **Bug Fixes** * Workflow-dependent task completion now recognizes custom lifecycle columns, including renamed boards. * Recovery requeues tasks to the configured destination and clears branch details only from the appropriate work-in-progress column. * Improved handling of completed tasks, orphaned work, shared worktrees, and scope evaluation across custom workflows. * Plugin completion hooks now trigger for any column configured as complete. * **Tests** * Added coverage for renamed workflow columns and custom completion, recovery, and rehoming behavior. * **Documentation** * Clarified CLI state terminology in internal developer comments. --------- Co-authored-by: Claude Opus 5 (1M context) --- .../auto-recovery-branch-worktree.test.ts | 158 +++++++++++++++++- .../__tests__/merger-orphan-rehome.test.ts | 88 +++++++++- .../src/__tests__/plugin-runner.test.ts | 57 +++++++ packages/engine/src/agent-reflection.ts | 35 +++- .../auto-recovery-handlers/branch-worktree.ts | 123 +++++++++++++- .../engine/src/cli-agent/state-machine.ts | 8 + packages/engine/src/cli-agent/task-session.ts | 2 + .../engine/src/merger-integration-worktree.ts | 21 ++- packages/engine/src/merger-orphan-rehome.ts | 14 +- .../engine/src/merger-scope-auto-widen.ts | 32 +++- packages/engine/src/plugin-runner.ts | 32 +++- packages/engine/src/run-audit.ts | 8 + packages/engine/src/worktree-pool.ts | 30 +++- .../lib/lifecycle-column-census-baseline.json | 16 +- 14 files changed, 587 insertions(+), 37 deletions(-) diff --git a/packages/engine/src/__tests__/auto-recovery-branch-worktree.test.ts b/packages/engine/src/__tests__/auto-recovery-branch-worktree.test.ts index 2bc3ed49a9..7132b716df 100644 --- a/packages/engine/src/__tests__/auto-recovery-branch-worktree.test.ts +++ b/packages/engine/src/__tests__/auto-recovery-branch-worktree.test.ts @@ -1,6 +1,8 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { AutoRecoveryContext, AutoRecoveryDecision, AutoRecoveryFailure } from "../auto-recovery.js"; import { BranchWorktreeAutoRecoveryHandler } from "../auto-recovery-handlers/branch-worktree.js"; +import { RENAMED_VOCAB, lifecycleIr } from "./_workflow-vocabulary-fixture.js"; +import { TransitionRejectionError } from "@fusion/core"; const branchConflictMocks = vi.hoisted(() => ({ inspectBranchConflict: vi.fn(), @@ -27,11 +29,24 @@ function createTask(overrides: Record = {}) { } as any; } -function createFixtures(taskOverrides: Record = {}, mode = "programmatic") { +/* +FNXC:WorkflowResolvedColumns 2026-07-30-13:50 (batch-engine tail): +`ir` is optional so every existing case is byte-identical: with no workflow the resolver degrades to the +built-in coding IR, whose wip lane is `in-progress` and whose rebound target is `todo` — exactly what +those cases already assert. +*/ +function createFixtures(taskOverrides: Record = {}, mode = "programmatic", ir?: unknown) { const task = createTask(taskOverrides); const taskStore = { updateTask: vi.fn(async () => undefined), moveTask: vi.fn(async () => undefined), + ...(ir + ? { + getTaskWorkflowSelectionAsync: async () => ({ workflowId: "recovery-lifecycle", stepIds: [] }), + getTaskWorkflowSelection: () => ({ workflowId: "recovery-lifecycle", stepIds: [] }), + getWorkflowDefinition: async (id: string) => (id === "recovery-lifecycle" ? { ir } : undefined), + } + : {}), } as any; const runAudit = { database: vi.fn(async () => undefined), git: vi.fn(), filesystem: vi.fn() } as any; const logger = { warn: vi.fn(), log: vi.fn(), error: vi.fn() } as any; @@ -57,6 +72,147 @@ describe("BranchWorktreeAutoRecoveryHandler", () => { expect(f.runAudit.database).toHaveBeenCalledWith(expect.objectContaining({ type: "branch-worktree:auto-requeue" })); }); + /* + FNXC:WorkflowResolvedColumns 2026-07-30-13:50 (batch-engine tail): + TWO defects, one case. The requeue destination was the hardcoded `todo` — CENSUS-INVISIBLE, because + the census scores comparisons and that is a call argument — so a board with no `todo` column was + requeued into a lane that does not exist. And the WIP test was the id `in-progress`, so the stale + branch/baseCommitSha were never cleared and the card carried a dead branch back into execution. + + REVERT CHECK, measured (both, independently): + - `moveTask(..., "todo", ...)` restored -> this fails; moveTask is called with "todo", not "backlog". + - `task.column === "in-progress"` restored -> this fails; updateTask is never called. + The legacy cases pass both ways, which is why they are kept alongside. + */ + it("requeues to the RESOLVED rebound target and clears the branch on a RENAMED board", async () => { + const f = createFixtures( + { column: RENAMED_VOCAB.wip }, + "programmatic", + lifecycleIr(RENAMED_VOCAB, "recovery-lifecycle"), + ); + branchConflictMocks.inspectBranchConflict.mockResolvedValue({ kind: "fully-subsumed", livePath: "/tmp/wt", tipSha: "abc" }); + + await f.handler.issueRetry(f.failure, f.decision, f.ctx); + + expect(f.taskStore.updateTask).toHaveBeenCalledWith("FN-4536", { branch: null, baseCommitSha: null }); + expect(f.taskStore.moveTask).toHaveBeenCalledWith( + "FN-4536", + RENAMED_VOCAB.hold, + expect.objectContaining({ moveSource: "engine", preserveWorktree: false }), + ); + expect(f.taskStore.moveTask).not.toHaveBeenCalledWith("FN-4536", "todo", expect.anything()); + }); + + /* + FNXC:WorkflowResolvedColumns 2026-07-30-13:45 (#2797 review — greptile): + + THE REQUEUE MUST NOT DIE ON A DESTINATION THE BOARD DOES NOT DECLARE. + + The review pointed at the `catch` retaining `reboundTarget = "todo"`. Writing the test for that + branch DISPROVED it as the main route: `resolveWorkflowIrForTask` degrades to the BUILT-IN IR rather + than throwing, so a task whose custom workflow cannot be read still resolves `todo` from the DEFAULT + board and the catch never runs. A guard on the throw path alone would have passed review and fixed + almost nothing. + + What actually bites is the move. `moveTaskInternal` REJECTS a column the workflow does not declare, + and unhandled that throws out of the recovery handler whose entire job is to unstick the task — so + the recovery became a second way to stay stuck, with no audit row explaining it. + + This drives the rejection itself, which is the behaviour every route ends at. + */ + it("records a skip instead of throwing when the rebound destination is rejected", async () => { + const f = createFixtures({ column: "building" }, "programmatic"); + /* + FNXC:WorkflowResolvedColumns 2026-07-30-17:45 (#2797 review — greptile P1): + A REAL TransitionRejectionError, not a look-alike Error whose message happens to read like one. + The reason is now derived from the typed `rejection.code`, so a plain Error must NOT be classified + as a lane problem — which is the point of the companion case below. + */ + f.taskStore.moveTask.mockRejectedValue( + new TransitionRejectionError( + { code: "unknown-column", messageKey: "transition.rejected.unknownColumn", retryable: false, detail: "Column 'todo' is not defined in this task's workflow" } as never, + "Invalid transition: 'building' -> 'todo'. Unknown column for this workflow.", + ), + ); + branchConflictMocks.inspectBranchConflict.mockResolvedValue({ kind: "fully-subsumed", livePath: "/tmp/wt", tipSha: "abc" }); + + /* The handler must not propagate — that is the regression. */ + await expect(f.handler.issueRetry(f.failure, f.decision, f.ctx)).resolves.not.toThrow(); + + expect(f.runAudit.database).toHaveBeenCalledWith(expect.objectContaining({ + type: "branch-worktree:auto-requeue-skipped", + metadata: expect.objectContaining({ reason: "rebound-target-rejected", rejectionCode: "unknown-column", reboundTarget: "todo" }), + })); + /* And the success audit must NOT be written for a move that did not happen. */ + expect(f.runAudit.database).not.toHaveBeenCalledWith(expect.objectContaining({ type: "branch-worktree:auto-requeue" })); + }); + + /* + FNXC:WorkflowResolvedColumns 2026-07-30-17:45 (#2797 review — greptile P1 "move failures become + successful retries"): + A moveTask failure that is NOT a lane problem — capacity exhaustion, a guard rejection, a deleted + task, a persistence error — was labelled `rebound-target-rejected` all the same, so the audit row + asserted a lane cause for something that has nothing to do with lanes and anyone debugging a stuck + card would chase the wrong thing. + + REVERT CHECK, measured: collapsing the reason back to the single literal makes this fail — the row + reads `rebound-target-rejected` for a plain persistence error. + */ + it("names a non-lane move failure honestly instead of blaming the rebound target", async () => { + const f = createFixtures({ column: "in-progress" }, "programmatic"); + f.taskStore.moveTask.mockRejectedValue(new Error("database connection lost")); + branchConflictMocks.inspectBranchConflict.mockResolvedValue({ kind: "fully-subsumed", livePath: "/tmp/wt", tipSha: "abc" }); + + await expect(f.handler.issueRetry(f.failure, f.decision, f.ctx)).resolves.not.toThrow(); + + expect(f.runAudit.database).toHaveBeenCalledWith(expect.objectContaining({ + type: "branch-worktree:auto-requeue-skipped", + metadata: expect.objectContaining({ reason: "requeue-move-failed" }), + })); + expect(f.runAudit.database).not.toHaveBeenCalledWith(expect.objectContaining({ + metadata: expect.objectContaining({ reason: "rebound-target-rejected" }), + })); + }); + + /* + FNXC:WorkflowResolvedColumns 2026-07-30-17:45 (#2797 review — greptile P1 "rejected move erases + branch linkage"): + The branch/baseCommitSha clear used to run BEFORE the move, so a rejected move left the card in its + wip lane with the only pointers back to its work already erased — the recovery destroyed the linkage + and then declined to requeue. Half-applied is worse than not applied; nothing reconstructs the branch + from the row afterwards. + + REVERT CHECK, measured: moving the clear back above the move makes this fail — updateTask is called + with { branch: null, baseCommitSha: null } on a move that never landed. + */ + it("preserves the branch linkage when the requeue move is rejected", async () => { + const f = createFixtures({ column: "in-progress" }, "programmatic"); + f.taskStore.moveTask.mockRejectedValue(new Error("database connection lost")); + branchConflictMocks.inspectBranchConflict.mockResolvedValue({ kind: "fully-subsumed", livePath: "/tmp/wt", tipSha: "abc" }); + + await f.handler.issueRetry(f.failure, f.decision, f.ctx); + + expect(f.taskStore.updateTask).not.toHaveBeenCalledWith("FN-4536", { branch: null, baseCommitSha: null }); + }); + + it("does not clear the branch when a RENAMED board's card is not in its wip lane", async () => { + /* + Non-vacuous companion: without it, a guard that cleared unconditionally would pass the case above. + Same renamed board, same failure — only the card's lane changes. + */ + const f = createFixtures( + { column: RENAMED_VOCAB.review }, + "programmatic", + lifecycleIr(RENAMED_VOCAB, "recovery-lifecycle"), + ); + branchConflictMocks.inspectBranchConflict.mockResolvedValue({ kind: "fully-subsumed", livePath: "/tmp/wt", tipSha: "abc" }); + + await f.handler.issueRetry(f.failure, f.decision, f.ctx); + + expect(f.taskStore.updateTask).not.toHaveBeenCalled(); + expect(f.taskStore.moveTask).toHaveBeenCalledWith("FN-4536", RENAMED_VOCAB.hold, expect.anything()); + }); + it("reanchors bootstrap misbinding then requeues", async () => { const f = createFixtures(); branchConflictMocks.inspectBranchConflict.mockResolvedValue({ kind: "reclaimable", livePath: "/tmp/wt", tipSha: "abc", taskAttributedCommitCount: 0, strandedCommits: [] }); diff --git a/packages/engine/src/__tests__/merger-orphan-rehome.test.ts b/packages/engine/src/__tests__/merger-orphan-rehome.test.ts index 60c995f1cc..b98b43ce4b 100644 --- a/packages/engine/src/__tests__/merger-orphan-rehome.test.ts +++ b/packages/engine/src/__tests__/merger-orphan-rehome.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, afterAll } from "vitest"; import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { join } from "node:path"; +import { RENAMED_VOCAB, lifecycleIr } from "./_workflow-vocabulary-fixture.js"; import { tmpdir } from "node:os"; import { execSync } from "node:child_process"; import { @@ -41,9 +42,21 @@ function setupRepo() { return dir; } -function makeFakeStore(tasks: Record) { +/* +FNXC:WorkflowResolvedColumns 2026-07-30-13:20 (batch-engine tail): +`ir` is optional so every existing caller is byte-identical: with no workflow the resolver degrades to +the built-in coding IR, whose complete lane IS `done`, which is what these cases already assert. +*/ +function makeFakeStore(tasks: Record, ir?: unknown) { return { getTask: async (id: string) => tasks[id.toUpperCase()] ?? null, + ...(ir + ? { + getTaskWorkflowSelectionAsync: async () => ({ workflowId: "orphan-lifecycle", stepIds: [] }), + getTaskWorkflowSelection: () => ({ workflowId: "orphan-lifecycle", stepIds: [] }), + getWorkflowDefinition: async (id: string) => (id === "orphan-lifecycle" ? { ir } : undefined), + } + : {}), } as any; } @@ -88,6 +101,79 @@ describe("classifyOrphanOurAdvance", () => { } }); + /* + FNXC:WorkflowResolvedColumns 2026-07-30-13:20 (batch-engine tail): + The case above proves orphan classification for the LEGACY complete id, which is what the guard + compared against — it passed before this conversion and would pass for a broken one. On a renamed + complete lane the source read as unfinished, so orphaned commits were never rehomed and stayed + stranded off the integration branch with no error surfaced. + + REVERT CHECK, measured: with `sourceTask.column !== "done"` restored, this fails with + `orphan: false, reason: "source-task-not-done"`. + */ + it("classifies the orphan when the source task sits in a RENAMED complete lane", async () => { + const dir = setupRepo(); + try { + git(dir, "git checkout -b sibling-orphan-renamed"); + writeFileSync(join(dir, "orphan-renamed.txt"), "orphan\n"); + git(dir, "git add orphan-renamed.txt"); + git(dir, `git commit -m "feat(FN-5551): orphaned squash" -m "Fusion-Task-Id: FN-5551"`); + const orphanSha = git(dir, "git rev-parse HEAD"); + git(dir, "git checkout main"); + + const result = await classifyOrphanOurAdvance({ + repoDir: dir, + taskStore: makeFakeStore( + { "FN-5551": { column: RENAMED_VOCAB.complete } }, + lifecycleIr(RENAMED_VOCAB, "orphan-lifecycle"), + ), + integrationBranch: "main", + currentTaskId: "FN-5419", + commitSha: orphanSha, + commitSubject: "feat(FN-5551): orphaned squash", + commitBody: "Fusion-Task-Id: FN-5551\n", + }); + + expect(result.orphan).toBe(true); + } finally { + removeTmpDirSync(dir); + } + }); + + it("still refuses on a RENAMED board when the source task is mid-flight", async () => { + /* + Non-vacuous companion: without it, a guard that treated every source as finished would pass the case + above. Same renamed board, same shape — only the source lane changes. + */ + const dir = setupRepo(); + try { + git(dir, "git checkout -b sibling-orphan-wip"); + writeFileSync(join(dir, "orphan-wip.txt"), "orphan\n"); + git(dir, "git add orphan-wip.txt"); + git(dir, `git commit -m "feat(FN-5551): orphaned squash" -m "Fusion-Task-Id: FN-5551"`); + const orphanSha = git(dir, "git rev-parse HEAD"); + git(dir, "git checkout main"); + + const result = await classifyOrphanOurAdvance({ + repoDir: dir, + taskStore: makeFakeStore( + { "FN-5551": { column: RENAMED_VOCAB.wip } }, + lifecycleIr(RENAMED_VOCAB, "orphan-lifecycle"), + ), + integrationBranch: "main", + currentTaskId: "FN-5419", + commitSha: orphanSha, + commitSubject: "feat(FN-5551): orphaned squash", + commitBody: "Fusion-Task-Id: FN-5551\n", + }); + + expect(result.orphan).toBe(false); + if (!result.orphan) expect(result.reason).toBe("source-task-not-done"); + } finally { + removeTmpDirSync(dir); + } + }); + it("refuses when source task is not done", async () => { const dir = setupRepo(); try { diff --git a/packages/engine/src/__tests__/plugin-runner.test.ts b/packages/engine/src/__tests__/plugin-runner.test.ts index 71b765243c..e9889ec7b9 100644 --- a/packages/engine/src/__tests__/plugin-runner.test.ts +++ b/packages/engine/src/__tests__/plugin-runner.test.ts @@ -7,6 +7,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { PluginRunner, type PluginRunnerOptions } from "../plugin-runner.js"; +import { RENAMED_VOCAB, lifecycleIr } from "./_workflow-vocabulary-fixture.js"; import { __resetWorkflowExtensionRegistryForTests, getWorkflowExtensionRegistry, @@ -1627,6 +1628,62 @@ describe("PluginRunner", () => { ); }); + /* + FNXC:WorkflowResolvedColumns 2026-07-30-12:40 (batch-engine tail): + The case above proves `onTaskCompleted` fires for the LEGACY id, which is what the guard compared + against — it passed before this conversion and would pass for a broken one. On a board whose complete + lane is renamed the hook NEVER fired: every plugin that closes an issue, posts a notification, or + records a metric on completion silently stopped, with nothing logged. + + REVERT CHECK, measured: with `if (to === "done")` restored, this fails — `invokeHook` is never called + with `onTaskCompleted`. The legacy case above passes both ways, which is why both are kept. + */ + it("should invoke onTaskCompleted when the complete lane is RENAMED", async () => { + const ir = lifecycleIr(RENAMED_VOCAB, "plugin-runner-lifecycle"); + mockTaskStore.getTaskWorkflowSelectionAsync = vi.fn(async () => ({ workflowId: "plugin-runner-lifecycle", stepIds: [] })); + mockTaskStore.getTaskWorkflowSelection = vi.fn(() => ({ workflowId: "plugin-runner-lifecycle", stepIds: [] })); + mockTaskStore.getWorkflowDefinition = vi.fn(async (id: string) => (id === "plugin-runner-lifecycle" ? { ir } : undefined)); + mockPluginLoader.invokeHook = vi.fn(); + await pluginRunner.init(); + + const movedHandler = mockTaskStore.on.mock.calls.find( + call => call[0] === "task:moved" + )?.[1]; + + const mockTask = { id: "FN-001", title: "Test Task" }; + if (movedHandler) { + movedHandler({ task: mockTask, from: RENAMED_VOCAB.review, to: RENAMED_VOCAB.complete }); + } + await flushMicrotasks(); + + expect(mockPluginLoader.invokeHook).toHaveBeenCalledWith("onTaskCompleted", mockTask); + }); + + it("should NOT invoke onTaskCompleted when a RENAMED board moves the card to a non-complete lane", async () => { + /* + Non-vacuous companion: without it, a guard that fired on EVERY move would satisfy the case above. + Same renamed board, same handler — only the destination lane changes. + */ + const ir = lifecycleIr(RENAMED_VOCAB, "plugin-runner-lifecycle"); + mockTaskStore.getTaskWorkflowSelectionAsync = vi.fn(async () => ({ workflowId: "plugin-runner-lifecycle", stepIds: [] })); + mockTaskStore.getTaskWorkflowSelection = vi.fn(() => ({ workflowId: "plugin-runner-lifecycle", stepIds: [] })); + mockTaskStore.getWorkflowDefinition = vi.fn(async (id: string) => (id === "plugin-runner-lifecycle" ? { ir } : undefined)); + mockPluginLoader.invokeHook = vi.fn(); + await pluginRunner.init(); + + const movedHandler = mockTaskStore.on.mock.calls.find( + call => call[0] === "task:moved" + )?.[1]; + + const mockTask = { id: "FN-001", title: "Test Task" }; + if (movedHandler) { + movedHandler({ task: mockTask, from: RENAMED_VOCAB.hold, to: RENAMED_VOCAB.wip }); + } + await flushMicrotasks(); + + expect(mockPluginLoader.invokeHook).not.toHaveBeenCalledWith("onTaskCompleted", mockTask); + }); + it("should NOT invoke onTaskCompleted when task moves elsewhere", async () => { mockPluginLoader.invokeHook = vi.fn(); await pluginRunner.init(); diff --git a/packages/engine/src/agent-reflection.ts b/packages/engine/src/agent-reflection.ts index 7ad3b90496..52dcfe465a 100644 --- a/packages/engine/src/agent-reflection.ts +++ b/packages/engine/src/agent-reflection.ts @@ -13,7 +13,7 @@ import type { TaskStore, } from "@fusion/core"; import { createLogger } from "./logger.js"; -import { resolveProjectDefaultModel } from "@fusion/core"; +import { resolveProjectDefaultModel, resolveWorkflowIrForTask, columnsWithFlag } from "@fusion/core"; import { createFnAgent, promptWithFallback } from "./pi.js"; import { resolveMcpServersForStore } from "./mcp-resolution.js"; import { createRunAuditor, generateSyntheticRunId, type EngineRunContext, type RunAuditor } from "./run-audit.js"; @@ -243,7 +243,7 @@ export class AgentReflectionService { return null; } - const outcome = this.classifyOutcome(task); + const outcome = await this.classifyOutcome(task); if (!outcome || outcome === "stuck") { await this.emitReflectionAudit(auditor, "reflection:skipped", agentId, trigger, { taskId, ...options }, { reason: "not-completed", @@ -519,7 +519,7 @@ export class AgentReflectionService { continue; } - const outcome = this.classifyOutcome(task); + const outcome = await this.classifyOutcome(task); if (!outcome) { continue; } @@ -700,7 +700,23 @@ export class AgentReflectionService { return ids; } - private classifyOutcome(task: Task): TaskOutcome["outcome"] | null { + /* + FNXC:WorkflowResolvedColumns 2026-07-30-13:10 (batch-engine tail): + "Completed" here is the COMPLETE and REVIEW roles, not the two ids. Keyed on the literals, a renamed + board returned `null` for every finished task, so BOTH callers treated it as nothing-to-reflect-on: + the per-task path recorded `reflection:skipped` with reason "not-completed" for work that had in fact + completed, and the sweep path silently `continue`d. Agent performance reflection therefore captured + nothing at all on a custom board. + + Async because the resolution is: the sync IR reader returns the DEFAULT workflow for every task in + production (see sync-workflow-ir-callsite-allowlist), so a sync guard here would read as converted and + still be wrong. Both call sites already sit inside async functions, so this adds no new seam. + + Unioned with the legacy pair — `resolveWorkflowIrForTask` degrades to the BUILT-IN IR rather than + throwing, and without the union a degraded board resolves a set excluding its own terminal lanes, + which reproduces the bug. + */ + private async classifyOutcome(task: Task): Promise { const normalizedStatus = task.status?.toLowerCase() ?? ""; const hasStuckSignal = normalizedStatus.includes("stuck") @@ -717,7 +733,16 @@ export class AgentReflectionService { return "failed"; } - if (task.column === "done" || task.column === "in-review") { + const completedColumns = new Set(["done", "in-review"]); + try { + const ir = await resolveWorkflowIrForTask(this.taskStore, task.id); + if (ir) { + for (const flag of ["complete", "mergeOrchestration", "mergeBlocker", "humanReview"] as const) { + for (const id of columnsWithFlag(ir, flag)) completedColumns.add(id); + } + } + } catch { /* degraded: legacy pair only */ } + if (completedColumns.has(task.column)) { return "completed"; } diff --git a/packages/engine/src/auto-recovery-handlers/branch-worktree.ts b/packages/engine/src/auto-recovery-handlers/branch-worktree.ts index 66bd5a34a4..71d97e96ad 100644 --- a/packages/engine/src/auto-recovery-handlers/branch-worktree.ts +++ b/packages/engine/src/auto-recovery-handlers/branch-worktree.ts @@ -2,6 +2,7 @@ import { exec } from "node:child_process"; import { existsSync } from "node:fs"; import { promisify } from "node:util"; import type { Task, TaskStore } from "@fusion/core"; +import { resolveWorkflowIrForTask, columnsWithFlag, resolveReboundTarget, TransitionRejectionError } from "@fusion/core"; import { classifyBootstrapMisbinding, inspectBranchConflict, @@ -122,15 +123,122 @@ export class BranchWorktreeAutoRecoveryHandler { private async requeueAfterRecovery(task: Task, failure: AutoRecoveryFailure, rationale: string, evidence: RecoveryEvidence): Promise { if (task.userPaused) return; - if (task.column === "in-progress") { + /* + FNXC:WorkflowResolvedColumns 2026-07-30-13:40 (batch-engine tail): + TWO defects here, and fixing only the counted one would have been half a fix. + + 1. The WIP test was the id `in-progress`, so on a renamed board the stale branch/baseCommitSha were + never cleared and the requeued card carried a dead branch back into execution. + 2. The requeue DESTINATION was the hardcoded `todo` — census-invisible, because the census scores + comparisons and this is a call argument. A board without a `todo` column was requeued into a lane + that does not exist. `resolveReboundTarget` is the shared helper for exactly this (KTD-10 ordering: + hold -> intake -> first column), and it is why the destination is resolved rather than guessed. + + Both fall back to the legacy ids: `resolveWorkflowIrForTask` degrades to the BUILT-IN IR rather than + throwing, so a degraded board behaves exactly as before. + */ + let wipColumns = new Set(["in-progress"]); + let reboundTarget = "todo"; + /* + FNXC:WorkflowResolvedColumns 2026-07-30-14:40 (#2797 review — coderabbit): + The catch no longer swallows. Falling back to the legacy ids stays — this is a RECOVERY path, and + refusing to act because a workflow could not be read would strand the very task the handler exists + to unstick — but the fallback is now RECORDED rather than silent, so "why did this land in `todo`" + is answerable after the fact. + + Note the fallback is rarely what routes a custom board here: `resolveWorkflowIrForTask` degrades to + the BUILT-IN IR instead of throwing, so an unreadable custom workflow resolves `todo` through the + resolver and never reaches this catch. That is why the real protection is the move-rejection guard + below, not this branch — this one only makes the rare hard failure visible. + */ + let laneResolutionError: string | undefined; + try { + const ir = await resolveWorkflowIrForTask(this.deps.taskStore, task.id); + if (ir) { + const resolvedWip = columnsWithFlag(ir, "countsTowardWip"); + if (resolvedWip.length > 0) wipColumns = new Set(resolvedWip); + reboundTarget = resolveReboundTarget(ir) ?? "todo"; + } + } catch (err) { + laneResolutionError = err instanceof Error ? err.message : String(err); + } + + + /* + FNXC:WorkflowResolvedColumns 2026-07-30-13:45 (#2797 review — greptile): + THE REQUEUE MUST NOT DIE ON A DESTINATION THE BOARD DOES NOT DECLARE. + + The review flagged the `catch` above retaining `reboundTarget = "todo"`. Tracing it, the exposure + is wider than that branch: `resolveWorkflowIrForTask` degrades to the BUILT-IN IR rather than + throwing, so a task whose custom workflow cannot be read resolves a target from the DEFAULT board + — also `todo` — without the catch ever running. Guarding only the throw path would have looked + like a fix and changed almost nothing. + + What actually bites is the move: `moveTaskInternal` REJECTS a destination the workflow does not + declare (`TransitionRejectionError: unknown-column`). Unhandled, that throws out of the recovery + handler whose whole job is to unstick the task — so the recovery became a second way for it to stay + stuck, with no audit row to explain it. + + Catching at the move covers every route to a wrong destination, resolved or guessed. A task left + parked WITH a record beats one parked by an exception nobody sees. + */ + /* + FNXC:WorkflowResolvedColumns 2026-07-30-17:45 (#2797 review — greptile P1 x2): + + ORDER: MOVE FIRST, THEN CLEAR THE BRANCH LINKAGE. The clear used to run BEFORE the move, so a + rejected move left the card in its wip lane with `branch`/`baseCommitSha` already erased — the + recovery destroyed the only pointers back to the work and then declined to requeue. Half-applied is + worse than not applied: nothing else can reconstruct the branch from the row afterwards. The clear + now happens only once the requeue has actually landed. + + REASON MUST NAME THE ACTUAL FAILURE. The catch labelled EVERY moveTask failure + `rebound-target-rejected` — capacity exhaustion, a guard rejection, a deleted task, a persistence + error — so the audit asserted a lane problem for causes that have nothing to do with lanes, and a + reader debugging a stuck card would chase the wrong thing. `TransitionRejectionError` carries a typed + `rejection.code`, so the unknown-column case is distinguishable exactly rather than by message match. + + Everything is still CAUGHT (an exception thrown out of the recovery handler is invisible), but the + row now says which failure it was. + */ + let moveFailure: { reason: string; code?: string } | undefined; + try { + await this.deps.taskStore.moveTask(task.id, reboundTarget, { + moveSource: "engine", + preserveResumeState: true, + preserveProgress: true, + preserveWorktree: false, + }); + } catch (err) { + const code = err instanceof TransitionRejectionError ? err.rejection.code : undefined; + moveFailure = { + reason: code === "unknown-column" ? "rebound-target-rejected" : "requeue-move-failed", + ...(code ? { code } : {}), + }; + } + + if (moveFailure) { + await this.deps.runAudit.database({ + type: "branch-worktree:auto-requeue-skipped", + target: task.id, + metadata: { + class: failure.class, + reason: moveFailure.reason, + ...(moveFailure.code ? { rejectionCode: moveFailure.code } : {}), + rationale, + ...(laneResolutionError ? { laneResolutionError } : {}), + reboundTarget, + column: task.column, + /* Branch linkage deliberately NOT cleared on this path — see the note above. */ + branchPreserved: true, + evidence, + }, + }); + return; + } + + if (wipColumns.has(task.column)) { await this.deps.taskStore.updateTask(task.id, { branch: null, baseCommitSha: null }); } - await this.deps.taskStore.moveTask(task.id, "todo", { - moveSource: "engine", - preserveResumeState: true, - preserveProgress: true, - preserveWorktree: false, - }); await this.deps.runAudit.database({ type: "branch-worktree:auto-requeue", target: task.id, @@ -138,6 +246,7 @@ export class BranchWorktreeAutoRecoveryHandler { class: failure.class, rationale, prevPausedReason: task.pausedReason ?? null, + ...(laneResolutionError ? { laneResolutionError } : {}), evidence, }, }); diff --git a/packages/engine/src/cli-agent/state-machine.ts b/packages/engine/src/cli-agent/state-machine.ts index da7247b96b..252178099a 100644 --- a/packages/engine/src/cli-agent/state-machine.ts +++ b/packages/engine/src/cli-agent/state-machine.ts @@ -290,6 +290,13 @@ export class CliSessionStateMachine { this.transition("busy"); } + /* + FNXC:CliAgentStateMachine 2026-07-30-12:20 DELIBERATE-LITERAL: + `done` here is a `CliMachineState`, NOT a lifecycle column — this machine tracks one CLI agent + process (`ready`/`busy`/`waitingOnInput`/`done`/`resuming`/`idle`) and never reads a board column. + The lifecycle-column census matches the bare string and counted it; converting it to a workflow role + would be nonsense, so it is marked rather than left to be "fixed" by a later sweep. + */ /** done → busy (follow-up). Alias for injectPrompt from the done state. */ followUp(): void { if (this.state !== "done") { @@ -365,6 +372,7 @@ export class CliSessionStateMachine { * Idle / output progress never reach here. */ signalDone(): void { + /* DELIBERATE-LITERAL — `CliMachineState`, not a board column. See followUp() above. */ if (this.state === "done") return; // idempotent if (this.state !== "busy" && this.state !== "waitingOnInput") { throw new InvalidCliTransitionError(this.state, "signalDone"); diff --git a/packages/engine/src/cli-agent/task-session.ts b/packages/engine/src/cli-agent/task-session.ts index c9c59b5099..7d142b2fdf 100644 --- a/packages/engine/src/cli-agent/task-session.ts +++ b/packages/engine/src/cli-agent/task-session.ts @@ -405,6 +405,8 @@ export class CliTaskSession { const machine = this.hub.getStateMachine(this.sessionId); if (machine) { try { + /* DELIBERATE-LITERAL — `CliMachineState` from the CLI agent state machine, not a lifecycle + column. See `state-machine.ts#followUp`. */ if (machine.getState() === "done") machine.followUp(); else if (machine.getState() === "ready" || machine.getState() === "resuming") { machine.injectPrompt(); diff --git a/packages/engine/src/merger-integration-worktree.ts b/packages/engine/src/merger-integration-worktree.ts index 6c77964e1b..e38b303870 100644 --- a/packages/engine/src/merger-integration-worktree.ts +++ b/packages/engine/src/merger-integration-worktree.ts @@ -3,6 +3,8 @@ import { exec, execFile } from "node:child_process"; import { promisify } from "node:util"; import { normalizeMergeIntegrationWorktreeMode, + resolveWorkflowIrForTask, + columnsWithFlag, } from "@fusion/core"; import type { MergeIntegrationWorktreeMode, @@ -10,6 +12,7 @@ import type { ProjectSettings, Task, TaskStore, + WorkflowIr, } from "@fusion/core"; import { activeSessionRegistry, @@ -334,11 +337,27 @@ export async function probeIntegrationWorktreeState( } } +/* +FNXC:WorkflowResolvedColumns 2026-07-30-13:10 (batch-engine tail): +"Someone else is still using this worktree" excludes tasks that have FINISHED. Keyed on the literal, a +renamed complete lane meant a shipped task still counted as a live user, so the integration worktree +could never be reused and the merge path took the slower rebuild every time. + +Resolved per CANDIDATE task (each may run its own workflow), one IR cache for the scan, and only for +the tasks that actually share the worktree path — the common case resolves nothing at all. +*/ async function findOtherWorktreeUser(store: TaskStore, worktreePath: string, excludeTaskId: string): Promise { const tasks = await store.listTasks({ slim: true, includeArchived: false } as never); + const irCache = new Map(); for (const task of tasks) { if (task.id === excludeTaskId) continue; - if (task.worktree === worktreePath && task.column !== "done") { + if (task.worktree !== worktreePath) continue; + const completeColumns = new Set(["done"]); + try { + const ir = await resolveWorkflowIrForTask(store, task.id, irCache); + if (ir) for (const id of columnsWithFlag(ir, "complete")) completeColumns.add(id); + } catch { /* degraded: legacy id only */ } + if (!completeColumns.has(task.column)) { return task.id; } } diff --git a/packages/engine/src/merger-orphan-rehome.ts b/packages/engine/src/merger-orphan-rehome.ts index b5ef223ffb..5d7a313b6e 100644 --- a/packages/engine/src/merger-orphan-rehome.ts +++ b/packages/engine/src/merger-orphan-rehome.ts @@ -25,6 +25,7 @@ import { execFile } from "node:child_process"; import { promisify } from "node:util"; import type { TaskStore } from "@fusion/core"; +import { resolveWorkflowIrForTask, columnsWithFlag } from "@fusion/core"; import type { RunAuditor } from "./run-audit.js"; import { advanceIntegrationBranchRef, @@ -91,7 +92,18 @@ export async function classifyOrphanOurAdvance( const sourceTask = await input.taskStore.getTask(sourceTaskId); if (!sourceTask) return { orphan: false, reason: "source-task-not-found" }; - if (sourceTask.column !== "done") return { orphan: false, reason: "source-task-not-done" }; + /* + FNXC:WorkflowResolvedColumns 2026-07-30-13:10 (batch-engine tail): + Orphan rehoming only applies once the SOURCE task has finished. Keyed on the literal, a renamed + complete lane made every source read as unfinished, so orphaned commits were never rehomed and simply + stayed stranded off the integration branch. + */ + const sourceCompleteColumns = new Set(["done"]); + try { + const sourceIr = await resolveWorkflowIrForTask(input.taskStore, sourceTaskId); + if (sourceIr) for (const id of columnsWithFlag(sourceIr, "complete")) sourceCompleteColumns.add(id); + } catch { /* degraded: legacy id only */ } + if (!sourceCompleteColumns.has(sourceTask.column)) return { orphan: false, reason: "source-task-not-done" }; const integrationRef = `refs/heads/${input.integrationBranch}`; if (await isAncestor(input.repoDir, input.commitSha, integrationRef)) { diff --git a/packages/engine/src/merger-scope-auto-widen.ts b/packages/engine/src/merger-scope-auto-widen.ts index 8bde97d9a1..1ec8c8571f 100644 --- a/packages/engine/src/merger-scope-auto-widen.ts +++ b/packages/engine/src/merger-scope-auto-widen.ts @@ -3,7 +3,8 @@ import { readFile, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { promisify } from "node:util"; -import type { Task, TaskStore } from "@fusion/core"; +import type { Task, TaskStore, WorkflowIr } from "@fusion/core"; +import { resolveWorkflowIrForTask, columnsWithFlag } from "@fusion/core"; import { toTaskToken } from "./merger.js"; @@ -118,11 +119,36 @@ export async function evaluateScopeAutoWiden(params: EvaluateScopeAutoWidenParam const allTasks = typeof store.listTasks === "function" ? await store.listTasks({ slim: true, includeArchived: false }) : []; + /* + FNXC:WorkflowResolvedColumns 2026-07-30-12:55 (batch-engine tail): + "Still active" is the COMPLETE and ARCHIVED roles, not the two ids. On a renamed board every FINISHED + card counted as an active other, so auto-widen refused to widen into files whose only other claimant + had already shipped — a merge blocked by a task that no longer exists in any meaningful sense. + + NOT the query-filter class: this query passes no `column`, so the predicate is the only lane gate here. + + Resolved per OTHER TASK (each may run its own workflow), one IR cache for the pass, unioned with the + legacy pair because `resolveWorkflowIrForTask` degrades to the BUILT-IN IR rather than throwing — + without the union a degraded board would treat every finished card as active, which is this bug. + */ + const irCache = new Map(); + const terminalByTaskId = new Map>(); + for (const other of allTasks) { + const columns = new Set(["done", "archived"]); + try { + const ir = await resolveWorkflowIrForTask(store as TaskStore, other.id, irCache); + if (ir) { + for (const flag of ["complete", "archived"] as const) { + for (const id of columnsWithFlag(ir, flag)) columns.add(id); + } + } + } catch { /* degraded: legacy pair only */ } + terminalByTaskId.set(other.id, columns); + } const activeOtherTasks = allTasks.filter((other) => ( other.id !== taskId && other.deletedAt == null && - other.column !== "done" && - other.column !== "archived" + terminalByTaskId.get(other.id)?.has(other.column) !== true )); for (const file of candidateFiles) { diff --git a/packages/engine/src/plugin-runner.ts b/packages/engine/src/plugin-runner.ts index f45d17d8ea..5b58aafb7e 100644 --- a/packages/engine/src/plugin-runner.ts +++ b/packages/engine/src/plugin-runner.ts @@ -43,6 +43,7 @@ import { evaluatePromptConditionDetailed, resolveEffectivePluginSettings, resolveWorkflowIrForTask, + columnsWithFlag, workflowExtensionRegistryId, } from "@fusion/core"; import { createLogger, executorLog } from "./logger.js"; @@ -1459,10 +1460,33 @@ export class PluginRunner { // Fire and forget - don't await void this.invokeHookSafe("onTaskMoved", task, from, to); - // If task completed, invoke onTaskCompleted hook - if (to === "done") { - void this.invokeHookSafe("onTaskCompleted", task); - } + /* + FNXC:WorkflowResolvedColumns 2026-07-30-12:30 (batch-engine tail): + "Completed" is the COMPLETE role, not the id `done`. Keyed on the literal, `onTaskCompleted` NEVER + fired on a board whose complete lane is renamed — every plugin that closes an issue, posts a + notification, or records a metric on completion silently stopped, with nothing logged. + + Resolved ASYNCHRONOUSLY inside the existing fire-and-forget seam rather than through the sync IR + reader: per `sync-workflow-ir-callsite-allowlist`, `resolveTaskWorkflowIrSync` returns the DEFAULT + workflow for every task in production, so a sync-resolved guard here would read as converted and + still be wrong for every custom board. This listener is already `void`-dispatched (the line above), + so awaiting inside it changes no ordering the caller can observe — the same shape + `NotificationService` uses for its `task:moved` handler. + + Unioned with the legacy id because `resolveWorkflowIrForTask` degrades to the BUILT-IN IR rather + than throwing; without it a degraded board resolves a complete set excluding its own complete lane + and the hook goes silent again. + */ + void (async () => { + const completeColumns = new Set(["done"]); + try { + const ir = await resolveWorkflowIrForTask(this.options.taskStore, task.id); + if (ir) for (const id of columnsWithFlag(ir, "complete")) completeColumns.add(id); + } catch { /* degraded: legacy id only */ } + if (completeColumns.has(to)) { + await this.invokeHookSafe("onTaskCompleted", task); + } + })(); }; /** diff --git a/packages/engine/src/run-audit.ts b/packages/engine/src/run-audit.ts index ad1ac304c9..082285bb48 100644 --- a/packages/engine/src/run-audit.ts +++ b/packages/engine/src/run-audit.ts @@ -807,6 +807,14 @@ export type DatabaseMutationType = | "message-delivery:retry-issued" | "message-delivery:park" | "branch-worktree:auto-requeue" + /* + FNXC:WorkflowResolvedColumns 2026-07-30-13:25 (#2797 review): + Emitted when the branch-worktree auto-requeue cannot resolve a rebound destination from the task's + own workflow. The requeue is SKIPPED rather than aimed at the legacy `todo`, because `moveTask` + rejects a column the board does not declare and the resulting throw left the task parked with no + record at all. Metadata stays ids/outcomes-only. + */ + | "branch-worktree:auto-requeue-skipped" | "branch-worktree:ai-session-spawned" | "branch-worktree:irreducible-pause" | "branch-worktree:foreign-branch-discarded" diff --git a/packages/engine/src/worktree-pool.ts b/packages/engine/src/worktree-pool.ts index ebf57d2eb0..1dfc8138a0 100644 --- a/packages/engine/src/worktree-pool.ts +++ b/packages/engine/src/worktree-pool.ts @@ -913,10 +913,36 @@ export async function scanIdleWorktrees( // Find worktree paths assigned to non-done tasks (active worktrees) const tasks = await store.listTasks({ slim: true, includeArchived: false, startupMemo: true }); const activeWorktrees = new Set(); + /* + FNXC:WorkflowResolvedColumns 2026-07-30-14:05 (batch-engine tail): + "Still holding its worktree" excludes tasks that have FINISHED. Keyed on the id, a renamed complete + lane kept every shipped task's worktree in the ACTIVE set, so this reclaim pass never returned it and + the board walked into worktree exhaustion — a stall whose cause is invisible from the symptom. + + NOT the query-filter class: this listTasks call passes no `column`. + + Resolved per TASK (each may run its own workflow) and ONLY for tasks that actually record a worktree, + with one IR cache for the pass. Unioned with the legacy id because `resolveWorkflowIrForTask` degrades + to the BUILT-IN IR rather than throwing — without the union a degraded board would hold every worktree + forever, which is this bug. + */ + const reclaimIrCache = new Map>>(); + const completeByTaskId = new Map>(); for (const task of tasks) { - if (task.worktree && task.column !== "done" && registeredWorktrees.has(resolve(task.worktree))) { + if (!task.worktree) continue; + const columns = new Set(["done"]); + try { + const ir = await resolveWorkflowIrForTask(store, task.id, reclaimIrCache); + if (ir) for (const id of columnsWithFlag(ir, "complete")) columns.add(id); + } catch { /* degraded: legacy id only */ } + completeByTaskId.set(task.id, columns); + } + const isUnfinished = (task: { id: string; column: string }) => + completeByTaskId.get(task.id)?.has(task.column) !== true; + for (const task of tasks) { + if (task.worktree && isUnfinished(task) && registeredWorktrees.has(resolve(task.worktree))) { activeWorktrees.add(resolve(task.worktree)); - } else if (task.worktree && task.column !== "done") { + } else if (task.worktree && isUnfinished(task)) { worktreePoolLog.debug(`Ignoring task ${task.id} worktree metadata because it is not a registered git worktree: ${task.worktree}`); } } diff --git a/scripts/lib/lifecycle-column-census-baseline.json b/scripts/lib/lifecycle-column-census-baseline.json index f88e4b0541..c8f9652fa5 100644 --- a/scripts/lib/lifecycle-column-census-baseline.json +++ b/scripts/lib/lifecycle-column-census-baseline.json @@ -45,11 +45,7 @@ "packages/dashboard/app/utils/taskTiming.ts": 2, "packages/dashboard/src/github-tracking-state.ts": 2, "packages/dashboard/src/server.ts": 2, - "packages/engine/src/agent-reflection.ts": 2, "packages/engine/src/auto-merge-finalization.ts": 2, - "packages/engine/src/cli-agent/state-machine.ts": 2, - "packages/engine/src/merger-scope-auto-widen.ts": 2, - "packages/engine/src/worktree-pool.ts": 2, "packages/core/src/eval-automation.ts": 1, "packages/core/src/eval-signal-collector.ts": 1, "packages/core/src/in-review-stall.ts": 1, @@ -87,22 +83,15 @@ "packages/dashboard/src/task-planner-chat-context.ts": 1, "packages/dashboard/src/task-planner-chat-metrics.ts": 1, "packages/dashboard/src/test/mockCoreEngine.ts": 1, - "packages/engine/src/auto-recovery-handlers/branch-worktree.ts": 1, "packages/engine/src/backlog-pressure-reporter.ts": 1, - "packages/engine/src/cli-agent/task-session.ts": 1, "packages/engine/src/ephemeral-worker-manager.ts": 1, - "packages/engine/src/merger-integration-worktree.ts": 1, - "packages/engine/src/merger-orphan-rehome.ts": 1, "packages/engine/src/merger.ts": 1, - "packages/engine/src/plugin-runner.ts": 1, "packages/engine/src/pr-comment-handler.ts": 1, "packages/engine/src/runtimes/in-process-runtime.ts": 1, "packages/engine/src/triage.ts": 1 }, "deliberateByFile": { "packages/dashboard/src/reliability-metrics.ts\u0000in-review": 4, - "packages/engine/src/scheduler.ts\u0000in-progress": 3, - "packages/engine/src/scheduler.ts\u0000in-review": 3, "packages/core/src/live-agent-count.ts\u0000in-progress": 2, "packages/core/src/live-agent-count.ts\u0000in-review": 2, "packages/core/src/store.ts\u0000in-review": 2, @@ -111,6 +100,9 @@ "packages/core/src/task-merge.ts\u0000in-review": 2, "packages/dashboard/app/components/TaskCard.tsx\u0000triage": 2, "packages/dashboard/app/components/TaskDetailModal.tsx\u0000triage": 2, + "packages/engine/src/cli-agent/state-machine.ts\u0000done": 2, + "packages/engine/src/scheduler.ts\u0000in-progress": 2, + "packages/engine/src/scheduler.ts\u0000in-review": 2, "packages/engine/src/usage-limit-detector.ts\u0000archived": 2, "packages/engine/src/usage-limit-detector.ts\u0000done": 2, "plugins/fusion-plugin-reports/src/store/report-store.ts\u0000archived": 2, @@ -148,13 +140,13 @@ "packages/dashboard/src/routes/register-task-workflow-routes.ts\u0000triage": 1, "packages/engine/src/agent-heartbeat.ts\u0000archived": 1, "packages/engine/src/agent-heartbeat.ts\u0000done": 1, + "packages/engine/src/cli-agent/task-session.ts\u0000done": 1, "packages/engine/src/hold-release.ts\u0000archived": 1, "packages/engine/src/hold-release.ts\u0000done": 1, "packages/engine/src/hold-release.ts\u0000in-review": 1, "packages/engine/src/project-engine.ts\u0000in-review": 1, "packages/engine/src/scheduler.ts\u0000archived": 1, "packages/engine/src/scheduler.ts\u0000done": 1, - "packages/engine/src/scheduler.ts\u0000todo": 1, "packages/engine/src/triage.ts\u0000triage": 1, "plugins/fusion-plugin-even-cards/src/cards/board-cards.ts\u0000archived": 1, "plugins/fusion-plugin-even-cards/src/cards/board-cards.ts\u0000done": 1,