From 3bd158fbb23241df31897f9ba47afac832ee2e1d Mon Sep 17 00:00:00 2001
From: Eugene Blikh <bigbes@gmail.com>
Date: Sun, 16 Aug 2026 11:14:11 +0300
Subject: [PATCH] mcphttp: pin Flush with a flush that precedes any write
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Deleting the Flush method left every test in this package green. All of
them wrote before flushing, which commits the headers through the Write
hook and leaves the flush path unmeasured — the second vacuous test in
this file, after the Unwrap one.

The case that needs Flush is a handler that flushes first, as an SSE
handler opening a stream does. It has to run against a real server:
httptest.ResponseRecorder's Header() hands back the live map, so a
recorder reports the headers as set whenever they were set, and passes
for exactly this bug. Found by an agent that measured it in a sibling
service's copy of this code.
---
 mcphttp/cache_test.go | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/mcphttp/cache_test.go b/mcphttp/cache_test.go
index 7191cc52b891715b6d70163c2aa8c7f3f9623429..3956cef30172953c381cd8ea24ac61f0493013d7 100644
--- a/mcphttp/cache_test.go
+++ b/mcphttp/cache_test.go
@@ -88,6 +88,44 @@ func TestOnlyTheFirstCommitWrites(t *testing.T) {
 	assert.Equal(t, "ab", rec.Body.String())
 }
 
+// TestAFlushBeforeAnyWriteStillCarriesTheHeaders is what the Flush method earns.
+//
+// A handler that flushes before writing anything commits the response through
+// the flush, not through Write — so the Write hook never runs, and without a
+// Flush of its own the wrapper is bypassed entirely and the response leaves with
+// no Cache-Control and no Vary. Every other test here writes first, which sets
+// the headers on the way in and leaves the flush path unmeasured: deleting Flush
+// kept all of them green.
+//
+// It has to run against a real server. httptest.ResponseRecorder's Header()
+// hands back the live map, so a recorder reports the headers as set no matter
+// when — or whether — the wrapper committed them, and reports a pass for exactly
+// this bug.
+func TestAFlushBeforeAnyWriteStillCarriesTheHeaders(t *testing.T) {
+	release := make(chan struct{})
+	srv := httptest.NewServer(mcphttp.PrivateCache(http.HandlerFunc(
+		func(w http.ResponseWriter, _ *http.Request) {
+			w.Header().Set("Content-Type", "text/event-stream")
+			if err := http.NewResponseController(w).Flush(); err != nil {
+				t.Errorf("flush: %v", err)
+			}
+			<-release
+			_, _ = w.Write([]byte(": keepalive\n\n"))
+		})))
+	t.Cleanup(func() {
+		close(release)
+		srv.Close()
+	})
+
+	resp, err := (&http.Client{Timeout: 5 * time.Second}).Get(srv.URL)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = resp.Body.Close() })
+
+	assert.Equal(t, wantCacheControl, resp.Header.Get("Cache-Control"),
+		"the response committed through Flush, so only Flush could have set this")
+	assert.Equal(t, wantVary, resp.Header.Get("Vary"))
+}
+
 // TestFlushOnTheWrapperStreams covers the flush path, which cacheWriter answers
 // itself rather than delegating.
 //