From 2243b9fbb994f679ad2160725d7bfce04d41d025 Mon Sep 17 00:00:00 2001 From: Josh France <12610835+16francej@users.noreply.github.com> Date: Wed, 29 Jul 2026 20:20:05 -0700 Subject: [PATCH 1/2] Let the declared model provider choose the base model defaultModelForHarness fell back to a provider-blind constant whenever no model was pinned. Provider availability was only ever used to reject a model, never to choose one, so a deployment holding only an OpenRouter or OpenAI key got an Anthropic default and refused every turn while check, plan, doctor and up all passed. The fallback is now provider-aware at the one function all eight resolution paths flow through. MODEL_PROVIDER reaches core as real configuration rather than only a secret gate, and availability is intersected with what the harness can route. With no declaration the keys present decide, which fixes the same defect for deployments that predate modelProvider. Harness and provider pairs that can never serve a turn, and a model pin from another vendor, are refused at config load in both the CLI and core instead of at the first message. Two invariants the base branch held together with comments are gone with the thing they explained: duplicate-name secret specs collapse to one per name via an any condition and an optionalOtherwise flag, so array position no longer decides a description, and requiredWhen takes a list, retiring a dedupe pass and an if-chain fallthrough that treated any unmatched gate as the Linear one. Both catalogs now have uniqueness tests. Co-Authored-By: Claude Opus 5 --- cli/src/backends/doctor.ts | 15 ++--- cli/src/config.ts | 35 ++++++++--- cli/src/provider-scaffold.ts | 12 ++-- cli/src/secrets.ts | 82 ++++++++++--------------- cli/templates/deployment/deployment.md | 22 +++++-- cli/test/config.test.ts | 25 ++++++++ cli/test/secrets.test.ts | 14 ++++- src/api/routes/admin/model-providers.ts | 2 +- src/config.ts | 63 +++++++++++++++++++ src/deployment/secret-schema.ts | 80 ++++++++++++------------ src/index.ts | 20 +++--- src/model/model-credential-store.ts | 9 +-- src/model/pi-models.ts | 35 +++++++++-- src/wiring.ts | 14 ++--- test/base-model-serviceability.test.ts | 67 +++++++++++++++++++- test/config.test.ts | 76 ++++++++++++++++++++++- test/pi-models.test.ts | 41 +++++++++++++ test/secret-schema-drift.test.ts | 33 ++++++++++ 18 files changed, 491 insertions(+), 154 deletions(-) diff --git a/cli/src/backends/doctor.ts b/cli/src/backends/doctor.ts index cd6db1395..837139193 100644 --- a/cli/src/backends/doctor.ts +++ b/cli/src/backends/doctor.ts @@ -1,6 +1,12 @@ import { existsSync, readFileSync } from "node:fs"; import { join, resolve } from "node:path"; -import { MODEL_PROVIDER_KEYS, validatePortalTrust, type ModelProvider, type QmConfig } from "../config.ts"; +import { + MODEL_PROVIDER_BASE_MODELS, + MODEL_PROVIDER_KEYS, + validatePortalTrust, + type ModelProvider, + type QmConfig, +} from "../config.ts"; import { CliError, errMessage, step, warn } from "../log.ts"; import { capture, deploymentSecretValue, flyBin, isInvalidSecret, readEnvFile, which } from "../util.ts"; import { computedSecrets } from "../secrets.ts"; @@ -192,11 +198,6 @@ export async function doctorCommon( await baseModelCheck(config, secrets); } -/** - * Prove the base-model key is accepted before the deployment is called finished. The Admin - * page validates a key on entry; a deployment that ships its key from `.env` gets no such - * feedback, and an unusable key would otherwise surface as a failed first chat message. - */ async function baseModelCheck(config: QmConfig, secrets: Map): Promise { const provider = config.modelProvider; if (!provider) { @@ -210,7 +211,7 @@ async function baseModelCheck(config: QmConfig, secrets: Map): P return; } await modelProviderCheck(provider, key); - step(`base model provider ${provider}: ${name} accepted`); + step(`base model provider ${provider}: ${name} accepted, serving ${MODEL_PROVIDER_BASE_MODELS[provider]}`); } const MODEL_PROVIDER_PROBES: Readonly< diff --git a/cli/src/config.ts b/cli/src/config.ts index e04c443bf..120de00f5 100644 --- a/cli/src/config.ts +++ b/cli/src/config.ts @@ -104,22 +104,27 @@ export function awsWorkloadArchitecture(config: QmConfig, workload: string): "ar return service.architecture ?? "arm64"; } -/** - * The vendor supplying the deployment's base model. Selecting one makes that vendor's - * API key a required deployment secret, so `qm setup` collects it and `qm up` refuses - * to deploy a stack that cannot run a single agent turn. Leaving it unset preserves the - * older flow, where an administrator supplies the key from the Admin page after deploy. - */ export const MODEL_PROVIDERS = ["anthropic", "openai", "openrouter"] as const; export type ModelProvider = (typeof MODEL_PROVIDERS)[number]; -/** The API key each provider's base model is billed against. */ export const MODEL_PROVIDER_KEYS: Readonly> = { anthropic: "ANTHROPIC_API_KEY", openai: "OPENAI_API_KEY", openrouter: "OPENROUTER_API_KEY", }; +export const MODEL_PROVIDER_HARNESSES: Readonly> = { + anthropic: ["pi", "opencode", "claude", "mock"], + openai: ["pi", "opencode", "codex", "mock"], + openrouter: ["pi", "mock"], +}; + +export const MODEL_PROVIDER_BASE_MODELS: Readonly> = { + anthropic: "claude-opus-5", + openai: "gpt-5.6-sol", + openrouter: "openrouter/auto", +}; + export const isModelProvider = (value: unknown): value is ModelProvider => typeof value === "string" && (MODEL_PROVIDERS as readonly string[]).includes(value); @@ -674,6 +679,7 @@ function validate(raw: unknown, path: string): QmConfig { out.aws = validateAws(o["aws"], path, runnableServices(services), configuredSecretNames); } if (target === "aws" && !out.aws) throw new CliError(`${path}: target "aws" requires an "aws" block`); + validateModelProvider(out, path); validatePortalTrust(out, path); if (target === "aws") { validateAwsFrontDoor(out, path); @@ -704,6 +710,21 @@ function validate(raw: unknown, path: string): QmConfig { return out; } +function configuredHarness(config: QmConfig): string { + return config.env.core?.HARNESS?.trim() || (config.target === "fly" ? "pi" : "mock"); +} + +function validateModelProvider(config: QmConfig, path: string): void { + const provider = config.modelProvider; + if (!provider) return; + const harness = configuredHarness(config); + if (!MODEL_PROVIDER_HARNESSES[provider].includes(harness)) { + throw new CliError( + `${path}: modelProvider "${provider}" cannot serve a base model on env.core.HARNESS "${harness}" — that harness runs no ${provider} model, so every agent turn would be refused. Use ${MODEL_PROVIDER_HARNESSES[provider].join(", ")}, or pick a provider that harness can bill.`, + ); + } +} + const validEmailDomain = (value: string): boolean => { if (value.length > 253 || !value.includes(".")) return false; return value diff --git a/cli/src/provider-scaffold.ts b/cli/src/provider-scaffold.ts index 1c718eb44..ab8e23252 100644 --- a/cli/src/provider-scaffold.ts +++ b/cli/src/provider-scaffold.ts @@ -47,14 +47,16 @@ function renderConfig(orgId: string, values: ConfigValues): string { // The vendor supplying the base model: "anthropic", "openai", or "openrouter" // (one key, many models). Naming one makes that vendor's API key a required - // deployment secret: \`qm setup\` collects it, \`qm doctor\` proves the provider - // accepts it, and \`qm up\` refuses a stack that cannot serve an agent turn. - // Delete this line to leave the base model unset and have an administrator add - // the key from the Admin page after deploy instead. + // deployment secret and points the base model at that vendor: \`qm setup\` + // collects the key, \`qm doctor\` proves the provider accepts it, and \`qm up\` + // refuses a stack that cannot serve an agent turn. Delete this line to leave + // the base model unset and have an administrator add the key from the Admin + // page after deploy instead. "modelProvider": ${JSON.stringify(values.modelProvider)}, // Optional base model id, passed to the harness (e.g. "claude-opus-4-6"). - // Omit it to use the harness default for the provider above. + // Omit it and the deployment uses the default model for the provider above. + // It must be a model that provider can bill. // "model": "", ${values.providerFields} // First-party services to run. The full set: "core" (the agent runtime and API, diff --git a/cli/src/secrets.ts b/cli/src/secrets.ts index c2631a8cb..5de7571a9 100644 --- a/cli/src/secrets.ts +++ b/cli/src/secrets.ts @@ -10,6 +10,7 @@ type SecretCondition = | { kind: "service-enabled"; service: DeclaredServiceName } | { kind: "service-absent"; service: DeclaredServiceName } | { kind: "all"; conditions: SecretCondition[] } + | { kind: "any"; conditions: SecretCondition[] } | { kind: "target"; target: QmConfig["target"] } | { kind: "model-provider"; provider: ModelProvider }; @@ -17,7 +18,7 @@ export interface SecretSpec { name: string; service: DeclaredServiceName; envName?: string; - required: boolean | { when: SecretCondition }; + required: boolean | { when: SecretCondition; optionalOtherwise?: true }; description: string; generate?: string; managedBy?: "operator" | "terraform"; @@ -38,52 +39,34 @@ export const MINT_JWK = "node -e \"const {generateKeyPairSync}=require('node:crypto');process.stdout.write(JSON.stringify(generateKeyPairSync('ec',{namedCurve:'P-256'}).privateKey.export({format:'jwk'})))\""; export const FIRST_PARTY_SECRET_SPECS: readonly SecretSpec[] = [ - // The base-model key is required whenever the deployment names its provider, so that - // `qm setup` collects it and `qm up` cannot produce a stack that fails its first agent - // turn. Anthropic and OpenRouter keep an unconditional twin further down: omitting - // `modelProvider` leaves the key an optional fallback an administrator supplies from the - // Admin page, which is how deployments predating `modelProvider` behave. Those two stay - // ahead of their twins — `computedSecrets` describes a secret from the first spec whose - // condition matches, and the base-model wording is the accurate one once it applies. { name: "ANTHROPIC_API_KEY", service: "core", - required: { when: { kind: "model-provider", provider: "anthropic" } }, - description: 'Anthropic API key for the deployment base model (modelProvider "anthropic").', + required: { when: { kind: "model-provider", provider: "anthropic" }, optionalOtherwise: true }, + description: + 'Anthropic API key: bills the base model when modelProvider is "anthropic", an optional deployment fallback otherwise.', }, { name: "OPENROUTER_API_KEY", service: "core", - required: { when: { kind: "model-provider", provider: "openrouter" } }, - description: 'OpenRouter API key for the deployment base model (modelProvider "openrouter").', - }, - { - name: "ANTHROPIC_API_KEY", - service: "core", - required: false, - description: "Optional deployment fallback for Pi; admins can configure the base model key after deploy.", - }, - // OPENAI_API_KEY keeps the Codex rule ahead of the base-model rule: it is the more - // descriptive of the two, and `.env.example` documents a dormant secret from its first - // spec. Whichever rule fires, `computedSecrets` collapses them to one required secret. - { - name: "OPENAI_API_KEY", - service: "core", - required: { when: { kind: "env-equals", service: "core", name: "HARNESS", value: "codex" } }, + required: { when: { kind: "model-provider", provider: "openrouter" }, optionalOtherwise: true }, description: - "OpenAI API key used by the Codex harness (its CLI cannot do browser OAuth in a container); an optional deployment fallback for Pi otherwise.", + 'OpenRouter API key: bills the base model when modelProvider is "openrouter", an optional deployment fallback otherwise.', }, { name: "OPENAI_API_KEY", service: "core", - required: { when: { kind: "model-provider", provider: "openai" } }, - description: 'OpenAI API key for the deployment base model (modelProvider "openai").', - }, - { - name: "OPENROUTER_API_KEY", - service: "core", - required: false, - description: "Optional deployment fallback for Pi; admins can configure the base model key after deploy.", + required: { + when: { + kind: "any", + conditions: [ + { kind: "env-equals", service: "core", name: "HARNESS", value: "codex" }, + { kind: "model-provider", provider: "openai" }, + ], + }, + }, + description: + 'OpenAI API key: the Codex harness needs it (its CLI cannot do browser OAuth in a container), and it bills the base model when modelProvider is "openai".', }, { name: "PUBLIC_API_URL", @@ -387,6 +370,7 @@ function conditionMatches(config: QmConfig, condition: SecretCondition): boolean if (condition.kind === "service-enabled") return config.services.includes(condition.service); if (condition.kind === "service-absent") return !config.services.includes(condition.service); if (condition.kind === "all") return condition.conditions.every((nested) => conditionMatches(config, nested)); + if (condition.kind === "any") return condition.conditions.some((nested) => conditionMatches(config, nested)); if (condition.kind === "target") return config.target === condition.target; if (condition.kind === "model-provider") return config.modelProvider === condition.provider; if (condition.kind === "env-all-absent") { @@ -418,12 +402,18 @@ function targetEnvDefault(config: QmConfig, service: string, name: string): stri return rendered; } +function requirementFor(config: QmConfig, spec: SecretSpec): boolean | null { + if (typeof spec.required === "boolean") return spec.required; + if (conditionMatches(config, spec.required.when)) return true; + return spec.required.optionalOtherwise ? false : null; +} + export function computedSecrets(config: QmConfig): ComputedSecret[] { const byName = new Map(); for (const spec of FIRST_PARTY_SECRET_SPECS) { if (!config.services.includes(spec.service)) continue; - if (typeof spec.required !== "boolean" && !conditionMatches(config, spec.required.when)) continue; - const required = spec.required !== false; + const required = requirementFor(config, spec); + if (required === null) continue; const current = byName.get(spec.name); if (current) { if (spec.envName) { @@ -558,6 +548,7 @@ function conditionClause(condition: SecretCondition): string { if (condition.kind === "service-enabled") return `the ${condition.service} service is enabled`; if (condition.kind === "service-absent") return `the ${condition.service} service is not enabled`; if (condition.kind === "all") return condition.conditions.map(conditionClause).join(" and "); + if (condition.kind === "any") return condition.conditions.map(conditionClause).join(" or "); if (condition.kind === "target") return `the target is ${condition.target}`; if (condition.kind === "env-all-absent") return `none of env.${condition.service}.{${condition.names.join(", ")}} are set`; @@ -594,22 +585,15 @@ export function renderEnvExample(config: QmConfig): string { } const activeNames = new Set(active.map((secret) => secret.name)); const inactive = FIRST_PARTY_SECRET_SPECS.filter( - (spec, i, all) => !activeNames.has(spec.name) && all.findIndex((s) => s.name === spec.name) === i, + (spec, i, all) => !activeNames.has(spec.name) && all.findIndex((other) => other.name === spec.name) === i, ); - const clauseFor = (spec: SecretSpec): string => - [ + for (const spec of inactive) { + const clauses = [ ...(config.services.includes(spec.service) ? [] : [`the ${spec.service} service is enabled`]), ...(typeof spec.required === "boolean" ? [] : [conditionClause(spec.required.when)]), - ].join(" and "); - for (const spec of inactive) { - // One secret can answer to several independent rules — an OpenAI base model or the - // Codex harness, say. List them as alternatives so the catalog does not imply the - // first rule is the only way the secret becomes required. - const rules = FIRST_PARTY_SECRET_SPECS.filter((other) => other.name === spec.name); - const clauses = [...new Set(rules.map(clauseFor).filter(Boolean))]; - const optionalEvenThen = rules.every((rule) => rule.required === false); + ]; lines.push(`# ${spec.description} (${spec.service})`); - lines.push(`# Needed when ${clauses.join(" or ")}${optionalEvenThen ? " (optional even then)" : ""}.`); + lines.push(`# Needed when ${clauses.join(" and ")}${spec.required === false ? " (optional even then)" : ""}.`); if (spec.generate) lines.push(`# Generate with: ${generate(spec.generate)}`); lines.push(spec.managedBy === "terraform" ? `# ${spec.name}= # populated by Terraform` : `# ${spec.name}=`); lines.push(""); diff --git a/cli/templates/deployment/deployment.md b/cli/templates/deployment/deployment.md index 175040af6..350df1fc3 100644 --- a/cli/templates/deployment/deployment.md +++ b/cli/templates/deployment/deployment.md @@ -155,6 +155,14 @@ has none. Treat a rejected key exactly like a rejected sign-in credential: stop and get a working one rather than deploying a stack that greets the administrator and then fails their first message. +`modelProvider` also picks the model itself, so no model id has to be chosen at +deploy time: Anthropic serves `claude-opus-5`, OpenAI `gpt-5.6-sol`, OpenRouter +`openrouter/auto`. Set `model` in `qm.config.jsonc` only to override that, and +only with a model the chosen provider can bill — a mismatch is refused at +startup rather than at the first message. The same rule covers the harness: +`HARNESS` `codex` runs OpenAI models alone, `claude` runs Anthropic models +alone, and `openrouter` needs the default `pi` harness. + An operator may still prefer to hold the key centrally and rotate it from the Admin page. That is a deliberate choice, not the default: drop `modelProvider` from `qm.config.jsonc`, note in the handoff that the deployment has no base model @@ -178,12 +186,14 @@ npm exec qm -- outputs --json ``` Open `adminOnboardingUrl` from the JSON output and confirm Model provider -already reports the deployment's base model. It does when `modelProvider` is -set: the key travelled with the rest of the deployment secrets, so there is -nothing to paste here. Enter and validate a key on that page only when the -operator chose to defer, or when they are replacing the deployment key with one -they would rather rotate from Admin — the write-only surface stores it in -durable encrypted storage and takes precedence over the deployment key. +reports the chosen vendor as configured, sourced from the environment. It does +when `modelProvider` is set: the key travelled with the rest of the deployment +secrets, so there is nothing to paste here. Enter and validate a key on that +page only when the operator chose to defer, or when they are replacing the +deployment key with one they would rather rotate from Admin — the write-only +surface stores it in durable encrypted storage and takes precedence over the +deployment key. On the deferred route, set Base model on that same page after +the key: a key alone leaves the deployment on a model it cannot bill. Never paste any provider key into chat or terminal output. `.env` is the one place a deployment key belongs, and `qm secrets push` moves it without printing diff --git a/cli/test/config.test.ts b/cli/test/config.test.ts index b44b6795c..e8cca5699 100644 --- a/cli/test/config.test.ts +++ b/cli/test/config.test.ts @@ -1135,3 +1135,28 @@ test("aws.services logGroup and stopTimeout adopt live task-def values and valid ); } }); + +test("modelProvider must name a vendor the configured harness can bill", () => { + withConfig({ modelProvider: "openrouter", env: { core: { HARNESS: "pi" } } }, ({ path }) => { + assert.equal(loadConfigAt(path).config.modelProvider, "openrouter"); + }); + withConfig({ modelProvider: "openrouter", env: { core: { HARNESS: "codex" } } }, ({ path }) => { + assert.throws( + () => loadConfigAt(path), + /modelProvider "openrouter" cannot serve a base model on env.core.HARNESS "codex"/, + ); + }); + withConfig({ modelProvider: "anthropic", env: { core: { HARNESS: "codex" } } }, ({ path }) => { + assert.throws(() => loadConfigAt(path), /cannot serve a base model/); + }); + withConfig({ modelProvider: "openai", env: { core: { HARNESS: "codex" } } }, ({ path }) => { + assert.equal(loadConfigAt(path).config.modelProvider, "openai"); + }); + withConfig({ modelProvider: "openrouter" }, ({ path }) => { + assert.equal( + loadConfigAt(path).config.modelProvider, + "openrouter", + "an unset harness is mock, which bills anything", + ); + }); +}); diff --git a/cli/test/secrets.test.ts b/cli/test/secrets.test.ts index 1427cf2d9..e44035be3 100644 --- a/cli/test/secrets.test.ts +++ b/cli/test/secrets.test.ts @@ -13,7 +13,7 @@ import { secretsForService, type ComputedSecret, } from "../src/secrets.ts"; -import { isReservedContainerName, pluginNameError } from "../src/services.ts"; +import { isReservedContainerName, pluginNameError, SERVICE_NAMES } from "../src/services.ts"; function makeConfig(overrides: Partial = {}): QmConfig { return { @@ -368,3 +368,15 @@ test("an explicit sandbox.backend wins, and non-fly targets keep their own defau }); assert.equal(sandboxCoreEnv(docker).env.SANDBOX_BACKEND, undefined); }); + +test("the .env.example catalog names every secret exactly once", () => { + for (const services of [["core"], ["core", "portal"], ["core", "portal", "auth"], SERVICE_NAMES] as const) { + const rendered = renderEnvExample(makeConfig({ services: [...services] as QmConfig["services"] })); + const declared = rendered + .split("\n") + .map((line) => /^#?\s*([A-Z0-9_]+)=/.exec(line)?.[1]) + .filter((name): name is string => Boolean(name)); + const duplicated = declared.filter((name, i) => declared.indexOf(name) !== i); + assert.deepEqual(duplicated, [], `services=${services.join("+")} lists a secret twice`); + } +}); diff --git a/src/api/routes/admin/model-providers.ts b/src/api/routes/admin/model-providers.ts index a156a7e54..66a2e0987 100644 --- a/src/api/routes/admin/model-providers.ts +++ b/src/api/routes/admin/model-providers.ts @@ -1,4 +1,4 @@ -import { isModelProvider, type ModelProvider } from "../../../model/model-credential-store.ts"; +import { isModelProvider, type ModelProvider } from "../../../model/pi-models.ts"; import { selectableModelCatalog } from "../../../model/model-catalog.ts"; import { sendJson } from "../../http.ts"; import type { ApiCtx } from "../route.ts"; diff --git a/src/config.ts b/src/config.ts index 6804a40b6..bc13f7ce9 100644 --- a/src/config.ts +++ b/src/config.ts @@ -11,6 +11,16 @@ import { validateCoreSecretEnv } from "./deployment/secret-schema.ts"; import { DEFAULT_CAPTURE_QUIET_MS } from "./memory/strategies/per-turn.ts"; import { parseSecurityPosture, type SecurityPosture } from "./security/security-posture.ts"; import { slackPluginConfigFromEnv, type SlackPluginConfig } from "./slack/config.ts"; +import { + MODEL_PROVIDERS, + defaultModelForProvider, + isModelProvider, + modelProviderAvailabilityFor, + onlyProvider, + resolveModel, + type ModelProvider, + type ModelProviderAvailability, +} from "./model/pi-models.ts"; export interface Config { production: boolean; @@ -41,6 +51,7 @@ export interface Config { anthropicApiKey?: string; openaiApiKey?: string; openrouterApiKey?: string; + modelProvider?: ModelProvider; piCaptureRequests: boolean; piSystemCacheSplit: boolean; sessionTapeMode: "shadow" | "serve"; @@ -141,6 +152,24 @@ export function configuredModelForHarness(config: Config, harness: string): stri return config.modelId; } +export function providerKeysPresent(config: Config): ModelProviderAvailability { + return { + anthropic: Boolean(config.anthropicApiKey), + openai: Boolean(config.openaiApiKey), + openrouter: Boolean(config.openrouterApiKey), + }; +} + +export function baseModelProviders(config: Config): ModelProviderAvailability { + const declared = config.modelProvider ? onlyProvider(config.modelProvider) : providerKeysPresent(config); + const routable = modelProviderAvailabilityFor(config.harness, declared); + return { + anthropic: declared.anthropic && routable.anthropic, + openai: declared.openai && routable.openai, + openrouter: declared.openrouter && routable.openrouter, + }; +} + interface AwsSandboxEnv { region: string; profile?: string; @@ -511,11 +540,44 @@ function defaultPluginSkillDirs(): string[] { } } +function modelProviderEnvStrict(env: NodeJS.ProcessEnv): ModelProvider | undefined { + const declared = env.MODEL_PROVIDER?.trim(); + if (!declared) return undefined; + if (!isModelProvider(declared)) { + throw new Error( + `MODEL_PROVIDER=${JSON.stringify(declared)} is not recognized — use ${MODEL_PROVIDERS.join(", ")}.`, + ); + } + const harness = harnessEnvStrict(env.HARNESS); + const base = defaultModelForProvider(harness, declared); + if (!base) { + throw new Error( + `MODEL_PROVIDER=${declared} cannot serve a base model on HARNESS=${harness} — that harness runs no ${declared} model, so every turn would be refused.`, + ); + } + const [pinName, pinned] = harnessModelPin(env, harness); + const pinnedProvider = pinned ? resolveModel(pinned)?.provider : undefined; + if (pinned && pinnedProvider && pinnedProvider !== declared) { + throw new Error( + `${pinName}=${pinned} is a ${pinnedProvider} model but MODEL_PROVIDER=${declared} — the deployment holds no key that can bill it.`, + ); + } + return declared; +} + +function harnessModelPin(env: NodeJS.ProcessEnv, harness: Config["harness"]): [string, string | undefined] { + if (harness === "codex") return ["CODEX_MODEL", env.CODEX_MODEL?.trim()]; + if (harness === "claude") return ["CLAUDE_MODEL", env.CLAUDE_MODEL?.trim()]; + if (harness === "opencode" && env.OPENCODE_MODEL?.trim()) return ["OPENCODE_MODEL", env.OPENCODE_MODEL.trim()]; + return ["PI_MODEL", env.PI_MODEL?.trim()]; +} + export function loadConfig(env: NodeJS.ProcessEnv = process.env): Config { const missingSecrets = validateCoreSecretEnv(env); if (missingSecrets.length) { throw new Error(`missing or insecure required core secrets: ${missingSecrets.join(", ")}`); } + const modelProvider = modelProviderEnvStrict(env); for (const key of ["SESSION_STORE", "RUN_STORE", "ARTIFACT_STORE"] as const) { if (env[key] === "sqlite") { throw new Error( @@ -682,6 +744,7 @@ export function loadConfig(env: NodeJS.ProcessEnv = process.env): Config { ...(env.ANTHROPIC_API_KEY ? { anthropicApiKey: env.ANTHROPIC_API_KEY } : {}), ...(env.OPENAI_API_KEY ? { openaiApiKey: env.OPENAI_API_KEY } : {}), ...(env.OPENROUTER_API_KEY ? { openrouterApiKey: env.OPENROUTER_API_KEY } : {}), + ...(modelProvider ? { modelProvider } : {}), ...(env.ADMIN_GRANTS ? { adminGrants: env.ADMIN_GRANTS } : {}), piCaptureRequests: boolEnvStrict("PI_CAPTURE_REQUESTS", env.PI_CAPTURE_REQUESTS) ?? true, piSystemCacheSplit: boolEnvStrict("PI_SYSTEM_CACHE_SPLIT", env.PI_SYSTEM_CACHE_SPLIT) ?? false, diff --git a/src/deployment/secret-schema.ts b/src/deployment/secret-schema.ts index a8d5b1ccd..47e2e86fa 100644 --- a/src/deployment/secret-schema.ts +++ b/src/deployment/secret-schema.ts @@ -1,21 +1,23 @@ import { isStrongSigningSecret } from "../auth/source-auth.ts"; +type SecretGate = + | "production" + | "codex" + | "postgres" + | "sprites" + | "fly-sandbox" + | "fly-deploy" + | "aws-deploy-gate" + | "google-oauth" + | "dropbox-oauth" + | "linear-oauth" + | "model-anthropic" + | "model-openai" + | "model-openrouter"; + export interface RuntimeSecretSpec { name: string; - requiredWhen: - | "production" - | "codex" - | "postgres" - | "sprites" - | "fly-sandbox" - | "fly-deploy" - | "aws-deploy-gate" - | "google-oauth" - | "dropbox-oauth" - | "linear-oauth" - | "model-anthropic" - | "model-openai" - | "model-openrouter"; + requiredWhen: SecretGate | readonly SecretGate[]; } export const CORE_SECRET_SPECS: readonly RuntimeSecretSpec[] = [ @@ -24,12 +26,8 @@ export const CORE_SECRET_SPECS: readonly RuntimeSecretSpec[] = [ { name: "CORE_SIGNING_SECRET", requiredWhen: "production" }, { name: "PORTAL_IDENTITY_SECRET", requiredWhen: "production" }, { name: "SKILL_SIGNING_SECRET", requiredWhen: "production" }, - { name: "OPENAI_API_KEY", requiredWhen: "codex" }, - // MODEL_PROVIDER names the vendor supplying the base model. The deployment CLI sets it - // from `modelProvider` in qm.config.jsonc, so a stack that declares a provider refuses - // to boot without that provider's key rather than accepting turns it cannot serve. + { name: "OPENAI_API_KEY", requiredWhen: ["codex", "model-openai"] }, { name: "ANTHROPIC_API_KEY", requiredWhen: "model-anthropic" }, - { name: "OPENAI_API_KEY", requiredWhen: "model-openai" }, { name: "OPENROUTER_API_KEY", requiredWhen: "model-openrouter" }, { name: "DATABASE_URL", requiredWhen: "postgres" }, { name: "SPRITES_TOKEN", requiredWhen: "sprites" }, @@ -41,32 +39,30 @@ export const CORE_SECRET_SPECS: readonly RuntimeSecretSpec[] = [ { name: "LINEAR_OAUTH_CLIENT_SECRET", requiredWhen: "linear-oauth" }, ]; +const GATE_PREDICATES: Readonly boolean>> = { + production: (env) => env.NODE_ENV === "production", + codex: (env) => env.HARNESS?.trim() === "codex", + postgres: (env) => env.SESSION_STORE === "postgres" || env.RUN_STORE === "postgres", + sprites: (env) => env.SANDBOX_BACKEND === "sprites" || env.SANDBOX_SECONDARY_BACKEND === "sprites", + "fly-sandbox": (env) => env.SANDBOX_BACKEND === "fly", + "fly-deploy": (env) => env.DEPLOY_PROVIDER === "fly", + "aws-deploy-gate": (env) => Boolean(env.AWS_DEPLOY_APPS_DOMAIN), + "google-oauth": (env) => Boolean(env.GOOGLE_OAUTH_CLIENT_ID), + "dropbox-oauth": (env) => Boolean(env.DROPBOX_OAUTH_CLIENT_ID), + "linear-oauth": (env) => Boolean(env.LINEAR_OAUTH_CLIENT_ID), + "model-anthropic": (env) => env.MODEL_PROVIDER?.trim() === "anthropic", + "model-openai": (env) => env.MODEL_PROVIDER?.trim() === "openai", + "model-openrouter": (env) => env.MODEL_PROVIDER?.trim() === "openrouter", +}; + export function validateCoreSecretEnv(env: NodeJS.ProcessEnv): string[] { const enabled = (spec: RuntimeSecretSpec): boolean => { - if (spec.requiredWhen === "production") return env.NODE_ENV === "production"; - if (spec.requiredWhen === "codex") return env.HARNESS?.trim() === "codex"; - if (spec.requiredWhen === "postgres") return env.SESSION_STORE === "postgres" || env.RUN_STORE === "postgres"; - if (spec.requiredWhen === "sprites") - return env.SANDBOX_BACKEND === "sprites" || env.SANDBOX_SECONDARY_BACKEND === "sprites"; - if (spec.requiredWhen === "fly-sandbox") return env.SANDBOX_BACKEND === "fly"; - if (spec.requiredWhen === "fly-deploy") return env.DEPLOY_PROVIDER === "fly"; - if (spec.requiredWhen === "aws-deploy-gate") return Boolean(env.AWS_DEPLOY_APPS_DOMAIN); - if (spec.requiredWhen === "google-oauth") return Boolean(env.GOOGLE_OAUTH_CLIENT_ID); - if (spec.requiredWhen === "dropbox-oauth") return Boolean(env.DROPBOX_OAUTH_CLIENT_ID); - if (spec.requiredWhen === "model-anthropic") return env.MODEL_PROVIDER?.trim() === "anthropic"; - if (spec.requiredWhen === "model-openai") return env.MODEL_PROVIDER?.trim() === "openai"; - if (spec.requiredWhen === "model-openrouter") return env.MODEL_PROVIDER?.trim() === "openrouter"; - return Boolean(env.LINEAR_OAUTH_CLIENT_ID); + const gates = typeof spec.requiredWhen === "string" ? [spec.requiredWhen] : spec.requiredWhen; + return gates.some((gate) => GATE_PREDICATES[gate](env)); }; - // OPENAI_API_KEY carries two specs (the Codex harness and an OpenAI base model), so - // deduplicate before reporting — a name should be named once however many rules want it. - return [ - ...new Set( - CORE_SECRET_SPECS.filter((spec) => enabled(spec) && isInvalidSecret(spec.name, env[spec.name])).map( - (spec) => spec.name, - ), - ), - ]; + return CORE_SECRET_SPECS.filter((spec) => enabled(spec) && isInvalidSecret(spec.name, env[spec.name])).map( + (spec) => spec.name, + ); } function isInvalidSecret(name: string, value: string | undefined): boolean { diff --git a/src/index.ts b/src/index.ts index 8190d010c..fbd390c33 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,4 +1,4 @@ -import { configuredModelForHarness, loadConfig } from "./config.ts"; +import { baseModelProviders, configuredModelForHarness, loadConfig, providerKeysPresent } from "./config.ts"; import { buildApp, stopWithBackstop } from "./wiring.ts"; import { createServer } from "./api/server.ts"; import { errMessage } from "./util/errors.ts"; @@ -25,17 +25,13 @@ const server = createServer(built.app, { ...(config.requireSignedPortalIdentity ? { requireSignedPortalIdentity: true } : {}), ...(built.replayDedupe ? { replayDedupe: built.replayDedupe } : {}), config: built.config, - baseModelDefault: defaultModelForHarness(config.harness, configuredModelForHarness(config, config.harness)), - modelProviders: modelProviderAvailabilityFor(config.harness, { - anthropic: Boolean(config.anthropicApiKey), - openai: Boolean(config.openaiApiKey), - openrouter: Boolean(config.openrouterApiKey), - }), - providerKeys: { - anthropic: Boolean(config.anthropicApiKey), - openai: Boolean(config.openaiApiKey), - openrouter: Boolean(config.openrouterApiKey), - }, + baseModelDefault: defaultModelForHarness( + config.harness, + configuredModelForHarness(config, config.harness), + baseModelProviders(config), + ), + modelProviders: modelProviderAvailabilityFor(config.harness, providerKeysPresent(config)), + providerKeys: providerKeysPresent(config), modelCredentials: built.modelCredentials, ...(config.brandingDefault ? { brandingDefault: config.brandingDefault } : {}), harnessId: config.harness, diff --git a/src/model/model-credential-store.ts b/src/model/model-credential-store.ts index c7e30d5da..5ec2cfb50 100644 --- a/src/model/model-credential-store.ts +++ b/src/model/model-credential-store.ts @@ -1,9 +1,6 @@ import { decryptSecret, deriveConnectorKey, encryptSecret } from "../connectors/connector-client-store.ts"; import type { DurableMap } from "../persistence/durable-map.ts"; -import type { ModelProviderAvailability } from "./pi-models.ts"; - -const MODEL_PROVIDERS = ["anthropic", "openai", "openrouter"] as const; -export type ModelProvider = (typeof MODEL_PROVIDERS)[number]; +import { MODEL_PROVIDERS, type ModelProvider, type ModelProviderAvailability } from "./pi-models.ts"; export interface StoredModelCredential { provider: ModelProvider; @@ -29,10 +26,6 @@ export interface ModelCredentialStore { availability(): Promise; } -export function isModelProvider(value: unknown): value is ModelProvider { - return typeof value === "string" && (MODEL_PROVIDERS as readonly string[]).includes(value); -} - export function createModelCredentialStore(input: { backing: DurableMap; keyMaterial: string | Buffer; diff --git a/src/model/pi-models.ts b/src/model/pi-models.ts index ddc2314b0..a57462ef8 100644 --- a/src/model/pi-models.ts +++ b/src/model/pi-models.ts @@ -9,6 +9,13 @@ export const THINKING_LEVELS = ["auto", "low", "medium", "high", "xhigh", "max", export const HARNESS_IDS = ["pi", "opencode", "codex", "claude", "mock"] as const; export type HarnessId = (typeof HARNESS_IDS)[number]; +export const MODEL_PROVIDERS = ["anthropic", "openai", "openrouter"] as const; +export type ModelProvider = (typeof MODEL_PROVIDERS)[number]; + +export function isModelProvider(value: unknown): value is ModelProvider { + return typeof value === "string" && (MODEL_PROVIDERS as readonly string[]).includes(value); +} + export function isHarnessId(value: unknown): value is HarnessId { return typeof value === "string" && (HARNESS_IDS as readonly string[]).includes(value); } @@ -96,10 +103,8 @@ export const SELECTABLE_BASE_MODELS: ReadonlyArray<{ id: string; name: string }> (m) => m.base, ).map((m) => ({ id: m.id, name: m.name })); -const KNOWN_PROVIDERS = ["anthropic", "openai", "openrouter"] as const; - function builtinModel(id: string): PiModel | undefined { - for (const provider of KNOWN_PROVIDERS) { + for (const provider of MODEL_PROVIDERS) { const m = getModel(provider, id); if (m) return m; } @@ -170,9 +175,18 @@ export function modelSupportedByHarness(id: string | undefined, harness: string) return false; } -export function defaultModelForHarness(harness: string, configured?: string): string { +export function defaultModelForHarness( + harness: string, + configured?: string, + providers?: ModelProviderAvailability, +): string { if (configured && modelSupportedByHarness(configured, harness)) return configured; - return harness === "codex" ? DEFAULT_CODEX_MODEL_ID : DEFAULT_AGENT_MODEL_ID; + const preferred = harness === "codex" ? DEFAULT_CODEX_MODEL_ID : DEFAULT_AGENT_MODEL_ID; + if (!providers || modelServiceable(preferred, providers)) return preferred; + const servable = SELECTABLE_BASE_MODELS.find( + (model) => modelSupportedByHarness(model.id, harness) && modelServiceable(model.id, providers), + ); + return servable?.id ?? preferred; } export interface ModelProviderAvailability { @@ -207,6 +221,17 @@ export function modelProviderAvailabilityFor( return ALL_PROVIDERS_AVAILABLE; } +export function onlyProvider(provider: ModelProvider): ModelProviderAvailability { + return { anthropic: false, openai: false, openrouter: false, [provider]: true }; +} + +export function defaultModelForProvider(harness: string, provider: ModelProvider): string | undefined { + const only = onlyProvider(provider); + if (!modelProviderAvailabilityFor(harness, only)[provider]) return undefined; + const model = defaultModelForHarness(harness, undefined, only); + return modelSupportedByHarness(model, harness) && modelServiceable(model, only) ? model : undefined; +} + export function getRequiredModel(id: string): PiModel { const model = resolveModel(id); if (!model) throw new Error(`Unsupported model: ${id}`); diff --git a/src/wiring.ts b/src/wiring.ts index 9f5821d4b..f78e98bdd 100644 --- a/src/wiring.ts +++ b/src/wiring.ts @@ -1,7 +1,7 @@ import { mkdirSync } from "node:fs"; import { randomBytes, randomUUID } from "node:crypto"; import { join, resolve } from "node:path"; -import { configuredModelForHarness, type Config } from "./config.ts"; +import { baseModelProviders, configuredModelForHarness, providerKeysPresent, type Config } from "./config.ts"; import { createIdentityService, type DeactivationRecord, type IdentityService } from "./identity/identity-service.ts"; import { createMemoryConfigStore, @@ -713,7 +713,11 @@ export function buildApp( const fallbackHarness = config.harness as HarnessId; const fallback = { harnessId: fallbackHarness, - modelId: defaultModelForHarness(fallbackHarness, configuredModelForHarness(config, fallbackHarness)), + modelId: defaultModelForHarness( + fallbackHarness, + configuredModelForHarness(config, fallbackHarness), + baseModelProviders(config), + ), }; const judgeModelId = (): string => config.judgeModelId ?? auxiliaryModelFor(orgBaseModelId() ?? fallback.modelId); const harness = createHarnessRouter(adapters, adapters.get(fallbackHarness)!, (input) => @@ -1029,11 +1033,7 @@ export function buildApp( const ackEmojiPicks: AckEmojiPickStore = config.databaseUrl ? createPostgresAckEmojiPickStore(config.databaseUrl) : createMemoryAckEmojiPickStore(); - const providerKeys = { - anthropic: Boolean(config.anthropicApiKey), - openai: Boolean(config.openaiApiKey), - openrouter: Boolean(config.openrouterApiKey), - }; + const providerKeys = providerKeysPresent(config); const app = createApp({ identity, ...(config.publicWebUrl ? { publicWebUrl: config.publicWebUrl } : {}), diff --git a/test/base-model-serviceability.test.ts b/test/base-model-serviceability.test.ts index 729afc86d..4aebc308b 100644 --- a/test/base-model-serviceability.test.ts +++ b/test/base-model-serviceability.test.ts @@ -8,25 +8,36 @@ import { join } from "node:path"; import type { AddressInfo } from "node:net"; import { createInsecureTestServer } from "../src/api/server.ts"; import { buildApp } from "../src/wiring.ts"; +import { baseModelProviders, configuredModelForHarness, providerKeysPresent } from "../src/config.ts"; +import { defaultModelForHarness } from "../src/model/pi-models.ts"; import { testConfig } from "./support/test-config.ts"; const ADMIN = { "content-type": "application/json", "x-admin-actor": "admin-alice@default-org" }; -function start() { - const built = buildApp(testConfig({ dataDir: mkdtempSync(join(tmpdir(), "base-model-svc-")) })); +function start(overrides: Parameters[0] = { anthropicApiKey: "deployment-anthropic-key" }) { + const config = testConfig({ dataDir: mkdtempSync(join(tmpdir(), "base-model-svc-")), harness: "pi", ...overrides }); + const built = buildApp(config); const server = createInsecureTestServer(built.app, { config: built.config, admin: built.admin, auditLog: built.auditLog, acl: built.acl, + modelCredentials: built.modelCredentials, harnessId: "pi", - providerKeys: { anthropic: true, openai: false, openrouter: false }, + baseModelDefault: defaultModelForHarness("pi", configuredModelForHarness(config, "pi"), baseModelProviders(config)), + providerKeys: providerKeysPresent(config), }); server.listen(0); const base = `http://localhost:${(server.address() as AddressInfo).port}`; return { base, close: () => new Promise((r) => server.close(() => r())) }; } +async function effectiveModel(base: string): Promise { + const res = await fetch(`${base}/v1/runtime-config?principalId=alice&scopeId=personal%3Aalice`); + assert.equal(res.status, 200); + return ((await res.json()) as { effective: { modelId: string } }).effective.modelId; +} + test("base-model set rejects a model whose provider key is absent (would fail provider-side)", async () => { const srv = start(); try { @@ -48,3 +59,53 @@ test("base-model set rejects a model whose provider key is absent (would fail pr await srv.close(); } }); + +test("a deployment that declares a provider runs that provider's base model", async () => { + for (const [modelProvider, key, expected] of [ + ["anthropic", "anthropicApiKey", "claude-opus-5"], + ["openai", "openaiApiKey", "gpt-5.6-sol"], + ["openrouter", "openrouterApiKey", "openrouter/auto"], + ] as const) { + const srv = start({ modelProvider, [key]: `deployment-${modelProvider}-key` }); + try { + assert.equal( + await effectiveModel(srv.base), + expected, + `modelProvider "${modelProvider}" must land on a model that provider can bill`, + ); + } finally { + await srv.close(); + } + } +}); + +test("a single provider key with no declaration still runs a base model it can bill", async () => { + const srv = start({ openrouterApiKey: "deployment-openrouter-key" }); + try { + assert.equal(await effectiveModel(srv.base), "openrouter/auto"); + } finally { + await srv.close(); + } +}); + +test("the declaration outranks a stray key from another vendor", async () => { + const srv = start({ + modelProvider: "openrouter", + openrouterApiKey: "deployment-openrouter-key", + anthropicApiKey: "stray-anthropic-key", + }); + try { + assert.equal(await effectiveModel(srv.base), "openrouter/auto"); + } finally { + await srv.close(); + } +}); + +test("a deployment with no provider key at all keeps the shipped default", async () => { + const srv = start({}); + try { + assert.equal(await effectiveModel(srv.base), "claude-opus-5"); + } finally { + await srv.close(); + } +}); diff --git a/test/config.test.ts b/test/config.test.ts index 691dae4cd..1738190a6 100644 --- a/test/config.test.ts +++ b/test/config.test.ts @@ -1,7 +1,7 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import { resolve } from "node:path"; -import { boolEnv, loadConfig, numEnv, CONFIG_DEFAULTS } from "../src/config.ts"; +import { baseModelProviders, boolEnv, loadConfig, numEnv, CONFIG_DEFAULTS } from "../src/config.ts"; const productionEnv = { NODE_ENV: "production", @@ -351,3 +351,77 @@ test("maxClaims defaults from CONFIG_DEFAULTS and MAX_CLAIMS overrides", () => { assert.equal(loadConfig({ MAX_CLAIMS: "5" }).maxClaims, 5); assert.throws(() => loadConfig({ MAX_CLAIMS: "lots" }), /MAX_CLAIMS="lots" is not a number/); }); + +test("MODEL_PROVIDER declares the vendor that bills the base model", () => { + assert.equal(loadConfig({}).modelProvider, undefined); + assert.equal(loadConfig({ MODEL_PROVIDER: " openrouter ", OPENROUTER_API_KEY: "k" }).modelProvider, "openrouter"); + assert.throws(() => loadConfig({ MODEL_PROVIDER: "bedrock" }), /MODEL_PROVIDER.*not recognized/); +}); + +test("MODEL_PROVIDER is refused when the harness can never run that vendor's models", () => { + assert.throws( + () => loadConfig({ MODEL_PROVIDER: "openrouter", HARNESS: "codex", OPENROUTER_API_KEY: "k", OPENAI_API_KEY: "k" }), + /cannot serve a base model on HARNESS=codex/, + ); + assert.throws( + () => loadConfig({ MODEL_PROVIDER: "anthropic", HARNESS: "codex", ANTHROPIC_API_KEY: "k", OPENAI_API_KEY: "k" }), + /cannot serve a base model on HARNESS=codex/, + ); + assert.equal( + loadConfig({ MODEL_PROVIDER: "openai", HARNESS: "codex", OPENAI_API_KEY: "k" }).modelProvider, + "openai", + "the one combination Codex can bill is accepted", + ); +}); + +test("a pinned model from another vendor is refused rather than silently swapped", () => { + assert.throws( + () => loadConfig({ MODEL_PROVIDER: "openrouter", PI_MODEL: "claude-opus-5", OPENROUTER_API_KEY: "k" }), + /PI_MODEL=claude-opus-5 is a anthropic model but MODEL_PROVIDER=openrouter/, + ); + assert.throws( + () => + loadConfig({ + MODEL_PROVIDER: "openai", + HARNESS: "opencode", + OPENCODE_MODEL: "claude-sonnet-5", + OPENAI_API_KEY: "k", + }), + /OPENCODE_MODEL=claude-sonnet-5 is a anthropic model but MODEL_PROVIDER=openai/, + "the guard reads whichever model field the harness actually uses", + ); + assert.throws( + () => loadConfig({ MODEL_PROVIDER: "openai", HARNESS: "codex", CODEX_MODEL: "claude-opus-5", OPENAI_API_KEY: "k" }), + /CODEX_MODEL=claude-opus-5 is a anthropic model but MODEL_PROVIDER=openai/, + ); + assert.equal( + loadConfig({ MODEL_PROVIDER: "openrouter", PI_MODEL: "openrouter/auto", OPENROUTER_API_KEY: "k" }).modelId, + "openrouter/auto", + ); + assert.equal( + loadConfig({ MODEL_PROVIDER: "openrouter", PI_MODEL: "not-a-real-model", OPENROUTER_API_KEY: "k" }).modelId, + "not-a-real-model", + "an unresolvable pin is left to the harness rather than blocking boot", + ); +}); + +test("baseModelProviders follows the declaration, and the keys present when there is none", () => { + assert.deepEqual( + baseModelProviders(loadConfig({ MODEL_PROVIDER: "openrouter", OPENROUTER_API_KEY: "k", ANTHROPIC_API_KEY: "k" })), + { + anthropic: false, + openai: false, + openrouter: true, + }, + ); + assert.deepEqual(baseModelProviders(loadConfig({ ANTHROPIC_API_KEY: "k" })), { + anthropic: true, + openai: false, + openrouter: false, + }); + assert.deepEqual( + baseModelProviders(loadConfig({ HARNESS: "opencode", OPENROUTER_API_KEY: "k" })), + { anthropic: false, openai: false, openrouter: false }, + "opencode has no OpenRouter route, so an OpenRouter key does not choose its base model", + ); +}); diff --git a/test/pi-models.test.ts b/test/pi-models.test.ts index 8beaf5001..5b03e44bc 100644 --- a/test/pi-models.test.ts +++ b/test/pi-models.test.ts @@ -4,10 +4,13 @@ import { auxiliaryModelFor, auxiliaryModelForProvider, defaultModelForHarness, + defaultModelForProvider, modelServiceable, modelSupportedByHarness, + onlyProvider, resolveModel, getRequiredModel, + MODEL_PROVIDERS, SELECTABLE_BASE_MODELS, contextTokenBudgetForModel, } from "../src/model/pi-models.ts"; @@ -45,6 +48,44 @@ test("native harnesses reject cross-provider pins and choose their own defaults" assert.equal(defaultModelForHarness("codex", "claude-opus-4-8"), "gpt-5.6-sol"); }); +test("the default base model follows the providers a deployment can actually bill", () => { + for (const provider of MODEL_PROVIDERS) { + const only = onlyProvider(provider); + const chosen = defaultModelForHarness("pi", undefined, only); + assert.equal( + modelServiceable(chosen, only), + true, + `a ${provider}-only deployment must default to a model ${provider} can serve, got ${chosen}`, + ); + } + assert.equal(defaultModelForHarness("pi", undefined, onlyProvider("openrouter")), "openrouter/auto"); + assert.equal(defaultModelForHarness("pi", undefined, onlyProvider("openai")), "gpt-5.6-sol"); +}); + +test("provider-blind callers and explicit pins keep the shipped default", () => { + assert.equal(defaultModelForHarness("pi"), "claude-opus-5"); + assert.equal(defaultModelForHarness("pi", undefined, onlyProvider("anthropic")), "claude-opus-5"); + assert.equal( + defaultModelForHarness("pi", "claude-sonnet-5", onlyProvider("openrouter")), + "claude-sonnet-5", + "an explicit pin is never silently swapped — the mismatch is rejected at config load instead", + ); + assert.equal( + defaultModelForHarness("pi", undefined, { anthropic: false, openai: false, openrouter: false }), + "claude-opus-5", + "with no provider at all the shipped default stands rather than an arbitrary pick", + ); +}); + +test("a provider that cannot serve a harness has no default model for it", () => { + assert.equal(defaultModelForProvider("pi", "openrouter"), "openrouter/auto"); + assert.equal(defaultModelForProvider("codex", "openai"), "gpt-5.6-sol"); + assert.equal(defaultModelForProvider("claude", "anthropic"), "claude-opus-5"); + assert.equal(defaultModelForProvider("codex", "anthropic"), undefined, "the Codex CLI runs no Anthropic model"); + assert.equal(defaultModelForProvider("claude", "openrouter"), undefined, "the Claude CLI runs no OpenRouter model"); + assert.equal(defaultModelForProvider("opencode", "openrouter"), undefined, "opencode has no OpenRouter route"); +}); + test("the curated catalog contains only current model families", () => { assert.deepEqual( SELECTABLE_BASE_MODELS.map((model) => model.id), diff --git a/test/secret-schema-drift.test.ts b/test/secret-schema-drift.test.ts index 272709474..209eaf1ac 100644 --- a/test/secret-schema-drift.test.ts +++ b/test/secret-schema-drift.test.ts @@ -2,7 +2,9 @@ import test from "node:test"; import assert from "node:assert/strict"; import { readFileSync } from "node:fs"; import { FIRST_PARTY_SECRET_SPECS } from "../cli/src/secrets.ts"; +import { MODEL_PROVIDERS, MODEL_PROVIDER_BASE_MODELS, MODEL_PROVIDER_HARNESSES } from "../cli/src/config.ts"; import { CORE_SECRET_SPECS, validateCoreSecretEnv } from "../src/deployment/secret-schema.ts"; +import { HARNESS_IDS, defaultModelForProvider } from "../src/model/pi-models.ts"; test("the standalone CLI and core agree on runtime-enforced core secret names", () => { const cli = new Set( @@ -62,6 +64,37 @@ test("an OpenAI base model on the Codex harness reports its one missing key once ); }); +test("each core secret is named by exactly one spec, so boot failures never repeat a name", () => { + const names = CORE_SECRET_SPECS.map((spec) => spec.name); + assert.deepEqual( + names.filter((name, i) => names.indexOf(name) !== i), + [], + "give a secret answering to several rules one spec with a requiredWhen list", + ); +}); + +test("the CLI's provider/harness table matches what the core model registry can actually serve", () => { + for (const provider of MODEL_PROVIDERS) { + assert.deepEqual( + HARNESS_IDS.filter((harness) => defaultModelForProvider(harness, provider) !== undefined).sort(), + [...MODEL_PROVIDER_HARNESSES[provider]].sort(), + `MODEL_PROVIDER_HARNESSES.${provider} has drifted from the registry`, + ); + } +}); + +test("the CLI names the same base model the runtime will pick for each provider", () => { + for (const provider of MODEL_PROVIDERS) { + for (const harness of MODEL_PROVIDER_HARNESSES[provider]) { + assert.equal( + defaultModelForProvider(harness, provider), + MODEL_PROVIDER_BASE_MODELS[provider], + `MODEL_PROVIDER_BASE_MODELS.${provider} has drifted for HARNESS=${harness}`, + ); + } + } +}); + test("production rejects weak encryption key material for managed credentials", () => { const strong = "x".repeat(32); const env = { From 37e28a1915f4c5783422214cccc5d998c74def3c Mon Sep 17 00:00:00 2001 From: Josh France <12610835+16francej@users.noreply.github.com> Date: Wed, 29 Jul 2026 20:49:04 -0700 Subject: [PATCH 2/2] Narrow the provider rule to the declaration, per adversarial review Codex challenged the diff and found two ways it could hurt a live stack. Inferring the base-model provider from whichever API keys happen to be in the environment silently moved an existing deployment, and its billing: a stack holding OPENROUTER_API_KEY as the documented optional fallback while actually running Anthropic from the Admin page would flip to openrouter/auto on upgrade. The inference is gone. Without MODEL_PROVIDER the shipped default stands exactly as before, so no deployment moves unless it says to. Refusing a cross-vendor model pin at boot could brick a rollout the CLI had already approved, because the CLI holds no model registry and could not mirror the check. That throw is gone too; a pin wins, as it always has. What remains is the harness/provider rule, which the CLI does mirror and a drift test pins to the registry. The CLI now reads the provider core will actually use, so an env.core.MODEL_PROVIDER override is validated rather than skipped. Co-Authored-By: Claude Opus 5 --- cli/src/config.ts | 10 ++++- cli/test/config.test.ts | 20 ++++++++- src/config.ts | 26 +----------- test/base-model-serviceability.test.ts | 27 ++++++------ test/config.test.ts | 58 ++++++-------------------- 5 files changed, 53 insertions(+), 88 deletions(-) diff --git a/cli/src/config.ts b/cli/src/config.ts index 120de00f5..9c0be6671 100644 --- a/cli/src/config.ts +++ b/cli/src/config.ts @@ -715,12 +715,18 @@ function configuredHarness(config: QmConfig): string { } function validateModelProvider(config: QmConfig, path: string): void { - const provider = config.modelProvider; + const override = config.env.core?.MODEL_PROVIDER?.trim(); + if (override !== undefined && override !== "" && !isModelProvider(override)) { + throw new CliError( + `${path}: env.core.MODEL_PROVIDER must be one of ${MODEL_PROVIDERS.join(", ")}, or unset to use "modelProvider"`, + ); + } + const provider = isModelProvider(override) ? override : config.modelProvider; if (!provider) return; const harness = configuredHarness(config); if (!MODEL_PROVIDER_HARNESSES[provider].includes(harness)) { throw new CliError( - `${path}: modelProvider "${provider}" cannot serve a base model on env.core.HARNESS "${harness}" — that harness runs no ${provider} model, so every agent turn would be refused. Use ${MODEL_PROVIDER_HARNESSES[provider].join(", ")}, or pick a provider that harness can bill.`, + `${path}: model provider "${provider}" cannot serve a base model on env.core.HARNESS "${harness}" — that harness runs no ${provider} model, so every agent turn would be refused. Use ${MODEL_PROVIDER_HARNESSES[provider].join(", ")}, or pick a provider that harness can bill.`, ); } } diff --git a/cli/test/config.test.ts b/cli/test/config.test.ts index e8cca5699..bd30c0441 100644 --- a/cli/test/config.test.ts +++ b/cli/test/config.test.ts @@ -1143,7 +1143,7 @@ test("modelProvider must name a vendor the configured harness can bill", () => { withConfig({ modelProvider: "openrouter", env: { core: { HARNESS: "codex" } } }, ({ path }) => { assert.throws( () => loadConfigAt(path), - /modelProvider "openrouter" cannot serve a base model on env.core.HARNESS "codex"/, + /model provider "openrouter" cannot serve a base model on env.core.HARNESS "codex"/, ); }); withConfig({ modelProvider: "anthropic", env: { core: { HARNESS: "codex" } } }, ({ path }) => { @@ -1160,3 +1160,21 @@ test("modelProvider must name a vendor the configured harness can bill", () => { ); }); }); + +test("env.core.MODEL_PROVIDER is validated as the provider core will actually use", () => { + withConfig( + { modelProvider: "openai", env: { core: { HARNESS: "codex", MODEL_PROVIDER: "anthropic" } } }, + ({ path }) => { + assert.throws(() => loadConfigAt(path), /model provider "anthropic" cannot serve a base model/); + }, + ); + withConfig( + { modelProvider: "anthropic", env: { core: { HARNESS: "codex", MODEL_PROVIDER: "openai" } } }, + ({ path }) => { + assert.equal(loadConfigAt(path).config.modelProvider, "anthropic", "the override decides, the declaration stays"); + }, + ); + withConfig({ env: { core: { HARNESS: "pi", MODEL_PROVIDER: "bedrock" } } }, ({ path }) => { + assert.throws(() => loadConfigAt(path), /env.core.MODEL_PROVIDER must be one of/); + }); +}); diff --git a/src/config.ts b/src/config.ts index bc13f7ce9..c274c4e7e 100644 --- a/src/config.ts +++ b/src/config.ts @@ -15,9 +15,7 @@ import { MODEL_PROVIDERS, defaultModelForProvider, isModelProvider, - modelProviderAvailabilityFor, onlyProvider, - resolveModel, type ModelProvider, type ModelProviderAvailability, } from "./model/pi-models.ts"; @@ -160,14 +158,8 @@ export function providerKeysPresent(config: Config): ModelProviderAvailability { }; } -export function baseModelProviders(config: Config): ModelProviderAvailability { - const declared = config.modelProvider ? onlyProvider(config.modelProvider) : providerKeysPresent(config); - const routable = modelProviderAvailabilityFor(config.harness, declared); - return { - anthropic: declared.anthropic && routable.anthropic, - openai: declared.openai && routable.openai, - openrouter: declared.openrouter && routable.openrouter, - }; +export function baseModelProviders(config: Config): ModelProviderAvailability | undefined { + return config.modelProvider ? onlyProvider(config.modelProvider) : undefined; } interface AwsSandboxEnv { @@ -555,23 +547,9 @@ function modelProviderEnvStrict(env: NodeJS.ProcessEnv): ModelProvider | undefin `MODEL_PROVIDER=${declared} cannot serve a base model on HARNESS=${harness} — that harness runs no ${declared} model, so every turn would be refused.`, ); } - const [pinName, pinned] = harnessModelPin(env, harness); - const pinnedProvider = pinned ? resolveModel(pinned)?.provider : undefined; - if (pinned && pinnedProvider && pinnedProvider !== declared) { - throw new Error( - `${pinName}=${pinned} is a ${pinnedProvider} model but MODEL_PROVIDER=${declared} — the deployment holds no key that can bill it.`, - ); - } return declared; } -function harnessModelPin(env: NodeJS.ProcessEnv, harness: Config["harness"]): [string, string | undefined] { - if (harness === "codex") return ["CODEX_MODEL", env.CODEX_MODEL?.trim()]; - if (harness === "claude") return ["CLAUDE_MODEL", env.CLAUDE_MODEL?.trim()]; - if (harness === "opencode" && env.OPENCODE_MODEL?.trim()) return ["OPENCODE_MODEL", env.OPENCODE_MODEL.trim()]; - return ["PI_MODEL", env.PI_MODEL?.trim()]; -} - export function loadConfig(env: NodeJS.ProcessEnv = process.env): Config { const missingSecrets = validateCoreSecretEnv(env); if (missingSecrets.length) { diff --git a/test/base-model-serviceability.test.ts b/test/base-model-serviceability.test.ts index 4aebc308b..8b7c766e6 100644 --- a/test/base-model-serviceability.test.ts +++ b/test/base-model-serviceability.test.ts @@ -79,12 +79,18 @@ test("a deployment that declares a provider runs that provider's base model", as } }); -test("a single provider key with no declaration still runs a base model it can bill", async () => { - const srv = start({ openrouterApiKey: "deployment-openrouter-key" }); - try { - assert.equal(await effectiveModel(srv.base), "openrouter/auto"); - } finally { - await srv.close(); +test("an undeclared deployment keeps the shipped default, whatever keys it holds", async () => { + for (const overrides of [{}, { openrouterApiKey: "k" }, { openaiApiKey: "k" }] as const) { + const srv = start(overrides); + try { + assert.equal( + await effectiveModel(srv.base), + "claude-opus-5", + "upgrading must not move an existing deployment's model or its billing", + ); + } finally { + await srv.close(); + } } }); @@ -100,12 +106,3 @@ test("the declaration outranks a stray key from another vendor", async () => { await srv.close(); } }); - -test("a deployment with no provider key at all keeps the shipped default", async () => { - const srv = start({}); - try { - assert.equal(await effectiveModel(srv.base), "claude-opus-5"); - } finally { - await srv.close(); - } -}); diff --git a/test/config.test.ts b/test/config.test.ts index 1738190a6..7d89b29ba 100644 --- a/test/config.test.ts +++ b/test/config.test.ts @@ -367,6 +367,11 @@ test("MODEL_PROVIDER is refused when the harness can never run that vendor's mod () => loadConfig({ MODEL_PROVIDER: "anthropic", HARNESS: "codex", ANTHROPIC_API_KEY: "k", OPENAI_API_KEY: "k" }), /cannot serve a base model on HARNESS=codex/, ); + assert.throws( + () => loadConfig({ MODEL_PROVIDER: "openrouter", HARNESS: "opencode", OPENROUTER_API_KEY: "k" }), + /cannot serve a base model on HARNESS=opencode/, + "opencode has no OpenRouter route", + ); assert.equal( loadConfig({ MODEL_PROVIDER: "openai", HARNESS: "codex", OPENAI_API_KEY: "k" }).modelProvider, "openai", @@ -374,54 +379,15 @@ test("MODEL_PROVIDER is refused when the harness can never run that vendor's mod ); }); -test("a pinned model from another vendor is refused rather than silently swapped", () => { - assert.throws( - () => loadConfig({ MODEL_PROVIDER: "openrouter", PI_MODEL: "claude-opus-5", OPENROUTER_API_KEY: "k" }), - /PI_MODEL=claude-opus-5 is a anthropic model but MODEL_PROVIDER=openrouter/, - ); - assert.throws( - () => - loadConfig({ - MODEL_PROVIDER: "openai", - HARNESS: "opencode", - OPENCODE_MODEL: "claude-sonnet-5", - OPENAI_API_KEY: "k", - }), - /OPENCODE_MODEL=claude-sonnet-5 is a anthropic model but MODEL_PROVIDER=openai/, - "the guard reads whichever model field the harness actually uses", - ); - assert.throws( - () => loadConfig({ MODEL_PROVIDER: "openai", HARNESS: "codex", CODEX_MODEL: "claude-opus-5", OPENAI_API_KEY: "k" }), - /CODEX_MODEL=claude-opus-5 is a anthropic model but MODEL_PROVIDER=openai/, - ); - assert.equal( - loadConfig({ MODEL_PROVIDER: "openrouter", PI_MODEL: "openrouter/auto", OPENROUTER_API_KEY: "k" }).modelId, - "openrouter/auto", - ); - assert.equal( - loadConfig({ MODEL_PROVIDER: "openrouter", PI_MODEL: "not-a-real-model", OPENROUTER_API_KEY: "k" }).modelId, - "not-a-real-model", - "an unresolvable pin is left to the harness rather than blocking boot", - ); -}); - -test("baseModelProviders follows the declaration, and the keys present when there is none", () => { +test("baseModelProviders constrains the base model only when a provider is declared", () => { assert.deepEqual( baseModelProviders(loadConfig({ MODEL_PROVIDER: "openrouter", OPENROUTER_API_KEY: "k", ANTHROPIC_API_KEY: "k" })), - { - anthropic: false, - openai: false, - openrouter: true, - }, + { anthropic: false, openai: false, openrouter: true }, + "the declaration outranks a stray key from another vendor", ); - assert.deepEqual(baseModelProviders(loadConfig({ ANTHROPIC_API_KEY: "k" })), { - anthropic: true, - openai: false, - openrouter: false, - }); - assert.deepEqual( - baseModelProviders(loadConfig({ HARNESS: "opencode", OPENROUTER_API_KEY: "k" })), - { anthropic: false, openai: false, openrouter: false }, - "opencode has no OpenRouter route, so an OpenRouter key does not choose its base model", + assert.equal( + baseModelProviders(loadConfig({ OPENROUTER_API_KEY: "k" })), + undefined, + "with no declaration the shipped default stands, so upgrading never moves a deployment's model or its billing", ); });