From dcbd4db40999d9c3c66bffcaa9815f4be3d72eb3 Mon Sep 17 00:00:00 2001
From: Trey <tgrunnagle@gmail.com>
Date: Fri, 12 Jun 2026 11:32:40 -0700
Subject: [PATCH 1/4] Add vMCP core/server config derivation helpers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Phase 3.1 of the RFC THV-0076 config decomposition. The single in-memory
server.Config is the last god-config that couples the domain core to the
transport. Introduce two unexported helpers that map it onto the already-
existing core.Config (Phase 1) and transport ServerConfig (Phase 2):

- deriveServerConfig projects the transport-only fields plus the
  cross-cutting TelemetryProvider/AuditConfig (R3); the built health
  monitor (A2), shared backend registry, and assembled session-manager
  FactoryConfig are threaded in by the composition root since
  server.Config does not carry them. AuthzMiddleware is not projected —
  ServerConfig has no such field; authz moved to the core admission seam.
- deriveCoreConfig assembles core.Config from cfg's cross-cutting fields
  (ServerName uses the raw cfg.Name for authz parity) plus the
  collaborators passed through rather than reached out of cfg.

Both treat cfg as read-only (no defaulting write-back like server.New).
This only adds the helpers and their tests; #5445 rewires server.New to
call them through Serve. server.New and all serialized formats are
unchanged.

Closes #5444

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 pkg/vmcp/server/derive.go      | 127 +++++++++++++++
 pkg/vmcp/server/derive_test.go | 284 +++++++++++++++++++++++++++++++++
 2 files changed, 411 insertions(+)
 create mode 100644 pkg/vmcp/server/derive.go
 create mode 100644 pkg/vmcp/server/derive_test.go

