Skip to content

feat: handle login-token rate limit clearly in boxel browse - #6199

Open
FadhlanR wants to merge 2 commits into
mainfrom
login-rate-limit-429
Open

FadhlanR wants to merge 2 commits into
mainfrom
login-rate-limit-429

Conversation

@FadhlanR

@FadhlanR FadhlanR commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Background and Goal

Minting a Matrix login token for boxel browse is rate-limited to about once per minute per account — a hardcoded Synapse anti-abuse limit that can't be raised. Running browse twice within that window fell through to an opaque error:

Error: Matrix login-token request failed: 429 {"errcode":"M_LIMIT_EXCEEDED",...}

This replaces that with a typed error and a clear, actionable message, plus an opt-in wait/retry.

What changed

  • auth.ts — new MatrixRateLimitError carrying the server-reported wait; requestLoginToken throws it on a 429 / M_LIMIT_EXCEEDED instead of the generic failure. The wait comes from the body's retry_after_ms (deprecated in Matrix v1.10 but still sent by Synapse), falling back to the standard Retry-After header.
  • browse.ts — on a rate limit, surface a message naming the ~1/min limit and the wait. Auto-wait and retry once when the reported wait is short (≤5s) so genuine back-to-back opens just work; --wait blocks out a longer window — the reported wait, or a full ~1min window when none is reported. A reported wait beyond 90s is pathological for a 1/min limit and errors immediately: sleeping less than the server asked guarantees the retry fails, so no time is burned waiting. The message suggests --wait only when it would change the outcome. --print-url gets the same handling (message to stderr, non-zero exit, no URL on stdout).

Key decision

No tokenless "open anyway" fallback. The CLI can't read the browser's session or know which account it holds. Loading the host app without a fresh token would land the user signed-in only if the browser already has a live session for the same account — otherwise it silently switches to the wrong account or a login wall. Since that can't be verified, we never open on a rate limit; we either wait for a real token (correct account guaranteed) or surface the error. The existing MatrixAuthError re-auth path is untouched — a 429 deliberately does not trigger re-auth (a fresh token hits the same per-account limit).

Testing

  • packages/boxel-cli unit suite green (browse.test.ts: 46 tests). New cases cover the 429 with/without retry_after_ms, the Retry-After header fallback, the M_LIMIT_EXCEEDED classification, short-wait auto-retry, --wait (reported wait, no-reported-wait fallback, over-the-ceiling fail-fast), a persistent limit after retry, a rate limit surfaced by the post-re-auth mint, and --print-url. Lint + tsc --noEmit clean.
  • Pre-existing smoke.test.ts / deadline-ladder.test.ts failures are unrelated (they require a built dist/ via pnpm build).

🤖 Generated with Claude Code

FadhlanR and others added 2 commits September 18, 2026 13:51
Minting a Matrix login token is rate-limited to ~1/min per account
(a hardcoded Synapse anti-abuse limit). A second `browse` within the
window fell through to an opaque `429 M_LIMIT_EXCEEDED` error.

- auth.ts: add `MatrixRateLimitError` carrying the server-reported
  `retry_after_ms`; `requestLoginToken` now throws it on a 429 /
  `M_LIMIT_EXCEEDED` instead of the generic failure.
- browse.ts: surface a clear, actionable message naming the ~1/min
  limit and the wait. Auto-wait and retry once when the wait is short
  (<=5s); add `--wait` to block out a longer window (capped at 90s).
  Never fall back to a tokenless open — the CLI can't tell which
  account the browser holds, so it could land on the wrong one.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A reported wait beyond the 90s ceiling now errors immediately instead of
sleeping a capped 90s into a still-open window, where the retry was
guaranteed to hit the limit again. `--wait` with no reported wait now
blocks a full ~1min window (was 5s, which usually landed inside the
window and failed). The rate-limit message only suggests `--wait` when
it would change the outcome. requestLoginToken falls back to the
standard Retry-After header when the deprecated retry_after_ms body
field is absent.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@FadhlanR
FadhlanR marked this pull request as ready for review September 21, 2026 05:35
@FadhlanR
FadhlanR requested a review from a team September 21, 2026 05:35
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 21, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-21T05:39:39.096517Z cd0d995 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cd0d995960

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +198 to +202
let mintToken = async () => {
try {
return await requestLoginToken(pm.getStoredMatrixAuth(profileId));
} catch (err) {
if (!(err instanceof MatrixAuthError)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Share the re-authentication budget across mint retries

When the stored token is rejected, the re-authenticated mint is rate-limited, and the post-wait mint is also rejected for authentication, the second call to mintToken() receives a fresh re-authentication allowance and prompts for credentials again. This breaks the existing once-only recovery behavior and can make users re-enter their password twice during one browse invocation; track whether re-authentication has already occurred across both mint attempts.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant