Remove dead session-factory aggregation and composite mirrors#5625
Merged
Conversation
The core is the single source of capability aggregation on the Serve path (AdvertiseFromCore), so the session factory's parallel aggregation is dead code. Part of the #5621 Stage 2 dead-factory-mirror removal: - session/factory.go: remove the `aggregator` field, `WithAggregator`, `buildRoutingTableWithAggregator`, and the `if f.aggregator != nil` branch — makeBaseSession always builds the routing table from raw backend capabilities (no overrides/conflict- resolution/filter; the core owns that). makeBaseSession's now-always-nil error return is dropped (+ its two callers). - aggregator: remove `Aggregator.ProcessPreQueriedCapabilities` (interface + impl + regenerated mock) — its only caller was buildRoutingTableWithAggregator. - cli/serve.go: inline the trivial createSessionFactory to NewSessionFactory and drop the WithAggregator(agg) wiring (agg still feeds the core via Config.Aggregator). Part of #5621. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The core owns composite-tool execution (executeComposite) and workflow telemetry (core_telemetry.go's telemetryComposer, #5561), and nothing sets FactoryConfig.WorkflowDefs/ ComposerFactory on the Serve path (server.New and the integration harness leave them unset), so the session factory's composite-tool decorator and its workflow-executor telemetry never run. Remove them — the second half of #5621 Stage 2: - sessionmanager/factory.go: delete compositeToolsDecorator, composerWorkflowExecutor, workflowExecutorInstruments + newWorkflowExecutorInstruments + wrapExecutor, and telemetryWorkflowExecutor; drop the instruments param from buildDecoratingFactory. The optimizer decorator and its telemetry (monitorOptimizer/telemetryOptimizer) are untouched. - FactoryConfig: drop the dead WorkflowDefs + ComposerFactory fields. - session_manager.go: drop the workflow-instruments setup and the now-dead "ComposerFactory required when WorkflowDefs set" validation. Part of #5621. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
JAORMX
approved these changes
Jun 24, 2026
jerm-dro
approved these changes
Jun 24, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5625 +/- ##
==========================================
+ Coverage 69.93% 70.13% +0.20%
==========================================
Files 650 652 +2
Lines 66289 66277 -12
==========================================
+ Hits 46358 46484 +126
+ Misses 16588 16434 -154
- Partials 3343 3359 +16 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
With the core established as the single source of capability aggregation, composite-tool
execution, and workflow telemetry on the Serve path (#5445), the session factory's parallel
aggregation/composite/telemetry machinery is unreachable dead code. This is Stage 2 of the
#5621 dead-code removal.
ce4dce6a): removesession.WithAggregator, the factoryaggregatorfield +
buildRoutingTableWithAggregator, andAggregator.ProcessPreQueriedCapabilities(interface + impl + regenerated mock).
makeBaseSessionalways builds the routing table fromraw backend capabilities (the core owns overrides/conflict-resolution/filtering); its
now-always-nil error return is dropped.
cli/serve.godrops theWithAggregator(agg)wiring(agg still feeds the core via
Config.Aggregator).549caf74): removecompositeToolsDecoratorand theworkflowExecutorInstrumentsfamily from the session manager, plus the deadFactoryConfig.WorkflowDefs/ComposerFactoryfields. The core owns composites(
executeComposite) and workflow telemetry (telemetryComposer, Forward passthrough headers on the Serve path #5561). The optimizerdecorator and its telemetry are untouched.
Verified dead: nothing sets
FactoryConfig.WorkflowDefs/ComposerFactory(production and theintegration harness leave them unset), and
ProcessPreQueriedCapabilities's only caller was theremoved factory aggregation.
Part of #5621.
Type of change
Test plan
task test)task lint)go test ./test/integration/vmcp/...)Build clean;
pkg/vmcp/...+test/integration/vmcp/...pass; lint clean except thepre-existing, unrelated
G115. Pure deletion of unreachable code (10 files, +14/−809).API Compatibility
session.WithAggregatorandAggregator.ProcessPreQueriedCapabilitiesare removed from theexported surface; their only in-tree users were the factory aggregation path (removed here) and
cli/serve.go.server.New's signature is unchanged — thediscoveryMgr-param change isStage 3 (#5621).
Does this introduce a user-facing change?
No.
Special notes for reviewers
Stage 2 of 3 in the #5621 dead-code removal (follows the merged Stage 1, #5622). The
optimizer decorator and its telemetry (
monitorOptimizer/telemetryOptimizer) are intentionallyretained — out of scope here. Stage 3 removes the discovery middleware + context seam, untangles
default_router, and changes theserver.Newsignature (7→6, droppingdiscoveryMgr).Generated with Claude Code