Remove dead vMCP discovery seam and default router#5627
Conversation
The legacy server.New path (s.core == nil) became unreachable once Serve always builds the core: every server now routes capability aggregation and call admission through the core. The discovery middleware, its context seam, the discovery.Manager, and the context-coupled defaultRouter were only reachable on that dead path. Delete the pkg/vmcp/discovery package and router.defaultRouter, drop the discovery.Manager parameter from server.New (7 -> 6 params), and point the CLI at an empty SessionRouter (core.New needs a non-nil router only for workflow validation, which does not route). Update the affected tests and the comments that referenced the removed code. Part of #5621 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
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 transformationAlternative:
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 Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5627 +/- ##
==========================================
- Coverage 70.04% 70.03% -0.02%
==========================================
Files 652 648 -4
Lines 66392 66208 -184
==========================================
- Hits 46505 46367 -138
+ Misses 16544 16503 -41
+ Partials 3343 3338 -5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Large PR justification has been provided. Thank you!
|
✅ Large PR justification has been provided. The size review has been dismissed and this PR can now proceed with normal review. |
Summary
Why: Once
Servebecame the sole*Serverconstructor and always builds the domain core, the legacyserver.Newpath (s.core == nil) became unreachable. Capability aggregation and call admission now run entirely through the core, so the discovery middleware, its context seam, thediscovery.Manager, and the context-coupleddefaultRouterwere dead code reachable only on that path. This is the final stage of the staged dead-code removal under #5621 (after #5622 and #5625).What:
pkg/vmcp/discoverypackage (manager, middleware, context seam, mocks) — it had zero importers after the seam removal.router.defaultRouter(+ test), whose only coupling was reading discovered capabilities fromcontext;core.Newonly needs a non-nil router for workflow validation, which does not route, so the CLI now passes an emptyNewSessionRouter(&vmcp.RoutingTable{}).discovery.Managerparameter fromserver.New(7 → 6 params) and remove the field, assignment, thes.core == nildiscovery-middleware leg inHandler(), and the discoveryStop()call.mockDiscoveryMgrsetup, swap routers) and the comments that referenced the removed code (including the e2e circuit-breaker note and the corefilterHealthyBackendsdoc, now the single source of truth for health filtering).Net: +86 / −2315 across 25 files (1941 lines are whole-file deletions).
Part of #5621
Type of change
Test plan
go build ./...— cleango vet ./...— cleango test -raceon all affected packages (pkg/vmcp/server,cli,router,core,aggregator,session,test/integration/vmcp) — all pass, including the goleak-guarded suites (no goroutine leaks)Does this introduce a user-facing change?
No.
Special notes for reviewers
server.Newdrops itsdiscovery.Managerparameter (7 → 6 args). In-tree callers (cli/serve.go, tests, the integration helper) are updated here; an out-of-tree embedder must drop that argument. TheNewdoc comment documents the change.filterHealthyBackendswas a copy of the core's own (pkg/vmcp/core/core_vmcp.go); circuit-breaker / unhealthy-backend filtering is unchanged and still covered byTestFilterHealthyBackends.convertAnnotationsdoc comment inpkg/vmcp/core/admission.goreferences aserver.convertAnnotationscopy that was already removed by an earlier stage; left for a focused follow-up. The vestigialConfig.AuthzMiddlewarefield likewise remains for a separate PR.Large PR Justification
Generated with Claude Code