From 6bc0c117d2de6b54dd33af566f4f2386ca583daf Mon Sep 17 00:00:00 2001 From: Eleven-is-cool Date: Tue, 28 Jul 2026 20:23:13 +0800 Subject: [PATCH 1/7] fix: preserve hosted image tool preferences --- .../ja/reference/configuration/providers.md | 1 + .../ko/reference/configuration/providers.md | 1 + .../docs/reference/configuration/providers.md | 1 + .../ru/reference/configuration/providers.md | 1 + .../reference/configuration/providers.md | 1 + src/adapters/openai-responses.ts | 136 ++++++- src/config.ts | 71 ++++ src/providers/openai-virtual-models.ts | 1 + src/responses/hosted-tool-policy.ts | 9 + src/server/auth-cors.ts | 8 + src/types.ts | 7 + structure/04_transports-and-sidecars.md | 11 + tests/config.test.ts | 152 ++++++++ tests/management-provider-validation.test.ts | 69 ++++ tests/openai-api-virtual-models.test.ts | 13 + tests/openai-responses-passthrough.test.ts | 355 ++++++++++++++++++ 16 files changed, 819 insertions(+), 18 deletions(-) create mode 100644 src/responses/hosted-tool-policy.ts diff --git a/docs-site/src/content/docs/ja/reference/configuration/providers.md b/docs-site/src/content/docs/ja/reference/configuration/providers.md index 38067d5226..42b5e59971 100644 --- a/docs-site/src/content/docs/ja/reference/configuration/providers.md +++ b/docs-site/src/content/docs/ja/reference/configuration/providers.md @@ -68,6 +68,7 @@ description: プロバイダー エントリ、認証、エンドポイント、 | `modelSupportsReasoningSummaries?` | `Record` |モデルを `false` に設定して、概要の広告を停止し、概要配信フィールドを削除します。 | | `modelReasoningSummaryDelivery?` | `Record` |モデルごとの応答配信列挙型。既存の配信フィールドを書き換えます。 | | `modelAdapters?` | `Record` | 混合配線ゲートウェイのモデルごとの `openai-chat` または `openai-responses` 配線オーバーライド。明示的なエントリはレジストリのデフォルトを破ります。DeepSeek のプリセットは `deepseek-v4-flash` のネイティブ Responses を選択でき、GitHub Copilot は GPT-5 ファミリー (`gpt-5.3-codex`, `gpt-5.4`, `gpt-5.4-mini`, `gpt-5.5`, `gpt-5.6-luna`, `gpt-5.6-sol`, `gpt-5.6-terra`) を Responses 専用デフォルトとして宣言します。これらのモデルはエージェント トラフィックで `/chat/completions` を拒否するためです。`gpt-5.4-nano` のようなビルトイン デフォルトのないモデルはここでオプトインできます。単線アップストリーム ピンと正規の ChatGPT 転送はオーバーライドを拒否します。 | +| `modelPreferHostedTools?` | `Record` | hosted tool namespace を予約する非 forward Responses gateway 向けの完全一致モデル opt-in。現在は `["image_generation"]` のみを受け付けます。一致したモデルは `openai-responses` wire を使い、その hosted tool をサポートする必要があります。競合するクライアント `image_gen` 宣言を除去し、呼び出し元の tool choice を維持するため selector も書き換えます。OpenAI API の仮想 `-pro` モデルでは、まず選択した公開 ID に一致させ、解決後のベース wire-model ID をフォールバックとして使用します。`modelAdapters` は公開 ID、次にベース ID の順に解決し、後者の結果が最終 wire を決めます。未設定のモデルは通常の alias 動作を維持します。 | | `reasoningEffortMap?` | `Record` |ラベルを推論するためのプロバイダー全体のワイヤ エイリアス。 | | `modelReasoningEffortMap?` | `Record>` |推論ラベルのモデルごとのワイヤ エイリアス。 | | `reasoningWireFormat?` | `"gateway-object"` | `reasoning_effort` ではなく `reasoning: { enabled, effort }` を受け取る OpenAI 互換ゲートウェイ用です。ClinePass プリセットが自動設定します。 | diff --git a/docs-site/src/content/docs/ko/reference/configuration/providers.md b/docs-site/src/content/docs/ko/reference/configuration/providers.md index 83ffabc15c..25b40955d4 100644 --- a/docs-site/src/content/docs/ko/reference/configuration/providers.md +++ b/docs-site/src/content/docs/ko/reference/configuration/providers.md @@ -68,6 +68,7 @@ description: 공급자 항목, 인증, 엔드포인트, 모델 카탈로그, 할 | `modelSupportsReasoningSummaries?` | `Record` | 모델을 `false`로 두면 summary 광고를 멈추고 summary 전달 필드를 제거합니다. | | `modelReasoningSummaryDelivery?` | `Record` | 모델별 Responses 전달 enum입니다. 기존 delivery 필드를 다시 씁니다. | | `modelAdapters?` | `Record` | 혼합 와이어 게이트웨이를 위한 모델별 `openai-chat` 또는 `openai-responses` 와이어 재정의입니다. 명시적 항목이 레지스트리 기본값보다 우선합니다. DeepSeek 프리셋은 `deepseek-v4-flash`에 네이티브 Responses를 선택할 수 있고, GitHub Copilot은 GPT-5 계열(`gpt-5.3-codex`, `gpt-5.4`, `gpt-5.4-mini`, `gpt-5.5`, `gpt-5.6-luna`, `gpt-5.6-sol`, `gpt-5.6-terra`)을 Responses 전용 기본값으로 선언합니다. 이 모델들은 에이전트 트래픽에서 `/chat/completions`를 거부하기 때문입니다. `gpt-5.4-nano`처럼 기본값이 없는 모델은 여기서 직접 옵트인할 수 있습니다. 단일 와이어 상위 항목과 정식 ChatGPT forward는 재정의를 거부합니다. | +| `modelPreferHostedTools?` | `Record` | hosted tool namespace를 예약하는 non-forward Responses gateway용 정확한 모델 ID opt-in입니다. 현재 `["image_generation"]`만 허용하며, 일치하는 모델은 `openai-responses` wire를 사용하고 해당 hosted tool을 지원해야 합니다. 충돌하는 클라이언트 `image_gen` 선언을 제거하고 호출자의 tool choice를 유지하도록 selector도 다시 씁니다. OpenAI API 가상 `-pro` 모델은 선택한 공개 ID를 먼저 일치시키고, 해석된 기본 wire-model ID를 대체값으로 사용합니다. `modelAdapters`는 공개 ID를 먼저, 그 다음 기본 ID를 해석하며, 두 번째 결과가 최종 wire를 결정합니다. 설정하지 않은 모델은 일반 alias 동작을 유지합니다. | | `reasoningEffortMap?` | `Record` | reasoning 레이블의 공급자 전반 와이어 별칭입니다. | | `modelReasoningEffortMap?` | `Record>` | reasoning 레이블의 모델별 와이어 별칭입니다. | | `reasoningWireFormat?` | `"gateway-object"` | `reasoning_effort` 대신 `reasoning: { enabled, effort }`를 받는 OpenAI 호환 게이트웨이용입니다. ClinePass 프리셋이 자동 설정합니다. | diff --git a/docs-site/src/content/docs/reference/configuration/providers.md b/docs-site/src/content/docs/reference/configuration/providers.md index 209a26d25f..80c7ed5206 100644 --- a/docs-site/src/content/docs/reference/configuration/providers.md +++ b/docs-site/src/content/docs/reference/configuration/providers.md @@ -79,6 +79,7 @@ differing backup and rewrites known legacy namespaced selected ids to bare ids. | `modelSupportsReasoningSummaries?` | `Record` | Set a model to `false` to stop advertising summaries and strip summary-delivery fields. | | `modelReasoningSummaryDelivery?` | `Record` | Per-model Responses delivery enum; rewrites an existing delivery field. | | `modelAdapters?` | `Record` | Per-model `openai-chat` or `openai-responses` wire override for mixed-wire gateways. Explicit entries beat registry defaults; DeepSeek's preset can select native Responses for `deepseek-v4-flash`, and GitHub Copilot declares Responses-only defaults for its GPT-5 family (`gpt-5.3-codex`, `gpt-5.4`, `gpt-5.4-mini`, `gpt-5.5`, `gpt-5.6-luna`, `gpt-5.6-sol`, `gpt-5.6-terra`) because those models reject `/chat/completions` for agent traffic. Models without a built-in default (for example `gpt-5.4-nano`) can be opted in here. Single-wire upstream pins and canonical ChatGPT forward reject overrides. | +| `modelPreferHostedTools?` | `Record` | Exact-model opt-in for non-forward Responses gateways that reserve a hosted-tool namespace. Currently accepts only `["image_generation"]`; a matching model must use the `openai-responses` wire and support that hosted tool. It removes colliding client `image_gen` declarations and rewrites their selectors to preserve caller tool choice. For OpenAI API virtual `-pro` models, the selected public ID is matched first and the resolved base wire-model ID is a fallback. `modelAdapters` resolves the public ID first, then the base ID; the second resolution determines the final wire. Other models retain normal alias behavior. | | `reasoningEffortMap?` | `Record` | Provider-wide wire aliases for reasoning labels. | | `modelReasoningEffortMap?` | `Record>` | Per-model wire aliases for reasoning labels. | | `reasoningWireFormat?` | `"gateway-object"` | For OpenAI-compatible gateways that accept `reasoning: { enabled, effort }` instead of `reasoning_effort`. The ClinePass preset sets this automatically. | diff --git a/docs-site/src/content/docs/ru/reference/configuration/providers.md b/docs-site/src/content/docs/ru/reference/configuration/providers.md index 6a9957d0b6..1ece0bb052 100644 --- a/docs-site/src/content/docs/ru/reference/configuration/providers.md +++ b/docs-site/src/content/docs/ru/reference/configuration/providers.md @@ -84,6 +84,7 @@ cross-route credential fallback не существует. Строки API GPT- | `modelSupportsReasoningSummaries?` | `Record` | Установите `false` для модели, чтобы перестать рекламировать summary и вырезать поля доставки summary. | | `modelReasoningSummaryDelivery?` | `Record` | Responses delivery enum по моделям; переписывает уже существующее поле delivery. | | `modelAdapters?` | `Record` | Wire-override по модели для `openai-chat` или `openai-responses` в gateway с несколькими wire-форматами. Явные записи имеют приоритет над default'ами registry; preset DeepSeek может выбирать native Responses для `deepseek-v4-flash`, а GitHub Copilot объявляет Responses-only default'ы для семейства GPT-5 (`gpt-5.3-codex`, `gpt-5.4`, `gpt-5.4-mini`, `gpt-5.5`, `gpt-5.6-luna`, `gpt-5.6-sol`, `gpt-5.6-terra`), потому что эти модели отклоняют `/chat/completions` для агентного трафика. Модели без встроенного default'а (например, `gpt-5.4-nano`) можно включить здесь. Single-wire upstream pin'ы и canonical ChatGPT forward override не принимают. | +| `modelPreferHostedTools?` | `Record` | Opt-in для точного model ID в non-forward Responses gateway, который резервирует namespace hosted tool. Сейчас допускается только `["image_generation"]`; совпавшая модель должна использовать wire `openai-responses` и поддерживать этот hosted tool. Прокси удаляет конфликтующие клиентские объявления `image_gen` и переписывает их selectors, сохраняя caller tool choice. Для виртуальных моделей OpenAI API `-pro` сначала сопоставляется выбранный публичный ID, а затем в качестве fallback используется ID базовой wire-модели. `modelAdapters` сначала разрешается по публичному ID, затем по базовому ID; второй результат определяет итоговый wire. Остальные модели сохраняют обычное alias-поведение. | | `reasoningEffortMap?` | `Record` | Provider-wide wire-alias'ы для reasoning-label'ов. | | `modelReasoningEffortMap?` | `Record>` | Wire-alias'ы для reasoning-label'ов по отдельным моделям. | | `reasoningWireFormat?` | `"gateway-object"` | Для OpenAI-совместимых шлюзов, принимающих `reasoning: { enabled, effort }` вместо `reasoning_effort`. Пресет ClinePass задаёт это автоматически. | diff --git a/docs-site/src/content/docs/zh-cn/reference/configuration/providers.md b/docs-site/src/content/docs/zh-cn/reference/configuration/providers.md index 139afb6769..9dc3f50d87 100644 --- a/docs-site/src/content/docs/zh-cn/reference/configuration/providers.md +++ b/docs-site/src/content/docs/zh-cn/reference/configuration/providers.md @@ -68,6 +68,7 @@ description: 提供者条目、身份验证、端点、模型目录、配额、 | `modelSupportsReasoningSummaries?` | `Record` | 将某个模型设为 `false`，即可停止暴露摘要并移除摘要交付字段。 | | `modelReasoningSummaryDelivery?` | `Record` | 按模型设置的 Responses 交付枚举；会重写现有的 delivery 字段。 | | `modelAdapters?` | `Record` | 按模型设置的 `openai-chat` 或 `openai-responses` 线协议覆盖项，用于混合线协议网关。显式条目优先于注册表默认值；DeepSeek 预设可以为 `deepseek-v4-flash` 选择原生 Responses，GitHub Copilot 则为 GPT-5 系列（`gpt-5.3-codex`、`gpt-5.4`、`gpt-5.4-mini`、`gpt-5.5`、`gpt-5.6-luna`、`gpt-5.6-sol`、`gpt-5.6-terra`）声明了 Responses 专用默认值，因为这些模型在代理流量下会拒绝 `/chat/completions`。没有内置默认值的模型（例如 `gpt-5.4-nano`）可以在此手动启用。单一线协议上游固定项和规范 ChatGPT forward 会拒绝覆盖。 | +| `modelPreferHostedTools?` | `Record` | 非 forward Responses gateway 的精确模型 ID opt-in，用于上游预留 hosted tool namespace 的情况。目前只支持 `["image_generation"]`；匹配模型必须使用 `openai-responses` wire 且支持该 hosted 工具。它会移除冲突的客户端 `image_gen` 声明，并改写其 selector 以保持调用方的 tool choice。对于 OpenAI API 的虚拟 `-pro` 模型，先匹配所选公开 ID，未命中时才使用解析出的基础 wire-model ID 作为回退。`modelAdapters` 会先按公开 ID、再按基础 ID 解析；后一次结果决定最终 wire。未配置模型保持普通 alias 行为。 | | `reasoningEffortMap?` | `Record` | 提供者级、用于推理标签的线协议别名。 | | `modelReasoningEffortMap?` | `Record>` | 按模型设置的推理标签线协议别名。 | | `reasoningWireFormat?` | `"gateway-object"` | 用于接受 `reasoning: { enabled, effort }` 而非 `reasoning_effort` 的 OpenAI 兼容 gateway。ClinePass preset 会自动设置。 | diff --git a/src/adapters/openai-responses.ts b/src/adapters/openai-responses.ts index 428042ffd8..937ca597b4 100644 --- a/src/adapters/openai-responses.ts +++ b/src/adapters/openai-responses.ts @@ -4,6 +4,7 @@ import { namespacedToolName, type AdapterEvent, type OcxParsedRequest, type OcxP import { catalogModelSupportsReasoningSummaries } from "../codex/catalog"; import { COMPACT_PROMPT, decodeCompactionSummary, SUMMARY_PREFIX } from "../responses/compaction"; import { collectResponsesToolGroups } from "../responses/tool-groups"; +import { isHostedToolUnsupportedForModel } from "../responses/hosted-tool-policy"; import { decodeServerSentEvents } from "../lib/sse-decoder"; import { isCanonicalOpenAiForwardProvider } from "../providers/openai-tiers"; import { OCX_REASONING_PREFIX } from "../responses/reasoning-envelope"; @@ -161,18 +162,6 @@ function scrubOcxCompactionItems(body: unknown): unknown { return changed ? { ...body, input } : body; } -/** - * Hosted (OpenAI-executed) tool types that specific native slugs reject at request time. Codex - * attaches these for app skills (e.g. `image_generation` for imagegen) regardless of the target - * model, and the passthrough path forwards the raw body untouched — so a slug that doesn't support - * the tool 400s (`Tool 'image_generation' is not supported with gpt-5.3-codex-spark.`). Each entry - * maps a model-slug matcher to the hosted tool types that must be stripped before forwarding. - * Extend this when another native slug rejects a hosted tool (e.g. `code_interpreter`). - */ -const UNSUPPORTED_HOSTED_TOOLS: ReadonlyArray<{ match: (model: string) => boolean; tools: ReadonlySet }> = [ - { match: model => model.includes("codex-spark"), tools: new Set(["image_generation", "tool_search"]) }, -]; - /** * Strip unsupported `reasoning` sub-parameters for native slugs that reject them (e.g. Spark). * codex-rs injects `reasoning.context` and `reasoning.summary` based on catalog flags; Spark's @@ -621,6 +610,114 @@ function declaresImageGenClientTool(tool: unknown): boolean { return isImageGenClientName(tool.name); } +/** Rewrite client image-gen selectors to the hosted tool without widening caller restrictions. */ +function preferHostedImageGenToolChoice(toolChoice: unknown): unknown { + if (!isPlainObject(toolChoice)) return toolChoice; + if ((toolChoice.type === "function" || toolChoice.type === "custom") && typeof toolChoice.name === "string") { + return isImageGenClientName(toolChoice.name) ? { type: HOSTED_IMAGE_GENERATION_TOOL } : toolChoice; + } + if (toolChoice.type !== "allowed_tools" || !Array.isArray(toolChoice.tools)) return toolChoice; + const hasHostedImageTool = toolChoice.tools.some(tool => isPlainObject(tool) && tool.type === HOSTED_IMAGE_GENERATION_TOOL); + let changed = false; + let addedHostedImageTool = false; + const tools: unknown[] = []; + for (const tool of toolChoice.tools) { + const isClientImageTool = isPlainObject(tool) + && (tool.type === "function" || tool.type === "custom") + && typeof tool.name === "string" + && isImageGenClientName(tool.name); + if (!isClientImageTool) { + tools.push(tool); + continue; + } + changed = true; + if (!hasHostedImageTool && !addedHostedImageTool) { + tools.push({ type: HOSTED_IMAGE_GENERATION_TOOL }); + addedHostedImageTool = true; + } + } + return changed ? { ...toolChoice, tools } : toolChoice; +} + +/** + * Some Responses-compatible gateways reserve the hosted image namespace even when the request + * does not explicitly declare `image_generation`. For an explicitly configured model, remove only + * colliding client declarations so the gateway's hosted tool can take precedence. + */ +function preferConfiguredHostedTools( + body: unknown, + provider: OcxProviderConfig, + modelId: string, + selectedModelId?: string, +): unknown { + // A virtual model's advertised id takes precedence over its resolved wire-model id. + const preferredTools = (selectedModelId ? provider.modelPreferHostedTools?.[selectedModelId] : undefined) + ?? provider.modelPreferHostedTools?.[modelId]; + if (!preferredTools?.includes(HOSTED_IMAGE_GENERATION_TOOL) || !isPlainObject(body)) return body; + + const stripGroup = (tools: unknown[]): unknown[] => { + const filtered = tools.filter(tool => !declaresImageGenClientTool(tool)); + return filtered.length === tools.length ? tools : filtered; + }; + + let changed = false; + let tools = body.tools; + let strippedTopLevelImageGenTool = false; + if (Array.isArray(body.tools)) { + tools = stripGroup(body.tools); + strippedTopLevelImageGenTool = tools !== body.tools; + changed ||= strippedTopLevelImageGenTool; + } + + let input = body.input; + let strippedAdditionalToolsIndex = -1; + if (Array.isArray(body.input)) { + let nestedChanged = false; + const mappedInput = body.input.map((item, index) => { + if (!isPlainObject(item) || item.type !== "additional_tools" || !Array.isArray(item.tools)) return item; + const nestedTools = stripGroup(item.tools); + if (nestedTools === item.tools) return item; + if (strippedAdditionalToolsIndex === -1) strippedAdditionalToolsIndex = index; + nestedChanged = true; + return { ...item, tools: nestedTools }; + }); + if (nestedChanged) { + input = mappedInput; + changed = true; + } + } + + const hasToolChoice = Object.hasOwn(body, "tool_choice"); + const toolChoice = hasToolChoice ? preferHostedImageGenToolChoice(body.tool_choice) : body.tool_choice; + const toolChoiceChanged = hasToolChoice && toolChoice !== body.tool_choice; + const hasHostedImageGenTool = (toolGroup: unknown): boolean => Array.isArray(toolGroup) + && toolGroup.some(tool => isPlainObject(tool) && tool.type === HOSTED_IMAGE_GENERATION_TOOL); + const hasHostedImageGenDeclaration = hasHostedImageGenTool(tools) + || (Array.isArray(input) && input.some(item => isPlainObject(item) + && item.type === "additional_tools" + && hasHostedImageGenTool(item.tools))); + if ((strippedTopLevelImageGenTool || strippedAdditionalToolsIndex >= 0) && !hasHostedImageGenDeclaration) { + if (strippedTopLevelImageGenTool && Array.isArray(tools)) { + tools = [...tools, { type: HOSTED_IMAGE_GENERATION_TOOL }]; + } else if (strippedAdditionalToolsIndex >= 0 && Array.isArray(input)) { + input = input.map((item, index) => index === strippedAdditionalToolsIndex + && isPlainObject(item) + && Array.isArray(item.tools) + ? { ...item, tools: [...item.tools, { type: HOSTED_IMAGE_GENERATION_TOOL }] } + : item); + } + } + changed ||= toolChoiceChanged; + if (!changed) return body; + const next: Record = { + ...body, + ...(Array.isArray(body.tools) ? { tools } : {}), + ...(Array.isArray(body.input) ? { input } : {}), + }; + if (toolChoiceChanged) next.tool_choice = toolChoice; + return next; +} + /** * Lower one complete Codex image-gen namespace to public Responses function tools. * @@ -879,13 +976,9 @@ function normalizeImageGenClientTools(body: unknown): unknown { function stripUnsupportedHostedTools(body: unknown): unknown { if (!isPlainObject(body) || !Array.isArray(body.tools)) return body; const model = typeof body.model === "string" ? body.model : ""; - const unsupported = UNSUPPORTED_HOSTED_TOOLS.filter(e => e.match(model)); - if (unsupported.length === 0) return body; - const tools = body.tools.filter(t => { const type = isPlainObject(t) && typeof t.type === "string" ? t.type : undefined; - if (!type) return true; - return !unsupported.some(e => e.tools.has(type)); + return !type || !isHostedToolUnsupportedForModel(model, type); }); return tools.length === body.tools.length ? body : { ...body, tools }; } @@ -1025,8 +1118,15 @@ export function createResponsesPassthroughAdapter(provider: OcxProviderConfig): } if (forward) { outBody = stripUnsupportedForwardParams(outBody); + } else { + outBody = preferConfiguredHostedTools( + outBody, + provider, + parsed.modelId, + parsed._openAiVirtualSelectedModelId, + ); + outBody = normalizeImageGenClientTools(outBody); } - else outBody = normalizeImageGenClientTools(outBody); if (forward || parsed._previousResponseInputExpanded === true) { outBody = repairOversizedReplayCallIds(outBody); } diff --git a/src/config.ts b/src/config.ts index 05f8547005..501e9dd2f9 100644 --- a/src/config.ts +++ b/src/config.ts @@ -28,6 +28,7 @@ import { isWirePinnedModel, MODEL_ADAPTER_OVERRIDE_ALLOWED, OPENAI_PROVIDER_TIER_VERSION, + pinnedWireAdapter, REASONING_SUMMARY_DELIVERY_VALUES, type OcxClaudeCodeConfig, type OcxConfig, @@ -35,6 +36,8 @@ import { type OcxProviderConfig, } from "./types"; import { isCanonicalOpenAiForwardProvider, OPENAI_CODEX_PROVIDER_ID } from "./providers/openai-tiers"; +import { getProviderRegistryEntry } from "./providers/registry"; +import { resolveOpenAiVirtualModel } from "./providers/openai-virtual-models"; import { parseDesktopProfile } from "./claude/desktop-profile"; import { isCodexReasoningEffort, modelRecordValue } from "./reasoning-effort"; import { @@ -42,6 +45,7 @@ import { MAX_APP_OWNED_MEMORY_BUDGET_MB, MIN_APP_OWNED_MEMORY_BUDGET_MB, } from "./lib/app-owned-memory"; +import { isHostedToolUnsupportedForModel } from "./responses/hosted-tool-policy"; let _atomicSeq = 0; @@ -702,6 +706,60 @@ export function reasoningSummaryDeliveryRecordConfigError( return null; } +const SUPPORTED_PREFERRED_HOSTED_TOOLS = new Set(["image_generation"]); + +export function modelPreferHostedToolsConfigError( + value: unknown, + field: string, + providerName: string, + provider: { adapter?: unknown; authMode?: unknown; modelAdapters?: unknown }, +): string | null { + if (value === undefined) return null; + if (!value || typeof value !== "object" || Array.isArray(value)) return `${field} must be a plain object`; + const prototype = Object.getPrototypeOf(value); + if (prototype !== Object.prototype && prototype !== null) return `${field} must be a plain object with own properties`; + const entries = Object.entries(value); + const registry = getProviderRegistryEntry(providerName); + if (entries.length > 0 && (registry?.authKind === "forward" || (!registry && provider.authMode === "forward"))) { + return `${field} is not supported on forward-auth Responses providers`; + } + const requestedWireFor = (modelId: string): unknown => provider.modelAdapters + && typeof provider.modelAdapters === "object" + && !Array.isArray(provider.modelAdapters) + ? (provider.modelAdapters as Record)[modelId] + : undefined; + const resolveEffectiveWire = (modelId: string, currentWire: unknown): unknown => { + const pinned = pinnedWireAdapter(providerName, modelId); + if (pinned) return pinned; + const requestedWire = requestedWireFor(modelId); + return typeof requestedWire === "string" && MODEL_ADAPTER_OVERRIDE_ALLOWED.has(requestedWire) + ? requestedWire + : currentWire; + }; + for (const [key, entry] of entries) { + if (!key.trim()) return `${field} keys must be nonblank model ids`; + if (!Array.isArray(entry)) return `${field}.${key} must be an array`; + if (entry.length === 0) return `${field}.${key} must include image_generation`; + for (const tool of entry) { + if (typeof tool !== "string" || !SUPPORTED_PREFERRED_HOSTED_TOOLS.has(tool)) { + return `${field}.${key} supports only image_generation`; + } + if (isHostedToolUnsupportedForModel(key, tool)) { + return `${field}.${key} cannot prefer ${tool}: the model does not support it`; + } + } + let effectiveWire = resolveEffectiveWire(key, registry?.adapter ?? provider.adapter); + const virtualWireModel = resolveOpenAiVirtualModel(providerName, key)?.wireModelId; + if (virtualWireModel && virtualWireModel !== key) { + effectiveWire = resolveEffectiveWire(virtualWireModel, effectiveWire); + } + if (effectiveWire !== "openai-responses") { + return `${field}.${key} requires the openai-responses wire`; + } + } + return null; +} + /** * Validate a provider's per-model wire override map (#404). * @@ -1006,6 +1064,19 @@ const configSchema = z.object({ message: modelAdaptersError, }); } + const preferHostedToolsError = modelPreferHostedToolsConfigError( + (provider as { modelPreferHostedTools?: unknown }).modelPreferHostedTools, + "modelPreferHostedTools", + name, + provider, + ); + if (preferHostedToolsError) { + ctx.addIssue({ + code: "custom", + path: ["providers", name, "modelPreferHostedTools"], + message: preferHostedToolsError, + }); + } const maxInputError = positiveIntegerRecordConfigError( (provider as { modelMaxInputTokens?: unknown }).modelMaxInputTokens, "modelMaxInputTokens", diff --git a/src/providers/openai-virtual-models.ts b/src/providers/openai-virtual-models.ts index 791f9124b8..ea05b4d3db 100644 --- a/src/providers/openai-virtual-models.ts +++ b/src/providers/openai-virtual-models.ts @@ -62,6 +62,7 @@ export function applyOpenAiVirtualModel( logCtx.resolvedModel = resolution.wireModelId; route.modelId = resolution.wireModelId; parsed.modelId = resolution.wireModelId; + parsed._openAiVirtualSelectedModelId = resolution.selectedModelId; if (parsed._rawBody && typeof parsed._rawBody === "object" && !Array.isArray(parsed._rawBody)) { const raw = parsed._rawBody as Record; diff --git a/src/responses/hosted-tool-policy.ts b/src/responses/hosted-tool-policy.ts new file mode 100644 index 0000000000..17607a873f --- /dev/null +++ b/src/responses/hosted-tool-policy.ts @@ -0,0 +1,9 @@ +/** Hosted tools rejected by specific native model slugs. */ +const UNSUPPORTED_HOSTED_TOOLS: ReadonlyArray<{ match: (model: string) => boolean; tools: ReadonlySet }> = [ + { match: model => model.includes("codex-spark"), tools: new Set(["image_generation", "tool_search"]) }, +]; + +/** True when forwarding this hosted tool to the model would be rejected upstream. */ +export function isHostedToolUnsupportedForModel(modelId: string, tool: string): boolean { + return UNSUPPORTED_HOSTED_TOOLS.some(entry => entry.match(modelId) && entry.tools.has(tool)); +} diff --git a/src/server/auth-cors.ts b/src/server/auth-cors.ts index d6baef6cfa..79b02c3292 100644 --- a/src/server/auth-cors.ts +++ b/src/server/auth-cors.ts @@ -4,6 +4,7 @@ import { apiKeyTransportConfigError, booleanRecordConfigError, modelAdapterRecordConfigError, + modelPreferHostedToolsConfigError, codexAutoStartEnabled, positiveIntegerConfigError, positiveIntegerRecordConfigError, @@ -432,6 +433,13 @@ export function providerManagementConfigError(name: unknown, provider: unknown): if (reasoningSummaryDeliveryError) return `provider ${name} ${reasoningSummaryDeliveryError}`; const modelAdaptersError = modelAdapterRecordConfigError(raw.modelAdapters, "modelAdapters", name, typed); if (modelAdaptersError) return `provider ${name} ${modelAdaptersError}`; + const preferHostedToolsError = modelPreferHostedToolsConfigError( + raw.modelPreferHostedTools, + "modelPreferHostedTools", + name, + typed, + ); + if (preferHostedToolsError) return `provider ${name} ${preferHostedToolsError}`; const defaultMaxOutputError = positiveIntegerConfigError(raw.defaultMaxOutputTokens, "defaultMaxOutputTokens"); if (defaultMaxOutputError) return `provider ${name} ${defaultMaxOutputError}`; const maxOutputError = positiveIntegerRecordConfigError(raw.modelMaxOutputTokens, "modelMaxOutputTokens"); diff --git a/src/types.ts b/src/types.ts index bd5f0ae1ef..0bbdc7e416 100644 --- a/src/types.ts +++ b/src/types.ts @@ -2,6 +2,8 @@ import type { KiroOAuthMetadata } from "./oauth/types"; export interface OcxParsedRequest { modelId: string; + /** Selected OpenAI API virtual-model id retained after it rewrites the upstream wire model. */ + _openAiVirtualSelectedModelId?: string; previousResponseId?: string; context: OcxContext; stream: boolean; @@ -1065,6 +1067,11 @@ export interface OcxProviderConfig { * Presence also advertises reasoning-summary support for that routed model. */ modelReasoningSummaryDelivery?: Record; + /** + * Exact-model hosted tools that win collisions with Codex client tool declarations. + * Use for non-forward Responses gateways that reserve a hosted tool namespace server-side. + */ + modelPreferHostedTools?: Record; /** Provider-wide mapping from Codex effort labels to upstream `reasoning_effort` values. */ reasoningEffortMap?: Record; /** Model-specific mapping from Codex effort labels to upstream `reasoning_effort` values. */ diff --git a/structure/04_transports-and-sidecars.md b/structure/04_transports-and-sidecars.md index 07be029480..b58505267e 100644 --- a/structure/04_transports-and-sidecars.md +++ b/structure/04_transports-and-sidecars.md @@ -97,6 +97,17 @@ hosted `image_generation` and deduplicates aliases in stable container order. Em namespaces do not remove the hosted fallback. Discovery and normalization span both top-level `body.tools` and Codex Desktop Responses Lite `input[].type = "additional_tools"` containers. +For a model explicitly listed in `modelPreferHostedTools`, a non-forward Responses provider may opt +to remove colliding client `image_gen` declarations before this normalization and rewrite their +selectors to hosted `image_generation`, so a provider-reserved hosted tool takes precedence without +loosening a caller's tool-choice restriction. The opt-in is intentionally model-scoped: the default +alias path remains safest for ordinary public Responses endpoints. + +For OpenAI API virtual `-pro` models, preference lookup checks the selected public ID first and +uses the resolved base wire-model ID as a fallback. `modelAdapters` resolves the public ID first and +the base ID second; the second pass selects the final adapter, and configuration validation mirrors +both steps. + Client-facing API-key responses perform the inverse mapping: JSON output and SSE function-call items restore `{ namespace: "image_gen", name: "" }` so Codex can dispatch the local extension. When item-id repair is also enabled, both transforms compose in one SSE parse/stringify diff --git a/tests/config.test.ts b/tests/config.test.ts index 30d63fd156..3977208b48 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -870,6 +870,20 @@ describe("opencodex config defaults", () => { }); expect(readConfigDiagnostics().error).toBeNull(); + writeConfig({ + port: 12345, + providers: { + custom: { + adapter: "openai-chat", + baseUrl: "https://example.test/v1", + modelAdapters: { "provider-image-model": "openai-responses" }, + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + }, + }, + defaultProvider: "custom", + }); + expect(readConfigDiagnostics().error).toBeNull(); + for (const provider of [ { adapter: "openai-chat", baseUrl: "https://gateway.example/v1", authMode: "key", apiKeyTransport: "bearer" }, { adapter: "anthropic", baseUrl: "https://gateway.example/v1", authMode: "oauth", apiKeyTransport: "bearer" }, @@ -1003,6 +1017,144 @@ describe("opencodex config defaults", () => { expect(readConfigDiagnostics().error).toContain("conflicts with modelSupportsReasoningSummaries=false"); }); + test("modelPreferHostedTools accepts only supported hosted-tool arrays", () => { + writeConfig({ + port: 12345, + providers: { + custom: { + adapter: "openai-responses", + baseUrl: "https://example.test/v1", + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + }, + }, + defaultProvider: "custom", + }); + expect(readConfigDiagnostics().error).toBeNull(); + + // Registry providers route through their registry wire, not this persisted adapter. + writeConfig({ + port: 12345, + providers: { + "openai-apikey": { + adapter: "openai-chat", + baseUrl: "https://api.openai.com/v1", + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + }, + }, + defaultProvider: "openai-apikey", + }); + expect(readConfigDiagnostics().error).toBeNull(); + + writeConfig({ + port: 12345, + providers: { + "openai-apikey": { + adapter: "openai-responses", + baseUrl: "https://api.openai.com/v1", + modelAdapters: { "gpt-5.6-sol": "openai-chat" }, + modelPreferHostedTools: { "gpt-5.6-sol-pro": ["image_generation"] }, + }, + }, + defaultProvider: "openai-apikey", + }); + expect(readConfigDiagnostics().source).toBe("fallback"); + expect(readConfigDiagnostics().error).toContain("requires the openai-responses wire"); + + for (const modelPreferHostedTools of [ + [], + { "": ["image_generation"] }, + { model: [] }, + { model: "image_generation" }, + { model: ["web_search"] }, + ]) { + writeConfig({ + port: 12345, + providers: { + custom: { + adapter: "openai-responses", + baseUrl: "https://example.test/v1", + modelPreferHostedTools, + }, + }, + defaultProvider: "custom", + }); + expect(readConfigDiagnostics().source).toBe("fallback"); + expect(readConfigDiagnostics().error).toContain("modelPreferHostedTools"); + } + + writeConfig({ + port: 12345, + providers: { + custom: { + adapter: "openai-chat", + baseUrl: "https://example.test/v1", + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + }, + }, + defaultProvider: "custom", + }); + expect(readConfigDiagnostics().source).toBe("fallback"); + expect(readConfigDiagnostics().error).toContain("requires the openai-responses wire"); + + writeConfig({ + port: 12345, + providers: { + openrouter: { + adapter: "openai-responses", + baseUrl: "https://openrouter.ai/api/v1", + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + }, + }, + defaultProvider: "openrouter", + }); + expect(readConfigDiagnostics().source).toBe("fallback"); + expect(readConfigDiagnostics().error).toContain("requires the openai-responses wire"); + + writeConfig({ + port: 12345, + providers: { + custom: { + adapter: "openai-responses", + baseUrl: "https://example.test/v1", + modelAdapters: { "provider-image-model": "openai-chat" }, + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + }, + }, + defaultProvider: "custom", + }); + expect(readConfigDiagnostics().source).toBe("fallback"); + expect(readConfigDiagnostics().error).toContain("requires the openai-responses wire"); + + writeConfig({ + port: 12345, + providers: { + custom: { + adapter: "openai-responses", + baseUrl: "https://example.test/v1", + modelPreferHostedTools: { "gpt-5.3-codex-spark": ["image_generation"] }, + }, + }, + defaultProvider: "custom", + }); + expect(readConfigDiagnostics().source).toBe("fallback"); + expect(readConfigDiagnostics().error).toContain("does not support"); + + writeConfig({ + port: 12345, + providers: { + openai: { + adapter: "openai-responses", + authMode: "forward", + baseUrl: "https://chatgpt.com/backend-api/codex", + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + }, + }, + defaultProvider: "openai", + }); + expect(readConfigDiagnostics().source).toBe("fallback"); + expect(readConfigDiagnostics().error).toContain("not supported on forward-auth"); + }); + test("modelAdapters accepts only allowed wires on eligible providers (#404)", () => { writeConfig({ port: 12345, diff --git a/tests/management-provider-validation.test.ts b/tests/management-provider-validation.test.ts index 6cf06b16b9..7f4d9e67eb 100644 --- a/tests/management-provider-validation.test.ts +++ b/tests/management-provider-validation.test.ts @@ -29,6 +29,7 @@ import { startServer, } from "../src/server"; import { handleManagementAPI } from "../src/server/management-api"; +import { providerManagementConfigError } from "../src/server/auth-cors"; import { clearModelCache, markProviderDiscoveryFailed } from "../src/codex/model-cache"; import type { OcxConfig } from "../src/types"; import { fakeChatGptJwt } from "./helpers/fake-chatgpt-jwt"; @@ -123,6 +124,74 @@ afterEach(() => { }); describe("provider management validation", () => { + test("provider management validates model hosted-tool preferences", () => { + const provider = { + adapter: "openai-responses", + baseUrl: "https://api.openai.com/v1", + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + }; + expect(providerManagementConfigError("custom", provider)).toBeNull(); + expect(providerManagementConfigError("custom", { + adapter: "openai-chat", + baseUrl: "https://api.openai.com/v1", + modelAdapters: { "provider-image-model": "openai-responses" }, + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + })).toBeNull(); + expect(providerManagementConfigError("openai-apikey", { + adapter: "openai-chat", + baseUrl: "https://api.openai.com/v1", + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + })).toBeNull(); + expect(providerManagementConfigError("openai-apikey", { + adapter: "openai-responses", + baseUrl: "https://api.openai.com/v1", + modelAdapters: { "gpt-5.6-sol": "openai-chat" }, + modelPreferHostedTools: { "gpt-5.6-sol-pro": ["image_generation"] }, + })).toContain("requires the openai-responses wire"); + + for (const modelPreferHostedTools of [ + [], + { "": ["image_generation"] }, + { model: [] }, + { model: "image_generation" }, + { model: ["web_search"] }, + ]) { + expect(providerManagementConfigError("custom", { + adapter: "openai-responses", + baseUrl: "https://api.openai.com/v1", + modelPreferHostedTools, + })).toContain("modelPreferHostedTools"); + } + + expect(providerManagementConfigError("custom", { + adapter: "openai-chat", + baseUrl: "https://api.openai.com/v1", + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + })).toContain("requires the openai-responses wire"); + expect(providerManagementConfigError("openrouter", { + adapter: "openai-responses", + baseUrl: "https://openrouter.ai/api/v1", + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + })).toContain("requires the openai-responses wire"); + expect(providerManagementConfigError("custom", { + adapter: "openai-responses", + baseUrl: "https://api.openai.com/v1", + modelAdapters: { "provider-image-model": "openai-chat" }, + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + })).toContain("requires the openai-responses wire"); + expect(providerManagementConfigError("custom", { + adapter: "openai-responses", + baseUrl: "https://api.openai.com/v1", + modelPreferHostedTools: { "gpt-5.3-codex-spark": ["image_generation"] }, + })).toContain("does not support"); + expect(providerManagementConfigError("custom-forward", { + adapter: "openai-responses", + baseUrl: "https://chatgpt.com/backend-api/codex", + authMode: "forward", + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + })).toContain("not supported on forward-auth"); + }); + test("provider discovery status is additive and omitted before an attempt", async () => { markProviderDiscoveryFailed("auth-broken", { reason: "http", httpStatus: 401 }); try { diff --git a/tests/openai-api-virtual-models.test.ts b/tests/openai-api-virtual-models.test.ts index 56a4037086..b148463871 100644 --- a/tests/openai-api-virtual-models.test.ts +++ b/tests/openai-api-virtual-models.test.ts @@ -12,6 +12,7 @@ import { validateOpenAiVirtualModelDefinition, } from "../src/providers/openai-virtual-models"; import { PROVIDER_REGISTRY } from "../src/providers/registry"; +import { resolveWireProtocolOverride } from "../src/server/adapter-resolve"; import { saveConfig } from "../src/config"; import { startServer } from "../src/server"; import { usageLogPath } from "../src/usage/log"; @@ -66,6 +67,17 @@ describe("OpenAI API virtual model resolution", () => { }); describe("applyOpenAiVirtualModel", () => { + test("resolves a base-model wire override after rewriting a Pro alias", () => { + const selectedWire = resolveWireProtocolOverride("openai-apikey", "gpt-5.6-sol-pro", { + adapter: "openai-responses", + modelAdapters: { "gpt-5.6-sol": "openai-chat" }, + } as any); + const wireModel = resolveWireProtocolOverride("openai-apikey", "gpt-5.6-sol", selectedWire); + + expect(selectedWire.adapter).toBe("openai-responses"); + expect(wireModel.adapter).toBe("openai-chat"); + }); + test("rewrites Pro request: model to base, merges reasoning.mode=pro, preserves effort", () => { const parsed = { modelId: "gpt-5.6-sol-pro", @@ -76,6 +88,7 @@ describe("applyOpenAiVirtualModel", () => { const logCtx = { model: "gpt-5.6-sol-pro", provider: "openai-apikey" } as any; applyOpenAiVirtualModel(parsed, route, logCtx); expect(parsed.modelId).toBe("gpt-5.6-sol"); + expect(parsed._openAiVirtualSelectedModelId).toBe("gpt-5.6-sol-pro"); expect(parsed._rawBody.model).toBe("gpt-5.6-sol"); expect(parsed._rawBody.reasoning).toEqual({ effort: "high", mode: "pro" }); expect(route.modelId).toBe("gpt-5.6-sol"); diff --git a/tests/openai-responses-passthrough.test.ts b/tests/openai-responses-passthrough.test.ts index 5566c4e887..d3a52e0161 100644 --- a/tests/openai-responses-passthrough.test.ts +++ b/tests/openai-responses-passthrough.test.ts @@ -951,6 +951,7 @@ describe("OpenAI Responses hosted-tool name conflicts", () => { const body = JSON.parse(request.body) as { tools: Array<{ type: string }>; input: Array<{ type: string; tools?: Array<{ type: string; name?: string }> }>; + tool_choice?: { type: string; name?: string }; }; const additionalTools = body.input.find(item => item.type === "additional_tools"); @@ -1085,6 +1086,360 @@ describe("OpenAI Responses hosted-tool name conflicts", () => { expect(body.tool_choice).toEqual({ type: "function", name: "image_gen.imagegen" }); }); + test("configured model removes an empty image_gen namespace and preserves hosted image generation", () => { + const adapter = createResponsesPassthroughAdapter({ + ...keyedProvider, + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + }); + const request = adapter.buildRequest({ + modelId: "provider-image-model", + context: { messages: [] }, + stream: true, + options: {}, + _rawBody: { + model: "provider-image-model", + tools: [{ type: "image_generation" }], + input: [{ + type: "additional_tools", + role: "developer", + tools: [ + { type: "namespace", name: "image_gen", tools: [] }, + { type: "web_search" }, + ], + }], + tool_choice: { type: "function", name: "image_gen.imagegen" }, + }, + }, meta); + const body = JSON.parse(request.body) as { + tools: Array<{ type: string }>; + input: Array<{ type: string; tools?: Array<{ type: string; name?: string }> }>; + }; + const additionalTools = body.input.find(item => item.type === "additional_tools"); + + expect(body.tools).toEqual([{ type: "image_generation" }]); + expect(additionalTools?.tools).toEqual([{ type: "web_search" }]); + expect(body.tool_choice).toEqual({ type: "image_generation" }); + }); + + test("configured model rewrites a custom image-gen selector", () => { + const adapter = createResponsesPassthroughAdapter({ + ...keyedProvider, + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + }); + const request = adapter.buildRequest({ + modelId: "provider-image-model", + context: { messages: [] }, + stream: true, + options: {}, + _rawBody: { + model: "provider-image-model", + input: [], + tools: [ + { type: "custom", name: "image_gen.render" }, + { type: "image_generation" }, + ], + tool_choice: { type: "custom", name: "image_gen.render" }, + }, + }, meta); + const body = JSON.parse(request.body) as { + tools: Array<{ type: string }>; + tool_choice: { type: string }; + }; + + expect(body.tools).toEqual([{ type: "image_generation" }]); + expect(body.tool_choice).toEqual({ type: "image_generation" }); + }); + + test("configured model retains a hosted declaration for a wrapper-only selector", () => { + const adapter = createResponsesPassthroughAdapter({ + ...keyedProvider, + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + }); + const request = adapter.buildRequest({ + modelId: "provider-image-model", + context: { messages: [] }, + stream: true, + options: {}, + _rawBody: { + model: "provider-image-model", + input: [], + tools: [{ type: "namespace", name: "image_gen", tools: [] }], + tool_choice: { type: "function", name: "image_gen.imagegen" }, + }, + }, meta); + const body = JSON.parse(request.body) as { + tools: Array<{ type: string }>; + tool_choice: { type: string }; + }; + + expect(body.tools).toEqual([{ type: "image_generation" }]); + expect(body.tool_choice).toEqual({ type: "image_generation" }); + }); + + test("configured model retains hosted image generation without a forced selector", () => { + const adapter = createResponsesPassthroughAdapter({ + ...keyedProvider, + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + }); + + for (const toolChoice of ["auto", "none", undefined] as const) { + const request = adapter.buildRequest({ + modelId: "provider-image-model", + context: { messages: [] }, + stream: true, + options: {}, + _rawBody: { + model: "provider-image-model", + input: [], + tools: [{ type: "namespace", name: "image_gen", tools: [] }], + ...(toolChoice === undefined ? {} : { tool_choice: toolChoice }), + }, + }, meta); + const body = JSON.parse(request.body) as { + tools: Array<{ type: string }>; + tool_choice?: string; + }; + + expect(body.tools).toEqual([{ type: "image_generation" }]); + expect(body.tool_choice).toBe(toolChoice); + } + }); + + test("configured model retains a hosted declaration in wrapper-only additional tools", () => { + const adapter = createResponsesPassthroughAdapter({ + ...keyedProvider, + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + }); + const request = adapter.buildRequest({ + modelId: "provider-image-model", + context: { messages: [] }, + stream: true, + options: {}, + _rawBody: { + model: "provider-image-model", + tools: [], + input: [{ + type: "additional_tools", + role: "developer", + tools: [{ type: "namespace", name: "image_gen", tools: [] }], + }], + tool_choice: { type: "function", name: "image_gen.imagegen" }, + }, + }, meta); + const body = JSON.parse(request.body) as { + input: Array<{ type: string; tools?: Array<{ type: string }> }>; + tool_choice: { type: string }; + }; + const additionalTools = body.input.find(item => item.type === "additional_tools"); + + expect(additionalTools?.tools).toEqual([{ type: "image_generation" }]); + expect(body.tool_choice).toEqual({ type: "image_generation" }); + }); + + test("configured model restores hosted image generation in unforced additional tools", () => { + const adapter = createResponsesPassthroughAdapter({ + ...keyedProvider, + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + }); + const request = adapter.buildRequest({ + modelId: "provider-image-model", + context: { messages: [] }, + stream: true, + options: {}, + _rawBody: { + model: "provider-image-model", + tools: [], + input: [{ + type: "additional_tools", + role: "developer", + tools: [{ type: "namespace", name: "image_gen", tools: [] }], + }], + tool_choice: "auto", + }, + }, meta); + const body = JSON.parse(request.body) as { + input: Array<{ type: string; tools?: Array<{ type: string }> }>; + tool_choice: string; + }; + const additionalTools = body.input.find(item => item.type === "additional_tools"); + + expect(additionalTools?.tools).toEqual([{ type: "image_generation" }]); + expect(body.tool_choice).toBe("auto"); + }); + + test("configured model retains unrelated allowed-tools selector entries", () => { + const adapter = createResponsesPassthroughAdapter({ + ...keyedProvider, + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + }); + const request = adapter.buildRequest({ + modelId: "provider-image-model", + context: { messages: [] }, + stream: true, + options: {}, + _rawBody: { + model: "provider-image-model", + input: [], + tools: [ + { type: "namespace", name: "image_gen", tools: [] }, + { type: "image_generation" }, + { type: "function", name: "exec_command", parameters: {} }, + ], + tool_choice: { + type: "allowed_tools", + mode: "required", + tools: [ + { type: "function", name: "image_gen.imagegen" }, + { type: "function", name: "exec_command" }, + ], + }, + }, + }, meta); + const body = JSON.parse(request.body) as { + tools: Array<{ type: string; name?: string }>; + tool_choice?: { type: string; mode: string; tools: Array<{ type: string; name?: string }> }; + }; + + expect(body.tools).toEqual([ + { type: "image_generation" }, + { type: "function", name: "exec_command", parameters: {} }, + ]); + expect(body.tool_choice).toEqual({ + type: "allowed_tools", + mode: "required", + tools: [ + { type: "image_generation" }, + { type: "function", name: "exec_command" }, + ], + }); + }); + + test("configured model rewrites custom image-gen entries in an allowed-tools selector", () => { + const adapter = createResponsesPassthroughAdapter({ + ...keyedProvider, + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + }); + const request = adapter.buildRequest({ + modelId: "provider-image-model", + context: { messages: [] }, + stream: true, + options: {}, + _rawBody: { + model: "provider-image-model", + input: [], + tools: [ + { type: "custom", name: "image_gen.render" }, + { type: "image_generation" }, + { type: "custom", name: "exec_command" }, + ], + tool_choice: { + type: "allowed_tools", + mode: "required", + tools: [ + { type: "custom", name: "image_gen.render" }, + { type: "custom", name: "exec_command" }, + ], + }, + }, + }, meta); + const body = JSON.parse(request.body) as { + tools: Array<{ type: string; name?: string }>; + tool_choice: { type: string; mode: string; tools: Array<{ type: string; name?: string }> }; + }; + + expect(body.tools).toEqual([ + { type: "image_generation" }, + { type: "custom", name: "exec_command" }, + ]); + expect(body.tool_choice).toEqual({ + type: "allowed_tools", + mode: "required", + tools: [ + { type: "image_generation" }, + { type: "custom", name: "exec_command" }, + ], + }); + }); + + test("hosted-tool preference stays scoped to its configured model", () => { + const adapter = createResponsesPassthroughAdapter({ + ...keyedProvider, + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + }); + const request = adapter.buildRequest({ + modelId: "other-model", + context: { messages: [] }, + stream: true, + options: {}, + _rawBody: { + model: "other-model", + input: [], + tools: [ + { type: "namespace", name: "image_gen", tools: [] }, + { type: "image_generation" }, + ], + }, + }, meta); + const body = JSON.parse(request.body) as { tools: Array> }; + + expect(body.tools).toEqual([ + { type: "namespace", name: "image_gen", tools: [] }, + { type: "image_generation" }, + ]); + }); + + test("hosted-tool preference uses the exact model id", () => { + const adapter = createResponsesPassthroughAdapter({ + ...keyedProvider, + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + }); + const request = adapter.buildRequest({ + modelId: "provider-image-model:variant", + context: { messages: [] }, + stream: true, + options: {}, + _rawBody: { + model: "provider-image-model:variant", + input: [], + tools: [ + { type: "namespace", name: "image_gen", tools: [] }, + { type: "image_generation" }, + ], + }, + }, meta); + const body = JSON.parse(request.body) as { tools: Array> }; + + expect(body.tools).toEqual([ + { type: "namespace", name: "image_gen", tools: [] }, + { type: "image_generation" }, + ]); + }); + + test("hosted-tool preference honors an OpenAI virtual model's selected id", () => { + const adapter = createResponsesPassthroughAdapter({ + ...keyedProvider, + modelPreferHostedTools: { "gpt-5.6-sol-pro": ["image_generation"] }, + }); + const request = adapter.buildRequest({ + modelId: "gpt-5.6-sol", + _openAiVirtualSelectedModelId: "gpt-5.6-sol-pro", + context: { messages: [] }, + stream: true, + options: {}, + _rawBody: { + model: "gpt-5.6-sol", + input: [], + tools: [ + { type: "namespace", name: "image_gen", tools: [] }, + { type: "image_generation" }, + ], + }, + }, meta); + const body = JSON.parse(request.body) as { tools: Array> }; + + expect(body.tools).toEqual([{ type: "image_generation" }]); + }); + test("keyed platform preserves hosted image_generation for replay-only image-gen calls", () => { const adapter = createResponsesPassthroughAdapter(keyedProvider); const request = adapter.buildRequest({ From b3329edf03f5bcc2ed80f90edcdc81663b09fd05 Mon Sep 17 00:00:00 2001 From: Ingwannu Date: Sat, 1 Aug 2026 04:58:49 +0000 Subject: [PATCH 2/7] test: align hosted-tool fixture with schema normalization --- tests/openai-responses-passthrough.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/openai-responses-passthrough.test.ts b/tests/openai-responses-passthrough.test.ts index d3a52e0161..d6179a38ed 100644 --- a/tests/openai-responses-passthrough.test.ts +++ b/tests/openai-responses-passthrough.test.ts @@ -1302,7 +1302,7 @@ describe("OpenAI Responses hosted-tool name conflicts", () => { expect(body.tools).toEqual([ { type: "image_generation" }, - { type: "function", name: "exec_command", parameters: {} }, + { type: "function", name: "exec_command", parameters: { type: "object" } }, ]); expect(body.tool_choice).toEqual({ type: "allowed_tools", From 6f8b1d64e12757d24d855cbca834c05cb7d4f9dd Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Mon, 3 Aug 2026 13:59:39 +0900 Subject: [PATCH 3/7] fix(config): honor registry wire defaults and own-property lookup for hosted-tool preferences MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects the plan audit found in this change, both reproduced before fixing. Validation disagreed with routing. `modelPreferHostedTools` resolved the effective wire from `registry.adapter` and an explicit `modelAdapters` entry, then stopped. At request time `resolveModelAdapter()` consults the registry's per-model `modelWireDefaults` before falling back to the provider-wide adapter. DeepSeek routes `deepseek-v4-flash` over native Responses for a Responses inbound while its provider-wide wire stays openai-chat, so a valid preference for that model was rejected at config load with "requires the openai-responses wire" — a config the runtime would have honored. The validator now walks the same order. Inherited keys threw before dispatch. `provider.modelPreferHostedTools?. [modelId]` walks the prototype chain, so a routed model id of `constructor` or `toString` resolved to a function and threw `TypeError: ... .includes is not a function` inside `preferConfiguredHostedTools`, failing the request. Lookup is now own-property only and requires an array. Both regressions were driven red first: the DeepSeek case produced the exact rejection above, and the inherited-key case produced the exact TypeError. 247 tests pass across the four affected files, typecheck clean. --- src/adapters/openai-responses.ts | 12 +++++++-- src/config.ts | 31 ++++++++++++++++++---- tests/config.test.ts | 19 +++++++++++++ tests/openai-responses-passthrough.test.ts | 26 ++++++++++++++++++ 4 files changed, 81 insertions(+), 7 deletions(-) diff --git a/src/adapters/openai-responses.ts b/src/adapters/openai-responses.ts index 937ca597b4..c8a75b709a 100644 --- a/src/adapters/openai-responses.ts +++ b/src/adapters/openai-responses.ts @@ -651,8 +651,16 @@ function preferConfiguredHostedTools( selectedModelId?: string, ): unknown { // A virtual model's advertised id takes precedence over its resolved wire-model id. - const preferredTools = (selectedModelId ? provider.modelPreferHostedTools?.[selectedModelId] : undefined) - ?? provider.modelPreferHostedTools?.[modelId]; + // Read own properties only: a routed model id of `constructor`/`toString` would + // otherwise resolve to an inherited Object.prototype function and throw on the + // membership test below, failing the request before it is dispatched. + const preferenceMap = provider.modelPreferHostedTools; + const ownPreference = (key: string | undefined): string[] | undefined => { + if (!key || !preferenceMap || !Object.prototype.hasOwnProperty.call(preferenceMap, key)) return undefined; + const entry = preferenceMap[key]; + return Array.isArray(entry) ? entry : undefined; + }; + const preferredTools = ownPreference(selectedModelId) ?? ownPreference(modelId); if (!preferredTools?.includes(HOSTED_IMAGE_GENERATION_TOOL) || !isPlainObject(body)) return body; const stripGroup = (tools: unknown[]): unknown[] => { diff --git a/src/config.ts b/src/config.ts index 501e9dd2f9..4aef1a4d33 100644 --- a/src/config.ts +++ b/src/config.ts @@ -36,7 +36,7 @@ import { type OcxProviderConfig, } from "./types"; import { isCanonicalOpenAiForwardProvider, OPENAI_CODEX_PROVIDER_ID } from "./providers/openai-tiers"; -import { getProviderRegistryEntry } from "./providers/registry"; +import { getProviderRegistryEntry, providerModelWireDefault } from "./providers/registry"; import { resolveOpenAiVirtualModel } from "./providers/openai-virtual-models"; import { parseDesktopProfile } from "./claude/desktop-profile"; import { isCodexReasoningEffort, modelRecordValue } from "./reasoning-effort"; @@ -712,7 +712,7 @@ export function modelPreferHostedToolsConfigError( value: unknown, field: string, providerName: string, - provider: { adapter?: unknown; authMode?: unknown; modelAdapters?: unknown }, + provider: { adapter?: unknown; authMode?: unknown; modelAdapters?: unknown; baseUrl?: unknown }, ): string | null { if (value === undefined) return null; if (!value || typeof value !== "object" || Array.isArray(value)) return `${field} must be a plain object`; @@ -732,9 +732,30 @@ export function modelPreferHostedToolsConfigError( const pinned = pinnedWireAdapter(providerName, modelId); if (pinned) return pinned; const requestedWire = requestedWireFor(modelId); - return typeof requestedWire === "string" && MODEL_ADAPTER_OVERRIDE_ALLOWED.has(requestedWire) - ? requestedWire - : currentWire; + if (typeof requestedWire === "string" && MODEL_ADAPTER_OVERRIDE_ALLOWED.has(requestedWire)) { + return requestedWire; + } + // No explicit override: fall back to the registry's per-model wire default before + // the provider-wide adapter, because that is the order `resolveModelAdapter()` + // uses at request time (src/server/adapter-resolve.ts:38-48). Skipping it rejected + // preferences the runtime would have honored — DeepSeek routes `deepseek-v4-flash` + // over native Responses for a Responses inbound while the provider-wide wire stays + // openai-chat. Hosted-tool preferences only apply to Responses traffic, so the + // inbound to ask about is "responses". + const registryDefault = typeof currentWire === "string" && typeof provider.baseUrl === "string" + ? providerModelWireDefault( + providerName, + { + baseUrl: provider.baseUrl, + adapter: currentWire, + ...(typeof provider.authMode === "string" ? { authMode: provider.authMode as OcxProviderConfig["authMode"] } : {}), + }, + modelId, + MODEL_ADAPTER_OVERRIDE_ALLOWED, + "responses", + ) + : undefined; + return registryDefault ?? currentWire; }; for (const [key, entry] of entries) { if (!key.trim()) return `${field} keys must be nonblank model ids`; diff --git a/tests/config.test.ts b/tests/config.test.ts index 3977208b48..a06d9fd2b0 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -1031,6 +1031,25 @@ describe("opencodex config defaults", () => { }); expect(readConfigDiagnostics().error).toBeNull(); + // A registry `modelWireDefaults` entry that selects openai-responses for a + // Responses inbound must be honored by validation, exactly as the runtime + // honors it. DeepSeek's preset routes `deepseek-v4-flash` over native + // Responses for a Responses inbound while the provider-wide wire stays + // openai-chat; validating from `registry.adapter` alone rejected a + // preference the runtime would have accepted. + writeConfig({ + port: 12345, + providers: { + deepseek: { + adapter: "openai-chat", + baseUrl: "https://api.deepseek.com/v1", + modelPreferHostedTools: { "deepseek-v4-flash": ["image_generation"] }, + }, + }, + defaultProvider: "deepseek", + }); + expect(readConfigDiagnostics().error).toBeNull(); + // Registry providers route through their registry wire, not this persisted adapter. writeConfig({ port: 12345, diff --git a/tests/openai-responses-passthrough.test.ts b/tests/openai-responses-passthrough.test.ts index d6179a38ed..65c1837636 100644 --- a/tests/openai-responses-passthrough.test.ts +++ b/tests/openai-responses-passthrough.test.ts @@ -1121,6 +1121,32 @@ describe("OpenAI Responses hosted-tool name conflicts", () => { expect(body.tool_choice).toEqual({ type: "image_generation" }); }); + test("an inherited Object.prototype key is not read as a preference", () => { + // `provider.modelPreferHostedTools?.[modelId]` walks the prototype chain, so a + // routed model literally named `constructor` or `toString` yielded a function + // and threw on `.includes` before the request was ever dispatched. + const adapter = createResponsesPassthroughAdapter({ + ...keyedProvider, + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + }); + for (const inheritedKey of ["constructor", "toString", "hasOwnProperty"]) { + const request = adapter.buildRequest({ + modelId: inheritedKey, + context: { messages: [] }, + stream: true, + options: {}, + _rawBody: { + model: inheritedKey, + tools: [{ type: "image_generation" }], + tool_choice: { type: "function", name: "image_gen.imagegen" }, + }, + }, meta); + const body = JSON.parse(request.body) as { tool_choice: unknown }; + // Unconfigured model: ordinary normalization applies, the hosted preference does not. + expect(body.tool_choice).toEqual({ type: "function", name: "image_gen.imagegen" }); + } + }); + test("configured model rewrites a custom image-gen selector", () => { const adapter = createResponsesPassthroughAdapter({ ...keyedProvider, From 8cf4e47785c3e9e4b64bcc00f309c937da549f18 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Mon, 3 Aug 2026 14:18:11 +0900 Subject: [PATCH 4/7] test(config): cover the preserved-destination mirror of the wire-defaults case The fix for registry wire defaults has a mirror the first regression did not reach. `volcengine-agent-plan` is a Responses registry row carrying `preserveCustomDestination`, so a config that reuses the id while pointing at a different endpoint keeps its own transport: `providerMatchesRegistryTransport()` returns false and `routedProviderConfig()` preserves the configured `openai-chat` adapter. Validating from `registry.adapter` unconditionally would accept a hosted-tool preference the Responses adapter never sees. Driven red against the pre-fix `src/config.ts` to confirm it is not vacuous. --- tests/config.test.ts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/config.test.ts b/tests/config.test.ts index a06d9fd2b0..e687d3b9d3 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -1050,6 +1050,26 @@ describe("opencodex config defaults", () => { }); expect(readConfigDiagnostics().error).toBeNull(); + // The mirror of the case above. `volcengine-agent-plan` is a Responses registry row + // with `preserveCustomDestination`, so a config reusing that id while pointing at a + // different endpoint keeps its own transport at runtime — + // `providerMatchesRegistryTransport()` returns false and `routedProviderConfig()` + // preserves the configured `openai-chat` adapter. Validating from `registry.adapter` + // unconditionally would accept a preference that the Responses adapter never sees. + writeConfig({ + port: 12345, + providers: { + "volcengine-agent-plan": { + adapter: "openai-chat", + baseUrl: "https://custom.example.test/v1", + modelPreferHostedTools: { "some-model": ["image_generation"] }, + }, + }, + defaultProvider: "volcengine-agent-plan", + }); + expect(readConfigDiagnostics().source).toBe("fallback"); + expect(readConfigDiagnostics().error).toContain("requires the openai-responses wire"); + // Registry providers route through their registry wire, not this persisted adapter. writeConfig({ port: 12345, From bc628a95bbcd6e8069e3a5fcc847cbc7acd65be2 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Mon, 3 Aug 2026 14:18:32 +0900 Subject: [PATCH 5/7] docs(devlog): carry the sweep plan onto the hosted-tool branch --- .../000_plan.md | 47 +++++ .../_plan/260803_pr_issue_sweep/000_plan.md | 199 ++++++++++++++++++ .../010_phase1_image_forwarding.md | 109 ++++++++++ .../020_phase2_hosted_tool_duplicate.md | 136 ++++++++++++ .../030_phase3_compact_alternate.md | 188 +++++++++++++++++ .../040_phase4_backlog_disposition.md | 135 ++++++++++++ .../050_phase5_916_disposition.md | 126 +++++++++++ .../260803_sparse_snapshot_repair/000_plan.md | 38 ++++ .../260803_transport_attribution/000_plan.md | 195 +++++++++++++++++ 9 files changed, 1173 insertions(+) create mode 100644 devlog/_plan/260803_cooldown_recovery_probe/000_plan.md create mode 100644 devlog/_plan/260803_pr_issue_sweep/000_plan.md create mode 100644 devlog/_plan/260803_pr_issue_sweep/010_phase1_image_forwarding.md create mode 100644 devlog/_plan/260803_pr_issue_sweep/020_phase2_hosted_tool_duplicate.md create mode 100644 devlog/_plan/260803_pr_issue_sweep/030_phase3_compact_alternate.md create mode 100644 devlog/_plan/260803_pr_issue_sweep/040_phase4_backlog_disposition.md create mode 100644 devlog/_plan/260803_pr_issue_sweep/050_phase5_916_disposition.md create mode 100644 devlog/_plan/260803_sparse_snapshot_repair/000_plan.md create mode 100644 devlog/_plan/260803_transport_attribution/000_plan.md diff --git a/devlog/_plan/260803_cooldown_recovery_probe/000_plan.md b/devlog/_plan/260803_cooldown_recovery_probe/000_plan.md new file mode 100644 index 0000000000..2277f19ff0 --- /dev/null +++ b/devlog/_plan/260803_cooldown_recovery_probe/000_plan.md @@ -0,0 +1,47 @@ +# 000 — Cooldown recovery probe (#915) + +Reserved unit. Deferred out of `260803_pr_issue_sweep` because the fix is not a +guard on an existing path — it needs a new contract. + +## The defect + +A reset-derived cooldown excludes account A from selection +(`src/codex/routing.ts:734-760`), and the alternate selectors only ever see the +eligible list (`:927-965`). Probe eligibility exists and is correctly limited to +non-`Retry-After` cooldowns with one lease per interval (`:369-423`) — but +`resolveCodexAuthContext()` selects the account first +(`src/codex/auth-context.ts:214-226`) and only then checks that selected +account's lease (`:237-253`). + +So when account B stays eligible, B is always selected, A is never selected, +and A never reaches the lease code that would probe it. A recovers upstream and +the pool never notices. + +A fresh WHAM read does not rescue it: pool results call +`setAccountQuotaFromParsed()` (`src/codex/auth-api.ts:590-626`), which writes +the quota cache (`src/codex/quota.ts:134-179`) and has no authority over +routing cooldown generations. + +## Why it is its own unit + +The fix is a background recovery worker with a claim/settle contract, fenced on +cooldown generation, quota scope, and credential generation — not a tweak to +account selection. It crosses routing state, auth resolution, WHAM refresh +concurrency, account generations, and quota scopes. Bundling that with four +other changes to the same subsystem, in one cycle, is how a subtle +concurrency bug ships. + +Two constraints already established and worth not rediscovering: + +- `clearCodexAccountCooldown()` must **not** be used. It clears every scope and + carries no credential fence. +- The existing per-account quota refresh at `src/codex/auth-api.ts:646-688` is + already generation-aware and single-flight, so it is the right thing to join + rather than duplicate. +- "A 100% WHAM snapshot must never move an existing thread" is a *policy* + change to the quota strategy's deliberate rebinding at + `src/codex/routing.ts:1179-1200`, not part of this fix. Do not smuggle it in. + +## Status + +Not started. Sequenced after `260803_pr_issue_sweep` closes. diff --git a/devlog/_plan/260803_pr_issue_sweep/000_plan.md b/devlog/_plan/260803_pr_issue_sweep/000_plan.md new file mode 100644 index 0000000000..9036937f93 --- /dev/null +++ b/devlog/_plan/260803_pr_issue_sweep/000_plan.md @@ -0,0 +1,199 @@ +# 000 — PR and issue sweep: land the image fix, resolve the duplicates, triage the backlog + +## Objective + +Bring the open bug surface to a state where every item is either landed, +closed with a reason, or carrying a verdict grounded in code somebody actually +read. Three fronts were named: the image-forwarding fix, the duplicate pull +requests, and the standing issue backlog. + +## What the research round overturned + +Five sol-medium explorers were dispatched in parallel against `origin/dev` at +`fa51fce5414260c1c9955a7e67b06e7a960bec05`. Two working assumptions did not +survive contact with the evidence, and one PR slated for closure turned out to +be load-bearing. + +**#837 and #616 are not two competing implementations.** They are the same +implementation. `git` authorship shows #837's substantive commit `89d51dbc` +carries author `Eleven-is-cool` with authored timestamp `2026-07-28T12:23:13Z` +— byte-identical to #616's `1aba0e4b` — replayed onto a newer base with +`Ingwannu` as committer. The diffs match at +819/-18 across the same 16 files. +The second #837 commit is a one-line fixture adjustment for schema +normalization that only exists on the newer base. So #837 is the integration +vehicle for #616's work, already crediting it, and the question is not which +author wins but whether the shared implementation is correct. + +**Nothing in the standing backlog was silently fixed.** The premise going in +was that recent merges (#892, #917, #880, #899) would have closed several +issues by side effect. Of twelve issues examined against current code: zero are +already-fixed. Eight are still real, three need reporter information, and one — +#553 — is closeable, but as an environmental fault rather than a shipped fix. +The reporter's own evidence showed a Shadowrocket fake-IP DNS route returning a +NetEase certificate; the Copilot transport correctly refuses to weaken TLS +verification for it. + +**#916 is not superseded by #917.** It was expected to overlap heavily with the +security work merged an hour earlier. It does not. #917 introduced +`managementPrincipal()` and closed the star-consent bypass; #916 authenticates +the CLI's *outbound* management listener, which is a different boundary. The +auditor reproduced four High-severity defects on current `dev` directly: + +```text +Claude: {"baseUrl":"https://attacker.example","token":null} +Bun: {"path":"/Users/jun/.bun/bin/bun","source":"override",...} +Health: {"seen":"Bearer ocx_admin_AAAA...","source":"management-api-unavailable"} +``` + +An ambient `ANTHROPIC_BASE_URL` survives credential stripping and redirects +OAuth-bearing traffic; `OPENCODEX_BUN_PATH` is reread after Bun loads project +dotenv, so a repository-local file can persist the durable executable; and the +admin token is handed to any listener that answers a forgeable `/healthz`. + +## Work-phase map + +Dependency order, not effort order. Each phase closes with something +independently verifiable. + +| Phase | Doc | Unit | Depends on | +|---|---|---|---| +| 1 | `010` | Land #912 tool-result image forwarding | — | +| 2 | `020` | Rebase #837, fix two shared defects, land, close #616 | — | +| 3 | `030` | Compact alternate-account attempt (#913) | — | +| 4 | `040` | Backlog disposition: comment, close, or record | — | +| 5 | `050` | #916 salvage plan and closure | 2 (shared rebase surface) | + +**Phases 1–4 are mutually independent.** Phase 3 is now the only code change to +the routing subsystem in this unit, confined to the native compact branch in +`src/server/responses/compact.ts`. + +**What used to be Phase 3 is gone.** It carried #914 and #919. Both left for +`devlog/_plan/260803_transport_attribution/` after the audit gate showed each +is a policy decision about account-health attribution rather than a local fix. +The earlier tables in this document showed a 3→4 hard dependency and then +dropped it; with the transport work gone entirely, no such edge exists. + +**Phase 5's dependency was originally stated wrong.** The first version claimed +#916 must land last because the transport phases would move +`src/server/index.ts`. They never touched that file; #916's conflict there is +purely against #917, which already landed. The real overlap is Phase 2: both +touch `src/config.ts`, `src/server/auth-cors.ts`, and `tests/config.test.ts`. +Current hunks dry-run cleanly, but Phase 2 lands first so #916 rebases onto a +settled config surface. + +## Out of scope + +Releases and version cuts. `scripts/release.ts` runs from `preview`/`main` and +has not been authorized. `dev` is 215 commits past `v2.10.0`; that is a +conversation to have, not a step to take inside this unit. + +#915 (cooldown recovery probe) is real but rated High cost: it crosses routing +state, auth resolution, WHAM refresh concurrency, account generations, and +quota scopes, and needs a generation-fenced background probe rather than +ordinary account selection. Deferred to its own unit, +`devlog/_plan/260803_cooldown_recovery_probe/`, named here so the deferral is a +scheduled unit rather than a comment on an issue that nobody reads again. + +#893 (sparse Responses snapshots) is real, but the closed PR #894 that +addressed it was 1,168 additions across 23 files. Narrowing that to a +provider-local default-off repair is its own unit, +`devlog/_plan/260803_sparse_snapshot_repair/`. + +#914 and #919 (transport failure attribution, before and after HTTP 200) left +this unit after four audit rounds. Their analysis and the full rejection +history live in `devlog/_plan/260803_transport_attribution/`. Both issues stay +**open**; nothing in this unit fixes either. + +The #915 and #893 deferrals were checked by the plan reviewer against the +issues themselves and judged defensible rather than scope evasion — but only on +the condition that they become named units. That condition is met above. The +#914/#919 deferral is a separate matter: it was not a scheduling choice but the +audit gate's own conclusion, recorded below. + +## Audit history + +The audit gate ran five rounds: four FAIL, then GO-WITH-FIXES. Every rejection +landed before a line of code was written. They came from a mix of runtime +probing, ordinary code reading, and — in the last round — reading this +repository's own archived decision records. The mix matters: a reviewer who +only ran probes would have missed the test and phase-contradiction blockers; +one who only read current code would have missed that the pinned runtime does +not emit the error codes the plan was matching on; and one who read neither the +archive would have let #919 through as a bug fix when it is a policy reversal. + +The outcome was not another revision of the guard. The whole transport- +attribution phase left this unit, taking #914 and #919 with it. What remains +here is work whose correctness does not depend on an unsettled policy question. + +**Round 1 — FAIL.** The plan matched Node error codes (`ENOTFOUND`, +`EAI_AGAIN`, …) on the fetch rejection. Bun 1.3.14 does not emit them: a +nonexistent hostname and a refused port both yield `ConnectionRefused`, +`errno: 0`, no `cause`. The guard would have passed a unit test that injected +the code by hand and never fired in production. Reproduced independently before +amending. Two further blockers: two existing tests were mischaracterized as +encoding the defect (they test valid lower-layer behavior and now stay +untouched), and Phase 4 contradicted Phase 3 on alternate-account attribution. + +**Round 2 — FAIL.** The amended plan kept a classifier but resolved the +hostname with `dns.lookup()` to decide neutrality. Rejected for two reasons. +Bun's labels are not a stable set — repeated fetches to the same `.invalid` +host alternate between `ConnectionRefused` and `FailedToOpenSocket` as Bun +evicts its DNS cache, so every other request would have skipped the probe. And +"the name resolves, therefore the account is at fault" does not follow: a +resolving host can still refuse TCP through a firewall, VPN, or fake-IP proxy — +a case this repository explicitly supports at +`src/lib/destination-policy.ts:175`. + +**Round 3 — FAIL.** The third design stopped classifying the error and used the +boundary instead: a rejected `fetch` means no response header arrived, so no +server evaluated the credential. The supporting fact — that +`applyCodexAuthContextToProvider()` (`src/codex/auth-context.ts:310`) swaps the +token without changing the destination — is true and verified. The boundary +claim is not. The reviewer drove two counterexamples through the real wrapper: +Bun follows redirects by default, so a server can receive the authenticated +request, return 307 to a dead host, and produce a rejection *after* headers +arrived; and a server that reads `Authorization` then closes the socket yields +`ECONNRESET` with the credential already seen. A credential-aware upstream can +do either differently for A than for B. + +Round 3's resolution moved #914 out and kept #919, on the grounds that #919 +"never depended on inference" — the synthetic/real distinction is already +carried on `RequestLogContext`, so the fix reads a field. + +**Round 4 — FAIL.** That was wrong too, and the reviewer proved it from the +repository's own history rather than from the runtime. The synthetic 502 that +#919 objects to was introduced deliberately, by +`devlog/_fin/260722_issue_bug_sweep/030_patch_s_sticky_502.md`, with a source +comment that is still there: report `failed` with a synthetic 502 *so that the +account-health recorder treats it as a transient upstream failure*. That +patch's own test matrix records the intended outcome as "transient 실패 기록, +affinity 해제" — the exact behavior #919 reports as a bug. + +So "the proxy invented this 502, therefore it is not evidence about the +account" does not hold. `terminalSource="synthetic"` proves the proxy +manufactured the *event*; the underlying socket reset is still an upstream read +failure, and account health tracks transient reliability, not only credential +validity. Changing it means reversing a considered decision, which is a policy +call needing evidence about whether mid-stream drops correlate with accounts. + +**Resolution: the whole transport-attribution phase leaves this unit.** Four +designs died at the same place across two issues, which is evidence about the +problem rather than about the designs. Each tried to answer "was this the +credential's fault?" from evidence insufficient in principle — Bun collapses +distinguishable network conditions into one label, the path can traverse an +authenticating server before failing, and after HTTP 200 the question stops +being about credentials at all and becomes about reliability. The likely +correct direction for both is separating host health from account health. That +work, and the full rejection history including the `5xx → retry rejection` +attempt-history hole, is in `devlog/_plan/260803_transport_attribution/`. + +Four rejected designs cost one session and shipped nothing broken. The +alternative was a guard that passed CI and silently changed routing behavior +the repository had already reasoned about once. + +## Evidence standard + +Every behavioral fix in this unit carries a red-green ablation: the regression +test must fail with the fix removed. A green suite proves nothing about a +branch no test drives. Structural assertions are acceptable only where the +property is invisible to behavior, and then they say so. diff --git a/devlog/_plan/260803_pr_issue_sweep/010_phase1_image_forwarding.md b/devlog/_plan/260803_pr_issue_sweep/010_phase1_image_forwarding.md new file mode 100644 index 0000000000..e382936dd8 --- /dev/null +++ b/devlog/_plan/260803_pr_issue_sweep/010_phase1_image_forwarding.md @@ -0,0 +1,109 @@ +# 010 — Phase 1: land #912, tool-result images to vision models + +## Unit + +PR #912 by @DevMello, head `d0a525d7226f2faedc14ed96270154ca5de7da24`, fixes +issue #888. Seven files, +207/−1. + +## Verdict from audit + +PASS, no blockers. The audit ran the merged tree in a scratch checkout: +typecheck exit 0, 42 image/EOF tests pass, 37 parser/vision tests pass. The +ablation is the part that matters — against unchanged `dev`, four +image-forwarding tests fail and the image-free control still passes. The test +is not tautological. + +## The change + +`src/adapters/openai-chat.ts` gains an image-part extractor and a deferred +carrier. Tool messages in the Chat Completions schema accept only strings or +text parts, so images cannot ride on the tool result itself. They are collected +while tool results are consumed and flushed as a following `user` message once +the tool round closes: + +```ts +const flushToolResultImages = (): void => { + if (pendingToolResultImageParts.length === 0) return; + out.push({ + role: "user", + content: [ + { type: "text", text: "[ocx] image output from the preceding tool result(s):" }, + ...pendingToolResultImageParts, + ], + }); + pendingToolResultImageParts = []; +}; +``` + +Flush points: the parallel-round close in `flushPendingToolCalls`, the +last-matching-result branch, and the orphan-result branch. + +This matches how the repository already carries tool-result images elsewhere — +`src/adapters/google.ts:196-208` puts `inline_data` beside the +`functionResponse` in a user turn, and `src/adapters/kiro.ts:511-531` uses the +same corresponding-user-carrier shape. + +## Drift after #880 + +#880 inserted five response-side helpers before `messagesToChatFormat`, moving +every target region: + +| Symbol | PR-era line | Merged line | +|---|---:|---:| +| `messagesToChatFormat` | ~80 | 195 | +| tool-round state | ~91 | 206–211 | +| deferred-barrier helper | ~109 | 222–242 | +| `flushPendingToolCalls` | ~122 | 247–259 | +| `toolResult` handling | ~232 | 360–402 | + +The hunks still land in the right regions. #912 changes request construction +only; the stream parser starts around merged line 802, so #896's finish-less +EOF logic is untouched — verified by the 37 EOF/terminal checks passing on the +merged tree. + +## Why this fixes #888 + +The reporter's route was traced end to end. Claude Code sends an Anthropic +`tool_result`; `src/claude/inbound.ts:96-113` preserves contained images as +`input_image`; `src/responses/parser.ts:547-555` converts to an internal +`toolResult` with structured image parts; Kimi OAuth +(`src/providers/registry.ts:712-741`) and Kimi API-key (`:1428-1444`) both +resolve to `openai-chat`. Current `dev` flattens that through +`contentPartsToText` — the `[image]` placeholder the reporter saw. + +The secondary `deepseek-v4-pro` report in the same issue is a different path: +built-in DeepSeek V4 models sit in `noVisionModels` +(`src/providers/registry.ts:1013-1016`) and route through the vision sidecar, +not this carrier. + +## Capability gating + +The adapter forwards valid image parts unconditionally, which is correct +because gating happens upstream: `provider.noVisionModels` at +`src/server/responses/core.ts:1536-1553` either invokes the sidecar or strips +images fail-closed, and `carriesImages()` includes `toolResult` +(`src/vision/index.ts:187-194`). A provider that rejects image parts declares +itself in `noVisionModels`; that is the existing registry location, and no new +gate is needed. + +## Plan + +1. Wait for the PR's own CI. It runs the **old** workflow (single `ubuntu` / + `windows` jobs) because the branch is 140 commits behind `dev`. That is + expected and not a defect. +2. Merge. Do not force-push the contributor's branch; the merge commit rebases + nothing and the audit already verified the merged tree. +3. Close #888 referencing the merge commit. + +## Accept criteria + +- `gh pr view 912 --json state` reports `MERGED`. +- The sharded lane is green on the resulting `dev` commit. +- `gh issue view 888 --json state` reports `CLOSED` with the commit named. + +## Residual + +Non-blocking: the five translated adapter doc bullets describe direct +`image_url` forwarding without noting that `noVisionModels` models receive +sidecar-generated text instead. Worth a follow-up sentence, not a merge +blocker. diff --git a/devlog/_plan/260803_pr_issue_sweep/020_phase2_hosted_tool_duplicate.md b/devlog/_plan/260803_pr_issue_sweep/020_phase2_hosted_tool_duplicate.md new file mode 100644 index 0000000000..02db83ca08 --- /dev/null +++ b/devlog/_plan/260803_pr_issue_sweep/020_phase2_hosted_tool_duplicate.md @@ -0,0 +1,136 @@ +# 020 — Phase 2: hosted image tool preferences (#837 lands, #616 closes) + +## The provenance finding + +These were filed as two pull requests solving the same problem. They are one +implementation submitted twice. + +| | #616 | #837 | +|---|---|---| +| Opened | 2026-07-28T12:13:33Z | 2026-08-01T04:59:16Z | +| Substantive commit | `1aba0e4b` | `89d51dbc` | +| Author | Eleven-is-cool | Eleven-is-cool | +| Authored at | 2026-07-28T12:23:13Z | 2026-07-28T12:23:13Z | +| Committer | Eleven-is-cool | Ingwannu | +| Diff | +819/−18, 16 files | identical | + +#837 replayed #616's commit onto a newer base, preserving authorship, and adds +one independent change: + +```diff +- { type: "function", name: "exec_command", parameters: {} }, ++ { type: "function", name: "exec_command", parameters: { type: "object" } }, +``` + +That is a fixture correction for schema normalization present only on the newer +base. #837's body already credits #616. + +So the closure reason for #616 is supersession by an integration that carries +the same authorship, not duplication and not "we fixed it elsewhere." + +## What the change does + +`hosted-tool-policy.ts` does not exist on `dev`. The policy currently lives as +an adapter-local constant at `src/adapters/openai-responses.ts:164-174`: + +```ts +const UNSUPPORTED_HOSTED_TOOLS = [ + { + match: model => model.includes("codex-spark"), + tools: new Set(["image_generation", "tool_search"]), + }, +]; +``` + +Both PRs extract it unchanged into a nine-line module so config validation can +also consult it, then add an exact-model opt-in +`modelPreferHostedTools?: Record` that runs *before* ordinary +normalization: + +```ts +} else { + outBody = preferConfiguredHostedTools( + outBody, + provider, + parsed.modelId, + parsed._openAiVirtualSelectedModelId, + ); + outBody = normalizeImageGenClientTools(outBody); +} +``` + +The defect being fixed: `normalizeImageGenClientTools()` gives a client +`image_gen` declaration precedence over hosted `image_generation`, preserving +even an empty client namespace. Gateways that reserve `image_gen` server-side +reject that empty namespace, and the user loses hosted image generation with no +way to express the preference. + +`src/server/auth-cors.ts` is in the file set because +`providerManagementConfigError()` happens to live there; the hunk wires the new +validator into `/api/providers` writes. No origin, header, cookie, or token +behavior changes. + +## Two defects both copies share + +Both must be fixed before landing. Both were reproduced directly. + +**Validation disagrees with runtime routing** — `src/config.ts:641`. +Configuring `modelPreferHostedTools` for `deepseek/deepseek-v4-flash` is +rejected with `requires the openai-responses wire`, but at runtime +`resolveWireProtocolOverride(..., "responses")` selects `openai-responses` from +registry `modelWireDefaults`. The validator starts from `registry.adapter` and +never consults the inbound-aware `providerModelWireDefault()` path. A valid +preference is refused at config load and through management writes. + +Fix: compute the effective wire through the same Responses-inbound +registry/default/override sequence the runtime uses. + +**Inherited model IDs throw before dispatch** — +`src/adapters/openai-responses.ts:641-643`. + +```ts +provider.modelPreferHostedTools?.[modelId] +``` + +reads `Object.prototype`, so a routed model ID of `constructor` or `toString` +yields a function and the subsequent `.includes` throws +`TypeError: ...?.includes is not a function`, failing the request before +upstream dispatch. + +Fix: own-property lookup plus `Array.isArray(preferredTools)` before `includes`. + +## Rebase surface + +Neither head applies cleanly to `fa51fce541`. Both conflict in exactly six +files: + +1. `docs-site/src/content/docs/reference/configuration.md` +2. `docs-site/src/content/docs/ja/reference/configuration.md` +3. `docs-site/src/content/docs/ko/reference/configuration.md` +4. `docs-site/src/content/docs/ru/reference/configuration.md` +5. `docs-site/src/content/docs/zh-cn/reference/configuration.md` +6. `src/config.ts` + +The adapter, policy module, virtual-model, types, CORS, structure doc, and all +four test files auto-merge. + +## Plan + +1. Rebase #837's head onto `origin/dev`, resolving the five locale docs and + `src/config.ts` by hand. Preserve `Eleven-is-cool` authorship on `89d51dbc` + through the rebase. +2. Fix the wire-resolution defect in `src/config.ts`; add a DeepSeek regression + to `tests/config.test.ts` and the management-boundary equivalent. +3. Fix the prototype-chain lookup in `src/adapters/openai-responses.ts`; add a + `constructor`/`toString` regression to + `tests/openai-responses-passthrough.test.ts`. +4. Ablate both new tests against the unfixed code. +5. Land #837; close #616 with the supersession reason and thanks to + Eleven-is-cool for the original implementation. + +## Accept criteria + +- Both new regressions fail with their fix reverted. +- `bun x tsc --noEmit` exit 0; focused suites green. +- `gh pr view 837 --json state` MERGED, `gh pr view 616 --json state` CLOSED + with a comment naming #837 and preserving credit. diff --git a/devlog/_plan/260803_pr_issue_sweep/030_phase3_compact_alternate.md b/devlog/_plan/260803_pr_issue_sweep/030_phase3_compact_alternate.md new file mode 100644 index 0000000000..193dcc16ed --- /dev/null +++ b/devlog/_plan/260803_pr_issue_sweep/030_phase3_compact_alternate.md @@ -0,0 +1,188 @@ +# 030 — Phase 3: compact endpoint alternate-account attempt (#913) + +## The gap + +`/v1/responses` already handles a pre-stream 429/402 by trying one eligible +alternate account inside the same logical request. +`retryCodexPoolOnAlternateAccount()` at `src/server/responses/core.ts:319-423` +excludes the first account, resolves an alternate, records and promotes the +rejected one, cancels the first body, rebuilds auth/provider/request, and sends +exactly once. It is activated before streaming at `:1679-1721`. Recognition is +narrow by design — only 429 and 402 (`:248-251`). + +`/v1/responses/compact` has none of it. It resolves one context and builds one +set of headers (`src/server/responses/compact.ts:211-256`), performs one fetch, +buffers, records status, returns (`:275-319`). When the affined account returns +429, the client sees the rejection and retries the compact task *outside* the +logical request, which is how a session ends up reporting exhausted retries +while another pool account sat idle. + +A second, smaller defect on the same path: `bufferCompactResponse()` +(`:120-156`) reconstructs the response with only `Content-Type`, dropping +`Retry-After` and reset headers from the original rejection. The client loses +the information it needs to back off correctly. + +## Change + +Restructure the native compact branch around a bounded local helper: + +```ts +type CompactRecovery = "normal" | "single"; +const sendCompactAttempt = async ( + authCtx: CodexAuthContext, + recovery: CompactRecovery, +): Promise => { /* ... */ }; +``` + +**The two recovery modes are the crux, not decoration.** Compact's current send +goes through `fetchWithTransientRetry()` +(`src/server/responses/compact.ts:275`), which makes up to three status +attempts, each wrapping its own reset retries (`src/lib/upstream-retry.ts:178`, +`:213`). So "exactly two sends" and "preserve A's existing recovery" contradict +each other unless the two accounts use different modes. An earlier draft said +"send B exactly once" without noticing the wrapper, and would have produced +either a B that retried up to nine times or an A that silently lost its reset +recovery. + +The regular path already solved this and the fix copies it: A keeps +`fetchWithTransientRetry()` (`recovery: "normal"`), B goes through a single +direct `fetchWithHeaderTimeout()` (`recovery: "single"`), exactly as +`retryCodexPoolOnAlternateAccount()` does at +`src/server/responses/core.ts:396`. The asymmetry is deliberate there: A's +retries happen before any alternate is considered, and the alternate is a last +bounded try rather than a second retry ladder. + +### The recorder has to change first + +`recordCompactPoolOutcome()` (`src/server/responses/compact.ts:259-273`) closes +over a single `authCtx` and accepts only `{ retryAfter, resetAt }`. It cannot +express "record this against A, and promote B", and it cannot record an outcome +against B at all. So the two-account flow is impossible until the recorder +takes its context explicitly: + +```ts +const recordCompactPoolOutcome = ( + ctx: CodexAuthContext, + outcome: CodexUpstreamOutcome, + meta: { + retryAfter?: string | null; + resetAt?: unknown | unknown[]; + promoteAccountId?: string; + } = {}, +) => { /* same body, ctx instead of the captured authCtx */ }; +``` + +Every existing call site passes `authCtx` and is otherwise unchanged, so this +is a mechanical widening — but it is a prerequisite, not an implementation +detail, and an earlier draft of this doc assumed a signature that does not +exist. + +Flow: + +1. Send A with `recovery: "normal"` — unchanged from today, including transient + and reset recovery. +2. On a pre-body 429/402 **only**, resolve + `resolveCodexAuthContext(..., { excludeAccountId: A, modelId })`. +3. No alternate: do not cancel A. Buffer, **record A's outcome exactly as today** + (`recordCompactPoolOutcome(authCtxA, upstream.status, { retryAfter, resetAt })` + after buffering, per the current `:309-318` ordering, including the 499 + early return), and return A's status, body, status text, and sanitized + headers unchanged. An earlier draft said only "buffer and return", which + would have silently dropped health recording on the most common path — + a regression disguised as a no-op. +4. Alternate B exists: + - apply A's quota headers to its quota cache; + - record A's actual rejection with retry/reset metadata, scope, writer + generation, and `promoteAccountId: B`; + - **build B's provider, headers, base URL, and auth context completely + first**, then cancel A's body. If B's construction throws, A's body is + still intact and its rejection can be returned to the client. Cancelling + first would leave nothing to fall back to. + - send B with `recovery: "single"` — one network send, no transient ladder, + no reset retry. +5. Buffer B's response and record its outcome **against B's context** + (`recordCompactPoolOutcome(authCtxB, ...)`), preserving the current + record-after-buffer ordering and the 499 early return. No recursion, no + reset-credit redemption. + +**Attribution of a transport exception during B's send — preserve current +behavior.** Earlier drafts of this doc tried to define a neutrality rule here, +first attributing every B exception to B, then deferring to a transport-phase +classifier. Both are now moot: #914 and #919 left this unit entirely +(`devlog/_plan/260803_transport_attribution/`) after four rejected designs +established that no local rule correctly decides attribution. + +So this phase does **not** change attribution semantics at all. A transport +exception during B's send records against B exactly as the current code records +against A — same helper, same outcome mapping. When +`260803_transport_attribution` settles the policy, it changes both call sites +together. + +This also removes the ordering dependency the earlier plan carried. With the +transport-attribution work gone, this phase shares no contract with any other +phase in the unit; it is confined to the native compact branch in +`src/server/responses/compact.ts`. + +MODIFY `bufferCompactResponse()` to preserve safe upstream headers, `Retry-After` +and reset headers in particular. + +## Explicitly not in scope + +The quota strategy deliberately rebinds an over-threshold thread at +`src/codex/routing.ts:1179-1200`, and tests cover that. The reporter's framing +suggests a 100% WHAM snapshot should never move an existing thread under any +strategy. That is a policy change with its own tradeoffs, not part of this fix. +Fill-first and round-robin already preserve ongoing affinity. Do not smuggle a +strategy change in under a compact-parity fix. + +## Activation scenario + +In `tests/responses-compaction-routing.test.ts`: + +- A returns 429, B returns 200. Assert two sends, B's credentials on send two, + A's body cancelled before send two, B's response returned. Repeat for 402. +- **No alternate available**: assert A's outcome is still recorded — same + status, same retry/reset metadata — proving the recorder widening did not + drop the common path. This is the regression the fourth-round audit caught in + the plan rather than in the code. +- **B's construction fails** (resolve or provider build throws after A's 429): + A's body is returned intact, not a 502. Proves the build-before-cancel + ordering. +- **Outcome attribution**: A's rejection is recorded against A with + `promoteAccountId: B`; B's final status is recorded against B. Assert the + account ids explicitly — with a single captured context this test cannot + pass, so it is the activation proof for the recorder change. +- **B returns a transient 5xx** (500/502/503/504/52x): assert B's `fetch` + invocation count is exactly **one**. Under `recovery: "normal"` it would be + three, so this assertion is what proves the mode is actually wired rather than + defaulted — the activation proof for the whole two-mode design. +- **B's first socket resets**: again exactly one B `fetch` invocation. The reset + ladder must not engage for the alternate. +- **A's transient recovery still works**: A returns 502 twice then 200, no + alternate involved — assert A's normal retry behavior is intact. The control + against "fixing" the cardinality by stripping A's recovery. +- No alternate available: exactly one send, and the returned rejection is + equivalent to A's **over the sanitized allowlist**. + `sanitizePassthroughHeaders()` (`src/server/relay.ts:1023`) deliberately drops + content-length, content-encoding, cookies, and hop-by-hop headers, so + byte-equality against the raw upstream response is the wrong assertion and + would fail for reasons unrelated to this change. Assert: status, body bytes, + `statusText`, and the presence and value of `Retry-After` plus reset headers. +- B also rejects: exactly two sends, B's rejection returned. +- Abort between attempts prevents B's send entirely. +- A cached at 100% under fill-first stays affined until A actually rejects — + the guard against the scope creep named above. +- **B's send rejects (fetch throws)**: recorded against B and returned as 502, + matching what the single-attempt path does today for A. Asserted so that a + later attribution change has to update this test deliberately rather than + silently. +- **B returns a real 5xx response**: B's outcome recorded, response returned. + +The send-count assertion is the activation proof: one send means the branch +never fired, three means it recursed. + +## Accept criteria + +- The 429→alternate test fails with the helper reverted to a single send. +- Header preservation asserted on the no-alternate path (`Retry-After` present). +- Focused suite `responses-compaction-routing` green; full suite green. diff --git a/devlog/_plan/260803_pr_issue_sweep/040_phase4_backlog_disposition.md b/devlog/_plan/260803_pr_issue_sweep/040_phase4_backlog_disposition.md new file mode 100644 index 0000000000..dac92cfa1d --- /dev/null +++ b/devlog/_plan/260803_pr_issue_sweep/040_phase4_backlog_disposition.md @@ -0,0 +1,135 @@ +# 040 — Phase 4: backlog disposition + +Twelve standing issues examined against `origin/dev` at `fa51fce541`. The +expectation was that recent merges had quietly fixed several. None had. + +| Issue | Verdict | Action | Cost | +|---|---|---|---| +| #907 stale Terra/Luna prices | STILL-REAL | Update metadata, regenerate | S, cross-repo | +| #893 sparse Responses snapshots | STILL-REAL | Narrowed resubmission of #894 | M/L, own unit | +| #888 Kimi K3 image forwarding | STILL-REAL | Closed by Phase 1 (#912) | S | +| #796 Volcengine Ark 400 | NEEDS-INFO | Ask for live Ark result | external | +| #586 no Pool/Direct UI switch | STILL-REAL | Add switch, one surface | S/M | +| #553 Copilot TLS mismatch | ENVIRONMENTAL | Close with explanation | 0 | +| #545 Claude Desktop classifier | STILL-REAL | Needs live OAuth validation | M/H | +| #418 V2 custom→custom delegation | NEEDS-INFO | Ask for current trace | M | +| #241 Desktop model picker | STILL-REAL | Upstream tracker, keep open | external | +| #92 encrypted NEW_TASK | STILL-REAL | Upstream-blocked, keep open | H/external | +| #904 Korean U+FFFD | NEEDS-INFO | Await failing capture | diagnostic | +| #417 voice transcript U+FFFD | STILL-REAL | Upstream tracker, keep open | external | + +## The closeable one + +**#553** is the only issue that ends this round. The Copilot transport accepts +only HTTPS `*.githubcopilot.com` endpoints +(`src/providers/github-copilot-transport.ts:43`), and PR #575 / commit +`fff8c369f` added TLS-interception identification with an SNI probe +(`src/server/responses/upstream-error.ts:18-25`). The reporter's own evidence +showed a Shadowrocket fake-IP DNS route returning a NetEase certificate, and a +later route returning the correct Copilot certificate. That is interception on +the reporter's machine, not a hostname rewrite in the adapter. The closure says +so plainly and notes that the transport deliberately does not weaken TLS +verification to accommodate it. + +## Corrections to record + +**#907** — the report is right that Terra and Luna carry pre-price-cut rates. +The bundle at `src/generated/jawcode-model-metadata.ts:47` still has Terra +`2.5/15/0.25/3.125` and Luna `1/6/0.1/1.25`. But the report's claim that cache +writes should be zero is wrong; current published rates +(`input/output/cacheRead/cacheWrite`) are Terra `2/12/0.20/2.50`, Luna +`0.20/1.20/0.02/0.25`, Sol `5/30/0.50/6.25`. The generator source is also +stale, so rerunning `bun run generate:jawcode-metadata` alone reproduces the +defect — the canonical `models.json` in the jawcode source has to move first. +That is a cross-repository step, which is why this is recorded rather than +patched here. + +**#796** — the fix may already be present. +`src/adapters/openai-chat.ts:491-515` detects the Ark host and emits a +structured placeholder, called from all three assistant placeholder sites +(`:315`, `:318`, `:347`), and commit `2eebd9268` is an ancestor of `dev`. But +`tests/volcengine-ark-assistant-content.test.ts:16-18` says outright that the +structured empty array is *inferred* and has never been validated against a +live Ark endpoint. Closing on an untested inference would be the exact +evidence failure this unit is trying to avoid. Ask the reporter. + +**#418** — not stale and not a duplicate of #92. The V2 model guidance path +still exists (`src/server/responses/collaboration.ts:266-275`). The newest +trace attached to the issue completed on an inherited native model because +`model` was omitted, which is a different failure from the reported +custom-parent → custom-child pairing. One current custom/custom trace settles +which it is. + +**#545** — narrower than filed. Gateway-key traffic bypasses native passthrough +and enters translation (`src/server/claude-messages.ts:596-604`), and Anthropic +OAuth always prepends the Claude Code identity +(`src/adapters/anthropic.ts:752-757`). Caller `max_tokens` and stop sequences +are preserved (`:750`, `:764`), so this is not generic parameter loss — it is a +Claude Desktop 3P / Anthropic OAuth compatibility problem. Commit `7fcaa911` +fixed only the false 502 request-log classification, not the repeated +classifier calls. + +**#241, #92, #417** stay open as upstream trackers. Routed entries are produced +with `visibility: "list"` (`src/codex/catalog/sync.ts:175-202`); the root +`model_catalog_json` key is written into the Codex config at +`src/codex/inject.ts:430-457` (`:348-357` is only the read/set helper pair — +the original citation here was imprecise and the plan reviewer caught it). The +remaining filter is Desktop's own `available_models` allowlist, which does not +exist in this repository. #92's +ciphertext is opaque to the parser (`src/responses/parser.ts:234-236`) and the +current hardening fails cleanly before routed dispatch +(`src/server/responses/core.ts:1381-1385`) — better diagnosis, still no +plaintext to deliver. + +**#904** — no new evidence since filing. The compaction/Kiro surrogate fixes in +#892 (`eeef7a32`) require astral characters at exact chunk boundaries and do +not explain Hangul-only corruption. Hangul sits in the BMP; it survives a +3-byte split differently than a surrogate pair does. Without an +`OCX_LIVE_FRAME_LOG` capture there is no justified repository fix, and guessing +at one would risk changing byte-transparent relay behavior that tests currently +prove correct (`tests/server-live.test.ts:642-675`). + +## Plan + +1. Close #553 with the environmental explanation and credit for the diagnostic + evidence the reporter supplied. +2. Close #888 as part of Phase 1. +3. Post evidence comments naming the current code path on #907, #893, #586, + #545, #241, #92, #417 — each says what is unfixed and where. +4. Post specific information requests on #796, #418, #904 — naming exactly what + capture or trace would settle it, not a generic "please provide more info". +5. #913 is handled by Phase 3 and gets its closure there. + + **#914 and #919 both stay open.** Four designs across the two were rejected + at the audit gate, and both moved to + `devlog/_plan/260803_transport_attribution/`. Their comments record what was + tried and why each failed, so the next attempt does not rediscover that Bun + collapses DNS failure and connection refusal into one label, that redirects + put credential-visible failures on the same path, that a 5xx observed during + a retry vanishes from the outer catch — or, for #919, that the synthetic 502 + it objects to was introduced deliberately in + `devlog/_fin/260722_issue_bug_sweep/030_patch_s_sticky_502.md` precisely so + account health would treat a mid-stream reset as transient. Reversing that + is a policy call, not a bug fix. Claiming either is fixed here would be the + overreach the audit prevented. +6. #915 gets an evidence comment recording the fix shape and pointing at its + own scheduled unit `devlog/_plan/260803_cooldown_recovery_probe/`: it crosses + routing state, auth resolution, WHAM refresh concurrency, account + generations, and quota scopes, and needs a generation-fenced background probe + rather than ordinary account selection. Notably + `clearCodexAccountCooldown()` must **not** be used — it clears every scope + and lacks the credential fence. +7. #893's comment points at `devlog/_plan/260803_sparse_snapshot_repair/` for + the same reason. A deferral that names a unit is a schedule; a deferral that + names nothing is an issue nobody reopens. + +## Accept criteria + +- Every issue in the table carries either a closure tied to a commit or a dated + comment naming a file:line and a verdict — **excluding the two closures this + phase delegates**: #888 closes in Phase 1 with #912's merge commit, and #913 + closes in Phase 3 with its own. Those two are outside this phase's completion + gate; if they were inside it, Phase 4 could never reach terminal state before + Phases 1 and 3, which would make the "phases 1–4 are independent" claim in + `000_plan.md` false. +- No issue is closed on inference. #796 is the test of this rule. diff --git a/devlog/_plan/260803_pr_issue_sweep/050_phase5_916_disposition.md b/devlog/_plan/260803_pr_issue_sweep/050_phase5_916_disposition.md new file mode 100644 index 0000000000..61e371cf9e --- /dev/null +++ b/devlog/_plan/260803_pr_issue_sweep/050_phase5_916_disposition.md @@ -0,0 +1,126 @@ +# 050 — Phase 5: #916 disposition + +## Scope note + +This PR is security work on unfixed defects. Per `AGENTS.md`, reproduction +detail, bypass reasoning, and severity assessment for anything not yet public +belong in scratch space, not here. This document records only the disposition: +which hunks are kept, which are rejected, and what has to happen before any of +it lands. The working notes live under a `mktemp -d` path and are deleted when +the work closes. + +## Verdict + +`SALVAGE-5-HUNKS`. #916 is not superseded by #917, and it does not land as-is. + +The expectation was overlap: #917 merged an hour before this audit and touched +`management-auth`, `auth-cors`, `bun-runtime`, and `server/index.ts`. But #917 +resolved the *inbound* management principal — who may call the star route. +#916 authenticates the CLI's *outbound* management listener — whether the thing +answering on the port deserves the admin token. Different boundary, no overlap +in effect. The auditor confirmed the outbound path still misbehaves on current +`dev` after #917. + +## Keep + +1. **Vertex location validation** — new `src/providers/google-vertex-location.ts` + single-label validator, checked in `src/adapters/google.ts` before ADC + acquisition and again at the management write boundary through + `providerManagementConfigError()`. Permits `global`, `us`, `us-central1`; + rejects anything that changes the request authority. Regression in + `tests/gcp-adc.test.ts`. +2. **Durable Bun runtime provenance** — `src/lib/bun-runtime.ts` accepts a + recorded runtime only when the path names `process.execPath`, and the + Bun-side override reselection is removed. Documented `ocx service install` + overrides keep working because `bin/ocx.mjs` selects and stamps before Bun + starts. Regressions in `tests/bun-runtime.test.ts`, `tests/service.test.ts`. +3. **Local management listener attestation** — new + `src/lib/local-management-attestation.ts` (challenge + PID + port HMAC, + timing-safe compare, fail-closed on malformed input), the runtime secret in + `src/config.ts` (`atomicWriteFile()` already writes mode `0600` and hardens + Windows ACLs before rename), start wiring in `src/cli/index.ts`, the attested + `/healthz` in `src/server/index.ts`, and the listener challenge in + `src/oauth/health.ts`. +4. **Proof-bound Claude destination provenance** — `bin/ocx.mjs` captures + destination and token slots before Bun loads dotenv and binds the snapshot to + a random argv proof; `src/cli/launcher-context.ts` enforces proof shape, + rejects duplicates, allowlists slots, and clears the provenance variables. + **Narrowed** — see the rejected hunk below. +5. **Documentation** — the corrected CI security-boundary comment and the + external approval-policy section, which the auditor verified against the live + API (`approval_policy=all_external_contributors`, no self-hosted runners + registered). + +## Reject + +**`src/cli/claude.ts` no-context `?? []` fallback.** With no launcher context, +the PR strips genuine shell-exported API credentials and redirects to the proxy +marker. Direct `bun src/cli/index.ts` is explicitly supported +(`structure/01_runtime.md:9`), so this breaks a documented entry path: a user +who exported a real key in their shell loses it. Fail-closed handling is right +for the OAuth-bearing subscription destination and wrong for credentials with +unknown provenance. `tests/claude-auth-mode.test.ts` currently encodes the +regression as intended behavior and must be rewritten with the fix. + +**`structure/06_docs-and-release.md` workflow table.** Written against the +pre-#899 workflow: it claims the Bun `test` job keeps PRs on hosted Windows. +Current reality is four Linux shards (`ci.yml:189-247`), full macOS +(`:304-349`), Windows only on main/preview push or dispatch (`:351-430`), and +the aggregate `ci` gate (`:485-556`). Rewrite, do not carry forward. + +**`structure/01_runtime.md` decision log** — mixed. The threat description and +the launcher-proof choice are accurate; the paragraph recording direct-Bun +fail-closed behavior documents the rejected regression. Revise alongside the +implementation fix. + +## Activation scenarios for the salvaged hunks + +The PR's own tests are stronger than this doc's first draft, which listed test +files without saying what makes each guard fire. Recording the triggers so the +rebase does not quietly lose one: + +| Hunk | Trigger | Observable proof | +|---|---|---| +| Vertex location | `provider.location` set to a value carrying `:`, `/`, or `#` | ADC acquisition never runs; management write returns a config error; `global`/`us-central1` still accepted | +| Bun provenance | Recorded runtime path that does not name `process.execPath` | Durable artifact keeps the launcher-stamped pair; unpaired late override ignored | +| Listener attestation | Fake listener answering `/healthz` on the configured port | No bearer in the outbound request; valid proof still authenticates the real listener | +| Claude destination | `ANTHROPIC_BASE_URL` present in dotenv but absent from the launcher snapshot | Destination replaced with the local proxy; the same variable exported by the parent shell is preserved | +| Direct-Bun credential (the fix) | `bun src/cli/index.ts` with a shell-exported key, no launcher context | Key preserved — this is the test that must fail with the `?? []` fallback restored | + +## Conflict surface + +`git merge-tree` reports exactly one conflicted file: `src/server/index.ts`. +#917 changed the management-auth import and the `/api` dispatch; #916 changes +the adjacent import/start/`/healthz` region. The resolution must retain +`managementPrincipal()` and the five-argument `handleManagementAPI(...)` call +while adding #916's attestation imports, the `startServer` secret argument, and +the proof response. `.github/workflows/ci.yml` and `src/server/auth-cors.ts` +auto-merge. + +## Plan + +Ordering correction: the first draft said this phase lands last because Phases +3–4 would move `src/server/index.ts`. That was wrong — they do not touch that +file, and the conflict is purely against #917, which has already landed. The +real dependency is Phase 2: both touch `src/config.ts`, +`src/server/auth-cors.ts`, and `tests/config.test.ts`. Phase 2 lands first so +this rebases onto a settled config surface. + +1. Rebase onto `dev` at whatever tip Phases 1–4 produce; resolve + `src/server/index.ts` preserving `managementPrincipal()`. +2. Fix the direct-Bun credential regression; replace the test that mandates it. +3. Rewrite the stale workflow table against #899's actual job graph. +4. Rerun focused security tests, typecheck, full suite, privacy scan. +5. **Keep the PR in draft pending explicit maintainer security review.** It + touches authentication, credential handling, and the durable launcher — + `MAINTAINERS.md` requires security review for exactly this surface. The + audit is not that review, and an agent does not substitute for it. + +## Accept criteria + +- The five salvaged hunks apply on a rebased branch with the suite green. +- The direct-Bun path preserves a shell-exported credential, proven by a test + that fails with the `?? []` fallback restored. +- The workflow table matches the current `ci.yml` job graph. +- Disposition and residual state recorded on the PR itself so @Ingwannu can see + which parts were kept and why the rest was not. diff --git a/devlog/_plan/260803_sparse_snapshot_repair/000_plan.md b/devlog/_plan/260803_sparse_snapshot_repair/000_plan.md new file mode 100644 index 0000000000..d1f943214d --- /dev/null +++ b/devlog/_plan/260803_sparse_snapshot_repair/000_plan.md @@ -0,0 +1,38 @@ +# 000 — Sparse Responses snapshot repair (#893) + +Reserved unit. Deferred out of `260803_pr_issue_sweep` on blast radius. + +## The defect + +Responses-compatible gateways can emit lifecycle snapshots that are valid but +sparse — missing fields Codex clients need before they will commit the turn. +`src/server/responses/core.ts:1819-1827` composes only image-generation and +item-ID rewrites; there is no snapshot repair. `responses-snapshot-repair.ts`, +its config flag, its types, and its tests do not exist on `dev`. + +## History + +PR #894 addressed this and was closed unmerged. None of its four commits is an +ancestor of `dev`, and the #892 stack did not absorb the feature — verified +rather than assumed. + +It was 1,168 additions across 23 files, and it carried a rewrite of the Darwin +relay design unit along with the fix. That breadth is why it did not land, not +the idea. + +## Shape of the narrowed resubmission + +- Provider-local and default-off. A repair that rewrites payloads for every + gateway is a compatibility hazard; one that arms per provider is a fix. +- Preserve upstream values. Repair fills absences; it never overwrites what the + gateway actually sent. +- Bound the reconstructed output and charge/release the translator budget, so a + hostile or broken gateway cannot turn repair into unbounded work. +- Cover both SSE and JSON response modes. +- Leave the relay design unit alone. + +## Status + +Not started. Sequenced after `260803_pr_issue_sweep` closes. Credit to +@0xWinner98 for the original report and to PR #894's author for the first +implementation attempt, which the narrowed version should draw from. diff --git a/devlog/_plan/260803_transport_attribution/000_plan.md b/devlog/_plan/260803_transport_attribution/000_plan.md new file mode 100644 index 0000000000..9a71798dbc --- /dev/null +++ b/devlog/_plan/260803_transport_attribution/000_plan.md @@ -0,0 +1,195 @@ +# 000 — Transport failure attribution (#914, #919) + +Split out of `260803_pr_issue_sweep` after successive designs were rejected at +the audit gate. The rejections are the useful content here: each one was +empirically grounded, and together they say the problem is not the shape of a +guard — it is that "should this failure count against the credential?" is a +policy question the codebase has already answered once, deliberately. + +Both issues live here because they are the same question at two points on the +timeline: #914 is the failure before any response, #919 is the failure after +HTTP 200. Neither is a local bug fix. + +## The defect + +`src/server/responses/core.ts:1640-1656` maps every non-timeout `fetch` +rejection to `connect_error` and records it against the selected account; +`src/server/responses/compact.ts:275-301` does the same. Routing treats +`connect_error` as transient (`src/codex/routing.ts:297-307`), and at +`upstreamFailoverThreshold` it soft-avoids the account, clears thread affinity, +and promotes an alternate (`src/codex/routing.ts:1417-1465`). + +The reporter's point stands: a DNS failure on the user's machine rotates them +off a working account, and the rotation cannot help. + +## Three rejected designs + +### 1. Match Node error codes on the rejection + +`ENOTFOUND`, `EAI_AGAIN`, `ENETUNREACH`, `ENETDOWN`, `EHOSTUNREACH`, +`ECONNREFUSED`. Dead on arrival: Bun 1.3.14's `fetch` does not emit them. + +```text +fetch("https://no-such-host.invalid/x") -> code:"ConnectionRefused", errno:0, no cause +fetch("http://127.0.0.1:1/x") -> code:"ConnectionRefused", errno:0, no cause +fetch("http://192.0.2.1:81/x") -> TimeoutError after 4s +dns.lookup("no-such-host.invalid") -> code:"ENOTFOUND" +``` + +A unit test injecting `code:"ENOTFOUND"` would have passed while the branch +never fired in production. Bun also gives DNS failure and connection refusal +the *same* label, so even a Bun-native code list cannot separate them. + +### 2. Resolve the hostname to decide + +On a rejection, run `dns.lookup()` (which does return Node codes under Bun, +including inside a live `Bun.serve` handler): lookup fails → neutral, lookup +succeeds → attributable. + +Rejected for two reasons. Bun's labels are not a stable set — repeated fetches +to the same `.invalid` host alternate `ConnectionRefused` / +`FailedToOpenSocket` as Bun evicts its DNS cache, so every other request would +skip the probe entirely. And the inference is wrong: a hostname that resolves +can still refuse TCP behind a firewall, VPN, captive portal, or fake-IP proxy — +a configuration this repository explicitly supports +(`src/lib/destination-policy.ts:175`). TOCTOU runs both ways: a resolver that +recovers between fetch and probe gives a false attributable; one that flakes +gives a false neutral. + +### 3. Use the boundary — "rejection means no headers arrived" + +Skip health recording for every rejection, on the reasoning that a rejected +fetch means no server evaluated the credential, and that +`applyCodexAuthContextToProvider()` (`src/codex/auth-context.ts:310`) swaps the +token without changing the destination — so rotation cannot help by +construction. + +The destination fact is true and verified. The boundary claim is not. Two +counterexamples reached the real wrapper: + +- **Redirects.** Bun follows them by default, per the Fetch standard. A server + received the authenticated request, returned `307` to `127.0.0.1:1`, and the + wrapper ultimately rejected with `ConnectionRefused`. Headers arrived; the + credential was seen; the rejection came after. +- **Read-then-close.** A raw server read `Authorization: Bearer credential-A` + and closed the socket without responding. Bun rejected with `ECONNRESET`. + +Both are credential-*visible* failures, and a credential-aware upstream can +behave differently for A than for B. So "rotation cannot possibly help" is not +structurally true, and a blanket guard can suppress a genuinely account-specific +failure. + +## What the three rejections have in common + +Every design tried to answer "was this the credential's fault?" from evidence +available at the catch. That evidence is insufficient in principle, not by +accident: Bun collapses distinguishable network conditions into one label, and +the request path can traverse an authenticating server before failing. + +So the next attempt should not be a fourth classifier. Two directions worth +exploring, in this unit, with their own audit: + +1. **Separate host health from account health.** The real content of #914 is + that connection-level failures belong to a (provider, host) pair, not a + credential — every pool account shares the host. A distinct health key with + its own threshold would make the attribution question moot for the common + case, and rotation would stop being the response to a network fault. +2. **`redirect: "manual"` plus an explicit 3xx policy**, which at least removes + the redirect counterexample and makes the remaining surface small enough to + reason about. It does not solve read-then-close on its own. + +Whichever is chosen, the accept criteria must include the redirect-chain and +credential-dependent socket-close activation cases the third design lacked, and +the residual false-negative class must be documented rather than argued away. + +## Status + +Not started. #914 and #919 stay **open** with comments recording this analysis. +Neither is fixed by `260803_pr_issue_sweep`, and claiming otherwise would be +the exact overreach the audit rounds prevented. + +## #919 — the post-200 half, and why it is not a quick fix either + +#919 spent one round in the sweep unit as "the easy one": the synthetic/real +distinction is already carried on `RequestLogContext` +(`src/server/request-log.ts:97`), so the fix looked like reading a field rather +than inferring anything. The reviewer rejected that framing by finding the +decision record for the code in question. + +`consumeForInspection()` reports a synthetic `failed, 502` when the upstream +body read rejects after HTTP 200 (`src/server/relay.ts:988`, +`src/server/relay-eager.ts:256`). That was not an accident of implementation. +`devlog/_fin/260722_issue_bug_sweep/030_patch_s_sticky_502.md:85` introduced it +with this comment, still in the source today: + +```ts +// Upstream read failure after HTTP 200 (mid-stream socket reset). This is +// NOT the protocol `response.incomplete` terminal — report `failed` with a +// synthetic 502 so the account-health recorder treats it as a transient +// upstream failure instead of a success. +``` + +And its test matrix (`:162`) records the intended outcome explicitly: +`upstream read rejection (200 후 socket reset)` → `transient 실패 기록, +affinity 해제`. The behavior #919 reports as a defect is the behavior that +patch was written to produce. + +So the sweep unit's framing — "a synthetic 502 the proxy invented is not +upstream's verdict on the credential, and fixing it needs no inference" — was +wrong on both halves. `terminalSource="synthetic"` proves the proxy +manufactured the *event*; it says nothing about whether the underlying reset +was account-neutral. Both relay implementations classify it as an upstream read +failure, which is what it is. + +### What the real question is + +A received HTTP 200 does prove the credential authenticated. But account health +does not track only credential validity — it tracks transient reliability, +which is why ordinary 5xx responses count. An upstream that accepts the request +and then drops the socket mid-stream is exhibiting exactly that kind of +unreliability. + +So #919 asks for a policy change: **should post-200 transport reliability +affect account routing at all?** Arguments exist both ways. + +- For the reporter: every pool account shares the host, so rotating away from + account A does not repair a host that drops connections. The rotation costs + thread affinity and buys nothing. +- For the current behavior: if the drops correlate with one account — a + per-account rate limiter that cuts the stream rather than returning 429, say + — then rotation is precisely the right response, and the 2026-07-22 patch was + written because something real motivated it. + +Settling that needs evidence about whether mid-stream drops correlate with +accounts in practice, not a preference. Which is the same shape as #914's +resolution: separate host health from account health, and the question stops +being a coin flip. + +### Constraints for whoever takes this + +- Do not silently revert the 2026-07-22 decision. If it is reversed, say so and + say why, in the same place it was recorded. +- Any activation scenario must drive **both** relay paths — `legacy-tee` + (`src/server/relay.ts`) and `eager-relay` (`src/server/relay-eager.ts`) set + these fields separately, so a test through one proves nothing about the + other. +- Client cancellation must keep recording nothing. That guard is load-bearing + and already tested. + +## One more rejected-design lesson, from Round 3 + +Recorded here because a future implementer will otherwise rediscover it: the +outer catch does not see the whole attempt history. + +`fetchWithTransientRetry` (`src/lib/upstream-retry.ts:213-238`) returns a +transient 5xx only when it is the *final* attempt's result. When attempt one +returns 503, the helper cancels that body and retries; if a later attempt +rejects, the outer catch sees only the rejection. A genuine, attributable +upstream response arrived and left no trace at the point where the decision +gets made. + +Verified by runtime probe: `503 Response → retry rejection` ends as a rejected +`fetchWithTransientRetry`. Any future design that keys on "the promise +rejected" inherits this hole, and any acceptance criteria must include the +mixed `5xx → rejection` case on the regular path, the compact path, and the +alternate-account send. From 57f0ec5c3e763063c02bfbbc676bc74b46f2571b Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Mon, 3 Aug 2026 14:20:11 +0900 Subject: [PATCH 6/7] fix(config): trust the registry adapter only when the transport still matches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Automated review on #924 found the mirror of the defect the previous commit fixed. That one made validation too strict; this one made it too loose. A `preserveCustomDestination` registry row reused under a different endpoint keeps its own adapter at runtime — `routedProviderConfig()` honors `providerMatchesRegistryTransport()`, which returns false once the endpoint diverges. `volcengine-agent-plan` is such a row with an `openai-responses` registry adapter, so a config naming that id while pointing elsewhere with `adapter: "openai-chat"` passed validation on the registry's adapter while the Responses adapter never ran. The preference was accepted and then silently ignored. Validation now starts from `provider.adapter` unless the configured transport still matches the registry's documented one. Driven red first: the volcengine-agent-plan config loaded with source "file" before the fix and now returns "requires the openai-responses wire". 279 tests pass across the five affected files, typecheck clean. --- src/config.ts | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/config.ts b/src/config.ts index 4aef1a4d33..005b44ae3c 100644 --- a/src/config.ts +++ b/src/config.ts @@ -36,7 +36,11 @@ import { type OcxProviderConfig, } from "./types"; import { isCanonicalOpenAiForwardProvider, OPENAI_CODEX_PROVIDER_ID } from "./providers/openai-tiers"; -import { getProviderRegistryEntry, providerModelWireDefault } from "./providers/registry"; +import { + getProviderRegistryEntry, + providerMatchesRegistryTransport, + providerModelWireDefault, +} from "./providers/registry"; import { resolveOpenAiVirtualModel } from "./providers/openai-virtual-models"; import { parseDesktopProfile } from "./claude/desktop-profile"; import { isCodexReasoningEffort, modelRecordValue } from "./reasoning-effort"; @@ -769,7 +773,20 @@ export function modelPreferHostedToolsConfigError( return `${field}.${key} cannot prefer ${tool}: the model does not support it`; } } - let effectiveWire = resolveEffectiveWire(key, registry?.adapter ?? provider.adapter); + // Start from the registry adapter only when this config still points at the registry's + // documented transport. A `preserveCustomDestination` row reused under a different + // endpoint keeps its own adapter at runtime (`routedProviderConfig()` honors + // `providerMatchesRegistryTransport()`), so trusting `registry.adapter` there would + // accept a preference the Responses adapter never sees. Raised by the automated + // review on #924. + const registryTransportMatches = typeof provider.baseUrl === "string" + && providerMatchesRegistryTransport(providerName, { + baseUrl: provider.baseUrl, + adapter: provider.adapter as OcxProviderConfig["adapter"], + ...(typeof provider.authMode === "string" ? { authMode: provider.authMode as OcxProviderConfig["authMode"] } : {}), + }); + const baseWire = registryTransportMatches ? registry?.adapter ?? provider.adapter : provider.adapter; + let effectiveWire = resolveEffectiveWire(key, baseWire); const virtualWireModel = resolveOpenAiVirtualModel(providerName, key)?.wireModelId; if (virtualWireModel && virtualWireModel !== key) { effectiveWire = resolveEffectiveWire(virtualWireModel, effectiveWire); From 396cf1cef103d0cef6df6d9d14dfe3b84f6f3121 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Mon, 3 Aug 2026 14:25:52 +0900 Subject: [PATCH 7/7] fix(openai-responses): restore hosted image generation in every stripped container MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stripping walked all `additional_tools` containers; restoration targeted only the first stripped index. A request carrying two containers, each declaring an empty `image_gen` namespace and no hosted declaration anywhere, ended with the first container repaired and the second left with an empty tool list — no image capability at all. Track every stripped index and restore each one. Found by the automated review on #924 and driven red first: the second container came back as `[]` before the fix. Full suite 7575 pass, 0 fail. --- src/adapters/openai-responses.ts | 13 ++++--- tests/openai-responses-passthrough.test.ts | 41 +++++++++++++++++++++- 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/src/adapters/openai-responses.ts b/src/adapters/openai-responses.ts index c8a75b709a..bfb9dcce15 100644 --- a/src/adapters/openai-responses.ts +++ b/src/adapters/openai-responses.ts @@ -678,14 +678,14 @@ function preferConfiguredHostedTools( } let input = body.input; - let strippedAdditionalToolsIndex = -1; + const strippedAdditionalToolsIndices = new Set(); if (Array.isArray(body.input)) { let nestedChanged = false; const mappedInput = body.input.map((item, index) => { if (!isPlainObject(item) || item.type !== "additional_tools" || !Array.isArray(item.tools)) return item; const nestedTools = stripGroup(item.tools); if (nestedTools === item.tools) return item; - if (strippedAdditionalToolsIndex === -1) strippedAdditionalToolsIndex = index; + strippedAdditionalToolsIndices.add(index); nestedChanged = true; return { ...item, tools: nestedTools }; }); @@ -704,11 +704,14 @@ function preferConfiguredHostedTools( || (Array.isArray(input) && input.some(item => isPlainObject(item) && item.type === "additional_tools" && hasHostedImageGenTool(item.tools))); - if ((strippedTopLevelImageGenTool || strippedAdditionalToolsIndex >= 0) && !hasHostedImageGenDeclaration) { + if ((strippedTopLevelImageGenTool || strippedAdditionalToolsIndices.size > 0) && !hasHostedImageGenDeclaration) { if (strippedTopLevelImageGenTool && Array.isArray(tools)) { tools = [...tools, { type: HOSTED_IMAGE_GENERATION_TOOL }]; - } else if (strippedAdditionalToolsIndex >= 0 && Array.isArray(input)) { - input = input.map((item, index) => index === strippedAdditionalToolsIndex + } else if (strippedAdditionalToolsIndices.size > 0 && Array.isArray(input)) { + // Restore in EVERY container we stripped, not just the first: a request carrying + // two `additional_tools` groups would otherwise leave the later ones with no image + // capability at all. Raised by the automated review on #924. + input = input.map((item, index) => strippedAdditionalToolsIndices.has(index) && isPlainObject(item) && Array.isArray(item.tools) ? { ...item, tools: [...item.tools, { type: HOSTED_IMAGE_GENERATION_TOOL }] } diff --git a/tests/openai-responses-passthrough.test.ts b/tests/openai-responses-passthrough.test.ts index 65c1837636..9b7040c0a6 100644 --- a/tests/openai-responses-passthrough.test.ts +++ b/tests/openai-responses-passthrough.test.ts @@ -1122,7 +1122,7 @@ describe("OpenAI Responses hosted-tool name conflicts", () => { }); test("an inherited Object.prototype key is not read as a preference", () => { - // `provider.modelPreferHostedTools?.[modelId]` walks the prototype chain, so a + // `provider.modelPreferHostedTools?.[modelId]` walked the prototype chain, so a // routed model literally named `constructor` or `toString` yielded a function // and threw on `.includes` before the request was ever dispatched. const adapter = createResponsesPassthroughAdapter({ @@ -1147,6 +1147,45 @@ describe("OpenAI Responses hosted-tool name conflicts", () => { } }); + test("every stripped additional_tools container gets hosted image generation restored", () => { + // Stripping runs over all containers, but restoration originally targeted only the + // first stripped index, so a second container lost its image capability entirely. + const adapter = createResponsesPassthroughAdapter({ + ...keyedProvider, + modelPreferHostedTools: { "provider-image-model": ["image_generation"] }, + }); + const request = adapter.buildRequest({ + modelId: "provider-image-model", + context: { messages: [] }, + stream: true, + options: {}, + _rawBody: { + model: "provider-image-model", + input: [ + { + type: "additional_tools", + role: "developer", + tools: [{ type: "namespace", name: "image_gen", tools: [] }, { type: "web_search" }], + }, + { + type: "additional_tools", + role: "developer", + tools: [{ type: "namespace", name: "image_gen", tools: [] }], + }, + ], + }, + }, meta); + const body = JSON.parse(request.body) as { + input: Array<{ type: string; tools?: Array<{ type: string }> }>; + }; + const containers = body.input.filter(item => item.type === "additional_tools"); + + expect(containers).toHaveLength(2); + for (const container of containers) { + expect(container.tools).toContainEqual({ type: "image_generation" }); + } + }); + test("configured model rewrites a custom image-gen selector", () => { const adapter = createResponsesPassthroughAdapter({ ...keyedProvider,