From e9bfd0d313b20dc8a4b0e52193af3f3bd75905d7 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Tue, 28 Jul 2026 22:19:22 -0700 Subject: [PATCH] =?UTF-8?q?test(engine):=20prove=20the=20admission-control?= =?UTF-8?q?=20agent=20count=20on=20a=20renamed=20board=20=E2=80=94=20and?= =?UTF-8?q?=20two=20of=20my=20own=20cases=20were=20vacuous=20(#2516)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test-only. Fifth E2E family. Closes three of the four `live-agent-count` classifications. ## Why this one is a scheduler bug, not a display bug `live-agent-count.ts` classifies a card's column, and `persistedTopLevelAgentSlotsFromStore` turns that into **the number admission control compares against the cap**. So a mis-classified column fails in whichever direction hurts: | mis-classification | consequence | |---|---| | wip column not recognised | under-count → **over-admits past the operator's cap** | | complete column not recognised | a finished card counts forever → **board silently stalls** | Both are silent, and both land only on a renamed board. Everything in the path is real: PostgreSQL store, real persisted workflows, cards walked through the real transition policy, and the real counting function resolving each card's own IR. Nothing about counting is reimplemented here. ## Two of my own cases were vacuous — mutation-testing caught it This is the more useful half of the PR. **1. "does not count a card in the COMPLETE column" passed with the terminal classification hardcoded to `done`.** `isRunningAgentTask` rejects that card at the *wip* check anyway, so the test was really asserting "shipped isn't a wip column". `terminalKind` short-circuits **first**, so it only changes the answer for a card whose status would otherwise make it count. Now covered by a complete card carrying a live-looking `planning` status — the state a crashed run leaves behind, which on a renamed board consumes a slot forever. **2. The review/merge lane had no case at all.** A review status is deliberately *not* globally live (a stale `fixing` in wip must not consume capacity), so it is gated on `columnIsReviewOrMerge`. If the renamed review lane isn't recognised, a genuinely-active reviewer stops counting and admission control lets another agent in over the cap. Now covered by a review card with an active merge-pipeline status. Both new cases assert their fixture took effect first, so they can't degrade back into the weaker version silently. ## Mutation-verified independently | classification | mutation | result | |---|---|---| | `countsTowardWip` | → `"in-progress"` | 3 renamed cases fail | | `complete` | → `id === "done"` | exactly the new terminal case fails | | `mergeBlocker` | → `id === "in-review"` | exactly the new review case fails | ## What this does NOT cover, stated plainly The fourth classification, `columnIsIntakeOrHold`, is read only by the **waiting** predicate, which the admission count never calls. It stays in the ledger as unproven rather than being claimed by proximity — the mistake I made last slice with `resolveMergeOrchestrationColumn`. ## Verification - five live-E2E suites green together: **52/52** - engine `tsc --noEmit` clean - `pnpm test:gate` green (309 + 10 + 71) 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Summary by CodeRabbit * **Tests** * Added end-to-end coverage for live agent-count admission across workflow lanes and lifecycle states. * Verified slot handling for active, completed, held, and mid-review tasks, including stale statuses. * Confirmed mixed-lane counts and renamed board vocabularies produce consistent results. * **Documentation** * Expanded coverage notes for live agent-count classifications and waiting-state behavior. Co-authored-by: Claude Opus 5 (1M context) --- .../workflow-agent-count-live-e2e.pg.test.ts | 190 ++++++++++++++++++ .../workflow-lifecycle-live-e2e.pg.test.ts | 13 +- 2 files changed, 202 insertions(+), 1 deletion(-) create mode 100644 packages/engine/src/__tests__/workflow-agent-count-live-e2e.pg.test.ts diff --git a/packages/engine/src/__tests__/workflow-agent-count-live-e2e.pg.test.ts b/packages/engine/src/__tests__/workflow-agent-count-live-e2e.pg.test.ts new file mode 100644 index 0000000000..e898bbb547 --- /dev/null +++ b/packages/engine/src/__tests__/workflow-agent-count-live-e2e.pg.test.ts @@ -0,0 +1,190 @@ +/* +FNXC:WorkflowLifecycleColumns 2026-07-28-15:10 (E2E — closing the live-agent-count ledger entry): + +`live-agent-count.ts` classifies a card's column into the four traits the running/ +waiting predicates read, and `persistedTopLevelAgentSlotsFromStore` turns that into +THE NUMBER ADMISSION CONTROL COMPARES AGAINST THE CAP +(`computeTopLevelConcurrencyClaimed`). So a mis-classified column is not a display +bug — it is a scheduler bug, and it fails in whichever direction hurts: + + a wip column not recognised as wip -> the board under-counts and OVER-ADMITS, + running more agents than the operator capped + a complete column not recognised -> a finished card counts forever and the + board silently stalls at the cap + +Both are the same failure this program keeps finding — silent, no error, no failing +test — and both land on a renamed board only, which is why nothing had caught them. + +WHAT IS REAL. A PostgreSQL TaskStore, real persisted workflow definitions, real +tasks moved through the real transition policy, and the real +`persistedTopLevelAgentSlotsFromStore` resolving each card's IR from the store. +Nothing about the counting is reimplemented here — the assertion is the number the +scheduler would actually use. +*/ +import { beforeAll, beforeEach, afterEach, afterAll, describe, expect, it } from "vitest"; +import "@fusion/core"; // registers the built-in column traits +import type { Task } from "@fusion/core"; + +import { + pgDescribe, + createSharedPgTaskStoreTestHarness, + type SharedPgTaskStoreHarness, +} from "../../../core/src/__test-utils__/pg-test-harness.js"; +import { persistedTopLevelAgentSlotsFromStore } from "../concurrency.js"; +import { DEFAULT_VOCAB, RENAMED_VOCAB, lifecycleIr, type Vocabulary } from "./_workflow-vocabulary-fixture.js"; + +pgDescribe("live agent-count E2E: what the cap is actually compared against", () => { + const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({ + prefix: "fusion_agent_count_e2e", + }); + + beforeAll(h.beforeAll); + beforeEach(h.beforeEach); + afterEach(h.afterEach); + afterAll(h.afterAll); + + async function seedWorkflow(v: Vocabulary, key: string): Promise { + const created = await h.store().createWorkflowDefinition({ + name: `Agent count ${key}`, + kind: "workflow", + ir: lifecycleIr(v, `custom:${key}`), + } as never); + return (created as { id: string }).id; + } + + /** Walk a card to `target` through the REAL transition policy, so the row is one a + * real run could produce rather than a hand-written column value. */ + async function seedTaskAt(taskId: string, v: Vocabulary, workflowId: string, target: string): Promise { + const store = h.store(); + await store.createTaskWithReservedId( + { description: `count ${taskId}`, column: v.hold } as never, + { taskId, applyDefaultWorkflowSteps: false } as never, + ); + await store.writeTaskWorkflowSelection(taskId, workflowId, []); + if (target === v.hold) { + store.taskCache.delete(taskId); + return; + } + await store.moveTask(taskId, v.wip, { moveSource: "user" } as never); + if (target !== v.wip) { + await store.moveTask(taskId, v.review, { moveSource: "user", allowDirectInReviewMove: true } as never); + } + if (target === v.complete) { + await store.moveTask(taskId, v.complete, { moveSource: "engine", skipMergeBlocker: true } as never); + } + store.taskCache.delete(taskId); + } + + /** The number admission control would compare against the cap, computed the way it + * computes it: through the store, resolving each card's own workflow. */ + async function claimedSlots(): Promise { + const store = h.store(); + const tasks = (await store.listTasks({ includeArchived: false })) as Task[]; + return persistedTopLevelAgentSlotsFromStore(store as never, tasks); + } + + describe.each([ + { label: "RENAMED vocabulary", vocab: RENAMED_VOCAB, key: "renamed" }, + { label: "DEFAULT vocabulary (regression floor)", vocab: DEFAULT_VOCAB, key: "default" }, + ])("$label", ({ vocab, key }) => { + it("counts a card in the WIP column as a claimed slot", async () => { + const workflowId = await seedWorkflow(vocab, `${key}-wip`); + await seedTaskAt(`FN-AC-${key}-1`, vocab, workflowId, vocab.wip); + + expect(await claimedSlots()).toBe(1); + }); + + it("does NOT count a card in the COMPLETE column — a finished card must release its slot", async () => { + /* The direction that silently stalls a board: if the terminal column is not + recognised, every completed card keeps consuming capacity forever. */ + const workflowId = await seedWorkflow(vocab, `${key}-complete`); + await seedTaskAt(`FN-AC-${key}-2`, vocab, workflowId, vocab.complete); + + expect(await claimedSlots()).toBe(0); + }); + + it("does NOT count a COMPLETE card that still carries a live-looking status", async () => { + /* + THE CASE THAT ACTUALLY TESTS THE TERMINAL CLASSIFICATION. The plain + complete-column case above cannot: `isRunningAgentTask` rejects that card at the + wip check anyway, so it stays at 0 even with the terminal classification keyed + on the literal `done` (verified by mutation — every other case here stayed green). + + `terminalKind` is checked FIRST and short-circuits, so it only changes the answer + for a card whose status would otherwise make it count. `status: "planning"` + returns true immediately after that check. A finished card carrying a stale + status is exactly the state a crashed or interrupted run leaves behind — and on a + renamed board it would then consume a slot forever, which is the silent stall + this file is about. + */ + const workflowId = await seedWorkflow(vocab, `${key}-terminal`); + const taskId = `FN-AC-${key}-7`; + await seedTaskAt(taskId, vocab, workflowId, vocab.complete); + await h.store().updateTask(taskId, { status: "planning" } as never); + h.store().taskCache.delete(taskId); + // Prove the fixture took: without the status this case degrades into the weaker + // one above and would pass for the wrong reason. + expect((await h.store().getTask(taskId)).status).toBe("planning"); + + expect(await claimedSlots()).toBe(0); + }); + + it("DOES count a card mid-review with an active merge-pipeline status", async () => { + /* + The third classification, and the other over-admission direction. A review + status is deliberately NOT globally live — a stale `fixing` in an intake or wip + column must not consume capacity — so `isRunningAgentTask` gates it on + `columnIsReviewOrMerge`. If the review lane is not recognised on a renamed + board, a genuinely-active reviewer stops counting and admission control lets + another agent in over the cap. + */ + const workflowId = await seedWorkflow(vocab, `${key}-review`); + const taskId = `FN-AC-${key}-8`; + await seedTaskAt(taskId, vocab, workflowId, vocab.review); + await h.store().updateTask(taskId, { status: "fixing" } as never); + h.store().taskCache.delete(taskId); + expect((await h.store().getTask(taskId)).status).toBe("fixing"); + + expect(await claimedSlots()).toBe(1); + }); + + it("does NOT count a card waiting in the HOLD column", async () => { + const workflowId = await seedWorkflow(vocab, `${key}-hold`); + await seedTaskAt(`FN-AC-${key}-3`, vocab, workflowId, vocab.hold); + + expect(await claimedSlots()).toBe(0); + }); + + it("counts exactly the WIP cards across a mixed board", async () => { + /* The number that actually matters: with one card in each lane, admission + control must see exactly one claimed slot. Over-counting stalls the board; + under-counting over-admits past the operator's cap. */ + const workflowId = await seedWorkflow(vocab, `${key}-mixed`); + await seedTaskAt(`FN-AC-${key}-4`, vocab, workflowId, vocab.hold); + await seedTaskAt(`FN-AC-${key}-5`, vocab, workflowId, vocab.wip); + await seedTaskAt(`FN-AC-${key}-6`, vocab, workflowId, vocab.complete); + + expect(await claimedSlots()).toBe(1); + }); + }); + + it("counts a RENAMED board identically to the default one — same shape, different ids", async () => { + /* The differential, run in one process against one store: two boards with the + same lane occupancy must claim the same number of slots. A classifier keyed on + legacy ids returns a different number for the renamed board, and the two counts + diverge here even though every other assertion in this file could still pass. */ + const renamedWf = await seedWorkflow(RENAMED_VOCAB, "diff-renamed"); + await seedTaskAt("FN-AC-D1", RENAMED_VOCAB, renamedWf, RENAMED_VOCAB.wip); + await seedTaskAt("FN-AC-D2", RENAMED_VOCAB, renamedWf, RENAMED_VOCAB.complete); + const renamedOnly = await claimedSlots(); + + const defaultWf = await seedWorkflow(DEFAULT_VOCAB, "diff-default"); + await seedTaskAt("FN-AC-D3", DEFAULT_VOCAB, defaultWf, DEFAULT_VOCAB.wip); + await seedTaskAt("FN-AC-D4", DEFAULT_VOCAB, defaultWf, DEFAULT_VOCAB.complete); + const both = await claimedSlots(); + + expect(renamedOnly).toBe(1); + // The default board added exactly one more claimed slot, not two and not zero. + expect(both - renamedOnly).toBe(1); + }); +}); diff --git a/packages/engine/src/__tests__/workflow-lifecycle-live-e2e.pg.test.ts b/packages/engine/src/__tests__/workflow-lifecycle-live-e2e.pg.test.ts index a323a16b9d..89e6d85422 100644 --- a/packages/engine/src/__tests__/workflow-lifecycle-live-e2e.pg.test.ts +++ b/packages/engine/src/__tests__/workflow-lifecycle-live-e2e.pg.test.ts @@ -685,6 +685,16 @@ two self-healing sweeps verified PER SITE — reverting one fails exactly its ow - task-agent-sync.ts resolveTaskLifecycleColumns — agent-link hygiene on a real TaskStore + real AgentStore + the real `task:moved` subscription (workflow-agent-link-live-e2e.pg.test.ts; mutation-verified) + - core/live-agent-count.ts THREE of the four classifications, through the number + admission control actually compares against the cap + (persistedTopLevelAgentSlotsFromStore; workflow-agent-count-live-e2e.pg.test.ts): + countsTowardWip -> mutation fails 3 renamed cases + terminal/complete -> needed its OWN case; the plain complete-column test is + rejected at the wip check anyway and stayed green under + mutation. Only a complete card carrying a live-looking + status discriminates. + mergeOrchestration/mergeBlocker -> needed its own case too (a review card with an + active merge-pipeline status) - auto-merge-finalization.ts completeColumn / mergeColumn / isCompleteColumn (workflow-merge-family-live-e2e.pg.test.ts; each of the three mutation-verified INDEPENDENTLY — the mergeColumn one needed its own case, see below) @@ -703,7 +713,8 @@ NOT PROVEN end to end — real callers this suite does not reach: - executor.ts:1763,6339,6341 rebound target, merge-orchestration probe, complete column - mesh-lease-manager.ts:61 resolveReboundTarget - core/task-store/reads.ts:130 listTasks hydration - - core/live-agent-count.ts:63-75 five columnHasFlag classifications + - core/live-agent-count.ts columnIsIntakeOrHold (the WAITING predicate) — the running + predicate never reads it, so the admission-count E2E cannot reach it - dashboard register-task-workflow-routes.ts:151,166,175,1797 WHY, and what each would take: