feat(registry): handle authentication for registry server api#1783
Conversation
|
@claude succinct review |
|
Claude finished @peppescg's task in 1m 3s —— View job Code Review
Overall this is well-structured — the hook extraction is clean and the error-handling logic is clearly separated. A few things worth addressing: Bugs / Correctness
UX / State
Schema
Minor
|
- Add client_id/issuer_url OAuth fields for Registry Server API type - Add login/logout flow when saving api_url with auth credentials - Show registry_auth_required error with field-level feedback: - Empty auth fields: red borders + general OIDC message - Populated fields: REGISTRY_WRONG_AUTH_TOAST on client_id - Preserve source URL after login failure (keepDirtyValues) - Clear form errors on registry type dropdown change - Refactor into useRegistryData + useRegistryForm hooks - Extract REGISTRY_FORM_TYPE constant for type-safe comparisons - Add OIDC_DISCOVERY_PATTERN and error constants to registry-list-error Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Test OIDC discovery failure shows issuer_url field error - Test dropdown type change clears previous form errors - Test source URL is preserved after login failure (keepDirtyValues) - Test issuer URL format validation - Test registry_auth_required with existing auth config shows client_id field error instead of general OIDC box message Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…onstants and extract OAuthField Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…igning in.../Saving... on load Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Display a dedicated error page and form-level error when the backend returns a 503 with code `registry_unavailable` (misconfigured API URL). Pre-fill the source field with the URL extracted from the error so the user can correct it directly. Refactor submit error handling into a switch-based helper function for readability. Made-with: Cursor
Remove re-exports of toast constants from use-registry-update-mutation (now imported directly from registry-list-error) and un-export REGISTRY_LIST_LOAD_FALLBACK_MESSAGE which is only used internally. Made-with: Cursor
- Deduplicate onValueChange logic in registry-type-field - Simplify errorMsg callback with set-based known toast lookup - Extract complex inline condition into named showSourceError variable - Remove unused registryUnavailableMessage from useRegistryForm return Made-with: Cursor
Pass auth from mutationFn into buildRegistryBody instead of calling buildRegistryAuth in both places independently. Made-with: Cursor
…ed types auth_config is already typed as GithubComStacklokToolhivePkgRegistryOAuthPublicConfig by the generated OpenAPI types — the Record<string, string> cast bypassed compile-time safety for field renames. Made-with: Cursor
When registry_unavailable fires, React Query may still hold the old cached source. Prioritize the URL extracted from the 503 error so the user sees the actual broken URL they need to fix. Made-with: Cursor
…ages - Show OIDC box message without field-level blame when auth config exists but registry_auth_required fires (the GET error does not mean credentials are wrong — login may not have been attempted yet) - Use field-specific messages for missing auth fields: "Client ID is required" / "Issuer URL is required" instead of generic "Both..." - Clear stale field errors at the start of the error effect to prevent unavailable message leaking into auth_required state - Simplify hasRegistryError to hasError && !isUnavailableError - Update auth_required UI message to clarify the registry requires authentication Made-with: Cursor
…and field errors Covers registry_unavailable source error + URL pre-fill, auth_required with existing OIDC config showing only the box message, and per-field validation messages on submit with missing auth fields. Made-with: Cursor
…rection Merge getErrorCode/getErrorMessage into a single getErrorField accessor. Replace getRegistryUnavailableMessage getter with an exported constant used directly in the form hook, removing a data-hook return value and a redundant effect dependency. Made-with: Cursor
…istry button Detect when thv exits due to a registry authentication error and redirect the user to Settings > Registry with a persistent error toast. - Add getToolhiveStatus IPC exposing exitReason from thv stderr - Extract shared ToolhiveStatus types to common/types - Redirect to /settings?tab=registry on registry-auth-required - Show persistent error toast in registry form when auth error detected - Add "Reset Registry" button to restore default registry config - Use mutation variables to derive isResetting state without extra useState - Guard setupSecretProvider in loader when thv is not running Made-with: Cursor
More descriptive name that clearly indicates a process-level error rather than a normal exit reason. Rename exitReason field to processError across common types, main, preload, and renderer. Made-with: Cursor
…efaults - Rename registry-list-error.ts to registry-errors.ts for clarity - Move REGISTRY_UNAVAILABLE_UI_MESSAGE to registry-errors.ts (was duplicated in registry-error.tsx and its test) - Extract inline form defaults to EMPTY_REGISTRY_DEFAULTS constant - Extract onValueChange logic to restoreFieldsForType function Made-with: Cursor
…et button - Merge two useEffects into one that reacts to both error state and type changes, clearing and recomputing form errors in a single pass - Flatten nested if in auth error handling with hasMissingCredentials - Simplify reset button: secondary variant, rounded, no icon Made-with: Cursor
- Rename registry-errors.ts to registry-errors-message.ts to avoid confusion with the registry-error.tsx component - Remove useEffect that imperatively synced query errors into form state via form.setError/clearErrors - Handle unavailable error declaratively via isUnavailableError prop passed to RegistrySourceField - Clear form errors on type change directly in RegistryTypeField onValueChange handler - Move auth-required toast to __root.tsx redirect block (event-driven) and dismiss it in onReset/onSubmit handlers - Invalidate registry query on successful api_url login so error state clears and form updates correctly after authentication Made-with: Cursor
There was a problem hiding this comment.
Pull request overview
Adds OAuth configuration and improved error handling for Registry Server API usage in the Registry Settings UI, including startup redirection when ToolHive indicates registry authentication is required.
Changes:
- Add Registry Server API OAuth fields (Client ID, Issuer URL), submit as
auth, and trigger login/logout flows. - Handle structured backend errors (
registry_auth_required,registry_unavailable) with targeted UI messaging (settings + full-page error state). - Refactor registry settings logic into
useRegistryData/useRegistryForm, plus expanded MSW fixtures and test coverage.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| renderer/src/routes/__root.tsx | Redirects to registry settings + persistent toast when ToolHive reports registry auth required at startup. |
| renderer/src/routes/(registry)/registry.tsx | Passes route error into RegistryError for structured rendering. |
| renderer/src/common/mocks/fixtures/registry_auth_logout/post.ts | Adds MSW fixture for registry auth logout endpoint. |
| renderer/src/common/mocks/fixtures/registry_auth_login/post.ts | Adds MSW fixture for registry auth login endpoint (+ error scenario). |
| renderer/src/common/mocks/fixtures/registry/get.ts | Adds MSW fixture for GET registry list endpoint (+ error scenario). |
| renderer/src/common/hooks/use-toast-mutation.ts | Extends toast mutation hook to support functional loading/error messages. |
| renderer/src/common/components/settings/tabs/tests/registry-tab.test.tsx | Adds extensive tests for OAuth fields, auth/unavailable errors, and submission error mappings. |
| renderer/src/common/components/settings/registry/utils.ts | Adds REGISTRY_FORM_TYPE constants and helper to extract auth config from registry info. |
| renderer/src/common/components/settings/registry/use-registry-update-mutation.ts | Centralizes PUT + auth login/logout flow, cache updates, and toast/error mapping. |
| renderer/src/common/components/settings/registry/use-registry-form.ts | Centralizes form state, submit/reset handlers, and field-error application. |
| renderer/src/common/components/settings/registry/use-registry-data.ts | Centralizes registry query + update mutation state and structured error detection. |
| renderer/src/common/components/settings/registry/schema.ts | Adds client/issuer fields + issuer URL validation; improves required-source validation copy for API URL. |
| renderer/src/common/components/settings/registry/registry-type-field.tsx | Clears errors and restores/clears dependent fields when switching registry type. |
| renderer/src/common/components/settings/registry/registry-tab.tsx | Refactors tab to use extracted hooks and pass richer props to RegistryForm. |
| renderer/src/common/components/settings/registry/registry-source-field.tsx | Improves per-error messaging (auth-required vs unavailable) and when to mark the source field errored. |
| renderer/src/common/components/settings/registry/registry-form.tsx | Adds OAuth fields section + reset button + improved submit labels/spinner behavior. |
| renderer/src/common/components/settings/registry/registry-errors-message.ts | Adds shared constants and helpers for structured registry errors and user-facing copy. |
| renderer/src/common/components/settings/registry/registry-api-oauth-fields.tsx | New OAuth fields component rendered only for Registry Server API type. |
| renderer/src/common/components/error/registry-error.tsx | Renders dedicated full-page states for auth-required/unavailable registry errors. |
| renderer/src/common/components/error/tests/registry-error.test.tsx | Adds tests for the new full-page registry error variants. |
| preload/src/api/toolhive.ts | Adds getToolhiveStatus() to the renderer-facing Electron API. |
| main/src/toolhive-manager.ts | Tracks a processError for registry-auth-required based on ToolHive stderr output. |
| main/src/ipc-handlers/toolhive.ts | Exposes get-toolhive-status IPC handler. |
| common/types/toolhive-status.ts | Adds shared ToolhiveStatus / ToolhiveProcessError types. |
…d borders - Only force form to API_URL type when the failing mutation was specifically for api_url, not for unrelated registry types - Add aria-invalid on empty OAuth fields when registry_auth_required fires, restoring the red border visual feedback declaratively - Fix JSDoc typo: "API Server API" → "Registry Server API" Made-with: Cursor
When registry_auth_required fires and Client ID / Issuer URL are empty,
display "{label} is required" below each field instead of a silent
FormMessage. Fields with existing values only show the general OAuth
box message without per-field blame.
Made-with: Cursor
samuv
left a comment
There was a problem hiding this comment.
I didn’t test it, but the code looks good. I just added a note about __root, but it’s independent of the main focus of this PR
…edirect on first detection Add 'registry-unavailable' process error for "registry login failed" and "upstream registry unavailable" stderr patterns. The beforeLoad guard now redirects to Settings > Registry for both registry-auth-required and registry-unavailable, showing a specific toast per error type. Also extracts __root.tsx beforeLoad/loader into individual guard files under routes/root/guards/ and errorComponent into routes/root/root-error.tsx for improved testability and maintainability. Made-with: Cursor
Remove registry-unavailable from ToolhiveProcessError, stderr matching, guard logic, and toast — this requires a backend-side fix instead. Fix CI failures in setup-secret-provider: add missing async to catch handler and cast mock queryKey to the generated type. Made-with: Cursor
The direct cast from [string] to the generated queryKey type is rejected by stricter CI TypeScript — types don't overlap enough. Cast through unknown first as recommended by the compiler. Made-with: Cursor
samuv
left a comment
There was a problem hiding this comment.
Nice, thank you for the refactoring
… to settings Add API-level detection in handleRegistryAuthRedirect: GET /registry now catches both registry_auth_required and registry_unavailable, redirecting to Settings > Registry with an appropriate toast message. Centralize the registry-auth-required constant and extract shared helpers for error classification and toast message resolution. Made-with: Cursor
Move the toast.error() call from the beforeLoad guard (where it fired before the Toaster component was mounted, causing it to be lost during the redirect cycle) into a useEffect hook in the root component. The guard now saves the pending toast message in the query client, and useRegistryErrorToast picks it up after mount. Made-with: Cursor
Made-with: Cursor
Summary
Kapture.2026-03-26.at.19.22.38.mp4
Adds OAuth configuration support for Registry Server API and handles structured backend errors (
registry_auth_required,registry_unavailable) with actionable UI feedback.OAuth fields for Registry Server API
authobject inPUT /api/v1beta/registry/defaultError handling —
registry_auth_required(HTTP 401)code: "registry_auth_required"from GET/api/v1beta/registryError handling —
registry_unavailable(HTTP 503)code: "registry_unavailable"from GET/api/v1beta/registrySubmit error handling
keepDirtyValues)Refactoring
useRegistryDataanduseRegistryFormhooks from registry tabapplySubmitFieldErrorswith switch-based error mappingREGISTRY_FORM_TYPEconstantserrorMsgtoast callback with set-based known message lookupOAuthFieldcomponent andOAUTH_FIELDSconfigonValueChangelogic in registry type dropdownTest plan
PUTincludesauthobjectPUTomitsauth, no login attemptregistry_auth_requiredwith no config → red borders + OIDC messageregistry_auth_requiredwith existing config → error on Client IDregistry_unavailable→ error on source field with pre-filled URLCloses #1760