Add vMCP assembly API in pkg/vmcp/app

[tgrunnagle]

Summary

Closes #5581

vMCP's assembly — turning a vmcpconfig.Config into a running server — previously lived
entirely in pkg/vmcp/cli/serve.go (~700 lines). Any embedder (e.g., the enterprise
gateway) that wanted a vMCP server had to replicate that wiring by hand, and would break
whenever OSS changed its assembly sequence or internal rules. This PR encapsulates the
assembly behind a public API in the new pkg/vmcp/app package so that callers hand over
config and get back a core.VMCP to decorate and a *server.ServerConfig to serve —
without reimplementing vMCP internals.

• New app.BuildCore derives core.Config from vmcpconfig.Config (+ options) and
  returns the assembled core.VMCP for decoration.
• New app.BuildServerConfig independently derives *server.ServerConfig from the same
  config — the transport config is not a byproduct of core construction.
• Functional Option values carry non-serializable runtime injectables: version, host/port,
  session TTL, telemetry provider, backend registry + watcher, REST config, embedded auth
  server run config, status reporter, and elicitation requester.
• Shared stateful collaborators (telemetry provider, K8s backend registry + watcher) are
  injected via options so one instance is shared between both Build* calls — no duplicate
  OTEL pipelines or K8s informer caches.
• pkg/vmcp/cli/serve.go is migrated to call app.BuildCore + app.BuildServerConfig,
  reducing it from ~700 lines to ~360 and removing all domain assembly logic from the CLI.
• LateBoundElicitationRequester and its constructor are exported so embedders can wire
  composite-tool elicitation without access to package-internal types.

The migration is behavior-preserving for the production CLI/operator path. Two behaviors
that the legacy server.New + hand-wired cli.Serve provided had to be carried into the
assembly, since the operator launches the same vmcp serve binary in its pods:

• Kubernetes "discovered" backends are seeded synchronously at startup. The discovered-mode
  path builds its registry via backendregistry.NewKubernetesBackendRegistry, which started
  empty and relied solely on the watcher's controller-runtime reconciler to populate it. That
  reconcile runs asynchronously and lags the informer cache-sync that the pod's /readyz
  probe gates on, so the server could pass readiness and begin serving (and reporting
  status.discoveredBackends) while the registry was still empty — a startup window where it
  advertised zero backends. The legacy cli.Serve avoided this by seeding the registry from a
  synchronous discovery before serving. To restore that, a new k8s.BackendWatcher.SyncRegistry
  lists the group's current backends through the same workloads.Discoverer conversion the
  reconciler uses and upserts them; NewKubernetesBackendRegistry calls it before returning, so
  the registry is populated before the server serves — for both cli.Serve and embedders. The
  watcher still keeps it current; a seed failure is logged and tolerated. This was caught by the
  operator lifecycle E2E (VirtualMCPServer Unauthenticated Backend Auth).
• Code mode is wired in BuildCore. server.New wrapped the core with
  codemode.NewDecorator when configured; BuildCore now does the same from
  vmcpConfig.CodeMode, so the execute_tool_script virtual tool is advertised on the
  assembly path (it sits below any Serve-layer optimizer). Caught by the VirtualMCPServer Code Mode E2E.

Public interfaces are unchanged: core.VMCP, core.New/Config, server.Serve/ServerConfig/New,
and backendregistry.NewKubernetesBackendRegistry's signature are byte-identical to main, so
downstream embedders are unaffected (the registry seed is a backward-compatible improvement —
callers now receive a populated registry instead of a transiently-empty one).

Large PR Justification