diff --git a/pkg/vmcp/server/derive.go b/pkg/vmcp/server/derive.go
new file mode 100644
index 0000000000..074c31c5ff
--- /dev/null
+++ b/pkg/vmcp/server/derive.go
@@ -0,0 +1,127 @@
+// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package server
+
+import (
+	"cmp"
+
+	"github.com/stacklok/toolhive/pkg/authz"
+	"github.com/stacklok/toolhive/pkg/vmcp"
+	"github.com/stacklok/toolhive/pkg/vmcp/aggregator"
+	"github.com/stacklok/toolhive/pkg/vmcp/composer"
+	"github.com/stacklok/toolhive/pkg/vmcp/core"
+	"github.com/stacklok/toolhive/pkg/vmcp/health"
+	"github.com/stacklok/toolhive/pkg/vmcp/router"
+	"github.com/stacklok/toolhive/pkg/vmcp/server/sessionmanager"
+)
+
+// deriveServerConfig projects the transport-only configuration out of the legacy,
+// in-memory server.Config onto the ServerConfig that Serve consumes. It is the
+// transport half of the Phase 3 config decomposition (#5444); the composition root
+// (#5445) will call it when server.New is reduced to a wrapper over
+// Serve(ctx, core.New(deriveCoreConfig(cfg, …)), deriveServerConfig(cfg, …)).
+//
+// cfg is treated as read-only (go-style: copy before mutating caller input); a fresh
+// ServerConfig is returned. The same transport defaults server.New applies are applied
+// here via cmp.Or against the shared default* constants, so the returned ServerConfig
+// is post-default and self-contained. buildServeConfig re-applies the identical cmp.Or
+// downstream, which is idempotent — the default values live in one place (the
+// constants), so the two cannot drift.
+//
+// Three collaborators the transport needs but server.Config does not carry directly are
+// passed in by the composition root rather than reached out of cfg:
+//   - healthMon: the *health.Monitor built at the composition root (A2). Serve owns only
+//     its Start/Stop; the same instance's StatusProvider is injected into the core via
+//     deriveCoreConfig. Nil disables health monitoring.
+//   - backendRegistry: the shared backend registry (also passed to deriveCoreConfig);
+//     the Serve session layer enumerates it when opening a session's connections.
+//   - sessionManagerConfig: the pre-assembled *sessionmanager.FactoryConfig. The legacy
+//     SessionFactory/OptimizerFactory/OptimizerConfig fields fold into it (#5440), and its
+//     ComposerFactory closes over the core-owned router/backend client, so only the
+//     composition root can build it.
+//
+// AuthzMiddleware is intentionally NOT projected: ServerConfig has no such field —
+// authorization moved to the core admission seam (#5438). The AuthzMiddleware field on
+// server.Config is kept as a vestigial input the Serve path ignores (cli/serve.go still
+// sets it and must stay unchanged); only the dead HTTP authz/annotation blocks that read
+// it are removed later (#5445).
+func deriveServerConfig(
+	cfg *Config,
+	healthMon *health.Monitor,
+	backendRegistry vmcp.BackendRegistry,
+	sessionManagerConfig *sessionmanager.FactoryConfig,
+) *ServerConfig {
+	return &ServerConfig{
+		Name:                    cmp.Or(cfg.Name, defaultServerName),
+		Version:                 cmp.Or(cfg.Version, defaultServerVersion),
+		GroupRef:                cfg.GroupRef,
+		Host:                    cmp.Or(cfg.Host, defaultHost),
+		Port:                    cfg.Port, // 0 means "OS-assigned"; intentionally not defaulted.
+		EndpointPath:            cmp.Or(cfg.EndpointPath, defaultEndpointPath),
+		SessionTTL:              cmp.Or(cfg.SessionTTL, defaultSessionTTL),
+		AuthMiddleware:          cfg.AuthMiddleware,
+		RateLimitMiddleware:     cfg.RateLimitMiddleware,
+		AuthInfoHandler:         cfg.AuthInfoHandler,
+		AuthServer:              cfg.AuthServer,
+		HealthMonitor:           healthMon,
+		StatusReportingInterval: cfg.StatusReportingInterval,
+		StatusReporter:          cfg.StatusReporter,
+		Watcher:                 cfg.Watcher,
+		BackendRegistry:         backendRegistry,
+		SessionStorage:          cfg.SessionStorage,
+		SessionManagerConfig:    sessionManagerConfig,
+		// Cross-cutting (also on core.Config) — R3, not a clean partition:
+		TelemetryProvider: cfg.TelemetryProvider,
+		AuditConfig:       cfg.AuditConfig,
+	}
+}
+
+// deriveCoreConfig assembles the core.Config that core.New consumes from the legacy
+// server.Config plus the collaborators the composition root builds. It is the core half
+// of the Phase 3 config decomposition (#5444); #5445 will call it inside the reduced
+// server.New as core.New(deriveCoreConfig(cfg, …)).
+//
+// cfg is treated as read-only; only its cross-cutting fields are read:
+//   - ServerName = cfg.Name — the Cedar resource entity name, matching the serverName the
+//     legacy HTTP authz path threads through (cli/serve.go passes vmcpCfg.Name). The raw
+//     name is used (not the transport default) so authorization keys on the real
+//     VirtualMCPServer name rather than the synthetic "toolhive-vmcp" fallback.
+//   - TelemetryProvider / AuditConfig — the cross-cutting fields shared with the transport
+//     (R3, not a clean partition); deriveServerConfig copies the same two.
+//
+// Everything else is a collaborator passed through rather than reached out of cfg: the
+// aggregator, router, backend client, backend registry, and workflow definitions that
+// arrive as separate server.New parameters today; the authz config that feeds the
+// admission seam (server.Config carries only the pre-built AuthzMiddleware, never the
+// authz.Config, so it cannot come from cfg); the elicitation requester; and the health
+// StatusProvider — the read-only view of the same *health.Monitor deriveServerConfig
+// places on ServerConfig.HealthMonitor (A2). A nil healthProvider means no health
+// filtering; a nil authzCfg means allow-all (matching today's AuthzMiddleware != nil
+// guard).
+func deriveCoreConfig(
+	cfg *Config,
+	agg aggregator.Aggregator,
+	rt router.Router,
+	backendClient vmcp.BackendClient,
+	backendRegistry vmcp.BackendRegistry,
+	workflowDefs map[string]*composer.WorkflowDefinition,
+	authzCfg *authz.Config,
+	elicitation vmcp.ElicitationRequester,
+	healthProvider health.StatusProvider,
+) *core.Config {
+	return &core.Config{
+		Aggregator:      agg,
+		Router:          rt,
+		BackendRegistry: backendRegistry,
+		BackendClient:   backendClient,
+		WorkflowDefs:    workflowDefs,
+		Authz:           authzCfg,
+		ServerName:      cfg.Name,
+		// Cross-cutting (also on ServerConfig) — R3, not a clean partition:
+		TelemetryProvider:    cfg.TelemetryProvider,
+		AuditConfig:          cfg.AuditConfig,
+		HealthStatusProvider: healthProvider,
+		Elicitation:          elicitation,
+	}
+}
diff --git a/pkg/vmcp/server/derive_test.go b/pkg/vmcp/server/derive_test.go
new file mode 100644
index 0000000000..bc0f5c32ae
--- /dev/null
+++ b/pkg/vmcp/server/derive_test.go
@@ -0,0 +1,284 @@
+// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package server
+
+import (
+	"net/http"
+	"reflect"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"go.uber.org/mock/gomock"
+
+	"github.com/stacklok/toolhive/pkg/audit"
+	asrunner "github.com/stacklok/toolhive/pkg/authserver/runner"
+	"github.com/stacklok/toolhive/pkg/authz"
+	"github.com/stacklok/toolhive/pkg/telemetry"
+	"github.com/stacklok/toolhive/pkg/vmcp"
+	aggmocks "github.com/stacklok/toolhive/pkg/vmcp/aggregator/mocks"
+	"github.com/stacklok/toolhive/pkg/vmcp/composer"
+	vmcpconfig "github.com/stacklok/toolhive/pkg/vmcp/config"
+	"github.com/stacklok/toolhive/pkg/vmcp/health"
+	vmcpmocks "github.com/stacklok/toolhive/pkg/vmcp/mocks"
+	routermocks "github.com/stacklok/toolhive/pkg/vmcp/router/mocks"
+)
+
+// stubStatusProvider is a non-nil health.StatusProvider for derivation tests; its
+// behavior is not exercised — only its identity is asserted (it is passed through
+// onto core.Config.HealthStatusProvider).
+type stubStatusProvider struct{}
+
+func (*stubStatusProvider) QueryBackendStatus(string) (vmcp.BackendHealthStatus, bool) {
+	return vmcp.BackendHealthy, true
+}
+
+// populatedLegacyConfig returns a server.Config with every field deriveServerConfig
+// reads set to a distinctive non-zero value, so a dropped or wrong-source mapping
+// surfaces in the assertions below. AuthzMiddleware is set too, to prove derivation
+// retains it on the legacy Config (vestigial) while never projecting it.
+func populatedLegacyConfig() *Config {
+	passthrough := func(h http.Handler) http.Handler { return h }
+	return &Config{
+		Name:                    "vmcp-name",
+		Version:                 "9.9.9",
+		GroupRef:                "grp",
+		Host:                    "0.0.0.0",
+		Port:                    7777,
+		EndpointPath:            "/custom",
+		SessionTTL:              17 * time.Minute,
+		AuthMiddleware:          passthrough,
+		AuthzMiddleware:         passthrough,
+		AuthInfoHandler:         http.NewServeMux(),
+		RateLimitMiddleware:     passthrough,
+		AuthServer:              &asrunner.EmbeddedAuthServer{},
+		TelemetryProvider:       &telemetry.Provider{},
+		AuditConfig:             &audit.Config{},
+		StatusReportingInterval: 11 * time.Second,
+		Watcher:                 stubWatcher{},
+		StatusReporter:          stubServeReporter{},
+		SessionStorage:          &vmcpconfig.SessionStorageConfig{},
+	}
+}
+
+func TestDeriveServerConfigProjectsTransportFields(t *testing.T) {
+	t.Parallel()
+
+	cfg := populatedLegacyConfig()
+	healthMon := &health.Monitor{}
+	registry := vmcp.NewImmutableRegistry([]vmcp.Backend{})
+	smCfg := testMinimalSessionManagerConfig()
+
+	got := deriveServerConfig(cfg, healthMon, registry, smCfg)
+
+	// Scalars projected verbatim (cfg's values are all non-default, so cmp.Or returns them).
+	assert.Equal(t, "vmcp-name", got.Name)
+	assert.Equal(t, "9.9.9", got.Version)
+	assert.Equal(t, "grp", got.GroupRef)
+	assert.Equal(t, "0.0.0.0", got.Host)
+	assert.Equal(t, 7777, got.Port)
+	assert.Equal(t, "/custom", got.EndpointPath)
+	assert.Equal(t, 17*time.Minute, got.SessionTTL)
+	assert.Equal(t, 11*time.Second, got.StatusReportingInterval)
+
+	// Func/handler/pointer fields projected by reference.
+	assert.NotNil(t, got.AuthMiddleware)
+	assert.NotNil(t, got.RateLimitMiddleware)
+	assert.Same(t, cfg.AuthInfoHandler, got.AuthInfoHandler)
+	assert.Same(t, cfg.AuthServer, got.AuthServer)
+	assert.Same(t, cfg.SessionStorage, got.SessionStorage)
+	assert.Equal(t, cfg.Watcher, got.Watcher)
+	assert.Equal(t, cfg.StatusReporter, got.StatusReporter)
+
+	// Cross-cutting fields shared with the core (R3).
+	assert.Same(t, cfg.TelemetryProvider, got.TelemetryProvider)
+	assert.Same(t, cfg.AuditConfig, got.AuditConfig)
+
+	// Collaborators passed in (not on server.Config) — threaded through, not from cfg.
+	assert.Same(t, healthMon, got.HealthMonitor)
+	assert.Same(t, registry, got.BackendRegistry)
+	assert.Same(t, smCfg, got.SessionManagerConfig)
+}
+
+func TestDeriveServerConfigAppliesTransportDefaults(t *testing.T) {
+	t.Parallel()
+
+	// Empty transport scalars exercise every default; Port stays zero (OS-assigned).
+	got := deriveServerConfig(&Config{}, nil, nil, nil)
+
+	assert.Equal(t, defaultServerName, got.Name)
+	assert.Equal(t, defaultServerVersion, got.Version)
+	assert.Equal(t, defaultHost, got.Host)
+	assert.Equal(t, defaultEndpointPath, got.EndpointPath)
+	assert.Equal(t, defaultSessionTTL, got.SessionTTL)
+	assert.Equal(t, 0, got.Port)
+}
+
+func TestDeriveServerConfigPropagatesNilCrossCutting(t *testing.T) {
+	t.Parallel()
+
+	// A deliberately-disabled subsystem (nil provider/config/monitor) must stay nil —
+	// derivation must not substitute a default or panic.
+	got := deriveServerConfig(&Config{}, nil, nil, nil)
+
+	assert.Nil(t, got.TelemetryProvider)
+	assert.Nil(t, got.AuditConfig)
+	assert.Nil(t, got.HealthMonitor)
+	assert.Nil(t, got.BackendRegistry)
+	assert.Nil(t, got.SessionManagerConfig)
+}
+
+// TestDeriveServerConfigMapsAllFields guards deriveServerConfig against silent drift:
+// with every readable source field non-zero and every collaborator param non-nil, every
+// ServerConfig field must be populated. If a future ServerConfig field is added and
+// deriveServerConfig forgets it, this fails (the field is zero). Add a deliberately-
+// unmapped field to the skip set with a reason, mirroring the doc comment.
+func TestDeriveServerConfigMapsAllFields(t *testing.T) {
+	t.Parallel()
+
+	got := deriveServerConfig(
+		populatedLegacyConfig(),
+		&health.Monitor{},
+		vmcp.NewImmutableRegistry([]vmcp.Backend{}),
+		testMinimalSessionManagerConfig(),
+	)
+
+	assertAllFieldsPopulated(t, *got, "ServerConfig", nil)
+}
+
+func TestDeriveCoreConfigAssemblesCollaborators(t *testing.T) {
+	t.Parallel()
+
+	ctrl := gomock.NewController(t)
+	cfg := &Config{
+		Name:              "core-name",
+		TelemetryProvider: &telemetry.Provider{},
+		AuditConfig:       &audit.Config{},
+	}
+	agg := aggmocks.NewMockAggregator(ctrl)
+	rt := routermocks.NewMockRouter(ctrl)
+	backendClient := vmcpmocks.NewMockBackendClient(ctrl)
+	registry := vmcpmocks.NewMockBackendRegistry(ctrl)
+	workflowDefs := map[string]*composer.WorkflowDefinition{"wf": {}}
+	authzCfg := &authz.Config{}
+	elicitation := vmcpmocks.NewMockElicitationRequester(ctrl)
+	healthProvider := &stubStatusProvider{}
+
+	got := deriveCoreConfig(cfg, agg, rt, backendClient, registry, workflowDefs, authzCfg, elicitation, healthProvider)
+
+	// Collaborators passed through by identity.
+	assert.Same(t, agg, got.Aggregator)
+	assert.Same(t, rt, got.Router)
+	assert.Same(t, backendClient, got.BackendClient)
+	assert.Same(t, registry, got.BackendRegistry)
+	assert.Same(t, authzCfg, got.Authz)
+	assert.Same(t, elicitation, got.Elicitation)
+	assert.Same(t, healthProvider, got.HealthStatusProvider)
+	assert.Equal(t, workflowDefs, got.WorkflowDefs)
+
+	// ServerName uses the raw cfg.Name for authz parity (no transport default applied).
+	assert.Equal(t, "core-name", got.ServerName)
+
+	// Cross-cutting fields shared with the transport (R3).
+	assert.Same(t, cfg.TelemetryProvider, got.TelemetryProvider)
+	assert.Same(t, cfg.AuditConfig, got.AuditConfig)
+}
+
+func TestDeriveCoreConfigUsesRawServerNameAndNilDefaults(t *testing.T) {
+	t.Parallel()
+
+	ctrl := gomock.NewController(t)
+	// Empty Name must NOT be defaulted (authz keys on the real VirtualMCPServer name),
+	// and nil admission/health inputs must propagate as nil (allow-all / no filtering).
+	got := deriveCoreConfig(
+		&Config{},
+		aggmocks.NewMockAggregator(ctrl),
+		routermocks.NewMockRouter(ctrl),
+		vmcpmocks.NewMockBackendClient(ctrl),
+		vmcpmocks.NewMockBackendRegistry(ctrl),
+		nil, // workflowDefs
+		nil, // authzCfg
+		nil, // elicitation
+		nil, // healthProvider
+	)
+
+	assert.Empty(t, got.ServerName)
+	assert.Nil(t, got.Authz)
+	assert.Nil(t, got.Elicitation)
+	assert.Nil(t, got.HealthStatusProvider)
+	assert.Nil(t, got.WorkflowDefs)
+}
+
+// TestDeriveCoreConfigMapsAllFields guards deriveCoreConfig against silent drift: with
+// every cross-cutting source field non-zero and every collaborator non-nil, every
+// core.Config field must be populated.
+func TestDeriveCoreConfigMapsAllFields(t *testing.T) {
+	t.Parallel()
+
+	ctrl := gomock.NewController(t)
+	cfg := &Config{
+		Name:              "core-name",
+		TelemetryProvider: &telemetry.Provider{},
+		AuditConfig:       &audit.Config{},
+	}
+
+	got := deriveCoreConfig(
+		cfg,
+		aggmocks.NewMockAggregator(ctrl),
+		routermocks.NewMockRouter(ctrl),
+		vmcpmocks.NewMockBackendClient(ctrl),
+		vmcpmocks.NewMockBackendRegistry(ctrl),
+		map[string]*composer.WorkflowDefinition{"wf": {}},
+		&authz.Config{},
+		vmcpmocks.NewMockElicitationRequester(ctrl),
+		&stubStatusProvider{},
+	)
+
+	assertAllFieldsPopulated(t, *got, "core.Config", nil)
+}
+
+// TestDeriveConfigsTreatInputAsReadOnly verifies neither helper mutates cfg. server.New
+// applies its defaults by writing back onto cfg; the derivation helpers must not, so a
+// caller's Config is unchanged after derivation (go-style: copy before mutating input).
+func TestDeriveConfigsTreatInputAsReadOnly(t *testing.T) {
+	t.Parallel()
+
+	ctrl := gomock.NewController(t)
+	authzFn := func(h http.Handler) http.Handler { return h }
+	cfg := &Config{AuthzMiddleware: authzFn} // empty scalars: defaulting would mutate them.
+
+	_ = deriveServerConfig(cfg, nil, nil, nil)
+	_ = deriveCoreConfig(
+		cfg,
+		aggmocks.NewMockAggregator(ctrl),
+		routermocks.NewMockRouter(ctrl),
+		vmcpmocks.NewMockBackendClient(ctrl),
+		vmcpmocks.NewMockBackendRegistry(ctrl),
+		nil, nil, nil, nil,
+	)
+
+	// Transport defaults were NOT written back onto the caller's Config.
+	assert.Empty(t, cfg.Name)
+	assert.Empty(t, cfg.Version)
+	assert.Empty(t, cfg.Host)
+	assert.Empty(t, cfg.EndpointPath)
+	assert.Zero(t, cfg.SessionTTL)
+	// The vestigial AuthzMiddleware field is retained on the legacy Config (never cleared).
+	assert.NotNil(t, cfg.AuthzMiddleware)
+}
+
+// assertAllFieldsPopulated asserts every field of the struct value v is non-zero,
+// skipping any name in skip. typeName labels failures.
+func assertAllFieldsPopulated(t *testing.T, v any, typeName string, skip map[string]struct{}) {
+	t.Helper()
+	rv := reflect.ValueOf(v)
+	rt := rv.Type()
+	for i := range rt.NumField() {
+		name := rt.Field(i).Name
+		if _, skipped := skip[name]; skipped {
+			continue
+		}
+		assert.Falsef(t, rv.Field(i).IsZero(), "%s.%s was not populated by derivation", typeName, name)
+	}
+}

