Skip to content

Add OpenSandbox backend for sandbox tools - #71676

Open
zozo123 wants to merge 1 commit into
apache:mainfrom
zozo123:agent/add-opensandbox-sandbox-backend
Open

zozo123 wants to merge 1 commit into
apache:mainfrom
zozo123:agent/add-opensandbox-sandbox-backend

Conversation

@zozo123

@zozo123 zozo123 commented Aug 16, 2026 •

Copy link
Copy Markdown
Contributor

Adds a self-hosted OpenSandbox backend for SandboxToolset, so Airflow workers can use a remote Docker or Kubernetes sandbox service without access to the container runtime.

What changed

  • maps SandboxSpec environment and network policy at creation, preserving the fail-closed contract;
  • streams byte-bounded command output and uses OpenSandbox's native file APIs;
  • applies server-side command deadlines and a server-side sandbox lifetime;
  • resolves the optional SDK and a generic Airflow connection lazily; and
  • documents the sandbox-opensandbox extra and adds unit plus system coverage.

Validation

Verified end to end against a real OpenSandbox server (opensandbox/server:v0.2.3, Docker runtime, SDK 0.1.16) — recording in the comments. That run covers API-key enforcement, all four toolset tools driven by a pydantic-ai agent, this PR's system-test Dag task, deny-all egress with allowlist narrowing, line-delimited output, and teardown.

Running it for real surfaced two defects, both fixed here:

  • the system test could never construct — it asked for a 30s command ceiling while leaving default_command_timeout at 60s, which SandboxToolset rejects;
  • command output lost every line break — execd streams one message per output line with the delimiter stripped, and the backend concatenated them, so printf 'A\nB\nC\n' came back as 'ABC'. Output reached the model as a single run-on line, which also left the toolset's line-based truncation with nothing to cut on.

Also:

  • 39 unit tests pass against both opensandbox 0.1.15 and 0.1.16;
  • Ruff, formatting, codespell, YAML/Markdown, provider metadata, dependency and documentation hooks pass locally, as do mypy for providers and the datamodel codegen hooks.

Related: #68847


Was generative AI tooling used to co-author this PR?
  • Yes — Codex (GPT-5.6) and Claude Code (Opus 5)

Generated-by: Codex (GPT-5.6), Claude Code (Opus 5) following the guidelines

Comment thread providers/common/ai/docs/toolsets.rst Outdated
@github-actions

github-actions Bot commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

uv.lock on main just moved via #73708 ("Mark the IBM MQ and IBM Db2 providers as ready for release"), commit 122e24c and this PR currently conflicts.

Quickest fix:

git fetch upstream main && git rebase upstream/main
rm uv.lock && uv lock
git add uv.lock && git rebase --continue
git push --force-with-lease

Automated nudge — ignore if you're not ready to rebase. This comment is updated in place on future uv.lock bumps.

@zozo123
zozo123 force-pushed the agent/add-opensandbox-sandbox-backend branch from 37cade3 to ffd06d4 Compare September 14, 2026 09:12
@zozo123
zozo123 requested a review from ashb as a code owner September 14, 2026 09:12
@zozo123

zozo123 commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor Author

Yes — ran it end to end against a real OpenSandbox server (Docker runtime, opensandbox/server:v0.2.3, SDK 0.1.16, arm64).

opensandbox-airflow

Every sandbox in the clip is a real container created and destroyed through the server's API, and the egress results come from a stdlib socket probe inside the sandbox, so a failure can't be mistaken for a missing tool. It covers API-key enforcement (401 → 200), all four SandboxToolset tools driven by a real pydantic-ai agent, this PR's own system-test Dag task, the SandboxSpec egress matrix, and teardown.

Trying it out turned up two things this PR had wrong. Both are fixed in the push above:

The system test could never have run. It asked for a 30s command ceiling while leaving default_command_timeout at 60s, which SandboxToolset rejects at construction — it failed before reaching a sandbox. example_sandbox_toolset_sbx.py on main has the same bug; I left it alone to keep this PR scoped, happy to send it separately.

Command output lost every line break. execd streams one message per output line with the delimiter stripped, and the backend concatenated them, so printf 'A\nB\nC\n' came back as 'ABC'. Everything reached the model as a single run-on line — and since command output is truncated by whole lines, max_output_lines was inert for this backend. Fixed with a regression test, verified byte-for-byte against the live server including the blank-line case.

Also rebased onto main with uv.lock regenerated, and moved to opensandbox>=0.1.16. 39 unit tests pass on both 0.1.15 and 0.1.16.


Drafted-by: Claude Code (Opus 5); reviewed by @zozo123 before posting

@zozo123
zozo123 requested a review from kaxil September 14, 2026 09:31
@zozo123
zozo123 force-pushed the agent/add-opensandbox-sandbox-backend branch 2 times, most recently from c6316d3 to 7e9ad96 Compare September 14, 2026 10:55
@kaxil

kaxil commented Sep 14, 2026

Copy link
Copy Markdown
Member

Yes — ran it end to end against a real OpenSandbox server (Docker runtime, opensandbox/server:v0.2.3, SDK 0.1.16, arm64).

opensandbox-airflow Every sandbox in the clip is a real container created and destroyed through the server's API, and the egress results come from a stdlib socket probe _inside_ the sandbox, so a failure can't be mistaken for a missing tool. It covers API-key enforcement (401 → 200), all four `SandboxToolset` tools driven by a real pydantic-ai agent, this PR's own system-test Dag task, the `SandboxSpec` egress matrix, and teardown.

Trying it out turned up two things this PR had wrong. Both are fixed in the push above:

The system test could never have run. It asked for a 30s command ceiling while leaving default_command_timeout at 60s, which SandboxToolset rejects at construction — it failed before reaching a sandbox. example_sandbox_toolset_sbx.py on main has the same bug; I left it alone to keep this PR scoped, happy to send it separately.

Command output lost every line break. execd streams one message per output line with the delimiter stripped, and the backend concatenated them, so printf 'A\nB\nC\n' came back as 'ABC'. Everything reached the model as a single run-on line — and since command output is truncated by whole lines, max_output_lines was inert for this backend. Fixed with a regression test, verified byte-for-byte against the live server including the blank-line case.

Also rebased onto main with uv.lock regenerated, and moved to opensandbox>=0.1.16. 39 unit tests pass on both 0.1.15 and 0.1.16.

Drafted-by: Claude Code (Opus 5); reviewed by @zozo123 before posting

Could you post what the task logs in Airflow UI looks like too please

@zozo123

zozo123 commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

Here’s the Airflow UI view from a fresh run of this PR’s common_ai_sandbox_toolset_opensandbox system-test DAG.

The manual run finished successfully in 16.4s. The task log shows the python:3.12-slim sandbox being created, passing its health check, torn down, and returning sandbox boundary e2e passed. The test exercises write_file, command execution, read_file, non-zero exit/stderr handling, directory listing/state persistence, and teardown.

Airflow UI showing the OpenSandbox E2E DAG progressing from trigger to successful task logs

Comment thread providers/common/ai/src/airflow/providers/common/ai/sandbox/opensandbox.py Outdated
Comment thread providers/common/ai/src/airflow/providers/common/ai/sandbox/opensandbox.py Outdated
Comment thread providers/common/ai/src/airflow/providers/common/ai/sandbox/opensandbox.py Outdated
Comment thread providers/common/ai/src/airflow/providers/common/ai/sandbox/opensandbox.py Outdated
@zozo123
zozo123 force-pushed the agent/add-opensandbox-sandbox-backend branch 4 times, most recently from 84f0eb7 to 1bc54e5 Compare September 22, 2026 08:44
@zozo123

zozo123 commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

