Skip to content

Reject durable replay and HITL review with a SandboxToolset, and fail the task when a sandbox cannot be provisioned - #73529

Merged
kaxil merged 1 commit into
apache:mainfrom
astronomer:sandbox-guards
Sep 22, 2026
Merged

kaxil merged 1 commit into
apache:mainfrom
astronomer:sandbox-guards

Conversation

@kaxil

@kaxil kaxil commented Sep 22, 2026

Copy link
Copy Markdown
Member

A SandboxToolset provisions its sandbox on the model's first tool call and destroys it when the agent run ends, so nothing in it survives into a task retry or a second run. Two AgentOperator features assume it does, and until now each of them produced a wrong answer rather than an error:

  • durable=True replays cached tool results on a retry without calling the backend. A replayed write_file reported success while no sandbox existed, and the first call that missed the cache ran against a fresh, empty one. The model was handed a filesystem that did not match what it had just been told, and nothing raised.
  • enable_hitl_review=True regenerates after reviewer feedback by starting a second agent run. That run got an empty sandbox while its message history still described the files the first run had written.

The docs called both combinations unsupported; this makes the operator refuse them at construction, the same way it already refuses durable with code_mode and with enable_hitl_review. The error names the two ways out: drop the flag, or move the sandbox work into its own task.

The check looks inside compositions, not only at the top level of toolsets=. .prefixed(), .filtered() and .prepared() each wrap the original toolset, several toolsets passed together become a CombinedToolset, and tools can also reach the agent through a Toolset capability in agent_params. Running two sandboxes on one agent, which the docs recommend, goes through .prefixed(), so a shallow isinstance would have missed exactly the documented shape. The walker is a small helper in utils/toolsets.py with its own tests.

A failed provisioning now fails the task on purpose. call_tool awaited _ensure_sandbox() outside the try that maps a recoverable SandboxError to a ModelRetry, so a backend raising the recoverable class from create() had it propagate untouched and fail the task by accident, while the contract said the model could work around it. The alternative was to move the await inside the try and let the model retry provisioning, but the model has no input into create(): it takes only the spec, which is fixed in the Dag file, so no retry the model makes can turn a bad image tag or a rejected credential into a working sandbox, and letting it try would spend its retry budget on a fact it cannot see. The toolset now re-raises a recoverable create error as SandboxTerminalError with the cause chained, so Airflow's own task retry attempts the provisioning again, and the create() docstring states the rule. The Modal backend already behaved this way deliberately; sbx raised the terminal class from most create paths, so the gap was latent there.

Also adds the test a reviewer asked for on #72910: the Modal liveness probe keeps the handle when the probe itself fails with something other than a Modal error, since that says nothing about the sandbox and the command it followed had already produced its output.

Behaviour change. A Dag that combined durable=True or enable_hitl_review=True with a SandboxToolset used to parse and run, wrongly; it now fails at parse time with the message above. A toolset resolved per run from a callable, such as a Toolset capability holding a factory, cannot be inspected when the operator is built, so it is the one composition the check does not see. The docs say so.

Two imports move from function bodies to the top of operators/agent.py. pydantic-ai is already imported at parse time through the hook, and the sandbox toolset does not import the operator, so there is no cycle. The Modal backend already wrapped a recoverable create error into the terminal class itself, and sbx raises only the terminal class from create(), so neither backend changes behaviour here; the fix is for the contract and for backends written against it.


  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

…olset, and fail the task when a sandbox cannot be provisioned

A sandbox is destroyed when the agent run ends, so two operator features that
assume otherwise produced wrong answers instead of errors. durable=True replayed
cached tool results such as 'wrote the file' against a sandbox that no longer
existed, then ran the first cache miss against a fresh empty one.
enable_hitl_review=True regenerated after feedback in a second run whose
message history described files the first run's sandbox had held. AgentOperator
now refuses both at construction, looking inside prefixed, filtered and combined
toolsets and inside Toolset capabilities, and names the two ways out.

A backend raising a recoverable SandboxError from create() used to escape the
toolset's error mapping and fail the task by accident. It now fails it on
purpose: the model has no input into provisioning, so nothing it retries can
help, and Airflow's own retry is the right one. The create() contract says so.

Also covers the liveness probe branch where a non-Modal error keeps the handle.
@kaxil
kaxil marked this pull request as ready for review September 22, 2026 10:34
@kaxil
kaxil merged commit 018169c into apache:main Sep 22, 2026
84 checks passed
@kaxil
kaxil deleted the sandbox-guards branch September 22, 2026 10:35
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