fix(grpc-harmony): dispatch image_generation as function tool for gpt-oss (R6.8) - #1368
Merged
Merged
Conversation
…-oss (R6.8) gpt-oss was not trained to emit `image_generation_call` as a native hosted-tool channel tag — per the openai-harmony spec, gpt-oss only knows `web_search_preview`, `web_search`, `code_interpreter` / `container`, `file_search` as builtin tools. Advertising `image_generation` in the harmony prompt's builtin-tools preamble produces undefined behavior: the model hallucinates a malformed call, ignores the advertisement, or emits `reasoning + message` instead of an `image_generation_call`, and the registered `image_generation` MCP server receives zero dispatches — the concrete failure R6.5 (#1365) documented on the harmony lane. R6.3 (#1359) wired the `image_generation_call` streaming-event plumbing for the harmony path but left gpt-oss with no way to emit the underlying tool call. This PR closes that integration gap. ## Approach Translate the hosted `image_generation` tool into a *function tool* that gpt-oss CAN emit, rendered into the developer-message custom- tool section. gpt-oss then emits `{"name": "image_generation", "arguments": {"prompt": "..."}}` on the commentary channel, and the shared MCP dispatch path — keyed on the exposed function-tool name against the registered `image_generation` MCP server — materializes the result as an `image_generation_call` output item (R6.1 plumbing). Scope discipline: R6.8 only touches the `image_generation` path. Generalizing to a hosted-tool → function-tool framework for the other non-native hosted tools (`shell`, `computer`, `local_shell`, `apply_patch`) is follow-up work (R6.9). ## What changed ### `model_gateway/src/routers/grpc/harmony/builder.rs` - **Drop `image_generation` from `BUILTIN_TOOLS`** — honoring the invariant PR #1353 proposed. `shell` stays for now, out of R6.8 scope. The new doc comment explains the gpt-oss-native set and why hosted tools outside it must route through the custom-tool path instead. - **`ToolLike for ResponseTool::is_custom()`** now returns `true` for `ImageGeneration`, so `has_custom_tools()` keeps the `commentary` channel in the system message and emits the developer-message tools section for image-generation-only requests. - **`ToolLike for ResponseTool::to_tool_description()`** synthesizes a JSON-schema function-tool descriptor via a new `image_generation_tool_description()` helper — `prompt` required, plus mirrored pass-through configuration fields from `ImageGenerationTool` (`background`, `model`, `output_format`, `quality`, `size`). - **`build_developer_message()` deduplicates function tools by name.** The MCP loop extends `request.tools` with an MCP-derived `ResponseTool::Function { name: "image_generation" }` once the MCP session resolves the hosted tool (see `execute_with_mcp_loop` in `harmony/responses/non_streaming.rs`). Without deduplication the rendered `namespace functions { … }` would carry two identically- named entries inside the developer message and confuse gpt-oss about which signature to follow. - **New `#[cfg(test)] mod tests`** with six regression tests: * `image_generation_is_not_a_builtin_tool` — locks PR #1353's invariant so future changes can't silently re-add the tool. * `response_tool_image_generation_is_custom` — exercises the `is_custom()` / `is_builtin()` flag flip. * `image_generation_tool_description_exposes_required_prompt` — asserts the synthesized JSON-schema shape (`prompt` required, `size`/`quality`/`background`/`output_format` pass-through). * `build_from_responses_renders_image_generation_as_function_tool` — end-to-end harmony-prompt decode: asserts `type image_generation = (…)` and `namespace functions` appear. * `has_custom_tools_true_for_image_generation_only_request` — catches regressions where the tool would be silently dropped. * `dedupes_duplicate_function_tool_names_from_mcp_loop` — after the MCP loop appends a same-named `ResponseTool::Function`, the rendered prompt still carries exactly one `image_generation` signature. ### `model_gateway/src/routers/grpc/common/responses/utils.rs` - **`ensure_mcp_connection()` now recognizes `ResponseTool::ImageGeneration`** alongside `WebSearchPreview` and `CodeInterpreter`. Without this arm the gating short-circuit returned `(false, Vec::new())`, the MCP loop was never entered, and the registered `image_generation` MCP server received zero dispatches on both the harmony and regular lanes (R6.5 #1365's documented failure). The helper is shared by harmony and regular, so this single change fixes both. ## Why - gpt-oss can only emit `image_generation_call` reliably if it sees the tool as a function in its developer-message tools section — the channel tag is a training artifact the gateway can't fake. - Preserving PR #1353's `BUILTIN_TOOLS` invariant keeps the architectural contract clean: that array stays scoped to gpt-oss's trained hosted-tool set, and per-worker hosted-tool capability flags (R0 follow-up) will replace it with a dynamic, model-aware gate. - The deduplication belongs in the harmony builder rather than the MCP loop because the harmony path is where the two tool sources (caller's `ImageGeneration` + MCP session's `Function(image_generation)`) first collide in prompt rendering; any caller that pre-populates `tools` inherits the fix for free. ## How it works end-to-end 1. Caller sends `tools: [{type: "image_generation", size: "512x512"}]`. 2. `ensure_mcp_connection` sees ImageGeneration, triggers the MCP loop, and `ensure_request_mcp_client` resolves the registered `image_generation` MCP server binding. 3. `execute_with_mcp_loop` appends the MCP-exposed `ResponseTool::Function { name: "image_generation" }` to the request's tools. 4. `HarmonyBuilder::build_from_responses` renders the developer message with a single `image_generation` function-tool entry (dedup keeps the synthesized schema from the original ImageGeneration declaration). 5. gpt-oss emits `{"name": "image_generation", "arguments": {"prompt": "a cat"}}` on the commentary channel. 6. The harmony responses loop partitions calls via `session.has_exposed_tool("image_generation")` → routed to `execute_mcp_tools`. 7. MCP session dispatches to the registered server, which returns the base64 image. 8. `to_response_item()` produces a `ResponseOutputItem::ImageGenerationCall` (R6.1 plumbing), and streaming events fire from `ResponseFormat::ImageGenerationCall` (R6.3 wiring). ## Test plan ### New unit tests `cargo test -p smg --lib routers::grpc::harmony::builder::tests` → 6/6 pass. ### Regression `cargo test -p smg --lib` → 639/643 pass (4 pre-existing ignored, 0 failed). ### Gates * `cargo fmt --all` — clean * `cargo check -p openai-protocol -p smg-mcp -p smg --lib --tests --benches` — clean * `cargo clippy -p openai-protocol -p smg-mcp -p smg --lib --tests --benches -- -D warnings` — clean ### E2E R6.5 (#1365) added harmony/regular lane tests behind `@pytest.mark.skip_for_runtime`. Un-skipping those after R6.8 merges will exercise the full gpt-oss → MCP → `image_generation_call` flow end-to-end. ## Follow-up (R6.9) The hosted-tool → function-tool translation pattern generalizes to the other non-native hosted tools (`shell`, `computer`, `computer_use_preview`, `local_shell`, `apply_patch`). R6.9 should factor the R6.8 synthesis into a reusable per-tool-kind helper, introduce per-worker hosted-tool capability flags, and apply the pattern to the remaining hosted tools so every tool gpt-oss wasn't trained on still has a dispatch path. Refs: R6.3 (#1359), R6.5 (#1365), PR #1353 (BUILTIN_TOOLS invariant) Co-authored-by: Tingting Zhou <zhoutt96@gmail.com> Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
github-actions
Bot
added
grpc
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughExpanded MCP routing for image_generation: ImageGeneration is no longer short-circuited as a native builtin and is treated as a custom/function tool with a synthesized tool description; request tools are stripped when MCP exposes an image_generation function so only MCP-routed tools remain discoverable. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HarmonyBuilder
participant MCP_Loop
participant McpToolSession
participant Dispatcher
Client->>HarmonyBuilder: submit ResponsesRequest (includes image_generation tool)
HarmonyBuilder->>MCP_Loop: detect MCP session for image_generation
MCP_Loop->>McpToolSession: establish MCP tools / inject function tool
MCP_Loop->>HarmonyBuilder: return injected MCP tool descriptions
HarmonyBuilder->>HarmonyBuilder: strip original image_generation tool from request.tools
HarmonyBuilder->>Dispatcher: dispatch request (only MCP-exposed function tools remain)
Dispatcher->>Client: return responses/events
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed both changed files end-to-end. The approach of demoting image_generation from a gpt-oss native builtin to a synthesized function tool is correct — the model was never trained to emit image_generation_call as a channel tag, so function-call dispatch through MCP is the right path. The deduplication logic, MCP connection gating, and test coverage are all solid. No issues found.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![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: 7c50201ac9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…ch (R6.8 review) Address Codex review comment on PR #1368: the R6.8 synthesized function-tool description hard-codes the name `image_generation`, but the MCP dispatch path routes calls via `McpToolSession::has_exposed_tool` keyed on the MCP server's `builtin_tool_name`. If a deployment configures `builtin_type: image_generation` with a non-canonical `builtin_tool_name` (e.g. `generate_image`), the developer message advertises both names and calls to the synthesized `image_generation` fall through to the unresolved function-call path instead of routing to the MCP server. ## What changed * `model_gateway/src/routers/grpc/harmony/responses/common.rs` (new helper + import). Added `strip_image_generation_from_request_tools(request, session)`: once the MCP session has exposed a tool with `ResponseFormat::ImageGenerationCall`, remove any `ResponseTool::ImageGeneration` entries from the request so the harmony builder does NOT synthesize a duplicate / conflicting `image_generation` function-tool schema alongside the MCP-exposed Function. Keeps the advertisement single-source (the MCP-exposed name), so every call the model can emit is a name the session will dispatch. Imported `ResponseTool` via `openai_protocol::responses` and `ResponseFormat` via `smg_mcp`. * `model_gateway/src/routers/grpc/harmony/responses/non_streaming.rs` (call site + import). After the existing MCP-tool extension in `execute_with_mcp_loop`, invoke the new strip helper with the live session. Also imported it from `super::common`. * `model_gateway/src/routers/grpc/harmony/responses/streaming.rs` (call site + import). Mirror change in the streaming MCP loop (`execute_mcp_tool_loop_streaming`) so both paths behave consistently. ## Why * No-MCP fallback (e.g. local dev without an image_generation MCP server configured): the R6.8 synthesis still advertises `image_generation` as a function tool, so gpt-oss can emit the call and the caller sees it as an unresolved function call rather than the pre-R6.8 silent drop. The helper is a no-op in this branch because `session.mcp_tools()` carries no `ImageGenerationCall`-formatted entries. * Canonical MCP deployment (`builtin_tool_name: image_generation`): the strip prevents the prior commit's dedup from being the only safety net — the builder now sees exactly one advertisement (the MCP-exposed Function), and gpt-oss always emits `image_generation`, which matches `has_exposed_tool` for correct dispatch. * Non-canonical MCP deployment (`builtin_tool_name: generate_image` or similar): the strip removes the `ResponseTool::ImageGeneration` tag so the builder never synthesizes a dangling `image_generation` entry. gpt-oss sees only `generate_image` in the developer-message tools section, emits `generate_image`, and dispatch routes correctly through MCP. ## Test plan * `cargo test -p smg --lib routers::grpc::harmony::builder::tests` — 6/6 pass. All existing regression tests (including `dedupes_duplicate_function_tool_names_from_mcp_loop`) still hold; the strip now handles the duplicate-by-name case at a layer above the builder but the builder's dedup remains a defensive safety net. * `cargo fmt --all` — clean. * `cargo clippy -p openai-protocol -p smg-mcp -p smg --lib --tests --benches -- -D warnings` — clean. The e2e coverage in R6.5 (#1365) will exercise the canonical deployment end-to-end once the harmony skip is removed. Refs: #1368 (the R6.8 PR), Codex automated review comment on `model_gateway/src/routers/grpc/harmony/builder.rs:226` (P1). Co-authored-by: Tingting Zhou <zhoutt96@gmail.com> Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ab415f6e7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
1 of 2 tasks
This was referenced Apr 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Problem
[R6.3 #1359] wired the
image_generation_callstreaming-event plumbing for gpt-oss via the harmony pipeline, but left gpt-oss with no way to emit the underlying tool call.gpt-oss was not trained to emit
image_generation_callas a native hosted-tool channel tag — per the openai-harmony spec, gpt-oss only knowsweb_search_preview,web_search,code_interpreter/container,file_searchas builtin tools. Advertisingimage_generationin the harmony prompt's builtin-tools preamble produces undefined behavior: the model hallucinates a malformed call, ignores the advertisement, or emitsreasoning + messageinstead of animage_generation_call.Net effect on the harmony lane of R6.5 (#1365):
reasoning + messageinstead ofimage_generation_call.Refs:
BUILTIN_TOOLSto the gpt-oss-native tool set (honored as an invariant here).Solution
Translate the hosted
image_generationtool into a function tool that gpt-oss CAN emit, rendered into the developer-message custom-tool section. gpt-oss then emits{"name": "image_generation", "arguments": {"prompt": "..."}}on the commentary channel, and the shared MCP dispatch path — keyed on the exposed function-tool name against the registeredimage_generationMCP server — materializes the result as animage_generation_calloutput item (R6.1 plumbing).Scope discipline: R6.8 only touches the
image_generationpath. Generalizing to a hosted-tool → function-tool framework for the other non-native hosted tools (shell,computer,local_shell,apply_patch) is follow-up work — flagged below as R6.9.Changes
model_gateway/src/routers/grpc/harmony/builder.rs"image_generation"fromBUILTIN_TOOLS(honoring the invariant PR fix(harmony): shrink BUILTIN_TOOLS to the gpt-oss-native tool set #1353 proposed)."shell"stays for now — out of R6.8 scope. The new doc comment explains the gpt-oss-native set and why hosted tools outside it must route through the custom-tool path instead.ToolLike for ResponseTool::is_custom()now returnstrueforImageGeneration, sohas_custom_tools()keeps thecommentarychannel in the system message and emits the developer-message tools section for image-generation-only requests.ToolLike for ResponseTool::to_tool_description()synthesizes a JSON-schema function-tool descriptor via a newimage_generation_tool_description()helper —promptrequired, plus mirrored pass-through configuration fields fromImageGenerationTool(background,model,output_format,quality,size).build_developer_message()deduplicates function tools by name. The MCP loop extendsrequest.toolswith an MCP-derivedResponseTool::Function { name: "image_generation" }once the MCP session resolves the hosted tool (seeexecute_with_mcp_loopinharmony/responses/non_streaming.rs). Without deduplication the renderednamespace functions { … }would carry two identically-named entries and confuse gpt-oss about which signature to follow.#[cfg(test)] mod testswith six regression tests locking the contract (see Test plan below).model_gateway/src/routers/grpc/common/responses/utils.rsensure_mcp_connection()now recognizesResponseTool::ImageGenerationalongsideWebSearchPreviewandCodeInterpreter. Without this arm the gating short-circuit returned(false, Vec::new()), the MCP loop was never entered, and the registeredimage_generationMCP server received zero dispatches on both the harmony and regular lanes — the concrete failure R6.5 (test(e2e): add mock MCP server + image_generation integration tests (R6.5) #1365) documented. The helper is shared by harmony and regular, so this single change benefits both.How it works end-to-end
tools: [{type: "image_generation", size: "512x512"}].ensure_mcp_connectionseesImageGeneration, triggers the MCP loop, andensure_request_mcp_clientresolves the registeredimage_generationMCP server binding.execute_with_mcp_loopappends the MCP-exposedResponseTool::Function { name: "image_generation" }to the request's tools.HarmonyBuilder::build_from_responsesrenders the developer message with a singleimage_generationfunction-tool entry (dedup keeps the synthesized schema from the originalImageGenerationdeclaration).{"name": "image_generation", "arguments": {"prompt": "a cat"}}on the commentary channel.session.has_exposed_tool("image_generation")→ routed toexecute_mcp_tools.to_response_item()produces aResponseOutputItem::ImageGenerationCall(R6.1 plumbing), and streaming events fire fromResponseFormat::ImageGenerationCall(R6.3 wiring).Test plan
New unit tests (
model_gateway/src/routers/grpc/harmony/builder.rs::tests)image_generation_is_not_a_builtin_tool— PR fix(harmony): shrink BUILTIN_TOOLS to the gpt-oss-native tool set #1353'sBUILTIN_TOOLSinvariant holds.response_tool_image_generation_is_custom—is_custom()/is_builtin()flag change.image_generation_tool_description_exposes_required_prompt— synthesized JSON-schema shape (promptrequired,size/quality/background/output_formatpass-through).build_from_responses_renders_image_generation_as_function_tool— end-to-end harmony-prompt decode assertstype image_generation = (…)andnamespace functionsappear.has_custom_tools_true_for_image_generation_only_request—has_custom_tools()returns true for image-generation-only requests.dedupes_duplicate_function_tool_names_from_mcp_loop— after the MCP loop appends a same-namedResponseTool::Function, the rendered prompt carries exactly oneimage_generationentry.Gates (all clean)
cargo fmt --allcargo check -p openai-protocol -p smg-mcp -p smg --lib --tests --benchescargo test -p smg --lib routers::grpc::harmony::builder::tests— 6/6 passcargo test -p smg --lib— 639/643 pass (4 pre-existing ignored, 0 failed)cargo clippy -p openai-protocol -p smg-mcp -p smg --lib --tests --benches -- -D warningsE2E coverage
R6.5 (#1365) added the harmony/regular lane tests (behind
@pytest.mark.skip_for_runtime). Un-skipping those after R6.8 merges will exercise the full gpt-oss → MCP →image_generation_callflow end-to-end.Follow-up (R6.9)
The hosted-tool → function-tool translation pattern generalizes to the other non-native hosted tools (
shell,computer,computer_use_preview,local_shell,apply_patch). R6.9 should factor the R6.8 synthesis into a reusable per-tool-kind helper, introduce per-worker hosted-tool capability flags, and apply the pattern to the remaining hosted tools so every tool gpt-oss wasn't trained on still has a dispatch path.Checklist
BUILTIN_TOOLSdoc comment explain the invariant)Summary by CodeRabbit
Improvements
Tests