Skip to content

feat(delegation): per-subagent terminal sandbox isolation for delegate_task - #83430

Open
DavidMetcalfe wants to merge 2 commits into
NousResearch:mainfrom
DavidMetcalfe:feat/4271-subagent-sandbox
Open

feat(delegation): per-subagent terminal sandbox isolation for delegate_task#83430
DavidMetcalfe wants to merge 2 commits into
NousResearch:mainfrom
DavidMetcalfe:feat/4271-subagent-sandbox

Conversation

@DavidMetcalfe

Copy link
Copy Markdown
Contributor

Summary

Adds an opt-in sandbox flag to delegate_task (top-level, and per-task inside a batch). Sandboxed subagents get their own fresh container — separate filesystem, isolated environment variables, and an independent working directory — instead of sharing the parent's terminal backend, so parallel workstreams can no longer clobber each other's files, env mutations, or installed packages. Implements feature request #4271.

Why

delegate_task(tasks=[...]) children share the parent's single container (one bash, one /workspace, one set of installed packages). Concurrent cds, env mutations, and writes to the same path collide across parallel workstreams (previously documented as a known limitation in configuration.md). The infrastructure for per-task isolation already exists — register_task_env_overrides() gives any task_id its own container key (used by RL/benchmark harnesses like TerminalBench2) — but it was never wired to delegation and is not model-facing.

Changes

  • tools/delegate_tool.py
    • New sandbox boolean param on delegate_task (top-level + per-task; per-task wins; default false).
    • _register_child_sandbox_overrides() registers env_type + the backend's configured image under the child's task_id, so _resolve_container_task_id keys the child to its own container; the parent-alias registration (register_container_alias) is skipped for sandboxed children.
    • Sandboxed children inherit a per-task image registered on the parent session (RL rollouts, ACP workspace sessions) before falling back to process-level config, so the fresh container mirrors the parent's actual environment.
    • Fails loudly on backends that cannot isolate (local/ssh/vercel_sandbox): tool_error at delegate_task entry before any child spawns, ValueError at spawn as a backstop. An explicit sandbox=True never silently degrades to the shared sandbox.
    • Overrides are cleared at child teardown (clear_task_env_overrides) — no registry leaks; the container itself is removed by the child's existing close()cleanup_vm path.
    • Per-task values are coerced with the shared truthy parser (is_truthy_value), matching the top-level flag, so a model-emitted "false" string behaves correctly.
  • run_agent.py — forwards sandbox through _dispatch_delegate_task.
  • website/docs/user-guide/configuration.md — documents per-subagent sandbox isolation and updates the known-limitations caveat.
  • tests/tools/test_delegate_sandbox_isolation.py — 21 new tests.

Behavior

Case Before After
delegate_task(goal=..., sandbox=True) child shares parent container child gets a fresh container using the same configured image
batch with per-task sandbox all children share parent container only flagged tasks are isolated; per-task wins over top-level
sandbox=True on local/ssh/vercel_sandbox backend clear error before any child spawns
default (no flag) child shares parent container unchanged

Validation

  • 21 new unit tests: schema exposure, per-backend override registration (incl. parent-image inheritance), isolation keying, spawn wiring (alias vs. overrides), entry validation (incl. string "false" coercion), teardown cleanup, dispatch forwarding.
  • 197 existing delegate/docker/terminal tests pass unchanged (218 total).
  • Not E2E-tested against a live Docker daemon in the authoring environment (none available); the underlying per-task container mechanism is covered by the existing docker session-isolation suite and was E2E-validated in fix(docker): per-session container isolation and session-scoped workspace mounts #82731.

Notes

