chore: split apply_patch logic out of codex.rs and into apply_patch.rs (#1703)
This is a straight refactor, moving apply-patch-related code from `codex.rs` and into the new `apply_patch.rs` file. The only "logical" change is inlining `#[allow(clippy::unwrap_used)]` instead of declaring `#![allow(clippy::unwrap_used)]` at the top of the file (which is currently the case in `codex.rs`). --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/1703). * #1705 * __->__ #1703 * #1702 * #1698 * #1697
This commit is contained in:
406
codex-rs/core/src/apply_patch.rs
Normal file
406
codex-rs/core/src/apply_patch.rs
Normal file
@@ -0,0 +1,406 @@
|
|||||||
|
use crate::codex::Session;
|
||||||
|
use crate::models::FunctionCallOutputPayload;
|
||||||
|
use crate::models::ResponseInputItem;
|
||||||
|
use crate::protocol::Event;
|
||||||
|
use crate::protocol::EventMsg;
|
||||||
|
use crate::protocol::FileChange;
|
||||||
|
use crate::protocol::PatchApplyBeginEvent;
|
||||||
|
use crate::protocol::PatchApplyEndEvent;
|
||||||
|
use crate::protocol::ReviewDecision;
|
||||||
|
use crate::safety::SafetyCheck;
|
||||||
|
use crate::safety::assess_patch_safety;
|
||||||
|
use anyhow::Context;
|
||||||
|
use codex_apply_patch::AffectedPaths;
|
||||||
|
use codex_apply_patch::ApplyPatchAction;
|
||||||
|
use codex_apply_patch::ApplyPatchFileChange;
|
||||||
|
use codex_apply_patch::print_summary;
|
||||||
|
use std::collections::HashMap;
|
||||||
|
use std::path::Path;
|
||||||
|
use std::path::PathBuf;
|
||||||
|
|
||||||
|
pub(crate) async fn apply_patch(
|
||||||
|
sess: &Session,
|
||||||
|
sub_id: String,
|
||||||
|
call_id: String,
|
||||||
|
action: ApplyPatchAction,
|
||||||
|
) -> ResponseInputItem {
|
||||||
|
let writable_roots_snapshot = {
|
||||||
|
#[allow(clippy::unwrap_used)]
|
||||||
|
let guard = sess.writable_roots.lock().unwrap();
|
||||||
|
guard.clone()
|
||||||
|
};
|
||||||
|
|
||||||
|
let auto_approved = match assess_patch_safety(
|
||||||
|
&action,
|
||||||
|
sess.approval_policy,
|
||||||
|
&writable_roots_snapshot,
|
||||||
|
&sess.cwd,
|
||||||
|
) {
|
||||||
|
SafetyCheck::AutoApprove { .. } => true,
|
||||||
|
SafetyCheck::AskUser => {
|
||||||
|
// Compute a readable summary of path changes to include in the
|
||||||
|
// approval request so the user can make an informed decision.
|
||||||
|
let rx_approve = sess
|
||||||
|
.request_patch_approval(sub_id.clone(), call_id.clone(), &action, None, None)
|
||||||
|
.await;
|
||||||
|
match rx_approve.await.unwrap_or_default() {
|
||||||
|
ReviewDecision::Approved | ReviewDecision::ApprovedForSession => false,
|
||||||
|
ReviewDecision::Denied | ReviewDecision::Abort => {
|
||||||
|
return ResponseInputItem::FunctionCallOutput {
|
||||||
|
call_id,
|
||||||
|
output: FunctionCallOutputPayload {
|
||||||
|
content: "patch rejected by user".to_string(),
|
||||||
|
success: Some(false),
|
||||||
|
},
|
||||||
|
};
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
SafetyCheck::Reject { reason } => {
|
||||||
|
return ResponseInputItem::FunctionCallOutput {
|
||||||
|
call_id,
|
||||||
|
output: FunctionCallOutputPayload {
|
||||||
|
content: format!("patch rejected: {reason}"),
|
||||||
|
success: Some(false),
|
||||||
|
},
|
||||||
|
};
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
// Verify write permissions before touching the filesystem.
|
||||||
|
let writable_snapshot = {
|
||||||
|
#[allow(clippy::unwrap_used)]
|
||||||
|
sess.writable_roots.lock().unwrap().clone()
|
||||||
|
};
|
||||||
|
|
||||||
|
if let Some(offending) = first_offending_path(&action, &writable_snapshot, &sess.cwd) {
|
||||||
|
let root = offending.parent().unwrap_or(&offending).to_path_buf();
|
||||||
|
|
||||||
|
let reason = Some(format!(
|
||||||
|
"grant write access to {} for this session",
|
||||||
|
root.display()
|
||||||
|
));
|
||||||
|
|
||||||
|
let rx = sess
|
||||||
|
.request_patch_approval(
|
||||||
|
sub_id.clone(),
|
||||||
|
call_id.clone(),
|
||||||
|
&action,
|
||||||
|
reason.clone(),
|
||||||
|
Some(root.clone()),
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
|
||||||
|
if !matches!(
|
||||||
|
rx.await.unwrap_or_default(),
|
||||||
|
ReviewDecision::Approved | ReviewDecision::ApprovedForSession
|
||||||
|
) {
|
||||||
|
return ResponseInputItem::FunctionCallOutput {
|
||||||
|
call_id,
|
||||||
|
output: FunctionCallOutputPayload {
|
||||||
|
content: "patch rejected by user".to_string(),
|
||||||
|
success: Some(false),
|
||||||
|
},
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
// user approved, extend writable roots for this session
|
||||||
|
#[allow(clippy::unwrap_used)]
|
||||||
|
sess.writable_roots.lock().unwrap().push(root);
|
||||||
|
}
|
||||||
|
|
||||||
|
let _ = sess
|
||||||
|
.tx_event
|
||||||
|
.send(Event {
|
||||||
|
id: sub_id.clone(),
|
||||||
|
msg: EventMsg::PatchApplyBegin(PatchApplyBeginEvent {
|
||||||
|
call_id: call_id.clone(),
|
||||||
|
auto_approved,
|
||||||
|
changes: convert_apply_patch_to_protocol(&action),
|
||||||
|
}),
|
||||||
|
})
|
||||||
|
.await;
|
||||||
|
|
||||||
|
let mut stdout = Vec::new();
|
||||||
|
let mut stderr = Vec::new();
|
||||||
|
// Enforce writable roots. If a write is blocked, collect offending root
|
||||||
|
// and prompt the user to extend permissions.
|
||||||
|
let mut result = apply_changes_from_apply_patch_and_report(&action, &mut stdout, &mut stderr);
|
||||||
|
|
||||||
|
if let Err(err) = &result {
|
||||||
|
if err.kind() == std::io::ErrorKind::PermissionDenied {
|
||||||
|
// Determine first offending path.
|
||||||
|
let offending_opt = action
|
||||||
|
.changes()
|
||||||
|
.iter()
|
||||||
|
.flat_map(|(path, change)| match change {
|
||||||
|
ApplyPatchFileChange::Add { .. } => vec![path.as_ref()],
|
||||||
|
ApplyPatchFileChange::Delete => vec![path.as_ref()],
|
||||||
|
ApplyPatchFileChange::Update {
|
||||||
|
move_path: Some(move_path),
|
||||||
|
..
|
||||||
|
} => {
|
||||||
|
vec![path.as_ref(), move_path.as_ref()]
|
||||||
|
}
|
||||||
|
ApplyPatchFileChange::Update {
|
||||||
|
move_path: None, ..
|
||||||
|
} => vec![path.as_ref()],
|
||||||
|
})
|
||||||
|
.find_map(|path: &Path| {
|
||||||
|
// ApplyPatchAction promises to guarantee absolute paths.
|
||||||
|
if !path.is_absolute() {
|
||||||
|
panic!("apply_patch invariant failed: path is not absolute: {path:?}");
|
||||||
|
}
|
||||||
|
|
||||||
|
let writable = {
|
||||||
|
#[allow(clippy::unwrap_used)]
|
||||||
|
let roots = sess.writable_roots.lock().unwrap();
|
||||||
|
roots.iter().any(|root| path.starts_with(root))
|
||||||
|
};
|
||||||
|
if writable {
|
||||||
|
None
|
||||||
|
} else {
|
||||||
|
Some(path.to_path_buf())
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
if let Some(offending) = offending_opt {
|
||||||
|
let root = offending.parent().unwrap_or(&offending).to_path_buf();
|
||||||
|
|
||||||
|
let reason = Some(format!(
|
||||||
|
"grant write access to {} for this session",
|
||||||
|
root.display()
|
||||||
|
));
|
||||||
|
let rx = sess
|
||||||
|
.request_patch_approval(
|
||||||
|
sub_id.clone(),
|
||||||
|
call_id.clone(),
|
||||||
|
&action,
|
||||||
|
reason.clone(),
|
||||||
|
Some(root.clone()),
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
if matches!(
|
||||||
|
rx.await.unwrap_or_default(),
|
||||||
|
ReviewDecision::Approved | ReviewDecision::ApprovedForSession
|
||||||
|
) {
|
||||||
|
// Extend writable roots.
|
||||||
|
#[allow(clippy::unwrap_used)]
|
||||||
|
sess.writable_roots.lock().unwrap().push(root);
|
||||||
|
stdout.clear();
|
||||||
|
stderr.clear();
|
||||||
|
result = apply_changes_from_apply_patch_and_report(
|
||||||
|
&action,
|
||||||
|
&mut stdout,
|
||||||
|
&mut stderr,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Emit PatchApplyEnd event.
|
||||||
|
let success_flag = result.is_ok();
|
||||||
|
let _ = sess
|
||||||
|
.tx_event
|
||||||
|
.send(Event {
|
||||||
|
id: sub_id.clone(),
|
||||||
|
msg: EventMsg::PatchApplyEnd(PatchApplyEndEvent {
|
||||||
|
call_id: call_id.clone(),
|
||||||
|
stdout: String::from_utf8_lossy(&stdout).to_string(),
|
||||||
|
stderr: String::from_utf8_lossy(&stderr).to_string(),
|
||||||
|
success: success_flag,
|
||||||
|
}),
|
||||||
|
})
|
||||||
|
.await;
|
||||||
|
|
||||||
|
match result {
|
||||||
|
Ok(_) => ResponseInputItem::FunctionCallOutput {
|
||||||
|
call_id,
|
||||||
|
output: FunctionCallOutputPayload {
|
||||||
|
content: String::from_utf8_lossy(&stdout).to_string(),
|
||||||
|
success: None,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
Err(e) => ResponseInputItem::FunctionCallOutput {
|
||||||
|
call_id,
|
||||||
|
output: FunctionCallOutputPayload {
|
||||||
|
content: format!("error: {e:#}, stderr: {}", String::from_utf8_lossy(&stderr)),
|
||||||
|
success: Some(false),
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Return the first path in `hunks` that is NOT under any of the
|
||||||
|
/// `writable_roots` (after normalising). If all paths are acceptable,
|
||||||
|
/// returns None.
|
||||||
|
fn first_offending_path(
|
||||||
|
action: &ApplyPatchAction,
|
||||||
|
writable_roots: &[PathBuf],
|
||||||
|
cwd: &Path,
|
||||||
|
) -> Option<PathBuf> {
|
||||||
|
let changes = action.changes();
|
||||||
|
for (path, change) in changes {
|
||||||
|
let candidate = match change {
|
||||||
|
ApplyPatchFileChange::Add { .. } => path,
|
||||||
|
ApplyPatchFileChange::Delete => path,
|
||||||
|
ApplyPatchFileChange::Update { move_path, .. } => move_path.as_ref().unwrap_or(path),
|
||||||
|
};
|
||||||
|
|
||||||
|
let abs = if candidate.is_absolute() {
|
||||||
|
candidate.clone()
|
||||||
|
} else {
|
||||||
|
cwd.join(candidate)
|
||||||
|
};
|
||||||
|
|
||||||
|
let mut allowed = false;
|
||||||
|
for root in writable_roots {
|
||||||
|
let root_abs = if root.is_absolute() {
|
||||||
|
root.clone()
|
||||||
|
} else {
|
||||||
|
cwd.join(root)
|
||||||
|
};
|
||||||
|
if abs.starts_with(&root_abs) {
|
||||||
|
allowed = true;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if !allowed {
|
||||||
|
return Some(candidate.clone());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
None
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(crate) fn convert_apply_patch_to_protocol(
|
||||||
|
action: &ApplyPatchAction,
|
||||||
|
) -> HashMap<PathBuf, FileChange> {
|
||||||
|
let changes = action.changes();
|
||||||
|
let mut result = HashMap::with_capacity(changes.len());
|
||||||
|
for (path, change) in changes {
|
||||||
|
let protocol_change = match change {
|
||||||
|
ApplyPatchFileChange::Add { content } => FileChange::Add {
|
||||||
|
content: content.clone(),
|
||||||
|
},
|
||||||
|
ApplyPatchFileChange::Delete => FileChange::Delete,
|
||||||
|
ApplyPatchFileChange::Update {
|
||||||
|
unified_diff,
|
||||||
|
move_path,
|
||||||
|
new_content: _new_content,
|
||||||
|
} => FileChange::Update {
|
||||||
|
unified_diff: unified_diff.clone(),
|
||||||
|
move_path: move_path.clone(),
|
||||||
|
},
|
||||||
|
};
|
||||||
|
result.insert(path.clone(), protocol_change);
|
||||||
|
}
|
||||||
|
result
|
||||||
|
}
|
||||||
|
|
||||||
|
fn apply_changes_from_apply_patch_and_report(
|
||||||
|
action: &ApplyPatchAction,
|
||||||
|
stdout: &mut impl std::io::Write,
|
||||||
|
stderr: &mut impl std::io::Write,
|
||||||
|
) -> std::io::Result<()> {
|
||||||
|
match apply_changes_from_apply_patch(action) {
|
||||||
|
Ok(affected_paths) => {
|
||||||
|
print_summary(&affected_paths, stdout)?;
|
||||||
|
}
|
||||||
|
Err(err) => {
|
||||||
|
writeln!(stderr, "{err:?}")?;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
fn apply_changes_from_apply_patch(action: &ApplyPatchAction) -> anyhow::Result<AffectedPaths> {
|
||||||
|
let mut added: Vec<PathBuf> = Vec::new();
|
||||||
|
let mut modified: Vec<PathBuf> = Vec::new();
|
||||||
|
let mut deleted: Vec<PathBuf> = Vec::new();
|
||||||
|
|
||||||
|
let changes = action.changes();
|
||||||
|
for (path, change) in changes {
|
||||||
|
match change {
|
||||||
|
ApplyPatchFileChange::Add { content } => {
|
||||||
|
if let Some(parent) = path.parent() {
|
||||||
|
if !parent.as_os_str().is_empty() {
|
||||||
|
std::fs::create_dir_all(parent).with_context(|| {
|
||||||
|
format!("Failed to create parent directories for {}", path.display())
|
||||||
|
})?;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
std::fs::write(path, content)
|
||||||
|
.with_context(|| format!("Failed to write file {}", path.display()))?;
|
||||||
|
added.push(path.clone());
|
||||||
|
}
|
||||||
|
ApplyPatchFileChange::Delete => {
|
||||||
|
std::fs::remove_file(path)
|
||||||
|
.with_context(|| format!("Failed to delete file {}", path.display()))?;
|
||||||
|
deleted.push(path.clone());
|
||||||
|
}
|
||||||
|
ApplyPatchFileChange::Update {
|
||||||
|
unified_diff: _unified_diff,
|
||||||
|
move_path,
|
||||||
|
new_content,
|
||||||
|
} => {
|
||||||
|
if let Some(move_path) = move_path {
|
||||||
|
if let Some(parent) = move_path.parent() {
|
||||||
|
if !parent.as_os_str().is_empty() {
|
||||||
|
std::fs::create_dir_all(parent).with_context(|| {
|
||||||
|
format!(
|
||||||
|
"Failed to create parent directories for {}",
|
||||||
|
move_path.display()
|
||||||
|
)
|
||||||
|
})?;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
std::fs::rename(path, move_path)
|
||||||
|
.with_context(|| format!("Failed to rename file {}", path.display()))?;
|
||||||
|
std::fs::write(move_path, new_content)?;
|
||||||
|
modified.push(move_path.clone());
|
||||||
|
deleted.push(path.clone());
|
||||||
|
} else {
|
||||||
|
std::fs::write(path, new_content)?;
|
||||||
|
modified.push(path.clone());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
Ok(AffectedPaths {
|
||||||
|
added,
|
||||||
|
modified,
|
||||||
|
deleted,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(crate) fn get_writable_roots(cwd: &Path) -> Vec<PathBuf> {
|
||||||
|
let mut writable_roots = Vec::new();
|
||||||
|
if cfg!(target_os = "macos") {
|
||||||
|
// On macOS, $TMPDIR is private to the user.
|
||||||
|
writable_roots.push(std::env::temp_dir());
|
||||||
|
|
||||||
|
// Allow pyenv to update its shims directory. Without this, any tool
|
||||||
|
// that happens to be managed by `pyenv` will fail with an error like:
|
||||||
|
//
|
||||||
|
// pyenv: cannot rehash: $HOME/.pyenv/shims isn't writable
|
||||||
|
//
|
||||||
|
// which is emitted every time `pyenv` tries to run `rehash` (for
|
||||||
|
// example, after installing a new Python package that drops an entry
|
||||||
|
// point). Although the sandbox is intentionally read‑only by default,
|
||||||
|
// writing to the user's local `pyenv` directory is safe because it
|
||||||
|
// is already user‑writable and scoped to the current user account.
|
||||||
|
if let Ok(home_dir) = std::env::var("HOME") {
|
||||||
|
let pyenv_dir = PathBuf::from(home_dir).join(".pyenv");
|
||||||
|
writable_roots.push(pyenv_dir);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
writable_roots.push(cwd.to_path_buf());
|
||||||
|
|
||||||
|
writable_roots
|
||||||
|
}
|
||||||
@@ -4,22 +4,17 @@
|
|||||||
use std::borrow::Cow;
|
use std::borrow::Cow;
|
||||||
use std::collections::HashMap;
|
use std::collections::HashMap;
|
||||||
use std::collections::HashSet;
|
use std::collections::HashSet;
|
||||||
use std::path::Path;
|
|
||||||
use std::path::PathBuf;
|
use std::path::PathBuf;
|
||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
use std::sync::Mutex;
|
use std::sync::Mutex;
|
||||||
use std::sync::atomic::AtomicU64;
|
use std::sync::atomic::AtomicU64;
|
||||||
use std::time::Duration;
|
use std::time::Duration;
|
||||||
|
|
||||||
use anyhow::Context;
|
|
||||||
use async_channel::Receiver;
|
use async_channel::Receiver;
|
||||||
use async_channel::Sender;
|
use async_channel::Sender;
|
||||||
use codex_apply_patch::AffectedPaths;
|
|
||||||
use codex_apply_patch::ApplyPatchAction;
|
use codex_apply_patch::ApplyPatchAction;
|
||||||
use codex_apply_patch::ApplyPatchFileChange;
|
|
||||||
use codex_apply_patch::MaybeApplyPatchVerified;
|
use codex_apply_patch::MaybeApplyPatchVerified;
|
||||||
use codex_apply_patch::maybe_parse_apply_patch_verified;
|
use codex_apply_patch::maybe_parse_apply_patch_verified;
|
||||||
use codex_apply_patch::print_summary;
|
|
||||||
use futures::prelude::*;
|
use futures::prelude::*;
|
||||||
use mcp_types::CallToolResult;
|
use mcp_types::CallToolResult;
|
||||||
use serde::Serialize;
|
use serde::Serialize;
|
||||||
@@ -34,6 +29,9 @@ use tracing::trace;
|
|||||||
use tracing::warn;
|
use tracing::warn;
|
||||||
use uuid::Uuid;
|
use uuid::Uuid;
|
||||||
|
|
||||||
|
use crate::apply_patch::convert_apply_patch_to_protocol;
|
||||||
|
use crate::apply_patch::get_writable_roots;
|
||||||
|
use crate::apply_patch::{self};
|
||||||
use crate::client::ModelClient;
|
use crate::client::ModelClient;
|
||||||
use crate::client_common::Prompt;
|
use crate::client_common::Prompt;
|
||||||
use crate::client_common::ResponseEvent;
|
use crate::client_common::ResponseEvent;
|
||||||
@@ -71,11 +69,8 @@ use crate::protocol::EventMsg;
|
|||||||
use crate::protocol::ExecApprovalRequestEvent;
|
use crate::protocol::ExecApprovalRequestEvent;
|
||||||
use crate::protocol::ExecCommandBeginEvent;
|
use crate::protocol::ExecCommandBeginEvent;
|
||||||
use crate::protocol::ExecCommandEndEvent;
|
use crate::protocol::ExecCommandEndEvent;
|
||||||
use crate::protocol::FileChange;
|
|
||||||
use crate::protocol::InputItem;
|
use crate::protocol::InputItem;
|
||||||
use crate::protocol::Op;
|
use crate::protocol::Op;
|
||||||
use crate::protocol::PatchApplyBeginEvent;
|
|
||||||
use crate::protocol::PatchApplyEndEvent;
|
|
||||||
use crate::protocol::ReviewDecision;
|
use crate::protocol::ReviewDecision;
|
||||||
use crate::protocol::SandboxPolicy;
|
use crate::protocol::SandboxPolicy;
|
||||||
use crate::protocol::SessionConfiguredEvent;
|
use crate::protocol::SessionConfiguredEvent;
|
||||||
@@ -84,7 +79,6 @@ use crate::protocol::TaskCompleteEvent;
|
|||||||
use crate::rollout::RolloutRecorder;
|
use crate::rollout::RolloutRecorder;
|
||||||
use crate::safety::SafetyCheck;
|
use crate::safety::SafetyCheck;
|
||||||
use crate::safety::assess_command_safety;
|
use crate::safety::assess_command_safety;
|
||||||
use crate::safety::assess_patch_safety;
|
|
||||||
use crate::shell;
|
use crate::shell;
|
||||||
use crate::user_notification::UserNotification;
|
use crate::user_notification::UserNotification;
|
||||||
use crate::util::backoff;
|
use crate::util::backoff;
|
||||||
@@ -189,19 +183,19 @@ impl Codex {
|
|||||||
/// A session has at most 1 running task at a time, and can be interrupted by user input.
|
/// A session has at most 1 running task at a time, and can be interrupted by user input.
|
||||||
pub(crate) struct Session {
|
pub(crate) struct Session {
|
||||||
client: ModelClient,
|
client: ModelClient,
|
||||||
tx_event: Sender<Event>,
|
pub(crate) tx_event: Sender<Event>,
|
||||||
ctrl_c: Arc<Notify>,
|
ctrl_c: Arc<Notify>,
|
||||||
|
|
||||||
/// The session's current working directory. All relative paths provided by
|
/// The session's current working directory. All relative paths provided by
|
||||||
/// the model as well as sandbox policies are resolved against this path
|
/// the model as well as sandbox policies are resolved against this path
|
||||||
/// instead of `std::env::current_dir()`.
|
/// instead of `std::env::current_dir()`.
|
||||||
cwd: PathBuf,
|
pub(crate) cwd: PathBuf,
|
||||||
base_instructions: Option<String>,
|
base_instructions: Option<String>,
|
||||||
user_instructions: Option<String>,
|
user_instructions: Option<String>,
|
||||||
approval_policy: AskForApproval,
|
pub(crate) approval_policy: AskForApproval,
|
||||||
sandbox_policy: SandboxPolicy,
|
sandbox_policy: SandboxPolicy,
|
||||||
shell_environment_policy: ShellEnvironmentPolicy,
|
shell_environment_policy: ShellEnvironmentPolicy,
|
||||||
writable_roots: Mutex<Vec<PathBuf>>,
|
pub(crate) writable_roots: Mutex<Vec<PathBuf>>,
|
||||||
disable_response_storage: bool,
|
disable_response_storage: bool,
|
||||||
|
|
||||||
/// Manager for external MCP servers/tools.
|
/// Manager for external MCP servers/tools.
|
||||||
@@ -1419,7 +1413,7 @@ async fn handle_container_exec_with_params(
|
|||||||
// check if this was a patch, and apply it if so
|
// check if this was a patch, and apply it if so
|
||||||
match maybe_parse_apply_patch_verified(¶ms.command, ¶ms.cwd) {
|
match maybe_parse_apply_patch_verified(¶ms.command, ¶ms.cwd) {
|
||||||
MaybeApplyPatchVerified::Body(changes) => {
|
MaybeApplyPatchVerified::Body(changes) => {
|
||||||
return apply_patch(sess, sub_id, call_id, changes).await;
|
return apply_patch::apply_patch(sess, sub_id, call_id, changes).await;
|
||||||
}
|
}
|
||||||
MaybeApplyPatchVerified::CorrectnessError(parse_error) => {
|
MaybeApplyPatchVerified::CorrectnessError(parse_error) => {
|
||||||
// It looks like an invocation of `apply_patch`, but we
|
// It looks like an invocation of `apply_patch`, but we
|
||||||
@@ -1668,384 +1662,6 @@ async fn handle_sandbox_error(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn apply_patch(
|
|
||||||
sess: &Session,
|
|
||||||
sub_id: String,
|
|
||||||
call_id: String,
|
|
||||||
action: ApplyPatchAction,
|
|
||||||
) -> ResponseInputItem {
|
|
||||||
let writable_roots_snapshot = {
|
|
||||||
let guard = sess.writable_roots.lock().unwrap();
|
|
||||||
guard.clone()
|
|
||||||
};
|
|
||||||
|
|
||||||
let auto_approved = match assess_patch_safety(
|
|
||||||
&action,
|
|
||||||
sess.approval_policy,
|
|
||||||
&writable_roots_snapshot,
|
|
||||||
&sess.cwd,
|
|
||||||
) {
|
|
||||||
SafetyCheck::AutoApprove { .. } => true,
|
|
||||||
SafetyCheck::AskUser => {
|
|
||||||
// Compute a readable summary of path changes to include in the
|
|
||||||
// approval request so the user can make an informed decision.
|
|
||||||
let rx_approve = sess
|
|
||||||
.request_patch_approval(sub_id.clone(), call_id.clone(), &action, None, None)
|
|
||||||
.await;
|
|
||||||
match rx_approve.await.unwrap_or_default() {
|
|
||||||
ReviewDecision::Approved | ReviewDecision::ApprovedForSession => false,
|
|
||||||
ReviewDecision::Denied | ReviewDecision::Abort => {
|
|
||||||
return ResponseInputItem::FunctionCallOutput {
|
|
||||||
call_id,
|
|
||||||
output: FunctionCallOutputPayload {
|
|
||||||
content: "patch rejected by user".to_string(),
|
|
||||||
success: Some(false),
|
|
||||||
},
|
|
||||||
};
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
SafetyCheck::Reject { reason } => {
|
|
||||||
return ResponseInputItem::FunctionCallOutput {
|
|
||||||
call_id,
|
|
||||||
output: FunctionCallOutputPayload {
|
|
||||||
content: format!("patch rejected: {reason}"),
|
|
||||||
success: Some(false),
|
|
||||||
},
|
|
||||||
};
|
|
||||||
}
|
|
||||||
};
|
|
||||||
|
|
||||||
// Verify write permissions before touching the filesystem.
|
|
||||||
let writable_snapshot = { sess.writable_roots.lock().unwrap().clone() };
|
|
||||||
|
|
||||||
if let Some(offending) = first_offending_path(&action, &writable_snapshot, &sess.cwd) {
|
|
||||||
let root = offending.parent().unwrap_or(&offending).to_path_buf();
|
|
||||||
|
|
||||||
let reason = Some(format!(
|
|
||||||
"grant write access to {} for this session",
|
|
||||||
root.display()
|
|
||||||
));
|
|
||||||
|
|
||||||
let rx = sess
|
|
||||||
.request_patch_approval(
|
|
||||||
sub_id.clone(),
|
|
||||||
call_id.clone(),
|
|
||||||
&action,
|
|
||||||
reason.clone(),
|
|
||||||
Some(root.clone()),
|
|
||||||
)
|
|
||||||
.await;
|
|
||||||
|
|
||||||
if !matches!(
|
|
||||||
rx.await.unwrap_or_default(),
|
|
||||||
ReviewDecision::Approved | ReviewDecision::ApprovedForSession
|
|
||||||
) {
|
|
||||||
return ResponseInputItem::FunctionCallOutput {
|
|
||||||
call_id,
|
|
||||||
output: FunctionCallOutputPayload {
|
|
||||||
content: "patch rejected by user".to_string(),
|
|
||||||
success: Some(false),
|
|
||||||
},
|
|
||||||
};
|
|
||||||
}
|
|
||||||
|
|
||||||
// user approved, extend writable roots for this session
|
|
||||||
sess.writable_roots.lock().unwrap().push(root);
|
|
||||||
}
|
|
||||||
|
|
||||||
let _ = sess
|
|
||||||
.tx_event
|
|
||||||
.send(Event {
|
|
||||||
id: sub_id.clone(),
|
|
||||||
msg: EventMsg::PatchApplyBegin(PatchApplyBeginEvent {
|
|
||||||
call_id: call_id.clone(),
|
|
||||||
auto_approved,
|
|
||||||
changes: convert_apply_patch_to_protocol(&action),
|
|
||||||
}),
|
|
||||||
})
|
|
||||||
.await;
|
|
||||||
|
|
||||||
let mut stdout = Vec::new();
|
|
||||||
let mut stderr = Vec::new();
|
|
||||||
// Enforce writable roots. If a write is blocked, collect offending root
|
|
||||||
// and prompt the user to extend permissions.
|
|
||||||
let mut result = apply_changes_from_apply_patch_and_report(&action, &mut stdout, &mut stderr);
|
|
||||||
|
|
||||||
if let Err(err) = &result {
|
|
||||||
if err.kind() == std::io::ErrorKind::PermissionDenied {
|
|
||||||
// Determine first offending path.
|
|
||||||
let offending_opt = action
|
|
||||||
.changes()
|
|
||||||
.iter()
|
|
||||||
.flat_map(|(path, change)| match change {
|
|
||||||
ApplyPatchFileChange::Add { .. } => vec![path.as_ref()],
|
|
||||||
ApplyPatchFileChange::Delete => vec![path.as_ref()],
|
|
||||||
ApplyPatchFileChange::Update {
|
|
||||||
move_path: Some(move_path),
|
|
||||||
..
|
|
||||||
} => {
|
|
||||||
vec![path.as_ref(), move_path.as_ref()]
|
|
||||||
}
|
|
||||||
ApplyPatchFileChange::Update {
|
|
||||||
move_path: None, ..
|
|
||||||
} => vec![path.as_ref()],
|
|
||||||
})
|
|
||||||
.find_map(|path: &Path| {
|
|
||||||
// ApplyPatchAction promises to guarantee absolute paths.
|
|
||||||
if !path.is_absolute() {
|
|
||||||
panic!("apply_patch invariant failed: path is not absolute: {path:?}");
|
|
||||||
}
|
|
||||||
|
|
||||||
let writable = {
|
|
||||||
let roots = sess.writable_roots.lock().unwrap();
|
|
||||||
roots.iter().any(|root| path.starts_with(root))
|
|
||||||
};
|
|
||||||
if writable {
|
|
||||||
None
|
|
||||||
} else {
|
|
||||||
Some(path.to_path_buf())
|
|
||||||
}
|
|
||||||
});
|
|
||||||
|
|
||||||
if let Some(offending) = offending_opt {
|
|
||||||
let root = offending.parent().unwrap_or(&offending).to_path_buf();
|
|
||||||
|
|
||||||
let reason = Some(format!(
|
|
||||||
"grant write access to {} for this session",
|
|
||||||
root.display()
|
|
||||||
));
|
|
||||||
let rx = sess
|
|
||||||
.request_patch_approval(
|
|
||||||
sub_id.clone(),
|
|
||||||
call_id.clone(),
|
|
||||||
&action,
|
|
||||||
reason.clone(),
|
|
||||||
Some(root.clone()),
|
|
||||||
)
|
|
||||||
.await;
|
|
||||||
if matches!(
|
|
||||||
rx.await.unwrap_or_default(),
|
|
||||||
ReviewDecision::Approved | ReviewDecision::ApprovedForSession
|
|
||||||
) {
|
|
||||||
// Extend writable roots.
|
|
||||||
sess.writable_roots.lock().unwrap().push(root);
|
|
||||||
stdout.clear();
|
|
||||||
stderr.clear();
|
|
||||||
result = apply_changes_from_apply_patch_and_report(
|
|
||||||
&action,
|
|
||||||
&mut stdout,
|
|
||||||
&mut stderr,
|
|
||||||
);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Emit PatchApplyEnd event.
|
|
||||||
let success_flag = result.is_ok();
|
|
||||||
let _ = sess
|
|
||||||
.tx_event
|
|
||||||
.send(Event {
|
|
||||||
id: sub_id.clone(),
|
|
||||||
msg: EventMsg::PatchApplyEnd(PatchApplyEndEvent {
|
|
||||||
call_id: call_id.clone(),
|
|
||||||
stdout: String::from_utf8_lossy(&stdout).to_string(),
|
|
||||||
stderr: String::from_utf8_lossy(&stderr).to_string(),
|
|
||||||
success: success_flag,
|
|
||||||
}),
|
|
||||||
})
|
|
||||||
.await;
|
|
||||||
|
|
||||||
match result {
|
|
||||||
Ok(_) => ResponseInputItem::FunctionCallOutput {
|
|
||||||
call_id,
|
|
||||||
output: FunctionCallOutputPayload {
|
|
||||||
content: String::from_utf8_lossy(&stdout).to_string(),
|
|
||||||
success: None,
|
|
||||||
},
|
|
||||||
},
|
|
||||||
Err(e) => ResponseInputItem::FunctionCallOutput {
|
|
||||||
call_id,
|
|
||||||
output: FunctionCallOutputPayload {
|
|
||||||
content: format!("error: {e:#}, stderr: {}", String::from_utf8_lossy(&stderr)),
|
|
||||||
success: Some(false),
|
|
||||||
},
|
|
||||||
},
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Return the first path in `hunks` that is NOT under any of the
|
|
||||||
/// `writable_roots` (after normalising). If all paths are acceptable,
|
|
||||||
/// returns None.
|
|
||||||
fn first_offending_path(
|
|
||||||
action: &ApplyPatchAction,
|
|
||||||
writable_roots: &[PathBuf],
|
|
||||||
cwd: &Path,
|
|
||||||
) -> Option<PathBuf> {
|
|
||||||
let changes = action.changes();
|
|
||||||
for (path, change) in changes {
|
|
||||||
let candidate = match change {
|
|
||||||
ApplyPatchFileChange::Add { .. } => path,
|
|
||||||
ApplyPatchFileChange::Delete => path,
|
|
||||||
ApplyPatchFileChange::Update { move_path, .. } => move_path.as_ref().unwrap_or(path),
|
|
||||||
};
|
|
||||||
|
|
||||||
let abs = if candidate.is_absolute() {
|
|
||||||
candidate.clone()
|
|
||||||
} else {
|
|
||||||
cwd.join(candidate)
|
|
||||||
};
|
|
||||||
|
|
||||||
let mut allowed = false;
|
|
||||||
for root in writable_roots {
|
|
||||||
let root_abs = if root.is_absolute() {
|
|
||||||
root.clone()
|
|
||||||
} else {
|
|
||||||
cwd.join(root)
|
|
||||||
};
|
|
||||||
if abs.starts_with(&root_abs) {
|
|
||||||
allowed = true;
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
if !allowed {
|
|
||||||
return Some(candidate.clone());
|
|
||||||
}
|
|
||||||
}
|
|
||||||
None
|
|
||||||
}
|
|
||||||
|
|
||||||
fn convert_apply_patch_to_protocol(action: &ApplyPatchAction) -> HashMap<PathBuf, FileChange> {
|
|
||||||
let changes = action.changes();
|
|
||||||
let mut result = HashMap::with_capacity(changes.len());
|
|
||||||
for (path, change) in changes {
|
|
||||||
let protocol_change = match change {
|
|
||||||
ApplyPatchFileChange::Add { content } => FileChange::Add {
|
|
||||||
content: content.clone(),
|
|
||||||
},
|
|
||||||
ApplyPatchFileChange::Delete => FileChange::Delete,
|
|
||||||
ApplyPatchFileChange::Update {
|
|
||||||
unified_diff,
|
|
||||||
move_path,
|
|
||||||
new_content: _new_content,
|
|
||||||
} => FileChange::Update {
|
|
||||||
unified_diff: unified_diff.clone(),
|
|
||||||
move_path: move_path.clone(),
|
|
||||||
},
|
|
||||||
};
|
|
||||||
result.insert(path.clone(), protocol_change);
|
|
||||||
}
|
|
||||||
result
|
|
||||||
}
|
|
||||||
|
|
||||||
fn apply_changes_from_apply_patch_and_report(
|
|
||||||
action: &ApplyPatchAction,
|
|
||||||
stdout: &mut impl std::io::Write,
|
|
||||||
stderr: &mut impl std::io::Write,
|
|
||||||
) -> std::io::Result<()> {
|
|
||||||
match apply_changes_from_apply_patch(action) {
|
|
||||||
Ok(affected_paths) => {
|
|
||||||
print_summary(&affected_paths, stdout)?;
|
|
||||||
}
|
|
||||||
Err(err) => {
|
|
||||||
writeln!(stderr, "{err:?}")?;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
Ok(())
|
|
||||||
}
|
|
||||||
|
|
||||||
fn apply_changes_from_apply_patch(action: &ApplyPatchAction) -> anyhow::Result<AffectedPaths> {
|
|
||||||
let mut added: Vec<PathBuf> = Vec::new();
|
|
||||||
let mut modified: Vec<PathBuf> = Vec::new();
|
|
||||||
let mut deleted: Vec<PathBuf> = Vec::new();
|
|
||||||
|
|
||||||
let changes = action.changes();
|
|
||||||
for (path, change) in changes {
|
|
||||||
match change {
|
|
||||||
ApplyPatchFileChange::Add { content } => {
|
|
||||||
if let Some(parent) = path.parent() {
|
|
||||||
if !parent.as_os_str().is_empty() {
|
|
||||||
std::fs::create_dir_all(parent).with_context(|| {
|
|
||||||
format!("Failed to create parent directories for {}", path.display())
|
|
||||||
})?;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
std::fs::write(path, content)
|
|
||||||
.with_context(|| format!("Failed to write file {}", path.display()))?;
|
|
||||||
added.push(path.clone());
|
|
||||||
}
|
|
||||||
ApplyPatchFileChange::Delete => {
|
|
||||||
std::fs::remove_file(path)
|
|
||||||
.with_context(|| format!("Failed to delete file {}", path.display()))?;
|
|
||||||
deleted.push(path.clone());
|
|
||||||
}
|
|
||||||
ApplyPatchFileChange::Update {
|
|
||||||
unified_diff: _unified_diff,
|
|
||||||
move_path,
|
|
||||||
new_content,
|
|
||||||
} => {
|
|
||||||
if let Some(move_path) = move_path {
|
|
||||||
if let Some(parent) = move_path.parent() {
|
|
||||||
if !parent.as_os_str().is_empty() {
|
|
||||||
std::fs::create_dir_all(parent).with_context(|| {
|
|
||||||
format!(
|
|
||||||
"Failed to create parent directories for {}",
|
|
||||||
move_path.display()
|
|
||||||
)
|
|
||||||
})?;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
std::fs::rename(path, move_path)
|
|
||||||
.with_context(|| format!("Failed to rename file {}", path.display()))?;
|
|
||||||
std::fs::write(move_path, new_content)?;
|
|
||||||
modified.push(move_path.clone());
|
|
||||||
deleted.push(path.clone());
|
|
||||||
} else {
|
|
||||||
std::fs::write(path, new_content)?;
|
|
||||||
modified.push(path.clone());
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
Ok(AffectedPaths {
|
|
||||||
added,
|
|
||||||
modified,
|
|
||||||
deleted,
|
|
||||||
})
|
|
||||||
}
|
|
||||||
|
|
||||||
fn get_writable_roots(cwd: &Path) -> Vec<PathBuf> {
|
|
||||||
let mut writable_roots = Vec::new();
|
|
||||||
if cfg!(target_os = "macos") {
|
|
||||||
// On macOS, $TMPDIR is private to the user.
|
|
||||||
writable_roots.push(std::env::temp_dir());
|
|
||||||
|
|
||||||
// Allow pyenv to update its shims directory. Without this, any tool
|
|
||||||
// that happens to be managed by `pyenv` will fail with an error like:
|
|
||||||
//
|
|
||||||
// pyenv: cannot rehash: $HOME/.pyenv/shims isn't writable
|
|
||||||
//
|
|
||||||
// which is emitted every time `pyenv` tries to run `rehash` (for
|
|
||||||
// example, after installing a new Python package that drops an entry
|
|
||||||
// point). Although the sandbox is intentionally read‑only by default,
|
|
||||||
// writing to the user's local `pyenv` directory is safe because it
|
|
||||||
// is already user‑writable and scoped to the current user account.
|
|
||||||
if let Ok(home_dir) = std::env::var("HOME") {
|
|
||||||
let pyenv_dir = PathBuf::from(home_dir).join(".pyenv");
|
|
||||||
writable_roots.push(pyenv_dir);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
writable_roots.push(cwd.to_path_buf());
|
|
||||||
|
|
||||||
writable_roots
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Exec output is a pre-serialized JSON payload
|
/// Exec output is a pre-serialized JSON payload
|
||||||
fn format_exec_output(output: &str, exit_code: i32, duration: Duration) -> String {
|
fn format_exec_output(output: &str, exit_code: i32, duration: Duration) -> String {
|
||||||
#[derive(Serialize)]
|
#[derive(Serialize)]
|
||||||
|
|||||||
@@ -5,6 +5,7 @@
|
|||||||
// the TUI or the tracing stack).
|
// the TUI or the tracing stack).
|
||||||
#![deny(clippy::print_stdout, clippy::print_stderr)]
|
#![deny(clippy::print_stdout, clippy::print_stderr)]
|
||||||
|
|
||||||
|
mod apply_patch;
|
||||||
mod bash;
|
mod bash;
|
||||||
mod chat_completions;
|
mod chat_completions;
|
||||||
mod client;
|
mod client;
|
||||||
|
|||||||
Reference in New Issue
Block a user