diff --git a/lib/crates/fabro-api/build.rs b/lib/crates/fabro-api/build.rs index 73bb5e0a8..0a81b9845 100644 --- a/lib/crates/fabro-api/build.rs +++ b/lib/crates/fabro-api/build.rs @@ -395,6 +395,7 @@ fn main() { "fabro_types::settings::run::McpTransport", &[], ), + ("McpTransportView", "fabro_types::McpTransportView", &[]), ("StageContextWindow", "fabro_types::StageContextWindow", &[]), ( "StageContextWindowProjection", @@ -649,8 +650,12 @@ fn main() { // generating parallel DTOs. The integer formats in the spec generate as // `i64`/`i32`, but the domain types use `u64`/`u16`; the replacements // ensure the signed widths never leak into the public client. - // (`McpTransport`/`McpHttpProtocol` are already mapped above.) - ("McpServer", "fabro_types::McpServerDefinition", &[]), + // (`McpTransport`/`McpTransportView`/`McpHttpProtocol` are mapped above.) + // + // Read responses use the value-omitting `McpServerView`/`McpTransportView` + // projection (env/header values are never returned); write requests carry + // the full transport, so they reuse the draft/replace domain types. + ("McpServer", "fabro_types::McpServerView", &[]), ("CreateMcpServerRequest", "fabro_types::McpServerDraft", &[]), ( "ReplaceMcpServerRequest", diff --git a/lib/crates/fabro-api/src/lib.rs b/lib/crates/fabro-api/src/lib.rs index f122007c7..da898f8c2 100644 --- a/lib/crates/fabro-api/src/lib.rs +++ b/lib/crates/fabro-api/src/lib.rs @@ -46,29 +46,30 @@ pub mod types { FailureCategory, FailureDetail, FailureSignature, GitContext, IdpIdentity, IntegrationConnectionKind, IntegrationConnectionState, IntegrationConnectionStatus, IntegrationProvider, IntegrationStatus, InterviewOption, InterviewQuestionRecord, - McpServerDefinition as McpServer, McpServerDraft as CreateMcpServerRequest, - McpServerProjection, McpServerReplace as ReplaceMcpServerRequest, McpServerStatus, Message, - PairId, PairMessageId, PairMessageRecord, PairMessageRequest, PairRecord, PairStartRequest, - PairStatus, PairTarget, PairTranscriptEntry, PairTranscriptResponse, - PendingInterviewRecord, PermissionLevel, PreRunPushOutcome, Principal, PullRequest, - PullRequestDetails, PullRequestDetailsStatus, PullRequestDetailsUnavailableReason, - PullRequestLink, PullRequestMeta, PullRequestResponse, QuestionType, RepositoryRef, Role, - Run, RunApproval, RunApprovalState, RunClientProvenance, RunEvent, - RunEventDetailContentKind, RunEventDetailResponse, RunFailure, RunPairStatusResponse, - RunProjection, RunProvenance, RunRunnableSource, RunSandbox, RunSandboxFailure, - RunSandboxInstance, RunSandboxKind, RunSandboxPlan, RunSandboxRuntime, RunServerProvenance, - RunSize, SandboxDetails, SandboxInfo, SandboxListMeta, SandboxListResponse, SandboxNetwork, - SandboxNetworkPolicy, SandboxNetworkPolicyMode, SandboxProviderKind, - SandboxProviderLookupError, SandboxResources, SandboxService, SandboxServiceListResponse, - SandboxState, SandboxTimestamps, SecretMetadata, SecretType, ServerSettings, SessionDetail, - SessionId, SessionMessage, SessionRecord, SessionStatus, SessionSummary, SessionTurn, - SkillsProjection, StageCompletion, StageContextWindow, StageContextWindowBreakdownItem, - StageContextWindowCategory, StageContextWindowCountMethod, StageContextWindowProjection, - StageContextWindowStaleness, StageContextWindowUnavailableReason, - StageContextWindowWarning, StageHandler, StageModelUsage, StageOutcome, StageProjection, - StageState, SubAgentProjection, SubAgentStatus, SystemActorKind, SystemIntegrationStatus, - SystemIntegrationsResponse, TodoListProjection, TurnId, UpdateVariableRequest, - UserPrincipal, Variable, VariableListResponse, WorkflowSettings, + McpServerDraft as CreateMcpServerRequest, McpServerProjection, + McpServerReplace as ReplaceMcpServerRequest, McpServerStatus, McpServerView as McpServer, + McpTransportView, Message, PairId, PairMessageId, PairMessageRecord, PairMessageRequest, + PairRecord, PairStartRequest, PairStatus, PairTarget, PairTranscriptEntry, + PairTranscriptResponse, PendingInterviewRecord, PermissionLevel, PreRunPushOutcome, + Principal, PullRequest, PullRequestDetails, PullRequestDetailsStatus, + PullRequestDetailsUnavailableReason, PullRequestLink, PullRequestMeta, PullRequestResponse, + QuestionType, RepositoryRef, Role, Run, RunApproval, RunApprovalState, RunClientProvenance, + RunEvent, RunEventDetailContentKind, RunEventDetailResponse, RunFailure, + RunPairStatusResponse, RunProjection, RunProvenance, RunRunnableSource, RunSandbox, + RunSandboxFailure, RunSandboxInstance, RunSandboxKind, RunSandboxPlan, RunSandboxRuntime, + RunServerProvenance, RunSize, SandboxDetails, SandboxInfo, SandboxListMeta, + SandboxListResponse, SandboxNetwork, SandboxNetworkPolicy, SandboxNetworkPolicyMode, + SandboxProviderKind, SandboxProviderLookupError, SandboxResources, SandboxService, + SandboxServiceListResponse, SandboxState, SandboxTimestamps, SecretMetadata, SecretType, + ServerSettings, SessionDetail, SessionId, SessionMessage, SessionRecord, SessionStatus, + SessionSummary, SessionTurn, SkillsProjection, StageCompletion, StageContextWindow, + StageContextWindowBreakdownItem, StageContextWindowCategory, StageContextWindowCountMethod, + StageContextWindowProjection, StageContextWindowStaleness, + StageContextWindowUnavailableReason, StageContextWindowWarning, StageHandler, + StageModelUsage, StageOutcome, StageProjection, StageState, SubAgentProjection, + SubAgentStatus, SystemActorKind, SystemIntegrationStatus, SystemIntegrationsResponse, + TodoListProjection, TurnId, UpdateVariableRequest, UserPrincipal, Variable, + VariableListResponse, WorkflowSettings, }; pub use crate::generated::types::*; diff --git a/lib/crates/fabro-api/tests/mcp_server_round_trip.rs b/lib/crates/fabro-api/tests/mcp_server_round_trip.rs index 91ce0ab3e..aabefde95 100644 --- a/lib/crates/fabro-api/tests/mcp_server_round_trip.rs +++ b/lib/crates/fabro-api/tests/mcp_server_round_trip.rs @@ -1,39 +1,47 @@ use fabro_api::types::{ CreateMcpServerRequest as ApiCreateMcpServerRequest, McpHttpProtocol as ApiMcpHttpProtocol, McpServer as ApiMcpServer, McpTransport as ApiMcpTransport, - ReplaceMcpServerRequest as ApiReplaceMcpServerRequest, + McpTransportView as ApiMcpTransportView, ReplaceMcpServerRequest as ApiReplaceMcpServerRequest, }; use fabro_types::settings::McpTransport; use fabro_types::settings::run::McpHttpProtocol; -use fabro_types::{McpServerDefinition, McpServerDraft, McpServerReplace}; +use fabro_types::{McpServerDraft, McpServerReplace, McpServerView, McpTransportView}; use serde_json::json; // Compile-time witnesses that the generated API types resolve to the same types // as the `fabro-types` domain types via `with_replacement(...)`. If progenitor // stops reusing the domain type, these functions stop type-checking and the -// build fails. This is what keeps the spec's signed integer formats (`i64` for -// the `u64` timeouts, `i32` for the `u16` sandbox port) from leaking into the +// build fails. This also keeps the spec's signed integer formats (`i64` for the +// `u64` timeouts, `i32` for the `u16` sandbox port) from leaking into the // public client. -const _: fn(ApiMcpServer) -> McpServerDefinition = |value| value; +// +// Read responses (`McpServer`) reuse the value-omitting `McpServerView` / +// `McpTransportView`; write requests reuse the full-transport draft/replace +// types. +const _: fn(ApiMcpServer) -> McpServerView = |value| value; +const _: fn(ApiMcpTransportView) -> McpTransportView = |value| value; const _: fn(ApiCreateMcpServerRequest) -> McpServerDraft = |value| value; const _: fn(ApiReplaceMcpServerRequest) -> McpServerReplace = |value| value; const _: fn(ApiMcpTransport) -> McpTransport = |value| value; const _: fn(ApiMcpHttpProtocol) -> McpHttpProtocol = |value| value; +// The response JSON below is the exact shape the spec's `McpServer` schema +// declares: `display_name` (not the internal `name`), and a `McpTransportView` +// transport whose env/header *values* are replaced by `*_keys` name lists. If +// the handler ever serialized the internal `McpServerDefinition` instead, these +// round-trips would fail. #[test] -fn mcp_server_response_round_trips_http_transport_json_shape() { +fn mcp_server_response_round_trips_http_transport_view_shape() { let value = json!({ "id": "sentry", "revision": "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", - "name": "Sentry", + "display_name": "Sentry", "description": "Production Sentry MCP server.", "transport": { "type": "http", "protocol": "streamable_http", "url": "https://sentry.example.com/mcp", - "headers": { - "X-Org": "fabro" - } + "header_keys": ["X-Org"] }, "startup_timeout_secs": 10, "tool_timeout_secs": 60 @@ -44,16 +52,16 @@ fn mcp_server_response_round_trips_http_transport_json_shape() { } #[test] -fn mcp_server_response_round_trips_null_description() { +fn mcp_server_response_round_trips_stdio_view_and_null_description() { let value = json!({ "id": "local", "revision": "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", - "name": "Local", + "display_name": "Local", "description": null, "transport": { "type": "stdio", "command": ["npx", "@modelcontextprotocol/server-filesystem"], - "env": {} + "env_keys": ["HOME", "PATH"] }, "startup_timeout_secs": 10, "tool_timeout_secs": 60 @@ -64,13 +72,37 @@ fn mcp_server_response_round_trips_null_description() { } #[test] -fn create_mcp_server_request_round_trips_sandbox_transport_json_shape() { - // The sandbox transport carries a `port`, whose spec format is `int32`. - // Reusing the domain type pins it to `u16`, so this round-trip also guards - // against the signed-width leak. +fn mcp_server_response_round_trips_sandbox_view_shape() { + // The sandbox view carries a `port`, whose spec format is `int32`. Reusing + // `McpTransportView` pins it to `u16`, so this round-trip also guards the + // signed-width leak on the read side. let value = json!({ "id": "sandbox-mcp", - "name": "Sandbox MCP", + "revision": "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + "display_name": "Sandbox MCP", + "description": null, + "transport": { + "type": "sandbox", + "protocol": "sse", + "command": ["./serve"], + "port": 8080, + "env_keys": ["NODE_ENV"] + }, + "startup_timeout_secs": 15, + "tool_timeout_secs": 90 + }); + + let api: ApiMcpServer = serde_json::from_value(value.clone()).unwrap(); + assert_eq!(serde_json::to_value(api).unwrap(), value); +} + +// Write requests use `display_name` and the full `McpTransport`, since a client +// must be able to supply env/header values when creating or replacing. +#[test] +fn create_mcp_server_request_round_trips_sandbox_transport_json_shape() { + let value = json!({ + "id": "sandbox-mcp", + "display_name": "Sandbox MCP", "transport": { "type": "sandbox", "protocol": "sse", @@ -91,7 +123,7 @@ fn create_mcp_server_request_round_trips_sandbox_transport_json_shape() { #[test] fn replace_mcp_server_request_round_trips_http_transport_json_shape() { let value = json!({ - "name": "Sentry v2", + "display_name": "Sentry v2", "description": "Updated.", "transport": { "type": "http", diff --git a/lib/crates/fabro-mcp-store/src/model.rs b/lib/crates/fabro-mcp-store/src/model.rs index e2564284b..5ae656b2b 100644 --- a/lib/crates/fabro-mcp-store/src/model.rs +++ b/lib/crates/fabro-mcp-store/src/model.rs @@ -21,7 +21,7 @@ use crate::error::McpServerStoreError; #[derive(Debug, Clone, PartialEq, Deserialize)] #[serde(deny_unknown_fields)] struct PersistedMcpServer { - name: String, + display_name: String, #[serde(default)] description: Option, transport: McpTransport, @@ -31,7 +31,7 @@ struct PersistedMcpServer { #[derive(Serialize)] struct PersistedMcpServerRef<'a> { - name: &'a str, + display_name: &'a str, #[serde(default, skip_serializing_if = "Option::is_none")] description: Option<&'a str>, transport: &'a McpTransport, @@ -42,7 +42,7 @@ struct PersistedMcpServerRef<'a> { impl<'a> From<&'a McpServerReplace> for PersistedMcpServerRef<'a> { fn from(value: &'a McpServerReplace) -> Self { Self { - name: &value.name, + display_name: &value.display_name, description: value.description.as_deref(), transport: &value.transport, startup_timeout_secs: value.startup_timeout_secs, @@ -54,7 +54,7 @@ impl<'a> From<&'a McpServerReplace> for PersistedMcpServerRef<'a> { impl From for McpServerReplace { fn from(value: PersistedMcpServer) -> Self { Self { - name: value.name, + display_name: value.display_name, description: value.description, transport: value.transport, startup_timeout_secs: value.startup_timeout_secs, @@ -102,7 +102,7 @@ fn assemble( McpServerDefinition { id, revision, - name: replace.name, + display_name: replace.display_name, description: replace.description, transport: replace.transport, startup_timeout_secs: replace.startup_timeout_secs, diff --git a/lib/crates/fabro-mcp-store/src/store.rs b/lib/crates/fabro-mcp-store/src/store.rs index 2440b5c5b..c8554a386 100644 --- a/lib/crates/fabro-mcp-store/src/store.rs +++ b/lib/crates/fabro-mcp-store/src/store.rs @@ -298,10 +298,10 @@ mod tests { } } - fn draft(id: &str, name: &str) -> McpServerDraft { + fn draft(id: &str, display_name: &str) -> McpServerDraft { McpServerDraft { id: McpServerId::new(id).unwrap(), - name: name.to_string(), + display_name: display_name.to_string(), description: None, transport: http_transport("https://example.com/mcp"), startup_timeout_secs: 10, @@ -309,9 +309,9 @@ mod tests { } } - fn replacement(name: &str) -> McpServerReplace { + fn replacement(display_name: &str) -> McpServerReplace { McpServerReplace { - name: name.to_string(), + display_name: display_name.to_string(), description: Some("updated".to_string()), transport: http_transport("https://example.com/mcp/v2"), startup_timeout_secs: 15, @@ -339,7 +339,7 @@ mod tests { fs::write( mcp_dir.join("sentry.toml"), r#" -name = "Sentry" +display_name = "Sentry" startup_timeout_secs = 10 tool_timeout_secs = 60 @@ -358,7 +358,7 @@ url = "https://sentry.example.com/mcp" assert_eq!(defs.len(), 1); assert_eq!(defs[0].id.as_str(), "sentry"); - assert_eq!(defs[0].name, "Sentry"); + assert_eq!(defs[0].display_name, "Sentry"); } #[tokio::test] @@ -379,7 +379,7 @@ url = "https://sentry.example.com/mcp" let dir = tempfile::tempdir().unwrap(); let mcp_dir = dir.path().join("mcps"); fs::create_dir_all(&mcp_dir).await.unwrap(); - fs::write(mcp_dir.join("Bad Name.toml"), "name = \"Bad\"") + fs::write(mcp_dir.join("Bad Name.toml"), "display_name = \"Bad\"") .await .unwrap(); @@ -396,7 +396,7 @@ url = "https://sentry.example.com/mcp" let created = store.create(draft("sentry", "Sentry")).await.unwrap(); let path = mcp_dir.join("sentry.toml"); let persisted = fs::read_to_string(&path).await.unwrap(); - assert!(persisted.contains("name = \"Sentry\"")); + assert!(persisted.contains("display_name = \"Sentry\"")); assert!(!top_level_lines(&persisted).any(|line| line.starts_with("id = "))); assert!(!top_level_lines(&persisted).any(|line| line.starts_with("revision = "))); assert_eq!( @@ -414,7 +414,7 @@ url = "https://sentry.example.com/mcp" .await .unwrap(); assert_ne!(replaced.revision, created.revision); - assert_eq!(replaced.name, "Sentry v2"); + assert_eq!(replaced.display_name, "Sentry v2"); assert_eq!( store.get(&created.id).await.unwrap().revision, replaced.revision diff --git a/lib/crates/fabro-server/src/server/handler/mcp_servers.rs b/lib/crates/fabro-server/src/server/handler/mcp_servers.rs index 8b0097ef6..f14d444e0 100644 --- a/lib/crates/fabro-server/src/server/handler/mcp_servers.rs +++ b/lib/crates/fabro-server/src/server/handler/mcp_servers.rs @@ -2,7 +2,9 @@ use std::sync::Arc; use axum::http::HeaderMap; use fabro_mcp_store::McpServerStoreError; -use fabro_types::{McpServerDefinition, McpServerDraft, McpServerId, McpServerReplace}; +use fabro_types::{ + McpServerDefinition, McpServerDraft, McpServerId, McpServerReplace, McpServerView, +}; use serde::Serialize; use super::super::{ @@ -13,7 +15,7 @@ use super::{json_with_etag_response, parse_required_if_match}; #[derive(Serialize)] struct McpServerListResponse { - data: Vec, + data: Vec, meta: McpServerListMeta, } @@ -37,8 +39,9 @@ pub(super) fn routes() -> Router> { } async fn list_mcp_servers(_auth: RequiredUser, State(state): State>) -> Response { - let data = state.mcp_server_store().list().await; - let total = data.len(); + let definitions = state.mcp_server_store().list().await; + let total = definitions.len(); + let data: Vec = definitions.into_iter().map(McpServerView::from).collect(); ( StatusCode::OK, Json(McpServerListResponse { @@ -115,7 +118,8 @@ fn parse_path_id(id: String) -> Result { fn mcp_server_with_etag_response(status: StatusCode, definition: McpServerDefinition) -> Response { let revision = definition.revision.clone(); - json_with_etag_response(status, "mcp server", &revision, definition) + let view = McpServerView::from(definition); + json_with_etag_response(status, "mcp server", &revision, view) } impl From for ApiError { diff --git a/lib/crates/fabro-server/tests/it/api/mcp_servers.rs b/lib/crates/fabro-server/tests/it/api/mcp_servers.rs index 5ae238fdd..a7f5b5be2 100644 --- a/lib/crates/fabro-server/tests/it/api/mcp_servers.rs +++ b/lib/crates/fabro-server/tests/it/api/mcp_servers.rs @@ -9,10 +9,10 @@ use tower::ServiceExt; use crate::helpers::{checked_response, response_json, response_status}; -fn mcp_server_body(id: &str, name: &str) -> Value { +fn mcp_server_body(id: &str, display_name: &str) -> Value { json!({ "id": id, - "name": name, + "display_name": display_name, "description": "Production MCP server.", "transport": { "type": "http", @@ -27,9 +27,9 @@ fn mcp_server_body(id: &str, name: &str) -> Value { }) } -fn replacement_body(name: &str) -> Value { +fn replacement_body(display_name: &str) -> Value { json!({ - "name": name, + "display_name": display_name, "description": null, "transport": { "type": "http", @@ -163,17 +163,34 @@ async fn create_mcp_server_returns_etag_and_persists_sibling_toml_file() { let body = crate::helpers::body_json(response.into_body()).await; assert_eq!(body["id"], "sentry"); - assert_eq!(body["name"], "Sentry"); + assert_eq!(body["display_name"], "Sentry"); assert_eq!(etag, format!("\"{}\"", revision_from(&body))); assert!(mcp_dir.join("sentry.toml").exists()); + // The response is the value-omitting view: header *names* are returned, but + // the stored header value is not. + assert_eq!(body["transport"]["header_keys"], json!(["X-Org"])); + assert!( + body["transport"].get("headers").is_none(), + "response must not echo transport header values" + ); + let persisted = persisted_mcp_server_toml(&mcp_dir, "sentry").await; assert_eq!( - persisted.get("name").and_then(toml::Value::as_str), + persisted.get("display_name").and_then(toml::Value::as_str), Some("Sentry") ); assert!(persisted.get("id").is_none()); assert!(persisted.get("revision").is_none()); + // The value the view omits is still persisted on disk for the runtime to use. + assert_eq!( + persisted + .get("transport") + .and_then(|transport| transport.get("headers")) + .and_then(|headers| headers.get("X-Org")) + .and_then(toml::Value::as_str), + Some("fabro") + ); } #[tokio::test] @@ -308,7 +325,7 @@ async fn replace_mcp_server_accepts_unquoted_if_match_and_returns_new_etag() { .to_string(); let body = crate::helpers::body_json(response.into_body()).await; - assert_eq!(body["name"], "Sentry v2"); + assert_eq!(body["display_name"], "Sentry v2"); assert_eq!(body["transport"]["protocol"], "sse"); assert_ne!(body["revision"], revision); assert_eq!(etag, format!("\"{}\"", revision_from(&body))); @@ -452,7 +469,7 @@ async fn delete_mcp_server_removes_file_and_resource() { async fn empty_mcp_server_name_is_unprocessable() { let (app, _temp_dir, _mcp_dir) = mcp_server_app(); let mut body = mcp_server_body("sentry", "Sentry"); - body["name"] = json!(" "); + body["display_name"] = json!(" "); let response = app .oneshot(json_request(Method::POST, "/mcp-servers", &body)) diff --git a/lib/crates/fabro-types/src/lib.rs b/lib/crates/fabro-types/src/lib.rs index 7edf91066..84567d913 100644 --- a/lib/crates/fabro-types/src/lib.rs +++ b/lib/crates/fabro-types/src/lib.rs @@ -78,7 +78,8 @@ pub use llm_backend::AgentBackend; pub use manifest_path::{ManifestPath, ManifestPathParseError}; pub use mcp_store::{ McpServerDefinition, McpServerDraft, McpServerId, McpServerReplace, McpServerRevision, - McpServerRevisionParseError, McpServerValidationError, validate_mcp_server_fields, + McpServerRevisionParseError, McpServerValidationError, McpServerView, McpTransportView, + validate_mcp_server_fields, }; pub use outcome::{ FailureCategory, FailureDetail, NodeResult, Outcome, OutcomeMeta, StageOutcome, StageState, diff --git a/lib/crates/fabro-types/src/mcp_store.rs b/lib/crates/fabro-types/src/mcp_store.rs index 8b2ff256d..69b1a6fdd 100644 --- a/lib/crates/fabro-types/src/mcp_store.rs +++ b/lib/crates/fabro-types/src/mcp_store.rs @@ -9,7 +9,13 @@ //! Transport is the existing [`McpTransport`](crate::settings::McpTransport) //! reused verbatim, so a stored definition uses the same `stdio`/`http`/ //! `sandbox` shape as inline MCP config. +//! +//! Read APIs do not return [`McpServerDefinition`] directly. They return +//! [`McpServerView`], a value-omitting projection whose transport carries only +//! the env/header *names* ([`McpTransportView`]) so stored secret-bearing +//! values are never sent back to a client. +use std::collections::HashMap; use std::fmt; use std::str::FromStr; @@ -18,21 +24,20 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer}; use sha2::{Digest, Sha256}; use crate::settings::McpTransport; +use crate::settings::run::McpHttpProtocol; /// A server-managed MCP server definition. /// /// `id` and `revision` are derived (filename stem + content hash of the -/// persisted TOML bytes) and are not stored in the persisted TOML body. It -/// derives serde because it is the public wire type for `GET`/`POST`/`PUT` MCP -/// server responses (reused by `fabro-api` via `with_replacement` rather than a -/// parallel DTO). The serialized shape is the struct's own fields — `id` and -/// `revision` are present on the wire but not in the persisted TOML body. -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] +/// persisted TOML bytes) and are not stored in the persisted TOML body. This is +/// the internal/persistence model and carries full transport values; it is not +/// serialized to clients. Read APIs return [`McpServerView`] instead, which +/// omits env/header values. +#[derive(Debug, Clone, PartialEq)] pub struct McpServerDefinition { pub id: McpServerId, pub revision: McpServerRevision, - pub name: String, + pub display_name: String, pub description: Option, pub transport: McpTransport, pub startup_timeout_secs: u64, @@ -45,7 +50,7 @@ pub struct McpServerDefinition { #[serde(deny_unknown_fields)] pub struct McpServerDraft { pub id: McpServerId, - pub name: String, + pub display_name: String, #[serde(default, skip_serializing_if = "Option::is_none")] pub description: Option, pub transport: McpTransport, @@ -59,7 +64,7 @@ pub struct McpServerDraft { #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub struct McpServerReplace { - pub name: String, + pub display_name: String, #[serde(default, skip_serializing_if = "Option::is_none")] pub description: Option, pub transport: McpTransport, @@ -70,7 +75,7 @@ pub struct McpServerReplace { impl From for (McpServerId, McpServerReplace) { fn from(value: McpServerDraft) -> Self { (value.id, McpServerReplace { - name: value.name, + display_name: value.display_name, description: value.description, transport: value.transport, startup_timeout_secs: value.startup_timeout_secs, @@ -79,6 +84,102 @@ impl From for (McpServerId, McpServerReplace) { } } +/// The read-side projection of a definition returned by catalog read APIs. +/// +/// Identical to [`McpServerDefinition`] except that the transport is the +/// value-omitting [`McpTransportView`], so stored env/header values never leave +/// the server. This is the wire type behind the OpenAPI `McpServer` schema +/// (reused by `fabro-api` via `with_replacement`). +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct McpServerView { + pub id: McpServerId, + pub revision: McpServerRevision, + pub display_name: String, + pub description: Option, + pub transport: McpTransportView, + pub startup_timeout_secs: u64, + pub tool_timeout_secs: u64, +} + +impl From for McpServerView { + fn from(value: McpServerDefinition) -> Self { + Self { + id: value.id, + revision: value.revision, + display_name: value.display_name, + description: value.description, + transport: value.transport.into(), + startup_timeout_secs: value.startup_timeout_secs, + tool_timeout_secs: value.tool_timeout_secs, + } + } +} + +/// The read-side projection of [`McpTransport`]. It mirrors the transport shape +/// but replaces the `env`/`headers` value maps with sorted `env_keys`/ +/// `header_keys` name lists, so secret-bearing values are never returned. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(tag = "type", rename_all = "snake_case")] +pub enum McpTransportView { + Stdio { + command: Vec, + env_keys: Vec, + }, + Http { + #[serde(default)] + protocol: McpHttpProtocol, + url: String, + header_keys: Vec, + }, + Sandbox { + #[serde(default)] + protocol: McpHttpProtocol, + command: Vec, + port: u16, + env_keys: Vec, + }, +} + +impl From for McpTransportView { + fn from(value: McpTransport) -> Self { + match value { + McpTransport::Stdio { command, env } => Self::Stdio { + command, + env_keys: sorted_keys(env), + }, + McpTransport::Http { + protocol, + url, + headers, + } => Self::Http { + protocol, + url, + header_keys: sorted_keys(headers), + }, + McpTransport::Sandbox { + protocol, + command, + port, + env, + } => Self::Sandbox { + protocol, + command, + port, + env_keys: sorted_keys(env), + }, + } + } +} + +/// Collect a value map's keys into a sorted list, so the projection is +/// deterministic regardless of the map's iteration order. +fn sorted_keys(map: HashMap) -> Vec { + let mut keys: Vec = map.into_keys().collect(); + keys.sort(); + keys +} + /// Validation errors for MCP server domain fields. #[derive(Debug, Clone, PartialEq, Eq)] pub enum McpServerValidationError { @@ -96,7 +197,7 @@ impl fmt::Display for McpServerValidationError { "mcp server id {value:?} must match [a-z0-9][a-z0-9-]{{0,62}}" ) } - Self::EmptyName => f.write_str("mcp server name must not be empty"), + Self::EmptyName => f.write_str("mcp server display name must not be empty"), Self::InvalidTransport { reason } => { write!(f, "mcp server transport is invalid: {reason}") } @@ -251,7 +352,7 @@ fn is_valid_mcp_server_id(value: &str) -> bool { pub fn validate_mcp_server_fields( replace: &McpServerReplace, ) -> Result<(), McpServerValidationError> { - if replace.name.trim().is_empty() { + if replace.display_name.trim().is_empty() { return Err(McpServerValidationError::EmptyName); } validate_transport(&replace.transport) @@ -324,7 +425,7 @@ mod tests { #[test] fn validation_rejects_empty_name() { let replace = McpServerReplace { - name: " ".to_string(), + display_name: " ".to_string(), description: None, transport: http_transport(), startup_timeout_secs: 10, @@ -336,7 +437,7 @@ mod tests { #[test] fn validation_rejects_empty_transport_command() { let replace = McpServerReplace { - name: "Local".to_string(), + display_name: "Local".to_string(), description: None, transport: McpTransport::Stdio { command: Vec::new(), @@ -351,7 +452,7 @@ mod tests { #[test] fn validation_rejects_blank_transport_program() { let replace = McpServerReplace { - name: "Local".to_string(), + display_name: "Local".to_string(), description: None, transport: McpTransport::Stdio { command: vec![" ".to_string(), "--arg".to_string()], @@ -366,7 +467,7 @@ mod tests { #[test] fn validation_accepts_well_formed_definition() { let replace = McpServerReplace { - name: "Sentry".to_string(), + display_name: "Sentry".to_string(), description: Some("Issue tracker".to_string()), transport: http_transport(), startup_timeout_secs: 10,