[TheSentinel454]

Summary

The relay builds kind:39000 (group metadata) and kind:39001 (admins) from a channel read that takes no lock, and replaces each one on its own transaction. A concurrent emission can therefore publish stale state with a newer created_at:

1. A membership emission reads the channel. The admission auto-join from feat(relay): auto-join new community members to default channels #8594 is one source of these.
2. A kind:9002 edit commits, for example auto_join=false.
3. The edit's own emission publishes the new metadata.
4. The first emission then replaces it with the old snapshot. Its created_at is head + 1, so the stale tags win.

Only kind:39002 had a freshness fence. This race made test_nip29_announce_channel_wire fail on main after #8594 (https://github.com/block/buzz/actions/runs/41341005385). The race itself is older: any membership change racing a metadata edit could hit it.

Change

• lock_member_snapshot now takes the replacement locks of all three group-state coordinates (39000, 39001, 39002), in kind order, before the membership lock. It also reads the live channel row on the same transaction.
• lock_member_snapshot reads the channel row once: the roster's identity tags (kept for a deleted channel) and the live row behind 39000/39001 come from the same read, so a labels-only edit cannot land between them.
• replace_member_event becomes replace_group_state_event. It accepts 39000, 39001 or 39002 for the locked channel and validates the roster only for 39002.
• emit_group_discovery_events builds all three events from that one snapshot, replaces them on its transaction, commits, and then fans out the events that became heads.

Every channel writer commits before it emits. So an emission either finishes before a later one starts, or it reads the later writer's committed change. A capture of older state can no longer replace a newer one.

The replacement keys are the same ones replace_addressable_event takes. During a rolling deploy, a relay that still writes each kind on its own transaction serializes with the new path. No writer holds one of these keys while waiting for another, so taking all three in order cannot deadlock.

What got simpler

• emit_addressable_discovery_event is deleted, along with its separate unlocked created_at lookup.
• The three discovery events now share one capture, one transaction and one timestamp rule (store_group_state_event), instead of two code paths with different freshness guarantees.
• A group-state publication is now atomic: a failure rolls back all three events instead of leaving 39000 published without 39002.

Testing

Verified on Blox at f7d8043:

• held_group_state_capture_cannot_replace_a_newer_metadata_edit (buzz-relay, Postgres):
  • Setup: hold an emission at a new test hook after it captures auto_join=true, commit auto_join=false, then start the edit's emission.
  • Asserts: the edit's emission waits for the held one, and the final kind:39000 head has no auto_join tag.
  • Mutation: moving the channel read back before the lock (the pre-fix shape) fails the test.
• locked_member_snapshot_blocks_post_capture_membership_mutation (buzz-db) is extended:
  • Asserts: the held snapshot captures the channel row and holds the 39000 and 39001 replacement keys.
  • Mutation: taking only the 39002 key fails the test.
• locked_member_snapshot_of_deleted_channel_keeps_identity (buzz-db): a deleted channel's snapshot has no live row but keeps its identity. Mutation: deriving identity from the filtered live row fails it.
• cargo fmt, clippy (-D warnings), and the full buzz-db (655) and buzz-relay (544) Postgres lanes pass; all three mutations turn their tests RED.
• E2E on an exact-head relay: 20/20 concurrent join + auto_join=false rounds left 39000/39001/39002 matching SQL, and plain edit/join/leave publish fresh state live.

buzz-review-completed

🤖 Generated with Claude Code

[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: cde6c1bf5b1fa1fde6045de8ea29f6ac1c7e6867...f7d8043d8c3a6d71db1787a063ff502823a14d94
• 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 regressions found in the authorized PR range.

Findings

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

Notes

• Review used read-only source and history inspection. Builds and tests were not run, as requested.

---

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

[wpfleger96]

🤖 Automated review at f7d8043: no blocking findings.

This review covered two independent source reads of the diff plus a live run of the PR head against an isolated relay and database.

Lock order. The snapshot now takes the 39000, 39001 and 39002 replacement keys, then the membership key. No writer on this head holds the membership key, the TTL lock or the channel row lock and then waits on a group-state replacement key. Settings writes take membership, TTL, then the row lock. Kick paths take only membership. Migration 0032's trigger adds membership after the 39002 key. replace_addressable_event and the admin backfill hold one coordinate per transaction. So taking all three in kind order cannot invert against any of them, including relays still running the old build during a rollout.

Behavior parity. The old 39000/39001 path read the channel and roster with the same deleted_at IS NULL filters the snapshot uses, so a deleted channel still returns ChannelNotFound and deletion still removes the discovery heads rather than publishing a final set. The locked replace keeps the old path's stale-write check, follow-up hook and mention indexing. Roster validation stays 39002-only. No references to the removed helpers remain, and the new test hook is gated by #[cfg(test)] at both the call site and the module.

Live run. In 24 unforced rounds, a kind:9021 join raced a kind:9002 edit setting auto_join=false. In every round, the final 39000/39001/39002 heads matched SQL, arrived live on a channel subscription, and had no stale auto_join or label tags. A plain edit, a labels-only edit, a join, an admin promotion and a leave each published three fresh heads with strictly increasing timestamps, with correct signatures, identity tags, admin roles and roster.

Note (non-blocking). Sharing replacement keys makes old and new relays serialize, but it does not make old relays fresh: an old relay still reads the channel before locking. Group-state freshness is fully guaranteed only once every relay runs this build. The live run did not exercise a mixed-version deployment.

[bradseiler]

🤖 Rollout notes from McKeon, Brad Seiler's review agent, at f7d8043. These are suggestions, not an approval.

This change has no flag: it takes effect when the relay image deploys. The PR body has no rollout section yet, so here's what I'd add:

1. Mixed-version window. Group-state freshness holds only once every relay runs this build. Old relays still read the channel row before taking any lock. The shared replacement keys make old and new relays serialize with each other, but they don't make old relays' captures fresh. A stale 39000 or 39001 is still possible until the rollout finishes.
2. What to watch during and after the deploy. 39000 and 39001 now wait behind the membership lock and the 39002 publication, under the 5 s lock_timeout.
   • Replacement and membership advisory-lock wait times (observe_advisory_lock, LockType::Replacement).
   • Lock-timeout errors (SQLSTATE 55P03) on emission.
   • NIP-29 group discovery emission failed warnings. Callers only log these. reconcile_large_channel_member_snapshots repairs 39002 only, so a 39000 or 39001 that times out stays stale until the next change to the channel.
3. Staging check after deploy. Re-run the concurrent join plus auto_join=false race from the Testing section, and confirm the 39000, 39001 and 39002 heads match SQL. Then do a plain edit, a join and a leave, and confirm each one publishes three fresh heads.
4. Rollback. A plain revert is safe. There's no schema or migration change, and both builds take the same replacement keys.

[presentyourlove]

Hi @TheSentinel454,

I have resolved the merge conflicts with the latest main branch:

1. crates/buzz-db/src/store/channel_members.rs: Combined the kind == 39002 check for roster snapshots with the superseded_with_mentions_sql! macro from main (fix(db): drop a superseded head's mention rows with it #8948) so superseded mentions are properly cleaned up across all group state events.
2. crates/buzz-relay/src/handlers/side_effects.rs: Retained the newly merged channel rename and operator authority helpers (PRIVILEGED_METADATA_TAGS, operator_may_rename) from main while cleanly removing the deprecated un-fenced emit_addressable_discovery_event.

A clean, zero-conflict branch is available at:
https://github.com/presentyourlove/buzz/tree/fix/group-metadata-fence-8898

You can pull it directly into your branch:

git fetch https://github.com/presentyourlove/buzz.git fix/group-metadata-fence-8898
git merge --ff-only FETCH_HEAD