Skip to content

Phase 3: Reduce server.New to wrapper + config split #5432

Description

@tgrunnagle

Description

Land the structural payoff of the vMCP domain/transport split: split the in-memory
server.Config into a core Config + a transport ServerConfig (via
deriveCoreConfig/deriveServerConfig), then reduce server.New's body to the
thin wrapper Serve(ctx, New(deriveCoreConfig(cfg)), deriveServerConfig(cfg))
removing both //nolint:gocyclo as the god-object dismantles. This story maps to
RFC Phase 3 and is the only phase that touches server.New's body; the
7-param signature and observable behavior stay byte-for-byte stable while the
implementation behind them collapses onto the New/Serve seam built in Phases 1–2.

Context

See RFC THV-0076: vMCP Core Interface
for full design details. Part of the vMCP interface refactor (epic #5419).

By the time this story runs, Phase 1 has produced the VMCP interface +
New(cfg) -> VMCP core (with the admission and elicitation seams) and Phase 2 has
produced Serve(ctx, VMCP, *ServerConfig) -> *Server with all transport concerns
(mcp-go server, SDK hooks, two-phase session creation, the full middleware chain,
AS runner, status reporter, optimizer, health monitor) already re-homed under it —
but Serve is not yet called by server.New. This story closes that loop. Per
architecture.md ("PR-Sized Decomposition Guidance → Phase 3" and "Key Files to
Modify"), the work is the config decomposition plus the wrapper reduction at
pkg/vmcp/server/server.go:301.

The config split is not a clean partition (R3): per architecture.md
"Constraints", the cross-cutting fields TelemetryProvider, AuditConfig, and the
health view are consumed on both sides (the core decorates the backend client
with telemetry and runs the workflow auditor; the transport adds telemetry/audit
middleware and lifecycles the health monitor). The health monitor is built at the
composition root
(A2): because New runs before Serve, the wrapper builds it once,
injects its StatusProvider into New, and hands the built *health.Monitor to Serve.
ServerConfig carries no AuthzMiddleware (authz lives in the core admission seam);
the AuthzMiddleware field on server.Config is kept vestigialcli/serve.go
sets it and stays unchanged — and only the dead HTTP authz/annotation blocks (plus the
discovery middleware/seam) are deleted in #5445 once the legacy path is gone. This is an
in-memory-only change:
vmcpconfig.Config, the CRD/YAML model, and the wire/storage format are unchanged
inputs to New.

This phase is small but high-stakes: it is gated by the full behavioral-parity
suite
(driving the stable server.New wrapper) plus the unchanged
thv vmcp serve E2E suite — the only safety net proving the reimplemented body is
observably identical.

RFC Phase(s): Phase 3
Dependencies: #5431

Scope

In scope

  • Split the in-memory-only server.Config (server.go:92-185) into the core
    Config (collaborators + workflowDefs + Authz for the admission seam +
    cross-cutting TelemetryProvider/AuditConfig) and a transport-only
    ServerConfig, exposed via deriveCoreConfig(cfg, …) and deriveServerConfig(cfg)
    (P3.1 deriveCoreConfig/deriveServerConfig config split #5444).
  • Pass the cross-cutting fields (TelemetryProvider, AuditConfig) to both New
    and Serve (R3 — not a clean partition); build the health monitor at the composition
    root and thread its StatusProvider into New + the built *Monitor into Serve
    (A2). ServerConfig omits AuthzMiddleware; the field on server.Config is kept
    vestigial
    (cli/serve.go unchanged) (P3.1 deriveCoreConfig/deriveServerConfig config split #5444).
  • Reduce server.New's body to
    Serve(ctx, New(deriveCoreConfig(cfg, …)), deriveServerConfig(cfg, healthMon)), keeping
    the 7-param signature byte-for-byte unchanged, and remove both //nolint:gocyclo
    (New @300, Start @682). With the legacy path now gone, complete the A1-deferred
    cleanup
    : delete the now-dead authz/annotation HTTP blocks and retire the discovery
    middleware/seam + its s.core == nil guard (keeping convertAnnotations and the
    vestigial AuthzMiddleware field). Highest-integration-risk PR; likely splits (P3.2 Reduce server.New body to the wrapper #5445).

Out of scope

Child Tasks

PR-sized tasks under this story (each ≤ 400 LOC, ≤ 10 files changed excluding
tests/docs/generated, one logical change):

Acceptance Criteria

  • All child tasks complete and merged
  • server.New's 7-param signature and observable behavior are unchanged
    (byte-for-byte signature; clients/embedders see no behavioral difference)
  • deriveCoreConfig/deriveServerConfig derive the core Config and the
    transport ServerConfig from the existing in-memory server.Config, with the
    cross-cutting TelemetryProvider/AuditConfig passed to both sides, the health
    monitor built at the composition root and threaded both ways (A2), and ServerConfig
    omitting AuthzMiddleware (the server.Config field is kept vestigial; cli/serve.go
    unchanged) (R3)
  • server.New's body is reduced to
    Serve(ctx, New(deriveCoreConfig(cfg, …)), deriveServerConfig(cfg, healthMon)), both
    //nolint:gocyclo (New @300, Start @682) are removed with lint clean, and the
    A1-deferred cleanup is completed (dead authz/annotation blocks + discovery middleware/seam
    deleted; convertAnnotations and the vestigial AuthzMiddleware field retained)
  • pkg/vmcp/cli/serve.go is unchanged; no in-repo caller (nor the external
    brood-box embedder) requires any change
  • No serialized / wire / CRD / YAML / storage format change — the split is
    in-memory only and vmcpconfig.Config is an unchanged input to New
  • Acceptance gate: the full behavioral-parity suite (tools/list,
    tools/call, resources, prompts, composite workflows, session lifecycle, cross-pod
    Redis paths) passes equivalently before/after, and the existing thv vmcp serve
    E2E suite passes unchanged

References

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    refactorvmcpVirtual MCP Server related issues

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions