Skip to content

Commit e284b23

Browse files
committed
fix: always send custom fields on save and add corner case tests
Fix a bug where removing all env vars/args/headers in the config modal would send undefined to the backend, which treats None as "keep existing" — so the fields were never actually cleared. Now always sends the values (even if empty) so clearing works correctly. Add corner case tests: - Rust unit: empty keys, key overwrite, special characters (unicode, quotes, equals in keys, newlines), clearing fields to empty, large args lists (100 items) - Rust integration: clearing custom fields via DB update, special characters round-trip through SQLite https://claude.ai/code/session_01V5tgbLyeWrPW5zZ1toRoPZ
1 parent c5ae9c5 commit e284b23

3 files changed

Lines changed: 228 additions & 4 deletions

File tree

apps/desktop/src/features/servers/ServersPage.tsx

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -587,14 +587,17 @@ export function ServersPage() {
587587
try {
588588
const { saveServerInputs } = await import('@/lib/api/registry');
589589

590-
// Save input values with env overrides, args, and headers
590+
// Save input values with env overrides, args, and headers.
591+
// Always send the values (even if empty) so that clearing them works.
592+
// Backend treats None as "keep existing", so we must send Some({}/[])
593+
// to actually clear fields the user removed.
591594
await saveServerInputs(
592595
serverId,
593596
configModal.inputValues,
594597
viewSpace?.id ?? '',
595-
Object.keys(configModal.envOverrides).length > 0 ? configModal.envOverrides : undefined,
596-
configModal.argsAppend.length > 0 ? configModal.argsAppend : undefined,
597-
Object.keys(configModal.extraHeaders).length > 0 ? configModal.extraHeaders : undefined,
598+
configModal.envOverrides,
599+
configModal.argsAppend,
600+
configModal.extraHeaders,
598601
);
599602

600603
setConfigModal({ open: false, server: null, inputValues: {}, envOverrides: {}, argsAppend: [], extraHeaders: {} });

crates/mcpmux-core/src/domain/installed_server.rs

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,4 +338,116 @@ mod tests {
338338
assert!(server.args_append.is_empty());
339339
assert!(server.extra_headers.is_empty());
340340
}
341+
342+
#[test]
343+
fn test_env_overrides_empty_key_allowed() {
344+
let mut server = InstalledServer::new("space_default", "test-server");
345+
server
346+
.env_overrides
347+
.insert("".to_string(), "value".to_string());
348+
349+
assert_eq!(server.env_overrides.get(""), Some(&"value".to_string()));
350+
351+
// Serialize and deserialize
352+
let json = serde_json::to_string(&server).expect("serialize");
353+
let deserialized: InstalledServer = serde_json::from_str(&json).expect("deserialize");
354+
assert_eq!(deserialized.env_overrides.get(""), Some(&"value".to_string()));
355+
}
356+
357+
#[test]
358+
fn test_env_overrides_overwrite_existing_key() {
359+
let mut server = InstalledServer::new("space_default", "test-server");
360+
server
361+
.env_overrides
362+
.insert("KEY".to_string(), "first".to_string());
363+
server
364+
.env_overrides
365+
.insert("KEY".to_string(), "second".to_string());
366+
367+
assert_eq!(server.env_overrides.len(), 1);
368+
assert_eq!(
369+
server.env_overrides.get("KEY"),
370+
Some(&"second".to_string())
371+
);
372+
}
373+
374+
#[test]
375+
fn test_special_characters_in_values() {
376+
let mut server = InstalledServer::new("space_default", "test-server");
377+
// Env var with special chars
378+
server.env_overrides.insert(
379+
"PATH_WITH=EQUALS".to_string(),
380+
"value with spaces & \"quotes\" and \nnewlines".to_string(),
381+
);
382+
// Args with special chars
383+
server.args_append = vec![
384+
"--config=/path/to/file".to_string(),
385+
"arg with spaces".to_string(),
386+
"unicode: 日本語".to_string(),
387+
];
388+
// Header with special chars
389+
server.extra_headers.insert(
390+
"X-Special".to_string(),
391+
"value/with:colons and;semicolons".to_string(),
392+
);
393+
394+
let json = serde_json::to_string(&server).expect("serialize");
395+
let deserialized: InstalledServer = serde_json::from_str(&json).expect("deserialize");
396+
397+
assert_eq!(
398+
deserialized.env_overrides.get("PATH_WITH=EQUALS"),
399+
Some(&"value with spaces & \"quotes\" and \nnewlines".to_string())
400+
);
401+
assert_eq!(deserialized.args_append[2], "unicode: 日本語");
402+
assert_eq!(
403+
deserialized.extra_headers.get("X-Special"),
404+
Some(&"value/with:colons and;semicolons".to_string())
405+
);
406+
}
407+
408+
#[test]
409+
fn test_clear_custom_fields_to_empty() {
410+
let mut server = InstalledServer::new("space_default", "test-server");
411+
// Set values
412+
server
413+
.env_overrides
414+
.insert("KEY".to_string(), "value".to_string());
415+
server.args_append = vec!["--flag".to_string()];
416+
server
417+
.extra_headers
418+
.insert("X-Test".to_string(), "test".to_string());
419+
420+
assert!(!server.env_overrides.is_empty());
421+
assert!(!server.args_append.is_empty());
422+
assert!(!server.extra_headers.is_empty());
423+
424+
// Clear all fields
425+
server.env_overrides = HashMap::new();
426+
server.args_append = Vec::new();
427+
server.extra_headers = HashMap::new();
428+
429+
assert!(server.env_overrides.is_empty());
430+
assert!(server.args_append.is_empty());
431+
assert!(server.extra_headers.is_empty());
432+
433+
// Verify empty fields serialize/deserialize correctly
434+
let json = serde_json::to_string(&server).expect("serialize");
435+
let deserialized: InstalledServer = serde_json::from_str(&json).expect("deserialize");
436+
assert!(deserialized.env_overrides.is_empty());
437+
assert!(deserialized.args_append.is_empty());
438+
assert!(deserialized.extra_headers.is_empty());
439+
}
440+
441+
#[test]
442+
fn test_large_args_list() {
443+
let mut server = InstalledServer::new("space_default", "test-server");
444+
server.args_append = (0..100).map(|i| format!("--arg-{}", i)).collect();
445+
446+
assert_eq!(server.args_append.len(), 100);
447+
448+
let json = serde_json::to_string(&server).expect("serialize");
449+
let deserialized: InstalledServer = serde_json::from_str(&json).expect("deserialize");
450+
assert_eq!(deserialized.args_append.len(), 100);
451+
assert_eq!(deserialized.args_append[99], "--arg-99");
452+
}
341453
}

tests/rust/tests/database/installed_server.rs

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -605,3 +605,112 @@ async fn test_installed_server_empty_custom_fields_by_default() {
605605
assert!(loaded.args_append.is_empty());
606606
assert!(loaded.extra_headers.is_empty());
607607
}
608+
609+
#[tokio::test]
610+
async fn test_installed_server_clear_custom_fields_via_update() {
611+
let test_db = TestDatabase::new();
612+
let db = Arc::new(Mutex::new(test_db.db));
613+
let server_repo = SqliteInstalledServerRepository::new(Arc::clone(&db));
614+
let space_repo = SqliteSpaceRepository::new(db);
615+
616+
let space = fixtures::test_space("Test Space");
617+
SpaceRepository::create(&space_repo, &space).await.unwrap();
618+
619+
// Install server WITH custom fields
620+
let mut server = fixtures::test_installed_server(&space.id.to_string(), "clearable-server");
621+
server
622+
.env_overrides
623+
.insert("KEY".to_string(), "value".to_string());
624+
server.args_append = vec!["--flag".to_string()];
625+
server
626+
.extra_headers
627+
.insert("X-Test".to_string(), "test".to_string());
628+
629+
let server_id = server.id;
630+
InstalledServerRepository::install(&server_repo, &server)
631+
.await
632+
.unwrap();
633+
634+
// Verify they're set
635+
let loaded = InstalledServerRepository::get(&server_repo, &server_id)
636+
.await
637+
.unwrap()
638+
.unwrap();
639+
assert_eq!(loaded.env_overrides.len(), 1);
640+
assert_eq!(loaded.args_append.len(), 1);
641+
assert_eq!(loaded.extra_headers.len(), 1);
642+
643+
// Clear all fields by updating with empty collections
644+
let mut to_update = loaded;
645+
to_update.env_overrides = HashMap::new();
646+
to_update.args_append = Vec::new();
647+
to_update.extra_headers = HashMap::new();
648+
649+
InstalledServerRepository::update(&server_repo, &to_update)
650+
.await
651+
.expect("Failed to update server");
652+
653+
// Verify they're cleared
654+
let cleared = InstalledServerRepository::get(&server_repo, &server_id)
655+
.await
656+
.unwrap()
657+
.unwrap();
658+
assert!(
659+
cleared.env_overrides.is_empty(),
660+
"env_overrides should be empty after clearing"
661+
);
662+
assert!(
663+
cleared.args_append.is_empty(),
664+
"args_append should be empty after clearing"
665+
);
666+
assert!(
667+
cleared.extra_headers.is_empty(),
668+
"extra_headers should be empty after clearing"
669+
);
670+
}
671+
672+
#[tokio::test]
673+
async fn test_installed_server_special_characters_persist() {
674+
let test_db = TestDatabase::new();
675+
let db = Arc::new(Mutex::new(test_db.db));
676+
let server_repo = SqliteInstalledServerRepository::new(Arc::clone(&db));
677+
let space_repo = SqliteSpaceRepository::new(db);
678+
679+
let space = fixtures::test_space("Test Space");
680+
SpaceRepository::create(&space_repo, &space).await.unwrap();
681+
682+
let mut server = fixtures::test_installed_server(&space.id.to_string(), "special-server");
683+
// Values with special characters
684+
server.env_overrides.insert(
685+
"PATH_WITH=EQUALS".to_string(),
686+
"value with \"quotes\" and 'apostrophes'".to_string(),
687+
);
688+
server.args_append = vec![
689+
"--config=/path/to/file with spaces".to_string(),
690+
"unicode: 日本語".to_string(),
691+
];
692+
server
693+
.extra_headers
694+
.insert("Authorization".to_string(), "Bearer tok3n+/=".to_string());
695+
696+
let server_id = server.id;
697+
InstalledServerRepository::install(&server_repo, &server)
698+
.await
699+
.unwrap();
700+
701+
let loaded = InstalledServerRepository::get(&server_repo, &server_id)
702+
.await
703+
.unwrap()
704+
.unwrap();
705+
706+
assert_eq!(
707+
loaded.env_overrides.get("PATH_WITH=EQUALS"),
708+
Some(&"value with \"quotes\" and 'apostrophes'".to_string())
709+
);
710+
assert_eq!(loaded.args_append[0], "--config=/path/to/file with spaces");
711+
assert_eq!(loaded.args_append[1], "unicode: 日本語");
712+
assert_eq!(
713+
loaded.extra_headers.get("Authorization"),
714+
Some(&"Bearer tok3n+/=".to_string())
715+
);
716+
}

0 commit comments

Comments
 (0)