From d156f53111f967aa6a2a28681940dabc082aec67 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Thu, 28 May 2026 20:52:24 -0700 Subject: [PATCH] feat(hooks): add explicit remote hook routing --- .../src/protocol/v2/config.rs | 3 + .../src/protocol/v2/plugin.rs | 1 + .../request_processors/catalog_processor.rs | 1 + .../request_processors/config_processor.rs | 2 + .../app-server/tests/suite/v2/hooks_list.rs | 4 + codex-rs/config/src/hook_config.rs | 2 + codex-rs/config/src/hooks_tests.rs | 34 ++ .../core/src/config/config_loader_tests.rs | 1 + codex-rs/core/src/session/mod.rs | 3 + codex-rs/core/src/session/session.rs | 9 +- codex-rs/core/src/session/tests.rs | 1 + codex-rs/core/src/unified_exec/process.rs | 2 +- .../core/src/unified_exec/process_tests.rs | 6 +- codex-rs/core/tests/suite/remote_env.rs | 116 +++++++ codex-rs/exec-server/src/client.rs | 16 +- codex-rs/exec-server/src/local_process.rs | 47 ++- codex-rs/exec-server/src/process.rs | 6 +- codex-rs/exec-server/src/protocol.rs | 4 +- codex-rs/exec-server/src/remote_process.rs | 8 +- codex-rs/exec-server/tests/exec_process.rs | 55 +++- codex-rs/hooks/src/declarations.rs | 1 + codex-rs/hooks/src/engine/command_runner.rs | 293 ++++++++++++++++-- codex-rs/hooks/src/engine/discovery.rs | 9 + codex-rs/hooks/src/engine/dispatcher.rs | 1 + codex-rs/hooks/src/engine/mod.rs | 15 + codex-rs/hooks/src/engine/mod_tests.rs | 159 ++++++---- codex-rs/hooks/src/events/compact.rs | 1 + 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 | 22 +- .../src/executor_process_transport.rs | 5 +- .../tui/src/bottom_pane/hooks_browser_view.rs | 1 + codex-rs/tui/src/debug_config.rs | 1 + codex-rs/tui/src/startup_hooks_review.rs | 1 + 37 files changed, 719 insertions(+), 116 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/v2/config.rs b/codex-rs/app-server-protocol/src/protocol/v2/config.rs index e30b3012ef..aaa3cd1b32 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/config.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/config.rs @@ -435,6 +435,9 @@ pub enum ConfiguredHookHandler { #[serde(rename = "commandWindows")] #[ts(rename = "commandWindows")] command_windows: Option, + #[serde(rename = "environmentId")] + #[ts(rename = "environmentId")] + environment_id: Option, #[serde(rename = "timeoutSec")] #[ts(rename = "timeoutSec")] timeout_sec: Option, diff --git a/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs b/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs index 70d283994e..27727cee07 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs @@ -499,6 +499,7 @@ pub struct HookMetadata { pub handler_type: HookHandlerType, pub matcher: Option, pub command: Option, + pub environment_id: Option, pub timeout_sec: u64, pub status_message: Option, pub source_path: AbsolutePathBuf, diff --git a/codex-rs/app-server/src/request_processors/catalog_processor.rs b/codex-rs/app-server/src/request_processors/catalog_processor.rs index ba6c535665..6972e09277 100644 --- a/codex-rs/app-server/src/request_processors/catalog_processor.rs +++ b/codex-rs/app-server/src/request_processors/catalog_processor.rs @@ -68,6 +68,7 @@ fn hooks_to_info(hooks: &[codex_hooks::HookListEntry]) -> Vec { handler_type: hook.handler_type.into(), matcher: hook.matcher.clone(), command: hook.command.clone(), + environment_id: hook.environment_id.clone(), timeout_sec: hook.timeout_sec, status_message: hook.status_message.clone(), source_path: hook.source_path.clone(), diff --git a/codex-rs/app-server/src/request_processors/config_processor.rs b/codex-rs/app-server/src/request_processors/config_processor.rs index 5ba6d02ecb..b2b47a7fa0 100644 --- a/codex-rs/app-server/src/request_processors/config_processor.rs +++ b/codex-rs/app-server/src/request_processors/config_processor.rs @@ -515,12 +515,14 @@ fn map_hook_handler_to_api(handler: CoreHookHandlerConfig) -> ConfiguredHookHand CoreHookHandlerConfig::Command { command, command_windows, + environment_id, timeout_sec, r#async, status_message, } => ConfiguredHookHandler::Command { command, command_windows, + environment_id, timeout_sec, r#async, status_message, 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 7e0b39f0c6..b534855300 100644 --- a/codex-rs/app-server/tests/suite/v2/hooks_list.rs +++ b/codex-rs/app-server/tests/suite/v2/hooks_list.rs @@ -54,6 +54,7 @@ fn command_hook_hash( hooks: vec![codex_config::HookHandlerConfig::Command { command: command.to_string(), command_windows: None, + environment_id: None, timeout_sec: Some(timeout_sec), r#async: false, status_message: status_message.map(ToOwned::to_owned), @@ -162,6 +163,7 @@ async fn hooks_list_shows_discovered_hook() -> Result<()> { handler_type: HookHandlerType::Command, matcher: Some("Bash".to_string()), command: Some("python3 /tmp/listed-hook.py".to_string()), + environment_id: None, timeout_sec: 5, status_message: Some("running listed hook".to_string()), source_path: config_path, @@ -240,6 +242,7 @@ async fn hooks_list_shows_discovered_plugin_hook() -> Result<()> { handler_type: HookHandlerType::Command, matcher: Some("Bash".to_string()), command: Some("echo plugin hook".to_string()), + environment_id: None, timeout_sec: 7, status_message: Some("running plugin hook".to_string()), source_path: plugin_hooks_path, @@ -365,6 +368,7 @@ timeout = 5 handler_type: HookHandlerType::Command, matcher: Some("Bash".to_string()), command: Some("echo project hook".to_string()), + environment_id: None, timeout_sec: 5, status_message: None, source_path: project_config_path, diff --git a/codex-rs/config/src/hook_config.rs b/codex-rs/config/src/hook_config.rs index eee66896af..83d27e13f4 100644 --- a/codex-rs/config/src/hook_config.rs +++ b/codex-rs/config/src/hook_config.rs @@ -142,6 +142,8 @@ pub enum HookHandlerConfig { command: String, #[serde(default, rename = "commandWindows", alias = "command_windows")] command_windows: Option, + #[serde(default, rename = "environmentId", alias = "environment_id")] + environment_id: Option, #[serde(default, rename = "timeout")] timeout_sec: Option, #[serde(default)] diff --git a/codex-rs/config/src/hooks_tests.rs b/codex-rs/config/src/hooks_tests.rs index 915bec7d0c..3da5f8df81 100644 --- a/codex-rs/config/src/hooks_tests.rs +++ b/codex-rs/config/src/hooks_tests.rs @@ -41,6 +41,7 @@ fn hooks_file_deserializes_existing_json_shape() { hooks: vec![HookHandlerConfig::Command { command: "python3 /tmp/pre.py".to_string(), command_windows: None, + environment_id: None, timeout_sec: Some(10), r#async: false, status_message: Some("checking".to_string()), @@ -76,6 +77,7 @@ statusMessage = "checking" hooks: vec![HookHandlerConfig::Command { command: "python3 /tmp/pre.py".to_string(), command_windows: None, + environment_id: None, timeout_sec: Some(10), r#async: false, status_message: Some("checking".to_string()), @@ -113,6 +115,7 @@ command = "python3 /tmp/pre.py" hooks: vec![HookHandlerConfig::Command { command: "python3 /tmp/pre.py".to_string(), command_windows: None, + environment_id: None, timeout_sec: None, r#async: false, status_message: None, @@ -158,6 +161,7 @@ command = "python3 /enterprise/place/pre.py" hooks: vec![HookHandlerConfig::Command { command: "python3 /enterprise/place/pre.py".to_string(), command_windows: None, + environment_id: None, timeout_sec: None, r#async: false, status_message: None, @@ -194,6 +198,7 @@ command_windows = "powershell -File C:\\enterprise\\hooks\\pre.ps1" command_windows: Some( r"powershell -File C:\enterprise\hooks\pre.ps1".to_string(), ), + environment_id: None, timeout_sec: None, r#async: false, status_message: None, @@ -229,6 +234,7 @@ commandWindows = "powershell -File C:\\enterprise\\hooks\\pre.ps1" command_windows: Some( r"powershell -File C:\enterprise\hooks\pre.ps1".to_string(), ), + environment_id: None, timeout_sec: None, r#async: false, status_message: None, @@ -238,3 +244,31 @@ commandWindows = "powershell -File C:\\enterprise\\hooks\\pre.ps1" } ); } + +#[test] +fn hook_events_deserialize_environment_id_from_toml() { + let parsed: HookEventsToml = toml::from_str( + r#" +[[PreToolUse]] +matcher = "^Bash$" + +[[PreToolUse.hooks]] +type = "command" +command = "echo remote" +environment_id = "devbox" +"#, + ) + .expect("hook environment TOML should deserialize"); + + assert_eq!( + parsed.pre_tool_use[0].hooks, + vec![HookHandlerConfig::Command { + command: "echo remote".to_string(), + command_windows: None, + environment_id: Some("devbox".to_string()), + timeout_sec: None, + r#async: false, + status_message: None, + }] + ); +} diff --git a/codex-rs/core/src/config/config_loader_tests.rs b/codex-rs/core/src/config/config_loader_tests.rs index 2682e6ef41..1f4fcc9eaf 100644 --- a/codex-rs/core/src/config/config_loader_tests.rs +++ b/codex-rs/core/src/config/config_loader_tests.rs @@ -1754,6 +1754,7 @@ async fn load_config_layers_includes_cloud_hook_requirements() -> anyhow::Result hooks: vec![codex_config::HookHandlerConfig::Command { command: format!("python3 {}/pre.py", managed_dir.display()), command_windows: None, + environment_id: None, timeout_sec: Some(10), r#async: false, status_message: Some("checking".to_string()), diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 8c535cd728..03138009de 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -1483,6 +1483,7 @@ impl Session { config.as_ref(), self.services.plugins_manager.as_ref(), self.services.user_shell.as_ref(), + Arc::clone(&self.services.environment_manager), ) .await; @@ -3292,6 +3293,7 @@ async fn build_hooks_for_config( config: &Config, plugins_manager: &PluginsManager, user_shell: &crate::shell::Shell, + environment_manager: Arc, ) -> Hooks { let mut hook_shell_argv = user_shell.derive_exec_args("", /*use_login_shell*/ false); let hook_shell_program = hook_shell_argv.remove(0); @@ -3309,6 +3311,7 @@ async fn build_hooks_for_config( plugin_hook_load_warnings, shell_program: Some(hook_shell_program), shell_args: hook_shell_argv, + environment_manager, }) } diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index c9e1807fe6..87cd33a858 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -935,8 +935,13 @@ impl Session { (None, None) }; - let hooks = - build_hooks_for_config(&config, plugins_manager.as_ref(), &default_shell).await; + let hooks = build_hooks_for_config( + &config, + plugins_manager.as_ref(), + &default_shell, + Arc::clone(&environment_manager), + ) + .await; for warning in hooks.startup_warnings() { post_session_configured_events.push(Event { id: INITIAL_SUBMIT_ID.to_owned(), diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 62321756db..90613918cc 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -1406,6 +1406,7 @@ async fn refresh_runtime_config_refreshes_hooks() -> anyhow::Result<()> { hooks: vec![codex_config::HookHandlerConfig::Command { command: "python3 /tmp/user.py".to_string(), command_windows: None, + environment_id: None, timeout_sec: Some(600), r#async: false, status_message: None, diff --git a/codex-rs/core/src/unified_exec/process.rs b/codex-rs/core/src/unified_exec/process.rs index 9671429d01..d670a55c62 100644 --- a/codex-rs/core/src/unified_exec/process.rs +++ b/codex-rs/core/src/unified_exec/process.rs @@ -137,7 +137,7 @@ impl UnifiedExecProcess { .await .map_err(|_| UnifiedExecError::WriteToStdin), ProcessHandle::ExecServer(process_handle) => { - match process_handle.write(data.to_vec()).await { + match process_handle.write(Some(data.to_vec()), false).await { Ok(response) => match response.status { WriteStatus::Accepted => Ok(()), WriteStatus::UnknownProcess | WriteStatus::StdinClosed => { diff --git a/codex-rs/core/src/unified_exec/process_tests.rs b/codex-rs/core/src/unified_exec/process_tests.rs index ee0ea1cba3..a4d2a7ca34 100644 --- a/codex-rs/core/src/unified_exec/process_tests.rs +++ b/codex-rs/core/src/unified_exec/process_tests.rs @@ -59,7 +59,11 @@ impl ExecProcess for MockExecProcess { })) } - async fn write(&self, _chunk: Vec) -> Result { + async fn write( + &self, + _chunk: Option>, + _close_stdin: bool, + ) -> Result { Ok(self.write_response.clone()) } diff --git a/codex-rs/core/tests/suite/remote_env.rs b/codex-rs/core/tests/suite/remote_env.rs index 34cd119f66..ac838f9564 100644 --- a/codex-rs/core/tests/suite/remote_env.rs +++ b/codex-rs/core/tests/suite/remote_env.rs @@ -27,6 +27,7 @@ use codex_utils_absolute_path::AbsolutePathBuf; use core_test_support::PathBufExt; use core_test_support::PathExt; use core_test_support::get_remote_test_env; +use core_test_support::hooks::trust_discovered_hooks; use core_test_support::responses::ev_apply_patch_custom_tool_call; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; @@ -345,6 +346,121 @@ async fn exec_command_routes_to_selected_remote_environment() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn structured_hook_routes_to_explicit_remote_environment() -> Result<()> { + skip_if_no_network!(Ok(())); + let Some(_remote_env) = get_remote_test_env() else { + return Ok(()); + }; + + let server = start_mock_server().await; + let response_mock = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_function_call( + "call-1", + "exec_command", + &serde_json::to_string(&json!({ + "shell": "/bin/sh", + "cmd": "printf done", + "login": false, + "yield_time_ms": 1_000, + "environment_id": REMOTE_ENVIRONMENT_ID, + }))?, + ), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]), + ], + ) + .await; + let hook_input_path = format!( + "/tmp/codex-remote-hook-input-{}.json", + SystemTime::now().duration_since(UNIX_EPOCH)?.as_millis() + ); + let hook_input_path_for_config = hook_input_path.clone(); + let builder = test_codex() + .with_pre_build_hook(move |home| { + let hooks = json!({ + "hooks": { + "PreToolUse": [{ + "matcher": ".*", + "hooks": [{ + "type": "command", + "command": format!("cat > {hook_input_path_for_config}; printf '{{\"hookSpecificOutput\":{{\"hookEventName\":\"PreToolUse\",\"additionalContext\":\"remote-hook\"}}}}'"), + "environmentId": REMOTE_ENVIRONMENT_ID, + }] + }] + } + }); + fs::write(home.join("hooks.json"), hooks.to_string()).expect("write hooks"); + }) + .with_config(trust_discovered_hooks); + let test = builder.build_with_remote_and_local_env(&server).await?; + let remote_cwd = PathBuf::from(format!( + "/tmp/codex-remote-hook-{}", + SystemTime::now().duration_since(UNIX_EPOCH)?.as_millis() + )) + .abs(); + test.fs() + .create_directory( + &remote_cwd, + CreateDirectoryOptions { recursive: true }, + /*sandbox*/ None, + ) + .await?; + test.submit_turn_with_environments( + "run remote hook", + Some(vec![TurnEnvironmentSelection { + environment_id: REMOTE_ENVIRONMENT_ID.to_string(), + cwd: remote_cwd.clone(), + }]), + ) + .await?; + + let output = response_mock + .function_call_output_text("call-1") + .context("missing exec output")?; + assert!(output.contains("done")); + assert!( + test.fs() + .read_file_text( + &AbsolutePathBuf::from_absolute_path(&hook_input_path)?, + /*sandbox*/ None, + ) + .await? + .contains("\"hook_event_name\":\"PreToolUse\"") + ); + test.fs() + .remove( + &remote_cwd, + RemoveOptions { + recursive: true, + force: true, + }, + /*sandbox*/ None, + ) + .await?; + test.fs() + .remove( + &AbsolutePathBuf::from_absolute_path(&hook_input_path)?, + RemoveOptions { + recursive: false, + force: true, + }, + /*sandbox*/ None, + ) + .await?; + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn apply_patch_freeform_routes_to_selected_remote_environment() -> Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/exec-server/src/client.rs b/codex-rs/exec-server/src/client.rs index e06a736c17..ab8d2f77ed 100644 --- a/codex-rs/exec-server/src/client.rs +++ b/codex-rs/exec-server/src/client.rs @@ -361,13 +361,15 @@ impl ExecServerClient { pub async fn write( &self, process_id: &ProcessId, - chunk: Vec, + chunk: Option>, + close_stdin: bool, ) -> Result { self.call( EXEC_WRITE_METHOD, &WriteParams { process_id: process_id.clone(), - chunk: chunk.into(), + chunk: chunk.map(Into::into), + close_stdin, }, ) .await @@ -720,8 +722,14 @@ impl Session { } } - pub(crate) async fn write(&self, chunk: Vec) -> Result { - self.client.write(&self.process_id, chunk).await + pub(crate) async fn write( + &self, + chunk: Option>, + close_stdin: bool, + ) -> Result { + self.client + .write(&self.process_id, chunk, close_stdin) + .await } pub(crate) async fn terminate(&self) -> Result<(), ExecServerError> { diff --git a/codex-rs/exec-server/src/local_process.rs b/codex-rs/exec-server/src/local_process.rs index bc69ec6105..31efbe16cd 100644 --- a/codex-rs/exec-server/src/local_process.rs +++ b/codex-rs/exec-server/src/local_process.rs @@ -352,7 +352,11 @@ impl LocalProcess { params: WriteParams, ) -> Result { let _process_id = params.process_id.clone(); - let _input_bytes = params.chunk.0.len(); + if params.chunk.is_none() && !params.close_stdin { + return Err(invalid_params( + "process/write requires chunk, closeStdin, or both".to_string(), + )); + } let writer_tx = { let process_map = self.inner.processes.lock().await; let Some(process) = process_map.get(¶ms.process_id) else { @@ -370,13 +374,28 @@ impl LocalProcess { status: WriteStatus::StdinClosed, }); } - process.session.writer_sender() + params + .chunk + .as_ref() + .map(|_| process.session.writer_sender()) }; - writer_tx - .send(params.chunk.into_inner()) - .await - .map_err(|_| internal_error("failed to write to process stdin".to_string()))?; + if let Some(chunk) = params.chunk { + writer_tx + .ok_or_else(|| internal_error("missing process stdin writer".to_string()))? + .send(chunk.into_inner()) + .await + .map_err(|_| internal_error("failed to write to process stdin".to_string()))?; + } + if params.close_stdin { + let process_map = self.inner.processes.lock().await; + let Some(ProcessEntry::Running(process)) = process_map.get(¶ms.process_id) else { + return Ok(WriteResponse { + status: WriteStatus::UnknownProcess, + }); + }; + process.session.close_stdin(); + } Ok(WriteResponse { status: WriteStatus::Accepted, @@ -479,8 +498,14 @@ impl ExecProcess for LocalExecProcess { .await } - async fn write(&self, chunk: Vec) -> Result { - self.backend.write(&self.process_id, chunk).await + async fn write( + &self, + chunk: Option>, + close_stdin: bool, + ) -> Result { + self.backend + .write(&self.process_id, chunk, close_stdin) + .await } async fn terminate(&self) -> Result<(), ExecServerError> { @@ -509,11 +534,13 @@ impl LocalProcess { async fn write( &self, process_id: &ProcessId, - chunk: Vec, + chunk: Option>, + close_stdin: bool, ) -> Result { self.exec_write(WriteParams { process_id: process_id.clone(), - chunk: chunk.into(), + chunk: chunk.map(Into::into), + close_stdin, }) .await .map_err(map_handler_error) diff --git a/codex-rs/exec-server/src/process.rs b/codex-rs/exec-server/src/process.rs index cb6c832138..2779f2acf1 100644 --- a/codex-rs/exec-server/src/process.rs +++ b/codex-rs/exec-server/src/process.rs @@ -176,7 +176,11 @@ pub trait ExecProcess: Send + Sync { wait_ms: Option, ) -> Result; - async fn write(&self, chunk: Vec) -> Result; + async fn write( + &self, + chunk: Option>, + close_stdin: bool, + ) -> Result; async fn terminate(&self) -> Result<(), ExecServerError>; } diff --git a/codex-rs/exec-server/src/protocol.rs b/codex-rs/exec-server/src/protocol.rs index e801a7f437..00fabb0108 100644 --- a/codex-rs/exec-server/src/protocol.rs +++ b/codex-rs/exec-server/src/protocol.rs @@ -127,7 +127,9 @@ pub struct ReadResponse { #[serde(rename_all = "camelCase")] pub struct WriteParams { pub process_id: ProcessId, - pub chunk: ByteChunk, + pub chunk: Option, + #[serde(default)] + pub close_stdin: bool, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] diff --git a/codex-rs/exec-server/src/remote_process.rs b/codex-rs/exec-server/src/remote_process.rs index d8d06735cd..185ca17c57 100644 --- a/codex-rs/exec-server/src/remote_process.rs +++ b/codex-rs/exec-server/src/remote_process.rs @@ -71,9 +71,13 @@ impl ExecProcess for RemoteExecProcess { self.session.read(after_seq, max_bytes, wait_ms).await } - async fn write(&self, chunk: Vec) -> Result { + async fn write( + &self, + chunk: Option>, + close_stdin: bool, + ) -> Result { trace!("exec process write"); - self.session.write(chunk).await + self.session.write(chunk, close_stdin).await } async fn terminate(&self) -> Result<(), ExecServerError> { diff --git a/codex-rs/exec-server/tests/exec_process.rs b/codex-rs/exec-server/tests/exec_process.rs index e1f330fc4e..c42175e83c 100644 --- a/codex-rs/exec-server/tests/exec_process.rs +++ b/codex-rs/exec-server/tests/exec_process.rs @@ -424,7 +424,10 @@ async fn assert_exec_process_write_then_read(use_remote: bool) -> Result<()> { assert_eq!(session.process.process_id().as_str(), process_id); tokio::time::sleep(Duration::from_millis(200)).await; - session.process.write(b"hello\n".to_vec()).await?; + session + .process + .write(Some(b"hello\n".to_vec()), false) + .await?; let StartedExecProcess { process } = session; let wake_rx = process.subscribe_wake(); let (output, exit_code, closed) = collect_process_output_from_reads(process, wake_rx).await?; @@ -461,7 +464,10 @@ async fn assert_exec_process_write_then_read_without_tty(use_remote: bool) -> Re assert_eq!(session.process.process_id().as_str(), process_id); tokio::time::sleep(Duration::from_millis(200)).await; - let write_response = session.process.write(b"hello\n".to_vec()).await?; + let write_response = session + .process + .write(Some(b"hello\n".to_vec()), false) + .await?; assert_eq!(write_response.status, WriteStatus::Accepted); let StartedExecProcess { process } = session; let wake_rx = process.subscribe_wake(); @@ -471,6 +477,36 @@ async fn assert_exec_process_write_then_read_without_tty(use_remote: bool) -> Re Ok(()) } +async fn assert_exec_process_write_can_close_stdin(use_remote: bool) -> Result<()> { + let context = create_process_context(use_remote).await?; + let session = context + .backend + .start(ExecParams { + process_id: ProcessId::from("proc-stdin-eof"), + argv: vec![ + "/bin/sh".to_string(), + "-c".to_string(), + "cat; printf ':eof\\n'".to_string(), + ], + cwd: std::env::current_dir()?, + env_policy: None, + env: Default::default(), + tty: false, + pipe_stdin: true, + arg0: None, + }) + .await?; + + let write_response = session.process.write(Some(b"hello".to_vec()), true).await?; + assert_eq!(write_response.status, WriteStatus::Accepted); + let StartedExecProcess { process } = session; + let wake_rx = process.subscribe_wake(); + let actual = collect_process_output_from_reads(process, wake_rx).await?; + + assert_eq!(actual, ("hello:eof\n".to_string(), Some(0), true)); + Ok(()) +} + async fn assert_exec_process_rejects_write_without_pipe_stdin(use_remote: bool) -> Result<()> { let context = create_process_context(use_remote).await?; let process_id = "proc-stdin-closed".to_string(); @@ -493,7 +529,10 @@ async fn assert_exec_process_rejects_write_without_pipe_stdin(use_remote: bool) .await?; assert_eq!(session.process.process_id().as_str(), process_id); - let write_response = session.process.write(b"ignored\n".to_vec()).await?; + let write_response = session + .process + .write(Some(b"ignored\n".to_vec()), false) + .await?; assert_eq!(write_response.status, WriteStatus::StdinClosed); let StartedExecProcess { process } = session; let wake_rx = process.subscribe_wake(); @@ -613,7 +652,7 @@ async fn remote_exec_process_reports_transport_disconnect() -> Result<()> { let write_result = timeout( Duration::from_secs(2), - session.process.write(b"hello".to_vec()), + session.process.write(Some(b"hello".to_vec()), false), ) .await .context("timed out waiting for write after disconnect")?; @@ -693,6 +732,14 @@ async fn exec_process_write_then_read_without_tty(use_remote: bool) -> Result<() assert_exec_process_write_then_read_without_tty(use_remote).await } +#[test_case(false ; "local")] +#[test_case(true ; "remote")] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[serial_test::serial(remote_exec_server)] +async fn exec_process_write_can_close_stdin(use_remote: bool) -> Result<()> { + assert_exec_process_write_can_close_stdin(use_remote).await +} + #[test_case(false ; "local")] #[test_case(true ; "remote")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] diff --git a/codex-rs/hooks/src/declarations.rs b/codex-rs/hooks/src/declarations.rs index 468fbc947a..b3a182c503 100644 --- a/codex-rs/hooks/src/declarations.rs +++ b/codex-rs/hooks/src/declarations.rs @@ -66,6 +66,7 @@ mod tests { HookHandlerConfig::Command { command: "echo hi".to_string(), command_windows: None, + environment_id: None, timeout_sec: None, r#async: false, status_message: None, diff --git a/codex-rs/hooks/src/engine/command_runner.rs b/codex-rs/hooks/src/engine/command_runner.rs index 7366d4ec51..8b0b11eb2a 100644 --- a/codex-rs/hooks/src/engine/command_runner.rs +++ b/codex-rs/hooks/src/engine/command_runner.rs @@ -3,6 +3,11 @@ use std::process::Stdio; use std::time::Duration; use std::time::Instant; +use codex_exec_server::ExecOutputStream; +use codex_exec_server::ExecParams; +use codex_exec_server::ExecProcess; +use codex_exec_server::ProcessId; +use codex_exec_server::WriteStatus; use tokio::io::AsyncWriteExt; use tokio::process::Command; use tokio::time::timeout; @@ -29,6 +34,18 @@ pub(crate) async fn run_command( ) -> CommandRunResult { let started_at = chrono::Utc::now().timestamp(); let started = Instant::now(); + if let Some(environment_id) = handler.environment_id.as_deref() { + let Some(environment) = shell.environment_manager.get_environment(environment_id) else { + return command_error( + started_at, + started, + format!("unknown hook environment id: {environment_id}"), + ); + }; + if environment.is_remote() { + return run_remote_command(handler, environment, input_json, cwd).await; + } + } let mut command = build_command(shell, handler); command @@ -41,15 +58,7 @@ pub(crate) async fn run_command( let mut child = match command.spawn() { Ok(child) => child, Err(err) => { - return CommandRunResult { - started_at, - completed_at: chrono::Utc::now().timestamp(), - duration_ms: started.elapsed().as_millis().try_into().unwrap_or(i64::MAX), - exit_code: None, - stdout: String::new(), - stderr: String::new(), - error: Some(err.to_string()), - }; + return command_error(started_at, started, err.to_string()); } }; @@ -57,15 +66,11 @@ pub(crate) async fn run_command( && let Err(err) = stdin.write_all(input_json.as_bytes()).await { let _ = child.kill().await; - return CommandRunResult { + return command_error( started_at, - completed_at: chrono::Utc::now().timestamp(), - duration_ms: started.elapsed().as_millis().try_into().unwrap_or(i64::MAX), - exit_code: None, - stdout: String::new(), - stderr: String::new(), - error: Some(format!("failed to write hook stdin: {err}")), - }; + started, + format!("failed to write hook stdin: {err}"), + ); } let timeout_duration = Duration::from_secs(handler.timeout_sec); @@ -73,25 +78,17 @@ pub(crate) async fn run_command( Ok(Ok(output)) => CommandRunResult { started_at, completed_at: chrono::Utc::now().timestamp(), - duration_ms: started.elapsed().as_millis().try_into().unwrap_or(i64::MAX), + duration_ms: elapsed_ms(started), exit_code: output.status.code(), stdout: String::from_utf8_lossy(&output.stdout).to_string(), stderr: String::from_utf8_lossy(&output.stderr).to_string(), error: None, }, - Ok(Err(err)) => CommandRunResult { - started_at, - completed_at: chrono::Utc::now().timestamp(), - duration_ms: started.elapsed().as_millis().try_into().unwrap_or(i64::MAX), - exit_code: None, - stdout: String::new(), - stderr: String::new(), - error: Some(err.to_string()), - }, + Ok(Err(err)) => command_error(started_at, started, err.to_string()), Err(_) => CommandRunResult { started_at, completed_at: chrono::Utc::now().timestamp(), - duration_ms: started.elapsed().as_millis().try_into().unwrap_or(i64::MAX), + duration_ms: elapsed_ms(started), exit_code: None, stdout: String::new(), stderr: String::new(), @@ -100,6 +97,118 @@ pub(crate) async fn run_command( } } +async fn run_remote_command( + handler: &ConfiguredHandler, + environment: std::sync::Arc, + input_json: &str, + cwd: &Path, +) -> CommandRunResult { + let started_at = chrono::Utc::now().timestamp(); + let started = Instant::now(); + let process = match environment + .get_exec_backend() + .start(ExecParams { + process_id: ProcessId::new(format!("hook-{}", uuid::Uuid::new_v4())), + argv: default_shell_argv(handler), + cwd: cwd.to_path_buf(), + env_policy: None, + env: handler.env.clone(), + tty: false, + pipe_stdin: true, + arg0: None, + }) + .await + { + Ok(started) => started.process, + Err(err) => return command_error(started_at, started, err.to_string()), + }; + if let Err(error) = write_remote_stdin(&process, input_json).await { + let _ = process.terminate().await; + return command_error(started_at, started, error); + } + + match timeout( + Duration::from_secs(handler.timeout_sec), + collect_output(std::sync::Arc::clone(&process)), + ) + .await + { + Ok(Ok((stdout, stderr, exit_code))) => CommandRunResult { + started_at, + completed_at: chrono::Utc::now().timestamp(), + duration_ms: elapsed_ms(started), + exit_code, + stdout, + stderr, + error: None, + }, + Ok(Err(error)) => { + let _ = process.terminate().await; + command_error(started_at, started, error) + } + Err(_) => { + let _ = process.terminate().await; + CommandRunResult { + started_at, + completed_at: chrono::Utc::now().timestamp(), + duration_ms: elapsed_ms(started), + exit_code: None, + stdout: String::new(), + stderr: String::new(), + error: Some(format!("hook timed out after {}s", handler.timeout_sec)), + } + } + } +} + +async fn write_remote_stdin( + process: &std::sync::Arc, + input_json: &str, +) -> Result<(), String> { + let response = process + .write(Some(input_json.as_bytes().to_vec()), true) + .await + .map_err(|err| format!("failed to write hook stdin: {err}"))?; + if response.status == WriteStatus::Accepted { + Ok(()) + } else { + Err(format!("failed to write hook stdin: {:?}", response.status)) + } +} + +async fn collect_output( + process: std::sync::Arc, +) -> Result<(String, String, Option), String> { + let mut stdout = Vec::new(); + let mut stderr = Vec::new(); + let mut after_seq = None; + loop { + let response = process + .read(after_seq, None, Some(50)) + .await + .map_err(|err| err.to_string())?; + for chunk in response.chunks { + match chunk.stream { + ExecOutputStream::Stdout | ExecOutputStream::Pty => { + stdout.extend_from_slice(&chunk.chunk.0); + } + ExecOutputStream::Stderr => stderr.extend_from_slice(&chunk.chunk.0), + } + } + if let Some(failure) = response.failure { + return Err(failure); + } + if response.closed { + return Ok(( + String::from_utf8_lossy(&stdout).to_string(), + String::from_utf8_lossy(&stderr).to_string(), + response.exit_code, + )); + } + after_seq = response.next_seq.checked_sub(1); + } +} + fn build_command(shell: &CommandShell, handler: &ConfiguredHandler) -> Command { let mut command = if shell.program.is_empty() { default_shell_command() @@ -116,6 +225,40 @@ fn build_command(shell: &CommandShell, handler: &ConfiguredHandler) -> Command { command } +#[cfg(windows)] +fn default_shell_argv(handler: &ConfiguredHandler) -> Vec { + vec![ + "cmd.exe".to_string(), + "/C".to_string(), + handler.command.clone(), + ] +} + +#[cfg(not(windows))] +fn default_shell_argv(handler: &ConfiguredHandler) -> Vec { + vec![ + "/bin/sh".to_string(), + "-lc".to_string(), + handler.command.clone(), + ] +} + +fn command_error(started_at: i64, started: Instant, error: String) -> CommandRunResult { + CommandRunResult { + started_at, + completed_at: chrono::Utc::now().timestamp(), + duration_ms: elapsed_ms(started), + exit_code: None, + stdout: String::new(), + stderr: String::new(), + error: Some(error), + } +} + +fn elapsed_ms(started: Instant) -> i64 { + started.elapsed().as_millis().try_into().unwrap_or(i64::MAX) +} + fn default_shell_command() -> Command { #[cfg(windows)] { @@ -133,3 +276,97 @@ fn default_shell_command() -> Command { command } } + +#[cfg(test)] +mod tests { + use codex_exec_server::EnvironmentManager; + use codex_protocol::protocol::HookEventName; + use codex_protocol::protocol::HookSource; + use codex_utils_absolute_path::test_support::PathBufExt; + use codex_utils_absolute_path::test_support::test_path_buf; + use pretty_assertions::assert_eq; + + use super::CommandShell; + use super::ConfiguredHandler; + use super::run_command; + + fn shell(environment_manager: std::sync::Arc) -> CommandShell { + CommandShell { + program: String::new(), + args: Vec::new(), + environment_manager, + } + } + + fn handler(command: &str, environment_id: Option<&str>) -> ConfiguredHandler { + ConfiguredHandler { + event_name: HookEventName::PreToolUse, + matcher: None, + command: command.to_string(), + environment_id: environment_id.map(str::to_string), + timeout_sec: 5, + status_message: None, + source_path: test_path_buf("/tmp/hooks.json").abs(), + source: HookSource::User, + display_order: 0, + env: Default::default(), + } + } + + #[tokio::test] + async fn omitted_environment_id_runs_locally() { + let result = run_command( + &shell(std::sync::Arc::new(EnvironmentManager::default_for_tests())), + &handler("printf local-hook", None), + "{}", + test_path_buf("/tmp").as_path(), + ) + .await; + + assert_eq!(result.exit_code, Some(0)); + assert_eq!(result.stdout, "local-hook"); + assert_eq!(result.error, None); + } + + #[tokio::test] + async fn unknown_environment_id_does_not_fall_back_to_local() { + let result = run_command( + &shell(std::sync::Arc::new(EnvironmentManager::default_for_tests())), + &handler("printf local-hook", Some("missing")), + "{}", + test_path_buf("/tmp").as_path(), + ) + .await; + + assert_eq!(result.exit_code, None); + assert_eq!(result.stdout, ""); + assert_eq!( + result.error.as_deref(), + Some("unknown hook environment id: missing") + ); + } + + #[tokio::test] + async fn explicit_remote_environment_uses_exec_backend() { + let environment_manager = std::sync::Arc::new(EnvironmentManager::default_for_tests()); + environment_manager + .upsert_environment("remote-hook".to_string(), "ws://127.0.0.1:1".to_string()) + .expect("remote hook environment"); + let result = run_command( + &shell(environment_manager), + &handler("printf local-hook", Some("remote-hook")), + "{}", + test_path_buf("/tmp").as_path(), + ) + .await; + + assert_eq!(result.exit_code, None); + assert_eq!(result.stdout, ""); + assert!( + result + .error + .as_deref() + .is_some_and(|error| error.contains("failed")) + ); + } +} diff --git a/codex-rs/hooks/src/engine/discovery.rs b/codex-rs/hooks/src/engine/discovery.rs index 3ddf8f2387..d9e57e2006 100644 --- a/codex-rs/hooks/src/engine/discovery.rs +++ b/codex-rs/hooks/src/engine/discovery.rs @@ -431,6 +431,7 @@ fn append_matcher_groups( HookHandlerConfig::Command { command, command_windows, + environment_id, timeout_sec, r#async, status_message, @@ -458,6 +459,7 @@ fn append_matcher_groups( let normalized_handler = HookHandlerConfig::Command { command: command.clone(), command_windows: None, + environment_id: environment_id.clone(), timeout_sec: Some(timeout_sec), r#async, status_message: status_message.clone(), @@ -481,6 +483,7 @@ fn append_matcher_groups( handler_type: HookHandlerType::Command, matcher: matcher.map(ToOwned::to_owned), command: Some(command.clone()), + environment_id: environment_id.clone(), timeout_sec, status_message: status_message.clone(), source_path: source.path.clone(), @@ -503,6 +506,7 @@ fn append_matcher_groups( event_name, matcher: matcher.map(ToOwned::to_owned), command, + environment_id, timeout_sec, status_message, source_path: source.path.clone(), @@ -678,6 +682,7 @@ mod tests { hooks: vec![HookHandlerConfig::Command { command: "echo hello".to_string(), command_windows: None, + environment_id: None, timeout_sec: None, r#async: false, status_message: None, @@ -710,6 +715,7 @@ mod tests { event_name: HookEventName::UserPromptSubmit, matcher: None, command: "echo hello".to_string(), + environment_id: None, timeout_sec: 600, status_message: None, source_path: source_path.clone(), @@ -745,6 +751,7 @@ mod tests { event_name: HookEventName::PreToolUse, matcher: Some("^Bash$".to_string()), command: "echo hello".to_string(), + environment_id: None, timeout_sec: 600, status_message: None, source_path: source_path.clone(), @@ -891,6 +898,7 @@ mod tests { hooks: vec![HookHandlerConfig::Command { command: "echo hello".to_string(), command_windows: None, + environment_id: None, timeout_sec: None, r#async: false, status_message: None, @@ -921,6 +929,7 @@ mod tests { hooks: vec![HookHandlerConfig::Command { command: "echo unix".to_string(), command_windows: Some("echo windows".to_string()), + environment_id: None, timeout_sec: None, r#async: false, status_message: None, diff --git a/codex-rs/hooks/src/engine/dispatcher.rs b/codex-rs/hooks/src/engine/dispatcher.rs index 50822bfc96..493b237993 100644 --- a/codex-rs/hooks/src/engine/dispatcher.rs +++ b/codex-rs/hooks/src/engine/dispatcher.rs @@ -174,6 +174,7 @@ mod tests { event_name, matcher: matcher.map(str::to_owned), command: command.to_string(), + environment_id: None, timeout_sec: 5, status_message: None, source_path: test_path_buf("/tmp/hooks.json").abs(), diff --git a/codex-rs/hooks/src/engine/mod.rs b/codex-rs/hooks/src/engine/mod.rs index 859fc54069..374cc2c364 100644 --- a/codex-rs/hooks/src/engine/mod.rs +++ b/codex-rs/hooks/src/engine/mod.rs @@ -22,6 +22,7 @@ use crate::events::user_prompt_submit::UserPromptSubmitOutcome; use crate::events::user_prompt_submit::UserPromptSubmitRequest; use crate::output_spill::HookOutputSpiller; use codex_config::ConfigLayerStack; +use codex_exec_server::EnvironmentManager; use codex_plugin::PluginHookSource; use codex_protocol::ThreadId; use codex_protocol::protocol::HookEventName; @@ -31,11 +32,13 @@ use codex_protocol::protocol::HookSource; use codex_protocol::protocol::HookTrustStatus; use codex_utils_absolute_path::AbsolutePathBuf; use std::collections::HashMap; +use std::sync::Arc; #[derive(Debug, Clone)] pub(crate) struct CommandShell { pub program: String, pub args: Vec, + pub environment_manager: Arc, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -43,6 +46,7 @@ pub(crate) struct ConfiguredHandler { pub event_name: codex_protocol::protocol::HookEventName, pub matcher: Option, pub command: String, + pub environment_id: Option, pub timeout_sec: u64, pub status_message: Option, pub source_path: AbsolutePathBuf, @@ -84,6 +88,7 @@ pub struct HookListEntry { pub handler_type: HookHandlerType, pub matcher: Option, pub command: Option, + pub environment_id: Option, pub timeout_sec: u64, pub status_message: Option, pub source_path: AbsolutePathBuf, @@ -121,6 +126,16 @@ impl ClaudeHooksEngine { output_spiller: HookOutputSpiller::new(), }; } + if shell.environment_manager.try_local_environment().is_none() { + return Self { + handlers: Vec::new(), + warnings: vec![ + "structured command hooks require a local execution environment".to_string(), + ], + shell, + output_spiller: HookOutputSpiller::new(), + }; + } let _ = schema_loader::generated_hook_schemas(); let discovered = discovery::discover_handlers( diff --git a/codex-rs/hooks/src/engine/mod_tests.rs b/codex-rs/hooks/src/engine/mod_tests.rs index 9b642503a0..738ff2ddde 100644 --- a/codex-rs/hooks/src/engine/mod_tests.rs +++ b/codex-rs/hooks/src/engine/mod_tests.rs @@ -17,6 +17,7 @@ use codex_config::MatcherGroup; use codex_config::RequirementSource; use codex_config::Sourced; use codex_config::TomlValue; +use codex_exec_server::EnvironmentManager; use codex_plugin::PluginHookSource; use codex_plugin::PluginId; use codex_protocol::ThreadId; @@ -36,6 +37,14 @@ fn cwd() -> AbsolutePathBuf { AbsolutePathBuf::current_dir().expect("current dir") } +fn shell() -> CommandShell { + CommandShell { + program: String::new(), + args: Vec::new(), + environment_manager: std::sync::Arc::new(EnvironmentManager::default_for_tests()), + } +} + fn managed_hooks_for_current_platform( managed_dir: impl AsRef, hooks: HookEventsToml, @@ -63,6 +72,7 @@ fn pre_tool_use_hook_events(command: impl Into) -> HookEventsToml { hooks: vec![HookHandlerConfig::Command { command: command.into(), command_windows: None, + environment_id: None, timeout_sec: Some(10), r#async: false, status_message: Some("checking".to_string()), @@ -170,6 +180,7 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle: hooks: vec![HookHandlerConfig::Command { command: format!("python3 {}", script_path.display()), command_windows: None, + environment_id: None, timeout_sec: Some(10), r#async: false, status_message: Some("checking".to_string()), @@ -200,10 +211,7 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle: Some(&config_layer_stack), Vec::new(), Vec::new(), - CommandShell { - program: String::new(), - args: Vec::new(), - }, + shell(), ); assert!(engine.warnings().is_empty()); @@ -273,6 +281,7 @@ async fn requirements_managed_hooks_execute_windows_command_override() { hooks: vec![HookHandlerConfig::Command { command: "exit 17".to_string(), command_windows: Some("exit /B 19".to_string()), + environment_id: None, timeout_sec: Some(10), r#async: false, status_message: Some("checking".to_string()), @@ -303,10 +312,7 @@ async fn requirements_managed_hooks_execute_windows_command_override() { Some(&config_layer_stack), Vec::new(), Vec::new(), - CommandShell { - program: String::new(), - args: Vec::new(), - }, + shell(), ); let outcome = engine @@ -352,6 +358,7 @@ fn unknown_requirement_source_hooks_stay_managed() { hooks: vec![HookHandlerConfig::Command { command: "python3 /tmp/managed.py".to_string(), command_windows: None, + environment_id: None, timeout_sec: Some(10), r#async: false, status_message: Some("checking".to_string()), @@ -382,10 +389,7 @@ fn unknown_requirement_source_hooks_stay_managed() { Some(&config_layer_stack), Vec::new(), Vec::new(), - CommandShell { - program: String::new(), - args: Vec::new(), - }, + shell(), ); assert_eq!(engine.handlers.len(), 1); @@ -420,6 +424,7 @@ fn user_disablement_filters_non_managed_hooks_but_not_managed_hooks() { hooks: vec![HookHandlerConfig::Command { command: "python3 /tmp/managed.py".to_string(), command_windows: None, + environment_id: None, timeout_sec: Some(10), r#async: false, status_message: Some("checking".to_string()), @@ -464,10 +469,7 @@ fn user_disablement_filters_non_managed_hooks_but_not_managed_hooks() { Some(&config_layer_stack), Vec::new(), Vec::new(), - CommandShell { - program: String::new(), - args: Vec::new(), - }, + shell(), ); assert_eq!(engine.handlers.len(), 1); @@ -527,10 +529,7 @@ fn user_disablement_does_not_filter_managed_layer_hooks() { Some(&config_layer_stack), Vec::new(), Vec::new(), - CommandShell { - program: String::new(), - args: Vec::new(), - }, + shell(), ); assert_eq!(engine.handlers.len(), 1); @@ -658,6 +657,7 @@ fn requirements_managed_hooks_load_when_managed_dir_is_missing() { hooks: vec![HookHandlerConfig::Command { command: "echo hi".to_string(), command_windows: None, + environment_id: None, timeout_sec: Some(10), r#async: false, status_message: Some("checking".to_string()), @@ -688,10 +688,7 @@ fn requirements_managed_hooks_load_when_managed_dir_is_missing() { Some(&config_layer_stack), Vec::new(), Vec::new(), - CommandShell { - program: String::new(), - args: Vec::new(), - }, + shell(), ); assert!(engine.warnings().is_empty()); @@ -744,10 +741,7 @@ fn allow_managed_hooks_only_false_keeps_unmanaged_hooks() { Some(&config_layer_stack), Vec::new(), Vec::new(), - CommandShell { - program: String::new(), - args: Vec::new(), - }, + shell(), ); assert!(engine.warnings().is_empty()); @@ -798,10 +792,7 @@ fn allow_managed_hooks_only_in_config_toml_does_not_enable_policy() { Some(&config_layer_stack), Vec::new(), Vec::new(), - CommandShell { - program: String::new(), - args: Vec::new(), - }, + shell(), ); assert!(engine.warnings().is_empty()); @@ -868,10 +859,7 @@ fn allow_managed_hooks_only_skips_unmanaged_json_and_toml_hooks() { Some(&config_layer_stack), Vec::new(), Vec::new(), - CommandShell { - program: String::new(), - args: Vec::new(), - }, + shell(), ); assert!(engine.handlers.is_empty()); @@ -907,10 +895,7 @@ fn allow_managed_hooks_only_skips_unmanaged_plugin_hooks() { Some(&config_layer_stack), plugin_hook_sources, Vec::new(), - CommandShell { - program: String::new(), - args: Vec::new(), - }, + shell(), ); assert!(engine.handlers.is_empty()); @@ -979,10 +964,7 @@ fn allow_managed_hooks_only_keeps_managed_requirement_and_config_layer_hooks() { Some(&config_layer_stack), Vec::new(), Vec::new(), - CommandShell { - program: String::new(), - args: Vec::new(), - }, + shell(), ); assert!(engine.warnings().is_empty()); @@ -1089,10 +1071,7 @@ fn discovers_hooks_from_json_and_toml_in_the_same_layer() { Some(&config_layer_stack), Vec::new(), Vec::new(), - CommandShell { - program: String::new(), - args: Vec::new(), - }, + shell(), ); assert!(engine.warnings().iter().any(|warning| { @@ -1164,6 +1143,7 @@ print(json.dumps({ hooks: vec![HookHandlerConfig::Command { command: format!("python3 {}", script_path.display()), command_windows: None, + environment_id: None, timeout_sec: Some(10), r#async: false, status_message: None, @@ -1182,10 +1162,7 @@ print(json.dumps({ Some(&config_layer_stack), plugin_hook_sources.clone(), Vec::new(), - CommandShell { - program: String::new(), - args: Vec::new(), - }, + shell(), ); let preview = engine.preview_pre_tool_use(&PreToolUseRequest { @@ -1278,6 +1255,7 @@ fn plugin_hook_sources_expand_plugin_placeholders() { "run ${PLUGIN_ROOT} ${CLAUDE_PLUGIN_ROOT} ${PLUGIN_DATA} ${CLAUDE_PLUGIN_DATA}" .to_string(), command_windows: None, + environment_id: None, timeout_sec: Some(5), r#async: false, status_message: None, @@ -1296,10 +1274,7 @@ fn plugin_hook_sources_expand_plugin_placeholders() { Some(&config_layer_stack), plugin_hook_sources, Vec::new(), - CommandShell { - program: String::new(), - args: Vec::new(), - }, + shell(), ); assert_eq!( @@ -1340,11 +1315,75 @@ fn plugin_hook_load_warnings_are_startup_warnings() { /*config_layer_stack*/ None, Vec::new(), vec!["failed plugin hook".to_string()], - CommandShell { - program: String::new(), - args: Vec::new(), - }, + shell(), ); assert_eq!(engine.warnings(), &["failed plugin hook".to_string()]); } + +#[test] +fn structured_hooks_require_local_environment() { + let engine = ClaudeHooksEngine::new( + /*enabled*/ true, + /*bypass_hook_trust*/ false, + /*config_layer_stack*/ None, + Vec::new(), + Vec::new(), + CommandShell { + program: String::new(), + args: Vec::new(), + environment_manager: std::sync::Arc::new(EnvironmentManager::without_environments()), + }, + ); + + assert!(engine.handlers.is_empty()); + assert_eq!( + engine.warnings(), + ["structured command hooks require a local execution environment"] + ); +} + +#[test] +fn explicit_environment_id_is_listed_and_discovered() { + let config = serde_json::from_value(serde_json::json!({ + "hooks": { + "PreToolUse": [{ + "hooks": [{ + "type": "command", + "command": "echo remote", + "environmentId": "devbox", + }], + }], + }, + })) + .expect("hooks config should deserialize"); + let config_layer_stack = ConfigLayerStack::new( + vec![ConfigLayerEntry::new( + ConfigLayerSource::User { + file: cwd().join("config.toml"), + profile: None, + }, + config, + )], + ConfigRequirements::default(), + ConfigRequirementsToml::default(), + ) + .expect("config layer stack"); + let engine = ClaudeHooksEngine::new( + /*enabled*/ true, + /*bypass_hook_trust*/ true, + Some(&config_layer_stack), + Vec::new(), + Vec::new(), + shell(), + ); + let listed = crate::list_hooks(crate::HooksConfig { + feature_enabled: true, + bypass_hook_trust: true, + config_layer_stack: Some(config_layer_stack), + ..Default::default() + }); + + assert_eq!(engine.handlers[0].environment_id.as_deref(), Some("devbox")); + assert_eq!(listed.hooks[0].environment_id.as_deref(), Some("devbox")); +} diff --git a/codex-rs/hooks/src/events/compact.rs b/codex-rs/hooks/src/events/compact.rs index cb3080219a..d7503f5a90 100644 --- a/codex-rs/hooks/src/events/compact.rs +++ b/codex-rs/hooks/src/events/compact.rs @@ -598,6 +598,7 @@ mod tests { event_name, matcher: None, command: "python3 compact_hook.py".to_string(), + environment_id: None, timeout_sec: 5, status_message: Some("running compact hook".to_string()), source_path: test_path_buf("/tmp/hooks.json").abs(), diff --git a/codex-rs/hooks/src/events/post_tool_use.rs b/codex-rs/hooks/src/events/post_tool_use.rs index f096de0110..8b132f48c1 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 { event_name: HookEventName::PostToolUse, matcher: Some("^Bash$".to_string()), command: "python3 post_tool_use_hook.py".to_string(), + environment_id: None, timeout_sec: 5, status_message: Some("running post tool use hook".to_string()), source_path: test_path_buf("/tmp/hooks.json").abs(), diff --git a/codex-rs/hooks/src/events/pre_tool_use.rs b/codex-rs/hooks/src/events/pre_tool_use.rs index b3579aba82..23ce8e3cec 100644 --- a/codex-rs/hooks/src/events/pre_tool_use.rs +++ b/codex-rs/hooks/src/events/pre_tool_use.rs @@ -743,6 +743,7 @@ mod tests { event_name: HookEventName::PreToolUse, matcher: Some("^Bash$".to_string()), command: "echo hook".to_string(), + environment_id: None, timeout_sec: 5, status_message: None, source_path: test_path_buf("/tmp/hooks.json").abs(), diff --git a/codex-rs/hooks/src/events/session_start.rs b/codex-rs/hooks/src/events/session_start.rs index bd1aa2096f..5679ccd957 100644 --- a/codex-rs/hooks/src/events/session_start.rs +++ b/codex-rs/hooks/src/events/session_start.rs @@ -517,6 +517,7 @@ mod tests { event_name, matcher: None, command: "echo hook".to_string(), + environment_id: None, timeout_sec: 600, status_message: None, source_path: test_path_buf("/tmp/hooks.json").abs(), diff --git a/codex-rs/hooks/src/events/stop.rs b/codex-rs/hooks/src/events/stop.rs index 24920c569b..7036b0a09c 100644 --- a/codex-rs/hooks/src/events/stop.rs +++ b/codex-rs/hooks/src/events/stop.rs @@ -633,6 +633,7 @@ mod tests { event_name: HookEventName::Stop, matcher: None, command: "echo hook".to_string(), + environment_id: None, timeout_sec: 600, status_message: None, source_path: test_path_buf("/tmp/hooks.json").abs(), diff --git a/codex-rs/hooks/src/events/user_prompt_submit.rs b/codex-rs/hooks/src/events/user_prompt_submit.rs index 2934bd3523..9d3f6b33bf 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 { event_name: HookEventName::UserPromptSubmit, matcher: None, command: "echo hook".to_string(), + environment_id: None, timeout_sec: 5, status_message: None, source_path: test_path_buf("/tmp/hooks.json").abs(), diff --git a/codex-rs/hooks/src/registry.rs b/codex-rs/hooks/src/registry.rs index 1f4e01aa59..11e852fea7 100644 --- a/codex-rs/hooks/src/registry.rs +++ b/codex-rs/hooks/src/registry.rs @@ -1,5 +1,7 @@ use codex_config::ConfigLayerStack; +use codex_exec_server::EnvironmentManager; use codex_plugin::PluginHookSource; +use std::sync::Arc; use tokio::process::Command; use crate::engine::ClaudeHooksEngine; @@ -26,7 +28,7 @@ use crate::types::HookEvent; use crate::types::HookPayload; use crate::types::HookResponse; -#[derive(Default, Clone)] +#[derive(Clone)] pub struct HooksConfig { pub legacy_notify_argv: Option>, pub feature_enabled: bool, @@ -36,6 +38,23 @@ pub struct HooksConfig { pub plugin_hook_load_warnings: Vec, pub shell_program: Option, pub shell_args: Vec, + pub environment_manager: Arc, +} + +impl Default for HooksConfig { + fn default() -> Self { + Self { + legacy_notify_argv: None, + feature_enabled: false, + bypass_hook_trust: false, + config_layer_stack: None, + plugin_hook_sources: Vec::new(), + plugin_hook_load_warnings: Vec::new(), + shell_program: None, + shell_args: Vec::new(), + environment_manager: Arc::new(EnvironmentManager::default_for_tests()), + } + } } #[derive(Debug, Clone, Default, PartialEq, Eq)] @@ -73,6 +92,7 @@ impl Hooks { CommandShell { program: config.shell_program.unwrap_or_default(), args: config.shell_args, + environment_manager: config.environment_manager, }, ); Self { diff --git a/codex-rs/rmcp-client/src/executor_process_transport.rs b/codex-rs/rmcp-client/src/executor_process_transport.rs index 41f0b7660d..bb57e66c6c 100644 --- a/codex-rs/rmcp-client/src/executor_process_transport.rs +++ b/codex-rs/rmcp-client/src/executor_process_transport.rs @@ -135,7 +135,10 @@ impl Transport for ExecutorProcessTransport { // the wire is JSON plus one newline delimiter. let mut bytes = to_vec(&item).map_err(io::Error::other)?; bytes.push(b'\n'); - let response = process.write(bytes).await.map_err(io::Error::other)?; + let response = process + .write(Some(bytes), false) + .await + .map_err(io::Error::other)?; match response.status { WriteStatus::Accepted => Ok(()), WriteStatus::UnknownProcess => { diff --git a/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs b/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs index ce31abf404..48aa7d1d9b 100644 --- a/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs +++ b/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs @@ -930,6 +930,7 @@ mod tests { is_managed, matcher: Some("Bash".to_string()), command: Some(command.to_string()), + environment_id: None, timeout_sec: 30, status_message: None, source_path: test_path_buf("/tmp/hooks.json").abs(), diff --git a/codex-rs/tui/src/debug_config.rs b/codex-rs/tui/src/debug_config.rs index b1f5608b89..f14b356b4b 100644 --- a/codex-rs/tui/src/debug_config.rs +++ b/codex-rs/tui/src/debug_config.rs @@ -974,6 +974,7 @@ approval_policy = "never" hooks: vec![HookHandlerConfig::Command { command: "python3 /enterprise/hooks/pre.py".to_string(), command_windows: None, + environment_id: None, timeout_sec: Some(10), r#async: false, status_message: Some("checking".to_string()), diff --git a/codex-rs/tui/src/startup_hooks_review.rs b/codex-rs/tui/src/startup_hooks_review.rs index 559c796aa0..09d87380f9 100644 --- a/codex-rs/tui/src/startup_hooks_review.rs +++ b/codex-rs/tui/src/startup_hooks_review.rs @@ -291,6 +291,7 @@ mod tests { is_managed: false, matcher: Some("Bash".to_string()), command: Some("/tmp/hook.sh".to_string()), + environment_id: None, timeout_sec: 30, status_message: None, source_path: test_path_buf("/tmp/hooks.json").abs(),