Skip to content

P3.1 deriveCoreConfig/deriveServerConfig config split #5444

@tgrunnagle

Description

@tgrunnagle

Description

Introduce deriveCoreConfig and deriveServerConfig to split the in-memory-only
server.Config into the core Config (collaborators + workflowDefs + Authz for the
admission seam + the cross-cutting TelemetryProvider/AuditConfig) and a transport-only
ServerConfig. This is the first half of RFC Phase 3 (the config decomposition); it sets up
the data shapes that #5445 then feeds into the Serve(ctx, New(deriveCoreConfig(cfg, …)), deriveServerConfig(cfg)) wrapper. ServerConfig carries no AuthzMiddleware field
(authz now lives in the core admission seam from Phase 1). The AuthzMiddleware field on
server.Config itself is kept (not removed in this epic): cli/serve.go sets it at
serve.go:~374, and the "cli/serve.go unchanged" constraint forbids touching that
assignment — so the field stays as a vestigial input the new path ignores.
deriveServerConfig simply omits it; the core admission seam derives its authorizer from
the Authz config instead. (The now-redundant HTTP authz/annotation blocks are deleted
as cleanup in #5445 once server.New routes through Serve — but the field itself
remains, so cli/serve.go needs no change.)

Context

By the time this task runs, Phase 1 has produced the VMCP core (New(cfg) -> VMCP with the
admission and elicitation seams) and Phase 2 has re-homed every transport concern under
Serve(ctx, VMCP, *ServerConfig) -> *Server — but server.New does not yet call Serve.
This task adds the two derivation helpers that map the existing single server.Config onto
the core/transport pair, without changing server.New's body or observable behavior yet
(that is #5445/P3.2).

The split is not a clean partition (Risk R3, architecture.md lines 483-487): 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 owns the health monitor — so they
are passed to both deriveCoreConfig and deriveServerConfig. The field-by-field
placement is given in the research.md Config table (lines 144-167) and architecture.md
"ServerConfig" (lines 243-274). This is an in-memory-only change: vmcpconfig.Config, the
CRD/YAML model, and the wire/storage format are all unchanged inputs to New.

Parent Story: #5432
Dependencies: #5439, #5440, #5441, #5442, #5443 (all of Phase 2 complete — the ServerConfig type and the relocated transport wiring must exist)
Blocks: #5445

Acceptance Criteria

  • deriveCoreConfig(cfg, …) and deriveServerConfig(cfg) produce, respectively, the core
    Config and the transport ServerConfig from the existing server.Config.
  • The core Config carries the collaborators (aggregator/router/backendClient/
    backendRegistry/discovery, relocated from server.New params), workflowDefs, and the
    Authz config for the admission seam.
  • The cross-cutting fields TelemetryProvider, AuditConfig, and the health view are
    populated on both the core Config and the ServerConfig (not a clean partition, R3).
  • All transport-only fields (Host, Port, EndpointPath, SessionTTL,
    AuthMiddleware, AuthInfoHandler, AuthServer, StatusReportingInterval, Watcher,
    StatusReporter, SessionFactory, SessionStorage, OptimizerFactory,
    OptimizerConfig) land on ServerConfig.
  • Health monitor (A2): the monitor is built at the composition root (P3.2 Reduce server.New body to the wrapper #5445) from
    HealthMonitorConfig; deriveServerConfig places the built *health.Monitor on
    ServerConfig.HealthMonitor (lifecycle) and deriveCoreConfig places the same instance's
    health.StatusProvider on the core Config (filtering). HealthMonitorConfig is consumed
    at the root, not carried verbatim onto ServerConfig.
  • ServerConfig carries no AuthzMiddleware field (authz lives in the core admission
    seam). The AuthzMiddleware field on server.Config is kept (not removed in this
    epic): cli/serve.go still sets it and must stay unchanged, so it remains a vestigial input
    the new path ignores (deriveServerConfig omits it; the core seam uses the Authz config).
    Only the redundant HTTP authz/annotation blocks are deleted later (P3.2 Reduce server.New body to the wrapper #5445).
  • No serialized/wire/CRD/YAML format change — the change is in-memory only;
    vmcpconfig.Config is unchanged.
  • PR is ≤ 400 LOC and ≤ 10 files changed (excluding tests/docs/generated)
  • 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 two unexported derivation helpers in pkg/vmcp/server (alongside New/Serve):

  • deriveServerConfig(cfg *Config) *ServerConfig — a pure projection of the transport-only
    fields out of the existing server.Config, plus the two cross-cutting fields. Per the
    go-style "Copy Before Mutating Caller Input" rule, treat cfg as read-only and construct a
    fresh ServerConfig; do not mutate cfg.
  • deriveCoreConfig(cfg *Config, …) — assembles the core Config from the cross-cutting
    fields on cfg (TelemetryProvider, AuditConfig, health view), the Authz config that
    feeds the admission seam, workflowDefs, and the collaborators that today arrive as
    separate server.New parameters (router, backendClient, discovery manager, backendRegistry,
    aggregator). The exact extra parameters mirror the New core constructor signature
    established in Phase 1 — pass the collaborators through rather than reaching back into
    cfg.

Keep the defaulting logic that server.New applies today (Host/EndpointPath/Name/
Version/SessionTTL at server.go:310-327) flowing into the derived configs so that derived
values are post-default — either derive after defaults are applied, or apply the same defaults
inside the helpers.

Do not remove the AuthzMiddleware field from server.Config — in this task or any
other. cli/serve.go (the composition root, serve.go:~374) assigns it, and the
"cli/serve.go unchanged" constraint forbids editing that assignment, so the field stays as a
vestigial input the new path ignores. deriveServerConfig simply does not propagate
AuthzMiddleware onto ServerConfig (which never had the field); the core admission seam
derives its authorizer from the Authz config instead. The two consumer blocks
(server.go:606-609, :611-617) still run on the legacy server.New path in this task
(#5441 left them inert only on the Serve path); they become dead and are deleted in
#5445 once server.New routes through Serve — the field itself remains.

For A2, the health monitor is built at the composition root (in #5445's wrapper, or a test
harness), so the derivers receive the built *health.Monitor and thread it: the *Monitor
onto ServerConfig.HealthMonitor (lifecycle, owned by Serve) and its StatusProvider onto
the core Config (filtering). The derivers do not call health.NewMonitor themselves.

This task only introduces the helpers and the field move; it does not yet rewire
server.New's body to call them through Serve (that is #5445). Wire the helpers so they
can be unit-tested in isolation against a representative server.Config.

Patterns & Frameworks

  • Decompose the top-level config at the composition root into small, focused config types —
    the core takes only what it needs, the transport takes only what it needs (counters
    anti-pattern Figure out ergonomics for exposing directories #6, "Configuration Object Passed Everywhere"). The cross-cutting overlap is the
    documented R3 exception.
  • Routing authz through the core admission seam (and deleting the dead HTTP authz/annotation
    blocks in P3.2 Reduce server.New body to the wrapper #5445 — the AuthzMiddleware field on server.Config is kept vestigial)
    avoids "middleware overuse" (anti-pattern Implement secret store #4) and keeps the SDK/transport boundary clean
    (anti-pattern Implement secret injection #5).
  • Treat cfg as read-only; build fresh derived structs (.claude/rules/go-style.md "Copy
    Before Mutating Caller Input").
  • Avoid parallel types that drift: ServerConfig already exists from P2.1 Serve skeleton + ServerConfig #5439; reuse it
    rather than defining a second transport config (.claude/rules/go-style.md "Interface
    Design").
  • stdlib testing.T + testify in pkg/vmcp/server; no Ginkgo. SPDX header on any new .go
    file.
  • Conventions: .claude/rules/go-style.md, .claude/rules/vmcp-anti-patterns.md,
    .claude/rules/security.md.

Code Pointers

  • pkg/vmcp/server/server.go:92-185 — the Config struct being split. Field-by-field
    placement is in research.md (lines 144-167) and architecture.md "ServerConfig"
    (lines 243-274).
  • pkg/vmcp/server/server.go:121-125AuthzMiddleware func(http.Handler) http.Handler;
    kept, not removed (in this task or any other) — cli/serve.go sets it and must stay
    unchanged, so it remains a vestigial input. deriveServerConfig omits it; the core seam
    uses the Authz config instead.
  • pkg/vmcp/server/server.go:606-609, :611-617 — the two s.config.AuthzMiddleware
    consumer blocks; left intact (inert on the Serve path via nil AuthzMiddleware, per
    P2.3 Move middleware chain under Serve; remove authz + annotation mw #5441) and physically removed in P3.2 Reduce server.New body to the wrapper #5445.
  • pkg/vmcp/server/server.go:136-147 — the cross-cutting fields TelemetryProvider and
    AuditConfig go to both derived configs. HealthMonitorConfig is consumed at the
    composition root (P3.2 Reduce server.New body to the wrapper #5445) to build the *health.Monitor; the built monitor (not the
    config) is then threaded — *MonitorServerConfig.HealthMonitor, StatusProvider
    core Config (A2).
  • pkg/vmcp/server/server.go:301-327server.New (7-param signature; stays stable). Its
    defaulting block (310-327) and its collaborator parameters (router, backendClient,
    discoveryMgr, backendRegistry, workflowDefs) are the inputs deriveCoreConfig assembles.
  • architecture.md "ServerConfig" (243-274) — the target ServerConfig shape and the core
    Config contents.
  • architecture.md "Cross-cutting config not a clean partition (R3)" (483-487) — why
    TelemetryProvider/AuditConfig/health go to both sides.
  • pkg/vmcp/server/server_test.go — existing pkg/vmcp/server unit-test patterns to mirror
    for the derivation-helper tests.

Component Interfaces

The shape, not the full implementation. ServerConfig is the type established in #5439
(architecture.md 250-267); the core Config is the Phase 1 core constructor input.

// deriveServerConfig projects the transport-only fields out of the in-memory server.Config,
// plus the two cross-cutting fields shared with the core (R3), plus the already-built health
// monitor (A2). cfg is treated as read-only. healthMon is built at the composition root.
func deriveServerConfig(cfg *Config, healthMon *health.Monitor) *ServerConfig {
    return &ServerConfig{
        Name: cfg.Name, Version: cfg.Version, GroupRef: cfg.GroupRef,
        Host: cfg.Host, Port: cfg.Port, EndpointPath: cfg.EndpointPath, SessionTTL: cfg.SessionTTL,
        AuthMiddleware:  cfg.AuthMiddleware,
        AuthInfoHandler: cfg.AuthInfoHandler,
        AuthServer:      cfg.AuthServer,
        HealthMonitor:           healthMon, // built at the composition root (A2); nil => disabled
        StatusReportingInterval: cfg.StatusReportingInterval,
        StatusReporter:          cfg.StatusReporter,
        Watcher:                 cfg.Watcher,
        SessionFactory:          cfg.SessionFactory,
        SessionStorage:          cfg.SessionStorage,
        OptimizerFactory:        cfg.OptimizerFactory,
        OptimizerConfig:         cfg.OptimizerConfig,
        // Cross-cutting (also on core Config) — R3, not a clean partition:
        TelemetryProvider: cfg.TelemetryProvider,
        AuditConfig:       cfg.AuditConfig,
    }
    // NOTE: ServerConfig has no AuthzMiddleware field — authz flows through the core admission
    // seam (Phase 1 / R1). The AuthzMiddleware field on server.Config is KEPT (vestigial;
    // cli/serve.go sets it and stays unchanged); only the dead HTTP blocks that read it are
    // deleted in #5445 (with the wrapper routing). deriveServerConfig simply omits it.
}

// deriveCoreConfig assembles the core Config from cfg's cross-cutting + admission inputs, the
// injected health StatusProvider (A2), and the collaborators that arrive as separate
// server.New parameters today.
func deriveCoreConfig(
    cfg *Config,
    rt router.Router,
    backendClient vmcp.BackendClient,
    discoveryMgr discovery.Manager,
    backendRegistry vmcp.BackendRegistry,
    workflowDefs map[string]*composer.WorkflowDefinition,
    healthProvider health.StatusProvider, // the same monitor built at the root (A2); nil => no filtering
) *coreConfig { // coreConfig = the Phase 1 core constructor input type
    // collaborators + workflowDefs + Authz (admission seam) + cross-cutting Telemetry/Audit +
    // HealthStatusProvider = healthProvider
}

Testing Strategy

Unit Tests

  • deriveServerConfig copies every transport-only field from a fully-populated
    server.Config onto the returned ServerConfig (table-style assertion over each field).
  • deriveServerConfig carries the cross-cutting TelemetryProvider and AuditConfig
    (and the built *health.Monitor) onto the ServerConfig, and does not project an
    AuthzMiddleware field — ServerConfig has none, and the server.Config field is
    retained (vestigial) and simply not projected.
  • deriveCoreConfig populates the core Config with the passed collaborators,
    workflowDefs, the Authz config, and the same cross-cutting fields.
  • Neither helper mutates the input cfg (assert cfg is unchanged after the call).

Integration / Behavioral Parity Tests

  • Existing pkg/vmcp/server tests stay green: because server.New is not yet rewired to
    call the helpers in this task, its observable behavior is unchanged (the parity gate over
    the server.New wrapper is exercised end-to-end in P3.2 Reduce server.New body to the wrapper #5445).

Edge Cases

  • Nil/zero cross-cutting fields (TelemetryProvider == nil, AuditConfig == nil,
    HealthMonitorConfig == nil) propagate as nil to both sides — derivation must not panic or
    substitute a default for a deliberately-disabled subsystem.
  • Default-applied scalars (Host/EndpointPath/Name/Version/SessionTTL) are
    reflected post-default in the derived ServerConfig (derive after defaults, or apply the
    same defaults in the helper).

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