Skip to content

Commit da68035

Browse files
committed
Address Claude's feedback
Signed-off-by: Radoslav Dimitrov <[email protected]>
1 parent ef2439b commit da68035

File tree

5 files changed

+63
-20
lines changed

5 files changed

+63
-20
lines changed

pkg/registry/converters/converters_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ func TestServerJSONToImageMetadata_NoPackages(t *testing.T) {
137137

138138
assert.Error(t, err)
139139
assert.Nil(t, imageMetadata)
140-
assert.Contains(t, err.Error(), "serverJSON has no packages")
140+
assert.Contains(t, err.Error(), "has no packages")
141141
}
142142

143143
func TestServerJSONToImageMetadata_NoOCIPackages(t *testing.T) {
@@ -157,7 +157,7 @@ func TestServerJSONToImageMetadata_NoOCIPackages(t *testing.T) {
157157

158158
assert.Error(t, err)
159159
assert.Nil(t, imageMetadata)
160-
assert.Contains(t, err.Error(), "serverJSON has no OCI packages")
160+
assert.Contains(t, err.Error(), "has no OCI packages")
161161
}
162162

163163
func TestServerJSONToImageMetadata_MultipleOCIPackages(t *testing.T) {
@@ -517,7 +517,7 @@ func TestServerJSONToRemoteServerMetadata_NoRemotes(t *testing.T) {
517517

518518
assert.Error(t, err)
519519
assert.Nil(t, remoteMetadata)
520-
assert.Contains(t, err.Error(), "serverJSON has no remotes")
520+
assert.Contains(t, err.Error(), "has no remotes")
521521
}
522522

523523
func TestServerJSONToRemoteServerMetadata_WithHeaders(t *testing.T) {

pkg/registry/converters/toolhive_to_upstream.go

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
1-
// Package converters provides conversion functions from toolhive ImageMetadata/RemoteServerMetadata formats
2-
// to upstream MCP ServerJSON format.
1+
// Package converters provides bidirectional conversion between toolhive registry formats
2+
// and the upstream MCP (Model Context Protocol) ServerJSON format.
3+
//
4+
// The package supports two conversion directions:
5+
// - toolhive → upstream: ImageMetadata/RemoteServerMetadata → ServerJSON (this file)
6+
// - upstream → toolhive: ServerJSON → ImageMetadata/RemoteServerMetadata (upstream_to_toolhive.go)
7+
//
8+
// Toolhive-specific fields (permissions, provenance, metadata) are stored in the upstream
9+
// format's publisher extensions under "io.github.stacklok", allowing additional metadata
10+
// while maintaining compatibility with the standard MCP registry format.
311
package converters
412

513
import (
@@ -35,6 +43,17 @@ func ImageMetadataToServerJSON(name string, imageMetadata *registry.ImageMetadat
3543
URL: imageMetadata.RepositoryURL,
3644
Source: "github", // Assume GitHub
3745
}
46+
} else {
47+
// Use toolhive-registry as fallback when no repository URL is available.
48+
// This is necessary for schema validation - the upstream Repository field is a struct
49+
// (not a pointer), so it can't be omitted with omitempty and would serialize as
50+
// empty strings {"url": "", "source": ""}, which fails URI format validation.
51+
// Using the toolhive-registry URL is reasonable since it's where these servers
52+
// are registered and documented.
53+
serverJSON.Repository = model.Repository{
54+
URL: "https://github.com/stacklok/toolhive-registry",
55+
Source: "github",
56+
}
3857
}
3958

4059
// Create package
@@ -73,6 +92,17 @@ func RemoteServerMetadataToServerJSON(name string, remoteMetadata *registry.Remo
7392
URL: remoteMetadata.RepositoryURL,
7493
Source: "github", // Assume GitHub
7594
}
95+
} else {
96+
// Use toolhive-registry as fallback when no repository URL is available.
97+
// This is necessary for schema validation - the upstream Repository field is a struct
98+
// (not a pointer), so it can't be omitted with omitempty and would serialize as
99+
// empty strings {"url": "", "source": ""}, which fails URI format validation.
100+
// Using the toolhive-registry URL is reasonable since it's where these servers
101+
// are registered and documented.
102+
serverJSON.Repository = model.Repository{
103+
URL: "https://github.com/stacklok/toolhive-registry",
104+
Source: "github",
105+
}
76106
}
77107

78108
// Create remote
@@ -115,12 +145,15 @@ func createPackagesFromImageMetadata(imageMetadata *registry.ImageMetadata) []mo
115145
}
116146

117147
// Add URL for non-stdio transports
148+
// Note: We use localhost as the host because container-based MCP servers run locally
149+
// and are accessed via port forwarding. The actual container may listen on 0.0.0.0,
150+
// but clients connect via localhost on the host machine.
118151
if transportType == model.TransportTypeStreamableHTTP || transportType == model.TransportTypeSSE {
119152
if imageMetadata.TargetPort > 0 {
120153
// Include port in URL if explicitly set
121154
transport.URL = fmt.Sprintf("http://localhost:%d", imageMetadata.TargetPort)
122155
} else {
123-
// No port specified - use URL without port
156+
// No port specified - use URL without port (standard HTTP port 80)
124157
transport.URL = "http://localhost"
125158
}
126159
}

pkg/registry/converters/upstream_to_toolhive.go

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,23 +22,25 @@ func ServerJSONToImageMetadata(serverJSON *upstream.ServerJSON) (*registry.Image
2222
}
2323

2424
if len(serverJSON.Packages) == 0 {
25-
return nil, fmt.Errorf("serverJSON has no packages (not a container-based server)")
25+
return nil, fmt.Errorf("server '%s' has no packages (not a container-based server)", serverJSON.Name)
2626
}
2727

2828
// Filter for OCI packages only
2929
var ociPackages []model.Package
30+
var packageTypes []string
3031
for _, pkg := range serverJSON.Packages {
3132
if pkg.RegistryType == model.RegistryTypeOCI {
3233
ociPackages = append(ociPackages, pkg)
3334
}
35+
packageTypes = append(packageTypes, string(pkg.RegistryType))
3436
}
3537

3638
if len(ociPackages) == 0 {
37-
return nil, fmt.Errorf("serverJSON has no OCI packages")
39+
return nil, fmt.Errorf("server '%s' has no OCI packages (found: %v)", serverJSON.Name, packageTypes)
3840
}
3941

4042
if len(ociPackages) > 1 {
41-
return nil, fmt.Errorf("serverJSON has %d OCI packages, expected exactly 1", len(ociPackages))
43+
return nil, fmt.Errorf("server '%s' has %d OCI packages, expected exactly 1", serverJSON.Name, len(ociPackages))
4244
}
4345

4446
pkg := ociPackages[0]
@@ -75,9 +77,16 @@ func ServerJSONToImageMetadata(serverJSON *upstream.ServerJSON) (*registry.Image
7577
if pkg.Transport.URL != "" {
7678
// Parse URL like "http://localhost:8080"
7779
parsedURL, err := url.Parse(pkg.Transport.URL)
78-
if err == nil && parsedURL.Port() != "" {
80+
if err != nil {
81+
// Log parse error to aid debugging but don't fail conversion
82+
fmt.Printf("⚠️ Failed to parse transport URL '%s' for server '%s': %v\n",
83+
pkg.Transport.URL, serverJSON.Name, err)
84+
} else if parsedURL.Port() != "" {
7985
if port, err := strconv.Atoi(parsedURL.Port()); err == nil {
8086
imageMetadata.TargetPort = port
87+
} else {
88+
fmt.Printf("⚠️ Failed to parse port from URL '%s' for server '%s': %v\n",
89+
pkg.Transport.URL, serverJSON.Name, err)
8190
}
8291
}
8392
}
@@ -101,7 +110,7 @@ func ServerJSONToRemoteServerMetadata(serverJSON *upstream.ServerJSON) (*registr
101110
}
102111

103112
if len(serverJSON.Remotes) == 0 {
104-
return nil, fmt.Errorf("serverJSON has no remotes (not a remote server)")
113+
return nil, fmt.Errorf("server '%s' has no remotes (not a remote server)", serverJSON.Name)
105114
}
106115

107116
remote := serverJSON.Remotes[0] // Use first remote

pkg/registry/official.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -232,16 +232,17 @@ func (*OfficialRegistry) createRepository(entry *types.RegistryEntry) model.Repo
232232
repositoryURL = entry.RemoteServerMetadata.RepositoryURL
233233
}
234234

235+
// If no repository URL is available, use toolhive-registry as fallback.
236+
// This is necessary for schema validation - the upstream Repository field is a struct
237+
// (not a pointer), so it can't be omitted with omitempty and would serialize as
238+
// empty strings {"url": "", "source": ""}, which fails URI format validation.
239+
// Using the toolhive-registry URL is reasonable since it's where these servers
240+
// are registered and documented.
235241
if repositoryURL == "" {
236-
// Use a toolhive-registry placeholder URL to satisfy validation when no repository is available for remote servers
237-
repositoryURL = "https://github.com/stacklok/toolhive-registry"
238-
if entry.IsRemote() {
239-
return model.Repository{
240-
URL: repositoryURL,
241-
Source: "github",
242-
}
242+
return model.Repository{
243+
URL: "https://github.com/stacklok/toolhive-registry",
244+
Source: "github",
243245
}
244-
return model.Repository{}
245246
}
246247

247248
return model.Repository{

schemas/registry.schema.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@
6161
}
6262
},
6363
"_schema_version": {
64-
"mcp_registry_version": "v1.3.3",
64+
"mcp_registry_version": "v1.3.5",
6565
"schema_date": "2025-10-17",
6666
"updated_at": "2025-10-22T12:35:00Z",
6767
"updated_by": "manual update to use static.modelcontextprotocol.io schema URL"

0 commit comments

Comments
 (0)