feat: integrate Sandbox with Agent

[gautamsirdeshmukh]

Description

Adds the Sandbox as a first-class parameter on Agent.

In Node, agents get a host-execution fallback automatically via a "node" export condition in package.json that loads index.node.ts. In browsers, accessing agent.sandbox without explicit configuration throws.

To make this Node/browser bundler compatible fallback split possible, we (also) introduce createDefaultSlot<T>() as a reusable primitive for environment-conditional defaults.

This is the third PR split off from strands-agents/sdk-typescript#1011, preceded by the base interface merged in strands-agents/sdk-typescript#1090 and Docker/SSH implementations merged in strands-agents/sdk-typescript#1110.

Developer Experience

Explicitly Configured Sandbox

const agent = new Agent({
  model: new BedrockModel(),
  sandbox: new DockerSandbox({ containerId: 'my-container' }),
})

agent.sandbox // DockerSandbox

Node: works without configuration (falls back to host-level execution)

const agent = new Agent({ model: new BedrockModel() })

agent.sandbox // NotASandboxLocalEnvironment

Browser: throws if not explicitly configured

...
agent.sandbox // throws DefaultNotConfiguredError

Related Issues

• feat: Sandbox parent PR (base/shell/remote/docker/tools/plugins) sdk-typescript#1011 (parent mega-PR)
• feat: base Sandbox interface sdk-typescript#1090 (base Sandbox / PosixShellSandbox interfaces)
• feat: add Sandbox implementations (Docker, SSH) sdk-typescript#1110 (Docker / SSH implementations)

Documentation PR

Not yet

Type of Change

New feature

Testing

How have you tested the change? Verify that the changes do not break functionality or introduce warnings in consuming repositories: agents-docs, agents-tools, agents-cli

• I ran all tests + added several

Checklist

• I have read the CONTRIBUTING document
• I have added any necessary tests that prove my fix is effective or my feature works
• I have updated the documentation accordingly
• I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
• My changes generate no new warnings
• Any dependent changes have been merged and published

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[github-actions]

Assessment: Comment

Clean integration of the Sandbox abstraction into the Agent. The Node/browser split via export conditions is well-architected and the test strategy (separate node/browser vitest projects) is solid.

Review Themes

• API surface: DefaultNotConfiguredError is thrown to consumers but not exported, limiting programmatic error handling in browser environments.
• Robustness: createDefaultSlot uses undefined as a sentinel, which is fragile for generic T types. A dedicated sentinel symbol would be safer.
• Semantic clarity: sandbox: false vs omission is documented as distinct but resolves identically in the implementation — a comment in the getter would help future maintainers understand this is intentional forward-compatibility.
• Testing: The reusable createDefaultSlot primitive lacks isolated unit tests despite being positioned as a general-purpose utility.

The overall design is well thought-out with good separation of the Node-specific registration from the platform-agnostic interface.

[github-actions]

Assessment: Approve

All previous review feedback has been addressed in the current code. The sentinel symbol, error export, @throws docs, and isolated tests are all in place.

Verification Summary

• Sentinel pattern: createDefaultSlot uses Symbol('empty') — correctly handles null, undefined, and 0 as valid T values (covered by default-slot.test.ts)
• Error export: DefaultNotConfiguredError lives in errors.ts and is exported from index.ts
• Interface docs: @throws on LocalAgent.sandbox surfaces the error to plugin/hook consumers
• Test coverage: Node tests, browser tests, integration tests (CJS + ESM), and isolated slot tests all present
• Security: Language parameter validated against LANGUAGE_PATTERN; cwd is shell-quoted; the class is explicitly labeled "no isolation" which is appropriate for the host fallback

Well-designed feature with clean separation between the platform-agnostic interface and Node-specific registration.

[gautamsirdeshmukh]

@agent-of-mkmeral feel free to have a look! "do the works" as your creator says

[agent-of-mkmeral]

Had a proper look @gautamsirdeshmukh — checked out the PR head, read the diff, and ran the suites locally. This is clean, well-architected work. ✅

What I verified locally (Node 22)

New tests for this PR — all green:

✓ src/__tests__/default-slot.test.ts                            (4 tests)
✓ src/sandbox/__tests__/default.test.node.ts                    (4 tests)
✓ src/sandbox/__tests__/not-a-sandbox-local-environment.test.node.ts (15 tests)
  23 passed | Type Errors: none

No regressions — full agent unit suite:

✓ src/agent/__tests__  →  14 files, 380 passed | Type Errors: none

Browser-bundler compatibility (the core goal of the PR) — check:browser-bundle passes:

esbuild src/index.ts --bundle --platform=browser --format=esm  →  412.3kb ⚡ Done in 63ms

Design review

• createDefaultSlot<T> sentinel pattern is the right call — Symbol('empty') correctly distinguishes "unset" from valid falsy values (0, null, undefined), and the test explicitly covers all three. 👍
• Node/browser split via the "node" export condition + index.node.ts side-effect registration is well done. The sideEffects allowlist in package.json is exactly what's needed to stop bundlers from tree-shaking the load-bearing defaultSandbox.set(...). The inline comment warning future maintainers not to mark it side-effect-free is a nice touch.
• sandbox: false vs omission resolving identically today is documented as intentional forward-compat in both the AgentConfig docstring and the test name. Fine as-is; your explanation in the thread covers the future divergence path.
• DefaultNotConfiguredError living in errors.ts and exported from index.ts, plus @throws on the LocalAgent interface — good for browser consumers doing instanceof checks.
• NotASandboxLocalEnvironment safety: language validated against LANGUAGE_PATTERN, cwd shell-quoted, code piped via base64+EOF heredoc. Correctly labeled "no isolation" for the host fallback. All 5 review threads resolved.

