diff --git a/apps/staged/src-tauri/src/session_runner.rs b/apps/staged/src-tauri/src/session_runner.rs index a5758b2f3..87959f3b2 100644 --- a/apps/staged/src-tauri/src/session_runner.rs +++ b/apps/staged/src-tauri/src/session_runner.rs @@ -1572,17 +1572,12 @@ pub fn start_session( .unwrap_or(CompletionReason::Interrupted); if let Err(ref e) = result { - if config.acp_config_selection.is_some() - && acp_client::is_config_selection_unavailable_error(e) - { - if let Err(clear_err) = - store_for_status.set_session_acp_config_selection(&session_id_for_status, None) - { - log::warn!( - "Failed to clear stale ACP config selection for session {session_id_for_status}: {clear_err}" - ); - } - } + clear_unavailable_acp_config_selection( + &store_for_status, + &session_id_for_status, + config.acp_config_selection.as_ref(), + e, + ); } // Transition the session to its terminal state, but only if it is @@ -1847,6 +1842,32 @@ fn stored_agent_session_id(store: &Store, session_id: &str) -> Result, + error: &str, +) -> bool { + if selection.is_none() || !acp_client::is_config_selection_unavailable_error(error) { + return false; + } + if let Err(clear_err) = store.set_session_acp_config_selection(session_id, None) { + log::warn!( + "Failed to clear stale ACP config selection for session {session_id}: {clear_err}" + ); + return false; + } + true +} + pub(crate) fn session_has_note_artifact(store: &Store, session_id: &str) -> bool { store .get_note_by_session(session_id) @@ -4964,6 +4985,88 @@ mod tests { assert_eq!(step.error, None); } + fn fable_selection() -> AcpConfigSelection { + AcpConfigSelection { + model: Some(crate::store::AcpConfigValueSelection { + config_id: "model".to_string(), + value_id: "claude-fable-5-1[1m]".to_string(), + label: Some("Fable".to_string()), + }), + effort: None, + } + } + + /// acp-client retries Claude's model confirmation once and then fails + /// with a plain sentence, which its `run` hands through unwrapped. The + /// post-run clearing must read that as "selection fine, try again" and + /// leave the saved pin alone, while a genuinely stale pin — still + /// Debug-wrapped like every other setup failure — keeps being cleared + /// exactly as before. + #[test] + fn a_failed_model_confirmation_keeps_the_saved_selection_but_a_stale_pin_is_cleared() { + let store = Store::in_memory().unwrap(); + let session = crate::store::Session::new_running("prompt", &PathBuf::from("/tmp")); + store.create_session(&session).unwrap(); + let selection = fable_selection(); + store + .set_session_acp_config_selection(&session.id, Some(&selection)) + .unwrap(); + + // Exactly what the driver's `run` records: the sentence, unwrapped. + assert!(!clear_unavailable_acp_config_selection( + &store, + &session.id, + Some(&selection), + acp_client::MODEL_CONFIRMATION_EXHAUSTED_MESSAGE, + )); + assert_eq!( + store + .get_session(&session.id) + .unwrap() + .unwrap() + .acp_config_selection, + Some(selection.clone()), + "a confirmation failure leaves the pin for the retry it asks for" + ); + + let stale = format!( + "ACP protocol failed: {:?}", + agent_client_protocol::util::internal_error( + "Selected ACP model value 'claude-fable-5-1[1m]' is no longer available for config option 'model'" + ) + ); + assert!(clear_unavailable_acp_config_selection( + &store, + &session.id, + Some(&selection), + &stale, + )); + assert_eq!( + store + .get_session(&session.id) + .unwrap() + .unwrap() + .acp_config_selection, + None + ); + } + + /// A run that never had a selection has nothing to clear, whatever it + /// failed with. + #[test] + fn a_run_without_a_selection_clears_nothing() { + let store = Store::in_memory().unwrap(); + let session = crate::store::Session::new_running("prompt", &PathBuf::from("/tmp")); + store.create_session(&session).unwrap(); + + assert!(!clear_unavailable_acp_config_selection( + &store, + &session.id, + None, + "Selected ACP model value 'x' is no longer available for config option 'model'", + )); + } + /// The immediate branch start has answered its client by the time the /// context build or `start_session` can fail, so the row is the only place /// the failure can go: it reads `error`/`Crashed` with the message, and diff --git a/crates/acp-client/src/driver.rs b/crates/acp-client/src/driver.rs index d98eba3fc..d4b156125 100644 --- a/crates/acp-client/src/driver.rs +++ b/crates/acp-client/src/driver.rs @@ -29,7 +29,7 @@ use agent_client_protocol::{ SelectedPermissionOutcome, SessionConfigKind, SessionConfigOption, SessionConfigOptionCategory, SessionConfigSelectOptions, SessionInfoUpdate, SessionModeState, SessionNotification, SessionUpdate, SetSessionConfigOptionRequest, - StopReason, TextContent, ToolCallContent, + SetSessionConfigOptionResponse, StopReason, TextContent, ToolCallContent, }, ProtocolVersion, }, @@ -1985,7 +1985,7 @@ impl AcpDriver { .map_err(agent_client_protocol::util::internal_error) }) .await - .map_err(|e| format!("ACP protocol failed: {e:?}")); + .map_err(describe_protocol_failure); writer.finalize().await; graceful_stop(&mut child, is_remote).await; @@ -5483,6 +5483,183 @@ pub fn is_config_selection_unavailable_error(error: &str) -> bool { || error.contains(CONFIG_SELECTION_STALE_NOT_SELECT))) } +// Claude's SDK confirms every `setModel` with a short API probe on a fixed +// five-second deadline, and a probe that misses it surfaces as this sentence +// in the `details` of an internal error. That is confirmation uncertainty, +// not an unavailable model: the same selection is routinely accepted seconds +// later. [`send_config_selection`] bounds its recovery to exactly this error. +// +// Provenance of the wording — re-verify whenever the adapter is upgraded. +// `@agentclientprotocol/claude-agent-acp` 0.90.0 does not word this itself: +// its `setSessionConfigOption` (`dist/acp-agent.js`) rethrows whatever +// `query.setModel(resolvedValue)` rejects with, and `@anthropic-ai/ +// claude-agent-sdk` 0.3.296 relays that from the Claude Code CLI it vendors +// (`@anthropic-ai/claude-agent-sdk-darwin-arm64/claude`, Claude Code 2.1.296). +// The CLI's model-validation module builds the message when its +// deadline-wrapped API probe returns without an answer. Identifiers are +// minified in the shipped binary, but the template literal and its +// `retryable:!0` (`true`) flag survive verbatim and can be read straight out +// of it: +// +// `Couldn't confirm model "${model}" with the API. Try again[…].` +// +// The same binary carries a *single-quoted* sibling that must not be matched: +// +// `Couldn't confirm model '${model}' is available (the check timed out) · try again` +// +// That one is the inline `/model` command's `{type: "text"}` reply, not an +// error, and never reaches `setModel`; the matcher keys on the double-quoted +// form for that reason. +// +// `${model}` is the value id the bridge passed to `setModel`, echoed verbatim +// including suffixes such as `[1m]` (session records show +// `claude-fable-5-1[1m]`), which is why the matcher is keyed to the *resolved* +// value. A rewording upstream silently disables the retry — nothing fails, the +// run just falls back to today's single attempt and the raw error — and only +// this repo's own fixtures in `driver/tests/model_confirmation.rs` pin the +// contract. +const MODEL_CONFIRMATION_FAILED_PREFIX: &str = "Couldn't confirm model \""; +const MODEL_CONFIRMATION_ATTEMPTS: u32 = 2; +const MODEL_CONFIRMATION_RETRY_DELAY: Duration = Duration::from_secs(1); + +/// What the user reads once every confirmation attempt has failed; +/// [`describe_protocol_failure`] delivers it to the session alert as this +/// sentence. Kept clear of the [`is_config_selection_unavailable_error`] +/// fragments on purpose: the selection itself is sound, so consumers must keep +/// it for the retry the sentence asks for. +pub const MODEL_CONFIRMATION_EXHAUSTED_MESSAGE: &str = "Claude couldn't confirm the selected model after two attempts. Your selection is unchanged; try again."; + +/// The text `run` records for a connection that ended in an error. +/// +/// Setup failures arrive here as the `internal_error` the `connect_with` +/// closure had to wrap their `String` in to satisfy the ACP error type, and +/// the default is to keep that Debug shape: the auth-recovery matchers here +/// and in the frontend (`authRecovery.ts`) are keyed to it, and the detail it +/// carries is what diagnostics want. Exhausted model confirmation is the one +/// setup failure whose wording is written for the user, so it alone is +/// unwrapped back to [`MODEL_CONFIRMATION_EXHAUSTED_MESSAGE`]; the per-attempt +/// records logged by [`send_config_selection`] keep the technical detail. +fn describe_protocol_failure(error: agent_client_protocol::Error) -> String { + if error.code == ErrorCode::InternalError + && error.data.as_ref().and_then(|data| data.as_str()) + == Some(MODEL_CONFIRMATION_EXHAUSTED_MESSAGE) + { + return MODEL_CONFIRMATION_EXHAUSTED_MESSAGE.to_string(); + } + format!("ACP protocol failed: {error:?}") +} + +/// Whether `adapter` — the agent's `initialize` name, or the driver label when +/// it sends none — is a Claude bridge. Only Claude's SDK produces the +/// confirmation failure, so only Claude gets the retry; another agent that +/// happened to word an error the same way keeps the single attempt. +fn supports_model_confirmation_recovery(adapter: &str) -> bool { + adapter.to_ascii_lowercase().contains("claude") +} + +/// Whether a `session/set_config_option` error is Claude failing to confirm +/// `requested_model`: an internal error whose `data` quotes that model after +/// [`MODEL_CONFIRMATION_FAILED_PREFIX`], either as the bridge's +/// `{"details": …}` object or as a bare string. Authentication errors, invalid +/// values, transport failures, and a different model are all not this error. +/// The wording's origin and upgrade caveats are on +/// [`MODEL_CONFIRMATION_FAILED_PREFIX`]. +fn is_model_confirmation_error( + error: &agent_client_protocol::Error, + requested_model: &str, +) -> bool { + if error.code != ErrorCode::InternalError { + return false; + } + let details = match &error.data { + Some(serde_json::Value::Object(data)) => data.get("details").and_then(|d| d.as_str()), + Some(serde_json::Value::String(details)) => Some(details.as_str()), + _ => None, + }; + details.is_some_and(|details| { + details.contains(&format!( + "{MODEL_CONFIRMATION_FAILED_PREFIX}{requested_model}\"" + )) + }) +} + +/// One `session/set_config_option` round trip for `selection`. +/// +/// When a Claude bridge can't confirm the selected model, the identical +/// request goes out exactly once more after [`MODEL_CONFIRMATION_RETRY_DELAY`] +/// on the same connection — never while an attempt is still unanswered, and +/// never for any other option, error, or adapter. The caller's setup timeout +/// and cancellation bound the whole exchange by dropping this future, so a +/// Stop during the delay or the second attempt sends nothing further. +/// Exhausted recovery fails with [`MODEL_CONFIRMATION_EXHAUSTED_MESSAGE`], +/// which [`describe_protocol_failure`] hands to the session alert as that +/// plain sentence while every other setup failure keeps its Debug shape. +/// +/// Each attempt logs a sanitized record — phase, adapter, requested and +/// resolved value, attempt, duration, classified outcome; never prompts, +/// credentials, or the raw response — so a failed start can be correlated +/// with the confirmation that produced it. +async fn send_config_selection( + connection: &ConnectionTo, + agent_session_id: &str, + adapter: &str, + selection: &AcpSessionConfigOptionSelection, + resolved: &ResolvedConfigSelection, +) -> Result { + let label = config_selection_label(&selection.category); + let requested = selection.value_id.as_str(); + let value = resolved.value.value_id.as_str(); + let recoverable = selection.category == SessionConfigOptionCategory::Model + && supports_model_confirmation_recovery(adapter); + let attempts = if recoverable { + MODEL_CONFIRMATION_ATTEMPTS + } else { + 1 + }; + let mut attempt = 1; + loop { + let started = Instant::now(); + let result = connection + .send_request(SetSessionConfigOptionRequest::new( + agent_session_id.to_string(), + resolved.config_id.clone(), + value, + )) + .block_task() + .await; + let diagnostic = format!( + "ACP config phase=setup adapter={adapter} option={label} requested={requested} \ + resolved={value} attempt={attempt}/{attempts} duration_ms={}", + started.elapsed().as_millis() + ); + match result { + Ok(response) => { + log::info!("{diagnostic} outcome=accepted"); + return Ok(response); + } + Err(error) if recoverable && is_model_confirmation_error(&error, value) => { + if attempt < attempts { + log::warn!( + "{diagnostic} outcome=unconfirmed; retrying in {}ms", + MODEL_CONFIRMATION_RETRY_DELAY.as_millis() + ); + tokio::time::sleep(MODEL_CONFIRMATION_RETRY_DELAY).await; + attempt += 1; + continue; + } + log::warn!("{diagnostic} outcome=unconfirmed; recovery exhausted"); + return Err(MODEL_CONFIRMATION_EXHAUSTED_MESSAGE.to_string()); + } + Err(error) => { + log::warn!("{diagnostic} outcome=failed code={}", i32::from(error.code)); + return Err(format!( + "Failed to set selected ACP {label} config option: {error:?}" + )); + } + } + } +} + // Fragment shared by `setup_acp_session`'s required-transport check and // [`is_missing_mcp_transport_error`] — the same producer/matcher contract as // the config-selection constants above. @@ -5499,6 +5676,7 @@ pub fn is_missing_mcp_transport_error(error: &str) -> bool { async fn apply_or_record_session_config_options( connection: &ConnectionTo, agent_session_id: &str, + adapter: &str, initial_options: Option<&[SessionConfigOption]>, selections: &[AcpSessionConfigOptionSelection], writer: &Arc, @@ -5548,20 +5726,9 @@ async fn apply_or_record_session_config_options( } Err(e) => return Err(e), }; - let response = connection - .send_request(SetSessionConfigOptionRequest::new( - agent_session_id.to_string(), - resolved.config_id, - resolved.value.value_id.as_str(), - )) - .block_task() - .await - .map_err(|e| { - format!( - "Failed to set selected ACP {} config option: {e:?}", - config_selection_label(&selection.category) - ) - })?; + let response = + send_config_selection(connection, agent_session_id, adapter, selection, &resolved) + .await?; // Announced only once the agent has accepted the substitute, so the // transcript never claims a fallback that did not take effect. if let Some(notice) = resolved.value.fallback_notice { @@ -6019,6 +6186,8 @@ async fn setup_acp_session(context: AcpSessionSetupContext<'_>) -> Result) -> Result) -> Result AcpInitializeMetad } } -fn log_initialize_response(agent_label: &str, init_response: &InitializeResponse) { - let agent_name = init_response +/// The name the agent gave itself on `initialize` (e.g. +/// `@agentclientprotocol/claude-agent-acp`), or the driver label when it sent +/// none — so a remote bridge is still recognized by what it is, not where it +/// runs. +fn adapter_name<'a>(agent_label: &'a str, init_response: &'a InitializeResponse) -> &'a str { + init_response .agent_info .as_ref() .map(|info| info.name.as_str()) - .unwrap_or(agent_label); + .unwrap_or(agent_label) +} + +fn log_initialize_response(agent_label: &str, init_response: &InitializeResponse) { + let agent_name = adapter_name(agent_label, init_response); let agent_version = init_response .agent_info .as_ref() @@ -6480,6 +6659,7 @@ impl MessageWriter for BasicMessageWriter { #[cfg(test)] mod tests { + mod model_confirmation; mod response_completion; mod settlement; @@ -6491,7 +6671,7 @@ mod tests { autoapprove_permission_decision, background_continuation_origin, background_task_tracking_meta, build_prompt_content_blocks, carry_config_value_hint, consume_remote_acp_line, decode_remote_acp_line, defensive_permission_decision, - describe_prompt_error, hold_for_background_quiescence, + describe_prompt_error, describe_protocol_failure, hold_for_background_quiescence, is_config_selection_unavailable_error, is_missing_mcp_transport_error, labeled_background_continuation_origin, mcp_server_transport_supported, origin_task_name_label, permission_response_for_decision, permission_response_for_options, @@ -6621,10 +6801,12 @@ mod tests { .expect_err("auth_required should be surfaced without authenticate"); assert!(format!("{error:?}").contains("ACP authentication is required")); - // The exact string the session alert shows once `run` has wrapped it; - // `authRecovery.test.ts` pins the same bytes on the frontend side. + // The exact string the session alert shows once `run` has passed it + // through `describe_protocol_failure`; `authRecovery.test.ts` pins the + // same bytes on the frontend side, so the unwrap there must leave + // auth-required errors in their Debug shape. assert_eq!( - format!("ACP protocol failed: {error:?}"), + describe_protocol_failure(error), r#"ACP protocol failed: Error { code: -32603: Internal error, message: "Internal error", data: Some(String("ACP authentication is required to create ACP session. Sign this agent in, then retry.")) }"# ); assert_eq!(calls.lock().unwrap().as_slice(), &["session/new"]); @@ -6682,13 +6864,11 @@ mod tests { ); assert!(error.contains("authentication")); - // `run` wraps every protocol failure the same way before it reaches - // the session alert; `authRecovery.test.ts` pins this exact string on - // the frontend side, so the two must agree byte for byte. - let wrapped = format!( - "ACP protocol failed: {:?}", - agent_client_protocol::util::internal_error(error) - ); + // `run` passes every protocol failure through + // `describe_protocol_failure` before it reaches the session alert; + // `authRecovery.test.ts` pins this exact string on the frontend side, + // so the two must agree byte for byte. + let wrapped = describe_protocol_failure(agent_client_protocol::util::internal_error(error)); assert_eq!( wrapped, r#"ACP protocol failed: Error { code: -32603: Internal error, message: "Internal error", data: Some(String("ACP authentication is required to run the prompt. Sign this agent in, then retry.")) }"# @@ -7151,8 +7331,10 @@ mod tests { .await .expect_err("auth_required at session/load should fail the setup"); + // Same contract as the `session/new` variant: `describe_protocol_failure` + // must keep auth-required errors in the Debug shape the frontend pins. assert_eq!( - format!("ACP protocol failed: {error:?}"), + describe_protocol_failure(error), r#"ACP protocol failed: Error { code: -32603: Internal error, message: "Internal error", data: Some(String("ACP authentication is required to load ACP session. Sign this agent in, then retry.")) }"# ); assert_eq!( @@ -7585,6 +7767,7 @@ mod tests { apply_or_record_session_config_options( &connection, "session-1", + "Claude Code", Some(&initial_options), &selections, &message_writer, @@ -7665,6 +7848,7 @@ agent: http=false, sse=false). Select a provider that supports MCP over HTTP/SSE apply_or_record_session_config_options( &connection, "session-1", + "Claude Code", None, &selections, &message_writer, @@ -7676,6 +7860,7 @@ agent: http=false, sse=false). Select a provider that supports MCP over HTTP/SSE let error = apply_or_record_session_config_options( &connection, "session-1", + "Claude Code", None, &selections, &message_writer, @@ -7751,6 +7936,7 @@ agent: http=false, sse=false). Select a provider that supports MCP over HTTP/SSE apply_or_record_session_config_options( &connection, "session-1", + "Claude Code", Some(&loaded_options), &selections, &message_writer, @@ -7820,6 +8006,7 @@ agent: http=false, sse=false). Select a provider that supports MCP over HTTP/SSE apply_or_record_session_config_options( &connection, "session-1", + "Claude Code", Some(&renamed_options()), &selections, &message_writer, @@ -7892,6 +8079,7 @@ agent: http=false, sse=false). Select a provider that supports MCP over HTTP/SSE apply_or_record_session_config_options( &connection, "session-1", + "Claude Code", Some(&renamed_options()), &selections, &message_writer, diff --git a/crates/acp-client/src/driver/tests/model_confirmation.rs b/crates/acp-client/src/driver/tests/model_confirmation.rs new file mode 100644 index 000000000..74dd66302 --- /dev/null +++ b/crates/acp-client/src/driver/tests/model_confirmation.rs @@ -0,0 +1,725 @@ +//! Bounded recovery for Claude's `Couldn't confirm model` failure while the +//! saved model selection is applied before the first prompt. +//! +//! The scenarios drive `run_acp_session` end to end against a scripted +//! in-process agent, so the setup timeout, the cancellation boundary, and +//! the prompt queue under test are the production ones. Time is paused: the +//! one-second retry delay and the 90-second setup deadline elapse instantly +//! once nothing else can make progress. + +use super::*; +use crate::driver::{ + describe_protocol_failure, is_model_confirmation_error, run_acp_session, + supports_model_confirmation_recovery, ACP_SETUP_TIMEOUT, MODEL_CONFIRMATION_EXHAUSTED_MESSAGE, +}; +use agent_client_protocol::schema::v1::{ + Implementation, InitializeRequest, InitializeResponse, PromptResponse, +}; +use std::cell::RefCell; +use std::collections::VecDeque; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; +use std::sync::Mutex; + +const CLAUDE_ADAPTER: &str = "@agentclientprotocol/claude-agent-acp"; +const FABLE: &str = "claude-fable-5-1[1m]"; +const SONNET: &str = "claude-sonnet-5-5"; + +/// The exact shape the installed Claude bridge returns: the SDK's sentence +/// under `details` of a plain internal error. +fn confirmation_error(model: &str) -> agent_client_protocol::Error { + agent_client_protocol::Error::internal_error().data(serde_json::json!({ + "details": format!( + "Couldn't confirm model \"{model}\" with the API. Try again, or run /model to see available models." + ), + })) +} + +fn model_option(current: &str) -> SessionConfigOption { + SessionConfigOption::select( + "model", + "Model", + current.to_string(), + vec![ + SessionConfigSelectOption::new(SONNET, "Sonnet"), + SessionConfigSelectOption::new(FABLE, "Fable"), + ], + ) + .category(SessionConfigOptionCategory::Model) +} + +fn effort_option(current: &str, values: &[&str]) -> SessionConfigOption { + SessionConfigOption::select( + "reasoning", + "Reasoning", + current.to_string(), + values + .iter() + .map(|value| SessionConfigSelectOption::new(value.to_string(), value.to_string())) + .collect::>(), + ) + .category(SessionConfigOptionCategory::ThoughtLevel) +} + +fn model_selection(value_id: &str) -> AcpSessionConfigOptionSelection { + AcpSessionConfigOptionSelection { + category: SessionConfigOptionCategory::Model, + config_id: "model".to_string(), + value_id: value_id.to_string(), + } +} + +fn effort_selection(value_id: &str) -> AcpSessionConfigOptionSelection { + AcpSessionConfigOptionSelection { + category: SessionConfigOptionCategory::ThoughtLevel, + config_id: "reasoning".to_string(), + value_id: value_id.to_string(), + } +} + +fn call(config_id: &str, value_id: &str) -> (String, String) { + (config_id.to_string(), value_id.to_string()) +} + +/// What the scripted agent does with the next `session/set_config_option`. +enum ConfigReply { + Accept(Vec), + Fail(agent_client_protocol::Error), + /// Never answer; the request stays in flight until the connection drops. + Hang, +} + +#[derive(Default)] +struct AgentLog { + config_calls: Mutex>, + prompts: AtomicUsize, +} + +impl AgentLog { + fn config_calls(&self) -> Vec<(String, String)> { + self.config_calls.lock().unwrap().clone() + } + + fn prompts(&self) -> usize { + self.prompts.load(Ordering::SeqCst) + } +} + +struct Script { + adapter: &'static str, + initial_options: Vec, + replies: Vec, + /// Cancel this token when the n-th (1-based) config request arrives, + /// before its scripted reply — a user Stop racing the recovery. + cancel_on_config_call: Option<(usize, CancellationToken)>, +} + +impl Script { + fn claude(initial_options: Vec, replies: Vec) -> Self { + Self { + adapter: CLAUDE_ADAPTER, + initial_options, + replies, + cancel_on_config_call: None, + } + } +} + +struct Run { + outcome: Result<(AgentRunOutcome, SessionSettleReason), String>, + agent: Arc, + writer: Arc, +} + +/// One queued prompt through the real `run_acp_session` against `script`. +async fn run_session( + script: Script, + selections: Vec, + cancel: CancellationToken, +) -> Run { + let log = Arc::new(AgentLog::default()); + let log_for_config = Arc::clone(&log); + let log_for_prompt = Arc::clone(&log); + let adapter = script.adapter; + let initial_options = script.initial_options; + let replies = Arc::new(Mutex::new(VecDeque::from(script.replies))); + let cancel_on_config_call = script.cancel_on_config_call; + let agent = agent_client_protocol::Agent + .builder() + .on_receive_request( + async move |request: InitializeRequest, responder, _cx| { + responder.respond( + InitializeResponse::new(request.protocol_version) + .agent_info(Implementation::new(adapter, "0.0.0-test")), + ) + }, + agent_client_protocol::on_receive_request!(), + ) + .on_receive_request( + async move |_request: NewSessionRequest, responder, _cx| { + responder.respond( + NewSessionResponse::new("session-1").config_options(initial_options.clone()), + ) + }, + agent_client_protocol::on_receive_request!(), + ) + .on_receive_request( + async move |request: SetSessionConfigOptionRequest, responder, _cx| { + let call_index = { + let mut calls = log_for_config.config_calls.lock().unwrap(); + calls.push(( + request.config_id.to_string(), + request + .value + .as_value_id() + .expect("selected config value should be a value ID") + .to_string(), + )); + calls.len() + }; + if let Some((at, token)) = &cancel_on_config_call { + if *at == call_index { + token.cancel(); + } + } + let reply = replies + .lock() + .unwrap() + .pop_front() + .expect("more config requests than scripted replies"); + match reply { + ConfigReply::Accept(options) => { + responder.respond(SetSessionConfigOptionResponse::new(options)) + } + ConfigReply::Fail(error) => responder.respond_with_error(error), + ConfigReply::Hang => { + std::future::pending::<()>().await; + unreachable!("a hung request never resolves") + } + } + }, + agent_client_protocol::on_receive_request!(), + ) + .on_receive_request( + async move |_request: PromptRequest, responder, _cx| { + log_for_prompt.prompts.fetch_add(1, Ordering::SeqCst); + responder.respond(PromptResponse::new(StopReason::EndTurn)) + }, + agent_client_protocol::on_receive_request!(), + ); + + let writer = Arc::new(RecordingMessageWriter::default()); + let message_writer: Arc = writer.clone(); + let handler = Arc::new(AcpNotificationHandler::new( + message_writer, + false, + vec![], + cancel.clone(), + )); + let store: Arc = Arc::new(RecordingStore::default()); + let (prompt_tx, mut prompt_rx) = mpsc::unbounded_channel(); + let (_hold_control_tx, mut hold_control_rx) = mpsc::unbounded_channel(); + let hold_active = AtomicBool::new(false); + let pending_reply = RefCell::new(None); + let child_exited = CancellationToken::new(); + let (reply_tx, _reply_rx) = oneshot::channel(); + prompt_tx + .send(QueuedSessionTurn { + prompt: "hello".to_string(), + images: vec![], + reply: reply_tx, + }) + .expect("queued turn should send"); + + let outcome = agent_client_protocol::Client + .connect_with(agent, async |connection| { + Ok(run_acp_session( + &connection, + Path::new("/tmp"), + &store, + "local-session", + None, + &selections, + &handler, + &[], + "Claude Code", + &cancel, + None, + None, + None, + &child_exited, + &mut prompt_rx, + &mut hold_control_rx, + &hold_active, + &pending_reply, + ) + .await) + }) + .await + .expect("the connection itself stays healthy"); + + Run { + outcome, + agent: log, + writer, + } +} + +#[test] +fn matcher_accepts_only_claudes_confirmation_failure_for_the_requested_model() { + assert!(is_model_confirmation_error( + &confirmation_error(FABLE), + FABLE + )); + // A bridge that flattens `details` to a bare string still matches. + assert!(is_model_confirmation_error( + &agent_client_protocol::Error::internal_error() + .data(format!("Couldn't confirm model \"{FABLE}\" with the API.")), + FABLE, + )); + + // A different model, another error code with the same words, an internal + // error with other details, and one with no data at all are not it. + assert!(!is_model_confirmation_error( + &confirmation_error("claude-opus-5-5"), + FABLE + )); + let mut auth_with_details = agent_client_protocol::Error::auth_required(); + auth_with_details.data = confirmation_error(FABLE).data; + assert!(!is_model_confirmation_error(&auth_with_details, FABLE)); + assert!(!is_model_confirmation_error( + &agent_client_protocol::Error::internal_error() + .data(serde_json::json!({ "details": format!("Invalid value for config option model: {FABLE}") })), + FABLE, + )); + assert!(!is_model_confirmation_error( + &agent_client_protocol::Error::internal_error(), + FABLE + )); +} + +#[test] +fn recovery_is_keyed_to_claude_bridges_only() { + assert!(supports_model_confirmation_recovery(CLAUDE_ADAPTER)); + assert!(supports_model_confirmation_recovery( + "@zed-industries/claude-code-acp" + )); + assert!(supports_model_confirmation_recovery("Claude Code")); + assert!(!supports_model_confirmation_recovery("@openai/codex-acp")); + assert!(!supports_model_confirmation_recovery("Goose")); + assert!(!supports_model_confirmation_recovery("Blox")); +} + +/// The terminal sentence must keep the saved selection — the runner clears a +/// selection only when `is_config_selection_unavailable_error` fires — and it +/// must reach the session alert as the sentence itself: `run` unwraps the +/// `internal_error` the connection closure wrapped it in, while every other +/// setup failure keeps the Debug shape the auth-recovery fixtures pin. +#[test] +fn exhausted_message_is_not_an_unavailable_selection() { + assert!(!is_config_selection_unavailable_error( + MODEL_CONFIRMATION_EXHAUSTED_MESSAGE + )); + // Exactly as the `connect_with` closure wraps a setup failure before + // `run` describes it for the session alert. + let shown = describe_protocol_failure(agent_client_protocol::util::internal_error( + MODEL_CONFIRMATION_EXHAUSTED_MESSAGE, + )); + assert_eq!(shown, MODEL_CONFIRMATION_EXHAUSTED_MESSAGE); + assert!(!is_config_selection_unavailable_error(&shown)); + + // Any other setup failure, and a bare transport error, keep the shape + // `isAuthenticationError` and the driver's auth tests are keyed to. + let auth = agent_client_protocol::util::internal_error( + "ACP authentication is required to create ACP session. Sign this agent in, then retry.", + ); + assert_eq!( + describe_protocol_failure(auth.clone()), + format!("ACP protocol failed: {auth:?}") + ); + let transport = agent_client_protocol::Error::internal_error(); + assert_eq!( + describe_protocol_failure(transport.clone()), + format!("ACP protocol failed: {transport:?}") + ); +} + +#[tokio::test(flavor = "current_thread", start_paused = true)] +async fn a_failed_confirmation_is_retried_once_then_effort_applies_and_one_prompt_runs() { + let after_model = vec![ + model_option(FABLE), + effort_option("medium", &["low", "medium", "high"]), + ]; + let after_effort = vec![ + model_option(FABLE), + effort_option("high", &["low", "medium", "high"]), + ]; + let run = run_session( + Script::claude( + vec![ + model_option(SONNET), + effort_option("medium", &["low", "medium", "high"]), + ], + vec![ + ConfigReply::Fail(confirmation_error(FABLE)), + ConfigReply::Accept(after_model), + ConfigReply::Accept(after_effort.clone()), + ], + ), + vec![model_selection(FABLE), effort_selection("high")], + CancellationToken::new(), + ) + .await; + + assert_eq!( + run.outcome + .expect("recovered setup should complete the turn"), + (AgentRunOutcome::Completed, SessionSettleReason::Immediate) + ); + assert_eq!( + run.agent.config_calls(), + vec![ + call("model", FABLE), + call("model", FABLE), + call("reasoning", "high") + ], + "the identical model request goes out exactly twice, then effort" + ); + assert_eq!(run.agent.prompts(), 1, "the queued prompt is released once"); + let updates = run.writer.config_option_updates.lock().unwrap(); + assert_eq!(updates.as_slice(), &[after_effort]); + assert!(run + .writer + .config_option_fallbacks + .lock() + .unwrap() + .is_empty()); +} + +#[tokio::test(flavor = "current_thread", start_paused = true)] +async fn two_failed_confirmations_fail_setup_plainly_without_prompting() { + let run = run_session( + Script::claude( + vec![model_option(SONNET)], + vec![ + ConfigReply::Fail(confirmation_error(FABLE)), + ConfigReply::Fail(confirmation_error(FABLE)), + ], + ), + vec![model_selection(FABLE)], + CancellationToken::new(), + ) + .await; + + let error = run.outcome.expect_err("exhausted recovery fails the setup"); + assert_eq!(error, MODEL_CONFIRMATION_EXHAUSTED_MESSAGE); + assert!( + !is_config_selection_unavailable_error(&error), + "the saved selection must survive a confirmation failure" + ); + assert_eq!( + run.agent.config_calls(), + vec![call("model", FABLE), call("model", FABLE)] + ); + assert_eq!(run.agent.prompts(), 0); + assert!(run.writer.config_option_updates.lock().unwrap().is_empty()); +} + +#[tokio::test(flavor = "current_thread", start_paused = true)] +async fn cancelling_during_the_retry_delay_sends_nothing_further() { + let cancel = CancellationToken::new(); + let run = run_session( + Script { + cancel_on_config_call: Some((1, cancel.clone())), + ..Script::claude( + vec![model_option(SONNET)], + vec![ConfigReply::Fail(confirmation_error(FABLE))], + ) + }, + vec![model_selection(FABLE)], + cancel, + ) + .await; + + assert_eq!( + run.outcome.expect("a cancel is a clean outcome"), + (AgentRunOutcome::Cancelled, SessionSettleReason::Cancelled) + ); + assert_eq!(run.agent.config_calls(), vec![call("model", FABLE)]); + assert_eq!(run.agent.prompts(), 0); +} + +#[tokio::test(flavor = "current_thread", start_paused = true)] +async fn cancelling_during_the_second_attempt_sends_no_prompt() { + let cancel = CancellationToken::new(); + let run = run_session( + Script { + cancel_on_config_call: Some((2, cancel.clone())), + ..Script::claude( + vec![model_option(SONNET)], + vec![ + ConfigReply::Fail(confirmation_error(FABLE)), + ConfigReply::Hang, + ], + ) + }, + vec![model_selection(FABLE)], + cancel, + ) + .await; + + assert_eq!( + run.outcome.expect("a cancel is a clean outcome"), + (AgentRunOutcome::Cancelled, SessionSettleReason::Cancelled) + ); + assert_eq!( + run.agent.config_calls(), + vec![call("model", FABLE), call("model", FABLE)] + ); + assert_eq!(run.agent.prompts(), 0); +} + +#[tokio::test(flavor = "current_thread", start_paused = true)] +async fn the_setup_deadline_still_bounds_a_hung_second_attempt() { + let run = run_session( + Script::claude( + vec![model_option(SONNET)], + vec![ + ConfigReply::Fail(confirmation_error(FABLE)), + ConfigReply::Hang, + ], + ), + vec![model_selection(FABLE)], + CancellationToken::new(), + ) + .await; + + let error = run.outcome.expect_err("the setup deadline fails the run"); + assert_eq!( + error, + format!( + "Timed out waiting for ACP protocol startup after {}s", + ACP_SETUP_TIMEOUT.as_secs() + ) + ); + assert_eq!( + run.agent.config_calls(), + vec![call("model", FABLE), call("model", FABLE)] + ); + assert_eq!(run.agent.prompts(), 0, "no late prompt after the deadline"); +} + +/// The retry carries the *resolved* value — here the hint-stripped rename a +/// resumed pin falls back to — and the fallback is announced once, only +/// after the agent has accepted it. +#[tokio::test(flavor = "current_thread", start_paused = true)] +async fn a_retried_fallback_value_is_announced_once_after_acceptance() { + let renamed = || { + vec![SessionConfigOption::select( + "model", + "Model", + "claude-fable-5-1", + vec![ + SessionConfigSelectOption::new("claude-fable-5-1", "Fable"), + SessionConfigSelectOption::new("claude-opus-5[1m]", "Opus"), + ], + ) + .category(SessionConfigOptionCategory::Model)] + }; + let run = run_session( + Script::claude( + renamed(), + vec![ + ConfigReply::Fail(confirmation_error("claude-fable-5-1")), + ConfigReply::Accept(renamed()), + ], + ), + vec![model_selection("claude-fable-5[1m]")], + CancellationToken::new(), + ) + .await; + + assert!(run.outcome.is_ok()); + assert_eq!( + run.agent.config_calls(), + vec![ + call("model", "claude-fable-5-1"), + call("model", "claude-fable-5-1") + ] + ); + assert_eq!(run.agent.prompts(), 1); + assert_eq!( + run.writer + .config_option_fallbacks + .lock() + .unwrap() + .as_slice(), + &[String::from( + "Selected model claude-fable-5[1m] was not available, falling back to \ + claude-fable-5-1 (Fable)." + )] + ); +} + +/// A model accepted on the retry can still narrow the effort options; the +/// existing stale-effort skip applies to the options the retry returned. +#[tokio::test(flavor = "current_thread", start_paused = true)] +async fn effort_options_changed_by_the_retried_model_keep_the_stale_effort_skip() { + let after_model = vec![ + model_option(FABLE), + effort_option("medium", &["low", "medium"]), + ]; + let run = run_session( + Script::claude( + vec![ + model_option(SONNET), + effort_option("medium", &["low", "medium", "high"]), + ], + vec![ + ConfigReply::Fail(confirmation_error(FABLE)), + ConfigReply::Accept(after_model.clone()), + ], + ), + vec![model_selection(FABLE), effort_selection("high")], + CancellationToken::new(), + ) + .await; + + assert!(run.outcome.is_ok()); + assert_eq!( + run.agent.config_calls(), + vec![call("model", FABLE), call("model", FABLE)], + "the now-unavailable effort is skipped, not sent" + ); + assert_eq!(run.agent.prompts(), 1); + assert_eq!( + run.writer.config_option_updates.lock().unwrap().as_slice(), + &[after_model] + ); +} + +/// The bridge re-applies an already-current model on purpose (to repair SDK +/// drift), so an advertised match must not short-circuit `setModel`. +#[tokio::test(flavor = "current_thread", start_paused = true)] +async fn an_already_current_model_is_still_applied() { + let run = run_session( + Script::claude( + vec![model_option(FABLE)], + vec![ConfigReply::Accept(vec![model_option(FABLE)])], + ), + vec![model_selection(FABLE)], + CancellationToken::new(), + ) + .await; + + assert!(run.outcome.is_ok()); + assert_eq!(run.agent.config_calls(), vec![call("model", FABLE)]); + assert_eq!(run.agent.prompts(), 1); +} + +/// Everything that is not Claude failing to confirm the selected model keeps +/// the single attempt and the existing Debug-shaped error. +#[tokio::test(flavor = "current_thread", start_paused = true)] +async fn other_failures_and_other_adapters_keep_the_single_attempt() { + struct Case { + name: &'static str, + adapter: &'static str, + selection: AcpSessionConfigOptionSelection, + error: agent_client_protocol::Error, + expected_call: (String, String), + expected_prefix: &'static str, + } + let cases = vec![ + Case { + name: "authentication required", + adapter: CLAUDE_ADAPTER, + selection: model_selection(FABLE), + error: agent_client_protocol::Error::auth_required(), + expected_call: call("model", FABLE), + expected_prefix: "Failed to set selected ACP model config option: Error { code: -32000", + }, + Case { + name: "unrelated internal error", + adapter: CLAUDE_ADAPTER, + selection: model_selection(FABLE), + error: agent_client_protocol::Error::internal_error(), + expected_call: call("model", FABLE), + expected_prefix: "Failed to set selected ACP model config option: Error { code: -32603", + }, + Case { + name: "confirmation failure for a different model", + adapter: CLAUDE_ADAPTER, + selection: model_selection(FABLE), + error: confirmation_error("claude-opus-5-5"), + expected_call: call("model", FABLE), + expected_prefix: "Failed to set selected ACP model config option: Error { code: -32603", + }, + Case { + name: "the same words from another adapter", + adapter: "@openai/codex-acp", + selection: model_selection(FABLE), + error: confirmation_error(FABLE), + expected_call: call("model", FABLE), + expected_prefix: "Failed to set selected ACP model config option: Error { code: -32603", + }, + Case { + name: "the same words on the effort option", + adapter: CLAUDE_ADAPTER, + selection: effort_selection("high"), + error: confirmation_error("high"), + expected_call: call("reasoning", "high"), + expected_prefix: + "Failed to set selected ACP effort config option: Error { code: -32603", + }, + ]; + + for case in cases { + let run = run_session( + Script { + adapter: case.adapter, + ..Script::claude( + vec![ + model_option(SONNET), + effort_option("medium", &["low", "medium", "high"]), + ], + vec![ConfigReply::Fail(case.error)], + ) + }, + vec![case.selection], + CancellationToken::new(), + ) + .await; + + let error = run + .outcome + .expect_err(&format!("{}: should fail the setup", case.name)); + assert!( + error.starts_with(case.expected_prefix), + "{}: unexpected error {error:?}", + case.name + ); + assert_eq!( + run.agent.config_calls(), + vec![case.expected_call], + "{}: exactly one attempt", + case.name + ); + assert_eq!(run.agent.prompts(), 0, "{}: no prompt", case.name); + } +} + +/// A stale pin is still reported as unavailable before any request goes out, +/// so the runner keeps clearing it exactly as before. +#[tokio::test(flavor = "current_thread", start_paused = true)] +async fn an_unavailable_selection_is_still_reported_without_a_request() { + let run = run_session( + Script::claude(vec![model_option(SONNET)], vec![]), + vec![model_selection("claude-haiku-4")], + CancellationToken::new(), + ) + .await; + + let error = run.outcome.expect_err("a stale pin fails the setup"); + assert!(is_config_selection_unavailable_error(&error), "{error}"); + assert!(run.agent.config_calls().is_empty()); + assert_eq!(run.agent.prompts(), 0); +} diff --git a/crates/acp-client/src/lib.rs b/crates/acp-client/src/lib.rs index f2084d29a..a474d0d48 100644 --- a/crates/acp-client/src/lib.rs +++ b/crates/acp-client/src/lib.rs @@ -37,6 +37,7 @@ pub use driver::{ BackgroundHoldStatus, BackgroundHoldTask, BasicMessageWriter, MessageWriter, OutOfTurnPermissionPolicy, PromptImage, ReplayBoundary, SessionConnection, SessionLifetime, SessionSettleReason, SessionSettled, Store, BACKGROUND_CONTINUATION_ORIGIN, + MODEL_CONFIRMATION_EXHAUSTED_MESSAGE, }; pub use simple::{run_acp_prompt, run_acp_prompt_with_interpreter_env_snapshot}; pub use types::{