diff --git a/tests/tools/test_search_error_guard.py b/tests/tools/test_search_error_guard.py index e045c8c3d524..8b827a6d4936 100644 --- a/tests/tools/test_search_error_guard.py +++ b/tests/tools/test_search_error_guard.py @@ -109,6 +109,26 @@ def test_truncation_no_false_error(self, method, tmp_path): assert res.error is None, f"truncated success wrongly errored: {res.error!r}" assert len(res.matches) == 5 + def test_files_only_preserves_spaced_paths_end_to_end(self, method, tmp_path): + root = tmp_path / "workspace" + root.mkdir() + spaced = root / "Meeting Notes.txt" + nested_dir = root / "project files" + nested_dir.mkdir() + nested = nested_dir / "release notes.md" + spaced.write_text("needle\n") + nested.write_text("needle\n") + + res = _search( + _ops(tmp_path), method, "needle", "workspace", output_mode="files_only" + ) + + assert res.error is None + assert set(res.files) == { + "workspace/Meeting Notes.txt", + "workspace/project files/release notes.md", + } + def test_files_only_excludes_diagnostics(self, method, partial_error_tree): # files_only mode must not list a diagnostic line as a fake file path. res = _search(_ops(partial_error_tree), method, "needle", @@ -216,3 +236,41 @@ def test_context_lines_and_separator_are_payload(self): assert diagnostics == "" assert "--" in payload assert "a.py-6-after" in payload + + def test_files_only_preserves_spaced_paths(self): + # Regression: files_only output is one bare path per line and may + # contain spaces. The content-mode shape regex forbids whitespace, so + # without mode awareness these valid matches are silently dropped. + out = "normal.py\nMy Document.md\nsub/Meeting Notes.txt\nsub/clean.txt\n" + diagnostics, payload = _split_tool_diagnostics(out, output_mode="files_only") + assert diagnostics == "" + lines = payload.split("\n") + assert "My Document.md" in lines + assert "sub/Meeting Notes.txt" in lines + assert len(lines) == 4 + + def test_files_only_still_drops_regex_parse_error(self): + # The hard-error block (indented pattern + caret, trailing "error: ") + # must still be classified as diagnostics in files_only mode so a real + # failure is surfaced rather than returned as fake file paths. + out = "rg: regex parse error:\n (?:[)\n ^\nerror: unclosed character class\n" + diagnostics, payload = _split_tool_diagnostics(out, output_mode="files_only") + assert payload.strip() == "" + assert "regex parse error" in diagnostics + + def test_files_only_keeps_path_named_like_error_summary(self): + # A file literally named "error: notes.md" is a valid match, not a + # diagnostic: outside an rg parse-error block, an "error:"-prefixed + # line is a real path and must be preserved. + out = "src/a.py\nerror: notes.md\nsrc/b.py\n" + diagnostics, payload = _split_tool_diagnostics(out, output_mode="files_only") + assert diagnostics == "" + assert "error: notes.md" in payload.split("\n") + + def test_files_only_keeps_indented_path_outside_error_block(self): + # A path with leading whitespace is still a real match when no + # parse-error block is active; it must not be mistaken for a caret line. + out = "src/a.py\n weird name.txt\nsrc/b.py\n" + diagnostics, payload = _split_tool_diagnostics(out, output_mode="files_only") + assert diagnostics == "" + assert " weird name.txt" in payload.split("\n") diff --git a/tools/file_operations.py b/tools/file_operations.py index 994723cf583b..6591bdd8605a 100644 --- a/tools/file_operations.py +++ b/tools/file_operations.py @@ -345,7 +345,7 @@ def _search_stdout_and_limit(result: ExecuteResult) -> tuple[str, Optional[str]] return result.stdout, None -def _split_tool_diagnostics(output: str) -> tuple[str, str]: +def _split_tool_diagnostics(output: str, output_mode: str = "content") -> tuple[str, str]: """Separate rg/grep diagnostic lines from real match output. ``_exec`` runs commands with ``stderr=subprocess.STDOUT``, so error and @@ -367,6 +367,7 @@ def _split_tool_diagnostics(output: str) -> tuple[str, str]: """ diagnostics: list[str] = [] payload: list[str] = [] + in_rg_parse_error = False for line in output.split('\n'): if not line.strip(): continue @@ -379,6 +380,28 @@ def _split_tool_diagnostics(output: str) -> tuple[str, str]: stripped = line.lstrip() if stripped.startswith("rg: ") or stripped.startswith("grep: "): diagnostics.append(line) + # rg's "regex parse error:" header is followed by several + # continuation lines with no tool prefix (an indented echo of the + # offending pattern + a caret, then a bare "error: "). + # Track that we're inside such a block so those continuations are + # classified as diagnostics — but only there. + in_rg_parse_error = "regex parse error" in stripped + continue + if output_mode == "files_only": + # files_only output is one bare path per line and may legitimately + # contain spaces ("My Document.md") or even start with "error:". + # The content-mode shape regex forbids whitespace, so using it here + # would misclassify valid paths as diagnostics and silently drop + # them. The ONLY non-path lines in this mode are the continuation + # of an rg regex-parse-error block, and a parse error aborts the + # search (no matches are interleaved with it). So strip a line only + # when it is a continuation INSIDE an active parse-error block; + # every other line is a real path and is preserved verbatim. + if in_rg_parse_error and (line[:1].isspace() or stripped.startswith("error: ")): + diagnostics.append(line) + else: + in_rg_parse_error = False + payload.append(line) continue # Otherwise classify by output shape. rg's regex-parse-error block # also emits an indented caret line and a trailing "error: ..." line @@ -2306,7 +2329,7 @@ def _search_with_rg(self, pattern: str, path: str, file_glob: Optional[str], # diagnostic lines ("rg: : ", "rg: regex parse error:") # are interleaved with match output. Split them out: diagnostics must # not be parsed as matches, and on a hard error they ARE the message. - diagnostics, payload = _split_tool_diagnostics(stdout) + diagnostics, payload = _split_tool_diagnostics(stdout, output_mode) # rg exit codes: 0=matches found, 1=no matches, 2=error. rg returns 2 # even on partial errors (e.g. one unreadable file in a tree that @@ -2434,7 +2457,7 @@ def _search_with_grep(self, pattern: str, path: str, file_glob: Optional[str], # ("grep: : ") are interleaved with matches. Split them # out so they're never parsed as matches and so a hard error has a # clean message. - diagnostics, payload = _split_tool_diagnostics(stdout) + diagnostics, payload = _split_tool_diagnostics(stdout, output_mode) # grep exit codes: 0=matches found, 1=no matches, 2=error. grep # returns 2 on partial errors (e.g. an unreadable file) even when