Skip to content

Commit 23b1bb8

Browse files
tclemCopilot
andauthored
Remove disabled_mcp_servers + internalize env_value_mode (cross-SDK parity) (#1215)
Two Rust-only items on SessionConfig + ResumeSessionConfig that aren't present in the Node, Python, Go, or .NET SDKs and shouldn't be on the public Rust surface either. disabled_mcp_servers: removed entirely. The field was schema-undocumented (not in api.schema.json's SessionCreateRequest) and silently ignored by the runtime on session.create. Runtime MCP server disablement lives in the typed RPC namespace already (session.rpc().mcp().disable() and session.rpc().mcp().config().disable()). env_value_mode: internalized. Hardcode envValueMode: "direct" on every session.create / session.resume payload, matching Node and Go's wire behaviour. Subprocess MCP server env values pass through to the child literally; consumers don't have a meaningful choice at the SDK boundary. Also fix the README's Session cheat-sheet, which advertised several methods (get_model, get_mode/set_mode, list_workspace_files / read_workspace_file, read_plan / update_plan, start_fleet) as if they existed on Session directly. They don't — replace those snippets with the typed session.rpc().*() namespace they actually use. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent f39d370 commit 23b1bb8

2 files changed

Lines changed: 80 additions & 34 deletions

File tree

rust/src/types.rs

Lines changed: 20 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -720,11 +720,8 @@ pub struct McpStdioServerConfig {
720720
/// Arguments to pass to the subprocess.
721721
#[serde(default)]
722722
pub args: Vec<String>,
723-
/// Environment variables to set on the subprocess.
724-
///
725-
/// Interpretation depends on the parent session's
726-
/// `env_value_mode`: `"direct"` (default) treats values as literals;
727-
/// `"indirect"` treats them as env-var names to look up at start time.
723+
/// Environment variables to set on the subprocess. Values are passed
724+
/// through literally to the child process.
728725
#[serde(default, skip_serializing_if = "HashMap::is_empty")]
729726
pub env: HashMap<String, String>,
730727
/// Working directory for the subprocess.
@@ -897,6 +894,14 @@ pub struct AzureProviderOptions {
897894
pub api_version: Option<String>,
898895
}
899896

897+
/// Wire default for [`SessionConfig::env_value_mode`] /
898+
/// [`ResumeSessionConfig::env_value_mode`]. The runtime understands
899+
/// `"direct"` (literal values) or `"indirect"` (env-var lookup); the SDK
900+
/// only ever sends `"direct"`.
901+
fn default_env_value_mode() -> String {
902+
"direct".into()
903+
}
904+
900905
/// Configuration for creating a new session via the `session.create` RPC.
901906
///
902907
/// All fields are optional — the CLI applies sensible defaults.
@@ -977,10 +982,11 @@ pub struct SessionConfig {
977982
/// MCP server configurations passed through to the CLI.
978983
#[serde(skip_serializing_if = "Option::is_none")]
979984
pub mcp_servers: Option<HashMap<String, McpServerConfig>>,
980-
/// How the CLI interprets env values in MCP server configs.
981-
/// `"direct"` = literal values; `"indirect"` = env var names to look up.
982-
#[serde(skip_serializing_if = "Option::is_none")]
983-
pub env_value_mode: Option<String>,
985+
/// Wire-format hint for MCP `env` map values. The runtime understands
986+
/// `"direct"` (literal values) and `"indirect"` (env-var lookup); the
987+
/// SDK only ever sends `"direct"` and consumers don't have a knob.
988+
#[serde(default = "default_env_value_mode", skip_deserializing)]
989+
pub(crate) env_value_mode: String,
984990
/// When true, the CLI runs config discovery (MCP config files, skills, plugins).
985991
#[serde(skip_serializing_if = "Option::is_none")]
986992
pub enable_config_discovery: Option<bool>,
@@ -1103,7 +1109,6 @@ impl std::fmt::Debug for SessionConfig {
11031109
.field("available_tools", &self.available_tools)
11041110
.field("excluded_tools", &self.excluded_tools)
11051111
.field("mcp_servers", &self.mcp_servers)
1106-
.field("env_value_mode", &self.env_value_mode)
11071112
.field("enable_config_discovery", &self.enable_config_discovery)
11081113
.field("request_user_input", &self.request_user_input)
11091114
.field("request_permission", &self.request_permission)
@@ -1162,7 +1167,7 @@ impl Default for SessionConfig {
11621167
available_tools: None,
11631168
excluded_tools: None,
11641169
mcp_servers: None,
1165-
env_value_mode: None,
1170+
env_value_mode: default_env_value_mode(),
11661171
enable_config_discovery: None,
11671172
request_user_input: Some(true),
11681173
request_permission: Some(true),
@@ -1349,13 +1354,6 @@ impl SessionConfig {
13491354
self
13501355
}
13511356

1352-
/// Set how the CLI interprets env values in MCP server configs
1353-
/// (`"direct"` literal vs `"indirect"` env var name lookup).
1354-
pub fn with_env_value_mode(mut self, mode: impl Into<String>) -> Self {
1355-
self.env_value_mode = Some(mode.into());
1356-
self
1357-
}
1358-
13591357
/// Enable or disable CLI config discovery (MCP config files, skills, plugins).
13601358
pub fn with_enable_config_discovery(mut self, enable: bool) -> Self {
13611359
self.enable_config_discovery = Some(enable);
@@ -1519,9 +1517,9 @@ pub struct ResumeSessionConfig {
15191517
/// Re-supply MCP servers so they remain available after app restart.
15201518
#[serde(skip_serializing_if = "Option::is_none")]
15211519
pub mcp_servers: Option<HashMap<String, McpServerConfig>>,
1522-
/// How the CLI interprets env values in MCP configs.
1523-
#[serde(skip_serializing_if = "Option::is_none")]
1524-
pub env_value_mode: Option<String>,
1520+
/// See [`SessionConfig::env_value_mode`]. Always `"direct"` on the wire.
1521+
#[serde(default = "default_env_value_mode", skip_deserializing)]
1522+
pub(crate) env_value_mode: String,
15251523
/// Enable config discovery on resume.
15261524
#[serde(skip_serializing_if = "Option::is_none")]
15271525
pub enable_config_discovery: Option<bool>,
@@ -1624,7 +1622,6 @@ impl std::fmt::Debug for ResumeSessionConfig {
16241622
.field("available_tools", &self.available_tools)
16251623
.field("excluded_tools", &self.excluded_tools)
16261624
.field("mcp_servers", &self.mcp_servers)
1627-
.field("env_value_mode", &self.env_value_mode)
16281625
.field("enable_config_discovery", &self.enable_config_discovery)
16291626
.field("request_user_input", &self.request_user_input)
16301627
.field("request_permission", &self.request_permission)
@@ -1681,7 +1678,7 @@ impl ResumeSessionConfig {
16811678
available_tools: None,
16821679
excluded_tools: None,
16831680
mcp_servers: None,
1684-
env_value_mode: None,
1681+
env_value_mode: default_env_value_mode(),
16851682
enable_config_discovery: None,
16861683
request_user_input: Some(true),
16871684
request_permission: Some(true),
@@ -1833,13 +1830,6 @@ impl ResumeSessionConfig {
18331830
self
18341831
}
18351832

1836-
/// Set how the CLI interprets env values in MCP configs (`"direct"` /
1837-
/// `"indirect"`).
1838-
pub fn with_env_value_mode(mut self, mode: impl Into<String>) -> Self {
1839-
self.env_value_mode = Some(mode.into());
1840-
self
1841-
}
1842-
18431833
/// Enable or disable CLI config discovery on resume.
18441834
pub fn with_enable_config_discovery(mut self, enable: bool) -> Self {
18451835
self.enable_config_discovery = Some(enable);
@@ -3075,7 +3065,6 @@ mod tests {
30753065
.with_available_tools(["bash", "view"])
30763066
.with_excluded_tools(["dangerous"])
30773067
.with_mcp_servers(HashMap::new())
3078-
.with_env_value_mode("direct")
30793068
.with_enable_config_discovery(true)
30803069
.with_request_user_input(false)
30813070
.with_skill_directories([PathBuf::from("/tmp/skills")])
@@ -3101,7 +3090,6 @@ mod tests {
31013090
Some(&["dangerous".to_string()][..])
31023091
);
31033092
assert!(cfg.mcp_servers.is_some());
3104-
assert_eq!(cfg.env_value_mode.as_deref(), Some("direct"));
31053093
assert_eq!(cfg.enable_config_discovery, Some(true));
31063094
assert_eq!(cfg.request_user_input, Some(false)); // overrode default
31073095
assert_eq!(cfg.request_permission, Some(true)); // default preserved
@@ -3131,7 +3119,6 @@ mod tests {
31313119
.with_available_tools(["bash", "view"])
31323120
.with_excluded_tools(["dangerous"])
31333121
.with_mcp_servers(HashMap::new())
3134-
.with_env_value_mode("indirect")
31353122
.with_enable_config_discovery(true)
31363123
.with_request_user_input(false)
31373124
.with_skill_directories([PathBuf::from("/tmp/skills")])
@@ -3157,7 +3144,6 @@ mod tests {
31573144
Some(&["dangerous".to_string()][..])
31583145
);
31593146
assert!(cfg.mcp_servers.is_some());
3160-
assert_eq!(cfg.env_value_mode.as_deref(), Some("indirect"));
31613147
assert_eq!(cfg.enable_config_discovery, Some(true));
31623148
assert_eq!(cfg.request_user_input, Some(false)); // overrode default
31633149
assert_eq!(cfg.request_permission, Some(true)); // default preserved

rust/tests/session_test.rs

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1957,6 +1957,66 @@ async fn request_elicitation_sent_in_create_params() {
19571957
timeout(TIMEOUT, create_handle).await.unwrap().unwrap();
19581958
}
19591959

1960+
#[tokio::test]
1961+
async fn env_value_mode_hardcoded_direct_on_create_and_resume() {
1962+
use github_copilot_sdk::types::ResumeSessionConfig;
1963+
1964+
let (client, mut server_read, mut server_write) = make_client();
1965+
1966+
let create_handle = tokio::spawn({
1967+
let client = client.clone();
1968+
async move {
1969+
client
1970+
.create_session(SessionConfig::default().with_handler(Arc::new(NoopHandler)))
1971+
.await
1972+
.unwrap()
1973+
}
1974+
});
1975+
1976+
let request = read_framed(&mut server_read).await;
1977+
assert_eq!(request["method"], "session.create");
1978+
assert_eq!(request["params"]["envValueMode"], "direct");
1979+
1980+
let id = request["id"].as_u64().unwrap();
1981+
let response = serde_json::json!({
1982+
"jsonrpc": "2.0",
1983+
"id": id,
1984+
"result": { "sessionId": "s-env-create" },
1985+
});
1986+
write_framed(&mut server_write, &serde_json::to_vec(&response).unwrap()).await;
1987+
timeout(TIMEOUT, create_handle).await.unwrap().unwrap();
1988+
1989+
let resume_handle = tokio::spawn({
1990+
let client = client.clone();
1991+
async move {
1992+
let cfg = ResumeSessionConfig::new(SessionId::from("s-env-create"))
1993+
.with_handler(Arc::new(NoopHandler));
1994+
client.resume_session(cfg).await.unwrap()
1995+
}
1996+
});
1997+
1998+
let request = read_framed(&mut server_read).await;
1999+
assert_eq!(request["method"], "session.resume");
2000+
assert_eq!(request["params"]["envValueMode"], "direct");
2001+
2002+
let id = request["id"].as_u64().unwrap();
2003+
let response = serde_json::json!({
2004+
"jsonrpc": "2.0",
2005+
"id": id,
2006+
"result": { "sessionId": "s-env-create" },
2007+
});
2008+
write_framed(&mut server_write, &serde_json::to_vec(&response).unwrap()).await;
2009+
2010+
// resume_session also fires `session.skills.reload`; respond so resume can return.
2011+
let reload = read_framed(&mut server_read).await;
2012+
assert_eq!(reload["method"], "session.skills.reload");
2013+
let id = reload["id"].as_u64().unwrap();
2014+
let response = serde_json::json!({ "jsonrpc": "2.0", "id": id, "result": {} });
2015+
write_framed(&mut server_write, &serde_json::to_vec(&response).unwrap()).await;
2016+
2017+
timeout(TIMEOUT, resume_handle).await.unwrap().unwrap();
2018+
}
2019+
19602020
#[tokio::test]
19612021
async fn elicitation_methods_fail_without_capability() {
19622022
let (session, _server) = create_session_pair(Arc::new(NoopHandler)).await;

0 commit comments

Comments
 (0)