HITL 装配收敛与工作区范围:后端装配落定(早前改动,随本批统一提交)
This commit is contained in:
@@ -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})
|
||||
|
||||
@@ -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."""
|
||||
|
||||
|
||||
@@ -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):
|
||||
|
||||
+73
-21
@@ -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) == []
|
||||
|
||||
|
||||
# =============================================================================
|
||||
|
||||
Reference in New Issue
Block a user