fix(desktop): skip a starter channel the relay will never create (#8911)

## Problem

On first run, the desktop app creates the `general` and
`welcome-everyone` starter channels at fixed ids when it can't find a
live one. Some refusals of that create are permanent. Any refusal
aborted setup **before it joined the new member to the starters that do
exist**:

| Starter state in the community | Relay reply to the create | Before |
|---|---|---|
| Another listed channel holds the name, archived ones included (with
#8782) | 400 `invalid: an open channel named "…" already exists` | setup
fails, not joined to `general` |
| An archived channel holds the fixed id | 400 `invalid: channel is
archived` (the relay checks this before duplicates) | setup fails |
| The channel at the fixed id was renamed or retyped | `duplicate` |
waits, then `starter channels created but metadata not yet available` |
| The channel at the fixed id was deleted | `duplicate`, and no metadata
is ever served | same |

Onboarding itself continues, because public starter failures are already
caught in `initializeStarterChannels`. The new member just never gets
joined to the other starter. With #8782 this happens to every new
desktop user in a community whose `welcome-everyone` or `general` is
archived.

## Fix

`ensure_starter_channels_inner` now skips a starter it can never create
or use, clears its pending-owner mark, and still joins the rest. Before
skipping after a permanent refusal, it reads the open directory again,
from the writer rather than a replica, once for all refused starters: a
live starter created after the first read is joined instead of skipped.
Any other refusal still fails setup. On the TS side,
`ensureStarterChannels` reports a skipped starter as `null` instead of
throwing. No caller reads those fields.

## Testing

- New tests run `ensure_starter_channels_inner` against a stub relay
that sends the relay's real reply shapes, one per row above, plus one
checking that other refusals still fail, and one where a live
`welcome-everyone` is listed only by writer-pinned reads (a lagging
replica) and gets joined. Each test fails when its guard is removed.
- Ran the real starter-setup code as a new user against a local relay
built from #8782, one community per row:
  - On `main`, rows 1, 2 and 4 fail.
- On this branch, all four join `general`, and each community still has
exactly one `welcome-everyone`.
  - A rerun is a no-op.
- `cargo test` (desktop Tauri, full), clippy, fmt, desktop `pnpm test`,
`tsc`, and biome all pass.

Related: #8782 (relay refuses taken names), #8807 (the same fix for
mobile). The archived-id case returns `channel is archived`, not
`duplicate`, so mobile needs to handle that reply too.

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

---------

Signed-off-by: Salman Mohammed <smohammed@squareup.com>
Co-authored-by: opus <f038cc09b32a776e7d95b65c267e9d851777ee2b6cdbd46a48edba01b1d10ebd@buzz.block.builderlab.xyz>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Salman Mohammed
2026-11-02 12:59:28 -05:00
committed by GitHub
parent 3ae2b6a377
commit f99807a79a
7 changed files with 409 additions and 21 deletions

View File

@@ -18,7 +18,9 @@ use crate::{
// the directory cursor, the not-modified hash, and member-count collection)
// lives in the `fetch` submodule to keep this file under the per-file line cap.
mod fetch;
use fetch::{compute_channels_hash, fetch_channels, DirectoryScope};
use fetch::{
compute_channels_hash, fetch_channels, fetch_open_directory_from_writer, DirectoryScope,
};
const STARTER_CHANNEL_NAMESPACE: uuid::Uuid = uuid::uuid!("3ce33bea-8f09-5f1b-9c85-8a7d2659e6b0");
@@ -230,6 +232,17 @@ fn is_duplicate_channel_rejection(error: &str) -> bool {
error.contains("relay rejected event:") && error.contains("duplicate: channel already exists")
}
/// A starter create the relay will refuse on every retry, with HTTP 400:
/// - another listed channel, archived ones included, holds its name:
/// `invalid: an open channel named "general" already exists`;
/// - an archived channel holds its deterministic id, which the relay checks
/// before duplicates: `invalid: channel is archived`.
fn is_permanent_starter_rejection(error: &str) -> bool {
error.starts_with("relay returned 400")
&& ((error.contains("invalid: an open channel named") && error.contains("already exists"))
|| error.contains("invalid: channel is archived"))
}
fn is_matching_starter_channel(channel: &ChannelInfo, spec: &StarterChannelSpec) -> bool {
normalize_channel_name(&channel.name) == normalize_channel_name(spec.name)
&& channel.channel_type == "stream"
@@ -237,12 +250,19 @@ fn is_matching_starter_channel(channel: &ChannelInfo, spec: &StarterChannelSpec)
&& channel.archived_at.is_none()
}
fn has_all_starter_channels(channels: &[ChannelInfo]) -> bool {
STARTER_CHANNELS.iter().all(|spec| {
channels
.iter()
.any(|channel| is_matching_starter_channel(channel, spec))
})
/// Every starter that was not skipped has a usable channel.
fn has_all_starter_channels(
channels: &[ChannelInfo],
skipped: &std::collections::HashSet<&str>,
) -> bool {
STARTER_CHANNELS
.iter()
.filter(|spec| !skipped.contains(spec.slug))
.all(|spec| {
channels
.iter()
.any(|channel| is_matching_starter_channel(channel, spec))
})
}
async fn ensure_starter_channel_memberships(
@@ -413,6 +433,14 @@ async fn ensure_starter_channels_inner(
let creator_pubkey = creator_keys.public_key().to_hex();
let mut starter_ids = Vec::with_capacity(STARTER_CHANNELS.len());
let mut created_ids = std::collections::HashSet::new();
// Starters whose deterministic id the relay already holds.
let mut duplicate_starters = Vec::new();
// Starters that can never be created here: their name or id belongs to a
// channel that cannot serve as the starter, such as an archived one. They
// are left out instead of blocking the remaining starters.
let mut skipped = std::collections::HashSet::new();
// Starters the relay refused for good, waiting on a second look.
let mut refused = Vec::new();
for spec in STARTER_CHANNELS {
if existing_channels
@@ -424,7 +452,6 @@ async fn ensure_starter_channels_inner(
let channel_uuid = starter_channel_uuid(&relay_scope, spec.slug);
let channel_uuid_string = channel_uuid.to_string();
starter_ids.push(channel_uuid_string.clone());
let builder = events::build_create_channel(
channel_uuid,
spec.name,
@@ -442,9 +469,40 @@ async fn ensure_starter_channels_inner(
}
Err(error) if is_duplicate_channel_rejection(&error) => {
state.mark_pending_owned_channel(&creator_pubkey, &channel_uuid_string);
duplicate_starters.push((spec, channel_uuid_string.clone()));
}
// A retry would be refused the same way. The name may still
// belong to a live starter the first read missed, such as one
// created since, so it is looked up again below.
Err(error) if is_permanent_starter_rejection(&error) => {
refused.push(spec);
continue;
}
Err(error) => return Err(error),
}
starter_ids.push(channel_uuid_string);
}
if !refused.is_empty() {
// Read from the writer: the relay refused the create against the
// writer's state, and a lagging replica could miss the same starter
// again.
let mut directory = fetch_open_directory_from_writer(state).await?;
for spec in refused {
match directory
.iter()
.position(|channel| is_matching_starter_channel(channel, spec))
{
Some(index) => {
let channel = directory.swap_remove(index);
existing_channels.retain(|existing| existing.id != channel.id);
existing_channels.push(channel);
}
None => {
skipped.insert(spec.slug);
}
}
}
}
for _ in 0..3 {
@@ -460,17 +518,37 @@ async fn ensure_starter_channels_inner(
existing_channels.push(channel);
}
}
if has_all_starter_channels(&existing_channels) {
// A live channel at the starter id that cannot serve as the starter
// (renamed or retyped) stays that way, so stop waiting on it.
for (spec, id) in &duplicate_starters {
if existing_channels
.iter()
.any(|channel| channel.id == *id && !is_matching_starter_channel(channel, spec))
{
state.clear_pending_owned_channel(&creator_pubkey, id);
skipped.insert(spec.slug);
}
}
if has_all_starter_channels(&existing_channels, &skipped) {
break;
}
tokio::time::sleep(std::time::Duration::from_millis(150)).await;
}
if !has_all_starter_channels(&existing_channels) {
if !has_all_starter_channels(&existing_channels, &skipped) {
existing_channels = fetch_channels(state, DirectoryScope::IncludeOpenDirectory).await?;
// A starter id the relay holds but still does not serve, even from
// the writer, most likely belongs to a channel this user cannot see,
// such as a deleted one.
for (spec, id) in &duplicate_starters {
if !existing_channels.iter().any(|channel| channel.id == *id) {
state.clear_pending_owned_channel(&creator_pubkey, id);
skipped.insert(spec.slug);
}
}
}
if !has_all_starter_channels(&existing_channels) {
if !has_all_starter_channels(&existing_channels, &skipped) {
return Err("starter channels created but metadata not yet available".to_string());
}

View File

@@ -49,6 +49,23 @@ async fn query_relay_all(
}
}
/// Every open channel's metadata, read from the writer so a lagging replica
/// cannot hide a channel the relay already holds. Membership is not resolved:
/// each channel is reported as not joined.
pub(super) async fn fetch_open_directory_from_writer(
state: &AppState,
) -> Result<Vec<ChannelInfo>, String> {
let events = query_relay_all(
state,
serde_json::json!({"kinds": [39000], "consistency": "strong"}),
)
.await?;
Ok(events
.iter()
.filter_map(|ev| nostr_convert::channel_info_from_event(ev, None, Some(false)).ok())
.collect())
}
/// Whether an open channel not yet in the real member set should still be
/// classified `is_member=true` via the pending-owner overlay. Pulled out of
/// `get_channels`'s open-channel branch so the exact `(d_tag, my_pubkey,

View File

@@ -38,3 +38,250 @@ async fn created_starter_channels_are_reported_when_metadata_is_unavailable() {
.collect();
assert_eq!(changed, expected);
}
const LIVE_GENERAL_ID: &str = "0b9f4c1e-6a51-4f7e-9d38-2f4b7f0e3c11";
const LIVE_WELCOME_ID: &str = "5d2a7e90-3c4b-4f18-8e6d-91a0b2c4d5e6";
/// A kind:39000 open stream, relay-signed like production.
fn channel_event(id: &str, name: &str) -> serde_json::Value {
let tags = vec![
nostr::Tag::parse(["d", id]).unwrap(),
nostr::Tag::parse(["name", name]).unwrap(),
nostr::Tag::parse(["t", "stream"]).unwrap(),
nostr::Tag::parse(["visibility", "open"]).unwrap(),
];
let event = nostr::EventBuilder::new(nostr::Kind::from_u16(39000), "")
.tags(tags)
.sign_with_keys(&Keys::generate())
.unwrap();
serde_json::to_value(&event).unwrap()
}
/// Runs starter setup against a fake relay whose directory lists a live
/// `general` this user has not joined. The relay refuses every channel create
/// with `create_refusal`: HTTP 400 for a rejected event, as the relay sends
/// it, or HTTP 200 with `accepted: false` for a duplicate. With
/// `renamed_starter`, a live channel under that name holds `welcome-everyone`'s
/// deterministic id, and is listed and served like any live channel. With
/// `writer_only_welcome`, a live `welcome-everyone` under another id is listed
/// only by reads pinned to the writer, as when a replica lags. Returns the
/// result, the kinds of the events the relay accepted, and whether the
/// deterministic id is still marked as pending-owned.
async fn ensure_with_unusable_welcome(
create_refusal: (axum::http::StatusCode, &'static str),
renamed_starter: Option<&'static str>,
writer_only_welcome: bool,
) -> (Result<Vec<ChannelInfo>, String>, Vec<u64>, bool) {
use std::sync::{Arc, Mutex};
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let state = crate::app_state::build_app_state();
let keys = Keys::generate();
*state.keys.lock().unwrap() = keys.clone();
*state.relay_url_override.lock().unwrap() =
Some(format!("http://{}", listener.local_addr().unwrap()));
let starter_id = starter_channel_uuid(
&relay_api_base_url_with_override(&state),
"welcome-everyone",
)
.to_string();
let general = channel_event(LIVE_GENERAL_ID, "general");
let welcome = channel_event(LIVE_WELCOME_ID, "welcome-everyone");
let renamed = renamed_starter.map(|name| channel_event(&starter_id, name));
let accepted = Arc::new(Mutex::new(Vec::new()));
let accepted_kinds = accepted.clone();
let router = Router::new()
.route(
"/query",
post(move |Json(filters): Json<serde_json::Value>| {
let filter = filters[0].clone();
let events = match (filter["kinds"][0].as_u64(), filter.get("#d")) {
// The open directory lists live channels only. A lagging
// replica misses the late `welcome-everyone`; the writer
// has it.
(Some(39000), None) => {
let from_writer = filter["consistency"] == "strong";
std::iter::once(general.clone())
.chain(renamed.clone())
.chain((writer_only_welcome && from_writer).then(|| welcome.clone()))
.collect()
}
// Metadata by id.
(Some(39000), Some(ids)) => renamed
.iter()
.filter(|event| ids.as_array().unwrap().contains(&event["tags"][0][1]))
.cloned()
.collect(),
_ => Vec::new(),
};
async move { Json(json!(events)) }
}),
)
.route(
"/events",
post(move |Json(event): Json<serde_json::Value>| {
let kind = event["kind"].as_u64().unwrap();
let (status, message) = create_refusal;
let response = if kind != 9007 {
accepted.lock().unwrap().push(kind);
(
status_ok(),
json!({"event_id": "0", "accepted": true, "message": ""}),
)
} else if status == status_ok() {
(
status,
json!({"event_id": "0", "accepted": false, "message": message}),
)
} else {
(status, json!({"error": message}))
};
async move { (response.0, Json(response.1)) }
}),
);
let server = tokio::spawn(async move { axum::serve(listener, router).await.unwrap() });
let mut changed = Vec::new();
let result = ensure_starter_channels_inner(&state, &mut changed).await;
server.abort();
let kinds = accepted_kinds.lock().unwrap().clone();
let still_pending = state.is_pending_owned_channel(&keys.public_key().to_hex(), &starter_id);
(result, kinds, still_pending)
}
fn status_ok() -> axum::http::StatusCode {
axum::http::StatusCode::OK
}
fn joined_live_general(channels: &[ChannelInfo]) -> bool {
channels
.iter()
.any(|channel| channel.id == LIVE_GENERAL_ID && channel.is_member)
}
#[tokio::test]
async fn a_starter_name_held_by_another_channel_is_skipped() {
let _serial = crate::relay_admission::TEST_SERIAL.lock().await;
let (result, accepted, _) = ensure_with_unusable_welcome(
(
axum::http::StatusCode::BAD_REQUEST,
"invalid: an open channel named \"welcome-everyone\" already exists",
),
None,
false,
)
.await;
let channels = result.expect("a taken starter name must not fail setup");
assert!(joined_live_general(&channels));
assert_eq!(accepted, vec![9021], "only the join to general");
}
#[tokio::test]
async fn a_starter_id_held_by_an_archived_channel_is_skipped() {
let _serial = crate::relay_admission::TEST_SERIAL.lock().await;
// The relay refuses any event aimed at an archived channel before its
// duplicate check.
let (result, accepted, _) = ensure_with_unusable_welcome(
(
axum::http::StatusCode::BAD_REQUEST,
"invalid: channel is archived",
),
None,
false,
)
.await;
let channels = result.expect("an archived starter must not fail setup");
assert!(joined_live_general(&channels));
assert_eq!(accepted, vec![9021], "only the join to general");
}
#[tokio::test]
async fn a_starter_id_held_by_a_renamed_channel_is_skipped() {
let _serial = crate::relay_admission::TEST_SERIAL.lock().await;
let (result, accepted, still_pending) = ensure_with_unusable_welcome(
(status_ok(), "duplicate: channel already exists"),
Some("intros"),
false,
)
.await;
let channels = result.expect("a renamed starter must not fail setup");
assert!(joined_live_general(&channels));
assert_eq!(accepted, vec![9021], "only the join to general");
assert!(
!still_pending,
"the renamed channel must not be shown as one this user owns"
);
}
#[tokio::test]
async fn a_starter_id_held_by_an_unseen_channel_is_skipped() {
let _serial = crate::relay_admission::TEST_SERIAL.lock().await;
// A deleted channel keeps its id, so the relay answers `duplicate`, but
// never serves its metadata.
let (result, accepted, still_pending) = ensure_with_unusable_welcome(
(status_ok(), "duplicate: channel already exists"),
None,
false,
)
.await;
let channels = result.expect("a deleted starter must not fail setup");
assert!(joined_live_general(&channels));
assert_eq!(accepted, vec![9021], "only the join to general");
assert!(
!still_pending,
"the deleted channel must not be shown as one this user owns"
);
}
#[tokio::test]
async fn other_create_refusals_still_fail_setup() {
let _serial = crate::relay_admission::TEST_SERIAL.lock().await;
let (result, accepted, _) = ensure_with_unusable_welcome(
(
axum::http::StatusCode::BAD_REQUEST,
"invalid: channel name too long",
),
None,
false,
)
.await;
assert_eq!(
result.err().as_deref(),
Some("relay returned 400 Bad Request: invalid: channel name too long")
);
assert!(accepted.is_empty());
}
#[tokio::test]
async fn a_starter_name_held_by_a_live_starter_a_replica_missed_is_joined() {
let _serial = crate::relay_admission::TEST_SERIAL.lock().await;
let (result, accepted, _) = ensure_with_unusable_welcome(
(
axum::http::StatusCode::BAD_REQUEST,
"invalid: an open channel named \"welcome-everyone\" already exists",
),
None,
true,
)
.await;
let channels = result.expect("a live starter must not fail setup");
assert!(joined_live_general(&channels));
assert!(
channels
.iter()
.any(|channel| channel.id == LIVE_WELCOME_ID && channel.is_member),
"the live welcome-everyone must be joined"
);
assert_eq!(
accepted,
vec![9021, 9021],
"joins to general and welcome-everyone"
);
}

View File

@@ -301,6 +301,25 @@ fn duplicate_channel_rejection_is_ensure_success_only() {
));
}
#[test]
fn permanent_starter_rejection_matches_only_the_relay_refusals() {
assert!(is_permanent_starter_rejection(
"relay returned 400 Bad Request: invalid: an open channel named \"General\" already exists"
));
assert!(is_permanent_starter_rejection(
"relay returned 400 Bad Request: invalid: channel is archived"
));
assert!(!is_permanent_starter_rejection(
"relay rejected event: duplicate: channel already exists"
));
assert!(!is_permanent_starter_rejection(
"relay returned 400 Bad Request: invalid: channel name too long"
));
assert!(!is_permanent_starter_rejection(
"invalid: an open channel named \"general\" already exists"
));
}
// ── compute_channels_hash ─────────────────────────────────────────────────────
fn make_channel(id: &str, name: &str, last_message_at: Option<String>) -> ChannelInfo {

View File

@@ -293,8 +293,9 @@ const EVENTS_INVENTORY: &[(&str, usize, usize)] = &[
// Stub-relay route in the tombstone-flush gate tests; production flush
// publishes through the guarded boundary-1 funnel.
("src/commands/teams/pending/tests/gate.rs", 1, 0),
// Accepting stub relay in the starter-channel creation tests.
("src/commands/channels/starter_tests.rs", 1, 0),
// Stub relays in the starter-channel creation tests: one accepts every
// event, the other refuses channel creates.
("src/commands/channels/starter_tests.rs", 2, 0),
// Admin API: every JSON body passes the guard in
// `helpers::send_admin_mutation` (injection tests in admin `direct_action_tests.rs`),
// and every admin API request URL passes it before signing or sending:

View File

@@ -332,6 +332,25 @@ test("ensureStarterChannels resumes when one starter channel is missing", async
assert.equal(ensureCalls, 1);
});
test("ensureStarterChannels keeps the starters setup did not skip", async () => {
// welcome-everyone's name belongs to an archived channel, so the native
// command skips it and returns only general.
const general = makeChannel({
id: "general-channel",
name: "general",
visibility: "open",
});
const result = await ensureStarterChannels({
getChannels: async () => [],
ensureStarterChannels: async () => [general],
});
assert.equal(result.generalChannel, general);
assert.equal(result.welcomeChannel, null);
assert.deepEqual(result.channels, [general]);
});
test("isWelcomeExperienceChannel matches legacy Welcome and starter welcome-everyone", () => {
assert.equal(isWelcomeExperienceChannel(makeChannel()), true);
assert.equal(

View File

@@ -41,8 +41,12 @@ type StarterChannelsClient = {
export type StarterChannelsResult = {
channels: Channel[];
generalChannel: Channel;
welcomeChannel: Channel;
/**
* Null when setup skipped this starter because its name or id belongs to a
* channel that cannot serve as it, such as an archived one.
*/
generalChannel: Channel | null;
welcomeChannel: Channel | null;
};
type WelcomeChannelOptions = {
@@ -269,15 +273,18 @@ export async function ensureStarterChannels(
};
}
// The native command fails unless every starter it did not skip is usable.
const ensuredChannels = await client.ensureStarterChannels();
const ensuredStarters = findStarterChannels(ensuredChannels);
if (!ensuredStarters) {
throw new Error("Starter channels were not available after setup");
}
return {
channels: ensuredChannels,
...ensuredStarters,
generalChannel: findStarterChannel(
ensuredChannels,
STARTER_GENERAL_CHANNEL_NAME,
),
welcomeChannel: findStarterChannel(
ensuredChannels,
STARTER_WELCOME_CHANNEL_NAME,
),
};
}