diff --git a/rust/src/types.rs b/rust/src/types.rs index 0d4598cff..d47d9f841 100644 --- a/rust/src/types.rs +++ b/rust/src/types.rs @@ -720,11 +720,8 @@ pub struct McpStdioServerConfig { /// Arguments to pass to the subprocess. #[serde(default)] pub args: Vec, - /// Environment variables to set on the subprocess. - /// - /// Interpretation depends on the parent session's - /// `env_value_mode`: `"direct"` (default) treats values as literals; - /// `"indirect"` treats them as env-var names to look up at start time. + /// Environment variables to set on the subprocess. Values are passed + /// through literally to the child process. #[serde(default, skip_serializing_if = "HashMap::is_empty")] pub env: HashMap, /// Working directory for the subprocess. @@ -897,6 +894,14 @@ pub struct AzureProviderOptions { pub api_version: Option, } +/// Wire default for [`SessionConfig::env_value_mode`] / +/// [`ResumeSessionConfig::env_value_mode`]. The runtime understands +/// `"direct"` (literal values) or `"indirect"` (env-var lookup); the SDK +/// only ever sends `"direct"`. +fn default_env_value_mode() -> String { + "direct".into() +} + /// Configuration for creating a new session via the `session.create` RPC. /// /// All fields are optional — the CLI applies sensible defaults. @@ -977,10 +982,11 @@ pub struct SessionConfig { /// MCP server configurations passed through to the CLI. #[serde(skip_serializing_if = "Option::is_none")] pub mcp_servers: Option>, - /// How the CLI interprets env values in MCP server configs. - /// `"direct"` = literal values; `"indirect"` = env var names to look up. - #[serde(skip_serializing_if = "Option::is_none")] - pub env_value_mode: Option, + /// Wire-format hint for MCP `env` map values. The runtime understands + /// `"direct"` (literal values) and `"indirect"` (env-var lookup); the + /// SDK only ever sends `"direct"` and consumers don't have a knob. + #[serde(default = "default_env_value_mode", skip_deserializing)] + pub(crate) env_value_mode: String, /// When true, the CLI runs config discovery (MCP config files, skills, plugins). #[serde(skip_serializing_if = "Option::is_none")] pub enable_config_discovery: Option, @@ -1103,7 +1109,6 @@ impl std::fmt::Debug for SessionConfig { .field("available_tools", &self.available_tools) .field("excluded_tools", &self.excluded_tools) .field("mcp_servers", &self.mcp_servers) - .field("env_value_mode", &self.env_value_mode) .field("enable_config_discovery", &self.enable_config_discovery) .field("request_user_input", &self.request_user_input) .field("request_permission", &self.request_permission) @@ -1162,7 +1167,7 @@ impl Default for SessionConfig { available_tools: None, excluded_tools: None, mcp_servers: None, - env_value_mode: None, + env_value_mode: default_env_value_mode(), enable_config_discovery: None, request_user_input: Some(true), request_permission: Some(true), @@ -1349,13 +1354,6 @@ impl SessionConfig { self } - /// Set how the CLI interprets env values in MCP server configs - /// (`"direct"` literal vs `"indirect"` env var name lookup). - pub fn with_env_value_mode(mut self, mode: impl Into) -> Self { - self.env_value_mode = Some(mode.into()); - self - } - /// Enable or disable CLI config discovery (MCP config files, skills, plugins). pub fn with_enable_config_discovery(mut self, enable: bool) -> Self { self.enable_config_discovery = Some(enable); @@ -1519,9 +1517,9 @@ pub struct ResumeSessionConfig { /// Re-supply MCP servers so they remain available after app restart. #[serde(skip_serializing_if = "Option::is_none")] pub mcp_servers: Option>, - /// How the CLI interprets env values in MCP configs. - #[serde(skip_serializing_if = "Option::is_none")] - pub env_value_mode: Option, + /// See [`SessionConfig::env_value_mode`]. Always `"direct"` on the wire. + #[serde(default = "default_env_value_mode", skip_deserializing)] + pub(crate) env_value_mode: String, /// Enable config discovery on resume. #[serde(skip_serializing_if = "Option::is_none")] pub enable_config_discovery: Option, @@ -1624,7 +1622,6 @@ impl std::fmt::Debug for ResumeSessionConfig { .field("available_tools", &self.available_tools) .field("excluded_tools", &self.excluded_tools) .field("mcp_servers", &self.mcp_servers) - .field("env_value_mode", &self.env_value_mode) .field("enable_config_discovery", &self.enable_config_discovery) .field("request_user_input", &self.request_user_input) .field("request_permission", &self.request_permission) @@ -1681,7 +1678,7 @@ impl ResumeSessionConfig { available_tools: None, excluded_tools: None, mcp_servers: None, - env_value_mode: None, + env_value_mode: default_env_value_mode(), enable_config_discovery: None, request_user_input: Some(true), request_permission: Some(true), @@ -1833,13 +1830,6 @@ impl ResumeSessionConfig { self } - /// Set how the CLI interprets env values in MCP configs (`"direct"` / - /// `"indirect"`). - pub fn with_env_value_mode(mut self, mode: impl Into) -> Self { - self.env_value_mode = Some(mode.into()); - self - } - /// Enable or disable CLI config discovery on resume. pub fn with_enable_config_discovery(mut self, enable: bool) -> Self { self.enable_config_discovery = Some(enable); @@ -3075,7 +3065,6 @@ mod tests { .with_available_tools(["bash", "view"]) .with_excluded_tools(["dangerous"]) .with_mcp_servers(HashMap::new()) - .with_env_value_mode("direct") .with_enable_config_discovery(true) .with_request_user_input(false) .with_skill_directories([PathBuf::from("/tmp/skills")]) @@ -3101,7 +3090,6 @@ mod tests { Some(&["dangerous".to_string()][..]) ); assert!(cfg.mcp_servers.is_some()); - assert_eq!(cfg.env_value_mode.as_deref(), Some("direct")); assert_eq!(cfg.enable_config_discovery, Some(true)); assert_eq!(cfg.request_user_input, Some(false)); // overrode default assert_eq!(cfg.request_permission, Some(true)); // default preserved @@ -3131,7 +3119,6 @@ mod tests { .with_available_tools(["bash", "view"]) .with_excluded_tools(["dangerous"]) .with_mcp_servers(HashMap::new()) - .with_env_value_mode("indirect") .with_enable_config_discovery(true) .with_request_user_input(false) .with_skill_directories([PathBuf::from("/tmp/skills")]) @@ -3157,7 +3144,6 @@ mod tests { Some(&["dangerous".to_string()][..]) ); assert!(cfg.mcp_servers.is_some()); - assert_eq!(cfg.env_value_mode.as_deref(), Some("indirect")); assert_eq!(cfg.enable_config_discovery, Some(true)); assert_eq!(cfg.request_user_input, Some(false)); // overrode default assert_eq!(cfg.request_permission, Some(true)); // default preserved diff --git a/rust/tests/session_test.rs b/rust/tests/session_test.rs index 32a4e0fc4..1f9873879 100644 --- a/rust/tests/session_test.rs +++ b/rust/tests/session_test.rs @@ -1957,6 +1957,66 @@ async fn request_elicitation_sent_in_create_params() { timeout(TIMEOUT, create_handle).await.unwrap().unwrap(); } +#[tokio::test] +async fn env_value_mode_hardcoded_direct_on_create_and_resume() { + use github_copilot_sdk::types::ResumeSessionConfig; + + let (client, mut server_read, mut server_write) = make_client(); + + let create_handle = tokio::spawn({ + let client = client.clone(); + async move { + client + .create_session(SessionConfig::default().with_handler(Arc::new(NoopHandler))) + .await + .unwrap() + } + }); + + let request = read_framed(&mut server_read).await; + assert_eq!(request["method"], "session.create"); + assert_eq!(request["params"]["envValueMode"], "direct"); + + let id = request["id"].as_u64().unwrap(); + let response = serde_json::json!({ + "jsonrpc": "2.0", + "id": id, + "result": { "sessionId": "s-env-create" }, + }); + write_framed(&mut server_write, &serde_json::to_vec(&response).unwrap()).await; + timeout(TIMEOUT, create_handle).await.unwrap().unwrap(); + + let resume_handle = tokio::spawn({ + let client = client.clone(); + async move { + let cfg = ResumeSessionConfig::new(SessionId::from("s-env-create")) + .with_handler(Arc::new(NoopHandler)); + client.resume_session(cfg).await.unwrap() + } + }); + + let request = read_framed(&mut server_read).await; + assert_eq!(request["method"], "session.resume"); + assert_eq!(request["params"]["envValueMode"], "direct"); + + let id = request["id"].as_u64().unwrap(); + let response = serde_json::json!({ + "jsonrpc": "2.0", + "id": id, + "result": { "sessionId": "s-env-create" }, + }); + write_framed(&mut server_write, &serde_json::to_vec(&response).unwrap()).await; + + // resume_session also fires `session.skills.reload`; respond so resume can return. + let reload = read_framed(&mut server_read).await; + assert_eq!(reload["method"], "session.skills.reload"); + let id = reload["id"].as_u64().unwrap(); + let response = serde_json::json!({ "jsonrpc": "2.0", "id": id, "result": {} }); + write_framed(&mut server_write, &serde_json::to_vec(&response).unwrap()).await; + + timeout(TIMEOUT, resume_handle).await.unwrap().unwrap(); +} + #[tokio::test] async fn elicitation_methods_fail_without_capability() { let (session, _server) = create_session_pair(Arc::new(NoopHandler)).await;