Skip to content

P2.1 Serve skeleton + ServerConfig #5439

@tgrunnagle

Description

@tgrunnagle

Description

Introduce the transport-side entry point of the New/Serve split: add
Serve(ctx, v VMCP, cfg *ServerConfig) (*Server, error) and the transport-only
ServerConfig struct in a new file pkg/vmcp/server/serve.go. Serve returns a
*Server wrapping the existing struct and builds the mcp-go server, the HTTP
route mux, and the HTTP lifecycle skeleton. This is the foundation the rest of
Phase 2 builds on — server.New is not yet routed through Serve, so observable
behavior is unchanged.

Context

Part of the vMCP interface refactor (epic #5419), which extracts an
identity-parameterized VMCP domain interface from the server.New god-object and
splits it into a core (New(cfg) -> VMCP) and a transport (Serve(ctx, VMCP, serverCfg) -> *Server). This task lands the Serve skeleton + ServerConfig; the
remaining transport concerns (SDK hooks, two-phase session creation, the middleware
chain, AS runner, status reporter, optimizer, health monitor) are relocated under
Serve by the subsequent Phase 2 tasks (#5440, #5441, #5443, #5442).

The guiding principle is relocate, don't rewrite: Serve returns the same
*Server struct, and all existing (*Server) accessors are carried forward
unchanged. See architecture.md "ServerConfig (R3)" (lines 243-274) and "Serve
(transport) wiring relocated" (lines 285-290); research.md "Config struct"
field-placement table (lines 144-167) for which fields land in ServerConfig.
ServerConfig is justified per go-style (which cautions against premature config
types) because these fields configure the HTTP/SDK runtime and are meaningless to
the core (R3).

Parent Story: #5431
Dependencies: #5437 (cross-story; needs VMCP/New from pkg/vmcp)
Blocks: #5440, #5441, #5443

Acceptance Criteria

  • Serve(ctx context.Context, v VMCP, cfg *ServerConfig) (*Server, error) and the
    ServerConfig struct (transport-only fields) exist in pkg/vmcp/server/serve.go.
  • Serve builds the mcp-go server (server.NewMCPServer), the HTTP route mux, and
    the HTTP lifecycle skeleton, and returns a *Server wrapping the existing struct.
  • (*Server).Handler / Start / Stop / Address / MCPServer / Ready (and the
    other carried-forward accessors) are preserved unchanged.
  • server.New is not yet routed through Serve; its 7-param signature and
    observable behavior are unchanged.
  • PR is ≤ 400 LOC and ≤ 10 files changed (excluding tests/docs/generated).
  • All tests pass (task test); lint clean (task lint-fix).
  • Code reviewed and approved.

Technical Approach

Recommended Implementation

Add serve.go next to server.go in pkg/vmcp/server. Define ServerConfig with
the transport-only fields enumerated in architecture.md (lines 250-267) — the subset
of today's server.Config (server.go:92) that the research Config table (lines
148-166) places on the transport side, plus the cross-cutting TelemetryProvider /
AuditConfig that are passed to both sides (R3).

Serve should, as a skeleton, perform only the transport-construction work that is
self-contained at this stage:

  • Apply the same transport defaults server.New applies today (server.go:311-327:
    Host127.0.0.1, EndpointPath/mcp, Name/Version/SessionTTL
    defaults). Keep the comment that Port == 0 means "OS-assigned" intact.
  • Construct the mcp-go server via server.NewMCPServer (mirroring server.go:333-340:
    dynamic tool/resource capabilities, logging, empty server.Hooks{} for now).
  • Build the route mux skeleton (health/ping/readyz/status/backend-health, optional
    /metrics, .well-known, embedded AS routes) mirroring the unauthenticated
    routes in Handler (server.go:538-572). The authenticated MCP endpoint /
    middleware chain is not moved here yet (that is P2.3 Move middleware chain under Serve; remove authz + annotation mw #5441).
  • Assemble and return the existing *Server struct (the one defined at
    server.go:188), populating it from v (the injected VMCP) and the
    ServerConfig fields so that the carried-forward Handler/Start/Stop continue
    to work unchanged.

Crucially, do not call Serve from server.New in this task. server.New
keeps its current body; Serve is additive and exercised only by its own tests until
#5444/#5445 (Phase 3) reduce server.New to the wrapper. This keeps each PR
within the LOC/file limits and keeps the parity surface flat for this task.

Patterns & Frameworks

  • mcp-go SDK boundary stays in Serveserver.NewMCPServer /
    NewStreamableHTTPServer / server.Hooks{} are transport concerns; no mcp-go types
    cross the VMCP boundary (Core Principle fix(typo): corrects readme #1; vmcp-anti-patterns.md Implement secret injection #5).
  • Thin orchestrator, not a new god-objectServe composes pre-built subsystems
    and returns the existing *Server; do not add new fields/concerns beyond wiring
    (vmcp-anti-patterns.md Bump golangci/golangci-lint-action from 2f856675483cb8b9378ee77ee0beb67955aca9d7 to 4696ba8babb6127d732c3c6dde519db15edab9ea #3).
  • ServerConfig is a focused transport config, decomposed at the composition root;
    it carries only what the transport needs plus the documented cross-cutting fields
    (go-style "Package API Surface"; vmcp-anti-patterns.md Figure out ergonomics for exposing directories #6).
  • Constructor validationServe returns an error for a nil cfg and a nil
    required collaborator (e.g. SessionFactory, which "must not be nil" per the
    research Config table, line 164); fail loudly (go-style "Constructor Validation").
  • stdlib testing.T + testify, no Ginkgo in pkg/vmcp/server (R8;
    testing.md). Add SPDX header to the new serve.go (go-style).

Code Pointers

  • pkg/vmcp/server/serve.go (new) — houses Serve + ServerConfig. See
    architecture.md "ServerConfig (R3)" (lines 243-274) and "Serve (transport) wiring
    relocated" (lines 285-290).
  • pkg/vmcp/server/server.go:92 — current Config struct; the source of the fields
    that partition into ServerConfig (transport) vs the core Config.
  • pkg/vmcp/server/server.go:188 — the existing Server struct that Serve wraps and
    returns (do not redefine it; reuse it).
  • pkg/vmcp/server/server.go:301server.New (7-param signature; stays stable
    and is NOT routed through Serve in this task). Defaults applied at 311-327; mcp-go
    server built at 333-340 — mirror these in Serve.
  • pkg/vmcp/server/server.go:528(*Server).Handler; NewStreamableHTTPServer at
    530; the route mux (health/metrics/.well-known/AS routes) at 538-572 — the mux
    skeleton Serve builds. The MCP endpoint + middleware chain (574-678) is deferred to
    P2.3 Move middleware chain under Serve; remove authz + annotation mw #5441.
  • pkg/vmcp/server/server.go:683(*Server).Start (HTTP lifecycle: handler build,
    http.Server with timeout constants, net.Listen, goroutine serve, ready
    close) — the lifecycle the skeleton must remain compatible with.
  • pkg/vmcp/server/server.go:798/868/1013/1019Stop / Address / MCPServer /
    Ready accessors that must be preserved (also SessionManager @976,
    GetBackendHealth*/GetHealthSummary). See research.md line 140 for the full
    carry-forward list.
  • research.md lines 144-167 — Config field-placement table driving which fields go
    on ServerConfig.

Component Interfaces

Shapes only (transcribed from architecture.md lines 243-274 and 130-134); not the full
implementation. Use literal placeholders where types are referenced by their existing
package aliases.

// pkg/vmcp/server/serve.go

// ServerConfig holds the transport-only runtime configuration for Serve.
// Justified (R3) because these fields configure the HTTP/SDK runtime and are
// meaningless to the core VMCP. The cross-cutting TelemetryProvider/AuditConfig
// are present on both ServerConfig and the core Config (not a clean partition).
// HealthMonitor is the ALREADY-BUILT *health.Monitor (constructed at the composition
// root so the same instance's StatusProvider can be injected into the core, which New
// builds before Serve runs — A2); Serve owns only its Start/Stop lifecycle. nil => disabled.
type ServerConfig struct {
    Name, Version, GroupRef string // Name/Version -> MCPServer; GroupRef -> status/logging
    Host         string
    Port         int
    EndpointPath string
    SessionTTL   time.Duration

    AuthMiddleware  func(http.Handler) http.Handler
    AuthInfoHandler http.Handler
    AuthServer      *asrunner.EmbeddedAuthServer

    HealthMonitor           *health.Monitor // built at composition root; Serve owns Start/Stop only (A2)
    StatusReportingInterval time.Duration
    StatusReporter          vmcpstatus.Reporter
    Watcher                 Watcher

    SessionFactory vmcpsession.MultiSessionFactory // required, must be non-nil
    SessionStorage *vmcpconfig.SessionStorageConfig

    OptimizerFactory func(context.Context, []server.ServerTool) (optimizer.Optimizer, error)
    OptimizerConfig  *optimizer.Config

    // Cross-cutting (also on the core Config) — passed to both New and Serve.
    TelemetryProvider *telemetry.Provider
    AuditConfig       *audit.Config
}

// Serve builds the mcp-go server, route mux, and HTTP lifecycle skeleton around an
// already-constructed VMCP, returning the existing *Server. Not yet called by
// server.New (Phase 3 routes it). Returns an error for nil cfg or nil required
// collaborators (e.g. SessionFactory).
func Serve(ctx context.Context, v vmcp.VMCP, cfg *ServerConfig) (*Server, error)

// Carried forward unchanged from server.go (528, 683, 798, 868, 1013, 1019, ...):
func (s *Server) Handler(ctx context.Context) (http.Handler, error)
func (s *Server) Start(ctx context.Context) error
func (s *Server) Stop(ctx context.Context) error
func (s *Server) Address() string
func (s *Server) MCPServer() *server.MCPServer
func (s *Server) Ready() <-chan struct{}

vmcp.VMCP and New(cfg) -> VMCP are defined upstream in #5437. This task only
consumes the VMCP interface; it does not define it.

Testing Strategy

Unit Tests (pkg/vmcp/server, stdlib testing.T + testify; mocks via gomock /
task gen)

  • Serve with a valid ServerConfig and a mock VMCP returns a non-nil *Server
    and a nil error.
  • Serve applies transport defaults identically to server.New
    (Host/EndpointPath/Name/Version/SessionTTL), and leaves Port == 0 as
    OS-assigned.
  • The returned *Server exposes a usable MCPServer() and the Handler(ctx) it
    builds registers the unauthenticated routes (/health, /ping, /readyz,
    /status, /api/backends/health, .well-known/) — and /metrics only when a
    TelemetryProvider with a Prometheus handler is configured.
  • Serve returns an error for a nil cfg and for a nil required collaborator
    (e.g. nil SessionFactory).

Integration / Behavioral Parity Tests

  • The existing server.New-driven behavioral-parity suite
    (pkg/vmcp/server/server_test.go, server/integration_test.go) stays green —
    server.New is untouched and not routed through Serve in this task.

Edge Cases

Out of Scope

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    refactorvmcpVirtual MCP Server related issues

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions