Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
51 changes: 37 additions & 14 deletions src/kimi_cli/tools/file/replace.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Comment on lines +182 to +198

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.

🔴 Edits spanning multiple lines stop working on Windows-style text files

The file's stored line endings are now matched literally against the model-supplied text (p.read_bytes() at src/kimi_cli/tools/file/replace.py:153) instead of being normalized first, so any edit spanning more than one line in a file saved with Windows line endings never matches and the edit is rejected.
Impact: Users editing CRLF files (common on Windows or in repos with CRLF checkouts) get "No replacements were made" for every multi-line edit.

Newline normalization lost by switching from text read to raw byte read

Previously the file was read with p.read_text(errors="replace"), which goes through kaos.readtext (packages/kaos/src/kaos/local.py:116-125) using Python's default universal-newlines mode, so \r\n in the file became \n in memory and a model-provided old containing \n matched.

The model only ever sees \n, because ReadFile also iterates lines in text mode (src/kimi_cli/tools/file/read.py:181 uses p.read_lines(...), universal newlines).

Now raw = await p.read_bytes() keeps \r\n, and _apply_edit_bytes encodes old as UTF-8 (src/kimi_cli/tools/file/replace.py:97) and searches the raw stream, so b"foo\nbar" cannot match b"foo\r\nbar"; the tool returns the "No replacements were made" error at src/kimi_cli/tools/file/replace.py:160-165.

The same literal-byte matching also breaks edits containing non-ASCII characters in files stored in a non-UTF-8 encoding (e.g. GBK/latin-1), which the old lossy-decode path could at least partially match.

A fix would be to detect the file's dominant line ending (and/or try a CRLF-normalized variant of old/new) before doing the byte-level search, while still writing back raw bytes.

Prompt for agents
StrReplaceFile now reads the file with read_bytes and matches the model-supplied `old` string as raw UTF-8 bytes (src/kimi_cli/tools/file/replace.py, _apply_edit_bytes). Previously it read with read_text, which performs universal-newline translation, so CRLF files were seen as LF and multi-line `old` strings containing \n matched. ReadFile also exposes file content to the model with universal newlines (src/kimi_cli/tools/file/read.py), so the model always emits \n. As a result, any multi-line edit against a CRLF-terminated file now fails with 'No replacements were made'. Consider detecting the file's line-ending style from the raw bytes and translating `old`/`new` accordingly (e.g. converting \n to \r\n when the target region/file uses CRLF), or falling back to a CRLF-normalized search when the literal byte search finds nothing, while still writing raw bytes so non-UTF-8 regions are preserved.
Open in Devin Review

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in 473b141: StrReplaceFile now detects the file's dominant line ending (CRLF if any CRLF is present) and rewrites model-supplied old/new (always LF) to match before the byte search, while still writing raw bytes so non-UTF-8 regions stay intact. Added regression tests for multi-line CRLF edits and CRLF + invalid UTF-8.


# 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
)
Expand All @@ -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,
Expand Down
20 changes: 20 additions & 0 deletions tests/tools/test_str_replace_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Loading