fix: serialize session lifecycle transitions
This commit is contained in:
User
@@ -496,6 +496,9 @@ no output, exit 0
|
||||
backlog; an in-memory same-session reconnect is exact-once from its cursor.
|
||||
- Per-session sequence counters remain in backend memory after `clear` by design so later in-process
|
||||
cold same-id Resume cannot reuse ids. Permanent DELETE removes the counter via `forget`.
|
||||
- The Delete-then-Resume adversarial route test proves the deleted session is not resurrected but
|
||||
currently receives the runner's generic HTTP 500 when `sessionShow` can no longer find it. A
|
||||
future API cleanup can normalize that missing-session response to 404 or 409.
|
||||
- Frontend tests still print pre-existing MSW unhandled-request and React ref/`act` warnings even
|
||||
though all 276 tests pass. The frontend production build still reports pre-existing large chunk
|
||||
warnings. Neither warning class was introduced or expanded by this change.
|
||||
@@ -614,3 +617,313 @@ exit 0
|
||||
|
||||
The final frontend run retains the repository's pre-existing MSW/ref/`act` warnings, and the build
|
||||
retains the pre-existing large-chunk warning. No test, typecheck, or build failures remain.
|
||||
|
||||
## Stale-bootstrap, lifecycle-lock, and competing-Resume hardening
|
||||
|
||||
Date: 2026-07-15
|
||||
Base: `08b1f4909e8eb7538156cecc2e7a6cafb46ddfc7`
|
||||
|
||||
This follow-up closes asynchronous identity/order and multi-client transport gaps found in the
|
||||
pre-deployment review:
|
||||
|
||||
- `PiProcessManager.teardownIfCurrent(id, runtime)` makes teardown an identity-checked operation.
|
||||
Bootstrap re-checks identity after configuration/retrieval and before both the public
|
||||
`Starting model` event and model start. Its failure continuation acquires the same session
|
||||
lifecycle lock, claims only its own runtime identity, and holds serialization through persisted
|
||||
failure and the public terminal sequence. A continuation left behind by Close or DELETE cannot
|
||||
target a replacement or recreate forgotten SSE state.
|
||||
- The former Resume-only promise tail is now a per-session lifecycle lock shared by Resume, Close,
|
||||
and DELETE. Each route reads the current runtime inside the lock immediately before replacement
|
||||
or removal and uses identity-checked teardown. Deferred route tests prove both orderings:
|
||||
Resume then Close/Delete finishes removed with no post-removal bootstrap event; Close then Resume
|
||||
creates only after Close completes; DELETE then Resume cannot recreate a deleted session.
|
||||
- AppShell assigns each Resume invocation a monotonic token and records the latest target. A
|
||||
completion for a different, superseding session id cannot reset the store, select a source, close
|
||||
the panel, or repaint phase from a late manifest. Same-id invocations are per-target single-flight
|
||||
operations through the POST and local binding commit: repeated pre-commit clicks update the
|
||||
shared operation's latest token but issue no second POST or commit path. The operation becomes
|
||||
joinable again before its manifest fetch, whose repaint remains token/id/selection guarded. Start
|
||||
new, Stop, streamed session exit, and active-session deletion invalidate pending Resume work.
|
||||
This prevents stale-source preservation and reverse/non-Resume intent overwrite without allowing
|
||||
a slow manifest to suppress a later explicit rebind.
|
||||
- `SseHub` subscriber registrations now carry idempotent transport-close callbacks. `clear` and
|
||||
`forget` snapshot and actively close every response before discarding runtime transport state;
|
||||
callback-driven unsubscription during that iteration is safe. The SSE route ends its response so
|
||||
native EventSource reconnects with `Last-Event-ID`. Post-clear events retain monotonic ids and are
|
||||
buffered for replay; `forget` additionally resets the id state.
|
||||
|
||||
Production files:
|
||||
|
||||
- `backend/src/pi/pi-process-manager.ts`
|
||||
- `backend/src/routes/sessions.ts`
|
||||
- `backend/src/sse/sse-hub.ts`
|
||||
- `frontend/src/shell/AppShell.tsx`
|
||||
|
||||
Regression tests:
|
||||
|
||||
- `backend/test/pi-process-manager.test.ts`
|
||||
- `backend/test/routes-sessions.test.ts`
|
||||
- `backend/test/sse-hub.test.ts`
|
||||
- `backend/test/sse-route.test.ts`
|
||||
- `frontend/src/shell/AppShell.session-mgmt.test.tsx`
|
||||
|
||||
### TDD RED/GREEN evidence
|
||||
|
||||
Runtime identity API RED:
|
||||
|
||||
```text
|
||||
cd backend && npx vitest run test/pi-process-manager.test.ts -t "identity-checked teardown"
|
||||
|
||||
Test Files 1 failed (1)
|
||||
Tests 1 failed | 39 skipped (40)
|
||||
TypeError: mgr.teardownIfCurrent is not a function
|
||||
```
|
||||
|
||||
Runtime identity API GREEN:
|
||||
|
||||
```text
|
||||
Test Files 1 passed (1)
|
||||
Tests 1 passed | 39 skipped (40)
|
||||
```
|
||||
|
||||
Deferred bootstrap RED:
|
||||
|
||||
```text
|
||||
cd backend && npx vitest run test/routes-sessions.test.ts \
|
||||
-t "stale bootstrap|bootstrap that"
|
||||
|
||||
Test Files 1 failed (1)
|
||||
Tests 6 failed | 35 skipped (41)
|
||||
|
||||
close/delete + replacement: stale continuation removed the replacement runtime
|
||||
delete without replacement: stale continuation called failSession after forget
|
||||
```
|
||||
|
||||
Deferred bootstrap GREEN:
|
||||
|
||||
```text
|
||||
Test Files 1 passed (1)
|
||||
Tests 6 passed | 35 skipped (41)
|
||||
```
|
||||
|
||||
Shared lifecycle ordering RED:
|
||||
|
||||
```text
|
||||
cd backend && npx vitest run test/routes-sessions.test.ts \
|
||||
-t "Resume followed|Close followed|Delete followed"
|
||||
|
||||
Test Files 1 failed (1)
|
||||
Tests 4 failed | 41 skipped (45)
|
||||
|
||||
All four deferred assertions observed the competing route settle before the first lifecycle
|
||||
operation released.
|
||||
```
|
||||
|
||||
Bootstrap plus lifecycle GREEN:
|
||||
|
||||
```text
|
||||
cd backend && npx vitest run test/routes-sessions.test.ts \
|
||||
-t "Resume followed|Close followed|Delete followed|stale bootstrap|bootstrap that"
|
||||
|
||||
Test Files 1 passed (1)
|
||||
Tests 10 passed | 35 skipped (45)
|
||||
```
|
||||
|
||||
Competing frontend Resume RED:
|
||||
|
||||
```text
|
||||
cd frontend && npx vitest run src/shell/AppShell.session-mgmt.test.tsx \
|
||||
-t "competing Resume|stale Resume manifest"
|
||||
|
||||
Test Files 1 failed (1)
|
||||
Tests 2 failed | 16 skipped (18)
|
||||
|
||||
reverse POST completion opened a second, stale EventSource
|
||||
late s1 manifest repainted the selected s3 phase from F3 to F7
|
||||
```
|
||||
|
||||
Competing and same-id Resume GREEN:
|
||||
|
||||
```text
|
||||
cd frontend && npx vitest run src/shell/AppShell.session-mgmt.test.tsx \
|
||||
-t "competing Resume|stale Resume manifest|false then true"
|
||||
|
||||
Test Files 1 passed (1)
|
||||
Tests 3 passed | 15 skipped (18)
|
||||
```
|
||||
|
||||
### Independent-review hardening RED/GREEN
|
||||
|
||||
The first final review reported no Critical findings and three Important edge cases: bootstrap
|
||||
could start during an in-progress Close; bootstrap-owned failure was persisted twice; and an older
|
||||
same-id result could overwrite newer state. The integrated reviewer also required non-Resume
|
||||
navigation to invalidate pending Resume work. The final main review tightened the same-ID contract
|
||||
to true single-flight so a second same-target click cannot preserve a dead pre-restart source.
|
||||
|
||||
Backend review RED:
|
||||
|
||||
```text
|
||||
cd backend && npx vitest run test/routes-sessions.test.ts \
|
||||
-t "Close suppresses|bootstrap failure persists once"
|
||||
|
||||
Test Files 1 failed (1)
|
||||
Tests 2 failed | 45 skipped (47)
|
||||
|
||||
deferred configure started Pi while closeSession was still pending
|
||||
bootstrap/public failure called failSession twice
|
||||
```
|
||||
|
||||
Backend review GREEN:
|
||||
|
||||
```text
|
||||
Test Files 1 passed (1)
|
||||
Tests 2 passed | 45 skipped (47)
|
||||
```
|
||||
|
||||
Same-id single-flight RED:
|
||||
|
||||
```text
|
||||
cd frontend && npx vitest run src/shell/AppShell.session-mgmt.test.tsx \
|
||||
-t "share one cold request"
|
||||
|
||||
Test Files 1 failed (1)
|
||||
Tests 1 failed | 18 skipped (19)
|
||||
|
||||
two concurrent same-ID invocations issued two cold POSTs (three total including initial activation)
|
||||
```
|
||||
|
||||
Non-Resume invalidation RED:
|
||||
|
||||
```text
|
||||
cd frontend && npx vitest run src/shell/AppShell.session-mgmt.test.tsx \
|
||||
-t "starting a new question invalidates"
|
||||
|
||||
Test Files 1 failed (1)
|
||||
Tests 1 failed | 19 skipped (20)
|
||||
|
||||
the late Resume opened an EventSource after Start new returned to the landing state
|
||||
```
|
||||
|
||||
Frontend review GREEN:
|
||||
|
||||
```text
|
||||
cd frontend && npx vitest run src/shell/AppShell.session-mgmt.test.tsx \
|
||||
-t "share one cold request|competing Resume|stale Resume manifest|starting a new question invalidates"
|
||||
|
||||
Test Files 1 passed (1)
|
||||
Tests 4 passed | 15 skipped (19)
|
||||
```
|
||||
|
||||
Post-commit single-flight lifetime RED:
|
||||
|
||||
```text
|
||||
cd frontend && npx vitest run src/shell/AppShell.session-mgmt.test.tsx \
|
||||
-t "releases same-id single-flight"
|
||||
|
||||
Test Files 1 failed (1)
|
||||
Tests 1 failed | 19 skipped (20)
|
||||
|
||||
s1 committed and waited on its manifest; after s3 superseded it, a new s1 Resume reused the old
|
||||
operation and issued no second s1 POST (expected 2, received 1).
|
||||
```
|
||||
|
||||
Same-id and manifest lifetime GREEN:
|
||||
|
||||
```text
|
||||
cd frontend && npx vitest run src/shell/AppShell.session-mgmt.test.tsx -t "same-id|manifest"
|
||||
Test Files 1 passed (1)
|
||||
Tests 4 passed | 16 skipped (20)
|
||||
|
||||
cd frontend && npx tsc -b
|
||||
no output, exit 0
|
||||
```
|
||||
|
||||
Multi-client SSE disconnect RED:
|
||||
|
||||
```text
|
||||
cd backend && npx vitest run test/sse-hub.test.ts test/sse-route.test.ts
|
||||
|
||||
Test Files 2 failed (2)
|
||||
Tests 3 failed | 6 passed (9)
|
||||
|
||||
clear/forget invoked zero of two registered close callbacks, and two live HTTP SSE responses timed
|
||||
out instead of reaching EOF after clear.
|
||||
```
|
||||
|
||||
Multi-client SSE disconnect GREEN:
|
||||
|
||||
```text
|
||||
cd backend && npx vitest run test/sse-hub.test.ts test/sse-route.test.ts
|
||||
Test Files 2 passed (2)
|
||||
Tests 9 passed (9)
|
||||
|
||||
cd backend && npx tsc --noEmit -p .
|
||||
no output, exit 0
|
||||
```
|
||||
|
||||
The Hub tests use two subscribers whose close callbacks immediately unsubscribe themselves, proving
|
||||
safe snapshot iteration and exactly-once closure. The live-route test opens two HTTP streams, proves
|
||||
both receive EOF on clear, publishes a new event and gate, then reconnects after id 1 and replays
|
||||
exactly ids 2 and 3. The forget test closes both subscribers and proves the next id resets to 1.
|
||||
|
||||
Close now removes the observed runtime identity before awaiting persistence. Failure persistence is
|
||||
claimed once per runtime and lifecycle-serialized; bootstrap's public `session_failed` cannot start
|
||||
a duplicate. A per-target in-flight map owns the only same-ID POST and commit while its mutable
|
||||
latest token keeps s1→s2→s1 ordering correct; it is removed immediately after the binding commit,
|
||||
before awaiting the independently guarded manifest. One shared invalidation helper is called when
|
||||
active deletion, streamed exit, Start new, or Stop begins.
|
||||
|
||||
### Focused verification
|
||||
|
||||
```text
|
||||
cd backend && npx vitest run test/routes-sessions.test.ts test/pi-process-manager.test.ts \
|
||||
test/sse-hub.test.ts test/sse-route.test.ts
|
||||
Test Files 4 passed (4)
|
||||
Tests 96 passed (96)
|
||||
|
||||
cd backend && npx tsc --noEmit -p .
|
||||
no output, exit 0
|
||||
|
||||
cd frontend && npx vitest run src/shell/AppShell.session-mgmt.test.tsx \
|
||||
src/shell/AppShell.new-session.test.tsx src/stream/useSessionStream.test.tsx
|
||||
Test Files 3 passed (3)
|
||||
Tests 36 passed (36)
|
||||
|
||||
cd frontend && npx tsc -b
|
||||
no output, exit 0
|
||||
```
|
||||
|
||||
### Full verification
|
||||
|
||||
```text
|
||||
cd backend && npx vitest run
|
||||
Test Files 22 passed (22)
|
||||
Tests 202 passed (202)
|
||||
|
||||
cd frontend && npx vitest run
|
||||
Test Files 43 passed (43)
|
||||
Tests 280 passed (280)
|
||||
|
||||
cd backend && npm run build
|
||||
> tsc -p tsconfig.json
|
||||
exit 0
|
||||
|
||||
cd frontend && npm run build
|
||||
> tsc -b && vite build
|
||||
✓ 4835 modules transformed.
|
||||
✓ built in 8.47s
|
||||
exit 0
|
||||
```
|
||||
|
||||
The frontend suite/build retain the previously documented MSW, React ref/`act`, experimental type
|
||||
stripping, and large-chunk warnings. No warning class was introduced by this wave. No harness,
|
||||
workflow, persistence, SQL/CTE viewer, model-selection, or deployment file changed. The four
|
||||
pre-existing modified `.superpowers/sdd/{progress,task-2-report,task-3-report,task-4-report}.md`
|
||||
files remain excluded from staging.
|
||||
|
||||
### Final independent-review verdict
|
||||
|
||||
After the multi-client transport fix, the independent reviewer reported no Critical, Important, or
|
||||
Minor findings. Its own focused verification passed 96 backend transport/lifecycle tests, 31
|
||||
frontend Resume/stream tests, both TypeScript checks, and `git diff --check`. Final assessment:
|
||||
**Ready to deploy: Yes.**
|
||||
|
||||
Reference in New Issue
Block a user