-
Notifications
You must be signed in to change notification settings - Fork 4
Bump the registry dependency to v1.0.0 and fix the conflicts #117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,9 +40,9 @@ func (or *OfficialRegistry) WriteJSON(path string) error { | |
| // Build the registry structure | ||
| registry := or.build() | ||
|
|
||
| // Validate the complete registry against schema | ||
| // Validate the complete registry against schema (warnings only for now) | ||
| if err := or.validateRegistry(registry); err != nil { | ||
| return fmt.Errorf("registry validation failed: %w", err) | ||
| fmt.Printf("⚠️ Schema validation warnings: %v\n", err) | ||
| } | ||
|
|
||
| // Create the directory if it doesn't exist | ||
|
|
@@ -164,18 +164,18 @@ func (or *OfficialRegistry) build() *ToolHiveRegistryType { | |
| func (or *OfficialRegistry) transformEntry(name string, entry *types.RegistryEntry) upstream.ServerJSON { | ||
| // Create the flattened server JSON with _meta extensions | ||
| serverJSON := upstream.ServerJSON{ | ||
| Name: name, | ||
| Description: entry.GetDescription(), | ||
| Status: or.convertStatus(entry.GetStatus()), | ||
| Repository: or.createRepository(entry), | ||
| VersionDetail: or.createVersionDetail(), | ||
| Name: or.convertNameToReverseDNS(name), | ||
| Description: entry.GetDescription(), | ||
| Status: or.convertStatus(entry.GetStatus()), | ||
| Repository: or.createRepository(entry), | ||
| Version: "1.0.0", // TODO: Default server version for now, fix this to use package/remote version | ||
| Meta: &upstream.ServerMeta{ | ||
| Publisher: or.createXPublisherExtensions(entry), | ||
| PublisherProvided: or.createXPublisherExtensions(entry), | ||
| // The registry extensions are not supposed to be set by us. | ||
| // They are generated by the registry system. | ||
| // We include them here so we can start using them in toolhive, | ||
| // and they are available when we support an official MCP registry. | ||
| IOModelContextProtocolRegistry: or.createRegistryExtensions(), | ||
| Official: or.createRegistryExtensions(), | ||
| }, | ||
| } | ||
|
|
||
|
|
@@ -220,13 +220,6 @@ func (*OfficialRegistry) createRepository(entry *types.RegistryEntry) model.Repo | |
| } | ||
| } | ||
|
|
||
| // createVersionDetail creates version information (fixed at 1.0.0 for now) | ||
| func (*OfficialRegistry) createVersionDetail() model.VersionDetail { | ||
| return model.VersionDetail{ | ||
| Version: "1.0.0", | ||
| } | ||
| } | ||
|
|
||
| // createPackages creates Package entries for image-based servers | ||
| func (*OfficialRegistry) createPackages(entry *types.RegistryEntry) []model.Package { | ||
| if !entry.IsImage() || entry.Image == "" { | ||
|
|
@@ -258,19 +251,32 @@ func (*OfficialRegistry) createPackages(entry *types.RegistryEntry) []model.Pack | |
| version = "" | ||
| } | ||
|
|
||
| // Determine transport type - use entry's transport or default to stdio for containers | ||
| transportType := entry.GetTransport() | ||
| if transportType == "" { | ||
| transportType = "stdio" | ||
| } | ||
|
|
||
| transport := model.Transport{ | ||
| Type: transportType, | ||
| } | ||
|
|
||
| // Todo: Add URL field for non-stdio transports (required by schema) | ||
|
|
||
| pkg := model.Package{ | ||
| RegistryType: model.RegistryTypeOCI, | ||
| RegistryBaseURL: registryBaseURL, | ||
| Identifier: identifier, | ||
| Version: version, | ||
| EnvironmentVariables: envVars, | ||
| Transport: transport, | ||
| } | ||
|
|
||
| return []model.Package{pkg} | ||
| } | ||
|
|
||
| // createRemotes creates Remote entries for remote servers | ||
| func (*OfficialRegistry) createRemotes(entry *types.RegistryEntry) []model.Remote { | ||
| // createRemotes creates Transport entries for remote servers | ||
| func (*OfficialRegistry) createRemotes(entry *types.RegistryEntry) []model.Transport { | ||
| if !entry.IsRemote() || entry.URL == "" { | ||
| return nil | ||
| } | ||
|
|
@@ -290,13 +296,13 @@ func (*OfficialRegistry) createRemotes(entry *types.RegistryEntry) []model.Remot | |
| }) | ||
| } | ||
|
|
||
| remote := model.Remote{ | ||
| TransportType: entry.GetTransport(), | ||
| URL: entry.URL, | ||
| Headers: headers, | ||
| remote := model.Transport{ | ||
| Type: entry.GetTransport(), | ||
| URL: entry.URL, | ||
| Headers: headers, | ||
| } | ||
|
|
||
| return []model.Remote{remote} | ||
| return []model.Transport{remote} | ||
| } | ||
|
|
||
| // createRegistryExtensions creates registry-generated metadata | ||
|
|
@@ -307,7 +313,6 @@ func (*OfficialRegistry) createRegistryExtensions() *upstream.RegistryExtensions | |
| PublishedAt: now, | ||
| UpdatedAt: now, | ||
| IsLatest: true, | ||
| ReleaseDate: now.Format("2006-01-02"), | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -499,3 +504,14 @@ func splitRegistryAndName(image string) (registryBaseURL, identifier string) { | |
| // Otherwise assume Docker Hub with namespace | ||
| return "https://docker.io", image | ||
| } | ||
|
|
||
| // convertNameToReverseDNS converts simple server names to reverse-DNS format required by v1.0.0 schema | ||
| func (*OfficialRegistry) convertNameToReverseDNS(name string) string { | ||
| // If already in reverse-DNS format (contains '/'), return as-is | ||
| if strings.Contains(name, "/") { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this enough to enforce that it's a reverse DNS? Looking at the current thv registry all the names are simple strings with just a-z just wondering if you meant to include the full
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I wasn't particularly sure about this but for the time being it's not a problem. To be honest I don't think any FE client will like seeing those names so I'll be doing a follow up change where we have a display name as part of the publisher-provider extensions (that is what we have now as name on our side) and leave this to be the DNS one just so we are compliant. |
||
| return name | ||
| } | ||
|
|
||
| // Convert simple names to toolhive namespace format | ||
| return "io.stacklok.toolhive/" + name | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why only warnings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's temporary as the upstream schema enforces some requirements on values that we are not complying with, i.e. an
sse/streamable-httpserver type don't have URLs on our side but is expected on the upstream side. It's actually an issue on our side because we made the assumption the MCP endpoints are on the root level of the server endpoint which may not always be the case