Skip to content

fix: preserve native Modal directory snapshots with disjoint mounts - #4567

Open
Filimoa wants to merge 1 commit into
openai:mainfrom
Filimoa:fix/modal-disjoint-snapshot-mounts
Open

fix: preserve native Modal directory snapshots with disjoint mounts#4567
Filimoa wants to merge 1 commit into
openai:mainfrom
Filimoa:fix/modal-disjoint-snapshot-mounts

Conversation

@Filimoa

@Filimoa Filimoa commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Summary

This pull request fixes Modal workspace_persistence="snapshot_directory" unnecessarily falling back to tar whenever any manifest mount cannot be detached, even when that mount is completely outside the directory being snapshotted.

For example, a workspace rooted at /workspace with a Modal cloud-bucket mount at /mnt/remote is a valid, disjoint topology. Modal's cloud-bucket strategy correctly reports that it cannot detach the mount, but the shared fallback check previously ignored the mount target and therefore downgraded Sandbox.snapshot_directory("/workspace") to tar. Because the mount cannot be included in that directory snapshot, its detach capability is irrelevant.

The fallback decision now accepts the resolved directory snapshot root and consults supports_native_snapshot_detach() only for mounts whose normalized POSIX targets overlap that root. Equality, descendants, and ancestors are treated as overlap. Filesystem-wide snapshot callers continue to inspect every mount.

The Modal adapter also preserves the no-copy boundary around mounted storage:

  • Detachable overlapping mounts are detached and restored around persistence.
  • Modal cloud-bucket mounts equal to, below, or containing the workspace root remain unsupported and are rejected before provider side effects, including trusted-manifest resume.
  • Disjoint cloud-bucket mounts remain manifest-owned ephemeral storage and are recreated when a fresh sandbox restores the native workspace image.
  • Mount contents, credentials, authority, native snapshot reference formats, restore behavior, and existing tar behavior are unchanged.

Test plan

  • Added generic fallback tests covering disjoint, equal, descendant, ancestor, normalized, double-slash, detachable, filesystem-wide, and Windows-flavored typed paths.
  • Added Modal provider-fake tests covering native directory-snapshot references for disjoint cloud mounts, overlapping rejection, detach/restore behavior, trusted resume validation, and fresh restore recreation.
  • Ran uv run pytest -q tests/sandbox/test_session_utils.py tests/extensions/sandbox/test_modal.py (155 passed).
  • Ran .agents/skills/code-change-verification/scripts/run.sh with the Codex sandbox environment and a writable temporary uv cache; formatting, linting, type checking, and the full test suite passed.
  • Completed the required independent final review with two clean reviewers on the final patch fingerprint.

Issue number

N/A

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant