[ravarora2]

Summary

Make existing user-deletion restrictions apply at the two shared event insertion functions. A fenced signer gets a refusal; the event and its transactional changes roll back. This covers every event kind reaching those functions, including messages, replies, reactions, edits, channel changes, deletion events and artifacts.

The transaction takes shared identity locks and checks the signer's global and community restrictions with one query. It retains the locks through commit or rollback. Stored owner identity coordinates locks; it is not an extra restriction check. Ownership changes produce a retryable internal error. Unrelated database errors keep their existing handling.

Identity archive/unarchive requests now persist the request, archive change and relay delta in one transaction. Notifications follow commit. Duplicate retries do not repeat the mutation. Snapshot refresh still runs after commit: failures propagate, and a retry repairs a missing or stale snapshot. If the snapshot already matches current archive state, it retains its event ID and timestamp and is not broadcast again. The no-op path rechecks canonical state for concurrent changes. There is no durable background retry queue for that refresh.

Some event callers hold domain locks before the identity guard, and some insert events from multiple signers. The documented worker contract now reflects that behavior: exclusive fencing transactions must not touch serving-domain rows. Cleanup belongs in later transactions. Per-call identity sorting alone does not prove every future multi-key workflow safe.

Scope remains event persistence. There are no migrations, user-specific leases, authentication changes, ownership-assignment changes or restriction-insertion behavior changes. Separate replaceable-event writers, read state, uploads, git, audio, ephemeral activity and deletion-worker coordination remain follow-up work. The real user-deletion worker must remain disabled.

This affects a critical shared write path and adds measurable database overhead, even while the restriction table is empty. See the local measurements below and the transaction rules and boundaries.

Related issue

Part of BUZZ-413. Follows merged #8632. No duplicate message-fencing PR found; #8489 covers workflow scaffolding and #8645 covers owned-community handlers.

Testing

Local database benchmark at 4aef33b95, before merging the newer main: 49,152 successful timed writes, excluding warmups. A temporary harness called Db::insert_event and Db::insert_event_with_thread_metadata with kind 9 events, human/agent signers, and one/eight concurrent writers. Signing occurred before timing; timing included pool acquisition, community admission, persistence and commit.

Environment: Apple M1 Max, 64 GiB RAM, PostgreSQL 17.11 in local Docker, TCP loopback, Cargo test profile (unoptimized), empty restriction table. Each case used 64 warmups and 512 measured writes per round. Compared the original two-query guard, the combined query, and a temporary no-op guard. Ran three rounds per variant and repeated the combined version. All temporary changes were restored; no benchmark bypass is committed.

Single-writer latency, in milliseconds. Cells show the median of each run's p50/p95 values; combined-query figures use six rounds, others use three.

Insert function	Signer	No guard p50/p95	Two queries p50/p95	Combined p50/p95
insert_event	human	3.41/4.31	6.33/7.83	5.71/7.58
insert_event	agent	3.52/4.96	6.11/7.72	5.82/7.63
insert_event_with_thread_metadata	human	3.23/4.03	6.07/8.77	5.46/7.02
insert_event_with_thread_metadata	agent	3.23/4.02	6.46/10.79	5.84/7.30

Combining the lookups reduced single-writer median latency about 5–10% versus two queries in these runs. The full guard still added about 2.2–2.6 ms (65–81%) versus no guard. This is a material cost, not evidence that fencing is free or ready for production capacity assumptions. Actions that also persist relay-generated events pay the guard cost for each additional insert; this benchmark does not measure complete HTTP actions.

Eight concurrent writers, same channel; median throughput across rounds:

Insert function	Signer	No guard events/s	Two queries events/s	Combined events/s
insert_event	human	1220	692	698
insert_event	agent	1310	593	672
insert_event_with_thread_metadata	human	1269	543	740
insert_event_with_thread_metadata	agent	1181	646	718

These are local debug measurements, not a production benchmark or capacity estimate. No injected network latency or populated restriction-table load was included in timing. Separate query-plan checks used a temporary table with the same two partial indexes; empty and 50,000-row cases used both indexes. The earlier 510-case lab matrix covered a broader revision and is not evidence for this version; no new UI lab run is claimed.

Generated with Codex

[presentyourlove]

Rebase & Conflict Resolution Report for PR #8658

