diff --git a/crates/application/src/agent/lifecycle.rs b/crates/application/src/agent/lifecycle.rs index 4b96f89b..2d6715ca 100644 --- a/crates/application/src/agent/lifecycle.rs +++ b/crates/application/src/agent/lifecycle.rs @@ -5581,9 +5581,9 @@ mod tests { #[test] fn codex_plugin_server_projection_removes_stale_managed_tables() { let current = assigned_plugin_server(); - let stale = "idea_plugin__stale__server"; + let stale = "idea_plugin__com_2eidea_2eunity_2ddeveloper_2dtools__unity_2ddeveloper_2dtools_2etools"; let existing = format!( - "user_key = \"keep\"\n[mcp_servers.\"{stale}\"]\ncommand = \"old\"\n\n[preview]\nenabled = true\n" + "user_key = \"keep\"\n[mcp_servers.user_server]\ncommand = \"user\"\n\n[mcp_servers.\"{stale}\"]\ncommand = \"old\"\n\n[preview]\nenabled = true\n" ); let declaration = codex_mcp_declaration( domain::profile::McpTransport::Stdio, @@ -5593,6 +5593,7 @@ mod tests { let rendered = codex_config_toml(Some(&existing), &declaration, "/run", "/project"); assert!(!rendered.contains(stale)); assert!(rendered.contains("user_key = \"keep\"")); + assert!(rendered.contains("[mcp_servers.user_server]\ncommand = \"user\"")); assert!(rendered.contains("[preview]\nenabled = true")); assert_eq!(rendered.matches(¤t.client_name()).count(), 1); } @@ -5600,10 +5601,12 @@ mod tests { #[test] fn internal_restart_refreshes_plugin_servers_without_replacing_real_idea_wiring() { let current = assigned_plugin_server(); + let stale = "idea_plugin__com_2eidea_2eunity_2ddeveloper_2dtools__unity_2ddeveloper_2dtools_2etools"; let existing_json = br#"{ "mcpServers": { "idea": { "command": "/opt/Idea.AppImage", "args": ["mcp-server", "--endpoint", "/live.sock"] }, - "idea_plugin__stale__server": { "command": "old" } + "user-server": { "command": "user" }, + "idea_plugin__com_2eidea_2eunity_2ddeveloper_2dtools__unity_2ddeveloper_2dtools_2etools": { "command": "old" } } }"#; let claude = reconcile_plugin_mcp_json(existing_json, std::slice::from_ref(¤t)) @@ -5613,18 +5616,18 @@ mod tests { claude["mcpServers"]["idea"]["command"], "/opt/Idea.AppImage" ); - assert!(claude["mcpServers"] - .get("idea_plugin__stale__server") - .is_none()); + assert!(claude["mcpServers"].get(stale).is_none()); + assert_eq!(claude["mcpServers"]["user-server"]["command"], "user"); assert_eq!( claude["mcpServers"][current.client_name()]["command"], current.command ); - let existing_toml = "[mcp_servers.idea]\ncommand = \"/opt/Idea.AppImage\"\nargs = [\"mcp-server\", \"--endpoint\", \"/live.sock\"]\n\n[mcp_servers.\"idea_plugin__stale__server\"]\ncommand = \"old\"\n"; - let codex = merge_codex_plugin_mcp_tables(existing_toml, std::slice::from_ref(¤t)); + let existing_toml = format!("[mcp_servers.idea]\ncommand = \"/opt/Idea.AppImage\"\nargs = [\"mcp-server\", \"--endpoint\", \"/live.sock\"]\n\n[mcp_servers.user_server]\ncommand = \"user\"\n\n[mcp_servers.\"{stale}\"]\ncommand = \"old\"\n"); + let codex = merge_codex_plugin_mcp_tables(&existing_toml, std::slice::from_ref(¤t)); assert!(codex.contains("command = \"/opt/Idea.AppImage\"")); - assert!(!codex.contains("idea_plugin__stale__server")); + assert!(codex.contains("[mcp_servers.user_server]\ncommand = \"user\"")); + assert!(!codex.contains(stale)); assert!(codex.contains(¤t.client_name())); } diff --git a/crates/application/src/project_plugin.rs b/crates/application/src/project_plugin.rs index 538334cb..cc738b91 100644 --- a/crates/application/src/project_plugin.rs +++ b/crates/application/src/project_plugin.rs @@ -910,7 +910,7 @@ mod tests { let server = &servers[0]; assert_eq!( server.client_name(), - "idea_plugin__com_2eidea_2eunity_2dtools__unity_2deditor" + "idea_plugin__com_idea_unity_tools__unity___904dacd241fbd4e7" ); assert_eq!(server.command, "/plugins/unity/bin/server"); assert_eq!( diff --git a/crates/domain/src/plugin.rs b/crates/domain/src/plugin.rs index be2d4366..15dfe0fd 100644 --- a/crates/domain/src/plugin.rs +++ b/crates/domain/src/plugin.rs @@ -2,6 +2,7 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; +use sha2::{Digest, Sha256}; use thiserror::Error; /// Plugin domain validation error. @@ -797,32 +798,41 @@ pub struct PluginMcpServerSpec { } impl PluginMcpServerSpec { - /// Stable, collision-free MCP client name for this plugin server. + /// Stable, bounded MCP client name for this plugin server. /// - /// Plugin/server ids may contain punctuation that is awkward in TOML table - /// paths and becomes part of the tool namespace in MCP clients. Encode every - /// non-alphanumeric byte instead of merely replacing it, so distinct ids can - /// never collapse onto the same session server name. + /// Codex prefixes this value with `mcp__` when it builds tool namespaces, whose + /// provider limit is 64 ASCII bytes. Keep this value at most 59 bytes and retain + /// collision resistance after truncating the readable stem by suffixing the + /// first 16 hexadecimal characters of SHA-256(`plugin_id + "\0" + server_id`). #[must_use] pub fn client_name(&self) -> String { - fn component(raw: &str) -> String { - let mut out = String::new(); - for byte in raw.bytes() { - if byte.is_ascii_alphanumeric() { - out.push(char::from(byte).to_ascii_lowercase()); - } else { - out.push('_'); - out.push_str(&format!("{byte:02x}")); - } - } - out - } + const PREFIX: &str = "idea_plugin__"; + const HASH_HEX_LEN: usize = 16; + const MAX_CLIENT_NAME_LEN: usize = 59; + const SEPARATOR: &str = "__"; + const MAX_STEM_LEN: usize = + MAX_CLIENT_NAME_LEN - PREFIX.len() - SEPARATOR.len() - HASH_HEX_LEN; - format!( - "idea_plugin__{}__{}", - component(self.plugin_id.as_str()), - component(self.server_id.as_str()) - ) + let raw_stem = format!("{}__{}", self.plugin_id.as_str(), self.server_id.as_str()); + let stem: String = raw_stem + .bytes() + .map(|byte| { + if byte.is_ascii_alphanumeric() { + char::from(byte).to_ascii_lowercase() + } else { + '_' + } + }) + .take(MAX_STEM_LEN) + .collect(); + + let mut hasher = Sha256::new(); + hasher.update(self.plugin_id.as_str().as_bytes()); + hasher.update([0]); + hasher.update(self.server_id.as_str().as_bytes()); + let hash = hex::encode(hasher.finalize()); + + format!("{PREFIX}{stem}{SEPARATOR}{}", &hash[..HASH_HEX_LEN]) } } @@ -893,11 +903,11 @@ mod tests { } #[test] - fn plugin_mcp_client_name_is_stable_and_collision_free_for_punctuation() { - fn spec(plugin: &str) -> PluginMcpServerSpec { + fn plugin_mcp_client_name_is_bounded_stable_and_collision_resistant() { + fn spec(plugin: &str, server: &str) -> PluginMcpServerSpec { PluginMcpServerSpec { plugin_id: PluginId::new(plugin).unwrap(), - server_id: PluginMcpServerId::new("unity-editor").unwrap(), + server_id: PluginMcpServerId::new(server).unwrap(), identity: String::new(), display_name: String::new(), command: String::new(), @@ -908,13 +918,46 @@ mod tests { } } - assert_eq!( - spec("com.idea.unity").client_name(), - "idea_plugin__com_2eidea_2eunity__unity_2deditor" + let unity = spec( + "com.idea.unity-developer-tools", + "unity-developer-tools.tools", ); + assert_eq!( + unity.client_name(), + "idea_plugin__com_idea_unity_developer_too__0d29c92c5eef2cb8" + ); + assert!(unity.client_name().is_ascii()); + assert_eq!(unity.client_name().len(), 59); + assert_eq!(format!("mcp__{}", unity.client_name()).len(), 64); + + let punctuation = spec("com.idea-unity", "unity.tools-v1").client_name(); + let stem = punctuation + .strip_prefix("idea_plugin__") + .unwrap() + .rsplit_once("__") + .unwrap() + .0; + assert!(stem + .bytes() + .all(|byte| byte.is_ascii_lowercase() || byte.is_ascii_digit() || byte == b'_')); + + let first = spec( + "com.idea.same-prefix-aaaaaaaaaaaaaaaaaaaaaaaa", + "server.one", + ); + let second = spec( + "com.idea.same-prefix-aaaaaaaaaaaaaaaaaaaaaaaa", + "server.two", + ); + assert_eq!( + first.client_name(), + "idea_plugin__com_idea_same_prefix_aaaaaaa__09f05fca6c0d055e" + ); + assert_eq!(first.client_name(), first.client_name()); assert_ne!( - spec("com.idea-unity").client_name(), - spec("com.idea.unity").client_name() + first.client_name(), + second.client_name(), + "different full identities with the same truncated stem need distinct hashes" ); } diff --git a/crates/infrastructure/src/session/codex.rs b/crates/infrastructure/src/session/codex.rs index a600dc09..5c93c3d6 100644 --- a/crates/infrastructure/src/session/codex.rs +++ b/crates/infrastructure/src/session/codex.rs @@ -63,6 +63,9 @@ pub struct ParsedLine { /// (Codex CLI ≥ 0.147). /// - `{"type":"turn.completed","usage":{…}}` ⇒ [`ReplyEvent::Heartbeat`] (le `Final` /// vient de l'`agent_message`, pas de `turn.completed`). +/// - `{"type":"turn.failed","error":{…}}` / `{"type":"error","message":…}` ⇒ +/// [`ReplyEvent::Error`] avec le message fournisseur. Une erreur de namespace MCP +/// trop long reçoit en plus le code lisible `MCP_NAMESPACE_TOO_LONG`. /// /// Ligne vide ⇒ ignorée ; type inconnu ⇒ ignoré sans erreur ; JSON illisible ⇒ /// [`AgentSessionError::Decode`] (jamais de JSON brut propagé). @@ -113,6 +116,11 @@ pub fn parse_event(line: &str) -> Result { }); events.push(ReplyEvent::Heartbeat); } + Some("turn.failed" | "error") => { + events.push(ReplyEvent::Error { + message: codex_terminal_error_message(&value), + }); + } Some("item.started" | "conversation.item.added" | "response.output_item.added") => { if let Some(item) = value.get("item") { append_item_progress( @@ -335,13 +343,35 @@ fn error_value_message(value: Option<&Value>) -> Option { } fn codex_error_message(root: &Value, item: &Value) -> String { - non_blank_string(item.get("text")) + let message = non_blank_string(item.get("text")) .or_else(|| non_blank_string(item.get("message"))) .or_else(|| error_value_message(item.get("error"))) .or_else(|| non_blank_string(root.get("stderr"))) .or_else(|| non_blank_string(root.get("message"))) .or_else(|| error_value_message(root.get("error"))) - .unwrap_or_else(|| CODEX_ERROR_FALLBACK.to_owned()) + .unwrap_or_else(|| CODEX_ERROR_FALLBACK.to_owned()); + classify_codex_error(message) +} + +fn codex_terminal_error_message(root: &Value) -> String { + let message = non_blank_string(root.get("message")) + .or_else(|| error_value_message(root.get("error"))) + .or_else(|| non_blank_string(root.get("stderr"))) + .unwrap_or_else(|| CODEX_ERROR_FALLBACK.to_owned()); + classify_codex_error(message) +} + +fn classify_codex_error(message: String) -> String { + let lowercase = message.to_ascii_lowercase(); + if lowercase.contains("namespace") + && (lowercase.contains("string_above_max_length") + || lowercase.contains("maximum length 64") + || lowercase.contains("max length 64")) + { + format!("MCP_NAMESPACE_TOO_LONG: {message}") + } else { + message + } } /// Adapter de session structurée Codex. diff --git a/crates/infrastructure/src/session/mod.rs b/crates/infrastructure/src/session/mod.rs index ef6ed4e0..09080fae 100644 --- a/crates/infrastructure/src/session/mod.rs +++ b/crates/infrastructure/src/session/mod.rs @@ -1167,6 +1167,32 @@ mod tests { ); } + #[test] + fn codex_turn_failed_surfaces_namespace_error_with_stable_code() { + let r = codex::parse_event( + r#"{"type":"turn.failed","error":{"message":"Invalid 'input[106].namespace': string too long. Expected a string with maximum length 64, but got a string with length 91 instead. [string_above_max_length]"}}"#, + ) + .unwrap(); + assert!(matches!( + r.events.as_slice(), + [ReplyEvent::Error { message }] + if message.starts_with("MCP_NAMESPACE_TOO_LONG: ") + && message.contains("input[106].namespace") + )); + } + + #[test] + fn codex_top_level_error_surfaces_provider_message() { + let r = + codex::parse_event(r#"{"type":"error","message":"provider request failed"}"#).unwrap(); + assert_eq!( + r.events, + vec![ReplyEvent::Error { + message: "provider request failed".into() + }] + ); + } + /// `agent_message` ⇒ Final (porte le texte de réponse). #[test] fn codex_agent_message_is_final() {