diff --git a/crates/buzz-acp/README.md b/crates/buzz-acp/README.md index e6164b02dd..2200d4345d 100644 --- a/crates/buzz-acp/README.md +++ b/crates/buzz-acp/README.md @@ -154,10 +154,15 @@ The gate applies to **all** inbound events — @mentions, DMs, thread replies, a | `!shutdown` | Gracefully exits the harness. | | `!cancel` | Cancels the current in-flight turn for that channel, if any. | | `!rotate` | Rotates the ACP session for that channel. If a turn is in-flight, it is cancelled and the channel session is invalidated when the task returns; otherwise the cached idle session is invalidated immediately. The next queued/received event starts a fresh session. | +| `!model [id]` | Switches the model backing that channel and replies in-channel. If a turn is in-flight, it is cancelled and re-run on the new model; otherwise the model applies on the next turn. With no `id`, lists the available models. | Use `!cancel` to stop only the current turn; it is a no-op when the channel is idle. Use `!rotate` when you want the next turn in the channel to start from a fresh ACP session, even if the channel is currently idle. -Owner control commands must be kind:9 stream messages from the owner, must mention this agent with a `p` tag, and are consumed by the harness instead of being forwarded to the agent. +`!model` matches model IDs exactly — adapters ship near-identical pairs (`opus[1m]` vs `claude-opus-5`, `gpt-5.3-codex` vs `gpt-5.3-codex/low`) where a prefix match would silently pick a different context lane and price point. An unknown ID is rejected with the list and leaves the current model untouched. + +Owner control commands must be kind:9 stream messages from the owner, must mention this agent with a `p` tag, and are consumed by the harness instead of being forwarded to the agent. The command may be typed on its own (`!rotate`) or immediately after the leading `@mention` tokens (`@Agent !rotate`, `@Sol @Eva !rotate`). + +The bang must be at that command position. A bang appearing later in the message is ordinary text, so asking an agent *about* a command — `@Agent what happens if I use !shutdown` — is delivered as a normal message rather than executed. Since the harness has no display-name list, a mention holding spaces (`@Will Pfleger !rotate`) does not reach command position either; address such an agent with a single-token handle to use control commands. > **Note:** The default mode is `owner-only`. Agents without a registered `agent_owner_pubkey` will not respond to any events until the owner is resolved. Set `--respond-to anyone` to disable the gate entirely. diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index c0147baf1b..c85010efaa 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -1869,6 +1869,30 @@ pub fn extract_model_config_options(result: &serde_json::Value) -> Vec Option<&str> { + config_opt + .get("configId") + .or_else(|| config_opt.get("id")) + .and_then(|v| v.as_str()) +} + +/// Human-readable label of a `configOptions` entry or one of its options. +/// +/// The schema field is `name`; `displayName` is a pre-standardization spelling +/// still emitted by some adapters. +pub fn config_option_label(value: &serde_json::Value) -> Option<&str> { + value + .get("name") + .or_else(|| value.get("displayName")) + .and_then(|v| v.as_str()) +} + /// Extract `SessionModelState` (unstable path) from a `session/new` result. /// /// Returns the `models` object if present: `{ currentModelId, availableModels: [...] }`. @@ -1889,14 +1913,7 @@ pub fn resolve_model_switch_method( // 1. Search stable configOptions for a "model"-category entry whose // options contain a value matching desired_model. for config_opt in extract_model_config_options(session_new_result) { - // Adapters disagree on the key: the ACP spec says `configId`, but - // claude-agent-acp emits `id`. Accept both; the set request always - // uses `configId` on the wire. - let config_id = match config_opt - .get("configId") - .or_else(|| config_opt.get("id")) - .and_then(|v| v.as_str()) - { + let config_id = match config_option_id(&config_opt) { Some(id) => id, None => continue, }; @@ -2571,6 +2588,43 @@ mod tests { ); } + #[test] + fn resolve_finds_config_options_only_model() { + // codex-acp shape: the halves disagree — configOptions offers clean ids + // while availableModels offers reasoning-suffixed ones. Only the stable + // half can serve `gpt-5.4`. + let result = serde_json::json!({ + "configOptions": [{ + "id": "model", + "category": "model", + "currentValue": "gpt-5.4", + "options": [{ "value": "gpt-5.4", "name": "GPT-5.4" }] + }], + "models": { + "currentModelId": "gpt-5.3-codex/medium", + "availableModels": [{ "modelId": "gpt-5.3-codex/medium" }] + } + }); + assert_eq!( + super::resolve_model_switch_method(&result, "gpt-5.4"), + Some(super::ModelSwitchMethod::ConfigOption { + config_id: "model".to_string(), + option_value: "gpt-5.4".to_string(), + }) + ); + } + + #[test] + fn config_option_label_prefers_schema_name() { + let schema = serde_json::json!({ "name": "Haiku", "displayName": "stale" }); + assert_eq!(super::config_option_label(&schema), Some("Haiku")); + + let legacy = serde_json::json!({ "displayName": "Haiku" }); + assert_eq!(super::config_option_label(&legacy), Some("Haiku")); + + assert_eq!(super::config_option_label(&serde_json::json!({})), None); + } + // ── model_in_catalog tests ──────────────────────────────────────────── #[test] diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index b11d96d8f7..4c5c2c5edb 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -838,6 +838,7 @@ fn handle_relay_observer_control_event( keys: &nostr::Keys, event: nostr::Event, pool: &mut AgentPool, + catalog: &pool::ModelCatalog, observer: Option<&observer::ObserverHandle>, owner_pubkey_hex: &str, ) { @@ -883,7 +884,7 @@ fn handle_relay_observer_control_event( handle_cancel_turn_control(&payload, pool, observer); } Some("switch_model") => { - handle_switch_model_control(&payload, pool, observer); + handle_switch_model_control(&payload, pool, catalog, observer); } _ => { tracing::debug!(payload = %payload, "ignoring unknown observer control frame"); @@ -926,34 +927,73 @@ fn handle_cancel_turn_control( } } -/// Handle a `switch_model` control frame (Phase 3a, Option ii). +/// Outcome of [`switch_model_for_channel`]. +#[derive(Debug, PartialEq, Eq)] +enum SwitchOutcome { + /// Delivered over the in-flight turn's control oneshot. + Sent, + /// The in-flight turn is already ending — the switch could not land on it. + TurnEnding, + /// Applied to an idle agent; takes effect on its next turn. + Switched, + /// Not in the process catalog — pick rejected, session untouched. + UnsupportedModel, + /// The process catalog has not been captured yet, so nothing can be + /// validated against — pick deferred, session untouched. + CatalogUnavailable, + /// No turn in flight and no idle agent holds a session for the channel. + NoActiveTurn, +} + +impl SwitchOutcome { + /// `control_result` status string. Consumed by the desktop's model picker — + /// do not change these without updating it. + /// + /// `CatalogUnavailable` reports as `unsupported_model` rather than adding a + /// status: both mean "pick rejected, session untouched", which is all the + /// picker acts on. It is also unreachable from the desktop, whose model + /// list is populated from this same catalog — with no catalog there is + /// nothing to pick. Only `!model`, which can be typed before the first + /// session binds, distinguishes the two, and it does so on the variant. + fn status(&self) -> &'static str { + match self { + Self::Sent => "sent", + Self::TurnEnding => "turn_ending", + Self::Switched => "switched", + Self::UnsupportedModel | Self::CatalogUnavailable => "unsupported_model", + Self::NoActiveTurn => "no_active_turn", + } + } +} + +/// Switch the model backing `channel_id` (Phase 3a, Option ii). +/// +/// Pre-cancel guard: every pick is validated against the process-wide catalog +/// before anything is disturbed, on both paths. No catalog means nothing to +/// validate against, so the pick is deferred rather than passed through: the +/// window where the catalog is missing is the first turn's `session/new`, which +/// is already registered in flight, so passing an unvalidated pick through +/// there cancels and requeues a live turn on the strength of a possible typo — +/// and the fresh session then falls back to the unchanged model anyway. /// /// 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). +/// re-runs on a fresh session under the new model. /// -/// 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. -fn handle_switch_model_control( - payload: &serde_json::Value, +/// Idle path: set `desired_model` + invalidate the channel's session. The +/// override takes visible effect on the agent's next turn. +fn switch_model_for_channel( pool: &mut AgentPool, - observer: Option<&observer::ObserverHandle>, -) { - let Some(channel_id) = payload - .get("channelId") - .and_then(|value| value.as_str()) - .and_then(|value| value.parse::().ok()) - else { - tracing::warn!("observer switch_model control frame missing valid channelId"); - return; - }; - let Some(model_id) = payload.get("modelId").and_then(|value| value.as_str()) else { - tracing::warn!("observer switch_model control frame missing modelId"); - return; + catalog: Option<&pool::AgentModelCapabilities>, + channel_id: Uuid, + model_id: &str, +) -> SwitchOutcome { + let Some(caps) = catalog else { + return SwitchOutcome::CatalogUnavailable; }; + if !caps.contains(model_id) { + return SwitchOutcome::UnsupportedModel; + } // 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 @@ -963,7 +1003,7 @@ fn handle_switch_model_control( .values() .any(|m| m.channel_id == Some(channel_id)); - let status = if turn_in_flight { + 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. @@ -972,18 +1012,41 @@ fn handle_switch_model_control( channel_id, ControlSignal::SwitchModel(model_id.to_string()), ) { - "sent" + SwitchOutcome::Sent } else { - "turn_ending" + SwitchOutcome::TurnEnding } } else { - // Idle path: validate against the cached catalog before invalidating. match pool.switch_idle_agent_model(channel_id, model_id) { - IdleSwitchResult::Switched => "switched", - IdleSwitchResult::UnsupportedModel => "unsupported_model", - IdleSwitchResult::NoIdleAgent => "no_active_turn", + IdleSwitchResult::Switched => SwitchOutcome::Switched, + IdleSwitchResult::NoIdleAgent => SwitchOutcome::NoActiveTurn, } + } +} + +/// Handle a `switch_model` control frame: the kind:24200 front door onto +/// [`switch_model_for_channel`] (`!model` is the chat one). +fn handle_switch_model_control( + payload: &serde_json::Value, + pool: &mut AgentPool, + catalog: &pool::ModelCatalog, + observer: Option<&observer::ObserverHandle>, +) { + let Some(channel_id) = payload + .get("channelId") + .and_then(|value| value.as_str()) + .and_then(|value| value.parse::().ok()) + else { + tracing::warn!("observer switch_model control frame missing valid channelId"); + return; }; + let Some(model_id) = payload.get("modelId").and_then(|value| value.as_str()) else { + tracing::warn!("observer switch_model control frame missing modelId"); + return; + }; + + let status = + switch_model_for_channel(pool, catalog.get().as_ref(), channel_id, model_id).status(); if let Some(observer) = observer { observer.emit( @@ -1004,6 +1067,91 @@ fn handle_switch_model_control( } } +/// Handle the owner's `!model [id]` chat command: the chat front door onto +/// [`switch_model_for_channel`] (kind:24200 `switch_model` is the other). +/// +/// Returns the reply to post back into the channel. With no argument — or on a +/// miss — it renders the catalog, so the "helpful list" is also the entire +/// error-recovery story. Matching is on exact IDs: adapters ship near-identical +/// pairs (`opus[1m]` vs `claude-opus-5`, `gpt-5.3-codex` vs +/// `gpt-5.3-codex/low`), so any prefix rule would silently pick a different +/// context lane and price point. +fn handle_model_command( + pool: &mut AgentPool, + catalog: &pool::ModelCatalog, + channel_id: Uuid, + model_id: &str, +) -> String { + // The channel's current model: an owner override if one is set on the agent + // serving this channel, else whatever the adapter last reported as selected. + // Snapshotted before the switch below takes the pool mutably. + let caps = catalog.get(); + let listing = caps.as_ref().and_then(|caps| { + let current = pool + .channel_model_override(channel_id) + .or_else(|| caps.reported_current()); + render_model_catalog(caps, current) + }); + + if model_id.is_empty() { + return listing.unwrap_or_else(|| { + "I don't know my model list yet — ask again in a moment.".to_string() + }); + } + + match switch_model_for_channel(pool, caps.as_ref(), channel_id, model_id) { + SwitchOutcome::Sent => { + format!("Switching to `{model_id}` — restarting the current turn on the new model.") + } + SwitchOutcome::Switched => { + format!("Model set to `{model_id}`. Takes effect on my next turn.") + } + SwitchOutcome::TurnEnding => { + "The current turn is already ending — re-send once it finishes.".to_string() + } + SwitchOutcome::UnsupportedModel => match listing { + Some(listing) => format!("`{model_id}` isn't one of my models.\n\n{listing}"), + None => format!("`{model_id}` isn't one of my models."), + }, + // Nothing to validate the pick against yet, so it was not applied. The + // no-argument branch above says the same thing for the same reason. + SwitchOutcome::CatalogUnavailable => { + "I don't know my model list yet — ask again in a moment.".to_string() + } + // No agent holds a session for this channel, so there is nothing to + // switch. Say so rather than claiming a switch that never happened. + SwitchOutcome::NoActiveTurn => { + "I have no session for this channel yet — message me first, then `!model`.".to_string() + } + } +} + +/// Render an agent's model catalog as chat markdown, marking `current`. +/// `None` when the agent exposes no selectable models. +fn render_model_catalog( + caps: &pool::AgentModelCapabilities, + current: Option<&str>, +) -> Option { + let models = caps.models(); + if models.is_empty() { + return None; + } + let mut listing = String::from("My models:"); + for (id, label) in models { + let label = match label.filter(|label| *label != id) { + Some(label) => format!(" — {label}"), + None => String::new(), + }; + let marker = if Some(id) == current { + " ← current" + } else { + "" + }; + listing.push_str(&format!("\n- `{id}`{label}{marker}")); + } + Some(listing) +} + /// 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. @@ -1561,6 +1709,7 @@ async fn tokio_main() -> Result<()> { memory_enabled: config.memory_enabled, harness_name: crate::config::normalize_agent_command_identity(&config.agent_command), relay_url: config.relay_url.clone(), + model_catalog: pool::ModelCatalog::default(), }); if !config.memory_enabled { @@ -1791,7 +1940,6 @@ async fn tokio_main() -> Result<()> { index: rr.index, acp, state: SessionState::default(), - model_capabilities: None, desired_model: config.model.clone(), model_overridden: false, agent_name, @@ -1890,7 +2038,7 @@ async fn tokio_main() -> Result<()> { match control_event { Some(event) => { if let Some(ref owner_hex) = owner_cache.pubkey { - handle_relay_observer_control_event(&config.keys, event, &mut pool, observer.as_ref(), owner_hex); + handle_relay_observer_control_event(&config.keys, event, &mut pool, &ctx.model_catalog, observer.as_ref(), owner_hex); } else { tracing::warn!("observer control frame received but no owner resolved — dropping"); } @@ -2030,107 +2178,97 @@ async fn tokio_main() -> Result<()> { continue; } - // Check: kind:9, content "!shutdown", from owner, mentions THIS agent. - let is_shutdown = is_owner_control_command( - &buzz_event.event, - kind_u32, - "!shutdown", - &pubkey_hex, - ); - if is_shutdown { - let owner = owner_cache.get(); - if let Some(owner) = owner { - if buzz_event.event.pubkey.to_hex() == *owner { - tracing::info!( - channel_id = %buzz_event.channel_id, - sender = %buzz_event.event.pubkey.to_hex(), - "shutdown command from owner — exiting gracefully" - ); - let _ = shutdown_tx.send(()); - continue; - } - } - // Not from owner — fall through to normal prompt handling. - // Don't drop it — it's a regular message that happens to - // contain "!shutdown" from a non-owner. - } - - // Mirrors !shutdown: kind:9, content "!cancel", from - // owner, mentions THIS agent. Must be BEFORE + // Owner control commands: kind:9 mentioning THIS + // agent, from the owner. Consumed by the harness + // and never forwarded to the agent. Must be BEFORE // queue.push() — the event content is moved by push. // - // Mode-independent: !cancel fires regardless of - // --multiple-event-handling. It is explicit user + // Mode-independent: they fire regardless of + // --multiple-event-handling. They are explicit user // intent, not an automatic policy decision. - let is_cancel = is_owner_control_command( - &buzz_event.event, - kind_u32, - "!cancel", - &pubkey_hex, - ); - if is_cancel { - if let Some(owner) = owner_cache.get() { - if buzz_event.event.pubkey.to_hex() == *owner { - let fired = signal_in_flight_task( - &mut pool, - buzz_event.channel_id, - ControlSignal::Cancel, + // + // An unrecognized command, an arity mismatch, or a + // non-owner author all fall through to normal prompt + // handling — a stranger typing "!cancel" is still + // sending the agent a message. + let command = owner_control_command(&buzz_event.event, kind_u32, &pubkey_hex) + .filter(|_| owner_cache.get() == Some(buzz_event.event.pubkey.to_hex().as_str())); + match command { + Some(("!shutdown", "")) => { + tracing::info!( + channel_id = %buzz_event.channel_id, + sender = %buzz_event.event.pubkey.to_hex(), + "shutdown command from owner — exiting gracefully" + ); + let _ = shutdown_tx.send(()); + continue; + } + Some(("!cancel", "")) => { + let fired = signal_in_flight_task( + &mut pool, + buzz_event.channel_id, + ControlSignal::Cancel, + ); + if !fired { + tracing::warn!( + channel_id = %buzz_event.channel_id, + "!cancel received but no in-flight task — no-op" ); - if !fired { - tracing::warn!( - channel_id = %buzz_event.channel_id, - "!cancel received but no in-flight task — no-op" - ); - } - continue; // consume event — do NOT push to queue } + continue; // consume event — do NOT push to queue } - // Not from owner — fall through to normal prompt handling. - } - - // Mirrors !shutdown / !cancel: kind:9, content - // "!rotate", from owner, mentions THIS agent. - // - // Rotation is explicit owner intent to start the - // next turn in this channel with a fresh ACP - // session. It is consumed by the harness and never - // forwarded to the agent. If a turn is in-flight, - // cancel it, drop its triggering batch, and - // invalidate the channel session when the task - // returns. If idle, invalidate the cached channel - // session immediately. Queued future events remain - // queued and will create a fresh session on dispatch. - let is_rotate = is_owner_control_command( - &buzz_event.event, - kind_u32, - "!rotate", - &pubkey_hex, - ); - if is_rotate { - if let Some(owner) = owner_cache.get() { - if buzz_event.event.pubkey.to_hex() == *owner { - let fired = signal_in_flight_task( - &mut pool, - buzz_event.channel_id, - ControlSignal::Rotate, + // Rotation is explicit owner intent to start the + // next turn in this channel with a fresh ACP + // session. If a turn is in-flight, cancel it, drop + // its triggering batch, and invalidate the channel + // session when the task returns. If idle, + // invalidate the cached channel session + // immediately. Queued future events remain queued + // and will create a fresh session on dispatch. + Some(("!rotate", "")) => { + let fired = signal_in_flight_task( + &mut pool, + buzz_event.channel_id, + ControlSignal::Rotate, + ); + if fired { + tracing::info!( + channel_id = %buzz_event.channel_id, + "!rotate received — cancelling in-flight turn and rotating session" + ); + } else { + let invalidated = pool.invalidate_channel_sessions(buzz_event.channel_id); + tracing::info!( + channel_id = %buzz_event.channel_id, + invalidated, + "!rotate received — invalidated idle channel session(s)" ); - if fired { - tracing::info!( - channel_id = %buzz_event.channel_id, - "!rotate received — cancelling in-flight turn and rotating session" - ); - } else { - let invalidated = pool.invalidate_channel_sessions(buzz_event.channel_id); - tracing::info!( - channel_id = %buzz_event.channel_id, - invalidated, - "!rotate received — invalidated idle channel session(s)" - ); - } - continue; // consume event — do NOT push to queue } + continue; // consume event — do NOT push to queue } - // Not from owner — fall through to normal prompt handling. + Some(("!model", model_id)) => { + let reply = handle_model_command( + &mut pool, + &ctx.model_catalog, + buzz_event.channel_id, + model_id, + ); + spawn_notice( + Some(&ctx.rest_client), + buzz_event.channel_id, + queue::parse_thread_tags(&buzz_event.event), + reply, + ); + continue; // consume event — do NOT push to queue + } + // Everything else falls through to normal prompt + // handling. The parser accepts any `!token`, so + // this arm is what keeps an unknown bang word — or + // a known one carrying args it does not take — a + // message to the agent rather than a swallowed + // event. See + // `owner_control_command_falls_through_on_unmatched_shapes`. + _ => {} } // Coarse security policy: drop events from disallowed @@ -2716,15 +2854,51 @@ fn event_mentions_agent(event: &nostr::Event, agent_pubkey_hex: &str) -> bool { }) } -fn is_owner_control_command( - event: &nostr::Event, +/// Split a kind:9 mention of this agent into `(command, args)` when its content +/// starts with a `!token`, or with an `@mention` followed by one. Ownership is +/// NOT checked here — callers re-check the author so a non-owner's `!cancel` +/// still reaches the agent as a prompt. +/// +/// The bang must sit at **command position**: either first, or immediately +/// after the run of leading `@mention` tokens that addresses the agent. Prose +/// never arms it. Anything looser makes an ordinary question — `@Agent what +/// happens if I run !shutdown` — indistinguishable from the command itself. +/// +/// A mention is one whitespace-delimited `@token`, so a display name holding +/// spaces (`@Will Pfleger !rotate`) does not reach command position and is +/// delivered to the agent as an ordinary message instead. Without the name +/// list — which the main loop does not have — where such a mention ends is +/// genuinely ambiguous, and that ambiguity is the whole vulnerability: any +/// rule permissive enough to end the mention at `Pfleger` also ends it at the +/// last word of a sentence. Not firing is the safe side of that trade. +fn owner_control_command<'a>( + event: &'a nostr::Event, kind_u32: u32, - command: &str, agent_pubkey_hex: &str, -) -> bool { - kind_u32 == KIND_STREAM_MESSAGE - && event.content.trim() == command - && event_mentions_agent(event, agent_pubkey_hex) +) -> Option<(&'a str, &'a str)> { + if kind_u32 != KIND_STREAM_MESSAGE || !event_mentions_agent(event, agent_pubkey_hex) { + return None; + } + let mut content = event.content.trim(); + // Mentioning is how a client addresses an agent, so `@Name !rotate` is the + // natural gesture, and a channel may address several agents at once + // (`@Sol @Eva !rotate`). Skip the leading mention tokens without matching a + // name — the `p` tag already proved the mention is ours. Every skipped + // token must itself start with `@`, which is what keeps the scan from + // running into prose. + while let Some(after_at) = content.strip_prefix('@') { + let token_len = after_at.find(char::is_whitespace).unwrap_or(after_at.len()); + if token_len == 0 { + return None; // bare `@` — not a mention + } + content = after_at[token_len..].trim_start(); + } + if !content.starts_with('!') { + return None; + } + let (command, args) = + content.split_at(content.find(char::is_whitespace).unwrap_or(content.len())); + Some((command, args.trim())) } // ── signal_in_flight_task ───────────────────────────────────────────────────── @@ -3008,29 +3182,38 @@ fn is_auth_error(error: &acp::AcpError) -> bool { message.contains("Re-authenticate") || message.contains("API Error: 401") } -/// Spawn a task that posts a user-visible failure notice to the relay. -/// -/// Shared by the hard-cap immediate dead-letter path and the retries-exhausted -/// dead-letter path so neither duplicates the tokio::spawn block. -fn spawn_failure_notice( +/// Spawn a task that posts a user-visible notice to the relay. +fn spawn_notice( rest_client: Option<&relay::RestClient>, - batch: &FlushBatch, + channel_id: Uuid, + thread_tags: ThreadTags, content: String, ) { if let Some(rest) = rest_client { - let thread_tags = batch - .events - .last() - .map(|be| queue::parse_thread_tags(&be.event)) - .unwrap_or_default(); let rest = rest.clone(); - let channel_id = batch.channel_id; tokio::spawn(async move { - pool::post_failure_notice(&rest, channel_id, &thread_tags, &content).await; + pool::post_notice(&rest, channel_id, &thread_tags, &content).await; }); } } +/// Spawn a task that posts a user-visible failure notice for a dead-lettered batch. +/// +/// Shared by the hard-cap immediate dead-letter path and the retries-exhausted +/// dead-letter path so neither duplicates the thread-tag extraction. +fn spawn_failure_notice( + rest_client: Option<&relay::RestClient>, + batch: &FlushBatch, + content: String, +) { + let thread_tags = batch + .events + .last() + .map(|be| queue::parse_thread_tags(&be.event)) + .unwrap_or_default(); + spawn_notice(rest_client, batch.channel_id, thread_tags, content); +} + #[allow(clippy::too_many_arguments)] fn handle_prompt_result( pool: &mut AgentPool, @@ -3797,7 +3980,6 @@ async fn initialize_agent_pool( index: i, acp, state: SessionState::default(), - model_capabilities: None, desired_model: startup.model.clone(), model_overridden: false, agent_name, @@ -4090,16 +4272,13 @@ async fn run_models(args: ModelsArgs) -> Result<()> { if !config_options.is_empty() { println!("Models (stable configOptions):"); for opt in &config_options { - let config_id = opt.get("configId").and_then(|v| v.as_str()).unwrap_or("?"); - let display = opt - .get("displayName") - .and_then(|v| v.as_str()) - .unwrap_or(config_id); + let config_id = acp::config_option_id(opt).unwrap_or("?"); + let display = acp::config_option_label(opt).unwrap_or(config_id); println!(" {display} (configId: {config_id})"); if let Some(options) = opt.get("options").and_then(|v| v.as_array()) { for o in options { let val = o.get("value").and_then(|v| v.as_str()).unwrap_or("?"); - let name = o.get("displayName").and_then(|v| v.as_str()).unwrap_or(val); + let name = acp::config_option_label(o).unwrap_or(val); println!(" - {name} (value: {val})"); } } @@ -4246,35 +4425,86 @@ mod owner_control_command_tests { } #[test] - fn owner_control_command_requires_kind_content_and_agent_mention() { + fn owner_control_command_splits_command_and_args() { let agent = "ab".repeat(32); + for (content, expected) in [ + (" !rotate ", Some(("!rotate", ""))), + ("!shutdown", Some(("!shutdown", ""))), + ("!cancel", Some(("!cancel", ""))), + ("!rotate\tnow\n", Some(("!rotate", "now"))), + // Arity is enforced by the caller's match arms, not the parser: + // `("!shutdown", "please")` matches no arm and falls through. + ("!shutdown please", Some(("!shutdown", "please"))), + ("!bogus", Some(("!bogus", ""))), + // Clients address an agent by mention, so leading `@token`s are + // skipped without matching a name — including several at once. + ("@Claude !rotate", Some(("!rotate", ""))), + ("@Claude !rotate now", Some(("!rotate", "now"))), + ("@Sol @Eva !cancel", Some(("!cancel", ""))), + ("@Claude", None), + // The bang must be at command position. Prose after the mention + // never arms it, or asking the agent *about* a command runs it. + ("@Claude please run !rotate", None), + ("@Claude what happens if I use !shutdown", None), + // A display name holding spaces does not reach command position: + // the parser has no name list, and any rule loose enough to end + // the mention at `Pfleger` also ends it mid-sentence. + ("@Will Pfleger !rotate", None), + ("@Codex (Sol) !cancel", None), + // A bang that is not first and not behind a leading mention is + // just text. + ("hello @Claude !rotate", None), + ("hello !rotate", None), + ("", None), + ("@", None), + ] { + let event = make_event(KIND_STREAM_MESSAGE, content, Some(&agent)); + assert_eq!( + owner_control_command(&event, KIND_STREAM_MESSAGE, &agent), + expected, + "content {content:?}" + ); + } + } - let event = make_event(KIND_STREAM_MESSAGE, " !rotate ", Some(&agent)); - assert!(is_owner_control_command( - &event, - KIND_STREAM_MESSAGE, - "!rotate", - &agent - )); + #[test] + fn owner_control_command_requires_kind_and_agent_mention() { + let agent = "ab".repeat(32); let wrong_kind = make_event(1, "!rotate", Some(&agent)); - assert!(!is_owner_control_command(&wrong_kind, 1, "!rotate", &agent)); - - let wrong_content = make_event(KIND_STREAM_MESSAGE, "!cancel", Some(&agent)); - assert!(!is_owner_control_command( - &wrong_content, - KIND_STREAM_MESSAGE, - "!rotate", - &agent - )); + assert!(owner_control_command(&wrong_kind, 1, &agent).is_none()); let no_mention = make_event(KIND_STREAM_MESSAGE, "!rotate", None); - assert!(!is_owner_control_command( - &no_mention, - KIND_STREAM_MESSAGE, - "!rotate", - &agent - )); + assert!(owner_control_command(&no_mention, KIND_STREAM_MESSAGE, &agent).is_none()); + + let other_agent = make_event(KIND_STREAM_MESSAGE, "!rotate", Some(&"cd".repeat(32))); + assert!(owner_control_command(&other_agent, KIND_STREAM_MESSAGE, &agent).is_none()); + } + + /// The parser accepts any `!token`, so the control-command match must reject + /// what it does not handle rather than consume it. + /// + /// Two shapes reach the match and must survive it: a bang word no arm names, + /// and a known command carrying args it does not take. Both parse cleanly — + /// that is the parser's job — and both have to fall through the `_` arm to + /// normal prompt handling, or the owner's message is silently swallowed with + /// no reply and no queued turn. + #[test] + fn owner_control_command_falls_through_on_unmatched_shapes() { + let agent = "ab".repeat(32); + + for content in ["!bogus", "!shutdown please"] { + let event = make_event(KIND_STREAM_MESSAGE, content, Some(&agent)); + let parsed = owner_control_command(&event, KIND_STREAM_MESSAGE, &agent); + assert!(parsed.is_some(), "content {content:?} must parse"); + assert!( + !matches!( + parsed, + Some(("!shutdown", "") | ("!cancel", "") | ("!rotate", "")) + ), + "content {content:?} must not match a control arm", + ); + } } #[test] @@ -4354,6 +4584,255 @@ mod owner_control_command_tests { ControlSignal::Rotate )); } + + /// A two-half catalog whose ids mirror the ambiguous pairs real adapters + /// ship, reporting `haiku` as the adapter's own current selection. + fn test_catalog() -> pool::ModelCatalog { + let catalog = pool::ModelCatalog::default(); + catalog.capture(pool::AgentModelCapabilities { + config_options_raw: vec![serde_json::json!({ + "id": "model", + "category": "model", + "currentValue": "haiku", + "options": [ + { "value": "haiku", "name": "Haiku" }, + { "value": "opus[1m]", "name": "Opus (1M context)" } + ] + })], + available_models_raw: Some(serde_json::json!({ + "availableModels": [{ "modelId": "claude-opus-5" }] + })), + }); + catalog + } + + /// A stock idle agent holding a session for `channel_id`: no `--model`, no + /// prior switch, so `desired_model` is `None` like the default deployment. + async fn idle_agent(index: usize, channel_id: Uuid) -> OwnedAgent { + let mut state = SessionState::default(); + state.sessions.insert(channel_id, "sess-1".to_string()); + OwnedAgent { + index, + acp: AcpClient::spawn("cat", &[], &[], false) + .await + .expect("spawn cat as inert agent"), + state, + desired_model: None, + model_overridden: false, + agent_name: "unknown".into(), + goose_system_prompt_supported: None, + protocol_version: 2, + } + } + + async fn pool_with_idle_agent(channel_id: Uuid) -> AgentPool { + AgentPool::from_slots(vec![Some(idle_agent(0, channel_id).await)]) + } + + /// On a stock agent nobody has overridden — the most common `!model` call — + /// the marker must come from the adapter's own reported selection. + #[tokio::test] + async fn model_command_without_args_marks_the_adapter_reported_current() { + let channel_id = Uuid::new_v4(); + let mut pool = pool_with_idle_agent(channel_id).await; + + assert_eq!( + handle_model_command(&mut pool, &test_catalog(), channel_id, ""), + "My models:\n- `haiku` — Haiku ← current\n- `opus[1m]` — Opus (1M context)\n- `claude-opus-5`" + ); + } + + /// An owner override outranks the adapter's reported selection: the agent + /// has not run a turn under it yet, so the adapter still reports the old one. + #[tokio::test] + async fn model_command_marks_the_owner_override_over_the_reported_current() { + let channel_id = Uuid::new_v4(); + let mut agent = idle_agent(0, channel_id).await; + agent.desired_model = Some("claude-opus-5".to_string()); + let mut pool = AgentPool::from_slots(vec![Some(agent)]); + + let listing = handle_model_command(&mut pool, &test_catalog(), channel_id, ""); + assert!(listing.contains("`claude-opus-5` ← current"), "{listing}"); + assert!(!listing.contains("`haiku` — Haiku ←"), "{listing}"); + } + + /// `desired_model` is per-agent-process, so with `--agents N > 1` only the + /// agent holding the channel's session speaks for the channel. Another + /// slot's override must never be marked as this channel's current. + #[tokio::test] + async fn model_command_ignores_another_agents_override() { + let channel_id = Uuid::new_v4(); + let mut other = idle_agent(0, Uuid::new_v4()).await; + other.desired_model = Some("opus[1m]".to_string()); + let mut pool = + AgentPool::from_slots(vec![Some(other), Some(idle_agent(1, channel_id).await)]); + + let listing = handle_model_command(&mut pool, &test_catalog(), channel_id, ""); + assert!(listing.contains("`haiku` — Haiku ← current"), "{listing}"); + assert!( + !listing.contains("`opus[1m]` — Opus (1M context) ←"), + "{listing}" + ); + } + + #[tokio::test] + async fn model_command_switches_idle_agent_on_exact_id() { + let channel_id = Uuid::new_v4(); + let mut pool = pool_with_idle_agent(channel_id).await; + + assert_eq!( + handle_model_command(&mut pool, &test_catalog(), channel_id, "claude-opus-5"), + "Model set to `claude-opus-5`. Takes effect on my next turn." + ); + assert_eq!( + pool.channel_model_override(channel_id), + Some("claude-opus-5") + ); + } + + #[tokio::test] + async fn model_command_rejects_ambiguous_prefix_with_the_listing() { + let channel_id = Uuid::new_v4(); + let mut pool = pool_with_idle_agent(channel_id).await; + + // `opus` prefixes both `opus[1m]` and `claude-opus-5` — matching is + // exact, so it is a miss, and the miss renders the list. + let reply = handle_model_command(&mut pool, &test_catalog(), channel_id, "opus"); + assert!( + reply.starts_with("`opus` isn't one of my models.\n\nMy models:"), + "{reply}" + ); + assert_eq!( + pool.channel_model_override(channel_id), + None, + "a miss must not change the model" + ); + } + + /// The default deployment is `--agents 1`, so during a turn the pool is + /// empty — the catalog must still be reachable, or a typo cancels the turn. + #[tokio::test] + async fn model_command_pre_validates_with_every_agent_checked_out() { + let channel_id = Uuid::new_v4(); + let mut pool = AgentPool::from_slots(vec![None]); + let catalog = test_catalog(); + let (control_tx, mut control_rx) = tokio::sync::oneshot::channel(); + let abort_handle = pool.join_set.spawn(async {}); + pool.task_map_mut().insert( + abort_handle.id(), + pool::TaskMeta { + agent_index: 0, + channel_id: Some(channel_id), + turn_id: "test-turn-id".to_string(), + recoverable_batch: None, + control_tx: Some(control_tx), + steer_tx: None, + }, + ); + + // No idle agent holds the session, so the marker comes from the + // adapter's reported current alone. + assert_eq!( + handle_model_command(&mut pool, &catalog, channel_id, "bogus-model"), + "`bogus-model` isn't one of my models.\n\nMy models:\n- `haiku` — Haiku ← current\n- `opus[1m]` — Opus (1M context)\n- `claude-opus-5`" + ); + assert!( + control_rx.try_recv().is_err(), + "a typo must not cancel the in-flight turn" + ); + + assert_eq!( + handle_model_command(&mut pool, &catalog, channel_id, "opus[1m]"), + "Switching to `opus[1m]` — restarting the current turn on the new model." + ); + assert_eq!( + control_rx.await.unwrap(), + ControlSignal::SwitchModel("opus[1m]".to_string()) + ); + } + + #[test] + fn model_command_defers_when_no_catalog_is_reachable() { + let channel_id = Uuid::new_v4(); + let mut pool = AgentPool::from_slots(vec![]); + let empty = pool::ModelCatalog::default(); + + assert_eq!( + handle_model_command(&mut pool, &empty, channel_id, ""), + "I don't know my model list yet — ask again in a moment." + ); + // A named pick is deferred for the same reason, not passed through: + // there is nothing to validate it against. + assert_eq!( + handle_model_command(&mut pool, &empty, channel_id, "haiku"), + "I don't know my model list yet — ask again in a moment." + ); + } + + /// The catalog is missing only until the process's first `session/new` + /// responds — and that call is already registered in flight. Passing an + /// unvalidated pick through there would cancel and requeue a live turn on + /// the strength of a possible typo, and the fresh session would then fall + /// back to the unchanged model anyway. + #[tokio::test] + async fn model_command_does_not_cancel_the_first_turn_without_a_catalog() { + let channel_id = Uuid::new_v4(); + let mut pool = AgentPool::from_slots(vec![None]); + let empty = pool::ModelCatalog::default(); + let (control_tx, mut control_rx) = tokio::sync::oneshot::channel(); + let abort_handle = pool.join_set.spawn(async {}); + pool.task_map_mut().insert( + abort_handle.id(), + pool::TaskMeta { + agent_index: 0, + channel_id: Some(channel_id), + turn_id: "test-turn-id".to_string(), + recoverable_batch: None, + control_tx: Some(control_tx), + steer_tx: None, + }, + ); + + assert_eq!( + handle_model_command(&mut pool, &empty, channel_id, "haiku"), + "I don't know my model list yet — ask again in a moment." + ); + assert!( + control_rx.try_recv().is_err(), + "an unvalidatable pick must not cancel the in-flight turn" + ); + } + + /// An adapter that reports no selectable models is not a catalog — latching + /// one would make every later `!model` claim the list is empty. + #[test] + fn model_catalog_ignores_a_capture_with_no_models() { + let catalog = pool::ModelCatalog::default(); + catalog.capture(pool::AgentModelCapabilities { + config_options_raw: vec![], + available_models_raw: None, + }); + assert!(catalog.get().is_none()); + } + + #[test] + fn switch_outcome_statuses_match_the_desktop_contract() { + // Consumed by the desktop's model picker. + assert_eq!(SwitchOutcome::Sent.status(), "sent"); + assert_eq!(SwitchOutcome::TurnEnding.status(), "turn_ending"); + assert_eq!(SwitchOutcome::Switched.status(), "switched"); + assert_eq!( + SwitchOutcome::UnsupportedModel.status(), + "unsupported_model" + ); + // Deliberately shares `unsupported_model`: both mean "rejected, session + // untouched", and the desktop cannot reach this variant anyway. + assert_eq!( + SwitchOutcome::CatalogUnavailable.status(), + "unsupported_model" + ); + assert_eq!(SwitchOutcome::NoActiveTurn.status(), "no_active_turn"); + } } #[cfg(test)] @@ -5248,7 +5727,6 @@ mod error_outcome_emission_tests { .await .expect("spawn cat as inert agent"), state: Default::default(), - model_capabilities: None, desired_model: None, model_overridden: false, agent_name: "unknown".into(), diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index 0c51fe954f..c279b261e3 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -29,7 +29,7 @@ use tokio::time::timeout; use uuid::Uuid; use crate::acp::{ - extract_model_config_options, extract_model_state, model_in_catalog, + config_option_label, extract_model_config_options, extract_model_state, model_in_catalog, resolve_model_switch_method, AcpClient, AcpError, McpServer, ModelSwitchMethod, StopReason, }; use crate::config::{compose_session_title, DedupMode, PermissionMode}; @@ -67,9 +67,10 @@ pub struct TaskMeta { pub steer_tx: Option>, } -/// Agent-level model capabilities. Populated on first session creation. -/// The catalog is the same across all sessions for a given agent process. +/// Model capabilities of the configured agent command. Populated on first +/// session creation and identical for every later session. /// Fields are read by the desktop's `get_agent_models` Tauri command (Phase 3). +#[derive(Clone)] #[allow(dead_code)] // Scaffolding for desktop integration — fields read via serde. pub struct AgentModelCapabilities { /// Stable: configOptions with category "model" from session/new. @@ -78,6 +79,100 @@ pub struct AgentModelCapabilities { pub available_models_raw: Option, } +impl AgentModelCapabilities { + /// Selectable `(id, label)` pairs across both catalog halves, in the order + /// [`resolve_model_switch_method`] searches them and deduplicated by id — + /// the halves overlap on some adapters and disagree on others. + pub fn models(&self) -> Vec<(&str, Option<&str>)> { + let config_options = self + .config_options_raw + .iter() + .filter_map(|opt| opt.get("options")?.as_array()) + .flatten() + .filter_map(|opt| Some((opt.get("value")?.as_str()?, config_option_label(opt)))); + let available_models = self + .available_models_raw + .iter() + .filter_map(|models| models.get("availableModels")?.as_array()) + .flatten() + .filter_map(|model| { + Some(( + model.get("modelId")?.as_str()?, + model.get("name").and_then(|name| name.as_str()), + )) + }); + + let mut models: Vec<(&str, Option<&str>)> = Vec::new(); + for (id, label) in config_options.chain(available_models) { + if !models.iter().any(|(seen, _)| *seen == id) { + models.push((id, label)); + } + } + models + } + + /// Whether `model_id` is selectable. Mirrors [`model_in_catalog`]. + pub fn contains(&self, model_id: &str) -> bool { + model_in_catalog( + &self.config_options_raw, + self.available_models_raw.as_ref(), + model_id, + ) + } + + /// The model the adapter itself reported as selected, in the same + /// half-precedence [`Self::models`] uses. This is the answer for an agent + /// nobody has overridden — `desired_model` is `None` until `--model` or a + /// live switch sets one. + pub fn reported_current(&self) -> Option<&str> { + let config_current = self + .config_options_raw + .iter() + .find_map(|opt| opt.get("currentValue")?.as_str()); + config_current.or_else(|| { + self.available_models_raw + .as_ref()? + .get("currentModelId")? + .as_str() + }) + } +} + +/// Process-wide model catalog for the configured agent command, shared between +/// the main loop and every in-flight prompt task. +/// +/// The catalog is a property of the agent *command*, not of a session or a pool +/// slot: all slots run the same binary and every session of a given process +/// reports the same list. Holding it here rather than on [`OwnedAgent`] is what +/// makes `!model` answerable while the agent is checked out mid-turn — with the +/// default `--agents 1` that is the entire busy path, and a pool-slot lookup +/// would find nothing. +/// +/// Captured task-side from the first session bound by any slot. +#[derive(Clone, Default)] +pub struct ModelCatalog(Arc>>); + +impl ModelCatalog { + /// Record the catalog carried by a freshly bound session, first one wins. + /// A response carrying no selectable models is not a catalog, so it is + /// ignored rather than latched as an empty list. + pub fn capture(&self, caps: AgentModelCapabilities) { + if caps.models().is_empty() { + return; + } + if let Ok(mut slot) = self.0.lock() { + slot.get_or_insert(caps); + } + } + + /// Snapshot of the catalog, or `None` before any session has been bound. + /// + /// Returns a clone so callers can hold it across a `&mut AgentPool` borrow. + pub fn get(&self) -> Option { + self.0.lock().ok()?.clone() + } +} + /// Per-channel session IDs and turn counters. /// /// Separated from `OwnedAgent` so the state machine is testable without @@ -151,8 +246,6 @@ pub struct OwnedAgent { pub index: usize, pub acp: AcpClient, pub state: SessionState, - /// Model catalog from first session/new. None until first session created. - pub model_capabilities: Option, /// Desired model ID (from `Config.model`). Applied after every `session_new_full()`. pub desired_model: Option, /// Whether `desired_model` was set by a live `SwitchModel` control signal @@ -532,6 +625,10 @@ pub struct PromptContext { /// the desktop keys per (agent, relay) pair, e.g. `session_config_captured`, /// mirroring the `managed_agent_runtime_lifecycle` frames. pub relay_url: String, + /// Model catalog of the configured agent command, captured task-side from + /// the first bound session and readable from the main loop while every + /// agent is mid-turn. See [`ModelCatalog`]. + pub model_catalog: ModelCatalog, } impl AgentPool { @@ -719,13 +816,40 @@ impl AgentPool { count } + /// The model override in force for `channel_id`, if the channel's own agent + /// is idle and carries one. + /// + /// `desired_model` is per-agent-*process*, so an arbitrary slot's pick is + /// some other conversation's, not this channel's. Two slots can honestly + /// answer for `channel_id`: the one holding its session, and — when the + /// pool has a single slot, the default — that slot, which necessarily + /// serves every channel. A just-switched channel has no session (the switch + /// invalidates it), which is why the second case is not redundant. + /// + /// `None` means "no override readable here", not "no current model": the + /// adapter's own answer lives in the catalog + /// ([`AgentModelCapabilities::reported_current`]). + pub fn channel_model_override(&self, channel_id: Uuid) -> Option<&str> { + let owning = self + .agents + .iter() + .flatten() + .find(|agent| agent.state.sessions.contains_key(&channel_id)); + let sole = match self.agents.as_slice() { + [only] => only.as_ref(), + _ => None, + }; + owning.or(sole)?.desired_model.as_deref() + } + /// Idle-path model switch: set `desired_model` on the idle agent for /// `channel_id` and invalidate its session so the next turn re-creates the /// session under the new model. /// - /// Pre-cancel guard: the desired model is validated against the agent's - /// cached catalog *before* the session is invalidated, so an unsupported - /// pick is rejected without disturbing the existing session. + /// Validation is the caller's job — [`crate::handle_model_command`] and the + /// desktop both pre-check against [`PromptContext::model_catalog`], which + /// (unlike anything reachable from a pool slot) is also readable while the + /// agent is mid-turn. /// /// Returns [`IdleSwitchResult`] describing what happened. The model does not /// take effect — and the panel does not reflect it — until the agent next @@ -746,18 +870,6 @@ impl AgentPool { return IdleSwitchResult::NoIdleAgent; }; - // Pre-cancel guard against the cached catalog. None = catalog not yet - // populated (no session ever created); defer validation to apply time. - if let Some(caps) = agent.model_capabilities.as_ref() { - if !model_in_catalog( - &caps.config_options_raw, - caps.available_models_raw.as_ref(), - model_id, - ) { - return IdleSwitchResult::UnsupportedModel; - } - } - agent.desired_model = Some(model_id.to_string()); agent.model_overridden = true; agent.state.invalidate_channel(&channel_id); @@ -770,10 +882,8 @@ impl AgentPool { pub enum IdleSwitchResult { /// `desired_model` set and the channel session invalidated. Switched, - /// Desired model is not in the agent's cached catalog — pick rejected, - /// session untouched. - UnsupportedModel, - /// No idle agent available (all checked out / none spawned). + /// No idle agent holds a session for the channel (checked out, or none + /// spawned). NoIdleAgent, } @@ -911,13 +1021,12 @@ 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), - available_models_raw: extract_model_state(&resp.raw), - }); - } + // Publish the catalog to the process-wide cache, so `!model` can list and + // validate while every agent is checked out mid-turn. + ctx.model_catalog.capture(AgentModelCapabilities { + config_options_raw: extract_model_config_options(&resp.raw), + available_models_raw: extract_model_state(&resp.raw), + }); // Apply desired_model if set, matching against the fresh session/new response. // Track whether the switch succeeded so session_config_captured reflects @@ -3547,11 +3656,11 @@ pub(crate) async fn reaction_add(rest: &crate::relay::RestClient, event_id: &str } } -/// Best-effort: post a visible failure notice (kind:9) to a channel after a -/// batch is dead-lettered. Replies into the thread of `thread_tags` when the -/// triggering event was threaded. Errors are logged and swallowed — the -/// notice must never take down the main loop. -pub(crate) async fn post_failure_notice( +/// Best-effort: post a visible notice (kind:9) to a channel — a dead-letter +/// warning or an owner-command reply. Replies into the thread of `thread_tags` +/// when the triggering event was threaded. Errors are logged and swallowed — +/// the notice must never take down the main loop. +pub(crate) async fn post_notice( rest: &crate::relay::RestClient, channel_id: Uuid, thread_tags: &ThreadTags, @@ -3573,21 +3682,21 @@ pub(crate) async fn post_failure_notice( match buzz_sdk::build_message(channel_id, content, thread_ref.as_ref(), &[], false, &[]) { Ok(b) => b, Err(e) => { - tracing::warn!(channel = %channel_id, "failure notice: build failed: {e}"); + tracing::warn!(channel = %channel_id, "notice: build failed: {e}"); return; } }; let event = match builder.sign_with_keys(&rest.keys) { Ok(e) => e, Err(e) => { - tracing::warn!(channel = %channel_id, "failure notice: sign failed: {e}"); + tracing::warn!(channel = %channel_id, "notice: sign failed: {e}"); return; } }; match tokio::time::timeout(Duration::from_secs(5), rest.submit_event(&event)).await { Ok(Ok(_)) => {} - Ok(Err(e)) => tracing::warn!(channel = %channel_id, "failure notice failed: {e}"), - Err(_) => tracing::warn!(channel = %channel_id, "failure notice timed out"), + Ok(Err(e)) => tracing::warn!(channel = %channel_id, "notice failed: {e}"), + Err(_) => tracing::warn!(channel = %channel_id, "notice timed out"), } } @@ -3712,6 +3821,83 @@ mod tests { use nostr::{EventBuilder, Keys, Kind, Tag, Timestamp}; use serde_json::json; + #[test] + fn model_capabilities_list_both_halves_deduplicated() { + let caps = AgentModelCapabilities { + config_options_raw: vec![json!({ + "id": "model", + "category": "model", + "options": [ + { "value": "haiku", "name": "Haiku" }, + { "value": "opus[1m]" } + ] + })], + available_models_raw: Some(json!({ + "availableModels": [ + { "modelId": "haiku", "name": "Haiku (unstable half)" }, + { "modelId": "claude-opus-5", "name": "Opus 5" } + ] + })), + }; + + // configOptions first (the resolve precedence), then the unstable half; + // `haiku` keeps its stable-half label rather than being listed twice. + assert_eq!( + caps.models(), + vec![ + ("haiku", Some("Haiku")), + ("opus[1m]", None), + ("claude-opus-5", Some("Opus 5")), + ] + ); + assert!(caps.contains("opus[1m]")); + assert!(!caps.contains("opus")); + } + + #[test] + fn model_capabilities_list_tolerates_missing_halves() { + let empty = AgentModelCapabilities { + config_options_raw: vec![], + available_models_raw: None, + }; + assert!(empty.models().is_empty()); + + let unstable_only = AgentModelCapabilities { + config_options_raw: vec![], + available_models_raw: Some(json!({ "availableModels": [{ "modelId": "gpt-5.4" }] })), + }; + assert_eq!(unstable_only.models(), vec![("gpt-5.4", None)]); + + let stable_only = AgentModelCapabilities { + config_options_raw: vec![json!({ "options": [{ "value": "gpt-5.4" }] })], + available_models_raw: None, + }; + assert_eq!(stable_only.models(), vec![("gpt-5.4", None)]); + } + + /// The adapter's own selection, used as "current" when nobody has + /// overridden the model — the default for every stock agent. + #[test] + fn model_capabilities_reported_current_prefers_the_stable_half() { + let both = AgentModelCapabilities { + config_options_raw: vec![json!({ "currentValue": "gpt-5.4" })], + available_models_raw: Some(json!({ "currentModelId": "gpt-5.3-codex/medium" })), + }; + assert_eq!(both.reported_current(), Some("gpt-5.4")); + + let unstable_only = AgentModelCapabilities { + config_options_raw: vec![json!({ "options": [] })], + available_models_raw: Some(json!({ "currentModelId": "haiku" })), + }; + assert_eq!(unstable_only.reported_current(), Some("haiku")); + + let neither = AgentModelCapabilities { + config_options_raw: vec![], + available_models_raw: None, + }; + assert_eq!(neither.reported_current(), None); + } + // These pin the initial_message dispatch path (run_prompt_task, ~line 855): // a legacy agent WITH a base_prompt must get [Base] prepended to the user // message. This is the exact regression that shipped in the round-2 bug. @@ -5054,7 +5240,6 @@ mod tests { index: 0, acp, state: SessionState::default(), - model_capabilities: None, desired_model: None, model_overridden: false, agent_name: "unknown".into(), @@ -5112,7 +5297,6 @@ mod tests { index: 0, acp, state: SessionState::default(), - model_capabilities: None, desired_model: None, model_overridden: false, agent_name: "unknown".into(), @@ -5370,6 +5554,7 @@ mod tests { memory_enabled: false, harness_name: "goose".to_string(), relay_url: "ws://127.0.0.1:3000".to_string(), + model_catalog: ModelCatalog::default(), } } diff --git a/docs/nips/NIP-AO.md b/docs/nips/NIP-AO.md index 36adea0487..1754ed0ec4 100644 --- a/docs/nips/NIP-AO.md +++ b/docs/nips/NIP-AO.md @@ -122,8 +122,20 @@ The `content` field decrypts to: } ``` -The only defined control type is `cancel_turn`. Implementations MUST ignore -events with unrecognized `type` values. +Two control types are defined. `cancel_turn` cancels the channel's in-flight +turn. `switch_model` carries an additional `modelId` and switches the model +backing the channel — cancelling and re-running the in-flight turn on the new +model, or applying to the next turn when the channel is idle: + +```json +{ + "type": "switch_model", + "channelId": "", + "modelId": "" +} +``` + +Implementations MUST ignore events with unrecognized `type` values. ## Ephemerality Contract