Skip to content

fix(kosong): recursively decode double-encoded tool-call arguments#2513

Open
nitishagar wants to merge 1 commit into
MoonshotAI:mainfrom
nitishagar:fix/2406-double-encoded-tool-args
Open

fix(kosong): recursively decode double-encoded tool-call arguments#2513
nitishagar wants to merge 1 commit into
MoonshotAI:mainfrom
nitishagar:fix/2406-double-encoded-tool-args

Conversation

@nitishagar

@nitishagar nitishagar commented Jul 19, 2026

Copy link
Copy Markdown

Summary

  • The Moonshot API can return function.arguments with nested array/object values as JSON strings (double-encoding). After a single json.loads, values like todos remained strings and failed Pydantic validation (Input should be a valid list).
  • Adds a shared decode_tool_arguments helper in kosong.utils.json_args that recursively unwraps any string that itself decodes to a dict or list. Scalars ("42", "true") and non-JSON strings ("{oops}") are left unchanged, so genuine string fields are not corrupted.
  • Consumed by both kosong.tooling.simple.SimpleToolset.handle and kimi_cli.soul.toolset.KimiToolset.handle.
  • Bumps kosong 0.55.00.56.0 and the kimi-cli dep pin; both changelogs updated under ## Unreleased.

Test plan

  • New packages/kosong/tests/utils/test_json_args.py covers E1–E8 (top-level and nested double-encoding, non-JSON strings preserved, scalar strings preserved, None/empty handling, normal passthrough, list-typed outer, mid-bracket values), outer-malformed re-raises, and two termination cases (bounded structural generators — no exponential string growth).
  • New tests/core/test_toolset.py::test_handle_unwraps_double_encoded_arguments reproduces the reported SetTodoList-style failure (string todos → list) and asserts the tool now executes.

Prior art

A prior PR #2407 attempted this but was closed without review. This PR supersedes it with a recursive (not single-level) unwrap, a shared helper, and the full edge-case suite.

Closes #2406; supersedes #2407 (closed).


Open in Devin Review

Some providers (notably Moonshot) return function.arguments with nested
array/object values as JSON strings. A single json.loads left them as
strings, failing Pydantic validation on tools like SetTodoList/ExitPlanMode/
StrReplaceFile. Add a shared decode_tool_arguments helper in kosong.utils
that recursively unwraps strings parsing to a dict or list (scalars and
non-JSON strings are left untouched), and consume it from both
SimpleToolset.handle and kimi-cli's KimiToolset.handle.

Bumps kosong 0.55.0 -> 0.56.0 and the kimi-cli dep pin.

Closes MoonshotAI#2406

@devin-ai-integration devin-ai-integration Bot left a comment

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.

Devin Review found 1 potential issue.

Open in Devin Review

Comment on lines +37 to +44
if isinstance(value, str):
try:
parsed = json.loads(value, strict=False)
except (json.JSONDecodeError, ValueError):
return value
if isinstance(parsed, (dict, list)):
return _unwrap(parsed)
return value

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.

🔴 Writing or editing files whose contents are valid JSON now fails

Any string argument value whose text happens to be a valid JSON object or array is silently turned into structured data (_unwrap at packages/kosong/src/kosong/utils/json_args.py:37-44) before the tool runs, so a request to write or edit a file with JSON contents has its text replaced by a dict/list and is then rejected.
Impact: Common operations like writing package.json/tsconfig.json or replacing a JSON snippet in a file break with a validation error, even though the model sent perfectly valid input.

How the recursive unwrap corrupts string-typed parameters

The fix targets double-encoded params (e.g. todos returned as a JSON string). But _unwrap unconditionally re-parses ANY string that decodes to a dict or list, with no knowledge of the tool's declared parameter types. Consider WriteFile (src/kimi_cli/tools/file/write.py:29, content: str) writing a JSON file. The provider sends arguments = '{"path":"a.json","content":"{\"a\":1}"}'. The outer json.loads at packages/kosong/src/kosong/utils/json_args.py:60 correctly yields content as the string {"a":1}. Then _unwrap sees that string parses to a dict and replaces it with the dict {"a":1} (json_args.py:42-43).

When CallableTool2.call runs self.params.model_validate(arguments) (packages/kosong/src/kosong/tooling/__init__.py:296), pydantic's lax mode does NOT coerce a dict/list into a str field, so it raises ValidationErrorToolValidateError. The same applies to StrReplaceFile.Edit.old/new (src/kimi_cli/tools/file/replace.py:23-24) when replacing JSON text. Previously (single json.loads) these worked correctly.

Prompt for agents
The recursive _unwrap in packages/kosong/src/kosong/utils/json_args.py promotes ANY string that parses to a dict/list into that structured value, with no awareness of the tool parameter schema. This corrupts legitimate string-typed parameters whose content is valid JSON text. Concrete regressions: WriteFile.content (src/kimi_cli/tools/file/write.py, content: str) writing a JSON file (e.g. package.json) — the content string {"a":1} gets turned into a dict and then rejected by pydantic model_validate (packages/kosong/src/kosong/tooling/__init__.py:296) since dict cannot coerce to str; and StrReplaceFile.Edit.old/new (src/kimi_cli/tools/file/replace.py) replacing JSON snippets.

The double-encoding unwrap needs to be schema-aware rather than blindly promoting every JSON-looking string. Possible approaches: (1) only unwrap a string when the target field's declared type actually expects a list/dict (requires plumbing the pydantic params model into the decode step), or (2) perform the unwrap lazily only when initial validation fails and retry, or (3) restrict unwrapping so that string values are only promoted when validation would otherwise fail. Any fix must preserve the ability to write/edit files whose contents are valid JSON documents.
Open in Devin Review

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: Tool call arguments double-encoding breaks array/dict parameters (Moonshot API)

1 participant