fix(mcp): merge workspace MCP config#2751
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
cyq1017 has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Thanks @cyq1017 for taking the time to contribute. This repository is currently observing a maintainer-managed contribution gate in dry-run mode, so this pull request is staying open. When enforcement is enabled, pull requests from contributors who are not listed in Please read |
![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 adds support for workspace-local MCP configurations (.codewhale/mcp.json) that merge with and override global configurations, and introduces a cwd field to McpServerConfig to run servers in specific directories. The McpPool is updated to track both global and project-level config files for lazy reloading. Feedback recommends canonicalizing the workspace path to prevent failures with relative paths containing .. components, resolving relative cwd paths in project configs relative to the workspace root, and skipping the merge process if the global and project config paths are identical.
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.
| pub fn workspace_mcp_config_path(workspace: &Path) -> PathBuf { | ||
| workspace.join(".codewhale").join("mcp.json") | ||
| } |
There was a problem hiding this comment.
If the workspace path contains .. components (e.g., when specified as a relative path like --workspace ../project), workspace_mcp_config_path will produce a path containing ... This causes mcp_config_mtime to silently fail (returning None) and prevents load_config from loading the file due to strict path traversal checks. Canonicalizing the workspace path first resolves these issues.
pub fn workspace_mcp_config_path(workspace: &Path) -> PathBuf {
workspace
.canonicalize()
.unwrap_or_else(|_| workspace.to_path_buf())
.join(".codewhale")
.join("mcp.json")
}| pub fn load_config_with_workspace(global_path: &Path, workspace: &Path) -> Result<McpConfig> { | ||
| let mut merged = load_config(global_path)?; | ||
| let project_path = workspace_mcp_config_path(workspace); | ||
| if !project_path.exists() { | ||
| return Ok(merged); | ||
| } | ||
|
|
||
| let mut project = load_config(&project_path)?; | ||
| for server in project.servers.values_mut() { | ||
| if server.cwd.is_none() && server.command.is_some() && server.url.is_none() { | ||
| server.cwd = Some(workspace.to_path_buf()); | ||
| } | ||
| } | ||
| merged.servers.extend(project.servers); | ||
| Ok(merged) | ||
| } |
There was a problem hiding this comment.
Ensure that relative cwd paths specified in the workspace-local MCP configuration are resolved relative to the workspace root rather than the process's current working directory. Additionally, canonicalize the workspace path to ensure robust path resolution, and skip merging if the global and project config paths are identical (e.g., when running from the home directory).
pub fn load_config_with_workspace(global_path: &Path, workspace: &Path) -> Result<McpConfig> {
let workspace_abs = workspace
.canonicalize()
.unwrap_or_else(|_| workspace.to_path_buf());
let mut merged = load_config(global_path)?;
let project_path = workspace_mcp_config_path(&workspace_abs);
if !project_path.exists() || global_path == project_path {
return Ok(merged);
}
let mut project = load_config(&project_path)?;
for server in project.servers.values_mut() {
if server.command.is_some() && server.url.is_none() {
if let Some(ref cwd) = server.cwd {
if cwd.is_relative() {
server.cwd = Some(workspace_abs.join(cwd));
}
} else {
server.cwd = Some(workspace_abs.clone());
}
}
}
merged.servers.extend(project.servers);
Ok(merged)
}
There was a problem hiding this comment.
cyq1017 has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Thanks @cyq1017 and @yekern. I harvested this into #2770 ( What landed preserves the intent from this PR: trusted workspaces now merge I also tightened the trust boundary before landing: project MCP is ignored until workspace trust is recorded in user-owned config, checked-in legacy trust markers do not unlock MCP subprocesses, trust changes are watched for reload, and project Verification run locally on the harvest branch:
Closing this PR as superseded by the harvested #2770. Thanks again for pushing the project MCP path forward. |
…mbown#2757 harvests and Hmbown#2742/Hmbown#2751/Hmbown#2755 dispositions Log the new community-PR harvests in CHANGELOG.md and crates/tui/CHANGELOG.md (MCP underscore server names, Xiaomi MiMo pricing, hydrated deferred-tool render, Token Plan region docs) with contributor credit, and update docs/V0_9_0_EXECUTION_MAP.md with evidence-backed dispositions for the newly-reviewed PRs, including the deferred Hmbown#2742 and forwarded Hmbown#2751/Hmbown#2755.
Merge global MCP config with trusted workspace .codewhale/mcp.json files so project MCP servers appear in TUI, CLI, doctor, and runtime API flows. Project stdio servers default cwd to the workspace, project cwd escapes are rejected, and project MCP is ignored until workspace trust is recorded in user-owned config. Fixes Hmbown#2749 Reported by @yekern Harvested from PR Hmbown#2751 by @cyq1017
2 participants
Summary
.codewhale/mcp.jsonservers with the global MCP config, with workspace servers overriding same-name global entriescodewhale mcpread/connect/validate flowsVerification
cargo test -p codewhale-tui workspace_mcp_ -- --nocapturecargo test -p codewhale-tui mcp::tests -- --nocapturecargo check -p codewhale-tuirustfmt --edition 2024 --check crates/tui/src/mcp.rs crates/tui/src/core/engine.rs crates/tui/src/runtime_api.rs crates/tui/src/main.rsgit diff --checkCloses #2749