mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix(agent): catch PermissionError in subdirectory hint discovery
Wrap is_dir() in _is_valid_subdir() and is_file() in _load_hints_for_directory() with OSError handlers so that inaccessible directories (e.g. /root from a non-root Daytona host user) are silently skipped instead of crashing the agent. The existing PermissionError PRs for prompt_builder.py (#6247, #6321, #6355) do not cover subdirectory_hints.py, which was identified as a separate crash path in the #6214 comments. Ref: #6214
This commit is contained in:
parent
161c2c4da4
commit
3c8ec7037c
2 changed files with 51 additions and 2 deletions
|
|
@ -159,7 +159,10 @@ class SubdirectoryHintTracker:
|
|||
|
||||
def _is_valid_subdir(self, path: Path) -> bool:
|
||||
"""Check if path is a valid directory to scan for hints."""
|
||||
if not path.is_dir():
|
||||
try:
|
||||
if not path.is_dir():
|
||||
return False
|
||||
except OSError:
|
||||
return False
|
||||
if path in self._loaded_dirs:
|
||||
return False
|
||||
|
|
@ -172,7 +175,10 @@ class SubdirectoryHintTracker:
|
|||
found_hints = []
|
||||
for filename in _HINT_FILENAMES:
|
||||
hint_path = directory / filename
|
||||
if not hint_path.is_file():
|
||||
try:
|
||||
if not hint_path.is_file():
|
||||
continue
|
||||
except OSError:
|
||||
continue
|
||||
try:
|
||||
content = hint_path.read_text(encoding="utf-8").strip()
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@
|
|||
import os
|
||||
import pytest
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
from agent.subdirectory_hints import SubdirectoryHintTracker
|
||||
|
||||
|
|
@ -189,3 +190,45 @@ class TestSubdirectoryHintTracker:
|
|||
"terminal", {"command": "curl https://example.com/frontend/api"}
|
||||
)
|
||||
assert result is None
|
||||
|
||||
|
||||
class TestPermissionErrorHandling:
|
||||
"""Regression tests for PermissionError in filesystem checks (ref #6214)."""
|
||||
|
||||
def test_is_valid_subdir_permission_error(self, tmp_path):
|
||||
"""_is_valid_subdir should return False when is_dir() raises PermissionError."""
|
||||
tracker = SubdirectoryHintTracker(working_dir=str(tmp_path))
|
||||
restricted = tmp_path / "restricted"
|
||||
restricted.mkdir()
|
||||
with patch.object(Path, "is_dir", side_effect=PermissionError("Permission denied")):
|
||||
assert tracker._is_valid_subdir(restricted) is False
|
||||
|
||||
def test_load_hints_permission_error_on_is_file(self, tmp_path):
|
||||
"""_load_hints_for_directory should skip files when is_file() raises PermissionError."""
|
||||
tracker = SubdirectoryHintTracker(working_dir=str(tmp_path))
|
||||
restricted = tmp_path / "restricted"
|
||||
restricted.mkdir()
|
||||
original_is_file = Path.is_file
|
||||
def patched_is_file(self):
|
||||
if "restricted" in str(self):
|
||||
raise PermissionError("Permission denied")
|
||||
return original_is_file(self)
|
||||
with patch.object(Path, "is_file", patched_is_file):
|
||||
result = tracker._load_hints_for_directory(restricted)
|
||||
assert result is None
|
||||
|
||||
def test_check_tool_call_survives_inaccessible_path(self, project):
|
||||
"""Full check_tool_call should not crash when a path is inaccessible."""
|
||||
tracker = SubdirectoryHintTracker(working_dir=str(project))
|
||||
original_is_dir = Path.is_dir
|
||||
def patched_is_dir(self):
|
||||
if "backend" in str(self) and "src" not in str(self):
|
||||
raise PermissionError("Permission denied")
|
||||
return original_is_dir(self)
|
||||
with patch.object(Path, "is_dir", patched_is_dir):
|
||||
# Should not raise — gracefully skip the inaccessible directory
|
||||
result = tracker.check_tool_call(
|
||||
"read_file", {"path": str(project / "backend" / "src" / "main.py")}
|
||||
)
|
||||
# Result may be None (backend skipped) — the key point is no crash
|
||||
assert result is None or isinstance(result, str)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue