Add durable Strands sandbox support - #1768
Conversation
A timeout is the deterministic outcome the caller's own `timeout` argument asked for, so retrying just re-runs the same hanging command. Under Temporal's unlimited-attempt default this meant SandboxTimeoutError never reached workflow code and the agent could never observe the timeout and adapt. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every built-in Strands sandbox raises a plain FileNotFoundError from read/write/remove — only list_files raises the SandboxPathNotFoundError subclass — so the old handlers were dead code and a missing path retried forever instead of reaching workflow code. Catch the documented base class instead, and carry the sandbox's own message through so a timeout reports the duration it actually enforced rather than the one the caller requested. Also fix the DockerSandbox import in the README, which is not re-exported from strands.sandbox, and note that the sandbox cache is per worker process. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
|
||
| The factory is called lazily on first use. Its sandbox instance is cached and | ||
| shared by all activities for that name for the worker's lifetime, so tools see | ||
| the same filesystem and working state. Provisioning and teardown of the backing |
There was a problem hiding this comment.
Is sharing one sandbox across all Workflow Executions intentional? Two concurrent Workflows using TemporalSandbox("build") can access the same files and processes, creating cross-tenant data exposure and race risks.
There was a problem hiding this comment.
You could do TemporalSandbox(workflow_id) to get per-workflow isolation, but good point, that's probably what we want as the default, actually.
There was a problem hiding this comment.
I don't think TemporalSandbox(workflow_id) works here because StrandsPlugin(sandboxes=...) has to know those names at Worker construction to register the activities, and workflow_id is only known at runtime.
I think the sandbox cache needs a composite key of (name, workflow_id). The factory then has to create something every worker can reach using that id. Maybe a named container with workflow_id?
Summary
Testing