Skip to content

feat: soft-delete for events/channels, enriched API responses, NIP-29 group management - #17

Merged
tlongwell-block merged 1 commit into
mainfrom
feat/channel-polish
Mar 10, 2026
Merged

tlongwell-block merged 1 commit into
mainfrom
feat/channel-polish

Conversation

@tlongwell-block

Copy link
Copy Markdown
Collaborator

Summary

Implements soft-delete for events and channels, enriches API responses with metadata, and completes NIP-29 group management (kinds 9005/9008). This is the Phase 6 "channel polish" milestone.

12 files changed, +866 -59

What changed

Soft-delete infrastructure

  • soft_delete_event() / soft_delete_channel() — idempotent UPDATE SET deleted_at = NOW(6) WHERE deleted_at IS NULL
  • soft_delete_event_and_update_thread() — atomic transaction wrapping delete + thread counter decrements (reply_count, descendant_count), floors at 0
  • insert_event_with_thread_metadata() — atomic transaction wrapping event insert + thread metadata + counter increments (prevents race where concurrent delete between separate inserts corrupts counters)
  • All read queries filter deleted_at IS NULL: query_events, get_event_by_id, is_member, get_members, get_member_role, get_accessible_channel_ids
  • get_event_by_id_including_deleted() variant for audit/compliance use cases

REST API endpoints

  • DELETE /api/messages/{event_id} — author or channel owner/admin auth, effective author resolution for relay-signed messages (real sender in p-tag), atomic delete + counter decrement, fails hard on metadata lookup errors
  • DELETE /api/channels/{channel_id} — owner-only auth, soft-delete + system message emission

Enriched API responses

  • Channel list (GET /api/channels) — now includes member_count, last_message_at, visibility, topic, purpose, created_by, created_at, updated_at, archived_at
  • Message list (GET /api/channels/{id}/messages) — thread summaries always included (not gated by param), bulk reaction counts embedded per message
  • Thread detail (GET /api/channels/{id}/messages/{id}/thread) — bulk reaction counts for root + all replies

NIP-29 group management completion

  • Kind 9005 (DELETE_EVENT) — pre-storage validation checks author (via p-tag) OR owner/admin, verifies target event belongs to h-tag channel (prevents cross-channel deletes), post-storage side effect performs atomic soft-delete
  • Kind 9008 (DELETE_GROUP) — soft-deletes channel, emits system message
  • NIP-11 supported_nips updated to [1, 11, 25, 29, 42]

Performance

  • Replaced N+1 queries in channels_handler with bulk queries: get_member_counts_bulk() and get_last_message_at_bulk() using QueryBuilder with separated() for IN clauses
  • Bulk reaction fetching via get_reactions_bulk() in message list and thread endpoints
  • reactions_to_json() helper deduplicates serialization logic

Authorization hardening

  • effective_author() / effective_message_author() — extracts real author from p-tag for relay-signed REST messages (used in delete auth and NIP-29 validation)
  • Cross-channel delete prevention in both pre-storage validation and post-storage side effects
  • NIP-29 kind 9005 no longer allows any member to delete — requires authorship or owner/admin role

Quality gates

Check Status
cargo build ✅ Clean, 0 warnings
cargo test --lib ✅ 121 passed
cargo fmt --check ✅ Clean
cargo clippy -- -D warnings ✅ Clean

Review notes

This went through 3 rounds of multi-model crossfire review (Claude, Codex, GPT-5.2, Opus). 13 issues were found and fixed:

  • Round 1: Double-decrement on repeated deletes, missing deleted_at filters on get_event_by_id/query_events, NIP-29 auth too permissive (any member could delete), N+1 queries in channel list
  • Round 2: Deleted channels still accessible via membership helpers (missing JOIN to channels.deleted_at IS NULL)
  • Round 3: REST delete silently corrupted counters on metadata lookup failure, NIP-29 delete emitted false audit records when deleted==false, pre-storage validation missing same-channel check for kind 9005

Known deferred items (not blocking)

  • Effective author on read paths — message lists still show relay pubkey for REST-created messages (requires DB struct changes)
  • last_reply_at recomputation on delete — deleting the latest reply does not recompute the timestamp (can add periodic reconciliation)
  • E2E regression tests for all 13 fixes

…pletion

Implements soft-delete for events (NIP-29 kind 9005) and channels (kind 9008),
replacing TODO stubs with real logic. Adds REST DELETE endpoints for both.

Changes:
- soft_delete_event/channel: UPDATE SET deleted_at = NOW(6) WHERE deleted_at IS NULL
- DELETE /api/messages/{event_id}: author or owner/admin auth, thread counter decrement
- DELETE /api/channels/{channel_id}: owner-only auth
- get_event_by_id now filters deleted_at IS NULL (+ _including_deleted variant)
- query_events now filters deleted_at IS NULL
- list_channels enriched: member_count, last_message_at, topic, purpose, visibility
- list_messages/get_thread: reaction counts embedded via get_reactions_bulk
- Thread summaries always included in channel history (no longer gated by param)
- NIP-11 supported_nips: [1, 11, 25, 29, 42]
- decrement_reply_count wired through Db, guarded against double-decrement
- VISION.md: channel features marked as implemented

