Extract vMCP serve/validate logic into pkg/vmcp/cli/#4891
Conversation
yrobla
requested review from
JAORMX,
amirejaz,
jerm-dro and
jhrozek
as code owners
There was a problem hiding this comment.
Pull request overview
This PR introduces a new importable pkg/vmcp/cli package that contains the extracted business logic for vmcp serve and vmcp validate, intended to be reused by both the standalone vmcp binary and future thv vmcp subcommands (per #4808 / #4879).
Changes:
- Added
pkg/vmcp/cli/serve.goimplementingcli.Serve(ctx, ServeConfig)plus internal helpers for config loading, auth server config loading, backend discovery, and session factory setup. - Added
pkg/vmcp/cli/validate.goimplementingcli.Validate(ctx, ValidateConfig)to load + validate a vMCP YAML config and log a validation summary.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pkg/vmcp/cli/serve.go | New exported Serve API with extracted server initialization sequence and helper functions. |
| pkg/vmcp/cli/validate.go | New exported Validate API with extracted config validation and summary output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4891 +/- ##
==========================================
- Coverage 69.12% 69.07% -0.06%
==========================================
Files 531 533 +2
Lines 55183 55195 +12
==========================================
- Hits 38146 38126 -20
- Misses 14113 14147 +34
+ Partials 2924 2922 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds a new importable pkg/vmcp/cli package that encapsulates the standalone vmcp CLI’s serve/validate business logic so it can be reused by other command entrypoints (e.g., future thv vmcp ... work) while keeping the existing binary unchanged for now.
Changes:
- Introduces
pkg/vmcp/cli/serve.goexportingServe(ctx, ServeConfig)plus supporting helpers for config loading, backend discovery, auth server config loading, etc. - Introduces
pkg/vmcp/cli/validate.goexportingValidate(ctx, ValidateConfig)to load + validate vMCP YAML configs and print a success summary.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/vmcp/cli/serve.go | New exported serve entrypoint and extracted server initialization helpers. |
| pkg/vmcp/cli/validate.go | New exported validate entrypoint and extracted config validation logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
github-actions
Bot
added
size/XL
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Large PR Detected
This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.
How to unblock this PR:
Add a section to your PR description with the following format:
## Large PR Justification
[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformationAlternative:
Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.
See our Contributing Guidelines for more details.
This review will be automatically dismissed once you add the justification section.
github-actions
Bot
dismissed
their stale review
Large PR justification has been provided. Thank you!
|
✅ Large PR justification has been provided. The size review has been dismissed and this PR can now proceed with normal review. |
github-actions
Bot
added
size/XL
There was a problem hiding this comment.
Pull request overview
This PR introduces a new importable pkg/vmcp/cli package to house the vMCP serve and validate business logic, enabling reuse across the standalone vmcp binary and future thv vmcp subcommands.
Changes:
- Added
pkg/vmcp/cli/serve.goimplementingServeplus the extracted helper logic (config load/validate, backend discovery, session factory, authserver sibling config loading). - Added
pkg/vmcp/cli/validate.goimplementingValidateand success-summary logging. - Refactored
cmd/vmcp/app/commands.goto delegateserve/validatetopkg/vmcp/cli, and adjusted a test package name to match the new package.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pkg/vmcp/cli/serve.go | New shared serve entrypoint and helpers extracted from the standalone CLI. |
| pkg/vmcp/cli/validate.go | New shared validate entrypoint and output/logging. |
| pkg/vmcp/cli/auth_server_config_test.go | Aligns test package to cli for testing new helpers. |
| cmd/vmcp/app/commands.go | Now thin-wraps serve/validate over the new pkg/vmcp/cli APIs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Create pkg/vmcp/cli/ with serve.go and validate.go, extracting all server-initialization and config-validation business logic from cmd/vmcp/app/commands.go into importable exported APIs (Serve, Validate). Thin-wrap cmd/vmcp/app/commands.go to delegate to the new package, and move the existing loadAuthServerConfig test to pkg/vmcp/cli/ where the function now lives. Closes #4879
github-actions
Bot
added
size/XL
There was a problem hiding this comment.
Pull request overview
Creates a new importable pkg/vmcp/cli package to host vMCP “serve” and “validate” business logic, and updates the standalone vmcp Cobra commands to call into those new APIs.
Changes:
- Added
pkg/vmcp/cli/serve.goimplementingServe(ctx, ServeConfig)plus extracted helpers (config load/validate, backend discovery, auth server config loading, session factory creation). - Added
pkg/vmcp/cli/validate.goimplementingValidate(ctx, ValidateConfig)and preserving the existing validation + summary logging behavior. - Refactored
cmd/vmcp/app/commands.goto delegateserveandvalidatecommand execution topkg/vmcp/cli.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/vmcp/cli/serve.go | New Serve entrypoint and extracted server initialization helpers. |
| pkg/vmcp/cli/validate.go | New Validate entrypoint extracted from the vmcp validate command. |
| pkg/vmcp/cli/auth_server_config_test.go | Adjusts test package to match new cli package. |
| cmd/vmcp/app/commands.go | Removes inlined business logic and delegates to pkg/vmcp/cli; also switches version reporting to versions.Version. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
github-actions
Bot
added
size/XL
There was a problem hiding this comment.
Large PR Detected
This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.
How to unblock this PR:
Add a section to your PR description with the following format:
## Large PR Justification
[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformationAlternative:
Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.
See our Contributing Guidelines for more details.
This review will be automatically dismissed once you add the justification section.
github-actions
Bot
added
size/XL
|
✅ Large PR justification has been provided. The size review has been dismissed and this PR can now proceed with normal review. |
github-actions
Bot
dismissed
their stale review
Large PR justification has been provided. Thank you!
github-actions
Bot
added
size/XL
There was a problem hiding this comment.
Pull request overview
This PR extracts the vMCP serve and validate business logic from the standalone cmd/vmcp/app Cobra command into a new importable package (pkg/vmcp/cli), leaving cmd/vmcp/app/commands.go as a thin flag-parsing wrapper. This aligns the vMCP CLI with the project’s thin-wrapper pattern and enables reuse by future callers.
Changes:
- Added
pkg/vmcp/cli/serve.goexposingServe()and private helpers for server initialization. - Added
pkg/vmcp/cli/validate.goexposingValidate()for config load + validation + summary output. - Moved
loadAuthServerConfigunit tests intopkg/vmcp/cli/and refactoredcmd/vmcp/app/commands.goto delegate to the new package (also switching version output toversions.Version).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/vmcp/cli/serve.go | New shared Serve() entry point and extracted server initialization helpers. |
| pkg/vmcp/cli/validate.go | New shared Validate() entry point for config validation and summary logging. |
| pkg/vmcp/cli/auth_server_config_test.go | Test moved to match new function location/package. |
| cmd/vmcp/app/commands.go | Reduced to Cobra scaffolding + flag parsing; delegates to pkg/vmcp/cli. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Create the new
pkg/vmcp/cli/package withserve.goandvalidate.go, extracting all server-initialization and config-validation business logic fromcmd/vmcp/app/commands.gointo importable exported APIs (Serve,Validate) backed by package-private helpers.This PR also performs the thin-wrap refactor originally scoped to #4880:
cmd/vmcp/app/commands.gois reduced to ~130 lines of Cobra scaffolding and flag parsing, delegating entirely topkg/vmcp/cli/. The existingloadAuthServerConfigunit test is moved fromcmd/vmcp/app/topkg/vmcp/cli/where the function now lives. #4880 has been closed as resolved here.Part of #4808
Closes #4879
Type of change
Test plan
task test)task test-e2e)task lint-fix)Changes
pkg/vmcp/cli/serve.goServeConfig+Serve()+ private helpers extracted fromcmd/vmcp/app/commands.gopkg/vmcp/cli/validate.goValidateConfig+Validate()entry pointpkg/vmcp/cli/auth_server_config_test.goloadAuthServerConfigunit tests moved fromcmd/vmcp/app/cmd/vmcp/app/commands.gopkg/vmcp/cli/cmd/vmcp/app/commands_test.gopkg/vmcp/cli/Does this introduce a user-facing change?
No. The standalone
vmcpbinary's flags, defaults, log output, and error messages are identical.Special notes for reviewers
cmd/vmcp/app/commands.goduplication is fully removed in this PR (not deferred to a follow-up).versions.Version(ldflag-overridable) is used consistently for version reporting in bothvmcp versionoutput andvmcpserver.Config.Version—getVersion()has been removed.//nolint:goseconos.ReadFileinloadAuthServerConfighas been updated to reflect the actual threat model (user-supplied local filesystem path) rather than referencing the CLI flag context.Large PR Justification
Refactoring PR involving extraction and movement of logic. The extraction and thin-wrap are tightly coupled and cannot be meaningfully split further.