diff --git a/CHANGELOG.md b/CHANGELOG.md index 180ed5518a..42d56829d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1043,6 +1043,16 @@ This release closes that milestone with **439 merged pull requests**. The handler now rejects after the toast, and the queued first send settles the submit with its own outcome, so the draft stays for a retry and clears only once the message is sent. ([#6407]) +- **sandbox:** AIO sandboxes no longer trust unverified state for credential + placement or container reuse. A failed provisioner capability probe no longer + reads as "broker off" (which bind-mounted plaintext lark credential dirs into + brokerless Pods for the negative-cache TTL); broker mode is attested per Pod + from the provisioner's atomic observation, lark provisioning-config conflicts + carry a capability-refresh marker so a deployment change self-heals on the + next acquire, and containers whose implicit shell ended ambiguously are fenced + by a persistent quarantine (`{DEER_FLOW_HOME}/sandbox-quarantine`) across warm + reuse, rediscovery and restarts, recycled under the existing ownership and + teardown fences, with records retired only after confirmed absence. ([#6436]) - **frontend:** A failed reconnect after a page refresh is now retried in the same tab. The SDK reconnects once from the tab's `lg:stream` pointer and keeps that pointer on error, and active-run recovery skipped any run with a matching @@ -9379,6 +9389,7 @@ with **180 merged pull requests** since the first 2.0 milestone tag. [#6407]: https://github.com/bytedance/deer-flow/pull/6407 [#6412]: https://github.com/bytedance/deer-flow/pull/6412 [#6426]: https://github.com/bytedance/deer-flow/pull/6426 +[#6436]: https://github.com/bytedance/deer-flow/pull/6436 [#6441]: https://github.com/bytedance/deer-flow/pull/6441 [#6445]: https://github.com/bytedance/deer-flow/pull/6445 [#6447]: https://github.com/bytedance/deer-flow/pull/6447 diff --git a/README.md b/README.md index 5d7bedcd20..fab3074612 100644 --- a/README.md +++ b/README.md @@ -2018,6 +2018,44 @@ For example, independent read-only research can run concurrently when the wall-c ### Sandbox & File System +AIO sandboxes recycle after an uncertain implicit-shell outcome or session +creation; a confirmed `hard_timeout` remains eligible for warm reuse. The Gateway +waits for all execution and upload holders to finish, including command-session +cleanup. An in-flight session creation alone does not trigger recycling or +interrupt a concurrent run. Before recycling, the Gateway records the container ID or Pod UID under +`{DEER_FLOW_HOME}/sandbox-quarantine`. These records survive failed stops and +Gateway restarts. Gateways sharing AIO containers must share this home in +addition to their ownership store. A new runtime instance can reuse the thread's +sandbox ID; an unverifiable generation fails closed against existing records. +Before recreating a quarantined ID, the Gateway retires its old records only +after the backend confirms that no container or Pod remains, while holding the +normal teardown fences. This also permits recovery when a runtime cannot report +a generation. If a partial deletion leaves a container or a Pod behind, the +Gateway inspects it independently of health checks and Service availability, +then retries cleanup only for a quarantined generation. A replacement generation +keeps its resources. A delete acknowledgement or failed health check is +insufficient to clear the records. +Recovery also clears leftover local proxy and network resources before recreation. + +Lark broker provisioning requires confirmed capabilities. Probe failures cannot +select credential mounts or overwrite a known broker requirement. Update the +Gateway and provisioner together: the provisioner reports each Pod's actual +broker mode, and incompatible Pods are replaced through the normal ownership +fences before reuse. Lark commands use that admitted mode. +If an older provisioner omits a Pod's broker mode, the Gateway refuses reuse +with an explicit upgrade error and preserves the Pod. An omitted mode is not +evidence that the existing runtime needs replacement. +Capability probes share one request per provisioner and briefly back off after +failure. A Pod mode that contradicts the cache triggers a fresh observation +before replacement. Failed admission after creation uses ownership-fenced cleanup. +A create request whose lark provisioning mode conflicts with the provisioner's +current configuration is marked as a capability refresh; the Gateway drops its +cached observation so the next acquire re-probes instead of repeating the +failure until the cache entry expires. +On sandboxes without an attested broker mode, lark-cli +commands fail with an explicit unverified-mode error instead of running +against an unauthenticated profile. + Host-externalized tool outputs use the Gateway's normal file-creation umask. Host and sandbox outputs use deterministic filenames hashed from the raw tool call ID and output content. Missing, colliding, or oversized IDs cannot overwrite diff --git a/backend/app/gateway/authz.py b/backend/app/gateway/authz.py index c8ef181a49..0275556e5e 100644 --- a/backend/app/gateway/authz.py +++ b/backend/app/gateway/authz.py @@ -821,15 +821,27 @@ async def try_acquire_sandbox_for_request( provider=None, ) - from deerflow.sandbox.lease import get_sandbox_lease_manager + from deerflow.sandbox.lease import SandboxClientUnavailableError, get_sandbox_lease_manager owner_id = f"{owner_prefix}:{uuid.uuid4()}" - sandbox_id = await get_sandbox_lease_manager(sandbox_provider).acquire_async( - owner_id, - thread_id, - user_id=user_id, - release_on_last=release_on_last, - ) + try: + sandbox_id = await get_sandbox_lease_manager(sandbox_provider).acquire_async( + owner_id, + thread_id, + user_id=user_id, + release_on_last=release_on_last, + ) + except SandboxClientUnavailableError as error: + # The manager already reconciled this acquire without binding our + # holder. Preserve the callers' missing-client error path and avoid + # requesting a second release. + return SandboxRequestLease( + sandbox=None, + sandbox_id=error.sandbox_id, + denied=False, + owner_id=None, + provider=sandbox_provider, + ) return SandboxRequestLease( sandbox=sandbox_provider.get(sandbox_id), sandbox_id=sandbox_id, diff --git a/backend/packages/harness/deerflow/community/aio_sandbox/AGENTS.md b/backend/packages/harness/deerflow/community/aio_sandbox/AGENTS.md index 14ee56cf74..0a683ff031 100644 --- a/backend/packages/harness/deerflow/community/aio_sandbox/AGENTS.md +++ b/backend/packages/harness/deerflow/community/aio_sandbox/AGENTS.md @@ -19,3 +19,77 @@ input. Window overrides must be finite positive seconds, at most 600. Broker INFO logs contain argc/exit/elapsed only, never argument values. Regressions: `tests/test_aio_sandbox.py`, `tests/test_lark_broker.py`. + +## Reuse admission + +An implicit-shell fence is sticky for the container's lifetime, even after a +successful explicit recovery session. `requires_container_recycle` includes +that fence and ambiguous shell or bash creation tombstones; a pending creation +is a transient in-flight request and does not fence reuse. Once all holders +and command cleanup have drained, release treats any remaining pending create +as abandoned and recycles conservatively. Release persists a +generation-bound record in `{DEER_FLOW_HOME}/sandbox-quarantine` before stopping; +failed stops, lease expiry and Gateway restarts never clear it. Docker discovery +restores the container ID; provisioner responses restore the Pod UID. Unknown +generations fail closed against existing records. New generations can reuse the +logical sandbox ID. Gateways sharing containers must share this home as well as +the configured ownership store. Keep `get`/`get_scoped` free of storage IO. +Before creation, retire records only under a local reservation and teardown +lease, after `SandboxBackend.is_absent` confirms logical-ID vacancy. It must +distinguish missing resources from stopped/Pending/Terminating resources and +probe failures; DELETE acceptance and discovery returning None are insufficient. +Local absence matching compares Docker's fixed error scaffolding +case-insensitively — CLI v29 lowercases `Error: No such object:` to +`error: no such object:` — while the resource name stays byte-exact and the +whole message must match; Apple Container forms remain exact-match. +Remote proofs use `/api/sandboxes/{id}/presence`, which inspects the Pod directly; +the ordinary status GET can return 404 merely because its Service is missing. +This covers runtimes without a recoverable generation, including Apple Container. +`inspect_runtime` returns metadata independently of health, running state or +Service availability; only confirmed resource absence returns None. Probe failures +raise, including Docker context/daemon errors. Under the same teardown fences, +retry partial cleanup only if that current generation is quarantined, then prove +absence before retiring records. Preserve an unquarantined replacement generation +and its predecessor's records; unknown generations remain fenced. +Local retries recover published ports, including stopped Docker bindings, and +retain pending reservations by immutable generation until resource teardown +succeeds. Apple logical IDs are not generations or Docker policy metadata. +Remembered ports without a generation may be released only after confirmed name +vacancy; never attach them to metadata for a present replacement runtime. +A fenced sandbox still tracked by this instance is recycled inline on a later +acquire only when no execution holder remains. Ordinary release and destruction +reserve that idle transition atomically in the provider's lease manager; new +bindings cannot enter until teardown finishes. Slow cleanup stays outside the +metadata lock. Local teardown or ownership refusal returns false so acquire +defers. Shutdown explicitly closes the whole provider, including unfinished +executions. Genuine uncertainty is persisted even when live holders defer its +recycle; pending creation alone never triggers inline replacement. +Post-acquire client loss still raises `SandboxNotFoundError` after lease rollback. +Gateway sync preserves its missing-client response without a second release; +unrelated provider or cleanup errors propagate. +A Pod-attested broker mode outranks an unavailable deployment-level probe: the probe failure delays drift detection but never denies reuse of an attested runtime, while an unattested mode stays fail-closed. An unattested mode is unknown, not contradictory: during Gateway/provisioner skew, reuse defers without replacement so the healthy Pod survives until the provisioner attests it. +Before retiring records, `complete_absent_teardown` releases pending local ports +under those same fences after confirmed absence, including a timed-out stop that +actually removed the container. +It also clears deterministic local proxy/network leftovers before recreation; +unknown cleanup errors retain the quarantine record. + +Broker capability failures are unknown, never confirmed negatives. Cache only +successful observations per provisioner endpoint/auth identity; a failed refresh +may retain a confirmed broker requirement. A provisioner lark-configuration conflict marks its create response; the +Gateway drops that endpoint's cached observation so the next acquire re-probes +instead of outliving the negative TTL. Acquisition validates actual mode in +active, warm, discovered and create responses. Policy replacement keeps the +existing ownership and local teardown fences; do not stop a live peer's Pod. +`bash` uses the admitted sandbox's mode, never a fresh deployment-wide probe. +Contradictory Pod observations bypass cached capabilities before replacement; +unattested observations are not contradictions and never trigger replacement. +One in-flight probe per endpoint/auth identity shares results/errors; cache locks +never cover network IO or waits. Failure retry time is separate from confirmation +freshness and cannot authorize credential mounts. All post-create registration +failures use the existing fenced cleanup, including policy/storage/probe errors. +Rejected provisioner creation metadata must retain its `SandboxInfo` for that +cleanup. Failed discovery registration closes only its own HTTP client. + +Regressions: `test_aio_sandbox_provider.py`, `test_sandbox_quarantine.py`, +`test_remote_sandbox_backend.py`, `test_provisioner_request_threading.py`. diff --git a/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox.py b/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox.py index e1aef6e73f..a57fa45bc1 100644 --- a/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox.py +++ b/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox.py @@ -75,6 +75,7 @@ class AioSandbox(Sandbox): home_dir: str | None = None, request_headers: dict[str, str] | None = None, default_command_timeout: float | None = None, + lark_cli_broker: bool | None = None, ): """Initialize the AIO sandbox. @@ -86,8 +87,14 @@ class AioSandbox(Sandbox): relay. These are never injected into sandbox commands. default_command_timeout: Provider-configured command deadline used when a command does not provide an explicit timeout. + lark_cli_broker: Pod-attested Lark broker mode. ``True``/``False`` + are attested states; the ``None`` default keeps an unattested + sandbox fail-closed, so a construction site that forgets to + thread the attested value cannot silently authorize the + plaintext credential-mount overlay. """ super().__init__(id) + self.lark_cli_broker = lark_cli_broker if default_command_timeout is None: self._default_command_timeout = self._DEFAULT_HARD_TIMEOUT else: @@ -314,12 +321,25 @@ class AioSandbox(Sandbox): tuple(self._bash_session_creation_state.ambiguous), ) + @property + def has_pending_session_creates(self) -> bool: + """Whether a shell/bash create still needs its outcome reconciled.""" + with self._session_creation_state_lock: + return bool(self._shell_session_creation_state.pending or self._bash_session_creation_state.pending) + @property def requires_container_recycle(self) -> bool: with self._session_creation_state_lock: shell = self._shell_session_creation_state bash = self._bash_session_creation_state - return bool(shell.pending or shell.ambiguous or bash.pending or bash.ambiguous) + # The implicit shell cannot be cleaned up by a known session id. + # Its fence must outlive this client, including release/reclaim. + # Pending creations are transient in-flight requests, not uncertain + # outcomes: an unresolved create becomes an ambiguous tombstone + # before its execution lease ends. Fencing on pending let a + # concurrent acquire recycle a healthy container that another run + # was actively using. + return bool(self._default_shell_corrupted or shell.ambiguous or bash.ambiguous) def _create_shell_session(self, client) -> str: session_id = str(uuid.uuid4()) diff --git a/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox_provider.py b/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox_provider.py index d1a73679d0..b29a5da1b6 100644 --- a/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox_provider.py +++ b/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox_provider.py @@ -46,18 +46,26 @@ from deerflow.config import get_app_config from deerflow.config.paths import VIRTUAL_PATH_PREFIX, get_paths, join_host_path from deerflow.constants import DEFAULT_SKILLS_CONTAINER_PATH from deerflow.integrations.lark_cli import INTEGRATION_ID as LARK_CLI_INTEGRATION_ID -from deerflow.integrations.lark_cli import LARK_CLI_SANDBOX_CONFIG_DIR, LARK_CLI_SANDBOX_DATA_DIR, LARK_CLI_SANDBOX_LOCKS_DIR, LARK_CLI_SANDBOX_RUNTIME_DIR, ensure_lark_cli_credential_tree, lark_skills_installed +from deerflow.integrations.lark_cli import ( + LARK_CLI_SANDBOX_CONFIG_DIR, + LARK_CLI_SANDBOX_DATA_DIR, + LARK_CLI_SANDBOX_LOCKS_DIR, + LARK_CLI_SANDBOX_RUNTIME_DIR, + LarkBrokerCapabilityUnknownError, + ensure_lark_cli_credential_tree, + lark_skills_installed, +) from deerflow.runtime.user_context import get_effective_user_id from deerflow.sandbox.acquire_serialization import AcquireSerializer from deerflow.sandbox.identity import derive_sandbox_scope_token -from deerflow.sandbox.lease import run_sync_lifecycle_operation +from deerflow.sandbox.lease import discard_sandbox_lease_manager, get_sandbox_lease_manager, run_sync_lifecycle_operation from deerflow.sandbox.sandbox import Sandbox from deerflow.sandbox.sandbox_provider import SandboxProvider from deerflow.skills.types import SkillCategory from deerflow.utils.file_io import await_drained from .aio_sandbox import AioSandbox -from .backend import SANDBOX_LOCAL_PROVIDER_READY_TIMEOUT, SandboxBackend, wait_for_sandbox_ready, wait_for_sandbox_ready_async +from .backend import SANDBOX_LOCAL_PROVIDER_READY_TIMEOUT, SandboxBackend, SandboxCreationError, wait_for_sandbox_ready, wait_for_sandbox_ready_async from .local_backend import LocalContainerBackend from .ownership import ( OwnershipBackendError, @@ -68,6 +76,7 @@ from .ownership import ( make_sandbox_ownership_store, resolve_ownership_config, ) +from .quarantine import SandboxQuarantine from .remote_backend import RemoteSandboxBackend, _normalize_skills_container_path from .sandbox_info import SandboxInfo @@ -100,10 +109,19 @@ class SandboxBeingDestroyedError(RuntimeError): class SandboxPolicyReplacementDeferredError(RuntimeError): - """An incompatible sandbox cannot be replaced until it is a true orphan.""" + """A quarantined or incompatible sandbox cannot be replaced until it is a true orphan.""" + + def __init__(self, sandbox_id: str, *, reason: str | None = None) -> None: + detail = reason or "has an incompatible provisioning policy; replacement is deferred until its current owner releases it" + super().__init__(f"sandbox {sandbox_id} {detail}") + self.sandbox_id = sandbox_id + + +class SandboxBrokerModeUnverifiedError(RuntimeError): + """An existing runtime lacks the attestation needed for Lark admission.""" def __init__(self, sandbox_id: str) -> None: - super().__init__(f"sandbox {sandbox_id} has an incompatible provisioning policy; replacement is deferred until its current owner releases it") + super().__init__(f"sandbox {sandbox_id} Lark broker mode is unverified; update the provisioner before retrying; the existing runtime is preserved") self.sandbox_id = sandbox_id @@ -845,6 +863,8 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): deferred = 0 for info in running: + if self._quarantine_store().contains(info): + info.requires_replacement = True age = current_time - info.created_at if info.created_at > 0 else float("inf") if info.requires_replacement: if self._replace_incompatible_sandbox(info, current_time): @@ -1260,7 +1280,7 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): return False @staticmethod - def _lark_broker_active(user_id: str | None = None) -> bool: + def _lark_broker_active(user_id: str | None = None, *, observed_mode: bool | None = None) -> bool: """Whether this user's sandbox should use the lark-cli broker (Pattern B). True only when the Lark pack is installed AND the remote provisioner @@ -1268,15 +1288,49 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): credentials in a sidecar and the sandbox gets only a shim, so the Gateway-side credential-mount overlay must not run either. """ - try: - if not AioSandboxProvider._lark_integration_active(user_id): - return False - from deerflow.integrations.lark_cli import sandbox_lark_broker_active - - return sandbox_lark_broker_active() - except Exception as e: # pragma: no cover - defensive - logger.warning(f"Could not determine Lark broker state: {e}") + if not AioSandboxProvider._lark_integration_active(user_id): return False + from deerflow.integrations.lark_cli import sandbox_lark_broker_active + + return sandbox_lark_broker_active(observed_mode=observed_mode) if observed_mode is not None else sandbox_lark_broker_active() + + def _quarantine_store(self) -> SandboxQuarantine: + store = getattr(self, "_quarantine", None) + if store is None: + scope = (self._config.get("provisioner_url") or "").rstrip("/") or f"local:{self._config.get('container_prefix', DEFAULT_CONTAINER_PREFIX)}" + store = SandboxQuarantine(get_paths().base_dir / "sandbox-quarantine", scope) + self._quarantine = store + return store + + def _check_sandbox_reuse_policy(self, info: SandboxInfo, user_id: str) -> None: + if self._quarantine_store().contains(info): + info.requires_replacement = True + if info.requires_replacement: + return + if isinstance(self._backend, RemoteSandboxBackend) and self._lark_integration_active(user_id): + # Unknown is not evidence of drift. An older provisioner cannot + # create an attested replacement either, so refuse admission without + # destroying the user's existing runtime or probing deployment mode. + if info.lark_cli_broker is None: + raise SandboxBrokerModeUnverifiedError(info.sandbox_id) + # A contradictory Pod observation bypasses the cache, sharing one + # refresh with other workers before deciding on replacement. + try: + required_broker = self._lark_broker_active(user_id, observed_mode=info.lark_cli_broker) + except LarkBrokerCapabilityUnknownError: + # The Pod's own attestation decides how it executes lark-cli, and + # execution uses that attested mode. An unavailable deployment-level + # probe only delays drift detection (replacement); it must not deny + # reuse of a healthy runtime whose mode is attested. The drift + # check re-runs on every later acquire once the probe recovers. + logger.warning( + "Lark broker capability probe unavailable; reusing sandbox %s with its attested mode %s", + info.sandbox_id, + info.lark_cli_broker, + ) + return + if info.lark_cli_broker is not required_broker: + info.requires_replacement = True @staticmethod def _get_lark_cli_runtime_mounts(*, user_id: str | None = None) -> list[tuple[str, str, bool]]: @@ -1652,9 +1706,29 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): effective_user_id, thread_id, ) - self.destroy(stale_id) + if not self._destroy_tracked(stale_id, still_reapable=lambda: self._thread_sandboxes.get(key) == stale_id): + raise SandboxPolicyReplacementDeferredError(stale_id, reason="has an outdated skills identity but is still in use or cannot be destroyed; retry shortly") return None + if info is not None: + self._check_sandbox_reuse_policy(info, effective_user_id) + if info.requires_replacement: + if self._recycle_fenced_tracked_sandbox(existing_id): + return None + raise SandboxPolicyReplacementDeferredError( + existing_id, + reason="is quarantined or carries an incompatible policy and its fenced recycle did not complete; retry shortly", + ) + with self._lock: + recycle_candidate = self._sandboxes.get(existing_id) + if recycle_candidate is not None and getattr(recycle_candidate, "requires_container_recycle", False) is True: + if self._recycle_fenced_tracked_sandbox(existing_id): + return None + raise SandboxPolicyReplacementDeferredError( + existing_id, + reason="is quarantined after an uncertain shell outcome and its fenced recycle did not complete; retry shortly", + ) + alive = self._check_tracked_sandbox_alive(existing_id, info) if info is not None else True if alive is False: self._drop_unhealthy_sandbox( @@ -1687,6 +1761,10 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): self._forget_lost_sandbox(existing_id) return None + if info is not None: + self._check_sandbox_reuse_policy(info, effective_user_id) + if info.requires_replacement: + raise SandboxPolicyReplacementDeferredError(existing_id) with self._lock: if self._being_torn_down_locally(existing_id): # The first reservation check ran before the backend health @@ -1707,8 +1785,46 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): # fresh client, and the lease we just took is already ours. logger.info("Cached sandbox %s was dropped while publishing ownership; falling through to discovery", existing_id) return None + if getattr(self._sandboxes[existing_id], "requires_container_recycle", False) is True: + raise SandboxPolicyReplacementDeferredError( + existing_id, + reason="became quarantined while ownership was published; the next acquire recycles it", + ) return existing_id + def _recycle_fenced_tracked_sandbox(self, sandbox_id: str) -> bool: + """Destroy a fenced sandbox still tracked by this instance (inline recycle). + + A failed ``release()`` recycle leaves the sandbox tracked with its fence + marked, and neither orphan reconciliation nor the idle reaper owns tracked + active entries — deferring here would fail every acquire until some later + release retried. Recycling inline turns that wedge into one slow acquire. + A live execution holder turns the answer back into a deferral: destroying + the runtime under a running run would tear it down mid-turn, so the + recycle waits for the holders to finish — their release retries it, and + a later acquire retries inline if that failed too. + Returns True when the residue is gone and the caller may fall through to + discovery/create; False when the destroy did not complete (the fence is + retained and the caller must defer). + """ + with self._lock: + sandbox = self._sandboxes.get(sandbox_id) + info = self._sandbox_infos.get(sandbox_id) + if sandbox is None: + return True + try: + # Persist genuine uncertainty even when a live holder postpones + # destruction. Pending creates never reach this path on their own. + if getattr(sandbox, "requires_container_recycle", False) is True: + if info is None: + raise RuntimeError(f"Cannot quarantine sandbox {sandbox_id} without its runtime metadata") + info.requires_replacement = True + self._quarantine_store().mark(info) + return self._destroy_tracked(sandbox_id, still_reapable=lambda: self._sandboxes.get(sandbox_id) is sandbox) + except Exception: + logger.warning("Failed to recycle fenced sandbox %s during acquire; deferring", sandbox_id, exc_info=True) + return False + def _reclaim_warm_pool_sandbox( self, thread_id: str | None, @@ -1736,6 +1852,17 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): info, _ = self._warm_pool[sandbox_id] + self._check_sandbox_reuse_policy(info, effective_user_id) + if info.requires_replacement: + if not self._destroy_warm_entry( + sandbox_id, + info, + reason="incompatible provisioning policy", + still_reapable=lambda: self._warm_pool.get(sandbox_id, (None,))[0] is info, + ): + raise SandboxPolicyReplacementDeferredError(sandbox_id) + return None + alive = self._check_tracked_sandbox_alive(sandbox_id, info) if alive is False: self._drop_unhealthy_sandbox( @@ -1756,6 +1883,9 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): self._forget_lost_sandbox(sandbox_id) return None + self._check_sandbox_reuse_policy(info, effective_user_id) + if info.requires_replacement: + raise SandboxPolicyReplacementDeferredError(sandbox_id) with self._lock: if self._being_torn_down_locally(sandbox_id): # Re-checked, because the first check was before the round trip. @@ -1778,6 +1908,7 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): base_url=info.sandbox_url, request_headers=info.request_headers, default_command_timeout=self._config.get("command_timeout", AioSandbox._DEFAULT_HARD_TIMEOUT), + lark_cli_broker=info.lark_cli_broker, ) self._sandboxes[sandbox_id] = sandbox self._sandbox_infos[sandbox_id] = info @@ -1810,6 +1941,7 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): prevent. The window is a peer's in-flight container stop, so the thread's next turn discovers nothing and cold-starts cleanly. """ + self._check_sandbox_reuse_policy(info, user_id) if info.requires_replacement: raise SandboxPolicyReplacementDeferredError(info.sandbox_id) key = self._thread_key(thread_id, user_id) @@ -1828,6 +1960,7 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): base_url=info.sandbox_url, request_headers=info.request_headers, default_command_timeout=self._config.get("command_timeout", AioSandbox._DEFAULT_HARD_TIMEOUT), + lark_cli_broker=info.lark_cli_broker, ) # Ownership first, so a failure cannot leave a tracked-but-unowned sandbox. # There is no container to roll back (we did not create it), but the @@ -1835,6 +1968,9 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): # same close-on-failure as `_register_created_sandbox`. try: self._publish_ownership(info.sandbox_id) + self._check_sandbox_reuse_policy(info, user_id) + if info.requires_replacement: + raise SandboxPolicyReplacementDeferredError(info.sandbox_id) with self._lock: if self._being_torn_down_locally(info.sandbox_id): # The pre-publish reservation check is only an early-out: a @@ -1858,11 +1994,7 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): self._active_sandbox_identity[info.sandbox_id] = key self._last_activity[info.sandbox_id] = time.time() self._thread_sandboxes[key] = info.sandbox_id - except ( - OwnershipBackendError, - SandboxBeingDestroyedError, - SandboxIdentityCollisionError, - ): + except Exception: try: sandbox.close() except Exception as e: @@ -1874,28 +2006,22 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): def _register_created_sandbox(self, thread_id: str | None, sandbox_id: str, info: SandboxInfo, *, user_id: str | None = None) -> str: """Track a newly-created sandbox in the active maps.""" - sandbox = AioSandbox( - id=sandbox_id, - base_url=info.sandbox_url, - request_headers=info.request_headers, - default_command_timeout=self._config.get("command_timeout", AioSandbox._DEFAULT_HARD_TIMEOUT), - ) - key = ( - self._thread_key( - thread_id, - self._effective_acquire_user_id(user_id), - ) - if thread_id - else None - ) - # Ownership first. Unlike the discover path there IS something to roll - # back: we just started this container, and an unowned running container - # is exactly what a peer's reconciliation adopts. Leaking it would hand a - # peer a container this instance is about to use. - # SandboxBeingDestroyedError is possible even here: a peer that died - # mid-stop leaves a teardown marker until its TTL lapses. Roll back on - # both, or the container we just started is leaked. + sandbox: AioSandbox | None = None + # Everything after backend.create belongs to the same rollback boundary. + # Creation can return an existing Pod, so cleanup must still respect + # peer ownership and this provider's active/warm reservations. try: + self._check_sandbox_reuse_policy(info, self._effective_acquire_user_id(user_id)) + if info.requires_replacement: + raise SandboxPolicyReplacementDeferredError(sandbox_id) + sandbox = AioSandbox( + id=sandbox_id, + base_url=info.sandbox_url, + request_headers=info.request_headers, + default_command_timeout=self._config.get("command_timeout", AioSandbox._DEFAULT_HARD_TIMEOUT), + lark_cli_broker=info.lark_cli_broker, + ) + key = self._thread_key(thread_id, self._effective_acquire_user_id(user_id)) if thread_id else None if key is not None: with self._lock: self._assert_active_identity_available_locked(sandbox_id, key) @@ -1915,19 +2041,16 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): self._last_activity[sandbox_id] = time.time() if key is not None: self._thread_sandboxes[key] = sandbox_id - except ( - OwnershipBackendError, - SandboxBeingDestroyedError, - SandboxIdentityCollisionError, - ): + except Exception: logger.error( "Could not register new sandbox %s; attempting ownership-fenced cleanup", sandbox_id, ) - try: - sandbox.close() - except Exception as e: - logger.warning(f"Error closing sandbox {sandbox_id} during ownership rollback: {e}") + if sandbox is not None: + try: + sandbox.close() + except Exception as e: + logger.warning(f"Error closing sandbox {sandbox_id} during ownership rollback: {e}") self._destroy_unready_sandbox(sandbox_id, info) raise @@ -2239,6 +2362,7 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): # Backend discovery: another process may have created the container. discovered = self._backend.discover(sandbox_id) if discovered is not None: + self._check_sandbox_reuse_policy(discovered, effective_user_id) if discovered.requires_replacement: if not self._replace_incompatible_sandbox(discovered, time.time()): raise SandboxPolicyReplacementDeferredError(sandbox_id) @@ -2281,6 +2405,7 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): # Docker and perform a health check; keep it off the event loop. discovered = await run_sync_lifecycle_operation(self._backend.discover, sandbox_id) if discovered is not None: + await run_sync_lifecycle_operation(self._check_sandbox_reuse_policy, discovered, effective_user_id) if discovered.requires_replacement: replaced = await run_sync_lifecycle_operation( self._replace_incompatible_sandbox, @@ -2363,6 +2488,62 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): finally: self._finish_local_teardown(sandbox_id) + def _retire_absent_quarantine(self, sandbox_id: str) -> None: + """Retry fenced cleanup and retire records only after confirmed absence.""" + info = SandboxInfo(sandbox_id, "") + store = self._quarantine_store() + if not store.contains(info): + return + if not self._reserve_local_teardown( + sandbox_id, + lambda: sandbox_id not in self._sandboxes and sandbox_id not in self._sandbox_infos and sandbox_id not in self._warm_pool, + ): + raise SandboxPolicyReplacementDeferredError( + sandbox_id, + reason="is quarantined and still tracked; its cleanup is deferred until the in-flight teardown finishes", + ) + try: + if not self._claim_ownership(sandbox_id, for_destroy=True): + raise SandboxPolicyReplacementDeferredError( + sandbox_id, + reason="is quarantined and owned by a peer; its cleanup is deferred until that owner releases it", + ) + with self._held_teardown_lease(sandbox_id): + if self._backend.is_absent(sandbox_id) is not True: + inspect_runtime = getattr(self._backend, "inspect_runtime", None) + if not callable(inspect_runtime): + raise RuntimeError(f"Runtime inspection is unavailable for sandbox {sandbox_id}") + current = inspect_runtime(sandbox_id) + if current is not None: + if not isinstance(current, SandboxInfo) or current.sandbox_id != sandbox_id: + raise RuntimeError(f"Runtime inspection returned invalid identity for sandbox {sandbox_id}") + # A replacement can share the logical ID while its Service + # is still unavailable. Do not stop that unquarantined + # runtime; create can repair its endpoint and validate the + # actual provisioning mode. The replacement itself proves + # the fenced generations are absent (logical IDs are unique + # per Pod/container), so prune only those stale records — + # a full retire() would also wipe a fence a peer may have + # just written for this live generation. + if not store.contains(current): + store.retire_replaced_generations(current) + return + # Inspection recovers runtime identity, but replacement + # cleanup must also remove its previous policy resources. + current.requires_replacement = True + self._backend.destroy(current) + # DELETE acceptance cannot prove the Pod/container vanished. + # Also recheck when it disappeared between our observations. + if self._backend.is_absent(sandbox_id) is not True: + raise SandboxPolicyReplacementDeferredError( + sandbox_id, + reason="is quarantined and its previous runtime is still present; the fence is retained", + ) + self._backend.complete_absent_teardown(sandbox_id) + store.retire(info) + finally: + self._finish_local_teardown(sandbox_id) + def _create_sandbox(self, thread_id: str | None, sandbox_id: str, *, user_id: str | None = None) -> str: """Create a new sandbox via the backend. @@ -2376,6 +2557,7 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): Raises: RuntimeError: If sandbox creation or readiness check fails. """ + self._retire_absent_quarantine(sandbox_id) effective_user_id = self._effective_acquire_user_id(user_id) extra_mounts = self._get_extra_mounts(thread_id, user_id=effective_user_id) provision_lark_cli_runtime = self._lark_integration_active(effective_user_id) @@ -2397,15 +2579,19 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): create_kwargs["config_mount_exclusion_root"] = config_mount_exclusion_root if isinstance(self._backend, RemoteSandboxBackend): create_kwargs["skills_container_path"] = self._configured_skills_container_path() - info = self._backend.create( - thread_id, - sandbox_id, - extra_mounts=extra_mounts or None, - user_id=effective_user_id, - provision_lark_cli_runtime=provision_lark_cli_runtime, - provision_lark_cli_broker=provision_lark_cli_broker, - **create_kwargs, - ) + try: + info = self._backend.create( + thread_id, + sandbox_id, + extra_mounts=extra_mounts or None, + user_id=effective_user_id, + provision_lark_cli_runtime=provision_lark_cli_runtime, + provision_lark_cli_broker=provision_lark_cli_broker, + **create_kwargs, + ) + except SandboxCreationError as exc: + self._destroy_unready_sandbox(sandbox_id, exc.info) + raise # Wait for sandbox to be ready readiness_kwargs = {"headers": info.request_headers} if info.request_headers else {} @@ -2421,6 +2607,7 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): async def _create_sandbox_async(self, thread_id: str | None, sandbox_id: str, *, user_id: str | None = None) -> str: """Async counterpart to ``_create_sandbox``.""" + await run_sync_lifecycle_operation(self._retire_absent_quarantine, sandbox_id) effective_user_id = self._effective_acquire_user_id(user_id) extra_mounts = await run_sync_lifecycle_operation(self._get_extra_mounts, thread_id, user_id=effective_user_id) provision_lark_cli_runtime = await run_sync_lifecycle_operation(self._lark_integration_active, effective_user_id) @@ -2443,16 +2630,20 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): create_kwargs["config_mount_exclusion_root"] = config_mount_exclusion_root if isinstance(self._backend, RemoteSandboxBackend): create_kwargs["skills_container_path"] = self._configured_skills_container_path() - info = await run_sync_lifecycle_operation( - self._backend.create, - thread_id, - sandbox_id, - extra_mounts=extra_mounts or None, - user_id=effective_user_id, - provision_lark_cli_runtime=provision_lark_cli_runtime, - provision_lark_cli_broker=provision_lark_cli_broker, - **create_kwargs, - ) + try: + info = await run_sync_lifecycle_operation( + self._backend.create, + thread_id, + sandbox_id, + extra_mounts=extra_mounts or None, + user_id=effective_user_id, + provision_lark_cli_runtime=provision_lark_cli_runtime, + provision_lark_cli_broker=provision_lark_cli_broker, + **create_kwargs, + ) + except SandboxCreationError as exc: + await run_sync_lifecycle_operation(self._destroy_unready_sandbox, sandbox_id, exc.info) + raise # Wait for sandbox to be ready without blocking the event loop. readiness_kwargs = {"headers": info.request_headers} if info.request_headers else {} @@ -2516,10 +2707,10 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): def release(self, sandbox_id: str) -> None: """Release a sandbox from active use. - Healthy sandboxes are parked in the warm pool for fast reuse. Sandboxes - quarantined after an ambiguous session-creation outcome are destroyed - instead so unresolved server-side session state is never deliberately - reused. + Healthy sandboxes are parked in the warm pool for fast reuse. An unsafe + implicit shell or ambiguous session creation is quarantined by runtime + generation before destruction, so a failed stop cannot make discovery + or a restarted Gateway erase unresolved server-side state. Release is best-effort at turn teardown: recycle failures are logged rather than propagated to the completed agent run. @@ -2527,12 +2718,24 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): Args: sandbox_id: The ID of the sandbox to release. """ + with get_sandbox_lease_manager(self).reserve_idle_teardown(sandbox_id) as reserved: + if not reserved: + logger.info("Deferring release of sandbox %s: an execution holder or another teardown is still active", sandbox_id) + return + self._release_idle_sandbox(sandbox_id) + + def _release_idle_sandbox(self, sandbox_id: str) -> None: + """Park or recycle while new execution bindings are excluded.""" with self._lock: recycle_sandbox = self._sandboxes.get(sandbox_id) - if recycle_sandbox is not None and recycle_sandbox.requires_container_recycle: + # At this boundary every admitted operation has drained. Any remaining + # pending create is abandoned, so conservatively retire its runtime. + # Read pending first so a pending -> ambiguous transition cannot slip + # between the two checks and accidentally enter the warm pool. + if recycle_sandbox is not None and (getattr(recycle_sandbox, "has_pending_session_creates", False) is True or recycle_sandbox.requires_container_recycle): logger.warning( - "Recycling sandbox %s instead of returning it to the warm pool after ambiguous session creation", + "Recycling sandbox %s instead of returning it to the warm pool after an uncertain shell outcome", sandbox_id, ) try: @@ -2542,7 +2745,7 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): ) except Exception: logger.error( - "Failed to recycle sandbox %s after ambiguous session creation", + "Failed to recycle quarantined sandbox %s", sandbox_id, exc_info=True, ) @@ -2590,7 +2793,7 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): logger.info(f"Released sandbox {sandbox_id} to warm pool (container still running)") - def destroy(self, sandbox_id: str) -> None: + def destroy(self, sandbox_id: str) -> bool: """Destroy a sandbox: stop the container and free all resources. Unlike release(), this actually stops the container. Use this for @@ -2602,10 +2805,16 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): Args: sandbox_id: The ID of the sandbox to destroy. - """ - self._destroy_tracked(sandbox_id, still_reapable=lambda: True) - def _destroy_tracked(self, sandbox_id: str, *, still_reapable: Callable[[], bool]) -> None: + Returns: + True when the container was destroyed. False when teardown was + deferred because an execution/upload holder or a peer teardown is + still active — the sandbox stays tracked and the deferred destroy + is retried by the release/idle paths once the holders drain. + """ + return self._destroy_tracked(sandbox_id, still_reapable=lambda: True) + + def _destroy_tracked(self, sandbox_id: str, *, still_reapable: Callable[[], bool], force: bool = False) -> bool: """``destroy()`` with a caller-supplied "is this still reapable" gate. Callers that decided to destroy *earlier* (the idle checker) pass their @@ -2613,22 +2822,38 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): section that reserves the teardown. ``destroy()`` itself passes a constant: an explicit destroy is a decision made now. """ - if not self._reserve_local_teardown(sandbox_id, still_reapable): - logger.info("Skipping destroy of sandbox %s: re-acquired by this instance or already being torn down", sandbox_id) - return + reservation = contextlib.nullcontext(True) if force else get_sandbox_lease_manager(self).reserve_idle_teardown(sandbox_id) + with reservation as reserved: + if not reserved: + logger.info("Deferring destroy of sandbox %s: an execution holder or another teardown is still active", sandbox_id) + return False + if not self._reserve_local_teardown(sandbox_id, still_reapable): + logger.info("Skipping destroy of sandbox %s: re-acquired by this instance or already being torn down", sandbox_id) + return False + try: + return self._destroy_reserved(sandbox_id) + finally: + self._finish_local_teardown(sandbox_id) - try: - self._destroy_reserved(sandbox_id) - finally: - self._finish_local_teardown(sandbox_id) + def _destroy_reserved(self, sandbox_id: str) -> bool: + with self._lock: + sandbox = self._sandboxes.get(sandbox_id) + info = self._sandbox_infos.get(sandbox_id) + if getattr(sandbox, "has_pending_session_creates", False) is True or getattr(sandbox, "requires_container_recycle", False) is True: + if info is None: + raise RuntimeError(f"Cannot quarantine sandbox {sandbox_id} without its runtime metadata") + # All destroy paths must persist before claiming/untracking. If + # storage fails, retain the dirty client and its renewal lease so + # a later idle/shutdown/explicit destroy retries this same fence. + info.requires_replacement = True + self._quarantine_store().mark(info) - def _destroy_reserved(self, sandbox_id: str) -> None: # Claim before untracking. The reverse order loses the container on a # refused claim: still running, and no longer in any of our maps, so # nothing here would ever reap or reclaim it. if not self._claim_ownership(sandbox_id, for_destroy=True): logger.warning("Refusing to destroy sandbox %s: owned by another instance", sandbox_id) - return + return False sandbox, info, _ = self._remove_tracked_sandbox(sandbox_id) @@ -2656,9 +2881,11 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): # marker the claim above wrote, so an untracked id cannot leave a # lease stuck in `del:`. self._release_ownership(sandbox_id) + return True def reset(self) -> None: """Release process-local acquire workers when this instance is detached.""" + discard_sandbox_lease_manager(self) self._acquire_serializer.close() self._acquire_worker_executor.shutdown(wait=False, cancel_futures=True) @@ -2694,7 +2921,9 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): for sandbox_id in sandbox_ids: try: - self.destroy(sandbox_id) + # Shutdown has stopped the maintenance workers and closes the + # entire provider. It must clean up even unfinished executions. + self._destroy_tracked(sandbox_id, still_reapable=lambda: True, force=True) except Exception as e: logger.error(f"Failed to destroy sandbox {sandbox_id} during shutdown: {e}") @@ -2711,5 +2940,6 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): except Exception as e: logger.warning(f"Error closing sandbox ownership store during shutdown: {e}") + discard_sandbox_lease_manager(self) self._acquire_serializer.close() self._acquire_worker_executor.shutdown(wait=False, cancel_futures=True) diff --git a/backend/packages/harness/deerflow/community/aio_sandbox/backend.py b/backend/packages/harness/deerflow/community/aio_sandbox/backend.py index 8e8508e8e7..7ad71fa02c 100644 --- a/backend/packages/harness/deerflow/community/aio_sandbox/backend.py +++ b/backend/packages/harness/deerflow/community/aio_sandbox/backend.py @@ -49,6 +49,14 @@ def sandbox_http_trust_env(sandbox_url: str) -> bool: SANDBOX_LOCAL_PROVIDER_READY_TIMEOUT = 60 +class SandboxCreationError(RuntimeError): + """Preserve a rejected creation result for ownership-fenced cleanup.""" + + def __init__(self, message: str, *, info: SandboxInfo): + super().__init__(message) + self.info = info + + def wait_for_sandbox_ready( sandbox_url: str, timeout: int = 30, @@ -202,6 +210,34 @@ class SandboxBackend(ABC): """ ... + def is_absent(self, sandbox_id: str) -> bool: + """Prove that no runtime exists under this logical ID, even if stopped. + + Unknown/unavailable runtimes must never return True. Backends without + an authoritative absence check retain their quarantine fences. + """ + return False + + def complete_absent_teardown(self, sandbox_id: str) -> None: + """Finish cleanup bookkeeping after authoritative runtime absence. + + The provider must confirm ``is_absent(sandbox_id) is True`` and hold + both its local teardown reservation and cross-worker teardown lease + throughout this call. Implementations may release pending resources + scoped to this absent logical ID. Failure must raise so quarantine is + retained and cleanup can be retried before creating a replacement. + Backends without pending cleanup require no completion work. + """ + + def inspect_runtime(self, sandbox_id: str) -> SandboxInfo | None: + """Inspect the runtime generation independently of endpoint readiness. + + Return None only after confirming no resource has this logical ID, + including stopped or terminating resources. Unavailable or unsupported + inspections must raise rather than imply absence. + """ + raise RuntimeError(f"Runtime inspection is unavailable for sandbox {sandbox_id}") + def list_running(self) -> list[SandboxInfo]: """Enumerate all running sandboxes managed by this backend. diff --git a/backend/packages/harness/deerflow/community/aio_sandbox/local_backend.py b/backend/packages/harness/deerflow/community/aio_sandbox/local_backend.py index a3c59008bb..3753c012cb 100644 --- a/backend/packages/harness/deerflow/community/aio_sandbox/local_backend.py +++ b/backend/packages/harness/deerflow/community/aio_sandbox/local_backend.py @@ -14,10 +14,12 @@ import logging import os import platform import posixpath +import re import secrets import shlex import socket import subprocess +import threading import time from dataclasses import dataclass from datetime import datetime @@ -49,6 +51,7 @@ class _ContainerInspection: networks: frozenset[str] relay_token: str | None = None max_shell_sessions: int | None = None + container_id: str | None = None @dataclass(frozen=True) @@ -88,18 +91,24 @@ def _parse_docker_timestamp(raw: str) -> float: return 0.0 -def _extract_host_port(inspect_entry: dict, container_port: int) -> int | None: +def _extract_host_port(inspect_entry: dict, container_port: int, *, include_stopped: bool = False) -> int | None: """Extract the host port mapped to ``container_port/tcp`` from a docker inspect entry. - Returns None if the container has no port mapping for that port. + Returns None if the container has no port mapping for that port. Teardown + may use configured bindings after Docker clears a stopped container's + active mappings; ordinary discovery still requires an active mapping. """ try: ports = (inspect_entry.get("NetworkSettings") or {}).get("Ports") or {} bindings = ports.get(f"{container_port}/tcp") or [] + if not bindings and include_stopped: + ports = (inspect_entry.get("HostConfig") or {}).get("PortBindings") or {} + bindings = ports.get(f"{container_port}/tcp") or [] if bindings: host_port = bindings[0].get("HostPort") if host_port: - return int(host_port) + parsed = int(host_port) + return parsed if 0 < parsed <= 65535 else None except (ValueError, TypeError, AttributeError): pass return None @@ -531,22 +540,58 @@ def _docker_resource_limit(env_name: str, default: str) -> str | None: return value +# Docker's missing-resource scaffolding drifted in case across CLI releases: +# v28 printed "Error: No such object: " while v29 lowercases the +# client-side form to "error: no such object: ". The daemon-side wrapper +# ("Error response from daemon: ") is stable but normalized here as well. +# Match the fixed scaffolding case-insensitively while capturing the resource +# name for a byte-exact comparison, so Docker context names and unrelated +# runtime failures containing "container"/"not found" can never pass. +_DOCKER_NO_SUCH_CONTAINER_RE = re.compile( + r"(?:error(?: response from daemon)?: )?no such (?:object|container): (?P.+)", + re.IGNORECASE, +) +_DOCKER_NO_SUCH_NETWORK_RE = re.compile( + r"(?:error(?: response from daemon)?: )?(?:no such network: (?P.+)|network (?P.+) not found)", + re.IGNORECASE, +) + + def _is_no_such_container_error(stderr: str, container_name: str) -> bool: """Return True only when stderr definitively says the container does not exist. - Docker reports "No such object" / "No such container". Apple Container - reports a generic "not found", so that phrase is only trusted when the - message also names the inspected container (or refers to a - container/object); transient failures whose text happens to contain - "not found" (e.g. "command not found", "context not found") must stay on - the raise path instead of being misread as a dead container. + Match the complete resource error and exact target, rather than searching + for "container" / "not found": Docker context names and runtime failures + can contain both. The Docker scaffolding is compared case-insensitively + (CLI v29 lowercases it) while the container name stays byte-exact. + Apple Container's missing-resource error must identify this container too; + its logical name is not proof of another missing file. """ - message = stderr.lower() - if "no such object" in message or "no such container" in message: + message = (stderr or "").strip() + docker_match = _DOCKER_NO_SUCH_CONTAINER_RE.fullmatch(message) + if docker_match is not None and docker_match["name"] == container_name: return True - if "not found" not in message: + apple_errors = { + f"Error: container not found: {container_name}", + f'Error: notFound: "container not found: {container_name}"', + f'notFound: "container not found: {container_name}"', + # Preserve the older generic response only for the exact quoted target. + f'Error: not found: "{container_name}"', + } + return message in apple_errors + + +def _is_no_such_network_error(stderr: str, network_name: str) -> bool: + """Accept Docker's complete missing-network response for this exact name. + + Same contract as ``_is_no_such_container_error``: case-insensitive + scaffolding, byte-exact network name, full-message match. + """ + message = (stderr or "").strip() + match = _DOCKER_NO_SUCH_NETWORK_RE.fullmatch(message) + if match is None: return False - return container_name.lower() in message or "container" in message or "object" in message + return (match["named"] or match["embedded"]) == network_name class LocalContainerBackend(SandboxBackend): @@ -597,6 +642,16 @@ class LocalContainerBackend(SandboxBackend): self._network_config = network_config or {"mode": "open"} self._network_mode = str(self._network_config.get("mode", "open")) self._allow_synthetic_dns = False + # Teardown reservations serialize each generation. Different sandbox + # generations have independent entries; process restart also resets + # the process-local PortAllocator, so these need no durable storage. + self._pending_cleanup_ports: dict[str, tuple[str, int]] = {} + # Native Apple / legacy metadata may lack a runtime generation. This + # ledger tracks only our local reservations, never container identity; + # present-resource inspection must not inherit it. Name vacancy under + # the provider's teardown fences is its sole fallback release proof. + self._unknown_cleanup_ports: dict[str, set[int]] = {} + self._pending_cleanup_ports_lock = threading.Lock() self._runtime = self._detect_runtime() if self._network_mode != "open": @@ -734,8 +789,7 @@ class LocalContainerBackend(SandboxBackend): except (FileNotFoundError, subprocess.TimeoutExpired, OSError) as exc: raise RuntimeError(f"Failed to inspect restricted sandbox network {network_name}") from exc if result.returncode != 0: - stderr = (result.stderr or "").lower() - if "not found" in stderr and (network_name.lower() in stderr or "network" in stderr): + if _is_no_such_network_error(result.stderr or "", network_name): return None raise RuntimeError(f"Failed to inspect restricted sandbox network {network_name}: {(result.stderr or '').strip()}") try: @@ -1220,27 +1274,83 @@ class LocalContainerBackend(SandboxBackend): def destroy(self, info: SandboxInfo) -> None: """Stop the container and release its port.""" - # Prefer container_id, fall back to container_name (both accepted by docker stop). - # This ensures containers discovered via list_running() (which only has the name) - # can also be stopped. + from urllib.parse import urlparse + + try: + port = urlparse(info.sandbox_url).port + except (TypeError, ValueError): + port = None + generation = info.container_id if self._runtime == "docker" else None + if generation: + with self._pending_cleanup_ports_lock: + pending = self._pending_cleanup_ports.get(generation) + port = pending[1] if pending is not None else port + if port: + self._pending_cleanup_ports[generation] = (info.sandbox_id, port) + elif port: + with self._pending_cleanup_ports_lock: + self._unknown_cleanup_ports.setdefault(info.sandbox_id, set()).add(port) + restricted_cleanup = self._runtime == "docker" and (self._network_mode != "open" or info.requires_replacement) + # Keep the main generation inspectable if proxy teardown fails. Once + # its proxy is removed, a failed main stop retains the known reservation + # above for a subsequent generation-fenced retry. + if restricted_cleanup: + self._remove_restricted_proxy(info.sandbox_id) + # Prefer Docker's generation ID; names also support older metadata and + # Apple Container's caller-selected identity. stop_target = info.container_id or info.container_name if stop_target: self._stop_container(stop_target) - # An incompatible sandbox discovered while the new process is in open - # mode may have been provisioned by a previous restricted-mode process. - # Remove its deterministic sidecar/networks from this provider-owned, - # fenced destroy path as well (never from discovery itself). - if self._runtime == "docker" and (self._network_mode != "open" or info.requires_replacement): - self._cleanup_restricted_resources(info.sandbox_id, stop_sandbox=False) - # Extract port from sandbox_url for release - try: - from urllib.parse import urlparse - - port = urlparse(info.sandbox_url).port - if port: + # The proxy has been removed and the main stop succeeded. Network + # cleanup cannot retain this host port, and a timeout there must not + # strand its reservation after the main generation disappears. + if generation: + with self._pending_cleanup_ports_lock: + pending = self._pending_cleanup_ports.get(generation) + if pending is not None: + release_port(pending[1]) + self._pending_cleanup_ports.pop(generation) + self._forget_unknown_cleanup_port(info.sandbox_id, pending[1]) + elif port: + with self._pending_cleanup_ports_lock: + release_port(port) + self._forget_unknown_cleanup_port(info.sandbox_id, port) + if restricted_cleanup: + self._cleanup_restricted_resources(info.sandbox_id, stop_sandbox=False, remove_proxy=False) + + def _forget_unknown_cleanup_port(self, sandbox_id: str, port: int) -> None: + """Remove a released reservation; caller holds the cleanup ledger lock.""" + pending = self._unknown_cleanup_ports.get(sandbox_id) + if pending is not None: + pending.discard(port) + if not pending: + self._unknown_cleanup_ports.pop(sandbox_id) + + def complete_absent_teardown(self, sandbox_id: str) -> None: + """Release local reservations after fenced, authoritative name vacancy. + + A timed-out stop can still remove its container. The provider calls this + only after confirming absence under both teardown fences, before it + retires quarantine records. Metadata inspection stays read-only. + """ + if self._runtime == "docker": + # A timeout can remove the main container before its deterministic + # sidecar/networks are cleaned. Retry even after a process restart, + # where the local reservation ledger is empty. Unknown cleanup + # failures must retain quarantine instead of stranding new creation + # behind the existing-resource mismatch guard. + self._cleanup_restricted_resources(sandbox_id, stop_sandbox=False, strict=True) + with self._pending_cleanup_ports_lock: + ports = {port for owner_id, port in self._pending_cleanup_ports.values() if owner_id == sandbox_id} + ports.update(self._unknown_cleanup_ports.pop(sandbox_id, set())) + # Drop every reservation owned by this logical ID from both ledgers + # in one pass. The set union releases each port at most once even if + # a reservation ever exists in both ledgers. + retained = {generation: entry for generation, entry in self._pending_cleanup_ports.items() if entry[0] != sandbox_id} + self._pending_cleanup_ports.clear() + self._pending_cleanup_ports.update(retained) + for port in ports: release_port(port) - except Exception: - pass def is_alive(self, info: SandboxInfo) -> bool: """Check if the container is still running (lightweight, no HTTP).""" @@ -1253,6 +1363,51 @@ class LocalContainerBackend(SandboxBackend): return True return False + def inspect_runtime(self, sandbox_id: str) -> SandboxInfo | None: + """Inspect resource identity without requiring a running, healthy sandbox. + + Only an exact runtime missing-resource response returns None. Docker's + immutable ID fences teardown by generation; Apple Container exposes a + caller-selected logical ID instead, so its generation stays unknown. + """ + name = f"{self._container_prefix}-{sandbox_id}" + entry = self._inspect_container_entries([name], timeout=5).get(name) + if entry is None: + return None + container_id = None + if self._runtime == "docker": + container_id = entry.get("Id") + if not isinstance(container_id, str) or not container_id.strip(): + raise RuntimeError(f"Docker did not report a runtime generation for container {name}") + host_port = _extract_host_port(entry, 8080, include_stopped=True) + if self._runtime == "docker": + config = entry.get("Config") or {} + labels = config.get("Labels") or {} + if labels.get("deerflow.network_mode") in {"allowlist", "isolated"} or self._network_mode != "open": + proxy_name, _ = self._resource_names(sandbox_id) + proxy = self._inspect_container_entries([proxy_name], timeout=5).get(proxy_name) + # Missing proxy metadata does not prove the sandbox is absent. + # Only an observed binding may release a local reservation. + if host_port is None and proxy is not None: + host_port = _extract_host_port(proxy, 8080, include_stopped=True) + sandbox_host = _normalize_sandbox_host_for_url(os.environ.get("DEER_FLOW_SANDBOX_HOST", "localhost")) + if container_id: + with self._pending_cleanup_ports_lock: + pending = self._pending_cleanup_ports.get(container_id) + if pending is not None: + host_port = pending[1] + return SandboxInfo( + sandbox_id=sandbox_id, + sandbox_url=f"http://{sandbox_host}:{host_port}" if host_port else "", + container_name=name, + container_id=container_id, + created_at=_parse_docker_timestamp(entry.get("Created", "")), + ) + + def is_absent(self, sandbox_id: str) -> bool: + """Only an explicit missing-container response proves name vacancy.""" + return self.inspect_runtime(sandbox_id) is None + def discover(self, sandbox_id: str) -> SandboxInfo | None: """Discover an existing container by its deterministic name. @@ -1308,6 +1463,7 @@ class LocalContainerBackend(SandboxBackend): sandbox_id=sandbox_id, sandbox_url="", container_name=container_name, + container_id=sandbox_inspection.container_id, created_at=created_at, requires_replacement=True, ) @@ -1316,6 +1472,7 @@ class LocalContainerBackend(SandboxBackend): sandbox_id=sandbox_id, sandbox_url="", container_name=container_name, + container_id=sandbox_inspection.container_id, created_at=created_at, requires_replacement=True, ) @@ -1333,6 +1490,7 @@ class LocalContainerBackend(SandboxBackend): sandbox_id=sandbox_id, sandbox_url="", container_name=container_name, + container_id=sandbox_inspection.container_id if sandbox_inspection else None, created_at=created_at, requires_replacement=True, ) @@ -1355,6 +1513,7 @@ class LocalContainerBackend(SandboxBackend): sandbox_id=sandbox_id, sandbox_url="", container_name=container_name, + container_id=sandbox_inspection.container_id if sandbox_inspection else None, created_at=created_at, requires_replacement=True, ) @@ -1369,6 +1528,7 @@ class LocalContainerBackend(SandboxBackend): sandbox_id=sandbox_id, sandbox_url=sandbox_url, container_name=container_name, + container_id=sandbox_inspection.container_id if sandbox_inspection else None, created_at=created_at, request_headers=request_headers, ) @@ -1503,6 +1663,7 @@ class LocalContainerBackend(SandboxBackend): sandbox_id=sandbox_id, sandbox_url=sandbox_url, container_name=container_name, + container_id=data.container_id, created_at=created_at, request_headers=request_headers, requires_replacement=requires_replacement, @@ -1512,11 +1673,8 @@ class LocalContainerBackend(SandboxBackend): logger.info(f"Found {len(infos)} running sandbox container(s)") return infos - def _cleanup_restricted_resources(self, sandbox_id: str, *, stop_sandbox: bool = True) -> None: - proxy_name, network_name = self._resource_names(sandbox_id) - egress_network_name = self._egress_network_name(sandbox_id) - if stop_sandbox: - self._stop_container(f"{self._container_prefix}-{sandbox_id}") + def _remove_restricted_proxy(self, sandbox_id: str) -> None: + proxy_name, _ = self._resource_names(sandbox_id) self._stop_container(proxy_name) # ``--rm`` removes a container after it has run and then stopped, but # not one left in Docker's Created state by a failure before start. @@ -1530,8 +1688,40 @@ class LocalContainerBackend(SandboxBackend): errors="replace", timeout=15, ) - if removed.returncode != 0 and "no such container" not in (removed.stderr or "").lower(): - logger.warning("Failed to remove sandbox network proxy %s: %s", proxy_name, removed.stderr.strip()) + if removed.returncode != 0 and not _is_no_such_container_error(removed.stderr or "", proxy_name): + raise RuntimeError(f"Failed to remove sandbox network proxy {proxy_name}: {(removed.stderr or '').strip()}") + + def _cleanup_restricted_resources(self, sandbox_id: str, *, stop_sandbox: bool = True, remove_proxy: bool = True, strict: bool = False) -> None: + """Tear down the restricted sidecar set. + + ``strict=True`` makes any stage failure raise so the caller retains the + quarantine fence; ``strict=False`` is the best-effort rollback contract: + every stage is attempted and failures are logged, never propagated. + """ + _, network_name = self._resource_names(sandbox_id) + egress_network_name = self._egress_network_name(sandbox_id) + if stop_sandbox: + try: + self._stop_container(f"{self._container_prefix}-{sandbox_id}") + except Exception: + if strict: + raise + logger.warning( + "Failed to stop sandbox container for %s during best-effort cleanup", + sandbox_id, + exc_info=True, + ) + if remove_proxy: + try: + self._remove_restricted_proxy(sandbox_id) + except Exception: + if strict: + raise + logger.warning( + "Failed to remove network proxy for %s during best-effort cleanup", + sandbox_id, + exc_info=True, + ) for current_network_name in (network_name, egress_network_name): result = subprocess.run( ["docker", "network", "rm", current_network_name], @@ -1541,8 +1731,13 @@ class LocalContainerBackend(SandboxBackend): errors="replace", timeout=15, ) - if result.returncode != 0 and "not found" not in (result.stderr or "").lower(): - logger.warning("Failed to remove sandbox network %s: %s", current_network_name, result.stderr.strip()) + if result.returncode != 0: + stderr = (result.stderr or "").strip() + if strict: + if not _is_no_such_network_error(stderr, current_network_name): + raise RuntimeError(f"Failed to remove sandbox network {current_network_name}: {stderr or ''}") + elif "not found" not in stderr.lower(): + logger.warning("Failed to remove sandbox network %s: %s", current_network_name, stderr) def consume_network_policy_events(self, sandbox_id: str) -> list[dict[str, object]]: if self._network_mode != "allowlist" or self._network_config.get("approval", "prompt") != "prompt": @@ -1610,12 +1805,8 @@ class LocalContainerBackend(SandboxBackend): ) return result.returncode == 0 - def _batch_inspect(self, container_names: list[str], *, strict: bool = False) -> dict[str, _ContainerInspection]: - """Batch-inspect containers in a single subprocess call. - - Returns creation/port plus policy-relevant labels, image, and networks. - Missing containers or parse failures are silently dropped from the result. - """ + def _inspect_container_entries(self, container_names: list[str], *, timeout: float = 15, runtime_identity: bool = True) -> dict[str, dict]: + """Read inspect JSON, accounting for every queried name or exact absence.""" if not container_names: return {} try: @@ -1625,42 +1816,70 @@ class LocalContainerBackend(SandboxBackend): text=True, encoding="utf-8", errors="replace", - timeout=15, + timeout=timeout, ) - except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError, OSError) as e: - if strict: - raise RuntimeError("Failed to batch-inspect containers") from e - logger.warning(f"Failed to batch-inspect containers: {e}") - return {} + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, OSError) as exc: + raise RuntimeError(f"Failed to inspect containers with {self._runtime}") from exc + missing: set[str] = set() if result.returncode != 0: stderr = (result.stderr or "").strip() - missing = "no such object" in stderr.lower() or "no such container" in stderr.lower() + # Deliberate all-or-nothing: one unrecognized stderr line fails the + # whole batch instead of risking a misread error as absence. Callers + # like list_running treat this as "reconcile nothing this tick" and + # retry; do not weaken this into per-container best effort. + for line in stderr.splitlines(): + absent = {name for name in container_names if _is_no_such_container_error(line, name)} + if not absent: + raise RuntimeError(f"Failed to inspect containers with {self._runtime}: {stderr or ''}") + missing.update(absent) if not missing: - if strict: - raise RuntimeError(f"Failed to batch-inspect containers with {self._runtime} inspect: {stderr or ''}") - logger.warning( - "Failed to batch-inspect containers with %s inspect (returncode=%s, stderr=%s)", - self._runtime, - result.returncode, - stderr or "", - ) - return {} + raise RuntimeError(f"Failed to inspect containers with {self._runtime}: {stderr or ''}") try: - payload = json.loads(result.stdout or "[]") - except json.JSONDecodeError as e: + payload = json.loads(result.stdout or ("[]" if missing else "")) + except (json.JSONDecodeError, TypeError) as exc: + raise RuntimeError("Failed to parse container inspection data") from exc + if not isinstance(payload, list): + raise RuntimeError("Container inspection data must be a list") + + entries: dict[str, dict] = {} + requested = set(container_names) + for entry in payload: + if not isinstance(entry, dict): + raise RuntimeError("Invalid container inspection entry") + if self._runtime == "docker" or not runtime_identity: + raw_name = entry.get("Name") + else: + config = entry.get("configuration") + raw_name = config.get("id") if isinstance(config, dict) else None + name = raw_name.lstrip("/") if isinstance(raw_name, str) else "" + if name not in requested or name in entries or name in missing: + raise RuntimeError("Container inspection returned an unexpected or duplicate resource") + entries[name] = entry + if requested != entries.keys() | missing: + raise RuntimeError("Container inspection did not account for every requested resource") + return entries + + def _batch_inspect(self, container_names: list[str], *, strict: bool = False) -> dict[str, _ContainerInspection]: + """Batch-inspect creation, port, generation and policy metadata. + + Exact missing resources are omitted. Strict callers propagate query and + parse failures; best-effort enumeration logs them and returns no entries. + """ + try: + # Apple configuration.id establishes raw resource presence, but its + # port/policy schema is not Docker's. Preserve the legacy Name-based + # batch path rather than misclassifying those entries for teardown. + entries = self._inspect_container_entries(container_names, runtime_identity=False) + except RuntimeError as exc: if strict: - raise RuntimeError("Failed to parse container inspection data") from e - logger.warning(f"Failed to parse docker inspect output as JSON: {e}") + raise + logger.warning("Failed to batch-inspect containers: %s", exc) return {} out: dict[str, _ContainerInspection] = {} - for entry in payload: - # ``Name`` is prefixed with ``/`` in the docker inspect response - name = (entry.get("Name") or "").lstrip("/") - if not name: - continue + for name, entry in entries.items(): created_at = _parse_docker_timestamp(entry.get("Created", "")) host_port = _extract_host_port(entry, 8080) config = entry.get("Config") or {} @@ -1684,6 +1903,7 @@ class LocalContainerBackend(SandboxBackend): networks=frozenset(str(value) for value in (network_settings.get("Networks") or {})), relay_token=_extract_container_environment(config, RELAY_TOKEN_ENV), max_shell_sessions=max_shell_sessions, + container_id=entry.get("Id") if isinstance(entry.get("Id"), str) and entry.get("Id") else None, ) return out @@ -1963,6 +2183,8 @@ class LocalContainerBackend(SandboxBackend): raise except subprocess.CalledProcessError as e: logger.warning(f"Failed to stop container {container_id}: {e.stderr}") + if not _is_no_such_container_error(e.stderr or "", container_id): + raise def _is_container_running(self, container_name: str) -> bool: """Check if a named container is currently running. diff --git a/backend/packages/harness/deerflow/community/aio_sandbox/quarantine.py b/backend/packages/harness/deerflow/community/aio_sandbox/quarantine.py new file mode 100644 index 0000000000..202c7a5c8f --- /dev/null +++ b/backend/packages/harness/deerflow/community/aio_sandbox/quarantine.py @@ -0,0 +1,103 @@ +"""Non-expiring quarantine records outside sandbox-visible mounts. + +Like the acquire flock, these records live in the shared Gateway home. A lease +can expire or disappear; uncertainty about a running command cannot. Records +are bound to the runtime instance id, so a replacement can reuse the logical +sandbox id without inheriting the old instance's fence. +""" + +from __future__ import annotations + +import hashlib +import os +from pathlib import Path + +from .sandbox_info import SandboxInfo + + +class SandboxQuarantine: + def __init__(self, root: Path, scope: str): + self._root = root / self._key(scope) + + @staticmethod + def _key(value: str) -> str: + return hashlib.sha256(value.encode("utf-8")).hexdigest() + + def _directory(self, info: SandboxInfo) -> Path: + return self._root / self._key(info.sandbox_id) + + @staticmethod + def _exists(path: Path) -> bool: + # Do not collapse storage/permission failures into "no quarantine". + try: + path.stat() + except FileNotFoundError: + return False + return True + + def mark(self, info: SandboxInfo) -> None: + directory = self._directory(info) + directory.mkdir(parents=True, exist_ok=True, mode=0o700) + generation = self._key(info.container_id) if info.container_id else "unknown" + # Empty records are atomic and idempotent: a process dying during the + # write still leaves a fence, never partially parsed metadata. + with (directory / generation).open("ab") as record: + os.fsync(record.fileno()) + + def contains(self, info: SandboxInfo) -> bool: + directory = self._directory(info) + if not self._exists(directory): + return False + if not info.container_id: + # Version skew must not allow discovery to forget the generation. + return True + if self._exists(directory / "unknown") or self._exists(directory / self._key(info.container_id)): + return True + # Crash residue: mark() died between mkdir and record creation, so the + # queried generation may be the unrecorded one. Fail closed. A directory + # already holding other generations proves this is a different one. + try: + return not any(directory.iterdir()) + except FileNotFoundError: + # A concurrent retire() removed the directory between the existence + # check and the listing: the fence is gone, exactly as _exists reports. + return False + + def retire_replaced_generations(self, current: SandboxInfo) -> None: + """Drop fences a live replacement proves absent; never the live one. + + An observed runtime under the same logical ID with a *different* + generation proves the fenced generations are gone (K8s Pod names and + Docker container names are unique per logical ID). Their records are + unreferenced garbage that would otherwise keep every future create on + the inspect-and-defer path. The live generation's own record — and an + unversioned ``unknown`` crash fence — are kept: deleting those could + silently drop a fence a peer has just written. + """ + directory = self._directory(current) + if not self._exists(directory): + return + keep = self._key(current.container_id) if current.container_id else None + for record in directory.iterdir(): + if record.name == "unknown" or (keep is not None and record.name == keep): + continue + record.unlink() + try: + directory.rmdir() + except OSError: + # A live or freshly fenced generation still has its record. + pass + + def retire(self, info: SandboxInfo) -> None: + """Remove fences only after the caller proves the logical ID is absent. + + The provider must hold its local reservation and the cross-worker + teardown lease throughout that proof and this removal. A successful + delete request or failed health check alone does not prove absence. + """ + directory = self._directory(info) + if not self._exists(directory): + return + for record in directory.iterdir(): + record.unlink() + directory.rmdir() diff --git a/backend/packages/harness/deerflow/community/aio_sandbox/remote_backend.py b/backend/packages/harness/deerflow/community/aio_sandbox/remote_backend.py index 3612b05d05..03929416b0 100644 --- a/backend/packages/harness/deerflow/community/aio_sandbox/remote_backend.py +++ b/backend/packages/harness/deerflow/community/aio_sandbox/remote_backend.py @@ -24,10 +24,15 @@ from pathlib import PurePosixPath import requests from deerflow.constants import DEFAULT_SKILLS_CONTAINER_PATH +from deerflow.integrations.lark_cli import ( + PROVISIONER_CAPABILITY_REFRESH_HEADER, + PROVISIONER_CAPABILITY_REFRESH_LARK_BROKER, + invalidate_cached_lark_broker_mode, +) from deerflow.runtime.user_context import get_effective_user_id from deerflow.skills.storage import user_should_see_legacy_skills -from .backend import SandboxBackend +from .backend import SandboxBackend, SandboxCreationError from .sandbox_info import SandboxInfo logger = logging.getLogger(__name__) @@ -182,6 +187,17 @@ class RemoteSandboxBackend(SandboxBackend): except (TypeError, ValueError): return True + def _sandbox_info(self, payload: dict, sandbox_id: str) -> SandboxInfo: + broker = payload.get("lark_cli_broker") + generation = payload.get("container_id") + return SandboxInfo( + sandbox_id=sandbox_id, + sandbox_url=payload["sandbox_url"], + container_id=generation if isinstance(generation, str) and generation else None, + lark_cli_broker=broker if isinstance(broker, bool) else None, + requires_replacement=self._requires_shell_capacity_replacement(payload), + ) + # ── SandboxBackend interface ────────────────────────────────────────── def create( @@ -218,6 +234,33 @@ class RemoteSandboxBackend(SandboxBackend): """Check whether the sandbox Pod is running.""" return self._provisioner_is_alive(info.sandbox_id) + def is_absent(self, sandbox_id: str) -> bool: + """Require a Pod-presence attestation independent of Service readiness.""" + return self.inspect_runtime(sandbox_id) is None + + def inspect_runtime(self, sandbox_id: str) -> SandboxInfo | None: + """Inspect Pod identity even when its Service is missing or unready.""" + try: + response = requests.get( + f"{self._provisioner_url}/api/sandboxes/{sandbox_id}/presence", + headers=self._auth_headers(), + timeout=10, + ) + response.raise_for_status() + payload = response.json() + if not isinstance(payload, dict) or payload.get("sandbox_id") != sandbox_id or not isinstance(payload.get("exists"), bool): + raise RuntimeError(f"Provisioner did not attest Pod presence for sandbox {sandbox_id}") + generation = payload.get("container_id") + if generation is not None and (not isinstance(generation, str) or not generation): + raise RuntimeError(f"Provisioner did not attest a valid Pod generation for sandbox {sandbox_id}") + if payload["exists"] is False: + if generation is not None: + raise RuntimeError(f"Provisioner reported a generation for absent sandbox {sandbox_id}") + return None + return SandboxInfo(sandbox_id, "", container_id=generation, lark_cli_broker=None) + except (requests.RequestException, ValueError) as exc: + raise RuntimeError(f"Could not inspect runtime for sandbox {sandbox_id}: {exc}") from exc + def discover(self, sandbox_id: str) -> SandboxInfo | None: """Discover an existing sandbox via the provisioner. @@ -264,13 +307,7 @@ class RemoteSandboxBackend(SandboxBackend): sandbox_id = sandbox.get("sandbox_id") sandbox_url = sandbox.get("sandbox_url") if isinstance(sandbox_id, str) and sandbox_id and isinstance(sandbox_url, str) and sandbox_url: - infos.append( - SandboxInfo( - sandbox_id=sandbox_id, - sandbox_url=sandbox_url, - requires_replacement=self._requires_shell_capacity_replacement(sandbox), - ) - ) + infos.append(self._sandbox_info(sandbox, sandbox_id)) logger.info("Provisioner list_running: %d sandbox(es) found", len(infos)) return infos @@ -321,19 +358,46 @@ class RemoteSandboxBackend(SandboxBackend): ) resp.raise_for_status() data = resp.json() + info = self._sandbox_info(data, sandbox_id) if self._max_shell_sessions is not None and "max_shell_sessions" not in data: - raise RuntimeError("Provisioner did not report max_shell_sessions; Gateway/provisioner version skew prevents shell-capacity validation") + raise SandboxCreationError("Provisioner did not report max_shell_sessions; Gateway/provisioner version skew prevents shell-capacity validation", info=info) if self._requires_shell_capacity_replacement(data): - raise RuntimeError(f"Provisioner returned sandbox {sandbox_id} with insufficient shell-session capacity") + raise SandboxCreationError(f"Provisioner returned sandbox {sandbox_id} with insufficient shell-session capacity", info=info) + if provision_lark_cli_runtime or provision_lark_cli_broker: + if data.get("lark_cli_broker") is not provision_lark_cli_broker: + raise SandboxCreationError( + f"Provisioner returned sandbox {sandbox_id} with unverified or incompatible Lark broker mode", + info=info, + ) logger.info(f"Provisioner created sandbox {sandbox_id}: sandbox_url={data['sandbox_url']}") - return SandboxInfo( - sandbox_id=sandbox_id, - sandbox_url=data["sandbox_url"], - ) + return info except requests.RequestException as exc: + self._drop_cached_lark_broker_mode_on_conflict(exc) logger.error(f"Provisioner create failed for {sandbox_id}: {exc}") raise RuntimeError(f"Provisioner create failed: {exc}") from exc + def _drop_cached_lark_broker_mode_on_conflict(self, exc: requests.RequestException) -> None: + """Honor the provisioner's capability-refresh marker on create conflicts. + + A 409/503 carrying the marker means the requested lark provisioning mode + conflicts with the provisioner's *current* configuration (e.g. the broker + image was added or removed after this process cached its capability + observation). Dropping that observation makes the next acquire re-probe + instead of repeating the same failure until the cache TTL — up to 300s + for a confirmed non-broker — expires. Coordination 409s (capacity, + incompatible existing Pod, still terminating) carry no marker and leave + the cache untouched. + """ + response = getattr(exc, "response", None) + if response is None: + return + try: + marked = response.headers.get(PROVISIONER_CAPABILITY_REFRESH_HEADER) == PROVISIONER_CAPABILITY_REFRESH_LARK_BROKER + except Exception: + return + if marked: + invalidate_cached_lark_broker_mode(self._provisioner_url, self._api_key) + def _provisioner_destroy(self, sandbox_id: str) -> None: """DELETE /api/sandboxes/{sandbox_id} → destroy Pod + Service.""" try: @@ -342,12 +406,11 @@ class RemoteSandboxBackend(SandboxBackend): headers=self._auth_headers(), timeout=15, ) - if resp.ok: - logger.info(f"Provisioner destroyed sandbox {sandbox_id}") - else: - logger.warning(f"Provisioner destroy returned {resp.status_code}: {resp.text}") + if resp.status_code != 404: + resp.raise_for_status() + logger.info(f"Provisioner destroyed sandbox {sandbox_id}") except requests.RequestException as exc: - logger.warning(f"Provisioner destroy failed for {sandbox_id}: {exc}") + raise RuntimeError(f"Provisioner destroy failed for {sandbox_id}: {exc}") from exc def _provisioner_is_alive(self, sandbox_id: str) -> bool: """GET /api/sandboxes/{sandbox_id} → check Pod phase.""" @@ -380,11 +443,7 @@ class RemoteSandboxBackend(SandboxBackend): return None resp.raise_for_status() data = resp.json() - return SandboxInfo( - sandbox_id=sandbox_id, - sandbox_url=data["sandbox_url"], - requires_replacement=self._requires_shell_capacity_replacement(data), - ) + return self._sandbox_info(data, sandbox_id) except requests.RequestException as exc: logger.debug(f"Provisioner discover failed for {sandbox_id}: {exc}") return None diff --git a/backend/packages/harness/deerflow/community/aio_sandbox/sandbox_info.py b/backend/packages/harness/deerflow/community/aio_sandbox/sandbox_info.py index c5e1ffb039..c64a750db4 100644 --- a/backend/packages/harness/deerflow/community/aio_sandbox/sandbox_info.py +++ b/backend/packages/harness/deerflow/community/aio_sandbox/sandbox_info.py @@ -18,7 +18,7 @@ class SandboxInfo: sandbox_id: str sandbox_url: str # e.g. http://localhost:8080 or http://k3s:30001 container_name: str | None = None # Only for local container backend - container_id: str | None = None # Only for local container backend + container_id: str | None = None # Runtime generation: container ID or Pod UID created_at: float = field(default_factory=time.time) # Ephemeral control-plane credentials reconstructed from local Docker # discovery. Intentionally excluded from to_dict() and repr so they cannot @@ -30,6 +30,8 @@ class SandboxInfo: # provider consumes this flag and performs replacement only after obtaining # its local teardown reservation and cross-instance teardown lease. requires_replacement: bool = field(default=False, repr=False, compare=False) + # None means an older remote provisioner did not attest its actual mode. + lark_cli_broker: bool | None = False def to_dict(self) -> dict: return { @@ -38,6 +40,7 @@ class SandboxInfo: "container_name": self.container_name, "container_id": self.container_id, "created_at": self.created_at, + "lark_cli_broker": self.lark_cli_broker, } @classmethod @@ -48,4 +51,5 @@ class SandboxInfo: container_name=data.get("container_name"), container_id=data.get("container_id"), created_at=data.get("created_at", time.time()), + lark_cli_broker=data.get("lark_cli_broker"), # Missing attestation stays unverified (None), never silently attested non-broker. ) diff --git a/backend/packages/harness/deerflow/integrations/lark_cli.py b/backend/packages/harness/deerflow/integrations/lark_cli.py index 721c65fa50..c6a9dd4e20 100644 --- a/backend/packages/harness/deerflow/integrations/lark_cli.py +++ b/backend/packages/harness/deerflow/integrations/lark_cli.py @@ -64,6 +64,7 @@ import urllib.parse import urllib.request import zipfile from collections.abc import Callable, Iterator +from concurrent.futures import Future from contextlib import contextmanager from ctypes import wintypes from dataclasses import dataclass @@ -1666,59 +1667,154 @@ def _resolve_sandbox_runtime_readiness( return "gateway-download", True, None, True +# Machine-readable marker the provisioner attaches to a lark +# provisioning-config conflict (409/503) so the Gateway drops its cached +# broker capability observation. Keep in sync with docker/provisioner/app.py. +PROVISIONER_CAPABILITY_REFRESH_HEADER = "X-DeerFlow-Capability-Refresh" +PROVISIONER_CAPABILITY_REFRESH_LARK_BROKER = "lark-broker" LARK_BROKER_MODE_TTL_SECONDS = 60 # Negative results (broker not active) are cached longer than positive ones: a # non-broker remote-provisioner deployment stays non-broker for the life of the # process far more often than it flips on, so this keeps the hot bash path from # re-probing every minute. A positive result still refreshes on the shorter TTL. LARK_BROKER_MODE_NEGATIVE_TTL_SECONDS = 300 -# Tight probe budget on the per-bash-call hot path: unlike the Settings status -# probe (5s, user is waiting on a page), this runs inline before a sandbox -# lark-cli command, so a slow/unreachable provisioner must not add seconds of -# latency to every first-call-per-TTL for non-broker deployments. +# Tight admission probe budget, separate from the Settings status probe (5s). +# Command execution uses the sandbox's attested mode and never probes here. LARK_BROKER_MODE_PROBE_TIMEOUT_SECONDS = 1.5 -# Guards the cache attribute on sandbox_lark_broker_active against concurrent -# bash invocations so the correctness story doesn't rely on idempotent races. +LARK_BROKER_MODE_PROBE_RETRY_SECONDS = 1.5 +# Only cache/flight bookkeeping holds this lock; HTTP and waiting never do. _LARK_BROKER_MODE_CACHE_LOCK = threading.Lock() +_LARK_BROKER_MODE_PROBES: dict[tuple[str, str], Future[bool]] = {} -def sandbox_lark_broker_active(config: AppConfig | None = None) -> bool: +@dataclass(frozen=True) +class _LarkBrokerModeCacheEntry: + active: bool | None + confirmed_at: float + retry_after: float = 0.0 + + +class LarkBrokerCapabilityUnknownError(RuntimeError): + """The provisioner's broker capability could not be confirmed. + + Raised instead of guessing: only a confirmed observation may select + credential placement. Callers holding a stronger, Pod-level attestation may + catch this and defer the deployment-level decision; everything else fails + closed. + """ + + +def _known_broker_requirement(active: bool | None) -> bool: + if active is True: + return True + raise LarkBrokerCapabilityUnknownError("Provisioner Lark broker capability is unknown; refusing to mount credentials in the sandbox") + + +def _fresh_broker_observation(cached: _LarkBrokerModeCacheEntry, now: float, observed_mode: bool | None) -> bool | None: + """Return the still-trustworthy cached observation, or None when a new probe is required. + + A contradictory ``observed_mode`` never receives the cached value: the + in-flight or next probe must confirm which mode the provisioner actually runs. + """ + if now < cached.retry_after: + return _known_broker_requirement(cached.active) + ttl = LARK_BROKER_MODE_TTL_SECONDS if cached.active else LARK_BROKER_MODE_NEGATIVE_TTL_SECONDS + if not cached.retry_after and now - cached.confirmed_at < ttl and (observed_mode is None or observed_mode is cached.active): + return cached.active + return None + + +def sandbox_lark_broker_active(config: AppConfig | None = None, *, observed_mode: bool | None = None) -> bool: """Whether sandbox ``lark-cli`` runs in broker mode (Pattern B). - Cached with a short TTL because it is consulted on every ``lark-cli`` bash - call and reads the provisioner capability over HTTP. Broker mode requires a - remote provisioner that reports a configured broker image; any other config - (local AIO, init-container binary mode, unreachable provisioner) is False, so - the caller falls back to the credential-mount overlay. - - The probe uses a tight timeout and negatives are cached longer than positives - so a non-broker remote-provisioner deployment does not pay a latency penalty - on the bash hot path. + Only confirmed capabilities may select credential placement. An unavailable + or malformed response is unknown, never a negative observation. Keep a known + broker requirement through outages; without one, fail closed. A confirmed + non-broker observation is likewise only trusted inside its TTL: once it goes + stale, or while a refresh is backing off after a probe failure, admission + raises instead of trusting a negative that would mount credentials on a pod + whose mode is no longer verified. Execution uses the mode reported for the + actual sandbox, rather than probing again. An attested ``observed_mode`` + contradicting a cached observation forces one shared refresh before + admission decides whether the Pod needs replacement. """ if config is None: - try: - from deerflow.config.app_config import get_app_config + from deerflow.config.app_config import get_app_config - config = get_app_config() - except Exception: # noqa: BLE001 - degrade to non-broker overlay - return False + config = get_app_config() - now = time.monotonic() + if not (_uses_aio_sandbox(config) and _uses_remote_provisioner(config)): + return False + + cache_key = (_sandbox_config_value(config, "provisioner_url").rstrip("/"), _sandbox_config_value(config, "provisioner_api_key")) + probe_owner = False with _LARK_BROKER_MODE_CACHE_LOCK: - cached = getattr(sandbox_lark_broker_active, "_cache", None) + cache = getattr(sandbox_lark_broker_active, "_cache", {}) + sandbox_lark_broker_active._cache = cache # type: ignore[attr-defined] + cached = cache.get(cache_key) + flight = _LARK_BROKER_MODE_PROBES.get(cache_key) + if flight is None: + now = time.monotonic() + if cached is not None: + fresh = _fresh_broker_observation(cached, now, observed_mode) + if fresh is not None: + return fresh + flight = Future() + _LARK_BROKER_MODE_PROBES[cache_key] = flight + probe_owner = True + + if not probe_owner: if cached is not None: - ts, value = cached - ttl = LARK_BROKER_MODE_TTL_SECONDS if value else LARK_BROKER_MODE_NEGATIVE_TTL_SECONDS - if now - ts < ttl: - return value + # A probe another caller started does not invalidate this caller's + # still-fresh observation; only a contradictory attestation waits. + fresh = _fresh_broker_observation(cached, time.monotonic(), observed_mode) + if fresh is not None: + return fresh + return flight.result() - active = False - if _uses_aio_sandbox(config) and _uses_remote_provisioner(config): + try: + # One flight per endpoint/auth identity shares success AND failure. + # It also orders observations without blocking unrelated endpoints. caps = _probe_provisioner_capabilities(config, timeout=LARK_BROKER_MODE_PROBE_TIMEOUT_SECONDS) - active = bool(caps and caps["lark_cli_broker_image"]) + active = caps.get("lark_cli_broker_image") if isinstance(caps, dict) else None + now = time.monotonic() + if isinstance(active, bool): + entry = _LarkBrokerModeCacheEntry(active, now) + else: + # Backoff is separate from confirmation freshness: failure never + # authorizes False or extends a previous observation's TTL. + entry = _LarkBrokerModeCacheEntry( + cached.active if cached is not None else None, + cached.confirmed_at if cached is not None else 0.0, + retry_after=now + LARK_BROKER_MODE_PROBE_RETRY_SECONDS, + ) + with _LARK_BROKER_MODE_CACHE_LOCK: + cache[cache_key] = entry + flight.set_result(active if isinstance(active, bool) else _known_broker_requirement(entry.active)) + except BaseException as exc: + # Publish every outcome so waiters cannot be stranded by an exception. + flight.set_exception(exc) + finally: + with _LARK_BROKER_MODE_CACHE_LOCK: + _LARK_BROKER_MODE_PROBES.pop(cache_key, None) + return flight.result() + + +def invalidate_cached_lark_broker_mode(provisioner_url: str, api_key: str | None = None) -> None: + """Drop the cached broker-mode observation for one provisioner identity. + + The provisioner marks a lark provisioning-config conflict (409/503) with + ``PROVISIONER_CAPABILITY_REFRESH_HEADER`` so a deployment change (e.g. the + broker image being added or removed) takes effect on the next acquire + instead of living out the cache TTL — up to 300s for a confirmed + non-broker. Only the cached observation is dropped; an in-flight probe is + left alone and repopulates the cache with a fresh observation. + """ + key = (provisioner_url.rstrip("/"), (api_key or "").strip()) with _LARK_BROKER_MODE_CACHE_LOCK: - sandbox_lark_broker_active._cache = (now, active) # type: ignore[attr-defined] - return active + cache = getattr(sandbox_lark_broker_active, "_cache", None) + if cache: + cache.pop(key, None) @dataclass(frozen=True) @@ -1964,9 +2060,8 @@ def _probe_provisioner_capabilities(config: AppConfig, *, timeout: float = 5.0) Returns the capability dict when the provisioner answers, or None when it can't be reached. Used both for the status readiness signal and to select - broker vs. binary mode on the bash hot path; failures degrade to "not - ready"/"not broker" rather than raising. ``timeout`` is caller-tunable so the - per-bash-call probe can use a tighter budget than the Settings status probe. + broker vs. binary mode when provisioning. Failures mean unknown; status + callers can report "not ready", while provisioning must fail closed. """ base = _sandbox_config_value(config, "provisioner_url") if not base: @@ -1978,11 +2073,11 @@ def _probe_provisioner_capabilities(config: AppConfig, *, timeout: float = 5.0) request = urllib.request.Request(url, headers={"User-Agent": "deer-flow", **headers}) with urllib.request.urlopen(request, timeout=timeout) as response: payload = json.loads(response.read().decode("utf-8")) - if not isinstance(payload, dict): + if not isinstance(payload, dict) or any(not isinstance(payload.get(key), bool) for key in ("lark_cli_init_image", "lark_cli_broker_image")): return None return { - "lark_cli_init_image": bool(payload.get("lark_cli_init_image")), - "lark_cli_broker_image": bool(payload.get("lark_cli_broker_image")), + "lark_cli_init_image": payload["lark_cli_init_image"], + "lark_cli_broker_image": payload["lark_cli_broker_image"], } except Exception: return None diff --git a/backend/packages/harness/deerflow/sandbox/AGENTS.md b/backend/packages/harness/deerflow/sandbox/AGENTS.md index 3e59b43a60..60e541bef1 100644 --- a/backend/packages/harness/deerflow/sandbox/AGENTS.md +++ b/backend/packages/harness/deerflow/sandbox/AGENTS.md @@ -15,6 +15,8 @@ cancellation before propagating it. **Execution leases** (`sandbox/lease.py`, #5128): cross-instance ownership selects which Gateway may reap a container; process-local `SandboxLeaseManager` tracks a client's concurrent lead, subagent, Gateway-request, and channel-upload users. Runs have ephemeral owners. Restore persisted sandboxes only after `get_scoped(user_id, thread_id)` succeeds; otherwise canonical `acquire` replaces the id. Last holder performs pending `SandboxProvider.release`. External run/state boundaries reject server-owned `sandbox`. Outer lifecycle fences repeat idempotent release after each graph/tool/request batch; per-tool terminal `Command` wrappers never release while siblings run. Fork-restored children and upload syncs hold without requesting release: server-created forks may borrow the parent client; ordinary checkpoint ids stay identity-scoped. Serialize persisted lookup and retention by `(user_id, thread_id)`; stale or mismatched bindings reacquire, and a post-acquire miss rolls back then raises. Provider I/O runs outside the metadata lock. Repeated cancellation cannot interrupt acquire/rollback/release reconciliation or let `to_thread` work outlive its execution/request holder; failures are logged without replacing the original cancellation. Lease/scope context IDs are server-owned: Gateway/worker scrub caller values; only internal subagents assign task IDs. Managers key by provider object identity (not hash/equality), so unhashable custom providers work. Subagent owners double as `sandbox_command_scope_id`: AIO gives each scope an ordered persistent shell, replaces it after `ErrorObservation` or a missing-session 404, and cleans it on lease release. Revalidate registry identity after each scope-lock wait so queued commands cannot resurrect released sessions. Env-bearing commands use fresh `bash.exec` sessions so secrets do not persist. **Authorization gate** (`sandbox:execute`, RFC #4063 Phase 3): every sandbox-backed tool call passes through the gate in `deerflow/authz/sandbox_authz.py` - a binary `authorize(principal, "sandbox", "execute", target="*")` check before either reusing a persisted sandbox id or calling `provider.acquire`. Rechecking reuse is required because authorization config and user roles can change while the sandbox remains cached. Sync tool invocations call `authorize_sandbox_execution`; async tool invocations await `authorize_sandbox_execution_async` exactly once. A task-local `ContextVar` scopes that single decision across the complete composed tool invocation, including `ReadBeforeWriteMiddleware`'s pre-write inspection, tool body, and post-read mark; the value is copied into `asyncio.to_thread` workers. Authorization denial is converted to the normal error `ToolMessage` at the composed middleware boundary and is explicitly excluded from the gate's generic fail-open handlers. Async config loading and provider class discovery/import are offloaded before `aauthorize()` so reused sandbox calls do not hash config files or import custom modules on the event loop; provider construction remains on the running event loop because async providers may initialize loop-affine clients. The gate lives at the single tool initialization entry point (`ensure_sandbox_initialized` / `ensure_sandbox_initialized_async` in `tools.py`), while `SandboxMiddleware.before_agent` / `abefore_agent` apply the matching sync/async check to eager acquisition. Deny raises `SandboxAuthorizationError` (`sandbox/exceptions.py`), which propagates out of ordinary tool execution as a friendly error `ToolMessage` ("sandbox execution is not permitted for your role") - the eager path catches it and skips acquisition instead, deferring the deny to the first sandbox-touching tool call so both paths share the same semantics. Provider errors (authorization calls and provider resolution) follow `authorization.fail_closed` / `fail_open`; no readable `config.yaml` or `authorization.enabled: false` makes the gate a no-op (`safe_app_config` tolerates missing config). Gateway upload/artifact sync calls `try_acquire_sandbox_for_request` (`app/gateway/authz.py`), which gates, returns a request lease, and skips sync on deny while preserving the primary operation. Callers release after their last sandbox operation; artifacts request normal parking, uploads do not. Tests: `tests/test_sandbox_authorization.py` and `tests/blocking_io/test_sandbox_authorization.py`. **Environment policy** (`sandbox/env_policy.py`): `build_sandbox_env()` scrubs inherited credential names via `is_blocked_env_name`, preserving benign vars (`PATH`, `HOME`, `LANG`, `VIRTUAL_ENV`). Explicit `sandbox.environment` injection survives scrubbing; request secrets override it. Local sandboxes validate keys at construction; generic and per-thread instances receive resolved config values verbatim. Local `bash` masks effective operator credentials and request secrets in output and command errors, preserving benign config values. +Holders stay bound through scope cleanup; AIO reserves idle teardown atomically. +Fork lookups reject invalidated clients; cancelled acquires leave borrowers a pending release. **Implementations**: - `LocalSandboxProvider` - Local filesystem execution. `acquire(thread_id)` returns a per-user/thread `LocalSandbox` (id `local:{user_id}:{thread_id}`) whose `path_mappings` resolve `/mnt/user-data/{workspace,uploads,outputs}` and `/mnt/acp-workspace` to that thread's host directories, so the public `Sandbox` API honours the `/mnt/user-data` contract uniformly with AIO. `acquire()` / `acquire(None)` keeps the legacy generic singleton (id `local`) for callers without a thread context. Per-thread sandboxes are held in an LRU cache (default 256 entries) guarded by a `threading.Lock`. Shared runs use category mappings; a policy-scoped run replaces them with one `/mnt/skills` root mapping to the coherent thread view, so structured file tools resolve through one managed boundary. This is not a host filesystem security boundary: an enabled host `bash` subprocess can use canonical paths without `PathMapping`, so `supports_agent_skill_isolation` is dynamic and explicit Agent policies fail closed while host bash is enabled; unrestricted runs then get the shared mappings, never a leftover thread view. Host-to-virtual output masking scans dynamic per-user/per-thread roots directly instead of compiling path-specific regexes, so evicted thread IDs do not remain in Python's global regex caches; a separate 256-entry root cache prevents repeated `realpath()` walks for every glob/grep match while bounding dynamic-path retention, and only the small process-stable skill/integration source set uses a bounded compiled cache. The shared `path_patterns.py` tail stops at `:`, so `$PATH`-style lists mask every entry; a mount symlink resolving outside all mounts keeps its mount path. On Windows, Git Bash/MSYS argument-conversion exclusions are limited to safe non-root virtual path prefixes; do not restore a blanket conversion disable, because host-native CLI launchers need normal MSYS path conversion for their own installation paths. PowerShell fallback pairs guarded console UTF-8 setters with explicit UTF-8 pipe decoding; cmd/MSYS retain locale decoding. Encoding regressions include real-pipe probes and Windows-only PowerShell roundtrips with and without a console. diff --git a/backend/packages/harness/deerflow/sandbox/lease.py b/backend/packages/harness/deerflow/sandbox/lease.py index 15170def15..8024f19924 100644 --- a/backend/packages/harness/deerflow/sandbox/lease.py +++ b/backend/packages/harness/deerflow/sandbox/lease.py @@ -13,11 +13,13 @@ import asyncio import logging import threading import uuid -from collections.abc import Callable +from collections.abc import Callable, Iterator +from contextlib import contextmanager from dataclasses import dataclass from typing import TYPE_CHECKING, Any from deerflow.sandbox.acquire_serialization import AcquireSerializer +from deerflow.sandbox.exceptions import SandboxNotFoundError if TYPE_CHECKING: from deerflow.sandbox.sandbox_provider import SandboxProvider @@ -140,6 +142,31 @@ class _LeaseBinding: release_on_last: bool = True +@dataclass(slots=True) +class _ClientLifecycleObservation: + revision: int = 0 + readers: int = 0 + + +class SandboxClientTeardownInProgressError(RuntimeError): + """A new holder must wait until the reserved client lifecycle finishes.""" + + def __init__(self, sandbox_id: str): + super().__init__(f"sandbox {sandbox_id} is reserved for teardown; retry after cleanup completes") + self.sandbox_id = sandbox_id + + +class SandboxClientUnavailableError(SandboxNotFoundError): + """Admission failed without a new holder; its acquire was reconciled. + + Existing borrowers can still defer provider release. This subtype lets + request callers distinguish that reconciled failure from provider errors. + """ + + def __init__(self, sandbox_id: str): + super().__init__("Sandbox not found after acquisition", sandbox_id=sandbox_id) + + class SandboxLeaseManager: """Coordinate active agent users of one sandbox provider. @@ -157,6 +184,8 @@ class SandboxLeaseManager: self._bindings_by_owner: dict[str, _LeaseBinding] = {} self._owners_by_sandbox: dict[str, set[str]] = {} self._release_pending_by_sandbox: set[str] = set() + self._teardowns_by_sandbox: dict[str, tuple[int, int]] = {} + self._client_lifecycle_observations: dict[str, _ClientLifecycleObservation] = {} @staticmethod def _thread_key(thread_id: str, user_id: str) -> tuple[str, str]: @@ -180,14 +209,12 @@ class SandboxLeaseManager: owners = self._owners_by_sandbox.get(binding.sandbox_id) if owners is None: release_provider = binding.sandbox_id in self._release_pending_by_sandbox - self._release_pending_by_sandbox.discard(binding.sandbox_id) return binding, release_provider owners.discard(owner_id) if owners: return binding, False self._owners_by_sandbox.pop(binding.sandbox_id, None) release_provider = binding.sandbox_id in self._release_pending_by_sandbox - self._release_pending_by_sandbox.discard(binding.sandbox_id) return binding, release_provider def _bind_locked( @@ -198,6 +225,8 @@ class SandboxLeaseManager: *, release_on_last: bool, ) -> tuple[_LeaseBinding | None, bool]: + if sandbox_id in self._teardowns_by_sandbox: + raise SandboxClientTeardownInProgressError(sandbox_id) existing = self._bindings_by_owner.get(owner_id) if existing is not None and existing.thread_key != key: raise RuntimeError(f"Sandbox lease owner {owner_id!r} cannot move between thread identities") @@ -237,8 +266,81 @@ class SandboxLeaseManager: them. """ with self._metadata_lock: - has_owners = bool(self._owners_by_sandbox.get(sandbox_id)) - if not has_owners: + # An unbound/cancelled acquire still owns cleanup responsibility. + # A fork may already have borrowed its client with release_on_last + # disabled; preserve the request until that borrower also drains. + self._release_pending_by_sandbox.add(sandbox_id) + self._release_if_idle(sandbox_id) + + @contextmanager + def reserve_idle_teardown(self, sandbox_id: str) -> Iterator[bool]: + """Atomically exclude holders without holding metadata locks over IO. + + New bindings, including forks with different thread keys, are rejected + until the reservation exits. Synchronous provider cleanup can nest on + the same worker thread; another worker must defer instead of waiting. + Never acquire the per-thread serializer from this provider-facing hook. + """ + worker = threading.get_ident() + with self._metadata_lock: + current = self._teardowns_by_sandbox.get(sandbox_id) + granted = not self._owners_by_sandbox.get(sandbox_id) and (current is None or current[0] == worker) + if granted: + if current is None and (observation := self._client_lifecycle_observations.get(sandbox_id)) is not None: + observation.revision += 1 + self._teardowns_by_sandbox[sandbox_id] = (worker, current[1] + 1 if current else 1) + try: + yield bool(granted) + finally: + if granted: + with self._metadata_lock: + _, depth = self._teardowns_by_sandbox[sandbox_id] + if depth == 1: + del self._teardowns_by_sandbox[sandbox_id] + else: + self._teardowns_by_sandbox[sandbox_id] = (worker, depth - 1) + + @contextmanager + def _observe_client_lifecycle(self, sandbox_id: str) -> Iterator[tuple[_ClientLifecycleObservation, int]]: + """Keep teardown history only while a client lookup is in flight.""" + with self._metadata_lock: + if sandbox_id in self._teardowns_by_sandbox: + raise SandboxClientTeardownInProgressError(sandbox_id) + observation = self._client_lifecycle_observations.setdefault(sandbox_id, _ClientLifecycleObservation()) + observation.readers += 1 + revision = observation.revision + try: + yield observation, revision + finally: + with self._metadata_lock: + observation.readers -= 1 + if observation.readers == 0: + del self._client_lifecycle_observations[sandbox_id] + + def _bind_live_client( + self, + owner_id: str, + sandbox_id: str, + key: tuple[str, str], + *, + release_on_last: bool, + allow_unscoped_borrow: bool, + ) -> tuple[_LeaseBinding | None, bool] | None: + with self._observe_client_lifecycle(sandbox_id) as (observation, revision): + sandbox = self._provider.get(sandbox_id) if allow_unscoped_borrow else self._provider.get_scoped(sandbox_id, thread_id=key[1], user_id=key[0]) + with self._metadata_lock: + # A fork's different serializer cannot protect its earlier + # lookup from a parent finishing teardown before this bind. + if sandbox is None or observation.revision != revision: + return None + return self._bind_locked(owner_id, sandbox_id, key, release_on_last=release_on_last) + + def _release_if_idle(self, sandbox_id: str) -> None: + with self.reserve_idle_teardown(sandbox_id) as reserved: + if not reserved: + return + with self._metadata_lock: + self._release_pending_by_sandbox.discard(sandbox_id) self._provider.release(sandbox_id) def _active_owner_binding( @@ -286,15 +388,18 @@ class SandboxLeaseManager: release_on_last: bool, ) -> str: sandbox_id = self._provider.acquire(thread_id, user_id=user_id) - with self._metadata_lock: - previous, release_previous = self._bind_locked( - owner_id, - sandbox_id, - key, - release_on_last=release_on_last, - ) + try: + # Observe after acquire: its own inline replacement may legitimately + # have torn down the old generation with this same logical id. + binding = self._bind_live_client(owner_id, sandbox_id, key, release_on_last=release_on_last, allow_unscoped_borrow=True) + if binding is None: + raise SandboxClientUnavailableError(sandbox_id) + previous, release_previous = binding + except BaseException: + self._release_unbound_acquire(sandbox_id) + raise if release_previous and previous is not None: - self._provider.release(previous.sandbox_id) + self._release_if_idle(previous.sandbox_id) return sandbox_id async def _acquire_and_bind_async( @@ -335,16 +440,17 @@ class SandboxLeaseManager: exc_info=True, ) raise cancellation - with self._metadata_lock: - previous, release_previous = self._bind_locked( - owner_id, - sandbox_id, - key, - release_on_last=release_on_last, - ) + try: + binding = self._bind_live_client(owner_id, sandbox_id, key, release_on_last=release_on_last, allow_unscoped_borrow=True) + if binding is None: + raise SandboxClientUnavailableError(sandbox_id) + previous, release_previous = binding + except BaseException: + await run_sync_lifecycle_operation(self._release_unbound_acquire, sandbox_id) + raise if release_previous and previous is not None: await run_sync_lifecycle_operation( - self._provider.release, + self._release_if_idle, previous.sandbox_id, ) return sandbox_id @@ -374,7 +480,7 @@ class SandboxLeaseManager: release_on_last=release_on_last, ) if release_previous and previous is not None: - self._provider.release(previous.sandbox_id) + self._release_if_idle(previous.sandbox_id) async def retain_async( self, @@ -397,7 +503,7 @@ class SandboxLeaseManager: ) if release_previous and previous is not None: await run_sync_lifecycle_operation( - self._provider.release, + self._release_if_idle, previous.sandbox_id, ) @@ -456,25 +562,11 @@ class SandboxLeaseManager: if existing_sandbox_id is not None: return existing_sandbox_id - sandbox = ( - self._provider.get(sandbox_id) - if allow_unscoped_borrow - else self._provider.get_scoped( - sandbox_id, - thread_id=thread_id, - user_id=user_id, - ) - ) - if sandbox is not None: - with self._metadata_lock: - previous, release_previous = self._bind_locked( - owner_id, - sandbox_id, - key, - release_on_last=release_on_last, - ) + binding = self._bind_live_client(owner_id, sandbox_id, key, release_on_last=release_on_last, allow_unscoped_borrow=allow_unscoped_borrow) + if binding is not None: + previous, release_previous = binding if release_previous and previous is not None: - self._provider.release(previous.sandbox_id) + self._release_if_idle(previous.sandbox_id) return sandbox_id return self._acquire_and_bind( @@ -533,26 +625,12 @@ class SandboxLeaseManager: if existing_sandbox_id is not None: return existing_sandbox_id - sandbox = ( - self._provider.get(sandbox_id) - if allow_unscoped_borrow - else self._provider.get_scoped( - sandbox_id, - thread_id=thread_id, - user_id=user_id, - ) - ) - if sandbox is not None: - with self._metadata_lock: - previous, release_previous = self._bind_locked( - owner_id, - sandbox_id, - key, - release_on_last=release_on_last, - ) + binding = self._bind_live_client(owner_id, sandbox_id, key, release_on_last=release_on_last, allow_unscoped_borrow=allow_unscoped_borrow) + if binding is not None: + previous, release_previous = binding if release_previous and previous is not None: await run_sync_lifecycle_operation( - self._provider.release, + self._release_if_idle, previous.sandbox_id, ) return sandbox_id @@ -577,16 +655,19 @@ class SandboxLeaseManager: current = self._bindings_by_owner.get(owner_id) if current is None: return - binding, release_provider = self._remove_binding_locked(owner_id) - assert binding is not None - + binding = current try: sandbox = self._provider.get(binding.sandbox_id) if sandbox is not None: sandbox.release_command_scope(owner_id) finally: + # Scope cleanup still uses the client: keep the holder until + # it drains, then atomically decide whether parking is pending. + with self._metadata_lock: + binding, release_provider = self._remove_binding_locked(owner_id) if release_provider: - self._provider.release(binding.sandbox_id) + assert binding is not None + self._release_if_idle(binding.sandbox_id) async def release_async(self, owner_id: str) -> None: """Release a lease without blocking the caller's event loop.""" diff --git a/backend/packages/harness/deerflow/sandbox/sandbox.py b/backend/packages/harness/deerflow/sandbox/sandbox.py index c94ad3d037..8de65c52ba 100644 --- a/backend/packages/harness/deerflow/sandbox/sandbox.py +++ b/backend/packages/harness/deerflow/sandbox/sandbox.py @@ -63,6 +63,20 @@ class Sandbox(ABC): #: ``False``). persistent_shell_sessions: bool | None = None + #: Whether sandbox ``lark-cli`` runs through a credential-owning broker + #: sidecar (Pattern B, issue #4338): ``True`` once the provider has + #: attested broker mode for this sandbox, ``False`` for the plaintext + #: config/data overlay. Tri-state, failing closed: ``None`` means the + #: mode is unverified and ``lark-cli`` execution must refuse to run until + #: the mode is attested. The default is ``False`` rather than ``None`` + #: because most providers have no broker sidecar, no credential mounts, + #: and no provisioner to attest a mode — the non-broker overlay is the + #: only mode that exists for them. Only the AIO sandbox overrides the + #: default with ``None``: its mode is attested per Pod by the + #: provisioner, and an unattested Pod must fail closed instead of + #: silently mounting credentials. + lark_cli_broker: bool | None = False + def __init__(self, id: str): self._id = id diff --git a/backend/packages/harness/deerflow/sandbox/tools.py b/backend/packages/harness/deerflow/sandbox/tools.py index a5eee65d29..6e007a79b4 100644 --- a/backend/packages/harness/deerflow/sandbox/tools.py +++ b/backend/packages/harness/deerflow/sandbox/tools.py @@ -2352,7 +2352,7 @@ def _github_env_from_runtime(runtime: Runtime) -> dict[str, str] | None: _LARK_CLI_COMMAND_RE = re.compile(r"(? dict[str, str] | None: +def _lark_cli_env_from_runtime(runtime: Runtime, command: str, *, sandbox_paths: bool, sandbox: Sandbox | None = None) -> dict[str, str] | None: """Expose Settings-page Lark auth to sandbox ``lark-cli`` commands. Settings authorizes ``lark-cli`` under DeerFlow's per-user integration @@ -2368,14 +2368,12 @@ def _lark_cli_env_from_runtime(runtime: Runtime, command: str, *, sandbox_paths: """ if not _LARK_CLI_COMMAND_RE.search(command): return None - try: - from deerflow.integrations.lark_cli import lark_cli_env_overlay, sandbox_lark_broker_active + from deerflow.integrations.lark_cli import lark_cli_env_overlay - broker = sandbox_paths and sandbox_lark_broker_active() - return lark_cli_env_overlay(resolve_runtime_user_id(runtime), sandbox_paths=sandbox_paths, broker=broker) - except Exception: - logger.warning("Could not build Lark CLI env overlay; running command without managed auth", exc_info=True) - return None + broker = getattr(sandbox, "lark_cli_broker", None) if sandbox_paths else False + if broker is None: + raise RuntimeError("Sandbox Lark broker mode is unverified; refusing to execute lark-cli") + return lark_cli_env_overlay(resolve_runtime_user_id(runtime), sandbox_paths=sandbox_paths, broker=broker) @tool("bash", parse_docstring=True) @@ -2412,7 +2410,7 @@ def bash_tool(runtime: Runtime, command: str, description: str = "") -> str: identity_prefix = _channel_identity_prefix(runtime) user_prefix = _user_identity_prefix(runtime) github_env = _github_env_from_runtime(runtime) - lark_cli_env = _lark_cli_env_from_runtime(runtime, command, sandbox_paths=not is_local_sandbox(runtime)) + lark_cli_env = _lark_cli_env_from_runtime(runtime, command, sandbox_paths=not is_local_sandbox(runtime), sandbox=sandbox) if github_env: injected_env = {**(injected_env or {}), **github_env} if lark_cli_env: diff --git a/backend/packages/harness/deerflow/skills/AGENTS.md b/backend/packages/harness/deerflow/skills/AGENTS.md index 65671347e8..80c0370be3 100644 --- a/backend/packages/harness/deerflow/skills/AGENTS.md +++ b/backend/packages/harness/deerflow/skills/AGENTS.md @@ -18,7 +18,7 @@ - `skills/describe.py` — `build_describe_skill_tool(catalog)` builds the `describe_skill` tool as a closure; `build_skill_search_setup(skills, enabled, ...)` produces a `SkillSearchSetup(describe_skill_tool, skill_names)` that is wired into both the LangGraph agent factory (`agent.py`) and the embedded client (`client.py`). - **Slash activation**: `/skill-name task` loads that enabled skill's `SKILL.md` for the current model call only. The resolver rejects leading whitespace, missing separators, reserved command syntax (`/new`, `/help`, `/bootstrap`, `/status`, `/models`, `/memory`, `/goal`, `/agent`, and the exact composer-only alias `/context compact`), disabled skills, and skills outside a custom agent's whitelist. A custom skill named `context` remains activatable for other task text. - **Installation**: `POST /api/skills/install` extracts .skill ZIP archive to custom/ directory -- **Managed integrations**: Lark/Feishu CLI support installs one global official `lark-*` pack as read-only `SkillCategory.INTEGRATION` entries under `/mnt/skills/integrations/lark-cli/...`; enabled flags, app configuration, and OAuth data remain per-user. Install resolves the newest `larksuite/cli` release from GitHub (`releases/latest`) at install time (falling back to a bottom-line pinned version if the lookup fails) rather than hard-coding the pack version; integrity relies on the official host + structural archive guards + a recorded hash of the effective installed tree after shared guidance injection (not a pinned archive-byte SHA, which GitHub does not keep stable). The Gateway image still installs a pinned `@larksuite/cli` binary, so `get_lark_integration_status` surfaces `latest_available_version` and `runtime_version_mismatch` for the UI. AIO installs additionally verify and publish official Linux amd64/arm64 binaries under `{DEER_FLOW_HOME}/integrations/lark-cli/sandbox-cli`, mounted read-only at `/mnt/integrations/lark-cli/runtime`; `/mnt/integrations/lark-cli/config` (app credentials, incl. the long-lived `appSecret`) is mounted **read-only** into the sandbox, its empty `config/locks` subdirectory is over-mounted writable for `lark-cli` coordination files, and `/mnt/integrations/lark-cli/data` (refreshable OAuth tokens) stays writable, all mapping to owner-only per-user directories. **Sandbox trust boundary:** the credential-bearing config and data dirs are still *readable* by arbitrary sandbox processes (the agent's `bash` tool, or code reached via prompt-injection in a tool result), so the app secret and tokens are exposed to sandbox-side code even though they never reach the browser — the read-only config mount only prevents in-sandbox tampering, not read/exfiltration. The sidecar credential-broker (Pattern B, issue #4338) is the fix that removes these plaintext mounts from sandbox execution: set `LARK_CLI_BROKER_IMAGE` on the provisioner (see `docker/lark-cli-broker/`) and the Gateway sends `provision_lark_cli_broker` on sandbox create. The provisioner then runs a `lark-cli-broker` sidecar that owns the per-user `config`/`config/locks`/`data` mounts (mounted into the **sidecar only**, at `/var/lark/{config,config/locks,data}` with only the nested locks mount writable) and serves the `lark-cli` command surface on Pod loopback (`http://127.0.0.1:8788`); a shim init container (`install-shim`) writes a forwarding `lark-cli` into the shared runtime `emptyDir`, so the sandbox gets `DEERFLOW_LARK_BROKER_URL` + a shim on PATH but **no** credential files. The on-PATH `bin/lark-cli` is a `/bin/sh` launcher that resolves a Python 3 interpreter and execs the Python shim body (`bin/lark-cli-shim.py`) beside it, so broker mode does not silently ENOEXEC on a sandbox image without a `#!/usr/bin/env python3`-resolvable interpreter — it fails loudly (exit 127, actionable message) and can be pinned with `DEERFLOW_LARK_BROKER_PYTHON`. The broker runs `lark-cli` in the sidecar's cwd and cannot see the sandbox filesystem, so cwd is intentionally **not** forwarded and file-I/O subcommands relative to the sandbox cwd are unsupported (command surface only). An optional `DEERFLOW_LARK_BROKER_DENY_SUBCOMMANDS` denylist (comma-separated command paths, matched in order through interleaved option values, forwarded from the provisioner) lets the broker refuse secret-dumping subcommands before spawning the binary. `lark_cli_env_overlay(broker=True)` therefore omits `LARKSUITE_CLI_CONFIG_DIR`/`DATA_DIR`; `sandbox_lark_broker_active()` (TTL-cached provisioner `/api/capabilities` probe, tight timeout + longer negative caching on the bash hot path) selects broker vs. binary mode for both the bash env overlay and status. `DEER_FLOW_LARK_CLI_SANDBOX_RUNTIME_DIR` supplies a validated, symlink-free pre-staged runtime for air-gapped deployments. For the remote provisioner (K8s), the runtime binary is otherwise provisioned by an optional init container + shared `emptyDir` (Pattern A): set `LARK_CLI_INIT_IMAGE` on the provisioner (see `docker/lark-cli-init/`) and the Gateway sends `provision_lark_cli_runtime` on sandbox create once the pack is installed, so remote installs skip the Gateway-side GitHub download entirely. Broker (Pattern B) supersedes the init-container binary (Pattern A) when both images are configured. `get_lark_integration_status(check_runtime=True)` surfaces `sandbox_runtime_mode` (`none` / `gateway-download` / `init-container` / `broker`) and `sandbox_runtime_ready` (remote modes read the provisioner `GET /api/capabilities`: `lark_cli_init_image` / `lark_cli_broker_image`), plus `sandbox_runtime_probed` separating a confirmed probe result from the conservative unprobed fallback (mutation responses carry a probed status built from a credential snapshot taken under the credential lock), so a green UI can't hide a chat-time `lark-cli: command not found`. Cheap status probes are explicitly not live-verified; users authorize or reconnect through the browser device-flow endpoints instead of running terminal commands. +- **Managed integrations**: Lark/Feishu CLI support installs one global official `lark-*` pack as read-only `SkillCategory.INTEGRATION` entries under `/mnt/skills/integrations/lark-cli/...`; enabled flags, app configuration, and OAuth data remain per-user. Install resolves the newest `larksuite/cli` release from GitHub (`releases/latest`) at install time (falling back to a bottom-line pinned version if the lookup fails) rather than hard-coding the pack version; integrity relies on the official host + structural archive guards + a recorded hash of the effective installed tree after shared guidance injection (not a pinned archive-byte SHA, which GitHub does not keep stable). The Gateway image still installs a pinned `@larksuite/cli` binary, so `get_lark_integration_status` surfaces `latest_available_version` and `runtime_version_mismatch` for the UI. AIO installs additionally verify and publish official Linux amd64/arm64 binaries under `{DEER_FLOW_HOME}/integrations/lark-cli/sandbox-cli`, mounted read-only at `/mnt/integrations/lark-cli/runtime`; `/mnt/integrations/lark-cli/config` (app credentials, incl. the long-lived `appSecret`) is mounted **read-only** into the sandbox, its empty `config/locks` subdirectory is over-mounted writable for `lark-cli` coordination files, and `/mnt/integrations/lark-cli/data` (refreshable OAuth tokens) stays writable, all mapping to owner-only per-user directories. **Sandbox trust boundary:** the credential-bearing config and data dirs are still *readable* by arbitrary sandbox processes (the agent's `bash` tool, or code reached via prompt-injection in a tool result), so the app secret and tokens are exposed to sandbox-side code even though they never reach the browser — the read-only config mount only prevents in-sandbox tampering, not read/exfiltration. The sidecar credential-broker (Pattern B, issue #4338) is the fix that removes these plaintext mounts from sandbox execution: set `LARK_CLI_BROKER_IMAGE` on the provisioner (see `docker/lark-cli-broker/`) and the Gateway sends `provision_lark_cli_broker` on sandbox create. The provisioner then runs a `lark-cli-broker` sidecar that owns the per-user `config`/`config/locks`/`data` mounts (mounted into the **sidecar only**, at `/var/lark/{config,config/locks,data}` with only the nested locks mount writable) and serves the `lark-cli` command surface on Pod loopback (`http://127.0.0.1:8788`); a shim init container (`install-shim`) writes a forwarding `lark-cli` into the shared runtime `emptyDir`, so the sandbox gets `DEERFLOW_LARK_BROKER_URL` + a shim on PATH but **no** credential files. The on-PATH `bin/lark-cli` is a `/bin/sh` launcher that resolves a Python 3 interpreter and execs the Python shim body (`bin/lark-cli-shim.py`) beside it, so broker mode does not silently ENOEXEC on a sandbox image without a `#!/usr/bin/env python3`-resolvable interpreter — it fails loudly (exit 127, actionable message) and can be pinned with `DEERFLOW_LARK_BROKER_PYTHON`. The broker runs `lark-cli` in the sidecar's cwd and cannot see the sandbox filesystem, so cwd is intentionally **not** forwarded and file-I/O subcommands relative to the sandbox cwd are unsupported (command surface only). An optional `DEERFLOW_LARK_BROKER_DENY_SUBCOMMANDS` denylist (comma-separated command paths, matched in order through interleaved option values, forwarded from the provisioner) lets the broker refuse secret-dumping subcommands before spawning the binary. `lark_cli_env_overlay(broker=True)` therefore omits `LARKSUITE_CLI_CONFIG_DIR`/`DATA_DIR`; `sandbox_lark_broker_active()` caches confirmed provisioner capabilities for admission and fails closed on unknowns; bash overlays use the admitted sandbox's actual mode. Configured broker provisioning rejects direct credential mounts. `DEER_FLOW_LARK_CLI_SANDBOX_RUNTIME_DIR` supplies a validated, symlink-free pre-staged runtime for air-gapped deployments. For the remote provisioner (K8s), the runtime binary is otherwise provisioned by an optional init container + shared `emptyDir` (Pattern A): set `LARK_CLI_INIT_IMAGE` on the provisioner (see `docker/lark-cli-init/`) and the Gateway sends `provision_lark_cli_runtime` on sandbox create once the pack is installed, so remote installs skip the Gateway-side GitHub download entirely. Broker (Pattern B) supersedes the init-container binary (Pattern A) when both images are configured. `get_lark_integration_status(check_runtime=True)` surfaces `sandbox_runtime_mode` (`none` / `gateway-download` / `init-container` / `broker`) and `sandbox_runtime_ready` (remote modes read the provisioner `GET /api/capabilities`: `lark_cli_init_image` / `lark_cli_broker_image`), plus `sandbox_runtime_probed` separating a confirmed probe result from the conservative unprobed fallback (mutation responses carry a probed status built from a credential snapshot taken under the credential lock), so a green UI can't hide a chat-time `lark-cli: command not found`. Cheap status probes are explicitly not live-verified; users authorize or reconnect through the browser device-flow endpoints instead of running terminal commands. - **SkillScan**: `packages/harness/deerflow/skills/skillscan/` is the native deterministic scanner for `.skill` archives and agent-managed skill writes. It runs offline before the LLM scanner, emits structured findings (`rule_id`, `severity`, `file`, `line`, `message`, `remediation`, redacted `evidence` — category/analyzer are encoded in the `rule_id` prefix), blocks `CRITICAL`, and passes warning findings into `scan_skill_content()`. The moderation adapter must normalize both plain-text responses and LangChain Responses API text blocks before parsing the required JSON decision. `scan_archive_preflight()` / `scan_skill_dir()` are pure sync functions (dispatch off the event loop); `enforce_static_scan()` applies the blocking policy and the `skill_scan.enabled` kill switch. `skills/package_files.py` is the single definition of code files (`scripts/` members, code suffixes, extensionless `#!` files) and executable magic bytes; the installer, export guard, and SkillScan all import it, so do not re-derive either rule locally. Files that fail NUL-free UTF-8 decoding are treated as binaries unless they are code files: interpreters still run those, so they raise `package-undecodable-script` (HIGH) and are analyzed over a lossy decode, keeping one stray byte from hiding a file or downgrading a `CRITICAL` match; one with executable magic skips the text rules, which only misread its string tables. The Python instance-client signal deliberately follows only a one-level, same-scope evidence chain (PR #4265 review): a proven imported constructor bound to a simple name, optional name-to-name alias propagation, rebinding invalidation, and a constructor-supported outbound method or context-manager use; bare canonical-looking names never fall back to module identity. Nested scopes never inherit client handles and inherit only constructor aliases proven stable by a binding-only enclosing-scope prepass. Comprehensions, walrus-bearing statements, annotations, executable expressions inside complex binding targets, unsupported operations, and ambiguous flows produce no finding from this signal; skipped constructs invalidate all names they may bind, while representative false negatives are pinned by `test_python_declared_false_negatives_stay_unreported`. Compound bodies are walked from isolated copies so wrapping code in `if True:` is not a bypass, while copied scope entries, binding-only prepasses, and AST visits consume a deterministic work budget and the walk stops after its first sink. Budget or recursion exhaustion skips only this best-effort signal and retains deterministic findings already collected for the file. Do not add Semgrep/OpenGrep or YAML rule-engine dependencies to the core path; Phase 1 rule specs live in Python constants next to their analyzers in `skillscan/orchestrator.py`. - **SkillScan HTTP hosts**: recognize HTTP/HTTPS schemes case-insensitively in both URL regexes and prefix checks, without changing source evidence. Use URL authority parsing for lowercase hosts and bracketed IPv6 loopback; userinfo must never hide an external host. Malformed URLs remain outbound for exfiltration checks. Cleartext HTTP detection covers IPv6 literals; cloud-metadata hostname matching is case-insensitive. Coverage: `tests/test_skillscan_native.py`. - **Python secret mappings**: `secret-env-assignment` scans literal container values and conditional branches under secret-like bindings. Dictionary keys are structural labels and contribute candidates only when their folded literal matches the shared `_SECRET_TOKEN_PATTERNS`; they must not inherit credential evidence from the enclosing variable name. Opaque keys without a recognized token format stay unreported. Regressions in `tests/test_skillscan_native.py` cover runtime-loaded mappings, nested labels, credential-value locations, and literal/concatenated token keys; `tests/test_skill_review_core.py` pins the CI CLI gate. diff --git a/backend/tests/blocking_io/test_dingtalk_receive_file.py b/backend/tests/blocking_io/test_dingtalk_receive_file.py index 1415adf20f..7f972799c2 100644 --- a/backend/tests/blocking_io/test_dingtalk_receive_file.py +++ b/backend/tests/blocking_io/test_dingtalk_receive_file.py @@ -72,7 +72,7 @@ async def test_receive_file_persist_does_not_block_event_loop(tmp_path, monkeypa monkeypatch.setattr( "app.channels.dingtalk.get_sandbox_provider", - lambda: SimpleNamespace(acquire_async=_acquire_async, get=lambda sid: None), + lambda: SimpleNamespace(uses_thread_data_mounts=True, acquire_async=_acquire_async, get=lambda sid: None), ) channel = DingTalkChannel(MessageBus(), config={}) diff --git a/backend/tests/test_aio_sandbox.py b/backend/tests/test_aio_sandbox.py index e56cdc5a3e..722cdf049d 100644 --- a/backend/tests/test_aio_sandbox.py +++ b/backend/tests/test_aio_sandbox.py @@ -109,6 +109,27 @@ def sandbox(): return sb +def test_lark_cli_broker_defaults_to_unverified(sandbox): + """The constructor default must stay None (unverified). + + False would mean "attested non-broker" and authorize the plaintext + credential-mount overlay, so a construction site that forgets to thread + the attested value must fail closed instead (#6436 review). + """ + assert sandbox.lark_cli_broker is None + + +def test_sandbox_info_from_dict_keeps_missing_broker_attestation_unverified(): + """A legacy record without the field must rehydrate as unverified (None), + never as attested non-broker — otherwise a future persistence path would + silently undo the fail-closed contract (#6436 review).""" + from deerflow.community.aio_sandbox.sandbox_info import SandboxInfo + + info = SandboxInfo.from_dict({"sandbox_id": "s", "sandbox_url": "http://x"}) + assert info.lark_cli_broker is None + assert SandboxInfo.from_dict({"sandbox_id": "s", "sandbox_url": "http://x", "lark_cli_broker": False}).lark_cli_broker is False + + def test_exec_command_appends_exit_marker_when_failure_has_output(sandbox): """The legacy exec path must propagate the structured exit_code into the output text (LocalSandbox parity) instead of discarding it.""" @@ -948,18 +969,17 @@ class TestShellSessionCreationOwnership: self, sandbox, ): - sandbox._default_shell_corrupted = True create_session = MagicMock(side_effect=httpx.ConnectError("connection refused")) cleanup_session = MagicMock() sandbox._client.shell.create_session = create_session sandbox._client.shell.cleanup_session = cleanup_session - assert sandbox.execute_command("first").startswith("Error:") + assert sandbox.execute_command_in_scope("first", scope_id="create-failure").startswith("Error:") assert sandbox.requires_container_recycle is False cleanup_session.assert_not_called() - sandbox.execute_command("second") + sandbox.execute_command_in_scope("second", scope_id="create-failure") assert create_session.call_count == 2 def test_shell_client_error_is_definite_and_does_not_quarantine( @@ -968,7 +988,6 @@ class TestShellSessionCreationOwnership: ): from agent_sandbox.core.api_error import ApiError - sandbox._default_shell_corrupted = True sandbox._client.shell.create_session = MagicMock( side_effect=ApiError( status_code=400, @@ -977,7 +996,7 @@ class TestShellSessionCreationOwnership: ) sandbox._client.shell.cleanup_session = MagicMock() - assert sandbox.execute_command("bad").startswith("Error:") + assert sandbox.execute_command_in_scope("bad", scope_id="create-failure").startswith("Error:") assert sandbox.requires_container_recycle is False sandbox._client.shell.cleanup_session.assert_not_called() @@ -1095,6 +1114,40 @@ class TestShellSessionCreationOwnership: assert "earlier ambiguous create outcome" in blocked assert create_calls == before + def test_in_flight_shell_create_does_not_fence_the_container(self, sandbox): + """A pending create is transient, not an uncertain outcome. + + While the create is in flight the container is healthy, so a + concurrent acquire must not read `requires_container_recycle` as a + recycle reason and destroy a live run's runtime (#6436 review P1). + Only the unresolved (ambiguous) outcome fences the container. + """ + entered = threading.Event() + release = threading.Event() + results: list[str] = [] + + def stalled_create(id, **kwargs): + entered.set() + assert release.wait(timeout=2) + raise httpx.ReadTimeout("create stalled") + + sandbox._client.shell.create_session = stalled_create + sandbox._client.shell.cleanup_session = MagicMock() + + thread = threading.Thread(target=lambda: results.append(sandbox.execute_command_in_scope("first", scope_id="scope-a"))) + thread.start() + try: + assert entered.wait(timeout=2) + assert sandbox.has_pending_session_creates is True + assert sandbox.requires_container_recycle is False + finally: + release.set() + thread.join(timeout=2) + assert not thread.is_alive() + assert "session creation outcome is unknown" in results[0] + assert sandbox.has_pending_session_creates is False + assert sandbox.requires_container_recycle is True + class TestBashSessionCreationOwnership: """Bash env-session creation follows the same bounded ownership contract.""" @@ -1186,6 +1239,43 @@ class TestBashSessionCreationOwnership: assert "earlier ambiguous create outcome" in second create_session.assert_not_called() + def test_in_flight_bash_create_does_not_fence_the_container(self, sandbox): + """The bash plane follows the same transient-pending contract.""" + entered = threading.Event() + release = threading.Event() + results: list[str] = [] + + def stalled_create(session_id, **kwargs): + entered.set() + assert release.wait(timeout=2) + raise httpx.ReadTimeout("create stalled") + + sandbox._client.bash.create_session = stalled_create + sandbox._client.bash.close_session = MagicMock() + sandbox._client.bash.exec = MagicMock() + + thread = threading.Thread( + target=lambda: results.append( + sandbox.execute_command( + "echo $TOKEN", + env={"TOKEN": "secret"}, + ) + ) + ) + thread.start() + try: + assert entered.wait(timeout=2) + assert sandbox.has_pending_session_creates is True + assert sandbox.requires_container_recycle is False + sandbox._client.bash.exec.assert_not_called() + finally: + release.set() + thread.join(timeout=2) + assert not thread.is_alive() + assert "session creation outcome is unknown" in results[0] + assert sandbox.has_pending_session_creates is False + assert sandbox.requires_container_recycle is True + def test_shell_creation_quarantine_does_not_block_bash_plane( self, sandbox, diff --git a/backend/tests/test_aio_sandbox_local_backend.py b/backend/tests/test_aio_sandbox_local_backend.py index 71b00d6737..c1ac23f39f 100644 --- a/backend/tests/test_aio_sandbox_local_backend.py +++ b/backend/tests/test_aio_sandbox_local_backend.py @@ -1770,7 +1770,7 @@ def test_restricted_destroy_stops_pair_and_removes_both_networks(monkeypatch): ) ) - assert stopped == ["sandbox-container-id", proxy_name] + assert stopped == [proxy_name, "sandbox-container-id"] assert ["docker", "rm", "-f", proxy_name] in commands assert ["docker", "network", "rm", network_name] in commands assert ["docker", "network", "rm", egress_network_name] in commands @@ -1779,12 +1779,13 @@ def test_restricted_destroy_stops_pair_and_removes_both_networks(monkeypatch): def test_open_mode_replacement_destroy_removes_restricted_sidecar_and_networks(monkeypatch): backend = _backend_for_inspect_tests() stopped: list[str] = [] - cleaned: list[tuple[str, bool]] = [] + cleaned: list[tuple[str, bool, bool]] = [] monkeypatch.setattr(backend, "_stop_container", stopped.append) + monkeypatch.setattr(backend, "_remove_restricted_proxy", lambda sandbox_id: cleaned.append((sandbox_id, False, True))) monkeypatch.setattr( backend, "_cleanup_restricted_resources", - lambda sandbox_id, *, stop_sandbox=True: cleaned.append((sandbox_id, stop_sandbox)), + lambda sandbox_id, *, stop_sandbox=True, remove_proxy=True: cleaned.append((sandbox_id, stop_sandbox, remove_proxy)), ) backend.destroy( @@ -1797,7 +1798,7 @@ def test_open_mode_replacement_destroy_removes_restricted_sidecar_and_networks(m ) assert stopped == ["sandbox-old-restricted"] - assert cleaned == [("old-restricted", False)] + assert cleaned == [("old-restricted", False, True), ("old-restricted", False, False)] def test_deny_pending_network_policy_events_uses_atomic_proxy_command(monkeypatch): @@ -1818,6 +1819,7 @@ def test_deny_pending_network_policy_events_uses_atomic_proxy_command(monkeypatc def test_is_container_running_false_on_apple_container_not_found(monkeypatch): """Apple Container's generic "not found" is trusted when it names the container.""" backend = _backend_for_inspect_tests() + backend._runtime = "container" def fake_run(cmd, **kwargs): return SimpleNamespace(stdout="", stderr='Error: not found: "sandbox-apple"', returncode=1) @@ -1840,6 +1842,754 @@ def test_is_container_running_raises_on_unrelated_not_found_error(monkeypatch): backend._is_container_running("sandbox-busy") +@pytest.mark.parametrize("runtime", ["docker", "container"]) +def test_absence_does_not_treat_stopped_container_as_missing(monkeypatch, runtime): + backend = _backend_for_inspect_tests() + backend._runtime = runtime + name = "sandbox-stopped" + entry = {"Name": f"/{name}", "Id": "stopped-generation", "State": {"Running": False}} if runtime == "docker" else {"configuration": {"id": name}, "status": "stopped"} + monkeypatch.setattr("subprocess.run", lambda *_args, **_kwargs: SimpleNamespace(stdout=json.dumps([entry]), stderr="", returncode=0)) + assert backend.is_absent("stopped") is False + + +@pytest.mark.parametrize("runtime", ["docker", "container"]) +def test_absence_requires_runtime_confirmation(monkeypatch, runtime): + backend = _backend_for_inspect_tests() + backend._runtime = runtime + name = f"{backend._container_prefix}-missing" + error = f"Error: No such container: {name}" if runtime == "docker" else f'Error: not found: "{name}"' + monkeypatch.setattr("subprocess.run", lambda *_args, **_kwargs: SimpleNamespace(stdout="", stderr=error, returncode=1)) + assert backend.is_absent("missing") is True + monkeypatch.setattr("subprocess.run", lambda *_args, **_kwargs: SimpleNamespace(stdout="", stderr="daemon unavailable", returncode=1)) + with pytest.raises(RuntimeError): + backend.is_absent("missing") + + +@pytest.mark.parametrize("operation", ["is_absent", "_is_container_running", "_stop_container"]) +@pytest.mark.parametrize( + "error", + [ + 'Failed to initialize: unable to resolve docker endpoint: context "deerflow-container-review-missing": context not found: open /synthetic-home/.docker/contexts/meta/context/meta.json: no such file or directory', + 'unable to resolve docker endpoint: context "sandbox-missing": context not found', + "Error: container runtime not found while inspecting sandbox-missing", + "OCI runtime failed for container sandbox-missing: executable file not found in $PATH", + "Error: No such object: sandbox-other", + "Error response from daemon: No such container: sandbox-missing-extra", + # Docker 29 lowercases the client-side scaffolding; the near-miss and + # case-sensitivity rules must survive the normalization. + "error: no such object: sandbox-other", + "error response from daemon: no such container: sandbox-missing-extra", + "error: no such object: Sandbox-Missing", + "error: no such object", + "error: no such image: sandbox-missing", + "prefix error: no such object: sandbox-missing", + "error: no such object: sandbox-missing suffix", + "error: no such object: sandbox-missing\nError response from daemon: permission denied", + "warning: using default context\nerror: no such object: sandbox-missing", + ], +) +def test_runtime_absence_rejects_errors_not_about_the_requested_container(monkeypatch, operation, error): + backend = _backend_for_inspect_tests() + + def failed(cmd, **kwargs): + if kwargs.get("check"): + raise subprocess.CalledProcessError(1, cmd, stderr=error) + return SimpleNamespace(stdout="", stderr=error, returncode=1) + + monkeypatch.setattr("subprocess.run", failed) + target = "missing" if operation == "is_absent" else "sandbox-missing" + with pytest.raises((RuntimeError, subprocess.CalledProcessError)): + getattr(backend, operation)(target) + + +@pytest.mark.parametrize("operation", ["is_absent", "_is_container_running", "_stop_container"]) +@pytest.mark.parametrize( + ("runtime", "error"), + [ + ("docker", "Error: No such object: sandbox-missing"), + ("docker", "Error response from daemon: No such container: sandbox-missing"), + # Docker CLI v29 lowercases the client-side error scaffolding + # ("error: no such object: "); the name stays byte-exact. + ("docker", "error: no such object: sandbox-missing"), + ("docker", "error response from daemon: no such container: sandbox-missing"), + # ArgumentParser prefers ContainerizationError.errorDescription, so + # ContainerInspect's missing-resource message has no type or quotes. + ("container", "Error: container not found: sandbox-missing"), + ("container", 'Error: notFound: "container not found: sandbox-missing"'), + ("container", 'notFound: "container not found: sandbox-missing"'), + ("container", 'Error: not found: "sandbox-missing"'), + ], +) +def test_runtime_absence_accepts_exact_missing_container_responses(monkeypatch, operation, runtime, error): + backend = _backend_for_inspect_tests() + backend._runtime = runtime + + def missing(cmd, **kwargs): + if kwargs.get("check"): + raise subprocess.CalledProcessError(1, cmd, stderr=error) + return SimpleNamespace(stdout="[]", stderr=error, returncode=1) + + monkeypatch.setattr("subprocess.run", missing) + target = "missing" if operation == "is_absent" else "sandbox-missing" + result = getattr(backend, operation)(target) + assert result is (True if operation == "is_absent" else False if operation == "_is_container_running" else None) + + +@pytest.mark.parametrize("state", ["created", "exited", "running", "dead"]) +def test_inspect_runtime_observes_docker_generation_without_health_or_port(monkeypatch, state): + backend = _backend_for_inspect_tests() + commands = [] + entry = {"Id": "immutable-docker-generation", "Name": "/sandbox-existing", "Created": "2026-04-08T01:22:50Z", "State": {"Status": state, "Running": state == "running"}} + + def inspect(cmd, **kwargs): + commands.append(cmd) + assert 0 < kwargs["timeout"] <= 15 + return SimpleNamespace(stdout=json.dumps([entry]), stderr="", returncode=0) + + monkeypatch.setattr("subprocess.run", inspect) + monkeypatch.setattr(backend, "_is_container_running", lambda *_args: pytest.fail("runtime inspection must not require running")) + monkeypatch.setattr(backend, "_get_container_port", lambda *_args: pytest.fail("runtime inspection must not require a port")) + monkeypatch.setattr(local_backend_module, "wait_for_sandbox_ready", lambda *_args, **_kwargs: pytest.fail("runtime inspection must not require health")) + + info = backend.inspect_runtime("existing") + + assert info is not None + assert info.sandbox_id == "existing" + assert info.container_name == "sandbox-existing" + assert info.container_id == "immutable-docker-generation" + assert info.created_at == 1775611370.0 + assert commands == [["docker", "inspect", "sandbox-existing"]] + + +def test_inspect_runtime_keeps_apple_resource_with_unknown_generation(monkeypatch): + backend = _backend_for_inspect_tests() + backend._runtime = "container" + # ManagedContainer serializes configuration.id, which is the user-selected + # logical name rather than an immutable generation; it cannot fence teardown. + entry = {"id": "sandbox-existing", "configuration": {"id": "sandbox-existing"}, "status": {"state": "stopped"}} + monkeypatch.setattr("subprocess.run", lambda *_args, **_kwargs: SimpleNamespace(stdout=json.dumps([entry]), stderr="", returncode=0)) + + info = backend.inspect_runtime("existing") + + assert info is not None + assert info.container_name == "sandbox-existing" + assert info.container_id is None + assert backend.is_absent("existing") is False + + +@pytest.mark.parametrize("stopped", [False, True]) +def test_destroy_retry_releases_port_recovered_from_runtime_metadata(monkeypatch, stopped): + from deerflow.utils.network import PortAllocator + + backend = _backend_for_inspect_tests() + allocator = PortAllocator() + monkeypatch.setattr(allocator, "_is_port_available", lambda port: port not in allocator._reserved_ports) + port = allocator.allocate(18080, max_range=1) + monkeypatch.setattr(local_backend_module, "release_port", allocator.release) + entry = { + "Id": "retained-generation", + "Name": "/sandbox-existing", + "NetworkSettings": {"Ports": {"8080/tcp": None if stopped else [{"HostPort": str(port)}]}}, + "HostConfig": {"PortBindings": {"8080/tcp": [{"HostPort": str(port)}]}}, + } + failed = True + + def runtime(cmd, **kwargs): + nonlocal failed + if "inspect" in cmd: + return SimpleNamespace(stdout=json.dumps([entry]), stderr="", returncode=0) + if failed: + failed = False + raise subprocess.CalledProcessError(1, cmd, stderr="daemon denied stop") + return SimpleNamespace(stdout="", stderr="", returncode=0) + + monkeypatch.setattr("subprocess.run", runtime) + original = SandboxInfo("existing", f"http://localhost:{port}", container_id="retained-generation") + with pytest.raises(subprocess.CalledProcessError): + backend.destroy(original) + with pytest.raises(RuntimeError, match="No available port"): + allocator.allocate(18080, max_range=1) + + recovered = backend.inspect_runtime("existing") + assert recovered is not None + backend.destroy(recovered) + assert allocator.allocate(18080, max_range=1) == port + + +@pytest.mark.parametrize("proxy_missing", [False, True]) +@pytest.mark.parametrize("persisted_mode", ["allowlist", "isolated"]) +def test_inspect_runtime_recovers_restricted_proxy_port_without_losing_main_identity(monkeypatch, proxy_missing, persisted_mode): + backend = _backend_for_inspect_tests() + proxy_name, _ = backend._resource_names("existing") + entry = {"Id": "main-generation", "Name": "/sandbox-existing", "Config": {"Labels": {"deerflow.network_mode": persisted_mode}}} + proxy = {"Id": "proxy-generation", "Name": f"/{proxy_name}", "HostConfig": {"PortBindings": {"8080/tcp": [{"HostPort": "18080"}]}}} + + def runtime(cmd, **kwargs): + if cmd[-1] == proxy_name: + return SimpleNamespace(stdout="[]" if proxy_missing else json.dumps([proxy]), stderr=f"Error: No such object: {proxy_name}" if proxy_missing else "", returncode=1 if proxy_missing else 0) + return SimpleNamespace(stdout=json.dumps([entry]), stderr="", returncode=0) + + monkeypatch.setattr("subprocess.run", runtime) + info = backend.inspect_runtime("existing") + assert info is not None + assert info.container_id == "main-generation" + assert info.sandbox_url == ("" if proxy_missing else "http://localhost:18080") + + +def test_inspect_runtime_propagates_unknown_proxy_port_probe(monkeypatch): + backend = _backend_for_inspect_tests() + proxy_name, _ = backend._resource_names("existing") + entry = {"Id": "main-generation", "Name": "/sandbox-existing", "Config": {"Labels": {"deerflow.network_mode": "allowlist"}}} + + def runtime(cmd, **kwargs): + if cmd[-1] == proxy_name: + return SimpleNamespace(stdout="", stderr="daemon unavailable", returncode=1) + return SimpleNamespace(stdout=json.dumps([entry]), stderr="", returncode=0) + + monkeypatch.setattr("subprocess.run", runtime) + with pytest.raises(RuntimeError, match="daemon unavailable"): + backend.inspect_runtime("existing") + + +def test_inspect_runtime_retains_direct_binding_during_network_mode_transition(monkeypatch): + backend = _backend_for_inspect_tests() + backend._network_mode = "allowlist" + proxy_name, _ = backend._resource_names("existing") + entry = {"Id": "legacy-generation", "Name": "/sandbox-existing", "HostConfig": {"PortBindings": {"8080/tcp": [{"HostPort": "18080"}]}}} + + def runtime(cmd, **kwargs): + if cmd[-1] == proxy_name: + return SimpleNamespace(stdout="[]", stderr=f"Error: No such object: {proxy_name}", returncode=1) + return SimpleNamespace(stdout=json.dumps([entry]), stderr="", returncode=0) + + monkeypatch.setattr("subprocess.run", runtime) + assert backend.inspect_runtime("existing").sandbox_url == "http://localhost:18080" + + +def test_inspect_runtime_does_not_inherit_another_generations_pending_port(monkeypatch): + backend = _backend_for_inspect_tests() + backend._pending_cleanup_ports["old-generation"] = ("existing", 18080) + entry = {"Id": "replacement-generation", "Name": "/sandbox-existing"} + monkeypatch.setattr("subprocess.run", lambda *_args, **_kwargs: SimpleNamespace(stdout=json.dumps([entry]), stderr="", returncode=0)) + info = backend.inspect_runtime("existing") + assert info.container_id == "replacement-generation" + assert info.sandbox_url == "" + assert backend._pending_cleanup_ports == {"old-generation": ("existing", 18080)} + + +def test_destroy_releases_port_after_both_containers_are_removed_despite_network_timeout(monkeypatch): + backend = _backend_for_inspect_tests() + backend._network_mode = "allowlist" + released = [] + monkeypatch.setattr(local_backend_module, "release_port", released.append) + proxy_name, _ = backend._resource_names("existing") + stopped = [] + monkeypatch.setattr(backend, "_stop_container", stopped.append) + + def runtime(cmd, **kwargs): + if cmd[:3] == ["docker", "network", "rm"]: + raise subprocess.TimeoutExpired(cmd, 15) + return SimpleNamespace(stdout="", stderr="", returncode=0) + + monkeypatch.setattr("subprocess.run", runtime) + info = SandboxInfo("existing", "http://localhost:18080", container_name="sandbox-existing", container_id="main-generation") + with pytest.raises(subprocess.TimeoutExpired): + backend.destroy(info) + assert stopped == [proxy_name, "main-generation"] + assert released == [18080] + assert backend._pending_cleanup_ports == {} + + +def test_confirmed_absent_teardown_releases_a_timed_out_but_removed_generations_port_once(monkeypatch): + from concurrent.futures import ThreadPoolExecutor + + from deerflow.utils.network import PortAllocator + + backend = _backend_for_inspect_tests() + allocator = PortAllocator() + monkeypatch.setattr(allocator, "_is_port_available", lambda port: port not in allocator._reserved_ports) + port = allocator.allocate(18080, max_range=1) + other_port = allocator.allocate(18081, max_range=1) + released = [] + removed = False + + def release(port): + released.append(port) + allocator.release(port) + + def runtime(cmd, **kwargs): + nonlocal removed + if "stop" in cmd: + if cmd[-1].startswith("deer-flow-netproxy-"): + raise subprocess.CalledProcessError(1, cmd, stderr=f"Error: No such container: {cmd[-1]}") + if cmd[-1] == "removed-generation": + removed = True + raise subprocess.TimeoutExpired(cmd, 120) + if cmd[:3] in [["docker", "rm", "-f"], ["docker", "network", "rm"]]: + return SimpleNamespace(stdout="", stderr="", returncode=0) + assert removed + return SimpleNamespace(stdout="[]", stderr="Error: No such object: sandbox-existing", returncode=1) + + monkeypatch.setattr("subprocess.run", runtime) + monkeypatch.setattr(local_backend_module, "release_port", release) + for sandbox_id, generation, reserved_port in [("other", "other-generation", other_port), ("existing", "removed-generation", port)]: + with pytest.raises(subprocess.TimeoutExpired): + backend.destroy(SandboxInfo(sandbox_id, f"http://localhost:{reserved_port}", container_id=generation)) + + assert backend.inspect_runtime("existing") is None + assert backend.is_absent("existing") is True + assert released == [] # Metadata and presence reads never mutate reservations. + with ThreadPoolExecutor(max_workers=2) as pool: + list(pool.map(backend.complete_absent_teardown, ["existing", "existing"])) + assert released == [port] + assert allocator.allocate(18080, max_range=1) == port + with pytest.raises(RuntimeError, match="No available port"): + allocator.allocate(18081, max_range=1) + backend.complete_absent_teardown("existing") + assert released == [port] + + +@pytest.mark.parametrize("restarted", [False, True]) +def test_absent_teardown_removes_leftover_networks_and_allows_restricted_creation(monkeypatch, restarted): + backend = _restricted_backend() + restarted_backend = _restricted_backend() if restarted else None + proxy_name, network_name = backend._resource_names("existing") + networks = {network_name, backend._egress_network_name("existing")} + released = [] + monkeypatch.setattr(local_backend_module, "release_port", released.append) + + def runtime(cmd, **kwargs): + if "stop" in cmd: + if cmd[-1] == "old-generation": + raise subprocess.TimeoutExpired(cmd, 120) + raise subprocess.CalledProcessError(1, cmd, stderr=f"Error: No such container: {cmd[-1]}") + if cmd[:3] == ["docker", "rm", "-f"]: + return SimpleNamespace(stdout="", stderr=f"Error: No such container: {proxy_name}", returncode=1) + if cmd[:3] == ["docker", "network", "rm"]: + networks.discard(cmd[-1]) + return SimpleNamespace(stdout=cmd[-1], stderr="", returncode=0) + if cmd[:3] == ["docker", "network", "inspect"]: + if cmd[-1] in networks: + return SimpleNamespace(stdout=json.dumps([{"Driver": "bridge", "Internal": True}]), stderr="", returncode=0) + return SimpleNamespace(stdout="[]", stderr=f"Error response from daemon: network {cmd[-1]} not found", returncode=1) + assert cmd[:2] == ["docker", "inspect"] + return SimpleNamespace(stdout="[]", stderr="\n".join(f"Error: No such container: {name}" for name in cmd[2:]), returncode=1) + + monkeypatch.setattr("subprocess.run", runtime) + with pytest.raises(subprocess.TimeoutExpired): + backend.destroy(SandboxInfo("existing", "http://localhost:18080", container_id="old-generation")) + assert backend.is_absent("existing") is True + assert backend._restricted_resources_status("existing") == "mismatch" + if restarted: + backend = restarted_backend + assert backend._pending_cleanup_ports == {} + backend.complete_absent_teardown("existing") + assert backend._restricted_resources_status("existing") == "missing" + assert released == ([] if restarted else [18080]) + monkeypatch.setattr(backend, "_create_internal_network", lambda *_args: None) + monkeypatch.setattr(backend, "_create_egress_network", lambda *_args: None) + monkeypatch.setattr(backend, "_start_network_proxy", lambda *_args: None) + monkeypatch.setattr(backend, "_start_container", lambda *_args, **_kwargs: "replacement-generation") + assert backend._start_restricted_sandbox("existing", "sandbox-existing", 18080, None, config_mount_exclusion_root=None, relay_token="token") == "replacement-generation" + + +@pytest.mark.parametrize( + "stderr, absent", + [ + ("Error response from daemon: network sandbox-net-x not found", True), + ("error: network sandbox-net-x not found", True), # Docker 29 lowercase + ("Error: No such network: sandbox-net-x", True), + # A different network's absence, a context failure, or a lookalike + # must raise instead of reading as absent. + ("Error response from daemon: network sandbox-net-x-extra not found", False), + ('context "sandbox-net-x": context not found', False), + ("command not found: docker network inspect", False), + ], +) +def test_inspect_network_uses_strict_missing_network_parser(monkeypatch, stderr, absent): + """The non-strict inspect path shares the strict parser: context-name + collisions and wrong-target errors raise instead of reading as absent.""" + backend = _restricted_backend() + monkeypatch.setattr( + "subprocess.run", + lambda *a, **k: SimpleNamespace(stdout="[]", stderr=stderr, returncode=1), + ) + + if absent: + assert backend._inspect_network("sandbox-net-x") is None + else: + with pytest.raises(RuntimeError): + backend._inspect_network("sandbox-net-x") + + +@pytest.mark.parametrize("error_kind", ["context", "wrong-target", "wrong-target-lowercase", "case-drift", "active-endpoint", "prefix", "suffix", "mixed-stderr", "empty"]) +def test_absent_teardown_preserves_pending_port_on_unknown_network_cleanup_error(monkeypatch, error_kind): + backend = _restricted_backend() + backend._pending_cleanup_ports["old-generation"] = ("existing", 18080) + proxy_name, network_name = backend._resource_names("existing") + errors = { + "context": f'context "{network_name}": context not found', + "wrong-target": f"Error response from daemon: network {network_name}-extra not found", + "wrong-target-lowercase": f"error: network {network_name}-extra not found", + "case-drift": f"error: network {network_name.upper()} not found", + "active-endpoint": f"Error response from daemon: network {network_name} has active endpoints", + "prefix": f"prefix error: network {network_name} not found", + "suffix": f"error: network {network_name} not found suffix", + "mixed-stderr": f"error: network {network_name} not found\nError response from daemon: permission denied", + "empty": "", + } + released = [] + monkeypatch.setattr(local_backend_module, "release_port", released.append) + monkeypatch.setattr(backend, "_stop_container", lambda *_args: None) + + def runtime(cmd, **kwargs): + if cmd == ["docker", "rm", "-f", proxy_name]: + return SimpleNamespace(stdout="", stderr="", returncode=0) + return SimpleNamespace(stdout="", stderr=errors[error_kind], returncode=1) + + monkeypatch.setattr("subprocess.run", runtime) + with pytest.raises(RuntimeError): + backend.complete_absent_teardown("existing") + assert released == [] + assert backend._pending_cleanup_ports == {"old-generation": ("existing", 18080)} + + +@pytest.mark.parametrize("error_style", ["legacy", "lowercase-scaffolding", "uppercase-scaffolding"]) +@pytest.mark.parametrize("sandbox_id", ["existing", "Mixed-Case"]) +def test_absent_teardown_accepts_only_exact_missing_networks(monkeypatch, error_style, sandbox_id): + backend = _restricted_backend() + backend._pending_cleanup_ports["old-generation"] = (sandbox_id, 18080) + released = [] + monkeypatch.setattr(local_backend_module, "release_port", released.append) + monkeypatch.setattr(backend, "_stop_container", lambda *_args: None) + commands = [] + # Docker CLI v29 lowercases client-side error scaffolding; the daemon-side + # wrapper is stable, but both shapes must prove absence identically. + error_formats = { + "legacy": ("Error: No such container: {}", "Error response from daemon: network {} not found"), + "lowercase-scaffolding": ("error: no such object: {}", "error: network {} not found"), + "uppercase-scaffolding": ("ERROR: NO SUCH OBJECT: {}", "ERROR: NETWORK {} NOT FOUND"), + } + container_error, network_error = error_formats[error_style] + + def runtime(cmd, **kwargs): + commands.append(cmd) + if cmd[:3] == ["docker", "rm", "-f"]: + return SimpleNamespace(stdout="", stderr=container_error.format(cmd[-1]), returncode=1) + return SimpleNamespace(stdout="", stderr=network_error.format(cmd[-1]), returncode=1) + + monkeypatch.setattr("subprocess.run", runtime) + backend.complete_absent_teardown(sandbox_id) + assert released == [18080] + assert backend._pending_cleanup_ports == {} + assert ["docker", "network", "rm", backend._resource_names(sandbox_id)[1]] in commands + assert ["docker", "network", "rm", backend._egress_network_name(sandbox_id)] in commands + + +@pytest.mark.parametrize( + ("message", "expected"), + [ + ("Error response from daemon: network sandbox-net not found", True), + ("error response from daemon: network sandbox-net not found", True), + ("error: network sandbox-net not found", True), + ("Error: No such network: sandbox-net", True), + ("error: no such network: sandbox-net", True), + ("No such network: sandbox-net", True), + ("network sandbox-net not found", True), + ("error: network sandbox-net-extra not found", False), + ("error: network Sandbox-Net not found", False), + ("error: no such network: sandbox-other", False), + ("error: network sandbox-net has active endpoints", False), + ('context "sandbox-net": context not found', False), + ("", False), + ], +) +def test_is_no_such_network_error_matches_scaffolding_case_insensitively(message, expected): + assert local_backend_module._is_no_such_network_error(message, "sandbox-net") is expected + + +@pytest.mark.parametrize( + ("message", "expected"), + [ + # Docker <=28 client-side and daemon-side forms. + ("Error: No such object: sandbox-x", True), + ("Error response from daemon: No such container: sandbox-x", True), + ("No such object: sandbox-x", True), + # Docker 29 lowercases the client-side scaffolding. + ("error: no such object: sandbox-x", True), + ("error response from daemon: no such container: sandbox-x", True), + # Scaffolding is normalized; the name stays byte-exact and the whole + # message must match. + ("error: no such object: Sandbox-X", False), + ("error: no such object: sandbox-x-extra", False), + ("error: no such object", False), + ("error: no such image: sandbox-x", False), + ("prefix error: no such object: sandbox-x", False), + ("error: no such object: sandbox-x suffix", False), + # Apple Container forms stay exact-match. + ("Error: container not found: sandbox-x", True), + ('notFound: "container not found: sandbox-x"', True), + ('Error: not found: "sandbox-x"', True), + ("error: container not found: sandbox-x", False), + ("", False), + ], +) +def test_is_no_such_container_error_matches_scaffolding_case_insensitively(message, expected): + assert local_backend_module._is_no_such_container_error(message, "sandbox-x") is expected + + +def test_apple_absent_teardown_releases_own_unknown_generation_port_after_timeout(monkeypatch): + backend = _backend_for_inspect_tests() + backend._runtime = "container" + released = [] + monkeypatch.setattr(local_backend_module, "release_port", released.append) + + def runtime(cmd, **kwargs): + if "stop" in cmd: + raise subprocess.TimeoutExpired(cmd, 120) + assert cmd[:2] == ["container", "inspect"] + return SimpleNamespace(stdout="[]", stderr="Error: container not found: sandbox-existing", returncode=1) + + monkeypatch.setattr("subprocess.run", runtime) + with pytest.raises(subprocess.TimeoutExpired): + backend.destroy(SandboxInfo("other", "http://localhost:18081", container_name="sandbox-other")) + with pytest.raises(subprocess.TimeoutExpired): + backend.destroy(SandboxInfo("existing", "http://localhost:18080", container_name="sandbox-existing")) + assert backend.inspect_runtime("existing") is None + assert backend.is_absent("existing") is True + assert released == [] + backend.complete_absent_teardown("existing") + backend.complete_absent_teardown("existing") + assert released == [18080] + backend.complete_absent_teardown("other") + assert released == [18080, 18081] + + +def test_apple_present_unknown_generation_does_not_inherit_pending_port(monkeypatch): + backend = _backend_for_inspect_tests() + backend._runtime = "container" + released = [] + monkeypatch.setattr(local_backend_module, "release_port", released.append) + + def runtime(cmd, **kwargs): + if "stop" in cmd: + raise subprocess.TimeoutExpired(cmd, 120) + entry = {"configuration": {"id": "sandbox-existing"}, "status": "running"} + return SimpleNamespace(stdout=json.dumps([entry]), stderr="", returncode=0) + + monkeypatch.setattr("subprocess.run", runtime) + with pytest.raises(subprocess.TimeoutExpired): + backend.destroy(SandboxInfo("existing", "http://localhost:18080", container_name="sandbox-existing")) + info = backend.inspect_runtime("existing") + assert info is not None + assert info.container_id is None + assert info.sandbox_url == "" + assert backend.is_absent("existing") is False + assert released == [] + + +@pytest.mark.parametrize("runtime", ["container", "docker"]) +def test_successful_unknown_generation_retry_does_not_release_a_reused_port_again(monkeypatch, runtime): + from deerflow.utils.network import PortAllocator + + backend = _backend_for_inspect_tests() + backend._runtime = runtime + allocator = PortAllocator() + monkeypatch.setattr(allocator, "_is_port_available", lambda port: port not in allocator._reserved_ports) + port = allocator.allocate(18080, max_range=1) + failed = False + released = [] + + def release(port): + released.append(port) + allocator.release(port) + + def stop(_target): + nonlocal failed + if not failed: + failed = True + raise subprocess.TimeoutExpired("stop", 120) + + monkeypatch.setattr(backend, "_stop_container", stop) + monkeypatch.setattr(local_backend_module, "release_port", release) + monkeypatch.setattr("subprocess.run", lambda *_args, **_kwargs: SimpleNamespace(stdout="", stderr="", returncode=0)) + original = SandboxInfo("existing", f"http://localhost:{port}", container_name="sandbox-existing") + with pytest.raises(subprocess.TimeoutExpired): + backend.destroy(original) + # Old Docker metadata can learn its immutable generation on a retry. + if runtime == "docker": + original.container_id = "learned-generation" + backend.destroy(original) + assert allocator.allocate(18080, max_range=1) == port + backend.complete_absent_teardown("existing") + assert released == [port] + with pytest.raises(RuntimeError, match="No available port"): + allocator.allocate(18080, max_range=1) + + +def test_absent_teardown_deduplicates_a_legacy_reservation_that_later_learns_its_generation(monkeypatch): + backend = _backend_for_inspect_tests() + released = [] + monkeypatch.setattr(local_backend_module, "release_port", released.append) + monkeypatch.setattr("subprocess.run", lambda *_args, **_kwargs: SimpleNamespace(stdout="", stderr="", returncode=0)) + + def timeout(_target): + raise subprocess.TimeoutExpired("stop", 120) + + monkeypatch.setattr(backend, "_stop_container", timeout) + info = SandboxInfo("existing", "http://localhost:18080", container_name="sandbox-existing") + with pytest.raises(subprocess.TimeoutExpired): + backend.destroy(info) + info.container_id = "learned-generation" + with pytest.raises(subprocess.TimeoutExpired): + backend.destroy(info) + monkeypatch.setattr(backend, "_stop_container", lambda *_args: None) + backend.complete_absent_teardown("existing") + backend.complete_absent_teardown("existing") + assert released == [18080] + + +@pytest.mark.parametrize("failed_stage", ["proxy-stop", "proxy-remove", "main-stop"]) +def test_restricted_partial_destroy_keeps_a_retryable_main_and_releases_port_once(monkeypatch, failed_stage): + from deerflow.utils.network import PortAllocator + + backend = _backend_for_inspect_tests() + backend._network_mode = "allowlist" + allocator = PortAllocator() + monkeypatch.setattr(allocator, "_is_port_available", lambda port: port not in allocator._reserved_ports) + port = allocator.allocate(18080, max_range=1) + released = [] + + def release(port): + released.append(port) + allocator.release(port) + + monkeypatch.setattr(local_backend_module, "release_port", release) + proxy_name, _ = backend._resource_names("existing") + main = {"Id": "main-generation", "Name": "/sandbox-existing", "Config": {"Labels": {"deerflow.network_mode": "allowlist"}}} + proxy = {"Id": "proxy-generation", "Name": f"/{proxy_name}", "HostConfig": {"PortBindings": {"8080/tcp": [{"HostPort": str(port)}]}}} + removed = set() + failed = False + + def runtime(cmd, **kwargs): + nonlocal failed + target = cmd[-1] + if "inspect" in cmd: + if target in removed: + return SimpleNamespace(stdout="[]", stderr=f"Error: No such object: {target}", returncode=1) + return SimpleNamespace(stdout=json.dumps([proxy if target == proxy_name else main]), stderr="", returncode=0) + stage = "proxy-remove" if cmd[:3] == ["docker", "rm", "-f"] else "proxy-stop" if "stop" in cmd and target == proxy_name else "main-stop" if "stop" in cmd else None + if stage == failed_stage and not failed: + failed = True + if kwargs.get("check"): + raise subprocess.CalledProcessError(1, cmd, stderr="daemon denied teardown") + return SimpleNamespace(stdout="", stderr="daemon denied teardown", returncode=1) + if stage == "main-stop": + removed.add("sandbox-existing") + elif stage == "proxy-remove": + removed.add(proxy_name) + return SimpleNamespace(stdout="", stderr="", returncode=0) + + monkeypatch.setattr("subprocess.run", runtime) + original = SandboxInfo("existing", f"http://localhost:{port}", container_name="sandbox-existing", container_id="main-generation") + with pytest.raises((RuntimeError, subprocess.CalledProcessError)): + backend.destroy(original) + assert backend.is_absent("existing") is False + assert released == [] + with pytest.raises(RuntimeError, match="No available port"): + allocator.allocate(18080, max_range=1) + recovered = backend.inspect_runtime("existing") + assert recovered is not None + backend.destroy(recovered) + assert allocator.allocate(18080, max_range=1) == port + assert released == [port] + + +def test_apple_list_does_not_mark_unsupported_runtime_metadata_for_replacement(monkeypatch): + backend = _backend_for_inspect_tests() + backend._runtime = "container" + entry = {"configuration": {"id": "sandbox-existing"}, "status": "running"} + + def runtime(cmd, **kwargs): + return SimpleNamespace(stdout="sandbox-existing\n" if "ps" in cmd else json.dumps([entry]), stderr="", returncode=0) + + monkeypatch.setattr("subprocess.run", runtime) + assert backend.list_running() == [] + assert backend.inspect_runtime("existing") is not None + + +def test_apple_batch_preserves_legacy_docker_shaped_metadata(monkeypatch): + backend = _backend_for_inspect_tests() + backend._runtime = "container" + entry = {"Name": "/sandbox-existing", "NetworkSettings": {"Ports": {"8080/tcp": [{"HostPort": "18080"}]}}} + monkeypatch.setattr("subprocess.run", lambda *_args, **_kwargs: SimpleNamespace(stdout=json.dumps([entry]), stderr="", returncode=0)) + assert backend._batch_inspect(["sandbox-existing"], strict=True)["sandbox-existing"].host_port == 18080 + + +@pytest.mark.parametrize("runtime", ["docker", "container"]) +def test_inspect_runtime_only_returns_none_for_authoritative_absence(monkeypatch, runtime): + backend = _backend_for_inspect_tests() + backend._runtime = runtime + error = "Error: No such object: sandbox-missing" if runtime == "docker" else "Error: container not found: sandbox-missing" + monkeypatch.setattr("subprocess.run", lambda *_args, **_kwargs: SimpleNamespace(stdout="[]", stderr=error, returncode=1)) + + assert backend.inspect_runtime("missing") is None + + +@pytest.mark.parametrize( + "payload", + [ + "", + "[]", + "{}", + "not JSON", + json.dumps([{"Name": "/sandbox-other", "Id": "generation"}]), + json.dumps([{"Name": "/sandbox-existing"}]), + json.dumps([{"Name": "/sandbox-existing", "Id": 42}]), + json.dumps([{"Name": "/sandbox-existing", "Id": "generation"}, {"Name": "/sandbox-existing", "Id": "other-generation"}]), + ], +) +def test_inspect_runtime_fails_closed_on_incomplete_or_invalid_docker_response(monkeypatch, payload): + backend = _backend_for_inspect_tests() + monkeypatch.setattr("subprocess.run", lambda *_args, **_kwargs: SimpleNamespace(stdout=payload, stderr="", returncode=0)) + + with pytest.raises(RuntimeError): + backend.inspect_runtime("existing") + with pytest.raises(RuntimeError): + backend.is_absent("existing") + + +@pytest.mark.parametrize("failure", [OSError("runtime unavailable"), subprocess.TimeoutExpired("inspect", 5)]) +def test_inspect_runtime_propagates_runtime_failures(monkeypatch, failure): + backend = _backend_for_inspect_tests() + + def failed(*_args, **_kwargs): + raise failure + + monkeypatch.setattr("subprocess.run", failed) + with pytest.raises(RuntimeError): + backend.inspect_runtime("existing") + + +def test_batch_inspect_keeps_present_generations_with_exact_partial_absence(monkeypatch): + backend = _backend_for_inspect_tests() + entry = {"Name": "/sandbox-existing", "Id": "immutable-generation"} + monkeypatch.setattr("subprocess.run", lambda *_args, **_kwargs: SimpleNamespace(stdout=json.dumps([entry]), stderr="Error: No such object: sandbox-missing", returncode=1)) + + inspections = backend._batch_inspect(["sandbox-existing", "sandbox-missing"], strict=True) + + assert list(inspections) == ["sandbox-existing"] + assert inspections["sandbox-existing"].container_id == "immutable-generation" + + +def test_batch_inspect_does_not_accept_absence_of_an_unrequested_resource(monkeypatch): + backend = _backend_for_inspect_tests() + monkeypatch.setattr("subprocess.run", lambda *_args, **_kwargs: SimpleNamespace(stdout="[]", stderr="Error: No such object: sandbox-other", returncode=1)) + + with pytest.raises(RuntimeError): + backend._batch_inspect(["sandbox-existing"], strict=True) + + def test_stop_container_passes_a_timeout(monkeypatch): """An unbounded `stop` can outlive the teardown lease that guards it. @@ -1862,6 +2612,43 @@ def test_stop_container_passes_a_timeout(monkeypatch): assert seen.get("timeout") == backend._STOP_TIMEOUT_SECONDS +def test_failed_stop_is_not_reported_as_destroyed(monkeypatch): + backend = _backend_for_inspect_tests() + + def denied(cmd, **_kwargs): + raise subprocess.CalledProcessError(1, cmd, stderr="daemon denied stop") + + monkeypatch.setattr("subprocess.run", denied) + with pytest.raises(subprocess.CalledProcessError): + backend.destroy(SandboxInfo("sandbox", "http://localhost:18080", container_id="generation")) + + +def test_docker_discovery_preserves_runtime_generation(monkeypatch): + import json + + import requests + + backend = _backend_for_inspect_tests() + payload = [ + { + "Id": "docker-generation", + "Name": "/sandbox-existing", + "Config": {"Image": "sandbox:latest", "Labels": {"deerflow.role": "sandbox", "deerflow.sandbox_id": "existing", "deerflow.network_mode": "open"}}, + "NetworkSettings": {"Ports": {"8080/tcp": [{"HostPort": "18080"}]}}, + } + ] + + def runtime(cmd, **_kwargs): + return SimpleNamespace(returncode=0, stdout="true" if "-f" in cmd else json.dumps(payload), stderr="") + + monkeypatch.setattr("subprocess.run", runtime) + monkeypatch.setattr(requests.Session, "get", lambda *_args, **_kwargs: SimpleNamespace(status_code=200)) + + info = backend.discover("existing") + assert info is not None + assert info.container_id == "docker-generation" + + def test_stop_container_propagates_a_timeout_instead_of_reporting_success(monkeypatch): """A timed-out stop must not be swallowed like a failed one. diff --git a/backend/tests/test_aio_sandbox_provider.py b/backend/tests/test_aio_sandbox_provider.py index 48bc3d7d37..534efa5d7f 100644 --- a/backend/tests/test_aio_sandbox_provider.py +++ b/backend/tests/test_aio_sandbox_provider.py @@ -8,8 +8,9 @@ import os import stat import threading from types import SimpleNamespace -from unittest.mock import ANY, MagicMock, patch +from unittest.mock import ANY, AsyncMock, MagicMock, patch +import httpx import pytest from _windows_acl_helpers import _windows_acl_owner_sid, _windows_acl_sids from pydantic import ValidationError @@ -121,6 +122,7 @@ def test_register_created_sandbox_forwards_configured_command_timeout(tmp_path): base_url="http://sandbox", request_headers=info.request_headers, default_command_timeout=42, + lark_cli_broker=False, ) @@ -282,16 +284,20 @@ def _make_provider(tmp_path): ``test_sandbox_ownership_store.py`` (store contract). """ from deerflow.community.aio_sandbox.ownership.memory import MemoryOwnershipStore + from deerflow.community.aio_sandbox.quarantine import SandboxQuarantine from deerflow.config.sandbox_config import SandboxOwnershipConfig aio_mod = importlib.import_module("deerflow.community.aio_sandbox.aio_sandbox_provider") with patch.object(aio_mod.AioSandboxProvider, "_start_idle_checker"): provider = aio_mod.AioSandboxProvider.__new__(aio_mod.AioSandboxProvider) provider._config = {"command_timeout": 600.0, "idle_timeout": 600, "replicas": 3} + provider._backend = SimpleNamespace() + provider._quarantine = SandboxQuarantine(tmp_path / "quarantine", "test") provider._sandboxes = {} provider._active_sandbox_identity = {} provider._warm_pool_identity = {} provider._local_teardown = set() + provider._unowned_since = {} provider._acquire_epoch = {} provider._acquire_epoch_counter = 0 provider._acquire_inflight = {} @@ -586,31 +592,42 @@ def test_shared_sandbox_identity_changes_when_custom_skills_root_changes( assert second_root_id != first_root_id +@pytest.mark.parametrize("active_holder", [False, True], ids=["idle", "in-use"]) def test_cached_sandbox_is_replaced_when_expected_identity_changes( tmp_path, monkeypatch, + active_holder, ): - provider = _make_provider(tmp_path) + from deerflow.sandbox.lease import get_sandbox_lease_manager + + provider, sandbox, aio_mod = _make_provider_with_active_sandbox(tmp_path, "stale-root-id") provider._config["skills_container_path"] = "/custom-skills" provider._thread_sandboxes = {("alice", "thread-shared"): "stale-root-id"} - provider._sandboxes = {"stale-root-id": object()} - provider._sandbox_infos = {} monkeypatch.setattr( provider, "_sandbox_id_for_thread", lambda *_args, **_kwargs: "new-root-id", ) - destroy = MagicMock() - monkeypatch.setattr(provider, "destroy", destroy) - - assert ( - provider._reuse_in_process_sandbox( - "thread-shared", - user_id="alice", - ) - is None - ) - destroy.assert_called_once_with("stale-root-id") + manager = get_sandbox_lease_manager(provider) + try: + if active_holder: + manager.retain("live-run", "stale-root-id", thread_id="thread-shared", user_id="alice") + with pytest.raises(aio_mod.SandboxPolicyReplacementDeferredError, match="outdated skills identity"): + provider._reuse_in_process_sandbox("thread-shared", user_id="alice") + assert provider.get("stale-root-id") is sandbox + sandbox.close.assert_not_called() + provider._backend.destroy.assert_not_called() + manager.release("live-run") + else: + info = provider._sandbox_infos["stale-root-id"] + assert provider._reuse_in_process_sandbox("thread-shared", user_id="alice") is None + sandbox.close.assert_called_once_with() + provider._backend.destroy.assert_called_once_with(info) + assert "stale-root-id" not in provider._sandboxes + assert ("alice", "thread-shared") not in provider._thread_sandboxes + finally: + provider.reset() + provider._ownership.close() def test_policy_scoped_create_excludes_local_config_mounts_below_skills_root( @@ -867,10 +884,10 @@ def test_discover_or_create_only_unlocks_when_lock_succeeds(tmp_path, monkeypatc @pytest.mark.anyio -async def test_acquire_async_uses_async_readiness_polling(monkeypatch): +async def test_acquire_async_uses_async_readiness_polling(tmp_path, monkeypatch): """AioSandboxProvider async creation must not use sync readiness polling.""" aio_mod = importlib.import_module("deerflow.community.aio_sandbox.aio_sandbox_provider") - provider = _make_provider(None) + provider = _make_provider(tmp_path) provider._config = {"command_timeout": 600.0, "replicas": 3} provider._warm_pool = {} provider._sandbox_infos = {} @@ -1819,6 +1836,26 @@ def test_create_sandbox_skips_runtime_when_lark_absent(tmp_path, monkeypatch): assert captured["provision_lark_cli_broker"] is False +@pytest.mark.asyncio +@pytest.mark.parametrize("async_create", [False, True]) +async def test_unknown_broker_probe_stops_creation_before_mounts(tmp_path, monkeypatch, async_create): + from deerflow.integrations import lark_cli + + provider, _, provider_mod = _make_provider_with_active_sandbox(tmp_path, "sandbox-probe") + monkeypatch.setattr(provider, "_get_extra_mounts", lambda *_args, **_kwargs: [("credentials", "/mnt/integrations/lark-cli/config", True)]) + monkeypatch.setattr(provider_mod.AioSandboxProvider, "_lark_integration_active", staticmethod(lambda *_args: True)) + monkeypatch.setattr(lark_cli, "sandbox_lark_broker_active", MagicMock(side_effect=RuntimeError("Provisioner capability is unknown"))) + provider._backend.create = MagicMock() + + with pytest.raises(RuntimeError, match="capability is unknown"): + if async_create: + await provider._create_sandbox_async("thread", "sandbox", user_id="alice") + else: + await asyncio.to_thread(provider._create_sandbox, "thread", "sandbox", user_id="alice") + provider._backend.create.assert_not_called() + provider.reset() + + # ── Sandbox client teardown (#2872) ────────────────────────────────────────── @@ -1849,6 +1886,1068 @@ def _make_provider_with_active_sandbox(tmp_path, sandbox_id: str): return provider, sandbox, aio_mod +@pytest.fixture() +def failure_recovery_lifecycle(tmp_path, monkeypatch): + """Keep server-side shell state while release/reclaim replaces SDK clients.""" + from deerflow.community.aio_sandbox import aio_sandbox as sandbox_mod + + provider, _, provider_mod = _make_provider_with_active_sandbox(tmp_path, "sandbox-failure-recovery") + provider._backend.complete_absent_teardown = MagicMock() + info = provider._sandbox_infos["sandbox-failure-recovery"] + info.sandbox_url = "https://sandbox.example.test" + info.container_id = "container-generation-1" + shell = MagicMock(spec=["exec_command", "create_session", "cleanup_session"]) + shell.exec_command.return_value = SimpleNamespace(data=SimpleNamespace(output="next-turn", exit_code=0, status="completed")) + shell.create_session.side_effect = lambda id, **_kwargs: SimpleNamespace(data=SimpleNamespace(session_id=id)) + monkeypatch.setattr(sandbox_mod, "AioSandboxClient", lambda **_kwargs: SimpleNamespace(shell=shell, close=MagicMock())) + sandbox = sandbox_mod.AioSandbox(id=info.sandbox_id, base_url=info.sandbox_url) + sandbox.lark_cli_broker = info.lark_cli_broker + provider._sandboxes[info.sandbox_id] = sandbox + provider._thread_sandboxes[("alice", "thread-failure-recovery")] = info.sandbox_id + provider._active_sandbox_identity[info.sandbox_id] = ("alice", "thread-failure-recovery") + monkeypatch.setattr(provider, "_check_tracked_sandbox_alive", lambda *_args: True) + monkeypatch.setattr(provider, "_ensure_skills_projection", lambda *_args: None) + monkeypatch.setattr(provider, "_thread_skill_projection_active", lambda *_args: False) + monkeypatch.setattr(provider, "_sandbox_id_for_thread", lambda *_args: info.sandbox_id) + monkeypatch.setattr(provider_mod, "get_paths", lambda: Paths(base_dir=tmp_path / "state")) + + try: + yield provider, sandbox, shell, info + finally: + for active_sandbox in list(provider._sandboxes.values()): + active_sandbox.close() + sandbox.close() + provider.reset() + provider._ownership.close() + + +@pytest.mark.parametrize( + "outcome", + [ + pytest.param( + "read_timeout", + id="transport-timeout", + ), + pytest.param( + "no_change_timeout", + id="no-change-timeout", + ), + pytest.param("hard_timeout", id="confirmed-termination-stays-warm"), + pytest.param("completed", id="completed-command-stays-warm"), + ], +) +def test_default_shell_outcome_release_reclaim_preserves_isolation(failure_recovery_lifecycle, outcome): + """A new client must not revive a fenced implicit session in the same container.""" + provider, sandbox, shell, info = failure_recovery_lifecycle + ambiguous = outcome in ("read_timeout", "no_change_timeout") + if outcome == "read_timeout": + first_response = httpx.ReadTimeout("command response timed out") + else: + first_exit_code = {"no_change_timeout": None, "hard_timeout": 124, "completed": 0}[outcome] + first_response = SimpleNamespace(data=SimpleNamespace(output="first-turn", exit_code=first_exit_code, status=outcome)) + shell.exec_command.side_effect = [first_response, shell.exec_command.return_value] + + first_output = sandbox.execute_command("first-command") + assert shell.exec_command.call_count == 1, "an ambiguous command must not be replayed" + assert shell.exec_command.call_args.kwargs.get("id") is None + if ambiguous: + assert "outcome is unknown" in first_output.lower() + elif outcome == "hard_timeout": + assert "Exit Code: 124" in first_output + + provider.release(info.sandbox_id) + reclaimed_id = provider._reclaim_warm_pool_sandbox("thread-failure-recovery", info.sandbox_id, user_id="alice") + if reclaimed_id is not None: + next_sandbox = provider.get(reclaimed_id) + assert next_sandbox is not sandbox + assert next_sandbox.execute_command("next-command") == "next-turn" + + implicit_commands = [call.kwargs["command"] for call in shell.exec_command.call_args_list if call.kwargs.get("id") is None] + if ambiguous: + # Either recycle the container or preserve the fence and use a new + # explicit session. Re-entering the old implicit shell is unsafe. + assert implicit_commands == ["first-command"], "release/reclaim must not erase the implicit-session fence" + else: + assert reclaimed_id == info.sandbox_id + assert implicit_commands == ["first-command", "next-command"] + provider._backend.destroy.assert_not_called() + + +def test_failed_dirty_release_cannot_readopt_same_container(failure_recovery_lifecycle): + """A failed stop must not make the same server generation safe to acquire.""" + provider, sandbox, shell, info = failure_recovery_lifecycle + shell.exec_command.side_effect = httpx.ReadTimeout("command response timed out") + assert "outcome is unknown" in sandbox.execute_command("first-command").lower() + shell.create_session.side_effect = httpx.ReadTimeout("recovery create response timed out") + assert "session creation outcome is unknown" in sandbox.execute_command("recovery-command") + assert sandbox.requires_container_recycle is True + provider._backend.destroy.side_effect = RuntimeError("container stop failed") + provider._backend.discover = MagicMock(return_value=info) + provider._backend.create = MagicMock(side_effect=AssertionError("the quarantined generation is still running")) + + provider.release(info.sandbox_id) + provider._backend.destroy.assert_called_once_with(info) + assert info.sandbox_id not in provider._warm_pool + + try: + acquired_id = provider.acquire("thread-failure-recovery", user_id="alice") + except RuntimeError as error: + # Unrelated acquire failures must not masquerade as a quarantine fix. + message = str(error).lower() + assert any(reason in message for reason in ("quarantin", "recycl", "incompatible provisioning policy")) + return + + assert acquired_id != info.sandbox_id, "a failed destroy must not permit discovery to clear the same container's quarantine" + + +def test_quarantined_generation_stays_fenced_after_provider_restart(failure_recovery_lifecycle, tmp_path): + provider, sandbox, shell, info = failure_recovery_lifecycle + shell.exec_command.side_effect = httpx.ReadTimeout("command response timed out") + sandbox.execute_command("uncertain-command") + provider._backend.destroy.side_effect = RuntimeError("stop failed") + provider.release(info.sandbox_id) + + restarted, old_client, _ = _make_provider_with_active_sandbox(tmp_path, info.sandbox_id) + restarted._sandboxes.clear() + restarted._sandbox_infos.clear() + old_client.close() + restarted._ensure_skills_projection = lambda *_args: None + restarted._thread_skill_projection_active = lambda *_args: False + restarted._sandbox_id_for_thread = lambda *_args: info.sandbox_id + # Fresh metadata intentionally drops all in-memory flags, as discovery does. + restarted._backend.discover = MagicMock(return_value=type(info).from_dict(info.to_dict())) + restarted._backend.destroy.side_effect = RuntimeError("stop still failing") + restarted._backend.create = MagicMock(side_effect=AssertionError("old generation is still running")) + try: + with pytest.raises(RuntimeError, match="incompatible provisioning policy"): + restarted.acquire("thread-failure-recovery", user_id="alice") + + restarted._backend.discover.return_value = type(info)(info.sandbox_id, info.sandbox_url, container_id="container-generation-2") + assert restarted.acquire("thread-failure-recovery", user_id="alice") == info.sandbox_id + assert restarted._sandbox_infos[info.sandbox_id].container_id == "container-generation-2" + restarted._backend.create.assert_not_called() + finally: + for active in restarted._sandboxes.values(): + active.close() + restarted.reset() + + +def test_failed_quarantine_write_keeps_dirty_container_tracked(failure_recovery_lifecycle, monkeypatch): + from pathlib import Path + + provider, sandbox, shell, info = failure_recovery_lifecycle + shell.exec_command.side_effect = httpx.ReadTimeout("command response timed out") + sandbox.execute_command("uncertain-command") + original_open = Path.open + with monkeypatch.context() as patcher: + patcher.setattr(Path, "open", MagicMock(side_effect=PermissionError("quarantine unavailable"))) + provider.release(info.sandbox_id) + provider._backend.destroy.assert_not_called() + assert provider.get(info.sandbox_id) is sandbox + with pytest.raises(PermissionError): + provider.destroy(info.sandbox_id) + assert provider.get(info.sandbox_id) is sandbox + assert Path.open is original_open + provider._backend.destroy.side_effect = RuntimeError("stop failed") + with pytest.raises(RuntimeError, match="stop failed"): + provider.destroy(info.sandbox_id) + assert provider._quarantine_store().contains(info) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("async_acquire", [False, True]) +@pytest.mark.parametrize("new_generation", [None, "replacement-generation"]) +async def test_unknown_generation_can_be_replaced_after_confirmed_absence(failure_recovery_lifecycle, monkeypatch, async_acquire, new_generation): + provider, sandbox, shell, info = failure_recovery_lifecycle + info.container_id = None # Apple Container discovery / older provisioner. + shell.exec_command.side_effect = httpx.ReadTimeout("command response timed out") + sandbox.execute_command("uncertain-command") + provider.release(info.sandbox_id) + assert provider._quarantine_store().contains(info) + fresh = type(info)(info.sandbox_id, info.sandbox_url, container_id=new_generation) + provider._backend.discover = MagicMock(return_value=None) + provider._backend.create = MagicMock(return_value=fresh) + + def confirm_absence(_sandbox_id): + assert provider._ownership.owner(info.sandbox_id) == provider._owner_id + assert info.sandbox_id in provider._local_teardown + return True + + provider._backend.is_absent = MagicMock(side_effect=confirm_absence) + monkeypatch.setattr(provider, "_get_extra_mounts", lambda *_args, **_kwargs: []) + monkeypatch.setattr(type(provider), "_lark_integration_active", staticmethod(lambda *_args: False)) + monkeypatch.setattr(importlib.import_module(type(provider).__module__), "wait_for_sandbox_ready", lambda *_args, **_kwargs: True) + monkeypatch.setattr(importlib.import_module(type(provider).__module__), "wait_for_sandbox_ready_async", AsyncMock(return_value=True)) + + if async_acquire: + acquired = await provider.acquire_async("thread-failure-recovery", user_id="alice") + else: + acquired = provider.acquire("thread-failure-recovery", user_id="alice") + + assert acquired == info.sandbox_id + assert provider._sandbox_infos[acquired] is fresh + assert not provider._quarantine_store().contains(fresh) + provider._backend.is_absent.assert_called_once_with(info.sandbox_id) + + +@pytest.mark.parametrize("blocked_by", ["existing-runtime", "probe-error", "peer"]) +def test_quarantine_retirement_cannot_bypass_absence_or_ownership(failure_recovery_lifecycle, monkeypatch, blocked_by): + provider, sandbox, shell, info = failure_recovery_lifecycle + info.container_id = None + shell.exec_command.side_effect = httpx.ReadTimeout("command response timed out") + sandbox.execute_command("uncertain-command") + provider.release(info.sandbox_id) + provider._backend.create = MagicMock(side_effect=AssertionError("quarantine must be checked before creation")) + provider._backend.is_absent = MagicMock(return_value=blocked_by == "peer") + if blocked_by == "probe-error": + provider._backend.is_absent.side_effect = RuntimeError("runtime unavailable") + if blocked_by == "peer": + monkeypatch.setattr(provider, "_claim_ownership", lambda *_args, **_kwargs: False) + monkeypatch.setattr(provider, "_get_extra_mounts", lambda *_args, **_kwargs: []) + monkeypatch.setattr(type(provider), "_lark_integration_active", staticmethod(lambda *_args: False)) + + with pytest.raises(RuntimeError): + provider._create_sandbox("thread-failure-recovery", info.sandbox_id, user_id="alice") + + provider._backend.create.assert_not_called() + assert provider._quarantine_store().contains(info) + assert info.sandbox_id not in provider._local_teardown + if blocked_by == "peer": + provider._backend.is_absent.assert_not_called() + + +def _prepare_quarantined_runtime_retry(lifecycle, monkeypatch, current): + provider, sandbox, shell, old = lifecycle + shell.exec_command.side_effect = httpx.ReadTimeout("uncertain command") + sandbox.execute_command("uncertain-command") + provider._backend.destroy.side_effect = RuntimeError("temporary stop failure") + provider.release(old.sandbox_id) + provider._backend.destroy.reset_mock() + provider._backend.destroy.side_effect = None + provider._backend.discover = MagicMock(return_value=None) + provider._backend.inspect_runtime = MagicMock(return_value=current) + provider._backend.create = MagicMock(return_value=current) + provider._backend.is_absent = MagicMock(return_value=False) + monkeypatch.setattr(provider, "_get_extra_mounts", lambda *_args, **_kwargs: []) + monkeypatch.setattr(type(provider), "_lark_integration_active", staticmethod(lambda *_args: False)) + provider_mod = importlib.import_module(type(provider).__module__) + monkeypatch.setattr(provider_mod, "wait_for_sandbox_ready", lambda *_args, **_kwargs: True) + monkeypatch.setattr(provider_mod, "wait_for_sandbox_ready_async", AsyncMock(return_value=True)) + return provider, old + + +@pytest.mark.asyncio +@pytest.mark.parametrize("async_acquire", [False, True], ids=["sync", "async"]) +@pytest.mark.parametrize("initially_absent", [False, True], ids=["after-delete", "already-absent"]) +@pytest.mark.parametrize("cleanup_failure", [False, True], ids=["complete", "cleanup-failed"]) +async def test_absent_teardown_completion_precedes_quarantine_retirement_under_fences(failure_recovery_lifecycle, monkeypatch, async_acquire, initially_absent, cleanup_failure): + old = failure_recovery_lifecycle[3] + current = type(old)(old.sandbox_id, "", container_id=old.container_id) + provider, old = _prepare_quarantined_runtime_retry(failure_recovery_lifecycle, monkeypatch, current) + fresh = type(old)(old.sandbox_id, old.sandbox_url, container_id="new-generation") + provider._backend.create.return_value = fresh + provider._backend.is_absent.side_effect = [True] if initially_absent else [False, True] + completed = [] + + def complete_absent_teardown(sandbox_id): + assert sandbox_id == old.sandbox_id + assert provider._backend.is_absent.call_count == (1 if initially_absent else 2) + lease = provider._ownership._leases[sandbox_id] + assert lease.owner_id == provider._owner_id and lease.destroying is True + assert sandbox_id in provider._local_teardown + assert provider._quarantine_store().contains(old) + provider._backend.create.assert_not_called() + completed.append(sandbox_id) + if cleanup_failure: + raise RuntimeError("absent runtime cleanup incomplete") + + provider._backend.complete_absent_teardown.side_effect = complete_absent_teardown + + async def acquire(): + if async_acquire: + return await provider.acquire_async("thread-failure-recovery", user_id="alice") + return await asyncio.to_thread(provider.acquire, "thread-failure-recovery", user_id="alice") + + if cleanup_failure: + with pytest.raises(RuntimeError, match="absent runtime cleanup incomplete"): + await acquire() + assert provider._quarantine_store().contains(old) + provider._backend.create.assert_not_called() + else: + assert await acquire() == old.sandbox_id + assert provider._sandbox_infos[old.sandbox_id] is fresh + assert not provider._quarantine_store().contains(old) + assert completed == [old.sandbox_id] + provider._backend.complete_absent_teardown.assert_called_once_with(old.sandbox_id) + assert old.sandbox_id not in provider._local_teardown + + +@pytest.mark.asyncio +@pytest.mark.parametrize("async_acquire", [False, True], ids=["sync", "async"]) +@pytest.mark.parametrize("generation", ["container-generation-1", None], ids=["known-generation", "unknown-generation"]) +async def test_quarantined_runtime_without_endpoint_is_deleted_before_replacement(failure_recovery_lifecycle, monkeypatch, async_acquire, generation): + old = failure_recovery_lifecycle[3] + current = type(old)(old.sandbox_id, "", container_id=generation) + provider, old = _prepare_quarantined_runtime_retry(failure_recovery_lifecycle, monkeypatch, current) + fresh = type(old)(old.sandbox_id, old.sandbox_url, container_id="new-generation") + provider._backend.create.return_value = fresh + operations = [] + + def confirm_absence(sandbox_id): + assert sandbox_id == old.sandbox_id + assert provider._ownership.owner(sandbox_id) == provider._owner_id + assert sandbox_id in provider._local_teardown + operations.append("absence") + return operations.count("absence") == 2 + + def inspect_runtime(sandbox_id): + assert provider._ownership.owner(sandbox_id) == provider._owner_id + assert sandbox_id in provider._local_teardown + operations.append("inspect") + return current + + def destroy(info): + assert info is current + # Runtime inspection restores identity, not the original cleanup intent. + # Force full replacement cleanup even after the deployment switches open. + assert info.requires_replacement is True + assert provider._ownership.owner(info.sandbox_id) == provider._owner_id + assert info.sandbox_id in provider._local_teardown + operations.append("destroy") + + provider._backend.is_absent.side_effect = confirm_absence + provider._backend.inspect_runtime.side_effect = inspect_runtime + provider._backend.destroy.side_effect = destroy + + if async_acquire: + acquired = await provider.acquire_async("thread-failure-recovery", user_id="alice") + else: + acquired = await asyncio.to_thread(provider.acquire, "thread-failure-recovery", user_id="alice") + + assert acquired == old.sandbox_id + assert operations == ["absence", "inspect", "destroy", "absence"] + assert provider._sandbox_infos[acquired] is fresh + assert not provider._quarantine_store().contains(old) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("blocked_step", ["destroy", "absent-completion"]) +async def test_cancelled_quarantine_retry_drains_cleanup_before_releasing_fences(failure_recovery_lifecycle, monkeypatch, blocked_step): + old = failure_recovery_lifecycle[3] + current = type(old)(old.sandbox_id, "", container_id=old.container_id) + provider, old = _prepare_quarantined_runtime_retry(failure_recovery_lifecycle, monkeypatch, current) + fresh = type(old)(old.sandbox_id, old.sandbox_url, container_id="new-generation") + provider._backend.create.return_value = fresh + cleanup_started = threading.Event() + finish_cleanup = threading.Event() + cleanup_finished = threading.Event() + unlocked = threading.Event() + provider._backend.is_absent.side_effect = [False, True] + provider_mod = importlib.import_module(type(provider).__module__) + original_unlock = provider_mod._unlock_file + + def wait_cleanup(): + lease = provider._ownership._leases[old.sandbox_id] + assert lease.owner_id == provider._owner_id and lease.destroying is True + assert old.sandbox_id in provider._local_teardown + assert provider._quarantine_store().contains(old) + cleanup_started.set() + assert finish_cleanup.wait(5) + cleanup_finished.set() + + def destroy(info): + assert info is current + assert info.requires_replacement is True + if blocked_step == "destroy": + wait_cleanup() + + def complete_absent_teardown(sandbox_id): + assert sandbox_id == old.sandbox_id + assert provider._backend.is_absent.call_count == 2 + if blocked_step == "absent-completion": + wait_cleanup() + + def unlock(lock_file): + assert cleanup_finished.is_set() + assert not provider._quarantine_store().contains(old) + original_unlock(lock_file) + unlocked.set() + + provider._backend.destroy.side_effect = destroy + provider._backend.complete_absent_teardown.side_effect = complete_absent_teardown + monkeypatch.setattr(provider_mod, "_unlock_file", unlock) + task = asyncio.create_task(provider.acquire_async("thread-failure-recovery", user_id="alice")) + try: + assert await asyncio.to_thread(cleanup_started.wait, 5) + task.cancel() + await asyncio.sleep(0) + task.cancel() + await asyncio.sleep(0) + assert not task.done() + assert not unlocked.is_set() + assert old.sandbox_id in provider._local_teardown + assert provider._quarantine_store().contains(old) + finish_cleanup.set() + with pytest.raises(asyncio.CancelledError): + await task + assert cleanup_finished.is_set() and unlocked.is_set() + assert old.sandbox_id not in provider._local_teardown + assert provider._sandbox_infos[old.sandbox_id] is fresh + finally: + finish_cleanup.set() + if not task.done(): + task.cancel() + with contextlib.suppress(asyncio.CancelledError, RuntimeError): + await task + + +@pytest.mark.asyncio +@pytest.mark.parametrize("async_acquire", [False, True], ids=["sync", "async"]) +async def test_old_quarantine_does_not_destroy_replacement_when_endpoint_is_missing(failure_recovery_lifecycle, monkeypatch, async_acquire): + old = failure_recovery_lifecycle[3] + replacement = type(old)(old.sandbox_id, old.sandbox_url, container_id="new-generation") + provider, old = _prepare_quarantined_runtime_retry(failure_recovery_lifecycle, monkeypatch, replacement) + + if async_acquire: + acquired = await provider.acquire_async("thread-failure-recovery", user_id="alice") + else: + acquired = await asyncio.to_thread(provider.acquire, "thread-failure-recovery", user_id="alice") + + assert acquired == old.sandbox_id + provider._backend.destroy.assert_not_called() + assert provider._sandbox_infos[acquired] is replacement + # The live replacement proves the fenced old generation is absent, so its + # stale record is pruned instead of stranding every future create on the + # inspect-and-defer path. The replacement itself stays unfenced. + assert not provider._quarantine_store().contains(old) + assert not provider._quarantine_store().contains(replacement) + provider._backend.complete_absent_teardown.assert_not_called() + + +@pytest.mark.asyncio +@pytest.mark.parametrize("async_acquire", [False, True], ids=["sync", "async"]) +@pytest.mark.parametrize("final_probe", ["still-present", "unavailable"]) +async def test_accepted_delete_keeps_quarantine_until_runtime_disappears(failure_recovery_lifecycle, monkeypatch, async_acquire, final_probe): + from deerflow.community.aio_sandbox.aio_sandbox_provider import SandboxPolicyReplacementDeferredError + + old = failure_recovery_lifecycle[3] + current = type(old)(old.sandbox_id, "", container_id=old.container_id) + provider, old = _prepare_quarantined_runtime_retry(failure_recovery_lifecycle, monkeypatch, current) + provider._backend.is_absent.return_value = False + expected_error = SandboxPolicyReplacementDeferredError + if final_probe == "unavailable": + expected_error = RuntimeError + provider._backend.is_absent.side_effect = [False, RuntimeError("final presence probe unavailable")] + + with pytest.raises(expected_error): + if async_acquire: + await provider.acquire_async("thread-failure-recovery", user_id="alice") + else: + await asyncio.to_thread(provider.acquire, "thread-failure-recovery", user_id="alice") + + provider._backend.destroy.assert_called_once_with(current) + provider._backend.create.assert_not_called() + assert provider._quarantine_store().contains(old) + assert old.sandbox_id not in provider._local_teardown + + fresh = type(old)(old.sandbox_id, old.sandbox_url, container_id="new-generation") + provider._backend.is_absent.side_effect = None + provider._backend.is_absent.return_value = True + provider._backend.create.return_value = fresh + if async_acquire: + acquired = await provider.acquire_async("thread-failure-recovery", user_id="alice") + else: + acquired = await asyncio.to_thread(provider.acquire, "thread-failure-recovery", user_id="alice") + + assert provider._sandbox_infos[acquired] is fresh + assert not provider._quarantine_store().contains(old) + + +@pytest.mark.parametrize("blocked_by", ["presence", "inspection", "storage", "peer", "local-reservation", "mismatched-identity"]) +def test_quarantined_runtime_retry_fails_closed_before_destroy(failure_recovery_lifecycle, monkeypatch, blocked_by): + old = failure_recovery_lifecycle[3] + current = type(old)(old.sandbox_id, "", container_id=old.container_id) + provider, old = _prepare_quarantined_runtime_retry(failure_recovery_lifecycle, monkeypatch, current) + expected_error = RuntimeError + if blocked_by == "presence": + provider._backend.is_absent.side_effect = RuntimeError("presence unavailable") + elif blocked_by == "inspection": + provider._backend.inspect_runtime.side_effect = RuntimeError("runtime inspection unavailable") + elif blocked_by == "storage": + expected_error = PermissionError + store = provider._quarantine_store() + original_contains = store.contains + + def contains(info): + if info is current: + raise PermissionError("quarantine unavailable") + return original_contains(info) + + monkeypatch.setattr(store, "contains", contains) + elif blocked_by == "peer": + monkeypatch.setattr(provider, "_claim_ownership", lambda *_args, **_kwargs: False) + elif blocked_by == "local-reservation": + provider._local_teardown.add(old.sandbox_id) + elif blocked_by == "mismatched-identity": + provider._backend.inspect_runtime.return_value = type(old)("another-sandbox", "", container_id=old.container_id) + + with pytest.raises(expected_error): + provider.acquire("thread-failure-recovery", user_id="alice") + + provider._backend.destroy.assert_not_called() + provider._backend.create.assert_not_called() + assert provider._quarantine_store().contains(old) + provider._backend.complete_absent_teardown.assert_not_called() + if blocked_by in {"peer", "local-reservation"}: + provider._backend.is_absent.assert_not_called() + provider._backend.inspect_runtime.assert_not_called() + if blocked_by == "local-reservation": + provider._local_teardown.discard(old.sandbox_id) + else: + assert old.sandbox_id not in provider._local_teardown + + +@pytest.mark.asyncio +@pytest.mark.parametrize("async_acquire", [False, True], ids=["sync", "async"]) +@pytest.mark.parametrize("blocked_by", ["unowned", "peer-owned", "local-reservation"]) +@pytest.mark.parametrize("rejection", ["broker-mode", "missing-capacity", "insufficient-capacity"]) +async def test_remote_creation_metadata_rejection_uses_fenced_cleanup(failure_recovery_lifecycle, monkeypatch, async_acquire, blocked_by, rejection): + import json + + import requests + + from deerflow.community.aio_sandbox import remote_backend as remote_mod + + provider, _, _, old = failure_recovery_lifecycle + sid = "new-sandbox" + provider._backend = remote_mod.RemoteSandboxBackend("http://provisioner:8002", max_shell_sessions=None if rejection == "broker-mode" else 13) + monkeypatch.setattr(provider, "_sandbox_id_for_thread", lambda *_args: sid) + monkeypatch.setattr(provider, "_get_extra_mounts", lambda *_args, **_kwargs: []) + monkeypatch.setattr(provider, "_lark_integration_active", lambda *_args: rejection == "broker-mode") + monkeypatch.setattr(provider, "_lark_broker_active", lambda *_args: rejection == "broker-mode") + monkeypatch.setattr(remote_mod, "user_should_see_legacy_skills", lambda _uid: False) + if blocked_by == "peer-owned": + monkeypatch.setattr(provider, "_claim_ownership", lambda *_args, **_kwargs: False) + + def response(status, payload): + result = requests.Response() + result.status_code = status + result._content = json.dumps(payload).encode() + return result + + monkeypatch.setattr(requests, "get", lambda *_args, **_kwargs: response(404, {})) + payload = {"sandbox_url": "http://sandbox", "container_id": "created-pod", "lark_cli_broker": False} + if rejection == "insufficient-capacity": + payload["max_shell_sessions"] = 10 + + def post(*_args, **_kwargs): + if blocked_by == "local-reservation": + provider._local_teardown.add(sid) + return response(200, payload) + + monkeypatch.setattr(requests, "post", post) + deleted_under_fence = [] + + def delete(*_args, **_kwargs): + deleted_under_fence.append((provider._ownership.owner(sid), sid in provider._local_teardown)) + return response(200, {}) + + monkeypatch.setattr(requests, "delete", delete) + + message = {"broker-mode": "broker mode", "missing-capacity": "version skew", "insufficient-capacity": "insufficient shell-session capacity"}[rejection] + with pytest.raises(RuntimeError, match=message): + if async_acquire: + await provider.acquire_async("new-thread", user_id="alice") + else: + await asyncio.to_thread(provider.acquire, "new-thread", user_id="alice") + + assert deleted_under_fence == ([(provider._owner_id, True)] if blocked_by == "unowned" else []) + assert sid not in provider._sandboxes + assert old.sandbox_id in provider._sandboxes + provider._local_teardown.discard(sid) + + +@pytest.mark.asyncio +async def test_cancelled_remote_creation_drains_metadata_rejection_cleanup(failure_recovery_lifecycle, monkeypatch): + import json + + import requests + + from deerflow.community.aio_sandbox import remote_backend as remote_mod + + provider = failure_recovery_lifecycle[0] + sid = "new-sandbox" + provider._backend = remote_mod.RemoteSandboxBackend("http://provisioner:8002") + monkeypatch.setattr(provider, "_sandbox_id_for_thread", lambda *_args: sid) + monkeypatch.setattr(provider, "_get_extra_mounts", lambda *_args, **_kwargs: []) + monkeypatch.setattr(provider, "_lark_integration_active", lambda *_args: True) + monkeypatch.setattr(provider, "_lark_broker_active", lambda *_args: True) + monkeypatch.setattr(remote_mod, "user_should_see_legacy_skills", lambda _uid: False) + post_started = threading.Event() + finish_post = threading.Event() + cleanup_started = threading.Event() + finish_cleanup = threading.Event() + cleanup_finished = threading.Event() + + def response(status, payload): + result = requests.Response() + result.status_code = status + result._content = json.dumps(payload).encode() + return result + + def post(*_args, **_kwargs): + post_started.set() + assert finish_post.wait(5) + return response(200, {"sandbox_url": "http://sandbox", "container_id": "created-pod", "lark_cli_broker": False}) + + def delete(*_args, **_kwargs): + assert provider._ownership.owner(sid) == provider._owner_id + assert sid in provider._local_teardown + cleanup_started.set() + assert finish_cleanup.wait(5) + cleanup_finished.set() + return response(200, {}) + + monkeypatch.setattr(requests, "get", lambda *_args, **_kwargs: response(404, {})) + monkeypatch.setattr(requests, "post", post) + monkeypatch.setattr(requests, "delete", delete) + task = asyncio.create_task(provider.acquire_async("new-thread", user_id="alice")) + try: + assert await asyncio.to_thread(post_started.wait, 5) + task.cancel() + finish_post.set() + assert await asyncio.to_thread(cleanup_started.wait, 5) + assert not task.done() + finish_cleanup.set() + with pytest.raises(asyncio.CancelledError): + await task + assert cleanup_finished.is_set() + assert sid not in provider._local_teardown + assert sid not in provider._sandboxes + finally: + finish_post.set() + finish_cleanup.set() + if not task.done(): + task.cancel() + with contextlib.suppress(asyncio.CancelledError, RuntimeError): + await task + + +@pytest.mark.asyncio +@pytest.mark.parametrize("async_acquire", [False, True], ids=["sync", "async"]) +@pytest.mark.parametrize("error_type", [PermissionError, RuntimeError], ids=["storage-error", "policy-error"]) +async def test_discovery_policy_failure_after_publish_closes_client_without_destroy(failure_recovery_lifecycle, monkeypatch, async_acquire, error_type): + provider, _, _, old = failure_recovery_lifecycle + discovered = type(old)("discovered-sandbox", "http://sandbox", container_id="discovered-generation") + provider._backend.discover = MagicMock(return_value=discovered) + provider._backend.create = MagicMock(side_effect=AssertionError("discovered Pod must not be recreated")) + monkeypatch.setattr(provider, "_sandbox_id_for_thread", lambda *_args: discovered.sandbox_id) + monkeypatch.setattr(provider, "_lark_integration_active", lambda *_args: False) + provider_mod = importlib.import_module(type(provider).__module__) + client = MagicMock() + monkeypatch.setattr(provider_mod, "AioSandbox", MagicMock(return_value=client)) + published = False + original_publish = provider._publish_ownership + + def publish(sandbox_id): + nonlocal published + original_publish(sandbox_id) + published = True + + store = provider._quarantine_store() + original_contains = store.contains + + def contains(info): + if published: + raise error_type("policy check failed after ownership publish") + return original_contains(info) + + monkeypatch.setattr(provider, "_publish_ownership", publish) + monkeypatch.setattr(store, "contains", contains) + + with pytest.raises(error_type, match="policy check failed after ownership publish"): + if async_acquire: + await provider.acquire_async("new-thread", user_id="alice") + else: + await asyncio.to_thread(provider.acquire, "new-thread", user_id="alice") + + client.close.assert_called_once_with() + provider._backend.destroy.assert_not_called() + assert discovered.sandbox_id not in provider._sandboxes + assert old.sandbox_id in provider._sandboxes + + +@pytest.mark.asyncio +@pytest.mark.parametrize("async_create", [False, True]) +@pytest.mark.parametrize("failure", ["probe", "storage", "policy"]) +@pytest.mark.parametrize("peer_owned", [False, True]) +async def test_post_creation_policy_failure_uses_fenced_rollback(failure_recovery_lifecycle, monkeypatch, async_create, failure, peer_owned): + provider, _, _, old = failure_recovery_lifecycle + fresh = type(old)("new-sandbox", "http://fresh", container_id="fresh-generation") + provider._backend.create = MagicMock(return_value=fresh) + monkeypatch.setattr(provider, "_get_extra_mounts", lambda *_args, **_kwargs: []) + monkeypatch.setattr(type(provider), "_lark_integration_active", staticmethod(lambda *_args: False)) + monkeypatch.setattr(importlib.import_module(type(provider).__module__), "wait_for_sandbox_ready", lambda *_args, **_kwargs: True) + monkeypatch.setattr(importlib.import_module(type(provider).__module__), "wait_for_sandbox_ready_async", AsyncMock(return_value=True)) + + def check_policy(info, _user): + if failure == "policy": + info.requires_replacement = True + elif failure == "storage": + raise PermissionError("quarantine unavailable") + else: + raise RuntimeError("capability unknown") + + monkeypatch.setattr(provider, "_check_sandbox_reuse_policy", check_policy) + if peer_owned: + monkeypatch.setattr(provider, "_claim_ownership", lambda *_args, **_kwargs: False) + + with pytest.raises((RuntimeError, PermissionError)): + if async_create: + await provider._create_sandbox_async("new-thread", fresh.sandbox_id, user_id="alice") + else: + provider._create_sandbox("new-thread", fresh.sandbox_id, user_id="alice") + + assert fresh.sandbox_id not in provider._sandbox_infos + assert fresh.sandbox_id not in provider._local_teardown + if peer_owned: + provider._backend.destroy.assert_not_called() + else: + provider._backend.destroy.assert_called_once_with(fresh) + + +@pytest.mark.parametrize("source", ["active", "warm", "discovered"]) +@pytest.mark.parametrize("actual_broker", [False, True]) +def test_mode_mismatch_refreshes_capabilities_before_replacing_pod(failure_recovery_lifecycle, monkeypatch, source, actual_broker): + from deerflow.community.aio_sandbox.remote_backend import RemoteSandboxBackend + from deerflow.config import app_config + from deerflow.integrations import lark_cli + + provider, sandbox, _, info = failure_recovery_lifecycle + info.lark_cli_broker = sandbox.lark_cli_broker = actual_broker + provider._backend = RemoteSandboxBackend("http://provisioner:8002") + provider._backend.destroy = MagicMock() + provider._backend.create = MagicMock() + provider._backend.discover = MagicMock(return_value=info) + monkeypatch.setattr(type(provider), "_lark_integration_active", staticmethod(lambda *_args: True)) + monkeypatch.setattr(provider, "_check_tracked_sandbox_alive", lambda *_args: True) + config = SimpleNamespace(sandbox=SimpleNamespace(use="deerflow.community.aio_sandbox:AioSandboxProvider", provisioner_url="http://provisioner:8002")) + monkeypatch.setattr(app_config, "get_app_config", lambda: config) + monkeypatch.setattr(lark_cli.sandbox_lark_broker_active, "_cache", {}, raising=False) + probe = MagicMock(side_effect=[{"lark_cli_broker_image": not actual_broker}, {"lark_cli_broker_image": actual_broker}]) + monkeypatch.setattr(lark_cli, "_probe_provisioner_capabilities", probe) + assert lark_cli.sandbox_lark_broker_active(config) is not actual_broker + if source != "active": + provider.release(info.sandbox_id) + if source == "discovered": + provider._warm_pool.clear() + + assert provider.acquire("thread-failure-recovery", user_id="alice") == info.sandbox_id + + assert provider.get(info.sandbox_id).lark_cli_broker is actual_broker + assert info.requires_replacement is False + assert probe.call_count == 2 + provider._backend.destroy.assert_not_called() + provider._backend.create.assert_not_called() + + +@pytest.mark.parametrize("source", ["active", "warm", "discovered"]) +def test_probe_recovery_cannot_readmit_non_broker_generation(failure_recovery_lifecycle, monkeypatch, source): + """Capability recovery must validate every reuse path before executing.""" + import json + + from deerflow.community.aio_sandbox import aio_sandbox_provider as provider_mod + from deerflow.community.aio_sandbox import remote_backend as remote_mod + from deerflow.config import app_config + from deerflow.integrations import lark_cli + + provider, _, shell, info = failure_recovery_lifecycle + provider._backend = remote_mod.RemoteSandboxBackend("http://provisioner:8002") + monkeypatch.setattr(provider_mod.AioSandboxProvider, "_lark_integration_active", staticmethod(lambda *_args: True)) + monkeypatch.setattr(provider, "_get_extra_mounts", lambda *_args, **_kwargs: []) + monkeypatch.setattr(provider_mod, "wait_for_sandbox_ready", lambda *_args, **_kwargs: True) + monkeypatch.setattr(remote_mod, "user_should_see_legacy_skills", lambda *_args: False) + config = SimpleNamespace(sandbox=SimpleNamespace(use="deerflow.community.aio_sandbox:AioSandboxProvider", provisioner_url="http://provisioner:8002")) + monkeypatch.setattr(app_config, "get_app_config", lambda: config) + capability = MagicMock() + capability.__enter__.return_value = capability + capability.read.return_value = json.dumps({"lark_cli_init_image": True, "lark_cli_broker_image": True}).encode() + probe = MagicMock(side_effect=[TimeoutError("capability timeout"), capability]) + monkeypatch.setattr(lark_cli.urllib.request, "urlopen", probe) + monkeypatch.setattr(lark_cli.sandbox_lark_broker_active, "_cache", {}, raising=False) + clock = SimpleNamespace(now=100.0) + monkeypatch.setattr(lark_cli, "time", SimpleNamespace(monotonic=lambda: clock.now)) + old_exists = [True] + + def response(payload, status=200): + return SimpleNamespace(status_code=status, ok=status == 200, json=lambda: payload, raise_for_status=lambda: None) + + def get(*_args, **_kwargs): + return response({**info.to_dict(), "status": "Running"}, 200 if old_exists[0] else 404) + + def delete(*_args, **_kwargs): + assert provider._ownership.owner(info.sandbox_id) == provider._owner_id + old_exists[0] = False + return response({}) + + def post(*_args, **kwargs): + assert kwargs["json"]["provision_lark_cli_broker"] is True + return response({"sandbox_url": info.sandbox_url, "lark_cli_broker": True, "container_id": "broker-generation"}) + + monkeypatch.setattr(remote_mod.requests, "get", get) + monkeypatch.setattr(remote_mod.requests, "delete", delete) + monkeypatch.setattr(remote_mod.requests, "post", post) + if source != "active": + provider.release(info.sandbox_id) + if source == "discovered": + provider._warm_pool.clear() + # Probe outage: the attested non-broker generation keeps serving. Its own + # attestation outranks the unreachable deployment-level probe; execution + # never ran, so nothing here weakens the credential-placement guarantee. + assert provider.acquire("thread-failure-recovery", user_id="alice") == info.sandbox_id + assert provider.get(info.sandbox_id).lark_cli_broker is False + shell.exec_command.assert_not_called() + clock.now += lark_cli.LARK_BROKER_MODE_PROBE_RETRY_SECONDS + 1 + # Probe recovered and now requires broker: every reuse path must replace the + # stale generation instead of readmitting it. + assert provider.acquire("thread-failure-recovery", user_id="alice") == info.sandbox_id + assert provider.get(info.sandbox_id).lark_cli_broker is True + assert provider._sandbox_infos[info.sandbox_id].container_id == "broker-generation" + shell.exec_command.assert_not_called() + assert probe.call_count == 2 + + +def test_acquire_recycles_quarantined_cached_sandbox_inline(tmp_path, monkeypatch): + """A failed release recycle must not wedge the thread: the next acquire destroys the fenced residue.""" + provider, sandbox, aio_mod = _make_provider_with_active_sandbox(tmp_path, "sandbox-q-residue") + info = provider._sandbox_infos["sandbox-q-residue"] + info.container_id = "generation-dirty" + sandbox.requires_container_recycle = True + provider._thread_sandboxes[("alice", "thread-q")] = "sandbox-q-residue" + provider._quarantine_store().mark(info) + provider._backend.discover = MagicMock(return_value=None) + monkeypatch.setattr(provider, "_sandbox_id_for_thread", lambda *_args: "sandbox-q-residue") + monkeypatch.setattr(provider, "_ensure_skills_projection", lambda *_args: None) + monkeypatch.setattr(aio_mod, "get_paths", lambda: Paths(base_dir=tmp_path / "state")) + monkeypatch.setattr(provider, "_create_sandbox", lambda *_args, **_kwargs: "sandbox-q-residue") + + assert provider.acquire("thread-q", user_id="alice") == "sandbox-q-residue" + + provider._backend.destroy.assert_called_once_with(info) + assert "sandbox-q-residue" not in provider._sandboxes + + +def test_pending_create_allows_upload_acquire_without_recycling_live_run(failure_recovery_lifecycle, monkeypatch): + from deerflow.sandbox.lease import discard_sandbox_lease_manager, get_sandbox_lease_manager + + provider, sandbox, shell, info = failure_recovery_lifecycle + manager = get_sandbox_lease_manager(provider) + manager.retain("running-agent", info.sandbox_id, thread_id="thread-failure-recovery", user_id="alice") + entered, complete = threading.Event(), threading.Event() + results: list[str] = [] + + def create_session(id, **_kwargs): + entered.set() + assert complete.wait(timeout=5) + return SimpleNamespace(data=SimpleNamespace(session_id=id)) + + shell.create_session.side_effect = create_session + runner = threading.Thread(target=lambda: results.append(sandbox.execute_command_in_scope("first", scope_id="running-agent"))) + try: + runner.start() + assert entered.wait(timeout=5) + assert manager.acquire("upload", "thread-failure-recovery", user_id="alice") == info.sandbox_id + assert provider.get(info.sandbox_id) is sandbox + provider._backend.destroy.assert_not_called() + assert not provider._quarantine_store().contains(info) + complete.set() + runner.join(timeout=5) + assert not runner.is_alive() + assert results == ["next-turn"] + assert sandbox.execute_command_in_scope("second", scope_id="running-agent") == "next-turn" + manager.release("upload") + assert provider.get(info.sandbox_id) is sandbox + manager.release("running-agent") + provider._backend.destroy.assert_not_called() + finally: + complete.set() + if runner.ident is not None: + runner.join(timeout=5) + discard_sandbox_lease_manager(provider) + + +def test_uncertain_runtime_preserves_live_holders_until_final_release(failure_recovery_lifecycle): + from deerflow.sandbox.lease import discard_sandbox_lease_manager, get_sandbox_lease_manager + + provider, sandbox, _, info = failure_recovery_lifecycle + manager = get_sandbox_lease_manager(provider) + manager.retain("parent", info.sandbox_id, thread_id="thread-failure-recovery", user_id="alice") + manager.retain("fork", info.sandbox_id, thread_id="fork-thread", user_id="alice", release_on_last=False) + sandbox._default_shell_corrupted = True + try: + provider.release(info.sandbox_id) + provider.destroy(info.sandbox_id) + assert provider.get(info.sandbox_id) is sandbox + provider._backend.destroy.assert_not_called() + with pytest.raises(RuntimeError, match="deferred|recycle did not complete"): + manager.acquire("upload", "thread-failure-recovery", user_id="alice") + assert manager.binding_for("upload") is None + provider._backend.destroy.assert_not_called() + manager.release("parent") + assert provider.get(info.sandbox_id) is sandbox + provider._backend.destroy.assert_not_called() + manager.release("fork") + provider._backend.destroy.assert_called_once_with(info) + assert provider.get(info.sandbox_id) is None + assert provider._quarantine_store().contains(info) + finally: + discard_sandbox_lease_manager(provider) + + +@pytest.mark.parametrize("plane", ["shell", "bash"]) +def test_idle_release_quarantines_abandoned_pending_create(failure_recovery_lifecycle, plane): + provider, sandbox, _, info = failure_recovery_lifecycle + sandbox._begin_session_creation(plane, "abandoned-session") + assert sandbox.has_pending_session_creates is True + assert sandbox.requires_container_recycle is False + + provider.release(info.sandbox_id) + + provider._backend.destroy.assert_called_once_with(info) + assert provider.get(info.sandbox_id) is None + assert info.sandbox_id not in provider._warm_pool + assert provider._quarantine_store().contains(info) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("async_acquire", [False, True], ids=["sync", "async"]) +async def test_lease_acquire_can_replace_its_own_fenced_generation(failure_recovery_lifecycle, async_acquire): + from deerflow.sandbox.lease import get_sandbox_lease_manager + + provider, sandbox, _, info = failure_recovery_lifecycle + sandbox._default_shell_corrupted = True + fresh = type(info)(info.sandbox_id, info.sandbox_url, container_id="container-generation-2", lark_cli_broker=False) + provider._backend.discover = MagicMock(return_value=fresh) + manager = get_sandbox_lease_manager(provider) + + if async_acquire: + acquired = await manager.acquire_async("next-run", "thread-failure-recovery", user_id="alice") + else: + acquired = await asyncio.to_thread(manager.acquire, "next-run", "thread-failure-recovery", user_id="alice") + + assert acquired == info.sandbox_id + assert manager.binding_for("next-run") == info.sandbox_id + assert provider.get(acquired) is not sandbox + assert provider._sandbox_infos[acquired].container_id == fresh.container_id + provider._backend.destroy.assert_called_once_with(info) + await manager.release_async("next-run") + + +@pytest.mark.parametrize("blocked_by", ["local", "ownership"]) +def test_inline_recycle_reports_a_refused_teardown_as_deferred(failure_recovery_lifecycle, blocked_by, monkeypatch): + provider, sandbox, _, info = failure_recovery_lifecycle + sandbox._default_shell_corrupted = True + if blocked_by == "local": + provider._local_teardown.add(info.sandbox_id) + else: + monkeypatch.setattr(provider, "_claim_ownership", lambda *_args, **_kwargs: False) + assert provider._recycle_fenced_tracked_sandbox(info.sandbox_id) is False + assert provider.get(info.sandbox_id) is sandbox + provider._backend.destroy.assert_not_called() + + +def test_acquire_recycle_failure_defers_with_quarantine_reason(tmp_path, monkeypatch): + """When the inline recycle cannot destroy the residue, acquire defers with an accurate reason.""" + provider, sandbox, aio_mod = _make_provider_with_active_sandbox(tmp_path, "sandbox-q-stuck") + info = provider._sandbox_infos["sandbox-q-stuck"] + info.container_id = "generation-stuck" + sandbox.requires_container_recycle = True + provider._thread_sandboxes[("alice", "thread-stuck")] = "sandbox-q-stuck" + provider._quarantine_store().mark(info) + provider._backend.destroy = MagicMock(side_effect=RuntimeError("docker daemon down")) + monkeypatch.setattr(provider, "_sandbox_id_for_thread", lambda *_args: "sandbox-q-stuck") + monkeypatch.setattr(provider, "_ensure_skills_projection", lambda *_args: None) + + with pytest.raises(aio_mod.SandboxPolicyReplacementDeferredError, match="quarantined"): + provider.acquire("thread-stuck", user_id="alice") + + provider._backend.destroy.assert_called_once_with(info) + assert provider._quarantine_store().contains(info) + + +@pytest.mark.parametrize("attested", [False, True]) +def test_reuse_policy_defers_to_attested_mode_when_probe_unavailable(tmp_path, monkeypatch, attested): + """A Pod-attested mode outranks an unavailable deployment-level capabilities probe.""" + from deerflow.community.aio_sandbox.remote_backend import RemoteSandboxBackend + from deerflow.integrations import lark_cli + + provider, _, aio_mod = _make_provider_with_active_sandbox(tmp_path, "sandbox-attested") + provider._backend = RemoteSandboxBackend("http://provisioner:8002") + monkeypatch.setattr(aio_mod.AioSandboxProvider, "_lark_integration_active", staticmethod(lambda *_args: True)) + + def _probe_unknown(*_args, **_kwargs): + raise lark_cli.LarkBrokerCapabilityUnknownError("capability is unknown") + + monkeypatch.setattr(aio_mod.AioSandboxProvider, "_lark_broker_active", staticmethod(_probe_unknown)) + info = provider._sandbox_infos["sandbox-attested"] + info.lark_cli_broker = attested + + provider._check_sandbox_reuse_policy(info, "alice") + + assert info.requires_replacement is False + + +@pytest.mark.parametrize("source", ["active", "warm", "discovered"]) +@pytest.mark.parametrize("required_broker", [False, True]) +def test_unattested_broker_mode_refuses_acquire_without_deleting_pod(failure_recovery_lifecycle, monkeypatch, source, required_broker): + """Version skew must refuse admission while preserving the existing Pod.""" + from deerflow.community.aio_sandbox import remote_backend as remote_mod + + provider, sandbox, _, info = failure_recovery_lifecycle + info.lark_cli_broker = sandbox.lark_cli_broker = None + if source != "active": + provider.release(info.sandbox_id) + if source == "discovered": + provider._warm_pool.clear() + provider._warm_pool_identity.clear() + backend = remote_mod.RemoteSandboxBackend("http://provisioner:8002") + backend.discover = MagicMock(return_value=info) + backend.destroy = MagicMock() + backend.create = MagicMock(side_effect=AssertionError("must preserve the existing Pod")) + provider._backend = backend + monkeypatch.setattr(provider, "_lark_integration_active", lambda *_args: True) + probe = MagicMock(return_value=required_broker) + monkeypatch.setattr(provider, "_lark_broker_active", probe) + + with pytest.raises(RuntimeError, match="broker mode is unverified"): + provider.acquire("thread-failure-recovery", user_id="alice") + + assert info.requires_replacement is False + backend.destroy.assert_not_called() + backend.create.assert_not_called() + probe.assert_not_called() + + +def test_reuse_policy_stays_fail_closed_for_unattested_mode_when_probe_unavailable(tmp_path, monkeypatch): + """Without a Pod attestation there is no stronger evidence: probe failure keeps refusing reuse.""" + from deerflow.community.aio_sandbox.remote_backend import RemoteSandboxBackend + from deerflow.integrations import lark_cli + + provider, _, aio_mod = _make_provider_with_active_sandbox(tmp_path, "sandbox-unattested") + provider._backend = RemoteSandboxBackend("http://provisioner:8002") + monkeypatch.setattr(aio_mod.AioSandboxProvider, "_lark_integration_active", staticmethod(lambda *_args: True)) + + def _probe_unknown(*_args, **_kwargs): + raise lark_cli.LarkBrokerCapabilityUnknownError("capability is unknown") + + monkeypatch.setattr(aio_mod.AioSandboxProvider, "_lark_broker_active", staticmethod(_probe_unknown)) + info = provider._sandbox_infos["sandbox-unattested"] + info.lark_cli_broker = None + + with pytest.raises(aio_mod.SandboxBrokerModeUnverifiedError, match="broker mode is unverified"): + provider._check_sandbox_reuse_policy(info, "alice") + + def test_reused_active_sandbox_requires_matching_releases(tmp_path): """The execution lease manager keeps AIO clients active until the final holder exits.""" from deerflow.sandbox.lease import SandboxLeaseManager @@ -1897,6 +2996,42 @@ def test_destroy_closes_cached_sandbox_client(tmp_path): assert "sandbox-destroy" not in provider._sandbox_infos +def test_destroy_reports_false_when_teardown_is_deferred(tmp_path): + """An explicit destroy under an active execution lease must surface the + deferral instead of letting the caller assume the container is gone.""" + import threading + + from deerflow.sandbox.lease import get_sandbox_lease_manager + + provider, sandbox, _ = _make_provider_with_active_sandbox(tmp_path, "sandbox-destroy-defer") + manager = get_sandbox_lease_manager(provider) + # reserve_idle_teardown is re-entrant per worker, so the competing + # reservation must come from another thread to force the deferral. + hold = threading.Event() + release = threading.Event() + + def hold_teardown(): + with manager.reserve_idle_teardown("sandbox-destroy-defer") as reserved: + assert reserved + hold.set() + release.wait(5) + + worker = threading.Thread(target=hold_teardown, daemon=True) + worker.start() + assert hold.wait(5) + try: + assert provider.destroy("sandbox-destroy-defer") is False + finally: + release.set() + worker.join(5) + + provider._backend.destroy.assert_not_called() + sandbox.close.assert_not_called() + assert "sandbox-destroy-defer" in provider._sandboxes + assert provider.destroy("sandbox-destroy-defer") is True + provider._backend.destroy.assert_called_once() + + def test_shutdown_closes_all_active_sandbox_clients(tmp_path): """shutdown() must close every cached AioSandbox client during teardown (#2872).""" provider, sandbox, _ = _make_provider_with_active_sandbox(tmp_path, "sandbox-shut") @@ -1994,7 +3129,7 @@ def test_release_dirty_sandbox_destroy_failure_is_logged_without_warming( assert "sandbox-dirty-fail" not in provider._warm_pool assert "sandbox-dirty-fail" not in provider._sandboxes assert "sandbox-dirty-fail" not in provider._sandbox_infos - assert "Failed to recycle sandbox sandbox-dirty-fail" in caplog.text + assert "Failed to recycle quarantined sandbox sandbox-dirty-fail" in caplog.text provider._backend.destroy.assert_called_once() sandbox.close.assert_called_once_with() diff --git a/backend/tests/test_dingtalk_channel.py b/backend/tests/test_dingtalk_channel.py index 193cd62cda..427b3cf742 100644 --- a/backend/tests/test_dingtalk_channel.py +++ b/backend/tests/test_dingtalk_channel.py @@ -1851,10 +1851,13 @@ def _patch_uploads(monkeypatch, uploads_dir, *, sandbox_id="local", sandbox=None async def _acquire_async(thread_id, user_id=None): return sandbox_id - monkeypatch.setattr( - "app.channels.dingtalk.get_sandbox_provider", - lambda: SimpleNamespace(acquire_async=_acquire_async, get=lambda sid: sandbox), + provider = SimpleNamespace( + uses_thread_data_mounts=sandbox_id == "local" or sandbox_id.startswith("local:"), + acquire_async=_acquire_async, + get=lambda sid: sandbox, + release=lambda sid: None, ) + monkeypatch.setattr("app.channels.dingtalk.get_sandbox_provider", lambda: provider) class TestExtractFiles: diff --git a/backend/tests/test_lark_cli_integration.py b/backend/tests/test_lark_cli_integration.py index ec9312b0f2..55dfbc1fcc 100644 --- a/backend/tests/test_lark_cli_integration.py +++ b/backend/tests/test_lark_cli_integration.py @@ -17,6 +17,7 @@ import zipfile from concurrent.futures import ThreadPoolExecutor from pathlib import Path from types import SimpleNamespace +from unittest.mock import MagicMock from uuid import uuid4 import pytest @@ -30,6 +31,7 @@ from app.gateway.routers import integrations as integrations_router from deerflow.config import paths as paths_module from deerflow.config.paths import Paths from deerflow.integrations import lark_cli +from deerflow.sandbox.sandbox import Sandbox from deerflow.sandbox.tools import _lark_cli_env_from_runtime from deerflow.skills.storage import reset_skill_storage from deerflow.skills.storage.user_scoped_skill_storage import UserScopedSkillStorage @@ -693,6 +695,7 @@ def test_status_explicitly_reports_remote_runtime_probe_state(monkeypatch, tmp_p def _reset_broker_mode_cache() -> None: if hasattr(lark_cli.sandbox_lark_broker_active, "_cache"): del lark_cli.sandbox_lark_broker_active._cache + lark_cli._LARK_BROKER_MODE_PROBES.clear() def test_sandbox_lark_broker_active_uses_tight_hot_path_timeout(monkeypatch, tmp_path) -> None: @@ -759,6 +762,314 @@ def test_sandbox_lark_broker_active_false_without_remote_provisioner(monkeypatch _reset_broker_mode_cache() +def test_sandbox_lark_broker_active_recovers_after_failed_probe(monkeypatch, tmp_path) -> None: + """A recovered provisioner must be usable before the disabled-mode TTL ends.""" + _reset_broker_mode_cache() + config = _config(tmp_path / "skills") + config.sandbox = SimpleNamespace( + use="deerflow.community.aio_sandbox:AioSandboxProvider", + provisioner_url="http://provisioner:8002", + ) + clock = SimpleNamespace(now=100.0) + monkeypatch.setattr(lark_cli, "time", SimpleNamespace(monotonic=lambda: clock.now)) + response = MagicMock() + response.__enter__.return_value = response + response.read.return_value = json.dumps({"lark_cli_init_image": True, "lark_cli_broker_image": True}).encode("utf-8") + urlopen = MagicMock(side_effect=[TimeoutError("capability response timed out"), response]) + monkeypatch.setattr(lark_cli.urllib.request, "urlopen", urlopen) + + try: + # Cold-start unknown may be reported as unavailable or raised; neither + # outcome may suppress the later successful capability response. + try: + lark_cli.sandbox_lark_broker_active(config) + except RuntimeError: + pass + assert urlopen.call_count == 1 + + clock.now += lark_cli.LARK_BROKER_MODE_NEGATIVE_TTL_SECONDS - 1 + recovered = lark_cli.sandbox_lark_broker_active(config) + + assert recovered is True, "a failed probe must not hide a recovered broker behind the disabled-mode TTL" + assert urlopen.call_count == 2 + finally: + _reset_broker_mode_cache() + + +def test_sandbox_lark_broker_active_preserves_confirmed_mode_through_probe_failure(monkeypatch, tmp_path) -> None: + """Refresh failure must not downgrade broker mode or prevent its recovery.""" + _reset_broker_mode_cache() + config = _config(tmp_path / "skills") + config.sandbox = SimpleNamespace( + use="deerflow.community.aio_sandbox:AioSandboxProvider", + provisioner_url="http://provisioner:8002", + ) + clock = SimpleNamespace(now=100.0) + monkeypatch.setattr(lark_cli, "time", SimpleNamespace(monotonic=lambda: clock.now)) + response = MagicMock() + response.__enter__.return_value = response + response.read.return_value = json.dumps({"lark_cli_init_image": True, "lark_cli_broker_image": True}).encode("utf-8") + urlopen = MagicMock(side_effect=[response, TimeoutError("capability response timed out"), response]) + monkeypatch.setattr(lark_cli.urllib.request, "urlopen", urlopen) + + try: + assert lark_cli.sandbox_lark_broker_active(config) is True + clock.now += lark_cli.LARK_BROKER_MODE_TTL_SECONDS + 1 + during_failure = lark_cli.sandbox_lark_broker_active(config) + assert urlopen.call_count == 2 + + clock.now += lark_cli.LARK_BROKER_MODE_NEGATIVE_TTL_SECONDS - 1 + after_recovery = lark_cli.sandbox_lark_broker_active(config) + + assert (during_failure, after_recovery) == (True, True), "a transient failure must not change the credential isolation mode" + assert urlopen.call_count == 3 + finally: + _reset_broker_mode_cache() + + +@pytest.mark.parametrize("payload", [None, {}, {"lark_cli_broker_image": "false"}]) +def test_unknown_broker_capability_cannot_authorize_credential_mounts(monkeypatch, tmp_path, payload): + _reset_broker_mode_cache() + config = _config(tmp_path / "skills") + config.sandbox = SimpleNamespace(use="deerflow.community.aio_sandbox:AioSandboxProvider", provisioner_url="http://provisioner:8002") + monkeypatch.setattr(lark_cli, "_probe_provisioner_capabilities", lambda *_args, **_kwargs: payload) + try: + with pytest.raises(RuntimeError, match="capabilit"): + lark_cli.sandbox_lark_broker_active(config) + finally: + _reset_broker_mode_cache() + + +def test_broker_capability_cache_is_scoped_to_provisioner(monkeypatch, tmp_path): + _reset_broker_mode_cache() + config = _config(tmp_path / "skills") + config.sandbox = SimpleNamespace(use="deerflow.community.aio_sandbox:AioSandboxProvider", provisioner_url="http://old-provisioner:8002") + probe = MagicMock(side_effect=[{"lark_cli_broker_image": False}, {"lark_cli_broker_image": True}]) + monkeypatch.setattr(lark_cli, "_probe_provisioner_capabilities", probe) + try: + assert lark_cli.sandbox_lark_broker_active(config) is False + config.sandbox.provisioner_url = "http://new-provisioner:8002" + assert lark_cli.sandbox_lark_broker_active(config) is True + assert probe.call_count == 2 + finally: + _reset_broker_mode_cache() + + +@pytest.mark.parametrize("confirmed", [False, True]) +def test_broker_capability_refreshes_a_conflicting_observation(monkeypatch, confirmed): + config = SimpleNamespace(sandbox=SimpleNamespace(use="aio_sandbox", provisioner_url="http://provisioner:8002")) + monkeypatch.setattr(lark_cli.sandbox_lark_broker_active, "_cache", {}, raising=False) + probe = MagicMock(side_effect=[{"lark_cli_broker_image": confirmed}, {"lark_cli_broker_image": not confirmed}]) + monkeypatch.setattr(lark_cli, "_probe_provisioner_capabilities", probe) + + assert lark_cli.sandbox_lark_broker_active(config) is confirmed + assert lark_cli.sandbox_lark_broker_active(config, observed_mode=not confirmed) is not confirmed + assert probe.call_count == 2 + + +def test_failed_broker_refresh_backs_off_without_extending_confirmation(monkeypatch): + config = SimpleNamespace(sandbox=SimpleNamespace(use="aio_sandbox", provisioner_url="http://provisioner:8002")) + monkeypatch.setattr(lark_cli.sandbox_lark_broker_active, "_cache", {}, raising=False) + clock = SimpleNamespace(now=100.0) + monkeypatch.setattr(lark_cli, "time", SimpleNamespace(monotonic=lambda: clock.now)) + probe = MagicMock(side_effect=[{"lark_cli_broker_image": True}, None, {"lark_cli_broker_image": False}]) + monkeypatch.setattr(lark_cli, "_probe_provisioner_capabilities", probe) + + assert lark_cli.sandbox_lark_broker_active(config) is True + clock.now += lark_cli.LARK_BROKER_MODE_TTL_SECONDS + 1 + assert lark_cli.sandbox_lark_broker_active(config) is True + assert lark_cli.sandbox_lark_broker_active(config) is True + assert probe.call_count == 2 + clock.now += lark_cli.LARK_BROKER_MODE_PROBE_RETRY_SECONDS + 1 + assert lark_cli.sandbox_lark_broker_active(config) is False + assert probe.call_count == 3 + + +def test_failed_conflicting_probe_cannot_reuse_a_cached_negative(monkeypatch): + config = SimpleNamespace(sandbox=SimpleNamespace(use="aio_sandbox", provisioner_url="http://provisioner:8002")) + monkeypatch.setattr(lark_cli.sandbox_lark_broker_active, "_cache", {}, raising=False) + clock = SimpleNamespace(now=100.0) + monkeypatch.setattr(lark_cli, "time", SimpleNamespace(monotonic=lambda: clock.now)) + probe = MagicMock(side_effect=[{"lark_cli_broker_image": False}, None, {"lark_cli_broker_image": True}]) + monkeypatch.setattr(lark_cli, "_probe_provisioner_capabilities", probe) + + assert lark_cli.sandbox_lark_broker_active(config) is False + with pytest.raises(RuntimeError, match="unknown"): + lark_cli.sandbox_lark_broker_active(config, observed_mode=True) + with pytest.raises(RuntimeError, match="unknown"): + lark_cli.sandbox_lark_broker_active(config) + assert probe.call_count == 2 + clock.now += lark_cli.LARK_BROKER_MODE_PROBE_RETRY_SECONDS + 1 + assert lark_cli.sandbox_lark_broker_active(config) is True + + +@pytest.mark.parametrize("previous_broker", [None, True]) +def test_concurrent_failed_broker_probes_share_one_observation(monkeypatch, previous_broker): + config = SimpleNamespace(sandbox=SimpleNamespace(use="aio_sandbox", provisioner_url="http://provisioner:8002")) + monkeypatch.setattr(lark_cli.sandbox_lark_broker_active, "_cache", {}, raising=False) + clock = SimpleNamespace(now=100.0) + monkeypatch.setattr(lark_cli, "time", SimpleNamespace(monotonic=lambda: clock.now)) + if previous_broker: + monkeypatch.setattr(lark_cli, "_probe_provisioner_capabilities", lambda *_args, **_kwargs: {"lark_cli_broker_image": True}) + assert lark_cli.sandbox_lark_broker_active(config) is True + clock.now += lark_cli.LARK_BROKER_MODE_TTL_SECONDS + 1 + entered = threading.Event() + release = threading.Event() + waiter_looked_up = threading.Event() + worker = threading.local() + lock = threading.Lock() + + class ObservedLock: + def __enter__(self): + lock.acquire() + + def __exit__(self, *_args): + lock.release() + if getattr(worker, "waiter", False): + waiter_looked_up.set() + + def probe(*_args, **_kwargs): + entered.set() + assert release.wait(5) + return None + + def invoke(*, waiter=False): + worker.waiter = waiter + try: + return lark_cli.sandbox_lark_broker_active(config) + except RuntimeError as exc: + assert "unknown" in str(exc) + return None + + monkeypatch.setattr(lark_cli, "_LARK_BROKER_MODE_CACHE_LOCK", ObservedLock()) + probe_mock = MagicMock(side_effect=probe) + monkeypatch.setattr(lark_cli, "_probe_provisioner_capabilities", probe_mock) + with ThreadPoolExecutor(max_workers=2) as pool: + first = pool.submit(invoke) + try: + assert entered.wait(3) + second = pool.submit(invoke, waiter=True) + assert waiter_looked_up.wait(3), "network I/O blocked the cache lookup" + finally: + release.set() + assert first.result(timeout=3) is previous_broker + assert second.result(timeout=3) is previous_broker + assert probe_mock.call_count == 1 + + +def test_slow_broker_probe_does_not_block_another_endpoint_cache(monkeypatch): + config = SimpleNamespace(sandbox=SimpleNamespace(use="aio_sandbox", provisioner_url="http://slow-provisioner:8002")) + other = SimpleNamespace(sandbox=SimpleNamespace(use="aio_sandbox", provisioner_url="http://other-provisioner:8002")) + monkeypatch.setattr(lark_cli.sandbox_lark_broker_active, "_cache", {}, raising=False) + clock = SimpleNamespace(now=100.0) + monkeypatch.setattr(lark_cli, "time", SimpleNamespace(monotonic=lambda: clock.now)) + monkeypatch.setattr(lark_cli, "_probe_provisioner_capabilities", MagicMock(side_effect=[{"lark_cli_broker_image": True}, {"lark_cli_broker_image": False}])) + assert lark_cli.sandbox_lark_broker_active(config) is True + assert lark_cli.sandbox_lark_broker_active(other) is False + clock.now += lark_cli.LARK_BROKER_MODE_TTL_SECONDS + 1 + entered = threading.Event() + release = threading.Event() + + def probe(*_args, **_kwargs): + entered.set() + assert release.wait(5) + return None + + monkeypatch.setattr(lark_cli, "_probe_provisioner_capabilities", probe) + with ThreadPoolExecutor(max_workers=2) as pool: + pending = pool.submit(lark_cli.sandbox_lark_broker_active, config) + try: + assert entered.wait(3) + cached = pool.submit(lark_cli.sandbox_lark_broker_active, other) + assert cached.result(timeout=3) is False + finally: + release.set() + assert pending.result(timeout=3) is True + + +def test_lark_cli_env_from_runtime_refuses_unverified_broker_mode(): + runtime = SimpleNamespace(context={"user_id": "alice"}) + sandbox = SimpleNamespace(lark_cli_broker=None) + + with pytest.raises(RuntimeError, match="unverified"): + _lark_cli_env_from_runtime(runtime, "lark-cli auth status", sandbox_paths=True, sandbox=sandbox) + + +def test_aio_constructor_without_attestation_refuses_lark_commands(monkeypatch): + from deerflow.community.aio_sandbox import aio_sandbox + + monkeypatch.setattr(aio_sandbox, "AioSandboxClient", MagicMock()) + sandbox = aio_sandbox.AioSandbox("unattested", "https://sandbox.example.test") + overlay = MagicMock(return_value={"LARKSUITE_CLI_CONFIG_DIR": "/mnt/config"}) + monkeypatch.setattr(lark_cli, "lark_cli_env_overlay", overlay) + try: + with pytest.raises(RuntimeError, match="unverified"): + _lark_cli_env_from_runtime(SimpleNamespace(context={"user_id": "alice"}), "lark-cli auth status", sandbox_paths=True, sandbox=sandbox) + overlay.assert_not_called() + finally: + sandbox.close() + + +@pytest.mark.parametrize("broker", [False, True]) +def test_lark_command_overlay_uses_actual_sandbox_mode(monkeypatch, broker): + runtime = SimpleNamespace(context={"user_id": "alice"}) + sandbox = SimpleNamespace(lark_cli_broker=broker) + probe = MagicMock(side_effect=AssertionError("execution must use the sandbox's mode")) + monkeypatch.setattr(lark_cli, "sandbox_lark_broker_active", probe) + + env = _lark_cli_env_from_runtime(runtime, "lark-cli auth status", sandbox_paths=True, sandbox=sandbox) + + assert ("DEERFLOW_LARK_BROKER_URL" in env) is broker + assert ("LARKSUITE_CLI_CONFIG_DIR" in env) is not broker + probe.assert_not_called() + + +def test_brokerless_sandbox_provider_keeps_non_broker_overlay(monkeypatch): + """Providers without a broker sidecar keep the pre-attestation behavior. + + E2B, OpenSandbox, Tenki, and Boxlite subclass ``Sandbox`` without their + own ``lark_cli_broker`` attestation; the base-class ``False`` declares + the only mode that exists for them, so the fail-closed raise stays + scoped to AioSandbox's deliberate ``None`` default (#6436 review). + """ + + class _BrokerlessSandbox(Sandbox): + def execute_command(self, command, env=None, timeout=None): + raise NotImplementedError + + def read_file(self, path, start_line=None, end_line=None): + raise NotImplementedError + + def download_file(self, path): + raise NotImplementedError + + def list_dir(self, path, max_depth=2): + raise NotImplementedError + + def write_file(self, path, content, append=False): + raise NotImplementedError + + def glob(self, path, pattern, *, include_dirs=False, max_results=200): + raise NotImplementedError + + def grep(self, path, pattern, *, glob=None, literal=False, case_sensitive=False, max_results=100): + raise NotImplementedError + + def update_file(self, path, content): + raise NotImplementedError + + runtime = SimpleNamespace(context={"user_id": "alice"}) + sandbox = _BrokerlessSandbox("foreign-provider") + probe = MagicMock(side_effect=AssertionError("execution must use the sandbox's mode")) + monkeypatch.setattr(lark_cli, "sandbox_lark_broker_active", probe) + + env = _lark_cli_env_from_runtime(runtime, "lark-cli auth status", sandbox_paths=True, sandbox=sandbox) + + assert "DEERFLOW_LARK_BROKER_URL" not in env + assert "LARKSUITE_CLI_CONFIG_DIR" in env + probe.assert_not_called() + + def test_install_lark_integration_is_idempotent_across_reinstalls(monkeypatch, tmp_path): reset_skill_storage() _patch_paths(monkeypatch, tmp_path / "home") @@ -2179,8 +2490,9 @@ def test_lark_cli_json_rehardens_auth_files_written_by_cli(monkeypatch, tmp_path def test_lark_cli_env_from_runtime_uses_container_paths_for_sandbox_lark_commands(): runtime = SimpleNamespace(context={"user_id": "alice"}) + sandbox = SimpleNamespace(lark_cli_broker=False) - env = _lark_cli_env_from_runtime(runtime, "/usr/bin/lark-cli auth status", sandbox_paths=True) + env = _lark_cli_env_from_runtime(runtime, "/usr/bin/lark-cli auth status", sandbox_paths=True, sandbox=sandbox) assert env is not None assert env["LARKSUITE_CLI_CONFIG_DIR"] == lark_cli.LARK_CLI_SANDBOX_CONFIG_DIR diff --git a/backend/tests/test_provisioner_pvc_volumes.py b/backend/tests/test_provisioner_pvc_volumes.py index bbc2f77322..ba9c895c8d 100644 --- a/backend/tests/test_provisioner_pvc_volumes.py +++ b/backend/tests/test_provisioner_pvc_volumes.py @@ -792,17 +792,13 @@ class TestLarkCliBrokerSidecar: ), ] - def test_no_sidecar_when_broker_image_unset(self, provisioner_module): + def test_broker_request_rejected_when_image_unset(self, provisioner_module): provisioner_module.SKILLS_PVC_NAME = "" provisioner_module.USERDATA_PVC_NAME = "" provisioner_module.LARK_CLI_BROKER_IMAGE = "" - pod = provisioner_module._build_pod( - "sandbox-1", - "thread-1", - provision_lark_cli_broker=True, - ) - container_names = {c.name for c in pod.spec.containers} - assert "lark-cli-broker" not in container_names + with pytest.raises(provisioner_module.HTTPException) as error: + provisioner_module._build_pod("sandbox-1", "thread-1", provision_lark_cli_broker=True) + assert error.value.status_code == 503 def test_no_sidecar_when_flag_disabled(self, provisioner_module): provisioner_module.SKILLS_PVC_NAME = "" diff --git a/backend/tests/test_provisioner_request_threading.py b/backend/tests/test_provisioner_request_threading.py index d3429fb96a..1fb81f9735 100644 --- a/backend/tests/test_provisioner_request_threading.py +++ b/backend/tests/test_provisioner_request_threading.py @@ -18,6 +18,31 @@ from blockbuster import BlockBuster from kubernetes.client.rest import ApiException +def test_presence_checks_pod_without_requiring_service_or_access_url(monkeypatch, provisioner_module): + core = MagicMock() + core.read_namespaced_pod.return_value = SimpleNamespace(metadata=SimpleNamespace(uid="pod-generation"), status=SimpleNamespace(phase="Pending")) + monkeypatch.setattr(provisioner_module, "core_v1", core) + access_url = MagicMock(side_effect=AssertionError("Service readiness cannot attest Pod absence")) + monkeypatch.setattr(provisioner_module, "_sandbox_access_url", access_url) + + assert provisioner_module.sandbox_presence("sandbox-id").model_dump() == {"sandbox_id": "sandbox-id", "exists": True, "container_id": "pod-generation"} + access_url.assert_not_called() + core.read_namespaced_pod.assert_called_once_with("sandbox-sandbox-id", provisioner_module.K8S_NAMESPACE) + + +@pytest.mark.parametrize("status", [404, 403, 503]) +def test_presence_only_attests_absence_on_kubernetes_not_found(monkeypatch, provisioner_module, status): + core = MagicMock() + core.read_namespaced_pod.side_effect = ApiException(status=status) + monkeypatch.setattr(provisioner_module, "core_v1", core) + if status == 404: + assert provisioner_module.sandbox_presence("sandbox-id").model_dump() == {"sandbox_id": "sandbox-id", "exists": False, "container_id": None} + else: + with pytest.raises(provisioner_module.HTTPException) as error: + provisioner_module.sandbox_presence("sandbox-id") + assert error.value.status_code == 500 + + def test_provisioner_thread_id_pattern_matches_gateway_contract(provisioner_module) -> None: from deerflow.utils.thread_id import THREAD_ID_PATTERN @@ -78,6 +103,37 @@ def test_provisioner_threads_shell_capacity_into_sandbox_pod( assert env["MAX_SHELL_SESSIONS"] == "13" +def test_broker_request_without_image_cannot_mount_credentials(monkeypatch, provisioner_module): + core = _RecordingCoreV1(event_loop_thread_id=-1) + monkeypatch.setattr(provisioner_module, "core_v1", core) + monkeypatch.setattr(provisioner_module, "LARK_CLI_BROKER_IMAGE", "") + with pytest.raises(provisioner_module.HTTPException) as error: + provisioner_module.create_sandbox(provisioner_module.CreateSandboxRequest(sandbox_id="new-sandbox", provision_lark_cli_broker=True)) + assert error.value.status_code == 503 + assert core.created_pods == [] + + +def test_broker_request_cannot_reuse_non_broker_pod(monkeypatch, provisioner_module): + core = _RecordingCoreV1(event_loop_thread_id=-1) + monkeypatch.setattr(provisioner_module, "core_v1", core) + monkeypatch.setattr(provisioner_module, "LARK_CLI_BROKER_IMAGE", "broker-image") + with pytest.raises(provisioner_module.HTTPException) as error: + provisioner_module.create_sandbox(provisioner_module.CreateSandboxRequest(sandbox_id="sandbox-existing", provision_lark_cli_broker=True)) + assert error.value.status_code == 409 + assert core.created_pods == [] + assert "sandbox-existing" in core.service_sandboxes + + +def test_stale_negative_capability_cannot_bypass_configured_broker(monkeypatch, provisioner_module): + core = _RecordingCoreV1(event_loop_thread_id=-1) + monkeypatch.setattr(provisioner_module, "core_v1", core) + monkeypatch.setattr(provisioner_module, "LARK_CLI_BROKER_IMAGE", "broker-image") + with pytest.raises(provisioner_module.HTTPException) as error: + provisioner_module.create_sandbox(provisioner_module.CreateSandboxRequest(sandbox_id="new-sandbox", provision_lark_cli_runtime=True, provision_lark_cli_broker=False)) + assert error.value.status_code == 409 + assert core.created_pods == [] + + def test_provisioner_rejects_insufficient_capacity_without_replacing_existing_pod( monkeypatch: pytest.MonkeyPatch, provisioner_module, @@ -120,7 +176,7 @@ def test_capacity_replacement_recovers_when_service_outlives_old_pod(monkeypatch def test_capacity_read_failure_does_not_authorize_creation(monkeypatch, provisioner_module, failure): core = _RecordingCoreV1(event_loop_thread_id=-1) monkeypatch.setattr(provisioner_module, "core_v1", core) - monkeypatch.setattr(provisioner_module, "_get_pod_shell_capacity", MagicMock(side_effect=failure)) + monkeypatch.setattr(provisioner_module, "_pod_shell_capacity", MagicMock(side_effect=failure)) with pytest.raises(provisioner_module.HTTPException) as error: provisioner_module.create_sandbox(provisioner_module.CreateSandboxRequest(sandbox_id="sandbox-existing", max_shell_sessions=13)) @@ -143,7 +199,7 @@ def test_list_sandboxes_skips_invalid_capacity_metadata( monkeypatch.setattr( provisioner_module, - "_get_pod_shell_capacity", + "_pod_shell_capacity", invalid_capacity, ) @@ -241,6 +297,119 @@ async def test_capacity_upgrade_preserves_peer_pod_after_discovery_error(monkeyp new._acquire_serializer.close() +@pytest.mark.asyncio +@pytest.mark.parametrize("async_acquire", [False, True], ids=["sync", "async"]) +async def test_quarantined_pod_cleanup_recovers_after_partial_delete(monkeypatch, tmp_path, provisioner_module, async_acquire): + """A lost Service must not strand the fenced Pod after a temporary delete failure.""" + from test_sandbox_orphan_reconciliation import _make_provider_for_reconciliation + + from deerflow.community.aio_sandbox import aio_sandbox_provider as provider_mod + from deerflow.community.aio_sandbox import remote_backend as remote_mod + from deerflow.community.aio_sandbox.quarantine import SandboxQuarantine + from deerflow.community.aio_sandbox.sandbox_info import SandboxInfo + from deerflow.config.paths import Paths + + provider = _make_provider_for_reconciliation() + sid = "sandbox-existing" + core = _RecordingCoreV1(event_loop_thread_id=threading.get_ident()) + old_info = SandboxInfo(sid, "http://sandbox", container_id="old-pod") + provider._backend = remote_mod.RemoteSandboxBackend("http://provisioner:8002") + provider._quarantine = SandboxQuarantine(tmp_path / "quarantine", "provisioner") + sandbox = MagicMock() + sandbox.requires_container_recycle = True + provider._sandboxes[sid] = sandbox + provider._sandbox_infos[sid] = old_info + provider._thread_sandboxes[("user-a", "thread-a")] = sid + provider._active_sandbox_identity[sid] = ("user-a", "thread-a") + provider._publish_ownership(sid) + monkeypatch.setattr(provisioner_module, "core_v1", core) + monkeypatch.setattr(provider_mod, "get_paths", lambda: Paths(base_dir=tmp_path / "state")) + monkeypatch.setattr(provider, "_ensure_skills_projection", lambda *_args: None) + monkeypatch.setattr(provider, "_thread_skill_projection_active", lambda *_args: False) + monkeypatch.setattr(provider, "_sandbox_id_for_thread", lambda *_args: sid) + monkeypatch.setattr(provider, "_get_extra_mounts", lambda *_args, **_kwargs: []) + monkeypatch.setattr(provider, "_lark_integration_active", lambda *_args: False) + monkeypatch.setattr(provider, "_lark_broker_active", lambda *_args: False) + monkeypatch.setattr(provider_mod, "wait_for_sandbox_ready", lambda *_args, **_kwargs: True) + monkeypatch.setattr(provider_mod, "wait_for_sandbox_ready_async", AsyncMock(return_value=True)) + monkeypatch.setattr(provider_mod, "AioSandbox", MagicMock()) + monkeypatch.setattr(remote_mod, "user_should_see_legacy_skills", lambda _uid: False) + original_read_pod = core.read_namespaced_pod + original_delete_pod = core.delete_namespaced_pod + deleted_under_fence = [] + + def read_pod(*args): + pod = original_read_pod(*args) + if not core.created_pods: + pod.metadata.uid = old_info.container_id + return pod + + def delete_pod(*args): + deleted_under_fence.append((provider._ownership.owner(sid), sid in provider._local_teardown)) + if len(deleted_under_fence) == 1: + raise ApiException(status=503, reason="temporary Kubernetes outage") + original_delete_pod(*args) + + def response(status, payload): + result = requests.Response() + result.status_code = status + result._content = json.dumps(payload).encode() + return result + + def get(url, **_kwargs): + try: + if url.endswith("/presence"): + payload = provisioner_module.sandbox_presence(sid).model_dump() + elif url.endswith("/api/sandboxes"): + payload = provisioner_module.list_sandboxes() + else: + payload = provisioner_module.get_sandbox(sid).model_dump() + return response(200, payload) + except provisioner_module.HTTPException as exc: + return response(exc.status_code, {"detail": exc.detail}) + + def post(_url, *, json, **_kwargs): + try: + result = provisioner_module.create_sandbox(provisioner_module.CreateSandboxRequest(**json)) + return response(200, result.model_dump()) + except provisioner_module.HTTPException as exc: + return response(exc.status_code, {"detail": exc.detail}) + + def delete(_url, **_kwargs): + try: + return response(200, provisioner_module.destroy_sandbox(sid)) + except provisioner_module.HTTPException as exc: + return response(exc.status_code, {"detail": exc.detail}) + + monkeypatch.setattr(core, "read_namespaced_pod", read_pod) + monkeypatch.setattr(core, "delete_namespaced_pod", delete_pod) + monkeypatch.setattr(core, "list_namespaced_service", lambda *_args, **_kwargs: SimpleNamespace(items=[_node_port_service(item) for item in core.service_sandboxes])) + monkeypatch.setattr(requests, "get", get) + monkeypatch.setattr(requests, "post", post) + monkeypatch.setattr(requests, "delete", delete) + try: + await asyncio.to_thread(provider.release, sid) + assert sid not in core.service_sandboxes + assert sid in core.pod_shell_capacities + assert provider._quarantine.contains(old_info) + await asyncio.to_thread(provider._reconcile_orphans) + + if async_acquire: + acquired = await provider.acquire_async("thread-a", user_id="user-a") + else: + acquired = await asyncio.to_thread(provider.acquire, "thread-a", user_id="user-a") + + assert acquired == sid + assert len(deleted_under_fence) == 2 + assert deleted_under_fence == [(provider._owner_id, True)] * 2 + assert sid in core.service_sandboxes + assert provider._sandbox_infos[sid].container_id != old_info.container_id + assert not provider._quarantine.contains(old_info) + finally: + provider.reset() + provider._ownership.close() + + class _RecordingCoreV1: def __init__( self, @@ -294,8 +463,11 @@ class _RecordingCoreV1: if capacity is None: raise ApiException(status=404) return SimpleNamespace( + metadata=SimpleNamespace(uid=f"pod-{sandbox_id}-{len(self.created_pods)}", deletion_timestamp=None), status=SimpleNamespace(phase="Running"), - spec=SimpleNamespace( + spec=self.created_pod_specs[sandbox_id].spec + if sandbox_id in self.created_pod_specs + else SimpleNamespace( containers=[ SimpleNamespace( name="sandbox", @@ -362,6 +534,7 @@ def test_sandbox_business_route_handlers_are_sync(provisioner_module) -> None: provisioner_module.create_sandbox, provisioner_module.destroy_sandbox, provisioner_module.get_sandbox, + provisioner_module.sandbox_presence, provisioner_module.list_sandboxes, ): assert not inspect.iscoroutinefunction(handler) @@ -375,9 +548,10 @@ def test_sandbox_business_route_handlers_are_sync(provisioner_module) -> None: ("POST", "/api/sandboxes", {"sandbox_id": "sandbox-new", "thread_id": "thread-1", "user_id": "user-1"}, "sandbox-new"), ("DELETE", "/api/sandboxes/sandbox-existing", None, None), ("GET", "/api/sandboxes/sandbox-existing", None, None), + ("GET", "/api/sandboxes/sandbox-existing/presence", None, None), ("GET", "/api/sandboxes", None, None), ], - ids=["create-existing", "create-new", "destroy", "get", "list"], + ids=["create-existing", "create-new", "destroy", "get", "presence", "list"], ) async def test_sandbox_business_routes_run_k8s_client_off_event_loop_thread( method: str, diff --git a/backend/tests/test_provisioner_sandbox_responses.py b/backend/tests/test_provisioner_sandbox_responses.py new file mode 100644 index 0000000000..9fc1ed0dcf --- /dev/null +++ b/backend/tests/test_provisioner_sandbox_responses.py @@ -0,0 +1,166 @@ +"""Regression tests for provisioner lark conflict markers and Terminating handling.""" + +from __future__ import annotations + +from types import SimpleNamespace +from unittest.mock import MagicMock + +import pytest + + +def _pod(*, phase: str = "Running", deletion_timestamp=None, broker: bool = False) -> SimpleNamespace: + containers = [SimpleNamespace(name="lark-cli-broker")] if broker else [] + init_containers = [SimpleNamespace(name="lark-cli-shim-init")] if broker else [] + return SimpleNamespace( + metadata=SimpleNamespace(uid="pod-generation", deletion_timestamp=deletion_timestamp), + status=SimpleNamespace(phase=phase), + spec=SimpleNamespace(containers=containers, init_containers=init_containers), + ) + + +@pytest.mark.parametrize( + "broker_image, runtime, broker, expected_status", + [ + ("", False, True, 503), # broker requested but the image is not configured + ("broker-image", True, False, 409), # broker configured but the request says non-broker + ], +) +def test_lark_provisioning_conflicts_carry_capability_refresh_marker(monkeypatch, provisioner_module, broker_image, runtime, broker, expected_status): + """Both lark config conflicts tell the Gateway to drop its cached capability.""" + monkeypatch.setattr(provisioner_module, "LARK_CLI_BROKER_IMAGE", broker_image) + + with pytest.raises(provisioner_module.HTTPException) as error: + provisioner_module._validate_lark_provisioning_request(runtime, broker) + + assert error.value.status_code == expected_status + assert error.value.headers[provisioner_module.CAPABILITY_REFRESH_HEADER] == "lark-broker" + + +def test_lark_provisioning_request_without_conflict_passes(monkeypatch, provisioner_module): + monkeypatch.setattr(provisioner_module, "LARK_CLI_BROKER_IMAGE", "broker-image") + provisioner_module._validate_lark_provisioning_request(True, True) + provisioner_module._validate_lark_provisioning_request(False, False) + + +@pytest.mark.parametrize( + "requirements", + [ + {"required_broker": False}, + {"required_broker": True}, + {"required_capacity": 5}, + ], +) +def test_sandbox_response_rejects_terminating_pod_when_create_requirements_present(monkeypatch, provisioner_module, requirements): + """A create coordination request must not settle on a Terminating Pod.""" + # The Pod must satisfy the requested mode, or the pre-existing mode check + # fires first; this test pins the Terminating check specifically. + pod = _pod(deletion_timestamp="2026-10-07T00:00:00Z", broker=requirements.get("required_broker") is True) + core = MagicMock() + core.read_namespaced_pod.return_value = pod + monkeypatch.setattr(provisioner_module, "core_v1", core) + + with pytest.raises(provisioner_module.HTTPException) as error: + provisioner_module._sandbox_response("sandbox-1", "http://sandbox", **requirements) + + assert error.value.status_code == 409 + assert "terminating" in error.value.detail + + +def test_sandbox_response_broker_conflict_carries_capability_refresh_marker(monkeypatch, provisioner_module): + """A post-build broker-mode 409 must also tell the Gateway to drop its cached + capability observation, or the change lives out the negative cache TTL.""" + core = MagicMock() + core.read_namespaced_pod.return_value = _pod(broker=False) + monkeypatch.setattr(provisioner_module, "core_v1", core) + + with pytest.raises(provisioner_module.HTTPException) as error: + provisioner_module._sandbox_response("sandbox-1", "http://sandbox", required_broker=True) + + assert error.value.status_code == 409 + assert error.value.headers[provisioner_module.CAPABILITY_REFRESH_HEADER] == provisioner_module.CAPABILITY_REFRESH_LARK_BROKER + + +@pytest.mark.parametrize("broker", [False, True]) +def test_sandbox_response_attests_pattern_a_and_b_modes(monkeypatch, provisioner_module, broker): + """Pod observation attests both real layouts: Pattern A (init image only) + reads attested non-broker; Pattern B (shim + sidecar) reads broker.""" + if broker: + pod = _pod(broker=True) + else: + # Pattern A: the init container is `lark-cli-init` (not the broker's + # shim-init), no sidecar, credentials mounted on the sandbox container. + pod = SimpleNamespace( + metadata=SimpleNamespace(uid="pod-generation", deletion_timestamp=None), + status=SimpleNamespace(phase="Running"), + spec=SimpleNamespace( + containers=[ + SimpleNamespace( + name="sandbox", + volume_mounts=[SimpleNamespace(mount_path=f"{provisioner_module.LARK_CLI_CONFIG_CONTAINER_PATH}/u")], + ) + ], + init_containers=[SimpleNamespace(name="lark-cli-init")], + ), + ) + core = MagicMock() + core.read_namespaced_pod.return_value = pod + monkeypatch.setattr(provisioner_module, "core_v1", core) + + response = provisioner_module._sandbox_response("sandbox-1", "http://sandbox", required_broker=broker) + + assert response.lark_cli_broker is broker + + +def test_sandbox_response_reports_terminating_status_without_requirements(monkeypatch, provisioner_module): + """get/list pass no requirements: the phase is data, not an error.""" + core = MagicMock() + core.read_namespaced_pod.return_value = _pod(deletion_timestamp="2026-10-07T00:00:00Z") + monkeypatch.setattr(provisioner_module, "core_v1", core) + + response = provisioner_module._sandbox_response("sandbox-1", "http://sandbox") + + assert response.status == "Terminating" + assert response.sandbox_url == "http://sandbox" + + +def test_list_sandboxes_keeps_terminating_entries_visible(monkeypatch, provisioner_module): + """One Terminating Pod must not fail or empty the whole fleet listing.""" + core = MagicMock() + + def read_pod(name, namespace): + assert namespace == provisioner_module.K8S_NAMESPACE + if name == "sandbox-sandbox-terminating": + return _pod(deletion_timestamp="2026-10-07T00:00:00Z") + return _pod(phase="Running") + + core.read_namespaced_pod.side_effect = read_pod + core.list_namespaced_service.return_value = SimpleNamespace( + items=[ + SimpleNamespace( + metadata=SimpleNamespace(labels={"sandbox-id": "sandbox-terminating"}), + spec=SimpleNamespace(ports=[SimpleNamespace(name="http", node_port=31001)]), + ), + SimpleNamespace( + metadata=SimpleNamespace(labels={"sandbox-id": "sandbox-running"}), + spec=SimpleNamespace(ports=[SimpleNamespace(name="http", node_port=31002)]), + ), + ] + ) + monkeypatch.setattr(provisioner_module, "core_v1", core) + + payload = provisioner_module.list_sandboxes() + + assert payload["count"] == 2 + statuses = {entry.sandbox_id: entry.status for entry in payload["sandboxes"]} + assert statuses == {"sandbox-terminating": "Terminating", "sandbox-running": "Running"} + + +def test_sandbox_response_running_pod_without_broker_requirement_still_succeeds(monkeypatch, provisioner_module): + core = MagicMock() + core.read_namespaced_pod.return_value = _pod(phase="Running") + monkeypatch.setattr(provisioner_module, "core_v1", core) + + response = provisioner_module._sandbox_response("sandbox-1", "http://sandbox") + + assert response.status == "Running" + assert response.sandbox_url == "http://sandbox" diff --git a/backend/tests/test_remote_sandbox_backend.py b/backend/tests/test_remote_sandbox_backend.py index e6a8c3ae93..78e84a89a7 100644 --- a/backend/tests/test_remote_sandbox_backend.py +++ b/backend/tests/test_remote_sandbox_backend.py @@ -1,5 +1,8 @@ from __future__ import annotations +import time +from unittest.mock import Mock + import pytest import requests @@ -17,16 +20,20 @@ class _StubResponse: status_code: int = 200, payload: object | None = None, json_exc: Exception | None = None, + headers: dict | None = None, ): self.status_code = status_code self._payload = {} if payload is None else payload self._json_exc = json_exc + self.headers = headers or {} self.ok = 200 <= status_code < 400 self.text = "" def raise_for_status(self) -> None: if self.status_code >= 400: - raise requests.HTTPError(f"HTTP {self.status_code}") + error = requests.HTTPError(f"HTTP {self.status_code}") + error.response = self + raise error def json(self) -> object: if self._json_exc is not None: @@ -34,6 +41,92 @@ class _StubResponse: return self._payload +@pytest.mark.parametrize("status", ["Running", "Pending", "Terminating", "Failed"]) +def test_absence_requires_missing_pod_not_an_unhealthy_phase(monkeypatch, status): + backend = RemoteSandboxBackend("http://provisioner:8002", api_key="test-key") + calls = [] + + def get(url, **kwargs): + calls.append((url, kwargs["headers"])) + return _StubResponse(payload={"sandbox_id": "sandbox-id", "exists": True, "status": status}) + + monkeypatch.setattr(requests, "get", get) + + assert backend.is_absent("sandbox-id") is False + assert calls == [("http://provisioner:8002/api/sandboxes/sandbox-id/presence", {"X-API-Key": "test-key"})] + + +def test_absence_accepts_only_confirmed_not_found(monkeypatch): + backend = RemoteSandboxBackend("http://provisioner:8002") + monkeypatch.setattr(requests, "get", lambda *_args, **_kwargs: _StubResponse(payload={"sandbox_id": "sandbox-id", "exists": False})) + assert backend.is_absent("sandbox-id") is True + # Old provisioners can report 404 for a missing Service while the Pod lives. + monkeypatch.setattr(requests, "get", lambda *_args, **_kwargs: _StubResponse(status_code=404)) + with pytest.raises(RuntimeError): + backend.is_absent("sandbox-id") + + +@pytest.mark.parametrize("payload", [{}, {"exists": False}, {"sandbox_id": "another-id", "exists": False}, {"sandbox_id": "sandbox-id", "exists": "false"}]) +def test_absence_rejects_unattested_or_mismatched_responses(monkeypatch, payload): + backend = RemoteSandboxBackend("http://provisioner:8002") + monkeypatch.setattr(requests, "get", lambda *_args, **_kwargs: _StubResponse(payload=payload)) + with pytest.raises(RuntimeError): + backend.is_absent("sandbox-id") + + +@pytest.mark.parametrize("generation", ["pod-generation", None]) +def test_runtime_inspection_preserves_pod_identity_without_a_service(monkeypatch, generation): + backend = RemoteSandboxBackend("http://provisioner:8002", api_key="test-key") + calls = [] + + def get(url, **kwargs): + calls.append((url, kwargs["headers"])) + return _StubResponse(payload={"sandbox_id": "sandbox-id", "exists": True, "container_id": generation}) + + monkeypatch.setattr(requests, "get", get) + + info = backend.inspect_runtime("sandbox-id") + assert info.sandbox_id == "sandbox-id" + assert info.container_id == generation + assert info.sandbox_url == "" + assert calls == [("http://provisioner:8002/api/sandboxes/sandbox-id/presence", {"X-API-Key": "test-key"})] + + +def test_runtime_inspection_returns_none_only_after_attested_absence(monkeypatch): + backend = RemoteSandboxBackend("http://provisioner:8002") + monkeypatch.setattr(requests, "get", lambda *_args, **_kwargs: _StubResponse(payload={"sandbox_id": "sandbox-id", "exists": False})) + assert backend.inspect_runtime("sandbox-id") is None + + +@pytest.mark.parametrize( + "payload", + [ + {"sandbox_id": "sandbox-id", "exists": True, "container_id": False}, + {"sandbox_id": "sandbox-id", "exists": True, "container_id": ""}, + {"sandbox_id": "sandbox-id", "exists": False, "container_id": "still-present"}, + ], +) +def test_runtime_inspection_rejects_inconsistent_generation_metadata(monkeypatch, payload): + backend = RemoteSandboxBackend("http://provisioner:8002") + monkeypatch.setattr(requests, "get", lambda *_args, **_kwargs: _StubResponse(payload=payload)) + with pytest.raises(RuntimeError): + backend.inspect_runtime("sandbox-id") + + +@pytest.mark.parametrize("failure", [requests.ConnectionError("provisioner unavailable"), ValueError("invalid response JSON")]) +def test_runtime_inspection_errors_do_not_attest_absence(monkeypatch, failure): + backend = RemoteSandboxBackend("http://provisioner:8002") + + def get(*_args, **_kwargs): + if isinstance(failure, requests.RequestException): + raise failure + return _StubResponse(json_exc=failure) + + monkeypatch.setattr(requests, "get", get) + with pytest.raises(RuntimeError): + backend.inspect_runtime("sandbox-id") + + @pytest.mark.parametrize( "container_path", [ @@ -273,6 +366,81 @@ def test_provisioner_create_returns_sandbox_info(monkeypatch): assert info.sandbox_url == "http://k3s:31001" +@pytest.mark.parametrize("reported", [None, False, True]) +def test_broker_create_requires_attested_mode(monkeypatch, reported): + backend = RemoteSandboxBackend("http://provisioner:8002") + monkeypatch.setattr(remote_backend_mod, "user_should_see_legacy_skills", lambda _user_id: False) + payload = {"sandbox_url": "http://sandbox", "container_id": "pod-generation", "lark_cli_broker": reported} + monkeypatch.setattr(requests, "post", lambda *_args, **_kwargs: _StubResponse(payload=payload)) + if reported is not True: + with pytest.raises(RuntimeError, match="broker"): + backend.create("thread", "sandbox", provision_lark_cli_broker=True) + else: + info = backend.create("thread", "sandbox", provision_lark_cli_broker=True) + assert info.lark_cli_broker is True + assert info.container_id == "pod-generation" + + +@pytest.mark.parametrize("status", [409, 503]) +def test_create_config_conflict_marker_drops_cached_lark_broker_mode(monkeypatch, status): + """A marked 409/503 means the provisioner's lark config changed: re-probe next acquire.""" + from deerflow.integrations import lark_cli + + backend = RemoteSandboxBackend("http://provisioner:8002", api_key="test-key") + monkeypatch.setattr(remote_backend_mod, "user_should_see_legacy_skills", lambda _user_id: False) + cache_key = ("http://provisioner:8002", "test-key") + stale = lark_cli._LarkBrokerModeCacheEntry(False, time.monotonic()) + monkeypatch.setattr(lark_cli.sandbox_lark_broker_active, "_cache", {cache_key: stale}, raising=False) + headers = {lark_cli.PROVISIONER_CAPABILITY_REFRESH_HEADER: lark_cli.PROVISIONER_CAPABILITY_REFRESH_LARK_BROKER} + monkeypatch.setattr(requests, "post", lambda *_args, **_kwargs: _StubResponse(status_code=status, headers=headers)) + + with pytest.raises(RuntimeError, match="Provisioner create failed"): + backend._provisioner_create("thread-1", "abc123", provision_lark_cli_runtime=True) + + assert cache_key not in lark_cli.sandbox_lark_broker_active._cache + + +def test_create_coordination_conflict_keeps_cached_lark_broker_mode(monkeypatch): + """A marker-less 409 (capacity / incompatible Pod / terminating) is not a config change.""" + from deerflow.integrations import lark_cli + + backend = RemoteSandboxBackend("http://provisioner:8002", api_key="test-key") + monkeypatch.setattr(remote_backend_mod, "user_should_see_legacy_skills", lambda _user_id: False) + cache_key = ("http://provisioner:8002", "test-key") + cached = lark_cli._LarkBrokerModeCacheEntry(False, time.monotonic()) + monkeypatch.setattr(lark_cli.sandbox_lark_broker_active, "_cache", {cache_key: cached}, raising=False) + monkeypatch.setattr(requests, "post", lambda *_args, **_kwargs: _StubResponse(status_code=409)) + + with pytest.raises(RuntimeError, match="Provisioner create failed"): + backend._provisioner_create("thread-1", "abc123", provision_lark_cli_runtime=True) + + assert lark_cli.sandbox_lark_broker_active._cache.get(cache_key) is cached + + +def test_create_request_error_without_response_keeps_cached_lark_broker_mode(monkeypatch): + from deerflow.integrations import lark_cli + + backend = RemoteSandboxBackend("http://provisioner:8002", api_key="test-key") + monkeypatch.setattr(remote_backend_mod, "user_should_see_legacy_skills", lambda _user_id: False) + cache_key = ("http://provisioner:8002", "test-key") + cached = lark_cli._LarkBrokerModeCacheEntry(False, time.monotonic()) + monkeypatch.setattr(lark_cli.sandbox_lark_broker_active, "_cache", {cache_key: cached}, raising=False) + monkeypatch.setattr(requests, "post", Mock(side_effect=requests.ConnectionError("unreachable"))) + + with pytest.raises(RuntimeError, match="Provisioner create failed"): + backend._provisioner_create("thread-1", "abc123", provision_lark_cli_runtime=True) + + assert lark_cli.sandbox_lark_broker_active._cache.get(cache_key) is cached + + +def test_discovery_reports_actual_broker_mode_and_generation(monkeypatch): + backend = RemoteSandboxBackend("http://provisioner:8002") + monkeypatch.setattr(requests, "get", lambda *_args, **_kwargs: _StubResponse(payload={"sandbox_url": "http://sandbox", "container_id": "pod-generation", "lark_cli_broker": True})) + info = backend.discover("sandbox") + assert info.lark_cli_broker is True + assert info.container_id == "pod-generation" + + def test_provisioner_create_forwards_supported_extra_mounts(monkeypatch): backend = RemoteSandboxBackend("http://provisioner:8002") monkeypatch.setattr(remote_backend_mod, "user_should_see_legacy_skills", lambda user_id: False) @@ -360,7 +528,7 @@ def test_provisioner_create_strips_runtime_mount_when_init_container_enabled(mon def mock_post(url: str, json: dict, timeout: int, headers=None): captured.update(json) - return _StubResponse(payload={"sandbox_id": "abc123", "sandbox_url": "http://k3s:31001"}) + return _StubResponse(payload={"sandbox_id": "abc123", "sandbox_url": "http://k3s:31001", "lark_cli_broker": False}) monkeypatch.setattr(requests, "post", mock_post) @@ -476,7 +644,7 @@ def test_provisioner_destroy_calls_delete(monkeypatch): backend._provisioner_destroy("abc123") -def test_provisioner_destroy_swallows_request_exception(monkeypatch): +def test_provisioner_destroy_propagates_request_exception(monkeypatch): backend = RemoteSandboxBackend("http://provisioner:8002") def mock_delete(url: str, timeout: int, headers=None): @@ -484,7 +652,8 @@ def test_provisioner_destroy_swallows_request_exception(monkeypatch): monkeypatch.setattr(requests, "delete", mock_delete) - backend._provisioner_destroy("abc123") + with pytest.raises(RuntimeError, match="destroy failed"): + backend._provisioner_destroy("abc123") def test_is_alive_delegates_to_provisioner_is_alive(monkeypatch): diff --git a/backend/tests/test_sandbox_authorization.py b/backend/tests/test_sandbox_authorization.py index 0f2f398b99..22844426e1 100644 --- a/backend/tests/test_sandbox_authorization.py +++ b/backend/tests/test_sandbox_authorization.py @@ -359,6 +359,59 @@ def test_authorize_sandbox_calls_provider_with_correct_request(monkeypatch): # artifact edit) still succeeds with the sandbox sync skipped. +def test_gateway_missing_client_keeps_infrastructure_error_lease(): + from app.gateway.authz import try_acquire_sandbox_for_request + from deerflow.sandbox.lease import discard_sandbox_lease_manager + + provider = MagicMock() + provider.acquire_async = AsyncMock(return_value="lost-after-acquire") + provider.get.return_value = None + + async def go(): + lease = await try_acquire_sandbox_for_request(None, provider, "thread-1", user_id="user-1", app_config=None) + assert lease.sandbox is None + assert lease.sandbox_id == "lost-after-acquire" + assert lease.denied is False + assert lease.owner_id is None + assert lease.provider is provider + await lease.release() + await lease.release() + + try: + asyncio.run(go()) + provider.release.assert_called_once_with("lost-after-acquire") + finally: + discard_sandbox_lease_manager(provider) + + +@pytest.mark.parametrize("failure", ["acquire", "lookup", "rollback"]) +def test_gateway_client_lease_preserves_provider_errors(failure): + from app.gateway.authz import try_acquire_sandbox_for_request + from deerflow.sandbox.lease import discard_sandbox_lease_manager + + error = RuntimeError(f"{failure} failed") + provider = MagicMock() + provider.acquire_async = AsyncMock(return_value="lost-after-acquire") + provider.get.return_value = None + if failure == "acquire": + provider.acquire_async.side_effect = error + elif failure == "lookup": + provider.get.side_effect = error + else: + provider.release.side_effect = error + + try: + with pytest.raises(RuntimeError) as exc_info: + asyncio.run(try_acquire_sandbox_for_request(None, provider, "thread-1", user_id="user-1", app_config=None)) + assert exc_info.value is error + if failure == "acquire": + provider.release.assert_not_called() + else: + provider.release.assert_called_once_with("lost-after-acquire") + finally: + discard_sandbox_lease_manager(provider) + + def _make_upload_app(monkeypatch, provider, *, fail_closed: bool = True): """Build a FastAPI app with the uploads router and authz enabled.""" from _router_auth_helpers import make_authed_test_app diff --git a/backend/tests/test_sandbox_leases.py b/backend/tests/test_sandbox_leases.py index 523d39d864..bd268309bc 100644 --- a/backend/tests/test_sandbox_leases.py +++ b/backend/tests/test_sandbox_leases.py @@ -6,6 +6,7 @@ from dataclasses import dataclass, field import pytest +from deerflow.sandbox.exceptions import SandboxNotFoundError from deerflow.sandbox.lease import ( SandboxLeaseManager, discard_sandbox_lease_manager, @@ -156,6 +157,157 @@ def test_last_execution_lease_is_the_only_provider_releaser() -> None: assert provider.sandbox.released_scopes == ["child-a", "parent", "child-b"] +def test_idle_teardown_reservation_excludes_live_holders_and_fork_retention() -> None: + provider = _LeaseProvider() + manager = SandboxLeaseManager(provider) + try: + manager.retain("parent", "shared", thread_id="parent-thread", user_id="user") + with manager.reserve_idle_teardown("shared") as reserved: + assert reserved is False + manager.release("parent") + + with manager.reserve_idle_teardown("shared") as reserved: + assert reserved is True + with manager.reserve_idle_teardown("shared") as nested: + assert nested is True + errors: list[Exception] = [] + + def retain_fork(): + try: + manager.retain("fork", "shared", thread_id="fork-thread", user_id="user", release_on_last=False) + except Exception as error: + errors.append(error) + + borrower = threading.Thread(target=retain_fork) + borrower.start() + borrower.join(timeout=5) + assert not borrower.is_alive() + assert len(errors) == 1 and "teardown" in str(errors[0]) + assert manager.binding_for("fork") is None + manager.retain("fork", "shared", thread_id="fork-thread", user_id="user", release_on_last=False) + assert manager.binding_for("fork") == "shared" + finally: + manager.close() + + +def test_failed_rebind_to_teardown_keeps_the_previous_execution_binding() -> None: + manager = SandboxLeaseManager(_LeaseProvider()) + try: + manager.retain("owner", "previous", thread_id="thread", user_id="user") + with manager.reserve_idle_teardown("shared") as reserved: + assert reserved is True + with pytest.raises(RuntimeError, match="teardown"): + manager.retain("owner", "shared", thread_id="thread", user_id="user") + assert manager.binding_for("owner") == "previous" + finally: + manager.close() + + +def test_execution_holder_survives_scope_cleanup_and_new_fork_defers_parking() -> None: + provider = _LeaseProvider() + manager = SandboxLeaseManager(provider) + manager.retain("parent", "shared", thread_id="parent-thread", user_id="user") + ordinary_cleanup = provider.sandbox.release_command_scope + + def cleanup(scope_id): + if scope_id == "parent": + assert manager.binding_for("parent") == "shared" + with manager.reserve_idle_teardown("shared") as reserved: + assert reserved is False + manager.retain("fork", "shared", thread_id="fork-thread", user_id="user", release_on_last=False) + ordinary_cleanup(scope_id) + + provider.sandbox.release_command_scope = cleanup + try: + manager.release("parent") + assert provider.release_calls == [] + assert manager.binding_for("fork") == "shared" + manager.release("fork") + assert provider.release_calls == ["shared"] + assert provider.sandbox.released_scopes == ["parent", "fork"] + finally: + manager.close() + + +@pytest.mark.parametrize("async_restore", [False, True], ids=["sync", "async"]) +def test_late_fork_lookup_cannot_bind_a_client_after_parent_parks_it(async_restore) -> None: + captured = threading.Event() + resume = threading.Event() + + class ParkingProvider(_LeaseProvider): + active = True + + def get(self, sandbox_id): + client = super().get(sandbox_id) if self.active else None + if threading.current_thread().name == "late-fork" and not captured.is_set(): + captured.set() + assert resume.wait(timeout=5) + return client + + def release(self, sandbox_id): + super().release(sandbox_id) + self.active = False + + def acquire(self, thread_id=None, *, user_id=None): + self.sandbox = _LeaseSandbox("replacement") + self.active = True + return super().acquire(thread_id, user_id=user_id) + + provider = ParkingProvider() + manager = SandboxLeaseManager(provider) + manager.retain("parent", "shared", thread_id="parent-thread", user_id="user") + results: list[str] = [] + errors: list[BaseException] = [] + + def restore_fork(): + try: + options = dict(thread_id="fork-thread", user_id="user", release_on_last=False, allow_unscoped_borrow=True) + result = asyncio.run(manager.reuse_or_acquire_async("fork", "shared", **options)) if async_restore else manager.reuse_or_acquire("fork", "shared", **options) + results.append(result) + except BaseException as error: + errors.append(error) + + fork = threading.Thread(target=restore_fork, name="late-fork") + fork.start() + try: + assert captured.wait(timeout=5) + manager.release("parent") + assert provider.release_calls == ["shared"] + resume.set() + fork.join(timeout=5) + assert not fork.is_alive() + assert errors == [] + assert results == ["replacement"] + assert manager.binding_for("fork") == "replacement" + assert provider.get("replacement") is provider.sandbox + manager.release("fork") + assert provider.release_calls == ["shared", "replacement"] + finally: + resume.set() + fork.join(timeout=5) + manager.close() + + +@pytest.mark.parametrize("async_acquire", [False, True], ids=["sync", "async"]) +def test_acquire_does_not_bind_a_client_that_disappeared_before_admission(async_acquire) -> None: + class MissingClientProvider(_LeaseProvider): + def get(self, sandbox_id): + return None + + provider = MissingClientProvider() + manager = SandboxLeaseManager(provider) + try: + with pytest.raises(SandboxNotFoundError, match="Sandbox not found after acquisition"): + if async_acquire: + asyncio.run(manager.acquire_async("owner", "thread", user_id="user")) + else: + manager.acquire("owner", "thread", user_id="user") + assert manager.binding_for("owner") is None + assert provider.release_calls == ["shared"] + finally: + manager.close() + + def test_non_releasing_holder_defers_parent_release_until_its_scope_is_clean() -> None: provider = _LeaseProvider() manager = SandboxLeaseManager(provider) @@ -456,6 +608,39 @@ async def test_cancelled_async_acquire_releases_unbound_provider_result() -> Non assert provider.release_calls == ["shared"] +@pytest.mark.anyio +async def test_cancelled_acquire_defers_rollback_until_a_fork_borrower_drains() -> None: + started = asyncio.Event() + finish = asyncio.Event() + + class BlockingProvider(_LeaseProvider): + async def acquire_async(self, thread_id=None, *, user_id=None): + started.set() + await finish.wait() + return self.sandbox.id + + provider = BlockingProvider() + manager = SandboxLeaseManager(provider) + task = asyncio.create_task(manager.acquire_async("cancelled", "parent-thread", user_id="user")) + try: + await started.wait() + manager.retain("fork", "shared", thread_id="fork-thread", user_id="user", release_on_last=False) + task.cancel() + finish.set() + with pytest.raises(asyncio.CancelledError): + await task + assert provider.release_calls == [] + await manager.release_async("fork") + assert provider.release_calls == ["shared"] + assert provider.sandbox.released_scopes == ["fork"] + finally: + finish.set() + if not task.done(): + task.cancel() + await asyncio.gather(task, return_exceptions=True) + manager.close() + + @pytest.mark.anyio async def test_repeated_cancellation_waits_for_provider_acquire_reconciliation() -> None: acquire_started = asyncio.Event() diff --git a/backend/tests/test_sandbox_middleware.py b/backend/tests/test_sandbox_middleware.py index fe8f8b7883..db3156f432 100644 --- a/backend/tests/test_sandbox_middleware.py +++ b/backend/tests/test_sandbox_middleware.py @@ -50,6 +50,10 @@ class _AgentSkillSyncProvider(_SyncProvider): def __init__(self) -> None: super().__init__() self.skill_syncs: list[tuple[str, str, str, object]] = [] + self.sandbox = _SandboxStub("sync-sandbox") + + def get(self, sandbox_id: str) -> Sandbox | None: + return self.sandbox if sandbox_id == self.sandbox.id else None def sync_agent_skills( self, diff --git a/backend/tests/test_sandbox_quarantine.py b/backend/tests/test_sandbox_quarantine.py new file mode 100644 index 0000000000..dfe9f087c5 --- /dev/null +++ b/backend/tests/test_sandbox_quarantine.py @@ -0,0 +1,92 @@ +"""Quarantine follows a container generation across provider restarts.""" + +import pytest + +from deerflow.community.aio_sandbox.quarantine import SandboxQuarantine +from deerflow.community.aio_sandbox.sandbox_info import SandboxInfo + + +def test_quarantine_survives_restart_and_does_not_taint_replacement(tmp_path): + old = SandboxInfo("thread-sandbox", "http://sandbox", container_id="old-generation") + replacement = SandboxInfo("thread-sandbox", "http://sandbox", container_id="new-generation") + SandboxQuarantine(tmp_path, "provisioner-a").mark(old) + + restarted = SandboxQuarantine(tmp_path, "provisioner-a") + assert restarted.contains(old) + assert not restarted.contains(replacement) + assert not SandboxQuarantine(tmp_path, "provisioner-b").contains(old) + + +def test_discovery_without_generation_cannot_bypass_quarantine(tmp_path): + store = SandboxQuarantine(tmp_path, "provisioner") + store.mark(SandboxInfo("thread-sandbox", "http://sandbox", container_id="old-generation")) + + assert store.contains(SandboxInfo("thread-sandbox", "http://sandbox")) + assert not store.contains(SandboxInfo("another-thread", "http://sandbox")) + + +def test_quarantine_storage_failure_is_not_treated_as_safe(tmp_path, monkeypatch): + store = SandboxQuarantine(tmp_path, "provisioner") + + def unavailable(*_args, **_kwargs): + raise PermissionError("quarantine storage unavailable") + + monkeypatch.setattr(type(tmp_path), "stat", unavailable) + with pytest.raises(PermissionError): + store.contains(SandboxInfo("thread-sandbox", "http://sandbox", container_id="generation")) + + +def test_crash_residue_between_directory_and_record_still_fences(tmp_path): + # mark() mkdirs the per-sandbox directory before writing the generation + # record; a process dying in between must not silently drop the fence. + store = SandboxQuarantine(tmp_path, "provisioner") + directory = tmp_path / store._key("provisioner") / store._key("thread-sandbox") + directory.mkdir(parents=True) + + assert store.contains(SandboxInfo("thread-sandbox", "http://sandbox")) + assert store.contains(SandboxInfo("thread-sandbox", "http://sandbox", container_id="crashed-generation")) + + +def test_retired_quarantine_allows_replacements_without_generation(tmp_path): + store = SandboxQuarantine(tmp_path, "provisioner") + old = SandboxInfo("thread-sandbox", "http://sandbox") + store.mark(old) + store.mark(SandboxInfo(old.sandbox_id, old.sandbox_url, container_id="old-generation")) + other = SandboxInfo("another-thread", "http://sandbox") + store.mark(other) + + store.retire(old) + + restarted = SandboxQuarantine(tmp_path, "provisioner") + assert not restarted.contains(old) + assert not restarted.contains(SandboxInfo(old.sandbox_id, old.sandbox_url, container_id="new-generation")) + assert restarted.contains(other) + + +def test_retire_replaced_generations_drops_only_absent_fences(tmp_path): + """A live replacement proves the fenced old generation is gone; its record + is pruned while the live generation's own fence survives.""" + store = SandboxQuarantine(tmp_path, "provisioner") + old = SandboxInfo("thread-sandbox", "http://sandbox", container_id="old-generation") + live = SandboxInfo("thread-sandbox", "http://sandbox", container_id="new-generation") + store.mark(old) + directory = tmp_path / store._key("provisioner") / store._key("thread-sandbox") + + store.retire_replaced_generations(live) + + assert not store.contains(old) + assert not directory.exists() # fully pruned, no stale record strands future creates + directory.mkdir() # a peer fences the live generation mid-flight + directory.joinpath(store._key("new-generation")).touch() + store.retire_replaced_generations(live) + assert store.contains(live) + + +def test_retire_replaced_generations_keeps_unversioned_crash_fence(tmp_path): + store = SandboxQuarantine(tmp_path, "provisioner") + store.mark(SandboxInfo("thread-sandbox", "http://sandbox")) # container_id-less mark → "unknown" + directory = tmp_path / store._key("provisioner") / store._key("thread-sandbox") + + store.retire_replaced_generations(SandboxInfo("thread-sandbox", "http://sandbox", container_id="new-generation")) + + assert (directory / "unknown").exists() diff --git a/backend/tests/test_uploads_router.py b/backend/tests/test_uploads_router.py index 11b723fcf7..5aab224ab9 100644 --- a/backend/tests/test_uploads_router.py +++ b/backend/tests/test_uploads_router.py @@ -965,10 +965,13 @@ def test_upload_files_does_not_sync_non_local_sandbox_when_total_size_exceeds_li assert exc_info.value.status_code == 413 provider.acquire.assert_not_called() provider.acquire_async.assert_awaited_once_with("thread-aio", user_id="owner-upload") - assert provider.get.call_count == 2 assert all(call.args == ("aio-1",) for call in provider.get.call_args_list) + sandbox.release_command_scope.assert_called_once() + assert get_sandbox_lease_manager(provider).binding_for(sandbox.release_command_scope.call_args.args[0]) is None provider.release.assert_called_once_with("aio-1") sandbox.update_file.assert_not_called() + assert not (thread_uploads_dir / "first.txt").exists() + assert not (thread_uploads_dir / "second.txt").exists() def test_upload_files_does_not_sync_non_local_sandbox_when_conversion_fails(tmp_path): @@ -996,8 +999,9 @@ def test_upload_files_does_not_sync_non_local_sandbox_when_conversion_fails(tmp_ assert exc_info.value.status_code == 500 provider.acquire.assert_not_called() provider.acquire_async.assert_awaited_once_with("thread-aio", user_id="owner-upload") - assert provider.get.call_count == 2 assert all(call.args == ("aio-1",) for call in provider.get.call_args_list) + sandbox.release_command_scope.assert_called_once() + assert get_sandbox_lease_manager(provider).binding_for(sandbox.release_command_scope.call_args.args[0]) is None provider.release.assert_called_once_with("aio-1") sandbox.update_file.assert_not_called() assert not (thread_uploads_dir / "report.pdf").exists() diff --git a/docker/provisioner/app.py b/docker/provisioner/app.py index 107b8e2723..aab7d21c88 100644 --- a/docker/provisioner/app.py +++ b/docker/provisioner/app.py @@ -12,6 +12,7 @@ Endpoints: POST /api/sandboxes — Create a sandbox Pod + Service DELETE /api/sandboxes/{sandbox_id} — Destroy a sandbox Pod + Service GET /api/sandboxes/{sandbox_id} — Get sandbox status & URL + GET /api/sandboxes/{sandbox_id}/presence — Inspect Pod presence & generation GET /api/sandboxes — List all sandboxes GET /health — Provisioner health check @@ -75,6 +76,14 @@ LARK_CLI_RUNTIME_VOLUME_NAME = "lark-cli-runtime" # broker off (Pattern A / legacy behavior). Broker supersedes Pattern A when both # are set. LARK_CLI_BROKER_IMAGE = os.environ.get("LARK_CLI_BROKER_IMAGE", "") +# Machine-readable marker attached to lark provisioning-config conflict +# responses (409/503) telling the Gateway to drop its cached broker +# capability observation for this endpoint. Keep in sync with +# deerflow.integrations.lark_cli.PROVISIONER_CAPABILITY_REFRESH_HEADER. +CAPABILITY_REFRESH_HEADER = "X-DeerFlow-Capability-Refresh" +# Marker value for lark broker capability changes. Keep in sync with +# deerflow.integrations.lark_cli.PROVISIONER_CAPABILITY_REFRESH_LARK_BROKER. +CAPABILITY_REFRESH_LARK_BROKER = "lark-broker" # Optional comma-separated lark-cli subcommand denylist forwarded to the broker # sidecar (issue #4338 hardening). Empty ⇒ no subcommand is blocked. See the # broker README's "subcommand denylist" section. @@ -282,6 +291,25 @@ def _lark_cli_broker_enabled(provision_lark_cli_broker: bool) -> bool: return bool(LARK_CLI_BROKER_IMAGE) and provision_lark_cli_broker +def _validate_lark_provisioning_request(runtime: bool, broker: bool) -> None: + # Both conflicts mean the Gateway's cached capability observation no longer + # matches this deployment's configuration; the marker tells it to re-probe + # on the next acquire instead of repeating the failure for the cache TTL. + conflict_headers = {CAPABILITY_REFRESH_HEADER: CAPABILITY_REFRESH_LARK_BROKER} + if broker and not LARK_CLI_BROKER_IMAGE: + raise HTTPException( + status_code=503, + detail="Requested Lark broker image is not configured", + headers=conflict_headers, + ) + if runtime and LARK_CLI_BROKER_IMAGE and not broker: + raise HTTPException( + status_code=409, + detail="Configured Lark broker is required; refresh provisioner capabilities", + headers=conflict_headers, + ) + + def _runtime_provided_extra_mounts( extra_mounts: list["ExtraMount"] | None, *, @@ -496,6 +524,14 @@ class SandboxResponse(BaseModel): sandbox_url: str status: str max_shell_sessions: int | None = None + container_id: str | None = None + lark_cli_broker: bool | None = None + + +class SandboxPresenceResponse(BaseModel): + sandbox_id: str + exists: bool + container_id: str | None = None # ── K8s resource helpers ───────────────────────────────────────────────── @@ -968,6 +1004,7 @@ def _build_pod( max_shell_sessions: int | None = None, ) -> k8s_client.V1Pod: """Construct a Pod manifest for a single sandbox.""" + _validate_lark_provisioning_request(provision_lark_cli_runtime, provision_lark_cli_broker) init_containers = ( _build_lark_cli_init_containers(provision_lark_cli_runtime, provision_lark_cli_broker) or None ) @@ -1139,18 +1176,7 @@ def _sandbox_access_url(sandbox_id: str, *, tolerate_read_errors: bool = False) return _url_from_service(svc, sandbox_id) -def _get_pod_phase(sandbox_id: str) -> str: - """Return the Pod phase (Pending / Running / Succeeded / Failed / Unknown).""" - try: - pod = core_v1.read_namespaced_pod(_pod_name(sandbox_id), K8S_NAMESPACE) - return pod.status.phase or "Unknown" - except ApiException: - return "NotFound" - - -def _get_pod_shell_capacity(sandbox_id: str) -> int: - """Return the effective AIO shell capacity persisted in the sandbox Pod.""" - pod = core_v1.read_namespaced_pod(_pod_name(sandbox_id), K8S_NAMESPACE) +def _pod_shell_capacity(pod) -> int: containers = getattr(getattr(pod, "spec", None), "containers", None) or [] sandbox_container = next((container for container in containers if getattr(container, "name", None) == "sandbox"), None) for env_var in getattr(sandbox_container, "env", None) or []: @@ -1166,6 +1192,58 @@ def _get_pod_shell_capacity(sandbox_id: str) -> int: return DEFAULT_MAX_SHELL_SESSIONS +def _sandbox_response( + sandbox_id: str, + sandbox_url: str, + *, + required_capacity: int | None = None, + required_broker: bool | None = None, +) -> SandboxResponse: + # Attest mode, generation and capacity from the same Pod observation, + # including idempotent create and the AlreadyExists race. + pod = core_v1.read_namespaced_pod(_pod_name(sandbox_id), K8S_NAMESPACE) + capacity = _pod_shell_capacity(pod) + containers = getattr(getattr(pod, "spec", None), "containers", None) or [] + init_containers = getattr(getattr(pod, "spec", None), "init_containers", None) or [] + sidecar = any(container.name == "lark-cli-broker" for container in containers) + shim = any(container.name == "lark-cli-shim-init" for container in init_containers) + sandbox = next((container for container in containers if container.name == "sandbox"), None) + credentials_mounted = any(mount.mount_path.startswith((LARK_CLI_CONFIG_CONTAINER_PATH, LARK_CLI_DATA_CONTAINER_PATH)) for mount in getattr(sandbox, "volume_mounts", None) or []) + broker = sidecar if sidecar == shim and not (sidecar and credentials_mounted) else None + if required_capacity is not None and capacity < required_capacity: + raise HTTPException( + status_code=409, + detail="Existing sandbox shell capacity is below the requested value; replacement must be coordinated by the Gateway", + ) + if required_broker is not None and broker is not required_broker: + # Post-build conflicts (idempotent re-create, attestation drift, + # post-create verification) reach the Gateway through the same 409 + # handling as the pre-build validation, so they must carry the + # capability-refresh marker too — otherwise a broker-mode deploy + # change lives out the Gateway's cache TTL instead of re-probing. + raise HTTPException( + status_code=409, + detail="Existing sandbox has an incompatible Lark broker mode; replacement must be coordinated by the Gateway", + headers={CAPABILITY_REFRESH_HEADER: CAPABILITY_REFRESH_LARK_BROKER}, + ) + metadata = getattr(pod, "metadata", None) + status = "Terminating" if getattr(metadata, "deletion_timestamp", None) else pod.status.phase or "Unknown" + # Only coordination requests (create with requirements) reject a + # Terminating Pod; status/get/list callers pass no requirements and read + # the phase as data — raising here would fail the whole fleet listing for + # one terminating Pod (HTTPException escapes their narrower catch clause). + if (required_capacity is not None or required_broker is not None) and status == "Terminating": + raise HTTPException(status_code=409, detail="Existing sandbox is still terminating") + return SandboxResponse( + sandbox_id=sandbox_id, + sandbox_url=sandbox_url, + status=status, + max_shell_sessions=capacity, + container_id=getattr(metadata, "uid", None), + lark_cli_broker=broker, + ) + + # ── API endpoints ──────────────────────────────────────────────────────── @@ -1201,12 +1279,12 @@ def create_sandbox(req: CreateSandboxRequest): thread_id = req.thread_id or sandbox_id user_id = req.user_id include_legacy_skills = req.include_legacy_skills - skills_container_path = _normalize_skills_container_path( - req.skills_container_path - ) + skills_container_path = _normalize_skills_container_path(req.skills_container_path) provision_lark_cli_runtime = req.provision_lark_cli_runtime provision_lark_cli_broker = req.provision_lark_cli_broker max_shell_sessions = req.max_shell_sessions + _validate_lark_provisioning_request(provision_lark_cli_runtime, provision_lark_cli_broker) + required_broker = provision_lark_cli_broker if provision_lark_cli_runtime or provision_lark_cli_broker else None logger.info( "Received request to create sandbox '%s' for thread '%s' user '%s' include_legacy_skills=%s skills_container_path=%s provision_lark_cli_runtime=%s provision_lark_cli_broker=%s max_shell_sessions=%s", @@ -1224,26 +1302,17 @@ def create_sandbox(req: CreateSandboxRequest): existing_url = _sandbox_access_url(sandbox_id, tolerate_read_errors=True) if existing_url: try: - existing_shell_capacity = _get_pod_shell_capacity(sandbox_id) + return _sandbox_response( + sandbox_id, + existing_url, + required_capacity=max_shell_sessions, + required_broker=required_broker, + ) except (ApiException, RuntimeError) as exc: # A Service can outlive its old Pod during asynchronous replacement. # Only a confirmed missing Pod may fall through to creation. if not isinstance(exc, ApiException) or exc.status != 404: raise HTTPException(status_code=500, detail=f"Could not verify existing sandbox shell capacity: {exc}") from exc - else: - if max_shell_sessions is not None and existing_shell_capacity < max_shell_sessions: - # Only the Gateway can fence replacement against active owners. - # A transient discovery failure can route a live Pod through create. - raise HTTPException( - status_code=409, - detail="Existing sandbox shell capacity is below the requested value; replacement must be coordinated by the Gateway", - ) - return SandboxResponse( - sandbox_id=sandbox_id, - sandbox_url=existing_url, - status=_get_pod_phase(sandbox_id), - max_shell_sessions=existing_shell_capacity, - ) # ── Create Pod ─────────────────────────────────────────────────── try: @@ -1293,18 +1362,14 @@ def create_sandbox(req: CreateSandboxRequest): # A concurrent creator can win the 409 race with a lower-capacity Pod. # Never claim that the requested value was applied without reading it back. try: - actual_shell_capacity = _get_pod_shell_capacity(sandbox_id) + return _sandbox_response( + sandbox_id, + sandbox_url, + required_capacity=max_shell_sessions, + required_broker=required_broker, + ) except (ApiException, RuntimeError) as exc: raise HTTPException(status_code=500, detail=f"Could not verify created sandbox shell capacity: {exc}") from exc - if max_shell_sessions is not None and actual_shell_capacity < max_shell_sessions: - raise HTTPException(status_code=409, detail="Existing sandbox shell capacity is below the requested value") - - return SandboxResponse( - sandbox_id=sandbox_id, - sandbox_url=sandbox_url, - status=_get_pod_phase(sandbox_id), - max_shell_sessions=actual_shell_capacity, - ) @app.delete("/api/sandboxes/{sandbox_id}") @@ -1334,6 +1399,22 @@ def destroy_sandbox(sandbox_id: str): return {"ok": True, "sandbox_id": sandbox_id} +@app.get("/api/sandboxes/{sandbox_id}/presence", response_model=SandboxPresenceResponse) +def sandbox_presence(sandbox_id: str): + """Attest Pod presence even while its Service is missing or unready.""" + try: + pod = core_v1.read_namespaced_pod(_pod_name(sandbox_id), K8S_NAMESPACE) + except ApiException as exc: + if exc.status != 404: + raise HTTPException(status_code=500, detail=f"Failed to check Pod presence: {exc.reason}") from exc + return SandboxPresenceResponse(sandbox_id=sandbox_id, exists=False) + return SandboxPresenceResponse( + sandbox_id=sandbox_id, + exists=True, + container_id=getattr(getattr(pod, "metadata", None), "uid", None), + ) + + @app.get("/api/sandboxes/{sandbox_id}", response_model=SandboxResponse) def get_sandbox(sandbox_id: str): """Return current status and URL for a sandbox.""" @@ -1341,12 +1422,7 @@ def get_sandbox(sandbox_id: str): if not sandbox_url: raise HTTPException(status_code=404, detail=f"Sandbox '{sandbox_id}' not found") - return SandboxResponse( - sandbox_id=sandbox_id, - sandbox_url=sandbox_url, - status=_get_pod_phase(sandbox_id), - max_shell_sessions=_get_pod_shell_capacity(sandbox_id), - ) + return _sandbox_response(sandbox_id, sandbox_url) @app.get("/api/sandboxes") @@ -1369,20 +1445,13 @@ def list_sandboxes(): if not sandbox_url: continue try: - shell_capacity = _get_pod_shell_capacity(sid) + response = _sandbox_response(sid, sandbox_url) except (ApiException, RuntimeError) as exc: if isinstance(exc, ApiException) and exc.status == 404: continue reason = getattr(exc, "reason", str(exc)) logger.warning("Skipping sandbox %s while inspecting shell capacity: %s", sid, reason) continue - sandboxes.append( - SandboxResponse( - sandbox_id=sid, - sandbox_url=sandbox_url, - status=_get_pod_phase(sid), - max_shell_sessions=shell_capacity, - ) - ) + sandboxes.append(response) return {"sandboxes": sandboxes, "count": len(sandboxes)}