feat(zoo-gateway): settings UI, validation, and i18n#345
Conversation
JamesRobert20
requested review from
edelauna,
hannesrudolph,
navedmerchant and
taltas
as code owners
|
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 (30)
✅ Files skipped from review due to trivial changes (10)
🚧 Files skipped from review as they are similar to previous changes (18)
📝 WalkthroughWalkthroughAdds a Zoo Gateway provider: new ZooGateway settings component and default-model selector, ApiOptions wiring and docs/model helper, auth-aware validation changes, tests for component/validation/fetcher, and localization entries across languages. ChangesZoo Gateway Provider Settings Integration
Sequence Diagram(s)sequenceDiagram
participant User
participant ApiOptions
participant ZooGateway
participant useExtensionState
participant pickZooGatewayDefaultModelId
participant setApiConfigurationField
participant ModelPicker
User->>ApiOptions: select provider zoo-gateway
ApiOptions->>ZooGateway: mount ZooGateway with props
ZooGateway->>useExtensionState: read zooCodeIsAuthenticated, zooSessionToken, dashboardBaseUrl
ZooGateway->>pickZooGatewayDefaultModelId: compute resolved default from routerModels["zoo-gateway"]
pickZooGatewayDefaultModelId-->>ZooGateway: defaultModelId
ZooGateway->>setApiConfigurationField: set zooGatewayModelId if missing/stale
ZooGateway->>ModelPicker: render with available models and selected/default id
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/api/providers/fetchers/__tests__/zoo-gateway.spec.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. webview-ui/src/components/settings/ApiOptions.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. webview-ui/src/components/settings/ModelPicker.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
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 |
JamesRobert20
force-pushed
the
feat/zoo-gateway-types-core
branch
from
f776483 to
dfb1eed
Compare
JamesRobert20
force-pushed
the
feat/zoo-gateway-settings-ui
branch
2 times, most recently
from
8634510 to
7bc8c0c
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
proyectoauraorg
left a comment
proyectoauraorg
left a comment
There was a problem hiding this comment.
Review: Settings UI, Validation, and i18n
Verdict: ✅ Approved
Verified
- ✅
ApiOptions.spec.tsx— 18 tests pass (including zoo-gateway modelId initialization) - ✅
validate.spec.ts— 20 tests pass - ✅
ZooGateway.tsx— clean component following existing provider pattern - ✅
validate.ts— requires authentication before use (good UX gate) - ✅ i18n — all 18 locales covered
Notes
vite.config.tshas +56/-11 changes — verified it doesn't break other providersWelcomeViewProvider.tsxintegration looks correct for onboarding flow- i18n keys
idandnlhave slightly fewer changes (+9/-1 vs +11/-3) — appears intentional
Dependency
Depends on #344. After #344 merges, this should rebase cleanly.
JamesRobert20
force-pushed
the
feat/zoo-gateway-settings-ui
branch
2 times, most recently
from
a40f6e9 to
983c133
Compare
JamesRobert20
force-pushed
the
feat/zoo-gateway-types-core
branch
from
6f2b189 to
2ae4393
Compare
JamesRobert20
force-pushed
the
feat/zoo-gateway-settings-ui
branch
2 times, most recently
from
6ae680e to
946d6d1
Compare
edelauna
left a comment
edelauna
left a comment
There was a problem hiding this comment.
Nice first pass! Had some feedback to prevent threading a boolean through layers.
Thanks again for breaking this up, makes review much faster / easier.
|
|
||
| const WelcomeViewProvider = () => { | ||
| const { apiConfiguration, currentApiConfigName, setApiConfiguration, uriScheme } = useExtensionState() | ||
| const { apiConfiguration, currentApiConfigName, setApiConfiguration, uriScheme, zooCodeIsAuthenticated } = |
There was a problem hiding this comment.
The deeper issue is that threading zooCodeIsAuthenticated through validation, props, and useCallback deps is a smell — it's caused by auth state living in two places (ProviderSettings.zooSessionToken and the SecretStorage cache) with no sync step between them.
VS Code has a first-class API for exactly this: vscode.authentication.registerAuthenticationProvider. Registering a provider means any caller — validation, handlers, UI — just asks VS Code for the session:
const session = await vscode.authentication.getSession('zoo-code', [], { createIfNone: false })
if (!session) return i18next.t('settings:validation.zooGatewaySignIn')This eliminates the threading entirely:
- Single source of truth — no split between profile token and cached token
onDidChangeSessionsfires automatically on sign-in/out, no manual state propagation needed- Validation reads only
ProviderSettingslike every other provider - VS Code's native Accounts menu (bottom-left status bar) handles sign-out for free
| case "zoo-gateway": | ||
| if (!apiConfiguration.zooSessionToken && !zooCodeIsAuthenticated) { | ||
| return i18next.t("settings:validation.zooGatewaySignIn") |
There was a problem hiding this comment.
Is this branch covered in validate.spec.ts? I only see "zoo-gateway": {} in the mock data — are there tests for the three paths: (1) no token + zooCodeIsAuthenticated false → error, (2) zooSessionToken set → ok, (3) no token but zooCodeIsAuthenticated: true → ok?
There was a problem hiding this comment.
Tests added in f6aae3c (validate.spec.ts):
validateApiConfigurationsuite: (1) no token +zooCodeIsAuthenticatedfalse returnssettings:validation.zooGatewaySignIn, (2) no token + auth true returns undefined, (3)zooSessionTokenset + auth false returns undefined.validateApiConfigurationExcludingModelErrorssuite: confirms the form-level path now no-ops for zoo-gateway regardless of token/auth state, since the inline<ApiErrorMessage>inZooGateway.tsxowns that UX (see the related thread onApiOptions.tsx).
| <a | ||
| href={authUrl} | ||
| className="inline-flex w-fit items-center rounded-sm bg-vscode-button-background px-3 py-1 text-xs text-vscode-button-foreground no-underline hover:bg-vscode-button-hoverBackground"> | ||
| {t("settings:providers.zooGateway.signInButton")} | ||
| </a> |
There was a problem hiding this comment.
Other providers with auth links use VSCodeButtonLink (e.g. OpenRouter.tsx and OpenCodeGo.tsx). Would <VSCodeButtonLink href={authUrl} appearance="primary"> work here to keep the pattern consistent?
There was a problem hiding this comment.
Already on VSCodeButtonLink in this file (the sign-in CTA at the bottom of the unauthenticated branch uses <VSCodeButtonLink href={authUrl} appearance="primary">). After f6aae3c the unauthenticated card stacks: inline <ApiErrorMessage> for the validation error, the existing description, then the same <VSCodeButtonLink>, which matches the OpenRouter/OpenCodeGo pattern.
There was a problem hiding this comment.
This change should be reverted — it's not related to zoo-gateway and it conflicts with #214 (chore(webview): migrate build to Vite 8), which deliberately introduced rolldownOptions, tsconfigPaths, and the simplified chunk config three days ago. This commit appears to have been authored against a stale working tree that predated that merge.
JamesRobert20
force-pushed
the
feat/zoo-gateway-settings-ui
branch
from
2dde8b5 to
7a25767
Compare
JamesRobert20
force-pushed
the
feat/zoo-gateway-types-core
branch
from
f067b91 to
9b56ac8
Compare
JamesRobert20
force-pushed
the
feat/zoo-gateway-settings-ui
branch
2 times, most recently
from
f780a86 to
d1e8b1c
Compare
JamesRobert20
force-pushed
the
feat/zoo-gateway-types-core
branch
from
8b677df to
9e8bab3
Compare
JamesRobert20
force-pushed
the
feat/zoo-gateway-settings-ui
branch
from
d1e8b1c to
2e677ba
Compare
| simplifySettings?: boolean | ||
| } | ||
|
|
||
| function isSonnet45ModelId(id: string) { |
There was a problem hiding this comment.
Not your fault because it's the standard, but putting numbers for function names feels so so wrong 😂😂
There was a problem hiding this comment.
Agreed, baking the version into the helper name was awkward. Renamed to isClaudeSonnetModelId in f6aae3c, with the version-priority chain (4.5 exact, 4.5 substring, 4.5 dash variant, 4 family, first sonnet) expressed directly inside pickZooGatewayDefaultModelId. The helper now just answers "is this a Claude Sonnet of any version".
| organizationAllowList, | ||
| setErrorMessage, | ||
| isRetiredSelectedProvider, | ||
| zooCodeIsAuthenticated, |
There was a problem hiding this comment.
As @edelauna mentioned below, we should be wary of pumping zooCodeIsAuthenticated in the useEffect here as well. , I don't think we should be pumpingzooCodeIsAuthenticated here in the useEffect through here. if you remove it, then add
+ if (apiConfiguration.apiProvider === "zoo-gateway") {
+ setErrorMessage(undefined)
+ return
+ }
You will get a generic error message if something goes wrong.
If you want to add specific error messaging you can add it on the UI side in webview-ui/src/components/settings/providers/ZooGateway.tsx with ApiErrorMessage component
+import { ApiErrorMessage } from "../ApiErrorMessage"
@@
{!zooCodeIsAuthenticated ? (
<div className="flex flex-col gap-1">
+ <ApiErrorMessage errorMessage={t("settings:validation.zooGatewaySignIn")} />
<p className="text-xs text-vscode-descriptionForeground">
{t("settings:providers.zooGateway.signInDescription")}
</p>
<VSCodeButtonLink href={authUrl} appearance="primary">
{t("settings:providers.zooGateway.signInButton")}
</VSCodeButtonLink>
</div>
) : (
There was a problem hiding this comment.
Done in f6aae3c, taking your suggestion exactly:
ApiOptions.tsxshort-circuits zoo-gateway in the validation effect and clears the error, sozooCodeIsAuthenticatedno longer needs to be in the dep list.validateApiConfigurationExcludingModelErrorsreturnsundefinedfor zoo-gateway entirely, so the form-level path stops carrying auth state.ZooGateway.tsxrenders the sign-in error inline using<ApiErrorMessage errorMessage={t("settings:validation.zooGatewaySignIn")} />above the existing description + sign-in button.
validateApiConfiguration (the welcome-view entry point) still threads zooCodeIsAuthenticated since the welcome screen does need to gate "Get Started" on auth state, and validate.spec.ts was reorganized so the three zoo-gateway branches (no token + unauth, no token + auth, token set) now exercise that entry point, with a separate suite asserting the form-level path no-ops for zoo-gateway.
React 19.2's useEffectEvent would be the textbook fit here, but webview-ui is still on React 18.3.1, so this approach is the closest equivalent until we upgrade.
JamesRobert20
force-pushed
the
feat/zoo-gateway-settings-ui
branch
3 times, most recently
from
f6aae3c to
e2e4536
Compare
JamesRobert20
force-pushed
the
feat/zoo-gateway-types-core
branch
from
2f9a5a0 to
5e6c278
Compare
taltas
left a comment
taltas
left a comment
There was a problem hiding this comment.
One other thing I noticed
There was a problem hiding this comment.
Not your fault, but I think we should replace this entire thing in with a separate file, the ApiOptions file is geting pretty long. Something like
const config = PROVIDER_UI_REGISTRY[value]?.modelConfig
const name = PROVIDER_UI_REGISTRY[selectedProvider]?.label
const slug = PROVIDER_UI_REGISTRY[selectedProvider]?.docsSlug || selectedProvider
Plus a new file with all the config:
export const PROVIDER_UI_REGISTRY = {
"zoo-gateway": {
label: "Zoo Gateway",
docsSlug: "zoo-gateway",
modelConfig: { field: "zooGatewayModelId", default: zooGatewayDefaultModelId },
},
...
} satisfies Partial<Record<ProviderName, {
label: string
docsSlug?: string
modelConfig?: { field: keyof ProviderSettings; default?: string }
}>>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@webview-ui/src/i18n/locales/fr/settings.json`:
- Line 912: Change the informal French register in validation.zooGatewaySignIn
to match the formal register used by providers.zooGateway.signInDescription;
replace "Connecte-toi à Zoo Code pour utiliser Zoo Gateway." with the formal
phrasing (e.g., "Connectez-vous à Zoo Code pour utiliser Zoo Gateway.") so both
keys use "vous" for consistency.
🪄 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: d99f9f3c-805c-400c-8a54-3b3d00a7d90a
📒 Files selected for processing (22)
webview-ui/src/components/settings/ApiOptions.tsxwebview-ui/src/components/settings/utils/providerModelConfig.tswebview-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
✅ Files skipped from review due to trivial changes (9)
- webview-ui/src/i18n/locales/it/settings.json
- webview-ui/src/i18n/locales/zh-TW/settings.json
- webview-ui/src/i18n/locales/tr/settings.json
- webview-ui/src/i18n/locales/ca/settings.json
- webview-ui/src/i18n/locales/nl/settings.json
- webview-ui/src/i18n/locales/de/settings.json
- webview-ui/src/i18n/locales/en/settings.json
- webview-ui/src/i18n/locales/ko/settings.json
- webview-ui/src/i18n/locales/ru/settings.json
🚧 Files skipped from review as they are similar to previous changes (9)
- webview-ui/src/i18n/locales/pl/settings.json
- webview-ui/src/utils/tests/validate.spec.ts
- webview-ui/src/i18n/locales/vi/settings.json
- webview-ui/src/i18n/locales/hi/settings.json
- webview-ui/src/i18n/locales/id/settings.json
- webview-ui/src/i18n/locales/pt-BR/settings.json
- webview-ui/src/i18n/locales/zh-CN/settings.json
- webview-ui/src/i18n/locales/es/settings.json
- webview-ui/src/i18n/locales/ja/settings.json
Co-authored-by: Cursor <cursoragent@cursor.com>
… fetch - Stop reassigning RouterProvider.client; thread Zoo enrichment headers through openAiHeaders so a single OpenAI client is used. - Replace npm_package_version (never populated at extension runtime) with Package.version from the shared package shim. - Default the model list to [] on a structurally broken response so we log and recover instead of crashing on response.data.data being undefined. - Bypass inFlightRefresh de-duplication for zoo-gateway: a refresh triggered after sign-out/sign-in must not return the previous user's in-flight response. - Add fetcher unit tests covering auth header, timeout, error redaction, and bad-response handling. Co-authored-by: Cursor <cursoragent@cursor.com>
The downstream stack (settings-ui) calls getCachedZooCodeToken and clearZooCodeToken from the auth handler. CI on stacked PRs merges base into head so this spec runs against the cached-token-aware handler; expand the auth module mock so the auth guard test exercises the real throw path instead of vitest's missing-mock-export error. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
- Resolve ModelPicker serviceUrl from zooCodeBaseUrl so staging/dev environments link to the matching dashboard. - Fall back to getCachedZooCodeToken() in the handler and model fetcher when the profile has not been seeded yet (auth before webview open). Co-authored-by: Cursor <cursoragent@cursor.com>
…-lockfile installs Co-authored-by: Cursor <cursoragent@cursor.com>
…ch 'free' search hint dropped on rebase Co-authored-by: Cursor <cursoragent@cursor.com>
… 4.5 Resolves Sonnet 4.5 from the gateway model catalog instead of a static Vercel slug so test (Bedrock) and live accounts both get a valid default. Reassigns stale profile model IDs when they are not in the catalog. Co-authored-by: Cursor <cursoragent@cursor.com>
Exports pickZooGatewayDefaultModelId so the helper is unit-testable and adds component tests for the auto-default useEffect (no-op while catalog loads, auto-pick on empty profile, repair stale id, no-op when valid). Co-authored-by: Cursor <cursoragent@cursor.com>
…rings Co-authored-by: Cursor <cursoragent@cursor.com>
… auth Co-authored-by: Cursor <cursoragent@cursor.com>
Move the zoo-gateway sign-in error out of the shared form-validation effect in ApiOptions and into ZooGateway.tsx, where it renders inline via ApiErrorMessage. ApiOptions can then drop zooCodeIsAuthenticated from its useEffect dependency list, and validateApiConfigurationExcludingModelErrors short-circuits zoo-gateway entirely. Also rename the inline isSonnet45ModelId helper to isClaudeSonnetModelId and let pickZooGatewayDefaultModelId express the version-priority order directly, so the helper has no version baked into its name. Co-authored-by: Cursor <cursoragent@cursor.com>
The settings-form short-circuit for zoo-gateway also bypassed the organization PROVIDER_NOT_ALLOWED check, so a workspace that disallows zoo-gateway could not surface that error. Scope the short-circuit to the keys/sign-in check only and let the org allowlist check run for every provider. Drop the quoted CTA from zooGatewaySignIn in all 18 locales: the sign-in button is rendered immediately below the inline error, so a duplicate label was just a drift hazard. Co-authored-by: Cursor <cursoragent@cursor.com>
…Options Lift the inline PROVIDER_MODEL_CONFIG map and the docs-slug lookup out of ApiOptions.tsx into webview-ui/src/components/settings/utils/providerModelConfig.ts behind getProviderModelConfig and getProviderDocsSlug helpers. ApiOptions now reads provider-specific model fields, defaults, and docs slugs through those helpers, leaving the component focused on rendering. Co-authored-by: Cursor <cursoragent@cursor.com>
validation.zooGatewaySignIn used "Connecte-toi" while providers.zooGateway.signInDescription uses "Connectez-vous". Use vous consistently across both strings. Co-authored-by: Cursor <cursoragent@cursor.com>
JamesRobert20
force-pushed
the
feat/zoo-gateway-settings-ui
branch
from
5aa42cc to
2fbf902
Compare
4 participants
Summary
zooGatewayModelIdwhen switching to Zoo GatewayPart 2 of the Zoo Gateway stack. Depends on #344.
Test plan
Made with Cursor
Summary by CodeRabbit