From b2f2c98a7cf7ded3337971cb9176022e7384356f Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Sun, 26 Apr 2026 13:10:30 -0700 Subject: [PATCH 01/10] Discover hooks bundled with plugins --- codex-rs/Cargo.lock | 2 + codex-rs/analytics/src/events.rs | 1 + .../schema/json/ServerNotification.json | 1 + .../codex_app_server_protocol.schemas.json | 1 + .../codex_app_server_protocol.v2.schemas.json | 1 + .../json/v2/HookCompletedNotification.json | 1 + .../json/v2/HookStartedNotification.json | 1 + .../schema/typescript/v2/HookSource.ts | 2 +- .../app-server-protocol/src/protocol/v2.rs | 1 + codex-rs/core-plugins/src/loader.rs | 272 +++++++++++++++++- codex-rs/core-plugins/src/manifest.rs | 52 ++++ codex-rs/core/config.schema.json | 6 + codex-rs/core/src/hook_runtime.rs | 1 + codex-rs/core/src/plugins/manager_tests.rs | 6 + codex-rs/core/src/session/session.rs | 12 + codex-rs/features/src/lib.rs | 8 + codex-rs/hooks/Cargo.toml | 1 + codex-rs/hooks/src/engine/command_runner.rs | 4 +- codex-rs/hooks/src/engine/discovery.rs | 78 ++++- codex-rs/hooks/src/engine/dispatcher.rs | 1 + codex-rs/hooks/src/engine/mod.rs | 12 +- codex-rs/hooks/src/engine/mod_tests.rs | 109 +++++++ codex-rs/hooks/src/events/post_tool_use.rs | 1 + codex-rs/hooks/src/events/pre_tool_use.rs | 1 + codex-rs/hooks/src/events/session_start.rs | 1 + codex-rs/hooks/src/events/stop.rs | 1 + .../hooks/src/events/user_prompt_submit.rs | 1 + codex-rs/hooks/src/registry.rs | 5 + codex-rs/plugin/Cargo.toml | 1 + codex-rs/plugin/src/lib.rs | 11 + codex-rs/plugin/src/load_outcome.rs | 19 ++ codex-rs/protocol/src/protocol.rs | 1 + 32 files changed, 596 insertions(+), 19 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 5ff3f462f10a..fdd119aa2ec9 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2744,6 +2744,7 @@ dependencies = [ "anyhow", "chrono", "codex-config", + "codex-plugin", "codex-protocol", "codex-utils-absolute-path", "futures", @@ -3065,6 +3066,7 @@ dependencies = [ name = "codex-plugin" version = "0.0.0" dependencies = [ + "codex-config", "codex-utils-absolute-path", "codex-utils-plugins", "thiserror 2.0.18", diff --git a/codex-rs/analytics/src/events.rs b/codex-rs/analytics/src/events.rs index 98d0e6ff6b99..24ae8e00b9c5 100644 --- a/codex-rs/analytics/src/events.rs +++ b/codex-rs/analytics/src/events.rs @@ -684,6 +684,7 @@ fn analytics_hook_source(source: HookSource) -> &'static str { HookSource::Project => "project", HookSource::Mdm => "mdm", HookSource::SessionFlags => "session_flags", + HookSource::Plugin => "plugin", HookSource::LegacyManagedConfigFile => "legacy_managed_config_file", HookSource::LegacyManagedConfigMdm => "legacy_managed_config_mdm", HookSource::Unknown => "unknown", diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index 629c0b97fa50..df78bc0c3f9a 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -1915,6 +1915,7 @@ "project", "mdm", "sessionFlags", + "plugin", "legacyManagedConfigFile", "legacyManagedConfigMdm", "unknown" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 2fc1be34693b..404f68194438 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -9737,6 +9737,7 @@ "project", "mdm", "sessionFlags", + "plugin", "legacyManagedConfigFile", "legacyManagedConfigMdm", "unknown" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 87e133a07ad7..83f58895664c 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -6367,6 +6367,7 @@ "project", "mdm", "sessionFlags", + "plugin", "legacyManagedConfigFile", "legacyManagedConfigMdm", "unknown" diff --git a/codex-rs/app-server-protocol/schema/json/v2/HookCompletedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/HookCompletedNotification.json index a4d378649b6c..7c03e35543bd 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/HookCompletedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/HookCompletedNotification.json @@ -160,6 +160,7 @@ "project", "mdm", "sessionFlags", + "plugin", "legacyManagedConfigFile", "legacyManagedConfigMdm", "unknown" diff --git a/codex-rs/app-server-protocol/schema/json/v2/HookStartedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/HookStartedNotification.json index ac77d6163f2e..d08300d52645 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/HookStartedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/HookStartedNotification.json @@ -160,6 +160,7 @@ "project", "mdm", "sessionFlags", + "plugin", "legacyManagedConfigFile", "legacyManagedConfigMdm", "unknown" diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/HookSource.ts b/codex-rs/app-server-protocol/schema/typescript/v2/HookSource.ts index 7edf61f9186f..24a06bd13850 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/HookSource.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/HookSource.ts @@ -2,4 +2,4 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -export type HookSource = "system" | "user" | "project" | "mdm" | "sessionFlags" | "legacyManagedConfigFile" | "legacyManagedConfigMdm" | "unknown"; +export type HookSource = "system" | "user" | "project" | "mdm" | "sessionFlags" | "plugin" | "legacyManagedConfigFile" | "legacyManagedConfigMdm" | "unknown"; diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index b7dccc8613bd..c0a76f1b7900 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -469,6 +469,7 @@ v2_enum_from_core!( Project, Mdm, SessionFlags, + Plugin, LegacyManagedConfigFile, LegacyManagedConfigMdm, Unknown, diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 589467199e34..0aa7a115fbde 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -1,4 +1,5 @@ use crate::OPENAI_CURATED_MARKETPLACE_NAME; +use crate::manifest::PluginManifestHooks; use crate::manifest::PluginManifestPaths; use crate::manifest::load_plugin_manifest; use crate::marketplace::MarketplacePluginSource; @@ -7,6 +8,7 @@ use crate::marketplace::load_marketplace; use crate::store::PluginStore; use crate::store::plugin_version_for_source; use codex_config::ConfigLayerStack; +use codex_config::HooksFile; use codex_config::types::McpServerConfig; use codex_config::types::PluginConfig; use codex_core_skills::SkillMetadata; @@ -19,6 +21,7 @@ use codex_exec_server::LOCAL_FS; use codex_plugin::AppConnectorId; use codex_plugin::LoadedPlugin; use codex_plugin::PluginCapabilitySummary; +use codex_plugin::PluginHookSource; use codex_plugin::PluginId; use codex_plugin::PluginIdError; use codex_plugin::PluginLoadOutcome; @@ -26,6 +29,7 @@ use codex_plugin::PluginTelemetryMetadata; use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_plugins::find_plugin_manifest_path; use serde::Deserialize; use serde_json::Map as JsonMap; use serde_json::Value as JsonValue; @@ -39,6 +43,7 @@ use tempfile::TempDir; use tracing::warn; const DEFAULT_SKILLS_DIR_NAME: &str = "skills"; +const DEFAULT_HOOKS_CONFIG_FILE: &str = "hooks/hooks.json"; const DEFAULT_MCP_CONFIG_FILE: &str = ".mcp.json"; const DEFAULT_APP_CONFIG_FILE: &str = ".app.json"; const CONFIG_TOML_FILE: &str = "config.toml"; @@ -477,6 +482,8 @@ async fn load_plugin( has_enabled_skills: false, mcp_servers: HashMap::new(), apps: Vec::new(), + hook_sources: Vec::new(), + hook_load_warnings: Vec::new(), error: None, }; @@ -484,14 +491,14 @@ async fn load_plugin( return loaded_plugin; } - let plugin_root = match plugin_id { - Ok(_) => match active_plugin_root { - Some(plugin_root) => plugin_root, - None => { + let (loaded_plugin_id, plugin_root) = match plugin_id { + Ok(plugin_id) => { + let Some(plugin_root) = active_plugin_root else { loaded_plugin.error = Some("plugin is not installed".to_string()); return loaded_plugin; - } - }, + }; + (plugin_id, plugin_root) + } Err(err) => { loaded_plugin.error = Some(err.to_string()); return loaded_plugin; @@ -545,6 +552,9 @@ async fn load_plugin( } loaded_plugin.mcp_servers = mcp_servers; loaded_plugin.apps = load_plugin_apps(plugin_root.as_path()).await; + let hook_discovery = load_plugin_hooks(&plugin_root, &loaded_plugin_id, manifest_paths); + loaded_plugin.hook_sources = hook_discovery.sources; + loaded_plugin.hook_load_warnings = hook_discovery.warnings; loaded_plugin } @@ -674,6 +684,97 @@ fn default_app_config_paths(plugin_root: &Path) -> Vec { paths } +#[derive(Debug, Default)] +pub struct PluginHookDiscovery { + pub sources: Vec, + pub warnings: Vec, +} + +pub fn load_plugin_hooks( + plugin_root: &AbsolutePathBuf, + plugin_id: &PluginId, + manifest_paths: &PluginManifestPaths, +) -> PluginHookDiscovery { + let mut discovery = PluginHookDiscovery::default(); + match &manifest_paths.hooks { + Some(PluginManifestHooks::Paths(paths)) => { + for path in paths { + append_plugin_hook_file(plugin_root, plugin_id, path, &mut discovery); + } + } + Some(PluginManifestHooks::Inline(hooks_files)) => { + let manifest_path = find_plugin_manifest_path(plugin_root.as_path()) + .and_then(|path| AbsolutePathBuf::try_from(path).ok()) + .unwrap_or_else(|| plugin_root.join(".codex-plugin/plugin.json")); + for (index, hooks_file) in hooks_files.iter().enumerate() { + if hooks_file.hooks.is_empty() { + continue; + } + discovery.sources.push(PluginHookSource { + plugin_id: plugin_id.clone(), + plugin_root: plugin_root.clone(), + source_path: manifest_path.clone(), + source_relative_path: format!("plugin.json#hooks[{index}]"), + hooks: hooks_file.hooks.clone(), + }); + } + } + None => { + let default_path = plugin_root.join(DEFAULT_HOOKS_CONFIG_FILE); + if default_path.as_path().is_file() { + append_plugin_hook_file(plugin_root, plugin_id, &default_path, &mut discovery); + } + } + } + discovery +} + +fn append_plugin_hook_file( + plugin_root: &AbsolutePathBuf, + plugin_id: &PluginId, + path: &AbsolutePathBuf, + discovery: &mut PluginHookDiscovery, +) { + let contents = match fs::read_to_string(path.as_path()) { + Ok(contents) => contents, + Err(err) => { + discovery.warnings.push(format!( + "failed to read plugin hooks config {}: {err}", + path.display() + )); + return; + } + }; + let parsed = match serde_json::from_str::(&contents) { + Ok(parsed) => parsed, + Err(err) => { + discovery.warnings.push(format!( + "failed to parse plugin hooks config {}: {err}", + path.display() + )); + return; + } + }; + if parsed.hooks.is_empty() { + return; + } + + discovery.sources.push(PluginHookSource { + plugin_id: plugin_id.clone(), + plugin_root: plugin_root.clone(), + source_path: path.clone(), + source_relative_path: plugin_relative_path(plugin_root.as_path(), path.as_path()), + hooks: parsed.hooks, + }); +} + +fn plugin_relative_path(plugin_root: &Path, path: &Path) -> String { + path.strip_prefix(plugin_root) + .unwrap_or(path) + .to_string_lossy() + .replace('\\', "/") +} + async fn load_apps_from_paths( plugin_root: &Path, app_config_paths: Vec, @@ -1111,6 +1212,165 @@ mod tests { assert_eq!(curated_plugin_cache_version("0123456"), "0123456"); } + #[test] + fn load_plugin_hooks_discovers_default_hooks_file() { + let tmp = tempfile::tempdir().expect("tempdir"); + let plugin_root = + AbsolutePathBuf::try_from(tmp.path().join("demo-plugin")).expect("plugin root"); + fs::create_dir_all(plugin_root.join(".codex-plugin")).expect("create manifest dir"); + fs::create_dir_all(plugin_root.join("hooks")).expect("create hooks dir"); + fs::write( + plugin_root.join(".codex-plugin/plugin.json"), + r#"{ "name": "demo-plugin" }"#, + ) + .expect("write manifest"); + fs::write( + plugin_root.join("hooks/hooks.json"), + r#"{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [{ "type": "command", "command": "echo default" }] + } + ] + } +}"#, + ) + .expect("write hooks"); + + let manifest = load_plugin_manifest(plugin_root.as_path()).expect("manifest"); + let plugin_id = PluginId::parse("demo-plugin@test-marketplace").expect("plugin id"); + let discovery = load_plugin_hooks(&plugin_root, &plugin_id, &manifest.paths); + + assert_eq!(discovery.warnings, Vec::::new()); + assert_eq!(discovery.sources.len(), 1); + assert_eq!( + discovery.sources[0].plugin_id, + PluginId::parse("demo-plugin@test-marketplace").expect("plugin id") + ); + assert_eq!( + discovery.sources[0].source_relative_path, + "hooks/hooks.json" + ); + assert_eq!(discovery.sources[0].hooks.handler_count(), 1); + } + + #[test] + fn load_plugin_hooks_manifest_paths_replace_default_hooks_file() { + let tmp = tempfile::tempdir().expect("tempdir"); + let plugin_root = + AbsolutePathBuf::try_from(tmp.path().join("demo-plugin")).expect("plugin root"); + fs::create_dir_all(plugin_root.join(".codex-plugin")).expect("create manifest dir"); + fs::create_dir_all(plugin_root.join("hooks")).expect("create hooks dir"); + fs::write( + plugin_root.join(".codex-plugin/plugin.json"), + r#"{ + "name": "demo-plugin", + "hooks": ["./hooks/one.json", "./hooks/two.json"] +}"#, + ) + .expect("write manifest"); + fs::write( + plugin_root.join("hooks/hooks.json"), + r#"{ + "hooks": { + "PreToolUse": [ + { + "hooks": [{ "type": "command", "command": "echo ignored" }] + } + ] + } +}"#, + ) + .expect("write default hooks"); + fs::write( + plugin_root.join("hooks/one.json"), + r#"{ + "hooks": { + "PreToolUse": [ + { + "hooks": [{ "type": "command", "command": "echo one" }] + } + ] + } +}"#, + ) + .expect("write first hooks"); + fs::write( + plugin_root.join("hooks/two.json"), + r#"{ + "hooks": { + "PostToolUse": [ + { + "hooks": [{ "type": "command", "command": "echo two" }] + } + ] + } +}"#, + ) + .expect("write second hooks"); + + let manifest = load_plugin_manifest(plugin_root.as_path()).expect("manifest"); + let plugin_id = PluginId::parse("demo-plugin@test-marketplace").expect("plugin id"); + let discovery = load_plugin_hooks(&plugin_root, &plugin_id, &manifest.paths); + + assert_eq!(discovery.warnings, Vec::::new()); + assert_eq!( + discovery + .sources + .iter() + .map(|source| source.source_relative_path.as_str()) + .collect::>(), + vec!["hooks/one.json", "hooks/two.json"] + ); + assert_eq!( + discovery + .sources + .iter() + .map(|source| source.hooks.handler_count()) + .collect::>(), + vec![1, 1] + ); + } + + #[test] + fn load_plugin_hooks_supports_inline_manifest_hooks() { + let tmp = tempfile::tempdir().expect("tempdir"); + let plugin_root = + AbsolutePathBuf::try_from(tmp.path().join("demo-plugin")).expect("plugin root"); + fs::create_dir_all(plugin_root.join(".codex-plugin")).expect("create manifest dir"); + fs::write( + plugin_root.join(".codex-plugin/plugin.json"), + r#"{ + "name": "demo-plugin", + "hooks": { + "hooks": { + "SessionStart": [ + { + "matcher": "startup", + "hooks": [{ "type": "command", "command": "echo inline" }] + } + ] + } + } +}"#, + ) + .expect("write manifest"); + + let manifest = load_plugin_manifest(plugin_root.as_path()).expect("manifest"); + let plugin_id = PluginId::parse("demo-plugin@test-marketplace").expect("plugin id"); + let discovery = load_plugin_hooks(&plugin_root, &plugin_id, &manifest.paths); + + assert_eq!(discovery.warnings, Vec::::new()); + assert_eq!(discovery.sources.len(), 1); + assert_eq!( + discovery.sources[0].source_relative_path, + "plugin.json#hooks[0]" + ); + assert_eq!(discovery.sources[0].hooks.handler_count(), 1); + } + #[test] fn materialize_git_subdir_uses_sparse_checkout() { let codex_home = tempfile::tempdir().expect("create codex home"); diff --git a/codex-rs/core-plugins/src/manifest.rs b/codex-rs/core-plugins/src/manifest.rs index 5b5366259985..12b738f537f8 100644 --- a/codex-rs/core-plugins/src/manifest.rs +++ b/codex-rs/core-plugins/src/manifest.rs @@ -1,3 +1,4 @@ +use codex_config::HooksFile; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_plugins::find_plugin_manifest_path; use serde::Deserialize; @@ -26,6 +27,8 @@ struct RawPluginManifest { #[serde(default)] apps: Option, #[serde(default)] + hooks: Option, + #[serde(default)] interface: Option, } @@ -43,6 +46,13 @@ pub struct PluginManifestPaths { pub skills: Option, pub mcp_servers: Option, pub apps: Option, + pub hooks: Option, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum PluginManifestHooks { + Paths(Vec), + Inline(Vec), } #[derive(Debug, Clone, Default, PartialEq, Eq)] @@ -114,6 +124,16 @@ enum RawPluginManifestDefaultPromptEntry { Invalid(JsonValue), } +#[derive(Debug, Deserialize)] +#[serde(untagged)] +enum RawPluginManifestHooks { + Path(String), + Paths(Vec), + Inline(HooksFile), + InlineList(Vec), + Invalid(JsonValue), +} + pub fn load_plugin_manifest(plugin_root: &Path) -> Option { let manifest_path = find_plugin_manifest_path(plugin_root)?; let contents = fs::read_to_string(&manifest_path).ok()?; @@ -126,6 +146,7 @@ pub fn load_plugin_manifest(plugin_root: &Path) -> Option { skills, mcp_servers, apps, + hooks, interface, } = manifest; let name = plugin_root @@ -219,6 +240,7 @@ pub fn load_plugin_manifest(plugin_root: &Path) -> Option { mcp_servers.as_deref(), ), apps: resolve_manifest_path(plugin_root, "apps", apps.as_deref()), + hooks: resolve_manifest_hooks(plugin_root, hooks), }, interface, }) @@ -233,6 +255,36 @@ pub fn load_plugin_manifest(plugin_root: &Path) -> Option { } } +fn resolve_manifest_hooks( + plugin_root: &Path, + hooks: Option, +) -> Option { + match hooks? { + RawPluginManifestHooks::Path(path) => { + resolve_manifest_path(plugin_root, "hooks", Some(&path)) + .map(|path| PluginManifestHooks::Paths(vec![path])) + } + RawPluginManifestHooks::Paths(paths) => { + let hooks = paths + .iter() + .filter_map(|path| resolve_manifest_path(plugin_root, "hooks", Some(path))) + .collect::>(); + (!hooks.is_empty()).then_some(PluginManifestHooks::Paths(hooks)) + } + RawPluginManifestHooks::Inline(hooks) => Some(PluginManifestHooks::Inline(vec![hooks])), + RawPluginManifestHooks::InlineList(hooks) => { + (!hooks.is_empty()).then_some(PluginManifestHooks::Inline(hooks)) + } + RawPluginManifestHooks::Invalid(value) => { + tracing::warn!( + "ignoring hooks: expected a string, string array, object, or object array; found {}", + json_value_type(&value) + ); + None + } + } +} + fn resolve_interface_asset_path( plugin_root: &Path, field: &'static str, diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 3fbbfaf6ebcd..fc314b3cdbea 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -463,6 +463,9 @@ "personality": { "type": "boolean" }, + "plugin_hooks": { + "type": "boolean" + }, "plugins": { "type": "boolean" }, @@ -2668,6 +2671,9 @@ "personality": { "type": "boolean" }, + "plugin_hooks": { + "type": "boolean" + }, "plugins": { "type": "boolean" }, diff --git a/codex-rs/core/src/hook_runtime.rs b/codex-rs/core/src/hook_runtime.rs index db47688685bc..b534c63cf422 100644 --- a/codex-rs/core/src/hook_runtime.rs +++ b/codex-rs/core/src/hook_runtime.rs @@ -473,6 +473,7 @@ fn hook_run_metric_tags(run: &HookRunSummary) -> [(&'static str, &'static str); HookSource::Project => "project", HookSource::Mdm => "mdm", HookSource::SessionFlags => "session_flags", + HookSource::Plugin => "plugin", HookSource::LegacyManagedConfigFile => "legacy_managed_config_file", HookSource::LegacyManagedConfigMdm => "legacy_managed_config_mdm", HookSource::Unknown => "unknown", diff --git a/codex-rs/core/src/plugins/manager_tests.rs b/codex-rs/core/src/plugins/manager_tests.rs index c8bbba01b9cd..60b7dd6f4d71 100644 --- a/codex-rs/core/src/plugins/manager_tests.rs +++ b/codex-rs/core/src/plugins/manager_tests.rs @@ -219,6 +219,8 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() { }, )]), apps: vec![AppConnectorId("connector_example".to_string())], + hook_sources: Vec::new(), + hook_load_warnings: Vec::new(), error: None, }] ); @@ -719,6 +721,8 @@ async fn load_plugins_preserves_disabled_plugins_without_effective_contributions has_enabled_skills: false, mcp_servers: HashMap::new(), apps: Vec::new(), + hook_sources: Vec::new(), + hook_load_warnings: Vec::new(), error: None, }] ); @@ -836,6 +840,8 @@ fn capability_index_filters_inactive_and_zero_capability_plugins() { has_enabled_skills: false, mcp_servers: HashMap::new(), apps: Vec::new(), + hook_sources: Vec::new(), + hook_load_warnings: Vec::new(), error: None, }; let summary = |config_name: &str, display_name: &str| PluginCapabilitySummary { diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 9520485a5b7f..409398f7887b 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -690,10 +690,22 @@ impl Session { default_shell.derive_exec_args("", /*use_login_shell*/ false); let hook_shell_program = hook_shell_argv.remove(0); let _ = hook_shell_argv.pop(); + let plugin_hooks_enabled = config.features.enabled(Feature::PluginHooks); + let (plugin_hook_sources, plugin_hook_load_warnings) = if plugin_hooks_enabled { + let plugin_outcome = plugins_manager.plugins_for_config(&config).await; + ( + plugin_outcome.effective_plugin_hook_sources(), + plugin_outcome.effective_plugin_hook_warnings(), + ) + } else { + (Vec::new(), Vec::new()) + }; let hooks = Hooks::new(HooksConfig { legacy_notify_argv: config.notify.clone(), feature_enabled: config.features.enabled(Feature::CodexHooks), config_layer_stack: Some(config.config_layer_stack.clone()), + plugin_hook_sources, + plugin_hook_load_warnings, shell_program: Some(hook_shell_program), shell_args: hook_shell_argv, }); diff --git a/codex-rs/features/src/lib.rs b/codex-rs/features/src/lib.rs index 6a2a2bc71767..3104bdd505db 100644 --- a/codex-rs/features/src/lib.rs +++ b/codex-rs/features/src/lib.rs @@ -158,6 +158,8 @@ pub enum Feature { ToolSuggest, /// Enable plugins. Plugins, + /// Enable plugin-bundled lifecycle hooks. + PluginHooks, /// Allow the in-app browser pane in desktop apps. /// /// Requirements-only gate: this should be set from requirements, not user config. @@ -872,6 +874,12 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::Stable, default_enabled: true, }, + FeatureSpec { + id: Feature::PluginHooks, + key: "plugin_hooks", + stage: Stage::UnderDevelopment, + default_enabled: false, + }, FeatureSpec { id: Feature::InAppBrowser, key: "in_app_browser", diff --git a/codex-rs/hooks/Cargo.toml b/codex-rs/hooks/Cargo.toml index d4d2f9cbc701..028a05542480 100644 --- a/codex-rs/hooks/Cargo.toml +++ b/codex-rs/hooks/Cargo.toml @@ -16,6 +16,7 @@ workspace = true anyhow = { workspace = true } chrono = { workspace = true, features = ["serde"] } codex-config = { workspace = true } +codex-plugin = { workspace = true } codex-protocol = { workspace = true } codex-utils-absolute-path = { workspace = true } futures = { workspace = true, features = ["alloc"] } diff --git a/codex-rs/hooks/src/engine/command_runner.rs b/codex-rs/hooks/src/engine/command_runner.rs index e0e08c3fa5e0..7366d4ec511b 100644 --- a/codex-rs/hooks/src/engine/command_runner.rs +++ b/codex-rs/hooks/src/engine/command_runner.rs @@ -108,12 +108,12 @@ fn build_command(shell: &CommandShell, handler: &ConfiguredHandler) -> Command { }; if shell.program.is_empty() { command.arg(&handler.command); - command } else { command.args(&shell.args); command.arg(&handler.command); - command } + command.envs(&handler.env); + command } fn default_shell_command() -> Command { diff --git a/codex-rs/hooks/src/engine/discovery.rs b/codex-rs/hooks/src/engine/discovery.rs index 4e704e0a0358..ebeda10ab97d 100644 --- a/codex-rs/hooks/src/engine/discovery.rs +++ b/codex-rs/hooks/src/engine/discovery.rs @@ -12,8 +12,10 @@ use codex_config::HooksFile; use codex_config::ManagedHooksRequirementsToml; use codex_config::MatcherGroup; use codex_config::RequirementSource; +use codex_plugin::PluginHookSource; use codex_utils_absolute_path::AbsolutePathBuf; use serde::Deserialize; +use std::collections::HashMap; use super::ConfiguredHandler; use crate::events::common::matcher_pattern_for_event; @@ -25,23 +27,34 @@ pub(crate) struct DiscoveryResult { pub warnings: Vec, } -#[derive(Clone, Copy)] +#[derive(Clone)] struct HookHandlerSource<'a> { path: &'a AbsolutePathBuf, is_managed: bool, source: HookSource, + env: HashMap, } -pub(crate) fn discover_handlers(config_layer_stack: Option<&ConfigLayerStack>) -> DiscoveryResult { +pub(crate) fn discover_handlers( + config_layer_stack: Option<&ConfigLayerStack>, + plugin_hook_sources: Vec, + plugin_hook_load_warnings: Vec, +) -> DiscoveryResult { let Some(config_layer_stack) = config_layer_stack else { - return DiscoveryResult { - handlers: Vec::new(), - warnings: Vec::new(), - }; + let mut handlers = Vec::new(); + let mut warnings = plugin_hook_load_warnings; + let mut display_order = 0_i64; + append_plugin_hook_sources( + &mut handlers, + &mut warnings, + &mut display_order, + plugin_hook_sources, + ); + return DiscoveryResult { handlers, warnings }; }; let mut handlers = Vec::new(); - let mut warnings = Vec::new(); + let mut warnings = plugin_hook_load_warnings; let mut display_order = 0_i64; append_managed_requirement_handlers( @@ -80,6 +93,7 @@ pub(crate) fn discover_handlers(config_layer_stack: Option<&ConfigLayerStack>) - path: &source_path, is_managed: false, source: hook_source, + env: HashMap::new(), }, hook_events, ); @@ -94,12 +108,20 @@ pub(crate) fn discover_handlers(config_layer_stack: Option<&ConfigLayerStack>) - path: &source_path, is_managed: false, source: hook_source, + env: HashMap::new(), }, hook_events, ); } } + append_plugin_hook_sources( + &mut handlers, + &mut warnings, + &mut display_order, + plugin_hook_sources, + ); + DiscoveryResult { handlers, warnings } } @@ -125,11 +147,45 @@ fn append_managed_requirement_handlers( path: &source_path, is_managed: true, source: hook_source_for_requirement_source(managed_hooks.source.as_ref()), + env: HashMap::new(), }, managed_hooks.get().hooks.clone(), ); } +fn append_plugin_hook_sources( + handlers: &mut Vec, + warnings: &mut Vec, + display_order: &mut i64, + plugin_hook_sources: Vec, +) { + // TODO(abhinav): check enabled/trusted state here before plugin hooks become runnable. + for source in plugin_hook_sources { + let PluginHookSource { + plugin_root, + source_path, + hooks, + .. + } = source; + let mut env = HashMap::new(); + let plugin_root_value = plugin_root.display().to_string(); + env.insert("AGENTS_PLUGIN_ROOT".to_string(), plugin_root_value.clone()); + env.insert("CLAUDE_PLUGIN_ROOT".to_string(), plugin_root_value); + append_hook_events( + handlers, + warnings, + display_order, + HookHandlerSource { + path: &source_path, + is_managed: false, + source: HookSource::Plugin, + env, + }, + hooks, + ); + } +} + fn managed_hooks_source_path( managed_hooks: &ManagedHooksRequirementsToml, requirement_source: Option<&RequirementSource>, @@ -278,7 +334,7 @@ fn append_hook_events( handlers, warnings, display_order, - source, + source.clone(), event_name, groups, ); @@ -298,7 +354,7 @@ fn append_matcher_groups( handlers, warnings, display_order, - source, + source.clone(), event_name, matcher_pattern_for_event(event_name, group.matcher.as_deref()), group.hooks, @@ -358,6 +414,7 @@ fn append_group_handlers( source_path: source.path.clone(), source: source.source, display_order: *display_order, + env: source.env.clone(), }); *display_order += 1; } @@ -431,6 +488,7 @@ mod tests { path, is_managed: false, source: hook_source(), + env: std::collections::HashMap::new(), } } @@ -475,6 +533,7 @@ mod tests { source_path: source_path.clone(), source: hook_source(), display_order: 0, + env: std::collections::HashMap::new(), }] ); } @@ -508,6 +567,7 @@ mod tests { source_path: source_path.clone(), source: hook_source(), display_order: 0, + env: std::collections::HashMap::new(), }] ); } diff --git a/codex-rs/hooks/src/engine/dispatcher.rs b/codex-rs/hooks/src/engine/dispatcher.rs index d1cda96541ab..c19b311843b1 100644 --- a/codex-rs/hooks/src/engine/dispatcher.rs +++ b/codex-rs/hooks/src/engine/dispatcher.rs @@ -164,6 +164,7 @@ mod tests { source_path: test_path_buf("/tmp/hooks.json").abs(), source: HookSource::User, display_order, + env: std::collections::HashMap::new(), } } diff --git a/codex-rs/hooks/src/engine/mod.rs b/codex-rs/hooks/src/engine/mod.rs index 3bfb17f6d6f0..89daf501caeb 100644 --- a/codex-rs/hooks/src/engine/mod.rs +++ b/codex-rs/hooks/src/engine/mod.rs @@ -4,7 +4,10 @@ pub(crate) mod dispatcher; pub(crate) mod output_parser; pub(crate) mod schema_loader; +use std::collections::HashMap; + use codex_config::ConfigLayerStack; +use codex_plugin::PluginHookSource; use codex_protocol::protocol::HookRunSummary; use codex_protocol::protocol::HookSource; use codex_utils_absolute_path::AbsolutePathBuf; @@ -39,6 +42,7 @@ pub(crate) struct ConfiguredHandler { pub source_path: AbsolutePathBuf, pub source: HookSource, pub display_order: i64, + pub env: HashMap, } impl ConfiguredHandler { @@ -74,6 +78,8 @@ impl ClaudeHooksEngine { pub(crate) fn new( enabled: bool, config_layer_stack: Option<&ConfigLayerStack>, + plugin_hook_sources: Vec, + plugin_hook_load_warnings: Vec, shell: CommandShell, ) -> Self { if !enabled { @@ -85,7 +91,11 @@ impl ClaudeHooksEngine { } let _ = schema_loader::generated_hook_schemas(); - let discovered = discovery::discover_handlers(config_layer_stack); + let discovered = discovery::discover_handlers( + config_layer_stack, + plugin_hook_sources, + plugin_hook_load_warnings, + ); Self { handlers: discovered.handlers, warnings: discovered.warnings, diff --git a/codex-rs/hooks/src/engine/mod_tests.rs b/codex-rs/hooks/src/engine/mod_tests.rs index 81004aefb42b..c64bb1fbf496 100644 --- a/codex-rs/hooks/src/engine/mod_tests.rs +++ b/codex-rs/hooks/src/engine/mod_tests.rs @@ -15,7 +15,10 @@ use codex_config::ManagedHooksRequirementsToml; use codex_config::MatcherGroup; use codex_config::RequirementSource; use codex_config::TomlValue; +use codex_plugin::PluginHookSource; +use codex_plugin::PluginId; use codex_protocol::ThreadId; +use codex_protocol::protocol::HookSource; use pretty_assertions::assert_eq; use tempfile::tempdir; @@ -105,6 +108,8 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle: let engine = ClaudeHooksEngine::new( /*enabled*/ true, Some(&config_layer_stack), + Vec::new(), + Vec::new(), CommandShell { program: String::new(), args: Vec::new(), @@ -188,6 +193,8 @@ fn requirements_managed_hooks_warn_when_managed_dir_is_missing() { let engine = ClaudeHooksEngine::new( /*enabled*/ true, Some(&config_layer_stack), + Vec::new(), + Vec::new(), CommandShell { program: String::new(), args: Vec::new(), @@ -295,6 +302,8 @@ fn discovers_hooks_from_json_and_toml_in_the_same_layer() { let engine = ClaudeHooksEngine::new( /*enabled*/ true, Some(&config_layer_stack), + Vec::new(), + Vec::new(), CommandShell { program: String::new(), args: Vec::new(), @@ -325,3 +334,103 @@ fn discovers_hooks_from_json_and_toml_in_the_same_layer() { assert_eq!(preview[0].source_path, hooks_json_path); assert_eq!(preview[1].source_path, config_path); } + +#[tokio::test] +async fn plugin_hook_sources_run_with_plugin_env_and_plugin_source() { + let temp = tempdir().expect("create temp dir"); + let plugin_root = + AbsolutePathBuf::try_from(temp.path().join("demo-plugin")).expect("plugin root"); + fs::create_dir_all(plugin_root.join("hooks")).expect("create hooks dir"); + let source_path = plugin_root.join("hooks/hooks.json"); + let log_path = plugin_root.join("env.json"); + let script_path = plugin_root.join("hooks/write_env.py"); + fs::write( + script_path.as_path(), + format!( + r#"import json +import os +from pathlib import Path + +Path(r"{log_path}").write_text(json.dumps({{ + "agents": os.environ.get("AGENTS_PLUGIN_ROOT"), + "claude": os.environ.get("CLAUDE_PLUGIN_ROOT"), +}}), encoding="utf-8") +"#, + log_path = log_path.display(), + ), + ) + .expect("write hook script"); + let plugin_id = PluginId::parse("demo-plugin@test-marketplace").expect("plugin id"); + let plugin_hook_sources = vec![PluginHookSource { + plugin_id, + plugin_root: plugin_root.clone(), + source_path: source_path.clone(), + source_relative_path: "hooks/hooks.json".to_string(), + hooks: HookEventsToml { + pre_tool_use: vec![MatcherGroup { + matcher: Some("Bash".to_string()), + hooks: vec![HookHandlerConfig::Command { + command: format!("python3 {}", script_path.display()), + timeout_sec: Some(5), + r#async: false, + status_message: None, + }], + }], + ..Default::default() + }, + }]; + let engine = ClaudeHooksEngine::new( + /*enabled*/ true, + None, + plugin_hook_sources, + Vec::new(), + CommandShell { + program: String::new(), + args: Vec::new(), + }, + ); + + let preview = engine.preview_pre_tool_use(&PreToolUseRequest { + session_id: ThreadId::new(), + turn_id: "turn-1".to_string(), + cwd: cwd(), + transcript_path: None, + model: "gpt-test".to_string(), + permission_mode: "default".to_string(), + tool_name: "Bash".to_string(), + matcher_aliases: Vec::new(), + tool_use_id: "tool-1".to_string(), + tool_input: serde_json::json!({ "command": "echo hello" }), + }); + assert_eq!(preview.len(), 1); + assert_eq!(preview[0].source, HookSource::Plugin); + assert_eq!(preview[0].source_path, source_path); + + let outcome = engine + .run_pre_tool_use(PreToolUseRequest { + session_id: ThreadId::new(), + turn_id: "turn-1".to_string(), + cwd: cwd(), + transcript_path: None, + model: "gpt-test".to_string(), + permission_mode: "default".to_string(), + tool_name: "Bash".to_string(), + matcher_aliases: Vec::new(), + tool_use_id: "tool-1".to_string(), + tool_input: serde_json::json!({ "command": "echo hello" }), + }) + .await; + + assert_eq!(outcome.hook_events.len(), 1); + assert_eq!(outcome.hook_events[0].run.source, HookSource::Plugin); + let logged: serde_json::Value = + serde_json::from_str(&fs::read_to_string(log_path.as_path()).expect("read env log")) + .expect("parse env log"); + assert_eq!( + logged, + serde_json::json!({ + "agents": plugin_root.display().to_string(), + "claude": plugin_root.display().to_string(), + }) + ); +} diff --git a/codex-rs/hooks/src/events/post_tool_use.rs b/codex-rs/hooks/src/events/post_tool_use.rs index 20cdfd201018..c01cebf78a26 100644 --- a/codex-rs/hooks/src/events/post_tool_use.rs +++ b/codex-rs/hooks/src/events/post_tool_use.rs @@ -551,6 +551,7 @@ mod tests { source_path: test_path_buf("/tmp/hooks.json").abs(), source: codex_protocol::protocol::HookSource::User, display_order: 0, + env: std::collections::HashMap::new(), } } diff --git a/codex-rs/hooks/src/events/pre_tool_use.rs b/codex-rs/hooks/src/events/pre_tool_use.rs index 46012150bb55..3b20c2c2c02c 100644 --- a/codex-rs/hooks/src/events/pre_tool_use.rs +++ b/codex-rs/hooks/src/events/pre_tool_use.rs @@ -542,6 +542,7 @@ mod tests { source_path: test_path_buf("/tmp/hooks.json").abs(), source: codex_protocol::protocol::HookSource::User, display_order: 0, + env: std::collections::HashMap::new(), } } diff --git a/codex-rs/hooks/src/events/session_start.rs b/codex-rs/hooks/src/events/session_start.rs index b1ccdd440a37..54c7f51732b8 100644 --- a/codex-rs/hooks/src/events/session_start.rs +++ b/codex-rs/hooks/src/events/session_start.rs @@ -364,6 +364,7 @@ mod tests { source_path: test_path_buf("/tmp/hooks.json").abs(), source: codex_protocol::protocol::HookSource::User, display_order: 0, + env: std::collections::HashMap::new(), } } diff --git a/codex-rs/hooks/src/events/stop.rs b/codex-rs/hooks/src/events/stop.rs index f376dccd2c07..392f15eee24f 100644 --- a/codex-rs/hooks/src/events/stop.rs +++ b/codex-rs/hooks/src/events/stop.rs @@ -531,6 +531,7 @@ mod tests { source_path: test_path_buf("/tmp/hooks.json").abs(), source: codex_protocol::protocol::HookSource::User, display_order: 0, + env: std::collections::HashMap::new(), } } diff --git a/codex-rs/hooks/src/events/user_prompt_submit.rs b/codex-rs/hooks/src/events/user_prompt_submit.rs index 2acd4808b8b4..8aaf3ad608e0 100644 --- a/codex-rs/hooks/src/events/user_prompt_submit.rs +++ b/codex-rs/hooks/src/events/user_prompt_submit.rs @@ -422,6 +422,7 @@ mod tests { source_path: test_path_buf("/tmp/hooks.json").abs(), source: codex_protocol::protocol::HookSource::User, display_order: 0, + env: std::collections::HashMap::new(), } } diff --git a/codex-rs/hooks/src/registry.rs b/codex-rs/hooks/src/registry.rs index 6f4e56b1bfaf..7dd93213a112 100644 --- a/codex-rs/hooks/src/registry.rs +++ b/codex-rs/hooks/src/registry.rs @@ -1,4 +1,5 @@ use codex_config::ConfigLayerStack; +use codex_plugin::PluginHookSource; use tokio::process::Command; use crate::engine::ClaudeHooksEngine; @@ -25,6 +26,8 @@ pub struct HooksConfig { pub legacy_notify_argv: Option>, pub feature_enabled: bool, pub config_layer_stack: Option, + pub plugin_hook_sources: Vec, + pub plugin_hook_load_warnings: Vec, pub shell_program: Option, pub shell_args: Vec, } @@ -53,6 +56,8 @@ impl Hooks { let engine = ClaudeHooksEngine::new( config.feature_enabled, config.config_layer_stack.as_ref(), + config.plugin_hook_sources, + config.plugin_hook_load_warnings, CommandShell { program: config.shell_program.unwrap_or_default(), args: config.shell_args, diff --git a/codex-rs/plugin/Cargo.toml b/codex-rs/plugin/Cargo.toml index b72d74682c63..a431a543d43e 100644 --- a/codex-rs/plugin/Cargo.toml +++ b/codex-rs/plugin/Cargo.toml @@ -13,6 +13,7 @@ path = "src/lib.rs" workspace = true [dependencies] +codex-config = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-plugins = { workspace = true } thiserror = { workspace = true } diff --git a/codex-rs/plugin/src/lib.rs b/codex-rs/plugin/src/lib.rs index b984b9d2fcd0..31ecf5601522 100644 --- a/codex-rs/plugin/src/lib.rs +++ b/codex-rs/plugin/src/lib.rs @@ -6,6 +6,8 @@ pub use codex_utils_plugins::plugin_namespace_for_skill_path; mod load_outcome; mod plugin_id; +use codex_config::HookEventsToml; +use codex_utils_absolute_path::AbsolutePathBuf; pub use load_outcome::EffectiveSkillRoots; pub use load_outcome::LoadedPlugin; pub use load_outcome::PluginLoadOutcome; @@ -27,6 +29,15 @@ pub struct PluginCapabilitySummary { pub app_connector_ids: Vec, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PluginHookSource { + pub plugin_id: PluginId, + pub plugin_root: AbsolutePathBuf, + pub source_path: AbsolutePathBuf, + pub source_relative_path: String, + pub hooks: HookEventsToml, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct PluginTelemetryMetadata { pub plugin_id: PluginId, diff --git a/codex-rs/plugin/src/load_outcome.rs b/codex-rs/plugin/src/load_outcome.rs index 062886be5c1d..0865b9020fcd 100644 --- a/codex-rs/plugin/src/load_outcome.rs +++ b/codex-rs/plugin/src/load_outcome.rs @@ -5,6 +5,7 @@ use codex_utils_absolute_path::AbsolutePathBuf; use crate::AppConnectorId; use crate::PluginCapabilitySummary; +use crate::PluginHookSource; const MAX_CAPABILITY_SUMMARY_DESCRIPTION_LEN: usize = 1024; @@ -21,6 +22,8 @@ pub struct LoadedPlugin { pub has_enabled_skills: bool, pub mcp_servers: HashMap, pub apps: Vec, + pub hook_sources: Vec, + pub hook_load_warnings: Vec, pub error: Option, } @@ -140,6 +143,22 @@ impl PluginLoadOutcome { apps } + pub fn effective_plugin_hook_sources(&self) -> Vec { + self.plugins + .iter() + .filter(|plugin| plugin.is_active()) + .flat_map(|plugin| plugin.hook_sources.iter().cloned()) + .collect() + } + + pub fn effective_plugin_hook_warnings(&self) -> Vec { + self.plugins + .iter() + .filter(|plugin| plugin.is_active()) + .flat_map(|plugin| plugin.hook_load_warnings.iter().cloned()) + .collect() + } + pub fn capability_summaries(&self) -> &[PluginCapabilitySummary] { &self.capability_summaries } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index c3254e92a753..c80b15be986f 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1645,6 +1645,7 @@ pub enum HookSource { Project, Mdm, SessionFlags, + Plugin, LegacyManagedConfigFile, LegacyManagedConfigMdm, #[default] From d2097f980273f26cf0fcf93f0b2405e46ff6181d Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Sun, 26 Apr 2026 13:32:55 -0700 Subject: [PATCH 02/10] Remove plugin hook load warnings from discovery --- codex-rs/core-plugins/src/loader.rs | 83 +++++++++------------- codex-rs/core/src/plugins/manager_tests.rs | 3 - codex-rs/core/src/session/session.rs | 10 +-- codex-rs/hooks/src/engine/discovery.rs | 8 +-- codex-rs/hooks/src/engine/mod.rs | 7 +- codex-rs/hooks/src/engine/mod_tests.rs | 8 +-- codex-rs/hooks/src/registry.rs | 2 - codex-rs/plugin/src/load_outcome.rs | 9 --- 8 files changed, 44 insertions(+), 86 deletions(-) diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 0aa7a115fbde..df2c6492b65b 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -483,7 +483,6 @@ async fn load_plugin( mcp_servers: HashMap::new(), apps: Vec::new(), hook_sources: Vec::new(), - hook_load_warnings: Vec::new(), error: None, }; @@ -552,9 +551,7 @@ async fn load_plugin( } loaded_plugin.mcp_servers = mcp_servers; loaded_plugin.apps = load_plugin_apps(plugin_root.as_path()).await; - let hook_discovery = load_plugin_hooks(&plugin_root, &loaded_plugin_id, manifest_paths); - loaded_plugin.hook_sources = hook_discovery.sources; - loaded_plugin.hook_load_warnings = hook_discovery.warnings; + loaded_plugin.hook_sources = load_plugin_hooks(&plugin_root, &loaded_plugin_id, manifest_paths); loaded_plugin } @@ -684,22 +681,19 @@ fn default_app_config_paths(plugin_root: &Path) -> Vec { paths } -#[derive(Debug, Default)] -pub struct PluginHookDiscovery { - pub sources: Vec, - pub warnings: Vec, -} - +// Discover plugin-bundled hooks from manifest `hooks` entries when present +// (path, paths, inline object, or inline objects), otherwise from the default +// `hooks/hooks.json` file. pub fn load_plugin_hooks( plugin_root: &AbsolutePathBuf, plugin_id: &PluginId, manifest_paths: &PluginManifestPaths, -) -> PluginHookDiscovery { - let mut discovery = PluginHookDiscovery::default(); +) -> Vec { + let mut sources = Vec::new(); match &manifest_paths.hooks { Some(PluginManifestHooks::Paths(paths)) => { for path in paths { - append_plugin_hook_file(plugin_root, plugin_id, path, &mut discovery); + append_plugin_hook_file(plugin_root, plugin_id, path, &mut sources); } } Some(PluginManifestHooks::Inline(hooks_files)) => { @@ -710,7 +704,7 @@ pub fn load_plugin_hooks( if hooks_file.hooks.is_empty() { continue; } - discovery.sources.push(PluginHookSource { + sources.push(PluginHookSource { plugin_id: plugin_id.clone(), plugin_root: plugin_root.clone(), source_path: manifest_path.clone(), @@ -722,36 +716,38 @@ pub fn load_plugin_hooks( None => { let default_path = plugin_root.join(DEFAULT_HOOKS_CONFIG_FILE); if default_path.as_path().is_file() { - append_plugin_hook_file(plugin_root, plugin_id, &default_path, &mut discovery); + append_plugin_hook_file(plugin_root, plugin_id, &default_path, &mut sources); } } } - discovery + sources } +// Load one resolved plugin hook file and keep source metadata with its parsed +// hook events so runtime discovery can report plugin-originated hook runs. fn append_plugin_hook_file( plugin_root: &AbsolutePathBuf, plugin_id: &PluginId, path: &AbsolutePathBuf, - discovery: &mut PluginHookDiscovery, + sources: &mut Vec, ) { let contents = match fs::read_to_string(path.as_path()) { Ok(contents) => contents, Err(err) => { - discovery.warnings.push(format!( - "failed to read plugin hooks config {}: {err}", - path.display() - )); + warn!( + path = %path.display(), + "failed to read plugin hooks config: {err}" + ); return; } }; let parsed = match serde_json::from_str::(&contents) { Ok(parsed) => parsed, Err(err) => { - discovery.warnings.push(format!( - "failed to parse plugin hooks config {}: {err}", - path.display() - )); + warn!( + path = %path.display(), + "failed to parse plugin hooks config: {err}" + ); return; } }; @@ -759,7 +755,7 @@ fn append_plugin_hook_file( return; } - discovery.sources.push(PluginHookSource { + sources.push(PluginHookSource { plugin_id: plugin_id.clone(), plugin_root: plugin_root.clone(), source_path: path.clone(), @@ -1241,19 +1237,15 @@ mod tests { let manifest = load_plugin_manifest(plugin_root.as_path()).expect("manifest"); let plugin_id = PluginId::parse("demo-plugin@test-marketplace").expect("plugin id"); - let discovery = load_plugin_hooks(&plugin_root, &plugin_id, &manifest.paths); + let sources = load_plugin_hooks(&plugin_root, &plugin_id, &manifest.paths); - assert_eq!(discovery.warnings, Vec::::new()); - assert_eq!(discovery.sources.len(), 1); + assert_eq!(sources.len(), 1); assert_eq!( - discovery.sources[0].plugin_id, + sources[0].plugin_id, PluginId::parse("demo-plugin@test-marketplace").expect("plugin id") ); - assert_eq!( - discovery.sources[0].source_relative_path, - "hooks/hooks.json" - ); - assert_eq!(discovery.sources[0].hooks.handler_count(), 1); + assert_eq!(sources[0].source_relative_path, "hooks/hooks.json"); + assert_eq!(sources[0].hooks.handler_count(), 1); } #[test] @@ -1313,20 +1305,17 @@ mod tests { let manifest = load_plugin_manifest(plugin_root.as_path()).expect("manifest"); let plugin_id = PluginId::parse("demo-plugin@test-marketplace").expect("plugin id"); - let discovery = load_plugin_hooks(&plugin_root, &plugin_id, &manifest.paths); + let sources = load_plugin_hooks(&plugin_root, &plugin_id, &manifest.paths); - assert_eq!(discovery.warnings, Vec::::new()); assert_eq!( - discovery - .sources + sources .iter() .map(|source| source.source_relative_path.as_str()) .collect::>(), vec!["hooks/one.json", "hooks/two.json"] ); assert_eq!( - discovery - .sources + sources .iter() .map(|source| source.hooks.handler_count()) .collect::>(), @@ -1360,15 +1349,11 @@ mod tests { let manifest = load_plugin_manifest(plugin_root.as_path()).expect("manifest"); let plugin_id = PluginId::parse("demo-plugin@test-marketplace").expect("plugin id"); - let discovery = load_plugin_hooks(&plugin_root, &plugin_id, &manifest.paths); + let sources = load_plugin_hooks(&plugin_root, &plugin_id, &manifest.paths); - assert_eq!(discovery.warnings, Vec::::new()); - assert_eq!(discovery.sources.len(), 1); - assert_eq!( - discovery.sources[0].source_relative_path, - "plugin.json#hooks[0]" - ); - assert_eq!(discovery.sources[0].hooks.handler_count(), 1); + assert_eq!(sources.len(), 1); + assert_eq!(sources[0].source_relative_path, "plugin.json#hooks[0]"); + assert_eq!(sources[0].hooks.handler_count(), 1); } #[test] diff --git a/codex-rs/core/src/plugins/manager_tests.rs b/codex-rs/core/src/plugins/manager_tests.rs index 60b7dd6f4d71..4e794506c010 100644 --- a/codex-rs/core/src/plugins/manager_tests.rs +++ b/codex-rs/core/src/plugins/manager_tests.rs @@ -220,7 +220,6 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() { )]), apps: vec![AppConnectorId("connector_example".to_string())], hook_sources: Vec::new(), - hook_load_warnings: Vec::new(), error: None, }] ); @@ -722,7 +721,6 @@ async fn load_plugins_preserves_disabled_plugins_without_effective_contributions mcp_servers: HashMap::new(), apps: Vec::new(), hook_sources: Vec::new(), - hook_load_warnings: Vec::new(), error: None, }] ); @@ -841,7 +839,6 @@ fn capability_index_filters_inactive_and_zero_capability_plugins() { mcp_servers: HashMap::new(), apps: Vec::new(), hook_sources: Vec::new(), - hook_load_warnings: Vec::new(), error: None, }; let summary = |config_name: &str, display_name: &str| PluginCapabilitySummary { diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 409398f7887b..e49e25ac6e8e 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -691,21 +691,17 @@ impl Session { let hook_shell_program = hook_shell_argv.remove(0); let _ = hook_shell_argv.pop(); let plugin_hooks_enabled = config.features.enabled(Feature::PluginHooks); - let (plugin_hook_sources, plugin_hook_load_warnings) = if plugin_hooks_enabled { + let plugin_hook_sources = if plugin_hooks_enabled { let plugin_outcome = plugins_manager.plugins_for_config(&config).await; - ( - plugin_outcome.effective_plugin_hook_sources(), - plugin_outcome.effective_plugin_hook_warnings(), - ) + plugin_outcome.effective_plugin_hook_sources() } else { - (Vec::new(), Vec::new()) + Vec::new() }; let hooks = Hooks::new(HooksConfig { legacy_notify_argv: config.notify.clone(), feature_enabled: config.features.enabled(Feature::CodexHooks), config_layer_stack: Some(config.config_layer_stack.clone()), plugin_hook_sources, - plugin_hook_load_warnings, shell_program: Some(hook_shell_program), shell_args: hook_shell_argv, }); diff --git a/codex-rs/hooks/src/engine/discovery.rs b/codex-rs/hooks/src/engine/discovery.rs index ebeda10ab97d..ce42908c097e 100644 --- a/codex-rs/hooks/src/engine/discovery.rs +++ b/codex-rs/hooks/src/engine/discovery.rs @@ -38,11 +38,10 @@ struct HookHandlerSource<'a> { pub(crate) fn discover_handlers( config_layer_stack: Option<&ConfigLayerStack>, plugin_hook_sources: Vec, - plugin_hook_load_warnings: Vec, ) -> DiscoveryResult { let Some(config_layer_stack) = config_layer_stack else { let mut handlers = Vec::new(); - let mut warnings = plugin_hook_load_warnings; + let mut warnings = Vec::new(); let mut display_order = 0_i64; append_plugin_hook_sources( &mut handlers, @@ -54,7 +53,7 @@ pub(crate) fn discover_handlers( }; let mut handlers = Vec::new(); - let mut warnings = plugin_hook_load_warnings; + let mut warnings = Vec::new(); let mut display_order = 0_i64; append_managed_requirement_handlers( @@ -169,7 +168,8 @@ fn append_plugin_hook_sources( } = source; let mut env = HashMap::new(); let plugin_root_value = plugin_root.display().to_string(); - env.insert("AGENTS_PLUGIN_ROOT".to_string(), plugin_root_value.clone()); + env.insert("PLUGIN_ROOT".to_string(), plugin_root_value.clone()); + // For OOTB compat with existing plugins that use this env var. env.insert("CLAUDE_PLUGIN_ROOT".to_string(), plugin_root_value); append_hook_events( handlers, diff --git a/codex-rs/hooks/src/engine/mod.rs b/codex-rs/hooks/src/engine/mod.rs index 89daf501caeb..5c121136f7a0 100644 --- a/codex-rs/hooks/src/engine/mod.rs +++ b/codex-rs/hooks/src/engine/mod.rs @@ -79,7 +79,6 @@ impl ClaudeHooksEngine { enabled: bool, config_layer_stack: Option<&ConfigLayerStack>, plugin_hook_sources: Vec, - plugin_hook_load_warnings: Vec, shell: CommandShell, ) -> Self { if !enabled { @@ -91,11 +90,7 @@ impl ClaudeHooksEngine { } let _ = schema_loader::generated_hook_schemas(); - let discovered = discovery::discover_handlers( - config_layer_stack, - plugin_hook_sources, - plugin_hook_load_warnings, - ); + let discovered = discovery::discover_handlers(config_layer_stack, plugin_hook_sources); Self { handlers: discovered.handlers, warnings: discovered.warnings, diff --git a/codex-rs/hooks/src/engine/mod_tests.rs b/codex-rs/hooks/src/engine/mod_tests.rs index c64bb1fbf496..4b382df4a634 100644 --- a/codex-rs/hooks/src/engine/mod_tests.rs +++ b/codex-rs/hooks/src/engine/mod_tests.rs @@ -109,7 +109,6 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle: /*enabled*/ true, Some(&config_layer_stack), Vec::new(), - Vec::new(), CommandShell { program: String::new(), args: Vec::new(), @@ -194,7 +193,6 @@ fn requirements_managed_hooks_warn_when_managed_dir_is_missing() { /*enabled*/ true, Some(&config_layer_stack), Vec::new(), - Vec::new(), CommandShell { program: String::new(), args: Vec::new(), @@ -303,7 +301,6 @@ fn discovers_hooks_from_json_and_toml_in_the_same_layer() { /*enabled*/ true, Some(&config_layer_stack), Vec::new(), - Vec::new(), CommandShell { program: String::new(), args: Vec::new(), @@ -352,7 +349,7 @@ import os from pathlib import Path Path(r"{log_path}").write_text(json.dumps({{ - "agents": os.environ.get("AGENTS_PLUGIN_ROOT"), + "plugin": os.environ.get("PLUGIN_ROOT"), "claude": os.environ.get("CLAUDE_PLUGIN_ROOT"), }}), encoding="utf-8") "#, @@ -383,7 +380,6 @@ Path(r"{log_path}").write_text(json.dumps({{ /*enabled*/ true, None, plugin_hook_sources, - Vec::new(), CommandShell { program: String::new(), args: Vec::new(), @@ -429,7 +425,7 @@ Path(r"{log_path}").write_text(json.dumps({{ assert_eq!( logged, serde_json::json!({ - "agents": plugin_root.display().to_string(), + "plugin": plugin_root.display().to_string(), "claude": plugin_root.display().to_string(), }) ); diff --git a/codex-rs/hooks/src/registry.rs b/codex-rs/hooks/src/registry.rs index 7dd93213a112..4509a8a63187 100644 --- a/codex-rs/hooks/src/registry.rs +++ b/codex-rs/hooks/src/registry.rs @@ -27,7 +27,6 @@ pub struct HooksConfig { pub feature_enabled: bool, pub config_layer_stack: Option, pub plugin_hook_sources: Vec, - pub plugin_hook_load_warnings: Vec, pub shell_program: Option, pub shell_args: Vec, } @@ -57,7 +56,6 @@ impl Hooks { config.feature_enabled, config.config_layer_stack.as_ref(), config.plugin_hook_sources, - config.plugin_hook_load_warnings, CommandShell { program: config.shell_program.unwrap_or_default(), args: config.shell_args, diff --git a/codex-rs/plugin/src/load_outcome.rs b/codex-rs/plugin/src/load_outcome.rs index 0865b9020fcd..40dba4ae2ae1 100644 --- a/codex-rs/plugin/src/load_outcome.rs +++ b/codex-rs/plugin/src/load_outcome.rs @@ -23,7 +23,6 @@ pub struct LoadedPlugin { pub mcp_servers: HashMap, pub apps: Vec, pub hook_sources: Vec, - pub hook_load_warnings: Vec, pub error: Option, } @@ -151,14 +150,6 @@ impl PluginLoadOutcome { .collect() } - pub fn effective_plugin_hook_warnings(&self) -> Vec { - self.plugins - .iter() - .filter(|plugin| plugin.is_active()) - .flat_map(|plugin| plugin.hook_load_warnings.iter().cloned()) - .collect() - } - pub fn capability_summaries(&self) -> &[PluginCapabilitySummary] { &self.capability_summaries } From 6655134d4954f954d21b9aa43ba919043f9348e8 Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Sun, 26 Apr 2026 13:43:54 -0700 Subject: [PATCH 03/10] Refactor plugin hook file loading --- codex-rs/core-plugins/src/loader.rs | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index df2c6492b65b..306c71960ad0 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -693,7 +693,9 @@ pub fn load_plugin_hooks( match &manifest_paths.hooks { Some(PluginManifestHooks::Paths(paths)) => { for path in paths { - append_plugin_hook_file(plugin_root, plugin_id, path, &mut sources); + if let Some(source) = load_plugin_hook_file(plugin_root, plugin_id, path) { + sources.push(source); + } } } Some(PluginManifestHooks::Inline(hooks_files)) => { @@ -715,8 +717,10 @@ pub fn load_plugin_hooks( } None => { let default_path = plugin_root.join(DEFAULT_HOOKS_CONFIG_FILE); - if default_path.as_path().is_file() { - append_plugin_hook_file(plugin_root, plugin_id, &default_path, &mut sources); + if default_path.as_path().is_file() + && let Some(source) = load_plugin_hook_file(plugin_root, plugin_id, &default_path) + { + sources.push(source); } } } @@ -725,12 +729,11 @@ pub fn load_plugin_hooks( // Load one resolved plugin hook file and keep source metadata with its parsed // hook events so runtime discovery can report plugin-originated hook runs. -fn append_plugin_hook_file( +fn load_plugin_hook_file( plugin_root: &AbsolutePathBuf, plugin_id: &PluginId, path: &AbsolutePathBuf, - sources: &mut Vec, -) { +) -> Option { let contents = match fs::read_to_string(path.as_path()) { Ok(contents) => contents, Err(err) => { @@ -738,7 +741,7 @@ fn append_plugin_hook_file( path = %path.display(), "failed to read plugin hooks config: {err}" ); - return; + return None; } }; let parsed = match serde_json::from_str::(&contents) { @@ -748,20 +751,20 @@ fn append_plugin_hook_file( path = %path.display(), "failed to parse plugin hooks config: {err}" ); - return; + return None; } }; if parsed.hooks.is_empty() { - return; + return None; } - sources.push(PluginHookSource { + Some(PluginHookSource { plugin_id: plugin_id.clone(), plugin_root: plugin_root.clone(), source_path: path.clone(), source_relative_path: plugin_relative_path(plugin_root.as_path(), path.as_path()), hooks: parsed.hooks, - }); + }) } fn plugin_relative_path(plugin_root: &Path, path: &Path) -> String { From 5aed78ae4b8d9e1babd521dd595a5681c1b76ed7 Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Sun, 26 Apr 2026 13:57:39 -0700 Subject: [PATCH 04/10] Fix plugin hook test argument comment --- codex-rs/hooks/src/engine/mod_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codex-rs/hooks/src/engine/mod_tests.rs b/codex-rs/hooks/src/engine/mod_tests.rs index 4b382df4a634..245c31ba5fd0 100644 --- a/codex-rs/hooks/src/engine/mod_tests.rs +++ b/codex-rs/hooks/src/engine/mod_tests.rs @@ -378,7 +378,7 @@ Path(r"{log_path}").write_text(json.dumps({{ }]; let engine = ClaudeHooksEngine::new( /*enabled*/ true, - None, + /*config_layer_stack*/ None, plugin_hook_sources, CommandShell { program: String::new(), From 803c16f2d34827f5853760a4eddc5e008e878bbc Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Sun, 26 Apr 2026 14:12:55 -0700 Subject: [PATCH 05/10] Move plugin loader tests out of implementation --- codex-rs/core-plugins/src/loader.rs | 296 +----------------- codex-rs/core-plugins/src/loader_tests.rs | 347 ++++++++++++++++++++++ 2 files changed, 349 insertions(+), 294 deletions(-) create mode 100644 codex-rs/core-plugins/src/loader_tests.rs diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 306c71960ad0..b2be23c45a45 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -1114,297 +1114,5 @@ fn run_git(args: &[&str], cwd: Option<&Path>) -> Result<(), String> { } #[cfg(test)] -mod tests { - use super::*; - use pretty_assertions::assert_eq; - - #[test] - fn plugin_mcp_file_supports_mcp_servers_object_format() { - let parsed = serde_json::from_str::( - r#"{ - "mcpServers": { - "sample": { - "command": "sample-mcp" - } - } -}"#, - ) - .expect("parse wrapped plugin mcp config") - .into_mcp_servers(); - - assert_eq!( - parsed, - HashMap::from([( - "sample".to_string(), - serde_json::json!({ - "command": "sample-mcp" - }), - )]) - ); - } - - #[test] - fn plugin_mcp_file_supports_mcp_servers_object_format_with_metadata() { - let parsed = serde_json::from_str::( - r#"{ - "$schema": "https://example.com/plugin-mcp.schema.json", - "mcpServers": { - "sample": { - "command": "sample-mcp" - } - } -}"#, - ) - .expect("parse plugin mcp config with metadata") - .into_mcp_servers(); - - assert_eq!( - parsed, - HashMap::from([( - "sample".to_string(), - serde_json::json!({ - "command": "sample-mcp" - }), - )]) - ); - } - - #[test] - fn plugin_mcp_file_supports_top_level_server_map_format() { - let parsed = serde_json::from_str::( - r#"{ - "linear": { - "type": "http", - "url": "https://mcp.linear.app/mcp" - } -}"#, - ) - .expect("parse flat plugin mcp config") - .into_mcp_servers(); - - assert_eq!( - parsed, - HashMap::from([( - "linear".to_string(), - serde_json::json!({ - "type": "http", - "url": "https://mcp.linear.app/mcp" - }), - )]) - ); - } - - #[test] - fn curated_plugin_cache_version_shortens_full_git_sha() { - assert_eq!( - curated_plugin_cache_version("0123456789abcdef0123456789abcdef01234567"), - "01234567" - ); - } - - #[test] - fn curated_plugin_cache_version_preserves_non_git_sha_versions() { - assert_eq!( - curated_plugin_cache_version("export-backup"), - "export-backup" - ); - assert_eq!(curated_plugin_cache_version("0123456"), "0123456"); - } - - #[test] - fn load_plugin_hooks_discovers_default_hooks_file() { - let tmp = tempfile::tempdir().expect("tempdir"); - let plugin_root = - AbsolutePathBuf::try_from(tmp.path().join("demo-plugin")).expect("plugin root"); - fs::create_dir_all(plugin_root.join(".codex-plugin")).expect("create manifest dir"); - fs::create_dir_all(plugin_root.join("hooks")).expect("create hooks dir"); - fs::write( - plugin_root.join(".codex-plugin/plugin.json"), - r#"{ "name": "demo-plugin" }"#, - ) - .expect("write manifest"); - fs::write( - plugin_root.join("hooks/hooks.json"), - r#"{ - "hooks": { - "PreToolUse": [ - { - "matcher": "Bash", - "hooks": [{ "type": "command", "command": "echo default" }] - } - ] - } -}"#, - ) - .expect("write hooks"); - - let manifest = load_plugin_manifest(plugin_root.as_path()).expect("manifest"); - let plugin_id = PluginId::parse("demo-plugin@test-marketplace").expect("plugin id"); - let sources = load_plugin_hooks(&plugin_root, &plugin_id, &manifest.paths); - - assert_eq!(sources.len(), 1); - assert_eq!( - sources[0].plugin_id, - PluginId::parse("demo-plugin@test-marketplace").expect("plugin id") - ); - assert_eq!(sources[0].source_relative_path, "hooks/hooks.json"); - assert_eq!(sources[0].hooks.handler_count(), 1); - } - - #[test] - fn load_plugin_hooks_manifest_paths_replace_default_hooks_file() { - let tmp = tempfile::tempdir().expect("tempdir"); - let plugin_root = - AbsolutePathBuf::try_from(tmp.path().join("demo-plugin")).expect("plugin root"); - fs::create_dir_all(plugin_root.join(".codex-plugin")).expect("create manifest dir"); - fs::create_dir_all(plugin_root.join("hooks")).expect("create hooks dir"); - fs::write( - plugin_root.join(".codex-plugin/plugin.json"), - r#"{ - "name": "demo-plugin", - "hooks": ["./hooks/one.json", "./hooks/two.json"] -}"#, - ) - .expect("write manifest"); - fs::write( - plugin_root.join("hooks/hooks.json"), - r#"{ - "hooks": { - "PreToolUse": [ - { - "hooks": [{ "type": "command", "command": "echo ignored" }] - } - ] - } -}"#, - ) - .expect("write default hooks"); - fs::write( - plugin_root.join("hooks/one.json"), - r#"{ - "hooks": { - "PreToolUse": [ - { - "hooks": [{ "type": "command", "command": "echo one" }] - } - ] - } -}"#, - ) - .expect("write first hooks"); - fs::write( - plugin_root.join("hooks/two.json"), - r#"{ - "hooks": { - "PostToolUse": [ - { - "hooks": [{ "type": "command", "command": "echo two" }] - } - ] - } -}"#, - ) - .expect("write second hooks"); - - let manifest = load_plugin_manifest(plugin_root.as_path()).expect("manifest"); - let plugin_id = PluginId::parse("demo-plugin@test-marketplace").expect("plugin id"); - let sources = load_plugin_hooks(&plugin_root, &plugin_id, &manifest.paths); - - assert_eq!( - sources - .iter() - .map(|source| source.source_relative_path.as_str()) - .collect::>(), - vec!["hooks/one.json", "hooks/two.json"] - ); - assert_eq!( - sources - .iter() - .map(|source| source.hooks.handler_count()) - .collect::>(), - vec![1, 1] - ); - } - - #[test] - fn load_plugin_hooks_supports_inline_manifest_hooks() { - let tmp = tempfile::tempdir().expect("tempdir"); - let plugin_root = - AbsolutePathBuf::try_from(tmp.path().join("demo-plugin")).expect("plugin root"); - fs::create_dir_all(plugin_root.join(".codex-plugin")).expect("create manifest dir"); - fs::write( - plugin_root.join(".codex-plugin/plugin.json"), - r#"{ - "name": "demo-plugin", - "hooks": { - "hooks": { - "SessionStart": [ - { - "matcher": "startup", - "hooks": [{ "type": "command", "command": "echo inline" }] - } - ] - } - } -}"#, - ) - .expect("write manifest"); - - let manifest = load_plugin_manifest(plugin_root.as_path()).expect("manifest"); - let plugin_id = PluginId::parse("demo-plugin@test-marketplace").expect("plugin id"); - let sources = load_plugin_hooks(&plugin_root, &plugin_id, &manifest.paths); - - assert_eq!(sources.len(), 1); - assert_eq!(sources[0].source_relative_path, "plugin.json#hooks[0]"); - assert_eq!(sources[0].hooks.handler_count(), 1); - } - - #[test] - fn materialize_git_subdir_uses_sparse_checkout() { - let codex_home = tempfile::tempdir().expect("create codex home"); - let repo = tempfile::tempdir().expect("create git repo"); - let plugin_dir = repo.path().join("plugins/toolkit"); - fs::create_dir_all(&plugin_dir).expect("create plugin directory"); - fs::create_dir_all(repo.path().join("plugins/other")).expect("create other plugin"); - fs::write(plugin_dir.join("marker.txt"), "toolkit").expect("write plugin marker"); - fs::write(repo.path().join("plugins/other/marker.txt"), "other") - .expect("write other marker"); - fs::write(repo.path().join("root.txt"), "root").expect("write root marker"); - - run_git(&["init"], Some(repo.path())).expect("init git repo"); - run_git( - &["config", "user.email", "test@example.com"], - Some(repo.path()), - ) - .expect("configure git email"); - run_git(&["config", "user.name", "Test User"], Some(repo.path())) - .expect("configure git name"); - run_git(&["add", "."], Some(repo.path())).expect("stage git repo"); - run_git(&["commit", "-m", "init"], Some(repo.path())).expect("commit git repo"); - - let materialized = materialize_marketplace_plugin_source( - codex_home.path(), - &MarketplacePluginSource::Git { - url: repo.path().display().to_string(), - path: Some("plugins/toolkit".to_string()), - ref_name: None, - sha: None, - }, - ) - .expect("materialize git source"); - - assert_eq!( - plugin_dir.file_name(), - materialized.path.as_path().file_name() - ); - assert!(materialized.path.as_path().join("marker.txt").is_file()); - let checkout_root = materialized - .path - .as_path() - .parent() - .and_then(Path::parent) - .expect("materialized path should be nested under checkout root"); - assert!(!checkout_root.join("root.txt").exists()); - assert!(!checkout_root.join("plugins/other/marker.txt").exists()); - } -} +#[path = "loader_tests.rs"] +mod tests; diff --git a/codex-rs/core-plugins/src/loader_tests.rs b/codex-rs/core-plugins/src/loader_tests.rs new file mode 100644 index 000000000000..92ac41606933 --- /dev/null +++ b/codex-rs/core-plugins/src/loader_tests.rs @@ -0,0 +1,347 @@ +use super::*; +use crate::manifest::load_plugin_manifest; +use codex_plugin::PluginId; +use pretty_assertions::assert_eq; + +#[test] +fn plugin_mcp_file_supports_mcp_servers_object_format() { + let parsed = serde_json::from_str::( + r#"{ + "mcpServers": { + "sample": { + "command": "sample-mcp" + } + } +}"#, + ) + .expect("parse wrapped plugin mcp config") + .into_mcp_servers(); + + assert_eq!( + parsed, + HashMap::from([( + "sample".to_string(), + serde_json::json!({ + "command": "sample-mcp" + }), + )]) + ); +} + +#[test] +fn plugin_mcp_file_supports_mcp_servers_object_format_with_metadata() { + let parsed = serde_json::from_str::( + r#"{ + "$schema": "https://example.com/plugin-mcp.schema.json", + "mcpServers": { + "sample": { + "command": "sample-mcp" + } + } +}"#, + ) + .expect("parse plugin mcp config with metadata") + .into_mcp_servers(); + + assert_eq!( + parsed, + HashMap::from([( + "sample".to_string(), + serde_json::json!({ + "command": "sample-mcp" + }), + )]) + ); +} + +#[test] +fn plugin_mcp_file_supports_top_level_server_map_format() { + let parsed = serde_json::from_str::( + r#"{ + "linear": { + "type": "http", + "url": "https://mcp.linear.app/mcp" + } +}"#, + ) + .expect("parse flat plugin mcp config") + .into_mcp_servers(); + + assert_eq!( + parsed, + HashMap::from([( + "linear".to_string(), + serde_json::json!({ + "type": "http", + "url": "https://mcp.linear.app/mcp" + }), + )]) + ); +} + +#[test] +fn curated_plugin_cache_version_shortens_full_git_sha() { + assert_eq!( + curated_plugin_cache_version("0123456789abcdef0123456789abcdef01234567"), + "01234567" + ); +} + +#[test] +fn curated_plugin_cache_version_preserves_non_git_sha_versions() { + assert_eq!( + curated_plugin_cache_version("export-backup"), + "export-backup" + ); + assert_eq!(curated_plugin_cache_version("0123456"), "0123456"); +} + +fn plugin_id() -> PluginId { + PluginId::parse("demo-plugin@test-marketplace").expect("plugin id") +} + +fn plugin_root() -> (tempfile::TempDir, AbsolutePathBuf) { + let tmp = tempfile::tempdir().expect("tempdir"); + let plugin_root = + AbsolutePathBuf::try_from(tmp.path().join("demo-plugin")).expect("plugin root"); + fs::create_dir_all(plugin_root.join(".codex-plugin")).expect("create manifest dir"); + fs::create_dir_all(plugin_root.join("hooks")).expect("create hooks dir"); + (tmp, plugin_root) +} + +fn write_manifest(plugin_root: &AbsolutePathBuf, manifest: &str) { + fs::write(plugin_root.join(".codex-plugin/plugin.json"), manifest).expect("write manifest"); +} + +fn write_hook_file(plugin_root: &AbsolutePathBuf, relative_path: &str, event: &str, command: &str) { + fs::write( + plugin_root.join(relative_path), + format!( + r#"{{ + "hooks": {{ + "{event}": [ + {{ + "hooks": [{{ "type": "command", "command": "{command}" }}] + }} + ] + }} +}}"# + ), + ) + .expect("write hooks"); +} + +fn load_sources(plugin_root: &AbsolutePathBuf) -> Vec { + let manifest = load_plugin_manifest(plugin_root.as_path()).expect("manifest"); + load_plugin_hooks(plugin_root, &plugin_id(), &manifest.paths) +} + +#[test] +fn load_plugin_hooks_discovers_default_hooks_file() { + let (_tmp, plugin_root) = plugin_root(); + write_manifest(&plugin_root, r#"{ "name": "demo-plugin" }"#); + fs::write( + plugin_root.join("hooks/hooks.json"), + r#"{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [{ "type": "command", "command": "echo default" }] + } + ] + } +}"#, + ) + .expect("write hooks"); + + let sources = load_sources(&plugin_root); + + assert_eq!(sources.len(), 1); + assert_eq!(sources[0].plugin_id, plugin_id()); + assert_eq!(sources[0].source_relative_path, "hooks/hooks.json"); + assert_eq!(sources[0].hooks.handler_count(), 1); +} + +#[test] +fn load_plugin_hooks_supports_manifest_hook_path() { + let (_tmp, plugin_root) = plugin_root(); + write_manifest( + &plugin_root, + r#"{ + "name": "demo-plugin", + "hooks": "./hooks/one.json" +}"#, + ); + write_hook_file(&plugin_root, "hooks/one.json", "PreToolUse", "echo one"); + + let sources = load_sources(&plugin_root); + + assert_eq!( + sources + .iter() + .map(|source| source.source_relative_path.as_str()) + .collect::>(), + vec!["hooks/one.json"] + ); + assert_eq!(sources[0].hooks.handler_count(), 1); +} + +#[test] +fn load_plugin_hooks_manifest_paths_replace_default_hooks_file() { + let (_tmp, plugin_root) = plugin_root(); + write_manifest( + &plugin_root, + r#"{ + "name": "demo-plugin", + "hooks": ["./hooks/one.json", "./hooks/two.json"] +}"#, + ); + write_hook_file( + &plugin_root, + "hooks/hooks.json", + "PreToolUse", + "echo ignored", + ); + write_hook_file(&plugin_root, "hooks/one.json", "PreToolUse", "echo one"); + write_hook_file(&plugin_root, "hooks/two.json", "PostToolUse", "echo two"); + + let sources = load_sources(&plugin_root); + + assert_eq!( + sources + .iter() + .map(|source| source.source_relative_path.as_str()) + .collect::>(), + vec!["hooks/one.json", "hooks/two.json"] + ); + assert_eq!( + sources + .iter() + .map(|source| source.hooks.handler_count()) + .collect::>(), + vec![1, 1] + ); +} + +#[test] +fn load_plugin_hooks_supports_inline_manifest_hooks() { + let (_tmp, plugin_root) = plugin_root(); + write_manifest( + &plugin_root, + r#"{ + "name": "demo-plugin", + "hooks": { + "hooks": { + "SessionStart": [ + { + "matcher": "startup", + "hooks": [{ "type": "command", "command": "echo inline" }] + } + ] + } + } +}"#, + ); + + let sources = load_sources(&plugin_root); + + assert_eq!(sources.len(), 1); + assert_eq!(sources[0].source_relative_path, "plugin.json#hooks[0]"); + assert_eq!(sources[0].hooks.handler_count(), 1); +} + +#[test] +fn load_plugin_hooks_supports_inline_manifest_hook_list() { + let (_tmp, plugin_root) = plugin_root(); + write_manifest( + &plugin_root, + r#"{ + "name": "demo-plugin", + "hooks": [ + { + "hooks": { + "SessionStart": [ + { + "hooks": [{ "type": "command", "command": "echo inline one" }] + } + ] + } + }, + { + "hooks": { + "Stop": [ + { + "hooks": [{ "type": "command", "command": "echo inline two" }] + } + ] + } + } + ] +}"#, + ); + + let sources = load_sources(&plugin_root); + + assert_eq!( + sources + .iter() + .map(|source| source.source_relative_path.as_str()) + .collect::>(), + vec!["plugin.json#hooks[0]", "plugin.json#hooks[1]"] + ); + assert_eq!( + sources + .iter() + .map(|source| source.hooks.handler_count()) + .collect::>(), + vec![1, 1] + ); +} + +#[test] +fn materialize_git_subdir_uses_sparse_checkout() { + let codex_home = tempfile::tempdir().expect("create codex home"); + let repo = tempfile::tempdir().expect("create git repo"); + let plugin_dir = repo.path().join("plugins/toolkit"); + fs::create_dir_all(&plugin_dir).expect("create plugin directory"); + fs::create_dir_all(repo.path().join("plugins/other")).expect("create other plugin"); + fs::write(plugin_dir.join("marker.txt"), "toolkit").expect("write plugin marker"); + fs::write(repo.path().join("plugins/other/marker.txt"), "other").expect("write other marker"); + fs::write(repo.path().join("root.txt"), "root").expect("write root marker"); + + run_git(&["init"], Some(repo.path())).expect("init git repo"); + run_git( + &["config", "user.email", "test@example.com"], + Some(repo.path()), + ) + .expect("configure git email"); + run_git(&["config", "user.name", "Test User"], Some(repo.path())).expect("configure git name"); + run_git(&["add", "."], Some(repo.path())).expect("stage git repo"); + run_git(&["commit", "-m", "init"], Some(repo.path())).expect("commit git repo"); + + let materialized = materialize_marketplace_plugin_source( + codex_home.path(), + &MarketplacePluginSource::Git { + url: repo.path().display().to_string(), + path: Some("plugins/toolkit".to_string()), + ref_name: None, + sha: None, + }, + ) + .expect("materialize git source"); + + assert_eq!( + plugin_dir.file_name(), + materialized.path.as_path().file_name() + ); + assert!(materialized.path.as_path().join("marker.txt").is_file()); + let checkout_root = materialized + .path + .as_path() + .parent() + .and_then(Path::parent) + .expect("materialized path should be nested under checkout root"); + assert!(!checkout_root.join("root.txt").exists()); + assert!(!checkout_root.join("plugins/other/marker.txt").exists()); +} From 108dc3514ca7b841813e4605f220f47dea2d2d96 Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Sun, 26 Apr 2026 21:14:48 -0700 Subject: [PATCH 06/10] Inline plugin hook relative path --- codex-rs/core-plugins/src/loader.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index b2be23c45a45..41217666ae9d 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -758,22 +758,22 @@ fn load_plugin_hook_file( return None; } + let source_relative_path = path + .as_path() + .strip_prefix(plugin_root.as_path()) + .unwrap_or(path.as_path()) + .to_string_lossy() + .replace('\\', "/"); + Some(PluginHookSource { plugin_id: plugin_id.clone(), plugin_root: plugin_root.clone(), source_path: path.clone(), - source_relative_path: plugin_relative_path(plugin_root.as_path(), path.as_path()), + source_relative_path, hooks: parsed.hooks, }) } -fn plugin_relative_path(plugin_root: &Path, path: &Path) -> String { - path.strip_prefix(plugin_root) - .unwrap_or(path) - .to_string_lossy() - .replace('\\', "/") -} - async fn load_apps_from_paths( plugin_root: &Path, app_config_paths: Vec, From 6d03af75a701b0af24e68a2088555526099fda59 Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Mon, 27 Apr 2026 09:57:54 -0700 Subject: [PATCH 07/10] Refactor plugin hook loader test assertions Co-authored-by: Codex --- codex-rs/core-plugins/src/loader_tests.rs | 72 +++++++++-------------- 1 file changed, 29 insertions(+), 43 deletions(-) diff --git a/codex-rs/core-plugins/src/loader_tests.rs b/codex-rs/core-plugins/src/loader_tests.rs index 92ac41606933..bbc0642cb596 100644 --- a/codex-rs/core-plugins/src/loader_tests.rs +++ b/codex-rs/core-plugins/src/loader_tests.rs @@ -136,6 +136,30 @@ fn load_sources(plugin_root: &AbsolutePathBuf) -> Vec { load_plugin_hooks(plugin_root, &plugin_id(), &manifest.paths) } +fn assert_sources(sources: &[PluginHookSource], expected_relative_paths: &[&str]) { + assert_eq!( + sources + .iter() + .map(|source| source.plugin_id.clone()) + .collect::>(), + vec![plugin_id(); expected_relative_paths.len()] + ); + assert_eq!( + sources + .iter() + .map(|source| source.source_relative_path.as_str()) + .collect::>(), + expected_relative_paths + ); + assert_eq!( + sources + .iter() + .map(|source| source.hooks.handler_count()) + .collect::>(), + vec![1; expected_relative_paths.len()] + ); +} + #[test] fn load_plugin_hooks_discovers_default_hooks_file() { let (_tmp, plugin_root) = plugin_root(); @@ -157,10 +181,7 @@ fn load_plugin_hooks_discovers_default_hooks_file() { let sources = load_sources(&plugin_root); - assert_eq!(sources.len(), 1); - assert_eq!(sources[0].plugin_id, plugin_id()); - assert_eq!(sources[0].source_relative_path, "hooks/hooks.json"); - assert_eq!(sources[0].hooks.handler_count(), 1); + assert_sources(&sources, &["hooks/hooks.json"]); } #[test] @@ -177,14 +198,7 @@ fn load_plugin_hooks_supports_manifest_hook_path() { let sources = load_sources(&plugin_root); - assert_eq!( - sources - .iter() - .map(|source| source.source_relative_path.as_str()) - .collect::>(), - vec!["hooks/one.json"] - ); - assert_eq!(sources[0].hooks.handler_count(), 1); + assert_sources(&sources, &["hooks/one.json"]); } #[test] @@ -208,20 +222,7 @@ fn load_plugin_hooks_manifest_paths_replace_default_hooks_file() { let sources = load_sources(&plugin_root); - assert_eq!( - sources - .iter() - .map(|source| source.source_relative_path.as_str()) - .collect::>(), - vec!["hooks/one.json", "hooks/two.json"] - ); - assert_eq!( - sources - .iter() - .map(|source| source.hooks.handler_count()) - .collect::>(), - vec![1, 1] - ); + assert_sources(&sources, &["hooks/one.json", "hooks/two.json"]); } #[test] @@ -246,9 +247,7 @@ fn load_plugin_hooks_supports_inline_manifest_hooks() { let sources = load_sources(&plugin_root); - assert_eq!(sources.len(), 1); - assert_eq!(sources[0].source_relative_path, "plugin.json#hooks[0]"); - assert_eq!(sources[0].hooks.handler_count(), 1); + assert_sources(&sources, &["plugin.json#hooks[0]"]); } #[test] @@ -283,20 +282,7 @@ fn load_plugin_hooks_supports_inline_manifest_hook_list() { let sources = load_sources(&plugin_root); - assert_eq!( - sources - .iter() - .map(|source| source.source_relative_path.as_str()) - .collect::>(), - vec!["plugin.json#hooks[0]", "plugin.json#hooks[1]"] - ); - assert_eq!( - sources - .iter() - .map(|source| source.hooks.handler_count()) - .collect::>(), - vec![1, 1] - ); + assert_sources(&sources, &["plugin.json#hooks[0]", "plugin.json#hooks[1]"]); } #[test] From 2c4c2c1030eb7a13095ef622fa57518ae46df761 Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Mon, 27 Apr 2026 19:25:03 -0700 Subject: [PATCH 08/10] Support plugin hook path substitution and data dirs --- codex-rs/core-plugins/src/loader.rs | 79 +++++++++++------- codex-rs/core-plugins/src/loader_tests.rs | 50 ++++++++++-- codex-rs/core-plugins/src/store.rs | 14 +++- codex-rs/core-plugins/src/store_tests.rs | 12 +++ codex-rs/core/src/plugins/manager_tests.rs | 3 + codex-rs/core/src/session/session.rs | 10 ++- codex-rs/hooks/src/engine/discovery.rs | 13 ++- codex-rs/hooks/src/engine/mod.rs | 7 +- codex-rs/hooks/src/engine/mod_tests.rs | 94 ++++++++++++++++++++++ codex-rs/hooks/src/registry.rs | 2 + codex-rs/plugin/src/lib.rs | 1 + codex-rs/plugin/src/load_outcome.rs | 9 +++ 12 files changed, 253 insertions(+), 41 deletions(-) diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 41217666ae9d..55b8c0b57c76 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -483,6 +483,7 @@ async fn load_plugin( mcp_servers: HashMap::new(), apps: Vec::new(), hook_sources: Vec::new(), + hook_load_warnings: Vec::new(), error: None, }; @@ -551,7 +552,14 @@ async fn load_plugin( } loaded_plugin.mcp_servers = mcp_servers; loaded_plugin.apps = load_plugin_apps(plugin_root.as_path()).await; - loaded_plugin.hook_sources = load_plugin_hooks(&plugin_root, &loaded_plugin_id, manifest_paths); + let (hook_sources, hook_load_warnings) = load_plugin_hooks( + &plugin_root, + &loaded_plugin_id, + &store.plugin_data_root(&loaded_plugin_id), + manifest_paths, + ); + loaded_plugin.hook_sources = hook_sources; + loaded_plugin.hook_load_warnings = hook_load_warnings; loaded_plugin } @@ -687,15 +695,22 @@ fn default_app_config_paths(plugin_root: &Path) -> Vec { pub fn load_plugin_hooks( plugin_root: &AbsolutePathBuf, plugin_id: &PluginId, + plugin_data_root: &AbsolutePathBuf, manifest_paths: &PluginManifestPaths, -) -> Vec { +) -> (Vec, Vec) { let mut sources = Vec::new(); + let mut warnings = Vec::new(); match &manifest_paths.hooks { Some(PluginManifestHooks::Paths(paths)) => { for path in paths { - if let Some(source) = load_plugin_hook_file(plugin_root, plugin_id, path) { - sources.push(source); - } + append_plugin_hook_file( + plugin_root, + plugin_id, + plugin_data_root, + path, + &mut sources, + &mut warnings, + ); } } Some(PluginManifestHooks::Inline(hooks_files)) => { @@ -709,6 +724,7 @@ pub fn load_plugin_hooks( sources.push(PluginHookSource { plugin_id: plugin_id.clone(), plugin_root: plugin_root.clone(), + plugin_data_root: plugin_data_root.clone(), source_path: manifest_path.clone(), source_relative_path: format!("plugin.json#hooks[{index}]"), hooks: hooks_file.hooks.clone(), @@ -717,45 +733,53 @@ pub fn load_plugin_hooks( } None => { let default_path = plugin_root.join(DEFAULT_HOOKS_CONFIG_FILE); - if default_path.as_path().is_file() - && let Some(source) = load_plugin_hook_file(plugin_root, plugin_id, &default_path) - { - sources.push(source); + if default_path.as_path().is_file() { + append_plugin_hook_file( + plugin_root, + plugin_id, + plugin_data_root, + &default_path, + &mut sources, + &mut warnings, + ); } } } - sources + (sources, warnings) } -// Load one resolved plugin hook file and keep source metadata with its parsed -// hook events so runtime discovery can report plugin-originated hook runs. -fn load_plugin_hook_file( +// Append one resolved plugin hook file, keeping source metadata for runtime +// reporting and collecting load warnings for startup surfacing. +fn append_plugin_hook_file( plugin_root: &AbsolutePathBuf, plugin_id: &PluginId, + plugin_data_root: &AbsolutePathBuf, path: &AbsolutePathBuf, -) -> Option { + sources: &mut Vec, + warnings: &mut Vec, +) { let contents = match fs::read_to_string(path.as_path()) { Ok(contents) => contents, Err(err) => { - warn!( - path = %path.display(), - "failed to read plugin hooks config: {err}" - ); - return None; + warnings.push(format!( + "failed to read plugin hooks config {}: {err}", + path.display() + )); + return; } }; let parsed = match serde_json::from_str::(&contents) { Ok(parsed) => parsed, Err(err) => { - warn!( - path = %path.display(), - "failed to parse plugin hooks config: {err}" - ); - return None; + warnings.push(format!( + "failed to parse plugin hooks config {}: {err}", + path.display() + )); + return; } }; if parsed.hooks.is_empty() { - return None; + return; } let source_relative_path = path @@ -765,13 +789,14 @@ fn load_plugin_hook_file( .to_string_lossy() .replace('\\', "/"); - Some(PluginHookSource { + sources.push(PluginHookSource { plugin_id: plugin_id.clone(), plugin_root: plugin_root.clone(), + plugin_data_root: plugin_data_root.clone(), source_path: path.clone(), source_relative_path, hooks: parsed.hooks, - }) + }); } async fn load_apps_from_paths( diff --git a/codex-rs/core-plugins/src/loader_tests.rs b/codex-rs/core-plugins/src/loader_tests.rs index bbc0642cb596..d9029c584eb2 100644 --- a/codex-rs/core-plugins/src/loader_tests.rs +++ b/codex-rs/core-plugins/src/loader_tests.rs @@ -131,9 +131,22 @@ fn write_hook_file(plugin_root: &AbsolutePathBuf, relative_path: &str, event: &s .expect("write hooks"); } -fn load_sources(plugin_root: &AbsolutePathBuf) -> Vec { +fn load_sources(plugin_root: &AbsolutePathBuf) -> (Vec, Vec) { let manifest = load_plugin_manifest(plugin_root.as_path()).expect("manifest"); - load_plugin_hooks(plugin_root, &plugin_id(), &manifest.paths) + let plugin_data_root = AbsolutePathBuf::try_from( + plugin_root + .as_path() + .parent() + .expect("plugin root parent") + .join("plugin-data"), + ) + .expect("plugin data root"); + load_plugin_hooks( + plugin_root, + &plugin_id(), + &plugin_data_root, + &manifest.paths, + ) } fn assert_sources(sources: &[PluginHookSource], expected_relative_paths: &[&str]) { @@ -179,8 +192,9 @@ fn load_plugin_hooks_discovers_default_hooks_file() { ) .expect("write hooks"); - let sources = load_sources(&plugin_root); + let (sources, warnings) = load_sources(&plugin_root); + assert_eq!(warnings, Vec::::new()); assert_sources(&sources, &["hooks/hooks.json"]); } @@ -196,8 +210,9 @@ fn load_plugin_hooks_supports_manifest_hook_path() { ); write_hook_file(&plugin_root, "hooks/one.json", "PreToolUse", "echo one"); - let sources = load_sources(&plugin_root); + let (sources, warnings) = load_sources(&plugin_root); + assert_eq!(warnings, Vec::::new()); assert_sources(&sources, &["hooks/one.json"]); } @@ -220,8 +235,9 @@ fn load_plugin_hooks_manifest_paths_replace_default_hooks_file() { write_hook_file(&plugin_root, "hooks/one.json", "PreToolUse", "echo one"); write_hook_file(&plugin_root, "hooks/two.json", "PostToolUse", "echo two"); - let sources = load_sources(&plugin_root); + let (sources, warnings) = load_sources(&plugin_root); + assert_eq!(warnings, Vec::::new()); assert_sources(&sources, &["hooks/one.json", "hooks/two.json"]); } @@ -245,11 +261,30 @@ fn load_plugin_hooks_supports_inline_manifest_hooks() { }"#, ); - let sources = load_sources(&plugin_root); + let (sources, warnings) = load_sources(&plugin_root); + assert_eq!(warnings, Vec::::new()); assert_sources(&sources, &["plugin.json#hooks[0]"]); } +#[test] +fn load_plugin_hooks_reports_invalid_hook_file() { + let (_tmp, plugin_root) = plugin_root(); + write_manifest(&plugin_root, r#"{ "name": "demo-plugin" }"#); + fs::write(plugin_root.join("hooks/hooks.json"), "{ not-json").expect("write invalid hooks"); + + let (sources, warnings) = load_sources(&plugin_root); + + assert_eq!(sources, Vec::::new()); + assert_eq!( + warnings, + vec![format!( + "failed to parse plugin hooks config {}: key must be a string at line 1 column 3", + plugin_root.join("hooks/hooks.json").display() + )] + ); +} + #[test] fn load_plugin_hooks_supports_inline_manifest_hook_list() { let (_tmp, plugin_root) = plugin_root(); @@ -280,8 +315,9 @@ fn load_plugin_hooks_supports_inline_manifest_hook_list() { }"#, ); - let sources = load_sources(&plugin_root); + let (sources, warnings) = load_sources(&plugin_root); + assert_eq!(warnings, Vec::::new()); assert_sources(&sources, &["plugin.json#hooks[0]", "plugin.json#hooks[1]"]); } diff --git a/codex-rs/core-plugins/src/store.rs b/codex-rs/core-plugins/src/store.rs index 757aec8bc53d..9d760da53e08 100644 --- a/codex-rs/core-plugins/src/store.rs +++ b/codex-rs/core-plugins/src/store.rs @@ -13,6 +13,7 @@ use std::path::PathBuf; pub const DEFAULT_PLUGIN_VERSION: &str = "local"; pub const PLUGINS_CACHE_DIR: &str = "plugins/cache"; +pub const PLUGINS_DATA_DIR: &str = "plugins/data"; #[derive(Debug, Clone, PartialEq, Eq)] pub struct PluginInstallResult { @@ -24,6 +25,7 @@ pub struct PluginInstallResult { #[derive(Debug, Clone)] pub struct PluginStore { root: AbsolutePathBuf, + data_root: AbsolutePathBuf, } impl PluginStore { @@ -35,8 +37,11 @@ impl PluginStore { pub fn try_new(codex_home: PathBuf) -> Result { let root = AbsolutePathBuf::from_absolute_path_checked(codex_home.join(PLUGINS_CACHE_DIR)) .map_err(|err| PluginStoreError::io("failed to resolve plugin cache root", err))?; + let data_root = + AbsolutePathBuf::from_absolute_path_checked(codex_home.join(PLUGINS_DATA_DIR)) + .map_err(|err| PluginStoreError::io("failed to resolve plugin data root", err))?; - Ok(Self { root }) + Ok(Self { root, data_root }) } pub fn root(&self) -> &AbsolutePathBuf { @@ -53,6 +58,13 @@ impl PluginStore { self.plugin_base_root(plugin_id).join(plugin_version) } + pub fn plugin_data_root(&self, plugin_id: &PluginId) -> AbsolutePathBuf { + self.data_root.join(format!( + "{}-{}", + plugin_id.plugin_name, plugin_id.marketplace_name + )) + } + pub fn active_plugin_version(&self, plugin_id: &PluginId) -> Option { let mut discovered_versions = fs::read_dir(self.plugin_base_root(plugin_id).as_path()) .ok()? diff --git a/codex-rs/core-plugins/src/store_tests.rs b/codex-rs/core-plugins/src/store_tests.rs index 45feff61bd5f..0ba6b0d2c6ea 100644 --- a/codex-rs/core-plugins/src/store_tests.rs +++ b/codex-rs/core-plugins/src/store_tests.rs @@ -109,6 +109,18 @@ fn plugin_root_derives_path_from_key_and_version() { ); } +#[test] +fn plugin_data_root_derives_path_from_key() { + let tmp = tempdir().unwrap(); + let store = PluginStore::new(tmp.path().to_path_buf()); + let plugin_id = PluginId::new("sample".to_string(), "debug".to_string()).unwrap(); + + assert_eq!( + store.plugin_data_root(&plugin_id).as_path(), + tmp.path().join("plugins/data/sample-debug") + ); +} + #[test] fn install_with_version_uses_requested_cache_version() { let tmp = tempdir().unwrap(); diff --git a/codex-rs/core/src/plugins/manager_tests.rs b/codex-rs/core/src/plugins/manager_tests.rs index 8eb7f5b0960d..fb4b5a62125a 100644 --- a/codex-rs/core/src/plugins/manager_tests.rs +++ b/codex-rs/core/src/plugins/manager_tests.rs @@ -220,6 +220,7 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() { )]), apps: vec![AppConnectorId("connector_example".to_string())], hook_sources: Vec::new(), + hook_load_warnings: Vec::new(), error: None, }] ); @@ -721,6 +722,7 @@ async fn load_plugins_preserves_disabled_plugins_without_effective_contributions mcp_servers: HashMap::new(), apps: Vec::new(), hook_sources: Vec::new(), + hook_load_warnings: Vec::new(), error: None, }] ); @@ -839,6 +841,7 @@ fn capability_index_filters_inactive_and_zero_capability_plugins() { mcp_servers: HashMap::new(), apps: Vec::new(), hook_sources: Vec::new(), + hook_load_warnings: Vec::new(), error: None, }; let summary = |config_name: &str, display_name: &str| PluginCapabilitySummary { diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index beaecab14ff2..351e3049ab92 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -755,17 +755,21 @@ impl Session { let hook_shell_program = hook_shell_argv.remove(0); let _ = hook_shell_argv.pop(); let plugin_hooks_enabled = config.features.enabled(Feature::PluginHooks); - let plugin_hook_sources = if plugin_hooks_enabled { + let (plugin_hook_sources, plugin_hook_load_warnings) = if plugin_hooks_enabled { let plugin_outcome = plugins_manager.plugins_for_config(&config).await; - plugin_outcome.effective_plugin_hook_sources() + ( + plugin_outcome.effective_plugin_hook_sources(), + plugin_outcome.effective_plugin_hook_warnings(), + ) } else { - Vec::new() + (Vec::new(), Vec::new()) }; let hooks = Hooks::new(HooksConfig { legacy_notify_argv: config.notify.clone(), feature_enabled: config.features.enabled(Feature::CodexHooks), config_layer_stack: Some(config.config_layer_stack.clone()), plugin_hook_sources, + plugin_hook_load_warnings, shell_program: Some(hook_shell_program), shell_args: hook_shell_argv, }); diff --git a/codex-rs/hooks/src/engine/discovery.rs b/codex-rs/hooks/src/engine/discovery.rs index ce42908c097e..f2e195bb9439 100644 --- a/codex-rs/hooks/src/engine/discovery.rs +++ b/codex-rs/hooks/src/engine/discovery.rs @@ -38,10 +38,11 @@ struct HookHandlerSource<'a> { pub(crate) fn discover_handlers( config_layer_stack: Option<&ConfigLayerStack>, plugin_hook_sources: Vec, + plugin_hook_load_warnings: Vec, ) -> DiscoveryResult { let Some(config_layer_stack) = config_layer_stack else { let mut handlers = Vec::new(); - let mut warnings = Vec::new(); + let mut warnings = plugin_hook_load_warnings; let mut display_order = 0_i64; append_plugin_hook_sources( &mut handlers, @@ -53,7 +54,7 @@ pub(crate) fn discover_handlers( }; let mut handlers = Vec::new(); - let mut warnings = Vec::new(); + let mut warnings = plugin_hook_load_warnings; let mut display_order = 0_i64; append_managed_requirement_handlers( @@ -162,15 +163,20 @@ fn append_plugin_hook_sources( for source in plugin_hook_sources { let PluginHookSource { plugin_root, + plugin_data_root, source_path, hooks, .. } = source; let mut env = HashMap::new(); let plugin_root_value = plugin_root.display().to_string(); + let plugin_data_root_value = plugin_data_root.display().to_string(); env.insert("PLUGIN_ROOT".to_string(), plugin_root_value.clone()); // For OOTB compat with existing plugins that use this env var. env.insert("CLAUDE_PLUGIN_ROOT".to_string(), plugin_root_value); + env.insert("PLUGIN_DATA".to_string(), plugin_data_root_value.clone()); + // For OOTB compat with existing plugins that use this env var. + env.insert("CLAUDE_PLUGIN_DATA".to_string(), plugin_data_root_value); append_hook_events( handlers, warnings, @@ -403,6 +409,9 @@ fn append_group_handlers( )); continue; } + let command = source.env.iter().fold(command, |command, (key, value)| { + command.replace(&format!("${{{key}}}"), value) + }); let timeout_sec = timeout_sec.unwrap_or(600).max(1); handlers.push(ConfiguredHandler { event_name, diff --git a/codex-rs/hooks/src/engine/mod.rs b/codex-rs/hooks/src/engine/mod.rs index 5c121136f7a0..89daf501caeb 100644 --- a/codex-rs/hooks/src/engine/mod.rs +++ b/codex-rs/hooks/src/engine/mod.rs @@ -79,6 +79,7 @@ impl ClaudeHooksEngine { enabled: bool, config_layer_stack: Option<&ConfigLayerStack>, plugin_hook_sources: Vec, + plugin_hook_load_warnings: Vec, shell: CommandShell, ) -> Self { if !enabled { @@ -90,7 +91,11 @@ impl ClaudeHooksEngine { } let _ = schema_loader::generated_hook_schemas(); - let discovered = discovery::discover_handlers(config_layer_stack, plugin_hook_sources); + let discovered = discovery::discover_handlers( + config_layer_stack, + plugin_hook_sources, + plugin_hook_load_warnings, + ); Self { handlers: discovered.handlers, warnings: discovered.warnings, diff --git a/codex-rs/hooks/src/engine/mod_tests.rs b/codex-rs/hooks/src/engine/mod_tests.rs index 245c31ba5fd0..b29542d8bb08 100644 --- a/codex-rs/hooks/src/engine/mod_tests.rs +++ b/codex-rs/hooks/src/engine/mod_tests.rs @@ -1,3 +1,4 @@ +use std::collections::HashMap; use std::fs; use std::path::Path; @@ -109,6 +110,7 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle: /*enabled*/ true, Some(&config_layer_stack), Vec::new(), + Vec::new(), CommandShell { program: String::new(), args: Vec::new(), @@ -193,6 +195,7 @@ fn requirements_managed_hooks_warn_when_managed_dir_is_missing() { /*enabled*/ true, Some(&config_layer_stack), Vec::new(), + Vec::new(), CommandShell { program: String::new(), args: Vec::new(), @@ -301,6 +304,7 @@ fn discovers_hooks_from_json_and_toml_in_the_same_layer() { /*enabled*/ true, Some(&config_layer_stack), Vec::new(), + Vec::new(), CommandShell { program: String::new(), args: Vec::new(), @@ -337,6 +341,8 @@ async fn plugin_hook_sources_run_with_plugin_env_and_plugin_source() { let temp = tempdir().expect("create temp dir"); let plugin_root = AbsolutePathBuf::try_from(temp.path().join("demo-plugin")).expect("plugin root"); + let plugin_data_root = + AbsolutePathBuf::try_from(temp.path().join("plugin-data")).expect("plugin data root"); fs::create_dir_all(plugin_root.join("hooks")).expect("create hooks dir"); let source_path = plugin_root.join("hooks/hooks.json"); let log_path = plugin_root.join("env.json"); @@ -361,6 +367,7 @@ Path(r"{log_path}").write_text(json.dumps({{ let plugin_hook_sources = vec![PluginHookSource { plugin_id, plugin_root: plugin_root.clone(), + plugin_data_root: plugin_data_root.clone(), source_path: source_path.clone(), source_relative_path: "hooks/hooks.json".to_string(), hooks: HookEventsToml { @@ -380,6 +387,7 @@ Path(r"{log_path}").write_text(json.dumps({{ /*enabled*/ true, /*config_layer_stack*/ None, plugin_hook_sources, + Vec::new(), CommandShell { program: String::new(), args: Vec::new(), @@ -430,3 +438,89 @@ Path(r"{log_path}").write_text(json.dumps({{ }) ); } + +#[test] +fn plugin_hook_sources_expand_plugin_placeholders() { + let temp = tempdir().expect("create temp dir"); + let plugin_root = + AbsolutePathBuf::try_from(temp.path().join("demo-plugin")).expect("plugin root"); + let plugin_data_root = + AbsolutePathBuf::try_from(temp.path().join("plugin-data")).expect("plugin data root"); + let source_path = plugin_root.join("hooks/hooks.json"); + let plugin_id = PluginId::parse("demo-plugin@test-marketplace").expect("plugin id"); + let plugin_hook_sources = vec![PluginHookSource { + plugin_id, + plugin_root: plugin_root.clone(), + plugin_data_root: plugin_data_root.clone(), + source_path, + source_relative_path: "hooks/hooks.json".to_string(), + hooks: HookEventsToml { + pre_tool_use: vec![MatcherGroup { + matcher: Some("Bash".to_string()), + hooks: vec![HookHandlerConfig::Command { + command: "run ${PLUGIN_ROOT} ${CLAUDE_PLUGIN_ROOT} ${PLUGIN_DATA} ${CLAUDE_PLUGIN_DATA}" + .to_string(), + timeout_sec: Some(5), + r#async: false, + status_message: None, + }], + }], + ..Default::default() + }, + }]; + let engine = ClaudeHooksEngine::new( + /*enabled*/ true, + /*config_layer_stack*/ None, + plugin_hook_sources, + Vec::new(), + CommandShell { + program: String::new(), + args: Vec::new(), + }, + ); + + assert_eq!( + engine.handlers[0].command, + format!( + "run {} {} {} {}", + plugin_root.display(), + plugin_root.display(), + plugin_data_root.display(), + plugin_data_root.display() + ) + ); + assert_eq!( + engine.handlers[0].env, + HashMap::from([ + ("PLUGIN_ROOT".to_string(), plugin_root.display().to_string()), + ( + "CLAUDE_PLUGIN_ROOT".to_string(), + plugin_root.display().to_string() + ), + ( + "PLUGIN_DATA".to_string(), + plugin_data_root.display().to_string() + ), + ( + "CLAUDE_PLUGIN_DATA".to_string(), + plugin_data_root.display().to_string() + ), + ]) + ); +} + +#[test] +fn plugin_hook_load_warnings_are_startup_warnings() { + let engine = ClaudeHooksEngine::new( + /*enabled*/ true, + /*config_layer_stack*/ None, + Vec::new(), + vec!["failed plugin hook".to_string()], + CommandShell { + program: String::new(), + args: Vec::new(), + }, + ); + + assert_eq!(engine.warnings(), &["failed plugin hook".to_string()]); +} diff --git a/codex-rs/hooks/src/registry.rs b/codex-rs/hooks/src/registry.rs index 4509a8a63187..7dd93213a112 100644 --- a/codex-rs/hooks/src/registry.rs +++ b/codex-rs/hooks/src/registry.rs @@ -27,6 +27,7 @@ pub struct HooksConfig { pub feature_enabled: bool, pub config_layer_stack: Option, pub plugin_hook_sources: Vec, + pub plugin_hook_load_warnings: Vec, pub shell_program: Option, pub shell_args: Vec, } @@ -56,6 +57,7 @@ impl Hooks { config.feature_enabled, config.config_layer_stack.as_ref(), config.plugin_hook_sources, + config.plugin_hook_load_warnings, CommandShell { program: config.shell_program.unwrap_or_default(), args: config.shell_args, diff --git a/codex-rs/plugin/src/lib.rs b/codex-rs/plugin/src/lib.rs index 31ecf5601522..2140645de398 100644 --- a/codex-rs/plugin/src/lib.rs +++ b/codex-rs/plugin/src/lib.rs @@ -33,6 +33,7 @@ pub struct PluginCapabilitySummary { pub struct PluginHookSource { pub plugin_id: PluginId, pub plugin_root: AbsolutePathBuf, + pub plugin_data_root: AbsolutePathBuf, pub source_path: AbsolutePathBuf, pub source_relative_path: String, pub hooks: HookEventsToml, diff --git a/codex-rs/plugin/src/load_outcome.rs b/codex-rs/plugin/src/load_outcome.rs index 40dba4ae2ae1..0865b9020fcd 100644 --- a/codex-rs/plugin/src/load_outcome.rs +++ b/codex-rs/plugin/src/load_outcome.rs @@ -23,6 +23,7 @@ pub struct LoadedPlugin { pub mcp_servers: HashMap, pub apps: Vec, pub hook_sources: Vec, + pub hook_load_warnings: Vec, pub error: Option, } @@ -150,6 +151,14 @@ impl PluginLoadOutcome { .collect() } + pub fn effective_plugin_hook_warnings(&self) -> Vec { + self.plugins + .iter() + .filter(|plugin| plugin.is_active()) + .flat_map(|plugin| plugin.hook_load_warnings.iter().cloned()) + .collect() + } + pub fn capability_summaries(&self) -> &[PluginCapabilitySummary] { &self.capability_summaries } From fee039dcb85340356bfa3321006e9bd2347f5e0d Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Tue, 28 Apr 2026 11:30:11 -0700 Subject: [PATCH 09/10] Add plugin hook integration test --- codex-rs/core/tests/suite/hooks.rs | 144 +++++++++++++++++++++++++++++ 1 file changed, 144 insertions(+) diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs index 74e9a7a6824d..fccde13f2772 100644 --- a/codex-rs/core/tests/suite/hooks.rs +++ b/codex-rs/core/tests/suite/hooks.rs @@ -1852,6 +1852,150 @@ async fn pre_tool_use_blocks_shell_command_before_execution() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn plugin_pre_tool_use_blocks_shell_command_before_execution() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let call_id = "plugin-pretooluse-shell-command"; + let marker = std::env::temp_dir().join("plugin-pretooluse-shell-command-marker"); + let command = format!("printf blocked > {}", marker.display()); + let args = serde_json::json!({ "command": command }); + let responses = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + core_test_support::responses::ev_function_call( + call_id, + "shell_command", + &serde_json::to_string(&args)?, + ), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "plugin hook blocked it"), + ev_completed("resp-2"), + ]), + ], + ) + .await; + + let home = Arc::new(TempDir::new()?); + let plugin_root = home.path().join("plugins/cache/test/sample/local"); + let hooks_dir = plugin_root.join("hooks"); + fs::create_dir_all(plugin_root.join(".codex-plugin")) + .context("create plugin manifest directory")?; + fs::create_dir_all(&hooks_dir).context("create plugin hooks directory")?; + fs::write( + plugin_root.join(".codex-plugin/plugin.json"), + r#"{"name":"sample"}"#, + ) + .context("write plugin manifest")?; + fs::write( + home.path().join("config.toml"), + r#"[plugins."sample@test"] +enabled = true +"#, + ) + .context("write plugin config")?; + + let script_path = hooks_dir.join("pre_tool_use_hook.py"); + let log_path = hooks_dir.join("pre_tool_use_hook_log.jsonl"); + fs::write( + &script_path, + format!( + r#"import json +from pathlib import Path +import sys + +payload = json.load(sys.stdin) +with Path(r"{log_path}").open("a", encoding="utf-8") as handle: + handle.write(json.dumps(payload) + "\n") + +print(json.dumps({{ + "hookSpecificOutput": {{ + "hookEventName": "PreToolUse", + "permissionDecision": "deny", + "permissionDecisionReason": "blocked by plugin hook" + }} +}})) +"#, + log_path = log_path.display(), + ), + ) + .context("write plugin pre tool use hook script")?; + fs::write( + hooks_dir.join("hooks.json"), + r#"{ + "hooks": { + "PreToolUse": [{ + "matcher": "^Bash$", + "hooks": [{ + "type": "command", + "command": "python3 ${PLUGIN_ROOT}/hooks/pre_tool_use_hook.py" + }] + }] + } +}"#, + ) + .context("write plugin hooks config")?; + + let mut builder = test_codex() + .with_home(Arc::clone(&home)) + .with_config(|config| { + config + .features + .enable(Feature::Plugins) + .expect("test config should allow feature update"); + config + .features + .enable(Feature::CodexHooks) + .expect("test config should allow feature update"); + config + .features + .enable(Feature::PluginHooks) + .expect("test config should allow feature update"); + }); + let test = builder.build(&server).await?; + + if marker.exists() { + fs::remove_file(&marker).context("remove leftover plugin pre tool use marker")?; + } + + test.submit_turn_with_policy( + "run the shell command blocked by a plugin hook", + codex_protocol::protocol::SandboxPolicy::DangerFullAccess, + ) + .await?; + + let requests = responses.requests(); + assert_eq!(requests.len(), 2); + let output_item = requests[1].function_call_output(call_id); + let output = output_item + .get("output") + .and_then(Value::as_str) + .expect("shell command output string"); + assert!( + output.contains("Command blocked by PreToolUse hook: blocked by plugin hook"), + "blocked tool output should surface the plugin hook reason", + ); + assert!( + !marker.exists(), + "plugin hook should block shell command execution" + ); + + let hook_inputs = read_hook_inputs_from_log(&log_path)?; + assert_eq!(hook_inputs.len(), 1); + assert_eq!(hook_inputs[0]["hook_event_name"], "PreToolUse"); + assert_eq!(hook_inputs[0]["tool_name"], "Bash"); + assert_eq!(hook_inputs[0]["tool_use_id"], call_id); + assert_eq!(hook_inputs[0]["tool_input"]["command"], command); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn pre_tool_use_blocks_shell_when_defined_in_config_toml() -> Result<()> { skip_if_no_network!(Ok(())); From f71c5113d9b1129cf2e9fc66b6a1985520fbfcb0 Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Tue, 28 Apr 2026 11:44:24 -0700 Subject: [PATCH 10/10] Make hooks integration tests single-threaded --- codex-rs/core/tests/suite/hooks.rs | 58 +++++++++++++++--------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs index fccde13f2772..ceddaa3c6c7a 100644 --- a/codex-rs/core/tests/suite/hooks.rs +++ b/codex-rs/core/tests/suite/hooks.rs @@ -732,7 +732,7 @@ fn request_message_input_texts(body: &[u8], role: &str) -> Vec { .collect() } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn stop_hook_can_block_multiple_times_in_same_turn() -> Result<()> { skip_if_no_network!(Ok(())); @@ -846,7 +846,7 @@ async fn stop_hook_can_block_multiple_times_in_same_turn() -> Result<()> { Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn session_start_hook_sees_materialized_transcript_path() -> Result<()> { skip_if_no_network!(Ok(())); @@ -891,7 +891,7 @@ async fn session_start_hook_sees_materialized_transcript_path() -> Result<()> { Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn resumed_thread_keeps_stop_continuation_prompt_in_history() -> Result<()> { skip_if_no_network!(Ok(())); @@ -967,7 +967,7 @@ async fn resumed_thread_keeps_stop_continuation_prompt_in_history() -> Result<() Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn multiple_blocking_stop_hooks_persist_multiple_hook_prompt_fragments() -> Result<()> { skip_if_no_network!(Ok(())); @@ -1033,7 +1033,7 @@ async fn multiple_blocking_stop_hooks_persist_multiple_hook_prompt_fragments() - Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn blocked_user_prompt_submit_persists_additional_context_for_next_turn() -> Result<()> { skip_if_no_network!(Ok(())); @@ -1116,7 +1116,7 @@ async fn blocked_user_prompt_submit_persists_additional_context_for_next_turn() Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn blocked_queued_prompt_does_not_strand_earlier_accepted_prompt() -> Result<()> { skip_if_no_network!(Ok(())); @@ -1281,7 +1281,7 @@ async fn blocked_queued_prompt_does_not_strand_earlier_accepted_prompt() -> Resu Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn permission_request_hook_allows_shell_command_without_user_approval() -> Result<()> { skip_if_no_network!(Ok(())); @@ -1360,7 +1360,7 @@ async fn permission_request_hook_allows_shell_command_without_user_approval() -> Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn permission_request_hook_allows_apply_patch_with_write_alias() -> Result<()> { skip_if_no_network!(Ok(())); @@ -1442,7 +1442,7 @@ async fn permission_request_hook_allows_apply_patch_with_write_alias() -> Result Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn permission_request_hook_sees_raw_exec_command_input() -> Result<()> { skip_if_no_network!(Ok(())); @@ -1523,7 +1523,7 @@ async fn permission_request_hook_sees_raw_exec_command_input() -> Result<()> { Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn permission_request_hook_allows_network_approval_without_prompt() -> Result<()> { skip_if_no_network!(Ok(())); @@ -1680,7 +1680,7 @@ allow_local_binding = true } #[cfg(not(target_os = "linux"))] -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn permission_request_hook_sees_retry_context_after_sandbox_denial() -> Result<()> { skip_if_no_network!(Ok(())); @@ -1750,7 +1750,7 @@ async fn permission_request_hook_sees_retry_context_after_sandbox_denial() -> Re Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn pre_tool_use_blocks_shell_command_before_execution() -> Result<()> { skip_if_no_network!(Ok(())); @@ -1852,7 +1852,7 @@ async fn pre_tool_use_blocks_shell_command_before_execution() -> Result<()> { Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn plugin_pre_tool_use_blocks_shell_command_before_execution() -> Result<()> { skip_if_no_network!(Ok(())); @@ -1996,7 +1996,7 @@ print(json.dumps({{ Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn pre_tool_use_blocks_shell_when_defined_in_config_toml() -> Result<()> { skip_if_no_network!(Ok(())); @@ -2079,7 +2079,7 @@ async fn pre_tool_use_blocks_shell_when_defined_in_config_toml() -> Result<()> { Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn pre_tool_use_merges_hooks_json_and_config_toml() -> Result<()> { skip_if_no_network!(Ok(())); @@ -2180,7 +2180,7 @@ async fn pre_tool_use_merges_hooks_json_and_config_toml() -> Result<()> { Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn pre_tool_use_blocks_local_shell_before_execution() -> Result<()> { skip_if_no_network!(Ok(())); @@ -2274,7 +2274,7 @@ async fn pre_tool_use_blocks_local_shell_before_execution() -> Result<()> { Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn pre_tool_use_blocks_exec_command_before_execution() -> Result<()> { skip_if_no_network!(Ok(())); @@ -2361,7 +2361,7 @@ async fn pre_tool_use_blocks_exec_command_before_execution() -> Result<()> { Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn pre_tool_use_blocks_apply_patch_before_execution() -> Result<()> { skip_if_no_network!(Ok(())); @@ -2438,7 +2438,7 @@ async fn pre_tool_use_blocks_apply_patch_before_execution() -> Result<()> { Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn pre_tool_use_blocks_apply_patch_with_write_alias() -> Result<()> { skip_if_no_network!(Ok(())); @@ -2513,7 +2513,7 @@ async fn pre_tool_use_blocks_apply_patch_with_write_alias() -> Result<()> { Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn pre_tool_use_does_not_fire_for_plan_tool() -> Result<()> { skip_if_no_network!(Ok(())); @@ -2585,7 +2585,7 @@ async fn pre_tool_use_does_not_fire_for_plan_tool() -> Result<()> { Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn post_tool_use_records_additional_context_for_shell_command() -> Result<()> { skip_if_no_network!(Ok(())); @@ -2682,7 +2682,7 @@ async fn post_tool_use_records_additional_context_for_shell_command() -> Result< Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn post_tool_use_block_decision_replaces_shell_command_output_with_reason() -> Result<()> { skip_if_no_network!(Ok(())); @@ -2750,7 +2750,7 @@ async fn post_tool_use_block_decision_replaces_shell_command_output_with_reason( Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn post_tool_use_continue_false_replaces_shell_command_output_with_stop_reason() -> Result<()> { skip_if_no_network!(Ok(())); @@ -2819,7 +2819,7 @@ async fn post_tool_use_continue_false_replaces_shell_command_output_with_stop_re Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn post_tool_use_records_additional_context_for_local_shell() -> Result<()> { skip_if_no_network!(Ok(())); @@ -2893,7 +2893,7 @@ async fn post_tool_use_records_additional_context_for_local_shell() -> Result<() Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn post_tool_use_exit_two_replaces_one_shot_exec_command_output_with_feedback() -> Result<()> { skip_if_no_network!(Ok(())); @@ -2968,7 +2968,7 @@ async fn post_tool_use_exit_two_replaces_one_shot_exec_command_output_with_feedb Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn post_tool_use_blocks_when_exec_session_completes_via_write_stdin() -> Result<()> { skip_if_no_network!(Ok(())); skip_if_windows!(Ok(())); @@ -3074,7 +3074,7 @@ async fn post_tool_use_blocks_when_exec_session_completes_via_write_stdin() -> R Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn post_tool_use_records_additional_context_for_apply_patch() -> Result<()> { skip_if_no_network!(Ok(())); @@ -3165,7 +3165,7 @@ async fn post_tool_use_records_additional_context_for_apply_patch() -> Result<() Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn post_tool_use_records_apply_patch_context_with_edit_alias() -> Result<()> { skip_if_no_network!(Ok(())); @@ -3238,7 +3238,7 @@ async fn post_tool_use_records_apply_patch_context_with_edit_alias() -> Result<( Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[tokio::test] async fn post_tool_use_does_not_fire_for_plan_tool() -> Result<()> { skip_if_no_network!(Ok(()));