Skip to content

Commit 1099366

Browse files
msfstefclaude
andauthored
fix(agents): reclaim zombie sandbox containers and create them lazily (#4513)
## What & why Opening the desktop app could leave **15+ leftover `electric-sbx-*` Docker containers running** that the user never started. They piled up because, in several places, a sandbox container could outlive the work it was created for. This PR closes each of those gaps so a container only exists while something is actually using it. ## Why the containers piled up Sandbox containers are meant to be short-lived: created for an agent's work, then torn down. Three gaps let them survive instead: 1. **We created containers we didn't need.** Every time an agent woke up — even for trivial bookkeeping (a scheduled tick with nothing to do, a drained message queue) — it started a container *before* knowing whether it would run anything. When the app reconnected and replayed a backlog of these wakes, each one spun up its own container at once. 2. **We didn't clean up on quit.** Idle containers are torn down by a short (~2-minute) delay timer. On quit, the app exits before those timers fire — so every quit left a container behind. 3. **We didn't clean up leftovers at the next startup either.** The startup cleanup deliberately skipped *running* containers (so it wouldn't kill one another process is actively using). But a container left by a crash or quit is still running — its main process is an idle loop that never exits on its own — so the cleanup never touched the very leftovers it was meant to reclaim. Two smaller bugs made it worse: a container could be stranded if its one-time setup step failed, and a Docker error code was occasionally misread so we'd "reconnect" to a container that had already been removed. ## How we fix it The approach attacks all three gaps so a container can't outlive its purpose: 1. **Create only when actually used (lazy creation).** A container is now started the first time an agent really uses it — runs a command, or reads/writes a file. Trivial wakes create nothing. (Edge cases preserved: an agent finishing for good still cleans up any workspace earlier runs created, and a child agent that "inherits" its parent's sandbox still makes sure that sandbox exists first.) 2. **Clean up on quit.** On shutdown we now run the pending teardowns immediately instead of letting the delay timers die with the process. This covers every exit path (desktop quit, restarts, Ctrl-C / kill) and is time-bounded so a stuck Docker daemon can't hang the quit. 3. **Reclaim true leftovers at startup.** Every container is tagged with the process that created it. At startup we check whether that process is still alive: if it's gone, the container is a genuine leftover and we reclaim it — deleting throwaway ones and stopping reusable ones (so their files survive for later reuse). Containers a live process is still using are left untouched, so we never disturb a peer. As a quality-of-life touch, every container is also labelled so Docker Desktop groups them under one collapsible **`electric-sandboxes`** entry you can stop or delete together (`docker compose -p electric-sandboxes down`). <details> <summary>Implementation details (for reviewers)</summary> ### Zombie reclamation - Containers carry a `com.electric.sandbox.owner-pid` label; the boot sweep probes it (`kill(pid, 0)`) and reclaims running orphans whose owner is dead — **remove** ephemeral, **stop** persistent (writable layer survives for reattach by key). The sweep probes + reclaims leftovers concurrently so boot latency stays flat as they accumulate. - Labels are immutable, so reattaching a dead creator's container records adoption in an in-container marker (`/tmp/.electric-sbx-owner-pid`, tmpfs ⇒ wiped on stop); the sweep probes it before reclaiming, so a live sibling's adopted container is never swept. The sweep is now awaited at boot so it can't race the first wake's reattach. - New `shutdownAllDockerSandboxes()` flushes pending debounced teardowns on runtime shutdown, wired through `AgentHandlerResult.shutdownSandboxes` → `BuiltinAgentsServer.stop()` (covers desktop quit, runtime restarts, CLI SIGINT/SIGTERM; bounded 5s). Live-leased entries are left to their own dispose (a sibling runtime in the same process may own them) — if the process dies first, the pid-sweep reclaims them at next boot. - Post-start init verifies the `mkdir` exit code and force-removes the container on any failure; `isNameConflict()` now matches the daemon's actual name-conflict message instead of bare 409. ### Lazy sandbox creation - New `lazySandbox()` wrapper (`agents-runtime/sandbox/lazy.ts`) defers the provider factory until the sandbox is actually **used** (exec/fs/fetch). The bootstrap `docker` profile returns it, so trivial wakes never create a container. Materialization is single-flight and retried on failure. - Terminal reclaim still works without use: `reclaimDockerSandboxByKey()` wipes an earlier wake's persistent workspace by key without creating a container just to delete it (owner leases only; defers to live sibling leases — the last one draining wipes it). - Spawn-`inherit` force-materializes the owner's workspace (`ensureSandboxMaterialized`) before spawning, so a child can attach even when the parent never ran a tool. - Concurrent container **creations** are capped at 4 process-wide (`withCreationSlot`) — bursts queue against the daemon instead of stampeding it; reattaches/execs are unlimited, total creations unbounded. ### Grouping All sandboxes carry `com.docker.compose.project=electric-sandboxes` (+ `com.docker.compose.service=<entity-type>`). </details> ## Testing - 13 new docker integration tests (running-orphan reclaim, adoption sparing, legacy-label safety, init-failure cleanup, shutdown flush, lazy composition, reclaim-by-key) and 13 unit tests for `lazySandbox` — written first, confirmed red, then green. - Full suites: agents-runtime 803 tests / 63 files, agents 55 / 11; `tsc` and eslint clean. ## Out of scope (follow-ups) - No cap on concurrent *wakes* (they're whole agent runs; capping would queue user-visible messages behind backlog replay — needs a product call). - The `e2b` remote profile stays eager (working directory not statically known at profile-build time); the same wrapper applies once it is. - Server-side orphaned-claim recovery (`dispatchRecoveryIntervalMs` is defined but unused) — expired runner leases still queue wakes forever. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 2e4feee commit 1099366

16 files changed

Lines changed: 1214 additions & 161 deletions
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
'@electric-ax/agents-runtime': patch
3+
---
4+
5+
Docker sandbox creation now pulls the image only when it isn't already present
6+
locally, honoring the documented `pullIfMissing` semantics. Previously every
7+
container create called `docker pull`, which round-trips to the registry even
8+
for a fully cached digest-pinned image — making creation needlessly slow and
9+
prone to failing whenever the registry was briefly unreachable.
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
'@electric-ax/agents-runtime': patch
3+
'@electric-ax/agents': patch
4+
---
5+
6+
Fix leftover Docker sandbox containers (`electric-sbx-*`) piling up.
7+
8+
Sandbox containers are meant to be short-lived, but several gaps let them
9+
outlive the work they were created for — opening the desktop app could leave
10+
15+ containers running that were never explicitly started. This closes those
11+
gaps so a container only exists while something is actually using it:
12+
13+
- **Created only when used.** A container now starts the first time an agent
14+
actually uses its sandbox (runs a command, reads/writes a file), so trivial
15+
wakes (scheduled ticks, bookkeeping) no longer spin one up.
16+
- **Cleaned up on quit.** Shutdown now tears down idle containers immediately
17+
instead of leaving their delayed-teardown timers to die with the process.
18+
- **Leftovers reclaimed at startup.** Containers are tagged with the process
19+
that created them; at startup, those whose owner is gone are reclaimed
20+
(throwaway ones removed, reusable ones stopped so their files survive), while
21+
containers a live process is still using are left untouched.
22+
23+
Also: a failed container setup step no longer strands an untracked container,
24+
and all sandboxes are grouped under one `electric-sandboxes` entry in Docker
25+
Desktop so they can be stopped/removed together.

‎packages/agents-runtime/src/process-wake.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { createEntityLogPrefix, runtimeLog } from './log'
1212
import { createRuntimeServerClient } from './runtime-server-client'
1313
import type { RuntimeServerClient } from './runtime-server-client'
1414
import { unrestrictedSandbox } from './sandbox/unrestricted'
15+
import { ensureSandboxMaterialized } from './sandbox/lazy'
1516
import { resolveSandboxIdentity } from './sandbox/identity'
1617
import { appendPathToUrl } from './url'
1718
import { manifestChildKey } from './manifest-helpers'
@@ -1322,7 +1323,7 @@ export async function processWake(
13221323
const requestedInherit =
13231324
opts?.sandbox === `inherit` ||
13241325
(typeof opts?.sandbox === `object` && opts.sandbox.inherit === true)
1325-
const sandbox = requestedInherit
1326+
const childSandbox = requestedInherit
13261327
? resolvedSandboxSelection
13271328
? {
13281329
profile: resolvedSandboxSelection.profile,
@@ -1336,6 +1337,12 @@ export async function processWake(
13361337
: opts?.sandbox === `inherit`
13371338
? undefined
13381339
: opts?.sandbox
1340+
// An inheriting child only ever ATTACHES by key — make sure the
1341+
// owner's (lazily-created) container/workspace actually exists before
1342+
// the child can wake, even if this wake never ran a tool itself.
1343+
if (requestedInherit && resolvedSandboxSelection && sandbox) {
1344+
await ensureSandboxMaterialized(sandbox)
1345+
}
13391346
return serverClient.spawnEntity({
13401347
type: childType,
13411348
id: childId,
@@ -1344,7 +1351,7 @@ export async function processWake(
13441351
initialMessage: opts?.initialMessage,
13451352
initialMessageType: opts?.initialMessageType,
13461353
tags: opts?.tags,
1347-
sandbox,
1354+
sandbox: childSandbox,
13481355
wake: wakeOpt,
13491356
})
13501357
},

‎packages/agents-runtime/src/sandbox-docker.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99

1010
export {
1111
dockerSandbox,
12+
reclaimDockerSandboxByKey,
13+
shutdownAllDockerSandboxes,
1214
sweepOrphanedDockerSandboxes,
1315
__resetPersistentRegistryForTests,
1416
} from './sandbox/docker'

‎packages/agents-runtime/src/sandbox.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ export type { RemoteProvider, RemoteSandboxOpts } from './sandbox/remote'
88
export type { RemoteSandboxClient } from './sandbox/remote/types'
99
export { isE2BAvailable } from './sandbox/remote/e2b'
1010
export { chooseDefaultSandbox } from './sandbox/default'
11+
export { ensureSandboxMaterialized, lazySandbox } from './sandbox/lazy'
12+
export type { LazySandboxOpts } from './sandbox/lazy'
1113
export { SandboxError } from './sandbox/types'
1214
export type {
1315
Sandbox,

0 commit comments

Comments
 (0)