fix(kimi): recover monthly Total usage from Desktop Local Storage - #3537
Open
OttoPrua wants to merge 1 commit into
Open
fix(kimi): recover monthly Total usage from Desktop Local Storage#3537OttoPrua wants to merge 1 commit into
OttoPrua wants to merge 1 commit into
Conversation
Current Desktop no longer writes kimi-auth cookies, so an exhausted membership pool stayed hidden behind unused Code rate-limit windows. Co-authored-by: Cursor <cursoragent@cursor.com>
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
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. |
clawsweeper
Bot
added
P2
|
Codex review: needs real behavior proof before merge. Reviewed September 10, 2026, 5:58 AM ET / 09:58 UTC. ClawSweeper reviewWhat this changesThe PR adds Kimi Desktop Local Storage token discovery and makes the automatic menu-bar indicator prioritize exhausted monthly usage, with tests and documentation. Merge readiness⛔ Blocked before merge - 8 items remain This remains useful work for the linked issue: neither current main nor v0.58.0 resolves both reported gaps. The patch has a credential-selection defect, a failing test fixture, and no after-fix runtime proof. Priority: P2 Review scores
Verification
How this fits togetherCodexBar’s Kimi provider combines Code usage with optional membership statistics fetched using a local web session. The resulting quota windows feed the menu-bar usage indicator. flowchart TD
A[Desktop session stores] --> B[Web token selection]
B --> C[Kimi membership requests]
D[Code API usage] --> E[Combined quota snapshot]
C --> E
E --> F[Automatic menu-bar indicator]
Decision needed
Why: VISION.md explicitly requires sign-off for provider-authentication and privacy behavior changes; the existing cookie reader does not establish approval for the new storage scope. Before merge
Findings
Agent review detailsSecurityNeeds attention: The new historical-token fallback can bypass local session deletion; no dependency, workflow, or supply-chain changes were introduced. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Use approved, read-only current-session discovery that honors deletions and existing credential controls, while retaining the focused exhausted-month presentation change. Do we have a high-confidence way to reproduce the issue? Yes, from source: current main ignores exhausted monthly extra windows, and its Desktop loader cannot read Local Storage. The introduced tombstone bypass and test-fixture mismatch are also source-traceable; no tests or live probes were run. Is this the best way to solve the issue? No, not as submitted: extending the existing owners is appropriate, but raw historical token scanning defeats current-session filtering and the new extraction test cannot pass. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 7fdc17636f16. LabelsLabel changes:
Label justifications:
EvidenceSecurity concerns:
What I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
![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: a7afc05e59
ℹ️ 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
+197
to
+201
| if let audience = claims["aud"] as? String { | ||
| return audience.contains("kimi.com") | ||
| } | ||
| if let audience = claims["aud"] as? [String] { | ||
| return audience.contains(where: { $0.contains("kimi.com") }) |
There was a problem hiding this comment.
When the raw LevelDB fallback returns an unrelated access JWT whose audience is, for example, notkimi.com or kimi.com.example, this substring check accepts it as a Kimi token. If it has the latest expiry, selectAccessToken then prefers it over the genuine token and sends it to Kimi in both the Authorization and Cookie headers, breaking session recovery and potentially disclosing an unrelated bearer token. Validate the audience as an exact Kimi host/domain rather than using contains for both scalar and array audiences.
Useful? React with 👍 / 👎.
steipete
added a commit
that referenced
this pull request
Sep 10, 2026
Prioritize the known exhausted kimi-monthly pool in automatic menu-bar usage and highest-usage ranking, preserving explicit window choices and normal Code-window fallback. Extracts the display correction from #3537 and partially addresses #3536; authentication/storage work remains separate. Includes regressions, isolated test fixture cleanup, signed synthetic native before/after proof, provider docs and changelog. Validation: 189 focused tests, full 1,061-selection suite, lint and independent P0–P2 review passed. Signed native baseline/final tests verify both status-image percentages in light and dark appearances using synthetic quotas. Co-authored-by: Otto <ottoprua@OttodeMac-mini-2.local>
Owner
|
I extracted and landed the independently verifiable display correction in #3543, with credit to @OttoPrua. A known exhausted monthly pool now takes precedence in Auto, with regression coverage and signed native before/after proof. This PR remains open for the Desktop authentication work. That portion still needs evidence that the selected credential belongs to the current active session and that sign-out/deletion state is respected before it can be adopted. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
kimi-authcookie, so Auto never callsGetSubscriptionStats.Total usage) in automatic menu-bar mode.Fixes #3536
Test plan
kimi-authcookie; Cookie source Automatic; API key configuredTotal usagefromGetSubscriptionStatsTotal usageinstead of unused 5-hour / 7-day windowskimi-authstill wins over Local Storage, and refresh / expired JWTs are ignoredswift test --filter KimiDesktopAuthTokenTestsandswift test --filter KimiProviderTestsMade with Cursor