mirror of
https://github.com/openai/codex.git
synced 2026-09-09 15:58:47 +00:00
Avoid Windows apply_patch command line limit
This commit is contained in:
@@ -39,6 +39,11 @@ pub const APPLY_PATCH_TOOL_INSTRUCTIONS: &str = include_str!("../apply_patch_too
|
||||
/// dispatcher.
|
||||
pub const CODEX_CORE_APPLY_PATCH_ARG1: &str = "--codex-run-as-apply-patch";
|
||||
|
||||
/// Variant of [`CODEX_CORE_APPLY_PATCH_ARG1`] where argv[2] is a UTF-8 path to
|
||||
/// the patch payload. This avoids Windows command-line length limits for large
|
||||
/// patches while keeping the hidden self-invocation contract explicit.
|
||||
pub const CODEX_CORE_APPLY_PATCH_FILE_ARG1: &str = "--codex-run-as-apply-patch-file";
|
||||
|
||||
#[derive(Debug, Error, PartialEq)]
|
||||
pub enum ApplyPatchError {
|
||||
#[error(transparent)]
|
||||
|
||||
@@ -4,6 +4,7 @@ use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use codex_apply_patch::CODEX_CORE_APPLY_PATCH_ARG1;
|
||||
use codex_apply_patch::CODEX_CORE_APPLY_PATCH_FILE_ARG1;
|
||||
use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0;
|
||||
use codex_utils_home_dir::find_codex_home;
|
||||
#[cfg(unix)]
|
||||
@@ -93,10 +94,21 @@ pub fn arg0_dispatch() -> Option<Arg0PathEntryGuard> {
|
||||
}
|
||||
|
||||
let argv1 = args.next().unwrap_or_default();
|
||||
if argv1 == CODEX_CORE_APPLY_PATCH_ARG1 {
|
||||
if argv1 == CODEX_CORE_APPLY_PATCH_ARG1 || argv1 == CODEX_CORE_APPLY_PATCH_FILE_ARG1 {
|
||||
let patch_arg = args.next().and_then(|s| s.to_str().map(str::to_owned));
|
||||
let exit_code = match patch_arg {
|
||||
Some(patch_arg) => {
|
||||
let patch = if argv1 == CODEX_CORE_APPLY_PATCH_FILE_ARG1 {
|
||||
match std::fs::read_to_string(&patch_arg) {
|
||||
Ok(patch) => patch,
|
||||
Err(err) => {
|
||||
eprintln!("Error: failed to read apply_patch payload file: {err}");
|
||||
std::process::exit(1);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
patch_arg
|
||||
};
|
||||
let mut stdout = std::io::stdout();
|
||||
let mut stderr = std::io::stderr();
|
||||
let cwd = match codex_utils_absolute_path::AbsolutePathBuf::current_dir() {
|
||||
@@ -111,7 +123,7 @@ pub fn arg0_dispatch() -> Option<Arg0PathEntryGuard> {
|
||||
Err(_) => std::process::exit(1),
|
||||
};
|
||||
match runtime.block_on(codex_apply_patch::apply_patch(
|
||||
&patch_arg,
|
||||
&patch,
|
||||
&cwd,
|
||||
&mut stdout,
|
||||
&mut stderr,
|
||||
@@ -122,7 +134,10 @@ pub fn arg0_dispatch() -> Option<Arg0PathEntryGuard> {
|
||||
}
|
||||
}
|
||||
None => {
|
||||
eprintln!("Error: {CODEX_CORE_APPLY_PATCH_ARG1} requires a UTF-8 PATCH argument.");
|
||||
eprintln!(
|
||||
"Error: {} requires a UTF-8 PATCH argument.",
|
||||
argv1.to_string_lossy()
|
||||
);
|
||||
1
|
||||
}
|
||||
};
|
||||
|
||||
@@ -22,6 +22,8 @@ use crate::tools::sandboxing::ToolRuntime;
|
||||
use crate::tools::sandboxing::with_cached_approval;
|
||||
use codex_apply_patch::ApplyPatchAction;
|
||||
use codex_apply_patch::CODEX_CORE_APPLY_PATCH_ARG1;
|
||||
#[cfg(target_os = "windows")]
|
||||
use codex_apply_patch::CODEX_CORE_APPLY_PATCH_FILE_ARG1;
|
||||
use codex_protocol::exec_output::ExecToolCallOutput;
|
||||
use codex_protocol::exec_output::StreamOutput;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
@@ -33,8 +35,19 @@ use codex_sandboxing::SandboxablePreference;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use futures::future::BoxFuture;
|
||||
use std::collections::HashMap;
|
||||
#[cfg(target_os = "windows")]
|
||||
use std::io::Write;
|
||||
use std::path::PathBuf;
|
||||
use std::time::Instant;
|
||||
use tempfile::NamedTempFile;
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
const APPLY_PATCH_INLINE_ARG_MAX_BYTES: usize = 16 * 1024;
|
||||
|
||||
struct BuiltApplyPatchCommand {
|
||||
command: SandboxCommand,
|
||||
_payload_file: Option<NamedTempFile>,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub struct ApplyPatchRequest {
|
||||
@@ -71,20 +84,20 @@ impl ApplyPatchRuntime {
|
||||
fn build_sandbox_command(
|
||||
req: &ApplyPatchRequest,
|
||||
codex_home: &std::path::Path,
|
||||
) -> Result<SandboxCommand, ToolError> {
|
||||
Ok(Self::build_sandbox_command_with_program(
|
||||
) -> Result<BuiltApplyPatchCommand, ToolError> {
|
||||
Self::build_sandbox_command_with_program(
|
||||
req,
|
||||
codex_windows_sandbox::resolve_current_exe_for_launch(codex_home, "codex.exe"),
|
||||
))
|
||||
)
|
||||
}
|
||||
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
fn build_sandbox_command(
|
||||
req: &ApplyPatchRequest,
|
||||
codex_self_exe: Option<&PathBuf>,
|
||||
) -> Result<SandboxCommand, ToolError> {
|
||||
) -> Result<BuiltApplyPatchCommand, ToolError> {
|
||||
let exe = Self::resolve_apply_patch_program(codex_self_exe)?;
|
||||
Ok(Self::build_sandbox_command_with_program(req, exe))
|
||||
Self::build_sandbox_command_with_program(req, exe)
|
||||
}
|
||||
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
@@ -97,18 +110,57 @@ impl ApplyPatchRuntime {
|
||||
.map_err(|e| ToolError::Rejected(format!("failed to determine codex exe: {e}")))
|
||||
}
|
||||
|
||||
fn build_sandbox_command_with_program(req: &ApplyPatchRequest, exe: PathBuf) -> SandboxCommand {
|
||||
SandboxCommand {
|
||||
fn build_sandbox_command_with_program(
|
||||
req: &ApplyPatchRequest,
|
||||
exe: PathBuf,
|
||||
) -> Result<BuiltApplyPatchCommand, ToolError> {
|
||||
let (arg1, patch_arg, payload_file) = Self::apply_patch_payload_arg(req)?;
|
||||
let command = SandboxCommand {
|
||||
program: exe.into_os_string(),
|
||||
args: vec![
|
||||
CODEX_CORE_APPLY_PATCH_ARG1.to_string(),
|
||||
req.action.patch.clone(),
|
||||
],
|
||||
args: vec![arg1.to_string(), patch_arg],
|
||||
cwd: req.action.cwd.to_path_buf(),
|
||||
// Run apply_patch with a minimal environment for determinism and to avoid leaks.
|
||||
env: HashMap::new(),
|
||||
additional_permissions: req.additional_permissions.clone(),
|
||||
};
|
||||
Ok(BuiltApplyPatchCommand {
|
||||
command,
|
||||
_payload_file: payload_file,
|
||||
})
|
||||
}
|
||||
|
||||
fn apply_patch_payload_arg(
|
||||
req: &ApplyPatchRequest,
|
||||
) -> Result<(&'static str, String, Option<NamedTempFile>), ToolError> {
|
||||
#[cfg(target_os = "windows")]
|
||||
if req.action.patch.len() > APPLY_PATCH_INLINE_ARG_MAX_BYTES {
|
||||
let mut file = tempfile::Builder::new()
|
||||
.prefix(".codex-apply-patch-")
|
||||
.suffix(".patch")
|
||||
.tempfile_in(req.action.cwd.as_path())
|
||||
.map_err(|err| {
|
||||
ToolError::Rejected(format!(
|
||||
"failed to create temporary apply_patch payload file: {err}"
|
||||
))
|
||||
})?;
|
||||
file.write_all(req.action.patch.as_bytes()).map_err(|err| {
|
||||
ToolError::Rejected(format!(
|
||||
"failed to write temporary apply_patch payload file: {err}"
|
||||
))
|
||||
})?;
|
||||
file.flush().map_err(|err| {
|
||||
ToolError::Rejected(format!(
|
||||
"failed to flush temporary apply_patch payload file: {err}"
|
||||
))
|
||||
})?;
|
||||
return Ok((
|
||||
CODEX_CORE_APPLY_PATCH_FILE_ARG1,
|
||||
file.path().to_string_lossy().into_owned(),
|
||||
Some(file),
|
||||
));
|
||||
}
|
||||
|
||||
Ok((CODEX_CORE_APPLY_PATCH_ARG1, req.action.patch.clone(), None))
|
||||
}
|
||||
|
||||
fn stdout_stream(ctx: &ToolCtx) -> Option<crate::exec::StdoutStream> {
|
||||
@@ -241,9 +293,13 @@ impl ToolRuntime<ApplyPatchRequest, ExecToolCallOutput> for ApplyPatchRuntime {
|
||||
}
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
let command = Self::build_sandbox_command(req, &ctx.turn.config.codex_home)?;
|
||||
let built_command = Self::build_sandbox_command(req, &ctx.turn.config.codex_home)?;
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
let command = Self::build_sandbox_command(req, ctx.turn.codex_self_exe.as_ref())?;
|
||||
let built_command = Self::build_sandbox_command(req, ctx.turn.codex_self_exe.as_ref())?;
|
||||
let BuiltApplyPatchCommand {
|
||||
command,
|
||||
_payload_file,
|
||||
} = built_command;
|
||||
let options = ExecOptions {
|
||||
expiration: req.timeout_ms.into(),
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
|
||||
@@ -98,7 +98,7 @@ fn build_sandbox_command_prefers_configured_codex_self_exe_for_apply_patch() {
|
||||
let command = ApplyPatchRuntime::build_sandbox_command(&request, Some(&codex_self_exe))
|
||||
.expect("build sandbox command");
|
||||
|
||||
assert_eq!(command.program, codex_self_exe.into_os_string());
|
||||
assert_eq!(command.command.program, codex_self_exe.into_os_string());
|
||||
}
|
||||
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
@@ -130,7 +130,7 @@ fn build_sandbox_command_falls_back_to_current_exe_for_apply_patch() {
|
||||
.expect("build sandbox command");
|
||||
|
||||
assert_eq!(
|
||||
command.program,
|
||||
command.command.program,
|
||||
std::env::current_exe()
|
||||
.expect("current exe")
|
||||
.into_os_string()
|
||||
|
||||
Reference in New Issue
Block a user