Skip to content

Commit e00eb50

Browse files
authored
feat: only wait for mutating tools for ghost commit (#6534)
1 parent 7d9ad3e commit e00eb50

File tree

5 files changed

+46
-12
lines changed

5 files changed

+46
-12
lines changed

codex-rs/core/src/tools/handlers/apply_patch.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ impl ToolHandler for ApplyPatchHandler {
4242
)
4343
}
4444

45+
fn is_mutating(&self, _invocation: &ToolInvocation) -> bool {
46+
true
47+
}
48+
4549
async fn handle(&self, invocation: ToolInvocation) -> Result<ToolOutput, FunctionCallError> {
4650
let ToolInvocation {
4751
session,

codex-rs/core/src/tools/handlers/shell.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use crate::codex::TurnContext;
1010
use crate::exec::ExecParams;
1111
use crate::exec_env::create_env;
1212
use crate::function_tool::FunctionCallError;
13+
use crate::is_safe_command::is_known_safe_command;
1314
use crate::tools::context::ToolInvocation;
1415
use crate::tools::context::ToolOutput;
1516
use crate::tools::context::ToolPayload;
@@ -77,6 +78,18 @@ impl ToolHandler for ShellHandler {
7778
)
7879
}
7980

81+
fn is_mutating(&self, invocation: &ToolInvocation) -> bool {
82+
match &invocation.payload {
83+
ToolPayload::Function { arguments } => {
84+
serde_json::from_str::<ShellToolCallParams>(arguments)
85+
.map(|params| !is_known_safe_command(&params.command))
86+
.unwrap_or(true)
87+
}
88+
ToolPayload::LocalShell { params } => !is_known_safe_command(&params.command),
89+
_ => true, // unknown payloads => assume mutating
90+
}
91+
}
92+
8093
async fn handle(&self, invocation: ToolInvocation) -> Result<ToolOutput, FunctionCallError> {
8194
let ToolInvocation {
8295
session,

codex-rs/core/src/tools/handlers/unified_exec.rs

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
use std::path::PathBuf;
22

3-
use async_trait::async_trait;
4-
use serde::Deserialize;
5-
63
use crate::function_tool::FunctionCallError;
4+
use crate::is_safe_command::is_known_safe_command;
75
use crate::protocol::EventMsg;
86
use crate::protocol::ExecCommandOutputDeltaEvent;
97
use crate::protocol::ExecOutputStream;
@@ -20,6 +18,8 @@ use crate::unified_exec::UnifiedExecContext;
2018
use crate::unified_exec::UnifiedExecResponse;
2119
use crate::unified_exec::UnifiedExecSessionManager;
2220
use crate::unified_exec::WriteStdinRequest;
21+
use async_trait::async_trait;
22+
use serde::Deserialize;
2323

2424
pub struct UnifiedExecHandler;
2525

@@ -74,6 +74,19 @@ impl ToolHandler for UnifiedExecHandler {
7474
)
7575
}
7676

77+
fn is_mutating(&self, invocation: &ToolInvocation) -> bool {
78+
let (ToolPayload::Function { arguments } | ToolPayload::UnifiedExec { arguments }) =
79+
&invocation.payload
80+
else {
81+
return true;
82+
};
83+
84+
let Ok(params) = serde_json::from_str::<ExecCommandArgs>(arguments) else {
85+
return true;
86+
};
87+
!is_known_safe_command(&["bash".to_string(), "-lc".to_string(), params.cmd])
88+
}
89+
7790
async fn handle(&self, invocation: ToolInvocation) -> Result<ToolOutput, FunctionCallError> {
7891
let ToolInvocation {
7992
session,

codex-rs/core/src/tools/parallel.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ use crate::tools::router::ToolCall;
1616
use crate::tools::router::ToolRouter;
1717
use codex_protocol::models::FunctionCallOutputPayload;
1818
use codex_protocol::models::ResponseInputItem;
19-
use codex_utils_readiness::Readiness;
2019

2120
pub(crate) struct ToolCallRuntime {
2221
router: Arc<ToolRouter>,
@@ -55,7 +54,6 @@ impl ToolCallRuntime {
5554
let tracker = Arc::clone(&self.tracker);
5655
let lock = Arc::clone(&self.parallel_execution);
5756
let started = Instant::now();
58-
let readiness = self.turn_context.tool_call_gate.clone();
5957

6058
let handle: AbortOnDropHandle<Result<ResponseInputItem, FunctionCallError>> =
6159
AbortOnDropHandle::new(tokio::spawn(async move {
@@ -65,9 +63,6 @@ impl ToolCallRuntime {
6563
Ok(Self::aborted_response(&call, secs))
6664
},
6765
res = async {
68-
tracing::trace!("waiting for tool gate");
69-
readiness.wait_ready().await;
70-
tracing::trace!("tool gate released");
7166
let _guard = if supports_parallel {
7267
Either::Left(lock.read().await)
7368
} else {

codex-rs/core/src/tools/registry.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,15 @@ use std::collections::HashMap;
22
use std::sync::Arc;
33
use std::time::Duration;
44

5-
use async_trait::async_trait;
6-
use codex_protocol::models::ResponseInputItem;
7-
use tracing::warn;
8-
95
use crate::client_common::tools::ToolSpec;
106
use crate::function_tool::FunctionCallError;
117
use crate::tools::context::ToolInvocation;
128
use crate::tools::context::ToolOutput;
139
use crate::tools::context::ToolPayload;
10+
use async_trait::async_trait;
11+
use codex_protocol::models::ResponseInputItem;
12+
use codex_utils_readiness::Readiness;
13+
use tracing::warn;
1414

1515
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
1616
pub enum ToolKind {
@@ -30,6 +30,10 @@ pub trait ToolHandler: Send + Sync {
3030
)
3131
}
3232

33+
fn is_mutating(&self, _invocation: &ToolInvocation) -> bool {
34+
false
35+
}
36+
3337
async fn handle(&self, invocation: ToolInvocation) -> Result<ToolOutput, FunctionCallError>;
3438
}
3539

@@ -106,6 +110,11 @@ impl ToolRegistry {
106110
let output_cell = &output_cell;
107111
let invocation = invocation;
108112
async move {
113+
if handler.is_mutating(&invocation) {
114+
tracing::trace!("waiting for tool gate");
115+
invocation.turn.tool_call_gate.wait_ready().await;
116+
tracing::trace!("tool gate released");
117+
}
109118
match handler.handle(invocation).await {
110119
Ok(output) => {
111120
let preview = output.log_preview();

0 commit comments

Comments
 (0)