Target PR: block/buzz#8658 — BUZZ-413: Fence stream and forum message writes
Author: @ravarora2 (Ravneet Arora)
Branch: fix/fence-stream-forum-writes-8658
Commit: 59097c94ca0fcb615988facc6877cdc467449c7d
Patch: 59097c94ca0fcb615988facc6877cdc467449c7d.patch

---

1. Problem & Root Cause

PR #8658 was marked as dirty (mergeable: false) on GitHub due to conflicts introduced when commit d6196a16c1 (#8237, revoke invitations and restrict claims) merged into main:

1. crates/buzz-db/src/error.rs:

   • main added InviteRestricted and InviteOwnerMismatch variants.
   • PR BUZZ-413: Fence shared event database writes #8658 added UserDeletionRestricted.
   • Resolution: Kept all variants so invitation restriction handling and user deletion restriction errors coexist seamlessly.

2. crates/buzz-db/src/store/user.rs:

   • In set_agent_owner_with_operation:
     • main added crate::relay_invite::lock_admission(tx.conn(), community_id).await?; to acquire the invitation admission lock on community write transactions.
     • PR BUZZ-413: Fence shared event database writes #8658 added tx.guard_agent_owner_assignment(agent_pubkey, owner_pubkey).await?; to verify user deletion fences and lock agent/owner identities.
   • Resolution: Kept both locks in canonical lock hierarchy order:

     let mut tx = crate::begin_community_write_transaction(pool, community_id, operation).await?;
     crate::relay_invite::lock_admission(tx.conn(), community_id).await?;
     tx.guard_agent_owner_assignment(agent_pubkey, owner_pubkey).await?;

     This ensures invitation admission and claim restrictions are enforced, user deletion fences guard agent ownership changes, and the conditional update remains atomic without deadlock.

---

2. GitHub PR Comment Draft (for @ravarora2)

Hi @ravarora2,

I rebased PR #8658 onto the latest `main` branch to resolve the merge conflict with commit `d6196a16c1` (#8237, *revoke invitations and restrict claims*):

- In `crates/buzz-db/src/error.rs`: Preserved both the new invitation restriction error variants (`InviteRestricted`, `InviteOwnerMismatch`) and `UserDeletionRestricted`.
- In `crates/buzz-db/src/store/user.rs`: In `set_agent_owner_with_operation`, preserved `lock_admission` alongside `guard_agent_owner_assignment` in canonical order so both invitation admission gating and user-deletion ownership fencing are held before the atomic `UPDATE`.

You can pull the rebased commit directly into your branch:

