Thin-wrap standalone vmcp binary over pkg/vmcp/cli/

[yrobla]

Description

Refactor cmd/vmcp/app/commands.go to become a thin wrapper over the new pkg/vmcp/cli/ package created in #4879. All serve and validate business logic is removed from cmd/vmcp/app/ and replaced with direct calls to pkg/vmcp/cli/Serve() and pkg/vmcp/cli/Validate(). The standalone vmcp binary's external behavior — flags, defaults, output, error messages — must remain identical; this is a pure internal restructuring with no user-visible changes.

Context

After #4879 creates pkg/vmcp/cli/ with the extracted business logic, cmd/vmcp/app/commands.go still contains the original copies of runServe, loadAndValidateConfig, discoverBackends, createSessionFactory, loadAuthServerConfig, and getStatusReportingInterval — approximately 350 lines of duplicated logic. This item removes that duplication by replacing each function body with a call to the corresponding exported function in pkg/vmcp/cli/. The result matches the thin wrapper principle enforced for all cmd/thv/app/ files and ensures that future callers (thv vmcp serve, thv vmcp validate) share a single implementation path.

This is a Phase 1 item in RFC THV-0059. It is safe to merge independently once #4879 is merged, and it must land before #4883 introduces thv vmcp serve/validate so those commands delegate to a single canonical implementation.

