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): satisfy MCP catalog clippy lints

Browse source
This commit is contained in:
Scott Werner 2026-06-30 14:46:27 -04:00
parent 0ce756ec64
commit 34cfbbed54
4 changed files with 63 additions and 68 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

76
lib/crates/fabro-config/src/layers/run.rs
View file

@ -576,44 +576,12 @@ impl<'de> Deserialize<'de> for McpEntryLayer {
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");
let has_inline_fields = map.contains_key("type")
|| map.contains_key("script")
|| map.contains_key("command")
|| map.contains_key("url")
|| map.contains_key("headers")
|| map.contains_key("port")
|| map.contains_key("env")
|| map.contains_key("startup_timeout")
|| map.contains_key("tool_timeout");
if has_reference_fields && has_inline_fields {
return Err(de::Error::custom(
"MCP entry cannot mix catalog reference fields (`id`, `enabled`) with inline \
server fields",
));
}
if has_reference_fields {
#[derive(Deserialize)]
#[serde(deny_unknown_fields)]
struct McpReferenceLayer {
id: String,
#[serde(default)]
enabled: Option<bool>,
}
let reference: McpReferenceLayer =
serde_json::from_value(value).map_err(de::Error::custom)?;
return Ok(Self::Reference {
id: reference.id,
enabled: reference.enabled,
});
#[derive(Deserialize)]
#[serde(deny_unknown_fields)]
struct McpReferenceLayer {
id: String,
#[serde(default)]
enabled: Option<bool>,
}
#[derive(Deserialize)]
@ -665,6 +633,38 @@ impl<'de> Deserialize<'de> for McpEntryLayer {
},
}
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");
let has_inline_fields = map.contains_key("type")
|| map.contains_key("script")
|| map.contains_key("command")
|| map.contains_key("url")
|| map.contains_key("headers")
|| map.contains_key("port")
|| map.contains_key("env")
|| map.contains_key("startup_timeout")
|| map.contains_key("tool_timeout");
if has_reference_fields && has_inline_fields {
return Err(de::Error::custom(
"MCP entry cannot mix catalog reference fields (`id`, `enabled`) with inline \
server fields",
));
}
if has_reference_fields {
let reference: McpReferenceLayer =
serde_json::from_value(value).map_err(de::Error::custom)?;
return Ok(Self::Reference {
id: reference.id,
enabled: reference.enabled,
});
}
match serde_json::from_value(value).map_err(de::Error::custom)? {
TaggedMcpEntryLayer::Http {
enabled,

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

@ -1019,7 +1019,9 @@ mod run_agent_mcps {
use std::collections::HashMap;
use fabro_types::settings::run::{McpServerSettings, McpTransport, ResolvedMcpEntry};
use fabro_types::settings::run::{
McpHttpProtocol, McpServerSettings, McpTransport, ResolvedMcpEntry,
};
use crate::layers::Combine;
use crate::tests::seeded_environment_catalog;
@ -1043,7 +1045,7 @@ mod run_agent_mcps {
HashMap::from([("sentry".to_string(), McpServerSettings {
name: "sentry".to_string(),
transport: McpTransport::Http {
protocol: fabro_types::settings::run::McpHttpProtocol::default(),
protocol: McpHttpProtocol::default(),
url: "https://sentry.example.com/mcp".to_string(),
headers: HashMap::from([(
"Authorization".to_string(),

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

@ -4,6 +4,7 @@ use std::path::{Path, PathBuf};
use std::sync::RwLock;
use std::time::{SystemTime, UNIX_EPOCH};
use fabro_types::settings::run::McpServerSettings;
use fabro_types::{
McpServerDefinition, McpServerDraft, McpServerId, McpServerReplace, McpServerRevision,
McpServerView,
@ -18,9 +19,8 @@ use crate::model;
/// Durable per-file TOML store for server-managed MCP server definitions.
///
/// Concrete by design (no trait): a future FS→SQL move is a one-time migration,
/// not a runtime backend choice. The migration seam is the async, storage-
/// agnostic method surface; only [`McpServerStore::load`] knows about the
/// filesystem.
/// not a runtime backend choice. The method surface keeps callers storage-
/// agnostic; only [`McpServerStore::load`] knows about the filesystem.
#[derive(Debug)]
pub struct McpServerStore {
dir: PathBuf,
@ -55,23 +55,21 @@ impl McpServerStore {
self.defs.write().expect("mcp server store lock poisoned")
}
pub async fn list(&self) -> Vec<McpServerDefinition> {
pub fn list(&self) -> Vec<McpServerDefinition> {
let defs = self.read_defs();
let mut values = defs.values().cloned().collect::<Vec<_>>();
values.sort_by(|left, right| left.id.cmp(&right.id));
values
}
pub async fn list_views(&self) -> Vec<McpServerView> {
pub fn list_views(&self) -> Vec<McpServerView> {
let defs = self.read_defs();
let mut values = defs.values().map(McpServerView::from).collect::<Vec<_>>();
values.sort_by(|left, right| left.id.cmp(&right.id));
values
}
pub fn catalog_settings(
&self,
) -> HashMap<String, fabro_types::settings::run::McpServerSettings> {
pub fn catalog_settings(&self) -> HashMap<String, McpServerSettings> {
let defs = self.read_defs();
defs.iter()
.map(|(id, definition)| (id.to_string(), server_settings_from_definition(definition)))
@ -81,14 +79,14 @@ impl McpServerStore {
/// Sorted ids only, without cloning the (potentially sensitive) env/header
/// maps carried by full definitions. Used by missing-reference errors to
/// list available ids cheaply.
pub async fn ids(&self) -> Vec<McpServerId> {
pub fn ids(&self) -> Vec<McpServerId> {
let defs = self.read_defs();
let mut ids = defs.keys().cloned().collect::<Vec<_>>();
ids.sort();
ids
}
pub async fn get(&self, id: &McpServerId) -> Option<McpServerDefinition> {
pub fn get(&self, id: &McpServerId) -> Option<McpServerDefinition> {
self.read_defs().get(id).cloned()
}
@ -171,10 +169,8 @@ fn check_revision(
Ok(())
}
fn server_settings_from_definition(
definition: &McpServerDefinition,
) -> fabro_types::settings::run::McpServerSettings {
fabro_types::settings::run::McpServerSettings {
fn server_settings_from_definition(definition: &McpServerDefinition) -> McpServerSettings {
McpServerSettings {
name: definition.id.to_string(),
transport: definition.transport.clone(),
current_dir: None,
@ -367,8 +363,8 @@ mod tests {
let dir = tempfile::tempdir().unwrap();
let store = McpServerStore::load(dir.path().join("mcps")).unwrap();
assert!(store.list().await.is_empty());
assert!(store.ids().await.is_empty());
assert!(store.list().is_empty());
assert!(store.ids().is_empty());
}
#[tokio::test]
@ -397,7 +393,7 @@ url = "https://sentry.example.com/mcp"
.unwrap();
let store = McpServerStore::load(&mcp_dir).unwrap();
let defs = store.list().await;
let defs = store.list();
assert_eq!(defs.len(), 1);
assert_eq!(defs[0].id.as_str(), "sentry");
@ -447,10 +443,10 @@ url = "https://sentry.example.com/mcp"
McpServerRevision::from_bytes(persisted.as_bytes())
);
assert_eq!(store.get(&created.id).await.unwrap(), created);
let listed = store.list().await;
assert_eq!(store.get(&created.id).unwrap(), created);
let listed = store.list();
assert_eq!(listed.len(), 1);
assert_eq!(store.ids().await, vec![created.id.clone()]);
assert_eq!(store.ids(), vec![created.id.clone()]);
let replaced = store
.replace(&created.id, &created.revision, replacement("Sentry v2"))
@ -458,13 +454,10 @@ url = "https://sentry.example.com/mcp"
.unwrap();
assert_ne!(replaced.revision, created.revision);
assert_eq!(replaced.display_name, "Sentry v2");
assert_eq!(
store.get(&created.id).await.unwrap().revision,
replaced.revision
);
assert_eq!(store.get(&created.id).unwrap().revision, replaced.revision);
store.delete(&created.id, &replaced.revision).await.unwrap();
assert!(store.get(&created.id).await.is_none());
assert!(store.get(&created.id).is_none());
assert!(!path.exists());
}
@ -482,7 +475,7 @@ url = "https://sentry.example.com/mcp"
assert!(matches!(err, McpServerStoreError::StaleRevision { .. }));
// The on-disk and in-memory definition is unchanged after a rejected replace.
assert_eq!(store.get(&created.id).await.unwrap(), created);
assert_eq!(store.get(&created.id).unwrap(), created);
}
#[tokio::test]

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

@ -39,7 +39,7 @@ 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_views().await;
let data = state.mcp_server_store().list_views();
let total = data.len();
(
StatusCode::OK,
@ -75,7 +75,7 @@ async fn get_mcp_server(
Path(id): Path<String>,
) -> Result<Response, ApiError> {
let id = parse_path_id(id)?;
match state.mcp_server_store().get(&id).await {
match state.mcp_server_store().get(&id) {
Some(definition) => Ok(mcp_server_with_etag_response(StatusCode::OK, definition)),
None => Err(ApiError::not_found(format!("mcp server not found: {id}"))),
}