Normalize shell and patch tool cards
This commit is contained in:
1 parent
4dc3e3d784
commit
b507656abd
11 files changed
+327
-16
No files matched your search
@@ -15,7 +15,7 @@ use std::sync::{Arc, Mutex};
|
||||
|
||||
use serde_json::{Value, json};
|
||||
|
||||
use super::super::driver::{Event, QuestionOption, SessionStatus, context_tokens};
|
||||
use super::super::driver::{Event, QuestionOption, SessionStatus, context_tokens, patch_start};
|
||||
use super::super::subagent::Subagents;
|
||||
|
||||
/// Whether this line is the CLI opening a fresh model call.
|
||||
@@ -125,6 +125,10 @@ pub(super) struct Translator {
|
||||
/// translator existed, which is every one of them after a backend
|
||||
/// restart adopts a running session.
|
||||
open_tasks: HashSet<String>,
|
||||
/// File-edit calls whose successful boilerplate result should not be drawn below their diff.
|
||||
/// Each leaves here with its `tool_result`; a failure keeps its text because that is the part a
|
||||
/// reader needs to act on.
|
||||
patches: HashSet<String>,
|
||||
/// Whether a turn is open, judged from this translator's own output: the
|
||||
/// events that [`super::proves_a_turn`] accepts open one, and the status
|
||||
/// that ends a turn closes it.
|
||||
@@ -151,6 +155,7 @@ impl Translator {
|
||||
rate_limited: false,
|
||||
tasks: HashMap::new(),
|
||||
open_tasks: HashSet::new(),
|
||||
patches: HashSet::new(),
|
||||
in_turn: false,
|
||||
}
|
||||
}
|
||||
@@ -763,7 +768,12 @@ impl Translator {
|
||||
if tool == "Task" || tool == "Agent" {
|
||||
self.start_subagent_from_task(&id, &input);
|
||||
}
|
||||
Event::ToolStart { id, tool, input }
|
||||
if tool == "Edit" {
|
||||
self.patches.insert(id.clone());
|
||||
patch_start(id, replacement_diff(&input))
|
||||
} else {
|
||||
Event::ToolStart { id, tool, input }
|
||||
}
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
@@ -936,7 +946,7 @@ impl Translator {
|
||||
/// into the session dir and referenced by an Image event. Replayed and
|
||||
/// synthetic user text is skipped -- the manager already recorded the
|
||||
/// user's side.
|
||||
fn translate_user(&self, message: &Value) -> Vec<Event> {
|
||||
fn translate_user(&mut self, message: &Value) -> Vec<Event> {
|
||||
// Only tool results are here. The CLI never echoes a person's own
|
||||
// message back on stdout -- measured, because the obvious way to learn
|
||||
// that a queued message had been taken was to watch for it coming back
|
||||
@@ -979,6 +989,10 @@ impl Translator {
|
||||
.and_then(Value::as_str)
|
||||
.unwrap_or_default()
|
||||
.to_string();
|
||||
let patch = self.patches.remove(&about);
|
||||
if patch && block.get("is_error").and_then(Value::as_bool) != Some(true) {
|
||||
texts.clear();
|
||||
}
|
||||
for image in images {
|
||||
events.push(Event::Image {
|
||||
image,
|
||||
@@ -999,6 +1013,38 @@ impl Translator {
|
||||
}
|
||||
}
|
||||
|
||||
fn replacement_diff(input: &Value) -> String {
|
||||
let path = input
|
||||
.get("file_path")
|
||||
.and_then(Value::as_str)
|
||||
.unwrap_or("file");
|
||||
let old = input
|
||||
.get("old_string")
|
||||
.and_then(Value::as_str)
|
||||
.unwrap_or_default();
|
||||
let new = input
|
||||
.get("new_string")
|
||||
.and_then(Value::as_str)
|
||||
.unwrap_or_default();
|
||||
format!(
|
||||
"--- {path}\n+++ {path}\n@@\n{}{}",
|
||||
prefixed_lines('-', old),
|
||||
prefixed_lines('+', new)
|
||||
)
|
||||
}
|
||||
|
||||
fn prefixed_lines(prefix: char, text: &str) -> String {
|
||||
text.split_inclusive('\n')
|
||||
.map(|line| {
|
||||
if line.ends_with('\n') {
|
||||
format!("{prefix}{line}")
|
||||
} else {
|
||||
format!("{prefix}{line}\n")
|
||||
}
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// The title to start a subagent under when its own first line arrives
|
||||
/// before (or without) its Task call ever being seen: the tool name of that
|
||||
/// first line, which is the only thing known about it yet. `"subagent"` for
|
||||
@@ -1351,6 +1397,44 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_edit_becomes_a_patch_and_drops_only_its_success_boilerplate() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let mut translator = Translator::new(dir.path().to_path_buf(), test_subagents(&dir));
|
||||
let events = translate_lines(
|
||||
&mut translator,
|
||||
&[
|
||||
r#"{"type":"assistant","message":{"role":"assistant","content":[{"type":"tool_use","id":"edit-1","name":"Edit","input":{"file_path":"src/main.rs","old_string":"old","new_string":"new"}}]},"parent_tool_use_id":null}"#,
|
||||
r#"{"type":"user","message":{"role":"user","content":[{"tool_use_id":"edit-1","type":"tool_result","content":"The file src/main.rs has been updated successfully.","is_error":false}]},"parent_tool_use_id":null}"#,
|
||||
],
|
||||
);
|
||||
assert_eq!(
|
||||
events,
|
||||
vec![
|
||||
patch_start(
|
||||
"edit-1".to_string(),
|
||||
"--- src/main.rs\n+++ src/main.rs\n@@\n-old\n+new\n".to_string()
|
||||
),
|
||||
Event::ToolEnd {
|
||||
id: "edit-1".to_string(),
|
||||
output: String::new()
|
||||
}
|
||||
]
|
||||
);
|
||||
|
||||
let failed = translate_lines(
|
||||
&mut translator,
|
||||
&[
|
||||
r#"{"type":"assistant","message":{"role":"assistant","content":[{"type":"tool_use","id":"edit-2","name":"Edit","input":{"file_path":"src/main.rs","old_string":"missing","new_string":"new"}}]},"parent_tool_use_id":null}"#,
|
||||
r#"{"type":"user","message":{"role":"user","content":[{"tool_use_id":"edit-2","type":"tool_result","content":"old_string was not found","is_error":true}]},"parent_tool_use_id":null}"#,
|
||||
],
|
||||
);
|
||||
assert!(matches!(
|
||||
&failed[1],
|
||||
Event::ToolEnd { output, .. } if output == "old_string was not found"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn subagent_events_are_not_duplicated_into_the_transcript() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
@@ -6,7 +6,7 @@
|
||||
|
||||
use serde_json::{Value, json};
|
||||
|
||||
use super::super::driver::{Event, SessionStatus};
|
||||
use super::super::driver::{Event, SessionStatus, patch_start};
|
||||
|
||||
#[derive(Default)]
|
||||
pub(super) struct Translator {
|
||||
@@ -150,6 +150,15 @@ impl Translator {
|
||||
}
|
||||
|
||||
fn start_item(item: &Value) -> Vec<Event> {
|
||||
if matches!(
|
||||
item.get("type").and_then(Value::as_str),
|
||||
Some("file_change" | "fileChange")
|
||||
) {
|
||||
let Some(id) = item.get("id").and_then(Value::as_str) else {
|
||||
return Vec::new();
|
||||
};
|
||||
return vec![patch_start(id.to_string(), file_change_diff(item))];
|
||||
}
|
||||
let Some((id, tool, input)) = tool(item) else {
|
||||
return Vec::new();
|
||||
};
|
||||
@@ -189,6 +198,16 @@ fn complete_item(item: &Value, include_agent_message: bool) -> Vec<Event> {
|
||||
})
|
||||
.unwrap_or_default(),
|
||||
Some("reasoning" | "userMessage") => Vec::new(),
|
||||
Some("file_change" | "fileChange") => item
|
||||
.get("id")
|
||||
.and_then(Value::as_str)
|
||||
.map(|id| {
|
||||
vec![Event::ToolEnd {
|
||||
id: id.to_string(),
|
||||
output: tool_output(item),
|
||||
}]
|
||||
})
|
||||
.unwrap_or_default(),
|
||||
_ => {
|
||||
let Some((id, _, _)) = tool(item) else {
|
||||
return Vec::new();
|
||||
@@ -203,14 +222,8 @@ fn tool(item: &Value) -> Option<(String, String, Value)> {
|
||||
let id = item.get("id")?.as_str()?.to_string();
|
||||
let kind = item.get("type")?.as_str()?;
|
||||
let (name, input) = match kind {
|
||||
"command_execution" | "commandExecution" => (
|
||||
"exec_command".to_string(),
|
||||
json!({"command": item.get("command").cloned().unwrap_or(Value::Null)}),
|
||||
),
|
||||
"file_change" | "fileChange" => (
|
||||
"apply_patch".to_string(),
|
||||
item.get("changes").cloned().unwrap_or(Value::Null),
|
||||
),
|
||||
"command_execution" | "commandExecution" => command_tool(item),
|
||||
"file_change" | "fileChange" => return None,
|
||||
"mcp_tool_call" | "mcpToolCall" => (
|
||||
item.get("tool")
|
||||
.or_else(|| item.get("name"))
|
||||
@@ -247,8 +260,85 @@ fn tool(item: &Value) -> Option<(String, String, Value)> {
|
||||
Some((id, name, input))
|
||||
}
|
||||
|
||||
/// Codex records the executor's argv, while Claude reports the script handed to its Bash tool.
|
||||
/// Collapse Codex's standard wrapper to the same common shape so the transcript describes the
|
||||
/// command a person wrote, not the implementation used to start it. An unfamiliar executable is
|
||||
/// left intact: hiding that would make a deliberately selected shell look like Bash.
|
||||
fn command_tool(item: &Value) -> (String, Value) {
|
||||
let command = item.get("command").cloned().unwrap_or(Value::Null);
|
||||
if let Some(script) = command.as_str() {
|
||||
return ("Bash".to_string(), json!({"command": script}));
|
||||
}
|
||||
if let Some(argv) = command.as_array()
|
||||
&& let [program, option, script] = argv.as_slice()
|
||||
&& program
|
||||
.as_str()
|
||||
.and_then(|program| program.rsplit('/').next())
|
||||
== Some("bash")
|
||||
&& matches!(option.as_str(), Some("-c" | "-lc"))
|
||||
&& let Some(script) = script.as_str()
|
||||
{
|
||||
return ("Bash".to_string(), json!({"command": script}));
|
||||
}
|
||||
let command = command
|
||||
.as_array()
|
||||
.map(|argv| argv.iter().map(shell_word).collect::<Vec<_>>().join(" "))
|
||||
.unwrap_or_else(|| command.to_string());
|
||||
("Shell".to_string(), json!({"command": command}))
|
||||
}
|
||||
|
||||
fn shell_word(word: &Value) -> String {
|
||||
let Some(word) = word.as_str() else {
|
||||
return word.to_string();
|
||||
};
|
||||
if !word.is_empty()
|
||||
&& word
|
||||
.chars()
|
||||
.all(|character| character.is_ascii_alphanumeric() || "/_-.=:,@+".contains(character))
|
||||
{
|
||||
return word.to_string();
|
||||
}
|
||||
format!("'{}'", word.replace('\'', "'\\''"))
|
||||
}
|
||||
|
||||
fn file_change_diff(item: &Value) -> String {
|
||||
let Some(changes) = item.get("changes").and_then(Value::as_object) else {
|
||||
return String::new();
|
||||
};
|
||||
changes
|
||||
.iter()
|
||||
.map(|(path, change)| {
|
||||
let kind = change.get("type").and_then(Value::as_str);
|
||||
let from = if kind == Some("add") {
|
||||
"/dev/null"
|
||||
} else {
|
||||
path
|
||||
};
|
||||
let to = if kind == Some("delete") {
|
||||
"/dev/null"
|
||||
} else {
|
||||
change
|
||||
.get("move_path")
|
||||
.and_then(Value::as_str)
|
||||
.unwrap_or(path)
|
||||
};
|
||||
let body = change
|
||||
.get("unified_diff")
|
||||
.and_then(Value::as_str)
|
||||
.unwrap_or_default();
|
||||
format!("--- {from}\n+++ {to}\n{body}")
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n")
|
||||
}
|
||||
|
||||
fn tool_output(item: &Value) -> String {
|
||||
match item.get("type").and_then(Value::as_str) {
|
||||
Some("file_change" | "fileChange")
|
||||
if item.get("status").and_then(Value::as_str) == Some("completed") =>
|
||||
{
|
||||
return String::new();
|
||||
}
|
||||
Some("dynamicToolCall") => {
|
||||
return content_text(item.get("contentItems"), "inputText");
|
||||
}
|
||||
@@ -376,9 +466,16 @@ mod tests {
|
||||
fn translates_tools_and_limits_without_matching_whole_records() {
|
||||
let mut translator = Translator::default();
|
||||
let started = translator.translate(&line(
|
||||
r#"{"type":"item.started","item":{"id":"item_1","type":"command_execution","command":"pwd","status":"in_progress"}}"#,
|
||||
r#"{"type":"item.started","item":{"id":"item_1","type":"command_execution","command":["/usr/bin/bash","-lc","pwd"],"status":"in_progress"}}"#,
|
||||
));
|
||||
assert!(matches!(&started[0], Event::ToolStart { tool, .. } if tool == "exec_command"));
|
||||
assert_eq!(
|
||||
started,
|
||||
vec![Event::ToolStart {
|
||||
id: "item_1".to_string(),
|
||||
tool: "Bash".to_string(),
|
||||
input: json!({"command": "pwd"})
|
||||
}]
|
||||
);
|
||||
let ended = translator.translate(&line(
|
||||
r#"{"type":"item.completed","item":{"id":"item_1","type":"command_execution","command":"pwd","aggregated_output":"/tmp\n","exit_code":0,"status":"completed"}}"#,
|
||||
));
|
||||
@@ -401,6 +498,57 @@ mod tests {
|
||||
assert!(translator.limited());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn command_translation_only_hides_the_known_bash_wrapper() {
|
||||
let legacy = tool(&line(
|
||||
r#"{"id":"old","type":"command_execution","command":"pwd"}"#,
|
||||
));
|
||||
assert_eq!(
|
||||
legacy,
|
||||
Some((
|
||||
"old".to_string(),
|
||||
"Bash".to_string(),
|
||||
json!({"command": "pwd"})
|
||||
))
|
||||
);
|
||||
|
||||
let fish = tool(&line(
|
||||
r#"{"id":"fish","type":"commandExecution","command":["/usr/bin/fish","-c","pwd"]}"#,
|
||||
));
|
||||
assert_eq!(
|
||||
fish,
|
||||
Some((
|
||||
"fish".to_string(),
|
||||
"Shell".to_string(),
|
||||
json!({"command": "/usr/bin/fish -c pwd"})
|
||||
))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_file_change_becomes_the_common_patch_shape() {
|
||||
let mut translator = Translator::default();
|
||||
let started = translator.translate(&line(
|
||||
r#"{"method":"item/started","params":{"item":{"id":"patch-1","type":"fileChange","changes":{"src/main.rs":{"type":"update","unified_diff":"@@ -1 +1 @@\n-old\n+new\n","move_path":null}},"status":"inProgress"}}}"#,
|
||||
));
|
||||
assert_eq!(
|
||||
started,
|
||||
vec![patch_start(
|
||||
"patch-1".to_string(),
|
||||
"--- src/main.rs\n+++ src/main.rs\n@@ -1 +1 @@\n-old\n+new\n".to_string()
|
||||
)]
|
||||
);
|
||||
assert_eq!(
|
||||
translator.translate(&line(
|
||||
r#"{"method":"item/completed","params":{"item":{"id":"patch-1","type":"fileChange","changes":{},"status":"completed","stdout":"Success"}}}"#
|
||||
)),
|
||||
vec![Event::ToolEnd {
|
||||
id: "patch-1".to_string(),
|
||||
output: String::new()
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn translates_native_app_server_streaming_without_repeating_the_final_item() {
|
||||
let mut translator = Translator::default();
|
||||
|
||||
@@ -389,6 +389,15 @@ pub enum Event {
|
||||
},
|
||||
}
|
||||
|
||||
/// The common presentation of a file change, whichever driver produced it.
|
||||
pub(super) fn patch_start(id: String, diff: String) -> Event {
|
||||
Event::ToolStart {
|
||||
id,
|
||||
tool: "Patch".to_string(),
|
||||
input: serde_json::json!({"diff": diff}),
|
||||
}
|
||||
}
|
||||
|
||||
/// How much the model was holding, from the three figures a turn reports:
|
||||
/// the input side only, prompt plus both cache figures. A cached token is
|
||||
/// cheaper but it is still one the model was given; output is what the turn
|
||||
|
||||
@@ -10,6 +10,8 @@
|
||||
//! - `/tool [input]` -- a full tool run, start through end.
|
||||
//! - `/bash [command]` -- a Bash call carrying that command, for what the
|
||||
//! phone's shell highlighting does to a particular line.
|
||||
//! - `/patch` -- one common patch call, for the diff presentation shared by
|
||||
//! real Codex and Claude sessions.
|
||||
//! - `/tools [n] [gap]` -- n calls back to back. `gap` is seconds between one
|
||||
//! call and the next, which is what makes a run *grow* while somebody is
|
||||
//! looking at it -- the only way to reach the state where a call opened on
|
||||
@@ -73,7 +75,7 @@ use std::sync::{Arc, Mutex};
|
||||
use std::time::Duration;
|
||||
|
||||
use super::driver::{
|
||||
AttachmentRef, Driver, Event, EventSink, QuestionOption, SessionStatus, Unqueued,
|
||||
AttachmentRef, Driver, Event, EventSink, QuestionOption, SessionStatus, Unqueued, patch_start,
|
||||
};
|
||||
use super::subagent::Subagents;
|
||||
|
||||
@@ -621,6 +623,7 @@ impl EchoDriver {
|
||||
let run_bash = text
|
||||
.strip_prefix("/bash")
|
||||
.map(|rest| rest.trim().to_string());
|
||||
let run_patch = text == "/patch";
|
||||
// Seconds to stay running before answering, default 30. Clamped rather
|
||||
// than trusted: a session pinned running for an hour by a typo is a
|
||||
// worse outcome than a short wait.
|
||||
@@ -790,6 +793,20 @@ impl EchoDriver {
|
||||
});
|
||||
}
|
||||
|
||||
if run_patch {
|
||||
let id = format!("p-{}", super::random_hex());
|
||||
send(patch_start(
|
||||
id.clone(),
|
||||
"--- src/example.rs\n+++ src/example.rs\n@@ -1,3 +1,3 @@\n fn answer() -> u8 {\n- 41\n+ 42\n }\n"
|
||||
.to_string(),
|
||||
));
|
||||
tokio::time::sleep(DELTA_DELAY).await;
|
||||
send(Event::ToolEnd {
|
||||
id,
|
||||
output: String::new(),
|
||||
});
|
||||
}
|
||||
|
||||
if let Some(input) = run_tool {
|
||||
let id = format!("t-{}", super::random_hex());
|
||||
send(Event::ToolStart {
|
||||
|
||||
Reference in new issue
Block a user