Repository navigation
feat(agent-core-v2): sanitize outgoing LLM request messages - #4274
kimi-agent-bot wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: eccd0b8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d038131cea
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1d98032bf
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const out = toolCalls.map((call) => { | ||
| const id = mapId(call.id, tally); | ||
| const name = fixString(call.name, tally); | ||
| const args = call.arguments === null ? null : fixString(call.arguments, tally); |
There was a problem hiding this comment.
When a historical tool call stores a literal JSON escape such as {"text":"\ud83d"} in arguments, this character scan sees only ASCII and records no repair. The Anthropic and Google lowerers then run JSON.parse on that string (src/human/llm/requester/bases/anthropic/lower.ts:75-80 and src/human/llm/requester/bases/google-genai/lower.ts:147-158), recreating the lone surrogate in the final request object, so a strict endpoint still rejects every replay of the session; decode and recursively sanitize valid argument JSON before it reaches those lowerers.
Useful? React with 👍 / 👎.
| for (const [key, entry] of Object.entries(value)) { | ||
| const next = sanitizeJsonValue(entry, tally); | ||
| changed = changed || next !== entry; | ||
| out[key] = next; |
There was a problem hiding this comment.
When a message-level tool schema contains a lone surrogate in a property or definition name, this loop sanitizes only entry; key is copied unchanged and, if the values are otherwise clean, no repair is recorded at all. The Kimi message-level tool path later serializes these schemas (src/human/llm-kimi/trait.ts:130-132), so a key such as \ud83d can still poison the request; run member names through the same repair and disambiguate replacement collisions.
Useful? React with 👍 / 👎.
| case 'audio_url': | ||
| return protocol !== 'anthropic'; |
There was a problem hiding this comment.
When an Anthropic tool result contains only a hosted audio_url, this branch marks it non-visible and padEmptyToolContent replaces it with Tool output is empty.. Before this change, audioToPlaceholder in src/human/llm/requester/bases/anthropic/patterns.ts:23-44 converted the same result to (audio omitted: not supported by this provider), so the default-on pass now tells the model the tool returned nothing even though it returned unsupported media; treat the remaining non-data audio part as visible or use the audio-specific placeholder here.
AGENTS.md reference: AGENTS.md:L110-L112
Useful? React with 👍 / 👎.
b1d9803 to
766c234
Compare
Add a last-mile sanitizer at the shared point before request serialization so a session whose history holds content the endpoint rejects stops failing with repeated 400s: - lone UTF-16 surrogate code units (left behind when tool output is truncated mid code point) are replaced with U+FFFD across message content, tool call names/arguments/ids, and declared tool parameters - tool call ids that collapse onto each other after repair are kept distinct with a stable ascending suffix, and the same raw id maps to the same new id on both sides of the assistant/tool pairing - tool messages with no wire-visible content are padded with placeholder text, and base64 video/audio tool-result parts are downgraded to placeholder text on protocols whose endpoint rejects them - repairs are reported once per request signature through a warn log and the llm_request_sanitized telemetry event - the KIMI_CODE_REQUEST_SANITIZE switch (on unless set to off) is read from the bootstrapped env channel when one is available, falling back to process.env, and the test harness wires the bootstrapped channel through the catalog so tests exercise the same path
…rations and log repaired tool call ids - repair records now carry the post-repair tool call id: the raw id can itself hold lone surrogates and would poison the warn-log JSON - the last-mile sanitizer now also repairs lone surrogates in the request-level system prompt and tool declarations (name/description/parameters) via a new sanitizeRequestSurface entry point; a poisoned declaration or prompt 400s every request exactly like a poisoned message, so the repairs feed the same warn-log and llm_request_sanitized telemetry signature accounting - clean system prompt and tool arrays pass through reference-equal - env-vars docs now state the switch covers the whole request surface
766c234 to
eccd0b8
Compare
Why
A session dies permanently when something the model will never accept gets into its history: from then on, every request replays the same poisoned content and gets the same 400. Three known poison shapes, all observed in the wild:
\ud83d-style escapes and strict endpoints 400 on it.tool_result.content; a folded empty output 400s withField required.tool_result.There is no layer today that stops this: writes are not sanitized, the wire journal round-trips lone surrogates faithfully, zod validates shape not content, the projector fixes structure not strings, and 400 is not retryable. One bad write bricks the session forever.
What
Add an outgoing-request sanitizer at the last shared point before the SDK serializes the request (
ModelRequesterImpl.runRequest, right beforetoLlmMessage). It repairs the projection copy only — the wire journal (source of truth) is never touched:"Tool output is empty."data:video/audio part in a tool message on a protocol known to reject it (anthropic)llm_request_sanitizedtelemetry (counts by rule kind, model, protocol).KIMI_CODE_REQUEST_SANITIZE=offkills the whole pass (default on).resolved.protocol), so the media rule is gated precisely; the conservative fallback (downgrade when protocol is unknown) only applies to standalone pure-function use.Tests
test/llm-adapter/model/requestSanitize.test.ts): lone surrogates in every position/count (valid pairs and emoji untouched, input immutability), empty tool content shapes,data:media downgrade + protocol gating, env off switch, repair counting, and 3ModelRequesterImplintegration cases.pnpm typecheckclean;oxlint --type-aware0 errors (warnings identical to main baseline);check-no-commentsOK.Notes
agent-core-v2sources (repocheck-no-commentsrule); rationale lives in naming and this description.pnpm lint:importsshows 4 pre-existing violations on main (loop.ts,loopService.ts,videoUpload.ts,origin.ts), unrelated to this change.2026-10-21 更新（系列重组：本 PR 职责不变「出站请求清洗」，补上评审发现的缺口）：
docs/en|zh/configuration/env-vars.md同步一句范围表述；sanitizeRequestSurface单测与两个集成测试（修复值到达 wire config、干净 tools 引用相等穿透）。系列关系：截断不产生孤代理（写入侧预防）→ #4282；空 tool_result 补占位 → #4300；媒体拒收恢复 → #4301。