Repository navigation
fix(cursor): recover Kimi tool calls emitted as history narration - #12723
diegosouzapw merged 17 commits into
Conversation
e1d16ac to
226e6ee
Compare
CI failures are upstream regressions on
|
|
Update (commit Refactored the two inline recovery blocks in
This PR's own tests: 9/9 kimi narration, 458/459 cursor+composer (1 skip), eslint clean on all touched files. The remaining red is upstream drift on |
|
Great root-cause write-up (the actual leaked production payload as validation is |
|
Fragment added: |
Follow-up (2026-09-16): streaming scrubber closes the remaining leak pathsLive traffic on What was still leaking
The fix
Verification
Reproducing the numbers
CI note — failures on this head are base debt, not from this diff
Every red gate was reproduced locally and verified to fail identically (or pass locally where CI env fails) without this PR's changes. Happy to help land #13826 if it unblocks this merge. |
90faf47 to
4a50fdd
Compare
c5c06a6 to
5d76a7a
Compare
5d76a7a to
50f95d3
Compare
Kimi-k3 / kimi-k3-high on the Cursor provider imitate the executor's own
flattenMessages history narration ('Assistant called tool <name> (<id>) with
arguments: <json>') when they decide to call a tool, then close with their
native generation delimiters (<|close|>argument<|sep|><|close|>call<|sep|><|close|>tools<|sep|>).
Cursor's protobuf backend passes the narration through verbatim as text with
finish_reason 'stop', so clients render raw narration + delimiters instead of
a tool card, and the leaked turn compounds on every subsequent request via
history re-send.
Add open-sse/utils/kimiToolCallNarration.ts: detect the narration line, scan
the balanced JSON arguments, strip the delimiter tail, and reassemble a
structured OpenAI tool_calls entry. Wire it into the cursor executor's two
finalization paths (finalizeSseStream streaming + buildResponseFromCtx
non-streaming), gated on 'no structured tool calls yet' so it never fires on
a healthy turn. Validated against the live leaked payload and 9 new unit
tests; existing cursor/composer/translator suites still green.
The 4 failing checks (Merge integrity, API Route Typecheck, Fast Quality Gates, Unit Tests 1-4) all fail on files this PR does not touch (glm.ts, changelog.d/fixes/reset-aware-model-family.md, image-gen routes, error-sanitizer suites, huggingchat/stream boundaries). None of those files exist on release/v3.8.51, on this branch, or in a local base+head merge — the CI run merged a transient upstream state at 09:46 UTC that has since been rewritten. Verified: pristine base reproduces the same failures; my 3-file diff (cursor.ts, kimiToolCallNarration.ts, kimi test) is clean and its 9 unit tests pass on the simulated merge ref.
…helper Collapses the two inline recovery blocks in cursor.ts into one-line calls to a new applyKimiToolCallRecovery() in kimiToolCallNarration.ts. The helper takes a structurally-typed ctx + optional emit callback (streaming) so no executor import is needed and no module cycle forms. This resolves the PR's Fast Quality Gate failures attributable to the change: - file-size: cursor.ts was 1804 > frozen 1759; now 1763, baseline ratcheted to 1764 with justification (file hard-capped, base already 1758, hook is irreducible at 2 call sites — gate-sanctioned last resort). - complexity-ratchets: cursor.ts 11→12 / 4→5 now OK (2762 vs baseline 3218). Behavior unchanged — 9/9 kimi tests, 458/459 cursor+composer (1 skip). Remaining CI failures (glm.ts TS2554, mutation quotaScoring, changelog fragment) are pre-existing upstream regressions on release/v3.8.51.
…er gate Split scanJsonObjectEnd's inline string/escape state machine into a nextScanState helper (was cognitive 16, threshold 15). Behavior unchanged — 9/9 kimi tests, 458/459 cursor+composer. This clears the last Fast Quality Gate failure attributable to this PR (cognitive-complexity new-code: kimiToolCallNarration.ts 0→1). Remaining red (glm.ts TS2554, mutation quotaScoring, changelog fragment) is pre-existing upstream drift on release/v3.8.51, not this PR.
…very Addresses maintainer review on cheaper-inference#12723 — the only ask before merge was a changelog.d/fixes/ fragment. Bullet summarizes the narration-imitation failure mode, the structured tool_calls recovery in both finalization paths, and the healthy-turn gate. Passes check:changelog-integrity (the remaining invalid fragment, reset-aware-model-family.md, is pre-existing upstream drift on release/v3.8.51, unrelated to this PR).
The v3851 recovery only fired when NO structured tool call existed, so a
turn that emitted a real tool call AND narrated the flattenMessages
dialect ('Assistant called tool ... <|close|>argument<|sep|>...') leaked
the narration verbatim into client-visible content. Kimi then mimics the
tool-RESULT serialization too ('Tool result (id):', 'User: <tool_result>
<tool_name>...<result>...</tool_result>'), which nothing caught —
compounding through history on every subsequent turn.
Three layers, one family:
1. Streaming holdback (cursor.ts): text deltas now pass through
createNarrationStreamScrubber — a state machine that holds back text
the moment it could start a dialect construct and only emits the
safe prefix (same design as the Composer inline-tool parser). A
narrated call is converted to a structured tool_calls chunk
mid-stream; finish() flushes partial-trigger prose at EOF.
2. Ungated scrub (kimiToolCallNarration.ts): applyKimiToolCallRecovery
now scrubs dialect constructs even when structured calls exist; only
call SYNTHESIS stays gated on toolCalls.length === 0. The scrubber
handles char-by-char and chunked arrival, truncated constructs at
EOF, and bare keywords mid-prose (only consumed when a delimiter
follows).
3. History hygiene (cursorAgentProtobuf.ts): flattenMessages scrubs
gateway-dialect constructs from prior assistant visible text so old
leaks stop re-anchoring the mimicry loop.
Fixes the 2026-09-15 leak class (Assistant called tool + User:
<tool_result> blobs rendered as User: content). 28 unit tests incl.
production leak specimen; 4/4 mutations killed.
…+ rebaseline cursor.ts (+34, annotated) The two flush+recover call sites shrink to one line each via the new finalizeKimiTurn helper in kimiToolCallNarration.ts; cursor.ts stays file-size compliant against the 67f6c03 tip (base growth consumed the old headroom; annotated rebaseline per repo convention).
…exity ratchets CI Fast Quality Gates flagged kimiToolCallNarration.ts as new-code complexity debt (cyclomatic 0->1 via max-lines-per-function on the 254-line scrubber factory, cognitive 0->2 on scanJsonObjectEnd and the inline narrJson scan). Hoist the closure state machine into a ScrubCtx object with module-level step functions dispatched through STEP_BY_STATE, and reuse a single stepJsonScan walker for both the batch and streaming JSON scans instead of duplicating the string/escape/depth branching. Behavior is unchanged; the factory is now a thin constructor. Adds two EOF tests for the unterminated-narration-head branch: the refactor surfaced that 'narrHead' residue flushing was never exercised (mutation survived). With the tests, reverting that branch turns 2 red. Tests: 30/30 in tests/unit/kimi-tool-call-narration.test.ts (was 28). Complexity: 0 violations in the touched file (was 3).
tests/unit/claude-passthrough-cache-rescue.test.ts belongs to a separate workstream and was swept in by a blanket 'git add -A'. It does not belong in this PR's diff.
…er-inference#13627 Rebasing cheaper-inference#12723 onto the tip that now includes cheaper-inference#13627's kv_after_text fix (merged into cursor.ts) produced a real conflict in config/quality/file-size-baseline.json across two historical commits on this branch (c4bdb8579, 8744eea27) that had each rebaselined cursor.ts for their own growth at the time. Resolved to the actual current measured line count (1847) after the rebase; codex.ts resolved to the higher/more permissive of the two conflicting values (1528, untouched by this PR). check:file-size, check:complexity-ratchets, check:cognitive-complexity, generate-agent-skills --check all green; tsc clean; 30/30 kimi tests pass.
50f95d3 to
3cada1d
Compare
…ount (cheaper-inference#13079) Merged after a maintainer rework that kept every one of @hartmark's commits intact. **What the rework added:** the reclaimable-space gate for the auto-cleanup VACUUM sits behind a default-off feature flag so the release default is unchanged, with the flag documented in `docs/reference/FEATURE_FLAGS.md` and described in all 66 locales; the rest is your change as submitted. Validated as a combined board first (this PR merged with the 21 siblings of the same wave on the release tip): eslint with the frozen suppressions, typecheck:core, check:open-sse-typecheck, complexity, cognitive-complexity, changelog-integrity, i18n new-key coverage, docs-counts, docs-sync, migration-numbering, provider-consistency and a duplicate-identifier audit all green, plus 176 passing / 0 failing focused node:test cases across the 25 test files the wave touches and the dashboard test under Vitest (2/0). Then re-validated alone on the fresh tip before this merge: conflicts re-resolved, file sizes rebaselined for this PR's own growth, eslint and this PR's focused tests re-run. Thank you — gating VACUUM on reclaimable pages instead of row count is the right signal.
…rmat key (cheaper-inference#13617) Merged after a maintainer rework that kept every one of @patrykkopycinski's commits intact, including the changelog fragment you added afterwards. **What the rework added:** the `eslint-suppressions.json` diff was corrected (the PR had dropped live entries) and a test now proves the CLI-probe fallback path is actually taken when the HTTP API rejects a CLI-format key — before, the fallback existed but nothing exercised it. Validated as a combined board first (this PR merged with the 21 siblings of the same wave on the release tip): eslint with the frozen suppressions, typecheck:core, check:open-sse-typecheck, complexity, cognitive-complexity, changelog-integrity, i18n new-key coverage, docs-counts, docs-sync, migration-numbering, provider-consistency and a duplicate-identifier audit all green, plus 176 passing / 0 failing focused node:test cases across the 25 test files the wave touches and the dashboard test under Vitest (2/0). Then re-validated alone on the fresh tip before this merge: conflicts re-resolved, file sizes rebaselined for this PR's own growth, eslint and this PR's focused tests re-run. Thank you.
…inned, all harnesses (cheaper-inference#13448) Merged after a maintainer rework that kept every one of @patrykkopycinski's commits intact — including the two refactors you pushed later (extracting the adaptive-effort wiring out of `chatCore.ts` and reading `x-omniroute-effort` inside the wiring module), which were merged into the rework rather than overwritten. **What the rework added:** the adaptive-effort wiring is scoped to OpenAI-dispatch requests only (the claim in `docs/routing` was corrected to match), and `defaultReasoningEffort` was widened to accept `auto` explicitly instead of relying on a loose string. Validated as a combined board first (this PR merged with the 21 siblings of the same wave on the release tip): eslint with the frozen suppressions, typecheck:core, check:open-sse-typecheck, complexity, cognitive-complexity, changelog-integrity, i18n new-key coverage, docs-counts, docs-sync, migration-numbering, provider-consistency and a duplicate-identifier audit all green, plus 176 passing / 0 failing focused node:test cases across the 25 test files the wave touches and the dashboard test under Vitest (2/0). Then re-validated alone on the fresh tip before this merge: conflicts re-resolved, file sizes rebaselined for this PR's own growth, eslint and this PR's focused tests re-run. Thank you — gateway-resolved, per-turn pinned effort is a real feature, and the header contract makes it usable from every harness.
…m send (cheaper-inference#13355) Merged. Internal `_omniroute*` markers must never reach an upstream: at best they are noise in someone else's logs, at worst they change the upstream's parse. Stripping every one of them before the send is the right invariant. Validated as a combined board first (this PR merged with the 21 siblings of the same wave on the release tip): eslint with the frozen suppressions, typecheck:core, check:open-sse-typecheck, complexity, cognitive-complexity, changelog-integrity, i18n new-key coverage, docs-counts, docs-sync, migration-numbering, provider-consistency and a duplicate-identifier audit all green, plus 176 passing / 0 failing focused node:test cases across the 25 test files the wave touches and the dashboard test under Vitest (2/0). Then re-validated alone on the fresh tip before this merge: conflicts re-resolved, file sizes rebaselined for this PR's own growth, eslint and this PR's focused tests re-run. Thank you.
…ounts, stale suppressions)
c4293e2
into
cheaper-inference:release/v3.8.51
…eaper-inference#12723) Merged. Kimi emitting tool calls as history narration is a provider quirk we have to absorb rather than pass through; recovering them keeps the tool contract intact for clients that never see the quirk. Validated as a combined board first (this PR merged with the 21 siblings of the same wave on the release tip): eslint with the frozen suppressions, typecheck:core, check:open-sse-typecheck, complexity, cognitive-complexity, changelog-integrity, i18n new-key coverage, docs-counts, docs-sync, migration-numbering, provider-consistency and a duplicate-identifier audit all green, plus 176 passing / 0 failing focused node:test cases across the 25 test files the wave touches and the dashboard test under Vitest (2/0). Then re-validated alone on the fresh tip before this merge: conflicts re-resolved, file sizes rebaselined for this PR's own growth, eslint and this PR's focused tests re-run. Thank you.
Problem
Kimi-k3 / kimi-k3-high served through the Cursor provider sometimes emit a tool call as visible text instead of a structured call. The client then renders raw narration plus delimiter junk, and because the bad turn is re-sent in conversation history, the leak compounds on every subsequent turn.
Observed in production via OmniRoute call logs (109 artifacts in a single session), e.g.:
with
finish_reason: "stop"and notool_callsfield on the message.Root cause
Cursor's agent API accepts one user message per Run, so
flattenMessages(open-sse/utils/cursorAgentProtobuf.ts) serializes priorassistant.tool_callsinto plain text for the model to read as context:Kimi imitates that narration format when it decides to call a tool: it emits the narration as visible content, appends the complete JSON arguments, then closes with its native generation-grammar delimiters (
<|close|>argument<|sep|><|close|>call<|sep|><|close|>tools<|sep|>). Cursor's protobuf backend passes the whole thing through verbatim. The existing DeepSeek inline-tool-call fallback never fires because (a)isComposerModel("kimi-k3-high")isfalseand (b) this is a different grammar anyway.Fix
New
open-sse/utils/kimiToolCallNarration.ts:Assistant called tool <name> (<id>) with arguments:narration linetool_callsentry and return the cleaned visible contentWired into the cursor executor's two finalization paths —
finalizeSseStream(streaming) andbuildResponseFromCtx(non-streaming) — gated onctx.toolCalls.length === 0so it never fires on a healthy turn.Validation
tests/unit/kimi-tool-call-narration.test.ts— all pass.buzz-dev-mcp__shell/ idtoolu_01PollBanner2with clean JSON args and prose-only content; returnsnullfor plain prose.tsc -p tsconfig.json --noEmitintroduces no new errors in the touched files.Rebased onto
release/v3.8.51(this PR supersedes a mis-based earlier attempt).