Skip to content

fix(claude): replay non-canonical OAuth tool names as the model produced them - #6248

Open
realies wants to merge 1 commit into
router-for-me:devfrom
realies:fix/claude-alias-signed-restore
Open

realies wants to merge 1 commit into
router-for-me:devfrom
realies:fix/claude-alias-signed-restore

Conversation

@realies

@realies realies commented Sep 30, 2026

Copy link
Copy Markdown

Problem

With Claude OAuth cloaking, the proxy renames client tools to MCP aliases on the way up and restores the client's names on the way down.

  • Restore (claudeMCPAliasResolver.resolve in internal/runtime/executor/claude_executor_request.go) accepts several non-canonical spellings of an alias and maps each one to the client name:

    • a repeated server prefix;
    • an alias suffix;
    • a parsed semantic match;
    • the semantic-suffix fallback;
    • the hybrid passthrough recoveries.

    A bare client name that the model writes passes through unchanged.

  • Replay: the request-side remap writes only the canonical alias back into the history's assistant messages, including the latest one.

So when the model itself writes a non-canonical name, the round trip loses the name it produced. The next request carries a different name in that assistant turn than the model's original response.

That matters for a latest assistant turn that carries signed thinking. We observed this error on such requests:

400 invalid_request_error: messages.N.content.M: `thinking` or `redacted_thinking` blocks in the latest assistant message cannot be modified. These blocks must remain as they were in the original response.

In the failing requests we inspected, the latest assistant message differed between the client's request and the request sent upstream only in tool_use.name: the client name became the canonical alias. The thinking blocks and the text were byte-identical.

Whether Anthropic's validation covers the tool_use name is not proven, because the original upstream responses were not captured. Two other things could produce the same symptom, and this change doesn't address them:

  • a cloak decision that changes between the request that produced a turn and its replay;
  • an alias that changes between requests.

Reproducing

Synthetic (also the acceptance test). For an upstream tool_use.name N, the invariant is: restore(N) → client history → remap(history) must return N.

  • A canonical alias survives the round trip.
  • Take a non-canonical alias the restore resolves (for example a repeated server prefix), or a bare client name. Restore returns the client name, the remap writes the canonical alias, and N is lost.

A one-way check that remaps a body already containing a non-canonical MCP-shaped name is not enough. The remap leaves MCP-shaped names alone, while real client history holds the restored plain name.

Server side (derived, not run).

  1. Run a cloaked session through a Claude OAuth credential, with upstream responses captured.
  2. Continue until the model writes a tool_use whose name is a non-canonical alias the restore resolves, next to signed thinking.
  3. The client records the plain name.
  4. The next request carries the canonical alias in that turn instead of the name the model produced.

Change

Keep a small in-memory record of produced names.

  • Restore (non-stream and stream paths): every response a keyed caller receives through Execute or ExecuteStream (JSON and SSE) replaces or drops the record for each tool_use id it carries.
    • This holds whether the request was cloaked or not, and whether or not the response has aliases to restore (for example an MCP-only or empty tool set).
    • The produced name is recorded when replaying the name the client receives would not reproduce it.
    • Otherwise any older record for the id is dropped, so a later response that reuses an id cannot inherit it.
    • A record is set or dropped in one cache operation. The new BoundedLRU.Set replaces a value under the cache lock, so concurrent restores of one id give last-write semantics.
    • The generic HttpRequest passthrough restores no tool names and doesn't drop records either.
  • Remap (batched and legacy paths): when a history tool_use block still carries the name the client was given, write the recorded name back. This holds whatever the remap would write now, so a tool-set change does not discard it.
  • Scope: only callers with their own downstream API key get records. The key is a hash of that API key plus the tool_use id, so one caller cannot read, replace or drop another caller's records. Keyless callers share one alias secret, which cannot keep their records apart, so they get none.
  • Bounds:
    • at most 10240 records, least recently used evicted first; a replay refreshes its record;
    • at most 512 bytes of id and names per record, stored as owned copies so no response or request buffer stays alive;
    • together that caps names and ids at 5 MiB, plus fixed per-entry overhead.

This is best effort, not a guarantee. Records live in one process. The replay writes the canonical alias, which is today's behaviour, in these cases:

  • after a restart;
  • on another replica;
  • once a record is evicted;
  • for a keyless caller;
  • for a name over the byte bound.

Because the bound is shared, heavy drift from one caller can evict another caller's records. Nothing changes for tool names that already round-trip.

Why this shape

The loss happens at restore time: several spellings collapse to one client name. So it cannot be fixed on the replay side alone.

  • Never rewriting signed turns, or skipping the rename when the latest turn is signed: this sends the client name where the model wrote the canonical alias, which breaks today's working case.
  • Dropping aliasing for histories with signed thinking: this changes the tool namespace mid-conversation.
  • Restoring only exact aliases in signed turns: this is stateless, but it cannot handle a bare client name. The client's history holds the same Bash whether it came from the canonical alias or was written bare. It would also turn drifted-but-resolvable tool calls into failed ones.

A turn stays replayable for as long as the client keeps the conversation, across restarts and replicas. So no process-local record can make every replay exact. Exact replay in every case would need shared, durable state, such as the home KV the thinking-replay caches use. This change stays small and covers a live process, which is where a tool round trip normally completes.

Check

TestClaudeOAuthToolNameRoundTripKeepsProducedName checks the invariant: restore(N) → client history → remap(history) must yield N. It runs through stream and non-stream restore, and through batched and legacy replay.

Case Without the record (as on dev) With it
Canonical alias (control) passes passes
Repeated server prefix fails on all 4 paths: the canonical alias is written instead of N passes
Bare client name fails on all 4 paths: the canonical alias is written instead of N passes

Record behaviour, each on all 4 paths unless noted:

  • TestClaudeProducedToolNameRecords:
    • callers with different keys that reuse an id for a caller-owned MCP tool each replay their own name;
    • a canonical restore of a reused id drops the older record;
    • a second drifted restore of the same id replaces the record;
    • a changed tool set keeps the record;
    • a name over the byte bound is still restored for the client, is not kept, and drops the older record;
    • a later response without aliases drops the older record; the test uses an MCP-only tool set and asserts its reverse map is empty;
    • a keyless caller gets no record.
  • TestClaudeExecutorUncloakedResponseDropsProducedToolNameRecord: an uncloaked Execute and ExecuteStream response of a keyed caller reuses a recorded id. The test reads the store before (record present) and after (record gone).
  • TestResolveClaudeMCPAliasOptions: only a caller with its own API key is keyed.
  • TestClaudeProducedToolNameRecordsConcurrentCallers: eight callers restore and replay one id concurrently, and each gets its own name.
  • TestClaudeProducedToolNameRecordEviction:
    • a record replayed within the last 10240 records survives;
    • an evicted record falls back to the canonical alias.
  • TestClaudeProducedToolNamesDoNotPinResponses: sixteen restores of 1 MiB responses must not keep them alive. Without owned copies, about 16 MiB stays live.
  • TestBoundedLRUSetReplacesAndEvicts: Set replaces a value in place, refreshes the key, and evicts the least recently used key when a new one crosses the bound.

Each check fails when the property it guards is removed:

  • the caller key, the drop on restore, the in-place replacement, mapping independence, the byte bound, or the owned copies;
  • the drop for responses without aliases;
  • the caller resolution for uncloaked requests, in Execute and ExecuteStream separately;
  • the keyless exclusion;
  • the refresh and eviction in Set.

The eviction test fails when no replay refreshes the record. Without any record, the round-trip test fails exactly as dev does.

Also checked:

  • go test ./internal/runtime/executor/... ./internal/cache/... passes.
  • The alias tests pass five shuffled -race runs.
  • go vet and go build ./... are clean.
  • Unrelated and already on dev: full-package -race runs can fail TestClaudeExecutorPrepareRequestAuthIsRaceFreeOnSharedCredential. It is a data race between the metadata write in internal/auth/claude/identity.go and the read in claude_executor_auth.go. It reproduces with that test alone on dev (-race -count=80: 3 failures).

…ced them

With Claude OAuth cloaking, the restore maps non-canonical MCP aliases
(repeated server prefix, alias or semantic suffix, hybrid passthrough) and
bare client names to the client's tool name, but the request-side remap
writes back only the canonical alias. When the model wrote a non-canonical
name, the replayed assistant turn then differs from the original response,
which matters for a latest assistant turn that carries signed thinking.

Record the produced name per caller and tool_use id when replaying the
client's name would not reproduce it, and write it back on replay while the
history block still carries the name the client was given, whatever the
remap would write now.

- Only callers with their own downstream API key get records, keyed by a
  hash of that key plus the tool_use id. Keyless callers share one alias
  secret, which cannot keep their records apart, so they get none.
- Every response a keyed caller receives through Execute or ExecuteStream,
  cloaked or not, replaces or drops the record for each tool_use id it
  carries in one cache operation (the new BoundedLRU.Set), so a later
  response that reuses an id cannot inherit it.
- Records hold owned copies of at most 512 bytes of id and names, and the
  least recently used of 10240 records is evicted first.

This is best effort: after a restart, on another replica, once a record is
evicted, or for a keyless caller, the replay writes the canonical alias, as
before.
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant