Motir

Type · test · Parent · MOTIR-3413 (discovery epic; no dependency edge into the finding card, so the epic is the container per log-bug.md's edge test) · Discovered in · MOTIR-3692, run 33075123375 · Resolution · open

tests/entitlementsService.test.ts > entitlementsService — work-item cap (§4.1) > serializes concurrent creates at the boundary via FOR UPDATE (no overage) has now failed on three unrelated pull requests, always on shard Vitest (3/3), always byte-identical:

AssertionError: expected [ { status: 'fulfilled', …(1) }, …(1) ] to have a length of 1 but got 2
 ❯ tests/entitlementsService.test.ts:241:23

None of the three diffs can reach the code under test. #2358's is the cleanest case: six files, every one of them under tests/e2e/ — the directory settles it without a reachability walk. Every other check on that commit was green, including Playwright E2E at-scale (billing-cloud).

The defect — VERIFIED by reading the test, not inferred from the failure

Read tests/entitlementsService.test.ts:201-247. The test seeds 249 items against a cap of 250, mints two positions, then races two withWorkspaceContext transactions that each assertWithinWorkItemCap and create. It then asserts, IN THIS ORDER:

expect(fulfilled).toHaveLength(1);                 // ← line 241, the one that fails
expect(rejected).toHaveLength(1);
expect((rejected[0] as PromiseRejectedResult).reason).toBeInstanceOf(EntitlementExceededError);

const finalCount = await adminDb.workItem.count({ where: { projectId: fx.projectId } });
expect(finalCount).toBe(250);                      // ← never reached on a red run

Two very different worlds produce fulfilled.length === 2, and the test cannot tell them apart:

• (a) The FOR UPDATE did not serialize — a REAL product defect, a 251-item overage. finalCount would be 251.
• (b) seedWorkItems(fx, 249) did not land 249 rows — a fixture problem, both creates legitimately under the cap. finalCount would be 250, i.e. the value the test calls correct.

finalCount is exactly the discriminator, and it is measured AFTER the assertion that throws, so it is never taken. The precondition is never asserted either: nothing checks the row count between the seed and the race. So the failure output states the one fact common to both worlds and discards both facts that separate them.

That is the bug, and it is independent of which world we are actually in. A test whose red cannot distinguish "a paying customer can exceed their cap" from "the fixture was slow" is not a usable signal for either — and it has now cost three CI cycles being re-classified by hand from an external argument ("the diff cannot reach it") rather than from its own output.

Fix direction

Make the failure name its world. Cheapest form:

1. Assert the precondition before the race — expect(await adminDb.workItem.count({ where: { projectId: fx.projectId } })).toBe(249). World (b) then fails HERE, naming the count it actually got.
2. Take finalCount before the fulfilled/rejected assertions and include it in the message (or assert it first). World (a) then fails naming 251.
3. Only then assert the 1-fulfilled / 1-rejected split.

The rewrite must stay a REAL concurrency test — do not replace the race with a sequential pair, and do not widen a timeout to chase it. If step 1 turns out to fail on a loaded shard, the fixture is the cause and the seed needs its own fix; if step 2 reports 251, this is a product defect on the cap and is its own highest card.

Acceptance criteria

• tests/entitlementsService.test.ts:201 asserts the seeded row count is 249 BEFORE the race, so an under-delivering fixture fails at that line naming the observed count.
• The post-race row count is captured before the fulfilled/rejected split is asserted and appears in the failure output, so a red run reports 250 (fixture short) or 251 (lock did not serialize).
• The test still exercises real concurrency — two withWorkspaceContext transactions raced with Promise.allSettled — and the assertions still FAIL when the serialization is genuinely removed. Exercise that direction (drop the FOR UPDATE locally, or race two creates with no cap assert) and quote the output in the pull-request body.
• pnpm vitest run tests/entitlementsService.test.ts passes locally on the changed file, and the pull request's Vitest shard is green.

Context refs

• tests/entitlementsService.test.ts:201-247 — the race test and its assertion order
• lib/services/entitlementsService.ts — assertWithinWorkItemCap, the FOR UPDATE under test
• lib/repositories/workItemRepository.ts — create, the second half of the raced transaction
• Runs 32628202745, 32999646685, 33075123375, all shard Vitest (3/3)