test(sandbox): cover the e2b and runloop cloud bucket mount strategies

[abhay-codes07]

Summary

e2b/mounts.py and runloop/mounts.py were the two least covered files in the sandbox providers. Both are now at 100%.

File	before	after
extensions/sandbox/e2b/mounts.py	62%	100%
extensions/sandbox/runloop/mounts.py	65%	100%

The gap was the strategy lifecycle. Only the session guards, the e2b FUSE happy path and rclone installation were exercised, so nothing pinned what activate, deactivate, teardown_for_snapshot or restore_after_snapshot actually do. These are also the hardest paths to exercise by hand, since reaching them normally needs a live provider account.

Behaviour now pinned, for both providers:

• The session guard runs before any command is issued, for all four lifecycle methods, so a foreign session cannot reach the sandbox.
• activate prepares FUSE and rclone before delegating, and hands the delegate the session-resolved pattern carrying --allow-other rather than the raw pattern.
• nfs mode skips FUSE preparation, still requires rclone, and reaches the delegate with the nfs pattern left untouched.
• restore_after_snapshot re-runs preparation, since a restored sandbox may be a fresh container.
• deactivate and teardown_for_snapshot issue no setup commands, so unmounting never tries to install tooling on the way out.
• Every failure branch of runloop's _ensure_fuse_support: missing /dev/fuse, missing kernel module, missing apt-get, a failed install, fusermount still absent after installing, and a failed chmod. Each asserts the error context so the diagnostics stay useful.

No source changes. Every assertion matches current behaviour, so this is a characterisation of what the code does today.

Ordering is asserted on a shared timeline

Recording delegate calls separately from exec calls would not have constrained their relative order. Both fakes record into one ordered session.events list, and the delegate hook appends to that same list, so delegation has to be the final entry.

I checked that this is load-bearing by mutating the source and confirming the tests go red:

mutation	result
runloop activate delegates before preparation	2 failed
runloop restore_after_snapshot delegates before preparation	1 failed
e2b activate + restore_after_snapshot delegate before preparation	3 failed

Test plan

Additions to tests/extensions/sandbox/test_runloop_mounts.py and tests/extensions/sandbox/test_e2b.py. Every pre-existing test in both files is retained, and the new cases reuse the existing fake session helpers, so the optional runloop_api_client dependency is not imported. I verified that by blocking the module with a sys.meta_path hook and confirming all added cases still pass. The delegate is stubbed via monkeypatch on InContainerMountStrategy so the tests assert sequencing without needing a real mount.

Command	Result
make format	clean
make lint	all checks passed
make mypy	5 errors, all pre-existing on main, none in the touched files
make pyright	0 errors on the touched files
uv run pytest tests/extensions/sandbox/	768 passed, 1 skipped
make tests	6034 passed

The full suite run was done on Windows, where a set of sandbox symlink and tracing timing tests fail independently of this change. I diffed the failing set against clean main in the same environment: identical, 54 either way, with 34 more tests passing on this branch.

Issue number

None. This is coverage work on the sandbox providers rather than a reported defect.

Checks

• I've added new tests, if relevant
• I've run .agents/skills/code-change-verification/scripts/run.sh
• I've confirmed all verification steps pass
• If using Codex, I've run /review before submitting this PR

The verification script is a bash script that shells out to make. I ran the underlying steps individually instead, with the results above.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cc9b8bf2b5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

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".

[chatgpt-codex-connector]

Avoid requiring Runloop's optional SDK for mount tests

Importing RunloopSandboxSessionState pulls in agents.extensions.sandbox.runloop.sandbox, which imports runloop_api_client.types at module import time. In environments that run these mount tests without installing the optional [runloop] extra, collection now fails before any test can run, even though the mount strategy under test does not need the provider SDK; keep the fake state as a lightweight object or otherwise avoid importing runloop.sandbox here.

Useful? React with 👍 / 👎.

[seratch]

