Skip to content

P3.2b: Remove dead legacy (s.core == nil) path after the New/Serve reroute #5621

Description

@tgrunnagle

Description

Now that #5445 (PR #5556) has merged, Serve is the sole *Server constructor and always
sets the core, so the entire legacy s.core == nil path in pkg/vmcp/server — and the
session-factory aggregation it fed — is unreachable dead code. Remove it. This is the
A1 dead-code / AC2 "factory never aggregates" cleanup that #5445 deferred, expanded per the
302a approval review to cover the whole legacy path.

Per @jerm-dro's 302a review:
delete the discovery middleware, backend_enrichment, GetAdaptedTools, and the
session-factory aggregation/composite/telemetry mirrors — not just the annotation_enrichment
already removed in #5556.

Context

server.Newcore.New + Serve is the single live path; s.core is never nil. The dead
surface spans ~1000+ LOC across ~6 packages and includes an API change (dropping the
discoveryMgr param from server.New, 7→6 — brood-box coordination) plus an
anti-pattern-#1 untangling (pkg/vmcp/router/default_router.go still reads the discovery
context seam). To keep each change reviewable and isolate the risky discovery/router work,
it is delivered in stages:

  • Stage 1 — unconditional core (first PR, 302b): collapse the dead s.core == nil
    branches; delete backend_enrichment.go, the legacy handleSessionRegistration
    registration tail, and the orphaned Manager.GetAdaptedTools/GetAdaptedResources.
    No API/signature change; no discovery/router touch.
  • Stage 2 — factory mirrors (follow-up): remove session.WithAggregator + the factory
    aggregator/buildRoutingTableWithAggregator, Aggregator.ProcessPreQueriedCapabilities,
    and the sessionmanager composite/telemetry decorators the core now owns.
  • Stage 3 — discovery seam (follow-up): remove the discovery middleware + context seam,
    the discoveryMgr field/param (server.New 7→6), the cli wiring, and untangle
    default_router.go; delete the discovery package if then dead.

Acceptance Criteria

Stage 1 (first PR):

  • The s.core == nil branches in (*Server).Handler, handleSessionRegistrationImpl,
    lazyInjectSessionTools, and the readiness/status helpers are removed; s.core is
    treated as always non-nil.
  • backend_enrichment.go (+ test) deleted.
  • Manager.GetAdaptedTools/GetAdaptedResources (+ session-manager interface methods)
    deleted (no remaining callers).
  • No server.New signature change in this PR.
  • task test passes; task lint clean; PR ≤ 400 LOC / 10 files (excl. tests).

Follow-up stages (tracked here, separate PRs): factory-mirror removal; discovery seam +
default_router untangle + server.New signature change.

Parent Story: #5432
Follows: #5445 (PR #5556)

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs-triageIssue needs initial triage by a maintainerrefactorvmcpVirtual MCP Server related issues

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions