mirror of
https://github.com/block/buzz.git
synced 2026-11-06 09:21:45 +00:00
## Summary Stacked on https://github.com/block/buzz/pull/8782 (merged), which makes the relay refuse a second open channel with a name that another channel in the community already has. That check lives in relay code, so an older relay, a direct database write or a missed code path can still create a duplicate. This PR adds the same rule to the database: a unique index on channel name, ignoring case, for the channels that show in the channel list. With the index in place, a duplicate fails at the write, whatever wrote it. The relay's own name check from https://github.com/block/buzz/pull/8782 is no longer needed, so this PR removes it. Clients still get the same refusal, `invalid: an open channel named "…" already exists`. The migration builds the index itself, also on a populated `channels` table, so an auto-migrating relay upgrades with no operator index step (the rule from https://github.com/block/buzz/pull/8802). It still refuses to run while duplicate names remain, because the index cannot exist over them. `docs/channel-name-uniqueness-deployment.md` gives the full rollout. ## Details - **The index** (migration `0083_channels_listed_name_unique.sql`): ```sql CREATE UNIQUE INDEX idx_channels_listed_name ON public.channels (community_id, lower(name)) WHERE visibility = 'open' AND NOT unlisted AND channel_type IN ('stream', 'forum') AND deleted_at IS NULL; ``` It covers open, not unlisted streams and forums that are not deleted, archived ones included. Private channels, DMs and unlisted channels may still repeat names. - **The migration builds the index inline, as `0049` does.** When `to_regclass` finds no index, it runs a plain `CREATE UNIQUE INDEX` under `lock_timeout` 1s and `statement_timeout` 5s. On a busy or very large table it fails on those budgets, records nothing, and the relay retries at its next start. When the index already exists (a concurrent prebuild), it skips `CREATE` so it never requests the write-blocking lock. In both cases it then checks that the index is valid, ready and has the exact expected definition. - **One refusal stays: duplicate names.** The migration refuses to run while listed duplicates remain, and its error names the rename script. The renames must go through the relay so clients get the new kind:39000, and a migration must not change rows (`docs/database.md` Rule 3). This is the "fails closed until its repair has run" case in `migrations/AGENTS.md`, so the writer release from https://github.com/block/buzz/pull/8782 and the rename are a required stop. - **Relay.** A write that takes the name of a concurrent, uncommitted write waits for it, and fails if that write commits. So the post-write check and the per-name lock from https://github.com/block/buzz/pull/8782 are removed. The store already maps the index violation to the same "name taken" error. - **Desired-state schema.** `schema/tables/public/channels.sql` has the same index, and the schema parity test compares its live shape between the desired-state schema and the migrations. - **Rollout** (`docs/channel-name-uniqueness-deployment.md`), in order: 1. Deploy https://github.com/block/buzz/pull/8782 and wait until no older relay pod serves. 2. Rename the existing duplicates in each community with that PR's script, while relays keep serving. The script needs `psql` and `python3`, and the operator key must be a member of each community. The doc covers archived communities and communities being deleted. 3. Confirm that no duplicate remains. 4. Only for a deployment that migrates out of band and is too large for the budgets: build the index with `CREATE UNIQUE INDEX CONCURRENTLY` and verify it. 5. Deploy this release. With auto-migration on, the relay applies 0083 at startup. With it off, run `buzz-admin migrate` first, never the file through `psql`. Rollback goes to the https://github.com/block/buzz/pull/8782 release only. Keep the index, and keep auto-migration off. - **Test fixtures.** Tests that inserted two open channels with one name now use distinct names. Tests that migrate a populated database through 0083 no longer prebuild the index. ### Related issue None found. Follows https://github.com/block/buzz/pull/8782. ### Testing - Migration tests (`listed_channel_name_postgres_tests.rs`) start from a database migrated through 0082: - With duplicates, 0083 fails and changes nothing. - After the rename plan is applied, a channel writer that holds its lock makes the inline build fail on the 1s lock budget, with nothing recorded. - With no writer, the populated table migrates with no prebuild, and a raw duplicate insert fails on the index. - An index with a different definition, and an invalid one, are refused. A valid prebuilt index is accepted, and not replaced, while another transaction is writing to `channels`. - The store and relay name tests from https://github.com/block/buzz/pull/8782 pass with the relay check removed: the index refuses the duplicate, and the client still gets `invalid:`. - The rename plan and script tests from https://github.com/block/buzz/pull/8782 run on a database at 0082, the state the script runs in. - Local Postgres runs on `108737dc0`, one fresh database per test as the nextest wrapper does: 38 of 40 buzz-db migration-lane tests pass (both listed-name tests, the migration parity test, the thread-window and invite-upgrade tests that migrate populated databases through 0083). The 2 `event_follow_up` contract tests fail locally the same way on main. All 49 buzz-db channel and channel-member store tests pass. ### Database review checklist - [x] Phase: contract. Before it: https://github.com/block/buzz/pull/8782 (writer release, required stop) and its rename script. Nothing after it. - [x] No trigger, function, procedure, rule or foreign key. - [x] No migration mutates rows. The renames happen before it, through the relay, with the script in https://github.com/block/buzz/pull/8782. - [x] Never refuses a table because it has rows. Fails closed only with duplicates (repair not run) or with an invalid or different index; then 0083 raises and the relay does not start. The release notes must name the release that contains https://github.com/block/buzz/pull/8782 as a required stop. - [x] Rule 4: `migrations/`, `schema/tables/public/channels.sql` and the parity test. A live catalog comparison covers the index. - [x] Deployed relay: the https://github.com/block/buzz/pull/8782 relay answers `invalid:` beside the index (its tests pass with the index present). A relay older than that still writes, but gets a database error, not `invalid:`, for a duplicate. That is why https://github.com/block/buzz/pull/8782 is a required stop. - [x] Timeouts and index build: `lock_timeout` 1s, `statement_timeout` 5s. The migration builds the index inline (plain `CREATE UNIQUE INDEX`, skipped when `to_regclass` finds a prebuilt one). Only out-of-band deployments too large for the budgets prebuild it with `CREATE UNIQUE INDEX CONCURRENTLY` (doc step 4). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Logan Johnson <loganj@squareup.com> Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
73 lines
4.0 KiB
SQL
73 lines
4.0 KiB
SQL
-- ── Channels ──────────────────────────────────────────────────────────────────
|
|
-- Conformance: "Channels and channel membership". `community_id` immutable.
|
|
-- Channel UUIDs stay valid wire identifiers, but they are NOT globally unique:
|
|
-- the PK is `(community_id, id)`, so the same UUID may legitimately exist in two
|
|
-- communities (conformance lists "same channel UUID collision in two
|
|
-- communities" as a required isolation test). Handlers always carry `ctx`, so
|
|
-- `(ctx.community, h)` names exactly one channel; a client-supplied `h` can
|
|
-- never reach another community's channel.
|
|
|
|
CREATE TABLE channels (
|
|
id UUID NOT NULL DEFAULT gen_random_uuid(),
|
|
community_id UUID NOT NULL REFERENCES communities(id),
|
|
name VARCHAR(255) NOT NULL,
|
|
channel_type channel_type NOT NULL DEFAULT 'stream',
|
|
visibility channel_visibility NOT NULL DEFAULT 'open',
|
|
description TEXT,
|
|
canvas TEXT,
|
|
created_by BYTEA NOT NULL,
|
|
created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(),
|
|
updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(),
|
|
archived_at TIMESTAMPTZ,
|
|
deleted_at TIMESTAMPTZ,
|
|
nip29_group_id VARCHAR(255),
|
|
topic_required BOOLEAN NOT NULL DEFAULT FALSE,
|
|
max_members INT,
|
|
topic TEXT,
|
|
topic_set_by BYTEA,
|
|
topic_set_at TIMESTAMPTZ,
|
|
purpose TEXT,
|
|
purpose_set_by BYTEA,
|
|
purpose_set_at TIMESTAMPTZ,
|
|
participant_hash BYTEA,
|
|
ttl_seconds INT,
|
|
ttl_deadline TIMESTAMPTZ,
|
|
-- Labels, published as ["t", <label>] after the channel type tag on the
|
|
-- relay-signed group-state events. The relay validates their shape.
|
|
labels TEXT[] NOT NULL DEFAULT '{}'
|
|
CONSTRAINT channels_labels_bounded CHECK (cardinality(labels) <= 8),
|
|
join_role TEXT NOT NULL DEFAULT 'member'
|
|
CHECK (join_role IN ('member', 'guest')),
|
|
-- An unlisted open channel is readable and joinable by id but left out of
|
|
-- channel listings for non-members. Matters only while the channel is open.
|
|
unlisted BOOLEAN NOT NULL DEFAULT FALSE,
|
|
auto_join BOOLEAN NOT NULL DEFAULT false,
|
|
PRIMARY KEY (community_id, id),
|
|
CONSTRAINT chk_channels_id_not_nil CHECK (id <> '00000000-0000-0000-0000-000000000000'::uuid)
|
|
);
|
|
|
|
-- nip29 group id and DM participant hash are unique WITHIN a community, not globally.
|
|
CREATE UNIQUE INDEX idx_channels_nip29_group ON channels (community_id, nip29_group_id)
|
|
WHERE nip29_group_id IS NOT NULL;
|
|
CREATE UNIQUE INDEX idx_channels_dm_hash ON channels (community_id, participant_hash)
|
|
WHERE participant_hash IS NOT NULL;
|
|
CREATE INDEX idx_channels_community_type ON channels (community_id, channel_type);
|
|
CREATE INDEX idx_channels_community_visibility ON channels (community_id, visibility);
|
|
CREATE INDEX idx_channels_created_by ON channels (community_id, created_by);
|
|
CREATE INDEX idx_channels_ttl_expiry ON channels (ttl_deadline)
|
|
WHERE ttl_seconds IS NOT NULL AND archived_at IS NULL AND deleted_at IS NULL;
|
|
-- Tenant-independent channel-id → community lookups (Db::communities_of_channels,
|
|
-- Db::community_of_channel) carry no community_id predicate, so no
|
|
-- community_id-leading index can serve them. Covering + partial: index-only scan.
|
|
-- Not UNIQUE — the same channel id may exist under more than one community.
|
|
CREATE INDEX idx_channels_id_live ON channels (id) INCLUDE (community_id)
|
|
WHERE deleted_at IS NULL;
|
|
-- Two listed channels in a community never share a name, ignoring case. A
|
|
-- listed channel is an open, not unlisted stream or forum that is not deleted;
|
|
-- an archived one still counts. Created by migration 0083; the relay maps a
|
|
-- refused write to ChannelNameTaken (LISTED_NAME_INDEX in
|
|
-- crates/buzz-db/src/store/channel.rs).
|
|
CREATE UNIQUE INDEX idx_channels_listed_name ON channels (community_id, lower(name))
|
|
WHERE visibility = 'open' AND NOT unlisted
|
|
AND channel_type IN ('stream', 'forum') AND deleted_at IS NULL;
|