```bash
git fetch https://github.com/presentyourlove/buzz.git fix/fence-stream-forum-writes-8658
git rebase 59097c94ca0fcb615988facc6877cdc467449c7d

Or apply the patch:

curl -sL https://github.com/presentyourlove/buzz/commit/59097c94ca0fcb615988facc6877cdc467449c7d.patch | git am

[wpfleger96]

🤖 Review at 0c3ebcb53. The core guard is sound: it checks only the signer, under both the global and community scopes, regardless of profile rows, rechecks the owner after locking, and holds the locks through commit. Five blocking issues, three minor, all inline:

IMPORTANT

1. Kind 9035/9036 archive/unarchive requests change archived_identities and publish deltas before the guard runs, so a restricted author's request takes effect and is then refused (event.rs L1884).
2. The new AccessDenied arm on the generic insert path sends community-admission errors to clients as restricted: (ingest.rs L3730).
3. The ownership-change refusal is AccessDenied, so clients see restricted:, invalid: or an internal error depending on the path. None of these tells them to retry (user_deletion_write_guard.rs L32).
4. Some callers take identity locks after row locks, against the ordering rules in user_deletion_fences.rs (event.rs L480).
5. About nine extra round trips on every event insert, with no measurement (user_deletion_write_guard.rs L66).

MINOR
6. The refusal mapping is duplicated.
7. The design doc reads as a PR narrative.
8. The test modules are chained through nested #[path] attributes.

[wpfleger96]

🤖 IMPORTANT — archive/unarchive takes effect despite the refusal. For kinds 9035/9036, ingest.rs L3082–3089 calls handle_identity_archive_event before the request reaches this inserter. That handler commits archived_identities changes in its own transaction and publishes the relay-signed 8002/8003 deltas and the 13535 snapshot. A restricted author's request therefore changes archive state, then gets refused here and is never stored. The published delta's ["e", request_id] then points at an event that doesn't exist.

Fix: apply the archive change and insert the request in one transaction that runs this guard, and publish after commit. A separate check before the handler would still race a restriction being installed. Please also add archive/unarchive to the HTTP/WS before-and-after-restriction tests, with archived_identities in the unchanged-state snapshot.

[ravarora2]

🤖 Fixed in 4688052, included in head 572e30e. Archive/unarchive validation now produces a plan without changing state. The guarded request insert, archived_identities mutation, and relay delta persist in one transaction; notification dispatch follows commit. A rejected request or failed delta insert rolls all three back. Duplicate requests do not repeat the mutation.

Added HTTP/WebSocket before-and-after-fence regressions, unchanged archive/snapshot assertions, delta-failure rollback, and fence commit/rollback races. These passed in the merged tree's PostgreSQL suite (1,189 tests).

Remaining boundary: the snapshot refresh is post-commit. Its failure propagates, and a duplicate retry refreshes it without repeating the archive mutation. There is no durable background refresh queue, so a crash after commit can leave the snapshot stale until a later refresh. This limitation is documented.

[ravarora2]

🤖 Follow-up fixed in 3564dc2: replaying an already-processed archive/unarchive request now leaves an already-current snapshot unchanged, including its event ID and timestamp, and does not broadcast a replacement. The publisher compares the saved snapshot with canonical archive state and rechecks that state before taking the no-op path. Missing/stale snapshots are still repaired after a failed publication; the existing post-write drift protection remains.

The new replay regression failed against the previous code, then passed with the fix. It covers repeated archive/unarchive requests and replay after a later opposite operation, asserting that all database rows remain unchanged. Recovery tests cover both missing and stale snapshots and a further retry after repair. A deterministic concurrency test changes canonical state while the publisher is paused before its no-op decision and verifies repair.

Validation: all 6 PostgreSQL archive/fencing tests and 8 existing archive consent/publisher tests passed, as did the normal pre-push hooks. The PostgreSQL fixture exercises the HTTP route and authenticated WebSocket event handler; I did not rerun the separate live WebSocket AUTH simulation. The full repository just ci run was not restarted.

[wpfleger96]

🤖 IMPORTANT — this arm changes behavior outside this PR's scope. On this path, AccessDenied also comes from community admission (guard_community_write / lock_community_deletion_shared). Examples: community <uuid> is write-fenced (<state>), missing or tombstoned, and deletion-lock admission requires read committed isolation, got …. Those used to surface as error: database error. With this arm, clients see internal state and config text under a restricted: prefix. Once the ownership-change case below has its own variant, nothing in this PR needs the arm. Suggest removing it.

[ravarora2]

🤖 Fixed in f1570c4, included in head 572e30e. Removed the generic AccessDenied-to-restricted arm. Only UserDeletionRestricted produces the user-deletion refusal. Other database errors, including community-admission errors, retain their existing caller fallback rather than exposing the internal reason under restricted:. Error-mapping and transport regressions passed.

[wpfleger96]

🤖 IMPORTANT — the ownership-change refusal can't be told apart from a permanent refusal. As AccessDenied, it surfaces three ways depending on the caller:

• restricted: on the generic insert path
• invalid: on the put-user and edit paths
• Internal through event_write_error, artifact acceptance and command persistence

None of these tells the client to retry, which contradicts the message. begin_community_sweep_transaction also already treats AccessDenied as "community fenced, skip". Suggest a dedicated DbError variant, mapped once in event_write_error to a retryable error: refusal.

[ravarora2]

🤖 Fixed in f1570c4, included in head 572e30e. Added DbError::EventAuthorOwnershipChanged and mapped it centrally through event_write_error. An ownership change now returns the retryable error category: WebSocket error: internal server error, or HTTP 500 with the sanitized internal server error message. It no longer uses AccessDenied or becomes restricted:/invalid:.

The PostgreSQL transport regressions force ownership to change while the writer waits for its identity lock, cover generic/membership/edit paths over HTTP and WebSocket, and verify a retry succeeds. This permits a client retry; it does not implement automatic retry behavior.

[wpfleger96]

🤖 IMPORTANT — identity locks taken after domain row locks. The ordering rules in user_deletion_fences.rs are: community admission, then all global user locks in one call, then community-scoped agent locks. Because the guard runs at the insert call rather than when the transaction starts, some callers lock rows first. store/artifact.rs accept writes artifact_revisions and upserts artifact_heads before inserting the author's event. On a Move, it then inserts the relay-signed removal marker, which takes a second global user lock after the author's community-scoped lock. Nothing deadlocks today, because normal writers take only shared locks and no exclusive locker touches these rows. But the deletion worker will depend on this order, and once it holds exclusive identity locks and writes these rows, these paths will deadlock and abort.

Two ways to fix it: run the guard before any domain row lock (at admission, or at the top of each caller), or rewrite the ordering rules to match what callers actually do and state the worker-side requirement that follows.

[ravarora2]

🤖 Addressed using the second option in 21f8dc6, included in head 572e30e. The documentation now describes the actual call order: domain locks may precede the guard, and later relay-signed inserts can acquire another global identity lock after the first scoped lock. Per-call sorting is explicitly not a transaction-wide guarantee.

The future exclusive fencing transaction must write only restriction/request/checkpoint state while holding identity locks. It must not lock or mutate serving-domain rows; cleanup must use later transactions. Complete exclusive identity sets must also be audited against multi-signer transactions before enabling the worker.

Added production artifact Move concurrency regressions for a fence winning first and a Move waiting for the relay identity lock while a user fence queues. The tested interleavings complete with the expected persisted/rolled-back state and zero recorded database deadlocks. The channel-TTL race also passes. These tests do not establish universal deadlock safety; the real deletion worker remains disabled.

[wpfleger96]

🤖 IMPORTANT — cost on the hottest write path, with no measurement. Each persistent event insert now adds roughly nine round trips: the owner read twice, reading, setting and restoring lock_timeout, 2–3 advisory locks, and these two restriction reads. That includes relay-signed side-effect events. Until the deletion worker is enabled, the restriction table is empty in production, so this is pure cost. Two asks:

1. Merge these reads into one query: WHERE pubkey = $1 AND (scope_community_id IS NULL OR scope_community_id = $2).
2. Add an insert-latency measurement before and after this change to the PR.

[ravarora2]

🤖 Combined the two restriction lookups in eb2c2c8 and measured the cost. The query is now pubkey = $1 AND (scope_community_id IS NULL OR scope_community_id = $2); it still checks the signer, not the owner's restriction.

Completed 49,152 successful timed writes at 4aef33b, before the main merge, comparing no guard, the original two-query guard, and the combined query. Covered both shared insert APIs, human/owned-agent signers, and one/eight concurrent writers. Full methodology and tables are in the updated PR description.

Single-writer p50 improved about 5–10% versus the two-query guard. The complete guard still added about 2.2–2.6 ms (65–81%) versus no guard. That is a material remaining cost, not a claim that overhead is solved. Measurements used local PostgreSQL, Cargo's unoptimized test profile, and an empty restriction table; they are not production capacity estimates. All temporary benchmark variants were restored; no bypass is committed.

[wpfleger96]

🤖 MINOR — duplicated mapping. The string restricted: identity is restricted for user deletion is written here and again at L3728. The downcast_ref sites also rebuild DbError::UserDeletionRestricted only to pass it back in here. Taking &DbError and returning Option<IngestError> (with the retryable ownership-change variant handled here too) would replace five match arms with one call per site.

[ravarora2]

🤖 Fixed in f1570c4, included in head 572e30e. event_write_error now takes &DbError and returns Option. It centralizes UserDeletionRestricted and EventAuthorOwnershipChanged mapping. Ingest, artifact, and command persistence callers use the helper and preserve their existing fallback for unrelated errors. The repeated refusal string and reconstructed error variants are removed. Mapping/transport regressions passed.

[wpfleger96]

🤖 MINOR — the doc describes the PR, not the system. It reads like a PR description: "This PR checks…", "The earlier 510-case live matrix validated a broader revision…", "Current validation results belong in the PR description." That goes stale on merge. Suggest rewriting it to state the rules that hold (which writes are fenced, the lock order, what is still unfenced, and what the worker must do before it's enabled), or moving it into the PR body.

[ravarora2]

🤖 Rewritten in 4aef33b, included in head 572e30e. The document now describes lasting behavior: guarded insert functions, transaction flow, signer/owner semantics, errors and atomicity, actual lock order, future exclusive-worker requirements, and remaining unfenced paths. PR history and the old live-matrix narrative are removed. Benchmark methodology/results are in the PR description.

[wpfleger96]

🤖 MINOR — nested #[path] chain. The test modules are chained message → expanded → lifecycle here, and write_guard → lock_order in buzz-db. Registering each one directly in its mod.rs (as with user_deletion_message_postgres_tests) makes them easy to find and keeps them independent.

[ravarora2]

🤖 Fixed in b76879c, included in head 572e30e. Each PostgreSQL test module is now registered directly in its package's mod.rs. Shared setup lives in user_deletion_test_support modules instead of depending on chained test-module imports. PostgreSQL test discovery passed across 638 Rust source files, and all 1,189 database/relay PostgreSQL tests passed after merging main.

[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: f3a7bab098a530ba12b909cb7121b4fa9ce70731...3564dc2dcdaec4c9007b95114d07fa21960f3d41
• 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 findings were identified 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 instructed.

---

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

[wpfleger96]

🤖 Re-review at 3564dc2dc. All eight findings from the review at 0c3ebcb53 are fixed. Archive/unarchive now stores the request, applies the archived_identities change and stores the delta in one guarded transaction. event_write_error matches only the two new error types. Ownership changes have their own retryable error. The lock order is documented as callers actually take it, with the requirements a future worker must meet. The lookup is one query and the PR now includes measurements. The doc describes the system rather than the PR, and the test modules are registered directly.

The atomic archive rewrite introduced one blocking issue and two minor ones:

IMPORTANT

1. The archive request and its 8002/8003 delta are no longer indexed in event_mentions, so #p queries for the target can't find them (archived_identities.rs L212, inline).

MINOR
2. The relay-signed delta now takes its created_at from the client's request instead of relay time (identity_archive.rs L95, inline).
3. The comment at ingest.rs L622 still says consent is checked "inside handle_identity_archive_event". That function is now a #[cfg(test)] wrapper, and production validation happens in validate_identity_archive_event. Please update the comment.

[wpfleger96]

🤖 IMPORTANT — archive requests and deltas are missing from event_mentions. Before this change, the request was stored through Db::insert_event_with_thread_metadata and the delta through Db::insert_event. Both wrappers call insert_mentions_in_transaction after a new insert (event.rs L2289–2292 and L2829–2831). This function calls the low-level insert_event_in_transaction for both events and never indexes their p tags. Single-#p queries and counts use an INNER JOIN event_mentions (event.rs L628–641, L851–861; the relay pushes #p filters down to this path in req.rs L1220–1231). As a result, newly accepted 9035/9036 requests and their 8002/8003 deltas no longer show up in a #p: [target] subscription or history query, which is how a client replays one identity's archive history.

Fix: call crate::insert_mentions_in_transaction(&mut tx, event, None) for the request when inserted, and for the delta when it's inserted, inside this transaction. Please add a regression test that queries #p = target through the production path and expects both events.

[wpfleger96]

🤖 MINOR — the relay-signed delta now uses the client's timestamp. build_nipia_delta used to stamp 8002/8003 with relay time. It now copies event.created_at from the request, which ingest accepts anywhere within ±900 s of server time (ingest.rs L2643). So a client can backdate or postdate a relay-signed event by up to 15 minutes. An archive followed by an unarchive can produce deltas whose timestamps are in the wrong order, and a backdated delta can fall before a subscriber's since. The NIP-IA vectors stamp the delta after the request (request + 1). The transaction doesn't need a deterministic timestamp, because a duplicate request never stores a second delta. Suggest building the delta with Timestamp::now().

[presentyourlove]

Hi @ravarora2! 👋

I've merged the latest main into a branch to resolve the merge conflicts on PR #8658:

Reconciled Changes:

1. crates/buzz-db/src/store/mod.rs: Preserved both pub mod verified_profiles; from main and your user deletion lock order tests.
2. crates/buzz-relay/src/handlers/command_executor.rs & side_effects.rs: Restored event_write_error identity-fencing mapping while using e.kind() for internal error formatting.
3. crates/buzz-relay/src/handlers/ingest.rs: Reconciled BanRefusal::banned with event_write_error, and preserved identity-fencing error handling across reaction, channel create, atomic deletion, edit metadata, and thread writes alongside the latest ChannelNameTaken checks.

Rebased/Merged Branch:

• Branch: https://github.com/presentyourlove/buzz/tree/fix/fence-shared-event-writes-8658
• To fetch and inspect directly:

  git fetch https://github.com/presentyourlove/buzz.git fix/fence-shared-event-writes-8658
  git merge FETCH_HEAD

Hope this helps move PR #8658 forward! 🚀