Never publish a leaked tool call as a review - #81
Merged
Conversation
A review on serge#79 was published with a body of nothing but Kimi's raw
tool-call markup and zero inline comments. Three defects compounded:
1. moonshotai/Kimi-K2.6 (HF Router) serialized a tool call into
`message.content` as chat-template special tokens, left the structured
`tool_calls` field empty, and returned finish_reason="stop". The agent
loop read that as "no tool calls, so this is the final answer".
2. `_extract_json`'s raw_decode-at-every-brace pass then accepted the
leaked call's own argument object — `{"path": ..., "start_line": ...}`
— as the review JSON, so nothing raised.
3. With summary and comments both empty, the stub-JSON salvage fell back
to the surrounding content, and `publish_review` posted it.
Fixed at all three layers:
- llm_client recovers text-serialized tool calls into real ToolCall
objects (both the streaming and buffered paths) and strips the markup
from the content, so the turn stays a tool turn. Structured
`tool_calls` still wins when the provider sends them.
- `_extract_json` takes an optional `require_any_key`; review and task
callers pass their contract keys so incidental JSON can't pass as a
result. Default behaviour is unchanged.
- `publish_review` raises `EmptyReviewError` rather than posting a review
with no summary and no comments, and a summary that is only leaked
markup is treated as no summary. All three publish paths report the
failure: the webhook publisher posts a failure comment, the UI publish
endpoint 409s so the human can edit and retry.
2 of 22 Kimi jobs in prod hit defect 1; no other model did.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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 happened
This review was published on #79 with a body of nothing but Kimi's raw tool-call markup and zero inline comments:
Root cause — three defects compounding
The provider leaked a tool call as text. On turn 8,
moonshotai/Kimi-K2.6via the HF Router serialized its tool call intomessage.contentusing chat-template special tokens, leftmessage.tool_callsempty, and returnedfinish_reason: "stop"._run_agentic_looptook thenot chat.tool_callsbranch and treated it as the final answer._needs_final_salvagedidn't fire either — the content wasn't blank and the finish reason wasn'tlength._extract_jsonaccepted the tool arguments as the review. Its third pass (raw_decodeat every{) parsed the leaked call's own argument object,{"path": "reviewbot/reviewer.py", "start_line": 248, "end_line": 280}, so no_UnparseableLLMOutputwas raised.The empty-summary salvage published the markup. With
summary=""and no comments, the stub-JSON fallback called_prose_outside_json, which stripped the JSON and returned the surrounding special tokens as the summary.publish_reviewposted it.Reproduced byte-for-byte against the persisted job row before fixing.
Scope
The serge#79 review is the one confirmed occurrence. A first scan of the prod jobs DB also flagged a
/tasksrun (Kimi-K2.7-Code, endedno_fix, nothing published), but that row has since been evicted byWEB_JOB_RETENTION=25— the table is a rolling window of the 25 most recent jobs — so it can no longer be verified. Treat the prevalence as "seen more than once on Kimi, never on another model", not as a measured rate.The fix
Each defect is fixed at its own layer, so no single one has to hold alone.
llm_client— recover text-serialized tool calls into realToolCallobjects and strip the markup from the content, so the turn stays a tool turn and the model gets the file it asked for. Covers the streaming and buffered paths (they converge on one place). Structuredtool_callsstill wins whenever the provider sends them. Tolerant of a truncated tail; drops a call whose id isn't recognizable rather than inventing a tool name._extract_json— new optionalrequire_any_key. The review caller passes("summary", "comments", "event")and the task callers pass("title", "body", "patch"), so incidental JSON can't pass as a result. Default behaviour is unchanged for any caller that doesn't opt in.publish_review— raises the newEmptyReviewErrorinstead of posting a review with no summary and no inline comments, and a summary that is only leaked markup counts as no summary. A markup-only summary that arrives with real inline comments still publishes, but renders the(no overall summary provided)line rather than the tokens. All three publish paths report the failure rather than swallowing it:errorand posts a failure commentrun_review→ posts a comment in the thread409 empty_review, so the human can edit the summary and retryA legitimate review that quotes a special token (
<|endoftext|>while discussing a tokenizer) is deliberately left untouched — the check only rejects text with no substance left, it never rewrites text that has some.Tests
29 new tests; 596 pass,
make formatclean.TextToolCallRecoveryTests— the exact prod payload, multiple calls, prose around the markup, truncated tail, unrecognizable id, both HTTP paths, structured-winsLeakedTextToolCallLoopTests— end-to-end through the real client with mocked HTTP: the leaked turn executesread_file, the output comes back as atoolmessage, the loop reaches a proper reviewExtractJsonRequiredKeysTests— the leaked arguments are rejected as a review but still accepted without the filterModelMarkupOnlyTests,EmptyReviewGateTests— the publish gate, including the quoted-special-token case🤖 Generated with Claude Code