Repository navigation
fix(mcp-host): alias tool names that exceed the provider name limit - #1445
Merged
Merged
Conversation
…aliases mcp-host prefixes every tool with `<server>__`, so the server name decides how much room a tool name has on the provider wire. The golden is emitted by the real generateRegistryName and the test fails when it goes stale (UPDATE_WIRE_GOLDENS=1 regenerates it). `catalog` stays empty on purpose: the catalog-slot name producer is not in this tree yet and will extend this same test when it lands. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t the LLM edge `<server>__<tool>` names can exceed what a provider accepts for a function name (64 on OpenAI, Azure, compatibles and Bedrock; 128 on Claude and Vertex; Vertex also rejects a leading digit). Today such a name makes every call of the context fail with a 400. Each direct adapter now projects the request through a per-request map (src/llm/toolWireNames.ts): names that fit the adapter's envelope are sent unchanged (byte-identical request), the rest get a readable alias `<server>__<tool head>_<12 hex>` (or a prefix-less form) with the canonical server/tool annotated in the description, and tool calls in the response are mapped back to the canonical name by exact lookup. History stays canonical, so nothing is persisted or migrated. Envelope per adapter: Claude and its token counter W_c, Vertex W_v, gemini/openrouter W_g, everything else W (an endpoint-profile field in the OpenAI family). The codex/grok proxies keep their own aliasing and are not touched. Closes #1388 Class-check: `name: (t|tool|tc)\.name` in mcp-host/src plus the openaiFamily protocol sites -> 25 sites -> 2 producers (manager.ts getAllTools, stepRouter.ts) reach 6 network edges, all projected: OpenAIProvider (chat + Responses, azure, compatibles), ClaudeProvider x2, googleGenerative, bedrockConverse, AnthropicTokenCounter.count. Excluded: codexSubscription.ts and grokSubscription.ts (the proxy aliases; pinned by T-W6). The rest never reach an LLM or build canonical tools upstream of an edge. Local estimators (tiktoken, FallbackTokenCounter, countSync, fitGfsImages) keep measuring canonical names; accepted, bounded drift. Repro-test: mcp-host/src/llm/__tests__/wireToolNames.providers.test.ts:95 (T-W2, 22/22 ids) and wireToolNames.edges.test.ts:84 (T-W8 b) fail on e611ebf Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
… account The public-boundary guard rejects any added PEM private-key marker, with no fixture exemption. The Vertex driver only checks that private_key is present and the harness fakes the SDK, so a plain placeholder is enough. Class-check: `BEGIN .*PRIVATE KEY` in lines added by this branch -> 1 site (wireToolNamesHarness.ts) -> fixed. Repro-test: scripts/tests/test-minikube-t2-public-boundary.sh rejects 98779cb and passes on this commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ssword heuristics CodeQL's js/insufficient-password-hash read the `1password...` test server names flowing into the alias hash as passwords. The hash only derives a deterministic alias suffix and never sees secrets. `1keychain` keeps the leading digit and the same length (9), so the Vertex/W_g leading-digit case and the 47/50-character server boundary are unchanged. The control alias literal is recomputed independently (sha256 over UTF-16LE, which reproduces the previous 4adbcea87d1d for the old name). The identity golden is regenerated with UPDATE_WIRE_GOLDENS=1 on the merge-base e611ebf; it differs from the previous one only by the rename. Class-check: `password|passwd|secret` in lines added by this branch -> 10 sites, all the `1password` test server name in 3 test files + identity golden -> all renamed; no other credential-shaped names reach createHash. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… path
The cache-aware Claude method is the one the port adapter picks whenever
system prompt parts are present, yet nothing failed if it converted the
canonical history instead of the projected one. Send a finished tool turn
naming a 129-character tool and a dotted one, and assert every tool_use name
in the body fits W_c and matches a declared tool.
Class-check: `grep -rn "toWireRequest(" src` (non-test) -> 6 projection sites
(claude.ts:212 and :295, googleGenerative.ts:136, openai.ts:131,
bedrockConverse.ts:138, anthropicTokenCounter.ts:71) -> only claude.ts:295
lacked a history assertion; the per-provider history test walks the other
four provider paths through completeSingleTurnWithTools, and the token
counter test compares counted vs sent messages. Fixed the one gap.
Repro-test: mcp-host/src/llm/__tests__/wireToolNames.edges.test.ts:52 (fails with mutant: convertToClaudeMessages(messages) at claude.ts:300 -> AssertionError, history name does not match W_c)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every model answer in the provider contract named a Notion tool that fits
the 128-character envelopes, so Vertex (and Claude's non-cache method) never
had to map a returned alias back: replacing fromWireResponse with identity
left the suite green and the alias would reach dispatch as an unknown tool.
- Provider contract: every direct provider answers with the alias of a
129-character tool, which no envelope accepts, and must return the
canonical name.
- End to end: Vertex behind the real LlmPortAdapter and McpManager answers
with that alias; the remote MCP server must receive the canonical tool.
Class-check: `grep -rn "fromWireResponse(" src` (non-test) -> 5 sites; each
mutated to identity against src/llm/__tests__: openai.ts:150 (27 red),
claude.ts:362 (1 red), bedrockConverse.ts:173 (2 red) were caught;
googleGenerative.ts:174 and claude.ts:256 survived. Both are now covered.
Repro-test: mcp-host/src/llm/__tests__/wireToolNames.providers.test.ts:114 (fails with mutant: fromWireResponse -> identity at googleGenerative.ts:174 and at claude.ts:256, AssertionError on the returned name)
Repro-test: mcp-host/src/llm/__tests__/wireToolNames.endToEnd.test.ts:142 (fails with mutant: fromWireResponse -> identity at googleGenerative.ts:174, AssertionError on the returned name)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The only projection test named the history tool in both the assistant tool_call and the tool message, so dropping either source from the set stayed green: the total toWire fallback still produced a valid alias. Add one case per source, alone, with a real tool in tools[] whose name equals the history name's base alias. Leaving the source out makes the history name go out under that tool's name; with it, the history name gets a distinct wire name that resolves back to its canonical. Class-check: `grep -n "set.push" mcp-host/src/llm/toolWireNames.ts` -> 3 sources (tools[].name, assistant tool_calls[].name, tool message name) -> tools[] is pinned by every provider test; the two history sources now have one isolating case each. Repro-test: mcp-host/src/llm/__tests__/toolWireNames.property.test.ts:588 (fails with mutant: drop the tool_calls[].name push in toWireRequest -> AssertionError, history wire name equals the tool's name) Repro-test: mcp-host/src/llm/__tests__/toolWireNames.property.test.ts:602 (fails with mutant: drop the tool message name push in toWireRequest -> AssertionError, history wire name equals the tool's name) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
No real slot is exactly 50 characters, so turning the prefix bound from <= into < stayed green. Cut the longest real slot to 49, 50 and 51 characters and assert the alias keeps the server prefix through 50 and drops it (no `__` at all) at 51, under W and W_g. Class-check: numeric bounds in mcp-host/src/llm/toolWireNames.ts (`grep -n "const [A-Z_]* = [0-9]" `) -> 3 (MAX_PREFIX_SERVER=50, HEAD_BUDGET=51, ANNOTATION_FIELD_CODE_POINTS=200) -> only MAX_PREFIX_SERVER lacked a boundary case; HEAD_BUDGET is pinned by the literal control aliases and the 200 cut by the annotation tests at 199/200 code points. Repro-test: mcp-host/src/llm/__tests__/toolWireNames.property.test.ts:348 (fails with mutant: server.length < MAX_PREFIX_SERVER at toolWireNames.ts:75 -> AssertionError at length 50) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ounter OpenAITokenCounter encoded the canonical request, while the wire request carries the alias and a "(MCP server: ...; tool: ...)" annotation in front of every aliased description: roughly 25-30 tokens short per aliased tool. Project the request with toWireRequest before counting, as the Anthropic counter already does, and feed the projected fields to both the tiktoken path and the byte-length fallback. Envelope: the factory builds this counter only for descriptors with tokenizer 'openai' (openai, azure), and both endpoint profiles declare W, so W is the exact envelope of every real use, not an approximation. The test builds both providers through the real factory and compares the count of the canonical tools with the count of the declarations their adapters actually sent. Class-check: local estimators that receive canonical tools (`grep -rln "heuristicCountTools\|countWithEncoder\|countTokens" src/core`) -> 4: OpenAITokenCounter (fixed), AnthropicTokenCounter (already projects; its heuristic offline/countSync fallback does not), FallbackTokenCounter (19 compat/own-SDK providers plus the two proxied ones, which send canonical names) and the contextManager heuristic. Not fixed: they are biased upper bounds, not tokenizers, and the right envelope is not knowable inside the counter without new per-provider plumbing (the proxied providers must not be projected at all). Reported as a follow-up. Repro-test: mcp-host/src/llm/__tests__/wireToolNames.edges.test.ts:121 (fails at 313b876: expected 175 to be 251) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ap.toWire
toWire was documented as total: a name outside the set fell back to
identity or its base alias. That base alias can already be the wire name of
another canonical of the set (a real name equal to it is reserved first),
so the caller would send the outside name under another tool's name and
fromWire would resolve it to that other tool. Throw ToolWireNameError
instead; the map only knows the names it was built with.
No caller depends on the fallback: the three production calls in
toWireRequest pass names collected into the set under the same
`typeof name === 'string'` guards, and every test call passes a member of
the map's own set (grep below).
Class-check: `grep -rn "\.toWire(" mcp-host/src` -> 3 production sites
(toolWireNames.ts toWireRequest: tools, tool_calls, tool message name) and
25 pre-existing test calls -> all pass set members, none changed. The fallback branch in
toWire was the only "total" claim in the module (`grep -n "Total" `).
Repro-test: mcp-host/src/llm/__tests__/toolWireNames.property.test.ts:438 (fails at 0965185: expected function to throw an error, but it didn't)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- toolWireNames.ts module doc: the alias is not a pure function of the canonical name; on a collision the suffix depends on the request's other names. Keep why that is fine: the map is local to one request and history is stored canonical. - PREFIX_SERVER: it does not mirror the MCP manager's server names (those are DNS subdomains and may carry dots). Describe what it accepts and why: a single lowercase label that already passes every envelope and keeps the alias's first `__` at the server boundary. - bedrockConverse.ts: the 64-character limit comes from AWS's Converse API reference, not from the SDK's types. Class-check: comments attributing an envelope or the alias rule (`grep -rn -B4 "toolNameEnvelope" src --exclude-dir=__tests__`, `grep -rn "pure function\|Same shape" src/llm`) -> 6 envelope sites (claude.ts, googleGenerative.ts, bedrockConverse.ts, endpointProfiles.ts x3) plus the two module comments -> corrected the three reported; the others either carry no source claim or state one that holds (gemini/openrouter "no published limit"). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… names The wire-name tests were labelled T-W1 ... T-W9 (with b variants), IDs of a test matrix that does not live in the repository. Drop them from describe names, section rules and file headers, keeping the description that already followed each ID. The (c)/(d)/(e) letters of the "tool calls coming back" cases were sub-items of that same matrix, split across two files, so they go too; letter lists complete within one describe (annotation, Claude token counter) stay. Class-check: `grep -rn 'T-W[0-9]' mcp-host/src` -> 21 lines in 5 files -> all replaced; `git grep 'T-W[0-9]'` outside mcp-host/src -> 0. Not touched: the P1 ... P8 property labels in toolWireNames.property.test.ts are the same kind of external ID, left out of this finding's scope and reported as a follow-up. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nter
The openai/azure tiktoken counter test only sent `[{role:'user'}]`, so a
counter that projected the tools but counted the canonical history stayed
green. The test now sends a finished tool turn through the real adapter:
two aliased calls (a >128-character name built from varied words, and
QUERY), with tool messages that carry the canonical `name`, which is the
shape the tool-use loop and the task executor produce
(toolUseLoopMessages.ts:211, toolUseLoopSingleTool.ts:87,
taskExecutor.ts:879,885). It compares `countSync(history, tools)` with the
count of what the adapter sent.
Comparison chosen: rebuilding ChatMessages from the captured body would be
per-family (chat `messages` vs Responses `input` items, string arguments,
null vs '' content) and would encode adapter layout the counter does not
model. Aliasing changes only names, so `sentHistory` is the canonical
history with the call aliases read off the captured body substituted, in
order; the `name` of its tool messages is that substitution (the alias of
the call each one answers), because neither family sends it. The test
asserts exactly that: in chat the body's tool messages carry no string
`name`, and the Responses body has no tool messages. It also asserts that
every call name changed, fits W and matches a declared alias, and, as an
explicit precondition, that the sent names cost a different number of
tokens than the canonical ones, measured through message content (never
projected), so a vanished margin turns red instead of silently losing
power. The `'x'.repeat` name is not used here: it tokenizes like its alias.
Class-check: `toWireRequest(` in token counters (src/core/tokenizer) -> 2
sites (anthropicTokenCounter.ts:71, openaiTokenCounter.ts:133) -> the
anthropic one is already covered with an aliased tool turn by "Claude token
counter (offline=false) (b)" (params.messages == sent.messages); this commit
covers the openai one.
Repro-test: mcp-host/src/llm/__tests__/wireToolNames.edges.test.ts:208 (fails with mutant: countSync projects only the tools, `toWireRequest([], canonicalTools, COUNTED_ENVELOPE)` counting canonicalMessages -> 459 vs 437; fails with mutant: tool messages counted with their canonical `name` -> 448 vs 437; both on openai and azure. A benign `+3` per tool in countWithEncoder stays green)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…files `COUNTED_ENVELOPE = 'W'` in the tiktoken counter repeats the envelope the openai and azure profiles declare, and the test could not tell `W` from `W_g`. A separate case now declares `1srv__ok` (fits W, not W_g: digit first) next to the twin `asrv__ok`, which fits every envelope. Comparing two counts that both go through the counter's own projection cannot discriminate the envelope (a W_g counter aliases the name the same way on both sides; checked). Instead the case compares the counted cost of the `1srv__ok` declaration with the cost of its twin and requires them to be within 4 tokens; an alias adds its annotation, 22 tokens here. The twin comparison does not depend on how the counter prices a declaration, so a benign formula change (per-tool overhead, a different serialization) does not go red. The case also asserts the adapter sent both declarations unchanged, so a profile switched to W_g goes red as well. Class-check: hardcoded envelopes outside the provider profiles (`ToolNameEnvelopeKey = '` / `toolNameEnvelope` in src, excluding toolWireNames.ts) -> 1 duplicate (openaiTokenCounter.ts:92); the anthropic counter receives the envelope from ClaudeProvider (claude.ts:63, 'W_c' only as an unset-option default) -> only the openai one needs pinning. Passing the envelope through the constructor is out of scope. Repro-test: mcp-host/src/llm/__tests__/wireToolNames.edges.test.ts:230 (fails with mutant: `COUNTED_ENVELOPE = 'W_g'` -> cost gap 22, expected < 5, on openai and azure; the openai profile switched to W_g fails at :222, sent declarations aliased. A benign `+3` per tool in countWithEncoder stays green) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…om dev dev (#1434) now wraps every provider from makeProvider with withWellFormedRequestText and adds ES2024.String to the tsconfig lib. Three comments of the wire-name projection became inaccurate: - the annotation helper said toWellFormed is unavailable under ES2022; - the UTF-16LE hash note implied injectivity over lone surrogates on the production path, where names arrive already repaired to U+FFFD; - the Claude counter claimed to count exactly the sent names, which no longer holds for a name with lone surrogates (the counter is unwrapped). Comment-only; no behaviour change. Clean merge, no textual conflict. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Since dev's makeProvider wraps providers with withWellFormedRequestText, lone surrogates in tool names are repaired to U+FFFD before the wire-name projection runs. A canonical with a lone surrogate therefore resolves back to its repaired form, and two names that differ only in the surrogate share one wire name. Both are accepted (no known MCP server emits such names); the tests pin the behaviour so a change to either layer revisits it. Both tests fail on 5ace877 (before the merge, without the wrapper). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Evenfire Reviewer — Round 1Reviewed head: Decision: Approve with follow-ups The projection does what it claims. A canonical Must fix
Verified
Validation gaps
PR descriptionAccurate. One optional clarification: on the captured Notion catalog the 7 surviving tool characters are the shared prefix Pre-existingNone identified during the scoped review. Next action
Full review archive: — Review performed by Evenfire Reviewer |
Contributor
Blind review — PR 1445 (head
|
The countTokens fallback logged `error: err.message`, a plain string, so the
structured logger's Error redaction (which reduces an Error to name, code and
status because provider error text can echo request payloads) never ran and
the raw provider message reached the log. Pass the Error itself as `err`.
redactUnknown recognises an Error under any key, so the field keeps only the
bounded classification. No test or consumer read the old `error` field
(`grep -rn "countTokens failed" mcp-host/src` -> only the log call).
Class-check: logger.{warn,error,info,debug} field built from raw error text
(`err.message`, `String(err)`, `errorText(err)`) in mcp-host/src, excluding
tests -> 8 sites: anthropicTokenCounter.ts:99, guardrailHookResolver.ts:180,
workflow/fonts.ts:138,187,193,486,849,949 -> fix anthropicTokenCounter.ts, the
only one this PR adds (`git diff $(git merge-base HEAD origin/dev)..HEAD`);
the other 7 predate the PR and are left untouched. The routes.ts hits of the
same grep are HTTP response bodies, not logger fields.
Repro-test: mcp-host/src/core/tokenizer/__tests__/anthropicTokenCounter.test.ts:47 (fails at 8e7931c: expected '{"provider":"claude","reason":"rate_l…' not to contain 'sk-ant-payload-secret')
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…labels toWireRequest said only the `auto` and `none` tool_choice modes are sent; `required` is a third mode (ToolCompletionRequest.tool_choice) that remoteLlmHook forwards. None of the three names a tool, so the projection still does not have to rewrite tool_choice; the comment now lists all three. The property tests and the captured fixture carried labels (`P1`..`P8`, `P3b`, `P3c`, `P6b`, "T1 fixtures") that point at documents outside the repo. They are replaced by descriptions of what each test or assertion checks. Comments and test names only: no logic changes, same 1407 cases in src/llm + src/core/tokenizer. Class-check: `git grep -nP '\b[PT][0-9]+[a-z]?\b|\b[PT][0-9]+(\.[0-9]+)+\b'` over the files this PR touches in mcp-host/src/llm and src/core/tokenizer -> 22 lines: 17 in toolWireNames.property.test.ts, 1 in remoteMcpToolLists.ts (all added by this PR, all replaced) and 4 in claude.ts (`T2.2`, `P1-006`), which predate the PR and are left untouched -> 0 PR-added labels remain. `tool_choice` mode lists: `grep -rn "auto.*none" mcp-host/src/llm/toolWireNames.ts` -> 1 site, fixed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ounter
createTokenCounter passes `this.toolNameEnvelope()` to AnthropicTokenCounter,
but the counter defaults to `W_c`, the same envelope ClaudeProvider returns,
so dropping the pass-through left the suite green. No production subclass of
ClaudeProvider overrides toolNameEnvelope (the Vertex and Bedrock drivers
that do are separate classes and count with their own counters), so the
test uses a subclass that narrows the envelope to `W` and checks that the
counted tool names equal the names the same provider sends.
Class-check: `grep -rn "toolNameEnvelope" mcp-host/src` (non-test) -> the
only counter fed by a provider envelope is AnthropicTokenCounter via
ClaudeProvider.createTokenCounter (1 site); the tiktoken counters of openai
and azure get the profile envelope from the factory and are already pinned
per profile.
Mutation: replacing the pass-through with `{ ...options }` fails only
mcp-host/src/llm/__tests__/wireToolNames.edges.test.ts:125 (AssertionError:
expected [ Array(1) ] to deeply equal [ Array(1) ]); 1407 other tests pass.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…cases
The three "tool calls coming back" cases that expect a name to come back
unchanged (an exact canonical, the canonical of an aliased tool, an unknown
name) asserted only that, so they also passed with the projection turned
off. Each now also asserts that, in the same request whose response it
checks, the long tool went out under an alias. The unknown-name case runs
over every direct provider, and `longA` fits the 128-character envelopes of
Claude and Vertex, so that case adds the 129-character tool to the request
as its witness; the other two run only on `W` providers, where `longA` is
aliased.
Class-check: `grep -n "toBe(shortB)\|toBe(longA)\|toBe('inventado_xyz')"
mcp-host/src/llm/__tests__/wireToolNames.providers.test.ts` -> 4 sites: the
3 pass-through cases (witness added) and the orphan-alias case, which
already asserts `alias !== longA` first.
Mutation: making toWireRequest return its inputs unchanged fails all 58
cases of the three tests (18 + 18 W providers, 22 direct providers), e.g.
wireToolNames.providers.test.ts:166 "expected
'mcp-evenfire-notion-remote-v1-0-0-b23…' not to be
'mcp-evenfire-notion-remote-v1-0-0-b23…'"; before this commit they passed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…est seams `wireToolAlias` and the `options.hash` parameter of ToolWireNameMap have no production caller: the property tests use the first as the expected wire name of a name whose base alias is free, and the second to force alias collisions with a weak hash. Both stay; a one-line comment on each says so, so neither reads as dead code or as a production knob. Comments only. Class-check: `grep -rn "wireToolAlias\|new ToolWireNameMap" mcp-host/src` outside __tests__ -> wireToolAlias: 0 callers; ToolWireNameMap: 1 production construction (toWireRequest), which passes no options. The other exports with no caller outside the module (TOOL_NAME_ENVELOPES, ToolWireNameError, ToolWireNameMap) are used by the module's own production path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… error
The redaction test threw a hand-built Error with `name: 'RateLimitError'`
and asserted that name in the log. The SDK (0.32.1) builds a 429 with
`APIError.generate`, which returns a RateLimitError instance whose `name`
is still 'Error', so the test pinned a field production never emits. It
now throws the error `Anthropic.APIError.generate(429, ...)` produces, with
the secret inside the provider body, and asserts `status: 429` and the
`name` the SDK actually sets.
Class-check: `git diff $(git merge-base HEAD origin/dev)..HEAD -- mcp-host/src
| grep "Object.assign(new Error"` -> 1 PR-added site (this test), fixed. The
other hand-built 429 in the same file ('falls back to heuristic × 1.5 on
error', line 40) predates the PR and asserts only the fallback count.
Repro-test: mcp-host/src/core/tokenizer/__tests__/anthropicTokenCounter.test.ts:48 (fails at 8e7931c: expected '{"provider":"claude","reason":"rate_l…' not to contain 'sk-ant-payload-secret')
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… list
The witness added in BR-R3-L5c, `expect(sentNames(id, body)[i]).not.toBe(X)`,
held vacuously when the request carried no tools: wireToolDecls returns
[], the element is undefined, and undefined is not X. A shared helper now
also asserts the number of tools sent and that the witnessed name fits the
provider's envelope before checking it differs from the canonical.
Class-check: `grep -n "sentNames(id, body)\[" mcp-host/src/llm/__tests__/wireToolNames.providers.test.ts`
-> 3 sites (the three pass-through cases) -> all moved to the helper. The
orphan-alias case reads `first.sent[0].name`, which throws on an empty list
instead of passing, so it is not part of the class.
Mutation: with toWireRequest returning `tools: []`, the three tests fail in
all 58 cases ("expected [] to have a length of 2 but got +0"); the previous
version of the file passed all 58. With the projection turned off (inputs
returned unchanged) both versions fail all 58.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The structured logger reduces an Error to name/code/status and keeps `name`
only when it fits a bounded identifier shape. Every redaction test fed a
plain Error, and the real SDK 429 now used by the token-counter test also
has `name === 'Error'`, so replacing the name with a constant 'Error'
survived the whole suite. Two cases go through the public logger's write
path: a real TypeError must log `name: 'TypeError'` without its message, and
a name outside the shape must fall back to 'Error' without leaking.
Class-check: `grep -rn "name: 'Error'\|err: {"` over the test files that use
the logger or redactUnknown -> 2 redactor assertions before this commit
(logger.test.ts:28, plain Error; anthropicTokenCounter.test.ts:77, SDK error
whose name is 'Error') -> both left as they are; they cover code/status
and the counter path, and the new cases cover the name branch.
Repro-test: mcp-host/src/logger.test.ts:53 and :59 pass at 8e7931c (the
branch already worked), so the regression check is by mutation: logger.ts:60
`name: 'Error'` fails :53 ("expected { err: { name: 'Error' }, …(3) } to
match object { err: { name: 'TypeError' }, …(1) }"); dropping the fallback
(always `error.name`) fails :59 ("expected '{"err":{"name":"Type Error
carrying s…' not to contain 'secret-name-value'"). Both restored.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
alfredolopez80
approved these changes
Nov 12, 2026
alfredolopez80
left a comment
alfredolopez80
left a comment
Contributor
There was a problem hiding this comment.
Approved on head 8e7931c. No blocking defects; the Low findings and the recommended mini-plan are in the blind-review comment above.
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.
What
mcp-host exposes every MCP tool to the LLM as
<server>__<tool>. That name can exceed what a provider accepts as a function name: 64 characters on OpenAI, Azure, OpenAI-compatible endpoints and Bedrock; 128 on Claude and Vertex (Vertex also rejects a leading digit). Today a single over-long tool makes every LLM call of that context fail with a 400.This PR projects tool names at the LLM edge, per request:
mcp-host/src/llm/toolWireNames.tsbuilds a per-request map from canonical name to wire name.<server>__<tool head>_<12 hex>when the server is a single lowercase label (/^[a-z][a-z0-9-]*$/) of at most 50 characters (subject to the 51-character cut below);__) a prefix-less form: the whole name sanitized, runs of_collapsed, then_<12 hex>, so the alias never carries a__that could be read as another server.__and a 50-character one is cut to 49 characters, so in practice the prefix form only holds up to 48 characters. Many sibling tools can share the same visible head and differ only in the hash. Routing is not affected: the lookup back is exact. The alias is therefore not meant to be readable on its own: the canonical server and tool are annotated in the tool description, and that annotation is what tells the model what it is calling._<k in base36>, so the suffix depends on the set of names in the request, not only on the name.tools[].Closes #1388
Why
Remote MCP servers with long tool names (Notion's hosted server is the real case, 77 tools captured; Atlassian's 26 tools are captured too) combined with long registry server names push canonical names into the 65 to 128 range. On a 64-envelope provider the context is unusable, not just the one tool.
How
OpenAIProvider.completeSingleTurnWithTools(covers both the Chat Completions and Responses protocols, Azure and compatibles, by wrapping encode/decode without touching the protocols),ClaudeProvider(both tool methods, including the cache-aware one),googleGenerative,bedrockConverse, plus the token countersAnthropicTokenCounter.countandOpenAITokenCounter(tiktoken), so their counts match the names and annotations actually sent.completeSingleTurnofClaudeProvider,OpenAIProvider, Bedrock and Gemini, andClaudeProvider.completeSingleTurnAndCache, send notools[](the Claude and OpenAI ones also reduce each message to role and content). Today no caller reaches them withtool_callsin the history (summarization flattens the history to markdown first). If one ever does, its history names go out canonical.gemini/openroutera conservative fixed envelope, everything else 64. In the OpenAI family the envelope is a field of the endpoint profile (toolNameEnvelope), decided by profile id.control-api/test/mcpHost.serverNames.golden.test.ts→mcp-host/src/llm/__tests__/fixtures/serverNames.json) is emitted by the realgenerateRegistryNameand fails when stale (UPDATE_WIRE_GOLDENS=1regenerates it). Itscatalogslot is intentionally empty: the catalog name producer is not indevyet.Testing
959314dee: 606 files / 8970 tests (1 skipped, 1 todo), typecheck clean.toolWireNames.property.test.ts(fast-check): envelope literals, control aliases, map properties (idempotence, order independence, stable wire name across requests, alias only claims its own server segment), forced collisions with an injected hash, description annotation bounds.wireToolNames.providers.test.ts: every direct provider sends only names inside its envelope and resolves an aliased call back to the canonical name, history projection, return mapping incl. unknown and orphan names, 63-character real server names.wireToolNames.identity.test.ts: byte identity of the request against a golden produced on the merge-base when all names fit, for every direct provider and the Claude cache-aware method.wireToolNames.edges.test.ts: Claude cache-aware method incl. history projection, token counters count the names sent, Responses continuation across an aliased turn, proxies untouched, and lone-surrogate canonical names on themakeProviderpath (see Risks).wireToolNames.endToEnd.test.ts: an alias returned by the model reaches the MCP server with the canonical name, (Notion, and Vertex with an over-128 tool), and deferred tools / approval see the right tool.61f32ae04.Risks / Follow-ups
devbefore feat: install remote OAuth MCP connectors from the Marketplace #1284, which extends the same golden test with catalog server names from its real producer and adds the "longest catalog name = 55" check.gemini/openrouterenvelope not observed live: no credentials were available to probe their real limit, so they use a fixed conservative envelope. Worth confirming when a key is available.openaiid (Responses) stays on the 64 envelope, the strictest in the family.makeProvidernow repairs lone surrogates to U+FFFD before this projection runs. A canonical tool name with a lone surrogate therefore resolves back to its repaired form (the loop answers "Tool not found"), and two names that differ only in the surrogate share one wire name. Without this PR that tool already failed with a 400. No known MCP server emits such names;wireToolNames.edges.test.tspins the behaviour.FallbackTokenCounter, the context manager heuristic, Anthropic's heuristic fallback) keep measuring canonical names and descriptions without the alias annotation; the drift is bounded and only affects estimates.clerum__tool_call({ name: "<alias>" })instead of calling the listed tool directly gets "Tool not available": the bridge target name is checked against canonical names and is not mapped back. The error is recoverable and the discovery text already tells the model not to bridge a listed tool. Left for a follow-up.MAX_PREFIX_SERVER = 50has no effect today because the 51-character head cut wins first; the shape above is the real one. Keeping a distinguishing slice of the tool for long registry server names is left for a follow-up.