fix(providers): teardown terminates a sandbox by id, so one that already finished is counted down

[earakely-scale]

Summary

A sandbox can finish before teardown reaches it: killed, out of memory, or past its timeout. On Modal, teardown then logged two warnings for a sandbox that was already down:

• teardown: sb-… (modal) was not terminated: ConflictError: Sandbox has already finished with status terminated
• episode …: ['sb-…'] may still be up

"may still be up" is how a real leak is reported, so a false one on every dead env hides the true ones.

Cause: both teardown paths (teardown_run and the teardown_sandboxes step) got a handle with get_sandbox before terminating. Modal's get_sandbox reads the sandbox's tunnels, which Modal refuses for a finished sandbox.

Why not fix get_sandbox: this PR's first version had get_sandbox return a handle for a finished sandbox. That would hide dead containers from env reattach, which relies on get_sandbox raising: _reattach_containers raises for a dead server and skips a dead database, and MultiEnv raises for a dead database.

Fix: a new SandboxProvider.terminate_sandbox(sandbox_id) terminates a sandbox the provider made, by id. A sandbox that has already finished or is gone counts as terminated. The base default is get_sandbox then terminate().

Provider	terminate_sandbox	Already finished or gone
modal, modal_vm	Sandbox.from_id then terminate(), with no tunnels and no exec	Modal's terminate() is a no-op once finished; NotFoundError counts as gone
e2b	the class-level kill(id), with no connect	kill returns False
tenki	client.get then the session close TenkiSandbox.terminate does, now shared as close_session	SessionNotFoundError, or an ended state
tensorlake	connect then terminate(), skipping info()	SandboxNotFoundError
sail_vm	through get_sandbox, since its terminate() also deletes the model-key policy that only the live Sailbox's applied policy names	NotFoundError
smol_vm	Machine.list, then connect and delete() for a listed machine, skipping the Docker setup and host configuration	the machine isn't listed
local, chained	the base default	the provider's own terminate()

Two choices worth a look:

• The chained provider keeps the base default. Its get_sandbox finds the provider that owns the sandbox. Looping over each provider's terminate_sandbox instead would let the first provider count another provider's sandbox as "gone".
• A record that doesn't name its backend still goes through a live handle. For the same reason, terminate_sandbox is only called when the record names the backend that made the sandbox. A record without one, which no current deploy path writes, still goes through the slot provider's get_sandbox. Local work-folder removal now keys on the provider's ON_THIS_MACHINE.

Known limits:

• SAIL: a Sailbox that's already gone leaves its model-key policies behind, since nothing can name them any more.
• smol: the SDK starts a stopped local machine when it connects, and it has no delete-by-name.

Lint commit: a separate commit clears the six ruff findings that main's unit job currently fails on. They're in SmolVM's checks: F811 on an imported fixture, and one placeholder-less f-string. The commit can be split out if preferred.

Test plan

• Unit tests for every provider:

  • a live sandbox is terminated through the SDK without get_sandbox's extras: no tunnels, no info(), no connect for E2B, no Docker setup for smol;
  • a gone or finished sandbox raises nothing;
  • Modal's get_sandbox still raises for a finished sandbox, which is the reattach contract.

• Teardown tests:

  • a typed record is terminated by id without a handle, and an untyped one through a handle;
  • a teardown_run regression test through the real Modal provider, with only the SDK faked: a finished sandbox lands in terminated, nothing is in still_up, and no tunnel is read.

• With main's teardown restored, the three new teardown tests fail.

• Full unit suite: 8119 passed. The one failure is smol_vm_provider_test::test_vm_host_entry_updates_without_dropping_other_guest_names, which fails on main too on macOS because BSD sed differs from GNU's. It's skipped in CI, where the smol extra isn't installed, which also means this PR's new smol test only runs where smolmachines is installed.

• ruff check is clean.

• Real backends, from Linux VMs, with this commit installed:

  Backend	Live sandbox	Again, once gone	Killed out-of-band, then teardown_run	Unknown id	Warnings
  Modal	terminated	ok	killed (137) and timed out (124) in terminated, nothing failed	ok	0
  SAIL	terminated	ok	terminated, nothing failed	ok	0
  SmolVM (local, KVM host)	terminated	ok	terminated, nothing failed	ok	0

  • Modal reattach contract: get_sandbox on a killed sandbox still raises ConflictError.
  • Modal soak rounds, 32 concurrent PortSim episodes per round: two kill rounds (4 env sandboxes killed mid-episode each), a steady round and an interrupt round. All pass, with 0 "was not terminated" and 0 "may still be up", where 0.9.1326 gave 4 per kill round. No sandbox is left.
  • Local Docker soak: kill, steady and interrupt rounds all pass, with 0 warnings and no containers, networks or ports left.
  • E2B, Tenki and Tensorlake are covered by unit tests only, as no credentials were available for a real run.

🤖 Generated with Claude Code

Confidence Score: 5/5

The PR appears safe to merge; no actionable new defect was established.

Summary

Adds SandboxProvider.terminate_sandbox and routes typed teardown records through it.

• Teardown stops named sandboxes by ID.

No actionable new issue was established. No previous review threads were supplied.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Recorded sandbox] --> B{Backend recorded?}
  B -->|Yes| C[Build recorded provider]
  C --> D[terminate_sandbox]
  D --> E[Backend-specific termination]
  B -->|No| F[Use configured provider]
  F --> G[get_sandbox]
  G --> H[Handle terminate]
  E --> I[Record successful teardown]
  H --> I
  E -->|Error| J[Report possible leftover]
  H -->|Error| J

Loading

_{Reviews (2) · Last reviewed commit: "fix(providers): teardown terminates a sa..." · Reviewed by Greptile}