diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 93109fa94df..2bd37070bea 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -13,13 +13,29 @@ use tokio::io::AsyncWriteExt; use tokio::process::{Child, ChildStdin, ChildStdout}; use tokio_util::codec::{FramedRead, LinesCodec, LinesCodecError}; -use crate::observer::{ObserverContext, ObserverHandle}; +use crate::config::{PermissionMode, PermissionPolicy, ResolvedPermissionConfig}; +use crate::observer::{AuthorizationEnvelope, ObserverContext, ObserverEvent, ObserverHandle}; use crate::usage::{TurnUsage, UsageTracker}; +use buzz_core::observer::OBSERVER_MAX_PLAINTEXT_LEN; /// Maximum allowed size of a single NDJSON line from the agent's stdout. /// Lines exceeding this limit are rejected to prevent OOM from rogue agents. const MAX_LINE_SIZE: usize = 10_000_000; // 10 MB +/// Maximum number of `session/request_permission` requests that may be +/// simultaneously pending under the `ask` policy. New requests beyond this +/// cap are denied immediately (fail closed) so the map remains bounded. +pub const PERMISSION_MAP_CAP: usize = 8; + +/// Maximum number of options in a single `session/request_permission` request. +/// Requests with more options are denied immediately (admission preflight). +const PERMISSION_OPTIONS_MAX: usize = 16; + +/// Per-request timeout under the `ask` policy. The desktop has at most this +/// long to deliver a `permission_decision` control frame before the harness +/// fails closed with the denial response. +const PERMISSION_ASK_TIMEOUT_SECS: u64 = 300; + /// An MCP server configuration passed to `session/new`. /// /// Corresponds to the `McpServerStdio` variant in the ACP schema. @@ -106,6 +122,16 @@ pub enum AcpError { #[error("Agent reported error (code {code}): {message}")] AgentError { code: i64, message: String }, + + /// A permission response write was interrupted mid-flight by a cancel. + /// + /// The process may have received the response bytes but may not have acted + /// on them — state is irrecoverably uncertain. The agent process MUST be + /// replaced (not returned to the pool) after this error. The cancel path + /// surfaces this through `cancel_with_cleanup_grace` so + /// `classify_control_cancel_failure` in `pool.rs` triggers respawn. + #[error("Permission response write was interrupted — process state uncertain")] + PermissionPoisoned, } /// Build an [`AcpError::AgentError`] from a JSON-RPC error object, @@ -132,6 +158,45 @@ fn build_initialize_params() -> serde_json::Value { }) } +/// A decision delivered by the desktop via a `permission_decision` control frame. +#[derive(Debug, Clone)] +pub struct PermissionDecision { + /// The nonce that was advertised in the `authorization` envelope of the + /// `acp_read` frame for this request. + pub request_nonce: String, + /// The `optionId` the owner chose. Must exactly match one of the options in + /// the original request. + pub option_id: String, +} + +/// Lifecycle state of a single `session/request_permission` request under +/// the `ask` policy. +#[derive(Debug)] +enum PermissionEntryState { + /// Registered and waiting for an owner decision. + Pending, + /// A decision arrived; we are in the process of writing the response. + /// Cancel during this state → `PermissionPoisoned`. + Writing, + /// Fully resolved — write confirmed. Kept in map until turn end to guard + /// against duplicate delivery. + Resolved, +} + +/// Per-request state tracked in `AcpClient::pending_permissions` under `ask`. +#[derive(Debug)] +struct PermissionEntry { + /// Nonce bound to this request — must match the desktop's decision. + nonce: String, + /// The exact options snapshot from the original request. + options_snapshot: Vec, + /// Current lifecycle state. + state: PermissionEntryState, + /// Per-request hard deadline: `min(registered_at + 300s, turn hard deadline)`. + /// Expiry → fail closed (denial + `cancelled` outcome). + deadline: tokio::time::Instant, +} + /// ACP client that owns an agent subprocess and communicates over its stdio. /// /// One `AcpClient` per agent process. Multiple sessions can be created on the @@ -153,11 +218,39 @@ pub struct AcpClient { /// permits both numeric and string IDs from the agent. /// Used by [`cancel_with_cleanup`](AcpClient::cancel_with_cleanup) to send /// a `cancelled` outcome before the agent returns from `session/prompt`. + /// + /// Under `reject` and `allow` policies only one request can be in-flight + /// (synchronous handling), so a single Option suffices. + /// Under `ask` the full map is `pending_permissions` below. pending_permission_id: Option, /// Whether we have already sent a response to the pending permission request. /// Guards against double-response if a timeout fires after the rejection /// response was written but before `pending_permission_id` was cleared. permission_responded: bool, + /// Pending `session/request_permission` entries under the `ask` policy. + /// + /// Keyed by request id (as JSON Value). Bounded at `PERMISSION_MAP_CAP`. + /// Entries transition: `Pending → Writing(optionId) → Resolved`. + /// Cancel during `Writing` → `PermissionPoisoned`. + /// Cleared at turn end. + pending_permissions: std::collections::HashMap, + /// Whether this process is poisoned due to a cancel-during-write. + /// + /// When `true` the process MUST NOT be returned to the pool — it must be + /// respawned. The cancel path surfaces this via `PermissionPoisoned`. + permission_poisoned: bool, + /// Resolved permission configuration. Determines how `handle_permission_request` + /// answers ACP `session/request_permission` frames. + permission_config: ResolvedPermissionConfig, + /// Whether an agent owner pubkey was resolved at startup. + /// + /// Used by the `ask` availability gate: `ask` without a known owner downgrades + /// to `reject` (the desktop needs an owner to route the permission card to). + owner_pubkey_known: bool, + /// Channel for delivering `permission_decision` control frames from the + /// observer dispatch loop into the read loop's decision arm. + /// Installed by `install_permission_decision_rx`; consumed by the read loop. + permission_decision_rx: Option>, /// The JSON-RPC id of the most recently sent `session/prompt` request. /// Used by [`cancel_with_cleanup`] to drain the correct response. /// Set in [`session_prompt_with_idle_timeout`]; consumed in [`cancel_with_cleanup`]. @@ -541,6 +634,16 @@ impl AcpClient { next_id: 0, pending_permission_id: None, permission_responded: false, + pending_permissions: std::collections::HashMap::new(), + permission_poisoned: false, + permission_config: ResolvedPermissionConfig { + policy: crate::config::PermissionPolicy::Reject, + effective_mode: PermissionMode::DontAsk, + mode_source: crate::config::ModeSource::Derived, + transmit_mode: true, + }, + owner_pubkey_known: false, + permission_decision_rx: None, last_prompt_id: None, current_hard_deadline: None, observer: None, @@ -559,6 +662,33 @@ impl AcpClient { self.observer_agent_index = Some(agent_index); } + /// Set the resolved permission configuration for this agent process. + /// + /// Called once after spawn (like `set_observer`) by `pool_lifecycle`. + pub fn set_permission_config(&mut self, config: ResolvedPermissionConfig) { + self.permission_config = config; + } + + /// Record whether the agent owner pubkey is known at startup. + /// + /// The `ask` availability gate downgrades to `reject` when the owner is + /// unknown — the desktop needs an owner to route the permission card. + pub fn set_owner_pubkey_known(&mut self, known: bool) { + self.owner_pubkey_known = known; + } + + /// Install the per-session `permission_decision` receiver. + /// + /// The matching `Sender` is held by `handle_observer_control` in `lib.rs` + /// and delivers `permission_decision` control frames into the read loop's + /// decision arm. Idempotent — replaces any previously installed receiver. + pub fn install_permission_decision_rx( + &mut self, + rx: tokio::sync::mpsc::Receiver, + ) { + self.permission_decision_rx = Some(rx); + } + /// Update metadata that will be attached to subsequent raw wire events. pub fn set_observer_context(&mut self, context: ObserverContext) { self.observer_context = context; @@ -586,6 +716,24 @@ impl AcpClient { } } + /// Emit a semantic event with an authorization envelope, if observer enabled. + fn observe_authorized( + &self, + kind: impl Into, + authorization: AuthorizationEnvelope, + payload: serde_json::Value, + ) { + if let Some(observer) = &self.observer { + observer.emit_authorized( + kind, + self.observer_agent_index, + &self.observer_context, + authorization, + payload, + ); + } + } + /// Send the `initialize` request and return the agent's response result value. /// /// Must be called exactly once, before any other ACP method. @@ -811,6 +959,10 @@ impl AcpClient { Ok(_) => { self.last_prompt_id = None; self.current_hard_deadline = None; + // Turn completed normally — drain resolved/expired permission entries. + // Pending entries are unexpected here (should be Resolved or expired), + // but drain unconditionally to guarantee the map never leaks across turns. + self.pending_permissions.clear(); } Err(AcpError::IdleTimeout(_) | AcpError::HardTimeout { .. }) => { // Leave last_prompt_id and current_hard_deadline set — @@ -819,6 +971,10 @@ impl AcpClient { Err(_) => { self.last_prompt_id = None; self.current_hard_deadline = None; + // Non-recoverable error — drain the map to prevent capacity leak + // if the pool reuses this process (poisoned processes are respawned, + // but clean error exits may be returned to the pool). + self.pending_permissions.clear(); } } self.parse_stop_reason(&result?) @@ -1003,8 +1159,81 @@ impl AcpClient { AcpError::Protocol("cancel_with_cleanup called with no in-flight prompt".into()) })?; - // Step 1: respond to any pending permission request with "cancelled", - // but only if we haven't already responded (guards against double-response race). + // Check for poisoning first: if a permission write is in progress we + // must not send any more bytes to this process — return the dedicated + // error so `classify_control_cancel_failure` triggers respawn. + if self.permission_poisoned { + tracing::error!( + target: "acp::cancel", + "cancel on poisoned process — triggering respawn" + ); + return Err(AcpError::PermissionPoisoned); + } + + // Step 1: respond to any pending permission request with "cancelled". + // + // Under `ask` policy: drain all pending entries (cancel each one); + // check for any entry currently in `Writing` state → that's a + // cancel-during-write, so poison the process. + // + // Under `reject`/`allow` policy: use the old single-id path. + let mut cancel_during_write = false; + + // Ask-policy pending map: drain every Pending entry with cancelled; + // Writing entries poison the process. + let ids_to_cancel: Vec = self.pending_permissions.keys().cloned().collect(); + for req_id_str in ids_to_cancel { + let entry = self.pending_permissions.remove(&req_id_str).unwrap(); + match entry.state { + PermissionEntryState::Writing => { + tracing::error!( + target: "acp::cancel", + "cancel during permission write for req_id={req_id_str} — poisoning process" + ); + cancel_during_write = true; + // Don't try to write anything to this process. + } + PermissionEntryState::Pending => { + // Parse id back to JSON value for the wire response. + let perm_id: serde_json::Value = serde_json::from_str(&req_id_str) + .unwrap_or_else(|_| serde_json::Value::String(req_id_str.clone())); + let response = permission_response_cancelled(&perm_id); + if let Err(e) = self.write_ndjson_no_observe(&response).await { + tracing::warn!( + target: "acp::cancel", + "failed to write cancelled for pending perm id={req_id_str}: {e}" + ); + // Best-effort; continue to session/cancel. + } else { + // Emit one authorized acp_write with the original nonce + // so the desktop can retire the card by nonce correlation. + self.observe_authorized( + "acp_write", + AuthorizationEnvelope { + request_nonce: entry.nonce.clone(), + actionable: false, + reason: Some("cancelled".to_string()), + }, + response, + ); + tracing::debug!( + target: "acp::cancel", + "responded cancelled to pending permission id={req_id_str}" + ); + } + } + PermissionEntryState::Resolved => { + // Already resolved — nothing to do. + } + } + } + + if cancel_during_write { + self.permission_poisoned = true; + return Err(AcpError::PermissionPoisoned); + } + + // Old single-id path (reject/allow policy). if let Some(perm_id) = self.pending_permission_id.clone() { if !self.permission_responded { let response = permission_response_cancelled(&perm_id); @@ -1038,6 +1267,9 @@ impl AcpClient { remaining, ) .await?; + // Cancel completed — drain any remaining permission entries (they were + // answered with cancelled above, but drain Resolved ones to free capacity). + self.pending_permissions.clear(); self.parse_stop_reason(&result) } @@ -1045,7 +1277,26 @@ impl AcpClient { /// /// Bounded by a 30-second write timeout. If the agent stops reading stdin /// (e.g., it's stuck or dead), the write would otherwise block forever. + /// + /// Emits a generic `acp_write` observer event. For permission response paths + /// that emit their own authorized event, use `write_ndjson_no_observe`. async fn write_ndjson(&mut self, value: &serde_json::Value) -> Result<(), AcpError> { + self.write_ndjson_inner(value, true).await + } + + /// Write NDJSON without emitting a generic `acp_write` observer event. + /// + /// Used for permission response paths that emit a single authorized event + /// themselves — prevents duplicate generic+authorized telemetry. + async fn write_ndjson_no_observe(&mut self, value: &serde_json::Value) -> Result<(), AcpError> { + self.write_ndjson_inner(value, false).await + } + + async fn write_ndjson_inner( + &mut self, + value: &serde_json::Value, + emit_observe: bool, + ) -> Result<(), AcpError> { const WRITE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); let line = serde_json::to_string(value)?; tokio::time::timeout(WRITE_TIMEOUT, async { @@ -1057,7 +1308,9 @@ impl AcpClient { .await .map_err(|_| AcpError::WriteTimeout(WRITE_TIMEOUT))? .map_err(AcpError::Io)?; - self.observe("acp_write", value.clone()); + if emit_observe { + self.observe("acp_write", value.clone()); + } Ok(()) } @@ -1239,7 +1492,26 @@ impl AcpClient { self.handle_goose_usage_update(&msg); } "session/request_permission" => { - self.handle_permission_request(&msg).await?; + // Pre-turn (session/new) path: no decision arm installed. + // Force reject regardless of policy — ask requests would + // register map entries that can never be resolved without + // the turn reader's decision arm. + let saved_policy = self.permission_config.policy; + if matches!(saved_policy, PermissionPolicy::Ask) { + // Temporarily downgrade to reject for this request only. + let saved = std::mem::replace( + &mut self.permission_config.policy, + PermissionPolicy::Reject, + ); + let deadline = tokio::time::Instant::now() + + std::time::Duration::from_secs(PERMISSION_ASK_TIMEOUT_SECS); + let _ = self.handle_permission_request(&msg, true, deadline).await; + self.permission_config.policy = saved; + } else { + let deadline = tokio::time::Instant::now() + + std::time::Duration::from_secs(PERMISSION_ASK_TIMEOUT_SECS); + self.handle_permission_request(&msg, true, deadline).await?; + } } other => { // If the unknown message has an id, it's a request expecting a reply. @@ -1309,6 +1581,11 @@ impl AcpClient { // so the ack_tx oneshot is never leaked silently). let mut steer_rx = self.steer_rx.take(); + // Take the per-session permission decision receiver into a local for + // the same reason: `self.reader` and `decision_rx` cannot both be + // borrowed inside `select!` via `self`. + let mut decision_rx = self.permission_decision_rx.take(); + // Tracks the in-flight steer write: `(request_id, transport, ack_tx)`. // While `Some`, the steer arm is gated off so we don't stack writes, // and a response matching `id` is routed to the ack_tx instead @@ -1328,14 +1605,48 @@ impl AcpClient { let mut last_activity_at = now; loop { + // If the process was poisoned by a cancel-during-write, surface the + // error immediately so the caller can respawn. + if self.permission_poisoned { + if let Some((_, _, ack_tx)) = pending_steer.take() { + let _ = ack_tx.send(crate::pool::SteerAck::PromptCompletedNeutral); + } + return Err(AcpError::PermissionPoisoned); + } + // Determine which deadline fires first BEFORE sleeping — this is // the classification we'll use on timeout, immune to scheduler jitter. - let idle_fires_first = idle_deadline < hard_deadline; - let next_deadline = if idle_fires_first { - idle_deadline + // + // Deadline logic: + // - When any Pending permission entries exist, suspend the idle + // deadline (owner is deciding; agent silence is expected) and + // wake on the earliest permission deadline instead. + // - Otherwise wake on min(idle, hard) as normal. + let has_pending_permissions = self + .pending_permissions + .values() + .any(|e| matches!(e.state, PermissionEntryState::Pending)); + let next_deadline; + let idle_fires_first; + if has_pending_permissions { + // Suspend idle; find earliest permission deadline (capped by hard). + let earliest_perm = self + .pending_permissions + .values() + .filter(|e| matches!(e.state, PermissionEntryState::Pending)) + .map(|e| e.deadline) + .min() + .unwrap_or(hard_deadline); + next_deadline = earliest_perm.min(hard_deadline); + idle_fires_first = false; // hard deadline governs if we wake } else { - hard_deadline - }; + idle_fires_first = idle_deadline < hard_deadline; + next_deadline = if idle_fires_first { + idle_deadline + } else { + hard_deadline + }; + } // Pre-select deadline check — required by Max's review. Under // `biased`, a continuously-ready reader arm wins every poll and @@ -1345,20 +1656,79 @@ impl AcpClient { // exists). Check the classified deadline here so a steady- // stream agent is still bounded. if Instant::now() >= next_deadline { - if let Some((_, _, ack_tx)) = pending_steer.take() { - // Prompt is timing out — release the withheld event via - // PromptCompletedNeutral (no fallback signal: there is - // no in-flight turn to signal once we return, and - // normal dispatch handles redelivery). - let _ = ack_tx.send(crate::pool::SteerAck::PromptCompletedNeutral); + // When we woke for a permission deadline (not the hard deadline), + // skip the error return — let the expiry block below process the + // timed-out entries, then continue the loop. + let is_permission_wake = has_pending_permissions && next_deadline != hard_deadline; + if !is_permission_wake { + if let Some((_, _, ack_tx)) = pending_steer.take() { + // Prompt is timing out — release the withheld event via + // PromptCompletedNeutral (no fallback signal: there is + // no in-flight turn to signal once we return, and + // normal dispatch handles redelivery). + let _ = ack_tx.send(crate::pool::SteerAck::PromptCompletedNeutral); + } + if idle_fires_first { + tracing::warn!("idle timeout ({idle_timeout:?}) — no agent activity"); + return Err(AcpError::IdleTimeout(idle_timeout)); + } else { + let silence = Instant::now().saturating_duration_since(last_activity_at); + tracing::warn!("hard turn timeout exceeded (silence {silence:?})"); + return Err(AcpError::HardTimeout { silence }); + } } - if idle_fires_first { - tracing::warn!("idle timeout ({idle_timeout:?}) — no agent activity"); - return Err(AcpError::IdleTimeout(idle_timeout)); - } else { - let silence = Instant::now().saturating_duration_since(last_activity_at); - tracing::warn!("hard turn timeout exceeded (silence {silence:?})"); - return Err(AcpError::HardTimeout { silence }); + } + + // Expire any pending `ask` permission entries whose per-request + // deadline has passed. Fail closed: write denial response for each + // expired entry and transition to Resolved. + { + let now = Instant::now(); + let expired: Vec<(String, serde_json::Value, Vec, String)> = + self.pending_permissions + .iter() + .filter(|(_, e)| { + matches!(e.state, PermissionEntryState::Pending) && now >= e.deadline + }) + .map(|(id_str, e)| { + ( + id_str.clone(), + serde_json::from_str(id_str) + .unwrap_or_else(|_| serde_json::Value::String(id_str.clone())), + e.options_snapshot.clone(), + e.nonce.clone(), + ) + }) + .collect(); + for (id_str, id_val, opts, nonce) in expired { + tracing::warn!( + target: "acp::permission", + "ask timeout for permission id={id_val} — failing closed" + ); + // Transition to Resolved so cancel doesn't drain twice. + if let Some(entry) = self.pending_permissions.get_mut(&id_str) { + entry.state = PermissionEntryState::Resolved; + } + if let Ok(response) = permission_denial_response(&id_val, &opts) { + // Write the denial without the generic observer (avoids duplicate). + // Best-effort; ignore error (we're already timing out). + let write_ok = self.write_ndjson_no_observe(&response).await.is_ok(); + // Emit one authorized acp_write correlated by nonce so the + // desktop can retire the card. Only emitted when the write + // actually reached the pipe — otherwise emit nothing rather + // than claim a response was delivered. + if write_ok { + self.observe_authorized( + "acp_write", + AuthorizationEnvelope { + request_nonce: nonce, + actionable: false, + reason: Some("timed_out".to_string()), + }, + response, + ); + } + } } } @@ -1366,6 +1736,113 @@ impl AcpClient { // read level — the buffer never grows beyond the limit. let read_result = tokio::select! { biased; + // Decision arm — must be FIRST in the biased select! (spec §9) so + // owner decisions are not starved by a continuously-ready stdout. + // Cancel-safe: `mpsc::Receiver::recv` does not lose messages on drop. + Some(decision) = async { + match decision_rx.as_mut() { + Some(rx) => rx.recv().await, + None => None, + } + } => { + // Find the pending entry by nonce match. + let entry_id = self.pending_permissions + .iter() + .find(|(_, e)| { + matches!(e.state, PermissionEntryState::Pending) + && e.nonce == decision.request_nonce + }) + .map(|(k, _)| k.clone()); + + if let Some(id_str) = entry_id { + // Validate the chosen option_id is in the snapshot. + let opt_valid = self.pending_permissions + .get(&id_str) + .map(|e| { + e.options_snapshot.iter().any(|opt| { + opt.get("optionId") + .and_then(|v| v.as_str()) + == Some(decision.option_id.as_str()) + }) + }) + .unwrap_or(false); + + if !opt_valid { + tracing::warn!( + target: "acp::permission", + "permission_decision optionId {:?} not in snapshot for id={id_str} — ignoring", + decision.option_id + ); + } else { + // Transition Pending → Writing. + let (nonce, opts, id_val) = { + let entry = self.pending_permissions.get_mut(&id_str).unwrap(); + entry.state = PermissionEntryState::Writing; + ( + entry.nonce.clone(), + entry.options_snapshot.clone(), + serde_json::from_str::(&id_str) + .unwrap_or_else(|_| serde_json::Value::String(id_str.clone())), + ) + }; + + let response = permission_response_selected(&id_val, &decision.option_id); + // Write bounded by min(30s, remaining hard deadline). + let write_deadline = (Instant::now() + + std::time::Duration::from_secs(30)) + .min(hard_deadline); + let write_result = tokio::time::timeout_at(write_deadline, self.write_ndjson_no_observe(&response)).await; + + match write_result { + Ok(Ok(())) => { + // Transition Writing → Resolved. + if let Some(entry) = self.pending_permissions.get_mut(&id_str) { + entry.state = PermissionEntryState::Resolved; + } + // Emit single authorized acp_write after confirmed write. + self.observe_authorized( + "acp_write", + AuthorizationEnvelope { + request_nonce: nonce.clone(), + actionable: false, + reason: Some("applied".to_string()), + }, + response, + ); + let _ = opts; // used above for validation + tracing::info!( + target: "acp::permission", + "permission id={id_val} answered: optionId={:?}", + decision.option_id + ); + } + Ok(Err(write_err)) => { + // Write failed — poison the process. + tracing::error!( + target: "acp::permission", + "permission write failed for id={id_val}: {write_err} — poisoning process" + ); + self.permission_poisoned = true; + } + Err(_timeout) => { + // Write timed out — poison the process. + tracing::error!( + target: "acp::permission", + "permission write timed out for id={id_val} — poisoning process" + ); + self.permission_poisoned = true; + } + } + } + } else { + tracing::warn!( + target: "acp::permission", + "permission_decision nonce {:?} has no matching pending entry — ignoring", + decision.request_nonce + ); + } + None // loop back; don't set read_result + } read_result = self.reader.next() => Some(read_result), // Steer arm: gated off whenever a steer write is already in // flight so we don't stack two writes against the same @@ -1470,16 +1947,24 @@ impl AcpClient { // would catch this anyway, but firing the deadline arm // here makes the wakeup immediate (no extra reader poll // round-trip when stdout is idle). - if let Some((_, _, ack_tx)) = pending_steer.take() { - let _ = ack_tx.send(crate::pool::SteerAck::PromptCompletedNeutral); - } - if idle_fires_first { - tracing::warn!("idle timeout ({idle_timeout:?}) — no agent activity"); - return Err(AcpError::IdleTimeout(idle_timeout)); + // For a permission-deadline wake, loop back to let the + // expiry block process timed-out entries. + let is_permission_wake = + has_pending_permissions && next_deadline != hard_deadline; + if is_permission_wake { + None // loop back; expiry block will fire } else { - let silence = Instant::now().saturating_duration_since(last_activity_at); - tracing::warn!("hard turn timeout exceeded (silence {silence:?})"); - return Err(AcpError::HardTimeout { silence }); + if let Some((_, _, ack_tx)) = pending_steer.take() { + let _ = ack_tx.send(crate::pool::SteerAck::PromptCompletedNeutral); + } + if idle_fires_first { + tracing::warn!("idle timeout ({idle_timeout:?}) — no agent activity"); + return Err(AcpError::IdleTimeout(idle_timeout)); + } else { + let silence = Instant::now().saturating_duration_since(last_activity_at); + tracing::warn!("hard turn timeout exceeded (silence {silence:?})"); + return Err(AcpError::HardTimeout { silence }); + } } } }; @@ -1538,7 +2023,16 @@ impl AcpClient { continue; } }; - self.observe("acp_read", msg.clone()); + // Suppress the generic `acp_read` for `session/request_permission` + // under the `ask` policy — `handle_permission_request` emits the + // single enveloped frame instead (spec §6 "one frame per request"). + let is_ask_permission_request = + matches!(self.permission_config.policy, PermissionPolicy::Ask) + && msg.get("method").and_then(|v| v.as_str()) + == Some("session/request_permission"); + if !is_ask_permission_request { + self.observe("acp_read", msg.clone()); + } let activity_now = Instant::now(); idle_deadline = activity_now + idle_timeout; @@ -1683,7 +2177,12 @@ impl AcpClient { self.handle_goose_usage_update(&msg); } "session/request_permission" => { - self.handle_permission_request(&msg).await?; + self.handle_permission_request( + &msg, + is_ask_permission_request, + hard_deadline, + ) + .await?; } other => { // If the unknown message has an id, it's a request expecting a reply. @@ -1871,57 +2370,292 @@ impl AcpClient { } } - /// Reject a `session/request_permission` request from the agent. + /// Handle a `session/request_permission` request from the agent. /// - /// Buzz has no human permission prompt in this harness, so selecting - /// `allow_once` would turn any admitted prompt into an implicit approval. - /// Find `reject_once` by kind when the adapter offers it; otherwise use the - /// protocol's cancelled outcome, which is also fail-closed. + /// Dispatches based on the resolved permission policy: + /// - `reject` — deny via `reject_once`/`cancelled` (byte-for-byte old behaviour). + /// - `allow` — auto-select the unique validated `allow_once` option; fail closed. + /// - `ask` — register in the pending map, emit an actionable frame, and return. + /// The read loop's decision arm (added to `select!`) delivers the owner decision. + /// This call is intentionally **non-blocking** for `ask`; the actual response is + /// written asynchronously via the decision arm. /// - /// The request `id` is stored as `serde_json::Value` to support both numeric - /// and string IDs per JSON-RPC 2.0. - async fn handle_permission_request(&mut self, msg: &serde_json::Value) -> Result<(), AcpError> { + /// **Admission preflight (always runs before any policy dispatch):** + /// options nonempty, count ≤ PERMISSION_OPTIONS_MAX, every optionId unique + + /// nonempty, required kind/name fields present, no duplicate live requestId, + /// plaintext size ≤ OBSERVER_MAX_PLAINTEXT_LEN. Fail → immediate denial + emit + /// with `actionable: false`. + /// + /// Under `ask`, the generic pre-dispatch `acp_read` (acp.rs:1697 seam) is + /// **suppressed** for permission requests; this method emits the single + /// post-preflight enveloped frame instead. + /// + /// Returns `Ok(true)` when the caller should suppress the normal `acp_read` emit + /// (i.e. this method already emitted the enveloped frame), `Ok(false)` otherwise. + pub(crate) async fn handle_permission_request( + &mut self, + msg: &serde_json::Value, + // When `true`, caller has NOT yet emitted acp_read for this message — + // this method emits it (enveloped) for permission frames under `ask`. + // When `false` (read_until_response, non-idle path), the caller already + // emitted it; we must not double-emit. + caller_will_emit_read: bool, + // Hard deadline for the current turn. Used to bound per-request ask timeouts. + hard_deadline: tokio::time::Instant, + ) -> Result { // Extract id as a Value — JSON-RPC 2.0 allows both numeric and string IDs. let id = msg .get("id") .cloned() .ok_or_else(|| AcpError::Protocol("permission request missing id".into()))?; - // Store pending permission id so cancel_with_cleanup can respond to it. - self.pending_permission_id = Some(id.clone()); - // Mark as not yet responded — guards against double-response race. - self.permission_responded = false; + let options = match msg["params"]["options"].as_array() { + Some(o) => o.clone(), + None => { + // Missing options — emit non-actionable frame and deny. + let reason = "missing or non-array options field"; + tracing::warn!(target: "acp::permission", "{reason}, id={id}"); + self.emit_permission_read_non_actionable(&id, msg, reason, caller_will_emit_read); + let response = permission_denial_response(&id, &[])?; + self.write_ndjson(&response).await?; + return Ok(true); + } + }; + + // ── Admission preflight ──────────────────────────────────────────────── + let preflight_result = run_admission_preflight( + &options, + msg, + // Check for duplicate live requestId under ask. + if matches!(self.permission_config.policy, PermissionPolicy::Ask) { + let id_str = id.to_string(); + self.pending_permissions.contains_key(&id_str) + } else { + false + }, + if matches!(self.permission_config.policy, PermissionPolicy::Ask) { + self.pending_permissions.len() >= PERMISSION_MAP_CAP + } else { + false + }, + &self.observer_context, + self.observer_agent_index, + ); - let options = msg["params"]["options"] - .as_array() - .ok_or_else(|| AcpError::Protocol("permission request missing options".into()))?; + if let Err(reason) = preflight_result { + tracing::warn!(target: "acp::permission", "preflight failed: {reason}, id={id}"); + self.emit_permission_read_non_actionable(&id, msg, &reason, caller_will_emit_read); + let response = permission_denial_response(&id, &options)?; + self.write_ndjson(&response).await?; + return Ok(true); + } + // ── Preflight passed ─────────────────────────────────────────────────── tracing::debug!( target: "acp::permission", - "session/request_permission id={id}, {} options", - options.len() + "session/request_permission id={id}, {} options, policy={}", + options.len(), + self.permission_config.policy ); - let response = permission_denial_response(&id, options)?; + match self.permission_config.policy { + PermissionPolicy::Reject => { + // Byte-for-byte old behaviour: deny, track pending id for cancel. + self.pending_permission_id = Some(id.clone()); + self.permission_responded = false; + + // For reject, the caller already emitted acp_read unconditionally; + // emit a non-actionable authorization envelope alongside. + let nonce = new_permission_nonce(); + self.emit_permission_read_with_nonce( + &id, + msg, + &nonce, + false, + Some("policy=reject"), + caller_will_emit_read, + ); - // Write the response first, then mark as responded. - // - // Previous ordering (flag-before-write) was intended to guard against a - // double-response if a timeout fires between write and flag-set. However, - // the deadlock risk is worse: if write_ndjson fails (e.g. WriteTimeout), - // the flag would be true but no response was actually sent. Then - // cancel_with_cleanup would see permission_responded=true, skip sending - // the cancelled outcome, and the agent would hang waiting for a reply - // that never arrives — a guaranteed deadlock. - // - // The correct fix: set the flag AFTER a successful write. The double- - // response window (between write completion and flag-set) is negligibly - // small and bounded by a single memory store; the deadlock window was - // unbounded. - self.write_ndjson(&response).await?; - self.permission_responded = true; - self.pending_permission_id = None; - Ok(()) + let response = permission_denial_response(&id, &options)?; + self.write_ndjson(&response).await?; + self.permission_responded = true; + self.pending_permission_id = None; + Ok(true) + } + PermissionPolicy::Allow => { + // Auto-select the unique allow_once option; fail closed otherwise. + self.pending_permission_id = Some(id.clone()); + self.permission_responded = false; + + match select_allow_once(&options) { + Ok(option_id) => { + tracing::info!( + target: "acp::permission", + "allow: selecting allow_once optionId={option_id:?} for id={id}" + ); + let nonce = new_permission_nonce(); + // Emit enveloped acp_read (non-actionable: auto-approved). + self.emit_permission_read_with_nonce( + &id, + msg, + &nonce, + false, + Some("policy=allow; auto-approved"), + caller_will_emit_read, + ); + let response = permission_response_selected(&id, &option_id); + self.write_ndjson(&response).await?; + // Emit enveloped acp_write after confirmed write. + self.observe_authorized( + "acp_write", + AuthorizationEnvelope { + request_nonce: nonce, + actionable: false, + reason: Some("auto-approved by policy=allow".to_string()), + }, + response, + ); + self.permission_responded = true; + self.pending_permission_id = None; + } + Err(reason) => { + // Fail closed. + tracing::warn!( + target: "acp::permission", + "allow: fail closed — {reason}, id={id}" + ); + let nonce = new_permission_nonce(); + self.emit_permission_read_with_nonce( + &id, + msg, + &nonce, + false, + Some(&format!("policy=allow; fail closed: {reason}")), + caller_will_emit_read, + ); + let response = permission_denial_response(&id, &options)?; + self.write_ndjson(&response).await?; + self.permission_responded = true; + self.pending_permission_id = None; + } + } + Ok(true) + } + PermissionPolicy::Ask => { + // Availability gate (spec §10): `ask` requires both an active observer + // and a known owner. Without either, downgrade to `reject` with a loud + // warning — never sideways to `allow`. + let observer_active = self.observer.is_some(); + if !observer_active || !self.owner_pubkey_known { + tracing::warn!( + target: "acp::permission", + "ask policy unavailable (observer={}, owner_known={}) — downgrading to reject for id={id}", + observer_active, + self.owner_pubkey_known + ); + // Fall through to the Reject arm's logic. + self.pending_permission_id = Some(id.clone()); + self.permission_responded = false; + let nonce = new_permission_nonce(); + self.emit_permission_read_with_nonce( + &id, + msg, + &nonce, + false, + Some("policy=ask unavailable (no observer/owner); downgraded to reject"), + caller_will_emit_read, + ); + let response = permission_denial_response(&id, &options)?; + self.write_ndjson(&response).await?; + self.permission_responded = true; + self.pending_permission_id = None; + return Ok(true); + } + + // Register in the pending map and emit the actionable frame. + // The read loop's decision arm delivers the response asynchronously. + let id_str = id.to_string(); + let nonce = new_permission_nonce(); + + // Emit the single enveloped acp_read — suppresses the caller's + // generic emit via the Ok(true) return. + self.observe_authorized( + "acp_read", + AuthorizationEnvelope { + request_nonce: nonce.clone(), + actionable: true, + reason: None, + }, + msg.clone(), + ); + + // Per-request deadline: min(now + 300s, turn hard deadline). + let ask_deadline = tokio::time::Instant::now() + + std::time::Duration::from_secs(PERMISSION_ASK_TIMEOUT_SECS); + let entry_deadline = ask_deadline.min(hard_deadline); + + self.pending_permissions.insert( + id_str, + PermissionEntry { + nonce, + options_snapshot: options.clone(), + state: PermissionEntryState::Pending, + deadline: entry_deadline, + }, + ); + + // Do NOT set pending_permission_id for ask — the map is the + // sole source of truth. The legacy single-id slot is only used + // by reject/allow (synchronous paths). + Ok(true) + } + } + } + + /// Emit a non-actionable `acp_read` authorization frame for a permission request. + fn emit_permission_read_non_actionable( + &self, + id: &serde_json::Value, + msg: &serde_json::Value, + reason: &str, + _caller_will_emit_read: bool, + ) { + let nonce = new_permission_nonce(); + self.observe_authorized( + "acp_read", + AuthorizationEnvelope { + request_nonce: nonce, + actionable: false, + reason: Some(reason.to_string()), + }, + msg.clone(), + ); + tracing::debug!(target: "acp::permission", "non-actionable permission read id={id}"); + } + + /// Emit an `acp_read` with an authorization envelope. + /// + /// When `caller_will_emit_read` is `false` the caller already emitted the + /// raw `acp_read`; we emit only the enveloped version. When `true` we emit + /// the enveloped version (the caller suppresses its normal emit). + fn emit_permission_read_with_nonce( + &self, + _id: &serde_json::Value, + msg: &serde_json::Value, + nonce: &str, + actionable: bool, + reason: Option<&str>, + _caller_will_emit_read: bool, + ) { + self.observe_authorized( + "acp_read", + AuthorizationEnvelope { + request_nonce: nonce.to_string(), + actionable, + reason: reason.map(str::to_string), + }, + msg.clone(), + ); } /// Parse `stopReason` from a `session/prompt` result value. @@ -2040,9 +2774,16 @@ fn permission_denial_response( return Ok(permission_response_cancelled(id)); }; - let option_id = opt["optionId"] - .as_str() - .ok_or_else(|| AcpError::Protocol("reject_once option missing optionId".into()))?; + let Some(option_id) = opt["optionId"].as_str().filter(|s| !s.is_empty()) else { + // reject_once found but optionId is missing or empty — malformed request; + // fall back to `cancelled` rather than returning a Protocol error so the + // adapter still receives a valid JSON-RPC response. + tracing::warn!( + target: "acp::permission", + "reject_once option has missing or empty optionId for id={id}, cancelling" + ); + return Ok(permission_response_cancelled(id)); + }; tracing::info!( target: "acp::permission", "rejecting permission id={id} with reject_once optionId={option_id:?}" @@ -2050,6 +2791,175 @@ fn permission_denial_response( Ok(permission_response_selected(id, option_id)) } +/// Generate a cryptographically random, URL-safe nonce string. +/// +/// Used as the `requestNonce` in [`crate::observer::AuthorizationEnvelope`]. +/// The nonce is single-use and bound to a specific permission request. +fn new_permission_nonce() -> String { + uuid::Uuid::new_v4().to_string() +} + +/// Select the unique `allow_once` option from a permission request's option list. +/// +/// Returns `Ok(option_id)` when there is exactly one option with `kind = +/// "allow_once"` and a non-empty `optionId`. Returns `Err(reason)` (fail +/// closed) when: +/// - zero `allow_once` options are present, +/// - multiple `allow_once` options are present (ambiguous), +/// - the matching option has a missing or empty `optionId`. +/// +/// `allow_always` options are deliberately not selected — they would grant +/// indefinite access without a per-request human decision. +fn select_allow_once(options: &[serde_json::Value]) -> Result { + let candidates: Vec<&serde_json::Value> = options + .iter() + .filter(|opt| { + opt.get("kind") + .and_then(|k| k.as_str()) + .map(|k| k == "allow_once") + .unwrap_or(false) + }) + .collect(); + + match candidates.len() { + 0 => Err("no allow_once option found".to_string()), + 2.. => Err(format!( + "multiple allow_once options found ({}); ambiguous", + candidates.len() + )), + 1 => { + let opt = candidates[0]; + let option_id = opt + .get("optionId") + .and_then(|v| v.as_str()) + .filter(|s| !s.is_empty()) + .ok_or_else(|| "allow_once option has missing or empty optionId".to_string())?; + Ok(option_id.to_string()) + } + } +} + +/// Validate a `session/request_permission` request before it touches the +/// pending map or policy dispatch. +/// +/// Returns `Ok(())` on a clean request; `Err(reason)` on the first violation. +/// +/// Checks (in order): +/// 1. `options` nonempty. +/// 2. `options` count ≤ `PERMISSION_OPTIONS_MAX`. +/// 3. Every `optionId` is present and non-empty. +/// 4. Every `optionId` is unique across the request. +/// 5. Every option has a non-empty `kind` and `name`. +/// 6. Duplicate live `requestId` (only relevant under `ask`, caller passes flag). +/// 7. Permission map at capacity (only relevant under `ask`, caller passes flag). +/// 8. Full serialised `ObserverEvent` (raw payload + all envelope fields + real +/// context) fits within `OBSERVER_MAX_PLAINTEXT_LEN` — no leaf surgery on frames. +fn run_admission_preflight( + options: &[serde_json::Value], + msg: &serde_json::Value, + is_duplicate_id: bool, + is_map_at_cap: bool, + observer_context: &ObserverContext, + agent_index: Option, +) -> Result<(), String> { + // 1. options nonempty + if options.is_empty() { + return Err("options array is empty".to_string()); + } + + // 2. count ≤ PERMISSION_OPTIONS_MAX + if options.len() > PERMISSION_OPTIONS_MAX { + return Err(format!( + "too many options: {} > {}", + options.len(), + PERMISSION_OPTIONS_MAX + )); + } + + // 3 & 4. optionId present, non-empty, unique + let mut seen_ids = std::collections::HashSet::new(); + for opt in options { + let option_id = opt + .get("optionId") + .and_then(|v| v.as_str()) + .filter(|s| !s.is_empty()) + .ok_or_else(|| "option has missing or empty optionId".to_string())?; + if !seen_ids.insert(option_id) { + return Err(format!("duplicate optionId: {option_id:?}")); + } + } + + // 5. required kind and name fields + for opt in options { + if opt + .get("kind") + .and_then(|v| v.as_str()) + .filter(|s| !s.is_empty()) + .is_none() + { + return Err("option has missing or empty kind".to_string()); + } + if opt + .get("name") + .and_then(|v| v.as_str()) + .filter(|s| !s.is_empty()) + .is_none() + { + return Err("option has missing or empty name".to_string()); + } + } + + // 6. duplicate live requestId (ask only — caller computes flag) + if is_duplicate_id { + return Err("duplicate live requestId".to_string()); + } + + // 7. map at capacity (ask only — caller computes flag) + if is_map_at_cap { + return Err(format!( + "pending permission map at capacity ({})", + PERMISSION_MAP_CAP + )); + } + + // 8. Full annotated `ObserverEvent` fits within `OBSERVER_MAX_PLAINTEXT_LEN`. + // + // Construct the exact production `ObserverEvent` with the real observer context + // and a representative nonce. Serialise it and reject if over cap. This is the + // same construction path the observer uses at emit time, so any payload that + // passes here is guaranteed to fit in the final frame — no leaf surgery needed. + // + // A UUID nonce is used for sizing; the actual nonce is generated after the + // preflight passes, but all nonces are the same UUID length. + let candidate_event = ObserverEvent { + seq: u64::MAX, // worst-case seq (19 digits) + timestamp: "2026-01-01T00:00:00.000000000+00:00".to_string(), // max RFC3339 len + kind: "acp_read".to_string(), + agent_index, + channel_id: observer_context.channel_id.clone(), + session_id: observer_context.session_id.clone(), + turn_id: observer_context.turn_id.clone(), + started_at: observer_context.started_at.clone(), + authorization: Some(AuthorizationEnvelope { + // UUID nonce — all production nonces are this length. + request_nonce: "00000000-0000-0000-0000-000000000000".to_string(), + actionable: true, + reason: None, + }), + payload: msg.clone(), + }; + let annotated_len = serde_json::to_string(&candidate_event) + .map(|s| s.len()) + .unwrap_or(usize::MAX); + if annotated_len > OBSERVER_MAX_PLAINTEXT_LEN { + return Err(format!( + "permission request payload too large: annotated size {annotated_len} > {OBSERVER_MAX_PLAINTEXT_LEN}" + )); + } + + Ok(()) +} + /// Full `session/new` response — session ID plus the raw JSON result. /// /// Callers use the extractor helpers to pull model info from `raw`. @@ -2259,6 +3169,7 @@ fn configure_no_window(cmd: &mut tokio::process::Command) { #[cfg(test)] mod tests { use super::*; + use crate::config::ModeSource; #[test] fn stop_reason_parses_all_known_values() { @@ -2369,17 +3280,21 @@ mod tests { assert_eq!(outcome(&response), Some("cancelled")); } - /// A `reject_once` option missing its `optionId` is a protocol violation. - /// Erroring propagates to the caller, which tears the turn down — still no - /// approval is ever sent. + /// A `reject_once` option missing its `optionId` falls back to a `cancelled` + /// response rather than propagating a Protocol error. This ensures the adapter + /// always receives a valid JSON-RPC response, even for malformed requests. #[test] - fn reject_once_without_option_id_is_a_protocol_error() { + fn reject_once_without_option_id_falls_back_to_cancelled() { let options = options(r#"[{"name": "Reject", "kind": "reject_once"}]"#); - let err = permission_denial_response(&serde_json::json!(1), &options) - .expect_err("missing optionId must error"); + let response = permission_denial_response(&serde_json::json!(1), &options) + .expect("malformed reject_once must not error"); - assert!(matches!(err, AcpError::Protocol(_)), "got {err:?}"); + assert_eq!( + response["result"]["outcome"]["outcome"].as_str(), + Some("cancelled"), + "malformed reject_once must produce cancelled, got: {response}" + ); } #[test] @@ -4648,4 +5563,1288 @@ mod tests { "error must mention sandbox_workspace_write" ); } + + // ══════════════════════════════════════════════════════════════════════════ + // ── Permission policy: pinned tests (#4938) ─────────────────────────────── + // ══════════════════════════════════════════════════════════════════════════ + // + // Tests are grouped by the pinned requirement they cover, labelled as + // "Pinned §N" matching the spec's numbered list. + // + // These tests use: + // • `spawn_inert_client()` (cat) for pure unit coverage of `handle_permission_request`. + // • `spawn_script(s)` for end-to-end coverage of `read_until_response_with_idle_timeout`. + // • `AcpClient::set_permission_config` / `set_owner_pubkey_known` helpers. + // + // "observer" is left None for tests that only care about deny/allow path; + // an in-process observer is installed for tests that verify acp_write events. + + // ── Helpers ─────────────────────────────────────────────────────────────── + + /// Build a minimal `session/request_permission` JSON-RPC message. + fn perm_request(id: u64, options: &[(&str, &str, &str)]) -> serde_json::Value { + let opts: Vec = options + .iter() + .map(|(opt_id, kind, name)| { + serde_json::json!({"optionId": opt_id, "kind": kind, "name": name}) + }) + .collect(); + serde_json::json!({ + "jsonrpc": "2.0", + "id": id, + "method": "session/request_permission", + "params": { + "sessionId": "sess-test", + "options": opts, + } + }) + } + + /// Canonical 3-option set used in most tests. + fn default_opts() -> &'static [(&'static str, &'static str, &'static str)] { + &[ + ("opt-allow", "allow_once", "Allow once"), + ("opt-reject", "reject_once", "Reject once"), + ("opt-always", "allow_always", "Always allow"), + ] + } + + /// Set policy=allow on a client and mark owner known. + fn set_policy(client: &mut AcpClient, policy: PermissionPolicy) { + let config = ResolvedPermissionConfig::resolve(policy, None).expect("valid policy"); + client.set_permission_config(config); + client.set_owner_pubkey_known(true); + } + + // ── Pinned §2: allow selector — unique/zero/multiple/malformed ──────────── + + #[test] + fn allow_selector_picks_unique_allow_once() { + // Unique allow_once → Ok with that optionId. + let opts = serde_json::from_str::>( + r#"[{"optionId":"opt-a","kind":"allow_once","name":"Allow"}, + {"optionId":"opt-r","kind":"reject_once","name":"Reject"}]"#, + ) + .unwrap(); + assert_eq!(select_allow_once(&opts), Ok("opt-a".to_string())); + } + + #[test] + fn allow_selector_fails_closed_on_zero_allow_once() { + // No allow_once options → fail closed. + let opts = serde_json::from_str::>( + r#"[{"optionId":"opt-r","kind":"reject_once","name":"Reject"}]"#, + ) + .unwrap(); + assert!(select_allow_once(&opts).is_err()); + } + + #[test] + fn allow_selector_fails_closed_on_multiple_allow_once() { + // Two allow_once candidates → ambiguous, fail closed. + let opts = serde_json::from_str::>( + r#"[{"optionId":"opt-a1","kind":"allow_once","name":"A1"}, + {"optionId":"opt-a2","kind":"allow_once","name":"A2"}]"#, + ) + .unwrap(); + assert!(select_allow_once(&opts).is_err()); + } + + #[test] + fn allow_selector_fails_closed_on_missing_option_id() { + // allow_once present but optionId absent → malformed, fail closed. + let opts = serde_json::from_str::>( + r#"[{"kind":"allow_once","name":"Allow"}]"#, + ) + .unwrap(); + assert!(select_allow_once(&opts).is_err()); + } + + #[test] + fn allow_selector_never_selects_allow_always() { + // allow_always must NOT be selected even when it is the only option + // with an "allow" kind — indefinite access without per-request approval. + let opts = serde_json::from_str::>( + r#"[{"optionId":"opt-aa","kind":"allow_always","name":"Always"}]"#, + ) + .unwrap(); + assert!( + select_allow_once(&opts).is_err(), + "allow_always must never be auto-selected" + ); + } + + // ── Pinned §3: duplicate option IDs ────────────────────────────────────── + + #[test] + fn admission_preflight_rejects_duplicate_option_ids() { + let msg = perm_request( + 1, + &[("dup", "allow_once", "A"), ("dup", "reject_once", "R")], + ); + let opts = msg["params"]["options"].as_array().unwrap().clone(); + let result = + run_admission_preflight(&opts, &msg, false, false, &ObserverContext::default(), None); + assert!(result.is_err(), "duplicate optionId must fail preflight"); + let reason = result.unwrap_err(); + assert!( + reason.contains("duplicate optionId"), + "reason must name the check, got: {reason}" + ); + } + + // ── Pinned §2: duplicate request ID ────────────────────────────────────── + + #[tokio::test] + async fn handle_permission_request_denies_duplicate_live_request_id() { + // Under ask policy, a second request with the same id while the first + // is still pending must be denied immediately without disturbing the original. + let mut client = spawn_inert_client().await; + set_policy(&mut client, PermissionPolicy::Ask); + // Simulate an already-registered pending entry with the same id. + client.pending_permissions.insert( + "1".to_string(), + PermissionEntry { + nonce: "nonce-abc".to_string(), + options_snapshot: vec![], + state: PermissionEntryState::Pending, + deadline: tokio::time::Instant::now() + std::time::Duration::from_secs(300), + }, + ); + let msg = perm_request(1, default_opts()); + let hard_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(30); + let result = client + .handle_permission_request(&msg, true, hard_deadline) + .await; + // Must succeed (Ok) — denial was written and the call itself doesn't error. + assert!( + result.is_ok(), + "duplicate-id must not propagate as Err, got {result:?}" + ); + // The original entry must still be in the map, untouched. + assert!( + client.pending_permissions.contains_key("1"), + "original pending entry must survive the duplicate-id rejection" + ); + // Only one entry should exist (the duplicate was denied, not registered). + assert_eq!( + client.pending_permissions.len(), + 1, + "no new entry should be added for the duplicate id" + ); + } + + // ── Pinned §4: oversize subject → plaintext cap exceeded ───────────────── + + #[test] + fn admission_preflight_rejects_oversize_msg_exceeding_plaintext_cap() { + // Construct a message large enough to exceed OBSERVER_MAX_PLAINTEXT_LEN. + // We embed the large payload directly in the msg so that + // `serde_json::to_string(msg).len() > OBSERVER_MAX_PLAINTEXT_LEN`. + let oversize_subject = "x".repeat(OBSERVER_MAX_PLAINTEXT_LEN + 1); + let msg = serde_json::json!({ + "jsonrpc": "2.0", + "id": 42, + "method": "session/request_permission", + "params": { + "sessionId": "sess", + "subject": oversize_subject, + "options": [{"optionId":"opt","kind":"allow_once","name":"A"}] + } + }); + let opts = vec![serde_json::json!({"optionId":"opt","kind":"allow_once","name":"A"})]; + let result = + run_admission_preflight(&opts, &msg, false, false, &ObserverContext::default(), None); + assert!(result.is_err(), "oversize msg must fail preflight"); + let reason = result.unwrap_err(); + assert!( + reason.contains("too large") || reason.contains("payload"), + "reason should mention payload size, got: {reason}" + ); + } + + #[test] + fn admission_preflight_rejects_payload_overflowing_after_full_event_construction() { + // Construct a context matching production (UUID-sized IDs) and compute the + // maximum msg payload that fits within OBSERVER_MAX_PLAINTEXT_LEN when + // serialised as the actual ObserverEvent. Then submit a payload one byte + // larger and verify the preflight rejects it. + // + // This exercises the production code path: the check constructs the + // exact ObserverEvent with real context fields, not an estimate. + use crate::observer::ObserverContext; + + let ctx = ObserverContext { + channel_id: Some("00000000-0000-0000-0000-000000000000".to_string()), + session_id: Some("sess-00000000-0000-0000-0000-000000000000".to_string()), + turn_id: Some("00000000-0000-0000-0000-000000000000".to_string()), + started_at: Some("2026-01-01T00:00:00.000000000+00:00".to_string()), + }; + + // Binary-search for the exact max subject length that still fits. + // We wrap it in a minimal msg structure to simulate a real request. + let template = |subject: &str| { + serde_json::json!({ + "jsonrpc": "2.0", + "id": 42, + "method": "session/request_permission", + "params": { + "sessionId": "sess", + "subject": subject, + "options": [{"optionId":"opt","kind":"allow_once","name":"A"}] + } + }) + }; + let opts = vec![serde_json::json!({"optionId":"opt","kind":"allow_once","name":"A"})]; + // Build the ObserverEvent exactly as the preflight does to find where the + // boundary is — then make a msg one byte over that boundary. + let make_candidate = |msg: &serde_json::Value| ObserverEvent { + seq: u64::MAX, + timestamp: "2026-01-01T00:00:00.000000000+00:00".to_string(), + kind: "acp_read".to_string(), + agent_index: None, + channel_id: ctx.channel_id.clone(), + session_id: ctx.session_id.clone(), + turn_id: ctx.turn_id.clone(), + started_at: ctx.started_at.clone(), + authorization: Some(AuthorizationEnvelope { + request_nonce: "00000000-0000-0000-0000-000000000000".to_string(), + actionable: true, + reason: None, + }), + payload: msg.clone(), + }; + + // Find a subject length that overflows after event wrapping. + // Start with a large subject known to overflow (cap worth of padding). + let overflow_subject = "z".repeat(OBSERVER_MAX_PLAINTEXT_LEN); + let overflow_msg = template(&overflow_subject); + let overflow_event_len = serde_json::to_string(&make_candidate(&overflow_msg)) + .unwrap() + .len(); + assert!( + overflow_event_len > OBSERVER_MAX_PLAINTEXT_LEN, + "test setup: overflow_event_len ({overflow_event_len}) must exceed cap" + ); + + // The preflight must reject this payload. + let result = run_admission_preflight(&opts, &overflow_msg, false, false, &ctx, None); + assert!( + result.is_err(), + "payload overflowing after event construction must fail preflight (event_len={overflow_event_len})" + ); + let reason = result.unwrap_err(); + assert!( + reason.contains("too large") || reason.contains("payload"), + "reason should mention payload size, got: {reason}" + ); + + // Sanity-check: an empty subject (tiny msg) must pass the preflight. + let tiny_msg = template(""); + let tiny_event_len = serde_json::to_string(&make_candidate(&tiny_msg)) + .unwrap() + .len(); + assert!( + tiny_event_len <= OBSERVER_MAX_PLAINTEXT_LEN, + "test setup: tiny_event_len ({tiny_event_len}) must be within cap" + ); + let ok_result = run_admission_preflight(&opts, &tiny_msg, false, false, &ctx, None); + assert!( + ok_result.is_ok(), + "small payload must pass preflight, got: {ok_result:?}" + ); + } + + #[test] + fn denial_response_with_malformed_reject_once_falls_back_to_cancelled() { + // A reject_once option with a missing optionId must produce a `cancelled` + // response, not a Protocol error — the adapter must always receive a valid + // JSON-RPC response. + let id = serde_json::json!(7); + let opts = vec![ + serde_json::json!({"kind": "reject_once", "name": "Reject"}), // no optionId + ]; + let response = permission_denial_response(&id, &opts) + .expect("malformed reject_once must not return Err"); + // The response must be a cancelled frame (no optionId in result.outcome). + let outcome = &response["result"]["outcome"]; + assert_eq!( + outcome["outcome"].as_str(), + Some("cancelled"), + "malformed reject_once must produce cancelled response, got: {response}" + ); + } + + // ── Pinned §5: map overflow ─────────────────────────────────────────────── + + #[tokio::test] + async fn handle_permission_request_denies_when_map_at_capacity() { + let mut client = spawn_inert_client().await; + set_policy(&mut client, PermissionPolicy::Ask); + + // Fill the map to PERMISSION_MAP_CAP. + for i in 0..PERMISSION_MAP_CAP { + client.pending_permissions.insert( + format!("{i}"), + PermissionEntry { + nonce: format!("nonce-{i}"), + options_snapshot: vec![], + state: PermissionEntryState::Pending, + deadline: tokio::time::Instant::now() + std::time::Duration::from_secs(300), + }, + ); + } + assert_eq!(client.pending_permissions.len(), PERMISSION_MAP_CAP); + + // One more request with a new id → must be denied. + let msg = perm_request(99, default_opts()); + let hard_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(30); + let result = client + .handle_permission_request(&msg, true, hard_deadline) + .await; + assert!( + result.is_ok(), + "map-at-cap must not propagate Err, got {result:?}" + ); + // Map must not have grown. + assert_eq!( + client.pending_permissions.len(), + PERMISSION_MAP_CAP, + "map must not grow beyond capacity after denial" + ); + } + + // ── Pinned §7: mode matrix — unset + every explicit mode × 3 policies ──── + + #[test] + fn resolved_permission_config_reject_unset_derives_dont_ask() { + let cfg = ResolvedPermissionConfig::resolve(PermissionPolicy::Reject, None).unwrap(); + assert_eq!(cfg.effective_mode, PermissionMode::DontAsk); + assert_eq!(cfg.mode_source, ModeSource::Derived); + assert!(cfg.transmit_mode, "transmit_mode must always be true"); + } + + #[test] + fn resolved_permission_config_ask_unset_derives_default() { + let cfg = ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).unwrap(); + assert_eq!(cfg.effective_mode, PermissionMode::Default); + assert_eq!(cfg.mode_source, ModeSource::Derived); + } + + #[test] + fn resolved_permission_config_allow_unset_derives_default_not_dont_ask() { + // allow + unset → default (NOT dontAsk — dontAsk self-denies before Buzz can answer) + let cfg = ResolvedPermissionConfig::resolve(PermissionPolicy::Allow, None).unwrap(); + assert_eq!(cfg.effective_mode, PermissionMode::Default); + assert!( + cfg.effective_mode != PermissionMode::DontAsk, + "allow policy must NOT derive dontAsk" + ); + } + + #[test] + fn resolved_permission_config_reject_plus_explicit_dont_ask_is_ok() { + // reject + dontAsk explicit is valid: both say "deny". + let cfg = ResolvedPermissionConfig::resolve( + PermissionPolicy::Reject, + Some(PermissionMode::DontAsk), + ) + .unwrap(); + assert_eq!(cfg.effective_mode, PermissionMode::DontAsk); + assert_eq!(cfg.mode_source, ModeSource::Explicit); + } + + #[test] + fn resolved_permission_config_ask_plus_explicit_dont_ask_is_startup_error() { + let result = + ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, Some(PermissionMode::DontAsk)); + assert!(result.is_err(), "ask + dontAsk must be a startup error"); + let msg = format!("{}", result.unwrap_err()); + assert!( + msg.contains("dontAsk"), + "error must mention dontAsk, got: {msg}" + ); + } + + #[test] + fn resolved_permission_config_allow_plus_explicit_dont_ask_is_startup_error() { + let result = ResolvedPermissionConfig::resolve( + PermissionPolicy::Allow, + Some(PermissionMode::DontAsk), + ); + assert!(result.is_err(), "allow + dontAsk must be a startup error"); + } + + #[test] + fn resolved_permission_config_ask_plus_explicit_accept_edits_is_ok() { + let cfg = ResolvedPermissionConfig::resolve( + PermissionPolicy::Ask, + Some(PermissionMode::AcceptEdits), + ) + .unwrap(); + assert_eq!(cfg.effective_mode, PermissionMode::AcceptEdits); + assert_eq!(cfg.mode_source, ModeSource::Explicit); + } + + #[test] + fn resolved_permission_config_allow_plus_explicit_plan_is_ok() { + let cfg = + ResolvedPermissionConfig::resolve(PermissionPolicy::Allow, Some(PermissionMode::Plan)) + .unwrap(); + assert_eq!(cfg.effective_mode, PermissionMode::Plan); + assert_eq!(cfg.mode_source, ModeSource::Explicit); + } + + #[test] + fn resolved_permission_config_transmit_mode_always_true() { + // transmit_mode is always true regardless of policy/mode combination. + for policy in [ + PermissionPolicy::Reject, + PermissionPolicy::Ask, + PermissionPolicy::Allow, + ] { + let cfg = ResolvedPermissionConfig::resolve(policy, None).unwrap(); + assert!(cfg.transmit_mode, "transmit_mode must be true for {policy}"); + } + } + + // ── Pinned §10: ask availability gate — no observer → downgrade to reject ─ + + #[tokio::test] + async fn ask_without_observer_downgrades_to_reject() { + // ask policy but no observer installed → must downgrade to reject, + // never sideways to allow. + let mut client = spawn_inert_client().await; + let config = ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).unwrap(); + client.set_permission_config(config); + client.set_owner_pubkey_known(true); + // No observer installed (default). + + let msg = perm_request(1, default_opts()); + let hard_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(30); + let result = client + .handle_permission_request(&msg, true, hard_deadline) + .await; + // Denial was written — Ok(true) means caller should suppress generic emit. + assert!( + result.is_ok(), + "ask downgrade to reject must not propagate Err" + ); + // Confirm nothing was left pending in the map — it was denied synchronously. + assert!( + client.pending_permissions.is_empty(), + "downgraded-to-reject must not leave a pending entry" + ); + } + + #[tokio::test] + async fn ask_without_owner_known_downgrades_to_reject() { + // ask policy with observer but unknown owner → downgrade to reject. + let mut client = spawn_inert_client().await; + let config = ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).unwrap(); + client.set_permission_config(config); + client.set_owner_pubkey_known(false); // explicitly unknown + + let msg = perm_request(2, default_opts()); + let hard_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(30); + let result = client + .handle_permission_request(&msg, true, hard_deadline) + .await; + assert!(result.is_ok()); + assert!(client.pending_permissions.is_empty()); + } + + // ── Pinned §1: ask path success — decision arrives → response written ───── + // + // This test verifies the biased select! decision arm: + // 1. permission request emitted on stdout + // 2. decision injected via permission_decision_tx + // 3. read loop writes the permission response + // 4. loop continues and the final id=999 response is matched → Ok + + #[tokio::test] + async fn ask_decision_consumed_writes_response_and_continues() { + // Setup: ask policy, observer + owner active, permission_decision channel installed. + // A Pending entry is pre-planted with a known nonce so we can deliver a matching + // decision without needing access to nonce generation inside the loop. + // The script immediately emits the terminal id=999 response (simulating the + // adapter continuing after the permission response was written to its stdin). + let script = r#"echo '{"jsonrpc":"2.0","id":999,"result":{"done":true}}'"#; + let mut client = spawn_script(script).await; + + let config = ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).unwrap(); + client.set_permission_config(config); + client.set_owner_pubkey_known(true); + let obs = crate::observer::ObserverHandle::in_process(); + client.set_observer(Some(obs.clone()), 0); + + let (perm_tx, perm_rx) = tokio::sync::mpsc::channel::(8); + client.install_permission_decision_rx(perm_rx); + + // Plant a Pending entry with a known nonce. + let known_nonce = "test-nonce-loop-success".to_string(); + let req_id_str = "42".to_string(); + client.pending_permissions.insert( + req_id_str.clone(), + PermissionEntry { + nonce: known_nonce.clone(), + options_snapshot: vec![ + serde_json::json!({"optionId":"opt-allow","kind":"allow_once","name":"Allow"}), + ], + state: PermissionEntryState::Pending, + deadline: tokio::time::Instant::now() + std::time::Duration::from_secs(300), + }, + ); + + // Deliver a matching decision (by nonce) with a valid optionId. + // The decision is already in the channel before the loop starts; the biased + // select! arm reads it on the first iteration. + perm_tx + .send(PermissionDecision { + request_nonce: known_nonce, + option_id: "opt-allow".to_string(), + }) + .await + .unwrap(); + + // Drive the loop. It should: (1) find the pre-delivered decision, write the + // permission response, transition entry → Resolved; (2) continue and read the + // id=999 terminal response from the script. + let idle = std::time::Duration::from_secs(5); + let max_dur = std::time::Duration::from_secs(10); + let hard_deadline = tokio::time::Instant::now() + max_dur; + let result = client + .read_until_response_with_idle_timeout( + "sess-ask-success", + 999, + idle, + hard_deadline, + max_dur, + ) + .await; + + assert!( + result.is_ok(), + "loop must succeed after decision is consumed, got: {result:?}" + ); + + // The entry must have been transitioned to Resolved (decision was applied). + let entry = client.pending_permissions.get(&req_id_str); + match entry { + Some(e) => assert!( + matches!(e.state, PermissionEntryState::Resolved), + "entry must be Resolved after decision applied, got: {:?}", + e.state + ), + None => { + // Entry may have been drained at turn end — also acceptable. + } + } + } + + // ── Production-path tests: real loop emits request, captures nonce ────── + + /// Full end-to-end production path test for the `ask` decision flow: + /// + /// 1. Script emits a real `session/request_permission` on stdout. + /// 2. The read loop processes it via `handle_permission_request()` — + /// no state is pre-planted. + /// 3. The nonce is captured from the observer. + /// 4. A valid decision is sent through the decision channel. + /// 5. The loop writes the permission response to the script's stdin. + /// 6. The script reads the response and emits the terminal id=999 reply. + /// 7. The loop returns `Ok` — the wire flow completes end-to-end. + #[tokio::test] + async fn ask_production_path_emits_request_captures_nonce_and_delivers_decision() { + // Script: emit permission request, wait for any stdin line (the harness's + // response), then emit the terminal session/prompt response. + let perm_req = r#"{"jsonrpc":"2.0","id":42,"method":"session/request_permission","params":{"sessionId":"sess","requestId":"req-prod","subject":"read a file","options":[{"optionId":"opt-allow","kind":"allow_once","name":"Allow"},{"optionId":"opt-deny","kind":"reject_once","name":"Deny"}]}}"#; + let terminal = r#"{"jsonrpc":"2.0","id":999,"result":{"stopReason":"end_turn"}}"#; + // Print the permission request, wait for one line of stdin (the harness's + // response), then print the terminal response. + let script = format!( + r#"printf '{perm_req}\n'; read -r _resp; printf '{terminal}\n'"#, + perm_req = perm_req, + terminal = terminal, + ); + + let mut client = spawn_script(&script).await; + let config = ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).unwrap(); + client.set_permission_config(config); + client.set_owner_pubkey_known(true); + + // Subscribe to the observer BEFORE starting the loop so we capture all events. + let obs = crate::observer::ObserverHandle::in_process(); + let mut obs_rx = obs.subscribe(); + client.set_observer(Some(obs.clone()), 0); + + let (perm_tx, perm_rx) = tokio::sync::mpsc::channel::(8); + client.install_permission_decision_rx(perm_rx); + + // Spawn a task that waits for the observer to emit the actionable acp_read + // (the permission request), then delivers a matching decision. + let decision_task = tokio::spawn(async move { + // Wait for the actionable acp_read from the observer. + let mut found_nonce: Option = None; + while let Ok(Ok(event)) = + tokio::time::timeout(std::time::Duration::from_secs(5), obs_rx.recv()).await + { + if event.kind == "acp_read" { + if let Some(auth) = &event.authorization { + if auth.actionable { + found_nonce = Some(auth.request_nonce.clone()); + break; + } + } + } + } + let nonce = found_nonce.expect("actionable acp_read must be emitted"); + // Deliver a valid decision by the captured nonce. + perm_tx + .send(PermissionDecision { + request_nonce: nonce, + option_id: "opt-allow".to_string(), + }) + .await + .expect("decision channel must accept"); + }); + + let idle = std::time::Duration::from_secs(5); + let max_dur = std::time::Duration::from_secs(15); + let hard_deadline = tokio::time::Instant::now() + max_dur; + let result = client + .read_until_response_with_idle_timeout("sess", 999, idle, hard_deadline, max_dur) + .await; + + assert!( + result.is_ok(), + "production-path ask loop must succeed after decision is delivered, got: {result:?}" + ); + assert_eq!( + result.unwrap().get("stopReason").and_then(|v| v.as_str()), + Some("end_turn"), + ); + + // Verify the observer emitted an authorized acp_write (the decision response). + let _ = decision_task.await; + let events = obs.snapshot(); + let write_events: Vec<_> = events + .iter() + .filter(|e| e.kind == "acp_write" && e.authorization.is_some()) + .collect(); + assert!( + !write_events.is_empty(), + "observer must emit at least one authorized acp_write after decision applied" + ); + } + + /// Cancel test: asserts exactly one JSON-RPC response per pending id, + /// no replay on subsequent cancel. + #[tokio::test] + async fn cancel_writes_exactly_one_response_per_pending_id_no_replay() { + // Script that stays alive but produces no output (simulates a hung agent). + let mut client = spawn_script("sleep 5").await; + client.set_permission_config( + ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).unwrap(), + ); + client.set_owner_pubkey_known(true); + + // Subscribe to observer to capture writes. + let obs = crate::observer::ObserverHandle::in_process(); + client.set_observer(Some(obs.clone()), 0); + + let (_tx, perm_rx) = tokio::sync::mpsc::channel::(8); + client.install_permission_decision_rx(perm_rx); + + // Plant two distinct Pending entries directly — this tests the cancel + // drain path without needing a live protocol exchange. + for i in 0..2u64 { + let hard_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(300); + let msg = perm_request(i, default_opts()); + client + .handle_permission_request(&msg, true, hard_deadline) + .await + .expect("ask registration must succeed"); + } + assert_eq!( + client.pending_permissions.len(), + 2, + "two pending entries must be registered before cancel" + ); + client.last_prompt_id = Some(999); + + // First cancel: must drain both entries. + let _ = client + .cancel_with_cleanup_grace("sess-exact-once", std::time::Duration::from_millis(200)) + .await; + assert!( + client.pending_permissions.is_empty(), + "all pending entries must be drained after cancel" + ); + + // Count authorized acp_write events (each must correspond to one drained entry). + let events_after_first = obs.snapshot(); + let write_count_first = events_after_first + .iter() + .filter(|e| e.kind == "acp_write" && e.authorization.is_some()) + .count(); + assert_eq!( + write_count_first, 2, + "cancel must emit exactly one authorized acp_write per pending id (got {write_count_first})" + ); + + // Second cancel on the same client: no pending entries remain, must not + // re-emit any additional acp_write (no replay). + let _ = client + .cancel_with_cleanup_grace("sess-exact-once", std::time::Duration::from_millis(200)) + .await; + let events_after_second = obs.snapshot(); + let write_count_second = events_after_second + .iter() + .filter(|e| e.kind == "acp_write" && e.authorization.is_some()) + .count(); + assert_eq!( + write_count_second, write_count_first, + "second cancel must not emit additional acp_writes (no replay): before={write_count_first}, after={write_count_second}" + ); + } + + /// Paused-time test: the permission deadline fires at exactly 300 seconds, + /// idle is suspended while a Pending entry exists, and capacity recovers + /// after more than eight sequential requests. + #[tokio::test(start_paused = true)] + async fn ask_permission_deadline_idle_suspension_and_capacity_recovery() { + // Script that emits nothing (simulates an agent waiting for permission response). + let mut client = spawn_script("sleep 600").await; + let config = ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).unwrap(); + client.set_permission_config(config); + client.set_owner_pubkey_known(true); + let obs = crate::observer::ObserverHandle::in_process(); + client.set_observer(Some(obs.clone()), 0); + let (_tx, perm_rx) = tokio::sync::mpsc::channel::(8); + client.install_permission_decision_rx(perm_rx); + + // ── Part 1: permission deadline fires before idle ──────────────────── + // Register one pending entry. + let msg = perm_request(1, default_opts()); + let hard_deadline = tokio::time::Instant::now() + + std::time::Duration::from_secs(PERMISSION_ASK_TIMEOUT_SECS + 10); + client + .handle_permission_request(&msg, true, hard_deadline) + .await + .expect("ask registration must succeed"); + assert_eq!(client.pending_permissions.len(), 1); + + // Drive the loop with a long idle timeout — idle must be SUSPENDED while + // the permission entry is pending; only the 300s permission deadline fires. + let idle = std::time::Duration::from_secs(5); // would fire immediately without suspension + let max_dur = std::time::Duration::from_secs(PERMISSION_ASK_TIMEOUT_SECS + 10); + let hard_deadline = tokio::time::Instant::now() + max_dur; + + // Advance time to just before the 300s deadline — idle should NOT fire. + tokio::time::advance(std::time::Duration::from_secs( + PERMISSION_ASK_TIMEOUT_SECS - 1, + )) + .await; + // Spawn a task to advance time past the deadline and check the loop exits + // via permission expiry (not idle timeout). + let advance_task = tokio::spawn(async { + tokio::time::advance(std::time::Duration::from_secs(2)).await; + }); + + let result = client + .read_until_response_with_idle_timeout("sess-tdl", 999, idle, hard_deadline, max_dur) + .await; + let _ = advance_task.await; + + // The loop must have processed the expired entry (transitioned to Resolved) + // and then continued. Because the script produces no output, after the + // permission entry expires the idle timeout fires next (5s). + // Either an IdleTimeout or HardTimeout is acceptable — the key check is + // that no PermissionPoisoned or unexpected error occurred AND the entry + // was processed (Resolved or drained). + assert!( + !matches!(result, Err(AcpError::PermissionPoisoned)), + "permission expiry must not poison the process, got: {result:?}" + ); + // After the deadline, the entry must have been transitioned to Resolved. + let entry_state = client.pending_permissions.get("1"); + let was_resolved = entry_state + .map(|e| matches!(e.state, PermissionEntryState::Resolved)) + .unwrap_or(true); // drain on turn exit is also acceptable + assert!( + was_resolved, + "entry must be Resolved or drained after permission deadline, got: {entry_state:?}" + ); + + // ── Part 2: capacity recovery after 8 sequential requests ─────────── + // Clear any stale entries and verify 8+ sequential requests can succeed + // when previous resolved entries are drained between turns. + let mut client2 = spawn_script("sleep 600").await; + client2.set_permission_config( + ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).unwrap(), + ); + client2.set_owner_pubkey_known(true); + let obs2 = crate::observer::ObserverHandle::in_process(); + client2.set_observer(Some(obs2), 0); + let (perm_tx2, perm_rx2) = tokio::sync::mpsc::channel::(16); + client2.install_permission_decision_rx(perm_rx2); + + // Send 9 sequential requests, processing each before sending the next. + // The map is bounded at PERMISSION_MAP_CAP = 8, but Resolved entries do + // not count toward the live-entry cap check — only Pending ones do. + // After each decision is applied (Resolved), the next request must succeed. + for i in 0..9u64 { + let hard = tokio::time::Instant::now() + std::time::Duration::from_secs(300); + let msg = perm_request(i + 100, default_opts()); + let result = client2.handle_permission_request(&msg, true, hard).await; + // If still Pending from prior iterations, the cap check blocks — this + // tests the case after decisions have been applied (Resolved). + // For this sequential test we deliver decisions immediately. + if result.is_ok() && result.unwrap() { + // Entry is now Pending; deliver a decision immediately. + // Capture the nonce from the freshly-inserted entry. + let id_str = (i + 100).to_string(); + let nonce = client2 + .pending_permissions + .get(&id_str) + .map(|e| e.nonce.clone()); + if let Some(nonce) = nonce { + perm_tx2 + .send(PermissionDecision { + request_nonce: nonce, + option_id: "opt-allow".to_string(), + }) + .await + .ok(); + } + } + } + // Drive the loop to process all queued decisions. + let hard = tokio::time::Instant::now() + std::time::Duration::from_secs(10); + let _ = tokio::time::timeout( + std::time::Duration::from_millis(500), + client2.read_until_response_with_idle_timeout( + "sess-cap", + 9999, + std::time::Duration::from_millis(100), + hard, + std::time::Duration::from_secs(10), + ), + ) + .await; + // After processing, no Pending entries should remain (all should be Resolved + // or the map may have been drained). This proves the capacity map doesn't + // permanently block after 8 requests. + let pending_count = client2 + .pending_permissions + .values() + .filter(|e| matches!(e.state, PermissionEntryState::Pending)) + .count(); + assert_eq!( + pending_count, 0, + "no Pending entries must remain after all decisions applied (capacity recovery confirmed)" + ); + } + + // ── Pinned §1 (simpler): ask entry registered synchronously ────────────── + + #[tokio::test] + async fn ask_registers_entry_in_pending_map() { + // Verify that handle_permission_request under ask policy inserts + // a Pending entry into the map (without needing a live decision loop). + let mut client = spawn_inert_client().await; + let config = ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).unwrap(); + client.set_permission_config(config); + client.set_owner_pubkey_known(true); + // Install an observer so the ask arm doesn't downgrade. + let obs = crate::observer::ObserverHandle::in_process(); + client.set_observer(Some(obs), 0); + // Install a permission decision channel (must be installed or take() panics). + let (_perm_tx, perm_rx) = tokio::sync::mpsc::channel::(8); + client.install_permission_decision_rx(perm_rx); + + let msg = perm_request(42, default_opts()); + let hard_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(30); + let result = client + .handle_permission_request(&msg, true, hard_deadline) + .await; + assert!( + result.is_ok(), + "ask must return Ok to suppress generic emit" + ); + assert!( + result.unwrap(), + "ask must return Ok(true) to suppress generic emit" + ); + assert_eq!( + client.pending_permissions.len(), + 1, + "exactly one entry must be registered after ask" + ); + let entry = client + .pending_permissions + .get("42") + .expect("entry under id=42"); + assert!( + matches!(entry.state, PermissionEntryState::Pending), + "entry must start in Pending state" + ); + } + + // ── Pinned §1 (cancel during write path): poison process test ──────────── + + #[test] + fn cancel_during_writing_poisons_process() { + // Simulate a process that has an entry in Writing state at cancel time. + // cancel_with_cleanup_until must return PermissionPoisoned and set the flag. + // + // We test this synchronously because cancel_with_cleanup_until is async + // and we need to manipulate state directly. We use a tokio runtime. + let rt = tokio::runtime::Runtime::new().unwrap(); + rt.block_on(async { + // Use a "sleep" script so the process is alive but won't emit responses. + let mut client = spawn_script("sleep 10").await; + client.set_permission_config( + ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).unwrap(), + ); + client.set_owner_pubkey_known(true); + + // Manually plant an entry in Writing state — this simulates cancel + // arriving while the harness was in the middle of writing. + client.pending_permissions.insert( + "99".to_string(), + PermissionEntry { + nonce: "n99".to_string(), + options_snapshot: vec![], + state: PermissionEntryState::Writing, + deadline: tokio::time::Instant::now() + std::time::Duration::from_secs(300), + }, + ); + // cancel_with_cleanup needs last_prompt_id to be Some. + client.last_prompt_id = Some(999); + + let err = client + .cancel_with_cleanup_grace("sess-poison", std::time::Duration::from_millis(500)) + .await + .expect_err("cancel during write must return Err"); + + assert!( + matches!(err, AcpError::PermissionPoisoned), + "expected PermissionPoisoned, got {err:?}" + ); + assert!( + client.permission_poisoned, + "poisoned flag must be set after cancel-during-write" + ); + }); + } + + #[test] + fn poisoned_process_surfaces_immediately_on_next_cancel() { + // Once poisoned, every subsequent cancel must immediately return PermissionPoisoned + // without writing anything — the process is unsafe to use. + let rt = tokio::runtime::Runtime::new().unwrap(); + rt.block_on(async { + let mut client = spawn_script("sleep 10").await; + client.permission_poisoned = true; + client.last_prompt_id = Some(1); + + let err = client + .cancel_with_cleanup_grace("sess", std::time::Duration::from_millis(200)) + .await + .expect_err("poisoned process must error immediately"); + assert!(matches!(err, AcpError::PermissionPoisoned)); + }); + } + + #[test] + fn poisoned_process_check_in_read_loop_returns_poison_error() { + // Once permission_poisoned is set, read_until_response_with_idle_timeout + // must return PermissionPoisoned on the next loop iteration. + let rt = tokio::runtime::Runtime::new().unwrap(); + rt.block_on(async { + let mut client = spawn_script("sleep 10").await; + client.permission_poisoned = true; + client.last_prompt_id = Some(42); + + let idle = std::time::Duration::from_secs(5); + let max_dur = std::time::Duration::from_secs(10); + let hard_deadline = tokio::time::Instant::now() + max_dur; + let result = client + .read_until_response_with_idle_timeout("sess", 42, idle, hard_deadline, max_dur) + .await; + assert!( + matches!(result, Err(AcpError::PermissionPoisoned)), + "expected PermissionPoisoned from poisoned-flag check, got {result:?}" + ); + }); + } + + // ── Pinned §5: cancel drains pending entries with cancelled ─────────────── + + #[test] + fn cancel_drains_pending_entries_with_cancelled_response() { + // Under ask policy: cancel must drain all Pending entries and write + // "cancelled" responses for each, then proceed to session/cancel. + // Verifies: + // - Map is empty after cancel (entries were drained). + // - Cancel result is NOT PermissionPoisoned (no Writing entries present). + // - Cancel exits normally (Ok or CancelDrainTimeout — sleep script never + // emits a response, so this exits via timeout, which is expected). + // + // We can verify that Pending entries are removed by checking the map post-cancel. + // We don't verify the wire bytes here (that requires a live script) — we verify + // the state machine: Pending entries disappear after cancel. + let rt = tokio::runtime::Runtime::new().unwrap(); + rt.block_on(async { + // Use a "sleep" script — stays alive but ignores stdin. + let mut client = spawn_script("sleep 5").await; + client.set_permission_config( + ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).unwrap(), + ); + + // Plant two Pending entries. + for i in 0..2u64 { + client.pending_permissions.insert( + format!("{i}"), + PermissionEntry { + nonce: format!("n{i}"), + options_snapshot: vec![ + serde_json::json!({"optionId":"opt","kind":"reject_once","name":"R"}), + ], + state: PermissionEntryState::Pending, + deadline: tokio::time::Instant::now() + std::time::Duration::from_secs(300), + }, + ); + } + client.last_prompt_id = Some(999); + + // cancel_with_cleanup_grace with short grace — the sleep script will + // never emit a response, so this exits via CancelDrainTimeout. + let result = client + .cancel_with_cleanup_grace("sess-drain", std::time::Duration::from_millis(200)) + .await; + + // Should NOT be PermissionPoisoned (no Writing entries). + assert!( + !matches!(result, Err(AcpError::PermissionPoisoned)), + "no Writing entries — must not be PermissionPoisoned" + ); + // Map must be empty — Pending entries were drained. + assert!( + client.pending_permissions.is_empty(), + "all Pending entries must be removed from the map after cancel" + ); + }); + } + + // ── Pinned §2: reject policy is byte-for-byte unchanged ─────────────────── + + #[tokio::test] + async fn reject_policy_denies_synchronously_and_returns_ok_true() { + let mut client = spawn_inert_client().await; + set_policy(&mut client, PermissionPolicy::Reject); + + let msg = perm_request(7, default_opts()); + let hard_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(30); + let result = client + .handle_permission_request(&msg, true, hard_deadline) + .await; + // Reject is synchronous — no pending entry, Ok(true) to suppress generic emit. + assert!(result.is_ok(), "reject must return Ok"); + assert!(result.unwrap(), "reject must return Ok(true)"); + assert!( + client.pending_permissions.is_empty(), + "reject must not leave pending entries" + ); + // Legacy single-id slot must also be cleared after the synchronous response. + assert!( + client.pending_permission_id.is_none(), + "pending_permission_id must be None after reject completes" + ); + assert!( + client.permission_responded, + "permission_responded must be true after reject completes" + ); + } + + // ── Pinned §2: allow policy auto-selects allow_once ─────────────────────── + + #[tokio::test] + async fn allow_policy_auto_selects_allow_once_and_returns_ok_true() { + let mut client = spawn_inert_client().await; + set_policy(&mut client, PermissionPolicy::Allow); + + let msg = perm_request(8, default_opts()); + let hard_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(30); + let result = client + .handle_permission_request(&msg, true, hard_deadline) + .await; + assert!(result.is_ok(), "allow auto-select must return Ok"); + assert!(result.unwrap(), "allow auto-select must return Ok(true)"); + // No pending entries — handled synchronously. + assert!(client.pending_permissions.is_empty()); + } + + #[tokio::test] + async fn allow_policy_fails_closed_with_no_allow_once_option() { + let mut client = spawn_inert_client().await; + set_policy(&mut client, PermissionPolicy::Allow); + + // Only reject_once offered — allow policy must fail closed. + let msg = perm_request(9, &[("opt-r", "reject_once", "Reject")]); + let hard_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(30); + let result = client + .handle_permission_request(&msg, true, hard_deadline) + .await; + // Fail closed: denial written, Ok(true) returned. + assert!(result.is_ok(), "fail-closed allow must return Ok"); + assert!(result.unwrap(), "fail-closed allow must return Ok(true)"); + assert!(client.pending_permissions.is_empty()); + } + + // ── Pinned §6: decision arm — validated option_id must be in snapshot ───── + + #[tokio::test] + async fn decision_with_unknown_option_id_is_ignored() { + // A decision carrying an optionId not in the snapshot must be ignored + // (no response written, entry stays Pending) — the loop continues. + // After the bad decision is processed, the loop times out on idle (since the + // script produces no output after the initial response) and the entry is + // still Pending at that point. + // + // The script produces the terminal id=999 response only AFTER a short delay, + // giving the loop time to process the bad decision and leave the entry Pending. + // We verify the entry is still Pending by running the loop until idle timeout. + let script = "sleep 2; echo '{\"jsonrpc\":\"2.0\",\"id\":999,\"result\":{\"done\":true}}'"; + let mut client = spawn_script(script).await; + client.set_owner_pubkey_known(true); + set_policy(&mut client, PermissionPolicy::Ask); + let obs = crate::observer::ObserverHandle::in_process(); + client.set_observer(Some(obs), 0); + + let nonce = "test-nonce-bad-opt".to_string(); + let req_id_str = "5".to_string(); + client.pending_permissions.insert( + req_id_str.clone(), + PermissionEntry { + nonce: nonce.clone(), + options_snapshot: vec![ + serde_json::json!({"optionId":"valid-opt","kind":"allow_once","name":"A"}), + ], + state: PermissionEntryState::Pending, + deadline: tokio::time::Instant::now() + std::time::Duration::from_secs(300), + }, + ); + + // Deliver a decision with a nonce that matches but an invalid optionId. + let bad_decision = PermissionDecision { + request_nonce: nonce, + option_id: "nonexistent-option".to_string(), + }; + + let (tx, rx) = tokio::sync::mpsc::channel::(1); + client.install_permission_decision_rx(rx); + // Send the bad decision; then close the sender so the channel is exhausted. + tx.send(bad_decision).await.unwrap(); + drop(tx); + + // Drive the loop with a short idle timeout — the bad decision is processed + // on the first iteration (entry stays Pending), then the loop idles. + let idle = std::time::Duration::from_millis(300); + let max_dur = std::time::Duration::from_secs(5); + let hard_deadline = tokio::time::Instant::now() + max_dur; + let result = client + .read_until_response_with_idle_timeout("sess-bad-opt", 5, idle, hard_deadline, max_dur) + .await; + + // The loop exits via idle timeout (script sleeps; bad decision was ignored, + // so no terminal response for id=5 was written, and idle fires). + // We accept either idle timeout OR id=999 match (if the script's sleep was short). + // The critical assertion is on the entry state. + let _ = result; // exit reason is not the focus + + // Entry must still be Pending — the bad decision did not mutate it. + let entry = client.pending_permissions.get(&req_id_str); + // The loop drains on non-recoverable errors; on idle timeout (recoverable) it + // does NOT drain — entry must still be there and Pending. + match entry { + Some(e) => assert!( + matches!(e.state, PermissionEntryState::Pending), + "entry must still be Pending after bad decision, got: {:?}", + e.state + ), + None => panic!("entry was removed — idle timeout should not drain the map"), + } + } + + // ── Pinned §7 (wire transmission): transmit_mode drives set_config_option ─ + + #[test] + fn resolved_permission_config_effective_mode_wire_string_is_correct() { + // Verify that effective_mode.as_wire_str() returns the correct ACP wire value. + let cfg = ResolvedPermissionConfig::resolve(PermissionPolicy::Reject, None).unwrap(); + assert_eq!(cfg.effective_mode.as_wire_str(), "dontAsk"); + + let cfg = ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).unwrap(); + assert_eq!(cfg.effective_mode.as_wire_str(), "default"); + + let cfg = ResolvedPermissionConfig::resolve(PermissionPolicy::Allow, None).unwrap(); + assert_eq!(cfg.effective_mode.as_wire_str(), "default"); + } + + // ── Pinned amendment: PermissionMode::Auto matrix row ──────────────────── + // + // `auto` = model-gated classifier — the adapter may self-approve most tool + // calls internally but can still forward residual permission requests to ACP. + // - allow + auto → compatible (transmit as-is; both want unattended approval) + // - ask + auto → compatible with warning (residual escalations surface cards; + // internally-approved calls bypass ask silently) + // - reject + auto → startup error (inverted security: policy says deny, adapter + // auto-approves everything) + + #[test] + fn resolved_permission_config_allow_plus_explicit_auto_is_ok() { + // allow + auto is compatible: both want unattended approval. + let cfg = + ResolvedPermissionConfig::resolve(PermissionPolicy::Allow, Some(PermissionMode::Auto)) + .unwrap(); + assert_eq!(cfg.effective_mode, PermissionMode::Auto); + assert_eq!(cfg.effective_mode.as_wire_str(), "auto"); + assert_eq!(cfg.mode_source, ModeSource::Explicit); + } + + #[test] + fn resolved_permission_config_ask_plus_explicit_auto_is_ok_with_warning() { + // ask + auto is compatible-with-warning: residual escalations still surface + // cards; internally-approved calls bypass the ask flow silently. + // `auto` is a model classifier, not a bypass — some requests still escalate. + let result = + ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, Some(PermissionMode::Auto)); + assert!( + result.is_ok(), + "ask + auto must succeed (warn only), got: {result:?}" + ); + let cfg = result.unwrap(); + assert_eq!(cfg.effective_mode, PermissionMode::Auto); + assert_eq!(cfg.mode_source, ModeSource::Explicit); + } + + #[test] + fn resolved_permission_config_reject_plus_explicit_auto_is_startup_error() { + // reject + auto: inverted-security worst case — policy says deny but + // adapter auto-approves everything internally. + let result = + ResolvedPermissionConfig::resolve(PermissionPolicy::Reject, Some(PermissionMode::Auto)); + assert!(result.is_err(), "reject + auto must be a startup error"); + let msg = format!("{}", result.unwrap_err()); + assert!(msg.contains("auto"), "error must mention auto, got: {msg}"); + } + + #[test] + fn permission_mode_auto_wire_string_is_correct() { + assert_eq!(PermissionMode::Auto.as_wire_str(), "auto"); + assert!(!PermissionMode::Auto.is_default()); + } } diff --git a/crates/buzz-acp/src/config.rs b/crates/buzz-acp/src/config.rs index d9596858460..dd43e18f224 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -115,6 +115,10 @@ impl std::fmt::Display for RespondTo { /// `configId: "mode"` (e.g. `claude-agent-acp`). /// /// - `default` — agent's built-in behaviour (permission requests per tool call). +/// - `auto` — fully autonomous execution; model-gated (requires `supportsAutoMode`); +/// the adapter degrades gracefully to `default` when the active model does not +/// support it. The adapter self-approves all tool calls internally — no +/// `session/request_permission` ever crosses ACP under this mode. /// - `acceptEdits` — auto-approve file edits, still ask for other tools. /// - `dontAsk` — never prompt; reject anything that would require permission. /// - `plan` — planning-only mode (no tool execution). @@ -123,6 +127,22 @@ pub enum PermissionMode { /// Agent default — permission requests per tool call. #[value(alias = "default")] Default, + /// Fully autonomous execution; model-gated (requires `supportsAutoMode`). + /// + /// `auto` is a model-gated classifier — the adapter self-approves most tool + /// calls internally, but can fall back to forwarding residual + /// `session/request_permission` requests to ACP when the model chooses manual + /// approval for a specific call. It is therefore **not** a hard bypass. + /// + /// Policy compatibility: + /// - `allow + auto` — compatible; both want unattended approval. + /// - `ask + auto` — compatible with a startup warning; residual escalations + /// still surface permission cards, but internally approved calls bypass the + /// ask flow silently. + /// - `reject + auto` — startup contradiction; adapter auto-approves + /// internally while the policy intends to deny — inverted-security worst case. + #[value(alias = "auto")] + Auto, /// Auto-approve file edits, still ask for other tools. #[value(alias = "acceptEdits")] AcceptEdits, @@ -140,6 +160,7 @@ impl PermissionMode { pub fn as_wire_str(&self) -> &'static str { match self { Self::Default => "default", + Self::Auto => "auto", Self::AcceptEdits => "acceptEdits", Self::DontAsk => "dontAsk", Self::Plan => "plan", @@ -148,6 +169,7 @@ impl PermissionMode { /// Returns `true` when the mode is the agent's built-in default and /// therefore doesn't need to be explicitly set. + #[cfg(test)] pub fn is_default(&self) -> bool { matches!(self, Self::Default) } @@ -159,6 +181,171 @@ impl std::fmt::Display for PermissionMode { } } +/// How Buzz responds to an ACP `session/request_permission` request. +/// +/// Injected as `BUZZ_ACP_PERMISSION_POLICY`. Desktop injects the resolved +/// per-agent or fleet-wide value; headless defaults to `reject`. +/// +/// - `allow` — auto-select the unique `allow_once` option; fail closed if zero or +/// multiple `allow_once` candidates, malformed options, or any validation error. +/// - `ask` — surface the request as an actionable card for the owner; fail closed +/// on timeout (300 s) or if the observer / owner is unavailable. +/// - `reject` — deny every request (today's behaviour, headless default). +#[derive(Debug, Clone, Copy, PartialEq, clap::ValueEnum)] +pub enum PermissionPolicy { + /// Auto-approve via the unique `allow_once` option; fail closed otherwise. + #[value(alias = "allow")] + Allow, + /// Surface as an actionable card; fail closed on timeout or unavailability. + #[value(alias = "ask")] + Ask, + /// Deny all requests — headless default, byte-for-byte today's behaviour. + #[value(alias = "reject")] + Reject, +} + +impl std::fmt::Display for PermissionPolicy { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(match self { + Self::Allow => "allow", + Self::Ask => "ask", + Self::Reject => "reject", + }) + } +} + +/// Whether an effective `PermissionMode` was derived by the harness or +/// supplied explicitly by the operator. +#[derive(Debug, Clone, Copy, PartialEq)] +pub enum ModeSource { + /// No `--permission-mode` was supplied; the harness derived the mode from + /// the active `PermissionPolicy`. + Derived, + /// An explicit `--permission-mode` / `BUZZ_ACP_PERMISSION_MODE` value was + /// supplied by the operator. + Explicit, +} + +impl std::fmt::Display for ModeSource { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(match self { + Self::Derived => "derived", + Self::Explicit => "explicit", + }) + } +} + +/// Resolved, immutable per-startup permission configuration. +/// +/// Computed once in `Config::from_args` from `policy` + optional `mode` and +/// carried through `PromptContext` (via `Arc`) so every task reads the same +/// value without re-deriving it. +/// +/// `transmit_mode` — set `session/set_config_option` for this mode whenever +/// the agent advertises it. **Always set** (including for `PermissionMode::Default`); +/// the caller decides whether to skip based on advertisement, not derivation. +#[derive(Debug, Clone)] +pub struct ResolvedPermissionConfig { + /// The high-level policy governing how permission requests are answered. + pub policy: PermissionPolicy, + /// The ACP mode that will be sent to the agent after session creation. + pub effective_mode: PermissionMode, + /// Whether `effective_mode` was derived or supplied explicitly. + pub mode_source: ModeSource, + /// `true` when the effective mode should be transmitted to the agent via + /// `session/set_config_option`, i.e. whenever the agent advertises it. + pub transmit_mode: bool, +} + +impl ResolvedPermissionConfig { + /// Derive the config from a `policy` and an optional explicit `mode`. + /// + /// Returns `Err` for contradictory combinations: + /// - `ask` + explicit `dontAsk` — harness would want the agent to + /// escalate, but `dontAsk` makes the agent self-deny internally. + /// - `allow` + explicit `dontAsk` — same contradiction. + /// - `reject` + explicit `auto` — inverted-security worst case: policy says + /// "deny" but the adapter auto-approves everything internally. + /// + /// Emits a warning (not an error) for `ask + auto`: internally-approved tool + /// calls bypass the ask flow silently, but residual escalations still surface + /// cards — the combination works, with the caveat that not all requests are seen. + pub fn resolve( + policy: PermissionPolicy, + explicit_mode: Option, + ) -> Result { + // Fail on contradictory ask/allow + dontAsk combinations. + if matches!(policy, PermissionPolicy::Ask | PermissionPolicy::Allow) + && explicit_mode == Some(PermissionMode::DontAsk) + { + return Err(ConfigError::ConfigFile(format!( + "permission_policy={policy} conflicts with permission_mode=dontAsk: \ + dontAsk makes the agent self-deny internally before Buzz can answer" + ))); + } + // Fail on reject + auto: inverted-security worst case — policy says "deny" + // but the adapter auto-approves everything internally. + // `ask` + auto is a warning-only case: the adapter MAY still forward residual + // permission requests to ACP (auto is a model classifier, not bypass mode); + // warn and transmit rather than fail startup. + // `allow` + auto is compatible: both policies want unattended approval. + if policy == PermissionPolicy::Reject && explicit_mode == Some(PermissionMode::Auto) { + return Err(ConfigError::ConfigFile(format!( + "permission_policy={policy} conflicts with permission_mode=auto: \ + auto makes the adapter self-approve internally, which bypasses the \ + reject policy — inverted-security worst case" + ))); + } + // Warn on ask + auto: residual permission requests may still reach ACP + // (auto is a model classifier, not bypass mode) so ask can still surface + // cards — but internally-approved calls will bypass the ask flow silently. + if policy == PermissionPolicy::Ask && explicit_mode == Some(PermissionMode::Auto) { + tracing::warn!( + "permission_policy=ask with permission_mode=auto: internally-approved \ + tool calls bypass Buzz ask flow; residual escalations will still \ + surface cards. Consider policy=allow if unattended approval is intended." + ); + } + + let (effective_mode, mode_source) = match explicit_mode { + Some(m) => (m, ModeSource::Explicit), + None => { + // Mode matrix — derived from policy when no explicit mode given: + // reject → dontAsk (harness rejects; adapter also self-denies for + // consistency — byte-for-byte today's behaviour) + // ask → default (keep the adapter escalating to Buzz) + // allow → default (keep the adapter escalating to Buzz; + // dontAsk would silently self-deny before we + // could auto-select allow_once) + let derived = match policy { + PermissionPolicy::Reject => PermissionMode::DontAsk, + PermissionPolicy::Ask | PermissionPolicy::Allow => PermissionMode::Default, + }; + (derived, ModeSource::Derived) + } + }; + + Ok(Self { + policy, + effective_mode, + mode_source, + // Always transmit — the caller skips based on agent advertisement, + // not on whether the mode is the default. + transmit_mode: true, + }) + } +} + +impl std::fmt::Display for ResolvedPermissionConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "policy={} mode={}({})", + self.policy, self.effective_mode, self.mode_source + ) + } +} + /// CLI args for `buzz-acp models` — query available models from an agent. /// /// This is a standalone `Parser` (not a subcommand variant) because the @@ -424,18 +611,32 @@ pub struct CliArgs { #[arg(long, env = "BUZZ_ACP_SESSION_TITLE")] pub session_title: Option, - /// Permission mode for agents that support `session/set_config_option` - /// with `configId: "mode"` (e.g. `claude-agent-acp`). + /// How Buzz responds to ACP `session/request_permission` requests. /// - /// Defaults to `dontAsk`, which rejects operations that need interactive - /// approval because Buzz does not expose a human permission prompt. + /// - `reject` (headless default) — deny all permission requests. + /// - `ask` — surface as an actionable card; auto-deny on timeout (300 s) or + /// when the observer / owner is unavailable. + /// - `allow` — auto-approve via the unique `allow_once` option; fail closed + /// if zero or multiple `allow_once` candidates. + /// + /// Desktop injects the resolved per-agent or fleet-wide value. + /// Headless installations should leave this unset (defaults to `reject`). #[arg( long, - env = "BUZZ_ACP_PERMISSION_MODE", - default_value = "dont-ask", + env = "BUZZ_ACP_PERMISSION_POLICY", + default_value = "reject", value_enum )] - pub permission_mode: PermissionMode, + pub permission_policy: PermissionPolicy, + + /// ACP permission mode sent to the agent via `session/set_config_option`. + /// + /// When unset the harness derives a sensible default from `permission_policy`: + /// `reject` → `dontAsk`, `ask` / `allow` → `default`. + /// Explicit values are validated: `ask` or `allow` + `dontAsk` is a startup + /// error because `dontAsk` makes the agent self-deny before Buzz can answer. + #[arg(long, env = "BUZZ_ACP_PERMISSION_MODE", value_enum)] + pub permission_mode: Option, /// Inbound author gate: which authors' events the harness forwards. /// Modes: owner-only (default), allowlist, anyone, nobody. @@ -530,8 +731,10 @@ pub struct Config { /// Sanitized session title, sent as `_meta.sessionTitle` on `session/new`. /// `None` when unset or when the configured value sanitized to empty. pub session_title: Option, - /// Permission mode to apply after session creation. `Default` = skip. - pub permission_mode: PermissionMode, + /// Resolved permission configuration — policy, effective ACP mode, and + /// how to transmit it. Computed once from `PermissionPolicy` + optional + /// explicit `PermissionMode` in `from_args`. + pub permission_config: ResolvedPermissionConfig, /// Inbound author gate mode. pub respond_to: RespondTo, /// Validated allowlist of pubkey hex strings (used when respond_to == Allowlist). @@ -1054,6 +1257,9 @@ impl Config { validate_multiple_event_handling(args.multiple_event_handling, args.dedup)?; + let permission_config = + ResolvedPermissionConfig::resolve(args.permission_policy, args.permission_mode)?; + let config = Config { keys, relay_url: args.relay_url, @@ -1092,7 +1298,7 @@ impl Config { .session_title .as_deref() .and_then(sanitize_session_title), - permission_mode: args.permission_mode, + permission_config, respond_to: args.respond_to, respond_to_allowlist, allowed_respond_to, @@ -1125,7 +1331,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={}({}) {}{}", self.relay_url, self.keys.public_key().to_hex(), self.agent_command, @@ -1145,7 +1351,8 @@ impl Config { self.typing_enabled, self.memory_enabled, self.model.as_deref().unwrap_or("(agent default)"), - self.permission_mode, + self.permission_config.effective_mode, + self.permission_config.mode_source, respond_to_detail, allowed_respond_to_detail, ) @@ -1463,7 +1670,11 @@ mod tests { memory_enabled: true, model: None, session_title: None, - permission_mode: PermissionMode::DontAsk, + permission_config: ResolvedPermissionConfig::resolve( + PermissionPolicy::Reject, + Some(PermissionMode::DontAsk), + ) + .expect("test config"), respond_to: RespondTo::Anyone, respond_to_allowlist: HashSet::new(), allowed_respond_to: Vec::new(), @@ -2285,7 +2496,11 @@ channels = "ALL" #[test] fn test_summary_includes_permission_mode() { let mut config = test_config(SubscribeMode::Mentions); - config.permission_mode = PermissionMode::DontAsk; + config.permission_config = ResolvedPermissionConfig::resolve( + PermissionPolicy::Reject, + Some(PermissionMode::DontAsk), + ) + .expect("test config"); let s = config.summary(); assert!( s.contains("permission_mode=dontAsk"), @@ -2296,7 +2511,8 @@ channels = "ALL" #[test] fn test_summary_permission_mode_default() { let mut config = test_config(SubscribeMode::Mentions); - config.permission_mode = PermissionMode::Default; + config.permission_config = + ResolvedPermissionConfig::resolve(PermissionPolicy::Ask, None).expect("test config"); let s = config.summary(); assert!( s.contains("permission_mode=default"), @@ -2307,7 +2523,10 @@ channels = "ALL" #[test] fn test_default_config_rejects_interactive_permissions() { let config = test_config(SubscribeMode::Mentions); - assert_eq!(config.permission_mode, PermissionMode::DontAsk); + assert_eq!( + config.permission_config.effective_mode, + PermissionMode::DontAsk + ); } #[test] diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 65c9dd6203c..9fb1d1e1316 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -594,6 +594,7 @@ fn batch_envelope(events: &[observer::ObserverEvent]) -> observer::ObserverEvent session_id: last.session_id.clone(), turn_id: last.turn_id.clone(), started_at: last.started_at.clone(), + authorization: None, payload: serde_json::json!({ "events": serde_json::to_value(events).unwrap_or_default(), }), @@ -1116,6 +1117,9 @@ fn handle_relay_observer_control_event( Some("switch_model") => { handle_switch_model_control(&payload, pool, observer); } + Some("permission_decision") => { + handle_permission_decision_control(&payload, pool, observer); + } _ => { tracing::debug!(payload = %payload, "ignoring unknown observer control frame"); } @@ -1235,6 +1239,120 @@ fn handle_switch_model_control( } } +/// Handle a `permission_decision` control frame. +/// +/// Extracts `channelId`, `requestNonce`, and `optionId` from the payload and +/// delivers a [`crate::acp::PermissionDecision`] to the in-flight read loop +/// via the per-task `permission_decision_tx` mpsc channel. +/// +/// If there is no in-flight task for the channel, or the sender is gone, the +/// frame is dropped silently (the per-request 300s timeout will fail the entry +/// closed on its own). +fn handle_permission_decision_control( + payload: &serde_json::Value, + pool: &mut AgentPool, + observer: Option<&observer::ObserverHandle>, +) { + let Some(channel_id) = payload + .get("channelId") + .and_then(|v| v.as_str()) + .and_then(|v| v.parse::().ok()) + else { + tracing::warn!("observer permission_decision control frame missing valid channelId"); + return; + }; + + let Some(request_nonce) = payload + .get("requestNonce") + .and_then(|v| v.as_str()) + .filter(|s| !s.is_empty()) + else { + tracing::warn!("observer permission_decision control frame missing requestNonce"); + return; + }; + + let Some(option_id) = payload + .get("optionId") + .and_then(|v| v.as_str()) + .filter(|s| !s.is_empty()) + else { + tracing::warn!("observer permission_decision control frame missing optionId"); + return; + }; + + let decision = crate::acp::PermissionDecision { + request_nonce: request_nonce.to_string(), + option_id: option_id.to_string(), + }; + + // Find the in-flight task for this channel and deliver via its mpsc. + let entry = pool + .task_map_mut() + .values_mut() + .find(|m| m.channel_id == Some(channel_id)); + + let status = if let Some(meta) = entry { + if let Some(tx) = &meta.permission_decision_tx { + match tx.try_send(decision) { + Ok(()) => { + tracing::info!( + channel = %channel_id, + nonce = %request_nonce, + option_id = %option_id, + "permission_decision delivered to read loop" + ); + "sent" + } + Err(tokio::sync::mpsc::error::TrySendError::Full(_)) => { + tracing::warn!( + channel = %channel_id, + "permission_decision channel full — dropping (will timeout)" + ); + "channel_full" + } + Err(tokio::sync::mpsc::error::TrySendError::Closed(_)) => { + tracing::warn!( + channel = %channel_id, + "permission_decision channel closed — read loop already exited" + ); + "channel_closed" + } + } + } else { + tracing::warn!( + channel = %channel_id, + "permission_decision_tx not installed for in-flight task" + ); + "no_channel" + } + } else { + tracing::warn!( + channel = %channel_id, + "permission_decision control frame for channel with no in-flight task" + ); + "no_active_turn" + }; + + if let Some(observer) = observer { + observer.emit( + "control_result", + None, + &observer::ObserverContext { + channel_id: Some(channel_id.to_string()), + session_id: None, + turn_id: None, + started_at: None, + }, + serde_json::json!({ + "type": "permission_decision", + "status": status, + "requestNonce": request_nonce, + "optionId": option_id, + }), + ); + } +} + /// Maximum crashes in a 60-second window before a slot's circuit opens. const CIRCUIT_BREAKER_THRESHOLD: usize = 3; /// Window for circuit-breaker crash counting. @@ -1835,7 +1953,7 @@ async fn tokio_main() -> Result<()> { channel_info: pool::ChannelInfoResolver::new(channel_info_map, relay.rest_client()), context_message_limit: config.context_message_limit, max_turns_per_session: config.max_turns_per_session, - permission_mode: config.permission_mode, + permission_config: config.permission_config.clone(), agent_keys: config.keys.clone(), agent_owner_pubkey: startup_owner .as_deref() @@ -3290,6 +3408,17 @@ fn dispatch_pending( agent.acp.install_steer_rx(rx); let steer_tx = Some(tx); + // Permission decision channel: delivers `permission_decision` control + // frames into the read loop's decision arm (spec §4). Installed + // per-session (the receiver is taken by the read loop and dropped + // when the turn ends; the next turn installs a fresh pair). Capacity + // matches PERMISSION_MAP_CAP so each pending entry gets a slot. + let (perm_tx, perm_rx) = tokio::sync::mpsc::channel::( + crate::acp::PERMISSION_MAP_CAP, + ); + agent.acp.install_permission_decision_rx(perm_rx); + let permission_decision_tx = Some(perm_tx); + // Prompt text is now built inside run_prompt_task (needs async for // context fetching). Pass None for prompt_text; batch carries the data. let (control_tx, control_rx) = tokio::sync::oneshot::channel::(); @@ -3318,6 +3447,7 @@ fn dispatch_pending( recoverable_batch, control_tx: Some(control_tx), steer_tx, + permission_decision_tx, }, ); dispatched_channels.push((channel_id, typing_scope)); @@ -3703,6 +3833,10 @@ fn handle_prompt_result( | acp::AcpError::WriteTimeout(_) | acp::AcpError::Timeout(_) | acp::AcpError::Protocol(_) + // A poisoned process wrote a partial permission response + // and must NOT be returned to the pool — the pipe state is + // uncertain and re-use would corrupt the next turn's writes. + | acp::AcpError::PermissionPoisoned ); let error_code = match &e { acp::AcpError::AgentError { code, .. } => Some(*code), @@ -3932,6 +4066,7 @@ fn dispatch_heartbeat( recoverable_batch: None, control_tx: None, steer_tx: None, + permission_decision_tx: None, }, ); *heartbeat_in_flight = true; @@ -4703,6 +4838,7 @@ mod owner_control_command_tests { recoverable_batch: None, control_tx: Some(control_tx), steer_tx: None, + permission_decision_tx: None, }, ); @@ -5208,6 +5344,7 @@ mod observer_publish_queue_tests { session_id: Some("session-1".to_string()), turn_id: Some("turn-1".to_string()), started_at: None, + authorization: None, payload: serde_json::json!({ "seq": seq }), } } @@ -6076,6 +6213,7 @@ mod observer_chunk_coalescer_tests { session_id: Some("session-1".to_string()), turn_id: Some("turn-1".to_string()), started_at: None, + authorization: None, payload: serde_json::json!({ "jsonrpc": "2.0", "method": "session/update", @@ -6104,6 +6242,7 @@ mod observer_chunk_coalescer_tests { session_id: Some("session-1".to_string()), turn_id: Some("turn-1".to_string()), started_at: None, + authorization: None, payload: serde_json::json!({ "type": "turn_started" }), } } @@ -6199,7 +6338,11 @@ mod build_mcp_servers_tests { memory_enabled: false, model: None, session_title: None, - permission_mode: config::PermissionMode::DontAsk, + permission_config: config::ResolvedPermissionConfig::resolve( + config::PermissionPolicy::Reject, + None, + ) + .expect("test config"), respond_to: config::RespondTo::Anyone, respond_to_allowlist: std::collections::HashSet::new(), allowed_respond_to: vec![], @@ -6421,7 +6564,11 @@ mod error_outcome_emission_tests { memory_enabled: false, model: None, session_title: None, - permission_mode: config::PermissionMode::DontAsk, + permission_config: config::ResolvedPermissionConfig::resolve( + config::PermissionPolicy::Reject, + None, + ) + .expect("test config"), respond_to: config::RespondTo::Anyone, respond_to_allowlist: HashSet::new(), allowed_respond_to: vec![], @@ -6493,6 +6640,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_decision_tx: None, }, ); @@ -6569,6 +6717,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_decision_tx: None, }, ); started_rx.await.unwrap(); @@ -6661,6 +6810,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_decision_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -6752,6 +6902,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_decision_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -6857,6 +7008,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_decision_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -6933,6 +7085,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_decision_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -7027,6 +7180,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_decision_tx: None, }, ); let config = test_config(); @@ -7143,6 +7297,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_decision_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -7282,6 +7437,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_decision_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -7470,6 +7626,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_decision_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -7555,6 +7712,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_decision_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -7617,6 +7775,7 @@ mod observer_payload_trim_tests { session_id: Some("sess-1".to_string()), turn_id: Some("turn-1".to_string()), started_at: None, + authorization: None, payload, } } diff --git a/crates/buzz-acp/src/observer.rs b/crates/buzz-acp/src/observer.rs index 7029e5af6d5..104b604cafe 100644 --- a/crates/buzz-acp/src/observer.rs +++ b/crates/buzz-acp/src/observer.rs @@ -30,6 +30,25 @@ pub struct ObserverContext { pub started_at: Option, } +/// Authorization envelope attached to permission-related observer events. +/// +/// Present on the single `acp_read` emitted after a permission request passes +/// the admission preflight, and on the corresponding `acp_write` after the +/// response is confirmed written. +#[derive(Clone, Debug, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct AuthorizationEnvelope { + /// Single-use nonce bound to this request — delivered to the desktop and + /// consumed exactly once when the owner makes a decision. + pub request_nonce: String, + /// `true` when the owner can take action (policy=ask, preflight passed, + /// owner/observer available). `false` for auto-deny / fail-closed paths. + pub actionable: bool, + /// Human-readable reason when `actionable` is `false`. + #[serde(skip_serializing_if = "Option::is_none")] + pub reason: Option, +} + /// Handle used by the harness to publish local observer events. #[derive(Clone)] pub struct ObserverHandle { @@ -74,6 +93,10 @@ pub struct ObserverEvent { /// RFC3339 timestamp at which the current turn began, when known. #[serde(skip_serializing_if = "Option::is_none")] pub started_at: Option, + /// Authorization envelope — present only on permission `acp_read` / + /// `acp_write` frames. `None` on all other event kinds. + #[serde(skip_serializing_if = "Option::is_none")] + pub authorization: Option, /// Raw or semantic event payload. pub payload: serde_json::Value, } @@ -107,6 +130,31 @@ impl ObserverHandle { agent_index: Option, context: &ObserverContext, payload: serde_json::Value, + ) { + self.emit_inner(kind, agent_index, context, None, payload); + } + + /// Emit a local observer event with an authorization envelope. + /// + /// Used for permission `acp_read` and `acp_write` frames. + pub fn emit_authorized( + &self, + kind: impl Into, + agent_index: Option, + context: &ObserverContext, + authorization: AuthorizationEnvelope, + payload: serde_json::Value, + ) { + self.emit_inner(kind, agent_index, context, Some(authorization), payload); + } + + fn emit_inner( + &self, + kind: impl Into, + agent_index: Option, + context: &ObserverContext, + authorization: Option, + payload: serde_json::Value, ) { let event = ObserverEvent { seq: self.inner.seq.fetch_add(1, Ordering::Relaxed), @@ -117,6 +165,7 @@ impl ObserverHandle { session_id: context.session_id.clone(), turn_id: context.turn_id.clone(), started_at: context.started_at.clone(), + authorization, payload, }; diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index 8430307d9cd..a4869924b8a 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -34,7 +34,7 @@ use crate::acp::{ resolve_model_switch_method, AcpClient, AcpError, EnvVar, McpServer, ModelSwitchMethod, StopReason, SystemPromptTransport, }; -use crate::config::{compose_session_title, DedupMode, PermissionMode}; +use crate::config::{compose_session_title, DedupMode, PermissionMode, ResolvedPermissionConfig}; use crate::observer; use crate::queue::{ CancelReason, ContextMessage, ConversationContext, FlushBatch, PromptChannelInfo, @@ -67,6 +67,13 @@ pub struct TaskMeta { /// tasks only — all prompt tasks install a steer channel regardless /// of the agent's name. pub steer_tx: Option>, + /// Permission decision channel — delivers `permission_decision` control + /// frames from the observer dispatch loop into the read loop's decision + /// arm. `None` until the first `ask`-policy permission request arrives + /// (installed per-session by the pool dispatch path). Cloned from the + /// sender end of the channel installed on `AcpClient` via + /// `install_permission_decision_rx`. + pub permission_decision_tx: Option>, } /// Agent-level model capabilities. Populated on first session creation. @@ -543,8 +550,8 @@ pub struct PromptContext { pub context_message_limit: u32, /// Max turns per session before proactive rotation. 0 = disabled. pub max_turns_per_session: u32, - /// Permission mode to apply after session creation. `Default` = skip. - pub permission_mode: PermissionMode, + /// Resolved permission configuration — policy, effective ACP mode, and how to transmit. + pub permission_config: ResolvedPermissionConfig, /// Agent identity — used to derive the NIP-AE conversation key at /// session creation for core injection. pub agent_keys: nostr::Keys, @@ -1014,14 +1021,20 @@ async fn create_session_and_apply_model( }), ); - // Apply permission mode if not the agent's built-in default AND the agent - // advertises the requested mode in session/new. Agents that don't support - // the mode (e.g., goose crashes on unrecognized set_config_option values) - // are safely skipped — the harness rejects interactive permission requests. - if !ctx.permission_mode.is_default() - && agent_supports_mode(&resp.raw, ctx.permission_mode.as_wire_str()) + // Apply permission mode whenever the agent advertises it (including `default`). + // The `transmit_mode` flag handles any future cases where transmission should be skipped. + if ctx.permission_config.transmit_mode + && agent_supports_mode( + &resp.raw, + ctx.permission_config.effective_mode.as_wire_str(), + ) { - apply_permission_mode(&mut agent.acp, &resp.session_id, &ctx.permission_mode).await?; + apply_permission_mode( + &mut agent.acp, + &resp.session_id, + &ctx.permission_config.effective_mode, + ) + .await?; } Ok(resp.session_id) @@ -1412,6 +1425,18 @@ pub async fn run_prompt_task( turn_id.clone(), turn_started_at.clone(), )); + + // Wire permission configuration and owner-knowledge into the ACP client so + // `handle_permission_request` can evaluate the ask availability gate. These + // values come from `PromptContext` (resolved once at startup from CLI args and + // desktop-injected env vars) and are idempotent to re-apply across turns. + agent + .acp + .set_permission_config(ctx.permission_config.clone()); + agent + .acp + .set_owner_pubkey_known(ctx.agent_owner_pubkey.is_some()); + let triggering_event_ids: Vec = batch .as_ref() .map(|b| b.events.iter().map(|be| be.event.id.to_hex()).collect()) @@ -6536,7 +6561,11 @@ mod tests { ), context_message_limit: 0, max_turns_per_session: 0, - permission_mode: PermissionMode::Default, + permission_config: ResolvedPermissionConfig::resolve( + crate::config::PermissionPolicy::Reject, + None, + ) + .expect("test config"), agent_keys: agent_keys.clone(), agent_owner_pubkey: owner_pubkey, memory_enabled: false, diff --git a/crates/buzz-backend-kubernetes/tests/fixtures/provider-wire/deploy-full-launch.request.json b/crates/buzz-backend-kubernetes/tests/fixtures/provider-wire/deploy-full-launch.request.json index beffc294408..8c243a0c343 100644 --- a/crates/buzz-backend-kubernetes/tests/fixtures/provider-wire/deploy-full-launch.request.json +++ b/crates/buzz-backend-kubernetes/tests/fixtures/provider-wire/deploy-full-launch.request.json @@ -25,6 +25,7 @@ "BUZZ_ACP_DISPLAY_NAME": "worker", "BUZZ_ACP_LAZY_POOL": "true", "BUZZ_ACP_MODEL": "gpt-5", + "BUZZ_ACP_PERMISSION_POLICY": "ask", "BUZZ_ACP_RELAY_OBSERVER": "true", "BUZZ_ACP_SESSION_TITLE": "worker", "GOOSE_MODE": "auto" diff --git a/desktop/src-tauri/src/commands/agent_config_tests.rs b/desktop/src-tauri/src/commands/agent_config_tests.rs index 55191535784..119eac8c433 100644 --- a/desktop/src-tauri/src/commands/agent_config_tests.rs +++ b/desktop/src-tauri/src/commands/agent_config_tests.rs @@ -116,6 +116,7 @@ fn agent_record() -> ManagedAgentRecord { definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, + permission_policy: None, agent_command_override: None, persona_source_version: None, provider: None, diff --git a/desktop/src-tauri/src/commands/agent_models.rs b/desktop/src-tauri/src/commands/agent_models.rs index 4704582372d..a96a3ef7987 100644 --- a/desktop/src-tauri/src/commands/agent_models.rs +++ b/desktop/src-tauri/src/commands/agent_models.rs @@ -695,35 +695,9 @@ use databricks::{ should_start_interactive_auth, }; use databricks::{discover_databricks_models, DatabricksAuthIntent}; - -/// Apply an `UpdateManagedAgentRequest`'s model/provider/system_prompt patch -/// to `record`, enforcing the linked-instance write guard: a definition-linked -/// record's model/provider/prompt are definition-authoritative (see -/// `effective_config::resolve_linked`), so writes to these three fields are -/// silently dropped for a linked instance rather than persisting a byte the -/// resolver will never read. Definition-less instances accept the patch -/// as-is. Extracted so the guard is exercised by both `update_managed_agent` -/// and its regression tests — a test that reimplements this check instead of -/// calling it can go green after the real guard is deleted. -fn apply_model_provider_prompt_update( - record: &mut crate::managed_agents::ManagedAgentRecord, - model: Option>, - provider: Option>, - system_prompt: Option>, -) { - if record.persona_id.is_some() { - return; - } - if let Some(model_update) = model { - record.model = model_update; - } - if let Some(provider_update) = provider { - record.provider = provider_update; - } - if let Some(prompt_update) = system_prompt { - record.system_prompt = prompt_update; - } -} +#[path = "agent_models_update.rs"] +mod update; +use update::{apply_model_provider_prompt_update, apply_permission_policy_update}; /// Update mutable fields on an existing managed agent record. /// @@ -852,6 +826,8 @@ pub async fn update_managed_agent( record.respond_to_allowlist = prospective_allowlist; } + apply_permission_policy_update(record, input.permission_policy)?; + record.updated_at = now_iso(); save_managed_agents(&app, &records)?; diff --git a/desktop/src-tauri/src/commands/agent_models_update.rs b/desktop/src-tauri/src/commands/agent_models_update.rs new file mode 100644 index 00000000000..d6797c9b254 --- /dev/null +++ b/desktop/src-tauri/src/commands/agent_models_update.rs @@ -0,0 +1,36 @@ +use crate::managed_agents::{permission_policy::PermissionPolicy, BackendKind, ManagedAgentRecord}; + +pub(super) fn apply_model_provider_prompt_update( + record: &mut ManagedAgentRecord, + model: Option>, + provider: Option>, + system_prompt: Option>, +) { + if record.persona_id.is_some() { + return; + } + if let Some(model_update) = model { + record.model = model_update; + } + if let Some(provider_update) = provider { + record.provider = provider_update; + } + if let Some(prompt_update) = system_prompt { + record.system_prompt = prompt_update; + } +} + +pub(super) fn apply_permission_policy_update( + record: &mut ManagedAgentRecord, + update: Option>, +) -> Result<(), String> { + let Some(policy) = update else { + return Ok(()); + }; + if matches!(&record.backend, BackendKind::Provider { .. }) && record.backend_agent_id.is_some() + { + return Err("permission_policy is read-only while the agent is deployed remotely; shut down and redeploy to change it".to_string()); + } + record.permission_policy = policy; + Ok(()) +} diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index dd61fc9398a..f0792a0aad4 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -6,18 +6,20 @@ use crate::{ managed_agents::{ build_managed_agent_summary, current_instance_id, discover_provider_candidates, ensure_persona_is_active, find_managed_agent_mut, load_managed_agents, load_personas, - load_teams, managed_agent_avatar_url, normalize_agent_args, provider_deploy, - resolve_provider_binary, save_managed_agents, start_managed_agent_process, - stop_managed_agent_process, stop_managed_agent_workspace_pair, - sync_managed_agent_processes, try_regenerate_nest, validate_provider_config, BackendKind, - CreateManagedAgentRequest, CreateManagedAgentResponse, ManagedAgentRecord, - ManagedAgentSummary, RelayMeshConfig, DEFAULT_ACP_COMMAND, DEFAULT_AGENT_PARALLELISM, - DEFAULT_AGENT_TURN_TIMEOUT_SECONDS, + load_teams, normalize_agent_args, provider_deploy, resolve_provider_binary, + save_managed_agents, start_managed_agent_process, stop_managed_agent_process, + stop_managed_agent_workspace_pair, sync_managed_agent_processes, try_regenerate_nest, + validate_provider_config, BackendKind, CreateManagedAgentRequest, + CreateManagedAgentResponse, ManagedAgentRecord, ManagedAgentSummary, RelayMeshConfig, + DEFAULT_ACP_COMMAND, DEFAULT_AGENT_PARALLELISM, DEFAULT_AGENT_TURN_TIMEOUT_SECONDS, }, relay::{relay_ws_url_with_override, sync_managed_agent_profile}, util::now_iso, }; +mod create_helpers; +use create_helpers::{normalize_relay_mesh, resolve_created_avatar_url, trim_to_optional_string}; + /// Read the workspace owner pubkey without holding the lock. Used to populate `BUZZ_ACP_AGENT_OWNER` /// as a fallback for legacy agent records that have no NIP-OA `auth_tag`. pub(super) fn workspace_owner_hex(state: &AppState) -> Result { @@ -201,51 +203,6 @@ pub(super) fn archive_managed_agent_pending(app: &AppHandle, state: &AppState, a } } -fn normalize_relay_mesh( - config: Option<&RelayMeshConfig>, - backend: &BackendKind, -) -> Result, String> { - let Some(config) = config else { - return Ok(None); - }; - - let model_ref = config.model_ref.trim(); - if model_ref.is_empty() { - return Err("Buzz shared compute model is required".to_string()); - } - if backend != &BackendKind::Local { - return Err("Buzz shared compute agents must use the local backend".to_string()); - } - - Ok(Some(RelayMeshConfig { - model_ref: model_ref.to_string(), - })) -} - -fn trim_to_optional_string(value: &str) -> Option { - let trimmed = value.trim(); - if trimmed.is_empty() { - None - } else { - Some(trimmed.to_string()) - } -} - -fn resolve_created_avatar_url( - requested_avatar_url: Option<&str>, - persona_avatar_url: Option, - agent_command: &str, -) -> Option { - requested_avatar_url - .and_then(trim_to_optional_string) - .or_else(|| { - persona_avatar_url - .as_deref() - .and_then(trim_to_optional_string) - }) - .or_else(|| managed_agent_avatar_url(agent_command)) -} - #[cfg(feature = "mesh-llm")] async fn ensure_relay_mesh_for_record( app: &AppHandle, @@ -913,6 +870,7 @@ pub async fn create_managed_agent( } else { relay_mesh.clone() }, + permission_policy: None, // inherits global default or built-in `ask` }; records.push(record); diff --git a/desktop/src-tauri/src/commands/agents/create_helpers.rs b/desktop/src-tauri/src/commands/agents/create_helpers.rs new file mode 100644 index 00000000000..33051af8b7b --- /dev/null +++ b/desktop/src-tauri/src/commands/agents/create_helpers.rs @@ -0,0 +1,46 @@ +use crate::managed_agents::{managed_agent_avatar_url, BackendKind, RelayMeshConfig}; + +pub(super) fn normalize_relay_mesh( + config: Option<&RelayMeshConfig>, + backend: &BackendKind, +) -> Result, String> { + let Some(config) = config else { + return Ok(None); + }; + + let model_ref = config.model_ref.trim(); + if model_ref.is_empty() { + return Err("Buzz shared compute model is required".to_string()); + } + if backend != &BackendKind::Local { + return Err("Buzz shared compute agents must use the local backend".to_string()); + } + + Ok(Some(RelayMeshConfig { + model_ref: model_ref.to_string(), + })) +} + +pub(super) fn trim_to_optional_string(value: &str) -> Option { + let trimmed = value.trim(); + if trimmed.is_empty() { + None + } else { + Some(trimmed.to_string()) + } +} + +pub(super) fn resolve_created_avatar_url( + requested_avatar_url: Option<&str>, + persona_avatar_url: Option, + agent_command: &str, +) -> Option { + requested_avatar_url + .and_then(trim_to_optional_string) + .or_else(|| { + persona_avatar_url + .as_deref() + .and_then(trim_to_optional_string) + }) + .or_else(|| managed_agent_avatar_url(agent_command)) +} diff --git a/desktop/src-tauri/src/commands/agents_deploy.rs b/desktop/src-tauri/src/commands/agents_deploy.rs index 47ee5f92d49..d1a495db987 100644 --- a/desktop/src-tauri/src/commands/agents_deploy.rs +++ b/desktop/src-tauri/src/commands/agents_deploy.rs @@ -46,6 +46,10 @@ pub(crate) fn resolve_deploy_model_provider( /// `descriptor.env` is the authoritative six-layer environment. Policy values /// are deliberately separate because providers apply them below that layered /// environment, preserving the local spawn's power-user override semantics. +/// +/// `effective_permission_policy` is the already-resolved per-agent → global → +/// built-in policy. Pass it from the caller so that this function does not need +/// the global config; tests can pass `None` to get the built-in default. pub(super) fn build_launch_block( record: &ManagedAgentRecord, descriptor: &crate::managed_agents::readiness::EffectiveHarnessDescriptor, @@ -53,6 +57,7 @@ pub(super) fn build_launch_block( effective_prompt: Option<&str>, effective_model: Option<&str>, owner_pubkey: &str, + effective_permission_policy: Option, ) -> serde_json::Value { use crate::managed_agents::{ known_acp_runtime, resolve_session_title, DISPLAY_NAME_ENV_VAR, SESSION_TITLE_ENV_VAR, @@ -101,6 +106,20 @@ pub(super) fn build_launch_block( policy_env.insert("BUZZ_ACP_TEAM_INSTRUCTIONS".into(), value); } + // Permission policy: use the caller-resolved value (per-agent → global → + // built-in), falling back to the built-in default if the caller did not + // provide one. Tests pass `None`; production callers pass the result of + // `resolve_effective_permission_policy(record, global_config)`. + { + let policy = effective_permission_policy.unwrap_or_else( + crate::managed_agents::permission_policy::PermissionPolicy::desktop_default, + ); + policy_env.insert( + "BUZZ_ACP_PERMISSION_POLICY".into(), + policy.as_str().to_string(), + ); + } + serde_json::json!({ "command": descriptor.command, "args": descriptor.args, @@ -149,6 +168,10 @@ pub(super) fn build_deploy_payload( crate::managed_agents::resolve_effective_harness_descriptor(record, &personas, &global) .map_err(|error| crate::managed_agents::user_facing_harness_error(&error))?; let owner_pubkey = super::workspace_owner_hex(state)?; + let (effective_policy, _) = + crate::managed_agents::permission_policy::resolve_effective_permission_policy( + record, &global, + ); let launch = build_launch_block( record, &descriptor, @@ -156,6 +179,7 @@ pub(super) fn build_deploy_payload( effective.system_prompt.value.as_deref(), effective.model.value.as_deref(), &owner_pubkey, + Some(effective_policy), ); let effective_parallelism = @@ -267,6 +291,7 @@ mod tests { Some("prompt"), Some("model"), "owner-hex", + None, ); assert_eq!(launch["command"], "goose"); @@ -305,7 +330,7 @@ mod tests { env: BTreeMap::new(), }; - let launch = build_launch_block(&record, &descriptor, &[], None, None, "owner-hex"); + let launch = build_launch_block(&record, &descriptor, &[], None, None, "owner-hex", None); assert_eq!( launch["policy_env"]["BUZZ_ACP_AGENTS"], @@ -327,7 +352,7 @@ mod tests { env: BTreeMap::new(), }; - let launch = build_launch_block(&record, &descriptor, &[], None, None, "owner-hex"); + let launch = build_launch_block(&record, &descriptor, &[], None, None, "owner-hex", None); assert_eq!( launch["policy_env"]["BUZZ_ACP_AGENTS"], "8", @@ -357,7 +382,7 @@ mod tests { }; let cap = crate::managed_agents::parallelism::OPENCLAW_MAX_PARALLELISM; - let launch = build_launch_block(&record, &descriptor, &[], None, None, "owner-hex"); + let launch = build_launch_block(&record, &descriptor, &[], None, None, "owner-hex", None); let effective_parallelism = crate::managed_agents::effective_parallelism(&descriptor.command, record.parallelism); let payload = deploy_payload_json( @@ -402,7 +427,7 @@ mod tests { env: BTreeMap::new(), }; - let launch = build_launch_block(&record, &descriptor, &[], None, None, "owner-hex"); + let launch = build_launch_block(&record, &descriptor, &[], None, None, "owner-hex", None); let effective_parallelism = crate::managed_agents::effective_parallelism(&descriptor.command, record.parallelism); let payload = deploy_payload_json( @@ -448,7 +473,7 @@ mod tests { }; let cap = crate::managed_agents::parallelism::OPENCLAW_MAX_PARALLELISM; - let launch = build_launch_block(&record, &descriptor, &[], None, None, "owner-hex"); + let launch = build_launch_block(&record, &descriptor, &[], None, None, "owner-hex", None); let effective_parallelism = crate::managed_agents::effective_parallelism(&descriptor.command, record.parallelism); let payload = deploy_payload_json( @@ -475,4 +500,81 @@ mod tests { "legacy top-level parallelism must match launch.policy_env — both must be {cap}" ); } + + /// `build_launch_block` with an explicit `allow` policy injects `allow`. + #[test] + fn launch_block_explicit_allow_policy_injected() { + let record = record(); + let descriptor = EffectiveHarnessDescriptor { + command: "goose".into(), + args: vec![], + env: BTreeMap::new(), + }; + let launch = build_launch_block( + &record, + &descriptor, + &[], + None, + None, + "owner-hex", + Some(crate::managed_agents::permission_policy::PermissionPolicy::Allow), + ); + assert_eq!( + launch["policy_env"]["BUZZ_ACP_PERMISSION_POLICY"], "allow", + "explicit allow policy must be injected into policy_env" + ); + } + + /// `build_launch_block` with `None` (test callers / no global) falls back to + /// the built-in desktop default (`ask`). + #[test] + fn launch_block_none_policy_falls_back_to_built_in_ask() { + let record = record(); + let descriptor = EffectiveHarnessDescriptor { + command: "goose".into(), + args: vec![], + env: BTreeMap::new(), + }; + let launch = build_launch_block(&record, &descriptor, &[], None, None, "owner-hex", None); + assert_eq!( + launch["policy_env"]["BUZZ_ACP_PERMISSION_POLICY"], "ask", + "None effective_permission_policy must fall back to built-in ask" + ); + } + + /// Production deploy path: global `allow` override is respected when the + /// record has no per-agent policy, matching the local-spawn resolver. + #[test] + fn launch_block_global_allow_policy_used_when_record_has_none() { + let mut record = record(); + record.permission_policy = None; + let descriptor = EffectiveHarnessDescriptor { + command: "goose".into(), + args: vec![], + env: BTreeMap::new(), + }; + let global = crate::managed_agents::global_config::GlobalAgentConfig { + permission_policy: Some( + crate::managed_agents::permission_policy::PermissionPolicy::Allow, + ), + ..Default::default() + }; + let (effective_policy, _) = + crate::managed_agents::permission_policy::resolve_effective_permission_policy( + &record, &global, + ); + let launch = build_launch_block( + &record, + &descriptor, + &[], + None, + None, + "owner-hex", + Some(effective_policy), + ); + assert_eq!( + launch["policy_env"]["BUZZ_ACP_PERMISSION_POLICY"], "allow", + "global allow policy must be injected when record has no per-agent policy" + ); + } } diff --git a/desktop/src-tauri/src/commands/agents_tests.rs b/desktop/src-tauri/src/commands/agents_tests.rs index 54a03e2babe..ec52fc1832a 100644 --- a/desktop/src-tauri/src/commands/agents_tests.rs +++ b/desktop/src-tauri/src/commands/agents_tests.rs @@ -58,6 +58,7 @@ fn bare_agent_record( source_team_persona_slug: None, catalog_source: None, relay_mesh: None, + permission_policy: None, auto_restart_on_config_change: false, definition_respond_to: None, definition_respond_to_allowlist: vec![], @@ -482,6 +483,7 @@ fn deploy_payload_matches_the_shared_full_launch_fixture() { None, Some("gpt-5"), "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + None, ); let agent = deploy_payload_json( &record, 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 8ff7cfbd9bd..b33c7feaa9d 100644 --- a/desktop/src-tauri/src/commands/personas/delete_cascade_tests.rs +++ b/desktop/src-tauri/src/commands/personas/delete_cascade_tests.rs @@ -66,6 +66,7 @@ fn make_agent( source_team_persona_slug: None, catalog_source: None, relay_mesh: None, + permission_policy: None, auto_restart_on_config_change: false, definition_respond_to: None, definition_respond_to_allowlist: vec![], 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 1005a83432d..327aef8a5bb 100644 --- a/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs +++ b/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs @@ -215,6 +215,7 @@ fn local_agent() -> ManagedAgentRecord { definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, + permission_policy: 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 b769d74d7bb..49d828b1e60 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot/fidelity_tests.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot/fidelity_tests.rs @@ -64,6 +64,7 @@ fn make_definition(slug: &str) -> ManagedAgentRecord { definition_respond_to_allowlist: vec![], definition_parallelism: None, relay_mesh: None, + permission_policy: None, } } diff --git a/desktop/src-tauri/src/commands/personas/snapshot/import.rs b/desktop/src-tauri/src/commands/personas/snapshot/import.rs index d7f0323304b..f4a4201029b 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot/import.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot/import.rs @@ -654,6 +654,7 @@ pub async fn confirm_agent_snapshot_import( relay_mesh: None, runtime: snapshot.definition.runtime.clone(), name_pool: snapshot.definition.name_pool.clone(), + permission_policy: None, }; records.push(record.clone()); diff --git a/desktop/src-tauri/src/commands/personas/snapshot/tests.rs b/desktop/src-tauri/src/commands/personas/snapshot/tests.rs index c453b09a9de..8e000e926c8 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot/tests.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot/tests.rs @@ -73,6 +73,7 @@ fn make_definition(slug: &str) -> ManagedAgentRecord { definition_respond_to_allowlist: vec![], definition_parallelism: None, relay_mesh: None, + permission_policy: 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 c60215ae4dd..9a066156e38 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 @@ -58,6 +58,7 @@ fn agent(persona_id: &str, name: &str, display_name: Option<&str>) -> ManagedAge definition_respond_to_allowlist: vec![], definition_parallelism: None, relay_mesh: None, + permission_policy: None, } } diff --git a/desktop/src-tauri/src/commands/team_snapshot.rs b/desktop/src-tauri/src/commands/team_snapshot.rs index 97cd11933d7..c8990738fdd 100644 --- a/desktop/src-tauri/src/commands/team_snapshot.rs +++ b/desktop/src-tauri/src/commands/team_snapshot.rs @@ -609,6 +609,7 @@ pub async fn confirm_team_snapshot_import( definition_respond_to_allowlist: definition.respond_to_allowlist.clone(), definition_parallelism: minted_parallelism, relay_mesh: None, + permission_policy: 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 c9a6d8812a5..3ec5ec16a8d 100644 --- a/desktop/src-tauri/src/commands/team_snapshot/tests.rs +++ b/desktop/src-tauri/src/commands/team_snapshot/tests.rs @@ -229,6 +229,7 @@ fn team_export_with_instance_and_memory_level_uses_supplied_entries() { definition_respond_to_allowlist: vec![], definition_parallelism: None, relay_mesh: None, + permission_policy: 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 4a7b80079d8..13f75c2eff4 100644 --- a/desktop/src-tauri/src/managed_agents/agent_events.rs +++ b/desktop/src-tauri/src/managed_agents/agent_events.rs @@ -216,6 +216,7 @@ mod tests { definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, + permission_policy: None, } } 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 8508c27073d..e553916c886 100644 --- a/desktop/src-tauri/src/managed_agents/agent_snapshot_envelope.rs +++ b/desktop/src-tauri/src/managed_agents/agent_snapshot_envelope.rs @@ -416,6 +416,7 @@ mod tests { definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, + permission_policy: None, agent_command_override: None, persona_source_version: None, provider: 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 b4492418e59..fc9759657cb 100644 --- a/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs +++ b/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs @@ -72,6 +72,7 @@ fn minimal_record() -> ManagedAgentRecord { definition_respond_to_allowlist: vec!["abc123def".to_string()], definition_parallelism: Some(4), relay_mesh: None, + permission_policy: None, } } 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 62caffeb2e4..08ce59cf5a7 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 @@ -115,6 +115,7 @@ fn test_record() -> ManagedAgentRecord { definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, + permission_policy: None, agent_command_override: None, persona_source_version: None, provider: None, diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests.rs b/desktop/src-tauri/src/managed_agents/discovery/tests.rs index 6fe6a77521b..df6ca23dff3 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -12,6 +12,9 @@ use super::{ }; use crate::managed_agents::AcpAvailabilityStatus; +mod record_helpers; +use record_helpers::record_with; + #[test] fn resolves_known_avatar_for_bare_command() { let avatar_url = managed_agent_avatar_url("goose").expect("goose avatar should resolve"); @@ -222,70 +225,6 @@ fn effective_agent_command_explicit_override_wins() { ); } -/// Minimal record for `record_agent_command` tests. Only the resolution -/// inputs (runtime / persona_id / agent_command_override) vary. -fn record_with( - runtime: Option<&str>, - persona_id: Option<&str>, - override_cmd: Option<&str>, -) -> crate::managed_agents::types::ManagedAgentRecord { - crate::managed_agents::types::ManagedAgentRecord { - pubkey: String::new(), - name: "r".to_string(), - persona_id: persona_id.map(str::to_string), - private_key_nsec: String::new(), - auth_tag: None, - relay_url: String::new(), - avatar_url: None, - acp_command: String::new(), - agent_command: String::new(), - agent_command_override: override_cmd.map(str::to_string), - agent_args: vec![], - mcp_command: String::new(), - turn_timeout_seconds: 0, - idle_timeout_seconds: None, - max_turn_duration_seconds: None, - parallelism: 1, - system_prompt: None, - model: None, - provider: None, - persona_source_version: None, - start_on_app_launch: false, - auto_restart_on_config_change: true, - runtime_pid: None, - backend: Default::default(), - backend_agent_id: None, - provider_binary_path: None, - team_id: None, - persona_team_dir: None, - persona_name_in_team: None, - env_vars: std::collections::BTreeMap::new(), - created_at: String::new(), - updated_at: String::new(), - last_started_at: None, - last_stopped_at: None, - last_exit_code: None, - last_error: None, - last_error_code: None, - respond_to: Default::default(), - respond_to_allowlist: vec![], - display_name: None, - slug: None, - runtime: runtime.map(str::to_string), - name_pool: Vec::new(), - is_builtin: false, - is_active: true, - shared: false, - source_team: None, - source_team_persona_slug: None, - catalog_source: None, - definition_respond_to: None, - definition_respond_to_allowlist: Vec::new(), - definition_parallelism: 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/discovery/tests/record_helpers.rs b/desktop/src-tauri/src/managed_agents/discovery/tests/record_helpers.rs new file mode 100644 index 00000000000..8d659a8fcf2 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/discovery/tests/record_helpers.rs @@ -0,0 +1,63 @@ +/// Minimal record for command-resolution tests. +pub(super) fn record_with( + runtime: Option<&str>, + persona_id: Option<&str>, + override_cmd: Option<&str>, +) -> crate::managed_agents::types::ManagedAgentRecord { + crate::managed_agents::types::ManagedAgentRecord { + pubkey: String::new(), + name: "r".to_string(), + persona_id: persona_id.map(str::to_string), + private_key_nsec: String::new(), + auth_tag: None, + relay_url: String::new(), + avatar_url: None, + acp_command: String::new(), + agent_command: String::new(), + agent_command_override: override_cmd.map(str::to_string), + agent_args: vec![], + mcp_command: String::new(), + turn_timeout_seconds: 0, + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + parallelism: 1, + system_prompt: None, + model: None, + provider: None, + persona_source_version: None, + start_on_app_launch: false, + auto_restart_on_config_change: true, + runtime_pid: None, + backend: Default::default(), + backend_agent_id: None, + provider_binary_path: None, + team_id: None, + persona_team_dir: None, + persona_name_in_team: None, + env_vars: std::collections::BTreeMap::new(), + created_at: String::new(), + updated_at: String::new(), + last_started_at: None, + last_stopped_at: None, + last_exit_code: None, + last_error: None, + last_error_code: None, + respond_to: Default::default(), + respond_to_allowlist: vec![], + display_name: None, + slug: None, + runtime: runtime.map(str::to_string), + name_pool: Vec::new(), + is_builtin: false, + is_active: true, + shared: false, + source_team: None, + source_team_persona_slug: None, + catalog_source: None, + definition_respond_to: None, + definition_respond_to_allowlist: Vec::new(), + definition_parallelism: None, + relay_mesh: None, + permission_policy: None, + } +} 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 c8e437809ce..230b9456441 100644 --- a/desktop/src-tauri/src/managed_agents/effective_config/tests.rs +++ b/desktop/src-tauri/src/managed_agents/effective_config/tests.rs @@ -88,6 +88,7 @@ fn record( source_team_persona_slug: None, catalog_source: None, relay_mesh: None, + permission_policy: None, auto_restart_on_config_change: false, definition_respond_to: None, definition_respond_to_allowlist: vec![], 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 162f447981b..50e30823944 100644 --- a/desktop/src-tauri/src/managed_agents/global_config/mod.rs +++ b/desktop/src-tauri/src/managed_agents/global_config/mod.rs @@ -70,6 +70,14 @@ pub struct GlobalAgentConfig { /// Preferred ACP runtime for definitions without an explicit runtime. #[serde(default)] pub preferred_runtime: Option, + /// Fleet-wide permission policy default. `None` = use the built-in + /// desktop default (`ask`). Per-agent `permission_policy` takes precedence. + /// + /// Semantics match the per-agent field: `ask` shows the Allow/Deny card, + /// `allow` auto-approves the unique `allow_once` option (explicit opt-in + /// only), `reject` auto-denies. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub permission_policy: 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 553596e226c..d9eb6f4c1db 100644 --- a/desktop/src-tauri/src/managed_agents/global_config/tests.rs +++ b/desktop/src-tauri/src/managed_agents/global_config/tests.rs @@ -267,6 +267,7 @@ fn roundtrip_serialization() { provider: Some("anthropic".to_string()), model: Some("claude-opus-4".to_string()), preferred_runtime: Some("claude".to_string()), + permission_policy: None, }; let json = serde_json::to_string(&config).expect("serialize"); let back: GlobalAgentConfig = serde_json::from_str(&json).expect("deserialize"); @@ -348,6 +349,7 @@ fn bare_record() -> ManagedAgentRecord { source_team_persona_slug: None, catalog_source: None, relay_mesh: None, + permission_policy: None, auto_restart_on_config_change: false, definition_respond_to: None, definition_respond_to_allowlist: vec![], @@ -592,6 +594,7 @@ fn populated_global_config_round_trips() { provider: Some("anthropic".to_string()), model: Some("claude-opus-4-5".to_string()), preferred_runtime: None, + permission_policy: None, }; let json = serde_json::to_string(&original).expect("serialization must not fail"); let decoded: GlobalAgentConfig = diff --git a/desktop/src-tauri/src/managed_agents/mod.rs b/desktop/src-tauri/src/managed_agents/mod.rs index fe90ce430fd..a0f9c6f9f58 100644 --- a/desktop/src-tauri/src/managed_agents/mod.rs +++ b/desktop/src-tauri/src/managed_agents/mod.rs @@ -3,6 +3,7 @@ mod agent_env; pub(crate) mod agent_events; pub(crate) mod agent_snapshot; pub(crate) mod agent_snapshot_envelope; +pub(crate) mod permission_policy; pub(crate) mod team_snapshot; pub(crate) use access_policy::{owner_only, owner_only_access_build, projected_access_with_policy}; pub(crate) use agent_env::{ diff --git a/desktop/src-tauri/src/managed_agents/nest/tests.rs b/desktop/src-tauri/src/managed_agents/nest/tests.rs index cbef171f6fd..1288c5ceb33 100644 --- a/desktop/src-tauri/src/managed_agents/nest/tests.rs +++ b/desktop/src-tauri/src/managed_agents/nest/tests.rs @@ -502,6 +502,7 @@ fn make_agent(name: &str, persona_id: Option<&str>) -> ManagedAgentRecord { definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, + permission_policy: None, } } diff --git a/desktop/src-tauri/src/managed_agents/parallelism.rs b/desktop/src-tauri/src/managed_agents/parallelism.rs index e1691575b11..0253d48158e 100644 --- a/desktop/src-tauri/src/managed_agents/parallelism.rs +++ b/desktop/src-tauri/src/managed_agents/parallelism.rs @@ -117,6 +117,7 @@ mod tests { definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, + permission_policy: None, } } diff --git a/desktop/src-tauri/src/managed_agents/permission_policy.rs b/desktop/src-tauri/src/managed_agents/permission_policy.rs new file mode 100644 index 00000000000..c962eb327c5 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/permission_policy.rs @@ -0,0 +1,160 @@ +//! Permission policy enum, source attribution, and the precedence resolver. +//! +//! `BUZZ_ACP_PERMISSION_POLICY` is in `RESERVED_ENV_KEYS` so users cannot +//! override it via the env-vars UI — a manual override would make the running +//! harness use a different policy than the saved/UI-visible setting. + +use serde::{Deserialize, Serialize}; + +use super::types::ManagedAgentRecord; + +/// How the agent answers `session/request_permission` requests. +/// +/// - `Ask` — show an Allow/Deny card; auto-deny after 300 s (desktop default). +/// - `Allow` — auto-select the unique `allow_once` option; explicit opt-in. +/// - `Reject` — deny immediately; headless/CLI default. +/// +/// Wire format is lowercase to match the harness CLI vocabulary and the +/// `BUZZ_ACP_PERMISSION_POLICY` env var the harness reads. +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "lowercase")] +pub enum PermissionPolicy { + Ask, + Allow, + Reject, +} + +impl PermissionPolicy { + /// The env-var wire string consumed by the harness + /// (`BUZZ_ACP_PERMISSION_POLICY`). + pub fn as_str(self) -> &'static str { + match self { + Self::Ask => "ask", + Self::Allow => "allow", + Self::Reject => "reject", + } + } + + /// The built-in desktop default: show the Allow/Deny card. + /// + /// Headless / bare-CLI callers use `Reject` — they never have a UI to + /// answer a card. The desktop injects the resolved effective policy so + /// headless sessions spawned by the desktop still pick up the user's + /// choice. + pub fn desktop_default() -> Self { + Self::Ask + } +} + +/// Where the effective [`PermissionPolicy`] came from. Serialized as a +/// `snake_case` string for TypeScript's exhaustive-switch pattern. +#[derive(Debug, Clone, Copy, Serialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum PermissionPolicySource { + /// Set explicitly on this agent record. + Agent, + /// Inherited from the global agent config. + GlobalDefault, + /// Neither per-agent nor global is set; using the built-in desktop default. + BuiltIn, +} + +/// Resolve the effective permission policy for an agent. +/// +/// Precedence (highest first): +/// 1. `record.permission_policy` — per-agent override. +/// 2. `global.permission_policy` — fleet-wide default. +/// 3. [`PermissionPolicy::desktop_default`] — built-in. +pub fn resolve_effective_permission_policy( + record: &ManagedAgentRecord, + global: &super::global_config::GlobalAgentConfig, +) -> (PermissionPolicy, PermissionPolicySource) { + if let Some(policy) = record.permission_policy { + return (policy, PermissionPolicySource::Agent); + } + if let Some(policy) = global.permission_policy { + return (policy, PermissionPolicySource::GlobalDefault); + } + ( + PermissionPolicy::desktop_default(), + PermissionPolicySource::BuiltIn, + ) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::managed_agents::global_config::GlobalAgentConfig; + + fn empty_record() -> ManagedAgentRecord { + serde_json::from_value(serde_json::json!({ + "pubkey": "abcd1234", + "name": "test", + "display_name": "Test", + "private_key_nsec": "nsec1fake", + "relay_url": "wss://relay.example", + "acp_command": "buzz-acp", + "agent_command": "goose", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 300, + "idle_timeout_seconds": 900, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z" + })) + .expect("minimal ManagedAgentRecord") + } + + #[test] + fn test_per_agent_policy_beats_global_and_built_in() { + let mut record = empty_record(); + record.permission_policy = Some(PermissionPolicy::Allow); + let global = GlobalAgentConfig { + permission_policy: Some(PermissionPolicy::Reject), + ..Default::default() + }; + + let (policy, source) = resolve_effective_permission_policy(&record, &global); + assert_eq!(policy, PermissionPolicy::Allow); + assert_eq!(source, PermissionPolicySource::Agent); + } + + #[test] + fn test_global_policy_beats_built_in_when_no_per_agent() { + let mut record = empty_record(); + record.permission_policy = None; + let global = GlobalAgentConfig { + permission_policy: Some(PermissionPolicy::Allow), + ..Default::default() + }; + + let (policy, source) = resolve_effective_permission_policy(&record, &global); + assert_eq!(policy, PermissionPolicy::Allow); + assert_eq!(source, PermissionPolicySource::GlobalDefault); + } + + #[test] + fn test_built_in_used_when_neither_per_agent_nor_global_is_set() { + let mut record = empty_record(); + record.permission_policy = None; + let global = GlobalAgentConfig::default(); // permission_policy = None + + let (policy, source) = resolve_effective_permission_policy(&record, &global); + assert_eq!(policy, PermissionPolicy::Ask); // desktop_default + assert_eq!(source, PermissionPolicySource::BuiltIn); + } + + #[test] + fn test_per_agent_reject_beats_global_allow() { + let mut record = empty_record(); + record.permission_policy = Some(PermissionPolicy::Reject); + let global = GlobalAgentConfig { + permission_policy: Some(PermissionPolicy::Allow), + ..Default::default() + }; + + let (policy, source) = resolve_effective_permission_policy(&record, &global); + assert_eq!(policy, PermissionPolicy::Reject); + assert_eq!(source, PermissionPolicySource::Agent); + } +} 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 0580b12ce21..91110836b41 100644 --- a/desktop/src-tauri/src/managed_agents/persona_events/tests.rs +++ b/desktop/src-tauri/src/managed_agents/persona_events/tests.rs @@ -58,6 +58,7 @@ pub(super) fn sample_record() -> ManagedAgentRecord { definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, + permission_policy: None, } } diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index c072448ff13..2fe73686e9b 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -659,6 +659,9 @@ mod tests { use super::*; use crate::managed_agents::discovery::known_acp_runtime_exact; + #[path = "effective_agent_env_tests.rs"] + mod effective_agent_env_tests; + /// Build a minimal `EffectiveAgentEnv` with the given env map and command. fn make_env(command: &str, env: BTreeMap) -> EffectiveAgentEnv { let runtime = known_acp_runtime_exact(command); @@ -1461,91 +1464,6 @@ mod tests { assert!(json["setup_copy"].as_str().unwrap().contains("codex login")); } - // ── resolve_effective_agent_env ───────────────────────────────────────── - - #[test] - fn resolve_effective_agent_env_user_env_wins_over_structured_fields() { - // A record whose env_vars explicitly set provider/model must win over - // any baked defaults. In OSS test builds the baked map is empty, so - // this test validates the user-env layer is present in the output. - let mut env_vars = BTreeMap::new(); - env_vars.insert("BUZZ_AGENT_PROVIDER".to_string(), "anthropic".to_string()); - env_vars.insert( - "BUZZ_AGENT_MODEL".to_string(), - "claude-opus-4-5".to_string(), - ); - - // Minimal record: only the fields resolve_effective_agent_env reads. - let record = crate::managed_agents::types::ManagedAgentRecord { - pubkey: "test-pubkey".to_string(), - name: "test-agent".to_string(), - persona_id: None, - private_key_nsec: String::new(), - auth_tag: None, - relay_url: String::new(), - avatar_url: None, - acp_command: "buzz-acp".to_string(), - agent_command: "buzz-agent".to_string(), - agent_command_override: None, - agent_args: vec![], - mcp_command: String::new(), - turn_timeout_seconds: 320, - idle_timeout_seconds: None, - max_turn_duration_seconds: None, - parallelism: 1, - system_prompt: None, - model: None, - provider: None, - persona_source_version: None, - env_vars, - start_on_app_launch: false, - auto_restart_on_config_change: true, - runtime_pid: None, - backend: Default::default(), - backend_agent_id: None, - provider_binary_path: None, - team_id: None, - persona_team_dir: None, - persona_name_in_team: None, - created_at: String::new(), - updated_at: String::new(), - last_started_at: None, - last_stopped_at: None, - last_exit_code: None, - last_error: None, - last_error_code: None, - respond_to: Default::default(), - respond_to_allowlist: vec![], - display_name: None, - slug: None, - runtime: None, - name_pool: Vec::new(), - is_builtin: false, - is_active: true, - shared: false, - source_team: None, - source_team_persona_slug: None, - catalog_source: None, - definition_respond_to: None, - definition_respond_to_allowlist: Vec::new(), - definition_parallelism: None, - relay_mesh: None, - }; - - let runtime = known_acp_runtime_exact("buzz-agent"); - let effective = resolve_effective_agent_env(&record, &[], runtime, &Default::default()); - - // User env_vars must be present in the output (last-write-wins). - assert_eq!( - effective.env.get("BUZZ_AGENT_PROVIDER").map(String::as_str), - Some("anthropic") - ); - assert_eq!( - effective.env.get("BUZZ_AGENT_MODEL").map(String::as_str), - Some("claude-opus-4-5") - ); - } - // ── provider-specific model fallback tests ──────────────────────────── #[test] diff --git a/desktop/src-tauri/src/managed_agents/readiness/tests/effective_agent_env_tests.rs b/desktop/src-tauri/src/managed_agents/readiness/tests/effective_agent_env_tests.rs new file mode 100644 index 00000000000..a8985de0c4b --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/readiness/tests/effective_agent_env_tests.rs @@ -0,0 +1,81 @@ +use super::{known_acp_runtime_exact, resolve_effective_agent_env}; +use std::collections::BTreeMap; + +#[test] +fn user_env_wins_over_structured_fields() { + let mut env_vars = BTreeMap::new(); + env_vars.insert("BUZZ_AGENT_PROVIDER".to_string(), "anthropic".to_string()); + env_vars.insert( + "BUZZ_AGENT_MODEL".to_string(), + "claude-opus-4-5".to_string(), + ); + + let record = crate::managed_agents::types::ManagedAgentRecord { + pubkey: "test-pubkey".to_string(), + name: "test-agent".to_string(), + persona_id: None, + private_key_nsec: String::new(), + auth_tag: None, + relay_url: String::new(), + avatar_url: None, + acp_command: "buzz-acp".to_string(), + agent_command: "buzz-agent".to_string(), + agent_command_override: None, + agent_args: vec![], + mcp_command: String::new(), + turn_timeout_seconds: 320, + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + parallelism: 1, + system_prompt: None, + model: None, + provider: None, + persona_source_version: None, + env_vars, + start_on_app_launch: false, + auto_restart_on_config_change: true, + runtime_pid: None, + backend: Default::default(), + backend_agent_id: None, + provider_binary_path: None, + team_id: None, + persona_team_dir: None, + persona_name_in_team: None, + created_at: String::new(), + updated_at: String::new(), + last_started_at: None, + last_stopped_at: None, + last_exit_code: None, + last_error: None, + last_error_code: None, + respond_to: Default::default(), + respond_to_allowlist: vec![], + display_name: None, + slug: None, + runtime: None, + name_pool: Vec::new(), + is_builtin: false, + is_active: true, + shared: false, + source_team: None, + source_team_persona_slug: None, + catalog_source: None, + definition_respond_to: None, + definition_respond_to_allowlist: Vec::new(), + definition_parallelism: None, + relay_mesh: None, + permission_policy: None, + }; + + let runtime = known_acp_runtime_exact("buzz-agent"); + let effective = resolve_effective_agent_env(&record, &[], runtime, &Default::default()); + + assert_eq!( + effective.env.get("BUZZ_AGENT_PROVIDER").map(String::as_str), + Some("anthropic") + ); + assert_eq!( + effective.env.get("BUZZ_AGENT_MODEL").map(String::as_str), + Some("claude-opus-4-5") + ); +} diff --git a/desktop/src-tauri/src/managed_agents/reserved_env_keys.rs b/desktop/src-tauri/src/managed_agents/reserved_env_keys.rs index 8698d3a51d1..3a972ed8493 100644 --- a/desktop/src-tauri/src/managed_agents/reserved_env_keys.rs +++ b/desktop/src-tauri/src/managed_agents/reserved_env_keys.rs @@ -70,6 +70,11 @@ pub(crate) const RESERVED_ENV_KEYS: &[&str] = &[ // for same-session sweep decisions. "BUZZ_MANAGED_AGENT", "BUZZ_MANAGED_AGENT_START_NONCE", + // Permission policy gate: Desktop resolves the effective policy + // (per-agent > global > built-in) and injects it here. A user-supplied + // override would make the running harness use a different policy than the + // saved/UI-visible setting — exactly the truthfulness failure #4938 fixes. + "BUZZ_ACP_PERMISSION_POLICY", ]; pub(crate) fn is_reserved_env_key(key: &str) -> bool { diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index ec804869c42..adb23279470 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -7,9 +7,10 @@ use super::agent_env::build_buzz_agent_provider_defaults; use crate::{ managed_agents::{ append_log_marker, known_acp_runtime, login_shell_path, managed_agent_log_path, - missing_command_message, normalize_agent_args, open_log_file, resolve_command, - spawn_key_refusal, KnownAcpRuntime, ManagedAgentPairRuntime, ManagedAgentRecord, - ManagedAgentRuntimeKey, ManagedAgentSummary, + missing_command_message, normalize_agent_args, open_log_file, + permission_policy::resolve_effective_permission_policy, resolve_command, spawn_key_refusal, + KnownAcpRuntime, ManagedAgentPairRuntime, ManagedAgentRecord, ManagedAgentRuntimeKey, + ManagedAgentSummary, }, util::now_iso, }; @@ -296,6 +297,9 @@ pub fn build_managed_agent_summary( .unwrap_or("") .to_string(); + let (effective_permission_policy_summary, effective_permission_policy_source) = + resolve_effective_permission_policy(record, global_config); + Ok(ManagedAgentSummary { pubkey: record.pubkey.clone(), name: record.name.clone(), @@ -338,6 +342,8 @@ pub fn build_managed_agent_summary( log_path, respond_to: record.respond_to, respond_to_allowlist: record.respond_to_allowlist.clone(), + permission_policy: effective_permission_policy_summary, + permission_policy_source: effective_permission_policy_source, }) } @@ -761,6 +767,14 @@ pub fn spawn_agent_child( command.env_remove(key); } + // Inject BUZZ_ACP_PERMISSION_POLICY — resolved here so the running process + // and the UI-visible setting are always in sync. + let (effective_permission_policy, _) = resolve_effective_permission_policy(record, &global); + command.env( + "BUZZ_ACP_PERMISSION_POLICY", + effective_permission_policy.as_str(), + ); + command.env("BUZZ_ACP_RELAY_OBSERVER", "true"); // ── Git credential helper for Buzz relay ────────────────────────── @@ -844,6 +858,7 @@ pub fn spawn_agent_child( system_prompt: effective_prompt.as_deref(), model: effective_model.as_deref(), provider: effective_provider.as_deref(), + permission_policy: effective_permission_policy, }, ); diff --git a/desktop/src-tauri/src/managed_agents/runtime/test_fixtures.rs b/desktop/src-tauri/src/managed_agents/runtime/test_fixtures.rs index 9836d983ed3..70ed9db6ab9 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/test_fixtures.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/test_fixtures.rs @@ -89,5 +89,6 @@ pub(super) fn fixture( definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, + permission_policy: None, } } diff --git a/desktop/src-tauri/src/managed_agents/spawn_snapshot.rs b/desktop/src-tauri/src/managed_agents/spawn_snapshot.rs index ba2129c9841..238b263f6f1 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_snapshot.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_snapshot.rs @@ -72,6 +72,8 @@ pub(crate) struct SpawnConfigInputs<'a> { pub system_prompt: Option<&'a str>, pub model: Option<&'a str>, pub provider: Option<&'a str>, + /// Resolved effective permission policy (per-agent > global > built-in). + pub permission_policy: super::permission_policy::PermissionPolicy, } /// The effective spawn configuration of one managed-agent process. @@ -123,6 +125,10 @@ pub(crate) struct SpawnConfigSnapshot { pub idle_timeout_seconds: Option, pub max_turn_duration_seconds: Option, pub parallelism: u32, + /// Effective permission policy at spawn time. Reaches the harness via + /// `BUZZ_ACP_PERMISSION_POLICY`. Tracked in the snapshot so an edit shows + /// in the `needsRestart` diff. + pub permission_policy: String, } impl SpawnConfigSnapshot { @@ -136,6 +142,7 @@ impl SpawnConfigSnapshot { system_prompt, model, provider, + permission_policy, } = inputs; Self { acp_command: record.acp_command.clone(), @@ -174,6 +181,7 @@ impl SpawnConfigSnapshot { // pool and must badge. The diff surface consequently displays the // effective value — that is correct, it is what actually runs. parallelism: super::effective_parallelism(&descriptor.command, record.parallelism), + permission_policy: permission_policy.as_str().to_string(), } } @@ -262,6 +270,10 @@ pub(crate) fn prospective_spawn_config_snapshot( system_prompt: prompt.as_deref(), model: model.as_deref(), provider: provider.as_deref(), + permission_policy: super::permission_policy::resolve_effective_permission_policy( + record, global, + ) + .0, }) } diff --git a/desktop/src-tauri/src/managed_agents/spawn_snapshot/diff/tests.rs b/desktop/src-tauri/src/managed_agents/spawn_snapshot/diff/tests.rs index a7a8cab93e7..ca9999f17e8 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_snapshot/diff/tests.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_snapshot/diff/tests.rs @@ -28,6 +28,7 @@ fn base() -> SpawnConfigSnapshot { idle_timeout_seconds: Some(600), max_turn_duration_seconds: Some(7200), parallelism: 1, + permission_policy: "ask".into(), } } @@ -70,6 +71,9 @@ fn mutations() -> Vec { s.max_turn_duration_seconds = None }), ("parallelism", |s| s.parallelism = 8), + ("permission_policy", |s| { + s.permission_policy = "allow".into() + }), ] } diff --git a/desktop/src-tauri/src/managed_agents/spawn_snapshot/tests.rs b/desktop/src-tauri/src/managed_agents/spawn_snapshot/tests.rs index 1ceeee372f1..00acfe7bde6 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_snapshot/tests.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_snapshot/tests.rs @@ -70,6 +70,7 @@ fn record() -> ManagedAgentRecord { definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, + permission_policy: None, } } diff --git a/desktop/src-tauri/src/managed_agents/team_snapshot.rs b/desktop/src-tauri/src/managed_agents/team_snapshot.rs index 96082acc76d..9db79e4c036 100644 --- a/desktop/src-tauri/src/managed_agents/team_snapshot.rs +++ b/desktop/src-tauri/src/managed_agents/team_snapshot.rs @@ -309,6 +309,7 @@ mod tests { definition_respond_to_allowlist: vec![], definition_parallelism: None, relay_mesh: None, + permission_policy: None, } } diff --git a/desktop/src-tauri/src/managed_agents/teams_tests.rs b/desktop/src-tauri/src/managed_agents/teams_tests.rs index 1ffa60eda97..4d4297ee27e 100644 --- a/desktop/src-tauri/src/managed_agents/teams_tests.rs +++ b/desktop/src-tauri/src/managed_agents/teams_tests.rs @@ -213,6 +213,7 @@ fn managed_agent(name: &str) -> ManagedAgentRecord { source_team_persona_slug: None, catalog_source: None, relay_mesh: None, + permission_policy: None, definition_respond_to: None, definition_respond_to_allowlist: vec![], definition_parallelism: None, diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index e5be105fed0..bc8dc3d2895 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -1,16 +1,8 @@ use serde::{Deserialize, Serialize}; use std::{collections::BTreeMap, path::PathBuf, process::Child}; -#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] -#[serde(tag = "type", rename_all = "snake_case")] -pub enum BackendKind { - #[default] - Local, - Provider { - id: String, - config: serde_json::Value, - }, -} +mod backend_kind; +pub use backend_kind::BackendKind; #[derive(Debug, Clone, Serialize, Deserialize)] pub struct AgentDefinition { @@ -153,6 +145,7 @@ impl AgentDefinition { definition_respond_to_allowlist: self.respond_to_allowlist, definition_parallelism: self.parallelism, relay_mesh: None, + permission_policy: None, } } } @@ -352,6 +345,8 @@ pub struct ManagedAgentRecord { /// Preserved across mode toggles so users don't lose state. #[serde(default)] pub respond_to_allowlist: Vec, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub permission_policy: 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")] @@ -566,6 +561,8 @@ pub struct ManagedAgentSummary { pub log_path: String, pub respond_to: RespondTo, pub respond_to_allowlist: Vec, + pub permission_policy: super::permission_policy::PermissionPolicy, + pub permission_policy_source: super::permission_policy::PermissionPolicySource, } #[derive(Debug, Serialize)] diff --git a/desktop/src-tauri/src/managed_agents/types/backend_kind.rs b/desktop/src-tauri/src/managed_agents/types/backend_kind.rs new file mode 100644 index 00000000000..7a7bb00b9e5 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/types/backend_kind.rs @@ -0,0 +1,12 @@ +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] +#[serde(tag = "type", rename_all = "snake_case")] +pub enum BackendKind { + #[default] + Local, + Provider { + id: String, + config: serde_json::Value, + }, +} diff --git a/desktop/src-tauri/src/managed_agents/types/requests.rs b/desktop/src-tauri/src/managed_agents/types/requests.rs index e28b0bd461a..ae90375297e 100644 --- a/desktop/src-tauri/src/managed_agents/types/requests.rs +++ b/desktop/src-tauri/src/managed_agents/types/requests.rs @@ -253,6 +253,12 @@ pub struct UpdateManagedAgentRequest { /// normalized server-side). #[serde(default)] pub respond_to_allowlist: Option>, + /// Absent = don't touch. `null` = clear per-agent override (revert to + /// global/built-in). Present string = set per-agent override. + /// Remote deployed agents: rejected server-side (displayed read-only in UI). + #[serde(default, deserialize_with = "crate::util::double_option")] + pub permission_policy: + Option>, } #[cfg(test)] diff --git a/desktop/src-tauri/src/managed_agents/types/tests.rs b/desktop/src-tauri/src/managed_agents/types/tests.rs index 1db7b9b5243..914568cf127 100644 --- a/desktop/src-tauri/src/managed_agents/types/tests.rs +++ b/desktop/src-tauri/src/managed_agents/types/tests.rs @@ -744,6 +744,9 @@ fn summary_fixture( log_path: String::new(), respond_to: RespondTo::OwnerOnly, respond_to_allowlist: Vec::new(), + permission_policy: crate::managed_agents::permission_policy::PermissionPolicy::Ask, + permission_policy_source: + crate::managed_agents::permission_policy::PermissionPolicySource::BuiltIn, } } diff --git a/desktop/src/features/agents/ui/AgentConfigFields.tsx b/desktop/src/features/agents/ui/AgentConfigFields.tsx index 11f68e8a564..67b5513bfdc 100644 --- a/desktop/src/features/agents/ui/AgentConfigFields.tsx +++ b/desktop/src/features/agents/ui/AgentConfigFields.tsx @@ -68,6 +68,7 @@ export const EMPTY_GLOBAL_CONFIG: GlobalAgentConfig = { provider: null, model: null, preferred_runtime: null, + permission_policy: null, }; const BAKED_STRUCTURED_KEYS = new Set([ diff --git a/desktop/src/features/agents/ui/AgentDefaultsEditor.tsx b/desktop/src/features/agents/ui/AgentDefaultsEditor.tsx index 69e8b3a5b43..71b7b25446c 100644 --- a/desktop/src/features/agents/ui/AgentDefaultsEditor.tsx +++ b/desktop/src/features/agents/ui/AgentDefaultsEditor.tsx @@ -17,7 +17,7 @@ import { getGlobalAgentConfig, setGlobalAgentConfig, } from "@/shared/api/tauriGlobalAgentConfig"; -import type { GlobalAgentConfig } from "@/shared/api/types"; +import type { GlobalAgentConfig, PermissionPolicy } from "@/shared/api/types"; import { getBakedBuildEnv, type BakedEnvEntry } from "@/shared/api/tauri"; import { globalAgentConfigQueryKey } from "@/features/agents/useGlobalAgentConfig"; import { @@ -294,6 +294,36 @@ export function AgentDefaultsEditor({ value={selectedRuntime?.id ?? ""} /> + {/* Fleet-wide permission policy default */} +
+ + +
{flatLayout ? ( {configFields ? ( diff --git a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx index f7ee098833b..660ac3773d3 100644 --- a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx +++ b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx @@ -83,8 +83,6 @@ import { getProviderApiKeyLabel, } from "./agentConfigOptions"; import { useAgentDialogDefaults } from "./useAgentDialogDefaults"; -import { AgentAiDefaultsNotice } from "./AgentAiDefaults"; -import { AgentDefaultsDialog } from "./AgentDefaultsDialog"; import { useProviderApiKeyFieldState } from "./providerApiKeyFieldState"; import { resolveModelFieldStatusMessage } from "./agentConfigControls"; import { AdvancedRequiredBadge } from "./AdvancedRequiredBadge"; @@ -95,6 +93,11 @@ import { runtimeDropdownAction, usePendingHarnessSelection, } from "./addCustomHarness"; +import { + EditAgentPermissionPolicy, + useEditAgentPermissionPolicy, +} from "./EditAgentPermissionPolicy"; +import { EditAgentDefaultsSection } from "./EditAgentDefaultsSection"; export function AgentInstanceEditDialog({ agent, @@ -120,8 +123,6 @@ export function AgentInstanceEditDialog({ const runtimes = runtimesQuery.data ?? []; const [name, setName] = React.useState(agent.name); - const [aiDefaultsOpen, setAiDefaultsOpen] = React.useState(false); - const aiDefaultsTriggerRef = React.useRef(null); const [acpCommand, setAcpCommand] = React.useState(agent.acpCommand); const [agentCommand, setAgentCommand] = React.useState(agent.agentCommand); const [originalAgentCommand, setOriginalAgentCommand] = React.useState( @@ -160,6 +161,7 @@ export function AgentInstanceEditDialog({ const [respondToAllowlist, setRespondToAllowlist] = React.useState( agent.respondToAllowlist, ); + const permissionPolicy = useEditAgentPermissionPolicy(agent, open); const [showAdvancedFields, setShowAdvancedFields] = React.useState(false); const [avatarUrl, setAvatarUrl] = React.useState(agent.avatarUrl ?? ""); const [isAvatarUploadPending, setIsAvatarUploadPending] = @@ -167,8 +169,7 @@ export function AgentInstanceEditDialog({ 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. + // The open-effect re-derives the runtime id after the catalog loads. const [selectedRuntimeId, setSelectedRuntimeId] = React.useState("custom"); // Tracks whether the user has made an in-dialog runtime selection. @@ -725,6 +726,7 @@ export function AgentInstanceEditDialog({ respondToAllowlist.join(",") !== agent.respondToAllowlist.join(",") ? respondToAllowlist : undefined, + permissionPolicy: permissionPolicy.update, }; const result = await updateMutation.mutateAsync(input); @@ -944,6 +946,12 @@ export function AgentInstanceEditDialog({ onAllowlistChange={setRespondToAllowlist} onModeChange={setRespondTo} /> + {/* Provider (runtime) */} @@ -1128,21 +1136,13 @@ export function AgentInstanceEditDialog({ ) : null} - setAiDefaultsOpen(true)} - triggerRef={aiDefaultsTriggerRef} + - - {/* Advanced settings */}
+ ); + })} +
+ ); +} + export function LifecycleActivity(props: ActivityRenderClassItemProps) { if (props.item.type === "tool") { return ; @@ -55,6 +137,11 @@ export function LifecycleActivity(props: ActivityRenderClassItemProps) { const { requestLines, optionsLine } = splitPermissionText(props.item.text); const outcome = props.item.outcome; const tone = outcome ? permissionOutcomeTone(outcome) : null; + const actionable = props.item.actionable ?? false; + const requestNonce = props.item.requestNonce; + const options = props.item.options ?? []; + const authorizationReason = props.item.authorizationReason; + const deliveryFailed = props.item.deliveryFailed; return (
· {requestLines} ) : null}
- {/* Row 2: options (muted sub-line) */} - {optionsLine ? ( + {/* Row 2: authorization reason (from envelope), if present */} + {authorizationReason ? ( +
{authorizationReason}
+ ) : null} + {/* Row 3: options sub-line (legacy fallback) */} + {optionsLine && !authorizationReason ? (
{optionsLine}
) : null} - {/* Row 3: decision — only when outcome is resolved */} + {/* Row 4: Allow/Deny buttons (actionable card awaiting decision) */} + {actionable && requestNonce && !outcome ? ( + + ) : null} + {/* Row 5: decision — only when outcome is resolved */} {outcome && tone ? ( <>
diff --git a/desktop/src/features/agents/ui/agentPermissionTranscript.ts b/desktop/src/features/agents/ui/agentPermissionTranscript.ts new file mode 100644 index 00000000000..5dd6a00ca82 --- /dev/null +++ b/desktop/src/features/agents/ui/agentPermissionTranscript.ts @@ -0,0 +1,80 @@ +import { asRecord, asString } from "./agentSessionUtils"; + +export function describePermissionRequest(payload: Record) { + const params = asRecord(payload.params); + const title = + asString(params.title) ?? + asString(params.message) ?? + asString(params.reason) ?? + "Permission requested"; + const toolCallId = + asString(params.toolCallId) ?? asString(params.tool_call_id); + + const optionNames = new Map(); + const options: Array<{ + optionId: string; + kind: string; + label?: string; + }> = []; + const optionDisplayNames: string[] = []; + if (Array.isArray(params.options)) { + for (const option of params.options) { + const record = asRecord(option); + const optionId = asString(record.optionId); + const kind = asString(record.kind); + const label = asString(record.label) ?? asString(record.name); + const displayName = + asString(record.name) ?? + asString(record.kind) ?? + asString(record.optionId); + if (displayName) optionDisplayNames.push(displayName); + if (optionId && kind) { + optionNames.set(optionId, kind); + options.push({ optionId, kind, ...(label ? { label } : {}) }); + } + } + } + + const detail: string[] = []; + if (title !== "Permission requested") detail.push(title); + if (toolCallId) detail.push(`Tool call: ${toolCallId}`); + if (optionDisplayNames.length > 0) { + detail.push(`Options: ${optionDisplayNames.join(", ")}`); + } + + return { + title, + text: detail.join("\n"), + optionNames, + options, + descriptor: { + renderClass: "permission" as const, + label: "Permission requested", + preview: title, + action: { verb: "Requested", object: title }, + tone: "admin" as const, + operation: "session/request_permission", + object: title, + source: "acp" as const, + groupKey: "permission:request", + }, + }; +} + +/** Format a human-readable ACP permission outcome. */ +export function describePermissionOutcome( + outcome: string, + optionId: string | null, + optionNames: Map, +): string { + if (outcome === "cancelled") return "Cancelled"; + if (outcome === "timed_out") return "Timed out"; + if (outcome === "uncertain") { + return "Approval outcome unknown; agent process stopped before it could continue."; + } + if (outcome === "selected" && optionId) { + const kind = optionNames.get(optionId) ?? optionId; + return `${kind.startsWith("reject") ? "Denied" : "Approved"} (${kind})`; + } + return outcome; +} diff --git a/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs b/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs index b4a139eb0ee..f4e520b0a64 100644 --- a/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs +++ b/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs @@ -2077,3 +2077,318 @@ test("buildTranscript session/new bare systemPrompt field takes precedence over "_meta.systemPrompt.append must not appear when bare field is present", ); }); + +// ── authorization envelope + nonce-keyed cards ──────────────────────────────── + +/** Build an acp_read permission event with a full authorization envelope. */ +function makePermissionRequestWithAuth( + seq, + requestId, + nonce, + { actionable = true, reason, turnId = "turn-1", channelId = "ch-1" } = {}, +) { + return { + seq, + timestamp: "2026-07-01T10:00:00.000Z", + kind: "acp_read", + agentIndex: 0, + channelId, + sessionId: "session-1", + turnId, + payload: { + jsonrpc: "2.0", + id: requestId, + method: "session/request_permission", + params: { + title: "Confirm push", + toolCallId: "tool-1", + options: [ + { optionId: "allow_once", kind: "allow_once", name: "Allow" }, + { optionId: "reject_once", kind: "reject_once", name: "Reject" }, + ], + }, + }, + authorization: { requestNonce: nonce, actionable, reason }, + }; +} + +test("buildTranscript_nonce_keyed_card_is_actionable_with_options", () => { + // An acp_read with an authorization envelope should produce one card + // keyed by nonce, with actionable=true and the parsed options attached. + const transcript = buildTranscript([ + makePermissionRequestWithAuth(1, "req-n1", "nonce-abc"), + ]); + + assert.equal(transcript.length, 1); + const item = transcript[0]; + assert.equal(item.type, "lifecycle"); + assert.equal(item.renderClass, "permission"); + assert.equal(item.requestNonce, "nonce-abc"); + assert.equal(item.actionable, true); + assert.equal(item.channelId, "ch-1"); + assert.ok(Array.isArray(item.options)); + assert.equal(item.options.length, 2); + assert.equal(item.options[0].optionId, "allow_once"); + // Card is keyed by nonce, not by turn. + assert.ok( + item.id.includes("nonce-abc"), + `expected nonce in id, got ${item.id}`, + ); +}); + +test("buildTranscript_actionable_false_envelope_produces_read_only_card", () => { + const transcript = buildTranscript([ + makePermissionRequestWithAuth(1, "req-n2", "nonce-readonly", { + actionable: false, + reason: "auto-rejected: reject policy", + }), + ]); + + assert.equal(transcript.length, 1); + const item = transcript[0]; + assert.equal(item.actionable, false); + assert.equal(item.authorizationReason, "auto-rejected: reject policy"); +}); + +test("buildTranscript_concurrent_requests_same_turn_produce_separate_cards", () => { + // Two permission requests in the same turn with different nonces must each + // get their own card — nonce is the unique key. + const transcript = buildTranscript([ + makePermissionRequestWithAuth(1, "req-c1", "nonce-c1", { + turnId: "turn-1", + }), + makePermissionRequestWithAuth(2, "req-c2", "nonce-c2", { + turnId: "turn-1", + }), + ]); + + // Two distinct cards. + const cards = transcript.filter((i) => i.renderClass === "permission"); + assert.equal(cards.length, 2, "expected two separate permission cards"); + const nonces = cards.map((c) => c.requestNonce).sort(); + assert.deepEqual(nonces, ["nonce-c1", "nonce-c2"]); + // Each card id is unique. + assert.notEqual(cards[0].id, cards[1].id); +}); + +test("buildTranscript_without_auth_envelope_falls_back_to_turn_keyed_card", () => { + // A permission request without an authorization envelope (legacy / reject + // policy path) still produces a card using the turn-based key. + const transcript = buildTranscript([ + { + seq: 1, + timestamp: "2026-07-01T10:00:00.000Z", + kind: "acp_read", + agentIndex: 0, + channelId: "ch-1", + sessionId: "session-1", + turnId: "turn-legacy", + payload: { + jsonrpc: "2.0", + id: "req-leg", + method: "session/request_permission", + params: { + title: "Confirm push", + toolCallId: "tool-1", + options: [ + { optionId: "allow_once", kind: "allow_once", name: "Allow" }, + ], + }, + }, + // No authorization field. + }, + ]); + + assert.equal(transcript.length, 1); + const item = transcript[0]; + assert.equal(item.renderClass, "permission"); + assert.equal(item.requestNonce, undefined); + assert.equal(item.actionable, undefined); + // Fall-back key uses turn id. + assert.ok( + item.id.includes("turn-legacy"), + `expected turn id in fallback key, got ${item.id}`, + ); +}); + +test("buildTranscript_uncertain_outcome_uses_pinned_copy", () => { + // The 'uncertain' terminal state must use the verbatim pinned copy, never + // "denied" or "failed closed". + const transcript = buildTranscript([ + makePermissionRequest(1, "req-unc"), + makePermissionResponse(2, "req-unc", "uncertain"), + ]); + + assert.equal(transcript.length, 1); + const item = transcript[0]; + assert.equal(item.renderClass, "permission"); + assert.match( + item.outcome ?? "", + /Approval outcome unknown.*agent process stopped/i, + "uncertain must use the pinned copy", + ); + // Must not use 'denied' or 'failed closed'. + assert.doesNotMatch(item.outcome ?? "", /denied/i); + assert.doesNotMatch(item.outcome ?? "", /failed closed/i); +}); + +test("buildTranscript_timed_out_outcome_renders_correctly", () => { + const transcript = buildTranscript([ + makePermissionRequest(1, "req-to"), + makePermissionResponse(2, "req-to", "timed_out"), + ]); + + const item = transcript[0]; + assert.equal(item.renderClass, "permission"); + assert.ok(item.outcome, "timed_out should produce an outcome string"); + assert.doesNotMatch(item.outcome ?? "", /Approved/i); +}); + +test("buildTranscript_nonce_card_channelId_is_threaded_from_event", () => { + // The channelId on the card must come from the event, not a hard-coded value, + // so PermissionDecisionButtons can pass it to sendPermissionDecision. + const transcript = buildTranscript([ + makePermissionRequestWithAuth(1, "req-ch", "nonce-ch", { + channelId: "specific-channel-id", + }), + ]); + + const item = transcript[0]; + assert.equal(item.channelId, "specific-channel-id"); +}); + +test("buildTranscript_control_result_non_sent_marks_card_delivery_failed", () => { + // A `control_result` with non-`sent` status must set deliveryFailed on the + // matching card so PermissionDecisionButtons can re-enable buttons for retry. + const nonce = "nonce-delivery-fail"; + const events = [ + // First: the permission request that creates the card. + makePermissionRequestWithAuth(1, "req-df", nonce), + // Second: a control_result with non-sent status. + { + seq: 2, + timestamp: "2026-07-01T10:00:01.000Z", + kind: "control_result", + agentIndex: 0, + channelId: "ch-1", + sessionId: "session-1", + turnId: "turn-1", + payload: { + type: "permission_decision", + status: "no_active_turn", + requestNonce: nonce, + optionId: "allow_once", + }, + }, + ]; + const transcript = buildTranscript(events); + + const card = transcript.find( + (i) => i.renderClass === "permission" && i.requestNonce === nonce, + ); + assert.ok(card, "permission card must exist"); + assert.equal( + card.deliveryFailed, + 1, + "deliveryFailed must be 1 after first non-sent control_result", + ); + // Card must still be actionable so the user can retry. + assert.equal( + card.actionable, + true, + "card must remain actionable after delivery failure", + ); +}); + +test("buildTranscript_control_result_second_failure_increments_delivery_failed", () => { + // A second non-`sent` control_result must increment deliveryFailed so the + // useEffect([deliveryFailed]) dependency in PermissionDecisionButtons + // re-fires and re-enables the buttons for a second retry attempt. + const nonce = "nonce-delivery-fail-2"; + const events = [ + makePermissionRequestWithAuth(1, "req-df2", nonce), + // First failure. + { + seq: 2, + timestamp: "2026-07-01T10:00:01.000Z", + kind: "control_result", + agentIndex: 0, + channelId: "ch-1", + sessionId: "session-1", + turnId: "turn-1", + payload: { + type: "permission_decision", + status: "no_active_turn", + requestNonce: nonce, + optionId: "allow_once", + }, + }, + // Second failure (user retried; harness still unavailable). + { + seq: 3, + timestamp: "2026-07-01T10:00:02.000Z", + kind: "control_result", + agentIndex: 0, + channelId: "ch-1", + sessionId: "session-1", + turnId: "turn-1", + payload: { + type: "permission_decision", + status: "channel_closed", + requestNonce: nonce, + optionId: "allow_once", + }, + }, + ]; + const transcript = buildTranscript(events); + + const card = transcript.find( + (i) => i.renderClass === "permission" && i.requestNonce === nonce, + ); + assert.ok(card, "permission card must exist"); + assert.equal( + card.deliveryFailed, + 2, + "deliveryFailed must be 2 after two non-sent control_results — each failure must increment the token", + ); + assert.equal( + card.actionable, + true, + "card must remain actionable after second delivery failure", + ); +}); + +test("buildTranscript_control_result_sent_does_not_mark_delivery_failed", () => { + // A `control_result` with `sent` status must NOT set deliveryFailed — the + // click reached the harness successfully. + const nonce = "nonce-delivery-ok"; + const events = [ + makePermissionRequestWithAuth(1, "req-ok", nonce), + { + seq: 2, + timestamp: "2026-07-01T10:00:01.000Z", + kind: "control_result", + agentIndex: 0, + channelId: "ch-1", + sessionId: "session-1", + turnId: "turn-1", + payload: { + type: "permission_decision", + status: "sent", + requestNonce: nonce, + optionId: "allow_once", + }, + }, + ]; + const transcript = buildTranscript(events); + + const card = transcript.find( + (i) => i.renderClass === "permission" && i.requestNonce === nonce, + ); + assert.ok(card, "permission card must exist"); + assert.equal( + card.deliveryFailed, + undefined, + "deliveryFailed must not be set on sent control_result", + ); +}); diff --git a/desktop/src/features/agents/ui/agentSessionTranscript.ts b/desktop/src/features/agents/ui/agentSessionTranscript.ts index e371bf5fc30..979585658a7 100644 --- a/desktop/src/features/agents/ui/agentSessionTranscript.ts +++ b/desktop/src/features/agents/ui/agentSessionTranscript.ts @@ -28,6 +28,10 @@ import { parseSystemPromptSections, } from "./agentSessionTranscriptHelpers"; import { friendlyTurnErrorCopy } from "../lib/friendlyAgentLastError"; +import { + describePermissionOutcome, + describePermissionRequest, +} from "./agentPermissionTranscript"; export { describeRawEvent } from "./agentSessionTranscriptHelpers"; @@ -47,6 +51,13 @@ export type TranscriptState = { string, { itemId: string; optionNames: Map } >; + /** + * Maps `requestNonce` → `itemId` for actionable permission cards. + * Populated alongside `pendingPermissions` when the `authorization` envelope + * is present on the `acp_read` frame. Used by the `permission_decision` + * `control_result` handler to retire the card on any terminal outcome. + */ + pendingPermissionsByNonce: Map; continuationSeq: number; latestSessionId: string | null; }; @@ -59,6 +70,7 @@ export function createEmptyTranscriptState(): TranscriptState { sealedKeys: new Set(), triggeringEventIdsByTurn: new Map(), pendingPermissions: new Map(), + pendingPermissionsByNonce: new Map(), continuationSeq: 0, latestSessionId: null, }; @@ -79,6 +91,7 @@ type TranscriptDraft = { string, { itemId: string; optionNames: Map } >; + pendingPermissionsByNonce: Map; continuationSeq: number; latestSessionId: string | null; changed: boolean; @@ -92,6 +105,7 @@ function draftFrom(state: TranscriptState): TranscriptDraft { sealedKeys: state.sealedKeys, triggeringEventIdsByTurn: state.triggeringEventIdsByTurn, pendingPermissions: state.pendingPermissions, + pendingPermissionsByNonce: state.pendingPermissionsByNonce, continuationSeq: state.continuationSeq, latestSessionId: state.latestSessionId, changed: false, @@ -171,85 +185,6 @@ function stringifyPayload(value: unknown) { } } -function describePermissionRequest(payload: Record) { - const params = asRecord(payload.params); - const title = - asString(params.title) ?? - asString(params.message) ?? - asString(params.reason) ?? - "Permission requested"; - const toolCallId = - asString(params.toolCallId) ?? asString(params.tool_call_id); - const options = Array.isArray(params.options) - ? params.options - .map((option) => { - const record = asRecord(option); - return ( - asString(record.name) ?? - asString(record.kind) ?? - asString(record.optionId) - ); - }) - .filter((option): option is string => Boolean(option)) - : []; - const detail: string[] = []; - if (title !== "Permission requested") detail.push(title); - if (toolCallId) detail.push(`Tool call: ${toolCallId}`); - if (options.length > 0) detail.push(`Options: ${options.join(", ")}`); - - // Build optionId → kind map for outcome labeling on the response. - const optionNames = new Map(); - if (Array.isArray(params.options)) { - for (const option of params.options) { - const record = asRecord(option); - const optionId = asString(record.optionId); - const kind = asString(record.kind); - if (optionId && kind) { - optionNames.set(optionId, kind); - } - } - } - - return { - title, - text: detail.join("\n"), - optionNames, - descriptor: { - renderClass: "permission" as const, - label: "Permission requested", - preview: title, - action: { verb: "Requested", object: title }, - tone: "admin" as const, - operation: "session/request_permission", - object: title, - source: "acp" as const, - groupKey: "permission:request", - }, - }; -} - -/** - * Format a human-readable outcome label from a permission response. - * kind values from ACP: allow_once, allow_always, reject_once, reject_always. - * "reject_*" kinds are denials; anything else that is selected is an approval. - */ -function describePermissionOutcome( - outcome: string, - optionId: string | null, - optionNames: Map, -): string { - if (outcome === "cancelled") { - return "Cancelled"; - } - if (outcome === "selected" && optionId) { - const kind = optionNames.get(optionId) ?? optionId; - const isDenial = kind.startsWith("reject"); - const verb = isDenial ? "Denied" : "Approved"; - return `${verb} (${kind})`; - } - return outcome; -} - /** * Stable map key for a JSON-RPC id, which may be a string or a finite number * per the spec. Using JSON.stringify avoids collisions between the number 1 and @@ -792,7 +727,13 @@ export function processTranscriptEvent( if (method === "session/request_permission") { const request = describePermissionRequest(payload); - const itemId = `permission:${ch}:${event.turnId ?? event.seq}`; + // Key by nonce when the authorization envelope is present — this gives + // each concurrent ACP request its own card. Fall back to the turn-based + // key for legacy/non-ask paths where no nonce is emitted. + const auth = event.authorization; + const itemId = auth?.requestNonce + ? `permission:${ch}:nonce:${auth.requestNonce}` + : `permission:${ch}:${event.turnId ?? event.seq}`; upsertLifecycleItem( d, itemId, @@ -804,6 +745,25 @@ export function processTranscriptEvent( "permission_request", request.descriptor, ); + + // Attach authorization-envelope fields to the item. The `authorization` + // object is on the ObserverEvent itself (not the payload — payloads are + // raw ACP with no `_buzz` wrapper). + if (auth) { + const existing = d.itemsById.get(itemId); + if (existing?.type === "lifecycle") { + replaceItem(d, itemId, { + ...existing, + requestNonce: auth.requestNonce, + actionable: auth.actionable, + authorizationReason: auth.reason, + options: request.options, + }); + } + d.pendingPermissionsByNonce = new Map(d.pendingPermissionsByNonce); + d.pendingPermissionsByNonce.set(auth.requestNonce, itemId); + } + // Index by JSON-RPC id so the response (acp_write with result.outcome, // no method) can correlate by id rather than by turn/seq. const requestId = jsonRpcId(payload.id); @@ -1138,6 +1098,45 @@ export function processTranscriptEvent( ); } } + } else if (event.kind === "control_result") { + // `control_result` for `permission_decision` is a **delivery confirmation**, + // not a terminal outcome. Status values are: sent | no_active_turn | + // channel_full | channel_closed | no_channel. + // + // A non-"sent" status means the click did not reach the harness — mark the + // card with `deliveryFailed = true` so buttons re-enable for retry. Terminal + // outcomes (applied, denied, timed_out, cancelled, uncertain) arrive as + // enveloped acp_write frames correlated by requestNonce. + const payload = asRecord(event.payload); + const frameType = asString(payload.type); + if (frameType === "permission_decision") { + const deliveryStatus = asString(payload.status); + if (deliveryStatus !== "sent") { + // Delivery failed — find the card by nonce and mark it retryable. + const nonce = asString(payload.requestNonce); + if (nonce) { + const itemId = d.pendingPermissionsByNonce.get(nonce); + if (itemId) { + const existing = d.itemsById.get(itemId); + if ( + existing?.type === "lifecycle" && + existing.renderClass === "permission" && + existing.actionable + ) { + replaceItem(d, itemId, { + ...existing, + // Increment the failure token so the effect in + // PermissionDecisionButtons re-fires even when a prior + // failure already set deliveryFailed (a sticky boolean + // value would not change on the second failure and the + // useEffect dependency would not trigger). + deliveryFailed: (existing.deliveryFailed ?? 0) + 1, + }); + } + } + } + } + } } if (!d.changed && d.latestSessionId === state.latestSessionId) { @@ -1151,6 +1150,7 @@ export function processTranscriptEvent( sealedKeys: d.sealedKeys, triggeringEventIdsByTurn: d.triggeringEventIdsByTurn, pendingPermissions: d.pendingPermissions, + pendingPermissionsByNonce: d.pendingPermissionsByNonce, continuationSeq: d.continuationSeq, latestSessionId: d.latestSessionId, }; diff --git a/desktop/src/features/agents/ui/agentSessionTypes.ts b/desktop/src/features/agents/ui/agentSessionTypes.ts index 578f98076cd..39168e0cd41 100644 --- a/desktop/src/features/agents/ui/agentSessionTypes.ts +++ b/desktop/src/features/agents/ui/agentSessionTypes.ts @@ -10,6 +10,17 @@ export type ObserverEvent = { turnId: string | null; startedAt?: string | null; payload: unknown; + /** + * Present on `acp_read` permission frames (kind === "acp_read" + method === + * "session/request_permission"). Carries the harness-level permission gate + * metadata — `requestNonce`, `actionable`, and an optional human-readable + * `reason`. Payloads are raw ACP; there is no `_buzz` wrapper field. + */ + authorization?: { + requestNonce: string; + actionable: boolean; + reason?: string; + }; }; export type ConnectionState = @@ -112,6 +123,37 @@ export type TranscriptItem = timestamp: string; descriptor?: AgentActivityDescriptor; acpSource?: TranscriptAcpSource; + /** + * Nonce from the `authorization` envelope on an `acp_read` permission + * frame. Present only on `renderClass === "permission"` items; used to + * correlate the `permission_decision` control response and to match + * incoming `control_result` frames back to this card. + */ + requestNonce?: string; + /** + * When `true`, this card is waiting for a user Allow/Deny decision. + * `false` (or absent) means the card is read-only (auto-handled, or the + * policy is not `ask`). + */ + actionable?: boolean; + /** + * Human-readable reason string from the `authorization` envelope. + * Displayed as context below the request description. + */ + authorizationReason?: string; + /** + * Parsed options from the request params, passed back for Allow/Deny + * button rendering. + */ + options?: Array<{ optionId: string; kind: string; label?: string }>; + /** + * Monotonically increasing token incremented on every `control_result` + * with a non-`sent` delivery status. The `PermissionDecisionButtons` + * component keys its re-enable effect on this value, so a second failure + * after a retry (same boolean value would not re-trigger the effect) + * still re-enables the buttons. `undefined` when no failure has occurred. + */ + deliveryFailed?: number; } & TranscriptItemIdentity) | ({ id: string; diff --git a/desktop/src/features/agents/useGlobalAgentConfig.ts b/desktop/src/features/agents/useGlobalAgentConfig.ts index 4b90beb43d8..294427742ed 100644 --- a/desktop/src/features/agents/useGlobalAgentConfig.ts +++ b/desktop/src/features/agents/useGlobalAgentConfig.ts @@ -19,6 +19,7 @@ const EMPTY_CONFIG: GlobalAgentConfig = { provider: null, model: null, preferred_runtime: null, + permission_policy: null, }; export const globalAgentConfigQueryKey = ["globalAgentConfig"] as const; diff --git a/desktop/src/shared/api/agentConfigTypes.ts b/desktop/src/shared/api/agentConfigTypes.ts new file mode 100644 index 00000000000..f94a6aef635 --- /dev/null +++ b/desktop/src/shared/api/agentConfigTypes.ts @@ -0,0 +1,29 @@ +/** Permission policy controlling ACP `session/request_permission` calls. */ +export type PermissionPolicy = "ask" | "allow" | "reject"; + +/** Where an agent's effective permission policy came from. */ +export type PermissionPolicySource = "agent" | "global_default" | "built_in"; + +/** Global agent configuration defaults applied to all agents. */ +export type GlobalAgentConfig = { + /** Global env vars injected into all agents unconditionally. */ + env_vars: Record; + /** Global fallback provider. */ + provider: string | null; + /** Global fallback model identifier. */ + model: string | null; + /** Preferred ACP runtime for agents without a persona-specific runtime. */ + preferred_runtime: string | null; + /** Fleet-wide permission policy fallback. */ + permission_policy: PermissionPolicy | null; +}; + +/** Result returned by `set_global_agent_config`. */ +export type GlobalAgentConfigSaveResult = { + /** The persisted global config after strip-on-write. */ + config: GlobalAgentConfig; + /** Number of local agents successfully stopped and restarted. */ + restarted_count: number; + /** Number of agents whose stop succeeded but respawn failed. */ + failed_restart_count: number; +}; diff --git a/desktop/src/shared/api/agentControl.ts b/desktop/src/shared/api/agentControl.ts index 677f0ffad49..47db543d85f 100644 --- a/desktop/src/shared/api/agentControl.ts +++ b/desktop/src/shared/api/agentControl.ts @@ -29,3 +29,29 @@ export async function switchManagedAgentModel( modelId, }); } + +/** + * Send a permission decision to a running agent's ACP harness. The decision + * is fire-and-forget: the harness receives it via the observer control channel + * and updates the permission card asynchronously via a `control_result` frame. + * + * @param pubkey - Agent's public key (hex or npub). + * @param channelId - The channel from which the permission request was issued. + * The harness validates this before looking up the nonce. + * @param nonce - `requestNonce` from the `authorization` envelope on the + * corresponding `acp_read` permission frame. + * @param optionId - The chosen option's `optionId` (e.g. `"allow_once"`). + */ +export async function sendPermissionDecision( + pubkey: string, + channelId: string, + nonce: string, + optionId: string, +): Promise { + await sendAgentObserverControl(pubkey, { + type: "permission_decision", + channelId, + requestNonce: nonce, + optionId, + }); +} diff --git a/desktop/src/shared/api/managedAgentWire.ts b/desktop/src/shared/api/managedAgentWire.ts new file mode 100644 index 00000000000..2d53235297b --- /dev/null +++ b/desktop/src/shared/api/managedAgentWire.ts @@ -0,0 +1,101 @@ +import type { + ManagedAgent, + ManagedAgentBackend, + PermissionPolicy, + PermissionPolicySource, +} from "./types"; +import type { RestartDiffEntry as RawRestartDiffEntry } from "./restartDiff"; + +export type RawManagedAgent = { + pubkey: string; + name: string; + persona_id: string | null; + runtime?: string | null; + team_id?: string | null; + relay_url: string; + acp_command: string; + agent_command: string; + agent_command_override?: string | null; + agent_args: string[]; + mcp_command: string; + turn_timeout_seconds: number; + idle_timeout_seconds: number | null; + max_turn_duration_seconds: number | null; + parallelism: number; + system_prompt: string | null; + avatar_url?: string | null; + model: string | null; + model_source?: ManagedAgent["modelSource"]; + provider: string | null; + persona_out_of_date: boolean; + persona_orphaned: boolean; + needs_restart: boolean; + restart_diff?: RawRestartDiffEntry[]; + env_vars?: Record; + status: ManagedAgent["status"]; + pid: number | null; + created_at: string; + updated_at: string; + last_started_at: string | null; + last_stopped_at: string | null; + last_exit_code: number | null; + last_error: string | null; + last_error_code: number | null; + log_path: string; + start_on_app_launch: boolean; + auto_restart_on_config_change?: boolean; + backend: ManagedAgentBackend; + backend_agent_id: string | null; + respond_to?: ManagedAgent["respondTo"]; + respond_to_allowlist?: string[]; + permission_policy?: PermissionPolicy; + permission_policy_source?: PermissionPolicySource; +}; + +export function fromRawManagedAgent(agent: RawManagedAgent): ManagedAgent { + return { + pubkey: agent.pubkey, + name: agent.name, + personaId: agent.persona_id, + runtime: agent.runtime ?? null, + teamId: agent.team_id ?? null, + relayUrl: agent.relay_url, + acpCommand: agent.acp_command, + agentCommand: agent.agent_command, + agentCommandOverride: agent.agent_command_override ?? null, + agentArgs: agent.agent_args, + mcpCommand: agent.mcp_command, + turnTimeoutSeconds: agent.turn_timeout_seconds, + idleTimeoutSeconds: agent.idle_timeout_seconds, + maxTurnDurationSeconds: agent.max_turn_duration_seconds, + parallelism: agent.parallelism, + systemPrompt: agent.system_prompt, + avatarUrl: agent.avatar_url ?? null, + model: agent.model, + modelSource: agent.model_source ?? null, + provider: agent.provider ?? null, + personaOutOfDate: agent.persona_out_of_date ?? false, + personaOrphaned: agent.persona_orphaned ?? false, + needsRestart: agent.needs_restart ?? false, + restartDiff: agent.restart_diff ?? [], + envVars: agent.env_vars ?? {}, + status: agent.status, + pid: agent.pid, + createdAt: agent.created_at, + updatedAt: agent.updated_at, + lastStartedAt: agent.last_started_at, + lastStoppedAt: agent.last_stopped_at, + lastExitCode: agent.last_exit_code, + lastError: agent.last_error, + lastErrorCode: agent.last_error_code ?? null, + logPath: agent.log_path, + startOnAppLaunch: agent.start_on_app_launch, + autoRestartOnConfigChange: agent.auto_restart_on_config_change ?? true, + backend: agent.backend, + backendAgentId: agent.backend_agent_id, + respondTo: agent.respond_to ?? "owner-only", + respondToAllowlist: agent.respond_to_allowlist ?? [], + permissionPolicy: agent.permission_policy ?? "ask", + permissionPolicySource: agent.permission_policy_source ?? "built_in", + }; +} diff --git a/desktop/src/shared/api/tauri.ts b/desktop/src/shared/api/tauri.ts index 67ba582a1d4..ceee28fe445 100644 --- a/desktop/src/shared/api/tauri.ts +++ b/desktop/src/shared/api/tauri.ts @@ -16,7 +16,6 @@ import type { GetHomeFeedInput, HomeFeedResponse, ManagedAgent, - ManagedAgentBackend, RelayAgent, RelayMember, RelayMemberRole, @@ -41,6 +40,9 @@ import type { GitBashPrerequisite, RuntimeConfigSurface, } from "@/shared/api/types"; +import { fromRawManagedAgent, type RawManagedAgent } from "./managedAgentWire"; + +export { fromRawManagedAgent, type RawManagedAgent }; export * from "@/shared/api/tauriChannels"; @@ -117,53 +119,6 @@ type RawRelayAgent = { respond_to_allowlist?: string[]; }; -import type { RestartDiffEntry as RawRestartDiffEntry } from "./restartDiff"; -export type RawManagedAgent = { - pubkey: string; - name: string; - persona_id: string | null; - // Optional: pre-feature fixtures may omit it. The record's harness/runtime id. - runtime?: string | null; - team_id?: string | null; - relay_url: string; - acp_command: string; - agent_command: string; - agent_command_override?: string | null; - agent_args: string[]; - mcp_command: string; - turn_timeout_seconds: number; - idle_timeout_seconds: number | null; - max_turn_duration_seconds: number | null; - parallelism: number; - system_prompt: string | null; - avatar_url?: string | null; - model: string | null; - model_source?: ManagedAgent["modelSource"]; - provider: string | null; - persona_out_of_date: boolean; - persona_orphaned: boolean; - needs_restart: boolean; - restart_diff?: RawRestartDiffEntry[]; - env_vars?: Record; - status: ManagedAgent["status"]; - pid: number | null; - created_at: string; - updated_at: string; - last_started_at: string | null; - last_stopped_at: string | null; - last_exit_code: number | null; - last_error: string | null; - last_error_code: number | null; - log_path: string; - start_on_app_launch: boolean; - auto_restart_on_config_change?: boolean; - backend: ManagedAgentBackend; - backend_agent_id: string | null; - // Pre-feature fixtures may omit these; mapped to "owner-only"/[] in fromRawManagedAgent. - respond_to?: ManagedAgent["respondTo"]; - respond_to_allowlist?: string[]; -}; - type RawCreateManagedAgentResponse = { agent: RawManagedAgent; private_key_nsec: string; @@ -687,52 +642,6 @@ function fromRawRelayAgent(agent: RawRelayAgent): RelayAgent { }; } -export function fromRawManagedAgent(agent: RawManagedAgent): ManagedAgent { - return { - pubkey: agent.pubkey, - name: agent.name, - personaId: agent.persona_id, - runtime: agent.runtime ?? null, - teamId: agent.team_id ?? null, - relayUrl: agent.relay_url, - acpCommand: agent.acp_command, - agentCommand: agent.agent_command, - agentCommandOverride: agent.agent_command_override ?? null, - agentArgs: agent.agent_args, - mcpCommand: agent.mcp_command, - turnTimeoutSeconds: agent.turn_timeout_seconds, - idleTimeoutSeconds: agent.idle_timeout_seconds, - maxTurnDurationSeconds: agent.max_turn_duration_seconds, - parallelism: agent.parallelism, - systemPrompt: agent.system_prompt, - avatarUrl: agent.avatar_url ?? null, - model: agent.model, - modelSource: agent.model_source ?? null, - provider: agent.provider ?? null, - personaOutOfDate: agent.persona_out_of_date ?? false, - personaOrphaned: agent.persona_orphaned ?? false, - needsRestart: agent.needs_restart ?? false, - restartDiff: agent.restart_diff ?? [], - envVars: agent.env_vars ?? {}, - status: agent.status, - pid: agent.pid, - createdAt: agent.created_at, - updatedAt: agent.updated_at, - lastStartedAt: agent.last_started_at, - lastStoppedAt: agent.last_stopped_at, - lastExitCode: agent.last_exit_code, - lastError: agent.last_error, - lastErrorCode: agent.last_error_code ?? null, - logPath: agent.log_path, - startOnAppLaunch: agent.start_on_app_launch, - autoRestartOnConfigChange: agent.auto_restart_on_config_change ?? true, - backend: agent.backend, - backendAgentId: agent.backend_agent_id, - respondTo: agent.respond_to ?? "owner-only", - respondToAllowlist: agent.respond_to_allowlist ?? [], - }; -} - export function fromRawAcpRuntimeCatalogEntry( entry: RawAcpRuntimeCatalogEntry, ): AcpRuntimeCatalogEntry { diff --git a/desktop/src/shared/api/types.ts b/desktop/src/shared/api/types.ts index 24ef6257832..658d60a8cfd 100644 --- a/desktop/src/shared/api/types.ts +++ b/desktop/src/shared/api/types.ts @@ -1,3 +1,15 @@ +import type { + PermissionPolicy, + PermissionPolicySource, +} from "./agentConfigTypes"; + +export type { + GlobalAgentConfig, + GlobalAgentConfigSaveResult, + PermissionPolicy, + PermissionPolicySource, +} from "./agentConfigTypes"; + export type ChannelType = "stream" | "forum" | "dm"; export type ChannelVisibility = "open" | "private"; export type ChannelRole = "owner" | "admin" | "member" | "guest" | "bot"; @@ -384,6 +396,16 @@ export type ManagedAgent = { * `"allowlist"`. Preserved across mode toggles. */ respondToAllowlist: string[]; + /** + * Effective permission policy at the last spawn. Determines how the ACP + * harness answers `session/request_permission` calls. + */ + permissionPolicy: PermissionPolicy; + /** + * Where the effective `permissionPolicy` value came from: a per-agent + * override, the fleet-wide global default, or the built-in desktop default. + */ + permissionPolicySource: PermissionPolicySource; }; /** Inbound author gate mode. Mirrors buzz-acp's --respond-to CLI flag. */ @@ -443,6 +465,8 @@ export type CreateManagedAgentInput = { */ respondToAllowlist?: string[]; relayMesh?: RelayMeshConfig; + /** Per-agent permission policy override. Omitted = inherit from global or built-in default. */ + permissionPolicy?: PermissionPolicy; }; export type CreateManagedAgentResponse = { @@ -475,9 +499,11 @@ export type SwitchManagedAgentModelStatus = | "no_active_turn"; export type ControlResultFrame = { - type: "cancel_turn" | "switch_model"; + type: "cancel_turn" | "switch_model" | "permission_decision"; status: string; modelId?: string; + /** Present on `permission_decision` results — identifies the request card to retire. */ + requestNonce?: string; }; export type GitBashPrerequisite = { @@ -706,6 +732,11 @@ export type UpdateManagedAgentInput = { * (validated & normalized server-side). */ respondToAllowlist?: string[]; + /** + * Absent = don't touch. Present = override (or `null` to clear back to inherit). + * Remote deployed agents: read-only; edit the deploy config and redeploy. + */ + permissionPolicy?: PermissionPolicy | null; }; export type AgentPersona = { id: string; @@ -992,38 +1023,3 @@ export type ChannelMessagesPageResponse = { /** Present only when a full page was returned — pass back to fetch the next (older) page. */ nextCursor: ChannelPageCursor | null; }; - -// ── Global agent configuration ──────────────────────────────────────────────── - -/** - * Global agent configuration defaults applied to ALL agents. - * - * Lowest user-settable layer — per-agent and persona values win on any key - * collision. Mirrors the Rust `GlobalAgentConfig` struct. - * - * Precedence: baked floor < global < persona < per-agent. - */ -export type GlobalAgentConfig = { - /** Global env vars injected into all agents unconditionally. */ - env_vars: Record; - /** Global fallback provider (e.g. "anthropic", "databricks_v2"). Null = no global default. */ - provider: string | null; - /** Global fallback model identifier. Null = no global default. */ - model: string | null; - /** Preferred ACP runtime for agents without a persona-specific runtime. */ - preferred_runtime: string | null; -}; - -/** - * Result returned by `set_global_agent_config`. - * - * Mirrors the Rust `GlobalAgentConfigSaveResult` struct. - */ -export type GlobalAgentConfigSaveResult = { - /** The persisted global config (after strip-on-write). */ - config: GlobalAgentConfig; - /** Number of local agents successfully stopped and restarted. */ - restarted_count: number; - /** Number of agents whose stop succeeded but respawn failed. */ - failed_restart_count: number; -}; diff --git a/desktop/tests/e2e/observer-feed-screenshots.spec.ts b/desktop/tests/e2e/observer-feed-screenshots.spec.ts index 44ff609c9ee..5e399f473d7 100644 --- a/desktop/tests/e2e/observer-feed-screenshots.spec.ts +++ b/desktop/tests/e2e/observer-feed-screenshots.spec.ts @@ -75,6 +75,11 @@ async function seedObserverEvents( sessionId: string | null; turnId: string | null; payload: unknown; + authorization?: { + requestNonce: string; + actionable: boolean; + reason?: string; + }; }>, ) { await page.evaluate( @@ -285,6 +290,72 @@ test.describe("observer feed screenshots", () => { }); }); + test("permission request stays actionable for a human decision", async ({ + page, + }) => { + await installMockBridge(page, { managedAgents: MANAGED_AGENTS }); + const feedPanel = await openObserverFeedPanel(page, OBSERVER_AGENT_PUBKEY); + + await seedObserverEvents(page, OBSERVER_AGENT_PUBKEY, [ + { + seq: 1, + timestamp: NOW, + kind: "acp_read", + agentIndex: 0, + channelId: CHANNEL_ID, + sessionId: "session-001", + turnId: "turn-001", + payload: { + jsonrpc: "2.0", + id: "permission-ask-1", + method: "session/request_permission", + params: { + title: "Write publishing plan", + options: [ + { optionId: "allow_once", kind: "allow_once", name: "Allow" }, + { optionId: "reject_once", kind: "reject_once", name: "Deny" }, + ], + }, + }, + authorization: { + requestNonce: "permission-nonce-1", + actionable: true, + reason: "Waiting for a human decision", + }, + }, + ]); + + await expect( + feedPanel.getByText("Waiting for a human decision"), + ).toBeVisible(); + const allow = feedPanel.getByTestId("permission-decision-allow_once"); + await expect(allow).toHaveText("Allow"); + await expect( + feedPanel.getByTestId("permission-decision-reject_once"), + ).toHaveText("Deny"); + + await allow.click(); + await page.waitForFunction(() => + window.__BUZZ_E2E_COMMAND_LOG__?.some( + (entry) => entry.command === "build_observer_control_event", + ), + ); + const command = await page.evaluate(() => + window.__BUZZ_E2E_COMMAND_LOG__?.find( + (entry) => entry.command === "build_observer_control_event", + ), + ); + expect(command?.payload).toEqual({ + agentPubkey: OBSERVER_AGENT_PUBKEY, + payload: { + type: "permission_decision", + channelId: CHANNEL_ID, + requestNonce: "permission-nonce-1", + optionId: "allow_once", + }, + }); + }); + test("04 — permission outcome (cancelled)", async ({ page }) => { await installMockBridge(page, { managedAgents: MANAGED_AGENTS }); const feedPanel = await openObserverFeedPanel(page, OBSERVER_AGENT_PUBKEY); diff --git a/docs/nips/NIP-AO.md b/docs/nips/NIP-AO.md index 36adea04871..1b29ab59e04 100644 --- a/docs/nips/NIP-AO.md +++ b/docs/nips/NIP-AO.md @@ -24,6 +24,11 @@ It is strictly scoped to the agent↔owner relationship and carries no durable s - **Owner**: The human (or system) whose pubkey the agent was provisioned under. - **Observer Frame**: A single kind 24200 event carrying one unit of telemetry or control. - **Session**: A bounded agent execution correlated by a shared `sessionId`. +- **Request nonce**: A single-use random token bound to one `session/request_permission` + call. The harness generates it on arrival of the request, embeds it in the + `authorization` envelope of the emitted `acp_read` telemetry frame, and consumes it + exactly once when a matching `permission_decision` control frame is received. A nonce + that is never matched expires with the per-request fail-closed timeout. ## Event Kinds @@ -58,8 +63,8 @@ Events MUST have exactly one `p` tag, exactly one `agent` tag, and exactly one `frame` MUST be `"telemetry"` or `"control"`. Relays SHOULD silently drop events with unrecognized `frame` values (returning OK to the publisher for forward -compatibility). Clients MUST ignore events with unrecognized `frame` values. An `h` tag MAY be included when the session runs within a NIP-29 group -context. +compatibility). Clients MUST ignore events with unrecognized `frame` values. An `h` +tag MAY be included when the session runs within a NIP-29 group context. ## Encryption @@ -80,14 +85,15 @@ The `content` field decrypts to an `ObserverEvent` JSON object: ```json { - "seq": , - "timestamp": "", - "kind": "", - "agentIndex": | null, - "channelId": "" | null, - "sessionId": "" | null, - "turnId": "" | null, - "payload": { ... } + "seq": , + "timestamp": "", + "kind": "", + "agentIndex": | null, + "channelId": "" | null, + "sessionId": "" | null, + "turnId": "" | null, + "authorization": { ... } | omitted, + "payload": { ... } } ``` @@ -99,21 +105,84 @@ gracefully. `seq` is monotonically increasing per session (drop detection). `timestamp` is an RFC 3339 datetime string with sub-second precision (e.g., `"2026-04-29T12:00:41.500Z"`). `agentIndex` identifies the agent in multi-agent scenarios. `sessionId`/`turnId` -correlate frames across a session and turn. `payload` is kind-specific (MAY be `{}`). -Unknown `kind` values MUST be ignored. +correlate frames across a session and turn. `payload` carries the raw ACP JSON frame +byte-for-byte — it is NEVER mutated by the harness. Unknown `kind` values MUST be +ignored. + +`authorization` is present only on `acp_read` and `acp_write` frames that correspond +to `session/request_permission` calls (see [Authorization Envelope](#authorization-envelope) +below). It is omitted on all other frame kinds. ### Frame Kinds -| `kind` | Description | -|--------------------|----------------------------------------------------------| -| `acp_read` | Inbound ACP protocol frame (model → harness) | -| `acp_write` | Outbound ACP protocol frame (harness → model) | -| `turn_started` | A new agent turn has begun | -| `session_resolved` | Session completed or terminated | +| `kind` | Description | +|--------------------|--------------------------------------------------------------------| +| `acp_read` | Inbound ACP protocol frame (model → harness) | +| `acp_write` | Outbound ACP protocol frame (harness → model) | +| `turn_started` | A new agent turn has begun | +| `session_resolved` | Session completed or terminated | +| `control_result` | Acknowledgement telemetry emitted after processing a control frame | + +Permission `acp_read` frames (carrying `session/request_permission` calls) always +include an `authorization` envelope. The corresponding `acp_write` (the harness +response) also includes an `authorization` envelope correlated by the same nonce — +this pairs the challenge and answer in the observer log. + +**One-write / one-observe contract.** Each pending permission entry produces at most +one ACP wire write and at most one authorized `acp_write` observer event. The write +and the observer event are always emitted together; if the write fails the observer +event is suppressed. The sole exception is the `uncertain` terminal (see below) in +which neither is emitted. + +### Authorization Envelope + +When an `acp_read` or `acp_write` frame relates to a `session/request_permission` +call, the `ObserverEvent` carries an `authorization` field: + +```json +{ + "requestNonce": "", + "actionable": true | false, + "reason": "" | omitted +} +``` + +- `requestNonce`: a single-use random token generated by the harness for this request. + It is embedded in the `acp_read` emit and MUST be echoed verbatim in the + `permission_decision` control frame sent by the desktop. The harness consumes the + nonce exactly once — a second `permission_decision` carrying the same nonce is + silently ignored. If no matching decision arrives before the per-request timeout, + the harness fails the request closed. +- `actionable`: `true` when the owner can act (policy=`ask`, preflight passed, owner + and observer available). `false` for auto-deny, fail-closed, and terminal outcomes. +- `reason`: present on every `acp_write` authorization envelope. Identifies the + terminal outcome for this request. Defined values: + + | Value | Meaning | + |-------|---------| + | `"applied"` | Owner decision was received and written to the agent pipe. | + | `"timed_out"` | No decision arrived before the 300-second per-request deadline; request failed closed (denial). | + | `"cancelled"` | The turn was cancelled while the request was pending; request failed closed (denial). | + + The `uncertain` terminal (cancel arriving while the write is in flight) does NOT + produce an `acp_write` observer event — the process is irrecoverably poisoned and + will be respawned by the pool. Desktop clients MUST NOT expect an `acp_write` for + every `acp_read` they receive; a missing `acp_write` after a `session_resolved` + frame with a poisoned outcome indicates the `uncertain` path. + +**Nonce binding.** The nonce is bound to the agent, channel, session, turn, request +ID, and exact option snapshot at generation time. It MUST NOT be reused across +requests, turns, or sessions. The harness rejects a `permission_decision` whose nonce +does not match any live pending entry. ### Control (`frame=control`) -The `content` field decrypts to: +The `content` field decrypts to a JSON object with a required `type` field. +Implementations MUST ignore events with unrecognized `type` values. + +#### `cancel_turn` + +Cancel the in-flight agent turn for the given channel. ```json { @@ -122,8 +191,84 @@ The `content` field decrypts to: } ``` -The only defined control type is `cancel_turn`. Implementations MUST ignore -events with unrecognized `type` values. +#### `switch_model` + +Switch the active model for the agent session in the given channel. + +- **Busy turn:** delivers `ControlSignal::SwitchModel` over the per-turn oneshot, + which triggers the harness to cancel the current turn and requeue with the new model. + If the oneshot is already consumed (a prior cancel/interrupt is in flight), the + switch cannot land and the current turn is left to complete with the old model. +- **Idle session:** validates the model against the cached catalog and, if valid, + invalidates and reapplies the agent's model config immediately. + +```json +{ + "type": "switch_model", + "channelId": "", + "modelId": "" +} +``` + +#### `permission_decision` + +Deliver the owner's decision for a pending `session/request_permission` call. +The harness matches `requestNonce` to a live pending entry and, if found, transitions +the entry from `pending` to `writing` and writes the ACP response. + +```json +{ + "type": "permission_decision", + "channelId": "", + "requestNonce": "", + "optionId": "" +} +``` + +The harness MUST: +1. Verify `requestNonce` matches a live pending entry (else ignore silently). +2. Verify `optionId` is present in the exact option snapshot recorded at nonce + generation time (else ignore silently — prevents replay with an altered option). +3. Transition the entry to `writing` atomically before performing the ACP write. +4. Emit an `acp_write` telemetry frame with a matching `authorization` envelope only + after the write is confirmed. + +**Best-effort delivery.** `permission_decision` frames ride the ordinary observer +control path — they are NOT guaranteed to arrive before the per-request timeout. +If no matching `permission_decision` is received within `min(300s, remaining hard +deadline)`, the harness fails the request closed (deny). The owner SHOULD respond +before this deadline; the desktop MAY surface the deadline to the owner in the +permission card UI. + +### `control_result` Telemetry + +After processing any control frame, the harness emits a `control_result` telemetry +event to confirm receipt. This is an `acp_read`-style telemetry frame (kind = +`control_result`) that carries a `payload` describing the outcome: + +**`cancel_turn`:** +```json +{ "type": "cancel_turn", "status": "sent" | "no_active_turn" } +``` + +**`switch_model`:** +```json +{ "type": "switch_model", "status": "sent" | "turn_ending" | "switched" | "unsupported_model" | "no_active_turn", "modelId": "..." } +``` + +**`permission_decision`:** +```json +{ + "type": "permission_decision", + "status": "sent" | "no_active_turn" | "channel_full" | "channel_closed" | "no_channel", + "requestNonce": "", + "optionId": "" +} +``` + +`status: "sent"` means the decision was delivered to the in-flight read loop. +Other statuses indicate delivery failure; the per-request timeout will fail the +entry closed. ## Ephemerality Contract @@ -132,7 +277,9 @@ events with unrecognized `type` values. - Relays MUST NOT include kind 24200 events in audit logs. - Relays SHOULD fan out kind 24200 events only via in-memory pub/sub, never via a database write path. -- Clients SHOULD subscribe with `since=`; historical replay is not supported. +- Clients SHOULD subscribe with `since=` to recover frames from the past + five minutes (e.g., after a brief reconnect); historical replay beyond this window + is not supported. - Clients SHOULD buffer received events in a bounded in-memory ring buffer. ## Authorization @@ -152,6 +299,9 @@ Both directions require relay confirmation of the agent-owner relationship via database lookup. `#p` tag matching alone is insufficient. Unauthorized publish or subscribe attempts MUST be rejected with `AUTH required`. +The harness additionally enforces a ±5-minute `created_at` freshness window on +incoming control frames as defense-in-depth against relay-captured replay. + ## Relay Behavior On receiving a kind 24200 event, a relay MUST: @@ -170,9 +320,12 @@ freshness window to prevent replay of captured events. Clients subscribe with: ```json -{"kinds": [24200], "#p": [""], "since": } +{"kinds": [24200], "#p": [""], "since": } ``` +The `since` lookback of 300 seconds (5 minutes) allows recovery of recent frames +after brief reconnects without enabling unbounded historical replay. + On receiving an event, a client MUST: 1. Verify the event signature. @@ -184,8 +337,8 @@ Clients SHOULD verify that the `agent` tag matches a known/trusted agent pubkey before decrypting. Clients SHOULD buffer events in a bounded ring buffer (RECOMMENDED maximum: 800 events). -Clients MUST NOT request historical kind 24200 events (no `since` in the past, no -`until`, no `ids` queries). +Clients MUST NOT request historical kind 24200 events beyond the 5-minute lookback +window (no `since` further in the past, no `until`, no `ids` queries). ## Security Considerations @@ -197,19 +350,34 @@ rate. For maximum metadata privacy, implementors MAY wrap events in NIP-59 gift agent's private key allows decryption of any captured ciphertext. **Replay attacks.** A captured, signed event could be replayed without a freshness -check. Relays are RECOMMENDED to enforce a `created_at` freshness window. +check. Relays are RECOMMENDED to enforce a `created_at` freshness window. The harness +enforces this as defense-in-depth on incoming control frames. **Rogue relays.** The ephemerality contract is relay policy, not cryptography. NIP-44 encryption ensures stored events remain opaque to the relay operator absent key compromise. **Best-effort delivery.** Control frames can be dropped during reconnect or queue -overflow. Control commands SHOULD be treated as advisory with idempotent semantics. -Agents MUST NOT rely on guaranteed delivery of control frames. +overflow. `permission_decision` frames follow the same best-effort path; the +mandatory per-request fail-closed timeout (max 300 seconds) ensures the harness never +blocks indefinitely waiting for a decision that never arrives. + +**Permission nonce security.** Request nonces are single-use and generated fresh per +request. A `permission_decision` carrying a nonce that does not match an active +pending entry is silently ignored. The harness verifies that the chosen `optionId` is +present in the exact option snapshot captured at nonce generation — preventing a +replayed or modified decision from selecting an option not offered in the original +request. + +**Cancel during write (poison).** If a cancel arrives while the harness is writing +an ACP permission response mid-flight, the process state is irrecoverably uncertain. +The harness surfaces a dedicated `PermissionPoisoned` error through `cancel_with_cleanup_grace`, +which causes the pool to respawn the agent process rather than return it. All other +pending permission entries for that session are drained with `cancelled` responses. **Operational persistence vectors.** Telemetry may transiently exist in process memory, crash dumps, and application logs. Implementations SHOULD minimize logging -of decrypted payloads and MUST NOT log it at INFO level or above. +of decrypted payloads and MUST NOT log them at INFO level or above. ## Relationship to Other NIPs @@ -295,6 +463,82 @@ of decrypted payloads and MUST NOT log it at INFO level or above. } ``` +--- + +### 3. Permission request (ask policy) — challenge + decision round trip + +**Step 1 — agent emits `session/request_permission`; harness emits `acp_read` telemetry:** + +```json +{ + "seq": 101, + "timestamp": "2026-08-01T10:00:00.000Z", + "kind": "acp_read", + "agentIndex": 0, + "channelId": "52a85618-0f8f-4542-94ec-599e6e1c6f2e", + "sessionId": "sess-abc", + "turnId": "turn-xyz", + "authorization": { + "requestNonce": "a9f3b2c1d4e5...", + "actionable": true + }, + "payload": { + "jsonrpc": "2.0", + "id": "req-17", + "method": "session/request_permission", + "params": { + "sessionId": "sess-abc", + "options": [ + { "optionId": "opt-allow", "kind": "allow_once", "name": "Allow once" }, + { "optionId": "opt-deny", "kind": "reject_once", "name": "Deny" } + ] + } + } +} +``` + +**Step 2 — desktop sends `permission_decision` control frame:** + +```json +{ + "type": "permission_decision", + "channelId": "52a85618-0f8f-4542-94ec-599e6e1c6f2e", + "requestNonce": "a9f3b2c1d4e5...", + "optionId": "opt-allow" +} +``` + +**Step 3 — harness writes ACP response and emits `acp_write` telemetry:** + +```json +{ + "seq": 102, + "timestamp": "2026-08-01T10:00:04.120Z", + "kind": "acp_write", + "agentIndex": 0, + "channelId": "52a85618-0f8f-4542-94ec-599e6e1c6f2e", + "sessionId": "sess-abc", + "turnId": "turn-xyz", + "authorization": { + "requestNonce": "a9f3b2c1d4e5...", + "actionable": false, + "reason": "applied" + }, + "payload": { + "jsonrpc": "2.0", + "id": "req-17", + "result": { "outcome": { "outcome": "selected", "optionId": "opt-allow" } } + } +} +``` + +Note: `actionable` is `false` on the `acp_write` telemetry frame — the decision has +been applied and the card is no longer actionable. `reason: "applied"` is the +standard terminal annotation for a successfully delivered decision. When the request +expires without a decision, the harness emits `reason: "timed_out"`. When the turn +is cancelled while the request is pending, the harness emits `reason: "cancelled"`. +If the cancel arrives mid-write (`uncertain`), no `acp_write` frame is emitted at all. + ## Reference Implementation -[block/sprout PR #421](https://github.com/block/sprout/pull/421) +[block/buzz PR #4938](https://github.com/block/buzz/pull/4938)