Skip to content

Commit 194c450

Browse files
committed
fix(authbridge): Stream text/event-stream responses frame-by-frame
Fixes #477. The forward proxy fully buffered outbound response bodies via io.ReadAll whenever any plugin in the outbound pipeline declared ReadsBody -- which today is every realistic pipeline (mcp-parser, inference-parser, ibac all set ReadsBody=true). For MCP servers using Streamable HTTP, tools/call responses arrive as text/event-stream that may stay open for the full duration of slow tool execution; buffering converted those streams into blocking Client.Do reads bounded by the hard-coded 30s http.Client.Timeout, producing 502s or apparent hangs. The reverse proxy had the same buffering for inbound A2A message/stream responses. This stream-aware path detects text/event-stream at the response header, then forwards SSE frames to the client and dispatches each JSON-RPC message to plugins as it arrives. application/json keeps the buffered path. Per-frame memory is capped at 1 MiB; total stream length is unbounded. - Add an optional pipeline.StreamingResponder interface with OnResponseFrame(ctx, pctx, frame, last). Plumbed via pipeline.Pipeline.RunResponseFrame and pipeline.Holder. - Drop the outbound http.Client.Timeout (which covered the body read and broke streaming); replace with Transport.ResponseHeaderTimeout (30s, time-to-headers ceiling) and a per-read idle deadline on streaming bodies. - forwardproxy: branch on Content-Type per response. SSE streams with HasStreamingResponders take a flush-per-frame path that invokes OnResponseFrame for each event and a final last=true call; RunResponse is not called on this path. application/json and any other Content-Type keep the existing buffered path, plus a single last=true OnResponseFrame call so streaming-aware plugins use one code path for both shapes. WritesBody=true forces buffered fallback with a warning (a body mutator can't rewrite a body already on the wire). - reverseproxy: mirror the same branch in modifyResponse via a streaming response body that pulls one SSE frame per Read, dispatches it to OnResponseFrame, and re-emits the SSE event; ReverseProxy.FlushInterval=-1 ferries each frame to the client immediately. Same WritesBody fallback contract. - Convert the aggregating parsers to fold-and-finalize: - mcp-parser: per-message recording (one JSON-RPC result per frame); OnResponse stays as a fallback for non-streaming-aware listeners. - inference-parser: accumulates content deltas + usage on a private SetState scratch across frames; finalizes on last=true. - a2a-parser: folds artifact text and final-status across message/stream events; finalizes on last=true. New extractStreamEvent helper shared by the buffered and streaming paths so one envelope shape is parsed in one place. - New authlib/listener/internal/sseframe package: a narrow SSE reader emitting one ([]byte, error) per data-bearing event. Per-frame size cap (default 1 MiB), CRLF tolerance, multi-line-data folding, comment lines silently skipped, trailing unterminated event delivered before EOF. - New tests cover: the SSE reader (multi-frame, CRLF, oversize, long lines, only-comments, multi-line data); RunResponseFrame dispatch ordering, off-policy skip, reject-stops-chain, and cancel; per-plugin OnResponseFrame for all three parsers (per-message MCP, fold-and-finalize inference + A2A, application/json one-shot, no-extension no-op, empty-stream skip pairing); end-to-end streaming through the forward and reverse proxies (frames arrive before upstream closes -- the regression check; WritesBody buffered fallback; application/json single last=true frame). The OnResponseFrame hook leaves room for per-message response-side enforcement later. Today's plugins are observability-only on the response side (IBAC's OnResponse is a no-op; it gates at request time), so the listener forwards-then-dispatches each frame; an inspect-before-forward variant would be a clean follow-up. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Kelly Abuelsaad <kaymar@gmail.com>
1 parent a7ebd87 commit 194c450

16 files changed

Lines changed: 2481 additions & 113 deletions

File tree

authbridge/authlib/listener/forwardproxy/server.go

