Skip to content

Extract shared vMCP logic into pkg/vmcp/cli/ (serve + validate) #4879

@yrobla

Description

@yrobla

Description

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.

Dependencies: #4808 (parent)
Blocks: #4880, #4881, #4882, #4883, #4884

Acceptance Criteria

  • 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.goServe() 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.goValidate() 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
  • .claude/rules/go-style.md — SPDX header requirement, error handling conventions, immutable variable assignment guidance
  • 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.
type ServeConfig struct {
    ConfigPath  string
    Host        string
    Port        int
    EnableAudit bool
}

// Serve loads configuration, initializes all subsystems, and starts the vMCP
// server. It blocks until the context is cancelled or the server stops.
func Serve(ctx context.Context, cfg ServeConfig) error { ... }
// pkg/vmcp/cli/validate.go

// ValidateConfig holds parameters for the validate command.
type ValidateConfig struct {
    ConfigPath string
}

// 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.
func Validate(ctx context.Context, cfg ValidateConfig) 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.

Unit Tests (minimal — full coverage in #4881)

  • Confirm go build ./pkg/vmcp/cli/... produces no errors (CI enforces this)
  • Confirm go vet ./pkg/vmcp/cli/... produces no warnings (CI enforces this)

Integration Tests

  • Existing test/integration/vmcp/vmcp_integration_test.go continues to pass (no behavior change — the standalone binary is untouched in this item)

Edge Cases

  • Verify the package compiles on both Linux/amd64 and Darwin/arm64 (CI matrix covers this)

Out of Scope

References

  • RFC THV-0059 — Authoritative design; specifies Phase 1 as extraction of shared logic into pkg/vmcp/cli/
  • GitHub Issue #4808 — Parent tracking issue
  • 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 for cmd/thv/app/

Metadata

Metadata

Assignees

Labels

cliChanges that impact CLI functionalityenhancementNew feature or requestvmcpVirtual MCP Server related issues

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions