diff --git a/docs-site/src/content/docs/reference/configuration/server.md b/docs-site/src/content/docs/reference/configuration/server.md index 34eb5a9375..e36975cd62 100644 --- a/docs-site/src/content/docs/reference/configuration/server.md +++ b/docs-site/src/content/docs/reference/configuration/server.md @@ -160,6 +160,8 @@ These settings govern `/v1/messages`, `/v1/messages/count_tokens`, the `ocx clau | `claudeCode.bodyMaxBytes?` | `number` | `67108864` | Cumulative native-passthrough body cap for streamed and buffered responses. Exactly `0` disables. | | `claudeCode.authMode?` | `"proxy" \| "subscription"` | auto | How launch handles `ANTHROPIC_AUTH_TOKEN`. Auto detects auth each launch; an explicit value is never overridden. | | `claudeCode.authModeMigratedAt?` | `string` | unset | Internal one-time upgrade marker. Do not set manually. | +| `claudeCode.classifierModel?` | `string` | unset | Explicit target for Claude Code Auto Mode classifier turns, as a qualified `provider/model` (for example `RelayA/claude-opus-5`). Auto Mode sends bare safety checks such as `claude-opus-5` with no provider, so without this they fall through to `defaultProvider` — which may not speak Anthropic at all. Nothing is inferred automatically: only a target you declare here is used. | +| `claudeCode.classifierFallbacks?` | `string[]` | unset | Ordered classifier targets used when `classifierModel` is not set. Same qualified `provider/model` form; the first usable entry wins. An explicit `modelMap` entry for the classifier model still outranks both. | | `claudeCode.subagentEffort?` | `"low" \| "medium" \| "high" \| "xhigh" \| "max"` | inherit | Effort written to generated `~/.claude/agents/ocx-*.md`; separate from Codex guidance and proxy caps. Restart through `ocx claude` to regenerate. | Auto auth selects subscription when stored Claude auth is found, proxy when none is found, and diff --git a/src/claude/inbound.ts b/src/claude/inbound.ts index 4b72da674d..90c4652cb1 100644 --- a/src/claude/inbound.ts +++ b/src/claude/inbound.ts @@ -25,7 +25,37 @@ function isRec(v: unknown): v is Rec { return !!v && typeof v === "object" && !Array.isArray(v); } -/** Alias first, then modelMap: exact id, then date-suffix-stripped (`-\d{8}$`), else passthrough. */ +function isClaudeClassifierModel(model: string): boolean { + const stripped = model.replace(/-\d{8}$/, ""); + return /^claude-opus-[45]/.test(stripped); +} + +/** + * Explicitly configured classifier route for Claude Code Auto Mode safety checks (#1697). + * + * Only OPERATOR-DECLARED targets are used: `classifierModel`, then the ordered + * `classifierFallbacks`. Both are qualified `provider/model` strings the operator chose, so + * routing them crosses no boundary the operator did not ask for. + * + * Deliberately NOT here: inferring a provider from `claudeCode.model`. That value is the + * injected/default config slot, not the provider the live session actually selected, so it goes + * stale the moment the user changes the model picker -- and acting on it would silently move a + * classifier turn onto a provider with its own privacy and billing consequences. Live session + * affinity needs the request/session state this function does not have; it is tracked as + * follow-up work rather than approximated from static config. + */ +function configuredClassifierRoute(cc?: OcxClaudeCodeConfig): string | undefined { + const explicit = typeof cc?.classifierModel === "string" ? cc.classifierModel.trim() : ""; + if (explicit.length > 0) return explicit; + if (Array.isArray(cc?.classifierFallbacks)) { + for (const candidate of cc.classifierFallbacks) { + if (typeof candidate === "string" && candidate.trim().length > 0) return candidate.trim(); + } + } + return undefined; +} + +/** Alias first, then modelMap: exact id, then date-suffix-stripped (`-\d{8}$`), then classifier affinity/config, else passthrough. */ export function resolveInboundModel(model: string, cc?: OcxClaudeCodeConfig): string { // Defensive: Desktop/CLI strip the [1m] context-variant marker client-side, but a // leaking build must not break alias decode (devlog 138 — the 1M signal is the @@ -47,6 +77,14 @@ export function resolveInboundModel(model: string, cc?: OcxClaudeCodeConfig): st const stripped = model.replace(/-\d{8}$/, ""); const dateless = map[stripped]; if (typeof dateless === "string" && dateless.length > 0) return dateless; + + // Claude Code Auto Mode classifier routing (#1697). Bare classifier checks such as + // `claude-opus-5` carry no provider, so without this they fall through to defaultProvider -- + // which may not speak Anthropic at all. Only an operator-declared target is used. + if (isClaudeClassifierModel(model)) { + const configured = configuredClassifierRoute(cc); + if (configured) return configured; + } return model; } diff --git a/src/config.ts b/src/config.ts index c0ca982a62..d4c0a3a0f3 100644 --- a/src/config.ts +++ b/src/config.ts @@ -2018,12 +2018,30 @@ function normalizePersistedClaudeCode(claudeCode: unknown): OcxConfig["claudeCod if (Object.hasOwn(normalized, "subagentEffort") && !isClaudeSubagentEffort(normalized.subagentEffort)) { delete normalized.subagentEffort; } + // A hand-authored config never passes through the management validator, so coerce here too. + // A malformed classifierFallbacks (a bare string, or an array with non-string entries) would + // otherwise reach the resolver unchecked. + if (Object.hasOwn(normalized, "classifierModel")) { + const value = typeof normalized.classifierModel === "string" ? normalized.classifierModel.trim() : ""; + if (value.length > 0) normalized.classifierModel = value; + else delete normalized.classifierModel; + } + if (Object.hasOwn(normalized, "classifierFallbacks")) { + const raw = normalized.classifierFallbacks; + const kept = Array.isArray(raw) + ? raw.filter((entry): entry is string => typeof entry === "string" && entry.trim().length > 0).map(entry => entry.trim()) + : []; + if (kept.length > 0) normalized.classifierFallbacks = kept; + else delete normalized.classifierFallbacks; + } return normalized as OcxConfig["claudeCode"]; } -function normalizeClaudeSubagentEffort(config: OcxConfig, rawParsed: unknown): OcxConfig { - const rawEffort = rawClaudeSubagentEffort(rawParsed); - if (rawEffort === undefined || isClaudeSubagentEffort(rawEffort)) return config; +function normalizeClaudeSubagentEffort(config: OcxConfig, _rawParsed: unknown): OcxConfig { + // Unconditional. This used to short-circuit when `subagentEffort` was absent or already valid, + // which meant a config whose ONLY defect was elsewhere in `claudeCode` was never normalized. + // The specialized subagentEffort WARNING is a separate concern and stays exactly as it is. + if (!config.claudeCode) return config; return { ...config, claudeCode: normalizePersistedClaudeCode(config.claudeCode) }; } diff --git a/src/router.ts b/src/router.ts index 791b8c77a2..6dcb00fba3 100644 --- a/src/router.ts +++ b/src/router.ts @@ -715,12 +715,17 @@ function routeByKnownModelPattern(config: OcxConfig, modelId: string): RouteResu for (const { providerNames, prefixes } of MODEL_PROVIDER_PATTERNS) { if (prefixes.some(prefix => modelId.startsWith(prefix))) { const matchingProvider = Object.entries(config.providers).find( - ([name]) => providerNames.some(providerName => name === providerName || name.startsWith(`${providerName}-`)) + ([name, prov]) => prov.disabled !== true && providerNames.some(providerName => name === providerName || name.startsWith(`${providerName}-`)) ); if (matchingProvider) { const [provName, prov] = matchingProvider; return routeResult(provName, prov, modelId, "explicit-provider", "model-pattern"); } + // Deliberately no "first provider with an Anthropic adapter" fallback here. Picking by + // object insertion order, without checking `models`, `selectedModels`, `disabledModels` or + // discovery state, silently moves a request onto a provider the operator never chose, with + // its own privacy and billing consequences (#1697). A classifier turn that needs a specific + // target gets it from operator-declared `claudeCode.classifierModel` / `classifierFallbacks`. } } return undefined; diff --git a/src/server/management/agent-settings-routes.ts b/src/server/management/agent-settings-routes.ts index dbd8088c28..fb3ef47ee4 100644 --- a/src/server/management/agent-settings-routes.ts +++ b/src/server/management/agent-settings-routes.ts @@ -1005,6 +1005,8 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise smallFastModel: config.claudeCode?.smallFastModel ?? "", tierModels: config.claudeCode?.tierModels ?? {}, modelMap: config.claudeCode?.modelMap ?? {}, + classifierModel: config.claudeCode?.classifierModel ?? "", + classifierFallbacks: config.claudeCode?.classifierFallbacks ?? [], systemEnv: config.claudeCode?.systemEnv === true, autoConnectSupported: process.platform === "darwin", maxContextTokens: config.claudeCode?.maxContextTokens ?? null, @@ -1042,7 +1044,7 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise return prototype === Object.prototype || prototype === null; }; if (!isPlainObject(parsedBody)) return jsonResponse({ error: "body must be an object" }, 400); - const body = parsedBody as { enabled?: unknown; authMode?: unknown; model?: unknown; smallFastModel?: unknown; modelMap?: unknown; systemEnv?: unknown; fastMode?: unknown; maxContextTokens?: unknown; alwaysEnableEffort?: unknown; tierModels?: unknown; autoContext?: unknown; autoCompactWindow?: unknown; blockedSkills?: unknown; injectAgents?: unknown; webSearchSidecar?: unknown; visionSidecar?: unknown }; + const body = parsedBody as { enabled?: unknown; authMode?: unknown; model?: unknown; smallFastModel?: unknown; modelMap?: unknown; classifierModel?: unknown; classifierFallbacks?: unknown; systemEnv?: unknown; fastMode?: unknown; maxContextTokens?: unknown; alwaysEnableEffort?: unknown; tierModels?: unknown; autoContext?: unknown; autoCompactWindow?: unknown; blockedSkills?: unknown; injectAgents?: unknown; webSearchSidecar?: unknown; visionSidecar?: unknown }; for (const field of ["webSearchSidecar", "visionSidecar"] as const) { const section = body[field]; if (section === undefined || section === null) continue; @@ -1182,13 +1184,31 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise } nextFastMode = body.fastMode === null ? undefined : body.fastMode; } - for (const field of ["model", "smallFastModel"] as const) { + for (const field of ["model", "smallFastModel", "classifierModel"] as const) { const value = body[field]; if (value === undefined) continue; if (typeof value !== "string") return jsonResponse({ error: `${field} must be a string` }, 400); if (value.trim() === "") delete next[field]; else next[field] = value.trim(); } + if (body.classifierFallbacks !== undefined) { + if (body.classifierFallbacks === null) { + delete next.classifierFallbacks; + } else { + if (!Array.isArray(body.classifierFallbacks)) { + return jsonResponse({ error: "classifierFallbacks must be an array of strings, or null" }, 400); + } + const list: string[] = []; + for (const entry of body.classifierFallbacks) { + if (typeof entry !== "string" || entry.trim() === "") { + return jsonResponse({ error: "classifierFallbacks entries must be non-empty strings" }, 400); + } + list.push(entry.trim()); + } + if (list.length > 0) next.classifierFallbacks = list; + else delete next.classifierFallbacks; + } + } if (body.modelMap !== undefined) { if (body.modelMap === null) { delete next.modelMap; diff --git a/src/types.ts b/src/types.ts index 7f409d3333..565118b287 100644 --- a/src/types.ts +++ b/src/types.ts @@ -457,6 +457,17 @@ export interface OcxClaudeCodeConfig { smallFastModel?: string; /** Inbound model id remaps: exact id first, then date-stripped (`-\d{8}$`). */ modelMap?: Record; + /** + * Explicit classifier model for Claude Code Auto Mode safety checks (e.g. "RelayA/claude-opus-5"). + * When unset, bare classifier requests check modelMap, then same-provider affinity from + * `claudeCode.model`, then compatible Anthropic-adapter providers, and finally fallbacks. + */ + classifierModel?: string; + /** + * Ordered fallback candidates for Claude Code Auto Mode classifier routing when the primary + * classifier route is not available. + */ + classifierFallbacks?: string[]; /** * Inject ANTHROPIC_BASE_URL etc. into the macOS user domain via `launchctl setenv` * so plain `claude` commands route through the proxy without `ocx claude`. Reverted diff --git a/tests/claude-inbound.test.ts b/tests/claude-inbound.test.ts index 74d120aa18..f56275e26d 100644 --- a/tests/claude-inbound.test.ts +++ b/tests/claude-inbound.test.ts @@ -285,6 +285,44 @@ describe("claude inbound translation", () => { expect(resolveInboundModel("anything", undefined)).toBe("anything"); }); + test("Claude Code Auto Mode classifier routing uses only operator-declared targets (#1697)", () => { + // A bare classifier check carries no provider, so without this it falls through to + // defaultProvider -- which may not speak Anthropic at all. What it must NOT do is pick a + // provider nobody chose. + + // 1. Explicit classifierModel is used. + const ccExplicit = { model: "RelayA/claude-fable-5", classifierModel: "RelayB/claude-opus-5" }; + expect(resolveInboundModel("claude-opus-5", ccExplicit)).toBe("RelayB/claude-opus-5"); + expect(resolveInboundModel("claude-opus-5-20250514", ccExplicit)).toBe("RelayB/claude-opus-5"); + + // 2. modelMap outranks it: an explicit per-model mapping is the operator's most specific say. + const ccWithModelMap = { + model: "RelayA/claude-fable-5", + classifierModel: "RelayB/claude-opus-5", + modelMap: { "claude-opus-5": "Custom/my-opus-5" }, + }; + expect(resolveInboundModel("claude-opus-5", ccWithModelMap)).toBe("Custom/my-opus-5"); + + // 3. Ordered fallbacks are used when no classifierModel is set. + const ccWithFallbacks = { classifierFallbacks: ["RelayC/claude-opus-5", "RelayD/claude-opus-5"] }; + expect(resolveInboundModel("claude-opus-5", ccWithFallbacks)).toBe("RelayC/claude-opus-5"); + + // 4. NO affinity inferred from cc.model. That value is the injected/default config slot, not + // the provider the live session actually selected, so it goes stale the moment the user + // changes the model picker -- and acting on it would silently move a classifier turn onto a + // provider with its own privacy and billing consequences. + expect(resolveInboundModel("claude-opus-5", { model: "RelayA/claude-fable-5" })).toBe("claude-opus-5"); + expect(resolveInboundModel("claude-opus-5", { model: "claude-ocx-RelayA--claude-fable-5" })).toBe("claude-opus-5"); + expect(resolveInboundModel("claude-opus-5", { model: "native/claude-opus-5" })).toBe("claude-opus-5"); + + // 5. Malformed operator config is ignored rather than half-applied. + expect(resolveInboundModel("claude-opus-5", { classifierModel: " " })).toBe("claude-opus-5"); + expect(resolveInboundModel("claude-opus-5", { classifierFallbacks: [] })).toBe("claude-opus-5"); + + // 6. A non-classifier model is untouched by any of this. + expect(resolveInboundModel("claude-fable-5", ccExplicit)).toBe("claude-fable-5"); + }); + test("error cases: no model, empty messages, bad role, bad tool_result", () => { expect(() => anthropicToResponsesBody({ max_tokens: 1, messages: [{ role: "user", content: "x" }] })).toThrow(AnthropicRequestError); expect(() => anthropicToResponsesBody({ model: "m", max_tokens: 1, messages: [] })).toThrow(AnthropicRequestError); diff --git a/tests/claude-management-api.test.ts b/tests/claude-management-api.test.ts index 936028ef91..444bd13b02 100644 --- a/tests/claude-management-api.test.ts +++ b/tests/claude-management-api.test.ts @@ -78,6 +78,57 @@ test("GET /api/claude-code returns defaults + available + aliases", async () => } }); + +test("PUT round-trips classifier routing settings and clears them with null (#1697)", async () => { + const server = startServer(0); + try { + const put = await fetch(new URL("/api/claude-code", server.url), { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + classifierModel: " mock/test-model ", + classifierFallbacks: [" mock/test-model ", "mock/other"], + }), + }); + expect(put.status).toBe(200); + + const get = await fetch(new URL("/api/claude-code", server.url)); + const d = await get.json() as Record; + expect(d.classifierModel).toBe("mock/test-model"); + expect(d.classifierFallbacks).toEqual(["mock/test-model", "mock/other"]); + + // null clears both, which is how the operator turns classifier routing back off. + const cleared = await fetch(new URL("/api/claude-code", server.url), { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ classifierModel: "", classifierFallbacks: null }), + }); + expect(cleared.status).toBe(200); + const after = await (await fetch(new URL("/api/claude-code", server.url))).json() as Record; + expect(after.classifierModel).toBe(""); + expect(after.classifierFallbacks).toEqual([]); + } finally { + await server.stop(true); + } +}); + +test("PUT rejects a malformed classifierFallbacks instead of persisting it (#1697)", async () => { + const server = startServer(0); + try { + for (const body of [{ classifierFallbacks: "mock/test-model" }, { classifierFallbacks: [1] }, { classifierFallbacks: [""] }]) { + const res = await fetch(new URL("/api/claude-code", server.url), { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body), + }); + expect(res.status).toBe(400); + const err = await res.json() as Record; + expect(String(err.error)).toContain("classifierFallbacks"); + } + } finally { + await server.stop(true); + } +}); test("PUT round-trips settings and persists to config", async () => { const server = startServer(0); try { diff --git a/tests/config.test.ts b/tests/config.test.ts index 1cd7d810cc..b949b9bd8b 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -93,6 +93,31 @@ function writeAccountNamespaceConfig( } describe("opencodex config defaults", () => { + test("malformed classifier config is normalized at load, even with subagentEffort absent (#1697)", () => { + // normalizePersistedClaudeCode used to be reached only through a subagentEffort short-circuit, + // so a config whose ONLY defect was elsewhere in claudeCode was never normalized. These + // fixtures deliberately omit subagentEffort, which is what the old path skipped on. + writeConfig({ + port: 10100, + providers: { p1: { adapter: "openai-chat", baseUrl: "https://p1.example/v1" } }, + claudeCode: { classifierFallbacks: "RelayC/claude-opus-5", classifierModel: " " }, + }); + const loaded = loadConfig() as Record; + expect(loaded.claudeCode?.classifierFallbacks).toBeUndefined(); + expect(loaded.claudeCode?.classifierModel).toBeUndefined(); + expect(loaded.providers.p1).toBeDefined(); + }); + + test("classifier fallback entries are filtered rather than trusted (#1697)", () => { + writeConfig({ + port: 10100, + providers: { p1: { adapter: "openai-chat", baseUrl: "https://p1.example/v1" } }, + claudeCode: { classifierFallbacks: [1, " RelayC/claude-opus-5 ", "", null] }, + }); + const loaded = loadConfig() as Record; + expect(loaded.claudeCode?.classifierFallbacks).toEqual(["RelayC/claude-opus-5"]); + }); + test("empty-completion retry is an explicit top-level opt-in", () => { const defaults = getDefaultConfig(); expect(defaults.emptyCompletionRetry).toBe(false); diff --git a/tests/router.test.ts b/tests/router.test.ts index e8ec59c2af..8b47e7e42e 100644 --- a/tests/router.test.ts +++ b/tests/router.test.ts @@ -563,4 +563,53 @@ describe("routeModel backfills google wire mode from the registry", () => { }; expect(routeModel(config, "gemini-3-pro").provider.googleMode).toBe("vertex"); }); + + test("a bare claude-* model is not silently rerouted to an unrelated Anthropic provider (#1697)", () => { + // The draft fix picked the first enabled provider whose adapter is anthropic, by object + // insertion order, checking neither `models`, `selectedModels`, `disabledModels` nor discovery. + // That crosses a provider/privacy/billing boundary the operator never asked for, so it is gone. + // Routing a classifier turn to a specific provider is an operator decision, expressed through + // `claudeCode.classifierModel` / `classifierFallbacks`. + const config: OcxConfig = { + port: 10100, + defaultProvider: "deepseek", + providers: { + deepseek: { + adapter: "openai-chat", + baseUrl: "https://api.deepseek.com", + }, + RelayA: { + adapter: "anthropic", + baseUrl: "https://api.anthropic.relay.example/v1", + }, + }, + }; + + const routed = routeModel(config, "claude-opus-5"); + expect(routed.providerName).toBe("deepseek"); + expect(routed.routeKind).toBe("default-provider"); + }); + + test("a disabled provider is not selected by the known-model pattern (#1697)", () => { + // This half of the draft is kept: matching a pattern provider that is disabled and routing to + // it anyway was a real defect. + const config: OcxConfig = { + port: 10100, + defaultProvider: "fallbackProvider", + providers: { + anthropic: { + adapter: "anthropic", + baseUrl: "https://api.anthropic.com", + disabled: true, + }, + fallbackProvider: { + adapter: "openai-chat", + baseUrl: "https://api.example.test/v1", + }, + }, + }; + + const routed = routeModel(config, "claude-opus-5"); + expect(routed.providerName).toBe("fallbackProvider"); + }); });