feat(mcp): hide internal self-brought MCP details from final responses#1061
Conversation
zhoug9127
requested review from
CatherineSue,
key4ng and
slin1237
as code owners
github-actions
bot
added
documentation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded an Changes
Sequence DiagramsequenceDiagram
autonumber
participant Client as Client
participant ToolLoop as Tool Loop
participant Session as McpToolSession
participant Orchestrator as McpOrchestrator
participant Response as Response Utils
Client->>ToolLoop: start tool execution (with MCP bindings)
ToolLoop->>Session: query binding/server label visibility
Session->>Orchestrator: request internal server snapshot
Orchestrator-->>Session: return internal server set
alt server/label is internal
Session-->>ToolLoop: mark binding internal
ToolLoop->>ToolLoop: skip injecting mcp_list_tools and filter call items
else visible server
Session-->>ToolLoop: mark binding visible
ToolLoop->>ToolLoop: inject MCP metadata and call items
end
ToolLoop->>Response: restore_original_tools(resp, original_body, Some(session))
Response->>Session: call is_internal_* helpers
Response->>Response: strip internal tools/output and rebuild client-visible tools
Response-->>Client: deliver cleaned response
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
|
Hi @zhoug9127, the DCO sign-off check has failed. All commits must include a To fix existing commits: # Sign off the last N commits (replace N with the number of unsigned commits)
git rebase HEAD~N --signoff
git push --force-with-leaseTo sign off future commits automatically:
|
|
Hi @zhoug9127, this PR has merge conflicts that must be resolved before it can be merged. Please rebase your branch: git fetch origin main
git rebase origin/main
# resolve any conflicts, then:
git push --force-with-lease |
mergify
bot
added
the
needs-rebase
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an internal flag for MCP servers, allowing them to be hidden from client-visible outputs such as tool lists and metadata. The implementation includes updates to the configuration schema, orchestrator, and response processing logic to ensure internal server details and their tool executions are stripped before the final response is sent. Feedback identifies a potential data leakage risk where certain tool types were omitted from the filtering logic and provides a refactored suggestion. Additionally, a performance improvement is recommended for the internal server name lookup process to reduce allocations and string cloning on hot paths.
![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: 2e61bb1368
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/reference/mcp-internal-servers.md`:
- Around line 6-20: The docs are ambiguous about the visibility scope of the
servers "internal: true" flag; update the text to explicitly state that
"internal: true" currently only applies to self-provided MCP servers (entries
under servers: with internal: true) and affects final assembled non-streaming
MCP responses, and does not hide or apply to streaming outputs or builtin-routed
MCP results. Locate the section showing the sample YAML (the servers: - name:
internal-memory / internal: true example) and add a clear sentence clarifying
that scope and the exceptions (streaming and builtin-routed outputs remain
visible) so readers won’t infer broader concealment.
In `@model_gateway/src/routers/openai/mcp/tool_loop.rs`:
- Around line 780-784: The current is_internal_mcp_response_item function treats
any item with a server_label as internal and thus hides builtin routed outputs;
modify is_internal_mcp_response_item so it first checks the item's type (e.g.,
item.get("type").and_then(|v| v.as_str())) and if the type matches builtin call
names ("web_search_call", "code_interpreter_call", "file_search_call") it
returns false (keep visible), otherwise fall back to the existing server_label
check using session.is_internal_server_label(server_label); update the function
body accordingly.
In `@model_gateway/src/routers/openai/responses/utils.rs`:
- Around line 324-335: The match arm that checks item type only handles
"function_call" but misses "function_tool_call", allowing internal tool names to
leak; update the pattern matching in the match block that inspects
item.get("type")/item.get("name")/item.get("server_label") to treat
"function_tool_call" the same as "function_call" (i.e., call
session.is_internal_tool(name) when name is present, and fallback to
session.is_internal_server_label(server_label) when server_label is present) so
both variants are redacted using the existing session.is_internal_tool and
session.is_internal_server_label checks.
In `@model_gateway/tests/common/mock_mcp_server.rs`:
- Around line 210-263: The start/url/stop/Drop implementations in
MockFailingMCPServer duplicate MockMCPServer lifecycle logic; extract a shared
harness (e.g., a MockServerHarness struct or a MockServer trait) that
encapsulates TcpListener binding, port storage, axum serve spawn, url(), stop()
async behavior and Drop abort logic, then have MockFailingMCPServer and
MockMCPServer delegate to or compose that harness (move the shared
StreamableHttpService construction into per-server factory functions while
reusing the harness for lifecycle management).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9801581f-4c65-456d-91a2-e770d554be4f
📒 Files selected for processing (14)
crates/mcp/src/core/config.rscrates/mcp/src/core/orchestrator.rscrates/mcp/src/core/pool.rscrates/mcp/src/core/proxy.rscrates/mcp/src/core/session.rsdocs/reference/mcp-internal-servers.mdmodel_gateway/src/routers/mcp_utils.rsmodel_gateway/src/routers/openai/mcp/tool_loop.rsmodel_gateway/src/routers/openai/responses/non_streaming.rsmodel_gateway/src/routers/openai/responses/streaming.rsmodel_gateway/src/routers/openai/responses/utils.rsmodel_gateway/tests/api/responses_api_test.rsmodel_gateway/tests/common/mock_mcp_server.rsmodel_gateway/tests/mcp_test.rs
zhoug9127
force-pushed
the
feat/mcp-internal-server-privacy
branch
from
2e61bb1 to
aab2524
Compare
Signed-off-by: zhoug9127 <daisy.zhou@oracle.com>
Signed-off-by: zhoug9127 <daisy.zhou@oracle.com>
zhoug9127
force-pushed
the
feat/mcp-internal-server-privacy
branch
from
aab2524 to
f3a7f54
Compare
mergify
bot
removed
the
needs-rebase
zhoug9127
force-pushed
the
feat/mcp-internal-server-privacy
branch
from
f3a7f54 to
2d6a4a3
Compare
Signed-off-by: zhoug9127 <daisy.zhou@oracle.com>
Signed-off-by: zhoug9127 <daisy.zhou@oracle.com>
zhoug9127
force-pushed
the
feat/mcp-internal-server-privacy
branch
3 times, most recently
from
dff1539 to
e6a2502
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6a2502e26
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/mcp/src/core/orchestrator.rs`:
- Around line 926-943: The current internal_server_names() returns every
internal server in the deployment causing request-scoped dynamic servers to be
mis-classified; update it to compute internals from the active McpServerBinding
set (or accept a &[McpServerBinding] parameter) and only return names that
appear in those bindings, i.e. build the HashSet from binding.config.name for
bindings where binding.config.internal is true (or intersect the existing global
set with the active bindings' names) so McpToolSession::new-based final-response
stripping uses the request's active bindings and not the global server list.
In `@model_gateway/src/routers/openai/responses/streaming.rs`:
- Line 480: The call to restore_original_tools(&mut final_response,
ctx.original_request, ctx.session) is putting the streaming response.completed
SSE into the internal-hiding scope by passing ctx.session; remove the session
from the streaming path so the client-visible SSE remains unhidden (i.e., call
restore_original_tools with only final_response and ctx.original_request or pass
None/NoSession), and ensure only the persistence path that stores the response
calls the function that accepts ctx.session (or separately calls
send_final_response_event with the session) so internal MCP details are only
hidden when persisting; update usages of
restore_original_tools/send_final_response_event accordingly to keep streaming
output visible.
In `@model_gateway/src/routers/openai/responses/utils.rs`:
- Around line 254-269: The current branch removes resp["tools"] when
restored_tools is empty but fails to account for ResponseTool::Function entries
(response_tool_to_value returns None for functions), which causes surviving user
function tools to be deleted; update the condition so you only remove "tools"
when there are no user function tools present: when restored_tools.is_empty()
compute had_restorable_original_tool as before AND check original_tools for any
ResponseTool::Function (e.g., original_tools.iter().any(|t| matches!(t,
ResponseTool::Function(_))) or a helper like response_tool_is_function), and
only remove the "tools" field if there are no function tools and no restorable
tools; also add a regression test that sends a mixed request containing a normal
function tool and an internal MCP tool to ensure the function tool remains in
resp["tools"] after strip_internal_mcp_tools().
In `@model_gateway/tests/common/mock_mcp_server.rs`:
- Around line 33-59: The test server's start() and stop() use fixed
tokio::time::sleep and spawn a detached task that may panic (losing serve
errors) — replace those sleeps by implementing a deterministic readiness and
shutdown handshake: in start() have the task send a Result via a
tokio::sync::oneshot::Sender (or a tokio::sync::watch) indicating whether
axum::serve successfully started (capture the serve error instead of .expect),
and wait on the receiver before returning Ok so start() only returns after
successful bind; for stop() use a graceful shutdown signal (e.g., a
tokio::sync::watch or oneshot shutdown trigger wired into
axum::Server::with_graceful_shutdown) and await the server task completion
instead of sleeping/abort; update references to server_handle, port(), url(),
start(), and stop() accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6a416657-2f68-435d-ba16-775421e7c73c
📒 Files selected for processing (14)
crates/mcp/src/core/config.rscrates/mcp/src/core/orchestrator.rscrates/mcp/src/core/pool.rscrates/mcp/src/core/proxy.rscrates/mcp/src/core/session.rsdocs/reference/mcp-internal-servers.mdmodel_gateway/src/routers/mcp_utils.rsmodel_gateway/src/routers/openai/mcp/tool_loop.rsmodel_gateway/src/routers/openai/responses/non_streaming.rsmodel_gateway/src/routers/openai/responses/streaming.rsmodel_gateway/src/routers/openai/responses/utils.rsmodel_gateway/tests/api/responses_api_test.rsmodel_gateway/tests/common/mock_mcp_server.rsmodel_gateway/tests/mcp_test.rs
|
Nice work on hiding internal MCP details! For teams implementing MCP servers with sensitive internals, our meridian-mcp-deploy can help configure proper isolation and security settings. The meridian-tooling-guide covers MCP security patterns including internal server configurations. |
Signed-off-by: zhoug9127 <zhoug9127@gmail.com>
|
Hi @zhoug9127, this PR has merge conflicts that must be resolved before it can be merged. Please rebase your branch: git fetch origin main
git rebase origin/main
# resolve any conflicts, then:
git push --force-with-lease |
mergify
bot
added
the
needs-rebase
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0edec81f37
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/mcp/src/core/session.rs`:
- Around line 79-82: The current session-level builtin_server_keys causes all
tools on a server with any builtin routed tool to be treated as builtin; instead
compute a per-tool "is_builtin_routed" during binding construction and remove
reliance on builtin_server_keys in the label logic. Update where
ExposedToolBinding objects are created to consult the configured builtin tool
binding and set an is_builtin_routed flag (use this flag name) for that binding,
change any uses of builtin_server_keys/internal_server_keys-based checks
(including in the label helper and in build_transformed_mcp_call_item filtering
paths) to use binding.is_builtin_routed, and ensure builtin responses (which
omit server_label) remain unaffected by server-wide builtin sets; this keeps
internal_server_keys for internal-only classification but scopes builtin
classification to each ExposedToolBinding.
In `@model_gateway/src/routers/openai/responses/utils.rs`:
- Around line 369-391: The current is_internal_mcp_tool_value only inspects a
top-level "name" field and misses tools shaped like
{"type":"function","function":{"name":"..."}}, so replace the direct top-level
name lookup with the existing extractor function_tool_name(tool) when handling
the (Some("function"), ...) arm; call function_tool_name(tool) to obtain the
function name, then check session.is_internal_tool(name) &&
!user_function_names.contains(name) (keeping the MCP server_label arm
unchanged), ensuring the function-path correctly identifies and strips
function-shaped tool entries.
- Around line 436-447: The current arms in build_incomplete_response that match
Some("function_call") and Some("function_tool_call") only check
session.is_internal_non_builtin_tool(name) and therefore can still leak raw
traces for builtin-backed internal tools; update each predicate to also exclude
builtin-backed tools by checking the builtin flag (e.g., ensure
!session.is_internal_builtin_tool(name) or !session.is_builtin_tool(name) in
addition to session.is_internal_non_builtin_tool(name)) so the final condition
becomes something like: name is an internal non-builtin tool AND not a
builtin-backed/internal-builtin tool AND not in user_function_names.
In `@model_gateway/tests/common/mock_mcp_server.rs`:
- Around line 336-341: Update the test_mock_failing_server_startup test to
exercise the failing-tool path: after starting
MockFailingMCPServer::start("marker") and asserting port/url, instantiate an
RMCP client pointed at server.url(), call the brave_web_search RPC (or client
method) that triggers the tool path, capture the returned raw error, and assert
the error string contains the supplied "marker"; finally stop the server. Use
the existing MockFailingMCPServer, test_mock_failing_server_startup, and the
RMCP client's brave_web_search call to locate where to add the connection,
invocation, and assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 78d4574f-7c98-4d51-8f35-4d5b4ac6eef0
📒 Files selected for processing (7)
crates/mcp/src/core/config.rscrates/mcp/src/core/session.rsmodel_gateway/src/routers/mcp_utils.rsmodel_gateway/src/routers/openai/mcp/tool_loop.rsmodel_gateway/src/routers/openai/responses/streaming.rsmodel_gateway/src/routers/openai/responses/utils.rsmodel_gateway/tests/common/mock_mcp_server.rs
Signed-off-by: Daisy Zhou <zhoug9127@gmail.com>
Signed-off-by: Daisy Zhou <zhoug9127@gmail.com>
mergify
bot
removed
the
needs-rebase
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e0c60b020
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
model_gateway/src/routers/openai/responses/utils.rs (1)
434-445:⚠️ Potential issue | 🟠 MajorHide raw builtin-backed function traces as well.
These branches still use
is_internal_non_builtin_tool, so an internal builtin server can leak the rawfunction_call/function_tool_callitem if that artifact survives response assembly. The transformed builtin item can stay visible; the internal trace should not.🔒 Suggested fix
Some("function_call") => item .get("name") .and_then(|value| value.as_str()) .is_some_and(|name| { - session.is_internal_non_builtin_tool(name) && !user_function_names.contains(name) + session.is_internal_tool(name) && !user_function_names.contains(name) }), Some("function_tool_call") => item .get("name") .and_then(|value| value.as_str()) .is_some_and(|name| { - session.is_internal_non_builtin_tool(name) && !user_function_names.contains(name) + session.is_internal_tool(name) && !user_function_names.contains(name) }),Based on learnings,
build_transformed_mcp_call_itemintentionally omitsserver_labelfor builtin formats, so label-based filtering cannot catch these raw traces.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@model_gateway/src/routers/openai/responses/utils.rs` around lines 434 - 445, The predicate that hides raw function_call / function_tool_call items only checks session.is_internal_non_builtin_tool, which lets internal builtin-backed traces slip through; update both branches (the Some("function_call") and Some("function_tool_call") arms) to also detect and treat internal builtin-backed tools as internal (e.g., extend the condition to session.is_internal_tool(name) or add an OR checking whatever API flags builtin-internal tools in your Session) so these raw items are filtered out (still keep the !user_function_names.contains(name) check); ensure this change aligns with build_transformed_mcp_call_item behavior so transformed builtin items remain visible while raw internal traces are hidden.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/mcp/src/core/config.rs`:
- Around line 216-218: Update the doc comment for the public field `internal` to
clarify its current behavior: state that setting `internal: true` hides tools
from standard client-visible outputs but does not yet redact or prevent
streaming Server-Sent Events (SSE) live tool events; mention this is a
limitation/known gap so operators understand live streams are still visible.
Edit the comment on the `internal` field (the `pub internal: bool` declaration)
to include this explicit caveat and keep the existing serde attribute and
semantics unchanged.
In `@model_gateway/src/routers/openai/responses/utils.rs`:
- Around line 327-333: When restored_tools.is_empty() we currently remove
"tools" but leave any existing "tool_choice" which can leak a pinned tool name;
update the empty-tools branch in the function handling obj/restored_tools to
also scrub "tool_choice" (remove the "tool_choice" entry from obj) before
returning so no upstream tool selection remains when all visible tools were
stripped.
---
Duplicate comments:
In `@model_gateway/src/routers/openai/responses/utils.rs`:
- Around line 434-445: The predicate that hides raw function_call /
function_tool_call items only checks session.is_internal_non_builtin_tool, which
lets internal builtin-backed traces slip through; update both branches (the
Some("function_call") and Some("function_tool_call") arms) to also detect and
treat internal builtin-backed tools as internal (e.g., extend the condition to
session.is_internal_tool(name) or add an OR checking whatever API flags
builtin-internal tools in your Session) so these raw items are filtered out
(still keep the !user_function_names.contains(name) check); ensure this change
aligns with build_transformed_mcp_call_item behavior so transformed builtin
items remain visible while raw internal traces are hidden.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 505a0016-2896-4f00-a449-e581833d2406
📒 Files selected for processing (7)
crates/mcp/src/core/config.rscrates/mcp/src/core/session.rsmodel_gateway/src/routers/mcp_utils.rsmodel_gateway/src/routers/openai/mcp/tool_loop.rsmodel_gateway/src/routers/openai/responses/streaming.rsmodel_gateway/src/routers/openai/responses/utils.rsmodel_gateway/tests/common/mock_mcp_server.rs
Signed-off-by: Daisy Zhou <zhoug9127@gmail.com>
Signed-off-by: Daisy Zhou <zhoug9127@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8637b877e6
ℹ️ 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".
Signed-off-by: Daisy Zhou <zhoug9127@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9a7b32697
ℹ️ 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".
Signed-off-by: Daisy Zhou <zhoug9127@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@model_gateway/src/routers/openai/responses/utils.rs`:
- Around line 333-340: The code currently preserves an existing object-style
"tool_choice" even when its referenced function is not present in the rebuilt
restored_tools array (variables: restored_tools, obj, key "tools" and
"tool_choice"), which leaks stripped internal tools; update the logic after
inserting "tools" so that if obj["tool_choice"] is an object with a
{"type":"function","name":X} selection, you check whether X exists in
restored_tools and if not either clear the "tool_choice" entry or rehydrate it
to a valid visible tool (e.g., set to "auto" or another allowed value), and add
a regression test that builds a mixed visible+internal tools input (visible
tools plus an internal tool removed by rebuild) with an object-style tool_choice
pointing to the internal name to verify the tool_choice is cleared or replaced
when the function is absent.
- Around line 307-318: The current loop in restored_tools (iterating
original_tools and matching ResponseTool::Function) unconditionally pops an
upstream_by_name entry and may reintroduce an ambiguous upstream schema on name
collisions; change the logic so you first inspect upstream_by_name.get(name) and
only pop_front() when the deque has exactly one candidate (no collision). If the
deque has length > 1 (or the entry is absent), skip using the upstream copy and
fall back to serde_json::to_value(original_tool) to serialize the original_tool,
ensuring strip_internal_mcp_tools() collisions do not revive internal MCP
schemas.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2d9fd637-7f0f-4a9b-9dd9-1c3efca6f2b2
📒 Files selected for processing (1)
model_gateway/src/routers/openai/responses/utils.rs
Signed-off-by: Daisy Zhou <zhoug9127@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f7b64a5af
ℹ️ 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".
Signed-off-by: Daisy Zhou <zhoug9127@gmail.com>
…ector Both have been the consistent primary authors of recent PRs in these subsystems: - @zhoug9127 (Daisy): #1168, #1149, #1065, #1061, #976 — mcp + data_connector - @zhaowenzi (Ziwen): #1174, #1163, #1123 — mcp Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
lightseekorg#1061) Signed-off-by: zhoug9127 <daisy.zhou@oracle.com> Signed-off-by: zhoug9127 <zhoug9127@gmail.com> Signed-off-by: Daisy Zhou <zhoug9127@gmail.com> Co-authored-by: zhoug9127 <daisy.zhou@oracle.com>
4 participants
Description
Problem
Self-brought MCP servers can power internal capabilities like LTM recall, but some deployments do not want those server labels, injected tools, MCP trace items, or internal error details to appear in client-visible final responses.
PR1 should stay narrow: this hiding capability is only for opt-in self-brought MCP servers. Users who do not opt in, or who rely on builtin-routed tools, should keep existing behavior.
Solution
Add a server-level
internal: truecapability for self-brought MCP servers and apply it only to the final assembled response surface.This PR:
toolsmcp_list_tools,mcp_call, and internal function-call trace items from final response outputweb_search_call,code_interpreter_call, andfile_search_callitems are explicitly out of scope for PR1PR2 plan: extend the same self-brought internal-server contract to streaming so live SSE tool events for those internal MCP servers are also hidden from the customer-facing stream, without changing builtin-routed behavior.
Changes
Test Plan
cargo +nightly fmt --all --checkcargo clippy --all-targets --all-features -- -D warningscargo build --workspacegit diff --checkcargo test -p smg build_transformed_mcp_call_item_does_not_add_server_label_for_builtin_formatscargo test -p smg restore_original_toolscargo test -p smg restore_original_tools_keeps_builtin_output_items_visiblecargo test -p smg restore_original_tools_strips_internal_mcp_output_itemscargo test -p smg --test api_tests test_final_response_hides_internal_mcp_trace_itemscargo test -p smg --test api_tests test_final_response_hides_internal_mcp_error_detailscargo test -p smg --test api_tests non_streaming_mcp_minimal_e2ecargo test -p smg --test api_tests streaming_with_mcp_tool_callscargo test -p smg --test mcp_testChecklist
cargo +nightly fmtpassescargo clippy --all-targets --all-features -- -D warningspassesSummary by CodeRabbit
New Features
Documentation
Tests
Refactor