github/codex
4
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-20 00:18:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

Support app targets in executor plugin hooks (#41456)

Browse Source 
## What changed

- Admit the curated remote Browser plugin's `Stop` and `SubagentStop` hooks when its listed `browser.turn_ended` tool matches the expected connector and is enabled by app policy.
- Carry trusted app routing metadata and the MCP environment through hook registration and execution.
- Register hooks from multiple executor environments, while deduplicating cleanup calls that share an event, MCP environment, server, and tool.

## Testing

- Cover Browser and Computer Use cleanup through separate MCP routes, including user- and managed-policy disablement.
- Verify routing metadata, environment selection, multi-environment registration, and target deduplication.

GitOrigin-RevId: 66de088fd201b9ed0db8b97c23d33de88135120d
This commit is contained in:
Samuel Yuan
2026-08-29 03:14:35 +00:00
committed by copyberry
parent 62b458c931
commit 2181224dad
10 changed files with 771 additions and 168 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/Cargo.lock
Generated
+1
View File
@@ -3943,6 +3943,7 @@ dependencies = [
"codex-utils-path-uri",
"codex-utils-plugins",
"pretty_assertions",
"serde_json",
"thiserror 2.0.18",
]
codex-rs/core-plugins/src/executor_hooks.rs
+180 -44
View File
@@ -1,124 +1,192 @@
//! Executor-discovered plugin hook admission and trusted MCP routing.
use codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID;
use codex_config::HookHandlerConfig;
use codex_exec_server::ExecutorCapabilityDiscoverySnapshot;
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
use codex_mcp::MCP_TOOL_CODEX_APPS_META_KEY;
use codex_mcp::ToolInfo;
use codex_plugin::ExecutorPluginHookSource;
use codex_plugin::PluginId;
use codex_plugin::manifest::PluginManifestHooks;
use codex_protocol::capabilities::CapabilityRootLocation;
use codex_protocol::protocol::HookEventName;
use serde_json::Map;
use serde_json::Value;
use crate::manifest::parse_plugin_manifest_uri;
struct AllowlistedExecutorPluginHook {
plugin_id: &'static str,
event: HookEventName,
server: &'static str,
tool: &'static str,
target: ExecutorPluginHookTarget,
}
enum ExecutorPluginHookTarget {
Executor {
server: &'static str,
tool: &'static str,
},
App {
connector_id: &'static str,
tool: &'static str,
},
}
// Executor plugin manifests are unsigned, so temporarily hardcode the expected
// bundled plugin identities, events, and MCP targets until plugin signing lands.
// plugin identities, events, and MCP targets until plugin signing lands.
const ALLOWLISTED_EXECUTOR_PLUGIN_HOOKS: &[AllowlistedExecutorPluginHook] = &[
AllowlistedExecutorPluginHook {
plugin_id: "browser@openai-bundled",
event: HookEventName::Stop,
server: "node_repl",
tool: "turn_ended",
target: ExecutorPluginHookTarget::Executor {
server: "node_repl",
tool: "turn_ended",
},
},
AllowlistedExecutorPluginHook {
plugin_id: "browser@openai-bundled",
event: HookEventName::Interrupt,
server: "node_repl",
tool: "turn_ended",
target: ExecutorPluginHookTarget::Executor {
server: "node_repl",
tool: "turn_ended",
},
},
AllowlistedExecutorPluginHook {
plugin_id: "browser@openai-bundled",
event: HookEventName::SubagentStop,
server: "node_repl",
tool: "turn_ended",
target: ExecutorPluginHookTarget::Executor {
server: "node_repl",
tool: "turn_ended",
},
},
AllowlistedExecutorPluginHook {
plugin_id: "chrome@openai-bundled",
event: HookEventName::Stop,
server: "node_repl",
tool: "turn_ended",
target: ExecutorPluginHookTarget::Executor {
server: "node_repl",
tool: "turn_ended",
},
},
AllowlistedExecutorPluginHook {
plugin_id: "chrome@openai-bundled",
event: HookEventName::Interrupt,
server: "node_repl",
tool: "turn_ended",
target: ExecutorPluginHookTarget::Executor {
server: "node_repl",
tool: "turn_ended",
},
},
AllowlistedExecutorPluginHook {
plugin_id: "chrome@openai-bundled",
event: HookEventName::SubagentStop,
server: "node_repl",
tool: "turn_ended",
target: ExecutorPluginHookTarget::Executor {
server: "node_repl",
tool: "turn_ended",
},
},
AllowlistedExecutorPluginHook {
plugin_id: "chrome-dev@openai-bundled",
event: HookEventName::Stop,
server: "node_repl",
tool: "turn_ended",
target: ExecutorPluginHookTarget::Executor {
server: "node_repl",
tool: "turn_ended",
},
},
AllowlistedExecutorPluginHook {
plugin_id: "chrome-dev@openai-bundled",
event: HookEventName::Interrupt,
server: "node_repl",
tool: "turn_ended",
target: ExecutorPluginHookTarget::Executor {
server: "node_repl",
tool: "turn_ended",
},
},
AllowlistedExecutorPluginHook {
plugin_id: "chrome-dev@openai-bundled",
event: HookEventName::SubagentStop,
server: "node_repl",
tool: "turn_ended",
target: ExecutorPluginHookTarget::Executor {
server: "node_repl",
tool: "turn_ended",
},
},
AllowlistedExecutorPluginHook {
plugin_id: "chrome-internal@openai-bundled",
event: HookEventName::Stop,
server: "node_repl",
tool: "turn_ended",
target: ExecutorPluginHookTarget::Executor {
server: "node_repl",
tool: "turn_ended",
},
},
AllowlistedExecutorPluginHook {
plugin_id: "chrome-internal@openai-bundled",
event: HookEventName::Interrupt,
server: "node_repl",
tool: "turn_ended",
target: ExecutorPluginHookTarget::Executor {
server: "node_repl",
tool: "turn_ended",
},
},
AllowlistedExecutorPluginHook {
plugin_id: "chrome-internal@openai-bundled",
event: HookEventName::SubagentStop,
server: "node_repl",
tool: "turn_ended",
target: ExecutorPluginHookTarget::Executor {
server: "node_repl",
tool: "turn_ended",
},
},
AllowlistedExecutorPluginHook {
plugin_id: "computer-use@openai-bundled",
event: HookEventName::Stop,
server: "node_repl",
tool: "turn_ended",
target: ExecutorPluginHookTarget::Executor {
server: "node_repl",
tool: "turn_ended",
},
},
AllowlistedExecutorPluginHook {
plugin_id: "computer-use@openai-bundled",
event: HookEventName::Interrupt,
server: "node_repl",
tool: "turn_ended",
target: ExecutorPluginHookTarget::Executor {
server: "node_repl",
tool: "turn_ended",
},
},
AllowlistedExecutorPluginHook {
plugin_id: "computer-use@openai-bundled",
event: HookEventName::SubagentStop,
server: "node_repl",
tool: "turn_ended",
target: ExecutorPluginHookTarget::Executor {
server: "node_repl",
tool: "turn_ended",
},
},
AllowlistedExecutorPluginHook {
plugin_id: "browser@openai-curated-remote",
event: HookEventName::Stop,
target: ExecutorPluginHookTarget::App {
connector_id: "connector_openai_browser",
tool: "browser.turn_ended",
},
},
AllowlistedExecutorPluginHook {
plugin_id: "browser@openai-curated-remote",
event: HookEventName::SubagentStop,
target: ExecutorPluginHookTarget::App {
connector_id: "connector_openai_browser",
tool: "browser.turn_ended",
},
},
];
/// Returns accepted inline hook sources from executor-discovered plugin manifests.
/// Each source carries its trusted MCP routing metadata. `lookup_enabled_tool` must use a
/// consistent catalog/config snapshot, include app-only tools, and exclude tools disabled by
/// app policy.
///
/// Executor scoped hooks are best-effort because executor capabilities can become available
/// after earlier lifecycle events have passed.
///
/// Note: Executor manifests are not signed yet, so temporarily we only admit the known cleanup
/// hook from the bundled Browser, Chrome, Chrome Dev, Chrome Internal, and Computer Use plugins.
pub fn executor_plugin_hook_sources(
/// hooks from the bundled plugins above and the remote Browser plugin.
pub fn executor_plugin_hook_sources<'a>(
snapshot: &ExecutorCapabilityDiscoverySnapshot,
lookup_enabled_tool: impl Fn(&str, &str) -> Option<&'a ToolInfo>,
) -> Vec<ExecutorPluginHookSource> {
let mut sources = Vec::new();
@@ -158,6 +226,8 @@ pub fn executor_plugin_hook_sources(
sources.push(ExecutorPluginHookSource {
plugin_id: plugin_id.clone(),
environment_id: environment_id.clone(),
mcp_environment_id: None,
mcp_metadata: None,
plugin_root: plugin_root.clone(),
manifest_path: plugin.manifest.path.clone(),
source_relative_path: format!("{manifest_relative_path}#hooks[{hook_index}]"),
@@ -167,10 +237,17 @@ pub fn executor_plugin_hook_sources(
}
// FIXME: Remove this temporary filter once executor plugin hooks can be trusted.
sources.into_iter().filter_map(allowlisted_source).collect()
sources
.into_iter()
.filter_map(|source| allowlisted_source(source, &lookup_enabled_tool))
.filter_map(|source| resolve_mcp_routing(source, &lookup_enabled_tool))
.collect()
}
fn allowlisted_source(mut source: ExecutorPluginHookSource) -> Option<ExecutorPluginHookSource> {
fn allowlisted_source<'a>(
mut source: ExecutorPluginHookSource,
lookup_enabled_tool: &impl Fn(&str, &str) -> Option<&'a ToolInfo>,
) -> Option<ExecutorPluginHookSource> {
let plugin_id = source.plugin_id.as_key();
for (event, groups) in source.hooks.matcher_groups_mut() {
groups.retain_mut(|group| {
@@ -178,15 +255,39 @@ fn allowlisted_source(mut source: ExecutorPluginHookSource) -> Option<ExecutorPl
return false;
}
group.hooks.retain(|handler| {
let HookHandlerConfig::McpTool { server, tool, .. } = handler else {
let HookHandlerConfig::McpTool {
server,
tool,
input,
..
} = handler
else {
return false;
};
ALLOWLISTED_EXECUTOR_PLUGIN_HOOKS.iter().any(|hook| {
hook.plugin_id == plugin_id
&& hook.event == event
&& hook.server == server
&& hook.tool == tool
})
let Some(hook) = ALLOWLISTED_EXECUTOR_PLUGIN_HOOKS
.iter()
.find(|hook| hook.plugin_id == plugin_id && hook.event == event)
else {
return false;
};
match hook.target {
ExecutorPluginHookTarget::Executor {
server: expected_server,
tool: expected_tool,
} => server == expected_server && tool == expected_tool,
ExecutorPluginHookTarget::App {
connector_id,
tool: expected_tool,
} => {
// Raw Apps tool names can collide; admission must also match the listed connector.
server == CODEX_APPS_MCP_SERVER_NAME
&& tool == expected_tool
&& input.is_empty()
&& lookup_enabled_tool(server, tool).is_some_and(|tool_info| {
tool_info.connector_id.as_deref() == Some(connector_id)
})
}
}
});
!group.hooks.is_empty()
});
@@ -194,6 +295,41 @@ fn allowlisted_source(mut source: ExecutorPluginHookSource) -> Option<ExecutorPl
(!source.hooks.is_empty()).then_some(source)
}
/// Resolves routing for an admitted MCP hook source.
fn resolve_mcp_routing<'a>(
mut source: ExecutorPluginHookSource,
lookup_enabled_tool: &impl Fn(&str, &str) -> Option<&'a ToolInfo>,
) -> Option<ExecutorPluginHookSource> {
let HookHandlerConfig::McpTool { server, tool, .. } = source
.hooks
.matcher_groups_mut()
.into_iter()
.find_map(|(_, groups)| groups.first())?
.hooks
.first()?
else {
return None;
};
if server != CODEX_APPS_MCP_SERVER_NAME {
return Some(source);
}
let tool_info = lookup_enabled_tool(server, tool)?;
let routing_metadata = tool_info
.tool
.meta
.as_ref()?
.get(MCP_TOOL_CODEX_APPS_META_KEY)?
.as_object()?;
routing_metadata.get("resource_uri")?.as_str()?;
source.mcp_environment_id = Some(DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string());
source.mcp_metadata = Some(Map::from_iter([(
MCP_TOOL_CODEX_APPS_META_KEY.to_string(),
Value::Object(routing_metadata.clone()),
)]));
Some(source)
}
#[cfg(test)]
#[path = "executor_hooks_tests.rs"]
mod tests;
codex-rs/core-plugins/src/executor_hooks_tests.rs
+142 -5
View File
@@ -80,6 +80,8 @@ fn expected_source(index: usize) -> ExecutorPluginHookSource {
ExecutorPluginHookSource {
plugin_id: PluginId::parse("computer-use@openai-bundled").expect("plugin id"),
environment_id: "executor-a".to_string(),
mcp_environment_id: None,
mcp_metadata: None,
plugin_root: PathUri::parse("file:///plugins/computer-use").expect("plugin root"),
manifest_path: PathUri::parse("file:///plugins/computer-use/.codex-plugin/plugin.json")
.expect("manifest path"),
@@ -128,7 +130,7 @@ fn discovers_allowlisted_executor_plugin_hook_sources() {
manifest,
);
let sources = executor_plugin_hook_sources(&snapshot);
let sources = executor_plugin_hook_sources(&snapshot, |_, _| None);
let mut interrupt_source = expected_source(/*index*/ 1);
interrupt_source.hooks.interrupt = std::mem::take(&mut interrupt_source.hooks.stop);
let mut stop_and_interrupt_source = expected_source(/*index*/ 2);
@@ -175,7 +177,10 @@ fn filters_mixed_handlers_without_rewriting_allowed_groups() {
manifest,
);
assert_eq!(executor_plugin_hook_sources(&snapshot), vec![expected]);
assert_eq!(
executor_plugin_hook_sources(&snapshot, |_, _| None),
vec![expected]
);
}
#[test]
@@ -204,7 +209,139 @@ fn preserves_allowlisted_executor_plugin_hook_options() {
};
expected.hooks.interrupt = expected.hooks.stop.clone();
assert_eq!(executor_plugin_hook_sources(&snapshot), vec![expected]);
assert_eq!(
executor_plugin_hook_sources(&snapshot, |_, _| None),
vec![expected]
);
}
#[test]
fn resolves_apps_hook_metadata_from_the_registered_connector() {
let mut manifest = cleanup_hook_manifest();
manifest["name"] = json!("browser");
let handler = &mut manifest["hooks"]["hooks"]["Stop"][0]["hooks"][0];
handler["server"] = json!("codex_apps");
handler["tool"] = json!("browser.turn_ended");
handler["input"] = json!({});
let mut expected = expected_source(/*index*/ 0);
expected.plugin_id = PluginId::parse("browser@openai-curated-remote").expect("plugin id");
expected.mcp_environment_id = Some(DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string());
let routing = json!({
"resource_uri": "/connector_openai_browser/browser-link/turn_ended",
"contains_mcp_source": true,
});
expected.mcp_metadata = Some(Map::from_iter([(
MCP_TOOL_CODEX_APPS_META_KEY.to_string(),
routing.clone(),
)]));
for (name, event, connector_id, input, routing_metadata, admitted) in [
(
"registered app",
"Stop",
Some("connector_openai_browser"),
json!({}),
routing.clone(),
true,
),
(
"colliding tool name",
"Stop",
Some("connector_other_browser"),
json!({}),
routing.clone(),
false,
),
(
"unavailable tool",
"Stop",
None,
json!({}),
routing.clone(),
false,
),
(
"registered subagent app",
"SubagentStop",
Some("connector_openai_browser"),
json!({}),
routing.clone(),
true,
),
(
"colliding subagent tool name",
"SubagentStop",
Some("connector_other_browser"),
json!({}),
routing.clone(),
false,
),
(
"manifest arguments",
"Stop",
Some("connector_openai_browser"),
json!({ "untrusted": "manifest-provided input" }),
routing,
false,
),
(
"missing routing metadata",
"Stop",
Some("connector_openai_browser"),
json!({}),
json!(null),
false,
),
(
"invalid resource URI",
"Stop",
Some("connector_openai_browser"),
json!({}),
json!({ "resource_uri": 42 }),
false,
),
] {
let mut manifest = manifest.clone();
manifest["hooks"]["hooks"]["Stop"][0]["hooks"][0]["input"] = input;
let events = manifest["hooks"]["hooks"]
.as_object_mut()
.expect("hook events");
let groups = events.remove("Stop").expect("stop groups");
events.insert(event.to_string(), groups);
let mut expected = expected.clone();
expected.hooks = serde_json::from_value(manifest["hooks"]["hooks"].clone()).expect("hooks");
let snapshot = snapshot_for_manifest(
"browser@openai-curated-remote",
"executor-a",
"file:///plugins/computer-use/.codex-plugin/plugin.json",
manifest,
);
let tool_info = connector_id.map(|connector_id| {
serde_json::from_value::<ToolInfo>(json!({
"server_name": "codex_apps",
"tool_name": "turn_ended",
"tool_namespace": "browser",
"connector_id": connector_id,
"tool": {
"name": "browser.turn_ended",
"inputSchema": { "type": "object" },
"_meta": { "_codex_apps": routing_metadata },
},
}))
.expect("listed tool")
});
let actual = executor_plugin_hook_sources(&snapshot, |server, tool| {
tool_info
.as_ref()
.filter(|info| info.server_name == server && info.tool.name == tool)
});
let expected = if admitted {
vec![expected.clone()]
} else {
Vec::new()
};
assert_eq!(actual, expected, "{name}");
}
}
#[test]
@@ -269,7 +406,7 @@ fn ignores_unallowlisted_executor_plugin_hooks() {
);
assert_eq!(
executor_plugin_hook_sources(&snapshot),
executor_plugin_hook_sources(&snapshot, |_, _| None),
Vec::<ExecutorPluginHookSource>::new(),
"{name}"
);
@@ -289,7 +426,7 @@ fn ignores_file_backed_executor_plugin_hooks() {
);
assert_eq!(
executor_plugin_hook_sources(&file_backed),
executor_plugin_hook_sources(&file_backed, |_, _| None),
Vec::<ExecutorPluginHookSource>::new()
);
}
codex-rs/core/src/hook_runtime.rs
+40 -8
View File
@@ -5,6 +5,8 @@ use std::time::Duration;
use codex_analytics::CompactionTrigger;
use codex_analytics::HookRunFact;
use codex_analytics::build_track_events_context;
use codex_connectors::AppToolPolicyEvaluator;
use codex_connectors::AppToolPolicyInput;
use codex_core_plugins::executor_plugin_hook_sources;
use codex_hooks::InterruptRequest;
use codex_hooks::PermissionRequestDecision;
@@ -23,8 +25,10 @@ use codex_hooks::UserPromptSubmitOutcome;
use codex_hooks::UserPromptSubmitRequest;
use codex_hooks::hook_execution_mode_label;
use codex_hooks::hook_handler_type_label;
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
use codex_otel::HOOK_RUN_DURATION_METRIC;
use codex_otel::HOOK_RUN_METRIC;
use codex_plugin::ExecutorPluginHookSource;
use codex_protocol::items::FunctionCallOutputItem;
use codex_protocol::items::TurnItem;
use codex_protocol::items::UserMessageItem;
@@ -311,6 +315,39 @@ pub(crate) async fn run_post_tool_use_hooks(
outcome
}
fn executor_hook_sources_for_step(step_context: &StepContext) -> Vec<ExecutorPluginHookSource> {
step_context
.executor_capability_discovery
.as_deref()
.map(|snapshot| {
let app_tool_policy =
AppToolPolicyEvaluator::new(&step_context.mcp.config().config_layer_stack);
executor_plugin_hook_sources(snapshot, |server, tool| {
step_context
.mcp
.tool_info(server, tool)
.filter(|tool_info| {
if server != CODEX_APPS_MCP_SERVER_NAME {
return true;
}
let annotations = tool_info.tool.annotations.as_ref();
app_tool_policy
.policy(AppToolPolicyInput {
connector_id: tool_info.connector_id.as_deref(),
tool_name: &tool_info.tool.name,
tool_title: tool_info.tool.title.as_deref(),
destructive_hint: annotations
.and_then(|annotations| annotations.destructive_hint),
open_world_hint: annotations
.and_then(|annotations| annotations.open_world_hint),
})
.enabled
})
})
})
.unwrap_or_default()
}
fn build_request_metadata(
step_context: Option<&StepContext>,
turn_context: &TurnContext,
@@ -404,11 +441,7 @@ pub(crate) async fn run_turn_stop_hooks(
last_assistant_message,
target,
};
let executor_hook_sources = step_context
.executor_capability_discovery
.as_deref()
.map(executor_plugin_hook_sources)
.unwrap_or_default();
let executor_hook_sources = executor_hook_sources_for_step(step_context);
let hooks = sess.hooks().with_executor_hooks(executor_hook_sources);
emit_hook_started_events(sess, turn_context, hooks.preview_stop(&request)).await;
@@ -461,9 +494,8 @@ pub(crate) async fn run_turn_interrupt_hooks(
// The active turn has already been detached. Reuse only its last executing step's discovery.
let last_known_step_context = turn_state.lock().await.last_known_step_context.clone();
let executor_hook_sources = last_known_step_context
.as_ref()
.and_then(|step_context| step_context.executor_capability_discovery.as_deref())
.map(executor_plugin_hook_sources)
.as_deref()
.map(executor_hook_sources_for_step)
.unwrap_or_default();
let has_executor_hooks = !executor_hook_sources.is_empty();
let hooks = sess.hooks().with_executor_hooks(executor_hook_sources);
codex-rs/core/tests/suite/hooks_executor.rs
+299 -59
View File
@@ -1,7 +1,9 @@
use anyhow::Context;
use anyhow::Result;
use codex_config::McpServerConfig;
use codex_config::test_support::CloudConfigBundleFixture;
use codex_core::EnvironmentConfig;
use codex_core::StartThreadOptions;
use codex_core::TurnInputRequest;
use codex_core::windows_sandbox::WindowsSandboxLevelExt;
use codex_exec_server::CreateDirectoryOptions;
@@ -14,10 +16,17 @@ use codex_protocol::models::PermissionProfileSnapshot;
use codex_protocol::protocol::EnvironmentConfigState;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::SessionSource;
use codex_protocol::protocol::SubAgentSource;
use codex_protocol::protocol::ThreadSettingsOverrides;
use codex_protocol::protocol::ThreadSource;
use codex_protocol::protocol::TurnEnvironmentSelection;
use codex_protocol::protocol::TurnEnvironmentSelections;
use codex_protocol::user_input::UserInput;
use codex_utils_path_uri::PathUri;
use core_test_support::apps_test_server::AppsTestServer;
use core_test_support::apps_test_server::apps_enabled_builder;
use core_test_support::apps_test_server::recorded_apps_tool_calls;
use core_test_support::responses::ResponseMock;
use core_test_support::responses::ev_assistant_message;
use core_test_support::responses::ev_completed;
@@ -30,6 +39,7 @@ use core_test_support::skip_if_no_network;
use core_test_support::skip_if_remote;
use core_test_support::submit_thread_settings;
use core_test_support::test_codex::TestCodex;
use core_test_support::test_codex::TestCodexBuilder;
use core_test_support::test_codex::test_codex;
use core_test_support::wait_for_event;
use core_test_support::wait_for_mcp_server;
@@ -38,12 +48,14 @@ use serde_json::Value;
use serde_json::json;
use std::sync::Arc;
use std::time::Duration;
use test_case::test_case;
use tokio::net::TcpListener;
use tokio::sync::Notify;
use wiremock::Mock;
use wiremock::MockServer;
use wiremock::Request;
use wiremock::ResponseTemplate;
use wiremock::matchers::body_partial_json;
use wiremock::matchers::method;
use wiremock::matchers::path;
@@ -350,6 +362,194 @@ async fn executor_stop_hook_rejects_mismatched_environment() -> Result<()> {
Ok(())
}
#[test_case("Stop", "", "", 1; "enabled")]
#[test_case("SubagentStop", "", "", 1; "subagent_enabled")]
#[test_case(
"Stop",
"[apps.connector_openai_browser]\nenabled = false\n",
"",
0;
"user_disabled"
)]
#[test_case(
"Stop",
"[apps.connector_openai_browser]\nenabled = true\n",
"[apps.connector_openai_browser]\nenabled = false\n",
0;
"managed_disabled"
)]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn executor_browser_and_computer_use_stop_hooks_use_separate_mcp_routes(
hook_event: &'static str,
user_config: &'static str,
requirements: &'static str,
expected_browser_calls: usize,
) -> Result<()> {
skip_if_no_network!(Ok(()));
let apps_server = start_mock_server().await;
let apps = AppsTestServer::mount(&apps_server).await?;
let routing = json!({
"resource_uri": "/connector_openai_browser/browser-link/turn_ended",
"contains_mcp_source": true,
});
let listed_routing = routing.clone();
Mock::given(method("POST"))
.and(path("/api/codex/ps/mcp"))
.and(body_partial_json(json!({ "method": "tools/list" })))
.respond_with(move |request: &Request| {
let request: Value = serde_json::from_slice(&request.body).expect("valid Apps request");
ResponseTemplate::new(200).set_body_json(json!({
"jsonrpc": "2.0",
"id": request["id"],
"result": { "tools": [{
"name": "browser.turn_ended",
"inputSchema": { "type": "object" },
"_meta": {
"connector_id": "connector_openai_browser",
"connector_name": "Browser",
"ui": { "visibility": ["app"] },
"_codex_apps": listed_routing,
},
}] },
}))
})
.with_priority(1)
.mount(&apps_server)
.await;
let plugins = [
(
"browser@openai-curated-remote",
json!({
"name": "browser",
"hooks": { "hooks": {
"Stop": [{ "hooks": [{
"type": "mcp_tool",
"server": "codex_apps",
"tool": "browser.turn_ended",
"input": {},
}] }],
"SubagentStop": [{ "hooks": [{
"type": "mcp_tool",
"server": "codex_apps",
"tool": "browser.turn_ended",
"input": {},
}] }],
} },
}),
),
("computer-use@openai-bundled", computer_use_hook_manifest()),
];
let mut fixture = executor_plugin_hook_fixture(
apps_enabled_builder(apps.chatgpt_base_url)
.with_pre_build_hook(move |home| {
std::fs::write(home.join("config.toml"), user_config)
.expect("write Browser app configuration");
})
.with_cloud_config_bundle(
CloudConfigBundleFixture::loader_with_enterprise_requirement(requirements),
),
&plugins,
vec![completed_turn_response("browser-turn")],
)
.await?;
if hook_event == "SubagentStop" {
let child = fixture
.test
.thread_manager
.start_thread(StartThreadOptions {
session_source: Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
parent_thread_id: fixture.test.session_configured.thread_id,
depth: 1,
agent_path: None,
agent_nickname: None,
agent_role: None,
})),
thread_source: Some(ThreadSource::Subagent),
environments: Some(fixture.test.codex.environment_selections().await),
..StartThreadOptions::new(fixture.test.config.clone())
})
.await?;
fixture.test.codex = child.thread;
fixture.test.session_configured = child.session_configured;
}
fixture.attach().await?;
if hook_event == "SubagentStop" {
// Cached subagent MCP servers start on first use; cleanup must not start them.
fixture
.test
.codex
.call_mcp_tool(
"node_repl",
"js",
Some(json!({ "code": "1 + 1" })),
/*meta*/ None,
)
.await?;
}
fixture.test.submit_text_turn("finish browsing").await?;
fixture.wait_for_hook_call().await?;
let node_calls = fixture.calls().await?;
let expected_node_tools = if hook_event == "SubagentStop" {
vec!["js", "turn_ended"]
} else {
vec!["turn_ended"]
};
assert_eq!(
node_calls
.iter()
.map(|call| call["params"]["name"].as_str().expect("Node tool name"))
.collect::<Vec<_>>(),
expected_node_tools
);
let node_params = &node_calls.last().context("Node cleanup call")?["params"];
assert_eq!(node_params["arguments"]["hook_event_name"], hook_event);
assert!(node_params["_meta"].get("_codex_apps").is_none());
// Executor cleanup runs in the background, so observe a no-call window after Node runs.
let browser_timeout = if expected_browser_calls == 0 {
Duration::from_secs(1)
} else {
Duration::from_secs(10)
};
let calls = tokio::time::timeout(browser_timeout, async {
loop {
let calls = recorded_apps_tool_calls(&apps_server).await;
if !calls.is_empty() {
break calls;
}
tokio::time::sleep(Duration::from_millis(10)).await;
}
})
.await
.unwrap_or_default();
assert_eq!(calls.len(), expected_browser_calls);
if calls.is_empty() {
return Ok(());
}
let browser_params = &calls[0]["params"];
let response_body = fixture.responses.single_request().body_json();
let thread_id = fixture.test.session_configured.thread_id.to_string();
assert_eq!(browser_params["name"], "browser.turn_ended");
assert_eq!(browser_params["arguments"], json!({}));
assert_eq!(browser_params["_meta"]["_codex_apps"], routing);
let turn_metadata = &browser_params["_meta"]["x-codex-turn-metadata"];
assert_eq!(
node_params["_meta"]["x-codex-turn-metadata"],
*turn_metadata
);
assert_eq!(turn_metadata["thread_id"], thread_id);
if hook_event == "SubagentStop" {
assert_eq!(turn_metadata["thread_source"], "subagent");
}
assert_eq!(
turn_metadata["turn_id"],
response_body["client_metadata"]["turn_id"]
);
Ok(())
}
fn completed_turn_response(id: &str) -> ResponseTemplate {
sse_response(sse(vec![
ev_response_created(id),
@@ -368,6 +568,58 @@ async fn executor_stop_hook_fixture() -> Result<ExecutorHookFixture> {
}
async fn executor_hook_fixture(responses: Vec<ResponseTemplate>) -> Result<ExecutorHookFixture> {
executor_plugin_hook_fixture(
test_codex(),
&[("computer-use@openai-bundled", computer_use_hook_manifest())],
responses,
)
.await
}
fn computer_use_hook_manifest() -> Value {
json!({
"name": "computer-use",
// Separate entries must all survive registration in the same environment.
"hooks": [
{ "hooks": { "Interrupt": [{ "hooks": [{
"type": "mcp_tool",
"server": "node_repl",
"tool": "turn_ended",
"input": {
"hook_event_name": "${hook_event_name}",
"session_id": "${session_id}",
"turn_id": "${turn_id}",
},
}] }] } },
{ "hooks": { "Stop": [{ "hooks": [{
"type": "mcp_tool",
"server": "node_repl",
"tool": "turn_ended",
"input": {
"hook_event_name": "${hook_event_name}",
"session_id": "${session_id}",
"turn_id": "${turn_id}",
},
}] }] } },
{ "hooks": { "SubagentStop": [{ "hooks": [{
"type": "mcp_tool",
"server": "node_repl",
"tool": "turn_ended",
"input": {
"hook_event_name": "${hook_event_name}",
"session_id": "${session_id}",
"turn_id": "${turn_id}",
},
}] }] } },
],
})
}
async fn executor_plugin_hook_fixture(
builder: TestCodexBuilder,
plugins: &[(&'static str, Value)],
responses: Vec<ResponseTemplate>,
) -> Result<ExecutorHookFixture> {
let server = start_mock_server().await;
let hook_called = Arc::new(Notify::new());
let hook_called_for_server = Arc::clone(&hook_called);
@@ -383,12 +635,14 @@ async fn executor_hook_fixture(responses: Vec<ResponseTemplate>) -> Result<Execu
"serverInfo": { "name": "node_repl", "version": "1.0.0" },
}),
Some("notifications/initialized") => return ResponseTemplate::new(202),
Some("tools/list") => json!({ "tools": [{
"name": "turn_ended",
"inputSchema": { "type": "object" },
}] }),
Some("tools/list") => json!({ "tools": [
{ "name": "js", "inputSchema": { "type": "object" } },
{ "name": "turn_ended", "inputSchema": { "type": "object" } },
] }),
Some("tools/call") => {
hook_called_for_server.notify_one();
if request["params"]["name"] == "turn_ended" {
hook_called_for_server.notify_one();
}
json!({ "content": [{ "type": "text", "text": "ok" }] })
}
method => panic!("unexpected Node REPL request: {method:?}"),
@@ -403,7 +657,7 @@ async fn executor_hook_fixture(responses: Vec<ResponseTemplate>) -> Result<Execu
.await;
let node_repl_url = format!("{}/node-repl", server.uri());
let mut builder = test_codex().with_config(move |config| {
let mut builder = builder.with_config(move |config| {
config
.features
.enable(Feature::ExecutorCapabilityDiscovery)
@@ -429,53 +683,33 @@ async fn executor_hook_fixture(responses: Vec<ResponseTemplate>) -> Result<Execu
let test = builder.build_with_auto_env(&server).await?;
wait_for_mcp_server(&test.codex, "node_repl").await?;
let plugin_root = test.workspace_path_uri("computer-use")?;
let plugin_directory = plugin_root.join(".codex-plugin")?;
let manifest_path = plugin_directory.join("plugin.json")?;
let mut plugin_roots = Vec::new();
let filesystem = test.fs();
filesystem
.create_directory(
&plugin_directory,
CreateDirectoryOptions {
recursive: true,
follow_symlinks: true,
},
/*sandbox*/ None,
)
.await?;
filesystem
.write_file(
&manifest_path,
serde_json::to_vec(&json!({
"name": "computer-use",
// Separate entries must both survive registration in the same environment.
"hooks": [
{ "hooks": { "Interrupt": [{ "hooks": [{
"type": "mcp_tool",
"server": "node_repl",
"tool": "turn_ended",
"input": {
"hook_event_name": "${hook_event_name}",
"session_id": "${session_id}",
"turn_id": "${turn_id}",
},
}] }] } },
{ "hooks": { "Stop": [{ "hooks": [{
"type": "mcp_tool",
"server": "node_repl",
"tool": "turn_ended",
"input": {
"hook_event_name": "${hook_event_name}",
"session_id": "${session_id}",
"turn_id": "${turn_id}",
},
}] }] } },
],
}))?,
Default::default(),
/*sandbox*/ None,
)
.await?;
for (plugin_id, manifest) in plugins {
let plugin_root =
test.workspace_path_uri(manifest["name"].as_str().context("plugin name")?)?;
let plugin_directory = plugin_root.join(".codex-plugin")?;
let manifest_path = plugin_directory.join("plugin.json")?;
filesystem
.create_directory(
&plugin_directory,
CreateDirectoryOptions {
recursive: true,
follow_symlinks: true,
},
/*sandbox*/ None,
)
.await?;
filesystem
.write_file(
&manifest_path,
serde_json::to_vec(manifest)?,
Default::default(),
/*sandbox*/ None,
)
.await?;
plugin_roots.push((*plugin_id, plugin_root));
}
let responses = mount_response_sequence(&server, responses).await;
@@ -484,6 +718,7 @@ async fn executor_hook_fixture(responses: Vec<ResponseTemplate>) -> Result<Execu
test,
responses,
hook_called,
plugin_roots,
})
}
@@ -492,6 +727,7 @@ struct ExecutorHookFixture {
test: TestCodex,
responses: ResponseMock,
hook_called: Arc<Notify>,
plugin_roots: Vec<(&'static str, PathUri)>,
}
impl ExecutorHookFixture {
@@ -525,13 +761,17 @@ impl ExecutorHookFixture {
exec_policy: None,
mcp_policy: None,
network_policy: None,
selected_capability_roots: vec![SelectedCapabilityRoot {
id: "computer-use@openai-bundled".to_string(),
location: CapabilityRootLocation::Environment {
environment_id: selection.environment_id.clone(),
path: self.test.workspace_path_uri("computer-use")?,
},
}],
selected_capability_roots: self
.plugin_roots
.iter()
.map(|(plugin_id, plugin_root)| SelectedCapabilityRoot {
id: plugin_id.to_string(),
location: CapabilityRootLocation::Environment {
environment_id: selection.environment_id.clone(),
path: plugin_root.clone(),
},
})
.collect(),
},
)
.await?;
codex-rs/hooks/src/engine/mcp_runner.rs
+24 -7
View File
@@ -39,17 +39,34 @@ pub(crate) async fn run_mcp_tool(
let hook_event: Value =
serde_json::from_str(hook_event_json).context("failed to parse hook event input")?;
let input = expand_mcp_argument_template(argument_template, &hook_event)?;
let (environment_id, mut call_metadata) = match &handler.source_path {
HandlerSourcePath::Local(_) => (None, None),
HandlerSourcePath::ExecutorScoped {
environment_id,
mcp_environment_id,
mcp_metadata,
..
} => (
Some(
mcp_environment_id
.as_ref()
.unwrap_or(environment_id)
.clone(),
),
mcp_metadata.as_deref().cloned(),
),
};
if let Some(metadata) = metadata {
call_metadata
.get_or_insert_with(Map::new)
.extend(metadata.clone());
}
executor
.execute(HookMcpCall {
server: server.to_string(),
tool: tool.to_string(),
environment_id: match &handler.source_path {
HandlerSourcePath::Local(_) => None,
HandlerSourcePath::ExecutorScoped { environment_id, .. } => {
Some(environment_id.clone())
}
},
metadata: metadata.cloned(),
environment_id,
metadata: call_metadata,
input,
timeout: Duration::from_secs(handler.timeout_sec),
})
codex-rs/hooks/src/engine/mod.rs
+29 -30
View File
@@ -40,6 +40,8 @@ use codex_protocol::protocol::HookSource;
use codex_protocol::protocol::HookTrustStatus;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
use serde_json::Map;
use serde_json::Value;
use std::collections::HashMap;
use std::collections::HashSet;
use std::sync::Arc;
@@ -70,8 +72,7 @@ pub(crate) struct ConfiguredHandler {
pub(crate) enum HandlerSourcePath {
Local(AbsolutePathBuf),
/// Executor-scoped handlers are currently excluded from user-visible hook reporting
/// (events, summary, telemetry) because the only supported 1p hook is
/// coupled to executor-scoped execution. Their handlers are always executed async.
/// (events, summary, telemetry). Their handlers are always executed async.
///
/// TODO: With CCA, all hooks will be executor-scoped, so user visibility
/// (participation in lifecycle events and summaries) and execution behavior
@@ -79,6 +80,8 @@ pub(crate) enum HandlerSourcePath {
ExecutorScoped {
plugin_id: PluginId,
environment_id: String,
mcp_environment_id: Option<String>,
mcp_metadata: Option<Box<Map<String, Value>>>,
manifest_path: PathUri,
source_relative_path: String,
},
@@ -266,30 +269,14 @@ impl ClaudeHooksEngine {
)
});
// All events share the first environment, even if it has no handler for an event.
let Some(selected_environment_id) = executor_hooks
.first()
.map(|source| source.environment_id.clone())
else {
return;
};
let environment_count = executor_hooks
let mut display_order = self
.handlers
.iter()
.map(|source| &source.environment_id)
.collect::<HashSet<_>>()
.len();
// FIXME: Remove this restriction once executor hooks support multiple environments.
if environment_count > 1 {
tracing::warn!(
executor_environment_count = environment_count,
selected_environment_id = %selected_environment_id,
"multiple executor environments found; only executing hooks in the first"
);
}
.map(|handler| handler.display_order)
.max()
.map_or(0, |display_order| display_order.saturating_add(1));
let mut seen_targets = HashSet::new();
for source in executor_hooks {
if source.environment_id != selected_environment_id {
continue;
}
for (event_name, groups) in source.hooks.into_matcher_groups() {
let Some(handler) = groups.into_iter().flat_map(|group| group.hooks).next() else {
continue;
@@ -305,12 +292,21 @@ impl ClaudeHooksEngine {
unreachable!("allowlisted executor handler must be an MCP tool");
};
let display_order = self
.handlers
.iter()
.map(|handler| handler.display_order)
.max()
.map_or(0, |display_order| display_order.saturating_add(1));
// Bundled plugins can share a cleanup target; run it once per event
// and MCP environment.
let target = (
std::mem::discriminant(&event_name),
source
.mcp_environment_id
.as_ref()
.unwrap_or(&source.environment_id)
.clone(),
server.clone(),
tool.clone(),
);
if !seen_targets.insert(target) {
continue;
}
self.handlers.push(ConfiguredHandler {
event_name,
matcher: None,
@@ -320,6 +316,8 @@ impl ClaudeHooksEngine {
source_path: HandlerSourcePath::ExecutorScoped {
plugin_id: source.plugin_id.clone(),
environment_id: source.environment_id.clone(),
mcp_environment_id: source.mcp_environment_id.clone(),
mcp_metadata: source.mcp_metadata.clone().map(Box::new),
manifest_path: source.manifest_path.clone(),
source_relative_path: source.source_relative_path.clone(),
},
@@ -331,6 +329,7 @@ impl ClaudeHooksEngine {
input,
},
});
display_order = display_order.saturating_add(1);
}
}
}
codex-rs/hooks/src/engine/mod_tests.rs
+48 -14
View File
@@ -2000,6 +2000,8 @@ fn executor_stop_hook_fixture() -> (
let source = ExecutorPluginHookSource {
plugin_id: PluginId::parse("computer-use@openai-bundled").expect("valid plugin ID"),
environment_id: "executor-a".to_string(),
mcp_environment_id: None,
mcp_metadata: None,
plugin_root: "file:///plugins/computer-use"
.parse()
.expect("valid plugin root URI"),
@@ -2036,6 +2038,8 @@ fn executor_stop_hook_fixture() -> (
source_path: HandlerSourcePath::ExecutorScoped {
plugin_id: PluginId::parse("computer-use@openai-bundled").expect("valid plugin ID"),
environment_id: "executor-a".to_string(),
mcp_environment_id: None,
mcp_metadata: None,
manifest_path: "file:///plugins/computer-use/.codex-plugin/plugin.json"
.parse()
.expect("valid plugin manifest URI"),
@@ -2202,19 +2206,22 @@ async fn executor_stop_hooks_run_unless_regular_hooks_block_without_stopping() {
}
#[test]
fn executor_stop_hooks_register_only_the_first_environment_and_handler() {
let (mut engine, _, _, _, mut first_source) = executor_stop_hook_fixture();
let expected_handlers = engine.handlers.clone();
let first_group = &mut first_source.hooks.stop[0];
let mut second_handler = first_group.hooks[0].clone();
let HookHandlerConfig::McpTool { tool, .. } = &mut second_handler else {
panic!("executor Stop handler should be an MCP tool");
};
*tool = "second_turn_ended".to_string();
first_group.hooks.push(second_handler);
fn executor_stop_hooks_register_each_target_environment_once() {
let (mut engine, _, _, _, first_source) = executor_stop_hook_fixture();
let mut expected_handlers = engine.handlers.clone();
let mut duplicate_target = first_source.clone();
duplicate_target.plugin_id = PluginId::parse("chrome@openai-bundled").expect("valid plugin ID");
let mut second_source = first_source.clone();
second_source.environment_id = "executor-b".to_string();
engine.set_executor_hooks(vec![first_source, second_source]);
let mut second_handler = expected_handlers[0].clone();
second_handler.display_order = 1;
let HandlerSourcePath::ExecutorScoped { environment_id, .. } = &mut second_handler.source_path
else {
panic!("executor Stop handler should have an executor source");
};
*environment_id = second_source.environment_id.clone();
expected_handlers.push(second_handler);
engine.set_executor_hooks(vec![first_source, duplicate_target, second_source]);
assert_eq!(engine.handlers, expected_handlers);
}
@@ -2223,6 +2230,7 @@ fn executor_stop_hooks_register_only_the_first_environment_and_handler() {
async fn executor_interrupt_hooks_register_and_run() {
let (mut engine, calls, stop_request, expected_stop_call, mut source) =
executor_stop_hook_fixture();
source.hooks.subagent_stop = source.hooks.stop.clone();
source.hooks.interrupt = source.hooks.stop.clone();
engine.set_executor_hooks(vec![source]);
@@ -2232,7 +2240,11 @@ async fn executor_interrupt_hooks_register_and_run() {
.iter()
.map(|handler| handler.event_name)
.collect::<Vec<_>>(),
vec![HookEventName::Stop, HookEventName::Interrupt]
vec![
HookEventName::SubagentStop,
HookEventName::Stop,
HookEventName::Interrupt,
]
);
assert_eq!(engine.preview_interrupt(), Vec::new());
@@ -2257,20 +2269,42 @@ async fn executor_interrupt_hooks_register_and_run() {
}
#[test]
fn executor_hooks_use_one_environment_for_all_events() {
fn executor_hooks_register_events_from_each_environment() {
let (mut engine, _, _, _, mut first_source) = executor_stop_hook_fixture();
first_source.hooks.interrupt = std::mem::take(&mut first_source.hooks.stop);
engine.set_executor_hooks(vec![first_source.clone()]);
let expected_handlers = engine.handlers.clone();
let mut expected_handlers = engine.handlers.clone();
let mut second_source = first_source.clone();
second_source.environment_id = "executor-b".to_string();
second_source.hooks.stop = std::mem::take(&mut second_source.hooks.interrupt);
let mut second_handler = expected_handlers[0].clone();
second_handler.event_name = HookEventName::Stop;
second_handler.display_order = 1;
let HandlerSourcePath::ExecutorScoped { environment_id, .. } = &mut second_handler.source_path
else {
panic!("executor Stop handler should have an executor source");
};
*environment_id = second_source.environment_id.clone();
expected_handlers.push(second_handler);
engine.set_executor_hooks(vec![first_source, second_source]);
assert_eq!(engine.handlers, expected_handlers);
}
#[tokio::test]
async fn executor_stop_hooks_use_an_admitted_mcp_environment() {
let (mut engine, calls, request, mut expected_call, mut source) = executor_stop_hook_fixture();
source.mcp_environment_id = Some("local".to_string());
engine.set_executor_hooks(vec![source]);
engine.run_stop(request).await;
wait_for_mcp_calls(&calls, /*count*/ 1).await;
expected_call.environment_id = Some("local".to_string());
assert_eq!(*calls.lock().expect("lock MCP calls"), vec![expected_call]);
}
#[tokio::test]
async fn memory_consolidation_stop_preserves_policy_and_executor_cleanup() {
for (source, runs_policy) in [
codex-rs/plugin/Cargo.toml
+1
View File
@@ -18,6 +18,7 @@ codex-protocol = { workspace = true }
codex-utils-absolute-path = { workspace = true }
codex-utils-path-uri = { workspace = true }
codex-utils-plugins = { workspace = true }
serde_json = { workspace = true }
thiserror = { workspace = true }
[dev-dependencies]
codex-rs/plugin/src/lib.rs
+7 -1
View File
@@ -23,6 +23,8 @@ pub use provider::PluginResourceLocator;
pub use provider::ResolvedPlugin;
pub use provider::ResolvedPluginError;
pub use provider::ResolvedPluginLocation;
use serde_json::Map;
use serde_json::Value;
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct AppConnectorId(pub String);
@@ -68,11 +70,15 @@ pub struct PluginHookSource {
pub hooks: HookEventsToml,
}
/// Inline plugin hooks whose paths and MCP target belong to an executor environment.
/// Inline plugin hooks discovered in an executor environment.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct ExecutorPluginHookSource {
pub plugin_id: PluginId,
pub environment_id: String,
/// An admitted MCP target can run outside the plugin's source environment.
pub mcp_environment_id: Option<String>,
/// Trusted MCP routing metadata for this cleanup target.
pub mcp_metadata: Option<Map<String, Value>>,
pub plugin_root: PathUri,
pub manifest_path: PathUri,
pub source_relative_path: String,