Compare commits

..

2 Commits

11 changed files with 264 additions and 119 deletions

View File

@@ -30,20 +30,60 @@ export function register_combos(parent) {
const data = res.ok ? await res.json() : await res.text(); const data = res.ok ? await res.json() : await res.text();
emit(data, gOpts); emit(data, gOpts);
}); });
tag.command("patch-api-combos-id-") tag.command("get-api-combos-id-")
.description("Update combo") .description("Get combo by ID")
.requiredOption("--id <id>", "")
.action(async (opts, cmd) => { .action(async (opts, cmd) => {
const gOpts = cmd.optsWithGlobals(); const gOpts = cmd.optsWithGlobals();
let url = "/api/combos/{id}"; let url = "/api/combos/{id}";
const res = await apiFetch(url, { method: "PATCH", baseUrl: gOpts.baseUrl, apiKey: gOpts.apiKey }); url = url.replace("{id}", encodeURIComponent(opts.id ?? ""));
const res = await apiFetch(url, { method: "GET", baseUrl: gOpts.baseUrl, apiKey: gOpts.apiKey });
const data = res.ok ? await res.json() : await res.text();
emit(data, gOpts);
});
tag.command("put-api-combos-id-")
.description("Update combo")
.requiredOption("--id <id>", "")
.option("--body <jsonOrPath>", "JSON body or @path/to/file.json")
.action(async (opts, cmd) => {
const gOpts = cmd.optsWithGlobals();
let url = "/api/combos/{id}";
url = url.replace("{id}", encodeURIComponent(opts.id ?? ""));
let body;
if (opts.body) {
body = opts.body.startsWith("@")
? JSON.parse(readFileSync(opts.body.slice(1), "utf8"))
: JSON.parse(opts.body);
}
const res = await apiFetch(url, { method: "PUT", body, baseUrl: gOpts.baseUrl, apiKey: gOpts.apiKey });
const data = res.ok ? await res.json() : await res.text();
emit(data, gOpts);
});
tag.command("patch-api-combos-id-")
.description("Update combo")
.requiredOption("--id <id>", "")
.option("--body <jsonOrPath>", "JSON body or @path/to/file.json")
.action(async (opts, cmd) => {
const gOpts = cmd.optsWithGlobals();
let url = "/api/combos/{id}";
url = url.replace("{id}", encodeURIComponent(opts.id ?? ""));
let body;
if (opts.body) {
body = opts.body.startsWith("@")
? JSON.parse(readFileSync(opts.body.slice(1), "utf8"))
: JSON.parse(opts.body);
}
const res = await apiFetch(url, { method: "PATCH", body, baseUrl: gOpts.baseUrl, apiKey: gOpts.apiKey });
const data = res.ok ? await res.json() : await res.text(); const data = res.ok ? await res.json() : await res.text();
emit(data, gOpts); emit(data, gOpts);
}); });
tag.command("delete-api-combos-id-") tag.command("delete-api-combos-id-")
.description("Delete combo") .description("Delete combo")
.requiredOption("--id <id>", "")
.action(async (opts, cmd) => { .action(async (opts, cmd) => {
const gOpts = cmd.optsWithGlobals(); const gOpts = cmd.optsWithGlobals();
let url = "/api/combos/{id}"; let url = "/api/combos/{id}";
url = url.replace("{id}", encodeURIComponent(opts.id ?? ""));
const res = await apiFetch(url, { method: "DELETE", baseUrl: gOpts.baseUrl, apiKey: gOpts.apiKey }); const res = await apiFetch(url, { method: "DELETE", baseUrl: gOpts.baseUrl, apiKey: gOpts.apiKey });
const data = res.ok ? await res.json() : await res.text(); const data = res.ok ? await res.json() : await res.text();
emit(data, gOpts); emit(data, gOpts);

View File

@@ -1 +0,0 @@
- fix(cli): always emit limit.output in generated OpenCode config so schema validation passes for metadata-less models (#10940)

View File

@@ -0,0 +1 @@
- fix(cli): resolve $ref path params and add PATCH combos requestBody in generated API commands (#10955)

View File

@@ -2105,11 +2105,26 @@ paths:
patch: patch:
tags: [Combos] tags: [Combos]
summary: Update combo summary: Update combo
description: >-
Partial update: the body is merged onto the stored combo, so a field left out keeps
its current value. An array that IS sent replaces the stored one outright.
parameters: parameters:
- $ref: "#/components/parameters/ResourceId" - $ref: "#/components/parameters/ResourceId"
requestBody:
required: true
content:
application/json:
schema:
type: object
responses: responses:
"200": "200":
description: Updated combo description: Updated combo
"400":
description: Invalid body, or the resulting combo fails validation
"404":
description: Combo not found
"409":
description: Name already taken, or the combo is quota-share managed
delete: delete:
tags: [Combos] tags: [Combos]
summary: Delete combo summary: Delete combo

View File

@@ -156,8 +156,11 @@ const IGNORE_FROM_CODE = new Set([
// X11/Wayland display server vars used by tray heuristic (isTraySupported). // X11/Wayland display server vars used by tray heuristic (isTraySupported).
"DISPLAY", "DISPLAY",
"WAYLAND_DISPLAY", "WAYLAND_DISPLAY",
// Build-time override for OpenAPI spec path used by generate-api-commands.mjs. // Build-time overrides for generate-api-commands.mjs (spec input / commands output dir).
// OPENAPI_OUT_DIR exists so tests/unit/cli-api-generator-ref-params.test.ts can regenerate
// into a scratch dir instead of the real bin/cli/api-commands/ tree.
"OPENAPI_SPEC", "OPENAPI_SPEC",
"OPENAPI_OUT_DIR",
// Aliases for documented vars handled via fallback ordering. // Aliases for documented vars handled via fallback ordering.
"API_KEY", "API_KEY",
"APP_URL", "APP_URL",

View File

@@ -11,7 +11,7 @@ import * as yaml from "js-yaml";
const __dirname = dirname(fileURLToPath(import.meta.url)); const __dirname = dirname(fileURLToPath(import.meta.url));
const ROOT = join(__dirname, "..", ".."); const ROOT = join(__dirname, "..", "..");
const SPEC_PATH = process.env.OPENAPI_SPEC || join(ROOT, "docs/openapi.yaml"); const SPEC_PATH = process.env.OPENAPI_SPEC || join(ROOT, "docs/openapi.yaml");
const OUT_DIR = join(ROOT, "bin/cli/api-commands"); const OUT_DIR = process.env.OPENAPI_OUT_DIR || join(ROOT, "bin/cli/api-commands");
// Operations already covered by hand-crafted commands — skip in generated output. // Operations already covered by hand-crafted commands — skip in generated output.
const IGNORED_OP_IDS = new Set([ const IGNORED_OP_IDS = new Set([
@@ -51,6 +51,29 @@ if (!existsSync(OUT_DIR)) mkdirSync(OUT_DIR, { recursive: true });
const spec = yaml.load(readFileSync(SPEC_PATH, "utf8")); const spec = yaml.load(readFileSync(SPEC_PATH, "utf8"));
// Minimal, scoped $ref resolver — only follows refs into components/parameters.
// This is not a generic dereferencer (no cycle handling, no cross-file refs):
// OpenAPI `parameters` entries in this spec only ever $ref a component parameter
// (see docs/openapi.yaml → components/parameters/ResourceId), so a full
// dereferencer would be scope creep. Without this, `p.in === "path"` silently
// drops every $ref'd path parameter (a bare `{ $ref }` object has no `.in`),
// which is what let generated PATCH/DELETE combo commands lose --id (#10955).
const PARAM_REF_PREFIX = "#/components/parameters/";
function resolveParam(p) {
if (p && typeof p === "object" && typeof p.$ref === "string") {
if (!p.$ref.startsWith(PARAM_REF_PREFIX)) {
throw new Error(`Unsupported parameter $ref (only ${PARAM_REF_PREFIX}* is resolved): ${p.$ref}`);
}
const name = p.$ref.slice(PARAM_REF_PREFIX.length);
const resolved = spec.components?.parameters?.[name];
if (!resolved) {
throw new Error(`Unresolvable parameter $ref: ${p.$ref}`);
}
return resolved;
}
return p;
}
/** @type {Record<string, Array<{path: string, method: string, opId: string, op: object}>>} */ /** @type {Record<string, Array<{path: string, method: string, opId: string, op: object}>>} */
const byTag = {}; const byTag = {};
@@ -89,7 +112,7 @@ for (const [tag, ops] of Object.entries(byTag)) {
for (const { path, method, opId, op } of ops) { for (const { path, method, opId, op } of ops) {
const cmdName = kebab(opId); const cmdName = kebab(opId);
const params = op.parameters || []; const params = (op.parameters || []).map(resolveParam);
const pathParams = params.filter((p) => p.in === "path"); const pathParams = params.filter((p) => p.in === "path");
const queryParams = params.filter((p) => p.in === "query"); const queryParams = params.filter((p) => p.in === "query");
const hasBody = !!op.requestBody; const hasBody = !!op.requestBody;

View File

@@ -60,6 +60,8 @@ curl -X PUT https://localhost:20128/api/combos/{id} \
Update combo Update combo
Partial update: the body is merged onto the stored combo, so a field left out keeps its current value. An array that IS sent replaces the stored one outright.
```bash ```bash
curl -X PATCH https://localhost:20128/api/combos/{id} \ curl -X PATCH https://localhost:20128/api/combos/{id} \
-H "Authorization: Bearer $OMNIROUTE_TOKEN" \ -H "Authorization: Bearer $OMNIROUTE_TOKEN" \

View File

@@ -243,11 +243,9 @@ function resolveContextLength(entry: CatalogModelEntry): number | undefined {
* 1. Existing manual override in the user's opencode.json (`limit.context`). * 1. Existing manual override in the user's opencode.json (`limit.context`).
* 2. Catalog `context_length` / `max_context_window_tokens`. * 2. Catalog `context_length` / `max_context_window_tokens`.
* *
* If neither is available, `limit.context` is simply omitted and OpenCode's * If neither is available, the entry is returned WITHOUT a `limit` block so
* own heuristics apply — we never fabricate a default context window. The * the caller can decide whether to skip the model entirely or surface a
* entry ALWAYS carries a `limit` block, though: `limit.output` is a * warning. We never fabricate a default context window.
* required field in OpenCode's v1 provider schema, so it is always emitted
* (falling back to 8K when nothing else is known) — see #10940.
*/ */
function buildModelEntry( function buildModelEntry(
id: string, id: string,
@@ -303,15 +301,16 @@ function buildModelEntry(
const output = const output =
typeof userOutput === "number" && userOutput > 0 ? userOutput : (catalogOutput ?? 8_192); typeof userOutput === "number" && userOutput > 0 ? userOutput : (catalogOutput ?? 8_192);
// `limit.output` is REQUIRED by OpenCode's v1 provider schema regardless of // Emit `limit` only if we have at least one of context/output. We never
// whether the catalog (or the user's existing config) knows the model's // emit a half-baked limit block with only an `output` (would be misleading).
// context window — a model with no catalog metadata at all must still get if (
// a `limit` block, or OpenCode rejects the whole config with "Missing key typeof context === "number" ||
// provider.omniroute.models.{model}.limit.output" (#10940). `output` above typeof userOutput === "number" ||
// already resolves to a safe fallback (8K) when nothing else is known, so typeof catalogOutput === "number"
// we always emit it; `context`/`input` are added only when actually known. ) {
const limit: { context?: number; input?: number; output?: number } = { output }; const limit: { context?: number; input?: number; output?: number } = {};
if (typeof context === "number") limit.context = context; if (typeof context === "number") limit.context = context;
limit.output = output;
const userInput = existing?.limit?.input; const userInput = existing?.limit?.input;
if (typeof userInput === "number" && userInput > 0) { if (typeof userInput === "number" && userInput > 0) {
limit.input = userInput; limit.input = userInput;
@@ -320,6 +319,7 @@ function buildModelEntry(
if (typeof maxInput === "number" && maxInput > 0) limit.input = maxInput; if (typeof maxInput === "number" && maxInput > 0) limit.input = maxInput;
} }
entry.limit = limit; entry.limit = limit;
}
return entry; return entry;
} }

View File

@@ -0,0 +1,150 @@
// Regression test for #10955: generated `combos patch-*` CLI command sent a
// literal PATCH /api/combos/{id} to the server (405) because the generator
// dropped $ref'd path parameters (a bare `{ $ref }` object has no `.in`, so
// the `p.in === "path"` filter silently excluded it) and never emitted
// --body for the requestBody-less PATCH spec entry.
import test from "node:test";
import assert from "node:assert/strict";
import { execFileSync } from "node:child_process";
import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, rmSync } from "node:fs";
import { join, dirname } from "node:path";
import { fileURLToPath } from "node:url";
import { tmpdir } from "node:os";
const __dirname = dirname(fileURLToPath(import.meta.url));
const ROOT = join(__dirname, "..", "..");
const GENERATOR = join(ROOT, "scripts", "cli", "generate-api-commands.mjs");
const REAL_COMBOS = join(ROOT, "bin", "cli", "api-commands", "combos.mjs");
// Minimal fixture spec reproducing the exact shape that broke: a path
// parameter declared via $ref to a components/parameters entry, on a PATCH
// operation that also carries a requestBody.
const FIXTURE_SPEC = `
openapi: 3.0.3
info:
title: fixture
version: "1"
paths:
/api/widgets/{id}:
patch:
tags: [Widgets]
summary: Update widget
parameters:
- $ref: "#/components/parameters/ResourceId"
requestBody:
required: true
content:
application/json:
schema:
type: object
responses:
"200":
description: Updated widget
components:
parameters:
ResourceId:
name: id
in: path
required: true
schema:
type: string
`;
function runGenerator(specPath, outDir) {
execFileSync(process.execPath, ["--import", "tsx/esm", GENERATOR], {
cwd: ROOT,
env: { ...process.env, OPENAPI_SPEC: specPath, OPENAPI_OUT_DIR: outDir },
stdio: "pipe",
});
}
test("generator resolves a $ref path parameter into --id and substitutes {id} in the URL", () => {
const workDir = mkdtempSync(join(tmpdir(), "cli-api-gen-ref-"));
const specPath = join(workDir, "fixture.yaml");
const outDir = join(workDir, "out");
mkdirSync(outDir, { recursive: true });
writeFileSync(specPath, FIXTURE_SPEC);
try {
runGenerator(specPath, outDir);
const generated = readFileSync(join(outDir, "widgets.mjs"), "utf8");
// The $ref'd path param must have produced a required --id flag.
assert.match(
generated,
/\.requiredOption\("--id <id>"/,
"generated command must declare --id from the resolved $ref path parameter"
);
// The URL must be built with {id} substitution, not sent literally.
assert.match(
generated,
/url = url\.replace\("\{id\}", encodeURIComponent\(opts\.id/,
"generated command must substitute {id} in the URL"
);
assert.doesNotMatch(generated, /url = "\/api\/widgets\/\{id\}";\s*\n\s*const res/);
// requestBody presence must still produce --body.
assert.match(
generated,
/\.option\("--body <jsonOrPath>"/,
"generated command must declare --body for the requestBody"
);
} finally {
rmSync(workDir, { recursive: true, force: true });
}
});
test("generator rejects an unsupported $ref target instead of silently dropping the parameter", () => {
const workDir = mkdtempSync(join(tmpdir(), "cli-api-gen-ref-bad-"));
const specPath = join(workDir, "fixture.yaml");
const outDir = join(workDir, "out");
mkdirSync(outDir, { recursive: true });
writeFileSync(
specPath,
`
openapi: 3.0.3
info:
title: fixture
version: "1"
paths:
/api/widgets/{id}:
get:
tags: [Widgets]
summary: Get widget
parameters:
- $ref: "#/components/schemas/NotAParameter"
responses:
"200":
description: ok
components:
schemas:
NotAParameter:
type: object
`
);
try {
assert.throws(() => runGenerator(specPath, outDir));
} finally {
rmSync(workDir, { recursive: true, force: true });
}
});
test("real generated bin/cli/api-commands/combos.mjs has --id and --body on the PATCH combo command (#10955)", () => {
const src = readFileSync(REAL_COMBOS, "utf8");
const patchBlockMatch = src.match(/ {2}tag\.command\("patch-[^"]*"\)[\s\S]*?\n {2}(?=tag\.command\(|\})/);
assert.ok(patchBlockMatch, "combos.mjs must have a generated patch-* command block");
const patchBlock = patchBlockMatch[0];
assert.match(patchBlock, /\.requiredOption\("--id <id>"/, "PATCH combo command must require --id");
assert.match(
patchBlock,
/\.option\("--body <jsonOrPath>"/,
"PATCH combo command must accept --body"
);
assert.match(
patchBlock,
/url = url\.replace\("\{id\}", encodeURIComponent\(opts\.id/,
"PATCH combo command must substitute {id} in the URL, not send it literally"
);
});

View File

@@ -603,15 +603,12 @@ describe("config-generator", () => {
input: 100000, input: 100000,
output: 32768, output: 32768,
}); });
// #10940: `limit.output` is REQUIRED by OpenCode's v1 provider schema, assert.strictEqual(models["no-metadata"].limit, undefined);
// so even a model with zero catalog metadata still gets a `limit`
// block carrying the fallback output value; `context`/`input` stay
// omitted since neither the catalog nor the user knows them.
assert.deepStrictEqual(models["no-metadata"].limit, { output: 8192 });
for (const model of Object.values(models) as Array<{ limit?: { output?: number } }>) { for (const model of Object.values(models) as Array<{ limit?: { output?: number } }>) {
assert.ok( assert.ok(
typeof model.limit?.output === "number" && model.limit.output > 0, model.limit === undefined ||
(typeof model.limit.output === "number" && model.limit.output > 0),
"every emitted limit must contain a positive output" "every emitted limit must contain a positive output"
); );
} }

View File

@@ -1,85 +0,0 @@
import { describe, it } from "node:test";
import assert from "node:assert";
/**
* Regression guard for #10940: OpenCode rejects a generated config with
* "Missing key provider.omniroute.models.{model}.limit.output" whenever a
* model has no catalog metadata (no `context_length`, no
* `max_output_tokens`) and no existing user override. `limit.output` is a
* REQUIRED field in OpenCode's v1 provider schema, so it must always be
* emitted — even when nothing is known about the model.
*/
describe("opencode config generator — limit.output always emitted (#10940)", () => {
function makeCatalogResponse(models: unknown[]): unknown {
return { object: "list", data: models };
}
function stubFetchOnce(body: unknown, status = 200) {
const original = globalThis.fetch;
// @ts-ignore — globalThis.fetch signature is compatible for our purposes
globalThis.fetch = (async () => {
return new Response(JSON.stringify(body), {
status,
headers: { "content-type": "application/json" },
});
}) as typeof fetch;
return {
restore: () => {
globalThis.fetch = original;
},
};
}
it("RED-proving case: a model with NO context_length and NO max_output_tokens still gets a numeric limit.output", async () => {
// This model has no metadata whatsoever beyond its id — exactly the
// shape that used to leave `entry.limit` undefined entirely (issue #10940).
const stub = stubFetchOnce(
makeCatalogResponse([{ id: "metadataless-model", owned_by: "someProvider" }])
);
try {
const { generateOpencodeConfig } = await import(
"../../src/lib/cli-helper/config-generator/opencode.ts"
);
const out = await generateOpencodeConfig({
baseUrl: "http://localhost:20128",
apiKey: "sk-test",
});
const cfg = JSON.parse(out);
const entry = cfg.provider.omniroute.models["metadataless-model"];
assert.ok(entry, "model entry must exist in the generated config");
assert.ok(entry.limit, "entry.limit must be present even without catalog metadata");
assert.strictEqual(
typeof entry.limit.output,
"number",
`entry.limit.output must be a number, got ${JSON.stringify(entry.limit?.output)}`
);
assert.ok(entry.limit.output > 0, "entry.limit.output must be a positive number");
// context stays unknown — we must NOT fabricate it.
assert.strictEqual(entry.limit.context, undefined);
} finally {
stub.restore();
}
});
it("honors the catalog's max_output_tokens when present", async () => {
const stub = stubFetchOnce(
makeCatalogResponse([
{ id: "has-output-meta", owned_by: "someProvider", max_output_tokens: 4096 },
])
);
try {
const { generateOpencodeConfig } = await import(
"../../src/lib/cli-helper/config-generator/opencode.ts"
);
const out = await generateOpencodeConfig({
baseUrl: "http://localhost:20128",
apiKey: "sk-test",
});
const cfg = JSON.parse(out);
const entry = cfg.provider.omniroute.models["has-output-meta"];
assert.strictEqual(entry.limit.output, 4096);
} finally {
stub.restore();
}
});
});