Skip to content

Propagate auth errors through stale cache fallback#4981

Merged
reyortiz3 merged 1 commit intomainfrom
worktree-stale-config-when-oauth-fails
Apr 21, 2026
Merged

Propagate auth errors through stale cache fallback#4981
reyortiz3 merged 1 commit intomainfrom
worktree-stale-config-when-oauth-fails

Conversation

@reyortiz3
Copy link
Copy Markdown
Contributor

Summary

  • Why: CachedAPIRegistryProvider.refreshCache() treated every API error as a signal to fall back to stale cached data — including auth failures. Users saw previously-cached registry contents with no error after their token was revoked (401/403) or after the OAuth browser flow timed out, masking the actual failure entirely (thv registry list serves stale cache when OAuth flow fails, masking auth failure #4950).
  • What: Two targeted fixes that keep the graceful degradation for transient failures while surfacing auth errors immediately:
    • auth.Transport.RoundTrip now wraps all Token() failures with ErrRegistryAuthRequired, classifying any token-acquisition failure as an auth error before it can be flattened into an UnavailableError.
    • refreshCache() checks for ErrRegistryAuthRequired / ErrRegistryUnauthorized before the stale-cache branch; transient errors (network blip, 5xx) still degrade gracefully.
  • Tests: Adds provider_cached_authbug_test.go with two tests (taken from draft PR Reproduce registry auth stale-cache bug #4951) that reproduce the two distinct error paths and now pass with the fix applied.

Fixes #4950

Type of change

  • Bug fix

Test plan

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

Changes

File Change
pkg/registry/auth/transport.go Wrap Token() errors with ErrRegistryAuthRequired sentinel
pkg/registry/provider_cached.go Guard stale-cache fallback against auth sentinel errors
pkg/registry/provider_cached_authbug_test.go Two regression tests reproducing the 401/403 and OAuth-timeout paths

Does this introduce a user-facing change?

Yes. thv registry list now returns an auth error (and exits non-zero) when the registry rejects credentials or the OAuth browser flow fails, instead of silently printing stale cached registry content.

Special notes for reviewers

  • The two-change approach is necessary: fixing only refreshCache() with a sentinel check would catch server-side 401/403 (Path 1) but miss OAuth browser-flow timeouts (Path 2), because that error chain carries no sentinel before the transport fix. The transport fix is the correct classification point — any failure to obtain a token is an auth error.
  • The existing TestTransport_RoundTrip/source_returns_error_propagates_error test still passes because it uses ErrorContains(t, err, "failed to get auth token") and the new message still contains that substring.
  • CachedAPIRegistryProvider is used by pkg/api/v1/registry*.go and pkg/mcp/server/search_registry.go too, so this fix benefits headless contexts beyond the CLI.
  • Draft PR Reproduce registry auth stale-cache bug #4951 (the reproducer) can be closed once this lands.

Generated with Claude Code

refreshCache() was returning stale cached data for any API error,
including auth failures (401/403 from registry and OAuth flow timeouts).
Users saw previously-cached registry content with no error after their
credentials expired or the OAuth browser flow timed out.

Fix in two parts:
- auth.Transport.RoundTrip now wraps all Token() errors with
  ErrRegistryAuthRequired, classifying token-acquisition failures as auth
  errors regardless of the underlying cause.
- refreshCache() checks for ErrRegistryAuthRequired/ErrRegistryUnauthorized
  before the stale-cache fallback; transient errors (network, 5xx) still
  degrade gracefully.

Fixes #4950

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions github-actions Bot added the size/S Small PR: 100-299 lines changed label Apr 21, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.62%. Comparing base (ee823d8) to head (45fd65b).
⚠️ Report is 27 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4981      +/-   ##
==========================================
+ Coverage   69.43%   69.62%   +0.18%     
==========================================
  Files         539      552      +13     
  Lines       55760    55951     +191     
==========================================
+ Hits        38716    38954     +238     
+ Misses      14069    13996      -73     
- Partials     2975     3001      +26     

☔ View full report in Codecov by Sentry.
📢 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.

@reyortiz3 reyortiz3 merged commit 63ee62a into main Apr 21, 2026
42 checks passed
@reyortiz3 reyortiz3 deleted the worktree-stale-config-when-oauth-fails branch April 21, 2026 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Small PR: 100-299 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

thv registry list serves stale cache when OAuth flow fails, masking auth failure

2 participants