mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat: intercept apply_patch for unified_exec (#7446)
This commit is contained in:
committed by
GitHub
Unverified
parent
37ee6bf2c3
commit
72b95db12f
@@ -34,6 +34,7 @@ use crate::project_doc::DEFAULT_PROJECT_DOC_FILENAME;
|
||||
use crate::project_doc::LOCAL_PROJECT_DOC_FILENAME;
|
||||
use crate::protocol::AskForApproval;
|
||||
use crate::protocol::SandboxPolicy;
|
||||
use crate::util::resolve_path;
|
||||
use codex_app_server_protocol::Tools;
|
||||
use codex_app_server_protocol::UserSavedConfig;
|
||||
use codex_protocol::config_types::ForcedLoginMethod;
|
||||
@@ -1016,15 +1017,8 @@ impl Config {
|
||||
let additional_writable_roots: Vec<PathBuf> = additional_writable_roots
|
||||
.into_iter()
|
||||
.map(|path| {
|
||||
let absolute = if path.is_absolute() {
|
||||
path
|
||||
} else {
|
||||
resolved_cwd.join(path)
|
||||
};
|
||||
match canonicalize(&absolute) {
|
||||
Ok(canonical) => canonical,
|
||||
Err(_) => absolute,
|
||||
}
|
||||
let absolute = resolve_path(&resolved_cwd, &path);
|
||||
canonicalize(&absolute).unwrap_or(absolute)
|
||||
})
|
||||
.collect();
|
||||
let active_project = cfg
|
||||
@@ -1299,11 +1293,7 @@ impl Config {
|
||||
return Ok(None);
|
||||
};
|
||||
|
||||
let full_path = if p.is_relative() {
|
||||
cwd.join(p)
|
||||
} else {
|
||||
p.to_path_buf()
|
||||
};
|
||||
let full_path = resolve_path(cwd, p);
|
||||
|
||||
let contents = std::fs::read_to_string(&full_path).map_err(|e| {
|
||||
std::io::Error::new(
|
||||
|
||||
@@ -2,6 +2,7 @@ use std::collections::HashSet;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use crate::util::resolve_path;
|
||||
use codex_app_server_protocol::GitSha;
|
||||
use codex_protocol::protocol::GitInfo;
|
||||
use futures::future::join_all;
|
||||
@@ -548,11 +549,7 @@ pub fn resolve_root_git_project_for_trust(cwd: &Path) -> Option<PathBuf> {
|
||||
.trim()
|
||||
.to_string();
|
||||
|
||||
let git_dir_path_raw = if Path::new(&git_dir_s).is_absolute() {
|
||||
PathBuf::from(&git_dir_s)
|
||||
} else {
|
||||
base.join(&git_dir_s)
|
||||
};
|
||||
let git_dir_path_raw = resolve_path(base, &PathBuf::from(&git_dir_s));
|
||||
|
||||
// Normalize to handle macOS /var vs /private/var and resolve ".." segments.
|
||||
let git_dir_path = std::fs::canonicalize(&git_dir_path_raw).unwrap_or(git_dir_path_raw);
|
||||
|
||||
@@ -6,6 +6,7 @@ use codex_apply_patch::ApplyPatchAction;
|
||||
use codex_apply_patch::ApplyPatchFileChange;
|
||||
|
||||
use crate::exec::SandboxType;
|
||||
use crate::util::resolve_path;
|
||||
|
||||
use crate::protocol::AskForApproval;
|
||||
use crate::protocol::SandboxPolicy;
|
||||
@@ -150,11 +151,7 @@ fn is_write_patch_constrained_to_writable_paths(
|
||||
// and roots are converted to absolute, normalized forms before the
|
||||
// prefix check.
|
||||
let is_path_writable = |p: &PathBuf| {
|
||||
let abs = if p.is_absolute() {
|
||||
p.clone()
|
||||
} else {
|
||||
cwd.join(p)
|
||||
};
|
||||
let abs = resolve_path(cwd, p);
|
||||
let abs = match normalize(&abs) {
|
||||
Some(v) => v,
|
||||
None => return false,
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use std::collections::BTreeMap;
|
||||
use std::path::Path;
|
||||
|
||||
use crate::apply_patch;
|
||||
use crate::apply_patch::InternalApplyPatchInvocation;
|
||||
@@ -7,7 +8,10 @@ use crate::client_common::tools::FreeformTool;
|
||||
use crate::client_common::tools::FreeformToolFormat;
|
||||
use crate::client_common::tools::ResponsesApiTool;
|
||||
use crate::client_common::tools::ToolSpec;
|
||||
use crate::codex::Session;
|
||||
use crate::codex::TurnContext;
|
||||
use crate::function_tool::FunctionCallError;
|
||||
use crate::tools::context::SharedTurnDiffTracker;
|
||||
use crate::tools::context::ToolInvocation;
|
||||
use crate::tools::context::ToolOutput;
|
||||
use crate::tools::context::ToolPayload;
|
||||
@@ -164,6 +168,80 @@ pub enum ApplyPatchToolType {
|
||||
Function,
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub(crate) async fn intercept_apply_patch(
|
||||
command: &[String],
|
||||
cwd: &Path,
|
||||
timeout_ms: Option<u64>,
|
||||
session: &Session,
|
||||
turn: &TurnContext,
|
||||
tracker: Option<&SharedTurnDiffTracker>,
|
||||
call_id: &str,
|
||||
tool_name: &str,
|
||||
) -> Result<Option<ToolOutput>, FunctionCallError> {
|
||||
match codex_apply_patch::maybe_parse_apply_patch_verified(command, cwd) {
|
||||
codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => {
|
||||
match apply_patch::apply_patch(session, turn, call_id, changes).await {
|
||||
InternalApplyPatchInvocation::Output(item) => {
|
||||
let content = item?;
|
||||
Ok(Some(ToolOutput::Function {
|
||||
content,
|
||||
content_items: None,
|
||||
success: Some(true),
|
||||
}))
|
||||
}
|
||||
InternalApplyPatchInvocation::DelegateToExec(apply) => {
|
||||
let emitter = ToolEmitter::apply_patch(
|
||||
convert_apply_patch_to_protocol(&apply.action),
|
||||
!apply.user_explicitly_approved_this_action,
|
||||
);
|
||||
let event_ctx =
|
||||
ToolEventCtx::new(session, turn, call_id, tracker.as_ref().copied());
|
||||
emitter.begin(event_ctx).await;
|
||||
|
||||
let req = ApplyPatchRequest {
|
||||
patch: apply.action.patch.clone(),
|
||||
cwd: apply.action.cwd.clone(),
|
||||
timeout_ms,
|
||||
user_explicitly_approved: apply.user_explicitly_approved_this_action,
|
||||
codex_exe: turn.codex_linux_sandbox_exe.clone(),
|
||||
};
|
||||
|
||||
let mut orchestrator = ToolOrchestrator::new();
|
||||
let mut runtime = ApplyPatchRuntime::new();
|
||||
let tool_ctx = ToolCtx {
|
||||
session,
|
||||
turn,
|
||||
call_id: call_id.to_string(),
|
||||
tool_name: tool_name.to_string(),
|
||||
};
|
||||
let out = orchestrator
|
||||
.run(&mut runtime, &req, &tool_ctx, turn, turn.approval_policy)
|
||||
.await;
|
||||
let event_ctx =
|
||||
ToolEventCtx::new(session, turn, call_id, tracker.as_ref().copied());
|
||||
let content = emitter.finish(event_ctx, out).await?;
|
||||
Ok(Some(ToolOutput::Function {
|
||||
content,
|
||||
content_items: None,
|
||||
success: Some(true),
|
||||
}))
|
||||
}
|
||||
}
|
||||
}
|
||||
codex_apply_patch::MaybeApplyPatchVerified::CorrectnessError(parse_error) => {
|
||||
Err(FunctionCallError::RespondToModel(format!(
|
||||
"apply_patch verification failed: {parse_error}"
|
||||
)))
|
||||
}
|
||||
codex_apply_patch::MaybeApplyPatchVerified::ShellParseError(error) => {
|
||||
tracing::trace!("Failed to parse apply_patch input, {error:?}");
|
||||
Ok(None)
|
||||
}
|
||||
codex_apply_patch::MaybeApplyPatchVerified::NotApplyPatch => Ok(None),
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns a custom tool that can be used to edit files. Well-suited for GPT-5 models
|
||||
/// https://platform.openai.com/docs/guides/function-calling#custom-tools
|
||||
pub(crate) fn create_apply_patch_freeform_tool() -> ToolSpec {
|
||||
|
||||
@@ -3,9 +3,6 @@ use codex_protocol::models::ShellCommandToolCallParams;
|
||||
use codex_protocol::models::ShellToolCallParams;
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::apply_patch;
|
||||
use crate::apply_patch::InternalApplyPatchInvocation;
|
||||
use crate::apply_patch::convert_apply_patch_to_protocol;
|
||||
use crate::codex::TurnContext;
|
||||
use crate::exec::ExecParams;
|
||||
use crate::exec_env::create_env;
|
||||
@@ -19,11 +16,10 @@ use crate::tools::context::ToolOutput;
|
||||
use crate::tools::context::ToolPayload;
|
||||
use crate::tools::events::ToolEmitter;
|
||||
use crate::tools::events::ToolEventCtx;
|
||||
use crate::tools::handlers::apply_patch::intercept_apply_patch;
|
||||
use crate::tools::orchestrator::ToolOrchestrator;
|
||||
use crate::tools::registry::ToolHandler;
|
||||
use crate::tools::registry::ToolKind;
|
||||
use crate::tools::runtimes::apply_patch::ApplyPatchRequest;
|
||||
use crate::tools::runtimes::apply_patch::ApplyPatchRuntime;
|
||||
use crate::tools::runtimes::shell::ShellRequest;
|
||||
use crate::tools::runtimes::shell::ShellRuntime;
|
||||
use crate::tools::sandboxing::ToolCtx;
|
||||
@@ -210,81 +206,19 @@ impl ShellHandler {
|
||||
}
|
||||
|
||||
// Intercept apply_patch if present.
|
||||
match codex_apply_patch::maybe_parse_apply_patch_verified(
|
||||
if let Some(output) = intercept_apply_patch(
|
||||
&exec_params.command,
|
||||
&exec_params.cwd,
|
||||
) {
|
||||
codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => {
|
||||
match apply_patch::apply_patch(session.as_ref(), turn.as_ref(), &call_id, changes)
|
||||
.await
|
||||
{
|
||||
InternalApplyPatchInvocation::Output(item) => {
|
||||
// Programmatic apply_patch path; return its result.
|
||||
let content = item?;
|
||||
return Ok(ToolOutput::Function {
|
||||
content,
|
||||
content_items: None,
|
||||
success: Some(true),
|
||||
});
|
||||
}
|
||||
InternalApplyPatchInvocation::DelegateToExec(apply) => {
|
||||
let emitter = ToolEmitter::apply_patch(
|
||||
convert_apply_patch_to_protocol(&apply.action),
|
||||
!apply.user_explicitly_approved_this_action,
|
||||
);
|
||||
let event_ctx = ToolEventCtx::new(
|
||||
session.as_ref(),
|
||||
turn.as_ref(),
|
||||
&call_id,
|
||||
Some(&tracker),
|
||||
);
|
||||
emitter.begin(event_ctx).await;
|
||||
|
||||
let req = ApplyPatchRequest {
|
||||
patch: apply.action.patch.clone(),
|
||||
cwd: apply.action.cwd.clone(),
|
||||
timeout_ms: exec_params.expiration.timeout_ms(),
|
||||
user_explicitly_approved: apply.user_explicitly_approved_this_action,
|
||||
codex_exe: turn.codex_linux_sandbox_exe.clone(),
|
||||
};
|
||||
let mut orchestrator = ToolOrchestrator::new();
|
||||
let mut runtime = ApplyPatchRuntime::new();
|
||||
let tool_ctx = ToolCtx {
|
||||
session: session.as_ref(),
|
||||
turn: turn.as_ref(),
|
||||
call_id: call_id.clone(),
|
||||
tool_name: tool_name.to_string(),
|
||||
};
|
||||
let out = orchestrator
|
||||
.run(&mut runtime, &req, &tool_ctx, &turn, turn.approval_policy)
|
||||
.await;
|
||||
let event_ctx = ToolEventCtx::new(
|
||||
session.as_ref(),
|
||||
turn.as_ref(),
|
||||
&call_id,
|
||||
Some(&tracker),
|
||||
);
|
||||
let content = emitter.finish(event_ctx, out).await?;
|
||||
return Ok(ToolOutput::Function {
|
||||
content,
|
||||
content_items: None,
|
||||
success: Some(true),
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
codex_apply_patch::MaybeApplyPatchVerified::CorrectnessError(parse_error) => {
|
||||
return Err(FunctionCallError::RespondToModel(format!(
|
||||
"apply_patch verification failed: {parse_error}"
|
||||
)));
|
||||
}
|
||||
codex_apply_patch::MaybeApplyPatchVerified::ShellParseError(error) => {
|
||||
tracing::trace!("Failed to parse shell command, {error:?}");
|
||||
// Fall through to regular shell execution.
|
||||
}
|
||||
codex_apply_patch::MaybeApplyPatchVerified::NotApplyPatch => {
|
||||
// Fall through to regular shell execution.
|
||||
}
|
||||
exec_params.expiration.timeout_ms(),
|
||||
session.as_ref(),
|
||||
turn.as_ref(),
|
||||
Some(&tracker),
|
||||
&call_id,
|
||||
tool_name,
|
||||
)
|
||||
.await?
|
||||
{
|
||||
return Ok(output);
|
||||
}
|
||||
|
||||
let source = ExecCommandSource::Agent;
|
||||
|
||||
@@ -13,6 +13,7 @@ use crate::tools::context::ToolPayload;
|
||||
use crate::tools::events::ToolEmitter;
|
||||
use crate::tools::events::ToolEventCtx;
|
||||
use crate::tools::events::ToolEventStage;
|
||||
use crate::tools::handlers::apply_patch::intercept_apply_patch;
|
||||
use crate::tools::registry::ToolHandler;
|
||||
use crate::tools::registry::ToolKind;
|
||||
use crate::unified_exec::ExecCommandRequest;
|
||||
@@ -103,6 +104,7 @@ impl ToolHandler for UnifiedExecHandler {
|
||||
let ToolInvocation {
|
||||
session,
|
||||
turn,
|
||||
tracker,
|
||||
call_id,
|
||||
tool_name,
|
||||
payload,
|
||||
@@ -153,12 +155,26 @@ impl ToolHandler for UnifiedExecHandler {
|
||||
)));
|
||||
}
|
||||
|
||||
let workdir = workdir
|
||||
.as_deref()
|
||||
.filter(|value| !value.is_empty())
|
||||
.map(PathBuf::from);
|
||||
let workdir = workdir.filter(|value| !value.is_empty());
|
||||
|
||||
let workdir = workdir.map(|dir| context.turn.resolve_path(Some(dir)));
|
||||
let cwd = workdir.clone().unwrap_or_else(|| context.turn.cwd.clone());
|
||||
|
||||
if let Some(output) = intercept_apply_patch(
|
||||
&command,
|
||||
&cwd,
|
||||
Some(yield_time_ms),
|
||||
context.session.as_ref(),
|
||||
context.turn.as_ref(),
|
||||
Some(&tracker),
|
||||
&context.call_id,
|
||||
tool_name.as_str(),
|
||||
)
|
||||
.await?
|
||||
{
|
||||
return Ok(output);
|
||||
}
|
||||
|
||||
let event_ctx = ToolEventCtx::new(
|
||||
context.session.as_ref(),
|
||||
context.turn.as_ref(),
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::time::Duration;
|
||||
|
||||
use rand::Rng;
|
||||
@@ -37,6 +39,14 @@ pub(crate) fn try_parse_error_message(text: &str) -> String {
|
||||
text.to_string()
|
||||
}
|
||||
|
||||
pub fn resolve_path(base: &Path, path: &PathBuf) -> PathBuf {
|
||||
if path.is_absolute() {
|
||||
path.clone()
|
||||
} else {
|
||||
base.join(path)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
Reference in New Issue
Block a user