RFC-0058: Inline aggregator into session factory and extract filter decorator

[jerm-dro]

Summary

Proposes decomposing the monolithic Aggregator interface by:

• Inlining its pipeline (overrides, conflict resolution, routing table construction) into the session factory
• Extracting the advertising filter as a session decorator above composite tools

This fixes toolhive#4287 where composite tool workflows fail type coercion for non-advertised backend tools, and simplifies the vMCP architecture by removing a confusing middleman layer.

Key decisions

• Renaming stays in the factory — can't be a decorator because CallTool routing requires private session state
• Filter becomes a decorator — sits above composite tools so they see all tool schemas
• Aggregator package becomes a utility library — conflict resolvers and override helpers stay, the interface and struct are deleted
• Depends on PR #4231 landing first (DecoratingFactory pattern)

🤖 Generated with Claude Code

[yrobla]

Good RFC — the problem statement is precise, the motivation is well-justified, and the decorator-stack diagram makes the target architecture clear. I have two findings that need addressing before this is ready to implement, plus one API clarification.

Summary of findings:

• 🔴 The sess.CallTool() path doesn't handle dot-convention names — the RFC claims it already does, but it doesn't. This needs to be specified as an explicit change.
• 🟡 The filter decorator's shouldAdvertise will match against resolved names (potentially prefixed), while the current implementation matches against post-override, pre-prefix names. This is a silent behavioral change for anyone using both prefix conflict resolution and filter. Needs to be specified or explicitly called out as a behavior change.
• 🟢 WithAggregationConfig signature description is self-contradictory about who owns the conflict resolver.

[yrobla]

🔴 Dot-convention support is missing from sess.CallTool()

The RFC says:

The ResolveToolName call in getToolInputSchema (for dot-convention support) moves to the session's CallTool implementation, which already handles this via the session router.

This is not accurate. defaultMultiSession.CallTool delegates to lookupBackend, which does an exact key lookup against the routing table:

// pkg/vmcp/session/default_session.go
target, ok := table[capName]  // exact match only — no dot-convention

Dot-convention resolution (backend-a.echo → backend-a_echo) lives in sessionRouter.RouteTool() and ResolveToolName(). The base session's CallTool does not call through the router at all.

If the workflow engine calls sess.CallTool(ctx, "backend-a.echo", args, nil), it will get ErrToolNotFound because "backend-a.echo" is not a routing table key.

This is a real gap: the RFC needs to either:

1. Add dot-convention handling to defaultMultiSession.CallTool (e.g., call through the session router before the exact lookup) — and describe this as an explicit change to the session package, or
2. Have the workflow engine resolve the tool name to its routing-table key before calling sess.CallTool() — keeping the resolution in the engine but calling through the session for the actual dispatch.

Option 1 is cleaner and consistent with the stated goal of having the session encapsulate routing, but it needs to be called out explicitly in the Detailed Design section.

[jerm-dro]

Thanks for catching this. Why do composite tools need the dot notation at all? Option 1 sounds good to me.

[yrobla]

🟡 shouldAdvertise will match against resolved names — behavior change for prefixed tools

The current shouldAdvertiseTool is called with rt.OriginalName, which is the post-override, pre-conflict-resolution name:

// pkg/vmcp/aggregator/default_aggregator.go:539
if a.shouldAdvertiseTool(rt.BackendID, rt.OriginalName) {

So today, a filter config ["echo"] matches a tool even when prefix conflict resolution renames it to backend-a_echo in the routing table.

In the filter decorator, the pseudocode iterates over sess.Tools() and calls d.shouldAdvertise(t). The tool's Name at that point is the conflict-resolved name (backend-a_echo). A filter config ["echo"] would no longer match — silent breakage.

The RFC acknowledges this partially with "can be matched by stripping the known prefix" but doesn't show the implementation. The shouldAdvertise method needs to be fully specified. Two reasonable options:

1. Match against original name — store the pre-prefix name on vmcp.Tool (e.g., add an OriginalName field, or reuse an existing metadata field) so the decorator can compare it against the filter config.
2. Match against resolved name — explicitly document this as a behavior change and update the configuration schema docs accordingly.

The Row 9 test (virtualmcp_overrides_filter_test.go) will expose this at E2E level, but it shouldn't be left to the test to define the semantics. Please specify the intended matching behavior here.

Note: this is a zero-backend-config impact change (filter only applies when filter: [...] is configured alongside a conflict resolver), but it should still be explicit.

[yrobla]

🟢 WithAggregationConfig description is self-contradictory

The RFC says:

WithAggregationConfig(cfg *config.AggregationConfig, cr ConflictResolver) — required conflict resolver and aggregation config. The conflict resolver always exists (NewConflictResolver defaults to prefix strategy).

These two sentences contradict each other. "Required" implies the caller must pass it; "always exists (NewConflictResolver defaults)" implies the factory creates one internally and the parameter may be optional or the factory always builds a default.

Suggested clarification: if the factory option always expects an explicit ConflictResolver (and callers always construct one via NewConflictResolver), say so. If nil triggers a default, specify that. Whichever you choose, update the sentence to be consistent.

[yrobla]

depending on the urgency we have for fixing it, keeping aggregator is not that bad at all... we could first do some fix for the filtering itself, and they proceed with aggregator refactor in next prs

[jerm-dro]

This is not very urgent, but I wanted to share how I think we should continue to simplify vMCP. I like the idea of doing smaller changes that move us in this direction.