test(sandbox): cover the modal and cloudflare bucket mount strategies

[abhay-codes07]

Summary

Completes coverage of the sandbox provider mount strategies, following #4240 which did e2b and runloop.

File	before	after
extensions/sandbox/modal/mounts.py	78%	100%
extensions/sandbox/cloudflare/mounts.py	85%	100%

Config building was already tested for both. The gap was the strategy lifecycle and the error branches, which normally need a live provider account to reach.

Cloudflare

• activate and deactivate drive mount_bucket and unmount_bucket with the resolved mount path and the built request options.
• The snapshot hooks unmount and then remount the same path, so a snapshot cannot capture a live bucket and the mount returns afterwards.
• All four lifecycle methods reject a session from another backend, asserted by checking that no bucket call was made rather than only that it raised.
• Prefix normalisation across None, empty, bare and wrapped forms, a half-supplied credential pair, and an unsupported mount type.

Modal

• supports_native_snapshot_detach is False, which is what routes snapshots through _native_snapshot_requires_tar_fallback instead of trusting a native snapshot taken while a bucket is attached. That link was previously unpinned, so it is worth having a test on it.
• activate and the snapshot hooks are no-ops, because Modal attaches buckets at sandbox create time, and activate and deactivate still reject a foreign session.
• Secret option validation, and the rule that inline credentials cannot be combined with a named secret, for s3, r2 and gcs.

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

Test plan

New tests/extensions/sandbox/test_modal_mounts.py, following the existing test_blaxel_mounts.py convention, plus additions to tests/extensions/sandbox/test_cloudflare.py. Cloudflare uses a recording session that captures mount_bucket and unmount_bucket calls on a single ordered timeline, so the tests assert an exact call sequence rather than checking two independent lists. Modal's strategy only inspects the session class name, so a bare stand-in is enough; the file stubs the optional modal SDK when it is absent, since the package __init__ imports the backend at module scope and would otherwise fail collection.

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/	773 passed, 1 skipped
make tests	6035 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: no new failures, with 35 more tests passing here.

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: 64f7b1f24a

ℹ️ 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 collecting Modal tests through the real extra

In a dev or packaged-test environment that does not install the optional modal extra, this top-level submodule import still executes agents.extensions.sandbox.modal.__init__, which imports .sandbox and then import modal, so pytest collection fails before these characterization tests can run. The existing Modal tests avoid that by installing a fake modal module before importing the backend; please load this strategy through that helper or otherwise skip/fake the optional dependency.

AGENTS.md reference: AGENTS.md:L78-L86

Useful? React with 👍 / 👎.

[seratch]

Thanks for sharing these changes. I don't have the bandwidth to check this type of changes right now, so holding off sending simple test changes just for coverage improvement would be appreciated.

[abhay-codes07]

Thanks, the Codex catch is right and is now fixed.

Optional modal extra at collection time. agents.extensions.sandbox.modal.__init__ imports .sandbox, which does import modal at module scope, so the top-level submodule import took the whole file down before any test could run. Confirmed by blocking the module with a sys.meta_path hook:

src\agents\extensions\sandbox\modal\sandbox.py:32: in <module>
    import modal
E   ImportError: blocked optional dependency: modal
ERROR tests/extensions/sandbox/test_modal_mounts.py
!!!!! Interrupted: 1 error during collection !!!!!

mounts.py itself imports nothing from the SDK, so the file now installs a minimal stub for modal, modal.config and modal.container_process only when the real package is absent, then imports the strategy. Under the same blocking hook it is now 17 passed, and unchanged at 17 passed with the extra installed.

I also tightened the bucket-call assertions. These were checking session.mounted and session.unmounted as two independent lists, which is the same shape of weak assertion @seratch flagged on #4240. Both are now recorded on one ordered session.events timeline, so the snapshot test asserts an exact sequence:

assert session.events == [f"unmount:{path}", f"mount:{path}"]

To be accurate about what that buys: for cloudflare the ordering was already implied by the test calling the two hooks in sequence, so this is not fixing a false pass the way it was on #4240. What it does add is that the exact call count and the absence of stray bucket calls are now pinned, and the foreign-session guard asserts session.events == [] rather than two empty lists.

Rebased on main. The diff is 380 insertions, 0 deletions.

Command	Result
make format / make lint	clean, all checks passed
make mypy	0 issues in the touched files
make pyright	0 errors
uv run pytest tests/extensions/sandbox/	773 passed, 1 skipped
make tests	6035 passed
coverage on modal/mounts.py and cloudflare/mounts.py	100% each

The full suite was run on Windows, where some sandbox symlink and tracing timing tests fail independently of this change. I diffed the failing set against clean main in the same environment: no new failures, with 35 more tests passing here.

[seratch]

@abhay-codes07 I should have mentioned you in the previous comment, but I don't have the bandwidth to review this type of PRs at this moment. Keeping this one is okay, but please hold off sending similar ones (I won't actively review those).

[abhay-codes07]

Understood, thanks for saying it plainly. I will stop sending coverage only test PRs. No rush on this one, pick it up whenever it suits you or leave it.