Dependencies: #4879
Blocks: (none — can merge independently after #4879)

Acceptance Criteria

• cmd/vmcp/app/commands.go no longer contains the bodies of runServe, loadAndValidateConfig, discoverBackends, createSessionFactory, loadAuthServerConfig, or getStatusReportingInterval; each is replaced by a delegation to pkg/vmcp/cli/
• newServeCmd().RunE delegates to pkg/vmcp/cli.Serve(ctx, cfg) — no business logic remains in the Cobra callback
• newValidateCmd().RunE delegates to pkg/vmcp/cli.Validate(ctx, cfg) — no business logic remains in the Cobra callback
• cmd/vmcp/app/commands.go only parses flags, builds a cli.ServeConfig or cli.ValidateConfig, and calls into pkg/vmcp/cli/
• The standalone vmcp serve binary behaves identically before and after the change: same flags, same defaults (--host 127.0.0.1, --port 4483), same log output, same error messages
• The standalone vmcp validate binary behaves identically before and after the change
• go build ./cmd/vmcp/... succeeds
• go vet ./cmd/vmcp/... passes with no issues
• The //nolint:gocyclo directive on runServe is removed (complexity now lives in pkg/vmcp/cli/serve.go which is the appropriate place to manage it)
• No new external Go module dependencies are introduced
• All existing tests pass (no regressions)
• Code reviewed and approved

Technical Approach

Recommended Implementation

cmd/vmcp/app/commands.go retains its Cobra command scaffolding (rootCmd, NewRootCmd, newServeCmd, newValidateCmd, newVersionCmd) and flag definitions unchanged. The implementation of runServe becomes a three-step function: extract flag values → build cli.ServeConfig → call vmcpcli.Serve(ctx, cfg). The implementation of newValidateCmd().RunE becomes: read --config → build cli.ValidateConfig → call vmcpcli.Validate(ctx, cfg).

The five helper functions (loadAndValidateConfig, discoverBackends, createSessionFactory, loadAuthServerConfig, getStatusReportingInterval) are deleted from cmd/vmcp/app/commands.go entirely. Their logic already lives in pkg/vmcp/cli/ after #4879 merges.

Pay attention to the enableAudit flag handling inside runServe (lines 393–399 of the current commands.go): this is flag-parsing logic that remains in the CLI layer, passed into ServeConfig.EnableAudit so pkg/vmcp/cli/Serve can act on it.

The getVersion() helper in cmd/vmcp/app/commands.go stays — it is injected by ldflags at build time and must be passed into ServeConfig as the version string.

Patterns & Frameworks

• Thin wrapper principle (hard rule from .claude/rules/cli-commands.md): cmd/ files must only parse flags, call pkg/ functions, and format output — zero business logic
• SPDX headers: The file already has the required SPDX header; preserve it unchanged
• Error wrapping: Errors from pkg/vmcp/cli/ bubble up directly; no re-wrapping in the CLI layer unless adding CLI-specific context
• Cobra flag extraction pattern: Use cmd.Flags().GetString("host"), cmd.Flags().GetInt("port"), cmd.Flags().GetBool("enable-audit") to build the config struct — same pattern as the current runServe
• Immutable variable assignment: Prefer cfg := cli.ServeConfig{...} over var cfg cli.ServeConfig followed by field mutations

Code Pointers

• cmd/vmcp/app/commands.go — The file being refactored; runServe (lines 378–665) and newValidateCmd().RunE (lines 144–190) are the primary replacement targets; the five helper functions (lines 199–373) are deleted
• pkg/vmcp/cli/serve.go — Created by Extract shared vMCP logic into pkg/vmcp/cli/ (serve + validate) #4879; provides Serve(ctx context.Context, cfg ServeConfig) error and the ServeConfig struct — this is what runServe must delegate to
• pkg/vmcp/cli/validate.go — Created by Extract shared vMCP logic into pkg/vmcp/cli/ (serve + validate) #4879; provides Validate(ctx context.Context, cfg ValidateConfig) error — this is what newValidateCmd().RunE must delegate to
• cmd/thv/app/mcp.go — Reference pattern for how a thin-wrapper CLI command calls pkg/ functions; observe flag extraction → struct population → single pkg/ call structure

Component Interfaces

After this item, the entirety of runServe should collapse to approximately the following shape. The exact field names depend on the ServeConfig struct defined in #4879.

// cmd/vmcp/app/commands.go (after refactor)

func runServe(cmd *cobra.Command, _ []string) error {
    ctx := cmd.Context()
    configPath := viper.GetString("config")
    if configPath == "" {
        return fmt.Errorf("no configuration file specified, use --config flag")
    }

    host, _ := cmd.Flags().GetString("host")
    port, _ := cmd.Flags().GetInt("port")
    enableAudit, _ := cmd.Flags().GetBool("enable-audit")

    return vmcpcli.Serve(ctx, vmcpcli.ServeConfig{
        ConfigPath:  configPath,
        Host:        host,
        Port:        port,
        EnableAudit: enableAudit,
        Version:     getVersion(),
    })
}

And newValidateCmd().RunE collapses to:

func(cmd *cobra.Command, _ []string) error {
    configPath := viper.GetString("config")
    if configPath == "" {
        return fmt.Errorf("no configuration file specified, use --config flag")
    }
    return vmcpcli.Validate(cmd.Context(), vmcpcli.ValidateConfig{
        ConfigPath: configPath,
    })
}

Testing Strategy

Unit Tests

• No new unit tests are required for this item — business logic tests live in pkg/vmcp/cli/ (Unit tests for pkg/vmcp/cli/ serve and validate logic #4881). The CLI layer is thin enough to be covered by E2E tests.

Integration Tests

• test/integration/vmcp/vmcp_integration_test.go must continue to pass without modification — it exercises the full serve path end-to-end via the compiled binary, verifying that the delegation chain produces identical behavior

Edge Cases

• vmcp serve with no --config flag still returns "no configuration file specified, use --config flag" (validation in the CLI layer, not delegated)
• vmcp validate with no --config flag still returns the same error
• vmcp serve with a valid config starts and accepts connections identically to the pre-refactor binary (verified by existing integration tests)

Out of Scope

• Creating pkg/vmcp/cli/ or any new files in that package (that is Extract shared vMCP logic into pkg/vmcp/cli/ (serve + validate) #4879)
• Adding unit tests for pkg/vmcp/cli/ (that is Unit tests for pkg/vmcp/cli/ serve and validate logic #4881)
• Creating cmd/thv/app/vmcp.go or adding thv vmcp subcommands (that is Add thv vmcp serve and thv vmcp validate subcommands #4883)
• Changing any flag name, default value, or external behavior of the vmcp binary
• Removing the standalone vmcp binary (it is preserved for K8s deployments)
• Any changes to pkg/vmcp/ packages other than importing the new pkg/vmcp/cli/
• Documentation updates (that is Architecture documentation for local vMCP #4890)

References

• RFC THV-0059 — Phase 1 implementation plan; Section "Standalone binary refactor" describes this thin-wrap step
• GitHub Issue #4808 — Parent tracking issue
• Extract shared vMCP logic into pkg/vmcp/cli/ (serve + validate) #4879 issue — Creates the pkg/vmcp/cli/ package that this item delegates to
• .claude/rules/cli-commands.md — Thin wrapper principle; adding new commands checklist
• .claude/rules/go-style.md — SPDX headers, error handling conventions
• cmd/thv/app/mcp.go — Canonical thin-wrapper pattern to follow

[yrobla]

Unified with #4879 — the thin-wrap refactor of cmd/vmcp/app/commands.go and the migration of the existing loadAuthServerConfig test to pkg/vmcp/cli/ were folded into the same PR to keep the change cohesive. Closing this as resolved by #4879.