fix(acp): recover stale OAuth on long-lived ACP sessions (all hosts)

[joelbrilliant]

Summary

General ACP reliability fix. Any long-lived ACP host can abort turns after idle with a stale OAuth access token, while Telegram / Discord / gateway / CLI on the same HERMES_HOME keep working.

Typical error:

HTTP 403: {"code":"unauthenticated:bad-credentials",
 "error":"The OAuth2 access token could not be validated."}

Who is affected

Surface	Affected?
Any long-lived ACP host (Zed, VS Code ACP client, JetBrains, Buzz, custom hermes acp stdio hosts)	Yes — process stays up, token goes stale
Telegram / Discord / gateway	No — already pass credential_pool into AIAgent
Short-lived CLI turns	Usually no — fresh process re-resolves credentials

Buzz Desktop dogfood is how we found it (owner-only Rocky after ~3h idle). The bug is in Hermes ACP construction and OAuth recovery, not in Buzz.

Root cause

1. ACP session construction called resolve_runtime_provider but did not pass credential_pool into AIAgent. Gateway/CLI do. Without the pool, _recover_with_credential_pool is a no-op on the ACP path.
2. xAI (and similar OAuth providers) often return HTTP 403 for a stale access token, not only 401. Singleton refresh only ran on 401, and generic 403 auth did not set should_rotate_credential.

Result: idle ACP → first multimodal or text turn after hours → non-retryable 403 → user thinks they must reauth, even though gateway still works on the same profile.

Fix

• Forward credential_pool from resolve_runtime_provider in ACP _make_agent (match gateway/CLI)
• Classify generic 403 auth with should_rotate_credential=True (billing/spend-limit 403s unchanged)
• Singleton xAI/Codex refresh on 403 when classifier says auth, not only 401

No config migration. Default editor behaviour unchanged when credentials are fresh.

Test plan

• scripts/run_tests.sh tests/acp/test_session.py tests/run_agent/test_codex_xai_oauth_recovery.py -q (97 passed)
• New unit: ACP forwards credential_pool from runtime
• Existing [Bug]: _is_entitlement_failure over-matches xAI 'bad-credentials' 403 — long-running TUI sessions can't auto-refresh stale OAuth tokens #29344 suite: bad-credentials 403 is auth, not entitlement; pool refresh path runs
• Live: long-lived ACP idle → next turn self-heals without reauth/restart (gateway already OK on same home)

Related

• Found while dogfooding Buzz managed Hermes: feat(acp): Hermes runtime discovery and durable session load block/buzz#2633 (companion host work; not required to land this fix)
• Orthogonal capability PR (acp.tool_policy: profile): feat(acp): opt-in profile tool policy for full-capability hosts #70326
• Prior related work: [Bug]: _is_entitlement_failure over-matches xAI 'bad-credentials' 403 — long-running TUI sessions can't auto-refresh stale OAuth tokens #29344 (entitlement vs bad-credentials disambiguation) — this PR closes the ACP wiring gap that still left long-lived ACP hosts unrecovered

[joelbrilliant]

Reframed for maintainers: this is not Buzz-specific.

Any long-lived hermes acp host (Zed, VS Code ACP, JetBrains, custom stdio hosts) can hit the same idle → 403 bad-credentials abort while Telegram/gateway on the same home stay healthy. Root cause is ACP omitting credential_pool + 403-only-as-auth recovery; gateway already did the right thing.

Buzz dogfood was the discovery path only.

[alt-glitch]

This was generated by AI during triage.

Related to #70293: both repair ACP credential-pool handoff, while this patch also changes generic 403 auth classification and the Codex/xAI refresh retry path. The mechanisms overlap but are not redundant.

[joelbrilliant]

Relationship to #70293

Thanks for the triage note. Fully agree these overlap but are not redundant.

Piece	#70293	This PR (#70465)
Forward credential_pool into ACP AIAgent	yes	yes (same intent)
403 bad-credentials / OAuth-shaped auth → rotate+refresh (not only 401)	no	yes
Classifier: keep generic OpenRouter 403 auth without rotate	n/a	yes (CI-fixed)