From 6d529d754f4b6e17da9c225040989f94a7ec898f Mon Sep 17 00:00:00 2001
From: Trey <tgrunnagle@gmail.com>
Date: Mon, 15 Jun 2026 10:04:31 -0700
Subject: [PATCH 2/4] Resolve vMCP transport defaults at the edge

Addresses PR review (Option 1): the cmp.Or transport defaults previously
lived in three projection functions (server.New, buildServeConfig,
deriveServerConfig) with a duplicated "which fields" list, and server.New
applied them by mutating the caller's Config in place.

Consolidate to a single resolver, WithDefaults, and resolve once at the
composition root (cli/serve.go):

- WithDefaults is now the only place the transport default list lives.
- deriveServerConfig and buildServeConfig become pure pass-through.
- server.New calls WithDefaults on a COPY, so it no longer mutates the
  caller's Config. That non-mutation lets #5445 hand the raw, un-defaulted
  cfg.Name to the core for Cedar authz parity (it removes the carry-forward
  gotcha that #5445 would otherwise have to work around). server.New keeps
  defaulting defensively until #5445 reduces it to a Serve wrapper.

Serve is now a pure pass-through, so its test fixtures resolve SessionTTL
explicitly (a zero TTL fails session-storage construction). Defaulting
behavior is covered by TestWithDefaults; the obsolete
TestServeAppliesTransportDefaults / TestDeriveServerConfigAppliesTransportDefaults
are removed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 pkg/vmcp/cli/serve.go                 |  9 +++-
 pkg/vmcp/server/derive.go             | 23 ++++-----
 pkg/vmcp/server/derive_test.go        | 28 ++++-------
 pkg/vmcp/server/serve.go              | 21 ++++----
 pkg/vmcp/server/serve_session_test.go |  6 +++
 pkg/vmcp/server/serve_test.go         | 54 ++++++++------------
 pkg/vmcp/server/server.go             | 50 ++++++++++++-------
 pkg/vmcp/server/server_test.go        | 72 +++++++++++++++++++++++++++
 8 files changed, 168 insertions(+), 95 deletions(-)

diff --git a/pkg/vmcp/cli/serve.go b/pkg/vmcp/cli/serve.go
index c26061c974..0a7cdcb032 100644
--- a/pkg/vmcp/cli/serve.go
+++ b/pkg/vmcp/cli/serve.go
@@ -398,7 +398,12 @@ func Serve(ctx context.Context, cfg ServeConfig) error {
 		}()
 	}
 
