From 2e4567b96d34dd2a4f519b05f15847c89127eca9 Mon Sep 17 00:00:00 2001 From: Justin Beckwith Date: Sun, 27 Sep 2026 09:54:02 -0700 Subject: [PATCH] fix(sandbox): preserve nested paths in workspace snapshots --- .../sandbox/session/base_sandbox_session.py | 8 ++- src/agents/sandbox/session/runtime_helpers.py | 9 ++- .../sandbox/session/snapshot_lifecycle.py | 11 +++- src/agents/sandbox/session/tar_workspace.py | 2 +- tests/extensions/sandbox/test_daytona.py | 4 +- tests/extensions/sandbox/test_e2b.py | 7 +-- tests/sandbox/test_runtime_helpers.py | 45 ++++++++++++++ tests/sandbox/test_snapshot.py | 60 ++++++++++++++++++- tests/sandbox/test_tar_workspace.py | 57 ++++++++++++++++-- 9 files changed, 180 insertions(+), 23 deletions(-) diff --git a/src/agents/sandbox/session/base_sandbox_session.py b/src/agents/sandbox/session/base_sandbox_session.py index ecb12c3291..1b7abb69cc 100644 --- a/src/agents/sandbox/session/base_sandbox_session.py +++ b/src/agents/sandbox/session/base_sandbox_session.py @@ -1387,10 +1387,14 @@ def _should_compute_snapshot_fingerprint_on_persist(self) -> bool: return True - async def _compute_and_cache_snapshot_fingerprint(self) -> dict[str, str]: + async def _compute_and_cache_snapshot_fingerprint( + self, *, version: str | None = None + ) -> dict[str, str]: """Compute the current workspace fingerprint in-container and atomically cache it.""" - return await snapshot_lifecycle.compute_and_cache_snapshot_fingerprint(self) + return await snapshot_lifecycle.compute_and_cache_snapshot_fingerprint( + self, version=version + ) async def _read_cached_snapshot_fingerprint(self) -> dict[str, str]: """Read the cached snapshot fingerprint record from the running sandbox.""" diff --git a/src/agents/sandbox/session/runtime_helpers.py b/src/agents/sandbox/session/runtime_helpers.py index 24fc89127b..46bbd5cbca 100644 --- a/src/agents/sandbox/session/runtime_helpers.py +++ b/src/agents/sandbox/session/runtime_helpers.py @@ -231,9 +231,14 @@ if [ "$escape_tar_patterns" -eq 1 ]; then rel=$(printf '%s\\n' "$rel" | sed 's/[][\\\\*?]/\\\\&/g') fi - quoted_rel=$(quote_sh "$rel") + # Reproduce released fingerprints when comparing a preserved live workspace. + # Switching algorithms before comparison could restore an incomplete old archive. + if [ "$version" = "workspace_tar_sha256_v1" ]; then + quoted_rel=$(quote_sh "$rel") + tar_cmd="$tar_cmd --exclude=$quoted_rel" + fi quoted_dot_rel=$(quote_sh "./$rel") - tar_cmd="$tar_cmd --exclude=$quoted_rel --exclude=$quoted_dot_rel" + tar_cmd="$tar_cmd --exclude=$quoted_dot_rel" done tar_cmd="$tar_cmd -C $(quote_sh "$workspace_root") -cf - ." diff --git a/src/agents/sandbox/session/snapshot_lifecycle.py b/src/agents/sandbox/session/snapshot_lifecycle.py index 3c8db9f8ef..afde426a48 100644 --- a/src/agents/sandbox/session/snapshot_lifecycle.py +++ b/src/agents/sandbox/session/snapshot_lifecycle.py @@ -15,7 +15,7 @@ if TYPE_CHECKING: from .base_sandbox_session import BaseSandboxSession -SNAPSHOT_FINGERPRINT_VERSION = "workspace_tar_sha256_v1" +SNAPSHOT_FINGERPRINT_VERSION = "workspace_tar_sha256_v2" async def persist_snapshot(session: BaseSandboxSession) -> None: @@ -64,7 +64,10 @@ async def live_workspace_matches_snapshot_on_resume(session: BaseSandboxSession) return False try: - cached_record = await session._compute_and_cache_snapshot_fingerprint() + # Compare old records with their original exclusions before deciding to clear + # a live workspace. The next persist upgrades both the archive and fingerprint. + version = stored_version if stored_version == "workspace_tar_sha256_v1" else None + cached_record = await session._compute_and_cache_snapshot_fingerprint(version=version) except Exception: return False @@ -101,12 +104,14 @@ def workspace_fingerprint_skip_relpaths(session: BaseSandboxSession) -> set[Path async def compute_and_cache_snapshot_fingerprint( session: BaseSandboxSession, + *, + version: str | None = None, ) -> dict[str, str]: helper_path = await session._ensure_runtime_helper_installed(WORKSPACE_FINGERPRINT_HELPER) command = [ str(helper_path), session._workspace_root_path().as_posix(), - session._snapshot_fingerprint_version(), + version if version is not None else session._snapshot_fingerprint_version(), session._snapshot_fingerprint_cache_path().as_posix(), session._resume_manifest_digest(), ] diff --git a/src/agents/sandbox/session/tar_workspace.py b/src/agents/sandbox/session/tar_workspace.py index 32229c59f7..6c94c38950 100644 --- a/src/agents/sandbox/session/tar_workspace.py +++ b/src/agents/sandbox/session/tar_workspace.py @@ -13,6 +13,6 @@ def shell_tar_exclude_args(skip_relpaths: Iterable[Path]) -> list[str]: rel_posix = rel.as_posix().lstrip("/") if not rel_posix or rel_posix in {".", "/"}: continue - excludes.append(f"--exclude={shlex.quote(rel_posix)}") + # Archives are rooted at "."; bare patterns also match unrelated nested paths. excludes.append(f"--exclude={shlex.quote(f'./{rel_posix}')}") return excludes diff --git a/tests/extensions/sandbox/test_daytona.py b/tests/extensions/sandbox/test_daytona.py index 5fa75dd900..0b20700095 100644 --- a/tests/extensions/sandbox/test_daytona.py +++ b/tests/extensions/sandbox/test_daytona.py @@ -1195,11 +1195,9 @@ async def test_persist_workspace_uses_nested_mount_targets_and_runtime_skip_path f"{daytona_module.DEFAULT_DAYTONA_WORKSPACE_ROOT}/repo/sub", } tar_command = sandbox.process.exec_calls[0][0] - assert "--exclude=repo" in tar_command assert "--exclude=./repo" in tar_command - assert "--exclude=repo/sub" in tar_command assert "--exclude=./repo/sub" in tar_command - assert "--exclude=runtime.tmp" in tar_command + assert "--exclude=./runtime.tmp" in tar_command @pytest.mark.asyncio async def test_persist_workspace_remounts_prior_mounts_after_unmount_failure( diff --git a/tests/extensions/sandbox/test_e2b.py b/tests/extensions/sandbox/test_e2b.py index d3680a8f99..877b017cf7 100644 --- a/tests/extensions/sandbox/test_e2b.py +++ b/tests/extensions/sandbox/test_e2b.py @@ -1628,10 +1628,7 @@ async def test_e2b_persist_workspace_excludes_runtime_skip_paths() -> None: archive = await session.persist_workspace() assert archive.read() == b"fake-tar-bytes" - expected_command = ( - "tar --exclude=logs/events.jsonl --exclude=./logs/events.jsonl " - "-C /workspace -cf - . | base64 -w0" - ) + expected_command = "tar --exclude=./logs/events.jsonl -C /workspace -cf - . | base64 -w0" assert sandbox.commands.calls == [ { "command": expected_command, @@ -1826,9 +1823,7 @@ async def test_e2b_persist_workspace_uses_nested_mount_targets_and_resolved_excl "/workspace/repo/sub", ] tar_command = str(sandbox.commands.calls[-1]["command"]) - assert "--exclude=repo" in tar_command assert "--exclude=./repo" in tar_command - assert "--exclude=repo/sub" in tar_command assert "--exclude=./repo/sub" in tar_command diff --git a/tests/sandbox/test_runtime_helpers.py b/tests/sandbox/test_runtime_helpers.py index 97494d60bd..5d2b5bc6ca 100644 --- a/tests/sandbox/test_runtime_helpers.py +++ b/tests/sandbox/test_runtime_helpers.py @@ -81,6 +81,51 @@ def fingerprint() -> str: assert fingerprint() != first +@pytest.mark.skipif( + sys.platform != "linux", + reason="cloud workspace fingerprints use Linux tar; BSD tar has different exclusion semantics", +) +def test_workspace_fingerprint_detects_nested_paths_with_excluded_name(tmp_path: Path) -> None: + helper_path = _install_fingerprint_helper(tmp_path) + workspace = tmp_path / "workspace" + excluded = workspace / "data/scratch.txt" + durable = workspace / "app/data/users.csv" + excluded.parent.mkdir(parents=True) + durable.parent.mkdir(parents=True) + excluded.write_text("scratch-one", encoding="utf-8") + durable.write_text("id,name", encoding="utf-8") + ordinary = workspace / "app/main.py" + ordinary.write_text("print(1)", encoding="utf-8") + + def fingerprint() -> str: + result = subprocess.run( + [ + str(helper_path), + str(workspace), + "test-version", + str(tmp_path / "fingerprint.json"), + "manifest-digest", + "data", + ], + check=True, + capture_output=True, + text=True, + ) + return str(json.loads(result.stdout)["fingerprint"]) + + first = fingerprint() + assert fingerprint() == first + excluded.write_text("scratch-two", encoding="utf-8") + assert fingerprint() == first + + durable.write_text("ID,NAME", encoding="utf-8") + nested_changed = fingerprint() + assert nested_changed != first + + ordinary.write_text("print(2)", encoding="utf-8") + assert fingerprint() != nested_changed + + @requires_posix_shell def test_resolve_workspace_path_helper_allows_extra_root_symlink_target(tmp_path: Path) -> None: helper_path = _install_resolve_helper(tmp_path) diff --git a/tests/sandbox/test_snapshot.py b/tests/sandbox/test_snapshot.py index 1dd8635fd8..4a107caafa 100644 --- a/tests/sandbox/test_snapshot.py +++ b/tests/sandbox/test_snapshot.py @@ -1,7 +1,9 @@ from __future__ import annotations import asyncio +import hashlib import io +import subprocess from pathlib import Path from typing import Literal @@ -9,7 +11,7 @@ from pydantic import PrivateAttr, ValidationError from agents.sandbox import Manifest, RemoteSnapshot, RemoteSnapshotSpec, resolve_snapshot -from agents.sandbox.entries import File +from agents.sandbox.entries import Dir, File from agents.sandbox.errors import SnapshotPersistError from agents.sandbox.materialization import MaterializationResult from agents.sandbox.sandboxes.unix_local import UnixLocalSandboxSessionState @@ -448,7 +450,7 @@ async def test_non_noop_snapshot_stop_records_snapshot_fingerprint(tmp_path: Pat await session.stop() assert session.state.snapshot_fingerprint is not None - assert session.state.snapshot_fingerprint_version == "workspace_tar_sha256_v1" + assert session.state.snapshot_fingerprint_version == "workspace_tar_sha256_v2" cache_payload = session._parse_snapshot_fingerprint_record( session._snapshot_fingerprint_cache_path().read_text() ) @@ -473,6 +475,60 @@ async def test_start_skips_snapshot_restore_when_live_workspace_fingerprint_matc assert session.apply_manifest_calls == [True] +@pytest.mark.asyncio +@pytest.mark.parametrize("workspace_drifted", [False, True]) +async def test_start_compares_legacy_fingerprint_before_restoring_workspace( + tmp_path: Path, workspace_drifted: bool +) -> None: + workspace = tmp_path / "workspace" + (workspace / "data").mkdir(parents=True) + (workspace / "app/data").mkdir(parents=True) + durable = workspace / "app/data/users.csv" + durable.write_bytes(b"id,name") + ordinary = workspace / "app/main.py" + ordinary.write_bytes(b"print(1)") + (workspace / "data/scratch.txt").write_bytes(b"scratch") + # Capture the released archive and fingerprint independently of the current helper. + legacy_archive = subprocess.run( + [ + "tar", + "--exclude=data", + "--exclude=./data", + "-C", + str(workspace), + "-cf", + "-", + ".", + ], + check=True, + capture_output=True, + ).stdout + session = _ResumeTrackingSession( + workspace_root=workspace, + snapshot=TestRestorableSnapshot(id="legacy", payload=legacy_archive), + ) + session.state.manifest = Manifest( + root=workspace.as_posix(), entries={"data": Dir(ephemeral=True)} + ) + archive_hash = hashlib.sha256(legacy_archive).hexdigest() + legacy_hash = hashlib.sha256( + f"{archive_hash}\n{session._resume_manifest_digest()}\n".encode() + ).hexdigest() + session.state.snapshot_fingerprint = legacy_hash + session.state.snapshot_fingerprint_version = "workspace_tar_sha256_v1" + if workspace_drifted: + ordinary.write_bytes(b"print(2)") + + await session.start() + + assert session.clear_calls == int(workspace_drifted) + assert session.hydrate_payloads == ([legacy_archive] if workspace_drifted else []) + if not workspace_drifted: + assert durable.read_bytes() == b"id,name" + await session.stop() + assert session.state.snapshot_fingerprint_version == "workspace_tar_sha256_v2" + + @pytest.mark.asyncio async def test_start_closes_restored_workspace_archive(tmp_path: Path) -> None: snapshot = TestClosingRestoreSnapshot(id="resume-snapshot") diff --git a/tests/sandbox/test_tar_workspace.py b/tests/sandbox/test_tar_workspace.py index a2671f3257..c104b693c7 100644 --- a/tests/sandbox/test_tar_workspace.py +++ b/tests/sandbox/test_tar_workspace.py @@ -1,5 +1,14 @@ +import shlex +import shutil +import subprocess +import sys +import tarfile from pathlib import Path +import pytest + +from agents.sandbox import Manifest +from agents.sandbox.entries import Dir from agents.sandbox.session.tar_workspace import shell_tar_exclude_args @@ -7,22 +16,62 @@ def test_shell_tar_exclude_args_skips_empty_and_dot_paths() -> None: assert shell_tar_exclude_args([Path(""), Path("."), Path("/")]) == [] -def test_shell_tar_exclude_args_sorts_and_adds_plain_and_dot_prefixed_patterns() -> None: +def test_shell_tar_exclude_args_sorts_and_roots_patterns() -> None: assert shell_tar_exclude_args( [ Path("logs/events.jsonl"), Path("cache dir/file.txt"), ] ) == [ - "--exclude='cache dir/file.txt'", "--exclude='./cache dir/file.txt'", - "--exclude=logs/events.jsonl", "--exclude=./logs/events.jsonl", ] def test_shell_tar_exclude_args_normalizes_absolute_paths() -> None: assert shell_tar_exclude_args([Path("/tmp/workspace/cache")]) == [ - "--exclude=tmp/workspace/cache", "--exclude=./tmp/workspace/cache", ] + + +@pytest.mark.skipif( + sys.platform != "linux" or shutil.which("tar") is None, + reason="cloud workspace archives use Linux tar; BSD tar has different exclusion semantics", +) +@pytest.mark.parametrize("skip_path", ["data", "cache/data", "cache dir"]) +def test_workspace_archive_preserves_nested_paths_with_excluded_name( + tmp_path: Path, skip_path: str +) -> None: + workspace = tmp_path / "workspace" + excluded = workspace / skip_path / "scratch.txt" + durable = workspace / "app" / skip_path / "users.csv" + excluded.parent.mkdir(parents=True) + durable.parent.mkdir(parents=True) + excluded.write_text("scratch", encoding="utf-8") + durable.write_text("id,name", encoding="utf-8") + (workspace / "app/main.py").write_text("print(1)", encoding="utf-8") + + manifest = Manifest(root=workspace.as_posix(), entries={skip_path: Dir(ephemeral=True)}) + excludes = " ".join(shell_tar_exclude_args(manifest.ephemeral_persistence_paths())) + archive_path = tmp_path / "workspace.tar" + subprocess.run( + [ + "sh", + "-c", + f"tar {excludes} -C {shlex.quote(workspace.as_posix())} " + f"-cf {shlex.quote(archive_path.as_posix())} .", + ], + check=True, + capture_output=True, + ) + + with tarfile.open(archive_path) as archive: + assert f"./{skip_path}/scratch.txt" not in archive.getnames() + for path, expected in [ + (f"./app/{skip_path}/users.csv", b"id,name"), + ("./app/main.py", b"print(1)"), + ]: + restored = archive.extractfile(path) + assert restored is not None + with restored: + assert restored.read() == expected