mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Preserve output from hooks that exit before reading stdin (#35194)
## Why A hook can exit successfully without consuming its JSON input. If it closes stdin while Codex is still writing, the resulting broken pipe should not hide the hook's exit status and output. ## What changed Ignore broken-pipe errors when writing hook stdin and continue waiting for the process result. Other stdin write errors remain failures. ## Testing Add a cross-platform regression test that sends a large input to a fast-exiting hook and verifies that its successful status and stdout are preserved. GitOrigin-RevId: e1b6e7c6ac5e39232e49b45245ea0df9d34d1fe6
This commit is contained in:
@@ -1,3 +1,4 @@
|
||||
use std::io::ErrorKind;
|
||||
use std::path::Path;
|
||||
use std::process::Stdio;
|
||||
use std::time::Duration;
|
||||
@@ -83,6 +84,7 @@ pub(crate) async fn run_command(
|
||||
|
||||
if let Some(mut stdin) = child.stdin.take()
|
||||
&& let Err(err) = stdin.write_all(input_json.as_bytes()).await
|
||||
&& err.kind() != ErrorKind::BrokenPipe
|
||||
{
|
||||
let _ = child.kill().await;
|
||||
return finish_command_run(
|
||||
@@ -208,6 +210,6 @@ fn default_shell_command() -> Command {
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(all(test, windows))]
|
||||
#[cfg(test)]
|
||||
#[path = "command_runner_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use std::collections::HashMap;
|
||||
#[cfg(windows)]
|
||||
use std::fs;
|
||||
|
||||
use codex_protocol::protocol::HookEventName;
|
||||
@@ -11,6 +12,7 @@ use super::CommandShell;
|
||||
use super::ConfiguredHandler;
|
||||
use super::run_command;
|
||||
|
||||
#[cfg(windows)]
|
||||
#[tokio::test]
|
||||
async fn cmd_shell_runs_quoted_hook_command_path() {
|
||||
let temp = tempdir().expect("create temp dir");
|
||||
@@ -62,3 +64,40 @@ async fn cmd_shell_runs_quoted_hook_command_path() {
|
||||
assert!(result.error.is_none());
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn fast_exiting_hook_preserves_stdout_when_stdin_is_not_consumed() {
|
||||
let temp = tempdir().expect("create temp dir");
|
||||
let source_path = AbsolutePathBuf::try_from(temp.path().join("hooks.json"))
|
||||
.expect("absolute hook configuration path");
|
||||
let handler = ConfiguredHandler {
|
||||
event_name: HookEventName::SessionStart,
|
||||
matcher: None,
|
||||
command: "echo hook-ran".to_string(),
|
||||
timeout_sec: 10,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path,
|
||||
source: HookSource::User,
|
||||
display_order: 0,
|
||||
env: HashMap::new(),
|
||||
};
|
||||
let shell = CommandShell {
|
||||
program: String::new(),
|
||||
args: Vec::new(),
|
||||
};
|
||||
let input_json = format!(r#"{{"padding":"{}"}}"#, "x".repeat(1024 * 1024));
|
||||
|
||||
let result = run_command(
|
||||
&shell,
|
||||
&handler,
|
||||
/*configured_order*/ 0,
|
||||
&input_json,
|
||||
temp.path(),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(result.exit_code, Some(0), "stderr: {}", result.stderr);
|
||||
assert_eq!(result.stdout.trim(), "hook-ran");
|
||||
assert_eq!(result.error, None);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user