Document in-memory rate limiter tradeoff in docs/accepted-risks.md - #624
Conversation
Add entry describing that rate limits use a module-level in-memory Map with fixed-window tracking: limits reset on process restart and are not shared across replicas, with burst traffic possible at window boundaries. Noted as acceptable for single-node internal ops tool deployment. Fixes #622 Signed-off-by: Saffron <263493777+itsmiso-ai@users.noreply.github.com>
There was a problem hiding this comment.
AI Automated Review
Full PR review.
Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic) — escalated (fast_low_confidence)
Recommendation
Approve. This PR adds docs/accepted-risks.md documenting the known tradeoff of the in-memory fixed-window rate limiter. It is a pure documentation addition with no code changes, no risk flags, and directly satisfies linked issue PR 622.
Change-by-Change Findings
docs/accepted-risks.md (new file, 9 lines)
The file documents three verified points about the rate limiter:
-
"module-level in-memory
Mapwith fixed-window tracking; limits reset on process restart and are not shared across replicas." — Verified againstsrc/lib/rate-limit.ts: the module declaresconst windows = new Map<string, WindowEntry>();(line 44 in evidence) with aresetAt-based fixed window. Limits do reset on restart and are not distributed. -
"The fixed-window check allows burst traffic at window boundaries." — Verified: when
now >= entry.resetAt, the code opens a fresh window (windows.set(key, { count: 1, resetAt: now + windowMs })), so all requests immediately after a boundary are allowed up to the limit again. This is the classic fixed-window boundary burst problem noted in the issue evidence (line 56). -
"Mitigation: acceptable for the current single-node internal ops tool deployment; move to a shared store (e.g., Redis) if the app is ever scaled horizontally or requires persistent limits across restarts." — Consistent with the module-level JSDoc comment in
rate-limit.tswhich states "Dispatch runs as a single-node internal ops tool" and with AGENTS.md's description of Dispatch as self-hosted.
The documentation is accurate, concise, and appropriately scoped. The file format (title, last-updated date, heading per risk, bullet points, mitigation) is sensible for an accepted-risks ledger.
Standards Compliance
- AGENTS.md conventions: No code standards are violated — this is a docs-only change. The
docs/path is a conventional location for project documentation. - No agent-specific names in generic docs: The file uses generic language ("mutating endpoints", "shared store", "Redis") with no specific agent names. Compliant.
Linked Issue Fit
Issue PR 622 acceptance criteria are fully satisfied:
| Acceptance criterion | Verification |
|---|---|
"Mention the single-node, in-memory rate-limiter tradeoff in docs/accepted-risks.md" |
✅ File created with a dedicated section "In-Memory Rate Limiting Is Per-Instance" |
Evidence: line 44 (const windows = new Map) |
✅ Verified present in src/lib/rate-limit.ts |
| Evidence: line 56 (fixed-window check) | ✅ Verified: burst behavior occurs at window reset boundary |
The issue's priority is p3/chore — appropriate for a single-file documentation PR with no code changes.
Tool Harness Findings
git_log: Commit91e58cd"Document in-memory rate limiter tradeoff in docs/accepted-risks.md" is present onforeman/wl-misospace-dispatch-622/issue-622.read_file(rate-limit.ts): Confirmed the in-memoryMap, fixed-window logic, and boundary burst behavior match the documented claims.
Unknowns or Needs Verification
No unknowns. The documented claims are verifiable from the source file, and the PR scope (docs-only) carries minimal risk. All CI checks passed (Docker Build, Lint, Typecheck, Tests, Build).
What
Adds
docs/accepted-risks.mddocumenting that rate limits on mutating endpoints (src/lib/rate-limit.ts) use a module-level in-memoryMapwith fixed-window tracking, noting that limits reset on process restart, are not shared across replicas, and allow burst traffi…Fixes #622
Opened by foreman on review GO (workload wl-misospace-dispatch-622).