From 7f2980972813eb153116ddb7f7af8aad346ebf8a Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sat, 1 Aug 2026 12:23:59 -0700 Subject: [PATCH] fix: exclude failed WIP from live capacity holders Failed parks left in the WIP column still counted as running agents and file-scope lease holders, so they consumed maxWorktrees/maxConcurrent and could serialize unrelated todos. Match review-lane semantics: status failed is never a live top-level holder. --- .../core/src/__tests__/live-agent-count.test.ts | 15 +++++++++++++++ packages/core/src/live-agent-count.ts | 11 +++++++++++ packages/engine/src/scheduler.ts | 17 ++++++++++++++++- 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/packages/core/src/__tests__/live-agent-count.test.ts b/packages/core/src/__tests__/live-agent-count.test.ts index 557dfef505..20293327be 100644 --- a/packages/core/src/__tests__/live-agent-count.test.ts +++ b/packages/core/src/__tests__/live-agent-count.test.ts @@ -30,6 +30,21 @@ describe("live agent count predicates", () => { expect(isRunningAgentTask(task({ column: "in-progress", columnCountsTowardWip: true, userPaused: true }))).toBe(false); }); + it("does not count failed WIP (or any failed row) as a live capacity holder", () => { + // Failed parks remain in WIP until rebound/operator action but must free maxWorktrees/maxConcurrent. + expect(isRunningAgentTask(task({ column: "in-progress", columnCountsTowardWip: true, status: "failed" }))).toBe(false); + expect(isRunningAgentTask(task({ column: "working", columnCountsTowardWip: true, status: "failed" }))).toBe(false); + expect(isRunningAgentTask(task({ + column: "in-progress", + columnCountsTowardWip: true, + status: "failed", + workflowStepResults: [{ workflowStepId: "code-review", workflowStepName: "Code Review", status: "pending" as const, startedAt: "2026-08-01T00:00:00.000Z" }], + }))).toBe(false); + expect(isRunningAgentTask(task({ column: "in-review", columnIsReviewOrMerge: true, status: "failed" }))).toBe(false); + // Still Waiting only for intake/hold — failed WIP is neither running nor waiting. + expect(isWaitingAgentTask(task({ column: "in-progress", columnCountsTowardWip: true, status: "failed" }))).toBe(false); + }); + it("counts only active review/merge statuses and excludes terminal columns", () => { for (const status of ["merging", "merging-pr", "merging-fix", "reviewing", "landing", "fixing"]) { expect(isRunningAgentTask(task({ column: "review", status, columnIsReviewOrMerge: true }))).toBe(true); diff --git a/packages/core/src/live-agent-count.ts b/packages/core/src/live-agent-count.ts index 5bf368a6e6..0fe07fb95e 100644 --- a/packages/core/src/live-agent-count.ts +++ b/packages/core/src/live-agent-count.ts @@ -171,6 +171,8 @@ function terminalKind(task: RunningAgentTaskShape): ColumnTerminalKind { * Active review/merge statuses count only in review/merge columns. * A live `pending` workflow-step lease (e.g. an in-flight Code Review gate) * counts in any non-terminal column, since gate sessions run with null status. + * A parked `failed` row never counts, even in WIP — failed WIP is operator- + * visible debris, not a live agent (must not consume maxWorktrees/maxConcurrent). */ /* FNXC:ConcurrencyIndicators 2026-07-30-03:40 DELIBERATE-LITERAL: the no-enrichment fallback only. @@ -180,6 +182,15 @@ guess, and a wrong guess under-reports the queued total. Fix at the CALLER by pa */ export function isRunningAgentTask(task: RunningAgentTaskShape): boolean { if (task.paused || task.userPaused || terminalKind(task) !== "none") return false; + /* + FNXC:ConcurrencyIndicators 2026-08-01-19:22: + Failed WIP parks (honest-blocked, graph parse failure, exhausted recovery) remain in the + WIP column until an operator or rebound moves them, but they are not live agents. Counting + them filled maxWorktrees/maxConcurrent and blocked ready work (e.g. FN-8704 sitting + in-progress/failed while todos waited on capacity). Match review-lane semantics, which + already exclude status:"failed" from active merge holders. + */ + if (task.status === "failed") return false; if (task.status === "planning") return true; // Review statuses are not globally live: a stale status in intake/WIP must not consume capacity. if (ACTIVE_IN_REVIEW_AGENT_STATUSES.has(String(task.status ?? ""))) { diff --git a/packages/engine/src/scheduler.ts b/packages/engine/src/scheduler.ts index 70f26c0f9b..91487aae05 100644 --- a/packages/engine/src/scheduler.ts +++ b/packages/engine/src/scheduler.ts @@ -563,6 +563,13 @@ export function shouldHoldActiveFileScopeLease( */ if (task.paused || task.userPaused) return false; /* + FNXC:OverlapScheduling 2026-08-01-19:22: + A failed park is not live work. Review already dropped status:"failed"; WIP must too, or a + stranded failed in-progress card keeps its file-scope lease and serializes unrelated todos + while consuming no real agent (same class as the capacity-holder fix in isRunningAgentTask). + */ + if (task.status === "failed") return false; + /* DELIBERATE-LITERAL — the documented default for an unconverted caller, reviewed 2026-07-30-20:40. Both scheduler call sites now pass the resolved answer, so these defaults are dead on the scheduler's own path; they exist for the self-healing / repair callers this predicate is shared @@ -2168,7 +2175,15 @@ export class Scheduler { */ const isReviewColumnTask = (task: Task): boolean => isReviewColumnRole(columnFlagsForTask(task), task.column); - const wipTaskIds = tasks.filter(isWipColumnTask).map((task) => task.id); + /* + FNXC:ConcurrencyIndicators 2026-08-01-19:22: + Failed WIP is not a live holder (isRunningAgentTask). Keep the WIP id list aligned so + diagnostic maxConcurrent holders and any WIP-only arithmetic do not re-count stranded failed + parks that the worktree ledger already excludes. + */ + const wipTaskIds = tasks + .filter((task) => isWipColumnTask(task) && task.status !== "failed") + .map((task) => task.id); /* FNXC:WorktreeCapacity 2026-08-01-04:38 (inactive retained-worktree capacity inversion): Worktree capacity is a LIVE-TASK budget, not a count of directories retained on disk. The