merge(domain,application,infrastructure): fix #333 — namespace MCP plugin borné à 64 chars pour Codex/OpenAI, client_name tronqué + hash déterministe, erreurs terminales Codex remontées (QA verte)
This commit is contained in:
@ -5581,9 +5581,9 @@ mod tests {
|
|||||||
#[test]
|
#[test]
|
||||||
fn codex_plugin_server_projection_removes_stale_managed_tables() {
|
fn codex_plugin_server_projection_removes_stale_managed_tables() {
|
||||||
let current = assigned_plugin_server();
|
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!(
|
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(
|
let declaration = codex_mcp_declaration(
|
||||||
domain::profile::McpTransport::Stdio,
|
domain::profile::McpTransport::Stdio,
|
||||||
@ -5593,6 +5593,7 @@ mod tests {
|
|||||||
let rendered = codex_config_toml(Some(&existing), &declaration, "/run", "/project");
|
let rendered = codex_config_toml(Some(&existing), &declaration, "/run", "/project");
|
||||||
assert!(!rendered.contains(stale));
|
assert!(!rendered.contains(stale));
|
||||||
assert!(rendered.contains("user_key = \"keep\""));
|
assert!(rendered.contains("user_key = \"keep\""));
|
||||||
|
assert!(rendered.contains("[mcp_servers.user_server]\ncommand = \"user\""));
|
||||||
assert!(rendered.contains("[preview]\nenabled = true"));
|
assert!(rendered.contains("[preview]\nenabled = true"));
|
||||||
assert_eq!(rendered.matches(¤t.client_name()).count(), 1);
|
assert_eq!(rendered.matches(¤t.client_name()).count(), 1);
|
||||||
}
|
}
|
||||||
@ -5600,10 +5601,12 @@ mod tests {
|
|||||||
#[test]
|
#[test]
|
||||||
fn internal_restart_refreshes_plugin_servers_without_replacing_real_idea_wiring() {
|
fn internal_restart_refreshes_plugin_servers_without_replacing_real_idea_wiring() {
|
||||||
let current = assigned_plugin_server();
|
let current = assigned_plugin_server();
|
||||||
|
let stale = "idea_plugin__com_2eidea_2eunity_2ddeveloper_2dtools__unity_2ddeveloper_2dtools_2etools";
|
||||||
let existing_json = br#"{
|
let existing_json = br#"{
|
||||||
"mcpServers": {
|
"mcpServers": {
|
||||||
"idea": { "command": "/opt/Idea.AppImage", "args": ["mcp-server", "--endpoint", "/live.sock"] },
|
"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))
|
let claude = reconcile_plugin_mcp_json(existing_json, std::slice::from_ref(¤t))
|
||||||
@ -5613,18 +5616,18 @@ mod tests {
|
|||||||
claude["mcpServers"]["idea"]["command"],
|
claude["mcpServers"]["idea"]["command"],
|
||||||
"/opt/Idea.AppImage"
|
"/opt/Idea.AppImage"
|
||||||
);
|
);
|
||||||
assert!(claude["mcpServers"]
|
assert!(claude["mcpServers"].get(stale).is_none());
|
||||||
.get("idea_plugin__stale__server")
|
assert_eq!(claude["mcpServers"]["user-server"]["command"], "user");
|
||||||
.is_none());
|
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
claude["mcpServers"][current.client_name()]["command"],
|
claude["mcpServers"][current.client_name()]["command"],
|
||||||
current.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 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));
|
let codex = merge_codex_plugin_mcp_tables(&existing_toml, std::slice::from_ref(¤t));
|
||||||
assert!(codex.contains("command = \"/opt/Idea.AppImage\""));
|
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()));
|
assert!(codex.contains(¤t.client_name()));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -910,7 +910,7 @@ mod tests {
|
|||||||
let server = &servers[0];
|
let server = &servers[0];
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
server.client_name(),
|
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!(server.command, "/plugins/unity/bin/server");
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
|
|||||||
@ -2,6 +2,7 @@
|
|||||||
|
|
||||||
use serde::{Deserialize, Serialize};
|
use serde::{Deserialize, Serialize};
|
||||||
use serde_json::Value;
|
use serde_json::Value;
|
||||||
|
use sha2::{Digest, Sha256};
|
||||||
use thiserror::Error;
|
use thiserror::Error;
|
||||||
|
|
||||||
/// Plugin domain validation error.
|
/// Plugin domain validation error.
|
||||||
@ -797,32 +798,41 @@ pub struct PluginMcpServerSpec {
|
|||||||
}
|
}
|
||||||
|
|
||||||
impl 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
|
/// Codex prefixes this value with `mcp__` when it builds tool namespaces, whose
|
||||||
/// paths and becomes part of the tool namespace in MCP clients. Encode every
|
/// provider limit is 64 ASCII bytes. Keep this value at most 59 bytes and retain
|
||||||
/// non-alphanumeric byte instead of merely replacing it, so distinct ids can
|
/// collision resistance after truncating the readable stem by suffixing the
|
||||||
/// never collapse onto the same session server name.
|
/// first 16 hexadecimal characters of SHA-256(`plugin_id + "\0" + server_id`).
|
||||||
#[must_use]
|
#[must_use]
|
||||||
pub fn client_name(&self) -> String {
|
pub fn client_name(&self) -> String {
|
||||||
fn component(raw: &str) -> String {
|
const PREFIX: &str = "idea_plugin__";
|
||||||
let mut out = String::new();
|
const HASH_HEX_LEN: usize = 16;
|
||||||
for byte in raw.bytes() {
|
const MAX_CLIENT_NAME_LEN: usize = 59;
|
||||||
if byte.is_ascii_alphanumeric() {
|
const SEPARATOR: &str = "__";
|
||||||
out.push(char::from(byte).to_ascii_lowercase());
|
const MAX_STEM_LEN: usize =
|
||||||
} else {
|
MAX_CLIENT_NAME_LEN - PREFIX.len() - SEPARATOR.len() - HASH_HEX_LEN;
|
||||||
out.push('_');
|
|
||||||
out.push_str(&format!("{byte:02x}"));
|
|
||||||
}
|
|
||||||
}
|
|
||||||
out
|
|
||||||
}
|
|
||||||
|
|
||||||
format!(
|
let raw_stem = format!("{}__{}", self.plugin_id.as_str(), self.server_id.as_str());
|
||||||
"idea_plugin__{}__{}",
|
let stem: String = raw_stem
|
||||||
component(self.plugin_id.as_str()),
|
.bytes()
|
||||||
component(self.server_id.as_str())
|
.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]
|
#[test]
|
||||||
fn plugin_mcp_client_name_is_stable_and_collision_free_for_punctuation() {
|
fn plugin_mcp_client_name_is_bounded_stable_and_collision_resistant() {
|
||||||
fn spec(plugin: &str) -> PluginMcpServerSpec {
|
fn spec(plugin: &str, server: &str) -> PluginMcpServerSpec {
|
||||||
PluginMcpServerSpec {
|
PluginMcpServerSpec {
|
||||||
plugin_id: PluginId::new(plugin).unwrap(),
|
plugin_id: PluginId::new(plugin).unwrap(),
|
||||||
server_id: PluginMcpServerId::new("unity-editor").unwrap(),
|
server_id: PluginMcpServerId::new(server).unwrap(),
|
||||||
identity: String::new(),
|
identity: String::new(),
|
||||||
display_name: String::new(),
|
display_name: String::new(),
|
||||||
command: String::new(),
|
command: String::new(),
|
||||||
@ -908,13 +918,46 @@ mod tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
assert_eq!(
|
let unity = spec(
|
||||||
spec("com.idea.unity").client_name(),
|
"com.idea.unity-developer-tools",
|
||||||
"idea_plugin__com_2eidea_2eunity__unity_2deditor"
|
"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!(
|
assert_ne!(
|
||||||
spec("com.idea-unity").client_name(),
|
first.client_name(),
|
||||||
spec("com.idea.unity").client_name()
|
second.client_name(),
|
||||||
|
"different full identities with the same truncated stem need distinct hashes"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -63,6 +63,9 @@ pub struct ParsedLine {
|
|||||||
/// (Codex CLI ≥ 0.147).
|
/// (Codex CLI ≥ 0.147).
|
||||||
/// - `{"type":"turn.completed","usage":{…}}` ⇒ [`ReplyEvent::Heartbeat`] (le `Final`
|
/// - `{"type":"turn.completed","usage":{…}}` ⇒ [`ReplyEvent::Heartbeat`] (le `Final`
|
||||||
/// vient de l'`agent_message`, pas de `turn.completed`).
|
/// 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 ⇒
|
/// Ligne vide ⇒ ignorée ; type inconnu ⇒ ignoré sans erreur ; JSON illisible ⇒
|
||||||
/// [`AgentSessionError::Decode`] (jamais de JSON brut propagé).
|
/// [`AgentSessionError::Decode`] (jamais de JSON brut propagé).
|
||||||
@ -113,6 +116,11 @@ pub fn parse_event(line: &str) -> Result<ParsedLine, AgentSessionError> {
|
|||||||
});
|
});
|
||||||
events.push(ReplyEvent::Heartbeat);
|
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") => {
|
Some("item.started" | "conversation.item.added" | "response.output_item.added") => {
|
||||||
if let Some(item) = value.get("item") {
|
if let Some(item) = value.get("item") {
|
||||||
append_item_progress(
|
append_item_progress(
|
||||||
@ -335,13 +343,35 @@ fn error_value_message(value: Option<&Value>) -> Option<String> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn codex_error_message(root: &Value, item: &Value) -> String {
|
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(|| non_blank_string(item.get("message")))
|
||||||
.or_else(|| error_value_message(item.get("error")))
|
.or_else(|| error_value_message(item.get("error")))
|
||||||
.or_else(|| non_blank_string(root.get("stderr")))
|
.or_else(|| non_blank_string(root.get("stderr")))
|
||||||
.or_else(|| non_blank_string(root.get("message")))
|
.or_else(|| non_blank_string(root.get("message")))
|
||||||
.or_else(|| error_value_message(root.get("error")))
|
.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.
|
/// Adapter de session structurée Codex.
|
||||||
|
|||||||
@ -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).
|
/// `agent_message` ⇒ Final (porte le texte de réponse).
|
||||||
#[test]
|
#[test]
|
||||||
fn codex_agent_message_is_final() {
|
fn codex_agent_message_is_final() {
|
||||||
|
|||||||
Reference in New Issue
Block a user