From 2bd71f96d41809b95ea881429a1b68eb48d089b6 Mon Sep 17 00:00:00 2001 From: rgaucher-oai Date: Sat, 5 Sep 2026 11:57:24 +0000 Subject: [PATCH] Refresh session hooks after external plugin updates (#42990) ## Why Plugin updates made by another process do not notify a loaded session's hook runtime, leaving it with stale hooks. ## What changed Compare the current plugin hook sources and load warnings with those retained by the hook runtime during turn construction. Refresh hooks when either differs so existing sessions pick up external plugin updates. ## Testing Add an app-server regression test that updates the shared plugin store without notifying the server and verifies that successive turns in the same thread run the installed hooks across an upgrade and a rollback. GitOrigin-RevId: d3a2653ac4c069d9d8a02f30e21e4506d76a091c --- .../app-server/tests/suite/v2/hooks_list.rs | 98 +++++++++++++++++++ codex-rs/core/src/session/turn_context.rs | 11 +++ codex-rs/hooks/src/registry.rs | 17 +++- codex-rs/plugin/src/load_outcome.rs | 16 ++- 4 files changed, 136 insertions(+), 6 deletions(-) diff --git a/codex-rs/app-server/tests/suite/v2/hooks_list.rs b/codex-rs/app-server/tests/suite/v2/hooks_list.rs index 9807718c3c..60ee539ad3 100644 --- a/codex-rs/app-server/tests/suite/v2/hooks_list.rs +++ b/codex-rs/app-server/tests/suite/v2/hooks_list.rs @@ -26,7 +26,9 @@ use codex_app_server_protocol::TurnStartParams; use codex_app_server_protocol::TurnStartResponse; use codex_app_server_protocol::UserInput as V2UserInput; use codex_core::config::set_project_trust_level; +use codex_core_plugins::store::PluginStore; use codex_features::Feature; +use codex_plugin::PluginId; use codex_protocol::config_types::TrustLevel; use codex_utils_absolute_path::AbsolutePathBuf; use core_test_support::skip_if_host_windows; @@ -550,6 +552,102 @@ async fn plugin_upgrade_refreshes_hook_runtime_for_loaded_session() -> Result<() Ok(()) } +#[tokio::test] +async fn loaded_session_refreshes_externally_updated_plugin_hooks() -> Result<()> { + skip_if_host_windows!(Ok(())); + skip_if_remote!(Ok(()), "command hooks use host-local script and log paths"); + + let server = create_mock_responses_server_sequence_unchecked(vec![ + create_final_assistant_message_sse_response("Version 1")?, + create_final_assistant_message_sse_response("Version 2")?, + create_final_assistant_message_sse_response("Rollback")?, + ]) + .await; + let codex_home = TempDir::new()?; + let source = TempDir::new()?; + let hook_log_path = codex_home.path().join("plugin-hook-versions.log"); + let plugin_id = PluginId::parse("demo@test")?; + let store = PluginStore::new(codex_home.path().to_path_buf()); + write_versioned_plugin_hook(source.path(), "1.0.0", &hook_log_path)?; + store.install(AbsolutePathBuf::try_from(source.path())?, plugin_id.clone())?; + MockResponsesConfig::new(&server.uri()) + .enable_feature(Feature::Plugins) + .enable_feature(Feature::CodexHooks) + .write(codex_home.path())?; + let mut config = std::fs::read_to_string(codex_home.path().join("config.toml"))?; + config.push_str("\n[plugins.\"demo@test\"]\nenabled = true\n"); + std::fs::write(codex_home.path().join("config.toml"), config)?; + + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .build_initialized_with_timeout(DEFAULT_TIMEOUT) + .await?; + let list_id = mcp + .send_hooks_list_request(HooksListParams { + cwds: vec![codex_home.path().to_path_buf()], + }) + .await?; + let HooksListResponse { data } = timeout(DEFAULT_TIMEOUT, mcp.read_response(list_id)).await??; + let trusted_hooks = data[0] + .hooks + .iter() + .map(|hook| { + ( + hook.key.clone(), + serde_json::json!({ "trusted_hash": hook.current_hash }), + ) + }) + .collect::>(); + let trust_id = mcp + .send_config_batch_write_request(ConfigBatchWriteParams { + edits: vec![ConfigEdit { + key_path: "hooks.state".to_string(), + value: serde_json::Value::Object(trusted_hooks), + merge_strategy: MergeStrategy::Upsert, + }], + file_path: None, + expected_version: None, + reload_user_config: true, + }) + .await?; + let _: codex_app_server_protocol::ConfigWriteResponse = + timeout(DEFAULT_TIMEOUT, mcp.read_response(trust_id)).await??; + let start_id = mcp + .send_thread_start_request_with_auto_env(ThreadStartParams { + model: Some("mock-model".to_string()), + ..Default::default() + }) + .await?; + let ThreadStartResponse { thread, .. } = + timeout(DEFAULT_TIMEOUT, mcp.read_response(start_id)).await??; + + let mut expected_versions = String::new(); + for version in ["1.0.0", "2.0.0", "0.9.0"] { + // This test process updates the shared store without notifying app-server. + write_versioned_plugin_hook(source.path(), version, &hook_log_path)?; + store.install(AbsolutePathBuf::try_from(source.path())?, plugin_id.clone())?; + let turn_id = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id.clone(), + input: vec![V2UserInput::Text { + text: format!("run version {version}"), + text_elements: Vec::new(), + }], + ..Default::default() + }) + .await?; + let _: TurnStartResponse = timeout(DEFAULT_TIMEOUT, mcp.read_response(turn_id)).await??; + timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + expected_versions.push_str(&format!("{version}\n")); + assert_eq!(std::fs::read_to_string(&hook_log_path)?, expected_versions); + } + Ok(()) +} + #[tokio::test] async fn automatic_marketplace_upgrade_refreshes_hook_runtime_for_loaded_session() -> Result<()> { skip_if_host_windows!(Ok(())); diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index c11e556901..08a5bcf930 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -964,6 +964,17 @@ impl Session { .plugins_manager .plugins_for_config(&plugins_input) .await; + // Cache changes from another process do not notify this session's hook runtime. + if !self.hooks().matches_plugin_hooks( + plugin_outcome.iter_effective_plugin_hook_sources(), + plugin_outcome.iter_effective_plugin_hook_warnings(), + ) { + // Keep the refresh state out of the enclosing turn-construction future. + Box::pin(self.refresh_hooks(Arc::clone( + &session_configuration.original_config_do_not_use, + ))) + .await; + } let trusted_plugin_roots = TrustedPluginRoots::from_plugin_load_outcome( &plugin_outcome, per_turn_config.codex_home.as_path(), diff --git a/codex-rs/hooks/src/registry.rs b/codex-rs/hooks/src/registry.rs index b322da55d8..5a033f9c64 100644 --- a/codex-rs/hooks/src/registry.rs +++ b/codex-rs/hooks/src/registry.rs @@ -63,6 +63,8 @@ pub struct Hooks { environment: Arc>, after_agent: Vec, engine: ClaudeHooksEngine, + plugin_hook_sources: Vec, + plugin_hook_load_warnings: Vec, } impl Hooks { @@ -98,6 +100,15 @@ impl Hooks { ) } + pub fn matches_plugin_hooks<'a>( + &self, + sources: impl IntoIterator, + warnings: impl IntoIterator, + ) -> bool { + self.plugin_hook_sources.iter().eq(sources) + && self.plugin_hook_load_warnings.iter().eq(warnings) + } + pub fn with_executor_hooks(&self, executor_hooks: Vec) -> Self { let mut hooks = self.clone(); hooks.engine.set_executor_hooks(executor_hooks); @@ -124,8 +135,8 @@ impl Hooks { config.feature_enabled, config.bypass_hook_trust, config.config_layer_stack.as_ref(), - config.plugin_hook_sources, - config.plugin_hook_load_warnings, + config.plugin_hook_sources.clone(), + config.plugin_hook_load_warnings.clone(), command_runtime, mcp_executor, ); @@ -133,6 +144,8 @@ impl Hooks { environment, after_agent, engine, + plugin_hook_sources: config.plugin_hook_sources, + plugin_hook_load_warnings: config.plugin_hook_load_warnings, } } diff --git a/codex-rs/plugin/src/load_outcome.rs b/codex-rs/plugin/src/load_outcome.rs index bfc9ded339..eda5871346 100644 --- a/codex-rs/plugin/src/load_outcome.rs +++ b/codex-rs/plugin/src/load_outcome.rs @@ -169,19 +169,27 @@ impl PluginLoadOutcome { } pub fn effective_plugin_hook_sources(&self) -> Vec { + self.iter_effective_plugin_hook_sources().cloned().collect() + } + + pub fn iter_effective_plugin_hook_sources(&self) -> impl Iterator { self.plugins .iter() .filter(|plugin| plugin.is_active()) - .flat_map(|plugin| plugin.hook_sources.iter().cloned()) - .collect() + .flat_map(|plugin| plugin.hook_sources.iter()) } pub fn effective_plugin_hook_warnings(&self) -> Vec { + self.iter_effective_plugin_hook_warnings() + .cloned() + .collect() + } + + pub fn iter_effective_plugin_hook_warnings(&self) -> impl Iterator { self.plugins .iter() .filter(|plugin| plugin.is_active()) - .flat_map(|plugin| plugin.hook_load_warnings.iter().cloned()) - .collect() + .flat_map(|plugin| plugin.hook_load_warnings.iter()) } pub fn capability_summaries(&self) -> &[PluginCapabilitySummary] {