Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions packages/kosong/src/kosong/tooling/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,20 @@ def base(self) -> Tool:
async def call(self, arguments: JsonType) -> ToolReturnValue:
from kosong.tooling.error import ToolValidateError

# Fix LLM double-serialization: coerce string values that look like
# JSON arrays/objects back to their proper types before validation.
# LLMs sometimes emit: {"todos": "[{\\"title\\": ...}]"} instead of
# {"todos": [{"title": ...}]}
if isinstance(arguments, dict):
for k, v in list(arguments.items()):
if isinstance(v, str) and v.startswith(("[", "{")):
try:
parsed = json.loads(v, strict=False)
if isinstance(parsed, (list, dict)):
arguments[k] = parsed
except (json.JSONDecodeError, ValueError):
pass

try:
jsonschema.validate(arguments, self.parameters)
except jsonschema.ValidationError as e:
Expand Down
14 changes: 14 additions & 0 deletions src/kimi_cli/soul/toolset.py
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,20 @@ def handle(self, tool_call: ToolCall) -> HandleResult:
)
return ToolResult(tool_call_id=tool_call.id, return_value=ToolParseError(str(e)))

# Fix LLM double-serialization: coerce string values that look like
# JSON arrays/objects back to their proper types before tool validation.
# LLMs sometimes emit: {"todos": "[{\\"title\\": ...}]"} instead of
# {"todos": [{"title": ...}]}
if isinstance(arguments, dict):
for k, v in list(arguments.items()):
if isinstance(v, str) and v.startswith(("[", "{")):
try:
parsed = json.loads(v, strict=False)
if isinstance(parsed, (list, dict)):
arguments[k] = parsed
except (json.JSONDecodeError, ValueError):
pass

canonical_args = _canonical_tool_arguments(arguments)
call_key = (tool_name, canonical_args)
call_index = len(self._current_step_calls)
Expand Down
13 changes: 13 additions & 0 deletions src/kimi_cli/telemetry/crash.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,19 @@ def _asyncio_handler(
exc = context.get("exception")
# CancelledError during shutdown/cancellation is normal control flow.
if exc is not None and not isinstance(exc, asyncio.CancelledError):
# Suppress MCP connection errors (server disconnect, broken pipe, etc.)
# These are expected when MCP servers restart or connections drop.
exc_msg = str(exc).lower()
_mcp_connection_patterns = (
"connection closed",
"connection reset",
"connection refused",
"broken pipe",
"eof",
)
if any(p in exc_msg for p in _mcp_connection_patterns):
return # Silently suppress — not a programming bug
Comment on lines +127 to +128

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Early return bypasses delegation to original asyncio exception handler, silently swallowing non-MCP exceptions

The return at line 128 exits _asyncio_handler before the delegation block at lines 142-146 runs. This completely swallows exceptions matching the string patterns — no telemetry, no logging, and no call to the original/default exception handler.

This breaks the function's documented contract (line 142: "Delegate so the original logging behavior (or custom handler) runs") and is inconsistent with how _excepthook handles filtered exceptions — in _excepthook (lines 85-87), ignored exceptions (KeyboardInterrupt, SystemExit) still get delegated to the original handler, and this is verified by test_original_handler_called_for_ignored_exceptions.

Furthermore, the pattern matching is applied to ALL asyncio exceptions regardless of origin, not just MCP. The patterns like "connection closed", "connection refused", and "eof" could match exceptions from non-MCP sources such as the LLM API (APIConnectionError), the telemetry transport, web tools, or the wire server — all of which use asyncio networking. These would be silently discarded with no logging or diagnostics.

Prompt for agents
The early `return` at line 128 in _asyncio_handler skips the delegation to the original/default exception handler (lines 142-146). This is inconsistent with how _excepthook handles filtered exceptions (it always delegates).

To fix this while preserving the intent of suppressing telemetry for MCP connection errors:

Option A: Move the MCP check so it only skips telemetry tracking but still delegates. For example, add a flag like `is_mcp_conn_error` and gate only the `track()` call on it:

    if exc is not None and not isinstance(exc, asyncio.CancelledError):
        exc_msg = str(exc).lower()
        _mcp_connection_patterns = (...)
        is_mcp_conn_error = any(p in exc_msg for p in _mcp_connection_patterns)
        if not is_mcp_conn_error:
            try:
                track(...)
            except Exception:
                pass

    # Delegation always runs
    if _original_asyncio_handler is not None:
        _original_asyncio_handler(loop, context)
    else:
        loop.default_exception_handler(context)

Option B: If truly wanting to suppress these entirely (no logging), keep the early return but be more precise about what gets suppressed — e.g., check the exception type or source rather than just substring-matching error messages. The pattern 'eof' is especially broad and could match unrelated errors.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


try:
from kimi_cli.telemetry import track

Expand Down
30 changes: 28 additions & 2 deletions src/kimi_cli/tools/file/replace.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
from collections.abc import Callable
from pathlib import Path
from typing import override
from typing import Any, override

from kaos.path import KaosPath
from kosong.tooling import CallableTool2, ToolError, ToolReturnValue
from pydantic import BaseModel, Field
from pydantic import BaseModel, Field, field_validator

from kimi_cli.soul.agent import Runtime
from kimi_cli.soul.approval import Approval
Expand Down Expand Up @@ -39,6 +39,32 @@ class Params(BaseModel):
)
)

