diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 78db7ff718..49e4d6cce0 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -558,6 +558,9 @@ impl AcpClient { /// `cwd` must be an absolute path. `mcp_servers` may be empty. /// `system_prompt` is included in the request when `Some` — agents that /// support the field will use it; others ignore unknown fields per JSON-RPC. + /// `session_title` rides in `_meta.sessionTitle` when `Some`; `_meta` is + /// omitted entirely otherwise, 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. pub async fn session_new_full( @@ -565,6 +568,7 @@ impl AcpClient { cwd: &str, mcp_servers: Vec, system_prompt: Option<&str>, + session_title: Option<&str>, ) -> Result { let mut params = serde_json::json!({ "cwd": cwd, @@ -573,6 +577,9 @@ impl AcpClient { if let Some(sp) = system_prompt { params["systemPrompt"] = serde_json::Value::String(sp.to_owned()); } + if let Some(title) = session_title { + params["_meta"] = serde_json::json!({ "sessionTitle": title }); + } let result = self.send_request("session/new", params).await?; let session_id = result["sessionId"] .as_str() @@ -594,9 +601,10 @@ impl AcpClient { cwd: &str, mcp_servers: Vec, system_prompt: Option<&str>, + session_title: Option<&str>, ) -> Result { Ok(self - .session_new_full(cwd, mcp_servers, system_prompt) + .session_new_full(cwd, mcp_servers, system_prompt, session_title) .await? .session_id) } @@ -2954,7 +2962,7 @@ mod tests { .expect("initialize should succeed"); let resp = client - .session_new_full("/tmp", vec![], Some("Custom system prompt")) + .session_new_full("/tmp", vec![], Some("Custom system prompt"), None) .await .expect("session_new_full should succeed"); @@ -3039,7 +3047,7 @@ mod tests { .expect("initialize should succeed"); let resp = client - .session_new_full("/tmp", vec![], None) + .session_new_full("/tmp", vec![], None, None) .await .expect("session_new_full should succeed"); @@ -3051,6 +3059,61 @@ mod tests { ); } + #[tokio::test] + async fn session_new_full_sends_session_title_in_meta_when_some() { + let script = r#" + read -t 2 _init + echo '{"jsonrpc":"2.0","id":0,"result":{"protocolVersion":1,"agentCapabilities":{}}}' + read -t 2 REQ + echo '{"jsonrpc":"2.0","id":1,"result":{"sessionId":"ses_test","_receivedRequest":'"$REQ"'}}' + sleep 1 + "#; + let mut client = spawn_script(script).await; + client + .initialize() + .await + .expect("initialize should succeed"); + + let resp = client + .session_new_full("/tmp", vec![], None, Some("Fizz · #buzz-dev")) + .await + .expect("session_new_full should succeed"); + + let received = &resp.raw["_receivedRequest"]; + assert_eq!( + received["params"]["_meta"]["sessionTitle"].as_str(), + Some("Fizz · #buzz-dev"), + "title should ride in _meta.sessionTitle, out of band from the prompt" + ); + } + + #[tokio::test] + async fn session_new_full_omits_meta_when_session_title_none() { + let script = r#" + read -t 2 _init + echo '{"jsonrpc":"2.0","id":0,"result":{"protocolVersion":1,"agentCapabilities":{}}}' + read -t 2 REQ + echo '{"jsonrpc":"2.0","id":1,"result":{"sessionId":"ses_test","_receivedRequest":'"$REQ"'}}' + sleep 1 + "#; + let mut client = spawn_script(script).await; + client + .initialize() + .await + .expect("initialize should succeed"); + + let resp = client + .session_new_full("/tmp", vec![], None, None) + .await + .expect("session_new_full should succeed"); + + let received = &resp.raw["_receivedRequest"]; + assert!( + received["params"].get("_meta").is_none(), + "_meta should be absent entirely, not an empty object or null" + ); + } + // ── Goose-native steer scaffold (PR follow-up to #1160) ────────────── /// Helper: spawn an inert `cat` subprocess so we have a real AcpClient diff --git a/crates/buzz-acp/src/config.rs b/crates/buzz-acp/src/config.rs index a38d6faa14..9a1b74c276 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -423,6 +423,12 @@ pub struct CliArgs { #[arg(long, env = "BUZZ_ACP_MODEL")] pub model: Option, + /// Title for the agent's ACP sessions, passed out-of-band in `session/new` + /// `_meta`. Adapters that recognize it name the session after this value; + /// others ignore it. Never enters the prompt. + #[arg(long, env = "BUZZ_ACP_SESSION_TITLE")] + pub session_title: Option, + /// Permission mode for agents that support `session/set_config_option` /// with `configId: "mode"` (e.g. `claude-agent-acp`). /// @@ -522,6 +528,9 @@ pub struct Config { pub memory_enabled: bool, /// Desired LLM model ID. Applied after every `session_new_full()`. pub model: Option, + /// Sanitized session title, sent as `_meta.sessionTitle` on `session/new`. + /// `None` when unset or when the configured value sanitized to empty. + pub session_title: Option, /// Permission mode to apply after session creation. `Default` = skip. pub permission_mode: PermissionMode, /// Inbound author gate mode. @@ -554,6 +563,68 @@ pub struct Config { pub base_prompt_content: Option, } +/// Maximum length, in characters, of a session title sent to the adapter. +const SESSION_TITLE_MAX_CHARS: usize = 80; + +/// Normalize a configured session title into something safe to hand an adapter. +/// +/// Control characters are dropped, runs of whitespace collapse to a single +/// space, and the result is trimmed and capped at +/// [`SESSION_TITLE_MAX_CHARS`]. Returns `None` when nothing printable is left. +/// +/// Buzz is the only guard here: Codex's own `normalize_thread_name` merely +/// trims, so an unbounded display name would be persisted verbatim into its +/// thread store. +fn sanitize_session_title(raw: &str) -> Option { + let collapsed = raw + .split_whitespace() + .map(|word| word.chars().filter(|c| !c.is_control()).collect::()) + .filter(|word| !word.is_empty()) + .collect::>() + .join(" "); + // Truncate by chars, not bytes, so a multi-byte name can't be cut mid-UTF-8. + let title: String = collapsed + .chars() + .take(SESSION_TITLE_MAX_CHARS) + .collect::() + .trim_end() + .to_string(); + if title.is_empty() { + None + } else { + Some(title) + } +} + +/// Separator between the agent name and the channel in a composed title. +/// U+00B7 MIDDLE DOT, spaces on both sides. +const SESSION_TITLE_SEPARATOR: &str = " · "; + +/// Compose a per-session title as `Agent · #channel`. +/// +/// One agent in five channels gets five sessions; a bare agent name would show +/// five identical rows in the adapter's thread list. Only the channel part is +/// truncated to fit [`SESSION_TITLE_MAX_CHARS`], so the agent name always +/// survives. Returns the bare agent name when there is no channel, the channel +/// name is blank, or no room is left for it. +pub(crate) fn compose_session_title(agent: &str, channel_name: Option<&str>) -> String { + let Some(channel) = channel_name.and_then(sanitize_session_title) else { + return agent.to_string(); + }; + // Reserve the separator and the `#` sigil alongside the agent name. + let reserved = agent.chars().count() + SESSION_TITLE_SEPARATOR.chars().count() + 1; + let channel: String = channel + .chars() + .take(SESSION_TITLE_MAX_CHARS.saturating_sub(reserved)) + .collect::() + .trim_end() + .to_string(); + if channel.is_empty() { + return agent.to_string(); + } + format!("{agent}{SESSION_TITLE_SEPARATOR}#{channel}") +} + /// Validate and deduplicate allowlist entries: each must be exactly 64 hex chars. fn validate_allowlist(entries: &[String]) -> Result, ConfigError> { let mut validated = HashSet::new(); @@ -992,6 +1063,10 @@ impl Config { typing_enabled: !args.no_typing, memory_enabled: args.memory && !args.no_memory, model, + session_title: args + .session_title + .as_deref() + .and_then(sanitize_session_title), permission_mode: args.permission_mode, respond_to: args.respond_to, respond_to_allowlist, @@ -1361,6 +1436,7 @@ mod tests { typing_enabled: true, memory_enabled: true, model: None, + session_title: None, permission_mode: PermissionMode::BypassPermissions, respond_to: RespondTo::Anyone, respond_to_allowlist: HashSet::new(), @@ -2706,4 +2782,63 @@ channels = "ALL" assert!(MAX_TURN_DURATION_CEILING_SECS < u64::MAX - 100); } } + + #[test] + fn sanitize_session_title_collapses_whitespace_and_strips_control_chars() { + assert_eq!( + sanitize_session_title(" Fizz\t\tthe\n Bot\u{7} "), + Some("Fizz the Bot".to_string()) + ); + } + + #[test] + fn sanitize_session_title_returns_none_when_nothing_printable_remains() { + assert_eq!(sanitize_session_title(" \n\t "), None); + assert_eq!(sanitize_session_title(""), None); + assert_eq!(sanitize_session_title("\u{1}\u{2}"), None); + } + + #[test] + fn sanitize_session_title_caps_length_without_splitting_multibyte_chars() { + let raw = "\u{1f41d}".repeat(SESSION_TITLE_MAX_CHARS + 10); + let title = sanitize_session_title(&raw).expect("emoji title survives sanitizing"); + assert_eq!(title.chars().count(), SESSION_TITLE_MAX_CHARS); + assert!(title.chars().all(|c| c == '\u{1f41d}')); + } + + #[test] + fn sanitize_session_title_does_not_leave_a_trailing_space_after_the_cap() { + // The cap lands mid-word, so trimming must not leave a dangling space. + let raw = format!("{} tail", "a".repeat(SESSION_TITLE_MAX_CHARS - 1)); + let title = sanitize_session_title(&raw).expect("title survives sanitizing"); + assert_eq!(title, "a".repeat(SESSION_TITLE_MAX_CHARS - 1)); + } + + #[test] + fn compose_session_title_qualifies_the_agent_name_with_the_channel() { + assert_eq!( + compose_session_title("Fizz", Some("buzz-dev")), + "Fizz · #buzz-dev" + ); + } + + #[test] + fn compose_session_title_falls_back_to_bare_agent_name_without_a_channel() { + assert_eq!(compose_session_title("Fizz", None), "Fizz"); + assert_eq!(compose_session_title("Fizz", Some(" ")), "Fizz"); + } + + #[test] + fn compose_session_title_truncates_the_channel_and_keeps_the_agent_name() { + let channel = "c".repeat(200); + let title = compose_session_title("Fizz", Some(&channel)); + assert_eq!(title.chars().count(), SESSION_TITLE_MAX_CHARS); + assert!(title.starts_with("Fizz · #c")); + } + + #[test] + fn compose_session_title_drops_the_channel_when_the_agent_name_fills_the_cap() { + let agent = "a".repeat(SESSION_TITLE_MAX_CHARS); + assert_eq!(compose_session_title(&agent, Some("buzz-dev")), agent); + } } diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 0230ea0875..4b6db6a5b0 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -1535,6 +1535,7 @@ async fn tokio_main() -> Result<()> { turn_liveness_interval: Duration::from_secs(config.turn_liveness_secs), dedup_mode: config.dedup_mode, system_prompt: config.system_prompt.clone(), + session_title: config.session_title.clone(), team_instructions: config.team_instructions.clone(), base_prompt: if config.no_base_prompt { None @@ -4026,7 +4027,7 @@ async fn run_models(args: ModelsArgs) -> Result<()> { // so shutdown() runs on all paths (success, error, timeout). let protocol_result = tokio::time::timeout(MODELS_TIMEOUT, async { let init = client.initialize().await?; - let session = client.session_new_full(&cwd, vec![], None).await?; + let session = client.session_new_full(&cwd, vec![], None, None).await?; Ok::<_, acp::AcpError>((init, session)) }) .await; @@ -4973,6 +4974,7 @@ mod build_mcp_servers_tests { typing_enabled: true, memory_enabled: false, model: None, + session_title: None, permission_mode: config::PermissionMode::BypassPermissions, respond_to: config::RespondTo::Anyone, respond_to_allowlist: std::collections::HashSet::new(), @@ -5139,6 +5141,7 @@ mod error_outcome_emission_tests { typing_enabled: true, memory_enabled: false, model: None, + session_title: None, permission_mode: config::PermissionMode::BypassPermissions, respond_to: config::RespondTo::Anyone, respond_to_allowlist: HashSet::new(), diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index cc537f8683..0c51fe954f 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -32,7 +32,7 @@ use crate::acp::{ extract_model_config_options, extract_model_state, model_in_catalog, resolve_model_switch_method, AcpClient, AcpError, McpServer, ModelSwitchMethod, StopReason, }; -use crate::config::{DedupMode, PermissionMode}; +use crate::config::{compose_session_title, DedupMode, PermissionMode}; use crate::observer; use crate::queue::{ CancelReason, ContextMessage, ConversationContext, FlushBatch, PromptChannelInfo, @@ -490,6 +490,9 @@ pub struct PromptContext { pub turn_liveness_interval: Duration, pub dedup_mode: DedupMode, pub system_prompt: Option, + /// Sanitized title for each new ACP session, sent as `_meta.sessionTitle` + /// on `session/new`. Never part of the prompt. + pub session_title: Option, pub team_instructions: Option, pub heartbeat_prompt: Option, /// Base prompt content, or `None` if `--no-base-prompt` was passed. @@ -795,6 +798,49 @@ const CONTROL_CANCEL_GRACE: Duration = Duration::from_secs(5); /// Timeout for permission-mode requests (`session/set_config_option` with `configId: "mode"`). const PERMISSION_MODE_TIMEOUT: Duration = Duration::from_secs(5); +/// Placeholder [`fetch_channel_info`] substitutes when a channel's metadata +/// event carries no `name` tag. Not a real channel name — consumers that need +/// an identifying name must treat it as absent. +const UNKNOWN_CHANNEL_NAME: &str = "unknown"; + +/// Channel-derived inputs for a new session — `(is_dm, title_channel)` — from +/// **one** metadata resolve. +/// +/// Both new-session consumers need the same lookup: the canvas block skips DMs +/// (and fails closed when the channel type can't be determined), and the +/// session title is qualified with the channel name. Resolving once is +/// load-bearing rather than tidy: [`ChannelInfoResolver`] caches only `Some`, +/// so two calls against an unresolvable channel pay the whole +/// [`fetch_channel_info`] retry sequence twice — two `CONTEXT_FETCH_TIMEOUT` +/// attempts plus `CONTEXT_FETCH_RETRY_DELAY` each, in front of `session/new`, +/// precisely when the relay is already degraded. +/// +/// `title_channel` is `None` whenever the channel can't usefully identify the +/// session: an unresolved channel, a DM (no meaningful name), or the literal +/// `"unknown"` that [`fetch_channel_info`] substitutes for a metadata event +/// with no `name` tag. Composing that sentinel would title every unnamed +/// channel identically (`Agent · #unknown`) — reintroducing the collision the +/// suffix exists to remove, while naming a channel something it isn't. The +/// startup cache already refuses `channel_type == "unknown"` for the same +/// reason. +/// +/// Renames do not retitle live sessions, and a **channel** rename is stickier +/// than an agent rename: `invalidate_channel` drops the session but not the +/// resolver's cached entry, so a renamed channel keeps its old suffix until the +/// process restarts. An agent rename lands on the next spawn (the desktop +/// restart badge covers it — see `spawn_config_hash`). +async fn resolve_new_session_channel_context( + channel_info: &ChannelInfoResolver, + channel_id: Uuid, +) -> (bool, Option) { + let Some(info) = channel_info.resolve(channel_id).await else { + return (true, None); + }; + let is_dm = info.channel_type == "dm"; + let title_channel = (!is_dm && info.name != UNKNOWN_CHANNEL_NAME).then_some(info.name); + (is_dm, title_channel) +} + /// Create a new ACP session via `session_new_full()`, populate model capabilities /// on the agent (first session only), and apply `desired_model` if set. /// @@ -806,6 +852,7 @@ async fn create_session_and_apply_model( ctx: &PromptContext, agent_core: Option<&str>, agent_canvas: Option<&str>, + channel_name: Option<&str>, ) -> Result { // Build base_prompt + system_prompt + agent core + canvas metadata into a // single prompt. Standard protocol-v2 agents receive it in `session/new`; @@ -825,6 +872,11 @@ async fn create_session_and_apply_model( agent_canvas, ); + let session_title = ctx + .session_title + .as_deref() + .map(|agent_name| compose_session_title(agent_name, channel_name)); + let resp = agent .acp .session_new_full( @@ -835,6 +887,7 @@ async fn create_session_and_apply_model( agent.protocol_version, combined_system_prompt.as_deref(), ), + session_title.as_deref(), ) .await?; @@ -1427,18 +1480,20 @@ pub async fn run_prompt_task( // prevents a stale revision A surviving a failed create and being re-used by // the next attempt after the canvas was cleared. let mut pending_canvas: Option<(Uuid, String)> = None; + // Channel name for the session title, from the same single resolve the + // canvas DM check uses — see `resolve_new_session_channel_context`. + let mut title_channel: Option = None; if let PromptSource::Channel(cid) = &source { let is_new_channel_session = !agent.state.sessions.contains_key(cid); - if is_new_channel_session && !agent.state.canvas_sections.contains_key(cid) { - // Resolve DM status: prefer the startup cache, lazy-fetch as fallback. - // Unknown → treat as DM (fail-closed). - let is_dm = ctx - .channel_info - .resolve(*cid) - .await - .map(|ci| ci.channel_type == "dm") - .unwrap_or(true); - if !is_dm { + let needs_canvas = is_new_channel_session && !agent.state.canvas_sections.contains_key(cid); + let needs_title = is_new_channel_session && ctx.session_title.is_some(); + if needs_canvas || needs_title { + let (is_dm, resolved_channel) = + resolve_new_session_channel_context(&ctx.channel_info, *cid).await; + title_channel = resolved_channel; + // A confirmed DM never receives a canvas section; an undeterminable + // channel type fails closed as a DM for the same reason. + if needs_canvas && !is_dm { if let Some(section) = fetch_canvas_section(*cid, &ctx.rest_client).await { pending_canvas = Some((*cid, section)); } @@ -1470,12 +1525,16 @@ pub async fn run_prompt_task( if let Some(sid) = agent.state.sessions.get(cid) { (sid.clone(), false) } else { - // Create new session with model application. + // The title is channel-qualified (`Agent · #channel`) so one + // agent in several channels doesn't produce identical session + // rows; `title_channel` comes from the single resolve above and + // is `None` for DM, unresolved, and unnamed channels. match create_session_and_apply_model( &mut agent, &ctx, agent_core.as_deref(), agent_canvas.as_deref(), + title_channel.as_deref(), ) .await { @@ -1523,7 +1582,7 @@ pub async fn run_prompt_task( if let Some(sid) = &agent.state.heartbeat_session { (sid.clone(), false) } else { - match create_session_and_apply_model(&mut agent, &ctx, None, None).await { + match create_session_and_apply_model(&mut agent, &ctx, None, None, None).await { Ok(sid) => { tracing::info!( target: "pool::session", @@ -2268,7 +2327,7 @@ pub(crate) async fn fetch_channel_info( } let channel_type = crate::relay::channel_type_from_tags(tags); Some(PromptChannelInfo { - name: name.unwrap_or("unknown").to_string(), + name: name.unwrap_or(UNKNOWN_CHANNEL_NAME).to_string(), channel_type, }) } @@ -3781,6 +3840,9 @@ mod tests { #[test] fn test_framed_system_prompt_both_present_carries_both_headers() { + // Also the regression guard against #2372: the session title travels + // out of band in `_meta.sessionTitle`, so this exact-bytes assertion is + // what pins the framing against a `[Session]` section reappearing here. let framed = framed_system_prompt("/", Some("base text"), Some("persona text")) .expect("both present yields Some"); assert_eq!(framed, "[Base]\nbase text\n\n[System]\npersona text"); @@ -5280,6 +5342,7 @@ mod tests { turn_liveness_interval: Duration::ZERO, dedup_mode: DedupMode::Drop, system_prompt: None, + session_title: None, team_instructions: None, heartbeat_prompt: None, base_prompt: None, @@ -5617,4 +5680,142 @@ mod tests { "timestamp must not use +00:00 offset" ); } + + // ── new-session channel context (one resolve, two consumers) ───────────── + + /// A [`ChannelInfoResolver`] whose lazy REST fallback is served by a local + /// HTTP server, plus a counter of the requests that actually reached it. + /// Counting real requests is the point: the composition tests are pure and + /// cannot see duplicated I/O. + async fn counting_resolver( + response: serde_json::Value, + ) -> ( + ChannelInfoResolver, + std::sync::Arc, + tokio::task::JoinHandle<()>, + ) { + use std::sync::atomic::{AtomicUsize, Ordering}; + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind test HTTP server"); + let base_url = format!("http://{}", listener.local_addr().unwrap()); + let requests = std::sync::Arc::new(AtomicUsize::new(0)); + let server_requests = requests.clone(); + let body = response.to_string(); + let server = tokio::spawn(async move { + while let Ok((mut socket, _)) = listener.accept().await { + let mut buf = vec![0; 8192]; + let _ = socket.read(&mut buf).await; + server_requests.fetch_add(1, Ordering::SeqCst); + let response = format!( + "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}", + body.len(), + body + ); + let _ = socket.write_all(response.as_bytes()).await; + } + }); + let rest = crate::relay::RestClient { + http: reqwest::Client::new(), + base_url, + keys: nostr::Keys::generate(), + auth_tag_json: None, + }; + ( + ChannelInfoResolver::new(std::collections::HashMap::new(), rest), + requests, + server, + ) + } + + fn channel_metadata_response(id: Uuid, tags: &[[&str; 2]]) -> serde_json::Value { + let mut event_tags = vec![json!(["d", id.to_string()])]; + event_tags.extend(tags.iter().map(|[k, v]| json!([k, v]))); + json!([{ "tags": event_tags }]) + } + + /// A normal channel yields a non-DM (canvas allowed) and its name for the + /// title suffix — and the second consumer reads it from cache, not the wire. + #[tokio::test] + async fn test_new_session_channel_context_qualifies_a_normal_channel() { + use std::sync::atomic::Ordering; + + let id = Uuid::new_v4(); + let response = channel_metadata_response(id, &[["name", "buzz-dev"], ["t", "stream"]]); + let (resolver, requests, server) = counting_resolver(response).await; + + let (is_dm, title_channel) = resolve_new_session_channel_context(&resolver, id).await; + assert!(!is_dm, "a stream channel is not a DM"); + assert_eq!(title_channel.as_deref(), Some("buzz-dev")); + assert_eq!(requests.load(Ordering::SeqCst), 1); + + let (_, again) = resolve_new_session_channel_context(&resolver, id).await; + assert_eq!(again.as_deref(), Some("buzz-dev")); + assert_eq!( + requests.load(Ordering::SeqCst), + 1, + "a resolved channel is cached — no second lookup" + ); + server.abort(); + } + + /// A DM carries no useful name, so it gets the bare agent title (and no + /// canvas section). + #[tokio::test] + async fn test_new_session_channel_context_leaves_a_dm_unqualified() { + let id = Uuid::new_v4(); + let response = channel_metadata_response(id, &[["name", "DM"], ["t", "dm"]]); + let (resolver, _requests, server) = counting_resolver(response).await; + + let (is_dm, title_channel) = resolve_new_session_channel_context(&resolver, id).await; + assert!(is_dm); + assert_eq!( + title_channel, None, + "a DM name must never reach the session title" + ); + server.abort(); + } + + /// The `"unknown"` placeholder `fetch_channel_info` substitutes for a + /// metadata event with no `name` tag is not a channel name: qualifying with + /// it would title every unnamed channel `Agent · #unknown`. + #[tokio::test] + async fn test_new_session_channel_context_treats_the_unknown_name_as_absent() { + let id = Uuid::new_v4(); + let response = channel_metadata_response(id, &[["t", "stream"]]); + let (resolver, _requests, server) = counting_resolver(response).await; + + let (is_dm, title_channel) = resolve_new_session_channel_context(&resolver, id).await; + assert!(!is_dm, "a nameless stream channel is still not a DM"); + assert_eq!( + title_channel, None, + "the `unknown` placeholder must yield a bare title" + ); + server.abort(); + } + + /// An unresolvable channel yields the bare title, fails closed as a DM, and + /// costs exactly ONE `fetch_channel_info` sequence — two attempts, because + /// `fetch_with_retry` retries once. `resolve()` caches only `Some`, so a + /// second resolve for the title would double this in front of `session/new`, + /// exactly when the relay is already degraded. + #[tokio::test] + async fn test_new_session_channel_context_attempts_an_unresolved_channel_once() { + use std::sync::atomic::Ordering; + + let (resolver, requests, server) = counting_resolver(json!([])).await; + + let (is_dm, title_channel) = + resolve_new_session_channel_context(&resolver, Uuid::new_v4()).await; + assert!(is_dm, "an undeterminable channel type must fail closed"); + assert_eq!(title_channel, None, "unresolved channels get a bare title"); + assert_eq!( + requests.load(Ordering::SeqCst), + 2, + "one fetch_channel_info sequence (initial attempt + single retry)" + ); + server.abort(); + } } diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 8bad876429..8de72ec1a8 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -189,10 +189,11 @@ const overrides = new Map([ // ownership (valid_agent_runtime_receipt uses buzz_sweep_owns_process). ["src-tauri/src/managed_agents/runtime/tests.rs", 1320], // runtime.rs re-entered the list after the #1968 merge: main's - // definition-authoritative resolver comments grew it to 982, and this PR's - // typed harness-descriptor resolution in spawn_agent_child (+38) lands on - // top. Queued to shrink with the next runtime split pass (#2974 follow-up). - ["src-tauri/src/managed_agents/runtime.rs", 1020], + // definition-authoritative resolver comments grew it to 982, and the BYOH + // typed harness-descriptor resolution in spawn_agent_child landed on top at + // 1020. This PR's session-title env write in spawn_agent_child adds 12. + // Queued to shrink with the next runtime split pass (#2974 follow-up). + ["src-tauri/src/managed_agents/runtime.rs", 1032], // applyWorkspace reposDir parameter plus the validateReposDir binding, // threaded through Tauri invokes for configurable repos_dir, plus the // harness-persona-sync `harnessOverride` create-input bit — load-bearing diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 32b3b328e9..3658f48a95 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -21,7 +21,10 @@ pub(crate) use path::should_skip_claude_executable; pub(crate) use path::should_use_inherited; mod metadata; -pub(crate) use metadata::{resolve_effective_prompt_model_provider, runtime_metadata_env_vars}; +pub(crate) use metadata::{ + resolve_effective_prompt_model_provider, resolve_session_title, runtime_metadata_env_vars, + SESSION_TITLE_ENV_VAR, +}; mod stop; pub(crate) use stop::managed_agent_runtime_keys; @@ -766,6 +769,15 @@ pub fn spawn_agent_child( } else { command.env_remove("BUZZ_ACP_MODEL"); } + // Session title for the harness to pass out-of-band on `session/new`. The + // adapter names the session after it; it never reaches the prompt, so this + // is display metadata only. `spawn_config_hash` hashes the same resolve, so + // a rename raises the restart badge instead of leaving the process stale. + if let Some(title) = resolve_session_title(record.display_name.as_deref(), &record.name) { + command.env(SESSION_TITLE_ENV_VAR, title); + } else { + command.env_remove(SESSION_TITLE_ENV_VAR); + } build_buzz_agent_provider_defaults(&mut command); if let Some(meta) = runtime_meta { for (key, value) in runtime_metadata_env_vars( diff --git a/desktop/src-tauri/src/managed_agents/runtime/metadata.rs b/desktop/src-tauri/src/managed_agents/runtime/metadata.rs index ac9e3a0a3f..288ce06b0a 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/metadata.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/metadata.rs @@ -24,6 +24,39 @@ pub(crate) fn runtime_metadata_env_vars<'a>( vars } +/// Env var carrying the session title to the harness. Shared with +/// `spawn_hash` so the restart badge hashes the same key the spawn writes. +pub(crate) const SESSION_TITLE_ENV_VAR: &str = "BUZZ_ACP_SESSION_TITLE"; + +/// Resolve the session title for an agent: its `display_name` when it has one, +/// otherwise its unique `name` handle. `None` when both are blank, so the +/// caller clears the env var rather than exporting an empty title. +/// +/// Control characters are stripped **here**, not left to the harness: an +/// interior NUL cannot cross the environment boundary at all, so +/// `Command::env` fails the whole spawn rather than passing it through (see +/// the same guard applied to user-supplied env in `env_vars::merged_user_env`). +/// A display name that is nothing but control characters therefore falls back +/// to `name` instead of turning display chrome into a spawn failure. +/// +/// The harness still owns whitespace collapsing, the length cap, and channel +/// qualification — see `sanitize_session_title` and `compose_session_title` in +/// `buzz-acp`. +pub(crate) fn resolve_session_title(display_name: Option<&str>, name: &str) -> Option { + [display_name, Some(name)] + .into_iter() + .flatten() + .map(|value| { + value + .chars() + .filter(|c| !c.is_control()) + .collect::() + .trim() + .to_string() + }) + .find(|value| !value.is_empty()) +} + /// Resolve effective prompt/model/provider using definition-authoritative /// semantics for linked instances. /// @@ -49,3 +82,69 @@ pub(crate) fn resolve_effective_prompt_model_provider( None => (record_prompt, record_model, record_provider), } } + +#[cfg(test)] +mod tests { + use super::resolve_session_title; + + #[test] + fn resolve_session_title_prefers_display_name() { + assert_eq!( + resolve_session_title(Some("Fizz"), "fizz-1").as_deref(), + Some("Fizz") + ); + } + + #[test] + fn resolve_session_title_falls_back_to_name_when_display_name_blank() { + assert_eq!( + resolve_session_title(None, "fizz-1").as_deref(), + Some("fizz-1") + ); + assert_eq!( + resolve_session_title(Some(" "), "fizz-1").as_deref(), + Some("fizz-1") + ); + } + + #[test] + fn resolve_session_title_returns_none_when_both_are_blank() { + assert_eq!(resolve_session_title(Some(""), " "), None); + } + + #[test] + fn resolve_session_title_trims_surrounding_whitespace() { + assert_eq!( + resolve_session_title(Some(" Fizz "), "fizz-1").as_deref(), + Some("Fizz") + ); + } + + /// An interior NUL cannot cross the env boundary — `Command::env` returns + /// `Err` for the whole spawn. Stripping it here keeps a corrupted record + /// from turning display chrome into a spawn failure. + #[test] + fn resolve_session_title_strips_control_chars_that_would_fail_the_spawn() { + let title = resolve_session_title(Some("Fi\u{0}zz\u{7}"), "fizz-1") + .expect("a name with strippable controls still yields a title"); + assert_eq!(title, "Fizz"); + assert!(!title.contains('\u{0}')); + } + + /// A display name that is *only* control characters is not a title, so the + /// unique handle takes over rather than exporting an empty value. + #[test] + fn resolve_session_title_falls_back_to_name_when_display_name_is_all_control_chars() { + assert_eq!( + resolve_session_title(Some("\u{0}\u{1}"), "fizz-1").as_deref(), + Some("fizz-1") + ); + } + + /// Both candidates unusable — the caller clears the env var instead of + /// exporting a NUL-bearing or empty title. + #[test] + fn resolve_session_title_returns_none_when_both_candidates_are_control_chars_only() { + assert_eq!(resolve_session_title(Some("\u{0}"), "\u{0}"), None); + } +} diff --git a/desktop/src-tauri/src/managed_agents/spawn_hash.rs b/desktop/src-tauri/src/managed_agents/spawn_hash.rs index e2884bfa15..648cc62bbe 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash.rs @@ -31,6 +31,7 @@ use super::{ effective_config::{resolve_effective_config, EffectiveConfigResult}, known_acp_runtime, normalize_agent_args, persona_events::preview_prospective_persona_snapshot, + runtime::{resolve_session_title, SESSION_TITLE_ENV_VAR}, types::{AgentDefinition, ManagedAgentRecord, TeamRecord}, GlobalAgentConfig, }; @@ -124,6 +125,16 @@ pub(crate) fn spawn_config_hash( resolved_prompt.hash(&mut hasher); resolved_model.hash(&mut hasher); resolved_provider.hash(&mut hasher); + // Session title: the same resolve `spawn_agent_child` performs for its env + // write, so a rename raises the restart badge. Skipped when a user env + // override shadows it — spawn writes the title BEFORE the user env layer, + // so the override is what actually runs, and it already reaches this hash + // through `descriptor.env` above. Hashing the record-derived value under an + // override would badge a rename that changes nothing. + let effective_session_title = (!descriptor.env.contains_key(SESSION_TITLE_ENV_VAR)) + .then(|| resolve_session_title(record.display_name.as_deref(), &record.name)) + .flatten(); + effective_session_title.hash(&mut hasher); record.auth_tag.hash(&mut hasher); record.respond_to.as_str().hash(&mut hasher); // The allowlist is hashed as the env receives it: spawn sets diff --git a/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs b/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs index 292cdbbb79..686ad52d4f 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs @@ -543,6 +543,72 @@ fn linked_instance_stale_prompt_bytes_are_inert_at_hash_time() { ); } +#[test] +fn display_name_edit_changes_hash() { + // The spawn writes BUZZ_ACP_SESSION_TITLE from display_name-or-name, so a + // rename must trip the badge: the running process keeps the old title + // until it restarts, and the operator has to be told that. + let rec = record(); + let mut renamed = record(); + renamed.display_name = Some("Fizz".into()); + assert_ne!( + spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), + spawn_config_hash(&renamed, &[], &[], "wss://ws.example", &Default::default()), + "a display-name rename changes the spawned session title and must badge" + ); +} + +#[test] +fn name_edit_changes_hash_when_display_name_is_absent() { + // With no display_name the title falls back to the unique handle, so the + // handle is what the env write carries and what must be hashed. + let rec = record(); + let mut renamed = record(); + renamed.name = "agent-2".into(); + assert_ne!( + spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), + spawn_config_hash(&renamed, &[], &[], "wss://ws.example", &Default::default()), + "the fallback title source must reach the hash too" + ); +} + +#[test] +fn display_name_edit_does_not_change_hash_under_an_explicit_title_override() { + // User env is written AFTER the Buzz-set title (last-wins), so an explicit + // BUZZ_ACP_SESSION_TITLE is what the child actually runs with. Renaming the + // record changes nothing about the spawned process, so badging it would be + // a false restart prompt. The override itself still reaches the hash + // through the effective env. + let mut rec = record(); + rec.env_vars + .insert("BUZZ_ACP_SESSION_TITLE".into(), "Pinned Title".into()); + let mut renamed = rec.clone(); + renamed.display_name = Some("Fizz".into()); + assert_eq!( + spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), + spawn_config_hash(&renamed, &[], &[], "wss://ws.example", &Default::default()), + "a rename shadowed by an explicit title override must not badge" + ); +} + +#[test] +fn title_override_edit_changes_hash() { + // Counterpart to the test above: the override is not inert — editing it + // changes what the child runs with and must badge. + let mut rec = record(); + rec.env_vars + .insert("BUZZ_ACP_SESSION_TITLE".into(), "Pinned Title".into()); + let mut edited = record(); + edited + .env_vars + .insert("BUZZ_ACP_SESSION_TITLE".into(), "Other Title".into()); + assert_ne!( + spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), + spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()), + "editing an explicit title override must badge" + ); +} + #[test] fn linked_instance_prompt_model_provider_resolve_from_one_call() { // The prompt for a linked instance must track the definition, exactly