Phase 3: Reduce server.New to wrapper + config split
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 2.2k
- Forks
- 300
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 184
Description
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 vestigial — cli/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+Authzfor the admission seam +
cross-cuttingTelemetryProvider/AuditConfig) and a transport-only
ServerConfig, exposed viaderiveCoreConfig(cfg, …)andderiveServerConfig(cfg)
(#5444). - Pass the cross-cutting fields (
TelemetryProvider,AuditConfig) to bothNew
andServe(R3 — not a clean partition); build the health monitor at the composition
root and thread itsStatusProviderintoNew+ the built*MonitorintoServe
(A2).ServerConfigomitsAuthzMiddleware; the field onserver.Configis kept
vestigial (cli/serve.gounchanged) (#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 + itss.core == nilguard (keepingconvertAnnotationsand the
vestigialAuthzMiddlewarefield). Highest-integration-risk PR; likely splits (#5445).
Out of scope
- Defining or moving any transport concern under
Serve, or theVMCP/Newcore
itself — all of that lands in #5430 (Phase 1) and #5431 (Phase 2);
this story only derives configs and wires the existingNew/Servetogether. - Any change to
server.New's signature — it is stable throughout the epic; only
the body is reimplemented. - Any change to
pkg/vmcp/cli/serve.go(the composition root) — it must require
no changes and keep callingvmcpserver.New(...)as today. - Any change to the CRD / YAML model or the
vmcpconfig.Configloaders / wire /
storage format — the split is in-memory only. - Docs (
docs/arch/vmcp-library.md,pkg/vmcp/doc.go) and the runnable decorator
example — #5433 (Phase 4).
Child Tasks
PR-sized tasks under this story (each ≤ 400 LOC, ≤ 10 files changed excluding
tests/docs/generated, one logical change):
- #5444: P3.1
deriveCoreConfig/deriveServerConfigconfig split - #5445: P3.2 Reduce
server.Newbody to the wrapper
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/deriveServerConfigderive the coreConfigand the
transportServerConfigfrom the existing in-memoryserver.Config, with the
cross-cuttingTelemetryProvider/AuditConfigpassed to both sides, the health
monitor built at the composition root and threaded both ways (A2), andServerConfig
omittingAuthzMiddleware(theserver.Configfield 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;convertAnnotationsand the vestigialAuthzMiddlewarefield retained) -
pkg/vmcp/cli/serve.gois unchanged; no in-repo caller (nor the external
brood-boxembedder) requires any change - No serialized / wire / CRD / YAML / storage format change — the split is
in-memory only andvmcpconfig.Configis an unchanged input toNew - 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 existingthv vmcp serve
E2E suite passes unchanged
References
- RFC THV-0076 (link above)
- Epic: #5419
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the Phase 3 guidance in architecture.md and the RFC, then inspect pkg/vmcp/server/server.go around Config and server.New, along with dependencies #5431, #5444, and #5445. Run the full behavioral-parity suite and the existing thv vmcp serve E2E suite before changing anything. Done means the stable New signature and behavior remain unchanged, cli/serve.go is untouched, and both suites pass after the config split and wrapper integration.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100