From b507656abd2d2718f78440b2c22e3c6880839eb4 Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Wed, 9 Sep 2026 20:30:52 -0400 Subject: [PATCH] Normalize shell and patch tool cards --- PLAN.md | 6 + .../kotlin/com/example/aiapp/CodeFence.kt | 1 + .../kotlin/com/example/aiapp/Highlighter.kt | 26 +++ .../kotlin/com/example/aiapp/Languages.kt | 3 +- .../main/kotlin/com/example/aiapp/Theme.kt | 2 + .../kotlin/com/example/aiapp/ToolInput.kt | 2 + .../com/example/aiapp/HighlighterTest.kt | 15 ++ server/src/session/claude/translate.rs | 90 +++++++++- server/src/session/codex/translate.rs | 170 ++++++++++++++++-- server/src/session/driver.rs | 9 + server/src/session/echo.rs | 19 +- 11 files changed, 327 insertions(+), 16 deletions(-) diff --git a/PLAN.md b/PLAN.md index 8efb01c..06cf294 100644 --- a/PLAN.md +++ b/PLAN.md @@ -114,6 +114,12 @@ seq N", so there is no separate history path to drift from the live one. by drivers**, so every device renders the conversation from one stream. - `AssistantText { delta }` — streaming text, rendered as markdown. - `ToolStart / ToolUpdate / ToolEnd { tool, input, output }`. + The tool vocabulary is common too (2026-09-09), not just the envelope: + Codex's `/usr/bin/bash -lc` argv and Claude's Bash call are both + `Bash { command }`, while Codex file changes and Claude Edit calls are both + `Patch { diff }`. Patch success boilerplate is omitted and failures remain + as output. This normalization belongs in the drivers, before persistence; + the phone never decodes a provider's tool schema. - `Image { ref }` — saved under the session dir, fetched by URL. - `Question { id, prompt, options }` — anything needing a human. Claude's AskUserQuestion and permission requests (canUseTool) are the same shape; diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/CodeFence.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/CodeFence.kt index 072f4e7..369a4cc 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/CodeFence.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/CodeFence.kt @@ -160,6 +160,7 @@ private val FENCE_LANGUAGES: Map = "shell" to Language.SHELL, "zsh" to Language.SHELL, "console" to Language.SHELL, + "diff" to Language.DIFF, "python" to Language.PYTHON, "py" to Language.PYTHON, "javascript" to Language.JAVASCRIPT, diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/Highlighter.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/Highlighter.kt index b3fcefd..46e4fa4 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/Highlighter.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/Highlighter.kt @@ -7,6 +7,8 @@ import androidx.compose.ui.text.buildAnnotatedString /** What a span of code is, in the terms the palette has a colour for. */ enum class Kind { + ADDITION, + DELETION, KEYWORD, STRING, LITERAL, @@ -24,6 +26,8 @@ data class Span(val start: Int, val end: Int, val kind: Kind) * one instance and lives with the rest of the palette. */ data class SyntaxPalette( + val addition: Color, + val deletion: Color, val keyword: Color, val string: Color, val literal: Color, @@ -34,6 +38,8 @@ data class SyntaxPalette( ) { fun of(kind: Kind): Color = when (kind) { + Kind.ADDITION -> addition + Kind.DELETION -> deletion Kind.KEYWORD -> keyword Kind.STRING -> string Kind.LITERAL -> literal @@ -44,6 +50,26 @@ data class SyntaxPalette( } } +/** A unified diff is line-oriented: colour the changed lines and leave context untouched. */ +fun scanDiff(code: String): List { + val spans = ArrayList() + var start = 0 + while (start < code.length) { + val end = code.indexOf('\n', start).let { if (it == -1) code.length else it } + val kind = + when { + code.startsWith("+++", start) || code.startsWith("---", start) -> Kind.METADATA + code.startsWith("+", start) -> Kind.ADDITION + code.startsWith("-", start) -> Kind.DELETION + code.startsWith("@@", start) -> Kind.METADATA + else -> null + } + if (kind != null) spans.add(Span(start, end, kind)) + start = if (end == code.length) end else end + 1 + } + return spans +} + /** * [code] with its keywords, strings and comments coloured, or plain if there is no language for it. * diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/Languages.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/Languages.kt index a1d54b1..f63c380 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/Languages.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/Languages.kt @@ -16,6 +16,7 @@ enum class Language { CPP, CSHARP, DART, + DIFF, FISH, GO, JAVA, @@ -100,7 +101,7 @@ fun spansOf(code: String, language: Language): List = SCANNERS.getValue(la // Lazy for the same reason [RULES] is, since it reads it. private val SCANNERS: Map List> by lazy { RULES.mapValues { (_, rules) -> { code: String -> scan(code, rules) } } + - mapOf(Language.MARKDOWN to ::scanMarkdown) + mapOf(Language.DIFF to ::scanDiff, Language.MARKDOWN to ::scanMarkdown) } private val C_STYLE = BlockComment("/*", "*/", nests = false) diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/Theme.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/Theme.kt index 214751f..d1c256e 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/Theme.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/Theme.kt @@ -213,6 +213,8 @@ val rawSurface: Color */ fun catppuccinSyntax(): SyntaxPalette = SyntaxPalette( + addition = Mocha.Green, + deletion = Mocha.Red, keyword = Mocha.Mauve, string = Mocha.Green, literal = Mocha.Peach, diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/ToolInput.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/ToolInput.kt index 029b139..b68a495 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/ToolInput.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/ToolInput.kt @@ -51,6 +51,8 @@ data class ToolInput( private val SUBJECTS: Map> = mapOf( "Bash" to ("command" to Language.SHELL), + "Shell" to ("command" to Language.SHELL), + "Patch" to ("diff" to Language.DIFF), "Read" to ("file_path" to null), "Write" to ("file_path" to null), "Edit" to ("file_path" to null), diff --git a/app/androidApp/src/test/kotlin/com/example/aiapp/HighlighterTest.kt b/app/androidApp/src/test/kotlin/com/example/aiapp/HighlighterTest.kt index ab4f529..8fe121c 100644 --- a/app/androidApp/src/test/kotlin/com/example/aiapp/HighlighterTest.kt +++ b/app/androidApp/src/test/kotlin/com/example/aiapp/HighlighterTest.kt @@ -338,6 +338,21 @@ class HighlighterTest { assertEquals("+[-]", highlight("+[-]", fenceLanguage("brainfuck")).text) } + @Test + fun `a diff colours changes and identifies its framing separately`() { + val code = "--- a/file\n+++ b/file\n@@ -1 +1 @@\n-old\n context\n+new" + assertSpans(code, Language.DIFF, Kind.DELETION, "-old") + assertSpans(code, Language.DIFF, Kind.ADDITION, "+new") + assertSpans( + code, + Language.DIFF, + Kind.METADATA, + "--- a/file", + "+++ b/file", + "@@ -1 +1 @@", + ) + } + @Test fun `every language the fence table knows has a scanner`() { Language.entries.forEach { spansOf("x", it) } diff --git a/server/src/session/claude/translate.rs b/server/src/session/claude/translate.rs index 2210b00..bbae2a4 100644 --- a/server/src/session/claude/translate.rs +++ b/server/src/session/claude/translate.rs @@ -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, + /// 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, /// 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 { + fn translate_user(&mut self, message: &Value) -> Vec { // 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"); diff --git a/server/src/session/codex/translate.rs b/server/src/session/codex/translate.rs index 604ab92..3095e2f 100644 --- a/server/src/session/codex/translate.rs +++ b/server/src/session/codex/translate.rs @@ -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 { + 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 { }) .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::>().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::>() + .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(); diff --git a/server/src/session/driver.rs b/server/src/session/driver.rs index e6ecf2f..774c5c3 100644 --- a/server/src/session/driver.rs +++ b/server/src/session/driver.rs @@ -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 diff --git a/server/src/session/echo.rs b/server/src/session/echo.rs index 96bb54b..c216211 100644 --- a/server/src/session/echo.rs +++ b/server/src/session/echo.rs @@ -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 {