test(core): pin the search archive-lane WIRING — the predicate was covered, the hand-off was not (#3220)
## 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) <noreply@anthropic.com>
This commit is contained in:
154
packages/core/src/__tests__/search-archived-lane-wiring.test.ts
Normal file
154
packages/core/src/__tests__/search-archived-lane-wiring.test.ts
Normal file
@@ -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<Record<string, unknown>>()),
|
||||
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<string>()),
|
||||
} as unknown as TaskStore;
|
||||
}
|
||||
|
||||
/** The `archivedColumns` handed to the primary search path on the most recent call. */
|
||||
function archivedColumnsPassed(): ReadonlySet<string> | undefined {
|
||||
const calls = searchTsvectorMock.mock.calls as unknown[][];
|
||||
const call = calls.length > 0 ? calls[calls.length - 1] : undefined;
|
||||
return (call?.[2] as { archivedColumns?: ReadonlySet<string> } | 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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user