-	serverCfg := &vmcpserver.Config{
+	// Resolve transport defaults once here at the composition root (Option 1): the
+	// vMCP config edge is the single place flags/CRD/YAML become a fully-resolved
+	// Config, so server.New, Serve, and the derive* helpers downstream are pure
+	// pass-through. WithDefaults fills any unset Host/EndpointPath/SessionTTL/Name/
+	// Version (EndpointPath in particular is never set by the CLI).
+	serverCfg := vmcpserver.WithDefaults(&vmcpserver.Config{
 		Name:                    vmcpCfg.Name,
 		Version:                 versions.Version,
 		GroupRef:                vmcpCfg.Group,
@@ -419,7 +424,7 @@ func Serve(ctx context.Context, cfg ServeConfig) error {
 		OptimizerConfig:         optCfg,
 		SessionFactory:          sessionFactory,
 		SessionStorage:          vmcpCfg.SessionStorage,
-	}
+	})
 
 	// Assign Watcher only when backendWatcher is non-nil. A typed nil
 	// *k8s.BackendWatcher assigned to the Watcher interface produces a
diff --git a/pkg/vmcp/server/derive.go b/pkg/vmcp/server/derive.go
index 074c31c5ff..5e70108020 100644
--- a/pkg/vmcp/server/derive.go
+++ b/pkg/vmcp/server/derive.go
@@ -4,8 +4,6 @@
 package server
 
 import (
-	"cmp"
-
 	"github.com/stacklok/toolhive/pkg/authz"
 	"github.com/stacklok/toolhive/pkg/vmcp"
 	"github.com/stacklok/toolhive/pkg/vmcp/aggregator"
@@ -23,11 +21,10 @@ import (
 // Serve(ctx, core.New(deriveCoreConfig(cfg, …)), deriveServerConfig(cfg, …)).
 //
 // cfg is treated as read-only (go-style: copy before mutating caller input); a fresh
-// ServerConfig is returned. The same transport defaults server.New applies are applied
-// here via cmp.Or against the shared default* constants, so the returned ServerConfig
-// is post-default and self-contained. buildServeConfig re-applies the identical cmp.Or
-// downstream, which is idempotent — the default values live in one place (the
-// constants), so the two cannot drift.
+// ServerConfig is returned. This is a pure projection: transport defaults are resolved
+// once at the composition root via WithDefaults (Option 1), so cfg already holds resolved
+// values and deriveServerConfig applies no defaulting of its own. Port 0 still means
+// "OS-assigned".
 //
 // Three collaborators the transport needs but server.Config does not carry directly are
 // passed in by the composition root rather than reached out of cfg:
@@ -53,13 +50,13 @@ func deriveServerConfig(
 	sessionManagerConfig *sessionmanager.FactoryConfig,
 ) *ServerConfig {
 	return &ServerConfig{
-		Name:                    cmp.Or(cfg.Name, defaultServerName),
-		Version:                 cmp.Or(cfg.Version, defaultServerVersion),
+		Name:                    cfg.Name,
+		Version:                 cfg.Version,
 		GroupRef:                cfg.GroupRef,
-		Host:                    cmp.Or(cfg.Host, defaultHost),
-		Port:                    cfg.Port, // 0 means "OS-assigned"; intentionally not defaulted.
-		EndpointPath:            cmp.Or(cfg.EndpointPath, defaultEndpointPath),
-		SessionTTL:              cmp.Or(cfg.SessionTTL, defaultSessionTTL),
+		Host:                    cfg.Host,
+		Port:                    cfg.Port, // 0 means "OS-assigned".
+		EndpointPath:            cfg.EndpointPath,
+		SessionTTL:              cfg.SessionTTL,
 		AuthMiddleware:          cfg.AuthMiddleware,
 		RateLimitMiddleware:     cfg.RateLimitMiddleware,
 		AuthInfoHandler:         cfg.AuthInfoHandler,
diff --git a/pkg/vmcp/server/derive_test.go b/pkg/vmcp/server/derive_test.go
index bc0f5c32ae..6bdd660d99 100644
--- a/pkg/vmcp/server/derive_test.go
+++ b/pkg/vmcp/server/derive_test.go
@@ -101,19 +101,9 @@ func TestDeriveServerConfigProjectsTransportFields(t *testing.T) {
 	assert.Same(t, smCfg, got.SessionManagerConfig)
 }
 
-func TestDeriveServerConfigAppliesTransportDefaults(t *testing.T) {
-	t.Parallel()
-
-	// Empty transport scalars exercise every default; Port stays zero (OS-assigned).
-	got := deriveServerConfig(&Config{}, nil, nil, nil)
-
-	assert.Equal(t, defaultServerName, got.Name)
-	assert.Equal(t, defaultServerVersion, got.Version)
-	assert.Equal(t, defaultHost, got.Host)
-	assert.Equal(t, defaultEndpointPath, got.EndpointPath)
-	assert.Equal(t, defaultSessionTTL, got.SessionTTL)
-	assert.Equal(t, 0, got.Port)
-}
+// deriveServerConfig no longer applies transport defaults — it is a pure projection now
+// that defaulting is resolved once at the composition root (Option 1). The WithDefaults
+// resolver carries that behavior and is covered by TestWithDefaults (server_test.go).
 
 func TestDeriveServerConfigPropagatesNilCrossCutting(t *testing.T) {
 	t.Parallel()
@@ -131,9 +121,11 @@ func TestDeriveServerConfigPropagatesNilCrossCutting(t *testing.T) {
 
 // TestDeriveServerConfigMapsAllFields guards deriveServerConfig against silent drift:
 // with every readable source field non-zero and every collaborator param non-nil, every
-// ServerConfig field must be populated. If a future ServerConfig field is added and
-// deriveServerConfig forgets it, this fails (the field is zero). Add a deliberately-
-// unmapped field to the skip set with a reason, mirroring the doc comment.
+// ServerConfig field must be populated. deriveServerConfig is a pure pass-through (no
+// defaulting), so this presence check covers every field uniformly — a dropped mapping
+// surfaces as a zero field. The skip set (none today) is the escape hatch for any future
+// semantically-zero-valid field — a bool/int that legitimately defaults to its zero value
+// — since a presence check cannot distinguish "unset" from "validly zero".
 func TestDeriveServerConfigMapsAllFields(t *testing.T) {
 	t.Parallel()
 
@@ -212,7 +204,9 @@ func TestDeriveCoreConfigUsesRawServerNameAndNilDefaults(t *testing.T) {
 
 // TestDeriveCoreConfigMapsAllFields guards deriveCoreConfig against silent drift: with
 // every cross-cutting source field non-zero and every collaborator non-nil, every
-// core.Config field must be populated.
+// core.Config field must be populated. Like TestDeriveServerConfigMapsAllFields this is a
+// pure presence check; the skip set is the escape hatch for any future semantically-zero-
+// valid field (a bool/int that legitimately defaults to its zero value).
 func TestDeriveCoreConfigMapsAllFields(t *testing.T) {
 	t.Parallel()
 
diff --git a/pkg/vmcp/server/serve.go b/pkg/vmcp/server/serve.go
index 6187cc88a9..916dc62b73 100644
--- a/pkg/vmcp/server/serve.go
+++ b/pkg/vmcp/server/serve.go
@@ -4,7 +4,6 @@
 package server
 
 import (
-	"cmp"
 	"context"
 	"fmt"
 	"net/http"
@@ -296,11 +295,11 @@ func Serve(ctx context.Context, v core.VMCP, cfg *ServerConfig) (*Server, error)
 	return srv, nil
 }
 
-// buildServeConfig maps the transport-only ServerConfig onto the existing *Config
-// the carried-forward (*Server) methods read from, applying the same transport
-// defaults server.New applies today. Defaults are applied here rather than by
-// mutating the caller's ServerConfig (go-style: copy before mutating caller input).
-// Port 0 is left untouched to mean "OS-assigned".
+// buildServeConfig maps the transport-only ServerConfig onto the existing *Config the
+// carried-forward (*Server) methods read from. It is a pure pass-through: transport
+// defaults are resolved once at the composition root via WithDefaults (Option 1), so the
+// incoming ServerConfig is already resolved and buildServeConfig applies no defaulting of
+// its own. Port 0 still means "OS-assigned".
 //
 // Several Config fields are deliberately NOT mapped at this phase (see
 // TestBuildServeConfigMapsSharedFields, which guards this list against drift):
@@ -320,13 +319,13 @@ func Serve(ctx context.Context, v core.VMCP, cfg *ServerConfig) (*Server, error)
 //     wiring), not via Config→New, so these Config fields are unused on the Serve path.
 func buildServeConfig(cfg *ServerConfig) *Config {
 	return &Config{
-		Name:                    cmp.Or(cfg.Name, defaultServerName),
-		Version:                 cmp.Or(cfg.Version, defaultServerVersion),
+		Name:                    cfg.Name,
+		Version:                 cfg.Version,
 		GroupRef:                cfg.GroupRef,
-		Host:                    cmp.Or(cfg.Host, defaultHost),
+		Host:                    cfg.Host,
 		Port:                    cfg.Port,
-		EndpointPath:            cmp.Or(cfg.EndpointPath, defaultEndpointPath),
-		SessionTTL:              cmp.Or(cfg.SessionTTL, defaultSessionTTL),
+		EndpointPath:            cfg.EndpointPath,
+		SessionTTL:              cfg.SessionTTL,
 		AuthMiddleware:          cfg.AuthMiddleware,
 		RateLimitMiddleware:     cfg.RateLimitMiddleware,
 		AuthInfoHandler:         cfg.AuthInfoHandler,
diff --git a/pkg/vmcp/server/serve_session_test.go b/pkg/vmcp/server/serve_session_test.go
index 34ed1331eb..965d7a04cb 100644
--- a/pkg/vmcp/server/serve_session_test.go
+++ b/pkg/vmcp/server/serve_session_test.go
@@ -204,6 +204,7 @@ func TestServeRegistersSessionHooks(t *testing.T) {
 	fc := &fakeCore{tools: []vmcp.Tool{testTool}}
 
 	srv, err := Serve(context.Background(), fc, &ServerConfig{
+		SessionTTL:           time.Minute,
 		SessionManagerConfig: &sessionmanager.FactoryConfig{Base: factory},
 		BackendRegistry:      vmcp.NewImmutableRegistry([]vmcp.Backend{}),
 	})
@@ -302,6 +303,7 @@ func TestServeLazyInjectsToolsForRehydratedSession(t *testing.T) {
 	fc := &fakeCore{tools: []vmcp.Tool{testTool}}
 
 	srv, err := Serve(context.Background(), fc, &ServerConfig{
+		SessionTTL:           time.Minute,
 		SessionManagerConfig: &sessionmanager.FactoryConfig{Base: factory},
 		BackendRegistry:      vmcp.NewImmutableRegistry([]vmcp.Backend{}),
 	})
@@ -365,6 +367,7 @@ func TestServeReturnsErrorWhenSessionManagerConstructionFails(t *testing.T) {
 	t.Parallel()
 
 	srv, err := Serve(context.Background(), &stubVMCP{}, &ServerConfig{
+		SessionTTL:           time.Minute,
 		SessionManagerConfig: &sessionmanager.FactoryConfig{Base: testMinimalFactory(), CacheCapacity: -1},
 		BackendRegistry:      vmcp.NewImmutableRegistry([]vmcp.Backend{}),
 	})
@@ -460,6 +463,7 @@ func TestServeHandlerSkipsDiscoveryAndRoutesCallThroughCore(t *testing.T) {
 	fc := &fakeCore{tools: []vmcp.Tool{testTool}}
 
 	srv, err := Serve(context.Background(), fc, &ServerConfig{
+		SessionTTL:           time.Minute,
 		SessionManagerConfig: &sessionmanager.FactoryConfig{Base: factory},
 		BackendRegistry:      vmcp.NewImmutableRegistry([]vmcp.Backend{}),
 	})
@@ -523,6 +527,7 @@ func TestServeEnforcesSessionBinding(t *testing.T) {
 	fc := &fakeCore{tools: []vmcp.Tool{testTool}}
 
 	srv, err := Serve(context.Background(), fc, &ServerConfig{
+		SessionTTL:           time.Minute,
 		SessionManagerConfig: &sessionmanager.FactoryConfig{Base: factory},
 		BackendRegistry:      vmcp.NewImmutableRegistry([]vmcp.Backend{}),
 	})
@@ -576,6 +581,7 @@ func registerServeSession(t *testing.T, fc *fakeCore) (*Server, string, string)
 	factory, _ := newToolSessionFactory(t, ctrl, fc.tools)
 
 	srv, err := Serve(context.Background(), fc, &ServerConfig{
+		SessionTTL:           time.Minute,
 		SessionManagerConfig: &sessionmanager.FactoryConfig{Base: factory},
 		BackendRegistry:      vmcp.NewImmutableRegistry([]vmcp.Backend{}),
 	})
diff --git a/pkg/vmcp/server/serve_test.go b/pkg/vmcp/server/serve_test.go
index 6e9d552044..5f2b6430d7 100644
--- a/pkg/vmcp/server/serve_test.go
+++ b/pkg/vmcp/server/serve_test.go
@@ -87,44 +87,29 @@ func testMinimalSessionManagerConfig() *sessionmanager.FactoryConfig {
 	return &sessionmanager.FactoryConfig{Base: testMinimalFactory()}
 }
 
-// testMinimalServeConfig returns a minimal valid ServerConfig for Serve tests that do
-// not exercise session creation: a non-nil SessionManagerConfig and an empty
-// BackendRegistry, the two required collaborators Serve validates.
+// testMinimalServeConfig returns a minimal valid, fully-resolved ServerConfig for Serve
+// tests that do not exercise session creation: the two required collaborators Serve
+// validates (a non-nil SessionManagerConfig and an empty BackendRegistry) plus the
+// transport scalars resolved to their defaults. Serve is a pure pass-through now —
+// transport defaulting moved to the composition root via WithDefaults (Option 1) — so the
+// scalars must be set here; in particular SessionTTL must be non-zero or the session data
+// storage construction fails ("ttl must be a positive duration").
 func testMinimalServeConfig() *ServerConfig {
 	return &ServerConfig{
+		Name:                 defaultServerName,
+		Version:              defaultServerVersion,
+		Host:                 defaultHost,
+		EndpointPath:         defaultEndpointPath,
+		SessionTTL:           defaultSessionTTL,
 		SessionManagerConfig: testMinimalSessionManagerConfig(),
 		BackendRegistry:      vmcp.NewImmutableRegistry([]vmcp.Backend{}),
 	}
 }
 
-func TestServeAppliesTransportDefaults(t *testing.T) {
-	t.Parallel()
-
-	// Empty transport fields exercise every default; Port is left zero.
-	cfg := testMinimalServeConfig()
-
-	srv, err := Serve(context.Background(), &stubVMCP{}, cfg)
-	require.NoError(t, err)
-	t.Cleanup(func() { _ = srv.Stop(context.Background()) })
-	require.NotNil(t, srv)
-	require.NotNil(t, srv.MCPServer())
-
-	// Defaults mirror server.New and are applied to the server's own config,
-	// leaving the caller's ServerConfig untouched. Asserting against the shared
-	// consts (not literals) keeps this test from being a third copy that can drift.
-	assert.Equal(t, defaultServerName, srv.config.Name)
-	assert.Equal(t, defaultServerVersion, srv.config.Version)
-	assert.Equal(t, defaultHost, srv.config.Host)
-	assert.Equal(t, defaultEndpointPath, srv.config.EndpointPath)
-	assert.Equal(t, defaultSessionTTL, srv.config.SessionTTL)
-	assert.Equal(t, 0, srv.config.Port) // Port 0 => OS-assigned
-
-	assert.Empty(t, cfg.Host, "caller config must not be mutated")
-
-	// Address reflects the defaulted host and unassigned port (no listener yet).
-	assert.Equal(t, defaultHost+":0", srv.Address())
-}
-
+// Transport defaulting is no longer Serve's job (Option 1: it is resolved once at the
+// composition root via WithDefaults), so there is no "Serve applies defaults" test — the
+// WithDefaults resolver is covered by TestWithDefaults, and Serve's faithful pass-through
+// of an already-resolved config is covered by TestServePreservesExplicitConfig below.
 func TestServePreservesExplicitConfig(t *testing.T) {
 	t.Parallel()
 
@@ -235,6 +220,7 @@ func TestServeHandlerRegistersMetricsWhenTelemetryEnabled(t *testing.T) {
 	t.Cleanup(func() { _ = provider.Shutdown(ctx) })
 
 	srv, err := Serve(ctx, &stubVMCP{}, &ServerConfig{
+		SessionTTL:           defaultSessionTTL,
 		SessionManagerConfig: testMinimalSessionManagerConfig(),
 		BackendRegistry:      vmcp.NewImmutableRegistry([]vmcp.Backend{}),
 		TelemetryProvider:    provider,
@@ -321,9 +307,9 @@ func TestServeValidation(t *testing.T) {
 // reason, mirroring the buildServeConfig doc comment.
 //
 // This is a PRESENCE check only — with every source field non-zero it cannot catch a
-// wrong-source mapping or default-value drift. Value correctness is carried by
-// TestServePreservesExplicitConfig (pass-through scalars) and
-// TestServeAppliesTransportDefaults (the cmp.Or defaults).
+// wrong-source mapping. buildServeConfig is a pure pass-through (defaulting moved to the
+// composition root via WithDefaults, Option 1), so value correctness is carried by
+// TestServePreservesExplicitConfig.
 func TestBuildServeConfigMapsSharedFields(t *testing.T) {
 	t.Parallel()
 
diff --git a/pkg/vmcp/server/server.go b/pkg/vmcp/server/server.go
index 2355bd103c..eb6aee17d2 100644
--- a/pkg/vmcp/server/server.go
+++ b/pkg/vmcp/server/server.go
@@ -9,6 +9,7 @@
 package server
 
 import (
+	"cmp"
 	"context"
 	"encoding/json"
 	"errors"
@@ -315,6 +316,28 @@ func buildSessionDataStorage(ctx context.Context, cfg *Config) (transportsession
 	return transportsession.NewRedisSessionDataStorage(ctx, redisCfg, keyPrefix, cfg.SessionTTL)
 }
 
+// WithDefaults returns a copy of cfg with the transport defaults applied to any field
+// left unset: Name, Version, Host, EndpointPath, and SessionTTL. It is the single place
+// these defaults are defined. The composition root (cli) — and any test that builds a
+// Config by hand — resolves its Config through WithDefaults before handing it to New, so
+// New, Serve, buildServeConfig, and the derive* helpers can all treat their input as
+// already-resolved pass-through (Option 1: default once at the edge, not per-constructor).
+//
+// Port 0 is left untouched — it means "let the OS assign a random port" (the CLI flag
+// supplies the production default of 4483).
+//
+// cfg is treated as read-only: a shallow copy is returned and the caller's value is not
+// mutated (go-style: copy before mutating caller input).
+func WithDefaults(cfg *Config) *Config {
+	resolved := *cfg
+	resolved.Name = cmp.Or(resolved.Name, defaultServerName)
+	resolved.Version = cmp.Or(resolved.Version, defaultServerVersion)
+	resolved.Host = cmp.Or(resolved.Host, defaultHost)
+	resolved.EndpointPath = cmp.Or(resolved.EndpointPath, defaultEndpointPath)
+	resolved.SessionTTL = cmp.Or(resolved.SessionTTL, defaultSessionTTL)
+	return &resolved
+}
+
 // New creates a new Virtual MCP Server instance.
 //
 // The backendRegistry parameter provides the list of available backends:
@@ -331,24 +354,15 @@ func New(
 	backendRegistry vmcp.BackendRegistry,
 	workflowDefs map[string]*composer.WorkflowDefinition,
 ) (*Server, error) {
-	// Apply defaults
-	if cfg.Host == "" {
-		cfg.Host = defaultHost
-	}
-	// Note: Port 0 means "let OS assign random port" - intentionally no default applied here.
-	// CLI provides default via flag (4483), so Port is only 0 in tests for dynamic port assignment.
-	if cfg.EndpointPath == "" {
-		cfg.EndpointPath = defaultEndpointPath
-	}
-	if cfg.Name == "" {
-		cfg.Name = defaultServerName
-	}
-	if cfg.Version == "" {
-		cfg.Version = defaultServerVersion
-	}
-	if cfg.SessionTTL == 0 {
-		cfg.SessionTTL = defaultSessionTTL
-	}
+	// Resolve transport defaults on a COPY. The composition root (cli) already resolves
+	// them at the edge via WithDefaults (Option 1: a single defaulting list); New repeats
+	// it defensively so legacy direct callers and tests that build a Config by hand keep
+	// working — but without mutating the caller's value (go-style: copy before mutating
+	// caller input). That non-mutation is what lets #5445 hand the raw, un-defaulted
+	// cfg.Name to the core for Cedar authz parity. New's own defaulting goes away when
+	// #5445 reduces it to a Serve(core.New(...)) wrapper; until then WithDefaults is the
+	// single place the default list lives, shared with the edge.
+	cfg = WithDefaults(cfg)
 
 	// Create hooks for SDK integration
 	hooks := &server.Hooks{}
diff --git a/pkg/vmcp/server/server_test.go b/pkg/vmcp/server/server_test.go
index 402f857e8e..79eaf1f32d 100644
--- a/pkg/vmcp/server/server_test.go
+++ b/pkg/vmcp/server/server_test.go
@@ -215,6 +215,78 @@ func TestNew(t *testing.T) {
 	}
 }
 
+// TestWithDefaults covers the single transport-defaulting resolver. It is the one place
+// the default list lives (Option 1); the composition root and the constructors route
+// their Config through it, so New/Serve/derive* downstream are pure pass-through.
+func TestWithDefaults(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+		in   *server.Config
+		want server.Config // only transport scalars are compared
+	}{
+		{
+			name: "fills every transport default on an empty config",
+			in:   &server.Config{},
+			want: server.Config{
+				Name: "toolhive-vmcp", Version: "0.1.0", Host: "127.0.0.1",
+				EndpointPath: "/mcp", SessionTTL: 30 * time.Minute, Port: 0,
+			},
+		},
+		{
+			name: "preserves explicitly set values",
+			in: &server.Config{
+				Name: "custom", Version: "1.2.3", Host: "0.0.0.0",
+				EndpointPath: "/rpc", SessionTTL: 7 * time.Minute, Port: 8080,
+			},
+			want: server.Config{
+				Name: "custom", Version: "1.2.3", Host: "0.0.0.0",
+				EndpointPath: "/rpc", SessionTTL: 7 * time.Minute, Port: 8080,
+			},
+		},
+		{
+			name: "fills only the unset fields",
+			in:   &server.Config{Host: "192.168.1.1", Port: 9000},
+			want: server.Config{
+				Name: "toolhive-vmcp", Version: "0.1.0", Host: "192.168.1.1",
+				EndpointPath: "/mcp", SessionTTL: 30 * time.Minute, Port: 9000,
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			got := server.WithDefaults(tt.in)
+			assert.Equal(t, tt.want.Name, got.Name)
+			assert.Equal(t, tt.want.Version, got.Version)
+			assert.Equal(t, tt.want.Host, got.Host)
+			assert.Equal(t, tt.want.EndpointPath, got.EndpointPath)
+			assert.Equal(t, tt.want.SessionTTL, got.SessionTTL)
+			assert.Equal(t, tt.want.Port, got.Port) // Port is never defaulted (0 => OS-assigned)
+		})
+	}
+}
+
+// TestWithDefaultsDoesNotMutateInput verifies WithDefaults treats its argument as
+// read-only: an all-unset Config passes through with no fields written back, so callers
+// (and the constructors that call it on a copy) never see their value mutated. This
+// non-mutation is what preserves the raw cfg.Name for downstream Cedar authz parity.
+func TestWithDefaultsDoesNotMutateInput(t *testing.T) {
+	t.Parallel()
+
+	in := &server.Config{} // all unset: defaulting would be visible as mutation
+	_ = server.WithDefaults(in)
+
+	assert.Empty(t, in.Name)
+	assert.Empty(t, in.Version)
+	assert.Empty(t, in.Host)
+	assert.Empty(t, in.EndpointPath)
+	assert.Zero(t, in.SessionTTL)
+}
+
 func TestServer_Address(t *testing.T) {
 	t.Parallel()
 

From bcc94c1d6d2e9af0f99469b669bdccdfd45f4479 Mon Sep 17 00:00:00 2001
From: Trey <tgrunnagle@gmail.com>
Date: Mon, 15 Jun 2026 10:44:08 -0700
Subject: [PATCH 3/4] Drop "Option 1" review shorthand from comments

"Option 1" referred to a PR-review discussion thread and is meaningless
to future readers of the code. Reword the comments to keep the substance
(transport defaults resolved once at the composition root via
WithDefaults) without the review-specific label.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 pkg/vmcp/cli/serve.go          | 2 +-
 pkg/vmcp/server/derive.go      | 2 +-
 pkg/vmcp/server/derive_test.go | 2 +-
 pkg/vmcp/server/serve.go       | 2 +-
 pkg/vmcp/server/serve_test.go  | 6 +++---
 pkg/vmcp/server/server.go      | 4 ++--
 pkg/vmcp/server/server_test.go | 2 +-
 7 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/pkg/vmcp/cli/serve.go b/pkg/vmcp/cli/serve.go
index 0a7cdcb032..88e81ded57 100644
--- a/pkg/vmcp/cli/serve.go
+++ b/pkg/vmcp/cli/serve.go
@@ -398,7 +398,7 @@ func Serve(ctx context.Context, cfg ServeConfig) error {
 		}()
 	}
 
-	// Resolve transport defaults once here at the composition root (Option 1): the
+	// Resolve transport defaults once here at the composition root: the
 	// vMCP config edge is the single place flags/CRD/YAML become a fully-resolved
 	// Config, so server.New, Serve, and the derive* helpers downstream are pure
 	// pass-through. WithDefaults fills any unset Host/EndpointPath/SessionTTL/Name/
diff --git a/pkg/vmcp/server/derive.go b/pkg/vmcp/server/derive.go
index 5e70108020..ed12513623 100644
--- a/pkg/vmcp/server/derive.go
+++ b/pkg/vmcp/server/derive.go
@@ -22,7 +22,7 @@ import (
 //
 // cfg is treated as read-only (go-style: copy before mutating caller input); a fresh
 // ServerConfig is returned. This is a pure projection: transport defaults are resolved
-// once at the composition root via WithDefaults (Option 1), so cfg already holds resolved
+// once at the composition root via WithDefaults, so cfg already holds resolved
 // values and deriveServerConfig applies no defaulting of its own. Port 0 still means
 // "OS-assigned".
 //
diff --git a/pkg/vmcp/server/derive_test.go b/pkg/vmcp/server/derive_test.go
index 6bdd660d99..378724a472 100644
--- a/pkg/vmcp/server/derive_test.go
+++ b/pkg/vmcp/server/derive_test.go
@@ -102,7 +102,7 @@ func TestDeriveServerConfigProjectsTransportFields(t *testing.T) {
 }
 
 // deriveServerConfig no longer applies transport defaults — it is a pure projection now
-// that defaulting is resolved once at the composition root (Option 1). The WithDefaults
+// that defaulting is resolved once at the composition root. The WithDefaults
 // resolver carries that behavior and is covered by TestWithDefaults (server_test.go).
 
 func TestDeriveServerConfigPropagatesNilCrossCutting(t *testing.T) {
diff --git a/pkg/vmcp/server/serve.go b/pkg/vmcp/server/serve.go
index 916dc62b73..b66cf80d4a 100644
--- a/pkg/vmcp/server/serve.go
+++ b/pkg/vmcp/server/serve.go
@@ -297,7 +297,7 @@ func Serve(ctx context.Context, v core.VMCP, cfg *ServerConfig) (*Server, error)
 
 // buildServeConfig maps the transport-only ServerConfig onto the existing *Config the
 // carried-forward (*Server) methods read from. It is a pure pass-through: transport
-// defaults are resolved once at the composition root via WithDefaults (Option 1), so the
+// defaults are resolved once at the composition root via WithDefaults, so the
 // incoming ServerConfig is already resolved and buildServeConfig applies no defaulting of
 // its own. Port 0 still means "OS-assigned".
 //
diff --git a/pkg/vmcp/server/serve_test.go b/pkg/vmcp/server/serve_test.go
index 5f2b6430d7..966186c3e5 100644
--- a/pkg/vmcp/server/serve_test.go
+++ b/pkg/vmcp/server/serve_test.go
@@ -91,7 +91,7 @@ func testMinimalSessionManagerConfig() *sessionmanager.FactoryConfig {
 // tests that do not exercise session creation: the two required collaborators Serve
 // validates (a non-nil SessionManagerConfig and an empty BackendRegistry) plus the
 // transport scalars resolved to their defaults. Serve is a pure pass-through now —
-// transport defaulting moved to the composition root via WithDefaults (Option 1) — so the
+// transport defaulting moved to the composition root via WithDefaults — so the
 // scalars must be set here; in particular SessionTTL must be non-zero or the session data
 // storage construction fails ("ttl must be a positive duration").
 func testMinimalServeConfig() *ServerConfig {
@@ -106,7 +106,7 @@ func testMinimalServeConfig() *ServerConfig {
 	}
 }
 
-// Transport defaulting is no longer Serve's job (Option 1: it is resolved once at the
+// Transport defaulting is no longer Serve's job (it is resolved once at the
 // composition root via WithDefaults), so there is no "Serve applies defaults" test — the
 // WithDefaults resolver is covered by TestWithDefaults, and Serve's faithful pass-through
 // of an already-resolved config is covered by TestServePreservesExplicitConfig below.
@@ -308,7 +308,7 @@ func TestServeValidation(t *testing.T) {
 //
 // This is a PRESENCE check only — with every source field non-zero it cannot catch a
 // wrong-source mapping. buildServeConfig is a pure pass-through (defaulting moved to the
-// composition root via WithDefaults, Option 1), so value correctness is carried by
+// composition root via WithDefaults), so value correctness is carried by
 // TestServePreservesExplicitConfig.
 func TestBuildServeConfigMapsSharedFields(t *testing.T) {
 	t.Parallel()
diff --git a/pkg/vmcp/server/server.go b/pkg/vmcp/server/server.go
index eb6aee17d2..cc05c80e90 100644
--- a/pkg/vmcp/server/server.go
+++ b/pkg/vmcp/server/server.go
@@ -321,7 +321,7 @@ func buildSessionDataStorage(ctx context.Context, cfg *Config) (transportsession
 // these defaults are defined. The composition root (cli) — and any test that builds a
 // Config by hand — resolves its Config through WithDefaults before handing it to New, so
 // New, Serve, buildServeConfig, and the derive* helpers can all treat their input as
-// already-resolved pass-through (Option 1: default once at the edge, not per-constructor).
+// already-resolved pass-through (default once at the edge, not per-constructor).
 //
 // Port 0 is left untouched — it means "let the OS assign a random port" (the CLI flag
 // supplies the production default of 4483).
@@ -355,7 +355,7 @@ func New(
 	workflowDefs map[string]*composer.WorkflowDefinition,
 ) (*Server, error) {
 	// Resolve transport defaults on a COPY. The composition root (cli) already resolves
-	// them at the edge via WithDefaults (Option 1: a single defaulting list); New repeats
+	// them at the edge via WithDefaults (a single defaulting list); New repeats
 	// it defensively so legacy direct callers and tests that build a Config by hand keep
 	// working — but without mutating the caller's value (go-style: copy before mutating
 	// caller input). That non-mutation is what lets #5445 hand the raw, un-defaulted
diff --git a/pkg/vmcp/server/server_test.go b/pkg/vmcp/server/server_test.go
index 79eaf1f32d..672217a712 100644
--- a/pkg/vmcp/server/server_test.go
+++ b/pkg/vmcp/server/server_test.go
@@ -216,7 +216,7 @@ func TestNew(t *testing.T) {
 }
 
 // TestWithDefaults covers the single transport-defaulting resolver. It is the one place
-// the default list lives (Option 1); the composition root and the constructors route
+// the default list lives; the composition root and the constructors route
 // their Config through it, so New/Serve/derive* downstream are pure pass-through.
 func TestWithDefaults(t *testing.T) {
 	t.Parallel()

From d000451b012a2bf9b61fed67a39213939f43ca1a Mon Sep 17 00:00:00 2001
From: Trey <tgrunnagle@gmail.com>
Date: Mon, 15 Jun 2026 10:50:14 -0700
Subject: [PATCH 4/4] Move WithDefaults to derive.go

WithDefaults is config plumbing of the same family as deriveServerConfig
and deriveCoreConfig (resolve/decompose the legacy Config at the
composition-root edge), so it belongs alongside them rather than in the
already-oversized server.go. No behavior change; cmp moves with it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 pkg/vmcp/server/derive.go | 24 ++++++++++++++++++++++++
 pkg/vmcp/server/server.go | 23 -----------------------
 2 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/pkg/vmcp/server/derive.go b/pkg/vmcp/server/derive.go
index ed12513623..6b304f2b45 100644
--- a/pkg/vmcp/server/derive.go
+++ b/pkg/vmcp/server/derive.go
@@ -4,6 +4,8 @@
 package server
 
 import (
+	"cmp"
+
 	"github.com/stacklok/toolhive/pkg/authz"
 	"github.com/stacklok/toolhive/pkg/vmcp"
 	"github.com/stacklok/toolhive/pkg/vmcp/aggregator"
@@ -14,6 +16,28 @@ import (
 	"github.com/stacklok/toolhive/pkg/vmcp/server/sessionmanager"
 )
 
+// WithDefaults returns a copy of cfg with the transport defaults applied to any field
+// left unset: Name, Version, Host, EndpointPath, and SessionTTL. It is the single place
+// these defaults are defined. The composition root (cli) — and any test that builds a
+// Config by hand — resolves its Config through WithDefaults before handing it to New, so
+// New, Serve, buildServeConfig, and the derive* helpers below can all treat their input as
+// already-resolved pass-through (default once at the edge, not per-constructor).
+//
+// Port 0 is left untouched — it means "let the OS assign a random port" (the CLI flag
+// supplies the production default of 4483).
+//
+// cfg is treated as read-only: a shallow copy is returned and the caller's value is not
+// mutated (go-style: copy before mutating caller input).
+func WithDefaults(cfg *Config) *Config {
+	resolved := *cfg
+	resolved.Name = cmp.Or(resolved.Name, defaultServerName)
+	resolved.Version = cmp.Or(resolved.Version, defaultServerVersion)
+	resolved.Host = cmp.Or(resolved.Host, defaultHost)
+	resolved.EndpointPath = cmp.Or(resolved.EndpointPath, defaultEndpointPath)
+	resolved.SessionTTL = cmp.Or(resolved.SessionTTL, defaultSessionTTL)
+	return &resolved
+}
+
 // deriveServerConfig projects the transport-only configuration out of the legacy,
 // in-memory server.Config onto the ServerConfig that Serve consumes. It is the
 // transport half of the Phase 3 config decomposition (#5444); the composition root
diff --git a/pkg/vmcp/server/server.go b/pkg/vmcp/server/server.go
index cc05c80e90..146a9799dd 100644
--- a/pkg/vmcp/server/server.go
+++ b/pkg/vmcp/server/server.go
@@ -9,7 +9,6 @@
 package server
 
 import (
-	"cmp"
 	"context"
 	"encoding/json"
 	"errors"
@@ -316,28 +315,6 @@ func buildSessionDataStorage(ctx context.Context, cfg *Config) (transportsession
 	return transportsession.NewRedisSessionDataStorage(ctx, redisCfg, keyPrefix, cfg.SessionTTL)
 }
 
-// WithDefaults returns a copy of cfg with the transport defaults applied to any field
-// left unset: Name, Version, Host, EndpointPath, and SessionTTL. It is the single place
-// these defaults are defined. The composition root (cli) — and any test that builds a
-// Config by hand — resolves its Config through WithDefaults before handing it to New, so
-// New, Serve, buildServeConfig, and the derive* helpers can all treat their input as
-// already-resolved pass-through (default once at the edge, not per-constructor).
-//
-// Port 0 is left untouched — it means "let the OS assign a random port" (the CLI flag
-// supplies the production default of 4483).
-//
-// cfg is treated as read-only: a shallow copy is returned and the caller's value is not
-// mutated (go-style: copy before mutating caller input).
-func WithDefaults(cfg *Config) *Config {
-	resolved := *cfg
-	resolved.Name = cmp.Or(resolved.Name, defaultServerName)
-	resolved.Version = cmp.Or(resolved.Version, defaultServerVersion)
-	resolved.Host = cmp.Or(resolved.Host, defaultHost)
-	resolved.EndpointPath = cmp.Or(resolved.EndpointPath, defaultEndpointPath)
-	resolved.SessionTTL = cmp.Or(resolved.SessionTTL, defaultSessionTTL)
-	return &resolved
-}
-
 // New creates a new Virtual MCP Server instance.
 //
 // The backendRegistry parameter provides the list of available backends: