ID-328 {328.3} TECH — cluster D: declare e2e data preconditions, close the teardown gaps

ID-328 {328.3} TECH — cluster D data preconditions

Date: 25/07/2026 (S492) Reads: {328.1} RESEARCH.md — findings, evidence and the four-spec disposition are there and are not restated. No PRODUCT.md: cluster D changes no user-facing behaviour, only how tests establish their preconditions. Scope: cluster D only. Cluster B and the PW_MAX_FAILURES cap are out.

---

1. Context

Three of the four cluster-D cases are already fixed on main ({135.32} retired bid-session.spec.ts:407; {128.23}/021dd78e fixed the change-reports hero copy and the two Skip → Next item label assertions). This spec covers what is left:

The residual defect. e2e/tests/governance-review.spec.ts asserts on a non-empty review queue in five tests, and its comments (:29, :38, :259) claim the worker fixture supplies it — but the file never destructures workerData, so Playwright never instantiates the fixture for that worker (fixtures are lazy; RESEARCH §“The latent defect”, proven empirically). The precondition is supplied by whichever other spec happened to run earlier on the same worker.

{128.23} raised the requirement from ≥1 to ≥2 queue items (governance-review.spec.ts:216-244 now asserts /^Review item 1 of / then /^Review item 2 of /). On a cold ephemeral branch a worker that has not instantiated workerData sees exactly 1 item — the seedPublicationReviewFixture row. That commit landed nine hours after the 25/07 nightly, so no run has observed it yet.

Relevant code:

• e2e/tests/governance-review.spec.ts — 11 of its 13 tests depend on a non-empty queue (enumerated in §2 M1); :156 is the only one needing ≥2 items.
• e2e/fixtures/test-data-fixture.ts:148 — the workerData worker fixture; seeds 12 source_documents (publication_status defaults 'published', verified_at NULL) → 12 queue items per instantiation.
• app/api/review/queue/route.ts:271-315 — default status='unverified': .neq('publication_status','draft') + .is('record_lifecycle.verified_at', null) + record_lifecycle!inner.
• app/review/review-content.tsx:397 — the queue’s own count, exposed as aria-label="Review queue — {n} items pending review". The observable the fix asserts on.
• e2e/global-teardown.ts:104-126 — the safety sweep. Covers q_a_pairs, source_documents, workspaces, notifications. No form_instances sweep — 381 orphan rows on staging (RESEARCH I-1).
• e2e/helpers/data-factory.ts:94-112 — createTestWorkspace, dead and broken (RESEARCH I-4).

---

2. Proposed changes

M1 — Declare the review-queue precondition (e2e/tests/governance-review.spec.ts)

Destructure workerData into every test that reads the queue. Eleven of the file’s thirteen tests need it — every test that resolves getByRole('toolbar', { name: 'Review actions' }), which only renders when queue.length > 0:

:28 shows the seeded review queue, :38 progress bar is displayed, :49 action bar shows verify, flag, next, exit, :76 verify button advances, :105 flag button shows flag input, :130 flag cancel hides the flag input, :156 next button advances (the only one needing ≥2), :190 back button is disabled on the first item, :206 exit button navigates away, :224 keyboard shortcut help dialog opens, :256 renders the seeded queue toolbar.

Only :20 (review page loads with heading) and :270 (accessible via navigation) are genuinely queue-independent and stay as they are. Each of the eleven becomes async ({ authenticatedPage: page, workerData }).

workerData is otherwise unused in the assertions — that is fine and is the point: the destructure is the dependency declaration. Add a void workerData; with a one-line comment naming why, matching the void prefix; convention already used at data-factory.ts:124, so a future lint pass does not “helpfully” remove it.

Why per-worker, not a seed:e2e-users baseline (resolves RESEARCH OQ-1). The {128.9} precedent (seedTaxonomyGovernanceFixture) covers reference data — singleton config rows with unique business keys that every worker needs identically. Review-queue content is not reference data; it is disposable corpus content the worker fixture already produces. Three concrete reasons the baseline route is worse:

1. It would give a floor of 2, sitting exactly on the assertion boundary. Any concurrent verify action drops it to 1 and the test fails.
2. Extending seedPublicationReviewFixture would couple governance-review.spec.ts to a row review-publication-tab.spec.ts mutates and resets in its own afterEach.
3. Baseline rows are owned by no worker, so a sibling worker’s teardown or the global sweep can reap them mid-read — the cross-shard race global-teardown.ts:22-26 already documents. A worker’s own fixture rows survive for that worker’s entire lifetime by construction.

workerData gives a hard floor of 12 owned rows, which is monotone-safe (RESEARCH §“Mutual satisfiability”: presence may be asserted at the write boundary as a lower bound).

