From c0d5d7add9ba26f205d68e5d824d58afe12d5783 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Fri, 24 Jul 2026 15:48:48 -0400 Subject: [PATCH] fix(acp): dead-letter auth errors immediately with re-auth hint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Auth-class errors (expired OAuth token, HTTP 401) are non-retryable: the token won't self-repair between attempts, so requeueing only wastes 10 retry slots and delays the visible failure by a long backoff window. Add is_auth_error() that matches AcpError::AgentError messages containing 'Re-authenticate' or 'API Error: 401' — two narrow patterns observed in Will's canary run. The conservative matching is intentional: a false positive (misclassifying a transient error) silently drops a user message, which is worse than a false negative (extra retries on an auth error). In handle_prompt_result, intercept the failing batch before queue.requeue() for auth-class errors and dead-letter immediately with a notice telling the user to re-authenticate the CLI (e.g. claude /login). The transport/application split in PromptOutcome::Error is untouched; this only changes batch fate after an application-class auth error. 6 tests: is_auth_error classification (Re-authenticate, 401, non-auth AgentError, transport), auth-error dead-letter (no requeue, 0 pending channels), non-auth application error still requeued (1 pending channel). Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- crates/buzz-acp/src/lib.rs | 270 +++++++++++++++++++++++++++++++++++++ 1 file changed, 270 insertions(+) diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 03b75a4211..0230ea0875 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -2978,6 +2978,35 @@ fn dispatch_pending( dispatched_channels } +/// Returns `true` when `error` is a non-retryable authentication failure. +/// +/// Retrying auth errors is harmful: the token won't self-repair between +/// attempts, so each retry wastes an attempt slot, delays the visible failure, +/// and burns the user's context window. Dead-letter immediately and surface a +/// re-authentication hint instead. +/// +/// # Classification rationale +/// +/// Auth failures arrive as [`acp::AcpError::AgentError`] with a message +/// surfaced from the upstream CLI. Two narrow patterns reliably identify +/// non-transient auth failures observed in the field: +/// +/// - `"Re-authenticate"` — emitted by the Claude CLI when an OAuth token has +/// expired ("OAuth access token has expired. Re-authenticate to continue."). +/// Specific to the auth-expiry flow; does not appear in unrelated errors. +/// - `"API Error: 401"` — present in Claude/Codex HTTP-401 responses; 401 is +/// the standard auth-failure status and does not arise from network blips. +/// +/// False positives (misclassifying a transient error as non-retryable) silently +/// drop a user message, which is worse than a false negative (extra retries on +/// an auth error). Both patterns are therefore chosen for high precision. +fn is_auth_error(error: &acp::AcpError) -> bool { + let acp::AcpError::AgentError { message, .. } = error else { + return false; + }; + 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 @@ -3098,6 +3127,21 @@ fn handle_prompt_result( } else { hard_timeout_fate_suffix = Some(" — requeued for retry (recently active)"); } + } else if matches!(&result.outcome, PromptOutcome::Error(e) if is_auth_error(e)) { + // Auth errors are non-retryable: the token won't self-repair + // between retries, so requeueing only wastes attempt slots and + // delays the visible failure. Dead-letter immediately and tell + // the user to re-authenticate the CLI. + tracing::warn!( + channel_id = %batch.channel_id, + events = batch.events.len(), + "dead-lettering batch immediately — non-retryable auth error" + ); + let content = "⚠️ I couldn't process the last request: authentication failed. \ + Please re-authenticate the CLI (e.g. run `claude /login` or `codex login`) \ + and then re-send." + .to_string(); + spawn_failure_notice(rest_client, &batch, content); } else if let Some(dead) = queue.requeue(batch) { let reason = match &result.outcome { PromptOutcome::Timeout(TimeoutKind::Idle) => "the turn timed out".to_string(), @@ -6048,6 +6092,232 @@ mod error_outcome_emission_tests { let app = AcpError::IdleTimeout(std::time::Duration::from_secs(1)); assert_eq!(turn_errors_emitted_for(PromptOutcome::Error(app)).await, 1); } + + // ── is_auth_error classification ─────────────────────────────────────── + + #[test] + fn is_auth_error_matches_reauthenticate_message() { + let e = acp::AcpError::AgentError { + code: -32000, + message: "API Error: OAuth access token has expired. Re-authenticate to continue." + .to_string(), + }; + assert!( + is_auth_error(&e), + "Re-authenticate variant must be classified as auth error" + ); + } + + #[test] + fn is_auth_error_matches_401_message() { + let e = acp::AcpError::AgentError { + code: -32000, + message: "Internal error: API Error: 401 OAuth access token has expired.".to_string(), + }; + assert!( + is_auth_error(&e), + "API Error: 401 variant must be classified as auth error" + ); + } + + #[test] + fn is_auth_error_rejects_other_agent_error_message() { + let e = acp::AcpError::AgentError { + code: -32601, + message: "Usage credits required for 1M context — turn on usage credits".to_string(), + }; + assert!( + !is_auth_error(&e), + "usage-credit error must NOT be classified as auth error" + ); + } + + #[test] + fn is_auth_error_rejects_transport_errors() { + let io = acp::AcpError::Io(std::io::Error::other("pipe broke")); + assert!( + !is_auth_error(&io), + "I/O error must not be classified as auth error" + ); + let timeout = acp::AcpError::WriteTimeout(std::time::Duration::from_secs(5)); + assert!( + !is_auth_error(&timeout), + "WriteTimeout must not be classified as auth error" + ); + } + + // ── auth error dead-letter behavior ──────────────────────────────────── + + /// An auth-class `PromptOutcome::Error` must dead-letter immediately + /// (the batch is never requeued) so the user sees a re-auth hint at once + /// rather than after 10 futile retries. + #[tokio::test] + async fn auth_error_dead_letters_immediately_without_requeueing() { + let keys = nostr::Keys::generate(); + let event = nostr::EventBuilder::new(nostr::Kind::Custom(9), "test") + .sign_with_keys(&keys) + .unwrap(); + let channel_id = uuid::Uuid::new_v4(); + let batch = FlushBatch { + channel_id, + events: vec![BatchEvent { + event, + prompt_tag: "test".into(), + received_at: std::time::Instant::now(), + }], + cancelled_events: vec![], + cancel_reason: None, + }; + + let auth_error = acp::AcpError::AgentError { + code: -32000, + message: "API Error: 401 OAuth access token has expired. Re-authenticate to continue." + .to_string(), + }; + + let agent = dummy_agent(0).await; + let mut pool = AgentPool::from_slots(vec![None]); + let task_id = pool.join_set.spawn(async {}).id(); + pool.task_map_mut().insert( + task_id, + crate::pool::TaskMeta { + agent_index: 0, + channel_id: None, + turn_id: "test-turn-id".to_string(), + recoverable_batch: None, + control_tx: None, + steer_tx: None, + }, + ); + let mut queue = EventQueue::new(config::DedupMode::Queue); + let config = test_config(); + let mut heartbeat_in_flight = false; + let removed_channels = std::collections::HashSet::new(); + let mut crash_history = vec![SlotCircuit { + crash_times: Vec::new(), + open_until: None, + respawn_in_flight: false, + }]; + let (respawn_tx, _respawn_rx) = mpsc::channel(8); + let mut respawn_tasks = tokio::task::JoinSet::new(); + let result = PromptResult { + agent, + source: PromptSource::Channel(channel_id), + turn_id: "test-turn-id".to_string(), + outcome: PromptOutcome::Error(auth_error), + batch: Some(batch), + }; + handle_prompt_result( + &mut pool, + &mut queue, + &config, + result, + &mut heartbeat_in_flight, + &removed_channels, + &mut crash_history, + &respawn_tx, + &mut respawn_tasks, + None, + None, + ); + + // The batch must not be requeued: pending_channels returns 0. + assert_eq!( + queue.pending_channels(), + 0, + "auth error must dead-letter immediately — batch must not be requeued" + ); + assert_eq!( + queue.queued_event_count(&channel_id), + 0, + "auth error must dead-letter immediately — no events should be pending" + ); + } + + /// A non-auth application error (e.g. usage credits) must still follow the + /// standard requeue path so today's behavior is unchanged. + #[tokio::test] + async fn non_auth_application_error_is_requeued() { + let keys = nostr::Keys::generate(); + let event = nostr::EventBuilder::new(nostr::Kind::Custom(9), "test") + .sign_with_keys(&keys) + .unwrap(); + let channel_id = uuid::Uuid::new_v4(); + let batch = FlushBatch { + channel_id, + events: vec![BatchEvent { + event, + prompt_tag: "test".into(), + received_at: std::time::Instant::now(), + }], + cancelled_events: vec![], + cancel_reason: None, + }; + + // Usage-credits error — AgentError but NOT an auth error. + let usage_error = acp::AcpError::AgentError { + code: -32000, + message: "Usage credits required for 1M context".to_string(), + }; + + let agent = dummy_agent(0).await; + let mut pool = AgentPool::from_slots(vec![None]); + let task_id = pool.join_set.spawn(async {}).id(); + pool.task_map_mut().insert( + task_id, + crate::pool::TaskMeta { + agent_index: 0, + channel_id: None, + turn_id: "test-turn-id".to_string(), + recoverable_batch: None, + control_tx: None, + steer_tx: None, + }, + ); + let mut queue = EventQueue::new(config::DedupMode::Queue); + let config = test_config(); + let mut heartbeat_in_flight = false; + let removed_channels = std::collections::HashSet::new(); + let mut crash_history = vec![SlotCircuit { + crash_times: Vec::new(), + open_until: None, + respawn_in_flight: false, + }]; + let (respawn_tx, _respawn_rx) = mpsc::channel(8); + let mut respawn_tasks = tokio::task::JoinSet::new(); + let result = PromptResult { + agent, + source: PromptSource::Channel(channel_id), + turn_id: "test-turn-id".to_string(), + outcome: PromptOutcome::Error(usage_error), + batch: Some(batch), + }; + handle_prompt_result( + &mut pool, + &mut queue, + &config, + result, + &mut heartbeat_in_flight, + &removed_channels, + &mut crash_history, + &respawn_tx, + &mut respawn_tasks, + None, + None, + ); + + // Non-auth application error: batch IS requeued (first attempt, retry budget > 0). + assert_eq!( + queue.pending_channels(), + 1, + "non-auth application error must requeue the batch for retry" + ); + assert_eq!( + queue.queued_event_count(&channel_id), + 1, + "non-auth application error must preserve the event for retry" + ); + } } #[cfg(test)]