refactor(settings): stage 6.3b promote mcp runtime types into fabro-mcp

Moves `McpServerEntry`, `McpServerSettings`, `McpTransport`, plus the
`default_startup_timeout_secs` / `default_tool_timeout_secs` helpers
from `fabro-types/src/settings/mcp.rs` into
`fabro-mcp/src/config.rs`. fabro-mcp was already the only crate that
re-exported them, so this deletes the `fabro-types` module entirely
and drops the `pub use mcp::*` re-export from `settings/mod.rs`.

`bridge_mcps` / `bridge_mcp_entry` (v2 `McpEntryLayer` → runtime
`McpServerEntry` converters) also move to `fabro-mcp/src/config.rs`.
`fabro-workflow::operations::start` and `fabro-cli::commands::exec`
now import `bridge_mcp_entry` from `fabro_mcp::config::bridge_mcp_entry`
instead of the v2 `to_runtime` module.

Four of the seven legacy runtime type modules are now gone; three
remain (run, sandbox, server). The `to_runtime.rs` module is down to
just sandbox, pull-request, merge-strategy, artifacts, and
worktree-mode helpers.

3,758 workspace tests pass. `cargo fmt --check --all` and
`cargo clippy --workspace -- -D warnings` are clean.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-04-09 18:12:00 -04:00
parent 2016c8e948
commit 38dacb8744
No known key found for this signature in database
6 changed files with 293 additions and 298 deletions

View file

@ -2,9 +2,8 @@ use anyhow::Result;
use fabro_agent::cli::{OutputFormat, run_with_args, run_with_args_and_client};
use fabro_llm::client::Client;
use fabro_llm::providers::FabroServerAdapter;
use fabro_mcp::config::McpServerSettings;
use fabro_mcp::config::{McpServerSettings, bridge_mcp_entry};
use fabro_types::settings::v2::InterpString;
use fabro_types::settings::v2::to_runtime::bridge_mcp_entry;
use std::collections::HashMap;
use std::sync::Arc;

View file

@ -1,4 +1,281 @@
pub use fabro_types::settings::mcp::{
McpServerEntry, McpServerSettings, McpTransport, default_startup_timeout_secs,
default_tool_timeout_secs,
};
//! MCP server configuration runtime types.
//!
//! The v2 parse tree lives in `fabro_types::settings::v2::run::McpEntryLayer`.
//! This module owns the runtime shape (flattened, with timeout helpers) that
//! the MCP client consumes at execution time. Conversion from the v2 shape
//! lives in [`bridge_mcp_entry`] / [`bridge_mcps`].
use std::collections::HashMap;
use std::time::Duration;
use fabro_types::settings::v2::InterpString;
use fabro_types::settings::v2::run::McpEntryLayer;
use serde::{Deserialize, Serialize};
#[must_use]
pub fn default_startup_timeout_secs() -> u64 {
10
}
#[must_use]
pub fn default_tool_timeout_secs() -> u64 {
60
}
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct McpServerSettings {
pub name: String,
pub transport: McpTransport,
#[serde(default = "default_startup_timeout_secs")]
pub startup_timeout_secs: u64,
#[serde(default = "default_tool_timeout_secs")]
pub tool_timeout_secs: u64,
}
impl McpServerSettings {
#[must_use]
pub fn startup_timeout(&self) -> Duration {
Duration::from_secs(self.startup_timeout_secs)
}
#[must_use]
pub fn tool_timeout(&self) -> Duration {
Duration::from_secs(self.tool_timeout_secs)
}
}
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
#[serde(tag = "type", rename_all = "snake_case")]
pub enum McpTransport {
Stdio {
command: Vec<String>,
#[serde(default)]
env: HashMap<String, String>,
},
Http {
url: String,
#[serde(default)]
headers: HashMap<String, String>,
},
/// MCP server that runs inside a sandbox and is accessed via HTTP preview URL.
/// During session init, the server is started inside the sandbox and this
/// variant is resolved into an `Http` transport using the sandbox's preview URL.
Sandbox {
command: Vec<String>,
port: u16,
#[serde(default)]
env: HashMap<String, String>,
},
}
/// MCP server entry as it appears in TOML config files (without a `name` field).
///
/// Converted to [`McpServerSettings`] via [`McpServerEntry::into_config`].
#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
pub struct McpServerEntry {
#[serde(flatten)]
pub transport: McpTransport,
#[serde(default = "default_startup_timeout_secs")]
pub startup_timeout_secs: u64,
#[serde(default = "default_tool_timeout_secs")]
pub tool_timeout_secs: u64,
}
impl McpServerEntry {
#[must_use]
pub fn into_config(self, name: String) -> McpServerSettings {
McpServerSettings {
name,
transport: self.transport,
startup_timeout_secs: self.startup_timeout_secs,
tool_timeout_secs: self.tool_timeout_secs,
}
}
}
/// Convert a map of v2 `McpEntryLayer` entries into runtime `McpServerEntry`s.
#[must_use]
pub fn bridge_mcps(mcps: &HashMap<String, McpEntryLayer>) -> HashMap<String, McpServerEntry> {
mcps.iter()
.map(|(name, entry)| (name.clone(), bridge_mcp_entry(entry)))
.collect()
}
/// Convert a single v2 `McpEntryLayer` into the runtime `McpServerEntry`.
#[must_use]
pub fn bridge_mcp_entry(entry: &McpEntryLayer) -> McpServerEntry {
let transport = match entry {
McpEntryLayer::Stdio {
script,
command,
env,
..
} => {
let command_vec: Vec<String> = if let Some(script) = script {
vec!["sh".into(), "-c".into(), interp_to_string(script)]
} else if let Some(command) = command {
command.iter().map(interp_to_string).collect()
} else {
Vec::new()
};
McpTransport::Stdio {
command: command_vec,
env: env
.iter()
.map(|(k, v)| (k.clone(), interp_to_string(v)))
.collect(),
}
}
McpEntryLayer::Http { url, headers, .. } => McpTransport::Http {
url: interp_to_string(url),
headers: headers
.iter()
.map(|(k, v)| (k.clone(), interp_to_string(v)))
.collect(),
},
McpEntryLayer::Sandbox {
script,
command,
port,
env,
..
} => {
let command_vec: Vec<String> = if let Some(script) = script {
vec!["sh".into(), "-c".into(), interp_to_string(script)]
} else if let Some(command) = command {
command.iter().map(interp_to_string).collect()
} else {
Vec::new()
};
McpTransport::Sandbox {
command: command_vec,
port: *port,
env: env
.iter()
.map(|(k, v)| (k.clone(), interp_to_string(v)))
.collect(),
}
}
};
let (startup_secs, tool_secs) = match entry {
McpEntryLayer::Http {
startup_timeout,
tool_timeout,
..
}
| McpEntryLayer::Stdio {
startup_timeout,
tool_timeout,
..
}
| McpEntryLayer::Sandbox {
startup_timeout,
tool_timeout,
..
} => (
startup_timeout.map_or(default_startup_timeout_secs(), |d| d.as_std().as_secs()),
tool_timeout.map_or(default_tool_timeout_secs(), |d| d.as_std().as_secs()),
),
};
McpServerEntry {
transport,
startup_timeout_secs: startup_secs,
tool_timeout_secs: tool_secs,
}
}
fn interp_to_string(value: &InterpString) -> String {
value.as_source()
}
#[cfg(test)]
mod tests {
use super::*;
use std::collections::HashMap;
#[test]
fn stdio_config_construction() {
let config = McpServerSettings {
name: "test-server".into(),
transport: McpTransport::Stdio {
command: vec![
"npx".into(),
"-y".into(),
"@modelcontextprotocol/server-filesystem".into(),
],
env: HashMap::new(),
},
startup_timeout_secs: 10,
tool_timeout_secs: 60,
};
assert_eq!(config.name, "test-server");
assert_eq!(config.startup_timeout(), Duration::from_secs(10));
assert_eq!(config.tool_timeout(), Duration::from_secs(60));
}
#[test]
fn http_config_construction() {
let config = McpServerSettings {
name: "remote-server".into(),
transport: McpTransport::Http {
url: "https://example.com/mcp".into(),
headers: HashMap::from([("Authorization".into(), "Bearer token".into())]),
},
startup_timeout_secs: 30,
tool_timeout_secs: 60,
};
assert_eq!(config.name, "remote-server");
assert_eq!(config.startup_timeout(), Duration::from_secs(30));
assert_eq!(config.tool_timeout(), Duration::from_secs(60));
}
#[test]
fn serde_round_trip_stdio() {
let config = McpServerSettings {
name: "fs".into(),
transport: McpTransport::Stdio {
command: vec!["node".into(), "server.js".into()],
env: HashMap::from([("NODE_ENV".into(), "production".into())]),
},
startup_timeout_secs: 15,
tool_timeout_secs: 90,
};
let json = serde_json::to_string(&config).unwrap();
let deserialized: McpServerSettings = serde_json::from_str(&json).unwrap();
assert_eq!(deserialized.name, "fs");
assert_eq!(deserialized.startup_timeout_secs, 15);
assert_eq!(deserialized.tool_timeout_secs, 90);
assert!(
matches!(deserialized.transport, McpTransport::Stdio { command, .. } if command == vec!["node", "server.js"])
);
}
#[test]
fn serde_round_trip_http() {
let config = McpServerSettings {
name: "remote".into(),
transport: McpTransport::Http {
url: "https://mcp.example.com".into(),
headers: HashMap::new(),
},
startup_timeout_secs: 10,
tool_timeout_secs: 60,
};
let json = serde_json::to_string(&config).unwrap();
let deserialized: McpServerSettings = serde_json::from_str(&json).unwrap();
assert_eq!(deserialized.name, "remote");
assert!(
matches!(deserialized.transport, McpTransport::Http { url, .. } if url == "https://mcp.example.com")
);
}
#[test]
fn serde_defaults_applied() {
let json = r#"{"name":"minimal","transport":{"type":"stdio","command":["echo"]}}"#;
let config: McpServerSettings = serde_json::from_str(json).unwrap();
assert_eq!(config.startup_timeout_secs, 10);
assert_eq!(config.tool_timeout_secs, 60);
}
}

View file

@ -1,186 +0,0 @@
use std::collections::HashMap;
use std::time::Duration;
use serde::{Deserialize, Serialize};
use crate::combine::Combine;
pub fn default_startup_timeout_secs() -> u64 {
10
}
pub fn default_tool_timeout_secs() -> u64 {
60
}
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct McpServerSettings {
pub name: String,
pub transport: McpTransport,
#[serde(default = "default_startup_timeout_secs")]
pub startup_timeout_secs: u64,
#[serde(default = "default_tool_timeout_secs")]
pub tool_timeout_secs: u64,
}
impl McpServerSettings {
#[must_use]
pub fn startup_timeout(&self) -> Duration {
Duration::from_secs(self.startup_timeout_secs)
}
#[must_use]
pub fn tool_timeout(&self) -> Duration {
Duration::from_secs(self.tool_timeout_secs)
}
}
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
#[serde(tag = "type", rename_all = "snake_case")]
pub enum McpTransport {
Stdio {
command: Vec<String>,
#[serde(default)]
env: HashMap<String, String>,
},
Http {
url: String,
#[serde(default)]
headers: HashMap<String, String>,
},
/// MCP server that runs inside a sandbox and is accessed via HTTP preview URL.
/// During session init, the server is started inside the sandbox and this
/// variant is resolved into an `Http` transport using the sandbox's preview URL.
Sandbox {
command: Vec<String>,
port: u16,
#[serde(default)]
env: HashMap<String, String>,
},
}
impl Combine for McpTransport {
fn combine(self, _other: Self) -> Self {
self
}
}
/// MCP server entry as it appears in TOML config files (without a `name` field).
///
/// Converted to [`McpServerSettings`] via [`McpServerEntry::into_config`].
#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
pub struct McpServerEntry {
#[serde(flatten)]
pub transport: McpTransport,
#[serde(default = "default_startup_timeout_secs")]
pub startup_timeout_secs: u64,
#[serde(default = "default_tool_timeout_secs")]
pub tool_timeout_secs: u64,
}
impl McpServerEntry {
pub fn into_config(self, name: String) -> McpServerSettings {
McpServerSettings {
name,
transport: self.transport,
startup_timeout_secs: self.startup_timeout_secs,
tool_timeout_secs: self.tool_timeout_secs,
}
}
}
impl Combine for McpServerEntry {
fn combine(self, _other: Self) -> Self {
self
}
}
#[cfg(test)]
mod tests {
use super::*;
use std::collections::HashMap;
#[test]
fn stdio_config_construction() {
let config = McpServerSettings {
name: "test-server".into(),
transport: McpTransport::Stdio {
command: vec![
"npx".into(),
"-y".into(),
"@modelcontextprotocol/server-filesystem".into(),
],
env: HashMap::new(),
},
startup_timeout_secs: 10,
tool_timeout_secs: 60,
};
assert_eq!(config.name, "test-server");
assert_eq!(config.startup_timeout(), Duration::from_secs(10));
assert_eq!(config.tool_timeout(), Duration::from_secs(60));
}
#[test]
fn http_config_construction() {
let config = McpServerSettings {
name: "remote-server".into(),
transport: McpTransport::Http {
url: "https://example.com/mcp".into(),
headers: HashMap::from([("Authorization".into(), "Bearer token".into())]),
},
startup_timeout_secs: 30,
tool_timeout_secs: 60,
};
assert_eq!(config.name, "remote-server");
assert_eq!(config.startup_timeout(), Duration::from_secs(30));
assert_eq!(config.tool_timeout(), Duration::from_secs(60));
}
#[test]
fn serde_round_trip_stdio() {
let config = McpServerSettings {
name: "fs".into(),
transport: McpTransport::Stdio {
command: vec!["node".into(), "server.js".into()],
env: HashMap::from([("NODE_ENV".into(), "production".into())]),
},
startup_timeout_secs: 15,
tool_timeout_secs: 90,
};
let json = serde_json::to_string(&config).unwrap();
let deserialized: McpServerSettings = serde_json::from_str(&json).unwrap();
assert_eq!(deserialized.name, "fs");
assert_eq!(deserialized.startup_timeout_secs, 15);
assert_eq!(deserialized.tool_timeout_secs, 90);
assert!(
matches!(deserialized.transport, McpTransport::Stdio { command, .. } if command == vec!["node", "server.js"])
);
}
#[test]
fn serde_round_trip_http() {
let config = McpServerSettings {
name: "remote".into(),
transport: McpTransport::Http {
url: "https://mcp.example.com".into(),
headers: HashMap::new(),
},
startup_timeout_secs: 10,
tool_timeout_secs: 60,
};
let json = serde_json::to_string(&config).unwrap();
let deserialized: McpServerSettings = serde_json::from_str(&json).unwrap();
assert_eq!(deserialized.name, "remote");
assert!(
matches!(deserialized.transport, McpTransport::Http { url, .. } if url == "https://mcp.example.com")
);
}
#[test]
fn serde_defaults_applied() {
let json = r#"{"name":"minimal","transport":{"type":"stdio","command":["echo"]}}"#;
let config: McpServerSettings = serde_json::from_str(json).unwrap();
assert_eq!(config.startup_timeout_secs, 10);
assert_eq!(config.tool_timeout_secs, 60);
}
}

View file

@ -19,16 +19,11 @@
//! owning consumer crates or replace their call sites with v2-native
//! accessors, at which point this module goes away.
pub mod mcp;
pub mod run;
pub mod sandbox;
pub mod server;
pub mod v2;
pub use mcp::{
McpServerEntry, McpServerSettings, McpTransport, default_startup_timeout_secs,
default_tool_timeout_secs,
};
pub use run::{
ArtifactsSettings, CheckpointSettings, GitHubSettings, LlmSettings, MergeStrategy,
PullRequestSettings, SetupSettings,

View file

@ -1,22 +1,20 @@
//! v2 → runtime-type conversion helpers.
//!
//! The runtime types in `fabro_types::settings::{mcp,run,sandbox}` are the
//! shapes that downstream crates (fabro-workflow, fabro-mcp, fabro-sandbox)
//! still consume at runtime. Each helper here reads the v2 parse tree and
//! The runtime types in `fabro_types::settings::{run,sandbox}` are the
//! shapes that downstream crates (fabro-workflow, fabro-sandbox) still
//! consume at runtime. Each helper here reads the v2 parse tree and
//! builds the equivalent runtime value.
//!
//! Hook bridging has moved to `fabro_hooks::config::bridge_hook`, which
//! owns its runtime shape. Consumer crates will pull the rest of these
//! helpers into their own crates in follow-up 6.3b passes.
use std::collections::HashMap;
//! Hook bridging has moved to `fabro_hooks::config::bridge_hook`; MCP
//! bridging has moved to `fabro_mcp::config::{bridge_mcps, bridge_mcp_entry}`.
//! Consumer crates will pull the rest of these helpers into their own
//! crates in follow-up 6.3b passes.
use super::interp::InterpString;
use super::run::{
McpEntryLayer, MergeStrategy as V2MergeStrategy, RunArtifactsLayer, RunPullRequestLayer,
RunSandboxLayer, WorktreeMode as V2WorktreeMode,
MergeStrategy as V2MergeStrategy, RunArtifactsLayer, RunPullRequestLayer, RunSandboxLayer,
WorktreeMode as V2WorktreeMode,
};
use crate::settings::mcp::{McpServerEntry, McpTransport};
use crate::settings::run::{
ArtifactsSettings, MergeStrategy as OldMergeStrategy, PullRequestSettings,
};
@ -116,95 +114,6 @@ pub fn bridge_run_artifacts(artifacts: &RunArtifactsLayer) -> ArtifactsSettings
}
}
pub fn bridge_mcps(mcps: &HashMap<String, McpEntryLayer>) -> HashMap<String, McpServerEntry> {
mcps.iter()
.map(|(name, entry)| (name.clone(), bridge_mcp_entry(entry)))
.collect()
}
pub fn bridge_mcp_entry(entry: &McpEntryLayer) -> McpServerEntry {
let transport = match entry {
McpEntryLayer::Stdio {
script,
command,
env,
..
} => {
let command_vec: Vec<String> = if let Some(script) = script {
vec!["sh".into(), "-c".into(), interp_to_string(script)]
} else if let Some(command) = command {
command.iter().map(interp_to_string).collect()
} else {
Vec::new()
};
McpTransport::Stdio {
command: command_vec,
env: env
.iter()
.map(|(k, v)| (k.clone(), interp_to_string(v)))
.collect(),
}
}
McpEntryLayer::Http { url, headers, .. } => McpTransport::Http {
url: interp_to_string(url),
headers: headers
.iter()
.map(|(k, v)| (k.clone(), interp_to_string(v)))
.collect(),
},
McpEntryLayer::Sandbox {
script,
command,
port,
env,
..
} => {
let command_vec: Vec<String> = if let Some(script) = script {
vec!["sh".into(), "-c".into(), interp_to_string(script)]
} else if let Some(command) = command {
command.iter().map(interp_to_string).collect()
} else {
Vec::new()
};
McpTransport::Sandbox {
command: command_vec,
port: *port,
env: env
.iter()
.map(|(k, v)| (k.clone(), interp_to_string(v)))
.collect(),
}
}
};
let (startup_secs, tool_secs) = match entry {
McpEntryLayer::Http {
startup_timeout,
tool_timeout,
..
}
| McpEntryLayer::Stdio {
startup_timeout,
tool_timeout,
..
}
| McpEntryLayer::Sandbox {
startup_timeout,
tool_timeout,
..
} => (
startup_timeout.map_or(10, |d| d.as_std().as_secs()),
tool_timeout.map_or(60, |d| d.as_std().as_secs()),
),
};
McpServerEntry {
transport,
startup_timeout_secs: startup_secs,
tool_timeout_secs: tool_secs,
}
}
fn interp_to_string(value: &InterpString) -> String {
value.as_source()
}

View file

@ -7,13 +7,14 @@ use std::time::{Duration, Instant};
use fabro_config::project as project_config;
use fabro_hooks::config::bridge_hook;
use fabro_interview::{AutoApproveInterviewer, Interviewer};
use fabro_mcp::config::bridge_mcp_entry;
use fabro_model::{Catalog, FallbackTarget, Provider};
use fabro_sandbox::{SandboxProvider, SandboxSpec};
use fabro_types::RunId;
use fabro_types::settings::sandbox::{self as sandbox_config, WorktreeMode};
use fabro_types::settings::v2::run::ModelRefOrSplice;
use fabro_types::settings::v2::to_runtime::{
bridge_mcp_entry, bridge_pull_request, bridge_sandbox, bridge_worktree_mode,
bridge_pull_request, bridge_sandbox, bridge_worktree_mode,
};
use fabro_types::settings::v2::{InterpString, SettingsFile};