Skip to content

fix: isolate request-scoped HTTP POST failures - #307

Merged
benbrandt merged 3 commits into
agentclientprotocol:mainfrom
federicociner:codex/isolate-http-post-failures
Oct 23, 2026
Merged

benbrandt merged 3 commits into
agentclientprotocol:mainfrom
federicociner:codex/isolate-http-post-failures

Conversation

@federicociner

@federicociner federicociner commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

A failed HTTP POST for one request could close the shared ACP connection, reject unrelated pending requests, abort all SSE streams, and issue a connection-level DELETE.

This change converts request-scoped HTTP failures into JSON-RPC errors for the originating request, keeping the shared transport usable. Errors retain the HTTP status and response body, capped at the smaller of 8 KiB or maxMessageBytes. Failed operations are not automatically retried.

HTTP 401/403 continue to close the shared connection. This isolation applies to outbound requests; failures sending notifications or responses retain their existing behaviour.

Reproduction

  1. Instantiate the real createHttpStream transport and JSON-RPC Connection, using a controlled fetch implementation to simulate HTTP responses and SSE streams.
  2. Initialize one connection shared by sessions A and B.
  3. Send a session/prompt request for A and leave its response pending.
  4. Send a session/load request for B whose POST returns HTTP 500.
  5. Attempt to complete A’s request over its SSE stream.

Before the fix, both requests rejected with B’s HTTP error, the connection closed, and one DELETE was issued.

After the fix, only B rejected. A completed over SSE, subsequent requests succeeded, and no DELETE was issued.

Validation

  • Repeated the reproduction with empty HTTP 500 bodies and oversized error bodies; details remained bounded.
  • Confirmed HTTP 404/410 remain request-scoped and HTTP 401/403 still close the connection.
  • Full npm run check passed, including all 490 existing tests.

The reproduction was an ad hoc script using the real SDK transport and connection with simulated network responses; no new automated tests were added.

federicociner and others added 3 commits October 22, 2026 21:53
…T failures

- Treat POST 404/410 (Unknown Acp-Connection-Id) as connection-scoped again so
  a lost connection closes the transport instead of failing every request.
- Skip the synthesized error when the request already settled while its POST
  was in flight, avoiding two responses for one id.
- Drop a multi-byte character split by error body truncation.
- Name the error body cap and pass truncate as an option.
- Document request-scoped POST failures on createHttpStream.
- Add tests for isolation, teardown statuses, notification failures,
  truncation, and duplicate responses.
@benbrandt
benbrandt marked this pull request as ready for review October 23, 2026 19:31
@benbrandt
benbrandt enabled auto-merge (squash) October 23, 2026 19:31
@benbrandt
benbrandt merged commit d9db04d into agentclientprotocol:main Oct 23, 2026
2 checks passed
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.

2 participants