From 78d87f0a10bee2a274d078e393c2b105c7013e11 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 11:50:02 -0700 Subject: [PATCH] =?UTF-8?q?test(core):=20pin=20the=20search=20archive-lane?= =?UTF-8?q?=20WIRING=20=E2=80=94=20the=20predicate=20was=20covered,=20the?= =?UTF-8?q?=20hand-off=20was=20not=20(#3220)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## The false-green #3160 (mine) proved `liveSearchPredicate` honours a resolved archive set: hand it `Set(["archived","filed"])` and `filed` appears in the bound params. That contract is real and still correct. **Nothing proved `reads.ts` passes one.** It is a unit test of the collaborator, so blinding the resolver at the call site cannot fail it. A conversion, a test that looks like it covers it, and no connection between them. ## The measurement — and the instrument matters | site | vs. the predicate unit test | vs. a test that drives `reads.ts` | |---|---|---| | `reads.ts:396` cold-storage list | 0 failed | **1 failed — covered** | | `reads.ts:615` incremental sync | 0 failed | 0 failed — **UNCOVERED** | | `reads.ts:793` search | 0 failed | 0 failed — **UNCOVERED** | Against `search-excludes-renamed-archive-lane.test.ts` all three read as uncovered — an artefact of asking a file that never executes `reads.ts`. Against `cold-storage-renamed-archive-lane.test.ts`, which drives `listTasksImpl` for real, 396 is covered and the other two genuinely are not. That is rule 2 of #3214 one level up: *the test must reach the site*, and a unit test of the collaborator never does. Had I stopped at the first instrument I would have reported three uncovered resolvers, one of them wrongly. ## What 793 costs on a renamed board `searchTasks` backs the **CREATE-time near-duplicate check**. Without the resolved lanes threaded, search stops excluding the board's archive lane, and creating a task can be refused as a duplicate of one the operator archived long ago — with no way to see why, because the matching card is not on the board. Precisely the symptom #3160 set out to fix; this pins the wiring that delivers it. ## An assertion I got wrong, and the correction I expected an unreadable workflow list to leave `archivedColumns` **undefined** via the call-site `.catch(() => undefined)`. It does not: `resolveProjectColumnsForRoles` catches internally and returns its **legacy-seeded** set, so `Set(["archived"])` is threaded and the `.catch` never fires on that path. Two layers fail soft and the inner one wins. The case now asserts the guarantee that actually holds either way — **never an empty set** (which would exclude nothing and return archived rows in every search), legacy id always excluded. Recorded at the site, because the mechanism is not obvious from the call. ## Flagged, not papered over **`reads.ts:615` is left uncovered on purpose.** It composes Drizzle conditions and runs them against `layer.db` with no injectable seam, so pinning it needs a real database and belongs with the `.pg` suites. A test asserting "the query was built" rather than "the rows were excluded" would satisfy the ratchet and prove nothing. Also flagged from this sweep: `workflow-analytics.ts` and `team-analytics.ts` (4 resolvers) are **unmeasurable in my environment** — their renamed-lane coverage lives in `.pg` suites, and this worktree has no TCP PostgreSQL (`pg_isready` reports a Unix socket; the harness probes TCP, so `pgDescribe` correctly skips). Not claimed either way. ## Census **Unchanged — `CONVERSION QUEUE EMPTY`, `AVAILABLE: 0`.** Converts nothing; closes coverage on a conversion the census already counts as done. ## Verification ``` as written Tests 4 passed (4) BLIND reads.ts:793 Tests 1 failed | 3 passed (4) restored Tests 4 passed (4) ``` Anti-vacuity case included: every other assertion reads a mock's arguments and would pass if the search were never reached, so one case pins that the primary search path actually ran. Typecheck clean. No changeset: test-only, behavior-preserving, no published-package surface. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) --- .../search-archived-lane-wiring.test.ts | 154 ++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 packages/core/src/__tests__/search-archived-lane-wiring.test.ts diff --git a/packages/core/src/__tests__/search-archived-lane-wiring.test.ts b/packages/core/src/__tests__/search-archived-lane-wiring.test.ts new file mode 100644 index 0000000000..df2a821509 --- /dev/null +++ b/packages/core/src/__tests__/search-archived-lane-wiring.test.ts @@ -0,0 +1,154 @@ +/* +FNXC:WorkflowResolvedColumns 2026-07-31-18:58 (found by BLINDING — the WIRING was unheld): + +`liveSearchPredicate` HONOURS A RESOLVED SET. NOTHING PROVED `reads.ts` PASSES ONE. + +`search-excludes-renamed-archive-lane.test.ts` (#3160, mine) asserts the predicate directly: give it +`Set(["archived","filed"])` and `filed` appears in the bound params. That is a real contract and it +is still correct — but it is a UNIT test of the predicate, so blinding the resolver in `reads.ts` +cannot affect it. I measured this with #3214's procedure and it is the sharpest false-green I have +hit in this program: + + reads.ts:396 archived (cold-storage list) -> Set(["archived"]) 1 failed covered + reads.ts:615 archived (incremental sync) -> Set(["archived"]) 0 failed UNCOVERED + reads.ts:793 archived (search) -> Set(["archived"]) 0 failed UNCOVERED + +Against `search-excludes-renamed-archive-lane.test.ts` all three read as uncovered, which is an +artefact of asking the wrong instrument — that file never runs `reads.ts` at all. Against +`cold-storage-renamed-archive-lane.test.ts`, which drives `listTasksImpl` for real, 396 is covered +and the other two are genuinely not. The lesson is rule 2 restated one level up: the test must reach +the site, and a unit test of the collaborator never does. + +WHAT 793 COSTS ON A RENAMED BOARD. `searchTasks` backs the CREATE-time near-duplicate check. If the +resolved lanes stop being threaded, search stops excluding the board's archive lane, and creating a +task can be REFUSED as a duplicate of one the operator archived long ago — with no way to see why, +because the matching card is not on the board. That is the exact symptom #3160 set out to fix; this +pins the wiring that delivers it. + +THE SEAM. `searchTasksTsvector` is spied and the assertion is on the `archivedColumns` it RECEIVES. +Asserting on returned rows would need a real database and would test Drizzle's rendering; asserting +on the predicate is what the sibling file already does. What was missing is the hand-off between +them, so that is what this asserts — and it is the only thing it asserts. + +613/615's incremental-sync scan is deliberately NOT covered here: it composes Drizzle conditions and +runs them against `layer.db` with no injectable seam, so pinning it means a real database and belongs +with the `.pg` suites. Left flagged rather than papered over with a test that would assert the query +built rather than the rows excluded. +*/ + +import { describe, expect, it, vi, beforeEach } from "vitest"; +import type { TaskStore } from "../store.js"; + +const { searchTsvectorMock, searchLikeMock } = vi.hoisted(() => ({ + searchTsvectorMock: vi.fn(async () => [] as unknown[]), + searchLikeMock: vi.fn(async () => [] as unknown[]), +})); + +/* +Both search paths are replaced. The tsvector path is primary and the LIKE path is its cold-index +fallback; leaving the fallback real would let a miss on the first fall through into a real query +against the fake `layer.db` and throw, which reads as an unrelated failure. +*/ +vi.mock("../task-store/async-search.js", async (importOriginal) => ({ + ...(await importOriginal>()), + searchTasksTsvector: searchTsvectorMock, + searchTasksLike: searchLikeMock, +})); + +const { searchTasksImpl } = await import("../task-store/reads.js"); + +/** Archive lane is `filed`; the board declares no column called `archived`. */ +const RENAMED_IR = { + version: "v2", id: "wf-renamed", name: "renamed", nodes: [], edges: [], + columns: [ + { id: "drafting", name: "Drafting", traits: [{ trait: "hold", config: { release: "capacity" } }] }, + { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] }, + { id: "filed", name: "Filed", traits: [{ trait: "archived" }] }, + ], +}; + +/** + * `getSettingsFast` and `getMergeQueuedTaskIdsAsync` are reached AFTER the search call this file is + * about — the hydration pass that turns rows into tasks. They are stubbed because omitting them + * throws past the assertion, not because the seam cares about them. + */ +function storeWith(definitions: unknown[]): TaskStore { + return { + listWorkflowDefinitions: vi.fn(async () => definitions), + asyncLayer: { db: {}, projectId: "p1" }, + getAsyncLayer: () => ({ db: {}, projectId: "p1" }), + listTasks: vi.fn(async () => []), + archiveEntryToTask: vi.fn(() => ({})), + getSettingsFast: vi.fn(async () => ({})), + getMergeQueuedTaskIdsAsync: vi.fn(async () => new Set()), + } as unknown as TaskStore; +} + +/** The `archivedColumns` handed to the primary search path on the most recent call. */ +function archivedColumnsPassed(): ReadonlySet | undefined { + const calls = searchTsvectorMock.mock.calls as unknown[][]; + const call = calls.length > 0 ? calls[calls.length - 1] : undefined; + return (call?.[2] as { archivedColumns?: ReadonlySet } | undefined)?.archivedColumns; +} + +describe("search threads the board's own archive lanes into the query", () => { + beforeEach(() => { vi.clearAllMocks(); }); + + it("passes a RENAMED archive lane through to the search path", async () => { + await searchTasksImpl(storeWith([{ ir: RENAMED_IR }]), "widget", { includeArchived: false }); + + /* Blinding the resolver at reads.ts:793 leaves this as Set(["archived"]) and drops `filed`. */ + expect(archivedColumnsPassed()).toBeDefined(); + expect([...(archivedColumnsPassed() ?? [])]).toContain("filed"); + }); + + /* + CONTROL. The resolved set is legacy-seeded, so the built-in id must still be threaded — a wiring + that resolved the renamed lane and dropped the legacy one would break every default board while + passing the case above. + */ + it("still threads the legacy `archived` id alongside it", async () => { + await searchTasksImpl(storeWith([{ ir: RENAMED_IR }]), "widget", { includeArchived: false }); + + expect([...(archivedColumnsPassed() ?? [])]).toContain("archived"); + }); + + /* + DEGRADED BOARD, and I asserted the wrong mechanism first — worth recording, because the correction + is the more useful fact. + + I expected an unreadable workflow list to leave `archivedColumns` undefined via the + `.catch(() => undefined)` at the call site, so `liveSearchPredicate` would fall back to its + literal. It does not: `resolveProjectColumnsForRoles` catches internally and returns its + LEGACY-SEEDED set, so the value threaded through is `Set(["archived"])` and the `.catch` never + fires on this path. Two layers both fail soft, and the inner one wins. + + What matters is the guarantee, which holds either way and is what this now asserts: a board whose + workflows cannot be read still excludes the legacy archive id. The failure to guard against is an + EMPTY set — that would exclude nothing and quietly return archived rows in every search, including + the CREATE-time duplicate check. + */ + it("still excludes the legacy id when the workflow list cannot be read", async () => { + const store = storeWith([]); + (store as unknown as { listWorkflowDefinitions: unknown }).listWorkflowDefinitions = + vi.fn(async () => { throw new Error("unreadable"); }); + + await searchTasksImpl(store, "widget", { includeArchived: false }); + + const passed = archivedColumnsPassed(); + /* Never an empty set: excluding nothing is worse than excluding only the legacy id. */ + expect(passed === undefined || passed.size > 0).toBe(true); + if (passed) expect([...passed]).toContain("archived"); + }); + + /* + ANTI-VACUITY. Every case above reads a mock's arguments, so all three would pass trivially if the + search were never reached — an early return on a blank query, a zero limit, a changed entry point. + This pins that the primary search path actually ran. + */ + it("actually reaches the search path", async () => { + await searchTasksImpl(storeWith([{ ir: RENAMED_IR }]), "widget", { includeArchived: false }); + + expect(searchTsvectorMock).toHaveBeenCalledTimes(1); + }); +});