From 12ca3b27c9d4aad82da7cd4c4817d6d6552a4c16 Mon Sep 17 00:00:00 2001 From: lex00 <121451605+lex00@users.noreply.github.com> Date: Mon, 3 Aug 2026 15:01:13 -0600 Subject: [PATCH] a plain GET on the exec path is the session list, not a failed upgrade MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The exec path serves two protocols in the real API. sprites-ex's Session.list_by_name/2 is literally Req.get(client.req, url: "/v1/sprites/#{name}/exec") and reads body["sessions"]. spritzer registered that path as a WebSocket handler only, so an unupgraded GET fell through to websocket.Accept and came back 426 with a protocol-violation body. Found from fountain, which asks for the session list before deciding whether to reattach or start fresh. The 426 aborted that decision and orphaned the turn before anything ran — INTENTIUS/spritzer#18. An idle sprite has no sessions and the honest answer is an empty list, which is also the answer that lets a client get on with it. A missing sprite is still a 404: "no sessions" and "no sprite" are different answers and a client reattaching should be able to tell them apart. The upgrade check reads both headers as the lists they are — Connection is commonly "keep-alive, Upgrade" from a proxy, and an equality check would route that to the session list and break exec behind any proxy. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WPvVBaE97cfmvzytEvJCBr --- internal/server/server.go | 2 +- internal/server/server_test.go | 69 +++++++++++++++++++++++++++++++++- internal/server/ws.go | 51 +++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 2 deletions(-) diff --git a/internal/server/server.go b/internal/server/server.go index b77c6ab..69af47c 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -20,7 +20,7 @@ import ( // implementedPaths backs the health/coverage endpoint. var implementedPaths = []string{ "POST /v1/sprites", - "GET /v1/sprites/{id}/exec (control WebSocket)", + "GET /v1/sprites/{id}/exec (control WebSocket; session list when not upgraded)", "POST /v1/sprites/{id}/checkpoint", "GET /v1/sprites/{id}/checkpoints", "GET /v1/sprites/{id}/checkpoints/{cid}", diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 6733cbc..a8098bf 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -178,9 +178,14 @@ func TestHealth(t *testing.T) { // The exec entry names the control WebSocket, and checkpoint create is the // singular path. joined := strings.Join(payload.Implemented, "\n") - if !strings.Contains(joined, "exec (control WebSocket)") { + if !strings.Contains(joined, "exec (control WebSocket") { t.Fatalf("coverage list missing WS exec entry: %v", payload.Implemented) } + // The same path answers a plain GET with the session list, and a client + // reading this list should be able to see that without trying it. + if !strings.Contains(joined, "session list when not upgraded") { + t.Fatalf("coverage list does not advertise the exec session list: %v", payload.Implemented) + } if !strings.Contains(joined, "POST /v1/sprites/{id}/checkpoint\n") && !strings.HasSuffix(joined, "POST /v1/sprites/{id}/checkpoint") { t.Fatalf("coverage list missing singular checkpoint create: %v", payload.Implemented) } @@ -516,3 +521,65 @@ func TestEmptyFSMarshalsAsObject(t *testing.T) { t.Fatalf("empty sprite GET body = %s, want fs:{} and checkpoints:[]", s) } } + +// A plain GET on the exec path is the session list, not a failed upgrade. +// +// sprites-ex asks for it before deciding whether to reattach, so answering 426 +// here stopped every fountain turn before anything ran (#18). +func TestExecPlainGetReturnsSessionList(t *testing.T) { + h := newHarness(t) + if code, body := h.do(http.MethodPost, "/v1/sprites", map[string]any{"name": "demo"}); code != http.StatusCreated { + t.Fatalf("create sprite: %d %s", code, body) + } + + code, body := h.do(http.MethodGet, "/v1/sprites/demo/exec", nil) + if code != http.StatusOK { + t.Fatalf("plain GET on exec: got %d, want 200 (body %q)", code, body) + } + + var payload struct { + Sessions []any `json:"sessions"` + } + h.mustJSON(body, &payload) + if payload.Sessions == nil { + t.Fatal(`sessions must be present and an array, not null: a client reads body["sessions"] directly`) + } + if len(payload.Sessions) != 0 { + t.Fatalf("a sprite that is not mid-exec has no sessions, got %d", len(payload.Sessions)) + } +} + +// "no sessions" and "no sprite" are different answers. +func TestExecPlainGetOnMissingSpriteIs404(t *testing.T) { + h := newHarness(t) + if code, body := h.do(http.MethodGet, "/v1/sprites/nope/exec", nil); code != http.StatusNotFound { + t.Fatalf("exec session list on a missing sprite: got %d, want 404 (body %q)", code, body) + } +} + +// A Connection header of "keep-alive, Upgrade" is still an upgrade — proxies +// routinely send the list form, and an equality check would route it to the +// session list and break exec behind any proxy. +func TestConnectionHeaderListIsAnUpgrade(t *testing.T) { + for _, tc := range []struct { + conn, upgrade string + want bool + }{ + {"Upgrade", "websocket", true}, + {"keep-alive, Upgrade", "websocket", true}, + {"upgrade", "WebSocket", true}, + {"keep-alive", "websocket", false}, + {"", "", false}, + } { + r := httptest.NewRequest(http.MethodGet, "/v1/sprites/demo/exec", nil) + if tc.conn != "" { + r.Header.Set("Connection", tc.conn) + } + if tc.upgrade != "" { + r.Header.Set("Upgrade", tc.upgrade) + } + if got := isWebSocketUpgrade(r); got != tc.want { + t.Errorf("Connection=%q Upgrade=%q: got %v, want %v", tc.conn, tc.upgrade, got, tc.want) + } + } +} diff --git a/internal/server/ws.go b/internal/server/ws.go index 8a9a85e..eb60e3e 100644 --- a/internal/server/ws.go +++ b/internal/server/ws.go @@ -31,6 +31,21 @@ const ( // [streamExit] and closes. func (s *Server) execSpriteWS(w http.ResponseWriter, r *http.Request) { id := r.PathValue("id") + + // The same path serves two protocols, which is the real API's shape rather + // than a convenience: upgraded, it is the control WebSocket below; plain, it + // is the session list. sprites-ex's Session.list_by_name/2 does exactly + // `Req.get("/v1/sprites/#{name}/exec")` and reads body["sessions"]. + // + // Without this, a plain GET fell through to websocket.Accept and came back + // 426, which is what orphaned every fountain turn — the client asks for the + // session list before it decides whether to reattach or start fresh, so the + // turn died before anything ran. See INTENTIUS/spritzer#18. + if !isWebSocketUpgrade(r) { + s.listExecSessions(w, r) + return + } + cmd := reconstructCmd(r) // Advertise the control-WebSocket capability on the 101 response so a client @@ -124,3 +139,39 @@ func writeFrame(ctx context.Context, c *websocket.Conn, streamID byte, payload [ frame = append(frame, payload...) return c.Write(ctx, websocket.MessageBinary, frame) } + +// isWebSocketUpgrade reports whether the request is asking to upgrade. +// +// Both headers are lists and both are case-insensitive, so this cannot be an +// equality check: Connection is commonly "keep-alive, Upgrade" from a proxy. +func isWebSocketUpgrade(r *http.Request) bool { + if !strings.EqualFold(r.Header.Get("Upgrade"), "websocket") { + return false + } + for _, part := range strings.Split(r.Header.Get("Connection"), ",") { + if strings.EqualFold(strings.TrimSpace(part), "upgrade") { + return true + } + } + return false +} + +// listExecSessions answers a plain GET on the exec path with the sprite's +// current exec sessions. +// +// spritzer runs each exec for the life of one WebSocket and keeps nothing +// afterwards, so a sprite that is not mid-exec has no sessions and the honest +// answer is an empty list. That is also the answer that lets a client get on +// with it: no active session means nothing to reattach to, so it starts a fresh +// turn instead of waiting for one that will never appear. +// +// A missing sprite is still a 404 here, the same as every other operation on +// one, rather than an empty list — "no sessions" and "no sprite" are different +// answers and a client reattaching should be able to tell them apart. +func (s *Server) listExecSessions(w http.ResponseWriter, r *http.Request) { + id := r.PathValue("id") + if _, err := s.store.Get(id); s.handleLookupError(w, id, err) { + return + } + writeJSON(w, http.StatusOK, map[string]any{"sessions": []any{}}) +}