Refactor MCP SSE response boundaries#59
Merged
Merged
Conversation
897371d to
f8e7f23
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This refactors Atryum's upstream MCP HTTP handling so raw transport, JSON-RPC message extraction, and downstream response negotiation are separate responsibilities.
The change keeps the narrow SSE decoding fix, but addresses the architectural issue behind the review feedback: preserving upstream SSE is correct at the transport layer, but Atryum still needs to parse MCP messages correctly and choose a downstream response shape that the calling harness can understand.
Previous Shape: Conflated Responsibilities
Previously
doHTTPEnvelopemixed transport and JSON-RPC parsing behavior:That made some current paths work, but hid two contracts inside one helper:
It also meant a future change that preserved raw SSE at transport level could accidentally leak raw SSE to downstream clients that only asked for JSON.
New Shape: Explicit Boundaries
This PR splits those responsibilities:
Behavior Changes
ForwardResultnow preserves upstream SSE body and content type as raw transport data.initialize,tools/list, andtools/calldecode JSON-RPC through one MCP response decoder instead of directly unmarshallingresult.Body.data:line is the response:data:fieldsAccept: text/event-streamgets upstream SSE preservedapplication/jsonWhat This Is Not
This is not full upstream streaming support. Atryum still consumes one matching JSON-RPC response for current request/response operations. The new structure is meant to make full streaming a cleaner follow-up by avoiding transport-layer lies and giving JSON-RPC extraction a real boundary.
Validation
go test ./..../integrations/scripts/agent_harness_integration_tests.sh run --harness fake-agent --auth no-auth --target calculator./integrations/scripts/agent_harness_integration_tests.sh run --harness fake-agent --auth no-auth --target calc-mcp