(motir-core) `shared-store.test.ts`'s atomicity case cannot tell an ATOMIC store from one that FAILED OPEN — 12 concurrent requests, 2 hit the 250ms store deadline, all 12 served
Repo: motir-core. One PR. Surfaced by the motir run of MOTIR-2648 (2026-08-11) and logged rather than absorbed (notes.html #27) — 2648's own "Out of scope" section names this arm explicitly, "a different bug if it ever fires, and one that produces a byte-identical symptom, so keep them apart." It has now fired.
The failure
Vitest (3/3) on PR #2035, job 93795321890:
FAIL tests/api/v1/shared-store.test.ts > the shared counter stays ATOMIC and time-bounded
under the wrapper > fires N SIMULTANEOUS requests against a budget of N−1 and refuses exactly one
AssertionError: expected [ 200, 200, … ] to have a length of 11 but got 12
tests/api/v1/shared-store.test.ts:221
All twelve requests were served and not one was refused. The discriminator is two stderr lines the same log attributes to that exact test:
[api/v1] rate-limit store unavailable; allowing the request
RateLimitStoreTimeoutError: The rate-limit store did not answer within 250ms.
So 2 of the 12 concurrent increments blew the deadline, took the fail-open arm (degraded: true, lib/api/v1/rateLimit.ts:257), and were never counted. The store saw 10, the budget was 11, and every request came back 200.
Why this is NOT the MOTIR-2101 window-straddle class
It presents identically — all-served, zero 429 — and that is precisely the trap 2648 warned about. The tell is in the log, not the assertion:
| straddle | this | |
|---|---|---|
| cause | a grid boundary falls mid-test, counter resets | increments never reach the store |
| log | silent | RateLimitStoreTimeoutError per failed-open call |
| fix | pin + align the window | not a window problem at all |
shared-store.test.ts:215 already does the right thing on the window axis — budget(N - 1, ALIGNED_WINDOW_MS) then await waitForWindowBoundary(ALIGNED_WINDOW_MS). It is a correct member of the aligned set and stays one. Do not "fix" this by touching the window.
The real defect is that the assertion cannot see the difference
The test reads only r.status, so a served-because-atomic-limit-not-reached and a served-because-the-store-timed-out are the same value to it. The property under test — the counter is atomic under concurrency — is not actually asserted: a run in which the store answered nothing at all would pass the 429 count by accident just as easily as it failed here.
degraded already exists on the decision (lib/api/v1/rateLimit.ts:233) and is exactly the signal needed; it simply never reaches the test.
Acceptance criteria
- The atomicity case FAILS LOUDLY AND SPECIFICALLY when any request in the batch was served degraded — the message must say the store timed out, not "expected 11 got 12". Assert on the
degradeddecision (surface it to the test via a header, a spy, or a returned decision), not on status alone. - The case is made robust to a loaded runner rather than merely re-run: give the store deadline a test-time override (
DEFAULT_RATE_LIMIT_STORE_TIMEOUT_MSatlib/rateLimit/postgresStore.ts:30is a hardcoded const, andpostgresStorealready acceptsoptions.timeoutMsat:80— the seam exists) and pin it generously for this case, so a 250ms Postgres hiccup under CI load is not a red PR. - A test proves the new assertion FAILS against a deliberately-degraded store — inject a store that times out and watch the atomicity case go red with the degraded message. A guard nobody has watched fail is indistinguishable from no guard.
- The window alignment at
:215is UNCHANGED and the file still satisfies the derived guard inrate-limit-window-alignment.test.ts(MOTIR-2648).
Out of scope
Changing the fail-open POLICY. Serving a request when the limiter's store is unreachable is a deliberate, documented product decision (lib/rateLimit/limiter.ts: "A brief over-serve beats an outage caused by the thing meant to prevent one") and is not in question here — this card is about a TEST that cannot observe it. Whether 250ms is the right production deadline is likewise a separate question.
Context refs
tests/api/v1/shared-store.test.ts:211–223— the atomicity case.lib/rateLimit/postgresStore.ts:30(DEFAULT_RATE_LIMIT_STORE_TIMEOUT_MS = 250) ·:41/:80(options.timeoutMs, the override seam).lib/api/v1/rateLimit.ts:233/:257—degradedon the decision, and the fail-open return.lib/rateLimit/limiter.ts— the same arm on the shared path, with the policy rationale.- PR #2035 / job
93795321890— the occurrence.