fix(sandbox): make an empty old_str a no-op in str_replace on any file (#4256)

str_replace guards the replacement with `if old_str not in content`, which
cannot reject an empty old_str -- `"" in content` is always true. So an
empty old_str reached `str.replace("", new_str)`, which inserts new_str at
every character boundary, and the tool rewrote the file while still
returning "OK":

    old_str='', new_str='# H\n'          -> OK, file silently prepended
    old_str='', new_str='X', replace_all -> OK, 'XdXeXfX XmXaXiXnX(X)X:X\nX...'

The empty-file branch above it already handles this case (`if not content:
if not old_str: return "OK"`), and the existing test states the intent
directly: "An empty old_str is a no-op edit and remains a benign OK". That
contract just never held once the file had content.

The tool is registered by default (config.example.yaml) and its schema
declares old_str as a plain string with no minLength, so a model can emit
"" legitimately; read-before-write only compares a hash and lets it past.

Check old_str first so the no-op holds whatever the file contains. The
empty-file case folds into the same not-found branch, which keeps its
message and behaviour.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

backend/packages/harness/deerflow/sandbox/tools.py

@@ -2279,11 +2279,11 @@ def str_replace_tool(
             # Custom mount paths are resolved by LocalSandbox._resolve_path()
         with get_file_operation_lock(sandbox, path):
             content = sandbox.read_file(path)
-            if not content:
-                if not old_str:
-                    return "OK"
-                return f"Error: String to replace not found in file: {requested_path}"
-            if old_str not in content:
+            if not old_str:
+                # A no-op edit. str.replace("", new_str) would insert new_str at
+                # every character boundary, so this cannot fall through.
+                return "OK"
+            if not content or old_str not in content:
                 return f"Error: String to replace not found in file: {requested_path}"
             if replace_all:
                 content = content.replace(old_str, new_str)

backend/tests/test_str_replace_empty_file.py

@@ -1,10 +1,16 @@
-"""str_replace tool behaviour on empty files.
+"""str_replace tool behaviour with an empty file or an empty ``old_str``.
 
 An empty file used to short-circuit to ``"OK"`` regardless of ``old_str``,
 so a real substring replacement silently "succeeded" without changing anything
 and without telling the model the target was missing. The fix only returns
 ``"OK"`` on an empty file when ``old_str`` is itself empty (a no-op edit);
 a non-empty ``old_str`` now reports the string was not found.
+
+The mirror case is an empty ``old_str`` against a *non-empty* file. ``old_str
+not in content`` cannot reject ``""`` because ``"" in content`` is always true,
+so it reached ``str.replace("", new_str)``, which inserts at every character
+boundary and rewrote the file while still returning ``"OK"``. An empty
+``old_str`` is now a no-op whatever the file holds.
 """
 
 from pathlib import Path
@@ -28,27 +34,54 @@ def _local_runtime(tmp_path: Path) -> SimpleNamespace:
     )
 
 
-def _str_replace(tmp_path, monkeypatch, *, old_str: str, new_str: str = "x") -> str:
+def _str_replace(
+    tmp_path,
+    monkeypatch,
+    *,
+    old_str: str,
+    new_str: str = "x",
+    content: str = "",
+    replace_all: bool = False,
+) -> tuple[str, str]:
     runtime = _local_runtime(tmp_path)
-    (tmp_path / "outputs" / "empty.txt").write_text("", encoding="utf-8")
+    target = tmp_path / "outputs" / "empty.txt"
+    target.write_text(content, encoding="utf-8")
     monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", lambda runtime: LocalSandbox("t1"))
     monkeypatch.setattr("deerflow.sandbox.tools.ensure_thread_directories_exist", lambda runtime: None)
-    return str_replace_tool.func(
+    result = str_replace_tool.func(
         runtime=runtime,
         description="replace in empty file",
         path="/mnt/user-data/outputs/empty.txt",
         old_str=old_str,
         new_str=new_str,
+        replace_all=replace_all,
     )
+    return result, target.read_text(encoding="utf-8")
 
 
 def test_empty_file_with_non_empty_old_str_reports_not_found(tmp_path, monkeypatch) -> None:
-    result = _str_replace(tmp_path, monkeypatch, old_str="something")
+    result, _ = _str_replace(tmp_path, monkeypatch, old_str="something")
     assert result.startswith("Error: String to replace not found in file")
     assert "empty.txt" in result
 
 
 def test_empty_file_with_empty_old_str_returns_ok(tmp_path, monkeypatch) -> None:
     # An empty old_str is a no-op edit and remains a benign "OK" on an empty file.
-    result = _str_replace(tmp_path, monkeypatch, old_str="")
+    result, _ = _str_replace(tmp_path, monkeypatch, old_str="")
     assert result == "OK"
+
+
+def test_non_empty_file_with_empty_old_str_is_a_no_op(tmp_path, monkeypatch) -> None:
+    # The same no-op contract has to hold once the file has content: str.replace("")
+    # would otherwise insert new_str at every character boundary.
+    source = "def main():\n    return 1\n"
+    result, after = _str_replace(tmp_path, monkeypatch, old_str="", new_str="# header\n", content=source)
+    assert result == "OK"
+    assert after == source
+
+
+def test_non_empty_file_with_empty_old_str_and_replace_all_is_a_no_op(tmp_path, monkeypatch) -> None:
+    source = "def main():\n    return 1\n"
+    result, after = _str_replace(tmp_path, monkeypatch, old_str="", new_str="X", content=source, replace_all=True)
+    assert result == "OK"
+    assert after == source