fix: search_files now reports error for non-existent paths instead of silent empty results
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.
This commit is contained in:
@@ -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")
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user