Partial, deliberately. The design and plan for #241 plus its first task, merged so the branch does not sit unmerged while other work happens. #241 stays open for Tasks 2–4.
Everything here is additive and inert: two documents, one helper with tests, and nothing calls the helper yet.
docs/superpowers/plans/2026-08-30-e2e-isolation.md — four tasks
frontend/tests/e2e/support/findOrFail.ts and its five Vitest tests
What the investigation actually found
The problem is narrower than #241 first claimed, and two of my own earlier assertions are retracted in the spec rather than quietly dropped:
The verification-resend limiter is not a shared axis. It is keyed customer:${id} and the customer fixture creates a uniqueEmail() per test. Believing otherwise would have justified per-worker isolation to fix something that is not shared.
The suite has no snapshot-style assertions. It already uses web-first assertions throughout. My first draft of the spec assumed otherwise; I checked before committing to it.
What remains is two mechanisms. Unchecked lookups — collection.find(...) dereferenced immediately at nine sites — which is a defect regardless of concurrency, because a missing row dies with Cannot read properties of undefined naming test plumbing instead of saying what was wanted. And load-induced timing, which is the larger share of what has actually been observed.
The helper
Lives in its own module importing nothing, so Vitest covers it under environment: 'node' without dragging @playwright/test and a browser harness into the unit suite to test six lines of pure logic. support/api.ts re-exports it so specs still reach it through fixtures.ts.
Its message names both what was sought and how many rows were searched — "0 rows" means the fixture never landed, "37 rows" means it landed and the predicate is wrong. Different bugs, and the message should tell them apart without a re-run.
Verification
30 frontend unit tests pass (5 new, 25 existing). Lint unchanged at 2 pre-existing warnings in src/filters.ts. Build clean.
Partial, deliberately. The design and plan for #241 plus its first task, merged so the branch does not sit unmerged while other work happens. **#241 stays open** for Tasks 2–4.
Everything here is additive and inert: two documents, one helper with tests, and nothing calls the helper yet.
## What is in it
- `docs/superpowers/specs/2026-08-30-e2e-isolation-design.md`
- `docs/superpowers/plans/2026-08-30-e2e-isolation.md` — four tasks
- `frontend/tests/e2e/support/findOrFail.ts` and its five Vitest tests
## What the investigation actually found
The problem is narrower than #241 first claimed, and **two of my own earlier assertions are retracted in the spec** rather than quietly dropped:
- The verification-resend limiter is **not** a shared axis. It is keyed `customer:${id}` and the `customer` fixture creates a `uniqueEmail()` per test. Believing otherwise would have justified per-worker isolation to fix something that is not shared.
- The suite has **no** snapshot-style assertions. It already uses web-first assertions throughout. My first draft of the spec assumed otherwise; I checked before committing to it.
What remains is two mechanisms. **Unchecked lookups** — `collection.find(...)` dereferenced immediately at nine sites — which is a defect regardless of concurrency, because a missing row dies with `Cannot read properties of undefined` naming test plumbing instead of saying what was wanted. And **load-induced timing**, which is the larger share of what has actually been observed.
## The helper
Lives in its own module importing nothing, so Vitest covers it under `environment: 'node'` without dragging `@playwright/test` and a browser harness into the unit suite to test six lines of pure logic. `support/api.ts` re-exports it so specs still reach it through `fixtures.ts`.
Its message names both what was sought and how many rows were searched — "0 rows" means the fixture never landed, "37 rows" means it landed and the predicate is wrong. Different bugs, and the message should tell them apart without a re-run.
## Verification
30 frontend unit tests pass (5 new, 25 existing). Lint unchanged at 2 pre-existing warnings in `src/filters.ts`. Build clean.
## Still to do under #241
Tasks 2–4: replace the nine lookups, raise the `expect` timeout from Playwright's unset default of 5 s to 10 s, and restore the test #245 skipped.
Worker-count reduction stays deliberately out of scope, so the next CI runs show whether the defect fixes worked rather than being masked by it.
Ref #241
Five distinct specs failed across two runs of identical code with no overlap between the sets, so which test fails is decided by the scheduler. The cost is already being paid: #245 skipped the only end-to-end check that adding an item reaches the database, purely to get main green.
The design separates two mechanisms that had been treated as one. Unchecked lookups into shared collections — `collection.find(...)` dereferenced immediately, found at eight or more sites — are a defect regardless of concurrency: when the row is missing the test dies with "Cannot read properties of undefined" naming test plumbing rather than failing an assertion that says what it wanted. Load-induced timing is the other, and is the larger share of what has actually been observed: three of four local failures and the CI one are assertions in a spec's own browser context that nothing else can touch.
Two claims from earlier in this investigation are retracted in the document rather than quietly dropped. The verification-resend limiter is not a shared axis — it is keyed per customer and every test registers its own — and the suite contains no snapshot-style assertions, so there is nothing to convert to web-first. Both were stated as fact on the issue, and both would have justified work that was not needed.
Per-worker databases are ruled out structurally: every worker talks to one backend on :3000, so isolation there means N backends, not N databases. Worker-count reduction is deliberately deferred rather than taken now, because applying it at the same time would mask whether fixing the defects worked.
The honest limit is recorded too. CI's load cannot be reproduced here on demand, so the timing changes rest on reasoning rather than a red-to-green demonstration, and the success criterion is several consecutive green runs rather than one.
Ref #241
Four tasks. A pure findOrFail helper with Vitest coverage, the nine unchecked lookups converted to use it, the expect timeout raised from Playwright's unset default of 5s to 10s, and the test #245 skipped brought back.
The helper deliberately imports nothing and lives apart from support/api.ts. api.ts imports @playwright/test, and vitest.config.ts runs with environment: 'node' over tests/unit only — putting the helper there would drag a browser harness into the unit suite to test six lines of pure logic.
Two things the survey changed. filters.spec.ts:216 is excluded: its .find() searches CSS class names on a string array rather than test data, so it has no missing-row failure mode. And three `expect(row).toBeTruthy()` assertions are deleted rather than kept, because findOrFail has already thrown by then — leaving them would tell the next reader the value might be falsy, which is the confusion the change exists to remove.
Worker-count reduction is explicitly not in this plan. It is a real lever and may still be needed, but applying it at the same time would make it impossible to tell which change fixed anything.
The plan states the criterion it cannot check: CI's load is not reproducible here, so two consecutive green local runs mean the refactor is sound, not that the flakiness is gone. Several consecutive green CI runs are the real bar, and #241 stays open until then.
Ref #241
Nine sites across five specs do `collection.find(...)` and dereference the result immediately. When the row is missing the test dies with "Cannot read properties of undefined" naming a line of test plumbing, which says nothing about what was expected — and that is exactly how favorites-filter:169 failed without producing a usable signal.
The message names what was wanted and how many rows were searched. That distinction carries real diagnostic weight: "0 rows" means the fixture never landed, "37 rows" means it landed and the predicate is wrong, and those are different bugs to chase.
It lives in its own module importing nothing, rather than in support/api.ts. That file imports @playwright/test, and vitest.config.ts runs tests/unit with environment: 'node' — putting six lines of pure logic there would drag a browser harness into the unit suite to test them. api.ts re-exports it so specs still reach it through fixtures.
Throws rather than returning null, because every caller wants the row: an error at the point of the miss beats a null threaded through three more lines before something unrelated fails.
Frontend: 30 unit tests pass, lint unchanged at 2 pre-existing warnings, build clean.
Ref #241
bermudalamb
changed title from Fix/241 e2e isolation to test(e2e): design, plan, and the find-or-fail helper (#241)2026-08-31 14:37:15 -05:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Partial, deliberately. The design and plan for #241 plus its first task, merged so the branch does not sit unmerged while other work happens. #241 stays open for Tasks 2–4.
Everything here is additive and inert: two documents, one helper with tests, and nothing calls the helper yet.
What is in it
docs/superpowers/specs/2026-08-30-e2e-isolation-design.mddocs/superpowers/plans/2026-08-30-e2e-isolation.md— four tasksfrontend/tests/e2e/support/findOrFail.tsand its five Vitest testsWhat the investigation actually found
The problem is narrower than #241 first claimed, and two of my own earlier assertions are retracted in the spec rather than quietly dropped:
customer:${id}and thecustomerfixture creates auniqueEmail()per test. Believing otherwise would have justified per-worker isolation to fix something that is not shared.What remains is two mechanisms. Unchecked lookups —
collection.find(...)dereferenced immediately at nine sites — which is a defect regardless of concurrency, because a missing row dies withCannot read properties of undefinednaming test plumbing instead of saying what was wanted. And load-induced timing, which is the larger share of what has actually been observed.The helper
Lives in its own module importing nothing, so Vitest covers it under
environment: 'node'without dragging@playwright/testand a browser harness into the unit suite to test six lines of pure logic.support/api.tsre-exports it so specs still reach it throughfixtures.ts.Its message names both what was sought and how many rows were searched — "0 rows" means the fixture never landed, "37 rows" means it landed and the predicate is wrong. Different bugs, and the message should tell them apart without a re-run.
Verification
30 frontend unit tests pass (5 new, 25 existing). Lint unchanged at 2 pre-existing warnings in
src/filters.ts. Build clean.Still to do under #241
Tasks 2–4: replace the nine lookups, raise the
expecttimeout from Playwright's unset default of 5 s to 10 s, and restore the test #245 skipped.Worker-count reduction stays deliberately out of scope, so the next CI runs show whether the defect fixes worked rather than being masked by it.
Ref #241
Fix/241 e2e isolationto test(e2e): design, plan, and the find-or-fail helper (#241)