From 5de8b3210e8aafda5b50bd864948bff6c0e94ee6 Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 20 Aug 2026 14:54:35 +0100 Subject: [PATCH] fix: daytona mini race --- application/sandbox/daytona.py | 27 +++++++++++- tests/sandbox/test_daytona.py | 62 +++++++++++++++++++++++++++ tests/sandbox/test_sandbox_manager.py | 26 ++++++++++- 3 files changed, 113 insertions(+), 2 deletions(-) diff --git a/application/sandbox/daytona.py b/application/sandbox/daytona.py index d9b5f8f3..b35d0353 100644 --- a/application/sandbox/daytona.py +++ b/application/sandbox/daytona.py @@ -134,6 +134,14 @@ class DaytonaSandbox(CodeSandbox): Reattach order: an in-memory handle, then (across process restarts) a live cloud sandbox carrying the session label, and only then a fresh ``create``. Enforces ``max_sandboxes`` so a flood of sessions cannot run up unbounded paid resources. + + Returns: + str: The live sandbox id, already registered for this session. + + Raises: + SandboxGoneError: The sandbox was confirmed gone before its workspace + could be primed. Nothing is cached, so a retry cold-starts. + RuntimeError: ``max_sandboxes`` live sandboxes already exist. """ self._validate_session_id(session_id) # Wait out any in-flight create/reattach for this session, then reuse its @@ -183,7 +191,24 @@ class DaytonaSandbox(CodeSandbox): except Exception as del_exc: # noqa: BLE001 - cleanup is best-effort logger.warning("Failed to delete orphaned Daytona sandbox during open: %s", del_exc) raise - self._prime(handle) + if not self._prime(handle): + # ``_prime`` only reports False once ``_sandbox_gone`` confirms it. + # Caching a dead handle would be worse than failing here: ``open`` + # returns cached handles without revalidating them, so every later + # open for this session would replay the same dead id. Forgetting + # it does not strand a paid resource even if the classification was + # a false positive -- the sandbox carries the session label, so + # ``_reattach_existing`` picks it up on the next open. + self._forget_handle(session_id, handle) + logger.warning( + "Freshly created Daytona sandbox %s for session %s was gone before " + "its workspace could be primed; not caching it", + sandbox_id, + session_id, + ) + raise SandboxGoneError( + f"Daytona sandbox {sandbox_id} vanished during workspace prime" + ) return sandbox_id finally: with self._create_cv: diff --git a/tests/sandbox/test_daytona.py b/tests/sandbox/test_daytona.py index ec8e9ef4..2387d335 100644 --- a/tests/sandbox/test_daytona.py +++ b/tests/sandbox/test_daytona.py @@ -946,6 +946,68 @@ def test_open_reattach_gone_falls_through_to_fresh_create(fake_sdk): assert box._handles["conv-1"].sandbox_id == "sbx-1" +def test_open_does_not_cache_a_fresh_sandbox_that_vanished_during_prime(fake_sdk): + """A just-created sandbox confirmed gone must not be handed back as live. + + ``_prime`` returns False only after ``_sandbox_gone`` confirms the sandbox + is gone, and its contract is that the caller discards the handle. The + reattach path obeys that; the fresh-create path must too -- ``open()`` + returns cached handles without revalidating them, so a dead id parked in + ``_handles`` is replayed by every later ``open()`` for that session. + """ + from application.sandbox.base import SandboxGoneError + from application.sandbox.daytona import DaytonaSandbox + + box = DaytonaSandbox(api_key="dtn_test", language="python") + + def _create_doomed(params=None, timeout=60): + sandbox = _FakeSandbox( + sandbox_id="sbx-dead", labels=getattr(params, "labels", None) or {} + ) + sandbox.fs.create_folder = mock.Mock( + side_effect=RuntimeError("not found (it has been deleted)") + ) + box._client.created.append((params, sandbox)) + return sandbox + + box._client.create = mock.Mock(side_effect=_create_doomed) + box._client.get = mock.Mock(side_effect=KeyError("gone")) # control plane: deleted + + with pytest.raises(SandboxGoneError): + box.open("conv-1") + + assert "conv-1" not in box._handles + # The in-flight slot is released, so the next open cold-starts rather than + # blocking on a create that already finished. + assert "conv-1" not in box._creating + + +def test_open_keeps_a_live_sandbox_whose_prime_merely_failed(fake_sdk): + """A prime that fails on a still-live sandbox is degraded, not fatal. + + ``exec`` prepends makedirs and ``put_file`` creates parent dirs, so the + workspace materializes on first use; failing the open here would reject a + perfectly usable sandbox. + """ + from application.sandbox.daytona import DaytonaSandbox + + box = DaytonaSandbox(api_key="dtn_test", language="python") + + def _create_degraded(params=None, timeout=60): + sandbox = _FakeSandbox( + sandbox_id="sbx-live", labels=getattr(params, "labels", None) or {} + ) + sandbox.fs.create_folder = mock.Mock(side_effect=RuntimeError("read timed out")) + box._client.created.append((params, sandbox)) + return sandbox + + box._client.create = mock.Mock(side_effect=_create_degraded) + + # ``get`` resolves and reports "started", so _sandbox_gone stays False. + assert box.open("conv-1") == "sbx-live" + assert box._handles["conv-1"].sandbox_id == "sbx-live" + + def test_put_file_gone_sandbox_raises_sandbox_gone_and_forgets(sandbox): from application.sandbox.base import SandboxGoneError diff --git a/tests/sandbox/test_sandbox_manager.py b/tests/sandbox/test_sandbox_manager.py index 7e47c16e..f4a6fac9 100644 --- a/tests/sandbox/test_sandbox_manager.py +++ b/tests/sandbox/test_sandbox_manager.py @@ -6,7 +6,7 @@ from typing import Dict, List import pytest -from application.sandbox.base import CodeSandbox, ExecResult +from application.sandbox.base import CodeSandbox, ExecResult, SandboxGoneError from application.sandbox.manager import SandboxCapacityError, SandboxManager @@ -336,6 +336,30 @@ def test_cap_eviction_picks_least_recently_used(backend, monkeypatch): assert mgr.has_session("a") and mgr.has_session("c") +def test_a_failed_cold_open_frees_the_reserved_cap_slot(backend, monkeypatch): + """A backend open that raises must not leak the slot it reserved. + + Reachable for real: ``DaytonaSandbox.open`` raises ``SandboxGoneError`` when + a freshly created sandbox is confirmed gone before its workspace is primed, + rather than caching a dead handle. + """ + mgr = SandboxManager(backend, max_ttl=600, max_sessions=1) + + def _vanished(session_id): + raise SandboxGoneError("sandbox vanished during workspace prime") + + monkeypatch.setattr(backend, "open", _vanished) + with pytest.raises(SandboxGoneError): + mgr.open("a") + + assert not mgr.has_session("a") + + # The reserved slot was released, so the retry cold-starts even at max_sessions=1. + monkeypatch.undo() + mgr.open("a") + assert mgr.has_session("a") + + def test_cap_rejects_when_all_sessions_busy(backend): mgr = SandboxManager(backend, max_ttl=600, max_sessions=1) mgr.open("a")