Skip to content

moooon

Motir

Vibe your whole project. Bring an idea — Motir's three AI layers plan it, track it, and ship it, end to end. You're looking at Motir, built in Motir.

  • Vibe Project
  • Open Source
  • AI Agent
  • AI Loop
1
requests
0
upvotes
145
planned
1,361
shipped

Motir · Work items

MOTIR-2658Done

(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:

straddlethis
causea grid boundary falls mid-test, counter resetsincrements never reach the store
logsilentRateLimitStoreTimeoutError per failed-open call
fixpin + align the windownot 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

  1. 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 degraded decision (surface it to the test via a header, a spy, or a returned decision), not on status alone.
  2. 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_MS at lib/rateLimit/postgresStore.ts:30 is a hardcoded const, and postgresStore already accepts options.timeoutMs at :80 — the seam exists) and pin it generously for this case, so a 250ms Postgres hiccup under CI load is not a red PR.
  3. 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.
  4. The window alignment at :215 is UNCHANGED and the file still satisfies the derived guard in rate-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 / :257degraded on 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.