diff --git a/EvoScientist/EvoScientist.py b/EvoScientist/EvoScientist.py index 176b5f3..75198f2 100644 --- a/EvoScientist/EvoScientist.py +++ b/EvoScientist/EvoScientist.py @@ -1043,10 +1043,12 @@ def _get_default_backend( return _get_legacy_backend( guard_dangerous=guard_dangerous, refuse_delete=refuse_delete ) - from .workspace_scope import create_workspace_backend_factory + from .workspace_scope import create_deferred_scoped_backend cfg = _ensure_config() - return create_workspace_backend_factory( + # deepagents 0.7 removed backend factories, so hand the middleware an + # instance that resolves this run's scope from the runnable config. + return create_deferred_scoped_backend( _get_legacy_backend, dangerous=cfg.dangerous_mode, allow_unscoped_legacy=False, @@ -1365,20 +1367,18 @@ def _get_default_agent(): else _get_default_middleware() ) - # HITL on main agent only (mirrors create_cli_agent). Use middleware, - # not interrupt_on= kwarg — the kwarg propagates to every subagent and - # breaks parallel execute calls (multi-pending-interrupt LangGraph - # error). See PR #202. + # HITL on main agent only (mirrors create_cli_agent). Arm it through the + # review middleware and NEVER through `create_deep_agent(interrupt_on=)`: + # that kwarg makes deepagents append its own plain + # HumanInTheLoopMiddleware, and the second, auto-blind layer interrupts + # even on a run the gateway verified as auto — silently disabling + # automatic approval. It also propagates to every subagent and breaks + # parallel execute calls (multi-pending-interrupt LangGraph error). + # See PR #202. `HITL_INTERRUPT_ON` is the single source of the tool set. from .middleware import DynamicReviewMiddleware mw.append( - DynamicReviewMiddleware( - interrupt_on={ - "execute": True, - "run_in_background": True, - "schedule_task": True, - } - ) + DynamicReviewMiddleware(interrupt_on=dict(HITL_INTERRUPT_ON)) ) if web_full: @@ -1412,9 +1412,10 @@ def _get_default_agent(): ) kwargs = _apply_budgeted_skill_context(kwargs, be) + # No `interrupt_on=` here: HITL is armed above, as a single layer, by the + # auto-aware review middleware (see the comment there). _EvoScientist_agent = create_deep_agent( **kwargs, - interrupt_on=_build_hitl_interrupt_on(auto_approve=cfg.auto_approve), ).with_config({"recursion_limit": cfg.recursion_limit}) return _EvoScientist_agent @@ -1694,25 +1695,20 @@ def create_cli_agent( if main_agent_outer_middlewares: mw = [*main_agent_outer_middlewares, *mw] - # HITL on main agent only — passing `interrupt_on=` to create_deep_agent - # would propagate it to every subagent, breaking parallel execute calls - # (multi-pending-interrupt LangGraph error). + # HITL on main agent only. Arm it as a middleware and NEVER through + # `create_deep_agent(interrupt_on=)`: that kwarg makes deepagents append its + # own plain HumanInTheLoopMiddleware, and that second, auto-blind layer + # interrupts even on a run the gateway verified as auto — silently disabling + # automatic approval. It also propagates to every subagent, breaking parallel + # execute calls (multi-pending-interrupt LangGraph error). Web runs defer to + # the gateway's per-thread review mode, so they always arm the auto-aware + # middleware; `HITL_INTERRUPT_ON` is the single source of the tool set. if is_web: from .middleware.dynamic_review import DynamicReviewMiddleware - mw.append(DynamicReviewMiddleware(interrupt_on={ - "execute": True, "run_in_background": True, "schedule_task": True, - })) - elif not cfg.auto_approve: - mw.append( - HumanInTheLoopMiddleware( - interrupt_on={ - "execute": True, - "run_in_background": True, - "schedule_task": True, - } - ) - ) + mw.append(DynamicReviewMiddleware(interrupt_on=dict(HITL_INTERRUPT_ON))) + elif _build_hitl_interrupt_on(auto_approve=cfg.auto_approve) is not None: + mw.append(HumanInTheLoopMiddleware(interrupt_on=dict(HITL_INTERRUPT_ON))) # Re-load MCP tools from current config (picks up /mcp add changes) kwargs = load_mcp_and_build_kwargs( @@ -1757,8 +1753,10 @@ def create_cli_agent( from deepagents import create_deep_agent as original_factory create_deep_agent.__kwdefaults__ = original_factory.__kwdefaults__ + # No `interrupt_on=` here: HITL is armed above, as a single middleware layer + # (see the comment there). Passing the kwarg would add a second, auto-blind + # layer that defeats verified auto approval. return create_deep_agent( **kwargs, checkpointer=checkpointer, - interrupt_on=_build_hitl_interrupt_on(auto_approve=cfg.auto_approve), ).with_config({"recursion_limit": cfg.recursion_limit}) diff --git a/EvoScientist/workspace_scope.py b/EvoScientist/workspace_scope.py index 37bae8e..e423395 100644 --- a/EvoScientist/workspace_scope.py +++ b/EvoScientist/workspace_scope.py @@ -354,21 +354,48 @@ class DeferredScopedBackend(SandboxBackendProtocol): def __init__( self, - config: _RuntimeScopeConfig, + config: _RuntimeScopeConfig | None, *, dangerous: bool, + legacy_backend: Callable[[], Any] | None = None, + allow_unscoped_legacy: bool = False, ) -> None: self._config = config self._dangerous = dangerous + self._legacy_backend = legacy_backend + self._allow_unscoped_legacy = allow_unscoped_legacy self._backend: Any | None = None self._backend_key: tuple[str, str, str, int] | None = None self._lock = threading.RLock() + def _scope_config(self) -> _RuntimeScopeConfig | None: + """Return this proxy's scope identity, resolving it lazily if needed. + + deepagents 0.7 removed backend factories: the middleware only accepts + an initialized ``BackendProtocol``, so a deploy-scoped proxy is built + before any run exists and must read its scope from the active runnable + config on each operation. ``asyncio.to_thread`` copies the contextvars + context, so the runnable config remains readable from the worker + threads the inherited async methods dispatch into. + """ + + if self._config is not None: + return self._config + return _runtime_scope_config(None, kind="filesystem backend") + @property def id(self) -> str: # This is queried while composing the model request; do not initialize # the real backend or touch the Registry here. - return f"scope-{self._config.scope_id[:8]}-{self._config.owner_id[:8]}" + config = self._config + if config is None: + try: + config = _runtime_scope_config(None, kind="filesystem backend") + except ScopeAccessError: + config = None + if config is None: + return "scope-deferred" + return f"scope-{config.scope_id[:8]}-{config.owner_id[:8]}" def _delegate(self) -> Any: """Validate the current scope and return a concrete backend. @@ -377,10 +404,18 @@ class DeferredScopedBackend(SandboxBackendProtocol): keep using a backend constructed before the lifecycle transition. """ - # Async backend methods run this code in a worker thread. LangGraph's - # RunnableConfig context variable is not available there, so validate - # the immutable scope parsed by the factory on the graph thread. - context = _resolve_scope_context(self._config) + # ``asyncio.to_thread`` copies the contextvars context, so the active + # runnable config is still readable here even though the inherited + # async methods dispatch this work to a worker thread. Registry + # validation therefore stays off the Agent event loop. + config = self._scope_config() + if config is None: + if not self._allow_unscoped_legacy or self._legacy_backend is None: + raise ScopeAccessError( + "deployed graph runs require a workspace scope" + ) + return self._legacy_backend() + context = _resolve_scope_context(config) if context is None: raise ScopeAccessError("scoped backend lost its workspace scope") if (is_required() or os.getenv("EVOSCIENTIST_DEPLOY_MODE", "").lower() == "full") and self._dangerous: @@ -478,6 +513,37 @@ def create_workspace_backend_factory( return factory +def create_deferred_scoped_backend( + legacy_backend: Callable[[], Any], + *, + dangerous: bool = False, + allow_unscoped_legacy: bool = False, +) -> Any: + """Return a deploy-scoped backend INSTANCE for deepagents >= 0.7. + + deepagents 0.7 removed backend factories: ``FilesystemMiddleware`` now + rejects any ``backend`` that is callable without being a + ``BackendProtocol`` instance. Return an instance instead and let it read + the run's workspace scope from the active runnable config on every + operation. Unscoped runs stay fail-closed unless the caller explicitly + allows the legacy backend. + """ + + if ( + is_required() + or os.getenv("EVOSCIENTIST_DEPLOY_MODE", "").lower() == "full" + ) and dangerous: + raise ScopeAccessError( + "dangerous_mode is incompatible with required isolation" + ) + return DeferredScopedBackend( + None, + dangerous=dangerous, + legacy_backend=legacy_backend, + allow_unscoped_legacy=allow_unscoped_legacy, + ) + + def workspace_metadata(record: ScopeRecord) -> dict[str, str | int]: """Metadata mirrored onto the LangGraph primary thread by trusted callers.""" diff --git a/tests/test_agent_factory_extensions.py b/tests/test_agent_factory_extensions.py index eebd5d2..374355f 100644 --- a/tests/test_agent_factory_extensions.py +++ b/tests/test_agent_factory_extensions.py @@ -9,9 +9,10 @@ def test_create_cli_agent_accepts_host_backend_and_memory_options( chat_model = object() class _CompositeBackend: - def __init__(self, *, default, routes): + def __init__(self, *, default, routes, artifacts_root=None): calls["default_backend"] = default calls["routes"] = routes + calls["artifacts_root"] = artifacts_root class _MemoryBackend: def __init__(self, **kwargs): diff --git a/tests/test_hitl.py b/tests/test_hitl.py index 3a38745..7fb9307 100644 --- a/tests/test_hitl.py +++ b/tests/test_hitl.py @@ -742,27 +742,57 @@ class TestInterruptOnWiring: assert cfg.auto_approve is True assert _build_hitl_interrupt_on(auto_approve=cfg.auto_approve) is None - def test_hitl_interrupt_on_reaches_create_deep_agent(self): - """The kwarg must actually reach ``create_deep_agent`` — not just the - pure helper — so a future edit that drops it or re-adds a bare - ``HumanInTheLoopMiddleware`` append gets caught.""" - import EvoScientist.EvoScientist as es_mod - from EvoScientist.EvoScientist import _build_hitl_interrupt_on + def test_hitl_is_armed_by_the_review_middleware_only(self): + """HITL must be exactly one layer, and it must be the auto-aware one. - captured = [] + ``create_deep_agent(interrupt_on=...)`` makes deepagents append its own + plain ``HumanInTheLoopMiddleware``. That second, auto-blind layer + interrupts even on a run the gateway verified as auto — which silently + disables automatic approval, because ``DynamicReviewMiddleware`` can only + decline to interrupt for itself. So the tool set is armed through the + middleware and the kwarg must stay out. + """ + import EvoScientist.EvoScientist as es_mod + from EvoScientist.EvoScientist import HITL_INTERRUPT_ON + from EvoScientist.middleware import DynamicReviewMiddleware + from langchain.agents.middleware import HumanInTheLoopMiddleware + + captured_kwargs = [] + captured_middleware = [] + + class _WebProfile(MagicMock): + """A Web execution profile: fixed name, every other field a mock.""" + + name = "web_v3" + + web_profile = _WebProfile() def fake_create_deep_agent(**kwargs): - captured.append(kwargs.get("interrupt_on", "MISSING")) + captured_kwargs.append(kwargs) agent = MagicMock() agent.with_config.return_value = agent return agent - for auto_approve in (False, True): + def fake_build_kwargs(_backend, middleware, **_kwargs): + captured_middleware.append(list(middleware)) + return {"name": "x"} + + def build(auto_approve, execution_profile=None): cfg = MagicMock() cfg.auto_approve = auto_approve cfg.dangerous_mode = False cfg.sandbox_execute_timeout = 300 cfg.recursion_limit = 100 + # A Web run must supply the host-provided backend and checkpointer. + web_only = ( + { + "memory_dir": "/tmp/test-interrupt-on-wiring-memory", + "workspace_backend": MagicMock(), + "checkpointer": MagicMock(), + } + if execution_profile is not None + else {} + ) with patch( "deepagents.create_deep_agent", side_effect=fake_create_deep_agent @@ -774,25 +804,47 @@ class TestInterruptOnWiring: with patch.object( es_mod, "load_mcp_and_build_kwargs", - return_value={"name": "x"}, + side_effect=fake_build_kwargs, ): es_mod.create_cli_agent( workspace_dir="/tmp/test-interrupt-on-wiring", config=cfg, chat_model=MagicMock(), + execution_profile=execution_profile, + **web_only, ) - assert captured == [ - _build_hitl_interrupt_on(auto_approve=False), - _build_hitl_interrupt_on(auto_approve=True), - ] - assert captured[0] == { - "execute": True, - "run_in_background": True, - "schedule_task": True, - "delete": True, - } - assert captured[1] is None + build(auto_approve=False) # attended CLI + build(auto_approve=False, execution_profile=web_profile) # Web run + build(auto_approve=True) # auto: nothing armed + + # The kwarg would add a second, auto-blind HITL layer: never pass it. + assert all("interrupt_on" not in kwargs for kwargs in captured_kwargs) + + def hitl_layers(index): + return [ + item + for item in captured_middleware[index] + if isinstance( + item, (DynamicReviewMiddleware, HumanInTheLoopMiddleware) + ) + ] + + # Attended CLI: one plain HITL layer covering the reviewed tool set. + cli_layers = hitl_layers(0) + assert len(cli_layers) == 1 + assert isinstance(cli_layers[0], HumanInTheLoopMiddleware) + assert set(cli_layers[0].interrupt_on) == set(HITL_INTERRUPT_ON) + + # Web: one layer, and it must be the auto-aware one — it is what lets a + # gateway-verified auto run proceed without interrupting. + web_layers = hitl_layers(1) + assert len(web_layers) == 1 + assert isinstance(web_layers[0], DynamicReviewMiddleware) + assert set(web_layers[0].interrupt_on) == set(HITL_INTERRUPT_ON) + + # auto_approve: nothing is armed at all. + assert hitl_layers(2) == [] # =============================================================================