diff --git a/openhands-agent-server/openhands/agent_server/hooks_router.py b/openhands-agent-server/openhands/agent_server/hooks_router.py index c893840f00..d576398d81 100644 --- a/openhands-agent-server/openhands/agent_server/hooks_router.py +++ b/openhands-agent-server/openhands/agent_server/hooks_router.py @@ -33,10 +33,10 @@ class HooksResponse(BaseModel): @hooks_router.post("", response_model=HooksResponse) def get_hooks(request: HooksRequest) -> HooksResponse: - """Load hooks from the workspace .openhands/hooks.json file. + """Load hooks from the workspace's hooks.json file. - This endpoint reads the hooks configuration from the project's - .openhands/hooks.json file if it exists. + This endpoint reads the hooks configuration from the first of the project's + `.agents/` or `.openhands/` directories that contains one. Args: request: HooksRequest containing the project directory path. diff --git a/openhands-agent-server/openhands/agent_server/hooks_service.py b/openhands-agent-server/openhands/agent_server/hooks_service.py index 12500e2256..5d9a460925 100644 --- a/openhands-agent-server/openhands/agent_server/hooks_service.py +++ b/openhands-agent-server/openhands/agent_server/hooks_service.py @@ -4,13 +4,11 @@ keeping the router clean and focused on HTTP concerns. Hook Sources: -- Project hooks: {workspace}/.openhands/hooks.json -- User hooks: ~/.openhands/hooks.json (future) +- Project hooks: {workspace}//hooks.json for each dir in HOOK_CONFIG_DIRS +- User hooks: ~//hooks.json (future) """ -from pathlib import Path - -from openhands.sdk.hooks import HookConfig +from openhands.sdk.hooks import HOOK_CONFIG_DIRS, HookConfig, find_hooks_file from openhands.sdk.logger import get_logger @@ -18,10 +16,10 @@ def load_hooks_from_workspace(project_dir: str | None = None) -> HookConfig | None: - """Load hooks from the workspace .openhands/hooks.json file. + """Load hooks from the workspace's hooks.json file. - This function reads the hooks configuration from the project's - .openhands/hooks.json file if it exists. + Looks for `hooks.json` under each of `HOOK_CONFIG_DIRS` inside the project + directory and reads the first one that exists. Args: project_dir: Workspace directory path for project hooks. @@ -33,10 +31,13 @@ def load_hooks_from_workspace(project_dir: str | None = None) -> HookConfig | No logger.debug("No project_dir provided, skipping hooks loading") return None - hooks_path = Path(project_dir) / ".openhands" / "hooks.json" + hooks_path = find_hooks_file(project_dir) - if not hooks_path.exists(): - logger.debug(f"No hooks.json found at {hooks_path}") + if hooks_path is None: + logger.debug( + f"No hooks.json found in {project_dir} " + f"(searched {', '.join(HOOK_CONFIG_DIRS)})" + ) return None try: diff --git a/openhands-sdk/openhands/sdk/hooks/__init__.py b/openhands-sdk/openhands/sdk/hooks/__init__.py index f76b8d38af..dfacd42412 100644 --- a/openhands-sdk/openhands/sdk/hooks/__init__.py +++ b/openhands-sdk/openhands/sdk/hooks/__init__.py @@ -6,11 +6,14 @@ """ from openhands.sdk.hooks.config import ( + HOOK_CONFIG_DIRS, + HOOK_CONFIG_FILENAME, HOOK_EVENT_FIELDS, HookConfig, HookDefinition, HookMatcher, HookType, + find_hooks_file, ) from openhands.sdk.hooks.conversation_hooks import ( HookEventProcessor, @@ -22,6 +25,8 @@ __all__ = [ + "HOOK_CONFIG_DIRS", + "HOOK_CONFIG_FILENAME", "HOOK_EVENT_FIELDS", "HookConfig", "HookDefinition", @@ -35,4 +40,5 @@ "HookDecision", "HookEventProcessor", "create_hook_callback", + "find_hooks_file", ] diff --git a/openhands-sdk/openhands/sdk/hooks/config.py b/openhands-sdk/openhands/sdk/hooks/config.py index 2398fe3e5d..60b3ef86d4 100644 --- a/openhands-sdk/openhands/sdk/hooks/config.py +++ b/openhands-sdk/openhands/sdk/hooks/config.py @@ -22,6 +22,38 @@ def _pascal_to_snake(name: str) -> str: return result +# Directories searched for ``hooks.json``, most preferred first. +# +# ``.agents`` is where the SDK already looks for skills, plugins and subagents +# (``SKILL_SEARCH_PATHS``, ``PLUGIN_SEARCH_PATHS``, ``.agents/agents``), so +# hooks belong there too. ``.openhands`` stays supported so existing projects +# keep working. +HOOK_CONFIG_DIRS: tuple[str, ...] = (".agents", ".openhands") + +HOOK_CONFIG_FILENAME = "hooks.json" + + +def find_hooks_file(base_dir: str | Path) -> Path | None: + """Return the first ``//hooks.json`` that exists. + + Directories are tried in ``HOOK_CONFIG_DIRS`` order, so a project carrying + more than one only has its preferred file loaded — configs are not merged + across directories. + + Args: + base_dir: Project or home directory to search under. + + Returns: + The path to the first existing hooks file, or None when none exist. + """ + base = Path(base_dir) + for directory in HOOK_CONFIG_DIRS: + candidate = base / directory / HOOK_CONFIG_FILENAME + if candidate.exists(): + return candidate + return None + + # Valid snake_case field names for hook events. # This is the single source of truth for hook event types. HOOK_EVENT_FIELDS: frozenset[str] = frozenset( @@ -159,8 +191,9 @@ def matches(self, tool_name: str | None) -> bool: class HookConfig(BaseModel): """Configuration for all hooks. - Hooks can be configured either by loading from `.openhands/hooks.json` or - by directly instantiating with typed fields: + Hooks can be configured either by loading a `hooks.json` (searched for in + `.agents/` then `.openhands/`) or by directly instantiating with typed + fields: # Direct instantiation with typed fields (recommended): config = HookConfig( @@ -173,7 +206,7 @@ class HookConfig(BaseModel): ) # Load from JSON file: - config = HookConfig.load(".openhands/hooks.json") + config = HookConfig.load(".agents/hooks.json") """ model_config = { @@ -273,24 +306,19 @@ def _normalize_hooks_input(cls, data: Any) -> Any: def load( cls, path: str | Path | None = None, working_dir: str | Path | None = None ) -> "HookConfig": - """Load config from path or search .openhands/hooks.json locations. + """Load config from path, or discover a project/user hooks.json. + + Discovery checks the project directory first and the user's home + directory second, trying ``HOOK_CONFIG_DIRS`` within each. Args: path: Explicit path to hooks.json file. If provided, working_dir is ignored. - working_dir: Project directory for discovering .openhands/hooks.json. + working_dir: Project directory to discover hooks.json under. Falls back to cwd if not provided. """ if path is None: - # Search for hooks.json in standard locations base_dir = Path(working_dir) if working_dir else Path.cwd() - search_paths = [ - base_dir / ".openhands" / "hooks.json", - Path.home() / ".openhands" / "hooks.json", - ] - for search_path in search_paths: - if search_path.exists(): - path = search_path - break + path = find_hooks_file(base_dir) or find_hooks_file(Path.home()) if path is None: return cls() diff --git a/tests/agent_server/test_hooks_service.py b/tests/agent_server/test_hooks_service.py index c92a9a14d8..15bdb68196 100644 --- a/tests/agent_server/test_hooks_service.py +++ b/tests/agent_server/test_hooks_service.py @@ -4,7 +4,10 @@ import tempfile from pathlib import Path +import pytest + from openhands.agent_server.hooks_service import load_hooks_from_workspace +from openhands.sdk.hooks import HOOK_CONFIG_DIRS class TestLoadHooksFromWorkspace: @@ -142,3 +145,49 @@ def test_load_hooks_pascal_case_format(self): assert not result.is_empty() assert len(result.stop) == 1 assert len(result.pre_tool_use) == 1 + + +class TestWorkspaceHooksDiscovery: + """The workspace loader must honour every supported config directory.""" + + @staticmethod + def write_hooks(base_dir: Path, directory: str, command: str) -> Path: + hooks_dir = base_dir / directory + hooks_dir.mkdir(parents=True, exist_ok=True) + hooks_file = hooks_dir / "hooks.json" + matcher = {"matcher": "*", "hooks": [{"command": command}]} + hooks_file.write_text(json.dumps({"hooks": {"stop": [matcher]}})) + return hooks_file + + @pytest.mark.parametrize("directory", HOOK_CONFIG_DIRS) + def test_loads_hooks_from_each_supported_directory( + self, tmp_path: Path, directory: str + ): + self.write_hooks(tmp_path, directory, f"{directory}.sh") + + result = load_hooks_from_workspace(project_dir=str(tmp_path)) + + assert result is not None + assert [hook.command for hook in result.stop[0].hooks] == [f"{directory}.sh"] + + def test_prefers_agents_over_the_legacy_openhands_directory(self, tmp_path: Path): + self.write_hooks(tmp_path, ".agents", "agents.sh") + self.write_hooks(tmp_path, ".openhands", "openhands.sh") + + result = load_hooks_from_workspace(project_dir=str(tmp_path)) + + assert result is not None + assert [hook.command for hook in result.stop[0].hooks] == ["agents.sh"] + + def test_ignores_a_home_directory_config( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ): + """Workspace loading is project-scoped; user hooks are not picked up.""" + project_dir = tmp_path / "project" + project_dir.mkdir() + home_dir = tmp_path / "home" + home_dir.mkdir() + self.write_hooks(home_dir, ".agents", "user.sh") + monkeypatch.setattr(Path, "home", classmethod(lambda cls: home_dir)) + + assert load_hooks_from_workspace(project_dir=str(project_dir)) is None diff --git a/tests/sdk/hooks/test_config_discovery.py b/tests/sdk/hooks/test_config_discovery.py new file mode 100644 index 0000000000..017b12e43a --- /dev/null +++ b/tests/sdk/hooks/test_config_discovery.py @@ -0,0 +1,115 @@ +"""Tests for discovering `hooks.json` across the supported config directories.""" + +import json +from pathlib import Path + +import pytest + +from openhands.sdk.hooks.config import HOOK_CONFIG_DIRS, HookConfig, find_hooks_file +from openhands.sdk.hooks.types import HookEventType + + +def write_hooks(base_dir: Path, directory: str, command: str) -> Path: + """Write a minimal hooks.json under `base_dir/directory`.""" + hooks_dir = base_dir / directory + hooks_dir.mkdir(parents=True, exist_ok=True) + hooks_file = hooks_dir / "hooks.json" + matcher = {"matcher": "*", "hooks": [{"command": command}]} + hooks_file.write_text(json.dumps({"hooks": {"PreToolUse": [matcher]}})) + return hooks_file + + +class TestFindHooksFile: + def test_returns_none_when_no_config_exists(self, tmp_path: Path): + assert find_hooks_file(tmp_path) is None + + @pytest.mark.parametrize("directory", HOOK_CONFIG_DIRS) + def test_finds_hooks_in_each_supported_directory( + self, tmp_path: Path, directory: str + ): + expected = write_hooks(tmp_path, directory, "run.sh") + assert find_hooks_file(tmp_path) == expected + + def test_prefers_agents_over_the_legacy_openhands_directory(self, tmp_path: Path): + expected = write_hooks(tmp_path, ".agents", "agents.sh") + write_hooks(tmp_path, ".openhands", "openhands.sh") + + assert find_hooks_file(tmp_path) == expected + + def test_accepts_a_string_base_dir(self, tmp_path: Path): + expected = write_hooks(tmp_path, ".agents", "run.sh") + assert find_hooks_file(str(tmp_path)) == expected + + +class TestLoadDiscovery: + @pytest.mark.parametrize("directory", HOOK_CONFIG_DIRS) + def test_load_discovers_each_supported_directory( + self, tmp_path: Path, directory: str + ): + write_hooks(tmp_path, directory, f"{directory}.sh") + + config = HookConfig.load(working_dir=tmp_path) + + hooks = config.get_hooks_for_event(HookEventType.PRE_TOOL_USE, "AnyTool") + assert [hook.command for hook in hooks] == [f"{directory}.sh"] + + def test_load_does_not_merge_across_directories(self, tmp_path: Path): + """Only the preferred directory is read — configs are not combined.""" + write_hooks(tmp_path, ".agents", "agents.sh") + write_hooks(tmp_path, ".openhands", "openhands.sh") + + config = HookConfig.load(working_dir=tmp_path) + + hooks = config.get_hooks_for_event(HookEventType.PRE_TOOL_USE, "AnyTool") + assert [hook.command for hook in hooks] == ["agents.sh"] + + def test_explicit_path_still_wins_over_discovery(self, tmp_path: Path): + write_hooks(tmp_path, ".agents", "discovered.sh") + explicit = write_hooks(tmp_path / "elsewhere", ".agents", "explicit.sh") + + config = HookConfig.load(path=explicit, working_dir=tmp_path) + + hooks = config.get_hooks_for_event(HookEventType.PRE_TOOL_USE, "AnyTool") + assert [hook.command for hook in hooks] == ["explicit.sh"] + + def test_falls_back_to_the_home_directory( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ): + project_dir = tmp_path / "project" + project_dir.mkdir() + home_dir = tmp_path / "home" + home_dir.mkdir() + write_hooks(home_dir, ".agents", "user.sh") + monkeypatch.setattr(Path, "home", classmethod(lambda cls: home_dir)) + + config = HookConfig.load(working_dir=project_dir) + + hooks = config.get_hooks_for_event(HookEventType.PRE_TOOL_USE, "AnyTool") + assert [hook.command for hook in hooks] == ["user.sh"] + + def test_project_config_wins_over_the_home_directory( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ): + project_dir = tmp_path / "project" + project_dir.mkdir() + home_dir = tmp_path / "home" + home_dir.mkdir() + # The project uses the least-preferred directory and the home dir uses + # the most-preferred one: project scope must still win. + write_hooks(project_dir, ".openhands", "project.sh") + write_hooks(home_dir, ".agents", "user.sh") + monkeypatch.setattr(Path, "home", classmethod(lambda cls: home_dir)) + + config = HookConfig.load(working_dir=project_dir) + + hooks = config.get_hooks_for_event(HookEventType.PRE_TOOL_USE, "AnyTool") + assert [hook.command for hook in hooks] == ["project.sh"] + + def test_returns_empty_config_when_nothing_is_found( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ): + home_dir = tmp_path / "home" + home_dir.mkdir() + monkeypatch.setattr(Path, "home", classmethod(lambda cls: home_dir)) + + assert HookConfig.load(working_dir=tmp_path).is_empty()