Skip to content

Commit 54d531d

Browse files
committed
test(core): cover the custom-server id-collision guard
Extracts the duplicate-normalized-id check in UserSpaceSyncService into `ensure_unique_server_ids` and adds unit tests: two mcpServers keys that normalize to the same id (e.g. "My Server" / "my_server" -> "myserver") are rejected with a clear error, and distinct ids pass. Covers the collision branch without needing a repo or an on-disk file. Signed-off-by: Mohammod Al Amin Ashik <maa.ashik00@gmail.com>
1 parent 0367c4a commit 54d531d

1 file changed

Lines changed: 63 additions & 18 deletions

File tree

crates/mcpmux-core/src/application/user_space_sync.rs

Lines changed: 63 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use anyhow::{Context, Result};
1111
use tracing::{debug, info};
1212

1313
use crate::domain::config::UserSpaceConfig;
14-
use crate::domain::{InstallationSource, InstalledServer};
14+
use crate::domain::{InstallationSource, InstalledServer, ServerDefinition};
1515
use crate::repository::InstalledServerRepository;
1616

1717
/// Result of a sync operation
@@ -48,6 +48,29 @@ impl UserSpaceSyncService {
4848
Self { installed_repo }
4949
}
5050

51+
/// Ensure no two user-config entries normalize to the same MCP server id.
52+
///
53+
/// User-config keys are normalized into MCP-safe server ids; if two entries
54+
/// collapse to the same id the sync loop would update the same
55+
/// `InstalledServer` row and appear to overwrite the previous custom server.
56+
/// Reject that up front with a clear error instead of silently dropping one.
57+
fn ensure_unique_server_ids(definitions: &[ServerDefinition]) -> Result<()> {
58+
let mut seen_ids: HashMap<String, String> = HashMap::new();
59+
for definition in definitions {
60+
if let Some(first_name) =
61+
seen_ids.insert(definition.id.clone(), definition.name.clone())
62+
{
63+
anyhow::bail!(
64+
"Multiple custom servers normalize to the same id '{}': '{}' and '{}'. Rename one mcpServers key to a distinct alphanumeric/hyphen/dot id.",
65+
definition.id,
66+
first_name,
67+
definition.name
68+
);
69+
}
70+
}
71+
Ok(())
72+
}
73+
5174
/// Sync servers from a user space JSON file into InstalledServer records
5275
///
5376
/// This performs a 3-way diff:
@@ -75,23 +98,10 @@ impl UserSpaceSyncService {
7598
// 2. Convert to ServerDefinitions
7699
let definitions = config.to_server_definitions(space_id, file_path.to_path_buf());
77100

78-
// User-config keys are normalized into MCP-safe server IDs. Do not allow
79-
// two entries to collapse to the same ID; that would make the sync loop
80-
// update the same InstalledServer row and appear to overwrite the previous
81-
// custom server.
82-
let mut seen_ids: HashMap<String, String> = HashMap::new();
83-
for definition in &definitions {
84-
if let Some(first_name) =
85-
seen_ids.insert(definition.id.clone(), definition.name.clone())
86-
{
87-
anyhow::bail!(
88-
"Multiple custom servers normalize to the same id '{}': '{}' and '{}'. Rename one mcpServers key to a distinct alphanumeric/hyphen/dot id.",
89-
definition.id,
90-
first_name,
91-
definition.name
92-
);
93-
}
94-
}
101+
// User-config keys are normalized into MCP-safe server IDs; reject two
102+
// entries that collapse to the same ID up front so the sync loop can't
103+
// silently overwrite one custom server with another.
104+
Self::ensure_unique_server_ids(&definitions)?;
95105

96106
let file_server_ids: HashSet<String> = definitions.iter().map(|d| d.id.clone()).collect();
97107

@@ -252,4 +262,39 @@ mod tests {
252262

253263
assert_eq!(result.total_changes(), 3);
254264
}
265+
266+
fn definitions_from(json: &str) -> Vec<ServerDefinition> {
267+
let config: UserSpaceConfig = serde_json::from_str(json).expect("valid config json");
268+
config.to_server_definitions("space-1", std::path::PathBuf::from("test.json"))
269+
}
270+
271+
#[test]
272+
fn ensure_unique_server_ids_rejects_colliding_normalized_ids() {
273+
// "My Server" and "my_server" both normalize to "myserver".
274+
let definitions = definitions_from(
275+
r#"{ "mcpServers": {
276+
"My Server": { "command": "echo" },
277+
"my_server": { "command": "echo" }
278+
} }"#,
279+
);
280+
281+
let err = UserSpaceSyncService::ensure_unique_server_ids(&definitions)
282+
.expect_err("colliding normalized ids must be rejected");
283+
assert!(
284+
err.to_string().contains("myserver"),
285+
"error should name the colliding id, got: {err}"
286+
);
287+
}
288+
289+
#[test]
290+
fn ensure_unique_server_ids_accepts_distinct_ids() {
291+
let definitions = definitions_from(
292+
r#"{ "mcpServers": {
293+
"alpha": { "command": "echo" },
294+
"beta": { "command": "echo" }
295+
} }"#,
296+
);
297+
298+
assert!(UserSpaceSyncService::ensure_unique_server_ids(&definitions).is_ok());
299+
}
255300
}

0 commit comments

Comments
 (0)