live SSE relay for tools/call#151
Draft
kam-validmind wants to merge 15 commits into
Draft
Conversation
Move transport, timeout, parsing, relay, and audit logic into focused files so each path can be understood and tested independently.
Bound upstream message memory, add reconnect backoff, and keep standalone progress streams aligned with MCP session renewal. Retry transient standalone failures and expose buffer drops to stream observers.
Replace per-invocation audit goroutines with an ordered shared dispatcher, classify quiet downstream cancellation, record terminal delivery separately, and avoid claiming success before durable persistence.
Correctness: - guard the stdio stderr boundedBuffer with a mutex: os/exec's copy goroutine can still be writing when error paths read Len/String before cmd.Wait runs - flag server-to-client requests routed via the standalone stream as ServerRequest, matching the POST-stream classification, so audit doesn't mislabel sampling/elicitation requests as notifications - normalize CR/CRLF in downstream SSE framing: a raw CR inside upstream JSON would split the data frame for a compliant parser Safety and audit truthfulness: - substitute derived stream bounds when SetStreamOptions was never called, so an embedder wiring NewService directly never runs an unbounded relay - reset the idle timeout on SSE keepalive/comment lines so a busy-but- quiet upstream heartbeating through a long tool run isn't cut off - send a static message in the terminal SSE error frame; internal error text (SQL/driver detail) stays in the server log - audit a bare request-context cancel as stream_canceled rather than stream_aborted_downstream: shutdown and a quiet disconnect are indistinguishable without a failed downstream write - rename the failure audit's events_relayed to events_total to match stream_completed; cap the session-expired body drain at 64KB Readability: - extract callTimeoutGuard.timeoutErr, drainTrailingProgress, and a shared toolCallEnvelopeID; move the header-timeout fallback into config.EffectiveStreamHeaderTimeoutSeconds; document the stream_completed durability-before-delivery latency bound Tests: - fix unsynchronized test-server counters (latent -race flakes) - pin drainTrailingProgress settle-window semantics deterministically - add end-to-end MaxDuration, resume-failure, StreamStats delivery, and keepalive-liveness coverage; poll for pending_approval instead of sleeping in the approval-gate test
- run buffered finalization writes on a bounded detached context so a downstream disconnect can't leave the row stuck executing (matches the streaming path; regression test added) - emit the mcp.tools.call trace event on the error-after-stream-started path (finalize_failed: true) - defer sseRelaySink.stopHeartbeat in the handler so the heartbeat goroutine can't outlive a panicking handler; count delivered (not attempted) frames in eventCount - shard index modulo in uint64; queue capacity floored at 1; wrap both persistence errors consistently - shared test upstream helpers fail cleanly (t.Errorf + HTTP error) instead of hanging on t.Fatalf from a handler goroutine - document relay behavior guarantees and the buffered fix in the changelog; note SetStreamRelayEnabled is startup-wiring only
acquireStandaloneStreamWithLimit only spawns the GET-connect goroutine and returns immediately; the caller then sent tools/call right away. If the upstream's first progress notification fired before that GET reached the server, it had nowhere to land and was silently dropped — reproduced as 2/3 progress notifications instead of 3 in TestMCPToolsCallAgainstRealStandaloneStreamServer. Add a ready signal closed after the stream's first connect attempt (success or failure), and wait on it before sending the dependent tools/call, bounded by HeaderTimeout so a hung upstream can't stall the call indefinitely.
The integration suite had zero coverage of the SSE relay: no streaming MCP
target, no timing-aware verification, and fake_agent.py never requested
SSE. Adds an everything-streaming target (the official reference
"everything" server over stdio) and a fake_agent.py --stream mode that
reads the tools/call response incrementally, timestamps each frame, and
fails unless progress measurably arrives before the terminal result --
proving live delivery over a real process/network boundary, distinct from
the existing Go E2E tests (mcp-everything-test, mcp-standalone-stream-test),
which prove the same for the upstream-facing HTTP relay internals against
real SDKs. Sharing the server-everything dependency with mcp-everything-test
is an accepted, minor redundancy in exchange for not maintaining a second
hand-rolled MCP fixture.
Also fixes two pre-existing bugs this surfaced, both blocking every case on
stock macOS (bash 3.2 + BSD awk), not just the new target:
render_atryum_config piping a multi-line TOML block through `awk -v`
(swapped for a small Python substitution), and five call sites expanding
empty bash arrays under `set -u` (fixed with the portable
"${arr[@]+"${arr[@]}"}" idiom).
"Upstream"/"downstream" were used throughout the doc (Runtime ingress, Component boundaries, the SSE relay section) but only ever defined ~200 lines in, under a feature-specific subsection. The System context diagram also had two different nodes both labeled "MCP client" — the downstream agent's connection and Atryum's own upstream-facing client — one of them literally named "Upstream", colliding with what "upstream" means everywhere else in the doc (the external tool server, the diagram's separate "Tools" node).
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.
What and why?
The problem: when an agent calls a slow tool through Atryum, Atryum used to wait for the tool to completely finish, then send back one final answer. If the tool took 30 seconds, the agent saw and heard nothing for 30 seconds, then got the result all at once.
The fix: Atryum can now forward "still working, X% done" style updates to the agent while the tool is still running, instead of making it wait for one final answer. This only happens if the agent says it can handle that (
Accept: text/event-stream) and the upstream tool server supports it — otherwise nothing changes, and the agent gets the same single response as before.Before vs. after:
sequenceDiagram autonumber participant Agent participant Atryum participant Tool as Upstream tool server rect rgb(245, 235, 235) Note over Agent,Tool: Before this PR Agent->>Atryum: "run this tool" Atryum->>Tool: "run this tool" Note over Atryum,Tool: (silence for the whole run, could be minutes) Tool-->>Atryum: final result Atryum-->>Agent: final result endsequenceDiagram autonumber participant Agent participant Atryum participant Tool as Upstream tool server rect rgb(230, 245, 235) Note over Agent,Tool: After this PR (only if Agent asks for it) Agent->>Atryum: "run this tool, I can read a live stream" Atryum->>Tool: "run this tool" loop while the tool keeps working Tool-->>Atryum: "33% done…" Atryum-->>Agent: "33% done…" end Tool-->>Atryum: final result Atryum-->>Agent: final result endA quick glossary, since this touches a protocol most of the codebase doesn't deal with directly:
Where the progress updates can come from. Most upstream servers put progress messages on the same HTTP response as the tool call itself ("Path A" below). A few SDKs (notably the reference Python MCP SDK) instead send progress on a second, separate connection shared by every in-flight call ("Path B"). Atryum listens on both, so either kind of upstream server works:
flowchart TB Downstream[Agent] Relay[Atryum] Upstream[Upstream tool server] CallPath["Path A: the response to this one tool call<br/>carries progress AND the final answer"] StandalonePath["Path B: one shared connection<br/>carries progress ONLY, for all active calls"] Downstream <-->|"tool call"| Relay Relay --- CallPath CallPath --- Upstream Relay --- StandalonePath StandalonePath --- UpstreamBecause Path B is shared across every call currently in flight, Atryum has to make sure one caller's progress updates never leak to a different caller. It does this by swapping in a unique progress token for each call before forwarding it upstream, then swapping the caller's original token back in before relaying the update — so two agents that happen to pick the same token number can never see each other's progress.
Safety nets included in this PR (see
docs/architecture.md→ "Live SSE relay for tools/call" for the full write-up, andCHANGELOG.mdfor the complete list):stream_relay_enabled = falseturns this whole feature off and restores the old one-response-at-the-end behavior, no rollback needed.invocation_events), same as before — reviewers/admins don't lose visibility just because delivery got more real-time.executingforever in the audit log, even though it had actually finished. That's fixed too.How to test
go test ./...(orjust check) — runs the full new test suite with no network access needed. If you want to look at specific pieces:internal/mcp/http_stream_test.go(Path A),internal/mcp/standalone_stream_test.go(Path B),internal/invocation/stream_execution_test.goandstream_sink_internal_test.go(audit logging),internal/api/sse_relay_test.go(the HTTP-facing relay).just mcp-everything-test— spins up the official@modelcontextprotocol/server-everythingreference server and calls itstrigger-long-running-operationtool through a real Atryum server. Needsnpx.just mcp-standalone-stream-test— spins up a small Python MCP server (internal/api/testdata/mcp_standalone_fixture/server.py) to test the shared-connection (Path B) case specifically. Needsuv.[[mcp_servers]]entry pointing at either server above, call one of its slow tools withAccept: text/event-streamset on the request, and watchinvocation_eventsfill in with multiplestream_eventrows as it runs instead of a single row at the end.What needs special review?
internal/api/sse_relay.go+internal/invocation/stream_sink.go— this is where "don't lose progress, but also don't let a slow audit write block a live update" is implemented. Trickiest part of the PR.internal/mcp/standalone_stream.go— the progress-token swap described above, plus what happens when the upstream session gets renewed mid-flight.stream_canceled/stream_aborted_downstream/stream_timeoutin the audit log — worth checking the reasoning holds up. Background in thefix: address stream relay review findingsandfinalize buffered calls after disconnect; harden relay lifecyclecommit messages.atryum.example.tomlare reasonable for real-world upstreams.Dependencies, breaking changes, and deployment notes
stream_relay_enabledcan disable the new behavior at startup, if something unexpected shows up with a production upstream.npx/uvon the machine running them, but they're excluded from the defaultgo test ./.../ CI run and skip themselves gracefully if those tools are missing.Data impact
Justification: No existing customer data, config format, or API response shape changes. The only new persisted data is additional audit rows (
stream_event,stream_completed,stream_deliveryininvocation_events) for calls that use streaming — purely additive telemetry, nothing existing is modified or removed.Release notes
Atryum can now stream live progress updates from a tool call back to the calling agent as they happen, instead of making the agent wait silently for one final answer. This works whether the upstream tool server sends progress on the tool call's own response or on a separate shared connection (used by some SDKs, including the reference Python MCP SDK). See the
[Unreleased]section ofCHANGELOG.mdfor the new configuration options and full behavior guarantees.Checklist