fix: support special --codex-run-as-apply-patch arg (#1702)
This introduces some special behavior to the CLIs that are using the
`codex-arg0` crate where if `arg1` is `--codex-run-as-apply-patch`, then
it will run as if `apply_patch arg2` were invoked. This is important
because it means we can do things like:
```
SANDBOX_TYPE=landlock # or seatbelt for macOS
codex debug "${SANDBOX_TYPE}" -- codex --codex-run-as-apply-patch PATCH
```
which gives us a way to run `apply_patch` while ensuring it adheres to
the sandbox the user specified.
While it would be nice to use the `arg0` trick like we are currently
doing for `codex-linux-sandbox`, there is no way to specify the `arg0`
for the underlying command when running under `/usr/bin/sandbox-exec`,
so it will not work for us in this case.
Admittedly, we could have also supported this via a custom environment
variable (e.g., `CODEX_ARG0`), but since environment variables are
inherited by child processes, that seemed like a potentially leakier
abstraction.
This change, as well as our existing reliance on checking `arg0`, place
additional requirements on those who include `codex-core`. Its
`README.md` has been updated to reflect this.
While we could have just added an `apply-patch` subcommand to the
`codex` multitool CLI, that would not be sufficient for the standalone
`codex-exec` CLI, which is something that we distribute as part of our
GitHub releases for those who know they will not be using the TUI and
therefore prefer to use a slightly smaller executable:
https://github.com/openai/codex/releases/tag/rust-v0.10.0
To that end, this PR adds an integration test to ensure that the
`--codex-run-as-apply-patch` option works with the standalone
`codex-exec` CLI.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/1702).
* #1705
* #1703
* __->__ #1702
* #1698
* #1697
This commit is contained in:
4
codex-rs/Cargo.lock
generated
4
codex-rs/Cargo.lock
generated
@@ -610,6 +610,7 @@ name = "codex-arg0"
|
|||||||
version = "0.0.0"
|
version = "0.0.0"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
"anyhow",
|
"anyhow",
|
||||||
|
"codex-apply-patch",
|
||||||
"codex-core",
|
"codex-core",
|
||||||
"codex-linux-sandbox",
|
"codex-linux-sandbox",
|
||||||
"dotenvy",
|
"dotenvy",
|
||||||
@@ -718,14 +719,17 @@ name = "codex-exec"
|
|||||||
version = "0.0.0"
|
version = "0.0.0"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
"anyhow",
|
"anyhow",
|
||||||
|
"assert_cmd",
|
||||||
"chrono",
|
"chrono",
|
||||||
"clap",
|
"clap",
|
||||||
"codex-arg0",
|
"codex-arg0",
|
||||||
"codex-common",
|
"codex-common",
|
||||||
"codex-core",
|
"codex-core",
|
||||||
"owo-colors",
|
"owo-colors",
|
||||||
|
"predicates",
|
||||||
"serde_json",
|
"serde_json",
|
||||||
"shlex",
|
"shlex",
|
||||||
|
"tempfile",
|
||||||
"tokio",
|
"tokio",
|
||||||
"tracing",
|
"tracing",
|
||||||
"tracing-subscriber",
|
"tracing-subscriber",
|
||||||
|
|||||||
@@ -12,6 +12,7 @@ workspace = true
|
|||||||
|
|
||||||
[dependencies]
|
[dependencies]
|
||||||
anyhow = "1"
|
anyhow = "1"
|
||||||
|
codex-apply-patch = { path = "../apply-patch" }
|
||||||
codex-core = { path = "../core" }
|
codex-core = { path = "../core" }
|
||||||
codex-linux-sandbox = { path = "../linux-sandbox" }
|
codex-linux-sandbox = { path = "../linux-sandbox" }
|
||||||
dotenvy = "0.15.7"
|
dotenvy = "0.15.7"
|
||||||
|
|||||||
@@ -30,7 +30,8 @@ where
|
|||||||
Fut: Future<Output = anyhow::Result<()>>,
|
Fut: Future<Output = anyhow::Result<()>>,
|
||||||
{
|
{
|
||||||
// Determine if we were invoked via the special alias.
|
// Determine if we were invoked via the special alias.
|
||||||
let argv0 = std::env::args_os().next().unwrap_or_default();
|
let mut args = std::env::args_os();
|
||||||
|
let argv0 = args.next().unwrap_or_default();
|
||||||
let exe_name = Path::new(&argv0)
|
let exe_name = Path::new(&argv0)
|
||||||
.file_name()
|
.file_name()
|
||||||
.and_then(|s| s.to_str())
|
.and_then(|s| s.to_str())
|
||||||
@@ -41,6 +42,26 @@ where
|
|||||||
codex_linux_sandbox::run_main();
|
codex_linux_sandbox::run_main();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let argv1 = args.next().unwrap_or_default();
|
||||||
|
if argv1 == "--codex-run-as-apply-patch" {
|
||||||
|
let patch_arg = args.next().and_then(|s| s.to_str().map(|s| s.to_owned()));
|
||||||
|
let exit_code = match patch_arg {
|
||||||
|
Some(patch_arg) => {
|
||||||
|
let mut stdout = std::io::stdout();
|
||||||
|
let mut stderr = std::io::stderr();
|
||||||
|
match codex_apply_patch::apply_patch(&patch_arg, &mut stdout, &mut stderr) {
|
||||||
|
Ok(()) => 0,
|
||||||
|
Err(_) => 1,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
None => {
|
||||||
|
eprintln!("Error: --codex-run-as-apply-patch requires a UTF-8 PATCH argument.");
|
||||||
|
1
|
||||||
|
}
|
||||||
|
};
|
||||||
|
std::process::exit(exit_code);
|
||||||
|
}
|
||||||
|
|
||||||
// This modifies the environment, which is not thread-safe, so do this
|
// This modifies the environment, which is not thread-safe, so do this
|
||||||
// before creating any threads/the Tokio runtime.
|
// before creating any threads/the Tokio runtime.
|
||||||
load_dotenv();
|
load_dotenv();
|
||||||
|
|||||||
@@ -2,9 +2,18 @@
|
|||||||
|
|
||||||
This crate implements the business logic for Codex. It is designed to be used by the various Codex UIs written in Rust.
|
This crate implements the business logic for Codex. It is designed to be used by the various Codex UIs written in Rust.
|
||||||
|
|
||||||
Though for non-Rust UIs, we are also working to define a _protocol_ for talking to Codex. See:
|
## Dependencies
|
||||||
|
|
||||||
- [Specification](../docs/protocol_v1.md)
|
Note that `codex-core` makes some assumptions about certain helper utilities being available in the environment. Currently, this
|
||||||
- [Rust types](./src/protocol.rs)
|
|
||||||
|
|
||||||
You can use the `proto` subcommand using the executable in the [`cli` crate](../cli) to speak the protocol using newline-delimited-JSON over stdin/stdout.
|
### macOS
|
||||||
|
|
||||||
|
Expects `/usr/bin/sandbox-exec` to be present.
|
||||||
|
|
||||||
|
### Linux
|
||||||
|
|
||||||
|
Expects the binary containing `codex-core` to run the equivalent of `codex debug landlock` when `arg0` is `codex-linux-sandbox`. See the `codex-arg0` crate for details.
|
||||||
|
|
||||||
|
### All Platforms
|
||||||
|
|
||||||
|
Expects the binary containing `codex-core` to simulate the virtual `apply_patch` CLI when `arg1` is `--codex-run-as-apply-patch`. See the `codex-arg0` crate for details.
|
||||||
|
|||||||
@@ -37,3 +37,8 @@ tokio = { version = "1", features = [
|
|||||||
] }
|
] }
|
||||||
tracing = { version = "0.1.41", features = ["log"] }
|
tracing = { version = "0.1.41", features = ["log"] }
|
||||||
tracing-subscriber = { version = "0.3.19", features = ["env-filter"] }
|
tracing-subscriber = { version = "0.3.19", features = ["env-filter"] }
|
||||||
|
|
||||||
|
[dev-dependencies]
|
||||||
|
assert_cmd = "2"
|
||||||
|
predicates = "3"
|
||||||
|
tempfile = "3.13.0"
|
||||||
|
|||||||
38
codex-rs/exec/tests/apply_patch.rs
Normal file
38
codex-rs/exec/tests/apply_patch.rs
Normal file
@@ -0,0 +1,38 @@
|
|||||||
|
use anyhow::Context;
|
||||||
|
use assert_cmd::prelude::*;
|
||||||
|
use std::fs;
|
||||||
|
use std::process::Command;
|
||||||
|
use tempfile::tempdir;
|
||||||
|
|
||||||
|
/// While we may add an `apply-patch` subcommand to the `codex` CLI multitool
|
||||||
|
/// at some point, we must ensure that the smaller `codex-exec` CLI can still
|
||||||
|
/// emulate the `apply_patch` CLI.
|
||||||
|
#[test]
|
||||||
|
fn test_standalone_exec_cli_can_use_apply_patch() -> anyhow::Result<()> {
|
||||||
|
let tmp = tempdir()?;
|
||||||
|
let relative_path = "source.txt";
|
||||||
|
let absolute_path = tmp.path().join(relative_path);
|
||||||
|
fs::write(&absolute_path, "original content\n")?;
|
||||||
|
|
||||||
|
Command::cargo_bin("codex-exec")
|
||||||
|
.context("should find binary for codex-exec")?
|
||||||
|
.arg("--codex-run-as-apply-patch")
|
||||||
|
.arg(
|
||||||
|
r#"*** Begin Patch
|
||||||
|
*** Update File: source.txt
|
||||||
|
@@
|
||||||
|
-original content
|
||||||
|
+modified by apply_patch
|
||||||
|
*** End Patch"#,
|
||||||
|
)
|
||||||
|
.current_dir(tmp.path())
|
||||||
|
.assert()
|
||||||
|
.success()
|
||||||
|
.stdout("Success. Updated the following files:\nM source.txt\n")
|
||||||
|
.stderr(predicates::str::is_empty());
|
||||||
|
assert_eq!(
|
||||||
|
fs::read_to_string(absolute_path)?,
|
||||||
|
"modified by apply_patch\n"
|
||||||
|
);
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user