feat(buzz-relay): NIP-FI stateless enforcement (S3) — upgrade gate, NIP-42 pairing, session lifetime, JWKS warm

[wpfleger96]

What this PR does

Implements NIP-FI stateless enforcement at the relay WebSocket upgrade point (S3 arc milestone). Every connection attempt on a NIP-FI–enabled relay must carry a valid federated identity assertion signed for the connecting Nostr pubkey; the relay validates it before the first frame is read, pairs it with NIP-42 AUTH, and enforces a bounded session lifetime without persistent state.

Core enforcement seam

• Assertion at upgrade (nip_fi_upgrade.rs): identity header is parsed, JWT verified (RS256/ES256/EdDSA), expiry checked, and the Nostr pubkey embedded in the sub claim must match the NIP-42 AUTH pubkey that arrives in the socket within the AUTH timeout. Denial returns exact byte contracts — 401 + WWW-Authenticate: Nostr + "authentication required\n" for missing evidence, 403 + "evidence rejected\n" for malformed or invalid evidence — before bind_community, so denied upgrades pay zero DB cost.
• NIP-42 pairing (nip_fi_gate.rs, audio/handler.rs): after the assertion is validated at upgrade, a SessionAdmissionGate is created with the assertion's expiry deadline and a CancellationToken. The gate is passed to every effect-acquiring path so no write can land after the session expires. AUTH is verified against the assertion pubkey; a mismatched AUTH key receives NOTICE restricted: authorization denied and a socket close.
• Session lifetime (nip_fi_session.rs): three independent deadlines bound each session — token exp, max assertion age from config, and relay max connection lifetime. The first to fire cancels the gate and sends the terminal denied frame.
• JWKS warm/refresh/fail-closed (main.rs, buzz-auth/nip_fi/jwks/mod.rs): ProductionJwksSource maintains a per-issuer snapshot cache with hard deadlines. The relay supervisor runs a per-issuer refresh loop; on startup all issuers warm concurrently. When the cache is cold or past its hard deadline the relay fails closed with 503 authorization unavailable\n. After a hard-dead snapshot, the supervisor drops to a 5-second fast-retry cadence (not the slow warm interval) until recovery.

Archive/join serialization (F2 fix)

commit_participant_join now takes a row-level write lock (SELECT ... FOR UPDATE) on the channel row at step 3. This serializes all join commits against concurrent archive_channel calls — the archive UPDATE blocks on the row lock, closing the READ COMMITTED race on both the Existing and AutoAddRequired paths.

HTTP/2 extended-CONNECT gate (F3 hardening, latent path)

The root WebSocket handler uses a belt-and-suspenders approach. The HTTP/1.1 path (currently the only live WebSocket shape — workspace Axum does not enable http2) is gated pre-extractor via the Upgrade: websocket + Connection: Upgrade header predicate. A second gate point inside the Ok(ws) arm is structural future-proofing: if http2 is ever enabled, any h2 extended-CONNECT shape the extractor accepts but the h1 predicate misses is caught there. Both fire-points sit before bind_community (zero DB cost on denial).

Off-mode compatibility

When BUZZ_NIP_FI_MODE=off (or unset) the relay ignores any identity header, skips all assertion checks, and behaves byte-identically to a build without this feature. Existing NIP-42 AUTH flows are unaffected.

Test coverage

• 41 NIP-FI unit/integration tests covering assertion contracts, NIP-42 pairing, session lifetime, JWKS cadence, and gate behavior
• F1 production-seam test (f1_supervisor_loop_drives_recovery_and_restores_admission): spawns the real run_jwks_refresh_supervisor — the exact function the production spawn calls — with a ProductionJwksSource<ToggleJwksFetcher> shared between the supervisor and IssuerKeySource. Drives warm → hard-dead → fast-retry (5 s cadence) → recovery with paused Tokio time; asserts IssuerKeySource::key_set returns Some after the supervisor drives the recovery. Three mutation oracles each turn the test red deterministically: (1) remove run_jwks_refresh_inner_task call from run_jwks_refresh_supervisor → no fetch → timeout Elapsed; (2) revert jwks_next_retry_after_failed_refresh to 300-s cadence → elapsed=300s > 10s; (3) ToggleJwksFetcher::fetch_jwks always returns Err → cache never warms → key_set stays None.
• F2 serialization tests (in postgres_tests, gated #[ignore = "requires Postgres — runs in postgres-ci nextest lane"]):
  • f2a_archived_channel_rejects_existing_join: archive committed before join starts → join rejects on the Existing path; removing the archive re-check returns Ok(_)
  • f2b_join_for_update_blocks_concurrent_archive_existing_path: drives the real commit_participant_join via the before_archive_recheck hook; proves the FOR UPDATE lock blocks a concurrent archive UPDATE (55P03) until the join commits (Existing path, join-commits-first ordering)
  • f2c_join_for_update_blocks_concurrent_archive_auto_add_path: same as F2b for the AutoAddRequired two-channel path
• 7 router gate tests covering h1 WS upgrade, NIP-11 content negotiation, and audio ingress
• Live E2E verified by Gurney at 9f0acac19: all admission, pairing, lifetime, JWKS, off-mode, and restart lanes passed

[github-actions]

🔐 Codex Security Review

Status: review required for the current range.

The current range is 44316ff72f5f7de014c66b01cbf534298a70c249...a9e091a712916ffcbca5bcf4e7cd1084a664071f.
A new review must complete for this exact range. When manual authorization
is required, a Block organization member must comment exactly
@buzz-security-review a9e091a712916ffcbca5bcf4e7cd1084a664071f to authorize a new review.
Any previous review applies only to its recorded range.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested

Reviewed exact head cbd0ded50af716f6a7c071a940e7394ffc761b77 against base 04babf02655440b4dfd37f2e2df605ead0a030d8. Source/metadata only: no checkout, build, tests, or PR-code execution. Independent configuration/JWKS, session/auth, and audio/discovery lanes are integrated.

The delivery contract is stateless S3 WebSocket enforcement: fail-closed upgrade, matching NIP-42 proof, bounded session authority, and private discovery, while preserving ordinary document serving. HTTP bridge enforcement and admin deny/boot belong to the separately planned S5/S4 work and are not blockers here.

1. P1: Install audio expiry enforcement before authentication and admission

Changed timer placement: audio/handler.rs:824–837

Upgrade with a valid assertion expiring in one second, then send matching-key AUTH after two seconds, still inside the five-second AUTH window. No FI timer is running during AUTH or setup. Pairing checks only the key, and calculating an expired deadline does not reject it (302–326). The connection proceeds through membership auto-add, room admission, roster publication and the awaited participant-joined event before installing the timer. The auto-add and event paths can persist changes (1424–1445, 1523–1526). A slow setup dependency extends this interval.

Enforce the deadline across community bootstrap, AUTH and setup, stopping expired admission and safely cleaning partial setup. Test the real handler with expiry during both AUTH and an admission dependency; assert closure and no new post-expiry membership/room/join side effects. The helper/writer expiry test does not exercise this placement. This violates NIP-FI’s equality-is-expired and maximum-session-bound contract.

2. P2: Fence root frame admission after expiry, not just the socket writer

Expiry integration: connection.rs:373–395; receive/dispatch: 589–610

Expiry cancels the token but leaves AuthState::Authenticated intact. If a buffered EVENT/REQ and cancellation are both ready, the unbiased receive select! can choose the frame and invoke handle_text_message. Neither dispatch, quota admission nor the handlers reject an expired/cancelled session, so it starts a new authorized handler after expiry. This can occur before the writer processes Close; it is not a request to roll back work admitted before the deadline. An assertion that expires during bootstrap likewise has no synchronous deadline guard before AUTH.

Check deadline/cancellation at the actual admission boundary, including AUTH, and prevent late async admission from publishing authenticated state. Add deterministic cancelled-plus-ready-frame and already-expired-bootstrap cases. Keep the fix scoped to stopping new admission, not a general handler-lifecycle rewrite.

3. P2: Preserve the terminal denial when the control queue is full

nip_fi_session.rs:183–195; root pairing: 107–111

Both expiry routes and root pairing discard try_send errors before cancelling. With the capacity-eight control queue full while the writer is temporarily backpressured, the denial is lost. When the writer recovers it drains the old frames and sends Close, but never the required restricted: authorization denied frame. The cancellation drain cannot recover a frame that was never queued.

Preserve the terminal reason independently of ordinary queue capacity, without making expiry depend on an unbounded send. Cover saturated queues for root pairing and both expiry routes, then release the writer and assert fixed denial followed by Close. Current tests only use available capacity.

4. P2: Do not apply the WebSocket gate to ordinary root document requests

router.rs:337–350

With enforce mode, a mapped non-admin host, Git web GUI enabled and a readable bundle, normal browser GET / with Accept: text/html now returns 401 before reaching the existing SPA fallback (404–415). Plain fallback NIP-11 requests also become 401; only explicit application/nostr+json is exempt. This is an unintended document-serving regression, not required WebSocket protection.

Separate non-upgrade document handling from upgrade admission. Test ordinary HTML and fallback JSON requests, while ensuring a genuine upgrade with HTML Accept remains gated. Do not create an Accept-header authentication bypass.

5. P2: Isolate the router fixture’s process-global environment access

router.rs:1437–1444

The new fixture removes NIP-FI variables without the lock used by nip_fi_config tests in the same test binary. One concrete interleaving: enforce_without_issuers_fails_closed sets mode to enforce; this fixture removes it; the config test reads Off and its expect_err fails. The reverse interleaving can make this fixture’s Config::from_env().expect(...) fail. Default parallel test execution therefore has a new order-dependent failure path.

Use a shared lock for the mutation/read window in all affected tests, or construct the fixture without process environment. Preserve the existing parallel suite behavior rather than relying on serial test flags.

Startup validation, per-issuer JWKS refresh/snapshot bounds, unconditional key pairing, and static private discovery otherwise hold in the inspected source. DenyProtected’s 503 implementation disagrees with its 403-oriented function documentation; its startup type describes misconfiguration-repair mode, so I am not treating that ambiguity as an additional blocker. Align and pin that contract separately.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested

Reviewed head 7d87ab3ac0b623522b2532c260c6a08a4945b57e against base 187df22252fa24cce2f3295fb9df9f4dc211b30a, including the changes since the previous reviewed head. The terminal-denial queue saturation and ordinary-document interception findings are addressed. Three prior findings remain, and the router reordering introduces one test-path regression.

P1: Audio still begins room admission after the session deadline

The new pairing-time check covers an assertion already expired when AUTH finishes, but the expiry task is still installed only after admission and lifecycle publication.

Concrete source-derived interleaving, with mesh disabled and an otherwise authorized member: pairing passes just before deadline D; the pre-join channel read returns after D; room.add_peer then begins a new admission, followed by joined/participant publication, without another deadline check. The expired connection enters the room before the timer can close it. This is new work admitted after expiry, not a request to roll back work admitted before expiry.

Enforce the effective deadline throughout AUTH/setup and fence room admission after awaited dependencies. Keep cleanup ownership for any already-acquired room/remote/lease resources rather than simply dropping the whole handler on timeout. Add a regression with a successful setup dependency deliberately spanning D; the new already-expired-at-pairing test does not cover it.

P2: Root dispatch still admits buffered requests after expiry

The added AUTH-finalization cancellation check does not protect an already-authenticated connection. recv_loop still makes an unbiased choice between a ready buffered frame and cancellation. Once the expiry task has cancelled the token, it may select a new EVENT/REQ; dispatch has no deadline/cancellation fence, and the quota gate and handlers still accept the unchanged Authenticated state. A valid event can therefore reach persistence after being newly admitted on an expired session.

Check cancellation and the absolute deadline at the root admission boundary, including AUTH before side-effecting gates; do not rely only on a separately scheduled timer. Cover cancelled-plus-buffered EVENT/REQ and immediately-ready expired AUTH through the production boundary. The current B2 test explicitly fails earlier at its lazy-DB ban lookup, so removing the new guard would not falsify it.

P2: The two ENV_LOCK statics do not serialize the shared environment

router.rs declares a new module-local mutex, distinct from nip_fi_config.rs's mutex. The comment calling them shared does not make them the same lock. For example, enforce_without_issuers_fails_closed can set MODE=enforce, the router fixture can remove it under its different mutex, and the config test then reads Off and fails expect_err. The reverse interleaving can make router configuration fail.

Remove process-environment mutation from these fixtures, or use one shared synchronization mechanism across the affected reads and writes. This remains a parallel-suite race, not a process-metadata concern.

P2: Root router tests now hit the unseeded database before their asserted gate

The gate moved below bind_community, but nip_fi_enforce_state still creates a lazy DB pool without a relay.example community fixture. On the intended no-infrastructure setup, or a database without that host, binding returns the generic 404 before NIP-FI or the document fallback is reached. The existing root missing-assertion/no-verifier tests expect 401/503, while the added plain-GET and HTML-Accept upgrade tests expect 200/401. These fail independently of the environment race and do not exercise their advertised seams.

Keep the genuine-upgrade gate before the tenant lookup where appropriate, or supply a controlled successful host-resolution fixture for tests that need the document/upgrade path. Do not change the assertions to accept the early 404: that would stop testing the fixes.

Scope and validation

Source/metadata-only review; no checkout, build, test, import, or PR-code execution. The interleavings above are source-derived, not executed reproductions. Applied exact-base product, architecture, AGENTS and TESTING guidance. HTTP data-plane enforcement remains S5; administrative disconnect/deny-set remains S4. Neither is a blocker for this S3 review. Dedicated terminal queues address ordinary-control saturation; document requests now bypass the FI gate, subject to the existing tenant boundary. Closeout requires the four concrete issues above, not a general lifecycle rewrite or rollback of previously admitted work.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested

Reviewed exact head 23f43850459156c3e180d79c3235810cf6b3d892 against base 187df22252fa24cce2f3295fb9df9f4dc211b30a, including the delta from previously reviewed 7d87ab3a. The new root cancellation check addresses the previously reported cancelled-plus-buffered-frame case, and audio now arms expiry before post-pairing admission. However, the new header condition introduces an assertion bypass, and the new audio cancellation return skips committed-join cleanup. Pending-auth lifetime and two fixture defects remain.

1. P1: Apply FI to every request the WebSocket extractor can accept

router.rs:381–403

A direct HTTP/1.1 GET / to a mapped, active host with ordinary WebSocket key/version headers, Upgrade: websocket, Connection: xupgrade, and no FI assertion skips this gate: the new token-equality test is false, so nip_fi_assertion becomes None. But pinned Axum 0.8.9 accepts Connection by substring, not token equality. Its extractor accepts this value; pinned Hyper 1.9.0 creates upgrade intent from the Upgrade header, and Axum constructs the WebSocket without another handshake validation.

Thus this reaches handle_connection with no assertion. Pairing treats None as not applicable, and no FI deadline is armed. A key satisfying ordinary NIP-42/local policy can establish a session without federated evidence in both Enforce and DenyProtected modes. This is an enforcement bypass, not merely invalid RFC syntax.

Do not use a narrower predicate to skip authorization than the actual upgrader uses. Gate every successful extraction before returning its upgrade response, or reject malformed candidates rather than passing them to a more permissive extractor. Preserve ordinary HTML/NIP-11 document behavior. Add a real upgrade regression with Connection: xupgrade in both modes; it must never reach 101 without the required FI decision.

2. P2: Finish teardown after a join has already been published

audio/handler.rs:875–900

On the local-owner path, the handler broadcasts joined, then awaits the persisted/fanned-out PARTICIPANT_JOINED event. If expiry fires during that await, the new check_cancel!(cleanup: ...) removes the peer and returns. It bypasses the normal left broadcast, PARTICIPANT_LEFT event, last-peer archive/end event, and owner release at 1066–1169.

Room::remove_peer publishes an internal roster delta, not the JSON left sent to same-pod clients. Existing peers and durable huddle history therefore retain a participant that has disconnected; a sole-peer huddle also skips normal auto-end. Route post-published-join cancellation through the complete teardown, retaining generation/ownership fences. Pause lifecycle emission across expiry in a production-path regression and assert join/leave symmetry and last-peer cleanup. The added mid-admission test pre-cancels before AUTH and does not reach this boundary.

3. P2: Cover pending authentication with the session deadline

audio/handler.rs:320–356

The timer is now before room admission, but still after the challenge write, five-second AUTH wait, and NIP-42 verification (231–290). Upgrade with a valid assertion having one second remaining and withhold AUTH: the socket survives until the independent five-second timeout, then drops without the FI expiry denial because no expiry task was created. The community-active await before this handler has the same unbounded pre-timer gap on both WebSocket routes.

NIP-FI Session policy requires termination at the earliest effective deadline, including a short configured maximum connection lifetime. Enforce that bound from upgrade through bootstrap and pending AUTH, while preserving cleanup ownership. Add a near-expiry/no-AUTH case, not just an already-expired assertion checked after successful pairing. This is the remaining lifetime portion of the earlier finding; the specific post-pairing room-entry race is addressed.

4. P2: Remove ambient NIP-FI reads from the router fixture

router.rs:1468–1477

Removing the fixture's environment mutations does not isolate its reads. Config::from_env().expect(...) still invokes fallible NipFiRelayConfig::from_env() (config.rs:1268) before the explicit override. In parallel, nip_fi_config tests set MODE=enforce without issuers or MODE=permissive under their private mutex (386–389, 418–419). The router reader takes neither lock, so it can panic before constructing the fixture. Use an environment-independent constructor or genuinely shared synchronization covering readers and writers. The current comment claiming these reads are irrelevant is incorrect.

5. P2: Make the root gate tests reach the intended route boundary

router.rs:1484–1514

The fixture still creates a lazy DB pool without seeding relay.example. Root binding at 344–355 precedes FI, so a missing/unavailable database or missing host returns 404 before the gate or document fallback. The tests at 1550, 1574, 1647, and 1674 expect 401/503/200/401 respectively. The new single-header negative tests can instead pass vacuously on that same 404. Explicit NIP-11 Accept negotiation and audio have different ordering and are not affected by this particular failure.

Provide controlled successful host resolution through the production route seam, or a real seeded tenant. Do not weaken expectations to accept 404. Verify both positive denial/document cases and malformed-header cases actually reach their intended decision. This remains independently broken even after fixing the environment race.

6. P2: Use the canonical FI denial for local-policy rejection

Root pairing integration, audio pairing integration

After a valid assertion and matching NIP-42 proof, root still returns ban-specific blocked: you are banned from this community (auth.rs:175–200) or restricted: not a relay member (245–255). Audio likewise distinguishes relay membership from channel membership (handler.rs:406–449). The new FI path therefore exposes which private local policy rejected the same supplied evidence, rather than the fixed restricted: authorization denied required by NIP-FI's rejection table (623–634).

Keep legacy responses when FI is off, but route FI local-policy admission failures through the canonical denial path. Test matching-key policy denials against the same public text/frame contract used for pairing mismatch. This is a concrete gap in the new FI integration, not a request to change off-mode UX.

Scope, evidence, and exit criteria

Source/metadata only: immutable blob verification, exact dependency-source inspection, and three independent lanes integrated. No checkout, build, tests, or PR code executed. Scenarios above are source-derived, not runtime reproductions. JWKS/config/verifier wiring and private NIP-11 capability shape were reviewed; HTTP data-plane enforcement remains S5 and issuer disconnect remains S4.

The exit criteria are the six bounded issues above. Existing detached-handler cleanup debt is not a new authority-bypass finding, and already-admitted work need not be rolled back. The dedicated terminal queue addresses the previously agreed ordinary-queue saturation case. Separately, a stalled socket writer can still delay shutdown; that broader existing lifecycle limitation is noted as follow-up rather than expanding that fix into another blocker here. No general handler-lifecycle rewrite is requested.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested

Reviewed head 4de38d1dc883aa48d3b8f7486309f62407fe5bfe against base 88687876f7808a2fd742b7eb2e4b9f87d999ad8d, reconciling the prior review at d4953871. This is a source/metadata-only review: no checkout, build, tests, or PR-code execution. Reproduction schedules below are source-derived, not runtime results.

Corrections credited: the successful-extractor FI backstop closes the prior assertion-free root socket bypass; audio now arms expiry before the NIP-42 wait/verification; newly acquired lease and remote-owner teardown cleanup improved; the specific postcommit remote direct-send early return is removed. These findings are not being repeated unchanged. Six prior findings remain or are partly repaired, and two current-base integration regressions need fixing.

1. P2: Preserve generation in the new transactional 48101 writer

New event content, base producer, Desktop live JOIN, reconciliation.

The replacement writer emits ephemeral_channel_id, roster_revision, and admission_id, but drops the generation that the exact base already emits. For an already-hydrated observer watching a new huddle, START followed by this live JOIN sets activeSessionGenerations[session] to "pending". The next liveness refresh (10-second cadence) returns the real process/mesh generation. reconcileLiveness(real, previous) sees "pending" != real and clears the just-admitted participants. The live participant disappears even though the room remains active. LEFT/ENDED still carry generation, so this also breaks the common producer/consumer generation contract.

Thread the already-computed lifecycle_generation into the transactional event content; retain admission-ID matching and the existing transaction. Cover live START/JOIN followed by the first liveness refresh, not just event row counts.

2. P2: The new FOR UPDATE creates an FK/advisory-lock inversion

Join row lock, membership advisory lock, membership INSERT, foreign key.

A fresh add_member transaction takes the channel membership advisory lock, then its INSERT needs a foreign-key KEY SHARE lock on the referenced channel. An AutoAddRequired join now takes FOR UPDATE on that channel first, then waits for the same advisory lock. Interleave them after the member transaction obtains the advisory lock and before its INSERT: the join waits for the member transaction, while the member INSERT waits for the join's incompatible row lock. PostgreSQL must abort a legitimate operation as a deadlock victim.

The same lock change invalidates the CW5-variant schedule: it pauses the join after FOR UPDATE at the pre-membership-lock hook, then awaits an external fresh member INSERT before releasing that hook (4690–4716). That INSERT cannot complete while the paused join owns the row lock.

Use compatible locking while preserving archive serialization, e.g. FOR NO KEY UPDATE for this non-key channel-state check, and cover a real concurrent membership writer. archive_channel itself is a plain read followed by UPDATE; no additional archive advisory-lock defect is asserted.

3. P2: FI lifetime enforcement still starts after community bootstrap

Root bootstrap, audio bootstrap, shared await.

The pending-audio-AUTH portion is fixed: the gate/task now precede that wait and verification. Both socket wrappers still await is_community_active before entering the active handler that creates the FI deadline task. Upgrade with a valid assertion having one second left, then delay that DB check beyond its deadline: neither socket enforces FI expiry while bootstrap is pending. Capturing connection_time at HTTP upgrade corrects the arithmetic but does not close the socket during this await.

Enforce the same earliest deadline across bootstrap, without changing the rule that already-admitted bounded effects may finish. Add a delayed-bootstrap witness for both routes.

4. P2: FI private-policy and expiry denials remain distinguishable

Root ban denial, root membership denial, audio policy denial, public contract.

A valid assertion paired with the correct NIP-42 key still receives blocked: you are banned from this community versus restricted: not a relay member; audio additionally distinguishes channel membership. These reveal which private local policy rejected the same supplied evidence, rather than the fixed restricted: authorization denied. EVENT, REQ, and COUNT gate rejections also emit restricted: session expired when the handler detects expiry before the timer's canonical denial (e.g. EVENT:702–710, REQ:223–227, COUNT:113–117).

Keep legacy off-mode UX, but use the established FI denial for FI local-policy/expiry rejection. Cover matching-key private-policy denials, not only key mismatch.

5. P2: Router fixtures still read fallible ambient FI config before overriding it

Fixture, Config parser, concurrent environment mutations.

Config::from_env().expect(...) still invokes NipFiRelayConfig::from_env() before the explicit override. Config tests temporarily select Enforce without required fields or the invalid permissive mode under a mutex this reader does not take. A parallel test run can panic before reaching the override. Removing fixture writes did not remove its ambient reads; the comment saying those reads are irrelevant is incorrect.

Use environment-independent fixture construction or synchronization shared by the actual readers and writers. Apply the same fix to the new fixtures using this fallible constructor.

6. P2: The no-Accept GET fixture still cannot reach the intended fallback

Unseeded lazy state, request/assertion, host binding before fallback.

This fixture sends plain GET / to unseeded relay.example, with neither Accept nor upgrade headers, and requires 200. It bypasses the explicit NIP-11 shortcut and FI precheck, then runs bind_community before the document fallback. Missing mapping or unavailable DB produces 404, not 200. The claimed DB-free fallback witness is therefore not valid; single-header negatives can also succeed on unrelated host-binding failure.

Provide controlled successful host resolution or an isolated seeded tenant. Preserve the no-Accept case; substituting the Accept shortcut or accepting 404 would erase its purpose.

7. P2: Failed pending audio admission still leaks client visibility and reused ownership

Pending snapshot, postcommit broadcast, guard cleanup.

Pause A after add_peer but before its transaction. B snapshots the unfiltered room, commits, and broadcasts a joined containing A. A then expires or fails its commit. Its guard removes A without a correcting client JSON message. room.remove_peer sends an internal roster delta, not the local WebSocket control JSON; Desktop merges the published peers (playout:542–575) and retains A. The new transaction row lock does not prevent A from pausing before its transaction.

Newly acquired lease cleanup is repaired, but reuse remains: committed owner B → pending reuse A → B leaves while A keeps the room nonempty → A fails. A's guard has no newly acquired lease and no registry/generation cleanup. It deletes the now-empty room without releasing the existing renewer (reuse:989–993; normal generation-fenced release only at 1522–1524).

Exclude pending peers from published state or balance their exposed control state, and carry generation-fenced owner cleanup through precommit last-peer removal. Cover both schedules. This is not a claim that A's rolled-back 48101 persists.

8. P2: Real-DB witnesses still bypass the isolated test database

AUTH fixture, audio fixture, new F2 caller, required isolated lane.

F2a/b/c are now correctly ignored under postgres_tests, and fail loudly when their fixture is unavailable. That fixes their discovery/skip behavior, not their database selection: they still call the helper that hard-codes the shared localhost buzz database and overwrites the configured URL. The isolated runner's per-process URLs are ignored. Earlier AUTH/audio DB witnesses, including CW5/CW8/CW10 and the new full-handler disconnect case, remain ordinary non-ignored tests that silently return success if that shared DB is unavailable.

Use the existing test_support::database_url()/runner-supplied isolated database, move all real-DB witnesses into the ignored lane, and fail on infrastructure errors there. Otherwise tests miss the sanctioned lane, mutate shared development state, or report success without exercising their boundary.

Credit, limits, and stable exit criteria

• New observer EVENT, huddle-liveness REQ, and search REQ branches hold effect permits across their bounded work. JWKS startup warming is concurrent; the production refresh supervisor now resets warm→unavailable cadence to five seconds. The new F1 witness drives the real supervisor and ProductionJwksSource cache with a fake fetcher. This is source-level coverage credit, not an executed cryptographic-admission or mutation result.
• CW5-variant adds a discriminating admin role assertion, but finding 2 prevents its current external-insert schedule from completing. CW8 now asserts room-map absence; CW10-full adds committed join/leave counts. Neither proves handler-task completion, local-event-ID publication, or initial remote socket-send failure. CW6/CW7 cover guard helpers; the old remote-only postcommit early return is removed, and its blocker is retired rather than expanded into generic writer-lifecycle work.
• Fix the eight concrete boundaries above and retain their discriminating witnesses. The root backstop still runs after host binding despite the zero-DB-cost comment: extractor-accepted noncanonical h1 headers can reach DB/404 before FI rejection. That is a narrower ordering/documentation residual, not the former session bypass and not an additional blocker here. HTTP S5, issuer disconnect/deny-set S4, generic stalled-writer/disconnect debt, and unrelated merged-base features remain outside this corrective review. Preserve already-admitted bounded effects; no general lifecycle rewrite is requested.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested

Reviewed head 488c73ffa4d2cd6b58c4f4b5b91a47e64b509d6b against base 4d447b9c20a23fb33c94778e6cf309424abea6c8, reconciling the prior review at 4de38d1d. Source/metadata only: no checkout, build, tests, or PR-code execution. The schedules below are source-derived, not executed reproductions.

Credit: the audio DB fixture now honors BUZZ_TEST_DATABASE_URL, which the isolated runner supplies. F2a/b/c therefore no longer bypass that runner’s database. The seven other findings remain; finding 8 is narrowed to the remaining AUTH URL and DB-test discovery/skip behavior. No new blocker is added in this corrective review.

1. P2: Transactional JOIN still drops the lifecycle generation

The 48101 writer omits generation, although the exact-base producer supplied it. For an already-hydrated observer watching a new huddle, START followed by live JOIN sets the active generation to "pending". The first 10-second liveness refresh returns the real generation; reconciliation clears the admissions on that mismatch. Active participants disappear from presence while the room remains live.

Pass the already-computed lifecycle_generation into the transactional content, retaining the transaction/admission ID. Cover live START/JOIN followed by first liveness refresh.

2. P2: Channel FOR UPDATE still inverts the membership lock order

The join takes a channel row lock before the membership advisory lock. A fresh add_member takes that advisory lock first, then its INSERT needs FK KEY SHARE on the referenced channel. Pause the member transaction after the advisory lock, let an AutoAddRequired join acquire FOR UPDATE, then resume the INSERT: each waits on the other, and PostgreSQL aborts a legitimate operation.

The CW5 variant also awaits that external INSERT while the join is paused holding the incompatible row lock, so its witness cannot complete as written. Use compatible locking, e.g. FOR NO KEY UPDATE, preserving archive serialization, and cover a real concurrent membership writer. No archive advisory-lock inversion is asserted.

3. P2: FI expiry still starts after community bootstrap

Both root and audio await the community-active check before constructing their FI deadline gate/task. Upgrade with a valid assertion having one second left, then delay that DB check beyond expiry: the established socket does not terminate at its deadline. Captured connection_time fixes later arithmetic, not this pending await.

Fence bootstrap with the same earliest deadline on both routes. The already-fixed pending-audio-AUTH timer is not being reopened; preserve completion of already-admitted bounded effects.

4. P2: FI private-policy and expiry denials remain distinguishable

With valid, matching assertion/NIP-42 evidence, root bans emit blocked: you are banned from this community, relay membership emits restricted: not a relay member, and audio distinguishes relay/channel membership. These expose which private policy rejected the principal, contrary to the fixed FI denial contract. Gate-denied EVENT/REQ/COUNT also emit restricted: session expired if their check wins the timer race (EVENT example).

Condition these responses on FI and use the canonical denial; retain legacy Off-mode UX. Cover matching-key private-policy outcomes and handler-detected expiry, not only pairing mismatch.

5. P2: Fixtures still read fallible ambient FI configuration

The router fixture calls Config::from_env().expect(...) before overriding FI. That constructor calls the FI parser, while config tests temporarily set incomplete Enforce or invalid permissive under a mutex the fixture does not acquire. A parallel cargo-test run can panic before the override. The audio/auth fixtures retain the same fallible read.

Use environment-independent fixtures or synchronization shared by the actual readers and writers. Removing fixture mutations alone does not remove this race.

6. P2: The no-Accept GET witness still hits unseeded host binding

The plain-GET test requires 200 for / on relay.example, with no Accept/upgrade header. Its lazy fixture never seeds that host. The request skips the explicit NIP-11 shortcut, then binds the community before fallback; missing mapping or unavailable DB returns 404. This is not a DB-free passing fallback witness, and single-header negatives can pass for the unrelated binding failure.

Provide controlled successful host resolution or an isolated seeded tenant. Keep the no-Accept request; accepting 404 or substituting the Accept shortcut would erase the witness.

7. P2: Failed pending audio admission still leaks visibility and reused ownership

Pause A after add_peer but before its transaction. B’s unfiltered snapshot and joined payload include A; B commits and broadcasts it. A then expires/fails. Its guard removes the peer without corrective client JSON. The room’s internal roster delta is not local WebSocket left/roster; Desktop merges joined peers, retaining phantom A.

Separately: committed owner B → pending owner-reuse A → B leaves while A keeps the room nonempty → A fails. Reuse records the generation outside the guard, but the guard owns no acquired lease/registry cleanup. It removes the last room peer without the normal generation-fenced release, leaving the renewer alive.

Exclude pending peers from published state or balance exposed control state, and carry generation-fenced cleanup through precommit last-peer removal. Cover both schedules. Fresh-acquired lease cleanup remains credited; this is not a claim that a rolled-back 48101 persists.

8. P2: Remaining DB witnesses still miss isolation and the required lane

Partly fixed: audio URL selection now consumes the runner-exported URL, and F2a/b/c are ignored postgres_tests with loud failure. Do not undo that repair.

The AUTH fixture still hard-codes the shared local buzz DB. W1 and the older audio DB witnesses remain non-ignored ordinary tests that return successfully if DB connection fails (W1, W9). They are not selected by the isolated lane’s postgres_tests filter plus --run-ignored ignored-only. Without the runner URL, audio still falls back to the shared development DB and seeds fixture data there; with no DB, the claimed boundary receives no execution despite test success.

Move the remaining real-DB witnesses into the required ignored PostgreSQL lane, use the existing test_support::database_url() contract consistently, and fail on infrastructure errors there. The exact-base testing contract requires per-process isolation.

Scope and stable exit criteria

Fix the eight numbered boundaries, with finding 8 narrowed as above, and retain discriminating production-seam witnesses. Root/audio upgrade → pairing → policy → effect admission/expiry, transactional audio → room/lease teardown → Desktop generation/roster consumers, and fixture/runner paths were reconciled; JWKS/config production sources are unchanged from the prior reviewed head. Previously credited root extractor backstop, pending-AUTH timer, fresh lease cleanup, and removed postcommit remote early return remain credited. HTTP S5, issuer disconnect/deny-set S4, generic stalled-writer/disconnect debt, and unrelated merged-base features remain out of scope. No general lifecycle rewrite is requested.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested

Reviewed head a9e091a712916ffcbca5bcf4e7cd1084a664071f against base 44316ff72f5f7de014c66b01cbf534298a70c249, reconciling the prior review at 488c73ff. Source/metadata only: no checkout, build, tests, or PR-code execution. Every schedule below is source-derived, not an executed reproduction.

Repairs credited: findings 1, 2, 6, and 8 are retired. The transactional 48101 now carries generation; FOR NO KEY UPDATE removes the identified FK/advisory-lock inversion while retaining archive serialization; the no-Accept GET witness now seeds a mapped host; AUTH uses the isolated database contract and the scoped AUTH/audio DB witnesses are ignored PostgreSQL-lane tests with loud infrastructure failure. Audio already honors the runner-exported URL. No claim is made that these tests were run.

Four prior boundaries remain partly repaired. Finding 7 now also has concrete compatibility regressions introduced by its new snapshot filter. The contract remains deadline-bounded FI admission with uniform private-policy denial, atomic audio commit/cleanup, and working released-client rosters, including Off mode.

3. P2: Bootstrap still retains the socket beyond the FI deadline

The timers now start before bootstrap on root and audio; that fixes their placement. But the shared wrapper still unconditionally awaits check_active() at line 204. It neither selects cancellation nor runs a terminal socket path while that await is pending.

Upgrade with a valid assertion having one second left, then delay is_community_active past expiry. The timer cancels the token and queues the denial, but the socket and receiver remain captured in the uncalled run closure. When the DB finally returns, lines 208–209 return without invoking that closure: the socket is dropped late and the queued denial is discarded. A stalled DB therefore controls the expired socket’s lifetime. Arming a task is not the same as terminating the connection.

Make bootstrap cancellation-aware and preserve a denial-drain/close path without admitting an inactive community. Cover delayed bootstrap through both outer socket wrappers, not only handle_active_*. Preserve completion of already-admitted bounded effects.

4. P2: Matching-evidence local-policy denials are only partly normalized

Ban, initial relay/channel membership, and EVENT/REQ/COUNT gate-expiry text are repaired. The root pubkey allowlist branch still returns auth-required: verification failed for a valid matching FI assertion and NIP-42 key when pubkey_allowlist_enabled=true and that key is not allowed. Other local-policy failures now return restricted: authorization denied, exposing which policy rejected the same evidence, contrary to the privacy contract.

The audio transactional parent-membership rejection also still emits error: not a member: an AutoAddRequired join that passes initial admission and loses parent membership before the transaction’s recheck takes this branch.

Use the canonical FI denial on these remaining policy exits, retaining legacy Off-mode messages. Cover matching-key allowlist denial and parent-membership loss at the existing transaction seam, rather than only pairing mismatch.

5. P2: AUTH/audio fixtures still race the FI environment writers

The targeted router fixtures now take the shared lock. The AUTH unit fixture and audio unit fixture still call Config::from_env().expect(...) without it. The AUTH DB fixture and audio DB fixture have the same unlocked constructor.

The FI config tests temporarily install incomplete enforce or invalid permissive state under NIP_FI_ENV_LOCK. In a parallel unit run, either unlocked unit fixture can observe that state and panic in the fallible constructor before any fixture override. The constructor’s FI parser call is still unconditional.

Use environment-independent fixtures or synchronize the actual fallible readers with the writers. Credit the router fix, but do not mistake that one reader’s lock for suite-wide isolation.

7a. P1: Successful local joins now omit the joining peer from the delivered roster

The snapshot and joined payload are built before commit. With the new committed-only snapshot filter, B’s own entry is absent from B’s peers[]. The transaction later marks B committed, but broadcasts the unchanged payload.

Ordinary two-party schedule: A is connected; B joins successfully. A receives B’s joined containing A but not B. Desktop merges only joined.peers, not the top-level joining identity, and drops binary audio for unknown roster indices. A therefore drops B’s audio until some later roster update happens to introduce B. This affects normal successful joins even with FI Off.

Build the published roster from the committed state at the publication boundary, including the joining peer while excluding unrelated pending peers. Cover the actual handler-produced payload consumed by an already-connected client. The new F7a helper test manually calls mark_committed; deleting its production caller would not make that test fail, contrary to its mutation comment.

7b. P2: Owner-side mesh snapshots permanently omit remote participants

Owner remote registration still calls room.add_peer, which now initializes committed=false, then returns a filtered PeerRegistered.roster. The only production mark_committed call in the pinned audio/{handler,join,room}.rs sources is in the ingress handler; that marks the ingress pod’s different Room/peer, not the owner’s remote peer.

Consequently, a later joining client’s owner snapshot omits already-live remote participants. Lag recovery and explicit resync omit them too. The ingress forwards that snapshot as a replacement roster; Desktop replaces its index maps and removes missing media state, so recovery can erase live participants and silence their audio.

Snapshot filtering also does not fence the other publication path: add_peer still emits a joined delta immediately, before commit, and owner control turns that delta into client joined JSON. Merely marking remote peers committed at registration would not establish the promised transaction-before-visibility boundary.

Keep authoritative owner/ingress committed state and ordered snapshots/deltas consistent, or choose a smaller publication strategy that preserves the existing mesh contract. Cover an already-live remote peer in a new admission snapshot and in resync, plus pending admission visibility. No general mesh rewrite is requested.

7c. P2: Early post-add expiry still leaks a reused owner lease

The transaction-error cleanup branches now release the matching owner generation; those repairs are credited. The earlier post-add_peer cancellation exit still discards release_before_commit()’s room_cleaned result at line 1010, before owner_generation is assigned.

Schedule: committed owner B; pending A reuses B’s registry entry and is added; B leaves while A keeps the room nonempty; A expires at this early gate. The guard removes A and cleans up the empty room, but it owns no newly acquired lease and this caller never releases the reused registry generation. The renewer remains alive for the empty room.

Carry the already-resolved generation into this early cleanup and honor the last-peer result. Cover this actual caller schedule, including generation fencing. The new F7b test exercises HuddleOwnerRegistry::release directly, not whether this caller invokes it.

Stable exit criteria and limits

Resolve the remaining 3/4/5/7 boundaries above and retain discriminating production-path witnesses. Keep findings 1/2/6/8 retired; do not undo those repairs. The F6 fixture now establishes seeded no-Accept fallback reachability, but does not explicitly select Enforce, so its comments do not prove FI-gate mutation sensitivity.

The independent lanes and integration pass covered root/audio upgrade, pairing and policy, effect admission/expiry, audio transaction/room/lease state, released Desktop roster consumers, and fixture/runner boundaries. Production JWKS recovery remains unchanged from the prior reviewed implementation. Previously credited extractor backstop, pending-AUTH timer, fresh-lease cleanup, and removed remote postcommit early return remain credited. HTTP S5, issuer disconnect/deny-set S4, generic stalled-writer/disconnect debt, and unrelated merged-base features stay outside this corrective review.