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.

[wesbillman]

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

Verdict: REQUEST CHANGES

Reviewed exact head: f707f732cc59578819e590462ada38cd8ce69954

Blocking: startup repair can run without migration 0032's rolling-deploy fence

BUZZ_AUTO_MIGRATE is opt-in (crates/buzz-relay/src/main.rs:188-198), but the new large-roster repair runs unconditionally (crates/buzz-relay/src/main.rs:537-548). Nothing verifies that migration 0032's trg_events_guard_channel_roster_snapshot is installed before the repair or normal new-binary kind 39002 publication proceeds.

That invalidates the mixed-version safety proof in a supported deployment state. If a new pod starts against a database where migration 0032 has not yet been applied, it can publish repaired roster B under the new replacement→membership lock order. An old pod that captured stale roster A earlier can then acquire the replacement lock and overwrite B because the database trigger that would acquire the membership lock and reject A does not exist. The exact-head regression at crates/buzz-db/src/lib.rs:5594-5729 proves this schedule is rejected only after migration 0032 is present.

Please fail closed on missing roster-fence coverage before any new startup repair runs. Prefer a live catalog/semantic verification covering the partitioned events parent and active partitions, then gate the repair with a loud diagnostic. Add a Postgres regression proving an unmigrated database does not perform the repair, followed by migration application and stale legacy-writer rejection. Consider gating all new kind 39002 publication paths if deployment ordering cannot guarantee schema-before-code.

The migration itself is additive and otherwise well structured: no table rewrite or index build; replacement and membership lock order is consistent; stale replacement rollback is transactional; migration/desired-schema parity and partition attachment are covered. Migration 0032 should still be applied as a controlled pre-rollout database step with lock monitoring because CREATE TRIGGER has no local lock_timeout and may wait behind writers.

[jedwards27]

:bot: Jude’s code review agent

Verdict: COMMENT — code clear; merge remains gated

Reviewed: bbd20fae75ecc3bd7a83cc12a65379fac22a2b79..be8ea0084f4d4c78c7c2550baad4399e4df8ce73
Risk: High — authoritative channel membership publication, mixed-version relay concurrency, partitioned database trigger coverage, and fail-closed startup behavior.

The missing-schema rolling-deploy blocker reported at f707f732cc59578819e590462ada38cd8ce69954 is closed. Independent systems/integration and product/adversarial passes found no remaining material code defect at this exact head.

Startup now verifies an enabled row-level BEFORE INSERT roster trigger bound to the expected function on the partitioned events parent and every attached child (crates/buzz-db/src/channel.rs:350-395). It then runs a rolled-back semantic probe through the live writer pool: a canonical empty roster must insert, while a stale non-member roster must fail with SQLSTATE 23514 (crates/buzz-db/src/channel.rs:397-470). This catches missing, disabled, structurally wrong, and intact-looking-but-inert fences.

Relay startup treats either verification failure as fatal after the auto-migration decision and before large-roster reconciliation or listener bind (crates/buzz-relay/src/main.rs:188-198,537-560,1280-1286,1355-1358). Migration execution also verifies the fence (crates/buzz-db/src/migration.rs:49-61), and the deployment guide now states the schema-before-code compatibility boundary (deploy/charts/buzz/README.md:204-210). The earlier replacement→membership lock order, exact (pubkey, role) comparison, transactional stale-write rollback, tenant coordinates, and relay-signer boundary remain intact. This additional startup availability contract is justified by the PR's mixed-version safety requirement rather than unrelated expansion.

Exact-head validation on clean detached trees:

• PASS — pre-0032 startup refusal → migration 0032 application → successful admission; inert-function refusal; disabled-child-trigger refusal (three focused ignored Postgres regressions, 1/1 each).
• PASS — mixed-version stale-writer and desired-schema stale-role regressions (1/1 each).
• PASS — causal mutation removing the semantic behavior call made the inert-function regression fail at its expected error assertion; source restored and tree clean.
• PASS — full cargo test -p buzz-db: 105 passed, 0 failed, 192 ignored; the five load-bearing ignored Postgres rows were run separately.
• PASS — git diff --check; local immutable head and refreshed GitHub head matched.
• FAIL/NOT ATTRIBUTED — full cargo test -p buzz-relay: 878 passed, 15 failed, 44 ignored. Most failures reported a missing local communities schema; a mesh-demo 504 and telemetry global-subscriber assertion also failed. The whole set was not reproduced against base, so it is not claimed unrelated. Exact-head Backend Integration and Relay E2E CI are green.

Outstanding merge gates:

1. Required exact-head Desktop Smoke E2E (3) and aggregate Desktop CI are red. The shard reports one persistent onboarding failure and one retry-flaky mentions failure; this nine-file PR contains no Desktop file, but the failure still needs a green rerun or accepted base/SHA disposition before merge.
2. The PR body still attributes validation and review to obsolete head 386ccc21e2095b563f4d59d3e732bb6a7058a4f0 and omits the new hard startup dependency on migration 0032. Update Summary/Validation to describe fail-closed startup, schema-before-code rollout, and exact-head evidence.

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

Residual risk: Deployment-shaped evidence remains below ideal without that live workflow. The exact Postgres failure/repair probes, mutation proof, package suite, and green relay/backend integration gates are sufficient for code clearance, but not to waive red required CI or a stale operator-facing PR contract. Any head movement invalidates this verdict.