fix(registry): surface legacy registry format as a structured API error (#5260)

* Surface legacy registry format as a structured API error

Before this change, when a configured custom registry URL served data in
the legacy ToolHive format, NewRemoteRegistryProvider returned a plain
fmt.Errorf that the API handler did not recognise. The API responded with
a generic HTTP 500 "Failed to get registry provider", and the actionable
migration hint was visible only in main.log. Desktop clients had no way
to distinguish this case from an internal error.

Add a typed *LegacyFormatError (sister of *UnavailableError) carrying the
offending source URL. Wire it through the four registry GET handlers and
the v0.1 router so the response is now HTTP 503 with a structured
"registry_legacy_format" code and the migration hint in the message body.

The upstream_parser sentinel errLegacyFormat is now an instance of
*LegacyFormatError; its Is() method preserves errors.Is(err, errLegacyFormat)
for existing callers while enabling errors.As extraction of the URL.

Fixes #5259

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Switch registry_legacy_format status code to 502 Bad Gateway

503 was inconsistent with the existing PUT registry handler that already
returns 502 for the same legacy-format detection via ErrRegistryValidationFailed
(pkg/api/v1/registry.go updateRegistry handler). 502 is also the correct
RFC 9110 §15.6.3 code: thv serve acts as a gateway to the upstream registry
and the upstream returned a response we cannot process.

503 is reserved for "temporary overload or scheduled maintenance" which does
not match a misconfigured registry source.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Fix stale 503 references in doc comments

Two doc comments missed the 502 update from the previous commit. The code
itself already returns 502 Bad Gateway via writeRegistryLegacyFormatError;
only the comments were inconsistent.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: peppescg

pkg/api/v1/registry.go

@@ -56,6 +56,14 @@ func writeRegistryAuthRequiredError(w http.ResponseWriter) {
 // structured JSON 503 response when the upstream registry is unreachable.
 const RegistryUnavailableCode = "registry_unavailable"
 
+// RegistryLegacyFormatCode is the machine-readable error code returned in the
+// structured JSON 502 response when the configured registry source serves data
+// in the legacy ToolHive format instead of the upstream MCP registry format.
+// Desktop clients (Studio) match on this value to display a targeted recovery
+// flow (reset to default registry, point to a `?format=upstream` endpoint, or
+// run `thv registry convert`).
+const RegistryLegacyFormatCode = "registry_legacy_format"
+
 // writeRegistryUnavailableError writes a structured JSON 503 response when the
 // upstream registry cannot be reached or returns an unexpected error (e.g. 404).
 func writeRegistryUnavailableError(w http.ResponseWriter, unavailableErr *regpkg.UnavailableError) {
@@ -68,6 +76,24 @@ func writeRegistryUnavailableError(w http.ResponseWriter, unavailableErr *regpkg
 	_ = json.NewEncoder(w).Encode(body)
 }
 
+// writeRegistryLegacyFormatError writes a structured JSON 502 Bad Gateway
+// response when the configured registry source returns data in the legacy
+// ToolHive format. HTTP 502 is correct per RFC 9110 §15.6.3: thv serve is
+// acting as a gateway to the upstream registry, and the upstream returned a
+// response we cannot process. It also aligns with the existing PUT handler
+// which already returns 502 for the same legacy-format detection. The message
+// embeds legacyhint.MigrationMessage so CLI users still see the actionable
+// migration step, while desktop clients can branch on Code.
+func writeRegistryLegacyFormatError(w http.ResponseWriter, legacyErr *regpkg.LegacyFormatError) {
+	body := registryErrorResponse{
+		Code:    RegistryLegacyFormatCode,
+		Message: legacyErr.Error(),
+	}
+	w.Header().Set("Content-Type", "application/json")
+	w.WriteHeader(http.StatusBadGateway)
+	_ = json.NewEncoder(w).Encode(body)
+}
+
 // resolveAuthStatus returns the auth_status and auth_type strings for API responses
 // by delegating to the AuthManager.
 func (rr *RegistryRoutes) resolveAuthStatus() (authStatus, authType string) {
@@ -267,6 +293,12 @@ func (rr *RegistryRoutes) getCurrentProvider(w http.ResponseWriter) (regpkg.Prov
 			writeRegistryAuthRequiredError(w)
 			return nil, false
 		}
+		var legacyErr *regpkg.LegacyFormatError
+		if errors.As(err, &legacyErr) {
+			slog.Error("upstream registry in legacy format", "error", err)
+			writeRegistryLegacyFormatError(w, legacyErr)
+			return nil, false
+		}
 		var unavailableErr *regpkg.UnavailableError
 		if errors.As(err, &unavailableErr) {
 			slog.Error("upstream registry unavailable", "error", err)
@@ -373,6 +405,12 @@ func (rr *RegistryRoutes) listRegistries(w http.ResponseWriter, _ *http.Request)
 			writeRegistryAuthRequiredError(w)
 			return
 		}
+		var legacyErr *regpkg.LegacyFormatError
+		if errors.As(err, &legacyErr) {
+			slog.Error("upstream registry in legacy format", "error", err)
+			writeRegistryLegacyFormatError(w, legacyErr)
+			return
+		}
 		var unavailableErr *regpkg.UnavailableError
 		if errors.As(err, &unavailableErr) {
 			slog.Error("upstream registry unavailable", "error", err)
@@ -454,6 +492,12 @@ func (rr *RegistryRoutes) getRegistry(w http.ResponseWriter, r *http.Request) {
 			writeRegistryAuthRequiredError(w)
 			return
 		}
+		var legacyErr *regpkg.LegacyFormatError
+		if errors.As(err, &legacyErr) {
+			slog.Error("upstream registry in legacy format", "error", err)
+			writeRegistryLegacyFormatError(w, legacyErr)
+			return
+		}
 		var unavailableErr *regpkg.UnavailableError
 		if errors.As(err, &unavailableErr) {
 			slog.Error("upstream registry unavailable", "error", err)
@@ -741,6 +785,12 @@ func (rr *RegistryRoutes) listServers(w http.ResponseWriter, r *http.Request) {
 			writeRegistryAuthRequiredError(w)
 			return
 		}
+		var legacyErr *regpkg.LegacyFormatError
+		if errors.As(err, &legacyErr) {
+			slog.Error("upstream registry in legacy format", "error", err)
+			writeRegistryLegacyFormatError(w, legacyErr)
+			return
+		}
 		var unavailableErr *regpkg.UnavailableError
 		if errors.As(err, &unavailableErr) {
 			slog.Error("upstream registry unavailable", "error", err)

pkg/api/v1/registry_test.go

@@ -140,6 +140,113 @@ func TestRegistryAPI_GetEndpoint_UnavailableUpstream(t *testing.T) {
 	}
 }
 
+// TestRegistryAPI_GetEndpoint_LegacyFormat tests that GET endpoints return 502
+// with a structured "registry_legacy_format" code when the configured custom
+// registry URL serves data in the legacy ToolHive format. 502 is correct per
+// RFC 9110 §15.6.3: thv serve acts as a gateway to the upstream registry and
+// the upstream returned a response we cannot process.
+//
+//nolint:paralleltest // Uses global registry provider singleton
+func TestRegistryAPI_GetEndpoint_LegacyFormat(t *testing.T) {
+	// Mock server that returns a valid HTTP 200 but with legacy ToolHive
+	// registry JSON (top-level "servers" instead of nested "data.servers").
+	// validateConnectivity() must detect this and reject the provider with a
+	// *LegacyFormatError.
+	legacyServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write([]byte(`{
+			"version": "1.0.0",
+			"servers": {"example": {"image": "example:latest"}}
+		}`))
+	}))
+	defer legacyServer.Close()
+
+	// Configure registry to point at the mock legacy-format server via the
+	// remote URL provider path (RegistryUrl, not RegistryApiUrl).
+	cfg := &config.Config{
+		RegistryUrl:            legacyServer.URL,
+		AllowPrivateRegistryIp: true,
+	}
+	configProvider, cleanup := CreateTestConfigProvider(t, cfg)
+	defer cleanup()
+
+	registry.ResetDefaultProvider()
+	t.Cleanup(registry.ResetDefaultProvider)
+
+	routes := &RegistryRoutes{
+		configProvider: configProvider,
+		configService:  registry.NewConfiguratorWithProvider(configProvider),
+		serveMode:      true,
+	}
+
+	endpoints := []struct {
+		name      string
+		method    string
+		path      string
+		handler   http.HandlerFunc
+		urlParams map[string]string
+	}{
+		{
+			name:    "listRegistries",
+			method:  http.MethodGet,
+			path:    "/",
+			handler: routes.listRegistries,
+		},
+		{
+			name:      "getRegistry",
+			method:    http.MethodGet,
+			path:      "/default",
+			handler:   routes.getRegistry,
+			urlParams: map[string]string{"name": "default"},
+		},
+		{
+			name:      "listServers",
+			method:    http.MethodGet,
+			path:      "/default/servers",
+			handler:   routes.listServers,
+			urlParams: map[string]string{"name": "default"},
+		},
+	}
+
+	for _, ep := range endpoints {
+		t.Run(ep.name, func(t *testing.T) {
+			registry.ResetDefaultProvider()
+
+			req := httptest.NewRequest(ep.method, ep.path, nil)
+			if ep.urlParams != nil {
+				rctx := chi.NewRouteContext()
+				for k, v := range ep.urlParams {
+					rctx.URLParams.Add(k, v)
+				}
+				req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx))
+			}
+
+			w := httptest.NewRecorder()
+			ep.handler(w, req)
+
+			assert.Equal(t, http.StatusBadGateway, w.Code,
+				"Expected 502 Bad Gateway when registry is in legacy format")
+
+			var body registryErrorResponse
+			err := json.NewDecoder(w.Body).Decode(&body)
+			require.NoError(t, err, "Response should be valid JSON")
+			assert.Equal(t, RegistryLegacyFormatCode, body.Code,
+				"Response code should be registry_legacy_format")
+			// The body must carry the actionable migration hint so CLI users
+			// and desktop clients both have something to act on.
+			assert.Contains(t, body.Message, "legacy ToolHive format",
+				"Response message should mention the legacy format")
+			assert.Contains(t, body.Message, "thv registry convert",
+				"Response message should include the migration command hint")
+			assert.Contains(t, body.Message, legacyServer.URL,
+				"Response message should identify the offending source URL")
+			assert.Contains(t, w.Header().Get("Content-Type"), "application/json",
+				"Response Content-Type should be application/json")
+		})
+	}
+}
+
 func TestRegistryRouter(t *testing.T) {
 	t.Parallel()
 

pkg/api/v1/registry_v01.go

@@ -52,6 +52,12 @@ func getRegistryProvider(w http.ResponseWriter) (regpkg.Provider, bool) {
 			writeRegistryAuthRequiredError(w)
 			return nil, false
 		}
+		var legacyErr *regpkg.LegacyFormatError
+		if errors.As(err, &legacyErr) {
+			slog.Error("upstream registry in legacy format", "error", err)
+			writeRegistryLegacyFormatError(w, legacyErr)
+			return nil, false
+		}
 		var unavailableErr *regpkg.UnavailableError
 		if errors.As(err, &unavailableErr) {
 			slog.Error("upstream registry unavailable", "error", err)

pkg/registry/errors.go

@@ -6,6 +6,8 @@ package registry
 import (
 	"errors"
 	"fmt"
+
+	"github.com/stacklok/toolhive/pkg/registry/legacyhint"
 )
 
 // ErrServerNotFound indicates a server was not found in the registry.
@@ -29,3 +31,31 @@ func (e *UnavailableError) Error() string {
 func (e *UnavailableError) Unwrap() error {
 	return e.Err
 }
+
+// LegacyFormatError indicates the registry source contains data in the legacy
+// ToolHive registry format instead of the upstream MCP registry format.
+// API handlers translate this into a structured HTTP 502 with a
+// "registry_legacy_format" code so desktop clients can surface a targeted
+// recovery flow (instead of a generic error screen).
+//
+// URL is optional and identifies the offending source (remote URL or local
+// file path) when known. The Error() message embeds legacyhint.MigrationMessage
+// so CLI consumers continue to see the same actionable hint.
+type LegacyFormatError struct {
+	URL string
+}
+
+func (e *LegacyFormatError) Error() string {
+	if e.URL != "" {
+		return fmt.Sprintf("registry at %s: %s", e.URL, legacyhint.MigrationMessage)
+	}
+	return legacyhint.MigrationMessage
+}
+
+// Is enables errors.Is matching against any *LegacyFormatError sentinel,
+// regardless of the URL field. Existing callers using a zero-value sentinel
+// (e.g. errLegacyFormat) keep matching when the returned error carries a URL.
+func (*LegacyFormatError) Is(target error) bool {
+	_, ok := target.(*LegacyFormatError)
+	return ok
+}

pkg/registry/errors_test.go

@@ -66,3 +66,82 @@ func TestUnavailableError_ErrorsAs(t *testing.T) {
 	assert.Equal(t, "https://example.com", target.URL)
 	assert.Equal(t, inner, target.Err)
 }
+
+// TestLegacyFormatError_Error covers the two surface forms (with and without
+// URL) so CLI users keep seeing the migration hint and remote callers also see
+// where the legacy content came from.
+func TestLegacyFormatError_Error(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name           string
+		err            *LegacyFormatError
+		expectedPrefix string
+	}{
+		{
+			name:           "with URL",
+			err:            &LegacyFormatError{URL: "https://example.com/registry.json"},
+			expectedPrefix: "registry at https://example.com/registry.json: ",
+		},
+		{
+			name:           "without URL",
+			err:            &LegacyFormatError{},
+			expectedPrefix: "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			msg := tt.err.Error()
+			if tt.expectedPrefix != "" {
+				assert.Truef(t,
+					len(msg) > len(tt.expectedPrefix) && msg[:len(tt.expectedPrefix)] == tt.expectedPrefix,
+					"expected message to start with %q, got %q", tt.expectedPrefix, msg)
+			}
+			// Migration hint must always be present so CLI users get the
+			// actionable step regardless of whether a URL was attached.
+			assert.Contains(t, msg, "legacy ToolHive format")
+			assert.Contains(t, msg, "thv registry convert")
+		})
+	}
+}
+
+// TestLegacyFormatError_ErrorsIs verifies the Is method matches any
+// *LegacyFormatError regardless of URL. This keeps errors.Is(err, errLegacyFormat)
+// working when the returned error carries a populated URL from a remote source.
+func TestLegacyFormatError_ErrorsIs(t *testing.T) {
+	t.Parallel()
+
+	withURL := &LegacyFormatError{URL: "https://example.com/registry.json"}
+	sentinel := &LegacyFormatError{}
+
+	assert.True(t, errors.Is(withURL, sentinel),
+		"errors.Is(withURL, sentinel) should match via Is method")
+	assert.True(t, errors.Is(sentinel, withURL),
+		"errors.Is(sentinel, withURL) should match via Is method")
+
+	// Wrapped via fmt.Errorf should still match through Unwrap chain.
+	wrapped := fmt.Errorf("custom registry at %s is not reachable: %w", withURL.URL, withURL)
+	assert.True(t, errors.Is(wrapped, sentinel),
+		"errors.Is should follow the Unwrap chain to find a *LegacyFormatError")
+
+	// A different error type must not match.
+	other := &UnavailableError{URL: "https://example.com", Err: fmt.Errorf("boom")}
+	assert.False(t, errors.Is(other, sentinel),
+		"errors.Is must not cross-match *UnavailableError as legacy")
+}
+
+// TestLegacyFormatError_ErrorsAs verifies the typed extraction used by the
+// API layer to surface a structured 502 response with the source URL.
+func TestLegacyFormatError_ErrorsAs(t *testing.T) {
+	t.Parallel()
+
+	original := &LegacyFormatError{URL: "https://example.com/registry.json"}
+	wrapped := fmt.Errorf("custom registry at %s is not reachable: %w", original.URL, original)
+
+	var target *LegacyFormatError
+	require.True(t, errors.As(wrapped, &target),
+		"errors.As must extract *LegacyFormatError from the Unwrap chain")
+	assert.Equal(t, "https://example.com/registry.json", target.URL)
+}

pkg/registry/provider_remote.go

@@ -82,7 +82,7 @@ func (p *RemoteRegistryProvider) validateConnectivity() error {
 	}
 
 	if legacyhint.Looks(data) {
-		return fmt.Errorf("registry at %s: %s", p.registryURL, legacyhint.MigrationMessage)
+		return &LegacyFormatError{URL: p.registryURL}
 	}
 
 	var upstream types.UpstreamRegistry

pkg/registry/upstream_parser.go

@@ -5,7 +5,6 @@ package registry
 
 import (
 	"encoding/json"
-	"errors"
 	"fmt"
 
 	v0 "github.com/modelcontextprotocol/registry/pkg/api/v0"
@@ -18,9 +17,13 @@ import (
 // registry format. Without this check, Go's JSON decoder silently produces an
 // empty UpstreamRegistry (the legacy top-level "servers" field does not match
 // upstream's "data.servers" path), leaving the caller with an empty registry
-// and no actionable error. The error wording carries the migration step so
-// consumers can surface it without a typed match.
-var errLegacyFormat = errors.New(legacyhint.MigrationMessage)
+// and no actionable error.
+//
+// It's an instance of *LegacyFormatError so the API layer can detect it via
+// errors.As and emit a structured "registry_legacy_format" response. The Is
+// method on LegacyFormatError keeps errors.Is(err, errLegacyFormat) working
+// even when the returned error carries a populated URL.
+var errLegacyFormat = &LegacyFormatError{}
 
 // parseRegistryData parses raw JSON in the upstream MCP registry format and
 // converts it into the internal types.Registry plus any embedded skills.