You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Create the new pkg/vmcp/cli/ package containing serve.go and validate.go by extracting the serve and validate business logic out of cmd/vmcp/app/commands.go. The five functions to move are runServe, loadAndValidateConfig, discoverBackends, createSessionFactory, and loadAuthServerConfig. This establishes a clean, importable library package that all subsequent thv vmcp subcommand work (#4880 through #4884) depends on, and formalizes the separation between CLI wiring and business logic that the thin wrapper principle requires.
Context
cmd/vmcp/app/commands.go (665 lines) currently bundles all serve and validate business logic directly inside the standalone vmcp binary. RFC THV-0059 calls for that logic to move into pkg/vmcp/cli/ so it can be reused by both the existing standalone binary and the new thv vmcp subcommand being introduced in later phases. This item is Phase 1 of the RFC implementation plan — no changes to the standalone binary's external behavior occur here, and the cmd/vmcp/app/commands.go file is not yet refactored (that is #4880). The sole deliverable is the new library package with the extracted functions exposed as exported Go APIs.
pkg/vmcp/cli/ does not currently exist. It is a purely additive package — it introduces no breaking changes to any existing package. The brood-box project (github.com/stacklok/brood-box) embeds pkg/vmcp/ as a library; the new package must not alter any existing import paths or change APIs of packages rated Stable in the RFC stability table.
pkg/vmcp/cli/ package exists with serve.go and validate.go files, each carrying the required SPDX copyright header
serve.go exports a Serve(ctx context.Context, cfg ServeConfig) error function (or equivalent top-level entry point) encapsulating the full server initialization sequence from the current runServe
validate.go exports a Validate(ctx context.Context, cfg ValidateConfig) error function (or equivalent) encapsulating the config load-and-validate sequence from the current newValidateCmd inline logic
LoadAndValidateConfig, DiscoverBackends, CreateSessionFactory, and LoadAuthServerConfig are exported (or package-private helpers called by exported entry points) and live in pkg/vmcp/cli/
The existing cmd/vmcp/app/commands.go is not yet changed — the standalone binary continues to compile and its original functions remain in place (duplication is acceptable at this stage; Thin-wrap standalone vmcp binary over pkg/vmcp/cli/ #4880 removes it)
go build ./pkg/vmcp/cli/... succeeds with no errors
go vet ./pkg/vmcp/cli/... passes with no issues
No new external Go module dependencies are introduced (all imports must already exist in go.mod)
No existing package import paths are changed
All existing tests pass (no regressions)
Code reviewed and approved
Technical Approach
Recommended Implementation
Create pkg/vmcp/cli/ as a new Go package. Copy the five target functions from cmd/vmcp/app/commands.go into appropriately split files, adjust visibility to exported, and define input configuration structs that capture the flag values the caller (currently Cobra, later cmd/thv/app/vmcp.go) would pass in. Keep the internal logic identical to the current implementation — this is a mechanical extraction, not a refactor.
Split the files as follows:
serve.go — Serve() entry point, ServeConfig struct, plus the helper logic from loadAndValidateConfig, discoverBackends, createSessionFactory, loadAuthServerConfig, and getStatusReportingInterval. These are all only needed by the serve path.
validate.go — Validate() entry point, ValidateConfig struct. Internally calls config.NewYAMLLoader and config.NewValidator directly (same code as the current inline newValidateCmd body).
Because cmd/vmcp/app/commands.go currently imports K8s-specific packages (k8s.io/client-go/rest, github.com/stacklok/toolhive/pkg/vmcp/k8s), the extracted code in pkg/vmcp/cli/serve.go will carry those same imports. This is acceptable — the K8s discovery path inside discoverBackends/runServe is guarded at runtime (cfg.OutgoingAuth.Source == "discovered" and runtime.IsKubernetesRuntime()), not at compile time. No build-tag separation is needed at this phase.
The existing //nolint:gocyclo comment on runServe may be carried forward initially. Use this extraction as an opportunity to decompose runServe into smaller, named helper calls within Serve() to reduce cyclomatic complexity and potentially eliminate the nolint directive — but do not block the PR on this cleanup.
Patterns & Frameworks
SPDX headers: Every new .go file must open with // SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. and // SPDX-License-Identifier: Apache-2.0
Package declaration: package cli (matching directory name pkg/vmcp/cli/)
Error wrapping: Use fmt.Errorf("...: %w", err) throughout; never suppress or swallow errors
No sensitive data in errors: Do not include credential values, tokens, or keys in error strings
Immutable variable assignment: Prefer immediately-assigned variables over mutable var with branches; use immediately-invoked anonymous functions where needed
Thin wrapper principle: This package is the business logic home; cmd/ files will become the thin wrappers that call it
Code Pointers
cmd/vmcp/app/commands.go — Primary extraction source; runServe (lines 378–665), loadAndValidateConfig (lines 211–237), loadAuthServerConfig (lines 243–259), discoverBackends (lines 264–319), createSessionFactory (lines 330–373) are the five functions to move
pkg/vmcp/config/config.go — The Config struct and NewYAMLLoader/NewValidator constructors that the extracted code depends on; do not change this file
cmd/thv/app/mcp.go — Reference for how cmd/thv/app/ will eventually call pkg/vmcp/cli/; drives the shape of ServeConfig/ValidateConfig structs
test/integration/vmcp/helpers/vmcp_server.go — Pattern for how the integration test layer constructs vMCP servers; unit tests in Unit tests for pkg/vmcp/cli/ serve and validate logic #4881 will follow a similar approach against pkg/vmcp/cli/
Component Interfaces
// pkg/vmcp/cli/serve.go// ServeConfig holds all parameters needed to start the vMCP server.// Populated by the caller (currently cmd/vmcp/app or later cmd/thv/app/vmcp.go)// from Cobra flag values.typeServeConfigstruct {
ConfigPathstringHoststringPortintEnableAuditbool
}
// Serve loads configuration, initializes all subsystems, and starts the vMCP// server. It blocks until the context is cancelled or the server stops.funcServe(ctx context.Context, cfgServeConfig) error { ... }
// pkg/vmcp/cli/validate.go// ValidateConfig holds parameters for the validate command.typeValidateConfigstruct {
ConfigPathstring
}
// Validate loads and validates a vMCP configuration file, returning a// descriptive error if the file is missing, malformed, or fails semantic// validation. On success it returns nil.funcValidate(ctx context.Context, cfgValidateConfig) error { ... }
The helper functions (loadAndValidateConfig, discoverBackends, createSessionFactory, loadAuthServerConfig, getStatusReportingInterval) may remain unexported package-level functions within serve.go if they are only called from Serve. Export them if #4881 unit tests need to call them directly; otherwise keep them unexported to minimize the surface area.
Testing Strategy
Note: Thorough unit tests for this package are deliberately placed in #4881 to keep this PR within the 400-line / 10-file PR size limit. This item only needs to verify that the package compiles and does not regress existing tests.
Description
Create the new
pkg/vmcp/cli/package containingserve.goandvalidate.goby extracting the serve and validate business logic out ofcmd/vmcp/app/commands.go. The five functions to move arerunServe,loadAndValidateConfig,discoverBackends,createSessionFactory, andloadAuthServerConfig. This establishes a clean, importable library package that all subsequentthv vmcpsubcommand work (#4880 through #4884) depends on, and formalizes the separation between CLI wiring and business logic that the thin wrapper principle requires.Context
cmd/vmcp/app/commands.go(665 lines) currently bundles all serve and validate business logic directly inside the standalonevmcpbinary. RFC THV-0059 calls for that logic to move intopkg/vmcp/cli/so it can be reused by both the existing standalone binary and the newthv vmcpsubcommand being introduced in later phases. This item is Phase 1 of the RFC implementation plan — no changes to the standalone binary's external behavior occur here, and thecmd/vmcp/app/commands.gofile is not yet refactored (that is #4880). The sole deliverable is the new library package with the extracted functions exposed as exported Go APIs.pkg/vmcp/cli/does not currently exist. It is a purely additive package — it introduces no breaking changes to any existing package. The brood-box project (github.com/stacklok/brood-box) embedspkg/vmcp/as a library; the new package must not alter any existing import paths or change APIs of packages rated Stable in the RFC stability table.Dependencies: #4808 (parent)
Blocks: #4880, #4881, #4882, #4883, #4884
Acceptance Criteria
pkg/vmcp/cli/package exists withserve.goandvalidate.gofiles, each carrying the required SPDX copyright headerserve.goexports aServe(ctx context.Context, cfg ServeConfig) errorfunction (or equivalent top-level entry point) encapsulating the full server initialization sequence from the currentrunServevalidate.goexports aValidate(ctx context.Context, cfg ValidateConfig) errorfunction (or equivalent) encapsulating the config load-and-validate sequence from the currentnewValidateCmdinline logicLoadAndValidateConfig,DiscoverBackends,CreateSessionFactory, andLoadAuthServerConfigare exported (or package-private helpers called by exported entry points) and live inpkg/vmcp/cli/cmd/vmcp/app/commands.gois not yet changed — the standalone binary continues to compile and its original functions remain in place (duplication is acceptable at this stage; Thin-wrap standalone vmcp binary over pkg/vmcp/cli/ #4880 removes it)go build ./pkg/vmcp/cli/...succeeds with no errorsgo vet ./pkg/vmcp/cli/...passes with no issuesgo.mod)Technical Approach
Recommended Implementation
Create
pkg/vmcp/cli/as a new Go package. Copy the five target functions fromcmd/vmcp/app/commands.gointo appropriately split files, adjust visibility to exported, and define input configuration structs that capture the flag values the caller (currently Cobra, latercmd/thv/app/vmcp.go) would pass in. Keep the internal logic identical to the current implementation — this is a mechanical extraction, not a refactor.Split the files as follows:
serve.go—Serve()entry point,ServeConfigstruct, plus the helper logic fromloadAndValidateConfig,discoverBackends,createSessionFactory,loadAuthServerConfig, andgetStatusReportingInterval. These are all only needed by the serve path.validate.go—Validate()entry point,ValidateConfigstruct. Internally callsconfig.NewYAMLLoaderandconfig.NewValidatordirectly (same code as the current inlinenewValidateCmdbody).Because
cmd/vmcp/app/commands.gocurrently imports K8s-specific packages (k8s.io/client-go/rest,github.com/stacklok/toolhive/pkg/vmcp/k8s), the extracted code inpkg/vmcp/cli/serve.gowill carry those same imports. This is acceptable — the K8s discovery path insidediscoverBackends/runServeis guarded at runtime (cfg.OutgoingAuth.Source == "discovered"andruntime.IsKubernetesRuntime()), not at compile time. No build-tag separation is needed at this phase.The existing
//nolint:gocyclocomment onrunServemay be carried forward initially. Use this extraction as an opportunity to decomposerunServeinto smaller, named helper calls withinServe()to reduce cyclomatic complexity and potentially eliminate the nolint directive — but do not block the PR on this cleanup.Patterns & Frameworks
.gofile must open with// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.and// SPDX-License-Identifier: Apache-2.0package cli(matching directory namepkg/vmcp/cli/)fmt.Errorf("...: %w", err)throughout; never suppress or swallow errorsvarwith branches; use immediately-invoked anonymous functions where neededcmd/files will become the thin wrappers that call itCode Pointers
cmd/vmcp/app/commands.go— Primary extraction source;runServe(lines 378–665),loadAndValidateConfig(lines 211–237),loadAuthServerConfig(lines 243–259),discoverBackends(lines 264–319),createSessionFactory(lines 330–373) are the five functions to movepkg/vmcp/config/config.go— TheConfigstruct andNewYAMLLoader/NewValidatorconstructors that the extracted code depends on; do not change this filecmd/thv/app/mcp.go— Reference for howcmd/thv/app/will eventually callpkg/vmcp/cli/; drives the shape ofServeConfig/ValidateConfigstructs.claude/rules/go-style.md— SPDX header requirement, error handling conventions, immutable variable assignment guidancetest/integration/vmcp/helpers/vmcp_server.go— Pattern for how the integration test layer constructs vMCP servers; unit tests in Unit tests forpkg/vmcp/cli/serve and validate logic #4881 will follow a similar approach againstpkg/vmcp/cli/Component Interfaces
The helper functions (
loadAndValidateConfig,discoverBackends,createSessionFactory,loadAuthServerConfig,getStatusReportingInterval) may remain unexported package-level functions withinserve.goif they are only called fromServe. Export them if #4881 unit tests need to call them directly; otherwise keep them unexported to minimize the surface area.Testing Strategy
Note: Thorough unit tests for this package are deliberately placed in #4881 to keep this PR within the 400-line / 10-file PR size limit. This item only needs to verify that the package compiles and does not regress existing tests.
Unit Tests (minimal — full coverage in #4881)
go build ./pkg/vmcp/cli/...produces no errors (CI enforces this)go vet ./pkg/vmcp/cli/...produces no warnings (CI enforces this)Integration Tests
test/integration/vmcp/vmcp_integration_test.gocontinues to pass (no behavior change — the standalone binary is untouched in this item)Edge Cases
Out of Scope
cmd/vmcp/app/commands.goto call the new package (that is Thin-wrap standalone vmcp binary over pkg/vmcp/cli/ #4880)pkg/vmcp/cli/init.goorpkg/vmcp/cli/embedding_manager.go(those are Addinit.gotopkg/vmcp/cli/for config scaffolding #4882 and Implement EmbeddingServiceManager in pkg/vmcp/cli/ #4884)thv vmcpCobra commands (that is Addthv vmcp serveandthv vmcp validatesubcommands #4883)vmcpbinary's external behaviorpkg/vmcp/cli/serve and validate logic #4881)References
pkg/vmcp/cli/docs/arch/10-virtual-mcp-architecture.md— Existing K8s-oriented vMCP architecture documentation.claude/rules/go-style.md— ToolHive Go coding conventions (SPDX, error handling, linting).claude/rules/cli-commands.md— Thin wrapper principle forcmd/thv/app/