chore(skills): regenerate cli-routing after combo create --models (#10954)

bin/cli/commands/combo.mjs

@@ -4,6 +4,7 @@ import { withRuntime } from "../runtime.mjs";
 import { t } from "../i18n.mjs";
 import { apiFetch } from "../api.mjs";
 import { emit } from "../output.mjs";
+import { resolveComboModels, collectModel } from "./comboModels.mjs";
 
 const VALID_STRATEGIES = [
   "priority",
@@ -125,10 +126,31 @@ export function registerCombo(program) {
         .choices(VALID_STRATEGIES)
         .default("priority")
     )
+    .option(
+      "--models <spec>",
+      "Models for the combo: comma-separated provider/model entries, or a JSON array " +
+        '(e.g. --models "openai/gpt-4o,anthropic/claude-3-opus" or ' +
+        '--models \'[{"model":"gpt-4o","providerId":"openai"}]\')'
+    )
+    .option(
+      "--model <spec>",
+      "Add one model to the combo (provider/model or bare model id) — repeatable",
+      collectModel,
+      []
+    )
     .action(async (name, opts, cmd) => {
       const globalOpts = cmd.parent.optsWithGlobals();
+      let models;
+      try {
+        models = resolveComboModels(opts);
+      } catch (err) {
+        console.error(`Error: ${err instanceof Error ? err.message : String(err)}`);
+        process.exit(1);
+        return;
+      }
       const exitCode = await runComboCreateCommand(name, opts.strategy, {
         ...opts,
+        models,
         output: globalOpts.output,
       });
       if (exitCode !== 0) process.exit(exitCode);
@@ -284,12 +306,14 @@ export async function runComboCreateCommand(name, strategy = "priority", opts =
     return 1;
   }
 
+  const models = Array.isArray(opts.models) ? opts.models : [];
+
   try {
     return await withRuntime(async ({ kind, api, db }) => {
       if (kind === "http") {
         const res = await api("/api/combos", {
           method: "POST",
-          body: { name, strategy, enabled: true, models: [], config: {} },
+          body: { name, strategy, enabled: true, models, config: {} },
           retry: false,
           acceptNotOk: true,
         });
@@ -305,7 +329,7 @@ export async function runComboCreateCommand(name, strategy = "priority", opts =
           console.error(`Combo '${name}' already exists. Delete it first.`);
           return 1;
         }
-        await db.combos.createCombo({ name, strategy, enabled: true, models: [], config: {} });
+        await db.combos.createCombo({ name, strategy, enabled: true, models, config: {} });
       }
 
       console.log(t("combo.created", { name }));

bin/cli/commands/comboModels.mjs

@@ -0,0 +1,142 @@
+// Parses the `--models` / `--model` options for `omniroute combo create` (#10954).
+//
+// Root cause of #10954: `combo create` only ever registered `--strategy`; the
+// HTTP body (POST /api/combos) and the local-db fallback (db.combos.createCombo)
+// both hardcoded `models: []`, so every combo created via the CLI came out
+// empty regardless of what the operator intended to route to.
+//
+// Accepted shapes mirror the server-side Zod union in
+// `src/shared/validation/schemas/combo.ts` (`comboModelEntry` /
+// `createComboSchema.models`) so a CLI-built payload never gets rejected by
+// the API that ultimately validates it:
+//   - a plain string ("provider/model" or a bare model id) — the server's
+//     `normalizeComboModels` (src/lib/combos/steps.ts) already splits the
+//     leading "provider/" segment off a plain string, so passing the raw
+//     token through is sufficient for the common case;
+//   - a structured `{ kind?: "model", model, providerId?, provider?, ... }`
+//     object;
+//   - a structured `{ kind: "combo-ref", comboName, ... }` object (nested
+//     combo reference).
+//
+// The CLI (bin/cli/**) ships as plain `.mjs` with relative-only imports — no
+// `@/` path aliases and no TS transpilation at runtime — so importing the
+// real Zod schema from `src/shared/validation/schemas/combo.ts` is not
+// viable here. This module instead validates the same minimal shape by hand
+// and stays a thin, independently testable unit.
+
+/**
+ * Validates one already-parsed combo model entry against the shape accepted
+ * by `comboModelEntry` (string | model-step | combo-ref). Throws with a
+ * 1-based, human-readable position when the entry does not match.
+ *
+ * @param {unknown} entry
+ * @param {number} index
+ * @returns {string | Record<string, unknown>}
+ */
+export function validateComboModelEntryShape(entry, index) {
+  const position = index + 1;
+
+  if (typeof entry === "string") {
+    const trimmed = entry.trim();
+    if (trimmed.length === 0) {
+      throw new Error(`--models entry #${position}: empty model string`);
+    }
+    if (trimmed.length > 300) {
+      throw new Error(`--models entry #${position}: model string exceeds 300 characters`);
+    }
+    return trimmed;
+  }
+
+  if (entry === null || typeof entry !== "object" || Array.isArray(entry)) {
+    throw new Error(`--models entry #${position}: must be a string or a JSON object`);
+  }
+
+  const kind = entry.kind;
+
+  if (kind === "combo-ref") {
+    if (typeof entry.comboName !== "string" || entry.comboName.trim().length === 0) {
+      throw new Error(
+        `--models entry #${position}: kind "combo-ref" requires a non-empty "comboName"`
+      );
+    }
+    return entry;
+  }
+
+  if (kind !== undefined && kind !== "model") {
+    throw new Error(`--models entry #${position}: unknown "kind" value ${JSON.stringify(kind)}`);
+  }
+
+  if (typeof entry.model !== "string" || entry.model.trim().length === 0) {
+    throw new Error(`--models entry #${position}: requires a non-empty "model"`);
+  }
+  if (entry.providerId !== undefined && typeof entry.providerId !== "string") {
+    throw new Error(`--models entry #${position}: "providerId" must be a string`);
+  }
+  if (entry.provider !== undefined && typeof entry.provider !== "string") {
+    throw new Error(`--models entry #${position}: "provider" must be a string`);
+  }
+
+  return entry;
+}
+
+/**
+ * Parses one `--models` spec — either a JSON array (`--models '[{"model":"gpt-4o"}]'`)
+ * or a comma-separated list of provider/model tokens
+ * (`--models 'openai/gpt-4o,anthropic/claude-3-opus'`) — into an array of
+ * combo model entries.
+ *
+ * @param {string} spec
+ * @returns {Array<string | Record<string, unknown>>}
+ */
+export function parseModelsSpec(spec) {
+  const trimmed = String(spec ?? "").trim();
+  if (trimmed.length === 0) return [];
+
+  if (trimmed.startsWith("[")) {
+    let parsed;
+    try {
+      parsed = JSON.parse(trimmed);
+    } catch (err) {
+      throw new Error(`--models: invalid JSON array (${err.message})`);
+    }
+    if (!Array.isArray(parsed)) {
+      throw new Error("--models: JSON value must be an array");
+    }
+    return parsed.map((entry, i) => validateComboModelEntryShape(entry, i));
+  }
+
+  return trimmed
+    .split(",")
+    .map((token) => token.trim())
+    .filter((token) => token.length > 0)
+    .map((token, i) => validateComboModelEntryShape(token, i));
+}
+
+/**
+ * Resolves the final `models` array for `combo create` from Commander opts:
+ * `--models <csv-or-json>` and/or repeatable `--model <spec>`.
+ *
+ * @param {{ models?: string, model?: string[] }} opts
+ * @returns {Array<string | Record<string, unknown>>}
+ */
+export function resolveComboModels(opts = {}) {
+  const result = [];
+
+  if (typeof opts.models === "string" && opts.models.trim().length > 0) {
+    result.push(...parseModelsSpec(opts.models));
+  }
+
+  if (Array.isArray(opts.model)) {
+    opts.model.forEach((token, i) => {
+      result.push(validateComboModelEntryShape(String(token).trim(), i));
+    });
+  }
+
+  return result;
+}
+
+/** Commander `collect`-style reducer for the repeatable `--model` option. */
+export function collectModel(value, previous) {
+  previous.push(value);
+  return previous;
+}

changelog.d/fixes/10954-combo-create-models.md

@@ -0,0 +1 @@
+- fix(cli): combo create accepts --models and no longer creates empty combos (#10954)

skills/cli-routing/SKILL.md

@@ -70,6 +70,11 @@ omniroute combo switch <name>
 
 Create a new routing combo
 
+**Flags:**
+
+- `--models <spec>`
+- `--model <spec>`
+
 **Example:**
 
 ```bash

tests/unit/cli-combo-create-models-10954.test.ts

@@ -0,0 +1,224 @@
+// Regression for #10954: `omniroute combo create` did not accept any way to
+// specify models — `bin/cli/commands/combo.mjs` only ever registered
+// `--strategy`, and both the HTTP body (POST /api/combos) and the local-db
+// fallback (db.combos.createCombo) hardcoded `models: []`. Every combo
+// created via the CLI came out empty.
+import test from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+import type { Command } from "commander";
+
+type CapturedOpts = Record<string, unknown>;
+
+interface MockFetchInit {
+  method?: string;
+  body?: string;
+}
+
+const ORIGINAL_DATA_DIR = process.env.DATA_DIR;
+const ORIGINAL_FETCH = globalThis.fetch;
+
+function createTempDataDir() {
+  return fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-cli-combo-models-"));
+}
+
+async function withComboEnv(fn: (dataDir: string) => Promise<void>) {
+  const dataDir = createTempDataDir();
+  process.env.DATA_DIR = dataDir;
+  // Mock fetch → simulates server offline so withRuntime falls back to DB.
+  globalThis.fetch = (async () => {
+    throw new Error("server offline");
+  }) as typeof fetch;
+
+  const originalLog = console.log;
+  console.log = () => {};
+
+  try {
+    await fn(dataDir);
+  } finally {
+    console.log = originalLog;
+    globalThis.fetch = ORIGINAL_FETCH;
+    fs.rmSync(dataDir, { recursive: true, force: true });
+
+    if (ORIGINAL_DATA_DIR === undefined) delete process.env.DATA_DIR;
+    else process.env.DATA_DIR = ORIGINAL_DATA_DIR;
+  }
+}
+
+function makeHealthAndComboFetch(capture: { body: CapturedOpts | null }) {
+  return (async (url: string, opts?: MockFetchInit) => {
+    if (String(url).includes("/api/health")) {
+      return {
+        ok: true,
+        status: 200,
+        json: async () => ({ status: "ok" }),
+        text: async () => "{}",
+        headers: new Headers(),
+      };
+    }
+    if (String(url).includes("/api/combos") && opts?.method === "POST") {
+      capture.body = opts?.body ? JSON.parse(opts.body) : null;
+      return {
+        ok: true,
+        status: 201,
+        json: async () => ({ id: "combo-1", ...capture.body }),
+        text: async () => JSON.stringify(capture.body),
+        headers: new Headers(),
+      };
+    }
+    throw new Error(`unexpected fetch: ${url}`);
+  }) as unknown as typeof fetch;
+}
+
+// RED (on untouched code): `combo create` only registers `--strategy` — an
+// unrecognized `--models` option makes Commander (in strict `exitOverride`
+// mode) throw "unknown option '--models'" instead of parsing.
+test("combo create — parses --models without throwing (Commander option registered)", async () => {
+  const { registerCombo } = await import("../../bin/cli/commands/combo.mjs");
+  const { Command } = await import("commander");
+
+  const prog = new Command().exitOverride();
+  registerCombo(prog);
+  const comboCmd = prog.commands.find((c: Command) => c.name() === "combo") as Command;
+  const createCmd = comboCmd.commands.find((c: Command) => c.name() === "create") as Command;
+
+  let capturedOpts: CapturedOpts | null = null;
+  createCmd.action((_name: string, opts: CapturedOpts) => {
+    capturedOpts = opts;
+  });
+
+  await prog.parseAsync(
+    ["node", "x", "combo", "create", "my-combo", "--models", "openai/gpt-4o,anthropic/claude-3-opus"],
+    { from: "node" }
+  );
+
+  assert.ok(capturedOpts, "action should have been called");
+  assert.equal(capturedOpts.models, "openai/gpt-4o,anthropic/claude-3-opus");
+});
+
+test("combo create — repeatable --model is registered and collected", async () => {
+  const { registerCombo } = await import("../../bin/cli/commands/combo.mjs");
+  const { Command } = await import("commander");
+
+  const prog = new Command().exitOverride();
+  registerCombo(prog);
+  const comboCmd = prog.commands.find((c: Command) => c.name() === "combo") as Command;
+  const createCmd = comboCmd.commands.find((c: Command) => c.name() === "create") as Command;
+
+  let capturedOpts: CapturedOpts | null = null;
+  createCmd.action((_name: string, opts: CapturedOpts) => {
+    capturedOpts = opts;
+  });
+
+  await prog.parseAsync(
+    [
+      "node",
+      "x",
+      "combo",
+      "create",
+      "my-combo",
+      "--model",
+      "openai/gpt-4o",
+      "--model",
+      "anthropic/claude-3-opus",
+    ],
+    { from: "node" }
+  );
+
+  assert.deepEqual(capturedOpts.model, ["openai/gpt-4o", "anthropic/claude-3-opus"]);
+});
+
+test("comboModels.resolveComboModels — parses CSV provider/model tokens", async () => {
+  const { resolveComboModels } = await import("../../bin/cli/commands/comboModels.mjs");
+  const models = resolveComboModels({ models: "openai/gpt-4o, anthropic/claude-3-opus" });
+  assert.deepEqual(models, ["openai/gpt-4o", "anthropic/claude-3-opus"]);
+});
+
+test("comboModels.resolveComboModels — parses a JSON array of structured entries", async () => {
+  const { resolveComboModels } = await import("../../bin/cli/commands/comboModels.mjs");
+  const models = resolveComboModels({
+    models: JSON.stringify([
+      { model: "gpt-4o", providerId: "openai" },
+      { kind: "combo-ref", comboName: "fallback-combo" },
+    ]),
+  });
+  assert.deepEqual(models, [
+    { model: "gpt-4o", providerId: "openai" },
+    { kind: "combo-ref", comboName: "fallback-combo" },
+  ]);
+});
+
+test("comboModels.resolveComboModels — rejects an invalid JSON entry shape", async () => {
+  const { resolveComboModels } = await import("../../bin/cli/commands/comboModels.mjs");
+  assert.throws(
+    () => resolveComboModels({ models: JSON.stringify([{ providerId: "openai" }]) }),
+    /requires a non-empty "model"/
+  );
+});
+
+test("comboModels.resolveComboModels — merges --models and repeated --model", async () => {
+  const { resolveComboModels } = await import("../../bin/cli/commands/comboModels.mjs");
+  const models = resolveComboModels({
+    models: "openai/gpt-4o",
+    model: ["anthropic/claude-3-opus"],
+  });
+  assert.deepEqual(models, ["openai/gpt-4o", "anthropic/claude-3-opus"]);
+});
+
+// GREEN: end-to-end through runComboCreateCommand — local-db fallback path.
+test("combo create (db fallback) — stores the parsed --models, no longer creates an empty combo", async () => {
+  await withComboEnv(async () => {
+    const { runComboCreateCommand } = await import("../../bin/cli/commands/combo.mjs");
+    const { resolveComboModels } = await import("../../bin/cli/commands/comboModels.mjs");
+
+    const models = resolveComboModels({ models: "openai/gpt-4o,anthropic/claude-3-opus" });
+    const result = await runComboCreateCommand("models-combo", "priority", { models });
+    assert.equal(result, 0);
+
+    const { getComboByName } = await import("../../src/lib/db/combos.ts");
+    const combo = await getComboByName("models-combo");
+    assert.ok(combo);
+    // The repository layer (src/lib/db/repositories/sqliteComboRepository.ts)
+    // normalizes plain "provider/model" strings into structured ComboStep
+    // objects on write — assert on the normalized shape rather than raw
+    // string equality, and above all assert the combo is no longer empty
+    // (the actual #10954 regression).
+    const storedModels = combo.models as Array<Record<string, unknown>>;
+    assert.equal(storedModels.length, 2, "combo must not be created empty");
+    assert.equal(storedModels[0].model, "openai/gpt-4o");
+    assert.equal(storedModels[0].providerId, "openai");
+    assert.equal(storedModels[1].model, "anthropic/claude-3-opus");
+    assert.equal(storedModels[1].providerId, "anthropic");
+  });
+});
+
+// GREEN: end-to-end through runComboCreateCommand — HTTP path, verifies the
+// POST /api/combos body actually carries the parsed models.
+test("combo create (HTTP) — POST /api/combos body carries the parsed models", async () => {
+  const dataDir = createTempDataDir();
+  process.env.DATA_DIR = dataDir;
+  const capture: { body: CapturedOpts | null } = { body: null };
+  globalThis.fetch = makeHealthAndComboFetch(capture);
+  const originalLog = console.log;
+  console.log = () => {};
+
+  try {
+    const { runComboCreateCommand } = await import("../../bin/cli/commands/combo.mjs");
+    const { resolveComboModels } = await import("../../bin/cli/commands/comboModels.mjs");
+
+    const models = resolveComboModels({ models: "openai/gpt-4o,anthropic/claude-3-opus" });
+    const result = await runComboCreateCommand("http-models-combo", "priority", { models });
+
+    assert.equal(result, 0);
+    assert.ok(capture.body, "POST /api/combos should have been called");
+    assert.deepEqual(capture.body.models, ["openai/gpt-4o", "anthropic/claude-3-opus"]);
+  } finally {
+    console.log = originalLog;
+    globalThis.fetch = ORIGINAL_FETCH;
+    fs.rmSync(dataDir, { recursive: true, force: true });
+    if (ORIGINAL_DATA_DIR === undefined) delete process.env.DATA_DIR;
+    else process.env.DATA_DIR = ORIGINAL_DATA_DIR;
+  }
+});