On the red CI ❌ (non-blocking)

The only failing check is Run integration tests (2 of 1188 failed). I traced both — neither is related to this PR:

1. test/integ/sandbox/docker.test.node.ts → "mounts host volumes when configured" — Docker volume-mount flakiness in CI, file not touched by this PR.
2. test/integ/agent.test.ts → "handles tool invocation" (~120s, retry x1) — long-running browser integ test timeout, file not touched by this PR.

Both are live-service/Docker integ tests, and the CI Gate check is green, so they're non-blocking. Separately, the type-check failure in test/integ/.../steering.test.node.ts also reproduces on main (it's not in this PR's changeset) — pre-existing, unrelated.

Verdict

LGTM. 412→ minimal, focused diff; solid test strategy (isolated slot tests + node/browser projects + CJS/ESM package tests); review feedback genuinely addressed. The red X is unrelated flaky integ tests, not a defect in this PR. 🚀

[agent-of-mkmeral]

Re @mkmeral's question on the agent.sandbox getter (agent.ts:329) — "is it the same instance or a new one each call?" and the shared-singleton "scary" concern:

I checked out the PR head and wrote a probe to answer it empirically rather than just reasoning about it. Same instance, not recreated per call:

const a = new Agent({ model })
a.sandbox === a.sandbox          // ✅ true  (same ref across reads)

const a1 = new Agent({ model })
const a2 = new Agent({ model })
a1.sandbox === a2.sandbox        // ✅ true  (ONE shared singleton across agents)

new Agent({ model, sandbox: false }).sandbox === a1.sandbox  // ✅ true

All 3 assertions pass. The getter return this._sandbox || defaultSandbox.get() just hands back the cached slot value — no allocation per read.

On the "scary" shared-singleton part — it's genuinely a non-issue today. I grepped NotASandboxLocalEnvironment and the base Sandbox class: zero instance fields. _resolvePath is a pure helper, and every operation keys off process.cwd() + the call args. So the shared singleton holds no per-agent state that could leak between agents.

@gautamsirdeshmukh's factory migration path is exactly right if/when a real default sandbox ever holds per-agent state: createDefaultSlot<() => Sandbox> + a per-agent _resolvedDefault ??= defaultSandbox.get()() cache gives one-instance-per-agent, non-breaking.

TL;DR: shared stateless singleton today (safe), clean non-breaking escape hatch reserved for the stateful future. No action needed on this PR — still LGTM. 🚀

[github-actions]

Assessment: Approve

All previous review feedback has been addressed in the current code. No new substantive issues found.

Verification Summary

• Sentinel pattern: createDefaultSlot uses Symbol('empty') — correctly handles null, undefined, and 0 as valid T values (covered by dedicated unit tests)
• Error export: DefaultNotConfiguredError lives in errors.ts and is exported from index.ts
• Interface docs: @throws on LocalAgent.sandbox surfaces the error to consumers; LocalAgent is @internal so adding the property is non-breaking
• Bundler setup: sideEffects array + "node" export condition + "types" ordering is correct
• Test coverage: Node tests, browser tests, CJS/ESM integration tests, and isolated slot tests — all solid
• Security: Language parameter validated against LANGUAGE_PATTERN; cwd is shell-quoted; class naming ("NotASandbox") makes the no-isolation semantics clear

Well-designed feature with clean separation between the platform-agnostic interface and Node-specific registration.

[agent-of-mkmeral]

Re-checked on the updated head de6085a8 (rebased/squashed onto latest main since my last look). Pulled it fresh and re-ran locally on Node 22 — still clean: ✅

✓ src/__tests__/default-slot.test.ts                                 (4)
✓ src/sandbox/__tests__/default.test.node.ts                         (4)
✓ src/sandbox/__tests__/not-a-sandbox-local-environment.test.node.ts (17)
  25 passed | Type Errors: none

esbuild src/index.ts --bundle --platform=browser --format=esm  →  412.5kb ⚡  (check:browser-bundle passes)

What's changed since June 4:

• CI is now fully green — the integ-test red X I traced earlier (Docker volume-mount + long browser integ flakes) cleared on the rebase. statusCheckRollup: SUCCESS.
• The not-a-sandbox suite picked up 2 more tests (15→17) — nice, more coverage on the host-fallback path.
• @mkmeral's instance-identity thread is resolved — the empirical answer (shared stateless singleton today, factory escape-hatch for the stateful future) settled it. All 6 threads now resolved.

Nothing new to fix on my end — the diff is the same focused 17-file change, design is sound, and it's MERGEABLE. Only thing outstanding is reviewDecision: REVIEW_REQUIRED, i.e. it needs a human maintainer's approving review to merge. From my side: LGTM, ship it. 🚀