@field_validator("edit", mode="before")
@classmethod
def _coerce_string_edit(cls, v: Any) -> Any:
"""Handle LLM double-serialization: LLMs sometimes emit object/array
values as JSON-encoded strings instead of actual JSON structures.

Example of the bug:
LLM outputs: {"edit": "{\"old\": \"foo\", \"new\": \"bar\"}"}
Expected: {"edit": {"old": "foo", "new": "bar"}}

LLM outputs: {"edit": "[{\"old\": \"foo\", \"new\": \"bar\"}]"}
Expected: {"edit": [{"old": "foo", "new": "bar"}]}

Context7 verified: Pydantic field_validator(mode='before') runs before
the standard validation, allowing type coercion at the boundary.
"""
if isinstance(v, str):
try:
import json
parsed = json.loads(v)
if isinstance(parsed, (dict, list)):
return parsed
except (json.JSONDecodeError, ValueError):
pass
return v


class StrReplaceFile(CallableTool2[Params]):
name: str = "StrReplaceFile"
Expand Down
24 changes: 23 additions & 1 deletion src/kimi_cli/tools/todo/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from typing import Any, Literal, cast, override

from kosong.tooling import CallableTool2, ToolReturnValue
from pydantic import BaseModel, Field
from pydantic import BaseModel, Field, field_validator

from kimi_cli.session_state import TodoItemState
from kimi_cli.soul.agent import Runtime
Expand All @@ -26,6 +26,28 @@ class Params(BaseModel):
),
)

@field_validator("todos", mode="before")
@classmethod
def _coerce_string_todos(cls, v: Any) -> Any:
"""Handle LLM double-serialization: LLMs sometimes emit array values
as JSON-encoded strings instead of actual JSON arrays.

Example of the bug:
LLM outputs: {"todos": "[{\"title\": \"Fix bug\", \"status\": \"pending\"}]"}
Expected: {"todos": [{"title": "Fix bug", "status": "pending"}]}

Context7 verified: Pydantic field_validator(mode='before') runs before
the standard validation, allowing type coercion at the boundary.
"""
if isinstance(v, str):
try:
parsed = json.loads(v)
if isinstance(parsed, list):
return parsed
except (json.JSONDecodeError, ValueError):
pass
return v


class SetTodoList(CallableTool2[Params]):
name: str = "SetTodoList"
Expand Down