diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 93109fa94d..db64879435 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -2105,6 +2105,32 @@ pub fn extract_model_config_options(result: &serde_json::Value) -> Vec Vec { + result["configOptions"] + .as_array() + .map(|arr| { + arr.iter() + .filter(|opt| { + matches!( + opt.get("category").and_then(|c| c.as_str()), + Some("model") | Some("thought_level") + ) + }) + .cloned() + .collect() + }) + .unwrap_or_default() +} + /// Extract `SessionModelState` (unstable path) from a `session/new` result. /// /// Returns the `models` object if present: `{ currentModelId, availableModels: [...] }`. @@ -2112,6 +2138,27 @@ pub fn extract_model_state(result: &serde_json::Value) -> Option Option { + let arr = result["configOptions"].as_array()?; + for opt in arr { + if opt.get("category").and_then(|c| c.as_str()) == Some("thought_level") { + let config_id = opt + .get("configId") + .or_else(|| opt.get("id")) + .and_then(|v| v.as_str())?; + return Some(config_id.to_string()); + } + } + None +} + /// Match a desired model ID against a fresh `session/new` response. /// /// Returns the correct ACP method to call, or `None` if no match. diff --git a/crates/buzz-acp/src/config.rs b/crates/buzz-acp/src/config.rs index d959685846..26d13c5f12 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -123,6 +123,11 @@ pub enum PermissionMode { /// Agent default — permission requests per tool call. #[value(alias = "default")] Default, + /// Auto mode — fully autonomous execution; model-gated (requires a model + /// that supports `supportsAutoMode`). Degrades gracefully to `default` + /// when the session's active model does not support it. + #[value(alias = "auto")] + Auto, /// Auto-approve file edits, still ask for other tools. #[value(alias = "acceptEdits")] AcceptEdits, @@ -140,6 +145,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", @@ -418,6 +424,14 @@ pub struct CliArgs { #[arg(long, env = "BUZZ_ACP_MODEL")] pub model: Option, + /// Persisted effort level value (e.g. "high", "medium", "low") to apply via + /// `session/set_config_option` at the first session creation. The configId is + /// resolved from the adapter's advertised `thought_level` capability — not + /// hardcoded. Non-fatal: if the adapter does not advertise `thought_level`, + /// the value is silently ignored and the persisted effort is not overwritten. + #[arg(long, env = "BUZZ_ACP_EFFORT_LEVEL")] + pub effort_level: Option, + /// Title for the agent's ACP sessions, passed out-of-band in `session/new` /// `_meta`. Adapters that recognize it name the session after this value; /// others ignore it. Never enters the prompt. @@ -527,6 +541,11 @@ pub struct Config { pub memory_enabled: bool, /// Desired LLM model ID. Applied after every `session_new_full()`. pub model: Option, + /// Persisted effort level value (e.g. "high", "medium", "low"). Resolved into a + /// real `desired_effort` at the first session creation by pairing with the + /// adapter's advertised `thought_level` configId. Non-fatal when absent or + /// when the adapter does not advertise `thought_level`. + pub effort_level: Option, /// Sanitized session title, sent as `_meta.sessionTitle` on `session/new`. /// `None` when unset or when the configured value sanitized to empty. pub session_title: Option, @@ -1088,6 +1107,7 @@ impl Config { typing_enabled: !args.no_typing, memory_enabled: args.memory && !args.no_memory, model, + effort_level: args.effort_level, session_title: args .session_title .as_deref() @@ -1475,6 +1495,7 @@ mod tests { agent_owner: None, no_base_prompt: false, base_prompt_content: None, + effort_level: None, } } @@ -2263,6 +2284,7 @@ channels = "ALL" #[test] fn test_permission_mode_wire_strings() { assert_eq!(PermissionMode::Default.as_wire_str(), "default"); + assert_eq!(PermissionMode::Auto.as_wire_str(), "auto"); assert_eq!(PermissionMode::AcceptEdits.as_wire_str(), "acceptEdits"); assert_eq!(PermissionMode::DontAsk.as_wire_str(), "dontAsk"); assert_eq!(PermissionMode::Plan.as_wire_str(), "plan"); @@ -2271,15 +2293,28 @@ channels = "ALL" #[test] fn test_permission_mode_is_default() { assert!(PermissionMode::Default.is_default()); + assert!(!PermissionMode::Auto.is_default()); assert!(!PermissionMode::AcceptEdits.is_default()); assert!(!PermissionMode::DontAsk.is_default()); assert!(!PermissionMode::Plan.is_default()); } + #[test] + fn test_permission_mode_auto_degrades_to_default_when_unsupported() { + // The wire string is "auto" — the adapter handles graceful downgrade + // to "default" when the active model does not support Auto mode. + // Verify only that the wire string is correct and distinct from "default". + let auto = PermissionMode::Auto; + assert_eq!(auto.as_wire_str(), "auto"); + assert_ne!(auto.as_wire_str(), "default"); + assert!(!auto.is_default()); + } + #[test] fn test_permission_mode_display() { assert_eq!(format!("{}", PermissionMode::DontAsk), "dontAsk"); assert_eq!(format!("{}", PermissionMode::Default), "default"); + assert_eq!(format!("{}", PermissionMode::Auto), "auto"); } #[test] diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 65c9dd6203..fc371e43f4 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -37,8 +37,8 @@ use filter::SubscriptionRule; use futures_util::FutureExt; use nostr::{PublicKey, ToBech32}; use pool::{ - AgentPool, ControlSignal, IdleSwitchResult, OwnedAgent, PromptContext, PromptOutcome, - PromptResult, PromptSource, SessionState, TimeoutKind, + AgentPool, ControlSignal, EffortReport, IdleSwitchResult, OwnedAgent, PromptContext, + PromptOutcome, PromptResult, PromptSource, SessionState, SetPoolEffortResult, TimeoutKind, }; use pool_lifecycle::PoolLifecycle; use queue::{CancelReason, EventQueue, FlushBatch, QueuedEvent, ThreadTags}; @@ -1116,6 +1116,9 @@ fn handle_relay_observer_control_event( Some("switch_model") => { handle_switch_model_control(&payload, pool, observer); } + Some("set_config_option") => { + handle_set_config_option_control(&payload, pool, observer); + } _ => { tracing::debug!(payload = %payload, "ignoring unknown observer control frame"); } @@ -1145,29 +1148,16 @@ fn handle_cancel_turn_control( None, &observer::ObserverContext { channel_id: Some(channel_id.to_string()), - session_id: None, - turn_id: None, - started_at: None, + ..Default::default() }, - serde_json::json!({ - "type": "cancel_turn", - "status": status, - }), + serde_json::json!({"type": "cancel_turn", "status": status}), ); } } -/// Handle a `switch_model` control frame (Phase 3a, Option ii). -/// -/// Busy path: deliver `SwitchModel` over the in-flight task's oneshot — the -/// task cancels the turn, sets `desired_model`, and requeues the batch so it -/// re-runs on a fresh session under the new model. A catalog miss surfaces -/// post-cancel via `create_session_and_apply_model` (the turn restarts on the -/// unchanged model + an `unsupported_model` result). -/// -/// Idle path: validate against the cached catalog *before* invalidating -/// (pre-cancel guard), then set `desired_model` + invalidate. The override -/// takes visible effect on the agent's next turn. +/// Handle a `switch_model` control frame. Busy path: deliver `SwitchModel` +/// over the in-flight task oneshot so it cancels + requeues on the new model. +/// Idle path: validate against catalog then set `desired_model` + invalidate. fn handle_switch_model_control( payload: &serde_json::Value, pool: &mut AgentPool, @@ -1186,18 +1176,14 @@ fn handle_switch_model_control( return; }; - // A turn is in flight for this channel iff a task_map entry exists. The - // agent is moved out of the pool during a turn, so the control oneshot is - // the only reachable lever; an idle channel has no such entry. + // A turn is in flight iff a task_map entry exists for this channel. let turn_in_flight = pool .task_map() .values() .any(|m| m.channel_id == Some(channel_id)); let status = if turn_in_flight { - // Busy path: deliver over the oneshot. `false` means the oneshot was - // already consumed this turn (a prior cancel/interrupt) — the turn is - // already ending, so the switch cannot land on it. + // Busy path: deliver over the oneshot (`false` = oneshot already consumed, turn ending). if signal_in_flight_task( pool, channel_id, @@ -1222,19 +1208,181 @@ fn handle_switch_model_control( None, &observer::ObserverContext { channel_id: Some(channel_id.to_string()), - session_id: None, - turn_id: None, - started_at: None, + ..Default::default() }, - serde_json::json!({ - "type": "switch_model", - "status": status, - "modelId": model_id, - }), + serde_json::json!({"type": "switch_model", "status": status, "modelId": model_id}), ); } } +/// Handle a `set_config_option` control frame. +/// +/// For the `thought_level` category (B5 effort path): validates the configId +/// against the pool's known capabilities, stores the pool-level `desired_effort`, +/// invalidates all idle sessions, and emits an immediate `"pending_session"` ack. +/// The final honest ack (`ok`, `failure`, or `cleared`) is emitted by the main +/// loop's `PoolEvent::EffortReport` arm via `pool.resolve_effort_report(...)`, +/// which runs immediately after `create_session_and_apply_model` sends the +/// `EffortReport` — before the prompt is issued — satisfying the Desktop's +/// 8-second `awaitEffortOutcome` window (F2). +/// +/// Unknown configIds and non-effort options are passed through with a synthetic +/// `"ok"` ack (the pre-B5 behaviour), so existing callers don't break. +/// +/// I-7 harness validation: if the incoming value is not in the adapter-advertised +/// options for the `thought_level` configId, returns `"invalid_value"` immediately +/// without updating the pool. +fn handle_set_config_option_control( + payload: &serde_json::Value, + pool: &mut AgentPool, + observer: Option<&observer::ObserverHandle>, +) { + let Some(obs) = observer else { return }; + let config_id = payload + .get("configId") + .and_then(|v| v.as_str()) + .unwrap_or("unknown"); + let value = payload.get("value").and_then(|v| v.as_str()).unwrap_or(""); + // Nonce from the Desktop — echoed in all acks so the Desktop can correlate + // results to the specific request that generated them, ignoring stale acks + // from superseded picks or cleared efforts. + let nonce = payload + .get("nonce") + .and_then(|v| v.as_str()) + .map(str::to_string); + + // B5: for the thought_level configId, forward to the pool and report the + // real outcome. The configId the caller sends must match what the adapter + // advertised in session/new (agentConfigCore.ts uses the one from the + // session cache via deferredUntilNativeOptionsAvailable resolution). + // + // V-2 fix: use pool-level capability cache instead of scanning idle agents. + // Checked-out workers leave None slots; the cache is written at return_agent + // once the first worker completes a session and returns to the pool. + // + // Four cases: + // A. effort_capabilities.config_id is Some and matches → full validation path + // B. effort_capabilities.config_id is None AND capabilities not yet discovered + // AND no category trust → unknown configId → synthetic ok (non-effort) + // C. effort_capabilities.config_id is None AND capabilities were discovered → + // all workers are currently busy; trust the incoming configId from the + // Desktop's session cache and store for apply at next checkout/session. + // D. effort_capabilities.config_id is None AND capabilities not yet discovered + // AND category == "thought_level" → pre-first-session window; Desktop + // sends its session-cache configId with category as the trust signal. + // Store the value and ack pending_session — the first session creation + // will apply it and emit the honest final ok/failure. + let thought_level_id = pool + .effort_capabilities + .config_id + .as_deref() + .map(str::to_string); + + // Category from the incoming frame (Desktop sends "thought_level" for all + // effort picks and clears, including during the pre-discovery window). + let frame_category = payload + .get("category") + .and_then(|v| v.as_str()) + .unwrap_or(""); + + // Determine if this is a thought_level pick: + // Case A: cache has a matching configId. + // Case C: capabilities known from a prior session, all workers busy. + // Case D: pre-first-session, Desktop asserts category = "thought_level". + // + // For the clear path (empty value), the same cases apply. + let cache_matches = thought_level_id.as_deref() == Some(config_id); + // Case C: capabilities were discovered before (so configId is known) but + // workers are currently checked out and the cache is temporarily None. + let all_busy_with_known_caps = + pool.capabilities_ever_discovered && thought_level_id.is_none() && config_id != "unknown"; + // Case D: pre-first-session; Desktop sends category as the trust signal. + let pre_discovery_trusted = !pool.capabilities_ever_discovered + && frame_category == "thought_level" + && config_id != "unknown"; + let is_thought_level = cache_matches || all_busy_with_known_caps || pre_discovery_trusted; + + let (status, include_category) = if is_thought_level { + // I-7: validate the incoming value against adapter-advertised options + // from the pool-level capability cache. Works even when all workers + // are checked out — the cache was written at the first session creation. + // Skip validation when cache is empty (case C — workers busy, no cache + // to validate against; trust the Desktop's session-cache configId). + let valid_values = &pool.effort_capabilities.valid_values; + + if !valid_values.is_empty() + && !value.is_empty() + && !valid_values.contains(&value.to_string()) + { + // Value is not in the adapter's advertised option set. + tracing::warn!( + target: "pool::effort", + "effort value {value:?} not in advertised options {valid_values:?} — rejecting" + ); + let mut ack = serde_json::json!({ + "type": "set_config_option", + "configId": config_id, + "status": "invalid_value", + "value": value, + }); + ack["category"] = serde_json::json!("thought_level"); + // P2-1: echo the nonce on the early-return rejection path, matching + // the behaviour of the common ack builder below. Without this the + // Desktop's awaitEffortOutcome nonce guard rejects the ack, causing + // an 8 s timeout instead of an immediate "invalid_value" outcome. + if let Some(ref n) = nonce { + ack["nonce"] = serde_json::json!(n); + } + obs.emit( + "control_result", + None, + &observer::ObserverContext::default(), + ack, + ); + return; + } + + // Empty value = clear (Auto). Bypass set_pool_effort. + if value.is_empty() { + pool.pending_effort_nonce = nonce.clone(); + pool.clear_pool_effort(); + ("pending_session", true) + } else { + pool.pending_effort_nonce = nonce.clone(); + let result = pool.set_pool_effort(config_id, value); + match result { + SetPoolEffortResult::Stored { .. } => ("pending_session", true), + } + } + } else { + // Not a thought_level option — synthetic ok (no-op behaviour unchanged). + ("ok", false) + }; + + // B5: include "category": "thought_level" ONLY on the real-forward branch. + // Synthetic acks carry no category so the Desktop observer cannot persist + // them as if they were confirmed thought_level changes. + let mut ack = serde_json::json!({ + "type": "set_config_option", + "configId": config_id, + "status": status, + "value": value, + }); + if include_category { + ack["category"] = serde_json::json!("thought_level"); + } + if let Some(ref n) = nonce { + ack["nonce"] = serde_json::json!(n); + } + + obs.emit( + "control_result", + None, + &observer::ObserverContext::default(), + ack, + ); +} + /// 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. @@ -1999,7 +2147,8 @@ async fn tokio_main() -> Result<()> { Result(Box), Panic(tokio::task::JoinError), SteerAck(SteerAckEvent), - Wake(u32, Result), + Wake(u32, Box>), + EffortReport(Box), } loop { @@ -2093,6 +2242,11 @@ async fn tokio_main() -> Result<()> { model_capabilities: None, desired_model: config.model.clone(), model_overridden: false, + desired_effort: None, + startup_effort: config.effort_level.clone(), + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, agent_name, goose_system_prompt_supported: None, protocol_version, @@ -2120,7 +2274,7 @@ async fn tokio_main() -> Result<()> { // Borrow result_rx and join_set simultaneously via split-borrow helper. let pool_event: Option = { - let (result_rx, join_set) = pool.rx_and_join_set(); + let (result_rx, join_set, effort_report_rx) = pool.rx_and_join_set(); tokio::select! { biased; // recv() returning None means all senders dropped (pool was torn down). @@ -2147,7 +2301,13 @@ async fn tokio_main() -> Result<()> { Some(PoolEvent::SteerAck(ack_event)) } Some((attempt, result)) = wake_rx.recv(), if config.lazy_pool && !pool_ready => { - Some(PoolEvent::Wake(attempt, result)) + Some(PoolEvent::Wake(attempt, Box::new(result))) + } + // Pre-prompt effort application results from worker tasks. Processed + // here — before turn completion — so the ack reaches the Desktop + // within the 8-second awaitEffortOutcome window (F2). + Some(report) = effort_report_rx.recv() => { + Some(PoolEvent::EffortReport(Box::new(report))) } // Gated on pending work: with an empty queue there is nothing // for the retry to dispatch, and a past `retry_at` would @@ -2885,6 +3045,7 @@ async fn tokio_main() -> Result<()> { } } Some(PoolEvent::Wake(attempt, result)) => { + let result = *result; let completion = result.as_ref().map(|_| ()).map_err(|error| error.clone()); if let Err(error) = pool_lifecycle.complete_wake(attempt, result, tokio::time::Instant::now()) @@ -2925,6 +3086,13 @@ async fn tokio_main() -> Result<()> { } } } + Some(PoolEvent::EffortReport(report)) => { + // Pre-prompt effort result from a worker task: apply the + // first-terminal-wins gate and emit the terminal ack before the + // prompt is sent, so the Desktop receives it within the 8-second + // awaitEffortOutcome window (F2). + pool.resolve_effort_report(*report, observer.as_ref()); + } None => {} // relay/heartbeat/shutdown branches handled inline above } } @@ -2962,7 +3130,7 @@ async fn tokio_main() -> Result<()> { // explicitly shut them down here to reap child processes. If the grace // period expires, remaining tasks are aborted and fall back to // AcpClient::Drop (start_kill + try_wait — best-effort, not guaranteed). - let (rx_ref, js_ref) = pool.rx_and_join_set(); + let (rx_ref, js_ref, _effort_report_rx) = pool.rx_and_join_set(); let shutdown_result = tokio::time::timeout(grace, async { loop { tokio::select! { @@ -3274,6 +3442,7 @@ fn dispatch_pending( }; let result_tx = pool.result_tx(); + let effort_report_tx = pool.effort_report_tx(); let ctx_clone = Arc::clone(ctx); let agent_index = agent.index; @@ -3303,6 +3472,7 @@ fn dispatch_pending( None, ctx_clone, result_tx, + effort_report_tx, Some(control_rx), task_turn_id, ) @@ -3905,6 +4075,7 @@ fn dispatch_heartbeat( .clone() .unwrap_or_else(default_heartbeat_prompt); let result_tx = pool.result_tx(); + let effort_report_tx = pool.effort_report_tx(); let ctx_clone = Arc::clone(ctx); let agent_index = agent.index; let turn_id = Uuid::new_v4().to_string(); @@ -3917,6 +4088,7 @@ fn dispatch_heartbeat( Some(prompt_text), ctx_clone, result_tx, + effort_report_tx, None, task_turn_id, ) @@ -4090,6 +4262,12 @@ struct PoolStartup { extra_env: Vec<(String, String)>, has_generated_codex_config: bool, model: Option, + /// Persisted effort value to apply at the first session creation by pairing + /// with the adapter's advertised `thought_level` configId. Carried from + /// `Config.effort_level` (the `BUZZ_ACP_EFFORT_LEVEL` env var). The configId + /// is unknown until capabilities arrive at session creation; it is never + /// hardcoded in the harness. + startup_effort: Option, observer: Option, } @@ -4102,6 +4280,7 @@ impl PoolStartup { extra_env: config.persona_env_vars.clone(), has_generated_codex_config: config.has_generated_codex_config, model: config.model.clone(), + startup_effort: config.effort_level.clone(), observer, } } @@ -4169,6 +4348,11 @@ async fn initialize_agent_pool( model_capabilities: None, desired_model: startup.model.clone(), model_overridden: false, + desired_effort: None, + startup_effort: startup.startup_effort.clone(), + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, agent_name, goose_system_prompt_supported: None, protocol_version, @@ -6211,6 +6395,7 @@ mod build_mcp_servers_tests { agent_owner: None, no_base_prompt: false, base_prompt_content: None, + effort_level: None, } } @@ -6433,6 +6618,7 @@ mod error_outcome_emission_tests { agent_owner: None, no_base_prompt: false, base_prompt_content: None, + effort_level: None, } } @@ -6465,6 +6651,11 @@ mod error_outcome_emission_tests { model_capabilities: None, desired_model: None, model_overridden: false, + desired_effort: None, + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, agent_name: "unknown".into(), goose_system_prompt_supported: None, // Error branches under test never read this; 1 is the legacy @@ -7851,3 +8042,546 @@ mod observer_payload_trim_tests { assert!(leaf.contains("[elided")); } } + +#[cfg(test)] +mod control_result_tests { + use super::*; + + // ── B5 harness-level tests for handle_set_config_option_control ────────── + // + // These tests verify the ack emitted by handle_set_config_option_control + // carries the real pool outcome, not a synthetic "ok". + // + // The observer is checked via snapshot() after the call to verify + // both the kind ("control_result") and the status field. + // + // Implementation note: the harness only enters the thought_level branch when + // thought_level_id matches the incoming configId. When no agent has a + // thought_level_config_id set, the harness falls back to synthetic "ok" + // (backward compatibility — it cannot identify the option as thought_level). + // The meaningful test cases are therefore: + // 1. thought_level_config_id IS set and matches → pool outcome reflects reality + // (Stored → "pending_session"; clear → "cleared"; invalid → "invalid_value") + // 2. thought_level_config_id is NOT set (or pool empty) → synthetic ok + // 3. unknown configId → synthetic ok regardless + + /// B5: when the pool has an agent whose thought_level_config_id matches + /// the incoming configId, the ack must carry the real pool outcome — + /// here Stored → "pending_session" (final result arrives from session creation). + #[tokio::test] + async fn test_b5_set_config_option_stored_emits_pending_session_ack_and_invalidates() { + use crate::acp::AcpClient; + use crate::pool::AgentModelCapabilities; + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("failed to spawn test agent"); + let ch = uuid::Uuid::new_v4(); + let mut state = SessionState::default(); + state.sessions.insert(ch, "sess-1".into()); + let agent = OwnedAgent { + index: 0, + acp, + state, + model_capabilities: Some(AgentModelCapabilities { + config_options_raw: vec![], + available_models_raw: None, + // thought_level_config_id matches the configId we'll send. + thought_level_config_id: Some("effort".to_string()), + }), + desired_model: None, + model_overridden: false, + desired_effort: None, + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + }; + let mut pool = AgentPool::from_slots(vec![Some(agent)]); + pool.notify_capabilities_discovered(&AgentModelCapabilities { + thought_level_config_id: Some("effort".to_string()), + config_options_raw: vec![], + available_models_raw: None, + }); + let obs = observer::ObserverHandle::in_process(); + let payload = serde_json::json!({ + "type": "set_config_option", + "configId": "effort", + "value": "high", + }); + handle_set_config_option_control(&payload, &mut pool, Some(&obs)); + let events = obs.snapshot(); + assert_eq!(events.len(), 1); + let ev = &events[0]; + assert_eq!(ev.kind, "control_result"); + assert_eq!(ev.payload["type"].as_str().unwrap(), "set_config_option"); + // Stored → "pending_session" ack — final result arrives from session creation. + assert_eq!( + ev.payload["status"].as_str().unwrap(), + "pending_session", + "Stored must yield pending_session ack (not ok — adapter hasn't confirmed yet)" + ); + // Real-forward ack must carry category so Desktop knows to persist. + assert_eq!( + ev.payload["category"].as_str().unwrap(), + "thought_level", + "real thought_level forward must include category field" + ); + // Session must be invalidated so next turn creates a fresh session. + let agent = pool.agents_mut().iter().flatten().next().unwrap(); + assert!( + agent.state.sessions.is_empty(), + "session must be invalidated after effort queued" + ); + } + + /// B5 I-1 / I-3: when value is empty (Auto selected), the immediate ack must + /// be "pending_session" (non-terminal) — the final "cleared" ack arrives via + /// `PoolEvent::EffortReport` → `resolve_effort_report` when the session first + /// runs without effort. + /// The pool-level desired_effort is cleared immediately so future sessions do + /// not apply a stale value. + #[tokio::test] + async fn test_b5_empty_value_emits_pending_session_ack() { + use crate::acp::AcpClient; + use crate::pool::AgentModelCapabilities; + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("failed to spawn test agent"); + let agent = OwnedAgent { + index: 0, + acp, + state: SessionState::default(), + model_capabilities: Some(AgentModelCapabilities { + config_options_raw: vec![], + available_models_raw: None, + thought_level_config_id: Some("effort".to_string()), + }), + desired_model: None, + model_overridden: false, + desired_effort: Some(("effort".to_string(), "high".to_string())), + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + }; + let mut pool = AgentPool::from_slots(vec![Some(agent)]); + pool.desired_effort = Some(("effort".to_string(), "high".to_string())); + pool.notify_capabilities_discovered(&AgentModelCapabilities { + thought_level_config_id: Some("effort".to_string()), + config_options_raw: vec![], + available_models_raw: None, + }); + let obs = observer::ObserverHandle::in_process(); + // Empty value = Auto (clear path). + let payload = serde_json::json!({ + "type": "set_config_option", + "configId": "effort", + "value": "", + }); + handle_set_config_option_control(&payload, &mut pool, Some(&obs)); + let events = obs.snapshot(); + assert_eq!(events.len(), 1); + assert_eq!( + events[0].payload["status"].as_str().unwrap(), + "pending_session", + "empty value must yield pending_session immediate ack (final cleared comes from session creation)" + ); + assert_eq!( + events[0].payload["category"].as_str().unwrap(), + "thought_level", + "pending_session ack must include thought_level category" + ); + // Pool desired_effort must be cleared so future sessions omit effort. + assert!( + pool.desired_effort.is_none(), + "pool desired_effort must be None after Auto" + ); + } + + /// B5 I-7: when the incoming value is not in the adapter-advertised options, + /// the ack must be "invalid_value" and the pool must NOT be updated. + #[tokio::test] + async fn test_b5_invalid_value_emits_invalid_value_ack_and_does_not_update_pool() { + use crate::acp::AcpClient; + use crate::pool::AgentModelCapabilities; + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("failed to spawn test agent"); + let agent = OwnedAgent { + index: 0, + acp, + state: SessionState::default(), + model_capabilities: Some(AgentModelCapabilities { + config_options_raw: vec![serde_json::json!({ + "id": "effort", + "category": "thought_level", + "options": [ + { "value": "low" }, + { "value": "medium" }, + { "value": "high" }, + ], + })], + available_models_raw: None, + thought_level_config_id: Some("effort".to_string()), + }), + desired_model: None, + model_overridden: false, + desired_effort: None, + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + }; + let mut pool = AgentPool::from_slots(vec![Some(agent)]); + pool.notify_capabilities_discovered(&AgentModelCapabilities { + thought_level_config_id: Some("effort".to_string()), + config_options_raw: vec![serde_json::json!({ + "id": "effort", + "category": "thought_level", + "options": [ + { "value": "low" }, + { "value": "medium" }, + { "value": "high" }, + ], + })], + available_models_raw: None, + }); + let obs = observer::ObserverHandle::in_process(); + // "ultra" is not in the advertised options. + let payload = serde_json::json!({ + "type": "set_config_option", + "configId": "effort", + "value": "ultra", + }); + handle_set_config_option_control(&payload, &mut pool, Some(&obs)); + let events = obs.snapshot(); + assert_eq!(events.len(), 1); + assert_eq!( + events[0].payload["status"].as_str().unwrap(), + "invalid_value", + "non-advertised value must yield invalid_value ack" + ); + // Pool must NOT have been updated with the invalid value. + assert!( + pool.desired_effort.is_none(), + "pool must not be updated on invalid_value" + ); + } + + /// P2-1: the invalid_value early-return ack must echo the nonce so the + /// Desktop's awaitEffortOutcome nonce guard can match it. Without the + /// nonce the guard rejects the ack and the outcome falls through to the + /// 8 s timeout, showing "applies at next session" instead of an immediate + /// rejection UI. + #[tokio::test] + async fn test_b5_invalid_value_ack_echoes_nonce() { + use crate::acp::AcpClient; + use crate::pool::AgentModelCapabilities; + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("spawn"); + let agent = OwnedAgent { + index: 0, + acp, + state: SessionState::default(), + model_capabilities: Some(AgentModelCapabilities { + config_options_raw: vec![serde_json::json!({ + "id": "effort", + "category": "thought_level", + "options": [{"value": "low"}, {"value": "high"}], + })], + available_models_raw: None, + thought_level_config_id: Some("effort".to_string()), + }), + desired_model: None, + model_overridden: false, + desired_effort: None, + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + }; + let mut pool = AgentPool::from_slots(vec![Some(agent)]); + pool.notify_capabilities_discovered(&AgentModelCapabilities { + thought_level_config_id: Some("effort".to_string()), + config_options_raw: vec![serde_json::json!({ + "id": "effort", + "category": "thought_level", + "options": [{"value": "low"}, {"value": "high"}], + })], + available_models_raw: None, + }); + let obs = observer::ObserverHandle::in_process(); + // "ultra" is not in the advertised options; frame carries a nonce. + let payload = serde_json::json!({ + "type": "set_config_option", + "configId": "effort", + "value": "ultra", + "nonce": "abc-123", + }); + handle_set_config_option_control(&payload, &mut pool, Some(&obs)); + let events = obs.snapshot(); + assert_eq!(events.len(), 1); + assert_eq!( + events[0].payload["status"].as_str().unwrap(), + "invalid_value" + ); + assert_eq!( + events[0].payload["nonce"].as_str().unwrap(), + "abc-123", + "P2-1: invalid_value ack must echo the nonce" + ); + } + + /// B5: when the pool has no capabilities (pre-first-return) AND the frame + /// does not include `category: "thought_level"`, the harness cannot identify + /// the option as thought_level and falls back to synthetic "ok". This is + /// case B — the caller did not assert the category, so we treat the configId + /// as unknown. + #[test] + fn test_b5_set_config_option_no_thought_level_id_emits_synthetic_ok() { + let mut pool = AgentPool::from_slots(vec![]); + let obs = observer::ObserverHandle::in_process(); + let payload = serde_json::json!({ + "type": "set_config_option", + "configId": "effort", + "value": "high", + // No category field — case B (unknown, synthetic ok). + }); + handle_set_config_option_control(&payload, &mut pool, Some(&obs)); + let events = obs.snapshot(); + assert_eq!(events.len(), 1); + // No thought_level trust (no category field) → falls back to synthetic ok. + assert_eq!( + events[0].payload["status"].as_str().unwrap(), + "ok", + "without category trust and no pool capabilities, harness emits synthetic ok" + ); + // Synthetic ok must NOT carry category — Desktop must not persist it. + assert!( + events[0].payload.get("category").is_none() || events[0].payload["category"].is_null(), + "synthetic ok must not carry category field" + ); + } + + /// B5: a non-thought_level configId must still receive a synthetic "ok" + /// for backward compatibility with unknown options. + #[test] + fn test_b5_set_config_option_unknown_config_id_emits_synthetic_ok() { + let mut pool = AgentPool::from_slots(vec![]); + let obs = observer::ObserverHandle::in_process(); + let payload = serde_json::json!({ + "type": "set_config_option", + "configId": "some_unknown_option", + "value": "x", + }); + handle_set_config_option_control(&payload, &mut pool, Some(&obs)); + let events = obs.snapshot(); + assert_eq!(events.len(), 1); + assert_eq!( + events[0].payload["status"].as_str().unwrap(), + "ok", + "unknown configId must yield synthetic ok for backward compat" + ); + } + + // ── Case D: pre-first-return window with category trust ────────────────── + + /// Case D: in the pre-first-return window (capabilities never discovered), + /// a frame with `category: "thought_level"` is treated as a real effort pick — + /// value is stored, `pending_session` ack emitted (not synthetic ok), and + /// the pool's `desired_effort` is set for application at the first session. + #[test] + fn test_b5_pre_discovery_pick_with_category_stores_and_emits_pending_session() { + // Pre-first-return: no capabilities, all slots empty. + let mut pool = AgentPool::from_slots(vec![]); + let obs = observer::ObserverHandle::in_process(); + let payload = serde_json::json!({ + "type": "set_config_option", + "configId": "effort", + "value": "high", + "category": "thought_level", + }); + handle_set_config_option_control(&payload, &mut pool, Some(&obs)); + let events = obs.snapshot(); + assert_eq!(events.len(), 1); + let ev = &events[0]; + // Must be pending_session — not synthetic ok. + assert_eq!( + ev.payload["status"].as_str().unwrap(), + "pending_session", + "case D: pre-discovery pick with category trust must emit pending_session, not synthetic ok" + ); + // Must carry category so observer knows this is a real forward. + assert_eq!( + ev.payload["category"].as_str().unwrap(), + "thought_level", + "case D: pending_session ack must carry category" + ); + // Value must be stored in the pool. + assert_eq!( + pool.desired_effort + .as_ref() + .map(|(id, v)| (id.as_str(), v.as_str())), + Some(("effort", "high")), + "case D: pool must store the effort value for application at first session" + ); + // effort_ever_picked set so startup seeding cannot clobber the pick. + assert!( + pool.effort_ever_picked, + "case D: effort_ever_picked must be set" + ); + } + + /// Case D: in the pre-first-return window, a clear (empty value) with + /// `category: "thought_level"` must NOT emit synthetic ok — it must emit + /// `pending_session` (non-terminal immediate ack) and set `effort_ever_picked`. + /// The final `cleared` ack arrives via `PoolEvent::EffortReport` → `resolve_effort_report`. + #[test] + fn test_b5_pre_discovery_clear_with_category_emits_pending_session_not_synthetic_ok() { + let mut pool = AgentPool::from_slots(vec![]); + let obs = observer::ObserverHandle::in_process(); + let payload = serde_json::json!({ + "type": "set_config_option", + "configId": "effort", + "value": "", + "category": "thought_level", + }); + handle_set_config_option_control(&payload, &mut pool, Some(&obs)); + let events = obs.snapshot(); + assert_eq!(events.len(), 1); + let ev = &events[0]; + // Must be pending_session — not synthetic ok (and not immediate cleared). + assert_eq!( + ev.payload["status"].as_str().unwrap(), + "pending_session", + "case D: pre-discovery clear with category trust must emit pending_session, not synthetic ok" + ); + assert_eq!(ev.payload["category"].as_str().unwrap(), "thought_level"); + assert!(pool.effort_ever_picked, "clear must set effort_ever_picked"); + assert!( + pool.desired_effort.is_none(), + "clear must set desired_effort to None" + ); + } + + /// B5 persistence gate — real-forward ack carries `"category": "thought_level"`. + /// The Desktop observer gates persistence on this field; renaming the adapter's + /// configId does not break persistence as long as the category is present. + #[tokio::test] + async fn test_b5_real_forward_ack_includes_thought_level_category() { + use crate::acp::AcpClient; + use crate::pool::AgentModelCapabilities; + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("failed to spawn test agent"); + // Use a renamed configId ("think_level_v2") to prove category-gating + // does not depend on a hardcoded "effort" literal. + let thought_level_id = "think_level_v2".to_string(); + let agent = OwnedAgent { + index: 0, + acp, + state: SessionState::default(), + model_capabilities: Some(AgentModelCapabilities { + config_options_raw: vec![], + available_models_raw: None, + thought_level_config_id: Some(thought_level_id.clone()), + }), + desired_model: None, + model_overridden: false, + desired_effort: None, + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + }; + let mut pool = AgentPool::from_slots(vec![Some(agent)]); + pool.notify_capabilities_discovered(&AgentModelCapabilities { + thought_level_config_id: Some(thought_level_id.clone()), + config_options_raw: vec![], + available_models_raw: None, + }); + let obs = observer::ObserverHandle::in_process(); + let payload = serde_json::json!({ + "type": "set_config_option", + "configId": thought_level_id, + "value": "high", + }); + handle_set_config_option_control(&payload, &mut pool, Some(&obs)); + let events = obs.snapshot(); + assert_eq!(events.len(), 1); + assert_eq!( + events[0].payload["status"].as_str().unwrap(), + "pending_session" + ); + // Real-forward ack must carry category so Desktop persists. + assert_eq!( + events[0].payload["category"].as_str().unwrap(), + "thought_level", + "real thought_level forward must include category field" + ); + } + + /// B5 persistence gate — synthetic ack (no thought_level_config_id in pool) + /// must NOT carry `"category"`. The Desktop observer gates persistence on the + /// category field; absent category means no persist. + #[test] + fn test_b5_synthetic_ok_ack_has_no_category() { + let mut pool = AgentPool::from_slots(vec![]); + let obs = observer::ObserverHandle::in_process(); + let payload = serde_json::json!({ + "type": "set_config_option", + "configId": "effort", + "value": "high", + }); + handle_set_config_option_control(&payload, &mut pool, Some(&obs)); + let events = obs.snapshot(); + assert_eq!(events.len(), 1); + assert_eq!(events[0].payload["status"].as_str().unwrap(), "ok"); + // Synthetic ack must NOT carry category — Desktop must not persist it. + assert!( + events[0].payload.get("category").is_none() || events[0].payload["category"].is_null(), + "synthetic ok ack must not carry category field" + ); + } +} diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index 8430307d9c..621109c50c 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -30,9 +30,9 @@ use tokio::time::timeout; use uuid::Uuid; use crate::acp::{ - extract_model_config_options, extract_model_state, model_in_catalog, - resolve_model_switch_method, AcpClient, AcpError, EnvVar, McpServer, ModelSwitchMethod, - StopReason, SystemPromptTransport, + extract_agent_config_options, extract_model_state, extract_thought_level_config_id, + model_in_catalog, resolve_model_switch_method, AcpClient, AcpError, EnvVar, McpServer, + ModelSwitchMethod, StopReason, SystemPromptTransport, }; use crate::config::{compose_session_title, DedupMode, PermissionMode}; use crate::observer; @@ -78,6 +78,10 @@ pub struct AgentModelCapabilities { pub config_options_raw: Vec, /// Unstable: SessionModelState from session/new. pub available_models_raw: Option, + /// B5: configId for the `thought_level` category option, if the adapter + /// advertised one in session/new. Stored so `handle_set_config_option_control` + /// can forward effort changes without hardcoding the adapter's configId. + pub thought_level_config_id: Option, } /// Per-channel session IDs and turn counters. @@ -162,6 +166,42 @@ pub struct OwnedAgent { /// desktop reader to distinguish a genuine runtime override from a stale /// session whose persona model was edited. Reset on spawn/restart. pub model_overridden: bool, + /// Task-local snapshot of the pool's `desired_effort` at checkout time. + /// Applied in every `create_session_and_apply_model` via `session/set_config_option`. + /// `config_id` is the adapter's actual id from `AgentModelCapabilities::thought_level_config_id`. + /// + /// Set to `pool.desired_effort` at checkout. On the first session creation, + /// if still `None`, `resolve_startup_effort` may arm it from `startup_effort` + /// and the capabilities-derived `thought_level` configId. When the agent is + /// returned to the pool, a freshly-resolved startup effort is propagated back + /// so the pool-level authority reflects the seeded value. + pub desired_effort: Option<(String, String)>, + /// Startup effort value from `BUZZ_ACP_EFFORT_LEVEL` (carried from the Desktop + /// record). At the first session creation, if `desired_effort` is None, this + /// value is paired with the capabilities-derived `thought_level` configId to + /// seed `desired_effort`. Non-fatal when absent or when the adapter does not + /// advertise `thought_level`. + pub startup_effort: Option, + /// Generation of the pool's `desired_effort` at the time this agent was + /// checked out. `None` for startup-seeded effort (no live pick has occurred). + /// + /// Used by `return_agent` to determine whether a `last_effort_result` is + /// current (generation matches `pool.effort_generation`) or stale (generation + /// superseded by a newer pick/clear). Stale results are discarded without + /// touching the pool's committed state. + pub desired_effort_gen: Option, + /// Nonce echoed in all effort acks for this checkout. Populated from the + /// incoming control frame in `handle_set_config_option_control` and forwarded + /// via `EffortReport` to `resolve_effort_report` (the main loop's + /// `PoolEvent::EffortReport` arm), which emits the terminal ack pre-prompt. + /// Desktop correlates acks by nonce to prevent stale results from settling a + /// newer pick's promise or persisting an overwritten value. + pub pending_effort_nonce: Option, + /// Result of applying `desired_effort` at the most recent session creation. + /// Set in `create_session_and_apply_model`; forwarded via `EffortReport` to + /// `resolve_effort_report` for commit/rollback and ack emission pre-prompt. + /// Also read by `return_agent` for V-3 discarded-failed invalidation. + pub last_effort_result: Option, /// Normalized agent name from initialize (`agentInfo.name`/`serverInfo.name`). pub agent_name: String, /// Whether Goose accepted its custom system-prompt method. `None` probes on @@ -216,6 +256,66 @@ impl OwnedAgent { self.goose_system_prompt_supported, ) } + + /// B5 startup-default: arms `desired_effort` from `startup_effort` + capabilities. + /// + /// Called once at first session creation after capabilities are populated. + /// No-op when `desired_effort` is already set (live pick takes precedence, + /// via the pool-level value copied at checkout), when a live pick/clear has + /// ever occurred (`desired_effort_gen.is_some()` — prevents resurrection of + /// the startup default after a user clear), when `startup_effort` is absent, + /// or when the adapter does not advertise a `thought_level` configId. + pub(crate) fn resolve_startup_effort(&mut self) { + if self.desired_effort.is_none() && self.desired_effort_gen.is_none() { + if let Some(ref value) = self.startup_effort.clone() { + if let Some(config_id) = self + .model_capabilities + .as_ref() + .and_then(|c| c.thought_level_config_id.clone()) + { + self.desired_effort = Some((config_id, value.clone())); + } + } + } + } +} + +/// Pool-level capability snapshot for the `thought_level` config option. +/// +/// Written at `return_agent` when a worker with populated capabilities returns +/// to the pool. Because checked-out agents carry their own `model_capabilities`, +/// this cache ensures `handle_set_config_option_control` can identify and +/// validate effort picks even when all workers are checked out (pool slots +/// are `None`). +#[derive(Debug, Clone, Default)] +pub struct PoolEffortCapabilities { + /// The adapter's `thought_level` configId from `session/new`. + /// `None` until the first session has been created. + pub config_id: Option, + /// Adapter-advertised option values for the `thought_level` config option. + /// Empty until the first session returns options. + pub valid_values: Vec, +} + +/// Pre-prompt effort application result sent from a worker task to the main loop +/// immediately after `session_set_config_option` resolves, before the prompt is +/// sent. The main loop processes this via `resolve_effort_report` under pool +/// authority so the first-terminal-wins gate, commit/rollback, and ack emission +/// all happen at a single site with the pool lock held — satisfying the 8-second +/// `awaitEffortOutcome` window in AgentConfigPanel regardless of turn duration. +#[derive(Debug)] +pub struct EffortReport { + /// Generation at checkout (`OwnedAgent::desired_effort_gen`). `None` for + /// startup-seeded effort; those are resolved via V-1 in `return_agent`. + pub checkout_gen: Option, + /// Outcome of the `session_set_config_option` call. + pub result: EffortApplicationResult, + /// Nonce from the originating control frame, echoed in all acks. + pub nonce: Option, + /// The configId used in the `session_set_config_option` call. + pub config_id: String, + /// The value submitted (empty string for a clear). + pub value: String, } /// Pool of agents with take-and-return ownership semantics. @@ -227,10 +327,64 @@ pub struct AgentPool { agents: Vec>, result_tx: mpsc::UnboundedSender, result_rx: mpsc::UnboundedReceiver, + /// Pre-prompt effort reports from worker tasks. Workers send here + /// immediately after `session_set_config_option` resolves (before + /// the prompt is issued), allowing the main loop to emit the terminal + /// ack well within the Desktop's 8-second `awaitEffortOutcome` window. + effort_report_tx: mpsc::UnboundedSender, + effort_report_rx: mpsc::UnboundedReceiver, pub join_set: JoinSet<()>, task_map: HashMap, + /// Pool-level desired effort `(config_id, value)` for the `thought_level` + /// config option. Single authoritative value shared by every worker. + /// + /// Applied in every `create_session_and_apply_model` call via + /// `session/set_config_option`. Set by `set_pool_effort` (live picker) and + /// seeded from the first agent's `startup_effort` at first session creation. + /// Clearing: `None` means "let the adapter choose its default." + /// + /// This is the **pending** value — what the next session will attempt to apply. + /// On adapter `ok`, `committed_effort` is updated to match. On `failure`, this + /// is rolled back to `committed_effort` so failed candidates are never retried. + pub desired_effort: Option<(String, String)>, + /// Last effort value confirmed by the adapter (`ok` from `session/set_config_option`, + /// or `None` for "no effort set / adapter default"). Rolled back to on failure. + pub committed_effort: Option<(String, String)>, + /// Monotonic generation counter. Incremented on every live pick or clear. + /// Carried on checked-out agents as `desired_effort_gen` and echoed in all acks + /// so Desktop can ignore stale results superseded by newer picks. + pub effort_generation: u64, + /// Nonce from the most recent `set_config_option` control frame. Carried to + /// checked-out agents as `pending_effort_nonce` and echoed in all acks + /// (immediate and final) so the Desktop can reject results from superseded picks. + pub pending_effort_nonce: Option, + /// Whether a live pick or clear has ever been applied to this pool via + /// `set_pool_effort` or `clear_pool_effort`. When `true`, `return_agent` + /// must NOT propagate a worker's startup-resolved `desired_effort` back to + /// pool level — a live pick/clear is always authoritative over startup seeding, + /// even if the pool value is `None` (i.e. the user explicitly cleared it). + pub effort_ever_picked: bool, + /// Pool-level capability cache for the `thought_level` config option. + /// + /// Written at `return_agent` (refreshed from the returned agent's + /// capabilities). Allows `handle_set_config_option_control` to identify + /// and validate effort picks regardless of idle occupancy. + pub effort_capabilities: PoolEffortCapabilities, + /// True once any worker has ever had capabilities populated and returned to + /// the pool. Used to distinguish "pre-first-return" (capabilities unknown + /// — case D trust path applies) from "all workers currently busy" + /// (capabilities known but cache temporarily empty — case C trust path) + /// in `handle_set_config_option_control`. + pub capabilities_ever_discovered: bool, + /// Generation of the last effort pick/clear for which a terminal ack + /// (ok/failure/cleared) has already been emitted. Used by `return_agent` + /// to emit exactly one terminal ack per generation when parallelism > 1: + /// the first worker whose result is processed owns the ack; subsequent + /// workers with the same generation are silenced. + /// + /// `None` means no terminal ack has been emitted yet for any generation. + pub last_acked_effort_gen: Option, } - /// Result returned by a completed prompt task. pub struct PromptResult { pub agent: OwnedAgent, @@ -575,12 +729,23 @@ impl AgentPool { /// the index invariant. pub fn from_slots(slots: Vec>) -> Self { let (result_tx, result_rx) = mpsc::unbounded_channel(); + let (effort_report_tx, effort_report_rx) = mpsc::unbounded_channel(); Self { agents: slots, result_tx, result_rx, + effort_report_tx, + effort_report_rx, join_set: JoinSet::new(), task_map: HashMap::new(), + desired_effort: None, + committed_effort: None, + effort_generation: 0, + pending_effort_nonce: None, + effort_ever_picked: false, + effort_capabilities: PoolEffortCapabilities::default(), + capabilities_ever_discovered: false, + last_acked_effort_gen: None, } } @@ -590,6 +755,10 @@ impl AgentPool { /// Pass 2: any idle agent. /// /// Returns `None` if all agents are checked out. + /// + /// At checkout, the pool's `desired_effort` is copied onto the returned agent + /// so it has the current pool-level value for the duration of its task. On + /// `return_agent`, a freshly-resolved startup effort is propagated back. pub fn try_claim(&mut self, channel_id: Option) -> Option { // Pass 1: prefer agent with existing session for this channel. if let Some(cid) = channel_id { @@ -599,18 +768,194 @@ impl AgentPool { .unwrap_or(false) }); if let Some(i) = idx { - return self.agents[i].take(); + let mut agent = self.agents[i].take().unwrap(); + // Always sync pool's desired_effort onto the agent at checkout so + // pool-level clears (clear_pool_effort) and live picks + // (set_pool_effort) are both reflected on the claimed agent. + agent.desired_effort = self.desired_effort.clone(); + // Carry the current generation so return_agent can determine + // whether this checkout's effort result is current or stale. + agent.desired_effort_gen = if self.effort_ever_picked { + Some(self.effort_generation) + } else { + None + }; + agent.pending_effort_nonce = self.pending_effort_nonce.clone(); + agent.last_effort_result = None; + return Some(agent); } } // Pass 2: first idle agent. let idx = self.agents.iter().position(|slot| slot.is_some()); - idx.map(|i| self.agents[i].take().unwrap()) + idx.map(|i| { + let mut agent = self.agents[i].take().unwrap(); + // Always sync pool's desired_effort (see above). + agent.desired_effort = self.desired_effort.clone(); + agent.desired_effort_gen = if self.effort_ever_picked { + Some(self.effort_generation) + } else { + None + }; + agent.last_effort_result = None; + agent.pending_effort_nonce = self.pending_effort_nonce.clone(); + agent + }) } /// Return an agent to its slot after a task completes. - pub fn return_agent(&mut self, agent: OwnedAgent) { + /// + /// Three seam fixes happen here: + /// + /// **V-1 (clear resurrection prevention):** propagates a startup-resolved + /// effort back to pool level ONLY when no live pick or clear has ever been + /// made (`!effort_ever_picked`). If the user has explicitly cleared the effort + /// (or made any live pick), the pool value is always authoritative — even when + /// it is `None` — and the returning worker must not re-adopt a stale value. + /// + /// **V-3 (convergence at return):** if the worker's checkout snapshot + /// (`agent.desired_effort`) differs from the current pool value (set OR + /// cleared while the worker was busy), the worker's sessions are invalidated + /// so the next session creation applies the current pool value. This closes + /// the window where a busy worker's surviving session runs at a stale effort. + /// + /// **Capability refresh:** the pool-level `effort_capabilities` cache is + /// updated from the returned agent's capabilities (if populated), ensuring + /// the cache is available even when all other workers are checked out. + pub fn return_agent(&mut self, mut agent: OwnedAgent) { let idx = agent.index; + + // V-1: propagate startup-resolved effort back to pool level — but ONLY + // when no live pick/clear has ever been made. A live pick/clear is always + // authoritative over startup seeding, even when pool.desired_effort is None + // (the user explicitly cleared it). + // + // P2-2: also seed committed_effort when the startup effort was successfully + // applied. Without this, committed_effort stays None after a startup + // application, so a subsequent rejected live pick rolls back to None instead + // of the confirmed startup value, contradicting the persisted record. + if !self.effort_ever_picked { + if let Some(ref e) = agent.desired_effort { + self.desired_effort = Some(e.clone()); + // Seed the committed baseline when this startup application succeeded. + // desired_effort_gen is None for startup agents (no live generation + // was assigned), so we can safely set committed_effort here without + // racing with the generation-based commit/rollback block below. + if matches!( + agent.last_effort_result, + Some(EffortApplicationResult::Applied) + ) { + self.committed_effort = Some(e.clone()); + } + } + } + + // P2-3 (F2): terminal acks are now emitted by `resolve_effort_report` + // (called from the main loop's `PoolEvent::EffortReport` arm) immediately + // after `session_set_config_option` resolves — before the prompt is sent. + // This ensures acks arrive well within the Desktop's 8-second + // `awaitEffortOutcome` window regardless of turn duration. The + // first-terminal-wins gate and state resolution (commit/rollback + + // last_acked_effort_gen) all happen there. + // + // This block retains only what resolve_effort_report cannot: the + // none-result path (no session created, skip everything) and the + // already-resolved skips for same-gen and stale-gen returns. + + // F1: stale-gen force-invalidation. + // + // When a worker returns with a stale checkout generation (checkout_gen != + // effort_generation) while the CURRENT generation is still unresolved + // (last_acked_effort_gen != Some(effort_generation)), force-invalidate the + // agent's sessions regardless of value equality. This ensures the next + // claim creates a fresh session under the current pool value and produces + // the terminal ack for the newest nonce. + // + // Without this fix: rapid high→medium→high picks on one worker leave g3 + // permanently unresolved when the worker returns for g1 with a value match + // (V-3's equality check skips invalidation, so no new session is created, + // so no ack is ever emitted for g3). The same None==None hole applies to + // repeated clears. + if let Some(checkout_gen) = agent.desired_effort_gen { + if checkout_gen != self.effort_generation + && self.last_acked_effort_gen != Some(self.effort_generation) + { + // Stale return while current gen is unresolved — force a fresh + // session so the current pick can be applied and acked. + agent.state.invalidate_all(); + } + } + + // V-3: if the worker's checkout snapshot differs from the current pool + // value (i.e. a pick or clear arrived while this worker was busy), invalidate + // the worker's sessions so the next claim creates a fresh session under + // the current pool value. + // + // Value-comparison covers most cases: + // - pool cleared (None) while agent carried Some(_) → mismatch → invalidate + // - pool picked Some(y) while agent carried Some(x) or None → mismatch → invalidate + // + // Unconditional Failed invalidation (subsumes the old discarded_failed predicate): + // A Failed application means session_set_config_option rejected the pick, + // so the live session ran at the agent default — not at the requested value. + // The session is stale regardless of value equality AND regardless of whether + // resolve_effort_report has already run (the return→report ordering race). + // We force-invalidate any returned worker whose last_effort_result is Failed. + // Worst case is one redundant session re-creation when the rollback target + // happens to equal the default. + let failed_application = matches!( + agent.last_effort_result, + Some(EffortApplicationResult::Failed) + ); + if agent.desired_effort != self.desired_effort || failed_application { + agent.state.invalidate_all(); + } + + // Capability refresh: write back the agent's capability snapshot to the + // pool-level cache so validation remains available when all workers are + // checked out. Always overwrite — returned workers have fresh capabilities. + if let Some(ref caps) = agent.model_capabilities { + if caps.thought_level_config_id.is_some() { + let valid_values = caps + .config_options_raw + .iter() + .find(|opt| { + opt.get("id") + .or_else(|| opt.get("configId")) + .and_then(|v| v.as_str()) + == caps.thought_level_config_id.as_deref() + }) + .and_then(|opt| opt.get("options")) + .and_then(|o| o.as_array()) + .map(|arr| { + arr.iter() + .filter_map(|v| { + v.get("value").and_then(|s| s.as_str()).map(str::to_string) + }) + .collect() + }) + .unwrap_or_default(); + self.effort_capabilities = PoolEffortCapabilities { + config_id: caps.thought_level_config_id.clone(), + valid_values, + }; + self.capabilities_ever_discovered = true; + } else if self.capabilities_ever_discovered { + // P3: the returning agent has populated capabilities but no + // thought_level configId — the model was swapped to one that + // does not support effort. Clear the pool-level cache so + // subsequent picks are not validated against stale options. + // + // capabilities_ever_discovered stays true: we have seen at + // least one session. This activates case C in + // handle_set_config_option_control, letting picks through + // without value-validation while the cache is empty (correct + // — the user can still send a pick; the adapter will reject + // it if the value is unsupported). + self.effort_capabilities = PoolEffortCapabilities::default(); + } + } + if self.agents[idx].is_some() { // This is a bug: two tasks returned the same agent index. Log it // loudly so it shows up in production logs, then overwrite — the @@ -700,13 +1045,25 @@ impl AgentPool { self.result_tx.clone() } - /// Split-borrow: returns mutable refs to `result_rx` and `join_set` - /// simultaneously. This lets callers poll both in a single `select!` - /// without a double-borrow error on `&mut AgentPool`. + pub fn effort_report_tx(&self) -> mpsc::UnboundedSender { + self.effort_report_tx.clone() + } + + /// Split-borrow: returns mutable refs to `result_rx`, `join_set`, and + /// `effort_report_rx` simultaneously. This lets callers poll all three in a + /// single `select!` without double-borrow errors on `&mut AgentPool`. pub fn rx_and_join_set( &mut self, - ) -> (&mut mpsc::UnboundedReceiver, &mut JoinSet<()>) { - (&mut self.result_rx, &mut self.join_set) + ) -> ( + &mut mpsc::UnboundedReceiver, + &mut JoinSet<()>, + &mut mpsc::UnboundedReceiver, + ) { + ( + &mut self.result_rx, + &mut self.join_set, + &mut self.effort_report_rx, + ) } /// Non-blocking drain of the result channel. Used during shutdown to @@ -715,6 +1072,112 @@ impl AgentPool { self.result_rx.try_recv() } + /// Resolve an effort application result reported pre-prompt from a worker task. + /// + /// This is the first-terminal-wins P2-3 gate that was previously embedded in + /// `return_agent`. Moving it here (called from the main loop's + /// `PoolEvent::EffortReport` arm) ensures the terminal ack is emitted + /// immediately after `session_set_config_option` resolves — before the prompt + /// is sent — satisfying the Desktop's 8-second `awaitEffortOutcome` window + /// regardless of how long the turn takes. + /// + /// `return_agent` retains V-1 (startup propagation), V-3 (convergence + /// invalidation), and capability refresh, but no longer emits effort acks. + /// + /// Startup reports (`checkout_gen == None`) are no-ops here: startup effort + /// is applied silently, resolved via V-1 at return_agent time. + pub fn resolve_effort_report( + &mut self, + report: EffortReport, + observer: Option<&observer::ObserverHandle>, + ) { + let Some(checkout_gen) = report.checkout_gen else { + // Startup agent — no ack needed. + return; + }; + let Some(obs) = observer else { return }; + + // Same first-terminal-wins gate as before (P2-3): only the FIRST same-gen + // report resolves state and emits the ack; later ones are silenced. + if checkout_gen != self.effort_generation + || self.last_acked_effort_gen == Some(checkout_gen) + { + return; + } + + let nonce_field = report.nonce.as_deref(); + match report.result { + EffortApplicationResult::Applied => { + self.committed_effort = self.desired_effort.clone(); + let mut ack = serde_json::json!({ + "type": "set_config_option", + "configId": report.config_id, + "value": report.value, + "status": "ok", + "category": "thought_level", + }); + if let Some(n) = nonce_field { + ack["nonce"] = serde_json::json!(n); + } + obs.emit( + "control_result", + None, + &observer::ObserverContext::default(), + ack, + ); + self.last_acked_effort_gen = Some(checkout_gen); + } + EffortApplicationResult::Cleared => { + self.committed_effort = None; + let mut ack = serde_json::json!({ + "type": "set_config_option", + "configId": report.config_id, + "value": "", + "status": "cleared", + "category": "thought_level", + }); + if let Some(n) = nonce_field { + ack["nonce"] = serde_json::json!(n); + } + obs.emit( + "control_result", + None, + &observer::ObserverContext::default(), + ack, + ); + self.last_acked_effort_gen = Some(checkout_gen); + } + EffortApplicationResult::Failed => { + // Roll back desired_effort to committed so the failed candidate + // is never recopied by try_claim. + self.desired_effort = self.committed_effort.clone(); + // Report the current committed value (post-rollback) in the ack. + let (ack_config_id, ack_value) = self + .desired_effort + .as_ref() + .map(|(cid, v)| (cid.as_str(), v.as_str())) + .unwrap_or((report.config_id.as_str(), "")); + let mut ack = serde_json::json!({ + "type": "set_config_option", + "configId": ack_config_id, + "value": ack_value, + "status": "failure", + "category": "thought_level", + }); + if let Some(n) = nonce_field { + ack["nonce"] = serde_json::json!(n); + } + obs.emit( + "control_result", + None, + &observer::ObserverContext::default(), + ack, + ); + self.last_acked_effort_gen = Some(checkout_gen); + } + } + } + /// Check whether a slot is alive: either idle in the pool or checked out /// for an in-flight task. Returns `false` only when the slot is truly /// empty and available for refill. @@ -795,6 +1258,97 @@ impl AgentPool { agent.state.invalidate_channel(&channel_id); IdleSwitchResult::Switched } + + /// B5: Set the pool-level desired effort `(config_id, value)`. + /// + /// Updates `AgentPool::desired_effort` so every subsequent session creation + /// (any worker) applies the new effort via `session_set_config_option`. + /// Invalidates all idle agents' sessions so the next turn immediately creates + /// a fresh session and applies the effort rather than waiting for the + /// current session to expire naturally. + /// + /// Sets `effort_ever_picked = true` so `return_agent` will never resurrect + /// a stale startup-resolved value. From this point on the pool value is always + /// authoritative over per-worker startup seeding. + /// + /// Returns the count of idle agents that had active sessions invalidated. + /// Callers should emit `"pending_session"` as the immediate ack; the final + /// `ok`/`failure` arrives via `PoolEvent::EffortReport` → `resolve_effort_report` + /// once a session is actually created and the ACP call completes. + /// + /// Clearing effort (value == "") is handled by the caller before this is + /// reached: the caller calls `clear_pool_effort` directly. This path is the + /// non-empty-value live-pick case. + pub fn set_pool_effort(&mut self, config_id: &str, value: &str) -> SetPoolEffortResult { + self.desired_effort = Some((config_id.to_string(), value.to_string())); + self.effort_ever_picked = true; + self.effort_generation += 1; + + // Invalidate all idle agents' sessions so the next turn applies the + // new effort immediately (rather than reusing a stale session). + let mut invalidated = 0u32; + for agent in self.agents.iter_mut().flatten() { + if !agent.state.sessions.is_empty() { + agent.state.invalidate_all(); + invalidated += 1; + } + } + SetPoolEffortResult::Stored { invalidated } + } + + /// Clear the pool-level desired effort. + /// + /// Sets `desired_effort` to `None` (adapter will use its own default) and + /// invalidates all idle sessions so the next session creation does not apply + /// a stale value. Called when the user selects "Auto (default)" in the + /// EffortPicker. + /// + /// Sets `effort_ever_picked = true` so `return_agent` will never resurrect + /// a stale startup-resolved value (V-1). + pub fn clear_pool_effort(&mut self) { + self.desired_effort = None; + self.effort_ever_picked = true; + self.effort_generation += 1; + for agent in self.agents.iter_mut().flatten() { + if !agent.state.sessions.is_empty() { + agent.state.invalidate_all(); + } + } + } + + /// Notify the pool that capabilities have been discovered for a worker. + /// + /// **Test-only helper.** In production the pool capability cache is written + /// by `return_agent` when a worker returns after completing its first session. + /// This function lets tests seed the cache directly without going through the + /// full spawn-session-return cycle. + #[cfg(test)] + pub fn notify_capabilities_discovered(&mut self, caps: &AgentModelCapabilities) { + if caps.thought_level_config_id.is_some() && self.effort_capabilities.config_id.is_none() { + let valid_values = caps + .config_options_raw + .iter() + .find(|opt| { + opt.get("id") + .or_else(|| opt.get("configId")) + .and_then(|v| v.as_str()) + == caps.thought_level_config_id.as_deref() + }) + .and_then(|opt| opt.get("options")) + .and_then(|o| o.as_array()) + .map(|arr| { + arr.iter() + .filter_map(|v| v.get("value").and_then(|s| s.as_str()).map(str::to_string)) + .collect() + }) + .unwrap_or_default(); + self.effort_capabilities = PoolEffortCapabilities { + config_id: caps.thought_level_config_id.clone(), + valid_values, + }; + self.capabilities_ever_discovered = true; + } + } } /// Outcome of [`AgentPool::switch_idle_agent_model`]. @@ -809,6 +1363,31 @@ pub enum IdleSwitchResult { NoIdleAgent, } +/// Outcome of [`AgentPool::set_pool_effort`]. +#[derive(Debug, PartialEq, Eq)] +pub enum SetPoolEffortResult { + /// Pool-level `desired_effort` stored and idle sessions invalidated. + /// `invalidated` is the count of idle agents whose sessions were cleared + /// (may be 0 if all agents are either checked out or have no session yet). + /// The final result arrives via `PoolEvent::EffortReport` → `resolve_effort_report`. + Stored { invalidated: u32 }, +} + +/// Result of applying the desired effort at session creation. +/// +/// Carried on [`OwnedAgent`] so [`AgentPool::return_agent`] can commit or +/// roll back the pool-level pending value without a separate side-channel. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum EffortApplicationResult { + /// Adapter accepted the value (`session/set_config_option` returned Ok). + Applied, + /// Adapter rejected or timed out — the failed value must be rolled back. + Failed, + /// Clear path: the session ran without an effort override, establishing + /// adapter default. Commits `committed_effort = None`. + Cleared, +} + /// Timeout for a single pre-prompt context fetch attempt (thread/DM history). /// Each call gets this budget; with one retry the total worst-case is /// 2 × CONTEXT_FETCH_TIMEOUT + CONTEXT_FETCH_RETRY_DELAY ≈ 6.5 s. @@ -882,6 +1461,7 @@ async fn resolve_new_session_channel_context( /// On error from `session_new_full()`, returns the `AcpError` — caller handles /// error reporting. Model-switch failures are logged and gracefully ignored /// (the agent proceeds with its default model). +#[allow(clippy::too_many_arguments)] async fn create_session_and_apply_model( agent: &mut OwnedAgent, ctx: &PromptContext, @@ -890,6 +1470,7 @@ async fn create_session_and_apply_model( channel_name: Option<&str>, channel_id: Option, channel_type: Option<&str>, + effort_report_tx: &mpsc::UnboundedSender, ) -> Result { // Build base_prompt + system_prompt + agent core + canvas metadata into a // single prompt. Standard protocol-v2 agents receive it in `session/new`; @@ -958,11 +1539,18 @@ async fn create_session_and_apply_model( // Populate model capabilities on first session creation. if agent.model_capabilities.is_none() { agent.model_capabilities = Some(AgentModelCapabilities { - config_options_raw: extract_model_config_options(&resp.raw), + config_options_raw: extract_agent_config_options(&resp.raw), available_models_raw: extract_model_state(&resp.raw), + thought_level_config_id: extract_thought_level_config_id(&resp.raw), }); } + // B5 startup-default: arm desired_effort from startup_effort + capabilities. + // No-op when desired_effort already set (live pick takes precedence, via the + // pool-level value copied at checkout) or when the adapter does not advertise + // thought_level for this model. + agent.resolve_startup_effort(); + // Apply desired_model if set, matching against the fresh session/new response. // Track whether the switch succeeded so session_config_captured reflects // the post-switch state (not the pre-switch desired state). @@ -996,6 +1584,97 @@ async fn create_session_and_apply_model( false }; + // B5: Apply desired_effort if set, or report a pending clear. + // + // After the real ACP call (or clear confirmation), send an `EffortReport` + // to the main loop via `effort_report_tx`. The main loop processes this in + // `PoolEvent::EffortReport` → `pool.resolve_effort_report(...)`, which emits + // the terminal `control_result` ack (ok/failure/cleared) under pool authority + // with the first-terminal-wins gate. Because this send happens here — before + // the prompt is issued — the ack arrives at the Desktop well within its + // 8-second `awaitEffortOutcome` window regardless of turn duration (F2). + // + // status: "ok" → adapter accepted; Desktop persists the value. + // status: "failure" → adapter rejected or timed out; Desktop does NOT persist. + // status: "cleared" → pending-clear confirmed (session ran without effort); + // Desktop persists null. + if let Some((ref config_id, ref value)) = agent.desired_effort.clone() { + let result = tokio::time::timeout(MODEL_SWITCH_TIMEOUT, async { + agent + .acp + .session_set_config_option(&resp.session_id, config_id, value) + .await + }) + .await; + let effort_result = match result { + Ok(Ok(_)) => { + tracing::info!( + target: "pool::effort", + "applied effort {value} via configId={config_id} on session {}", + resp.session_id + ); + EffortApplicationResult::Applied + } + Ok(Err(e @ AcpError::Io(_))) + | Ok(Err(e @ AcpError::WriteTimeout(_))) + | Ok(Err(e @ AcpError::Timeout(_))) + | Ok(Err(e @ AcpError::Protocol(_))) + | Ok(Err(e @ AcpError::AgentExited)) => { + tracing::error!( + target: "pool::effort", + "fatal error applying effort {value} via configId={config_id}: {e}" + ); + return Err(e); + } + Ok(Err(e)) => { + tracing::warn!( + target: "pool::effort", + "non-fatal error applying effort {value}: {e} — proceeding with agent default" + ); + EffortApplicationResult::Failed + } + Err(_timeout) => { + tracing::warn!( + target: "pool::effort", + "effort switch {value} timed out — proceeding with agent default" + ); + EffortApplicationResult::Failed + } + }; + // Record for V-3 (return_agent convergence check) and send the pre-prompt + // EffortReport so the main loop can ack before the turn starts. + agent.last_effort_result = Some(effort_result.clone()); + // Fire-and-forget: the receiver is the main loop which always outlives + // worker tasks. A send error here would mean the pool was torn down, in + // which case the turn itself is also about to fail. + let _ = effort_report_tx.send(EffortReport { + checkout_gen: agent.desired_effort_gen, + result: effort_result, + nonce: agent.pending_effort_nonce.clone(), + config_id: config_id.clone(), + value: value.clone(), + }); + } else if agent.desired_effort_gen.is_some() { + // Pending clear: the session ran without an effort override. + // Report via EffortReport so the main loop emits the "cleared" ack. + agent.last_effort_result = Some(EffortApplicationResult::Cleared); + // For the cleared config_id, use the agent's capabilities if available, + // falling back to "effort" (same fallback as the former return_agent path). + let cleared_config_id = agent + .model_capabilities + .as_ref() + .and_then(|c| c.thought_level_config_id.as_deref()) + .unwrap_or("effort") + .to_string(); + let _ = effort_report_tx.send(EffortReport { + checkout_gen: agent.desired_effort_gen, + result: EffortApplicationResult::Cleared, + nonce: agent.pending_effort_nonce.clone(), + config_id: cleared_config_id, + value: String::new(), + }); + } + // Emit session config for desktop consumption (config bridge tier 1b). // Emitted AFTER desired_model resolution so the desktop caches the // post-switch state. modelOverridden reflects whether the switch actually @@ -1387,12 +2066,14 @@ fn send_prompt_result( /// /// The agent is ALWAYS returned — even on panic the `JoinSet` detects the /// abort and the caller uses `task_map` to recover the agent index. +#[allow(clippy::too_many_arguments)] pub async fn run_prompt_task( mut agent: OwnedAgent, batch: Option, prompt_text: Option, ctx: Arc, result_tx: mpsc::UnboundedSender, + effort_report_tx: mpsc::UnboundedSender, control_rx: Option>, turn_id: String, ) { @@ -1610,6 +2291,7 @@ pub async fn run_prompt_task( title_channel.as_deref(), Some(*cid), origin_channel_type.as_deref(), + &effort_report_tx, ) .await { @@ -1657,8 +2339,17 @@ pub async fn run_prompt_task( if let Some(sid) = &agent.state.heartbeat_session { (sid.clone(), false) } else { - match create_session_and_apply_model(&mut agent, &ctx, None, None, None, None, None) - .await + match create_session_and_apply_model( + &mut agent, + &ctx, + None, + None, + None, + None, + None, + &effort_report_tx, + ) + .await { Ok(sid) => { tracing::info!( @@ -6015,6 +6706,11 @@ mod tests { model_capabilities: None, desired_model: None, model_overridden: false, + desired_effort: None, + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, agent_name: "unknown".into(), goose_system_prompt_supported: None, protocol_version: 2, @@ -6073,6 +6769,11 @@ mod tests { model_capabilities: None, desired_model: None, model_overridden: false, + desired_effort: None, + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, agent_name: "unknown".into(), goose_system_prompt_supported: None, protocol_version: 2, @@ -6996,3 +7697,1868 @@ mod tests { server.abort(); } } + +// ── B5 effort-switch pool tests ─────────────────────────────────────────────── + +#[cfg(test)] +mod effort_tests { + use super::*; + + /// `extract_thought_level_config_id` finds the configId for `thought_level` + /// category in a session/new response. + #[test] + fn test_extract_thought_level_config_id_from_session_new() { + let session_new = serde_json::json!({ + "configOptions": [ + { "id": "model", "category": "model", "options": [] }, + { "id": "effort", "category": "thought_level", "options": [ + { "value": "low" }, + { "value": "medium" }, + { "value": "high" }, + ]}, + ] + }); + let id = crate::acp::extract_thought_level_config_id(&session_new); + assert_eq!(id.as_deref(), Some("effort")); + } + + /// `extract_thought_level_config_id` returns None when no thought_level entry. + #[test] + fn test_extract_thought_level_config_id_returns_none_when_absent() { + let session_new = serde_json::json!({ + "configOptions": [ + { "id": "model", "category": "model", "options": [] }, + ] + }); + let id = crate::acp::extract_thought_level_config_id(&session_new); + assert_eq!(id, None); + } + + /// `extract_thought_level_config_id` accepts `configId` key (spec spelling). + #[test] + fn test_extract_thought_level_config_id_accepts_configid_key() { + let session_new = serde_json::json!({ + "configOptions": [ + { "configId": "thinking_effort", "category": "thought_level", "options": [] }, + ] + }); + let id = crate::acp::extract_thought_level_config_id(&session_new); + assert_eq!(id.as_deref(), Some("thinking_effort")); + } + + /// `extract_thought_level_config_id` returns None on empty configOptions. + #[test] + fn test_extract_thought_level_config_id_returns_none_on_empty() { + let session_new = serde_json::json!({ "configOptions": [] }); + let id = crate::acp::extract_thought_level_config_id(&session_new); + assert_eq!(id, None); + } + + /// `extract_thought_level_config_id` returns None when configOptions absent. + #[test] + fn test_extract_thought_level_config_id_returns_none_when_no_config_options() { + let session_new = serde_json::json!({}); + let id = crate::acp::extract_thought_level_config_id(&session_new); + assert_eq!(id, None); + } + + /// `set_pool_effort` stores the value even on an empty pool (no agents, no + /// capabilities discovered). The caller is responsible for gating calls on + /// `is_thought_level` — `set_pool_effort` always stores. + #[test] + fn test_set_pool_effort_stores_on_empty_pool() { + let mut pool = AgentPool::from_slots(vec![]); + let result = pool.set_pool_effort("effort", "high"); + assert_eq!(result, SetPoolEffortResult::Stored { invalidated: 0 }); + assert_eq!( + pool.desired_effort + .as_ref() + .map(|(id, v)| (id.as_str(), v.as_str())), + Some(("effort", "high")), + ); + } + + /// `set_pool_effort` stores the value when agents exist but no session has + /// been created yet (pre-first-return window). `set_pool_effort` always stores. + #[test] + fn test_set_pool_effort_stores_before_capabilities_discovered() { + // Pool with a None slot (agent not yet spawned or checked out). + let mut pool = AgentPool::from_slots(vec![None]); + let result = pool.set_pool_effort("effort", "high"); + assert_eq!(result, SetPoolEffortResult::Stored { invalidated: 0 }); + assert_eq!( + pool.desired_effort + .as_ref() + .map(|(id, v)| (id.as_str(), v.as_str())), + Some(("effort", "high")), + ); + } + + /// `AgentModelCapabilities::thought_level_config_id` is populated from the + /// correct field in the session/new response. + #[test] + fn test_thought_level_config_id_stored_in_capabilities() { + let caps = AgentModelCapabilities { + config_options_raw: vec![], + available_models_raw: None, + thought_level_config_id: Some("effort".to_string()), + }; + assert_eq!(caps.thought_level_config_id.as_deref(), Some("effort")); + } + + /// `set_pool_effort` stores the pool-level `desired_effort` and invalidates + /// all idle agents' sessions so the next turn creates a fresh one. + #[tokio::test] + async fn test_set_pool_effort_stores_and_invalidates_all_idle_sessions() { + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("failed to spawn test agent"); + let ch = uuid::Uuid::new_v4(); + let mut state = SessionState::default(); + state.sessions.insert(ch, "sess-1".into()); + let agent = OwnedAgent { + index: 0, + acp, + state, + model_capabilities: Some(AgentModelCapabilities { + config_options_raw: vec![], + available_models_raw: None, + thought_level_config_id: Some("effort".to_string()), + }), + desired_model: None, + model_overridden: false, + desired_effort: None, + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + }; + let mut pool = AgentPool::from_slots(vec![Some(agent)]); + pool.notify_capabilities_discovered(&AgentModelCapabilities { + thought_level_config_id: Some("effort".to_string()), + config_options_raw: vec![], + available_models_raw: None, + }); + let result = pool.set_pool_effort("effort", "high"); + assert_eq!( + result, + SetPoolEffortResult::Stored { invalidated: 1 }, + "must return Stored with invalidated=1" + ); + // Pool-level desired_effort must be set. + assert_eq!( + pool.desired_effort + .as_ref() + .map(|(id, v)| (id.as_str(), v.as_str())), + Some(("effort", "high")), + "pool-level desired_effort must be set" + ); + // Session must be invalidated so the next turn creates a fresh one. + let agent = pool.agents_mut().iter().flatten().next().unwrap(); + assert!( + agent.state.sessions.is_empty(), + "session must be invalidated after set_pool_effort" + ); + } + + /// `set_pool_effort` on a multi-worker pool invalidates ALL idle agents and + /// propagates the pool-level value to each worker at checkout via `try_claim`. + #[tokio::test] + async fn test_set_pool_effort_invalidates_all_workers_and_propagates_at_checkout() { + // Build two idle agents, each with a session. + let ch_a = uuid::Uuid::new_v4(); + let ch_b = uuid::Uuid::new_v4(); + + async fn make_worker(index: usize, ch: uuid::Uuid, session_id: &str) -> OwnedAgent { + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("spawn"); + let mut state = SessionState::default(); + state.sessions.insert(ch, session_id.to_string()); + OwnedAgent { + index, + acp, + state, + model_capabilities: Some(AgentModelCapabilities { + config_options_raw: vec![], + available_models_raw: None, + thought_level_config_id: Some("effort".to_string()), + }), + desired_model: None, + model_overridden: false, + desired_effort: None, + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + } + } + + let a = make_worker(0, ch_a, "sess-a").await; + let b = make_worker(1, ch_b, "sess-b").await; + let mut pool = AgentPool::from_slots(vec![Some(a), Some(b)]); + pool.notify_capabilities_discovered(&AgentModelCapabilities { + thought_level_config_id: Some("effort".to_string()), + config_options_raw: vec![], + available_models_raw: None, + }); + + // set_pool_effort must invalidate both idle workers. + let result = pool.set_pool_effort("effort", "high"); + assert_eq!( + result, + SetPoolEffortResult::Stored { invalidated: 2 }, + "both idle workers must be invalidated" + ); + + // Pool-level value set. + assert_eq!( + pool.desired_effort + .as_ref() + .map(|(id, v)| (id.as_str(), v.as_str())), + Some(("effort", "high")) + ); + + // At checkout, the pool's desired_effort is copied onto the claimed agent. + let claimed = pool.try_claim(Some(ch_a)).unwrap(); + assert_eq!( + claimed + .desired_effort + .as_ref() + .map(|(id, v)| (id.as_str(), v.as_str())), + Some(("effort", "high")), + "checked-out agent must carry pool's desired_effort" + ); + } + + /// `clear_pool_effort` sets pool-level `desired_effort` to None and + /// invalidates all idle sessions. A subsequently claimed agent has no + /// desired_effort (adapter uses its default). + #[tokio::test] + async fn test_clear_pool_effort_clears_and_invalidates_sessions() { + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("spawn"); + let ch = uuid::Uuid::new_v4(); + let mut state = SessionState::default(); + state.sessions.insert(ch, "sess-1".into()); + let agent = OwnedAgent { + index: 0, + acp, + state, + model_capabilities: Some(AgentModelCapabilities { + config_options_raw: vec![], + available_models_raw: None, + thought_level_config_id: Some("effort".to_string()), + }), + desired_model: None, + model_overridden: false, + desired_effort: Some(("effort".to_string(), "high".to_string())), + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + }; + let mut pool = AgentPool::from_slots(vec![Some(agent)]); + pool.desired_effort = Some(("effort".to_string(), "high".to_string())); + + pool.clear_pool_effort(); + + assert!( + pool.desired_effort.is_none(), + "pool desired_effort must be None after clear" + ); + // Session must be invalidated. + let agent = pool.agents_mut().iter().flatten().next().unwrap(); + assert!( + agent.state.sessions.is_empty(), + "session must be invalidated after clear" + ); + + // A claimed agent must not carry a desired_effort. + let claimed = pool.try_claim(Some(ch)).unwrap(); + assert!( + claimed.desired_effort.is_none(), + "claimed agent must not carry desired_effort after pool clear" + ); + } + + // ── resolve_startup_effort ──────────────────────────────────────────────── + + /// Helper to build a minimal `OwnedAgent` for `resolve_startup_effort` tests. + /// The bash subprocess is needed for `AcpClient` construction; none of the + /// tests below send any ACP messages. + async fn make_agent_for_startup_effort( + startup_effort: Option<&str>, + thought_level_config_id: Option<&str>, + desired_effort: Option<(&str, &str)>, + ) -> OwnedAgent { + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("failed to spawn test agent"); + OwnedAgent { + index: 0, + acp, + state: SessionState::default(), + model_capabilities: thought_level_config_id.map(|id| AgentModelCapabilities { + config_options_raw: vec![], + available_models_raw: None, + thought_level_config_id: Some(id.to_string()), + }), + desired_model: None, + model_overridden: false, + desired_effort: desired_effort.map(|(id, v)| (id.to_string(), v.to_string())), + startup_effort: startup_effort.map(str::to_string), + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + } + } + + /// `resolve_startup_effort` arms `desired_effort` from `startup_effort` and the + /// capabilities-derived `thought_level` configId on the first session creation. + #[tokio::test] + async fn test_resolve_startup_effort_arms_desired_effort_from_startup_value() { + let mut agent = make_agent_for_startup_effort(Some("high"), Some("tlevel-id"), None).await; + agent.resolve_startup_effort(); + assert_eq!( + agent + .desired_effort + .as_ref() + .map(|(id, v)| (id.as_str(), v.as_str())), + Some(("tlevel-id", "high")), + "desired_effort must be armed from startup_effort + thought_level configId" + ); + } + + /// `resolve_startup_effort` is a no-op when `desired_effort` is already set + /// (a live user pick must not be overridden by the startup default). + #[tokio::test] + async fn test_resolve_startup_effort_does_not_override_live_pick() { + let mut agent = make_agent_for_startup_effort( + Some("high"), + Some("tlevel-id"), + Some(("tlevel-id", "low")), // live pick already present + ) + .await; + agent.resolve_startup_effort(); + assert_eq!( + agent + .desired_effort + .as_ref() + .map(|(id, v)| (id.as_str(), v.as_str())), + Some(("tlevel-id", "low")), + "live desired_effort must not be overridden by startup default" + ); + } + + /// `resolve_startup_effort` is a no-op when no `startup_effort` was provided + /// (agent record had no persisted effort level). + #[tokio::test] + async fn test_resolve_startup_effort_noop_when_startup_effort_absent() { + let mut agent = make_agent_for_startup_effort(None, Some("tlevel-id"), None).await; + agent.resolve_startup_effort(); + assert!( + agent.desired_effort.is_none(), + "desired_effort must stay None when no startup_effort set" + ); + } + + /// `resolve_startup_effort` is a no-op when the adapter has no `thought_level` + /// configId (model does not support thinking effort). + #[tokio::test] + async fn test_resolve_startup_effort_noop_when_model_lacks_thought_level() { + let mut agent = make_agent_for_startup_effort( + Some("high"), + None, // no thought_level_config_id → not supported + None, + ) + .await; + agent.resolve_startup_effort(); + assert!( + agent.desired_effort.is_none(), + "desired_effort must stay None when adapter does not advertise thought_level" + ); + } + + /// `resolve_startup_effort` is a no-op when `desired_effort_gen` is `Some`, + /// which signals that a live pick or clear has already occurred. This prevents + /// the startup default from resurrecting a value the user explicitly cleared. + /// + /// Without this guard: user clears → pool.desired_effort=None, gen=Some(N) → + /// session creates, resolve_startup_effort re-arms Some(old) → applies old + /// value, emits ok with the clear's nonce → observer persists old over the clear. + #[tokio::test] + async fn test_resolve_startup_effort_noop_after_live_clear_gen_is_some() { + let mut agent = make_agent_for_startup_effort(Some("high"), Some("tlevel-id"), None).await; + // Simulate post-clear checkout: desired_effort=None, gen=Some (a clear occurred) + agent.desired_effort_gen = Some(3); + agent.resolve_startup_effort(); + assert!( + agent.desired_effort.is_none(), + "resolve_startup_effort must not re-arm after a live clear (gen is Some)" + ); + } + + // ── V-1: clear-while-busy resurrection prevention ──────────────────────── + + /// V-1: when the user clears effort while a worker is busy (checked out), + /// `return_agent` must NOT resurrect the cleared value. The pool stays None + /// and the returning worker's sessions are invalidated (V-3) so the next + /// session creates a fresh one without any effort applied. + /// + /// Sequence: + /// 1. Pool has effort "high" set; worker checks out carrying Some("high"). + /// 2. User selects Auto (clear) → `clear_pool_effort()` → pool.desired_effort = None, + /// effort_ever_picked = true. + /// 3. Worker returns carrying Some("high") (its checkout snapshot). + /// 4. `return_agent` sees effort_ever_picked = true → must NOT adopt worker's value. + /// 5. Pool stays None; worker's sessions are invalidated (checkout vs pool mismatch). + #[tokio::test] + async fn test_v1_clear_while_busy_return_does_not_resurrect_cleared_effort() { + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("spawn"); + let ch = uuid::Uuid::new_v4(); + let mut state = SessionState::default(); + state.sessions.insert(ch, "sess-1".into()); + // Worker has desire_effort=Some("high") — the checkout snapshot. + let agent = OwnedAgent { + index: 0, + acp, + state, + model_capabilities: Some(AgentModelCapabilities { + config_options_raw: vec![], + available_models_raw: None, + thought_level_config_id: Some("effort".to_string()), + }), + desired_model: None, + model_overridden: false, + desired_effort: Some(("effort".to_string(), "high".to_string())), + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + }; + let mut pool = AgentPool::from_slots(vec![None]); // slot empty — worker is "checked out" + pool.desired_effort = Some(("effort".to_string(), "high".to_string())); + pool.notify_capabilities_discovered(&AgentModelCapabilities { + thought_level_config_id: Some("effort".to_string()), + config_options_raw: vec![], + available_models_raw: None, + }); + + // User clears effort while worker is busy. + pool.clear_pool_effort(); + assert!( + pool.desired_effort.is_none(), + "pool must be None after clear" + ); + assert!( + pool.effort_ever_picked, + "effort_ever_picked must be set after clear" + ); + + // Worker returns carrying its old checkout snapshot (Some("high")). + pool.return_agent(agent); + + // V-1: pool must still be None — worker's stale value must not resurrect it. + assert!( + pool.desired_effort.is_none(), + "V-1: return_agent must NOT resurrect cleared effort when effort_ever_picked=true" + ); + + // V-3: returning worker's sessions must be invalidated (checkout != pool). + let returned = pool.agents_mut().iter().flatten().next().unwrap(); + assert!( + returned.state.sessions.is_empty(), + "V-3: sessions must be invalidated when checkout snapshot differs from pool value" + ); + } + + // ── V-2: pick-while-all-busy is stored, not dropped ────────────────────── + + /// V-2: when all workers are busy (slots are None) and capabilities are + /// known from a prior session, `set_pool_effort` stores the effort and + /// returns `Stored`. The `handle_set_config_option_control` path emits a + /// real `pending_session` ack (not a synthetic ok). + /// + /// The full `handle_set_config_option_control` path (pending_session ack, not + /// synthetic ok) is covered by `test_b5_set_config_option_stored_emits_pending_session_ack_and_invalidates` + /// in lib.rs. This test verifies the pool-layer storage guarantee. + #[test] + fn test_v2_pick_while_all_busy_is_stored_not_dropped() { + let mut pool = AgentPool::from_slots(vec![None]); // slot empty — all workers busy + pool.notify_capabilities_discovered(&AgentModelCapabilities { + thought_level_config_id: Some("effort".to_string()), + config_options_raw: vec![], + available_models_raw: None, + }); + + // All slots are None (workers checked out) but capabilities ARE known. + assert!(pool.effort_capabilities.config_id.is_some()); + assert!(pool.capabilities_ever_discovered); + assert!(pool.agents_mut().iter().flatten().next().is_none()); + + // set_pool_effort should store, not return NoCatalog. + let result = pool.set_pool_effort("effort", "high"); + assert!( + matches!(result, SetPoolEffortResult::Stored { .. }), + "V-2: set_pool_effort must store the value even when all workers are busy" + ); + assert_eq!( + pool.desired_effort + .as_ref() + .map(|(id, v)| (id.as_str(), v.as_str())), + Some(("effort", "high")), + "V-2: pool must store the effort value even when all workers are busy" + ); + } + + // ── V-3: convergence at return_agent ───────────────────────────────────── + + /// V-3: when a live pick arrives while a worker is busy, the returning + /// worker's surviving sessions must be invalidated so the next `try_claim` + /// creates a fresh session and applies the new pool value. + /// + /// Sequence: + /// 1. Worker is busy with checkout snapshot desired_effort = None. + /// 2. User picks "high" → `set_pool_effort` → pool.desired_effort = Some("high"). + /// 3. Worker returns — its checkout snapshot (None) differs from pool (Some("high")). + /// 4. `return_agent` invalidates the worker's sessions. + /// 5. Next `try_claim` syncs `desired_effort = Some("high")` at checkout. + #[tokio::test] + async fn test_v3_busy_worker_sessions_invalidated_on_return_after_pick() { + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("spawn"); + let ch = uuid::Uuid::new_v4(); + let mut state = SessionState::default(); + state.sessions.insert(ch, "old-sess".into()); + // Worker has desired_effort=None — its checkout snapshot. + let agent = OwnedAgent { + index: 0, + acp, + state, + model_capabilities: Some(AgentModelCapabilities { + config_options_raw: vec![], + available_models_raw: None, + thought_level_config_id: Some("effort".to_string()), + }), + desired_model: None, + model_overridden: false, + desired_effort: None, + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + }; + let mut pool = AgentPool::from_slots(vec![None]); // slot empty — worker is checked out + pool.notify_capabilities_discovered(&AgentModelCapabilities { + thought_level_config_id: Some("effort".to_string()), + config_options_raw: vec![], + available_models_raw: None, + }); + + // Live pick while worker is busy. + let result = pool.set_pool_effort("effort", "high"); + assert!(matches!(result, SetPoolEffortResult::Stored { .. })); + assert_eq!( + pool.desired_effort + .as_ref() + .map(|(id, v)| (id.as_str(), v.as_str())), + Some(("effort", "high")) + ); + + // Worker returns with old checkout snapshot (desired_effort = None). + pool.return_agent(agent); + + // V-3: sessions must be invalidated (None != Some("high") mismatch). + let returned = pool.agents_mut().iter().flatten().next().unwrap(); + assert!( + returned.state.sessions.is_empty(), + "V-3: busy worker's sessions must be invalidated on return when pool effort changed" + ); + + // Next try_claim syncs the new pool value onto the worker. + let claimed = pool.try_claim(Some(ch)).unwrap(); + assert_eq!( + claimed + .desired_effort + .as_ref() + .map(|(id, v)| (id.as_str(), v.as_str())), + Some(("effort", "high")), + "V-3: claimed agent must carry the updated pool effort value" + ); + } + + // ── Startup propagation without live pick ───────────────────────────────── + + /// Startup propagation: when no live pick/clear has ever been made + /// (`effort_ever_picked = false`), `return_agent` propagates a + /// startup-resolved `desired_effort` back to pool level. + /// + /// This seeds the pool on the first worker return so subsequent workers + /// that were idle at startup also pick up the persisted startup default + /// without needing a process restart. + #[tokio::test] + async fn test_startup_effort_propagates_to_pool_on_first_return_when_no_live_pick() { + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("spawn"); + // Worker was resolved via startup: desired_effort is Some from resolve_startup_effort. + let agent = OwnedAgent { + index: 0, + acp, + state: SessionState::default(), + model_capabilities: Some(AgentModelCapabilities { + config_options_raw: vec![], + available_models_raw: None, + thought_level_config_id: Some("effort".to_string()), + }), + desired_model: None, + model_overridden: false, + desired_effort: Some(("effort".to_string(), "medium".to_string())), + startup_effort: Some("medium".to_string()), + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + }; + let mut pool = AgentPool::from_slots(vec![None]); // slot empty + // No live pick has ever been made. + assert!(!pool.effort_ever_picked); + assert!(pool.desired_effort.is_none()); + + pool.return_agent(agent); + + // Pool should now carry the startup-resolved effort. + assert_eq!( + pool.desired_effort + .as_ref() + .map(|(id, v)| (id.as_str(), v.as_str())), + Some(("effort", "medium")), + "startup-resolved effort must propagate to pool when no live pick occurred" + ); + } + + // ── Generation-based commit/rollback ────────────────────────────────────── + + /// IMPORTANT-1: when the adapter rejects a pick (EffortApplicationResult::Failed) + /// and the agent's checkout generation is still current, resolve_effort_report + /// rolls back desired_effort to committed_effort so the failed candidate is never + /// recopied by try_claim or retried at the next session creation. + #[tokio::test] + async fn test_failure_rolls_back_desired_effort_to_committed() { + let obs = observer::ObserverHandle::in_process(); + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("spawn"); + let mut pool = AgentPool::from_slots(vec![Some(OwnedAgent { + index: 0, + acp, + state: SessionState::default(), + model_capabilities: None, + desired_model: None, + model_overridden: false, + desired_effort: None, + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + })]); + // Establish committed baseline: "medium" was previously confirmed. + pool.committed_effort = Some(("effort".to_string(), "medium".to_string())); + // Now the user picks "high" — generation 1 pending. + pool.set_pool_effort("effort", "high"); + let gen = pool.effort_generation; + // Check out the worker (desired_effort = Some("high"), gen = 1). + let agent = pool.try_claim(None).unwrap(); + assert_eq!(agent.desired_effort_gen, Some(gen)); + assert_eq!( + agent.desired_effort.as_ref().map(|(_, v)| v.as_str()), + Some("high") + ); + // resolve_effort_report (pre-prompt) must roll back to committed. + pool.resolve_effort_report( + EffortReport { + checkout_gen: Some(gen), + result: EffortApplicationResult::Failed, + nonce: None, + config_id: "effort".to_string(), + value: "high".to_string(), + }, + Some(&obs), + ); + // Return the agent; return_agent handles V-3/V-1. + pool.return_agent(agent); + assert_eq!( + pool.desired_effort.as_ref().map(|(_, v)| v.as_str()), + Some("medium"), + "failed pick must roll back desired_effort to committed_effort" + ); + // committed_effort unchanged. + assert_eq!( + pool.committed_effort.as_ref().map(|(_, v)| v.as_str()), + Some("medium"), + "committed_effort must not change on failure" + ); + } + + /// IMPORTANT-1: when the adapter accepts a pick (EffortApplicationResult::Applied) + /// and the generation is current, resolve_effort_report must commit + /// desired_effort to committed_effort. + #[tokio::test] + async fn test_applied_commits_desired_effort_to_committed() { + let obs = observer::ObserverHandle::in_process(); + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("spawn"); + let mut pool = AgentPool::from_slots(vec![Some(OwnedAgent { + index: 0, + acp, + state: SessionState::default(), + model_capabilities: None, + desired_model: None, + model_overridden: false, + desired_effort: None, + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + })]); + pool.set_pool_effort("effort", "high"); + let gen = pool.effort_generation; + let mut agent = pool.try_claim(None).unwrap(); + agent.last_effort_result = Some(EffortApplicationResult::Applied); + // F2: resolve_effort_report emits the terminal ack pre-prompt and commits + // desired_effort to committed_effort under pool authority. + pool.resolve_effort_report( + EffortReport { + checkout_gen: Some(gen), + result: EffortApplicationResult::Applied, + nonce: None, + config_id: "effort".to_string(), + value: "high".to_string(), + }, + Some(&obs), + ); + pool.return_agent(agent); + assert_eq!( + pool.committed_effort.as_ref().map(|(_, v)| v.as_str()), + Some("high"), + "applied pick must update committed_effort" + ); + } + + /// IMPORTANT-2: a stale generation must not touch pool state — desired_effort + /// must not be rolled back even though last_effort_result is Failed. + /// resolve_effort_report gates on checkout_gen == effort_generation, so it is + /// a no-op for stale results; return_agent handles only V-3 convergence. + #[tokio::test] + async fn test_stale_gen_failure_does_not_rollback_pending_pick() { + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("spawn"); + let acp2 = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("spawn2"); + let mut pool = AgentPool::from_slots(vec![ + Some(OwnedAgent { + index: 0, + acp, + state: SessionState::default(), + model_capabilities: None, + desired_model: None, + model_overridden: false, + desired_effort: None, + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + }), + Some(OwnedAgent { + index: 1, + acp: acp2, + state: SessionState::default(), + model_capabilities: None, + desired_model: None, + model_overridden: false, + desired_effort: None, + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test2".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + }), + ]); + pool.committed_effort = Some(("effort".to_string(), "low".to_string())); + // Pick "medium" — gen 1. + pool.set_pool_effort("effort", "medium"); + // Agent A checks out with gen 1. + let mut agent_a = pool.try_claim(None).unwrap(); + assert_eq!(agent_a.desired_effort_gen, Some(1)); + // User now picks "high" — gen 2 supersedes. + pool.set_pool_effort("effort", "high"); + assert_eq!(pool.effort_generation, 2); + // Agent A's adapter rejects the stale "medium" pick (gen 1 vs pool gen 2). + agent_a.last_effort_result = Some(EffortApplicationResult::Failed); + // Return agent A — stale gen; must NOT roll back desired_effort to "low". + pool.return_agent(agent_a); + assert_eq!( + pool.desired_effort.as_ref().map(|(_, v)| v.as_str()), + Some("high"), + "stale-gen failure must not roll back the current pending pick" + ); + assert_eq!( + pool.committed_effort.as_ref().map(|(_, v)| v.as_str()), + Some("low"), + "stale-gen failure must not touch committed_effort" + ); + } + + /// IMPORTANT-3: when a pending clear is confirmed (EffortApplicationResult::Cleared) + /// and the generation is current, resolve_effort_report must commit + /// committed_effort = None. + #[tokio::test] + async fn test_cleared_commits_none_to_committed_effort() { + let obs = observer::ObserverHandle::in_process(); + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("spawn"); + let mut pool = AgentPool::from_slots(vec![Some(OwnedAgent { + index: 0, + acp, + state: SessionState::default(), + model_capabilities: None, + desired_model: None, + model_overridden: false, + desired_effort: None, + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + })]); + // Prior committed state: "high" was accepted. + pool.committed_effort = Some(("effort".to_string(), "high".to_string())); + // User clears (Auto) — desired_effort = None, gen 1. + pool.clear_pool_effort(); + let gen = pool.effort_generation; + let mut agent = pool.try_claim(None).unwrap(); + assert_eq!(agent.desired_effort_gen, Some(gen)); + assert!( + agent.desired_effort.is_none(), + "clear checkout carries None" + ); + agent.last_effort_result = Some(EffortApplicationResult::Cleared); + // F2: resolve_effort_report emits the "cleared" ack and commits None. + pool.resolve_effort_report( + EffortReport { + checkout_gen: Some(gen), + result: EffortApplicationResult::Cleared, + nonce: None, + config_id: "effort".to_string(), + value: String::new(), + }, + Some(&obs), + ); + pool.return_agent(agent); + assert!( + pool.committed_effort.is_none(), + "cleared confirmation must set committed_effort to None" + ); + } + + // ── P2-2: committed_effort seeded on startup application ───────────────── + + /// P2-2 regression: startup effort is applied → committed baseline is set. + /// A subsequent rejected live pick must roll back to the startup value, not + /// to None. + #[tokio::test] + async fn test_startup_success_seeds_committed_effort_for_rollback() { + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("spawn"); + // Simulate a worker that resolved startup effort and applied it. + let agent = OwnedAgent { + index: 0, + acp, + state: SessionState::default(), + model_capabilities: Some(AgentModelCapabilities { + config_options_raw: vec![], + available_models_raw: None, + thought_level_config_id: Some("effort".to_string()), + }), + desired_model: None, + model_overridden: false, + desired_effort: Some(("effort".to_string(), "medium".to_string())), + startup_effort: Some("medium".to_string()), + desired_effort_gen: None, // startup agent: no live generation + pending_effort_nonce: None, + last_effort_result: Some(EffortApplicationResult::Applied), + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + }; + let mut pool = AgentPool::from_slots(vec![None]); + assert!(!pool.effort_ever_picked); + + pool.return_agent(agent); + + // V-1 propagation: pool.desired_effort = "medium". + assert_eq!( + pool.desired_effort.as_ref().map(|(_, v)| v.as_str()), + Some("medium"), + "startup effort must propagate to desired_effort" + ); + // P2-2: committed_effort must also be seeded from the successful startup + // application so a rollback from a later failed live pick lands here. + assert_eq!( + pool.committed_effort.as_ref().map(|(_, v)| v.as_str()), + Some("medium"), + "P2-2: successful startup application must seed committed_effort" + ); + + // Now simulate a live pick → failure → rollback must restore "medium", + // not roll back to None. + pool.set_pool_effort("effort", "high"); + let gen = pool.effort_generation; + let acp2 = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("spawn2"); + let agent2 = OwnedAgent { + index: 0, + acp: acp2, + state: SessionState::default(), + model_capabilities: None, + desired_model: None, + model_overridden: false, + desired_effort: Some(("effort".to_string(), "high".to_string())), + startup_effort: None, + desired_effort_gen: Some(gen), + pending_effort_nonce: None, + last_effort_result: Some(EffortApplicationResult::Failed), + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + }; + // Put agent2 into the pool so try_claim can hand it out (needed for index + // tracking), then manually call return_agent to simulate the failed return. + pool.agents_mut()[0] = Some(agent2); + let mut a = pool.try_claim(None).unwrap(); + a.last_effort_result = Some(EffortApplicationResult::Failed); + // F2: resolve_effort_report runs pre-prompt, rolling back desired_effort + // to committed ("medium"). return_agent then handles V-3. + let obs = observer::ObserverHandle::in_process(); + pool.resolve_effort_report( + EffortReport { + checkout_gen: Some(gen), + result: EffortApplicationResult::Failed, + nonce: None, + config_id: "effort".to_string(), + value: "high".to_string(), + }, + Some(&obs), + ); + pool.return_agent(a); + + assert_eq!( + pool.desired_effort.as_ref().map(|(_, v)| v.as_str()), + Some("medium"), + "P2-2: failed live pick must roll back to startup-seeded committed_effort, not None" + ); + } + + // ── P2-3: first-terminal-result-wins ───────────────────────────────────── + + /// P2-3 order-independence A: fail arrives first, then success. + /// + /// Two workers carry the same generation. Worker A (fail) returns first: + /// resolve_effort_report rolls back desired_effort to committed, emits failure + /// ack, marks gen resolved. return_agent then handles V-3. Worker B (Applied) + /// fires resolve_effort_report second: gen already resolved → no state change, + /// no second ack. return_agent handles V-3 for the discarded worker. + /// + /// Final state: desired=low, committed=low (matches the failure ack). + /// The discarded Applied worker (B) has value-mismatch after rollback: + /// its desired_effort("high") != pool.desired_effort("low") → V-3 + /// invalidates its sessions so it retries at the correct value. + #[tokio::test] + async fn test_p2_3_fail_then_success_emits_exactly_one_ack() { + let obs = observer::ObserverHandle::in_process(); + let mut pool = AgentPool::from_slots(vec![None, None]); + pool.committed_effort = Some(("effort".to_string(), "low".to_string())); + pool.set_pool_effort("effort", "high"); + let gen = pool.effort_generation; + + // Populate both slots so try_claim hands out two agents. + for slot in pool.agents_mut().iter_mut() { + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("spawn"); + *slot = Some(OwnedAgent { + index: 0, // will be reassigned by try_claim below + acp, + state: SessionState::default(), + model_capabilities: None, + desired_model: None, + model_overridden: false, + desired_effort: None, + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + }); + } + // Fix indices. + pool.agents_mut()[0].as_mut().unwrap().index = 0; + pool.agents_mut()[1].as_mut().unwrap().index = 1; + + let mut worker_a = pool.try_claim(None).unwrap(); + let mut worker_b = pool.try_claim(None).unwrap(); + assert_eq!(worker_a.desired_effort_gen, Some(gen)); + assert_eq!(worker_b.desired_effort_gen, Some(gen)); + + // Worker A: failed — first terminal result, wins the gate. + worker_a.last_effort_result = Some(EffortApplicationResult::Failed); + worker_a.pending_effort_nonce = Some("nonce-xyz".to_string()); + // F2: resolve_effort_report fires pre-prompt with the terminal ack. + pool.resolve_effort_report( + EffortReport { + checkout_gen: Some(gen), + result: EffortApplicationResult::Failed, + nonce: Some("nonce-xyz".to_string()), + config_id: "effort".to_string(), + value: "high".to_string(), + }, + Some(&obs), + ); + worker_a.acp.set_observer(Some(obs.clone()), worker_a.index); + pool.return_agent(worker_a); + + let events_after_a = obs.snapshot(); + assert_eq!( + events_after_a.len(), + 1, + "fail→ack expected after first resolve_effort_report" + ); + assert_eq!( + events_after_a[0].payload["status"].as_str().unwrap(), + "failure" + ); + assert_eq!(pool.last_acked_effort_gen, Some(gen)); + // Fail rolled back desired_effort to committed ("low"). + assert_eq!( + pool.desired_effort.as_ref().map(|(_, v)| v.as_str()), + Some("low"), + "fail rolls back desired_effort to committed" + ); + + // Worker B: success — arrives later, gen already resolved → no state change, no ack. + // worker_b.desired_effort is Some("high") (checked out before rollback). + worker_b.last_effort_result = Some(EffortApplicationResult::Applied); + worker_b.pending_effort_nonce = Some("nonce-xyz".to_string()); + // F2: resolve_effort_report is silenced by the first-terminal-wins gate. + pool.resolve_effort_report( + EffortReport { + checkout_gen: Some(gen), + result: EffortApplicationResult::Applied, + nonce: Some("nonce-xyz".to_string()), + config_id: "effort".to_string(), + value: "high".to_string(), + }, + Some(&obs), + ); + worker_b.acp.set_observer(Some(obs.clone()), worker_b.index); + pool.return_agent(worker_b); + + // Exactly one ack total. + let events_after_b = obs.snapshot(); + assert_eq!( + events_after_b.len(), + 1, + "P2-3: second same-gen return must NOT emit another ack" + ); + + // Pool state matches the failure ack: desired=low, committed=low. + assert_eq!( + pool.desired_effort.as_ref().map(|(_, v)| v.as_str()), + Some("low"), + "P2-3: desired_effort must stay at rollback value (low) after discarded Applied" + ); + assert_eq!( + pool.committed_effort.as_ref().map(|(_, v)| v.as_str()), + Some("low"), + "P2-3: committed_effort unchanged (low) — discarded Applied must not mutate it" + ); + } + + /// P2-3 order-independence B: success arrives first, then fail. + /// + /// Worker A (Applied) fires resolve_effort_report first — emits the ack and + /// commits. Worker B (Failed) fires resolve_effort_report second — gen already + /// resolved: no state change, no second ack. return_agent handles V-3 for both. + /// + /// Final state: desired=high, committed=high (matches the ok ack). + /// + /// Discarded Failed worker (B): its desired_effort("high") == pool.desired_effort("high"), + /// so the value comparison alone would skip V-3. But the worker's live session ran at + /// DEFAULT (adapter rejected), not at "high" — the session is stale. The + /// discarded-failed special case in V-3 force-invalidates it. + #[tokio::test] + async fn test_p2_3_success_then_fail_emits_exactly_one_ack() { + let obs = observer::ObserverHandle::in_process(); + let mut pool = AgentPool::from_slots(vec![None, None]); + pool.committed_effort = Some(("effort".to_string(), "low".to_string())); + pool.set_pool_effort("effort", "high"); + let gen = pool.effort_generation; + + for (idx, slot) in pool.agents_mut().iter_mut().enumerate() { + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("spawn"); + *slot = Some(OwnedAgent { + index: idx, + acp, + state: SessionState::default(), + model_capabilities: None, + desired_model: None, + model_overridden: false, + desired_effort: None, + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + }); + } + + let mut worker_a = pool.try_claim(None).unwrap(); + let mut worker_b = pool.try_claim(None).unwrap(); + assert_eq!(worker_a.desired_effort_gen, Some(gen)); + let worker_b_index = worker_b.index; + + // Worker A: success — first terminal result, wins the gate. + worker_a.last_effort_result = Some(EffortApplicationResult::Applied); + worker_a.pending_effort_nonce = Some("nonce-abc".to_string()); + // F2: resolve_effort_report fires pre-prompt with the terminal ack. + pool.resolve_effort_report( + EffortReport { + checkout_gen: Some(gen), + result: EffortApplicationResult::Applied, + nonce: Some("nonce-abc".to_string()), + config_id: "effort".to_string(), + value: "high".to_string(), + }, + Some(&obs), + ); + worker_a.acp.set_observer(Some(obs.clone()), worker_a.index); + pool.return_agent(worker_a); + + let events_after_a = obs.snapshot(); + assert_eq!( + events_after_a.len(), + 1, + "ok ack expected from first resolve_effort_report" + ); + assert_eq!(events_after_a[0].payload["status"].as_str().unwrap(), "ok"); + assert_eq!(pool.last_acked_effort_gen, Some(gen)); + + // Worker B: fail — arrives after, gen already resolved → no state change, no ack. + // worker_b.desired_effort is Some("high"); pool is now committed=high, desired=high. + // Value match would skip V-3, but the discarded-failed special case must invalidate. + worker_b.last_effort_result = Some(EffortApplicationResult::Failed); + worker_b.pending_effort_nonce = Some("nonce-abc".to_string()); + // Seed a dummy session so we can observe the invalidation. + let dummy_channel = Uuid::new_v4(); + worker_b + .state + .sessions + .insert(dummy_channel, "old-sess".into()); + // F2: resolve_effort_report is silenced by the first-terminal-wins gate. + pool.resolve_effort_report( + EffortReport { + checkout_gen: Some(gen), + result: EffortApplicationResult::Failed, + nonce: Some("nonce-abc".to_string()), + config_id: "effort".to_string(), + value: "high".to_string(), + }, + Some(&obs), + ); + worker_b.acp.set_observer(Some(obs.clone()), worker_b.index); + pool.return_agent(worker_b); + + // Exactly one ack total. + let events_after_b = obs.snapshot(); + assert_eq!( + events_after_b.len(), + 1, + "P2-3: second same-gen fail must NOT emit another ack" + ); + + // Pool state matches the ok ack: desired=high, committed=high. + assert_eq!( + pool.desired_effort.as_ref().map(|(_, v)| v.as_str()), + Some("high"), + "P2-3: desired_effort stays high after discarded Failed" + ); + assert_eq!( + pool.committed_effort.as_ref().map(|(_, v)| v.as_str()), + Some("high"), + "P2-3: committed_effort stays high (committed by worker_a Applied)" + ); + + // V-3 discarded-failed special case: worker B's slot must have been + // invalidated even though desired_effort values matched. + let returned_b = pool.agents_mut()[worker_b_index].as_ref().unwrap(); + assert!( + returned_b.state.sessions.is_empty(), + "P2-3: discarded Failed worker must have its sessions invalidated by V-3 (discarded_failed path)" + ); + } + + // ── Failed-worker ordering tests ───────────────────────────────────────── + + /// Return→report ordering (the failure race): + /// + /// Worker sends EffortReport::Failed pre-prompt but the main loop polls the + /// PromptResult first (biased select), so return_agent runs while + /// last_acked_effort_gen is still None. The discarded_failed predicate + /// requires last_acked_effort_gen == Some(g) — false here — and the value + /// comparison passes (high == high), so the failed session would survive + /// under the old code. The fix: any returned worker with + /// last_effort_result == Some(Failed) invalidates unconditionally. + /// + /// After fix: failed worker's sessions empty; then resolve_effort_report + /// acks failure and rolls back pool state; both orderings converge. + #[tokio::test] + async fn test_failed_worker_return_before_report_invalidates_session() { + let obs = observer::ObserverHandle::in_process(); + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("spawn"); + let mut pool = AgentPool::from_slots(vec![Some(OwnedAgent { + index: 0, + acp, + state: SessionState::default(), + model_capabilities: None, + desired_model: None, + model_overridden: false, + desired_effort: None, + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + })]); + pool.committed_effort = Some(("effort".into(), "low".into())); + pool.set_pool_effort("effort", "high"); + let gen = pool.effort_generation; + + let mut worker = pool.try_claim(None).unwrap(); + worker.last_effort_result = Some(EffortApplicationResult::Failed); + worker.pending_effort_nonce = Some("nonce-r".to_string()); + // Seed a session — this session ran at DEFAULT (adapter rejected "high"). + let ch = Uuid::new_v4(); + worker.state.sessions.insert(ch, "default-sess".into()); + + // RETURN FIRST (before resolve_effort_report): + // This is the race ordering — generation still unresolved. + assert_eq!(pool.last_acked_effort_gen, None); + pool.return_agent(worker); + + // Fix: returned Failed worker must have sessions invalidated unconditionally. + let returned = pool.agents_mut()[0].as_ref().unwrap(); + assert!( + returned.state.sessions.is_empty(), + "return-before-report: Failed worker session must be invalidated even before ack" + ); + + // THEN resolve_effort_report (as the main loop would, moments later): + pool.resolve_effort_report( + EffortReport { + checkout_gen: Some(gen), + result: EffortApplicationResult::Failed, + nonce: Some("nonce-r".to_string()), + config_id: "effort".into(), + value: "high".into(), + }, + Some(&obs), + ); + + // One terminal failure ack. + let events = obs.snapshot(); + assert_eq!(events.len(), 1, "exactly one failure ack"); + assert_eq!(events[0].payload["status"].as_str().unwrap(), "failure"); + assert_eq!(events[0].payload["nonce"].as_str().unwrap(), "nonce-r"); + + // Pool state: desired rolled back to committed ("low"), last_acked resolved. + assert_eq!( + pool.desired_effort.as_ref().map(|(_, v)| v.as_str()), + Some("low"), + "return-before-report: desired_effort rolled back to committed" + ); + assert_eq!(pool.last_acked_effort_gen, Some(gen)); + } + + /// Report→return ordering (the normal path — should also converge): + /// + /// resolve_effort_report fires first (pre-prompt, as intended). The main loop + /// processes it, emits the failure ack, rolls back pool state. Then + /// return_agent runs. The session must still be invalidated: the failed + /// application means the live session ran at DEFAULT regardless of timing. + /// + /// Both orderings must end with: failed worker's sessions empty, exactly one + /// terminal failure ack, pool state matching that ack. + #[tokio::test] + async fn test_failed_worker_report_before_return_invalidates_session() { + let obs = observer::ObserverHandle::in_process(); + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("spawn"); + let mut pool = AgentPool::from_slots(vec![Some(OwnedAgent { + index: 0, + acp, + state: SessionState::default(), + model_capabilities: None, + desired_model: None, + model_overridden: false, + desired_effort: None, + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + })]); + pool.committed_effort = Some(("effort".into(), "low".into())); + pool.set_pool_effort("effort", "high"); + let gen = pool.effort_generation; + + let mut worker = pool.try_claim(None).unwrap(); + worker.last_effort_result = Some(EffortApplicationResult::Failed); + worker.pending_effort_nonce = Some("nonce-rp".to_string()); + let ch = Uuid::new_v4(); + worker.state.sessions.insert(ch, "default-sess".into()); + + // REPORT FIRST (normal pre-prompt path): + pool.resolve_effort_report( + EffortReport { + checkout_gen: Some(gen), + result: EffortApplicationResult::Failed, + nonce: Some("nonce-rp".to_string()), + config_id: "effort".into(), + value: "high".into(), + }, + Some(&obs), + ); + + // One terminal failure ack. + let events_pre = obs.snapshot(); + assert_eq!(events_pre.len(), 1, "exactly one failure ack pre-return"); + assert_eq!(events_pre[0].payload["status"].as_str().unwrap(), "failure"); + assert_eq!(pool.last_acked_effort_gen, Some(gen)); + + // THEN return_agent (after the turn finishes): + pool.return_agent(worker); + + // No second ack. + let events_post = obs.snapshot(); + assert_eq!( + events_post.len(), + 1, + "report-before-return: no second ack after return_agent" + ); + + // Failed worker's session invalidated. + let returned = pool.agents_mut()[0].as_ref().unwrap(); + assert!( + returned.state.sessions.is_empty(), + "report-before-return: Failed worker session must be invalidated" + ); + + // Pool state: desired=low (rolled back), committed=low. + assert_eq!( + pool.desired_effort.as_ref().map(|(_, v)| v.as_str()), + Some("low"), + "report-before-return: desired rolled back to committed" + ); + assert_eq!( + pool.committed_effort.as_ref().map(|(_, v)| v.as_str()), + Some("low") + ); + } + + // ── P3: stale capability cache cleared on model swap ───────────────────── + + /// P3: when a returning agent has populated capabilities but no + /// thought_level configId (model swapped to non-effort), the pool-level + /// effort_capabilities cache is cleared so stale options are not used for + /// validation on the next pick. + #[tokio::test] + async fn test_capability_cache_cleared_when_model_loses_thought_level() { + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("spawn"); + let mut pool = AgentPool::from_slots(vec![Some(OwnedAgent { + index: 0, + acp, + state: SessionState::default(), + model_capabilities: None, + desired_model: None, + model_overridden: false, + desired_effort: None, + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + })]); + // Establish a known capabilities state (effort-capable model). + pool.notify_capabilities_discovered(&AgentModelCapabilities { + thought_level_config_id: Some("effort".to_string()), + config_options_raw: vec![serde_json::json!({"id": "effort", "options": [{"value": "low"}, {"value": "high"}]})], + available_models_raw: None, + }); + assert!(pool.effort_capabilities.config_id.is_some()); + assert!(pool.capabilities_ever_discovered); + + // Worker returns after a model swap — no thought_level in new model. + let mut agent = pool.try_claim(None).unwrap(); + agent.model_capabilities = Some(AgentModelCapabilities { + thought_level_config_id: None, // model swapped to non-effort + config_options_raw: vec![], + available_models_raw: None, + }); + pool.return_agent(agent); + + // P3: cache must be cleared. + assert!( + pool.effort_capabilities.config_id.is_none(), + "P3: effort_capabilities must be cleared when model loses thought_level" + ); + assert!( + pool.effort_capabilities.valid_values.is_empty(), + "P3: valid_values must be cleared" + ); + // capabilities_ever_discovered stays true (case C remains active). + assert!( + pool.capabilities_ever_discovered, + "capabilities_ever_discovered must stay true after cache clear" + ); + } + + // ── F1: stale-generation force-invalidation ─────────────────────────────── + + /// F1 A: rapid high→medium→high picks on one worker. + /// + /// Worker checks out at gen g1 ("high"), user picks "medium" (g2) then "high" + /// again (g3) while the worker runs. Worker returns stale (g1) with + /// Applied and value "high" — value matches pool.desired_effort("high") so + /// V-3 alone would skip invalidation. But g3 is unresolved: F1 must + /// force-invalidate so the next claim creates a fresh session and produces + /// the terminal ack for g3's nonce. + #[tokio::test] + async fn test_f1_stale_gen_force_invalidates_when_current_gen_unresolved() { + let obs = observer::ObserverHandle::in_process(); + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("spawn"); + let mut pool = AgentPool::from_slots(vec![Some(OwnedAgent { + index: 0, + acp, + state: SessionState::default(), + model_capabilities: None, + desired_model: None, + model_overridden: false, + desired_effort: None, + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + })]); + + // Pick "high" — gen 1. + pool.set_pool_effort("effort", "high"); + let gen1 = pool.effort_generation; + let mut worker = pool.try_claim(None).unwrap(); + assert_eq!(worker.desired_effort_gen, Some(gen1)); + + // While worker runs: user picks "medium" (gen 2) then "high" again (gen 3). + pool.set_pool_effort("effort", "medium"); + pool.pending_effort_nonce = Some("nonce-g3".into()); + pool.set_pool_effort("effort", "high"); + let gen3 = pool.effort_generation; + assert_eq!(gen3, 3); + // g3 is unresolved (no ack yet). + assert_eq!(pool.last_acked_effort_gen, None); + + // Seed a session on the worker so we can verify it gets invalidated. + let ch = Uuid::new_v4(); + worker.state.sessions.insert(ch, "stale-sess".into()); + + // Worker returns stale (gen 1), Applied, value "high" — matches pool value. + worker.last_effort_result = Some(EffortApplicationResult::Applied); + pool.return_agent(worker); + + // F1: stale gen while current gen is unresolved → force-invalidated. + // Session must be cleared so the next claim starts fresh and produces the g3 ack. + let returned = pool.agents_mut()[0].as_ref().unwrap(); + assert!( + returned.state.sessions.is_empty(), + "F1: stale-gen return with value match must invalidate sessions when current gen is unresolved" + ); + + // Convergence: the next claim gets the newest-gen worker and the newest nonce. + let mut new_worker = pool.try_claim(None).unwrap(); + assert_eq!( + new_worker.desired_effort_gen, + Some(gen3), + "F1: next claim must be at gen3" + ); + assert_eq!( + new_worker.pending_effort_nonce.as_deref(), + Some("nonce-g3"), + "F1: next claim must carry the newest nonce" + ); + + // Simulate the new session applying the effort successfully. + new_worker.last_effort_result = Some(EffortApplicationResult::Applied); + pool.resolve_effort_report( + EffortReport { + checkout_gen: Some(gen3), + result: EffortApplicationResult::Applied, + nonce: Some("nonce-g3".into()), + config_id: "effort".into(), + value: "high".into(), + }, + Some(&obs), + ); + + // Exactly one terminal ack, carrying the newest nonce. + let events = obs.snapshot(); + assert_eq!(events.len(), 1, "F1: exactly one terminal ack for gen3"); + assert_eq!(events[0].payload["status"].as_str().unwrap(), "ok"); + assert_eq!( + events[0].payload["nonce"].as_str().unwrap(), + "nonce-g3", + "F1: ack must carry the newest nonce" + ); + // Persistence-eligible: desired=high, committed=high. + assert_eq!( + pool.desired_effort.as_ref().map(|(_, v)| v.as_str()), + Some("high") + ); + assert_eq!( + pool.committed_effort.as_ref().map(|(_, v)| v.as_str()), + Some("high") + ); + assert_eq!(pool.last_acked_effort_gen, Some(gen3)); + } + + /// F1 B: repeated clears — None==None hole. + /// + /// Pool is at gen g1 (clear: desired=None). Worker checks out at g1, + /// user clears again (g2) while the worker runs. Worker returns stale (g1) + /// with Cleared and desired_effort=None — None==None value match would skip + /// V-3. But g2 is unresolved: F1 must force-invalidate. + #[tokio::test] + async fn test_f1_repeated_clear_none_none_value_match_invalidates() { + let obs = observer::ObserverHandle::in_process(); + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("spawn"); + let mut pool = AgentPool::from_slots(vec![Some(OwnedAgent { + index: 0, + acp, + state: SessionState::default(), + model_capabilities: None, + desired_model: None, + model_overridden: false, + desired_effort: None, + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + })]); + + // First clear — gen 1. + pool.clear_pool_effort(); + let gen1 = pool.effort_generation; + let mut worker = pool.try_claim(None).unwrap(); + assert_eq!(worker.desired_effort_gen, Some(gen1)); + + // User clears again while worker is running — gen 2, still unresolved. + pool.pending_effort_nonce = Some("nonce-g2".into()); + pool.clear_pool_effort(); + let gen2 = pool.effort_generation; + assert_eq!(gen2, 2); + assert_eq!(pool.last_acked_effort_gen, None); + + // Seed a session so we can observe invalidation. + let ch = Uuid::new_v4(); + worker.state.sessions.insert(ch, "stale-sess".into()); + + // Worker returns stale (gen 1), Cleared, desired_effort=None — None==None match. + worker.last_effort_result = Some(EffortApplicationResult::Cleared); + pool.return_agent(worker); + + // F1: stale gen while current gen unresolved → force-invalidated. + let returned = pool.agents_mut()[0].as_ref().unwrap(); + assert!( + returned.state.sessions.is_empty(), + "F1: repeated-clear None==None match must invalidate sessions when current gen is unresolved" + ); + + // Convergence: next claim at gen2 with the newest nonce. + let mut new_worker = pool.try_claim(None).unwrap(); + assert_eq!(new_worker.desired_effort_gen, Some(gen2)); + assert_eq!(new_worker.pending_effort_nonce.as_deref(), Some("nonce-g2")); + + // Simulate the new session confirming the clear. + new_worker.last_effort_result = Some(EffortApplicationResult::Cleared); + pool.resolve_effort_report( + EffortReport { + checkout_gen: Some(gen2), + result: EffortApplicationResult::Cleared, + nonce: Some("nonce-g2".into()), + config_id: "effort".into(), + value: "".into(), + }, + Some(&obs), + ); + + // Exactly one terminal cleared ack with the newest nonce. + let events = obs.snapshot(); + assert_eq!(events.len(), 1, "F1 B: exactly one terminal ack for gen2"); + assert_eq!(events[0].payload["status"].as_str().unwrap(), "cleared"); + assert_eq!(events[0].payload["nonce"].as_str().unwrap(), "nonce-g2"); + assert_eq!(pool.last_acked_effort_gen, Some(gen2)); + // Persistence-eligible: desired=None, committed=None. + assert!(pool.desired_effort.is_none()); + assert!(pool.committed_effort.is_none()); + } + + /// F1 C: stale-gen Failed with value match — session ran at DEFAULT. + /// + /// Pool picks "high" (g1). Worker A checks out at g1. Pool then picks "high" + /// again (g2, same value, unresolved). Worker A returns stale (g1) with + /// Failed and desired_effort="high" — value matches pool. Its session ran at + /// DEFAULT (adapter rejected g1 pick). Without F1: the session survives, so + /// the pool silently serves a DEFAULT-effort session for g2. + /// With F1: stale gen + g2 unresolved → force-invalidate. + #[tokio::test] + async fn test_f1_stale_gen_failed_session_invalidated_when_current_gen_unresolved() { + let acp = AcpClient::spawn( + "bash", + &["-c".to_string(), "sleep 10".to_string()], + &[], + false, + ) + .await + .expect("spawn"); + let mut pool = AgentPool::from_slots(vec![Some(OwnedAgent { + index: 0, + acp, + state: SessionState::default(), + model_capabilities: None, + desired_model: None, + model_overridden: false, + desired_effort: None, + startup_effort: None, + desired_effort_gen: None, + pending_effort_nonce: None, + last_effort_result: None, + agent_name: "test".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + })]); + + // Gen 1: pick "high". + pool.set_pool_effort("effort", "high"); + let gen1 = pool.effort_generation; + let mut worker = pool.try_claim(None).unwrap(); + assert_eq!(worker.desired_effort_gen, Some(gen1)); + + // Gen 2: pick "high" again (same value, different generation, unresolved). + pool.set_pool_effort("effort", "high"); + let gen2 = pool.effort_generation; + assert_eq!(gen2, 2); + assert_eq!(pool.last_acked_effort_gen, None); + + // Seed a session — the session ran at DEFAULT (adapter rejected g1 pick). + let ch = Uuid::new_v4(); + worker + .state + .sessions + .insert(ch, "default-effort-sess".into()); + + // Worker returns stale (gen 1), Failed, value "high" — matches pool value. + worker.last_effort_result = Some(EffortApplicationResult::Failed); + pool.return_agent(worker); + + // F1: stale gen while current gen is unresolved → force-invalidate. + // The DEFAULT-effort session must be cleared. + let returned = pool.agents_mut()[0].as_ref().unwrap(); + assert!( + returned.state.sessions.is_empty(), + "F1: stale-gen Failed with value match must invalidate sessions when current gen is unresolved" + ); + } +} diff --git a/desktop/src-tauri/src/commands/agent_config.rs b/desktop/src-tauri/src/commands/agent_config.rs index 2dc0ba0d69..98b726bc4c 100644 --- a/desktop/src-tauri/src/commands/agent_config.rs +++ b/desktop/src-tauri/src/commands/agent_config.rs @@ -13,9 +13,9 @@ use crate::{ }, }, current_instance_id, is_reserved_env_key, is_safe_to_reveal, is_well_formed_env_key, - known_acp_runtime, load_managed_agents, load_personas, save_managed_agents, - sync_managed_agent_processes, AgentDefinition, GlobalAgentConfig, KnownAcpRuntime, - ManagedAgentRecord, ManagedAgentRuntimeKey, MAX_ENV_VALUE_BYTES, + known_acp_runtime, load_managed_agents, load_personas, resolve_effective_agent_env, + save_managed_agents, sync_managed_agent_processes, AgentDefinition, GlobalAgentConfig, + KnownAcpRuntime, ManagedAgentRecord, ManagedAgentRuntimeKey, MAX_ENV_VALUE_BYTES, }, }; @@ -121,6 +121,7 @@ fn resolve_config_surface( runtime_meta: Option<&KnownAcpRuntime>, session_cache: Option<&SessionConfigCache>, global: &GlobalAgentConfig, + claude_config_dir: Option, ) -> RuntimeConfigSurface { // Linked instances are definition-authoritative: clear stale materialized // model/provider/prompt so they can never masquerade as BuzzExplicit and @@ -138,7 +139,13 @@ fn resolve_config_surface( global, ); - read_config_surface(&record, runtime_meta, session_cache, &tiers) + read_config_surface( + &record, + runtime_meta, + session_cache, + &tiers, + claude_config_dir.as_deref(), + ) } /// Get the file-layer config for a runtime — used by the Create/Edit/Persona @@ -288,12 +295,36 @@ pub async fn get_agent_config_surface( let session_cache = state.get_session_cache(&runtime_key); let global = crate::managed_agents::load_global_agent_config(&app).unwrap_or_default(); + // #3493: for claude agents, resolve the settings.json and .claude.json paths + // from the agent's effective CLAUDE_CONFIG_DIR env var (if set), falling + // back to ~/.claude/ and ~/.claude.json. We never provision this dir + // ourselves — we only respect what the user configured. + // + // Use resolve_effective_agent_env so the lookup covers all tiers (baked + // floor → definition → global → persona → record) and cannot diverge from + // what the spawned process actually sees. + let claude_config_dir: Option = if runtime_meta + .is_some_and(|m| m.id == "claude") + { + let effective_env = resolve_effective_agent_env(&record, &personas, runtime_meta, &global); + // Treat empty or blank CLAUDE_CONFIG_DIR as unset, matching Claude's + // `CLAUDE_CONFIG_DIR || homedir()` resolver semantics. + effective_env + .env + .get("CLAUDE_CONFIG_DIR") + .filter(|v| !v.trim().is_empty()) + .map(std::path::PathBuf::from) + } else { + None + }; + Ok(resolve_config_surface( record, &personas, runtime_meta, session_cache.as_ref(), &global, + claude_config_dir, )) } @@ -503,6 +534,37 @@ fn parse_models(raw: Option<&serde_json::Value>) -> (Vec, Option< (models, current_model) } +/// Persist the canonical effort level for a managed agent after a positive ACP ack. +/// +/// B5: called by the TypeScript observer when `session/set_config_option` for +/// the "effort" config option receives a positive acknowledgement. The record +/// is updated in-place and persisted. At the next spawn the Desktop injects +/// `BUZZ_ACP_EFFORT_LEVEL` so the harness applies this value via +/// `session/set_config_option` at session creation, pairing with the +/// adapter-advertised `thought_level` configId discovered from capabilities. +/// +/// `effort_level` is the acknowledged value. Pass `None` to clear the +/// canonical effort (reverts to adapter default on next spawn). +#[tauri::command] +pub fn persist_agent_effort_level( + pubkey: String, + effort_level: Option, + app: AppHandle, + state: State<'_, AppState>, +) -> Result<(), String> { + let _store_guard = state + .managed_agents_store_lock + .lock() + .map_err(|e| e.to_string())?; + let mut records = load_managed_agents(&app)?; + let record = records + .iter_mut() + .find(|r| r.pubkey == pubkey) + .ok_or_else(|| format!("agent {pubkey} not found"))?; + record.effort_level = effort_level; + save_managed_agents(&app, &records) +} + #[cfg(test)] #[path = "agent_config_tests.rs"] mod tests; diff --git a/desktop/src-tauri/src/commands/agent_config_tests.rs b/desktop/src-tauri/src/commands/agent_config_tests.rs index 5519153578..7e9b031409 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, + effort_level: None, agent_command_override: None, persona_source_version: None, provider: None, @@ -181,6 +182,7 @@ fn linked_stale_record_model_never_outranks_persona_model() { Some(goose_runtime()), None, &Default::default(), + None, ); let model = surface.normalized.model.as_ref().expect("model resolved"); @@ -205,7 +207,14 @@ fn linked_blank_definition_model_falls_through_to_global_default() { ..Default::default() }; - let surface = resolve_config_surface(record, &personas, Some(goose_runtime()), None, &global); + let surface = resolve_config_surface( + record, + &personas, + Some(goose_runtime()), + None, + &global, + None, + ); let model = surface.normalized.model.as_ref().expect("model resolved"); assert_eq!(model.value.as_deref(), Some("global-model")); @@ -228,6 +237,7 @@ fn definition_less_explicit_record_model_keeps_buzz_explicit_origin() { Some(goose_runtime()), None, &Default::default(), + None, ); let model = surface.normalized.model.as_ref().expect("model resolved"); @@ -255,6 +265,7 @@ fn pending_pick_keeps_explicit_x_and_does_not_surface_live_y() { Some(goose_runtime()), Some(&cache), &Default::default(), + None, ); let model = surface.normalized.model.expect("model resolved"); @@ -283,6 +294,7 @@ fn genuine_explicit_live_switch_renders_y_over_x_buzz_explicit_secondary() { Some(goose_runtime()), Some(&cache), &Default::default(), + None, ); let model = surface.normalized.model.expect("model resolved"); @@ -318,6 +330,7 @@ fn genuine_explicit_live_switch_to_same_model_yields_clean_field() { Some(goose_runtime()), Some(&cache), &Default::default(), + None, ) }); let model = surface.normalized.model.expect("model resolved"); @@ -346,6 +359,7 @@ fn persona_linked_live_switch_keeps_persona_default_secondary() { Some(goose_runtime()), Some(&cache), &Default::default(), + None, ); let model = surface.normalized.model.expect("model resolved"); @@ -381,6 +395,7 @@ fn global_default_live_switch_renders_global_model_as_secondary_global_default() Some(goose_runtime()), Some(&cache), &global, + None, ); let model = surface.normalized.model.expect("model resolved"); diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index dd61fc9398..0ced25a05c 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -913,6 +913,7 @@ pub async fn create_managed_agent( } else { relay_mesh.clone() }, + effort_level: None, }; records.push(record); @@ -1331,16 +1332,10 @@ pub async fn delete_managed_agent( return Err(format!("agent {pubkey} not found")); } save_managed_agents(&app, &records)?; - // Remove the agent's nsec from the keyring after the record is gone. crate::managed_agents::delete_agent_key(&pubkey); - // Tombstone-after-validation: only reached past the deployed-remote - // guard above and a confirmed removal — never orphan a live remote - // deployment's relay record. Inside the lock, before the block closes - // (no .await here). Every agent published, so every delete tombstones. + // Tombstone after confirmed removal (inside lock; every published agent tombstones). tombstone_managed_agent_pending(&app, &state, &pubkey); - // NIP-IA: archive the deleted agent's identity on the relay so it - // stops appearing in member pickers and autocomplete. Same - // best-effort, inside-the-lock contract as the tombstone above. + // NIP-IA: archive deleted agent identity so it stops appearing in pickers. archive_managed_agent_pending(&app, &state, &pubkey); } try_regenerate_nest(&app); diff --git a/desktop/src-tauri/src/commands/agents_deploy.rs b/desktop/src-tauri/src/commands/agents_deploy.rs index 47ee5f92d4..59573ee404 100644 --- a/desktop/src-tauri/src/commands/agents_deploy.rs +++ b/desktop/src-tauri/src/commands/agents_deploy.rs @@ -83,7 +83,25 @@ pub(super) fn build_launch_block( policy_env.insert("BUZZ_ACP_SYSTEM_PROMPT".into(), value.to_string()); } if let Some(value) = effective_model { - policy_env.insert("BUZZ_ACP_MODEL".into(), value.to_string()); + // B2: remote env-authority model key. Claude's startup model authority + // is ANTHROPIC_MODEL (same as the local A1 path — the harness reads it + // first and skips the BUZZ_ACP_MODEL catalog-switch path that would + // introduce a second startup authority). All other runtimes use + // BUZZ_ACP_MODEL, which the harness reads into desired_model at spawn. + let is_claude = runtime.map(|r| r.id == "claude").unwrap_or(false); + let model_key = if is_claude { + "ANTHROPIC_MODEL" + } else { + "BUZZ_ACP_MODEL" + }; + policy_env.insert(model_key.into(), value.to_string()); + } + // I-4: remote parity for persisted startup effort. Mirrors the local spawn + // path in runtime.rs. The harness reads BUZZ_ACP_EFFORT_LEVEL into + // PoolStartup.startup_effort and applies it at first session creation via + // resolve_startup_effort(). + if let Some(ref value) = record.effort_level { + policy_env.insert("BUZZ_ACP_EFFORT_LEVEL".into(), value.clone()); } if let Some(value) = record.idle_timeout_seconds { policy_env.insert("BUZZ_ACP_IDLE_TIMEOUT".into(), value.to_string()); @@ -284,13 +302,80 @@ mod tests { assert_eq!(launch["policy_env"]["BUZZ_ACP_SESSION_TITLE"], "Agent Name"); assert_eq!(launch["policy_env"]["BUZZ_ACP_DISPLAY_NAME"], "Agent Name"); assert_eq!(launch["policy_env"]["BUZZ_ACP_SYSTEM_PROMPT"], "prompt"); + // goose runtime: model goes via BUZZ_ACP_MODEL (non-claude path). assert_eq!(launch["policy_env"]["BUZZ_ACP_MODEL"], "model"); + assert!( + launch["policy_env"]["ANTHROPIC_MODEL"].is_null(), + "goose must NOT receive ANTHROPIC_MODEL" + ); assert_eq!(launch["policy_env"]["BUZZ_ACP_IDLE_TIMEOUT"], "17"); assert_eq!(launch["policy_env"]["BUZZ_ACP_MAX_TURN_DURATION"], "23"); assert_eq!(launch["policy_env"]["BUZZ_ACP_AGENTS"], "4"); assert_eq!(launch["owner_pubkey"], "owner-hex"); } + #[test] + fn launch_block_claude_runtime_uses_anthropic_model_not_buzz_acp_model() { + // B2: remote claude deploys must send ANTHROPIC_MODEL, not BUZZ_ACP_MODEL, + // so the remote harness has a single startup model authority matching A1. + let record = record(); + let descriptor = EffectiveHarnessDescriptor { + command: "claude".into(), + args: vec![], + env: BTreeMap::new(), + }; + let teams: Vec = vec![]; + let launch = build_launch_block( + &record, + &descriptor, + &teams, + None, + Some("claude-opus-4"), + "owner-hex", + ); + assert_eq!( + launch["policy_env"]["ANTHROPIC_MODEL"], "claude-opus-4", + "claude remote must receive ANTHROPIC_MODEL" + ); + assert!( + launch["policy_env"]["BUZZ_ACP_MODEL"].is_null(), + "claude remote must NOT receive BUZZ_ACP_MODEL" + ); + } + + #[test] + fn launch_block_claude_runtime_injects_effort_level_when_set() { + // I-4: remote parity — record.effort_level → BUZZ_ACP_EFFORT_LEVEL in policy_env. + let mut record = record(); + record.effort_level = Some("high".to_string()); + let descriptor = EffectiveHarnessDescriptor { + command: "claude".into(), + args: vec![], + env: BTreeMap::new(), + }; + let launch = build_launch_block(&record, &descriptor, &[], None, None, "owner-hex"); + assert_eq!( + launch["policy_env"]["BUZZ_ACP_EFFORT_LEVEL"], "high", + "claude remote must receive BUZZ_ACP_EFFORT_LEVEL when effort_level is set" + ); + } + + #[test] + fn launch_block_does_not_inject_effort_level_when_absent() { + // I-4: no BUZZ_ACP_EFFORT_LEVEL in policy_env when record.effort_level is None. + let record = record(); // effort_level is None by default + let descriptor = EffectiveHarnessDescriptor { + command: "claude".into(), + args: vec![], + env: BTreeMap::new(), + }; + let launch = build_launch_block(&record, &descriptor, &[], None, None, "owner-hex"); + assert!( + launch["policy_env"]["BUZZ_ACP_EFFORT_LEVEL"].is_null(), + "policy_env must NOT contain BUZZ_ACP_EFFORT_LEVEL when effort_level is None" + ); + } + /// OpenClaw descriptor: `launch.policy_env["BUZZ_ACP_AGENTS"]` must be "5" /// even when the record's requested parallelism is 10. This is the direct /// `launch.policy_env` seam test — the executable contract for remote providers. diff --git a/desktop/src-tauri/src/commands/agents_tests.rs b/desktop/src-tauri/src/commands/agents_tests.rs index 54a03e2bab..ec583e8860 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, + effort_level: 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/delete_cascade_tests.rs b/desktop/src-tauri/src/commands/personas/delete_cascade_tests.rs index 8ff7cfbd9b..e8a47d1694 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, + effort_level: 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 1005a83432..181fa94c1a 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, + effort_level: None, } } diff --git a/desktop/src-tauri/src/commands/personas/snapshot/fidelity_tests.rs b/desktop/src-tauri/src/commands/personas/snapshot/fidelity_tests.rs index b769d74d7b..fe2ca43234 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, + effort_level: None, } } diff --git a/desktop/src-tauri/src/commands/personas/snapshot/import.rs b/desktop/src-tauri/src/commands/personas/snapshot/import.rs index d7f0323304..9089fd0718 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot/import.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot/import.rs @@ -652,6 +652,7 @@ pub async fn confirm_agent_snapshot_import( definition_respond_to_allowlist: minted.respond_to_allowlist.clone(), definition_parallelism: minted_parallelism, relay_mesh: None, + effort_level: None, runtime: snapshot.definition.runtime.clone(), name_pool: snapshot.definition.name_pool.clone(), }; diff --git a/desktop/src-tauri/src/commands/personas/snapshot/tests.rs b/desktop/src-tauri/src/commands/personas/snapshot/tests.rs index c453b09a9d..483e14e19c 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, + effort_level: None, } } diff --git a/desktop/src-tauri/src/commands/personas/update/name_propagation_tests.rs b/desktop/src-tauri/src/commands/personas/update/name_propagation_tests.rs index c60215ae4d..4d891a1caf 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, + effort_level: None, } } diff --git a/desktop/src-tauri/src/commands/team_snapshot.rs b/desktop/src-tauri/src/commands/team_snapshot.rs index 97cd11933d..db2c214192 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, + effort_level: None, runtime: member.definition.runtime.clone(), name_pool: member.definition.name_pool.clone(), }; diff --git a/desktop/src-tauri/src/commands/team_snapshot/tests.rs b/desktop/src-tauri/src/commands/team_snapshot/tests.rs index c9a6d8812a..d3427b4095 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, + effort_level: None, runtime: None, name_pool: vec![], }; diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 4f935631b6..cee9c06c5b 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -780,6 +780,7 @@ pub fn run() { get_baked_build_env_keys, get_baked_build_env, put_agent_session_config, + persist_agent_effort_level, get_global_agent_config, set_global_agent_config, mesh_start_node, diff --git a/desktop/src-tauri/src/managed_agents/agent_events.rs b/desktop/src-tauri/src/managed_agents/agent_events.rs index 4a7b80079d..6e4e4c2b25 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, + effort_level: 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 8508c27073..69bce06702 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, + effort_level: 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 b4492418e5..a2d6893d55 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, + effort_level: None, } } diff --git a/desktop/src-tauri/src/managed_agents/claude_config/mod.rs b/desktop/src-tauri/src/managed_agents/claude_config/mod.rs new file mode 100644 index 0000000000..2a728003dd --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/claude_config/mod.rs @@ -0,0 +1,28 @@ +//! Claude Code agent spawn-time env helpers. +//! +//! A1 contract: `ANTHROPIC_MODEL` is the single startup model authority for +//! local Claude Code agents. `BUZZ_ACP_MODEL` is removed from the spawned +//! env so the harness never sees two model authorities simultaneously. + +/// Apply the A1 model authority: inject `ANTHROPIC_MODEL` from `effective_model` +/// (or remove it if `None`) and strip `BUZZ_ACP_MODEL` from the spawned env. +/// +/// Must be called after `descriptor.env` is written so that any user-supplied +/// `ANTHROPIC_MODEL` is overridden by the Buzz-resolved value. +pub fn apply_claude_model_env(command: &mut std::process::Command, effective_model: Option<&str>) { + // Remove BUZZ_ACP_MODEL — the catalog-switch path is for live ACP switches + // only; at spawn time ANTHROPIC_MODEL is the sole authority. + command.env_remove("BUZZ_ACP_MODEL"); + match effective_model { + Some(m) => { + command.env("ANTHROPIC_MODEL", m); + } + None => { + command.env_remove("ANTHROPIC_MODEL"); + } + } +} + +#[cfg(test)] +#[path = "tests.rs"] +mod tests; diff --git a/desktop/src-tauri/src/managed_agents/claude_config/tests.rs b/desktop/src-tauri/src/managed_agents/claude_config/tests.rs new file mode 100644 index 0000000000..326fb8c34e --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/claude_config/tests.rs @@ -0,0 +1,55 @@ +use super::apply_claude_model_env; + +/// A1: BUZZ_ACP_MODEL must NOT be present in the spawned-child env after +/// `apply_claude_model_env`, even if it was set before (dual-authority defect). +/// ANTHROPIC_MODEL must be set to the resolved model. +#[test] +fn a1_buzz_acp_model_absent_anthropic_model_present_after_env_apply() { + let mut cmd = std::process::Command::new("true"); + // Simulate descriptor.env writing BUZZ_ACP_MODEL (the pre-A1 path). + cmd.env("BUZZ_ACP_MODEL", "claude-opus-4"); + apply_claude_model_env(&mut cmd, Some("claude-opus-4")); + + let env_map: std::collections::HashMap<_, _> = cmd.get_envs().collect(); + + // BUZZ_ACP_MODEL must be removed. Command::get_envs returns None for + // explicitly-removed keys. + let buzz_acp = env_map.get(std::ffi::OsStr::new("BUZZ_ACP_MODEL")); + assert!( + buzz_acp.is_none() || buzz_acp.unwrap().is_none(), + "BUZZ_ACP_MODEL must be absent (or explicitly removed) after A1 policy" + ); + + // ANTHROPIC_MODEL must be set to the resolved model value. + let anthropic = env_map.get(std::ffi::OsStr::new("ANTHROPIC_MODEL")); + assert!(anthropic.is_some(), "ANTHROPIC_MODEL must be present"); + assert_eq!( + anthropic.unwrap().unwrap_or_default(), + "claude-opus-4", + "ANTHROPIC_MODEL must equal the effective model" + ); +} + +/// A1: when no model is resolved, ANTHROPIC_MODEL must be removed so Claude +/// uses its own default rather than inheriting a stale env value. +#[test] +fn a1_anthropic_model_removed_when_no_effective_model() { + let mut cmd = std::process::Command::new("true"); + // Pre-set a stale value that might have leaked in. + cmd.env("ANTHROPIC_MODEL", "claude-3-5-sonnet"); + cmd.env("BUZZ_ACP_MODEL", "claude-3-5-sonnet"); + apply_claude_model_env(&mut cmd, None); + + let env_map: std::collections::HashMap<_, _> = cmd.get_envs().collect(); + + let anthropic = env_map.get(std::ffi::OsStr::new("ANTHROPIC_MODEL")); + assert!( + anthropic.is_none() || anthropic.unwrap().is_none(), + "ANTHROPIC_MODEL must be absent when no effective model" + ); + let buzz_acp = env_map.get(std::ffi::OsStr::new("BUZZ_ACP_MODEL")); + assert!( + buzz_acp.is_none() || buzz_acp.unwrap().is_none(), + "BUZZ_ACP_MODEL must always be absent after A1 policy" + ); +} diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/claude.rs b/desktop/src-tauri/src/managed_agents/config_bridge/claude.rs index 449197a3b3..b2ec061807 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/claude.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/claude.rs @@ -1,10 +1,29 @@ use super::types::{ExtensionEntry, RuntimeFileConfig}; -/// Read Claude Code config from `~/.claude/settings.json` and `~/.claude.json`. -pub(super) fn read_config_file() -> Option { +/// Read Claude Code config from `settings.json` and `.claude.json`. +/// +/// `config_dir` — when `Some`, reads both `settings.json` and `.claude.json` +/// from that directory (the agent's effective `CLAUDE_CONFIG_DIR`). +/// Defaults to `~/.claude/settings.json` and `~/.claude.json` when `None`. +/// +/// Both files are resolved from the same directory: the claude 2.1.x binary +/// resolves `.claude.json` as `join(process.env.CLAUDE_CONFIG_DIR || homedir(), +/// ".claude.json")`, mirroring the `settings.json` resolver. A user-set +/// `CLAUDE_CONFIG_DIR` therefore remaps both files — honoring only +/// `settings.json` would misrepresent the agent's actual MCP config. +pub(super) fn read_config_file(config_dir: Option<&std::path::Path>) -> Option { let home = dirs::home_dir()?; - let settings_path = home.join(".claude").join("settings.json"); - let mcp_path = home.join(".claude.json"); + + // #3493: honor user-set CLAUDE_CONFIG_DIR for both settings.json and + // .claude.json — the binary resolves both relative to CLAUDE_CONFIG_DIR. + // Panel reflects the actual config the agent reads. + let settings_path = config_dir + .map(|d| d.join("settings.json")) + .unwrap_or_else(|| home.join(".claude").join("settings.json")); + + let mcp_path = config_dir + .map(|d| d.join(".claude.json")) + .unwrap_or_else(|| home.join(".claude.json")); let settings = read_json_file(&settings_path); let mcp_config = read_json_file(&mcp_path); @@ -35,6 +54,7 @@ pub(super) fn read_config_file() -> Option { name: name.clone(), kind: "mcp".to_string(), enabled: true, + source: None, }); } } @@ -60,144 +80,57 @@ fn json_string(val: &serde_json::Value, key: &str) -> Option { #[cfg(test)] mod tests { use super::*; + use std::io::Write; - /// Parse a settings JSON string into a RuntimeFileConfig using the same - /// logic as read_config_file but without touching the filesystem. - fn parse_settings(json: &str) -> RuntimeFileConfig { - let val: serde_json::Value = serde_json::from_str(json).unwrap(); - let skip = &["model", "effortLevel"]; - RuntimeFileConfig { - model: json_string(&val, "model"), - thinking_effort: json_string(&val, "effortLevel"), - extra: super::super::schema_walker::extract_config_fields(&val, skip), - ..Default::default() - } + fn write_tmp_settings(dir: &std::path::Path, content: &[u8]) { + std::fs::create_dir_all(dir).unwrap(); + let mut f = std::fs::File::create(dir.join("settings.json")).unwrap(); + f.write_all(content).unwrap(); } #[test] - fn parse_model_from_settings() { - let cfg = parse_settings(r#"{"model": "claude-sonnet-4-20250514"}"#); - assert_eq!(cfg.model.as_deref(), Some("claude-sonnet-4-20250514")); - } - - #[test] - fn effort_level_maps_to_thinking_effort() { - let cfg = parse_settings(r#"{"effortLevel": "high"}"#); + fn reads_model_from_settings_json() { + let dir = tempfile::tempdir().unwrap(); + write_tmp_settings( + dir.path(), + br#"{"model": "claude-opus-4", "effortLevel": "high"}"#, + ); + let cfg = read_config_file(Some(dir.path())).unwrap(); + assert_eq!(cfg.model.as_deref(), Some("claude-opus-4")); assert_eq!(cfg.thinking_effort.as_deref(), Some("high")); - // effortLevel must NOT appear in extra (it's in the skip list) - assert!(!cfg.extra.contains_key("effortLevel")); } #[test] - fn always_thinking_enabled_appears_in_extra() { - let cfg = parse_settings(r#"{"alwaysThinkingEnabled": true}"#); - assert_eq!( - cfg.extra.get("alwaysThinkingEnabled").map(|s| s.as_str()), - Some("true"), - "alwaysThinkingEnabled should appear in extra" - ); + fn returns_none_when_no_files_found() { + let dir = tempfile::tempdir().unwrap(); + // No settings.json and no ~/.claude.json (home path won't have a test file) + let result = read_config_file(Some(dir.path())); + // May return Some if ~/.claude.json exists on the test machine — we + // only assert the settings fields are absent when no settings.json. + if let Some(cfg) = result { + assert!(cfg.model.is_none()); + assert!(cfg.thinking_effort.is_none()); + } } #[test] - fn env_vars_flattened_in_extra() { - let cfg = parse_settings( - r#"{"env": {"CLAUDE_CODE_EFFORT_LEVEL": "high", "ANTHROPIC_MODEL": "claude-opus-4"}}"#, - ); - assert_eq!( - cfg.extra - .get("env.CLAUDE_CODE_EFFORT_LEVEL") - .map(|s| s.as_str()), - Some("high"), - "env.CLAUDE_CODE_EFFORT_LEVEL should appear in extra" - ); - assert_eq!( - cfg.extra.get("env.ANTHROPIC_MODEL").map(|s| s.as_str()), - Some("claude-opus-4"), - "env.ANTHROPIC_MODEL should appear in extra" - ); + fn defaults_to_home_claude_dir_when_no_config_dir() { + // Calling with None falls back to ~/.claude/settings.json. + // This is a compile-time path test — we just verify the call compiles + // and returns without panic; we can't assert the result without HOME. + let _result = read_config_file(None); } #[test] - fn arbitrary_env_var_surfaced_without_schema() { - // Config-driven: any env var the user has set appears, even if no schema - // defines it — this is the core benefit over the schema-driven approach. - let cfg = parse_settings(r#"{"env": {"MY_CUSTOM_VAR": "hello"}}"#); - assert_eq!( - cfg.extra.get("env.MY_CUSTOM_VAR").map(|s| s.as_str()), - Some("hello"), - "arbitrary env vars should appear in extra" + fn unknown_fields_appear_in_extra() { + let dir = tempfile::tempdir().unwrap(); + write_tmp_settings( + dir.path(), + br#"{"model": "m", "someUnknownField": "value", "anotherField": true}"#, ); - } - - #[test] - fn enabled_plugins_flattened_in_extra() { - let cfg = parse_settings(r#"{"enabledPlugins": {"plugin-a": true, "plugin-b": true}}"#); - // Walker flattens one level: enabledPlugins.plugin-a = "true" + let cfg = read_config_file(Some(dir.path())).unwrap(); assert!( - cfg.extra.contains_key("enabledPlugins.plugin-a") - || cfg.extra.contains_key("enabledPlugins.plugin-b"), - "enabledPlugins entries should appear as enabledPlugins. in extra" - ); - } - - #[test] - fn parse_permissions_and_hooks() { - let cfg = parse_settings( - r#"{"permissions": {"default": "bypassPermissions"}, "hooks": {"pre-commit": {}}}"#, - ); - // permissions is an object — flattened as permissions.default - assert_eq!( - cfg.extra.get("permissions.default").map(|s| s.as_str()), - Some("bypassPermissions") - ); - // hooks.pre-commit is an empty object — emits placeholder - assert_eq!( - cfg.extra.get("hooks.pre-commit").map(|s| s.as_str()), - Some("{...}") - ); - } - - #[test] - fn parse_mcp_servers() { - let json = - r#"{"mcpServers": {"filesystem": {"command": "npx"}, "github": {"command": "gh"}}}"#; - let val: serde_json::Value = serde_json::from_str(json).unwrap(); - let mut extensions = Vec::new(); - if let Some(servers) = val.get("mcpServers").and_then(|v| v.as_object()) { - for (name, _) in servers { - extensions.push(ExtensionEntry { - name: name.clone(), - kind: "mcp".to_string(), - enabled: true, - }); - } - } - assert_eq!(extensions.len(), 2); - } - - #[test] - fn empty_settings_returns_defaults() { - let cfg = parse_settings("{}"); - assert!(cfg.model.is_none()); - assert!(cfg.thinking_effort.is_none()); - assert!(cfg.system_prompt.is_none()); - } - - #[test] - fn model_not_duplicated_in_extra() { - let cfg = parse_settings(r#"{"model": "claude-opus-4", "effortLevel": "high"}"#); - assert!(!cfg.extra.contains_key("model")); - assert!(!cfg.extra.contains_key("effortLevel")); - } - - #[test] - fn unknown_future_field_appears_in_extra() { - // Config-driven: any field the user has set appears, even if we've never - // heard of it. No schema gate. - let cfg = parse_settings(r#"{"someNewClaudeField": "value"}"#); - assert_eq!( - cfg.extra.get("someNewClaudeField").map(|s| s.as_str()), - Some("value"), + !cfg.extra.is_empty(), "unknown future fields should appear in extra" ); } diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/codex.rs b/desktop/src-tauri/src/managed_agents/config_bridge/codex.rs index c7c7135ccb..9ba0b1bb3a 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/codex.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/codex.rs @@ -79,6 +79,7 @@ fn parse_mcp_servers(table: &toml::Table) -> Vec { name: name.clone(), kind: "mcp".to_string(), enabled: true, + source: None, }) .collect() } diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/goose.rs b/desktop/src-tauri/src/managed_agents/config_bridge/goose.rs index fce54edc40..d94cefea09 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/goose.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/goose.rs @@ -130,6 +130,7 @@ fn parse_extensions( name, kind, enabled, + source: None, }) }) .collect() diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs index c51f325cf3..361b849b11 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs @@ -9,11 +9,17 @@ use super::types::*; /// persona and global tiers assembled at the command boundary. Each field /// builder constructs its own candidate list and resolves via /// `resolve_with_override`. +/// +/// `claude_config_dir` — when `Some`, the panel reads claude `settings.json` +/// from that directory (the agent's effective `CLAUDE_CONFIG_DIR` value) +/// instead of `~/.claude/`. Implements the #3493 respect-fix: display the +/// config the agent actually reads without enforcing any layout ourselves. pub(crate) fn read_config_surface( record: &ManagedAgentRecord, runtime_meta: Option<&KnownAcpRuntime>, session_cache: Option<&SessionConfigCache>, tiers: &InheritedConfigTiers, + claude_config_dir: Option<&std::path::Path>, ) -> RuntimeConfigSurface { let is_pre_spawn = session_cache.is_none(); @@ -22,7 +28,7 @@ pub(crate) fn read_config_surface( .map(|m| m.id) .and_then(|id| match id { "goose" => super::goose::read_config_file().map(|c| (c, true)), - "claude" => super::claude::read_config_file().map(|c| (c, true)), + "claude" => super::claude::read_config_file(claude_config_dir).map(|c| (c, true)), "codex" => super::codex::read_config_file().map(|c| (c, true)), "buzz-agent" => super::buzz_agent::read_config_file().map(|c| (c, true)), _ => None, @@ -148,7 +154,8 @@ pub(crate) fn read_config_surface( let config_file_path = runtime_meta .and_then(|m| m.config_file_path) .map(resolve_tilde); - let mcp_config_file_path = runtime_meta.and_then(mcp_config_file_path_for_runtime); + let mcp_config_file_path = + runtime_meta.and_then(|m| mcp_config_file_path_for_runtime(m, claude_config_dir)); let extensions = file_config.extensions.clone(); let sources = ConfigSourceReport { @@ -189,15 +196,58 @@ pub(crate) fn read_config_surface( advanced, extensions, sources, + claude_config_dir_custom: claude_config_dir.is_some(), + effort_config_id: if runtime_meta.map(|m| m.id == "claude").unwrap_or(false) { + // B5: extract the thought_level configId from the session cache so the + // UI can call set_config_option without hardcoding the adapter's id. + session_cache.and_then(|c| { + c.config_options + .iter() + .find(|opt| opt.category.as_deref() == Some("thought_level")) + .map(|opt| opt.config_id.clone()) + }) + } else { + None + }, + effort_options: if runtime_meta.map(|m| m.id == "claude").unwrap_or(false) { + // I-7: expose the adapter-advertised option values so the UI renders + // the real option set instead of hardcoded low/medium/high. + session_cache + .and_then(|c| { + c.config_options + .iter() + .find(|opt| opt.category.as_deref() == Some("thought_level")) + .map(|opt| opt.options.clone()) + }) + .unwrap_or_default() + } else { + Vec::new() + }, } } -fn mcp_config_file_path_for_runtime(runtime: &KnownAcpRuntime) -> Option { +fn mcp_config_file_path_for_runtime( + runtime: &KnownAcpRuntime, + claude_config_dir: Option<&std::path::Path>, +) -> Option { match runtime.id { "goose" => { super::goose::goose_config_path().map(|path| path.to_string_lossy().into_owned()) } - "claude" => Some(resolve_tilde("~/.claude.json")), + // #3493: the claude 2.1.x binary resolves .claude.json as + // join(CLAUDE_CONFIG_DIR || homedir(), ".claude.json"), so the MCP + // config file moves with a user-set CLAUDE_CONFIG_DIR. + "claude" => Some( + claude_config_dir + .map(|d| d.join(".claude.json")) + .unwrap_or_else(|| { + dirs::home_dir() + .map(|h| h.join(".claude.json")) + .unwrap_or_default() + }) + .to_string_lossy() + .into_owned(), + ), "codex" => { super::codex::codex_config_path().map(|path| path.to_string_lossy().into_owned()) } @@ -491,7 +541,13 @@ fn build_thinking_field( session_cache: Option<&SessionConfigCache>, tiers: &InheritedConfigTiers, ) -> Option { - // Tier ordering: record env > ACP > persona env > global env > definition env > config file. + // Tier ordering: + // record env > record.effort_level (canonical Buzz-persisted) > ACP > + // persona env > global env > definition env > config file. + // + // record.effort_level is the B5 canonical value persisted from a positive + // ACP ack. It represents the "configured" value in the B4 status contract + // and is applied at next session start via create_session_and_apply_model. let [rec_env, pers_env, glob_env, def_env] = thinking_env_var .map(|k| { env_candidates( @@ -504,8 +560,11 @@ fn build_thinking_field( }) .unwrap_or([None, None, None, None]); + let canonical_effort = record.effort_level.as_deref(); + let tiers_list: &[(Option<&str>, ConfigOrigin)] = &[ (rec_env, ConfigOrigin::BuzzExplicit), + (canonical_effort, ConfigOrigin::BuzzExplicit), (acp_effort.as_deref(), ConfigOrigin::AcpConfigOption), (pers_env, ConfigOrigin::PersonaDefault), (glob_env, ConfigOrigin::GlobalDefault), 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 62caffeb2e..1943a18c78 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, + effort_level: None, agent_command_override: None, persona_source_version: None, provider: None, @@ -167,7 +168,7 @@ fn persona_and_global_env_tiers( fn pre_spawn_surface_reports_pending_acp_tiers() { let record = test_record(); let runtime = test_runtime(); - let surface = read_config_surface(&record, Some(runtime), None, &no_tiers()); + let surface = read_config_surface(&record, Some(runtime), None, &no_tiers(), None); assert!(surface.is_pre_spawn); assert_eq!(surface.sources.acp_native, ConfigTierStatus::Pending); @@ -183,7 +184,7 @@ fn surface_reports_mcp_specific_config_path() { let record = test_record(); let runtime = test_runtime(); let surface = with_goose_path_root(None, || { - read_config_surface(&record, Some(runtime), None, &no_tiers()) + read_config_surface(&record, Some(runtime), None, &no_tiers(), None) }); let path = surface @@ -202,7 +203,7 @@ fn goose_mcp_config_path_follows_path_root_override() { let record = test_record(); let runtime = test_runtime(); let surface = with_goose_path_root(Some("/tmp/buzz-goose-root"), || { - read_config_surface(&record, Some(runtime), None, &no_tiers()) + read_config_surface(&record, Some(runtime), None, &no_tiers(), None) }); let expected_path = Path::new("/tmp/buzz-goose-root") @@ -226,7 +227,7 @@ fn claude_surface_uses_mcp_config_path_not_settings_path() { config_file_path: Some("~/.claude/settings.json"), ..*test_runtime() }; - let surface = read_config_surface(&record, Some(runtime), None, &no_tiers()); + let surface = read_config_surface(&record, Some(runtime), None, &no_tiers(), None); assert!(surface .sources @@ -246,7 +247,7 @@ fn record_model_overrides_file_model() { record.model = Some("explicit-model".to_string()); let runtime = test_runtime(); - let surface = read_config_surface(&record, Some(runtime), None, &no_tiers()); + let surface = read_config_surface(&record, Some(runtime), None, &no_tiers(), None); let model = surface.normalized.model.unwrap(); assert_eq!(model.value.as_deref(), Some("explicit-model")); assert_eq!(model.origin, ConfigOrigin::BuzzExplicit); @@ -259,7 +260,7 @@ fn provider_locked_shows_locked() { provider_locked: true, ..*test_runtime() }; - let surface = read_config_surface(&record, Some(runtime), None, &no_tiers()); + let surface = read_config_surface(&record, Some(runtime), None, &no_tiers(), None); let provider = surface.normalized.provider.unwrap(); assert_eq!(provider.value.as_deref(), Some("Anthropic (locked)")); assert_eq!(provider.origin, ConfigOrigin::HarnessConstraint); @@ -285,7 +286,7 @@ fn post_spawn_with_model_config_option_uses_acp() { captured_at: "".to_string(), }; - let surface = read_config_surface(&record, Some(runtime), Some(&cache), &no_tiers()); + let surface = read_config_surface(&record, Some(runtime), Some(&cache), &no_tiers(), None); assert!(!surface.is_pre_spawn); let model = surface.normalized.model.unwrap(); assert_eq!(model.value.as_deref(), Some("claude-opus-4")); @@ -309,7 +310,7 @@ fn acp_model_overrides_file_model_with_override_tracking() { captured_at: "".to_string(), }; - let surface = read_config_surface(&record, Some(runtime), Some(&cache), &no_tiers()); + let surface = read_config_surface(&record, Some(runtime), Some(&cache), &no_tiers(), None); let model = surface.normalized.model.unwrap(); assert_eq!(model.value.as_deref(), Some("acp-model")); assert_eq!(model.origin, ConfigOrigin::AcpConfigOption); @@ -330,7 +331,7 @@ fn persona_model_tier_produces_persona_default_origin() { ..Default::default() }; - let surface = read_config_surface(&record, Some(runtime), None, &tiers); + let surface = read_config_surface(&record, Some(runtime), None, &tiers, None); let model = surface.normalized.model.unwrap(); assert_eq!(model.value.as_deref(), Some("persona-model")); @@ -346,7 +347,7 @@ fn global_model_tier_produces_global_default_origin() { ..Default::default() }; - let surface = read_config_surface(&record, Some(runtime), None, &tiers); + let surface = read_config_surface(&record, Some(runtime), None, &tiers, None); let model = surface.normalized.model.unwrap(); assert_eq!(model.value.as_deref(), Some("global-model")); @@ -362,7 +363,7 @@ fn persona_provider_tier_produces_persona_default_origin() { ..Default::default() }; - let surface = read_config_surface(&record, Some(runtime), None, &tiers); + let surface = read_config_surface(&record, Some(runtime), None, &tiers, None); let provider = surface.normalized.provider.unwrap(); assert_eq!(provider.value.as_deref(), Some("anthropic")); @@ -378,7 +379,7 @@ fn persona_prompt_tier_produces_persona_default_origin() { ..Default::default() }; - let surface = read_config_surface(&record, Some(runtime), None, &tiers); + let surface = read_config_surface(&record, Some(runtime), None, &tiers, None); let prompt = surface.normalized.system_prompt.unwrap(); assert_eq!( @@ -415,7 +416,7 @@ fn runtime_override_wins_display_when_model_overridden_is_true() { ..Default::default() }; - let surface = read_config_surface(&record, Some(runtime), Some(&cache), &tiers); + let surface = read_config_surface(&record, Some(runtime), Some(&cache), &tiers, None); let model = surface.normalized.model.unwrap(); // Override wins the display value with a runtime-override origin. @@ -447,7 +448,7 @@ fn no_runtime_override_when_model_overridden_is_false() { ..Default::default() }; - let surface = read_config_surface(&record, Some(runtime), Some(&cache), &tiers); + let surface = read_config_surface(&record, Some(runtime), Some(&cache), &tiers, None); let model = surface.normalized.model.unwrap(); // model_overridden is false => the override branch is not taken. @@ -479,7 +480,7 @@ fn no_false_positive_override_when_persona_edited_mid_life() { ..Default::default() }; - let surface = read_config_surface(&record, Some(runtime), Some(&cache), &tiers); + let surface = read_config_surface(&record, Some(runtime), Some(&cache), &tiers, None); let model = surface.normalized.model.unwrap(); // model_overridden is false => no RuntimeOverride, even though @@ -538,7 +539,7 @@ fn explicit_record_model_not_retagged_when_already_present() { record.model = Some("explicit-model".to_string()); let runtime = test_runtime(); - let surface = read_config_surface(&record, Some(runtime), None, &no_tiers()); + let surface = read_config_surface(&record, Some(runtime), None, &no_tiers(), None); let model = surface.normalized.model.unwrap(); assert_eq!(model.value.as_deref(), Some("explicit-model")); @@ -561,7 +562,7 @@ fn extra_env_vars_appear_in_advanced_as_buzz_explicit() { .insert("SPROUT_ACP_MEMORY".to_string(), "mem-value".to_string()); let runtime = test_runtime(); - let surface = read_config_surface(&record, Some(runtime), None, &no_tiers()); + let surface = read_config_surface(&record, Some(runtime), None, &no_tiers(), None); let advanced_keys: Vec<&str> = surface.advanced.iter().map(|f| f.key.as_str()).collect(); assert!( @@ -600,7 +601,7 @@ fn extra_env_var_skipped_when_already_in_file_config_extra() { .insert("GOOSE_THINKING_EFFORT".to_string(), "high".to_string()); let runtime = test_runtime(); - let surface = read_config_surface(&record, Some(runtime), None, &no_tiers()); + let surface = read_config_surface(&record, Some(runtime), None, &no_tiers(), None); let advanced_keys: Vec<&str> = surface.advanced.iter().map(|f| f.key.as_str()).collect(); assert!( @@ -661,7 +662,7 @@ fn buzz_agent_max_output_tokens_from_env_is_buzz_explicit() { ); let runtime = buzz_agent_runtime(); - let surface = read_config_surface(&record, Some(runtime), None, &no_tiers()); + let surface = read_config_surface(&record, Some(runtime), None, &no_tiers(), None); let field = surface.normalized.max_output_tokens.unwrap(); assert_eq!(field.value.as_deref(), Some("8192")); @@ -682,7 +683,7 @@ fn buzz_agent_context_limit_from_env_is_buzz_explicit() { ); let runtime = buzz_agent_runtime(); - let surface = read_config_surface(&record, Some(runtime), None, &no_tiers()); + let surface = read_config_surface(&record, Some(runtime), None, &no_tiers(), None); let field = surface.normalized.context_limit.unwrap(); assert_eq!(field.value.as_deref(), Some("100000")); @@ -700,7 +701,7 @@ fn buzz_agent_max_tokens_absent_when_no_env_var_or_file() { let record = test_record(); let runtime = buzz_agent_runtime(); - let surface = read_config_surface(&record, Some(runtime), None, &no_tiers()); + let surface = read_config_surface(&record, Some(runtime), None, &no_tiers(), None); assert!( surface.normalized.max_output_tokens.is_none(), @@ -725,7 +726,7 @@ fn buzz_agent_max_tokens_env_var_not_double_surfaced_in_advanced() { ); let runtime = buzz_agent_runtime(); - let surface = read_config_surface(&record, Some(runtime), None, &no_tiers()); + let surface = read_config_surface(&record, Some(runtime), None, &no_tiers(), None); let advanced_keys: Vec<&str> = surface.advanced.iter().map(|f| f.key.as_str()).collect(); assert!( @@ -746,7 +747,7 @@ fn buzz_agent_thinking_effort_from_env_is_buzz_explicit() { .insert("BUZZ_AGENT_THINKING_EFFORT".to_string(), "high".to_string()); let runtime = buzz_agent_runtime(); - let surface = read_config_surface(&record, Some(runtime), None, &no_tiers()); + let surface = read_config_surface(&record, Some(runtime), None, &no_tiers(), None); let field = surface.normalized.thinking_effort.unwrap(); assert_eq!(field.value.as_deref(), Some("high")); @@ -767,7 +768,7 @@ fn buzz_agent_thinking_effort_env_var_not_double_surfaced_in_advanced() { ); let runtime = buzz_agent_runtime(); - let surface = read_config_surface(&record, Some(runtime), None, &no_tiers()); + let surface = read_config_surface(&record, Some(runtime), None, &no_tiers(), None); let advanced_keys: Vec<&str> = surface.advanced.iter().map(|f| f.key.as_str()).collect(); assert!( @@ -829,7 +830,7 @@ fn global_effort_surfaces_as_global_default_when_record_has_none() { let runtime = buzz_agent_rt(); let tiers = global_env_tiers("BUZZ_AGENT_THINKING_EFFORT", "high"); - let surface = read_config_surface(&record, Some(runtime), None, &tiers); + let surface = read_config_surface(&record, Some(runtime), None, &tiers, None); let effort = surface .normalized @@ -846,7 +847,7 @@ fn persona_effort_shadows_global_and_tags_persona_default() { let runtime = buzz_agent_rt(); let tiers = persona_and_global_env_tiers("BUZZ_AGENT_THINKING_EFFORT", "medium", "high"); - let surface = read_config_surface(&record, Some(runtime), None, &tiers); + let surface = read_config_surface(&record, Some(runtime), None, &tiers, None); let effort = surface .normalized @@ -870,7 +871,7 @@ fn record_effort_outranks_persona_and_global_keeps_buzz_explicit() { let runtime = buzz_agent_rt(); let tiers = persona_and_global_env_tiers("BUZZ_AGENT_THINKING_EFFORT", "medium", "high"); - let surface = read_config_surface(&record, Some(runtime), None, &tiers); + let surface = read_config_surface(&record, Some(runtime), None, &tiers, None); let effort = surface .normalized @@ -886,7 +887,7 @@ fn no_effort_anywhere_yields_no_thinking_effort_field() { let record = test_record(); let runtime = buzz_agent_rt(); - let surface = read_config_surface(&record, Some(runtime), None, &no_tiers()); + let surface = read_config_surface(&record, Some(runtime), None, &no_tiers(), None); assert!( surface.normalized.thinking_effort.is_none(), @@ -917,7 +918,7 @@ fn acp_effort_wins_over_inherited_global_effort_as_secondary() { }; let tiers = global_env_tiers("BUZZ_AGENT_THINKING_EFFORT", "high"); - let surface = read_config_surface(&record, Some(runtime), Some(&cache), &tiers); + let surface = read_config_surface(&record, Some(runtime), Some(&cache), &tiers, None); let effort = surface .normalized @@ -941,7 +942,7 @@ fn numeric_max_tokens_inherits_from_global_env() { let runtime = buzz_agent_runtime(); let tiers = global_env_tiers("BUZZ_AGENT_MAX_OUTPUT_TOKENS", "16384"); - let surface = read_config_surface(&record, Some(runtime), None, &tiers); + let surface = read_config_surface(&record, Some(runtime), None, &tiers, None); let field = surface.normalized.max_output_tokens.unwrap(); assert_eq!(field.value.as_deref(), Some("16384")); diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests_ext.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests_ext.rs index 8613124f25..823a41657c 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests_ext.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests_ext.rs @@ -16,7 +16,7 @@ fn numeric_context_limit_inherits_from_persona_env() { let runtime = buzz_agent_runtime(); let tiers = persona_env_tiers("BUZZ_AGENT_MAX_CONTEXT_TOKENS", "200000"); - let surface = read_config_surface(&record, Some(runtime), None, &tiers); + let surface = read_config_surface(&record, Some(runtime), None, &tiers, None); let field = surface.normalized.context_limit.unwrap(); assert_eq!(field.value.as_deref(), Some("200000")); @@ -33,7 +33,7 @@ fn record_max_tokens_overrides_global_env_with_secondary() { let runtime = buzz_agent_runtime(); let tiers = global_env_tiers("BUZZ_AGENT_MAX_OUTPUT_TOKENS", "16384"); - let surface = read_config_surface(&record, Some(runtime), None, &tiers); + let surface = read_config_surface(&record, Some(runtime), None, &tiers, None); let field = surface.normalized.max_output_tokens.unwrap(); assert_eq!(field.value.as_deref(), Some("8192")); @@ -64,7 +64,7 @@ fn global_env_prompt_wins_over_persona_structured_prompt() { ..Default::default() }; - let surface = read_config_surface(&record, Some(runtime), None, &tiers); + let surface = read_config_surface(&record, Some(runtime), None, &tiers, None); let prompt = surface.normalized.system_prompt.unwrap(); assert_eq!(prompt.value.as_deref(), Some("global-env-prompt")); @@ -87,7 +87,7 @@ fn persona_env_model_wins_over_persona_structured_model() { ..Default::default() }; - let surface = read_config_surface(&record, Some(runtime), None, &tiers); + let surface = read_config_surface(&record, Some(runtime), None, &tiers, None); let model = surface.normalized.model.unwrap(); // persona env outranks persona struct because env candidates precede struct @@ -106,7 +106,7 @@ fn structured_fallback_intact_when_no_env_representation() { ..Default::default() }; - let surface = read_config_surface(&record, Some(runtime), None, &tiers); + let surface = read_config_surface(&record, Some(runtime), None, &tiers, None); let model = surface.normalized.model.unwrap(); assert_eq!(model.value.as_deref(), Some("struct-persona-model")); @@ -130,7 +130,7 @@ fn post_sanitization_empty_global_env_falls_through_to_persona_tier() { // No global env (stripped); persona provides the valid fallback. let tiers = persona_env_tiers("BUZZ_AGENT_THINKING_EFFORT", "medium"); - let surface = read_config_surface(&record, Some(runtime), None, &tiers); + let surface = read_config_surface(&record, Some(runtime), None, &tiers, None); // Persona value surfaces instead of the stripped global value. let effort = surface.normalized.thinking_effort.unwrap(); @@ -157,7 +157,7 @@ fn record_env_prompt_wins_over_record_struct_prompt_as_buzz_explicit() { ); let runtime = test_runtime(); - let surface = read_config_surface(&record, Some(runtime), None, &no_tiers()); + let surface = read_config_surface(&record, Some(runtime), None, &no_tiers(), None); let prompt = surface.normalized.system_prompt.unwrap(); assert_eq!(prompt.value.as_deref(), Some("env-prompt-B")); @@ -189,7 +189,7 @@ fn definition_env_beats_structured_persona_model() { ..Default::default() }; - let surface = read_config_surface(&record, Some(runtime), None, &tiers); + let surface = read_config_surface(&record, Some(runtime), None, &tiers, None); let model = surface.normalized.model.unwrap(); assert_eq!(model.value.as_deref(), Some("harness-model")); @@ -222,7 +222,7 @@ fn global_env_beats_definition_env() { ..Default::default() }; - let surface = read_config_surface(&record, Some(runtime), None, &tiers); + let surface = read_config_surface(&record, Some(runtime), None, &tiers, None); let model = surface.normalized.model.unwrap(); assert_eq!(model.value.as_deref(), Some("global-model")); @@ -249,10 +249,136 @@ fn reserved_key_absent_from_definition_env_falls_through() { ..Default::default() }; - let surface = read_config_surface(&record, Some(runtime), None, &tiers); + let surface = read_config_surface(&record, Some(runtime), None, &tiers, None); let model = surface.normalized.model.unwrap(); // Falls through to persona structured model. assert_eq!(model.value.as_deref(), Some("persona-struct-model")); assert_eq!(model.origin, ConfigOrigin::PersonaDefault); } + +// ── B4/B5 canonical effort_level tier tests ──────────────────────────────── +// +// record.effort_level is the Buzz-canonical seeded value (B5 persisted from +// a positive ACP ack). It must surface as BuzzExplicit and take precedence +// over the config-file tier (but not over record env vars). + +/// B4: record.effort_level surfaces as BuzzExplicit when no env var is set. +#[test] +fn b4_canonical_effort_level_surfaces_as_buzz_explicit() { + let mut record = test_record(); + record.effort_level = Some("high".to_string()); + let runtime = buzz_agent_runtime(); + let surface = read_config_surface(&record, Some(runtime), None, &no_tiers(), None); + let effort = surface + .normalized + .thinking_effort + .expect("effort must surface from canonical record tier"); + assert_eq!(effort.value.as_deref(), Some("high")); + assert_eq!(effort.origin, ConfigOrigin::BuzzExplicit); +} + +/// B4: record.effort_level shadows the config-file tier. +#[test] +fn b4_canonical_effort_level_shadows_file_tier() { + let mut record = test_record(); + // canonical effort takes precedence over file tier + record.effort_level = Some("medium".to_string()); + // no env var set — config-file would otherwise win if canonical absent + let runtime = buzz_agent_runtime(); + let surface = read_config_surface(&record, Some(runtime), None, &no_tiers(), None); + let effort = surface + .normalized + .thinking_effort + .expect("canonical effort must shadow file tier"); + assert_eq!(effort.value.as_deref(), Some("medium")); + assert_eq!(effort.origin, ConfigOrigin::BuzzExplicit); +} + +/// B4: a record env var override still wins over record.effort_level. +#[test] +fn b4_record_env_var_wins_over_canonical_effort_level() { + let mut record = test_record(); + record.effort_level = Some("low".to_string()); + record + .env_vars + .insert("BUZZ_AGENT_THINKING_EFFORT".to_string(), "high".to_string()); + let runtime = buzz_agent_runtime(); + let surface = read_config_surface(&record, Some(runtime), None, &no_tiers(), None); + let effort = surface + .normalized + .thinking_effort + .expect("env var must win over canonical effort"); + assert_eq!(effort.value.as_deref(), Some("high")); + assert_eq!(effort.origin, ConfigOrigin::BuzzExplicit); + // canonical is the overridden baseline + assert_eq!(effort.overridden_value.as_deref(), Some("low")); +} + +/// B4: None effort_level does not introduce a spurious tier. +#[test] +fn b4_none_canonical_effort_does_not_surface() { + let record = test_record(); // effort_level defaults to None + let runtime = buzz_agent_runtime(); + let surface = read_config_surface(&record, Some(runtime), None, &no_tiers(), None); + // No env var, no session cache, no file config → effort_level field absent. + assert!( + surface.normalized.thinking_effort.is_none(), + "effort field must be absent when no tier has a value" + ); +} + +// ── CLAUDE_CONFIG_DIR path resolution ───────────────────────────────────────── + +#[test] +fn claude_mcp_config_path_honors_custom_claude_config_dir() { + // M-2: mcp_config_file_path_for_runtime must use the custom dir when + // claude_config_dir is Some, not fall back to ~/.claude.json. + let record = test_record(); + let runtime = &KnownAcpRuntime { + id: "claude", + config_file_path: Some("~/.claude/settings.json"), + ..*test_runtime() + }; + let custom_dir = std::path::PathBuf::from("/custom/config/dir"); + let surface = read_config_surface(&record, Some(runtime), None, &no_tiers(), Some(&custom_dir)); + + let mcp_path = surface + .sources + .mcp_config_file_path + .expect("mcp_config_file_path must be present for claude runtime"); + assert_eq!( + std::path::Path::new(&mcp_path), + custom_dir.join(".claude.json"), + "mcp config path must be /.claude.json when CLAUDE_CONFIG_DIR is set" + ); + assert!( + surface.claude_config_dir_custom, + "claude_config_dir_custom must be true when a custom dir was passed" + ); +} + +#[test] +fn claude_config_dir_none_falls_back_to_home_claude_json() { + // M-3: None (i.e. the caller stripped an empty string) must resolve to + // the default ~/.claude.json path, matching Claude's `CLAUDE_CONFIG_DIR || homedir()`. + let record = test_record(); + let runtime = &KnownAcpRuntime { + id: "claude", + config_file_path: Some("~/.claude/settings.json"), + ..*test_runtime() + }; + let surface = read_config_surface(&record, Some(runtime), None, &no_tiers(), None); + assert!( + !surface.claude_config_dir_custom, + "claude_config_dir_custom must be false when dir is None (unset)" + ); + assert!( + surface + .sources + .mcp_config_file_path + .as_deref() + .is_some_and(|p| p.ends_with(".claude.json")), + "mcp path must fall back to ~/.claude.json when no custom dir" + ); +} diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/types.rs b/desktop/src-tauri/src/managed_agents/config_bridge/types.rs index 6ca2592538..81970a9959 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/types.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/types.rs @@ -175,6 +175,25 @@ pub struct RuntimeConfigSurface { pub advanced: Vec, pub extensions: Vec, pub sources: ConfigSourceReport, + /// #3493: `true` when the panel is reading from a user-set `CLAUDE_CONFIG_DIR` + /// rather than the default `~/.claude/`. Used to show the Keychain caveat + /// note in the panel: a custom config dir means a fresh Keychain namespace + /// (hash-suffixed), so the agent will be logged out unless the user also + /// manages `CLAUDE_SECURESTORAGE_CONFIG_DIR`. + #[serde(default)] + pub claude_config_dir_custom: bool, + /// B5: the real `configId` for the `thought_level` ACP config option, + /// as advertised by the adapter in `session/new`. Present only for claude + /// runtimes after the first session is created. The UI uses this to send + /// `set_config_option` without hardcoding the configId. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub effort_config_id: Option, + /// B5/I-7: the adapter-advertised option values for the `thought_level` + /// config option. Present when `effort_config_id` is Some. The UI renders + /// these instead of hardcoded low/medium/high so model-specific option sets + /// are reflected correctly. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub effort_options: Vec, } /// Raw config values extracted from a runtime's config file. @@ -198,6 +217,11 @@ pub struct ExtensionEntry { pub name: String, pub kind: String, pub enabled: bool, + /// Provenance tag for display. `Some("owner_user_scope")` means the entry + /// was inherited from the owner's user-scope `~/.claude.json` by B8. + /// `None` means the entry was read directly from the runtime's config file. + #[serde(skip_serializing_if = "Option::is_none")] + pub source: Option, } /// Cached ACP session config from a running agent. diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests.rs b/desktop/src-tauri/src/managed_agents/discovery/tests.rs index 6fe6a77521..fc751fd186 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -283,13 +283,13 @@ fn record_with( definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, + effort_level: None, } } #[test] fn record_agent_command_own_runtime_wins_over_persona() { - // A record with its own materialized runtime never consults the - // persona list — the unified-model resolution. + // A record with its own runtime never consults the persona list. let personas = vec![persona_with_runtime("p1", Some("goose"))]; let record = record_with(Some("claude"), Some("p1"), None); assert_eq!(record_agent_command(&record, &personas), "claude-agent-acp"); diff --git a/desktop/src-tauri/src/managed_agents/effective_config/tests.rs b/desktop/src-tauri/src/managed_agents/effective_config/tests.rs index c8e437809c..b4c08804c7 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, + effort_level: 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/tests.rs b/desktop/src-tauri/src/managed_agents/global_config/tests.rs index 553596e226..5c6e11e606 100644 --- a/desktop/src-tauri/src/managed_agents/global_config/tests.rs +++ b/desktop/src-tauri/src/managed_agents/global_config/tests.rs @@ -352,6 +352,7 @@ fn bare_record() -> ManagedAgentRecord { definition_respond_to: None, definition_respond_to_allowlist: vec![], definition_parallelism: None, + effort_level: None, } } diff --git a/desktop/src-tauri/src/managed_agents/mod.rs b/desktop/src-tauri/src/managed_agents/mod.rs index fe90ce430f..b8cae925fb 100644 --- a/desktop/src-tauri/src/managed_agents/mod.rs +++ b/desktop/src-tauri/src/managed_agents/mod.rs @@ -9,6 +9,7 @@ pub(crate) use agent_env::{ baked_build_env, build_buzz_agent_provider_defaults, discovery_env_with_baked_floor, }; mod backend; +pub(crate) mod claude_config; pub(crate) mod config_bridge; pub(crate) mod custom_harnesses; mod discovery; diff --git a/desktop/src-tauri/src/managed_agents/nest/tests.rs b/desktop/src-tauri/src/managed_agents/nest/tests.rs index cbef171f6f..c71a240245 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, + effort_level: None, } } diff --git a/desktop/src-tauri/src/managed_agents/parallelism.rs b/desktop/src-tauri/src/managed_agents/parallelism.rs index e1691575b1..bc312375e7 100644 --- a/desktop/src-tauri/src/managed_agents/parallelism.rs +++ b/desktop/src-tauri/src/managed_agents/parallelism.rs @@ -116,6 +116,7 @@ mod tests { definition_respond_to: None, definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, + effort_level: None, relay_mesh: None, } } 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 0580b12ce2..2ff165201d 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, + effort_level: None, } } diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index c072448ff1..e935d22a61 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -1465,9 +1465,8 @@ mod tests { #[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. + // User env_vars must win over baked defaults; in OSS builds baked map is empty, + // so this 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( @@ -1530,6 +1529,7 @@ mod tests { definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, + effort_level: None, }; let runtime = known_acp_runtime_exact("buzz-agent"); diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index ec804869c4..c9d28aef9a 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -14,6 +14,7 @@ use crate::{ util::now_iso, }; +use super::claude_config::apply_claude_model_env; mod path; pub(in crate::managed_agents) use path::build_augmented_path; pub(crate) use path::{compose_path_entries, should_skip_claude_executable, should_use_inherited}; @@ -718,6 +719,12 @@ pub fn spawn_agent_child( } else { command.env_remove("BUZZ_ACP_MODEL"); } + // B5: carry persisted effort; harness resolves thought_level configId at first session. + if let Some(ref effort) = record.effort_level { + command.env("BUZZ_ACP_EFFORT_LEVEL", effort); + } else { + command.env_remove("BUZZ_ACP_EFFORT_LEVEL"); + } // Session title for the harness to pass out-of-band on `session/new`. The // adapter names the session after it; it never reaches the prompt, so this // is display metadata only. The spawn-config snapshot records the same @@ -763,17 +770,8 @@ pub fn spawn_agent_child( command.env("BUZZ_ACP_RELAY_OBSERVER", "true"); - // ── Git credential helper for Buzz relay ────────────────────────── - // - // Agents need to clone/push repos hosted on the Buzz relay's git - // server, which authenticates via NIP-98. The `git-credential-nostr` - // binary signs auth events using the agent's nostr key. - // - // We configure git via GIT_CONFIG_COUNT env vars (ephemeral, no - // filesystem writes) scoped to the relay's git URL so we don't - // interfere with other remotes (e.g. GitHub). - // - // NOSTR_PRIVATE_KEY mirrors BUZZ_PRIVATE_KEY — keep in sync. + // Git credential helper: NIP-98 auth for Buzz relay git via git-credential-nostr. + // Ephemeral GIT_CONFIG_COUNT env vars scoped to relay HTTP URL; NOSTR_PRIVATE_KEY mirrors BUZZ_PRIVATE_KEY. if let Some(cred_helper) = resolve_command("git-credential-nostr") { let relay_http_url = crate::relay::relay_http_base_url(&effective_relay_url); @@ -798,17 +796,19 @@ pub fn spawn_agent_child( ); } - // ── User env vars: definition floor + global + live persona + agent overrides ── - // - // `descriptor.env` is the fully-layered result from `resolve_effective_harness_descriptor`: - // baked floor → runtime metadata → definition env (harness author defaults) → - // global → live persona → per-agent, with reserved-key and malformed-key filtering - // applied. Writing it last lets user-provided values win over every Buzz-set env - // written above — reserved keys were already stripped from descriptor.env so they - // cannot clobber BUZZ_PRIVATE_KEY, NOSTR_PRIVATE_KEY, etc. + // User env (descriptor.env): fully-layered floor→runtime→definition→global→persona→agent, + // reserved-key filtered. Written last so user-explicit values win over Buzz-set env. for (key, value) in &descriptor.env { command.env(key, value); } + + // A1: for local claude agents, ANTHROPIC_MODEL is the single startup model authority. + // BUZZ_ACP_MODEL is removed (live ACP switches only; two authorities in the same env + // would be ambiguous). + if record.backend == super::BackendKind::Local && runtime_meta.is_some_and(|r| r.id == "claude") + { + apply_claude_model_env(&mut command, effective_model.as_deref()); + } configure_runtime_cli(&mut command, runtime_meta); // Buzz shared compute is stored as a native provider; derive the OpenAI-compatible 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 9836d983ed..22190fca51 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, + effort_level: None, } } diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index 762b0fe2a6..c72a1ccb61 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -71,9 +71,8 @@ fn identifier_empty_returns_false() { #[test] fn marker_entry_is_namespaced_by_instance_id() { - // The spawn stamp and sweep matcher both go through buzz_marker_entry, pinning the on-the-wire - // format and guards against a dev build (`...app.dev`) matching a - // release build's (`...app`) agents. + // spawn stamp and sweep matcher both go through buzz_marker_entry (pins wire format, + // guards dev build `...app.dev` from matching release `...app` agents). assert_eq!( super::buzz_marker_entry("xyz.block.buzz.app"), b"BUZZ_MANAGED_AGENT=xyz.block.buzz.app".to_vec() 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 1ceeee372f..4b675e5e94 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, + effort_level: None, } } diff --git a/desktop/src-tauri/src/managed_agents/team_snapshot.rs b/desktop/src-tauri/src/managed_agents/team_snapshot.rs index 96082acc76..e821ef9bf1 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, + effort_level: None, } } diff --git a/desktop/src-tauri/src/managed_agents/teams_tests.rs b/desktop/src-tauri/src/managed_agents/teams_tests.rs index 1ffa60eda9..ae4bb0cda8 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, + effort_level: 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 e5be105fed..503b35914c 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -153,6 +153,7 @@ impl AgentDefinition { definition_respond_to_allowlist: self.respond_to_allowlist, definition_parallelism: self.parallelism, relay_mesh: None, + effort_level: None, } } } @@ -438,24 +439,10 @@ pub struct ManagedAgentRecord { /// deserialize as `None`. #[serde(default, skip_serializing_if = "Option::is_none")] pub relay_mesh: Option, -} - -/// Typed relay-mesh configuration carried on a [`ManagedAgentRecord`]. -/// -/// Feature-independent on purpose: the field is always present in the record -/// schema so saved agents round-trip identically whether or not the `mesh-llm` -/// feature is compiled in. -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] -pub struct RelayMeshConfig { - /// The served model id this agent routes to (e.g. "Qwen3"). - /// - /// `alias` because this struct crosses two boundaries with different - /// casing conventions: the TS create request sends camelCase - /// (`relayMesh: { modelRef }` — `rename_all` on the request does not - /// recurse into nested structs), while persisted records use snake_case. - /// Serialization stays `model_ref` so saved records are stable. - #[serde(alias = "modelRef")] - pub model_ref: String, + /// Canonical Claude Code effort level. Injected as `BUZZ_ACP_EFFORT_LEVEL` at spawn + /// so the harness applies it via `session/set_config_option` at session creation. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub effort_level: Option, } #[derive(Debug)] @@ -990,6 +977,8 @@ pub fn resolve_mint_behavioral_defaults( mod catalog_source; pub use catalog_source::CatalogSource; +mod relay_mesh; +pub use relay_mesh::RelayMeshConfig; mod requests; pub use requests::*; diff --git a/desktop/src-tauri/src/managed_agents/types/relay_mesh.rs b/desktop/src-tauri/src/managed_agents/types/relay_mesh.rs new file mode 100644 index 0000000000..a9ec2d2838 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/types/relay_mesh.rs @@ -0,0 +1,19 @@ +use serde::{Deserialize, Serialize}; + +/// Typed relay-mesh configuration carried on a [`super::ManagedAgentRecord`]. +/// +/// Feature-independent on purpose: the field is always present in the record +/// schema so saved agents round-trip identically whether or not the `mesh-llm` +/// feature is compiled in. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct RelayMeshConfig { + /// The served model id this agent routes to (e.g. "Qwen3"). + /// + /// `alias` because this struct crosses two boundaries with different + /// casing conventions: the TS create request sends camelCase + /// (`relayMesh: { modelRef }` — `rename_all` on the request does not + /// recurse into nested structs), while persisted records use snake_case. + /// Serialization stays `model_ref` so saved records are stable. + #[serde(alias = "modelRef")] + pub model_ref: String, +} diff --git a/desktop/src/features/agents/lib/effortOutcome.test.mjs b/desktop/src/features/agents/lib/effortOutcome.test.mjs new file mode 100644 index 0000000000..737a86795d --- /dev/null +++ b/desktop/src/features/agents/lib/effortOutcome.test.mjs @@ -0,0 +1,298 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { awaitEffortOutcome } from "./effortOutcome.ts"; + +const CONFIG_ID = "effort"; + +function frame(status, overrides = {}) { + return { + type: "set_config_option", + status, + configId: CONFIG_ID, + value: "high", + category: "thought_level", + ...overrides, + }; +} + +/** + * A controllable test harness that wires awaitEffortOutcome with: + * - a pub/sub whose unsubscribe genuinely detaches (post-detach pushes no-op) + * - a manual timeout the test fires explicitly + * - a deferred send the test can inspect + */ +function harness(value = "high") { + let listener = null; + let timeoutCb = null; + let unsubscribeCalls = 0; + let cancelTimeoutCalls = 0; + let sendCalled = false; + + const outcome = awaitEffortOutcome({ + configId: CONFIG_ID, + value, + subscribe: (fn) => { + listener = fn; + return () => { + unsubscribeCalls += 1; + listener = null; + }; + }, + send: () => { + sendCalled = true; + return Promise.resolve(); + }, + scheduleTimeout: (cb) => { + timeoutCb = cb; + return () => { + cancelTimeoutCalls += 1; + }; + }, + }); + + return { + outcome, + push: (f) => listener?.(f), + fireTimeout: () => timeoutCb?.(), + get sendCalled() { + return sendCalled; + }, + get unsubscribeCalls() { + return unsubscribeCalls; + }, + get cancelTimeoutCalls() { + return cancelTimeoutCalls; + }, + }; +} + +// ── subscription ordering ───────────────────────────────────────────────────── + +test("awaitEffortOutcome subscribes before sending so no ack is dropped", async () => { + const h = harness(); + // Push an ack synchronously — if subscribe happened after send, this would + // be dropped and the outcome would never resolve (only the timeout would). + h.push(frame("ok")); + assert.equal(await h.outcome, "ok"); + assert.equal(h.sendCalled, true, "send must have been called"); +}); + +// ── terminal statuses ───────────────────────────────────────────────────────── + +test("awaitEffortOutcome resolves 'ok' on accepted ack", async () => { + const h = harness(); + h.push(frame("ok")); + assert.equal(await h.outcome, "ok"); +}); + +test("awaitEffortOutcome resolves 'failure' on rejected ack", async () => { + const h = harness(); + h.push(frame("failure")); + assert.equal(await h.outcome, "failure"); +}); + +test("awaitEffortOutcome resolves 'invalid_value' on validation rejection", async () => { + const h = harness(); + h.push(frame("invalid_value")); + assert.equal(await h.outcome, "invalid_value"); +}); + +// ── clear path (I-1) ────────────────────────────────────────────────────────── + +test("awaitEffortOutcome resolves 'cleared' when Auto (empty value) is selected", async () => { + const h = harness(""); // empty value = clear + h.push( + frame("cleared", { value: "" }), // harness sends value: "" in the clear ack + ); + assert.equal(await h.outcome, "cleared"); +}); + +// ── pending_session / deferred path ────────────────────────────────────────── + +test("awaitEffortOutcome keeps waiting on pending_session and resolves ok on final ack", async () => { + const h = harness(); + let settled = false; + void h.outcome.then(() => { + settled = true; + }); + + const drain = async () => { + for (let i = 0; i < 5; i++) await Promise.resolve(); + }; + + h.push(frame("pending_session")); + await drain(); + assert.equal(settled, false, "pending_session must not settle the outcome"); + + h.push(frame("ok")); + assert.equal(await h.outcome, "ok"); +}); + +test("awaitEffortOutcome resolves 'pending_session' via timeout when harness never replies", async () => { + const h = harness(); + h.fireTimeout(); + assert.equal(await h.outcome, "pending_session"); + assert.equal(h.unsubscribeCalls, 1, "timeout must unsubscribe"); +}); + +// ── correlation ─────────────────────────────────────────────────────────────── + +test("awaitEffortOutcome ignores acks for a different configId", async () => { + const h = harness(); + h.push(frame("ok", { configId: "some_other_option" })); + let settled = false; + void h.outcome.then(() => { + settled = true; + }); + await Promise.resolve(); + assert.equal(settled, false, "unrelated configId must not advance outcome"); + + h.push(frame("ok")); + assert.equal(await h.outcome, "ok"); +}); + +test("awaitEffortOutcome ignores acks for a different value when non-empty", async () => { + const h = harness("high"); + // An ack for a different value — stale ack from a prior pick. + h.push(frame("ok", { value: "low" })); + let settled = false; + void h.outcome.then(() => { + settled = true; + }); + await Promise.resolve(); + assert.equal( + settled, + false, + "stale ack for different value must not settle outcome", + ); + + h.push(frame("ok", { value: "high" })); + assert.equal(await h.outcome, "ok"); +}); + +test("awaitEffortOutcome ignores acks of wrong control type", async () => { + const h = harness(); + h.push({ type: "switch_model", status: "ok", configId: CONFIG_ID }); + let settled = false; + void h.outcome.then(() => { + settled = true; + }); + await Promise.resolve(); + assert.equal(settled, false, "wrong type must not settle outcome"); + + h.push(frame("ok")); + assert.equal(await h.outcome, "ok"); +}); + +// ── cleanup ─────────────────────────────────────────────────────────────────── + +test("awaitEffortOutcome unsubscribes and cancels timeout exactly once on success", async () => { + const h = harness(); + h.push(frame("ok")); + await h.outcome; + assert.equal(h.unsubscribeCalls, 1); + assert.equal(h.cancelTimeoutCalls, 1); + + // A late ack must not re-unsubscribe — listener is already detached. + h.push(frame("ok")); + assert.equal(h.unsubscribeCalls, 1, "no double-unsubscribe on late ack"); +}); + +// ── nonce correlation (IMPORTANT-2) ────────────────────────────────────────── + +/** + * A controllable harness with nonce support for correlation tests. + */ +function harnessWithNonce(value = "high", nonce = "nonce-1") { + let listener = null; + let timeoutCb = null; + + const outcome = awaitEffortOutcome({ + configId: CONFIG_ID, + value, + nonce, + subscribe: (fn) => { + listener = fn; + return () => { + listener = null; + }; + }, + send: () => Promise.resolve(), + scheduleTimeout: (cb) => { + timeoutCb = cb; + return () => {}; + }, + }); + + return { + outcome, + push: (f) => listener?.(f), + fireTimeout: () => timeoutCb?.(), + }; +} + +test("awaitEffortOutcome rejects acks with a different nonce (stale superseded pick)", async () => { + // Simulates: high→low→high, where the old 'high' ok arrives carrying a stale nonce. + const h = harnessWithNonce("high", "nonce-current"); + + // Stale ack from a prior 'high' pick — different nonce. + h.push(frame("ok", { value: "high", nonce: "nonce-old" })); + let settled = false; + void h.outcome.then(() => { + settled = true; + }); + await Promise.resolve(); + assert.equal( + settled, + false, + "stale nonce must not settle the current pick's promise", + ); + + // Correct nonce — current pick's final result. + h.push(frame("ok", { value: "high", nonce: "nonce-current" })); + assert.equal(await h.outcome, "ok"); +}); + +test("awaitEffortOutcome accepts ack with matching nonce regardless of value equality", async () => { + // Nonce is the primary key; value equality is fallback-only. + const h = harnessWithNonce("high", "nonce-abc"); + h.push(frame("ok", { value: "high", nonce: "nonce-abc" })); + assert.equal(await h.outcome, "ok"); +}); + +test("awaitEffortOutcome resolves pending_session via timeout (late result after timeout does not re-settle)", async () => { + const h = harnessWithNonce("high", "nonce-xyz"); + h.fireTimeout(); + // Outcome is now pending_session. + assert.equal(await h.outcome, "pending_session"); + + // A late final ack arriving after timeout — listener is already detached. + // This must be a no-op; since the Promise already resolved, no assertion + // is possible on the outcome value, but the push must not throw. + h.push(frame("ok", { value: "high", nonce: "nonce-xyz" })); + // If we got here without error the post-timeout push was handled safely. +}); + +test("awaitEffortOutcome ignores acks from a superseded nonce after clear", async () => { + // Simulates: pick 'high' then clear (Auto) while 'high' ok is in flight. + // The clear wins; the stale 'high' ok must not settle. + const h = harnessWithNonce("high", "nonce-clear"); + + // Stale 'high' ok with a different (old) nonce. + h.push(frame("ok", { value: "high", nonce: "nonce-old" })); + let settled = false; + void h.outcome.then(() => { + settled = true; + }); + await Promise.resolve(); + assert.equal( + settled, + false, + "stale pick ok must not settle after clear was dispatched", + ); + + // The clear's final ack arrives with the current nonce. + h.push(frame("cleared", { value: "", nonce: "nonce-clear" })); + assert.equal(await h.outcome, "cleared"); +}); diff --git a/desktop/src/features/agents/lib/effortOutcome.ts b/desktop/src/features/agents/lib/effortOutcome.ts new file mode 100644 index 0000000000..1f8493ae8c --- /dev/null +++ b/desktop/src/features/agents/lib/effortOutcome.ts @@ -0,0 +1,107 @@ +import type { ControlResultFrame } from "@/shared/api/types"; + +/** + * Await the outcome of an effort set (or clear) request. + * + * Sends a `set_config_option` frame and waits for the matching final + * `control_result` from the harness. Two phases: + * + * 1. Immediate ack from `handle_set_config_option_control`: + * `pending_session` (stored, will apply at next session — for both picks + * and clears) or `invalid_value` (rejected by harness validation). + * + * 2. Final ack from the harness (`pool.resolve_effort_report`, emitted via the + * main loop's `PoolEvent::EffortReport` arm immediately after + * `session_set_config_option` resolves — before the prompt is sent): + * `ok` (adapter accepted; Desktop persists) or + * `failure` (adapter rejected or timeout) or + * `cleared` (session ran without effort override; Desktop persists null). + * + * The function resolves with the first *terminal* status received: + * - `"ok"` / `"failure"` / `"invalid_value"` / `"cleared"` — terminal. + * - `"pending_session"` — non-terminal; awaiting final result from the harness. + * + * Correlation: when `nonce` is provided, only acks carrying the same nonce are + * considered. This prevents a stale ack from a superseded pick from settling + * the current promise. Without nonce, correlation falls back to configId+value. + * + * If no terminal result arrives within the timeout, resolves with + * `"pending_session"` (the effort will be applied at the next session — the UI + * should show this as a deferred confirmation). + */ +export async function awaitEffortOutcome({ + configId, + value, + nonce, + subscribe, + send, + scheduleTimeout, +}: { + /** The thought_level configId from the session cache. */ + configId: string; + /** The value being set (or "" for clear). */ + value: string; + /** Nonce echoed by the harness in all acks for this request. When provided, + * used as the primary correlation key so stale acks from prior picks are + * ignored even if they share the same configId and value. */ + nonce?: string; + /** Register a control-result listener; returns an unsubscribe function. */ + subscribe: (listener: (frame: ControlResultFrame) => void) => () => void; + /** Fire the set_config_option send. */ + send: () => Promise; + /** Schedule the no-reply fallback; returns a cancel function. */ + scheduleTimeout: (onTimeout: () => void) => () => void; +}): Promise< + "ok" | "failure" | "invalid_value" | "cleared" | "pending_session" +> { + type Outcome = + | "ok" + | "failure" + | "invalid_value" + | "cleared" + | "pending_session"; + + const settled = new Promise((resolve) => { + let unsubscribe = () => {}; + let cancelTimeout = () => {}; + const finish = (outcome: Outcome) => { + cancelTimeout(); + unsubscribe(); + resolve(outcome); + }; + cancelTimeout = scheduleTimeout(() => finish("pending_session")); + unsubscribe = subscribe((frame) => { + if (frame.type !== "set_config_option" || frame.configId !== configId) { + return; + } + // Primary correlation: nonce when provided. Rejects stale acks from + // superseded picks even when they share configId and value. + if (nonce !== undefined) { + if ((frame as Record).nonce !== nonce) { + return; + } + } else { + // Fallback: correlate by value for non-clear picks. + if (value !== "" && frame.value !== value) { + return; + } + } + const s = frame.status; + if ( + s === "ok" || + s === "failure" || + s === "invalid_value" || + s === "cleared" + ) { + finish(s); + return; + } + // pending_session is non-terminal — keep waiting for the final ack. + // The timeout will eventually fire if no final ack arrives. + }); + }); + + await send(); + + return settled; +} diff --git a/desktop/src/features/agents/observerRelayNonceGate.test.mjs b/desktop/src/features/agents/observerRelayNonceGate.test.mjs new file mode 100644 index 0000000000..0ae53500a8 --- /dev/null +++ b/desktop/src/features/agents/observerRelayNonceGate.test.mjs @@ -0,0 +1,150 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + registerEffortNonce, + resetAgentObserverStore, + _testNonceGate, + _testClearEffortNonceStorage, +} from "./observerRelayStore.ts"; + +const PUBKEY = "aabb"; + +// Minimal localStorage stub for the nonce-persistence tests. +function makeLocalStorage() { + const store = new Map(); + return { + get length() { + return store.size; + }, + key: (i) => [...store.keys()][i] ?? null, + getItem: (key) => store.get(key) ?? null, + setItem: (key, value) => store.set(key, String(value)), + removeItem: (key) => { + store.delete(key); + }, + }; +} + +// Reset store state AND localStorage before each test so nonce map is clean. +function setup() { + // Install a fresh localStorage stub so each test has an isolated store. + globalThis.localStorage = makeLocalStorage(); + resetAgentObserverStore(); +} + +// Full teardown: clear both the in-memory map and localStorage entries. +function fullReset() { + _testClearEffortNonceStorage(); + resetAgentObserverStore(); +} + +// ── startup path (no nonce ever registered) ─────────────────────────────────── + +test("nonce gate: passes ack with no nonce when no nonce has been registered (startup path)", () => { + setup(); + // Pre-any-pick: startup-applied effort acks carry no nonce. + assert.equal(_testNonceGate(PUBKEY, undefined), true); +}); + +test("nonce gate: rejects ack WITH a nonce when no nonce has been registered", () => { + setup(); + // Startup path: a nonce-bearing ack should not pass through the no-nonce gate. + // This prevents a stale ack from an old session from persisting during startup. + assert.equal(_testNonceGate(PUBKEY, "some-nonce"), false); +}); + +// ── post-registration path (nonce registered via registerEffortNonce) ───────── + +test("nonce gate: passes ack with matching nonce once a nonce is registered", () => { + setup(); + registerEffortNonce(PUBKEY, "nonce-42"); + assert.equal(_testNonceGate(PUBKEY, "nonce-42"), true); +}); + +test("nonce gate: rejects ack with mismatched nonce once a nonce is registered", () => { + setup(); + registerEffortNonce(PUBKEY, "nonce-current"); + // Stale ack from a prior pick. + assert.equal(_testNonceGate(PUBKEY, "nonce-old"), false); +}); + +test("nonce gate: rejects nonce-less ack once a nonce is registered (P3 bypass fix)", () => { + setup(); + // P3: once a nonce has been registered, a nonce-less ok/cleared ack must NOT + // pass through — it is a stale result from before the nonce system was in place + // or from a superseded pick. Without this fix the ack would bypass the gate. + registerEffortNonce(PUBKEY, "nonce-registered"); + assert.equal(_testNonceGate(PUBKEY, undefined), false); +}); + +// ── per-agent isolation ─────────────────────────────────────────────────────── + +test("nonce gate: different agents are isolated — no cross-registration", () => { + setup(); + registerEffortNonce("agent-a", "nonce-a"); + // agent-b has no registered nonce → startup path applies. + assert.equal(_testNonceGate("agent-b", undefined), true); + assert.equal(_testNonceGate("agent-b", "nonce-a"), false); +}); + +// ── reset restores from localStorage (F3 durability) ───────────────────────── + +test("resetAgentObserverStore restores nonce from localStorage — replay accepted post-reset", () => { + setup(); + registerEffortNonce(PUBKEY, "nonce-durable"); + // Before reset: matching ack passes. + assert.equal(_testNonceGate(PUBKEY, "nonce-durable"), true); + + // Reset clears the in-memory map, then immediately restores from localStorage. + // An ack arriving after reset (e.g. a remote agent ack replayed after + // community switch) should still be accepted. + resetAgentObserverStore(); + assert.equal( + _testNonceGate(PUBKEY, "nonce-durable"), + true, + "post-reset: matching nonce must pass (restored from localStorage)", + ); + // A nonce-less ack must still be rejected (nonce is registered in storage). + assert.equal( + _testNonceGate(PUBKEY, undefined), + false, + "post-reset: nonce-less ack must be rejected when nonce is in storage", + ); +}); + +test("Desktop restart simulation — nonce persists, matching ack accepted", () => { + setup(); + registerEffortNonce(PUBKEY, "nonce-restart"); + + // Simulate Desktop restart: clear the in-memory map without touching + // localStorage (the process dies and a new one starts, which calls + // loadNoncesFromStorage on module init). Here we simulate that init by + // calling _testClearEffortNonceStorage first to prove it's the storage that + // provides the restoration, then calling fullReset + re-init via reset. + // + // Pattern: clear in-memory only, then reset (which calls loadNoncesFromStorage). + resetAgentObserverStore(); + // localStorage still has the nonce — gate must accept the matching ack. + assert.equal( + _testNonceGate(PUBKEY, "nonce-restart"), + true, + "restart-simulation: matching nonce from localStorage must pass the gate", + ); +}); + +test("full reset (in-memory + storage) restores startup path", () => { + setup(); + registerEffortNonce(PUBKEY, "nonce-registered"); + // Before clear: nonce-less ack is blocked. + assert.equal(_testNonceGate(PUBKEY, undefined), false); + + // Clear both in-memory and storage — simulates a full logout/re-auth. + fullReset(); + // After full reset: no nonce registered → startup path active again. + assert.equal( + _testNonceGate(PUBKEY, undefined), + true, + "full reset: startup path must be restored after clearing both memory and storage", + ); +}); diff --git a/desktop/src/features/agents/observerRelayStore.ts b/desktop/src/features/agents/observerRelayStore.ts index 611fdd489d..8ceb0a8288 100644 --- a/desktop/src/features/agents/observerRelayStore.ts +++ b/desktop/src/features/agents/observerRelayStore.ts @@ -3,7 +3,10 @@ import * as React from "react"; import { subscribeToAgentObserverFrames } from "@/shared/api/observerRelay"; import type { RelayEvent, ManagedAgent } from "@/shared/api/types"; import type { ControlResultFrame } from "@/shared/api/types"; -import { putAgentSessionConfig } from "@/shared/api/tauri"; +import { + putAgentSessionConfig, + persistAgentEffortLevel, +} from "@/shared/api/tauri"; import { putManagedAgentRuntimeLifecycle } from "@/shared/api/tauriManagedAgents"; import { getIdentity } from "@/shared/api/tauriIdentity"; import { decryptObserverEvent } from "@/shared/api/tauriObserver"; @@ -169,6 +172,53 @@ let startPromise: Promise | null = null; let eventProcessingQueue: Promise = Promise.resolve(); let generation = 0; +/** + * Tracks the most recently dispatched nonce per agent. The observer persistence + * gate for effort acks checks this: only acks whose nonce matches the current + * entry are persisted. This prevents a stale ack from a superseded pick (or a + * late result after an 8s timeout) from overwriting a newer persisted value. + * + * Key: normalized agent pubkey. Value: nonce string from the last dispatch. + * + * This map is the authoritative in-memory view. It is populated from + * `localStorage` on module load and on every `resetAgentObserverStore` call so + * that legitimate acks survive Desktop restart or community switch. + */ +const currentEffortNonce = new Map(); + +/** + * `localStorage` key prefix for persisted effort nonces. + * Full key: `buzz:effort-nonce:`. + */ +const EFFORT_NONCE_KEY_PREFIX = "buzz:effort-nonce:"; + +/** + * Load any previously-persisted effort nonces from `localStorage` into the + * in-memory map. Best-effort: errors are silently ignored so a corrupted + * entry never blocks startup. + */ +function loadNoncesFromStorage(): void { + try { + const storage = globalThis.localStorage; + if (!storage) return; + for (let i = 0; i < storage.length; i++) { + const key = storage.key(i); + if (key?.startsWith(EFFORT_NONCE_KEY_PREFIX)) { + const pubkey = key.slice(EFFORT_NONCE_KEY_PREFIX.length); + const nonce = storage.getItem(key); + if (pubkey && nonce) { + currentEffortNonce.set(pubkey, nonce); + } + } + } + } catch { + // localStorage unavailable (e.g., test environment without a mock) — ignore. + } +} + +// Populate from storage on module init so persisted nonces survive restart. +loadNoncesFromStorage(); + function notifyListeners() { for (const listener of listeners) { listener(); @@ -525,10 +575,64 @@ function isControlResultFrame(payload: unknown): payload is ControlResultFrame { ); } +/** + * Pure predicate: returns `true` when `ackNonce` matches the registered nonce + * for `agentPubkey` in the current `currentEffortNonce` map state. + * + * Rules (P3 nonce gate): + * - No nonce ever registered for this agent (startup path): ack-nonce must + * also be absent. + * - A nonce is registered: ack-nonce must be present and equal to it. + * + * Used by both `dispatchControlResult` (production gate) and the nonce-gate + * test helpers, ensuring tests exercise the same logic as production. + */ +function effortNonceMatches(agentPubkey: string, ackNonce: unknown): boolean { + const registered = currentEffortNonce.get(normalizePubkey(agentPubkey)); + return registered === undefined + ? ackNonce === undefined + : ackNonce !== undefined && ackNonce === registered; +} + function dispatchControlResult(agentPubkey: string, payload: unknown) { if (!isControlResultFrame(payload)) { return; } + // B5: on a positive set_config_option ack for a confirmed thought_level option, + // persist the canonical value. Two persistence triggers: + // 1. status === "ok" + category === "thought_level": terminal applied ack from + // `pool.resolve_effort_report` (main loop `PoolEvent::EffortReport` arm), + // emitted pre-prompt — the adapter accepted the value. + // 2. status === "cleared" + category === "thought_level": terminal clear ack + // from `pool.resolve_effort_report` — session ran without effort override. + // Gate on `category === "thought_level"` (present only on thought_level acks) + // so synthetic acks (no category) never trigger persistence. + // Gate on nonce: if the harness echoes a nonce, it must match the most recently + // dispatched nonce for this agent. Mismatches indicate stale results from + // superseded picks (e.g. old `ok` arriving after a newer pick has been sent, + // or a late final ack after the 8s timeout). + if ( + payload.type === "set_config_option" && + payload.category === "thought_level" + ) { + const ackNonce = (payload as Record).nonce; + const nonceOk = effortNonceMatches(agentPubkey, ackNonce); + if (nonceOk) { + if (payload.status === "ok") { + void persistAgentEffortLevel(agentPubkey, payload.value || null).catch( + (err: unknown) => { + console.warn("Failed to persist effort level:", err); + }, + ); + } else if (payload.status === "cleared") { + void persistAgentEffortLevel(agentPubkey, null).catch( + (err: unknown) => { + console.warn("Failed to clear effort level:", err); + }, + ); + } + } + } const subscribers = controlResultListeners.get(normalizePubkey(agentPubkey)); if (!subscribers) { return; @@ -572,6 +676,26 @@ export function subscribeControlResults( }; } +/** + * Register the nonce for the most recently dispatched effort pick/clear for a + * given agent. The persistence gate in `dispatchControlResult` checks this: only + * `ok`/`cleared` acks whose echoed nonce matches the registered one are persisted. + * This prevents stale results from superseded picks from clobbering newer values. + * + * The registration is persisted to `localStorage` (keyed by normalized pubkey) + * so it survives Desktop restart and community-switch store resets. The gate + * therefore correctly rejects replayed acks even after a full restart. + */ +export function registerEffortNonce(agentPubkey: string, nonce: string): void { + const key = normalizePubkey(agentPubkey); + currentEffortNonce.set(key, nonce); + try { + globalThis.localStorage?.setItem(EFFORT_NONCE_KEY_PREFIX + key, nonce); + } catch { + // localStorage full or unavailable — in-memory registration still holds. + } +} + export function getAgentObserverSnapshot( agentPubkey?: string | null, // `_enabled` previously gated store reads — now only gates the relay @@ -785,6 +909,12 @@ export function resetAgentObserverStore() { onSessionConfigCaptured = null; connectionState = "idle"; errorMessage = null; + // Clear the in-memory nonce map, then immediately restore from localStorage. + // This ensures an in-flight ack that arrives after a community switch or + // store reset is still validated against the registered nonce rather than + // treated as a stale startup ack. + currentEffortNonce.clear(); + loadNoncesFromStorage(); notifyListeners(); void unsubscribe?.(); } @@ -814,3 +944,48 @@ export function _testGetArchivedChannelEvents( archiveEventsByChannel.get(archiveChannelKey(agentPubkey, channelId)) ?? [] ); } + +/** + * Test-only: evaluate the persistence nonce gate for a given agent and ack + * nonce against the current `currentEffortNonce` map state. + * + * Delegates to the production `effortNonceMatches` predicate — tests exercise + * the same logic that runs in `dispatchControlResult`. + * + * Use `registerEffortNonce` to prime state before calling, and + * `resetAgentObserverStore` + `_testClearEffortNonceStorage` to clean up + * between tests. + * Only call from tests — never from production code. + */ +export function _testNonceGate( + agentPubkey: string, + ackNonce: unknown, +): boolean { + return effortNonceMatches(agentPubkey, ackNonce); +} + +/** + * Test-only: remove all `buzz:effort-nonce:*` entries from `localStorage` so + * the nonce gate's startup path is fully reset between tests. Call alongside + * `resetAgentObserverStore` when a test needs a clean-slate simulation of + * Desktop restart. + * Only call from tests — never from production code. + */ +export function _testClearEffortNonceStorage(): void { + try { + const storage = globalThis.localStorage; + if (!storage) return; + const keysToRemove: string[] = []; + for (let i = 0; i < storage.length; i++) { + const key = storage.key(i); + if (key?.startsWith(EFFORT_NONCE_KEY_PREFIX)) { + keysToRemove.push(key); + } + } + for (const key of keysToRemove) { + storage.removeItem(key); + } + } catch { + // Best-effort. + } +} diff --git a/desktop/src/features/agents/ui/AgentConfigPanel.tsx b/desktop/src/features/agents/ui/AgentConfigPanel.tsx index 67c544257c..5b0523e254 100644 --- a/desktop/src/features/agents/ui/AgentConfigPanel.tsx +++ b/desktop/src/features/agents/ui/AgentConfigPanel.tsx @@ -13,7 +13,12 @@ import { PenOff, Server, } from "lucide-react"; -import { useAgentConfigSurface } from "../hooks"; +import { useQueryClient } from "@tanstack/react-query"; +import { + useAgentConfigSurface, + managedAgentsQueryKey, + agentConfigSurfaceQueryKey, +} from "../hooks"; import { cn } from "@/shared/lib/cn"; import { copyTextToClipboard } from "@/shared/lib/clipboard"; import { Spinner } from "@/shared/ui/spinner"; @@ -26,6 +31,12 @@ import type { NormalizedField, } from "@/shared/api/types"; import { providerDisplayLabel } from "./agentConfigOptions"; +import { sendSetConfigOption } from "@/shared/api/agentControl"; +import { + subscribeControlResults, + registerEffortNonce, +} from "@/features/agents/observerRelayStore"; +import { awaitEffortOutcome } from "@/features/agents/lib/effortOutcome"; type Props = { pubkey: string; @@ -345,6 +356,148 @@ function AdvancedRow({ return
{content}
; } +// ── Claude effort picker (B5) ──────────────────────────────────────────────── +// +// Renders a live effort control for claude runtimes when the session-level +// `thought_level` configId is available (i.e. at least one session has been +// created). Calls `sendSetConfigOption` so the harness forwards the change to +// the adapter via `session/set_config_option`. +// +// The picker subscribes to `control_result` BEFORE sending and awaits the +// correlated final result (ok / failure / invalid_value / cleared / timeout +// resolved as pending_session). Persistence is handled by the observer store +// on the `ok` and `cleared` acks — the picker only drives UI state. +// +// I-7: option values come from the adapter-advertised `effortOptions` rather +// than a hardcoded list, so model-specific option sets are reflected correctly. + +function EffortPicker({ + pubkey, + effortConfigId, + currentEffort, + effortOptions, +}: { + pubkey: string; + effortConfigId: string; + currentEffort: string | null; + effortOptions: Array<{ value: string; displayName?: string }>; +}) { + const [saving, setSaving] = React.useState(false); + const [statusMsg, setStatusMsg] = React.useState<{ + kind: "info" | "error"; + text: string; + } | null>(null); + const queryClient = useQueryClient(); + + const handleChange = async (value: string) => { + setSaving(true); + setStatusMsg(null); + // Generate a per-request nonce so the harness can echo it in all acks and + // the Desktop can reject stale results from superseded picks. + const nonce = crypto.randomUUID(); + // Register with the store so the global persistence gate can validate. + registerEffortNonce(pubkey, nonce); + try { + const outcome = await awaitEffortOutcome({ + configId: effortConfigId, + value, + nonce, + subscribe: (listener) => subscribeControlResults(pubkey, listener), + send: async () => { + await sendSetConfigOption( + pubkey, + effortConfigId, + value, + "thought_level", + nonce, + ); + }, + scheduleTimeout: (onTimeout) => { + const id = window.setTimeout(onTimeout, 8_000); + return () => window.clearTimeout(id); + }, + }); + + if (outcome === "ok" || outcome === "cleared") { + // Observer already persisted. Invalidate both managed-agents (record + // snapshot) and the config surface (source of currentEffort). + void queryClient.invalidateQueries({ + queryKey: managedAgentsQueryKey, + }); + void queryClient.invalidateQueries({ + queryKey: agentConfigSurfaceQueryKey(pubkey), + }); + setStatusMsg(null); + } else if (outcome === "pending_session") { + setStatusMsg({ kind: "info", text: "Applies at next session" }); + } else if (outcome === "failure") { + setStatusMsg({ kind: "error", text: "Adapter rejected — try again" }); + } else if (outcome === "invalid_value") { + setStatusMsg({ kind: "error", text: "Value not supported by adapter" }); + } + } catch (err) { + setStatusMsg({ + kind: "error", + text: err instanceof Error ? err.message : String(err), + }); + } finally { + setSaving(false); + } + }; + + // Fall back to low/medium/high when the adapter advertises no options (older + // adapters that support thought_level but predate the options field). + const options = + effortOptions.length > 0 + ? effortOptions + : [ + { value: "low", displayName: "Low" }, + { value: "medium", displayName: "Medium" }, + { value: "high", displayName: "High" }, + ]; + + return ( +
+

+ + Thinking / Effort +

+
+ + {saving ? ( + Setting… + ) : null} + {statusMsg ? ( + + {statusMsg.text} + + ) : null} +
+

+ Live — persisted after agent acknowledges +

+
+ ); +} + // ── Main component ──────────────────────────────────────────────────────────── export function AgentConfigPanel({ @@ -373,8 +526,17 @@ export function AgentConfigPanel({ ); } - const { normalized, advanced, extensions, runtimeId, sources, isPreSpawn } = - data; + const { + normalized, + advanced, + extensions, + runtimeId, + sources, + isPreSpawn, + claudeConfigDirCustom, + effortConfigId, + effortOptions = [], + } = data; const configFilePath = sources.configFilePath; const normalizedEntries = ( @@ -475,6 +637,32 @@ export function AgentConfigPanel({ ) : null} ) : null} + + {claudeConfigDirCustom ? ( +
+

+ ⚠ Custom{" "} + CLAUDE_CONFIG_DIR active + — config is read from that directory. Note: Claude Code keys its + login to the config-dir path, so a custom dir creates a new Keychain + namespace. The agent will need to re-authenticate unless you also + set{" "} + + CLAUDE_SECURESTORAGE_CONFIG_DIR + {" "} + to match your default login. +

+
+ ) : null} + + {effortConfigId ? ( + + ) : null} ); } diff --git a/desktop/src/shared/api/agentControl.ts b/desktop/src/shared/api/agentControl.ts index 677f0ffad4..dcb485788b 100644 --- a/desktop/src/shared/api/agentControl.ts +++ b/desktop/src/shared/api/agentControl.ts @@ -29,3 +29,34 @@ export async function switchManagedAgentModel( modelId, }); } + +/** + * Send a `set_config_option` control frame to a running agent. The harness + * acknowledges via a `control_result` observer frame with `type: + * "set_config_option"`. The caller uses this ack to persist the canonical + * value (e.g. `effort_level`) so it takes effect on the next agent spawn. + * + * Pass `category` when the caller knows the option category (e.g. + * `"thought_level"` for effort picks). The harness uses it as a trust signal + * during the pre-discovery window (before the first session/new response), + * ensuring picks during the first turn are stored rather than silently dropped. + * + * Pass `nonce` to enable per-request correlation. The harness echoes the nonce + * in all acks (immediate and final) so the Desktop can reject stale results + * from superseded picks without relying on value equality alone. + */ +export async function sendSetConfigOption( + pubkey: string, + configId: string, + value: string, + category?: string, + nonce?: string, +): Promise { + await sendAgentObserverControl(pubkey, { + type: "set_config_option", + configId, + value, + ...(category !== undefined ? { category } : {}), + ...(nonce !== undefined ? { nonce } : {}), + }); +} diff --git a/desktop/src/shared/api/tauri.ts b/desktop/src/shared/api/tauri.ts index 6e29f77c14..4ca5dc0088 100644 --- a/desktop/src/shared/api/tauri.ts +++ b/desktop/src/shared/api/tauri.ts @@ -218,18 +218,15 @@ type RawGitBashPrerequisite = { install_instructions_url: string; install_hint: string; }; - type RawCommandAvailability = { command: string; resolved_path: string | null; available: boolean; }; - type RawManagedAgentPrereqs = { acp: RawCommandAvailability; mcp: RawCommandAvailability; }; - type RawRelayMember = { pubkey: string; role: string; @@ -240,7 +237,6 @@ type RawRelayMember = { type RawListRelayMembersResponse = { members: RawRelayMember[]; }; - type RawCanvasResponse = { content: string | null; updated_at: number | null; @@ -1015,7 +1011,11 @@ export async function putAgentSessionConfig( ): Promise { return invokeTauri("put_agent_session_config", { pubkey, payload }); } - +export const persistAgentEffortLevel = (p: string, l: string | null) => + invokeTauri("persist_agent_effort_level", { + pubkey: p, + effortLevel: l, + }); /** File-layer config for a runtime (e.g. `~/.config/goose/config.yaml`). */ export type RuntimeFileConfigSubset = { /** Provider set in the harness config file. */ diff --git a/desktop/src/shared/api/types.ts b/desktop/src/shared/api/types.ts index 24ef625783..8108fce3d1 100644 --- a/desktop/src/shared/api/types.ts +++ b/desktop/src/shared/api/types.ts @@ -339,18 +339,9 @@ export type ManagedAgent = { modelSource: "definition" | "global" | "instance_legacy" | null; /** LLM inference provider, from the agent's pinned record snapshot. */ provider: string | null; - /** - * `true` when the linked persona has been edited since this agent was - * created — the running agent uses the older pinned snapshot. Surface a - * "out of date" marker and prompt the user to delete + respawn to update. - * Always `false` for non-persona agents and for orphaned agents. - */ + /** True when the linked persona has been edited since this agent was created. */ personaOutOfDate: boolean; - /** - * `true` when the agent's linked persona no longer exists. Distinct from - * out-of-date: there is no current persona to respawn into, so do not prompt - * a respawn — the pinned snapshot is all the config that remains. - */ + /** True when this agent's linked persona no longer exists. */ personaOrphaned: boolean; /** * `true` when the running process was spawned with a config that no longer @@ -461,12 +452,7 @@ export type CancelManagedAgentTurnResult = { status: "sent" | "no_active_turn"; }; -/** - * Outcome of a live `switch_model` control frame, surfaced asynchronously via - * the agent's `control_result` observer frame. Busy path: `sent` (cancel + - * requeue on the new model) or `turn_ending` (oneshot already consumed this - * turn). Idle path: `switched`, `unsupported_model`, or `no_active_turn`. - */ +/** Outcome of a live `switch_model` control frame (`control_result` observer). */ export type SwitchManagedAgentModelStatus = | "sent" | "turn_ending" @@ -474,12 +460,18 @@ export type SwitchManagedAgentModelStatus = | "unsupported_model" | "no_active_turn"; -export type ControlResultFrame = { - type: "cancel_turn" | "switch_model"; +export type SetConfigOptionResult = { + type: "set_config_option"; status: string; - modelId?: string; + configId: string; + value: string; + category?: "thought_level"; }; +export type ControlResultFrame = + | { type: "cancel_turn" | "switch_model"; status: string; modelId?: string } + | SetConfigOptionResult; + export type GitBashPrerequisite = { available: boolean; path: string | null; @@ -655,7 +647,12 @@ export type ConfigSourceReport = { mcpConfigFilePath: string | null; }; -export type ExtensionEntry = { name: string; kind: string; enabled: boolean }; +export type ExtensionEntry = { + name: string; + kind: string; + enabled: boolean; + source?: string; +}; export type NormalizedConfig = { model: NormalizedField | null; @@ -675,6 +672,9 @@ export type RuntimeConfigSurface = { advanced: ConfigField[]; extensions: ExtensionEntry[]; sources: ConfigSourceReport; + claudeConfigDirCustom?: boolean; + effortConfigId?: string; + effortOptions?: Array<{ value: string; displayName?: string }>; }; export type UpdateManagedAgentInput = {