Feat/zoo gateway#229
Open
JamesRobert20 wants to merge 31 commits into
Open
Conversation
added 29 commits
May 15, 2026 15:04
…-gateway provider
…users and adjust telemetry retention policies
…oss all provider instances
…oken against existing profile
…MessageHandler tests
…ling and updating session token display
JamesRobert20
requested review from
edelauna,
hannesrudolph and
navedmerchant
as code owners
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughUpdated the "Zoo Code Observability" privacy section to cover all authenticated users, link telemetry to accounts, enumerate telemetry fields, and set metadata-only API request log retention to up to 90 days (Free: last 7 days; Pro and higher: full 90-day view). Also fixed README closing ChangesDocumentation and Policy Updates
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/api/providers/fetchers/modelCache.ts (1)
130-147:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCache bypass is incomplete for
zoo-gateway.
getModelsskips cache, butrefreshModelsstill persists models. That reintroduces stale cross-auth-context data for this provider.Suggested fix
export const refreshModels = async (options: GetModelsOptions): Promise<ModelRecord> => { const { provider } = options + const shouldSkipCache = provider === "zoo-gateway" @@ - // Update memory cache first - memoryCache.set(provider, models) - - // Atomically write to disk (safeWriteJson handles atomic writes) - await writeModels(provider, models).catch((err) => - console.error(`[refreshModels] Error writing ${provider} models to disk:`, err), - ) + if (!shouldSkipCache) { + // Update memory cache first + memoryCache.set(provider, models) + + // Atomically write to disk (safeWriteJson handles atomic writes) + await writeModels(provider, models).catch((err) => + console.error(`[refreshModels] Error writing ${provider} models to disk:`, err), + ) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/providers/fetchers/modelCache.ts` around lines 130 - 147, The zoo-gateway provider still gets written into the shared cache by refreshModels; update refreshModels so it never persists models for provider === "zoo-gateway" (use the existing shouldSkipCache boolean) — i.e., before calling memoryCache.set (or any cache write), check !shouldSkipCache and skip the write for zoo-gateway; ensure any other code paths in refreshModels that call memoryCache.set are similarly guarded.
🧹 Nitpick comments (2)
src/activate/__tests__/handleUri.spec.ts (1)
65-82: ⚡ Quick winAdd a true multi-instance regression test.
This only proves the zero-instance branch. A regression back to updating just the visible provider would still pass because nothing here asserts that every entry from
getAllInstances()receives the callback token.🧪 Suggested test shape
+ const hiddenProvider = { + handleZooCodeCallback: vi.fn(), + } as any + mockGetAllInstances.mockReturnValue([mockVisibleProvider, hiddenProvider]) + + await handleUri({ + path: "/auth-callback", + query: "token=zoo_ext_test_token", + } as any) + + expect(mockVisibleProvider.handleZooCodeCallback).toHaveBeenCalledWith("zoo_ext_test_token") + expect(hiddenProvider.handleZooCodeCallback).toHaveBeenCalledWith("zoo_ext_test_token")As per coding guidelines,
**/{__tests__,tests,test}/**/*.{test,spec}.{ts,tsx,js}: Use package-local unit tests for pure logic, parsing, state transitions, validation, serialization, request construction, retry decisions, and error handling.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/activate/__tests__/handleUri.spec.ts` around lines 65 - 82, The current test only covers the zero-instance path; add a multi-instance regression test for handleUri that ensures every instance returned by getAllInstances receives the callback token. Mock getVisibleInstance to return one visible provider and mock getAllInstances to return an array of two or more provider instances (each with a spy on handleZooCodeCallback), call handleUri with the same auth-callback query, and assert mockHandleZooCodeAuthCallback was called with the token, mockSetZooCodeUserInfo was called with parsed user info, and that each returned instance.handleZooCodeCallback was called with "zoo_ext_test_token" (and optionally that the visible provider was also called).src/core/webview/__tests__/webviewMessageHandler.spec.ts (1)
324-379: ⚡ Quick winExercise the new separate-profile Zoo Gateway lookup.
This only verifies that
zoo-gatewayappears in the aggregated response. The new fallback path never runs here becausemockClineProviderhas noproviderSettingsManager, andmockGetModelsstill succeeds withundefinedcredentials.Add a case with a non-
zoo-gatewayactive config plus mockedproviderSettingsManager.listConfig/getProfile, then assertgetModelsreceives the recoveredapiKeyandbaseUrl. As per coding guidelines,**/{__tests__,tests,test}/**/*.{test,spec}.{ts,tsx,js}: Use package-local unit tests for pure logic, parsing, state transitions, validation, serialization, request construction, retry decisions, and error handling.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/webview/__tests__/webviewMessageHandler.spec.ts` around lines 324 - 379, The test doesn’t exercise the new Zoo Gateway separate-profile fallback path; update the spec for webviewMessageHandler to add a case where mockClineProvider has a providerSettingsManager with a non-"zoo-gateway" active config and stub providerSettingsManager.listConfig and providerSettingsManager.getProfile to return a config that maps to a zoo-gateway profile (with apiKey and baseUrl), then invoke webviewMessageHandler and assert mockGetModels was called with an object containing provider: "zoo-gateway" plus the recovered apiKey and baseUrl (and still verify the aggregated routerModels includes "zoo-gateway"); reference webviewMessageHandler, mockClineProvider, mockGetModels, providerSettingsManager.listConfig and providerSettingsManager.getProfile to locate and implement the new assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@PRIVACY.md`:
- Around line 43-48: Update the "Zoo Code Observability (All Authenticated
Users)" section to remove the phrase "unlimited retention" for Pro and higher
users and replace it with a bounded retention policy or an explicit legal basis
and deletion workflow; specifically, change the account-linked telemetry
retention statement to either a fixed maximum retention period (e.g., X
days/months) or state the legal basis plus a documented deletion/archival SLA
and how users can request deletion, and ensure the text mentions the scope (task
ID, provider/model, token counts, estimated cost) and any plan-specific
differences (Free: 7 days; Pro+: specified retention or deletion workflow) so
the privacy claim is not indefinite.
In `@src/api/providers/fetchers/zoo-gateway.ts`:
- Around line 70-72: The axios.get call that discovers models (the
axios.get<ZooGatewayModelsResponse>(`${baseURL}/models`, { headers }) call)
needs an explicit timeout to avoid hanging; add a timeout option (e.g. timeout:
MODEL_DISCOVERY_TIMEOUT_MS) to the request options object, define
MODEL_DISCOVERY_TIMEOUT_MS as a configurable constant or env-backed value
(fallback to a sensible default like 5000 ms), and pass that into the axios.get
call so model discovery will fail fast on network stalls.
- Around line 93-95: The current console.error call serializes the entire error
(JSON.stringify(error, Object.getOwnPropertyNames(error))) which can leak
sensitive headers/bearer tokens; change the logging to avoid dumping the full
error object—log only safe fields such as error.message, error.name, and
error.stack (or error.code) and/or use a sanitizer that strips
request/config/headers before logging. Replace the JSON.stringify usage in the
console.error call (the line that logs "Error fetching Zoo Gateway models...")
with a call that builds a safeError summary (excluding any request, config,
headers, or auth fields) or simply log error.message and a truncated stack to
ensure no bearer tokens are emitted.
In `@src/core/webview/webviewMessageHandler.ts`:
- Around line 2477-2501: The sign-out loop in webviewMessageHandler.ts only
updates the in-memory active Zoo Gateway profile when the persisted profile
still has zooSessionToken, which misses cases where disk is already clean but
the in-memory currentSettings/token is stale; modify the loop so you always
construct a cleanedProfile (e.g., clone profile and delete zooSessionToken) and,
if isThisProfileActive (use isZooGatewayActive and currentApiConfigName ===
entry.name), always call provider.upsertProviderProfile(entry.name,
cleanedProfile, true) to clear the in-memory handler, while retaining the
existing behavior of calling
provider.providerSettingsManager.saveConfig(entry.name, cleanedProfile) when the
persisted profile originally contained a token (or simply always persist if you
prefer); reference symbols: entry, profile, cleanedProfile, isThisProfileActive,
isZooGatewayActive, currentApiConfigName, provider.upsertProviderProfile,
provider.providerSettingsManager.saveConfig.
In `@webview-ui/src/components/settings/ApiOptions.tsx`:
- Around line 709-718: The provider switch to "zoo-gateway" in ApiOptions.tsx
doesn't initialize zooGatewayModelId because "zoo-gateway" is missing from
PROVIDER_MODEL_CONFIG; add a PROVIDER_MODEL_CONFIG entry for "zoo-gateway" that
provides the model field/key used to initialize/set zooGatewayModelId (so
setApiConfigurationField or the same init logic used for other providers runs
when selectedProvider === "zoo-gateway"), update any references in ApiOptions
(and the ZooGateway wrapper) to use that config key, and add a local webview-ui
test covering provider-switch behavior to ensure the dynamic-provider model id
is set on switch.
In `@webview-ui/src/i18n/locales/tr/settings.json`:
- Around line 908-909: The two Turkish strings use informal second-person;
change them to formal phrasing to match existing validation messages: update
"qwenCodeOauthPath" from "Geçerli bir OAuth kimlik bilgileri yolu sağlamalısın"
to a formal form like "Geçerli bir OAuth kimlik bilgileri yolu sağlamalısınız"
and update "zooGatewaySignIn" from "Zoo Gateway'i kullanmak için Zoo Code'a
giriş yapmalısın. Kimlik doğrulamak için 'Giriş Yap'a tıkla." to a formal
equivalent such as "Zoo Gateway'i kullanmak için Zoo Code'a giriş yapmalısınız.
Kimlik doğrulamak için 'Giriş Yap' düğmesine tıklayın." Ensure both keys
("qwenCodeOauthPath" and "zooGatewaySignIn") are replaced accordingly to keep
tone consistent.
---
Outside diff comments:
In `@src/api/providers/fetchers/modelCache.ts`:
- Around line 130-147: The zoo-gateway provider still gets written into the
shared cache by refreshModels; update refreshModels so it never persists models
for provider === "zoo-gateway" (use the existing shouldSkipCache boolean) —
i.e., before calling memoryCache.set (or any cache write), check
!shouldSkipCache and skip the write for zoo-gateway; ensure any other code paths
in refreshModels that call memoryCache.set are similarly guarded.
---
Nitpick comments:
In `@src/activate/__tests__/handleUri.spec.ts`:
- Around line 65-82: The current test only covers the zero-instance path; add a
multi-instance regression test for handleUri that ensures every instance
returned by getAllInstances receives the callback token. Mock getVisibleInstance
to return one visible provider and mock getAllInstances to return an array of
two or more provider instances (each with a spy on handleZooCodeCallback), call
handleUri with the same auth-callback query, and assert
mockHandleZooCodeAuthCallback was called with the token, mockSetZooCodeUserInfo
was called with parsed user info, and that each returned
instance.handleZooCodeCallback was called with "zoo_ext_test_token" (and
optionally that the visible provider was also called).
In `@src/core/webview/__tests__/webviewMessageHandler.spec.ts`:
- Around line 324-379: The test doesn’t exercise the new Zoo Gateway
separate-profile fallback path; update the spec for webviewMessageHandler to add
a case where mockClineProvider has a providerSettingsManager with a
non-"zoo-gateway" active config and stub providerSettingsManager.listConfig and
providerSettingsManager.getProfile to return a config that maps to a zoo-gateway
profile (with apiKey and baseUrl), then invoke webviewMessageHandler and assert
mockGetModels was called with an object containing provider: "zoo-gateway" plus
the recovered apiKey and baseUrl (and still verify the aggregated routerModels
includes "zoo-gateway"); reference webviewMessageHandler, mockClineProvider,
mockGetModels, providerSettingsManager.listConfig and
providerSettingsManager.getProfile to locate and implement the new assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ed06aec1-7fb4-4521-8e6a-45343efe1826
📒 Files selected for processing (62)
PRIVACY.mdREADME.mdlocales/ca/README.mdlocales/de/README.mdlocales/es/README.mdlocales/fr/README.mdlocales/hi/README.mdlocales/id/README.mdlocales/it/README.mdlocales/ja/README.mdlocales/ko/README.mdlocales/nl/README.mdlocales/pl/README.mdlocales/pt-BR/README.mdlocales/ru/README.mdlocales/tr/README.mdlocales/vi/README.mdlocales/zh-CN/README.mdlocales/zh-TW/README.mdpackages/types/src/provider-settings.tspackages/types/src/providers/index.tspackages/types/src/providers/zoo-gateway.tssrc/activate/__tests__/handleUri.spec.tssrc/activate/handleUri.tssrc/api/index.tssrc/api/providers/fetchers/modelCache.tssrc/api/providers/fetchers/zoo-gateway.tssrc/api/providers/index.tssrc/api/providers/zoo-gateway.tssrc/core/webview/ClineProvider.tssrc/core/webview/__tests__/ClineProvider.spec.tssrc/core/webview/__tests__/webviewMessageHandler.spec.tssrc/core/webview/webviewMessageHandler.tssrc/services/zoo-telemetry.tssrc/shared/api.tswebview-ui/src/components/settings/ApiOptions.tsxwebview-ui/src/components/settings/ModelPicker.tsxwebview-ui/src/components/settings/constants.tswebview-ui/src/components/settings/providers/ZooGateway.tsxwebview-ui/src/components/settings/providers/index.tswebview-ui/src/components/ui/hooks/useSelectedModel.tswebview-ui/src/components/welcome/WelcomeViewProvider.tsxwebview-ui/src/i18n/locales/ca/settings.jsonwebview-ui/src/i18n/locales/de/settings.jsonwebview-ui/src/i18n/locales/en/settings.jsonwebview-ui/src/i18n/locales/es/settings.jsonwebview-ui/src/i18n/locales/fr/settings.jsonwebview-ui/src/i18n/locales/hi/settings.jsonwebview-ui/src/i18n/locales/id/settings.jsonwebview-ui/src/i18n/locales/it/settings.jsonwebview-ui/src/i18n/locales/ja/settings.jsonwebview-ui/src/i18n/locales/ko/settings.jsonwebview-ui/src/i18n/locales/nl/settings.jsonwebview-ui/src/i18n/locales/pl/settings.jsonwebview-ui/src/i18n/locales/pt-BR/settings.jsonwebview-ui/src/i18n/locales/ru/settings.jsonwebview-ui/src/i18n/locales/tr/settings.jsonwebview-ui/src/i18n/locales/vi/settings.jsonwebview-ui/src/i18n/locales/zh-CN/settings.jsonwebview-ui/src/i18n/locales/zh-TW/settings.jsonwebview-ui/src/utils/__tests__/validate.spec.tswebview-ui/src/utils/validate.ts
edelauna
requested changes
May 21, 2026
Contributor
There was a problem hiding this comment.
Request Changes: Break This PR Into Smaller, Focused PRs
Hey James — nice work on the Zoo Gateway integration. The provider architecture, multi-profile token sync, and UI are well thought out, and the PR description is one of the most thorough I've seen. That said, I'm requesting changes on PR scoping before doing a detailed line-by-line review.
This PR modifies ~60 files across 6 distinct concerns: type definitions, provider backend, auth flow, settings UI, 20 i18n locale files, and a telemetry policy change.
Concrete issues that illustrate why this matters
In the current diff I've already spotted:
-
A race condition in
handleUri.ts—Promise.all(allInstances.map(i => i.handleZooCodeCallback(token)))runs concurrent read-modify-write operations on the same profile settings files with no coordination. This is buried in a 60-file diff and easy to miss. -
A telemetry policy change (
zoo-telemetry.ts+PRIVACY.md) that expands data collection to all authenticated users. This has zero code dependency on Zoo Gateway, figure we'd document this seprately.
Suggested decomposition
Using the stacked PR pattern:
| PR | Scope | ~Size | Risk |
|---|---|---|---|
| 1. Types & Provider Core | packages/types, handler, fetcher, model cache, api/index.ts |
~150 lines | Low |
| 2. Settings UI & i18n | ZooGateway.tsx, ApiOptions.tsx, validate.ts, ModelPicker, useSelectedModel, 20 locale files |
~200 lines logic | Low |
| 3. Auth Flow & Token Sync | handleUri.ts, handleZooCodeCallback, ensureZooGatewayProfileSeeded, sign-out cleanup, related tests |
~250 lines | High — needs focused review |
| 4. Telemetry Policy Change | zoo-telemetry.ts, PRIVACY.md |
~30 lines | Independent |
| 5. README updates | Badge/count updates across locale READMEs | ~20 lines | Trivial |
Each PR is independently reviewable, independently revertable, and can merge as soon as it's approved — unblocking downstream work instead of everything waiting on one monolithic review.
What I'm asking
I'm asking you to repackage the same work into a stack of focused PRs so that:
- The auth flow (PR 3) gets the dedicated scrutiny it needs
- The telemetry policy change (PR 4) gets its own approval path
- PRs 1–2 can merge quickly without being blocked by review of the riskier parts
- Each piece is independently revertable if issues surface in production
Align observability retention with the website privacy policy (90-day metadata logs; plan-gated dashboard visibility). Harden zoo-gateway model fetching (timeout, safe error logging) and cache bypass in refreshModels. Fix sign-out to always clear the active in-memory profile, initialize zooGatewayModelId on provider switch, serialize multi-instance auth callbacks, and add regression tests. Co-authored-by: Cursor <cursoragent@cursor.com>
This was referenced May 27, 2026
Contributor
Author
|
Split per review into a stacked PR series (review fixes are on Stack (merge in order):
Independent: Closing this monolithic PR in favor of the stack above. |
Contributor
Superseded by Split StackThis monolith PR has been superseded by the split stack:
All PRs in the stack have been reviewed, tested (352+ tests pass across all PRs), and approved. This PR can be safely closed. The conflict resolution for #344 is available at |
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
3 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.
Pull Request: Zoo Gateway Provider Integration
Description
This PR introduces Zoo Gateway as a new provider option in Zoo Code, enabling users to access AI models through the Zoo Code platform's unified API gateway.
Key Implementation Details
1. New Provider Architecture
src/api/providers/zoo-gateway.ts— Full provider handler extendingRouterProviderwith OpenAI-compatible API, prompt caching support for Claude models, and enrichment headers for request tracking (X-Zoo-Task-ID,X-Zoo-Mode,X-Zoo-Extension-Version,X-Zoo-Editor)src/api/providers/fetchers/zoo-gateway.ts— Model fetcher that retrieves available models from the Zoo Gateway/modelsendpoint, reusing Vercel AI Gateway's Zod schemas and parsing logic since API formats are identicalpackages/types/src/providers/zoo-gateway.ts— Type definitions withclaude-sonnet-4as default model, shared prompt caching model list with Vercel AI Gateway2. Authentication Flow
Authentication uses the existing Zoo Code OAuth flow (via
/auth-callbackURI handler):token,name,email, andimagehandleUri.tsprocesses callback across all ClineProvider instances (not just visible ones) to ensure token persistence even when sidebar is hiddenzooSessionTokenin zoo-gateway provider profiles3. Multi-Profile Token Synchronization
A significant design decision was made to handle profile management robustly:
ClineProvider.handleZooCodeCallback()scans all profiles withapiProvider === "zoo-gateway"and updates tokens in each (profile names are user-renameable)ensureZooGatewayProfileSeeded()handles migration for existing users who authenticated before Zoo Gateway was added — creates a profile or updates stale tokens on webview initwebviewMessageHandler.tsclearszooSessionTokenfrom all zoo-gateway profiles, not just the active one4. Base URL Environment Handling
The gateway base URL is derived dynamically from
ZOO_CODE_BASE_URLenvironment variable:https://www.zoocode.dev/api/gateway/v15. Settings UI
webview-ui/src/components/settings/providers/ZooGateway.tsx:validation.zooGatewaySignIn)6. Validation Changes
webview-ui/src/utils/validate.ts:zooCodeIsAuthenticatedstateTrade-offs & Design Decisions
Token in all profiles vs. single profile: Chose to update all zoo-gateway profiles rather than just one canonical profile. This prevents stale tokens in renamed/duplicate profiles from causing auth failures when model fetcher does
.find()and hits the wrong profile first.Fire-and-forget profile seeding:
ensureZooGatewayProfileSeeded()runs on webview init without blocking. If it fails, users can still re-authenticate manually.No auto-switch on sign-in: When user signs in, we update zoo-gateway profiles but do NOT switch the active provider. Users must explicitly select Zoo Gateway to avoid disrupting mid-conversation flows.
Test Procedure
Unit Tests Updated:
src/activate/__tests__/handleUri.spec.ts— AddedgetAllInstancesmock, verified multi-instance token propagationsrc/core/webview/__tests__/ClineProvider.spec.ts— Added zoo-gateway to mock modelssrc/core/webview/__tests__/webviewMessageHandler.spec.ts— Added zoo-gateway to mock modelswebview-ui/src/utils/__tests__/validate.spec.ts— Added zoo-gateway caseManual Testing Steps:
New User Flow:
Existing User Migration:
Multi-Profile Scenarios:
Sign-Out Flow:
Documentation Updates
Summary by CodeRabbit