M2 — Make the precondition fail loudly, not as a locator timeout

Before the position assertions in next button advances, assert the queue count directly off review-content.tsx:397:

CORRECTED S494 — the regex as first drafted here was wrong. It read /Review queue — \d+ items pending review/, but app/review/review-content.tsx:397 singularises at n=1 (${queue.length} ${queue.length === 1 ? 'item' : 'items'}). A 1-item queue therefore renders …— 1 item pending review, which that pattern does not match — so the exact case M2 exists to catch would have failed as “locator not found”, telling the next triager nothing. items? is required. As landed:

const queueRegion = page.getByRole('region', {
  name: /^Review queue — \d+ items? pending review$/,
});
await expect(queueRegion).toBeVisible({ timeout: 15000 });

const queueLength = Number(
  /(\d+)/.exec((await queueRegion.getAttribute('aria-label')) ?? '')?.[1] ?? 0,
);
expect(queueLength, `…needs >=2 — the queue has ${queueLength}…`).toBeGreaterThanOrEqual(2);

Asserting visibility alone was also insufficient: the region renders at any length, so the count must be parsed and compared for the failure message to name the length. (Note the path is app/review/review-content.tsx, not the bare review-content.tsx used above.)

(review-content.tsx:399 renders a <section aria-label=…>, which maps to ARIA role region — not main. Verified correct.)

then assert /^Review item 1 of / → click → /^Review item 2 of / as {128.23} wrote it. A run with a 1-item queue then fails on a message that names the queue length rather than on getByRole('article', { name: /^Review item 2 of / }) timing out — which reads as a navigation bug and would send the next triage session down the wrong path.

Also correct the stale file header (:8, RESEARCH I-5): it advertises “empty queue handling”; there is no empty-queue test. Replace with a note that queue emptiness is not assertable against a shared DB at the write boundary, citing the §3 invariant below.

M3 — form_instances safety sweep (e2e/global-teardown.ts)

Add a prefix sweep alongside the existing workspaces/notifications sweeps:

for (const prefix of E2E_CONTENT_PREFIXES) {
  await supabase.from('form_instances').delete().like('name', `${prefix}%`);
}

