Skip to content

Commit 65af1fd

Browse files
dcramercodex
andcommitted
browser: teardown runtimes on shutdown and avoid default warmup container
- add manager/provider shutdown lifecycle to remove dedicated browser containers on service stop - make sandbox warmup preflight-only for scoped runtimes to avoid spawning default-scope container - add regression tests for warmup and shutdown behavior Co-Authored-By: GPT-5 Codex <codex@openai.com>
1 parent 2b7ce85 commit 65af1fd

6 files changed

Lines changed: 91 additions & 8 deletions

File tree

src/ash/browser/manager.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,23 @@ async def warmup_default_provider(self) -> None:
9292
extra={"browser.provider": provider_key},
9393
)
9494

95+
async def shutdown(self) -> None:
96+
"""Best-effort shutdown for provider runtimes during service teardown."""
97+
for provider_key, provider in self._providers.items():
98+
shutdown = getattr(provider, "shutdown", None)
99+
if not callable(shutdown):
100+
continue
101+
try:
102+
await shutdown()
103+
except Exception as e:
104+
logger.warning(
105+
"browser_runtime_shutdown_failed",
106+
extra={
107+
"browser.provider": provider_key,
108+
"error.message": str(e),
109+
},
110+
)
111+
95112
def _action_timeout_seconds(self, action: str) -> float:
96113
base = self._clamp_timeout_seconds(float(self._config.browser.timeout_seconds))
97114
if action == "session.start":

src/ash/browser/providers/sandbox.py

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,33 @@ async def close_session(self, *, provider_session_id: str | None) -> None:
162162
return None
163163

164164
async def warmup(self) -> None:
165-
"""Boot and verify warm browser runtime ahead of first user action."""
166-
await self._ensure_runtime(scope_hash=self._scope_hash(None))
165+
"""Warmup preflight for scoped dedicated runtime.
166+
167+
Dedicated runtime containers are keyed by effective user scope, which is
168+
unknown at process startup. Warmup therefore validates that the browser
169+
image is available but does not start a default container.
170+
"""
171+
inspect_image = await self._execute_host_command(
172+
["docker", "image", "inspect", self._container_image],
173+
timeout_seconds=15,
174+
)
175+
if not inspect_image.success:
176+
raise ValueError(
177+
"sandbox_browser_launch_failed: missing_browser_image:"
178+
f"{self._container_image}"
179+
)
180+
181+
async def shutdown(self) -> None:
182+
"""Best-effort runtime shutdown for service lifecycle teardown."""
183+
async with self._runtime_lock:
184+
self._sessions.clear()
185+
self._session_targets.clear()
186+
runtime = self._runtime
187+
self._runtime = None
188+
self._active_scope_hash = None
189+
if runtime is None:
190+
return
191+
await self._kill_runtime(runtime)
167192

168193
async def goto(
169194
self,

src/ash/integrations/browser.py

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,14 +65,21 @@ async def on_startup(self, context: IntegrationContext) -> None:
6565
)
6666

6767
async def on_shutdown(self, context: IntegrationContext) -> None:
68-
_ = context
6968
if self._warmup_task is None:
69+
pass
70+
else:
71+
if not self._warmup_task.done():
72+
self._warmup_task.cancel()
73+
with contextlib.suppress(asyncio.CancelledError):
74+
await self._warmup_task
75+
self._warmup_task = None
76+
77+
manager = getattr(context.components, "browser_manager", None)
78+
if manager is None:
7079
return
71-
if not self._warmup_task.done():
72-
self._warmup_task.cancel()
73-
with contextlib.suppress(asyncio.CancelledError):
74-
await self._warmup_task
75-
self._warmup_task = None
80+
shutdown = getattr(manager, "shutdown", None)
81+
if callable(shutdown):
82+
await shutdown()
7683

7784
def augment_prompt_context(
7885
self,

tests/test_browser.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,14 @@ class _FakeWarmupProvider(_FakeProvider):
113113

114114
def __init__(self) -> None:
115115
self.warmup_calls = 0
116+
self.shutdown_calls = 0
116117

117118
async def warmup(self) -> None:
118119
self.warmup_calls += 1
119120

121+
async def shutdown(self) -> None:
122+
self.shutdown_calls += 1
123+
120124

121125
class _FailingRuntimeProvider(_FakeProvider):
122126
async def start_session(
@@ -540,6 +544,17 @@ async def test_browser_manager_warmup_default_provider(tmp_path) -> None:
540544
assert provider.warmup_calls == 1
541545

542546

547+
@pytest.mark.asyncio
548+
async def test_browser_manager_shutdown_calls_provider_shutdown(tmp_path) -> None:
549+
store = BrowserStore(tmp_path / "browser")
550+
provider = _FakeWarmupProvider()
551+
manager = BrowserManager(
552+
config=_config(), store=store, providers={"sandbox": provider}
553+
)
554+
await manager.shutdown()
555+
assert provider.shutdown_calls == 1
556+
557+
543558
@pytest.mark.asyncio
544559
async def test_create_browser_manager_omits_kernel_when_unconfigured(
545560
tmp_path, monkeypatch: pytest.MonkeyPatch

tests/test_browser_sandbox_provider.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,19 @@ async def _bridge_timeout(
182182
)
183183

184184

185+
@pytest.mark.asyncio
186+
async def test_sandbox_provider_warmup_does_not_start_default_container() -> None:
187+
stub = _HostDockerStub()
188+
provider = SandboxBrowserProvider()
189+
provider._execute_host_command = stub.run # type: ignore[assignment]
190+
191+
await provider.warmup()
192+
193+
assert any(args[:3] == ["docker", "image", "inspect"] for args in stub.calls)
194+
assert not any(args[:2] == ["docker", "create"] for args in stub.calls)
195+
assert not any(args[:2] == ["docker", "start"] for args in stub.calls)
196+
197+
185198
async def _noop_wait(*, runtime: Any) -> None:
186199
_ = runtime
187200

tests/test_integrations_builtin.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,10 +167,14 @@ async def test_browser_integration_owns_manager_tool_and_warmup(monkeypatch) ->
167167
class _FakeManager:
168168
def __init__(self) -> None:
169169
self.warmup_calls = 0
170+
self.shutdown_calls = 0
170171

171172
async def warmup_default_provider(self) -> None:
172173
self.warmup_calls += 1
173174

175+
async def shutdown(self) -> None:
176+
self.shutdown_calls += 1
177+
174178
fake_manager = _FakeManager()
175179
monkeypatch.setattr(
176180
"ash.browser.create_browser_manager",
@@ -184,6 +188,8 @@ async def warmup_default_provider(self) -> None:
184188
await integration.on_startup(context)
185189
await asyncio.sleep(0)
186190
assert fake_manager.warmup_calls == 1
191+
await integration.on_shutdown(context)
192+
assert fake_manager.shutdown_calls == 1
187193

188194

189195
@pytest.mark.asyncio

0 commit comments

Comments
 (0)