Centralize replaceable event persistence - #6660
Conversation
Signed-off-by: tornquist <tornquist@squareup.com>
Signed-off-by: tornquist <tornquist@squareup.com>
Signed-off-by: tornquist <tornquist@squareup.com>
Signed-off-by: tornquist <tornquist@squareup.com>
wpfleger96
left a comment
There was a problem hiding this comment.
🤖 Reviewed at 561de54 against the old implementations in buzz-db/src/lib.rs and the relay command executor. The moved state machine is semantics-preserving: lock-key derivation is byte-identical (including the membership-snapshot lock-order call site), NIP-33 dominance/same-second tie-break, NIP-RS watermark + hard-delete gating, and the event-before-mentions delete order all carry over. The workflow CAS mapping in the command path (replay-before-validation for malformed legacy revisions, dominated-CAS conflict, revision missing/mismatch) matches the old behavior, and the savepoint correctly contains mention-index failures and the buzz.nip_rs_hard_delete opt-in inside a caller-owned transaction.
Two intentional behavior deltas I verified and think are fine: mention-index failure on the NIP-33 ingest path now rejects instead of warn-and-accept (atomicity improvement, tested), and workflow-def command events now get mention indexing they previously skipped. One small ask for the follow-up PR you mentioned: the old replace_parameterized_event doc comment explaining the channel policy (replacement keys on (kind, pubkey, d_tag) globally, contrast with replace_addressable_event) and the migration 0009/0011 rationale comments got trimmed in the move — worth restoring, they're load-bearing. The CI-gate coverage gap is deferred to that separate PR per discussion.
…-rc3 * origin/main: (86 commits) Centralize replaceable event persistence (#6660) feat(workflows): discover trigger filter values (#6712) feat(desktop): simplify the message action rail (#6529) fix(desktop): restore icon-only remote marker (#6491) fix(ci): prevent poisoned Rust caches (#6618) docs(security): route reports through private advisories (#6728) fix(composer): wrap Buzz chip labels without orphaning icons (#6581) fix(desktop): bound thread /query and surface load errors, not false-empty (#6447) fix(messages): route edits to the owning composer (#6575) fix(mobile): join starter channels after accepting invite (#5915) Add mobile profile editing (#6583) fix(desktop): align jump-to-latest pill with composer height (#6606) fix(desktop): emit singular `mention` feed category so alerts route correctly (#6665) fix(mobile): recover stale and shuffled messages (#6691) feat(mobile): browse and join open channels (#6243) show mention counts in channel notifications (#6696) fix(desktop): hide selection formatting tray on composer right-click (#6683) fix(desktop): stabilize members dialog scrolling (#6670) fix(desktop): keep member runtime status off the UI thread (#6445) perf(desktop): persist channel heads, collapse thread reads and reply sends (#6572) ...
…ifications-pr * origin/main: Centralize replaceable event persistence (#6660) feat(workflows): discover trigger filter values (#6712) feat(desktop): simplify the message action rail (#6529) fix(desktop): restore icon-only remote marker (#6491) fix(ci): prevent poisoned Rust caches (#6618) docs(security): route reports through private advisories (#6728) fix(composer): wrap Buzz chip labels without orphaning icons (#6581) fix(desktop): bound thread /query and surface load errors, not false-empty (#6447) Signed-off-by: Tom Brow <tomb@block.xyz>
## Why Community persistence is the next incremental `buzz-db` store extraction, keeping tenant lifecycle SQL, records, tests, and instrumentation out of the database runtime module without changing behavior. ## What - Move community records and the existing `impl Db` operations into `community.rs` while preserving crate-root re-exports. - Move focused PostgreSQL tests with the implementation and enforce single ownership for each method and datastore span. ## Risk Assessment Low — this is a structural move of the existing records, SQL, method bodies, and focused tests; database runtime concerns, schema, and behavior remain unchanged. ## References - Architecture guardrail: TheSentinel454#34 - Incremental tracker: TheSentinel454#2 - Primary task: TheSentinel454#5 - Stacked on: #6660 Generated with Codex Signed-off-by: tornquist <tornquist@squareup.com>
…c-agent-commit-identity * origin/main: (54 commits) Extract community persistence (#6668) Fix mobile Huddle agent voice turn states (#6611) Add inline profile camera capture (#6680) Hide Huddles in mobile agent DMs (#6676) fix(desktop): polish inline chip states (#6718) Centralize replaceable event persistence (#6660) feat(workflows): discover trigger filter values (#6712) feat(desktop): simplify the message action rail (#6529) fix(desktop): restore icon-only remote marker (#6491) fix(ci): prevent poisoned Rust caches (#6618) docs(security): route reports through private advisories (#6728) fix(composer): wrap Buzz chip labels without orphaning icons (#6581) fix(desktop): bound thread /query and surface load errors, not false-empty (#6447) fix(messages): route edits to the owning composer (#6575) fix(mobile): join starter channels after accepting invite (#5915) Add mobile profile editing (#6583) fix(desktop): align jump-to-latest pill with composer height (#6606) fix(desktop): emit singular `mention` feed category so alerts route correctly (#6665) fix(mobile): recover stale and shuffled messages (#6691) feat(mobile): browse and join open channels (#6243) ... Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
…trigger-foundation * origin/main: Extract community persistence (#6668) Fix mobile Huddle agent voice turn states (#6611) Add inline profile camera capture (#6680) Hide Huddles in mobile agent DMs (#6676) fix(desktop): polish inline chip states (#6718) Centralize replaceable event persistence (#6660) feat(workflows): discover trigger filter values (#6712) feat(desktop): simplify the message action rail (#6529) Signed-off-by: Logan Johnson <loganj@squareup.com> # Conflicts: # crates/buzz-relay/src/handlers/command_executor.rs
…picker * origin/main: (57 commits) Add staging dev relay image workflow (#6709) Extract community persistence (#6668) Fix mobile Huddle agent voice turn states (#6611) Add inline profile camera capture (#6680) Hide Huddles in mobile agent DMs (#6676) fix(desktop): polish inline chip states (#6718) Centralize replaceable event persistence (#6660) feat(workflows): discover trigger filter values (#6712) feat(desktop): simplify the message action rail (#6529) fix(desktop): restore icon-only remote marker (#6491) fix(ci): prevent poisoned Rust caches (#6618) docs(security): route reports through private advisories (#6728) fix(composer): wrap Buzz chip labels without orphaning icons (#6581) fix(desktop): bound thread /query and surface load errors, not false-empty (#6447) fix(messages): route edits to the owning composer (#6575) fix(mobile): join starter channels after accepting invite (#5915) Add mobile profile editing (#6583) fix(desktop): align jump-to-latest pill with composer height (#6606) fix(desktop): emit singular `mention` feed category so alerts route correctly (#6665) fix(mobile): recover stale and shuffled messages (#6691) ... Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
…arer-auth * origin/main: (58 commits) Fix TipTap editor mount race (#6779) feat(buzz-agent): gate LLM tool calls on session/request_permission (#5712) Add staging dev relay image workflow (#6709) Extract community persistence (#6668) Fix mobile Huddle agent voice turn states (#6611) Add inline profile camera capture (#6680) Hide Huddles in mobile agent DMs (#6676) fix(desktop): polish inline chip states (#6718) Centralize replaceable event persistence (#6660) feat(workflows): discover trigger filter values (#6712) feat(desktop): simplify the message action rail (#6529) fix(desktop): restore icon-only remote marker (#6491) fix(ci): prevent poisoned Rust caches (#6618) docs(security): route reports through private advisories (#6728) fix(composer): wrap Buzz chip labels without orphaning icons (#6581) fix(desktop): bound thread /query and surface load errors, not false-empty (#6447) fix(messages): route edits to the owning composer (#6575) fix(mobile): join starter channels after accepting invite (#5915) Add mobile profile editing (#6583) fix(desktop): align jump-to-latest pill with composer height (#6606) ... Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz> # Conflicts: # crates/buzz-db/src/lib.rs
Co-authored-by: Matt Kursmark <kursmark@squareup.com> Signed-off-by: Matt Kursmark <kursmark@squareup.com> * origin/main: (21 commits) feat: navigate images across message threads (block#6705) Add database pressure observability (block#6700) revert fixed mention highlight (block#6716) highlight search terms in results and messages (block#6702) fix(desktop): make lightbox zoom controls interactive (block#6710) Support community deletion in versioned media buckets (block#6738) Fix TipTap editor mount race (block#6779) feat(buzz-agent): gate LLM tool calls on session/request_permission (block#5712) Add staging dev relay image workflow (block#6709) Extract community persistence (block#6668) Fix mobile Huddle agent voice turn states (block#6611) Add inline profile camera capture (block#6680) Hide Huddles in mobile agent DMs (block#6676) fix(desktop): polish inline chip states (block#6718) Centralize replaceable event persistence (block#6660) feat(workflows): discover trigger filter values (block#6712) feat(desktop): simplify the message action rail (block#6529) fix(desktop): restore icon-only remote marker (block#6491) fix(ci): prevent poisoned Rust caches (block#6618) docs(security): route reports through private advisories (block#6728) ... Signed-off-by: Matt Kursmark <kursmark@squareup.com>
## Why Finish the replaceable-event slice of [tracker #2](TheSentinel454#2) and [domain issue #6](TheSentinel454#6) without disturbing the runtime/store boundary established by #6660 and #6668. PR #6700 has merged; this PR now targets current main containing its replacement lock and transaction observability. ## What - Move `Db::replace_addressable_event`, its SQL, lock/transaction instrumentation, and focused addressable/parameterized replacement tests from `lib.rs` to `replaceable.rs` - Preserve the transaction-required parameterized API, replacement ordering, rollback semantics, mention indexing, and exactly one datastore span per public operation - Keep implementation and focused PostgreSQL tests co-located; follow-up review removed the dedicated replaceable ownership source guard as low value ## Stack - Exact base: main at f249710 - Exact head: codex/issue-6-finish-replaceable-store at da01840 - Tracker: TheSentinel454#2 - Domain: TheSentinel454#6 - Test/span acceptance: TheSentinel454#17 and TheSentinel454#19 ## Non-goals - No SQL, schema, retry, timeout, lock ordering, transaction boundary, or client-visible behavior changes - No store traits, domain-handle redesign, `PgExecutor` migration, raw pool accessor, new crate, or directory reorganization - No changes to, retargeting of, or merge action on PR #6700 ## Risk Assessment Low. This is a mechanical ownership move with unchanged signatures and SQL. The primary review risk is losing or nesting instrumentation, covered by exact moved PostgreSQL tests and the cumulative production-diff audit. ## Blox Verification Author workstation: `buzz-tornquist-issue-2-store-stack` (`2046520`), exact head `2de5444e14606ce6912d1bb932bc88bffa379a77`. - `cargo fmt --all --check` — passed - `cargo clippy -p buzz-db -p buzz-relay --all-targets -- -D warnings` — passed - `cargo test -p buzz-db --quiet` — 108 passed, 200 ignored; both integration source guards passed - Native PostgreSQL: `cargo test -p buzz-db replaceable::tests:: -- --ignored --test-threads=1` — 11 passed - `cargo test -p buzz-relay --lib api::mesh_demo::tests::demo_join_forwarded_arm_round_trips_echo -- --exact --test-threads=1` — passed - Full serial relay suite was also exercised: 908 tests passed but the existing load-sensitive mesh demo test returned 504 under suite load, matching PR #6700's documented baseline; it passed in isolation and this diff does not touch that subsystem Independent exact-head Blox review: `buzz-tornquist-pr-6777-review` (`2047422`) at `2de5444e14606ce6912d1bb932bc88bffa379a77`. - Review findings — none (critical, important, or minor) - `cargo fmt --all --check` and the ownership guard — passed - `cargo clippy -p buzz-db -p buzz-relay --all-targets -- -D warnings` — passed - `cargo test -p buzz-db --quiet` — 108 passed, 200 ignored; source guards passed - Native PostgreSQL moved suite — 11 passed - `cargo test -p buzz-relay --lib` — 909 passed, 48 ignored Generated with Codex ## Superseded pre-comment restack verification PR #6700 merged before publication completed. This layer was restacked onto current main through the exact parent named above; the final cumulative tip is 2ddcc8a. Cumulative author gates passed: formatting and diff checks; buzz-db and buzz-relay all-target clippy with -D warnings; DB lib 111 passed / 200 ignored; ownership 22/22; observability 1/1; the full isolated PostgreSQL domain matrix; and relay lib 910 passed / 49 ignored. - Workstation: `buzz-tornquist-pr-6777-final-review` (`2057617`), fresh shallow checkout - Base: `f24971033178926153b49d320bd876d15d9cb2bf` - Head: `ffbeaaf00810aa359ab85818ff4820f92263e45f` - Findings: none Reviewed `base..head` for the replaceable-store extraction. The addressable replacement SQL, stale/duplicate outcomes, advisory-lock ordering, transaction rollback/commit boundaries, mention-index atomicity, and public `Db` signature are retained in `replaceable.rs`. The PR #6700 transaction timer remains around the same logical transaction through `TransactionTimer::observe`, and the public operation has exactly one datastore span. Focused addressable and parameterized tests moved with the implementation; the ownership guard excludes duplicates from `lib.rs`. Verification: format and diff checks passed; `buzz-db --all-targets` clippy passed with `-D warnings`; DB lib tests passed (111 passed, 200 PostgreSQL tests ignored); ownership (1/1) and observability (1/1) guards passed; all 11 replaceable PostgreSQL tests passed on native PostgreSQL 17 with migrations 1-32 successful; relay lib test target compiled successfully. Final worktree was detached at the exact head and clean. Complete evidence archive SHA-256: `e37f92bbde4e2343196085095b3093b1bc78196be58bc86826325e0e1639d7ea`. ## Comment-addressed restack Review follow-up removed the dedicated `replaceable_store_has_single_ownership` source test as requested. No production code changed. - Exact base: `f24971033178926153b49d320bd876d15d9cb2bf` - Exact head: `da018405cc83605362125c2d5e5a3f91492431ac` - Final cumulative tip: `6fa2f104d42c6ba85bdf62e7ccb74ceaf4a84f67` - Focused PR verification: formatting and diff checks passed; strict `buzz-db`/`buzz-relay` Clippy passed; DB lib 111 passed / 200 ignored; all 11 replaceable PostgreSQL tests passed. - Cumulative Blox gate: formatting and diff checks; strict `buzz-db`/`buzz-relay` Clippy; DB lib 111 passed / 200 ignored; ownership 21/21; observability 1/1; every moved PostgreSQL test; relay lib 910 passed / 49 ignored. - Independent re-review at this exact head: no findings; fresh exact-parent/head Blox review passed fmt/diff, strict `buzz-db` Clippy, DB lib 111 passed / 200 ignored, manual one-owner/one-span replacement checks, observability, 11 replaceable PostgreSQL tests, and relay compilation. --------- Signed-off-by: tornquist <tornquist@squareup.com>
Why
Command persistence duplicated NIP-33 replacement SQL in the relay and obscured the boundary between database runtime concerns and domain-store behavior. This proving slice establishes that boundary inside the existing
buzz-dbcrate.What
buzz-dbRisk Assessment
Medium — this changes core event persistence and command idempotency paths, while intentionally preserving NIP-33, NIP-RS, mesh, and workflow conflict semantics.
References
Generated with Codex