Repository navigation
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between 9b5538d and ec4fee4b2cf6653cf9f7baa22a2c6d99d4f93967. 📒 Files selected for processing (58)
🚧 Files skipped from review as they are similar to previous changes (39)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughAdds Zhipu/Z.AI credential storage, cc-switch import, quota fetching, and rate-limit state management. Exposes credential and refresh operations through Electron IPC, preload APIs, and browser fallbacks. Adds Zhipu account settings, localized text, status-bar visibility, icons, provider routing, and usage indicators. Updates shared state contracts and persists a one-time migration that adds Zhipu to the default status-bar items. Adds tests for fetching, importing, IPC registration, renderer behavior, visibility rules, and state initialization. Merge Risk: ⚪ Minimal · up to The PR adds Zhipu usage monitoring and its related settings, status-bar, credential, and quota-fetching integration without any identified current-head merge-blocking risk; it is merge-ready after normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 67080387-5860-47a9-8900-131da58ddde5
📥 Commits
Reviewing files that changed from the base of the PR and between 165e4e0 and 1ae47a972822fa47aedd14da4f3023a71398ca79.
📒 Files selected for processing (51)
src/main/codex-accounts/service.test.tssrc/main/ipc/rate-limits.tssrc/main/ipc/register-core-handlers.tssrc/main/ipc/zhipu-credentials.tssrc/main/rate-limits/service.test.tssrc/main/rate-limits/service.tssrc/main/rate-limits/zhipu-fetcher.test.tssrc/main/rate-limits/zhipu-fetcher.tssrc/main/runtime/rpc/methods/client-ui-schemas.tssrc/main/runtime/rpc/methods/client-ui.test.tssrc/main/zhipu/cc-switch-import.test.tssrc/main/zhipu/cc-switch-import.tssrc/main/zhipu/zhipu-credential-store.tssrc/preload/api-types.tssrc/preload/index.tssrc/renderer/src/components/settings/AccountsPane.tsxsrc/renderer/src/components/settings/AppearanceWindowSidebarSection.tsxsrc/renderer/src/components/settings/ZhipuAccountsSection.test.tsxsrc/renderer/src/components/settings/ZhipuAccountsSection.tsxsrc/renderer/src/components/settings/accounts-search.tssrc/renderer/src/components/settings/appearance-status-bar-minimax-toggle-search.tssrc/renderer/src/components/settings/appearance-status-bar-search.tssrc/renderer/src/components/settings/appearance-status-bar-zhipu-toggle-search.tssrc/renderer/src/components/stats/GrokUsagePane.test.tsxsrc/renderer/src/components/status-bar/StatusBar.tsxsrc/renderer/src/components/status-bar/icons.tsxsrc/renderer/src/components/status-bar/status-bar-agent-gating.test.tssrc/renderer/src/components/status-bar/status-bar-provider-visibility.test.tssrc/renderer/src/components/status-bar/status-bar-provider-visibility.tssrc/renderer/src/components/status-bar/tooltip.test.tssrc/renderer/src/components/status-bar/tooltip.tsxsrc/renderer/src/components/status-bar/usage-error-copy.test.tssrc/renderer/src/components/status-bar/usage-error-copy.tssrc/renderer/src/components/status-bar/usage-provider-settings-target.test.tssrc/renderer/src/components/status-bar/usage-provider-settings-target.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.jsonsrc/renderer/src/store/slices/rate-limits.test.tssrc/renderer/src/store/slices/rate-limits.tssrc/renderer/src/store/slices/ui.test.tssrc/renderer/src/store/slices/ui.tssrc/renderer/src/web/web-preload-api.test.tssrc/renderer/src/web/web-preload-api.tssrc/shared/rate-limit-types.test.tssrc/shared/rate-limit-types.tssrc/shared/status-bar-defaults.tssrc/shared/types.tssrc/shared/zhipu-usage.ts
|
Hi! I addressed the CodeRabbit findings in 8076d32 and re-ran the targeted tests plus localization verification. Happy to split out the cc-switch import if you prefer a smaller review surface. |
8076d32 to
251c7da
Compare
|
Rebased onto the latest main (251c7da8) to resolve the merge conflicts. The conflict was limited to the i18n locale files, where main landed plugin copy in the same region this PR adds Zhipu copy — all five locales now carry both. Re-ran verify:localization-catalog, verify:localization-coverage, and typecheck:web locally; all green. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -30,4 +30,5 @@ export function registerRateLimitHandlers( | |||
| ) | |||
| ipcMain.handle('rateLimits:refreshMiniMax', () => rateLimits.refresh()) | |||
| ipcMain.handle('rateLimits:refreshGrok', () => rateLimits.refreshGrok()) | |||
| ipcMain.handle('rateLimits:refreshZhipu', () => rateLimits.refresh()) | |||
There was a problem hiding this comment.
refreshZhipu triggers a full provider refresh rather than a Zhipu-specific one
rateLimits:refreshZhipu is wired to rateLimits.refresh(), which refreshes every provider (Claude, Codex, Gemini, MiniMax, Grok, etc.) in parallel. By contrast, rateLimits:refreshGrok calls the dedicated rateLimits.refreshGrok(). When a user clicks "Refresh usage" in the Zhipu settings panel, all other provider fetches are triggered as a side-effect. This is consistent with the MiniMax pattern so it won't break anything, but if a dedicated refreshZhipu() method is ever added to RateLimitService (as was done for Grok), the IPC handler here will need to be updated in tandem.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Greptile SummaryThis PR adds Zhipu / Z.AI as a new usage provider in Orca, following the same architecture used by MiniMax, Grok, and other existing providers. It wires Zhipu quota fetching through credential storage, IPC/preload APIs, rate-limit service orchestration, status-bar visibility, settings UI, localization, and one-shot migration of the default status-bar item.
Confidence Score: 5/5
|
| Filename | Overview |
|---|---|
| src/main/rate-limits/zhipu-fetcher.ts | New Zhipu usage fetcher — host allowlist validation, timeout chaining, graceful status mapping, and robust JSON parsing are all implemented correctly. |
| src/main/zhipu/zhipu-credential-store.ts | Credential store follows the safeStorage envelope pattern used by other providers; cache invalidation and hardening on read/check are handled correctly. |
| src/main/zhipu/cc-switch-import.ts | cc-switch import uses read-only SQLite with query_only pragma and parameterized queries; Zhipu host validation guards the imported base URL before saving. |
| src/main/ipc/zhipu-credentials.ts | IPC handler validates argument types, delegates persistence to the credential store, and triggers invalidation + refresh consistently with other providers. |
| src/main/rate-limits/service.ts | Zhipu is integrated into the fetch orchestration with generation-based stale-result protection, config-hash change detection, and isolated credential-read-failure handling. |
| src/renderer/src/components/status-bar/StatusBar.tsx | Previously reported bugs (missing comma in usageSettings, wrong argument count to isUsageEmptyState) are both fixed in this revision; Zhipu is wired in consistently with MiniMax/Grok. |
| src/renderer/src/components/settings/ZhipuAccountsSection.tsx | Settings component follows the established pattern; double-refresh after save (component calls refreshZhipuRateLimits after the IPC handler already triggers refresh) is benign and consistent with the MiniMax section. |
| src/renderer/src/components/status-bar/status-bar-provider-visibility.ts | Zhipu correctly added to hasUsageProviderSettings, hasUsageProviderSettingsForProvider, and isUsageEmptyState; no gaps found. |
| src/shared/zhipu-usage.ts | Allowlist and normalization helpers are clean; isZhipuUsageHost lowercases the host before lookup, which is correct. |
| src/renderer/src/store/slices/ui.ts | Zhipu status-bar default migration follows the same one-shot flag pattern used for Grok, Kimi, MiniMax, and Antigravity. |
| src/main/rate-limits/service-zhipu-usage.test.ts | Tests cover credential read failures, stale in-flight result protection, and state isolation from other providers. |
| src/main/ipc/rate-limits.ts | rateLimits:refreshZhipu calls rateLimits.refresh() (full provider refresh) instead of a Zhipu-specific method, matching the MiniMax pattern but diverging from Grok's dedicated method. |
Sequence Diagram
sequenceDiagram
participant User as User (Renderer)
participant ZSec as ZhipuAccountsSection
participant IPC as IPC (zhipu-credentials)
participant Store as ZhipuCredentialStore
participant Svc as RateLimitService
participant API as Zhipu API
User->>ZSec: Paste token + Save
ZSec->>IPC: zhipuCredentials:save(baseUrl, authToken)
IPC->>Store: saveZhipuCredentials()
IPC->>Svc: invalidateZhipuCredentialState()
Note over Svc: zhipuFetchGeneration++, clear snapshot
IPC->>Svc: refresh() (all providers)
ZSec->>IPC: rateLimits:refreshZhipu (redundant, benign)
IPC->>Svc: refresh() (all providers)
Svc->>Store: readZhipuCredentials()
alt credentials ok
Svc->>API: GET /api/monitor/usage/quota/limit
API-->>Svc: "{data:{limits:[TOKENS_LIMIT, TIME_LIMIT]}}"
Svc-->>User: "zhipu: {status:"ok", session, monthly}"
else credentials unreadable
Svc-->>User: "zhipu: {status:"error", failureKind:"keychain-unavailable"}"
else no credentials
Svc-->>User: "zhipu: {status:"unavailable"}"
end
Reviews (4): Last reviewed commit: "feat: add Zhipu / Z.AI usage monitoring" | Re-trigger Greptile
251c7da to
3664e12
Compare
| grokAuthConfigured: rateLimits.grokAuthConfigured | ||
| zhipuCredentialsConfigured: rateLimits.zhipuCredentialsConfigured |
There was a problem hiding this comment.
Line 2089 is missing a trailing comma after grokAuthConfigured: rateLimits.grokAuthConfigured. Without the comma, the next line (zhipuCredentialsConfigured) is an invalid continuation inside the object literal — this is a JavaScript syntax error that prevents the file from being parsed. The usageSettings object will never be constructed, so visibleZhipu, every getVisibleUsageProvider call, and all downstream status-bar visibility logic will fail at the component level.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/rate-limits/service.ts (1)
346-366: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
hasZhipuCredentials()reintroduces sync I/O on the hotgetState()path.
zhipuCredentialsConfigured: hasZhipuCredentials()(Line 354) does a synchronousexistsSyncplus a conditionalchmod(hardenExistingSecureFile) on everygetState()call.getState()backspushToRenderer(), which fires multiple times per fetch cycle and once per account inside the inactive-account fetch loops — this is exactly the "hot" path the file's own comment at Line 1613 calls out when explaining why Grok's sync auth-file probe was deliberately moved out ofgetState()into the fetch cycle (grokAuthConfiguredis just a cached boolean field). Mirroring the Grok approach here (cache azhipuCredentialsConfiguredboolean updated once perrunFetchAllCycle/resolveZhipuCredentials()) avoids adding sync filesystem/chmod calls to a path invoked far more frequently than the poll interval.♻️ Proposed fix
- private zhipuFetchGeneration = 0 + private zhipuFetchGeneration = 0 + private zhipuCredentialsConfigured = false ... getState(): RateLimitState { this.pruneInactiveClaudeState() this.pruneInactiveCodexState() return { ...this.state, minimaxCookieConfigured: hasMiniMaxSessionCookie(), grokAuthConfigured: this.grokAuthConfigured, - zhipuCredentialsConfigured: hasZhipuCredentials(), + zhipuCredentialsConfigured: this.zhipuCredentialsConfigured, ... } } ... // in runFetchAllCycle, alongside resolveZhipuCredentials(): + this.zhipuCredentialsConfigured = hasZhipuCredentials()
🧹 Nitpick comments (1)
src/main/rate-limits/service.test.ts (1)
2255-2286: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a test for the Zhipu credential-read error path.
Current tests cover the happy path and
zhipuCredentialsConfiguredflag, but notreadZhipuCredentials()throwing (exercisinggetZhipuCredentialError/keychain-unavailable) orinvalidateZhipuCredentialState()'s generation-bump race handling, similar to what's likely covered for MiniMax elsewhere in this suite.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4fe9c9cf-5436-49e0-875e-58998286c3ea
📥 Commits
Reviewing files that changed from the base of the PR and between 251c7da8effbf2b94e76aaf09940ace663a85bb6 and 3664e12d6a53691fa638f699f4a498e488e51d34.
📒 Files selected for processing (6)
src/main/codex-accounts/service.test.tssrc/main/ipc/rate-limits.tssrc/main/ipc/register-core-handlers.tssrc/main/ipc/zhipu-credentials.tssrc/main/rate-limits/service.test.tssrc/main/rate-limits/service.ts
3664e12 to
0cc4fa3
Compare
| const isEmptyUsageState = isUsageEmptyState( | ||
| { claude, codex, gemini, opencodeGo, kimi, antigravity, minimax, grok }, | ||
| { claude, codex, gemini, opencodeGo, kimi, antigravity, minimax, grok, zhipu }, | ||
| usageSettings | ||
| ) |
There was a problem hiding this comment.
isUsageEmptyState called with an extra argument, dropping the real settings
isUsageEmptyState accepts exactly two parameters: (providers, settings). This call passes three. At runtime: the first object (without zhipu) becomes providers, the second rate-limits snapshot becomes settings, and the actual usageSettings is the third argument — silently ignored by JavaScript.
Concrete failures this causes today:
isProviderSnapshotPending(providers.zhipu)is called withundefined(notnull), so it returnsfalse; the "don't show the CTA while Zhipu is loading" guard never fires.hasUsageProviderSettings(settings)receives aProviderRateLimitssnapshot object; every property lookup (settings?.zhipuCredentialsConfigured,settings?.grokAuthConfigured,settings?.codexManagedAccounts, etc.) returnsundefined/false, so the function always returnsfalse. Any user whose only configured provider is Zhipu, Grok, MiniMax, or a managed Codex account will incorrectly see the "Set up usage tracking" empty-state CTA.
The correct call is:
const isEmptyUsageState = isUsageEmptyState(
{ claude, codex, gemini, opencodeGo, kimi, antigravity, minimax, grok, zhipu },
usageSettings
)
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
7b36e27 to
9c6345b
Compare
7a75fe3 to
ec4fee4
Compare
|
Rebased onto latest main and pushed ec4fee4b. Fixed the StatusBar syntax/empty-state issues, cached Zhipu credential status off getState(), and added coverage for credential-read errors plus stale in-flight invalidation. Local checks passed: typecheck:web, typecheck:node, targeted vitest suites, targeted oxlint, localization checks, max-lines ratchet, build:electron-vite, and git diff --check.\n\n@coderabbitai review |
|
🧠 Learnings used✅ Action performedReview finished.
|
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
c88bb5c to
99fd3c2
Compare
Re-applied onto latest main; resolved service.ts Kimi refactor and 5 i18n locale conflicts.
150edc4 to
86cbc5f
Compare
# Conflicts: # src/main/ipc/register-core-handlers/register-core-handlers.test.ts # src/main/ipc/register-core-handlers/register-core-handlers.ts # src/main/rate-limits/rate-limit-service-test-harness.ts # src/main/rate-limits/service-account-target-selection.test.ts # src/main/rate-limits/service-inactive-account-previews.test.ts # src/main/rate-limits/service-live-claude-usage.test.ts # src/main/rate-limits/service-refresh-orchestration.test.ts # src/main/rate-limits/service-window-activation.test.ts # src/main/rate-limits/service/service-configuration.ts # src/main/rate-limits/service/service-full-cycle-preparation.ts # src/main/rate-limits/service/service-polling.ts # src/main/rate-limits/service/service-state.ts # src/main/rate-limits/service/service-types.ts # src/preload/index.ts # src/renderer/src/components/settings/AccountsPane.tsx # src/renderer/src/components/settings/AppearanceWindowSidebarSection.tsx # src/renderer/src/components/settings/accounts-search.ts # src/renderer/src/components/settings/appearance-status-bar-minimax-toggle-search.ts # src/renderer/src/components/settings/appearance-status-bar-search.ts # src/renderer/src/components/status-bar/StatusBarProviderSegment.tsx # src/renderer/src/components/status-bar/StatusBarVisibilityMenu.tsx # src/renderer/src/components/status-bar/status-bar-provider-visibility.test.ts # src/renderer/src/components/status-bar/status-bar-provider-visibility.ts # src/renderer/src/components/status-bar/tooltip.tsx # src/renderer/src/components/status-bar/usage-error-copy.ts # src/renderer/src/components/status-bar/usage-provider-settings-target.ts # src/renderer/src/components/status-bar/use-status-bar-controller.ts # src/renderer/src/store/slices/ui-hydration-workspace-preferences.test.ts # src/renderer/src/store/slices/ui/ui-slice-hydration-actions.ts # src/renderer/src/web/preload-api/web-agent-accounts-api.ts # src/shared/persisted-ui-state-types.ts # src/shared/rate-limit-state-factory.ts # src/shared/rate-limit-types.test.ts # src/shared/rate-limit-types.ts # src/shared/status-bar-defaults.ts # src/shared/ui-chrome-types.ts
|
Rebased onto the latest main and resolved all conflicts. What the resolution does:
Verification: node + web typecheck pass; targeted vitest suites (register-core-handlers, status-bar-provider-visibility, ui-hydration-workspace-preferences) pass — 79 tests green. |
|
Cross-linking: the repeated per-provider wiring in this PR (scan worker registration, preload bridge, IPC surface, store, UI) is what motivated #23580 — a proposal to document a one-page spec for adding a usage provider and close the contract drift gaps. If it lands, future providers should not have to re-derive these steps by hand. I'm happy to write the doc once the scope is agreed. |
ELI5
Adds a Zhipu / Z.AI usage meter so Orca can show GLM Coding Plan quota in the same places it already shows Claude, Codex, Gemini, MiniMax, Grok, Kimi, and OpenCode Go usage.
What Changed
origin/main, adapted the PR to the split rate-limit/preload/store test structure, and resolved stale conflicts from deleted/moved files.zhipuCredentialsConfiguredoff the hotgetState()path;StatusBar.tsxmissing comma;isUsageEmptyStateto pass the correct settings argument;Why
Zhipu / Z.AI has a GLM Coding Plan quota model that users need to monitor while working. This follows the existing provider architecture instead of adding a separate one-off path, so settings, status-bar visibility, refresh state, and error handling behave consistently with the other usage providers.
Linked Issue
N/A — no upstream issue is linked for this external contribution.
Visual Proof
Before: N/A — Zhipu was not available as a usage provider.
After:
Testing
Local checks run on Windows:
pnpm run typecheck:webpnpm run typecheck:nodepnpm exec vitest run --config config/vitest.config.ts src/main/rate-limits/service-zhipu-usage.test.ts src/main/rate-limits/zhipu-fetcher.test.ts src/main/rate-limits/service-refresh-orchestration.test.ts src/main/rate-limits/service-minimax-usage.test.ts src/main/rate-limits/service-account-target-selection.test.ts src/main/rate-limits/service-inactive-account-previews.test.ts src/main/rate-limits/service-live-claude-usage.test.ts src/main/rate-limits/service-window-activation.test.tspnpm exec vitest run --config config/vitest.config.ts src/main/codex-accounts/service.test.ts src/main/ipc/register-core-handlers.test.ts src/main/ipc/rate-limits.test.ts src/main/runtime/rpc/methods/client-ui.test.ts src/main/zhipu/cc-switch-import.test.ts src/renderer/src/components/settings/ZhipuAccountsSection.test.tsx src/renderer/src/components/settings/accounts-search.test.ts src/renderer/src/components/settings/appearance-status-bar-search.test.ts src/renderer/src/components/stats/GrokUsagePane.test.tsxpnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/status-bar/status-bar-agent-gating.test.ts src/renderer/src/components/status-bar/status-bar-provider-visibility.test.ts src/renderer/src/components/status-bar/tooltip.test.ts src/renderer/src/components/status-bar/usage-error-copy.test.ts src/renderer/src/components/status-bar/usage-provider-settings-target.test.ts src/renderer/src/store/slices/rate-limits.test.ts src/renderer/src/store/slices/ui-hydration-workspace-preferences.test.ts src/renderer/src/web/web-preload-api-agent-providers.test.ts src/preload/usage-provider-api.test.ts src/shared/rate-limit-types.test.tspnpm exec oxlint ...on the touched filespnpm run verify:localization-catalogpnpm run verify:localization-extractionpnpm run verify:localization-coveragepnpm run check:max-lines-ratchetpnpm run build:electron-vitegit diff --checkAI Disclosure
Codex (GPT-5) was used to rebase the PR, resolve conflicts, address CodeRabbit/Greptile feedback, update tests, and refresh this PR description.
Review
Self-reviewed for:
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
RateLimitStatefields are additive, and status-bar hydration includes a one-shot migration for the default Zhipu item.getState()to avoid sync filesystem work on renderer push paths.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)pnpm lint/ fullpnpm testwere not run in this rebase pass; targetedoxlint, targeted tests, typechecks, localization checks, max-lines ratchet, andbuild:electron-vitepassed locally.Author
Parking note: this PR is parked pending the stable usage-provider API / plugin system (tracked in #23580) — it will be ported to that surface when it lands, per the discussion there.