CL-6478: Sanitize tool-call names before persisting turn history - #210
Merged
Merged
Conversation
decodeToolName is deliberately total: a hallucinated or provider-mangled tool name comes back verbatim instead of throwing. encodeToolName is not total -- it throws when a name can't be put back on the wire. Add sanitizeToolNameForPersistence so any name that fails that round trip collapses to a stable placeholder instead of being written down as-is.
A malformed tool-call name (e.g. a small model leaking wire fragments into a function name) was being persisted verbatim. The next turn's outbound request re-encodes every persisted tool-call name, and that encode throws on a name it can't re-invert -- so the room died rebuilding its request, permanently, every turn after the bad one landed. event-collector.ts now runs a tool_call block's name through sanitizeToolNameForPersistence before it reaches turnPart, so a bad name fails its own turn cleanly instead of wedging the room.
Ledger row, VENDORED-FROM delta line, and kill-date tree hash for the sanitize-tool-name.ts addition to vendor/intx/hub-sessions.
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.
Summary
qwen3.8:27bemitted\n</parameterinside a function name).@intx/inference'sdecodeToolNameis deliberately total and returns such a name verbatim rather than throwing, butencodeToolNameis not total -- it throws when that same name is later put back on the wire to build the next turn's outbound request. Persisting the decoded name unchecked meant the fault surfaced one turn late, after the bad data was already durable: every following turn in the room died rebuilding its request, permanently.vendor/intx/hub-sessions/src/event-collector.tsis the one place a decoded tool-call name is accepted into turn history (turnPart). It now runs the name through a newsanitizeToolNameForPersistence(vendor/intx/hub-sessions/src/sanitize-tool-name.ts) before writing it: a nameencodeToolNamecan re-invert persists as-is, anything else collapses to a stablemalformed_tool_callplaceholder. A bad tool-call name now fails its own turn cleanly instead of wedging the room.encodeToolName/decodeToolNamethemselves live only in the published@intx/inferencenpm package (already retired from vendoring inVENDORED.md), so this fixes the boundary on our side rather than touching that pair.vendor/intx/hub-sessionsis already a vendored, editable tree (VENDORED.md), so the fix, its delta line, and the kill-date tree hash land together.Not done (out of timebox)
turnPartrows (or a read-side sanitize wherever persisted turns are reloaded to build the next request) and I did not implement or verify it.@intx/agent/@intx/inferencewhere a persisted turn's tool-call name is re-encoded for the next outbound request (that code lives entirely in the npm-only package, not in this repo), so the "request-building dies in ~40ms" failure is reproduced by direct assertion (encodeToolNameon the previously-malformed persisted name no longer throws) rather than an end-to-end run through the real agent/harness.bun run check:killdatesreports one pre-existing failure,vendor/intx/dbhash drift -- confirmed present on a pristineorigin/maincheckout untouched by this branch, unrelated to this change, not fixed here.Test plan
bun test vendor/intx/hub-sessions/src/-- 59 pass, including the new CL-6478 regression suite (malformed name persists as a safe placeholder,encodeToolNameon the persisted name doesn't throw, and the collector accepts a second turn afterward) and the sanitize-tool-name round-trip property testsbun test packages/tool-registry-publish/src/tool-name-limits.test.ts-- 15 pass (the existing 64-char shipped-tool-name guard, unchanged)bun run lintfrom repo root -- 0 errors (12 pre-existing warnings in files this PR doesn't touch)tsc --noEmitinvendor/intx/hub-sessions-- cleanDO NOT MERGE -- for peer review.
Fixes CL-6478.