Pushed the round for the 2026-09-17 review; replies are inline on each thread.

Behaviour changes, all confined to the OpenSandbox backend: the network policy is read back after create and a sandbox the server did not restrict is destroyed; a stalled command stream is abandoned 30s past its budget with the sandbox destroyed and sandbox_terminated set; an exit status the SDK could not parse is no longer a terminal error; oversized reads report the file's real size; listings are capped with guidance; a connection without a host is refused instead of falling back to localhost:8080; reconnect time is not charged to the command; destroy skips the readiness poll; and every sandbox carries created-by: airflow metadata.

Two threads asked for evidence against a real server rather than fixtures. Both are now covered, against a local opensandbox-server on Docker:

case result
server with no egress sidecar, default deny-all spec server refuses the create -- 400 [SANDBOX::INVALID_PARAMETER] egress.image must be configured when networkPolicy is provided -- surfaced as SandboxTerminalError. Fails closed at create; the read-back stays as the backstop for a build that would accept it silently.
real timeout, 5s budget against sleep 60 elapsed=6.0s exit_code=-1 timed_out=True sandbox_terminated=False, stderr signal: killed
signal kill, sh -c 'kill -9 $$' on a 30s budget elapsed=1.0s exit_code=-1 timed_out=False sandbox_terminated=False, stderr signal: killed

The third is the one the exit_code thread was about: execd words error.value as the prose signal: killed, which _infer_foreground_exit_code cannot parse. It is now an ordinary failed command, and because it died well inside its budget it is not misreported as a timeout either.

The system test also passed end to end against that server (1 passed in 20.83s), exercising create, two commands, write, read, mkdir, list, the egress policy read-back and teardown, with no containers left behind.

Rebased onto current main, which reorganised the provider docs twice today, so the backend is documented in docs/sandbox/backends.rst beside Modal and sbx rather than in toolsets.rst. Also in this push: the SDK moves to opensandbox>=1.1.0, which I checked is API- and behaviour-identical for everything this backend touches, including the disabled SSE read timeout, _infer_foreground_exit_code and the unbounded frame buffer; it needs a temporary exclude-newer-package override until the rolling window passes 2026-09-21. Two packaging fixes came out of that review: the SDK had been listed under the llamaindex extra rather than [dependency-groups].dev, which both dragged it into [llamaindex] and made the unit tests importorskip away in the main lane, and the extras table in docs/index.rst had mismatched column rules with no sandbox-opensandbox row.

63 unit tests, ruff and the applicable prek hooks pass locally; mypy is clean on the changed files with the SDK installed.


Drafted-by: Claude Code (Opus 5); reviewed by @zozo123 before posting

@zozo123
zozo123 requested a review from kaxil September 22, 2026 09:27
@kaxil

kaxil commented Sep 22, 2026

Copy link
Copy Markdown
Member

I went back through each round-2 thread against the opensandbox 1.1.0 source rather than the fixtures, and the fixes hold: the dispatcher records error and complete regardless of skip_accumulation, so the exit-code path sees real data; a plain 404 on connect is not retried by the readiness budget, so destroy on a gone sandbox returns at once; get_egress_policy, the metadata kwarg and skip_health_check all exist with the shapes used here. Nothing blocking from me. Three non-blocking notes, here rather than as a review round:

  1. docs/sandbox/backends.rst L228, the "Command timeouts" bullet says a timeout destroys an OpenSandbox sandbox and its files. Your own measurement above (sleep 60 on a 5s budget gives sandbox_terminated=False) shows a server-enforced timeout leaves the sandbox and its files intact, and only a stalled stream destroys it. OpenSandbox belongs on the Modal side of that sentence, with the stall case as the exception.

  2. _get_network_policy sends NetworkPolicy(defaultAction="allow") for block_network=False. The server refuses any create that carries a networkPolicy when egress.image is not configured (ensure_egress_configured in server/opensandbox_server/services/validators.py returns early only on a falsy policy), and the Docker runtime refuses one under network_mode=host or a user-defined network as well (_ensure_network_policy_support in services/docker/networking.py). So a deployment without the sidecar cannot run an open-network spec even though there is nothing to enforce. Returning None for the open case would let those deployments work, and create already skips the read-back for it.

  3. The Modal backend's extra is modal; this one is sandbox-opensandbox. Extras are permanent public surface, so worth deciding now whether it should be opensandbox to match.