This PR is a single, atomic logical change — encapsulating vMCP assembly behind the
new pkg/vmcp/app API (issue #5581) — that cannot be cleanly split:

• The API and its sole production caller are interdependent. The new app package
  (BuildCore / BuildServerConfig / Option) and the migration of pkg/vmcp/cli/serve.go
  onto it are two halves of one change. Migrating cli.Serve is an explicit acceptance
  criterion of Encapsulate vMCP assembly behind a config-driven public API #5581 ("prove the API is sufficient for the real caller"). Landing the API
  without the migration would add dead code; landing the migration without the API is
  impossible. Splitting them would leave a broken or dead intermediate state.
• Much of the diff is net-neutral or test/doc. The new production code is offset by a
  large reduction in cli/serve.go (~700 → ~360 lines; 499 changed, mostly deletions).
  Roughly 600 added lines are tests (6 files), the runnable example embedder, and docs.
• It is already validated end-to-end. Unit tests plus the operator lifecycle E2E suite
  (discovered mode, code mode, auth discovery, aggregation) pass on a Kind cluster running
  the branch's image.

Splitting into <1000-line PRs would increase review overhead and risk without improving
reviewability, since the pieces only make sense together.

Type of change

• New feature

Test plan

• Unit tests (task test)
• Linting (task lint-fix)
• Operator lifecycle E2E (task thv-operator-e2e-test-run on a local Kind cluster)

Unit tests in pkg/vmcp/app/build_core_test.go and pkg/vmcp/app/build_server_config_test.go
cover both static-backends and dynamic-discovery modes, option application, and nil-config
rejection. The pkg/vmcp/app/internal/exampleembedder package demonstrates the full
BuildCore → decorate → BuildServerConfig → server.Serve flow and validates that the
public API types plug directly into server.Serve.

The operator lifecycle E2E suite was run on a local Kind cluster against a vmcp image built
from this branch, confirming the discovered-mode seeding and code-mode fixes: the
VirtualMCPServer Unauthenticated Backend Auth, Code Mode, Auth Discovery, and discovered-mode
aggregation/circuit-breaker specs pass. (The optimizer specs require the TEI embeddings image,
which has no arm64 manifest, so they were exercised in CI rather than locally.)

Changes

File	Change
pkg/vmcp/app/options.go	New: Option functional type and all With* constructors
pkg/vmcp/app/build_core.go	New: BuildCore, backend-registry resolution, and code-mode decorator wiring
pkg/vmcp/app/build_core_test.go	New: unit tests for BuildCore
pkg/vmcp/app/build_server_config.go	New: BuildServerConfig and transport derivation helpers
pkg/vmcp/app/build_server_config_test.go	New: unit tests for BuildServerConfig
pkg/vmcp/app/discovery_internal_test.go	New: ported runDiscovery/vmcpNamespace tests from cli
pkg/vmcp/app/importgraph_test.go	New: build-graph anchor for the example embedder
pkg/vmcp/app/header_forward_env.go	Moved from pkg/vmcp/cli/ (no logic change)
pkg/vmcp/app/header_forward_env_test.go	Moved from pkg/vmcp/cli/ (no logic change)
pkg/vmcp/app/internal/exampleembedder/embedder.go	New: runnable embedder example
pkg/vmcp/backendregistry/registry.go	Seed the registry via SyncRegistry before returning (discovered mode)
pkg/vmcp/k8s/manager.go	New BackendWatcher.SyncRegistry: synchronous initial backend seed
pkg/vmcp/config/config.go	New OutgoingAuthSourceInline/OutgoingAuthSourceDiscovered constants
pkg/vmcp/config/validator.go	Use the new outgoing-auth source constants
pkg/vmcp/cli/serve.go	Migrated to call app.BuildCore + app.BuildServerConfig
pkg/vmcp/cli/serve_test.go	Trimmed to match reduced surface of serve.go
pkg/vmcp/cli/header_forward_env.go	Deleted (moved to pkg/vmcp/app/)
pkg/vmcp/server/elicitation_latebound.go	Exported LateBoundElicitationRequester, NewLateBoundElicitationRequester, and Bind
docs/arch/vmcp-library.md	Added pkg/vmcp/app stability-table row; noted the elicitation export

Does this introduce a user-facing change?

No. The CLI behavior of thv vmcp serve is unchanged. The new pkg/vmcp/app package is
additive public API intended for embedders; no existing user-facing behavior is modified.

Special notes for reviewers

• The two Build* functions are intentionally independent derivations from the same
  vmcpconfig.Config, mirroring the existing deriveCoreConfig/deriveServerConfig split.
  BuildCore does not return a ServerConfig — the transport config is not a byproduct of
  core construction.
• For the Kubernetes "discovered" outgoing-auth source, callers must provide
  WithBackendRegistry; BuildServerConfig returns an error if it is absent, to prevent
  starting two K8s informer caches.
• The LateBoundElicitationRequester export (pkg/vmcp/server) adds a small amount of
  surface to an existing unexported type. The package-internal newLateBoundElicitationRequester
  and bind aliases are retained so server.New call sites need no change.
• BackendWatcher.SyncRegistry reads the current backends with a direct, uncached client
  (the manager cache isn't started yet at seed time) and reuses the reconciler's
  workloads.Discoverer.GetWorkloadAsVMCPBackend conversion, so the seeded set is identical to
  what the watcher would produce. It is the seam that makes the discovered-mode assembly path
  behavior-preserving versus the legacy cli.Serve; worth extra scrutiny.

Generated with Claude Code

[github-actions]

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.

---

This review will be automatically dismissed once you add the justification section.

[codecov]

Codecov Report

❌ Patch coverage is 62.67465% with 187 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.72%. Comparing base (2aca563) to head (7c97f44).

Files with missing lines	Patch %	Lines
pkg/vmcp/app/build_core.go	56.54%	57 Missing and 16 partials ⚠️
pkg/vmcp/app/build_server_config.go	69.71%	34 Missing and 9 partials ⚠️
pkg/vmcp/app/builder.go	67.08%	21 Missing and 5 partials ⚠️
pkg/vmcp/k8s/manager.go	25.00%	17 Missing and 1 partial ⚠️
pkg/vmcp/cli/serve.go	59.09%	8 Missing and 1 partial ⚠️
pkg/vmcp/app/internal/exampleembedder/embedder.go	0.00%	8 Missing ⚠️
pkg/vmcp/app/options.go	71.42%	7 Missing and 1 partial ⚠️
pkg/vmcp/backendregistry/registry.go	60.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5672      +/-   ##
==========================================
+ Coverage   70.61%   70.72%   +0.10%     
==========================================
  Files         667      672       +5     
  Lines       67607    67856     +249     
==========================================
+ Hits        47743    47989     +246     
+ Misses      16410    16382      -28     
- Partials     3454     3485      +31     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tgrunnagle]

Multi-Agent Consensus Review

Agents consulted: api-design, architecture, go-idioms, test-coverage, general-quality

Consensus Summary

#	Finding	File	Consensus	Severity	Action
F1	Discovered-mode watcher silently discarded in BuildCore	build_core.go:199	9/10	MEDIUM	Fix
F2	exampleembedder not anchored in CI build graph	exampleembedder/embedder.go:1	9/10	MEDIUM	Fix
F3	Static/dynamic registries built twice (double-discovery risk)	build_core.go:146	7/10	MEDIUM	Discuss
F4	TestBuildCore_InlineDiscovery bypasses the path it claims to test	build_core_test.go:43	8/10	MEDIUM	Fix
F5	Three deleted CLI tests have no port in new package	serve_test.go (deletions)	7/10	MEDIUM	Fix
F6	Shallow copy for OutgoingAuth missing — mutation escapes to caller	build_server_config.go:58	8/10	MEDIUM	Fix
F7	"discovered" compared as magic string literal	build_core.go:149	7/10	LOW	Fix
F8	WithBackendRegistry accepts nil registry silently	options.go:144	7/10	LOW	Fix
F9	Asymmetric docstring language for discovered mode	build_server_config.go:29	7/10	LOW	Fix
F10	applyOptions panics on nil Option in slice	options.go:184	7/10	LOW	Fix
F11	pkg/vmcp/app missing from vmcp-library.md stability table	docs/arch/vmcp-library.md	7/10	LOW	Fix
F12	LateBoundElicitationRequester lacks stability annotation	elicitation_latebound.go:27	7/10	LOW	Fix
F13	Close() error discarded without explanation	build_core.go:143	7/10	LOW	Fix
F14	VMCP_NAMESPACE error message starts uppercase	build_core.go:181	8/10	LOW	Fix
F15	Rate-limit cleanup captures context.Background()	build_server_config.go:148	7/10	LOW	Fix
F16	Log message starts uppercase, inconsistent	build_core.go:177	7/10	LOW	Fix
F17	Cleanup-idempotency tests are vacuously true	build_server_config_test.go:153	9/10	LOW	Fix
F18	deriveHealthMonitorConfig error path untested	build_core_test.go	7/10	LOW	Fix
F19	BuildServerConfig static-backend mode untested	build_server_config_test.go	7/10	LOW	Fix
F20	exampleembedder cleanup guard pattern is brittle	exampleembedder/embedder.go:59	7/10	LOW	Polish

Overall

This PR introduces pkg/vmcp/app as the new public assembly package for vMCP, implementing the design specified in issue #5581. The approach — two independent derivation functions (BuildCore, BuildServerConfig) sharing stateful collaborators via functional options — is architecturally sound and correctly reflects the "independent projections from the same config" pattern the issue describes. The CLI migration (serve.go reduced from ~700 to ~360 lines) validates the API in the real production caller.

The main risk is a behavioral asymmetry in the Kubernetes "discovered" mode: BuildServerConfig enforces WithBackendRegistry at runtime (returning an error if absent), while BuildCore silently starts its own K8s informer with no readiness-gating handle when the option is omitted. An embedder who provides WithBackendRegistry to BuildServerConfig but forgets BuildCore gets two informer caches and no clear error. The docstring says "SHOULD provide" rather than enforcing it — this understates the risk for a public API intended for external embedders.

The test gaps (F4, F5: misleadingly-named test, three deleted tests with no port) and the missing importgraph anchor for the exampleembedder (F2) are addressable without design changes and worth fixing before downstream adoption.

Documentation

docs/arch/vmcp-library.md: The stability table has no entry for the new pkg/vmcp/app package (intended for downstream embedders). Add a row with an Experimental designation covering BuildCore, BuildServerConfig, and Option. Also note the LateBoundElicitationRequester addition to the pkg/vmcp/server entry.

F5 — Deleted tests with no port (no diff line available): TestRunDiscovery_KubernetesGroupNotFound, TestRunDiscovery_ZeroBackends, and TestVMCPNamespace were deleted from pkg/vmcp/cli/serve_test.go. The behavior each tested still exists verbatim in build_core.go (lines 240–253) and build_server_config.go. Port all three to pkg/vmcp/app/build_core_test.go / pkg/vmcp/app/build_server_config_test.go.

---

Generated with Claude Code

Large PR justification has been provided. Thank you!

[github-actions]

✅ Large PR justification has been provided. The size review has been dismissed and this PR can now proceed with normal review.

[jerm-dro]

Design question on the assembly API shape — would a builder fit better here?

Reading through the new app package and the example embedder, a pattern stood out: correct usage is largely the caller's responsibility. To assemble a server you have to (a) pass the same opts slice to both BuildCore and BuildServerConfig, (b) know that telemetry and the K8s registry must be built once and injected or you get duplicates, (c) create a LateBoundElicitationRequester, thread it in, and remember to Bind it after Serve, and (d) choreograph the cleanup funcs from both calls plus Serve. The example embedder gets all of this right, but it's ~60 lines, and it lives in internal/ so embedders can only copy it, not call it.

I think these are all symptoms of one thing: BuildCore and BuildServerConfig are peers with no shared owner, so "construct once / wire once" can't be a guarantee — it has to be a caller convention.

The elicitation case is the clearest example. The late-bind exists because core.New needs a requester before the mcp-go server exists, and the real requester needs the server. That's a genuine internal chicken-and-egg — but it only leaks out to the embedder because the embedder is the one orchestrating core → serve → bind. Whoever owns that sequence can keep the late-bind private.

Which made me wonder whether a small builder that owns Finish() would be a better public face:

b := app.NewBuilder(ctx, cfg, app.WithVersion(v), app.WithListenAddress(host, port))
b.Decorate(myDecorator)            // the THV-0076 seam
// optional overrides with config-derived defaults: WithTelemetryProvider, WithBackendRegistry, ...
srv, core, err := b.Finish()       // builds the registry once, wires elicitation internally, owns cleanup

Finish() returns both the server and the assembled core — the latter for callers who want to use it directly for other purposes.

If Finish() is the single construction pass, then: the registry is built once and shared (no double discovery / divergent snapshots between core and server), the config copy + subject-provider injection happens once before that single registry is built (so core and server can't disagree on backend auth metadata), the elicitation bind becomes internal (and LateBoundElicitationRequester no longer needs to be exported), and cleanup collapses to one owner. It'd also let the builder build telemetry from cfg.Telemetry by default instead of silently ignoring it unless injected. The existing BuildCore/BuildServerConfig could become the private internals of Finish() rather than being thrown away.

That said, I may well be missing constraints that make this awkward — e.g. an embedder needing to do something between core and server beyond decoration, or a reason the two derivations genuinely need to stay independent. wdyt? Happy to be talked out of it if there's complexity here I'm not seeing.

[tgrunnagle]

Thanks @jerm-dro — this was a good call, implemented in 14ddcec9.

app.Builder is now the primary assembly entry point and owns the single "construct once / wire once" pass you described:

srv, core, cleanup, err := app.NewBuilder(ctx, cfg,
    app.WithVersion(v), app.WithHost(host, port)).
    Decorate(myDecorator).
    Finish()
defer cleanup()

Finish() builds the shared collaborators exactly once (telemetry from cfg.Telemetry; the K8s registry + watcher for the discovered source — no double discovery), does the config copy + subject-provider injection once so the core's backend client and the transport session factory can't disagree on backend-auth metadata, wires elicitation internally (creates and binds the LateBoundElicitationRequester, so the construction-order chicken-and-egg no longer leaks), applies the decorator, and collapses cleanup to one owner.

One deliberate deviation from the proposal: I kept BuildCore/BuildServerConfig (and LateBoundElicitationRequester) public rather than making them private internals. The reason is composition: "replace an injectable" (e.g. a custom BackendRegistry) works on either surface via the interface-typed With* options, but "compose the derived pieces" — inspect/modify the *server.ServerConfig before serving, reuse the assembled core.VMCP elsewhere, or run a custom serve loop — is only possible if the primitives stay exported. The Builder is implemented on top of them, so the two surfaces stay in sync. Finish() returns the assembled core alongside the server for the same reason.

The one place I didn't fully collapse: Finish() returns (srv, core, cleanup, err) — a single cleanup rather than swallowing it — because srv.Stop() doesn't release everything BuildServerConfig acquires (rate-limit middleware, embedded auth server). It's one cleanup instead of the previous three.

cli.Serve and the example embedder now assemble via the Builder (dogfooded as the single entry point), and the operator lifecycle E2E passes on a Kind cluster running the branch's image. Happy to adjust the shape further if you'd prefer the primitives unexported.

[jerm-dro]

Thanks for turning this around so fast — the Builder landed the "construct once / wire once" pass exactly as hoped. Keeping the config copy + subject-provider injection in one place (so the core client and session factory can't disagree) and pulling the elicitation bind inside Finish() are both clean, and building telemetry from cfg.Telemetry by default is a nice touch.

To be clear, I'm not asking for any changes here — just thinking out loud, since I found the public-surface question interesting. One thread I keep pulling on: keeping BuildCore/BuildServerConfig public is the one spot that leaves an assembly path stateless. Two standalone calls can't share a single registry, so an embedder who composes them directly gets back the things the Builder exists to prevent — double discovery, a divergent core/server backend view in dynamic mode, "pass the same opts to both," and separate cleanups.

What I couldn't talk myself out of is that each composition case you cited seems expressible as a Builder method or return value rather than an exported primitive:

• Reuse the assembled core — Finish() already returns it, fully wired.
• Custom serve loop — Finish() returns the un-started *Server, so the caller already owns Start/Stop.
• Inspect/modify the ServerConfig before serving — a registered hook, e.g. b.ConfigureServer(func(*ServerConfig)) invoked inside the single pass, rather than a getter that hands the struct out mid-assembly. (Or targeted options for the fields people actually change — host/port/TTL already are.)

Concretely, what I'm imagining is a public surface of just NewBuilder and the Builder's own methods (Decorate, an optional ConfigureServer, Finish) plus the options — with BuildCore/BuildServerConfig (and LateBoundElicitationRequester) becoming private. That's the version that fully closes the divergence: one owner, no stateless side door. You may well have a consumer in mind that needs the free-function form — something wanting the ServerConfig for a non-Serve path, say — in which case the current surface is the right call. wdyt?

[jerm-dro]

Approving. Remaining items are all non-blocking follow-ups: the public-surface question in the comment above, the static/dynamic registry double-build, and unit coverage for the telemetry-from-config and don't-mutate-caller-config paths.

[JAORMX]

Multi-agent panel review 🤖

Reviewed by a 5-axis panel (correctness · security · architecture/API · tests · vMCP anti-patterns). Non-blocking COMMENT.

Consensus: approve-with-nits. No critical/high bug or vulnerability on the production CLI/operator path (still validated and unchanged). This is a strong refactor — it removes two documented mirrors and shrinks cli/serve.go from ~700 to ~360 lines. Findings below are prioritized; the tests axis alone leaned toward request-changes.

Highest priority

1. Registry "construct once" divergence + a reachable ordering asymmetry (correctness + vMCP, medium)
The Builder builds a single shared registry only for the discovered (K8s) source. For static/dynamic modes, BuildCore and BuildServerConfig each resolve and run discovery independently, producing two snapshots (core aggregates against one, the session manager connects against another) — contradicting the Builder's "construct once" docstring. Compounding this, the two resolvers check branches in different order:

• resolveBackendRegistryForCore (build_core.go:242-257) tests len(cfg.Backends) > 0 before discovered.
• resolveBackendRegistryForServerConfig (build_server_config.go:831-857) tests discovered first.

validator.go has no cross-field rule rejecting non-empty Backends together with source: discovered, so an advanced embedder passing that config gets success from one primitive and errDiscoveredModeRequiresRegistry from the other — despite both doc comments claiming identical behavior.
Suggested fix: build the registry once for all modes and inject via WithBackendRegistry; or make the branch order identical and add the validator cross-field rule.

2. New assembly API skips validation → default-open auth (security, medium)
The public assembly path never calls vmcpconfig.NewValidator().Validate. resolveIncomingAuth silently defaults a nil IncomingAuth to anonymous, and deriveAuthzConfig returns nil (allow-all) — a combination the validator explicitly rejects (validator.go:143). An embedder building a config programmatically and calling Finish() gets a fully unauthenticated, allow-all vMCP fronting every backend, instead of a loud startup error. The shipped internal/exampleembedder passes config straight through, making the unvalidated path the path of least resistance. (Production CLI/operator still validates, so this is a hardening gap in the new public contract, not a shipped vuln.)
Suggested fix: validate inside BuildCore/BuildServerConfig/Finish, or fail loudly on nil IncomingAuth (per go-style.md "fail loudly"); validate in the example embedder.

3. SyncRegistry — the PR's own "extra scrutiny" seam — has zero unit/integration coverage (tests, high)
k8s/manager.go:289 (SyncRegistry: direct-client construction, per-workload conversion, skip-on-error, skip-nil, upsert tolerance, seeded count) is exercised only by the added operator E2E. A regression in the skip/count/upsert logic would silently reintroduce the transiently-empty-registry window this PR fixes, and no unit test would catch it. envtest infra already exists (backend_reconciler_integration_test.go). Related test gaps: TestNewKubernetesBackendRegistry_StartsEmpty now passes only incidentally and its name/comment describe the pre-PR mechanism; decorator ordering (applyCoreDecorators), deriveHealthMonitorConfig happy path, and the embedded-auth-server branch are untested; TestBuildCore_WithInjectedRegistry_SkipsDiscovery / ..._SharedCollaboratorBuiltOnce over-claim with AnyTimes().

Worth addressing

4. docs/arch contradicts the new "primary entry point" (architecture, medium) — docs/arch/vmcp-library.md now calls Builder the primary entry point, but the "Library Embedding Pattern" prose still hand-wires server.New(...) — the exact wiring this PR encapsulates. Add a Builder-based embedding example; note server.New as the low-level alternative.

5. Speculative public surface (vMCP medium / architecture low) — Three exported entry points (Builder + BuildCore + BuildServerConfig), but only Builder has production consumers; the primitives are consumed only by Builder (same package) and tests. Consider unexporting them until a real embedder needs the split, or explicitly marking the surface speculative in the stability table.

Lower / nits

• Doubled side-effecting discovery (EnsureDefaultGroupExists + Discover) on the local CLI path — same root as #1.
• Builder stores context.Context in a struct field (documented tradeoff).
• WithBackendRegistry panics on nil while every other With* option is a total function — asymmetric.
• Exporting LateBoundElicitationRequester/Bind makes mcp-go's two-phase-lifecycle race window permanent public surface (correctly marked Experimental); redundant Bind/bind + dual constructors.
• Helpers thread the full *vmcpconfig.Config with nested nil-guards instead of the specific sub-struct (mirrors pre-existing derive.go).
• header_forward_env.go lost the doc sentence explaining secret-backed header values arrive via secretKeyRef and never enter the operator's view (logic unchanged).

Positives

Removes the documented mirror in backendregistry (closing the transiently-empty-registry window); SyncRegistry reuses the reconciler's conversion instead of duplicating it (no new mirror); god-function removed from the CLI layer; SPDX/naming/%w-wrapping conform; the redis-session E2E Eventually change is a sound de-flake.

_{Generated with Claude Code — multi-agent panel review.}

[tgrunnagle]

Thanks for the panel review — really thorough. Addressed all the recommended findings across three focused commits; details below (finding numbers match the review).

Fixed

#1 — registry "construct once" divergence + ordering asymmetry — c8792b1e
The Builder now resolves the backend registry once for every mode (static, dynamic, and discovered) and injects it into both derivations via WithBackendRegistry, so core aggregation and the session manager operate on a single snapshot — the "construct once" docstring now holds for all modes, not just discovered. The two standalone-primitive resolvers now check branches in an identical order (injected → discovered → static → dynamic) so they can no longer disagree, and validator.go gained a cross-field rule rejecting a non-empty Backends list together with outgoingAuth.source: discovered.

#2 — assembly API skipped validation → default-open auth — c8792b1e
BuildCore and BuildServerConfig (and therefore Finish) now run vmcpconfig.NewValidator().Validate(cfg) up front, so a programmatic embedder gets a loud ErrInvalidConfig instead of a nil IncomingAuth silently degrading to anonymous + allow-all. The example embedder inherits this transitively.

#3 — SyncRegistry had zero unit coverage — c496098f
Extracted SyncRegistry's per-workload conversion+upsert loop into seedRegistryFromDiscoverer and unit-tested it against a mock discoverer (k8s/seed_registry_test.go): list-error-is-fatal, empty group, happy-path seeded count, skip-on-conversion-error, and skip-nil-backend. Also renamed the two over-claiming tests you flagged — TestNewKubernetesBackendRegistry_StartsEmpty → ..._ReturnsRegistryAndWatcher (with a corrected comment: the empty result now reflects an unreachable fake API, not a by-design empty start) and TestBuildCore_..._SharedCollaboratorBuiltOnce → ..._InjectedRegistryUsedByBothBuilds — and added TestDeriveHealthMonitorConfig (disabled + happy paths, default and explicit timeout + circuit breaker) and TestValidator_RejectsBackendsWithDiscoveredSource.

#4 — docs/arch hand-wired server.New — f5e2c904
docs/arch/vmcp-library.md now leads with an app.Builder embedding example as the recommended path and repositions server.New as the low-level composition root (noting brood-box currently uses it; both remain supported).

#5 (partial) — speculative surface marked in the stability table — f5e2c904
The stability table now explicitly flags BuildCore/BuildServerConfig/Option as speculative lower-level primitives that may be reshaped or unexported once the Builder covers observed embedder needs. (Kept public — see rebuttal below.)

#6 — doubled side-effecting discovery on the local CLI path — c8792b1e
Same root as #1: with the registry built once and injected, the CLI path no longer runs discovery twice.

#9 — redundant bind/Bind + dual constructors — f5e2c904
Dropped the unexported newLateBoundElicitationRequester/bind aliases; the app package consumes the exported NewLateBoundElicitationRequester/Bind directly. (The type stays exported + Experimental, as you noted — that's intentional, it's what lets pkg/vmcp/app assemble the late-bound elicitation seam.)

#11 — header_forward_env.go lost doc sentence — f5e2c904
Restored the sentence noting secret-backed header values are delivered via valueFrom.secretKeyRef and never enter the operator's view (logic unchanged).

Kept as-is (with rationale)

#5 — unexporting BuildCore/BuildServerConfig. Keeping these public is a deliberate design decision: the Builder is the primary/supported entry point, but the primitives stay exported for advanced downstream composition (e.g. an embedder that needs to inspect the derived *server.ServerConfig before serving, or reuse the assembled core.VMCP). They're now clearly marked speculative so we retain the freedom to reshape them, without foreclosing that use case today.

#7 — Builder stores context.Context in a struct field. Documented tradeoff (and you flagged it as such). The Builder is a short-lived, single-shot object — NewBuilder(ctx, …) → Decorate → Finish() — where the stored ctx is exactly the build-operation context threaded straight into BuildCore. containedctx isn't enabled, and threading ctx through every fluent method would hurt the builder ergonomics for no lifecycle benefit.

#8 — WithBackendRegistry panics on nil. Intentional asymmetry: a nil registry is a programmer error at wire time (fail-loudly per go-style.md), whereas the other With* options accept any well-typed value. Failing at the option call site gives a clearer stack than a deferred nil-deref during build.

#10 — helpers thread the full *vmcpconfig.Config. This mirrors the pre-existing derive.go pattern; narrowing to per-subsystem sub-structs is a worthwhile cleanup but is broader than this PR and would touch the existing code it mirrors — better as a focused follow-up so it can be reviewed against the anti-pattern rule on its own.

All unit tests + lint (0 issues) pass.