mirror of
https://github.com/openai/codex.git
synced 2026-11-09 20:50:33 +00:00
feat(hooks): add explicit remote hook routing
This commit is contained in:
@@ -435,6 +435,9 @@ pub enum ConfiguredHookHandler {
|
||||
#[serde(rename = "commandWindows")]
|
||||
#[ts(rename = "commandWindows")]
|
||||
command_windows: Option<String>,
|
||||
#[serde(rename = "environmentId")]
|
||||
#[ts(rename = "environmentId")]
|
||||
environment_id: Option<String>,
|
||||
#[serde(rename = "timeoutSec")]
|
||||
#[ts(rename = "timeoutSec")]
|
||||
timeout_sec: Option<u64>,
|
||||
|
||||
@@ -499,6 +499,7 @@ pub struct HookMetadata {
|
||||
pub handler_type: HookHandlerType,
|
||||
pub matcher: Option<String>,
|
||||
pub command: Option<String>,
|
||||
pub environment_id: Option<String>,
|
||||
pub timeout_sec: u64,
|
||||
pub status_message: Option<String>,
|
||||
pub source_path: AbsolutePathBuf,
|
||||
|
||||
@@ -68,6 +68,7 @@ fn hooks_to_info(hooks: &[codex_hooks::HookListEntry]) -> Vec<HookMetadata> {
|
||||
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(),
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -142,6 +142,8 @@ pub enum HookHandlerConfig {
|
||||
command: String,
|
||||
#[serde(default, rename = "commandWindows", alias = "command_windows")]
|
||||
command_windows: Option<String>,
|
||||
#[serde(default, rename = "environmentId", alias = "environment_id")]
|
||||
environment_id: Option<String>,
|
||||
#[serde(default, rename = "timeout")]
|
||||
timeout_sec: Option<u64>,
|
||||
#[serde(default)]
|
||||
|
||||
@@ -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,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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()),
|
||||
|
||||
@@ -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<EnvironmentManager>,
|
||||
) -> 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,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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 => {
|
||||
|
||||
@@ -59,7 +59,11 @@ impl ExecProcess for MockExecProcess {
|
||||
}))
|
||||
}
|
||||
|
||||
async fn write(&self, _chunk: Vec<u8>) -> Result<WriteResponse, ExecServerError> {
|
||||
async fn write(
|
||||
&self,
|
||||
_chunk: Option<Vec<u8>>,
|
||||
_close_stdin: bool,
|
||||
) -> Result<WriteResponse, ExecServerError> {
|
||||
Ok(self.write_response.clone())
|
||||
}
|
||||
|
||||
|
||||
@@ -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(()));
|
||||
|
||||
@@ -361,13 +361,15 @@ impl ExecServerClient {
|
||||
pub async fn write(
|
||||
&self,
|
||||
process_id: &ProcessId,
|
||||
chunk: Vec<u8>,
|
||||
chunk: Option<Vec<u8>>,
|
||||
close_stdin: bool,
|
||||
) -> Result<WriteResponse, ExecServerError> {
|
||||
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<u8>) -> Result<WriteResponse, ExecServerError> {
|
||||
self.client.write(&self.process_id, chunk).await
|
||||
pub(crate) async fn write(
|
||||
&self,
|
||||
chunk: Option<Vec<u8>>,
|
||||
close_stdin: bool,
|
||||
) -> Result<WriteResponse, ExecServerError> {
|
||||
self.client
|
||||
.write(&self.process_id, chunk, close_stdin)
|
||||
.await
|
||||
}
|
||||
|
||||
pub(crate) async fn terminate(&self) -> Result<(), ExecServerError> {
|
||||
|
||||
@@ -352,7 +352,11 @@ impl LocalProcess {
|
||||
params: WriteParams,
|
||||
) -> Result<WriteResponse, JSONRPCErrorError> {
|
||||
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<u8>) -> Result<WriteResponse, ExecServerError> {
|
||||
self.backend.write(&self.process_id, chunk).await
|
||||
async fn write(
|
||||
&self,
|
||||
chunk: Option<Vec<u8>>,
|
||||
close_stdin: bool,
|
||||
) -> Result<WriteResponse, ExecServerError> {
|
||||
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<u8>,
|
||||
chunk: Option<Vec<u8>>,
|
||||
close_stdin: bool,
|
||||
) -> Result<WriteResponse, ExecServerError> {
|
||||
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)
|
||||
|
||||
@@ -176,7 +176,11 @@ pub trait ExecProcess: Send + Sync {
|
||||
wait_ms: Option<u64>,
|
||||
) -> Result<ReadResponse, ExecServerError>;
|
||||
|
||||
async fn write(&self, chunk: Vec<u8>) -> Result<WriteResponse, ExecServerError>;
|
||||
async fn write(
|
||||
&self,
|
||||
chunk: Option<Vec<u8>>,
|
||||
close_stdin: bool,
|
||||
) -> Result<WriteResponse, ExecServerError>;
|
||||
|
||||
async fn terminate(&self) -> Result<(), ExecServerError>;
|
||||
}
|
||||
|
||||
@@ -127,7 +127,9 @@ pub struct ReadResponse {
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct WriteParams {
|
||||
pub process_id: ProcessId,
|
||||
pub chunk: ByteChunk,
|
||||
pub chunk: Option<ByteChunk>,
|
||||
#[serde(default)]
|
||||
pub close_stdin: bool,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
|
||||
|
||||
@@ -71,9 +71,13 @@ impl ExecProcess for RemoteExecProcess {
|
||||
self.session.read(after_seq, max_bytes, wait_ms).await
|
||||
}
|
||||
|
||||
async fn write(&self, chunk: Vec<u8>) -> Result<WriteResponse, ExecServerError> {
|
||||
async fn write(
|
||||
&self,
|
||||
chunk: Option<Vec<u8>>,
|
||||
close_stdin: bool,
|
||||
) -> Result<WriteResponse, ExecServerError> {
|
||||
trace!("exec process write");
|
||||
self.session.write(chunk).await
|
||||
self.session.write(chunk, close_stdin).await
|
||||
}
|
||||
|
||||
async fn terminate(&self) -> Result<(), ExecServerError> {
|
||||
|
||||
@@ -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)]
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<codex_exec_server::Environment>,
|
||||
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<dyn ExecProcess>,
|
||||
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<dyn ExecProcess>,
|
||||
) -> Result<(String, String, Option<i32>), 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<String> {
|
||||
vec![
|
||||
"cmd.exe".to_string(),
|
||||
"/C".to_string(),
|
||||
handler.command.clone(),
|
||||
]
|
||||
}
|
||||
|
||||
#[cfg(not(windows))]
|
||||
fn default_shell_argv(handler: &ConfiguredHandler) -> Vec<String> {
|
||||
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<EnvironmentManager>) -> 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"))
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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<String>,
|
||||
pub environment_manager: Arc<EnvironmentManager>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
@@ -43,6 +46,7 @@ pub(crate) struct ConfiguredHandler {
|
||||
pub event_name: codex_protocol::protocol::HookEventName,
|
||||
pub matcher: Option<String>,
|
||||
pub command: String,
|
||||
pub environment_id: Option<String>,
|
||||
pub timeout_sec: u64,
|
||||
pub status_message: Option<String>,
|
||||
pub source_path: AbsolutePathBuf,
|
||||
@@ -84,6 +88,7 @@ pub struct HookListEntry {
|
||||
pub handler_type: HookHandlerType,
|
||||
pub matcher: Option<String>,
|
||||
pub command: Option<String>,
|
||||
pub environment_id: Option<String>,
|
||||
pub timeout_sec: u64,
|
||||
pub status_message: Option<String>,
|
||||
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(
|
||||
|
||||
@@ -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<Path>,
|
||||
hooks: HookEventsToml,
|
||||
@@ -63,6 +72,7 @@ fn pre_tool_use_hook_events(command: impl Into<String>) -> 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"));
|
||||
}
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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<Vec<String>>,
|
||||
pub feature_enabled: bool,
|
||||
@@ -36,6 +38,23 @@ pub struct HooksConfig {
|
||||
pub plugin_hook_load_warnings: Vec<String>,
|
||||
pub shell_program: Option<String>,
|
||||
pub shell_args: Vec<String>,
|
||||
pub environment_manager: Arc<EnvironmentManager>,
|
||||
}
|
||||
|
||||
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 {
|
||||
|
||||
@@ -135,7 +135,10 @@ impl Transport<RoleClient> 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 => {
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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()),
|
||||
|
||||
@@ -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(),
|
||||
|
||||
Reference in New Issue
Block a user