[bradseiler]

Summary

When a community starts requiring NIP-FI, the periodic community revalidator now closes its existing legacy root and audio WebSockets with Close 1008. It reads current policy from the writer by immutable community ID, with a 500 ms budget per read. Failed reads retry on later sweeps; late registrations are caught by the next pass.

Enforced FI admissions survive. Optional-shadow admissions remain legacy. Off and repair mode retain their existing behavior. No client request or Redis notification is needed.

The default sweep interval is 30 seconds, not a deadline during database failure, overload or paused processes. Already-running requests and descendant work may finish. Receipts prove policy acceptance and remain pending. The pending_reasons value changes from registry_baseline_unverified to connection_closure_unverified; clients must tolerate unknown reasons. This change does not certify fleet-wide completion.

Related issue

BUZZ-512, under the revised eventual-closure scope. Builds on merged #8985 and #8988. The replacement starts from main and adds no activation schema or migration.

Rollout and limits

There is no new flag. Count already-required communities before deployment: each relay closes their legacy sockets on its first successful sweep. Closures within a community can cause simultaneous reconnects. Keep new upgrades disabled until every serving relay runs this version.

Observe buzz_community_auth_policy_checks_total{outcome} and buzz_community_auth_policy_disconnects_total per relay, and verify real idle root/audio Close 1008 probes plus FI reconnects. Counters record checks and cancellation requests, not fleet completion. A binary rollback requires no migration and restores the old pending reason, but loses eventual closure; it does not undo durable policy. See the operator contract.

Testing

The real-socket journey runs a local relay against isolated PostgreSQL and Redis. It covers idle root/audio closure after a durable change without pubsub, actual read timeouts and recovery, late registration on both routes, FI reconnects, unrelated sockets, and Off/repair/Enforce controls.

With local PostgreSQL and Redis configured per the repository's test guide, run:

CARGO_TARGET_DIR=target/fi-closure-review scripts/postgres-test-run.sh -p buzz-relay --lib -E 'test(sweep_closes_idle_root_audio_and_late_registrations_after_read_recovery)'

Expected: one passing journey, covering both late-registration routes and all socket controls.

The journey passed at 1c74532516f697c2f06498646326f8c479574b14 in a branch-specific build target (5.56 seconds). Independent source review has no remaining P1/P2 findings. Earlier shared-target results are unreliable and are not acceptance evidence. All mandatory push hooks passed in the same isolated target, including Rust unit tests, both Tauri Clippy modes, Tauri tests and file-size checks. Hosted CI passed at this head. The full local just ci chain was not completed. Human runtime acceptance remains open. No merge or deployment is included.

Generated with Codex

[wpfleger96]

🤖 Automated review of 6c67f9ee: changes requested. This head can't merge into current main, and one of the problems won't show up as a git conflict.

