Skip to content

Commit 6b0cdf3

Browse files
Phase B: MCP tools tri-state Option<Vec<String>>
McpStdioServerConfig.tools and McpHttpServerConfig.tools change from Vec<String> to Option<Vec<String>> with #[serde(default, skip_serializing_if = Option::is_none)]. Semantics now match the runtime contract and the other SDKs: - None (field omitted on wire) -> expose ALL tools - Some(vec![]) -> expose NO tools - Some(non-empty) -> explicit allowlist Previously an empty Vec was treated as 'no tools' on the wire but the runtime treats omission as 'all tools', so there was no way to opt back into 'all' once the field was set. Adds four tri-state serde tests (serialize + deserialize for each of stdio and http). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent b6ab0b8 commit 6b0cdf3

4 files changed

Lines changed: 124 additions & 13 deletions

File tree

rust/src/types.rs

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -736,7 +736,7 @@ impl CloudSessionOptions {
736736
/// servers.insert(
737737
/// "playwright".to_string(),
738738
/// McpServerConfig::Stdio(McpStdioServerConfig {
739-
/// tools: vec!["*".to_string()],
739+
/// tools: Some(vec!["*".to_string()]),
740740
/// command: "npx".to_string(),
741741
/// args: vec!["-y".to_string(), "@playwright/mcp".to_string()],
742742
/// ..Default::default()
@@ -745,7 +745,7 @@ impl CloudSessionOptions {
745745
/// servers.insert(
746746
/// "weather".to_string(),
747747
/// McpServerConfig::Http(McpHttpServerConfig {
748-
/// tools: vec!["forecast".to_string()],
748+
/// tools: Some(vec!["forecast".to_string()]),
749749
/// url: "https://example.com/mcp".to_string(),
750750
/// ..Default::default()
751751
/// }),
@@ -772,9 +772,13 @@ pub enum McpServerConfig {
772772
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
773773
#[serde(rename_all = "camelCase")]
774774
pub struct McpStdioServerConfig {
775-
/// Tools to expose from this server. `["*"]` exposes all; `[]` exposes none.
776-
#[serde(default)]
777-
pub tools: Vec<String>,
775+
/// Tools to expose from this server.
776+
///
777+
/// - `None` (field omitted on the wire) — expose **all** tools.
778+
/// - `Some(vec![])` — expose **no** tools.
779+
/// - `Some(vec!["a", ...])` — expose only the listed tools.
780+
#[serde(default, skip_serializing_if = "Option::is_none")]
781+
pub tools: Option<Vec<String>>,
778782
/// Optional timeout in milliseconds for tool calls to this server.
779783
#[serde(default, skip_serializing_if = "Option::is_none")]
780784
pub timeout: Option<i64>,
@@ -798,9 +802,13 @@ pub struct McpStdioServerConfig {
798802
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
799803
#[serde(rename_all = "camelCase")]
800804
pub struct McpHttpServerConfig {
801-
/// Tools to expose from this server. `["*"]` exposes all; `[]` exposes none.
802-
#[serde(default)]
803-
pub tools: Vec<String>,
805+
/// Tools to expose from this server.
806+
///
807+
/// - `None` (field omitted on the wire) — expose **all** tools.
808+
/// - `Some(vec![])` — expose **no** tools.
809+
/// - `Some(vec!["a", ...])` — expose only the listed tools.
810+
#[serde(default, skip_serializing_if = "Option::is_none")]
811+
pub tools: Option<Vec<String>>,
804812
/// Optional timeout in milliseconds for tool calls to this server.
805813
#[serde(default, skip_serializing_if = "Option::is_none")]
806814
pub timeout: Option<i64>,

rust/tests/e2e/mcp_and_agents.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ async fn accept_mcp_server_config_without_args() {
5151
let mcp_servers = HashMap::from([(
5252
"test-server".to_string(),
5353
McpServerConfig::Stdio(McpStdioServerConfig {
54-
tools: vec!["*".to_string()],
54+
tools: Some(vec!["*".to_string()]),
5555
command: "echo".to_string(),
5656
..McpStdioServerConfig::default()
5757
}),
@@ -389,7 +389,7 @@ fn multiple_mcp_servers() -> HashMap<String, McpServerConfig> {
389389
servers.insert(
390390
"server2".to_string(),
391391
McpServerConfig::Stdio(McpStdioServerConfig {
392-
tools: vec!["*".to_string()],
392+
tools: Some(vec!["*".to_string()]),
393393
command: echo_command(),
394394
args: echo_args("server2"),
395395
..McpStdioServerConfig::default()
@@ -402,7 +402,7 @@ fn test_mcp_servers(message: &str) -> HashMap<String, McpServerConfig> {
402402
HashMap::from([(
403403
"test-server".to_string(),
404404
McpServerConfig::Stdio(McpStdioServerConfig {
405-
tools: vec!["*".to_string()],
405+
tools: Some(vec!["*".to_string()]),
406406
command: echo_command(),
407407
args: echo_args(message),
408408
..McpStdioServerConfig::default()

rust/tests/e2e/rpc_mcp_and_skills.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,7 @@ fn test_mcp_servers(message: &str) -> HashMap<String, McpServerConfig> {
438438
HashMap::from([(
439439
message.to_string(),
440440
McpServerConfig::Stdio(McpStdioServerConfig {
441-
tools: vec!["*".to_string()],
441+
tools: Some(vec!["*".to_string()]),
442442
command: echo_command(),
443443
args: echo_args(message),
444444
..McpStdioServerConfig::default()

rust/tests/session_test.rs

Lines changed: 104 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -544,7 +544,7 @@ fn mcp_server_config_roundtrips_through_tagged_enum() {
544544
args: vec!["server.js".to_string()],
545545
env: HashMap::new(),
546546
cwd: None,
547-
tools: vec!["*".to_string()],
547+
tools: Some(vec!["*".to_string()]),
548548
timeout: None,
549549
});
550550
let json = serde_json::to_value(&stdio).unwrap();
@@ -566,6 +566,109 @@ fn mcp_server_config_roundtrips_through_tagged_enum() {
566566
assert_eq!(cfg_json["github"]["type"], "stdio");
567567
}
568568

569+
#[test]
570+
fn mcp_stdio_tools_tri_state_serializes_correctly() {
571+
use github_copilot_sdk::McpStdioServerConfig;
572+
573+
// None → field omitted (= "expose all tools")
574+
let cfg = McpStdioServerConfig {
575+
command: "echo".into(),
576+
tools: None,
577+
..Default::default()
578+
};
579+
let json = serde_json::to_value(&cfg).unwrap();
580+
assert!(
581+
json.get("tools").is_none(),
582+
"tools=None must be omitted on the wire; got {json}"
583+
);
584+
585+
// Some(empty) → field present as []
586+
let cfg = McpStdioServerConfig {
587+
command: "echo".into(),
588+
tools: Some(vec![]),
589+
..Default::default()
590+
};
591+
let json = serde_json::to_value(&cfg).unwrap();
592+
assert_eq!(json["tools"], serde_json::json!([]));
593+
594+
// Some(non-empty) → field present as the explicit list
595+
let cfg = McpStdioServerConfig {
596+
command: "echo".into(),
597+
tools: Some(vec!["a".into(), "b".into()]),
598+
..Default::default()
599+
};
600+
let json = serde_json::to_value(&cfg).unwrap();
601+
assert_eq!(json["tools"], serde_json::json!(["a", "b"]));
602+
}
603+
604+
#[test]
605+
fn mcp_stdio_tools_tri_state_deserializes_correctly() {
606+
use github_copilot_sdk::McpStdioServerConfig;
607+
608+
// Missing field → None
609+
let cfg: McpStdioServerConfig =
610+
serde_json::from_value(serde_json::json!({ "command": "echo" })).unwrap();
611+
assert_eq!(cfg.tools, None);
612+
613+
// Empty list → Some(empty)
614+
let cfg: McpStdioServerConfig =
615+
serde_json::from_value(serde_json::json!({ "command": "echo", "tools": [] })).unwrap();
616+
assert_eq!(cfg.tools, Some(vec![]));
617+
618+
// Non-empty list → Some(list)
619+
let cfg: McpStdioServerConfig =
620+
serde_json::from_value(serde_json::json!({ "command": "echo", "tools": ["x"] })).unwrap();
621+
assert_eq!(cfg.tools, Some(vec!["x".to_string()]));
622+
}
623+
624+
#[test]
625+
fn mcp_http_tools_tri_state_serializes_correctly() {
626+
use github_copilot_sdk::McpHttpServerConfig;
627+
628+
let cfg = McpHttpServerConfig {
629+
url: "https://example.com".into(),
630+
tools: None,
631+
..Default::default()
632+
};
633+
assert!(
634+
serde_json::to_value(&cfg).unwrap().get("tools").is_none(),
635+
"tools=None must be omitted on the wire"
636+
);
637+
638+
let cfg = McpHttpServerConfig {
639+
url: "https://example.com".into(),
640+
tools: Some(vec![]),
641+
..Default::default()
642+
};
643+
assert_eq!(
644+
serde_json::to_value(&cfg).unwrap()["tools"],
645+
serde_json::json!([])
646+
);
647+
648+
let cfg = McpHttpServerConfig {
649+
url: "https://example.com".into(),
650+
tools: Some(vec!["a".into()]),
651+
..Default::default()
652+
};
653+
assert_eq!(
654+
serde_json::to_value(&cfg).unwrap()["tools"],
655+
serde_json::json!(["a"])
656+
);
657+
}
658+
659+
#[test]
660+
fn mcp_http_tools_tri_state_deserializes_correctly() {
661+
use github_copilot_sdk::McpHttpServerConfig;
662+
663+
let cfg: McpHttpServerConfig =
664+
serde_json::from_value(serde_json::json!({ "url": "https://e.com" })).unwrap();
665+
assert_eq!(cfg.tools, None);
666+
667+
let cfg: McpHttpServerConfig =
668+
serde_json::from_value(serde_json::json!({ "url": "https://e.com", "tools": [] })).unwrap();
669+
assert_eq!(cfg.tools, Some(vec![]));
670+
}
671+
569672
#[test]
570673
fn permission_request_data_extracts_typed_kind() {
571674
use github_copilot_sdk::{PermissionRequestData, PermissionRequestKind};

0 commit comments

Comments
 (0)