Report a rejected stored credential as re-login required

[aponcedeleonch]

Summary

A non-interactive token source that had exhausted every cache tier returned its last error verbatim. For a refresh token the IdP had rejected (expired, revoked, or rotated out from under us) that meant a raw invalid_grant surfaced instead of the caller's FallbackErr sentinel.

That inverts the two outcomes:

• A cache miss gets ErrTokenRequired, the actionable "you must log in again" error.
• A dead credential, the case that genuinely requires an interactive login, gets an opaque OAuth error that every downstream consumer reads as a transient provider fault.

Concretely, pkg/llm/proxy keys off ErrTokenRequired to return a 401 naming thv llm setup, and everything else becomes a 502 server_error. So a user whose refresh token died was told the gateway was having a problem, with nothing to act on, and retry loops kept hammering the token endpoint with a credential the IdP had already refused.

This was found while diagnosing a real incident: an agent lost gateway access for nearly three hours because the only error it could show was "provider unhealthy, retry in 30s", while the underlying invalid_grant never surfaced.

What changed:

• Token() now classifies the terminal error. An RFC 6749 invalid_grant is reported as FallbackErr with the cause still reachable via errors.As; everything else keeps surfacing verbatim so callers can retry or fix the real problem.
• The sentinel is built for invalid_grant alone. invalid_client, unauthorized_client, and invalid_scope are just as permanent and just as pointless to retry, but they indict the client registration or the requested scopes rather than the refresh token, so a fresh login reproduces them unchanged. Reporting them as FallbackErr would send the user in a circle while hiding the code that names the real problem.
• The rendered message interpolates nothing from the token endpoint. Both the raw response body (an *oauth2.RetrieveError's own Error() embeds it, and a token endpoint can echo back bearer material) and the parsed error field (arbitrary server-chosen text that may carry secrets or control characters) are untrusted. Since the sentinel is only ever built for invalid_grant, the code conveys nothing the fixed sentence does not. There are tests pinning this.
• The transient/permanent rules move to a new leaf package, pkg/auth/oautherr, so the token source and the workload auth monitor share one implementation. The monitor keeps the broad IsPermanentCredentialError ("should I stop retrying"); the token source uses the narrower IsRejectedRefreshGrant ("is the stored credential dead"). The monitor's behaviour and tests are unchanged.
• ErrTokenRequired's wording widens from "no cached credentials found" to "no usable cached credentials", which is accurate for both an empty cache and a rejected one.

Deliberately not done: the rejected credential is not deleted from the secrets provider. With an IdP that rotates refresh tokens, a sibling process may have just written a newer token under the same key, and the next Token() call re-reads the provider. Deleting on a rejection would destroy that cross-process recovery. Throttling comes from callers no longer treating the failure as retryable.

Type of change

• Bug fix

Test plan

• Unit tests (task test)
• Linting (task lint-fix)

go test -race ./pkg/auth/... and go test ./pkg/llm/... are green; task lint-fix reports 0 issues.

New tests in pkg/auth/tokensource, each verified to fail before the fix:

Test	Asserts
RejectedGrant_ReportsFallbackErr	invalid_grant reports as FallbackErr
RejectedGrant_KeepsCauseReachable	the *oauth2.RetrieveError stays reachable via errors.As
RejectedGrant_MessageCarriesNoServerText	the rendered message is the sentinel plus fixed text, with no response body and no error_description
ClientVerdict_SurfacesVerbatim	invalid_client, unauthorized_client, and invalid_scope are not reported as a dead credential
HostileErrorCode_NeverReachesSentinel	an error field carrying CRLF and a secret cannot reach the sentinel message
TransientVerdict_SurfacesVerbatim	a 5xx is not reported as a dead credential
UnparseableRejection_SurfacesVerbatim	a 4xx with no OAuth error code is not either

Plus table-driven suites for the new pkg/auth/oautherr package covering both predicates.

API Compatibility

• This PR does not break the v1beta1 API.

Changes

File	Change
pkg/auth/oautherr/oautherr.go	New leaf package: the transient/permanent rules plus the narrower IsRejectedRefreshGrant
pkg/auth/tokensource/tokensource.go	classifyTerminalError + credentialRejectedError
pkg/auth/monitored_token_source.go	Private helpers delegate to oautherr; no behaviour change
pkg/llm/tokensource.go	ErrTokenRequired wording covers a rejected credential

Does this introduce a user-facing change?

Yes. A user whose stored LLM gateway credential has expired or been revoked now gets "authentication required, run thv llm setup" instead of an opaque OAuth error, and the LLM proxy returns a 401 with that remediation rather than a 502 server_error.

Special notes for reviewers

The one judgement call worth scrutiny is the boundary of "the stored credential is dead". It is deliberately narrow: only the literal RFC 6749 invalid_grant, and only on a response the transient classifier does not already excuse (so a 429 or 5xx claiming invalid_grant still surfaces verbatim). Everything else, including the other permanent codes, keeps its own error so the operator sees what the IdP actually said.

That narrowness is also what removes the last untrusted interpolation from the message: with only one possible code, there is nothing left to render. Note that on the verbatim path an *oauth2.RetrieveError still prints its response body, which is pre-existing behaviour this PR does not change.

Note also that a locked keyring intentionally still surfaces verbatim rather than as ErrTokenRequired. "Unlock your keyring" and "log in again" are different remediations, and an existing test pins that distinction.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.72%. Comparing base (0b8acf2) to head (880a1aa).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6389      +/-   ##
==========================================
+ Coverage   73.04%   77.72%   +4.67%     
==========================================
  Files         745      750       +5     
  Lines       79208    72128    -7080     
==========================================
- Hits        57857    56060    -1797     
+ Misses      17300    16063    -1237     
+ Partials     4051        5    -4046     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.