diff --git a/crates/buzz-acp/src/config.rs b/crates/buzz-acp/src/config.rs index dab61be30a..c50016cb0d 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -111,6 +111,37 @@ impl std::fmt::Display for RespondTo { } } +/// Where the harness should place an ordinary response to an inbound event. +/// +/// `Thread` preserves the historical Buzz behavior. `TopLevel` keeps +/// human-facing responses at the channel root. `FollowScope` keeps top-level +/// channel events at the channel root while preserving the root of an +/// existing thread. Agent-to-agent turns remain unforced so coordination can +/// still nest intentionally. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, clap::ValueEnum)] +pub enum ReplyPlacement { + /// Historical behavior: human-facing top-level events open a thread. + #[default] + #[value(name = "thread")] + Thread, + /// Always keep human-facing responses at the channel root. + #[value(name = "top-level")] + TopLevel, + /// Match the scope of the human-facing inbound event. + #[value(name = "follow-scope")] + FollowScope, +} + +impl std::fmt::Display for ReplyPlacement { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(match self { + Self::Thread => "thread", + Self::TopLevel => "top-level", + Self::FollowScope => "follow-scope", + }) + } +} + /// Permission mode for agents that support `session/set_config_option` with /// `configId: "mode"` (e.g. `claude-agent-acp`). /// @@ -453,6 +484,15 @@ pub struct CliArgs { )] pub respond_to: RespondTo, + /// Reply placement policy for human-facing turns. + #[arg( + long, + env = "BUZZ_ACP_REPLY_PLACEMENT", + default_value = "thread", + value_enum + )] + pub reply_placement: ReplyPlacement, + /// Comma-separated 64-char hex pubkeys for allowlist mode. /// Owner pubkey is always implicitly included. #[arg(long, env = "BUZZ_ACP_RESPOND_TO_ALLOWLIST", value_delimiter = ',')] @@ -535,6 +575,8 @@ pub struct Config { pub permission_mode: PermissionMode, /// Inbound author gate mode. pub respond_to: RespondTo, + /// Reply placement policy for human-facing turns. + pub reply_placement: ReplyPlacement, /// Validated allowlist of pubkey hex strings (used when respond_to == Allowlist). pub respond_to_allowlist: HashSet, /// Allowed `respond_to` modes. Empty = all modes allowed. @@ -1093,6 +1135,7 @@ impl Config { .and_then(sanitize_session_title), permission_mode: args.permission_mode, respond_to: args.respond_to, + reply_placement: args.reply_placement, respond_to_allowlist, allowed_respond_to, persona_env_vars, @@ -1123,7 +1166,7 @@ impl Config { format!(" allowed_respond_to=[{}]", modes.join(",")) }; format!( - "relay={} pubkey={} agent_cmd={} {} mcp_cmd={} idle_timeout={}s max_turn={}s agents={} heartbeat={}s subscribe={:?} dedup={:?} meh={:?} ignore_self={} context_limit={} max_turns_per_session={} presence={} typing={} memory={} model={} permission_mode={} {}{}", + "relay={} pubkey={} agent_cmd={} {} mcp_cmd={} idle_timeout={}s max_turn={}s agents={} heartbeat={}s subscribe={:?} dedup={:?} meh={:?} ignore_self={} context_limit={} max_turns_per_session={} presence={} typing={} memory={} model={} permission_mode={} reply_placement={} {}{}", self.relay_url, self.keys.public_key().to_hex(), self.agent_command, @@ -1144,6 +1187,7 @@ impl Config { self.memory_enabled, self.model.as_deref().unwrap_or("(agent default)"), self.permission_mode, + self.reply_placement, respond_to_detail, allowed_respond_to_detail, ) @@ -1463,6 +1507,7 @@ mod tests { session_title: None, permission_mode: PermissionMode::BypassPermissions, respond_to: RespondTo::Anyone, + reply_placement: ReplyPlacement::Thread, respond_to_allowlist: HashSet::new(), allowed_respond_to: Vec::new(), persona_env_vars: vec![], @@ -2432,6 +2477,30 @@ channels = "ALL" ); } + #[test] + fn test_reply_placement_defaults_to_thread() { + assert_eq!(ReplyPlacement::default(), ReplyPlacement::Thread); + } + + #[test] + fn test_reply_placement_display_and_value_enum_parsing() { + assert_eq!(ReplyPlacement::Thread.to_string(), "thread"); + assert_eq!(ReplyPlacement::TopLevel.to_string(), "top-level"); + assert_eq!(ReplyPlacement::FollowScope.to_string(), "follow-scope"); + assert_eq!( + ReplyPlacement::from_str("follow-scope", true).unwrap(), + ReplyPlacement::FollowScope + ); + assert!(ReplyPlacement::from_str("invalid", true).is_err()); + } + + #[test] + fn test_summary_includes_reply_placement() { + let mut config = test_config(SubscribeMode::Mentions); + config.reply_placement = ReplyPlacement::FollowScope; + assert!(config.summary().contains("reply_placement=follow-scope")); + } + #[test] fn test_summary_includes_respond_to() { let config = test_config(SubscribeMode::Mentions); diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 403512a322..f17c3bd08d 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -1559,6 +1559,7 @@ async fn tokio_main() -> Result<()> { memory_enabled: config.memory_enabled, harness_name: crate::config::normalize_agent_command_identity(&config.agent_command), relay_url: config.relay_url.clone(), + reply_placement: config.reply_placement, }); if !config.memory_enabled { @@ -5026,6 +5027,7 @@ mod build_mcp_servers_tests { session_title: None, permission_mode: config::PermissionMode::BypassPermissions, respond_to: config::RespondTo::Anyone, + reply_placement: config::ReplyPlacement::Thread, respond_to_allowlist: std::collections::HashSet::new(), allowed_respond_to: vec![], persona_env_vars: vec![], @@ -5247,6 +5249,7 @@ mod error_outcome_emission_tests { session_title: None, permission_mode: config::PermissionMode::BypassPermissions, respond_to: config::RespondTo::Anyone, + reply_placement: config::ReplyPlacement::Thread, respond_to_allowlist: HashSet::new(), allowed_respond_to: vec![], persona_env_vars: vec![], diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index 348bc138e4..6d7c8f2995 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -551,6 +551,8 @@ pub struct PromptContext { /// the desktop keys per (agent, relay) pair, e.g. `session_config_captured`, /// mirroring the `managed_agent_runtime_lifecycle` frames. pub relay_url: String, + /// Reply placement policy for human-facing channel turns. + pub reply_placement: crate::config::ReplyPlacement, } impl AgentPool { @@ -1861,6 +1863,7 @@ pub async fn run_prompt_task( system_prompt: ctx.system_prompt.as_deref(), team_instructions: ctx.team_instructions.as_deref(), agent_canvas: agent_canvas.as_deref(), + reply_placement: ctx.reply_placement, }, ) } else { @@ -6413,6 +6416,7 @@ mod tests { memory_enabled: false, harness_name: "goose".to_string(), relay_url: "ws://127.0.0.1:3000".to_string(), + reply_placement: crate::config::ReplyPlacement::Thread, } } diff --git a/crates/buzz-acp/src/queue.rs b/crates/buzz-acp/src/queue.rs index 029bf86dbf..d6c0ce5b07 100644 --- a/crates/buzz-acp/src/queue.rs +++ b/crates/buzz-acp/src/queue.rs @@ -18,7 +18,7 @@ use std::collections::{HashMap, HashSet, VecDeque}; use std::time::{Duration, Instant}; use uuid::Uuid; -use crate::config::DedupMode; +use crate::config::{DedupMode, ReplyPlacement}; /// Maximum events queued per channel before oldest events are dropped. const MAX_PENDING_PER_CHANNEL: usize = 500; @@ -1156,19 +1156,18 @@ fn append_reply_instruction(s: &mut String, event_id: &str) { )); } -/// Append a new-thread reply instruction for a human-facing top-level mention. +/// Append a channel-root reply instruction for a human-facing turn. /// -/// The triggering mention has no thread tags, so the agent's reply becomes the -/// thread root. Anchoring to the triggering event (rather than leaving the -/// choice open) prevents replying into a stale/unrelated prior thread. -fn append_new_thread_reply_instruction(s: &mut String, event_id: &str) { - s.push_str(&format!( - "\nIMPORTANT: This is a new top-level message. For ordinary replies in \ - this turn, use `--reply-to {event_id}` on `buzz messages send` — the \ - triggering message is the thread root. Do NOT reply into any other \ - (older) thread. If the human explicitly asks for a channel-root, \ - top-level, or broadcast post, send that message without `--reply-to`." - )); +/// The caller has already established that this is a human-facing turn. The +/// instruction is intentionally explicit so a stale supplied thread root +/// cannot win over the selected placement policy. +fn append_channel_root_reply_instruction(s: &mut String) { + s.push_str( + "\nIMPORTANT: Keep this human-facing response at the channel root. Send \ + it with `buzz messages send` without `--reply-to`. Do NOT reply into \ + the triggering event or any older thread. If the human explicitly \ + asks for a threaded reply, follow that explicit request.", + ); } /// Decide whether a turn is human-facing for reply-anchor purposes. @@ -1200,9 +1199,11 @@ fn turn_is_human_facing( /// Resolve the `--reply-to` anchor for a non-DM turn. /// -/// Returns `Some(id)` only for human-facing turns (see [`turn_is_human_facing`]): -/// - in a thread → the thread ROOT, keeping the reply flat at layer 1 -/// - top-level → the triggering event id, which becomes the new thread root +/// Returns an anchor only for human-facing turns (see +/// [`turn_is_human_facing`]) and only when the selected policy requires one: +/// - `thread`: thread root, or the triggering event for a top-level turn +/// - `top-level`: no anchor +/// - `follow-scope`: thread root only /// /// Returns `None` for agent↔agent turns, leaving the agent free to nest deeply /// (intentional for agent coordination). @@ -1211,25 +1212,46 @@ fn resolve_reply_anchor( thread_tags: &ThreadTags, triggering_event_id: &str, profile_lookup: Option<&PromptProfileLookup>, + reply_placement: ReplyPlacement, ) -> Option { if !turn_is_human_facing(sender_pubkey, thread_tags, profile_lookup) { return None; } - Some( - thread_tags - .root_event_id - .clone() - .unwrap_or_else(|| triggering_event_id.to_string()), - ) + match reply_placement { + ReplyPlacement::Thread => Some( + thread_tags + .root_event_id + .clone() + .unwrap_or_else(|| triggering_event_id.to_string()), + ), + ReplyPlacement::TopLevel => None, + ReplyPlacement::FollowScope => thread_tags.root_event_id.clone(), + } +} + +/// Whether a non-DM turn needs an explicit no-anchor instruction. +fn force_channel_root( + sender_pubkey: &str, + thread_tags: &ThreadTags, + profile_lookup: Option<&PromptProfileLookup>, + reply_placement: ReplyPlacement, +) -> bool { + if !turn_is_human_facing(sender_pubkey, thread_tags, profile_lookup) { + return false; + } + match reply_placement { + ReplyPlacement::Thread => false, + ReplyPlacement::TopLevel => true, + ReplyPlacement::FollowScope => thread_tags.root_event_id.is_none(), + } } /// Format a `[Context]` hints section based on event scope. /// /// `reply_anchor` is the pre-resolved `--reply-to` target for this turn (see -/// [`resolve_reply_anchor`]). In the thread/DM branches it threads ordinary -/// replies; in the channel branch a `Some` anchor means a human-facing -/// top-level mention whose reply should open a new thread rooted at the -/// triggering event. +/// [`resolve_reply_anchor`]). A `Some` anchor threads ordinary replies; +/// `force_channel_root` adds the explicit no-anchor instruction when the +/// selected policy requires a channel-root response. fn format_context_hints( channel_id: Uuid, channel_info: Option<&PromptChannelInfo>, @@ -1237,6 +1259,7 @@ fn format_context_hints( is_dm: bool, has_conversation_context: bool, reply_anchor: Option<&str>, + force_channel_root: bool, ) -> String { let channel_display = match channel_info { Some(ci) => format!("{} (#{channel_id})", ci.name), @@ -1297,6 +1320,8 @@ fn format_context_hints( s.push_str(&format!("\n{ctx_hint}")); if let Some(event_id) = reply_anchor { append_reply_instruction(&mut s, event_id); + } else if force_channel_root { + append_channel_root_reply_instruction(&mut s); } s } else { @@ -1307,7 +1332,9 @@ fn format_context_hints( Hint: Use `buzz messages get --channel ` for recent messages if needed." ); if let Some(event_id) = reply_anchor { - append_new_thread_reply_instruction(&mut s, event_id); + append_reply_instruction(&mut s, event_id); + } else if force_channel_root { + append_channel_root_reply_instruction(&mut s); } s } @@ -1372,6 +1399,8 @@ pub struct FormatPromptArgs<'a> { /// For legacy agents it rides in the user message on every turn of the /// session, alongside `[Base]`/`[System]`/`[Agent Memory — core]`. pub agent_canvas: Option<&'a str>, + /// Reply placement policy for human-facing channel turns. + pub reply_placement: ReplyPlacement, } /// Format the `[Base]` section for the base prompt. @@ -1461,7 +1490,7 @@ pub fn format_prompt(batch: &FlushBatch, args: &FormatPromptArgs<'_>) -> Vec) -> Vec) -> Vec assert!( + top_prompt.contains(&format!("--reply-to {anchor}")), + "mode={mode}, prompt={top_prompt}" + ), + None => assert!( + !top_prompt.contains(&format!("--reply-to {top_level_id}")), + "mode={mode}, prompt={top_prompt}" + ), + } + + let thread_batch = FlushBatch { + channel_id: Uuid::new_v4(), + events: vec![BatchEvent { + event: threaded.clone(), + prompt_tag: "@mention".into(), + received_at: Instant::now(), + }], + cancelled_events: vec![], + cancel_reason: None, + }; + let thread_prompt = format_prompt( + &thread_batch, + &FormatPromptArgs { + reply_placement: mode, + ..Default::default() + }, + ) + .join("\n\n"); + match expected_thread_anchor { + Some(anchor) => assert!( + thread_prompt.contains(&format!("--reply-to {anchor}")), + "mode={mode}, prompt={thread_prompt}" + ), + None => assert!( + !thread_prompt.contains(&format!("--reply-to {root_id}")), + "mode={mode}, prompt={thread_prompt}" + ), + } + } + } + + #[test] + fn test_follow_scope_agent_only_coordination_is_not_forced_to_root() { + let root_id = "b".repeat(64); + let event = make_event_with_tags( + "agent coordination", + vec![vec!["p".into(), AGENT_B_PK.into()]], + ); + let sender_id = event.pubkey.to_hex(); + let profiles = HashMap::from([ + (sender_id, profile(true)), + (AGENT_B_PK.to_string(), profile(true)), + ]); + let top_batch = FlushBatch { + channel_id: Uuid::new_v4(), + events: vec![BatchEvent { + event, + prompt_tag: "@mention".into(), + received_at: Instant::now(), + }], + cancelled_events: vec![], + cancel_reason: None, + }; + let top_prompt = format_prompt( + &top_batch, + &FormatPromptArgs { + profile_lookup: Some(&profiles), + reply_placement: ReplyPlacement::FollowScope, + ..Default::default() + }, + ) + .join("\n\n"); + assert!(!top_prompt.contains("--reply-to"), "{top_prompt}"); + + let threaded_event = make_event_with_tags( + "agent coordination in thread", + vec![ + vec!["e".into(), root_id, "".into(), "reply".into()], + vec!["p".into(), AGENT_B_PK.into()], + ], + ); + let threaded_sender_id = threaded_event.pubkey.to_hex(); + let threaded_profiles = HashMap::from([ + (threaded_sender_id, profile(true)), + (AGENT_B_PK.to_string(), profile(true)), + ]); + let threaded_batch = FlushBatch { + channel_id: Uuid::new_v4(), + events: vec![BatchEvent { + event: threaded_event, + prompt_tag: "@mention".into(), + received_at: Instant::now(), + }], + cancelled_events: vec![], + cancel_reason: None, + }; + let threaded_prompt = format_prompt( + &threaded_batch, + &FormatPromptArgs { + profile_lookup: Some(&threaded_profiles), + reply_placement: ReplyPlacement::FollowScope, + ..Default::default() + }, + ) + .join("\n\n"); + assert!(!threaded_prompt.contains("--reply-to"), "{threaded_prompt}"); + } + #[test] fn test_sanitize_prompt_label_strips_newlines_and_control_chars() { assert_eq!( @@ -3941,6 +4167,50 @@ mod tests { ); } + #[test] + fn test_reply_placement_does_not_change_dm_thread_semantics() { + let ch = Uuid::new_v4(); + let event = make_event_with_tags( + "DM reply", + vec![vec!["e".into(), "a".repeat(64), "".into(), "reply".into()]], + ); + let event_id = event.id.to_hex(); + let batch = FlushBatch { + channel_id: ch, + events: vec![BatchEvent { + event, + prompt_tag: "dm".into(), + received_at: Instant::now(), + }], + cancelled_events: vec![], + cancel_reason: None, + }; + let channel_info = PromptChannelInfo { + name: "DM".into(), + channel_type: "dm".into(), + }; + + for mode in [ + ReplyPlacement::Thread, + ReplyPlacement::TopLevel, + ReplyPlacement::FollowScope, + ] { + let prompt = format_prompt( + &batch, + &FormatPromptArgs { + channel_info: Some(&channel_info), + reply_placement: mode, + ..Default::default() + }, + ) + .join("\n\n"); + assert!( + prompt.contains(&format!("--reply-to {event_id}")), + "DM mode={mode} should preserve the reply anchor: {prompt}" + ); + } + } + #[test] fn test_reply_instruction_present_for_top_level_human_message() { let ch = Uuid::new_v4(); @@ -3957,17 +4227,212 @@ mod tests { cancel_reason: None, }; - // Top-level human message (no lookup → human): the reply opens a new - // thread anchored to the triggering event, preventing replies into a - // stale older thread. + // Default mode preserves the historical behavior: a top-level human + // message opens a new thread anchored to the triggering event. let prompt = format_prompt(&batch, &FormatPromptArgs::default()).join("\n\n"); assert!( prompt.contains(&format!("--reply-to {event_id}")), "top-level human message should anchor a new thread at the triggering event" ); assert!( - prompt.contains("new top-level message"), - "top-level human message should use the new-thread instruction" + prompt.contains("For ordinary replies in this turn"), + "top-level human message should use the thread instruction" + ); + } + + #[test] + fn test_follow_scope_top_level_stays_at_channel_root() { + let batch = make_single_batch("hello from the channel"); + let prompt = format_prompt( + &batch, + &FormatPromptArgs { + reply_placement: ReplyPlacement::FollowScope, + ..Default::default() + }, + ) + .join("\n\n"); + let event_id = batch.events.last().unwrap().event.id.to_hex(); + assert!( + !prompt.contains(&format!("--reply-to {event_id}")), + "{prompt}" + ); + assert!(prompt.contains("without `--reply-to`"), "{prompt}"); + assert!(prompt.contains("older thread"), "{prompt}"); + assert!( + prompt.contains("explicitly asks for a threaded reply"), + "{prompt}" + ); + } + + #[test] + fn test_follow_scope_thread_uses_only_thread_root() { + let root_id = "f".repeat(64); + let parent_id = "e".repeat(64); + let event = make_event_with_tags( + "reply in the existing thread", + vec![ + vec!["e".into(), root_id.clone(), "".into(), "root".into()], + vec!["e".into(), parent_id.clone(), "".into(), "reply".into()], + ], + ); + let batch = FlushBatch { + channel_id: Uuid::new_v4(), + events: vec![BatchEvent { + event, + prompt_tag: "@mention".into(), + received_at: Instant::now(), + }], + cancelled_events: vec![], + cancel_reason: None, + }; + let prompt = format_prompt( + &batch, + &FormatPromptArgs { + reply_placement: ReplyPlacement::FollowScope, + ..Default::default() + }, + ) + .join("\n\n"); + assert!( + prompt.contains(&format!("--reply-to {root_id}")), + "{prompt}" + ); + assert!( + !prompt.contains(&format!("--reply-to {parent_id}")), + "{prompt}" + ); + } + + #[test] + fn test_follow_scope_uses_latest_batched_event_scope() { + let root_id = "c".repeat(64); + let top_level = make_event("older top-level event"); + let top_level_id = top_level.id.to_hex(); + let threaded = make_event_with_tags( + "newer threaded event", + vec![vec!["e".into(), root_id.clone(), "".into(), "reply".into()]], + ); + let batch = FlushBatch { + channel_id: Uuid::new_v4(), + events: vec![ + BatchEvent { + event: top_level, + prompt_tag: "@mention".into(), + received_at: Instant::now(), + }, + BatchEvent { + event: threaded, + prompt_tag: "@mention".into(), + received_at: Instant::now(), + }, + ], + cancelled_events: vec![], + cancel_reason: None, + }; + let prompt = format_prompt( + &batch, + &FormatPromptArgs { + reply_placement: ReplyPlacement::FollowScope, + ..Default::default() + }, + ) + .join("\n\n"); + assert!( + prompt.contains(&format!("--reply-to {root_id}")), + "{prompt}" + ); + assert!( + !prompt.contains(&format!("--reply-to {top_level_id}")), + "{prompt}" + ); + } + + #[test] + fn test_follow_scope_latest_batched_top_level_does_not_reuse_stale_root() { + let root_id = "d".repeat(64); + let threaded = make_event_with_tags( + "older threaded event", + vec![vec!["e".into(), root_id.clone(), "".into(), "reply".into()]], + ); + let top_level = make_event("newer top-level event"); + let batch = FlushBatch { + channel_id: Uuid::new_v4(), + events: vec![ + BatchEvent { + event: threaded, + prompt_tag: "@mention".into(), + received_at: Instant::now(), + }, + BatchEvent { + event: top_level, + prompt_tag: "@mention".into(), + received_at: Instant::now(), + }, + ], + cancelled_events: vec![], + cancel_reason: None, + }; + let prompt = format_prompt( + &batch, + &FormatPromptArgs { + reply_placement: ReplyPlacement::FollowScope, + ..Default::default() + }, + ) + .join("\n\n"); + assert!( + !prompt.contains(&format!("--reply-to {root_id}")), + "{prompt}" + ); + assert!(prompt.contains("older thread"), "{prompt}"); + let top_level_id = batch.events.last().unwrap().event.id.to_hex(); + assert!( + !prompt.contains(&format!("--reply-to {top_level_id}")), + "{prompt}" + ); + assert!( + prompt.contains(&root_id), + "thread context should remain visible" + ); + } + + #[test] + fn test_top_level_mode_flattens_threaded_human_reply() { + let root_id = "a".repeat(64); + let event = make_event_with_tags( + "flatten this response", + vec![vec!["e".into(), root_id.clone(), "".into(), "reply".into()]], + ); + let batch = FlushBatch { + channel_id: Uuid::new_v4(), + events: vec![BatchEvent { + event, + prompt_tag: "@mention".into(), + received_at: Instant::now(), + }], + cancelled_events: vec![], + cancel_reason: None, + }; + let prompt = format_prompt( + &batch, + &FormatPromptArgs { + reply_placement: ReplyPlacement::TopLevel, + ..Default::default() + }, + ) + .join("\n\n"); + assert!( + !prompt.contains(&format!("--reply-to {root_id}")), + "{prompt}" + ); + assert!(prompt.contains("channel root"), "{prompt}"); + assert!( + prompt.contains(&root_id), + "thread context should remain visible" + ); + assert!( + prompt.contains("explicitly asks for a threaded reply"), + "{prompt}" ); } @@ -4143,16 +4608,17 @@ mod tests { cancel_reason: None, }; - // Last event is top-level and human-facing → opens a new thread - // anchored to that top-level event (NOT the earlier thread's root). + // Last event is top-level and human-facing → preserves the historical + // thread mode, anchored to that top-level event (NOT the earlier + // thread's root). let prompt = format_prompt(&batch, &FormatPromptArgs::default()).join("\n\n"); assert!( prompt.contains(&format!("--reply-to {plain_id}")), "batched top-level-last prompt should anchor to the last (top-level) event" ); assert!( - prompt.contains("new top-level message"), - "batched top-level-last prompt should use the new-thread instruction" + prompt.contains("For ordinary replies in this turn"), + "batched top-level-last prompt should use the thread instruction" ); } diff --git a/crates/buzz-acp/src/setup_mode.rs b/crates/buzz-acp/src/setup_mode.rs index b1a9372ea4..0908b8f0e7 100644 --- a/crates/buzz-acp/src/setup_mode.rs +++ b/crates/buzz-acp/src/setup_mode.rs @@ -72,7 +72,7 @@ pub(crate) enum AcpAvailabilityStatus { use crate::{ author_allowed, - config::Config, + config::{Config, ReplyPlacement}, event_mentions_agent, filter, relay::{HarnessRelay, RelayEventPublisher}, }; @@ -467,6 +467,7 @@ pub(crate) async fn run_setup_listener(config: Config, payload: SetupPayload) -> buzz_event.channel_id, &buzz_event.event, &payload, + config.reply_placement, ) .await { @@ -590,35 +591,18 @@ async fn handle_setup_membership( /// Build and publish a setup nudge reply to the triggering event. /// -/// Threading: flat reply to the thread root if one exists; otherwise reply -/// to the triggering event itself. P-tags the asker. +/// Threading follows the configured reply-placement policy. P-tags the asker. async fn publish_setup_nudge( publisher: &RelayEventPublisher, keys: &nostr::Keys, channel_id: Uuid, triggering_event: &nostr::Event, payload: &SetupPayload, + reply_placement: ReplyPlacement, ) -> Result<()> { - use buzz_sdk::ThreadRef; - // Parse NIP-10 thread tags to determine reply target. let thread_tags = crate::queue::parse_thread_tags(triggering_event); - - let thread_ref = if let Some(root_str) = &thread_tags.root_event_id { - // Threaded event: reply flat to the root. - let root_id = nostr::EventId::from_hex(root_str) - .map_err(|e| anyhow::anyhow!("invalid root event id: {e}"))?; - Some(ThreadRef { - root_event_id: root_id, - parent_event_id: root_id, - }) - } else { - // Top-level event: reply to the triggering event. - Some(ThreadRef { - root_event_id: triggering_event.id, - parent_event_id: triggering_event.id, - }) - }; + let thread_ref = resolve_setup_thread_ref(reply_placement, triggering_event.id, &thread_tags)?; let body = payload.nudge_body(); let author_hex = triggering_event.pubkey.to_hex(); @@ -645,6 +629,35 @@ async fn publish_setup_nudge( Ok(()) } +/// Resolve the thread reference for a setup nudge without publishing it. +/// +/// This mirrors the normal prompt routing policy: `thread` preserves the +/// historical behavior, `top-level` removes the anchor, and `follow-scope` +/// anchors only when the inbound event is already threaded. +fn resolve_setup_thread_ref( + reply_placement: ReplyPlacement, + triggering_event_id: nostr::EventId, + thread_tags: &crate::queue::ThreadTags, +) -> Result> { + let root_id = thread_tags + .root_event_id + .as_deref() + .map(nostr::EventId::from_hex) + .transpose() + .map_err(|e| anyhow::anyhow!("invalid root event id: {e}"))?; + + let event_id = match reply_placement { + ReplyPlacement::Thread => root_id.or(Some(triggering_event_id)), + ReplyPlacement::TopLevel => None, + ReplyPlacement::FollowScope => root_id, + }; + + Ok(event_id.map(|id| buzz_sdk::ThreadRef { + root_event_id: id, + parent_event_id: id, + })) +} + // ── Tests ───────────────────────────────────────────────────────────────────── #[cfg(test)] @@ -986,6 +999,44 @@ mod tests { ); } + #[test] + fn setup_nudge_reply_placement_matrix_matches_inbound_scope() { + let triggering_event_id = EventId::from_byte_array([0x11; 32]); + let root_hex = "a".repeat(64); + let threaded_tags = crate::queue::ThreadTags { + root_event_id: Some(root_hex.clone()), + parent_event_id: Some("b".repeat(64)), + mentioned_pubkeys: vec![], + }; + let top_level_tags = crate::queue::ThreadTags { + root_event_id: None, + parent_event_id: None, + mentioned_pubkeys: vec![], + }; + let root_id = EventId::from_hex(&root_hex).unwrap(); + + let cases = [ + ( + ReplyPlacement::Thread, + Some(triggering_event_id), + Some(root_id), + ), + (ReplyPlacement::TopLevel, None, None), + (ReplyPlacement::FollowScope, None, Some(root_id)), + ]; + for (mode, expected_top_level, expected_thread) in cases { + let top_level = resolve_setup_thread_ref(mode, triggering_event_id, &top_level_tags) + .unwrap() + .map(|thread| thread.root_event_id); + assert_eq!(top_level, expected_top_level, "mode={mode:?}"); + + let threaded = resolve_setup_thread_ref(mode, triggering_event_id, &threaded_tags) + .unwrap() + .map(|thread| thread.root_event_id); + assert_eq!(threaded, expected_thread, "mode={mode:?}"); + } + } + // ── should_nudge_for_event gate tests ───────────────────────────────────── // // These tests exercise the loop-wiring for the two safety-critical guards: diff --git a/desktop/src-tauri/src/commands/agent_config.rs b/desktop/src-tauri/src/commands/agent_config.rs index 5a26f0f645..a88d6a8959 100644 --- a/desktop/src-tauri/src/commands/agent_config.rs +++ b/desktop/src-tauri/src/commands/agent_config.rs @@ -383,7 +383,7 @@ pub async fn get_agent_config_surface( ), )?; let session_cache = state.get_session_cache(&runtime_key); - let global = crate::managed_agents::load_global_agent_config(&app).unwrap_or_default(); + let global = crate::managed_agents::load_global_agent_config(&app)?; Ok(resolve_config_surface( record, @@ -678,6 +678,7 @@ mod tests { last_error_code: None, respond_to: RespondTo::OwnerOnly, respond_to_allowlist: vec![], + reply_placement: None, display_name: None, slug: None, runtime: None, @@ -691,13 +692,13 @@ mod tests { definition_respond_to: None, definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, + definition_reply_placement: None, relay_mesh: None, agent_command_override: None, persona_source_version: None, provider: None, } } - fn persona_with_model(model: &str) -> AgentDefinition { AgentDefinition { id: "persona-1".to_string(), @@ -718,11 +719,11 @@ mod tests { respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, created_at: "".to_string(), updated_at: "".to_string(), } } - /// A post-spawn session cache whose live model is `current_model` and whose /// `model_overridden` flag records whether a `SwitchModel` control signal set /// it (the live-switch signal). @@ -737,7 +738,6 @@ mod tests { captured_at: "".to_string(), } } - /// Definition-authoritative: a stale materialized `record.model` on a /// linked instance must never outrank (or even be consulted against) the /// linked persona's model. `update_managed_agent` already blocks writing diff --git a/desktop/src-tauri/src/commands/agent_models.rs b/desktop/src-tauri/src/commands/agent_models.rs index 7ce03b140b..d4dc90fd9a 100644 --- a/desktop/src-tauri/src/commands/agent_models.rs +++ b/desktop/src-tauri/src/commands/agent_models.rs @@ -905,7 +905,6 @@ pub async fn update_managed_agent( record.model = Some(model_ref.clone()); record.relay_mesh = Some(crate::managed_agents::RelayMeshConfig { model_ref }); } - // Inbound author gate: merge patch onto current values, then validate // the merged state. This lets a single update switch to Allowlist AND // supply pubkeys atomically. @@ -928,11 +927,11 @@ pub async fn update_managed_agent( if input.respond_to_allowlist.is_some() { record.respond_to_allowlist = prospective_allowlist; } - + if let Some(reply_placement) = input.reply_placement { + record.reply_placement = reply_placement; + } record.updated_at = now_iso(); - save_managed_agents(&app, &records)?; - let record = records .iter() .find(|r| r.pubkey == input.pubkey) @@ -970,13 +969,8 @@ pub async fn update_managed_agent( let summary = { let personas = load_personas(&app).unwrap_or_default(); - build_managed_agent_summary( - &app, - record, - &runtimes, - &personas, - &crate::managed_agents::load_global_agent_config(&app).unwrap_or_default(), - )? + let global = crate::managed_agents::load_global_agent_config(&app)?; + build_managed_agent_summary(&app, record, &runtimes, &personas, &global)? }; let rollback = name_changed.then(|| AgentUpdateRollback::new(previous_record, record)); (summary, sync_params, rollback) diff --git a/desktop/src-tauri/src/commands/agent_models_tests.rs b/desktop/src-tauri/src/commands/agent_models_tests.rs index 14c981d730..bcc0ab5ab7 100644 --- a/desktop/src-tauri/src/commands/agent_models_tests.rs +++ b/desktop/src-tauri/src/commands/agent_models_tests.rs @@ -402,6 +402,7 @@ fn model_discovery_ignores_stale_record_for_linked_agent() { respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, created_at: "".to_string(), updated_at: "".to_string(), }; diff --git a/desktop/src-tauri/src/commands/agent_settings.rs b/desktop/src-tauri/src/commands/agent_settings.rs index 2317930c1e..bbb9376ac1 100644 --- a/desktop/src-tauri/src/commands/agent_settings.rs +++ b/desktop/src-tauri/src/commands/agent_settings.rs @@ -57,13 +57,8 @@ pub async fn set_managed_agent_start_on_app_launch( .find(|record| record.pubkey == pubkey) .ok_or_else(|| format!("agent {pubkey} not found"))?; let personas = load_personas(&app).unwrap_or_default(); - build_managed_agent_summary( - &app, - record, - &runtimes, - &personas, - &crate::managed_agents::load_global_agent_config(&app).unwrap_or_default(), - ) + let global = crate::managed_agents::load_global_agent_config(&app)?; + build_managed_agent_summary(&app, record, &runtimes, &personas, &global) }) .await .map_err(|e| format!("spawn_blocking failed: {e}"))? @@ -108,13 +103,8 @@ pub async fn set_managed_agent_auto_restart( .find(|record| record.pubkey == pubkey) .ok_or_else(|| format!("agent {pubkey} not found"))?; let personas = load_personas(&app).unwrap_or_default(); - build_managed_agent_summary( - &app, - record, - &runtimes, - &personas, - &crate::managed_agents::load_global_agent_config(&app).unwrap_or_default(), - ) + let global = crate::managed_agents::load_global_agent_config(&app)?; + build_managed_agent_summary(&app, record, &runtimes, &personas, &global) }) .await .map_err(|e| format!("spawn_blocking failed: {e}"))? diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 0758fc3aac..37cac33f6f 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -254,7 +254,6 @@ async fn ensure_relay_mesh_for_record( ) -> Result<(), String> { crate::commands::ensure_relay_mesh_for_record(app, model_id, allow_fresh_create_start).await } - #[cfg(not(feature = "mesh-llm"))] async fn ensure_relay_mesh_for_record( _app: &AppHandle, @@ -263,7 +262,6 @@ async fn ensure_relay_mesh_for_record( ) -> Result<(), String> { Ok(()) } - pub(super) async fn start_local_agent_pairs_with_preflight( app: &AppHandle, state: &AppState, @@ -284,8 +282,7 @@ pub(super) async fn start_local_agent_pairs_with_preflight( return Err(format!("agent {pubkey} is not a local agent")); } let personas_for_preflight = load_personas(app).unwrap_or_default(); - let global_for_preflight = - crate::managed_agents::load_global_agent_config(app).unwrap_or_default(); + let global_for_preflight = crate::managed_agents::load_global_agent_config(app)?; let mesh_model_id = crate::managed_agents::effective_config::resolve_effective_relay_mesh_model_id( &record_snapshot, @@ -293,7 +290,6 @@ pub(super) async fn start_local_agent_pairs_with_preflight( &global_for_preflight, ); ensure_relay_mesh_for_record(app, mesh_model_id.as_deref(), false).await?; - { let _store_guard = state .managed_agents_store_lock @@ -313,7 +309,6 @@ pub(super) async fn start_local_agent_pairs_with_preflight( retain_managed_agent_pending(app, state, saved_record); } } - let mut errors = Vec::new(); for relay_url in relay_urls { if let Err(error) = crate::managed_agents::start_managed_agent_runtime_pair_lazy( @@ -330,7 +325,6 @@ pub(super) async fn start_local_agent_pairs_with_preflight( errors.join("; ") )); } - let _store_guard = state .managed_agents_store_lock .lock() @@ -345,13 +339,8 @@ pub(super) async fn start_local_agent_pairs_with_preflight( .iter() .find(|record| record.pubkey == pubkey) .ok_or_else(|| format!("agent {pubkey} not found"))?; - build_managed_agent_summary( - app, - record, - &runtimes, - &personas, - &crate::managed_agents::load_global_agent_config(app).unwrap_or_default(), - ) + let global = crate::managed_agents::load_global_agent_config(app)?; + build_managed_agent_summary(app, record, &runtimes, &personas, &global) } pub(super) async fn start_local_agent_with_preflight( @@ -386,7 +375,7 @@ pub(super) async fn start_local_agent_with_preflight( // for a global-inherited blank definition, it also folds in the global // default, which record-byte sniffing could never see. let personas = load_personas(app).unwrap_or_default(); - let global = crate::managed_agents::load_global_agent_config(app).unwrap_or_default(); + let global = crate::managed_agents::load_global_agent_config(app)?; let mesh_model_id = crate::managed_agents::effective_config::resolve_effective_relay_mesh_model_id( &record_snapshot, @@ -394,7 +383,6 @@ pub(super) async fn start_local_agent_with_preflight( &global, ); ensure_relay_mesh_for_record(app, mesh_model_id.as_deref(), allow_fresh_create_start).await?; - let _store_guard = state .managed_agents_store_lock .lock() @@ -438,13 +426,8 @@ pub(super) async fn start_local_agent_with_preflight( .iter() .find(|record| record.pubkey == pubkey) .ok_or_else(|| format!("agent {pubkey} not found"))?; - build_managed_agent_summary( - app, - record, - &runtimes, - &personas, - &crate::managed_agents::load_global_agent_config(app).unwrap_or_default(), - ) + let global = crate::managed_agents::load_global_agent_config(app)?; + build_managed_agent_summary(app, record, &runtimes, &personas, &global) } /// Deploy an agent to a provider backend. Resolves the binary, calls deploy via @@ -543,13 +526,11 @@ pub async fn list_managed_agents(app: AppHandle) -> Result Err(format!( "agent {pubkey} has unsupported backend kind: {backend:?}" @@ -1258,13 +1221,8 @@ pub async fn stop_managed_agent( .find(|record| record.pubkey == pubkey) .ok_or_else(|| format!("agent {pubkey} not found"))?; let personas = load_personas(&app).unwrap_or_default(); - build_managed_agent_summary( - &app, - record, - &runtimes, - &personas, - &crate::managed_agents::load_global_agent_config(&app).unwrap_or_default(), - ) + let global = crate::managed_agents::load_global_agent_config(&app)?; + build_managed_agent_summary(&app, record, &runtimes, &personas, &global) }) .await .map_err(|e| format!("spawn_blocking failed: {e}"))? @@ -1362,6 +1320,8 @@ use deploy::build_deploy_payload; #[cfg(test)] use deploy::deploy_payload_json; #[cfg(test)] +pub(crate) use deploy::resolve_deploy_config; +#[cfg(test)] pub(crate) use deploy::resolve_deploy_model_provider; #[path = "agents_profile.rs"] diff --git a/desktop/src-tauri/src/commands/agents_deploy.rs b/desktop/src-tauri/src/commands/agents_deploy.rs index af785711d5..e062ae4f48 100644 --- a/desktop/src-tauri/src/commands/agents_deploy.rs +++ b/desktop/src-tauri/src/commands/agents_deploy.rs @@ -4,11 +4,13 @@ use tauri::AppHandle; -#[cfg(test)] use crate::managed_agents::AgentDefinition; use crate::{ app_state::AppState, - managed_agents::{load_personas, ManagedAgentRecord}, + managed_agents::{ + load_personas, EffectiveHarnessDescriptor, GlobalAgentConfig, ManagedAgentRecord, + ReplyPlacement, + }, relay::relay_ws_url_with_override, }; @@ -36,15 +38,31 @@ pub(crate) fn resolve_deploy_model_provider( .unwrap_or((None, None)) } +/// Resolve the global config and effective reply placement used by a provider +/// deploy. Keeping the disk-load result in this helper makes the deploy +/// boundary fail closed on malformed persisted config instead of silently +/// reverting to the historical `thread` mode. +pub(crate) fn resolve_deploy_config( + record: &ManagedAgentRecord, + personas: &[AgentDefinition], + global_config: Result, +) -> Result<(GlobalAgentConfig, ReplyPlacement), String> { + let global_config = global_config?; + let reply_placement = crate::managed_agents::resolve_effective_reply_placement( + record, + personas, + global_config.reply_placement, + )?; + Ok((global_config, reply_placement)) +} + /// Build the standard agent JSON payload for provider deploy calls. /// -/// Like local spawn, provider deploy re-reads live persona env vars and -/// structured model/provider so remote agents receive current credentials -/// and the same authoritative values that local spawn derives from -/// `runtime_metadata_env_vars`. The only field still pinned is -/// `agent_command`/`agent_args` — those were captured at create time. -/// The only read-time resolution is `relay_url`: a blank pin resolves to -/// the active workspace relay here, matching the create-path contract. +/// Like local spawn, provider deploy re-reads the live persona and global +/// configuration, then carries the complete effective harness descriptor so +/// the provider does not have to duplicate desktop-side command, argument, or +/// environment resolution. The legacy top-level fields remain for protocol +/// compatibility; `launch` is the authoritative execution contract. /// /// Fails closed when the private key is unavailable (keyring outage leaves /// it empty after hydration): without this guard a provider deploy would @@ -63,12 +81,18 @@ pub(super) fn build_deploy_payload( return Err(err); } + let personas = load_personas(app).unwrap_or_default(); + let (global_config, reply_placement) = resolve_deploy_config( + record, + &personas, + crate::managed_agents::load_global_agent_config(app), + )?; + // Merge global + persona + agent env_vars for provider deploy — the same // live-persona-under-overrides semantics as local spawn. Global env vars // are the lowest user-settable layer: global < persona < agent (last-wins // on key collision). Without this, provider-backed agents wouldn't receive // credentials saved on the persona or the agent itself. - let global_config = crate::managed_agents::load_global_agent_config(app).unwrap_or_default(); let global_env = global_config.env_vars.clone(); let persona_env = crate::managed_agents::resolve_persona_env(app, record.persona_id.as_deref())?; @@ -78,7 +102,6 @@ pub(super) fn build_deploy_payload( let merged_env = crate::managed_agents::merged_user_env(&global_persona_merged, &record.env_vars); - let personas = load_personas(app).unwrap_or_default(); let cfg = crate::managed_agents::effective_config::resolve_effective_config( record, &personas, @@ -88,31 +111,89 @@ pub(super) fn build_deploy_payload( let effective_model = cfg.model.value; let effective_provider = cfg.provider.value; let effective_prompt = cfg.system_prompt.value; + let launch = crate::managed_agents::resolve_effective_harness_descriptor( + record, + &personas, + &global_config, + )?; + let teams = crate::managed_agents::load_teams(app)?; + let policy_env = crate::managed_agents::resolve_effective_launch_policy_env( + record, + &launch.command, + &teams, + effective_prompt.as_deref(), + effective_model.as_deref(), + reply_placement, + true, + ); + let owner_pubkey = Some(super::workspace_owner_hex(state)?); Ok(deploy_payload_json( record, - crate::relay::effective_agent_relay_url( - &record.relay_url, - &relay_ws_url_with_override(state), - ), - effective_model, - effective_provider, - effective_prompt, - merged_env, + DeployPayloadContext { + relay_url: crate::relay::effective_agent_relay_url( + &record.relay_url, + &relay_ws_url_with_override(state), + ), + effective_model, + effective_provider, + effective_prompt, + reply_placement, + merged_env, + launch, + policy_env, + owner_pubkey, + }, )) } +pub(super) struct DeployPayloadContext { + pub(super) relay_url: String, + pub(super) effective_model: Option, + pub(super) effective_provider: Option, + pub(super) effective_prompt: Option, + pub(super) reply_placement: ReplyPlacement, + pub(super) merged_env: std::collections::BTreeMap, + pub(super) launch: EffectiveHarnessDescriptor, + pub(super) policy_env: std::collections::BTreeMap, + pub(super) owner_pubkey: Option, +} + /// Pure serialization half of [`build_deploy_payload`] — every field the /// provider harness receives is deliberately listed here, so payload /// completeness is testable without an `AppHandle`. pub(super) fn deploy_payload_json( record: &ManagedAgentRecord, - relay_url: String, - effective_model: Option, - effective_provider: Option, - effective_prompt: Option, - merged_env: std::collections::BTreeMap, + context: DeployPayloadContext, ) -> serde_json::Value { + let DeployPayloadContext { + relay_url, + effective_model, + effective_provider, + effective_prompt, + reply_placement, + merged_env, + launch, + policy_env, + owner_pubkey, + } = context; + + // The shared descriptor resolver already strips reserved keys while + // layering user env, but enforce the same invariant at the provider wire + // boundary so hand-built/legacy descriptors cannot smuggle a policy gate + // into launch.env. Reply placement is transported in policy_env and is + // emitted below as the single authoritative value. + let mut launch_env = launch.env; + launch_env.retain(|key, _| !crate::managed_agents::is_reserved_env_key(key)); + let mut launch_policy_env = policy_env; + // This is the only reserved policy key. Enforce it again at the wire + // boundary so a hand-built/legacy policy map cannot make the provider run + // a mode different from the typed effective value. + launch_policy_env.insert( + "BUZZ_ACP_REPLY_PLACEMENT".to_string(), + reply_placement.as_str().to_string(), + ); + serde_json::json!({ "name": &record.name, "relay_url": relay_url, @@ -123,12 +204,23 @@ pub(super) fn deploy_payload_json( "system_prompt": effective_prompt, "model": effective_model, "provider": effective_provider, + "reply_placement": reply_placement.as_str(), "turn_timeout_seconds": record.turn_timeout_seconds, "idle_timeout_seconds": record.idle_timeout_seconds, "max_turn_duration_seconds": record.max_turn_duration_seconds, "parallelism": record.parallelism, "respond_to": record.respond_to, "respond_to_allowlist": &record.respond_to_allowlist, - "env_vars": merged_env, + "env_vars": &merged_env, + // Provider launchers apply this desktop-resolved policy env to the + // remote harness. It is separate from user env so a persisted + // BUZZ_ACP_REPLY_PLACEMENT value cannot override the effective mode. + "launch": { + "command": launch.command, + "args": launch.args, + "env": launch_env, + "policy_env": launch_policy_env, + "owner_pubkey": owner_pubkey, + }, }) } diff --git a/desktop/src-tauri/src/commands/agents_tests.rs b/desktop/src-tauri/src/commands/agents_tests.rs index 03389d1d18..1302be1237 100644 --- a/desktop/src-tauri/src/commands/agents_tests.rs +++ b/desktop/src-tauri/src/commands/agents_tests.rs @@ -1,3 +1,4 @@ +use super::deploy::DeployPayloadContext; use super::*; use crate::managed_agents::AgentDefinition; @@ -47,6 +48,7 @@ fn bare_agent_record( last_error_code: None, respond_to: RespondTo::OwnerOnly, respond_to_allowlist: vec![], + reply_placement: None, display_name: None, slug: None, runtime: None, @@ -62,6 +64,7 @@ fn bare_agent_record( definition_respond_to: None, definition_respond_to_allowlist: vec![], definition_parallelism: None, + definition_reply_placement: None, } } fn persona_record(id: &str, model: Option<&str>, provider: Option<&str>) -> AgentDefinition { @@ -85,6 +88,7 @@ fn persona_record(id: &str, model: Option<&str>, provider: Option<&str>) -> Agen respond_to: None, respond_to_allowlist: vec![], parallelism: None, + reply_placement: None, created_at: "".to_string(), updated_at: "".to_string(), } @@ -429,13 +433,30 @@ fn deploy_payload_carries_the_full_behavioral_quad() { )) .expect("sample record"); + let launch = crate::managed_agents::EffectiveHarnessDescriptor { + command: "goose".to_string(), + args: vec!["--proof".to_string()], + env: std::collections::BTreeMap::from([( + "FAKE_LAUNCH_MARKER".to_string(), + "from-descriptor".to_string(), + )]), + }; let payload = deploy_payload_json( &record, - "wss://relay.example".to_string(), - Some("gpt-x".to_string()), - Some("openai".to_string()), - None, - std::collections::BTreeMap::new(), + DeployPayloadContext { + relay_url: "wss://relay.example".to_string(), + effective_model: Some("gpt-x".to_string()), + effective_provider: Some("openai".to_string()), + effective_prompt: None, + reply_placement: crate::managed_agents::ReplyPlacement::Thread, + merged_env: std::collections::BTreeMap::new(), + launch, + policy_env: std::collections::BTreeMap::from([( + "BUZZ_ACP_SYSTEM_PROMPT".to_string(), + "policy-prompt".to_string(), + )]), + owner_pubkey: Some("owner-proof".to_string()), + }, ); assert_eq!(payload["parallelism"], 4); @@ -444,4 +465,316 @@ fn deploy_payload_carries_the_full_behavioral_quad() { assert_eq!(payload["model"], "gpt-x"); assert_eq!(payload["provider"], "openai"); assert_eq!(payload["relay_url"], "wss://relay.example"); + assert_eq!(payload["launch"]["command"], "goose"); + assert_eq!(payload["launch"]["args"][0], "--proof"); + assert_eq!( + payload["launch"]["env"]["FAKE_LAUNCH_MARKER"], + "from-descriptor" + ); + assert_eq!(payload["launch"]["owner_pubkey"], "owner-proof"); + assert_eq!( + payload["launch"]["policy_env"]["BUZZ_ACP_SYSTEM_PROMPT"], + "policy-prompt" + ); +} + +#[test] +fn deploy_payload_carries_each_reply_placement_to_provider_launch_env() { + use crate::managed_agents::ReplyPlacement; + + for (mode, wire) in [ + (ReplyPlacement::Thread, "thread"), + (ReplyPlacement::TopLevel, "top-level"), + (ReplyPlacement::FollowScope, "follow-scope"), + ] { + let record = bare_agent_record(None, None, None); + let global = crate::managed_agents::GlobalAgentConfig { + reply_placement: Some(mode), + ..Default::default() + }; + let (_, resolved_mode) = resolve_deploy_config(&record, &[], Ok(global)) + .expect("provider deploy mode should resolve through the shared helper"); + let payload = deploy_payload_json( + &record, + DeployPayloadContext { + relay_url: "wss://relay.example".to_string(), + effective_model: None, + effective_provider: None, + effective_prompt: None, + reply_placement: resolved_mode, + merged_env: std::collections::BTreeMap::new(), + launch: crate::managed_agents::EffectiveHarnessDescriptor { + command: "goose".to_string(), + args: vec![], + env: std::collections::BTreeMap::new(), + }, + policy_env: std::collections::BTreeMap::new(), + owner_pubkey: Some("owner-proof".to_string()), + }, + ); + + assert_eq!(payload["reply_placement"], wire); + assert_eq!( + payload["launch"]["policy_env"]["BUZZ_ACP_REPLY_PLACEMENT"], + wire + ); + } +} + +#[test] +fn provider_deploy_refuses_malformed_global_config() { + let record = bare_agent_record(None, None, None); + let err = resolve_deploy_config( + &record, + &[], + Err("failed to parse global agent config: invalid json".to_string()), + ) + .expect_err("provider deploy must not substitute the thread default"); + + assert!(err.contains("failed to parse global agent config")); +} + +#[test] +fn provider_policy_env_contains_all_desktop_resolved_launch_defaults() { + use crate::managed_agents::{ReplyPlacement, TeamRecord}; + + let mut record = bare_agent_record(None, None, None); + record.display_name = Some("Policy Display".to_string()); + record.team_id = Some("team-1".to_string()); + record.idle_timeout_seconds = Some(11); + record.max_turn_duration_seconds = Some(22); + record.parallelism = 4; + + let teams = vec![TeamRecord { + id: "team-1".to_string(), + name: "Team".to_string(), + description: None, + instructions: Some("shared team instructions".to_string()), + persona_ids: vec![], + is_builtin: false, + source_dir: None, + is_symlink: false, + symlink_target: None, + version: None, + created_at: String::new(), + updated_at: String::new(), + }]; + + let policy = crate::managed_agents::resolve_effective_launch_policy_env( + &record, + "goose", + &teams, + Some("desktop prompt"), + Some("desktop model"), + ReplyPlacement::FollowScope, + true, + ); + + for (key, expected) in [ + ("GOOSE_MODE", "auto"), + ("BUZZ_ACP_LAZY_POOL", "true"), + ("BUZZ_ACP_RELAY_OBSERVER", "true"), + ("BUZZ_ACP_SYSTEM_PROMPT", "desktop prompt"), + ("BUZZ_ACP_MODEL", "desktop model"), + ("BUZZ_ACP_IDLE_TIMEOUT", "11"), + ("BUZZ_ACP_MAX_TURN_DURATION", "22"), + ("BUZZ_ACP_AGENTS", "4"), + ("BUZZ_ACP_SESSION_TITLE", "Policy Display"), + ("BUZZ_ACP_TEAM_INSTRUCTIONS", "shared team instructions"), + ("BUZZ_ACP_REPLY_PLACEMENT", "follow-scope"), + ] { + assert_eq!(policy.get(key).map(String::as_str), Some(expected), "{key}"); + } + + let hooks = crate::managed_agents::resolve_effective_launch_policy_env( + &record, + "buzz-agent", + &teams, + None, + None, + ReplyPlacement::Thread, + true, + ); + assert_eq!(hooks.get("MCP_HOOK_SERVERS").map(String::as_str), Some("*")); +} + +/// The provider boundary must be executable, not just JSON-shaped: a fake +/// provider applies the launch contract to a fake harness and the harness +/// records the environment it received. The reserved reply-placement key is +/// deliberately poisoned in the user env to prove the policy value wins. +#[cfg(unix)] +#[test] +fn provider_deploy_executes_launch_contract_and_preserves_reply_placement() { + use crate::managed_agents::provider_deploy; + use std::fs; + use std::os::unix::fs::PermissionsExt; + use std::path::Path; + + fn shell_quote(path: &Path) -> String { + let value = path.to_string_lossy().replace('\'', "'\\\"'\\\"'"); + format!("'{value}'") + } + + fn make_executable(path: &Path) { + let mut permissions = fs::metadata(path).expect("script metadata").permissions(); + permissions.set_mode(0o700); + fs::set_permissions(path, permissions).expect("make script executable"); + } + + let dir = tempfile::tempdir().expect("temporary provider directory"); + let harness_path = dir.path().join("fake-harness"); + let observed_path = dir.path().join("observed-env"); + let provider_path = dir.path().join("fake-provider"); + + fs::write( + &harness_path, + format!( + "#!/bin/sh\nset -eu\nprintf '%s|%s|%s|%s|%s|%s|%s|%s|%s|%s|%s|%s' \\ +\"$FAKE_LAUNCH_MARKER\" \\ +\"$BUZZ_ACP_REPLY_PLACEMENT\" \\ +\"$BUZZ_ACP_SYSTEM_PROMPT\" \\ +\"$BUZZ_ACP_IDLE_TIMEOUT\" \\ +\"$BUZZ_ACP_MAX_TURN_DURATION\" \\ +\"$BUZZ_ACP_AGENTS\" \\ +\"$GOOSE_MODE\" \\ +\"$BUZZ_ACP_LAZY_POOL\" \\ +\"$BUZZ_ACP_RELAY_OBSERVER\" \\ +\"$BUZZ_ACP_SESSION_TITLE\" \\ +\"$BUZZ_ACP_TEAM_INSTRUCTIONS\" \\ +\"$1:$2\" > {}\n", + shell_quote(&observed_path) + ), + ) + .expect("write fake harness"); + make_executable(&harness_path); + + fs::write( + &provider_path, + r#"#!/usr/bin/env python3 +import json +import os +import subprocess +import sys + +request = json.load(sys.stdin) +agent = request["agent"] +launch = agent["launch"] +assert isinstance(launch["command"], str) and launch["command"] +assert isinstance(launch["args"], list) +assert isinstance(launch["env"], dict) +assert isinstance(launch["policy_env"], dict) +assert launch["owner_pubkey"] +policy_env = launch["policy_env"] +launch_env = launch["env"] +env = os.environ.copy() +env["BUZZ_RELAY_URL"] = agent["relay_url"] +env["BUZZ_PRIVATE_KEY"] = agent["private_key_nsec"] +if agent.get("auth_tag"): + env["BUZZ_AUTH_TAG"] = agent["auth_tag"] +else: + env["BUZZ_ACP_AGENT_OWNER"] = launch["owner_pubkey"] +env.update(policy_env) +env.update(launch_env) +# The policy key is reserved: it wins even if launch.env contained a stale value. +env["BUZZ_ACP_REPLY_PLACEMENT"] = policy_env["BUZZ_ACP_REPLY_PLACEMENT"] +subprocess.run( + [launch["command"], *launch["args"]], + check=True, + env=env, +) +print(json.dumps({"ok": True, "agent_id": "fake-provider"})) +"#, + ) + .expect("write fake provider"); + make_executable(&provider_path); + + let _path_guard = crate::managed_agents::lock_path_mutex(); + let original_path = std::env::var_os("PATH").unwrap_or_default(); + let provider_path_value = format!( + "{}:{}", + dir.path().display(), + original_path.to_string_lossy() + ); + std::env::set_var("PATH", provider_path_value); + + for mode in [ + crate::managed_agents::ReplyPlacement::Thread, + crate::managed_agents::ReplyPlacement::TopLevel, + crate::managed_agents::ReplyPlacement::FollowScope, + ] { + let launch = crate::managed_agents::EffectiveHarnessDescriptor { + command: "fake-harness".to_string(), + args: vec!["--contract".to_string(), "v1".to_string()], + env: std::collections::BTreeMap::from([ + ("BUZZ_ACP_REPLY_PLACEMENT".to_string(), "thread".to_string()), + ( + "FAKE_LAUNCH_MARKER".to_string(), + "from-descriptor".to_string(), + ), + ( + "BUZZ_ACP_SYSTEM_PROMPT".to_string(), + "user-prompt".to_string(), + ), + ("BUZZ_ACP_IDLE_TIMEOUT".to_string(), "99".to_string()), + ]), + }; + let policy_env = std::collections::BTreeMap::from([ + ( + "BUZZ_ACP_SYSTEM_PROMPT".to_string(), + "desktop-prompt".to_string(), + ), + ("BUZZ_ACP_IDLE_TIMEOUT".to_string(), "11".to_string()), + ("BUZZ_ACP_MAX_TURN_DURATION".to_string(), "22".to_string()), + ("BUZZ_ACP_AGENTS".to_string(), "4".to_string()), + ("GOOSE_MODE".to_string(), "auto".to_string()), + ("BUZZ_ACP_LAZY_POOL".to_string(), "true".to_string()), + ("BUZZ_ACP_RELAY_OBSERVER".to_string(), "true".to_string()), + ( + "BUZZ_ACP_SESSION_TITLE".to_string(), + "Policy Display".to_string(), + ), + ( + "BUZZ_ACP_TEAM_INSTRUCTIONS".to_string(), + "shared team instructions".to_string(), + ), + ( + "BUZZ_ACP_REPLY_PLACEMENT".to_string(), + mode.as_str().to_string(), + ), + ]); + let payload = deploy_payload_json( + &bare_agent_record(None, None, None), + DeployPayloadContext { + relay_url: "wss://relay.example".to_string(), + effective_model: None, + effective_provider: None, + effective_prompt: None, + reply_placement: mode, + merged_env: std::collections::BTreeMap::from([( + "BUZZ_ACP_REPLY_PLACEMENT".to_string(), + "thread".to_string(), + )]), + launch, + policy_env, + owner_pubkey: Some("owner-proof".to_string()), + }, + ); + + let agent_id = provider_deploy(&provider_path, &payload, &serde_json::json!({})) + .expect("fake provider deploy"); + assert_eq!(agent_id, "fake-provider"); + assert!(payload["launch"]["env"] + .get("BUZZ_ACP_REPLY_PLACEMENT") + .is_none()); + let observed = fs::read_to_string(&observed_path).expect("fake harness output"); + assert_eq!( + observed, + format!( + "from-descriptor|{}|user-prompt|99|22|4|auto|true|true|Policy Display|shared team instructions|--contract:v1", + mode.as_str() + ) + ); + } + + std::env::set_var("PATH", original_path); } diff --git a/desktop/src-tauri/src/commands/media_download.rs b/desktop/src-tauri/src/commands/media_download.rs index 7bc94da25d..65183ca677 100644 --- a/desktop/src-tauri/src/commands/media_download.rs +++ b/desktop/src-tauri/src/commands/media_download.rs @@ -621,6 +621,7 @@ mod tests { parallelism: None, respond_to: None, respond_to_allowlist: vec![], + reply_placement: None, name_pool: vec![], idle_timeout_seconds: None, max_turn_duration_seconds: None, @@ -671,6 +672,7 @@ mod tests { parallelism: None, respond_to: None, respond_to_allowlist: vec![], + reply_placement: None, name_pool: vec![], idle_timeout_seconds: None, max_turn_duration_seconds: None, @@ -717,6 +719,7 @@ mod tests { parallelism: None, respond_to: None, respond_to_allowlist: vec![], + reply_placement: None, name_pool: vec![], idle_timeout_seconds: None, max_turn_duration_seconds: None, diff --git a/desktop/src-tauri/src/commands/media_snapshot_png.rs b/desktop/src-tauri/src/commands/media_snapshot_png.rs index bcaec6a592..a7cc352589 100644 --- a/desktop/src-tauri/src/commands/media_snapshot_png.rs +++ b/desktop/src-tauri/src/commands/media_snapshot_png.rs @@ -166,6 +166,7 @@ mod tests { parallelism: Some(1), respond_to: None, respond_to_allowlist: vec![], + reply_placement: None, idle_timeout_seconds: None, max_turn_duration_seconds: None, name_pool: vec![], diff --git a/desktop/src-tauri/src/commands/personas/create.rs b/desktop/src-tauri/src/commands/personas/create.rs index c00de1c6da..8a1b60a0f3 100644 --- a/desktop/src-tauri/src/commands/personas/create.rs +++ b/desktop/src-tauri/src/commands/personas/create.rs @@ -70,6 +70,7 @@ pub async fn create_persona( respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, created_at: now.clone(), updated_at: now, }; diff --git a/desktop/src-tauri/src/commands/personas/delete_cascade_tests.rs b/desktop/src-tauri/src/commands/personas/delete_cascade_tests.rs index 8ff7cfbd9b..aae900a6be 100644 --- a/desktop/src-tauri/src/commands/personas/delete_cascade_tests.rs +++ b/desktop/src-tauri/src/commands/personas/delete_cascade_tests.rs @@ -55,6 +55,7 @@ fn make_agent( last_error_code: None, respond_to: RespondTo::OwnerOnly, respond_to_allowlist: vec![], + reply_placement: None, display_name: None, slug: None, runtime: None, @@ -70,6 +71,7 @@ fn make_agent( definition_respond_to: None, definition_respond_to_allowlist: vec![], definition_parallelism: None, + definition_reply_placement: None, } } diff --git a/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs b/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs index 1005a83432..e2a632718e 100644 --- a/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs +++ b/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs @@ -28,6 +28,7 @@ fn local_in_app() -> AgentDefinition { respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, created_at: "2025-01-01T00:00:00Z".to_string(), updated_at: "2025-01-01T00:00:00Z".to_string(), } @@ -55,6 +56,7 @@ fn inbound_for(d_tag: &str, display_name: &str) -> AgentDefinition { respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, created_at: "2025-06-01T00:00:00Z".to_string(), updated_at: "2025-06-01T00:00:00Z".to_string(), } @@ -201,6 +203,7 @@ fn local_agent() -> ManagedAgentRecord { last_error_code: None, respond_to: crate::managed_agents::RespondTo::OwnerOnly, respond_to_allowlist: vec![], + reply_placement: None, display_name: None, slug: None, runtime: None, @@ -214,6 +217,7 @@ fn local_agent() -> ManagedAgentRecord { definition_respond_to: None, definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, + definition_reply_placement: None, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/commands/personas/mod.rs b/desktop/src-tauri/src/commands/personas/mod.rs index 0cd7ad0324..e2a9812543 100644 --- a/desktop/src-tauri/src/commands/personas/mod.rs +++ b/desktop/src-tauri/src/commands/personas/mod.rs @@ -312,8 +312,8 @@ pub use card::*; #[cfg(test)] pub(crate) use snapshot::import::decode_snapshot_from_bytes; pub(crate) use snapshot::import::{ - parse_snapshot_payload_from_bytes, resolve_snapshot_import_behavior, MAX_SNAPSHOT_JSON_BYTES, - MAX_SNAPSHOT_PNG_BYTES, + parse_snapshot_payload_from_bytes, resolve_snapshot_import_behavior_with_reply, + MAX_SNAPSHOT_JSON_BYTES, MAX_SNAPSHOT_PNG_BYTES, }; pub use snapshot::{confirm_agent_snapshot_import, preview_agent_snapshot_import}; pub use snapshot::{encode_agent_snapshot_for_send, export_agent_snapshot}; diff --git a/desktop/src-tauri/src/commands/personas/pending.rs b/desktop/src-tauri/src/commands/personas/pending.rs index cab5fababc..69e9db69d4 100644 --- a/desktop/src-tauri/src/commands/personas/pending.rs +++ b/desktop/src-tauri/src/commands/personas/pending.rs @@ -272,6 +272,7 @@ mod tests { respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, created_at: "2026-07-27T00:00:00Z".to_string(), updated_at: "2026-07-27T00:00:00Z".to_string(), } diff --git a/desktop/src-tauri/src/commands/personas/sharing.rs b/desktop/src-tauri/src/commands/personas/sharing.rs index 914c56252d..c6a2fa2c3b 100644 --- a/desktop/src-tauri/src/commands/personas/sharing.rs +++ b/desktop/src-tauri/src/commands/personas/sharing.rs @@ -161,6 +161,7 @@ mod tests { respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, created_at: "2026-07-27T00:00:00Z".to_string(), updated_at: "2026-07-27T00:00:00Z".to_string(), } diff --git a/desktop/src-tauri/src/commands/personas/snapshot.rs b/desktop/src-tauri/src/commands/personas/snapshot.rs index e7bd1597e6..c75e53ea52 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot.rs @@ -487,6 +487,7 @@ mod png_body_tests { parallelism: None, respond_to: None, respond_to_allowlist: vec![], + reply_placement: None, name_pool: vec![], idle_timeout_seconds: None, max_turn_duration_seconds: None, diff --git a/desktop/src-tauri/src/commands/personas/snapshot/behavior.rs b/desktop/src-tauri/src/commands/personas/snapshot/behavior.rs new file mode 100644 index 0000000000..44692ac395 --- /dev/null +++ b/desktop/src-tauri/src/commands/personas/snapshot/behavior.rs @@ -0,0 +1,70 @@ +use crate::managed_agents::{ + resolve_mint_behavioral_defaults, validate_respond_to_allowlist, ReplyPlacement, RespondTo, +}; + +/// Resolve the behavioral defaults for an incoming agent snapshot. +/// +/// This is the single authoritative selection path for import-time allowlist +/// and behavioral decisions. The Keep/Clear toggle is shown whenever the raw +/// allowlist is non-empty, regardless of the source mode. +#[cfg(test)] +pub(crate) fn resolve_snapshot_import_behavior( + raw_respond_to: Option<&str>, + raw_allowlist: &[String], + parallelism: Option, + keep_allowlist: bool, +) -> Result { + resolve_snapshot_import_behavior_with_reply( + raw_respond_to, + raw_allowlist, + parallelism, + None, + keep_allowlist, + ) +} + +pub(crate) fn resolve_snapshot_import_behavior_with_reply( + raw_respond_to: Option<&str>, + raw_allowlist: &[String], + parallelism: Option, + raw_reply_placement: Option<&str>, + keep_allowlist: bool, +) -> Result { + let normalized_allowlist = validate_respond_to_allowlist(raw_allowlist)?; + let source_mode = raw_respond_to.map(RespondTo::parse_wire).transpose()?; + let is_source_allowlist_mode = source_mode == Some(RespondTo::Allowlist); + let has_source_allowlist = !normalized_allowlist.is_empty(); + + if is_source_allowlist_mode && !has_source_allowlist { + return Err( + "snapshot respond-to mode is 'allowlist' but the allowlist is empty — \ + cannot import: no pubkeys to grant access to" + .to_string(), + ); + } + + let (resolved_mode, resolved_allowlist) = if has_source_allowlist { + if keep_allowlist { + (source_mode, normalized_allowlist) + } else if is_source_allowlist_mode { + (Some(RespondTo::OwnerOnly), Vec::new()) + } else { + (source_mode, Vec::new()) + } + } else { + (source_mode, normalized_allowlist) + }; + + let reply_placement = raw_reply_placement + .map(ReplyPlacement::parse_wire) + .transpose()?; + + resolve_mint_behavioral_defaults( + resolved_mode, + resolved_allowlist, + parallelism, + reply_placement, + None, + None, + ) +} diff --git a/desktop/src-tauri/src/commands/personas/snapshot/fidelity_tests.rs b/desktop/src-tauri/src/commands/personas/snapshot/fidelity_tests.rs index b769d74d7b..931a121508 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot/fidelity_tests.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot/fidelity_tests.rs @@ -52,6 +52,7 @@ fn make_definition(slug: &str) -> ManagedAgentRecord { last_error_code: None, respond_to: RespondTo::default(), respond_to_allowlist: vec![], + reply_placement: None, runtime: None, name_pool: vec![], is_builtin: false, @@ -63,6 +64,7 @@ fn make_definition(slug: &str) -> ManagedAgentRecord { definition_respond_to: None, definition_respond_to_allowlist: vec![], definition_parallelism: None, + definition_reply_placement: None, relay_mesh: None, } } @@ -85,6 +87,7 @@ fn make_snapshot( parallelism: None, respond_to: None, respond_to_allowlist: vec![], + reply_placement: None, name_pool: vec![], idle_timeout_seconds: None, max_turn_duration_seconds: None, diff --git a/desktop/src-tauri/src/commands/personas/snapshot/import.rs b/desktop/src-tauri/src/commands/personas/snapshot/import.rs index d7f0323304..19146b71ee 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot/import.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot/import.rs @@ -25,6 +25,12 @@ use crate::{ util::now_iso, }; +#[path = "behavior.rs"] +mod behavior; +#[cfg(test)] +pub(crate) use behavior::resolve_snapshot_import_behavior; +pub(crate) use behavior::resolve_snapshot_import_behavior_with_reply; + /// Maximum snapshot file size accepted before decode (5 MiB for JSON, /// 10 MiB for PNG). Mirrors the established persona-import limits. pub(crate) const MAX_SNAPSHOT_JSON_BYTES: usize = 5 * 1024 * 1024; @@ -122,94 +128,6 @@ pub struct AgentSnapshotImportResult { // ── Import helpers ───────────────────────────────────────────────────────── -/// Resolve the behavioral defaults for an incoming agent snapshot. -/// -/// This is the single authoritative selection path for all import-time -/// allowlist and behavioral decisions. It is extracted as a pure, testable -/// function so that unit tests exercise the exact production logic rather -/// than a reconstruction of it. -/// -/// # UI contract -/// -/// The Keep/Clear toggle is shown whenever `has_source_allowlist` is true -/// (i.e. the raw allowlist is non-empty), regardless of the source mode. -/// The mode (`respond_to` wire string) and the list are independent axes. -/// -/// # Decision table -/// -/// | Source mode | Non-empty list | keep=true | keep=false | -/// |--------------|----------------|----------------------|-------------------------| -/// | allowlist | yes | preserve mode + list | owner-only + empty | -/// | allowlist | no | **Err** (reject) | **Err** (reject) | -/// | non-allowlist| yes | preserve mode + list | preserve mode + empty | -/// | non-allowlist| no | preserve mode | preserve mode | -/// -/// Allowlist-mode + empty list is always rejected: the UI showed no choice -/// and there is no coherent value to write. -/// -/// Non-allowlist + non-empty + Clear: preserve the source mode but empty the -/// list. Only allowlist-mode requires a mode downgrade on Clear, because -/// `allowlist` without entries is an invalid state. Non-allowlist modes -/// remain valid with an empty list. -pub(crate) fn resolve_snapshot_import_behavior( - raw_respond_to: Option<&str>, - raw_allowlist: &[String], - parallelism: Option, - keep_allowlist: bool, -) -> Result { - use crate::managed_agents::{ - resolve_mint_behavioral_defaults, validate_respond_to_allowlist, RespondTo, - }; - - // Step 1: normalize allowlist; reject malformed pubkeys immediately. - let normalized_allowlist = validate_respond_to_allowlist(raw_allowlist)?; - - // Step 2: detect source mode and whether a list was present. - let source_mode: Option = match raw_respond_to { - Some(wire) => Some(RespondTo::parse_wire(wire)?), - None => None, - }; - let is_source_allowlist_mode = source_mode == Some(RespondTo::Allowlist); - let has_source_allowlist = !normalized_allowlist.is_empty(); - - // Step 3: hard-reject allowlist-mode + empty list before any key - // generation — no coherent value can be written either way. - if is_source_allowlist_mode && !has_source_allowlist { - return Err( - "snapshot respond-to mode is 'allowlist' but the allowlist is empty — \ - cannot import: no pubkeys to grant access to" - .to_string(), - ); - } - - // Step 4: apply Keep/Clear when the toggle was visible (list non-empty), - // or preserve the source mode when it was not. - let (resolved_mode, resolved_allowlist) = if has_source_allowlist { - if keep_allowlist { - // Keep: preserve source mode and validated list. - (source_mode, normalized_allowlist) - } else if is_source_allowlist_mode { - // Clear on allowlist-mode: must downgrade mode to owner-only because - // allowlist mode without entries is an invalid state. - (Some(RespondTo::OwnerOnly), Vec::new()) - } else { - // Clear on non-allowlist mode: preserve source mode, empty the list. - // Non-allowlist modes are valid without entries. - (source_mode, Vec::new()) - } - } else { - // No list present → toggle was never shown; preserve source mode as-is. - (source_mode, normalized_allowlist) - }; - - resolve_mint_behavioral_defaults( - resolved_mode, - resolved_allowlist, - parallelism, - None, // no definition record; all inputs are explicit from the snapshot - ) -} - const PNG_MAGIC: [u8; 4] = [0x89, 0x50, 0x4e, 0x47]; /// Decode a `buzz-agent-snapshot v1` manifest from raw bytes. @@ -479,10 +397,11 @@ pub async fn confirm_agent_snapshot_import( } // ── Resolve behavioral defaults ────────────────────────────────────────── - let minted = resolve_snapshot_import_behavior( + let minted = resolve_snapshot_import_behavior_with_reply( snapshot.definition.respond_to.as_deref(), &snapshot.definition.respond_to_allowlist, snapshot.definition.parallelism, + snapshot.definition.reply_placement.as_deref(), input.keep_allowlist, )?; let minted_parallelism = minted.parallelism; @@ -582,6 +501,7 @@ pub async fn confirm_agent_snapshot_import( respond_to: respond_to_wire.clone(), respond_to_allowlist: minted.respond_to_allowlist.clone(), parallelism: minted_parallelism, + reply_placement: snapshot.definition.reply_placement.clone(), created_at: now.clone(), updated_at: now.clone(), }; @@ -642,6 +562,9 @@ pub async fn confirm_agent_snapshot_import( // are always consistent at mint time. respond_to: minted.respond_to, respond_to_allowlist: minted.respond_to_allowlist.clone(), + // The imported persona owns the portable default. Leave the + // instance override empty so the definition remains authoritative. + reply_placement: None, is_builtin: false, is_active: true, shared: false, @@ -651,6 +574,7 @@ pub async fn confirm_agent_snapshot_import( definition_respond_to: respond_to_wire.clone(), definition_respond_to_allowlist: minted.respond_to_allowlist.clone(), definition_parallelism: minted_parallelism, + definition_reply_placement: snapshot.definition.reply_placement.clone(), relay_mesh: None, runtime: snapshot.definition.runtime.clone(), name_pool: snapshot.definition.name_pool.clone(), diff --git a/desktop/src-tauri/src/commands/personas/snapshot/tests.rs b/desktop/src-tauri/src/commands/personas/snapshot/tests.rs index c453b09a9d..ce0a6bf260 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot/tests.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot/tests.rs @@ -61,6 +61,7 @@ fn make_definition(slug: &str) -> ManagedAgentRecord { last_error_code: None, respond_to: RespondTo::default(), respond_to_allowlist: vec![], + reply_placement: None, runtime: None, name_pool: vec![], is_builtin: false, @@ -72,6 +73,7 @@ fn make_definition(slug: &str) -> ManagedAgentRecord { definition_respond_to: None, definition_respond_to_allowlist: vec![], definition_parallelism: None, + definition_reply_placement: None, relay_mesh: None, } } @@ -105,6 +107,7 @@ fn make_snapshot( parallelism: None, respond_to: None, respond_to_allowlist: vec![], + reply_placement: None, name_pool: vec![], idle_timeout_seconds: None, max_turn_duration_seconds: None, diff --git a/desktop/src-tauri/src/commands/personas/update/name_propagation_tests.rs b/desktop/src-tauri/src/commands/personas/update/name_propagation_tests.rs index c60215ae4d..11df5bd70d 100644 --- a/desktop/src-tauri/src/commands/personas/update/name_propagation_tests.rs +++ b/desktop/src-tauri/src/commands/personas/update/name_propagation_tests.rs @@ -44,6 +44,7 @@ fn agent(persona_id: &str, name: &str, display_name: Option<&str>) -> ManagedAge last_error_code: None, respond_to: Default::default(), respond_to_allowlist: vec![], + reply_placement: None, display_name: display_name.map(str::to_string), slug: None, runtime: None, @@ -57,6 +58,7 @@ fn agent(persona_id: &str, name: &str, display_name: Option<&str>) -> ManagedAge definition_respond_to: None, definition_respond_to_allowlist: vec![], definition_parallelism: None, + definition_reply_placement: None, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/commands/team_snapshot.rs b/desktop/src-tauri/src/commands/team_snapshot.rs index 97cd11933d..2e25ca5e5d 100644 --- a/desktop/src-tauri/src/commands/team_snapshot.rs +++ b/desktop/src-tauri/src/commands/team_snapshot.rs @@ -10,7 +10,9 @@ use uuid::Uuid; use crate::{ app_state::AppState, - commands::{export_util::save_bytes_with_dialog, personas::resolve_snapshot_import_behavior}, + commands::{ + export_util::save_bytes_with_dialog, personas::resolve_snapshot_import_behavior_with_reply, + }, managed_agents::team_snapshot::{ build_team_snapshot, decode_team_snapshot_json, decode_team_snapshot_png, encode_team_snapshot_json, encode_team_snapshot_png, TeamSnapshot, @@ -109,10 +111,11 @@ fn definition_from_snapshot( keep_allowlist: bool, now: &str, ) -> Result { - let behavior = resolve_snapshot_import_behavior( + let behavior = resolve_snapshot_import_behavior_with_reply( member.definition.respond_to.as_deref(), &member.definition.respond_to_allowlist, member.definition.parallelism, + member.definition.reply_placement.as_deref(), keep_allowlist, )?; let respond_to = (behavior.respond_to != crate::managed_agents::RespondTo::default()) @@ -137,6 +140,9 @@ fn definition_from_snapshot( respond_to, respond_to_allowlist: behavior.respond_to_allowlist, parallelism: behavior.parallelism, + reply_placement: (behavior.reply_placement + != crate::managed_agents::ReplyPlacement::default()) + .then(|| behavior.reply_placement.as_str().to_string()), created_at: now.to_string(), updated_at: now.to_string(), }) @@ -599,6 +605,7 @@ pub async fn confirm_team_snapshot_import( .unwrap_or_default() }, respond_to_allowlist: definition.respond_to_allowlist.clone(), + reply_placement: None, is_builtin: false, is_active: true, shared: false, @@ -608,6 +615,7 @@ pub async fn confirm_team_snapshot_import( definition_respond_to: respond_to_wire.clone(), definition_respond_to_allowlist: definition.respond_to_allowlist.clone(), definition_parallelism: minted_parallelism, + definition_reply_placement: definition.reply_placement.clone(), relay_mesh: None, runtime: member.definition.runtime.clone(), name_pool: member.definition.name_pool.clone(), diff --git a/desktop/src-tauri/src/commands/team_snapshot/tests.rs b/desktop/src-tauri/src/commands/team_snapshot/tests.rs index c9a6d8812a..cfef4c4ab4 100644 --- a/desktop/src-tauri/src/commands/team_snapshot/tests.rs +++ b/desktop/src-tauri/src/commands/team_snapshot/tests.rs @@ -21,6 +21,7 @@ fn member(name: &str) -> AgentSnapshot { parallelism: Some(2), respond_to: Some("allowlist".to_string()), respond_to_allowlist: vec!["ab".repeat(32)], + reply_placement: None, name_pool: vec![], idle_timeout_seconds: None, max_turn_duration_seconds: None, @@ -73,6 +74,7 @@ fn team_export_round_trip_preserves_team_and_excludes_member_memory() { respond_to: None, respond_to_allowlist: vec![], parallelism: None, + reply_placement: None, created_at: "now".to_string(), updated_at: "now".to_string(), }, @@ -95,6 +97,7 @@ fn team_export_round_trip_preserves_team_and_excludes_member_memory() { respond_to: None, respond_to_allowlist: vec![], parallelism: None, + reply_placement: None, created_at: "now".to_string(), updated_at: "now".to_string(), }, @@ -158,6 +161,7 @@ fn team_export_with_instance_and_memory_level_uses_supplied_entries() { respond_to: None, respond_to_allowlist: vec![], parallelism: None, + reply_placement: None, created_at: "now".to_string(), updated_at: "now".to_string(), }]; @@ -219,6 +223,7 @@ fn team_export_with_instance_and_memory_level_uses_supplied_entries() { last_error_code: None, respond_to: crate::managed_agents::RespondTo::default(), respond_to_allowlist: vec![], + reply_placement: None, is_builtin: false, is_active: true, shared: false, @@ -228,6 +233,7 @@ fn team_export_with_instance_and_memory_level_uses_supplied_entries() { definition_respond_to: None, definition_respond_to_allowlist: vec![], definition_parallelism: None, + definition_reply_placement: None, relay_mesh: None, runtime: None, name_pool: vec![], diff --git a/desktop/src-tauri/src/managed_agents/agent_events.rs b/desktop/src-tauri/src/managed_agents/agent_events.rs index 4a7b80079d..ed9ffff39c 100644 --- a/desktop/src-tauri/src/managed_agents/agent_events.rs +++ b/desktop/src-tauri/src/managed_agents/agent_events.rs @@ -26,7 +26,7 @@ use buzz_core_pkg::kind::KIND_MANAGED_AGENT; use nostr::{EventBuilder, Kind, Tag}; use serde::{Deserialize, Serialize}; -use super::{ManagedAgentRecord, RespondTo}; +use super::{ManagedAgentRecord, ReplyPlacement, RespondTo}; /// The JSON body stored in a managed-agent event's content field. /// @@ -57,6 +57,10 @@ pub struct ManagedAgentEventContent { /// public keys, not secrets. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub respond_to_allowlist: Vec, + /// Explicit instance override. Omitted means the linked definition or + /// global default supplies the effective mode. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub reply_placement: Option, } /// Project a `ManagedAgentRecord` onto the content fields published in @@ -103,6 +107,7 @@ pub fn agent_event_content(record: &ManagedAgentRecord) -> ManagedAgentEventCont parallelism: record.parallelism, respond_to: record.respond_to, respond_to_allowlist: record.respond_to_allowlist.clone(), + reply_placement: record.reply_placement, } } @@ -200,6 +205,7 @@ mod tests { last_error_code: None, respond_to: RespondTo::Allowlist, respond_to_allowlist: vec!["79be667e".to_string()], + reply_placement: None, // Unified-model fields carry real values so the exclusion test // proves they are absent from the wire, not vacuously empty. display_name: Some("Display Name Secretish".to_string()), @@ -215,6 +221,7 @@ mod tests { definition_respond_to: None, definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, + definition_reply_placement: None, relay_mesh: None, } } @@ -338,6 +345,16 @@ mod tests { assert_eq!(a, b); } + #[test] + fn projection_persists_explicit_reply_placement_override() { + let mut agent = sample_agent(); + agent.reply_placement = Some(ReplyPlacement::TopLevel); + let content = agent_event_content(&agent); + assert_eq!(content.reply_placement, Some(ReplyPlacement::TopLevel)); + let json = serde_json::to_string(&content).unwrap(); + assert!(json.contains("\"reply_placement\":\"top-level\"")); + } + /// Mutating only runtime fields must NOT change the projection — the /// guarantee that operational start/stop never republishes. #[test] diff --git a/desktop/src-tauri/src/managed_agents/agent_snapshot.rs b/desktop/src-tauri/src/managed_agents/agent_snapshot.rs index 7c08e7095f..106897ac9b 100644 --- a/desktop/src-tauri/src/managed_agents/agent_snapshot.rs +++ b/desktop/src-tauri/src/managed_agents/agent_snapshot.rs @@ -111,6 +111,9 @@ pub struct AgentSnapshotDefinition { /// source environment and are meaningless on the importer's relay. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub respond_to_allowlist: Vec, + /// Definition-level reply-placement default in harness wire form. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub reply_placement: Option, #[serde(default, skip_serializing_if = "Vec::is_empty")] pub name_pool: Vec, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -207,6 +210,10 @@ pub fn build_snapshot( parallelism: record.definition_parallelism.or(Some(record.parallelism)), respond_to: record.definition_respond_to.clone(), respond_to_allowlist: record.definition_respond_to_allowlist.clone(), + reply_placement: record + .definition_reply_placement + .clone() + .or_else(|| record.reply_placement.map(|mode| mode.as_str().to_string())), name_pool: record.name_pool.clone(), idle_timeout_seconds: record.idle_timeout_seconds, max_turn_duration_seconds: record.max_turn_duration_seconds, diff --git a/desktop/src-tauri/src/managed_agents/agent_snapshot_envelope.rs b/desktop/src-tauri/src/managed_agents/agent_snapshot_envelope.rs index 8508c27073..657b13d4f1 100644 --- a/desktop/src-tauri/src/managed_agents/agent_snapshot_envelope.rs +++ b/desktop/src-tauri/src/managed_agents/agent_snapshot_envelope.rs @@ -340,6 +340,7 @@ mod tests { parallelism: Some(1), respond_to: None, respond_to_allowlist: Vec::new(), + reply_placement: None, name_pool: Vec::new(), idle_timeout_seconds: None, max_turn_duration_seconds: None, @@ -402,6 +403,7 @@ mod tests { last_error_code: None, respond_to: crate::managed_agents::types::RespondTo::OwnerOnly, respond_to_allowlist: vec![], + reply_placement: None, display_name: None, slug: None, runtime: None, @@ -415,6 +417,7 @@ mod tests { definition_respond_to: None, definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, + definition_reply_placement: None, relay_mesh: None, agent_command_override: None, persona_source_version: None, diff --git a/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs b/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs index b4492418e5..8a1ae074a8 100644 --- a/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs +++ b/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs @@ -59,6 +59,7 @@ fn minimal_record() -> ManagedAgentRecord { last_error_code: Some(42), // MUST NOT appear respond_to: RespondTo::default(), respond_to_allowlist: vec!["pubkey1hex".to_string()], + reply_placement: None, slug: Some("test-agent".to_string()), runtime: Some("goose".to_string()), name_pool: vec!["Alice".to_string(), "Bob".to_string()], @@ -71,6 +72,7 @@ fn minimal_record() -> ManagedAgentRecord { catalog_source: None, definition_respond_to_allowlist: vec!["abc123def".to_string()], definition_parallelism: Some(4), + definition_reply_placement: None, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/managed_agents/backend.rs b/desktop/src-tauri/src/managed_agents/backend.rs index 5debae41cb..0a72338cfd 100644 --- a/desktop/src-tauri/src/managed_agents/backend.rs +++ b/desktop/src-tauri/src/managed_agents/backend.rs @@ -333,23 +333,36 @@ pub(crate) fn redact_secrets_with(s: &str, extras: &[&str]) -> String { result } -/// Collect string values from `request["agent"]["env_vars"]` (if present) -/// to feed into [`redact_secrets_with`]. Returns an empty Vec if the -/// request shape doesn't match, which is fine — falls back to the default -/// prefix-based scrubbing. +/// Collect non-empty string values from every provider-request environment +/// map that can carry a user or desktop-resolved value. Providers may echo +/// any of these maps in stderr or a structured error response, so all three +/// must feed the literal-value redactor: +/// `agent.env_vars`, `agent.launch.env`, and `agent.launch.policy_env`. +/// Missing, null, malformed, or non-object maps are ignored safely. +fn append_env_string_values(value: Option<&serde_json::Value>, secrets: &mut Vec) { + let Some(obj) = value.and_then(|value| value.as_object()) else { + return; + }; + secrets.extend( + obj.values() + .filter_map(|value| value.as_str()) + .filter(|value| !value.is_empty()) + .map(String::from), + ); +} + fn env_secrets_from_request(request: &serde_json::Value) -> Vec { - request - .get("agent") - .and_then(|a| a.get("env_vars")) - .and_then(|e| e.as_object()) - .map(|obj| { - obj.values() - .filter_map(|v| v.as_str()) - .filter(|s| !s.is_empty()) - .map(String::from) - .collect() - }) - .unwrap_or_default() + let Some(agent) = request.get("agent").and_then(|value| value.as_object()) else { + return Vec::new(); + }; + + let mut secrets = Vec::new(); + append_env_string_values(agent.get("env_vars"), &mut secrets); + if let Some(launch) = agent.get("launch").and_then(|value| value.as_object()) { + append_env_string_values(launch.get("env"), &mut secrets); + append_env_string_values(launch.get("policy_env"), &mut secrets); + } + secrets } /// Public-in-crate helper: redact every non-empty value from `env` (plus @@ -632,12 +645,22 @@ mod tests { "EMPTY": "", "NUMERIC": 42, }, + "launch": { + "env": { + "LAUNCH_SECRET": "launch-secret-value" + }, + "policy_env": { + "POLICY_SECRET": "policy-secret-value" + } + } }, }); let secrets = env_secrets_from_request(&req); assert!(secrets.iter().any(|v| v == "sk-ant-test")); - // Empty and non-string values are filtered out. - assert_eq!(secrets.len(), 1); + assert!(secrets.iter().any(|v| v == "launch-secret-value")); + assert!(secrets.iter().any(|v| v == "policy-secret-value")); + // Empty and non-string values are filtered out across all three maps. + assert_eq!(secrets.len(), 3); } #[test] @@ -647,6 +670,75 @@ mod tests { assert!( env_secrets_from_request(&serde_json::json!({"agent": {"env_vars": null}})).is_empty() ); + assert!(env_secrets_from_request(&serde_json::json!({ + "agent": {"launch": {"env": [], "policy_env": "not-an-object"}} + })) + .is_empty()); + } + + #[cfg(unix)] + fn write_provider_script(body: &str) -> (tempfile::TempDir, std::path::PathBuf) { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().expect("provider temp directory"); + let path = dir.path().join("fake-provider"); + std::fs::write(&path, format!("#!/bin/sh\nset -eu\n{body}\n")).expect("provider script"); + let mut permissions = std::fs::metadata(&path) + .expect("provider metadata") + .permissions(); + permissions.set_mode(0o700); + std::fs::set_permissions(&path, permissions).expect("provider executable"); + (dir, path) + } + + #[cfg(unix)] + fn request_with_launch_secrets() -> serde_json::Value { + serde_json::json!({ + "op": "deploy", + "agent": { + "env_vars": {"LEGACY_SECRET": "legacy-secret-value"}, + "launch": { + "env": {"LAUNCH_SECRET": "launch-secret-value"}, + "policy_env": {"POLICY_SECRET": "policy-secret-value"} + } + } + }) + } + + #[cfg(unix)] + #[test] + fn invoke_provider_redacts_launch_only_secret_from_stderr() { + let secret = "launch-secret-value"; + let (_dir, provider) = write_provider_script(&format!( + "printf 'provider failure: {secret}\\n' >&2\nexit 1" + )); + let error = invoke_provider( + &provider, + &request_with_launch_secrets(), + Duration::from_secs(5), + ) + .expect_err("provider stderr failure must surface as an error"); + + assert!(!error.contains(secret), "launch secret leaked: {error}"); + assert!(error.contains("[REDACTED]"), "missing redaction: {error}"); + } + + #[cfg(unix)] + #[test] + fn invoke_provider_redacts_launch_only_secret_from_structured_error() { + let secret = "policy-secret-value"; + let (_dir, provider) = write_provider_script(&format!( + "printf '%s\\n' '{{\"ok\":false,\"error\":\"provider echoed {secret}\"}}'" + )); + let error = invoke_provider( + &provider, + &request_with_launch_secrets(), + Duration::from_secs(5), + ) + .expect_err("structured provider error must surface as an error"); + + assert!(!error.contains(secret), "policy secret leaked: {error}"); + assert!(error.contains("[REDACTED]"), "missing redaction: {error}"); } #[test] diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs index 4ee4ec79c3..1b3ce2b7f3 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs @@ -100,6 +100,7 @@ fn test_record() -> ManagedAgentRecord { last_error_code: None, respond_to: crate::managed_agents::types::RespondTo::OwnerOnly, respond_to_allowlist: vec![], + reply_placement: None, display_name: None, slug: None, runtime: None, @@ -113,6 +114,7 @@ fn test_record() -> ManagedAgentRecord { definition_respond_to: None, definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, + definition_reply_placement: None, relay_mesh: None, agent_command_override: None, persona_source_version: None, diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests.rs b/desktop/src-tauri/src/managed_agents/discovery/tests.rs index 6fe6a77521..3a3e69a268 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -207,11 +207,11 @@ fn persona_with_runtime(id: &str, runtime: Option<&str>) -> crate::managed_agent respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, created_at: "2026-06-09T00:00:00Z".to_string(), updated_at: "2026-06-09T00:00:00Z".to_string(), } } - #[test] fn effective_agent_command_explicit_override_wins() { // An explicit pin beats the persona's runtime. @@ -221,7 +221,6 @@ fn effective_agent_command_explicit_override_wins() { "codex-acp" ); } - /// Minimal record for `record_agent_command` tests. Only the resolution /// inputs (runtime / persona_id / agent_command_override) vary. fn record_with( @@ -269,6 +268,7 @@ fn record_with( last_error_code: None, respond_to: Default::default(), respond_to_allowlist: vec![], + reply_placement: None, display_name: None, slug: None, runtime: runtime.map(str::to_string), @@ -282,10 +282,10 @@ fn record_with( definition_respond_to: None, definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, + definition_reply_placement: None, relay_mesh: None, } } - #[test] fn record_agent_command_own_runtime_wins_over_persona() { // A record with its own materialized runtime never consults the diff --git a/desktop/src-tauri/src/managed_agents/effective_config/tests.rs b/desktop/src-tauri/src/managed_agents/effective_config/tests.rs index c8e437809c..5232ff3873 100644 --- a/desktop/src-tauri/src/managed_agents/effective_config/tests.rs +++ b/desktop/src-tauri/src/managed_agents/effective_config/tests.rs @@ -26,6 +26,7 @@ fn definition( respond_to: None, respond_to_allowlist: vec![], parallelism: None, + reply_placement: None, created_at: "".to_string(), updated_at: "".to_string(), } @@ -77,6 +78,7 @@ fn record( last_error_code: None, respond_to: RespondTo::OwnerOnly, respond_to_allowlist: vec![], + reply_placement: None, display_name: None, slug: None, runtime: None, @@ -92,6 +94,7 @@ fn record( definition_respond_to: None, definition_respond_to_allowlist: vec![], definition_parallelism: None, + definition_reply_placement: None, } } diff --git a/desktop/src-tauri/src/managed_agents/env_vars.rs b/desktop/src-tauri/src/managed_agents/env_vars.rs index 592a5cbbd9..2fa228e382 100644 --- a/desktop/src-tauri/src/managed_agents/env_vars.rs +++ b/desktop/src-tauri/src/managed_agents/env_vars.rs @@ -51,10 +51,11 @@ pub(crate) fn is_derived_provider_model_key(key: &str) -> bool { /// shows owner-only while the running agent answers anyone, for /// example), or redirect the agent to an attacker-controlled relay. /// -/// This list is deliberately narrow — it only covers keys with security -/// implications. Behavior knobs (GOOSE_MODE, BUZZ_ACP_MODEL, BUZZ_ACP_SYSTEM_PROMPT, …) remain freely -/// overridable; those have dedicated UI fields but power users may want -/// to bypass them. +/// This list is deliberately narrow — it covers keys with identity, code +/// execution, or security-gate implications. Ordinary behavior knobs +/// (GOOSE_MODE, BUZZ_ACP_MODEL, BUZZ_ACP_SYSTEM_PROMPT, …) remain freely +/// overridable; reply placement is reserved because it controls the +/// protocol-level destination of human-facing replies. pub(crate) const RESERVED_ENV_KEYS: &[&str] = &[ // Identity / secrets. "BUZZ_PRIVATE_KEY", @@ -77,6 +78,7 @@ pub(crate) const RESERVED_ENV_KEYS: &[&str] = &[ "BUZZ_ACP_RESPOND_TO", "BUZZ_ACP_RESPOND_TO_ALLOWLIST", "BUZZ_ACP_AGENT_OWNER", + "BUZZ_ACP_REPLY_PLACEMENT", // Readiness handoff: desktop is the ONLY readiness source. A saved or // ambient env var must not be able to forge setup mode (NotReady) on a // Ready agent or suppress it (empty/stale payload) on a NotReady one. diff --git a/desktop/src-tauri/src/managed_agents/env_vars/tests.rs b/desktop/src-tauri/src/managed_agents/env_vars/tests.rs index cf57b12546..1300d2e0f3 100644 --- a/desktop/src-tauri/src/managed_agents/env_vars/tests.rs +++ b/desktop/src-tauri/src/managed_agents/env_vars/tests.rs @@ -158,6 +158,16 @@ fn reserved_keys_include_respond_to_gate() { } } +#[test] +fn reply_placement_is_reserved_and_cannot_be_user_overridden() { + assert!(is_reserved_env_key("BUZZ_ACP_REPLY_PLACEMENT")); + let agent = map(&[("BUZZ_ACP_REPLY_PLACEMENT", "top-level")]); + let merged = merged_user_env(&BTreeMap::new(), &agent); + assert!(merged.is_empty()); + let err = validate_user_env_keys(&agent).expect_err("reply placement is desktop-owned"); + assert!(err.contains("BUZZ_ACP_REPLY_PLACEMENT"), "got: {err}"); +} + #[test] fn reserved_keys_include_code_execution_surface() { // The agent/MCP command + args are what Buzz actually exec's. diff --git a/desktop/src-tauri/src/managed_agents/global_config/mod.rs b/desktop/src-tauri/src/managed_agents/global_config/mod.rs index 162f447981..40c768e3e4 100644 --- a/desktop/src-tauri/src/managed_agents/global_config/mod.rs +++ b/desktop/src-tauri/src/managed_agents/global_config/mod.rs @@ -31,7 +31,7 @@ use crate::managed_agents::env_vars::{ validate_user_env_keys, DERIVED_PROVIDER_MODEL_ENV_KEYS, MAX_ENV_VALUE_BYTES, }; use crate::managed_agents::storage::{atomic_write_json_restricted, managed_agents_base_dir}; -use crate::managed_agents::types::{AgentDefinition, ManagedAgentRecord}; +use crate::managed_agents::types::{AgentDefinition, ManagedAgentRecord, ReplyPlacement}; /// The global agent configuration record. /// @@ -70,6 +70,12 @@ pub struct GlobalAgentConfig { /// Preferred ACP runtime for definitions without an explicit runtime. #[serde(default)] pub preferred_runtime: Option, + + /// Global fallback for ordinary human-facing reply placement. A managed + /// agent or persona may override this value; `None` preserves the + /// historical `thread` behavior. + #[serde(default)] + pub reply_placement: Option, } /// Validate a `GlobalAgentConfig` before persisting it. diff --git a/desktop/src-tauri/src/managed_agents/global_config/tests.rs b/desktop/src-tauri/src/managed_agents/global_config/tests.rs index 553596e226..4a0283ce61 100644 --- a/desktop/src-tauri/src/managed_agents/global_config/tests.rs +++ b/desktop/src-tauri/src/managed_agents/global_config/tests.rs @@ -4,7 +4,9 @@ use super::{ normalize_global_config_fields, resolve_effective_model_provider, strip_empty_env_vars, validate_global_config, GlobalAgentConfig, }; -use crate::managed_agents::{AgentDefinition, BackendKind, ManagedAgentRecord, RespondTo}; +use crate::managed_agents::{ + AgentDefinition, BackendKind, ManagedAgentRecord, ReplyPlacement, RespondTo, +}; fn config_with_env(pairs: &[(&str, &str)]) -> GlobalAgentConfig { GlobalAgentConfig { @@ -267,6 +269,7 @@ fn roundtrip_serialization() { provider: Some("anthropic".to_string()), model: Some("claude-opus-4".to_string()), preferred_runtime: Some("claude".to_string()), + reply_placement: Some(ReplyPlacement::FollowScope), }; let json = serde_json::to_string(&config).expect("serialize"); let back: GlobalAgentConfig = serde_json::from_str(&json).expect("deserialize"); @@ -293,6 +296,10 @@ fn default_global_config_serializes_all_fields() { json.contains("\"model\""), "serialized JSON must always include model; got: {json}" ); + assert!( + json.contains("\"reply_placement\""), + "serialized JSON must always include reply_placement; got: {json}" + ); } // ── resolve_effective_model_provider ───────────────────────────────────────── @@ -337,6 +344,7 @@ fn bare_record() -> ManagedAgentRecord { last_error_code: None, respond_to: RespondTo::OwnerOnly, respond_to_allowlist: vec![], + reply_placement: None, display_name: None, slug: None, runtime: None, @@ -352,6 +360,7 @@ fn bare_record() -> ManagedAgentRecord { definition_respond_to: None, definition_respond_to_allowlist: vec![], definition_parallelism: None, + definition_reply_placement: None, } } @@ -375,6 +384,7 @@ fn persona(id: &str, model: Option<&str>, provider: Option<&str>) -> AgentDefini respond_to: None, respond_to_allowlist: vec![], parallelism: None, + reply_placement: None, created_at: "".to_string(), updated_at: "".to_string(), } @@ -592,6 +602,7 @@ fn populated_global_config_round_trips() { provider: Some("anthropic".to_string()), model: Some("claude-opus-4-5".to_string()), preferred_runtime: None, + reply_placement: Some(ReplyPlacement::TopLevel), }; let json = serde_json::to_string(&original).expect("serialization must not fail"); let decoded: GlobalAgentConfig = @@ -636,6 +647,7 @@ fn record_runtime_wins_over_persona_runtime_for_command_resolution() { respond_to: None, respond_to_allowlist: vec![], parallelism: None, + reply_placement: None, created_at: "".to_string(), updated_at: "".to_string(), }; diff --git a/desktop/src-tauri/src/managed_agents/mod.rs b/desktop/src-tauri/src/managed_agents/mod.rs index 772d707f27..71a4003110 100644 --- a/desktop/src-tauri/src/managed_agents/mod.rs +++ b/desktop/src-tauri/src/managed_agents/mod.rs @@ -64,7 +64,7 @@ pub use personas::*; pub use process_lifecycle::*; pub(crate) use readiness::{ agent_readiness, resolve_effective_agent_env, resolve_effective_harness_descriptor, - AgentReadiness, Requirement, + AgentReadiness, EffectiveHarnessDescriptor, Requirement, }; pub use relay_mesh::*; pub use repos::{ diff --git a/desktop/src-tauri/src/managed_agents/nest/tests.rs b/desktop/src-tauri/src/managed_agents/nest/tests.rs index 031b049a49..fd28524570 100644 --- a/desktop/src-tauri/src/managed_agents/nest/tests.rs +++ b/desktop/src-tauri/src/managed_agents/nest/tests.rs @@ -442,6 +442,7 @@ fn make_persona(id: &str, display_name: &str) -> AgentDefinition { respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, created_at: String::new(), updated_at: String::new(), } @@ -487,6 +488,7 @@ fn make_agent(name: &str, persona_id: Option<&str>) -> ManagedAgentRecord { last_error_code: None, respond_to: RespondTo::default(), respond_to_allowlist: vec![], + reply_placement: None, env_vars: std::collections::BTreeMap::new(), display_name: None, slug: None, @@ -501,6 +503,7 @@ fn make_agent(name: &str, persona_id: Option<&str>) -> ManagedAgentRecord { definition_respond_to: None, definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, + definition_reply_placement: None, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/managed_agents/persona_events.rs b/desktop/src-tauri/src/managed_agents/persona_events.rs index 6afc18a501..235f89e024 100644 --- a/desktop/src-tauri/src/managed_agents/persona_events.rs +++ b/desktop/src-tauri/src/managed_agents/persona_events.rs @@ -48,6 +48,9 @@ pub struct PersonaEventContent { pub respond_to_allowlist: Vec, #[serde(default, skip_serializing_if = "Option::is_none")] pub parallelism: Option, + /// Definition-level reply-placement default in harness wire form. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub reply_placement: Option, } /// Derive the d-tag (persona slug) from a `AgentDefinition`. @@ -200,6 +203,7 @@ pub fn persona_from_event(event: &nostr::Event) -> Result PersonaEventContent { respond_to: record.respond_to.clone(), respond_to_allowlist: record.respond_to_allowlist.clone(), parallelism: record.parallelism, + reply_placement: record.reply_placement.clone(), } } diff --git a/desktop/src-tauri/src/managed_agents/persona_events/tests.rs b/desktop/src-tauri/src/managed_agents/persona_events/tests.rs index b9542f9a87..cbd6bba2de 100644 --- a/desktop/src-tauri/src/managed_agents/persona_events/tests.rs +++ b/desktop/src-tauri/src/managed_agents/persona_events/tests.rs @@ -44,6 +44,7 @@ fn sample_record() -> ManagedAgentRecord { last_error_code: None, respond_to: RespondTo::OwnerOnly, respond_to_allowlist: vec![], + reply_placement: None, display_name: None, slug: None, runtime: None, @@ -57,6 +58,7 @@ fn sample_record() -> ManagedAgentRecord { definition_respond_to: None, definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, + definition_reply_placement: None, relay_mesh: None, } } @@ -159,6 +161,7 @@ fn sample_persona() -> AgentDefinition { respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, created_at: "2025-01-01T00:00:00Z".to_string(), updated_at: "2025-01-01T00:00:00Z".to_string(), } @@ -325,6 +328,7 @@ fn content_matches_nip_ap_vector() { respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, }; assert_eq!( serde_json::to_string(&content).unwrap(), @@ -386,6 +390,7 @@ fn content_matches_nip_ap_vector() { respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, created_at: "2025-01-01T00:00:00Z".to_string(), updated_at: "2025-01-01T00:00:00Z".to_string(), }; @@ -417,6 +422,7 @@ fn round_trip_minimal_persona() { respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, created_at: "2025-01-01T00:00:00Z".to_string(), updated_at: "2025-01-01T00:00:00Z".to_string(), }; @@ -514,6 +520,7 @@ fn quad_absent_definition_hash_stable_across_activation() { respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, created_at: "2026-01-01T00:00:00Z".to_string(), updated_at: "2026-01-01T00:00:00Z".to_string(), }; @@ -558,6 +565,7 @@ fn persona_from_event_content_for_test(content: PersonaEventContent) -> AgentDef respond_to: content.respond_to, respond_to_allowlist: content.respond_to_allowlist, parallelism: content.parallelism, + reply_placement: content.reply_placement, created_at: "2026-01-01T00:00:00Z".to_string(), updated_at: "2026-01-01T00:00:00Z".to_string(), } @@ -576,6 +584,7 @@ fn persona_content_hash_is_deterministic() { respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, }; let hash1 = persona_content_hash(&content); let hash2 = persona_content_hash(&content); @@ -596,6 +605,7 @@ fn persona_content_hash_changes_on_edit() { respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, }; let mut content2 = content1.clone(); content2.system_prompt = Some("Goodbye".to_string()); diff --git a/desktop/src-tauri/src/managed_agents/personas.rs b/desktop/src-tauri/src/managed_agents/personas.rs index 9bf7ab74b0..c1e286d4b5 100644 --- a/desktop/src-tauri/src/managed_agents/personas.rs +++ b/desktop/src-tauri/src/managed_agents/personas.rs @@ -129,6 +129,7 @@ fn built_in_persona_records(now: &str) -> Vec { respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, created_at: now.to_string(), updated_at: now.to_string(), }) diff --git a/desktop/src-tauri/src/managed_agents/personas/tests.rs b/desktop/src-tauri/src/managed_agents/personas/tests.rs index 387b4d72c6..8d422416e1 100644 --- a/desktop/src-tauri/src/managed_agents/personas/tests.rs +++ b/desktop/src-tauri/src/managed_agents/personas/tests.rs @@ -26,6 +26,7 @@ fn custom_persona(id: &str, display_name: &str) -> AgentDefinition { respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, created_at: "2026-03-19T00:00:00Z".to_string(), updated_at: "2026-03-19T00:00:00Z".to_string(), } diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index fa8eb36fa1..690e6eeda8 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -1518,6 +1518,7 @@ mod tests { last_error_code: None, respond_to: Default::default(), respond_to_allowlist: vec![], + reply_placement: None, display_name: None, slug: None, runtime: None, @@ -1531,9 +1532,9 @@ mod tests { definition_respond_to: None, definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, + definition_reply_placement: None, relay_mesh: None, }; - let runtime = known_acp_runtime_exact("buzz-agent"); let effective = resolve_effective_agent_env(&record, &[], runtime, &Default::default()); @@ -1547,7 +1548,6 @@ mod tests { Some("claude-opus-4-5") ); } - // ── provider-specific model fallback tests ──────────────────────────── #[test] diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 37927961ed..ea48144257 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; use tauri::AppHandle; @@ -194,15 +194,11 @@ pub fn build_managed_agent_summary( ) } }; - let (persona_out_of_date, persona_orphaned) = persona_drift_state(record, personas); - - let global_for_summary = - crate::managed_agents::load_global_agent_config(app).unwrap_or_default(); let effective_cfg = crate::managed_agents::effective_config::resolve_effective_config( record, personas, - &global_for_summary, + global_config, ); let (effective_model, effective_provider, effective_prompt, model_source) = match effective_cfg { @@ -243,7 +239,6 @@ pub fn build_managed_agent_summary( // Global config drives both the restart-drift hash and descriptor env // layering below — the caller loads it once and passes it in, so // list-style callers pay one disk read per call rather than one per record. - let needs_restart = pair_key .as_ref() .and_then(|key| runtimes.get(key).map(|runtime| (key, runtime))) @@ -338,6 +333,12 @@ pub fn build_managed_agent_summary( log_path, respond_to: record.respond_to, respond_to_allowlist: record.respond_to_allowlist.clone(), + reply_placement: super::types::resolve_effective_reply_placement( + record, + personas, + global_config.reply_placement, + )?, + reply_placement_override: record.reply_placement, }) } @@ -353,7 +354,6 @@ pub fn build_managed_agent_summary( fn restart_eligible(persona_orphaned: bool, hash_drift: bool, availability_drift: bool) -> bool { !persona_orphaned && (hash_drift || availability_drift) } - pub fn find_managed_agent_mut<'a>( records: &'a mut [ManagedAgentRecord], pubkey: &str, @@ -363,7 +363,6 @@ pub fn find_managed_agent_mut<'a>( .find(|record| record.pubkey == pubkey) .ok_or_else(|| format!("agent {pubkey} not found")) } - /// Pure decision function for the inbound author gate env vars. /// /// Returns the env vars to **set** and the env vars to **remove**. Removal is @@ -421,6 +420,85 @@ pub(crate) fn build_respond_to_env( Ok((set, remove)) } +/// Resolve the policy environment that a provider must apply before the +/// descriptor's layered user environment. +/// +/// Local spawn writes these values at several points because it also has to +/// account for the desktop process environment and local process ownership. +/// A remote provider has neither of those surfaces, so this helper produces a +/// pure, record-derived policy map. The provider then applies `launch.env` +/// over this map, preserving the local "user env wins" behavior; reply +/// placement is re-applied last by `deploy_payload_json` because it is the +/// one reserved policy key. +/// +/// `lazy` is deliberately an argument: local manual starts use the eager arm, +/// while provider-backed launches always pass `true` to avoid warming idle LLM +/// pools in a remote environment. +pub(crate) fn resolve_effective_launch_policy_env( + record: &ManagedAgentRecord, + effective_command: &str, + teams: &[super::types::TeamRecord], + effective_prompt: Option<&str>, + effective_model: Option<&str>, + reply_placement: super::types::ReplyPlacement, + lazy: bool, +) -> BTreeMap { + let mut policy = BTreeMap::new(); + + // Runtime defaults are resolved from the static runtime catalog rather + // than copied from the desktop's ambient process environment. A remote + // pod must not inherit a host-specific accident such as GOOSE_MODE. + if let Some(runtime) = known_acp_runtime(effective_command) { + for (key, value) in runtime.default_env { + policy.insert((*key).to_string(), (*value).to_string()); + } + if runtime.mcp_hooks { + policy.insert("MCP_HOOK_SERVERS".to_string(), "*".to_string()); + } + } + + policy.insert( + "BUZZ_ACP_LAZY_POOL".to_string(), + if lazy { "true" } else { "false" }.to_string(), + ); + policy.insert("BUZZ_ACP_RELAY_OBSERVER".to_string(), "true".to_string()); + + if let Some(prompt) = effective_prompt { + policy.insert("BUZZ_ACP_SYSTEM_PROMPT".to_string(), prompt.to_string()); + } + if let Some(model) = effective_model { + policy.insert("BUZZ_ACP_MODEL".to_string(), model.to_string()); + } + if let Some(idle) = record.idle_timeout_seconds { + policy.insert("BUZZ_ACP_IDLE_TIMEOUT".to_string(), idle.to_string()); + } + if let Some(max_duration) = record.max_turn_duration_seconds { + policy.insert( + "BUZZ_ACP_MAX_TURN_DURATION".to_string(), + max_duration.to_string(), + ); + } + policy.insert( + "BUZZ_ACP_AGENTS".to_string(), + record.parallelism.to_string(), + ); + + if let Some(title) = resolve_session_title(record.display_name.as_deref(), &record.name) { + policy.insert(SESSION_TITLE_ENV_VAR.to_string(), title); + } + if let Some(instructions) = super::spawn_hash::effective_team_instructions(record, teams) { + policy.insert("BUZZ_ACP_TEAM_INSTRUCTIONS".to_string(), instructions); + } + + // The serializer also enforces this key so callers cannot accidentally + // construct a provider payload with a stale or user-supplied mode. + policy.insert( + "BUZZ_ACP_REPLY_PLACEMENT".to_string(), + reply_placement.as_str().to_string(), + ); + policy +} + pub(crate) fn configure_runtime_cli( command: &mut std::process::Command, runtime: Option<&KnownAcpRuntime>, @@ -444,7 +522,6 @@ pub(crate) fn configure_runtime_cli( command.env("CLAUDE_CODE_EXECUTABLE", cli_path); } } - /// Spawn an agent process without holding any locks on records or runtimes. /// Returns the child process and log path on success. The caller is responsible /// for updating `ManagedAgentRecord` fields and inserting into the runtimes map. @@ -471,8 +548,9 @@ pub fn spawn_agent_child( let teams = super::load_teams(app).unwrap_or_default(); // Load global config once; used for runtime_metadata_env_vars (model/provider fallback) // and for the env-var merge at spawn time. - let global = crate::managed_agents::load_global_agent_config(app).unwrap_or_default(); - + // A malformed persisted reply-placement value must stop the spawn rather + // than silently reverting to the historical thread behavior. + let global = crate::managed_agents::load_global_agent_config(app)?; // Resolve model/provider/prompt ONCE, here, at the shared spawn boundary — // the single source both the env writes below and `spawn_config_hash` // read from. Previously prompt was read from the record's own (possibly @@ -488,7 +566,6 @@ pub fn spawn_agent_child( record, &personas, &global, ) .require_resolved()?; - // Single typed resolver: validates runtime id (dangling harness → Err), resolves // command, args (instance wins over definition default), and the full env layer stack. // This is the sole path for harness-definition lookup — spawn, hash, summary, and @@ -506,7 +583,6 @@ pub fn spawn_agent_child( })?; let effective_command = &descriptor.command; let agent_args = &descriptor.args; - let log_path = super::managed_agent_runtime_log_path(app, &runtime_key)?; append_log_marker( &log_path, @@ -517,7 +593,6 @@ pub fn spawn_agent_child( now_iso() ), )?; - let stdout = open_log_file(&log_path)?; let stderr = stdout .try_clone() @@ -799,7 +874,6 @@ pub fn spawn_agent_child( } else { command.env_remove("BUZZ_AUTH_TAG"); } - // Inbound author gate: who is this agent allowed to respond to? // Validation is strict here — a malformed allowlist on disk fails before // we spawn anything (the harness would also reject it, but we'd rather @@ -812,6 +886,14 @@ pub fn spawn_agent_child( command.env_remove(key); } + let reply_placement = + super::types::resolve_effective_reply_placement(record, &personas, global.reply_placement)?; + // This is a desktop-owned routing/security setting. It is set before the + // user-env layer below, and the descriptor's reserved-key filter keeps + // persona/agent env maps from overriding it. Provider serialization applies + // the same value again after launch.env, so the remote child receives the + // UI-visible value under both execution paths. + command.env("BUZZ_ACP_REPLY_PLACEMENT", reply_placement.as_str()); command.env("BUZZ_ACP_RELAY_OBSERVER", "true"); // ── Git credential helper for Buzz relay ────────────────────────── diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index 3f6ee996f6..86fd8f8b38 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -167,6 +167,7 @@ fn fixture( last_error_code: None, respond_to, respond_to_allowlist: allowlist, + reply_placement: None, display_name: None, slug: None, runtime: None, @@ -180,10 +181,10 @@ fn fixture( definition_respond_to: None, definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, + definition_reply_placement: None, relay_mesh: None, } } - #[test] fn build_env_owner_only_sets_mode_and_removes_others() { let rec = fixture(RespondTo::OwnerOnly, vec![], Some("tag".into())); @@ -198,7 +199,6 @@ fn build_env_owner_only_sets_mode_and_removes_others() { // auth_tag is present → no AGENT_OWNER fallback fires. assert!(remove.contains(&"BUZZ_ACP_AGENT_OWNER")); } - // select_untracked_bundle_harnesses tests live in runtime/sweep.rs (mod tests). #[test] @@ -302,11 +302,11 @@ fn persona_with_provider( respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, created_at: "2026-06-09T00:00:00Z".to_string(), updated_at: "2026-06-09T00:00:00Z".to_string(), } } - // ── persona env refresh acceptance ────────────────────────────────────── // // The refresh lifecycle Wes decided: `record.env_vars` holds agent-level diff --git a/desktop/src-tauri/src/managed_agents/spawn_hash.rs b/desktop/src-tauri/src/managed_agents/spawn_hash.rs index 648cc62bbe..c9c776e85b 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash.rs @@ -149,6 +149,14 @@ pub(crate) fn spawn_config_hash( .unwrap_or_else(|_| record.respond_to_allowlist.clone()) .hash(&mut hasher); } + // Hash the same effective reply placement the spawn path writes. Invalid + // imported persona values remain visible as drift instead of being + // silently treated as the historical default. + match super::types::resolve_effective_reply_placement(record, personas, global.reply_placement) + { + Ok(mode) => mode.as_str().hash(&mut hasher), + Err(error) => error.hash(&mut hasher), + } record.idle_timeout_seconds.hash(&mut hasher); record.max_turn_duration_seconds.hash(&mut hasher); record.parallelism.hash(&mut hasher); 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 f4ad404814..3afc606ec3 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs @@ -1,5 +1,5 @@ use super::*; -use crate::managed_agents::types::RespondTo; +use crate::managed_agents::types::{ReplyPlacement, RespondTo}; use std::collections::BTreeMap; fn record() -> ManagedAgentRecord { @@ -43,6 +43,7 @@ fn record() -> ManagedAgentRecord { last_error_code: None, respond_to: Default::default(), respond_to_allowlist: vec![], + reply_placement: None, display_name: None, slug: None, runtime: None, @@ -56,6 +57,7 @@ fn record() -> ManagedAgentRecord { definition_respond_to: None, definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, + definition_reply_placement: None, relay_mesh: None, } } @@ -80,6 +82,7 @@ fn persona(id: &str, runtime: Option<&str>, prompt: &str) -> AgentDefinition { respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, created_at: "now".into(), updated_at: "now".into(), } @@ -506,6 +509,25 @@ fn global_model_change_trips_hash_without_model_env_var() { ); } +#[test] +fn global_reply_placement_change_trips_hash() { + let rec = record(); + let global_thread = GlobalAgentConfig { + reply_placement: Some(ReplyPlacement::Thread), + ..Default::default() + }; + let global_follow_scope = GlobalAgentConfig { + reply_placement: Some(ReplyPlacement::FollowScope), + ..Default::default() + }; + + assert_ne!( + spawn_config_hash(&rec, &[], &[], "wss://ws.example", &global_thread,), + spawn_config_hash(&rec, &[], &[], "wss://ws.example", &global_follow_scope,), + "changing the global reply-placement default must trip the hash" + ); +} + #[test] fn linked_instance_stale_prompt_bytes_are_inert_at_hash_time() { // Regression for the split-resolve defect: prompt used to be read from diff --git a/desktop/src-tauri/src/managed_agents/team_snapshot.rs b/desktop/src-tauri/src/managed_agents/team_snapshot.rs index 96082acc76..2f8e901780 100644 --- a/desktop/src-tauri/src/managed_agents/team_snapshot.rs +++ b/desktop/src-tauri/src/managed_agents/team_snapshot.rs @@ -296,6 +296,7 @@ mod tests { last_error_code: None, respond_to: RespondTo::default(), respond_to_allowlist: vec![], + reply_placement: None, slug: Some(name.to_string()), runtime: Some("goose".to_string()), name_pool: vec![], @@ -308,6 +309,7 @@ mod tests { catalog_source: None, definition_respond_to_allowlist: vec![], definition_parallelism: None, + definition_reply_placement: None, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/managed_agents/teams_tests.rs b/desktop/src-tauri/src/managed_agents/teams_tests.rs index 1ffa60eda9..38cbd707e5 100644 --- a/desktop/src-tauri/src/managed_agents/teams_tests.rs +++ b/desktop/src-tauri/src/managed_agents/teams_tests.rs @@ -202,6 +202,7 @@ fn managed_agent(name: &str) -> ManagedAgentRecord { last_error_code: None, respond_to: crate::managed_agents::RespondTo::OwnerOnly, respond_to_allowlist: vec![], + reply_placement: None, display_name: None, slug: None, runtime: None, @@ -216,6 +217,7 @@ fn managed_agent(name: &str) -> ManagedAgentRecord { definition_respond_to: None, definition_respond_to_allowlist: vec![], definition_parallelism: None, + definition_reply_placement: None, } } diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index fcd8b13fc9..0ddb381cb2 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -88,6 +88,10 @@ pub struct AgentDefinition { pub respond_to_allowlist: Vec, #[serde(default, skip_serializing_if = "Option::is_none")] pub parallelism: Option, + /// NIP-AP reply placement default in wire form. Parsed at instance mint + /// so invalid values fail closed instead of silently changing routing. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub reply_placement: Option, pub created_at: String, pub updated_at: String, } @@ -138,6 +142,9 @@ impl AgentDefinition { last_error_code: None, respond_to: RespondTo::default(), respond_to_allowlist: Vec::new(), + // A definition-backed record inherits the definition/global default + // until minting or an explicit instance override chooses a mode. + reply_placement: None, display_name: Some(self.display_name), slug: Some(self.id), runtime: self.runtime, @@ -152,6 +159,7 @@ impl AgentDefinition { definition_respond_to: self.respond_to, definition_respond_to_allowlist: self.respond_to_allowlist, definition_parallelism: self.parallelism, + definition_reply_placement: self.reply_placement, relay_mesh: None, } } @@ -187,6 +195,7 @@ impl ManagedAgentRecord { respond_to: self.definition_respond_to.clone(), respond_to_allowlist: self.definition_respond_to_allowlist.clone(), parallelism: self.definition_parallelism, + reply_placement: self.definition_reply_placement.clone(), created_at: self.created_at.clone(), updated_at: self.updated_at.clone(), }) @@ -207,6 +216,8 @@ pub struct RelayAgentInfo { pub respond_to: Option, #[serde(default)] pub respond_to_allowlist: Vec, + #[serde(default)] + pub reply_placement: Option, } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] pub struct ManagedAgentRecord { @@ -352,6 +363,11 @@ pub struct ManagedAgentRecord { /// Preserved across mode toggles so users don't lose state. #[serde(default)] pub respond_to_allowlist: Vec, + /// Explicit per-instance reply-placement override. `None` means the + /// effective value is inherited from the linked definition/global config, + /// with the harness's historical `thread` behavior as the final fallback. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub reply_placement: Option, /// Optional display name distinct from the unique `name` handle. Absorbed /// from `AgentDefinition.display_name` (unified agent model, Phase 1A). #[serde(default, skip_serializing_if = "Option::is_none")] @@ -423,6 +439,10 @@ pub struct ManagedAgentRecord { pub definition_respond_to_allowlist: Vec, #[serde(default, skip_serializing_if = "Option::is_none")] pub definition_parallelism: Option, + /// NIP-AP definition-level reply placement default, kept in wire form + /// until the instance mint boundary validates it. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub definition_reply_placement: Option, /// Typed marker for relay-mesh agents. `Some(_)` means this agent runs its /// inference through Buzz's relay-mesh local endpoint; the `model_ref` is /// the served model id to route to. `None` is a normal agent. @@ -566,6 +586,11 @@ pub struct ManagedAgentSummary { pub log_path: String, pub respond_to: RespondTo, pub respond_to_allowlist: Vec, + /// Effective mode after resolving the instance override, persona default, + /// global default, and the backward-compatible `thread` fallback. + pub reply_placement: ReplyPlacement, + /// Explicit per-agent override, if one is stored. + pub reply_placement_override: Option, } #[derive(Debug, Serialize)] @@ -907,89 +932,11 @@ pub fn validate_respond_to_allowlist(input: &[String]) -> Result, St Ok(out) } -/// The behavioral fields resolved for a new instance at mint time. -#[derive(Debug, PartialEq, Eq)] -pub struct MintBehavioralDefaults { - pub respond_to: RespondTo, - pub respond_to_allowlist: Vec, - /// Validated (1..=32) when present; caller applies its own default. - pub parallelism: Option, -} - -/// Resolve the NIP-AP behavioral quad for a new instance: explicit input -/// wins, then the linked definition's defaults, then client defaults. -/// -/// This is the ONLY place definition behavioral strings are parsed — an -/// unrecognized `respond_to` mode or out-of-range `parallelism` on a -/// definition fails the mint loudly instead of silently substituting a -/// default the definition author did not choose. The empty-allowlist guard -/// fires here too, because inbound definitions bypass the dialog entirely. -/// -/// `input_allowlist` must already be normalized via -/// [`validate_respond_to_allowlist`]; the definition's allowlist is -/// validated here since it arrives from the wire. -pub fn resolve_mint_behavioral_defaults( - input_respond_to: Option, - input_allowlist: Vec, - input_parallelism: Option, - definition: Option<&AgentDefinition>, -) -> Result { - let (respond_to, respond_to_allowlist) = match input_respond_to { - // Explicit instance-level choice: the definition default is ignored - // wholesale (mode AND list travel together). - Some(mode) => (mode, input_allowlist), - None => match definition.and_then(|d| d.respond_to.as_deref()) { - Some(wire) => { - let mode = RespondTo::parse_wire(wire)?; - let list = if input_allowlist.is_empty() { - validate_respond_to_allowlist( - definition - .map(|d| d.respond_to_allowlist.as_slice()) - .unwrap_or(&[]), - ) - .map_err(|e| format!("definition respond-to allowlist is invalid: {e}"))? - } else { - input_allowlist - }; - (mode, list) - } - None => (RespondTo::default(), input_allowlist), - }, - }; - if respond_to == RespondTo::Allowlist && respond_to_allowlist.is_empty() { - return Err( - "respond-to mode 'allowlist' requires at least one pubkey in the allowlist".to_string(), - ); - } - - let parallelism = match input_parallelism { - // Explicit input is validated here too (not just at the command - // call sites) so the "validated when present" contract on - // `MintBehavioralDefaults.parallelism` is unskippable. - Some(count) if (1..=32).contains(&count) => Some(count), - Some(count) => { - return Err(format!( - "parallelism {count} is out of range (must be between 1 and 32)" - )) - } - None => match definition.and_then(|d| d.parallelism) { - Some(count) if (1..=32).contains(&count) => Some(count), - Some(count) => { - return Err(format!( - "parallelism {count} on the linked agent definition is out of range (must be between 1 and 32)" - )) - } - None => None, - }, - }; - - Ok(MintBehavioralDefaults { - respond_to, - respond_to_allowlist, - parallelism, - }) -} - +mod reply_placement; +pub use reply_placement::{ + resolve_effective_reply_placement, resolve_mint_behavioral_defaults, MintBehavioralDefaults, + ReplyPlacement, +}; mod catalog_source; pub use catalog_source::CatalogSource; mod requests; diff --git a/desktop/src-tauri/src/managed_agents/types/reply_placement.rs b/desktop/src-tauri/src/managed_agents/types/reply_placement.rs new file mode 100644 index 0000000000..406ffac953 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/types/reply_placement.rs @@ -0,0 +1,142 @@ +use super::{validate_respond_to_allowlist, AgentDefinition, ManagedAgentRecord, RespondTo}; +use serde::{Deserialize, Serialize}; + +/// Where a managed agent should place ordinary human-facing replies. +/// +/// The enum is intentionally mirrored in `buzz-acp::config` rather than shared +/// across crates so each boundary validates its own input and the desktop can +/// reject malformed persisted records before spawning a child process. +#[derive(Debug, Clone, Copy, Default, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "kebab-case")] +pub enum ReplyPlacement { + #[default] + Thread, + TopLevel, + FollowScope, +} + +impl ReplyPlacement { + pub fn as_str(self) -> &'static str { + match self { + Self::Thread => "thread", + Self::TopLevel => "top-level", + Self::FollowScope => "follow-scope", + } + } + + pub fn parse_wire(value: &str) -> Result { + match value { + "thread" => Ok(Self::Thread), + "top-level" => Ok(Self::TopLevel), + "follow-scope" => Ok(Self::FollowScope), + other => Err(format!( + "reply placement '{other}' is not a recognized mode (expected 'thread', 'top-level', or 'follow-scope')" + )), + } + } +} + +/// Resolve the mode that will reach `BUZZ_ACP_REPLY_PLACEMENT`. +pub fn resolve_effective_reply_placement( + record: &ManagedAgentRecord, + personas: &[AgentDefinition], + global_reply_placement: Option, +) -> Result { + if let Some(mode) = record.reply_placement { + return Ok(mode); + } + + if let Some(persona_id) = record.persona_id.as_deref() { + if let Some(persona) = personas.iter().find(|persona| persona.id == persona_id) { + if let Some(wire) = persona.reply_placement.as_deref() { + return ReplyPlacement::parse_wire(wire); + } + } + } + + Ok(global_reply_placement.unwrap_or_default()) +} + +/// The behavioral fields resolved for a new instance at mint time. +#[derive(Debug, PartialEq, Eq)] +pub struct MintBehavioralDefaults { + pub respond_to: RespondTo, + pub respond_to_allowlist: Vec, + pub reply_placement: ReplyPlacement, + /// Validated (1..=32) when present; caller applies its own default. + pub parallelism: Option, +} + +/// Resolve the NIP-AP behavioral defaults for a new instance. +pub fn resolve_mint_behavioral_defaults( + input_respond_to: Option, + input_allowlist: Vec, + input_parallelism: Option, + input_reply_placement: Option, + definition: Option<&AgentDefinition>, + global_reply_placement: Option, +) -> Result { + let (respond_to, respond_to_allowlist) = match input_respond_to { + // Explicit instance-level choice: the definition default is ignored + // wholesale (mode AND list travel together). + Some(mode) => (mode, input_allowlist), + None => match definition.and_then(|d| d.respond_to.as_deref()) { + Some(wire) => { + let mode = RespondTo::parse_wire(wire)?; + let list = if input_allowlist.is_empty() { + validate_respond_to_allowlist( + definition + .map(|d| d.respond_to_allowlist.as_slice()) + .unwrap_or(&[]), + ) + .map_err(|e| format!("definition respond-to allowlist is invalid: {e}"))? + } else { + input_allowlist + }; + (mode, list) + } + None => (RespondTo::default(), input_allowlist), + }, + }; + if respond_to == RespondTo::Allowlist && respond_to_allowlist.is_empty() { + return Err( + "respond-to mode 'allowlist' requires at least one pubkey in the allowlist".to_string(), + ); + } + + let reply_placement = match input_reply_placement { + Some(mode) => mode, + None => match definition.and_then(|d| d.reply_placement.as_deref()) { + Some(wire) => ReplyPlacement::parse_wire(wire)?, + None => global_reply_placement.unwrap_or_default(), + }, + }; + + let parallelism = match input_parallelism { + // Explicit input is validated here too (not just at the command + // call sites) so the "validated when present" contract on + // `MintBehavioralDefaults.parallelism` is unskippable. + Some(count) if (1..=32).contains(&count) => Some(count), + Some(count) => { + return Err(format!( + "parallelism {count} is out of range (must be between 1 and 32)" + )) + } + None => match definition.and_then(|d| d.parallelism) { + Some(count) if (1..=32).contains(&count) => Some(count), + Some(count) => { + return Err(format!( + "parallelism {count} on the linked agent definition is out of range (must be between 1 and 32)" + )) + } + None => None, + }, + }; + + Ok(MintBehavioralDefaults { + respond_to, + respond_to_allowlist, + reply_placement, + parallelism, + }) +} diff --git a/desktop/src-tauri/src/managed_agents/types/requests.rs b/desktop/src-tauri/src/managed_agents/types/requests.rs index e28b0bd461..ce61b53dc1 100644 --- a/desktop/src-tauri/src/managed_agents/types/requests.rs +++ b/desktop/src-tauri/src/managed_agents/types/requests.rs @@ -7,7 +7,7 @@ use serde::Deserialize; use super::{ default_start_on_app_launch, validate_respond_to_allowlist, AgentDefinition, BackendKind, - CatalogSource, RelayMeshConfig, RespondTo, + CatalogSource, RelayMeshConfig, ReplyPlacement, RespondTo, }; /// The NIP-AP behavioral group as one grouped request field. @@ -26,6 +26,8 @@ pub struct PersonaBehaviorRequest { pub respond_to_allowlist: Vec, #[serde(default)] pub parallelism: Option, + #[serde(default)] + pub reply_placement: Option, } /// Validate a behavior group and apply it onto a persona record. @@ -68,6 +70,9 @@ pub fn apply_persona_behavior( Vec::new() }; record.parallelism = behavior.parallelism; + record.reply_placement = behavior + .reply_placement + .map(|mode| mode.as_str().to_string()); Ok(()) } @@ -186,6 +191,10 @@ pub struct CreateManagedAgentRequest { /// before being written to the record. #[serde(default)] pub respond_to_allowlist: Vec, + /// Optional instance-level reply placement. Omitted means the linked + /// persona default applies; definition-less instances use `thread`. + #[serde(default)] + pub reply_placement: Option, #[serde(default)] pub relay_mesh: Option, } @@ -253,6 +262,10 @@ pub struct UpdateManagedAgentRequest { /// normalized server-side). #[serde(default)] pub respond_to_allowlist: Option>, + /// Absent = don't touch. `null` clears the instance override so the agent + /// follows its persona/global fallback; a concrete value pins the mode. + #[serde(default, deserialize_with = "crate::util::double_option")] + pub reply_placement: Option>, } #[cfg(test)] @@ -287,6 +300,7 @@ mod tests { respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, created_at: "2026-01-01T00:00:00Z".to_string(), updated_at: "2026-01-01T00:00:00Z".to_string(), } @@ -313,6 +327,7 @@ mod tests { respond_to: Some(RespondTo::Anyone), respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, }), ) .unwrap(); @@ -321,6 +336,21 @@ mod tests { assert_eq!(record.parallelism, None); } + #[test] + fn update_reply_placement_supports_absent_null_and_value() { + let absent: UpdateManagedAgentRequest = + serde_json::from_str(r#"{"pubkey":"agent"}"#).unwrap(); + assert_eq!(absent.reply_placement, None); + + let clear: UpdateManagedAgentRequest = + serde_json::from_str(r#"{"pubkey":"agent","replyPlacement":null}"#).unwrap(); + assert_eq!(clear.reply_placement, Some(None)); + + let set: UpdateManagedAgentRequest = + serde_json::from_str(r#"{"pubkey":"agent","replyPlacement":"follow-scope"}"#).unwrap(); + assert_eq!(set.reply_placement, Some(Some(ReplyPlacement::FollowScope))); + } + #[test] fn allowlist_mode_with_empty_list_is_rejected() { let mut record = record_without_quad(); @@ -400,6 +430,7 @@ mod tests { respond_to: Some(RespondTo::Allowlist), respond_to_allowlist: vec!["c".repeat(64)], parallelism: Some(3), + reply_placement: Some(ReplyPlacement::FollowScope), }), ) .unwrap(); @@ -407,6 +438,7 @@ mod tests { assert_eq!(content.respond_to.as_deref(), Some("allowlist")); assert_eq!(content.respond_to_allowlist, vec!["c".repeat(64)]); assert_eq!(content.parallelism, Some(3)); + assert_eq!(content.reply_placement.as_deref(), Some("follow-scope")); } #[test] diff --git a/desktop/src-tauri/src/managed_agents/types/tests.rs b/desktop/src-tauri/src/managed_agents/types/tests.rs index 96ed556068..f4c922c313 100644 --- a/desktop/src-tauri/src/managed_agents/types/tests.rs +++ b/desktop/src-tauri/src/managed_agents/types/tests.rs @@ -93,7 +93,9 @@ fn managed_agent_record_with_auth_tag_round_trips() { // ── Inbound author gate tests ──────────────────────────────────────── -use super::{validate_respond_to_allowlist, RespondTo}; +use super::{ + resolve_effective_reply_placement, validate_respond_to_allowlist, ReplyPlacement, RespondTo, +}; #[test] fn respond_to_default_is_owner_only() { @@ -130,6 +132,82 @@ fn respond_to_rejects_unknown_modes() { assert!(serde_json::from_str::("\"OwnerOnly\"").is_err()); } +#[test] +fn reply_placement_serde_is_kebab_case_and_closed() { + assert_eq!( + serde_json::to_string(&ReplyPlacement::Thread).unwrap(), + "\"thread\"" + ); + assert_eq!( + serde_json::to_string(&ReplyPlacement::TopLevel).unwrap(), + "\"top-level\"" + ); + assert_eq!( + serde_json::to_string(&ReplyPlacement::FollowScope).unwrap(), + "\"follow-scope\"" + ); + assert_eq!( + serde_json::from_str::("\"follow-scope\"").unwrap(), + ReplyPlacement::FollowScope + ); + assert!(serde_json::from_str::("\"invalid\"").is_err()); +} + +#[test] +fn effective_reply_placement_uses_instance_persona_global_thread_precedence() { + let mut record = sample_persona().into_agent_record(); + record.persona_id = Some("custom:helper".to_string()); + let mut persona = sample_persona(); + persona.reply_placement = Some("top-level".to_string()); + + assert_eq!( + resolve_effective_reply_placement( + &record, + std::slice::from_ref(&persona), + Some(ReplyPlacement::FollowScope), + ) + .unwrap(), + ReplyPlacement::TopLevel + ); + + record.reply_placement = Some(ReplyPlacement::FollowScope); + assert_eq!( + resolve_effective_reply_placement( + &record, + std::slice::from_ref(&persona), + Some(ReplyPlacement::Thread), + ) + .unwrap(), + ReplyPlacement::FollowScope + ); + + record.reply_placement = None; + persona.reply_placement = None; + assert_eq!( + resolve_effective_reply_placement( + &record, + std::slice::from_ref(&persona), + Some(ReplyPlacement::TopLevel), + ) + .unwrap(), + ReplyPlacement::TopLevel + ); + assert_eq!( + resolve_effective_reply_placement(&record, &[], None).unwrap(), + ReplyPlacement::Thread + ); +} + +#[test] +fn effective_reply_placement_rejects_invalid_persona_wire_value() { + let mut record = sample_persona().into_agent_record(); + record.persona_id = Some("custom:helper".to_string()); + let mut persona = sample_persona(); + persona.reply_placement = Some("follow_scope".to_string()); + let error = resolve_effective_reply_placement(&record, &[persona], None).unwrap_err(); + assert!(error.contains("follow_scope"), "{error}"); +} + /// Records persisted before this feature must continue to load, /// defaulting to OwnerOnly (the safe, matches-harness-default value). #[test] @@ -490,6 +568,7 @@ fn sample_persona() -> AgentDefinition { respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, created_at: "2026-01-01T00:00:00Z".to_string(), updated_at: "2026-01-02T00:00:00Z".to_string(), } @@ -602,11 +681,14 @@ fn mint_explicit_input_wins_over_definition() { Some(RespondTo::OwnerOnly), Vec::new(), Some(2), + None, Some(&definition), + None, ) .unwrap(); assert_eq!(minted.respond_to, RespondTo::OwnerOnly); assert_eq!(minted.parallelism, Some(2)); + assert_eq!(minted.reply_placement, ReplyPlacement::Thread); } #[test] @@ -614,7 +696,8 @@ fn mint_copies_definition_quad_when_input_silent() { let allow = "a".repeat(64); let definition = quad_definition("allowlist", vec![&allow]); let minted = - resolve_mint_behavioral_defaults(None, Vec::new(), None, Some(&definition)).unwrap(); + resolve_mint_behavioral_defaults(None, Vec::new(), None, None, Some(&definition), None) + .unwrap(); assert_eq!(minted.respond_to, RespondTo::Allowlist); assert_eq!(minted.respond_to_allowlist, vec![allow]); assert_eq!(minted.parallelism, Some(8)); @@ -622,7 +705,8 @@ fn mint_copies_definition_quad_when_input_silent() { #[test] fn mint_without_definition_or_input_uses_client_defaults() { - let minted = resolve_mint_behavioral_defaults(None, Vec::new(), None, None).unwrap(); + let minted = + resolve_mint_behavioral_defaults(None, Vec::new(), None, None, None, None).unwrap(); assert_eq!(minted.respond_to, RespondTo::default()); assert!(minted.respond_to_allowlist.is_empty()); assert_eq!(minted.parallelism, None); @@ -635,7 +719,8 @@ fn mint_fails_loudly_on_unknown_definition_respond_to() { // move. The error must carry the offending string. let definition = quad_definition("allowlst", vec![]); let err = - resolve_mint_behavioral_defaults(None, Vec::new(), None, Some(&definition)).unwrap_err(); + resolve_mint_behavioral_defaults(None, Vec::new(), None, None, Some(&definition), None) + .unwrap_err(); assert!( err.contains("allowlst"), "error must name the bad mode: {err}" @@ -648,7 +733,8 @@ fn mint_fails_loudly_on_empty_definition_allowlist() { // boundary is the backstop against a crash-looping instance. let definition = quad_definition("allowlist", vec![]); let err = - resolve_mint_behavioral_defaults(None, Vec::new(), None, Some(&definition)).unwrap_err(); + resolve_mint_behavioral_defaults(None, Vec::new(), None, None, Some(&definition), None) + .unwrap_err(); assert!( err.contains("at least one pubkey"), "unexpected error: {err}" @@ -660,7 +746,8 @@ fn mint_fails_loudly_on_out_of_range_definition_parallelism() { let mut definition = quad_definition("anyone", vec![]); definition.parallelism = Some(64); let err = - resolve_mint_behavioral_defaults(None, Vec::new(), None, Some(&definition)).unwrap_err(); + resolve_mint_behavioral_defaults(None, Vec::new(), None, None, Some(&definition), None) + .unwrap_err(); assert!(err.contains("64"), "error must name the bad value: {err}"); } @@ -669,7 +756,8 @@ fn mint_normalizes_definition_allowlist_from_wire() { let upper = "A".repeat(64); let definition = quad_definition("allowlist", vec![&upper]); let minted = - resolve_mint_behavioral_defaults(None, Vec::new(), None, Some(&definition)).unwrap(); + resolve_mint_behavioral_defaults(None, Vec::new(), None, None, Some(&definition), None) + .unwrap(); assert_eq!(minted.respond_to_allowlist, vec!["a".repeat(64)]); } @@ -678,7 +766,8 @@ fn mint_resolves_each_behavioral_field_independently() { // PR #1667 review (convergent): the input-wins rule is per-FIELD, not let definition = quad_definition("anyone", vec![]); let minted = - resolve_mint_behavioral_defaults(None, Vec::new(), None, Some(&definition)).unwrap(); + resolve_mint_behavioral_defaults(None, Vec::new(), None, None, Some(&definition), None) + .unwrap(); assert_eq!(minted.respond_to, RespondTo::Anyone, "inherited"); assert_eq!(minted.parallelism, Some(8), "inherited"); } @@ -687,10 +776,65 @@ fn mint_resolves_each_behavioral_field_independently() { fn mint_rejects_out_of_range_input_parallelism() { // The "validated when present" contract on MintBehavioralDefaults holds // for the INPUT branch too, not just definition values. - let err = resolve_mint_behavioral_defaults(None, Vec::new(), Some(64), None).unwrap_err(); + let err = + resolve_mint_behavioral_defaults(None, Vec::new(), Some(64), None, None, None).unwrap_err(); assert!(err.contains("64"), "error must name the bad value: {err}"); assert!( !err.contains("definition"), "input-branch error must not blame the definition: {err}" ); } + +#[test] +fn mint_reply_placement_uses_input_then_definition_then_global_then_thread() { + let mut definition = quad_definition("anyone", vec![]); + definition.reply_placement = Some("follow-scope".to_string()); + + let explicit = resolve_mint_behavioral_defaults( + None, + Vec::new(), + None, + Some(ReplyPlacement::TopLevel), + Some(&definition), + Some(ReplyPlacement::Thread), + ) + .unwrap(); + assert_eq!(explicit.reply_placement, ReplyPlacement::TopLevel); + + let inherited = resolve_mint_behavioral_defaults( + None, + Vec::new(), + None, + None, + Some(&definition), + Some(ReplyPlacement::Thread), + ) + .unwrap(); + assert_eq!(inherited.reply_placement, ReplyPlacement::FollowScope); + + definition.reply_placement = None; + let global = resolve_mint_behavioral_defaults( + None, + Vec::new(), + None, + None, + Some(&definition), + Some(ReplyPlacement::TopLevel), + ) + .unwrap(); + assert_eq!(global.reply_placement, ReplyPlacement::TopLevel); + + let fallback = + resolve_mint_behavioral_defaults(None, Vec::new(), None, None, None, None).unwrap(); + assert_eq!(fallback.reply_placement, ReplyPlacement::Thread); +} + +#[test] +fn mint_rejects_invalid_definition_reply_placement() { + let mut definition = quad_definition("anyone", vec![]); + definition.reply_placement = Some("top_level".to_string()); + let error = + resolve_mint_behavioral_defaults(None, Vec::new(), None, None, Some(&definition), None) + .unwrap_err(); + assert!(error.contains("top_level"), "{error}"); +} diff --git a/desktop/src-tauri/src/mesh_llm/recovery.rs b/desktop/src-tauri/src/mesh_llm/recovery.rs index 7933fd291e..af6584b6d4 100644 --- a/desktop/src-tauri/src/mesh_llm/recovery.rs +++ b/desktop/src-tauri/src/mesh_llm/recovery.rs @@ -469,6 +469,7 @@ mod tests { respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, created_at: "2026-01-01T00:00:00Z".to_string(), updated_at: "2026-01-01T00:00:00Z".to_string(), } diff --git a/desktop/src-tauri/src/migration_avatar_tests.rs b/desktop/src-tauri/src/migration_avatar_tests.rs index 39dfc988dd..75556eb58b 100644 --- a/desktop/src-tauri/src/migration_avatar_tests.rs +++ b/desktop/src-tauri/src/migration_avatar_tests.rs @@ -43,6 +43,7 @@ fn refresh_builtin_agent_avatars_updates_seeded_values_and_preserves_customizati respond_to: None, respond_to_allowlist: Vec::new(), parallelism: None, + reply_placement: None, created_at: "before".to_string(), updated_at: "before".to_string(), }; diff --git a/desktop/src/features/agents/AGENTS.md b/desktop/src/features/agents/AGENTS.md index d9222c7032..0b3960426e 100644 --- a/desktop/src/features/agents/AGENTS.md +++ b/desktop/src/features/agents/AGENTS.md @@ -148,6 +148,13 @@ with a TypeScript lookup table or an id comparison in a component. expose `respond-to`, `allowlist`, Nostr, or harness jargon in primary UI copy. +12. **Reply placement is a typed, inherited managed-agent setting.** The + Desktop surfaces expose `thread`, `top-level`, and `follow-scope`; the + effective precedence is instance override → persona → global → historical + `thread` fallback. Persisted persona wire values must remain fail-closed at + the spawn/mint boundary, and the launcher must pass the resolved mode via + `BUZZ_ACP_REPLY_PLACEMENT` rather than allowing user env vars to override it. + ## The tests that enforce this - `lib/agentConfigCore.test.mjs` — field model per harness × scope, clearing @@ -173,6 +180,8 @@ with a TypeScript lookup table or an id comparison in a component. - Rust: `runtime_metadata_env_vars` tests pin spawn-time key application. - Rust: persona sharing/retention tests pin relay+owner scoping, durable enqueue errors, relay rejection/unavailability, and accepted publication. +- Rust/UI: reply-placement routing, inheritance, persistence, and launcher + propagation tests cover all three modes and legacy records without the field. ## Keep this file true diff --git a/desktop/src/features/agents/lib/personaCatalogRelay.test.mjs b/desktop/src/features/agents/lib/personaCatalogRelay.test.mjs index fbaf1f5274..16029a2bc1 100644 --- a/desktop/src/features/agents/lib/personaCatalogRelay.test.mjs +++ b/desktop/src/features/agents/lib/personaCatalogRelay.test.mjs @@ -21,6 +21,7 @@ function personaEvent({ shared = true, avatarUrl = null, respondTo = null, + replyPlacement = null, sharedTag, }) { return { @@ -47,6 +48,7 @@ function personaEvent({ respond_to: respondTo, respond_to_allowlist: respondTo === "allowlist" ? [BOB] : undefined, parallelism: 4, + reply_placement: replyPlacement, }), sig: "sig", }; @@ -66,6 +68,34 @@ test("a shared kind 30175 persona from Alice is discoverable by Bob", () => { assert.equal(personas[0].catalogSource.isOwn, false); }); +test("catalog preserves a valid reply-placement setting", () => { + const publications = catalogPublicationsFromEvents([ + personaEvent({ + createdAt: 1, + id: "follow-scope", + replyPlacement: "follow-scope", + }), + ]); + const personas = catalogPersonasFromPublications(publications, [], BOB); + + assert.equal(personas[0].replyPlacement, "follow-scope"); +}); + +test("catalog drops an invalid reply-placement value to the safe fallback", () => { + const event = personaEvent({ + createdAt: 1, + id: "invalid-reply-placement", + }); + const content = JSON.parse(event.content); + content.reply_placement = "follow_scope"; + event.content = JSON.stringify(content); + + const publications = catalogPublicationsFromEvents([event]); + const personas = catalogPersonasFromPublications(publications, [], BOB); + + assert.equal(personas[0].replyPlacement, null); +}); + test("a newer unshared head hides the older shared head", () => { const publications = catalogPublicationsFromEvents([ personaEvent({ createdAt: 1, id: "shared" }), @@ -356,7 +386,7 @@ test("test_foreign_entry_with_no_local_copy_stays_unselected", () => { BOB, ); - assert.equal(personas[0].id, "catalog:" + ALICE + ":reviewer"); + assert.equal(personas[0].id, `catalog:${ALICE}:reviewer`); assert.equal(personas[0].isActive, false); }); @@ -377,7 +407,7 @@ test("test_catalog_source_match_is_scoped_to_the_publishing_owner", () => { ALICE, ); - assert.equal(personas[0].id, "catalog:" + BOB + ":reviewer"); + assert.equal(personas[0].id, `catalog:${BOB}:reviewer`); assert.equal(personas[0].isActive, false); }); diff --git a/desktop/src/features/agents/lib/personaCatalogRelay.ts b/desktop/src/features/agents/lib/personaCatalogRelay.ts index 02c3f8e202..c4cdd86aa2 100644 --- a/desktop/src/features/agents/lib/personaCatalogRelay.ts +++ b/desktop/src/features/agents/lib/personaCatalogRelay.ts @@ -3,6 +3,7 @@ import type { AgentPersona, CatalogSourceCoordinate, RelayEvent, + ReplyPlacementMode, RespondToMode, } from "@/shared/api/types"; import { KIND_PERSONA } from "@/shared/constants/kinds"; @@ -19,6 +20,7 @@ type CatalogAgentProjection = { namePool: string[]; respondTo: RespondToMode | null; parallelism: number | null; + replyPlacement: ReplyPlacementMode | null; }; export type PersonaCatalogPublication = { @@ -165,6 +167,15 @@ function parsePersonaContent(event: RelayEvent): CatalogAgentProjection | null { parsed.parallelism <= 32 ? parsed.parallelism : null; + const rawReplyPlacement = parsed.reply_placement; + const replyPlacement = + rawReplyPlacement === undefined || rawReplyPlacement === null + ? null + : rawReplyPlacement === "thread" || + rawReplyPlacement === "top-level" || + rawReplyPlacement === "follow-scope" + ? rawReplyPlacement + : null; return { displayName: parsed.display_name, @@ -177,6 +188,7 @@ function parsePersonaContent(event: RelayEvent): CatalogAgentProjection | null { namePool, respondTo, parallelism, + replyPlacement, }; } @@ -306,6 +318,7 @@ function publicationToPersona( respondTo: publication.agent.respondTo, respondToAllowlist: [], parallelism: publication.agent.parallelism, + replyPlacement: publication.agent.replyPlacement, createdAt: timestamp, updatedAt: timestamp, }; diff --git a/desktop/src/features/agents/ui/AgentConfigFields.tsx b/desktop/src/features/agents/ui/AgentConfigFields.tsx index 1bd8af8976..9b22748108 100644 --- a/desktop/src/features/agents/ui/AgentConfigFields.tsx +++ b/desktop/src/features/agents/ui/AgentConfigFields.tsx @@ -61,6 +61,7 @@ export const EMPTY_GLOBAL_CONFIG: GlobalAgentConfig = { provider: null, model: null, preferred_runtime: null, + reply_placement: null, }; /** Baked env keys that route to structured controls, not the generic env editor. */ diff --git a/desktop/src/features/agents/ui/AgentDefaultsEditor.tsx b/desktop/src/features/agents/ui/AgentDefaultsEditor.tsx index 69e8b3a5b4..dbe7c9d7ed 100644 --- a/desktop/src/features/agents/ui/AgentDefaultsEditor.tsx +++ b/desktop/src/features/agents/ui/AgentDefaultsEditor.tsx @@ -37,6 +37,7 @@ import { AgentConfigFields, EMPTY_GLOBAL_CONFIG, } from "@/features/agents/ui/AgentConfigFields"; +import { ReplyPlacementField } from "@/features/agents/ui/ReplyPlacementField"; import { cn } from "@/shared/lib/cn"; import { Button } from "@/shared/ui/button"; @@ -294,6 +295,16 @@ export function AgentDefaultsEditor({ value={selectedRuntime?.id ?? ""} /> + + handleConfigChange({ + ...config, + reply_placement: replyPlacement, + }) + } + value={config.reply_placement ?? "thread"} + /> {flatLayout ? ( {configFields ? ( diff --git a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx index 79d1e9a790..83d4411b42 100644 --- a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx +++ b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx @@ -2,7 +2,6 @@ import * as React from "react"; import { ChevronDown } from "lucide-react"; import { AnimatePresence, motion, useReducedMotion } from "motion/react"; import { toast } from "sonner"; - import { useAcpRuntimesQuery, useAgentConfigSurface, @@ -65,6 +64,7 @@ import { AgentCreationPreview } from "./AgentCreationPreview"; import type { EnvVarsValue } from "./EnvVarsEditor"; import { useRequiredCredentialState } from "./useRequiredCredentialState"; import { CreateAgentRespondToField } from "./RespondToField"; +import { ReplyPlacementField } from "./ReplyPlacementField"; import { PersonaDropdownField } from "./PersonaDropdownField"; import { MODEL_DISCOVERY_LOADING_VALUE, @@ -89,12 +89,10 @@ import { runtimeDropdownAction, usePendingHarnessSelection, } from "./addCustomHarness"; - const ADVANCED_FIELDS_MOTION_TRANSITION = { duration: 0.18, ease: [0.23, 1, 0.32, 1], } as const; - export function AgentInstanceEditDialog({ agent, initialFocus, @@ -117,7 +115,6 @@ export function AgentInstanceEditDialog({ const runtimesQuery = useAcpRuntimesQuery({ enabled: open }); const configSurfaceQuery = useAgentConfigSurface(open ? agent.pubkey : null); const runtimes = runtimesQuery.data ?? []; - const [name, setName] = React.useState(agent.name); const [aiDefaultsOpen, setAiDefaultsOpen] = React.useState(false); const aiDefaultsTriggerRef = React.useRef(null); @@ -159,13 +156,15 @@ export function AgentInstanceEditDialog({ const [respondToAllowlist, setRespondToAllowlist] = React.useState( agent.respondToAllowlist, ); + const [replyPlacement, setReplyPlacement] = React.useState( + agent.replyPlacementOverride, + ); const [showAdvancedFields, setShowAdvancedFields] = React.useState(false); const [avatarUrl, setAvatarUrl] = React.useState(agent.avatarUrl ?? ""); const [isAvatarUploadPending, setIsAvatarUploadPending] = React.useState(false); const [isAddHarnessOpen, setIsAddHarnessOpen] = React.useState(false); const shouldReduceMotion = useReducedMotion(); - // Runtime selector: defaults to "custom" until the dialog opens and the // catalog loads. The open-effect re-derives the correct id from the catalog. const [selectedRuntimeId, setSelectedRuntimeId] = React.useState("custom"); @@ -195,6 +194,7 @@ export function AgentInstanceEditDialog({ setAutoRestartOnConfigChange(agent.autoRestartOnConfigChange); setRespondTo(agent.respondTo); setRespondToAllowlist(agent.respondToAllowlist); + setReplyPlacement(agent.replyPlacementOverride); setAvatarUrl(agent.avatarUrl ?? ""); setShowAdvancedFields(false); setIsAvatarUploadPending(false); @@ -207,7 +207,6 @@ export function AgentInstanceEditDialog({ updateMutation.reset(); } }, [open, agent.pubkey]); - // Re-derive the runtime id when the catalog loads. React.useEffect(() => { if (!open || runtimeTouched.current || runtimes.length === 0) { @@ -233,7 +232,6 @@ export function AgentInstanceEditDialog({ ); const runtimeDropdownValue = selectedRuntimeId || NO_RUNTIME_DROPDOWN_VALUE; - const runtimeDropdownOptions: PersonaDropdownOption[] = React.useMemo(() => { const options: PersonaDropdownOption[] = [ ...sortedRuntimes.map((candidate) => ({ @@ -255,7 +253,6 @@ export function AgentInstanceEditDialog({ options.push(ADD_CUSTOM_HARNESS_OPTION); return options; }, [sortedRuntimes, selectedRuntimeId]); - // Resolve the dialog-opening command as the catalog loads. Edit-state runtime // ids mutate during selection changes and cannot identify the original state. const originalRuntimeSupportsProvider = React.useMemo(() => { @@ -306,7 +303,6 @@ export function AgentInstanceEditDialog({ const llmProviderFieldVisible = runtimeSupportsLlmProviderSelection(prospectiveRuntimeId); - // One-shot focus: when the dialog opens from a card deep-link, scroll and // focus the relevant field. The effect re-runs when `llmProviderFieldVisible` // changes so a provider-field focus request fires once the field materializes. @@ -375,7 +371,6 @@ export function AgentInstanceEditDialog({ }, inheritedEnvVars: inheritedEnvVarsForAdvanced, } = useAgentDialogDefaults({ inheritedEnvVars, open }); - // Runtime/provider-required credential state, derived from the PROSPECTIVE // post-submit runtime — see the hook for the inherit-transition rationale. // Pass globalProvider so the hook uses it as a fallback when the per-agent @@ -394,7 +389,6 @@ export function AgentInstanceEditDialog({ }); const { data: bakedEnvKeys } = useBakedBuildEnvKeysQuery({ enabled: open }); - // Merge global env as the base layer so credential keys satisfied via global // config (e.g. ANTHROPIC_API_KEY) are available to model discovery. Use // `inheritedSubmission.envVars` (the same snapshot the credential gate @@ -422,7 +416,6 @@ export function AgentInstanceEditDialog({ provider: providerForDiscovery, selectedRuntime, }); - // D2: derive advancedRequiredEnvKeys for EnvVarsEditor display. // The full requiredEnvKeys/requiredEnvKeyMissing continue driving Save gating. // D2/D3: the top-level API key owns display, while the readiness gate keeps @@ -554,7 +547,6 @@ export function AgentInstanceEditDialog({ handleRuntimeDropdownChange, open, ); - function handleProviderDropdownChange(nextValue: string) { const nextProvider = nextValue === AUTO_PROVIDER_DROPDOWN_VALUE ? "" : nextValue; @@ -574,7 +566,6 @@ export function AgentInstanceEditDialog({ model: nextProvider === "relay-mesh" ? "auto" : nextSelection.model, }); } - function handleModelDropdownChange(nextValue: string) { applySelection( selectionOnModelDropdownChange(selection, { @@ -584,11 +575,9 @@ export function AgentInstanceEditDialog({ }), ); } - function handleOpenChange(next: boolean) { onOpenChange(next); } - const providerValid = isEditAgentProviderSaveValid({ llmProviderFieldVisible, currentProvider: provider, @@ -596,7 +585,6 @@ export function AgentInstanceEditDialog({ globalProvider: inheritedProviderDefault.value, originalRuntimeSupportsProvider, }); - const canSubmit = computeEditAgentFormValidity({ name, @@ -723,6 +711,10 @@ export function AgentInstanceEditDialog({ respondToAllowlist.join(",") !== agent.respondToAllowlist.join(",") ? respondToAllowlist : undefined, + replyPlacement: + replyPlacement === agent.replyPlacementOverride + ? undefined + : replyPlacement, }; const result = await updateMutation.mutateAsync(input); @@ -946,6 +938,14 @@ export function AgentInstanceEditDialog({ variant="persona" /> + + {/* Provider (runtime) */}