createSession adopts a running container, so a failed start can kill another process's live session

[amondnet]

Found by cubic on the third review pass over the same catch in #13. It is not a defect #13 introduced — git diff origin/main...feat/define-agent-api is empty for both files involved. It has been on main since e245fc1 (#7), and defineAgent only made it reachable from a new entry point.

Reproduction

1. Process A holds a live session on sessionId = "x". Its container is running.
2. Process B calls createSession({ sessionId: "x" }). B onCreate runs an exec, which acquires the sandbox.
3. docker/container.ts acquire() finds a container with that name and adopts it rather than failing — adopt() returns immediately for status === "running".
4. B onCreate rejects (a corepack enable that exits non-zero, a proxy that is unreachable).
5. B catch calls destroy(). A live session dies.

A second, quieter consequence of the same step 3: when B onCreate succeeds, A and B are now running two sessions inside one container, sharing a filesystem, with neither aware of the other.

Why no reap policy fixes it

Three rounds on #13 produced three positions, each correct against the one before it:

Policy	Fixes	Breaks
unconditional reap	never leaks	destroys a container this call adopted
reap only a generated id	keeps the adopted container	leaks a caller-named container that did not exist
unconditional reap (shipped)	never leaks	can destroy another process live session

By the time the catch runs the damage is upstream. packages/core/src/agent/define.ts is backend-agnostic by design and cannot see whether the acquisition adopted or created.

Where the fix belongs

HarnessV1SandboxProvider separates the two operations — createSession is the fresh path, and resumeSession "reattach[es] to an existing sandbox previously created with the same sessionId". packages/core/src/sandbox/harness/provider.ts routes both to the same session(id) and says so:

The contract cannot report whether session(id) found a sandbox or minted one, so createSession is taken as the fresh path and resumeSession as the returning one.

The primitive a real split needs already exists: ContainerHandle.peek() — "the container name if it is already running, without creating or starting anything."

Why this is a decision, not just a patch

The same file documents adoption as the feature:

Adoption is what makes a sandbox id resumable across processes: a second process asking for the same id finds the running container instead of colliding with it.

So removing adoption from the create path removes intended behaviour. The decision is where adoption moves to and what createSession does with a name that is taken — refuse, or create under a different one:

• Should createSession peek() first and throw on a live name?
• Does resumeSession become the only adopting path, and does our SandboxProvider need to distinguish them at all, or only the harness layer?
• What happens to a stopped container of the same name — adopt (today) or recreate?

Independent of #16: this holds even if defineAgent never supports resume, because createSession adopting a live container is wrong on its own.

[amondnet]

Correcting the framing above: it sends the reader to the wrong file, and the destructive half is smaller than it claims.

I wrote that no fix is possible without deciding the createSession / resumeSession split. That holds for the sharing consequence. It does not hold for the destructive one, which is a plain ownership bug inside packages/core/src/sandbox/docker/container.ts:

remove: async () => {
  acquisition = undefined
  const result = await runDocker([rm, --force, --volumes, name])

remove() is addressed by name and blind to ownership. A handle that adopted another process container will force-remove it — and so will a handle that never called ready() at all. Meanwhile acquire() already knows which of the two things it did, since it branches on exactly that. The information is present one function away from the call that needs it.

So the reap-policy argument on #13 was chasing the wrong thing on all three passes. No policy in agent/define.ts could have fixed this, not because the signal was missing from the contract, but because it never had to leave container.ts.

Being fixed now, ownership only: acquire() reports created-vs-adopted, the handle records it across the latch resets that remove() and a failed acquisition already perform, and remove() removes only a container this handle created — adopted ones and never-acquired handles resolve without issuing anything. No contract change, no change to agent/define.ts, and acquire() keeps adopting exactly as it does today.

Still open, and still belonging to #16: the quieter consequence — a successful onCreate on an adopted container leaves two sessions sharing one filesystem, neither aware of the other. Narrowing what createSession adopts removes a behaviour harness/provider.ts documents as the feature that makes an id resumable across processes, so it is a decision about where adoption lives, not a bug fix.