DRAFT: Wire non-interactive registry auth for serve mode

[ChrisJBurns]

Summary

• Why: When thv serve runs with OAuth-authenticated registries, it must never open a browser for OAuth flows — it's a headless API server. Desktop clients (ToolHive Studio) also need machine-readable auth status in API responses to show the correct UI state, and structured error responses when auth is missing so they can prompt the user to run thv registry login.

• What: This is Phase 2A of RFC-0043 (Registry Authentication). It adds:

  • Non-interactive registry provider for serve mode (WithInteractive(false))
  • auth_status and auth_type fields in registry API responses
  • Structured JSON 503 errors with code: "registry_auth_required" when tokens are missing
  • Typed RegistryHTTPError with Unwrap() for 401/403 detection from registry servers
  • Actionable error messages when unauthenticated registries return 401

Type of change

• New feature

Test plan

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

Changes

File	Change
pkg/registry/api/client.go	Add ErrRegistryUnauthorized, RegistryHTTPError type with Unwrap() for 401/403. Replace generic fmt.Errorf with typed errors in GetServer, fetchServersPage, SearchServers.
pkg/registry/factory.go	Add GetNonInteractiveProviderWithConfig() for serve mode — creates provider with WithInteractive(false).
pkg/registry/provider_api.go	Wrap 401/403 from API as ErrRegistryAuthRequired in GetRegistry(). Produce actionable 401 message in validation probe.
pkg/api/v1/registry.go	Add auth_status/auth_type fields to response structs. Add resolveAuthStatus(), writeRegistryAuthRequiredError(), isRegistryAuthError(). Add serve-mode support to RegistryRoutes. Catch auth errors in listRegistries, getRegistry, listServers.
pkg/api/server.go	Pass serveMode: true to RegistryRouter().
pkg/registry/api/client_test.go	Tests for RegistryHTTPError formatting, Unwrap(), and errors.Is behavior.
pkg/api/v1/registry_auth_test.go	Tests for resolveAuthStatus, isRegistryAuthError, and writeRegistryAuthRequiredError.

Does this introduce a user-facing change?

Yes. Registry API responses now include auth_status and auth_type fields when auth is configured. When thv serve cannot authenticate to a registry (no cached tokens), it returns HTTP 503 with a structured JSON error instead of a generic 500.

Special notes for reviewers

• The auth_status field cannot detect "expired" from config alone (would require calling the token source). We return "authenticated" when a token ref exists and rely on Studio handling subsequent 401s as a signal to re-login.
• HTTP 503 (not 401) is intentional per RFC-0043: Studio is authenticated to the thv serve API — the problem is that thv serve itself lacks a registry credential. 503 correctly signals a server-side dependency issue.
• Phase 2B (next PR) will add thv registry login/logout CLI commands and a POST /api/v1beta/registry/auth/login endpoint so Studio can trigger the browser OAuth flow without the user needing a terminal.

Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 13.63636% with 133 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.70%. Comparing base (ba38a12) to head (cf7770e).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/api/v1/registry.go	10.83%	103 Missing and 4 partials ⚠️
pkg/registry/api/client.go	0.00%	13 Missing ⚠️
pkg/registry/provider_api.go	0.00%	11 Missing ⚠️
pkg/api/server.go	0.00%	1 Missing ⚠️
pkg/registry/factory.go	50.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4111      +/-   ##
==========================================
- Coverage   68.88%   68.70%   -0.18%     
==========================================
  Files         461      464       +3     
  Lines       46562    46873     +311     
==========================================
+ Hits        32075    32205     +130     
- Misses      11987    12130     +143     
- Partials     2500     2538      +38     

☔ 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.

[jhrozek]

Automated review from Claude Code. 14 findings total (3 HIGH, 6 MEDIUM, 5 LOW/INFO). 4 HIGH/MEDIUM findings are pre-existing from PR #3908 and should be tracked separately. 6 inline comments below on code introduced in this PR.

Pre-existing issues to track separately (from PR #3908):

• HIGH: Refresh token rotation not persisted on cache restore path (oauth_token_source.go:94)
• HIGH: Data race between ResetDefaultProvider and GetDefaultProviderWithConfig (factory.go)
• MEDIUM: updateConfigTokenRef bypasses injected config provider (oauth_token_source.go:213)
• MEDIUM: Panic on empty baseURL in api.NewClient (client.go:111)

MEDIUM (not inline — outside diff hunk): resolveTokenSource in factory.go:144 passes cfg.RegistryApiUrl directly, but auth.Login uses registryURLFromConfig(cfg) which falls back to cfg.RegistryUrl. Since the URL feeds DeriveSecretKey, a mismatch means Login could store the token under a different key than factory looks up.

🤖 Generated with Claude Code

[jhrozek]

[ARCHITECTURE] Duplicated cache path derivation (HIGH)

This sha256 → hash[:4] → "toolhive/cache/registry-%x.json" formula is duplicated from pkg/registry/provider_cached.go:74-75, where it uses the persistentCacheSubdir constant. Here it's hardcoded as "cache".

If the cache path formula ever changes in provider_cached.go (different hash length, different subdir), this logout code will silently fail to clear the correct cache file.

Suggestion: Extract a shared RegistryCachePath(registryURL string) (string, error) function in pkg/registry/ and call it from both places. Or add a ClearCache(url string) error function to the registry package.

[jhrozek]

[ARCHITECTURE] resolveAuthStatus overlaps with AuthManager.GetAuthInfo() (MEDIUM)

This function reimplements the same config inspection as AuthManager.GetAuthInfo() in pkg/registry/auth_manager.go:81-91 (checks CachedRefreshTokenRef), but returns three-state strings vs boolean. GetAuthInfo() currently has zero production callers.

Per ToolHive's thin-wrapper principle, consider extending AuthManager with a GetAuthStatus() (status, authType string) method so the API layer just calls through.

[jhrozek]

[DESIGN QUESTION] Browser-open from server process (MEDIUM)

This endpoint calls auth.Login(...) with interactive=true, triggering performOAuthFlow which opens the user's default browser and starts a local callback listener.

Is this intentional for desktop-only serve mode (Studio running thv serve locally)? In headless/remote/container environments this will fail or hang.

Would it be better to return a redirect URL in the JSON response that the client (Studio) opens itself, rather than having the server process open the browser? That would also work behind reverse proxies where localhost:PORT/callback isn't reachable from the browser.

[jhrozek]

[CONVENTIONS] Test plan at repository root (LOW)

This manual test plan references a different branch (feat/registry-auth-serve-mode), mentions "PR 2B, not yet implemented" for login (which is now implemented), and includes an internal registry URL.

Consider removing from committed code and including in the PR description instead, or moving to docs/testing/ if it should persist.

[ChrisJBurns]

Yeah this was just me asking Claude to output an .md file so i can run through things manually locally. This was only committed so I didn't lose it across branches. This won't stay for long after I've proved everything locally.

[jhrozek]

[CONVENTIONS] Missing omitempty on auth_status/auth_type (INFO)

The AuthStatus/AuthType fields on registryInfo and getRegistryResponse (lines 768-771, 798-801) serialize auth_type as "" when no auth is configured. This is inconsistent with other optional fields like Audience in UpdateRegistryAuthRequest which use omitempty.

Is this intentional for easier client parsing (always present)? If so, a brief comment would clarify.

[jhrozek]

[OAUTH] Default scopes defined in three locations (LOW)

The default ["openid", "offline_access"] scopes are defined here, in cmd/thv/app/config_registryauth.go (flag default), and a different set ["openid", "profile", "email"] in pkg/auth/oauth/oidc.go:227.

Consider extracting a single DefaultRegistryScopes constant to avoid divergence.