diff --git a/packages/engine/src/__tests__/agent-assignment.test.ts b/packages/engine/src/__tests__/agent-assignment.test.ts index a66f937ded..f530e7ba6c 100644 --- a/packages/engine/src/__tests__/agent-assignment.test.ts +++ b/packages/engine/src/__tests__/agent-assignment.test.ts @@ -232,7 +232,12 @@ describe("assignment load resolves the board's own active lanes", () => { task: makeTask({ id: "FN-NEW" }), agentStore: { listAgents: async () => agents, getChainOfCommand: async () => [] } as never, taskStore: store(columnOfBusyWork), - ...(activeColumns ? { activeColumns } : {}), + /* + #2787 review, third round: the option is now a PER-TASK predicate rather than a board-wide set, + because a project runs several workflows and a column id means something only relative to its + own. The tests keep expressing intent as a set and adapt it here. + */ + ...(activeColumns ? { countsAsAssignmentLoad: (t: { column: string }) => activeColumns.has(t.column) } : {}), }); it("prefers the idle agent when the busy one's work sits in a RENAMED wip lane", async () => { @@ -267,3 +272,99 @@ describe("assignment load resolves the board's own active lanes", () => { expect(selected?.id).toBe("AG-BUSY"); }); }); + +/* +FNXC:WorkflowLifecycleColumns 2026-07-31-11:40 (#2787 review — greptile P1, third round): + +THE INVARIANT: load is counted per task, against the task's OWN workflow. + +My first wiring resolved lanes from the CANDIDATE task's workflow and applied that flat set to every +assigned row. On a project running several workflows — the normal case, not an exotic one — +assignments in another workflow's load-bearing lanes vanished from the tally, and the +already-loaded-agent-wins bug returned through a different door. + +A column id means something only RELATIVE TO ITS OWN WORKFLOW. `blocker-fanout.ts` states this and +offers a per-task `classify`; the option is now the same shape rather than a third invention. + +REVERT PROOF, measured: answer the predicate from one workflow's lanes for every row (the flat-set +shape) and the cross-workflow case below picks the loaded agent. +*/ +/* +FNXC:WorkflowResolvedColumns 2026-07-30-12:25 (#2796 review — greptile): + +THE PREDICATE MUST SEE THE HELPER'S OWN ROWS, NOT A CALLER'S EARLIER SNAPSHOT. + +The scheduler built a Set of load-bearing task IDs from its own `listTasks` read, and this helper +then applied the predicate to rows from ITS read. Anything changing in between diverged in both +directions: a task MOVED out of a load-bearing lane kept its id in the set and still counted, while a +task created or newly assigned in between was missing and counted as zero. + +The fix memoises the resolved LANES per task and tests them against `candidate.column`, so the verdict +comes from the row the helper actually holds. That only works if the helper passes its own live rows +to the predicate — this pins that contract. If the helper ever pre-resolved or cached rows, the +scheduler's fix would silently go back to answering about a board that no longer exists. + +It is a contract test, not an end-to-end reproduction: the race lives in a dispatch path this suite +cannot stand up, and the existing `scheduler-load-lane-union` test says the same of its own call site. +*/ +describe("countsAsAssignmentLoad is called with the helper's own task rows", () => { + it("passes the live column, so a caller keyed on a stale snapshot cannot win", async () => { + const agents = [ + makeAgent({ id: "AG-A", createdAt: "2026-01-01T00:00:00.000Z" }), + makeAgent({ id: "AG-B", createdAt: "2026-01-02T00:00:00.000Z" }), + ]; + /* + The helper's snapshot: FN-MOVED has already left the load-bearing lane and sits in `shipped`. + A caller that decided "FN-MOVED bears load" from an earlier read must not be able to impose that. + */ + const taskStore = { + listTasks: async () => [ + makeTask({ id: "FN-MOVED", assignedAgentId: "AG-A", column: "shipped" } as never), + ], + } as never; + + const seen: Array<{ id: string; column: string }> = []; + const selected = await selectPermanentAgentForTask({ + task: makeTask({ id: "FN-NEW" }), + agentStore: { listAgents: async () => agents, getChainOfCommand: async () => [] } as never, + taskStore, + countsAsAssignmentLoad: (t: { id: string; column: string }) => { + seen.push({ id: t.id, column: t.column }); + /* The shape the scheduler now uses: resolved lanes for this task, tested against its LIVE column. */ + return new Set(["backlog", "building", "signoff"]).has(t.column); + }, + }); + + /* The predicate saw the helper's row, with the column as it is NOW. */ + expect(seen).toEqual([{ id: "FN-MOVED", column: "shipped" }]); + /* And therefore AG-A carries no load, so the older agent wins on the tiebreaker. */ + expect(selected?.id).toBe("AG-A"); + }); +}); + +describe("assignment load is counted per task, across workflows", () => { + it("counts an assignment held in ANOTHER workflow's wip lane", async () => { + const agents = [ + makeAgent({ id: "AG-BUSY", createdAt: "2026-01-01T00:00:00.000Z" }), + makeAgent({ id: "AG-IDLE", createdAt: "2026-01-02T00:00:00.000Z" }), + ]; + // The new card's board calls its wip lane `building`; the busy agent's existing work sits in a + // DIFFERENT workflow whose wip lane is `implementing`. + const taskStore = { + listTasks: async () => [ + makeTask({ id: "FN-OTHER-WF", assignedAgentId: "AG-BUSY", column: "implementing" } as never), + ], + } as never; + + const selected = await selectPermanentAgentForTask({ + task: makeTask({ id: "FN-NEW" }), + agentStore: { listAgents: async () => agents, getChainOfCommand: async () => [] } as never, + taskStore, + // Per-task: each row answered against its own workflow's lanes. + countsAsAssignmentLoad: (t: { column: string }) => + ["backlog", "building", "signoff"].includes(t.column) || ["queued", "implementing"].includes(t.column), + }); + + expect(selected?.id).toBe("AG-IDLE"); + }); +}); diff --git a/packages/engine/src/agent-assignment.ts b/packages/engine/src/agent-assignment.ts index f1747fac95..0c725dbb60 100644 --- a/packages/engine/src/agent-assignment.ts +++ b/packages/engine/src/agent-assignment.ts @@ -23,8 +23,21 @@ 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; + /* + FNXC:WorkflowLifecycleColumns 2026-07-31-11:40 (#2787 review — greptile P1, third round): + A PER-TASK predicate, not a flat set. + + The flat `activeColumns` I first added was resolved from the CANDIDATE task's workflow and then + applied to every row `listTasks` returned. On a project running several workflows, assignments in + another workflow's load-bearing lanes were omitted from the tally — the same + already-loaded-agent-wins bug this parameter exists to fix, now reachable through a different door. + + A column id is meaningful only RELATIVE TO ITS OWN WORKFLOW. `blocker-fanout.ts` documents exactly + this and offers `classify` for it; this mirrors that shape rather than inventing a third one. + + Omitted → the legacy trio, i.e. today's behaviour. + */ + countsAsAssignmentLoad?: (task: Task) => boolean; }; function isAgentEnabled(agent: Agent): boolean { @@ -66,7 +79,7 @@ function taskLinksToScope(task: Pick, scop return false; } -export async function selectPermanentAgentForTask({ task, agentStore, taskStore, activeColumns }: SelectPermanentAgentForTaskOptions): Promise { +export async function selectPermanentAgentForTask({ task, agentStore, taskStore, countsAsAssignmentLoad }: SelectPermanentAgentForTaskOptions): Promise { const eligibleAgents = await listEligibleExecutorAgents(agentStore); if (eligibleAgents.length === 0) { @@ -99,7 +112,11 @@ export async function selectPermanentAgentForTask({ task, agentStore, taskStore, const assignmentLoad = new Map(); for (const taskItem of allTasks) { - if (!taskItem.assignedAgentId || !(activeColumns ?? LEGACY_ACTIVE_COLUMNS).has(taskItem.column)) continue; + const bearsLoad = countsAsAssignmentLoad + ? countsAsAssignmentLoad(taskItem) + /* DELIBERATE-LITERAL — the unconverted-caller default, reviewed 2026-07-31-05:40. */ + : LEGACY_ACTIVE_COLUMNS.has(taskItem.column); + if (!taskItem.assignedAgentId || !bearsLoad) continue; assignmentLoad.set(taskItem.assignedAgentId, (assignmentLoad.get(taskItem.assignedAgentId) ?? 0) + 1); } diff --git a/packages/engine/src/scheduler.ts b/packages/engine/src/scheduler.ts index ea12f1764e..9a2c38c628 100644 --- a/packages/engine/src/scheduler.ts +++ b/packages/engine/src/scheduler.ts @@ -2309,35 +2309,67 @@ export class Scheduler { 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. + FNXC:WorkflowLifecycleColumns 2026-07-31-11:40 (#2787 review — greptile P1, third round): + RESOLVE PER TASK, because a project runs several workflows at once. - 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. + My first wiring resolved the lanes from the CANDIDATE task's workflow and handed that flat + set to a tally that runs over EVERY assigned row. Assignments living in another workflow's + load-bearing lanes were therefore omitted — the same already-loaded-agent-wins bug the + parameter exists to fix, reached through a different door. A column id means something + only relative to its OWN workflow; `blocker-fanout.ts` documents exactly this and offers a + per-task `classify`, so this passes a per-task predicate rather than a board-wide set. - 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. + One IR cache for the whole selection, per the caller-owned-cache contract, so a board + spanning three workflows reads three IRs and not one per assigned card. */ - 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 loadLaneIrCache = new Map>>(); + const resolveLoadLanes = async (candidate: Task): Promise> => { + const ir = await resolveWorkflowIrForTask(this.store, candidate.id, loadLaneIrCache).catch(() => undefined); + /* DELIBERATE-LITERAL — the unresolvable-workflow default. */ + if (!ir) return new Set(["todo", "in-progress", "in-review"]); + return new Set([ + ...columnsWithFlag(ir, "intake"), + ...columnsWithFlag(ir, "hold"), + ...columnsWithFlag(ir, "countsTowardWip"), + ...columnsWithFlag(ir, "mergeOrchestration"), + ...columnsWithFlag(ir, "mergeBlocker"), + ...columnsWithFlag(ir, "humanReview"), ]); + }; + /* + FNXC:WorkflowResolvedColumns 2026-07-30-12:10 (#2796 review — greptile): + MEMOISE THE LANES, NOT THE VERDICT — the two snapshots are not the same list. + + This pre-computed a Set of load-bearing task IDs from ITS OWN `listTasks` read, and + `selectPermanentAgentForTask` then applies the predicate to rows from ITS read. Anything + that changes in between diverges, and it diverges in both directions: a task MOVED out of + a load-bearing lane keeps its id in the set and is still counted, while a task created or + newly assigned in between is missing from the set and counts as zero. Either way the + balancer acts on a board that no longer exists. + + Caching the resolved LANES per task instead of a boolean removes the dependency. Lane + membership is a property of the task's workflow, which a move does not change, so the + predicate can be evaluated against the column on the row the helper actually holds. Only + the workflow lookup is memoised; the comparison is live. + + A task absent from the map (created between the two reads) falls back to the same legacy + trio the resolver itself uses when a workflow will not resolve, rather than silently + counting as no load. + */ + const LEGACY_LOAD_LANES: ReadonlySet = new Set(["todo", "in-progress", "in-review"]); + const loadLanesByTaskId = new Map>(); + for (const candidate of await this.store.listTasks({ slim: true })) { + if (!candidate.assignedAgentId) continue; + loadLanesByTaskId.set(candidate.id, await resolveLoadLanes(candidate)); + } + const selectedAgent = await selectPermanentAgentForTask({ task: freshTask, agentStore: this.options.agentStore, taskStore: this.store, - ...(activeLoadColumns && activeLoadColumns.size > 0 ? { activeColumns: activeLoadColumns } : {}), + countsAsAssignmentLoad: (candidate: Task) => + (loadLanesByTaskId.get(candidate.id) ?? LEGACY_LOAD_LANES).has(candidate.column), }); if (!selectedAgent) { await this.store.updateTask(task.id, { status: "queued" }); diff --git a/scripts/lib/lifecycle-column-census-baseline.json b/scripts/lib/lifecycle-column-census-baseline.json index 9065615db3..f88e4b0541 100644 --- a/scripts/lib/lifecycle-column-census-baseline.json +++ b/scripts/lib/lifecycle-column-census-baseline.json @@ -101,6 +101,8 @@ }, "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, @@ -109,8 +111,6 @@ "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/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, @@ -154,6 +154,7 @@ "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,