Skip to content

[buzz] Recover interrupted default-channel auto-joins - #9045

Merged
TheSentinel454 merged 10 commits into
mainfrom
flipper/default-channel-recovery
Nov 6, 2026
Merged

TheSentinel454 merged 10 commits into
mainfrom
flipper/default-channel-recovery

Conversation

@htian-dev

@htian-dev htian-dev commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Bug

Default channels are channels that new community members should join automatically. If the database could not list those channels—for example, because its connection pool was busy—an invite claim or an in-app admin add still admitted the member to the community. That member could then remain outside the default channels permanently.

Cause

Community admission was saved before channel joins started. If listing the channels failed, the relay logged the error and stopped. Retrying the invite or admin add did not help: the member already existed, so the relay skipped the joins.

Fix

Save a marker for unfinished channel joins in the same database transaction as admission. If listing or joining channels fails, keep that marker. Retry when the member next authenticates, claims the invite again, or is added again by an admin. One channel's failure does not stop attempts to join the others.

Retries preserve the existing membership rules:

  • A member who leaves a channel during recovery stays out of it.
  • Work from an earlier admission cannot add channels or clear pending work after the member is removed and admitted again. Each admission has a unique marker, checked under a database lock.
  • A reconnect still checks whether the member allows the original admin to add them to channels.
  • Once recovery finishes, later connections do not enroll the member in newly designated default channels. The operator command buzz-admin add-member continues to add community membership only.

Limits and rollout risks

Recovery needs one of those later requests; there is no background retry timer. This change does not repair members missed before it was installed.

Channel announcements and notifications keep their existing best-effort behavior. A failure after channel membership is saved can still lose an announcement; this change does not replay it.

Migration 0084 adds an optional UUID column to relay_members. Apply it before running the new code. Older relays ignore the column. The migration has a five-second lock timeout, so it fails rather than waiting indefinitely for its table lock. It adds no triggers, functions, procedures, rules, foreign keys, or data backfill.

@htian-dev
htian-dev force-pushed the flipper/default-channel-recovery branch from f749a96 to f61b350 Compare November 4, 2026 23:40
@htian-dev
htian-dev marked this pull request as ready for review November 4, 2026 23:58
@htian-dev
htian-dev requested a review from a team as a code owner November 4, 2026 23:58
@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 5, 2026
@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

🔐 Codex Security Review

Status: review required for the current range.

The current range is 4e28176f13957f4bb77f58c5b4aa0ad137017423...91ea7c292c6283307cad369c80ba551906abfd00.
A new review must complete for this exact range. When manual authorization
is required, a user with write access must comment exactly
@buzz-security-review 91ea7c292c6283307cad369c80ba551906abfd00 to authorize a new review.
Any previous review applies only to its recorded range.

matt2e added a commit that referenced this pull request Nov 5, 2026
## Problem

`test_invite_claim_auto_joins_default_channel`
(crates/buzz-test-client/tests/e2e_relay.rs) failed in "Relay and
PostgreSQL / Relay E2E" at :363 with "recv kind:44100 notification for
the default channel: Timeout". Two of the three reruns passed.

- run 41568808045 attempt 1 (rerun passed):
  https://github.com/block/buzz/actions/runs/41568808045/job/125701098947
- run 41657074260:
  https://github.com/block/buzz/actions/runs/41657074260/job/125997465078
- run 41716543775 attempt 1 (rerun passed):
  https://github.com/block/buzz/actions/runs/41716543775/job/126206390776

## Cause

The e2e relay runs without `BUZZ_REQUIRE_RELAY_MEMBERSHIP`, so every
human NIP-42 admission spawns `auto_join_never_joined_channels`
(handlers/auth.rs). The test connected the joiner and sent its kind:44100
REQ before minting and claiming the invite. The admission therefore
joined the joiner to the flagged channel before the claim did. The
claim's `auto_join_default_channels` then got `AlreadyMember` and emitted
nothing, so the admission notice was the only kind:44100.

All three relay logs show this. The joiner's "auto-joined default channel
on admission" appears 39, 52 and 106 ms after its NIP-42 auth, before
"relay member added via v2 invite". The claim logs no join for the
joiner.

