fix(tools): fail StrReplaceFile when a multi-edit hunk is unmatched

src/kimi_cli/tools/file/replace.py

@@ -134,15 +134,61 @@ async def __call__(self, params: Params) -> ToolReturnValue:
             original_content = content
             edits = [params.edit] if isinstance(params.edit, Edit) else params.edit
 
-            # Apply all edits
-            for edit in edits:
+            # Apply edits sequentially. Each edit applies to the result of the
+            # previous one, so a later edit may legitimately target text an
+            # earlier edit introduced. Validate that every edit's old string is
+            # present at its turn and fail the whole call otherwise: silently
+            # skipping a non-matching edit while still reporting success drops
+            # requested changes without the model noticing.
+            total_replacements = 0
+            for index, edit in enumerate(edits):
+                # Reject an empty old string: `str.count('')` returns
+                # `len(content) + 1` (never 0), so the occurrence check below
+                # cannot catch it, and `str.replace('', new)` would splice
+                # `new` between every character.
+                if edit.old == "":
+                    detail = (
+                        "old string must not be empty."
+                        if len(edits) == 1
+                        else f"Edit #{index + 1}'s old string must not be empty."
+                    )
+                    return ToolError(
+                        message=f"No replacements were made. {detail}",
+                        brief="Empty old string",
+                    )
+                occurrences = content.count(edit.old)
+                if occurrences == 0:
+                    detail = (
+                        "The old string was not found in the file."
+                        if len(edits) == 1
+                        else (
+                            f"Edit #{index + 1}'s old string was not found "
+                            "(an earlier edit in this call may have changed that text)."
+                        )
+                    )
+                    return ToolError(
+                        message=(
+                            f"No replacements were made. {detail} "
+                            "The file contents may be out of date; "
+                            "use the Read tool to reload it."
+                        ),
+                        brief="No replacements made",
+                    )
                 content = self._apply_edit(content, edit)
+                total_replacements += occurrences if edit.replace_all else 1
 
-            # Check if any changes were made
+            # Every edit matched (validated above) but the result is identical
+            # to the original — e.g. an edit whose old string equals its new
+            # string, or edits that cancel out. Report a no-op rather than the
+            # misleading "old string was not found", which would send the model
+            # re-reading the file for a string that is actually present.
             if content == original_content:
                 return ToolError(
-                    message="No replacements were made. The old string was not found in the file.",
-                    brief="No replacements made",
+                    message=(
+                        "No replacements were made: the edit(s) matched but left the file "
+                        "unchanged (an old string equals its new string, or the edits cancel out)."
+                    ),
+                    brief="No changes made",
                 )
 
             diff_blocks: list[DisplayBlock] = await build_diff_blocks(
@@ -169,14 +215,6 @@ async def __call__(self, params: Params) -> ToolReturnValue:
             # Write the modified content back to the file
             await p.write_text(content, errors="replace")
 
-            # Count changes for success message
-            total_replacements = 0
-            for edit in edits:
-                if edit.replace_all:
-                    total_replacements += original_content.count(edit.old)
-                else:
-                    total_replacements += 1 if edit.old in original_content else 0
-
             return ToolReturnValue(
                 is_error=False,
                 output="",

tests/tools/test_str_replace_file.py

@@ -246,3 +246,90 @@ 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_multiple_edits_one_not_found_is_atomic(
+    str_replace_file_tool: StrReplaceFile, temp_work_dir: KaosPath
+):
+    """A multi-edit where one edit's old string is missing must fail the whole
+    call, not silently apply the matching edits and report success."""
+    file_path = temp_work_dir / "test.txt"
+    original_content = "Hello world! Goodbye world!"
+    await file_path.write_text(original_content)
+
+    result = await str_replace_file_tool(
+        Params(
+            path=str(file_path),
+            edit=[
+                Edit(old="Hello", new="Hi"),
+                Edit(old="does-not-exist", new="x"),
+            ],
+        )
+    )
+
+    assert result.is_error
+    assert "No replacements were made" in result.message
+    # The whole batch fails atomically — the file is left untouched even though
+    # the first edit on its own would have matched.
+    assert await file_path.read_text() == original_content
+
+
+async def test_replace_sequential_dependent_edits_count(
+    str_replace_file_tool: StrReplaceFile, temp_work_dir: KaosPath
+):
+    """Edits apply in sequence, so a later edit can target text an earlier edit
+    introduced, and the reported replacement count reflects what actually ran."""
+    file_path = temp_work_dir / "test.txt"
+    await file_path.write_text("foo")
+
+    result = await str_replace_file_tool(
+        Params(
+            path=str(file_path),
+            edit=[
+                Edit(old="foo", new="bar"),
+                Edit(old="bar", new="baz"),
+            ],
+        )
+    )
+
+    assert not result.is_error
+    assert await file_path.read_text() == "baz"
+    assert "2 total replacement(s)" in result.message
+
+
+async def test_replace_noop_reports_unchanged_not_missing(
+    str_replace_file_tool: StrReplaceFile, temp_work_dir: KaosPath
+):
+    """An edit whose old string equals its new string matches but changes
+    nothing. The error must say the file was left unchanged, not the misleading
+    "old string was not found" (which would send the model re-reading the file
+    for a string that is actually present)."""
+    file_path = temp_work_dir / "test.txt"
+    original_content = "Hello world!"
+    await file_path.write_text(original_content)
+
+    result = await str_replace_file_tool(
+        Params(path=str(file_path), edit=Edit(old="world", new="world"))
+    )
+
+    assert result.is_error
+    assert "left the file unchanged" in result.message
+    assert "not found" not in result.message
+    assert await file_path.read_text() == original_content
+
+
+async def test_replace_rejects_empty_old_string(
+    str_replace_file_tool: StrReplaceFile, temp_work_dir: KaosPath
+):
+    """An empty old string must be rejected. `str.count('')` is never 0 and
+    `str.replace('', new)` would splice new between every character, so the
+    occurrence check cannot guard it — reject it explicitly instead."""
+    file_path = temp_work_dir / "test.txt"
+    original_content = "Hello world!"
+    await file_path.write_text(original_content)
+
+    result = await str_replace_file_tool(Params(path=str(file_path), edit=Edit(old="", new="X")))
+
+    assert result.is_error
+    assert "must not be empty" in result.message
+    assert await file_path.read_text() == original_content