diff --git a/@omniroute/opencode-plugin-v2/src/cache.ts b/@omniroute/opencode-plugin-v2/src/cache.ts index 58aca429e4..0f7ad948bd 100644 --- a/@omniroute/opencode-plugin-v2/src/cache.ts +++ b/@omniroute/opencode-plugin-v2/src/cache.ts @@ -10,6 +10,7 @@ import type { OmniRouteRawCombo, OmniRouteRawModelEntry, } from "./shared/index.js"; +import { isHttpUrl } from "./shared/index.js"; export const DEFAULT_MODEL_CACHE_TTL_MS = 300_000 as const; @@ -34,8 +35,9 @@ export const SNAPSHOT_FORMAT_VERSION = 2 as const; /** * A raw snapshot entry is stale when it cannot be mapped to a publishable - * model: no string `id` (unroutable) or a pre-mapped `api` block without a - * valid `npm` package (the runner would reject it as `Unsupported package`). + * model: no string `id` (unroutable), or a pre-mapped `api` block missing a + * valid `npm` package (the runner would reject it as `Unsupported package`) + * or a usable `url` (the host would reach the AI SDK with no baseURL). * Plain `/v1/models` entries carry no `api` block -- it is synthesized at * publish time -- so only a present-but-invalid block drops the entry. */ @@ -47,7 +49,11 @@ export function isStaleSnapshotModel(entry: unknown): boolean { if (api === undefined) return false; if (!api || typeof api !== "object") return true; const npm = (api as { npm?: unknown }).npm; - return typeof npm !== "string" || npm.length === 0; + if (typeof npm !== "string" || npm.length === 0) return true; + // Same requirement as `npm`, and the same predicate the options schema + // applies to `baseURL`: a pre-mapped block without a callable `url` publishes + // a model the host cannot route -- see `legacyApiToInfoApi`. + return !isHttpUrl((api as { url?: unknown }).url); } interface DiskSnapshotV2 { @@ -145,7 +151,7 @@ export async function readDiskSnapshot( (entry) => !isStaleSnapshotModel(entry) ); if (stale > 0) { - logger?.warn(`[omniroute-v2] dropping ${stale} stale snapshot entries without api block`); + logger?.warn(`[omniroute-v2] dropping ${stale} stale snapshot entries with an unusable api block`); } if (models.length === 0) return undefined; return { diff --git a/@omniroute/opencode-plugin-v2/src/catalog.ts b/@omniroute/opencode-plugin-v2/src/catalog.ts index 73c3f4ab70..6766b49b7c 100644 --- a/@omniroute/opencode-plugin-v2/src/catalog.ts +++ b/@omniroute/opencode-plugin-v2/src/catalog.ts @@ -3,6 +3,7 @@ import { type HostContract, detectHostContract, emitsLegacyFields } from "./comp import type { Model as LegacyModelV2 } from "@opencode-ai/sdk/v2"; import type { ModelV2Info, ProviderV2Info } from "@opencode-ai/sdk/v2/types"; import { + isHttpUrl, type ApiFormatV2, type LogLevel, type Logger, @@ -142,6 +143,15 @@ export function legacyApiToInfoApi(api: LegacyModelV2["api"]): ModelV2Info["api" "[omniroute-v2] refusing to publish a model without an api block (missing api.npm)" ); } + // The host reads `api.url` in `prepareOptions` and never falls back to the + // provider's own, so a model published without one reaches the AI SDK with no + // baseURL and fails at call time with a bare `Invalid URL` — no request on the + // wire, nothing in the gateway logs, no model named. + if (!isHttpUrl(api.url)) { + throw new Error( + "[omniroute-v2] refusing to publish a model whose api block carries no http(s) url" + ); + } return { id: api.id, type: "aisdk", package: api.npm, url: api.url }; } diff --git a/@omniroute/opencode-plugin-v2/src/options.ts b/@omniroute/opencode-plugin-v2/src/options.ts index 9782ca74e6..9f9f23041c 100644 --- a/@omniroute/opencode-plugin-v2/src/options.ts +++ b/@omniroute/opencode-plugin-v2/src/options.ts @@ -1,5 +1,7 @@ import { z } from "zod"; +import { isHttpUrl } from "./shared/models-map.js"; + const apiFormatSchema = z .object({ allowAnthropic: z.boolean().optional(), @@ -28,7 +30,10 @@ const pluginOptionsSchema = z .regex(/^[A-Za-z0-9._-]+$/, "providerId may only contain letters, digits, '.', '_' and '-'") .refine((v) => v !== "." && v !== "..", "providerId cannot be a path segment") .default("omniroute"), - baseURL: z.string().url(), + baseURL: z + .string() + .trim() + .refine(isHttpUrl, "baseURL must be an http(s) URL, for example http://localhost:20128"), apiKey: z.string().optional(), displayName: z.string().optional(), managementReadToken: z.string().optional(), diff --git a/@omniroute/opencode-plugin-v2/src/shared/models-map.ts b/@omniroute/opencode-plugin-v2/src/shared/models-map.ts index 625e02f232..a750ef2e0b 100644 --- a/@omniroute/opencode-plugin-v2/src/shared/models-map.ts +++ b/@omniroute/opencode-plugin-v2/src/shared/models-map.ts @@ -111,6 +111,22 @@ function trimTrailingSlashes(value: string): string { * (it appends `/v1/messages` automatically), so callers should branch on * format first. */ +/** + * A url the AI SDK can actually call. `new URL()` alone is not enough: it + * parses `localhost:20128` as the scheme `localhost:` and `ftp://host` as ftp, + * both of which reach `fetch` and fail there. Mirrors the `isHttpUrl` guard the + * settings schema applies to `headroomUrl`. + */ +export function isHttpUrl(value: unknown): boolean { + if (typeof value !== "string") return false; + try { + const { protocol } = new URL(value); + return protocol === "http:" || protocol === "https:"; + } catch { + return false; + } +} + export function ensureV1Suffix(url: string): string { const trimmed = trimTrailingSlashes(url); return trimmed.endsWith("/v1") ? trimmed : `${trimmed}/v1`; diff --git a/@omniroute/opencode-plugin-v2/tests/options.test.ts b/@omniroute/opencode-plugin-v2/tests/options.test.ts index 6a09ade0cc..cf36b1331d 100644 --- a/@omniroute/opencode-plugin-v2/tests/options.test.ts +++ b/@omniroute/opencode-plugin-v2/tests/options.test.ts @@ -29,6 +29,38 @@ describe("parsePluginOptions", () => { it("requires baseURL", () => { assert.throws(() => parsePluginOptions({}), /baseURL/); }); + it("rejects a baseURL that is not an http(s) URL", () => { + // `new URL()` reads "localhost:20128" as the scheme "localhost:" followed + // by a path, so a gateway address typed without "http://" parses. Every + // model would then be published with "localhost:20128/v1" as its api url + // and every call would fail in the client on an unknown scheme, with no + // request on the wire and nothing in the gateway logs. + for (const baseURL of [ + "localhost:20128", + "localhost:20128/v1", + "ftp://gw.example.com/v1", + "gw.example.com/v1", + ]) { + assert.throws( + () => parsePluginOptions({ baseURL }), + /baseURL must be an http\(s\) URL/, + `expected ${baseURL} to be rejected` + ); + } + }); + it("accepts http and https baseURLs, with or without a port or path", () => { + for (const baseURL of [ + "http://localhost:20128/v1", + "http://localhost:20128", + "https://gw.example.com/v1", + "https://gw.example.com/omniroute/v1", + ]) { + assert.equal(parsePluginOptions({ baseURL }).baseURL, baseURL); + // Padding a copied address is trimmed rather than rejected, matching the + // treatment `headroomUrl` already gets in the settings schema. + assert.equal(parsePluginOptions({ baseURL: ` ${baseURL} ` }).baseURL, baseURL); + } + }); it("rejects unknown top-level keys (strict)", () => { assert.throws(() => parsePluginOptions({ baseURL: "https://gw.example.com", bogus: 1 })); }); diff --git a/@omniroute/opencode-plugin-v2/tests/snapshot-stale-entries.test.ts b/@omniroute/opencode-plugin-v2/tests/snapshot-stale-entries.test.ts index 3d08cc5d09..1bf90eff63 100644 --- a/@omniroute/opencode-plugin-v2/tests/snapshot-stale-entries.test.ts +++ b/@omniroute/opencode-plugin-v2/tests/snapshot-stale-entries.test.ts @@ -5,7 +5,11 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { createHash } from "node:crypto"; import plugin from "../src/index.js"; -import { diskSnapshotPath, snapshotIdentityFingerprint } from "../src/cache.js"; +import { + diskSnapshotPath, + isStaleSnapshotModel, + snapshotIdentityFingerprint, +} from "../src/cache.js"; import { legacyApiToInfoApi } from "../src/catalog.js"; function isolateDisk(): { dir: string; restore: () => void } { @@ -97,7 +101,7 @@ function downFetch(): typeof fetch { const fingerprint = snapshotIdentityFingerprint("https://gw.example.com", "k-snapfix", "k-snapfix"); describe("plugin-v2 snapshot stale-entry filter", () => { - it("snapshot with 2 entries without api block + 1 valid: only the valid one is published + warn emitted", async () => { + it("snapshot with 3 unusable pre-mapped entries + 1 valid: only the valid one is published + warn emitted", async () => { const disk = isolateDisk(); const providerId = "snapfix-mixed"; mkdirSync(join(disk.dir, "plugins"), { recursive: true }); @@ -106,11 +110,15 @@ describe("plugin-v2 snapshot stale-entry filter", () => { JSON.stringify({ v: 2, identityFingerprint: fingerprint, - // Two pre-mapped entries with a broken api block (missing npm) plus - // one plain raw entry (no api block: synthesized at publish time). + // Three pre-mapped entries with an unusable api block — missing npm, + // empty npm, and a well-formed npm with no url (the shape a snapshot + // written by an older build carries, and the one that reaches the host + // as a bare `Invalid URL`) — plus one plain raw entry, which has no api + // block at all and gets one synthesized at publish time. models: [ { id: "stale-a", api: {} }, { id: "stale-b", api: { npm: "" } }, + { id: "stale-c", api: { id: "openai-compatible", npm: "@ai-sdk/openai-compatible" } }, { id: "good-1", context_length: 128000 }, ], combos: [], @@ -137,7 +145,7 @@ describe("plugin-v2 snapshot stale-entry filter", () => { ); }); assert.ok( - warns.some((w) => w.includes("dropping 2 stale snapshot entries without api block")), + warns.some((w) => w.includes("dropping 3 stale snapshot entries with an unusable api block")), `expected stale-drop warn, got: ${JSON.stringify(warns)}` ); } finally { @@ -216,4 +224,56 @@ describe("plugin-v2 snapshot stale-entry filter", () => { // Sanity: sha256 helper used above matches the plugin identity scheme. assert.equal(createHash("sha256").update("x").digest("hex").length, 64); }); + + it("legacyApiToInfoApi throws unless api.url is an http(s) url", () => { + const npm = "@ai-sdk/openai-compatible"; + for (const api of [ + { id: "openai-compatible", npm }, + { id: "openai-compatible", npm, url: "" }, + { id: "openai-compatible", npm, url: " " }, + // Non-empty but uncallable: the AI SDK reaches `fetch` and fails there. + { id: "openai-compatible", npm, url: "/v1" }, + { id: "openai-compatible", npm, url: "gw.example.com/v1" }, + { id: "openai-compatible", npm, url: "ftp://gw.example.com/v1" }, + ]) { + assert.throws( + () => legacyApiToInfoApi(api as unknown as { id: string; npm: string; url: string }), + /api block carries no http\(s\) url/, + `expected a publish-time refusal for ${JSON.stringify(api)}` + ); + } + // A complete block still publishes unchanged. + assert.deepEqual( + legacyApiToInfoApi({ + id: "openai-compatible", + npm: "@ai-sdk/openai-compatible", + url: "https://gw.example.com/v1", + }), + { + id: "openai-compatible", + type: "aisdk", + package: "@ai-sdk/openai-compatible", + url: "https://gw.example.com/v1", + } + ); + }); + + it("isStaleSnapshotModel drops a pre-mapped entry whose api.url is unusable", () => { + const npm = "@ai-sdk/openai-compatible"; + // Present-but-unusable url: stale, for the same reason a missing npm is. + for (const url of [undefined, "", " ", "/v1", "gw.example.com/v1", "ftp://gw/v1"]) { + assert.equal( + isStaleSnapshotModel({ id: "a/b", api: { id: "x", npm, ...(url === undefined ? {} : { url }) } }), + true, + `expected ${JSON.stringify(url)} to be treated as stale` + ); + } + // Complete block: publishable. + assert.equal( + isStaleSnapshotModel({ id: "a/b", api: { id: "x", npm, url: "https://gw/v1" } }), + false + ); + // No api block at all stays publishable: it is synthesized at publish time. + assert.equal(isStaleSnapshotModel({ id: "a/b" }), false); + }); }); diff --git a/@omniroute/opencode-plugin/src/index.ts b/@omniroute/opencode-plugin/src/index.ts index 4738c0e422..39e3c6f274 100644 --- a/@omniroute/opencode-plugin/src/index.ts +++ b/@omniroute/opencode-plugin/src/index.ts @@ -220,7 +220,11 @@ const optionsSchema = z * to 60000. Default when unset: 300000. */ autoSyncIntervalMs: z.number().int().nonnegative().optional(), - baseURL: z.string().url().optional(), + baseURL: z + .string() + .trim() + .refine(isHttpUrl, "baseURL must be an http(s) URL, for example http://localhost:20128") + .optional(), managementReadToken: z.string().min(1).optional(), features: featuresSchema.optional(), }) @@ -482,6 +486,22 @@ export const DEFAULT_ANTHROPIC_PREFIXES = ["cc", "claude", "anthropic", "kiro", * (it appends `/v1/messages` automatically), so callers should branch on * format first. */ +/** + * A url the AI SDK can actually call. `new URL()` alone is not enough: it + * parses `localhost:20128` as the scheme `localhost:` and `ftp://host` as ftp, + * both of which reach `fetch` and fail there. Mirrors the `isHttpUrl` guard the + * settings schema applies to `headroomUrl`. + */ +export function isHttpUrl(value: unknown): boolean { + if (typeof value !== "string") return false; + try { + const { protocol } = new URL(value); + return protocol === "http:" || protocol === "https:"; + } catch { + return false; + } +} + export function ensureV1Suffix(url: string): string { const trimmed = trimTrailingSlashes(url); return trimmed.endsWith("/v1") ? trimmed : `${trimmed}/v1`; diff --git a/@omniroute/opencode-plugin/tests/options-schema.test.ts b/@omniroute/opencode-plugin/tests/options-schema.test.ts index 435363946c..e941276247 100644 --- a/@omniroute/opencode-plugin/tests/options-schema.test.ts +++ b/@omniroute/opencode-plugin/tests/options-schema.test.ts @@ -59,6 +59,26 @@ test("parseOmniRoutePluginOptions: invalid baseURL (not a URL) → throws", () = assert.throws(() => parseOmniRoutePluginOptions({ baseURL: "not-a-url" }), /baseURL/i); }); +test("parseOmniRoutePluginOptions: baseURL without an http(s) scheme → throws", () => { + // `new URL()` reads "localhost:20128" as the scheme "localhost:" followed by + // a path, so the address parses and the models are published with an api url + // no client can call. + for (const baseURL of ["localhost:20128", "localhost:20128/v1", "ftp://or.example.com", "or.example.com"]) { + assert.throws( + () => parseOmniRoutePluginOptions({ baseURL }), + /baseURL must be an http\(s\) URL/, + `expected ${baseURL} to be rejected` + ); + } +}); + +test("parseOmniRoutePluginOptions: http and https baseURLs are accepted, padding trimmed", () => { + for (const baseURL of ["http://localhost:20128", "https://or.example.com/v1"]) { + assert.equal(parseOmniRoutePluginOptions({ baseURL }).baseURL, baseURL); + assert.equal(parseOmniRoutePluginOptions({ baseURL: ` ${baseURL} ` }).baseURL, baseURL); + } +}); + test("parseOmniRoutePluginOptions: unknown key → throws (strict mode catches typos)", () => { assert.throws( () => diff --git a/changelog.d/fixes/13142-plugin-v2-model-api-url.md b/changelog.d/fixes/13142-plugin-v2-model-api-url.md new file mode 100644 index 0000000000..34f1c9a172 --- /dev/null +++ b/changelog.d/fixes/13142-plugin-v2-model-api-url.md @@ -0,0 +1 @@ +- **fix(opencode):** both OpenCode plugins now reject a gateway address typed without `http://` at configuration time, instead of publishing every model with an api url no client can call, and the v2 plugin no longer publishes a model card whose api url is blank or relative ([#13142](https://github.com/diegosouzapw/OmniRoute/pull/13142)) — thanks @maxmad64bis diff --git a/docs/guides/OPENCODE-V2-PLUGIN.md b/docs/guides/OPENCODE-V2-PLUGIN.md index c79c31dac6..cc27ab54f2 100644 --- a/docs/guides/OPENCODE-V2-PLUGIN.md +++ b/docs/guides/OPENCODE-V2-PLUGIN.md @@ -77,7 +77,7 @@ naming the endpoint and what was lost — so a degraded picker is never a myster | Key | Default | Notes | | -------------------------------- | ---------------------------------------------- | ------------------------------------------------------------------------------------------------------ | | `providerId` | `"omniroute"` | Provider id, integration id, and the prefix models appear under | -| `baseURL` | required | Gateway root; the `/v1` suffix is added where needed | +| `baseURL` | required | Gateway root, `http(s)` only; the `/v1` suffix is added where needed | | `apiKey` | connected credential, then `OMNIROUTE_API_KEY` | Chat key for `/v1/*` | | `managementReadToken` | falls back to `apiKey` | Key for `/api/*` — usually **not** the same one | | `displayName` | `"OmniRoute"` | Provider name in the picker |