Verdict: APPROVE for canary deploy. REQUEST CHANGES held open only for the main merge, pending the eee686a exact CI receipt with the race step. Reviewer: Claude Fable 5.1, model ID claude-fable-5-1. Read-only, nothing run or written.
The test is correct and proves what it claims. Traced TestHomeConnectionKeepsCurrentCallAndAgentRoutes in internal/meet/home_release_test.go:16 against the handlers:
- Home-method session is real. The fixture's link helper ends with a fresh assertion exchange after sign-out, so the session method is Home. Every later route passes
requirePerson, whose Home branch resolves through the link store and membership check atinternal/meet/home.go:85. - Probe 204 is the fresh-open path. The probe goes through standing, then the hub's live-session check at
internal/meet/stream.go:259. That proves the hub session key derives correctly from the Home cookie plus subject. - The SSE hello is a real HTTP stream. The httptest server carries the Home cookies and the app host. The handler flushes each frame, so the scanner sees the hello without waiting on a heartbeat. The 5-second context bounds a hang.
- Admission is proven by ICE, not by the join's 200. The signal handler answers 200 for a full mesh too, but the ICE route calls hub Authorize at
internal/meet/ice.go:103, which refuses anything not admitted. So the ICE 200 is the admission proof, and it is the same session key the stream minted. - API and CSRF assertions hold. Bearer GET and POST with no Origin succeed. Cookie-only API answers 401. A cookie-bearing browser POST without Origin answers 403 under the Home middleware.
- Sign-out revokes and the holding page redirects afterwards.
Delta claim spot-checked. The prior review's 2ed8bcf source has no httptest server in the Home tests. The r2 source does. I could not diff the two trees without git, so "runtime identical" rests on your statement.
Gates as I read them:
| Gate | Status | |---|---| | Exact CI 2ed8bcf, including race | passed, receipt present | | Browser gate | PASS, 523s, log carries no SHA | | make ci r2 on eee686a | exit 0, but no race step and no SHA in the log | | Exact CI eee686a with race | pending |
The race step matters for this delta specifically. The new test runs a stream goroutine on a live server while the test goroutine calls signal and ICE against the same hub. That is the concurrency AGENTS.md says make ci cannot see. Local make ci does not include race, per the Makefile.
Missing coverage, all non-blocking:
- No assertion after sign-out that stream, signal or ICE for the still-live hub participation are refused. A single probe with the peer after sign-out expecting 303 would close it.
- No resume probe under Home, meaning the query form with a peer that reaches CanResume at
internal/meet/stream.go:187. - The test never pins the session method directly. It is implied by the fixture, and pinned only by the separate disabled-Home test.
- The fixture bypasses
rastrillo.Handler, so the framework Host check is not exercised. That stays a canary condition. - The ICE 200 with an empty relay config proves authorization only, not credential minting.
Canary conditions before main merge, unchanged from the prior review: exact CI receipt with race on eee686a; with Home on, sign in, open a room, probe answers 204 and ICE answers 200 for the admitted peer at the real edge; unsigned GET on the sign-in path at the real hostname answers 303, not 404.
Exact eee686a CI is now green including race; receipt /home/paulca/.cache/oficina-rollout-20260911/meet-r2-ci.json. Canary deploy underway. No main merge yet; real-edge authenticated evidence still pending.