refactor(tui): split MCP header helpers#3333
Conversation
Signed-off-by: cyq <15000851237@163.com>
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Code Review
This pull request refactors MCP HTTP header handling by moving related helper functions and constants from crates/tui/src/mcp.rs into a new submodule crates/tui/src/mcp/headers.rs. The feedback suggests using explicit module path qualifiers (self::headers and super::headers) when importing from the new module to avoid potential name resolution conflicts with the external headers crate.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| use crate::child_env; | ||
| use crate::network_policy::{Decision, NetworkPolicyDecider, host_from_url}; | ||
| use crate::utils::write_atomic; | ||
| use headers::{apply_safe_custom_headers, with_default_mcp_http_headers}; |
There was a problem hiding this comment.
Using use headers::... can cause ambiguity or compilation errors if an external crate named headers is present in the dependency tree (which is a very common crate in the Rust ecosystem). It is safer and more idiomatic to use self::headers to explicitly refer to the local module.
| use headers::{apply_safe_custom_headers, with_default_mcp_http_headers}; | |
| use self::headers::{apply_safe_custom_headers, with_default_mcp_http_headers}; |
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
| use headers::{MCP_HTTP_ACCEPT, is_safe_custom_header}; |
There was a problem hiding this comment.
Signed-off-by: cyq <15000851237@163.com>
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Problem
crates/tui/src/mcp.rsstill keeps HTTP header framing and custom-header filtering inline with transport code, which makes the MCP transport split in #3310 harder to review incrementally.Change
mcp::headersVerification
cargo test -p codewhale-tui mcp -- --nocapturecargo fmt --check --allgit diff --checkcargo clippy -p codewhale-tui --all-targets --locked -- -D warnings -A clippy::uninlined_format_args -A clippy::too_many_arguments -A clippy::unnecessary_map_or -A clippy::assertions_on_constantsPart of #3310.