From da10131d3f0b772f957102bd10d3afb64c2e233a Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 09:07:45 -0700 Subject: [PATCH] =?UTF-8?q?test(triage):=20the=20mock=20store=20could=20no?= =?UTF-8?q?t=20be=20ASKED=20for=20a=20selection=20=E2=80=94=20231=20cases?= =?UTF-8?q?=20exercised=20a=20shape=20production=20cannot=20produce=20(#31?= =?UTF-8?q?89)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `createMockStore` in `triage.test.ts` defined **neither** `getTaskWorkflowSelection` nor its async twin. So `resolveWorkflowIrForTaskWithProvenance` **threw** calling them and took its catch branch, reporting `source: "default"` in the sense of *"the lookup failed"*. Production stores always expose both readers — every case in this file was exercising a store shape that cannot exist. Returning `undefined` models the real answer: the store **can** be asked and says there is no selection row, which is what a pre-U11 card actually presents. ## Why it mattered `triage.ts`'s post-U11 intake recovery gates on that provenance. A *failed* lookup correctly refuses to claim a workflow lacks `triage`, so the orphan arm stayed off and the recovery depended on `resolvePlannerLanes` **failing** and falling back to legacy ids — correctness resting on a resolver's failure mode. In #3141 I measured the async conversion of that site as failing 13 cases and **twice reported it as a production constraint**. It was this harness. That is the concrete cost of a mock that cannot answer a question production always can. ## Behaviour-preserving on its own **380 passed across 26 triage/recovery suites.** ## What this deliberately does NOT do It does not convert the site. I prototyped the full unblock — a `selectionAbsent` flag on the determinate `!workflowId` branch, its single consumer, and the async conversion — and it works: the previously-failing suite goes **237 passed**. But with a realistic store the orphan arm starts firing for no-selection rows, which changes recovery flow in **5 `triage-stuck-requeue-preserve-draft` cases** that currently assert the refusing behaviour. Whether accepting a legacy `triage` row there is correct is a lifecycle-semantics decision about migration, not a harness fix. So it is reverted and reported rather than bundled. Findings and the measured branch table are on #3141. ## One correction carried from this work I filed #3187 claiming provenance verifies resolution via `ir.id === workflowId`, which cannot pass for builtins. **That was wrong** — the live code uses a symbol marker, and the text I quoted was historical prose describing what was removed. Closed with the measurement: ``` store with NO selection readers -> source: default (catch: could not ask) store answering builtin selection -> source: selection ✓ ``` That is the same class of error this PR fixes — reasoning from what something says rather than what it does. ## Summary by CodeRabbit * **Tests** * Improved workflow-resolution test coverage by supporting stores with no selected workflow. * Added synchronous and asynchronous test readers for workflow selection. Co-authored-by: Claude Opus 5 (1M context) --- packages/engine/src/__tests__/triage.test.ts | 21 ++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/packages/engine/src/__tests__/triage.test.ts b/packages/engine/src/__tests__/triage.test.ts index e8a48bf702..d27a9deb56 100644 --- a/packages/engine/src/__tests__/triage.test.ts +++ b/packages/engine/src/__tests__/triage.test.ts @@ -127,6 +127,27 @@ async function cleanupTriageFixtureRoot(rootDir: string | undefined): Promise = {}): TaskStore { let store: Partial; store = { + /* + FNXC:WorkflowResolvedColumns 2026-07-31-23:59: + THE MOCK MUST BE ABLE TO ANSWER "no selection" — it could not even be ASKED. + + Neither `getTaskWorkflowSelection` nor its async twin was defined here, so + `resolveWorkflowIrForTaskWithProvenance` threw calling them and took its CATCH branch, reporting + `source: "default"` in the sense of "the lookup failed". Production stores always expose both + readers, so that shape cannot occur there — every case in this file was exercising a store that + does not exist. + + It matters because `triage.ts`'s post-U11 intake recovery gates on that provenance: a failed + lookup correctly refuses to claim a workflow lacks `triage`, so the orphan arm stayed off and the + recovery depended on `resolvePlannerLanes` FAILING and falling back to legacy ids. Measured in + #3141: converting that site to the async resolver failed 13 cases against this harness, and I + twice mistook that for a production constraint. + + Returning `undefined` models the real "no selection row" answer — the store CAN be asked and says + there is none — which is the case a pre-U11 row actually presents. + */ + getTaskWorkflowSelection: vi.fn(() => undefined), + getTaskWorkflowSelectionAsync: vi.fn(async () => undefined), getTask: vi.fn(), listTasks: vi.fn().mockResolvedValue([]), createTask: vi.fn(),