2 files changed, 52 insertions(+), 15 deletions(-)

M mcphttp/cache.go
M mcphttp/cache_test.go

@@ 83,11 83,17 @@ func (w *cacheWriter) commit() {
 	w.Header().Set("Vary", vary)
 }
 
-// Unwrap is load-bearing, not boilerplate: it is what keeps the streamable
-// transport working through this wrapper. http.NewResponseController follows
-// Unwrap to reach the real writer's Flush, and an SSE stream that could not be
-// flushed would be a response no client sees until the handler returns — which,
-// for a stream, is a response nobody sees at all. Deleting this method breaks
-// streaming while leaving every status code and header test green, so
-// TestUnwrapReachesTheUnderlyingFlusher exists to fail instead.
+// Unwrap is what http.NewResponseController follows to reach the real writer.
+//
+// It is not what keeps flushing working — this type has its own Flush, and the
+// controller prefers a method on the writer it is handed over one reached by
+// unwrapping, so the flush path never gets here. That is worth saying because
+// the comment here used to claim otherwise, and the test named after the claim
+// passed with the method deleted.
+//
+// What does need it is everything else the controller offers: SetWriteDeadline,
+// SetReadDeadline, Hijack. A long-lived MCP stream is precisely the response
+// that wants its write deadline pushed out, and without this method that call
+// is ErrNotSupported. TestUnwrapReachesTheWriterBelow measures that against a
+// real server, because a recorder supports no deadlines either way.
 func (w *cacheWriter) Unwrap() http.ResponseWriter { return w.ResponseWriter }

@@ 2,6 2,7 @@ package mcphttp_test
 
 import (
 	"bufio"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"testing"


@@ 87,15 88,16 @@ func TestOnlyTheFirstCommitWrites(t *testing.T) {
 	assert.Equal(t, "ab", rec.Body.String())
 }
 
-// TestUnwrapReachesTheUnderlyingFlusher is the test the Unwrap method exists
-// for.
+// TestFlushOnTheWrapperStreams covers the flush path, which cacheWriter answers
+// itself rather than delegating.
 //
-// cacheWriter embeds the http.ResponseWriter *interface*, so it promotes no
-// Flush of its own; http.NewResponseController can only reach the real writer's
-// through Unwrap. Delete the method and this fails with ErrNotSupported while
-// every status-code and header test above stays green — which is precisely why
-// it is written as a test and not as a comment.
-func TestUnwrapReachesTheUnderlyingFlusher(t *testing.T) {
+// It is deliberately NOT named for Unwrap. An earlier version of this test was
+// named that and claimed to be what the Unwrap method exists for — and it passed
+// with Unwrap deleted, because cacheWriter has a Flush method and
+// http.NewResponseController prefers a method on the writer it is handed over
+// one reached by unwrapping. It never got that far. See
+// TestUnwrapReachesTheWriterBelow for the property that does need Unwrap.
+func TestFlushOnTheWrapperStreams(t *testing.T) {
 	var flushErr error
 	h := mcphttp.PrivateCache(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		_, _ = w.Write([]byte("event: message\n"))


@@ 110,6 112,35 @@ func TestUnwrapReachesTheUnderlyingFlusher(t *testing.T) {
 	assert.Equal(t, wantCacheControl, rec.Header().Get("Cache-Control"))
 }
 
+// TestUnwrapReachesTheWriterBelow is the test the Unwrap method actually earns.
+//
+// cacheWriter embeds the http.ResponseWriter *interface*, so it promotes nothing
+// but the four methods that interface declares. Flush it answers itself; every
+// other thing http.ResponseController offers — the deadlines, Hijack — can only
+// be reached by unwrapping. A long-lived MCP stream is exactly the response that
+// wants a write deadline pushed out, so this is not a hypothetical.
+//
+// It has to run against a real server: httptest.ResponseRecorder supports no
+// deadlines at all, so a recorder would report ErrNotSupported whether Unwrap
+// were there or not — the same vacuity the old test had.
+func TestUnwrapReachesTheWriterBelow(t *testing.T) {
+	var deadlineErr error
+	srv := httptest.NewServer(mcphttp.PrivateCache(http.HandlerFunc(
+		func(w http.ResponseWriter, _ *http.Request) {
+			deadlineErr = http.NewResponseController(w).SetWriteDeadline(time.Now().Add(time.Minute))
+			_, _ = w.Write([]byte("ok"))
+		})))
+	t.Cleanup(srv.Close)
+
+	resp, err := (&http.Client{Timeout: 5 * time.Second}).Get(srv.URL)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = resp.Body.Close() })
+	_, _ = io.Copy(io.Discard, resp.Body)
+
+	require.NoError(t, deadlineErr,
+		"without Unwrap the controller cannot reach the real writer and this is ErrNotSupported")
+}
+
 // TestAStreamReachesTheClientBeforeTheHandlerReturns is the same property
 // measured rather than asserted: over a real connection, with a real client,
 // bytes written and flushed inside the handler have to arrive while the handler