Suggested landing order (happy either way):

1. Prefer merging fix(acp): forward resolved credential pool #70293 first if maintainers want the minimal pool-handoff alone.
2. We will rebase this PR onto main after that and drop the duplicated credential_pool line if it is already present, keeping only the 403 recovery path (needed for xAI SuperGrok OAuth, which often returns 403 not 401 for a stale access token).
3. Or merge this PR as the fuller fix if that is simpler for review.

Dogfood: same HERMES_HOME had Telegram healthy while long-lived ACP aborted after ~3h idle with xAI 403 bad-credentials. Pool handoff alone is necessary; the 403 refresh path is what makes idle ACP recover without restart/reauth for that provider.

No conflict intended with @stefanpieter — happy to reshape this PR however maintainers prefer.

[teknium1]

Thanks for isolating the ACP runtime handoff; current main still drops credential_pool between acp_adapter/session.py:636-645, and recovery exits when that field is absent (agent/agent_runtime_helpers.py:930-932).

Problems

• The new 403 condition is too broad. xAI subscription 403s classify as auth (agent/error_classifier.py:797-824), while pool recovery deliberately declines them as entitlement (agent/agent_runtime_helpers.py:1123-1179). After that decline, the proposed conversation_loop.py condition would force-refresh the singleton and retry the entitlement request once.
• The new classifier should_rotate_credential value is not currently consumed for recovery; the loop logs it at agent/conversation_loop.py:3677-3682 but makes the proposed retry decision from classified.reason.
• The added test covers ACP construction only. The modified 403 behavior needs stale-token and entitlement regression cases.

Suggested changes

• Narrow the 403 singleton retry to the existing stale-token discriminator, while preserving current 401 handling.
• Add tests proving WKE/OAuth-validation 403 refreshes, while the known subscription 403 does not refresh.

This is an automated hermes-sweeper review.

[GottZ]

This was generated by AI during triage.

Summary

Three PRs address #70292: #70293 and #77029 contain the same minimal ACP credential-pool handoff and regression assertion, while #70465 includes that handoff plus stale-token-specific 403 classification, refresh logic, and entitlement regressions.

Related pull requests

• fix(acp): forward resolved credential pool #70293 [closed] best fix — (+6/-3) — n/a (recorded best fix for ACP drops credential pool, requiring restart after OAuth token expiry #70292): forwards the resolved credential pool into ACP-created agents and directly tests object retention. Despite the automated keep_open review, this closed PR remains relevant as the source implementation; contributor kshitijk4poor reports that it was cherry-picked with authorship preserved into fix(acp): forward resolved credential pool into ACP-created agents #77029.
• fix(acp): recover stale OAuth on long-lived ACP sessions (all hosts) #70465 related — (+178/-4) — n/a: duplicates the ACP handoff and additionally gates 403 OAuth refresh on stale-token markers, with tests showing stale-token retry and no retry for subscription entitlement failures. This current diff addresses teknium1's blocking concern about broad auth-based 403 retries and supports the COMMENTED keep_open, high-salvageability review.
• fix(acp): forward resolved credential pool into ACP-created agents #77029 best fix — (+6/-3) — n/a (recorded best fix for ACP drops credential pool, requiring restart after OAuth token expiry #70292): carries the same focused credential-pool handoff and sentinel identity regression as fix(acp): forward resolved credential pool #70293, and is the active minimal replacement for that closed PR.

Duplicates

#70293 and #77029 are substantively identical; #70293 is the closed source implementation superseded by #77029. #70465 overlaps on that handoff but is not a full duplicate because it also implements discriminator-gated 403 OAuth recovery and entitlement regressions.

Suggested consolidation

Keep #77029 open as the active minimal handoff replacement and leave #70293 closed as superseded by #77029. For #70465, author action: rebase onto main once the handoff is present there, or split out the salvageable stale-token-specific 403 recovery and entitlement regression coverage; this preserves the keep_open review's requested path without duplicating #77029.

Cross-PR triage: Reviewed 3 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 15 kB of PR diffs, 11 kB of issue/PR text, 5 kB of discussion (8 comments), 4 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.