mirror of
https://github.com/openai/codex.git
synced 2026-11-07 16:35:32 +00:00
fix: codex-arg0 no longer depends on codex-core (#12434)
## Why `codex-rs/arg0` only needed two things from `codex-core`: - the `find_codex_home()` wrapper - the special argv flag used for the internal `apply_patch` self-invocation path That made `codex-arg0` depend on `codex-core` for a very small surface area. This change removes that dependency edge and moves the shared `apply_patch` invocation flag to a more natural boundary (`codex-apply-patch`) while keeping the contract explicitly documented. ## What Changed - Moved the internal `apply_patch` argv[1] flag constant out of `codex-core` and into `codex-apply-patch`. - Renamed the constant to `CODEX_CORE_APPLY_PATCH_ARG1` and documented that it is part of the Codex core process-invocation contract (even though it now lives in `codex-apply-patch`). - Updated `arg0`, the core apply-patch runtime, and the `codex-exec` apply-patch test to import the constant from `codex-apply-patch`. - Updated `codex-rs/arg0` to call `codex_utils_home_dir::find_codex_home()` directly instead of `codex_core::config::find_codex_home()`. - Removed the `codex-core` dependency from `codex-rs/arg0` and added the needed direct dependency on `codex-utils-home-dir`. - Added `codex-apply-patch` as a dev-dependency for `codex-rs/exec` tests (the apply-patch test now imports the moved constant directly). ## Verification - `cargo test -p codex-apply-patch` - `cargo test -p codex-arg0` - `cargo test -p codex-core --lib apply_patch` - `cargo test -p codex-exec test_standalone_exec_cli_can_use_apply_patch` - `cargo shear`
This commit is contained in:
1 parent
1af2a37ada
commit
2fe4be1aa9
9 files changed
+29
-16
No files matched your search
Generated
+2
-1
@@ -1398,8 +1398,8 @@ version = "0.0.0"
|
|||||||
dependencies = [
|
dependencies = [
|
||||||
"anyhow",
|
"anyhow",
|
||||||
"codex-apply-patch",
|
"codex-apply-patch",
|
||||||
"codex-core",
|
|
||||||
"codex-linux-sandbox",
|
"codex-linux-sandbox",
|
||||||
|
"codex-utils-home-dir",
|
||||||
"dotenvy",
|
"dotenvy",
|
||||||
"tempfile",
|
"tempfile",
|
||||||
"tokio",
|
"tokio",
|
||||||
@@ -1735,6 +1735,7 @@ dependencies = [
|
|||||||
"anyhow",
|
"anyhow",
|
||||||
"assert_cmd",
|
"assert_cmd",
|
||||||
"clap",
|
"clap",
|
||||||
|
"codex-apply-patch",
|
||||||
"codex-arg0",
|
"codex-arg0",
|
||||||
"codex-cloud-requirements",
|
"codex-cloud-requirements",
|
||||||
"codex-core",
|
"codex-core",
|
||||||
|
|||||||
@@ -25,6 +25,15 @@ use crate::invocation::ExtractHeredocError;
|
|||||||
/// Detailed instructions for gpt-4.1 on how to use the `apply_patch` tool.
|
/// 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");
|
pub const APPLY_PATCH_TOOL_INSTRUCTIONS: &str = include_str!("../apply_patch_tool_instructions.md");
|
||||||
|
|
||||||
|
/// Special argv[1] flag used when the Codex executable self-invokes to run the
|
||||||
|
/// internal `apply_patch` path.
|
||||||
|
///
|
||||||
|
/// Although this constant lives in `codex-apply-patch` (to avoid forcing
|
||||||
|
/// `codex-arg0` to depend on `codex-core`), it is part of the "codex core"
|
||||||
|
/// process-invocation contract between the apply-patch runtime and the arg0
|
||||||
|
/// dispatcher.
|
||||||
|
pub const CODEX_CORE_APPLY_PATCH_ARG1: &str = "--codex-run-as-apply-patch";
|
||||||
|
|
||||||
#[derive(Debug, Error, PartialEq)]
|
#[derive(Debug, Error, PartialEq)]
|
||||||
pub enum ApplyPatchError {
|
pub enum ApplyPatchError {
|
||||||
#[error(transparent)]
|
#[error(transparent)]
|
||||||
|
|||||||
@@ -14,8 +14,8 @@ workspace = true
|
|||||||
[dependencies]
|
[dependencies]
|
||||||
anyhow = { workspace = true }
|
anyhow = { workspace = true }
|
||||||
codex-apply-patch = { workspace = true }
|
codex-apply-patch = { workspace = true }
|
||||||
codex-core = { workspace = true }
|
|
||||||
codex-linux-sandbox = { workspace = true }
|
codex-linux-sandbox = { workspace = true }
|
||||||
|
codex-utils-home-dir = { workspace = true }
|
||||||
dotenvy = { workspace = true }
|
dotenvy = { workspace = true }
|
||||||
tempfile = { workspace = true }
|
tempfile = { workspace = true }
|
||||||
tokio = { workspace = true, features = ["rt-multi-thread"] }
|
tokio = { workspace = true, features = ["rt-multi-thread"] }
|
||||||
@@ -3,7 +3,8 @@ use std::future::Future;
|
|||||||
use std::path::Path;
|
use std::path::Path;
|
||||||
use std::path::PathBuf;
|
use std::path::PathBuf;
|
||||||
|
|
||||||
use codex_core::CODEX_APPLY_PATCH_ARG1;
|
use codex_apply_patch::CODEX_CORE_APPLY_PATCH_ARG1;
|
||||||
|
use codex_utils_home_dir::find_codex_home;
|
||||||
#[cfg(unix)]
|
#[cfg(unix)]
|
||||||
use std::os::unix::fs::symlink;
|
use std::os::unix::fs::symlink;
|
||||||
use tempfile::TempDir;
|
use tempfile::TempDir;
|
||||||
@@ -46,7 +47,7 @@ pub fn arg0_dispatch() -> Option<Arg0PathEntryGuard> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
let argv1 = args.next().unwrap_or_default();
|
let argv1 = args.next().unwrap_or_default();
|
||||||
if argv1 == CODEX_APPLY_PATCH_ARG1 {
|
if argv1 == CODEX_CORE_APPLY_PATCH_ARG1 {
|
||||||
let patch_arg = args.next().and_then(|s| s.to_str().map(str::to_owned));
|
let patch_arg = args.next().and_then(|s| s.to_str().map(str::to_owned));
|
||||||
let exit_code = match patch_arg {
|
let exit_code = match patch_arg {
|
||||||
Some(patch_arg) => {
|
Some(patch_arg) => {
|
||||||
@@ -58,7 +59,7 @@ pub fn arg0_dispatch() -> Option<Arg0PathEntryGuard> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
None => {
|
None => {
|
||||||
eprintln!("Error: {CODEX_APPLY_PATCH_ARG1} requires a UTF-8 PATCH argument.");
|
eprintln!("Error: {CODEX_CORE_APPLY_PATCH_ARG1} requires a UTF-8 PATCH argument.");
|
||||||
1
|
1
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
@@ -139,7 +140,7 @@ const ILLEGAL_ENV_VAR_PREFIX: &str = "CODEX_";
|
|||||||
/// Security: Do not allow `.env` files to create or modify any variables
|
/// Security: Do not allow `.env` files to create or modify any variables
|
||||||
/// with names starting with `CODEX_`.
|
/// with names starting with `CODEX_`.
|
||||||
fn load_dotenv() {
|
fn load_dotenv() {
|
||||||
if let Ok(codex_home) = codex_core::config::find_codex_home()
|
if let Ok(codex_home) = find_codex_home()
|
||||||
&& let Ok(iter) = dotenvy::from_path_iter(codex_home.join(".env"))
|
&& let Ok(iter) = dotenvy::from_path_iter(codex_home.join(".env"))
|
||||||
{
|
{
|
||||||
set_filtered(iter);
|
set_filtered(iter);
|
||||||
@@ -175,7 +176,7 @@ where
|
|||||||
/// IMPORTANT: This function modifies the PATH environment variable, so it MUST
|
/// IMPORTANT: This function modifies the PATH environment variable, so it MUST
|
||||||
/// be called before multiple threads are spawned.
|
/// be called before multiple threads are spawned.
|
||||||
pub fn prepend_path_entry_for_codex_aliases() -> std::io::Result<Arg0PathEntryGuard> {
|
pub fn prepend_path_entry_for_codex_aliases() -> std::io::Result<Arg0PathEntryGuard> {
|
||||||
let codex_home = codex_core::config::find_codex_home()?;
|
let codex_home = find_codex_home()?;
|
||||||
#[cfg(not(debug_assertions))]
|
#[cfg(not(debug_assertions))]
|
||||||
{
|
{
|
||||||
// Guard against placing helpers in system temp directories outside debug builds.
|
// Guard against placing helpers in system temp directories outside debug builds.
|
||||||
@@ -242,7 +243,7 @@ pub fn prepend_path_entry_for_codex_aliases() -> std::io::Result<Arg0PathEntryGu
|
|||||||
&batch_script,
|
&batch_script,
|
||||||
format!(
|
format!(
|
||||||
r#"@echo off
|
r#"@echo off
|
||||||
"{}" {CODEX_APPLY_PATCH_ARG1} %*
|
"{}" {CODEX_CORE_APPLY_PATCH_ARG1} %*
|
||||||
"#,
|
"#,
|
||||||
exe.display()
|
exe.display()
|
||||||
),
|
),
|
||||||
|
|||||||
@@ -9,8 +9,6 @@ use codex_apply_patch::ApplyPatchFileChange;
|
|||||||
use std::collections::HashMap;
|
use std::collections::HashMap;
|
||||||
use std::path::PathBuf;
|
use std::path::PathBuf;
|
||||||
|
|
||||||
pub const CODEX_APPLY_PATCH_ARG1: &str = "--codex-run-as-apply-patch";
|
|
||||||
|
|
||||||
pub(crate) enum InternalApplyPatchInvocation {
|
pub(crate) enum InternalApplyPatchInvocation {
|
||||||
/// The `apply_patch` call was handled programmatically, without any sort
|
/// The `apply_patch` call was handled programmatically, without any sort
|
||||||
/// of sandbox, because the user explicitly approved it. This is the
|
/// of sandbox, because the user explicitly approved it. This is the
|
||||||
@@ -20,7 +18,8 @@ pub(crate) enum InternalApplyPatchInvocation {
|
|||||||
/// The `apply_patch` call was approved, either automatically because it
|
/// The `apply_patch` call was approved, either automatically because it
|
||||||
/// appears that it should be allowed based on the user's sandbox policy
|
/// appears that it should be allowed based on the user's sandbox policy
|
||||||
/// *or* because the user explicitly approved it. In either case, we use
|
/// *or* because the user explicitly approved it. In either case, we use
|
||||||
/// exec with [`CODEX_APPLY_PATCH_ARG1`] to realize the `apply_patch` call,
|
/// exec with [`codex_apply_patch::CODEX_CORE_APPLY_PATCH_ARG1`] to realize
|
||||||
|
/// the `apply_patch` call,
|
||||||
/// but [`ApplyPatchExec::auto_approved`] is used to determine the sandbox
|
/// but [`ApplyPatchExec::auto_approved`] is used to determine the sandbox
|
||||||
/// used with the `exec()`.
|
/// used with the `exec()`.
|
||||||
DelegateToExec(ApplyPatchExec),
|
DelegateToExec(ApplyPatchExec),
|
||||||
|
|||||||
@@ -144,7 +144,6 @@ pub(crate) use codex_shell_command::is_safe_command;
|
|||||||
pub(crate) use codex_shell_command::parse_command;
|
pub(crate) use codex_shell_command::parse_command;
|
||||||
pub(crate) use codex_shell_command::powershell;
|
pub(crate) use codex_shell_command::powershell;
|
||||||
|
|
||||||
pub use apply_patch::CODEX_APPLY_PATCH_ARG1;
|
|
||||||
pub use client::X_CODEX_TURN_METADATA_HEADER;
|
pub use client::X_CODEX_TURN_METADATA_HEADER;
|
||||||
pub use exec_policy::ExecPolicyError;
|
pub use exec_policy::ExecPolicyError;
|
||||||
pub use exec_policy::check_execpolicy_for_warnings;
|
pub use exec_policy::check_execpolicy_for_warnings;
|
||||||
|
|||||||
@@ -4,7 +4,6 @@
|
|||||||
//! decision to avoid re-prompting, builds the self-invocation command for
|
//! decision to avoid re-prompting, builds the self-invocation command for
|
||||||
//! `codex --codex-run-as-apply-patch`, and runs under the current
|
//! `codex --codex-run-as-apply-patch`, and runs under the current
|
||||||
//! `SandboxAttempt` with a minimal environment.
|
//! `SandboxAttempt` with a minimal environment.
|
||||||
use crate::CODEX_APPLY_PATCH_ARG1;
|
|
||||||
use crate::exec::ExecToolCallOutput;
|
use crate::exec::ExecToolCallOutput;
|
||||||
use crate::sandboxing::CommandSpec;
|
use crate::sandboxing::CommandSpec;
|
||||||
use crate::sandboxing::SandboxPermissions;
|
use crate::sandboxing::SandboxPermissions;
|
||||||
@@ -20,6 +19,7 @@ use crate::tools::sandboxing::ToolError;
|
|||||||
use crate::tools::sandboxing::ToolRuntime;
|
use crate::tools::sandboxing::ToolRuntime;
|
||||||
use crate::tools::sandboxing::with_cached_approval;
|
use crate::tools::sandboxing::with_cached_approval;
|
||||||
use codex_apply_patch::ApplyPatchAction;
|
use codex_apply_patch::ApplyPatchAction;
|
||||||
|
use codex_apply_patch::CODEX_CORE_APPLY_PATCH_ARG1;
|
||||||
use codex_protocol::protocol::AskForApproval;
|
use codex_protocol::protocol::AskForApproval;
|
||||||
use codex_protocol::protocol::FileChange;
|
use codex_protocol::protocol::FileChange;
|
||||||
use codex_protocol::protocol::ReviewDecision;
|
use codex_protocol::protocol::ReviewDecision;
|
||||||
@@ -57,7 +57,10 @@ impl ApplyPatchRuntime {
|
|||||||
let program = exe.to_string_lossy().to_string();
|
let program = exe.to_string_lossy().to_string();
|
||||||
Ok(CommandSpec {
|
Ok(CommandSpec {
|
||||||
program,
|
program,
|
||||||
args: vec![CODEX_APPLY_PATCH_ARG1.to_string(), req.action.patch.clone()],
|
args: vec![
|
||||||
|
CODEX_CORE_APPLY_PATCH_ARG1.to_string(),
|
||||||
|
req.action.patch.clone(),
|
||||||
|
],
|
||||||
cwd: req.action.cwd.clone(),
|
cwd: req.action.cwd.clone(),
|
||||||
expiration: req.timeout_ms.into(),
|
expiration: req.timeout_ms.into(),
|
||||||
// Run apply_patch with a minimal environment for determinism and to avoid leaks.
|
// Run apply_patch with a minimal environment for determinism and to avoid leaks.
|
||||||
|
|||||||
@@ -51,6 +51,7 @@ uuid = { workspace = true }
|
|||||||
|
|
||||||
[dev-dependencies]
|
[dev-dependencies]
|
||||||
assert_cmd = { workspace = true }
|
assert_cmd = { workspace = true }
|
||||||
|
codex-apply-patch = { workspace = true }
|
||||||
codex-utils-cargo-bin = { workspace = true }
|
codex-utils-cargo-bin = { workspace = true }
|
||||||
core_test_support = { workspace = true }
|
core_test_support = { workspace = true }
|
||||||
libc = { workspace = true }
|
libc = { workspace = true }
|
||||||
|
|||||||
@@ -2,7 +2,7 @@
|
|||||||
|
|
||||||
use anyhow::Context;
|
use anyhow::Context;
|
||||||
use assert_cmd::prelude::*;
|
use assert_cmd::prelude::*;
|
||||||
use codex_core::CODEX_APPLY_PATCH_ARG1;
|
use codex_apply_patch::CODEX_CORE_APPLY_PATCH_ARG1;
|
||||||
use core_test_support::responses::ev_apply_patch_custom_tool_call;
|
use core_test_support::responses::ev_apply_patch_custom_tool_call;
|
||||||
use core_test_support::responses::ev_apply_patch_function_call;
|
use core_test_support::responses::ev_apply_patch_function_call;
|
||||||
use core_test_support::responses::ev_completed;
|
use core_test_support::responses::ev_completed;
|
||||||
@@ -24,7 +24,7 @@ fn test_standalone_exec_cli_can_use_apply_patch() -> anyhow::Result<()> {
|
|||||||
fs::write(&absolute_path, "original content\n")?;
|
fs::write(&absolute_path, "original content\n")?;
|
||||||
|
|
||||||
Command::new(codex_utils_cargo_bin::cargo_bin("codex-exec")?)
|
Command::new(codex_utils_cargo_bin::cargo_bin("codex-exec")?)
|
||||||
.arg(CODEX_APPLY_PATCH_ARG1)
|
.arg(CODEX_CORE_APPLY_PATCH_ARG1)
|
||||||
.arg(
|
.arg(
|
||||||
r#"*** Begin Patch
|
r#"*** Begin Patch
|
||||||
*** Update File: source.txt
|
*** Update File: source.txt
|
||||||
|
|||||||
Reference in new issue
Block a user