fix(moonshot): preserve native Kimi tool_call IDs in openai-completions replay#70030
Conversation
Greptile SummaryThis PR adds a Confidence Score: 5/5Safe to merge — additive opt-out with correct default, well-tested at three seams, no behavioral change for any other provider. The change is minimal and strictly additive. The No files require special attention. Reviews (1): Last reviewed commit: "fix(moonshot): preserve native Kimi tool..." | Re-trigger Greptile |
2a96c52 to
d4fffd4
Compare
d4fffd4 to
36037fc
Compare
|
Landed via rebase merge. Thanks @LeoDu0314. Validation:
Landed SHAs: |
|
Follow-up issue closure:
|
Summary
[a-zA-Z0-9], which rewrites Kimi K2.6's native IDs (functions.<name>:<index>, e.g.,functions.read:0→functionsread0). Kimi's serving layer then fails to match the mangled IDs back to the original tool definitions in multi-turn history.finish_reason: "stop"returned instead of"tool_calls"~80% of the time.sanitizeToolCallIdsopt-out to the sharedopenai-compatiblereplay family helper (buildOpenAICompatibleReplayPolicy+buildProviderReplayFamilyHooks), and wired the Moonshot plugin to opt out. Default behavior for all other openai-compatible providers is unchanged.ToolCallIdModeenum orsanitizeToolCallId()implementation. Not touching thekimi-codingplugin (its policy is minimal and runs onanthropic-messages, not openai-completions; the 2026-04-10 comment about it being "also affected" is not reproducible from the code and deserves its own repro before scope expansion). No@mariozechner/pi-aipatches.Change Type (select all)
Scope (select all touched areas)
(Additive opt-out on
buildProviderReplayFamilyHooks/buildOpenAICompatibleReplayPolicy; Moonshot provider plugin wiring.)Linked Issue/PR
Root Cause
...OPENAI_COMPATIBLE_REPLAY_HOOKS, which unconditionally returns{ sanitizeToolCallIds: true, toolCallIdMode: "strict" }. Kimi K2.6 returns IDs containing.and:, which are not in[a-zA-Z0-9], so strict sanitization drops them. The mangled ID is then sent back in conversation history; Kimi's serving layer can't match it, and emits text instead of a structured tool call.mistraldoes with"strict9"). That friction pushed Moonshot toward copying the default.call_<uuid>and the generic alphanumeric tests pass through sanitization losslessly, so the bug only surfaces for providers whose native IDs contain non-alphanumeric structure. Kimi is the first such case in the bundled set.Regression Test Plan
src/plugins/provider-replay-helpers.test.ts— two new cases forbuildOpenAICompatibleReplayPolicy(api, { sanitizeToolCallIds: false })on bothopenai-completionsandopenai-responses.src/plugin-sdk/provider-model-shared.test.ts— new case forbuildProviderReplayFamilyHooks({ family: "openai-compatible", sanitizeToolCallIds: false }).extensions/moonshot/index.test.ts— flipped the existing plugin-boundary assertion: Moonshot's policy must not carrysanitizeToolCallIdsortoolCallIdModeon openai-completions, while keepingapplyAssistantFirstOrderingFix/validateGeminiTurns/validateAnthropicTurns.openai-completions, the resolved policy passes through tool_call IDs unchanged so nativefunctions.<name>:<index>IDs survive round-trip.src/agents/pi-embedded-runner.openai-tool-id-preservation.test.tsowns the downstream replay behavior but is keyed ontoolCallIdMode: "strict". With Moonshot opting out, those paths simply don't apply — no contradiction.User-visible / Behavior Changes
openai-completionsnow survives more than ~2 rounds.true(existing behavior) and must be explicitly opted out.Diagram
Security Impact (required)
Yes, explain risk + mitigation: N/A.Repro + Verification
Environment
Steps
{ modelApi: "openai-completions", modelId: "kimi-k2.6" }.applyAssistantFirstOrderingFix / validateGeminiTurns / validateAnthropicTurnsbut does not carrysanitizeToolCallIdsortoolCallIdMode.openai-completions) continue to carrysanitizeToolCallIds: trueandtoolCallIdMode: "strict".Expected
Actual
Evidence
Before the fix (on the new tests):
After the fix:
Issue reporter's independent streaming-API measurements (pre-merge):
functionsread0(current strict)functions.read:0(native, as this PR preserves)call_abc123def456(OpenAI style)Human Verification (required)
What I personally verified (not just CI), and how:
buildOpenAICompatibleReplayPolicy("openai-completions", { sanitizeToolCallIds: false })omits bothsanitizeToolCallIdsandtoolCallIdModeand keeps the openai-completions-shaped fields."openai-responses"(which has the non-openai-completions shape).buildProviderReplayFamilyHooks({ family: "openai-compatible", sanitizeToolCallIds: false })threads through to the helper.pnpm build— bundled plugin dist emits cleanly, no[INEFFECTIVE_DYNAMIC_IMPORT]warnings.pnpm plugin-sdk:api:check— no baseline drift (the added optional field lives on a module-internal type).pnpm check:changed— typecheck (core + core tests + extensions + extension tests), lint (core + extensions), import cycles, webhook/pairing guards all green.openai-completionsvsopenai-responsesbranch (both covered by unit tests).OPENAI_COMPATIBLE_REPLAY_HOOKSwithout the opt-out.kimi-codingplugin behavior. The 2026-04-10 comment flagging it as "also affected" is not consistent with its code (api: "anthropic-messages", minimalKIMI_REPLAY_POLICY, defaultsanitizeToolCallIds: falsemerge). Left for a separate issue/PR with a fresh repro.pnpm check:changedreports 2 failures insrc/agents/tools/web-fetch.provider-fallback.test.ts(SSRF guard rejects DNS that resolves to a private IP in my local network environment). These fail identically on a stashed cleanupstream/main— not introduced by this PR. All guards relevant to this diff are green.Review Conversations
Compatibility / Migration
sanitizeToolCallIdsdefaults totrue;OPENAI_COMPATIBLE_REPLAY_HOOKSunchanged; only Moonshot opts out).Risks and Mitigations
sanitizeToolCallIds: false); defaults still sanitize. The inline comment atextensions/moonshot/index.tsdocuments the rationale so future readers understand the opt-out is Kimi-specific.tool_call_id; Kimi's native format is already valid wire-level and works across all three formats tested in the issue.