Skip to content

Commit 1e3dacd

Browse files
jerm-droclaude
andcommitted
Simplify workflow engine by routing through session
The workflow engine directly uses Router + BackendClient, bypassing the session's CallTool entirely. Replace with sess.CallTool() and sess.Tools() — the session already handles routing and backend dispatch. This eliminates the abstraction leak and naturally fixes #4287 since sess.Tools() below the filter decorator includes all tool schemas. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 5645d9e commit 1e3dacd

1 file changed

Lines changed: 41 additions & 6 deletions

File tree

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

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ When backend tools are excluded from advertising via `excludeAll` or `filter`, c
1919

2020
The root cause: the advertising filter runs inside the aggregator **before** session creation. The session's `tools` field only contains advertised tools. The composite tools decorator builds its workflow engine from `sess.Tools()`, so it never sees schemas for non-advertised tools.
2121

22+
### Abstraction leak: workflow engine bypasses the session
23+
24+
The workflow engine directly uses `Router.RouteTool()` to resolve a `BackendTarget`, then calls `BackendClient.CallTool()` with that target — completely bypassing the session's `CallTool` method and every decorator in the stack. The session already encapsulates routing + backend dispatch in its own `CallTool`, making the engine's direct use of `Router` and `BackendClient` redundant. This leak is why `getToolInputSchema()` must maintain its own tool list rather than relying on the session — the engine doesn't call through the session at all.
25+
2226
### Architectural issue: the aggregator is a monolithic middleman
2327

2428
The `Aggregator` interface has 7 responsibilities: backend capability querying, per-backend overrides, conflict resolution, advertising filtering, routing table construction, name reversal, and discovery pipeline orchestration. It sits between the HTTP middleware and the session object — operating on data that logically belongs to session construction.
@@ -32,6 +36,7 @@ As the aggregator's responsibilities shift (e.g. removing the filter), it become
3236
- Fix composite tool type coercion for non-advertised backend tools
3337
- Eliminate the `Aggregator` interface and `defaultAggregator` struct
3438
- Move advertising filtering to a session decorator in the correct position in the decorator stack
39+
- Simplify the workflow engine by routing tool calls through the session instead of directly using `Router` + `BackendClient`
3540
- Keep conflict resolvers and per-backend override utilities as a reusable library
3641
- No breaking changes to configuration or client-visible behavior
3742

@@ -57,6 +62,8 @@ Decorator stack (top → bottom):
5762

5863
The session factory directly orchestrates: per-backend overrides → conflict resolution → routing table construction → advertising set computation. No `Aggregator` object. The advertising filter becomes a session decorator that sits above composite tools, so composite tools see all tool schemas while clients only see advertised tools.
5964

65+
The workflow engine is simplified: instead of directly using `Router` and `BackendClient` to dispatch tool calls, it calls `sess.CallTool()`. The session already handles routing and backend dispatch. This eliminates the abstraction leak and means the engine's `getToolInputSchema()` can use `sess.Tools()` — which, at the composite tools layer (below the filter), includes all tools.
66+
6067
### Detailed Design
6168

6269
#### Session factory inlines aggregator pipeline
@@ -113,6 +120,31 @@ func (d *filterDecorator) Tools() []vmcp.Tool {
113120

114121
`CallTool`, `ReadResource`, `GetPrompt` pass through unchanged — the filter only affects what's advertised via `Tools()`, not what's callable.
115122

123+
#### Workflow engine calls through the session
124+
125+
`pkg/vmcp/composer/workflow_engine.go`
126+
127+
The workflow engine currently takes a `Router` and `BackendClient` and dispatches tool calls directly:
128+
129+
```go
130+
// Current: bypasses the session entirely
131+
target, err := e.router.RouteTool(ctx, step.Tool)
132+
result, err := e.backendClient.CallTool(ctx, target, step.Tool, args, nil)
133+
```
134+
135+
This is replaced with a call through the session:
136+
137+
```go
138+
// New: routes through the session's CallTool
139+
result, err := e.session.CallTool(ctx, step.Tool, args, nil)
140+
```
141+
142+
The session already handles routing table lookup, backend capability name reversal, and backend dispatch. The engine no longer needs `Router` or `BackendClient` interfaces — it only needs a reference to the `MultiSession` it decorates.
143+
144+
Similarly, `getToolInputSchema()` currently maintains its own `e.tools` list. It can instead call `e.session.Tools()`, which at the composite tools layer (below the filter) returns all tools including non-advertised ones. This naturally fixes the #4287 bug: the schema for every backend tool is available regardless of advertising.
145+
146+
The `ResolveToolName` call in `getToolInputSchema` (for dot-convention support) moves to the session's `CallTool` implementation, which already handles this via the session router.
147+
116148
#### Decorator wiring
117149

118150
`pkg/vmcp/server/sessionmanager/factory.go` (assumes PR #4231 landed)
@@ -148,7 +180,9 @@ Delete:
148180

149181
### API Changes
150182

151-
**Removed**: `WithAggregator(agg aggregator.Aggregator)` session factory option
183+
**Removed**:
184+
- `WithAggregator(agg aggregator.Aggregator)` session factory option
185+
- `Router` and `BackendClient` dependencies from the workflow engine (tool calls route through the session)
152186

153187
**Added**:
154188
- `WithAggregationConfig(cfg *config.AggregationConfig, cr ConflictResolver)` — required conflict resolver and aggregation config. The conflict resolver always exists (`NewConflictResolver` defaults to prefix strategy).
@@ -232,7 +266,7 @@ All configuration fields work identically. Client-visible behavior is unchanged.
232266
| Feature | Current flow | New flow | Impact |
233267
|---|---|---|---|
234268
| **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 |
235-
| **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 |
269+
| **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 calls `sess.CallTool()` — routes through the session instead of bypassing it. `getToolInputSchema()` uses `sess.Tools()` which, below the filter decorator, includes all tools | **Bug fix** — coercion works for non-advertised tools. **Simplification** — engine no longer depends on `Router` or `BackendClient` |
236270
| **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 |
237271
| **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 |
238272
| **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 |
@@ -253,13 +287,14 @@ The decorator-based architecture makes it straightforward to add new session-lev
253287
2. Inline aggregator pipeline into `buildRoutingTable` in `pkg/vmcp/session/factory.go` — always use conflict resolver
254288
3. Create filter decorator at `pkg/vmcp/session/filterdec/decorator.go`
255289
4. Wire filter decorator in `pkg/vmcp/server/sessionmanager/factory.go`
256-
5. Update server wiring in `cmd/vmcp/app/commands.go` and `pkg/vmcp/server/server.go`
290+
5. Simplify workflow engine: replace `Router` + `BackendClient` with `sess.CallTool()`; replace `e.tools` with `sess.Tools()`
291+
6. Update server wiring in `cmd/vmcp/app/commands.go` and `pkg/vmcp/server/server.go`
257292

258293
### Phase 2: Cleanup
259294

260-
6. Delete `Aggregator` interface, `defaultAggregator`, and associated methods
261-
7. Delete dead types (`AggregatedCapabilities`, `BackendDiscoverer`, etc.)
262-
8. Delete aggregator tests for removed methods; add integration tests on MultiSession construction/decoration
295+
7. Delete `Aggregator` interface, `defaultAggregator`, and associated methods
296+
8. Delete dead types (`AggregatedCapabilities`, `BackendDiscoverer`, etc.)
297+
9. Delete aggregator tests for removed methods; remove `Router` and `BackendClient` interfaces from workflow engine
263298

264299
## Testing Strategy
265300

0 commit comments

Comments
 (0)