mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-13 18:32:12 +03:00
fix(opencode): require an http(s) baseURL in both OpenCode plugins (#13142)
Real and nasty precisely because it is silent: `z.string().url()` accepts `localhost:20128` as scheme `localhost:` plus a path, every model gets published with an unusable api url, and the failure happens inside the client so the gateway logs show nothing. Backing the option schema, the publish boundary and the snapshot filter with one `isHttpUrl` in v2 is the right call — those three cannot drift apart. Duplicating the predicate in v1 rather than sharing it is also correct, since the two packages ship independently. --- Validated in one consolidated worktree cut from `release/v3.8.51`, boarded together with the rest of this batch — zero conflicts between them. - `typecheck:core` clean; `check:changelog-integrity` OK - complexity 2799 / baseline 3218 and cognitive-complexity 1265 / baseline 1437 — both under baseline - 86 focused assertions green across the batch's 10 unit test files, plus 16/16 on the v1 plugin option schema and 16/16 on the v2 option tests - `check-file-size` rebaselined for this batch's real growth (annotation `_rebaseline_2026_09_11_mergebatch_v3851_maxmad_opencode`, landed on #13141). `open-sse/utils/stream.ts` was deliberately left frozen: it is already 3115 > 3098 on the pure tip with zero contribution from this batch. ⚠️ base-red inherited: #12732 — `Docs Gates`, `Merge integrity`, `No new ESLint warnings`, `Unit Tests fast-path` and `Fast Quality Gates` all reproduce on the pure `release/v3.8.51` tip (provider count 356 vs the 358 the modules define, SKILL.md drift, and the `stream.ts` freeze above). None of them touch these diffs. Thanks @maxmad64bis.
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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 };
|
||||
}
|
||||
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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`;
|
||||
|
||||
@@ -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 }));
|
||||
});
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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`;
|
||||
|
||||
@@ -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(
|
||||
() =>
|
||||
|
||||
1
changelog.d/fixes/13142-plugin-v2-model-api-url.md
Normal file
1
changelog.d/fixes/13142-plugin-v2-model-api-url.md
Normal file
@@ -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
|
||||
@@ -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 |
|
||||
|
||||
Reference in New Issue
Block a user