Repository navigation
feat(sidecar): add provider manifest client (port #6042) - #265
Conversation
Port upstream diegosouzapw#6042. Adds an HTTP client for the read-only provider plugin manifest so sidecars/relays discover providers over the network instead of hard-coding the /api/v1/provider-plugin-manifest route. - resolveProviderPluginManifestUrl: explicit manifestUrl > OMNIROUTE_PROVIDER_MANIFEST_URL env > baseUrl > local API default - fetchProviderPluginManifest: fetch + schemaVersion 1 validation - getProviderPluginManifestEntryForModelFromManifest / fetchProviderPluginManifestEntryForModel: model->provider lookup by prefix, alias, or model id Excludes branch-drift noise from the upstream diff (translator responses test rename, should-promote-latest.sh CI tweak) unrelated to the sidecar client.
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Warning Review limit reached
Next review available in: 16 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
Note
|
L17 Latency Budget ReportChecked against: budgets/rest-endpoints.yaml. |
L17 Latency Regression ReportThreshold: 10% p99 regression. |
|
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Files Reviewed (3 files)
Reviewed by laguna-m.1-20260312:free · Input: 255.3K · Output: 7.5K · Cached: 1.3M |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| ProviderPluginManifestEntry, | ||
| } from "./providerPluginManifest.ts"; | ||
|
|
||
| export const PROVIDER_PLUGIN_MANIFEST_PATH = "/api/v1/provider-plugin-manifest"; |
There was a problem hiding this comment.
CRITICAL: Duplicate export - PROVIDER_PLUGIN_MANIFEST_PATH is already exported from providerPluginManifestUrl.ts line 2. Exporting the same constant from two modules creates ambiguity and potential for divergent values.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| } from "./providerPluginManifest.ts"; | ||
|
|
||
| export const PROVIDER_PLUGIN_MANIFEST_PATH = "/api/v1/provider-plugin-manifest"; | ||
| export const PROVIDER_PLUGIN_MANIFEST_ENV = "OMNIROUTE_PROVIDER_MANIFEST_URL"; |
There was a problem hiding this comment.
CRITICAL: Duplicate export - PROVIDER_PLUGIN_MANIFEST_ENV duplicates the env var name already used in providerPluginManifestUrl.ts. Consider importing and re-exporting instead.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| } | ||
|
|
||
| function trimTrailingSlash(value: string): string { | ||
| return value.replace(/\/+$/, ""); |
There was a problem hiding this comment.
CRITICAL: Inconsistent regex behavior - trimTrailingSlash uses /\/+$/ (removes ALL trailing slashes) while providerPluginManifestUrl.ts line 5 uses /\/$/ (removes only ONE trailing slash). This will cause different URL normalization behavior depending on which module is used.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| ): Promise<ProviderPluginManifest> { | ||
| const fetcher = options.fetchImpl ?? fetch; | ||
| const url = resolveProviderPluginManifestUrl(options); | ||
| const response = await fetcher(url, { |
There was a problem hiding this comment.
WARNING: Missing default timeout - HTTP fetch operations in this codebase typically use AbortSignal.timeout() (e.g., qoderCli.ts, claude-web.ts) to prevent indefinite hangs. Consider adding a default timeout when no signal is provided.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
1 participant
Ports upstream diegosouzapw#6042
feat(sidecar): add provider manifest clientas a self-contained follow-up.What
Adds
open-sse/config/providerPluginManifestClient.ts— an HTTP client for the read-only provider plugin manifest (GET /api/v1/provider-plugin-manifest, introduced by the manifest system in #253) so sidecars/relays discover providers over the network instead of hard-coding the route.resolveProviderPluginManifestUrl— resolution order: explicitmanifestUrl→OMNIROUTE_PROVIDER_MANIFEST_URLenv →baseUrl→ local OmniRoute API default (HOST/PORT).fetchProviderPluginManifest— fetch +schemaVersion === 1/providersarray validation; throws on non-OK HTTP or malformed body.getProviderPluginManifestEntryForModelFromManifest/fetchProviderPluginManifestEntryForModel— model→provider lookup byprovider/prefix, alias, then model id.Files (3)
open-sse/config/providerPluginManifestClient.ts(new, 87 lines)tests/unit/provider-plugin-manifest-client.test.ts(new, 5 tests)docs/reference/PROVIDER_PLUGIN_MANIFEST.md(keep-both merge: retains the existingX-OmniRoute-Provider-Manifest-Urlheader-advertising paragraph AND adds the new client-usage guidance)Excluded (branch-drift noise)
The upstream 3-dot diff carried two unrelated changes that do not belong to the sidecar client; both reverted to base:
tests/unit/translator-openai-responses-req.test.ts→-chat.test.tsrename (446-line translator test churn).scripts/ci/should-promote-latest.sh(CI release-promotion regex hardening).The upstream diff also touched
route.ts,API_REFERENCE.md,stryker.conf.json, and the route test — those already match main (landed via the #253/#261 manifest system) so the patch was a no-op there.Verification
node --import tsx/esm --test tests/unit/provider-plugin-manifest-client.test.ts→ 5/5 pass.npm run typecheck:core→ clean (0 errors). Manifest type fields (id/alias/schemaVersion/models[].id) verified present on main — no fork divergence.