From 21fe80c370775fbbe47d176b5b0ab3db40482427 Mon Sep 17 00:00:00 2001 From: Salman Mohammed Date: Sun, 1 Nov 2026 20:02:30 -0500 Subject: [PATCH 1/4] fix(desktop): skip a starter channel the relay will never create First-run starter setup creates `general` and `welcome-everyone` at fixed ids when it can't find a live one. Some refusals of that create are permanent, and any refusal aborted setup before it joined the new member to the starter channels that do exist: - Another listed channel, archived ones included, holds the starter's name. Relays that reserve listed names refuse with HTTP 400 `invalid: an open channel named "..." already exists`. - An archived channel holds the starter's fixed id. The relay refuses any event aimed at an archived channel, before its duplicate check, with HTTP 400 `invalid: channel is archived`. - A live channel at the fixed id was renamed or retyped. The relay answers `duplicate`, and setup waited for a starter that never appears, then failed. Setup now skips such a starter and still joins the rest. The frontend reports a skipped starter as null instead of throwing. Co-authored-by: opus Co-Authored-By: Claude Opus 5.5 Signed-off-by: Salman Mohammed --- desktop/src-tauri/src/commands/channels.rs | 61 +++++- .../src/commands/channels/starter_tests.rs | 181 ++++++++++++++++++ .../src-tauri/src/commands/channels_tests.rs | 19 ++ desktop/src-tauri/src/egress_guard_tests.rs | 5 +- .../src/features/onboarding/welcome.test.mjs | 19 ++ desktop/src/features/onboarding/welcome.ts | 23 ++- 6 files changed, 288 insertions(+), 20 deletions(-) diff --git a/desktop/src-tauri/src/commands/channels.rs b/desktop/src-tauri/src/commands/channels.rs index 54a6037c239..fb2267ea7d5 100644 --- a/desktop/src-tauri/src/commands/channels.rs +++ b/desktop/src-tauri/src/commands/channels.rs @@ -230,6 +230,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 +248,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 +431,12 @@ 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(); for spec in STARTER_CHANNELS { if existing_channels @@ -424,7 +448,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 +465,16 @@ 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. + Err(error) if is_permanent_starter_rejection(&error) => { + skipped.insert(spec.slug); + continue; } Err(error) => return Err(error), } + starter_ids.push(channel_uuid_string); } for _ in 0..3 { @@ -460,17 +490,28 @@ 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?; } - 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()); } diff --git a/desktop/src-tauri/src/commands/channels/starter_tests.rs b/desktop/src-tauri/src/commands/channels/starter_tests.rs index 71bbbd0e894..6dbf340c1c2 100644 --- a/desktop/src-tauri/src/commands/channels/starter_tests.rs +++ b/desktop/src-tauri/src/commands/channels/starter_tests.rs @@ -38,3 +38,184 @@ 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"; + +/// 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 only 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 served when metadata is read by id. Returns the +/// result, the kinds of the events the relay accepted, and whether that 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>, +) -> (Result, String>, Vec, 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 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| { + let filter = filters[0].clone(); + let events = match (filter["kinds"][0].as_u64(), filter.get("#d")) { + // The open directory lists live channels only. + (Some(39000), None) => vec![general.clone()], + // 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| { + 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, + ) + .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, + ) + .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"), + ) + .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 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, + ) + .await; + + assert_eq!( + result.err().as_deref(), + Some("relay returned 400 Bad Request: invalid: channel name too long") + ); + assert!(accepted.is_empty()); +} diff --git a/desktop/src-tauri/src/commands/channels_tests.rs b/desktop/src-tauri/src/commands/channels_tests.rs index d31acb08f8f..9f475c8ad4f 100644 --- a/desktop/src-tauri/src/commands/channels_tests.rs +++ b/desktop/src-tauri/src/commands/channels_tests.rs @@ -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) -> ChannelInfo { diff --git a/desktop/src-tauri/src/egress_guard_tests.rs b/desktop/src-tauri/src/egress_guard_tests.rs index c2d63b3cfff..77ac6479633 100644 --- a/desktop/src-tauri/src/egress_guard_tests.rs +++ b/desktop/src-tauri/src/egress_guard_tests.rs @@ -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: diff --git a/desktop/src/features/onboarding/welcome.test.mjs b/desktop/src/features/onboarding/welcome.test.mjs index e0a57a4586b..34d5f27ffe3 100644 --- a/desktop/src/features/onboarding/welcome.test.mjs +++ b/desktop/src/features/onboarding/welcome.test.mjs @@ -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( diff --git a/desktop/src/features/onboarding/welcome.ts b/desktop/src/features/onboarding/welcome.ts index 359c8341b23..e666d661c2f 100644 --- a/desktop/src/features/onboarding/welcome.ts +++ b/desktop/src/features/onboarding/welcome.ts @@ -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, + ), }; } From c702a943fe2fff31120b639676fec6f97a680068 Mon Sep 17 00:00:00 2001 From: Salman Mohammed Date: Sun, 1 Nov 2026 20:08:36 -0500 Subject: [PATCH 2/4] fix(desktop): skip a starter whose id belongs to a channel the user can't see A deleted starter keeps its fixed id, so the relay answers `duplicate` but never serves its metadata. Setup waited for it, then failed before joining the other starter. Co-authored-by: opus Co-Authored-By: Claude Opus 5.5 Signed-off-by: Salman Mohammed --- desktop/src-tauri/src/commands/channels.rs | 9 +++++++++ .../src/commands/channels/starter_tests.rs | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/desktop/src-tauri/src/commands/channels.rs b/desktop/src-tauri/src/commands/channels.rs index fb2267ea7d5..cc5cf5a5313 100644 --- a/desktop/src-tauri/src/commands/channels.rs +++ b/desktop/src-tauri/src/commands/channels.rs @@ -509,6 +509,15 @@ async fn ensure_starter_channels_inner( if !has_all_starter_channels(&existing_channels, &skipped) { existing_channels = fetch_channels(state, DirectoryScope::IncludeOpenDirectory).await?; + // A starter id the relay holds but never serves, even from the + // writer, 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, &skipped) { diff --git a/desktop/src-tauri/src/commands/channels/starter_tests.rs b/desktop/src-tauri/src/commands/channels/starter_tests.rs index 6dbf340c1c2..ed5d080bc4b 100644 --- a/desktop/src-tauri/src/commands/channels/starter_tests.rs +++ b/desktop/src-tauri/src/commands/channels/starter_tests.rs @@ -201,6 +201,24 @@ async fn a_starter_id_held_by_a_renamed_channel_is_skipped() { ); } +#[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) + .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; From 02ea558a852c495a8b9086367e956e0093ef8856 Mon Sep 17 00:00:00 2001 From: Salman Mohammed Date: Mon, 2 Nov 2026 08:45:18 -0500 Subject: [PATCH 3/4] fix(desktop): join a live starter that holds the name before skipping it A taken-name refusal can come from a live starter the first directory read missed, such as one created since. Read the directory again and join a matching starter; skip only when none is usable. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Salman Mohammed Co-authored-by: opus --- desktop/src-tauri/src/commands/channels.rs | 15 ++++- .../src/commands/channels/starter_tests.rs | 59 +++++++++++++++++-- 2 files changed, 67 insertions(+), 7 deletions(-) diff --git a/desktop/src-tauri/src/commands/channels.rs b/desktop/src-tauri/src/commands/channels.rs index cc5cf5a5313..4856c86f797 100644 --- a/desktop/src-tauri/src/commands/channels.rs +++ b/desktop/src-tauri/src/commands/channels.rs @@ -467,9 +467,20 @@ async fn ensure_starter_channels_inner( 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. + // 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 look again before giving up on it. Err(error) if is_permanent_starter_rejection(&error) => { - skipped.insert(spec.slug); + let refreshed = fetch_channels(state, DirectoryScope::IncludeOpenDirectory).await?; + match refreshed + .into_iter() + .find(|channel| is_matching_starter_channel(channel, spec)) + { + Some(channel) => existing_channels.push(channel), + None => { + skipped.insert(spec.slug); + } + } continue; } Err(error) => return Err(error), diff --git a/desktop/src-tauri/src/commands/channels/starter_tests.rs b/desktop/src-tauri/src/commands/channels/starter_tests.rs index ed5d080bc4b..19fb5148a38 100644 --- a/desktop/src-tauri/src/commands/channels/starter_tests.rs +++ b/desktop/src-tauri/src/commands/channels/starter_tests.rs @@ -40,6 +40,7 @@ async fn created_starter_channels_are_reported_when_metadata_is_unavailable() { } 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 { @@ -63,10 +64,13 @@ fn channel_event(id: &str, name: &str) -> serde_json::Value { /// `renamed_starter`, a live channel under that name holds `welcome-everyone`'s /// deterministic id and is served when metadata is read by id. Returns the /// result, the kinds of the events the relay accepted, and whether that id is -/// still marked as pending-owned. +/// still marked as pending-owned. With `late_welcome`, a live +/// `welcome-everyone` under another id joins the directory after the first +/// read. async fn ensure_with_unusable_welcome( create_refusal: (axum::http::StatusCode, &'static str), renamed_starter: Option<&'static str>, + late_welcome: bool, ) -> (Result, String>, Vec, bool) { use std::sync::{Arc, Mutex}; @@ -83,6 +87,8 @@ async fn ensure_with_unusable_welcome( .to_string(); let general = channel_event(LIVE_GENERAL_ID, "general"); + let welcome = channel_event(LIVE_WELCOME_ID, "welcome-everyone"); + let directory_reads = Arc::new(Mutex::new(0)); let renamed = renamed_starter.map(|name| channel_event(&starter_id, name)); let accepted = Arc::new(Mutex::new(Vec::new())); let accepted_kinds = accepted.clone(); @@ -93,7 +99,15 @@ async fn ensure_with_unusable_welcome( let filter = filters[0].clone(); let events = match (filter["kinds"][0].as_u64(), filter.get("#d")) { // The open directory lists live channels only. - (Some(39000), None) => vec![general.clone()], + (Some(39000), None) => { + let mut reads = directory_reads.lock().unwrap(); + *reads += 1; + if late_welcome && *reads > 1 { + vec![general.clone(), welcome.clone()] + } else { + vec![general.clone()] + } + } // Metadata by id. (Some(39000), Some(ids)) => renamed .iter() @@ -156,6 +170,7 @@ async fn a_starter_name_held_by_another_channel_is_skipped() { "invalid: an open channel named \"welcome-everyone\" already exists", ), None, + false, ) .await; @@ -175,6 +190,7 @@ async fn a_starter_id_held_by_an_archived_channel_is_skipped() { "invalid: channel is archived", ), None, + false, ) .await; @@ -189,6 +205,7 @@ async fn a_starter_id_held_by_a_renamed_channel_is_skipped() { let (result, accepted, still_pending) = ensure_with_unusable_welcome( (status_ok(), "duplicate: channel already exists"), Some("intros"), + false, ) .await; @@ -206,9 +223,12 @@ 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) - .await; + 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)); @@ -228,6 +248,7 @@ async fn other_create_refusals_still_fail_setup() { "invalid: channel name too long", ), None, + false, ) .await; @@ -237,3 +258,31 @@ async fn other_create_refusals_still_fail_setup() { ); assert!(accepted.is_empty()); } + +#[tokio::test] +async fn a_starter_name_held_by_a_live_starter_the_first_read_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" + ); +} From d258fe5a4d77b56d59f4d5956a4d37488cab3313 Mon Sep 17 00:00:00 2001 From: Salman Mohammed Date: Mon, 2 Nov 2026 11:42:45 -0500 Subject: [PATCH 4/4] fix(desktop): read refused starter names from the writer The second look at a refused starter used the replica-eligible directory, so a lagging replica could hide a live starter the relay had just refused against the writer, and setup skipped it. Read the open directory pinned to the writer, once for all refused starters. The renamed-starter test now lists the renamed channel in the directory, as a real relay does, so it fails when its guard is removed. Co-Authored-By: Claude Opus 5.5 Co-authored-by: opus Signed-off-by: Salman Mohammed --- desktop/src-tauri/src/commands/channels.rs | 47 +++++++++++++------ .../src-tauri/src/commands/channels/fetch.rs | 17 +++++++ .../src/commands/channels/starter_tests.rs | 33 +++++++------ 3 files changed, 65 insertions(+), 32 deletions(-) diff --git a/desktop/src-tauri/src/commands/channels.rs b/desktop/src-tauri/src/commands/channels.rs index 4856c86f797..9c5f0ea1c8b 100644 --- a/desktop/src-tauri/src/commands/channels.rs +++ b/desktop/src-tauri/src/commands/channels.rs @@ -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"); @@ -437,6 +439,8 @@ async fn ensure_starter_channels_inner( // 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 @@ -469,18 +473,9 @@ async fn ensure_starter_channels_inner( } // 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 look again before giving up on it. + // created since, so it is looked up again below. Err(error) if is_permanent_starter_rejection(&error) => { - let refreshed = fetch_channels(state, DirectoryScope::IncludeOpenDirectory).await?; - match refreshed - .into_iter() - .find(|channel| is_matching_starter_channel(channel, spec)) - { - Some(channel) => existing_channels.push(channel), - None => { - skipped.insert(spec.slug); - } - } + refused.push(spec); continue; } Err(error) => return Err(error), @@ -488,6 +483,28 @@ async fn ensure_starter_channels_inner( 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 { let metadata = fetch_starter_channel_metadata(state, &starter_ids).await?; for mut channel in metadata { @@ -520,9 +537,9 @@ async fn ensure_starter_channels_inner( if !has_all_starter_channels(&existing_channels, &skipped) { existing_channels = fetch_channels(state, DirectoryScope::IncludeOpenDirectory).await?; - // A starter id the relay holds but never serves, even from the - // writer, belongs to a channel this user cannot see, such as a - // deleted one. + // 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); diff --git a/desktop/src-tauri/src/commands/channels/fetch.rs b/desktop/src-tauri/src/commands/channels/fetch.rs index 993d40807e2..0dd2ee96608 100644 --- a/desktop/src-tauri/src/commands/channels/fetch.rs +++ b/desktop/src-tauri/src/commands/channels/fetch.rs @@ -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, 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, diff --git a/desktop/src-tauri/src/commands/channels/starter_tests.rs b/desktop/src-tauri/src/commands/channels/starter_tests.rs index 19fb5148a38..c70dd4ba47a 100644 --- a/desktop/src-tauri/src/commands/channels/starter_tests.rs +++ b/desktop/src-tauri/src/commands/channels/starter_tests.rs @@ -57,20 +57,20 @@ fn channel_event(id: &str, name: &str) -> serde_json::Value { serde_json::to_value(&event).unwrap() } -/// Runs starter setup against a fake relay whose directory lists only a live +/// 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 served when metadata is read by id. Returns the -/// result, the kinds of the events the relay accepted, and whether that id is -/// still marked as pending-owned. With `late_welcome`, a live -/// `welcome-everyone` under another id joins the directory after the first -/// read. +/// 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>, - late_welcome: bool, + writer_only_welcome: bool, ) -> (Result, String>, Vec, bool) { use std::sync::{Arc, Mutex}; @@ -88,7 +88,6 @@ async fn ensure_with_unusable_welcome( let general = channel_event(LIVE_GENERAL_ID, "general"); let welcome = channel_event(LIVE_WELCOME_ID, "welcome-everyone"); - let directory_reads = Arc::new(Mutex::new(0)); let renamed = renamed_starter.map(|name| channel_event(&starter_id, name)); let accepted = Arc::new(Mutex::new(Vec::new())); let accepted_kinds = accepted.clone(); @@ -98,15 +97,15 @@ async fn ensure_with_unusable_welcome( post(move |Json(filters): Json| { let filter = filters[0].clone(); let events = match (filter["kinds"][0].as_u64(), filter.get("#d")) { - // The open directory lists live channels only. + // The open directory lists live channels only. A lagging + // replica misses the late `welcome-everyone`; the writer + // has it. (Some(39000), None) => { - let mut reads = directory_reads.lock().unwrap(); - *reads += 1; - if late_welcome && *reads > 1 { - vec![general.clone(), welcome.clone()] - } else { - vec![general.clone()] - } + 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 @@ -260,7 +259,7 @@ async fn other_create_refusals_still_fail_setup() { } #[tokio::test] -async fn a_starter_name_held_by_a_live_starter_the_first_read_missed_is_joined() { +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( (