clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-05-21 19:45:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

Remove MCP prefixes from hook display names

Browse Source
This commit is contained in:
pakrym-oai
2026-05-06 20:32:04 -07:00
parent f459c8360d
commit 9d0a9253ef
9 changed files with 93 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
codex-rs/core/src/mcp_tool_call.rs
View File

@@ -115,7 +115,7 @@ pub(crate) async fn handle_mcp_tool_call(
call_id: String,
server: String,
tool_name: String,
hook_tool_name: String,
hook_tool_name: HookToolName,
arguments: String,
) -> HandledMcpToolCall {
// Parse the `arguments` as JSON. An empty string is OK, but invalid JSON
@@ -220,7 +220,7 @@ pub(crate) async fn handle_mcp_tool_call(
turn_context,
&call_id,
&invocation,
&hook_tool_name,
hook_tool_name,
metadata.as_ref(),
approval_mode,
)
@@ -1144,7 +1144,7 @@ async fn maybe_request_mcp_tool_approval(
turn_context: &Arc<TurnContext>,
call_id: &str,
invocation: &McpInvocation,
hook_tool_name: &str,
hook_tool_name: HookToolName,
metadata: Option<&McpToolApprovalMetadata>,
approval_mode: AppToolApproval,
) -> Option<McpToolApprovalDecision> {
@@ -1204,7 +1204,7 @@ async fn maybe_request_mcp_tool_approval(
turn_context,
call_id,
PermissionRequestPayload {
tool_name: HookToolName::new(hook_tool_name),
tool_name: hook_tool_name,
tool_input: invocation
.arguments
.clone()

36
codex-rs/core/src/mcp_tool_call_tests.rs
View File

@@ -55,6 +55,14 @@ fn annotations(
}
}
fn hook_tool_name(name: &str) -> HookToolName {
HookToolName::new(name)
}
fn hook_tool_name_with_legacy_alias(name: &str, legacy_name: &str) -> HookToolName {
HookToolName::new_with_aliases(name, vec![legacy_name.to_string()])
}
fn approval_metadata(
connector_id: Option<&str>,
connector_name: Option<&str>,
@@ -2186,7 +2194,7 @@ async fn approve_mode_skips_when_annotations_do_not_require_approval() {
&turn_context,
"call-1",
&invocation,
"mcp__test__tool",
hook_tool_name("read_only_tool"),
Some(&metadata),
AppToolApproval::Approve,
)
@@ -2259,7 +2267,7 @@ async fn guardian_mode_skips_auto_when_annotations_do_not_require_approval() {
&turn_context,
"call-guardian",
&invocation,
"mcp__test__tool",
hook_tool_name("read_only_tool"),
Some(&metadata),
AppToolApproval::Auto,
)
@@ -2315,7 +2323,7 @@ async fn permission_request_hook_allows_mcp_tool_call() {
&turn_context,
"call-mcp-hook",
&invocation,
"mcp__memory__create_entities",
hook_tool_name_with_legacy_alias("create_entities", "mcp__memory__create_entities"),
Some(&metadata),
AppToolApproval::Auto,
)
@@ -2336,7 +2344,7 @@ async fn permission_request_hook_allows_mcp_tool_call() {
"transcript_path": null,
"model": turn_context.model_info.slug,
"permission_mode": "default",
"tool_name": "mcp__memory__create_entities",
"tool_name": "create_entities",
"hook_event_name": "PermissionRequest",
"tool_input": {
"entities": [{
@@ -2375,7 +2383,7 @@ async fn permission_request_hook_uses_hook_tool_name_without_metadata() {
&turn_context,
"call-mcp-hook-no-metadata",
&invocation,
"mcp__memory__create_entities",
hook_tool_name_with_legacy_alias("create_entities", "mcp__memory__create_entities"),
/*metadata*/ None,
AppToolApproval::Auto,
)
@@ -2396,7 +2404,7 @@ async fn permission_request_hook_uses_hook_tool_name_without_metadata() {
"transcript_path": null,
"model": turn_context.model_info.slug,
"permission_mode": "default",
"tool_name": "mcp__memory__create_entities",
"tool_name": "create_entities",
"hook_event_name": "PermissionRequest",
"tool_input": { "entities": [] }
})]
@@ -2452,7 +2460,7 @@ async fn permission_request_hook_runs_after_remembered_mcp_approval() {
&turn_context,
"call-mcp-remembered",
&invocation,
"mcp__memory__create_entities",
hook_tool_name_with_legacy_alias("create_entities", "mcp__memory__create_entities"),
Some(&metadata),
AppToolApproval::Auto,
)
@@ -2532,7 +2540,7 @@ async fn guardian_mode_mcp_denial_returns_rationale_message() {
&turn_context,
"call-guardian-deny",
&invocation,
"mcp__test__tool",
hook_tool_name("dangerous_tool"),
Some(&metadata),
AppToolApproval::Auto,
)
@@ -2589,7 +2597,7 @@ async fn prompt_mode_waits_for_approval_when_annotations_do_not_require_approval
&turn_context,
"call-prompt",
&invocation,
"mcp__test__tool",
hook_tool_name("read_only_tool"),
Some(&metadata),
AppToolApproval::Prompt,
)
@@ -2664,7 +2672,7 @@ async fn approve_mode_skips_arc_interrupt_for_model() {
&turn_context,
"call-2",
&invocation,
"mcp__test__tool",
hook_tool_name("dangerous_tool"),
Some(&metadata),
AppToolApproval::Approve,
)
@@ -2731,7 +2739,7 @@ async fn custom_approve_mode_skips_arc_interrupt_for_model() {
&turn_context,
"call-2-custom",
&invocation,
"mcp__test__tool",
hook_tool_name("dangerous_tool"),
Some(&metadata),
AppToolApproval::Approve,
)
@@ -2798,7 +2806,7 @@ async fn approve_mode_skips_arc_interrupt_without_annotations() {
&turn_context,
"call-3",
&invocation,
"mcp__test__tool",
hook_tool_name("dangerous_tool"),
Some(&metadata),
AppToolApproval::Approve,
)
@@ -2875,7 +2883,7 @@ async fn full_access_mode_skips_arc_monitor_for_all_approval_modes() {
&turn_context,
"call-2",
&invocation,
"mcp__test__tool",
hook_tool_name("dangerous_tool"),
Some(&metadata),
approval_mode,
)
@@ -2978,7 +2986,7 @@ async fn approve_mode_skips_arc_and_guardian_in_every_permission_mode() {
&turn_context,
"call-3",
&invocation,
"mcp__test__tool",
hook_tool_name("dangerous_tool"),
Some(&metadata),
AppToolApproval::Approve,
)

43
codex-rs/core/src/tools/handlers/mcp.rs
View File

@@ -39,13 +39,16 @@ impl ToolHandler for McpHandler {
}
fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
let ToolPayload::Mcp { raw_arguments, .. } = &invocation.payload else {
let ToolPayload::Mcp {
tool,
raw_arguments,
..
} = &invocation.payload
else {
return None;
};
let tool_name = &self.tool_name;
Some(PreToolUsePayload {
tool_name: HookToolName::new(flat_tool_name(tool_name).into_owned()),
tool_name: mcp_hook_tool_name(tool, &self.tool_name),
tool_input: mcp_hook_tool_input(raw_arguments),
})
}
@@ -55,15 +58,14 @@ impl ToolHandler for McpHandler {
invocation: &ToolInvocation,
result: &Self::Output,
) -> Option<PostToolUsePayload> {
let ToolPayload::Mcp { .. } = &invocation.payload else {
let ToolPayload::Mcp { tool, .. } = &invocation.payload else {
return None;
};
let tool_response =
result.post_tool_use_response(&invocation.call_id, &invocation.payload)?;
let tool_name = &self.tool_name;
Some(PostToolUsePayload {
tool_name: HookToolName::new(flat_tool_name(tool_name).into_owned()),
tool_name: mcp_hook_tool_name(tool, &self.tool_name),
tool_use_id: invocation.call_id.clone(),
tool_input: result.tool_input.clone(),
tool_response,
@@ -96,14 +98,14 @@ impl ToolHandler for McpHandler {
let arguments_str = raw_arguments;
let started = Instant::now();
let hook_tool_name = flat_tool_name(&self.tool_name);
let hook_tool_name = mcp_hook_tool_name(&tool, &self.tool_name);
let result = handle_mcp_tool_call(
Arc::clone(&session),
&turn,
call_id.clone(),
server,
tool,
hook_tool_name.into_owned(),
hook_tool_name,
arguments_str,
)
.await;
@@ -118,6 +120,15 @@ impl ToolHandler for McpHandler {
}
}
fn mcp_hook_tool_name(raw_tool_name: &str, canonical_tool_name: &ToolName) -> HookToolName {
let legacy_name = flat_tool_name(canonical_tool_name);
if legacy_name.as_ref() == raw_tool_name {
HookToolName::new(raw_tool_name.to_string())
} else {
HookToolName::new_with_aliases(raw_tool_name.to_string(), vec![legacy_name.into_owned()])
}
}
fn mcp_hook_tool_input(raw_arguments: &str) -> Value {
if raw_arguments.trim().is_empty() {
return Value::Object(serde_json::Map::new());
@@ -138,7 +149,7 @@ mod tests {
use tokio::sync::Mutex;
#[tokio::test]
async fn mcp_pre_tool_use_payload_uses_model_tool_name_and_raw_args() {
async fn mcp_pre_tool_use_payload_uses_raw_mcp_tool_name_and_args() {
let payload = ToolPayload::Mcp {
server: "memory".to_string(),
tool: "create_entities".to_string(),
@@ -168,7 +179,10 @@ mod tests {
payload,
}),
Some(PreToolUsePayload {
tool_name: HookToolName::new("mcp__memory__create_entities"),
tool_name: HookToolName::new_with_aliases(
"create_entities",
vec!["mcp__memory__create_entities".to_string()],
),
tool_input: json!({
"entities": [{
"name": "Ada",
@@ -180,7 +194,7 @@ mod tests {
}
#[tokio::test]
async fn mcp_post_tool_use_payload_uses_model_tool_name_args_and_result() {
async fn mcp_post_tool_use_payload_uses_raw_mcp_tool_name_args_and_result() {
let payload = ToolPayload::Mcp {
server: "filesystem".to_string(),
tool: "read_file".to_string(),
@@ -223,7 +237,10 @@ mod tests {
assert_eq!(
handler.post_tool_use_payload(&invocation, &output),
Some(PostToolUsePayload {
tool_name: HookToolName::new("mcp__filesystem__read_file"),
tool_name: HookToolName::new_with_aliases(
"read_file",
vec!["mcp__filesystem__read_file".to_string()],
),
tool_use_id: "call-mcp-post".to_string(),
tool_input: json!({
"path": {

9
codex-rs/core/src/tools/hook_names.rs
View File

@@ -25,6 +25,15 @@ impl HookToolName {
}
}
/// Builds a hook tool name with compatibility aliases used for matcher
/// selection only.
pub(crate) fn new_with_aliases(name: impl Into<String>, matcher_aliases: Vec<String>) -> Self {
Self {
name: name.into(),
matcher_aliases,
}
}
/// Returns the hook identity for file edits performed through `apply_patch`.
///
/// The serialized name remains `apply_patch` so logs and policies can key

6
codex-rs/core/src/tools/mod.rs
View File

@@ -33,9 +33,9 @@ pub(crate) const TELEMETRY_PREVIEW_MAX_LINES: usize = 64; // lines
pub(crate) const TELEMETRY_PREVIEW_TRUNCATION_NOTICE: &str =
"[... telemetry preview truncated ...]";
/// Legacy boundaries such as hook payloads, telemetry tags, and Responses tool
/// names still require a single flattened string. Keep comparisons and sorting
/// on `ToolName` itself; use this only when crossing those boundaries.
/// Legacy boundaries such as telemetry tags and Responses tool names still
/// require a single flattened string. Keep comparisons and sorting on
/// `ToolName` itself; use this only when crossing those boundaries.
pub(crate) fn flat_tool_name(tool_name: &ToolName) -> Cow<'_, str> {
match tool_name.namespace.as_deref() {
Some(namespace) => {

8
codex-rs/core/src/tools/registry.rs
View File

@@ -627,8 +627,10 @@ async fn dispatch_after_tool_use_hook(
let session = invocation.session.as_ref();
let turn = invocation.turn.as_ref();
let tool_input = HookToolInput::from(&invocation.payload);
let tool_name = &invocation.tool_name;
let hook_tool_name = flat_tool_name(tool_name);
let hook_tool_name = match &invocation.payload {
ToolPayload::Mcp { tool, .. } => tool.clone(),
_ => flat_tool_name(&invocation.tool_name).into_owned(),
};
let hook_outcomes = session
.hooks()
.dispatch(HookPayload {
@@ -640,7 +642,7 @@ async fn dispatch_after_tool_use_hook(
event: HookEventAfterToolUse {
turn_id: turn.sub_id.clone(),
call_id: invocation.call_id.clone(),
tool_name: hook_tool_name.into_owned(),
tool_name: hook_tool_name,
tool_kind: hook_tool_kind(&tool_input),
tool_input,
executed: dispatch.executed,

7
codex-rs/core/src/unavailable_tool.rs
View File

@@ -1,6 +1,7 @@
use std::collections::BTreeMap;
use std::collections::HashSet;
use crate::tools::flat_tool_name;
use codex_protocol::models::ResponseItem;
use codex_tools::ToolName;
@@ -11,7 +12,7 @@ pub(crate) fn collect_unavailable_called_tools(
let mut unavailable_tools = BTreeMap::new();
let exposed_display_names = exposed_tool_names
.iter()
.map(ToolName::display)
.map(|name| flat_tool_name(name).into_owned())
.collect::<HashSet<_>>();
for item in input {
@@ -29,8 +30,8 @@ pub(crate) fn collect_unavailable_called_tools(
Some(namespace) => ToolName::namespaced(namespace.clone(), name.clone()),
None => ToolName::plain(name.clone()),
};
let display_name = tool_name.display();
if exposed_display_names.contains(&display_name) {
let display_name = flat_tool_name(&tool_name);
if exposed_display_names.contains(display_name.as_ref()) {
continue;
}

2
codex-rs/core/tests/suite/hooks_mcp.rs
View File

@@ -27,7 +27,7 @@ use serde_json::json;
const RMCP_SERVER: &str = "rmcp";
const RMCP_NAMESPACE: &str = "mcp__rmcp__";
const RMCP_ECHO_TOOL_NAME: &str = "mcp__rmcp__echo";
const RMCP_ECHO_TOOL_NAME: &str = "echo";
const RMCP_HOOK_MATCHER: &str = "mcp__rmcp__.*";
const RMCP_ECHO_MESSAGE: &str = "hook e2e ping";

19
codex-rs/protocol/src/tool_name.rs
View File

@@ -36,10 +36,7 @@ impl ToolName {
impl fmt::Display for ToolName {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match &self.namespace {
Some(namespace) => write!(f, "{namespace}{}", self.name),
None => f.write_str(&self.name),
}
f.write_str(&self.name)
}
}
@@ -74,3 +71,17 @@ impl From<&str> for ToolName {
Self::plain(name)
}
}
#[cfg(test)]
mod tests {
use super::*;
use pretty_assertions::assert_eq;
#[test]
fn display_omits_namespace_prefix() {
assert_eq!(
ToolName::namespaced("mcp__server__", "lookup").to_string(),
"lookup"
);
}
}