Repository navigation
Hold a rate-limited refresh, explain a blocked device login, and read invalid_grant by its code - #854
Conversation
… token-error-retry-after)
The hold was written for an agent's mint, and the next change has a refresh keep one too: a 429 on refresh is held on the login exactly as a 429 on a mint is. The type, its status report, and the file are renamed for what they now cover. The stored key stays "mint_hold", so a hold an earlier version wrote is still read, and one this version writes is still read by an earlier one. No behavior changes here.
… read invalid_grant by its code A refresh answered 429 was reported and forgotten, so the next command sent it again. A scheduled job running every two minutes into Basecamp's abuse block — which answers every OAuth request from the address for up to a day once enough refreshes have failed — sent one doomed refresh per run, for hours. The 429 is now held on the stored login until its Retry-After, the same hold an agent's mint already keeps, so every later process answers it locally. It is capped at MaxServerWait like the agent's; a request inside the block is answered without being counted against the address, so the cap costs one request and notices a block lifted early. A hold names the refresh token it was given for, so a login another process has rotated, or a fresh one, is not held; a successful refresh clears it, and `auth status` reports it. A device login refused 429 said "device authorization failed with status 429". It now says Basecamp is refusing sign-ins from this address, until when on the reader's clock, and that an old copy of a login elsewhere is what keeps the block going. invalid_grant is read off the SDK's typed refusal (OAuthError) instead of the text of its message, so deleting a stored login no longer hangs on how an error is worded.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
github-actions
Bot
added
commands
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f267ed4cb6
ℹ️ 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".
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 14 files
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
…for its review fixes
…sks a credential that cannot refresh
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eaa48a2af2
ℹ️ 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".
…ure is masked by it
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Review summary at
|
#854 renamed MintHold and its helpers to RenewalHold after this branch was cut, so the session mint and its tests named things that no longer exist.
What
Retry-After, using the same hold an agent's mint already keeps. Every later process (another shell, the next run of a scheduled job) answers it locally, without a request. The hold:MaxServerWait, like the agent's;auth statusunderrenewal_refused.device authorization failed with status 429, it says: "Basecamp is refusing sign-ins from this address for now; try again after Oct 6 4:40 PM PDT (in 2h)". The hint says that an old copy of a login somewhere else keeps the block going.invalid_grantis read from the SDK's typed refusal (OAuthError,OAuthErrorDescription), not by matching"token error: invalid_grant"in the message. Deleting a stored login no longer depends on how an error is worded.MintHoldis nowRenewalHold(in its own commit, so it can be skipped in review). The stored key staysmint_hold, so old and new versions read each other's holds.Why
A user locked themselves out. Two containers held copies of one login, and the second refresh revoked it. A scheduled job then retried every two minutes, and bc3's abuse tracker escalated to a 4-hour block on every OAuth endpoint for
basecamp-clifrom that address. That included device authorization, so they couldn't sign in again either, and all they saw was "status 429". 0.12.0 already forgets a login after its firstinvalid_grant. This covers the rest:A request made while a block is active is answered 429 without being counted against the address. So the 30-minute cap costs one request per cap, it never escalates the block, and the CLI notices if the block lifts early.
Depends on
basecamp/basecamp-sdk#972, pinned here at
db402d93(its last code change; later commits there touch only tests) withmake bump-sdk REF=db402d937b7482fd6894249613c713a1ca89edd6. That SDK PR is itself stacked on basecamp/basecamp-sdk#971, so the pin is an unreleased pseudo-version until both merge, the same way the pin was held during the event-feed work. The vendored MCP model was re-synced from that commit (provenance only, no model change).Testing
New tests in
internal/auth/refusal_test.go. Each failed before the change:TestInvalidGrant_ReadsTheTypedCode: the typed code is detected, and text that only looks like it isn't.TestRefresh_RateLimitIsHeldOnTheLogin: one request; then a held answer with the remaining wait and nothing sent; then a fresh request once the wait is over.TestRefresh_RateLimitHoldIsCappedButSaysWhatTheServerAsked: a 4-hourRetry-Afteris held for 30 minutes, and the message gives the server's own deadline.TestRefreshRefusal_ReportsTheHoldandTestRefresh_SuccessClearsTheHold.TestRefresh_HoldIsForTheRefusedToken: guard only. It passed before as well, because nothing was held then.TestLoginDevice_RateLimitSaysUntilWhenandTestLoginDevice_RateLimitWithoutRetryAfter.make checkpassesSummary by cubic
Holds a rate-limited refresh on the stored login until its
Retry-After, so every later process — another shell, the next run of a scheduled job — answers it locally instead of resending a doomed request. This stops a scheduled job with a rotated-away login from hammering Basecamp's abuse block, which can answer every OAuth request from an address for up to a day.Also makes two related failures readable: a device login refused with a 429 now says "Basecamp is refusing sign-ins from this address for now; try again after …" instead of "device authorization failed with status 429", and
invalid_grantis read off the SDK's typed refusal rather than matched in the error message's wording.Changes
MaxServerWait, named to the refresh token they were set for, cleared by a successful refresh, and reported inauth statusunderrenewal_refused.MintHoldis renamedRenewalHold; the stored key staysmint_holdso old and new versions read each other's holds.basecamp/basecamp-sdk#972).Written for commit 7d1cc71. Summary will update on new commits.