Crossfire reviewed (Claude 8/10, Codex 3/10, GPT-5.2 6/10).
Three critical issues found and fixed:
1. Double-decrement guard on repeated NIP-29 deletes
2. get_event_by_id/query_events now filter soft-deleted rows
3. get_event_by_id_including_deleted added for audit use cases

12 files changed, +403 -22 lines. All quality gates pass.
@tlongwell-block
tlongwell-block merged commit 69b36d1 into main Mar 10, 2026
8 checks passed
@tlongwell-block
tlongwell-block deleted the feat/channel-polish branch March 10, 2026 20:01
tlongwell-block added a commit that referenced this pull request Mar 11, 2026
* origin/main:
  feat: soft-delete for events/channels, enriched API responses, NIP-29 group management (#17)
  feat: Channel management, messaging, threads, DMs, reactions, and NIP-29 support (#16)
  Improve chat scrolling and multiline composer (#14)
  chore: remove redundant inline comments across all crates (#13)
  Initial backend revisions, workflow expansion (#5)
  Add desktop Home feed (#12)
  Add desktop Playwright e2e harness (#11)
  Update desktop icon and persist window state (#9)
  feat: add channel creation flow (#8)

@kyoko-antigency kyoko-antigency left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Open 4.8 Adversarial Review — PR #17

Repository: antigency/buzz-server
PR Number: #17
Branch: kyoko/rustsec-cascade-independent
Base: commercial/main
Head SHA: 1a036e715e91d5543219a5f54878f22f4df9d057

KIMI_REVIEW_HEAD: 1a036e715e91d5543219a5f54878f22f4df9d057


Executive Summary

KIMI_REVIEW_VERDICT: SHIP

This is a clean, minimal, and correctly-scoped security dependency update that resolves 11 RustSec advisories with no functional changes to application code. All version bumps are justified by documented security fixes, and the deny.toml suppression is valid (dev-dep only, no production exposure).


Changes Review

1. Dependency Version Bumps

nostr: 0.44.7 → 0.44.8

File: Cargo.toml (workspace root)
Justification: Resolves 6 RustSec advisories:

  • RUSTSEC-2026-0216 (NIP-44 v2 DoS)
  • RUSTSEC-2026-0219 (NIP-04 IV DoS)
  • RUSTSEC-2026-0225 (NIP-46/60 debug credential leak)
  • RUSTSEC-2026-0226 (wallet event auth bypass)
  • RUSTSEC-2026-0229 (NIP-98 resource exhaustion)
  • RUSTSEC-2026-0230 (NIP-50 empty search panic)

Assessment: ✅ APPROVED

  • Patch-level bump (0.44.7 → 0.44.8)
  • Minimal scope, standard practice for security fixes
  • All 6 advisories are legitimate security issues in the nostr crate

nostr-relay-pool: 0.44.1 → 0.44.3 (transitive)

Dependency Path: Transitive via mesh-llm → iroh
Justification: Resolves 3 advisories:

  • RUSTSEC-2026-0224 (verification cache poisoning)
  • RUSTSEC-2026-0231 (AUTH challenge memory exhaustion)
  • RUSTSEC-2026-0232 (unverified relay event processing)

Assessment: ✅ APPROVED

  • Transitive update via Cargo.lock cascade (correct mechanism)
  • No direct dependency in workspace manifests (appropriate)
  • Patch-level bump (0.44.1 → 0.44.3)

webbrowser: 1.2.1 → 1.2.4

File: crates/buzz-agent/Cargo.toml
Justification: Resolves RUSTSEC-2026-0257 (BROWSER env var argument injection)

Assessment: ✅ APPROVED

  • Patch-level bump (1.2.1 → 1.2.4)
  • Correctly scoped to buzz-agent (only direct consumer)
  • The manifest change ("1" → "1.2") is a semver-spec tightening to pin to 1.2.x series (acceptable practice for security-sensitive transitive deps)

2. deny.toml Suppression

Added Entry:

# nostr-relay-pool 0.44.3 — unmaintained (RUSTSEC-2026-0243). Transitive dev-dep
# via mesh-llm → iroh → nostr-relay-pool; no production code path. Awaiting
# upstream mesh-llm / iroh dependency update.
{ id = "RUSTSEC-2026-0243", reason = "dev-dep only via mesh-llm; no production impact; awaiting upstream bump" },

Assessment: ✅ APPROVED

  • Reasoning is accurate: nostr-relay-pool is a transitive dev-dep via mesh-llm
  • Production code paths do not exercise mesh-llm (dev-dependency of buzz-relay for local testing)
  • Suppression is time-bounded (awaiting upstream fix) — appropriate interim measure
  • RUSTSEC-2026-0243 declares the crate "unmaintained," but the advisory describes no active vulnerability (standard practice to suppress unmaintained-only advisories when no exploit path exists)

Verification:

# From dependency tree in verification run:
nostr-relay-pool v0.44.3
└── iroh v1.0.2
    └── mesh-llm-* (multiple workspace members)
        └── (dev) buzz-relay v0.2.0

Confirmed: dev-dependency only. No production exposure.


3. Cargo.lock Changes

Lines Modified: 53 insertions(+), 23 deletions(-)
Assessment: ✅ APPROVED

  • Lockfile cascade matches expected transitive updates
  • No unrelated package version changes observed
  • objc2-app-kit v0.3.2 addition is a new transitive dep of webbrowser 1.2.4 (macOS-specific; expected for browser-launch crate)

Validation Evidence

Security Gates

cargo deny check advisories: ok (11 vulnerability errors → 0)

Assessment: ✅ PASS
All advisories documented in the commit message are resolved. No new advisories introduced.

Functional Tests

cargo test -p buzz-agent: ok (274 passed)

Assessment: ✅ PASS
Direct consumer of webbrowser (buzz-agent) passes all unit tests.

Build Verification

cargo build -p buzz-relay -p buzz-agent: ok

Assessment: ✅ PASS
Core packages build cleanly with updated dependencies.


Adversarial Checks

1. Version Bump Scope Inflation

Check: Do any version bumps exceed documented advisory fixes?
Result: ✅ NO
All bumps are patch-level and directly tied to RustSec advisories.

2. Unintended Dependency Introduction

Check: Does this PR introduce new production dependencies?
Result: ✅ NO
objc2-app-kit is a new transitive dep of webbrowser (macOS-only; expected). No new direct dependencies.

3. Suppression Validity

Check: Is the deny.toml suppression masking a production vulnerability?
Result: ✅ NO
nostr-relay-pool is dev-dep only. RUSTSEC-2026-0243 documents "unmaintained" status, not an active CVE.

4. Lockfile Integrity

Check: Does Cargo.lock contain unexpected changes?
Result: ✅ NO
All changes are cascaded updates from the three documented dependency bumps.

5. Commit Message Accuracy

Check: Does the commit message accurately describe all changes?
Result: ✅ YES
All advisories, version bumps, and validation steps are documented.


Security Impact Assessment

Advisory Severity Resolved? Production Exposure
RUSTSEC-2026-0216 HIGH Yes (nostr in buzz-relay)
RUSTSEC-2026-0219 HIGH Yes (nostr in buzz-relay)
RUSTSEC-2026-0224 MEDIUM No (dev-dep only)
RUSTSEC-2026-0225 MEDIUM Yes (nostr in buzz-relay)
RUSTSEC-2026-0226 HIGH Yes (nostr in buzz-relay)
RUSTSEC-2026-0229 MEDIUM Yes (nostr in buzz-relay)
RUSTSEC-2026-0230 MEDIUM Yes (nostr in buzz-relay)
RUSTSEC-2026-0231 MEDIUM No (dev-dep only)
RUSTSEC-2026-0232 HIGH No (dev-dep only)
RUSTSEC-2026-0243 INFO Suppressed No (dev-dep only)
RUSTSEC-2026-0257 MEDIUM Limited (buzz-agent OAuth flow only)

Net Impact: 7 production-impacting advisories resolved (3 HIGH, 4 MEDIUM). No new vulnerabilities introduced.


Recommendation

SHIP — This PR:

  1. Correctly addresses all documented RustSec advisories
  2. Uses minimal version bumps (patch-level only)
  3. Includes valid dev-dep suppression with clear reasoning
  4. Passes all security and functional validation gates
  5. Introduces no unintended scope changes

Merge Conditions: All CI gates green.


Reviewer: Open 4.8 (Claude 3.7 Sonnet adversarial security agent)
Review Date: 2026-08-12 07:40 ET
Commit Reviewed: 1a036e715e91d5543219a5f54878f22f4df9d057

@antigency antigency left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Open 4.8 Adversarial Review — PR #17

Repository: antigency/buzz-server
PR Number: #17
Branch: kyoko/rustsec-cascade-independent
Base: commercial/main
Head SHA: 1a036e715e91d5543219a5f54878f22f4df9d057

KIMI_REVIEW_HEAD: 1a036e715e91d5543219a5f54878f22f4df9d057


Executive Summary

KIMI_REVIEW_VERDICT: SHIP

This is a clean, minimal, and correctly-scoped security dependency update that resolves 11 RustSec advisories with no functional changes to application code. All version bumps are justified by documented security fixes, and the deny.toml suppression is valid (dev-dep only, no production exposure).


Changes Review

1. Dependency Version Bumps

nostr: 0.44.7 → 0.44.8

File: Cargo.toml (workspace root)
Justification: Resolves 6 RustSec advisories:

  • RUSTSEC-2026-0216 (NIP-44 v2 DoS)
  • RUSTSEC-2026-0219 (NIP-04 IV DoS)
  • RUSTSEC-2026-0225 (NIP-46/60 debug credential leak)
  • RUSTSEC-2026-0226 (wallet event auth bypass)
  • RUSTSEC-2026-0229 (NIP-98 resource exhaustion)
  • RUSTSEC-2026-0230 (NIP-50 empty search panic)

Assessment: ✅ APPROVED

  • Patch-level bump (0.44.7 → 0.44.8)
  • Minimal scope, standard practice for security fixes
  • All 6 advisories are legitimate security issues in the nostr crate

nostr-relay-pool: 0.44.1 → 0.44.3 (transitive)

Dependency Path: Transitive via mesh-llm → iroh
Justification: Resolves 3 advisories:

  • RUSTSEC-2026-0224 (verification cache poisoning)
  • RUSTSEC-2026-0231 (AUTH challenge memory exhaustion)
  • RUSTSEC-2026-0232 (unverified relay event processing)

Assessment: ✅ APPROVED

  • Transitive update via Cargo.lock cascade (correct mechanism)
  • No direct dependency in workspace manifests (appropriate)
  • Patch-level bump (0.44.1 → 0.44.3)

webbrowser: 1.2.1 → 1.2.4

File: crates/buzz-agent/Cargo.toml
Justification: Resolves RUSTSEC-2026-0257 (BROWSER env var argument injection)

Assessment: ✅ APPROVED

  • Patch-level bump (1.2.1 → 1.2.4)
  • Correctly scoped to buzz-agent (only direct consumer)
  • The manifest change ("1" → "1.2") is a semver-spec tightening to pin to 1.2.x series (acceptable practice for security-sensitive transitive deps)

2. deny.toml Suppression

Added Entry:

# nostr-relay-pool 0.44.3 — unmaintained (RUSTSEC-2026-0243). Transitive dev-dep
# via mesh-llm → iroh → nostr-relay-pool; no production code path. Awaiting
# upstream mesh-llm / iroh dependency update.
{ id = "RUSTSEC-2026-0243", reason = "dev-dep only via mesh-llm; no production impact; awaiting upstream bump" },

Assessment: ✅ APPROVED

  • Reasoning is accurate: nostr-relay-pool is a transitive dev-dep via mesh-llm
  • Production code paths do not exercise mesh-llm (dev-dependency of buzz-relay for local testing)
  • Suppression is time-bounded (awaiting upstream fix) — appropriate interim measure
  • RUSTSEC-2026-0243 declares the crate "unmaintained," but the advisory describes no active vulnerability (standard practice to suppress unmaintained-only advisories when no exploit path exists)

Verification:

# From dependency tree in verification run:
nostr-relay-pool v0.44.3
└── iroh v1.0.2
    └── mesh-llm-* (multiple workspace members)
        └── (dev) buzz-relay v0.2.0

Confirmed: dev-dependency only. No production exposure.


3. Cargo.lock Changes

Lines Modified: 53 insertions(+), 23 deletions(-)
Assessment: ✅ APPROVED

  • Lockfile cascade matches expected transitive updates
  • No unrelated package version changes observed
  • objc2-app-kit v0.3.2 addition is a new transitive dep of webbrowser 1.2.4 (macOS-specific; expected for browser-launch crate)

Validation Evidence

Security Gates

cargo deny check advisories: ok (11 vulnerability errors → 0)

Assessment: ✅ PASS
All advisories documented in the commit message are resolved. No new advisories introduced.

Functional Tests

cargo test -p buzz-agent: ok (274 passed)

Assessment: ✅ PASS
Direct consumer of webbrowser (buzz-agent) passes all unit tests.

Build Verification

cargo build -p buzz-relay -p buzz-agent: ok

Assessment: ✅ PASS
Core packages build cleanly with updated dependencies.


Adversarial Checks

1. Version Bump Scope Inflation

Check: Do any version bumps exceed documented advisory fixes?
Result: ✅ NO
All bumps are patch-level and directly tied to RustSec advisories.

2. Unintended Dependency Introduction

Check: Does this PR introduce new production dependencies?
Result: ✅ NO
objc2-app-kit is a new transitive dep of webbrowser (macOS-only; expected). No new direct dependencies.

3. Suppression Validity

Check: Is the deny.toml suppression masking a production vulnerability?
Result: ✅ NO
nostr-relay-pool is dev-dep only. RUSTSEC-2026-0243 documents "unmaintained" status, not an active CVE.

4. Lockfile Integrity

Check: Does Cargo.lock contain unexpected changes?
Result: ✅ NO
All changes are cascaded updates from the three documented dependency bumps.

5. Commit Message Accuracy

Check: Does the commit message accurately describe all changes?
Result: ✅ YES
All advisories, version bumps, and validation steps are documented.


Security Impact Assessment

Advisory Severity Resolved? Production Exposure
RUSTSEC-2026-0216 HIGH Yes (nostr in buzz-relay)
RUSTSEC-2026-0219 HIGH Yes (nostr in buzz-relay)
RUSTSEC-2026-0224 MEDIUM No (dev-dep only)
RUSTSEC-2026-0225 MEDIUM Yes (nostr in buzz-relay)
RUSTSEC-2026-0226 HIGH Yes (nostr in buzz-relay)
RUSTSEC-2026-0229 MEDIUM Yes (nostr in buzz-relay)
RUSTSEC-2026-0230 MEDIUM Yes (nostr in buzz-relay)
RUSTSEC-2026-0231 MEDIUM No (dev-dep only)
RUSTSEC-2026-0232 HIGH No (dev-dep only)
RUSTSEC-2026-0243 INFO Suppressed No (dev-dep only)
RUSTSEC-2026-0257 MEDIUM Limited (buzz-agent OAuth flow only)

Net Impact: 7 production-impacting advisories resolved (3 HIGH, 4 MEDIUM). No new vulnerabilities introduced.


Recommendation

SHIP — This PR:

  1. Correctly addresses all documented RustSec advisories
  2. Uses minimal version bumps (patch-level only)
  3. Includes valid dev-dep suppression with clear reasoning
  4. Passes all security and functional validation gates
  5. Introduces no unintended scope changes

Merge Conditions: All CI gates green.


Reviewer: Open 4.8 (Claude 3.7 Sonnet adversarial security agent)
Review Date: 2026-08-12 07:40 ET
Commit Reviewed: 1a036e715e91d5543219a5f54878f22f4df9d057

BradGroux pushed a commit to BradGroux/buzz that referenced this pull request Aug 23, 2026
TheSentinel454 added a commit that referenced this pull request Aug 28, 2026
## Current reconstructed head

Exact base: `codex/issue-7-roster-test-fixtures` at
`5df440c411be9705eb29a57f0c41f7239767e007`
Exact head: `codex/issue-7-channel-membership-store` at
`8ad0782ee311f5f51b714494ce750c5937f127cc`

This current head removes `crates/buzz-db/tests/store_ownership.rs`; no
replacement path-sensitive ownership test is introduced. Apart from
removing that complete test-file diff, the production patch is
byte-for-byte identical to the previously reviewed slice. This remains
part of tracker #2 and the #17/#19 acceptance work.

Independent exact-head review from a separate clean Blox workstation
found no issues. Current-head evidence passed formatting, strict
`buzz-db` clippy, 111 non-PostgreSQL library tests with 200 PostgreSQL
tests ignored, the observability source test, relay consumer
compilation, exact ownership/unique-span review checks, and 3 channel
and 19 membership PostgreSQL tests on native PostgreSQL where
applicable.

## Why
Complete the channel ownership slice of [tracker
#2](TheSentinel454#2) and [domain issue
#7](TheSentinel454#7) while preserving
the runtime/store boundary established by #6660 and #6668. This child
stacks on the test-only fixture prerequisite #6819 above #6777 and
carries forward PR #6700's membership/replacement lock timing without
changing lock or transaction behavior.

## What
- Keep channel lifecycle, metadata, TTL advisory locking, and lifecycle
tests in `channel.rs`
- Move membership/roster records, SQL, advisory-lock helpers, `Db`
methods, focused tests, and datastore spans to a dedicated
`channel_members.rs`
- Preserve existing `buzz_db::channel::*` paths with compatibility
re-exports while exposing the dedicated module
- Move the four roster-fence PostgreSQL tests out of `lib.rs`

## Stack
- Exact base: codex/issue-7-roster-test-fixtures at
21d1b26
([#6819](#6819))
- Exact head: codex/issue-7-channel-membership-store at
25138bf
- Tracker: TheSentinel454#2
- Domain: TheSentinel454#7
- 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 change to channel TTL lifecycle ownership merely because lifecycle
bootstrap writes an owner membership row
- No store traits, domain-handle redesign, broad `PgExecutor` migration,
raw pool accessor, new crate, or directory-wide reorganization
- No changes to, retargeting of, or merge action on PR #6700 or #6777

## Risk Assessment
Moderate review surface, low semantic risk. The file split is large, but
method signatures, SQL, bind order, membership and replacement lock
namespaces, transaction boundaries, and span names remain unchanged.
Compatibility re-exports preserve existing `buzz_db::channel::*`
consumers.

## Blox Verification
Author workstation: `buzz-tornquist-issue-2-store-stack` (`2046520`),
exact head `8376e19d0da3ec77550590cd91cc3dfe284d95d6`.

- `cargo fmt --all --check` — passed
- `cargo clippy -p buzz-db -p buzz-relay --all-targets -- -D warnings` —
passed
- Native PostgreSQL channel lifecycle suite — 3 passed
- Native PostgreSQL membership/roster suite — 17 passed; two
pre-existing ignored-test fixture failures reproduced identically on the
untouched parent `2de5444`:
`large_roster_reconciliation_candidates_respect_snapshot_count_and_signer`
and `locked_member_snapshot_blocks_post_capture_membership_mutation`
both receive the migration-0032 `23514` invalid-`p`-tag rejection. This
extraction intentionally does not fold a test-behavior fix into the
move.
- `cargo test -p buzz-relay --lib -- --test-threads=1` — 908 passed, 48
ignored; the existing load-sensitive mesh demo test returned 504,
matching the #6700/parent baseline
- `cargo test -p buzz-relay --lib
api::mesh_demo::tests::demo_join_forwarded_arm_round_trips_echo --
--exact --test-threads=1` — passed

Independent exact-head review: `buzz-tornquist-pr-6782-review`
(`2048397`) found no remaining critical, important, or minor issues. The
full implementation review also independently reproduced both stated
PostgreSQL fixture failures on the exact parent and passed the relay
library suite (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-6782-final-review` (`2057620`), fresh
shallow checkout
- Base: `c60e793eadde79d9eab9f48bbb2ede0ad4831f9b`
- Head: `fa09b6c81c4db3b3e1940a2117a97ab2186e49f7`
- Findings: none

Reviewed both commits in `base..head`. Channel lifecycle/metadata, TTL
transitions, and their lock rationale remain in `channel.rs`; membership
authorization, roster fencing/snapshots, membership advisory locking,
membership records, and focused tests move together to
`channel_members.rs`. SQL, transaction, and lock sequences are
preserved.

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 (2/2) and observability (1/1)
guards passed; native PostgreSQL 17 passed 3 channel lifecycle tests
plus 19 membership/roster tests 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:
`81ae374095f649ca7a25d8b9a4fc864257b7925d1b44657a69ca111523adf36e`.

## Comment-addressed restack

Review follow-up on #6777 removed only the low-value replaceable
ownership source test. This PR was restacked onto its rewritten parent;
its production patch is unchanged.

- Exact base: `21d1b265c133292e6707e766cd4204e6a43f08af`
- Exact head: `25138bfd6588e046170dbdbc4ed953bdc3cf7ed1`
- Final cumulative tip: `6fa2f104d42c6ba85bdf62e7ccb74ceaf4a84f67`
- Per-layer patch-ID and tree audits confirm this PR’s production diff
is unchanged from its pre-comment head.
- 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 Clippy, DB lib 111
passed / 200 ignored, current ownership/observability guards, 3 channel
plus 19 membership PostgreSQL tests, and relay compilation.

Signed-off-by: OpenAI Codex <codex@openai.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
TheSentinel454 added a commit that referenced this pull request Aug 28, 2026
## Summary

Finish the remaining database-store extraction tracked by
[TheSentinel454#2](TheSentinel454#2)
in one reviewable PR.

This consolidates the previously stacked domain slices after #6782
merged. It preserves the runtime/store boundary established by #6660,
#6668, #6700, and #6782 while separating database runtime infrastructure
from domain-owned persistence:

- `runtime/` owns pool construction and sizing, writer/reader routing,
read sessions and route proofs, transaction infrastructure,
observability primitives, replica fencing, health support, migrations,
and cross-cutting runtime tests.
- `store/` owns domain records, SQL, row parsing, locks and invariants,
`Db` domain methods, focused tests, and logical-operation datastore
spans.
- `lib.rs` remains a 57-line compatibility facade that preserves
existing crate-root paths and `Db` method signatures through re-exports.

Domain coverage includes API tokens, authentication allowlists,
reminders, event queries, threads, reactions, feeds, users and DMs,
push, workflows/runs/approvals, relay membership and invites, product
feedback, moderation/admin moderation, relay admin actions/operators,
git repositories, archived identities, usage, partition maintenance,
deletion, channel membership inherited from merged #6782, and the final
runtime/store layout.

The branch has been rebased onto current `main`. Database changes that
landed there were incorporated rather than overwritten:
`relay_admin_actions.rs` and `relay_operators.rs` now live under
`store/`, their 27 public `Db` wrappers and existing behavior remain
intact, and every wrapper has exactly one fixed-name datastore span.
Concurrent changes to migration, moderation, admin moderation, and error
handling are also retained.

### Exact base and head

- Base: `main` at `ed11c8d8bf0a17402be5cf243724f89471530d2f`
- Head: `codex/issue-2-store-extraction` at
`be24430472d1a87ac5c0d6026c620cd6caea3537`

### Related issue

- Structural tracker:
[TheSentinel454#2](TheSentinel454#2)
- Domain trackers:
[#6](TheSentinel454#6),
[#7](TheSentinel454#7),
[#12](TheSentinel454#12),
[#13](TheSentinel454#13)
- Acceptance trackers:
[#17](TheSentinel454#17),
[#19](TheSentinel454#19)

This supersedes #6783, #6784, #6787, #6788, #6789, #6792, #6820, #6794,
#6796, #6797, #6798, #6799, #6804, #6805, #6806, #6808, #6809, #6811,
#6812, #6813, #6814, #6815, and #6890. Their discussions remain
available for review history.

### #17 / #19 acceptance

- Preserves the metric names, fixed labels, transaction/lock timing
boundaries, and privacy/cardinality constraints introduced by #6700.
- Keeps exactly one datastore span per public logical operation,
including the 27 relay-admin wrappers added on `main`.
- Removes `store_ownership.rs`; physical ownership and focused source
guards now enforce the boundary directly.
- Leaves no `impl Db`, domain SQL, focused domain test group, or
datastore span in `lib.rs`.
- Preserves existing public paths such as `buzz_db::channel`,
`buzz_db::event`, and `buzz_db::workflow` through crate-root re-exports
while keeping internal `runtime` and `store` namespaces private.

### Non-goals

- No SQL, schema, locking, transaction, retry, timeout, or
client-visible behavior changes.
- No generic store traits, domain handles, broad `PgExecutor` migration,
new store crate, raw pool accessor, or broader directory reorganization.
- No tracker issues are closed by this PR.

### Risk

The cumulative diff is large but structural. Risk is primarily
module-path, ownership, or conflict-resolution drift. It is mitigated by
preserving public re-exports, comparing the newly moved `main`
implementations to their upstream source, source guards, touched-crate
compilation, PostgreSQL-backed test coverage, and an independent
exact-head review on a separate clean Blox workstation.

### Testing

Author workstation `buzz-tornquist-pr-6987-rebase`, rebased branch
ending at exact head `be24430472d1a87ac5c0d6026c620cd6caea3537`:

- `cargo fmt --all --check`
- `cargo clippy -p buzz-db -p buzz-relay --all-targets -- -D warnings`
- `cargo test -p buzz-db --lib` — 113 passed, 240 PostgreSQL tests
intentionally ignored
- `cargo test -p buzz-db --test observability_source` — 2 passed
- PostgreSQL-backed `buzz-db` coverage under native PostgreSQL — 235
passed in the shared serial run; the five shared-state/config-sensitive
cases passed as isolated reruns against fresh schemas, including the two
owner-limit tests with their fixture's
`BUZZ_MAX_COMMUNITIES_PER_OWNER=3`
- `cargo test -p buzz-relay --lib -- --test-threads=1` under native
PostgreSQL/Redis — 991 passed; the three current-month
partition-sensitive identity-archive cases passed after provisioning the
August 2026 test partition; 87 infrastructure-marked tests remained
ignored
- Source/diff guards — relay-admin implementation bodies match current
`main`; all 27 public wrapper signatures are retained; exactly one
datastore span wraps each wrapper; `lib.rs` has zero `impl Db` blocks
and zero datastore spans; no duplicate top-level relay-admin modules or
`store_ownership.rs`; `error.rs` matches current `main`

Independent clean review workstation `buzz-tornquist-pr-6987-review`,
detached at exact head `be24430472d1a87ac5c0d6026c620cd6caea3537`:

- `cargo fmt --all --check`
- `cargo clippy -p buzz-db -p buzz-relay --all-targets -- -D warnings`
- `cargo test -p buzz-db --lib` — 113 passed, 240 ignored
- `cargo test -p buzz-db --test observability_source` — 2 passed
- Exact-head ownership/re-export/instrumentation audit — no remaining
actionable findings

---------

Signed-off-by: OpenAI Codex <codex@openai.com>
Signed-off-by: tornquist <tornquist@squareup.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
wpfleger96 pushed a commit that referenced this pull request Aug 29, 2026
## Summary

Finish the remaining database-store extraction tracked by
[TheSentinel454#2](TheSentinel454#2)
in one reviewable PR.

This consolidates the previously stacked domain slices after #6782
merged. It preserves the runtime/store boundary established by #6660,
#6668, #6700, and #6782 while separating database runtime infrastructure
from domain-owned persistence:

- `runtime/` owns pool construction and sizing, writer/reader routing,
read sessions and route proofs, transaction infrastructure,
observability primitives, replica fencing, health support, migrations,
and cross-cutting runtime tests.
- `store/` owns domain records, SQL, row parsing, locks and invariants,
`Db` domain methods, focused tests, and logical-operation datastore
spans.
- `lib.rs` remains a 57-line compatibility facade that preserves
existing crate-root paths and `Db` method signatures through re-exports.

Domain coverage includes API tokens, authentication allowlists,
reminders, event queries, threads, reactions, feeds, users and DMs,
push, workflows/runs/approvals, relay membership and invites, product
feedback, moderation/admin moderation, relay admin actions/operators,
git repositories, archived identities, usage, partition maintenance,
deletion, channel membership inherited from merged #6782, and the final
runtime/store layout.

The branch has been rebased onto current `main`. Database changes that
landed there were incorporated rather than overwritten:
`relay_admin_actions.rs` and `relay_operators.rs` now live under
`store/`, their 27 public `Db` wrappers and existing behavior remain
intact, and every wrapper has exactly one fixed-name datastore span.
Concurrent changes to migration, moderation, admin moderation, and error
handling are also retained.

### Exact base and head

- Base: `main` at `ed11c8d8bf0a17402be5cf243724f89471530d2f`
- Head: `codex/issue-2-store-extraction` at
`be24430472d1a87ac5c0d6026c620cd6caea3537`

### Related issue

- Structural tracker:
[TheSentinel454#2](TheSentinel454#2)
- Domain trackers:
[#6](TheSentinel454#6),
[#7](TheSentinel454#7),
[#12](TheSentinel454#12),
[#13](TheSentinel454#13)
- Acceptance trackers:
[#17](TheSentinel454#17),
[#19](TheSentinel454#19)

This supersedes #6783, #6784, #6787, #6788, #6789, #6792, #6820, #6794,
#6796, #6797, #6798, #6799, #6804, #6805, #6806, #6808, #6809, #6811,
#6812, #6813, #6814, #6815, and #6890. Their discussions remain
available for review history.

### #17 / #19 acceptance

- Preserves the metric names, fixed labels, transaction/lock timing
boundaries, and privacy/cardinality constraints introduced by #6700.
- Keeps exactly one datastore span per public logical operation,
including the 27 relay-admin wrappers added on `main`.
- Removes `store_ownership.rs`; physical ownership and focused source
guards now enforce the boundary directly.
- Leaves no `impl Db`, domain SQL, focused domain test group, or
datastore span in `lib.rs`.
- Preserves existing public paths such as `buzz_db::channel`,
`buzz_db::event`, and `buzz_db::workflow` through crate-root re-exports
while keeping internal `runtime` and `store` namespaces private.

### Non-goals

- No SQL, schema, locking, transaction, retry, timeout, or
client-visible behavior changes.
- No generic store traits, domain handles, broad `PgExecutor` migration,
new store crate, raw pool accessor, or broader directory reorganization.
- No tracker issues are closed by this PR.

### Risk

The cumulative diff is large but structural. Risk is primarily
module-path, ownership, or conflict-resolution drift. It is mitigated by
preserving public re-exports, comparing the newly moved `main`
implementations to their upstream source, source guards, touched-crate
compilation, PostgreSQL-backed test coverage, and an independent
exact-head review on a separate clean Blox workstation.

### Testing

Author workstation `buzz-tornquist-pr-6987-rebase`, rebased branch
ending at exact head `be24430472d1a87ac5c0d6026c620cd6caea3537`:

- `cargo fmt --all --check`
- `cargo clippy -p buzz-db -p buzz-relay --all-targets -- -D warnings`
- `cargo test -p buzz-db --lib` — 113 passed, 240 PostgreSQL tests
intentionally ignored
- `cargo test -p buzz-db --test observability_source` — 2 passed
- PostgreSQL-backed `buzz-db` coverage under native PostgreSQL — 235
passed in the shared serial run; the five shared-state/config-sensitive
cases passed as isolated reruns against fresh schemas, including the two
owner-limit tests with their fixture's
`BUZZ_MAX_COMMUNITIES_PER_OWNER=3`
- `cargo test -p buzz-relay --lib -- --test-threads=1` under native
PostgreSQL/Redis — 991 passed; the three current-month
partition-sensitive identity-archive cases passed after provisioning the
August 2026 test partition; 87 infrastructure-marked tests remained
ignored
- Source/diff guards — relay-admin implementation bodies match current
`main`; all 27 public wrapper signatures are retained; exactly one
datastore span wraps each wrapper; `lib.rs` has zero `impl Db` blocks
and zero datastore spans; no duplicate top-level relay-admin modules or
`store_ownership.rs`; `error.rs` matches current `main`

Independent clean review workstation `buzz-tornquist-pr-6987-review`,
detached at exact head `be24430472d1a87ac5c0d6026c620cd6caea3537`:

- `cargo fmt --all --check`
- `cargo clippy -p buzz-db -p buzz-relay --all-targets -- -D warnings`
- `cargo test -p buzz-db --lib` — 113 passed, 240 ignored
- `cargo test -p buzz-db --test observability_source` — 2 passed
- Exact-head ownership/re-export/instrumentation audit — no remaining
actionable findings

---------

Signed-off-by: OpenAI Codex <codex@openai.com>
Signed-off-by: tornquist <tornquist@squareup.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants