scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-10-08 03:10:26 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

feat(types): introduce ResolvedMcpEntry for run agent MCP entries (#526)

Browse source 
## What

Changes `RunAgentSettings.mcps` from `HashMap<String,
McpServerSettings>` to `HashMap<String, ResolvedMcpEntry>`, a two-state
enum:

- `Resolved(McpServerSettings)` — an inline, fully-resolved MCP server
(every code path produces this today).
- `Reference { id, enabled }` — an unresolved reference to a named
server in the MCP catalog.

This is the **type-shape foundation only**: every current path still
produces `Resolved`, and no reference parsing or catalog lookup is added
here. It unblocks a later server-side pass that swaps `Reference` →
`Resolved` against the MCP server store before a run spec is persisted,
so persisted runs stay self-contained snapshots.

## Why this shape

- `ResolvedMcpEntry` is `#[serde(untagged)]` with `Resolved` first, so a
resolved entry (de)serializes as a bare `McpServerSettings` with no enum
tag — preserving backward compatibility with run specs persisted before
the enum existed.
- `McpServerRef` uses `deny_unknown_fields`, so the two variants can
never collide (`McpServerSettings` requires `name` + `transport`, which
a reference rejects).
- `McpServerRef.id` is a plain `String`, keeping `fabro-types` decoupled
from the MCP store crate.

## Consumers updated

- **fabro-config** `resolve_agent`: wraps each enabled inline entry as
`Resolved`, reusing the shared `resolve_enabled_mcps` enable-filter.
- **fabro-types** `RunNamespace::substitute_variables`: only walks
`Resolved` entries (references carry no templates).
- **fabro-workflow** `operations/start.rs`: extracts `Resolved` at the
post-persistence worker-startup consumer; a surviving `Reference` is an
invariant violation, guarded with `debug_assert!` plus a hard error.
- **fabro-cli** `exec.rs`: the `run.agent.mcps` fallback for `fabro
exec` keeps only `Resolved` inline servers; catalog references are
run-only on this CLI-direct path (no server-side resolver).

## Tests

- Back-compat round-trip proving old-format bare-`McpServerSettings`
maps (JSON and TOML) deserialize as all-`Resolved`.
- A `{ id, enabled }` value parses as `Reference` while a full server
config parses as `Resolved`.
- `Resolved` serializes back out as a bare `McpServerSettings`.

Independent of the in-flight MCP server store and OpenAPI-spec PRs;
mergeable on its own.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Scott Werner 2026-06-25 15:50:36 -04:00 • committed by GitHub
parent c311b6c67f
commit 8a7ad7200b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 396 additions and 46 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

65
lib/crates/fabro-cli/src/commands/exec.rs
View file

@ -17,6 +17,7 @@ use fabro_llm::types::{
use fabro_mcp::config::McpServerSettings;
use fabro_model::ProviderId;
use fabro_types::settings::cli::OutputFormat as SettingsOutputFormat;
use fabro_types::settings::run::ResolvedMcpEntry;
use fabro_util::exit::{self, ErrorExt, ExitClass};
use futures::stream;
use serde::Deserialize;
@ -289,6 +290,24 @@ fn process_env_var(name: &str) -> Option<String> {
std::env::var(name).ok()
}
fn run_mcp_servers_for_exec(
mcps: &HashMap<String, ResolvedMcpEntry>,
) -> AnyResult<Vec<McpServerSettings>> {
mcps.iter()
.map(|(key, entry)| match entry {
ResolvedMcpEntry::Resolved(server) => Ok(server.clone()),
ResolvedMcpEntry::Reference(reference) => {
anyhow::bail!(
"fabro exec cannot resolve run.agent.mcps.{key} catalog reference \
(id `{}`); define an inline server under [cli.exec.agent.mcps.{key}] or \
remove the run-level reference",
reference.id
);
}
})
.collect()
}
pub(crate) async fn execute(mut args: ExecArgs, ctx: &CommandContext) -> AnyResult<()> {
use fabro_agent::cli::PermissionLevel as AgentPermissionLevel;
use fabro_types::settings::run::AgentPermissions;
@ -317,7 +336,9 @@ pub(crate) async fn execute(mut args: ExecArgs, ctx: &CommandContext) -> AnyResu
Some(mcps) => mcps.values().cloned().collect(),
None => ctx
.run_settings()
.map(|settings| settings.agent.mcps.values().cloned().collect())
.ok()
.map(|settings| run_mcp_servers_for_exec(&settings.agent.mcps))
.transpose()?
.unwrap_or_default(),
};
// Resolve `{{ env.* }}` in MCP transport config at the exec boundary,
@ -368,3 +389,45 @@ pub(crate) async fn execute(mut args: ExecArgs, ctx: &CommandContext) -> AnyResu
Ok(())
}
#[cfg(test)]
mod tests {
use std::collections::HashMap;
use fabro_types::settings::run::{McpServerRef, McpServerSettings, ResolvedMcpEntry};
use super::run_mcp_servers_for_exec;
#[test]
fn run_mcp_servers_for_exec_rejects_catalog_references() {
let err = run_mcp_servers_for_exec(&HashMap::from([(
"sentry".to_string(),
ResolvedMcpEntry::Reference(McpServerRef {
id: "catalog/sentry".to_string(),
enabled: None,
}),
)]))
.expect_err("fabro exec should reject unresolved run-level MCP references");
assert!(
err.to_string()
.contains("fabro exec cannot resolve run.agent.mcps.sentry catalog reference"),
"unexpected error: {err}"
);
}
#[test]
fn run_mcp_servers_for_exec_keeps_resolved_servers() {
let servers = run_mcp_servers_for_exec(&HashMap::from([(
"inline".to_string(),
ResolvedMcpEntry::Resolved(McpServerSettings {
name: "inline".to_string(),
..McpServerSettings::default()
}),
)]))
.expect("resolved inline server should be usable by fabro exec");
assert_eq!(servers.len(), 1);
assert_eq!(servers[0].name, "inline");
}
}

15
lib/crates/fabro-config/src/resolve/mod.rs
View file

@ -101,7 +101,9 @@ pub(crate) fn warn_if_demoted_template(field: &str, value: Option<&str>) {
mod tests {
use std::collections::HashMap;
use fabro_types::settings::run::{HookType, McpHttpProtocol, McpTransport, TlsMode};
use fabro_types::settings::run::{
HookType, McpHttpProtocol, McpTransport, ResolvedMcpEntry, TlsMode,
};
use crate::SettingsLayer;
use crate::tests::workflow_settings_from_layer;
@ -151,9 +153,14 @@ Authorization = "Bearer {{ env.HOOK_TOKEN }}"
.expect("run settings should resolve")
.run;
let mcps = &resolved.agent.mcps;
let transport = |name: &str| {
mcps.get(name)
.and_then(ResolvedMcpEntry::as_resolved)
.map(|server| &server.transport)
};
assert_eq!(
mcps.get("stdio").map(|mcp| &mcp.transport),
transport("stdio"),
Some(&McpTransport::Stdio {
command: vec!["fabro-mcp".to_string(), "--stdio".to_string()],
env: HashMap::from([(
@ -163,7 +170,7 @@ Authorization = "Bearer {{ env.HOOK_TOKEN }}"
})
);
assert_eq!(
mcps.get("http").map(|mcp| &mcp.transport),
transport("http"),
Some(&McpTransport::Http {
protocol: McpHttpProtocol::default(),
url: "https://mcp.example.com".to_string(),
@ -174,7 +181,7 @@ Authorization = "Bearer {{ env.HOOK_TOKEN }}"
})
);
assert_eq!(
mcps.get("sandbox").map(|mcp| &mcp.transport),
transport("sandbox"),
Some(&McpTransport::Sandbox {
protocol: McpHttpProtocol::default(),
command: vec!["fabro-mcp".to_string(), "--sandbox".to_string()],

21
lib/crates/fabro-config/src/resolve/run.rs
View file

@ -4,11 +4,11 @@ use fabro_types::settings::InterpString;
use fabro_types::settings::run::{
ArtifactsSettings, GitAuthorSettings, HookDefinition, HookType, InterviewProviderSettings,
McpServerSettings, McpTransport, MergeStrategy, NotificationProviderSettings,
NotificationRouteSettings, PullRequestSettings, RunAgentSettings, RunBranchSettings,
RunCheckpointSettings, RunCloneSettings, RunExecutionSettings, RunGitSettings, RunGoal,
RunIntegrationsGithubSettings, RunIntegrationsSettings, RunInterviewsSettings,
RunMetaBranchSettings, RunModelControls, RunModelSettings, RunNamespace, RunPrepareSettings,
RunScmSettings, ScmGitHubSettings, TlsMode,
NotificationRouteSettings, PullRequestSettings, ResolvedMcpEntry, RunAgentSettings,
RunBranchSettings, RunCheckpointSettings, RunCloneSettings, RunExecutionSettings,
RunGitSettings, RunGoal, RunIntegrationsGithubSettings, RunIntegrationsSettings,
RunInterviewsSettings, RunMetaBranchSettings, RunModelControls, RunModelSettings, RunNamespace,
RunPrepareSettings, RunScmSettings, ScmGitHubSettings, TlsMode,
};
use super::{ResolveError, resolve_run_environment};
@ -288,7 +288,9 @@ fn resolve_agent(agent: Option<&RunAgentLayer>) -> RunAgentSettings {
RunAgentSettings {
fabro_tools: agent.fabro_tools.unwrap_or(false),
permissions: agent.permissions,
mcps: resolve_enabled_mcps(&agent.mcps),
mcps: enabled_mcp_settings(&agent.mcps)
.map(|(name, settings)| (name, ResolvedMcpEntry::Resolved(settings)))
.collect(),
}
}
@ -299,10 +301,15 @@ fn resolve_agent(agent: Option<&RunAgentLayer>) -> RunAgentSettings {
pub(crate) fn resolve_enabled_mcps(
mcps: &StickyMap<McpEntryLayer>,
) -> HashMap<String, McpServerSettings> {
enabled_mcp_settings(mcps).collect()
}
fn enabled_mcp_settings(
mcps: &StickyMap<McpEntryLayer>,
) -> impl Iterator<Item = (String, McpServerSettings)> + '_ {
mcps.iter()
.filter(|(_, entry)| entry.is_enabled())
.map(|(name, entry)| (name.clone(), resolve_mcp_entry(name, entry)))
.collect()
}
#[expect(

9
lib/crates/fabro-config/src/tests/resolve_run.rs
View file

@ -1017,7 +1017,7 @@ mod run_agent_mcps {
//! Layer + resolver tests for `[run.agent.mcps]`: same-key replacement
//! across layers (`StickyMap`) and honoring `enabled = false`.
use fabro_types::settings::run::McpTransport;
use fabro_types::settings::run::{McpTransport, ResolvedMcpEntry};
use crate::SettingsLayer;
use crate::layers::Combine;
@ -1028,8 +1028,9 @@ mod run_agent_mcps {
.expect("fixture should parse via SettingsLayer")
}
fn stdio_command(transport: &McpTransport) -> &[String] {
match transport {
fn stdio_command(entry: &ResolvedMcpEntry) -> &[String] {
let server = entry.as_resolved().expect("expected resolved inline entry");
match &server.transport {
McpTransport::Stdio { command, .. } => command,
other => panic!("expected stdio transport, got {other:?}"),
}
@ -1071,7 +1072,7 @@ command = ["extra-server"]
// Same-key `fs` is replaced by the higher layer; different-key `extra`
// is additive and inherited from the lower layer.
assert_eq!(stdio_command(&mcps["fs"].transport), &[
assert_eq!(stdio_command(&mcps["fs"]), &[
"fs-server".to_string(),
"--workflow".to_string()
],);

12
lib/crates/fabro-types/src/settings/mod.rs
View file

@ -37,12 +37,12 @@ pub use run::{
ArtifactsSettings, DockerfileSource, EnvironmentImageSettings, EnvironmentLifecycleSettings,
EnvironmentNetworkMode, EnvironmentNetworkSettings, EnvironmentProvider,
EnvironmentResourcesSettings, EnvironmentSettings, GitAuthorSettings, HookDefinition, HookType,
InterviewProviderSettings, McpServerSettings, McpTransport, NotificationProviderSettings,
NotificationRouteSettings, PullRequestSettings, RunAgentSettings, RunCheckpointSettings,
RunEnvironmentSettings, RunExecutionSettings, RunGitSettings, RunGoal,
RunIntegrationsGithubSettings, RunIntegrationsSettings, RunInterviewsSettings,
RunModelControls, RunModelSettings, RunNamespace, RunPrepareSettings, RunScmSettings,
ScmGitHubSettings, TlsMode,
InterviewProviderSettings, McpServerRef, McpServerSettings, McpTransport,
NotificationProviderSettings, NotificationRouteSettings, PullRequestSettings, ResolvedMcpEntry,
RunAgentSettings, RunCheckpointSettings, RunEnvironmentSettings, RunExecutionSettings,
RunGitSettings, RunGoal, RunIntegrationsGithubSettings, RunIntegrationsSettings,
RunInterviewsSettings, RunModelControls, RunModelSettings, RunNamespace, RunPrepareSettings,
RunScmSettings, ScmGitHubSettings, TlsMode,
};
pub use server::{
GithubIntegrationSettings, IntegrationWebhooksSettings, LogDestination, ObjectStoreSettings,

300
lib/crates/fabro-types/src/settings/run.rs
View file

@ -10,6 +10,7 @@ use std::collections::HashMap;
use std::path::PathBuf;
use std::time::Duration as StdDuration;
use serde::de::{self, Deserializer};
use serde::ser::SerializeStruct;
use serde::{Deserialize, Serialize};
@ -106,9 +107,13 @@ impl RunNamespace {
// run.scm.owner/repository were demoted and removed from this pass
// (D2): values stay literal.
substitute_string_vec(&mut self.prepare.commands, &mut lookup)?;
for mcp in self.agent.mcps.values_mut() {
substitute_string(&mut mcp.name, &mut lookup)?;
substitute_mcp_transport(&mut mcp.transport, &mut lookup)?;
// Only resolved inline servers carry substitutable templates; an
// unresolved reference holds just an id + enabled flag.
for entry in self.agent.mcps.values_mut() {
if let ResolvedMcpEntry::Resolved(mcp) = entry {
substitute_string(&mut mcp.name, &mut lookup)?;
substitute_mcp_transport(&mut mcp.transport, &mut lookup)?;
}
}
for hook in &mut self.hooks {
substitute_option_string(&mut hook.name, &mut lookup)?;
@ -356,21 +361,24 @@ mod run_namespace_variable_substitution_tests {
timeout_ms: 1_000,
},
agent: super::RunAgentSettings {
mcps: HashMap::from([("http".to_string(), McpServerSettings {
name: "http".to_string(),
transport: McpTransport::Http {
protocol: McpHttpProtocol::default(),
url: "https://{{ vars.HOST }}/mcp".to_string(),
headers: HashMap::from([(
"X-Env".to_string(),
"{{ vars.ENV }}".to_string(),
)]),
},
current_dir: None,
clear_env: false,
startup_timeout_secs: 10,
tool_timeout_secs: 60,
})]),
mcps: HashMap::from([(
"http".to_string(),
super::ResolvedMcpEntry::Resolved(McpServerSettings {
name: "http".to_string(),
transport: McpTransport::Http {
protocol: McpHttpProtocol::default(),
url: "https://{{ vars.HOST }}/mcp".to_string(),
headers: HashMap::from([(
"X-Env".to_string(),
"{{ vars.ENV }}".to_string(),
)]),
},
current_dir: None,
clear_env: false,
startup_timeout_secs: 10,
tool_timeout_secs: 60,
}),
)]),
..super::RunAgentSettings::default()
},
hooks: vec![HookDefinition {
@ -412,7 +420,9 @@ mod run_namespace_variable_substitution_tests {
assert_eq!(run.prepare.commands, vec![
"echo prod {{ env.REGION }}".to_string()
]);
let mcp = &run.agent.mcps["http"];
let mcp = run.agent.mcps["http"]
.as_resolved()
.expect("expected resolved inline mcp entry");
match &mcp.transport {
McpTransport::Http { url, headers, .. } => {
assert_eq!(url, "https://mcp.example/mcp");
@ -1053,12 +1063,95 @@ pub struct RunAgentSettings {
#[serde(default)]
pub fabro_tools: bool,
pub permissions: Option<AgentPermissions>,
pub mcps: HashMap<String, McpServerSettings>,
pub mcps: HashMap<String, ResolvedMcpEntry>,
}
/// An MCP entry in a run's agent settings: either an inline resolved server or
/// an unresolved reference to a server-side catalog definition.
///
/// `Resolved` (de)serializes as a **bare** [`McpServerSettings`] - with no enum
/// tag - for backward compatibility with run specs persisted before this enum
/// existed. Serialization stays `#[serde(untagged)]`, and custom
/// deserialization preserves the same wire shapes while rejecting entries that
/// mix catalog-reference fields (`id`, `enabled`) with inline server fields.
///
/// Invariant: no `Reference` survives run creation. References are resolved to
/// `Resolved` on the server's run-preparation path before the run spec is
/// persisted; any `Reference` reaching a post-persistence consumer is a bug.
#[derive(Debug, Clone, PartialEq, Serialize)]
#[serde(untagged)]
pub enum ResolvedMcpEntry {
Resolved(McpServerSettings),
Reference(McpServerRef),
}
impl<'de> Deserialize<'de> for ResolvedMcpEntry {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'de>,
{
let value = serde_json::Value::deserialize(deserializer)?;
let serde_json::Value::Object(map) = &value else {
return Err(de::Error::custom("MCP entry must be a table"));
};
let has_reference_fields = map.contains_key("id") || map.contains_key("enabled");
let has_inline_server_fields = map.contains_key("name")
|| map.contains_key("transport")
|| map.contains_key("current_dir")
|| map.contains_key("clear_env")
|| map.contains_key("startup_timeout_secs")
|| map.contains_key("tool_timeout_secs");
if has_reference_fields && has_inline_server_fields {
return Err(de::Error::custom(
"MCP entry cannot mix catalog reference fields (`id`, `enabled`) with inline \
server fields",
));
}
if has_reference_fields {
serde_json::from_value(value)
.map(Self::Reference)
.map_err(de::Error::custom)
} else {
serde_json::from_value(value)
.map(Self::Resolved)
.map_err(de::Error::custom)
}
}
}
impl ResolvedMcpEntry {
/// The inline resolved server, or `None` for an unresolved [`Reference`].
///
/// [`Reference`]: ResolvedMcpEntry::Reference
#[must_use]
pub fn as_resolved(&self) -> Option<&McpServerSettings> {
match self {
Self::Resolved(server) => Some(server),
Self::Reference(_) => None,
}
}
}
/// An unresolved reference to a server-defined MCP catalog entry.
///
/// `id` is kept as a plain `String` to keep `fabro-types` decoupled from the
/// `fabro-mcp-store` crate; the resolver maps it to a store id at resolve time.
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
#[serde(deny_unknown_fields)]
pub struct McpServerRef {
pub id: String,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub enabled: Option<bool>,
}
#[cfg(test)]
mod run_agent_settings_tests {
use super::RunAgentSettings;
use super::{
McpServerRef, McpServerSettings, McpTransport, ResolvedMcpEntry, RunAgentSettings,
};
#[test]
fn deserializes_missing_fabro_tools_as_false() {
@ -1070,6 +1163,171 @@ mod run_agent_settings_tests {
assert!(!settings.fabro_tools);
}
/// Critical back-compat guarantee: a run spec persisted before
/// `ResolvedMcpEntry` existed stores `mcps` as a map of bare
/// `McpServerSettings` (no enum tag). It must still deserialize, with every
/// entry landing as `ResolvedMcpEntry::Resolved`.
#[test]
fn deserializes_old_format_bare_mcps_as_resolved_json() {
let settings: RunAgentSettings = serde_json::from_value(serde_json::json!({
"fabro_tools": true,
"permissions": null,
"mcps": {
"filesystem": {
"name": "filesystem",
"transport": {
"type": "stdio",
"command": ["npx", "server-filesystem"],
"env": {}
},
"startup_timeout_secs": 10,
"tool_timeout_secs": 60
}
}
}))
.expect("old-format run agent settings should deserialize");
let entry = settings
.mcps
.get("filesystem")
.expect("filesystem entry should be present");
match entry {
ResolvedMcpEntry::Resolved(server) => {
assert_eq!(server.name, "filesystem");
assert!(matches!(server.transport, McpTransport::Stdio { .. }));
}
ResolvedMcpEntry::Reference(_) => {
panic!("bare McpServerSettings must deserialize as Resolved, not Reference")
}
}
}
/// The same guarantee via TOML, since run specs are also persisted/read as
/// TOML config.
#[test]
fn deserializes_old_format_bare_mcps_as_resolved_toml() {
let toml = r#"
fabro_tools = true
[mcps.http_server]
name = "http_server"
startup_timeout_secs = 10
tool_timeout_secs = 60
[mcps.http_server.transport]
type = "http"
url = "https://example.com/mcp"
[mcps.http_server.transport.headers]
"#;
let settings: RunAgentSettings =
toml::from_str(toml).expect("old-format TOML run agent settings should deserialize");
match settings.mcps.get("http_server") {
Some(ResolvedMcpEntry::Resolved(server)) => {
assert_eq!(server.name, "http_server");
assert!(matches!(server.transport, McpTransport::Http { .. }));
}
other => panic!("expected Resolved http_server entry, got {other:?}"),
}
}
/// A `{ id, enabled }`-shaped value deserializes as `Reference` (it has no
/// `name`/`transport`, and `deny_unknown_fields` keeps it from matching a
/// server config), while a full server config deserializes as `Resolved`.
#[test]
fn distinguishes_reference_from_resolved() {
let settings: RunAgentSettings = serde_json::from_value(serde_json::json!({
"mcps": {
"sentry": { "id": "sentry", "enabled": true },
"linear": { "id": "linear" },
"inline": {
"name": "inline",
"transport": {
"type": "stdio",
"command": ["my-server"],
"env": {}
},
"startup_timeout_secs": 5,
"tool_timeout_secs": 30
}
}
}))
.expect("mixed reference/resolved settings should deserialize");
assert_eq!(
settings.mcps.get("sentry"),
Some(&ResolvedMcpEntry::Reference(McpServerRef {
id: "sentry".to_string(),
enabled: Some(true),
}))
);
assert_eq!(
settings.mcps.get("linear"),
Some(&ResolvedMcpEntry::Reference(McpServerRef {
id: "linear".to_string(),
enabled: None,
}))
);
match settings.mcps.get("inline") {
Some(ResolvedMcpEntry::Resolved(server)) => assert_eq!(server.name, "inline"),
other => panic!("expected Resolved inline entry, got {other:?}"),
}
}
#[test]
fn rejects_entry_mixing_reference_and_inline_server_fields() {
let err = serde_json::from_value::<RunAgentSettings>(serde_json::json!({
"mcps": {
"mixed": {
"id": "catalog-server",
"name": "inline",
"transport": {
"type": "stdio",
"command": ["my-server"],
"env": {}
},
"startup_timeout_secs": 5,
"tool_timeout_secs": 30
}
}
}))
.expect_err("mixed reference/server entry should be rejected");
assert!(
err.to_string()
.contains("cannot mix catalog reference fields"),
"unexpected error: {err}"
);
}
/// `Resolved` serializes back out as a bare `McpServerSettings` (no enum
/// tag) so newly written run specs stay readable by any old reader and the
/// round-trip is stable.
#[test]
fn resolved_serializes_as_bare_server_settings() {
let server = McpServerSettings {
name: "demo".to_string(),
transport: McpTransport::Stdio {
command: vec!["demo-server".to_string()],
env: std::collections::HashMap::new(),
},
..McpServerSettings::default()
};
let entry = ResolvedMcpEntry::Resolved(server.clone());
let value = serde_json::to_value(&entry).expect("entry should serialize");
// No "Resolved" tag — it serializes as the bare server settings.
assert_eq!(
value,
serde_json::to_value(&server).expect("server serializes")
);
let round_tripped: ResolvedMcpEntry =
serde_json::from_value(value).expect("entry should round-trip");
assert_eq!(round_tripped, entry);
}
}
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]

20
lib/crates/fabro-workflow/src/operations/start.rs
View file

@ -18,7 +18,7 @@ use fabro_static::EnvVars;
use fabro_types::settings::run::{
ApprovalMode, HookDefinition as ResolvedHookDefinition, HookEvent as ResolvedHookEvent,
HookType as ResolvedHookType, McpServerSettings as ResolvedMcpServerSettings,
PullRequestSettings, RunMode, RunModelSettings as ResolvedRunModelSettings,
PullRequestSettings, ResolvedMcpEntry, RunMode, RunModelSettings as ResolvedRunModelSettings,
RunNamespace as ResolvedRunSettings, TlsMode as ResolvedTlsMode,
};
use fabro_types::settings::{ModelRegistry, ResolvedModelRef};
@ -377,8 +377,22 @@ impl RunSession {
let mcp_servers = resolved
.agent
.mcps
.values()
.map(|settings| runtime_mcp_server(settings, process_env_var))
.iter()
.map(|(key, entry)| match entry {
ResolvedMcpEntry::Resolved(server) => runtime_mcp_server(server, process_env_var),
// References must be resolved to concrete servers before the run
// spec is persisted (server-side run-preparation pass). Reaching
// worker startup with an unresolved reference is an invariant
// violation, so fail loudly rather than silently dropping it.
ResolvedMcpEntry::Reference(reference) => {
let message = format!(
"unresolved MCP server reference `{key}` (id `{}`) reached worker \
startup; references must be resolved before the run spec is persisted",
reference.id
);
Err(Error::engine(message))
}
})
.collect::<Result<Vec<_>, _>>()?;
let sandbox = match sandbox_provider {