From 3fb8938cd35c6cf24739d667cf898c918360c45d Mon Sep 17 00:00:00 2001 From: teknium1 Date: Sun, 8 Mar 2026 16:47:20 -0700 Subject: [PATCH] fix: search_files now reports error for non-existent paths instead of silent empty results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, search_files would silently return 0 results when the search path didn't exist (e.g., /root/.hermes/... when HOME is /home/user). The path was passed to rg/grep/find which would fail silently, and the empty stdout was parsed as 'no matches found'. Changes: - Add path existence check at the top of search() using test -e. Returns SearchResult with a clear error message when path doesn't exist. - Add exit code 2 checks in _search_with_rg() and _search_with_grep() as secondary safety net for other error types (bad regex, permissions). - Add 4 new tests covering: nonexistent path (content mode), nonexistent path (files mode), existing path proceeds normally, rg error exit code. Tests: 37 → 41 in test_file_operations.py, full suite 2330 passed. --- tests/tools/test_file_operations.py | 64 +++++++++++++++++++++++++++++ tools/file_operations.py | 18 ++++++++ 2 files changed, 82 insertions(+) diff --git a/tests/tools/test_file_operations.py b/tests/tools/test_file_operations.py index b427826e5..0db3fb43b 100644 --- a/tests/tools/test_file_operations.py +++ b/tests/tools/test_file_operations.py @@ -259,6 +259,70 @@ class TestShellFileOpsHelpers: assert ops.cwd == "/" +class TestSearchPathValidation: + """Test that search() returns an error for non-existent paths.""" + + def test_search_nonexistent_path_returns_error(self, mock_env): + """search() should return an error when the path doesn't exist.""" + def side_effect(command, **kwargs): + if "test -e" in command: + return {"output": "not_found", "returncode": 1} + if "command -v" in command: + return {"output": "yes", "returncode": 0} + return {"output": "", "returncode": 0} + mock_env.execute.side_effect = side_effect + ops = ShellFileOperations(mock_env) + result = ops.search("pattern", path="/nonexistent/path") + assert result.error is not None + assert "not found" in result.error.lower() or "Path not found" in result.error + + def test_search_nonexistent_path_files_mode(self, mock_env): + """search(target='files') should also return error for bad paths.""" + def side_effect(command, **kwargs): + if "test -e" in command: + return {"output": "not_found", "returncode": 1} + if "command -v" in command: + return {"output": "yes", "returncode": 0} + return {"output": "", "returncode": 0} + mock_env.execute.side_effect = side_effect + ops = ShellFileOperations(mock_env) + result = ops.search("*.py", path="/nonexistent/path", target="files") + assert result.error is not None + assert "not found" in result.error.lower() or "Path not found" in result.error + + def test_search_existing_path_proceeds(self, mock_env): + """search() should proceed normally when the path exists.""" + def side_effect(command, **kwargs): + if "test -e" in command: + return {"output": "exists", "returncode": 0} + if "command -v" in command: + return {"output": "yes", "returncode": 0} + # rg returns exit 1 (no matches) with empty output + return {"output": "", "returncode": 1} + mock_env.execute.side_effect = side_effect + ops = ShellFileOperations(mock_env) + result = ops.search("pattern", path="/existing/path") + assert result.error is None + assert result.total_count == 0 # No matches but no error + + def test_search_rg_error_exit_code(self, mock_env): + """search() should report error when rg returns exit code 2.""" + call_count = {"n": 0} + def side_effect(command, **kwargs): + call_count["n"] += 1 + if "test -e" in command: + return {"output": "exists", "returncode": 0} + if "command -v" in command: + return {"output": "yes", "returncode": 0} + # rg returns exit 2 (error) with empty output + return {"output": "", "returncode": 2} + mock_env.execute.side_effect = side_effect + ops = ShellFileOperations(mock_env) + result = ops.search("pattern", path="/some/path") + assert result.error is not None + assert "search failed" in result.error.lower() or "Search error" in result.error + + class TestShellFileOpsWriteDenied: def test_write_file_denied_path(self, file_ops): result = file_ops.write_file("~/.ssh/authorized_keys", "evil key") diff --git a/tools/file_operations.py b/tools/file_operations.py index 182d35f5f..3f72c5fdb 100644 --- a/tools/file_operations.py +++ b/tools/file_operations.py @@ -819,6 +819,14 @@ class ShellFileOperations(FileOperations): # Expand ~ and other shell paths path = self._expand_path(path) + # Validate that the path exists before searching + check = self._exec(f"test -e {self._escape_shell_arg(path)} && echo exists || echo not_found") + if "not_found" in check.stdout: + return SearchResult( + error=f"Path not found: {path}. Verify the path exists (use 'terminal' to check).", + total_count=0 + ) + if target == "files": return self._search_files(pattern, path, limit, offset) else: @@ -919,6 +927,11 @@ class ShellFileOperations(FileOperations): cmd = " ".join(cmd_parts) result = self._exec(cmd, timeout=60) + # rg exit codes: 0=matches found, 1=no matches, 2=error + if result.exit_code == 2 and not result.stdout.strip(): + error_msg = result.stderr.strip() if hasattr(result, 'stderr') and result.stderr else "Search error" + return SearchResult(error=f"Search failed: {error_msg}", total_count=0) + # Parse results based on output mode if output_mode == "files_only": all_files = [f for f in result.stdout.strip().split('\n') if f] @@ -1013,6 +1026,11 @@ class ShellFileOperations(FileOperations): cmd = " ".join(cmd_parts) result = self._exec(cmd, timeout=60) + # grep exit codes: 0=matches found, 1=no matches, 2=error + if result.exit_code == 2 and not result.stdout.strip(): + error_msg = result.stderr.strip() if hasattr(result, 'stderr') and result.stderr else "Search error" + return SearchResult(error=f"Search failed: {error_msg}", total_count=0) + if output_mode == "files_only": all_files = [f for f in result.stdout.strip().split('\n') if f] total = len(all_files)