fix(relay-admin): make thread deletions atomic and fence expired action leases under row lock

[wpfleger96]

What

Two fixes to crates/buzz-db/src/store/relay_admin_actions.rs and one cleanup in crates/buzz-db/src/store/thread.rs.

1. Thread counter atomicity

The admin delete action now calls the canonical soft_delete_event_and_update_thread_in_tx body shared with NIP-29 and NIP-09 event-ID deletion. The delete, parent reply_count decrement, root descendant_count decrement, and step marker commit or roll back together in one transaction. A second action against an already-deleted target is a no-op and does not double-decrement. After a committed delete, emit_live_thread_summary refreshes the live thread count the same way DELETE_EVENT does.

2. Lock-first lease expiry fence (all five admin-action paths)

On main, the final marker UPDATE in every admin-action path (execute_ban_with_marker, execute_timeout_with_marker, execute_kick_with_marker, execute_delete_with_marker, record_failure) checks clock_timestamp() in the WHERE clause. PostgreSQL can evaluate that predicate before waiting to lock the target row. If another transaction holds the action row and releases it unchanged after the lease expires, the update commits under the stale timestamp.

On this branch, each path acquires the action row with SELECT … FOR UPDATE inside the same transaction before any domain write. The wall-clock expiry check in the marker UPDATE runs after that lock is held, not before. The pre-entry ownership SELECT EXISTS checks remain — they are early-rejection optimizations that are safe because the correct final fence rolls back any provisional work.

record_failure on main uses a single-statement execute(pool). On this branch it begins a transaction, acquires the row lock with SELECT … FOR UPDATE and an explicit expiry check, then writes state = 'failed' — matching the four mutation functions.

Lease validity is checked at the protected ownership check under the action-row lock, not at physical COMMIT completion.

Function	Change
execute_ban_with_marker	SELECT … FOR UPDATE added after pre-entry check, before domain write
execute_timeout_with_marker	same
execute_kick_with_marker	same
execute_delete_with_marker	same
record_failure	wrapped in transaction; SELECT … FOR UPDATE + clock check before state update

3. Unused standalone counter helpers removed

increment_reply_count, decrement_reply_count (both in thread.rs), and Db::decrement_reply_count have no production callers outside their forwarding wrappers. Both perform unguarded autocommit writes that bypass the deletion-transition/atomicity invariant. Removed to prevent future misuse.

[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: 8f6b66f9ff6b2e42cd713506805763269375b473...7b1edfe150ce3049ac3811cfd102bbe68b282bfa
• Model: gpt-5.6-sol

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

---

Review Summary

Overall Risk: NONE

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

Findings

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

Notes

• No additional limitations were reported.

---

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

[wesbillman]

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

Review clear: no blocking code, product, or security defects found. Reviewed HEAD dc6c535da566c5c392cf9f33a023a6b904eae8eb against BASE d01e5f82058463709a22e93bb4cd795da5f53e10.

Source review covered canonical deletion/counter atomicity and idempotency, tenant scope, removed-helper callers, all five final lease fences and ownership/recovery paths, and canonical best-effort live-summary integration. One non-blocking timing-test robustness note is inline.

Validation: existing PostgreSQL CI passed 476 tests, including all four new regressions, on GitHub’s merge of this head into the pinned base. This review ran no code or tests.

CI is not fully green: Desktop Smoke E2E (4) fails the middle-page scroll coverage assertion. That unchanged spec runs against an in-browser mock bridge without the Rust relay/database, so it does not exercise this change. The desktop gate still needs separate disposition; this is not a merge approval.

[wesbillman]

Non-blocking test robustness: the 500ms lease starts before enforcing_action completes its database setup. If setup consumes that interval, execute_delete_with_marker exits at the unchanged pre-entry ownership check, and the assertions still pass without reaching the final clock_timestamp() fence. Please ensure the lease is live after setup (and fail explicitly if that precondition is lost), rather than allowing slow setup to silently turn this into another already-expired-lease test. No production fencing defect found; this is a weakness in the regression test's evidence.

[wesbillman]

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

Review clear: no blocking code, product, or security defects found in the corrective change. HEAD 16cb762f692aa0a289bdd6fd40d024b177c9fe51, BASE d01e5f82058463709a22e93bb4cd795da5f53e10.

Follow-up to the prior review: checked all five lock-first/final-clock fences, ownership changes during waits, deletion/counter rollback, and post-commit summary integration. The new observed-lock/DB-clock regressions address the earlier timing-test concern. Lease validity is checked under the action-row lock, not at physical COMMIT completion.

Validation: existing PostgreSQL CI passed 477/477 on merge c98b1710d0ebedd869af1b787b7daf54e4293944, containing this head but using base 99c2acf90cfbb1cb2d3a8bd900c0ec1642e20540. This is not standalone-head or pinned-base certification. This source-only review ran no code or tests.

Non-blocking housekeeping: name the two new pg_stat_activity regressions with cluster_global_, per crates/buzz-db/TESTING.md:40–43. Both queries filter to their database and exact blocker PID; no concrete cross-test failure was established, so this is not a correctness blocker.

Broader CI was red in the snapshot, with relay/integration E2E failures and some checks still running. Their cause was not established here; those gates still need disposition. This is not merge approval.

[wesbillman]

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

CLEAR: no actionable regression found since the prior clear review.

• The branch-specific delta renames two PostgreSQL lease-expiry tests into the existing cluster_global_ serial group, resolving the prior housekeeping note. The production deletion/counter, lease-fence, and live-summary paths are unchanged. Main-merge integration was reviewed separately.
• Source-only review with an independent DB lane; no builds, tests, or PR code executed. Existing exact-head PostgreSQL CI reports success. The broader snapshot had 50 successful checks, 26 skipped and two desktop smoke checks still running, so this is not an all-green claim.
• No code fixes requested. Required CI remains a separate merge gate; unchanged unrelated surfaces were excluded.

Head: 7b1edfe150ce3049ac3811cfd102bbe68b282bfa. Base: 02753722a7dd06560402a5b92491b048968c1a63.