Address review comments on RFC-0058

- Remove nil conflict resolver path; always use default prefix strategy
- Filter decorator evaluates directly on sess.Tools(), no advertisedSetProvider
- WithConflictResolver always required in factory options
- Move processBackendTools and actualBackendCapabilityName to session package
- Expand backward compatibility table with all config fields
- Remove moot open question about filter name matching
- Rewrite testing strategy as feature combination matrix covering
  filter, renames, conflict resolution, composite tools, optimizer,
  and authn interactions across 11 E2E test scenarios

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: jerm-dro

rfcs/THV-0058-inline-aggregator-filter-decorator.md

@@ -3,7 +3,7 @@
 - **Status**: Draft
 - **Author(s)**: Jeremy Drouillard (@jerm-dro)
 - **Created**: 2026-03-19
-- **Last Updated**: 2026-03-19
+- **Last Updated**: 2026-03-20
 - **Target Repository**: toolhive
 - **Related Issues**: [toolhive#4287](https://github.com/stacklok/toolhive/issues/4287)
 
@@ -69,82 +69,77 @@ Merge `buildRoutingTable` and `buildRoutingTableWithAggregator` into a single fu
 func buildRoutingTable(
     ctx context.Context,
     results []initResult,
-    conflictResolver aggregator.ConflictResolver,
+    conflictResolver ConflictResolver,
     toolConfigMap map[string]*config.WorkloadToolConfig,
     excludeAllTools bool,
-) (*vmcp.RoutingTable, []vmcp.Tool, []vmcp.Resource, []vmcp.Prompt, map[string]bool, error)
+) (*vmcp.RoutingTable, []vmcp.Tool, []vmcp.Resource, []vmcp.Prompt, error)
 ```
 
-When `conflictResolver` or `toolConfigMap` is non-nil:
+A conflict resolver is always provided — `NewConflictResolver(nil)` defaults to prefix strategy with `{workload}_` format, matching current behavior. The pipeline:
 
 1. Group tools by `BackendID`
-2. Apply `aggregator.ProcessBackendTools()` per backend (overrides)
+2. Apply `processBackendTools()` per backend (overrides)
 3. Call `conflictResolver.ResolveToolConflicts()` (conflict resolution)
-4. Build routing table keyed by resolved names, with `OriginalCapabilityName` set via `aggregator.ActualBackendCapabilityName()`
+4. Build routing table keyed by resolved names, with `OriginalCapabilityName` set via `actualBackendCapabilityName()`
 5. Build `allTools` from ALL resolved tools (no filtering)
-6. Compute `advertisedSet map[string]bool` keyed by resolved name — evaluates the filter logic inline
 
-When both are nil: current first-writer-wins logic, `advertisedSet = nil`.
-
-The `advertisedSet` is stored on `defaultMultiSession` as a private field.
+The old `buildRoutingTable` (first-writer-wins, no conflict resolution) is removed entirely — the conflict resolver always handles name conflicts.
 
 #### Filter decorator
 
 `pkg/vmcp/session/filterdec/decorator.go` (new)
 
+The filter decorator evaluates the advertising filter directly on `sess.Tools()` using the tool config. Each tool in the session has `BackendID` set, which is sufficient to evaluate per-workload `excludeAll` and `filter` rules. The filter config references post-override tool names, which match `tool.Name` after conflict resolution when no prefix was applied, or can be matched by stripping the known prefix.
+
 ```go
 type filterDecorator struct {
     sessiontypes.MultiSession
-    advertisedSet map[string]bool
+    toolConfigMap   map[string]*config.WorkloadToolConfig
+    excludeAllTools bool
 }
 
 func (d *filterDecorator) Tools() []vmcp.Tool {
     all := d.MultiSession.Tools()
     result := make([]vmcp.Tool, 0, len(all))
     for _, t := range all {
         // Composite tools (empty BackendID) always pass through
-        if t.BackendID == "" || d.advertisedSet[t.Name] {
+        if t.BackendID == "" || d.shouldAdvertise(t) {
             result = append(result, t)
         }
     }
     return result
 }
 ```
 
-The decorator extracts `advertisedSet` from the base session via a private interface:
-
-```go
-type advertisedSetProvider interface {
-    AdvertisedSet() map[string]bool
-}
-```
-
 `CallTool`, `ReadResource`, `GetPrompt` pass through unchanged — the filter only affects what's advertised via `Tools()`, not what's callable.
 
 #### Decorator wiring
 
 `pkg/vmcp/server/sessionmanager/factory.go` (assumes PR #4231 landed)
 
-Insert filter decorator between composite tools and optimizer:
+Insert filter decorator between composite tools and optimizer. The filter decorator is always applied — it receives the tool config and evaluates advertising decisions against `sess.Tools()` at decoration time:
 
 ```go
 decorators = append(decorators, compositeToolsDecorator(...))
-decorators = append(decorators, filterDecoratorFn())  // extracts advertisedSet from session
+decorators = append(decorators, filterDecoratorFn(toolConfigMap, excludeAllTools))
 decorators = append(decorators, optimizerDecoratorFn(...))
 ```
 
 #### Aggregator package becomes a utility library
 
-Keep:
+Keep in aggregator package:
 - `ConflictResolver` interface + 3 implementations (prefix, priority, manual)
-- `ProcessBackendTools` (exported) — applies per-backend overrides
-- `ActualBackendCapabilityName` (exported) — reverses overrides for backend forwarding
+- `NewConflictResolver` factory function
 - `ResolvedTool`, `BackendCapabilities`, `ResolvedCapabilities` types
 
+Move to session package (implementation detail):
+- `processBackendTools` — used only by the session factory
+- `actualBackendCapabilityName` — used only by the session factory
+
 Delete:
 - `Aggregator` interface, `defaultAggregator` struct
 - `ProcessPreQueriedCapabilities`, `AggregateCapabilities`, `QueryCapabilities`, `QueryAllCapabilities`, `MergeCapabilities`
-- `shouldAdvertiseTool` (logic inlined into factory)
+- `shouldAdvertiseTool` (logic moves to filter decorator)
 - `AggregatedCapabilities`, `AggregationMetadata`, `BackendDiscoverer` types
 
 #### Why renaming can't be a decorator
@@ -156,22 +151,19 @@ Delete:
 **Removed**: `WithAggregator(agg aggregator.Aggregator)` session factory option
 
 **Added**:
-- `WithConflictResolver(cr aggregator.ConflictResolver)` — optional conflict resolver
-- `WithToolConfig(cfg *config.AggregationConfig)` — optional aggregation config (overrides, filters)
-
-Or a single `WithAggregationConfig` option that carries both.
+- `WithAggregationConfig(cfg *config.AggregationConfig, cr ConflictResolver)` — required conflict resolver and aggregation config. The conflict resolver always exists (`NewConflictResolver` defaults to prefix strategy).
 
-**Exported**:
-- `aggregator.ProcessBackendTools` (was `processBackendTools`)
-- `aggregator.ActualBackendCapabilityName` (was `actualBackendCapabilityName`)
+**Moved to session package** (implementation detail, not exported):
+- `processBackendTools` — moves from `aggregator` to `session` package
+- `actualBackendCapabilityName` — moves from `aggregator` to `session` package
 
 ### Configuration Changes
 
 None. All existing configuration fields (`aggregation.excludeAllTools`, `aggregation.tools[].excludeAll`, `aggregation.tools[].filter`, `aggregation.tools[].overrides`) continue to work identically. The implementation moves from the aggregator to the factory + decorator.
 
 ### Data Model Changes
 
-`defaultMultiSession` gains a private `advertisedSet map[string]bool` field. Not exposed on any public interface.
+None. The filter decorator holds the tool config directly and evaluates advertising decisions at decoration time against `sess.Tools()`.
 
 ## Security Considerations
 
@@ -233,7 +225,17 @@ The `advertisedSet` is computed once at session creation and is immutable. The f
 
 ### Backward Compatibility
 
-Fully backward compatible. All configuration fields work identically. Client-visible `tools/list` output is unchanged. The `Aggregator` interface is removed but it's an internal interface — no external consumers.
+All configuration fields work identically. Client-visible behavior is unchanged. The `Aggregator` interface is removed but it's internal — no external consumers.
+
+**Feature-by-feature analysis of the new decorator ordering:**
+
+| Feature | Current flow | New flow | Impact |
+|---|---|---|---|
+| **Authentication (Cedar)** | HTTP middleware intercepts `tools/list` and `tools/call` at the JSON-RPC level, evaluates Cedar policies against resolved tool names | Unchanged — Cedar middleware sits above the entire session decorator stack, sees the same resolved tool names in requests/responses | None |
+| **Composite tool calling** | Workflow engine receives `sess.Tools()` (advertised only) and `sess.GetRoutingTable()`. Calls `backendClient.CallTool()` directly (bypasses session decorators). `getToolInputSchema()` fails for non-advertised tools | Workflow engine receives `sess.Tools()` (ALL tools, since composite tools decorator sits below the filter). `backendClient.CallTool()` still bypasses decorators. `getToolInputSchema()` now finds schemas for all tools | **Bug fix** — coercion works for non-advertised tools |
+| **Advertising filter** | Aggregator applies `shouldAdvertiseTool` before session creation. `sess.Tools()` returns only advertised tools | Filter decorator applies after composite tools. `sess.Tools()` at the filter level returns only advertised tools + composite tools. Below the filter (where composite tools operate), `sess.Tools()` returns all tools | Same client-visible result; composite tools see more |
+| **Optimizer** | Wraps session after composite tools, indexes `sess.Tools()` (advertised + composite) into find_tool/call_tool | Wraps session after filter, indexes `sess.Tools()` (advertised + composite) — same tools as before | None |
+| **Tool call routing** | `defaultMultiSession.CallTool` looks up routing table → `GetBackendCapabilityName()` → backend HTTP call | Unchanged — routing table is built with the same resolved names and `OriginalCapabilityName` values, just by the factory instead of the aggregator | None |
 
 ### Forward Compatibility
 
@@ -247,29 +249,137 @@ The decorator-based architecture makes it straightforward to add new session-lev
 
 ### Phase 1: Core changes
 
-1. Inline aggregator pipeline into `buildRoutingTable` in `pkg/vmcp/session/factory.go`
-2. Export `ProcessBackendTools` and `ActualBackendCapabilityName` from aggregator package
+1. Move `processBackendTools` and `actualBackendCapabilityName` from aggregator to session package
+2. Inline aggregator pipeline into `buildRoutingTable` in `pkg/vmcp/session/factory.go` — always use conflict resolver
 3. Create filter decorator at `pkg/vmcp/session/filterdec/decorator.go`
 4. Wire filter decorator in `pkg/vmcp/server/sessionmanager/factory.go`
 5. Update server wiring in `cmd/vmcp/app/commands.go` and `pkg/vmcp/server/server.go`
 
 ### Phase 2: Cleanup
 
 6. Delete `Aggregator` interface, `defaultAggregator`, and associated methods
-7. Delete dead discovery types (`AggregatedCapabilities`, `BackendDiscoverer`, etc.)
-8. Update tests
+7. Delete dead types (`AggregatedCapabilities`, `BackendDiscoverer`, etc.)
+8. Delete aggregator tests for removed methods; add integration tests on MultiSession construction/decoration
 
 ## Testing Strategy
 
-- **Unit tests**: Filter decorator (`filterdec/decorator_test.go`) — `Tools()` filtering, composite tool pass-through, `CallTool` pass-through
-- **Unit tests**: Unified `buildRoutingTable` — with/without conflict resolver, `advertisedSet` computation
-- **Regression test**: Workflow engine coercion — non-advertised tool receives `float64(42)` not `"42"` (`composer/workflow_engine_test.go`)
-- **Existing tests**: Conflict resolver tests remain unchanged. Aggregator tests for deleted methods are removed.
+All validation is at the K8s/Ginkgo E2E level (`test/e2e/thv-operator/virtualmcp/`). Tests deploy real MCPServers + VirtualMCPServers to a Kind cluster and exercise the full stack via MCP clients.
+
+### Feature combination matrix
+
+The features being modified interact with each other. The matrix below maps every relevant combination to an E2E test — either an existing one that provides coverage or a new one that must be written.
+
+**Features**: **F** = Filter (per-workload), **EA** = ExcludeAll (per-workload or global), **R** = Renames/Overrides, **CR** = Conflict Resolution, **CT** = Composite Tools, **O** = Optimizer, **A** = Authn (non-anonymous)
+
+| # | F | EA | R | CR | CT | O | A | E2E test | Status |
+|---|---|---|---|---|---|---|---|----------|--------|
+| 1 | | G | | prefix | | | anon | `virtualmcp_excludeall_global_test.go` | Existing |
+| 2 | Y | Y | | prefix | | | anon | `virtualmcp_aggregation_filtering_test.go` | Existing |
+| 3 | Y | Y | | prefix | Y | | anon | `virtualmcp_composite_hidden_tools_test.go` | Existing |
+| 4 | | | | prefix/priority/manual | | | anon | `virtualmcp_conflict_resolution_test.go` | Existing |
+| 5 | | | Y | prefix | | | anon | `virtualmcp_aggregation_overrides_test.go` | Existing |
+| 6 | Y | | Y | prefix | | | anon | `virtualmcp_toolconfig_test.go` (MCPToolConfig CRD) | Existing |
+| 7 | | | Y | prefix | Y | Y | anon | `virtualmcp_optimizer_composite_test.go` | Existing |
+| 8 | Y | Y | | prefix | Y | | anon | **`virtualmcp_composite_coercion_test.go`** | **New** |
+| 9 | Y | | Y | prefix | | | anon | **`virtualmcp_overrides_filter_test.go`** | **New** |
+| 10 | Y | Y | Y | prefix | Y | Y | anon | **`virtualmcp_full_stack_test.go`** | **New** |
+| 11 | | | | prefix | | | token | `virtualmcp_external_auth_test.go` | Existing |
+
+**Legend**: G = global ExcludeAllTools, Y = feature active in test, blank = not exercised.
+
+### Existing tests (regression gates)
+
+All existing tests in the matrix must pass without modification after the refactor. Any failure is a regression.
+
+- **Row 1**: Global `ExcludeAllTools: true` hides all backend tools; MCP server still responds.
+- **Row 2**: Per-workload `Filter` + `ExcludeAll` applied to different backends; only filtered tools exposed.
+- **Row 3**: Composite tool calls hidden backend tools (ExcludeAll + Filter). Validates #3636 fix. Only tests string params — does NOT exercise type coercion.
+- **Row 4**: Prefix, priority, and manual conflict resolution with two backends sharing tool names.
+- **Row 5**: Tool name/description overrides; overridden tool callable by new name.
+- **Row 6**: MCPToolConfig CRD drives filtering + overrides simultaneously.
+- **Row 7**: Optimizer wraps composite tools + overridden backend tools through `find_tool`/`call_tool`.
+- **Row 11**: Token-based incoming auth with backend routing.
+
+### New E2E tests
+
+#### Row 8: Composite tool type coercion with hidden tools (regression test for #4287)
+
+**File**: `test/e2e/thv-operator/virtualmcp/virtualmcp_composite_coercion_test.go`
+
+The specific bug this RFC fixes. When a composite tool workflow step calls a hidden backend tool with a numeric parameter, `getToolInputSchema()` fails to find the schema, skips coercion, and the backend rejects `"42"` (string) instead of `float64(42)`.
+
+**Setup**:
+- Backend with a tool accepting an integer parameter (requires adding an `add` tool to yardstick: `{"a": integer, "b": integer}` → returns sum)
+- `ExcludeAll: true` for the backend
+- Composite tool whose workflow step calls the hidden tool via template expansion (`"{{ .params.a }}"`)
+
+**Assertions**:
+- `tools/list` returns only the composite tool
+- Calling the composite tool succeeds — backend receives coerced numeric args
+- Response contains correct output (the sum), proving type coercion worked
+
+**Must fail before the refactor, pass after.**
+
+#### Row 9: Overrides + filter on the same backend
+
+**File**: `test/e2e/thv-operator/virtualmcp/virtualmcp_overrides_filter_test.go`
+
+No existing test exercises overrides and filter together. The `shouldAdvertiseTool` comment bug (says "before overrides" but receives post-override name) was never caught because this combination is untested.
+
+**Setup**:
+- Backend with tool `echo`, overridden to `custom_echo`
+- Filter configured with `["echo"]` (pre-override name)
+
+**Assertions**:
+- Document whether the filter matches pre- or post-override names
+- Verify the overridden tool is callable by its new name
+- Lock in the actual behavior so the refactor preserves it
+
+#### Row 10: Full stack — all features combined
+
+**File**: `test/e2e/thv-operator/virtualmcp/virtualmcp_full_stack_test.go`
+
+Exercises the entire decorator stack with all features active simultaneously.
+
+**Setup**:
+- Two backends with conflicting tool names (prefix conflict resolution)
+- Backend A: tool overridden (`echo` → `custom_echo`), plus filter hiding some tools
+- Backend B: `ExcludeAll: true`
+- Composite tool calling tools from both backends (including hidden + overridden)
+- Optimizer enabled
+
+**Assertions**:
+- `tools/list` via optimizer returns only `find_tool`/`call_tool`
+- `find_tool` returns the composite tool + filtered backend A tools (not excluded backend B tools, not hidden backend A tools)
+- `call_tool` with composite tool succeeds — reaches both backends with correct original names
+- Overridden tool callable by its new name through the optimizer
+- Backend B tools reachable via composite tool despite `ExcludeAll`
+
+### Unit tests for the filter decorator
+
+**File**: `pkg/vmcp/session/filterdec/decorator_test.go`
+
+Lightweight unit tests for the new filter decorator in isolation:
+- `Tools()` with `excludeAll` → only composite tools returned
+- `Tools()` with `filter` → only matching backend tools + composite tools
+- `CallTool` passes through for both advertised and non-advertised tools
+- No filter config → all tools pass through
+
+### Cleanup
+
+- Delete aggregator unit tests for removed methods (`ProcessPreQueriedCapabilities`, `MergeCapabilities`, `shouldAdvertiseTool`)
+- Conflict resolver unit tests remain unchanged
+
+### Verification
 
 ```bash
+# Unit tests
 go vet ./pkg/vmcp/... ./cmd/vmcp/...
 go test ./pkg/vmcp/... ./cmd/vmcp/...
 task lint-fix
+
+# E2E tests (Kind cluster)
+task test-e2e
 ```
 
 ## Documentation
@@ -279,8 +389,7 @@ task lint-fix
 
 ## Open Questions
 
-1. Should the `shouldAdvertiseTool` comment (incorrectly says "before overrides" but actually receives post-override names) be fixed as part of this work, or is it moot since the method is deleted?
-2. Should we add a Filter+Override combined test before deleting the aggregator, to document the actual behavior?
+None — all design questions resolved during review.
 
 ## References