feat: use the arg0 trick with apply_patch (#2646)
Historically, Codex CLI has treated `apply_patch` (and its sometimes misspelling, `applypatch`) as a "virtual CLI," intercepting it when it appears as the first arg to `command` for the `"container.exec", `"shell"`, or `"local_shell"` tools. This approach has a known limitation where if, say, the model created a Python script that runs `apply_patch` and then tried to run the Python script, we have no insight as to what the model is trying to do and the Python Script would fail because `apply_patch` was never really on the `PATH`. One way to solve this problem is to require users to install an `apply_patch` executable alongside the `codex` executable (or at least put it someplace where Codex can discover it). Though to keep Codex CLI as a standalone executable, we exploit "the arg0 trick" where we create a temporary directory with an entry named `apply_patch` and prepend that directory to the `PATH` for the duration of the invocation of Codex. - On UNIX, `apply_patch` is a symlink to `codex`, which now changes its behavior to behave like `apply_patch` if arg0 is `apply_patch` (or `applypatch`) - On Windows, `apply_patch.bat` is a batch script that runs `codex --codex-run-as-apply-patch %*`, as Codex also changes its behavior if the first argument is `--codex-run-as-apply-patch`.
This commit is contained in:
@@ -7,6 +7,10 @@ version = { workspace = true }
|
||||
name = "codex_apply_patch"
|
||||
path = "src/lib.rs"
|
||||
|
||||
[[bin]]
|
||||
name = "apply_patch"
|
||||
path = "src/main.rs"
|
||||
|
||||
[lints]
|
||||
workspace = true
|
||||
|
||||
@@ -18,5 +22,6 @@ tree-sitter = "0.25.8"
|
||||
tree-sitter-bash = "0.25.0"
|
||||
|
||||
[dev-dependencies]
|
||||
assert_cmd = "2"
|
||||
pretty_assertions = "1.4.1"
|
||||
tempfile = "3.13.0"
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
mod parser;
|
||||
mod seek_sequence;
|
||||
mod standalone_executable;
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::path::Path;
|
||||
@@ -19,6 +20,8 @@ use tree_sitter::LanguageError;
|
||||
use tree_sitter::Parser;
|
||||
use tree_sitter_bash::LANGUAGE as BASH;
|
||||
|
||||
pub use standalone_executable::main;
|
||||
|
||||
/// Detailed instructions for gpt-4.1 on how to use the `apply_patch` tool.
|
||||
pub const APPLY_PATCH_TOOL_INSTRUCTIONS: &str = include_str!("../apply_patch_tool_instructions.md");
|
||||
|
||||
|
||||
3
codex-rs/apply-patch/src/main.rs
Normal file
3
codex-rs/apply-patch/src/main.rs
Normal file
@@ -0,0 +1,3 @@
|
||||
pub fn main() -> ! {
|
||||
codex_apply_patch::main()
|
||||
}
|
||||
59
codex-rs/apply-patch/src/standalone_executable.rs
Normal file
59
codex-rs/apply-patch/src/standalone_executable.rs
Normal file
@@ -0,0 +1,59 @@
|
||||
use std::io::Read;
|
||||
use std::io::Write;
|
||||
|
||||
pub fn main() -> ! {
|
||||
let exit_code = run_main();
|
||||
std::process::exit(exit_code);
|
||||
}
|
||||
|
||||
/// We would prefer to return `std::process::ExitCode`, but its `exit_process()`
|
||||
/// method is still a nightly API and we want main() to return !.
|
||||
pub fn run_main() -> i32 {
|
||||
// Expect either one argument (the full apply_patch payload) or read it from stdin.
|
||||
let mut args = std::env::args_os();
|
||||
let _argv0 = args.next();
|
||||
|
||||
let patch_arg = match args.next() {
|
||||
Some(arg) => match arg.into_string() {
|
||||
Ok(s) => s,
|
||||
Err(_) => {
|
||||
eprintln!("Error: apply_patch requires a UTF-8 PATCH argument.");
|
||||
return 1;
|
||||
}
|
||||
},
|
||||
None => {
|
||||
// No argument provided; attempt to read the patch from stdin.
|
||||
let mut buf = String::new();
|
||||
match std::io::stdin().read_to_string(&mut buf) {
|
||||
Ok(_) => {
|
||||
if buf.is_empty() {
|
||||
eprintln!("Usage: apply_patch 'PATCH'\n echo 'PATCH' | apply-patch");
|
||||
return 2;
|
||||
}
|
||||
buf
|
||||
}
|
||||
Err(err) => {
|
||||
eprintln!("Error: Failed to read PATCH from stdin.\n{err}");
|
||||
return 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
// Refuse extra args to avoid ambiguity.
|
||||
if args.next().is_some() {
|
||||
eprintln!("Error: apply_patch accepts exactly one argument.");
|
||||
return 2;
|
||||
}
|
||||
|
||||
let mut stdout = std::io::stdout();
|
||||
let mut stderr = std::io::stderr();
|
||||
match crate::apply_patch(&patch_arg, &mut stdout, &mut stderr) {
|
||||
Ok(()) => {
|
||||
// Flush to ensure output ordering when used in pipelines.
|
||||
let _ = stdout.flush();
|
||||
0
|
||||
}
|
||||
Err(_) => 1,
|
||||
}
|
||||
}
|
||||
3
codex-rs/apply-patch/tests/all.rs
Normal file
3
codex-rs/apply-patch/tests/all.rs
Normal file
@@ -0,0 +1,3 @@
|
||||
// Single integration test binary that aggregates all test modules.
|
||||
// The submodules live in `tests/suite/`.
|
||||
mod suite;
|
||||
90
codex-rs/apply-patch/tests/suite/cli.rs
Normal file
90
codex-rs/apply-patch/tests/suite/cli.rs
Normal file
@@ -0,0 +1,90 @@
|
||||
use assert_cmd::prelude::*;
|
||||
use std::fs;
|
||||
use std::process::Command;
|
||||
use tempfile::tempdir;
|
||||
|
||||
#[test]
|
||||
fn test_apply_patch_cli_add_and_update() -> anyhow::Result<()> {
|
||||
let tmp = tempdir()?;
|
||||
let file = "cli_test.txt";
|
||||
let absolute_path = tmp.path().join(file);
|
||||
|
||||
// 1) Add a file
|
||||
let add_patch = format!(
|
||||
r#"*** Begin Patch
|
||||
*** Add File: {file}
|
||||
+hello
|
||||
*** End Patch"#
|
||||
);
|
||||
Command::cargo_bin("apply_patch")
|
||||
.expect("should find apply_patch binary")
|
||||
.arg(add_patch)
|
||||
.current_dir(tmp.path())
|
||||
.assert()
|
||||
.success()
|
||||
.stdout(format!("Success. Updated the following files:\nA {file}\n"));
|
||||
assert_eq!(fs::read_to_string(&absolute_path)?, "hello\n");
|
||||
|
||||
// 2) Update the file
|
||||
let update_patch = format!(
|
||||
r#"*** Begin Patch
|
||||
*** Update File: {file}
|
||||
@@
|
||||
-hello
|
||||
+world
|
||||
*** End Patch"#
|
||||
);
|
||||
Command::cargo_bin("apply_patch")
|
||||
.expect("should find apply_patch binary")
|
||||
.arg(update_patch)
|
||||
.current_dir(tmp.path())
|
||||
.assert()
|
||||
.success()
|
||||
.stdout(format!("Success. Updated the following files:\nM {file}\n"));
|
||||
assert_eq!(fs::read_to_string(&absolute_path)?, "world\n");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_apply_patch_cli_stdin_add_and_update() -> anyhow::Result<()> {
|
||||
let tmp = tempdir()?;
|
||||
let file = "cli_test_stdin.txt";
|
||||
let absolute_path = tmp.path().join(file);
|
||||
|
||||
// 1) Add a file via stdin
|
||||
let add_patch = format!(
|
||||
r#"*** Begin Patch
|
||||
*** Add File: {file}
|
||||
+hello
|
||||
*** End Patch"#
|
||||
);
|
||||
let mut cmd =
|
||||
assert_cmd::Command::cargo_bin("apply_patch").expect("should find apply_patch binary");
|
||||
cmd.current_dir(tmp.path());
|
||||
cmd.write_stdin(add_patch)
|
||||
.assert()
|
||||
.success()
|
||||
.stdout(format!("Success. Updated the following files:\nA {file}\n"));
|
||||
assert_eq!(fs::read_to_string(&absolute_path)?, "hello\n");
|
||||
|
||||
// 2) Update the file via stdin
|
||||
let update_patch = format!(
|
||||
r#"*** Begin Patch
|
||||
*** Update File: {file}
|
||||
@@
|
||||
-hello
|
||||
+world
|
||||
*** End Patch"#
|
||||
);
|
||||
let mut cmd =
|
||||
assert_cmd::Command::cargo_bin("apply_patch").expect("should find apply_patch binary");
|
||||
cmd.current_dir(tmp.path());
|
||||
cmd.write_stdin(update_patch)
|
||||
.assert()
|
||||
.success()
|
||||
.stdout(format!("Success. Updated the following files:\nM {file}\n"));
|
||||
assert_eq!(fs::read_to_string(&absolute_path)?, "world\n");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
1
codex-rs/apply-patch/tests/suite/mod.rs
Normal file
1
codex-rs/apply-patch/tests/suite/mod.rs
Normal file
@@ -0,0 +1 @@
|
||||
mod cli;
|
||||
Reference in New Issue
Block a user