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

fix(server): return credential-omitting view; align display_name with spec

Browse source 
The mcp-servers handlers serialized the internal McpServerDefinition, which
diverged from the OpenAPI `McpServer` schema (and the generated TS client) two
ways: the field is `display_name` not `name`, and read responses use
`McpTransportView` (env/header *names* only) rather than the full transport with
values. So a spec-conformant client could neither create/replace (requests send
`display_name`, rejected by deny_unknown_fields) nor read the returned fields,
and stored env/header secret values were echoed back on GET/list.

- Rename `name` -> `display_name` across the domain and persistence types so the
  write bodies match `CreateMcpServerRequest`/`ReplaceMcpServerRequest`.
- Add `McpServerView`/`McpTransportView` read projections (sorted, value-omitting
  `env_keys`/`header_keys`) and return them from every read/write response.
- Map `McpServer` -> `McpServerView` and `McpTransportView` via with_replacement;
  `McpServerDefinition` is no longer a wire type.
- Rewrite the parity test to assert the spec JSON shape (display_name + view
  transport) instead of round-tripping the internal model, and assert the view
  omits values in the handler integration tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Scott Werner 2026-06-29 22:17:42 -04:00
parent 20cde16b59
commit e2021f3cb0
9 changed files with 250 additions and 89 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
lib/crates/fabro-api/build.rs
View file

@ -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",

47
lib/crates/fabro-api/src/lib.rs
View file

@ -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::*;

70
lib/crates/fabro-api/tests/mcp_server_round_trip.rs
View file

@ -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",

10
lib/crates/fabro-mcp-store/src/model.rs
View file

@ -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<String>,
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<PersistedMcpServer> 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,

18
lib/crates/fabro-mcp-store/src/store.rs
View file

@ -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

14
lib/crates/fabro-server/src/server/handler/mcp_servers.rs
View file

@ -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<McpServerDefinition>,
data: Vec<McpServerView>,
meta: McpServerListMeta,
}
@ -37,8 +39,9 @@ pub(super) fn routes() -> Router<Arc<AppState>> {
}
async fn list_mcp_servers(_auth: RequiredUser, State(state): State<Arc<AppState>>) -> 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<McpServerView> = definitions.into_iter().map(McpServerView::from).collect();
(
StatusCode::OK,
Json(McpServerListResponse {
@ -115,7 +118,8 @@ fn parse_path_id(id: String) -> Result<McpServerId, ApiError> {
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<McpServerStoreError> for ApiError {

33
lib/crates/fabro-server/tests/it/api/mcp_servers.rs
View file

@ -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))

3
lib/crates/fabro-types/src/lib.rs
View file

@ -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,

135
lib/crates/fabro-types/src/mcp_store.rs
View file

@ -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<String>,
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<String>,
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<String>,
pub transport: McpTransport,
@ -70,7 +75,7 @@ pub struct McpServerReplace {
impl From<McpServerDraft> 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<McpServerDraft> 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<String>,
pub transport: McpTransportView,
pub startup_timeout_secs: u64,
pub tool_timeout_secs: u64,
}
impl From<McpServerDefinition> 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<String>,
env_keys: Vec<String>,
},
Http {
#[serde(default)]
protocol: McpHttpProtocol,
url: String,
header_keys: Vec<String>,
},
Sandbox {
#[serde(default)]
protocol: McpHttpProtocol,
command: Vec<String>,
port: u16,
env_keys: Vec<String>,
},
}
impl From<McpTransport> 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<String, String>) -> Vec<String> {
let mut keys: Vec<String> = 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,