Skip to content

fix(http): only open session streams for sessions the connection knows - #483

Merged
benbrandt merged 1 commit into
mainfrom
fix-http-unknown-session-streams
Nov 2, 2026
Merged

benbrandt merged 1 commit into
mainfrom
fix-http-unknown-session-streams

Conversation

@benbrandt

Copy link
Copy Markdown
Member

Bug

The HTTP server created a session SSE mailbox on demand for any Acp-Session-Id, from a GET or from any session-scoped POST. A client holding a valid connection ID could make it allocate streams for made-up session IDs without limit, and mailboxes for real sessions were never removed. The streamable HTTP RFD says an unknown Acp-Session-Id gets 404.

Fix

  • A session is known on a connection once the agent names it: in a session/new / session/fork result, in a successful response to a request scoped to it (session/load, session/resume, ...), or in a request or notification it sends. The session is recorded before the response that names it is queued, so a GET sent right after that response finds it.
  • POSTs no longer allocate session streams. A response for a session with no stream goes to the connection stream, so the client still gets its error.
  • The RFD's resume flow, the md guide, and HttpClient all open the session GET before session/load, so a strict 404 there would break HttpClient and RFD-conformant clients. Instead, up to 16 unconfirmed session streams are allowed per connection. Each is forgotten when the client closes it before the agent names the session. Beyond that, the GET gets 404 Not Found.
  • A successful session/close / session/delete removes the session's stream. A reader that is still attached gets the response, and its stream stays open until it disconnects.

No public API changes.

Tests

  • connection.rs unit tests: the unconfirmed stream cap and release, a session created by session/new being openable right after its response even with every unconfirmed slot taken, a POST for an unknown session allocating nothing and its error reaching the connection stream, a pre-load stream confirmed by its result (with replay/response order kept), a failed load leaving the session unknown, and close removing the stream.
  • http_server.rs: a GET past the cap gets 404, and 100 POSTs with made-up session IDs allocate no streams.
  • New tests/session_streams.rs: HttpClient against AcpHttpServer running session/new + session/prompt, session/load with the stream opened first, and a prompt for an unknown session getting its error.

The HTTP server created a session SSE mailbox on demand for any
`Acp-Session-Id`, from both GET and session-scoped POSTs, so a client could
make it allocate streams for made-up session IDs without limit. Mailboxes
for real sessions were also never removed.

A session is now known on a connection once the agent names it: in a
`session/new` or `session/fork` result, in a successful response to a
request scoped to it (such as `session/load` or `session/resume`), or in a
request or notification it sends. The session is recorded before the
response naming it is queued, so a GET sent right after that response
finds it. POSTs no longer allocate streams, and a response for a session
with no stream goes to the connection stream.

A client may still open a stream before `session/load` so it sees the
replay, as the RFD's resume flow and `HttpClient` do. Up to 16 such
unconfirmed streams are allowed per connection, each forgotten when the
client closes it before the agent names the session. Beyond that, a GET for
an unknown session gets 404. A successful `session/close` or
`session/delete` removes the session's stream.

Co-Authored-By: Claude <noreply@anthropic.com>
@benbrandt
benbrandt merged commit 39915a2 into main Nov 2, 2026
10 checks passed
@benbrandt
benbrandt deleted the fix-http-unknown-session-streams branch November 2, 2026 10:53
@acp-release-bot acp-release-bot Bot mentioned this pull request Nov 2, 2026
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