Skip to content

Conversation

@lujunsan
Copy link
Contributor

Summary

  • Adds new thv group run <group-name> command that deploys all MCP servers from a registry group
  • Supports both container-based and remote servers with OAuth authentication flows
  • Maintains identical behavior and code paths as individual thv run commands when possible

New Command: thv group run

The new command allows deploying entire groups of MCP servers with a single command:

Basic usage - deploy all servers in a group

thv group run simple-test

With secrets and environment variables

  thv group run simple-test \
    --secret GITHUB_TOKEN,target=github.GITHUB_PERSONAL_ACCESS_TOKEN \
    --secret API_KEY,target=fetch.API_KEY \
    --env fetch.DEBUG=true \
    --env github.LOG_LEVEL=info

Key Features

  • Registry groups: Reads metadata from registry groups instead of top level registry entries if given a group name
  • Full server type support: Deploys both container servers and remote servers with OAuth authentication
  • Targeted secrets & environment variables: Use --secret VAR,target=server.TARGET_VAR and --env server.VAR=value syntax to specify per-server configuration
  • Comprehensive validation: Pre-flight checks ensure no naming conflicts and validate secret/env variable targets exist in the group

Architecture Changes

  • Enhanced registry reading: GetMCPServer now accepts group names and reads from group.Servers and group.RemoteServers collections if given a group name
  • Shared deployment logic: Extracted runSingleServer function used by both thv run and thv group run commands
  • Smart filtering system: Automatically filters secrets/env vars per server and converts between group syntax (server.VAR) and individual run syntax (VAR)

Testing Examples

Test basic group deployment

thv group run simple-test

Test with targeted secrets

thv group run simple-test --secret GITHUB_TOKEN,target=github.GITHUB_PERSONAL_ACCESS_TOKEN

Test with environment variables

thv group run simple-test --env fetch.DEBUG=true --env github.LOG_LEVEL=info

Signed-off-by: lujunsan <luisjuncaldev@gmail.com>
@lujunsan
Copy link
Contributor Author

@rdimitrov @eleftherias this ended up being more complex than I had anticipated, would appreciate a thorough review! Happy to change the approach if something seems out of place. I've manually tested this extensively locally, but would also appreciate a bit of testing if possible!

@codecov
Copy link

codecov bot commented Sep 17, 2025

Codecov Report

❌ Patch coverage is 15.74803% with 107 lines in your changes missing coverage. Please review.
✅ Project coverage is 46.96%. Comparing base (6084080) to head (cd57ef4).
⚠️ Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
pkg/runner/retriever/retriever.go 17.43% 84 Missing and 6 partials ⚠️
pkg/registry/provider_local.go 0.00% 7 Missing and 1 partial ⚠️
pkg/registry/provider_remote.go 0.00% 8 Missing ⚠️
pkg/mcp/server/run_server.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1912      +/-   ##
==========================================
+ Coverage   46.78%   46.96%   +0.18%     
==========================================
  Files         220      220              
  Lines       27384    27483      +99     
==========================================
+ Hits        12811    12907      +96     
+ Misses      13591    13584       -7     
- Partials      982      992      +10     

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

yrobla
yrobla previously approved these changes Sep 17, 2025
Copy link
Contributor

@yrobla yrobla left a comment

Choose a reason for hiding this comment

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

approving, i do not like the nolint, but it's something to be refactored as you said

Copy link
Member

@eleftherias eleftherias left a comment

Choose a reason for hiding this comment

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

I haven't done manual testing yet, but I have some thoughts on the structure. I'd lean towards having a more clearly separate path for running a group even if it involves some duplication. This is just a matter of preference so feel free to disagree and ignore the comments.

Signed-off-by: lujunsan <luisjuncaldev@gmail.com>
Signed-off-by: lujunsan <luisjuncaldev@gmail.com>
@coveralls
Copy link
Collaborator

Coverage Status

coverage: 44.006% (-0.2%) from 44.166%
when pulling cd57ef4 on add-registry-groups-cli-support
into 6084080 on main.

@lujunsan lujunsan merged commit 8cb3500 into main Sep 18, 2025
19 checks passed
@lujunsan lujunsan deleted the add-registry-groups-cli-support branch September 18, 2025 08: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.

6 participants