Decide whether defineAgent supports session resume

[amondnet]

Two independent reviews of #13 landed on the same gap from opposite sides, and both fixes there are partial because the underlying semantics are undecided.

What is undefined

defineAgent().createSession({ sessionId }) accepts a caller-supplied id, and sandbox/docker/container.ts adopts a container that already carries that name rather than failing. So a supplied id can mean either "resume the sandbox I had" or "create one under a name I chose", and nothing in the API distinguishes them.

Three behaviours currently follow from that ambiguity:

1. Re-seeding over live work. onSession seeds the workspace on every session, so reconnecting with a known id overwrites files the previous session edited (the example's sum.js goes back to its buggy original). Per-session seeding is the documented design; what is missing is what a reopen should do instead.
2. Reaping the wrong container. feat(core): add defineAgent and defineSandbox as the first public API #13 originally destroyed the sandbox on any createSession failure, which threw away an adopted container's resumable state. It now reaps only when the id was generated here (created = options?.sessionId === undefined).
3. The residual leak that fix leaves. created records how the id was obtained, not whether this call created the container. A caller-supplied id whose container did not previously exist is created by onCreate and then not reaped on failure — it leaks, and on a paid backend it bills until its own timeout.

Why it is not a one-line fix

The precise signal exists in the backend — container.ts knows whether it adopted or created — but SandboxProvider exposes only session(sandboxId) and portEndpoint(...), so there is no vendor-neutral way to ask. Surfacing it is a contract change, and per CLAUDE.md that is an API decision to surface rather than invent.

What needs deciding

• Does a supplied sessionId mean resume or name? If both are wanted, they are two call shapes, not one.
• Should the sandbox contract report whether a session handle was adopted or created?
• On a reopen, does the workspace re-seed, skip, or merge?

Where this came from

• Reviewed on feat(core): add defineAgent and defineSandbox as the first public API #13: packages/core/src/agent/define.ts — the createSession try/catch, and the onSession seed above it.
• Raised independently by cubic (P1, confidence 9) and by a local /code-review pass (low, "no defined semantics yet").

[amondnet]

Re-confirmed on the fix itself. After #13 narrowed the reap to created = options?.sessionId === undefined, cubic reviewed the new code and flagged consequence 3 above independently, at confidence 10:

When a caller supplies a new sessionId, Docker creates its container, but this flag remains false; a later setup failure then skips cleanup and leaks the container. Track ownership from backend acquisition rather than inferring it from whether the ID was generated.

That is three independent readers on the same line — the test author who wrote the coverage, cubic, and the review reply. It is not a disagreement about whether the gap is real, only about where the signal has to come from, and everyone lands on the same place: the backend.

Thread left open on #13 rather than resolved: #13 (comment)

[amondnet]

Correcting this issue: its central premise was wrong, and most of it is already answered by the contract.

"Does a supplied sessionId mean resume or name?" is not open. It means name. The AI SDK is explicit that a sessionId alone does not carry prior state — it creates a new session under that id — and that resumable state travels in resumeFrom, used together with the original sessionId.

"Should the sandbox contract report whether a handle was adopted or created?" is not open either. HarnessV1SandboxProvider already separates the operations: createSession is the fresh path, and resumeSession "reattach[es] to an existing sandbox previously created with the same sessionId" (optional; the harness throws HarnessCapabilityUnsupportedError against a provider that omits it). packages/core/src/sandbox/harness/provider.ts maps both onto the same session(id) and says why in a comment — this repo had the distinction and collapsed it.

Consequence 3 (the leak) is fixed, by removing the guard rather than refining it — 2725afa on #13. Agent.createSession accepts { sessionId } only and always calls the fresh path, so defineAgent cannot produce a resumable session; the guard defended an unreachable case and leaked in a reachable one. The unconditional reap is correct for the API as it stands.

What is actually left

One question, narrower than the three above:

Should defineAgent support resuming at all — exposing resumeFrom (and continueFrom) on createSession, and routing to the provider resumeSession path when it does?

Everything else follows from that answer rather than needing its own decision:

• Consequence 1 (re-seeding over live work) stops being a defect the moment reopening is a distinct operation: onSession seeds a fresh session, which is correct for the only path that exists today.
• Sandbox lifetime on the resume path is already specified upstream — a caller-supplied sandboxSession means the caller owns the sandbox and destroy() leaves it alone.
• docker/container.ts adopting inside its create path is what makes a reused id look resumable today. That is worth revisiting when resume support lands, not before.

Retitling would help: this is "Decide whether defineAgent supports session resume", not "define what reusing a sessionId means".

Thanks to [the reviewer] for the pointer to the session-lifecycle docs — the original framing here was built on my own inference rather than on that page.

[amondnet]

A concurrency scenario, found by cubic on the third pass over the same line, is now the strongest argument here — and it is the first one that no reap policy in define.ts can fix.

The scenario

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 — and docker/container.ts's acquire() adopts a running container by name rather than failing.
3. B onCreate rejects.
4. B catch destroys the container. A live session dies.

No resume semantics are involved, so the reasoning that closed the earlier rounds — naming is not resuming, sessionId alone creates a fresh session, resumable state travels in resumeFrom — does not reach it.

Why the reap policy is the wrong place to argue about it

Three rounds on the same catch produced three positions, each correct against the previous one:

Policy	What it fixes	What it breaks
unconditional reap	never leaks	destroys a container this call adopted
reap only a generated id	keeps an adopted container	leaks a caller-named container that did not exist
unconditional reap (current)	never leaks	can destroy another process live session

The oscillation is the diagnostic. By the time the catch runs, the damage is upstream: createSession adopted a container it should not have. That is not a decision define.ts can make — it is backend-agnostic by design, and the adoption happens two layers down.

What this adds to the decision

It strengthens the case for actually implementing the create/resume split rather than leaving it collapsed, and it makes that independent of whether defineAgent ever exposes resumeFrom. Even with no resume support at all, createSession adopting a live container is wrong.

The primitive already exists: ContainerHandle.peek() — "the container name if it is already running, without creating or starting anything" — is exactly what a create path needs to refuse a name that is taken. What is missing is that harness/provider.ts routes both createSession and resumeSession to the same session(id).

Thread on #13 left open rather than resolved: #13 (comment)

[amondnet]

The concurrency scenario in my comment above has been split out into #18, where it belongs: it is a concrete defect with a reproduction, it has been on main since e245fc1 rather than arriving with #13, and it holds independently of this issue — createSession adopting a live container is wrong even if defineAgent never supports resume.

What stays here is the design question alone: should defineAgent expose resumeFrom (and continueFrom) on createSession, routing to the provider resumeSession path when it does?

My reading, for whoever picks this up: not yet. resumeFrom is opaque state the application has to persist, and nothing in please persists anything — workflows are undesigned by README.md own account. Adding resume now would design the resume story before the thing that needs it exists. #18 does not wait on that answer.

[amondnet]

Decision: not yet

defineAgent does not expose resumeFrom or continueFrom, and Agent.createSession keeps its single option, { sessionId?: string }. Every session it makes is a fresh one.

Three reasons, in order of weight.

Nothing here can hold resumeFrom. It is opaque state the application persists between runs — the AI SDK hands it back and expects it back — and please persists nothing. There is no store, no lifecycle that outlives a process, and the layer that would own one is undesigned by README.md's own account: "Undecided: most of the rest. How a workflow is expressed…". Shipping resume now would fix the resume story to whatever shape today's single example happens to need, and that shape would then constrain the workflow design rather than follow from it.

The harm that made this look urgent has been fixed without it. #16 was the parent of a concurrency defect — process B's failed createSession force-removing process A's live container — and the whole argument for deciding resume quickly was that no reap policy in define.ts could fix that. Correct, and irrelevant: the fix never had to leave container.ts. #18 shipped in #25 (cdd91d0), where acquire() reports created-vs-adopted and remove() removes only what this handle created. So this issue is no longer blocking a live bug, and can be decided on design grounds alone.

The API is small and the cost of waiting is one additional parameter. createSession({ sessionId }) today, createSession({ sessionId, resumeFrom }) later. No caller has to change, and no shape shipped now has to be unshipped.

What this decision does not cover

One thing from #18 landed back here and is still open: acquire() adopts a running container on the create path, so two createSession calls on the same id that both succeed end up as two sessions inside one container, sharing a filesystem, neither aware of the other. #25 fixed the destructive half of that (removal) and deliberately left adoption exactly as it was.

That defect does not depend on this decision, and I do not think it should wait for it. HarnessV1SandboxProvider already gives us both entry points — createSession is the fresh path, resumeSession "reattach[es] to an existing sandbox previously created with the same sessionId" — and packages/core/src/sandbox/harness/provider.ts:90 routes both to the same session(id), saying so in a comment. Splitting them is filling in a contract we already implement, not adding public API, and it is correct whether or not defineAgent ever calls the resuming path. The primitive is already there: ContainerHandle.peek() — "the container name if it is already running, without creating or starting anything."

One sub-decision needs a call before that can be written, because both readings are defensible:

• createSession against a name that is already running — refuse (surfacing the collision to the caller), or create under a different name (silently making the id non-unique)? I would refuse: a name collision is either a bug in the caller's id generation or a genuine reopen, and both are better as an error than as a share.
• createSession against a name that exists but is stopped — today adopt() restarts it, so a "fresh" session inherits the previous one's filesystem. Same question, and I would treat it the same way.

Say the word and I will file that as its own issue with the two options written out, the way #18 was split from here.

What a resume implementation has to settle, when it lands

Recording these so the answer is reusable rather than re-derived:

• Call shape. resumeFrom travels with the original sessionId, not instead of it, and routes to the provider's resumeSession. A sessionId alone stays what the AI SDK says it is — a name for a new session, not a reference to an old one.
• Workspace on reopen. onSession seeds on every session, which is right for a fresh one and wrong for a reopen — the example's sum.js reverting to its buggy original is the demonstration. A reopen must skip the seed; whether it may also merge is a separate question that only matters once a workspace can change between runs.
• Sandbox lifetime. Already specified upstream, nothing for us to decide: a caller-supplied sandboxSession means the caller owns the sandbox, and destroy() leaves it alone.
• Adoption's new home. Whatever the sub-decision above settles, resumeSession becomes the only path allowed to adopt.

What would change this answer

A caller that actually persists resumeFrom — the first workflow, the first channel handler, the first deploy target that survives a restart. When one of those exists it will say what resume has to look like, and that is a better input than this issue is.

Until then I would keep this open as the design record rather than close it, since the question is deferred rather than answered "no".

[amondnet]

Split out as #26, with one correction to the comment above.

I wrote that splitting createSession from resumeSession adds no API because ContainerHandle.peek() is already there. Half right. peek() exists on every backend — docker/container.ts:192, local/root.ts:104, just-bash/sandbox.ts:46, all with the same meaning — but all three are backend internals, and harness/provider.ts holds only a SandboxProvider, whose surface is backend / session(id) / portEndpoint(...). Nothing on it, or on SandboxSession, reports whether a sandbox is already live. So the split does need a contract decision, which is why #26 is written as options rather than as a patch.

That does not change this issue's answer. #26 holds with or without resume support.