mirror of
https://github.com/openai/codex.git
synced 2026-05-16 01:02:48 +00:00
hooks: align dynamic tool names with Responses namespaces
This commit is contained in:
@@ -241,7 +241,7 @@ mod tests {
|
||||
assert_eq!(
|
||||
DynamicToolHandler.pre_tool_use_payload(&invocation),
|
||||
Some(PreToolUsePayload {
|
||||
tool_name: HookToolName::new("dynamic__default__automation_update"),
|
||||
tool_name: HookToolName::new("automation_update"),
|
||||
tool_input: json!({ "id": 1 }),
|
||||
})
|
||||
);
|
||||
@@ -265,7 +265,7 @@ mod tests {
|
||||
assert_eq!(
|
||||
DynamicToolHandler.post_tool_use_payload(&invocation, &output),
|
||||
Some(PostToolUsePayload {
|
||||
tool_name: HookToolName::new("dynamic__codex_app__automation_update"),
|
||||
tool_name: HookToolName::new("codex_app__automation_update"),
|
||||
tool_use_id: "call-dynamic".to_string(),
|
||||
tool_input: json!({ "job": "sync" }),
|
||||
tool_response: json!("ok"),
|
||||
|
||||
@@ -29,24 +29,24 @@ impl HookToolName {
|
||||
|
||||
/// Builds the canonical hook-facing identity for dynamic tools.
|
||||
///
|
||||
/// Dynamic tools always use `dynamic__namespace__tool` so hooks can target
|
||||
/// the dynamic-tool surface without colliding with plain Codex tools or
|
||||
/// MCP names. Unnamespaced tools are assigned to the synthetic `default`
|
||||
/// namespace for hook identity purposes.
|
||||
/// Plain dynamic tools keep their plain tool name. Namespaced dynamic tools
|
||||
/// flatten to `namespace__tool`, mirroring the namespaced form used across
|
||||
/// the model-facing tool surface.
|
||||
///
|
||||
/// Each segment is escaped independently to keep the flattened form
|
||||
/// unambiguous even when namespaces or tool names contain separator-like
|
||||
/// substrings or punctuation.
|
||||
/// Each segment is escaped independently so the structural `__` separator
|
||||
/// stays injective even when identifiers contain edge underscores or runs
|
||||
/// of multiple underscores. Other bytes remain percent-encoded
|
||||
/// defensively, though new dynamic tool registration already narrows the
|
||||
/// upstream contract to Responses-compatible ASCII identifiers.
|
||||
pub(crate) fn for_dynamic_tool(tool_name: &ToolName) -> Self {
|
||||
let namespace = tool_name
|
||||
.namespace
|
||||
.as_deref()
|
||||
.unwrap_or(DEFAULT_DYNAMIC_HOOK_NAMESPACE);
|
||||
Self::new(format!(
|
||||
"{DYNAMIC_HOOK_PREFIX}{}__{}",
|
||||
encode_dynamic_segment(namespace),
|
||||
encode_dynamic_segment(&tool_name.name),
|
||||
))
|
||||
match tool_name.namespace.as_deref() {
|
||||
Some(namespace) => Self::new(format!(
|
||||
"{}__{}",
|
||||
encode_dynamic_segment(namespace),
|
||||
encode_dynamic_segment(&tool_name.name),
|
||||
)),
|
||||
None => Self::new(encode_dynamic_segment(&tool_name.name)),
|
||||
}
|
||||
}
|
||||
|
||||
/// Builds the canonical hook-facing identity for MCP tools.
|
||||
@@ -105,12 +105,12 @@ fn encode_dynamic_segment(segment: &str) -> String {
|
||||
|
||||
fn should_preserve_dynamic_byte(bytes: &[u8], index: usize, byte: u8) -> bool {
|
||||
match byte {
|
||||
b'a'..=b'z' | b'A'..=b'Z' | b'0'..=b'9' => true,
|
||||
b'a'..=b'z' | b'A'..=b'Z' | b'0'..=b'9' | b'-' => true,
|
||||
b'_' => {
|
||||
index > 0
|
||||
&& index + 1 < bytes.len()
|
||||
&& bytes[index - 1].is_ascii_alphanumeric()
|
||||
&& bytes[index + 1].is_ascii_alphanumeric()
|
||||
&& (bytes[index - 1].is_ascii_alphanumeric() || bytes[index - 1] == b'-')
|
||||
&& (bytes[index + 1].is_ascii_alphanumeric() || bytes[index + 1] == b'-')
|
||||
&& bytes[index - 1] != b'_'
|
||||
&& bytes[index + 1] != b'_'
|
||||
}
|
||||
@@ -118,8 +118,6 @@ fn should_preserve_dynamic_byte(bytes: &[u8], index: usize, byte: u8) -> bool {
|
||||
}
|
||||
}
|
||||
|
||||
const DYNAMIC_HOOK_PREFIX: &str = "dynamic__";
|
||||
const DEFAULT_DYNAMIC_HOOK_NAMESPACE: &str = "default";
|
||||
const HEX_DIGITS: &[u8; 16] = b"0123456789ABCDEF";
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -129,10 +127,10 @@ mod tests {
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn for_dynamic_tool_assigns_default_namespace_to_plain_tool_names() {
|
||||
fn for_dynamic_tool_keeps_plain_tool_names_plain() {
|
||||
assert_eq!(
|
||||
HookToolName::for_dynamic_tool(&ToolName::plain("tool_search")),
|
||||
HookToolName::new("dynamic__default__tool_search"),
|
||||
HookToolName::new("tool_search"),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -145,22 +143,22 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn for_dynamic_tool_prefixes_dynamic_namespaces() {
|
||||
fn for_dynamic_tool_uses_namespace_separator_for_namespaced_tools() {
|
||||
assert_eq!(
|
||||
HookToolName::for_dynamic_tool(
|
||||
&ToolName::namespaced("codex_app", "automation_update",)
|
||||
),
|
||||
HookToolName::new("dynamic__codex_app__automation_update"),
|
||||
HookToolName::new("codex_app__automation_update"),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn for_dynamic_tool_does_not_spoof_mcp_namespaces() {
|
||||
fn for_dynamic_tool_does_not_spoof_plain_namespaced_shapes() {
|
||||
assert_eq!(
|
||||
HookToolName::for_dynamic_tool(
|
||||
&ToolName::namespaced("mcp__filesystem__", "read_file",)
|
||||
),
|
||||
HookToolName::new("dynamic__mcp%5F%5Ffilesystem%5F%5F__read_file"),
|
||||
HookToolName::new("mcp%5F%5Ffilesystem%5F%5F__read_file"),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -169,16 +167,16 @@ mod tests {
|
||||
let first = HookToolName::for_dynamic_tool(&ToolName::namespaced("foo__bar", "baz"));
|
||||
let second = HookToolName::for_dynamic_tool(&ToolName::namespaced("foo", "bar__baz"));
|
||||
|
||||
assert_eq!(first, HookToolName::new("dynamic__foo%5F%5Fbar__baz"));
|
||||
assert_eq!(second, HookToolName::new("dynamic__foo__bar%5F%5Fbaz"));
|
||||
assert_eq!(first, HookToolName::new("foo%5F%5Fbar__baz"));
|
||||
assert_eq!(second, HookToolName::new("foo__bar%5F%5Fbaz"));
|
||||
assert_ne!(first, second);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn for_dynamic_tool_escapes_punctuation_and_edge_underscores() {
|
||||
fn for_dynamic_tool_escapes_edge_underscores_and_preserves_hyphens() {
|
||||
assert_eq!(
|
||||
HookToolName::for_dynamic_tool(&ToolName::namespaced("_google-drive", "update.file_",)),
|
||||
HookToolName::new("dynamic__%5Fgoogle%2Ddrive__update%2Efile%5F"),
|
||||
HookToolName::new("%5Fgoogle-drive__update%2Efile%5F"),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -186,15 +184,15 @@ mod tests {
|
||||
fn for_dynamic_tool_keeps_single_internal_underscores() {
|
||||
assert_eq!(
|
||||
HookToolName::for_dynamic_tool(&ToolName::namespaced("codex_app", "automation_update")),
|
||||
HookToolName::new("dynamic__codex_app__automation_update"),
|
||||
HookToolName::new("codex_app__automation_update"),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn for_dynamic_tool_percent_encodes_utf8_bytes() {
|
||||
fn for_dynamic_tool_percent_encodes_unsupported_bytes_defensively() {
|
||||
assert_eq!(
|
||||
HookToolName::for_dynamic_tool(&ToolName::namespaced("検", "索")),
|
||||
HookToolName::new("dynamic__%E6%A4%9C__%E7%B4%A2"),
|
||||
HookToolName::new("%E6%A4%9C__%E7%B4%A2"),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -36,8 +36,8 @@ use wiremock::MockServer;
|
||||
|
||||
const DYNAMIC_TOOL_NAME: &str = "automation_update";
|
||||
const DYNAMIC_NAMESPACE: &str = "codex_app";
|
||||
const PLAIN_DYNAMIC_HOOK_NAME: &str = "dynamic__default__automation_update";
|
||||
const DYNAMIC_HOOK_NAME: &str = "dynamic__codex_app__automation_update";
|
||||
const PLAIN_DYNAMIC_HOOK_NAME: &str = "automation_update";
|
||||
const DYNAMIC_HOOK_NAME: &str = "codex_app__automation_update";
|
||||
|
||||
fn dynamic_tool(namespace: Option<&str>, name: &str) -> DynamicToolSpec {
|
||||
DynamicToolSpec {
|
||||
|
||||
@@ -216,12 +216,12 @@ mod tests {
|
||||
Some("lookup+ticket")
|
||||
));
|
||||
assert!(matches_matcher(
|
||||
Some("dynamic__codex_app__automation_update"),
|
||||
Some("dynamic__codex_app__automation_update")
|
||||
Some("codex_app__automation_update"),
|
||||
Some("codex_app__automation_update")
|
||||
));
|
||||
assert!(!matches_matcher(
|
||||
Some("dynamic__codex_app"),
|
||||
Some("dynamic__codex_app__automation_update")
|
||||
Some("codex_app"),
|
||||
Some("codex_app__automation_update")
|
||||
));
|
||||
assert!(matches_matcher(
|
||||
Some("mcp__memory__create_entities"),
|
||||
@@ -260,40 +260,37 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dynamic_matchers_support_regex_wildcards() {
|
||||
fn namespaced_dynamic_matchers_support_regex_wildcards() {
|
||||
assert!(matches_matcher(
|
||||
Some("dynamic__codex_app__.*"),
|
||||
Some("dynamic__codex_app__automation_update")
|
||||
Some("codex_app__.*"),
|
||||
Some("codex_app__automation_update")
|
||||
));
|
||||
assert!(matches_matcher(
|
||||
Some("dynamic__.*__automation_update"),
|
||||
Some("dynamic__codex_app__automation_update")
|
||||
Some(".*__automation_update"),
|
||||
Some("codex_app__automation_update")
|
||||
));
|
||||
assert!(!matches_matcher(
|
||||
Some("dynamic__other_app__.*"),
|
||||
Some("dynamic__codex_app__automation_update")
|
||||
Some("other_app__.*"),
|
||||
Some("codex_app__automation_update")
|
||||
));
|
||||
assert_eq!(validate_matcher_pattern("dynamic__codex_app__.*"), Ok(()));
|
||||
assert_eq!(validate_matcher_pattern("codex_app__.*"), Ok(()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dynamic_matchers_support_percent_encoded_names() {
|
||||
assert!(matches_matcher(
|
||||
Some("dynamic__default__%E6%A4%9C%E7%B4%A2"),
|
||||
Some("dynamic__default__%E6%A4%9C%E7%B4%A2")
|
||||
Some("%E6%A4%9C%E7%B4%A2"),
|
||||
Some("%E6%A4%9C%E7%B4%A2")
|
||||
));
|
||||
assert!(!matches_matcher(
|
||||
Some("dynamic__default__%E6%A4%9C%E7%B4%A2"),
|
||||
Some("dynamic__default__%E6%A4%9C%E7%B4%A2_extra")
|
||||
Some("%E6%A4%9C%E7%B4%A2"),
|
||||
Some("%E6%A4%9C%E7%B4%A2_extra")
|
||||
));
|
||||
assert!(matches_matcher(
|
||||
Some("dynamic__default__%E6%A4%9C.*"),
|
||||
Some("dynamic__default__%E6%A4%9C%E7%B4%A2")
|
||||
Some("%E6%A4%9C.*"),
|
||||
Some("%E6%A4%9C%E7%B4%A2")
|
||||
));
|
||||
assert_eq!(
|
||||
validate_matcher_pattern("dynamic__default__%E6%A4%9C.*"),
|
||||
Ok(())
|
||||
);
|
||||
assert_eq!(validate_matcher_pattern("%E6%A4%9C.*"), Ok(()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user