From cc528dbe0538cff0357b7644a9466ad689a1874f Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Fri, 2 Oct 2026 14:46:47 -0400 Subject: [PATCH] feat(agents): request summarized Claude thinking and label empty rows (#8051) Claude Code agents on Opus showed Thinking rows that opened to nothing. Opus returns no readable thinking unless Claude Code is asked for summaries, and Buzz never asked. Sonnet returns readable thinking either way. **Request summaries from Claude Code.** `session_new_full` now adds `_meta.claudeCode.options.extraArgs: {"thinking-display": "summarized"}` for the Claude adapter. claude-agent-acp spreads `extraArgs` into the CLI's arguments; 0.36.1, 0.60.0 and 0.66.0 all read the same shape. The value is merged into the existing `_meta`, so `systemPrompt` and `sessionTitle` are kept. `session_new_full` is the only place Buzz sends `session/new`, so agent sessions and model discovery both get it. **Only for a CLI that accepts the flag.** `--thinking-display` first shipped in Claude Code 2.1.94, and older CLIs exit on unknown flags, which would fail every session start. When the adapter is spawned, Buzz runs `$CLAUDE_CODE_EXECUTABLE --version` once with a 5-second limit, reading at most 256 bytes of output and killing its whole process group on timeout, and sends the flag only on 2.1.94 or newer. The flag is left off, and sessions start exactly as before, when: - the CLI is older than 2.1.94; - `CLAUDE_CODE_EXECUTABLE` is unset (the adapter then runs its own bundled CLI, which Buzz can't inspect) or empty; - the binary hangs, fails, or prints anything other than a clean `major.minor.patch` version; - `BUZZ_ACP_LAUNCH_PREFIX` is set, since the wrapper can change which CLI the adapter actually runs. The result is logged once at debug level. **Label rows that still have no text.** A Thinking row whose text is empty or only whitespace keeps its header and shows a muted "No readable reasoning tokens", with nothing to expand. This covers models or CLIs that still return no readable thinking. If text arrives later in the same row, it renders as an expandable section as before. `ThoughtActivity` is the only component that draws Thinking rows. --------- Signed-off-by: Will Pfleger Co-authored-by: Hayt <211b96e6a2b7f45fd4047988976c7bbbeeda0c15f3ae7b32eec20834b5a55118@buzz.block.builderlab.xyz> --- crates/buzz-acp/src/acp.rs | 249 +++++++++++++++++- crates/buzz-acp/src/acp/launch/tests.rs | 24 +- .../ThoughtActivity.jsdom-test.mjs | 72 +++++ .../ThoughtActivity.render.test.mjs | 62 +++++ .../activityRenderClasses/ThoughtActivity.tsx | 31 ++- 5 files changed, 424 insertions(+), 14 deletions(-) create mode 100644 desktop/src/features/agents/ui/activityRenderClasses/ThoughtActivity.jsdom-test.mjs create mode 100644 desktop/src/features/agents/ui/activityRenderClasses/ThoughtActivity.render.test.mjs diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 07d704e2f2..8528744300 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -221,6 +221,10 @@ pub struct AcpClient { standard_usage: StandardUsageTracker, /// Known adapter identity for prompt-response usage mapping. standard_adapter: Option, + /// Ask Claude Code for summarized thinking on `session/new`. Opus omits + /// readable thinking unless asked; set only when the CLI the adapter runs + /// is new enough to accept `--thinking-display`. + claude_thinking_summaries: bool, } /// Recursively merge `overlay` into `base`, with `overlay` winning on scalar/shape @@ -478,6 +482,9 @@ impl AcpClient { ) -> Result { use std::process::Stdio; + // A launch prefix may change the CLI or environment the adapter sees, + // so the version check below could inspect the wrong binary. + let launch_prefixed = std::env::var_os(launch::PREFIX_ENV).is_some(); let mut cmd = launch::command(command, args)?; if crate::config::normalize_agent_command_identity(command) == BUZZ_PI_ACP_NAME { if !args.iter().any(|arg| arg == "--") { @@ -589,6 +596,9 @@ impl AcpClient { }; cmd.envs(launch_env.iter().cloned()); cmd.env_remove(launch::PREFIX_ENV); + let claude_thinking_summaries = standard_adapter == Some(StandardAdapterKind::Claude) + && !launch_prefixed + && claude_cli_accepts_thinking_display(&cmd).await; let mut child = cmd.spawn().map_err(|error| { std::io::Error::new( error.kind(), @@ -623,6 +633,7 @@ impl AcpClient { goose_usage: UsageTracker::default(), standard_usage: StandardUsageTracker::default(), standard_adapter, + claude_thinking_summaries, }) } @@ -703,10 +714,11 @@ impl AcpClient { /// - `Some(SystemPromptTransport::ClaudeMeta(text))` — `_meta.systemPrompt` /// as `{"append": text}`, keeping claude-agent-acp's native preset intact. /// - /// `session_title` rides in `_meta.sessionTitle` when `Some`; `_meta` is - /// omitted entirely otherwise, since adapters may distinguish an absent - /// member from a null one. Metadata prompt transports and the title are - /// merged into a single object. + /// `session_title` rides in `_meta.sessionTitle` when `Some`, and a Claude + /// CLI that supports it gets `_meta.claudeCode.options.extraArgs` asking + /// for summarized thinking. These and the metadata prompt transports merge + /// into a single object; with none of them, `_meta` is omitted entirely, + /// since adapters may distinguish an absent member from a null one. /// /// Callers use [`extract_model_config_options`] and [`extract_model_state`] /// to pull model info from the raw result. @@ -738,6 +750,11 @@ impl AcpClient { // Merge — _meta may already carry a system prompt from an adapter extension. params["_meta"]["sessionTitle"] = serde_json::Value::String(title.to_owned()); } + if self.claude_thinking_summaries { + // Merge — claude-agent-acp spreads these into the CLI's argv. + params["_meta"]["claudeCode"]["options"]["extraArgs"]["thinking-display"] = + serde_json::Value::String("summarized".to_owned()); + } let result = self.send_request("session/new", params).await?; let session_id = result["sessionId"] .as_str() @@ -2400,6 +2417,99 @@ fn kill_process_group(_pid: u32) -> bool { false } +/// First Claude Code release with `--thinking-display`. Older CLIs exit on +/// unknown flags, so sending it to them would fail every `session/new`. +const CLAUDE_THINKING_DISPLAY_MIN_VERSION: (u64, u64, u64) = (2, 1, 94); + +/// Parse `claude --version` output such as `2.1.284 (Claude Code)`. The +/// first word must be exactly three dot-separated digit runs. +fn parse_claude_version(output: &str) -> Option<(u64, u64, u64)> { + let mut parts = output.split_whitespace().next()?.split('.'); + let mut next = || { + let part = parts.next()?; + part.bytes() + .all(|b| b.is_ascii_digit()) + .then(|| part.parse().ok())? + }; + let version = (next()?, next()?, next()?); + parts.next().is_none().then_some(version) +} + +/// Whether the CLI claude-agent-acp will run accepts `--thinking-display`. +/// +/// The adapter runs `CLAUDE_CODE_EXECUTABLE` when set and its bundled CLI +/// otherwise; the bundled one cannot be inspected from here, so an unset +/// variable, a hung or failing binary, or unparseable output all answer +/// `false` and leave the session exactly as before. +async fn claude_cli_accepts_thinking_display(cmd: &tokio::process::Command) -> bool { + const KEY: &str = "CLAUDE_CODE_EXECUTABLE"; + let executable = cmd + .as_std() + .get_envs() + .find(|(key, _)| *key == KEY) + .map(|(_, value)| value.map(std::ffi::OsStr::to_os_string)) + .unwrap_or_else(|| std::env::var_os(KEY)) + .filter(|path| !path.is_empty()); + let Some(executable) = executable else { + tracing::debug!(target: "acp::spawn", "{KEY} unset; not requesting thinking summaries"); + return false; + }; + let mut version_cmd = tokio::process::Command::new(&executable); + version_cmd.arg("--version"); + configure_version_probe(&mut version_cmd); + let version = match version_cmd.spawn() { + Ok(child) => claude_version_within(child, std::time::Duration::from_secs(5)).await, + Err(_) => None, + }; + let accepts = version.is_some_and(|v| v >= CLAUDE_THINKING_DISPLAY_MIN_VERSION); + tracing::debug!( + target: "acp::spawn", + "{executable:?} --version = {version:?}; thinking summaries requested: {accepts}" + ); + accepts +} + +/// Pipe stdout only, and put the probe in its own process group (like the +/// adapter) so a timeout also kills a launcher's children. +fn configure_version_probe(cmd: &mut tokio::process::Command) { + cmd.stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::null()) + .kill_on_drop(true); + #[cfg(unix)] + cmd.process_group(0); + configure_no_window(cmd); +} + +/// Read at most a version line from `child` and require a clean exit within +/// `limit`. On timeout the whole process group is killed. +async fn claude_version_within( + mut child: tokio::process::Child, + limit: std::time::Duration, +) -> Option<(u64, u64, u64)> { + use tokio::io::AsyncReadExt; + const MAX_VERSION_BYTES: u64 = 256; + let pid = child.id(); + let probe = async { + let mut stdout = Vec::new(); + let pipe = child.stdout.take()?; + pipe.take(MAX_VERSION_BYTES) + .read_to_end(&mut stdout) + .await + .ok()?; + child.wait().await.ok()?.success().then_some(stdout) + }; + match tokio::time::timeout(limit, probe).await { + Ok(stdout) => parse_claude_version(&String::from_utf8_lossy(&stdout?)), + Err(_) => { + if !pid.is_some_and(kill_process_group) { + let _ = child.start_kill(); + } + None + } + } +} + /// Suppress the console window that Windows otherwise allocates for every /// console-subsystem child process spawned from a GUI (non-console) parent. /// No-op on non-Windows platforms. @@ -3694,6 +3804,137 @@ mod tests { ); } + #[test] + fn claude_version_gate_admits_only_cli_with_thinking_display() { + let accepts = |out: &str| { + parse_claude_version(out).is_some_and(|v| v >= CLAUDE_THINKING_DISPLAY_MIN_VERSION) + }; + assert!(!accepts("2.1.92 (Claude Code)"), "below the minimum"); + assert!(!accepts("1.0.100 (Claude Code)"), "older major"); + assert!(accepts("2.1.94 (Claude Code)"), "exactly the minimum"); + assert!(accepts("2.1.284 (Claude Code)"), "newer"); + assert!(accepts("3.0.0"), "newer major"); + assert!(!accepts(""), "empty output"); + assert!(!accepts("error: unknown"), "unparseable output"); + assert!(!accepts("2.1.284.garbage"), "trailing junk"); + assert!(!accepts("2.1.+284"), "sign in a component"); + } + + #[tokio::test] + async fn claude_thinking_display_check_fails_closed_without_readable_cli() { + let mut cmd = tokio::process::Command::new("true"); + cmd.env("CLAUDE_CODE_EXECUTABLE", "/nonexistent/claude"); + assert!(!claude_cli_accepts_thinking_display(&cmd).await); + cmd.env("CLAUDE_CODE_EXECUTABLE", ""); + assert!(!claude_cli_accepts_thinking_display(&cmd).await); + } + + #[cfg(unix)] + #[tokio::test] + async fn claude_version_timeout_kills_launcher_children() { + let dir = std::env::temp_dir().join(format!("buzz-acp-hang-{}", uuid::Uuid::new_v4())); + std::fs::create_dir_all(&dir).expect("create dir"); + let pidfile = dir.join("child.pid"); + // A launcher that starts a child holding stdout open, then hangs. + let mut cmd = tokio::process::Command::new("sh"); + cmd.arg("-c") + .arg(format!("sleep 300 & echo $! > {}; wait", pidfile.display())); + configure_version_probe(&mut cmd); + let child = cmd.spawn().expect("spawn launcher"); + let version = claude_version_within(child, std::time::Duration::from_millis(500)).await; + assert_eq!(version, None); + let grandchild = std::fs::read_to_string(&pidfile).expect("pidfile"); + let grandchild = nix::unistd::Pid::from_raw(grandchild.trim().parse().expect("pid")); + let mut alive = true; + for _ in 0..50 { + alive = nix::sys::signal::kill(grandchild, None).is_ok(); + if !alive { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + } + std::fs::remove_dir_all(&dir).expect("remove dir"); + assert!(!alive, "the launcher's child must not outlive the timeout"); + } + + /// Spawn a fake `claude-agent-acp` whose `CLAUDE_CODE_EXECUTABLE` is a + /// fake CLI running `cli_body`, then return the `_meta` its `session/new` + /// actually received. + #[cfg(unix)] + async fn claude_session_meta_with_cli(cli_body: &str) -> serde_json::Value { + use std::os::unix::fs::PermissionsExt; + + let dir = + std::env::temp_dir().join(format!("buzz-acp-claude-cli-{}", uuid::Uuid::new_v4())); + std::fs::create_dir_all(&dir).expect("create fake dir"); + let write = |name: &str, body: &str| { + let path = dir.join(name); + std::fs::write(&path, format!("#!/bin/sh\n{body}\n")).expect("write fake"); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)) + .expect("chmod fake"); + path + }; + let cli = write("claude", cli_body); + let adapter = write( + "claude-agent-acp", + r#"read -r _init +echo '{"jsonrpc":"2.0","id":0,"result":{"protocolVersion":1,"agentCapabilities":{}}}' +read -r REQ +echo '{"jsonrpc":"2.0","id":1,"result":{"sessionId":"ses_test","_receivedRequest":'"$REQ"'}}' +sleep 1"#, + ); + let mut client = AcpClient::spawn_with_env( + adapter.to_str().expect("utf8 path"), + &[], + &[], + false, + &[( + "CLAUDE_CODE_EXECUTABLE".into(), + cli.to_str().expect("utf8 path").into(), + )], + ) + .await + .expect("spawn fake claude adapter"); + client.initialize().await.expect("initialize"); + let resp = client + .session_new_full( + "/tmp", + vec![], + Some(SystemPromptTransport::ClaudeMeta("be brief")), + Some("Fizz"), + ) + .await + .expect("session_new_full"); + client.shutdown().await; + std::fs::remove_dir_all(&dir).expect("remove fake dir"); + resp.raw["_receivedRequest"]["params"]["_meta"].clone() + } + + #[cfg(unix)] + #[tokio::test] + async fn claude_spawn_requests_thinking_summaries_only_from_supporting_cli() { + let meta = claude_session_meta_with_cli("echo '2.1.284 (Claude Code)'").await; + assert_eq!( + meta["claudeCode"]["options"]["extraArgs"], + serde_json::json!({ "thinking-display": "summarized" }) + ); + assert_eq!( + meta["systemPrompt"], + serde_json::json!({ "append": "be brief" }) + ); + assert_eq!(meta["sessionTitle"], "Fizz"); + + for (case, cli) in [ + ("old CLI", "echo '2.1.93 (Claude Code)'"), + ("failing CLI", "echo '2.1.284 (Claude Code)'; exit 1"), + ("unclean version", "echo '2.1.284.garbage'"), + ] { + let meta = claude_session_meta_with_cli(cli).await; + assert!(meta.get("claudeCode").is_none(), "{case}: {meta}"); + assert_eq!(meta["sessionTitle"], "Fizz", "{case}"); + } + } + // ── claude-agent-acp _meta.systemPrompt transport ───────────────────── include!("acp/system_prompt_tests.rs"); diff --git a/crates/buzz-acp/src/acp/launch/tests.rs b/crates/buzz-acp/src/acp/launch/tests.rs index be2a08e85a..6356542b79 100644 --- a/crates/buzz-acp/src/acp/launch/tests.rs +++ b/crates/buzz-acp/src/acp/launch/tests.rs @@ -46,13 +46,19 @@ for line in sys.stdin: result = {'protocolVersion': 1, 'argv': sys.argv[1:], 'hermes': os.environ.get('HERMES_ACP_SKIP_CONFIGURED_MCP'), 'codex': os.environ.get('CODEX_CONFIG'), - 'prefix': os.environ.get('BUZZ_ACP_LAUNCH_PREFIX')} + 'prefix': os.environ.get('BUZZ_ACP_LAUNCH_PREFIX'), + 'sessionId': 'ses_test', 'meta': request.get('params', {}).get('_meta')} print(json.dumps({'jsonrpc': '2.0', 'id': request['id'], 'result': result}), flush=True) "#, ) .unwrap(); fs::set_permissions(path, fs::Permissions::from_mode(0o700)).unwrap(); } + // A CLI new enough for `--thinking-display`, so only the prefix can + // keep summaries off. + let claude = dir.join("claude"); + fs::write(&claude, "#!/bin/sh\necho '2.1.284 (Claude Code)'\n").unwrap(); + fs::set_permissions(&claude, fs::Permissions::from_mode(0o700)).unwrap(); Self(dir) } fn prefix(&self) -> String { @@ -75,6 +81,7 @@ for line in sys.stdin: .env(CHILD, mode) .env_remove("CODEX_CONFIG") .env_remove("HERMES_ACP_SKIP_CONFIGURED_MCP") + .env("CLAUDE_CODE_EXECUTABLE", self.0.join("claude")) .current_dir(&self.0); if let Some(prefix) = prefix { cmd.env(PREFIX_ENV, prefix); @@ -216,6 +223,21 @@ async fn spawn_child() { _ => None, }; assert_eq!(client.standard_adapter, adapter); + if name == "claude-agent-acp" { + let meta = client + .session_new_full("/tmp", vec![], None, None) + .await + .unwrap() + .raw["meta"] + .clone(); + let expected = match std::env::var_os(PREFIX_ENV) { + Some(_) => Value::Null, + None => json!({"claudeCode": {"options": {"extraArgs": { + "thinking-display": "summarized" + }}}}), + }; + assert_eq!(meta, expected, "prefix leaves summaries off"); + } client.shutdown().await; } } diff --git a/desktop/src/features/agents/ui/activityRenderClasses/ThoughtActivity.jsdom-test.mjs b/desktop/src/features/agents/ui/activityRenderClasses/ThoughtActivity.jsdom-test.mjs new file mode 100644 index 0000000000..4d96b56284 --- /dev/null +++ b/desktop/src/features/agents/ui/activityRenderClasses/ThoughtActivity.jsdom-test.mjs @@ -0,0 +1,72 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { + RouterContextProvider, + createMemoryHistory, + createRootRoute, + createRouter, +} from "@tanstack/react-router"; +import React, { act } from "react"; +import { createRoot } from "react-dom/client"; + +import { ThoughtActivity } from "./ThoughtActivity.tsx"; + +function thoughtTree(router, queryClient, text) { + // Markdown reads router and query context, so mount minimal providers. + return React.createElement( + RouterContextProvider, + { router }, + React.createElement( + QueryClientProvider, + { client: queryClient }, + React.createElement(ThoughtActivity, { + agentAvatarUrl: null, + agentName: "Test Agent", + agentPubkey: "pubkey123", + item: { + agentPubkey: "pubkey123", + sessionId: "session-001", + turnId: null, + id: "thought:1", + type: "thought", + renderClass: "thought", + title: "Thinking", + text, + timestamp: "2026-10-02T14:27:48.000Z", + }, + }), + ), + ); +} + +test("ThoughtActivity: a mounted empty thought becomes expandable when text streams in", async () => { + const router = createRouter({ + routeTree: createRootRoute(), + history: createMemoryHistory({ initialEntries: ["/"] }), + }); + const queryClient = new QueryClient({ + defaultOptions: { queries: { enabled: false } }, + }); + const container = document.createElement("div"); + document.body.append(container); + const root = createRoot(container); + try { + await act(async () => root.render(thoughtTree(router, queryClient, ""))); + assert.ok(container.textContent.includes("No readable reasoning tokens")); + assert.equal(container.querySelector("details"), null); + + await act(async () => + root.render( + thoughtTree(router, queryClient, "Weighing the two options."), + ), + ); + assert.ok(container.querySelector("details"), "filled thought must expand"); + assert.ok(container.textContent.includes("Weighing the two options.")); + assert.ok(!container.textContent.includes("No readable reasoning tokens")); + } finally { + await act(async () => root.unmount()); + container.remove(); + } +}); diff --git a/desktop/src/features/agents/ui/activityRenderClasses/ThoughtActivity.render.test.mjs b/desktop/src/features/agents/ui/activityRenderClasses/ThoughtActivity.render.test.mjs new file mode 100644 index 0000000000..c9cceab198 --- /dev/null +++ b/desktop/src/features/agents/ui/activityRenderClasses/ThoughtActivity.render.test.mjs @@ -0,0 +1,62 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { + RouterContextProvider, + createMemoryHistory, + createRootRoute, + createRouter, +} from "@tanstack/react-router"; +import React from "react"; +import { renderToStaticMarkup } from "react-dom/server"; + +import { ThoughtActivity } from "./ThoughtActivity.tsx"; + +function renderThought(text) { + // Markdown reads router and query context, so mount minimal providers. + const router = createRouter({ + routeTree: createRootRoute(), + history: createMemoryHistory({ initialEntries: ["/"] }), + }); + const thought = React.createElement(ThoughtActivity, { + agentAvatarUrl: null, + agentName: "Test Agent", + agentPubkey: "pubkey123", + item: { + agentPubkey: "pubkey123", + sessionId: "session-001", + turnId: null, + id: "thought:1", + type: "thought", + renderClass: "thought", + title: "Thinking", + text, + timestamp: "2026-10-02T14:27:48.000Z", + }, + }); + return renderToStaticMarkup( + React.createElement( + RouterContextProvider, + { router }, + React.createElement( + QueryClientProvider, + { + client: new QueryClient({ + defaultOptions: { queries: { enabled: false } }, + }), + }, + thought, + ), + ), + ); +} + +for (const text of ["", " \n"]) { + test(`ThoughtActivity: textless thought (${JSON.stringify(text)}) shows a non-expandable notice`, () => { + const html = renderThought(text); + assert.ok(html.includes("Thinking")); + assert.ok(html.includes("No readable reasoning tokens")); + assert.ok(!html.includes(" + ); + const title = formatTranscriptTimestampTitle(props.item.timestamp); + + // Some models return thinking sealed (signature only, no readable text). + // Keep the row so it is clear the model did think, but nothing expands. + if (!text) { + return ( + + {label} + + No readable reasoning tokens + + + ); + } + return ( - - + + {label} - + );