@kaxil kaxil left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving; please settle the extra name (note 3 in my comment above) before merge, since that one is permanent once released.

zozo123 added a commit to zozo123/airflow that referenced this pull request Sep 22, 2026
zozo123 added a commit to zozo123/airflow that referenced this pull request Sep 22, 2026
zozo123 added a commit to zozo123/airflow that referenced this pull request Sep 22, 2026
zozo123 added a commit to zozo123/airflow that referenced this pull request Sep 22, 2026
zozo123 added a commit to zozo123/airflow that referenced this pull request Sep 22, 2026
zozo123 added a commit to zozo123/airflow that referenced this pull request Sep 22, 2026
zozo123 added a commit to zozo123/airflow that referenced this pull request Sep 22, 2026
@zozo123
zozo123 force-pushed the agent/add-opensandbox-sandbox-backend branch from 8ca5ff7 to 14381a3 Compare September 22, 2026 13:14
@zozo123

zozo123 commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

All three notes from your pass earlier today are addressed; the extra name is settled.

1. Command-timeout docs. docs/sandbox/backends.rst now puts OpenSandbox on the Modal side of that sentence, with the stall case as the exception:

A timeout destroys an sbx sandbox and its files; Modal and a server-enforced OpenSandbox timeout preserve the sandbox and files. OpenSandbox destroys it only if the command event stream itself stalls past the client-side grace period.

2. Open-network specs. _get_network_policy returns None when block_network=False and no allowlist is set, so a deployment without the egress sidecar can run an open-network spec. allow_egress_to_cidrs is now refused up front, naming the dns+nft read-back gap as the reason rather than accepting a policy it cannot prove is enforced.

3. Extra name. Renamed to opensandbox, matching modal. Applied across providers/common/ai/pyproject.toml, docs/installation.rst, docs/index.rst, README.rst and the SDK-missing error message; no occurrence of the old spelling remains.

While re-checking that rename I found it had shortened the extras-table cell by eight characters without re-padding the row, so the second column started inside the first column's span. docutils rejects that with "Malformed table. Text in column margin" and parses zero rows, which drops the whole Optional dependencies table from the rendered docs and from README.rst rather than just misaligning one line. Fixed in d12cf0e.

64/64 unit tests pass.

Drafted-by: Claude Code (Claude Opus 5); reviewed by @zozo123 before posting

@zozo123

zozo123 commented Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

CI is green on 425079a — 295 passed, 0 failed — and the extra name you asked me to settle before
merge is now opensandbox, matching modal.

The five threads still open are the round-2 ones; each has a reply and the fixes are in the current
head: _CREATED_BY_METADATA applied on every create (L332), an empty connection host refused
(L243), _get_sandbox moved ahead of the elapsed-time clock (L363/L366),
_LIST_DIRECTORY_MAX_ENTRIES = 10_000 (L58/L507), and destroy reconnecting with
skip_health_check=True (L528). Happy to resolve them myself if you would rather — I left them for
you since they are your comments.

Anything else you want before this goes in?

Drafted-by: Claude Code (Claude Opus 5); reviewed by @zozo123 before posting

Rebase the reviewed OpenSandbox backend onto current Airflow main, preserving current dependency state and dropping the expired temporary SDK quarantine override.
@zozo123
zozo123 force-pushed the agent/add-opensandbox-sandbox-backend branch from 8952e87 to 36cb7e6 Compare September 26, 2026 08:13

This branch has not been deployed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants