From 76c73238a087304b0bcafd51e67c5c592ef8a16a Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 12:14:40 -0700 Subject: [PATCH] test(core): pin the engine-downtime shift's wip read (428 tests could not see it) (#3225) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What Pins the **engine-downtime timing shift's wip read** in `packages/core/src/store.ts`. Test-only — no product change. First audited site in `core`. `reconcileActiveTimingForEngineDowntime` (FN-7011/FN-7975) excludes proven stopped-engine wall-clock from a card's active time. It finds the cards to fix by querying the board's wip lane. **Blinding that read back to `["in-progress"]` left every test that touches the sweep green — 4 in this file plus 424 in the two engine files that exercise it, 428 in total.** ## Why 428 tests were blind to it The existing store double is 10 lines and contains **both** documented anti-patterns, either one sufficient on its own: 1. **`listTasks: vi.fn(async () => tasks)` ignores its `column` argument** — it returns the same rows whichever lane is requested. A fake that ignores its own filter cannot see a filter bug, which is exactly the bug this resolver exists to fix. 2. **No `listWorkflowDefinitions`** — `resolveProjectColumnsForRoles` then returns the legacy ids and nothing else (an intentional degrade in `project-lane-vocabulary.ts` so an unreadable workflow list cannot fail a sweep). The resolved set and the literal set were *equal by construction*. The new double fixes both and changes nothing else. **The existing cases keep the original double on purpose:** they are about heartbeat and threshold arithmetic, not lanes, and rewriting them would put unrelated churn in the same commit. ## Measured | | default (control) | renamed | differential | non-wip card | |---|---|---|---|---| | converted | pass | pass | pass | pass | | blinded to `["in-progress"]` | pass | **FAIL** | **FAIL** | pass | ``` converted: Test Files 1 passed (1) / Tests 8 passed (8) blinded: Test Files 1 failed (1) / Tests 2 failed | 6 passed (8) engine neighbours (project-engine-unpause-active-timing + self-healing): 424 tests, green and unchanged lint clean; fnxc-future-dates: none added; census unchanged ``` Blind confirmed applied with `git diff --stat` before each run, not inferred from the tool's exit code. ## What breaks without it On a board whose wip lane is `building`, the sweep queries `in-progress`, finds **no tasks**, and shifts no anchor. Every card silently absorbs the stopped-engine wall-clock the sweep exists to exclude. The reported active time is simply wrong and nothing fails to signal it — the same silent-wrong-number shape as the evaluator defect in #3224. ## Also covers the complement A held card *outside* the wip lane is **not** shifted. Widening a lane read is the kind of change that can quietly turn a targeted sweep into a board-wide rewrite; a card in `todo` has no stopped-engine time to exclude, and there is now a case saying so. ## Scope note `packages/core` is not my package. This is an additive test file with no product change, so collision risk is low, but I am flagging it rather than assuming: **16 of core's 17 files with resolver call sites remain unaudited** and I claim nothing about them. The audit method and its failure modes are documented in #3223 if core's owner wants to continue it. --- .../src/__tests__/store-active-timing.test.ts | 105 ++++++++++++++++++ 1 file changed, 105 insertions(+) diff --git a/packages/core/src/__tests__/store-active-timing.test.ts b/packages/core/src/__tests__/store-active-timing.test.ts index d4a1967f33..bea941af59 100644 --- a/packages/core/src/__tests__/store-active-timing.test.ts +++ b/packages/core/src/__tests__/store-active-timing.test.ts @@ -84,3 +84,108 @@ describe("TaskStore.reconcileActiveTimingForEngineDowntime", () => { expect(thresholdStore.updateTask).not.toHaveBeenCalled(); }); }); + +/* +FNXC:WorkflowResolvedColumns 2026-07-31-20:15: +THE DOWNTIME SHIFT'S WIP READ, on a RENAMED board. + +This sweep excludes stopped-engine wall-clock from a card's active time. It finds the cards to fix by +querying the board's wip lane, a read converted to `resolveProjectColumnsForRoles(this, +["countsTowardWip"])`. + +WHY THESE CASES EXIST. Blinding that resolver back to `["in-progress"]` left every test that touches +this sweep green — 4 here plus 424 in the two engine files that exercise it, 428 in total. The double +above cannot see the conversion for two independent reasons, and it takes only one: + + 1. `listTasks: vi.fn(async () => tasks)` ignores its `column` argument, so it returns the same rows + whichever lane is requested. A fake that ignores its own filter cannot see a filter bug — which + is precisely the bug this resolver exists to fix. + 2. `resolveProjectColumnsForRoles` returns the LEGACY ids and nothing else when the store has no + `listWorkflowDefinitions` (an intentional degrade in project-lane-vocabulary.ts so an unreadable + workflow list cannot fail a sweep). Without that method the resolved set and the literal set are + equal by construction. + +The double below fixes both and changes nothing else. The existing cases keep the original double on +purpose: they are about heartbeat and threshold arithmetic, not lanes, and rewriting them would put +unrelated churn in the same commit. + +WHAT BREAKS WITHOUT THE CONVERSION. On a board whose wip lane is `building`, the sweep queries +`in-progress`, finds NO tasks, and shifts no anchor. Every card silently absorbs the stopped-engine +wall-clock the sweep exists to exclude — the task's reported active time is simply wrong, with +nothing failing to signal it. +*/ + +const RENAMED_WIP = "building"; + +function createLaneAwareStoreDouble( + settings: Record, + tasks: Array, + wipColumn: string, +) { + const updateTask = vi.fn(async (id: string, patch: { executionStartedAt: string }) => { + const task = tasks.find((candidate) => candidate.id === id); + if (task) task.executionStartedAt = patch.executionStartedAt; + }); + const ir = { + version: "v2", + id: "custom:renamed-wip", + nodes: [], + edges: [], + columns: [ + { id: "todo", label: "Hold", traits: [{ trait: "hold", config: { release: "capacity" } }] }, + { id: wipColumn, label: "Wip", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "done", label: "Complete", traits: [{ trait: "complete" }] }, + ], + }; + return { + getSettings: vi.fn(async () => settings), + /* Honours `column`, unlike the double above. */ + listTasks: vi.fn(async (query?: { column?: string }) => + (query?.column ? tasks.filter((task) => task.column === query.column) : tasks)), + updateTask, + /* Without this the resolver hands back legacy ids only. */ + listWorkflowDefinitions: vi.fn(async () => [{ ir }]), + getWorkflowDefinition: vi.fn(async () => ({ ir })), + }; +} + +describe("TaskStore.reconcileActiveTimingForEngineDowntime resolves the board's own wip lane", () => { + async function shiftedIdsFor(wipColumn: string): Promise { + const tasks = [{ id: "FN-active", executionStartedAt: startedBeforeHeartbeat, column: wipColumn }]; + const store = createLaneAwareStoreDouble( + { pollIntervalMs: 15_000, engineLastActiveAt: staleHeartbeat }, + tasks, + wipColumn, + ); + const result = await TaskStore.prototype.reconcileActiveTimingForEngineDowntime.call(store as never, now); + return result.shiftedTaskIds; + } + + it("default vocabulary: shifts a card resting in the wip lane", async () => { + expect(await shiftedIdsFor("in-progress")).toEqual(["FN-active"]); + }); + + it("renamed vocabulary: shifts a card resting in the RENAMED wip lane", async () => { + expect(await shiftedIdsFor(RENAMED_WIP)).toEqual(["FN-active"]); + }); + + it("both vocabularies reach the SAME outcome — no column-id literal survives on this path", async () => { + expect(await shiftedIdsFor(RENAMED_WIP)).toEqual(await shiftedIdsFor("in-progress")); + }); + + it("does not shift a card outside the wip lane on a renamed board", async () => { + /* + The complement of the case above: widening the lane read must not turn the sweep into a + board-wide rewrite. A held card has no stopped-engine time to exclude. + */ + const tasks = [{ id: "FN-held", executionStartedAt: startedBeforeHeartbeat, column: "todo" }]; + const store = createLaneAwareStoreDouble( + { pollIntervalMs: 15_000, engineLastActiveAt: staleHeartbeat }, + tasks, + RENAMED_WIP, + ); + const result = await TaskStore.prototype.reconcileActiveTimingForEngineDowntime.call(store as never, now); + expect(result.shiftedTaskIds).toEqual([]); + expect(store.updateTask).not.toHaveBeenCalled(); + }); +});