U11: resolve the worktree-acquisition requeue column by trait (2 sites, both branches) (#2496)

Based on `main`. First of my U11 conversion PRs — small, green, independently revertable. Both heartbeat worktree-acquisition requeue sites hardcoded `"todo"`. ## Why this is critical path, not a renamed-workflow nicety **U11 deletes the `todo` column from the builtin workflows.** After that, these two sites would requeue every acquisition-failed card into a column that no longer exists. ## Both sites converted together They are different branches of the same failure: - the **bounded-retry** requeue, and - the **retry-cap-exhausted** terminal park. Converting one and not the other would leave the rarer path — which fires only after three consecutive failures, so it's the one least likely to be noticed — still writing the literal. Target is the KTD-10 ordering via `resolveReboundTarget` (hold → intake → first column): the same helper `self-healing` and `mesh-lease-manager` already use for "requeue a recovered card", so the recovery paths cannot drift apart. ## What is deliberately untouched `preserveStatus: true` on the exhausted path. It exists because reopen-to-todo semantics would otherwise wipe the `status: "failed"` written immediately before (FN-7721) — changing the column must not disturb that flag. A test asserts the full options object, not just the column. Fail-soft to the legacy id: a requeue must not be abandoned because a workflow lookup failed, or the card is left holding a worktree it could not acquire. Covered by a regression-floor test. ## Verification - **Mutation-verified:** restoring the literal fails 2 of the 3 new tests - 7 tests green (3 new + the 4 pre-existing worktree tests, unchanged) - tsc clean, lint clean, merge gate green (299 + 10 + 71) ## Measured progress **2 of the 74** code-level `"todo"` sites in my unit (engine recovery/scheduling core) are now trait-resolved. Remaining in-unit: `self-healing` 48, `scheduler` 15, `triage` 8, `replan-target` 1. `stuck-task-detector` needs **no work** — all 4 of its occurrences are comments, not code. No changeset: `@fusion/engine` is private. 🤖 Generated with [Claude Code](https://claude.com/claude-code)  ## Summary by CodeRabbit * **Bug Fixes** * Tasks now return to the workflow’s configured hold column when heartbeat worktree acquisition fails, including workflows that use a renamed hold column. * Retry and retry-limit handling now preserves task progress and, when applicable, status. * Added a safe fallback to the default “todo” column when workflow details cannot be resolved.
2026-07-28 11:36:13 -07:00
parent 8492278fdd
commit 8aba310d78
2 changed files with 184 additions and 2 deletions
--- a/packages/engine/src/tests/agent-heartbeat-worktree-renamed-hold.test.ts
+++ b/packages/engine/src/tests/agent-heartbeat-worktree-renamed-hold.test.ts
@@ -0,0 +1,158 @@
+/*
+FNXC:WorkflowLifecycleColumns 2026-07-28-09:10 (U11 conversion — engine recovery core):
+
+When worktree acquisition fails, the heartbeat requeues the card to the BACKLOG so
+another cycle can retry it. Both requeue sites hardcoded `"todo"`, so for a
+workflow whose hold column is named anything else the card was shoved into a
+column that workflow does not declare.
+
+This is the rebound-target shape already converted in mesh-lease-manager and
+`recoverStrandedCompletedTodoTasks`: the target is the KTD-10 ordering
+`resolveReboundTarget` (hold -> intake -> first column), not the literal `todo`.
+
+It matters more than usual here because U11 DELETES the `todo` column from the
+builtin workflows. After that these two sites would requeue every
+acquisition-failed card into a column that no longer exists.
+
+Both sites are covered, because they are different branches of the same failure:
+the bounded-retry requeue and the retry-cap-exhausted terminal park. Converting
+one and not the other would leave the rarer path — the one that fires only after
+three consecutive failures — still writing the literal.
+
+Written against the literal implementation and observed FAILING first.
+*/
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
+import type { Agent, AgentHeartbeatRun, WorkflowIr } from "@fusion/core";
+import { HeartbeatMonitor } from "../agent-heartbeat.js";
+import * as worktreeAcquisition from "../worktree-acquisition.js";
+import * as piModule from "../pi.js";
+
+const WF = "custom:wf";
+
+/** A workflow whose hold column is `drafting` — it declares NO `todo` column. */
+function renamedIr(): WorkflowIr {
+  return {
+    version: "v2",
+    id: WF,
+    nodes: [],
+    edges: [],
+    columns: [
+      { id: "inbox", name: "inbox", traits: [{ trait: "intake" }] },
+      { id: "drafting", name: "drafting", traits: [{ trait: "hold", config: { release: "capacity" } }] },
+      { id: "building", name: "building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] },
+      { id: "shipped", name: "shipped", traits: [{ trait: "complete" }] },
+    ],
+  } as unknown as WorkflowIr;
+}
+
+describe("heartbeat worktree-acquisition requeue under a renamed hold column", () => {
+  let store: any;
+  let taskStore: any;
+  const agent: Agent = {
+    id: "a1", name: "A", role: "executor", state: "active", taskId: "FN-1",
+    createdAt: "", updatedAt: "", metadata: {},
+  } as any;
+
+  beforeEach(() => {
+    vi.restoreAllMocks();
+    vi.spyOn(piModule, "createFnAgent").mockResolvedValue({ session: { prompt: vi.fn(), dispose: vi.fn() } } as any);
+
+    const run: AgentHeartbeatRun = {
+      id: "r1", agentId: "a1", status: "active", startedAt: new Date().toISOString(), endedAt: null,
+    } as any;
+    store = {
+      startHeartbeatRun: vi.fn().mockResolvedValue(run),
+      saveRun: vi.fn(),
+      getRunDetail: vi.fn().mockResolvedValue(run),
+      getAgent: vi.fn().mockResolvedValue(agent),
+      updateAgentState: vi.fn(),
+      updateAgent: vi.fn(),
+      endHeartbeatRun: vi.fn(),
+      assignTask: vi.fn(),
+      getBudgetStatus: vi.fn().mockResolvedValue({ isOverBudget: false, isOverThreshold: false, usagePercent: 0 }),
+      getCachedAgent: vi.fn().mockReturnValue(null),
+      getLastBlockedState: vi.fn().mockResolvedValue(null),
+      setLastBlockedState: vi.fn(),
+      clearLastBlockedState: vi.fn(),
+      appendRunLog: vi.fn(),
+      getAgentsByReportsTo: vi.fn().mockResolvedValue([]),
+      recordHeartbeat: vi.fn(),
+    };
+    const selection = { workflowId: WF, stepIds: [] };
+    taskStore = {
+      getSettings: vi.fn().mockResolvedValue({}),
+      getTask: vi.fn().mockResolvedValue({
+        id: "FN-1", title: "t", description: "d", column: "drafting", dependencies: [], steps: [], log: [],
+      }),
+      moveTask: vi.fn(),
+      updateTask: vi.fn(),
+      logEntry: vi.fn(),
+      appendAgentLog: vi.fn(),
+      listTasks: vi.fn().mockResolvedValue([]),
+      selectNextTaskForAgent: vi.fn().mockResolvedValue(null),
+      getTaskWorkflowSelection: vi.fn(() => selection),
+      getTaskWorkflowSelectionAsync: vi.fn(async () => selection),
+      getWorkflowDefinition: vi.fn(async () => ({ ir: renamedIr() })),
+    };
+  });
+
+  afterEach(() => {
+    vi.restoreAllMocks();
+  });
+
+  it("requeues to the workflow's HOLD column, not the literal todo", async () => {
+    vi.spyOn(worktreeAcquisition, "acquireTaskWorktree").mockRejectedValueOnce(new Error("nope"));
+    const monitor = new HeartbeatMonitor({ store, taskStore, rootDir: "/repo" });
+
+    await monitor.executeHeartbeat({ agentId: "a1", source: "on_demand" });
+
+    expect(taskStore.moveTask).toHaveBeenCalledWith("FN-1", "drafting", { preserveProgress: true });
+    expect(taskStore.moveTask).not.toHaveBeenCalledWith("FN-1", "todo", expect.anything());
+  });
+
+  it("parks to the HOLD column on the retry-cap-exhausted path too", async () => {
+    /*
+    The rarer branch, which only fires after three consecutive failures. It keeps
+    `preserveStatus: true` so the `status: "failed"` written just before is not
+    wiped by reopen-to-todo semantics (FN-7721) — converting the column must not
+    disturb that flag.
+    */
+    vi.spyOn(worktreeAcquisition, "acquireTaskWorktree").mockRejectedValue(new Error("branch exists"));
+    const monitor = new HeartbeatMonitor({ store, taskStore, rootDir: "/repo" });
+
+    let recoveryRetryCount: number | null | undefined;
+    taskStore.updateTask.mockImplementation((_id: string, patch: Record<string, unknown>) => {
+      if ("recoveryRetryCount" in patch) recoveryRetryCount = patch.recoveryRetryCount as number | null;
+      return Promise.resolve();
+    });
+
+    for (let cycle = 0; cycle < 3; cycle++) {
+      taskStore.getTask.mockResolvedValue({
+        id: "FN-1", title: "t", description: "d", column: "drafting",
+        dependencies: [], steps: [], log: [], recoveryRetryCount,
+      });
+      await monitor.executeHeartbeat({ agentId: "a1", source: "on_demand" });
+    }
+
+    expect(taskStore.moveTask).toHaveBeenCalledWith("FN-1", "drafting", {
+      preserveProgress: true,
+      preserveStatus: true,
+    });
+    expect(taskStore.moveTask).not.toHaveBeenCalledWith("FN-1", "todo", expect.anything());
+  });
+
+  it("still requeues to todo when the workflow cannot be resolved (regression floor)", async () => {
+    /* Conservative fallback: an unresolvable workflow must behave exactly as it
+       did before this conversion rather than guessing a column. */
+    taskStore.getWorkflowDefinition = vi.fn(async () => null);
+    taskStore.getTask.mockResolvedValue({
+      id: "FN-1", title: "t", description: "d", column: "todo", dependencies: [], steps: [], log: [],
+    });
+    vi.spyOn(worktreeAcquisition, "acquireTaskWorktree").mockRejectedValueOnce(new Error("nope"));
+    const monitor = new HeartbeatMonitor({ store, taskStore, rootDir: "/repo" });
+
+    await monitor.executeHeartbeat({ agentId: "a1", source: "on_demand" });
+
+    expect(taskStore.moveTask).toHaveBeenCalledWith("FN-1", "todo", { preserveProgress: true });
+  });
+});
--- a/packages/engine/src/agent-heartbeat.ts
+++ b/packages/engine/src/agent-heartbeat.ts
@@ -34,6 +34,8 @@ import {
  formatAssignedTasksWakeDeltaSection,
  resolveEffectiveSettingsById,
  resolveEffectivePlannerHeartbeatPatrolEnabled,
+  resolveReboundTarget,
+  resolveWorkflowIrForTask,
 } from "@fusion/core";
 import type { ToolDefinition } from "@earendil-works/pi-coding-agent";
 import { Type, type Static } from "@earendil-works/pi-ai";
@@ -97,6 +99,28 @@ import { trimPromptMd, trimTaskDescription, trimTriggeringComments } from "./hea
 import { detectDeicticReference, extractAntecedentCandidates, renderAmbiguityPromptBlock, scoreReferentConfidence } from "./room-ambiguity.js";
 import { countActiveAgentMembers, decideRoomCoordination, detectTaskFilingIntent, renderRoomCoordinationPromptBlock } from "./room-coordination.js";
 import { evaluateParkedAgentTaskLink, isParkedTaskColumn, type AgentTaskLinkExecutionProof } from "./task-agent-sync.js";
+
+/*
+FNXC:WorkflowLifecycleColumns 2026-07-28-09:25 (U11 conversion):
+Where a worktree-acquisition failure requeues the card. KTD-10 ordering via
+`resolveReboundTarget` (hold -> intake -> first column) — the same helper
+self-healing and mesh-lease-manager use for "requeue a recovered card", so the
+recovery paths cannot drift apart.
+
+This matters beyond renamed workflows: U11 DELETES the `todo` column from the
+builtin workflows, after which the old literal would requeue every
+acquisition-failed card into a column that no longer exists.
+
+Fail-soft to the legacy id: a requeue must not be abandoned because a workflow
+lookup failed, or the card is left holding a worktree it could not acquire.
+*/
+async function resolveHeartbeatReboundColumn(taskStore: TaskStore, taskId: string): Promise<string> {
+  try {
+    return resolveReboundTarget(await resolveWorkflowIrForTask(taskStore, taskId)) ?? "todo";
+  } catch {
+    return "todo";
+  }
+}
 import { classifyReportHealth } from "./reports-health.js";
 import { accumulateSessionTokenUsage, captureSessionTokenBaseline } from "./session-token-usage.js";

@@ -2848,11 +2872,11 @@ export class HeartbeatMonitor {
                 * reassigned and retried from scratch, defeating the terminal-
                 * failure intent of this fix (FN-7721).
                 */
-                await taskStore.moveTask(taskDetail.id, "todo", { preserveProgress: true, preserveStatus: true });
+                await taskStore.moveTask(taskDetail.id, await resolveHeartbeatReboundColumn(taskStore, taskDetail.id), { preserveProgress: true, preserveStatus: true });
                this.onTaskAcquisitionExhausted?.(taskDetail.id, exhaustionMessage);
              } else {
                await taskStore.updateTask(taskDetail.id, { recoveryRetryCount: attemptsSoFar });
-                await taskStore.moveTask(taskDetail.id, "todo", { preserveProgress: true });
+                await taskStore.moveTask(taskDetail.id, await resolveHeartbeatReboundColumn(taskStore, taskDetail.id), { preserveProgress: true });
              }
            }
            await this.completeRun(agentId, run.id, {