fix(api): report remaining time in Retry-After - #1603

QA Review: fix/1498-retry-after

Scope: 6 files, +59/−11 · reviewed: 25967370
Risk: LOW
Why this exists: a 429 from the chat, tool, or MCP limiter told clients to wait the full 60 s window even when only a few seconds remained, so they backed off too long and every rejected caller retried at the same moment (#1498).

Critical (must fix)

None.

Fix in-PR (apply mechanically — no adjudication essay expected)

• r1f1 [src/api/rate-limiter.ts:15] windowSeconds has no caller in src/ any more. Its only user was the middleware's old Retry-After, and now only its own test (test/unit/rate-limiter.test.ts:49, "exposes windowSeconds") reads it. Delete the getter and that test.
• r1f2 [docs/src/content/docs/config/environment.mdx:43] "that identity's fixed one-minute window" is wrong for /mcp. That limiter keys on identity and grant kind (src/api/routes/mcp.ts:124), so one identity can hold two windows. Say "that caller's" instead.

Suggestions (optional)

• r1s1 [src/api/routes/hooks.ts:216] The webhook receiver's 429 still sends a fixed Retry-After: 60, which is the same issue on a different surface. It is outside Rate limiter's Retry-After is always the full window, not the time left #1498's charter, and consume is deliberately left unchanged, so this is not filed. If you are already in that file, the same consumeWithRetryAfter swap applies.

What Looks Good

• The remaining time is computed from the same Date.now() reading that rejects the request, so the header and the window cannot disagree. A rejection always yields ≥ 1, because now - windowStart < windowMs on that branch.
• consume() delegates to the new method, so the webhook callers keep their exact semantics without a second copy of the window logic.
• Tests are deterministic (Date.now spy). A mutation check confirmed they bite: restoring String(limiter.windowSeconds) in the middleware fails "returns only the remaining seconds in Retry-After".
• The class comment now says fixed window, which matches the code.

Body claims

• ✅ "17 passed" for the three named test files: reproduced locally, 17 pass / 0 fail.
• ✅ consume(key): boolean is unchanged for webhook callers (hooks.ts:184, :203).
• ✅ The tests cover halfway, 1 ms before reset, and admission at reset (rate-limiter.test.ts). The middleware test asserts the emitted header.
• ℹ️ CI has not run: both workflows show action_required (a fork PR waiting for maintainer approval). The full suite on Linux is still unverified.

Next step for the author: answer these by id (the pr-adjudicate flow). Applying r1f1/r1f2 with one line each is enough.

{
  "kind": "review", "pr": 1603, "round": 1,
  "reviewed": "25967370b4a2866964577fb76004b2bbc19abd5b", "delta_base": null,
  "risk": "LOW", "stop_gate": "n/a",
  "verdict": "Correct core; two mechanical cleanups; CI awaiting fork approval",
  "findings": [
    {"id": "r1f1", "bucket": "fix-in-pr", "file": "src/api/rate-limiter.ts", "line": 15,
     "claim": "windowSeconds getter has no src caller after this change; only its own test reads it",
     "evidence": "reproduced", "receipt": "grep -rn windowSeconds src/ -> only the definition",
     "preconditions": null, "fix": "delete getter and the 'exposes windowSeconds' test", "scope": "core"},
    {"id": "r1f2", "bucket": "fix-in-pr", "file": "docs/src/content/docs/config/environment.mdx", "line": 43,
     "claim": "doc says the window is per identity, but the MCP limiter keys on identity plus grant kind",
     "evidence": "reproduced", "receipt": "src/api/routes/mcp.ts:124 keyOf = `${identity.id}:${grant.kind}`",
     "preconditions": null, "fix": "say \"that caller's\" window", "scope": "peripheral"},
    {"id": "r1s1", "bucket": "suggestion", "file": "src/api/routes/hooks.ts", "line": 216,
     "claim": "webhook 429 still sends a fixed Retry-After: 60",
     "evidence": "reproduced", "receipt": "sed -n 214,217p src/api/routes/hooks.ts",
     "preconditions": null, "fix": "use consumeWithRetryAfter in the hooks route", "scope": "peripheral"}
  ]
}

Adjudication: round 1

• r1f1: applied, 37458c75. Removed the windowSeconds getter and its test.
• r1f2: applied, 37458c75. The doc now says "that caller's fixed one-minute window".
• r1s1: filed as Webhook 429s always send Retry-After: 60, not the time left #1649.

The three rate-limit test files pass (16, after dropping the windowSeconds test). tsc --noEmit and biome are clean.

{"kind": "adjudication", "pr": 1603, "round": 1, "fixed_at": "37458c75923c9c00a86710e6a2c41053d474e58d",
 "findings": [
  {"id": "r1f1", "verdict": "accepted", "fixed_at": "37458c75"},
  {"id": "r1f2", "verdict": "accepted", "fixed_at": "37458c75"},
  {"id": "r1s1", "verdict": "split", "issue": 1649}
 ]}

Thanks for the contribution @Coding-Professional

Follow up issue if you want to grab that too: #1649