fix(frontend): reconnect stream on same-session resume
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -32,6 +32,7 @@ import { useEffect, useMemo, useRef, useState } from "react";
|
||||
*/
|
||||
export function AppShell() {
|
||||
const [activeSessionId, setActiveSessionId] = useState<string | null>(null);
|
||||
const [streamGeneration, setStreamGeneration] = useState(0);
|
||||
const [creatingSession, setCreatingSession] = useState(false);
|
||||
const [awaitingQuestion, setAwaitingQuestion] = useState(false);
|
||||
const { data: sessions = [] } = useQuery<SessionSummary[]>({
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
@@ -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 };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user