diff --git a/desktop/src-tauri/src/commands/channels.rs b/desktop/src-tauri/src/commands/channels.rs index 54a6037c23..9c5f0ea1c8 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"); @@ -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()); } diff --git a/desktop/src-tauri/src/commands/channels/fetch.rs b/desktop/src-tauri/src/commands/channels/fetch.rs index 993d40807e..0dd2ee9660 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 71bbbd0e89..c70dd4ba47 100644 --- a/desktop/src-tauri/src/commands/channels/starter_tests.rs +++ b/desktop/src-tauri/src/commands/channels/starter_tests.rs @@ -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, 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 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| { + 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| { + 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" + ); +} diff --git a/desktop/src-tauri/src/commands/channels_tests.rs b/desktop/src-tauri/src/commands/channels_tests.rs index d31acb08f8..9f475c8ad4 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 c2d63b3cff..77ac647963 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 e0a57a4586..34d5f27ffe 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 359c8341b2..e666d661c2 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, + ), }; }