diff --git a/crates/buzz-agent/src/agent.rs b/crates/buzz-agent/src/agent.rs index 054c334405..403201b77f 100644 --- a/crates/buzz-agent/src/agent.rs +++ b/crates/buzz-agent/src/agent.rs @@ -144,6 +144,12 @@ pub struct RunCtx<'a> { pub history: &'a mut Vec, pub original_task: &'a mut Option, pub handoff_count: &'a mut usize, + /// ACP v2 session identifier for this prompt turn. Used to derive + /// per-message `messageId` values that are unique within the ACP session. + /// Distinct from `session_id` (which is the ACP session); this is a + /// per-`session/prompt` random token so that IDs from one prompt invocation + /// never collide with those from another even within the same session. + pub run_id: String, /// Cache-summed input tokens reported by the provider on this session's /// most recent request (persists across `session/prompt` calls), or `None` /// before the first response and immediately after a handoff resets the @@ -460,6 +466,23 @@ impl RunCtx<'_> { self.emit_usage_update().await; } + // Stable per-kind message IDs for ACP v2 ContentChunk compliance. + // ACP v2 requires every ContentChunk to carry `messageId`; all chunks + // that belong to the same logical message must share the same ID, and + // IDs must be unique per message within the ACP session. + // + // A provider round produces at most one thought and one assistant + // message (the parsers collapse all provider output into one + // LlmResponse.reasoning string and one LlmResponse.text string). + // These are two *distinct* logical messages, so they get distinct IDs. + // + // `run_id` is a fresh random token per `session/prompt` invocation, + // so `-thought-` and `-message-` are + // unique within the ACP session even across multiple prompts. + // + // ACP v1 allows the field, so this is a backwards-safe addition. + let thought_msg_id = format!("{}-thought-{round}", self.run_id); + let message_msg_id = format!("{}-message-{round}", self.run_id); if !response.reasoning.is_empty() { wire::send( self.wire, @@ -467,6 +490,7 @@ impl RunCtx<'_> { self.session_id, json!({ "sessionUpdate": "agent_thought_chunk", + "messageId": &thought_msg_id, "content": { "type": "text", "text": &response.reasoning } }), ), @@ -481,6 +505,7 @@ impl RunCtx<'_> { self.session_id, json!({ "sessionUpdate": "agent_message_chunk", + "messageId": &message_msg_id, "content": { "type": "text", "text": &response.text } }), ), diff --git a/crates/buzz-agent/src/config.rs b/crates/buzz-agent/src/config.rs index 439e49f4e5..ecd2e88668 100644 --- a/crates/buzz-agent/src/config.rs +++ b/crates/buzz-agent/src/config.rs @@ -98,20 +98,29 @@ fn strip_catalog_prefix(model: &str) -> &str { /// Build the Anthropic thinking/effort request fields for the given model and effort level. /// -/// API shape selection (per Anthropic extended-thinking support table, -/// https://platform.claude.com/docs/en/build-with-claude/extended-thinking, July 2025): +/// API shape selection (per Anthropic thinking docs and per-model support table, +/// https://platform.claude.com/docs/en/build-with-claude/thinking and +/// https://platform.claude.com/docs/en/build-with-claude/thinking-troubleshooting#supported-models): /// -/// **Adaptive families** — `thinking: {type:"adaptive"}` + `output_config: {effort}`. -/// These models use adaptive thinking; `thinking:{type:"adaptive"}` is required to enable -/// thinking — without it requests run without thinking even when `output_config.effort` is set. -/// Doc-verified (extended-thinking table): Opus 4.8, Opus 4.7, Opus 4.6, Sonnet 5.x, Sonnet 4.6. -/// Matched by explicit version strings (no wildcard over version numbers). +/// **Adaptive families — `thinking:{type:"adaptive"}` activates effort control**: +/// +/// - Opus 4.6, Opus 4.7, Opus 4.8, Sonnet 4.6: status **Off** — thinking is OFF by default; +/// `thinking:{type:"adaptive"}` is required to enable thinking; without it no thinking occurs. +/// - Opus 5, Sonnet 5: status **On** — thinking is on by default (can be disabled); +/// we still send `thinking:{type:"adaptive"}` so `output_config.effort` is honoured. +/// - Fable 5, Mythos 5, Mythos Preview: status **Always on** — thinking cannot be disabled; +/// we still send `thinking:{type:"adaptive"}` so `output_config.effort` is honoured. +/// +/// In all three sub-buckets `output_config: {effort}` controls depth, clamped per-model. +/// Also sends `thinking: {display:"summarized"}` so thinking text is always visible in the +/// observer feed (without this, Anthropic defaults to `display:"omitted"` on newest models). /// /// **Manual-budget families** — `thinking: {type:"enabled", budget_tokens}`. /// `budget_tokens` is clamped to `min(level_budget, max_output_tokens - 1024)` to preserve /// at least 1024 answer tokens. If the result is < 1024 (i.e., `max_output_tokens <= 2047`), /// thinking is omitted entirely with a `warn!`. /// Doc-verified: claude-3* (legacy), claude-opus-4-5 (effort page: "uses manual thinking"). +/// Also sends `display:"summarized"` to ensure thinking text is returned. /// /// **Everything else** — omit both fields. This includes unknown/future `claude-*` names /// not yet in the support table. Safer to omit than to guess an unverified shape. @@ -155,17 +164,20 @@ pub fn anthropic_thinking_config( return (None, None); } ( - Some(json!({ "type": "enabled", "budget_tokens": budget })), + Some(json!({ "type": "enabled", "budget_tokens": budget, "display": "summarized" })), None, ) } else if is_adaptive_thinking_model(model) { - // Adaptive families: thinking must be explicitly enabled via type:"adaptive". - // output_config.effort controls the depth. Both fields are required together. + // Adaptive families: we always send type:"adaptive" to activate output_config.effort. + // Sub-bucket A (Off: Opus 4.6/4.7/4.8, Sonnet 4.6): this field is required to enable + // thinking at all. Sub-bucket B (On: Opus 5/Sonnet 5) and sub-bucket C (Always on: + // Fable 5/Mythos 5/Mythos Preview): thinking is already on; we send the field so + // output_config.effort is honoured, not to enable thinking. // Apply per-model effort clamping: if the requested level exceeds the model's // doc-verified maximum, clamp down to the highest supported level with a warning. let clamped = clamp_adaptive_effort(model, effort); ( - Some(json!({ "type": "adaptive" })), + Some(json!({ "type": "adaptive", "display": "summarized" })), Some(json!({ "effort": clamped.anthropic_effort_str() })), ) } else { @@ -588,14 +600,23 @@ fn is_manual_budget_model(model: &str) -> bool { model.starts_with("claude-3") || model == "claude-opus-4-5" } -/// Returns true for Claude model families that use adaptive thinking (doc-verified, July 2025). +/// Returns true for Claude model families that use adaptive thinking (doc-verified against +/// https://platform.claude.com/docs/en/build-with-claude/thinking-troubleshooting#supported-models). +/// +/// **Sub-bucket A — status Off (thinking OFF until `thinking:{type:"adaptive"}` is sent)**: +/// Opus 4.6, Opus 4.7, Opus 4.8, Sonnet 4.6. /// -/// Sources: https://platform.claude.com/docs/en/build-with-claude/extended-thinking (support table) -/// https://platform.claude.com/docs/en/build-with-claude/effort (effort page) +/// **Sub-bucket B — status On (thinking on by default; can be disabled)**: +/// Opus 5, Sonnet 5. +/// We still send `thinking:{type:"adaptive"}` so `output_config.effort` is honoured. /// -/// Adaptive thinking models (always-on or default-on): -/// Opus 4.8, Opus 4.7, Opus 4.6, Sonnet 5.x, Sonnet 4.6, -/// Fable 5 (always-on), Mythos 5 (always-on), Mythos Preview (default-on). +/// **Sub-bucket C — status Always on (thinking cannot be disabled)**: +/// Fable 5, Mythos 5, Mythos Preview. +/// We still send `thinking:{type:"adaptive"}` so `output_config.effort` is honoured. +/// +/// All three sub-buckets accept the same request shape. The distinction matters only when +/// thinking effort is NOT configured: sub-bucket B/C models still produce thinking even +/// without us sending the field; sub-bucket A models do not. /// /// Note: Opus 4.5 is NOT in this bucket — it uses manual budget (see `is_manual_budget_model`). /// No prefix wildcards over version numbers; each entry is doc-verified explicitly. @@ -612,14 +633,66 @@ fn is_adaptive_thinking_model(model: &str) -> bool { || model.starts_with("claude-sonnet-5") // Sonnet 4.6 exactly (not Sonnet 4.5 or earlier — not in the adaptive table). || model.starts_with("claude-sonnet-4-6") - // Fable 5 and Mythos 5 (always-on adaptive thinking, July 2025). + // Fable 5 and Mythos 5 (Always on — thinking cannot be disabled, July 2025). || model.starts_with("claude-fable-5") || model.starts_with("claude-mythos-5") - // Mythos Preview (default-on adaptive thinking, July 2025). + // Mythos Preview (Always on — thinking cannot be disabled, July 2025). // Note: xhigh is NOT available on Mythos Preview — clamp_adaptive_effort handles this. || model.starts_with("claude-mythos-preview") } +/// Reasoning summary mode for the OpenAI Responses API route. +/// +/// Controls the `reasoning.summary` field sent alongside `reasoning.effort` in +/// `responses_body`. The Responses API only returns populated `summary` arrays +/// when a summary mode is requested — without it, `summary: []` is returned and +/// the observer feed shows no reasoning text even though the model billed thinking +/// tokens. +/// +/// **Responses-route only.** On the Anthropic route, thinking blocks contain the +/// full reasoning text directly (no summary concept); this field is ignored there. +/// On Chat Completions and OpenRouter paths the field is also ignored. +/// +/// Set via `BUZZ_AGENT_THINKING_SUMMARY` (`auto|concise|detailed`). +/// Unset/empty → `auto` (the provider chooses the best available summary for the +/// model). Use `detailed` for maximum reasoning visibility in the observer feed. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ThinkingSummary { + /// Provider selects the best available summary format for the model. + Auto, + /// Shorter summaries — lower token overhead. + Concise, + /// Full-length summaries — maximum reasoning visibility. + Detailed, +} + +impl ThinkingSummary { + /// The string value sent in the `reasoning.summary` field. + pub fn as_str(self) -> &'static str { + match self { + ThinkingSummary::Auto => "auto", + ThinkingSummary::Concise => "concise", + ThinkingSummary::Detailed => "detailed", + } + } +} + +/// Parse `BUZZ_AGENT_THINKING_SUMMARY`. Pure (env-free) for testability. +/// +/// Unset or empty → `Auto` (the safe default that works for all Responses-capable models). +/// Invalid value → startup error. +pub fn parse_thinking_summary(raw: Option<&str>) -> Result { + match raw.map(|s| s.trim().to_ascii_lowercase()).as_deref() { + None | Some("") => Ok(ThinkingSummary::Auto), + Some("auto") => Ok(ThinkingSummary::Auto), + Some("concise") => Ok(ThinkingSummary::Concise), + Some("detailed") => Ok(ThinkingSummary::Detailed), + Some(other) => Err(format!( + "config: BUZZ_AGENT_THINKING_SUMMARY={other} not supported (use auto|concise|detailed)" + )), + } +} + /// Parse `BUZZ_AGENT_THINKING_EFFORT`. Pure (env-free) for testability. pub fn parse_thinking_effort(raw: Option<&str>) -> Result, String> { match raw.map(|s| s.trim().to_ascii_lowercase()).as_deref() { @@ -770,6 +843,12 @@ pub struct Config { /// Thinking/reasoning effort level. `None` = use provider default (no /// thinking config sent). Set via `BUZZ_AGENT_THINKING_EFFORT`. pub thinking_effort: Option, + /// Reasoning summary mode for the OpenAI Responses route. Controls the + /// `reasoning.summary` field emitted alongside `reasoning.effort`; only + /// takes effect when `thinking_effort` is also set. Default `Auto`. + /// Set via `BUZZ_AGENT_THINKING_SUMMARY`. Ignored on Anthropic, Chat + /// Completions, and OpenRouter routes. + pub thinking_summary: ThinkingSummary, /// Emit Anthropic `cache_control` breakpoints on the stable prefix /// (tools + system prompt) and the rolling conversation tail. Default on; /// disable with `BUZZ_AGENT_PROMPT_CACHING=0`. Consulted on every route that @@ -885,6 +964,9 @@ impl Config { hook_servers: parse_hook_servers_env("MCP_HOOK_SERVERS"), hints_enabled: parse_env("BUZZ_AGENT_NO_HINTS", 0u8)? == 0, thinking_effort: parse_thinking_effort(env("BUZZ_AGENT_THINKING_EFFORT").as_deref())?, + thinking_summary: parse_thinking_summary( + env("BUZZ_AGENT_THINKING_SUMMARY").as_deref(), + )?, prompt_caching: parse_env("BUZZ_AGENT_PROMPT_CACHING", 1u8)? != 0, }; cfg.validate()?; @@ -928,6 +1010,7 @@ impl Config { hook_servers: HookServers::None, hints_enabled: false, thinking_effort: None, + thinking_summary: ThinkingSummary::Auto, prompt_caching: false, } } @@ -1415,6 +1498,60 @@ mod tests { ); } + #[test] + fn parse_thinking_summary_round_trips_all_values() { + for (raw, expected) in [ + ("auto", ThinkingSummary::Auto), + ("concise", ThinkingSummary::Concise), + ("detailed", ThinkingSummary::Detailed), + ] { + assert_eq!( + parse_thinking_summary(Some(raw)).unwrap(), + expected, + "raw={raw:?}" + ); + } + } + + #[test] + fn parse_thinking_summary_unset_and_empty_yield_auto() { + assert_eq!(parse_thinking_summary(None).unwrap(), ThinkingSummary::Auto); + assert_eq!( + parse_thinking_summary(Some("")).unwrap(), + ThinkingSummary::Auto + ); + assert_eq!( + parse_thinking_summary(Some(" ")).unwrap(), + ThinkingSummary::Auto + ); + } + + #[test] + fn parse_thinking_summary_is_case_insensitive() { + assert_eq!( + parse_thinking_summary(Some("DETAILED")).unwrap(), + ThinkingSummary::Detailed + ); + assert_eq!( + parse_thinking_summary(Some(" Concise ")).unwrap(), + ThinkingSummary::Concise + ); + } + + #[test] + fn parse_thinking_summary_rejects_unknown_value() { + let err = parse_thinking_summary(Some("verbose")).unwrap_err(); + assert!(err.contains("BUZZ_AGENT_THINKING_SUMMARY=verbose"), "{err}"); + assert!(err.contains("auto|concise|detailed"), "{err}"); + } + + #[test] + fn thinking_summary_as_str_mapping() { + assert_eq!(ThinkingSummary::Auto.as_str(), "auto"); + assert_eq!(ThinkingSummary::Concise.as_str(), "concise"); + assert_eq!(ThinkingSummary::Detailed.as_str(), "detailed"); + } + #[test] fn thinking_effort_anthropic_budget_tokens_mapping() { assert_eq!(ThinkingEffort::Low.anthropic_budget_tokens(), 1_024); @@ -1713,6 +1850,56 @@ mod tests { assert_eq!(oc["effort"], "max"); } + // ---- anthropic_thinking_config: display:"summarized" in all enabled shapes ---- + + #[test] + fn anthropic_thinking_config_adaptive_emits_display_summarized() { + // Adaptive families (Opus 4.7, Sonnet 5, Fable 5, etc.) must include + // display:"summarized" so thinking text is returned, not omitted. + for model in &[ + "claude-opus-4-7", + "claude-opus-4-8", + "claude-sonnet-5-20250901", + "claude-fable-5", + "claude-mythos-5", + ] { + let (thinking, _) = anthropic_thinking_config(model, ThinkingEffort::High, 32_768); + let t = thinking + .unwrap_or_else(|| panic!("thinking must be present for adaptive model {model}")); + assert_eq!( + t["display"], "summarized", + "display:summarized must be present for adaptive model {model}: got {t}" + ); + } + } + + #[test] + fn anthropic_thinking_config_manual_budget_emits_display_summarized() { + // Manual-budget families (claude-3.x, opus-4-5) must also include + // display:"summarized" so thinking text is returned. + for model in &["claude-3-7-sonnet-20250219", "claude-opus-4-5"] { + let (thinking, _) = anthropic_thinking_config(model, ThinkingEffort::High, 65_536); + let t = thinking.unwrap_or_else(|| { + panic!("thinking must be present for manual-budget model {model}") + }); + assert_eq!( + t["display"], "summarized", + "display:summarized must be present for manual-budget model {model}: got {t}" + ); + } + } + + #[test] + fn anthropic_thinking_config_omitted_when_no_thinking_has_no_display_field() { + // Models that don't produce a thinking field at all should have no display key. + let (thinking, _) = + anthropic_thinking_config("claude-haiku-4-5", ThinkingEffort::High, 32_768); + assert!( + thinking.is_none(), + "thinking must be absent for unknown model" + ); + } + // ---- clamp_adaptive_effort — per-model clamping tests ---- #[test] @@ -2143,7 +2330,7 @@ mod tests { #[test] fn anthropic_thinking_config_mythos_preview_emits_adaptive_and_effort() { - // Mythos Preview — default-on adaptive thinking. + // Mythos Preview — Always on adaptive thinking. let (thinking, output_config) = anthropic_thinking_config("claude-mythos-preview", ThinkingEffort::Low, 32_768); let t = thinking.expect("thinking must be present for claude-mythos-preview"); diff --git a/crates/buzz-agent/src/lib.rs b/crates/buzz-agent/src/lib.rs index 940bd2a9c2..f222ce7ac2 100644 --- a/crates/buzz-agent/src/lib.rs +++ b/crates/buzz-agent/src/lib.rs @@ -714,6 +714,7 @@ async fn run_prompt(app: Arc, id: Value, params: Value, wire_tx: WireSender history: &mut history, original_task: &mut original_task, handoff_count: &mut handoff_count, + run_id, last_request_input_tokens: &mut last_request_input_tokens, last_request_history_bytes: &mut last_request_history_bytes, turn_input_tokens: &mut turn_input_tokens, @@ -830,6 +831,13 @@ async fn acquire_session( if s.busy { return Err("prompt already in flight"); } + // Generate the run id before mutating session state. On RNG failure we reject + // the prompt cleanly: the session stays idle and the caller can retry. Generating + // after `s.busy = true` with `?` would wedge the session permanently busy. + let run_id = format!( + "run_{}", + session_token().map_err(|_| "rng failure; retry prompt")? + ); s.busy = true; let (tx, rx) = watch::channel(false); s.cancel_tx = tx; @@ -839,7 +847,6 @@ async fn acquire_session( // Fresh run id + steer channel for this turn. The run id lets steer-capable // clients target *this* turn (rejecting steers aimed at a turn that already // ended); the channel carries mid-turn injections to the run loop. - let run_id = format!("run_{}", session_token().unwrap_or_else(|_| "x".into())); s.active_run_id = Some(run_id.clone()); let (steer_tx, steer_rx) = mpsc::unbounded_channel(); s.steer_tx = Some(steer_tx); diff --git a/crates/buzz-agent/src/llm.rs b/crates/buzz-agent/src/llm.rs index c7bc31312e..bdab509165 100644 --- a/crates/buzz-agent/src/llm.rs +++ b/crates/buzz-agent/src/llm.rs @@ -1085,7 +1085,10 @@ fn responses_body( "input": input, }); if let Some(e) = effort { - body["reasoning"] = json!({ "effort": e.openai_effort_str() }); + body["reasoning"] = json!({ + "effort": e.openai_effort_str(), + "summary": cfg.thinking_summary.as_str(), + }); } if !tools_json.is_empty() { body["tools"] = Value::Array(tools_json); @@ -2475,7 +2478,7 @@ fn apply_anthropic_cache_control(body: &mut serde_json::Map) { #[cfg(test)] mod tests { use super::*; - use crate::config::{Config, HookServers, OpenAiApi, Provider}; + use crate::config::{Config, HookServers, OpenAiApi, Provider, ThinkingSummary}; use crate::types::{HistoryItem, ToolCall, ToolResult, ToolResultContent}; use std::collections::VecDeque; use std::time::Duration; @@ -2513,6 +2516,7 @@ mod tests { prefer_mesh_for_auto: false, hints_enabled: true, thinking_effort: None, + thinking_summary: ThinkingSummary::Auto, prompt_caching: true, } } @@ -3930,6 +3934,75 @@ mod tests { Some(ThinkingEffort::Low), ); assert_eq!(body["reasoning"]["effort"], "low"); + // summary defaults to "auto" when effort is set. + assert_eq!(body["reasoning"]["summary"], "auto"); + } + + #[test] + fn responses_body_summary_present_iff_effort_set() { + // effort set → reasoning object present with both effort and summary. + let body_with_effort = responses_body( + &cfg_responses(), + "system", + &[HistoryItem::User("hi".into())], + &[], + "model", + Some(ThinkingEffort::Medium), + ); + assert!( + body_with_effort.get("reasoning").is_some(), + "reasoning must be present when effort is set" + ); + assert_eq!(body_with_effort["reasoning"]["effort"], "medium"); + assert_eq!(body_with_effort["reasoning"]["summary"], "auto"); + + // effort None → reasoning object entirely absent. + let body_no_effort = responses_body( + &cfg_responses(), + "system", + &[HistoryItem::User("hi".into())], + &[], + "model", + None, + ); + assert!( + body_no_effort.get("reasoning").is_none(), + "reasoning must be absent when effort is None" + ); + } + + #[test] + fn responses_body_emits_configured_summary_mode() { + let mut cfg = cfg_responses(); + cfg.thinking_summary = ThinkingSummary::Detailed; + let body = responses_body( + &cfg, + "system", + &[HistoryItem::User("hi".into())], + &[], + "model", + Some(ThinkingEffort::High), + ); + assert_eq!(body["reasoning"]["effort"], "high"); + assert_eq!( + body["reasoning"]["summary"], "detailed", + "configured summary mode must be forwarded to reasoning object" + ); + } + + #[test] + fn responses_body_concise_summary_mode() { + let mut cfg = cfg_responses(); + cfg.thinking_summary = ThinkingSummary::Concise; + let body = responses_body( + &cfg, + "system", + &[HistoryItem::User("hi".into())], + &[], + "model", + Some(ThinkingEffort::Low), + ); + assert_eq!(body["reasoning"]["summary"], "concise"); } #[test] diff --git a/crates/buzz-agent/tests/golden_transcripts.rs b/crates/buzz-agent/tests/golden_transcripts.rs index 4ac3503464..92e9e11dc9 100644 --- a/crates/buzz-agent/tests/golden_transcripts.rs +++ b/crates/buzz-agent/tests/golden_transcripts.rs @@ -808,3 +808,137 @@ async fn test_cancel_notification_no_reply() { h.shutdown().await; } + +/// ACP v2 ContentChunk compliance: both `agent_thought_chunk` and +/// `agent_message_chunk` must carry `messageId` and `content` when the +/// client negotiates protocol version 2. +/// +/// ACP v2 requires `ContentChunk.messageId` (required in v2 schema at +/// agentclientprotocol/agent-client-protocol schema/v2/schema.json @d13d1baa). +/// ACP v1 allows the field, so adding it is backwards-safe. +/// +/// Additional invariants verified here: +/// - The thought and assistant message IDs are **distinct** (two logical messages). +/// - IDs do **not** recur across two consecutive `session/prompt` calls in the same +/// ACP session (`run_id` is fresh per prompt, so no cross-turn collision). +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_acp_v2_chunks_carry_message_id() { + // OpenAI Responses API: reasoning item + text item. Both emitted chunks + // must have messageId + content on a v2 connection. + // Two responses so we can send two session/prompt calls and verify no ID reuse. + let url = spawn_fake_llm(vec![ + responses_reasoning_response("Thinking about it.", "Here is my response."), + responses_reasoning_response("Thinking again.", "Second response."), + ]) + .await; + let mut h = Harness::spawn(&[ + ("BUZZ_AGENT_PROVIDER", "openai"), + ("OPENAI_COMPAT_API_KEY", "test"), + ("OPENAI_COMPAT_MODEL", "fake-model"), + ("OPENAI_COMPAT_API", "responses"), + ("OPENAI_COMPAT_BASE_URL", &url), + ]) + .await; + + let sid = handshake(&mut h).await; // negotiates protocolVersion: 2 + + // ── First prompt ────────────────────────────────────────────────────────── + let p1 = h + .send( + "session/prompt", + json!({ + "sessionId": sid, + "prompt": [{ "type": "text", "text": "think and respond" }], + }), + ) + .await; + let updates1 = collect_updates_until_done(&mut h, p1).await; + + let thought1 = updates1 + .iter() + .find(|u| u["sessionUpdate"] == "agent_thought_chunk") + .expect("agent_thought_chunk must be emitted on prompt 1"); + let message1 = updates1 + .iter() + .find(|u| u["sessionUpdate"] == "agent_message_chunk") + .expect("agent_message_chunk must be emitted on prompt 1"); + + // ACP v2 ContentChunk compliance: messageId must be present and non-empty. + let thought_id1 = thought1["messageId"] + .as_str() + .expect("agent_thought_chunk must carry messageId (ACP v2 required field)"); + assert!( + !thought_id1.is_empty(), + "agent_thought_chunk messageId must not be empty" + ); + + let message_id1 = message1["messageId"] + .as_str() + .expect("agent_message_chunk must carry messageId (ACP v2 required field)"); + assert!( + !message_id1.is_empty(), + "agent_message_chunk messageId must not be empty" + ); + + // Thought and assistant message are two distinct logical messages — their IDs must differ. + assert_ne!( + thought_id1, message_id1, + "agent_thought_chunk and agent_message_chunk are distinct logical messages; their messageIds must differ" + ); + + // content must be present and correct. + assert_eq!( + thought1["content"]["text"], "Thinking about it.", + "thought content mismatch" + ); + assert_eq!( + message1["content"]["text"], "Here is my response.", + "message content mismatch" + ); + + // ── Second prompt (same ACP session) ───────────────────────────────────── + let p2 = h + .send( + "session/prompt", + json!({ + "sessionId": sid, + "prompt": [{ "type": "text", "text": "think again" }], + }), + ) + .await; + let updates2 = collect_updates_until_done(&mut h, p2).await; + + let thought2 = updates2 + .iter() + .find(|u| u["sessionUpdate"] == "agent_thought_chunk") + .expect("agent_thought_chunk must be emitted on prompt 2"); + let message2 = updates2 + .iter() + .find(|u| u["sessionUpdate"] == "agent_message_chunk") + .expect("agent_message_chunk must be emitted on prompt 2"); + + let thought_id2 = thought2["messageId"] + .as_str() + .expect("agent_thought_chunk must carry messageId on prompt 2"); + let message_id2 = message2["messageId"] + .as_str() + .expect("agent_message_chunk must carry messageId on prompt 2"); + + // IDs from prompt 2 must be distinct from each other. + assert_ne!( + thought_id2, message_id2, + "prompt 2: thought and message IDs must differ" + ); + + // IDs must NOT recur across prompts — ACP requires session-unique messageIds. + assert_ne!( + thought_id1, thought_id2, + "thought messageId must not recur across session/prompt calls (run_id must differ)" + ); + assert_ne!( + message_id1, message_id2, + "message messageId must not recur across session/prompt calls (run_id must differ)" + ); + + h.shutdown().await; +} diff --git a/desktop/src-tauri/src/commands/agent_config_tests.rs b/desktop/src-tauri/src/commands/agent_config_tests.rs index 5519153578..b63370b95f 100644 --- a/desktop/src-tauri/src/commands/agent_config_tests.rs +++ b/desktop/src-tauri/src/commands/agent_config_tests.rs @@ -623,6 +623,19 @@ fn baked_env_thinking_effort_is_unmasked() { assert!(!effort.masked); } +#[test] +fn baked_env_thinking_summary_is_unmasked() { + // BUZZ_AGENT_THINKING_SUMMARY is a non-secret enum — must not be masked. + let entries = baked_env_from_map(&[("BUZZ_AGENT_THINKING_SUMMARY", "detailed")]); + assert_eq!(entries.len(), 1); + let summary = entries + .iter() + .find(|e| e.key == "BUZZ_AGENT_THINKING_SUMMARY") + .unwrap(); + assert_eq!(summary.value, "detailed"); + assert!(!summary.masked); +} + #[test] fn baked_env_allowlist_is_case_insensitive() { // Known-safe keys — case-insensitive match must allow them. @@ -632,6 +645,8 @@ fn baked_env_allowlist_is_case_insensitive() { assert!(super::is_safe_to_reveal("BUZZ_AGENT_MODEL")); assert!(super::is_safe_to_reveal("buzz_agent_thinking_effort")); assert!(super::is_safe_to_reveal("BUZZ_AGENT_THINKING_EFFORT")); + assert!(super::is_safe_to_reveal("buzz_agent_thinking_summary")); + assert!(super::is_safe_to_reveal("BUZZ_AGENT_THINKING_SUMMARY")); assert!(super::is_safe_to_reveal("databricks_host")); assert!(super::is_safe_to_reveal("DATABRICKS_HOST")); assert!(super::is_safe_to_reveal("databricks_model")); diff --git a/desktop/src-tauri/src/managed_agents/env_vars.rs b/desktop/src-tauri/src/managed_agents/env_vars.rs index 9ca5fd080d..de6ec28c41 100644 --- a/desktop/src-tauri/src/managed_agents/env_vars.rs +++ b/desktop/src-tauri/src/managed_agents/env_vars.rs @@ -179,12 +179,14 @@ pub fn validate_user_env_keys(env_vars: &BTreeMap) -> Result<(), /// Allowlist (case-insensitive): /// - `BUZZ_AGENT_PROVIDER`, `BUZZ_AGENT_MODEL` — agent runtime selection /// - `BUZZ_AGENT_THINKING_EFFORT` — non-secret enum (none/minimal/low/medium/high/xhigh/max) +/// - `BUZZ_AGENT_THINKING_SUMMARY` — non-secret enum (auto/concise/detailed) /// - `DATABRICKS_HOST`, `DATABRICKS_MODEL` — Block non-secret defaults pub(crate) fn is_safe_to_reveal(key: &str) -> bool { const SAFE_KEYS: &[&str] = &[ "BUZZ_AGENT_PROVIDER", "BUZZ_AGENT_MODEL", "BUZZ_AGENT_THINKING_EFFORT", + "BUZZ_AGENT_THINKING_SUMMARY", "DATABRICKS_HOST", "DATABRICKS_MODEL", ];