@sigitcodereview / sigit / commits / 3a96ea9

Give every ACP tool-call card something to expand

Zed only showed the disclosure arrow on a handful of tool calls (the model download/switch spinners) because those were the only places sigit set `content`; everything else carried only rawInput/rawOutput, fields ACP gives clients no display guidance for. This attaches a fenced content block to every tool call: the pretty-printed arguments while it runs, then the tool's output once it finishes (with a "(no output)" placeholder for a silent command like `git add`), fenced wide enough that output containing its own backticks can't break out. It also gives cards a readable `<tool> · <arg>` title instead of the bare tool name, and sets `locations` for the path-bearing tools so Zed can follow along. Session replay gets the same treatment so a reopened thread looks like a live one. Fixes #78 Co-Authored-By: siGit Code <noreply@sigit.si>

Seto Elkahfi committed Sep 9, 2026 at 09:33 UTC 3a96ea9aff16580b921f5d0d67cbc86cf5f44f63

2 files changed +152 -7

src/main.rs

+137 -6

@@ -81,7 +81,8 @@ use agent_client_protocol::schema::v1::{

81 SessionConfigOptionCategory, SessionConfigSelectOption, SessionConfigValueId,

82 SessionForkCapabilities, SessionId, SessionNotification, SessionUpdate,

83 SetSessionConfigOptionRequest, SetSessionConfigOptionResponse, StopReason, ToolCall,

84 - ToolCallStatus, ToolCallUpdate, ToolCallUpdateFields, ToolKind, UnstructuredCommandInput,

84 + ToolCallContent, ToolCallLocation, ToolCallStatus, ToolCallUpdate, ToolCallUpdateFields,

85 + ToolKind, UnstructuredCommandInput,

86 };

87 use agent_client_protocol::{Agent, ByteStreams, Client, ConnectionTo, Responder};

88 use onde::inference::{ChatEngine, GgufModelConfig};

@@ -349,6 +350,66 @@ fn tool_kind_for(tool_name: &str) -> ToolKind {

350 }

351 }

352

353 +/// The longest run of consecutive backticks in `text`, so a fence built around

354 +/// it can't be broken out of by output that itself contains ```` ``` ````.

355 +fn longest_backtick_run(text: &str) -> usize {

356 + let mut max = 0;

357 + let mut current = 0;

358 + for c in text.chars() {

359 + if c == '`' {

360 + current += 1;

361 + max = max.max(current);

362 + } else {

363 + current = 0;

364 + }

365 + }

366 + max

367 +}

368 +

369 +/// Fences `text` as a Markdown code block, sizing the fence from the longest

370 +/// backtick run already in `text` so it can't break out of the block.

371 +fn fenced_code_block(language: &str, text: &str) -> String {

372 + let fence = "`".repeat((longest_backtick_run(text) + 1).max(3));

373 + format!("{fence}{language}\n{text}\n{fence}")

374 +}

375 +

376 +/// Wraps a tool's output as the display content of its ACP tool-call card

377 +/// (issue #78). Capped via [`chat::cap_output_preview`] — display only, the

378 +/// model still gets the full output — and fenced so plain-text output (or a

379 +/// stray tag, per issue #73) renders literally instead of as markdown.

380 +fn tool_output_content(output: &str) -> ToolCallContent {

381 + if output.trim().is_empty() {

382 + return "(no output)".to_string().into();

383 + }

384 + fenced_code_block("", &chat::cap_output_preview(output)).into()

385 +}

386 +

387 +/// A single-file location for the path-bearing tools, so ACP clients can

388 +/// follow the agent through a file as it works (issue #78's "follow-along").

389 +fn tool_call_locations(name: &str, arguments_json: &str) -> Vec<ToolCallLocation> {

390 + if !matches!(

391 + name,

392 + "read_file" | "create_file" | "edit_file" | "multi_edit" | "delete_file"

393 + ) {

394 + return Vec::new();

395 + }

396 + let Ok(args) = serde_json::from_str::<serde_json::Value>(arguments_json) else {

397 + return Vec::new();

398 + };

399 + let Some(path) = args.get("path").and_then(|v| v.as_str()) else {

400 + return Vec::new();

401 + };

402 + let path_buf = PathBuf::from(path);

403 + let absolute = if path_buf.is_absolute() {

404 + path_buf

405 + } else {

406 + std::env::current_dir()

407 + .unwrap_or_else(|_| PathBuf::from("."))

408 + .join(path_buf)

409 + };

410 + vec![ToolCallLocation::new(absolute)]

411 +}

