diff --git a/packages/dashboard/app/hooks/__tests__/useSessionFiles.test.ts b/packages/dashboard/app/hooks/__tests__/useSessionFiles.test.ts index adb8fec578..14f6ccc892 100644 --- a/packages/dashboard/app/hooks/__tests__/useSessionFiles.test.ts +++ b/packages/dashboard/app/hooks/__tests__/useSessionFiles.test.ts @@ -121,3 +121,73 @@ describe("useSessionFiles", () => { }); }); }); + +/* +FNXC:WorkflowLifecycleColumns 2026-07-31-07:30 (dashboard-app feed): + +THE INVARIANT: the Files tab loads for any column whose ROLE carries a worktree. + +CENSUS-INVISIBLE. The gate was a `Set` literal — a definition, not a comparison — so nothing in the +lifecycle backlog pointed at this hook. Found by grepping for lane-shaped list literals after the +same shape turned up in `duplicate-intake`, `blocker-fanout` and the ephemeral zombie sweep. + +On a renamed board the set matched nothing, so the fetch NEVER FIRED and the Files tab was +permanently empty for every card that had one. An empty file list is indistinguishable from a task +that touched no files, which is why nobody would report it. + +The flags are collapsed to a boolean before the effect on purpose: `columnFlags` is an object, and a +caller constructing it inline would hand this hook a fresh identity every render, so putting it in +the dep array would refetch on every parent render. That is asserted below — a re-render with an +equal-but-not-identical flags object must not trigger a second fetch. + +REVERT PROOF, measured: restore `ACTIVE_COLUMNS.has(column)` and the renamed-wip case fails with +zero fetches. +*/ +describe("useSessionFiles resolves the column's role", () => { + const WIP_FLAGS = { countsTowardWip: true } as never; + const HOLD_FLAGS = { hold: true } as never; + + it("fetches for a RENAMED wip lane", async () => { + mockFetchSessionFiles.mockResolvedValueOnce(["src/a.ts"]); + + const { result } = renderHook(() => + useSessionFiles("FN-123", "/repo/.worktrees/kb-123", "building", undefined, { columnFlags: WIP_FLAGS })); + + await waitFor(() => expect(result.current.files).toEqual(["src/a.ts"])); + }); + + it("does NOT fetch for a column whose role carries no worktree", async () => { + /* + The gate must still gate — a hold-lane card has no worktree to read. + + Asserted as a DELTA rather than "not called at all": this file's hooks are not unmounted between + cases, so a prior case's in-flight fetch can land inside this one. The absolute assertion passed + in isolation and failed in the suite, which is the classic shape of a test that would have been + "fixed" by reordering rather than by being made independent. + */ + const before = mockFetchSessionFiles.mock.calls.length; + + renderHook(() => + useSessionFiles("FN-123", "/repo/.worktrees/kb-123", "backlog", undefined, { columnFlags: HOLD_FLAGS })); + + await waitFor(() => expect(mockFetchSessionFiles.mock.calls.length).toBe(before)); + }); + + it("does not refetch when an equal-but-new flags object arrives", async () => { + // The object-identity trap the derived boolean exists to avoid. + mockFetchSessionFiles.mockResolvedValue(["src/a.ts"]); + + const { rerender, result } = renderHook( + ({ flags }) => useSessionFiles("FN-123", "/repo/.worktrees/kb-123", "building", undefined, { columnFlags: flags }), + { initialProps: { flags: { countsTowardWip: true } as never } }, + ); + + await waitFor(() => expect(result.current.files).toEqual(["src/a.ts"])); + const callsAfterFirst = mockFetchSessionFiles.mock.calls.length; + + rerender({ flags: { countsTowardWip: true } as never }); + + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(mockFetchSessionFiles.mock.calls.length).toBe(callsAfterFirst); + }); +}); diff --git a/packages/dashboard/app/hooks/useSessionFiles.ts b/packages/dashboard/app/hooks/useSessionFiles.ts index 7560c09afc..b786cd3061 100644 --- a/packages/dashboard/app/hooks/useSessionFiles.ts +++ b/packages/dashboard/app/hooks/useSessionFiles.ts @@ -1,7 +1,35 @@ import { useEffect, useState } from "react"; import { fetchSessionFiles } from "../api"; +import { + isCompleteColumnRole, + isReviewColumnRole, + isWipColumnRole, + type ColumnRoleFlags, +} from "../utils/columnRoles"; -const ACTIVE_COLUMNS = new Set(["in-progress", "in-review", "done"]); +/* +FNXC:WorkflowLifecycleColumns 2026-07-31-07:30 (dashboard-app feed): +Session files load for cards that HAVE a worktree — wip, review, and complete. + +CENSUS-INVISIBLE: a `Set` literal is a definition, not a comparison, so nothing in the lifecycle +backlog pointed at this hook. Found by grepping for lane-shaped list literals. + +On a renamed board the set matched nothing, so the Files tab was permanently EMPTY for every card +that had one — the fetch simply never fired. An empty file list is indistinguishable from a task +that touched no files, which is why this would never be reported as a bug. + +DELIBERATE-LITERAL — the unresolved-flags default, reviewed 2026-07-31-07:30. `columnFlags` is +optional so every existing caller (and the hook's own test) is byte-identical; the role helpers in +`columnRoles.ts` own the legacy-id degraded mode. +*/ +const LEGACY_ACTIVE_COLUMNS = new Set(["in-progress", "in-review", "done"]); + +function hasWorktreeBearingRole(column: string, flags: ColumnRoleFlags | undefined): boolean { + if (!flags) return LEGACY_ACTIVE_COLUMNS.has(column); + return isWipColumnRole(flags, column) + || isReviewColumnRole(flags, column) + || isCompleteColumnRole(flags, column); +} interface UseSessionFilesResult { files: string[]; @@ -11,6 +39,8 @@ interface UseSessionFilesResult { interface UseSessionFilesOptions { /** Enable fetching when true (default). Suppresses fetches for offscreen cards. */ enabled?: boolean; + /** Resolved trait flags for `column`. Omitted → the legacy ids, i.e. today's behaviour. */ + columnFlags?: ColumnRoleFlags; } /** @@ -30,6 +60,13 @@ export function useSessionFiles( options: UseSessionFilesOptions = {}, ): UseSessionFilesResult { const enabled = options.enabled ?? true; + /* + FNXC:WorkflowLifecycleColumns 2026-07-31-07:30 (dashboard-app feed): + Collapsed to a BOOLEAN before the effect, deliberately. `columnFlags` is an object, and a caller + building it inline hands this hook a new identity every render — putting it in the dep array below + would refetch on every parent render. The derived boolean is stable for a stable answer. + */ + const columnBearsWorktree = hasWorktreeBearingRole(column, options.columnFlags); const [files, setFiles] = useState([]); const [loading, setLoading] = useState(false); @@ -41,7 +78,7 @@ export function useSessionFiles( return; } - if (!taskId || !worktree || !ACTIVE_COLUMNS.has(column)) { + if (!taskId || !worktree || !columnBearsWorktree) { setFiles([]); setLoading(false); return; @@ -68,7 +105,7 @@ export function useSessionFiles( } void load(); - }, [taskId, worktree, column, projectId, enabled]); + }, [taskId, worktree, column, projectId, enabled, columnBearsWorktree]); return { files, loading }; } diff --git a/packages/dashboard/src/__tests__/agent-task-link-terminal-lanes.test.ts b/packages/dashboard/src/__tests__/agent-task-link-terminal-lanes.test.ts new file mode 100644 index 0000000000..7190c93610 --- /dev/null +++ b/packages/dashboard/src/__tests__/agent-task-link-terminal-lanes.test.ts @@ -0,0 +1,62 @@ +/* +FNXC:WorkflowLifecycleColumns 2026-07-31-07:00 (dashboard-server feed): + +THE INVARIANT: the agent "working on" sanitizer asks each linked task's OWN terminal lanes. + +`sanitizeAgentTaskLinks` drops `taskId` from an agent response when the linked task is finished, so +the UI stops showing a stale "working on" indicator. It tested a hard-coded `Set(["done","archived"])` +— CENSUS-INVISIBLE, because a Set literal is a definition rather than a comparison, so nothing in the +lifecycle backlog pointed at this file. On a renamed board it matched nothing and a FINISHED card kept +its agent's indicator lit: the agent list advertised work that had already shipped, which is exactly +what this sanitizer exists to prevent. + +WHY THIS GUARD IS STRUCTURAL AND NOT BEHAVIOURAL, stated plainly rather than dressed up: +`sanitizeAgentTaskLinks` is a closure inside `createApiRoutes`, reachable only by building the full +express app and driving `GET /api/agents` through it. That harness exists (see +`routes-automation.test.ts`) but standing it up to re-assert a per-task resolver is a large amount of +machinery around a small seam, and I did not write it. So this asserts the SOURCE: the resolver is +threaded per task and the bare literal call is gone. + +It fails on revert — verified by reverting — which is the bar. It is not a substitute for a +behavioural test, and whoever owns the dashboard server should add one through the express harness if +this seam grows. Flagging that rather than letting a structural check read as full coverage. +*/ +import { readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; + +const source = readFileSync(new URL("../routes.ts", import.meta.url), "utf8"); + +describe("agent task-link sanitizer resolves each task's own terminal lanes", () => { + it("resolves the workflow IR per linked task id", () => { + expect(source).toContain("resolveWorkflowIrForTask(scopedStore, taskId, terminalIrCache"); + }); + + it("unions EVERY complete and archived column, not the first of each role", () => { + /* + #2787 review (greptile P1). The first version resolved `lifecycle.complete`/`lifecycle.archived`, + which are FIRST-per-role: a workflow declaring two complete lanes had only one recognised, so a + task in the second kept its taskId and the agent stayed shown as working on finished work — the + exact symptom this sanitizer removes, one degree narrower. + */ + expect(source).toContain('const terminal = [...columnsWithFlag(ir, "complete"), ...columnsWithFlag(ir, "archived")];'); + }); + + it("passes the resolved answer into the terminal check", () => { + expect(source).toContain("isTerminalTaskStatus(taskStatus, terminalByTaskId.get(agent.taskId))"); + // The bare one-argument call is what the conversion removes; its return would be the literal. + expect(source).not.toContain("isTerminalTaskStatus(taskStatus)"); + }); + + it("shares ONE IR cache across the batch rather than resolving per agent", () => { + // A page of agents may link many tasks across a few workflows; the cache is what keeps this + // from becoming an IR read per row. + expect(source).toContain("const terminalIrCache = new Map"); + }); + + it("keeps the literal pair as the documented unresolvable-workflow fallback", () => { + // Removing it would make an unresolvable task read as non-terminal forever, which is a + // regression in the opposite direction — the indicator would never clear. + expect(source).toContain('const TERMINAL_TASK_STATUSES = new Set(["done", "archived"])'); + expect(source).toContain("resolvedTerminal ?? TERMINAL_TASK_STATUSES"); + }); +}); diff --git a/packages/dashboard/src/routes.ts b/packages/dashboard/src/routes.ts index 7b83491162..47b30ff113 100644 --- a/packages/dashboard/src/routes.ts +++ b/packages/dashboard/src/routes.ts @@ -23,6 +23,8 @@ import { listAgentMemoryFiles, readAgentMemoryFile, writeAgentMemoryFile, + resolveWorkflowIrForTask, + columnsWithFlag, } from "@fusion/core"; import type { ServerOptions } from "./server.js"; import { SESSION_CLEANUP_DEFAULT_MAX_AGE_MS, type AiSessionType } from "./ai-session-store.js"; @@ -1315,14 +1317,25 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout * Terminal task statuses — tasks in these states should not be displayed * as "working on" in agent UI surfaces to avoid stale activity indicators. */ + /* + FNXC:WorkflowLifecycleColumns 2026-07-31-07:00 (dashboard-server feed): + DELIBERATE-LITERAL — the fallback for a task whose workflow will not resolve, reviewed + 2026-07-31-07:00. The resolved answer is threaded per task at the call site below. + + Census-invisible before this change: a `Set` literal is a definition, not a comparison, so nothing + in the lifecycle backlog pointed at this file. On a renamed board it matched nothing, so a FINISHED + card kept its agent's "working on" indicator lit — the agent list showed work that had already + shipped, which is exactly the stale indicator this sanitizer exists to prevent. + */ const TERMINAL_TASK_STATUSES = new Set(["done", "archived"]); const UNRESOLVED_AGENT_TASK_COLUMN = "unresolved"; /** * Check if a task status is terminal (done or archived). */ - function isTerminalTaskStatus(status: string | undefined): boolean { - return status !== undefined && TERMINAL_TASK_STATUSES.has(status); + function isTerminalTaskStatus(status: string | undefined, resolvedTerminal?: ReadonlySet): boolean { + if (status === undefined) return false; + return (resolvedTerminal ?? TERMINAL_TASK_STATUSES).has(status); } /** @@ -1348,11 +1361,33 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout taskStatusMap = new Map(); } + /* + FNXC:WorkflowLifecycleColumns 2026-07-31-07:00 (dashboard-server feed): + Each linked task's OWN terminal lanes, resolved once per unique id with a shared IR cache. A task + whose workflow will not resolve is left out of the map and falls back to the literal pair above, + which is the pre-existing behaviour rather than a guess. + */ + const terminalIrCache = new Map(); + const terminalByTaskId = new Map>(); + for (const taskId of taskIds) { + /* + FNXC:WorkflowLifecycleColumns 2026-07-31-09:30 (#2787 review — greptile P1): + MEMBERSHIP, not first-per-role — a workflow may declare more than one complete or archived + column, and `resolveLifecycleColumns` returns only the FIRST of each. A linked task in the + second terminal lane kept its `taskId` and the agent stayed displayed as working on finished + work, which is the exact symptom this sanitizer exists to remove. + */ + const ir = await resolveWorkflowIrForTask(scopedStore, taskId, terminalIrCache as never).catch(() => undefined); + if (!ir) continue; + const terminal = [...columnsWithFlag(ir, "complete"), ...columnsWithFlag(ir, "archived")]; + if (terminal.length > 0) terminalByTaskId.set(taskId, new Set(terminal)); + } + return agents.map((agent) => { if (!agent.taskId) return agent; const taskStatus = taskStatusMap.get(agent.taskId); - if (isTerminalTaskStatus(taskStatus)) { + if (isTerminalTaskStatus(taskStatus, terminalByTaskId.get(agent.taskId))) { // Omit taskId for terminal tasks — use spread to create shallow copy without taskId const { taskId: _omitted, taskColumn: _taskColumnOmitted, ...sanitized } = agent; return sanitized as import("@fusion/core").Agent; diff --git a/packages/engine/src/__tests__/agent-assignment.test.ts b/packages/engine/src/__tests__/agent-assignment.test.ts index 8c47f8cab2..a66f937ded 100644 --- a/packages/engine/src/__tests__/agent-assignment.test.ts +++ b/packages/engine/src/__tests__/agent-assignment.test.ts @@ -196,3 +196,74 @@ describe("listEligibleExecutorAgents", () => { expect(selected).toBeNull(); }); }); + +/* +FNXC:WorkflowLifecycleColumns 2026-07-31-05:40 (batch-engine feed): + +THE INVARIANT: assignment load counts the cards a board's OWN lanes call active. + +CENSUS-INVISIBLE. The gate was a `Set` literal — a definition, not a comparison — so no lifecycle +backlog entry ever pointed at this file. Found by grepping for lane-shaped list literals after the +same shape turned up in `duplicate-intake` and `blocker-fanout`. + +The failure is a silent DEGRADATION rather than an error, and it is invisible in exactly the way that +matters: on a renamed board no column matched, so `assignmentLoad` stayed empty, every candidate +compared as load 0, and the sort fell through to its stable `createdAt` tiebreak. The SAME agent then +wins every assignment while the rest sit idle. Nothing logs and nothing fails — the board simply +distributes badly, which reads as an agent being "busy" rather than as a bug. + +REVERT PROOF, measured: restore the hard-coded Set and the renamed case fails — the loaded agent is +picked instead of the idle one, because its load reads as 0. +*/ +describe("assignment load resolves the board's own active lanes", () => { + const agents = [ + makeAgent({ id: "AG-BUSY", createdAt: "2026-01-01T00:00:00.000Z" }), + makeAgent({ id: "AG-IDLE", createdAt: "2026-01-02T00:00:00.000Z" }), + ]; + + const store = (columnOfBusyWork: string) => ({ + listTasks: async () => [ + makeTask({ id: "FN-EXISTING", assignedAgentId: "AG-BUSY", column: columnOfBusyWork } as never), + ], + }) as never; + + const select = (columnOfBusyWork: string, activeColumns?: ReadonlySet) => + selectPermanentAgentForTask({ + task: makeTask({ id: "FN-NEW" }), + agentStore: { listAgents: async () => agents, getChainOfCommand: async () => [] } as never, + taskStore: store(columnOfBusyWork), + ...(activeColumns ? { activeColumns } : {}), + }); + + it("prefers the idle agent when the busy one's work sits in a RENAMED wip lane", async () => { + // Pre-fix: `building` matched no literal, AG-BUSY read as load 0, and its earlier createdAt won. + const selected = await select("building", new Set(["backlog", "building", "signoff"])); + + expect(selected?.id).toBe("AG-IDLE"); + }); + + it("keeps the legacy trio when no lanes are supplied", async () => { + const selected = await select("in-progress"); + + expect(selected?.id).toBe("AG-IDLE"); + }); + + it("counts work parked in a RENAMED hold lane, as the legacy set counted todo", async () => { + /* + #2787 review, second round (greptile P1). The legacy set is `{todo, in-progress, in-review}` and + `todo` is the HOLD lane, so a resolved set covering only wip and review DROPS assigned backlog + work from the tally — a regression against legacy introduced by the argument meant to fix the + renamed case. The resolved answer must cover every role the literal covered. + */ + const selected = await select("backlog", new Set(["backlog", "building", "signoff"])); + + expect(selected?.id).toBe("AG-IDLE"); + }); + + it("does not count work parked outside the supplied lanes", async () => { + // A finished card must not hold load against its agent, or the agent looks busy forever. + const selected = await select("shipped", new Set(["backlog", "building", "signoff"])); + + expect(selected?.id).toBe("AG-BUSY"); + }); +}); diff --git a/packages/engine/src/__tests__/ephemeral-worker-manager.test.ts b/packages/engine/src/__tests__/ephemeral-worker-manager.test.ts index 3697b1aaf2..3565705afc 100644 --- a/packages/engine/src/__tests__/ephemeral-worker-manager.test.ts +++ b/packages/engine/src/__tests__/ephemeral-worker-manager.test.ts @@ -436,3 +436,140 @@ describe("EphemeralWorkerManager", () => { expect(harness.manager.getOwner("FN-RESET")).toBeUndefined(); }); }); + +/* +FNXC:WorkflowLifecycleColumns 2026-07-31-06:10 (engine feed): + +THE INVARIANT: the zombie sweep asks the task's OWN workflow whether its worker is still working. + +A LIVE WORKER WAS REAPED ON A RENAMED BOARD, and the two guards compounded in the worst possible +order. `shouldDeleteOnSweep` tested a hard-coded terminal Set first, then fell through to +`return task.column !== "in-progress"`. On a renamed board the terminal test missed, and the fallthrough +is TRUE for a renamed wip lane — so an ephemeral worker ACTIVELY EXECUTING a task was classified as a +zombie and deleted, destroying work in flight. + +Census-invisible on both halves: the terminal check is a `Set` literal (a definition, not a +comparison), and the wip check was reached only after it. Found by grepping for lane-shaped list +literals, not by the backlog. + +THE FALLBACK IS DELIBERATELY ASYMMETRIC. An unresolvable workflow keeps the legacy literals rather +than guessing: failing to reap a dead worker costs a slot, reaping a live one destroys work. Those +are not symmetric, so the uncertain case must fail toward keeping the worker. + +REVERT PROOF, measured: restore the literal pair and the renamed-wip case fails — the live worker is +deleted. +*/ +describe("ephemeral zombie sweep resolves the board's own lanes", () => { + const RENAMED_IR = { + version: "v2", id: "wf-renamed", name: "renamed", nodes: [], edges: [], + columns: [ + { id: "backlog", name: "Backlog", traits: [{ trait: "intake" }, { trait: "hold" }] }, + { id: "building", name: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] }, + ], + }; + + function renamedHarness() { + const harness = createHarness(); + const selection = { workflowId: "wf-renamed", stepIds: [] as string[] }; + Object.assign(harness.taskStore as unknown as Record, { + getTaskWorkflowSelection: () => selection, + getTaskWorkflowSelectionAsync: async () => selection, + getWorkflowDefinition: async () => ({ ir: RENAMED_IR }), + }); + return harness; + } + + it("KEEPS a worker whose task sits in a RENAMED wip lane", async () => { + const harness = renamedHarness(); + const live = makeAgent("renamed-live", { metadata: { agentKind: "task-worker" }, taskId: "FN-LIVE" }); + harness.agentStore.agents.set(live.id, live); + harness.taskStore.tasks.set("FN-LIVE", makeTask("FN-LIVE", { column: "building" })); + + await harness.manager.reconcileOrphaned(); + + expect(harness.agentStore.agents.has(live.id)).toBe(true); + }); + + it("still reaps a worker whose task reached a RENAMED complete lane", async () => { + // The sweep must keep sweeping — keeping everything would be its own leak. + const harness = renamedHarness(); + const done = makeAgent("renamed-done", { metadata: { agentKind: "task-worker" }, taskId: "FN-DONE" }); + harness.agentStore.agents.set(done.id, done); + harness.taskStore.tasks.set("FN-DONE", makeTask("FN-DONE", { column: "shipped" })); + + await harness.manager.reconcileOrphaned(); + + expect(harness.agentStore.agents.has(done.id)).toBe(false); + }); + + it("still reaps a worker parked in a RENAMED hold lane", async () => { + const harness = renamedHarness(); + const parked = makeAgent("renamed-parked", { metadata: { agentKind: "task-worker" }, taskId: "FN-PARKED" }); + harness.agentStore.agents.set(parked.id, parked); + harness.taskStore.tasks.set("FN-PARKED", makeTask("FN-PARKED", { column: "backlog" })); + + await harness.manager.reconcileOrphaned(); + + expect(harness.agentStore.agents.has(parked.id)).toBe(false); + }); +}); + +/* +FNXC:WorkflowLifecycleColumns 2026-07-31-09:30 (#2787 review — greptile P1): +A SECOND wip lane must keep its worker too. + +`resolveLifecycleColumns` returns the FIRST column carrying each trait, so the first version of this +fix recognised only one implementation lane. A board declaring two — a common shape once a workflow +splits implementation from, say, an integration lane — still reaped a live worker in the second. The +same defect this suite exists to prevent, one degree narrower, and it would have surfaced the first +time someone added that column rather than at conversion time. + +The guard now unions `columnsWithFlag(ir, "countsTowardWip")`, which is every wip-bearing column. + +REVERT PROOF, measured: narrow the check back to `lanes.wip` and the second-lane case below fails. +*/ +describe("the zombie sweep honours EVERY wip lane, not the first", () => { + const TWO_WIP_IR = { + version: "v2", id: "wf-two-wip", name: "two wip", nodes: [], edges: [], + columns: [ + { id: "backlog", name: "Backlog", traits: [{ trait: "intake" }, { trait: "hold" }] }, + { id: "building", name: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "integrating", name: "Integrating", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] }, + ], + }; + + function twoWipHarness() { + const harness = createHarness(); + const selection = { workflowId: "wf-two-wip", stepIds: [] as string[] }; + Object.assign(harness.taskStore as unknown as Record, { + getTaskWorkflowSelection: () => selection, + getTaskWorkflowSelectionAsync: async () => selection, + getWorkflowDefinition: async () => ({ ir: TWO_WIP_IR }), + }); + return harness; + } + + it("KEEPS a worker whose task sits in the SECOND wip lane", async () => { + const harness = twoWipHarness(); + const live = makeAgent("second-lane-live", { metadata: { agentKind: "task-worker" }, taskId: "FN-LIVE" }); + harness.agentStore.agents.set(live.id, live); + harness.taskStore.tasks.set("FN-LIVE", makeTask("FN-LIVE", { column: "integrating" })); + + await harness.manager.reconcileOrphaned(); + + expect(harness.agentStore.agents.has(live.id)).toBe(true); + }); + + it("still keeps a worker in the FIRST wip lane", async () => { + const harness = twoWipHarness(); + const live = makeAgent("first-lane-live", { metadata: { agentKind: "task-worker" }, taskId: "FN-LIVE" }); + harness.agentStore.agents.set(live.id, live); + harness.taskStore.tasks.set("FN-LIVE", makeTask("FN-LIVE", { column: "building" })); + + await harness.manager.reconcileOrphaned(); + + expect(harness.agentStore.agents.has(live.id)).toBe(true); + }); +}); diff --git a/packages/engine/src/__tests__/eval-followups.test.ts b/packages/engine/src/__tests__/eval-followups.test.ts index 59dfa2d5de..27b1709d6b 100644 --- a/packages/engine/src/__tests__/eval-followups.test.ts +++ b/packages/engine/src/__tests__/eval-followups.test.ts @@ -83,6 +83,54 @@ describe("normalizeEvalFollowUps", () => { expect(followUps[0]?.matchedTaskId).toBe("FN-open"); }); + /* + FNXC:WorkflowLifecycleColumns 2026-07-31-06:40 (engine feed): + THE INVARIANT: "open" is the negation of the card's OWN terminal lanes. + + Census-invisible: the old gate was `OPEN_COLUMNS`, a `Set` literal — a definition, not a + comparison — so nothing in the lifecycle backlog pointed at this file. + + On a renamed board that set matched NOTHING, so `openTasks` was empty and this dedup had no live + work to compare against. Every eval run re-filed follow-ups it had already filed. The symptom is + DUPLICATE TASK CREATION, which reads as the evaluator being thorough rather than as a bug. + + REVERT PROOF, measured: restore `OPEN_COLUMNS.has(task.column)` and the first case below fails — + the duplicate is created instead of suppressed. + */ + it("suppresses a duplicate of an open task sitting in a RENAMED wip lane", async () => { + const followUps = await normalizeEvalFollowUps({ + parentTaskId: "FN-1", + runId: "ER-1", + overallBand: "weak", + drafts: [{ title: "Investigate flaky verification command", description: "Investigate flaky verification command causing reruns.", reason: "Failed verification", evidenceRefs: ["workflow-1"] }], + store: makeStore({ + openTasks: [{ id: "FN-open", column: "building", title: "Investigate flaky verification command", description: "x" }], + terminalColumnsByTaskId: { "FN-open": ["shipped", "vault"] }, + }), + policyMode: "persist_only", + }); + + expect(followUps[0]?.state).toBe("suppressed"); + expect(followUps[0]?.matchedTaskId).toBe("FN-open"); + }); + + it("does NOT suppress against a card in a RENAMED terminal lane", async () => { + // The dedup must stay scoped to live work: a finished card should not block a fresh follow-up. + const followUps = await normalizeEvalFollowUps({ + parentTaskId: "FN-1", + runId: "ER-1", + overallBand: "weak", + drafts: [{ title: "Investigate flaky verification command", description: "Investigate flaky verification command causing reruns.", reason: "Failed verification", evidenceRefs: ["workflow-1"] }], + store: makeStore({ + openTasks: [{ id: "FN-shipped", column: "shipped", title: "Investigate flaky verification command", description: "x" }], + terminalColumnsByTaskId: { "FN-shipped": ["shipped", "vault"] }, + }), + policyMode: "persist_only", + }); + + expect(followUps[0]?.suppressedReason).not.toBe("duplicate_open_task"); + }); + it("suppresses duplicates from prior eval results", async () => { const priorKey = normalizeEvalFollowUpText("FN-1:Add regression test for merge flow:Add regression test for merge flow regressions."); const followUps = await normalizeEvalFollowUps({ diff --git a/packages/engine/src/__tests__/scheduler-load-lane-union.test.ts b/packages/engine/src/__tests__/scheduler-load-lane-union.test.ts new file mode 100644 index 0000000000..3b4703327c --- /dev/null +++ b/packages/engine/src/__tests__/scheduler-load-lane-union.test.ts @@ -0,0 +1,75 @@ +// @vitest-environment node +/* +FNXC:WorkflowLifecycleColumns 2026-07-31-10:40 (#2787 review — greptile P1, second round): + +THE INVARIANT: the resolved load-lane set covers EVERY role the legacy literal covered. + +The legacy set is `{todo, in-progress, in-review}`, and `todo` is the HOLD/INTAKE lane. My first +resolved union covered only wip and review — and because passing the argument OVERRIDES the +fallback rather than extending it, assigned backlog work stopped counting as load. A regression +against legacy behaviour, introduced by the argument meant to fix the renamed case. + +That is the general trap with override-shaped options: the resolved answer must be a superset of what +the literal answered, or wiring the parameter is a downgrade for the roles it forgot. Cheap to get +wrong, invisible in a test that only checks the renamed lane. + +This asserts the union the scheduler builds, driven by the real trait resolver, since the call site +sits inside a dispatch path a unit test has no business standing up. +*/ +import { describe, expect, it } from "vitest"; +import { columnsWithFlag } from "@fusion/core"; +import type { WorkflowIr } from "@fusion/core"; + +const RENAMED_IR = { + version: "v2", id: "wf-renamed", name: "renamed", nodes: [], edges: [], + columns: [ + { id: "inbox", name: "Inbox", traits: [{ trait: "intake" }] }, + { id: "backlog", name: "Backlog", traits: [{ trait: "hold" }] }, + { id: "building", name: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "signoff", name: "Sign-off", traits: [{ trait: "merge" }] }, + { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] }, + ], +} as unknown as WorkflowIr; + +/** Mirrors the scheduler's union. Kept in step with `scheduler.ts` by the assertions below. */ +function loadLanes(ir: WorkflowIr): Set { + return new Set([ + ...columnsWithFlag(ir, "intake"), + ...columnsWithFlag(ir, "hold"), + ...columnsWithFlag(ir, "countsTowardWip"), + ...columnsWithFlag(ir, "mergeOrchestration"), + ...columnsWithFlag(ir, "mergeBlocker"), + ...columnsWithFlag(ir, "humanReview"), + ]); +} + +describe("the scheduler's load-lane union covers every legacy role", () => { + it("includes the hold and intake lanes — the roles `todo` filled", () => { + const lanes = loadLanes(RENAMED_IR); + + expect(lanes.has("backlog")).toBe(true); + expect(lanes.has("inbox")).toBe(true); + }); + + it("includes the wip and review lanes", () => { + const lanes = loadLanes(RENAMED_IR); + + expect(lanes.has("building")).toBe(true); + expect(lanes.has("signoff")).toBe(true); + }); + + it("excludes terminal lanes — finished work must not hold load against an agent", () => { + expect(loadLanes(RENAMED_IR).has("shipped")).toBe(false); + }); + + it("the scheduler builds this same union", () => { + // Guards the mirror above against drift: if scheduler.ts stops unioning a role, this fails. + const source = readFileSync(new URL("../scheduler.ts", import.meta.url), "utf8"); + + for (const flag of ["intake", "hold", "countsTowardWip", "mergeOrchestration", "mergeBlocker", "humanReview"]) { + expect(source).toContain(`...columnsWithFlag(loadLaneIr, "${flag}")`); + } + }); +}); + +import { readFileSync } from "node:fs"; diff --git a/packages/engine/src/agent-assignment.ts b/packages/engine/src/agent-assignment.ts index dba25db03b..f1747fac95 100644 --- a/packages/engine/src/agent-assignment.ts +++ b/packages/engine/src/agent-assignment.ts @@ -1,12 +1,30 @@ import type { Agent, AgentStore, Task, TaskStore } from "@fusion/core"; import { isAgentAutoAssignable, isEphemeralAgent } from "@fusion/core"; -const ACTIVE_COLUMNS = new Set(["todo", "in-progress", "in-review"]); +/* +FNXC:WorkflowLifecycleColumns 2026-07-31-05:40 (batch-engine feed): +The lanes an assigned card still counts as LOAD against its agent. + +CENSUS-INVISIBLE: this is a `Set` literal, i.e. a definition rather than a comparison, so nothing in +the lifecycle backlog ever pointed at this file. Found by grepping for lane-shaped list literals +after the same shape turned up in `duplicate-intake` and `blocker-fanout`. + +The failure is a silent DEGRADATION, not an error. This set gates the per-agent assignment-load +tally used to pick the least-loaded agent. On a renamed board no task's column matched, so +`assignmentLoad` stayed empty, every candidate compared as load 0, and the sort fell straight through +to its `createdAt` tiebreak — which is stable. The result is that the SAME agent wins every +assignment while the others sit idle. Nothing logs, nothing fails; the board just distributes badly. + +DELIBERATE-LITERAL — the fallback for a caller that cannot resolve lanes, reviewed 2026-07-31-05:40. +*/ +const LEGACY_ACTIVE_COLUMNS: ReadonlySet = new Set(["todo", "in-progress", "in-review"]); type SelectPermanentAgentForTaskOptions = { task: Task; agentStore: Pick; taskStore: Pick; + /** Resolved lanes that count as load. Omitted → the legacy trio, i.e. today's behaviour. */ + activeColumns?: ReadonlySet; }; function isAgentEnabled(agent: Agent): boolean { @@ -48,7 +66,7 @@ function taskLinksToScope(task: Pick, scop return false; } -export async function selectPermanentAgentForTask({ task, agentStore, taskStore }: SelectPermanentAgentForTaskOptions): Promise { +export async function selectPermanentAgentForTask({ task, agentStore, taskStore, activeColumns }: SelectPermanentAgentForTaskOptions): Promise { const eligibleAgents = await listEligibleExecutorAgents(agentStore); if (eligibleAgents.length === 0) { @@ -81,7 +99,7 @@ export async function selectPermanentAgentForTask({ task, agentStore, taskStore const assignmentLoad = new Map(); for (const taskItem of allTasks) { - if (!taskItem.assignedAgentId || !ACTIVE_COLUMNS.has(taskItem.column)) continue; + if (!taskItem.assignedAgentId || !(activeColumns ?? LEGACY_ACTIVE_COLUMNS).has(taskItem.column)) continue; assignmentLoad.set(taskItem.assignedAgentId, (assignmentLoad.get(taskItem.assignedAgentId) ?? 0) + 1); } diff --git a/packages/engine/src/ephemeral-worker-manager.ts b/packages/engine/src/ephemeral-worker-manager.ts index 5466834279..b37d9a8c2d 100644 --- a/packages/engine/src/ephemeral-worker-manager.ts +++ b/packages/engine/src/ephemeral-worker-manager.ts @@ -20,7 +20,7 @@ * sweep here close that gap. */ import type { AgentStore, AgentState, Agent, TaskStore, Task, Settings } from "@fusion/core"; -import { isEphemeralAgent } from "@fusion/core"; +import { isEphemeralAgent, resolveWorkflowIrForTask, columnsWithFlag } from "@fusion/core"; export interface TaskOwner { agentId: string; @@ -339,8 +339,44 @@ export class EphemeralWorkerManager { try { const task = await this.taskStore.getTask(agent.taskId); if (!task) return true; - if (TERMINAL_TASK_COLUMNS.has(task.column)) return true; - return task.column !== "in-progress"; + /* + FNXC:WorkflowLifecycleColumns 2026-07-31-06:10 (engine feed): + A LIVE WORKER WAS REAPED AS A ZOMBIE ON A RENAMED BOARD. + + Census-invisible: the terminal check is a `Set` literal (a definition, not a comparison), and + the wip check below it was the bare literal. Both missed on a renamed board, and they compound + in the WORST order — the terminal test failed, so control fell to + `return task.column !== "in-progress"`, which is TRUE for a renamed wip lane. An ephemeral + worker actively executing a task was therefore classified as a zombie and deleted by the sweep. + + Resolved from the task's OWN workflow. The `catch` below already treats an unreadable task as a + broken binding, so an unresolvable workflow keeps the documented literals rather than inventing + a lane: failing to reap a dead worker costs a slot, while reaping a live one destroys work in + flight, and those are not symmetric. + */ + /* + FNXC:WorkflowLifecycleColumns 2026-07-31-09:30 (#2787 review — greptile P1): + MEMBERSHIP, not first-per-role. `resolveLifecycleColumns` returns the FIRST column carrying each + trait, so a workflow declaring TWO implementation lanes had only one of them recognised — a live + worker in the second lane was still classified as a zombie and deleted. Same defect this commit + exists to fix, one degree narrower, and it would have reappeared the first time someone declared + a second wip lane. + + `columnsWithFlag` returns every column carrying the trait, so both halves are unions. + */ + const ir = await resolveWorkflowIrForTask(this.taskStore, task.id).catch(() => undefined); + if (ir === undefined) { + /* DELIBERATE-LITERAL — the unresolvable-workflow default, reviewed 2026-07-31-06:10. */ + if (TERMINAL_TASK_COLUMNS.has(task.column)) return true; + return task.column !== "in-progress"; + } + const terminalLanes = new Set([ + ...columnsWithFlag(ir, "complete"), + ...columnsWithFlag(ir, "archived"), + ]); + if (terminalLanes.has(task.column)) return true; + const wipLanes = new Set(columnsWithFlag(ir, "countsTowardWip")); + return !wipLanes.has(task.column); } catch { // If we can't even read the task, assume the binding is broken. return true; diff --git a/packages/engine/src/eval-followups.ts b/packages/engine/src/eval-followups.ts index 821b1e373a..3888b1bc76 100644 --- a/packages/engine/src/eval-followups.ts +++ b/packages/engine/src/eval-followups.ts @@ -8,8 +8,8 @@ import { type TaskStore, } from "@fusion/core"; import { resolveTerminalColumnsFor } from "./executor.js"; +import type { resolveWorkflowIrForTask } from "@fusion/core"; -const OPEN_COLUMNS = new Set(["triage", "todo", "in-progress", "in-review"]); /* FNXC:Evals 2026-07-26-00:00: Eval follow-ups are a real product feature, but they used to borrow the shared automated-recovery follow-up engine (`createAutomatedFollowup` in verification-followup-dedup.ts) purely for its dedup pass. That engine was deleted along with the recovery follow-up cards it existed to file, so the one dedup rule this feature actually needs is inlined here: never create a second card for the same `suggestionId` under the same parent while one is still open. Closed columns (done/archived) are excluded so a re-run after the follow-up is finished can legitimately file a fresh card. @@ -105,9 +105,41 @@ export function resolveEvalFollowUpPolicyMode(policy?: "off" | "suggest" | "crea export async function normalizeEvalFollowUps(input: NormalizeEvalFollowUpsInput): Promise { const { parentTaskId, runId, drafts, overallBand, store, policyMode } = input; - const openTasks = (await store.listTasks({ slim: true, includeArchived: false })).filter((task) => - OPEN_COLUMNS.has(task.column) - ); + /* + FNXC:WorkflowLifecycleColumns 2026-07-31-06:40 (engine feed): + "Open" is the negation of the task's OWN terminal lanes, not a hard-coded list of four ids. + + Census-invisible: `OPEN_COLUMNS` is a `Set` literal — a definition, not a comparison — so nothing + in the lifecycle backlog pointed here. Found by grepping for lane-shaped list literals. + + Consequence on a renamed board: the set matched NOTHING, so `openTasks` was empty and the + dedupe below had no live work to compare against. Every eval run then re-filed follow-ups it had + already filed — the failure is DUPLICATE TASK CREATION, which looks like the evaluator being + thorough rather than like a bug. + + Uses `resolveTerminalColumnsFor` — ALREADY IMPORTED IN THIS FILE for the candidate loop below, and + the same helper the executor uses. It unions the task's resolved terminals with the legacy pair for + the degraded-IR reason documented at its definition, so there is no fallback to hand-write here. + + My first draft manufactured synthetic trait flags from lane equality to call `isTerminalColumnRole` + instead. That is the anti-pattern I flagged in `task-update.ts` two commits ago — a longer way to + write the same comparison while LOOKING like it consulted the trait registry — and it ignored a + correct helper sitting three lines above the import. Replaced before commit. + */ + const allLiveTasks = await store.listTasks({ slim: true, includeArchived: false }); + /* + FNXC:WorkflowLifecycleColumns 2026-07-31-09:30 (#2787 review — greptile P2): + ONE IR read per WORKFLOW, not per card. This loop runs over every live task on the board, so + without the shared cache a large board paid a store read per card before a single follow-up draft + was processed — measurable added latency on every scheduled evaluation. The cache is the + caller-owned shape the rest of this program already uses. + */ + const openIrCache = new Map>>(); + const openTasks: typeof allLiveTasks = []; + for (const task of allLiveTasks) { + const terminal = await resolveTerminalColumnsFor(store, task.id, openIrCache); + if (!terminal.includes(task.column)) openTasks.push(task); + } // FNXC:Evals 2026-06-27-12:40: // getEvalStore() returns EvalStore | AsyncEvalStore (PG backend mode); await // resolves the sync array and the async promise alike. diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 119c6b498c..7f03b456f4 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -1789,7 +1789,17 @@ EXPORTED rather than copied. `eval-followups.ts` and `pr-comment-handler.ts` eac and fourth copy of the union-with-legacy reasoning is exactly the drift this program exists to remove. Nothing else about the function changes. */ -export async function resolveTerminalColumnsFor(store: TaskStore, taskId: string): Promise { +export async function resolveTerminalColumnsFor( + store: TaskStore, + taskId: string, + /* + FNXC:WorkflowLifecycleColumns 2026-07-31-09:30 (#2787 review — greptile P2): + Optional CALLER-OWNED IR cache, matching the contract on `resolveTaskLifecycleColumns`. Sweeps that + call this once per card on a whole board must read one IR per WORKFLOW, not one per task; callers + resolving a single task pass nothing and are unaffected. + */ + irCache?: Map>>, +): Promise { /* FNXC:WorkflowLifecycleColumns 2026-07-31-12:20 (PR #2568 review — greptile): THE UNION IS DELIBERATE, and the `catch` alone was not enough. @@ -1812,7 +1822,7 @@ export async function resolveTerminalColumnsFor(store: TaskStore, taskId: string column, which is the failure the conversion exists to prevent. */ try { - const resolved = resolveTerminalColumns(await resolveWorkflowIrForTask(store, taskId)); + const resolved = resolveTerminalColumns(await resolveWorkflowIrForTask(store, taskId, irCache)); return [...new Set([...resolved, ...LEGACY_TERMINAL_COLUMNS])]; } catch { return LEGACY_TERMINAL_COLUMNS; diff --git a/packages/engine/src/scheduler.ts b/packages/engine/src/scheduler.ts index ae152b0623..ea12f1764e 100644 --- a/packages/engine/src/scheduler.ts +++ b/packages/engine/src/scheduler.ts @@ -2296,10 +2296,48 @@ export class Scheduler { return null; } + /* + FNXC:WorkflowLifecycleColumns 2026-07-31-09:30 (#2787 review — greptile P1): + PASS THE RESOLVED LANES. Without this the optional parameter added to + `selectPermanentAgentForTask` is never supplied by the only production caller, so the + predicate keeps its legacy default and the load tally stays empty on a renamed board — + a converted function reachable only through an argument nobody passes is the + guard-that-cannot-fire pattern, and shipping one would have been worse than leaving the + literal in place, because the site then reads as done. + + The set is a MEMBERSHIP union of every wip/review lane the board declares, not the + first-per-role ids: a workflow may declare more than one implementation lane, and load + held in the second must still count. + */ + const loadLaneIr = await resolveWorkflowIrForTask(this.store, freshTask.id).catch(() => undefined); + /* + FNXC:WorkflowLifecycleColumns 2026-07-31-10:40 (#2787 review — greptile P1, second round): + THE HOLD AND INTAKE LANES COUNT AS LOAD TOO. + + The legacy set is `{todo, in-progress, in-review}` — and `todo` is the HOLD/INTAKE lane. + My first union covered only wip and review, so passing it OVERRODE the fallback and + dropped assigned backlog work from the tally: a regression against the legacy behaviour + for that lane, introduced by the very argument meant to fix the renamed case. + + That is the trap in overriding a default rather than extending it — the resolved answer + must cover EVERY role the literal covered, or wiring the parameter is a downgrade for the + roles it forgot. + */ + const activeLoadColumns = loadLaneIr === undefined + ? undefined + : new Set([ + ...columnsWithFlag(loadLaneIr, "intake"), + ...columnsWithFlag(loadLaneIr, "hold"), + ...columnsWithFlag(loadLaneIr, "countsTowardWip"), + ...columnsWithFlag(loadLaneIr, "mergeOrchestration"), + ...columnsWithFlag(loadLaneIr, "mergeBlocker"), + ...columnsWithFlag(loadLaneIr, "humanReview"), + ]); const selectedAgent = await selectPermanentAgentForTask({ task: freshTask, agentStore: this.options.agentStore, taskStore: this.store, + ...(activeLoadColumns && activeLoadColumns.size > 0 ? { activeColumns: activeLoadColumns } : {}), }); if (!selectedAgent) { await this.store.updateTask(task.id, { status: "queued" });