Thanks for adding coverage for these provider mount lifecycles. The direction is good, but before merging, please make the lifecycle tests prove the ordering they claim. Currently session.exec_calls and delegate.calls are recorded separately, so moving delegation before FUSE/rclone preparation would still leave the tests green.

Please use a shared event timeline, or assert the completed preparation state from inside the delegate, for both E2B and Runloop activate and restore_after_snapshot. Please also assert that the NFS activation path still reaches the delegate with the NFS pattern.

[abhay-codes07]

Thanks both, and apologies: the review caught a real problem, but there was a worse one underneath it that I should flag first.

I deleted nine existing tests. tests/extensions/sandbox/test_runloop_mounts.py already existed on main, and I wrote the file rather than extending it. That silently removed test_runloop_package_re_exports_cloud_bucket_strategy, test_runloop_extension_re_exports_cloud_bucket_strategy, test_runloop_ensure_rclone_installs_verified_release and test_runloop_rclone_pattern_adds_fuse_access_args, along with five others I had happened to write near-equivalents for. The -137 in the original diffstat was the signal and I missed it. The rclone release-pin test in particular guards the pinned-checksum install path from #3911, so losing it would have been a genuine regression in coverage. Sorry for the noise.

The branch is now rebased on main and the diff is 408 insertions, 0 deletions across the two files. All nine original tests are back untouched, and my additions sit alongside them using the file's existing _FakeRunloopMountSession, _exec_ok and _exec_fail helpers and its test_runloop_* naming. I dropped my duplicates of tests that already existed.

On the ordering, @seratch: you were right, and the tests were passing for the wrong reason.

session.exec_calls and delegate.calls were separate lists, so nothing actually constrained their relative order. Both fakes now record into a single ordered session.events timeline, and the delegate hook appends to that same list, so delegation has to be the last entry:

assert session.events[-1] == "delegate:activate"
prepared = session.events[:-1]
assert any("/dev/fuse" in event for event in prepared)
assert any("rclone" in event for event in prepared)

I confirmed this is load-bearing by mutating the source rather than trusting it. Moving delegation ahead of preparation in activate and restore_after_snapshot:

mutation	before	after
runloop activate delegates first	passed	2 failed
runloop restore_after_snapshot delegates first	passed	1 failed
e2b activate + restore_after_snapshot delegate first	passed	3 failed

Under the old assertions all of those stayed green, which is exactly the gap you described.

NFS now asserts it reaches the delegate carrying the NFS pattern, rather than only asserting FUSE setup was skipped:

assert session.events[-1] == "delegate:activate"
assert [name for name, _ in delegate.calls] == ["activate"]
delegated_pattern = delegate.calls[0][1]
assert delegated_pattern.mode == "nfs"
assert "--allow-other" not in delegated_pattern.extra_args

The last line pins the other half of rclone_pattern_for_session, which returns the pattern untouched for non-fuse modes.

On the optional import, @chatgpt-codex-connector: correct, and restoring the original file resolves it. main's _FakeRunloopMountSession uses a types.SimpleNamespace state and never imports runloop.sandbox, so runloop_api_client is not pulled in. I verified by blocking the module through a sys.meta_path hook: all 16 of my added cases still pass. The only two that fail under that hook are the pre-existing re-export tests, which legitimately need the SDK for the lazy package attribute.

Verification, on the rebased branch:

Command	Result
ruff format / ruff check	clean, all checks passed
mypy .	5 errors, all pre-existing on main, none in the touched files
pyright on both files	0 errors
pytest tests/extensions/sandbox/	768 passed, 1 skipped
coverage on e2b/mounts.py and runloop/mounts.py	100% each

Happy to split the coverage additions out from the ordering rework if that is easier to review.

[chatgpt-codex-connector]

Note

To use Codex here, create an environment for this repo.

[abhay-codes07]

The requested changes are pushed. Saw your note on #4246 about bandwidth, so no rush here either. Happy to leave this until you have time.