When the admission notice reached the client before EOSE,
`collect_until_eose` consumed it, and the test discarded that result. The
test then waited 5 s for a live event that never came. When the notice
arrived after EOSE, the test passed, but on the admission path. Either
way, the test never covered the claim-time auto-join it is named for.

## Fix

Only the test changes. The joiner opens no WebSocket until the claim
returns. Over NIP-98, the claim awaits the join, the kind:39002 roster
and the kind:44100 before its 200. The test now:

- checks the roster before the joiner connects, so only the claim can
  have joined them;
- then connects and REQs kind:44100 `#p=joiner` with `since` taken
  before the claim (minus the file's usual 5 s margin);
- asserts that the events `collect_until_eose` returns hold exactly one
  notice for the channel, signed by the relay's NIP-11 `self` key.

Nothing depends on EOSE-vs-live ordering any more. The admission that
follows skips the channel, because the claim already added a member
row. No timeouts change, no retries are added, and no assertions are
loosened. The repeat-claim and cleanup steps are unchanged.

No open PR touches this test. Draft #9045 (default-channel recovery)
changes invites.rs, but not this test or this ordering.

## Validation

Local Linux workstation, fresh database, relay started by
scripts/start-relay-for-tests.sh. Redis was the host's own, and
`BUZZ_METRICS_PORT` was moved off 9102, which bloxlet owns. Each
iteration ran CI's `e2e_relay invite` selection (8 tests in parallel).

- Unmodified test: 12/60 failed at :363 with the CI message.
- Adversarial ordering, using temporary, uncommitted changes: a 500 ms
  sleep between connect and REQ, plus logging of what
  `collect_until_eose` returned. 20/20 failed with the CI signature, and
  each time `collect_until_eose` had returned the channel's notice. In
  the relay log, the admission join came 28 ms after auth, the REQ
  queued one event, and the claim emitted nothing.
- Fixed test: 100/100 passed. With the same 500 ms sleep: 30/30 passed,
  each with exactly one notice, from the claim. Four concurrent loops of
  25: 100/100 passed.
- Falsifiability, using a temporary relay build without the claim-time
  `auto_join_default_channels` call: the fixed test failed 10/10 at the
  roster assertion (`None`). The old test passed 18/20 against that
  build; its two failures were its own :363 flake.
- The full CI "Relay E2E tests" step list passed once.
- `cargo fmt --check`, `cargo clippy -p buzz-test-client --tests -D
  warnings`, `just file-size-check` and `git diff --check` are clean.

A green run doesn't prove the flake is gone. Hosted CI covers broad
validation.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: mtoohey <mtoohey@squareup.com>
(cherry picked from commit 303ea56e40227e2b44f5e2df221bdbf519473d08)

@joahg joahg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤖 Read the pending-token fencing (FOR SHARE under the channel write, removal/re-admission stale tokens) and the expand-only migration; logic looks sound. The PR is conflicting with main: main already has migrations/0083_channels_listed_name_unique.sql, so renumber this one to 0084 and bump the migration-count test when rebasing.

@TheSentinel454 TheSentinel454 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 Two findings at fa241cc, inline.

Comment thread crates/buzz-relay/src/handlers/auth.rs Resolved
matt2e added a commit that referenced this pull request Nov 5, 2026
…#9098)

## Summary

Three tests asserted on outcomes that depended on how fast something
unordered happened. The first depended on dial latency inside a 150 ms
acquire budget, the second on whether two aux-page replies landed in one
poll, and the third on whether NIP-42 admission auto-joined the joiner
before the invite claim did. Each test now sets up the state it asserts
before the racing actor can run: it fills the reader warm floor first,
pauses the writer's proxy after the first aux page, or claims the invite
before the joiner connects. Only tests change. No assertions are
loosened, and no sleeps, retries or timeouts are added.

### test(db): fill the reader warm floor outside the acquire budget
first

#### Problem

`reader_warm_at_production_budget_ends_partial_when_dials_stall` failed
on main at reader_pool_postgres_tests.rs:177, "one partial wave, then
one wave that opens nothing": left (15, 0, 5), right (15, 3, 7).
Postgres lanes run with no retries.
- main (d3a4909):
-
https://github.com/block/buzz/actions/runs/41693681008/job/126127000284

The test is unchanged on current main.

#### Cause

The test builds the reader pool with the production constructor, so
every warm acquire, dial included, gets the 150 ms
`READER_ACQUIRE_TIMEOUT`. It relayed the proxy's first 3 sessions and
stalled the rest, so it needed the first wave's 3 relayed dials (TCP
through the proxy, backend fork, SCRAM, `after_connect`) to finish
inside that budget. On a loaded runner (4 vCPUs, nextest test-threads=8)
none did. All 5 first-wave acquires missed, and the pass ended after one
empty wave as the product should, which gives (15, 0, 5). The warm pass
was correct, but the test's outcome depended on dial latency.

The session counts had a second race. The proxy accepts connections on
the test's current-thread runtime. If that thread is descheduled past
the budget, the timed-out acquires and the listener's readiness are
handled in the same tick. So `proxy.sessions() == 10`, checked right
after the pass, could miss a stalled dial still waiting to be accepted.

#### Fix

Fill the warm floor before stalling anything, as boot does.
`Db::read_pool_boot_ping_once` closes its probe connection, and sqlx's
floor maintenance then dials the 3 floor connections under its own 300 s
deadline, outside the acquire budget. The test waits for them with
`await_idle_floor`, then stalls every session from the recorded baseline
(`proxy.stall_from(proxy.sessions())`) instead of a hardcoded 3. Wave 1
now holds the 3 idle connections without dialing, and its 2 dials stall.
Wave 2's 5 dials stall, so it makes no progress and the pass ends. That
gives (15, 3, 7) at any dial latency. The session count is now relative
to the baseline and asserted once, after the existing settle. That
single check covers both "the pass stops dialing" and "nothing
re-dials". The report assertion, the `elapsed < 2s` bound and the
production pool are unchanged. Only the test changes.

The setup still needs one of the boot ping's five attempts (spaced over
about 4 s by `READER_BOOT_PROBE_BACKOFF`) to dial within 150 ms, the
same exposure as `reader_boot_ping_warms_pool_to_min_connections`.
Before, all 3 concurrent first-wave dials had to make it on their first
try.

#### Validation

Local Linux workstation, on a private postgres:17-alpine. The test
processes and Postgres shared 4 pinned CPUs, with 8 parallel workers.
"Before" is the test as on main.
- CPU stress, no busy loops: before, 2/40 failed with (15, 0, 5) at
:177, the CI signature. After, 120/120 passed.
- Plus 4 busy loops on those CPUs: before, 80/80 failed with (15, 0, 5).
After, 80/80 passed. In 80 more instrumented runs (all passed), the boot
ping retried 16 times: 15 runs needed 2 attempts and 1 needed 3.
- Adversarial latency, using a temporary, uncommitted proxy hook that
delays chosen sessions' first server reply by 200 ms (past the budget),
10 runs each:
- every relayed session slow: before, 10/10 (15, 0, 5). After, 10/10
failed at "boot ping must reach the reader", the bound above.
- the first 4 sessions slow: before, 10/10 (15, 0, 5). After, 10/10
passed, with the ping succeeding on its 5th attempt.
- every session but the first slow: before, 10/10 (15, 1, 9). After,
10/10 passed.
- Production mutations, each of which fails the test: dropping the
no-progress break gives (15, 3, 27). A 2 s acquire budget fails the
elapsed bound at 4.0 s. Closing the warmed connections instead of
releasing them leaves the floor at 0 idle.
- All 7 `reader_pool_postgres_tests` pass. `cargo fmt --check`, `cargo
clippy -p buzz-db --all-targets -- -D warnings` and `git diff --check`
are clean.

A green run doesn't prove the flake is gone. Hosted CI covers broad
validation.

**Rebased onto origin/main `64d12e297` (this branch):** Linux
workstation, private `postgres:16` container with the desired-state
schema applied as in the PostgreSQL lane (pgschema plus the reconcile
script).

- `cargo test -p buzz-db`: 191 lib, 21 `observability_source` and 5 doc
tests passed.
- `cargo test -p buzz-db --lib reader_pool_postgres_tests -- --ignored
--test-threads=1`: 7/7 passed.
`reader_warm_at_production_budget_ends_partial_when_dials_stall` alone:
20/20 passed.
- `cargo clippy -p buzz-db -p buzz-relay -p buzz-test-client
--all-targets -- -D warnings`, `cargo fmt -p buzz-db -p buzz-relay -p
buzz-test-client -- --check` and `git diff --check` are clean.

### test(relay): hold the writer's second aux page too

#### Problem

`thread_window_batch_discards_earlier_windows_after_revocation` failed
on main at failure_postgres_tests.rs:55, "barrier must precede closure
completion": left 2, right 1. Postgres lanes run with no retries.
- main (d3a4909):
-
https://github.com/block/buzz/actions/runs/41693681008/job/126127000284

The test and the code it covers are unchanged on current main.

#### Cause

This is the race #8740 (322fdbf) fixed for the replica test only.
`pause_after_aux_page` polls the request until a `thread_window_aux`
span closes, then expects exactly one closed. `query` runs the closure's
two aux hops (reactions and edits, then their deletions) back to back,
one span each. On the writer, each hop is a pool acquire with its idle
ping, BEGIN, two SET LOCALs, the SELECT and ROLLBACK. #8740 left the
writer callers on `None`, so only the request returning Pending on a
reply that had not arrived yet held hop 2. sqlx tries each read before
it waits for readiness, and tokio leaves the socket marked readable
after the previous reply, so a reply Postgres has already sent is
consumed in the same poll. If each of hop 2's replies lands before the
test thread reads it, for example when a loaded runner deschedules that
thread after its writes, hop 2 closes in the poll that closed hop 1 and
the barrier counts 2. `assert_aux_access_change`, behind the grant and
revocation tests, uses the same `None` path.

#### Fix

`proxied_state` gives the paused request a copy of the fixture's state
whose production writer connects through a `PausingProxy`.
`production_db_at` is `production_db` with a URL override. `AuxPages`
pauses the proxy when hop 1's span closes. That runs inside the
request's poll, before hop 2's first bytes are written, so hop 2 stays
pending however fast Postgres answers. Each test resumes the proxy after
its access change, just before `request.await`. Only the request uses
the proxied state: `assert_aux_access_change` reads through the
fixture's direct `state.db` while the request is held, and through the
proxy that read would deadlock. No caller passes `None` any more, so the
`Option` goes. Only tests change. No assertions are loosened, and no
sleeps, retries or timeouts are added.

Draft #8991 (BUZZ-512) also edits `AuxPages` and `pause_after_aux_page`,
adding a waker for its detached HTTP handler. It also moves these two
callers onto a proxy, through its own `proxy_request_pool`, which
replaces `f.state.db` with a raw pool. Rebasing it onto this keeps its
waker on the non-`Option` proxy field and can use `proxied_state` for
those two callers.

#### Validation

Local Linux workstation, Postgres 17 in docker, one test per process as
nextest runs them. "Before" is the test as on main.
- Adversarial ordering, using a temporary, uncommitted change as in
#8740: after the barrier, ten more polls of the request over 500 ms,
then the barrier's count assertion. Before, the batch, grant and
revocation tests failed 30/30 at that assertion with left 2, right 1,
the CI signature, and the replica test passed 10/10. After, 40/40
passed.
- Natural contention: the three tests and a private postgres:17-alpine
shared 2 pinned CPUs with 4 busy loops, 3 processes at a time. Before,
450/450 passed, so the same-poll burst did not occur locally. After,
450/450 passed.
- Production mutation: skipping `query_batch`'s final implicit
access-set check fails all three tests with 200 instead of 503.
- All 19 `api::bridge::thread_window` Postgres tests pass, run serially
against one database. `cargo fmt --check`, `cargo clippy -p buzz-relay
--all-targets -- -D warnings` (1.98.1), `git diff --check` and
`check-postgres-test-discovery.py` are clean.

Running several of these tests in one libtest process can fail them at
:41 with no aux span seen, on main too. tracing-core builds a new
callsite's interest from the registering thread's default while only one
scoped dispatcher exists. nextest runs one test per process, so CI is
not exposed. The runs above use one process per test.

A green run doesn't prove the flake is gone. Hosted CI covers broad
validation.

**Rebased onto origin/main `64d12e297` (this branch):** same database
setup as above.

- Each of the 19 `api::bridge::thread_window` Postgres tests ran in its
own process and its own database cloned from the desired-state template,
as nextest runs them: 19/19 passed.
- The batch, grant, revocation and replica tests, 5 more runs each in
the same way: 20/20 passed.

### test(relay): claim the invite before the joiner connects

#### Problem

`test_invite_claim_auto_joins_default_channel`
(crates/buzz-test-client/tests/e2e_relay.rs) failed in "Relay and
PostgreSQL / Relay E2E" at :363 with "recv kind:44100 notification for
the default channel: Timeout". Two of the three reruns passed.

- run 41568808045 attempt 1 (rerun passed):
-
https://github.com/block/buzz/actions/runs/41568808045/job/125701098947
- run 41657074260:
-
https://github.com/block/buzz/actions/runs/41657074260/job/125997465078
- run 41716543775 attempt 1 (rerun passed):
-
https://github.com/block/buzz/actions/runs/41716543775/job/126206390776

#### Cause

The e2e relay runs without `BUZZ_REQUIRE_RELAY_MEMBERSHIP`, so every
human NIP-42 admission spawns `auto_join_never_joined_channels`
(handlers/auth.rs). The test connected the joiner and sent its
kind:44100 REQ before minting and claiming the invite. The admission
therefore joined the joiner to the flagged channel before the claim did.
The claim's `auto_join_default_channels` then got `AlreadyMember` and
emitted nothing, so the admission notice was the only kind:44100.

All three relay logs show this. The joiner's "auto-joined default
channel on admission" appears 39, 52 and 106 ms after its NIP-42 auth,
before "relay member added via v2 invite". The claim logs no join for
the joiner.

When the admission notice reached the client before EOSE,
`collect_until_eose` consumed it, and the test discarded that result.
The test then waited 5 s for a live event that never came. When the
notice arrived after EOSE, the test passed, but on the admission path.
Either way, the test never covered the claim-time auto-join it is named
for.

#### Fix

Only the test changes. The joiner opens no WebSocket until the claim
returns. Over NIP-98, the claim awaits the join, the kind:39002 roster
and the kind:44100 before its 200. The test now:

