feat(engine): hand MCP deferral to anthropic's server-side tool search - #191
feat(engine): hand MCP deferral to anthropic's server-side tool search#191andybons wants to merge 1 commit into
Conversation
The deferral core defers MCP tool schemas with harness's own mechanism: a name-only catalog in the system prompt plus the mcp tool's search and select actions. On a model that supports Anthropic's server-side tool search there is a strictly better path, and this wires it: the provider keeps deferred definitions out of the context window, runs the search itself, and expands what the model discovers, with no harness round-trip. The mode is chosen per REQUEST, from the model that will actually serve it. streamTurn passes params.Model, so a chat.params hook that rewrites the model moves the session onto the mechanism that model can honor, and a mid-session SetModel swap does the same. The one state that must never exist is a session deferring with no discovery path, and choosing per request is what prevents it: a swap away from a capable model brings the catalog back on the very next turn. Native mode differs from client-side mode in three ways, each a deliberate non-action. Every MCP definition is sent, deferred or not, because the API needs the full definition to search it and to expand the tool_reference it returns. No catalog segment is rendered, because a harness catalog would be a second copy of the list the API is already withholding, spending the tokens deferral exists to save. And the selected set is neither consulted nor reaped, because discovery is the API's job. That last one is the persistence answer the design asked for, and the continuation rules settle it: the API expands tool_reference blocks throughout the conversation history, so a natively-discovered tool stays usable in later turns and after a reload with no harness bookkeeping at all. recMCPToolsSelected stays what it always was, the client-side path's mechanism, and native sessions simply never write to it. Also lands a regression test for a shape a GATEWAY produces, observed live against Bifrost while server-side tool search was active: a content_block_start carrying an index and no content_block field at all, because the gateway does not model the block type the API emitted. The stream must not fail on it and the husk must not become a phantom part in history. It already behaved correctly -- an unknown block type assembles as an inert empty block -- but nothing pinned it, and this is now a shape we know real infrastructure produces. Verification: three guards red-verified against the exact mechanism each names -- native mode engaging on a capable model, non-capable routes keeping the client-side catalog, and the stripped block staying inert. The request-level test drives Session.Prompt and asserts on the provider.Request an adapter would transcode. go test -race ./... green.
|
Claude finished @andybons's task in 4m 23s —— View job Review: hand MCP deferral to Anthropic's server-side tool searchRead Verified correct
Findings (both inline, neither a merge blocker on its own)
One process note: the PR body says the stripped-block guard was "red-verified," but this PR adds no production change in Nothing here touches the concurrency, startup-speed, or transcode-replay invariants. Since this is explicitly Do not merge (stacked wiring for #189), the two findings are fine to fold into a follow-up or address before the stack lands. Todo
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| // model separately when a chat.params hook has rewritten it; see | ||
| // planMCPToolsForModel. | ||
| func (s *Session) nativeToolSearch(model message.ModelRef) bool { | ||
| return s.sessionCanDefer() && modelmeta.SupportsToolSearch(model) |
There was a problem hiding this comment.
Dead method — never called, and it duplicates the live decision. nativeToolSearch is defined here but has no callers (grep -rn "nativeToolSearch(" --include=*.go finds only this definition). The actual decision in planMCPToolsForModel inlines the same two predicates independently:
deferring := s.sessionCanDefer()
if !deferring {
return mcpToolPlan{defs: all}
}
if modelmeta.SupportsToolSearch(model) {
return s.nativeMCPPlan(all)
}The doc comment claims this method "reports whether THIS session should hand deferral to the provider" — i.e. it reads as the gate — but it governs nothing. That's a divergence trap: a future change to the native-mode condition touches one copy and silently leaves the other stale, and the misleading comment points a reader at the wrong function.
Either wire it in (if s.nativeToolSearch(model) { return s.nativeMCPPlan(all) }, which also collapses the two-step sessionCanDefer/SupportsToolSearch branch into the named predicate) or delete it. Fix this →
| func (s *Session) nativeMCPPlan(all []provider.ToolDef) mcpToolPlan { | ||
| overThreshold := len(all) > s.mcpDeferThreshold() | ||
| defs := make([]provider.ToolDef, 0, len(all)) | ||
| for _, d := range all { | ||
| server, _, ok := splitMCPToolName(d.Name) | ||
| if ok && s.resolveMCPLoading(server, overThreshold) == MCPToolLoadingLazy { | ||
| d.DeferLoading = true | ||
| } | ||
| defs = append(defs, d) | ||
| } | ||
| return mcpToolPlan{defs: defs, native: true} | ||
| } |
There was a problem hiding this comment.
Native sessions still advertise the mcp tool's search/select actions, whose description references a catalog native mode never renders. sessionCanDefer() is true for a native session, so runMCPTool (engine/mcp_tool.go:171-186) dispatches search/select, and mcpToolDeferDescription (mcp_tool.go:131) tells the model "Some MCP tools are DEFERRED: the system prompt lists their names and one-line descriptions" — but in native mode planMCPToolsForModel renders no catalog segment, so that list is absent.
Functionally this degrades gracefully rather than breaking: mcpLoadedNames (mcp_search.go:395) runs the native plan, which returns every def, so search reports every tool loaded: true and select becomes a no-op. But the model is now handed two discovery mechanisms (Anthropic's tool_search tool and harness's mcp search/select) plus a description pointing it at a nonexistent prompt list — a plausible source of wasted turns and confused behavior on exactly the capable models this PR targets.
Worth deciding explicitly: either suppress search/select for native sessions, or adjust the description so it's accurate when the API owns discovery. Not a correctness bug, but the inconsistency is introduced by this change and deserves a conscious call rather than falling out of reusing sessionCanDefer.
1 participant
Slice 2, stacked on #189. Do not merge. This is the wiring that makes #189 reachable — #189 alone sets no
DeferLoading, so the live-fire evidence in #189's body was produced by this branch's binary.What it does
Chooses the deferral mechanism per request, from the model that will actually serve it (
streamTurnpassesparams.Model, so achat.paramsrewrite or a mid-sessionSetModelboth move the session onto the mechanism that model can honor). Native mode: every MCP definition sent, deferred ones marked, no catalog segment, selected set neither consulted nor reaped.The persistence question, answered from the continuation rules
The API expands
tool_referenceblocks throughout conversation history, so a natively-discovered tool stays usable in later turns and across a reload with no harness bookkeeping.recMCPToolsSelectedstays the client-side path's mechanism; native sessions never write to it.TestNativeModeIgnoresSelectionStateencodes it. Confirmed live — see #189's live-fire section, where a resumed session called the discovered tool immediately with no re-search.What to scrutinize
TestModelSwapMovesBetweenMechanisms): swap away from a capable model and the catalog must come back the next turn.provider/anthropic). Observed live against Bifrost:content_block_startwith an index and nocontent_blockfield, because the gateway does not model the block type the API emitted. The stream must not fail and the husk must not become a phantom history part.Verification
Three guards red-verified: native mode engaging on a capable model, non-capable routes keeping the catalog, and the stripped block staying inert.
TestNativeRequestCarriesDeferLoadingdrivesSession.Promptand asserts on theprovider.Request.go test -race ./...green;go vet,gofmtclean.