P2.5 Move AS runner, status reporter, optimizer, health monitor under Serve #5443

Description

Relocate the remaining transport lifecycle concerns under Serve's Start/Stop
orchestration: the embedded authorization-server (AS) runner, the status reporter +
periodicStatusReporting goroutine, the optimizer cleanup, and the health monitor's
lifecycle. This is the final Phase 2 relocation — after it, Serve owns every
transport subsystem's start/stop. Crucially, Serve does not construct the
health.Monitor: because the Phase-3 wrapper is
Serve(ctx, New(deriveCoreConfig(cfg)), deriveServerConfig(cfg)), New is evaluated
before Serve, so the monitor must already exist when the core is constructed. The
monitor is therefore built at the composition root, injected into the core as a
health.StatusProvider (#5437), and handed to Serve as a built *health.Monitor
via ServerConfig so Serve owns only its Start/Stop lifecycle. Teardown order and
shutdownFuncs are preserved byte-for-byte; server.New's signature and observable
behavior are unchanged.

Context

Part of the vMCP interface refactor (epic #5419), which splits the server.New
god-object into a core (New(cfg) -> VMCP) and a transport (Serve(ctx, VMCP, serverCfg) -> *Server). #5439 landed the Serve skeleton + ServerConfig;
#5440/203/204 relocated the SDK hooks, the middleware chain, and the
discovery-into-context path. This task moves the last lifecycle pieces — AS runner,
status reporter goroutine, optimizer cleanup, and the health-monitor lifecycle — into
Serve's orchestration.

Health-monitor ownership & construction order (A2). Today a single server.New
builds the monitor (server.go:436-450) and uses it at Handler-time (server.go:625-634)
— there is no separate "core" object needing it at construction, so no ordering tension
exists. The New/Serve split introduces one: the core's filterHealthyBackends needs a
health.StatusProvider at construction, but New runs before Serve. Resolution
(per architecture.md R5/R7 and the resolved review decision): build the health.Monitor
once at the composition root (the server.New wrapper in #5445; the test harness
in standalone Phase-2 tests), inject it as a StatusProvider into New (#5437), and
pass the same *health.Monitor into ServerConfig so Serve owns its Start/Stop
lifecycle. "Owned by Serve" therefore means lifecycle, not construction. The old
server.go:625-634 injection was a Handler-time inject into the monolith; it does
not translate to injecting into a separately-constructed core, so this task does not
reproduce it — the StatusProvider reaches the core at New-time instead.

See architecture.md "Serve (transport) wiring relocated" (lines 285-290), "Health monitor
… owned by Serve, injected into the core as a health.StatusProvider" (lines 297-299),
and "Statelessness, Session Consistency & Health Filtering (R5/R7)" (305-329); research.md
health-filtering findings.

Parent Story: #5431
Dependencies: #5439 (the Serve skeleton + ServerConfig, whose health field carries the built *health.Monitor)
Blocks: #5444

Split note: This PR MAY SPLIT into health + AS and status + optimizer
if the single PR exceeds the 400 LOC / 10 file limit. Use the /split-pr skill if
so. The two halves remain under #5431 and both fan into #5444 the same way
#5443 does.

Acceptance Criteria

• The embedded AS runner, the status reporter goroutine, the optimizer cleanup, and
  the health monitor's Start/Stop are driven from within Serve's Start/Stop
  orchestration.
• Serve does not construct the health.Monitor; it receives an already-built
  *health.Monitor via ServerConfig (nil ⇒ disabled) and owns only its lifecycle. The
  monitor is built at the composition root so the same instance is injected as a
  health.StatusProvider into the core at New-time (P1.4 New(cfg) -> VMCP core constructor #5437) — honoring that New
  precedes Serve in the wrapper.
• Teardown order and the shutdownFuncs slice are preserved exactly (HTTP shutdown →
  health monitor stop → shutdownFuncs → session manager → discovery manager → session
  data storage close).
• Health-monitoring, status-reporting, and readiness tests stay green
  (health_monitoring_test.go, status_reporting_test.go, readiness_test.go).
• PR is ≤ 400 LOC and ≤ 10 files changed (excluding tests/docs/generated) — or split
  per the note above.
• 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

Move the lifecycle of the four remaining subsystems out of server.New / the
carried-forward (*Server).Start/Stop and into Serve's orchestration, keeping every
behavior identical:

• Health monitor (lifecycle only). Serve receives the built *health.Monitor from
  ServerConfig (field added in P2.1 Serve skeleton + ServerConfig #5439; built at the composition root — the wrapper in
  P3.2 Reduce server.New body to the wrapper #5445, or the test harness here in Phase 2). Serve retains the reference via the
  healthMonitor field (guarded by healthMonitorMu). It does not call
  health.NewMonitor. Start it in Start (mirror server.go:733-749: on start failure,
  log a WARN and set healthMonitor = nil so getters report "disabled"); stop it in
  Stop (mirror server.go:819-828, before draining shutdownFuncs). The construction
  (server.go:436-450: nil-config ⇒ disabled with an INFO log; otherwise
  health.NewMonitor(backendClient, backendRegistry.List(ctx), *cfg.HealthMonitorConfig))
  moves to the composition root, not into Serve — and the resulting StatusProvider
  is injected into the core there (P1.4 New(cfg) -> VMCP core constructor #5437). A nil monitor (disabled) is valid; the core
  treats a nil StatusProvider as "no health filtering" (all backends included), matching
  today's no-monitor behavior.
• Embedded AS runner — register its routes during mux construction when
  cfg.AuthServer != nil (mirror server.go:568-572: AuthServer.RegisterHandlers(mux)
  • the debug log). Keep this within the unauthenticated-route section of the mux that
    Serve already builds.
• Status reporter + periodicStatusReporting — in Start, when
  cfg.StatusReporter != nil, call StatusReporter.Start(ctx), append the returned
  shutdown to shutdownFuncs, derive a cancelable statusReportingCtx, launch
  go s.periodicStatusReporting(statusReportingCtx, statusConfig) (config from
  DefaultStatusReportingConfig() overridden by cfg.StatusReportingInterval), then
  append the cancel to shutdownFuncs after the goroutine starts (preserve the
  ordering comment about the Stop-called-immediately race at server.go:773-778).
• Optimizer cleanup — sessionmanager.New (server.go:462) returns an
  optimizerCleanup; when non-nil, append it to shutdownFuncs (mirror
  server.go:485-487) at the point Serve assembles the session manager.

Keep periodicStatusReporting and DefaultStatusReportingConfig in
status_reporting.go unchanged (only the call site moves). Do not alter the
StatusProvider interface or the health.Monitor API. The HTTP server start/serve
skeleton itself was already relocated in #5439; this task only moves the four
subsystem lifecycle blocks into it.

Patterns & Frameworks

• Serve owns the lifecycle; the composition root constructs — the monitor is built
  once at the composition root so the same instance can be injected into the core
  (StatusProvider) and lifecycled by Serve; the core receives only a read-only view
  and runs filterHealthyBackends itself (Core Principle: core filters, Serve caches;
  R5/R7; security.md "Route Through Security-Enforcing Components").
• No new god-object — Serve composes pre-built subsystems and reuses the existing
  *Server struct + shutdownFuncs; do not add fields or concerns beyond relocating
  the wiring (vmcp-anti-patterns.md Bump golangci/golangci-lint-action from 2f856675483cb8b9378ee77ee0beb67955aca9d7 to 4696ba8babb6127d732c3c6dde519db15edab9ea #3).
• Resource lifecycle hygiene — every started subsystem has a matching stop:
  shutdownFuncs appends for the status reporter shutdown + cancel and the optimizer
  cleanup; explicit healthMon.Stop(); preserve the existing teardown ordering
  (go-style "Resource Leaks").
• No mcp-go / transport types cross the VMCP boundary — AS routes, status
  reporter, and health monitor are transport concerns and stay in Serve
  (vmcp-anti-patterns.md Implement secret injection #5).
• stdlib testing.T + testify, no Ginkgo in pkg/vmcp/server (R8; testing.md).
  Add SPDX headers to any new file (go-style).

Code Pointers

• pkg/vmcp/server/server.go:683-795 — (*Server).Start; the health-monitor start
  block (733-749) and the status-reporter + periodicStatusReporting block (751-779)
  move into Serve's Start orchestration.
• pkg/vmcp/server/server.go:798-864 — (*Server).Stop; preserve the exact teardown
  order: HTTP Shutdown (804-812) → health monitor Stop (819-828) → shutdownFuncs
  drain (830-835) → session manager (837-842) → discovery manager (844-847) → session
  data storage Close (849-855).
• pkg/vmcp/server/server.go:436-450 — health monitor construction (nil-config
  disabled path, health.NewMonitor, "health monitoring enabled" log). This moves to the
  composition root (P3.2 Reduce server.New body to the wrapper #5445 wrapper / the test harness), not into Serve. Serve
  receives the built *health.Monitor via ServerConfig.
• pkg/vmcp/server/server.go:625-634 — today's Handler-time injection of healthMon as
  a health.StatusProvider into the handler/core factory. In the split this does not
  translate: the StatusProvider is injected into the core at New-time (P1.4 New(cfg) -> VMCP core constructor #5437), not at
  Handler-time. Do not reproduce this injection in Serve.
• pkg/vmcp/server/server.go:462 / :485-487 — sessionmanager.New returns
  optimizerCleanup; appended to shutdownFuncs when non-nil.
• pkg/vmcp/server/server.go:568-572 — embedded AS routes
  (s.config.AuthServer.RegisterHandlers(mux)) registered during mux construction;
  AuthServer is *asrunner.EmbeddedAuthServer (server.go:134).
• pkg/vmcp/server/status_reporting.go:30 (DefaultStatusReportingConfig), :44
  (periodicStatusReporting) — unchanged; only the call site moves.
• pkg/vmcp/health/monitor.go:42 — StatusProvider interface (do not modify);
  :246/:298 — Monitor.Start/Stop. NewMonitor is called at the composition root.
• pkg/vmcp/server/serve.go — the Serve orchestrator (added in P2.1 Serve skeleton + ServerConfig #5439) that gains
  these lifecycle blocks; its ServerConfig carries the built *health.Monitor (P2.1 Serve skeleton + ServerConfig #5439).

Component Interfaces

Shapes only — these are the existing signatures whose call sites relocate into
Serve (for lifecycle) or the composition root (for construction); do not change the
signatures themselves. Use literal placeholders for existing package-aliased types.

// pkg/vmcp/health — unchanged. The Monitor is BUILT at the composition root; Serve
// receives the built *Monitor (lifecycle) and the core receives it as a StatusProvider.
type StatusProvider interface {
    QueryBackendStatus(backendID string) (vmcp.BackendHealthStatus, bool)
    // ... existing methods (unchanged)
}
func NewMonitor(client BackendClient, backends []vmcp.Backend, cfg MonitorConfig) (*Monitor, error)
func (m *Monitor) Start(ctx context.Context) error
func (m *Monitor) Stop() error

// pkg/vmcp/server — unchanged; only the call site moves into Serve's Start.
func (s *Server) periodicStatusReporting(ctx context.Context, config StatusReportingConfig)
func DefaultStatusReportingConfig() StatusReportingConfig

// Composition root (#5445 wrapper / Phase-2 test harness), NOT Serve:
//   mon, _ := health.NewMonitor(backendClient, backendRegistry.List(ctx), *cfg.HealthMonitorConfig) // nil if disabled
//   core, _ := vmcp.New(deriveCoreConfig(cfg, ...).withStatusProvider(mon))  // mon injected at New-time
//   srv, _  := Serve(ctx, core, deriveServerConfig(cfg).withHealthMonitor(mon)) // same *Monitor for lifecycle
//
// pkg/vmcp/server/serve.go — Serve's Start gains (sketch, not full impl):
//   - take cfg.HealthMonitor (*health.Monitor, may be nil); store on s.healthMonitor; do NOT build it
//   - if s.healthMonitor != nil: s.healthMonitor.Start(ctx) (WARN+nil on failure)
//   - if cfg.AuthServer != nil: cfg.AuthServer.RegisterHandlers(mux)   // during mux build
//   - if optimizerCleanup != nil: s.shutdownFuncs = append(s.shutdownFuncs, optimizerCleanup)
//   - if cfg.StatusReporter != nil:
//       shutdown, _ := cfg.StatusReporter.Start(ctx); append to shutdownFuncs
//       ctx, cancel := context.WithCancel(ctx); go s.periodicStatusReporting(ctx, statusConfig)
//       append(cancel) to shutdownFuncs AFTER launching the goroutine

Testing Strategy

Unit Tests (pkg/vmcp/server, stdlib testing.T + testify)

• health_monitoring_test.go stays green: disabled path (nil *health.Monitor in
  ServerConfig), enabled path (a pre-built monitor is started), startup-failure graceful
  degradation (healthMonitor set to nil on start error), and
  TestServer_Stop_StopsHealthMonitor (monitor stopped, struct retained) — now exercising
  the Serve-lifecycled monitor it was given, not one it built.
• status_reporting_test.go stays green: periodicStatusReporting and
  DefaultStatusReportingConfig behavior unchanged; the status reporter Start/shutdown
  • cancel are appended to shutdownFuncs and run on Stop.
• readiness_test.go stays green: /readyz semantics unchanged after relocation.
• Optimizer cleanup (sessionmanager.New non-nil cleanup) is invoked on Stop via
  shutdownFuncs.
• Serve does not call health.NewMonitor (assert via a constructor spy / no-build);
  a nil cfg.HealthMonitor yields disabled getters without error.

Integration / Behavioral Parity Tests

• The server.New-driven parity suite (server_test.go, integration_test.go)
  stays green — server.New signature/behavior unchanged.
• Embedded AS routes are registered when AuthServer is configured (route-presence
  assertion mirroring today).

Edge Cases

• Teardown order preserved when multiple subsystems are active (verify
  shutdownFuncs and explicit stops fire in the documented order, errors joined).
• Stop called immediately after Start does not race the status-reporting
  goroutine (cancel appended after goroutine launch — preserve server.go:773-778).
• A pre-built monitor whose Start fails disables monitoring (getters report
  disabled) without failing Serve.Start.

Out of Scope

• Routing server.New through Serve (Phase 3 — P3.1 deriveCoreConfig/deriveServerConfig config split #5444/P3.2 Reduce server.New body to the wrapper #5445).
• The server.Config → core Config / ServerConfig split (P3.1 deriveCoreConfig/deriveServerConfig config split #5444), incl. adding the
  *health.Monitor field to ServerConfig (that field is established in P2.1 Serve skeleton + ServerConfig #5439; the
  composition root that builds the monitor is P3.2 Reduce server.New body to the wrapper #5445).
• Any change to the health.StatusProvider interface or the health.Monitor /
  status-reporter / optimizer APIs (lifecycle relocation only).
• The SDK hooks (P2.2 Move SDK hooks + two-phase session creation under Serve #5440), middleware chain (P2.3 Move middleware chain under Serve; remove authz + annotation mw #5441), and discovery-into-context
  replacement (P2.4 Replace discovery-into-context with direct VMCP calls #5442) — already handled by sibling tasks.
• Any change to server.New's 7-param signature or observable behavior.

References

• RFC THV-0076: /Users/trey/Documents/GitHub/stacklok/toolhive-rfcs/rfcs/THV-0076-vmcp-core-interface.md
• architecture.md "Serve (transport) wiring relocated" (285-290); "Health monitor …
  owned by Serve, injected into the core as a health.StatusProvider" (297-299);
  "Statelessness, Session Consistency & Health Filtering (R5/R7)" (305-329).
• research.md — health-filtering findings; carried-forward Server methods.
• Parent story: Phase 2: Serve transport helper, re-home transport, replace discovery #5431
• Epic: vMCP interface refactor #5419