Skip to content

Conversation

@rdimitrov
Copy link
Member

@rdimitrov rdimitrov commented Nov 5, 2025

The following PR:

  • Adds converter functions for going back and forth between the 2 formats (toolhive's current format and the upstream one). This is temporary as those should live under toolhive-registry and not toolhive.
  • Implements the registry provider interface by adding support for using upstream API compliant registries. This allows users to discover and run servers they already published upstream (or in another compliant registry)
  • Fixes a few issues discovered while implementing this, i.e. we were not reseting the default registry when the config was changed.

Notes:

To test it:

  1. Build it
task build
  1. Set the registry
./bin/thv config set-registry-api https://registry.example.com
  1. Confirm it's set
./bin/thv config get-registry
  1. Search for a server
./bin/thv search io.github.github/github-mcp-server
  1. Run a server
./bin/thv run io.github.github/github-mcp-server -e GITHUB_PERSONAL_ACCESS_TOKEN=test123

@rdimitrov rdimitrov force-pushed the poc branch 2 times, most recently from a80e60f to 42536b9 Compare November 6, 2025 08:04
@codecov
Copy link

codecov bot commented Nov 6, 2025

Codecov Report

❌ Patch coverage is 43.94231% with 583 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.17%. Comparing base (cd66940) to head (f37a2fa).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pkg/registry/provider_cached.go 0.00% 175 Missing ⚠️
pkg/registry/api/client.go 0.00% 146 Missing ⚠️
pkg/registry/provider_api.go 0.00% 99 Missing ⚠️
pkg/config/registry.go 2.85% 33 Missing and 1 partial ⚠️
pkg/registry/converters/upstream_to_toolhive.go 85.46% 23 Missing and 10 partials ⚠️
pkg/api/v1/registry.go 44.44% 23 Missing and 7 partials ⚠️
pkg/auth/remote/config.go 0.00% 14 Missing ⚠️
pkg/registry/converters/toolhive_to_upstream.go 94.52% 4 Missing and 4 partials ⚠️
pkg/registry/provider_base.go 38.46% 8 Missing ⚠️
pkg/runner/retriever/retriever.go 46.15% 7 Missing ⚠️
... and 11 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2469      +/-   ##
==========================================
- Coverage   55.80%   55.17%   -0.64%     
==========================================
  Files         299      305       +6     
  Lines       28341    29218     +877     
==========================================
+ Hits        15816    16120     +304     
- Misses      11097    11667     +570     
- Partials     1428     1431       +3     

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

Copy link
Collaborator

@dmartinol dmartinol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT, what else is missing to un-draft it?

@rdimitrov
Copy link
Member Author

rdimitrov commented Nov 6, 2025

LGMT, what else is missing to un-draft it?

I just added a change for setting the User Agent when we reach out a registry server but nothing else major I think, it was mostly the changes on the operator side (which we've agreed are out of scope).

The things I'd like to get covered before merging are:

  • More people testing this locally and starting servers from the upstream registry. I'll update the description above how to configure the upstream one.
  • Ensure this works well with Studio
  • Should we cache the results from querying the registry? In general yes, but I think we can do that as a follow up as the value is to confirm the conversion works now.

@rdimitrov rdimitrov force-pushed the poc branch 4 times, most recently from a34121c to 49b13a3 Compare November 7, 2025 14:15
@rdimitrov rdimitrov changed the title Draft: Add registry format converters and support for the Official MCP Registry Add registry format converters and support for the Official MCP Registry Nov 7, 2025
rdimitrov and others added 16 commits November 11, 2025 00:29
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
The API endpoints use v0.1 as the version path component.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
…converter

Support extracting environment variables from both sources in the MCP Registry API format:
1. The dedicated environmentVariables field (preferred way)
2. The -e/--env flags in runtimeArguments (Docker CLI pattern)

Many servers in the MCP registry (like github-mcp-server) use the Docker CLI pattern
of specifying environment variables as runtime arguments with -e flags, rather than
using the dedicated environmentVariables field. This is a valid approach according to
the MCP registry schema.

Changes:
- Add extractEnvironmentVariables() to handle both sources
- Add extractEnvFromRuntimeArgs() to parse -e/--env flags
- Add parseEnvVarFromValue() to extract variable metadata
- Parse variable references like {token} and extract metadata (isSecret, isRequired, etc.)
- Handle static values and complex values with multiple equals signs
- Add comprehensive test suite with 15+ test cases covering all scenarios
- Add integration test with realistic GitHub MCP server data

This ensures ToolHive can properly consume all servers from the official MCP registry,
regardless of which format they use for environment variables.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
@rdimitrov rdimitrov merged commit 8739ec4 into main Nov 11, 2025
31 checks passed
@rdimitrov rdimitrov deleted the poc branch November 11, 2025 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants