fix(responses): let a provider declare the hosted tools it rejects - #5021
Conversation
An OpenAI-compatible Responses gateway does not necessarily accept every capability OpenAI accepts, and until now it had no way to say so. The only mechanism was UNSUPPORTED_HOSTED_TOOLS in src/responses/hosted-tool-policy.ts, a table of (model, baseUrl) predicates, so supporting a narrower destination meant shipping a proxy release that named its endpoint. The reported destination (#5002) accepts plain Responses requests and function tools but rejects hosted web_search with HTTP 400 unsupported_request. Codex forwards its hosted declaration with every request, so even "Reply exactly with OK" failed before the model answered, and the only workaround was disabling web search globally for every provider. Add unsupportedHostedTools to the provider config. stripUnsupportedHostedTools now consults it alongside the built-in table and removes denied declarations from tools, from client-loaded additional_tools, and from tool_choice before serialization. The declaration is additive: the table still covers destinations that reject a tool regardless of configuration, so a declaration can only deny more, never re-enable a known-broken pairing. Two deliberate properties. Spelling variants of one capability are aliased, so declaring web_search also denies web_search_preview; the rest of the proxy already folds that pair into a single tool, and honouring only the spelling the operator wrote would reproduce the original 400 while the config claimed to have prevented it. And the value is validated against a closed vocabulary, because the provider schema ends in .passthrough(): an unvalidated misspelling would be persisted and then match no tool, leaving the operator with the rejection the field exists to prevent and nothing explaining why. That is the codexToolMode lesson from #2106. A provider can no longer both deny a hosted tool and prefer it in modelPreferHostedTools; the denial wins at request time, so accepting the pair would silently ignore the preference. The custom-tool half of the report needs no change: supportsResponsesCustomTools already exists as a provider capability and the reporter confirmed it works. The two are independent and are denied independently.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (12)
Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review. 📝 WalkthroughWalkthroughThe PR adds ChangesHosted-tool capability declarations
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Feature · Severity of issue fixed: Medium Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ProviderConfig
participant OpenAIResponsesAdapter
participant HostedToolPolicy
participant ResponsesGateway
ProviderConfig->>OpenAIResponsesAdapter: configure unsupportedHostedTools
OpenAIResponsesAdapter->>HostedToolPolicy: check declared and built-in exclusions
HostedToolPolicy-->>OpenAIResponsesAdapter: return filtering decisions
OpenAIResponsesAdapter->>ResponsesGateway: send filtered tools and tool_choice
Merge Risk: ⚪ Minimal · up to No actionable current-head issue remains from this review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 70.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 8 files. (4 skipped: 4 unsupported.) ✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
리뷰 · 우선순위 72 / 80이 PR은 지금 이 변경이 넣는 답은 제공자 설정 키 설계에서 특히 잘 막은 두 가지는 본문이 직접 적은 그대로다. (1) 관리 왕복도 빠지지 않았다. 라인 hosted-tool-policy.ts / DECLARABLE_HOSTED_TOOL_TYPES - 닫힌 목록이 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f739213a1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| > Decision record: [ADR-0052](../decisions/ADR-0052-reasoning-and-tool-result-compatibility.md) | ||
|
|
||
| ## Declared hosted-tool denials |
There was a problem hiding this comment.
This adds the contract only to providers/chat-compat.md, although structure/INDEX.md maps the changed src/adapters/, src/config/, src/responses/, and src/server/ areas to several additional owners, including config.md, transports/responses.md, providers-and-adapters.md, and gui-and-management-api.md. Those canonical documents therefore remain stale about the new provider capability and its management/configuration behavior; update every mapped owner in this change, using links where repeating the full contract would cause drift.
AGENTS.md reference: structure/AGENTS.md:L44-L50
Useful? React with 👍 / 👎.
|
Merging with macOS legs outstanding, and recording why rather than leaving it implicit. At this exact head the full Linux suite (test 1/4 through 4/4), This change is platform-neutral, so waiting on a queue that is both saturated and known-unreliable would delay the work without adding information. The evidence that governs the release is not per-PR macOS legs; it is the full-platform Stating the boundary plainly: this is merged on Linux, gates and cross-platform smoke evidence at its exact head, with macOS coverage deferred to the candidate run rather than claimed here. |
1 participant
Summary
An OpenAI-compatible Responses gateway does not necessarily accept every capability OpenAI accepts, and until now it had no way to say so. The only mechanism was
UNSUPPORTED_HOSTED_TOOLSinsrc/responses/hosted-tool-policy.ts, a table of(model, baseUrl)predicates, so supporting a narrower destination meant shipping a proxy release that named its endpoint.The reported destination accepts plain Responses requests and
functiontools but rejects hostedweb_searchwith HTTP 400unsupported_request. Codex forwards its hosted declaration with every request, so evenReply exactly with OKfailed before the model answered:The only workaround was
web_search="disabled", which turns hosted web search off for every provider including the ones that support it.This adds
unsupportedHostedToolsto the provider config.stripUnsupportedHostedToolsnow consults it alongside the built-in table and removes denied declarations fromtools, from client-loadedadditional_tools, and fromtool_choicebefore serialization. The reporter's own suggested configuration now works as written, on a provider the registry has never heard of:{ "providers": { "agent-space": { "adapter": "openai-responses", "supportsResponsesCustomTools": false, "unsupportedHostedTools": ["web_search", "web_search_preview"] } } }The declaration is additive, not a replacement. The built-in table still covers destinations that reject a tool regardless of configuration, so an operator who never heard of the field stays protected, and a declaration can only deny more — it can never re-enable a known-broken pairing such as
grok-4.6on OpenCode Go.Two deliberate design choices, both called out because they are the kind of thing that is easier to review than to rediscover:
web_searchalso deniesweb_search_preview. The rest of the proxy already folds that pair into a single tool — the parser maps both to one name, Chat ingress accepts them together, and the canonical-field strip lists them in onetoolTypesset. Honouring only the spelling the operator happened to write would reproduce the original 400 while the config claimed to have prevented it..passthrough(), so an unvalidatedweb_serchwould be accepted, persisted, and then match no tool — leaving the operator with the exact upstream rejection this field exists to prevent, and nothing explaining why. This is thecodexToolModelesson recorded atsrc/config/schema/leaf-validators.tsfor Feature: per-provider opt-out of code_mode_only tool mode for routed models (deepseek-v4-flash: undeclared exec_command aborts stream, reconnect then fails reasoning replay with DeepSeek 400) #2106.A provider can also no longer both deny a hosted tool and prefer it in
modelPreferHostedTools. The denial wins at request time, so accepting the pair would silently ignore the preference.The custom-tool half of the report needs no code change:
supportsResponsesCustomToolsalready exists as a provider capability with registry inheritance and editor exposure, and the reporter confirmed it works. The two capabilities are independent and are denied independently; a gateway that rejects both sets both.Scope note: this is provider-wide, matching the reported need and the
supportsResponsesCustomToolsprecedent. The repository'smodel*convention is available if a per-model denial is ever required; no registry entry declares the new field, so nothing ships dead.Closes #5002
Verification
Local verification was not run, because this lane forbids it. No test, focused test, typecheck, build, install, or
ocxinvocation was executed. Hosted CI on this branch is the executable verification for this change.Static verification performed instead:
isHostedToolUnsupportedForModel.src/adapters/openai-responses/tool-schema.tspasses the expanded set;src/config/schema/leaf-validators.tsstill calls it with two arguments, which the new optional parameters preserve.stripUnsupportedHostedTools(src/adapters/openai-responses/passthrough.ts) already passes the full provider object, so only thePickneeded widening.providerManagementConfigError,PROVIDER_CONFIG_FIELD_POLICY(satisfies Record<keyof OcxProviderConfig, ...>, so omitting it would fail typecheck), and theprovider-routesPATCH handler plus read DTO.normalizeFunctionToolSchemareturns non-functiontools unchanged, and the xAI, Muse, and OpenCode Go normalizers are all destination-gated.DECLARABLE_HOSTED_TOOL_TYPEScovers every inboundhostedToolType; the new test asserts that superset relation against the now-exportedHOSTED_TOOL_TYPESrather than a copied list, so the two cannot drift silently.Ratchet and layout, checked because both have broken other pull requests this cycle:
tests/responses/openai-responses-passthrough.test.ts, sits at exactly its recorded cap (4,809 lines intests/fixtures/file-size-baseline.json) and the cap only moves downward, so appending there would have failed the ratchet for every later PR. The regression is a sibling file instead, followingd3ca5522db, test(server): hold the prototype-named override case in a sibling file #5011 and test: hold the newest catalog and provider cases in sibling files #5018.responses-hosted-tool-declaration.test.tsis registered in bothscripts/test-layout/layout.json(explicit) andtests/fixtures/test-layout-expected.json; the two maps were compared key-by-key and are exactly equal at 1,356 entries.src/server/management/provider-routes.ts, now 1,860 lines) stays under the 2,000-line threshold for unbaselined files.Union-with-
devcheck, since a green PR is not a green merge result: this change adds one provider config key and one exported set. It touches no roster, no hard-coded count, no exhaustive adapter or provider map, no locale catalog, and no generated or golden file.#4871landed the same provider-capability shape across the same files earlier today and is an ancestor of this branch, so the two patterns agree rather than collide.Checklist
Docs: the field is documented in
docs-site/src/content/docs/reference/configuration/providers.mdand the capability boundary instructure/providers/chat-compat.md, whose manifest entry ownssrc/adapters/andsrc/responses/. Security: the change only ever removes a tool declaration from an outbound body. It adds no credential handling, no logging of request contents, and no network or auth behaviour, and it cannot widen a caller's tool selection.Summary by CodeRabbit
New Features
Documentation
Tests