Coalesce intercepted HTTP response chunks

[stephentoub]

Summary

• add bounded 32 KiB read-ahead and response-chunk coalescing to the Node.js, Python, Go, .NET, and Java SDKs
• keep at most one llmInference.httpResponseChunk data RPC outstanding while preserving byte/error order and immediate partial flushing
• promptly cancel response sources on runtime cancellation, RPC rejection, and connection loss
• encode Go response bytes as binary data instead of lossy JSON text
• add protocol-level tests for withheld acknowledgements, bounded backpressure, cancellation, connection loss, and upstream errors
• make Java JSON-RPC requests fail immediately after connection closure so terminal handling cannot strand a worker

Validation

• Node.js: TypeScript check and 6 focused Vitest cases
• Python: Ruff and 5 focused pytest cases; full unit suite also passed (543 tests)
• Go: gofmt and go test ./...
• .NET: formatting verification and 7 focused tests on .NET 8
• Java: Spotless, Checkstyle, and 35 focused tests
• Opus 5 fix-first review completed with no unresolved high-confidence findings

Throwaway real-transport benchmarks showed substantial gains for typical small streaming fragments across all five SDKs; no benchmark code is included.

Generated by Copilot

[Copilot]

Copilot review overview

🟡 Changes recommended

The Go no-progress read path can ignore cancellation and indefinitely block response cleanup.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

• Check stop signal to prevent infinite read loop after Close · New

What changed in this PR

Adds bounded HTTP response read-ahead and chunk coalescing across five SDKs while improving cancellation and connection-loss handling.

Changes:

• Coalesces response fragments with one outstanding data RPC.
• Adds prompt source cancellation and binary-safe Go forwarding.
• Adds protocol tests and Java JSON-RPC close handling.

File	Description
python/​test_copilot_request_handler_response.py	Tests response forwarding and cancellation.
python/​copilot/​copilot_request_handler.py	Adds bounded response read-ahead.
python/​copilot/​client.py	Cancels pending exchanges on disconnect.
nodejs/​tsconfig.test.json	Includes the new protocol tests.
nodejs/​test/​copilot-request-handler.test.ts	Tests coalescing and cancellation.
nodejs/​src/​copilotRequestHandler.ts	Adds response buffering and source cancellation.
nodejs/​src/​client.ts	Cancels pending responses on disconnect.
java/​sdk/​src/​test/​java/​com/​github/​copilot/​JsonRpcClientTest.java	Tests invocation failure after closure.
java/​sdk/​src/​test/​java/​com/​github/​copilot/​HttpResponseForwardingTest.java	Tests Java response forwarding.
java/​sdk/​src/​main/​java/​com/​github/​copilot/​LlmInferenceExchange.java	Adds asynchronous response writes.
java/​sdk/​src/​main/​java/​com/​github/​copilot/​LlmInferenceAdapter.java	Cancels pending exchanges.
java/​sdk/​src/​main/​java/​com/​github/​copilot/​JsonRpcClient.java	Fails pending requests on closure.
java/​sdk/​src/​main/​java/​com/​github/​copilot/​HttpResponseReader.java	Implements bounded background reading.
java/​sdk/​src/​main/​java/​com/​github/​copilot/​CopilotRequestHandler.java	Integrates asynchronous read-ahead.
java/​sdk/​src/​main/​java/​com/​github/​copilot/​CopilotClient.java	Wires disconnect cancellation.
go/​http_response_reader.go	Implements bounded response buffering.
go/​http_response_forwarding_test.go	Tests Go protocol behavior.
go/​copilot_request_handler.go	Adds binary forwarding and cancellation.
go/​client.go	Tracks and closes request adapters.
dotnet/​test/​Unit/​CopilotRequestHandlerProtocolTests.cs	Tests .NET response forwarding.
dotnet/​src/​CopilotRequestHandler.cs	Adds bounded asynchronous buffering.
dotnet/​src/​Client.cs	Cancels pending exchanges on disconnect.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

Cross-SDK Consistency Review — PR #2734

This PR ("Coalesce intercepted HTTP response chunks" + follow-up fixes) touches Node.js, Python, Go, .NET, and Java, adding two related capabilities to the llmInference request-handler seam:

1. Bounded 32 KiB HTTP response read-ahead (BoundedHttpResponseReader / httpResponseReader / HttpResponseReader) so response chunks are pulled from the upstream ahead of the RPC acknowledgement, bounded to one buffered chunk.
2. Cancel-pending-on-teardown: stop()/force_stop()/connection-close now proactively cancel any in-flight llmInference exchanges (CancelPendingExternalTools() → LlmInferenceAdapter.CancelPending() in .NET, closeCopilotRequestAdapter()/adapter.close() in Go, cancelPending() in Java/Node.js, cancel_pending() in Python), instead of leaving them to hang or error out only via the transport tearing down underneath them.

Finding: Rust SDK not updated for capability #2

Feature parity gap. The Rust SDK (rust/src/copilot_request_handler.rs) already has an equivalent bounded read-ahead reader (HttpResponseReader, 32 KiB CHUNK_SIZE) predating this PR, so capability #1 is already at parity — no action needed there.

However, capability #2 (proactively cancelling pending llmInference exchanges on Client::stop() / Client::force_stop() / connection close) does not appear to exist in Rust:

• CopilotRequestDispatcher::pending (rust/src/copilot_request_handler.rs) is only ever pruned per-exchange via remove_pending() when a request finishes normally; there's no cancel_pending()/close() equivalent invoked from Client::stop() (rust/src/lib.rs:2824) or Client::force_stop() (rust/src/lib.rs:2960), unlike the changes made to dotnet/src/Client.cs, go/client.go, java/.../CopilotClient.java, nodejs/src/client.ts, and python/copilot/client.py in this PR.
• JsonRpcClient::force_close() (rust/src/jsonrpc.rs:346) clears outgoing pending_requests, but there's no analogous step that cancels the inbound llmInference exchanges tracked by CopilotRequestDispatcher, so a handler awaiting httpRequestChunk/httpResponseStart acks could keep running (or eventually surface a generic connection error) instead of observing a deterministic cancellation the way the other five SDKs now guarantee.

Suggestion: Consider adding a CopilotRequestDispatcher::cancel_pending() (or similar) that cancels each CopilotRequestExchange's CancellationToken and clears pending, invoked from Client::stop()/Client::force_stop() (and wherever the JSON-RPC connection-closed path is handled), mirroring the semantics added to the other five SDKs in this PR. This is a suggestion for follow-up parity work, not a blocker for merging the current PR.

Everything else: consistent ✅

• API naming follows each language's conventions consistently (CancelPending/cancel_pending/cancelPending).
• The read-ahead bound (32 KiB) and cancellation/backpressure semantics are implemented equivalently across Node.js, Python, Go, .NET, and Java, each with dedicated protocol-level test coverage for backpressure, cancellation, connection loss, and upstream errors.
• No other language-specific behavior differences were observed that would need to be mirrored elsewhere.

Generated by SDK Consistency Review Agent for #2734 · copilot · sonnet50 · 297.3 AIC · ⌖ 10.8 AIC · ⊞ 7.8K · ◷