diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 5797a7b1..efec4794 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -51,7 +51,7 @@ uv sync # create .venv + install runtime + dev de .venv/bin/python -m pytest test/unit/cli/test_loader.py # one file .venv/bin/python -m pytest test/unit/cli/test_loader.py::TestLoadAdapterClass # one class .venv/bin/ruff check . -.venv/bin/ruff format . +.venv/bin/black . # formatter gate (see .pre-commit-config.yaml) .venv/bin/mypy agent_assembly # strict; type-check the package, not test/ .venv/bin/pre-commit run --all-files ``` diff --git a/agent_assembly/adapters/_shared/tool_governance.py b/agent_assembly/adapters/_shared/tool_governance.py index 54e9dae0..6e4f8392 100644 --- a/agent_assembly/adapters/_shared/tool_governance.py +++ b/agent_assembly/adapters/_shared/tool_governance.py @@ -218,7 +218,11 @@ async def run_governed_async_tool( ) status, reason = _normalize_decision(final_decision, enforce=enforce) - if status == "deny": + # Fail closed: only an explicit "allow" may proceed. A terminal "pending" + # (approval timed out or the resolver returned pending again) is a + # non-decision, not a grant — blocking it here stops it from falling through + # and running the tool, matching the LangChain handler. + if status != "allow": if is_pending_flow: raise _build_pending_rejected_error(tool_name, reason) raise _build_denied_error(tool_name, reason) diff --git a/agent_assembly/adapters/crewai/patch.py b/agent_assembly/adapters/crewai/patch.py index 4f635cb1..dfb2d351 100644 --- a/agent_assembly/adapters/crewai/patch.py +++ b/agent_assembly/adapters/crewai/patch.py @@ -392,7 +392,11 @@ def patched_run(self: Any, *args: Any, **kwargs: Any) -> Any: ) status, reason = _normalize_decision(final_decision, enforce=enforce) - if status == "deny": + # Fail closed: only an explicit "allow" may proceed. A terminal "pending" + # (approval timed out or the resolver returned pending again) is a + # non-decision, not a grant — blocking it here stops it from falling + # through and running the tool, matching the LangChain handler. + if status != "allow": if is_pending_flow: return _format_approval_rejected_message(reason) return _format_blocked_message(reason) diff --git a/test/unit/adapters/_shared/test_tool_governance.py b/test/unit/adapters/_shared/test_tool_governance.py new file mode 100644 index 00000000..4f59009a --- /dev/null +++ b/test/unit/adapters/_shared/test_tool_governance.py @@ -0,0 +1,49 @@ +"""Regression tests for the shared async tool-governance flow. + +These pin the fail-closed guarantee of ``run_governed_async_tool``: only an +explicit ``allow`` verdict may run the wrapped tool. A terminal ``pending`` +verdict (approval round-trip that resolves to ``pending`` again) is a +non-decision and must block, mirroring the LangChain handler (AAASM-4898). +""" + +from __future__ import annotations + +from typing import Any + +import pytest + +from agent_assembly.adapters._shared import tool_governance +from agent_assembly.exceptions import PolicyViolationError + + +class _TerminalPendingHandler: + def check_tool_start(self, **kwargs: Any) -> dict[str, str]: + del kwargs + return {"status": "pending", "reason": "awaiting approval"} + + def wait_for_tool_approval(self, **kwargs: Any) -> dict[str, str]: + del kwargs + return {"status": "pending", "reason": "still pending"} + + +@pytest.mark.asyncio +async def test_terminal_pending_blocks_and_never_invokes_original() -> None: + handler = _TerminalPendingHandler() + ran: list[bool] = [] + + def invoke_original() -> str: + ran.append(True) + return "tool-result" + + with pytest.raises(PolicyViolationError, match="rejected during approval"): + await tool_governance.run_governed_async_tool( + handler, + enforce=True, + tool_name="fake_tool", + tool_args={"text": "hi"}, + agent_id="agent-1", + run_id=None, + invoke_original=invoke_original, + ) + + assert ran == [] diff --git a/test/unit/adapters/crewai/test_patch.py b/test/unit/adapters/crewai/test_patch.py index b0dabf66..e103d98f 100644 --- a/test/unit/adapters/crewai/test_patch.py +++ b/test/unit/adapters/crewai/test_patch.py @@ -391,6 +391,39 @@ def wait_for_tool_approval(self, **kwargs: object) -> dict[str, str]: assert "approval timeout" in result +def test_terminal_pending_blocks_tool_and_does_not_run( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """AAASM-4898: a still-"pending" verdict after the approval round-trip must + block, not fall through and run the tool. "pending" is a terminal + non-decision here, so it fails closed like the LangChain handler.""" + FakeBaseTool, _ = _install_fake_crewai_modules(monkeypatch) + recorded_results: list[object] = [] + + class TerminalPendingInterceptor: + def check_tool_start(self, **kwargs: object) -> dict[str, str]: + del kwargs + return {"status": "pending", "reason": "awaiting approval"} + + def wait_for_tool_approval(self, **kwargs: object) -> dict[str, str]: + del kwargs + return {"status": "pending", "reason": "still pending"} + + def record_result(self, **kwargs: object) -> None: + recorded_results.append(kwargs.get("result")) + + patcher = crewai_patch.CrewAIPatch(TerminalPendingInterceptor()) + assert patcher.apply() is True + + tool = FakeBaseTool() + result = tool.run(param="value") + + assert isinstance(result, str) + assert result.startswith("[APPROVAL REJECTED]") + # The original tool never executed, so its result was never recorded. + assert recorded_results == [] + + def test_task_start_and_complete_events_are_recorded( monkeypatch: pytest.MonkeyPatch, ) -> None: diff --git a/test/unit/core/test_spawn_context.py b/test/unit/core/test_spawn_context.py index 99246552..203296f5 100644 --- a/test/unit/core/test_spawn_context.py +++ b/test/unit/core/test_spawn_context.py @@ -4,9 +4,24 @@ import pytest +from agent_assembly.core import assembly as core_assembly from agent_assembly.core.spawn import _SPAWN_CTX, SpawnContext, spawn_context_scope +@pytest.fixture(autouse=True) +def force_pure_python_registration(monkeypatch: pytest.MonkeyPatch) -> None: + """Keep init_assembly() hermetic regardless of whether the native ``_core`` + extension is built (AAASM-4898). + + These spawn-context tests stub GatewayClient / adapter / network layers but + not the native gRPC registration path (``connect_runtime_client`` / + ``register_agent``), so a stray native ``.so`` made init dial a real gateway. + Forcing the pure-Python path (the CI default) keeps them deterministic in + both build modes. + """ + monkeypatch.setattr(core_assembly, "_native_core_available", lambda: False) + + class TestSpawnContext: def test_dataclass_fields(self) -> None: ctx = SpawnContext(parent_agent_id="parent-1", depth=2, spawned_by_tool="tool-x") diff --git a/test/unit/test_assembly.py b/test/unit/test_assembly.py index 48f4b904..637df845 100644 --- a/test/unit/test_assembly.py +++ b/test/unit/test_assembly.py @@ -33,6 +33,21 @@ def unregister_hooks(self) -> None: self._registered = False +@pytest.fixture(autouse=True) +def force_pure_python_registration(monkeypatch: pytest.MonkeyPatch) -> None: + """Keep init_assembly() hermetic regardless of whether the native ``_core`` + extension is built (AAASM-4898). + + When the native extension is present, ``_native_core_available()`` returns + True and init dials the gateway over gRPC (``connect_runtime_client`` / + ``register_agent``). These plumbing tests stub the adapter and network layers + but not that registration path, so a stray native ``.so`` made them hit a real + gateway and fail. Forcing the pure-Python path (the CI default) makes every + init_assembly() call here deterministic in both build modes. + """ + monkeypatch.setattr(core_assembly, "_native_core_available", lambda: False) + + @pytest.fixture(autouse=True) def cleanup_active_context() -> None: active_context = core_assembly._ACTIVE_CONTEXT