From dd3dfc4e28f7b38859480ec1222951434bc0348b Mon Sep 17 00:00:00 2001 From: 686f6c61 Date: Thu, 6 Aug 2026 19:33:04 +0200 Subject: [PATCH 1/4] fix(tools): preserve non-UTF-8 bytes in StrReplaceFile edits StrReplaceFile used read_text(errors=replace) and write_text, so any invalid UTF-8 sequence anywhere in the file became U+FFFD (EF BF BD) even when the edit only touched valid text elsewhere (#2591). Apply old/new as UTF-8 byte substrings on the raw buffer and write_bytes so unrelated regions are bit- identical. Fixes #2591 --- src/kimi_cli/tools/file/replace.py | 51 ++++++++++++++++++++-------- tests/tools/test_str_replace_file.py | 20 +++++++++++ 2 files changed, 57 insertions(+), 14 deletions(-) diff --git a/src/kimi_cli/tools/file/replace.py b/src/kimi_cli/tools/file/replace.py index 4f551de4f4..17cb884c84 100644 --- a/src/kimi_cli/tools/file/replace.py +++ b/src/kimi_cli/tools/file/replace.py @@ -79,12 +79,32 @@ async def _validate_path(self, path: KaosPath) -> ToolError | None: return None def _apply_edit(self, content: str, edit: Edit) -> str: - """Apply a single edit to the content.""" + """Apply a single edit to the content (string form; for display/tests).""" if edit.replace_all: return content.replace(edit.old, edit.new) else: return content.replace(edit.old, edit.new, 1) + def _apply_edit_bytes(self, content: bytes, edit: Edit) -> bytes: + """Apply a single edit on raw bytes so non-UTF-8 regions stay intact. + + ``old``/``new`` come from the model as Unicode and are encoded as UTF-8. + Searching/replacing in the raw byte stream avoids the + decode(errors=replace) → edit → re-encode round-trip that permanently + rewrites invalid sequences (e.g. ``\\xff`` → U+FFFD / ``EF BF BD``) + far from the requested edit (#2591). + """ + old_b = edit.old.encode("utf-8") + new_b = edit.new.encode("utf-8") + if not old_b: + return content + if edit.replace_all: + return content.replace(old_b, new_b) + idx = content.find(old_b) + if idx < 0: + return content + return content[:idx] + new_b + content[idx + len(old_b) :] + @override async def __call__(self, params: Params) -> ToolReturnValue: if not params.path: @@ -128,23 +148,24 @@ async def __call__(self, params: Params) -> ToolReturnValue: brief="Invalid path", ) - # Read the file content - content = await p.read_text(errors="replace") - - original_content = content + # Read raw bytes so non-UTF-8 sequences outside the edit are preserved + # (#2591 / same whole-file rewrite class as #2191). + raw = await p.read_bytes() + original_raw = raw edits = [params.edit] if isinstance(params.edit, Edit) else params.edit - # Apply all edits for edit in edits: - content = self._apply_edit(content, edit) + raw = self._apply_edit_bytes(raw, edit) - # Check if any changes were made - if content == original_content: + if raw == original_raw: return ToolError( message="No replacements were made. The old string was not found in the file.", brief="No replacements made", ) + # Diff is display-only: lossy decode is fine for the approval UI. + original_content = original_raw.decode("utf-8", errors="replace") + content = raw.decode("utf-8", errors="replace") diff_blocks: list[DisplayBlock] = await build_diff_blocks( str(p), original_content, content ) @@ -166,16 +187,18 @@ async def __call__(self, params: Params) -> ToolReturnValue: if not result: return result.rejection_error() - # Write the modified content back to the file - await p.write_text(content, errors="replace") + await p.write_bytes(raw) - # Count changes for success message + # Count changes for success message (byte-accurate for the edit strings) total_replacements = 0 for edit in edits: + old_b = edit.old.encode("utf-8") + if not old_b: + continue if edit.replace_all: - total_replacements += original_content.count(edit.old) + total_replacements += original_raw.count(old_b) else: - total_replacements += 1 if edit.old in original_content else 0 + total_replacements += 1 if old_b in original_raw else 0 return ToolReturnValue( is_error=False, diff --git a/tests/tools/test_str_replace_file.py b/tests/tools/test_str_replace_file.py index a16dad303b..f7ca9a6393 100644 --- a/tests/tools/test_str_replace_file.py +++ b/tests/tools/test_str_replace_file.py @@ -246,3 +246,23 @@ async def test_replace_empty_strings( assert not result.is_error assert "successfully edited" in result.message assert await file_path.read_text() == "Hello !" + + +async def test_replace_preserves_invalid_utf8_bytes_outside_edit( + str_replace_file_tool: StrReplaceFile, temp_work_dir: KaosPath +): + """Invalid UTF-8 far from the edit must not become U+FFFD (#2591).""" + file_path = temp_work_dir / "mixed.bin" + # 25 bytes: valid text with a lone 0xff between spaces + original = b"alpha\nbeta \xff gamma\ndelta\n" + await file_path.write_bytes(original) + + result = await str_replace_file_tool( + Params(path=str(file_path), edit=Edit(old="alpha", new="ALPHA")) + ) + + assert not result.is_error + out = await file_path.read_bytes() + assert out == b"ALPHA\nbeta \xff gamma\ndelta\n" + assert b"\xef\xbf\xbd" not in out # U+FFFD as UTF-8 + assert len(out) == len(original) + (len(b"ALPHA") - len(b"alpha")) From 473b141a961f69dce129b6a7e1f77e82e4db2898 Mon Sep 17 00:00:00 2001 From: 686f6c61 Date: Thu, 6 Aug 2026 19:38:25 +0200 Subject: [PATCH 2/4] fix(tools): match CRLF when applying multi-line StrReplaceFile edits Byte-level matching preserved non-UTF-8 regions but broke multi-line edits on CRLF files because the model always supplies LF. Detect the file's dominant line ending and rewrite old/new before the byte search. --- src/kimi_cli/tools/file/replace.py | 33 +++++++++++++++++++--- tests/tools/test_str_replace_file.py | 42 ++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/src/kimi_cli/tools/file/replace.py b/src/kimi_cli/tools/file/replace.py index 17cb884c84..ec44936f80 100644 --- a/src/kimi_cli/tools/file/replace.py +++ b/src/kimi_cli/tools/file/replace.py @@ -85,17 +85,41 @@ def _apply_edit(self, content: str, edit: Edit) -> str: else: return content.replace(edit.old, edit.new, 1) + @staticmethod + def _detect_line_ending(content: bytes) -> bytes: + """Return the file's dominant newline bytes (CRLF if any, else LF). + + ReadFile exposes lines with universal newlines, so the model always + supplies ``\\n`` in multi-line ``old``/``new``. Byte matching must + re-apply the on-disk ending or CRLF files reject every multi-line edit. + """ + if b"\r\n" in content: + return b"\r\n" + return b"\n" + + @staticmethod + def _encode_edit_text(text: str, line_ending: bytes) -> bytes: + """Encode model text as UTF-8 using the file's on-disk newlines.""" + # Model / tool JSON always uses LF; normalize any mixed endings first. + normalized = text.replace("\r\n", "\n").replace("\r", "\n") + encoded = normalized.encode("utf-8") + if line_ending == b"\r\n": + return encoded.replace(b"\n", b"\r\n") + return encoded + def _apply_edit_bytes(self, content: bytes, edit: Edit) -> bytes: """Apply a single edit on raw bytes so non-UTF-8 regions stay intact. - ``old``/``new`` come from the model as Unicode and are encoded as UTF-8. + ``old``/``new`` come from the model as Unicode and are encoded as UTF-8, + with newlines rewritten to match the file's dominant line ending. Searching/replacing in the raw byte stream avoids the decode(errors=replace) → edit → re-encode round-trip that permanently rewrites invalid sequences (e.g. ``\\xff`` → U+FFFD / ``EF BF BD``) far from the requested edit (#2591). """ - old_b = edit.old.encode("utf-8") - new_b = edit.new.encode("utf-8") + line_ending = self._detect_line_ending(content) + old_b = self._encode_edit_text(edit.old, line_ending) + new_b = self._encode_edit_text(edit.new, line_ending) if not old_b: return content if edit.replace_all: @@ -190,9 +214,10 @@ async def __call__(self, params: Params) -> ToolReturnValue: await p.write_bytes(raw) # Count changes for success message (byte-accurate for the edit strings) + line_ending = self._detect_line_ending(original_raw) total_replacements = 0 for edit in edits: - old_b = edit.old.encode("utf-8") + old_b = self._encode_edit_text(edit.old, line_ending) if not old_b: continue if edit.replace_all: diff --git a/tests/tools/test_str_replace_file.py b/tests/tools/test_str_replace_file.py index f7ca9a6393..73e086b3cf 100644 --- a/tests/tools/test_str_replace_file.py +++ b/tests/tools/test_str_replace_file.py @@ -266,3 +266,45 @@ async def test_replace_preserves_invalid_utf8_bytes_outside_edit( assert out == b"ALPHA\nbeta \xff gamma\ndelta\n" assert b"\xef\xbf\xbd" not in out # U+FFFD as UTF-8 assert len(out) == len(original) + (len(b"ALPHA") - len(b"alpha")) + + +async def test_replace_multiline_crlf_file( + str_replace_file_tool: StrReplaceFile, temp_work_dir: KaosPath +): + """Multi-line edits must match CRLF files (model always supplies LF).""" + file_path = temp_work_dir / "crlf.txt" + original = b"Line 1\r\nLine 2\r\nLine 3\r\n" + await file_path.write_bytes(original) + + result = await str_replace_file_tool( + Params( + path=str(file_path), + edit=Edit(old="Line 2\nLine 3", new="Modified 2\nModified 3"), + ) + ) + + assert not result.is_error + out = await file_path.read_bytes() + assert out == b"Line 1\r\nModified 2\r\nModified 3\r\n" + # Non-edited region stays bit-identical (CRLF preserved) + assert out.startswith(b"Line 1\r\n") + + +async def test_replace_preserves_invalid_utf8_with_crlf_multiline( + str_replace_file_tool: StrReplaceFile, temp_work_dir: KaosPath +): + """CRLF multi-line edit must not corrupt invalid UTF-8 elsewhere.""" + file_path = temp_work_dir / "mixed-crlf.bin" + original = b"head\r\nkeep \xff me\r\ntail\r\n" + await file_path.write_bytes(original) + + result = await str_replace_file_tool( + Params( + path=str(file_path), + edit=Edit(old="head\nkeep", new="HEAD\nKEEP"), + ) + ) + assert not result.is_error + out = await file_path.read_bytes() + assert out == b"HEAD\r\nKEEP \xff me\r\ntail\r\n" + assert b"\xef\xbf\xbd" not in out From 4d385efdcce5fb1f01170f766b70431a4590f9c9 Mon Sep 17 00:00:00 2001 From: 686f6c61 Date: Thu, 6 Aug 2026 20:50:26 +0200 Subject: [PATCH 3/4] fix(tools): reject empty old string in StrReplaceFile Byte-path search no-ops on an empty needle; fail explicitly with a clear ToolError instead of the generic "no replacements" path. --- src/kimi_cli/tools/file/replace.py | 9 +++++++++ tests/tools/test_str_replace_file.py | 14 ++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/kimi_cli/tools/file/replace.py b/src/kimi_cli/tools/file/replace.py index ec44936f80..1d291875ab 100644 --- a/src/kimi_cli/tools/file/replace.py +++ b/src/kimi_cli/tools/file/replace.py @@ -178,6 +178,15 @@ async def __call__(self, params: Params) -> ToolReturnValue: original_raw = raw edits = [params.edit] if isinstance(params.edit, Edit) else params.edit + for edit in edits: + # Empty old is invalid: str.replace("", ...) is not a meaningful + # edit, and the byte path intentionally no-ops on empty needles. + if edit.old == "": + return ToolError( + message="The old string to replace cannot be empty.", + brief="Empty old string", + ) + for edit in edits: raw = self._apply_edit_bytes(raw, edit) diff --git a/tests/tools/test_str_replace_file.py b/tests/tools/test_str_replace_file.py index 73e086b3cf..bd1428bdbe 100644 --- a/tests/tools/test_str_replace_file.py +++ b/tests/tools/test_str_replace_file.py @@ -248,6 +248,20 @@ async def test_replace_empty_strings( assert await file_path.read_text() == "Hello !" +async def test_replace_rejects_empty_old_string( + str_replace_file_tool: StrReplaceFile, temp_work_dir: KaosPath +): + """Empty old is not a valid edit (Copilot review on byte-path semantics).""" + file_path = temp_work_dir / "test.txt" + await file_path.write_text("hello") + + result = await str_replace_file_tool(Params(path=str(file_path), edit=Edit(old="", new="x"))) + + assert result.is_error + assert "cannot be empty" in result.message + assert await file_path.read_text() == "hello" + + async def test_replace_preserves_invalid_utf8_bytes_outside_edit( str_replace_file_tool: StrReplaceFile, temp_work_dir: KaosPath ): From 169363af5653071e7b163b677268c8fc907a2b99 Mon Sep 17 00:00:00 2001 From: 686f6c61 Date: Sat, 8 Aug 2026 00:01:53 +0200 Subject: [PATCH 4/4] fix(tools): count-dominant line endings in StrReplaceFile Detect CRLF vs LF by occurrence count instead of "any CRLF wins", so a mostly-LF file with a stray CRLF still matches model-supplied multi-line edits. Addresses Copilot feedback on mixed-newline files. --- src/kimi_cli/tools/file/replace.py | 11 +++++++++-- tests/tools/test_str_replace_file.py | 21 +++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/kimi_cli/tools/file/replace.py b/src/kimi_cli/tools/file/replace.py index 1d291875ab..e2e4e9eea1 100644 --- a/src/kimi_cli/tools/file/replace.py +++ b/src/kimi_cli/tools/file/replace.py @@ -87,13 +87,20 @@ def _apply_edit(self, content: str, edit: Edit) -> str: @staticmethod def _detect_line_ending(content: bytes) -> bytes: - """Return the file's dominant newline bytes (CRLF if any, else LF). + """Return the file's dominant newline bytes by counting occurrences. ReadFile exposes lines with universal newlines, so the model always supplies ``\\n`` in multi-line ``old``/``new``. Byte matching must re-apply the on-disk ending or CRLF files reject every multi-line edit. + + Dominance is by count (not “any CRLF wins”): a mostly-LF file with a + stray ``\\r\\n`` keeps LF so multi-line ``old`` still matches. + Ties prefer LF. """ - if b"\r\n" in content: + crlf = content.count(b"\r\n") + # Newlines that are not the second byte of a CRLF pair. + lf_only = content.count(b"\n") - crlf + if crlf > lf_only: return b"\r\n" return b"\n" diff --git a/tests/tools/test_str_replace_file.py b/tests/tools/test_str_replace_file.py index bd1428bdbe..aa3a20b690 100644 --- a/tests/tools/test_str_replace_file.py +++ b/tests/tools/test_str_replace_file.py @@ -304,6 +304,27 @@ async def test_replace_multiline_crlf_file( assert out.startswith(b"Line 1\r\n") +async def test_replace_multiline_mostly_lf_with_stray_crlf( + str_replace_file_tool: StrReplaceFile, temp_work_dir: KaosPath +): + """A stray CRLF must not force whole-file CRLF rewriting of old/new (Copilot).""" + file_path = temp_work_dir / "mostly-lf.txt" + # Three LF newlines dominate a single CRLF elsewhere in the file. + original = b"Line 1\nLine 2\nLine 3\ntrailer\r\n" + await file_path.write_bytes(original) + + result = await str_replace_file_tool( + Params( + path=str(file_path), + edit=Edit(old="Line 2\nLine 3", new="Modified 2\nModified 3"), + ) + ) + + assert not result.is_error + out = await file_path.read_bytes() + assert out == b"Line 1\nModified 2\nModified 3\ntrailer\r\n" + + async def test_replace_preserves_invalid_utf8_with_crlf_multiline( str_replace_file_tool: StrReplaceFile, temp_work_dir: KaosPath ):