Open question: per-subagent resource limits (issue concern #4 — per-child CPU/memory/disk) are intentionally deferred. The container knobs exist (container_cpu/container_memory/container_disk) but wiring them per child is a follow-up.

Open question: this deliberately uses a boolean sandbox flag rather than a backend="..." string, so it doesn't pre-empt the named-backend-pool syntax proposed in #32141. Per-backend selection can layer on top later.

Closes #4271

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/delegate Subagent delegation sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Aug 10, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

feat(delegation): per-subagent terminal sandbox isolation for delegate_task — no blocking issues found. A few minor observations:

  1. tools/delegate_tool.py::_SANDBOXABLE_BACKENDS includes singularity, and _register_child_sandbox_overrides registers {env_type}_imagesingularity_image. Confirm tools/terminal_tool.py actually honors a per-task singularity_image override; if it doesn't, sandbox=True on singularity would register an ineffective key — exactly the false-isolation failure this PR exists to prevent.

  2. Container lifecycle: _clear_child_sandbox only drops registry overrides; container removal depends on the child's close() → cleanup_vm path. A child killed by a hard interrupt, a parent crash, or a process kill leaves an orphan container with no reaper. Consider a registry stamp + cleanup sweep on next gateway start so stale sandbox containers don't accumulate.

  3. In the sandbox path, _seed_child_terminal_env records the parent's cwd for the child, but the child's fresh container starts at its image's default working directory — so the recorded cwd may not match the container's actual cwd. Minor inconsistency, same as the shared path, but worth a comment so future readers don't assume the record equals the container state.

  4. sandbox normalization happens at two sites (delegate_task top-level and _run_single_child per-task via is_truthy_value), and the schema exposes it in both the top-level and per-task positions — the double surface is fine, but the two normalization sites could drift; a single normalization helper would keep the semantics in one place.

…e_task (NousResearch#4271)

Add an opt-in sandbox flag to delegate_task: each sandboxed child gets its
own container via the existing per-task env-override mechanism
(register_task_env_overrides), so parallel workstreams no longer collide on
a shared filesystem/env/workspace. Non-sandboxed children keep the
documented shared-parent-container contract.

- delegate_task(sandbox=True) (top-level or per-task in a batch) registers
  env_type + image overrides for the child's task_id, which makes
  _resolve_container_task_id return the child's own container key instead of
  the parent's; the parent-alias registration is skipped for sandboxed
  children. A per-task image registered on the parent session (RL rollouts,
  ACP workspaces) is inherited so the fresh container mirrors the parent's
  actual environment.
- Fails loudly on backends that cannot isolate (local/ssh/vercel_sandbox):
  tool_error at delegate_task entry before any child spawns, ValueError at
  spawn as a backstop. An explicit sandbox=True must never silently degrade
  to the shared parent sandbox.
- Overrides are cleared at child teardown (clear_task_env_overrides) so the
  per-task registry cannot leak; the container itself is removed by the
  child's existing close() -> cleanup_vm path.
- Per-task sandbox values are coerced with the shared truthy parser
  (is_truthy_value), so model-emitted string 'false'/'true' behave like the
  top-level flag instead of bool('false') == True.
- Schema: sandbox (boolean, default false) on the top-level properties and
  per-task items; forwarded through _dispatch_delegate_task and the registry
  handler.
- Docs: per-subagent sandbox isolation section in configuration.md.

Tests: 21 new cases (schema, override registration per backend incl. parent
image inheritance, isolation keying, spawn wiring, entry validation incl.
string coercion, teardown cleanup, dispatch forwarding); 197 existing
delegate/docker/terminal tests pass unchanged.
…erage, cwd-workdir doc

Review feedback (Enough1122): fold the three is_truthy_value sandbox
sites into _normalize_sandbox(value, default); pin singularity image
override registration (test gap found by the review); document that the
seeded child cwd feeds the fresh container's workdir.
@DavidMetcalfe
DavidMetcalfe force-pushed the feat/4271-subagent-sandbox branch from 2e98eaf to 94e0c92 Compare August 17, 2026 03:39
@DavidMetcalfe

Copy link
Copy Markdown
Contributor Author

@Enough1122 — addressed all four points. Also rebased onto current main (it had moved ~15 commits since the PR branched on Aug 10; the PR is now mergeable). All citations below are at the rebased head 94e0c92a64.

1. Singularity override — confirmed honored, and now pinned by a test. Both container-creation sites resolve overrides.get("singularity_image") or config["singularity_image"] for env_type == "singularity" (tools/terminal_tool.py:2087 in ensure_task_env, :2612 in the env build), and singularity_image is in _ISOLATION_OVERRIDE_KEYS (:1336), so _resolve_container_task_id (:1354) treats it as a real isolation signal — not a dead registration. The probe found a genuine coverage gap: the suite had docker/modal/daytona registration tests but no singularity mirror. Added test_singularity_registers_singularity_image, and mutation-checked it: removing singularity from _SANDBOXABLE_BACKENDS (tools/delegate_tool.py:67) fails exactly that test (ValueError at the override-registration site), restored → passes.

2. Orphan containers — the existing reapers already cover the sandboxed child (all three predate this PR, which touches none of them):

  • Idle reaper _cleanup_inactive_envs (terminal_tool.py:1948, 300s default, worker at :2015) sweeps _active_environments by _last_activity and calls env.cleanup(). The child's container is keyed under its own task_id with an activity stamp at creation (:2080/:2130/:2765) refreshed on every use — so if the child is hard-interrupted while the parent process stays alive, its container goes idle and is reclaimed within the lifetime window. The normal path removes it eagerly: child.close()cleanup_vm, then the override is cleared (delegate_tool.py:3305-3309).
  • _atexit_cleanup (terminal_tool.py:2259) closes all remaining envs on graceful exit.
  • If the parent process itself dies (crash/kill), the Docker orphan reaper _maybe_reap_docker_orphans (:1106, issue Bug: Docker containers accumulate - /stop doesn't clean, multiple per session, async race #20561, once per process) reclaims stale hermes-tagged containers at next startup — which is also the documented reclaim path for persist-mode idle envs (:2210). The only residue after a hard interrupt is the in-memory registry override entry, which dies with the process and cannot accumulate across restarts.

3. cwd record vs container cwd — they can't diverge at creation: the record is the source of the workdir. The env build derives the container's working directory from the child's session-cwd record (cwd = overrides.get("cwd") or get_session_cwd(task_id) or config["cwd"], terminal_tool.py:2620, passed to docker run -w per the guard at :2638), and the record was seeded from the parent at spawn (delegate_tool.py:2421). Post-creation cds update the record in lockstep. The one divergence edge — a host path remapped to /workspace by _is_unusable_container_cwd (:1497, applied at :2638) — is pre-existing, logged, and identical on the shared path. Added a docstring sentence in _seed_child_terminal_env (delegate_tool.py:2410) making the record→workdir flow explicit so future readers don't assume otherwise.

4. Normalization drift — agreed, folded into one helper. There were three is_truthy_value sites (top-level delegate_tool.py:3626, batch validation :3727, per-task :3886/:3912); they now share _normalize_sandbox(value, default) (:2310), and the redundant if sandbox is not None guard at the top-level site is gone (is_truthy_value(None, default=False) already returns False). Semantics unchanged.

Verification: pytest tests/tools/test_delegate_sandbox_isolation.py — 22 passed (21 existing + the new singularity test); 166 passed across the delegate + docker session-isolation suites on the rebased head.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data tool/delegate Subagent delegation type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Per-subagent terminal backend isolation for parallel workstreams

3 participants