test(core): fix red main — assert the delete re-home against the resolver, not "triage" (#3229)
## What **Fixes a red main.** `workflow-reconciliation-production-shape.pg.test.ts` has been failing with `expected 'todo' to be 'triage'`. Test-only. Found while establishing a clean baseline for an unrelated coverage audit — my tree was clean at `origin/main` (`76c73238a0`), so this is not something I introduced. It is in the non-blocking suite, which is why it has stayed red. ## It is not a regression — the test was the stale half The delete path was deliberately fixed to re-home occupants using `resolveEntryColumnId(resolveDefaultWorkflowIr())` instead of `BUILTIN_CODING_WORKFLOW_IR`. This assertion was not updated with it. The two IRs are **not the same board**: | IR | entry column | |---|---| | `BUILTIN_CODING_WORKFLOW_IR` (`builtin:legacy-coding`) | `triage` | | `resolveDefaultWorkflowIr()` (the catalog default) | `todo` | Re-homing into `triage` put cards in a column the default board never declares. It slipped past `moveTask`'s undeclared-target guard **only because `triage` is a legacy id** and the recovery-rehome path exempts those — so the guard that exists to stop exactly this could not see it. So `todo` is the correct behaviour and the literal `"triage"` was what needed fixing. ## Why it asserts a resolver rather than `"todo"` Swapping one hardcoded id for another would be the identical trap one rename later — the same class of defect this whole program exists to remove. The expectation now derives from **the same two functions the product path calls**, so it cannot drift out of sync with them again. I also added the complement: the card must genuinely have **left** the vanished column, not merely match whatever a resolver returns. Without it, a resolver that started returning `custom-hold` would pass. ## Proven not appeasement Reverting the product line to the legacy IR — the original defect — fails this test: ``` AssertionError: expected 'triage' to be 'todo' Test Files 1 failed (1) / Tests 1 failed | 6 passed (7) ``` That is the check that matters for a test edit that turns a red green. It fails on the defect it describes. ## Measured ``` before: Tests 1 failed | 6 passed (7) after: Tests 7 passed (7) 16-file detector set: 181 passed (16 files) [was 1 failed | 180 passed] lint clean ``` ## Note on the reading I got this wrong twice before getting it right, and the record is worth having. My first read was "the test is stale, `triage` was merged away." My second was "the builtin IR still declares `triage`, so the *behaviour* is the defect" — which the IR file superficially supports. Only the third reading, of the FNXC note at the fix site, showed the file I was reading is the **legacy** IR and not the default one. Two of those three readings would have produced a confidently wrong PR; the deciding evidence was the comment the fixing author left at the call site, which is a good argument for writing them.
This commit is contained in:
@@ -39,6 +39,8 @@ reverted code. Anchor on surrounding context. Measured output is in the PR descr
|
||||
*/
|
||||
import { afterAll, afterEach, beforeAll, beforeEach, expect, it } from "vitest";
|
||||
import { BUILTIN_CODING_WORKFLOW_IR } from "../../builtin-coding-workflow-ir.js";
|
||||
import { resolveDefaultWorkflowIr } from "../../builtin-workflows.js";
|
||||
import { resolveEntryColumnId } from "../../workflow-reconciliation.js";
|
||||
import type { WorkflowIrV2 } from "../../workflow-ir-types.js";
|
||||
import {
|
||||
createSharedPgTaskStoreTestHarness,
|
||||
@@ -136,10 +138,29 @@ pgDescribe("U5 workflow reconciliation guards — production shape (no workflowC
|
||||
|
||||
await store.deleteWorkflowDefinition(workflow.id);
|
||||
|
||||
// Immediately after the delete — not at the next engine startup sweep — the card
|
||||
// must be out of the vanished column and in the default workflow's entry column.
|
||||
expect((await store.getTask(task.id)).column).toBe("triage");
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-21:30:
|
||||
ASSERTED AGAINST THE RESOLVER, NOT A LITERAL. This expectation was `"triage"` and had been
|
||||
failing on main: the delete path was fixed to re-home occupants using
|
||||
`resolveEntryColumnId(resolveDefaultWorkflowIr())` instead of `BUILTIN_CODING_WORKFLOW_IR`, and
|
||||
this assertion was not updated with it.
|
||||
|
||||
The two IRs are not the same board. `BUILTIN_CODING_WORKFLOW_IR` is `builtin:legacy-coding` and
|
||||
declares `triage`; the catalog's actual default does not. Re-homing into `triage` put cards in a
|
||||
column the default board never declares — it slipped past `moveTask`'s undeclared-target guard
|
||||
only because `triage` is a legacy id and the recovery-rehome path exempts those.
|
||||
|
||||
So `todo` is the CORRECT answer and the literal was the stale half. It is now derived from the
|
||||
same two functions the product path calls, which is the only form of this assertion that cannot
|
||||
drift out of sync with them again — a hardcoded `"todo"` would be the identical trap one rename
|
||||
later.
|
||||
*/
|
||||
const defaultEntry = resolveEntryColumnId(resolveDefaultWorkflowIr());
|
||||
expect(defaultEntry).toBeDefined();
|
||||
expect((await store.getTask(task.id)).column).toBe(defaultEntry);
|
||||
|
||||
/* The card must genuinely have left the vanished column, not merely match a resolver. */
|
||||
expect((await store.getTask(task.id)).column).not.toBe("custom-hold");
|
||||
});
|
||||
|
||||
// ── Guard 3: workflow switch ──────────────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user