- checks the roster before the joiner connects, so only the claim can
have joined them;
- then connects and REQs kind:44100 `#p=joiner` with `since` taken
before the claim (minus the file's usual 5 s margin);
- asserts that the events `collect_until_eose` returns hold exactly one
notice for the channel, signed by the relay's NIP-11 `self` key.

Nothing depends on EOSE-vs-live ordering any more. The admission that
follows skips the channel, because the claim already added a member row.
No timeouts change, no retries are added, and no assertions are
loosened. The repeat-claim and cleanup steps are unchanged.

No open PR touches this test. Draft #9045 (default-channel recovery)
changes invites.rs, but not this test or this ordering.

#### Validation

Local Linux workstation, fresh database, relay started by
scripts/start-relay-for-tests.sh. Redis was the host's own, and
`BUZZ_METRICS_PORT` was moved off 9102, which bloxlet owns. Each
iteration ran CI's `e2e_relay invite` selection (8 tests in parallel).

- Unmodified test: 12/60 failed at :363 with the CI message.
- Adversarial ordering, using temporary, uncommitted changes: a 500 ms
sleep between connect and REQ, plus logging of what `collect_until_eose`
returned. 20/20 failed with the CI signature, and each time
`collect_until_eose` had returned the channel's notice. In the relay
log, the admission join came 28 ms after auth, the REQ queued one event,
and the claim emitted nothing.
- Fixed test: 100/100 passed. With the same 500 ms sleep: 30/30 passed,
each with exactly one notice, from the claim. Four concurrent loops of
25: 100/100 passed.
- Falsifiability, using a temporary relay build without the claim-time
`auto_join_default_channels` call: the fixed test failed 10/10 at the
roster assertion (`None`). The old test passed 18/20 against that build;
its two failures were its own :363 flake.
- The full CI "Relay E2E tests" step list passed once.
- `cargo fmt --check`, `cargo clippy -p buzz-test-client --tests -D
warnings`, `just file-size-check` and `git diff --check` are clean.

A green run doesn't prove the flake is gone. Hosted CI covers broad
validation.

**Rebased onto origin/main `64d12e297` (this branch):** the relay was
built from this branch with `--profile ci` (its relay code is identical
to main) and run with CI's environment. It used a fresh desired-state
database with the deployment community seeded, the host's Redis, and a
private MinIO-compatible container (the image docker-compose pins) for
the startup git probe. `BUZZ_METRICS_PORT` was moved off 9102.

- CI's `e2e_relay invite` selection: 30/30 iterations passed, 8 tests
each.
- CI's full "Relay E2E tests" step list, once: 10 of 11 steps passed.
`test_nip29_announce_channel_wire` failed at e2e_relay.rs:3862: the
kind:39000 metadata still carried `auto_join` right after the relay
owner cleared it. This branch doesn't touch that test or any relay code.
Run alone against the same relay, that test failed 8 of 20 times.
Re-running the whole step against one database isn't meaningful, because
`test_nip29_announce_channel_wire_overlong_name_is_refused_before_storage`
reuses a fixed channel name.

## Coordination

- #8991 ("BUZZ-512: Track session application and drain") edits
`AuxPages` and `pause_after_aux_page` in
`crates/buzz-relay/src/api/bridge/thread_window/postgres_tests/failure_postgres_tests.rs`,
and `git merge-tree` shows a conflict there. #8991 already conflicts
with main in 15 files, so it needs a rebase anyway. The second commit's
message describes that rebase: keep #8991's waker on the non-`Option`
proxy field, and use `proxied_state` for the two callers it moves onto a
proxy.
- #6642 and #7569 touch `crates/buzz-test-client/tests/e2e_relay.rs` and
merge cleanly with this branch.
- #7852 touches `thread_window/postgres_tests.rs` and `e2e_relay.rs`. It
already conflicts with main, and this branch adds no new conflicted file
to it. Older open PRs that touch `e2e_relay.rs` also already conflict
with main, and none gains a new conflict from this branch.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Signed-off-by: mtoohey <mtoohey@squareup.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot removed the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 5, 2026
@htian-dev
htian-dev deployed to codex-review November 5, 2026 23:21 — with GitHub Actions Active
@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 5, 2026
htian-dev and others added 8 commits November 6, 2026 10:37
Record pending joins atomically with roster admissions. Resume them on
repeated claims/adds and authenticated connections, preserving leaves,
original actor policy and admission generations. Keep client discovery
pending until its required state and notification have been written.

Co-authored-by: Flipper <1ef64189596befbc8b93f9021bff35ae5d02b155d9e05fc124bca7349927d597@buzz.block.builderlab.xyz>
Signed-off-by: Helen Tian <htian@squareup.com>
Keep healthy default-channel joins progressing when another channel fails.
Use the persisted admission identity for retried notices, and serialize
notice insertion with membership removal and admission generation changes.
Cover failures, later builder clocks, leaves, kicks and roster removal.

Co-authored-by: Flipper <1ef64189596befbc8b93f9021bff35ae5d02b155d9e05fc124bca7349927d597@buzz.block.builderlab.xyz>
Signed-off-by: Helen Tian <htian@squareup.com>
Date first recovery notices at emission time so startup subscription
cutoffs include them. Reuse the first persisted notice as the per-channel
admission identity, checking its tags under the membership lock to keep
later retries idempotent without backdating newly recovered notices.

Co-authored-by: Flipper <1ef64189596befbc8b93f9021bff35ae5d02b155d9e05fc124bca7349927d597@buzz.block.builderlab.xyz>
Signed-off-by: Helen Tian <htian@squareup.com>
Keep pending admission work and safe membership retries. Use the existing announcement path only for newly joined channels; remove discovery replay and admission-specific notice persistence.

Co-authored-by: Flipper <1ef64189596befbc8b93f9021bff35ae5d02b155d9e05fc124bca7349927d597@buzz.block.builderlab.xyz>
Signed-off-by: Helen Tian <htian@squareup.com>
Opt into durable default-channel work only from the existing kind:9030 path. Keep add_relay_member roster-only for buzz-admin and verify that authentication does not enroll its additions.

Co-authored-by: Flipper <1ef64189596befbc8b93f9021bff35ae5d02b155d9e05fc124bca7349927d597@buzz.block.builderlab.xyz>
Signed-off-by: Helen Tian <htian@squareup.com>
Use the explicit default-channel admission entry for the joiner while keeping the owner roster-only. Preserve the archive lock barrier and all membership and notice assertions.

Co-authored-by: Flipper <1ef64189596befbc8b93f9021bff35ae5d02b155d9e05fc124bca7349927d597@buzz.block.builderlab.xyz>
Signed-off-by: Helen Tian <htian@squareup.com>
Keep snapshot fan-out intentionally unused in the isolated archive-race fixture, matching existing seed helpers and satisfying warnings-as-errors builds.

Co-authored-by: Flipper <1ef64189596befbc8b93f9021bff35ae5d02b155d9e05fc124bca7349927d597@buzz.block.builderlab.xyz>
Signed-off-by: Helen Tian <htian@squareup.com>
The recovery check runs on every AUTH admission, including roster
communities that spawned nothing before. Reading it through the writer
added a primary-pool acquire per connection. Route the AUTH read through
the bounded replica path, as is_relay_member is. A stale answer only
defers the work or carries an old token, and every write re-checks the
token on the writer. Invite claims and admin adds keep the writer read,
because they just committed the token.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Helen Tian <htian@squareup.com>
@htian-dev
htian-dev force-pushed the flipper/default-channel-recovery branch from 7dc36ff to b920d00 Compare November 5, 2026 23:48
@htian-dev
htian-dev deployed to codex-review November 5, 2026 23:51 — with GitHub Actions Active
@github-actions github-actions Bot removed the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 5, 2026
htian-dev and others added 2 commits November 6, 2026 13:39
Assert that migration 0083 has a successful ledger entry after startup migrations complete. Later migrations, including default-channel recovery 0084, must not invalidate the listed-name index tests.

Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: Helen Tian <htian@squareup.com>
Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: Helen Tian <htian@squareup.com>
@htian-dev
htian-dev deployed to codex-review November 6, 2026 02:58 — with GitHub Actions Active
@github-actions github-actions Bot added codex-security-review-current The posted Codex security review matches its recorded range. and removed codex-security-review-current The posted Codex security review matches its recorded range. labels Nov 6, 2026
@htian-dev
htian-dev enabled auto-merge (squash) November 6, 2026 03:31
matt2e added a commit that referenced this pull request Nov 6, 2026
Every community gets at most one landing channel: the open stream a person
joins first when they enter the community, and where the community's
custom welcome message is published. This implements the relay half of
the landing-channel plan (#note:8de4748c §4–§6) and its wire contract in
NOSTR.md.

Storage (migration 0084): two nullable columns on `communities`,
`landing_channel_id` and `welcome_message`. They have no default, no
backfill and no foreign key, so the deployed relay is unaffected. The
row is the single source of truth, so "exactly one" holds by
construction.

Advertisement:
- NIP-11 advertises the `buzz-landing-channel` extension and
  `landing_channel`. NIP-11 is the authoritative read; it reads the icon
  and the designation in one query.
- The landing channel's relay-signed kind:39000 carries a bare
  `["landing"]` tag, plus `["welcome", text]` while a custom message is
  set. This is a projection of the column. A move re-signs the previous
  landing channel's 39000 without both tags. Plan §3.2 said "no landing
  marker tag"; that no longer holds, and NOSTR.md and NIP-CL say so.

kind:9002 `["landing","true"]` and `["welcome", text|""]`:
- Both are relay owner/admin tags, like `auto_join`, and API tokens need
  the `AdminChannels` scope.
- Validation follows the plan's exact receipt texts.
- The write decides eligibility under the channel row lock, on the state
  the event leaves behind: an open, unarchived stream with no TTL.
- The client's full-state form `[landing, welcome]` is accepted both on
  the current landing channel (idempotent, not reported as a move) and
  as a move.
- `welcome` alone is accepted only on the landing channel.

Protections: the landing channel cannot be archived, made private, given
a TTL or deleted. The checks are backstops in `set_archived_on`,
`update_channel_on` and `soft_delete_on`, so every path is covered,
including the 9002 lock-time path. kind:9008 also gets a pre-check,
because its side effect runs after the event is stored.

Adoption (`ensure_landing_channel`):
- Adopt the oldest eligible `general`, listed before unlisted.
- If there is none, create a listed `general` owned by the earliest relay
  owner, else the earliest admin.
- Do nothing when there is no steward, or when the name is held by an
  archived, forum or TTL `general`.
- A NULL designation is the retry record. Adoption runs after operator
  provisioning (both branches) and at every human AUTH.

Enrollment:
- The landing channel is joined first, then the `auto_join` defaults, as
  kind:9021 self-joins. Agents are not joined.
- Roster mode:
  - The invite claim and kind:9030 join the landing channel inside
    `auto_join_default_channels`.
  - AUTH recovers a person with no entry evidence since their
    `relay_members.created_at`, joining the landing channel and then the
    defaults. This replaces the roster-free-only spawn gate.
- Roster-free mode: AUTH joins a person with no evidence at all, then
  the never-joined defaults as before.
- Evidence (`has_entered_community`) is a stream or forum membership,
  excluding private channels the person created themselves (Solo and
  Bestie chats).
- `put_first_member_since` keeps a leave since admission standing.
- A landing join that fails for a reason other than a refusal skips the
  defaults, so the next AUTH retries both. `join_channel_as_self` now
  returns typed refusals so this can tell them apart.

Deliberate deviations and tensions:
- Boot-time convergence for the deployment community: main.rs spawns a
  best-effort ensure after AppState is built. That way a local demo
  (`ensure_deployment_community` plus `bootstrap_owner`) has a landing
  `general` before anyone connects. The plan said "never at startup".
  This is only the deployment community and is off the boot path; there
  is no sweep of other tenants.
- In a roster-free community with no relay owner or admin, there is no
  steward. A missing `general` is not created, and an eligible one is
  still adopted.
- The shared e2e community now has a steward-owned `general`, and every
  fresh key gets its 44100. Three e2e tests that took the first
  membership notice now match on the channel's `h` tag.
- This is independent of the unmerged #9045, as the plan's merge-order
  rules say. 0084 is the next free migration here. If #9045 lands first,
  its pending-work token replaces this change's evidence-gated
  roster-mode recovery of defaults.

Tests:
- buzz-db Postgres: designation moves and the idempotent full-state form,
  ineligible post-states, protections on every path, the adoption rule
  table, concurrent ensures, the entry-evidence table and the
  `put_first_member_since` table.
- buzz-relay: tag rules (unit); the 9002 authorization matrix with exact
  receipts; the 39000 projection, move re-emission and NIP-11;
  protections including 9008; provisioning; AUTH journeys in both modes.
- e2e: a landing journey on the shared relay.
- None of these have been run locally; only `cargo check` was. The
  Postgres and e2e suites run in CI.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>

@TheSentinel454 TheSentinel454 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 LGTM at 91ea7c2. Both earlier findings are fixed (0084 renumber, replica-routed AUTH read). Token fencing and leave preservation look right, and all five new Postgres tests ran green in CI's PostgreSQL lane.

@TheSentinel454
TheSentinel454 merged commit 39d5159 into main Nov 6, 2026
90 checks passed
@TheSentinel454
TheSentinel454 deleted the flipper/default-channel-recovery branch November 6, 2026 21:27

This branch was successfully deployed

1 active deployment
codex-review — 91ea7c29 Deployed Nov 6, 2026 by htian-dev via Run Codex Security Review #13373
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants