diff --git a/CHANGELOG.md b/CHANGELOG.md index fa0812663..6cf149fab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ Only write entries that are worth mentioning to users. ## Unreleased +- Soul: Sessions killed mid-turn (e.g. under memory pressure) now resume correctly — `normalize_history` injects placeholder responses for orphan `tool_call_id`s so the next provider request stops failing with `400 ... tool_call_ids did not have response messages` + ## 1.47.0 (2026-06-05) - Shell: Guide users to the new standalone Kimi Code — adds a `/upgrade` command that installs it (migrating your config & sessions automatically), a welcome-screen nudge, and a once-per-day tip shown on exit diff --git a/src/kimi_cli/soul/dynamic_injection.py b/src/kimi_cli/soul/dynamic_injection.py index 16630cdd8..05aea7577 100644 --- a/src/kimi_cli/soul/dynamic_injection.py +++ b/src/kimi_cli/soul/dynamic_injection.py @@ -5,7 +5,7 @@ from dataclasses import dataclass from typing import TYPE_CHECKING -from kosong.message import Message +from kosong.message import Message, TextPart from kimi_cli.notifications import is_notification_message @@ -55,6 +55,61 @@ async def on_afk_changed(self, enabled: bool) -> None: return None +# Placeholder body for synthetic tool responses inserted to repair an +# assistant ``tool_calls`` message whose matching ``tool`` response was lost +# (e.g. when the previous session was killed mid-turn under memory pressure). +# Kept short so it adds minimal context tokens on resume. +_ORPHAN_TOOL_PLACEHOLDER = ( + "(tool result unavailable: the previous session was interrupted before " + "this tool call completed)" +) + + +def _repair_orphan_tool_calls(history: list[Message]) -> list[Message]: + """Insert placeholder tool responses for any orphan ``tool_call_id``. + + A persisted assistant message can carry ``tool_calls`` without the + matching ``tool`` role responses ever being flushed -- e.g. the CLI + process was killed mid-turn or the worker crashed before the tool + result was written. OpenAI-compatible providers then reject the next + request with ``400 ... tool_call_ids did not have response messages``, + permanently breaking conversation resume (regression #2336). + + Scan the history for assistant messages with ``tool_calls``, gather + their declared ids, and check the immediately following ``tool`` role + messages. For any id without a response, insert a synthetic placeholder + so the API call shape stays valid. The stored history itself is left + untouched -- only the sequence sent to the provider is patched. + """ + if not history: + return history + result: list[Message] = [] + for i, msg in enumerate(history): + result.append(msg) + if msg.role != "assistant" or not msg.tool_calls: + continue + expected_ids = [tc.id for tc in msg.tool_calls if tc.id] + if not expected_ids: + continue + seen: set[str] = set() + for follower in history[i + 1 :]: + if follower.role != "tool": + break + if follower.tool_call_id: + seen.add(follower.tool_call_id) + for expected_id in expected_ids: + if expected_id in seen: + continue + result.append( + Message( + role="tool", + content=[TextPart(text=_ORPHAN_TOOL_PLACEHOLDER)], + tool_call_id=expected_id, + ) + ) + return result + + def normalize_history(history: Sequence[Message]) -> list[Message]: """Merge adjacent user messages to produce a clean API input sequence. @@ -64,6 +119,10 @@ def normalize_history(history: Sequence[Message]) -> list[Message]: Only ``user`` role messages are merged. Assistant and tool messages are never merged because their ``tool_calls`` / ``tool_call_id`` fields form linked pairs that must stay intact. + + Orphan ``tool_calls`` whose tool responses were lost (e.g. mid-turn + crash) are patched with placeholder ``tool`` messages so the provider + request stays well-formed; see ``_repair_orphan_tool_calls``. """ if not history: return [] @@ -81,4 +140,4 @@ def normalize_history(history: Sequence[Message]) -> list[Message]: result[-1] = Message(role="user", content=merged_content) else: result.append(msg) - return result + return _repair_orphan_tool_calls(result) diff --git a/tests/core/test_normalize_history.py b/tests/core/test_normalize_history.py index 8b6bb77df..d18e2e8cc 100644 --- a/tests/core/test_normalize_history.py +++ b/tests/core/test_normalize_history.py @@ -2,7 +2,7 @@ from __future__ import annotations -from kosong.message import ContentPart, Message, TextPart +from kosong.message import ContentPart, Message, TextPart, ToolCall from kimi_cli.soul.dynamic_injection import normalize_history @@ -12,6 +12,10 @@ def _text(part: ContentPart) -> str: return part.text +def _tool_call(call_id: str, name: str = "Shell") -> ToolCall: + return ToolCall(id=call_id, function=ToolCall.FunctionBody(name=name, arguments="{}")) + + def test_empty_history() -> None: assert normalize_history([]) == [] @@ -123,3 +127,127 @@ def test_notification_messages_not_merged_with_user_messages() -> None: ] result = normalize_history(msgs) assert len(result) == 2 + + +# --------------------------------------------------------------------------- +# Orphan tool_call repair (regression #2336) +# --------------------------------------------------------------------------- + + +def test_orphan_tool_call_synthesized_when_followed_by_user() -> None: + """A persisted assistant message whose tool response was lost must + not break the next API call -- a placeholder tool message is inserted.""" + msgs = [ + Message(role="user", content=[TextPart(text="run it")]), + Message( + role="assistant", + content=[TextPart(text="ok")], + tool_calls=[_tool_call("Shell:206")], + ), + Message(role="user", content=[TextPart(text="any update?")]), + ] + result = normalize_history(msgs) + + assert [m.role for m in result] == ["user", "assistant", "tool", "user"] + assert result[2].tool_call_id == "Shell:206" + assert "interrupted" in _text(result[2].content[0]) + + +def test_orphan_tool_call_synthesized_at_history_tail() -> None: + """Assistant with tool_calls at the very end of history (no follower) + should still get a placeholder so the next /resume turn is valid.""" + msgs = [ + Message(role="user", content=[TextPart(text="run it")]), + Message( + role="assistant", + content=[TextPart(text="ok")], + tool_calls=[_tool_call("Shell:1")], + ), + ] + result = normalize_history(msgs) + + assert [m.role for m in result] == ["user", "assistant", "tool"] + assert result[2].tool_call_id == "Shell:1" + + +def test_complete_tool_response_not_duplicated() -> None: + """A well-formed assistant+tool pair must round-trip unchanged.""" + msgs = [ + Message(role="user", content=[TextPart(text="hi")]), + Message( + role="assistant", + content=[TextPart(text="")], + tool_calls=[_tool_call("t1")], + ), + Message(role="tool", content=[TextPart(text="result")], tool_call_id="t1"), + ] + result = normalize_history(msgs) + + assert [m.role for m in result] == ["user", "assistant", "tool"] + assert result[2].tool_call_id == "t1" + assert _text(result[2].content[0]) == "result" + + +def test_partial_orphan_only_missing_ids_synthesized() -> None: + """When parallel tool_calls have a mix of responded/missing ids, + only the missing ones get placeholders.""" + msgs = [ + Message( + role="assistant", + content=[TextPart(text="")], + tool_calls=[_tool_call("t1"), _tool_call("t2"), _tool_call("t3")], + ), + Message(role="tool", content=[TextPart(text="r1")], tool_call_id="t1"), + Message(role="tool", content=[TextPart(text="r3")], tool_call_id="t3"), + Message(role="user", content=[TextPart(text="next")]), + ] + result = normalize_history(msgs) + + tool_msgs = [m for m in result if m.role == "tool"] + assert sorted(m.tool_call_id for m in tool_msgs if m.tool_call_id) == ["t1", "t2", "t3"] + + synth = next(m for m in tool_msgs if m.tool_call_id == "t2") + assert "interrupted" in _text(synth.content[0]) + + +def test_multiple_assistant_tool_call_groups_independent() -> None: + """Repair must operate per assistant message; an earlier orphan + must not consume responses meant for a later assistant message.""" + msgs = [ + Message( + role="assistant", + content=[TextPart(text="")], + tool_calls=[_tool_call("a1")], + ), + # a1 orphan + Message(role="user", content=[TextPart(text="continue")]), + Message( + role="assistant", + content=[TextPart(text="")], + tool_calls=[_tool_call("a2")], + ), + Message(role="tool", content=[TextPart(text="ok")], tool_call_id="a2"), + ] + result = normalize_history(msgs) + + assert [m.role for m in result] == [ + "assistant", + "tool", + "user", + "assistant", + "tool", + ] + assert result[1].tool_call_id == "a1" + assert "interrupted" in _text(result[1].content[0]) + assert result[4].tool_call_id == "a2" + assert _text(result[4].content[0]) == "ok" + + +def test_assistant_without_tool_calls_untouched() -> None: + msgs = [ + Message(role="user", content=[TextPart(text="hi")]), + Message(role="assistant", content=[TextPart(text="hello")]), + Message(role="user", content=[TextPart(text="bye")]), + ] + result = normalize_history(msgs) + assert [m.role for m in result] == ["user", "assistant", "user"]