Skip to content

fix(acp): classify 'Authentication required' as non-retryable auth error (#3831) #3881

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
iroiro147 wants to merge 2 commits into from
Closed

fix(acp): classify 'Authentication required' as non-retryable auth error (#3831) #3881

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
41f7020
fix(acp): classify 'Authentication required' as non-retryable auth error
iroiro147 Jul 31, 2026
ee70a11
fix(acp): flip presence offline on non-retryable auth dead-letter
iroiro147 Jul 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 57 additions & 1 deletion crates/buzz-acp/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2366,6 +2366,7 @@ async fn tokio_main() -> Result<()> {
&mut respawn_tasks,
observer.clone(),
Some(&ctx.rest_client),
Some((&presence_publisher, &presence_keys)),
) == LoopAction::Exit
{
break;
Expand Down Expand Up @@ -3025,7 +3026,9 @@ 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")
message.contains("Re-authenticate")
|| message.contains("API Error: 401")
|| message.contains("Authentication required")
}

/// Spawn a task that posts a user-visible failure notice to the relay.
Expand Down Expand Up @@ -3064,6 +3067,7 @@ fn handle_prompt_result(
respawn_tasks: &mut tokio::task::JoinSet<()>,
observer: Option<observer::ObserverHandle>,
rest_client: Option<&relay::RestClient>,
presence: Option<(&relay::RelayEventPublisher, &nostr::Keys)>,
) -> LoopAction {
let before = pool.task_map().len();
let agent_index = result.agent.index;
Expand Down Expand Up @@ -3163,6 +3167,31 @@ fn handle_prompt_result(
and then re-send."
.to_string();
spawn_failure_notice(rest_client, &batch, content);
// Auth tokens don't self-repair between heartbeats, and without this the
// presence heartbeat keeps publishing "online" while every subsequent
// mention dead-letters — the UI keeps showing the agent as available during
// a guaranteed outage. Flip presence to offline (best-effort, non-blocking)
// so the UI reflects the real state instead. Mirrors the shutdown flip.
if let Some((publisher, keys)) = presence {
if config.presence_enabled {
let publisher = publisher.clone();
let keys = keys.clone();
tokio::spawn(async move {
match tokio::time::timeout(
Duration::from_secs(2),
publish_presence(&publisher, &keys, "offline"),
)
.await
{
Ok(Ok(_)) => tracing::info!("presence set to offline after auth dead-letter"),
Ok(Err(e)) => {
tracing::warn!("failed to set offline presence after auth dead-letter: {e}")
}
Err(_) => tracing::warn!("auth dead-letter offline presence timed out"),
}
});
}
}
} else if let Some(dead) = queue.requeue(batch) {
let reason = match &result.outcome {
PromptOutcome::Timeout(TimeoutKind::Idle) => "the turn timed out".to_string(),
Expand Down Expand Up @@ -5352,6 +5381,7 @@ mod error_outcome_emission_tests {
&mut respawn_tasks,
Some(observer.clone()),
None,
None,
);

let turn_errors: Vec<_> = observer
Expand Down Expand Up @@ -5517,6 +5547,7 @@ mod error_outcome_emission_tests {
&mut respawn_tasks,
Some(observer.clone()),
None,
None,
);
let events = observer.snapshot();
let turn_error = events.iter().find(|e| e.kind == "turn_error").unwrap();
Expand Down Expand Up @@ -5607,6 +5638,7 @@ mod error_outcome_emission_tests {
&mut respawn_tasks,
None,
None,
None,
);
(
queue.pending_channels(),
Expand Down Expand Up @@ -5712,6 +5744,7 @@ mod error_outcome_emission_tests {
&mut respawn_tasks,
None,
None,
None,
);
(
queue.pending_channels(),
Expand Down Expand Up @@ -5803,6 +5836,7 @@ mod error_outcome_emission_tests {
&mut respawn_tasks,
Some(observer.clone()),
None,
None,
);

let events = observer.snapshot();
Expand Down Expand Up @@ -5896,6 +5930,7 @@ mod error_outcome_emission_tests {
&mut respawn_tasks,
Some(observer.clone()),
None,
None,
);

let events = observer.snapshot();
Expand Down Expand Up @@ -6011,6 +6046,7 @@ mod error_outcome_emission_tests {
&mut respawn_tasks,
Some(observer.clone()),
None,
None,
);

// Batch preserved as a cancelled merge, not dead-lettered — same
Expand Down Expand Up @@ -6143,6 +6179,7 @@ mod error_outcome_emission_tests {
&mut respawn_tasks,
Some(observer.clone()),
None,
None,
);

// No batch to merge — the queue has nothing pending for any channel.
Expand Down Expand Up @@ -6226,6 +6263,23 @@ mod error_outcome_emission_tests {
);
}

#[test]
fn is_auth_error_matches_authentication_required_message() {
// Reporter-observed subprocess message (claude-agent-acp 0.63.0, buzz
// #3831): expired subscription credential surfaces as a bare
// "Authentication required" with code -32000. Without this variant the
// immediate dead-letter + re-auth notice never fires and every mention
// burns ~10 retries before dead-lettering.
let e = acp::AcpError::AgentError {
code: -32000,
message: "Authentication required".to_string(),
};
assert!(
is_auth_error(&e),
"Authentication required variant must be classified as auth error"
);
}

#[test]
fn is_auth_error_rejects_other_agent_error_message() {
let e = acp::AcpError::AgentError {
Expand Down Expand Up @@ -6325,6 +6379,7 @@ mod error_outcome_emission_tests {
&mut respawn_tasks,
None,
None,
None,
);

// The batch must not be requeued: pending_channels returns 0.
Expand Down Expand Up @@ -6410,6 +6465,7 @@ mod error_outcome_emission_tests {
&mut respawn_tasks,
None,
None,
None,
);

// Non-auth application error: batch IS requeued (first attempt, retry budget > 0).
Expand Down