Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 14 additions & 15 deletions agent-client-protocol-schema/src/v2/agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -930,8 +930,8 @@ pub struct AuthMethodTerminal {
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub args: Vec<String>,
/// Additional environment variables to set when running the agent binary for terminal auth.
#[serde(default, skip_serializing_if = "HashMap::is_empty")]
pub env: HashMap<String, String>,
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub env: Vec<EnvVariable>,
/// 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.
Expand All @@ -951,7 +951,7 @@ impl AuthMethodTerminal {
name: name.into(),
description: None,
args: Vec::new(),
env: HashMap::new(),
env: Vec::new(),
meta: None,
}
}
Expand All @@ -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<String, String>) -> Self {
pub fn env(mut self, env: Vec<EnvVariable>) -> Self {
self.env = env;
self
}
Expand Down Expand Up @@ -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();
Expand All @@ -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"
}
]
})
);

Expand All @@ -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"),
}
Expand Down
78 changes: 76 additions & 2 deletions agent-client-protocol-schema/src/v2/conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()?,
})
}
Expand All @@ -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::<Result<Vec<_>>>()?;
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()?,
})
}
Expand Down Expand Up @@ -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 =
Expand Down
4 changes: 2 additions & 2 deletions docs/protocol/v2/draft/schema.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -2440,7 +2440,7 @@ See protocol docs: [Extensibility](https://agentclientprotocol.com/protocol/v2/d
<ResponseField name="description" type={"string | null"} >
Optional description providing more details about this authentication method.
</ResponseField>
<ResponseField name="env" type={"object"} >
<ResponseField name="env" type={<a href="#envvariable">EnvVariable[]</a>} >
Additional environment variables to set when running the agent binary for terminal auth.
</ResponseField>
<ResponseField name="id" type={<a href="#authmethodid">AuthMethodId</a>} required>
Expand Down Expand Up @@ -2629,7 +2629,7 @@ See protocol docs: [Extensibility](https://agentclientprotocol.com/protocol/v2/d
<ResponseField name="description" type={"string | null"} >
Optional description providing more details about this authentication method.
</ResponseField>
<ResponseField name="env" type={"object"} >
<ResponseField name="env" type={<a href="#envvariable">EnvVariable[]</a>} >
Additional environment variables to set when running the agent binary for terminal auth.
</ResponseField>
<ResponseField name="id" type={<a href="#authmethodid">AuthMethodId</a>} required>
Expand Down
46 changes: 23 additions & 23 deletions schema/v2/schema.unstable.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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",
Expand Down