feat(zoo-gateway): provider types, handler, and model fetcher#344
Open
JamesRobert20 wants to merge 6 commits into
Open
feat(zoo-gateway): provider types, handler, and model fetcher#344JamesRobert20 wants to merge 6 commits into
JamesRobert20 wants to merge 6 commits into
Conversation
JamesRobert20
requested review from
edelauna,
hannesrudolph,
navedmerchant and
taltas
as code owners
2 tasks
Contributor
📝 WalkthroughWalkthroughAdds a new dynamic provider "zoo-gateway": provider types and defaults, a /models fetcher with auth and Zod validation, cache/refresh logic that skips persistent caching for per-user lists, a ZooGatewayHandler for streaming and non-streaming completions, and UI wiring/tests. ChangesZoo Gateway Provider Integration
Sequence Diagram(s)sequenceDiagram
participant Requestor
participant ModelCache
participant ZooFetcher
participant ZooAPI
Requestor->>ModelCache: getModels(zoo-gateway, apiKey?, baseUrl?)
ModelCache-->>ZooFetcher: fetchModelsFromProvider(zoo-gateway)
ZooFetcher->>ZooAPI: GET /models (timeout, optional Bearer)
ZooAPI-->>ZooFetcher: models[]
ZooFetcher->>ModelCache: parsed ModelInfo (no persistent write)
ModelCache-->>Requestor: models map
sequenceDiagram
participant Client
participant ZooGatewayHandler
participant OpenAIClient
participant ZooAPI
Client->>ZooGatewayHandler: createMessage(system, messages, metadata)
ZooGatewayHandler->>OpenAIClient: stream chat completion (converted messages, headers)
OpenAIClient->>ZooAPI: POST /chat/completions (stream)
ZooAPI-->>OpenAIClient: stream events (deltas, usage)
OpenAIClient-->>ZooGatewayHandler: events
ZooGatewayHandler-->>Client: text chunks, tool-call chunks, usage (tokens, cache fields, cost)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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/__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. 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 |
This was referenced May 27, 2026
Co-authored-by: Cursor <cursoragent@cursor.com>
![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: 1
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)
174-185:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not share in-flight refresh promises for auth-scoped Zoo Gateway models.
Line 182 deduplicates by provider only. For
zoo-gateway, concurrent calls from different auth sessions can receive the same in-flight result, leaking cross-user model visibility.Proposed fix
const shouldSkipCache = provider === "zoo-gateway" + const canShareInFlight = !shouldSkipCache - const existingRequest = inFlightRefresh.get(provider) - if (existingRequest) { - return existingRequest + if (canShareInFlight) { + const existingRequest = inFlightRefresh.get(provider) + if (existingRequest) { + return existingRequest + } } @@ - inFlightRefresh.set(provider, refreshPromise) + if (canShareInFlight) { + inFlightRefresh.set(provider, refreshPromise) + }Also applies to: 237-239
🤖 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 174 - 185, The current inFlightRefresh dedupes only by provider (inFlightRefresh.get(provider)), which lets different authenticated sessions share the same in-flight promise for the auth-scoped "zoo-gateway" provider; change the dedupe key to include an auth-scoped identifier when shouldSkipCache is true. Concretely, compute a cacheKey = shouldSkipCache ? `${provider}:${authIdOrToken}` : provider (use the request's existing auth/session identifier already available to getModels — e.g., sessionId, userId, or authToken), then replace uses of inFlightRefresh.get(provider)/set(provider, promise)/delete(provider) with inFlightRefresh.get(cacheKey)/set(cacheKey,...)/delete(cacheKey). Apply this same change at both places noted (the existing block you saw and the 237-239 block) so in-flight promises for "zoo-gateway" are scoped per-auth.
🧹 Nitpick comments (2)
src/api/providers/zoo-gateway.ts (2)
61-70: ⚖️ Poor tradeoffConsider passing headers to parent instead of recreating client.
Overriding
clientvia(this as any)bypasses type safety and creates the OpenAI client twice. IfRouterProviderdoesn't support custom headers in its constructor options, consider extending it to accept adefaultHeadersparameter, which would avoid the double instantiation and the unsafe cast.🤖 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/zoo-gateway.ts` around lines 61 - 70, The code recreates the OpenAI client via (this as any).client = new OpenAI(...) which bypasses types and double-instantiates the client; instead extend RouterProvider to accept a defaultHeaders (or openAiOptions) parameter and pass enrichmentHeaders and options.zooSessionToken into the parent constructor so the existing client instance is created with the correct headers; remove the unsafe cast and the new OpenAI(...) call in ZooGateway (or the class containing this.client) and update the parent constructor signature (RouterProvider) to merge DEFAULT_HEADERS, enrichmentHeaders, and options.openAiHeaders when constructing the OpenAI client.
109-111: ⚡ Quick winSet tool-related parameters only when tools are provided.
When
toolsis undefined,tool_choiceandparallel_tool_callsshould also be omitted. Some OpenAI-compatible APIs may error or behave unexpectedly when these parameters are present without tools.♻️ Proposed fix
const body: OpenAI.Chat.ChatCompletionCreateParams = { model: modelId, messages: openAiMessages, temperature: this.supportsTemperature(modelId) ? (this.options.modelTemperature ?? ZOO_GATEWAY_DEFAULT_TEMPERATURE) : undefined, max_completion_tokens: info.maxTokens, stream: true, stream_options: { include_usage: true }, - tools: this.convertToolsForOpenAI(metadata?.tools), - tool_choice: metadata?.tool_choice, - parallel_tool_calls: metadata?.parallelToolCalls ?? true, + ...(metadata?.tools && { + tools: this.convertToolsForOpenAI(metadata.tools), + tool_choice: metadata.tool_choice, + parallel_tool_calls: metadata.parallelToolCalls ?? true, + }), }🤖 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/zoo-gateway.ts` around lines 109 - 111, When building the request payload in zoo-gateway.ts (around the object using convertToolsForOpenAI), only include tool-related keys when metadata?.tools is present: call convertToolsForOpenAI(metadata.tools) and, if that returns a non-empty array, set tools, tool_choice (from metadata?.tool_choice) and parallel_tool_calls (from metadata?.parallelToolCalls ?? true); otherwise omit tool_choice and parallel_tool_calls entirely. Update the payload construction logic to conditionally add these properties instead of unconditionally setting tool_choice and parallel_tool_calls when tools are undefined.
🤖 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 `@src/api/providers/fetchers/zoo-gateway.ts`:
- Around line 77-79: The code currently falls back to unvalidated
response.data.data after zooGatewayModelsResponseSchema.safeParse fails; change
this to “fail closed” by not using response.data when result.success is false —
either throw or set data to a safe default (e.g., empty array/object) and log
the validation issues from result.error; update the assignment that creates data
(the block that uses zooGatewayModelsResponseSchema.safeParse, result, and
response) so callers only ever iterate/consume validated data (or the empty
default) instead of untrusted response.data.data.
---
Outside diff comments:
In `@src/api/providers/fetchers/modelCache.ts`:
- Around line 174-185: The current inFlightRefresh dedupes only by provider
(inFlightRefresh.get(provider)), which lets different authenticated sessions
share the same in-flight promise for the auth-scoped "zoo-gateway" provider;
change the dedupe key to include an auth-scoped identifier when shouldSkipCache
is true. Concretely, compute a cacheKey = shouldSkipCache ?
`${provider}:${authIdOrToken}` : provider (use the request's existing
auth/session identifier already available to getModels — e.g., sessionId,
userId, or authToken), then replace uses of
inFlightRefresh.get(provider)/set(provider, promise)/delete(provider) with
inFlightRefresh.get(cacheKey)/set(cacheKey,...)/delete(cacheKey). Apply this
same change at both places noted (the existing block you saw and the 237-239
block) so in-flight promises for "zoo-gateway" are scoped per-auth.
---
Nitpick comments:
In `@src/api/providers/zoo-gateway.ts`:
- Around line 61-70: The code recreates the OpenAI client via (this as
any).client = new OpenAI(...) which bypasses types and double-instantiates the
client; instead extend RouterProvider to accept a defaultHeaders (or
openAiOptions) parameter and pass enrichmentHeaders and options.zooSessionToken
into the parent constructor so the existing client instance is created with the
correct headers; remove the unsafe cast and the new OpenAI(...) call in
ZooGateway (or the class containing this.client) and update the parent
constructor signature (RouterProvider) to merge DEFAULT_HEADERS,
enrichmentHeaders, and options.openAiHeaders when constructing the OpenAI
client.
- Around line 109-111: When building the request payload in zoo-gateway.ts
(around the object using convertToolsForOpenAI), only include tool-related keys
when metadata?.tools is present: call convertToolsForOpenAI(metadata.tools) and,
if that returns a non-empty array, set tools, tool_choice (from
metadata?.tool_choice) and parallel_tool_calls (from metadata?.parallelToolCalls
?? true); otherwise omit tool_choice and parallel_tool_calls entirely. Update
the payload construction logic to conditionally add these properties instead of
unconditionally setting tool_choice and parallel_tool_calls when tools are
undefined.
🪄 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: 1a8899a1-e2c0-42e5-8da6-017407719daa
📒 Files selected for processing (9)
packages/types/src/provider-settings.tspackages/types/src/providers/index.tspackages/types/src/providers/zoo-gateway.tssrc/api/index.tssrc/api/providers/fetchers/modelCache.tssrc/api/providers/fetchers/zoo-gateway.tssrc/api/providers/index.tssrc/api/providers/zoo-gateway.tssrc/shared/api.ts
Comment on lines
+77
to
+79
| const result = zooGatewayModelsResponseSchema.safeParse(response.data) | ||
| const data = result.success ? result.data.data : response.data.data | ||
|
|
Contributor
There was a problem hiding this comment.
Avoid processing unvalidated response data after schema failure.
Line 78 falls back to response.data.data when validation fails, which bypasses your schema guarantees. Fail closed (or default to empty) instead of iterating untrusted shape.
Proposed fix
const result = zooGatewayModelsResponseSchema.safeParse(response.data)
- const data = result.success ? result.data.data : response.data.data
+ const data = result.success ? result.data.data : []
if (!result.success) {
console.error(`Zoo Gateway models response is invalid ${JSON.stringify(result.error.format())}`)
+ return models
}🤖 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/zoo-gateway.ts` around lines 77 - 79, The code
currently falls back to unvalidated response.data.data after
zooGatewayModelsResponseSchema.safeParse fails; change this to “fail closed” by
not using response.data when result.success is false — either throw or set data
to a safe default (e.g., empty array/object) and log the validation issues from
result.error; update the assignment that creates data (the block that uses
zooGatewayModelsResponseSchema.safeParse, result, and response) so callers only
ever iterate/consume validated data (or the empty default) instead of untrusted
response.data.data.
… 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>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…RouterName stays exhaustive Co-authored-by: Cursor <cursoragent@cursor.com>
…terName exhaustiveness holds Co-authored-by: Cursor <cursoragent@cursor.com>
JamesRobert20
force-pushed
the
feat/zoo-gateway-types-core
branch
from
f776483 to
dfb1eed
Compare
Co-authored-by: Cursor <cursoragent@cursor.com>
Cover constructor auth guard, base URL resolution, streaming, task/mode headers, temperature, cache breakpoints, tool calls, and completePrompt. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/api/providers/__tests__/zoo-gateway.spec.ts (1)
183-197: ⚡ Quick winConsider fully consuming the stream in assertion tests.
The test only calls
.next()once to trigger the request, but doesn't consume the full stream. This pattern appears in several tests (lines 186, 205, 218, 235). While it's sufficient to verify the request shape, fully consuming the stream would also verify that:
- The stream completes without errors
- No unexpected chunks are emitted
- Cleanup/finalization logic runs correctly
Consider using a pattern like:
const chunks = [] for await (const chunk of handler.createMessage(...)) { chunks.push(chunk) } expect(mockCreate).toHaveBeenCalledWith(...)This is a recommended improvement for more robust test coverage.
🤖 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/__tests__/zoo-gateway.spec.ts` around lines 183 - 197, The tests call ZooGatewayHandler.createMessage(...) and only invoke .next() which doesn't fully consume the async iterable; update the test cases (the spec that uses new ZooGatewayHandler(mockOptions) and mockCreate) to fully iterate the returned async generator (e.g., for-await ... push chunks into an array) so the stream is drained and completes before assertions; then assert mockCreate was called with the expected headers and optionally assert the collected chunks/that iteration completes without throwing to ensure cleanup/finalization runs.
🤖 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.
Nitpick comments:
In `@src/api/providers/__tests__/zoo-gateway.spec.ts`:
- Around line 183-197: The tests call ZooGatewayHandler.createMessage(...) and
only invoke .next() which doesn't fully consume the async iterable; update the
test cases (the spec that uses new ZooGatewayHandler(mockOptions) and
mockCreate) to fully iterate the returned async generator (e.g., for-await ...
push chunks into an array) so the stream is drained and completes before
assertions; then assert mockCreate was called with the expected headers and
optionally assert the collected chunks/that iteration completes without throwing
to ensure cleanup/finalization runs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ac3b4bc2-5058-4caf-8700-abd30d236ee0
📒 Files selected for processing (1)
src/api/providers/__tests__/zoo-gateway.spec.ts
Contributor
Conflict Resolution PatchHi @JamesRobert20 — I've prepared a rebased version of this PR against current main (4d71e5f) that resolves the merge conflicts caused by #319 (opencode-go). Branch: What was done
Verification
How to applyYou can cherry-pick the resolution or rebase your branch onto this one: git fetch proyectoauraorg feat/zoo-gateway-types-core-rebased
git rebase proyectoauraorg/feat/zoo-gateway-types-core-rebasedHappy to help with any further adjustments! |
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
Part 1 of the Zoo Gateway stack (split from #229).
Test plan
Made with Cursor
Summary by CodeRabbit
New Features
Behavior
Tests