Lines changed: 336 additions & 23 deletions
Large diffs are not rendered by default.
Lines changed: 321 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,321 @@
1+
package forwardproxy
2+
3+
import (
4+
"bufio"
5+
"bytes"
6+
"context"
7+
"fmt"
8+
"io"
9+
"net/http"
10+
"net/http/httptest"
11+
"strings"
12+
"sync"
13+
"testing"
14+
"time"
15+
16+
"github.com/kagenti/kagenti-extensions/authbridge/authlib/pipeline"
17+
)
18+
19+
// streamingProbe is a minimal Plugin + StreamingResponder used to
20+
// observe how the proxy dispatches frames. It records every frame
21+
// it sees along with the last flag, so tests can assert on order
22+
// and finalization. ReadsBody=true so the listener takes the
23+
// NeedsBody path on requests.
24+
type streamingProbe struct {
25+
mu sync.Mutex
26+
frames [][]byte
27+
lasts []bool
28+
caps pipeline.PluginCapabilities
29+
}
30+
31+
func newStreamingProbe(writesBody bool) *streamingProbe {
32+
return &streamingProbe{
33+
caps: pipeline.PluginCapabilities{
34+
ReadsBody: true,
35+
WritesBody: writesBody,
36+
},
37+
}
38+
}
39+
40+
func (p *streamingProbe) Name() string { return "streaming-probe" }
41+
func (p *streamingProbe) Capabilities() pipeline.PluginCapabilities { return p.caps }
42+
func (p *streamingProbe) OnRequest(_ context.Context, _ *pipeline.Context) pipeline.Action {
43+
return pipeline.Action{Type: pipeline.Continue}
44+
}
45+
func (p *streamingProbe) OnResponse(_ context.Context, _ *pipeline.Context) pipeline.Action {
46+
return pipeline.Action{Type: pipeline.Continue}
47+
}
48+
func (p *streamingProbe) OnResponseFrame(_ context.Context, _ *pipeline.Context, frame []byte, last bool) pipeline.Action {
49+
p.mu.Lock()
50+
defer p.mu.Unlock()
51+
cp := make([]byte, len(frame))
52+
copy(cp, frame)
53+
p.frames = append(p.frames, cp)
54+
p.lasts = append(p.lasts, last)
55+
return pipeline.Action{Type: pipeline.Continue}
56+
}
57+
58+
func (p *streamingProbe) snapshot() ([][]byte, []bool) {
59+
p.mu.Lock()
60+
defer p.mu.Unlock()
61+
frames := make([][]byte, len(p.frames))
62+
copy(frames, p.frames)
63+
lasts := make([]bool, len(p.lasts))
64+
copy(lasts, p.lasts)
65+
return frames, lasts
66+
}
67+
68+
// TestForwardProxy_Streaming_FramesFlowThrough asserts that an upstream
69+
// emitting SSE frames over time has those frames flushed downstream as
70+
// they arrive — no full-body buffering, no 30s timeout firing, and
71+
// each frame reaches the StreamingResponder plugin with last=false.
72+
// A final last=true call is made at end-of-stream.
73+
func TestForwardProxy_Streaming_FramesFlowThrough(t *testing.T) {
74+
// Upstream sends 3 SSE frames with small idle gaps. We use real
75+
// timers (not mock clocks) because the proxy reads off net/http
76+
// internals; the gaps are tens of milliseconds, well under any
77+
// timeout, and far longer than the per-frame work.
78+
upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
79+
w.Header().Set("Content-Type", "text/event-stream")
80+
w.WriteHeader(http.StatusOK)
81+
flusher := w.(http.Flusher)
82+
for i := 1; i <= 3; i++ {
83+
fmt.Fprintf(w, "data: {\"id\":%d}\n\n", i)
84+
flusher.Flush()
85+
time.Sleep(20 * time.Millisecond)
86+
}
87+
}))
88+
defer upstream.Close()
89+
90+
probe := newStreamingProbe(false)
91+
pipe, err := pipeline.New([]pipeline.Plugin{probe})
92+
if err != nil {
93+
t.Fatalf("New pipeline: %v", err)
94+
}
95+
srv, err := NewServer(pipeline.NewHolder(pipe), nil, nil)
96+
if err != nil {
97+
t.Fatalf("NewServer: %v", err)
98+
}
99+
proxy := httptest.NewServer(srv.Handler())
100+
defer proxy.Close()
101+
102+
proxyClient := &http.Client{
103+
Transport: &http.Transport{
104+
Proxy: http.ProxyURL(mustParseURL(proxy.URL)),
105+
},
106+
}
107+
req, _ := http.NewRequest("GET", upstream.URL+"/stream", nil)
108+
resp, err := proxyClient.Do(req)
109+
if err != nil {
110+
t.Fatalf("Do: %v", err)
111+
}
112+
defer resp.Body.Close()
113+
114+
if resp.StatusCode != http.StatusOK {
115+
t.Fatalf("status = %d", resp.StatusCode)
116+
}
117+
if got := resp.Header.Get("Content-Type"); !strings.HasPrefix(got, "text/event-stream") {
118+
t.Errorf("Content-Type = %q, want text/event-stream", got)
119+
}
120+
121+
body, err := io.ReadAll(resp.Body)
122+
if err != nil {
123+
t.Fatalf("ReadAll: %v", err)
124+
}
125+
// Body should contain 3 SSE events.
126+
if got := bytes.Count(body, []byte("data:")); got != 3 {
127+
t.Errorf("body has %d data: lines, want 3 — body=%q", got, body)
128+
}
129+
130+
frames, lasts := probe.snapshot()
131+
// We expect 3 frames + 1 last=true call.
132+
if len(frames) != 4 {
133+
t.Fatalf("plugin saw %d calls, want 4 (3 frames + 1 final) — frames=%v lasts=%v", len(frames), framesAsStrings(frames), lasts)
134+
}
135+
for i := 0; i < 3; i++ {
136+
if lasts[i] {
137+
t.Errorf("frame %d last=true, want false", i)
138+
}
139+
if !bytes.Contains(frames[i], []byte(fmt.Sprintf(`"id":%d`, i+1))) {
140+
t.Errorf("frame %d = %q, missing id %d", i, frames[i], i+1)
141+
}
142+
}
143+
if !lasts[3] {
144+
t.Error("final call last=false, want true")
145+
}
146+
}
147+
148+
// TestForwardProxy_Streaming_WritesBodyFallsBackToBuffered asserts the
149+
// safety guard: a pipeline with a WritesBody plugin can't take the
150+
// streaming path (the plugin can't rewrite a body we've already
151+
// started forwarding). The proxy logs a warning and falls back to
152+
// buffered, so the response is delivered correctly even though it
153+
// loses the streaming property.
154+
func TestForwardProxy_Streaming_WritesBodyFallsBackToBuffered(t *testing.T) {
155+
upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
156+
w.Header().Set("Content-Type", "text/event-stream")
157+
w.WriteHeader(http.StatusOK)
158+
flusher := w.(http.Flusher)
159+
fmt.Fprintf(w, "data: {\"id\":1}\n\n")
160+
flusher.Flush()
161+
}))
162+
defer upstream.Close()
163+
164+
probe := newStreamingProbe(true) // WritesBody=true → buffered fallback
165+
pipe, err := pipeline.New([]pipeline.Plugin{probe})
166+
if err != nil {
167+
t.Fatalf("New pipeline: %v", err)
168+
}
169+
srv, err := NewServer(pipeline.NewHolder(pipe), nil, nil)
170+
if err != nil {
171+
t.Fatalf("NewServer: %v", err)
172+
}
173+
proxy := httptest.NewServer(srv.Handler())
174+
defer proxy.Close()
175+
176+
proxyClient := &http.Client{
177+
Transport: &http.Transport{
178+
Proxy: http.ProxyURL(mustParseURL(proxy.URL)),
179+
},
180+
}
181+
req, _ := http.NewRequest("GET", upstream.URL+"/stream", nil)
182+
resp, err := proxyClient.Do(req)
183+
if err != nil {
184+
t.Fatalf("Do: %v", err)
185+
}
186+
defer resp.Body.Close()
187+
body, _ := io.ReadAll(resp.Body)
188+
if !bytes.Contains(body, []byte(`{"id":1}`)) {
189+
t.Errorf("body did not contain expected payload: %q", body)
190+
}
191+
// Buffered path: streaming-aware plugins still see one last=true
192+
// frame carrying the whole body. Sanity-check.
193+
_, lasts := probe.snapshot()
194+
if len(lasts) == 0 || !lasts[len(lasts)-1] {
195+
t.Errorf("last call lasts = %v; expected final last=true on buffered fallback", lasts)
196+
}
197+
}
198+
199+
// TestForwardProxy_BufferedDeliversLastTrueFrame asserts the
200+
// application/json one-shot contract: streaming-aware plugins receive
201+
// the buffered body as a single OnResponseFrame call with last=true,
202+
// so the same code path handles streaming and non-streaming responses.
203+
func TestForwardProxy_BufferedDeliversLastTrueFrame(t *testing.T) {
204+
upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
205+
w.Header().Set("Content-Type", "application/json")
206+
w.WriteHeader(http.StatusOK)
207+
w.Write([]byte(`{"ok":true}`))
208+
}))
209+
defer upstream.Close()
210+
211+
probe := newStreamingProbe(false)
212+
pipe, err := pipeline.New([]pipeline.Plugin{probe})
213+
if err != nil {
214+
t.Fatalf("New pipeline: %v", err)
215+
}
216+
srv, err := NewServer(pipeline.NewHolder(pipe), nil, nil)
217+
if err != nil {
218+
t.Fatalf("NewServer: %v", err)
219+
}
220+
proxy := httptest.NewServer(srv.Handler())
221+
defer proxy.Close()
222+
223+
proxyClient := &http.Client{
224+
Transport: &http.Transport{
225+
Proxy: http.ProxyURL(mustParseURL(proxy.URL)),
226+
},
227+
}
228+
req, _ := http.NewRequest("GET", upstream.URL+"/oneshot", nil)
229+
resp, err := proxyClient.Do(req)
230+
if err != nil {
231+
t.Fatalf("Do: %v", err)
232+
}
233+
defer resp.Body.Close()
234+
io.Copy(io.Discard, resp.Body)
235+
236+
frames, lasts := probe.snapshot()
237+
if len(frames) != 1 || !lasts[0] {
238+
t.Fatalf("frames=%v lasts=%v; want exactly one call with last=true", framesAsStrings(frames), lasts)
239+
}
240+
if !bytes.Equal(frames[0], []byte(`{"ok":true}`)) {
241+
t.Errorf("frame[0] = %q, want JSON body", frames[0])
242+
}
243+
}
244+
245+
// TestForwardProxy_Streaming_HeadersAndBodyArriveBeforeUpstreamCloses
246+
// is the regression test for #477 — the agent should see frames
247+
// arriving as the upstream produces them, not after the upstream
248+
// finishes. We assert that by reading the first frame from the
249+
// proxy's response BEFORE the upstream writes the second frame, with
250+
// the upstream gated on a channel.
251+
func TestForwardProxy_Streaming_HeadersAndBodyArriveBeforeUpstreamCloses(t *testing.T) {
252+
gate := make(chan struct{})
253+
upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
254+
w.Header().Set("Content-Type", "text/event-stream")
255+
w.WriteHeader(http.StatusOK)
256+
flusher := w.(http.Flusher)
257+
fmt.Fprintf(w, "data: {\"id\":1}\n\n")
258+
flusher.Flush()
259+
<-gate // hold the upstream open until the test releases it
260+
fmt.Fprintf(w, "data: {\"id\":2}\n\n")
261+
flusher.Flush()
262+
}))
263+
defer upstream.Close()
264+
defer close(gate)
265+
266+
probe := newStreamingProbe(false)
267+
pipe, err := pipeline.New([]pipeline.Plugin{probe})
268+
if err != nil {
269+
t.Fatalf("New pipeline: %v", err)
270+
}
271+
srv, err := NewServer(pipeline.NewHolder(pipe), nil, nil)
272+
if err != nil {
273+
t.Fatalf("NewServer: %v", err)
274+
}
275+
proxy := httptest.NewServer(srv.Handler())
276+
defer proxy.Close()
277+
278+
proxyClient := &http.Client{
279+
Transport: &http.Transport{
280+
Proxy: http.ProxyURL(mustParseURL(proxy.URL)),
281+
},
282+
}
283+
req, _ := http.NewRequest("GET", upstream.URL+"/stream", nil)
284+
resp, err := proxyClient.Do(req)
285+
if err != nil {
286+
t.Fatalf("Do: %v", err)
287+
}
288+
defer resp.Body.Close()
289+
290+
// Read the first SSE event — must arrive even though upstream is
291+
// still holding open. If buffering had been left in place, this
292+
// read would block until upstream closes (and a 30s timeout would
293+
// likely fire — that was the bug).
294+
br := bufio.NewReader(resp.Body)
295+
deadline := time.Now().Add(2 * time.Second)
296+
var firstFrame []byte
297+
for time.Now().Before(deadline) {
298+
line, err := br.ReadString('\n')
299+
if err != nil {
300+
t.Fatalf("ReadString: %v", err)
301+
}
302+
if bytes.HasPrefix([]byte(line), []byte("data:")) {
303+
firstFrame = []byte(line)
304+
break
305+
}
306+
}
307+
if firstFrame == nil {
308+
t.Fatal("first frame did not arrive within 2s — proxy likely buffered")
309+
}
310+
if !bytes.Contains(firstFrame, []byte(`"id":1`)) {
311+
t.Errorf("first frame = %q, missing id 1", firstFrame)
312+
}
313+
}
314+
315+
func framesAsStrings(frames [][]byte) []string {
316+
out := make([]string, len(frames))
317+
for i, f := range frames {
318+
out[i] = string(f)
319+
}
320+
return out
321+
}

0 commit comments

Comments
 (0)