fix(acp): classify 'Authentication required' as non-retryable auth error (#3831) - #3881
fix(acp): classify 'Authentication required' as non-retryable auth error (#3831)#3881iroiro147 wants to merge 2 commits into
Conversation
Expired subscription credentials surface from claude-agent-acp as a bare 'Authentication required' AgentError (code -32000), distinct from the two variants PR block#2751 already classified ('Re-authenticate', 'API Error: 401'). is_auth_error() missed it, so these errors fell into the generic requeue path and burned ~10 retry slots over a long backoff before dead-lettering, while presence showed the agent 'online' with no user-visible signal. Add the 'Authentication required' pattern so the expired-credential case hits the immediate dead-letter + in-channel re-auth notice path. The classifier's precision bar is preserved: the pattern is a credential hand, not a generic failure, and false-positive risk (silently dropping a user message) stays nil for transient/transport errors, which remain non-AgentError and unclassified. Fail-first test is_auth_error_matches_authentication_required_message covers the reporter-observed message; adjacent rejection tests (usage-credit, I/O, WriteTimeout) confirm no over-classification. Refs block#3831 Signed-off-by: iroiro147 <sarthak.singh@juspay.in>
|
treating auth-required as non-retryable is the right call for #3831. maybe also flip presence offline in the same path so the ui doesn't keep saying online. |
The auth dead-letter path kept the presence heartbeat publishing 'online' while every subsequent mention dead-letters, so the UI shows the agent as available through a guaranteed outage. Thread the loop-scoped (presence_publisher, presence_keys) into handle_prompt_result and, on the non-retryable auth branch, best-effort publish kind:20001 'offline' (2s timeout, mirrors the shutdown flip). Gated on presence_enabled; the spawned task is non-blocking and never panics the dispatcher. Review suggestion from Chessing234 on block/buzz PR block#3881. Signed-off-by: iroiro147 <sarthak.singh@juspay.in>
|
@Chessing234 Done — presence now flips offline on the same non-retryable auth path. Expired credentials previously dead-lettered while the heartbeat kept publishing
So the moment the credential is known-dead, presence reflects reality instead of waiting for the agent to exit. All 22 error-emission tests still green, including If you'd rather not tie this to the existing PR's classification change, happy to split it into a follow-up — but it felt tightly scoped to the same wart. |
|
Done — |
Fixes #3831
Problem
Expired Claude subscription credentials surface from
claude-agent-acpas a bare AgentError:is_auth_error()only matched the two variants PR #2751 added (Re-authenticate,API Error: 401), so this third variant fell into the genericqueue.requeue()path. Every mention burned ~10 retry slots over a long backoff before dead-lettering — while presence kept the agentonlineand nothing told the user the agent was effectively dead. The reporter (#3831) had a containerUp, socket connected, and no signal for hours after credential expiry.Root cause
crates/buzz-acp/src/lib.rsis_auth_error()pattern-match missed the subprocess's bare-credential message.Fix (narrow, +1 arm)
Add
message.contains("Authentication required")alongside the two existing arms inis_auth_error(). Now the expired-credential case hits the existing immediate dead-letter + in-channel re-auth notice path (post_failure_notice), so each failed mention surfaces where the user is already looking.Precision bar preserved (per the fn's doc-comment): the matched token is a subprocess credential hand, not a generic failure string. Adjacent tests prove no over-classification — transient/transport (
Io,WriteTimeout) and non-authAgentError(usage-credit) variants remainfalse.Not in scope (deliberately)
Both are listed as follow-ups on #3831; this PR closes the silent-failure root for the documented message variant.
Test
is_auth_error_matches_authentication_required_message— fail-first (red onorigin/mainbefore the fix, green after).is_auth_error_*(reauthenticate / 401 / reject-non-auth / reject-transport) all pass.cargo test -p buzz-acp --libgreen (650/650 serially; 2 keepalive/idle timers are pre-existing flakes under parallel load, unrelated — pass in isolation on cleanorigin/main).cargo clippy -p buzz-acp --libclean.DCO
--signoffpresent. No changeset (repo FILE-SIZE/no-changeset convention).