412 +

413 /// The visible text of one saved history message. `content` is normally a

414 /// string, but an OpenAI-compatible endpoint may hand back the block form, and

415 /// tool-call-only assistant turns carry `null`.

@@ -431,9 +492,12 @@ fn history_replay_updates(history: &[serde_json::Value]) -> Vec<SessionUpdate> {

492 } else {

493 id.to_string()

494 };

434 - let mut tool_call = ToolCall::new(replay_id, name)

495 + let output = outputs.get(id).copied().unwrap_or_default();

496 + let mut tool_call = ToolCall::new(replay_id, chat::tool_title(name, arguments))

497 .kind(tool_kind_for(name))

498 .status(ToolCallStatus::Completed)

499 + .content(vec![tool_output_content(output)])

500 + .locations(tool_call_locations(name, arguments))

501 .raw_input(serde_json::from_str::<serde_json::Value>(arguments).ok());

502 if let Some(output) = outputs.get(id) {

503 tool_call =

@@ -1818,13 +1882,17 @@ impl SiGitAgent {

1882 log::warn!("malformed JSON in tool arguments for '{}': {e}", tc.name);

1883 serde_json::Value::String(tc.arguments.clone())

1884 });

1885 + let invocation_content =

1886 + fenced_code_block("json", &chat::pretty_tool_arguments(&tc.arguments));

1887 self.send_tool_call_update(

1888 cx,

1889 session_id.clone(),

1890 SessionUpdate::ToolCall(

1825 - ToolCall::new(tc.id.clone(), tc.name.clone())

1891 + ToolCall::new(tc.id.clone(), chat::tool_title(&tc.name, &tc.arguments))

1892 .kind(tool_kind_for(&tc.name))

1893 .status(ToolCallStatus::InProgress)

1894 + .content(vec![invocation_content.into()])

1895 + .locations(tool_call_locations(&tc.name, &tc.arguments))

1896 .raw_input(raw_input),

1897 ),

1898 )

@@ -1901,6 +1969,7 @@ impl SiGitAgent {

1969 ),

1970 });

1971 }

1972 + let cancelled_reason = "Not executed: the user cancelled the turn at the permission prompt.";

1973 self.send_tool_call_update(

1974 cx,

1975 session_id.clone(),

@@ -1908,9 +1977,11 @@ impl SiGitAgent {

1977 tc.id.clone(),

1978 ToolCallUpdateFields::new()

1979 .status(ToolCallStatus::Failed)

1980 + .content(vec![tool_output_content(

1981 + cancelled_reason,

1982 + )])

1983 .raw_output(serde_json::Value::String(

1912 - "Not executed: the user cancelled the turn at the permission prompt."

1913 - .to_string(),

1984 + cancelled_reason.to_string(),

1985 )),

1986 )),

1987 )

@@ -1933,6 +2004,7 @@ impl SiGitAgent {

2004 tc.id.clone(),

2005 ToolCallUpdateFields::new()

2006 .status(ToolCallStatus::Completed)

2007 + .content(vec![tool_output_content(&output)])

2008 .raw_output(serde_json::Value::String(output.clone())),

2009 )),

2010 )

@@ -4005,7 +4077,7 @@ mod tests {

4077 }

4078 match &updates[2] {

4079 SessionUpdate::ToolCall(call) => {

4008 - assert_eq!(call.title, "read_file");

4080 + assert_eq!(call.title, "read_file · /tmp/CHANGELOG.md");

4081 assert_eq!(call.kind, ToolKind::Read);

4082 // Nothing is still running in a saved session.

4083 assert_eq!(call.status, ToolCallStatus::Completed);

@@ -4018,6 +4090,11 @@ mod tests {

4090 call.raw_output,

4091 Some(serde_json::Value::String("# Changelog".to_string()))

4092 );

4093 + // Replay is expandable exactly like a live card: the saved

4094 + // output shows up as displayable content, not just raw_output.

4095 + assert!(format!("{:?}", call.content).contains("# Changelog"));

4096 + assert_eq!(call.locations.len(), 1);

4097 + assert_eq!(call.locations[0].path, PathBuf::from("/tmp/CHANGELOG.md"));

4098 }

4099 other => panic!("expected the tool call, got {other:?}"),

4100 }

