kaxil opened a new pull request, #73529: URL: https://github.com/apache/airflow/pull/73529
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 t he 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](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#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](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed. * When adding dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x). * For significant user-facing changes create newsfragment: `{pr_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments). You can add this file in a follow-up commit after the PR is created so you know the PR number. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
