Skip to content

Resume a persisted ACP session instead of always starting a new one - #3

Open
tmecklem wants to merge 2 commits into
mainfrom
resume-persisted-acp-sessions
Open

Resume a persisted ACP session instead of always starting a new one#3
tmecklem wants to merge 2 commits into
mainfrom
resume-persisted-acp-sessions

Conversation

@tmecklem

Copy link
Copy Markdown
Member

The problem

ACPRuntime.Session is started from a persisted session row, and that row already carries an acp_session_id whenever the session has run before — after the local runtime crashed, or was lost to a host restart.

Client.start_session/7 ignored it and always called session/new. Two consequences:

  • The user got a fresh, empty transcript for work that was still there. The agent still held the conversation; nothing asked for it back.
  • The previous session was stranded inside the agent, with nothing pointing at it.

Downstream (in Launchbox) this shows up as a coding-agent tab that silently disappears when you close and reopen a card: the runtime can be restarted from the row, but reconnecting produced a blank session, so there was nothing worth showing.

The fix

start_session/7 accepts a :resume_session_id, which Session.connect/2 fills in from the persisted acp_session_id. When one is present the client calls session/load, and the agent replays the conversation as session/update notifications.

opts = Keyword.put(state.client_options, :resume_session_id, state.acp_session_id)

The load is best-effort. An agent that doesn't advertise loadSession, or one that has since expired the session, falls back to session/new rather than failing the connect — a resume that can't be honoured still has to leave the user with a working agent.

Behaviour is unchanged when there's nothing to resume (acp_session_id is nil), which is every session that has not run before.

Notes

  • ExMCP.ACP.Client.load_session/4 already existed and gates on the agent's loadSession capability internally, so there's no duplicated capability check here — an unsupported load simply returns an error and falls through.
  • No public API breakage: start_session/7 keeps its arity and the new value rides in the existing opts keyword, so alternative client_module implementations are unaffected.

Test plan

mix precommit — 32 tests, clean build, formatted.

New coverage:

  • client_test.exs — a resumed session sends session/load with the right sessionId/cwd, and returns that id.
  • client_test.exs — an agent that rejects the load falls back to session/new and still returns a working session.
  • session_test.exs — a persisted acp_session_id is passed down as :resume_session_id; a session with nothing to resume passes nil.

Also verified against a real consumer: pointed Launchbox at this checkout as a path dep and confirmed a Launchbox-persisted acp_session_id reaches the client as :resume_session_id, and that the assertion fails without the Session.connect/2 change. Launchbox's own suite passes against this branch.

A Session restarted from its persisted row — after the local runtime crashed,
or was lost to a restart — always called session/new. The agent still held the
conversation, so the user was handed an empty transcript for work that was
still there, and the old session was left stranded in the agent.

start_session/7 now takes a :resume_session_id, which Session fills in from the
persisted acp_session_id. When present, the client calls session/load so the
agent replays the conversation as session/update notifications.

The load is best-effort: an agent that doesn't advertise loadSession, or one
that has since expired the session, falls back to session/new rather than
failing the connect. A resume that can't be honoured still has to leave the
user with a working agent.
A host whose only record of a running agent is the environment itself needs
that process addressable and tagged from the moment it spawns — not once the
runner happens to report back, which leaves a window where a live agent is
untagged and therefore undiscoverable.

start_session/7 forwards :process_session_id, :process_name and
:process_metadata to the process runner. All optional; runners that ignore them
behave exactly as before.
@tmecklem
tmecklem requested a review from superchris August 27, 2026 04:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant