diff --git a/docs-site/src/content/docs/guides/sidecars.md b/docs-site/src/content/docs/guides/sidecars.md index a397f08a9f..e62e55c3ef 100644 --- a/docs-site/src/content/docs/guides/sidecars.md +++ b/docs-site/src/content/docs/guides/sidecars.md @@ -100,6 +100,11 @@ model field. keys (Anthropic keys omit it, since that field is ignored there); mutable `https:` images are not cached. +The management API and Dashboard picker now list models that can actually accept image input. +When the matching backend is available, `gpt-5.6-luna` (OpenAI) and `claude-haiku-4-5` (Anthropic) +are always offered as baseline options. `PUT /api/sidecar-settings` rejects a model known to be +text-only, but still accepts an unknown id so custom or ahead-of-catalog names keep working. + ```json { "visionSidecar": { diff --git a/src/server/management/agent-settings-routes.ts b/src/server/management/agent-settings-routes.ts index 17f7c00b32..2ffc4c44d4 100644 --- a/src/server/management/agent-settings-routes.ts +++ b/src/server/management/agent-settings-routes.ts @@ -51,6 +51,11 @@ import { type DebugFlag, } from "../../lib/debug-settings"; import type { OcxClaudeCodeConfig, OcxConfig, OcxCustomModel, OcxProviderConfig } from "../../types"; +import { + visionCandidateRows, + visionDescriberIsProvablyBlind, + visionDescriberRejection, +} from "./vision-sidecar-options"; import { drainAndShutdown } from "../lifecycle"; import { filterRequestLogs, getRequestLogEntries, type RequestLogEntry } from "../request-log"; import { estimateComboCost, estimateRequestCost, normalizeCostTokens, tokensPerSecond } from "../../usage/cost"; @@ -1010,6 +1015,19 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise if (section.model !== undefined && typeof section.model !== "string") { return jsonResponse({ error: `${field}.model must be a string` }, 400); } + // Vision override only: reject a model we can prove is blind. Unknown ids stay + // allowed; webSearchSidecar has no vision requirement and is left alone. Shares + // one policy module with /api/sidecar-settings so the two gates cannot drift. + if (field === "visionSidecar" && typeof section.model === "string" && section.model !== "") { + const requested = section.model; + const candidates = await visionCandidateRows(config); + const hint = section.backend === "anthropic" || section.backend === "openai" + ? section.backend + : config.claudeCode?.visionSidecar?.backend; + if (visionDescriberIsProvablyBlind(config, requested, candidates, hint)) { + return jsonResponse(visionDescriberRejection("visionSidecar.model", requested, config, candidates), 400); + } + } } const next = { ...(config.claudeCode ?? {}) }; for (const field of ["webSearchSidecar", "visionSidecar"] as const) { diff --git a/src/server/management/config-routes.ts b/src/server/management/config-routes.ts index 4e6ac68268..0e3cffc67e 100644 --- a/src/server/management/config-routes.ts +++ b/src/server/management/config-routes.ts @@ -54,6 +54,13 @@ import { stripCodexRuntimeProviderFields } from "../../codex/auth-context"; import { getProviderRegistryEntry } from "../../providers/registry"; import { VISION_REASONING_EFFORTS, isVisionReasoningEffort } from "../../reasoning-effort"; import { normalizeVisionReasoningForModel } from "../../vision/reasoning"; +import { findAnthropicVisionProvider, resolveEffectiveVisionModel, resolveVisionBackend } from "../../vision"; +import { + visionCandidateRows, + visionDescriberIsProvablyBlind, + visionDescriberRejection, + visionModelOptionsFor, +} from "./vision-sidecar-options"; import { getDebugLogEntries } from "../../lib/debug-log-buffer"; import { getInjectionDebugLogEntries } from "../../lib/injection-debug-log"; import { @@ -80,6 +87,27 @@ import type { MetricUnavailableReason, TokPerSecondResult, CostEstimateReason, C import type { ManagementContext } from "./context"; import { readManagementJsonBody, rethrowManagementBodyTooLarge } from "./body"; +async function sidecarVisionResponseSettings(config: OcxConfig): Promise<{ + model: string; + reasoning: string; + models: Awaited>; +}> { + const vs = config.visionSidecar ?? {}; + // Match the runtime's one selected Anthropic executor for both backend fallback + // and catalog reachability; resolving it once prevents the two projections drifting. + const anthropicSidecar = findAnthropicVisionProvider(config); + const backend = resolveVisionBackend(vs.backend, anthropicSidecar); + const model = resolveEffectiveVisionModel(config, backend); + const reasoning = normalizeVisionReasoningForModel(model, vs.reasoning) ?? "low"; + const models = await visionModelOptionsFor(config, anthropicSidecar); + // Display-only grandfather: a persisted id stays selectable, but the write gate + // remains stricter and rejects a model that is positively proven blind. + if (!models.some(option => option.value === model)) { + models.unshift({ value: model, label: model, backend }); + } + return { model, reasoning, models }; +} + export async function handleConfigRoutes(ctx: ManagementContext): Promise { const { req, url, config, deps, convergeCodexCatalog, syncClaudeAgentDefsBestEffort } = ctx; if (url.pathname === "/api/config" && req.method === "GET") { @@ -380,16 +408,16 @@ export async function handleConfigRoutes(ctx: ManagementContext): Promise; let visionReasoningTouched = false; @@ -469,17 +512,17 @@ export async function handleConfigRoutes(ctx: ManagementContext): Promise 0) backends.push("openai"); + if (anthropicSidecar) backends.push("anthropic"); + // Neither side resolvable (fresh install, no login): fall back to both so the + // picker is populated rather than empty, matching the permissive-unknown rule. + return backends.length > 0 ? backends : ["openai", "anthropic"]; +} + +/** Visible catalog rows in the shape the eligibility predicate consumes. */ +export async function visionCandidateRows(config: OcxConfig): Promise { + let rows: Awaited> = []; + // A catalog outage must not 500 the settings route nor reject a write; with [] + // the option list degrades to the baselines, which is the intended floor. + try { rows = await listManagementModelRows(config); } catch { rows = []; } + return rows.filter(row => row.disabled !== true).map(row => ({ + provider: row.provider, + id: row.id, + ...(row.inputModalities ? { inputModalities: row.inputModalities } : {}), + ...(row.native ? { native: true } : {}), + })); +} + +export function visionModelOptionsFrom( + config: OcxConfig, + candidates: readonly VisionCandidateModel[], + anthropicSidecar: AnthropicVisionProvider | undefined, +): VisionModelOption[] { + return visionEligibleModelOptions( + config, + candidates, + enabledVisionBackends(config, anthropicSidecar), + anthropicSidecar?.providerName, + ); +} + +/** Convenience for read paths that have no candidate list in hand yet. */ +export async function visionModelOptionsFor( + config: OcxConfig, + anthropicSidecar: AnthropicVisionProvider | undefined, +): Promise { + return visionModelOptionsFrom(config, await visionCandidateRows(config), anthropicSidecar); +} + +/** + * Can we PROVE the requested describer is blind? + * + * Only a positive `false` rejects. An id no source knows stays allowed, because + * the runtime never required catalog membership and an operator may be ahead of + * our tables. + * + * When no catalog row matches, the caller's `backend` is only a HINT, never the + * authority. Trusting it let a client launder a known-blind OpenAI model past the + * gate by claiming `backend: "anthropic"`, since the id is absent from the + * Anthropic table and absence reads as "unknown". Both families are therefore + * consulted and any positive text-only verdict wins. That is safe precisely + * because the two vendor tables share no bare model id, so they can never + * disagree about one. + */ +export function visionDescriberIsProvablyBlind( + config: OcxConfig, + requested: string, + candidates: readonly VisionCandidateModel[], + backendHint: VisionSidecarBackend | undefined, +): boolean { + // Catalog rows can carry operator-authored modalities. A positive claim from one + // must not hide another row or canonical metadata that proves this id is blind. + if (candidates.some(candidate => candidate.id === requested + && modelAcceptsImageInput(config, candidate) === false)) return true; + + const hinted: VisionSidecarBackend = backendHint === "anthropic" ? "anthropic" : "openai"; + const probed: VisionSidecarBackend[] = hinted === "anthropic" + ? ["anthropic", "openai"] + : ["openai", "anthropic"]; + return probed.some(provider => modelAcceptsImageInput(config, { provider, id: requested }) === false); +} + +/** The 400 body both routes return, so the two errors cannot diverge either. */ +export function visionDescriberRejection( + field: "vision.model" | "visionSidecar.model", + requested: string, + config: OcxConfig, + candidates: readonly VisionCandidateModel[], +): { error: string; allowed: string[] } { + return { + error: `${field} "${requested}" cannot describe images: it has no image input support, or it is a model the vision sidecar describes FOR.`, + // The rejection path is not hot, so it resolves the executor itself rather than + // making every 400 caller thread one through. + allowed: visionModelOptionsFrom(config, candidates, findAnthropicVisionProvider(config)).map(option => option.value), + }; +} diff --git a/src/vision/index.ts b/src/vision/index.ts index 0e2fdf38af..89abe4088f 100644 --- a/src/vision/index.ts +++ b/src/vision/index.ts @@ -201,6 +201,16 @@ export function resolveOpenAiVisionModel(config: Pick, + backend: "openai" | "anthropic", +): string { + return backend === "anthropic" + ? config.visionSidecar?.model || DEFAULT_ANTHROPIC_VISION_MODEL + : resolveOpenAiVisionModel(config); +} + /** A user/developer/toolResult message can carry images (toolResult: e.g. Codex view_image output). */ function carriesImages(role: string): boolean { return role === "user" || role === "developer" || role === "toolResult"; @@ -250,11 +260,11 @@ export function planVisionSidecar( if (cfg.enabled === false) return undefined; const anthropicSidecar = findAnthropicVisionProvider(config); const backend = resolveVisionBackend(cfg.backend, anthropicSidecar); + const model = resolveEffectiveVisionModel(config, backend); const maxDescriptionsPerTurn = resolveMaxDescriptionsPerTurn(cfg.maxDescriptionsPerTurn); if (backend === "anthropic") { if (!anthropicSidecar) return undefined; - const model = cfg.model || DEFAULT_ANTHROPIC_VISION_MODEL; return { backend, anthropicSidecar, @@ -268,7 +278,6 @@ export function planVisionSidecar( } if (!openAiSidecar) return undefined; - const model = resolveOpenAiVisionModel(config); return { backend, forwardSidecar: openAiSidecar, diff --git a/tests/sidecar-settings-vision-filter.test.ts b/tests/sidecar-settings-vision-filter.test.ts new file mode 100644 index 0000000000..c7a52e2814 --- /dev/null +++ b/tests/sidecar-settings-vision-filter.test.ts @@ -0,0 +1,315 @@ +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { handleManagementAPI } from "../src/server/management-api"; +import * as modelRows from "../src/server/management/model-rows"; +import type { OcxConfig } from "../src/types"; +import { BASELINE_VISION_MODELS } from "../src/vision/eligibility"; +import { ManagementRequest as Request } from "./helpers/management-auth"; + +async function getSidecarSettings(config: OcxConfig): Promise { + const url = new URL("http://localhost/api/sidecar-settings"); + const response = await handleManagementAPI(new Request(url), url, config); + if (!response) throw new Error("sidecar settings route did not handle GET"); + return response; +} + +async function putSidecarSettings(config: OcxConfig, vision: Record): Promise { + const url = new URL("http://localhost/api/sidecar-settings"); + const response = await handleManagementAPI( + new Request(url, { + method: "PUT", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ vision }), + }), + url, + config, + ); + if (!response) throw new Error("sidecar settings route did not handle PUT"); + return response; +} + +async function putClaudeCode(config: OcxConfig, body: Record): Promise { + const url = new URL("http://localhost/api/claude-code"); + const response = await handleManagementAPI( + new Request(url, { + method: "PUT", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }), + url, + config, + ); + if (!response) throw new Error("claude-code route did not handle PUT"); + return response; +} + +function emptyConfig(overrides: Partial = {}): OcxConfig { + return { + port: 10100, + defaultProvider: "none", + providers: {}, + ...overrides, + } as OcxConfig; +} + +describe("sidecar-settings vision model filter", () => { + let previousHome: string | undefined; + let isolatedHome: string | undefined; + + beforeEach(() => { + previousHome = process.env.OPENCODEX_HOME; + isolatedHome = mkdtempSync(join(tmpdir(), "ocx-sidecar-vision-filter-")); + process.env.OPENCODEX_HOME = isolatedHome; + }); + + afterEach(() => { + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + if (isolatedHome) rmSync(isolatedHome, { recursive: true, force: true }); + isolatedHome = undefined; + }); + + test("1. GET returns the allowed list containing gpt-5.6-luna", async () => { + const config = emptyConfig(); + const response = await getSidecarSettings(config); + expect(response.status).toBe(200); + const body = await response.json() as { + visionModels?: Array<{ value: string; label: string; backend: string; baseline?: boolean }>; + }; + expect(Array.isArray(body.visionModels)).toBe(true); + expect(body.visionModels!.some(option => option.value === BASELINE_VISION_MODELS.openai)).toBe(true); + expect(body.visionModels!.some(option => option.value === "gpt-5.6-luna")).toBe(true); + }); + + test("2. GET keeps a configured-but-ineligible model selectable", async () => { + const config = emptyConfig({ + visionSidecar: { model: "o3-mini", backend: "openai" }, + }); + const response = await getSidecarSettings(config); + expect(response.status).toBe(200); + const body = await response.json() as { + vision: { model: string }; + visionModels: Array<{ value: string }>; + }; + expect(body.vision.model).toBe("o3-mini"); + expect(body.visionModels.some(option => option.value === "o3-mini")).toBe(true); + }); + + test("3. PUT rejects an ineligible model with 400 and does not persist", async () => { + const config = emptyConfig({ + visionSidecar: { model: "gpt-5.6-luna", reasoning: "low" }, + }); + const before = structuredClone(config.visionSidecar); + const response = await putSidecarSettings(config, { model: "o3-mini" }); + expect(response.status).toBe(400); + const body = await response.json() as { error?: string; allowed?: string[] }; + expect(typeof body.error).toBe("string"); + expect(Array.isArray(body.allowed)).toBe(true); + expect(body.allowed!.length).toBeGreaterThan(0); + expect(config.visionSidecar).toEqual(before); + }); + + test("4. PUT accepts an eligible model", async () => { + const config = emptyConfig(); + const response = await putSidecarSettings(config, { model: "claude-haiku-4-5" }); + expect(response.status).toBe(200); + const body = await response.json() as { + vision: { model: string }; + visionModels: Array<{ value: string }>; + }; + expect(body.vision.model).toBe("claude-haiku-4-5"); + expect(config.visionSidecar?.model).toBe("claude-haiku-4-5"); + expect(Array.isArray(body.visionModels)).toBe(true); + }); + + test("5. PUT with model: \"\" still clears the override", async () => { + const config = emptyConfig({ + visionSidecar: { model: "gpt-5.6-luna", reasoning: "low" }, + }); + const response = await putSidecarSettings(config, { model: "" }); + expect(response.status).toBe(200); + const body = await response.json() as { vision: { model: string } }; + // Empty string clears the override; the effective reported model is the fallback. + expect(config.visionSidecar?.model).toBeUndefined(); + expect(body.vision.model).toBe("gpt-5.4-mini"); + }); + + test("6. catalog failure degrades to baselines", async () => { + const rowsSpy = spyOn(modelRows, "listManagementModelRows").mockImplementation(async () => { + throw new Error("catalog unavailable"); + }); + try { + const config = emptyConfig(); + const response = await getSidecarSettings(config); + expect(response.status).toBe(200); + const body = await response.json() as { + visionModels: Array<{ value: string; baseline?: boolean }>; + }; + const values = body.visionModels.map(option => option.value); + expect(values).toContain(BASELINE_VISION_MODELS.openai); + expect(values).toContain(BASELINE_VISION_MODELS.anthropic); + } finally { + rowsSpy.mockRestore(); + } + }); + + test("7. PUT keeps an id no source knows (regression guard)", async () => { + const config = emptyConfig({ providers: {} }); + const response = await putSidecarSettings(config, { model: "custom-vision" }); + expect(response.status).toBe(200); + expect(config.visionSidecar).toMatchObject({ model: "custom-vision" }); + }); + + test("8. a custom image declaration cannot mask an authoritative blind OpenAI model", async () => { + // Custom rows may declare modalities, but their claim cannot overrule the native + // OpenAI table for a bare OpenAI model id. The write gate must inspect both. + const rowsSpy = spyOn(modelRows, "listManagementModelRows").mockResolvedValue([{ + provider: "custom", + id: "o3-mini", + namespaced: "custom/o3-mini", + disabled: false, + inputModalities: ["text", "image"], + }]); + try { + const config = emptyConfig(); + const response = await putSidecarSettings(config, { model: "o3-mini" }); + expect(response.status).toBe(400); + expect(config.visionSidecar?.model).toBeUndefined(); + } finally { + rowsSpy.mockRestore(); + } + }); + + test("9. GET reports the effective Anthropic default for an explicitly selected backend", async () => { + // Reports what the runtime WOULD use for this backend. No OAuth account is set up + // here, so no plan would run; the point is that the projection stops answering + // gpt-5.4-mini for a configuration the OpenAI describer does not own. + const config = emptyConfig({ visionSidecar: { backend: "anthropic" } }); + const response = await getSidecarSettings(config); + expect(response.status).toBe(200); + const body = await response.json() as { vision: { model: string; backend?: string } }; + expect(body.vision).toMatchObject({ model: "claude-sonnet-5", backend: "anthropic" }); + }); + + test("10. GET exposes only catalog rows reachable by the executing Anthropic OAuth provider", async () => { + writeFileSync(join(isolatedHome!, "auth.json"), JSON.stringify({ + "anthropic-oauth": { + activeAccountId: "active", + accounts: [{ + id: "active", + credential: { access: "access", refresh: "refresh", expires: 9_999_999_999_999 }, + }], + }, + })); + const rowsSpy = spyOn(modelRows, "listManagementModelRows").mockResolvedValue([ + { + provider: "anthropic-key", + id: "key-only-vision", + namespaced: "anthropic-key/key-only-vision", + disabled: false, + inputModalities: ["text", "image"], + }, + { + provider: "anthropic-oauth", + id: "oauth-vision", + namespaced: "anthropic-oauth/oauth-vision", + disabled: false, + inputModalities: ["text", "image"], + }, + ]); + try { + const config = emptyConfig({ + providers: { + "anthropic-key": { adapter: "anthropic", authMode: "key", baseUrl: "https://api.anthropic.com" }, + "anthropic-oauth": { adapter: "anthropic", authMode: "oauth", baseUrl: "https://api.anthropic.com" }, + }, + }); + const response = await getSidecarSettings(config); + expect(response.status).toBe(200); + const body = await response.json() as { visionModels: Array<{ value: string; backend: string }> }; + expect(body.visionModels).toContainEqual(expect.objectContaining({ value: "oauth-vision", backend: "anthropic" })); + expect(body.visionModels.some(option => option.value === "key-only-vision")).toBe(false); + } finally { + rowsSpy.mockRestore(); + } + }); + + test("11. PUT /api/claude-code rejects a provably blind vision override and accepts unknown", async () => { + const rejectConfig = emptyConfig({ + claudeCode: { + visionSidecar: { model: "gpt-5.6-luna" }, + }, + }); + const before = structuredClone(rejectConfig.claudeCode); + const rejected = await putClaudeCode(rejectConfig, { + visionSidecar: { model: "o3-mini" }, + }); + expect(rejected.status).toBe(400); + const rejectedBody = await rejected.json() as { error?: string; allowed?: string[] }; + expect(typeof rejectedBody.error).toBe("string"); + expect(Array.isArray(rejectedBody.allowed)).toBe(true); + expect(rejectConfig.claudeCode).toEqual(before); + + const acceptConfig = emptyConfig(); + const accepted = await putClaudeCode(acceptConfig, { + visionSidecar: { model: "custom-vision" }, + }); + expect(accepted.status).toBe(200); + expect(acceptConfig.claudeCode?.visionSidecar?.model).toBe("custom-vision"); + }); + + test("12. a blind model cannot be laundered by claiming the other backend", async () => { + // Regression: the gate used to synthesize the candidate's provider from the + // caller's `backend`, so `backend: "anthropic"` made a known text-only OpenAI + // model look merely unknown (absent from the Anthropic table) and it saved. + // The backend is a hint, never the authority. + for (const backend of ["openai", "anthropic"] as const) { + const config = emptyConfig(); + const response = await putSidecarSettings(config, { model: "o3-mini", backend }); + expect(response.status).toBe(400); + expect(config.visionSidecar?.model).toBeUndefined(); + } + + const claudeConfig = emptyConfig(); + const claudeResponse = await putClaudeCode(claudeConfig, { + visionSidecar: { model: "o3-mini", backend: "anthropic" }, + }); + expect(claudeResponse.status).toBe(400); + expect(claudeConfig.claudeCode?.visionSidecar?.model).toBeUndefined(); + }); + + test("13. the reject path does not persist to disk", async () => { + // In-memory equality alone would not prove a disk write did not happen if the + // gate were ever reordered after the mutation block. + const configModule = await import("../src/config"); + const saveSpy = spyOn(configModule, "saveConfigPreservingClaudeCode"); + try { + const config = emptyConfig(); + const response = await putSidecarSettings(config, { model: "o3-mini" }); + expect(response.status).toBe(400); + expect(saveSpy).not.toHaveBeenCalled(); + } finally { + saveSpy.mockRestore(); + } + }); + + test("14. the web-search sidecar is deliberately NOT gated", async () => { + // False-positive guard: only the vision describer needs eyes. If this ever + // starts failing, the gate has leaked into the wrong field. + const config = emptyConfig(); + const url = new URL("http://localhost/api/sidecar-settings"); + const response = await handleManagementAPI( + new Request(url, { + method: "PUT", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ webSearch: { model: "o3-mini" } }), + }), + url, + config, + ); + expect(response?.status).toBe(200); + expect(config.webSearchSidecar?.model).toBe("o3-mini"); + }); +});