mirror of
https://github.com/block/buzz.git
synced 2026-10-27 20:41:57 +00:00
codex/buzz-268-draft-prep
203 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Luke Tornquist
|
1163e202ee |
Merge main into invitation revocation candidate
Main's #8605 took migrations 0072 and 0073 for the user deletion target, so the invitation migration moves to 0074_invite_revocation.sql. Upgrade fixtures now start from main's schema through 0073, the migrator test expects 74 contiguous migrations with the invitation assertions at index 73, and both TESTING.md files name 0074. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Luke Tornquist <tornquist@squareup.com> |
||
|
ravarora2
|
834dca26bf |
Generate user community inventory for the dummy deletion workflow (#8605)
## Summary User deletion can now discover current communities from a hex public key or npub, save that inventory, and run it through the existing approval and dummy workflow. `--pubkey demo` remains the default. Execution only prints messages; it does not fence users or delete data. Discovery combines explicit relay memberships with active channel memberships, including users who have no relay membership row. It deduplicates communities and preserves owner relationships. The saved manifest contains only community IDs, labels and relationships. Historical discovery and owned-community execution remain later work. Submission rejects owner inventories atomically with a specific unsupported-execution error; preview still includes owners. This is an expand-schema change after #8520. Migrations 0072–0073 replace the demo-only target check with a bounded identifier check, with 5-second lock and 30-second statement timeouts. The database keeps the user-only entity-type check while allowing different identity formats in the target field. Workflow-specific commands reject entity types they cannot execute. The application currently resolves demo and hex/npub inputs; account-ID lookup is future work. The desired-state constraint matches. No backfill, trigger, function, or foreign key is added. Existing demo rows remain valid. Old binaries fail closed on new manifests; use the new CLI for their approval and execution. Older auto-migrating binaries need the documented rollback procedure. Identity parsing and demo fixture selection stay in intake. Approval and resume use the saved target without reparsing its format. The user workflow defines its stage names and handlers; CLI stage validation reads those rules instead of a separate three-name list. Shared persistence accepts caller-defined stages. Permanent target uniqueness is unchanged. Dummy submissions and execution belong in isolated, disposable development databases; production preview uses read-only access. CLI operator names remain self-asserted. ### Related issue Follows #8520. Searched open PRs; no duplicate inventory change found. #8489 is earlier workflow scaffolding. ### Testing Built `buzz-admin` locally and applied all migrations to a fresh disposable PostgreSQL database. Ran the actual CLI through preview → submit → inspect → approve → `run --next-step` → resume → completed for both demo and a public key with a channel-only membership fixture. Demo completed five stages; the public-key run completed three. Injected `--fail-stage purge_db`, waited for the retry deadline, and resumed the same failed stage successfully. Membership data remained intact. Repeated the CLI workflow on October 26 for demo and an npub backed by a fresh production inventory. Production access used read-only credentials and the real `inventory --pubkey "$SUBJECT"` handler. Copied only the relevant discovery fields into an isolated local PostgreSQL database; local channel parent rows supplied placeholder fields for foreign keys. The inventory saved by local submission exactly matched the production result. All workflow writes stayed local. For each input, ran `submit --pubkey "$SUBJECT" --requested-by local-e2e-submitter`, inspected the saved inventory, and confirmed execution before approval was rejected. Approved its digest, then ran each handler with `run "$REQUEST_ID" --next-step --stage-delay-ms 2500` in a new process. Captured all three workflow tables during and after every stage. Demo completed five stages across two communities; the npub completed three stages for its discovered community. Each handler saved `dummy: true`. Both requests finished with no current stage, no worker lease, and no retries. Copied memberships remained intact. This additional run exercised orderly resume; it did not inject a failure. The reproducible command sequence and fixture setup are in [the inventory walkthrough](docs/operator-user-deletion-inventory.md). Production was not changed. The earlier full-workflow runs used synthetic local fixtures; the additional npub run used captured production discovery fields. Repository-wide `just ci` was not run; validation focused on the affected packages and PostgreSQL workflow. The pre-push desktop build failed with “Too many open files in system.” Stopped the broader hook run and bypassed it to publish the earlier commits; the affected-package Clippy and formatting checks had passed. Generated with Codex --------- Signed-off-by: Ravneet Arora <rarora@squareup.com> |
||
|
Luke Tornquist
|
6916ef18a5 |
Merge main into invitation revocation candidate
Main's #8579 took migration 0071 for the unlisted channel setting, so the invitation migration moves to 0072_invite_revocation.sql. Upgrade fixtures now start from main's schema through 0071, the migrator test expects 72 contiguous migrations with the unlisted assertion at index 70 and the invitation assertions at index 71, and both TESTING.md files name 0072. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Luke Tornquist <tornquist@squareup.com> |
||
|
Luke Tornquist
|
0e36dbc823 |
fix(db): keep the reader pool warm and drop its pre-acquire ping (#8495)
## Summary
The reader pool's 150ms `acquire_timeout` covers the whole acquire: the
pre-acquire idle-connection ping **and** any new-connection dial (TCP +
TLS + SCRAM). When readers slow down for a moment, the timeout:
- abandons in-flight dials mid-handshake (Postgres logs `could not
accept SSL connection: EOF detected`),
- closes healthy idle connections whose ping overran instead of
returning them to the pool, and
- falls every miss back to the writer, which then exhausts its own pool.
Across a fleet of relay pods this shows up as a burst of abandoned
handshakes on the readers that starts on nearly every pod at once,
followed seconds later by writer pool-acquire timeouts. The database
itself reports no statement, lock, or connection-limit errors before the
burst: the client is tearing down its own reader connections.
This PR removes that amplifier. It does not change the request budget,
writer fallback, or the writer pool.
### Change (`crates/buzz-db/src/runtime/mod.rs`)
- **Warm floor:** reader `min_connections` is now `max / 5`, at least 1,
derived from the existing reader pool size (no new config knob). sqlx
re-dials the floor in background maintenance, outside the request
budget, from its reaper and whenever a connection is closed instead of
released. The floor is per relay process, so standing reader connections
are the floor times the number of relay processes; size the reader's
`max_connections` for it.
- **Floor fills at boot:** with `max_lifetime`/`idle_timeout` set, a
lazy sqlx 0.9 pool does not dial `min_connections` at construction. The
existing boot ping now closes its probe connection instead of returning
it, which triggers sqlx's floor maintenance. Probe acquires that overrun
150ms retry with bounded backoff (250ms, 500ms, 1s, 2s). The doc comment
that said sqlx dials the minimum eagerly was wrong for this config and
is corrected.
- **`test_before_acquire(false)` on the reader:** the routed read's
`BEGIN … READ ONLY` already proves the connection and fails closed to
the writer (`reader_validation_error`). This removes one round trip per
checkout and the warm-connection churn.
- **Bounded fence proof:** with no pre-acquire ping, the proof (Aurora
capability probe, `BEGIN`, heartbeat observation) is the first round
trip on a checkout. It now runs under `READER_PROOF_TIMEOUT` (1s), so a
reader that stops answering without closing its sockets (partition, host
loss, failover) fails closed to the writer as the new route reason
`reader_proof_timeout` instead of hanging the request until TCP gives
up. 1s is well above a normal round trip, so a brief reader stall waits
on the reader rather than flooding the writer.
- **Bounded release:** every connection returned to the reader pool
first passes a release check bounded at 5s (`Db::read_pool_options`).
sqlx's own release ping has no timeout, so without this a socket whose
reader went silent would hold its slot until the kernel gives up; with
every slot held that way, the pool cannot dial the reader that replaced
it, and all routed reads fall back to the writer. A check that times out
or errors closes the socket and frees the slot, and floor maintenance
dials a replacement. A connection still inside a transaction is closed
instead of returned: a checkout dropped while its `BEGIN` was in flight
(a timed-out proof, a cancelled caller) has no `ROLLBACK` queued,
because sqlx counts the transaction only once `BEGIN` is acknowledged.
Closing it ends the session, and the server rolls it back. An expired
proof is therefore simply dropped. Maintenance checkouts (fleet usage
collection) also keep their close-on-drop: when one is abandoned at its
deadline, its socket is closed under sqlx's 5s close bound rather than
going through the release path.
- **Pinned isolation default:** reader sessions set
`default_transaction_isolation=read committed` in the startup packet,
which takes precedence over database and role defaults. The release
check depends on this, because it detects an open routed-read
transaction by its `REPEATABLE READ` isolation.
- **Metric:** each close is counted in
`buzz_db_reader_release_closed_total{reason}` (`timeout`, `error`,
`open_transaction`). Operator notes for the floor,
`reader_proof_timeout`, and this counter are in
`deploy/charts/buzz/README.md`.
### What got simpler
- The reader checkout path does one less round trip and no longer closes
healthy connections on a slow ping.
- Boot warm-up reuses sqlx's own `min_connections` maintenance rather
than re-implementing it.
- One release rule now covers every reader checkout, so the
open-transaction leak on cancelled reads no longer needs special
handling per call site.
- No new config knob.
- The reader test proxy now lives once in `test_support` instead of
inside `store/usage.rs`.
- Tests build their reader pools from the production options rather than
hand-copied ones.
### Tradeoffs
- The reader is now dialed at boot (previously "no reader dial at
boot"). If the reader is down at boot, this only produces log noise, and
reads still fail closed to the writer.
- The release check is one trivial statement (`SELECT
current_setting(...)`) per released reader connection. It runs in sqlx's
release task, after the response, so request latency is unchanged.
- The check detects an open transaction by comparing the current
isolation level with the session default. That is only sound because the
reader session default is pinned to `READ COMMITTED` and routed reads
begin `REPEATABLE READ`.
- **Residual window:** after the release check passes, sqlx sends its
own release ping (one more round trip), and that ping has no timeout. A
serving-read connection whose endpoint goes silent between those two
round trips still holds its slot until TCP gives up. Only releases
already mid-ping at that instant are exposed: later releases fail the
bounded check first and are closed. sqlx behaves the same way for every
pool on `main`. Maintenance checkouts avoid it through close-on-drop.
Bounding it for serving reads would mean replacing sqlx's release path
or putting a pooler in front of the reader, so it is left as a
follow-up.
- Boot fill is best effort. Each boot-probe retry still uses the 150ms
serving acquire budget, so if cold handshakes consistently take longer
than that, all five attempts fail and the floor is not filled at boot.
It fills the first time any reader connection is closed, which the
reaper and the release check both do, and until then reads dial as
before.
- This also closes pre-existing debt: a caller that dropped a routed
read mid-`BEGIN` (client disconnect) could leave a connection `idle in
transaction`. The release check covers that path too.
### Related issue
N/A (production incident follow-up; none found).
### Testing
- Postgres tests (`runtime/tests/reader_pool_postgres_tests.rs`):
- the boot ping leaves the reader pool at exactly its floor, all idle
(base leaves one connection);
- a warm reader whose heartbeat read blocks (exclusive lock) falls back
to the writer as `reader_proof_timeout` within one proof budget;
- a warm reader connection terminated server-side falls back as
`reader_validation_error` with no client error;
- a proof that times out mid-`BEGIN` on a paused reader (a test proxy
that holds bytes, like `docker pause`) falls back as
`reader_proof_timeout`. The reader database default is set to
`REPEATABLE READ` for this test. Once the reader resumes, it leaves no
`idle in transaction` reader backend, and the close is counted as
`open_transaction`. The test fails if the release check is made blind to
open transactions, or if the isolation pin is removed.
- Failover: every established reader session is stalled while new
sessions get through. Each stalled warm connection costs one
`reader_proof_timeout`, then routed reads reach the reader again through
new connections within 8s, and the closes are counted as `timeout`. The
test fails with the release bound removed.
- `store/usage.rs`: a fleet collection abandoned at its deadline on a
paused reader frees its slot within 3s while the reader is still paused,
which is faster than the 5s release check. The test fails with
maintenance close-on-drop removed.
- Unit tests: the min-connections mapping, the reader pool options, and
that construction still dials nothing.
- Blox, at `f3e3bf158`: `just fmt-check` and `just clippy` are clean,
and `buzz-db` unit tests are 191/191.
- Blox, at `01745b21e`: fmt is clean, the targeted reader tests passed 3
of 3 runs, and the release-bound and transaction-blind mutations both
turn the targeted tests red.
- Blox, at `2f8e482fb` (before the rebase onto `main`): fmt and clippy
are clean, the targeted reader tests passed 3 of 3 runs, `buzz-db` unit
tests pass, and each of four mutations turns its targeted test red:
release bound, transaction-blind check, isolation pin, and maintenance
close-on-drop.
- Blox, at the rebased head `e80bc2bed`: the same four mutations each
turn their targeted test red, and fmt, clippy, the targeted reader tests
(3 of 3 runs) and `buzz-db` unit tests are clean.
- CI Postgres lane at `e80bc2bed`: 1130/1130 passed, including every new
reader-pool test.
- Live check at `e80bc2bed`: a relay with a two-slot reader pool behind
a proxy, which was then frozen for 18s. All 246 history reads succeeded,
and the slowest took 1.0s. During the freeze, 2 proof timeouts and 2
release-timeout closes were counted. Reads returned to the reader within
36ms of the resume, and no reader backend was left `idle in
transaction`.
- Rebase note: `main` added a reader `after_connect` hook that tightens
`statement_timeout`. It now lives in `Db::read_pool_options`, so the
tests' reader pools get it too.
---------
Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
Bradley Axen
|
a3a47e6294 |
feat(relay): add an unlisted channel setting (#8579)
🤖 ## Summary Sharing a Solo chat with a large channel today means adding every person as a member, one at a time, and the app blocks the share past 100 people. The plan is to share it the way Google Docs does with "anyone at the company with the link". Buzz's `open` visibility already gives that kind of access: anyone in the community can read and join an open channel. The problem is that an open channel also shows up in everyone's channel list and search, and a private chat shouldn't appear there just because its link was posted somewhere. This PR adds a channel setting, `unlisted`. An **unlisted open channel** can be read and joined by anyone who names it by id (from a shared link). For non-members, it is left out of every read that does not name it. Members see it as before. This is the relay side only; the buzz-app change comes later. #8575 (batch adds) remains the path for small audiences. Unlisted is not an access control. Anyone who has the id can read the channel, and that is intended. ## Solo flows this supports Each user action is one event. The buzz-app side comes later; the relay supports all of these now. | Flow | Event | Result | | --- | --- | --- | | New Solo chat | kind:9007 `visibility private` + `["unlisted", "true"]` | The chat starts unlisted, so it stays unlisted when any client later opens it. | | Share | One kind:9002: `visibility open` + `unlisted true` + `join_role guest\|member`. Then post the link. | Anyone with the link can read the chat by id, both history and live. Nobody is added as a member, so the chat isn't in anyone's channel list. A non-member can't post; `join_role` decides whether someone who joins can. | | View | A REQ that names the chat by `#h`, or by `#d` for its metadata | A non-member can read it. It is never listed for them. | | Reply | kind:9021 join (the app sends it before a viewer's first reply) | Joining makes them a member with the role from `join_role`, so with `member` they can post. A kind:9022 leave makes them a viewer again; the chat stays readable by id. | | Unshare | kind:9002 `visibility private` | The existing open→private eviction closes non-members' live subscriptions. New reads by non-members are refused. | | Convert to a channel | One kind:9002: `["t", ""]` + `unlisted false`, plus `visibility` | Clears the Solo label. Non-members can now find it in discovery. | **Sidebar pins are not relay state.** Pin and unpin live in the app's existing kind:30078 sidebar preferences, which sync across devices and don't touch membership. Don't map them to join/leave: someone a chat was shared with by member add is already a member, so a leave would remove their access. Your own Solo chats keep showing up, because you're their owner member, and members see unlisted channels everywhere. Existing Solo chats aren't backfilled. They get the flag when they're shared, because the share event sets it. **Open question, not built here:** someone who only views a shared chat isn't a member. A mention of them doesn't reach their inbox (`#p:[me]` without `#h`) and sends them no push. ## Behavior | Reader | Reads that don't name the channel (kind:39000 without `#d`, `#P` creator queries, messages, COUNT, NIP-50 search, artifact queries, channel list) | Reads that name it (`#h`, or `#d` on 39000–39003) | kind:9021 join | | --- | --- | --- | --- | | Member | Listed | Readable | n/a | | Non-member, unlisted open channel | **Not listed** | Readable | Works; `join_role` still decides the role | | Non-member, private channel (unlisted or not) | Not listed | Not readable | Refused | - **Setting it:** kind:9002 `["unlisted", "true"]` / `["unlisted", "false"]`. Owner or admin only, like `visibility`. A missing tag means no change. It can be sent in the same event as `["visibility", "open"]`, so a private chat becomes open and unlisted in one step. Any other value is refused before the event is stored. The relay posts an `unlisted_changed` system message and clears the accessible-channels cache across nodes. Nobody loses access when a channel is unlisted, so no subscriptions are evicted. - **At creation:** kind:9007 also takes `["unlisted", "true"|"false"]`. A missing tag means listed. Any other value is refused before the event is stored, as on kind:9002. It is stored with the channel row in the create's transaction, on both the client-UUID path and the relay-chosen-UUID path. - **kind:39000** carries a bare `["unlisted"]` flag while the setting is on. It does not add NIP-29 `hidden`, so the metadata stays readable by id. - **NIP-11** advertises `buzz-unlisted-channels` in `supported_extensions`. A relay without this PR would apply `["visibility", "open"]` and ignore `unlisted`, which would list the chat publicly. So buzz-app must check for the extension before it sends that pair. - **CLI:** `buzz channels update --channel <id> --unlisted true|false` and `buzz channels create ... --unlisted`. `buzz_sdk::build_update_channel` takes a new `unlisted: Option<bool>` argument, and `buzz_sdk::build_create_channel` takes `unlisted: bool`, which emits the tag only when true. ### Open question: does `unlisted` only mean something when the channel is open? **Decision: store it on its own; it only has an effect while the channel is open.** Private channels are already invisible to non-members, so on a private channel the flag does nothing. It is still stored and published, for three reasons: - a client can mark a Solo chat unlisted before opening it, or in the same event that opens it, so the chat is never listed even for a moment; - switching private → open → private → open keeps the setting; - there is no cross-field rule in kind:9002 to validate or keep in sync. kind:39000 publishes the flag as stored, whatever the visibility, so clients can show the setting. ## How The relay already has a "readable only when named" path for system channels (NIP-CL). Unlisted open channels reuse it: - **`get_accessible_channel_ids`**: the open half (the implicit scope of a REQ that doesn't name a channel) adds `AND NOT unlisted`. The member half is unchanged, so members still have the channel. - **`get_readable_named_only_channels`**: also returns unlisted open channels of the legacy types. Every reader of that set (REQ, COUNT, NIP-50 search, HTTP `/query` and `/count`, thread windows, workflow run reads) adds a named-only channel only to a filter that names it, through `filter_channel_scope`. An unlisted channel's type is `stream`/`forum`, so the `#t:["system"]`/`#P` system opt-in never adds it. - **`get_accessible_channels`** (the channel list): open channels are listed to non-members only when not unlisted. - **`query_artifacts`**: an artifact in an unlisted open channel reaches a non-member only when the query's `#h` names that channel. - **Live fan-out:** no change was needed. A global subscription never receives channel-scoped events, kind:39000 included (`fan_out_scoped`). A channel-scoped subscription has to name the channel, and that goes through the same named-only authorization. **Left alone on purpose:** access checks that read a named channel or event keep treating an unlisted channel as open. These are `handle_join_request` (kind:9021), the ingest write gate, operator-listener mention delivery (`load_deliverable_event`), personal read-state writes, and push matching. Push matching still requires membership, so non-members get no pushes from an unlisted channel. **Named reads checked:** these callers use the implicit channel set (`get_accessible_channel_ids`, its cache, `get_accessible_channels`). Every one that reads a channel the request names also adds the named-only set, so a non-member can still read an unlisted open chat by id. That covers WS REQ (history, live registration, NIP-50 search), WS COUNT, HTTP `/query` and `/count`, thread windows and workflow run reads. Other named reads don't use the implicit set: they check visibility or membership, so an unlisted open chat counts as open. Those are live fan-out, the ingest write gate, typing, kind:9021 join, huddle audio join, operator-listener delivery, the workflow sink and read-state writes. Feed queries (mentions, needs-action, activity) and the `/buzz/v1/me/sidebar` accessory API are lists, so leaving out chats you only view is intended. Media reads don't check channels. `buzz-auth`'s `ChannelAccessChecker` has no production implementation. ## Database (docs/database.md checklist) - **Phase:** expand only. The migration adds a column with a constant default, and the same release ships the code that reads and writes it. No backfill or contract step follows. - No trigger, function, procedure, rule or foreign key is added. - No migration changes existing rows. `ADD COLUMN ... BOOLEAN NOT NULL DEFAULT FALSE` stores the constant default in the catalog, so the table is not rewritten. - Every location in Rule 4 is updated: `migrations/0069_channel_unlisted.sql`, `schema/tables/public/channels.sql`, and the migration count. No seeds, types or reconcile changes are needed. `admin_schema_parity_between_desired_state_and_migrations` now compares `channels` columns between the pgschema desired state and the migrated database. I failed it once by removing the column from `channels.sql`. Only columns are compared for `channels`: on current `main`, pgschema already re-emits `idx_channels_id_live` without its `INCLUDE (community_id)`, an index drift that predates this PR and is noted in the test. - **Currently deployed relay against the new schema:** it never selects or writes `unlisted`, and every insert gets the default. All channel-record queries name their columns. Nobody can set the flag until this relay ships, because the old relay refuses a kind:9002 whose only tag is `unlisted`. - The migration starts with `SET LOCAL lock_timeout = '5s'`. No index is added: the new predicates ride the existing `(community_id, visibility)` index. ## Rollout 1. Deploy. Every channel stays listed, so nothing changes. 2. **Don't unlist any channel while an old relay is still running.** An old relay ignores the column and lists the channel to everyone. The NIP-11 extension lets clients gate on this. Unlisted isn't an access boundary, so a mixed fleet only exposes the channel in listings for a while. 3. **Rollback:** an older relay ignores the column, so unlisted channels show up as ordinary open channels again. Nothing breaks. ## Tests Postgres (`buzz-db`, discovered by the Postgres lane): - `unlisted_open_channel_is_named_only_for_non_members`: sets the flag through `update_channel`. Then checks that a non-member doesn't get it from `get_accessible_channel_ids` or `get_accessible_channels`, and does get it from `get_readable_named_only_channels`; that a member still lists it; that `put_member` (the kind:9021 path) lets the non-member join, after which they list it; and that listing it again shows it to a new reader. - `unlisted_private_channel_stays_private_and_opens_unlisted`: a private unlisted channel is not readable by name, and opening it keeps the flag. - `group_metadata_publishes_the_stored_unlisted_flag`, plus the unit test `publishes_the_unlisted_flag_as_stored`. - `unlisted_channel_artifacts_are_read_by_name_for_non_members`. - `channel_created_unlisted_is_never_listed`: creating through `create_channel_with_event` (the kind:9007 path) and `create_labeled_channel` stores the flag, and the channel is never listed to a non-member. - Mutation-checked: each of `NOT unlisted` in the implicit scope, `NOT c.unlisted` in the channel list, the unlisted branch of the named-only query, its `visibility = 'open'` guard, and the artifact condition was removed one at a time, and each removal failed at least one of these tests. Relay E2E (`test_nip29_unlisted_channel_wire`, added to the CI relay E2E step), against a local relay built from this branch: - A non-member or an unknown value is refused. - kind:39000 by `#d` carries `unlisted` and `public`, and not `hidden`. - kind:39000 discovery without `#d`: non-members don't see the channel, members do. - History and NIP-50 search: unnamed, a non-member doesn't see the channel's messages; with `#h`, they do; a member sees them unnamed. - A kind:9021 join works, and the joiner then lists the channel. - Listing it again shows it to everyone. - A reader whose channel set was cached before each change sees the change at once. I ran this against a relay with the cache invalidation removed, and it failed. Relay E2E (`test_nip29_solo_share_and_convert_wire`, added to the CI relay E2E step), each flow as a single event: - kind:9007 with an unknown `unlisted` value is refused. A chat created with `unlisted true` publishes the flag, and stays unlisted after a plain `visibility open`. - Share (open + unlisted + `join_role guest` in one kind:9002): a non-member doesn't list the chat. They read it by `#h`, both history and a live message, and can't post. A kind:9021 join lists it for them as a read-only guest. A kind:9022 leave unlists it again, and it stays readable by `#h`. - Unshare (`visibility private`): the viewer's live subscription gets CLOSED, and a new `#h` read is refused. - Convert (`["t", ""]` + `unlisted false`): the Solo label is cleared, and a non-member finds the chat in discovery. - I ran this against a relay that dropped the kind:9007 flag, and it failed. Also run: `cargo test` for buzz-db, buzz-sdk, buzz-cli, buzz-core, buzz-relay and buzz-test-client; the Postgres lane for buzz-db and buzz-relay (1045 tests); and the existing unlisted, announce, departure, guest, role-change and NIP-43 snapshot wire tests. In the lane, two tests failed on pool timeouts (`resend_validated_after_the_first_commit_is_a_duplicate`, `edit_replaces_and_clears_labels`) and passed when rerun alone. An earlier run had the same kind of flake: `routed_fallback_spends_one_acquire_budget_when_aurora_cache_is_cold` (a timing budget) and `resent_edit_is_a_duplicate` (a pool timeout). In a full local `e2e_relay` run, three tests outside CI's list also failed (`test_invite_*` twice, `test_unarchive_emits_member_added_notification`). They fail the same way on a relay built from `main`. ## Not in this PR - The buzz-app follow-up: support open Solo chats, the join-from-card view, the large-audience prompt, and gating on `buzz-unlisted-channels`. - A NIP-CL update. The behavior is documented in NOSTR.md ("Unlisted channels"). buzz-review-completed --------- Signed-off-by: Bradley Axen <baxen@squareup.com> Signed-off-by: Luke Tornquist <tornquist@squareup.com> Co-authored-by: ART <ea39c1c8acf57180161ef4ba4ed68abf8589f5291956b909a5aa991fc66664ca@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Co-authored-by: Luke Tornquist <tornquist@squareup.com> Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> |
||
|
Luke Tornquist
|
8dad74faef |
test(db): run invite upgrade pgschema from the repository root
The invitation upgrade test launched bin/pgschema from the package directory, so pgschema missed the root .pgschemaignore and exercised a different apply path than every other caller. Set the working directory to the repository root, matching the runtime/migration.rs helper. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Luke Tornquist <tornquist@squareup.com> |
||
|
Luke Tornquist
|
c4101383f2 |
Merge main into invitation revocation candidate
Main added migrations 0069 and 0070 (entity deletion approval), so the invitation schema moves to 0071 and its upgrade fixtures start from the complete main schema through 0070. Main's NIP-FI owner linking (#8401) replaced the boolean owner materialization with OwnerLink { Linked, Conflict, Unavailable }, which already separates a recorded-owner conflict from a storage failure. Invite minting now maps those variants to 403 invite_owner_mismatch and 503, and this branch's Result<bool, DbError> variant of materialize_nip_oa_owner is dropped, along with the call-site adaptations it required in auth, audio and bridge. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Luke Tornquist <tornquist@squareup.com> |
||
|
Tom Brow
|
b8b1ab5b3c |
perf(push): avoid empty push delivery claims (#8524)
Each enabled push delivery worker currently opens a claim transaction for every active community, even when its queue is empty. Discover eligible communities in bounded pages first, then use the existing claim and delivery path. No schema or index change is needed. Per-delivery optimizations (sharing serving-write leases across a batch and combining revalidation or membership checks), enqueue batching, and outbox pruning are out of scope and are being investigated separately, including their deletion, revocation, and stale-worker safety guarantees. ### Related issue No matching existing issue or PR found. ### Validation - 25 focused PostgreSQL tests passed, covering real-worker delivery, eligibility filtering, pagination fairness, concurrent claims, expired-claim recovery, and stale-worker fencing. The [full PostgreSQL CI suite](https://github.com/block/buzz/actions/runs/39675698081/job/119490943190) passed all 1,090 tests, including all seven new regressions. - Discovery was benchmarked with **43,000 communities**, existing indexes, and 30 warm executions per scenario. Empty queues required no community lookups; eight ready communities spread across the ID range required eight community index lookups. Results and the alternative-query comparison are below. - In the separate real-worker test with 1,000 empty communities, the original worker made 1,401 claim admissions in 1.8 seconds; the fixed worker made zero. One and four workers made 3 and 12 discovery reads respectively. - In the delivery test with 65 ready communities plus 1,000 empty communities, all 81 notifications reached an HTTP test gateway in 0.83 seconds versus 1.55 seconds with the original loop. - Independent standards and specification reviews, plus [Codex review](https://github.com/block/buzz/pull/8524#issuecomment-6326568721), found no remaining blockers. Final Rust linting, the full local unit/integration run, desktop JavaScript tests/build, web build, and mobile analysis/tests passed. The full local desktop test run had one unrelated timing failure in `reaps_descendant_on_timeout`; its isolated recheck passed. [CI](https://github.com/block/buzz/actions/runs/39675698081) passed, including the Windows retry after an unrelated subprocess timeout. These are isolated local measurements, not production latency guarantees or a simulation of full production concurrency. Production delivery validation remains part of the release process; this PR does not re-enable push or deploy the change. ### Alternative considered An outbox-first query selects due pending jobs and expired sending jobs with `UNION ALL`, applies the same eligibility checks, then returns distinct community IDs. This could avoid scanning communities with no ready work. However, the current query's `EXISTS` clause does not force PostgreSQL to probe the outbox once per community: the measured plans already avoid that work when queues are empty. Both query shapes were tested against the same **43,000-community** dataset using existing indexes. Median database execution times over 30 warm runs: | Scenario | Current query | Outbox-first query | | --- | ---: | ---: | | Empty outbox | 0.038 ms | 0.041 ms | | No ready work, 27,336 retained jobs | 0.060 ms | 0.066 ms | | Eight ready communities spread across the ID range, including expired sending jobs, plus retained history | 0.109 ms | 0.126 ms | | Eight ready communities, 128 invalid jobs, and 27,200 terminal jobs | 0.225 ms | 0.335 ms | Both queries returned identical community IDs in every scenario. Forced generic prepared plans produced similar results. The current query was retained because the alternative did not reduce measured execution time or eliminate a community scan present in the empty-queue plan. No query rewrite or additional index was justified by these measurements. --------- Signed-off-by: Tom Brow <tomb@block.xyz> |
||
|
Bradley Axen
|
d0dee1cb50 |
feat(relay): add several people with one kind:9000 (#8575)
## What
A NIP-29 kind:9000 (put-user) can now name up to 100 people, one `p` tag
each. A client sharing a channel with many people can do it in one
signed write instead of one event per person.
- **Role per person:** NIP-29 style `["p", pubkey, role]`. When the
third element is absent or empty, the event's `["role", r]` tag applies.
- **Same checks, per person:** each person gets the checks a single add
does, in the same order: `decide_put_user`, then the agent's
`channel_add_policy`, then the admission rules again under the channel's
membership lock. One refused person no longer refuses the others.
- **Per-person results:** when the event names two or more people, the
OK message reports each result. Only non-empty lists appear, and
refusals are grouped by reason so 100 people stay well under a client's
16 KB receipt limit:
```
members:
{"added":[hex…],"updated":[hex…],"already_member":[hex…],"rejected":[{"reason":"…","pubkeys":[hex…]}]}
```
- **When the event is refused:** if it is malformed (bad `p` hex,
unknown role, more than 100 people, missing `h`, archived channel), or
if nobody it names may be added. With one person the message keeps
today's `invalid: <reason>`. With several it is `invalid: members: {…}`.
- **One-person events don't change:** a successful single add still
returns an empty OK message, which an existing e2e test pins.
- **Re-adding someone at the role they already have** no longer posts a
second `member_joined` timeline notice. The member-added notification
(kind:44100) still goes out, so an agent that missed its first one is
still re-signaled.
- **Group-state events (39000–39002)** are now published once per event
instead of once per person.
## Cost of a batch
The database work for one event does not grow with the number of people:
- **One membership transaction.** `buzz-db` gains `put_members`: one
transaction, one membership lock, one channel read, one read of every
involved role, the owner count only if a demotion needs it, and one
`UNNEST` upsert with `RETURNING`. `put_member` is now `put_members` with
one person, so there is one implementation of the admission and
last-owner rules, and a single add makes two fewer round trips than
before.
- **Same answers as one write per person.** People are decided in tag
order, and each decision sees the roles earlier people were given,
including the actor's own (an owner who steps down mid-batch can't then
grant admin), and the owners they add or remove. A new buzz-db test pins
this.
- **One policy query.** Validation reads every third-party target's
`channel_add_policy` with one `= ANY($2)` query
(`get_agent_channel_policies`) instead of one per person.
- **One channel-type read** for all the member-added notifications,
instead of one per notification.
What still scales with people is what each person must receive: their
`member_joined` timeline notice and their kind:44100 notification, one
stored event each.
## Why
This is for Solo chat sharing in buzz-app.
[block/buzz-app#1605](https://github.com/block/buzz-app/pull/1605)
routes every share through one helper, `shareChatsWithChannel`, which
today sends one kind:9000 per missing member. A small follow-up there
switches it to one batch, then re-reads the roster and adds anyone still
missing one at a time. That keeps it correct on relays without this
change, which accept the event but ignore every `p` after the first.
The batch stays a signed NIP-29 event rather than a new HTTP endpoint,
per VISION.md and AGENTS.md ("Prefer Nostr events over new HTTP
endpoints").
## Wire changes for existing senders
- Any malformed `p` tag now refuses the event. Before, malformed `p`
tags were skipped and the first valid one was used.
- Extra `p` tags are now targets. Before, they were ignored.
- A third `p` element that isn't a role, such as a NIP-01 relay hint,
refuses the event with `invalid role: …`. No sender in this repo does
that: `build_add_member` sends one bare `p`.
## Tests
**buzz-db (`channel_members` Postgres tests):** new
`put_members_decides_each_person_in_order`: the sole owner can't step
down before an heir is named in the same call, but the heir is still
added; with the heir counted the owner can step down, and the admin
grant after it is then refused; a no-op re-add reports its prior role; a
repeated pubkey refuses the call. The other `channel_members` Postgres
tests pass, including the announce-admission and concurrent `join_role`
tests that exercise `put_member`.
**buzz-relay (`put_user_postgres_tests.rs`), 5 tests:**
- mixed results in one event: added, already a member, an agent with
`nobody` policy rejected, a per-`p` `bot` role, a duplicate `p`, a
single roster publish
- `updated` for a role change, with the event `role` tag as fallback
- a plain member may add someone, but may not grant an elevated role on
a `p`
- a single add keeps its empty OK message
- an all-refused event: single text and grouped JSON
- a malformed `p` tag
- the 100-person cap: 101 refused, 100 accepted
Related relay Postgres groups (`group_state`, `create_with_members`,
`edit_metadata`, `membership`) pass.
**Live relay:** `e2e_relay` against a local relay at 9c27a0f15: 58 of 61
passed, including `test_nip29_batch_put_user_wire` (added to the CI
Relay E2E list) and every NIP-29 put-user, announce, guest and departure
test in that list. The 3 failures don't touch this change: the two
invite tests assume the relay's seeded host, and this relay ran on
`:3030`. `test_unarchive_emits_member_added_notification` timed out once
and passed when re-run, as it did on the first commit.
**Local caveats:** the local `channel_members` roster-fence test and the
deletion tests fail on a shared local database because other tests leave
its fence trigger disabled, so the PostgreSQL Tests job in CI is the
authority for the full suite. The pre-push `rust-tests` hook passed.
`desktop-tauri-checks` failed on `chunks_exact` lints in desktop files
this PR doesn't touch, so the push used `--no-verify`. `cargo clippy
--all-targets` (buzz-db, buzz-relay) and `cargo fmt --check` are clean.
---------
Signed-off-by: Bradley Axen <baxen@squareup.com>
Signed-off-by: Luke Tornquist <tornquist@squareup.com>
Co-authored-by: ART <ea39c1c8acf57180161ef4ba4ed68abf8589f5291956b909a5aa991fc66664ca@buzz.block.builderlab.xyz>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: claude00 <fb8b76a01257932b8ca35060862c29bd09b2137638573731c2d5074fc02c5385@buzz.block.builderlab.xyz>
Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz>
Co-authored-by: Luke Tornquist <tornquist@squareup.com>
Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
|
||
|
ravarora2
|
4250014bae |
Run user deletion through saved stage transitions (#8520)
## Summary An operator can submit, approve, and run a synthetic user-deletion request through a durable workflow. The actions print messages; they do not gate real users or delete data. The inventory contains artificial participant and former-participant communities. The worker calls one transition function, `get_next_stage(lease, previous_stage_id)`. It receives a full stage row, passes it to the handler, then asks again. All branching, community tracking, initialization, and recovery live behind that function. Existing community-deletion execution is unchanged. ### Related issue Builds on merged #8496 and targets `main`. Inventory, separate approval records, and the dummy executor remain together in this PR. ### Execution design ```text approve → save approval; create no stages run → claim request get_next_stage(lease, None) → full stage row handler(lease, row) → persist status and result get_next_stage(lease, Some(row.id)) → next row / Finished / WaitUntil repeat, then release ownership ``` - **Approval:** bind an operator's decision to the saved inventory digest. Approval creates no execution rows. Operator names remain self-asserted demo input. - **Initialization:** claiming preserves the parent lifecycle. The first transition creates `gate_user` and changes `approved` to `running` atomically. A crash after claim but before initialization can safely restart. - **Handler:** read `metadata_json.inputs`, mark the row running, and save its result or failure. A successful action leaves its row `completed` and `is_current = true`. This is a durable transition waiting to be processed. - **Transition:** lock the parent, validate ownership, reload history and the handler's saved result, and choose an allowed destination. Clear the old current flag, save the decision, and insert the successor with its inputs in one transaction. Terminal transitions complete the parent instead. An insertion failure leaves the completed old row current. - **Recovery:** `previous_stage_id` identifies the handler that just returned; it must belong to the request. A restarted worker passes `None` and finds the saved current row. If that row is completed, transition it without repeating its action. If the switch already committed, return the saved successor. Repeated calls never apply the old transition twice. - **Progress:** reconstruct covered communities from saved decisions and the approved inventory. Each stage records its predecessor ID, so reconstruction does not depend on timestamp ordering. Missing current state in a started request, disconnected history, unknown stages, and malformed results fail without inventing progress. - **Ownership:** parent leases last 60 seconds and handlers renew them every ten seconds. Generation checks reject stale writes. Handler and renewal futures remain independently polled; renewal cannot prevent a handler holding the parent lock from committing. - **Step-by-step testing:** `run --next-step` executes one handler, commits its transition, and exits with the successor pending. Repeat the same command to advance again; the final step completes the request. The former `--max-stages` and `--pause-after-handler` flags are removed. - **Failure:** the handler saves the current row's error and a 30-second retry deadline. The transition returns `WaitUntil`; the worker exits and releases its lease. A later invocation retries the same row. No background scheduler or automatic retry daemon is added. The user workflow definition in `buzz-deletion/src/user/workflow.rs` declares allowed destinations and a decision function per stage. `gate_user` enters the first uncovered community. `purge_db` reads saved `has_agents`: true selects `purge_agents`; false completes the community immediately. Community completion selects the next uncovered community, or finishes the request when all are covered. No full stage plan is created upfront. The transition transaction contains no stage-name comparisons or fixed community count. Stage metadata stores `inputs`, `result`, `transition`, and `previous_stage_id`. The parent retains inventory, ownership, retry, and overall status. Approval evidence stays in `entity_deletion_approvals`. A completed-current row bridges the two transactions; real handlers will still need idempotent or reconciled actions if a process dies before saving its result. ### Code organization `buzz-deletion/src/user/worker.rs` runs the loop and heartbeat. Sibling `handlers.rs` executes dummy actions. `transition.rs` defines `UserDeletionTransition.get_next_stage()`, and `workflow.rs` owns the user stage rules and community-progress reconstruction. The CLI remains in `user.rs`. `buzz-db/src/store/entity_deletion/` contains shared workflow persistence: a small `mod.rs`, request reads/inspection in `requests.rs`, approval evidence in `approvals.rs`, ownership in `leases.rs`, and stage outcomes/atomic transitions in `stages.rs`. User-specific submission, demo inventory, and inventory validation live separately in `buzz-db/src/store/user_deletion/{requests,inventory}.rs`; its `mod.rs` only defines and exports the store. The worker calls the user transition object, which supplies a synchronous decision function to `EntityDeletionStore.transition_stage`. The store locks the parent, reads saved state, invokes that function, checks the returned operation, and commits its writes. The decision function performs no SQL or external calls. Reading, deciding, and writing remain in one transaction; there is no stale-state gap. Shared storage contains no user stage names or community-routing rules. User-specific approval validation also runs inside the shared approval transaction. This reorganization changes no tables, migrations, stage metadata, or CLI behavior. Requests using the existing saved-result format continue to work. `--next-step` still runs one handler and commits its transition before returning. ### Testing The walkthrough in [docs/operator-user-deletion-executor.md](https://github.com/block/buzz/blob/rarora/user-deletion-approval/docs/operator-user-deletion-executor.md) includes commands, SQL inspection, expected rows, and recovery boundaries. Submitted and approved a fresh local request and verified zero stage rows. Ran `run --next-step` five times. Each call completed exactly one handler, released ownership, and left its successor pending: B DB, B agents, C DB, C agents, then request completed. A sixth call left saved state unchanged. Verified both removed flags are rejected. Ran `--demo-no-agents-in 00000000-0000-0000-0000-00000000000c` and verified four completed rows, with no C agents stage. Injected a DB-stage failure, verified immediate retry refusal, waited for the real deadline, and resumed the same stage row. Observed heartbeat renewal and rejection of a competing worker, killed the owned worker process, waited for its actual lease expiry, and resumed the same unfinished stage with a new generation. Re-running a completed request left its saved state unchanged. Repeated the CLI walkthrough after the file reorganization, including runtime branching and a retry after the real deadline; a completed request created before the reorganization remained unchanged. Mutation-tested the ownership and recovery guards against local PostgreSQL 17. Reclaiming with the same executor ID rejects old-generation operations. Independently disabling the ownership generation check, release generation check, saved-transition/result comparison, or heartbeat-failure handling made the corresponding test fail. Restored all four protections afterward. The heartbeat test takes over the request while its handler is already running. ### Schema and rollout Unmerged migrations 0069–0070 add inventory and execution fields, widen request lifecycle states, and create operator-global approval and stage-history tables. They set five-second lock and thirty-second statement timeouts; constraint validation has its own transaction. No triggers, functions, foreign keys, or data backfills are added. Desired-state and migration catalog parity cover all three tables. The transition redesign adds no further schema changes. The serving relay does not schedule these requests. Only the explicit operator CLI runs them, using PostgreSQL. Only `demo:` targets are accepted. Real account discovery, user-write fencing, owned-community handling, and actual deletion remain future work. Verify staging with a new demo label and the documented commands. Older draft requests do not have the new stage metadata contract. Preserve them and create a new demo request for this version; they are not a supported resume format. Use a fresh database if an earlier draft applied a different migration checksum. Never alter sqlx migration checksums. With Helm `migrate.autoMigrate=true` (the chart default), a relay binary that lacks 0069/0070 fails at boot with sqlx `VersionMissing` after those migrations are applied. Recovery for this rollout is a roll-forward: deploy a corrected binary that retains the migrations. Do not delete migration records or change checksums to force an older binary to boot. Rolling back with auto-migration disabled requires a separately verified schema-compatibility plan. Demo requests, stage history, and approvals are intentionally retained; this scaffold has no cleanup command or retention job. Recheck migration numbering before merge. Generated with Codex --------- Signed-off-by: Ravneet Arora <rarora@squareup.com> |
||
|
seiler
|
1a9d70f698 |
Merge pinned main primary-key repair into invitation candidate
Preserve invitation migration 0069 and admission semantics while integrating main events_pkey reconciliation and schema-parity coverage. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: seiler <seiler@squareup.com> |
||
|
Ian Oberst
|
20ba95ed89 |
fix(db): rebuild events_pkey in declared order after pgschema apply (#8525)
## Summary Re-applying `schema/schema.sql` to a database that `pgschema apply` and `scripts/reconcile-schema-after-pgschema.sql` already bootstrapped fails with `cannot drop inherited constraint "events_p2026_01_pkey" of relation "events_p2026_01" (SQLSTATE 42P16)`. Because of that, `scripts/start-relay-for-tests.sh` breaks against a reused database. **Cause:** pgschema 1.7.4 moves a partitioned table's partition key to the front of its primary key when it creates the table. A pgschema-bootstrapped `events` therefore gets `PRIMARY KEY (created_at, community_id, id)`. `schema/tables/public/events.sql` and `migrations/0001` both declare `(community_id, created_at, id)`. Every later plan drops and re-adds `events_pkey` and each declared child's key. This happens even on a database that was never reconciled. The plan only becomes unexecutable after the reconcile script attaches the children, because their keys are then inherited. As a side effect, pgschema-bootstrapped databases also had a differently ordered PK index than migration-managed ones. **Fix:** after the partition ATTACH block, the reconcile script now compares `events_pkey` with the declared `PRIMARY KEY (community_id, created_at, id)`. If they differ, it rebuilds the key on the parent. It then asserts that `events` and every partition carry the declared key, which is the "convergence plus live assertion" shape `docs/database.md` requires. PostgreSQL recreates each partition's key with the same `<child>_pkey` names, so a reconciled re-apply now plans nothing. The block does nothing when the key is already correct. `migrations/0001` is untouched. Only pgschema-bootstrapped dev and test databases run this script, so it does not touch the out-of-band partitioned-table procedure. `delivery_log` is not affected. Its key `(delivered_at, id)` already starts with the partition key. **Existing local databases:** a database reconciled before this change would still fail its next `pgschema apply`, because the reconcile script only runs after apply. The repair therefore also lives in `scripts/repair-events-pkey.sql`. That file does nothing when `events` doesn't exist yet, and `scripts/start-relay-for-tests.sh` runs it before apply; it's the only launcher that reuses its database. The reconcile script keeps a verbatim copy for fresh bootstraps, and a unit test keeps the two identical. The rebuild drops the key with `IF EXISTS`, so a database that has lost its key still converges. It also takes its locks with a 10s `lock_timeout`. If a relay still holds `events`, it fails with "Stop any relay using this database, then rerun" instead of hanging. `start-relay-for-tests.sh` now builds the relay's `DATABASE_URL` from the `PG*` variables it already exports. The secret scanner flags the literal URI once the file is touched. The value is unchanged. pgschema 1.13.3 keeps the declared order and creates children as native `PARTITION OF`. Upgrading would make this block and the ATTACH blocks unnecessary, but it changes behavior for every caller, so it is left for a separate change. ### Related issue Follow-up to block/buzz#8487, which added the probe test and `.pgschemaignore`. Now rebased onto `main` after that PR merged. ### Testing - `pgschema_reapply_keeps_runtime_partitions` now asserts the plan after the maintenance pass is **empty**, which replaces the `DROP TABLE` and per-monthly plan checks. It then applies and reconciles a second time. Without the new block it fails with `ALTER TABLE events DROP CONSTRAINT events_pkey; ...`. With the block it passes. - It also includes the optional notes from the #8487 review: - After maintenance, it checks that both catch-alls are still present and attached, not only that the zipped bounds differ. - The first runtime monthly comes from the declared catch-all's lower bound instead of a hard-coded `p2026_07`. - The doc comment now states only what the test pins: bounds are not compared, and the plan is empty. - I broke the rebuild on purpose by re-adding the key in the wrong order. The new assertion fails the reconcile script with `events and its partitions must have PRIMARY KEY (community_id, created_at, id)`. - CLI runs against local Postgres 17: - Fresh DB: apply → reconcile → apply → reconcile → plan. The second apply reports no changes and the plan is empty. - DB reconciled with the old script: the next apply fails with SQLSTATE 42P16. After running the new reconcile script once, apply reports no changes and the plan is empty. - `events_pkey_repair_runs_before_reused_apply_and_matches_reconcile` checks two things: the reconcile script carries the repair block verbatim, and `start-relay-for-tests.sh` runs the repair before `pgschema apply`. It fails if the two copies drift. - I ran the repair script and then the bootstrap against four local probe databases: one reconciled with main's script, an empty one, one applied but never reconciled, and one with no `events` key. All four end with an empty plan. With another session holding a lock on `events`, the repair fails after 10s with the hint and leaves the key untouched. It succeeds once the lock is released. - `admin_schema_parity_between_desired_state_and_migrations` and the `runtime::migration` unit tests pass. fmt and clippy are clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Ian Oberst <ioberst@block.xyz> Co-authored-by: Ian Oberst <ioberst@block.xyz> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
seiler
|
2fabeed17e |
Merge main into invitation revocation candidate
Preserve entity deletion migration 0068 and move the schema-only invitation expansion to 0069. Keep both migration inventory contracts and exercise populated invitation upgrades from the complete main schema through 0068. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: seiler <seiler@squareup.com> |
||
|
ravarora2
|
2d289f720d |
BUZZ-301: Add user deletion request intake (#8496)
## Summary
Add the first testable piece of user deletion: submit a synthetic
request, list requests, and inspect one by UUID. Operators can verify
that intake reaches PostgreSQL before we add inventory, approval, and
execution.
Add one operator-global `entity_deletion_requests` table with an
additive migration and matching desired-state schema. Identical
submissions return the same row. Conflicting submissions fail without
changing it. Only `demo:` targets are accepted, and requests stay
`submitted`.
Keep the commands in `buzz-deletion`, reached through a thin `buzz-admin
user-deletions` adapter. Reuse the existing database connection setup.
This change adds no worker, stage history, community gates, or deletion
operations.
### Rollout plan
This is an expand-only schema change plus an operator CLI. Existing
relay code does not use the new request table. No backfill or real
deletion is part of this release.
1. **Order the PRs.** Land #8496 first, using 0068; main now owns 0067
for channel join roles. Rework #8489 to build on this table with the
next available migration number. Renumbering its existing CREATE TABLE
migration alone is insufficient; both PRs currently create the same
request table.
2. **Apply on staging.** For deployments that manage schema separately,
apply the reviewed additive migration before using the new CLI. Where
auto-migration is enabled, the new relay binary applies it at startup.
Keep existing tables and records unchanged.
3. **Verify on staging.** Use the new `buzz-admin` binary with
`DATABASE_URL` pointing to staging. Submit a unique synthetic label,
list requests, and inspect the returned UUID:
```bash
buzz-admin user-deletions submit --target demo:staging-intake-1
--requested-by <operator>
buzz-admin user-deletions list --limit 5
buzz-admin user-deletions inspect <returned-uuid>
```
Verify one `entity_deletion_requests` row with status `submitted`.
Repeat the identical submission and verify the same UUID and timestamps.
Use a new label if this label was used by an earlier staging test.
4. **Apply beyond staging.** After verification, use the deployment's
reviewed schema/release workflow to add the table in the remaining
environments. Requests run only through the operator CLI; this release
starts no deletion worker. No feature flag is needed for this synthetic
intake: the database accepts only `demo:` targets, and real deletion is
absent.
5. **Rollback.** Keep the additive table and saved requests; stop using
the new commands and roll back the binary if needed. After 0068 is
applied, an older binary with auto-migration enabled can fail at startup
with sqlx `VersionMissing`. A binary rollback therefore requires
migration execution to be disabled on the older relay
(`BUZZ_AUTO_MIGRATE=false`, including the corresponding deployment
configuration).
### Related work
This is a smaller first slice of the full-workflow draft
[#8489](https://github.com/block/buzz/pull/8489), which is preserved for
later extraction. The two PRs overlap and should not both be merged as
written.
### Testing
The author submitted `demo:rarora-manual-1` to a separate local
PostgreSQL 17 database. Repeating the same submission returned its
original UUID and timestamps. List, inspect, and a direct SQL query all
returned the same saved request.
<details>
<summary>Actual command inputs and outputs</summary>
Commands ran from the new worktree with `DATABASE_URL` set to the local
`buzz_user_deletion_submit` database. Outputs below are stdout;
connection diagnostics on stderr are omitted.
**Submit again — returns the existing request without creating a
duplicate**
```bash
./target/debug/buzz-admin user-deletions submit \
--target demo:rarora-manual-1 --requested-by rarora
```
```json
{
"id": "2e5bcdec-ab68-44d2-9743-1dd9fa7d2f89",
"entity_type": "user",
"target": "demo:rarora-manual-1",
"status": "submitted",
"requested_by": "rarora",
"reason": null,
"created_at": "2026-10-20T19:57:44.516698Z",
"updated_at": "2026-10-20T19:57:44.516698Z"
}
```
**List saved requests**
```bash
./target/debug/buzz-admin user-deletions list --limit 5
```
```json
[
{
"id": "2e5bcdec-ab68-44d2-9743-1dd9fa7d2f89",
"entity_type": "user",
"target": "demo:rarora-manual-1",
"status": "submitted",
"requested_by": "rarora",
"reason": null,
"created_at": "2026-10-20T19:57:44.516698Z",
"updated_at": "2026-10-20T19:57:44.516698Z"
}
]
```
**Inspect one request**
```bash
./target/debug/buzz-admin user-deletions inspect 2e5bcdec-ab68-44d2-9743-1dd9fa7d2f89
```
```json
{
"id": "2e5bcdec-ab68-44d2-9743-1dd9fa7d2f89",
"entity_type": "user",
"target": "demo:rarora-manual-1",
"status": "submitted",
"requested_by": "rarora",
"reason": null,
"created_at": "2026-10-20T19:57:44.516698Z",
"updated_at": "2026-10-20T19:57:44.516698Z"
}
```
**Read the table directly**
```bash
docker exec buzz-user-deletion-dev-20261020 \
psql -X -U buzz -d buzz_user_deletion_submit -c \
"SELECT id, entity_type, target, status, requested_by
FROM entity_deletion_requests ORDER BY created_at DESC;"
```
```text
id | entity_type | target | status | requested_by
--------------------------------------+-------------+----------------------+-----------+--------------
2e5bcdec-ab68-44d2-9743-1dd9fa7d2f89 | user | demo:rarora-manual-1 | submitted | rarora
(1 row)
```
</details>
A separate CLI smoke test also verified that a conflicting operator and
a non-demo target fail without changing the saved request. The existing
community-deletion `list --limit 1` command passed against the smoke
database.
See [the intake
walkthrough](
|
||
|
Logan Johnson
|
a2d28eb6c4 |
fix(relay): apply channel setting changes atomically and lock every membership writer (#8516)
🤖 ## Summary This PR fixes two ways the relay could report a channel change that did not fully happen. **1. Channel settings could be saved only in part, while the app was told that they all worked.** When an owner or admin changed a channel's settings (name, description, topic, public or private, message lifetime, archived or not, who can join), the relay saved the request first and then applied each setting with its own database write. If one write failed, the relay only logged the error. It skipped the remaining settings and still reported success. Sending the request again did nothing, because the relay saw it as already saved. Example: a switch from public to private could fail while the app showed success, and the channel stayed public. Now saving the request, applying every setting and saving the "settings changed" messages in the channel happen in one database transaction. Either all of it is saved or none of it is: - If anything fails, nothing is saved and the client gets an error. It can send the same request again. - A repeat of a request that already succeeded is reported as a duplicate and changes nothing. This is also true when the repeat arrives while the first copy is still being saved. - A request to restore a channel that is not archived, or to change an archived channel without restoring it, is refused and nothing is saved. Before, the relay saved the request and a "settings changed" message, and then the restore failed. - Live updates go out only after the save commits. These are: removing access for people who lose it, and pushing the new messages and channel info to clients. **2. A published member list could include someone who had just been removed.** For some channels the relay publishes a list of the channel's members. While it builds that list, it holds a per-channel membership lock. Kicks by a relay admin and the automatic kick after a deploy did not take that lock. A kick could commit after the list was read, so the published list still showed the removed member. Channel deletion also did not wait for the lock. Kicks, channel deletion and the automatic add when someone joins a Huddle (voice call) now all take the lock first. The published list then includes the change, or the change waits until the list is saved. ## Details - **One lock order for all channel settings writes.** Before, each writer took its own locks. Review kept finding writers that took them in a different order or forgot one. `lock_channel_for_settings_write` (`crates/buzz-db/src/store/channel.rs`) is now the only place that takes them. The order is: 1. the membership lock, only when the public/private setting or the "who can join" setting (`join_role`) changes. Without it, a member add could read the old rule while the change commits. 2. the message-lifetime (TTL) lock: exclusive for a lifetime change, shared otherwise. 3. the channel row (`FOR NO KEY UPDATE`). This does not block the key lock that a member insert takes. The row lock also orders two copies of the same request, so the second copy sees the first as already stored. - The function returns a `ChannelSettingsWriter`. The writer borrows the transaction that took the locks, so the compiler stops it from being used with another transaction or after commit. The settings writes (`update_channel_on`, `set_topic_on`, `set_purpose_on`, `set_archived_on`) require the writer. They refuse a change its locks do not cover. - `Db::apply_channel_metadata_event` stores the channel-edit event (kind:9002) and its mentions, applies the settings and stores the "settings changed" messages (kind:40099) in one transaction. The older single-purpose writes (`update_channel`, `set_topic`, `set_purpose`, `archive_channel`, `unarchive_channel`) use the same helpers and behave as before. - `ingest.rs` and `validate_admin_event`: channel edits no longer check archived/active state before the write. The transaction checks it after its duplicate check, under the row lock. A refusal there goes back to the client as `invalid:`. Channel edits have their own save path and skip the general post-save step. `handle_side_effects` refuses them, so no caller can go around the transaction. - `side_effects.rs`: `emit_system_message` is split into signing and sending. The messages are signed before the transaction and sent after it. All "settings changed" messages from one edit share one timestamp and come in a fixed order (topic, purpose, visibility, lifetime, archive). - A change to "who can join" (`join_role`) is applied in the same transaction and announced with its own `join_role_changed` message. - **Membership lock outside settings** (`relay_admin_actions.rs`, `channel_members.rs`, `audio/handler.rs`): both kick paths (`execute_kick_with_marker`, `deploy_kick_member`) take the membership lock before they remove the member. The marker kick still undoes the removal if its lease check fails after waiting. The Huddle join takes its locks through a new `ChannelMembershipLocks` token, and the auto-add is a method on that token, so it cannot run without the lock. `soft_delete_channel` takes the full settings lock order. - **Limit of the membership lock:** it covers member changes, public/private and "who can join" changes, and channel deletion. It does not cover archiving: archive writes and the cleanup of expired temporary channels do not take it. The cleanup keeps its single UPDATE. A comment in `channel.rs` explains why that is safe and why it must not be split into a read and a write. ### Related issue Follow-up to the review of #8143 (which added the "who can join" setting). That review found that channel edits could be applied in part. No separate issue. ### Testing New PostgreSQL tests in `crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs`: - `failed_edit_stores_nothing`: a test trigger makes one write fail partway through the transaction. Nothing is stored or changed, on the first send or a resend. Without the trigger, the same signed request applies in full. - `resent_edit_is_a_duplicate`, `resent_archive_and_unarchive_are_duplicates`: a resend after success is a duplicate and adds nothing. - `resend_validated_after_the_first_commit_is_a_duplicate`: a resend validated after its first copy committed is a duplicate, with no second change and no second message. - `concurrent_copies_of_a_restore_apply_once`: two copies of one restore, held at the channel lock until both are waiting, apply once. The other copy is a duplicate. - `unarchive_of_an_active_channel_is_refused`, `join_role_edit_is_applied_and_announced`. Lock tests in buzz-db: each one holds a member list in progress and checks that the writer waits for it. They cover a "who can join" change (`channel_members.rs`), the deploy kick and the marker kick (`relay_admin_actions.rs`), and channel deletion (`channel.rs`). The kick and delete tests fail when the new locks are removed. These tests and the existing channel-edit tests in `group_state_postgres_tests` pass locally against a migrated PostgreSQL database. --------- Signed-off-by: Logan Johnson <loganj@squareup.com> Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> |
||
|
seiler
|
85f08b9692 |
fix(db): order invitation expansion after channel join roles
Keep main migration 0067 intact and move the unpublished invitation expansion to 0068. Exercise populated upgrades from complete main through 0067, retaining guest channel admission policy on both schema paths. Signed-off-by: seiler <seiler@squareup.com> Co-authored-by: Codex <noreply@openai.com> |
||
|
Brad Seiler
|
60d4441b90 |
Merge main into invitation repair candidate
Signed-off-by: Brad Seiler <seiler@squareup.com> |
||
|
Ian Oberst
|
347ce88d5d |
fix(db): keep pgschema re-apply from dropping runtime partitions (#8487)
## Summary pgschema doesn't compare partition bounds. Re-applying `schema/schema.sql` to a database a relay has used therefore plans `DROP TABLE IF EXISTS ... CASCADE` for every `events` / `delivery_log` monthly the partition manager created at runtime, and those tables' rows are deleted with them. Since #8464 the manager advances the catch-all by default, so any local dev or test database a relay has started against now hits this. One example is the persistent `buzz` database that `scripts/start-relay-for-tests.sh` re-applies. This adds a root `.pgschemaignore` that hides dated monthlies (`events_p2*`, `delivery_log_p2*`) from pgschema. The monthlies declared in `schema/partitions/` are exempted with `!` patterns, so pgschema still manages them. pgschema reads the file from its working directory, and every caller already runs it from the repo root: the test-relay launchers, `postgres-test-setup.sh`, the desktop release smoke and `_ci-relay.yml`. Both partition files now carry a note pointing at it, and AGENTS.md gotcha #7 adds "run pgschema from the repo root" next to the reconcile rule. New-install layout is unchanged. **Why the drops haven't deleted data yet:** on a reconciled database a re-apply also plans to drop and re-add the `events` primary key on the parent and on each declared child. The apply then fails with `cannot drop inherited constraint "events_p2026_01_pkey"` (42P16). That happens with or without this PR. The planned drops run in the same single transaction, so they roll back, and the runtime monthlies have survived so far. Once that primary-key drift is fixed, re-applies will succeed and this ignore file is what keeps them from dropping the runtime monthlies. The primary-key fix is a separate follow-up. ### Related issue Follow-up to #8464. Salvaged from an earlier branch; no existing issue found. ### Testing - `pgschema_reapply_keeps_runtime_partitions` (Postgres) follows the real bootstrap path in a dedicated database: - `bin/pgschema apply` from the repo root, then `scripts/reconcile-schema-after-pgschema.sql`; - the relay's default maintenance pass (create plus advance); - asserts that both catch-alls were attached before the pass and that their bounds moved after it; - asserts that each parent's `p2026_07` monthly exists. The catch-all advanced past the declared bounds but kept its name. - asserts that `pgschema plan` contains no `DROP TABLE` and no line mentioning any runtime monthly. - Falsified both ways: - With `.pgschemaignore` removed, the test fails and prints the planned drops. - Without the reconcile step, it fails with `reconcile did not attach both catch-alls`. - `pgschemaignore_exempts_exactly_the_declared_monthlies` checks that the `!` exemptions exactly match the dated monthlies in `schema/partitions/`. - The admin parity test now shares the `pgschema` helper, so it also runs from the repo root like the real callers. - All `runtime::migration` and `store::partition` tests pass against local Postgres after merging current main, including the ignored ones: 104 passed, run with `--test-threads=1` because the full nextest Postgres lane needs local `dropdb`. `clippy -D warnings` and the pre-push lanes pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Ian Oberst <ioberst@block.xyz> Co-authored-by: Ian Oberst <ioberst@block.xyz> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Luke Tornquist
|
2f2f6e2774 |
fix(db): bound every statement and the #p mention scan (#8461)
## Problem An agent `#p` + `#h` + `since` REQ (the `require_mention` subscription in `buzz-acp`) ran for over 45 minutes on a production writer and held `AccessShare` on every `events` partition. There were two causes: 1. **The mention side of the `#p` join was unbounded.** `build_query_events_sql` joined `event_mentions` on `(community_id, event_id)` only. The planner picked a nested loop whose outer side was every mention of the pubkey, ever, and whose inner side range-scanned the channel index from `since` on each partition. Cost ≈ total mentions × channel rows since `since`. 2. **Nothing cancelled it.** The writer `statement_timeout` defaulted to `0`. Only `#e` reads had a deadline (the 20s cap from #7854), and read-replica sessions had none at all. `pg_stat_statements` shows this shape has a multi-thousand-second max going back months, so it is chronic and was not introduced by a recent change. ## Change **`#p` mention bound** (`store/event.rs`, `push_mention_join`, shared by the page SELECT and COUNT) - Join on the full event key, `AND e.created_at = m.event_created_at`. Mention rows copy the event's `created_at` at insert (migration 0007 already relies on this equality), so the join can prune `events` partitions by time. - Repeat `since`/`until` on `m.event_created_at`, so the outer scan is a range on the existing `idx_event_mentions_pubkey_created`. `until` is a superset bound for the keyset cursor; the exact `(created_at, id)` cursor test stays on `e`. - Repeat `kinds` on `m.event_kind` when the filter has kinds. Every production mention writer binds the event kind (`insert_mentions` in `store/runtime/mod.rs`, and the replaceable-event path in `store/replaceable.rs`, back to the first Postgres schema), and no migration inserts mention rows, so this drops no rows written by the relay. Rows imported out of band with a NULL `event_kind` would no longer match a kinds-filtered `#p` read; a read-only production count on 2026-10-20 found 0 such rows on both production reader databases. The pushdown makes `idx_event_mentions_pubkey_kind_created` available to the planner for the `require_mention` kind-9 shape; its use has not been observed (see below). An E2E fixture with a hand-set NULL `event_kind` row confirmed the caveat: base returns 8 events, head 7. - The feed's mentions and needs-action queries (`store/feed.rs`, `build_mentions_query` / `build_needs_action_query`) join `event_mentions` the same way, so each joined row probes one `events` partition instead of all of them. Their `ORDER BY m.event_created_at ... LIMIT` already kept them from running away; this removes the per-row cross-partition probe. - Observed plan (E2E at `9cb6638b1`, before the `kinds` pushdown; 60,000 historical mentions + 2,500 recent decoys, stock planner, `EXPLAIN (ANALYZE, BUFFERS)` of the captured production SQL): the mention side is a bounded range on `idx_event_mentions_pubkey_created` (13 rows), then a hash join with a scan of the recent `events` partition. Base scanned 2,509 recent events and probed `event_mentions_pkey` 2,509 times. Head: **1.1 ms / 319 shared hits**; base: **8.5 ms / 10,049 shared hits**. Not measured against the production replica. A fresh `EXPLAIN (ANALYZE, BUFFERS)` replay at `a7065c367`, after the pushdown, ran on a smaller fixture where the planner chose a sequential mention scan, so it neither confirms nor replaces these numbers. **Reads: 20s deadline on every generic event read** - `query_events_on` and `count_events_on` always run under the transaction-local 20s deadline that `#e` reads used (`fetch_with_read_deadline`, formerly `fetch_with_e_tag_deadline`). It only tightens: shorter operator or caller caps are kept. - Read-replica sessions get a 20s session default through `after_connect`, using the same tighten-only SQL. - Cost: every generic REQ history read and COUNT now pays BEGIN + `set_config` + ROLLBACK (three extra round trips; before, only `#e` reads did). In the local E2E the end-to-end history read was still faster at head (median 4.1 ms vs 10.2 ms at base, 9 samples each, warm cache), so the round trips are dominated by the bounded plan; this does not isolate them or establish production p99. The round trips also lengthen how long each such read holds its writer connection; that hold time has not been measured against production pool sizing. - A replica read cancelled at its deadline (57014) is returned, not re-run on the writer. Usually the cancel means expensive work the writer would only repeat. It can also mean the replica waited on a lock only it holds (a replayed `ACCESS EXCLUSIVE` from primary DDL), when the writer would have answered quickly; that miss is the accepted price of keeping runaway reads off the primary. The rationale now lives once on `DbError::is_statement_cancelled` instead of in a dozen copied comments. The generic routed reads and `ReadSession` already did this; the eight routed reads with their own SQL (feed mentions, needs-action and activity; thread replies; channel window; relay membership; thread window and its auxiliary scan) now do too. A cancelled auxiliary scan also keeps its replica session instead of degrading it to the writer for the rest of the request. Other replica failures (e.g. hot-standby recovery conflicts) still fall back to the writer. - Paths with their own tighter `SET LOCAL` (artifact 2–5s, thread window 4s, personal reads, usage 5–15s, serving lease 1.5s) are unchanged. **Writes: every writer session is bounded** - The default `statement_timeout` changes from `0` to 120s (`DEFAULT_STATEMENT_TIMEOUT_MS`). - `BUZZ_DB_STATEMENT_TIMEOUT_MS` still overrides it. `0` or any value above 300s installs 300s (`MAX_STATEMENT_TIMEOUT_MS`), so a session can no longer disable it, and a clamped value logs a `warn!` when the pool is built. Only the operator-approved, fenced destruction paths below lift it, per transaction. **Long-running writer paths, audited** | Path | Behavior | |---|---| | Migrations / schema destruction (`with_exclusive_schema_destruction_lock`) | Unchanged: these lift lock and statement timeouts on their own detached connection | | Partition maintenance | Unchanged: own 5s `statement_timeout` | | Partition audit (`buzz-admin`) | Unchanged: 30s | | Community deletion inventory, purge and logical verification (`inventory_schema` whole-tenant counts, `purge_postgres` whole-community `DELETE` per table in one tx, `verify_postgres_logically_deleted` whole-tenant `EXISTS` per table right after the purge) | Lifted (`statement_timeout = 0`) transaction-locally by `lift_statement_timeout`, like migrations; lock and idle timeouts still bound them. Inventory counts each table in its own lifted transaction, so each table's lock and snapshot are released as soon as its count finishes, as the autocommit counts did. A cancelled purge would retry from scratch and eventually block, so a cap here would turn large-tenant deletion into an operator override | | Audit appends (`buzz-audit`, audit pool inherits the writer cap) | Waits on the audit-chain advisory lock. With `BUZZ_DB_LOCK_TIMEOUT_MS=0` (or at least the statement cap), `statement_timeout` (57014) cancels the wait instead of `lock_timeout` (55P03). The relay's audit worker now retries both with the same backoff instead of dropping the entry on 57014; admission refusal stays terminal | On HTTP routes a statement timeout already maps to the distinct retryable 503 (`api/mod.rs`, `statement_timeout_maps_to_distinct_503`). A cancelled REQ fails its history read instead of hanging. ## Tests - `p_tag_query_bounds_mention_side_by_time` (unit): page and COUNT SQL carry the `created_at` join and the `since`/`until`/`kinds` mention bounds, and the bounds are absent when the filter has none. - `p_tag_read_with_mention_bounds_keeps_results` (Postgres): through the production mention insert, unbounded, `since`, window and keyset-cursor reads return exactly the mentioned rows, and COUNT matches. Every bound sits exactly on a mentioned row's second and the cursor page includes a same-second sibling after the cursor id, so an off-by-one on either mention-side bound fails it. - `plain_read_is_cancelled_at_deadline_when_timeout_disabled` / `count_events_cancels_at_read_deadline_when_timeout_disabled` (Postgres): the production `#p` + `#h` + `since` shape and a plain COUNT are cancelled at 20s with the session timeout disabled. Previously these covered `#e` only. - `writer_statement_timeout_is_always_bounded` (unit) and `session_statement_timeouts_bound_writer_and_reader_sessions` (Postgres): through `Db::new`, writer sessions install `2min` / `5min` (for `0` and for 600s) / `45s`, and replica sessions install `20s`. - `deletion_whole_tenant_stages_outlive_the_writer_statement_timeout` (Postgres): on a scratch desired-state database with a 500ms writer cap, the real `inventory_schema`, `purge_postgres` and `verify_postgres_logically_deleted` each wait behind a held `ACCESS EXCLUSIVE` lock on `events` for over 1s and still complete, a plain statement is cancelled (57014), and pooled sessions still read `500ms` afterwards. A lifted transaction reads `statement_timeout = 0`, so a bounded lift (e.g. `'300000'`) fails it, as does removing any of the three `lift_statement_timeout` calls. - `mentions_query_is_tenant_scoped_and_joins_mentions_by_composite_key` / `needs_action_query_is_tenant_scoped_and_joins_mentions_by_composite_key` (unit): both feed queries carry the `e.created_at = m.event_created_at` join. - `replica_statement_cancel_propagates_without_writer_rerun` (Postgres): with the replica locked past its cap, each of the six routed reads above returns 57014 instead of the writer's copy. Removing the cancel arm in any of them fails it. - `thread_window_statement_cancel_propagates_without_writer_rerun` (Postgres): a thread-window read parked behind a replica lock and cancelled (57014; the window's own 1s `lock_timeout` would otherwise fire first) returns the cancel instead of the unlocked writer's window, and an auxiliary scan timed out on a replica session returns 57014 and leaves the session on the replica. Removing either guard fails it. - `audit_worker_retries_statement_timeout_until_original_entry_is_appended_once` (Postgres, `buzz-relay`): the production audit worker, with `lock_timeout` disabled and a 100ms statement cap, keeps the entry through repeated 57014 cancels while the chain lock is held and appends it exactly once after release. Treating only 55P03 as retryable fails it. - The existing writer and audit-pool GUC tests now expect `5min` for a disabled statement timeout. ## After deploy, watch - `buzz_db_route_decision{decision="replica",reason="statement_cancelled"}`: replica reads that now fail instead of falling back to the writer, including the replica-only lock case above. - `buzz_db_pool_acquire_wait_seconds` and `buzz_db_pool_waiters` on the writer pool: every generic REQ history read and COUNT now holds its connection for three extra round trips. ## Database checklist (`docs/database.md` § Review checklist) This PR changes schema-dependent `buzz-db` queries (the `#p` join and its `m.event_kind` pushdown). It does not change the schema. - **Phase:** none. No schema change, so this is not an expand, backfill or contract step. Nothing before or after it is required. - **Triggers, functions, procedures, rules, foreign keys:** none added. - **Row mutations / backfill:** no migration, and no row is mutated or backfilled. The one data-shape dependency is mention rows with a NULL `event_kind`: a kinds-filtered `#p` read no longer matches them (see the `kinds` bullet under Change). Relay writers always bind `event_kind`, so only rows imported out of band are affected. A read-only production count on 2026-10-20 found 0 such rows on both production reader databases. - **Contract migration:** N/A. - **Rule 4 locations:** none touched. No table, index or desired-state file changes, so no parity test applies. The new bounds use the existing `idx_event_mentions_pubkey_created` and `idx_event_mentions_pubkey_kind_created`. - **Deployed relay vs. new schema:** there is no new schema. The currently deployed relay and this relay run against the same tables. - **Lock and statement timeouts:** this PR is about them. Generic reads get a 20s deadline, replica sessions a 20s default, and writer sessions a 120s default capped at 300s. Community-deletion transactions lift `statement_timeout` per transaction; migrations and schema destruction are unchanged (see "Long-running writer paths, audited"). No index builds and no operator prebuild step. ## What got simpler - One read-deadline path instead of an `#e`-only branch in `query_events_on` and `count_events_on`. - The `#p` join SQL that was duplicated between SELECT and COUNT now lives in one `push_mention_join`. - The deletion tests share one scratch desired-state database helper instead of an inline copy. - "Is this query bounded?" now has one answer: yes, unless it runs in a migration, schema destruction, or community-deletion transaction. ## Identified debt (not in this PR) - `buzz-acp` re-sends the same filter after a timeout `CLOSED` (no backoff or narrowing). With the `#p` bound the `require_mention` read should finish well inside 20s, so this is a follow-up, not a blocker. --------- Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> |
||
|
Logan Johnson
|
d97f6b4c6e |
feat(relay): add announce channels with a join_role setting (#8143)
🤖 ## Summary Channels are Buzz's main way to control access, but today they cannot separate reading from writing. In an open channel, anyone who can read can also write. So there is no way to say "many people read, a few people write". Announcements, release feeds, plugin release records and shared configuration all need that. This PR adds **announce channels**: anyone can follow and read them, but only members can post. It does this with one new channel setting and without changing what any role means. - **Roles mean the same thing in every channel.** A *guest* can read and leave, but cannot write. A *member* (or *bot*) can write. *Admins* and *owners* also manage the channel. The relay already had the guest role, but no client used it until now. - **A new setting, `join_role` (`member` | `guest`), picks the role a new person gets** when nobody chooses one: when they join by themselves, or when someone adds them without a role. The default, `member`, keeps today's behavior for every existing channel. - **An open channel with `join_role = guest` is an announce channel.** Anyone can find, read and join it. People who join are guests, so they cannot write until someone makes them a member. The relay enforces these rules, so a client cannot get around them. What each role can do (the same in every channel): | Action | Guest | Member or bot | Admin / owner | | --- | --- | --- | --- | | Read, leave | ✅ | ✅ | ✅ | | Post, reply, edit, delete own, react, artifacts, canvas, forum, pins, topic, typing, workflow messages, save or run workflows, huddles | ❌ | ✅ | ✅ | | Add people | ❌ | ✅ as member, guest or bot | ✅ any role | | Make a guest a member (person or agent) | ❌ | ✅ | ✅ | | Any other role change | ❌ | ❌ | ✅ | | Change name, about, visibility, TTL, `join_role` | ❌ | ❌ | ✅ | Other rules: - **Joining by yourself.** A person who joins (kind:9021), or adds themselves (kind:9000 with their own key), gets the channel's `join_role`. They may ask for `join_role` or a lower role, never a higher one. So in an announce channel nobody can join as a member by themselves. - **Non-members.** A non-member may write only where joining would let them write: an open channel with `join_role = member`. That is today's behavior for open channels. - **Agents.** Agents follow the same rules as people. Any member who can write may make a guest agent a member. Limits that only an agent's owner may lift belong to the later IFC work (its "restricted" agent level), not to this PR. That work only refuses more actions, so it can be added later without breaking anything here. - **Leaving always works.** A member may leave an archived channel too (kind:9022, or kind:9001 naming themselves). The last owner still cannot leave. - **Agent owners keep edit and delete.** Today the human owner of an agent can edit (kind:40003) and delete (kind:9005) that agent's messages without being a member. This PR keeps that existing behavior in announce channels too, and documents and tests it. It does not let the owner post. - **Changing the setting changes no roles.** Existing members can still write after a change to `guest`. Existing guests stay read-only after a change back to `member`. A refused write gets `restricted: only members can post in this channel`. ## Details - **One write rule.** The relay's channel write check decides from the actor's role and the channel's `join_role`. Some event types skip that check and run their own: message edit, deleting your own message (kind:9005), topic and purpose (kind:9002), add-member (kind:9000), workflow messages and joining a huddle. Each of these now uses the same rule. Membership changes (add, remove, leave) do not need write access, so a guest can always leave. If the relay finds a stored `join_role` value it does not know, it treats it as `guest`, so an unknown value can never give more access. - **Admission is decided in the database transaction.** The membership write (`put_member`) makes the whole admission decision inside its transaction, after it takes the channel's membership lock: `join_role` for a missing role tag, the self-join ceiling, guests cannot add people, who may grant admin or owner, and role changes (`MemberRole::may_change_role`). The relay's request check uses the same rules, but the database is the authority, so a write that skips the request check cannot get around them. A change to `visibility` or `join_role` takes the same lock, so an add and a setting change cannot interleave. - **No creator exception.** `put_member` used to give owner to a channel's creator who added themselves to a private channel without an active membership. A removed creator could use that: their join (kind:9021) passes the open-channel check, the channel becomes private before the join takes the membership lock, and the join restored them as owner. Channel creation already makes the creator owner in the same transaction, so the exception is removed. A removed creator is now admitted like anyone else. - **Huddles.** Joining a huddle needs write access in the huddle channel **and** in its parent channel. The relay checks both before the join. When it commits the join, it applies the same rule again while it holds both channels' membership locks: a member needs a role that can post, and a non-member needs an open channel whose `join_role` is `member`. A guest in either channel cannot join. A join is also refused when the joiner loses membership or the channel's `join_role` changes before the commit. - **One lock order.** Each writer that takes more than one of a channel's locks takes them in this order: membership lock, TTL lock, channel row. Settings changes, roster publication, TTL refresh and huddle joins all use this order. Before this change, a `join_role` change on an ephemeral channel at the same time as a roster publication could deadlock. - **Workflows.** Saving a workflow (kind:30620) skips the generic write check, so it now checks the sender's role itself and needs a writing role before anything is stored. Every path that starts a workflow run, including the manual trigger (kind:46020), needs the workflow's owner to hold a writing role now (and an elevated role for an elevated workflow). So a workflow saved while its owner was a member stops running after the owner is made a guest. - **Channel names are checked before storage.** kind:9002 and kind:9007 refuse a name longer than the `channels.name` column (255 characters) before the event is stored. Before this, the relay accepted the event, then the name update failed, and the rest of the update (for example `join_role`) was silently skipped. This PR only adds this check. Storing the event and applying the update in one transaction is a larger change, and it is not in this PR. - **Changing the setting.** Owners and admins send a channel-metadata update (kind:9002) with `["join_role", "guest"]` or `["join_role", "member"]`. Buzz applies these updates as partial updates; a missing tag means "no change". The relay posts a `join_role_changed` system message in the channel. - **What clients see.** The relay signs a channel description event (kind:39000) that clients read. One shared builder now writes it for both the relay and the `buzz-admin` backfill, which had a copy that had drifted from the relay's. It always publishes an explicit `["join_role", …]` tag, plus the standard NIP-29 group access flags: | `visibility` | `join_role` | `public` / `private` | `restricted` | `closed` | | --- | --- | --- | --- | --- | | open | `member` | public | | | | open | `guest` | public | ✓ | | | private | either | private | ✓ | ✓ | This also fixes an old bug: `closed` ("join requests are ignored") was published for every channel, even open channels that accept joins. - **CLI and SDK.** `buzz channels update --join-role guest|member`. `buzz_sdk::build_update_channel` takes a new `join_role` argument and always writes it as an explicit tag. - **Docs.** `NOSTR.md` has an "Announce channels" section with the role table and the flag table. - **Tests.** Relay end-to-end tests, run in the CI relay E2E step: - `test_nip29_announce_channel_wire`: who can set `join_role`, the kind:39000 flags, joining as a guest, refused guest and non-member posts, reactions and adds, member posts, a member making a guest a member who then posts, changing the setting back without changing roles, and leaving. - `test_nip29_member_raises_guest_agent`: a member who is not the agent's owner can make a guest agent a member. - `test_nip29_announce_channel_wire_workflow_send_needs_write_role`: a guest's workflow message is refused; a member's is accepted. - `test_nip29_departure_wire_archived_channel_allows_self_leave`: guests and members can leave an archived channel. - `test_private_channel_any_member_can_invite` now expects a private-channel guest to be refused when it adds someone (it used to expect success). - `test_nip29_announce_channel_wire_workflow_commands_need_write_role`: saving and triggering a workflow, by a member and by a guest, in announce and normal channels; a guest cannot run a workflow saved while it was a member. - `test_nip29_announce_channel_wire_overlong_name_is_refused_before_storage`: a kind:9002 with an overlong name and `join_role` is refused, and kind:39000 still shows the old settings. - `test_nip29_announce_channel_wire_agent_owner_exception`: an agent's owner, as a guest and as a non-member, can edit and delete the agent's messages, while ordinary guests are still refused. - `test_nip29_guest_is_read_only_in_normal_channel`: a guest in an ordinary channel cannot write, and joining an ordinary channel still gives `member`. Unit tables cover the self-join rule for each `join_role`, and every actor, current role and requested role for the role-change rule. Database tests check the admission rules in `put_member` (`put_member_enforces_announce_admission`), that an add which waits on an uncommitted change to `guest` is decided on the new setting (`put_member_waits_for_a_concurrent_join_role_change`), and that a member can make a guest a member but not an admin. Unit tests cover the name limit and the workflow owner rule (`owner_authority_denies_roles_that_cannot_write`). A relay test covers huddle joins with different roles in the huddle and its parent, and a commit after a demotion, a `join_role` change or a lost membership (`huddle_join_needs_a_write_role_in_the_huddle_and_its_parent`). Lock-order tests: `join_role_change_waits_for_a_roster_publication_on_an_ephemeral_channel` reproduced a real deadlock (`40P01`) against the old order. `f2d_join_lock_order_serializes_membership_and_settings_writers` checks that membership and settings writers wait for a join and then succeed. `removed_creator_cannot_rejoin_a_private_channel` checks that `put_member` refuses a removed creator on a private channel, and `removed_creator_join_racing_a_private_switch_is_refused` holds the membership lock so a removed creator's kind:9021 passes the open-channel check, then makes the channel private before the join continues. Each new test was run against the old code and failed. ## Rollout 1. **Before the deploy, count guest rows** (`SELECT count(*) FROM channel_members WHERE role = 'guest' AND removed_at IS NULL`). We expect none. Any guest that exists loses the ability to post, change the topic and add people when this ships. If some exist, decide first whether to make them members. 2. Deploy. Every channel stays `join_role = member`, so nothing else changes. 3. **Do not set any channel to `join_role = guest` until no old relay is running.** Old relays ignore the column, so they would let guests and non-members write in that channel. 4. Rerun the `buzz-admin` channel reconcile so open channels drop the wrong `closed` flag. ## Migration and rollback - Migration `0067` adds `channels.join_role TEXT NOT NULL DEFAULT 'member'` with a CHECK constraint. Postgres stores a constant default in table metadata, so existing rows are not rewritten and there is no backfill. The relay runs the migration at startup. - Existing channels keep the old `closed` flag until their next change, or until the reconcile in Rollout step 4. - **Before rolling back the relay**, set every announce channel back to `join_role = member`. An older relay ignores the column and does not stop guests from writing, so those channels would accept posts from their guests and give `member` to new joiners again. ## Not in this PR Client UI: a Follow button, hiding the composer for guests, and an admin toggle. Clients that ignore the new tag keep working. A guest who tries to post sees the `restricted` error. ## Stack 1. #8143 (this PR): announce channels 2. #8494: store the bot role only for keys with a recorded agent owner --------- Signed-off-by: Logan Johnson <loganj@squareup.com> Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> |
||
|
seiler
|
9017435270 |
Separate invite history repair from schema rollout
Signed-off-by: seiler <seiler@squareup.com> Co-authored-by: Codex <noreply@openai.com> |
||
|
Logan Johnson
|
4dea51f644 |
fix(relay): scope push matches with the read path's channel rule (#8507)
🤖 ## Summary This is a consistency and forward-compatibility fix. It does not change what users of current clients see. Mobile push notifications now decide which channels can notify a device with the same rule that reads use, with one exception that the protocol requires: system channels never push. Before this change, push and reads used two separate rules: - **Reads** (subscriptions, counts, search and the bridge) show only the original channel types (stream, forum, DM, workflow) unless the filter names the channel. - **Push** skipped only `system` channels. Today `system` is the only non-original type, so the two rules give the same result. But a channel type added later would be hidden from reads and would still send push notifications to every member. This PR removes that gap: push now calls the same function that reads use, so a future type gets the correct push behavior with no extra work. ## What changes - A channel of an original type pushes as before. Membership is still required. - A system channel never pushes, even when a push filter names it. NIP-CL says: "The relay MUST NOT send push notifications for system channels." This is the same as before. - A channel of a type added later pushes only to a push filter that names that channel with `#h` (the channel tag), as reads do. - A push filter that names one channel does not get pushes from other named-only channels. - Events that are not in a channel (for example gift wraps) match as before. Push filters cannot carry the `#t` or `#P` opt-in that reads accept, so for push, naming the channel means `#h` only. ## Impact on current clients: none - System channels still never push, and no current channel type is affected by the later-type rule. - The mobile app names channels by `#h` only for joined, unarchived DM channels (`mobile/lib/shared/push/push_subscription_provider.dart`). DMs are an original type. - Naming a channel cannot get around membership. A push filter that names a channel still gets nothing unless its owner is a member of that channel. - Database cost is unchanged: one per-batch query replaces another, on the same channel list, with no extra round trip. - The push docs (NIP-PL and the push suppression doc) do not mention channel types, so they need no update. ## Details - The push matcher loads the named-only channels in each batch and checks each lease filter with `filter_channel_scope`, the function REQ, COUNT, search and the bridge already use. Before that check, it rejects system channels outright. - This removes `system_channel_ids_among`, which push was the last caller of. ### Related issue Part of the system channels spec (§2, rule 5). Builds on #8181 and #8469. No duplicate PR found. ### Testing - A `match_job` unit test covers: unnamed filters on an original-type channel, on a channel-less event, on a system channel and on a later-type channel; a later-type channel named by `#h` (1 wake); a system channel named by `#h` (0 wakes); an unrelated `#h` (no widening); and non-members. Removing either the shared scope check or the system exception fails it. - Before this PR, no test covered the push channel filter, not even for `system`. --------- Signed-off-by: Logan Johnson <loganj@squareup.com> Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> |
||
|
seiler
|
ee0ecfa929 |
Classify invite upgrade fixture as migration schema
Signed-off-by: seiler <seiler@squareup.com> Co-authored-by: Codex <noreply@openai.com> |
||
|
seiler
|
0b18470638 |
Merge latest pinned main into invitation repair candidate
Signed-off-by: seiler <seiler@squareup.com> |
||
|
Ian Oberst
|
ca3be55018 |
fix(relay): advance the partition catch-all by default (#8464)
## Summary
The migrations leave `events` and `delivery_log` with a catch-all
partition that already covers every month from 2026-07. Only catch-all
advancement replaces it with monthly partitions. Advancement was off by
default, so any deployment that left out
`BUZZ_PARTITION_MANAGER_ADVANCE_ENABLED` routed every new row into the
catch-all. An operator then had to repair it by hand: drain the
catch-all, swap it for monthly partitions, and re-attach. This has
already happened in one staging environment.
- **Advancement on by default.**
`BUZZ_PARTITION_MANAGER_ADVANCE_ENABLED` now defaults to `true`, and an
explicit `false` still disables it.
`BUZZ_PARTITION_MANAGER_CREATE_ENABLED=false` remains the kill switch
for all automatic partition DDL.
- **Prompt retry while the current month routes to the catch-all.** If a
startup or periodic pass leaves the current month routed to an empty
catch-all that the maintenance planner would replace, the audit loop
retries on its existing backoff. That is 5 s, doubling to a 60 s cap,
for at most 8 retries (about 5 minutes). After that it returns to
`BUZZ_PARTITION_AUDIT_INTERVAL_SECS`. Before this change, a failed first
pass, such as a lock timeout during a rollout, waited the full 15
minutes.
- The routine monthly advance, where only the last lookahead month falls
into the catch-all, stays on the normal interval. Each attempt queues an
`ACCESS EXCLUSIVE` request on the parent, so it shouldn't be retried
without urgency.
- The budget resets once the catch-all no longer awaits advancement.
- A failed attempt keeps the last known state.
- Layouts the planner refuses, such as a catch-all that holds rows, need
an operator, so they are never retried early. Neither is a policy with
create or advance disabled.
- The check is `PartitionTableAudit::awaits_catch_all_advance(policy)`,
plus a `PartitionAudit` helper that applies it to every table. It is
derived from `plan_maintenance`, so it can't drift from the planner's
rules.
- **Fresh-install test.** A new `#[ignore = "requires Postgres"]` test
runs every migration in a dedicated database. It checks that both tables
start out waiting on the catch-all and that create-only maintenance does
not fix them. It then checks that the first pass with advancement
produces canonical monthlies, an empty `{table}_p_future`, trigger
parity and a non-degraded audit, and that a second pass is a no-op.
### Related issue
None found.
### Testing
**Automated:**
- `cargo test -p buzz-db --lib partition`, including `--ignored` against
local Postgres: 21 unit tests and 55 Postgres tests pass.
- `cargo test -p buzz-relay` for the config tests and the partition
schedule tests passes.
- `cargo clippy -p buzz-db -p buzz-relay --all-targets -- -D warnings`
and `cargo fmt --check` are clean.
- Mutation checks on `awaits_catch_all_advance`. Each fails the new unit
test:
- matching any window month instead of the current month;
- replacing the planner check with the policy flags alone;
- short-circuiting the current-month gate.
**Local relay** (`BUZZ_AUTO_MIGRATE=true`, no advance env var):
- **Fresh database:** startup committed `advanced` for both tables,
creating `p2026_07` through `p2027_04` plus `_p_future`.
- **Failed first pass:** I held `LOCK TABLE ONLY communities IN SHARE
ROW EXCLUSIVE MODE` for 4 s across startup. The startup pass failed with
`lock_timeout` at 17:07:20.8, and the first periodic retry committed
`advanced` for both tables at 17:07:29.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Signed-off-by: Ian Oberst <ioberst@block.xyz>
Co-authored-by: Ian Oberst <ioberst@block.xyz>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
Luke Tornquist
|
039e1c8d59 |
feat(relay): write the NIP-43 member list in the membership transaction (#8482)
## Summary The relay-signed NIP-43 member list (kind:13534) was published after each membership change had already committed. If that publish failed, or if a change never republished at all (startup `bootstrap_owner`), the list stayed wrong until the background reconcile sweep found and fixed it. #8467 moved that sweep off the startup path. This PR removes the reason the sweep exists. Now every `buzz-db` function that changes `relay_members` writes the new list **in the same transaction**, as its last statement before commit: - It takes the per-community list lock last, reads the members, signs the list, retires the old one and inserts the new one. - If the list write fails, the membership change rolls back and the caller gets an error. - A call that changes nothing (already a member, role mismatch, host exists, limit reached) writes no list. The functions return `MembershipWrite<T> { outcome, snapshot }`. Callers push `snapshot` to live subscribers after commit. That push is the only thing left outside the transaction; if it fails, live clients see the change on their next fetch, and the stored list is already correct. Write paths covered (RPR #2, every write path): `add_relay_member`, `claim_relay_membership`, `claim_relay_invite`, `remove_relay_member`, `remove_relay_member_if_role`, `update_relay_member_role`, `bootstrap_owner` / `provision_owner`, `transfer_ownership`, `backfill_from_allowlist`, and the owner insert in `create_community_with_owner[_and_nip_fi_issuers]`. Community deletion removes the members together with the community, so it writes no list. The relay keypair is passed in explicitly instead of being stored on `Db`, so the compiler finds every caller. ### Behavior changes - **Open relays get an accurate list too.** Provisioning and ownership transfer used to publish only when `require_relay_membership` was on. The write is now unconditional. - **Same-second changes can't reuse an id.** The list's `created_at` is one second past the newest earlier list for that community, including retired ones, or the current time if that is later. Before, adding and then removing a member in the same second could produce an event with the same id as an earlier list. The insert then failed, and the stale list stayed live. A burst of changes can push `created_at` ahead of the clock by up to one second per change; it returns to the clock once the burst ends. - **`buzz-admin add-member` / `remove-member`** no longer build their own list (`publish_membership_list_with_bump` is deleted). They push the list the database wrote. Concurrent CLI runs now serialize on the database lock rather than depending on `run.sh` running one at a time. - **Owner mutations lock the community row `FOR NO KEY UPDATE`.** `transfer_ownership` and `bootstrap_owner` used `FOR UPDATE`. The list insert takes `FOR KEY SHARE` on that row through `events.community_id`'s foreign key, so a repair holding the list lock, or a removal or role update holding a member row, deadlocked with a concurrent owner change. `FOR NO KEY UPDATE` still conflicts with archive, unarchive and deletion admission, so the lifecycle ordering is unchanged. - **Startup delivers its list to other replicas.** When backfill or `bootstrap_owner` changes members at startup, the relay fans the newest list out through pub/sub once its state is ready, so clients connected to other replicas during a rolling deploy see an owner rotation without a refetch. - **New `buzz-admin republish-members`.** Rewrites the community's list from the stored members in one community-write transaction and pushes it live. This replaces the sweep as the repair for a list that is missing or stale: a `relay_members` change made directly in SQL, a restore, or a deployment that turns on `BUZZ_REQUIRE_RELAY_MEMBERSHIP` after running without it. ## Rollout Nothing rewrites a list automatically once the sweep is gone. A community whose list drifted before this deploys stays wrong until its next membership change or until an operator runs `buzz-admin republish-members`. Either deploy #8467 and let its sweep complete one pass first, or run `republish-members` for each community after this deploys. The command repairs only the community that `RELAY_URL` maps to, so on a relay hosting many communities, run it once per community host. ## What got simpler Deleted: - `reconcile_nip43_membership_snapshots`, `spawn_nip43_membership_reconciler`, its leader lock key and #8467's standby retry - the three `nip43_membership_snapshot_needs_reconciliation*` drift reads (one already deprecated) - `publish_nip43_membership_list` / `publish_nip43_membership_locked`, the separate after-commit republish at every call site, and `buzz-admin`'s duplicate publisher - the reconcile metrics `buzz_nip43_membership_reconciliations_total` and `buzz_nip43_membership_reconciliation_failures_total` - the after-commit publish metrics `buzz_nip43_membership_publications_total` and `buzz_nip43_membership_publication_seconds`, and the `publish_nip43_membership_locked` transaction metric label. A failed list write is now a failed membership write and shows up in the caller's existing error handling and the transaction metrics of its write path. - the `BUZZ_NIP43_RECONCILE_INTERVAL_SECS` setting There is now one way the list gets written, and it can't disagree with `relay_members` after commit. **Unused deploy config (outside this repo):** hosted deployments that set `BUZZ_NIP43_RECONCILE_INTERVAL_SECS` can drop it. It is now ignored. Dashboards or alerts on the four deleted metrics will stop receiving data. ## Testing New Postgres tests (`buzz-db`): - `relay_members::every_membership_write_commits_its_list`: for each write path, the returned list is the single live kind:13534 and names exactly the committed members. No-op calls return no list and leave the live list alone. - `relay_members::a_failed_list_write_rolls_back_the_membership_change`: holds the list lock, waits for the join to block on it, cancels that statement, and checks that the member was not added and the previous list is still live. - `relay_members::concurrent_joins_all_land_in_the_list`: 8 concurrent joins; the final list names all 8. - `relay_members::same_second_return_to_an_earlier_membership_writes_a_new_list`: add, remove, add, remove within a second; every list is newer than the last and no id repeats. - `relay_members::owner_transfer_and_list_repair_both_commit` and `relay_members::owner_transfer_and_member_removal_both_commit`: hold a repair (list lock) or removal (member row) open, wait until a transfer blocks behind it, then finish it. Both transactions commit; with `FOR UPDATE` back in place, one of them fails. - `community::create_community_with_owner_is_atomic_and_create_only` and `relay_invite::bounded_claim_exhausts_and_existing_member_retry_does_not_consume` now also check the list. Each new test was checked to fail against a mutation of the new code that brings back the old behavior (details in the thread). Stacked on #8467. Merge order: #8467, then #8468, then this. --------- Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> |
||
|
Luke Tornquist
|
c99872f749 |
fix(relay): delete the startup NIP-33 d_tag backfill (#8468)
## Summary
Delete the NIP-33 `d_tag` backfill that ran on every relay boot.
On every boot, before the listener opened, the relay ran:
```sql
SELECT DISTINCT community_id FROM events
WHERE kind BETWEEN 30000 AND 39999 AND d_tag IS NULL
```
No index covers `d_tag IS NULL`. The only `d_tag` index is the partial
`idx_events_parameterized`, which is `WHERE d_tag IS NOT NULL`. So this
query scanned every partition of `events`. In production it averaged
about 11s per boot, max 19s
(`buzz_startup_phase_seconds{phase="d_tag_backfill"}`), and found
nothing.
These rows shouldn't exist anymore:
- The backfill was the one-time migration for #246, the change that
added `d_tag`.
- In `crates/buzz-db/src`, every production write path sets the column
for kinds 30000–39999 through `extract_d_tag`: the main insert,
`replaceable.rs`, `channel_members.rs`, `relay_members.rs`, and
`push.rs`. A new source test,
`every_production_event_insert_names_d_tag`, now enforces that.
- The `events` inserts that omit `d_tag` are all test fixtures.
`scripts/backfill-d-tag.sql` is still there as the manual fallback for
any database that predates #246 and somehow never booted a relay since.
### What got simpler
- Removed `Db::backfill_d_tags` and its admission test.
- Removed the source-shape assertion in `observability_source.rs`.
- Removed the `d_tag_backfill` startup phase.
- `StartupStep::ALL` is now a slice, so removing a step no longer means
editing a hand-kept array length.
### Notes
- The `buzz_startup_phase_seconds{phase="d_tag_backfill"}` series stops
reporting.
- **Rebased onto main after #8467 merged.** Both remove a startup phase.
With the fixed-length `ALL` array the two merged cleanly in git into a
17-entry `[Self; 18]`, which doesn't compile. This PR makes `ALL` a
slice.
- **Before merging**, run this read-only query against production and
confirm it returns `0`: `SELECT count(*) FROM events WHERE kind BETWEEN
30000 AND 39999 AND d_tag IS NULL`. It scans every partition, so use a
read replica. If it isn't `0`, run `scripts/backfill-d-tag.sql` first.
The cutover script copies legacy `d_tag` values as-is, so only this
query proves historical data is clean.
- **Self-hosted installs** should run the same check before upgrading.
### Related issue
None found. Companion to #8467, which also targets relay startup time.
### Testing
- New `every_production_event_insert_names_d_tag`
(`crates/buzz-db/tests/observability_source.rs`): every production
`INSERT INTO events` must name `d_tag` in its column list. Dropping
`d_tag` from `push.rs` makes it fail and name the function.
- Runtime E2E against a real relay: boot has no `d_tag_backfill` phase;
kind:30023 replacement via WebSocket and HTTP keeps only the newer
event, an unrelated `d` survives, and an older replay can't win
(confirmed in SQL).
- Stacked on #8467 at `501dfd83`: a fresh two-replica boot runs all
enabled startup phases, with neither `nip43_reconcile` nor
`d_tag_backfill` present. Both replicas pass readiness and return the
same signed message over authenticated HTTP and WebSocket, and outsiders
are denied.
- Blox, at `501dfd83`: `just fmt-check clippy security-review-check
file-size-check` passes. Every `test-unit` step and `just test` pass
except the `buzz-acp` `git_bootstrap` native-git tests, which fail the
same way at base `b6aa5539` on Blox, and one `buzz-agent` catalog timing
test that fails only under full-suite load and passes when run alone.
- `e27139de` adds only a doc comment on
`every_production_event_insert_names_d_tag`, saying which SQL shapes the
text match covers. No code changed, so the results above still apply.
---------
Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
|
||
|
Logan Johnson
|
25bcb791b4 |
fix(relay): show only legacy channel types to reads that name no channel (#8469)
🤖 ## Summary Every Buzz channel has a **type**, such as an ordinary chat channel (`stream`), a forum, a DM or a workflow. A client app can only show the types it was built to understand. This PR makes the relay (the Buzz server) follow one rule everywhere: **a client sees a channel only if it knows that channel's type.** #8406 applied this rule to membership notices. This PR applies it to the reads that list channels without naming one: the channel list, "channels I belong to", the sidebar's unread list and artifact lists. Before, these reads left out one type by name: system channels. Now they return only the four types that every client has always known: `stream`, `forum`, `dm` and `workflow`. Today the result is the same for every user, because `system` is the only other type. ### Backward compatibility: old clients stay safe - A client that was released before a new channel type exists will never receive channels of that type in its lists. It cannot show such a channel correctly, so it does not get one. - Requests that old clients send today return exactly what they returned before. Nothing changes for them now or when a type is added later. ### Forward compatibility: new types can be added safely - Before, the rule was a deny list: "everything except `system`". Each new type would appear in every deployed client by default, unless someone remembered to update five separate checks in the relay. A missed check would show the new type to clients that cannot handle it. - Now the rule is an allow list that is frozen: the four original types. A new type is hidden by default, with no relay change needed. Clients that understand it can request it on purpose, the same way current clients request system channels today. Each new type gets its own opt-in, so adding a type cannot break clients that are already in use. ## Details - One frozen list, `buzz_core::channel::LEGACY_CHANNEL_TYPES`. Membership notices (#8406) now use the same constant, so channel reads and notices cannot drift apart. The duplicate `LEGACY_NOTICE_CHANNEL_TYPES` is removed. - One SQL helper, `legacy_channel_type_sql`, builds `channel_type IN ('stream', 'forum', 'dm', 'workflow')` from that list. It replaces the five `<> 'system'` checks: - `get_accessible_channel_ids` (both halves): the implicit channel scope of any filter without `#h`, for example kind 39000 channel metadata, REQ, COUNT and search; - `get_accessible_channels`: the channel list; - the personal-read sidebar roster; - artifact queries. - A filter that names a channel (`#h`, or `#d` on kinds 39000–39003) reads it under the ordinary rules, whatever its type: the reader is a member, or the channel is open. The uncached named-channel lookup, `get_readable_named_only_channels`, now returns every readable non-legacy channel with its type, not only system channels. Before, a non-member could not read an open later-type channel, and `#d` read nothing. Workflow run reads and thread windows use the same lookup. - The relay keeps each named channel in the scope of the filter that names it only. This also holds when it re-checks membership for a named channel that the cache lacks. - The opt-in to system channels (`#t:["system"]` or `#P` on kinds 39000–39003) adds readable **system** channels only. It never adds a channel of another type, even one that a sibling filter in the same request names. The relay scope type `NamedOnlyChannels` keeps the type with each channel, so the opt-in can tell them apart. - NIP-CL and NOSTR.md state the rule. ### Related issue Follows #8406 (the same rule for membership notices). ### Testing New Postgres tests use a scratch database with a `future_type` value added to `channel_type`, as a later migration would add one: - `reads_that_name_no_channel_leave_out_a_type_added_later`: the channel scope, the channel list and the sidebar roster leave out the later type and keep the legacy types. - `artifacts_of_a_later_channel_type_are_read_only_by_name`: artifact current and history queries and counts without `#h` leave out the later type. With `#h` naming the channel, they return it. - `channels_of_a_later_type_are_read_only_by_name`: production read paths (WS REQ, WS COUNT, HTTP `/query` and `/count`). A member reads a private later-type channel and anyone reads an open one, by `#h` and by `#d`; an outsider cannot read the private one; a read that names no channel returns neither. In one request, a filter that names the channel plus a `#P` or `#t:["system", <label>]` sibling: the sibling does not get it. - `a_named_channel_of_a_later_type_stays_in_its_named_filter`: the membership re-check for a channel joined mid-request, with an unnamed sibling and a `#P` sibling. Only the naming filter gets the channel. Each test fails when its predicate goes back to `<> 'system'`, when the named lookup loads only `system`, or when the opt-in adds every named-only channel. The #8406 notice tests and the system channel Postgres tests pass on this branch. --------- Signed-off-by: Logan Johnson <loganj@squareup.com> Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> |
||
|
Luke Tornquist
|
e6c56ee5ee |
fix(relay): take NIP-43 snapshot reconcile off the startup path (#8467)
## Summary
Before the relay opened its listener, it reconciled the NIP-43
membership snapshot for every active community. Each community costs two
writer round trips, one after the other. On a production deployment with
about 80k communities, this one step averaged 174s of a roughly 180s
boot (max 324s). Pods in an availability zone farther from the writer
took 3–4× longer than pods in the writer's zone.
The step doesn't need to block startup. The snapshots live in the shared
database, and the replicas that are already running serve the same
snapshots. A new replica has nothing of its own to repair before it
serves. The sweep before bind did the same work as the background
maintenance sweep; only the metric labels differed.
This PR:
- Removes the NIP-43 sweep that ran before bind.
- Moves the background loop into
`side_effects::spawn_nip43_membership_reconciler`. Its first sweep now
runs as soon as the task starts. Before, the loop skipped its first
tick, so the first maintenance sweep waited a full
`BUZZ_NIP43_RECONCILE_INTERVAL_SECS`.
- Runs that loop on one replica only. The replica holding the
`NIP43_RECONCILE_LOCK_KEY` session advisory lock sweeps; the others
retry the lock every `NIP43_STANDBY_RETRY` (5s, capped at the interval).
This reuses the usage-metrics leader lease (a detached Postgres
session), renamed from `UsageMetricsLeader`/`try_lock_usage_metrics` to
the generic `SessionLeader`/`try_lock_session_leader`.
- Waits a full interval after each sweep (sleep after sweep), so a sweep
longer than the interval never runs straight into the next one.
- `reconcile_nip43_membership_snapshots` is now the only entry point and
keeps its original signature (`-> Result<usize>`). It's no longer
deprecated.
### What got simpler
Deleted:
- `Nip43ReconciliationPurpose` (the `Bootstrap`/`Maintenance` purposes)
and `reconcile_nip43_membership_snapshots_with_purpose`
- `Db::bootstrap_community_hosts` and
`Db::nip43_membership_snapshot_needs_reconciliation_for_bootstrap`
- The startup progress log (`NIP-43 startup reconciliation progress`,
`nip43_progress_due`) and its test
- The `nip43_reconcile` startup phase: `StartupStep::ALL` goes from 19
to 18 steps
The leader lock reuses the existing usage-metrics lease rather than
adding a second leader primitive. `StorageAccountingLeader` is still a
near-duplicate of it; folding that in is left as identified debt.
### Trade-offs
- **Repair before serving.** A newly started replica no longer
guarantees it has repaired stale snapshots before serving. The running
fleet doesn't guarantee that today either: the background sweep is what
repairs drift, and the leader starts it as soon as it takes the lock.
- **Rollout load.** Before, every replica swept before binding, and
every replica also ran the background loop. Now there is one sweeper in
steady state, so writer load from this job drops from one sweep per
replica to one per deployment, and rollouts no longer stack boot sweeps.
Standbys add one lock attempt every 5s each.
- **Leader handoff.** When the leader's process exits (including
SIGKILL), its connection closes, Postgres releases the session lock, and
a standby retries the lock at least every `NIP43_STANDBY_RETRY` (5s) and
sweeps as soon as it gets it. Each attempt also waits for a writer
connection, so under database pressure takeover can take longer than 5s.
Before this, a standby waited a full interval (3600s in production), so
after a rollout replaced the leader nothing swept for up to an hour. If
the leader is lost with its host or network instead, the lock is held
until the database server's TCP keepalive drops the session, and nobody
sweeps meanwhile. The usage-metrics leader already has the same
exposure. The lease is checked only between sweeps, so a leader that
loses its session mid-sweep finishes that sweep while the new leader
starts one; the per-community publication lock keeps that overlap safe.
- **Snapshot window.** A missing or stale kind:13534 snapshot is served
until the leader's sweep reaches that community; before, a booting
replica blocked until it was repaired. Admission reads canonical
`relay_members`, so this window doesn't change who's admitted. It does
affect clients that read the snapshot: desktop onboarding shows the
membership-denied page to a member missing from it, the desktop/mobile
member lists and join alerts lag, and desktop mesh discovery keeps its
existing allowlist when the snapshot is missing. In production the
inline publications that keep it current have not failed since
2026-10-07, and the sweep repaired no snapshots in the last 30 days.
- **Dashboards and alerts.** The
`buzz_startup_phase_seconds{phase="nip43_reconcile"}` series stops
reporting. Anything charting or alerting on it should move to the
sweep's existing counters
(`buzz_nip43_membership_reconciliations_total`,
`buzz_nip43_membership_reconciliation_failures_total`).
### Related issue
None found.
### Merge order
#8468 is stacked on this PR. Both remove a startup phase, and with the
old fixed-length `StartupStep::ALL` array, merging both would have built
a 17-entry `[Self; 18]` that doesn't compile. Merge this PR first; #8468
makes `ALL` a slice.
### Testing
- `background_reconciler_sweeps_immediately` (private scratch database):
spawns the production reconciler with a 24h interval and 24h standby
retry, and requires the first sweep to publish a missing snapshot. Fails
if the skipped first tick is restored.
- `background_reconciler_standby_takes_over_on_short_retry` (private
scratch database): with a 24h interval and 100ms standby retry, nothing
publishes while another session holds the leader lock; once it's
released the standby must publish within 10s. Fails if the standby
sleeps the full interval again (checked by reverting that line).
- `test_usage_metrics_lock_has_single_owner_and_releases_on_drop` covers
the renamed lease.
- `maintenance_sweep_skips_archived_and_deleted_communities` now calls
the single entry point.
- Runtime E2E at `574b28b3`: two real relays sharing one database, 3600s
sweep interval, production 5s standby retry. While the leader was alive,
the standby made only lock attempts (no sweep or snapshot-comparison
reads). SIGKILL of the actual lock holder, 3/3 trials: the survivor took
the lock in 4.81–4.86s and repaired the snapshot within 10ms after that.
Both replicas served authenticated reads while the initial publication
was stalled.
- Blox, at `574b28b3`: `just fmt-check clippy security-review-check
file-size-check` passes. Every `test-unit` step and `just test` (118
suites) passes except the `buzz-acp` `git_bootstrap` native-git tests,
which fail the same way at base `b6aa5539` on Blox (environment, not
this change).
---------
Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
|
||
|
Wes
|
6e6173acd5 |
test(buzz-db): fix two PostgreSQL-lane timing flakes (#8472)
## Summary Two timing flakes in the PostgreSQL lane, one fix each. ### Deletion lock order `abort_serializes_before_fence_without_deadlock` failed on main ([run 39330176879](https://github.com/block/buzz/actions/runs/39330176879)): `post-lock lease verification rejects aborted request: 1`. The test slept 50ms and assumed `abort` had queued on the community advisory lock by then. On a slow runner, `fence` queued first, won the lock, and succeeded. The test now waits until `pg_locks` shows each contender waiting on that community's lock, `abort` first and then `fence`. Postgres grants queued advisory locks in arrival order, so `abort` always wins. The sleeps and their timing guess are gone. ### Saturated-reader fixture `routed_fallback_spends_one_acquire_budget_when_aurora_cache_is_cold` failed this PR's first run ([run 39335038111](https://github.com/block/buzz/actions/runs/39335038111)): `establish the reader's sole connection: PoolTimedOut`. Setup opens the reader's only connection through the production lazy pool, which has a 150ms acquire timeout. On a loaded runner that first connect took longer, so the test failed before reaching the budget it measures. Setup now retries on `PoolTimedOut` within 5s. The pool and its 150ms budget are unchanged for the routed read under test. Test-only. Production code is unchanged. ## Validation - Deletion test against an isolated local Postgres 17 database: 40/40 passes. Saturated-reader test: 15/15. - `cargo fmt --check`, `cargo clippy -p buzz-db --tests -D warnings`, pre-push Rust unit tests: pass. - Three other `store::deletion::postgres_tests` fail locally on unmodified `main` as well (local environment). CI's PostgreSQL lane is the authoritative check for them. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Wes <wesbillman@users.noreply.github.com> Co-authored-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
seiler
|
28fdf29d61 |
Merge pinned main into invite revocation candidate
Signed-off-by: seiler <seiler@squareup.com> |
||
|
Wes
|
b6aa5539e3 |
feat(mobile): account onboarding, key recovery and hosted reconnect (#8375)
## Behavior Adds optional mobile Builderlab sign-in, identity backup/recovery, community discovery/creation, and account-aware reconnect. **Builderlab features are disabled by default:** when `BUZZ_AUTH_DOMAINS` is unset or empty, account login, creation, backup/recovery, reconnect prompts, and background account restoration are hidden/disabled. QR/code pairing, links, and invites retain Nostr-only entry. Saved account state is retained for later opt-in. `--dart-define=BUZZ_AUTH_DOMAINS=communities.buzz.xyz,communities.public.builderlabstaging.xyz` opts those DNS domains and their subdomains into required account setup. The same policy gates pairing, deep links, durable community admission, and reconnect. A nonempty list enables Builderlab features; `BUZZ_BUILDERLAB_URL` alone does not. Enabled flows still require their backend services. Existing Nostr relay authentication remains mandatory. `BUZZ_HOSTED_RELAY_AUTH=true` enables scoped relay assertions and reconnect prompts only for configured authentication domains. Configure assertion issuance and each community's issuer binding before enabling assertions; enable relay enforcement last. Account credentials stay at Builderlab. Wrong-account recovery preserves keys; failed writes are not replayed; authentication is bounded by request deadlines. Hosted flows preserve keys across cancellation/restart, reconcile interrupted creation/invitation acceptance, and support manual nsec file import/export. Account identities are scoped to backend and authenticated subject; reusing an existing local key for an unbound account requires an explicit selection. Recovery reconciles the advertised and recovered inventories before automatic selection. Successful self-hosted admission retires its pending key, and iOS startup removes interrupted export files. Adds authenticated relay `GET /operator/communities/joined` for the companion Builderlab membership endpoint. ## Validation - Follow-up review repair at `66a12ac92`: self-hosted entry presents readable choices and import despite incomplete enumeration, while generation still requires a complete empty inventory; explicit selection uses exact-key lookup. Consuming a value-equal duplicate now advances dispatch. Reconnect requires an existing account binding or a subject-scoped backup receipt before signing an assertion, and pins the subject inside the serialized account request. An empty different account is refused without sending key proof. Focused checks: configured domains 58 passed; default 49 passed, 9 hosted-only skips. Separate isolated mutations fail the incomplete-inventory sheet, exact-key reuse, duplicate dispatch and reconnect journeys (including an empty-account-only bypass); restored controls pass. Evidence uses synthetic storage/network boundaries, not physical Android or live backend association. All mandatory commit/push hooks passed; fresh GitHub CI is pending. - Bounded review repair at `d3cc0003f`: incomplete secure-storage enumeration now blocks automatic identity generation while explicit known-key reads retain their fallback; declined/failed onboarding links release the queue; HTTP 429/5xx keeps Retry regardless of response encoding. Default app-gate coverage exercises a bind link followed by ordinary community entry. Focused checks: configured domains 47 passed; default 38 passed, 9 hosted-only skips. Removing the three production fixes in an isolated source/Git/build copy fails the identity-resolution, declined/failed queue, and Retry journey assertions; clean and restored controls pass. Storage and HTTP failures are synthetic; physical Android and live outage behavior were not exercised. Published after all mandatory commit/push hooks passed (mobile analysis/tests, branch-skew and file-size gates); fresh GitHub CI remains pending. - Existing whole-PR source review cleared the reconnect defects at `fb15d5b82`. - Domain-policy increment: focused environment, entry, app-gate, pairing and onboarding journeys pass with empty and configured domains. Existing pairing provider/page tests pass. Changed Dart files analyze cleanly. - Isolated source/Git/build mutation: forcing paired credentials through hosted login fails the default-build journey; clean and restored controls pass. CI now runs configured-domain journeys in addition to the default-build suite. - Integrated main through `5b433d929` at `0067c069c`, including its pinned Flutter upgrade. Preserved the PR welcome UI with the updated transition API and added Material surfaces around onboarding selection/consent rows. Focused post-merge runs pass in both configurations; all mandatory local push gates passed (mobile analysis/tests, Rust tests, desktop-native checks, branch-skew and file-size checks). - Merge-review repair at `a315bce11`: focused identity/storage/admission and widget journeys pass, including account switching with an empty or already-bound second account, explicit reuse, and self-hosted removal/rejoin with another key. Default-domain journeys pass too (hosted-only cases intentionally skip there). - Isolated source/Git/build mutation checks: bypassing account scoping, reuse consent, pending-key retirement, or recovery inventory reconciliation fails the relevant journey/invariant assertions; clean and restored controls pass. - Native `RunnerTests/IdentityFilePluginTests`: both tests pass on the iPhone 18 Pro iOS 27 simulator with an isolated app identifier. At `24a0dcaac`, the picker journey seeds residue in Library/Caches, matching path_provider_foundation, and verifies plugin startup removes it while preserving an unrelated file. The earlier tmp-only fixture did not exercise the actual export location. This is synthetic native cleanup/picker evidence, not a real terminated-share or live account round trip. - Follow-up: Caches and tmp cleanup; fallback to individually known identity slots when Android bulk enumeration fails; explicit replacement of a refused self-hosted key preserves its original before swapping the retry slot. Configured-domain journeys/invariants: 34 passed; default: 31 passed, 3 hosted-only skips. Isolated enumeration/retry mutations fail with passing controls. Bulk-read fallback preserves but cannot enumerate arbitrary other-account slots until bulk reads recover. - Configured-domain CI exposed a stale pairing-status assertion after account scoping. The repaired test checks the admission mismatch, preserved account/transfer keys, and no admitted community. The configured-domain CI selection passes locally (56 tests); removing the production mismatch guard fails the updated test, with clean/restored controls passing. This is synthetic coordinator evidence, not physical QR acceptance. - Final review repair (`40e1e6101`): native iOS export creation uses atomic complete file protection; normal and restart cleanup remain. Account/policy/claim HTTP uses one 30-second total deadline with request abort and body cancellation. Removed the unrelated NIP-FA spec edit. - Focused account/admission/export tests: 25 pass; configured-domain account/admission/onboarding journeys: 33 pass. Isolated deadline and native-dispatch mutations fail, with passing clean/restored controls. Native export/picker tests: 2 pass on iOS 27 simulator. The real file-protection attribute assertion is physical-device-only because simulator provides no Data Protection attribute; locked-device protection and interrupted real sharing have not been observed. - Opt-in UI: configured-domain CI selection passes (57 tests); default selection passes (7 tests, 10 hosted-only skips); URL-only plus whitespace/empty domains passes (6 tests, 9 hosted-only skips). Changed Dart files analyze cleanly. Independent mutations that unconditionally initialize the coordinator in the app gate, welcome entry, or link form fail the default journey; clean/restored controls pass. - Native welcome screens exercised on iPhone 18 Pro / iOS 27 simulator in default and configured builds: default exposes community link / QR / pairing code only; configured build additionally exposes Builderlab connection and community creation. No live hosted login was exercised in this check. - Delivery at `e48fe73a9`: all mandatory push hooks passed (full mobile analysis/tests, Rust tests, desktop-native checks, branch and file-size checks). GitHub checks had not yet populated at the immediate post-push snapshot. ## Review disposition | Feedback | Disposition | | --- | --- | | Explicit self-hosted entry under unreadable inventory, duplicate invite dispatch, reconnect cross-account binding ([entry](https://github.com/block/buzz/pull/8375#discussion_r4318421622), [queue](https://github.com/block/buzz/pull/8375#discussion_r4318421640), [reconnect](https://github.com/block/buzz/pull/8375#discussion_r4318455512)) | Repaired at `66a12ac92`; evidence above. Reconnect refuses unproven ownership rather than silently establishing a binding. | | Incomplete Android identity inventory, stranded deep-link queue, transient HTTP retry classification ([review](https://github.com/block/buzz/pull/8375#pullrequestreview-5576864143)) | All three repaired in `d3cc0003f`; focused journeys and isolated mutation evidence above. | | Account cross-link, incomplete recovery inventory, committed invite checkpoint, pending local key retirement/replacement | Repaired in the preceding commits; journey and mutation evidence above. | | iOS interrupted key export ([review](https://github.com/block/buzz/pull/8375#pullrequestreview-5575253252)) | Native cleanup plus complete protection at creation implemented. Physical-device lock and real interrupted sharing acceptance remains pending. | | Trickle responses bypass idle timeout; unrelated NIP-FA edit ([review](https://github.com/block/buzz/pull/8375#pullrequestreview-5575764556)) | Total request deadline/abort implemented; spec edit removed. | | Production domains rejected | Ordinary entry now needs no Builderlab configuration. Opt-in account-required domains must still use supported community mappings. | | Lost Android Keystore recovery | Backup exclusions prevent the reviewed stale-restore scenario; genuine key loss still has no in-app recovery/reset and remains deferred. | | Native secp256k1 dependency question | Obsolete against the current base: upstream C integration landed in #8398; no package diff remains here. | | Operator membership endpoint contract | Documentation now states the operator chooses the pubkey; no end-user session binding is implied. Additional handler-level coverage is optional follow-up. | | Optional Android partial document cleanup/activity retention; local work after backup cancellation | Follow-up hardening, not claimed implemented. Cancellation prevents remote backup upload. | | Semgrep crypto inventory notices | Informational inventory, not demonstrated defects. | ## Remaining draft gates Native QR/browser-return acceptance remains unverified for this revision. Optional Builderlab onboarding still needs backend backup-status and joined-community routes, recovery policy/configuration, and real backup/recovery/reconnect acceptance. The empty-domain build removes these services as prerequisites for ordinary pairing; it does not make the optional hosted flows backend-ready. Human acceptance and current CI are still required before merge. Review dispositions retained: production auth-domain configuration must use supported community mappings; enabling `builderlab.xyz` does not add support for `buzz.block.builderlab.xyz`. Recovery after genuinely losing an Android Keystore key remains a follow-up, not a newly implemented capability. Human acceptance steps: build once without `BUZZ_AUTH_DOMAINS` (including with a previously saved account) and confirm no Builderlab controls/restoration while ordinary link and QR/code entry work. Build with supported domains and confirm connection/creation appear; exercise browser return and live backup/recovery/reconnect. On physical iOS, exercise export and interrupted sharing/locked-device protection. Record results before marking review complete. --------- Signed-off-by: Wes <wesbillman@users.noreply.github.com> Co-authored-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz> Co-authored-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@wes.communities.public.builderlabstaging.xyz> |
||
|
Logan Johnson
|
cc6336d985 |
feat(relay): tag every membership notice with its channel type (#8406)
🤖
## Summary
- When someone is added to or removed from a system channel, the relay
now sends them the usual "you were added / removed" notice. Before, it
sent nothing, so a client could learn about a new system channel only by
rereading every channel.
- **A client sees a notice only if it knows the channel's type.** Every
membership notice now carries its channel type. Clients that do not send
`#t` (every client today) see only the types that existed before system
channels, so they see no change. A type that Buzz adds later is hidden
from them by default.
- The notices are stored like every other membership notice, so a client
that was offline can catch up with `since` instead of rereading all its
channels.
## Details
- Every kind:44100 (added) and kind:44101 (removed) notice now carries
`["t", <channel_type>]`, for example `["t","stream"]` or
`["t","system"]`.
- **One place reads the channel type:** `emit_membership_notification`
in `side_effects.rs`, where the relay signs the notice. A channel's type
is set when it is created and no code path changes it, so the tag stays
correct. If the channel row is missing, the relay sends no notice rather
than an untagged one.
- **The rule (NIP-CL):**
- a filter with `#t` matches a notice only when the notice's type is one
of its `#t` values;
- a filter without `#t` matches only the **legacy types**: `stream`,
`forum`, `dm`, `workflow`. This list is frozen
(`LEGACY_NOTICE_CHANNEL_TYPES`), so a new type never joins it;
- a notice without a `t` tag was stored before this change. It is always
a legacy type, so the relay treats it as one, and replay across the
upgrade stays complete.
- **Fails closed.** The rule lives in two shared places, and both
default to the legacy types. A read path that is not updated behaves as
it does today:
- `filters_match` / `notice_visible` (buzz-core). Live fan-out, the REQ
history post-filter, COUNT, the HTTP bridge and push all match through
it.
- `EventQuery::notice_channel_types` (buzz-db). Its SQL keeps only
visible notices before `LIMIT`, so page limits and COUNT stay exact.
`None` means legacy types. Only `filter_to_query_params` sets it, from
the filter's `#t`.
- A client that knows every current type sends one filter:
`{"kinds":[44100,44101],"#p":[<me>],"#t":["stream","forum","dm","workflow","system"]}`.
NIP-CL and NOSTR.md describe this.
- **Only the target receives a notice.** A notice names the target, the
channel and who made the change, and the relay signs it, so knowing its
id is not permission to read it. 44100/44101 are now in
`RESULT_GATED_KINDS`, the one list used by the `ids` exemption check,
the per-result reader check (WS and HTTP history, push), live fan-out
(same pod and Redis) and COUNT. This also closes the same id lookup for
ordinary channels' notices.
- No migration: 44100/44101 are already excluded from search indexing.
## Testing
- `buzz-core`: a table test covers untagged, `stream`, `system` and a
future type against filters without `#t`, with `#t` for one type, and
with `#t` for every type. Other kinds keep the plain `#t` match.
- Postgres tests (`system_channel_postgres_tests.rs`): live fan-out, REQ
history, COUNT and the HTTP bridge, for a filter without `#t`, a
`#t:["system"]` filter and an all-types filter; an untagged legacy
notice; a non-target with an `ids` filter on each path; and a raw
`EventQuery` that does not set the field.
- I broke each part on purpose and the tests failed: no type tag on the
notice, no rule in `filters_match`, no SQL rule, and no untagged clause
in the SQL.
- End to end: I built a relay container from
|
||
|
Will Pfleger
|
5b433d9292 |
feat(relay): resolve NIP-FI issuer bindings from the database (#8377)
Stack: #8372 (merged) → this PR → #8399 → #8401 Makes relay login checks read the per-community NIP-FI issuer bindings that #8372 stores. Without this, the relay can't keep up with bb-public, where new communities are created at every signup and a static `BUZZ_NIP_FI_COMMUNITIES` list won't do. **Source switch.** `BUZZ_NIP_FI_COMMUNITY_SOURCE=database` resolves each community's trusted issuers from `community_nip_fi_issuers`, filtered by the issuer registry, instead of the static list. Each community gets its own policy ID: the base ID no longer hashes every community audience, and the final ID hashes the base plus this community's audience, so policy IDs change once (see `docs/operator-nip-fi-bindings.md`). `Config::from_env` refuses to start, in every mode, on an unknown source, on `database` together with `BUZZ_NIP_FI_COMMUNITIES`, and on `database` without `BUZZ_NIP_FI_PROVISION_BINDING`. `BUZZ_NIP_FI_RESOLVER_*` and `BUZZ_NIP_FI_SNAPSHOT_*` are validated in every mode, so a bad value fails the deploy that sets it. In `off` mode no resolver is built and no snapshot is loaded. **Resolver (`nip_fi_resolver.rs`).** - **Snapshot:** each pod loads every binding into memory, capped at `BUZZ_NIP_FI_SNAPSHOT_MAX_ROWS` (going over is a failed load). It refreshes every 30 s, start to start, with jitter that only shortens the delay and backoff after failures. Each attempt is limited by `BUZZ_NIP_FI_SNAPSHOT_REFRESH_TIMEOUT_MS`, which must be shorter than the interval. A refresh whose deadline passes before it publishes publishes nothing. The old snapshot is dropped off the runtime. - **Freshness:** a host in a snapshot loaded under two minutes ago is answered from memory. Any other host, and every host once the snapshot is stale, goes to a live lookup. Every answer carries a deadline two minutes after the read behind it started, and WS, HTTP and audio recheck it just before admitting. - **Live lookups:** bounded and single-flight. In-flight lookups are capped by `BUZZ_NIP_FI_RESOLVER_MAX_INFLIGHT` (default 8, clamped below the database pool size), and the pending hosts and their waiters have fixed caps. Positive answers are used for that admission only. A "not found" is cached for 5 s, up to `BUZZ_NIP_FI_RESOLVER_NEGATIVE_CACHE_MAX` hosts, and only when no snapshot was published and no binding was created while it ran. A lookup that panics still leaves the pending map. - **Removal:** a removed binding stops admitting new connections within 30 s plus the refresh timeout (40 s by default) while refreshes succeed, and within two minutes of the last good load while they fail. Open WebSocket sessions are not rechecked when a binding is removed; they end at their own session deadline. **Shadow mode** resolves through the same path as enforce, including the database lookup and the freshness rules. It keeps the resolved binding's deadline and checks it at the same point as enforce; a binding that goes stale during the tenant lookup is recorded as `stage=deadline, outcome=unavailable`, without changing the legacy response or closing the socket. With `source=database` its `community` metric label is only `bound` or `unmapped`; with `source=env` it keeps the existing per-community audience label, so current shadow dashboards are unchanged. Each request resolves its Host once: the router guard's resolution, failures included, is reused by shadow recording and admission. **Provisioning.** Both operator provisioning paths drop this pod's cached "not found" for the new host after a commit that bound issuers, so the creator's first login looks it up. **Startup and readiness.** With `source=database` and a mode other than `off`, the first snapshot loads before any listener binds, under its own startup phase `nip_fi_snapshot_load`. A failed load exits non-zero, so a pod is never ready without a snapshot. **Operational consequence:** boot already fails when the database is unreachable; what is new is that a snapshot over `BUZZ_NIP_FI_SNAPSHOT_MAX_ROWS` or a load past its timeout also crash-loops new pods. After two minutes the old pods' snapshots go stale and most logins fall through to the 8-slot live lookup, which sheds the rest with 503. The way out is raising the cap or timeout, or `mode=off`. **Metrics.** `buzz_nip_fi_binding_snapshot_*` covers loads, failures, rows and freshness. `buzz_nip_fi_binding_snapshot_loaded_timestamp_seconds` is written on every refresh attempt, so alert on `time() - x > 120` together with `absent(x)`. `buzz_nip_fi_binding_snapshot_rows` against `buzz_nip_fi_binding_snapshot_max_rows` gives the headroom to the row cap. `buzz_nip_fi_resolver_*` covers live lookups and the negative cache. The env vars and alerts are documented in `.env.example`, the chart README and `docs/operator-nip-fi-bindings.md`, which also has the staged off → shadow → enforce rollout for `source=database` with go/no-go signals at each step, and the over-cap and load-timeout failure modes with their ways out. **Regression coverage.** Router-level tests run the database resolver through the real `build_router` for root WS, audio and protected HTTP, in enforce and shadow mode. Over PostgreSQL and Redis, two pods using the env community source (two exact bindings) replay assertions retained from before a disconnect through a real root NIP-42 session, the audio deny-set check and signed NIP-98 requests: they are denied on both pods, and a root session upgraded before the disconnect is refused by the post-registration deny-set check. Postgres tests in `buzz-db` check that archived and quiescing communities, and issuers missing from the registry, never reach the snapshot or the live lookup. Process-level `boot_lifecycle` tests spawn the real relay with `source=database`: an over-cap or timed-out startup load exits non-zero with no health listener bound, and a loadable snapshot lets readiness turn ready. --------- Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Co-authored-by: Hayt <211b96e6a2b7f45fd4047988976c7bbbeeda0c15f3ae7b32eec20834b5a55118@buzz.block.builderlab.xyz> Co-authored-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz> |
||
|
Bradley Axen
|
3ffbfa693c |
feat(relay): start a channel with its members in one create (#8421)
## What
A kind:9007 can now name the channel's first members with `p` tags.
Those members are added in the same database transaction that creates
the channel. Resending a create whose OK was lost is now accepted
instead of refused.
## Why
Starting a new Solo chat today takes about 6–8 sequential relay round
trips before the first message goes out: create, read, add each agent
(kind:9000), read the roster, then send. With members in the create, a
client can start a chat with its agents as two writes: the create, then
the message as soon as the create is OK.
The resend fix makes the create safe for an outbox to retry. Today an
identical resend after a lost OK is refused with `duplicate: channel
already exists`, so the client can't tell success from a real UUID
collision.
## How
- **buzz-db:** `Db::create_channel_with_event` writes the channel, its
owner, its members (role `member`, `invited_by` = creator) and the 9007
event itself in one transaction. A channel row therefore never exists
without its create event. `create_channel_with_id` is unchanged for its
other callers.
- **Validation** (`side_effects::initial_members` and the
`validate_admin_event` 9007 branch):
- Each `p` must be a 32-byte hex pubkey.
- Duplicates and the creator are dropped.
- At most 32 members.
- An `h` tag is required when there are members.
- Each member goes through the same `decide_channel_add_policy` check a
kind:9000 uses, so an agent that refuses third-party adds keeps the
whole create out.
- **Side effects:** each initial member gets the same `member_joined`
system message and member-added notification a kind:9000 would post. The
fallback in `handle_create_group` creates a bare channel when the
pre-created row is missing; it announces no members, because it added
none.
- **Resend:** when the UUID is already taken, the same transaction
checks whether this exact event is the stored create.
- If it is, the create is OK'd as `duplicate:` (it is the same signed
create).
- Anything else, including a changed create from the same author, is
refused and writes nothing, so it can never re-run side effects or
announce members.
- A resend that arrives while the first create is still uncommitted
waits on its channel insert. It is confirmed once the first commits, or
creates the channel itself if the first rolled back. Before this, a
resend in that window was refused, and the app then withdrew a chat that
existed.
- **No compensation:** ingest no longer soft-deletes a pre-created
channel when the event insert fails. The rollback covers it.
- **Announcements:** a failed `member_joined` notice for one initial
member is logged at `error` and the loop continues, so the members after
it still get theirs. A resend writes nothing, so nothing would retry
them.
## Compatibility
A 9007 with malformed `p` tags, or more than 32 of them, used to be
accepted with the tags ignored. It is now rejected with `invalid:`. The
relay's own SDK and CLI builders send no `p` tags on 9007.
## Out of scope
- The kind:9000 OK still doesn't prove the membership change committed:
side-effect failures are logged and OK'd, and a duplicate replay skips
side effects. The new path doesn't depend on kind:9000.
- Events in separate HTTP requests are still unordered. Clients send the
first message after the create's OK.
## Tests
`store::channel::postgres_tests` (buzz-db):
- `a_resend_beside_an_open_create_confirms_it_once_committed`: holds a
create open, checks that the resend waits, commits, and expects `Exists
{ resent: true }` with one owner and one member.
- `a_resend_beside_a_failed_create_creates_the_channel`: the same, but
the first create rolls back.
- `a_create_whose_event_fails_leaves_no_channel`: an event the insert
refuses leaves no channel row, and a retry creates the channel.
- `another_create_for_a_taken_channel_writes_nothing`.
Two mutations, each caught: committing the channel before its event
fails the "leaves no channel" test, and looking up the event before the
channel insert waits fails the "confirmed once committed" test.
`handlers/create_with_members_postgres_tests.rs`, all through the real
ingest path:
- `create_adds_its_p_tags_as_members` checks:
- the roles and the 39002 roster
- exactly one `member_joined` notice
- that the agent can post with no separate add
- `create_respects_the_members_add_policy`: an agent with policy
`nobody` gets an `invalid:` rejection, and no channel is created.
- `resending_a_create_confirms_it`:
- an identical resend is accepted
- a changed create (from the creator or from someone else) is refused
- the roster is unchanged and nobody new is announced
- Unit tests for `initial_members`: dedupe, creator excluded, bad hex,
the 32-member cap, and the missing `h` tag.
Each new test was mutation-checked against the change it covers: no
members, no policy check, refuse every resend, and let a changed creator
create through. Each mutation turns its test red.
Agent exercise on a local relay, over `POST /events` the way the app
sends:
- The 9007 with `p` = agent was accepted in 54–97ms (debug build).
- The kind 9 was accepted 12–15ms after the create's OK.
- The agent was in the 39002 roster, and the operator listener delivered
the mention.
- An identical resend came back `duplicate:` (accepted), and a changed
create was refused.
Local suite runs at
|
||
|
seiler
|
66fabf9e64 |
test(db): upgrade invite repair from schema 66
Co-authored-by: Codex <noreply@openai.com> Signed-off-by: seiler <seiler@squareup.com> |
||
|
seiler
|
22b5282a64 |
Repair strict legacy invite revocation
Signed-off-by: seiler <seiler@squareup.com> Co-authored-by: Codex <noreply@openai.com> |
||
|
Local Builder
|
a3b7b181a8 |
fix(db): reconcile invitation admission with main
Adapt invitation helpers to the community-admitted transaction API and move the unreleased revocation migration after the current schema changes. Exercise upgrades from the latest pre-revocation schema. Signed-off-by: Local Builder <1c476cb1127ea270cb900bdc3891060140bdd1173f91498030ee9b4db01c2f20@buzz.block.builderlab.xyz> |
||
|
Local Builder
|
2ab99e9b7e |
fix(db): own invite revocation in writer transactions
Acquire admission before domain row locks in restriction and owner writers. Revoke invitations in the same commit without SQL triggers, while keeping existing owner links and ordinary user inserts lock-free. Signed-off-by: Local Builder <1c476cb1127ea270cb900bdc3891060140bdd1173f91498030ee9b4db01c2f20@buzz.block.builderlab.xyz> |
||
|
Brad Seiler
|
c83c9bb1c3 |
fix(db): log transactional claim denials
Co-authored-by: Codex <codex@openai.com> Signed-off-by: Brad Seiler <seiler@squareup.com> Signed-off-by: Local Builder <1c476cb1127ea270cb900bdc3891060140bdd1173f91498030ee9b4db01c2f20@buzz.block.builderlab.xyz> |
||
|
Brad Seiler
|
d6b9fbc240 |
fix(db): distinguish issuer timeout denials
Co-authored-by: Codex <codex@openai.com> Signed-off-by: Brad Seiler <seiler@squareup.com> Signed-off-by: Local Builder <1c476cb1127ea270cb900bdc3891060140bdd1173f91498030ee9b4db01c2f20@buzz.block.builderlab.xyz> |
||
|
Brad Seiler
|
c674d64354 |
fix(db): commit invitation ownership on admission
Co-authored-by: Codex <codex@openai.com> Signed-off-by: Brad Seiler <seiler@squareup.com> Signed-off-by: Local Builder <1c476cb1127ea270cb900bdc3891060140bdd1173f91498030ee9b4db01c2f20@buzz.block.builderlab.xyz> |
||
|
Brad Seiler
|
e6b00214b1 |
test(db): derive push lease fixture time from persisted receipt
Avoid comparing host and PostgreSQL clocks in the async enqueue guard regression. Assert strict lease ordering before releasing activation.
Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: Brad Seiler <seiler@squareup.com>
(cherry picked from commit
|
||
|
Brad Seiler
|
d0441db919 |
test(relay): select invite boundaries and seed fenced upgrade fixtures
Co-authored-by: Codex <codex@openai.com> Signed-off-by: Brad Seiler <seiler@squareup.com> Signed-off-by: Local Builder <1c476cb1127ea270cb900bdc3891060140bdd1173f91498030ee9b4db01c2f20@buzz.block.builderlab.xyz> |
||
|
Brad Seiler
|
d2c2a74943 |
fix(relay): revoke invitations durably and fence restricted claims
Keep v1 natural drain with an optional UTC cutoff. Serialize claim and mint policy reads with restriction and owner writes. Share historical revocation repair across migration and desired-state upgrades while retaining deletion fences. Co-authored-by: Codex <codex@openai.com> Signed-off-by: Brad Seiler <seiler@squareup.com> Signed-off-by: Local Builder <1c476cb1127ea270cb900bdc3891060140bdd1173f91498030ee9b4db01c2f20@buzz.block.builderlab.xyz> |
||
|
Will Pfleger
|
6286a54321 |
feat(relay): store per-community NIP-FI issuer bindings (#8372)
Stack: this PR → #8377 → #8399 → #8401 bb-public creates communities all the time, so a hand-edited `BUZZ_NIP_FI_COMMUNITIES` map can't list them. This PR stores an explicit NIP-FI issuer allowlist for each community in the database and writes it when the community is created. Login checks don't read the table yet; that comes in #8377 together with the database-backed resolver. Operator steps are in [`docs/operator-nip-fi-bindings.md`](docs/operator-nip-fi-bindings.md). **Storage.** Migration `0066_community_nip_fi_issuers` adds `community_nip_fi_issuers (community_id, issuer, created_at)`, with primary key `(community_id, issuer)` and the community write fence attached. The assertion audience is `https://<communities.host>`, so the table has no `aud` column. Zero rows means no binding. The table is included in whole-community deletion and purge. **One bindable rule.** `BUZZ_NIP_FI_PROVISION_BINDING` (`{"host_suffix": ..., "authorized_issuers": [...]}`) is parsed in `buzz-auth` with `BUZZ_NIP_FI_ISSUERS`, whatever `BUZZ_NIP_FI_MODE` is set to, and both the relay and `buzz-admin` use the same parser. Each authorized issuer must be a configured issuer (1 to 2048 bytes), and the suffix must not cover any host declared in `BUZZ_NIP_FI_COMMUNITIES`; an unreadable declaration refuses startup even in mode `off`. `ProvisionBindingPolicy::is_bindable(host)` is true for a host exactly one label under the suffix that is not reserved. It decides creation-time binding, explicit `bind`, and what the lookup route reports. **Binding on creation.** Rows are written in the same transaction as the community insert on all three creation paths (`POST /operator/communities` `create_only`, its legacy convergence branch, and startup bootstrap), and only when the call actually created the community, so existing rows are never rewritten. **Reserved host.** A host whose `https://<host>` equals a configured `command_audiences` value never holds a binding. Operator provisioning of it is rejected with a 400 and the `reserved_host` error code, bootstrap creates it unbound, and the availability route reports it unavailable with an error code. `main.rs` calls `verify_reserved_hosts_unbound` at startup; if a reserved host has rows the relay refuses to start and names the fix (`buzz-admin nip-fi unbind --host <h> --issuer <i>`, or revert the command audience). A database failure during that check is a separate `Check` error. **Operator commands.** - `buzz-admin nip-fi backfill` pages unbound communities by host (1000 per page) and binds each page with one `INSERT ... SELECT` limited to active communities. It skips communities being deleted, always prints `{rows_bound, unbound, unbound_hosts, truncated}`, and exits non-zero while any bindable community is still unbound. Archived communities are bound on purpose. Both bind paths take the shared deletion lock and then lock the community row (`FOR NO KEY UPDATE`, in id order for a batch), the same order deletion uses, and the backfill insert also requires the community to still have no binding, so an operator `bind` that lands after the backfill listed a community wins and the default issuer is not added beside it. - `buzz-admin nip-fi bind --host --issuer` refuses a reserved host, a host outside the suffix, or an unconfigured issuer. - `buzz-admin nip-fi unbind --host --issuer` refuses to remove a community's last issuer (archive or delete the community instead), with one exception: when a valid provisioning policy shows that backfill can't bind the host (it's reserved, or outside the suffix), the last issuer can be removed. That is the reserved-host recovery path in the runbook. A missing or malformed policy doesn't block `unbind`; it just means the exception never applies. **Lookup route.** `GET /operator/communities/nip-fi-binding?host=` (NIP-98, `RELAY_OPERATOR_PUBKEYS`) reads the database and returns `{host, active, issuers[]}`. `host` is the stored host. It returns 404 for an unknown host, 200 with `active:false` for an archived or quiescing community, and `issuers:[]` when no provisioning policy is set. It lists only issuers that are configured and on a bindable host. kgoose depends on this contract; it is also documented in the runbook. **NIP-FA.** The `relay_url` bullet now also says: "An adapter MAY obtain its exact relay list from a configured, authenticated source; it MUST still compare `relay_url` exactly." **Deployment dependency.** The kgoose config side is already handled by [cash-server#137671](https://github.com/squareup/cash-server/pull/137671). In public-community mode it requires the command audience not to be a community audience. --------- Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz> Co-authored-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz> |
||
|
Luke Tornquist
|
97e1f6a082 |
fix(db): validate channel roster snapshots in the app writer (#8316)
## Summary Phase 1 of retiring the channel roster snapshot trigger (`trg_events_guard_channel_roster_snapshot`, migration 0032). Every admitted writer that can store a kind 39002 roster snapshot now runs the same check the trigger runs, on the same transaction and under the same lock: - `LockedMemberSnapshot::replace_member_event` (relay's live roster publisher) validates the snapshot it already holds the membership lock for, before soft-deleting the old head. - `insert_event_on` and `insert_event_with_thread_metadata_tx` (generic event writes). - `replace_parameterized_event_in_transaction_impl` (inside its existing savepoint) and `Db::replace_addressable_event` (admin reconciliation path), after dominance checks and before the old head is deleted. The validator (`validate_channel_roster_snapshot`) is a direct port of `guard_channel_roster_snapshot()`: four-field `p` tags, 64-char hex pubkey, empty relay field, valid role, and an exact match against active `channel_members` rows. It raises the same `23514` messages, so callers still see `DbError::Sqlx(sqlx::Error::Database(..))` with the same code and text. **Lock order:** replacement key, then channel-membership key, then the canonical read, then the old-head `UPDATE` and the write. Legacy writers took the membership key later, at the trigger's INSERT, after the old-head row locks. Taking it before the `UPDATE` is not the same order, but it is strictly safer: it removes the row-lock/membership-key cycle. The membership writers in `channel_members.rs` take the same membership key before they change rows. Not every membership writer does yet: `execute_kick_with_marker` (reached from report enforcement) and `deploy_kick_member` in `relay_admin_actions.rs` set `removed_at` without the key, so a kick can commit after a roster passes validation and before that roster commits, leaving the removed member listed. The retained trigger has the same gap, so this PR neither causes nor closes it. When the trigger also runs it re-takes a key this transaction already holds; transaction advisory locks are re-entrant, so this adds no wait. The validator is one verdict query on the happy path. On failure it raises the trigger's exact error via a `DO` block, so mixed-version behavior is identical; phase 2 can switch to a typed `DbError`. **Unchanged on purpose:** the trigger, its function, migration 0032, and the startup probe `verify_channel_roster_fence_behavior`. Pods still on the old version depend on them. Phase 2 drops them once this version is running everywhere. ## Tests New PG tests in `channel_members.rs` `postgres_tests`, each with only the roster trigger disabled (the helper asserts the catalog check then fails): - `app_roster_guard_matches_trigger_for_roster_input_table`: 19 shape and membership cases (3/5-field tags, short and non-hex pubkeys, relay field, unknown role, duplicate, role mismatch, missing/extra member, uppercase hex, non-`p` tags, empty rosters, all roles). For each one, the app outcome through `Db::replace_addressable_event` must equal the trigger's outcome on a raw INSERT, including SQLSTATE and message. - `app_roster_guard_locked_writer_rejects_stale_and_accepts_current_without_trigger` - `app_roster_guard_covers_generic_and_parameterized_writers_without_trigger` - `app_roster_guard_rechecks_after_concurrent_membership_commit_without_trigger`: the 0032 race. A newer membership commits while an older snapshot waits on the membership key. The barrier is `pg_locks` with a bound, not sleeps. `stale_legacy_roster_cannot_replace_new_locked_snapshot` (`replaceable.rs`) and `desired_schema_rejects_stale_legacy_roster_role` (`channel_members.rs`) now simulate the old binary inline (replacement key, old-head `UPDATE`, raw INSERT) instead of calling the app API, so they test the retained trigger again. The replaceable.rs one keeps its one-connection pool, so it still catches self-deadlock. `observability_source.rs` gains `every_production_event_insert_runs_the_roster_guard`, which requires a real guard call in every production `INSERT INTO events` function, with explicit exceptions (NIP-43 membership and push lease rows, both `channel_id` NULL, and the two rolled-back startup probes), plus a self-test. Falsified: removing one guard call fails the source policy; dropping the role clause or `lower()` fails the parity table; disabling the trigger fails both legacy tests. The PG tests are picked up by the `postgres-ci` nextest filter (`test(/postgres_tests::/)`); the source policy runs in `just test-unit`. ### Coverage limits (review follow-up) - `every_production_event_insert_runs_the_roster_guard` is a presence check. It rejects missing, commented, and quoted calls, but it does not prove a call is awaited, reachable, or passed the event's real kind and channel. Behavior is covered by the PG tests above. The test now says this in its doc comment. - Exemptions match by function header. A kind 39002 insert added inside one of the four exempt functions would pass the policy. This is documented next to the exemption list. - The 19-case parity table and the race test drive `Db::replace_addressable_event` only. The generic, parameterized, and locked writers get the mismatch case only, and the parameterized path's membership-key-before-delete ordering is not exercised concurrently. All five writers call the same validator, so this PR doesn't widen these tests. - `disable_roster_trigger` commits `DISABLE TRIGGER` on the test database. That is safe under the per-test database harness the `postgres-ci` profile uses, but not against a shared database. Its doc comment now says so. ## Validation Run on a Blox workstation (private PG 16 and Redis) against a tree byte-identical to this commit: - `cargo fmt --all -- --check`, `cargo clippy --workspace --all-targets -- -D warnings`, `cargo nextest run -p buzz-db --lib`: pass. - `cargo nextest run -p buzz-db --test observability_source`: 19 passed. - `scripts/postgres-test-run.sh -p <pkg>` for buzz-admin, buzz-audit, buzz-deletion, buzz-push-gateway, buzz-relay, buzz-search, buzz-workflow: pass. - buzz-db: the full run passed 513 and was stopped at a 240 s deadline on `writer_pool_rejects_non_read_committed_database_default`, which hangs the same way on the unmodified base. A second run excluding the three host-flaky runtime tests already reproduced on the base passed all 511 others. ## Simpler? Not yet. While the trigger is still in place, the check runs twice. Phase 2 deletes the trigger, its function, the startup catalog and behavior probes, the migration.rs equivalence asserts, and the pgschema reconcile entry, which leaves one check in Rust. ## Phase 2 (separate PR, after fleet rollout) **Prerequisite before claiming complete roster serialization (phase 2 or anywhere else):** make both kick paths take the channel membership key before they change rows, and add a concurrent kick-vs-roster-publication test. The retained trigger does not prove this; it has the same gap. A new migration drops the trigger from `events` and its partitions, then drops `guard_channel_roster_snapshot()`. It also removes the schema files and includes, `verify_channel_roster_fence*` and the relay startup call, the migration.rs asserts, and the probe tests, and updates `scripts/reconcile-schema-after-pgschema.sql`. --------- Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> |
||
|
Luke Tornquist
|
60de1db0b8 |
fix(db): admit fleet sweeps per tenant instead of in-SQL fence predicate (#8318)
## What Removes every production call to the SQL fence predicate `community_write_allowed()` from application SQL, so no application mutation depends on the boolean predicate anymore. This is one prerequisite for BUZZ-315 part B; #8322 (tolerating absent fence objects) is a separate one. The triggers and SQL functions stay in this PR because mixed-version pods still depend on them. The predicate isn't inlined. It's a VOLATILE plpgsql function that takes the deletion lock and *then* reads lifecycle state on a fresh snapshot, and an inline expression would read state from the statement's start snapshot. A separate earlier check would race deletion. So each sweep now: 1. reads its candidate community IDs with a plain read, bounded the same way the original statement was, 2. opens one `AdmittedTx` per community (`begin_community_sweep_transaction`, a thin wrapper over `begin_community_write_transaction` that turns an admission refusal into "skip this tenant"; the four periodic sweeps go through `sweep_admitted_communities`, which also owns their error handling), 3. runs the original mutation scoped to `community_id = $1` and commits. | Site | Change | | --- | --- | | `runtime/push_enqueue.rs` | Deleted the redundant predicate. This transaction already calls `guard_transaction`. | | `store/relay_invite.rs` `reap_expired_relay_invites` | Keeps the global expiry order and the 1,000-row ceiling. Candidates are pre-filtered to active tenants so fenced invites can't fill the batch. Admission is still what authorizes the write. | | `store/workflow.rs` `prune_scheduled_workflow_fires_before` | Prunes per admitted community and sums the row counts. | | `store/channel.rs` `reap_expired_ephemeral_channels` | Archives per admitted community and returns the committed rows, so the relay still runs their system and discovery follow-ups. The candidate read has no lifecycle pre-filter (it has no `LIMIT`, so admission alone decides). | | `store/event.rs` `backfill_d_tags` | Backfills per admitted community at startup. | | `store/push.rs` `claim_due_match_batch_with_loader` | Picks the earliest due active community, admits it, and claims under the same `FOR UPDATE OF q SKIP LOCKED` and batch limit. It skips at most 16 tenants that fence between the read and admission. | | `store/push.rs` `reap_exhausted_matches` | Reaps per admitted community. | **Tenant failures.** The four periodic sweeps (invite reap, workflow fire prune, channel reap, exhausted match reap) log a warning and continue when one tenant fails. That tenant's rows are left unmodified and stay eligible, so the next tick retries them. A tenant error doesn't abort processing of the other tenants already in the selected candidate page. Invite cleanup keeps its oldest-first 1,000-row selection, so if one tenant owns that entire page and its delete keeps failing, tenants outside the page wait. No persistent tenant-local failure for that delete is known in the current schema, and base aborted the whole page on any error. The returned count or rows cover the tenants that committed. Claim, enqueue and the startup backfill still return the error to the caller. **Database outages.** A connection-level error (pool acquire timeout, closed pool, socket I/O; `DbError::is_connection_unavailable`) isn't tenant-local, so it stops the periodic sweep at that tenant instead of moving on. Without this, an outage that began after the candidate read cost one writer acquire timeout and one warning per remaining tenant, and invite cleanup runs inside the usage-metrics leader tick. Rows already committed are still returned, so reaped channels keep their follow-ups. Remaining tenants retry next tick. **Known limit (pre-existing, tracked separately):** - *Claim at a locked head.* `claim_due_match_batch_with_loader` returns `None` once an admitted tenant claims zero rows. If every due row in the oldest active tenant is row-locked, younger tenants aren't tried on that call. Base behaved the same way. This PR doesn't change it. ## Tests All tests use a one-session fixture (`test_support::admission_fixture`) with `session_replication_role = replica`, so the trigger backstop is off and only application admission protects the fenced tenants. The fixture shadows `community_write_allowed` with a function that raises, in a per-call `admission_test_<uuid>` schema. - One test per site: a quiescing tenant's rows are untouched while an active tenant in the same sweep still progresses. - Race tests for invite reap, channel reap and claim. These are the sites where the candidate read already filters out fenced tenants. Each test holds the tenant's deletion lock, waits until the sweep is blocked in admission, fences the tenant in that transaction, and asserts its rows are untouched. The claim test also asserts that a second active tenant is claimed in the same call. - `channel_reap_returns_committed_rows_after_earlier_tenant_error` and `periodic_reaps_continue_after_tenant_error_and_retry_retained_rows` inject a per-tenant failure. They check that later tenants commit, that the failed tenant's rows are retained, and that the next call processes them. - `fleet_sweep_stops_at_first_unavailable_connection` runs the sweep helper against a closed pool with three candidates and asserts only the first is attempted. `fleet_sweep_returns_committed_results_when_database_drops_mid_page` (PostgreSQL) commits the first tenant, closes the pool, and asserts the first tenant's result is returned and the sweep stops at the second of three candidates. `connection_unavailable_covers_only_pool_and_io_failures` pins the classification; the per-tenant failure tests above confirm a database-raised error still continues. Falsification, each with the tests failing as expected: | Mutation | Result | | --- | --- | | Original SQL restored at invite sweep, push claim, d-tag backfill | 0/3 pass (`legacy SQL fence predicate called`) | | `guard_community_write` returns `Ok(())` after taking the lock | race tests + channel reap test 0/4 pass | | Per-tenant errors abort the sweep again | 0/2 pass | | Sweep helper `continue`s instead of stopping on a connection-level error | `fleet_sweep_stops_at_first_unavailable_connection` and `fleet_sweep_returns_committed_results_when_database_drops_mid_page` fail | | Sweep helper drops already-committed results when it stops (`return Vec::new()`) | `fleet_sweep_returns_committed_results_when_database_drops_mid_page` fails | | Sweep helper removed from the source-policy chokepoint check | `serving_table_writes_expose_syntactic_chokepoint_or_guarded_tx_routes` fails | The PostgreSQL tests are under `postgres_tests` modules with `#[ignore = "requires Postgres"]`, which puts them in the `.config/nextest.toml` PostgreSQL filter. CI evidence is under Verification. ## Verification CI at head `5d37fbda` ([run 38925996275](https://github.com/block/buzz/actions/runs/38925996275)) is green. The PostgreSQL job ([job 117062895683](https://github.com/block/buzz/actions/runs/38925996275/job/117062895683)) passed 1017/1017. That includes named PASS lines for all 30 `admission_postgres_tests`, `fleet_sweep_returns_committed_results_when_database_drops_mid_page`, and both writer-pool tests listed below. Unit Tests ([job 117058136482](https://github.com/block/buzz/actions/runs/38925996275/job/117058136482)) has PASS lines for `connection_unavailable_covers_only_pool_and_io_failures` and `fleet_sweep_stops_at_first_unavailable_connection`. Local gates ran on a Linux cloud workstation against private PostgreSQL 16 and Redis. The tree was `735170df`, which is this head rebased on `main` `c0ab789`, so the gates built the merge result. | Gate | Result | | --- | --- | | `cargo fmt --all -- --check` | pass | | `cargo clippy --workspace --all-targets -- -D warnings` | pass | | `cargo nextest run -p buzz-db --lib --tests` | 185 passed | | `scripts/test-postgres-test-discovery.sh` | pass | | PostgreSQL lane for buzz-db (excluding the two writer-pool tests below) | 530 passed | | PostgreSQL lanes for buzz-admin, buzz-audit, buzz-deletion, buzz-push-gateway, buzz-search, buzz-workflow | all pass | | PostgreSQL lane for buzz-relay | 413/426. The 13 `group_state_postgres_tests` fail with `PoolTimedOut` on unmodified `main` `c0ab789` too (see below) | | The three mutations in the falsification table above (`continue` ×2, `return Vec::new()`) | each fails its target test | These failures happen only on the workstation, and each one passes in CI's PostgreSQL job at this head: - `writer_pool_metrics_stop_after_session_timeout_setup_failure` stalls. It's an unchanged writer-pool configuration test, and it stalled the same way at earlier heads. - The 13 relay `group_state_postgres_tests` fail at the same point, `ensure community: Sqlx(PoolTimedOut)`, on unmodified `main` `c0ab789`. This PR doesn't touch `buzz-relay`. ## What got simpler - No application mutation calls the boolean predicate `community_write_allowed()` anymore. Dropping the fence objects still has other dependencies: serving-write lease acquisition calls `assert_community_write_allowed()` (`store/deletion.rs:3068`), catalog validation requires both functions (`store/deletion.rs:1195-1198`), and the triggers and compatibility SQL still use them. Those belong to #8322 and part B, not this PR. - The claim's `target` CTE is gone, and every sweep uses the same admission-and-skip helper. - The four periodic sweeps share one loop (`sweep_admitted_communities`) for admission, skip, continue-on-tenant-error and stop-on-outage, instead of four copies of it. - The source policy treats that helper as a tenant-local chokepoint, so `backfill_d_tags` no longer needs a reviewed exception. - The per-tenant loops add code, because each community now gets its own admitted transaction. --------- Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> |
||
|
Luke Tornquist
|
2745a419a8 |
buzz-db: tolerate absent universal write-fence objects (expand) (#8322)
## What This is the expand step for retiring the 32 universal `community_write_fence_*` triggers and their SQL functions (BUZZ-315). Relay pods auto-migrate at boot (`BUZZ_AUTO_MIGRATE`). The drop migration therefore runs while the previous release is still serving, and that release must not depend on the objects it drops. Today the app depends on them in four places, and this PR removes all four from its own code paths. Seven in-SQL `community_write_allowed` predicates in fleet sweeps and push (`push_enqueue.rs`, `channel.rs`, `event.rs`, `push.rs`, `relay_invite.rs`, `workflow.rs`) are #8318's scope. The release is independent of the fence functions only with both this PR and #8318. **It changes no schema.** The objects stay and keep working until the follow-up drop PR. | Old dependency | Now | |---|---| | Startup and readiness (`validate_serving_catalog_on`) require `community_write_allowed`, `assert_community_write_allowed`, `enforce_community_write_fence` and all 32 triggers | No longer required. Still required: the tenant tables, `community_deletion_lock_key` and the enabled community tombstone trigger. | | Serving-lease admission runs `SELECT assert_community_write_allowed($1)` | Uses `guard_community_write`, the same shared lock and state check as ordinary serving writes, in the same transaction as the lease `INSERT`. Fenced or missing communities still map to `AccessDenied`. | | Live scoped-table discovery calls `community_write_fence_excluded_table()` | Uses a Rust `EXCLUDED_SCOPED_TABLES` list. A unit test pins it equal to the SQL list while both exist. | | The deletion executor compares the live `fenced_tables` against the frozen inventory | Only tenant-table equality is compared. Inventories **still record** live fences, so an older executor that validates an inventory frozen by this release still matches. The digest still covers the full frozen manifest. | `lock_community_deletion_shared` now refuses transactions that are not READ COMMITTED, so every lock-then-read admission gets the check: `guard_community_write`, serving-lease finalization (`guard_transaction_with_serving_lease`), lease renewal/verification, owner preparation and owner-mutation admission. The old SQL assertion did this check for the lease path, and the trigger backed up the rest. The isolation level comes back from the existing `pg_advisory_xact_lock_shared` statement, so the hot path makes no extra round trip. The refusal comes before any lifecycle-state read. ## Mixed versions | Combination | Why it is safe | |---|---| | Previous release, this DB | No schema change, so all old checks still see the 32 fences. | | This release, DB with fence objects | Extra objects are tolerated, and the trigger backstop still applies. | | This release **plus #8318**, DB without fence objects | Serving, readiness, lease admission and deletion proven by `migration_schema_fence_objects_are_optional_for_serving_and_deletion`, which drops them inline. Without #8318, serving and deletion still work, but the push matcher, exhausted-job reaper, invite retention, ephemeral reaper and startup d_tag backfill fail with `42883 community_write_allowed(uuid) does not exist` (seen in deploy-stack E2E). | | Older executor, inventory frozen by this release | `fenced_tables` is still populated from the live catalog (32). | This PR does not depend on #8318 to merge and can ship alongside it, but it is not drop-safe alone. The drop PR (#8323) needs #8219, #8318 and this PR deployed on every pod, and every rollback image after the drop must include #8318. ## Tests - `store::deletion::postgres_tests::migration_schema_fence_objects_are_optional_for_serving_and_deletion` migrates to 0060. It then runs the following with the fence objects present, then again after dropping them inline: - serving startup - readiness - the deletion catalog check - `inventory_schema` - lease acquire/release - `AccessDenied` for quiescing, fenced, tombstoned and missing communities - `store::deletion::postgres_tests::deletion_lock_admission_rejects_pinned_snapshot_isolation` checks that REPEATABLE READ and SERIALIZABLE are refused by both `guard_community_write` and `guard_transaction_with_serving_lease`. The lease case uses a never-acquired lease, so if the isolation check is skipped it fails as stale instead. - `buzz-deletion` `postgres_tests::approved_legacy_fence_inventory_accepts_current_catalog` checks that a legacy digest with 32 fences validates against a live catalog with none, and that an unknown tenant table is still rejected. - `store::deletion::tests::scoped_table_exclusions_match_sql_during_transition` checks exclusion-list parity. - `store::deletion::postgres_tests::serving_lease_acquire_waits_for_fence_lock_then_refuses` holds the exclusive deletion lock and checks that lease acquisition stays pending. It then commits `fenced` and requires `AccessDenied` with no lease row persisted. Removing the lease path's `guard_community_write` call fails this test, while the other four lease/admission tests above still pass. - `buzz-deletion` `serving_guard_heartbeats_long_operation_through_quiescing` now requires `AccessDenied` for a lease acquired during quiescing, rather than any error. Each test was confirmed to fail when its fix is removed (rc 100, 0/1): the isolation refusal, the three validators restored to the base version, the fence-list comparison restored, and one name dropped from the Rust list. ## What got simpler The lease path loses its special SQL assertion and its error-code translation and reuses the ordinary app guard. The serving and deletion catalog checks each lose one requirement. No new abstraction. The `fenced_tables` reader stays only for mixed-version executors; the drop PR deletes it. **Identified debt (pre-existing, outside this diff):** `runtime::postgres_tests::writer_pool_rejects_non_read_committed_database_default` hangs on clean `177551dda` too. ## Verification (Blox, tree identical to this commit) | Gate | Result | |---|---| | `cargo fmt --all -- --check` | pass | | `cargo clippy --workspace --all-targets -- -D warnings` | pass | | `cargo nextest run -p buzz-db --lib` | 167/167 | | `scripts/test-postgres-test-discovery.sh` | pass | | `scripts/postgres-test-run.sh -p` buzz-admin / audit / deletion / push-gateway / relay / search / workflow | all rc 0 (4, 7, 12, 11, 402, 20, 2) | | `scripts/postgres-test-run.sh -p buzz-db` | 510 pass, 1 fail: the pre-existing hang above, interrupted | | Round 2 (this head): `-p buzz-db` / `-p buzz-deletion` | 511 pass + the same pre-existing hang interrupted / 12/12 | The new PG tests are selected by the `postgres-ci` filter `(test(/postgres_tests::/) or binary(/^postgres_/)) and not test(/(^|::)external_infra[^:]*::/)`. buzz-review-completed --------- Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> |
||
|
Luke Tornquist
|
c0ab789499 |
feat(buzz-db): retire the channel-TTL trigger (#8210)
## Summary This drops the channel-TTL event trigger. The application already refreshes ephemeral-channel deadlines in the event transaction (`event_follow_up::refresh_channel_ttls`, from #8135), so the trigger only repeats that work. This PR originally retired both follow-up triggers. #8352 has since retired the push-match trigger in migration 0063, so after the rebase this PR covers only the TTL half. Migration `0065_retire_channel_ttl_trigger.sql`: - `SET LOCAL lock_timeout = '5s'` (the same bound as 0063) - drops `events_refresh_channel_ttl` and `refresh_channel_ttl_after_event_insert()` Dropping the trigger on the partitioned parent also drops its partition clones. Published migrations, including 0022 and 0024, stay checksum-frozen. The same objects are removed from `schema/` (`schema/functions/…`, `schema/triggers/events.sql`, `schema/schema.sql`) and from the pgschema reconcile script. After this, migrated and desired-state databases have no event follow-up triggers. Tests: - `follow_up_retirement_tests.rs` (renamed from `push_retirement_tests.rs`): the push-retirement upgrade test now stops at 0063. A new test upgrades 0064 → 0065 and checks that the TTL trigger is gone from the parent and every partition, that the function is gone, and that the published checksums haven't changed. - `event_follow_up_postgres_tests.rs`: the contract now has two arms. `AppOnly` is the shipped schema, where both triggers are absent. `Dual` reconstructs the historical overlap of both triggers. The `PushRetired` arm (the push trigger dropped, the TTL trigger kept) was the shipped state before this PR. Its tests are removed because `Dual` and `AppOnly` together cover it. - `embedded_migrator_contains_consolidated_initial_schema` pins the 0065 statements and checks that both objects are absent from the desired schema. **Deploy gate:** do not run 0065 against a database still served by a relay older than #8135. After 0065, raw SQL writers no longer refresh TTL deadlines. `docs/push-gateway-deployment.md` is updated to match. ### What got simpler The trigger and the app no longer both refresh TTL deadlines; the app is now the only producer. One plpgsql function and one trigger are gone from both schema paths, and 8 per-partition drops are gone from the reconcile script. The follow-up contract drops from three arms to two. ### Related issue BUZZ-313 (Linear), part of the trigger-retirement work. ### Testing Head is `4f7d0baf0`. Results below are at `678927730` unless noted. `678927730..4f7d0baf0` changes only one test in `crates/buzz-db/src/store/event.rs`, described in the first bullet. - **Fixed test, `permanent_channel_event_commits_do_not_lock_the_channel_row`** (`4f7d0baf0`). - **Why it changed:** after 0065, its raw-pool inserts no longer ran any TTL refresh, so the test passed whatever `refresh_channel_ttls` did. This was raised in review. - **What it does now:** it inserts through the production `AdmittedTx` path and keeps the barrier and the tuple-lock probe. It commits all N transactions while an external transaction holds `FOR SHARE` on the channel row. It also asserts that no `buzz_db_channel_ttl_refresh_failures_total` is counted. - **Mutation check:** with `SELECT … FOR UPDATE` injected into `refresh_channel_ttl`, the new test fails (`must not wait on the channel row`, timeout at 10s). The old test passes under the same mutation. - **Blox:** fmt and `buzz-db` clippy pass. - **GitHub CI at `4f7d0baf0`:** every required check passes (56 pass, 0 fail). CI builds the merge ref with `main` `93ffc37cb`. - **New and renamed tests, checked by name in the CI logs at `4f7d0baf0`.** These pass in PostgreSQL Domain / PostgreSQL Tests ([117033725006](https://github.com/block/buzz/actions/runs/38915998692/job/117033725006)): - `permanent_channel_event_commits_do_not_lock_the_channel_row` - `migration_schema_ttl_retirement_drops_every_partition_trigger_and_keeps_checksums` - `migration_schema_push_retirement_preserves_ttl_and_published_checksums` - `event_insert_ttl_refresh_handles_permanent_ephemeral_duplicate_and_activation_race` - `{desired,migration_schema}_event_follow_up_contract_{dual,app_only}` (four tests) `embedded_migrator_contains_consolidated_initial_schema` passes in Rust / Unit Tests ([117024125423](https://github.com/block/buzz/actions/runs/38915998692/job/117024125423)). No `*_push_retired` test runs anywhere. - **Blox gates at `678927730`** (private PG16 + Redis, on a workstation shared with other sessions): - `cargo fmt --check`, workspace `clippy -D warnings` and `buzz-db --lib` all pass. - The `buzz-admin`, `buzz-audit`, `buzz-deletion`, `buzz-push-gateway`, `buzz-search` and `buzz-workflow` Postgres packages pass. - `buzz-db`: 517 of 519 tests passed. The 2 that hung, both `writer_pool_*` tests, were killed. `buzz-relay`: 413 of 426 passed. The 13 failures are covered below. - **Uncontended reruns at `4f7d0baf0`** on a fresh, idle workstation (PG16.15, `competing=0` for every arm): - **`writer_pool_rejects_non_read_committed_database_default` predates this PR.** It timed out 3/3 at head and 3/3 at `main` `93ffc37cb`, with a 300s timeout per run. - **`writer_pool_metrics_stop_after_session_timeout_setup_failure` also predates this PR.** It timed out 2/3 at head and 2/3 at `main`, and passed in the remaining run on each side. - Both tests pass in CI. The gate accepts them as a named exception for this PR, as a pre-existing hang on the PG16 test environment. - **`buzz-db`:** 518 of 519 tests passed. The single failure is `writer_pool_rejects_non_read_committed_database_default`, killed after it hung (the exception above). - **`buzz-relay`: still open.** 413 of 426 tests passed. The same 13 `handlers::group_state_postgres_tests` failed as on the shared box, each panicking at once in its fixture's `ensure community` step with `Sqlx(PoolTimedOut)`. Contention does not explain them. The fixture's community setup is in `buzz-db`, which this PR changes. A head/`main` comparison of the full `buzz-relay` package on the same idle box is running to find out whether this predates the PR. All 13 pass in CI at this head. - **E2E (Gimli, Blox)** at `050e593af`. `050e593af..678927730` changes only tests and docs; the migration, schema and runtime code are byte-identical. Gimli's audited verdict: **EXERCISED AND WORKS**, for DB and relay runtime only. Gimli ran it at `050e593af`; this head's runtime is byte-identical but was not freshly executed. - **Upgrade:** a real relay startup took the schema from 0064 to 0065. That removed the TTL function and the trigger from the parent table and from all 8 partitions. Checksums for migrations 1–64 are unchanged. - **Lock-timeout rollback:** with a real lock held, the migration failed with `55P03` at 5.0007s and the schema rolled back fully to 64. Once the lock was released, a retry applied 65. - **Fresh desired schema:** after pgschema plus reconcile, the retired objects are absent. - **Real signed kind-9 sends:** on both schema paths, these refreshed ephemeral-channel deadlines to about +600s. Permanent channels stayed NULL. - **Raw SQL insert:** at 64 it refreshed the deadline. After 65 it left the deadline unchanged, on both the upgraded and the fresh schema. - **Not covered:** native or device testing, and deploy. - **Review:** Legolas found 0 P0 and 0 P1. His three optional P2s are fixed in `678927730`. - **No human app test:** this is a DB-only change. - **Migration numbering:** #8237 and #8372 also add a `0065_*` migration. Whichever PR merges second must renumber. --------- Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> |