Repair stale large channel roster snapshots

[wesbillman]

Summary

• detect existing relay-authored NIP-29 kind 39002 roster snapshots truncated by the former 1,000-member query cap
• repair stale large rosters automatically during relay startup with a roster-only emitter
• preserve tenant and relay-signer boundaries, bypass the 1,000-channel list ceiling, and converge after repair without rewriting kinds 39000 or 39001

Validation

• cargo test -p buzz-db large_roster_reconciliation_candidates_respect_snapshot_count_and_signer -- --ignored --nocapture
• cargo test -p buzz-relay group_members_snapshot_keeps_members_past_one_thousand -- --nocapture
• cargo check -p buzz-relay
• pre-push hook: branch skew, file-size check, Rust tests, and desktop Tauri checks passed on 386ccc21e2095b563f4d59d3e732bb6a7058a4f0

Review

Independent full review found no blocking issues on exact head 386ccc21e2095b563f4d59d3e732bb6a7058a4f0. Residual risk: no end-to-end startup invocation test; the path composes the focused Postgres candidate regression with the existing transactional addressable replacement emitter.

[jedwards27]

:bot: Jude’s code review agent

Verdict: REQUEST CHANGES

Reviewed: 417eea2230c1864e8c77f6440dbcfa109bfb63f6..386ccc21e2095b563f4d59d3e732bb6a7058a4f0
Risk: High — this changes authoritative membership discovery during relay startup and rolling deployment.

Blocking — startup repair can supersede a newer canonical roster

crates/buzz-relay/src/handlers/side_effects.rs:3087-3098 reads the canonical member list and later publishes it without a freshness fence shared with membership mutation/publication. Publication independently reads the current kind 39002 timestamp and chooses a newer timestamp at crates/buzz-relay/src/handlers/side_effects.rs:996-1032; replace_addressable_event then orders only by that event timestamp/id at crates/buzz-db/src/lib.rs:4876-4906, not by the canonical membership generation represented by the tags.

This permits the following rolling-deploy interleaving with another relay instance using the same DB:

1. The starting instance reads roster A.
2. The existing instance commits an add/remove and publishes roster B.
3. The starting instance observes B's event timestamp, assigns +1, and replaces B with stale roster A.

Membership writers release their channel transaction lock before normal discovery publication (crates/buzz-db/src/channel.rs:397-401,578-583; crates/buzz-relay/src/handlers/side_effects.rs:1331-1358,1402-1430), so that lock does not close this interval. The result can hide a newly added member from channel discovery or restore a removed member in the authoritative snapshot. Because candidate selection compares only p-tag count at crates/buzz-db/src/channel.rs:823-838, a remove/add exchange preserving count may not be selected for repair on the next startup either.

Please add an atomic freshness fence spanning canonical roster capture through kind 39002 replacement—such as a shared membership generation/version with CAS/retry, or equivalent serialization covering all membership writers/publication paths—and a deterministic concurrency regression proving a repair cannot supersede a post-scan add/remove. Timestamp freshness is not state freshness. The dungeon has supplied enough stale truth already.

Other traced contracts: tenant/community and relay-signer boundaries are preserved in candidate selection and publication; the four-file diff does not introduce unrelated UI/schema/release scope. No additional material findings.

Validation at exact head: required GitHub checks are green, including Unit Tests, Relay E2E, and Backend Integration. Local focused roster-tag test passed (cargo test -p buzz-relay group_members_snapshot_keeps_members_past_one_thousand -- --nocapture, 1 passed). Full local relay package runs were not clean due environment/pre-existing failures: independent runs reported 879/891 and 890/891 passing, with missing local Postgres schema failures and a mesh-demo 504 reproduced on base. buzz-db reported 105 passed with 186 Postgres tests ignored; the new DB candidate test is ignored without Postgres. Existing coverage does not exercise the blocking read/mutate/replace interleaving.

Residual risk: no end-to-end startup/rolling-deployment concurrency proof exists at this head; that is the blocking gap rather than evidence to waive it.

[wesbillman]

Addressed the rolling-deploy race at 0d59ad8c469d463642a50404f7619e1613c71509.

The relay now captures canonical membership while holding the same per-channel advisory lock used by add_member/remove_member, and keeps that lock through kind 39002 replacement. Normal discovery publication uses the same fence, so publication order now follows canonical roster capture instead of timestamp assignment. Added a deterministic Postgres regression proving a post-capture membership writer cannot acquire the lock until publication releases it.

Validation:

• locked_member_snapshot_blocks_post_capture_membership_mutation: passed against Postgres
• group_members_snapshot_keeps_members_past_one_thousand: passed
• cargo check -p buzz-db -p buzz-relay: passed
• pre-push Rust/unit and other applicable gates passed; an unrelated buzz-terminal permission-fixture test failed once and passed immediately in focused rerun

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

[wesbillman]

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

Verdict: SAFE TO MERGE AFTER CI

Reviewed exact head: 0d59ad8c469d463642a50404f7619e1613c71509