@@ -4048,6 +4125,28 @@ mod tests {

4125 }

4126 }

4127

4128 + #[test]

4129 + fn history_replay_shows_a_placeholder_for_a_call_with_no_recorded_output() {

4130 + // A session file can carry a tool call whose result was never saved

4131 + // (e.g. an older session format). Replay must not show an empty card.

4132 + let history = vec![serde_json::json!({

4133 + "role": "assistant",

4134 + "content": null,

4135 + "tool_calls": [{

4136 + "id": "call_1", "type": "function",

4137 + "function": { "name": "run_command", "arguments": "{\"command\":\"git add -A\"}" },

4138 + }],

4139 + })];

4140 +

4141 + match history_replay_updates(&history).as_slice() {

4142 + [SessionUpdate::ToolCall(call)] => {

4143 + assert!(format!("{:?}", call.content).contains("(no output)"));

4144 + assert_eq!(call.raw_output, None);

4145 + }

4146 + other => panic!("expected one tool call, got {other:?}"),

4147 + }

4148 + }

4149 +

4150 #[test]

4151 fn history_replay_skips_messages_with_nothing_to_show() {

4152 // A tool-call-only assistant turn (`content: null`), an empty reply and

@@ -4266,4 +4365,36 @@ mod tests {

4365 .collect();

4366 assert_eq!(inference_names, ["Local", "Cloud"]);

4367 }

4368 +

4369 + #[test]

4370 + fn tool_output_content_shows_a_placeholder_for_empty_output() {

4371 + let content = tool_output_content(" \n\t ");

4372 + assert!(format!("{content:?}").contains("(no output)"));

4373 + }

4374 +

4375 + #[test]

4376 + fn tool_output_content_widens_the_fence_around_embedded_backticks() {

4377 + let output = "here is a fence:\n```\ncode\n```";

4378 + let content = tool_output_content(output);

4379 + let rendered = format!("{content:?}");

4380 + // A three-backtick fence would terminate early on the embedded ```;

4381 + // the wrapping fence must be longer than any run already present.

4382 + assert!(rendered.contains("````"));

4383 + }

4384 +

4385 + #[test]

4386 + fn tool_output_content_caps_long_output_via_cap_output_preview() {

4387 + // Comfortably past chat's internal preview cap without depending on

4388 + // its exact value, which is private to that module.

4389 + let output = "x".repeat(10_000);

4390 + let content = tool_output_content(&output);

4391 + let rendered = format!("{content:?}");

4392 + assert!(rendered.contains("truncated"));

4393 + }

4394 +

4395 + #[test]

4396 + fn longest_backtick_run_finds_the_widest_run() {

4397 + assert_eq!(longest_backtick_run("no backticks here"), 0);

4398 + assert_eq!(longest_backtick_run("one ` two `` three ``` four"), 3);

4399 + }

4400 }

tests/acp_session_load.rs

+15 -1

@@ -280,7 +280,15 @@ fn loading_a_saved_session_replays_it_to_the_client() {

280 .iter()

281 .find(|update| update["sessionUpdate"] == "tool_call")

282 .unwrap_or_else(|| panic!("the tool call must come back: {replay:#?}"));

283 - assert_eq!(tool_call["title"], "read_file");

283 + // The full path can run past the title's truncation cap under a long

284 + // scratch dir, so check the readable prefix rather than an exact match.

285 + assert!(

286 + tool_call["title"]

287 + .as_str()

288 + .unwrap_or_default()

289 + .starts_with("read_file · "),

290 + "title should read '<tool> · <path>': {tool_call}"

291 + );

292 // Nothing is still running in a saved session, and the result is folded

293 // back into its call rather than replayed as a loose message.

294 assert_eq!(tool_call["status"], "completed");

@@ -291,6 +299,12 @@ fn loading_a_saved_session_replays_it_to_the_client() {

299 .contains("hello from the notes"),

300 "the tool result must ride along with its call: {tool_call}"

301 );

302 + // The card must be expandable on replay, not just carry raw_output —

303 + // that's the point of issue #78.

304 + assert!(

305 + format!("{:?}", tool_call["content"]).contains("hello from the notes"),

306 + "the replayed card must show its output as content: {tool_call}"

307 + );

308

309 // The seeded system context is the model's, not the user's — it must never

310 // surface as a message in the thread.