From 1393c6358edc841bba8be709e356a11657fba510 Mon Sep 17 00:00:00 2001 From: User Date: Tue, 14 Jul 2026 20:53:19 +0200 Subject: [PATCH] fix(frontend): reconnect stream on same-session resume --- .superpowers/sdd/task-3-report.md | 94 ++++++++++--------- .../src/shell/AppShell.session-mgmt.test.tsx | 20 ++++ frontend/src/shell/AppShell.tsx | 5 +- frontend/src/stream/useSessionStream.test.tsx | 14 +++ frontend/src/stream/useSessionStream.ts | 4 +- 5 files changed, 90 insertions(+), 47 deletions(-) diff --git a/.superpowers/sdd/task-3-report.md b/.superpowers/sdd/task-3-report.md index b6696028..0228878d 100644 --- a/.superpowers/sdd/task-3-report.md +++ b/.superpowers/sdd/task-3-report.md @@ -1,59 +1,65 @@ -# Task 3 report — one secret bundle for local services +# Task 3 report — reconnect SSE on same-session Resume ## Status -Complete. Local pgvector bootstrap, reconciliation, migration, and preprocess services now -mount only `/run/secrets/thothii.secrets`. `deploy/vector/secret-policy.sh` validates the -whole bundle (allowlist, duplicate/empty/unknown keys, comments/blank lines, mode and symlink -policy) and returns only the requested value. The core entrypoint exposes DWH/vector/CA values -to the harness and materializes short-lived 0600 password files for workspace resolution. +Complete. A successful Resume of the currently active session now replaces its existing +`EventSource` connection. Resuming a different session continues to reconnect through the +session ID change only, without a generation-driven second connection. + +## Implementation + +- `useSessionStream` accepts an optional `generation` argument (default `0`) and includes it + in the stream effect dependencies. A generation change therefore runs the existing cleanup, + closes the old source, and opens the same URL again. +- `AppShell` captures whether the requested Resume ID is already active before its existing + optimistic state updates. It increments the stream generation only after `resumeSession(id)` + succeeds and only for that same-ID case. +- The existing optimistic session switch, phase refresh, and failed-Resume rollback remain + unchanged. A failed POST cannot increment the generation. ## TDD evidence -- RED: `./scripts/test-preprocess-compose-config.sh` failed on the pre-existing - `vector_reader_password` Compose secret declaration. -- GREEN: the same command passes after the bundle conversion and verifies local-vector - workspace interpolation and shared secret mounts. -- `./scripts/test-vector-secret-policy.sh` covers comments/blank lines and rejects an - unrelated duplicate key. +- RED command: + `cd frontend && npx vitest run src/stream/useSessionStream.test.tsx src/shell/AppShell.session-mgmt.test.tsx` +- RED result: 2 expected failures and 15 passes. The hook test observed + `first.closed === false`; the AppShell test observed one `FakeEventSource` instead of two + after the second same-ID Resume. +- GREEN focused result: the same command passed 2/2 files and 17/17 tests after the minimal + production wiring. -## Verification +## Full verification -- `./scripts/test-vector-secret-policy.sh` — passed. -- `./scripts/test-preprocess-compose-config.sh` — passed. -- `./scripts/test-vector-backup-restore-safety.sh` — passed. -- `./scripts/test-default-compose.sh` — passed. -- `./scripts/test-container-deployment.sh` — passed. -- `./scripts/local-vector-smoke.sh` — passed with real Docker (bootstrap rotation, role - reconciliation, migration, persistence and restart). -- `./scripts/preprocess-smoke.sh` — passed with real Docker (unchanged rerun, mutation, DWH - job, ACTIVE publication and cleanup). -- `./scripts/preprocess-smoke.sh --cleanup-failure` — passed. -- `git diff --check` and `sh -n` gates — passed. +- Baseline before edits: `cd frontend && npx vitest run` — 42/42 files and 251/251 tests passed. +- Focused tests: 2/2 files and 17/17 tests passed. +- Full frontend suite: `cd frontend && npx vitest run` — 42/42 files and 253/253 tests passed. +- Typecheck: `cd frontend && npx tsc -b` — exit 0. +- Production build: `cd frontend && npm run build` — exit 0; Vite transformed 4,835 modules + and completed the production bundle. +- `git diff --check` — passed. -## Critical review fix +The suite and build retained the pre-existing MSW unhandled-request, React ref/`act`, Node type +stripping, and Vite chunk-size warnings. This task introduced no new warning category. -`buildPiChildEnv` now removes `THT_DWH_API_KEY`, `THT_VEC_API_KEY`, `THT_VEC_WRITE_API_KEY`, -`THT_SSL_CA`, `THT_CA`, and their file metadata before spawning Pi. A regression test proves -that neither secret values nor bundle/file metadata are inherited by the Pi child. +## Files -## Commits +- `frontend/src/stream/useSessionStream.ts` +- `frontend/src/stream/useSessionStream.test.tsx` +- `frontend/src/shell/AppShell.tsx` +- `frontend/src/shell/AppShell.session-mgmt.test.tsx` +- `.superpowers/sdd/task-3-report.md` -- `70a19f2 feat(compose): use one secret bundle for local services` -- `d500563 fix(security): scrub deployment secrets from Pi child` -- `8518a73 fix(security): scrub raw deployment secret values` +## Self-review -## Concern +- Confirmed the old EventSource is closed before the replacement is retained by React's effect + lifecycle, and the replacement uses the identical session URL. +- Confirmed same-ID detection happens before the optimistic `setActiveSessionId(id)` call. +- Confirmed the generation increments only after a successful Resume POST; the catch/rollback + branch is unchanged. +- Confirmed a different ID leaves the generation unchanged, so the existing session-ID effect + change creates exactly one replacement connection. +- Confirmed the diff is frontend-only apart from this report and contains no backend, Docker, + configuration, or session changes. -The rotation helper retains its old/new scratch-file CLI contract; smoke tests keep those files -outside Compose and mount only the bundle. +## Concerns -## Whole-branch review fixes - -- `core-entrypoint.sh` validates `THT_SECRETS_FILE` fail-closed before optional lookups; malformed, - duplicate, unknown, oversized, or overlong bundles stop startup with sanitized diagnostics. -- Runtime password files are cleaned after child exit via signal forwarding and `wait`, rather - than being orphaned by `exec`. -- The shell loader accepts CRLF bundles (Windows/Notepad) consistently with the TypeScript loader. -- Optional key lookup distinguishes an absent key from an invalid value; present malformed - credentials now stop entrypoint startup instead of being silently ignored. +None. diff --git a/frontend/src/shell/AppShell.session-mgmt.test.tsx b/frontend/src/shell/AppShell.session-mgmt.test.tsx index 6335b4e1..5ae6fb2a 100644 --- a/frontend/src/shell/AppShell.session-mgmt.test.tsx +++ b/frontend/src/shell/AppShell.session-mgmt.test.tsx @@ -96,6 +96,26 @@ test("Resume paints the re-entry phase from the manifest (optimistic, before the await waitFor(() => expect(useSessionStore.getState().currentPhase).toBe("F4")); }); +test("resuming the active session reconnects its EventSource", async () => { + server.use( + http.post("http://localhost:8787/sessions/:id/resume", () => + new HttpResponse(null, { status: 204 })), + http.get("http://localhost:8787/sessions/:id", () => + HttpResponse.json({ id: "s1", status: "open", phase: 1 })), + ); + wrap(); + await userEvent.click(await screen.findByText("Attiva uno")); + await userEvent.click(await screen.findByRole("button", { name: /resume/i })); + await waitFor(() => expect(FakeEventSource.instances).toHaveLength(1)); + const first = FakeEventSource.instances[0]; + + await userEvent.click(screen.getByText("Attiva uno")); + await userEvent.click(await screen.findByRole("button", { name: /resume/i })); + + await waitFor(() => expect(FakeEventSource.instances).toHaveLength(2)); + expect(first.closed).toBe(true); +}); + test("a failed resume keeps the panel open and does not activate the session", async () => { server.use(http.post("http://localhost:8787/sessions/:id/resume", () => new HttpResponse(null, { status: 409 }))); wrap(); diff --git a/frontend/src/shell/AppShell.tsx b/frontend/src/shell/AppShell.tsx index 05a59e92..ec6dcc7f 100644 --- a/frontend/src/shell/AppShell.tsx +++ b/frontend/src/shell/AppShell.tsx @@ -32,6 +32,7 @@ import { useEffect, useMemo, useRef, useState } from "react"; */ export function AppShell() { const [activeSessionId, setActiveSessionId] = useState(null); + const [streamGeneration, setStreamGeneration] = useState(0); const [creatingSession, setCreatingSession] = useState(false); const [awaitingQuestion, setAwaitingQuestion] = useState(false); const { data: sessions = [] } = useQuery({ @@ -100,6 +101,7 @@ export function AppShell() { } async function doResume(id: string) { const s = sessions.find((x) => x.id === id) ?? null; + const reconnectSameSession = activeSessionId === id; // Optimistic switch: change to the session view IMMEDIATELY so the click feels // instant (the resume POST spawns a Pi process and can take seconds). The // working spinner shows straight away; the backend calls run after. @@ -111,6 +113,7 @@ export function AppShell() { setActiveSessionId(id); try { await resumeSession(id); + if (reconnectSameSession) setStreamGeneration((value) => value + 1); // Optimistic phase paint: colour the re-entry phase before the first gate. // The manifest's `phase` is the 1-based current phase (1..8). try { @@ -218,7 +221,7 @@ export function AppShell() { // session sits idle or a gate awaits the reviewer (pendingWidget). const running = working && !finalized; - useSessionStream(activeSessionId); + useSessionStream(activeSessionId, streamGeneration); // A backend "session_exit" system event (e.g. the replay server emitting it // when the reviewer picks "Esci") asks us to leave the live session view and diff --git a/frontend/src/stream/useSessionStream.test.tsx b/frontend/src/stream/useSessionStream.test.tsx index 77c7f8b5..ce4d4988 100644 --- a/frontend/src/stream/useSessionStream.test.tsx +++ b/frontend/src/stream/useSessionStream.test.tsx @@ -46,3 +46,17 @@ test("closes the stream on unmount", () => { unmount(); expect(es.closed).toBe(true); }); + +test("reconnects the same session when the generation changes", () => { + const { rerender } = renderHook( + ({ generation }) => useSessionStream("s1", generation), + { initialProps: { generation: 0 } }, + ); + const first = FakeEventSource.instances[0]; + + rerender({ generation: 1 }); + + expect(first.closed).toBe(true); + expect(FakeEventSource.instances).toHaveLength(2); + expect(FakeEventSource.instances[1].url).toBe(first.url); +}); diff --git a/frontend/src/stream/useSessionStream.ts b/frontend/src/stream/useSessionStream.ts index 182b0edd..75f88e1d 100644 --- a/frontend/src/stream/useSessionStream.ts +++ b/frontend/src/stream/useSessionStream.ts @@ -4,7 +4,7 @@ import { joinBackendPath } from "../api/runtime-config"; import { useSessionStore } from "../store/sessionStore"; import type { StreamEvent } from "../api/types"; -export function useSessionStream(sessionId: string | null) { +export function useSessionStream(sessionId: string | null, generation = 0) { const [connected, setConnected] = useState(false); const applyEvent = useSessionStore((s) => s.applyEvent); @@ -40,7 +40,7 @@ export function useSessionStream(sessionId: string | null) { es.close(); setConnected(false); }; - }, [sessionId, applyEvent]); + }, [sessionId, generation, applyEvent]); return { connected }; }