IMPORTANT: two migrations would share version 83. This PR adds migrations/0083_community_auth_activation.sql. Main has since merged migrations/0083_channels_listed_name_unique.sql (#8787). The filenames differ, so a rebase applies cleanly and git won't flag it. sqlx tracks applied migrations by version number alone (sqlx-core 0.9.0, Migrator::run_direct). Any database that has already run one 0083 fails startup with VersionMismatch(83) when it reaches the other. On a fresh database, the second insert of version 83 into _sqlx_migrations fails. Fix: renumber this migration to the next free version (0084 on current main) while rebasing, and update the "Migration 0083 is additive" line in docs/community-auth-upgrades.md to match.

Rebase needed: GitHub reports this PR as conflicting. The head still carries the 21 BUZZ-511 commits that main squash-merged as #8988. A merge against current main conflicts in 15 files: Justfile, crates/buzz-db/src/store/community_auth.rs and its Postgres tests, crates/buzz-pubsub/src/{cache_invalidation,lib}.rs, and in crates/buzz-relay/src/: api/git/transport.rs, api/operator.rs, audio/handler.rs, nip11.rs, nip_fi_policy.rs with both of its test files, router.rs and state.rs, plus docs/community-auth-upgrades.md. The description also says a main integration with the fleet activation lock correction isn't published yet. A source review of this head would cover integration code that the rebase will replace, so the full source review and live verification will run on the rebased head instead.

[wpfleger96]

🤖 Automated review of 44d6dd4e, combining two independent source reviews. This is a comment, not a request for changes. I didn't run anything locally; CI was green when I looked, with Codex Security Review and Mesh Lifecycle Smoke still running.

The shared-ticket design is easy to follow, ACKs are exact on operation, boot and generation, and I couldn't find a path where a relay ACKs a drain it hasn't finished through tracked work. The problems are in what tracking doesn't cover, and in a few ways a single bad event can leave upgrades pending forever. Since production activation is hard-disabled, none of this is live yet, but most of it would bite as soon as tracking is armed.

Blocking, inline below:

• historical receipt issuer overrides current issuer trust, so issuer rotation rejects valid reconnects
• boot registered before listener binds and never retired on those errors
• a coalesced-refresh timeout leaves an entry that blocks every later drain
• ACK and retire wait on per-community row locks while holding the fleet-wide scope lock
• obligations are inserted for every upgrade ever accepted and never pruned, so retire cost grows forever
• require() scans every tracked ticket on every request to a required community
• TicketBody::drop drains the inner body to EOF, so a cancelled git read keeps running

Blocking, in files this PR doesn't touch, so it can't go inline: session-descended work still escapes tracking through plain tokio::spawn. run_frame (connection.rs:1343) moves a cancelled in-flight frame into tokio::spawn(frame), which drops the WORK task-local. That frame then runs ticketless, enforce_current() returns true without a ticket (auth_activation.rs:440-443), and anything it spawns is untracked. The same thing happens in artifact_sync.rs:311 (publish retry), side_effects.rs:2897-2917 (kind:9008 delete-group member notifications, which insert events at side_effects.rs:1405-1409 after the parent returns) and verified_profiles.rs:49-90 (profile persistence after admission). If one of those stalls after the parent ticket drops, reconcile at auth_activation.rs:579-590 can ACK while it can still write, and boot retirement has the same gap. Fix: use auth_activation::spawn or an explicitly retained ticket for these session-owned descendants (not every background service). A test through kind:9008 that holds the notice barrier at side_effects.rs:1390-1391, finishes the parent, and checks the ACK stays blocked would pin it.

Design question, nonblocking for this PR but needed before rollout: only a boot can retire itself (retire_auth_boot), and snapshot copies every unretired boot into new obligations (community_auth/activation.rs:81). The doc treats an OOM or SIGKILL'd boot staying pending as intended. In practice one OOM'd pod makes every later upgrade boot_drain_pending forever, and community_auth_boots has no host or pod column for an operator to tell which row is dead. I think this needs an authenticated operator path to retire a boot with evidence its workload is gone, plus something that identifies the pod.

Nonblocking, not inline:

• docs/community-auth-upgrades.md:113-116 lists boot_application_pending and boot_unreachable, which nothing emits, and leaves out boot_drain_pending (community_auth.rs:127) and relay_mode_off (operator.rs:1245), which the code does emit.
• PR title needs a conventional-commit type since it becomes the squash subject (CONTRIBUTING.md:48), e.g. feat(relay): track session application and drain for community auth upgrades.
• the two polling loops in activation_postgres_tests.rs re-query Postgres with only yield_now() between iterations; a short sleep would avoid hammering the shared CI database.

[wpfleger96]

🤖 blocking: every boot registration inserts an obligation for every upgrade ever accepted, including completed, superseded and deleted-target ones, and nothing deletes obligation or boot rows. retire_auth_boot then runs lock_operation once per obligation in one transaction under the scope lock, so shutdown retire grows with restarts × upgrades. Eventually it outlasts the pod's grace period, the boot stays unretired, and it blocks every future operation. snapshot (:81) also scans community_auth_boots WHERE retired_at IS NULL with no index on a table that's never pruned, again under the fleet lock. Fix: skip already-activated or gone operations at registration (or prune obligations of retired boots), have retire touch only the boot's own rows since complete re-checks under row locks anyway, and add a partial index on unretired boots.

[wpfleger96]

🤖 nit: the 1.1s drain only runs when the community doesn't require FI yet, so for one that already does, a failed try_lock rolls back and retries with just yield_now(), opening writer transactions in a tight loop for the full 5s while an ACK holds scope. A small sleep between attempts would fix it.

[wpfleger96]

🤖 nit: BUZZ_RELAY_IMAGE_DIGEST isn't in .env.example or the doc, and nothing outside a test sets it, so every production boot records unverified-version:<pkg version>. The doc tells operators to compare registered images, so it's worth documenting where image comes from and that the fallback isn't evidence of the image.

[wpfleger96]

🤖 nit, latent since nothing outside tests calls these yet: invalidate_auth_baseline is an UPDATE only, so it's a no-op if no boot has registered yet (the row is created lazily in prepare_auth_boot). Re-verifying also doesn't re-run complete, so a receipt stays pending after invalidate plus re-verify until some relay ACKs again, and relays stop ACKing once satisfied_at is set. Worth handling before a production verifier lands.

[wpfleger96]

🤖 nit: the 0084 assertions landed inside the "Listed channel names (0083)" comment block, so that comment now describes the wrong thing. They need their own comment.

[wpfleger96]

🤖 nit: activation_history is JSONB NOT NULL DEFAULT '[]', so not all the new receipt columns are nullable. Something like "additive columns, one with a constant default".

[wpfleger96]

🤖 Re-review of 9ce0bd66, against the earlier review at 44d6dd4e. I read the six fix commits, the merge from main and the two test-fixture commits; I didn't run anything locally. CI had no failures, but the Rust and Postgres jobs hadn't run yet at this head. This isn't an approval yet: two blocking threads from the last review haven't been touched, and one new blocking issue turned up.

Fixed, and the threads are resolved:

• issuer rotation: the tracker now fences only on generation, and root and audio re-check the current issuer bindings in the same statement as the policy. The new rotation tests drive the real HTTP, root and audio handlers.
• listeners bind before the boot registers, so a failed bind can't strand a boot row.
• a coalesced-refresh timeout no longer keeps its ticket forever.
• a dropped response stream now drops the inner body, so StreamingGit::drop kills and reaps the git child with the ticket still held.
• ACK and retire give up on a busy community row after 25ms and release the fleet scope.
• the None-issuer spurious cancel is gone along with the issuer slot.
• cancelled frames, kind:9008 delete notices and verified-profile publication keep their parent ticket.
• the merge from main resolves cleanly (only the Justfile test selection conflicted, and both sides' tests are kept), and 0084 is still the next free migration.

Still open from the last review, unchanged at this head:

• blocking: WorkTracker::require still walks every work entry for the host under the global mutex on every policy read, even when the floor didn't change (auth_activation.rs:37-51).
• blocking: prepare_auth_boot still adds an obligation for every upgrade ever accepted, nothing prunes obligations or boots, and retire cost keeps growing (community_auth/activation.rs:142).
• blocking: the artifact publish retry (handlers/artifact_sync.rs:311-320) is still a plain tokio::spawn. It isn't an independent background service: tracked dispatch (handlers/event.rs:462-505, via handlers/artifact.rs:51-54,81-87) creates it for one admitted event. During a Redis outage it can outlive the parent ticket, so reconcile can ACK (auth_activation.rs:539-548) and shutdown can retire while it can still publish once Redis recovers. It doesn't write to Postgres again, but the doc's producer-completion contract (docs/community-auth-upgrades.md:422-427) covers async producers too. Keeping the originating ticket (or handing it to an owner that's part of drain) fixes it. A test that fails the first publish, finishes the parent and checks the drain stays pending would pin it.
• nonblocking, still open: the activation tracking flag isn't parsed with parse_bool (config.rs:1511), shutdown doesn't override the policy close reason (state.rs:219), acceptance retries on a busy scope lock with only yield_now() (community_auth.rs:368-374), BUZZ_RELAY_IMAGE_DIGEST isn't documented, invalidate_auth_baseline and re-verify don't re-run complete, the 0084 assertions sit under the 0083 comment (migration.rs:912), the doc's pending_reasons list (docs/community-auth-upgrades.md:113-116) and "nullable receipt columns" line (:431) don't match the code, and the PR title still needs a conventional-commit type.

New in this pass, inline below: shutdown retirement gives up on the first database error (this was already true at 44d6dd4e, and I missed it last time), plus two nits on the new code.

[wpfleger96]

🤖 nit: SET LOCAL keeps the 25ms lock_timeout for the rest of the transaction, including complete() and the boot UPDATE, and a 55P03 there comes back as Err rather than Ok(false). It's unlikely since those rows are already locked by this transaction, but resetting the timeout after the row locks (or mapping 55P03 the same way everywhere) would make the contract hold for the whole transaction.

[wpfleger96]

🤖 nit: ingest also starts the tracked post-commit dispatcher before returning (handlers/ingest.rs:3851-3859), and it holds this same ticket. So this assertion can still pass while that task is alive, even if the delete notice lost its ticket. Waiting for the dispatcher to finish before checking the held notice would make the test fail without the side_effects.rs fix.

[wpfleger96]

🤖 Re-review of 29d46edc, against the previous re-review at 9ce0bd66. I read the two new commits (db71bd39, 29d46edc) at source and the CI logs for this head; I didn't run anything locally. Requesting changes: hosted CI is red at this head on code this PR adds or changes, the branch conflicts with main again, and three blocking threads are still untouched.

Fixed in this push (thread resolved): shutdown retirement no longer gives up on the first database error. drain_and_retire_auth_activation now retries pool-unavailable errors and the lock/deadlock/admin-shutdown SQLSTATEs (55P03, 40P01, 57P01-57P03) with capped, jittered backoff, and still returns permanent errors. The new tests cover both the classification and an unreachable pool that stays pending until the caller's hard backstop cancels it.

Blocking: CI at this head (run 42136441052). CI at 9ce0bd66 and db71bd39 was cancelled, so this is the first full run since 44d6dd4e. Main is green at this PR's base cec47c20 and on every completed main run since.

• Rust Lint: cargo fmt --check fails at crates/buzz-relay/src/handlers/coalesced_refresh.rs:335 (the assert!(tracker.is_drained("community", 1), ...) line needs wrapping).
• Windows Rust: clippy -D warnings fails on clippy::redundant_async_block at router/tests/postgres_tests/activation_postgres_tests.rs:60 and clippy::single_match at router/tests/postgres_tests/socket_activation_postgres_tests.rs:332. The Linux lint stopped at fmt before clippy ran, so these will most likely fail there too.
• PostgreSQL Tests, four failures:
  • boot_lifecycle.rs:664 in tcp_only_sigterm_retires_only_after_real_held_work_drains and audio_sigterm_retires_only_after_real_held_teardown_drains: "health must not bind before registration". The listener-ordering fix now binds listeners before the boot registers, and these tests still assert the opposite. Decide which contract you want (if health must not answer before registration, the fix needs to keep health unbound or not ready until then) and make the code and tests agree.
  • nip_fi_resolver/tests/router_tests.rs:885 (every_reachable_handler_resolves_its_host_exactly_once) and :895 (snapshot_going_stale_before_final_admission_admits_nowhere): every handler returns 503 authorization unavailable instead of being reached. This PR doesn't modify router_tests.rs, and these tests passed on main and at 44d6dd4e, so something between 44d6dd4e and this head now denies valid admission in Enforce. The issuer-binding re-check that root and audio now do alongside the policy read is the likeliest candidate.

Blocking: conflicts with main. GitHub reports the PR as conflicting; git merge-tree against current main conflicts in Justfile only.

Still open from earlier reviews, unchanged at this head (the push only touched auth_activation.rs retirement and rotation_postgres_tests.rs):

• blocking: WorkTracker::require still walks every work entry for the host under the global mutex on every policy read (auth_activation.rs:37).
• blocking: prepare_auth_boot still adds an obligation for every upgrade ever accepted, and nothing prunes obligations or boots (community_auth/activation.rs:142).
• blocking: the artifact publish retry (handlers/artifact_sync.rs:311-320) is still a plain tokio::spawn that can outlive its parent ticket.
• The nonblocking threads from the earlier reviews are all still open.

The PR description still names 9ce0bd66 as the published head and says current-head CI is pending. It should describe this head and these two commits.

[github-actions]

🔐 Codex Security Review

Note: This is an automated, security-focused review generated by Codex.
Use it as a supplement to human review; false positives are possible.

Scope

• Exact PR diff: bb3c5f7175041907db50df9640d6377a69cf9be9...1c74532516f697c2f06498646326f8c479574b14
• Model: gpt-6.1-sol

💡 Click "edited" above to see earlier reviews for this PR.

---

Review Summary

Overall Risk: NONE

No concrete security, correctness, or reliability issues found in the authorized PR range.

Findings

No concrete security, correctness, or reliability findings were identified.

Notes

• Read-only review traced policy reads, admission markers, connection registration, and root/audio cleanup paths. Builds and tests were not executed.

---

_{Generated by Codex Security Review |
Requested by: @bradseiler |
Workflow run}

[wpfleger96]

🤖 Approving the rewrite at 1c745325. Two of us reviewed it independently (source reads, nothing run locally), and both found no blocking issues. CI is green on Rust unit, PostgreSQL and relay E2E; one desktop E2E shard was still running when I looked.

Every blocking finding from the earlier reviews was in the activation layer (work tickets, boot and obligation tables, drain ACKs, the issuer slot), which this version removes, so none of them carry over. What we checked in the new code:

• the mode gates match admission: Off and repair stay lifecycle-only, optional Shadow stays legacy, and required Shadow and global Enforce close legacy sockets
• root and audio both attach the enforced FI marker before registering, so valid FI sessions survive the sweep, and a later NIP-42 AUTH can't turn a legacy socket into an FI one
• archived and deleted communities still fall through to the existing row-locked inactive fence, since community_auth_policy uses the same lifecycle predicate as is_community_active
• a slow or failed read keeps the sockets and retries on the next sweep instead of guessing policy
• the doc is upfront that this is eventual closure, in-flight requests may finish, and receipts stay pending

Nonblocking nits, two inline plus one here: some source comments still promise the old drain. nip_fi_policy.rs:1-3 ("existing work is drained separately by BUZZ-512"), nip_fi_policy.rs:114-117 ("activation fences its work") and buzz-db/src/store/community_auth.rs:64 ("Pending until BUZZ-512 supplies applied-and-drained evidence") should describe the closure-only contract now.

[wpfleger96]

🤖 nit: this walks every local socket, and the sweep calls it once per effectively-required community, so each pass is O(required communities × sockets). That keeps going after every legacy socket is gone, and under global Enforce every bound community counts. Probably fine at today's sizes, but collecting the required IDs and closing in one pass over the registry would make it O(sockets) per sweep.

[wpfleger96]

🤖 nit: this line continues the sentence after the semicolon on the previous line, so it should start with a lowercase "a".