form_instances CASCADEs to form_questions → form_responses/form_response_history, so no dependency ordering is needed. Both existing prefixes ([E2E-, [E2E Test]) are covered by reusing E2E_CONTENT_PREFIXES rather than a new literal.

CORRECTED S494 — the constraint cited above was misnamed, and the cascade set is larger than stated. The inbound FK is on form_instance_id (supabase/migrations/20260712062000_id145_w1c_rename_reshape.sql:97), not form_questions_form_template_id_fkey — that name belongs to a different column and survives only because the RENAME COLUMN left the constraint name untouched (test-data-fixture.ts:859-865 repeats the same conflation). There is also a second inbound FK this spec did not list: form_attachments.form_instance_id (20260716113306_id147_form_attachments.sql:39). Both are ON DELETE CASCADE, so the conclusion — no ordering needed — still holds, for a broader reason than stated.

Predicate verified S494 (read-only, Platform staging). name LIKE '[E2E-%' matches exactly 381 of 396 rows, and [E2E Test]% matches 0 (the legacy prefix was never used on this table; it is retained for symmetry with the other sweeps). The 15 unmatched rows are all genuine app-created records (ingest_source app_upload/minted, plain human names), and no row carries a non-[E2E- bracket prefix — so the [S224-W4C-…] shape that escaped the S492 sweeps has no analogue here.

Correction to RESEARCH I-1’s “accumulating for a month”. All 381 orphans share one identical created_at (2026-06-25 17:16:25.502398+00), as do 8 of the 15 legitimate rows — a column-backfill stamp, not an insert time. Rows created after that date carry genuine varied timestamps, so the stamp marks everything that existed when the column was added. The correct reading is that this population predates 2026-06-25 and has not grown since: zero [E2E- orphans carry a post-backfill timestamp. The leak is real and worth closing, but it is a bounded historic population, not an actively growing one — and created_at must not be trusted as an age signal on rows older than the backfill.

This closes the future leak. The 381 existing orphans are a one-off cleanup — see §5.

M4 — Delete createTestWorkspace (e2e/helpers/data-factory.ts:94-112)

Zero callers (verified by grep across e2e/), and it inserts type: 'kb_section', a column dropped at S246 WP2b T2. It would throw Could not find the 'type' column of 'workspaces' for anyone who used it. Delete rather than repair: test-data-fixture.ts:447-461 already shows the correct application_type_id resolution pattern, and reviving a helper with no consumer just recreates the drift.

---

3. The invariant this establishes

State it once, in docs/reference/testing/testing-patterns.md, and cite it from the governance-review.spec.ts header:

Shared-DB precondition rule. An e2e assertion of presence may be enforced at the write boundary — seed it, and assert a lower bound (seeding is monotone under parallel workers). An assertion of absence must be enforced at the read boundary — page.route-intercept the endpoint, or narrow the query with a worker-unique predicate the UI genuinely sends. Absence can never be established by seeding, because any concurrent worker falsifies it.

A precondition a spec relies on must be declared — by destructuring the fixture that supplies it. A comment claiming a fixture ran is not a dependency; Playwright worker fixtures are lazy and are never set up unless requested.

change-reports-page.spec.ts is the reference implementation of the absence half (stubEmptyChangeReports, :52-88) and needs no change — RESEARCH confirms it was written correctly, and that the route mock is load-bearing rather than defensive, because fullyParallel: true lets its empty state test and its populated state serial block land on different shards concurrently.

---

4. Testing and validation

Behaviour invariants for this change, each with its verification:

#	Invariant	Verification
BI-1	governance-review.spec.ts instantiates the worker fixture	Run the file alone with --workers=1 and assert a [Worker 0] Seeded: … line appears in stdout. Today it does not (RESEARCH, proven) — this is the regression test for M1
BI-2	The queue has ≥2 items before the advance assertions	M2’s count assertion; fails with the queue length in the message
BI-3	The eleven queue tests pass against a cold DB	The only honest gate. See below
BI-4	No [E2E-*] form_instances rows survive a completed run	select count(*) from form_instances where name like '[E2E%' = 0 after a local run
BI-5	Deleting createTestWorkspace breaks nothing	bun lint + bunx tsc --noEmit; grep confirms zero call sites

BI-3 is the one that matters and the one the repo cannot currently run. Local and PR-smoke both target Platform staging, which carries 45 ambient unverified source_documents — enough to mask the defect permanently (RESEARCH §“What are we not thinking about” #1). Two options, cheapest first:

1. Targeted proof against a cold branch. Provision one throwaway branch via bun run scripts/e2e-ephemeral-branch.ts create --run-id=<id>, run only governance-review.spec.ts against it with --workers=1, confirm red before M1 and green after, then delete. This is the minimum credible evidence and costs one short-lived Micro compute. Recommended.
2. Simulated cold DB. Point the run at staging but pre-filter the queue to the worker prefix — not possible: /review exposes no prefix filter, which is precisely why mechanism (b) of the §3 rule is unavailable here.

Do not accept a green run against Platform staging as evidence for BI-3; it proves nothing about this defect.

Cross-check after landing: the next e2e-nightly run must show governance-review.spec.ts fully green and the [Worker n] Seeded: line present in the shard log for whichever shard owns it.

---

5. Risks and mitigations

Risk	Mitigation
M3’s sweep deletes a form_instances row a concurrent shard is mid-read on	Same class as the documented pub-review race (global-teardown.ts:22-26); global teardown runs once after all shards in a run. Real exposure is only the nightly’s 4 independent shard jobs, whose teardowns are unordered. Accepted — it is strictly better than the current unbounded leak, and the rows are per-worker-prefixed so a sweep only reaps its own run’s shape
The 381 existing orphans are deleted by a broad sweep that also hits something live	Do not fold the historic cleanup into M3. It is a separate, reviewed one-off, scoped by name like '[E2E-W%' and created_at < now() - interval '7 days', run with a select first. Recommend routing to {128.18} (RESEARCH OQ-2)
Orphaned record_embeddings stay unreachable (RESEARCH I-2)	Owned by id-364 (record_embeddings.owner_id referential integrity, S480) — 206 of 384 schema-wide, of which 66 are the e2e-attributable slice. Out of scope here; do not duplicate. id-328 only inherits the constraint that a seed shape must not add to the class
M1 makes governance-review.spec.ts slower (fixture seed on a worker that previously borrowed)	Real but small — the fixture is worker-scoped, so it is paid once per worker, and the worker almost always already pays it for another spec. No mitigation needed

---

6. Follow-ups

• Audit the other eighteen. Nineteen of 36 specs never destructure workerData (RESEARCH §“What are we not thinking about” #6). This spec proves the mechanism for one. The same audit — “does this spec assert on corpus-wide state without declaring the fixture that supplies it?” — has not been run across the rest. Worth its own subtask.
• RESEARCH OQ-3 (change-reports hero: leave the relaxed both-variants assertion, or pin isNewAccount and assert exact copy) and OQ-4 (appetite for a cold-DB pre-merge gate, or a lint rule that flags a spec whose comments claim the fixture seeded something but which does not destructure workerData) are open owner calls, deliberately not decided here.
• source_documents.content_owner_id is absent from staging (RESEARCH I-6) — schema parity, already in the {128.10} tail. Recorded so it is not re-diagnosed as cluster D.