Repository navigation
feat(channels): auto-join existing starter channels on first run - #2029
TheSentinel454 wants to merge 2 commits into
Conversation
On the first session in a community, join the public general, welcome-everyone and announcements channels it already has. Joins use the existing channel-lifecycle Join one at a time, never create a channel, and are marked done per viewer and community only after a pass without a failed join. 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>
…oreground actions A starter lookup that hit the directory read bound no longer counts as absent: the pass joins what it found but saves no done marker, so the next session looks again. Starter joins now run as background channel-lifecycle actions. The lifecycle owner keeps one action in flight; a background action waits for any other, and a foreground action waits only for a background one (still honoring its own signal), instead of failing as busy. Two foreground actions still refuse as before. 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>
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Carl, an automated reviewer, commenting via Wes's GitHub account.
Verdict: not ready. The approach needs a decision before more polish. Reviewed at 0fb38458; I re-checked after the push that landed mid-review. That commit fixes two earlier problems: a partial directory read now holds back the done marker, and starter joins wait on the lifecycle owner instead of making a person's own Join/Leave/Archive fail as busy. The queue change in channel-lifecycle.ts looks correct to me.
Still open, each explained inline:
- Two owners for one behavior. This overlaps #1624 / block/buzz#8594 (owner-controlled
auto_join). - Wrong trigger. It runs whenever this device has no marker, not when someone is new to the community, so it re-joins channels people left.
- The parity claim is inaccurate in the docs and the PR body.
- Optional items: permanent refusals retry forever, and it joins every exact-name match. Desktop, mobile and #1624 also open a starter channel for new members; this pass never does.
Validation gaps: this was a source-only review; I ran nothing. The tests fake both searchPublic and lifecycle Join, so the guest role, roster confirmation and the no-navigation behavior are still unverified against a live relay (the PR notes this too).
To merge: a human decision on whether this PR or #1624's auto_join owns default-channel joins. If this PR stays, gate the pass on a fresh admission and correct the parity wording.
|
|
||
| /** The public channels a community already has that every new member joins. | ||
| * Never created here: a community without one simply has fewer. */ | ||
| export const STARTER_CHANNELS = [ |
There was a problem hiding this comment.
Blocking, needs a decision: this overlaps the owner-controlled auto_join design. #1624, together with block/buzz#8594, lets owners choose which channels new members join. The relay adds new members at invite claim, and the client runs a once-per-admission fallback. This list joins by hard-coded name no matter what, so if an owner turns auto-join off for #general, buzz-app still joins everyone to it. If both PRs land, two background 9021 passes start when the session is ready, each with its own marker and retry rules.
Fix: choose one owner for this behavior. If auto_join is the direction, drop this PR, and have owners (or a one-time relay migration) flag the existing general / welcome-everyone / announcements channels. If this PR is the direction, #1624's fallback needs reconciling with it.
| ): Promise<void> { | ||
| const { channels, channelLifecycle: lifecycle } = session; | ||
| if (!lifecycle.available || !channels.searchPublic) return; | ||
| if (readView<boolean>(scope, DONE_KEY, false)) return; |
There was a problem hiding this comment.
Blocking: this runs whenever the device has no marker, not when someone is new. The marker is device-local view state, so the pass runs on the upgrade, then again on every new device or browser profile and after cleared storage. Each run re-joins channels the person left on purpose. It also re-adds people an admin removed from an open channel, because the relay doesn't count a soft-removed row as a member (decide_put_user in block/buzz crates/buzz-relay/src/handlers/channel_authz.rs). The PR body's "their first session after this ships" understates this.
Every comparable path is scoped to a fresh admission:
- feat(channels): open the default channel for new members and add the auto-join switch #1624 writes its
pendingmarker only when the invite claim returnsjoined. - feat(relay): auto-join new community members to default channels buzz#8594 joins at claim time.
- Mobile runs after an invite claim.
- Desktop runs when onboarding completes, and feat(desktop): auto-join an existing #announcements as a starter channel buzz#8795 says an app update doesn't join existing users.
Fix: start the pass only from a fresh admission, for example a pending marker written when the claim reports joined. A missing marker then means there's nothing to do.
| ); | ||
| } catch { | ||
| signal.throwIfAborted(); | ||
| incomplete = true; |
There was a problem hiding this comment.
Optional: permanent refusals retry forever. A PublishRejected, or "Only active public channels can be joined", sets incomplete, so the pass reruns on every reconnect. That means every return to the service.ts ready path, not once per launch, and each run reads the whole public directory again. #1624's settles() already separates settled outcomes from retryable ones; reusing that split would stop the loop.
| }); | ||
| // A directory read that hit its bound cannot say the starter is absent. | ||
| if (partial) incomplete = true; | ||
| for (const channel of found) { |
There was a problem hiding this comment.
Optional: this joins every exact-name match. Channel names aren't unique on the relay (the only unique indexes are on the channel id and the DM hash), so a community with two open general streams gets both joined. Desktop (find) and mobile (firstOrNull) join only the first match. Stopping after the first match would keep parity.
|
|
||
| On first run in a community, the session also joins the public starter channels | ||
| the community already has: `general`, `welcome-everyone` and `announcements` | ||
| (`starter-channels.ts`, mirroring the desktop's `ensure_starter_channels`). Once |
There was a problem hiding this comment.
Inaccurate parity claim. On block/buzz main (f3a7bab098), the desktop and mobile starters are only general and welcome-everyone (desktop/src-tauri/src/commands/channels.rs, mobile/lib/app.dart), and both hosts create them when they're missing. announcements exists only in block/buzz#8795, which is still open and also limits desktop to new users. Please describe this as buzz-app's own behavior, or cite #8795 once it merges. The PR body needs the same fix: it says this "matches what Buzz desktop and mobile do" and still says the marker requires that "every join succeeded".
wpfleger96
left a comment
There was a problem hiding this comment.
🤖 Automated review from Will's agent team at head 0fb38458b7f6519afb6c94651dad86118f8494cb. Blocking: two correctness problems need fixing before merge. Two of us reviewed the source independently. There was no live relay run.
Blocking: a queued lifecycle action survives a cache clear
The new wait loop in run() (src/features/relay/channel-lifecycle.ts:371-393) runs before owned(). A waiting action therefore has no entry in controllers, and run() never checks epoch. So clear() (:549-555) and cancel(ids) can't reach it.
Here's how it fails:
- Starter join A holds the gate.
- The person submits Leave, Archive or Delete B, which queues behind A.
- A cache clear aborts A.
- A's
finallyreleases the gate. B wakes up and enters a freshowned()scope with a new controller. - B re-authorizes against the still-live session and publishes after the clear.
Before this PR, B would have failed as busy, so this is a new path. It breaks the cancellation contract in docs/channels.md ("Cancellation/cache clear/session replacement fence late results"). The service aborts A's connection signal, not B's dialog signal.
Fix: make cancellation cover run() from entry, including the wait. For example, register the waiter's controller with its channel id before waiting, or capture epoch at entry and refuse to proceed if it changed. Add tests that queue B behind a held A, run clear() and cancel([id]), release A, and assert that B never signs or publishes.
Blocking: a missing device marker is treated as a new member
The done marker is only device-local view state (starter-channels.ts:45, written at :79). So the pass runs again on every new device and browser profile, and after storage is cleared, not just once on upgrade. Each run silently re-joins channels the person left on purpose.
It also re-adds people an admin removed from an open channel. The relay's self-join check reads only active rows (decide_put_user in block/buzz crates/buzz-relay/src/handlers/channel_authz.rs), so a soft-removed member is just a non-member who may join. The pass also runs for communities the person only connects to in the background, without opening them.
Fix: gate the pass on actual admission. The invite claim in src/features/communities/join.ts already separates joined from already_member. Record intent only on joined, and treat missing intent as "do nothing". Then a missing marker can never cause a join.
Non-blocking
- Parity wording.
docs/relay-queries.md:384says this mirrors desktop'sensure_starter_channels, and the PR body says it matches desktop and mobile. On block/buzzmain, desktop (desktop/src-tauri/src/commands/channels.rs) and mobile (mobile/lib/app.dart) both use onlygeneralandwelcome-everyone, and both create missing starters.announcementsexists only in the open block/buzz#8795. Please describe this as buzz-app's own behavior. The body's "every join succeeded" also predates the partial-read rule. - Overlap with owner-controlled
auto_join(#1624, block/buzz#8594). If an owner turnsauto_joinoff for#general, this hard-coded pass still joins everyone to it. If both land, two 9021 passes run when the session is ready. Pick one owner for default-channel joins before both ship. - Every exact-name match gets joined. Channel names aren't unique, so two open
generalstreams both get joined. Desktop and mobile take only the first match. - Permanent refusals retry forever. A
PublishRejected, or "Only active public channels can be joined", setsincomplete. The pass then reruns, and re-reads the public directory, on every return to the ready path. - A starter join can jump the queue. When a foreground action is waiting on a background one, the next starter join can claim the gate first, because waiters aren't tracked. The doc comment says background work "yields to every other action", but here the person just waits longer. The cancellation fix above is a natural place to fix this too.
What holds
- No marker is written after a failed join, a thrown read, a partial directory read, or an abort. A missing starter correctly counts as complete.
- Cache-only sessions and sessions without a lifecycle writer return before any read or marker write.
- The gate is released in
finallyon every exit. A caller that aborts while waiting is rejected without publishing. We found no deadlock: no action holdsinflightwhile waiting, and two foreground actions still refuse as before. - Reset and disposal stop the active join. The pass uses a bare 9021 and doesn't create channels, override roles, or navigate.
CI at this head is red only on WebKit shard 3/14: message-actions.spec.mjs:15, "coordination keeps its first line visible throughout expansion and collapse". That test doesn't touch starter channels or the channel lifecycle, so it looks unrelated. It needs a rerun.
## Summary Adds **default channels**: relay owners and admins can flag channels that every new community member is automatically joined into. **Setting (contract)** - New `channels.auto_join` column (migration `0080`, constant default `false`, so no table rewrite). - kind:9002 accepts `["auto_join", "true"|"false"]`. Authorization comes from the actor's **relay** roster role (owner/admin), not a channel role, so the actor doesn't need to be a channel member. API tokens need `AdminChannels`. - Only open, unarchived `stream` channels qualify. This is checked under the channel row lock, so one event can open and flag a channel in one step, and a concurrent archive can't slip past the check. - kind:39000 carries a bare `auto_join` tag only while the setting is effective (on, open and unarchived). Every change posts one `auto_join_changed` kind:40099 notice. - SDK `build_update_channel(.., auto_join)`, `buzz channels update --auto-join <true|false>`, a NOSTR.md "Default channels" section, and a NIP-CL tag-table row. **Behaviour** - kind:9021's body is pulled out into `join_channel_as_self`, so auto-join uses the same path as a self-join: the channel's `join_role` applies, and it emits `member_joined` 40099, 39000/39002 and 44100. - `auto_join_default_channels` runs inline right after a member is added at three sites: the v2 invite claim, the v1 invite claim, and kind:9030 add. It does not run on login or reconnect, so a later kind:9022 leave stands. - Best effort: each failure is logged and counted (`buzz_auto_join_failures_total`) but never retried, because the roster membership has already committed. This deliberately departs from Review-Proven Rule 1 and is documented in NOSTR.md. `buzz-admin add-member` writes the roster row only. **Communities without a membership roster** (`BUZZ_REQUIRE_RELAY_MEMBERSHIP=false`) - These have no invite claim or kind:9030 to hook, so the three sites above join nobody there. Instead, after the AUTH `OK`, the relay joins a person to every effective default channel they have **never had a membership row in** (`auto_join_never_joined_channels`, spawned so admission never waits on it). - The channel's own membership history is the once-only record, so this adds no new table, column or migration. A kind:9022 leave or a kind:9001 removal keeps the row with `removed_at`, so the person stays out. A new device or another relay pod finds the row and does nothing. `put_first_member` checks the row again under the `put_member` locks, so two admissions racing each other join once. - A channel flagged later is joined on each person's next admission. A failed membership write leaves no row, so the next admission retries it. - Each channel's join holds the session's NIP-FI effect permit, so once the socket closes or its admission expires, no further channels are joined. - Agents are skipped: any socket with an admitted owner (a NIP-OA tag, a NIP-FI agent token, or a stored `agent_owner_pubkey`). - The kind:9021 side effects moved into `announce_self_join`, so both join paths emit the same `member_joined`, 39000/39002 and 44100. **Review follow-ups** - `get_accessible_channels` now selects `auto_join`. Without it, every call failed with `ColumnNotFound`. - The final authorization branch of the 9002 validator now refuses by default. `list_auto_join_channels` filters to `stream` channels itself. ## Testing - Postgres-lane journeys: - `edit_metadata_postgres_tests`: who may set `auto_join`, the 40099 notice, the effective-state tag, and the refusals. - `claim_auto_joins_default_channels_once`: v2 and v1 claims, and leave then re-claim. - `adding_a_member_auto_joins_default_channels`: kind:9030 add. - Removing the auto-join call at any one of the three sites fails the matching test (checked by mutation). - `root_auth_joins_people_to_default_channels_without_a_roster`: through `handle_auth` on a roster-less community. A person is joined, a NIP-OA agent is not, and a leave holds when the person is admitted again. Mutations checked: removing the spawn, and dropping the agent check. Each check waits for that admission's own auto-join outcome (a skip reports through the same test hook as a finished task) and fails if none arrives. A third mutation drops the agent check and delays the task 3s; it still fails. - `admission_auto_join_stops_when_the_session_ends`: closing the socket before the first or second channel stops the remaining joins; a later admission joins what was skipped. Mutation checked: dropping the effect permit. - `first_member_put_joins_once_and_never_undoes_a_leave` (buzz-db): the person is joined once with the channel's `join_role` (`guest` on an announce channel), a second put does nothing, after a leave the channel is neither listed nor joined, and two racing puts write exactly once. Mutations checked: removing the row check, removing the channel lock, and removing the `NOT EXISTS`. - e2e `test_invite_claim_auto_joins_default_channel` against a local relay. - Unit coverage in the SDK and CLI builders and `group_metadata_tags`. ## What got simpler The roster-less path adds no schema and no new concept: it reuses the channel's membership history as its record. It also shares the kind:9021 join code. It replaces the per-device client workarounds, [buzz-app#2029](block/buzz-app#2029) and [buzz#8795](#8795), which are now closed. ## Related - Client half (buzz-app): block/buzz-app#1624 ## Demo Recorded against a local relay built from this branch with throwaway identities (Alice = owner, Bob = new member); no real accounts or hosted relays involved. Relay at 141aa01, client at buzz-app bb0d97d86 (browser dev build). **Owner turns on "Starter channel for new members" on two channels.** Alice creates `#general` and `#welcome`, then flips the switch on each (~0:25 and ~0:41). The switch greys out while the kind 9002 is in flight and only turns on once the republished 39000 carries the `auto_join` tag. https://github.com/user-attachments/assets/e590d343-11d3-4385-af43-2cfaf19f8468 **A brand-new member joins and lands in a starter channel.** Bob claims Alice's invite with empty storage; from ~0:18 he is reading `#welcome` with both `#general` and `#welcome` in the sidebar, and at 0:27 opens `#general` as a member with no Join prompt. The relay logged one `auto-joined default channel` line per flagged channel. https://github.com/user-attachments/assets/037a0aaa-b25d-474d-a316-8d25e5e54f89 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Matt Toohey <contact@matttoohey.com> Signed-off-by: Luke Tornquist <tornquist@squareup.com> Signed-off-by: tornquist <tornquist@squareup.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: Luke Tornquist <tornquist@squareup.com> Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Summary
On first run in a community, buzz-app now joins that community's public starter channels:
general,welcome-everyoneandannouncements. This matches what Buzz desktop and mobile do for new users. buzz-app had no starter-channel step before, so new people started with an empty sidebar.channels.searchPublic(name, { exact: true }). An existing, active, open stream channel is joined through the existing channel-lifecycle Join. A community without one of these channels just gets fewer joins.join_role. An announce channel (join_role = guest) makes the person a read-only guest.starter-channels-doneview-state marker is scoped toorigin:viewer. It's written only after a pass in which every join succeeded, so a failed pass retries next session. Leaving the community clears it along with the rest of its view state.Hook point:
src/features/relay/service.ts, right afterchannels.ensureList()on the ready path.session.tsis a FOUNDATION file, so it isn't touched.Behavior to know about: existing buzz-app users have no marker yet, so their first session after this ships runs one pass too. That joins them to whichever of the three channels they aren't in, including a channel they left on purpose. After that pass the marker is set and they aren't joined again.
What got simpler: nothing was removed. This adds one small owner module,
src/features/relay/starter-channels.ts, built only from existing public-search, lifecycle-join and view-state pieces, with no new protocol path.Testing
src/features/relay/starter-channels.test.ts(10 tests): joins existing starters one at a time and leaves other channels alone; never creates a channel and skips a missing starter; does nothing once the marker is set; keeps the marker per viewer and community; leaves no marker after a failed join; matches names case-insensitively; joins only active public streams; skips sessions that can't write; waits for the channel list; stops with the session and leaves no marker.13177209: the pre-commit checks (staged names, icons, biome--error-on-warnings) passed. The pre-pushcheck-push.mjstypes and related unit tests passed (26 files, 354 tests), and so did its--designcheck. The fullsrc/features/relay/vitest run passed (126 files, 2504 tests, 4 skipped).🤖 Generated with Claude Code