From cbcbbdd706359872837f2110f126bb0631fd887c Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sun, 9 Aug 2026 11:05:15 +0900 Subject: [PATCH 1/3] feat(management): publish vision describers and refuse the blind ones GET and PUT /api/sidecar-settings now carry a visionModels list, and both routes that can set a vision describer refuse a model we can prove cannot see. The GUI filter alone would have been cosmetic: anything holding the admin token could still write a blind model into visionSidecar.model. The gate rejects only a PROVEN-blind model. An id no source knows stays allowed, because the runtime never required catalog membership and an operator may be ahead of our tables; tests/vision-reasoning-contract.ts pins custom-vision at 200 and this change keeps it there. The caller's backend is a hint, never the authority. Trusting it opened a laundering path: o3-mini is text-only in the OpenAI table but absent from the Anthropic one, so a client could claim backend anthropic and have a blind model read as merely unknown. Both families are now consulted and any positive text-only verdict wins, which is unambiguous because the two vendor tables share no bare model id. One policy module serves both routes. A gate on /api/sidecar-settings with a private copy in the Claude Code override is the same as no gate once the two drift. Plan: devlog/_plan/260809_vision_sidecar_model_filter/020 --- docs-site/src/content/docs/guides/sidecars.md | 5 + .../management/agent-settings-routes.ts | 18 ++ src/server/management/config-routes.ts | 36 +++ .../management/vision-sidecar-options.ts | 102 ++++++++ tests/sidecar-settings-vision-filter.test.ts | 241 ++++++++++++++++++ 5 files changed, 402 insertions(+) create mode 100644 src/server/management/vision-sidecar-options.ts create mode 100644 tests/sidecar-settings-vision-filter.test.ts 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..6ef010b749 100644 --- a/src/server/management/config-routes.ts +++ b/src/server/management/config-routes.ts @@ -54,6 +54,12 @@ 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 { + visionCandidateRows, + visionDescriberIsProvablyBlind, + visionDescriberRejection, + visionModelOptionsFor, +} from "./vision-sidecar-options"; import { getDebugLogEntries } from "../../lib/debug-log-buffer"; import { getInjectionDebugLogEntries } from "../../lib/injection-debug-log"; import { @@ -382,6 +388,13 @@ export async function handleConfigRoutes(ctx: ManagementContext): Promise option.value === visionModel)) { + visionModels.unshift({ value: visionModel, label: visionModel, backend: vs.backend ?? "openai" }); + } return jsonResponse({ webSearch: { model: ws.model ?? "gpt-5.6-luna", backend: ws.backend }, vision: { @@ -390,6 +403,7 @@ export async function handleConfigRoutes(ctx: ManagementContext): Promise; let visionReasoningTouched = false; @@ -471,6 +500,12 @@ export async function handleConfigRoutes(ctx: ManagementContext): Promise option.value === visionModel)) { + visionModels.unshift({ value: visionModel, label: visionModel, backend: vs.backend ?? "openai" }); + } return jsonResponse({ ok: true, webSearch: { model: ws.model ?? "gpt-5.6-luna", backend: ws.backend }, @@ -480,6 +515,7 @@ export async function handleConfigRoutes(ctx: ManagementContext): Promise 0) backends.push("openai"); + if (findAnthropicVisionProvider(config)) 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[], +): VisionModelOption[] { + return visionEligibleModelOptions(config, candidates, enabledVisionBackends(config)); +} + +/** Convenience for read paths that have no candidate list in hand yet. */ +export async function visionModelOptionsFor(config: OcxConfig): Promise { + return visionModelOptionsFrom(config, await visionCandidateRows(config)); +} + +/** + * 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 { + const row = candidates.find(candidate => candidate.id === requested); + if (row) return modelAcceptsImageInput(config, row) === false; + + 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.`, + allowed: visionModelOptionsFrom(config, candidates).map(option => option.value), + }; +} diff --git a/tests/sidecar-settings-vision-filter.test.ts b/tests/sidecar-settings-vision-filter.test.ts new file mode 100644 index 0000000000..5cf68f7ed3 --- /dev/null +++ b/tests/sidecar-settings-vision-filter.test.ts @@ -0,0 +1,241 @@ +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; +import { mkdtempSync, rmSync } 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 UNKNOWN id (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. 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("9. 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("10. 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("11. 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"); + }); +}); From 9532f2f94f95bcbf85a06362a25080da6cacbb1e Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sun, 9 Aug 2026 14:11:41 +0900 Subject: [PATCH 2/3] fix(management): judge describers against the executor that would run them Three corrections, one theme: the API answered from a projection of the config rather than from what the runtime would actually do. - The write gate no longer stops at the first catalog row matching the id. An operator-authored row declaring image input could hide the canonical table that proves the same id blind, so o3-mini could be persisted as a describer. Every matching row and both backend probes are consulted; any positive blind verdict rejects, and an id no source knows is still accepted. - GET and the post-PUT response shared a copied block that defaulted to gpt-5.4-mini even under an anthropic backend, whose runtime default is claude-sonnet-5. Both now call one helper built on resolveEffectiveVisionModel, which planVisionSidecar also uses, so the report cannot drift from the run. - Anthropic catalog options are scoped to the OAuth provider the runtime would dispatch through, resolved once per request and threaded into the filter. --- src/server/management/config-routes.ts | 53 +++++++----- .../management/vision-sidecar-options.ts | 30 +++++-- src/vision/index.ts | 13 ++- tests/sidecar-settings-vision-filter.test.ts | 83 +++++++++++++++++-- 4 files changed, 140 insertions(+), 39 deletions(-) diff --git a/src/server/management/config-routes.ts b/src/server/management/config-routes.ts index 6ef010b749..0e3cffc67e 100644 --- a/src/server/management/config-routes.ts +++ b/src/server/management/config-routes.ts @@ -54,6 +54,7 @@ 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, @@ -86,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") { @@ -386,24 +408,16 @@ export async function handleConfigRoutes(ctx: ManagementContext): Promise option.value === visionModel)) { - visionModels.unshift({ value: visionModel, label: visionModel, backend: vs.backend ?? "openai" }); - } + const vision = await sidecarVisionResponseSettings(config); return jsonResponse({ webSearch: { model: ws.model ?? "gpt-5.6-luna", backend: ws.backend }, vision: { - model: visionModel, + model: vision.model, backend: vs.backend, - reasoning: visionReasoning, + reasoning: vision.reasoning, maxDescriptionsPerTurn: vs.maxDescriptionsPerTurn, }, - visionModels, + visionModels: vision.models, }); } @@ -498,24 +512,17 @@ export async function handleConfigRoutes(ctx: ManagementContext): Promise option.value === visionModel)) { - visionModels.unshift({ value: visionModel, label: visionModel, backend: vs.backend ?? "openai" }); - } + const vision = await sidecarVisionResponseSettings(config); return jsonResponse({ ok: true, webSearch: { model: ws.model ?? "gpt-5.6-luna", backend: ws.backend }, vision: { - model: visionModel, + model: vision.model, backend: vs.backend, - reasoning: visionReasoning, + reasoning: vision.reasoning, maxDescriptionsPerTurn: vs.maxDescriptionsPerTurn, }, - visionModels, + visionModels: vision.models, }); } diff --git a/src/server/management/vision-sidecar-options.ts b/src/server/management/vision-sidecar-options.ts index 60bf171ec6..c0f8c6375a 100644 --- a/src/server/management/vision-sidecar-options.ts +++ b/src/server/management/vision-sidecar-options.ts @@ -8,7 +8,7 @@ * the same as no gate at all. */ import type { OcxConfig } from "../../types"; -import { findAnthropicVisionProvider } from "../../vision"; +import { findAnthropicVisionProvider, type AnthropicVisionProvider } from "../../vision"; import { modelAcceptsImageInput, visionEligibleModelOptions, @@ -20,12 +20,15 @@ import { listOpenAiForwardSidecarCandidates } from "../../providers/openai-sidec import { listManagementModelRows } from "./model-rows"; /** Backends whose executor could actually run: openai forward, anthropic OAuth. */ -export function enabledVisionBackends(config: OcxConfig): VisionSidecarBackend[] { +export function enabledVisionBackends( + config: OcxConfig, + anthropicSidecar = findAnthropicVisionProvider(config), +): VisionSidecarBackend[] { const backends: VisionSidecarBackend[] = []; // The OpenAI describer needs a CANONICAL ChatGPT forward provider, not merely a // provider keyed "openai" — same predicate the runtime sidecar resolver uses. if (listOpenAiForwardSidecarCandidates(config).length > 0) backends.push("openai"); - if (findAnthropicVisionProvider(config)) backends.push("anthropic"); + 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"]; @@ -48,13 +51,22 @@ export async function visionCandidateRows(config: OcxConfig): Promise { - return visionModelOptionsFrom(config, await visionCandidateRows(config)); +export async function visionModelOptionsFor( + config: OcxConfig, + anthropicSidecar: AnthropicVisionProvider | undefined = findAnthropicVisionProvider(config), +): Promise { + return visionModelOptionsFrom(config, await visionCandidateRows(config), anthropicSidecar); } /** @@ -78,8 +90,10 @@ export function visionDescriberIsProvablyBlind( candidates: readonly VisionCandidateModel[], backendHint: VisionSidecarBackend | undefined, ): boolean { - const row = candidates.find(candidate => candidate.id === requested); - if (row) return modelAcceptsImageInput(config, row) === false; + // 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" 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 index 5cf68f7ed3..cee7e99bc7 100644 --- a/tests/sidecar-settings-vision-filter.test.ts +++ b/tests/sidecar-settings-vision-filter.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; -import { mkdtempSync, rmSync } from "node:fs"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { handleManagementAPI } from "../src/server/management-api"; @@ -155,14 +155,85 @@ describe("sidecar-settings vision model filter", () => { } }); - test("7. PUT keeps an UNKNOWN id (regression guard)", async () => { + 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. PUT /api/claude-code rejects a provably blind vision override and accepts unknown", async () => { + 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 runtime Anthropic default when that backend has no override", async () => { + 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" }, @@ -186,7 +257,7 @@ describe("sidecar-settings vision model filter", () => { expect(acceptConfig.claudeCode?.visionSidecar?.model).toBe("custom-vision"); }); - test("9. a blind model cannot be laundered by claiming the other backend", async () => { + 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. @@ -206,7 +277,7 @@ describe("sidecar-settings vision model filter", () => { expect(claudeConfig.claudeCode?.visionSidecar?.model).toBeUndefined(); }); - test("10. the reject path does not persist to disk", async () => { + 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"); @@ -221,7 +292,7 @@ describe("sidecar-settings vision model filter", () => { } }); - test("11. the web-search sidecar is deliberately NOT gated", async () => { + 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(); From 6196fc5cd7bb10f8d18243c80fae60bcc71ba352 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sun, 9 Aug 2026 14:17:41 +0900 Subject: [PATCH 3/3] refactor(management): make the resolved vision executor an explicit argument MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit follow-up. Defaulting the parameter to findAnthropicVisionProvider(config) read like sugar, but an explicit undefined argument — precisely the no-executor case — re-triggered the default in each helper, so one response could read the OAuth account store four times. The threaded parameter is now required, and only the top-level callers resolve it. --- .../management/vision-sidecar-options.ts | 19 ++++++++++++++----- tests/sidecar-settings-vision-filter.test.ts | 5 ++++- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/server/management/vision-sidecar-options.ts b/src/server/management/vision-sidecar-options.ts index c0f8c6375a..aefbb4dc86 100644 --- a/src/server/management/vision-sidecar-options.ts +++ b/src/server/management/vision-sidecar-options.ts @@ -19,10 +19,17 @@ import { import { listOpenAiForwardSidecarCandidates } from "../../providers/openai-sidecar"; import { listManagementModelRows } from "./model-rows"; -/** Backends whose executor could actually run: openai forward, anthropic OAuth. */ +/** + * Backends whose executor could actually run: openai forward, anthropic OAuth. + * + * `anthropicSidecar` is REQUIRED rather than defaulted. `findAnthropicVisionProvider` + * reads the OAuth account store from disk, and a default argument made every helper + * in this chain re-resolve it whenever a caller passed an explicit `undefined` — + * which is exactly the no-executor case. Passing it in keeps one read per request. + */ export function enabledVisionBackends( config: OcxConfig, - anthropicSidecar = findAnthropicVisionProvider(config), + anthropicSidecar: AnthropicVisionProvider | undefined, ): VisionSidecarBackend[] { const backends: VisionSidecarBackend[] = []; // The OpenAI describer needs a CANONICAL ChatGPT forward provider, not merely a @@ -51,7 +58,7 @@ export async function visionCandidateRows(config: OcxConfig): Promise { return visionModelOptionsFrom(config, await visionCandidateRows(config), anthropicSidecar); } @@ -111,6 +118,8 @@ export function visionDescriberRejection( ): { 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.`, - allowed: visionModelOptionsFrom(config, candidates).map(option => option.value), + // 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/tests/sidecar-settings-vision-filter.test.ts b/tests/sidecar-settings-vision-filter.test.ts index cee7e99bc7..c7a52e2814 100644 --- a/tests/sidecar-settings-vision-filter.test.ts +++ b/tests/sidecar-settings-vision-filter.test.ts @@ -182,7 +182,10 @@ describe("sidecar-settings vision model filter", () => { } }); - test("9. GET reports the runtime Anthropic default when that backend has no override", async () => { + 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);