From dd09e57511a12f40f4bf21b16c76f25177d2c02e Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 12:31:04 -0700 Subject: [PATCH] =?UTF-8?q?test(core):=20fix=20red=20main=20=E2=80=94=20as?= =?UTF-8?q?sert=20the=20delete=20re-home=20against=20the=20resolver,=20not?= =?UTF-8?q?=20"triage"=20(#3229)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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. --- ...reconciliation-production-shape.pg.test.ts | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/packages/core/src/__tests__/postgres/workflow-reconciliation-production-shape.pg.test.ts b/packages/core/src/__tests__/postgres/workflow-reconciliation-production-shape.pg.test.ts index 90e3d904b8..0a14442d27 100644 --- a/packages/core/src/__tests__/postgres/workflow-reconciliation-production-shape.pg.test.ts +++ b/packages/core/src/__tests__/postgres/workflow-reconciliation-production-shape.pg.test.ts @@ -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 ──────────────────────────────────────────────