MoonshotAI / MoonshotAI/kimi-cli
StrReplaceFile corrupts undecodable bytes outside the edited region
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 11.4k
- Forks
- 1.3k
- Avg merge
- 9h 47m
- Merged PRs (30d)
- 2
Description
Summary
StrReplaceFile decodes the whole file with errors="replace", edits the string, and writes the whole string back. Any byte in the file that isn't valid UTF-8 — anywhere, including far from the edit — is replaced by U+FFFD and written to disk as EF BF BD. The file's length and contents change outside the requested edit, permanently, and the approval diff can't show it because the diff is computed from the already-lossy string.
This is the same root cause as #2191 / #1952 (whole-file round-trip silently rewriting content the edit never touched), but for undecodable bytes rather than line endings.
Where
src/kimi_cli/tools/file/replace.py
# line 132
content = await p.read_text(errors="replace")
...
# line 170
await p.write_text(content, errors="replace")
Reproduction
A file with a single invalid byte, unrelated to the edit:
orig = b"alpha\nbeta \xff gamma\ndelta\n" # 25 bytes
content = orig.decode("utf-8", errors="replace")
content = content.replace("alpha", "ALPHA") # the requested edit
out = content.encode("utf-8")
before: b'beta \xff gamma' 25 bytes
after: b'beta \xef\xbf\xbd gamma' 27 bytes
The \xff is gone and the file grew by two bytes, on an edit that only asked to touch alpha.
One thing worth flagging before anyone fixes this
Dropping errors="replace" from the write at line 170 does not fix it. I checked — U+FFFD is a perfectly valid character to encode, so it still writes EF BF BD:
read=replace write=replace -> b'beta \xef\xbf\xbd gamma' 25->27
read=replace write=strict -> b'beta \xef\xbf\xbd gamma' 25->27 (identical)
The loss happens at the read. That said, line 170 does look inconsistent with the project's own rule in tests_ai/test_encoding_error_handling.md — "Writing files and encoding Python strings to bytes do not require errors="replace"" — and with the sibling tool, write.py:158, which writes without it. Worth tidying either way, just not as the fix.
write.py can't hit this bug: it reads with errors="replace" only to build a display diff, and writes params.content (fresh model-supplied text). StrReplaceFile is the only file tool that reads, edits, and writes back the same content.
Possible directions
I didn't open a PR because the sensible fix is a design decision that's yours to make, and each option has a real cost:
-
surrogateescapeon both ends — byte-exact round-trip (verified: 25 → 25 bytes,\xffpreserved). But lone surrogates raiseUnicodeEncodeErrorwhen the string is later encoded, which this codebase has already been bitten by once in #420. The edited content flows intobuild_diff_blocksand the approval display, so that risk is live here. It would also neederrorswidening inkaos.path, where it's typedLiteral["strict", "ignore", "replace"]. -
Refuse the edit — read strict, and return a
ToolErrorwhen the file isn't valid UTF-8. Small, can't crash, and arguably right for a text-editing tool. Cost: it declines edits the tool currently performs (badly). -
Splice at the byte level — apply the replacement to the original bytes and only rewrite the changed span. Most correct, most work, and would also fix the CRLF class in #2191 / #1953.
Happy to send a PR for whichever direction you'd prefer.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in src/kimi_cli/tools/file/replace.py, then read tests_ai/tests_ai/test_encoding_error_handling.md and the sibling write.py implementation. Compare the listed handling options and trace how content reaches build_diff_blocks and approval display; done means a regression test covers an unrelated undecodable byte without silently rewriting it, with the chosen behavior documented and verified.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- cli
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100