Skip to content

P1.4 New(cfg) -> VMCP core constructor #5437

@tgrunnagle

Description

@tgrunnagle

Description

Implement the concrete *coreVMCP and New(cfg *Config) (VMCP, error) constructor
in the root pkg/vmcp package by relocating (not rewriting) the domain wiring that
lives in server.New today. This is the structural heart of RFC Phase 1: it turns the
VMCP contract from #5434 into a working, identity-parameterized core that aggregates
capabilities, routes calls, and drives composite workflows — without any mcp-go types
crossing the boundary and without touching server.New.

Context

The core constructor relocates the collaborator wiring currently buried in server.New's
god-object body (server.go:330-465): telemetry backend-client decoration, the workflow
auditor, the in-memory state store + NewWorkflowEngine, the per-session
sessionComposerFactory/NewSessionRouter pattern, and validateWorkflows fail-fast.
Per architecture.md ("New (core) wiring relocated", lines ~278-284 and
"Statelessness, Session Consistency & Health Filtering (R5/R7)"), the core filters,
Serve caches
: the core is stateless w.r.t. sessions, takes an injected
health.StatusProvider, and runs filterHealthyBackends internally over the backend
registry before aggregating — preserving today's exact include/exclude rules. The
StatusProvider is built at the composition root (the server.New wrapper / test
harness, per A2) and the same *health.Monitor is lifecycled by Serve (#5443); the
core only reads it, and a nil StatusProvider means no health filtering (all backends
included), matching today's no-monitor behavior. ListTools
drives aggregator.AggregateCapabilities on demand; CallTool/ReadResource/GetPrompt
route via router.Router + composer; Lookup* resolve advertised names (incl.
BackendID) without invoking. The admission seam is NOT wired here — it lands in
#5438. Identity is an explicit *auth.Identity on every method, never read from
context (anti-pattern #1).

Parent Story: #[#5430]
Dependencies: #[#5434] (VMCP interface + core Config), #[#5436] (domain-typed ElicitationRequester)
Blocks: #5438, #5439

Split note (pre-planned axis): This PR is likely to exceed the 400 LOC / 10 file
limit — it relocates the whole server.go:330-465 wiring and implements 10 interface
methods and unit tests — so plan to split it along this axis:

  • TASK-104a — the New constructor + collaborator wiring + the injected
    health.StatusProvider/filterHealthyBackends + the list/lookup paths
    (ListTools/ListResources/ListPrompts/Lookup*) + Close. The shared
    constructor wiring and health filtering land here.
  • TASK-104b — the call/read/get paths (CallTool/ReadResource/GetPrompt),
    including composite-tool execution through the per-session composer; reuses the
    *coreVMCP constructed in 104a.

If split: TASK-104a depends on #5434 + #5436, TASK-104b depends on TASK-104a, and
both fan into #5438 / #5439 the same way #5437 does. Use the /split-pr skill.

Acceptance Criteria

  • New(cfg *Config) (VMCP, error) returns a working *coreVMCP with all interface
    methods implemented: ListTools/CallTool/ListResources/ReadResource/
    ListPrompts/GetPrompt/LookupTool/LookupResource/LookupPrompt/Close.
  • The core runs filterHealthyBackends from an injected health.StatusProvider
    (built at the composition root, not constructed by the core; a nil provider ⇒ no
    filtering, all backends included) over backendRegistry.List(ctx) before aggregating,
    preserving today's exact include/exclude rules (healthy/degraded/empty included;
    unhealthy/unknown/unauthenticated excluded — discovery/middleware.go:185-188).
  • ListTools drives aggregator.AggregateCapabilities on demand (no per-session
    state held in the core); Lookup* resolve advertised names incl. BackendID without
    invoking the backend.
  • Nil-identity / anonymous semantics match ShouldAllowAnonymous /
    ErrNilCaller / ErrUnauthorizedCaller (session/types/session.go:182,224-232).
  • Close() is implemented and idempotent (releases core-held resources).
  • No mcp-go types cross the VMCP boundary (anti-pattern Implement secret injection #5); identity is never
    logged (it redacts Token/UpstreamTokens).
  • The admission seam is not wired here (it lands in P1.5 Core admission seam (bounded rewrite) #5438).
  • PR is ≤ 400 LOC and ≤ 10 files changed (excluding tests/docs/generated). If it
    exceeds the limit, split via /split-pr into TASK-104a / TASK-104b.
  • server.New signature and observable behavior unchanged.
  • All tests pass (task test); lint clean (task lint-fix).
  • Code reviewed and approved.

Technical Approach

Recommended Implementation

Add pkg/vmcp/core.go (or fold into vmcp.go) holding *coreVMCP. New assembles the
collaborators by relocating the wiring from server.New (server.go:330-465) — do
not redesign it:

  • Telemetry backend-client decoration — when cfg.TelemetryProvider != nil, decorate
    the backend client via monitorBackends (server.go:353-367) before building the
    workflow engine, so workflow backend calls are instrumented.
  • Workflow auditor — when cfg.AuditConfig != nil, validate and build the
    audit.WorkflowAuditor (server.go:370-381).
  • State store + workflow enginecomposer.NewInMemoryStateStore(5*time.Minute, 1*time.Hour) + composer.NewWorkflowEngine(rt, backendClient, elicitationHandler, stateStore, workflowAuditor, nil) (server.go:386-387). The elicitationHandler is
    built from the domain-typed ElicitationRequester delivered by P1.3 Domain-typed ElicitationRequester (bounded rewrite) #5436 — no mcp-go
    types here.
  • Per-session composer factory — relocate the sessionComposerFactory closure that
    builds a per-session composer.Composer bound to the session's routing table via
    router.NewSessionRouter(sessionRT) (server.go:393-398). This is the decoupled
    composite-routing pattern the refactor generalizes (it already removes composite tools'
    dependency on context-injected DiscoveredCapabilities).
  • Workflow validationvalidateWorkflows(workflowComposer, workflowDefs) fail-fast
    (server.go:402; impl server.go:1211).
  • Health filtering — store the injected health.StatusProvider; each ListTools/
    Lookup* call lists backends from the registry and runs filterHealthyBackends
    (copied from discovery/middleware.go:157) before calling
    aggregator.AggregateCapabilities. This is an intentional, temporary duplication
    (C2): the discovery middleware keeps its own copy on the legacy server.New path
    (guarded to legacy in P2.4 Replace discovery-into-context with direct VMCP calls #5442) until that path is removed in Phase 3 (P3.2 Reduce server.New body to the wrapper #5445) — call
    it out for reviewers so it is not mistaken for accidental duplication. A nil
    StatusProvider ⇒ no filtering (all backends included).

CallTool/ReadResource/GetPrompt route through router.Router + the composer for
composite tools. Lookup* resolve an advertised name/URI to the capability (incl.
BackendID) without invoking, returning an error for an unknown/unadvertised name — the
validation seam for the call path. Apply nil/anonymous semantics matching
ShouldAllowAnonymous / ErrNilCaller / ErrUnauthorizedCaller. Copy args/meta maps
before mutating (maps.Clone).

Patterns & Frameworks

Code Pointers

  • pkg/vmcp/core.go (new, or vmcp.go) — *coreVMCP + New(cfg *Config) (VMCP, error)
    (architecture.md "New (core) wiring relocated", lines ~278-284).
  • pkg/vmcp/server/server.go:330-465 - the domain wiring being relocated:
    monitorBackends (353-367), workflow auditor (370-381), state store +
    NewWorkflowEngine (386-387), sessionComposerFactory (393-398),
    validateWorkflows (402; impl 1211).
  • pkg/vmcp/server/server.go:393 - sessionComposerFactory /
    router.NewSessionRouter(sessionRT) — the decoupled per-session composite-routing
    pattern to generalize into the core.
  • pkg/vmcp/server/server.go:301 - server.New (7-param signature; stays stable,
    not modified in this task).
  • pkg/vmcp/aggregator/aggregator.go:63,78 - AggregateCapabilities /
    ProcessPreQueriedCapabilities — the on-demand aggregation ListTools drives.
  • pkg/vmcp/discovery/middleware.go:157 - filterHealthyBackends relocated into the core;
    include/exclude rules at 185-188 (R5/R7). Already takes a health.StatusProvider.
  • pkg/vmcp/session/types/session.go:182,224-232 - ShouldAllowAnonymous,
    ErrNilCaller / ErrUnauthorizedCaller — the nil-identity / anonymous semantics the
    core reproduces.
  • pkg/vmcp/types.go:130 - BackendTarget.GetBackendCapabilityName (renamed/prefixed
    name resolution used by LookupTool/CallTool).

Component Interfaces

The core implements the VMCP contract from #5434 (architecture.md "API Contracts").
New accepts the relocated collaborators on the core Config; the
health.StatusProvider is injected for internal health filtering. Shape only — not the
full implementation:

// New constructs the core VMCP by relocating server.New's domain wiring.
// cfg carries the relocated collaborators: aggregator, router.Router,
// vmcp.BackendRegistry, vmcp.BackendClient, the domain ElicitationRequester,
// workflowDefs, and the injected health.StatusProvider (core filters, Serve caches).
// Admission is NOT wired here (#5438).
func New(cfg *Config) (VMCP, error)

type coreVMCP struct {
    aggregator      aggregator.Aggregator
    router          router.Router
    backendRegistry vmcp.BackendRegistry
    backendClient   vmcp.BackendClient
    composerFactory func(sessionRT *vmcp.RoutingTable, sessionTools []vmcp.Tool) composer.Composer
    workflowEngine  composer.Composer
    workflowDefs    map[string]*composer.WorkflowDefinition
    health          health.StatusProvider // injected; core runs filterHealthyBackends
    // ... (no admission seam yet — #5438)
}

// ListTools health-filters the registry, then aggregates on demand. Stateless
// w.r.t. sessions. identity is explicit and never read from context.
func (c *coreVMCP) ListTools(ctx context.Context, identity *auth.Identity) ([]Tool, error)

// CallTool routes via router.Router + composer; nil/anonymous semantics match
// ShouldAllowAnonymous / ErrNilCaller / ErrUnauthorizedCaller. args/meta copied
// before mutating.
func (c *coreVMCP) CallTool(ctx context.Context, identity *auth.Identity, name string,
    args map[string]any, meta map[string]any) (*ToolCallResult, error)

// LookupTool resolves an advertised name (incl. BackendID) without invoking;
// errors on an unknown/unadvertised name. Same health/advertising view as ListTools.
func (c *coreVMCP) LookupTool(ctx context.Context, identity *auth.Identity, name string) (*Tool, error)

// Close releases core-held resources. Idempotent.
func (c *coreVMCP) Close() error

Testing Strategy

Unit Tests (stdlib testing.T + testify; mocks via gomock + task gen; no Ginkgo, R8)

  • New returns a working VMCP and errors loudly on nil required collaborators.
  • ListTools/ListResources/ListPrompts aggregate on demand against a mock backend
    client/registry and return advertised capabilities with BackendID populated.
  • Health filtering: backends reported unhealthy/unknown/unauthenticated by the injected
    health.StatusProvider are excluded; healthy/degraded/empty included (parity with
    discovery/middleware.go:185-188).
  • LookupTool/LookupResource/LookupPrompt resolve renamed/prefixed names back via
    GetBackendCapabilityName; return an error for an unknown/unadvertised name.
  • CallTool/ReadResource/GetPrompt route to the correct backend via router.Router;
    composite tools execute through the per-session composer.
  • Close() is idempotent (second call is a no-op, no panic).
  • args/meta maps passed by the caller are not mutated (copy-before-mutate).

Integration / Behavioral Parity Tests

  • Capability set produced by the core's ListTools is consistent with the
    session-factory path (aggregator.ProcessPreQueriedCapabilities) — same aggregator,
    same filter, same registry — confirming no double-aggregation / no parity drift.
  • server.New wrapper behavior unchanged (the core is additive and not yet wired in).

Edge Cases

  • Nil identity → anonymous semantics per ShouldAllowAnonymous.
  • Bound-session caller mismatch / nil caller surfaces ErrUnauthorizedCaller /
    ErrNilCaller semantics consistent with the session layer.
  • No mcp-go type appears in any VMCP method signature or returned value.
  • Identity is never written to logs (verify Token/UpstreamTokens not logged).

Out of Scope

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestrefactorvmcpVirtual 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