test(core): pin the engine-downtime shift's wip read (428 tests could not see it) (#3225)
## 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.
This commit is contained in:
@@ -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<string, unknown>,
|
||||
tasks: Array<TimingTask & { column: string }>,
|
||||
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<string[]> {
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user