fix(kosong): recursively unwrap double-encoded JSON in tool-call arguments - #2572
fix(kosong): recursively unwrap double-encoded JSON in tool-call arguments#2572aalhadxx wants to merge 2 commits into
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| def _unwrap(value: object) -> object: | ||
| if isinstance(value, dict): | ||
| return {k: _unwrap(v) for k, v in cast("dict[str, object]", value).items()} | ||
| if isinstance(value, list): | ||
| return [_unwrap(x) for x in cast("list[object]", value)] | ||
| if isinstance(value, str): | ||
| # Fast-path: skip strings that cannot be JSON objects/arrays. | ||
| # The lstrip handles leading whitespace (rare but valid). | ||
| stripped = value.lstrip() | ||
| if not stripped or stripped[0] not in ("[", "{"): | ||
| return value | ||
| try: | ||
| parsed = json.loads(value, strict=False) | ||
| except (json.JSONDecodeError, ValueError): | ||
| return value | ||
| if isinstance(parsed, (dict, list)): | ||
| return _unwrap(parsed) | ||
| return value | ||
| return value |
There was a problem hiding this comment.
🔴 Tool calls whose text argument contains JSON break, e.g. writing a JSON file
Any text value that begins with a curly brace or square bracket is silently converted into structured data (json.loads at packages/kosong/src/kosong/utils/json_args.py:40) before the tool receives it, so ordinary operations such as writing or editing a JSON file now fail.
Impact: Writing or replacing content in JSON/JSONL files (and any other tool argument whose text happens to start with { or [) errors out instead of working.
Mechanism: unconditional recursive unwrap ignores the declared parameter type
decode_tool_arguments calls _unwrap (packages/kosong/src/kosong/utils/json_args.py:28-46) on every string in the payload, regardless of what type the tool's schema declares. For example WriteFile declares content: str (src/kimi_cli/tools/file/write.py:29). A call like {"path": "pkg.json", "content": "{\"a\": 1}"} is now decoded to {"path": ..., "content": {"a": 1}}, and Pydantic rejects it with "Input should be a valid string". The same applies to StrReplaceFile's old_string/new_string and to any text argument holding a JSON/array-looking snippet. The previous single json.loads preserved these values.
Additionally, _unwrap keeps recursing into the parsed result, so text nested inside genuinely structured arguments is corrupted the same way.
Prompt for agents
decode_tool_arguments in packages/kosong/src/kosong/utils/json_args.py unwraps ANY string value that parses as a JSON object/array, without consulting the tool's parameter schema. This corrupts genuine string parameters that legitimately hold JSON text — most notably WriteFile.content (src/kimi_cli/tools/file/write.py) and StrReplaceFile old_string/new_string — turning them into dicts/lists so Pydantic validation fails with 'Input should be a valid string'. Consider a schema-aware or failure-driven approach instead: e.g. validate the arguments as-is first and only attempt the double-encoding unwrap when validation fails, or only unwrap a field when the tool's JSON schema declares that field as array/object. Restricting unwrapping to the top level (not recursing into already-structured data) would also reduce blast radius.
Was this helpful? React with 👍 or 👎 to provide feedback.
…ments
Some providers (notably the Moonshot API) return function.arguments with
nested array/object values as JSON strings. A single json.loads leaves these
inner values as strings, which then fail Pydantic validation with errors like
"Input should be a valid list".
Adds kosong.utils.json_args.decode_tool_arguments — a shared helper that:
- Parses the outer JSON payload (re-raises JSONDecodeError for malformed outer
input so callers can surface ToolParseError, preserving today's contract).
- Recursively walks dicts/lists and unwraps any string that itself decodes to a
dict or list.
- Leaves scalar strings ("42", "true", "hello") untouched so genuine string
fields are never corrupted.
- Uses a fast-path prefix check (v[0] in "[", "{") to avoid unnecessary
json.loads calls on typical non-JSON string values.
Replaces the single json.loads call in both:
- kosong.tooling.simple.SimpleToolset.handle
- kimi_cli.soul.toolset.KimiToolset.handle
Tests:
- 23 unit tests covering E1–E10 edge cases, termination (deep nesting / bounded
structural generators), malformed outer input, regression (SetTodoList /
StrReplaceFile / ExitPlanMode shapes), mixed genuine+encoded fields, unicode.
- 3 integration tests in tests/core/test_toolset.py exercising the fix end-to-end
through KimiToolset.
Fixes MoonshotAI#2406; improves upon MoonshotAI#2513 (adds fast-path and broader edge-case coverage).
4e604e3 to
60e5702
Compare
…xt strings
The recursive unwrapping in decode_tool_arguments converts any string
starting with [ or { into structured data. This breaks tool calls where
text fields legitimately contain JSON text (e.g. WriteFile.content with
'{"foo": "bar"}').
This change implements a failure-driven approach in SimpleToolset.handle():
1. Try strict json.loads first (no recursive unwrapping)
2. Call the tool with strict arguments
3. If validation fails (ToolValidateError), retry with decode_tool_arguments
to handle double-encoded values
4. Return whichever result succeeds
This preserves the fix for double-encoded Moonshot API responses while
avoiding corruption of genuine JSON text strings in tool arguments.
Addresses devin-ai-integration review feedback on MoonshotAI#2572.
|
@devin-ai-integration good catch — you are absolutely right. Unconditionally unwrapping every string is too aggressive. Pushed a fix that makes the unwrapping failure-driven:
This means genuine JSON text in or strings stays untouched unless the strict parse actually fails validation — which is exactly the case for double-encoded Moonshot API responses. Also added regression tests documenting the case. Let me know if you see any other issues. |
1 participant
Problem
Tool calls with array or object parameters (e.g. SetTodoList, ExitPlanMode, StrReplaceFile) fail with Pydantic validation errors when using providers that double-encode nested values:
The Moonshot API returns "function.arguments" where inner array/object values are themselves JSON strings. A single json.loads leaves these as strings, which then fail validation.
A prior fix was attempted in #2513 (Devin found 1 potential issue; author has not responded). This PR supersedes it with a cleaner implementation and broader test coverage.
Solution
Introduce — a shared helper consumed by both and .
Key improvements over #2513:
Files changed
Tests
Unit tests cover:
Integration tests verify the fix end-to-end through with a dummy tool mirroring 's parameter.
Fixes #2406
Supersedes / improves upon #2513