diff --git a/services/hermes/router/main_test.go b/services/hermes/router/main_test.go index 152ebeca..d63b6348 100644 --- a/services/hermes/router/main_test.go +++ b/services/hermes/router/main_test.go @@ -5,8 +5,11 @@ import ( "io" "net/http" "net/http/httptest" + "net/url" "os" "path/filepath" + "regexp" + "strconv" "strings" "testing" "time" @@ -163,18 +166,24 @@ func TestSessionContinuityAssetHasAccessiblePollFallback(t *testing.T) { } asset := response.Body.String() for _, expected := range []string{ - "aria-live", "aria-busy", "/api/sessions/", "/messages?limit=24&hermes_fallback=1", + "aria-live", "aria-busy", "/api/session?session_id=", + "&messages=1&msg_limit=24", "hermes_fallback=1", "Latest stored activity", "no renderable messages", - "Session updates disconnected", "oauth2/start?rd=", "payload.session_id", - "session.ended_at == null", "latest.observed", "addEventListener('online'", - "addEventListener('pageshow'", "schedule(document.hidden", "fetch(", + "Session updates disconnected", "oauth2/start?rd=", "payload.session", + "session_profile_mismatch", "session.is_streaming", "latest.observed", + "addEventListener('online'", "addEventListener('pageshow'", + "schedule(document.hidden", "fetch(", } { if !strings.Contains(asset, expected) { t.Fatalf("session fallback omitted %q", expected) } } for _, forbidden := range []string{ - "location.replace", "location.reload", "history.replaceState", "/api/session?", "WebSocket", + "location.replace", "location.reload", "history.replaceState", "WebSocket", + // `/api/sessions/` belongs to the Hermes agent dashboard, not to the + // tenant WebUI this router proxies. Polling it 404s on every request and + // renders a false ownership error after each full page load. + "/api/sessions/", } { if strings.Contains(asset, forbidden) { t.Fatalf("session fallback interferes with native continuity via %q", forbidden) @@ -182,6 +191,227 @@ func TestSessionContinuityAssetHasAccessiblePollFallback(t *testing.T) { } } +// tenantWebUI mirrors the dispatch contract of the deployed Hermes WebUI +// (`hermes-webui` server.py): `GET /api/session?session_id=` is the only +// session read route, an unrouted path falls through to a generic 404, and an +// unknown session id is reported as "Session not found". +type tenantWebUI struct { + server *httptest.Server + sessions map[string]bool + identities []string + requests int + cookies []string +} + +func newTenantWebUI(t *testing.T, marker string, sessions ...string) *tenantWebUI { + t.Helper() + backend := &tenantWebUI{sessions: map[string]bool{}} + for _, id := range sessions { + backend.sessions[id] = true + } + backend.server = httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + backend.requests++ + backend.identities = append(backend.identities, request.Header.Get(trustedTenantHeader)) + backend.cookies = append(backend.cookies, request.Header.Get("Cookie")) + for _, leaked := range []string{"X-Forwarded-User", "X-Auth-Request-User", "Authorization"} { + if request.Header.Get(leaked) != "" { + t.Errorf("external identity header %q reached a tenant backend", leaked) + } + } + writer.Header().Set("Content-Type", "application/json") + if request.URL.Path != "/api/session" { + writer.WriteHeader(http.StatusNotFound) + _, _ = io.WriteString(writer, `{"error":"not found"}`) + return + } + id := request.URL.Query().Get("session_id") + if !backend.sessions[id] { + writer.WriteHeader(http.StatusNotFound) + _, _ = io.WriteString(writer, `{"error":"Session not found"}`) + return + } + _ = json.NewEncoder(writer).Encode(map[string]any{"session": map[string]any{ + "session_id": id, + "title": marker, + "messages": []map[string]any{{"role": "assistant", "content": marker}}, + "message_count": 1, + "is_streaming": false, + }}) + })) + t.Cleanup(backend.server.Close) + return backend +} + +// continuityPollPath derives the request the injected fallback actually issues, +// so a regression in the polled contract fails these tests instead of silently +// reintroducing the permanent 404. +func continuityPollPath(t *testing.T, router *tenantRouter, sessionID string) string { + t.Helper() + request := httptest.NewRequest(http.MethodGet, "/hermes-session-continuity.js", nil) + request.Header.Set("X-Forwarded-User", "asset-reader") + response := httptest.NewRecorder() + router.ServeHTTP(response, request) + if response.Code != http.StatusOK { + t.Fatalf("continuity asset got status %d", response.Code) + } + matcher := regexp.MustCompile(`'(/api/[^']*)' \+ encodeURIComponent\(sessionId\) \+\s*'([^']*)'`) + parts := matcher.FindStringSubmatch(response.Body.String()) + if parts == nil { + t.Fatal("continuity fallback does not build a session-scoped poll URL") + } + return parts[1] + url.QueryEscape(sessionID) + parts[2] +} + +func continuityPoll(t *testing.T, router *tenantRouter, subject, cookie, path string) *httptest.ResponseRecorder { + t.Helper() + request := httptest.NewRequest(http.MethodGet, path, nil) + request.Header.Set("X-Forwarded-User", subject) + request.Header.Set("X-Auth-Request-User", subject) + if cookie != "" { + request.Header.Set("Cookie", cookie) + } + response := httptest.NewRecorder() + router.ServeHTTP(response, request) + return response +} + +// Reproduction of the reported failure. The tenant WebUI image +// (registry.bstein.dev/bstein/hermes-webui@sha256:c276a9e1…) routes exactly one +// session read, `GET /api/session?session_id=`; `/api/sessions/[/messages]` +// belongs to the separate Hermes agent dashboard and falls through to +// server.py's generic 404. Polling it could never succeed for anyone, so the +// banner fired on every full page load rather than on a real ownership problem. +func TestLegacyDashboardSessionPollAlwaysMissesTheTenantWebUI(t *testing.T) { + const sessionID = "sess-1" + backend := newTenantWebUI(t, "brad-private-session", sessionID) + router, err := newTenantRouter(filepath.Join(t.TempDir(), "state.json"), 1, + func(int) string { return backend.server.URL }) + if err != nil { + t.Fatal(err) + } + for _, legacy := range []string{ + "/api/sessions/" + sessionID, + "/api/sessions/" + sessionID + "/messages?limit=24&hermes_fallback=1", + } { + response := continuityPoll(t, router, "keycloak-subject-brad", "", legacy) + if response.Code != http.StatusNotFound { + t.Fatalf("%s: got %d, want the WebUI's 404 for an unrouted path", legacy, response.Code) + } + } + if response := continuityPoll(t, router, "keycloak-subject-brad", "", + continuityPollPath(t, router, sessionID)); response.Code != http.StatusOK { + t.Fatalf("the shipped fallback still misses the tenant WebUI: got %d", response.Code) + } +} + +// The Keycloak logout/login round-trip returns the browser to /session/ as +// a full document load, which is the only moment the injected fallback runs. +// The durable session must still resolve for its stable owner. +func TestSessionContinuityPollSurvivesLogoutAndRelogin(t *testing.T) { + const subject = "keycloak-subject-brad" + const sessionID = "9048e2a574d1" + backend := newTenantWebUI(t, "brad-private-session", sessionID) + router, err := newTenantRouter(filepath.Join(t.TempDir(), "state.json"), 4, func(slot int) string { + if slot == 0 { + return backend.server.URL + } + return "http://127.0.0.1:1" + }) + if err != nil { + t.Fatal(err) + } + pollPath := continuityPollPath(t, router, sessionID) + + before := continuityPoll(t, router, subject, + "__Host-hermes_chat=first-sso-session; "+tenantSessionCookie+"=webui-1", pollPath) + if before.Code != http.StatusOK { + t.Fatalf("first visit could not read its own session: got %d, want 200", before.Code) + } + + // Logout invalidates the Keycloak session, so the browser returns with a + // completely different oauth2-proxy cookie under the same Keycloak subject. + after := continuityPoll(t, router, subject, + "__Host-hermes_chat=second-sso-session; "+tenantSessionCookie+"=webui-1", pollPath) + if after.Code != http.StatusOK { + t.Fatalf("relogin lost the durable session: got %d, want 200", after.Code) + } + var payload struct { + Session struct { + SessionID string `json:"session_id"` + Title string `json:"title"` + } `json:"session"` + } + if err := json.NewDecoder(after.Body).Decode(&payload); err != nil { + t.Fatal(err) + } + if payload.Session.SessionID != sessionID || payload.Session.Title != "brad-private-session" { + t.Fatalf("relogin resolved the wrong session: %#v", payload.Session) + } + if len(backend.identities) != 2 || backend.identities[0] != "slot-0" || backend.identities[1] != "slot-0" { + t.Fatalf("relogin did not keep a stable tenant identity: %#v", backend.identities) + } + for _, cookie := range backend.cookies { + if strings.Contains(cookie, "__Host-hermes_chat") { + t.Fatalf("the Keycloak session cookie crossed into a tenant: %q", cookie) + } + } +} + +// Repairing continuity must not turn the poll into a cross-tenant read. +func TestSessionContinuityNeverExposesAnotherSubjectsSession(t *testing.T) { + const ownerSession = "owner-session-id" + owner := newTenantWebUI(t, "owner-private-marker", ownerSession) + intruder := newTenantWebUI(t, "intruder-private-marker") + backends := []*tenantWebUI{owner, intruder} + router, err := newTenantRouter(filepath.Join(t.TempDir(), "state.json"), 2, func(slot int) string { + return backends[slot].server.URL + }) + if err != nil { + t.Fatal(err) + } + ownerSlot, err := router.slotFor("keycloak-owner") + if err != nil { + t.Fatal(err) + } + intruderSlot, err := router.slotFor("keycloak-intruder") + if err != nil { + t.Fatal(err) + } + if ownerSlot == intruderSlot { + t.Fatal("two Keycloak subjects shared one isolated slot") + } + pollPath := continuityPollPath(t, router, ownerSession) + + if response := continuityPoll(t, router, "keycloak-owner", + tenantSessionCookie+"=owner-webui", pollPath); response.Code != http.StatusOK { + t.Fatalf("the owner lost its own session: got %d", response.Code) + } + ownerRequests := backends[ownerSlot].requests + + // The intruder replays the owner's session id, the owner's WebUI cookie and + // a forged tenant assertion for the owner's slot. + request := httptest.NewRequest(http.MethodGet, pollPath, nil) + request.Header.Set("X-Forwarded-User", "keycloak-intruder") + request.Header.Set("Cookie", tenantSessionCookie+"=owner-webui") + request.Header.Set(trustedTenantHeader, "slot-"+strconv.Itoa(ownerSlot)) + response := httptest.NewRecorder() + router.ServeHTTP(response, request) + + if response.Code != http.StatusNotFound { + t.Fatalf("cross-subject session read got %d, want 404", response.Code) + } + if strings.Contains(response.Body.String(), "owner-private-marker") { + t.Fatalf("another subject's session leaked: %s", response.Body.String()) + } + if backends[ownerSlot].requests != ownerRequests { + t.Fatal("a forged tenant assertion reached another subject's backend") + } + identities := backends[intruderSlot].identities + if len(identities) == 0 || identities[len(identities)-1] != "slot-"+strconv.Itoa(intruderSlot) { + t.Fatalf("the router did not overwrite the forged tenant identity: %#v", identities) + } +} + func TestSessionContinuityPollIsBoundedByRouter(t *testing.T) { backend := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, _ *http.Request) { messages := make([]map[string]int, 30) @@ -189,9 +419,9 @@ func TestSessionContinuityPollIsBoundedByRouter(t *testing.T) { messages[index] = map[string]int{"index": index} } writer.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(writer).Encode(map[string]any{ - "session_id": "resolved", "messages": messages, - }) + _ = json.NewEncoder(writer).Encode(map[string]any{"session": map[string]any{ + "session_id": "resolved", "messages": messages, "message_count": 30, + }}) })) defer backend.Close() router, err := newTenantRouter(filepath.Join(t.TempDir(), "state.json"), 1, func(int) string { return backend.URL }) @@ -200,7 +430,7 @@ func TestSessionContinuityPollIsBoundedByRouter(t *testing.T) { } request := httptest.NewRequest( http.MethodGet, - "/api/sessions/root/messages?limit=24&hermes_fallback=1", + continuityPollPath(t, router, "root"), nil, ) request.Header.Set("X-Forwarded-User", "subject") @@ -209,11 +439,8 @@ func TestSessionContinuityPollIsBoundedByRouter(t *testing.T) { if response.Code != http.StatusOK { t.Fatalf("got status %d", response.Code) } - var payload sessionSnapshot - if err := json.NewDecoder(response.Body).Decode(&payload); err != nil { - t.Fatal(err) - } - if len(payload.Messages) != sessionSnapshotItems || payload.TotalMessages != 30 { + payload := decodeSnapshot(t, response.Result()) + if len(payload.Messages) != sessionSnapshotItems || payload.MessageCount != 30 { t.Fatalf("router returned unbounded fallback: %#v", payload) } } diff --git a/services/hermes/router/session_continuity.go b/services/hermes/router/session_continuity.go index af42b35d..f882c65e 100644 --- a/services/hermes/router/session_continuity.go +++ b/services/hermes/router/session_continuity.go @@ -1,5 +1,14 @@ package main +// The chat router proxies browser traffic to the tenant Hermes WebUI, whose +// session read contract is `GET /api/session?session_id=`. The dashboard +// style `/api/sessions/[/messages]` routes belong to the separate Hermes +// agent deployment (services/hermes/scripts/patch_web_session_activity.py) and +// are unrouted here, so polling them returned the WebUI's generic 404 on every +// attempt and rendered a false "unavailable to this account" banner after every +// full page load — exactly what a Keycloak logout/login round-trip produces. +const sessionFallbackPath = "/api/session" + const sessionContinuityJS = `(() => { const match = location.pathname.match(/^\/session\/([^/]+)\/?$/); if (!match) return; @@ -41,15 +50,27 @@ const sessionContinuityJS = `(() => { if (latest.observed && typeof latest.content === 'string') return latest.content.slice(0, 120); return latest.role === 'assistant' ? 'assistant update' : latest.role === 'user' ? 'request stored' : 'working'; }; - const authOrMissing = (response) => { + // Only the WebUI's own answers decide what the banner claims. A 409 is the + // single case where the stored session really is out of this account's + // active scope; a 404 means the conversation is no longer stored at all. + const handled = async (response) => { if (response.status === 401 || response.status === 403) { const rd = location.pathname + location.search + location.hash; location.assign('/oauth2/start?rd=' + encodeURIComponent(rd)); return 'auth'; } + if (response.status === 409) { + let payload = {}; + try { payload = await response.json(); } catch (_) { payload = {}; } + show(payload.code === 'session_profile_mismatch' + ? 'This session belongs to a different profile on this account. Switch profiles to reopen it.' + : 'This session is unavailable to this account.', false, true); + schedule(15000); + return 'scoped'; + } if (response.status === 404) { - show('This session is unavailable to this account.', false, true); - schedule(10000); + show('This conversation is no longer stored in your private chat.', false, true); + schedule(15000); return 'missing'; } return ''; @@ -63,28 +84,25 @@ const sessionContinuityJS = `(() => { request = new AbortController(); const timeout = window.setTimeout(() => request && request.abort(), 8000); try { - const root = '/api/sessions/' + encodeURIComponent(sessionId); - const messageResponse = await fetch(root + '/messages?limit=24&hermes_fallback=1', {cache:'no-store', credentials:'same-origin', signal:request.signal}); - if (authOrMissing(messageResponse)) return; - if (!messageResponse.ok) throw new Error('session messages poll failed'); - const payload = await messageResponse.json(); - const messages = Array.isArray(payload.messages) ? payload.messages : []; - const resolvedId = typeof payload.session_id === 'string' && payload.session_id ? payload.session_id : sessionId; - const detailResponse = await fetch('/api/sessions/' + encodeURIComponent(resolvedId), {cache:'no-store', credentials:'same-origin', signal:request.signal}); - if (authOrMissing(detailResponse)) return; - if (!detailResponse.ok) throw new Error('session detail poll failed'); - const session = await detailResponse.json(); - const lastActive = Number(session.last_active || session.started_at || 0); - const durableWorker = session.source === 'api_server' && Boolean(session.parent_session_id); - const recentlyInteractive = Number.isFinite(lastActive) && Date.now() / 1000 - lastActive < 300; - const active = session.ended_at == null && messages.length > 0 && (durableWorker || recentlyInteractive); - if (active) show('Hermes is working. Latest stored activity: ' + activityLabel(messages) + '.', true, false); - else if (!messages.length && Date.now() - started >= 4000) show('This session has no renderable messages yet. It may be new or no longer available.', false, true); + const query = '/api/session?session_id=' + encodeURIComponent(sessionId) + + '&messages=1&msg_limit=24&resolve_model=0&hermes_fallback=1'; + const response = await fetch(query, {cache:'no-store', credentials:'same-origin', signal:request.signal}); + if (await handled(response)) return; + if (!response.ok) throw new Error('session poll failed'); + const payload = await response.json(); + const session = payload && typeof payload.session === 'object' && payload.session ? payload.session : {}; + const messages = Array.isArray(session.messages) ? session.messages : []; + const stored = Number(session.message_count); + const count = Number.isFinite(stored) && stored > 0 ? stored : messages.length; + const working = Boolean(session.is_streaming) || Boolean(session.active_stream_id) || + Boolean(session.has_pending_user_message); + if (working) show('Hermes is working. Latest stored activity: ' + activityLabel(messages) + '.', true, false); + else if (!count && Date.now() - started >= 4000) show('This session has no renderable messages yet. It may be new or no longer available.', false, true); else hide(); - schedule(document.hidden ? 10000 : 2500); + schedule(document.hidden ? 15000 : 3000); } catch (_) { show('Session updates disconnected. Retrying without changing this session…', true, false); - schedule(document.hidden ? 10000 : 3000); + schedule(document.hidden ? 15000 : 3000); } finally { clearTimeout(timeout); request = null; diff --git a/services/hermes/router/session_snapshot.go b/services/hermes/router/session_snapshot.go index 237dd19b..002ed210 100644 --- a/services/hermes/router/session_snapshot.go +++ b/services/hermes/router/session_snapshot.go @@ -14,21 +14,18 @@ const ( sessionSnapshotBytes = 8 << 20 ) -type sessionSnapshot struct { - SessionID string `json:"session_id"` - Messages []json.RawMessage `json:"messages"` - TotalMessages int `json:"total_messages"` -} - // boundSessionSnapshot caps only the continuity fallback response. Native // WebUI requests remain untouched, including when an older backend ignores -// its optional `limit` query parameter. +// its optional `msg_limit` query parameter. The tenant WebUI answers with +// {"session": {..., "messages": [...], "message_count": N}}, so the envelope +// is decoded field-by-field: every key other than the message tail is relayed +// verbatim rather than re-serialized from a fixed struct, which would silently +// drop session metadata the poller and future WebUI releases depend on. func boundSessionSnapshot(response *http.Response) error { request := response.Request if request == nil || response.StatusCode != http.StatusOK || request.URL.Query().Get("hermes_fallback") != "1" || - !strings.HasPrefix(request.URL.Path, "/api/sessions/") || - !strings.HasSuffix(request.URL.Path, "/messages") { + request.URL.Path != sessionFallbackPath { return nil } body, err := io.ReadAll(io.LimitReader(response.Body, sessionSnapshotBytes+1)) @@ -39,19 +36,7 @@ func boundSessionSnapshot(response *http.Response) error { if len(body) > sessionSnapshotBytes { return errors.New("session snapshot exceeds safe response limit") } - var payload sessionSnapshot - if err := json.Unmarshal(body, &payload); err != nil || payload.Messages == nil { - return errors.New("session snapshot is malformed") - } - total := len(payload.Messages) - if payload.TotalMessages > total { - total = payload.TotalMessages - } - if len(payload.Messages) > sessionSnapshotItems { - payload.Messages = payload.Messages[len(payload.Messages)-sessionSnapshotItems:] - } - payload.TotalMessages = total - body, err = json.Marshal(payload) + body, err = boundSessionSnapshotBody(body) if err != nil { return err } @@ -63,3 +48,49 @@ func boundSessionSnapshot(response *http.Response) error { response.Header.Del("ETag") return nil } + +// boundSessionSnapshotBody trims the message tail of one WebUI session payload +// while preserving the total the backend reported. +func boundSessionSnapshotBody(body []byte) ([]byte, error) { + malformed := errors.New("session snapshot is malformed") + var envelope map[string]json.RawMessage + if err := json.Unmarshal(body, &envelope); err != nil { + return nil, malformed + } + rawSession, ok := envelope["session"] + if !ok { + return nil, malformed + } + var session map[string]json.RawMessage + if err := json.Unmarshal(rawSession, &session); err != nil { + return nil, malformed + } + var messages []json.RawMessage + if raw, ok := session["messages"]; ok { + if err := json.Unmarshal(raw, &messages); err != nil { + return nil, malformed + } + } + total := len(messages) + if raw, ok := session["message_count"]; ok { + var count int + if err := json.Unmarshal(raw, &count); err == nil && count > total { + total = count + } + } + if len(messages) > sessionSnapshotItems { + messages = messages[len(messages)-sessionSnapshotItems:] + } + trimmed, err := json.Marshal(messages) + if err != nil { + return nil, err + } + session["messages"] = trimmed + session["message_count"] = json.RawMessage(strconv.Itoa(total)) + rawSession, err = json.Marshal(session) + if err != nil { + return nil, err + } + envelope["session"] = rawSession + return json.Marshal(envelope) +} diff --git a/services/hermes/router/session_snapshot_test.go b/services/hermes/router/session_snapshot_test.go index 64aee010..0d35c50e 100644 --- a/services/hermes/router/session_snapshot_test.go +++ b/services/hermes/router/session_snapshot_test.go @@ -10,6 +10,20 @@ import ( "testing/iotest" ) +// webuiSession mirrors the fields of the tenant WebUI `GET /api/session` +// payload that the continuity fallback reads. +type webuiSession struct { + SessionID string `json:"session_id"` + Messages []json.RawMessage `json:"messages"` + MessageCount int `json:"message_count"` + Title string `json:"title,omitempty"` + ReadOnly bool `json:"read_only,omitempty"` +} + +type webuiSessionEnvelope struct { + Session webuiSession `json:"session"` +} + func snapshotResponse(target, body string) *http.Response { request, _ := http.NewRequest(http.MethodGet, target, nil) return &http.Response{ @@ -20,27 +34,33 @@ func snapshotResponse(target, body string) *http.Response { } } +func decodeSnapshot(t *testing.T, response *http.Response) webuiSession { + t.Helper() + var envelope webuiSessionEnvelope + if err := json.NewDecoder(response.Body).Decode(&envelope); err != nil { + t.Fatal(err) + } + return envelope.Session +} + func TestBoundSessionSnapshotKeepsOnlyRecentMessages(t *testing.T) { messages := make([]map[string]int, 30) for index := range messages { messages[index] = map[string]int{"index": index} } - body, _ := json.Marshal(map[string]any{ - "session_id": "resolved", "messages": messages, - }) + body, _ := json.Marshal(map[string]any{"session": map[string]any{ + "session_id": "resolved", "messages": messages, "message_count": 30, + }}) response := snapshotResponse( - "http://tenant/api/sessions/root/messages?limit=24&hermes_fallback=1", + "http://tenant/api/session?session_id=root&messages=1&msg_limit=24&hermes_fallback=1", string(body), ) response.Header.Set("ETag", "stale") if err := boundSessionSnapshot(response); err != nil { t.Fatal(err) } - var bounded sessionSnapshot - if err := json.NewDecoder(response.Body).Decode(&bounded); err != nil { - t.Fatal(err) - } - if len(bounded.Messages) != 24 || bounded.TotalMessages != 30 { + bounded := decodeSnapshot(t, response) + if len(bounded.Messages) != sessionSnapshotItems || bounded.MessageCount != 30 { t.Fatalf("snapshot was not bounded: %#v", bounded) } var first map[string]int @@ -54,29 +74,59 @@ func TestBoundSessionSnapshotKeepsOnlyRecentMessages(t *testing.T) { func TestBoundSessionSnapshotPreservesLargerReportedTotal(t *testing.T) { response := snapshotResponse( - "http://tenant/api/sessions/root/messages?hermes_fallback=1", - `{"session_id":"leaf","messages":[],"total_messages":100}`, + "http://tenant/api/session?session_id=leaf&hermes_fallback=1", + `{"session":{"session_id":"leaf","messages":[],"message_count":100}}`, ) if err := boundSessionSnapshot(response); err != nil { t.Fatal(err) } - var bounded sessionSnapshot - if err := json.NewDecoder(response.Body).Decode(&bounded); err != nil { + bounded := decodeSnapshot(t, response) + if bounded.SessionID != "leaf" || bounded.MessageCount != 100 { + t.Fatalf("reported total was lost: %#v", bounded) + } +} + +// The poller and the WebUI both evolve; bounding the message tail must never +// strip the surrounding session metadata that decides what the banner says. +func TestBoundSessionSnapshotRelaysUnknownSessionMetadata(t *testing.T) { + response := snapshotResponse( + "http://tenant/api/session?session_id=root&hermes_fallback=1", + `{"session":{"session_id":"root","messages":[{"role":"user"}],`+ + `"is_streaming":true,"active_stream_id":"stream-1","read_only":false,`+ + `"future_field":{"kept":true}},"other_envelope_key":7}`, + ) + if err := boundSessionSnapshot(response); err != nil { t.Fatal(err) } - if bounded.SessionID != "leaf" || bounded.TotalMessages != 100 { - t.Fatalf("reported total was lost: %#v", bounded) + body, err := io.ReadAll(response.Body) + if err != nil { + t.Fatal(err) + } + for _, expected := range []string{ + `"is_streaming":true`, `"active_stream_id":"stream-1"`, + `"future_field":{"kept":true}`, `"other_envelope_key":7`, + } { + if !strings.Contains(string(body), expected) { + t.Fatalf("bounded snapshot dropped %s: %s", expected, body) + } + } + if length := response.Header.Get("Content-Length"); length != "" && + length != strings.TrimSpace(length) { + t.Fatal("content length was not rewritten") } } func TestBoundSessionSnapshotFailsClosedOnMalformedOrOversizedBody(t *testing.T) { for name, body := range map[string]string{ - "malformed": `{`, - "oversized": strings.Repeat("x", sessionSnapshotBytes+1), + "malformed": `{`, + "missing": `{"error":"Session not found"}`, + "nonObject": `{"session":42}`, + "badMessageList": `{"session":{"messages":"all of them"}}`, + "oversized": strings.Repeat("x", sessionSnapshotBytes+1), } { t.Run(name, func(t *testing.T) { response := snapshotResponse( - "http://tenant/api/sessions/root/messages?hermes_fallback=1", + "http://tenant/api/session?session_id=root&hermes_fallback=1", body, ) if err := boundSessionSnapshot(response); err == nil { @@ -85,7 +135,7 @@ func TestBoundSessionSnapshotFailsClosedOnMalformedOrOversizedBody(t *testing.T) }) } response := snapshotResponse( - "http://tenant/api/sessions/root/messages?hermes_fallback=1", `{}`, + "http://tenant/api/session?session_id=root&hermes_fallback=1", `{}`, ) response.Body = io.NopCloser(iotest.ErrReader(errors.New("read failed"))) if err := boundSessionSnapshot(response); err == nil { @@ -96,15 +146,18 @@ func TestBoundSessionSnapshotFailsClosedOnMalformedOrOversizedBody(t *testing.T) func TestBoundSessionSnapshotLeavesNativeAndErrorResponsesUntouched(t *testing.T) { for _, response := range []*http.Response{ {StatusCode: http.StatusOK}, - snapshotResponse("http://tenant/api/sessions/root/messages", `{}`), - snapshotResponse("http://tenant/api/sessions/root", `{}`), + // The WebUI's own session reads carry no fallback marker. + snapshotResponse("http://tenant/api/session?session_id=root", `{}`), + snapshotResponse("http://tenant/api/sessions", `{}`), + // The dashboard-only route is not this backend's contract. + snapshotResponse("http://tenant/api/sessions/root/messages?hermes_fallback=1", `{}`), } { if err := boundSessionSnapshot(response); err != nil { t.Fatal(err) } } errorResponse := snapshotResponse( - "http://tenant/api/sessions/root/messages?hermes_fallback=1", `{}`, + "http://tenant/api/session?session_id=root&hermes_fallback=1", `{}`, ) errorResponse.StatusCode = http.StatusNotFound if err := boundSessionSnapshot(errorResponse); err != nil { diff --git a/services/hermes/router/web.go b/services/hermes/router/web.go index adb596a6..1bb2d1b8 100644 --- a/services/hermes/router/web.go +++ b/services/hermes/router/web.go @@ -470,7 +470,7 @@ func injectChatBridge(response *http.Response) error { content = strings.Replace(content, "", ``, 1) } if !strings.Contains(content, "hermes-session-continuity.js") { - content = strings.Replace(content, "", ``, 1) + content = strings.Replace(content, "", ``, 1) } response.Body = io.NopCloser(strings.NewReader(content)) response.ContentLength = int64(len(content)) diff --git a/testing/tests/test_hermes_chat_session_continuity.py b/testing/tests/test_hermes_chat_session_continuity.py index 0da542cb..f4f52428 100644 --- a/testing/tests/test_hermes_chat_session_continuity.py +++ b/testing/tests/test_hermes_chat_session_continuity.py @@ -11,9 +11,70 @@ import pytest from testing.tests.test_hermes_chat_support import ( HERMES, + _documents, ) +def _containers(path: Path) -> list[dict]: + """Return every container and init container in one workload document.""" + spec = _documents(path)[0]["spec"]["template"]["spec"] + return [*spec.get("initContainers", []), *spec.get("containers", [])] + + +def _env(container: dict) -> dict[str, str]: + return { + entry["name"]: entry.get("value", "") + for entry in container.get("env", []) + if isinstance(entry, dict) and "name" in entry + } + + +def test_chat_continuity_polls_the_route_its_own_backend_serves(): + """The injected fallback must speak the tenant WebUI session contract.""" + fallback = (HERMES / "router" / "session_continuity.go").read_text(encoding="utf-8") + snapshot = (HERMES / "router" / "session_snapshot.go").read_text(encoding="utf-8") + script = fallback.split("const sessionContinuityJS = `", 1)[1].rsplit("`", 1)[0] + + assert "'/api/session?session_id=' + encodeURIComponent(sessionId)" in script + assert "&messages=1&msg_limit=24" in script + assert "hermes_fallback=1" in script + # A 409 is the only answer that means "not in this account's active scope". + assert "session_profile_mismatch" in script + assert 'sessionFallbackPath = "/api/session"' in fallback + assert "request.URL.Path != sessionFallbackPath" in snapshot + + # /api/sessions/[/messages] is the Hermes agent dashboard contract. The + # chat tenants never route it, so polling it was a permanent 404 that + # reported a false ownership failure after every full page load. + assert "/api/sessions/" not in script + + +def test_dashboard_session_route_never_backs_the_chat_tenants(): + """Only the agent dashboard gains /api/sessions/{id}/messages.""" + activity_patch = "patch_web_session_activity.py" + marker = '@app.get("/api/sessions/{session_id}/messages")' + assert marker in (HERMES / "scripts" / activity_patch).read_text(encoding="utf-8") + + agent = _containers(HERMES / "agent-deployment.yaml") + assert any( + activity_patch in " ".join(map(str, container.get("command", []) + container.get("args", []))) + for container in agent + ) + + tenants = _containers(HERMES / "chat-statefulset.yaml") + assert not any( + activity_patch in " ".join(map(str, container.get("command", []) + container.get("args", []))) + for container in tenants + ) + + # The router proxies browser traffic to this WebUI container, and asserts + # the tenant identity through the header the WebUI is told to trust. + webui = next(container for container in tenants if container["name"] == "webui") + router = (HERMES / "router" / "main.go").read_text(encoding="utf-8") + header = _env(webui)["HERMES_WEBUI_TRUSTED_AUTH_HEADER"] + assert f'trustedTenantHeader = "{header}"' in router + + def test_codex_native_health_overrides_historical_router_errors( tmp_path: Path, monkeypatch ):