feat(providers): add Baseten Model APIs preset - #653
Conversation
📝 WalkthroughWalkthroughThis PR adds bounded, registry-driven provider model discovery, transport-aware registry matching, and the Baseten Model APIs preset. It integrates discovery into catalog loading, API-key validation, routing, management probes, tests, and multilingual documentation. ChangesProvider discovery and registry integration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs-site/src/content/docs/ru/reference/configuration.md (1)
166-166: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRussian docs missing the new collision-preservation
baseUrlbehavior that zh-cn received.
docs-site/src/content/docs/zh-cn/reference/configuration.mdwas updated in this same PR (lines 138 and 184-190) to document that newly-added, collision-protected fixed API-key presets (like Baseten) preserve an existing same-named custom provider'sbaseUrl/adapter instead of redirecting it to the new registry host. This Russian page'sbaseUrlrow (line 166) and its "fixed provider endpoints" section (unchanged lines 209-278) were not updated to match, leaving ru readers with an incomplete/outdated description ofbaseUrloverride precedence — directly contradicting the path-instruction requirement that "translated locale pages ... are not left contradicting the English source."As per path instructions,
docs-site/**guidance requires that "translated locale pages (ja, ko, ru, zh-cn) are not left contradicting the English source."📝 Suggested ru addition (adapt zh-cn wording)
-| `baseUrl` | `string` | Базовый URL вышестоящего API. | +| `baseUrl` | `string` | Базовый URL вышестоящего API. Большинство встроенных provider с фиксированными конечными точками игнорируют несовпадающий адрес; новые пресеты с ключевой аутентификацией, включающие защиту от коллизий, сохраняют цель ранее существовавшего custom provider с тем же именем. См. [Фиксированные конечные точки провайдера](`#фиксированные-конечные-точки-провайдера`). |And extend the "fixed endpoint" section (wherever it lives in the unchanged 209-278 range) with a fourth bullet mirroring zh-cn lines 189-190.
🤖 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 `@docs-site/src/content/docs/ru/reference/configuration.md` at line 166, Update the Russian configuration documentation to match the English and zh-cn descriptions of collision-protected fixed API-key presets: clarify in the baseUrl documentation that an existing same-named custom provider preserves its baseUrl and adapter, then add the corresponding fourth bullet to the fixed provider endpoints section. Keep the wording consistent with the existing Russian terminology and zh-cn guidance.Source: Path instructions
🤖 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 `@docs-site/src/content/docs/guides/providers.md`:
- Around line 222-226: Remove the blank line inside the synchronized Baseten
blockquotes at docs-site/src/content/docs/guides/providers.md lines 222-226,
docs-site/src/content/docs/ja/guides/providers.md lines 156-160, and
docs-site/src/content/docs/ko/guides/providers.md lines 156-160, keeping each
blockquote contiguous.
In `@docs-site/src/content/docs/ru/getting-started/quickstart.md`:
- Around line 16-17: Update the provider-selection sentence to use the correct
Russian numeral agreement, changing “один из 61 встроенного пресета” to “один из
61 встроенных пресетов” while preserving the rest of the quickstart content.
In `@src/providers/model-discovery.ts`:
- Around line 273-280: Add documentation to the predicate union declaration in
the registry module describing the `containsAny` and `containsAll` semantics:
string upstream values use substring matching, while array values use exact
element matching. Keep the existing comparison behavior in the `containsAny` and
`containsAll` branches of model discovery unchanged; only make this contract
clear for registry authors.
---
Outside diff comments:
In `@docs-site/src/content/docs/ru/reference/configuration.md`:
- Line 166: Update the Russian configuration documentation to match the English
and zh-cn descriptions of collision-protected fixed API-key presets: clarify in
the baseUrl documentation that an existing same-named custom provider preserves
its baseUrl and adapter, then add the corresponding fourth bullet to the fixed
provider endpoints section. Keep the wording consistent with the existing
Russian terminology and zh-cn guidance.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6f2e0626-310e-41e7-b32d-b92ab1438d20
📒 Files selected for processing (35)
docs-site/src/content/docs/getting-started/quickstart.mddocs-site/src/content/docs/guides/providers.mddocs-site/src/content/docs/ja/getting-started/quickstart.mddocs-site/src/content/docs/ja/guides/providers.mddocs-site/src/content/docs/ja/reference/configuration.mddocs-site/src/content/docs/ko/getting-started/quickstart.mddocs-site/src/content/docs/ko/guides/providers.mddocs-site/src/content/docs/ko/reference/configuration.mddocs-site/src/content/docs/reference/configuration.mddocs-site/src/content/docs/ru/getting-started/quickstart.mddocs-site/src/content/docs/ru/guides/providers.mddocs-site/src/content/docs/ru/reference/configuration.mddocs-site/src/content/docs/zh-cn/getting-started/quickstart.mddocs-site/src/content/docs/zh-cn/guides/providers.mddocs-site/src/content/docs/zh-cn/reference/configuration.mdsrc/codex/catalog/parsing.tssrc/codex/catalog/provider-fetch.tssrc/oauth/index.tssrc/oauth/key-providers.tssrc/oauth/login-cli.tssrc/providers/derive.tssrc/providers/model-discovery.tssrc/providers/registry.tssrc/router.tssrc/server/auth-cors.tssrc/server/management/provider-routes.tssrc/server/management/shared.tsstructure/01_runtime.mdtests/baseten-provider.test.tstests/codex-catalog.test.tstests/fixtures/baseten-models.jsontests/fixtures/provider-model-discovery.jsontests/provider-model-discovery-contract.test.tstests/provider-registry-parity.test.tstests/umans-provider.test.ts
9cdb84c to
7faf2e4
Compare
7faf2e4 to
124f427
Compare
|
Review follow-up for the outside-diff Russian collision-preservation finding: that review was anchored to the old |
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/codex/catalog/provider-fetch.ts (1)
219-239: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPrioritize canonical capability flags before truncation.
If
capabilities/featuresalready contributes 32 values, the latersupports_toolsandsupports_reasoningadditions are removed byslice(0, 32). A valid upstream boolean can therefore be omitted from the catalog. Add canonical flags before untrusted labels, or cap generic additions while reserving capacity.Proposed fix
function modelCapabilities(item: ProviderModelsApiItem): string[] | undefined { const metadata = plainRecord(item.metadata); const metadataCapabilities = metadata?.capabilities; const capabilityRecord = plainRecord(metadataCapabilities) ?? plainRecord(item.capabilities); const out = new Set<string>(); + for (const field of ["supports_tools", "supports_tool_calling", "supports_function_calling"] as const) { + if (item[field] === true) out.add("tools"); + } + for (const field of ["supports_reasoning", "reasoning"] as const) { + if (item[field] === true) out.add("reasoning"); + } for (const list of [item.capabilities, item.features, item.supported_features, metadataCapabilities]) { - for (const capability of normalizedStringList(list) ?? []) out.add(capability); + for (const capability of normalizedStringList(list) ?? []) { + if (out.size >= 32) break; + out.add(capability); + } }🤖 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/codex/catalog/provider-fetch.ts` around lines 219 - 239, Update the capability aggregation flow so canonical boolean-derived values from supports_tools/supports_tool_calling/supports_function_calling and supports_reasoning/reasoning are added before generic capabilities and metadata labels. Preserve the 32-value limit while ensuring these canonical “tools” and “reasoning” values are retained when present, then add untrusted labels only with remaining capacity.
🤖 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 `@docs-site/src/content/docs/ja/guides/providers.md`:
- Around line 156-160: Remove the blank separator between the consecutive
Baseten and Tencent blockquotes in
docs-site/src/content/docs/ja/guides/providers.md lines 156-160 and
docs-site/src/content/docs/ko/guides/providers.md lines 156-160, keeping the
blockquote lines contiguous to satisfy markdownlint MD028.
---
Outside diff comments:
In `@src/codex/catalog/provider-fetch.ts`:
- Around line 219-239: Update the capability aggregation flow so canonical
boolean-derived values from
supports_tools/supports_tool_calling/supports_function_calling and
supports_reasoning/reasoning are added before generic capabilities and metadata
labels. Preserve the 32-value limit while ensuring these canonical “tools” and
“reasoning” values are retained when present, then add untrusted labels only
with remaining capacity.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a890a1e3-1a47-4f3d-a121-0a23983f7ff4
📒 Files selected for processing (24)
docs-site/src/content/docs/getting-started/quickstart.mddocs-site/src/content/docs/guides/providers.mddocs-site/src/content/docs/ja/getting-started/quickstart.mddocs-site/src/content/docs/ja/guides/providers.mddocs-site/src/content/docs/ja/reference/configuration.mddocs-site/src/content/docs/ko/getting-started/quickstart.mddocs-site/src/content/docs/ko/guides/providers.mddocs-site/src/content/docs/ko/reference/configuration.mddocs-site/src/content/docs/reference/configuration.mddocs-site/src/content/docs/ru/getting-started/quickstart.mddocs-site/src/content/docs/ru/guides/providers.mddocs-site/src/content/docs/ru/reference/configuration.mddocs-site/src/content/docs/zh-cn/getting-started/quickstart.mddocs-site/src/content/docs/zh-cn/guides/providers.mdsrc/codex/catalog/provider-fetch.tssrc/oauth/index.tssrc/providers/registry.tssrc/server/management/provider-routes.tsstructure/01_runtime.mdtests/baseten-provider.test.tstests/fixtures/baseten-models.jsontests/provider-connection-test.test.tstests/provider-model-discovery-contract.test.tstests/provider-registry-parity.test.ts
124f427 to
f345775
Compare
Resolve conflicts with lidge-jun#617 array support by teaching extractProviderModelItems to accept top-level /models arrays while retaining bounded reads, redirect:error, destination policy, and eligibility filtering on the management probe.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@docs-site/src/content/docs/ja/guides/providers.md`:
- Around line 156-160: Remove the blank separator between consecutive blockquote
paragraphs in docs-site/src/content/docs/ja/guides/providers.md lines 156-160,
docs-site/src/content/docs/ko/guides/providers.md lines 156-160, and
docs-site/src/content/docs/ru/guides/providers.md lines 167-171, preserving the
separate blockquote paragraphs.
In `@structure/01_runtime.md`:
- Around line 80-81: Update the discovery-limit guidance in
structure/01_runtime.md lines 80-81,
docs-site/src/content/docs/ru/reference/configuration.md lines 330-334, and
docs-site/src/content/docs/zh-cn/reference/configuration.md lines 287-290 to
state the generic 4 MiB response and 2,000 raw-row defaults, plus the Baseten
preset overrides of 1 MiB and 256 models at every site. Keep all translated
pages synchronized with the runtime behavior.
In `@tests/baseten-provider.test.ts`:
- Around line 273-280: Update the adapter-mismatch test around routeModel and
basetenConfig so it supplies a non-canonical custom baseUrl while retaining
adapter: "anthropic". Assert that customAdapter preserves this destination,
ensuring registry retargeting cannot pass the test by leaving the canonical URL
unchanged; keep the existing provider and authMode assertions.
- Around line 217-225: Update the registry assertion’s reasoning configuration
in the toMatchObject block around noReasoningModels or the model-specific
reasoning map to explicitly include moonshotai/Kimi-K2.6. Ensure the existing
Kimi request assertion in the Kimi route test is backed by that registry entry
and continues suppressing reasoning_effort.
In `@tests/provider-model-discovery-contract.test.ts`:
- Around line 34-69: Ensure discovery-cache cleanup is uniform across temporary
registry mutations. In tests/provider-model-discovery-contract.test.ts lines
34-69, update withRegistryDiscovery to call clearModelCache(providerId) in
finally and export the shared helper as needed; in
tests/provider-connection-test.test.ts lines 153-182, replace the inline
together discovery mutate/restore logic with that helper so the temporary state
is restored and cached rows are cleared before subsequent probes.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a4308713-4f36-49b7-adae-bd345c8a28b0
📒 Files selected for processing (36)
docs-site/src/content/docs/getting-started/quickstart.mddocs-site/src/content/docs/guides/providers.mddocs-site/src/content/docs/ja/getting-started/quickstart.mddocs-site/src/content/docs/ja/guides/providers.mddocs-site/src/content/docs/ja/reference/configuration.mddocs-site/src/content/docs/ko/getting-started/quickstart.mddocs-site/src/content/docs/ko/guides/providers.mddocs-site/src/content/docs/ko/reference/configuration.mddocs-site/src/content/docs/reference/configuration.mddocs-site/src/content/docs/ru/getting-started/quickstart.mddocs-site/src/content/docs/ru/guides/providers.mddocs-site/src/content/docs/ru/reference/configuration.mddocs-site/src/content/docs/zh-cn/getting-started/quickstart.mddocs-site/src/content/docs/zh-cn/guides/providers.mddocs-site/src/content/docs/zh-cn/reference/configuration.mdsrc/codex/catalog/parsing.tssrc/codex/catalog/provider-fetch.tssrc/oauth/index.tssrc/oauth/key-providers.tssrc/oauth/login-cli.tssrc/providers/derive.tssrc/providers/model-discovery.tssrc/providers/registry.tssrc/router.tssrc/server/auth-cors.tssrc/server/management/provider-routes.tssrc/server/management/shared.tsstructure/01_runtime.mdtests/baseten-provider.test.tstests/codex-catalog.test.tstests/fixtures/baseten-models.jsontests/fixtures/provider-model-discovery.jsontests/provider-connection-test.test.tstests/provider-model-discovery-contract.test.tstests/provider-registry-parity.test.tstests/umans-provider.test.ts
| > **Baseten の対象範囲:** このプリセットは Baseten の共有 [Model APIs](https://docs.baseten.co/inference/model-apis/overview) | ||
| > のみを対象とします。ローカル利用では個人の [API キー](https://docs.baseten.co/organization/api-keys)を、 | ||
| > 共有/本番利用では **Call Model APIs** 権限を持つチームキーを使用してください。専用 Truss `predict` | ||
| > エンドポイントはホストとスキーマが異なるため、このプリセットではルーティングされません。 | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove blank separators between consecutive blockquotes.
These blank lines trigger markdownlint MD028 and should be removed while preserving the separate blockquote paragraphs.
docs-site/src/content/docs/ja/guides/providers.md#L156-L160: remove Line 160.docs-site/src/content/docs/ko/guides/providers.md#L156-L160: remove Line 160.docs-site/src/content/docs/ru/guides/providers.md#L167-L171: remove Line 171.
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 160-160: Blank line inside blockquote
(MD028, no-blanks-blockquote)
📍 Affects 3 files
docs-site/src/content/docs/ja/guides/providers.md#L156-L160(this comment)docs-site/src/content/docs/ko/guides/providers.md#L156-L160docs-site/src/content/docs/ru/guides/providers.md#L167-L171
🤖 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 `@docs-site/src/content/docs/ja/guides/providers.md` around lines 156 - 160,
Remove the blank separator between consecutive blockquote paragraphs in
docs-site/src/content/docs/ja/guides/providers.md lines 156-160,
docs-site/src/content/docs/ko/guides/providers.md lines 156-160, and
docs-site/src/content/docs/ru/guides/providers.md lines 167-171, preserving the
separate blockquote paragraphs.
Source: Linters/SAST tools
| config. A response is rejected before caching when it exceeds 4 MiB, contains more than 2,000 raw | ||
| rows, has a malformed OpenAI list envelope, or includes an invalid model id. Tests use fixtures and |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep discovery-limit documentation consistent across locales.
The generic 4 MiB / 2,000-row limits are documented without the Baseten preset’s stricter 1 MiB response and 256-model caps. Clarify the generic defaults and add the Baseten override at every affected site.
structure/01_runtime.md#L80-L81: document the Baseten-specific 1 MiB / 256-model limits alongside the generic thresholds.docs-site/src/content/docs/ru/reference/configuration.md#L330-L334: add the same Baseten-specific limits to the Russian fallback guidance.docs-site/src/content/docs/zh-cn/reference/configuration.md#L287-L290: add the same Baseten-specific limits to the Chinese fallback guidance.
As per path instructions, translated Astro/Starlight pages must stay synchronized with actual runtime behavior.
📍 Affects 3 files
structure/01_runtime.md#L80-L81(this comment)docs-site/src/content/docs/ru/reference/configuration.md#L330-L334docs-site/src/content/docs/zh-cn/reference/configuration.md#L287-L290
🤖 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 `@structure/01_runtime.md` around lines 80 - 81, Update the discovery-limit
guidance in structure/01_runtime.md lines 80-81,
docs-site/src/content/docs/ru/reference/configuration.md lines 330-334, and
docs-site/src/content/docs/zh-cn/reference/configuration.md lines 287-290 to
state the generic 4 MiB response and 2,000 raw-row defaults, plus the Baseten
preset overrides of 1 MiB and 256 models at every site. Keep all translated
pages synchronized with the runtime behavior.
Source: Path instructions
| const kimiRoute = routeModel(basetenConfig(), "baseten/moonshotai/Kimi-K2.6"); | ||
| const kimiBody = JSON.parse(String(createOpenAIChatAdapter(kimiRoute.provider).buildRequest({ | ||
| modelId: kimiRoute.modelId, | ||
| context: { messages: [{ role: "user", content: "ping", timestamp: 0 }] }, | ||
| stream: false, | ||
| options: { reasoning: "high" }, | ||
| }).body)) as Record<string, unknown>; | ||
| // K2.6 uses chat_template_args instead of reasoning_effort; do not send an ignored claim. | ||
| expect(kimiBody).not.toHaveProperty("reasoning_effort"); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Which registry field suppresses reasoning_effort for Kimi-K2.6?
fd -t f 'registry.ts' src/providers --exec rg -n -C4 'Kimi-K2\.6|noReasoningModels'Repository: lidge-jun/opencodex
Length of output: 6716
Pin moonshotai/Kimi-K2.6 in the registry assertion
tests/baseten-provider.test.ts:67-94 should include the moonshotai/Kimi-K2.6 entry in the toMatchObject block that governs reasoning behavior; otherwise tests/baseten-provider.test.ts:217-225 only proves reasoning_effort is omitted indirectly, and a later registry edit could make the test pass for the wrong reason. Add the model to the field that actually suppresses reasoning_effort (noReasoningModels or the model-specific reasoning map) so the contract is explicit.
🤖 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 `@tests/baseten-provider.test.ts` around lines 217 - 225, Update the registry
assertion’s reasoning configuration in the toMatchObject block around
noReasoningModels or the model-specific reasoning map to explicitly include
moonshotai/Kimi-K2.6. Ensure the existing Kimi request assertion in the Kimi
route test is backed by that registry entry and continues suppressing
reasoning_effort.
Source: Path instructions
| const customAdapter = routeModel(basetenConfig({ | ||
| adapter: "anthropic", | ||
| }), "baseten/custom-model"); | ||
| expect(customAdapter.provider).toMatchObject({ | ||
| adapter: "anthropic", | ||
| baseUrl: "https://inference.baseten.co/v1", | ||
| authMode: "key", | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
The adapter-mismatch case asserts a value it never varied — it cannot fail.
basetenConfig({ adapter: "anthropic" }) leaves baseUrl at the canonical https://inference.baseten.co/v1, so the Line 278 expectation is satisfied whether or not registry retargeting is suppressed. Only the first case (Lines 264-271) actually exercises destination preservation. Vary the destination in the adapter case too, so a regression that re-pins a transport-mismatched provider to the registry base URL is caught.
♻️ Make the adapter case non-tautological
const customAdapter = routeModel(basetenConfig({
adapter: "anthropic",
+ baseUrl: "https://custom.example/anthropic",
}), "baseten/custom-model");
expect(customAdapter.provider).toMatchObject({
adapter: "anthropic",
- baseUrl: "https://inference.baseten.co/v1",
+ baseUrl: "https://custom.example/anthropic",
authMode: "key",
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const customAdapter = routeModel(basetenConfig({ | |
| adapter: "anthropic", | |
| }), "baseten/custom-model"); | |
| expect(customAdapter.provider).toMatchObject({ | |
| adapter: "anthropic", | |
| baseUrl: "https://inference.baseten.co/v1", | |
| authMode: "key", | |
| }); | |
| const customAdapter = routeModel(basetenConfig({ | |
| adapter: "anthropic", | |
| baseUrl: "https://custom.example/anthropic", | |
| }), "baseten/custom-model"); | |
| expect(customAdapter.provider).toMatchObject({ | |
| adapter: "anthropic", | |
| baseUrl: "https://custom.example/anthropic", | |
| authMode: "key", | |
| }); |
🤖 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 `@tests/baseten-provider.test.ts` around lines 273 - 280, Update the
adapter-mismatch test around routeModel and basetenConfig so it supplies a
non-canonical custom baseUrl while retaining adapter: "anthropic". Assert that
customAdapter preserves this destination, ensuring registry retargeting cannot
pass the test by leaving the canonical URL unchanged; keep the existing provider
and authMode assertions.
| async function withTogetherDiscovery<T>( | ||
| spec: ProviderModelDiscoverySpec, | ||
| run: () => Promise<T> | T, | ||
| ): Promise<T> { | ||
| const entry = togetherEntry(); | ||
| const original = entry.modelDiscovery; | ||
| const originalPreserveCustomDestination = entry.preserveCustomDestination; | ||
| entry.modelDiscovery = spec; | ||
| entry.preserveCustomDestination = true; | ||
| try { | ||
| return await run(); | ||
| } finally { | ||
| if (original === undefined) delete entry.modelDiscovery; | ||
| else entry.modelDiscovery = original; | ||
| if (originalPreserveCustomDestination === undefined) delete entry.preserveCustomDestination; | ||
| else entry.preserveCustomDestination = originalPreserveCustomDestination; | ||
| clearModelCache("together"); | ||
| } | ||
| } | ||
|
|
||
| async function withRegistryDiscovery<T>( | ||
| providerId: string, | ||
| spec: ProviderModelDiscoverySpec, | ||
| run: () => Promise<T> | T, | ||
| ): Promise<T> { | ||
| const entry = PROVIDER_REGISTRY.find(row => row.id === providerId); | ||
| if (!entry) throw new Error(`missing ${providerId} registry entry`); | ||
| const original = entry.modelDiscovery; | ||
| entry.modelDiscovery = spec; | ||
| try { | ||
| return await run(); | ||
| } finally { | ||
| if (original === undefined) delete entry.modelDiscovery; | ||
| else entry.modelDiscovery = original; | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Shared root cause: hand-rolled save/restore of PROVIDER_REGISTRY.modelDiscovery without a uniform discovery-cache reset. Three separate blocks now mutate the same registry singleton, and only withTogetherDiscovery clears cached rows for the provider it perturbed — so whether a temporary contract leaks into a later test depends on which block a given test happened to use.
tests/provider-model-discovery-contract.test.ts#L34-L69: addclearModelCache(providerId)towithRegistryDiscovery'sfinally, matchingwithTogetherDiscovery(Line 50), and export/reuse one helper.tests/provider-connection-test.test.ts#L153-L182: replace the inline mutate/restore block with that shared helper so thetogetherdiscovery state is reset before the top-level-array probe at Lines 202-213 runs.
📍 Affects 2 files
tests/provider-model-discovery-contract.test.ts#L34-L69(this comment)tests/provider-connection-test.test.ts#L153-L182
🤖 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 `@tests/provider-model-discovery-contract.test.ts` around lines 34 - 69, Ensure
discovery-cache cleanup is uniform across temporary registry mutations. In
tests/provider-model-discovery-contract.test.ts lines 34-69, update
withRegistryDiscovery to call clearModelCache(providerId) in finally and export
the shared helper as needed; in tests/provider-connection-test.test.ts lines
153-182, replace the inline together discovery mutate/restore logic with that
helper so the temporary state is restored and cached rows are cleared before
subsequent probes.
f345775 to
879efb2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server/auth-cors.ts (1)
179-182: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse the shared admin-token resolver.
Line 179 accepts only the environment token, while
src/oauth/login-cli.tsLine 13 sends the file-backed fallback fromconfiguredAdminToken(). With noOPENCODEX_ADMIN_AUTH_TOKENbut a configured token file, proxy-update requests send a token the server rejects. Delegate this helper toconfiguredAdminToken()and add a file-only-token regression test.Proposed fix
+import { configuredAdminToken } from "../lib/admin-secrets"; + export function configuredAdminAuthToken(): string | undefined { - const token = process.env.OPENCODEX_ADMIN_AUTH_TOKEN?.trim(); - return token || undefined; + return configuredAdminToken() ?? undefined; }As per path instructions, “A behavior change in src/ should come with a focused regression test near the existing tests for that subsystem.”
🤖 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/server/auth-cors.ts` around lines 179 - 182, Update configuredAdminAuthToken() to delegate to the shared configuredAdminToken() resolver so environment and file-backed tokens use identical precedence. Add a focused regression test near the existing auth/CORS tests covering a configured token file when OPENCODEX_ADMIN_AUTH_TOKEN is absent, and assert that the file token is returned.Source: Path instructions
🤖 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 `@docs-site/src/content/docs/zh-cn/guides/providers.md`:
- Around line 147-151: Update the Baseten Truss sentence in the Chinese provider
documentation to describe dedicated predict endpoints as using different hosts
and request schemas/formats, not different protocols. Preserve the surrounding
wording and keep the blank separator before the Tencent callout unchanged.
In `@src/providers/model-discovery.ts`:
- Around line 87-93: Update the path validation in the spec.path guard to reject
any path segment equal to "..", including repeated or embedded traversal
segments such as "../../internal/models". Preserve acceptance of ordinary
relative paths like "models" and the existing scheme, protocol-relative, query,
fragment, and length checks.
---
Outside diff comments:
In `@src/server/auth-cors.ts`:
- Around line 179-182: Update configuredAdminAuthToken() to delegate to the
shared configuredAdminToken() resolver so environment and file-backed tokens use
identical precedence. Add a focused regression test near the existing auth/CORS
tests covering a configured token file when OPENCODEX_ADMIN_AUTH_TOKEN is
absent, and assert that the file token is returned.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bb92fa4f-53dd-4d98-9d51-77b07ecba819
📒 Files selected for processing (36)
docs-site/src/content/docs/getting-started/quickstart.mddocs-site/src/content/docs/guides/providers.mddocs-site/src/content/docs/ja/getting-started/quickstart.mddocs-site/src/content/docs/ja/guides/providers.mddocs-site/src/content/docs/ja/reference/configuration.mddocs-site/src/content/docs/ko/getting-started/quickstart.mddocs-site/src/content/docs/ko/guides/providers.mddocs-site/src/content/docs/ko/reference/configuration.mddocs-site/src/content/docs/reference/configuration.mddocs-site/src/content/docs/ru/getting-started/quickstart.mddocs-site/src/content/docs/ru/guides/providers.mddocs-site/src/content/docs/ru/reference/configuration.mddocs-site/src/content/docs/zh-cn/getting-started/quickstart.mddocs-site/src/content/docs/zh-cn/guides/providers.mddocs-site/src/content/docs/zh-cn/reference/configuration.mdsrc/codex/catalog/parsing.tssrc/codex/catalog/provider-fetch.tssrc/oauth/index.tssrc/oauth/key-providers.tssrc/oauth/login-cli.tssrc/providers/derive.tssrc/providers/model-discovery.tssrc/providers/registry.tssrc/router.tssrc/server/auth-cors.tssrc/server/management/provider-routes.tssrc/server/management/shared.tsstructure/01_runtime.mdtests/baseten-provider.test.tstests/codex-catalog.test.tstests/fixtures/baseten-models.jsontests/fixtures/provider-model-discovery.jsontests/provider-connection-test.test.tstests/provider-model-discovery-contract.test.tstests/provider-registry-parity.test.tstests/umans-provider.test.ts
| > **Baseten 范围：**该预设仅覆盖 Baseten 的共享 [Model APIs](https://docs.baseten.co/inference/model-apis/overview)。 | ||
| > 本地使用可选择个人 [API 密钥](https://docs.baseten.co/organization/api-keys)；共享或生产用途请使用具备 | ||
| > **Call Model APIs** 权限的团队密钥。 | ||
| > 专用 Truss `predict` 端点使用不同的主机和协议，不由此预设路由。 | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Describe Truss differences as hosts and schemas, not protocols.
The English source and runtime objective say dedicated Truss predict endpoints use different hosts and schemas. “不同的主机和协议” can incorrectly imply a different network protocol; use wording such as “不同的主机和请求 schema/格式” instead. Keep the blank separator before the Tencent callout unchanged.
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 151-151: Blank line inside blockquote
(MD028, no-blanks-blockquote)
🤖 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 `@docs-site/src/content/docs/zh-cn/guides/providers.md` around lines 147 - 151,
Update the Baseten Truss sentence in the Chinese provider documentation to
describe dedicated predict endpoints as using different hosts and request
schemas/formats, not different protocols. Preserve the surrounding wording and
keep the blank separator before the Tencent callout unchanged.
Source: Path instructions
| if (spec.path !== undefined) { | ||
| const path = spec.path.trim(); | ||
| if (!path || path.length > 512) return "discovery path must be 1-512 characters"; | ||
| if (/^[a-z][a-z\d+.-]*:/i.test(path) || path.startsWith("//") || path.includes("?") || path.includes("#")) { | ||
| return "discovery path must be a query-free relative/origin path"; | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Reject .. segments in spec.path — the validator's other path guards leave this one escape vector open.
Line 90 rejects schemes, protocol-relative //, ?, and #, but a path such as "../../internal/models" passes. At resolution time (Lines 152-156) new URL(spec.path, base) will climb above /v1 and hit a different resource on the same trusted host, and appendDiscoveryQuery then sends the provider credential there. The host cannot change, so this is hardening rather than an exploitable hole today (specs are registry-only and Baseten ships path: "models"), but this function is the designated choke point and the check is one line.
🛡️ Proposed hardening
if (spec.path !== undefined) {
const path = spec.path.trim();
if (!path || path.length > 512) return "discovery path must be 1-512 characters";
if (/^[a-z][a-z\d+.-]*:/i.test(path) || path.startsWith("//") || path.includes("?") || path.includes("#")) {
return "discovery path must be a query-free relative/origin path";
}
+ if (path.split("/").includes("..")) return "discovery path must not contain parent-directory segments";
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (spec.path !== undefined) { | |
| const path = spec.path.trim(); | |
| if (!path || path.length > 512) return "discovery path must be 1-512 characters"; | |
| if (/^[a-z][a-z\d+.-]*:/i.test(path) || path.startsWith("//") || path.includes("?") || path.includes("#")) { | |
| return "discovery path must be a query-free relative/origin path"; | |
| } | |
| } | |
| if (spec.path !== undefined) { | |
| const path = spec.path.trim(); | |
| if (!path || path.length > 512) return "discovery path must be 1-512 characters"; | |
| if (/^[a-z][a-z\d+.-]*:/i.test(path) || path.startsWith("//") || path.includes("?") || path.includes("#")) { | |
| return "discovery path must be a query-free relative/origin path"; | |
| } | |
| if (path.split("/").includes("..")) return "discovery path must not contain parent-directory segments"; | |
| } |
🤖 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/providers/model-discovery.ts` around lines 87 - 93, Update the path
validation in the spec.path guard to reject any path segment equal to "..",
including repeated or embedded traversal segments such as
"../../internal/models". Preserve acceptance of ordinary relative paths like
"models" and the existing scheme, protocol-relative, query, fragment, and length
checks.
2 participants
Summary
basetenas a first-class API-key preset for the shared Baseten Model APIs endpoint athttps://inference.baseten.co/v1./modelsdiscovery through the contract in feat(providers): add bounded model discovery contract #652, with a 1 MiB response limit, a 256-row limit, and same-named custom-provider destination preservation.This PR is a pilot provider consumer under umbrella issue #572. The umbrella issue must remain open for the remaining provider batches.
Dependency And Review Order
This is stacked on #652 and targets
dev. Until #652 merges, GitHub will show both the contract commit and the Baseten commit in this PR. The Baseten-specific change is the final commit in this stack; after #652 merges, the PR diff will automatically reduce to that provider commit.Scope
predictendpoints, which use different hosts, authentication, and arbitrary schemas.Security
This adds a credential-bearing provider preset. It uses the contract's redirect denial and
preserveCustomDestinationprotection, does not persist discovery trust policy, and introduces no key logging. Explicit maintainer security review is requested before merge.Verification
bun run typecheckbun run privacy:scanbun run lint:guiChecklist
Summary by CodeRabbit
New Features
Bug Fixes
Documentation