diff --git a/CLAUDE.md b/CLAUDE.md index 6beff7a..caa61b6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -161,12 +161,11 @@ Conventions that follow from it: lives in `commitScope` where no caller can forget it. - **Request deadlines are extended through `responseController(c)`, never - `http.NewResponseController(c.Writer)`.** The logging middleware wraps the - writer in a type with no `Unwrap`, so a controller built from a handler's - writer can't reach the socket: every `SetWriteDeadline`/`SetReadDeadline` - returns `ErrNotSupported` — in production only. `captureController` is the - first middleware in `api.New` precisely so the controller exists before - anything wraps the writer. Corollary for tests: a deadline test must run + `http.NewResponseController(c.Writer)`.** A controller built in a handler + can't reach the socket — the logging middleware wraps the writer — so every + deadline call silently returns `ErrNotSupported`, in production only; + `internal/api/deadlines.go` has the mechanism and why `captureController` + must stay the first middleware. Corollary for tests: a deadline test must run through `New()`, not `gin.New()` — the #78 fix shipped fully tested on a bare engine and never worked on the live instance. diff --git a/internal/api/agent.go b/internal/api/agent.go index 95cf9c3..e1e8325 100644 --- a/internal/api/agent.go +++ b/internal/api/agent.go @@ -165,10 +165,8 @@ type eventStream struct { // dropped connection. Hence a deadline set up front and refreshed per frame, // rather than anything checked after the fact. // -// The controller comes from responseController, NOT from c.Writer: by the time -// a handler runs, the logging middleware has wrapped the writer in something a -// controller can't unwrap, and every deadline call fails — which is how this -// fix shipped, tested, and stayed broken live (see deadlines.go). +// The controller comes from responseController, not from c.Writer — a +// controller built here can't reach the socket; deadlines.go says why. func openEventStream(c *gin.Context) *eventStream { c.Header("Content-Type", "text/event-stream") c.Header("Cache-Control", "no-cache") diff --git a/internal/api/sse_deadline_test.go b/internal/api/sse_deadline_test.go index f5a476c..b3396c2 100644 --- a/internal/api/sse_deadline_test.go +++ b/internal/api/sse_deadline_test.go @@ -112,18 +112,12 @@ func TestEventStreamRefreshesDeadlinePerFrame(t *testing.T) { // production middleware stack — which is where it was still broken. // // The two tests above passed while the deployed instance cut every agent turn -// at exactly 30s with "The connection dropped partway through." The difference -// is the middleware: the logging middleware in New replaces c.Writer with a -// wrapper that embeds the gin.ResponseWriter INTERFACE, which has no Unwrap. An -// http.ResponseController built from the handler's c.Writer unwraps layer by -// layer looking for SetWriteDeadline, can't see past that wrapper, and returns -// ErrNotSupported — putting the stream back on the server's absolute -// WriteTimeout. The first write past it fails, which cancels the request -// context and closes the socket under the client mid-frame. -// -// So: the same scenario as the first test, hosted on the engine New builds, in -// the order cmd/pansy runs it. Any future middleware that wraps the writer, or a -// reorder that puts one ahead of the controller capture, fails here. +// at exactly 30s: they host openEventStream on a bare engine, and it is the +// logging middleware in New that hides the socket from a ResponseController +// built in a handler (deadlines.go has the mechanism). So: the same scenario as +// the first test, hosted on the engine New builds, in the order cmd/pansy runs +// it. Any future middleware that wraps the writer, or a reorder that puts one +// ahead of the controller capture, fails here. func TestEventStreamOutlivesWriteTimeoutBehindMiddleware(t *testing.T) { got, err := streamFrames(t, authEngine(t, localCfg()), 300*time.Millisecond, 300*time.Millisecond, 3) if err != nil {