fix(cursor): prefer enterprise auth and handle missing limits#261
fix(cursor): prefer enterprise auth and handle missing limits#261drewwells wants to merge 3 commits intorobinebers:mainfrom
Conversation
This change fixes a multi-account Cursor edge case by preferring
keychain auth when SQLite indicates a free account and the token
subjects differ, ensuring enterprise usage is attributed to the
correct account. It also handles percent-only usage payloads when
planUsage.limit is missing, avoiding false errors while still
rendering valid usage lines.
Regression tests were added for both auth-source selection and
missing-limit percent usage.
drewwells
requested review from
davidarny,
robinebers and
validatedev
as code owners
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="plugins/cursor/plugin.js">
<violation number="1" location="plugins/cursor/plugin.js:505">
P2: Percent-only usage can still fail when plan info is unavailable because the new fallback triggers on missing limit before percent-based rendering.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="plugins/cursor/plugin.js">
<violation number="1" location="plugins/cursor/plugin.js:506">
P2: Added `!hasTotalUsagePercent` can skip unknown-plan REST fallback and cause hard failure for team-inferred accounts missing `planUsage.limit`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
@codex review |
![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: cbb7b08204
ℹ️ 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".
| if ((keychainAccessToken || keychainRefreshToken) && sqliteLooksFree && hasDifferentSubjects) { | ||
| ctx.host.log.info("sqlite auth looks free and differs from keychain account; preferring keychain token") | ||
| return { | ||
| accessToken: keychainAccessToken, | ||
| refreshToken: keychainRefreshToken, |
There was a problem hiding this comment.
In loadAuthState, this branch prefers keychain auth whenever the subjects differ and SQLite looks free, but it does not verify that the keychain credentials are still usable. If the keychain access token is expired and its refresh token is missing/invalid (while SQLite still has a valid token), probe will now fail with auth errors instead of using the working SQLite session, which is a regression introduced by this new preference logic.
Useful? React with 👍 / 👎.
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
robinebers
left a comment
robinebers
left a comment
There was a problem hiding this comment.
Thanks for the update — I found one remaining edge case to fix before approval.
In plugins/cursor/plugin.js, when:
planUsage.limitis missing,planUsage.totalPercentUsedexists,- plan info is unavailable,
- and the account is team-inferred from
spendLimitUsage,
the current flow can skip request-based fallback and then throw Total usage limit missing from API response.
Can you update the fallback logic so this path still uses request-based fallback (or otherwise avoids hard failure), and add a regression test for this exact scenario?
Please re-request review after that.
|
Your comment doesn’t make any sense. Is this a chat bot you are working on? The ChatGPT bot wants me to prefer a free token even when an account token is found. If their credentials are expired, they will refresh them. We don’t want to show a free account in that scenario. |
…nUsage.limit When an account is team-inferred via spendLimitUsage but planUsage.limit is missing and plan info is unavailable, the previous code threw a hard failure. Now falls back to the REST usage API instead. Made-with: Cursor
|
Fed the pr comments into a cursor and let it make changes. The cursor IDE will use those embedded sqlite credentials if the keychain ones disappear. I didn't add verifying creds, that should probably be handled by shared logic elsewhere. I went to see if the crusor ide will actually use those sqlite creds, it does in fact fallback to the embedded sqlite creds if keychain is invalid. I imagine that's temporary while they move everybody to keychain. Enjoy the unencrypted credentials until then 🙃 |
3 participants
Description
Related Issue
Type of Change
Testing
bun run buildand it succeededbun run testand all tests passbun tauri devScreenshots
Checklist
mainbranchSummary by cubic
Fixes a multi-account auth edge case by preferring keychain tokens when SQLite looks free and JWT subjects differ, so enterprise usage is attributed to the right account. Also handles Connect usage responses that omit planUsage.limit by using percent-only data when available and falling back to request-based metrics when needed, including team-inferred accounts.
Written for commit ad4ed73. Summary will update on new commits.