The stale-publication blocker from the prior review is fixed. Membership-derived publication now captures the canonical roster while holding the existing per-community/channel membership advisory transaction lock and retains that fence through kind 39002 replacement. Normal discovery publication uses the same fence, which is necessary to prevent two committed mutations from publishing in reverse order. The startup repair path likewise cannot publish captured roster A after a writer has committed/published roster B.

The added ignored Postgres regression deterministically proves that a post-capture membership writer cannot acquire the shared advisory key until the snapshot guard releases it. This is the right essential concurrency contract; an end-to-end two-relay publication test would add confidence but is not required to establish the ordering proof here.

Royal Court reconciliation found no additional material defect. Tenant scoping, relay-signer filtering, NIP-16 replacement ordering, unbounded canonical roster capture, and targeting of only stale >1,000-member snapshots remain internally consistent.

Validation on this exact head:

• cargo fmt --check passed.
• cargo check -p buzz-db -p buzz-relay passed.
• cargo test -p buzz-db locked_member_snapshot_blocks_post_capture_membership_mutation -- --ignored passed (1 test).
• cargo test -p buzz-relay group_members_snapshot_keeps_members_past_one_thousand passed (1 test).
• Pre-push broad Rust/file-size gates passed. A desktop terminal lifecycle test failed once under the broad hook and passed immediately in focused rerun; it is unrelated to this DB/relay diff.

CI for this new head was still starting when this review was submitted and remains the merge gate. GitHub does not permit Wes’s account to approve its own PR, so this is recorded as a non-blocking review comment rather than an approval state.

[jedwards27]

:bot: Jude’s code review agent

Verdict: APPROVE

Reviewed: 93114c9c65138397de39729fde0a816eb9f314ab..f707f732cc59578819e590462ada38cd8ce69954
Risk: High — authoritative channel membership publication, cross-instance concurrency, rolling-version compatibility, and database migration behavior.

The stale-publication blocker from my review at 386ccc21e2095b563f4d59d3e732bb6a7058a4f0 is closed.

The replacement path now takes the canonical replacement advisory key and then the per-community/channel membership key before canonical capture, retaining the same transaction and connection through timestamp selection, soft-delete, validated insert, mention indexing, and commit (crates/buzz-db/src/channel.rs:369-539). Both normal discovery and startup repair use that path and dispatch only after commit (crates/buzz-relay/src/handlers/side_effects.rs:1203-1214,3127-3141). A membership mutation therefore cannot commit between the roster capture and its replacement publication.

Rolling old/new relay deployments are also fenced. Migration 0032's insert trigger takes the membership key after the replacement key and rejects kind 39002 snapshots whose exact active (pubkey, role) set differs from canonical membership (migrations/0032_channel_roster_snapshot_fence.sql:1-76). A stale legacy writer's insert raises SQLSTATE 23514; its preceding soft-delete is rolled back in the same transaction, preserving the fresh live head. The lock order is consistently replacement → membership, with no inversion found in the traced membership mutation paths.

The deterministic mixed-version regression exercises the formerly blocking schedule, including same-count role-only drift: stale legacy A is captured, canonical state advances, locked writer B commits, then newer-timestamp A is attempted through the legacy replacement path. It requires 23514, proves B remains the sole live head, and proves A did not persist (crates/buzz-db/src/lib.rs:5594-5729). Desired-state bootstrap parity and stale-role rejection are covered at crates/buzz-db/src/migration.rs:1040-1068 and crates/buzz-db/src/lib.rs:5732-5824.

The final pgschema delta is appropriately bounded: each standalone event partition drops only the copied roster trigger immediately before attachment, allowing PostgreSQL to recreate the inherited parent trigger (scripts/attach-schema-partitions.sql:17-124). No additional material tenant-isolation, signer-boundary, destructive-state, UI, or unrelated-scope finding remains. The change preserves VISION.md's relay-enforced membership and community-isolation contracts.

Exact-head validation:

• PASS — cargo test -p buzz-db stale_legacy_roster_cannot_replace_new_locked_snapshot -- --ignored --nocapture (1/1).
• PASS — cargo test -p buzz-db desired_schema_rejects_stale_legacy_roster_role -- --ignored --nocapture (1/1).
• PASS — cargo test -p buzz-db (105 passed, 0 failed, 189 ignored; ignored Postgres rows were run separately above).
• PASS — cargo test -p buzz-db embedded_migrator_contains_consolidated_initial_schema -- --nocapture (1/1), independently rerun with clean tree and matching HEAD.
• PASS — git diff --check 93114c9c65138397de39729fde0a816eb9f314ab...f707f732cc59578819e590462ada38cd8ce69954.
• PASS — all required GitHub checks at this exact head, including Unit Tests, Backend Integration, Relay E2E, Security, cross-compiles, and both relay Docker architectures.
• Environment-only local relay package failures were confined to tests requiring a missing local communities schema; they are not counted as product evidence. The corresponding exact-head CI integration gates are green.

Manual/live evidence: No fresh live >1,000-member workflow was run at this head.

Residual risk: The unrun live large-roster startup workflow leaves deployment-shaped evidence below ideal, but the exact mixed-version Postgres regressions, desired-schema proof, package suite, and green relay/backend/pgschema Docker gates are sufficient for merge confidence. Any head movement invalidates this approval.