diff --git a/agent-client-protocol-schema/src/v2/agent.rs b/agent-client-protocol-schema/src/v2/agent.rs index 5eb812895..b6d008aeb 100644 --- a/agent-client-protocol-schema/src/v2/agent.rs +++ b/agent-client-protocol-schema/src/v2/agent.rs @@ -5,7 +5,7 @@ use std::{collections::BTreeMap, path::PathBuf, sync::Arc}; -#[cfg(any(feature = "unstable_auth_methods", feature = "unstable_llm_providers"))] +#[cfg(feature = "unstable_llm_providers")] use std::collections::HashMap; use derive_more::{Display, From}; @@ -930,8 +930,8 @@ pub struct AuthMethodTerminal { #[serde(default, skip_serializing_if = "Vec::is_empty")] pub args: Vec, /// Additional environment variables to set when running the agent binary for terminal auth. - #[serde(default, skip_serializing_if = "HashMap::is_empty")] - pub env: HashMap, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub env: Vec, /// The _meta property is reserved by ACP to allow clients and agents to attach additional /// metadata to their interactions. Implementations MUST NOT make assumptions about values at /// these keys. @@ -951,7 +951,7 @@ impl AuthMethodTerminal { name: name.into(), description: None, args: Vec::new(), - env: HashMap::new(), + env: Vec::new(), meta: None, } } @@ -965,7 +965,7 @@ impl AuthMethodTerminal { /// Additional environment variables to set when running the agent binary for terminal auth. #[must_use] - pub fn env(mut self, env: HashMap) -> Self { + pub fn env(mut self, env: Vec) -> Self { self.env = env; self } @@ -5892,15 +5892,10 @@ mod test_serialization { #[cfg(feature = "unstable_auth_methods")] #[test] fn test_auth_method_terminal_with_args_and_env_serialization() { - use std::collections::HashMap; - - let mut env = HashMap::new(); - env.insert("TERM".to_string(), "xterm-256color".to_string()); - let method = AuthMethod::Terminal( AuthMethodTerminal::new("tui-auth", "Terminal Auth") .args(vec!["--interactive".to_string(), "--color".to_string()]) - .env(env), + .env(vec![EnvVariable::new("TERM", "xterm-256color")]), ); let json = serde_json::to_value(&method).unwrap(); @@ -5911,9 +5906,12 @@ mod test_serialization { "name": "Terminal Auth", "type": "terminal", "args": ["--interactive", "--color"], - "env": { - "TERM": "xterm-256color" - } + "env": [ + { + "name": "TERM", + "value": "xterm-256color" + } + ] }) ); @@ -5922,7 +5920,8 @@ mod test_serialization { AuthMethod::Terminal(AuthMethodTerminal { args, env, .. }) => { assert_eq!(args, vec!["--interactive", "--color"]); assert_eq!(env.len(), 1); - assert_eq!(env.get("TERM").unwrap(), "xterm-256color"); + assert_eq!(env[0].name, "TERM"); + assert_eq!(env[0].value, "xterm-256color"); } _ => panic!("Expected Terminal variant"), } diff --git a/agent-client-protocol-schema/src/v2/conversion.rs b/agent-client-protocol-schema/src/v2/conversion.rs index 0eba7192c..283da4e4f 100644 --- a/agent-client-protocol-schema/src/v2/conversion.rs +++ b/agent-client-protocol-schema/src/v2/conversion.rs @@ -3127,12 +3127,32 @@ impl IntoV1 for super::AuthMethodTerminal { env, meta, } = self; + let env = env + .into_iter() + .map(|env_var| { + let super::EnvVariable { name, value, meta } = env_var; + if meta.is_some() { + return Err(ProtocolConversionError::new( + "v2 AuthMethodTerminal env variable `_meta` cannot be represented in v1", + )); + } + Ok((name.into_v1()?, value.into_v1()?)) + }) + .try_fold(HashMap::new(), |mut env, item| { + let (name, value) = item?; + if env.insert(name.clone(), value).is_some() { + return Err(ProtocolConversionError::new(format!( + "v2 AuthMethodTerminal env variable `{name}` is duplicated and cannot be represented in v1", + ))); + } + Ok(env) + })?; Ok(crate::v1::AuthMethodTerminal { id: id.into_v1()?, name: name.into_v1()?, description: description.into_v1()?, args: args.into_v1()?, - env: env.into_v1()?, + env, meta: meta.into_v1()?, }) } @@ -3151,12 +3171,17 @@ impl IntoV2 for crate::v1::AuthMethodTerminal { env, meta, } = self; + let mut env = env + .into_iter() + .map(|(name, value)| Ok(super::EnvVariable::new(name.into_v2()?, value.into_v2()?))) + .collect::>>()?; + env.sort_by(|left, right| left.name.cmp(&right.name)); Ok(super::AuthMethodTerminal { id: id.into_v2()?, name: name.into_v2()?, description: description.into_v2()?, args: args.into_v2()?, - env: env.into_v2()?, + env, meta: meta.into_v2()?, }) } @@ -9266,6 +9291,55 @@ mod tests { assert!(v1_after.terminal); } + #[cfg(feature = "unstable_auth_methods")] + #[test] + fn auth_method_terminal_env_converts_between_map_and_variable_array() { + let mut env = HashMap::new(); + env.insert("TERM".to_string(), "xterm-256color".to_string()); + env.insert("API_KEY".to_string(), "secret".to_string()); + + let v1_method = v1::AuthMethodTerminal::new("tui-auth", "Terminal Auth").env(env); + let v2_method: v2::AuthMethodTerminal = v1_to_v2(v1_method).expect("v1 -> v2 conversion"); + let v2_json = serde_json::to_value(&v2_method).expect("v2 serialize"); + assert_eq!( + v2_json.pointer("/env"), + Some(&serde_json::json!([ + { + "name": "API_KEY", + "value": "secret" + }, + { + "name": "TERM", + "value": "xterm-256color" + } + ])) + ); + + let v1_after: v1::AuthMethodTerminal = v2_to_v1(v2_method).expect("v2 -> v1 conversion"); + assert_eq!( + v1_after.env.get("TERM").map(String::as_str), + Some("xterm-256color") + ); + assert_eq!( + v1_after.env.get("API_KEY").map(String::as_str), + Some("secret") + ); + } + + #[cfg(feature = "unstable_auth_methods")] + #[test] + fn auth_method_terminal_duplicate_env_names_do_not_convert_to_v1() { + let v2_method = v2::AuthMethodTerminal::new("tui-auth", "Terminal Auth").env(vec![ + v2::EnvVariable::new("TERM", "xterm"), + v2::EnvVariable::new("TERM", "xterm-256color"), + ]); + + assert_v2_to_v1_error( + v2_method, + "v2 AuthMethodTerminal env variable `TERM` is duplicated and cannot be represented in v1", + ); + } + #[test] fn v1_client_fs_and_terminal_capabilities_are_removed_in_v2() { let v1_capabilities = diff --git a/docs/protocol/v2/draft/schema.mdx b/docs/protocol/v2/draft/schema.mdx index 4f99e2b9a..7a6865821 100644 --- a/docs/protocol/v2/draft/schema.mdx +++ b/docs/protocol/v2/draft/schema.mdx @@ -2440,7 +2440,7 @@ See protocol docs: [Extensibility](https://agentclientprotocol.com/protocol/v2/d Optional description providing more details about this authentication method. - +EnvVariable[]} > Additional environment variables to set when running the agent binary for terminal auth. AuthMethodId} required> @@ -2629,7 +2629,7 @@ See protocol docs: [Extensibility](https://agentclientprotocol.com/protocol/v2/d Optional description providing more details about this authentication method. - +EnvVariable[]} > Additional environment variables to set when running the agent binary for terminal auth. AuthMethodId} required> diff --git a/schema/v2/schema.unstable.json b/schema/v2/schema.unstable.json index 75114e09e..2c7e45151 100644 --- a/schema/v2/schema.unstable.json +++ b/schema/v2/schema.unstable.json @@ -3600,6 +3600,26 @@ }, "required": ["id", "name", "vars"] }, + "EnvVariable": { + "description": "An environment variable to set when launching an MCP server.", + "type": "object", + "properties": { + "name": { + "description": "The name of the environment variable.", + "type": "string" + }, + "value": { + "description": "The value to set for the environment variable.", + "type": "string" + }, + "_meta": { + "description": "The _meta property is reserved by ACP to allow clients and agents to attach additional\nmetadata to their interactions. Implementations MUST NOT make assumptions about values at\nthese keys.\n\nSee protocol docs: [Extensibility](https://agentclientprotocol.com/protocol/extensibility)", + "type": ["object", "null"], + "additionalProperties": true + } + }, + "required": ["name", "value"] + }, "AuthMethodTerminal": { "description": "**UNSTABLE**\n\nThis capability is not part of the spec yet, and may be removed or changed at any point.\n\nTerminal-based authentication method.\n\nThe client runs an interactive terminal for the user to authenticate via a TUI.", "type": "object", @@ -3629,9 +3649,9 @@ }, "env": { "description": "Additional environment variables to set when running the agent binary for terminal auth.", - "type": "object", - "additionalProperties": { - "type": "string" + "type": "array", + "items": { + "$ref": "#/$defs/EnvVariable" } }, "_meta": { @@ -7122,26 +7142,6 @@ }, "required": ["name", "id"] }, - "EnvVariable": { - "description": "An environment variable to set when launching an MCP server.", - "type": "object", - "properties": { - "name": { - "description": "The name of the environment variable.", - "type": "string" - }, - "value": { - "description": "The value to set for the environment variable.", - "type": "string" - }, - "_meta": { - "description": "The _meta property is reserved by ACP to allow clients and agents to attach additional\nmetadata to their interactions. Implementations MUST NOT make assumptions about values at\nthese keys.\n\nSee protocol docs: [Extensibility](https://agentclientprotocol.com/protocol/extensibility)", - "type": ["object", "null"], - "additionalProperties": true - } - }, - "required": ["name", "value"] - }, "McpServerStdio": { "description": "Stdio transport configuration for MCP.", "type": "object",