diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index 9099beff69e..69250c4b537 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -86,6 +86,9 @@ export default defineConfig({ "**/home-collapsed-top-chrome.spec.ts", "**/top-chrome-zoom-clearance.spec.ts", "**/thread-unread.spec.ts", + "**/thread-load-failure.spec.ts", + "**/project-conversation-load-failure.spec.ts", + "**/huddle-thread-load-failure.spec.ts", "**/workspace-rail.spec.ts", "**/community-rail.spec.ts", "**/boot-splash.spec.ts", diff --git a/desktop/src-tauri/src/relay.rs b/desktop/src-tauri/src/relay.rs index bd3fefb1259..f408ef2afda 100644 --- a/desktop/src-tauri/src/relay.rs +++ b/desktop/src-tauri/src/relay.rs @@ -16,6 +16,19 @@ const DEFAULT_RELAY_WS_URL: &str = "ws://localhost:3000"; // classifier keys on. Extracted to a const so a test can pin that contract. const MALFORMED_RESPONSE_MESSAGE: &str = "relay returned malformed response: not valid JSON"; +// Per-request deadline for the `POST /query` HTTP bridge, covering both the +// header exchange and full body consumption. The shared `http_client` sets no +// client-level timeout — deliberately, because it is also used for long-running +// STT/TTS model downloads, builderlab auth, and the media proxy — so a stalled +// or half-open `/query` connection would otherwise leave the request pending +// forever, hanging the caller (e.g. a thread-history load that never resolves +// and shows a permanent skeleton). A per-request timeout scoped to `/query` +// bounds that without affecting the client's other users. A timeout surfaces +// through `classify_request_error` as the stable `"relay unreachable: request +// timed out"` string. Set above the 25s WS history timeout so a slow-but-live +// relay is not cut off before the WebSocket path would be. +const QUERY_REQUEST_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); + fn configured_env_var(name: &str) -> Option { std::env::var(name) .ok() @@ -167,6 +180,22 @@ pub(crate) fn classify_request_error(e: &reqwest::Error) -> String { } } +/// Preserve a body-consumption timeout as the stable connectivity classification. +/// +/// `send()` resolves once response headers arrive, so a body that stalls past +/// the request deadline trips the timeout during body consumption rather than +/// at `send()`. That is a connectivity failure, not a malformed body or a plain +/// status error. Both body-consumption paths — the 2xx `parse_json_response` +/// and the non-2xx `relay_error_message` — route their consumption error +/// through this one helper so a stalled body can never be classified as +/// "request timed out" on one path while the other buries it under a malformed +/// or status label. Returns `Some("relay unreachable: request timed out")` for +/// a timeout; `None` otherwise, leaving the caller to apply its own non-timeout +/// label. +fn classify_body_timeout(e: &reqwest::Error) -> Option { + e.is_timeout().then(|| classify_request_error(e)) +} + /// Detect responses that were intercepted by a captive portal or auth proxy. /// /// Returns `Some(msg)` when the response clearly did not come from the relay: @@ -230,10 +259,16 @@ pub(crate) async fn parse_json_response( // "relay unreachable:" bucket so it surfaces loudly instead of being treated // as a transient unreachable-relay condition. The reqwest error detail is // dropped because it contains the raw URL. - response - .json::() - .await - .map_err(|_| MALFORMED_RESPONSE_MESSAGE.to_string()) + // + // A body-consumption timeout is the exception: `send()` resolves once + // headers arrive, so a body that stalls past the request deadline trips the + // timeout HERE rather than at send(). That is a connectivity failure, not a + // malformed body, so route it through `classify_body_timeout` — the same + // helper the non-2xx error-body path uses — to preserve the stable + // "relay unreachable: request timed out" label. + response.json::().await.map_err(|e| { + classify_body_timeout(&e).unwrap_or_else(|| MALFORMED_RESPONSE_MESSAGE.to_string()) + }) } /// Extract the `retry in Ns` hint from a rate-limit error string. @@ -264,7 +299,21 @@ pub async fn relay_error_message(response: reqwest::Response) -> String { } // Real relay error: extract the structured message field if available. - let body = response.text().await.unwrap_or_default(); + // `text()` consumes the body, which — like the 2xx path — can trip the + // request deadline if the relay sends status headers then stalls the body. + // Preserve that timeout as the stable connectivity classification via the + // shared helper instead of letting `unwrap_or_default` swallow it into a + // bare status label. A non-timeout body error still degrades to an empty + // body → status-only message, exactly as before. + let body = match response.text().await { + Ok(body) => body, + Err(e) => { + if let Some(timeout) = classify_body_timeout(&e) { + return timeout; + } + String::new() + } + }; // 429 Too Many Requests → typed `relay rate-limited:` prefix so the TS // client can activate the rate-limit gate without confusing it with a @@ -328,22 +377,15 @@ pub async fn query_relay_at( let body_bytes = serde_json::to_vec(filters).map_err(|e| format!("filter serialization failed: {e}"))?; let auth = build_nip98_auth_header(&Method::POST, &url, &body_bytes, state)?; - - let response = state - .http_client - .post(&url) - .header("Authorization", auth) - .header("Content-Type", "application/json") - .body(body_bytes) - .send() - .await - .map_err(|e| classify_request_error(&e))?; - - if !response.status().is_success() { - return Err(relay_error_message(response).await); - } - - parse_json_response(response).await + send_query_request( + &state.http_client, + &url, + &auth, + None, + body_bytes, + QUERY_REQUEST_TIMEOUT, + ) + .await } pub async fn query_relay_at_with_keys( @@ -358,11 +400,38 @@ pub async fn query_relay_at_with_keys( let body_bytes = serde_json::to_vec(filters).map_err(|e| format!("filter serialization failed: {e}"))?; let auth = build_nip98_auth_header_for_keys(keys, &Method::POST, &url, &body_bytes)?; - let mut request = state - .http_client - .post(&url) + send_query_request( + &state.http_client, + &url, + &auth, + auth_tag, + body_bytes, + QUERY_REQUEST_TIMEOUT, + ) + .await +} + +/// Issue an authenticated `POST /query` and parse the response, applying the +/// per-request `timeout` that bounds a stalled or half-open relay connection. +/// +/// Both `/query` builders funnel through this one helper so the timeout can +/// never be applied to one builder and dropped from the other, and so a test +/// can drive the real send/timeout/classify path with a short deadline against +/// a stalled loopback. A timeout surfaces through `classify_request_error` as +/// the stable `"relay unreachable: request timed out"` string. +async fn send_query_request( + http_client: &reqwest::Client, + url: &str, + auth: &str, + auth_tag: Option<&str>, + body_bytes: Vec, + timeout: std::time::Duration, +) -> Result, String> { + let mut request = http_client + .post(url) .header("Authorization", auth) - .header("Content-Type", "application/json"); + .header("Content-Type", "application/json") + .timeout(timeout); if let Some(tag) = auth_tag { request = request.header("x-auth-tag", tag); } @@ -611,384 +680,4 @@ pub async fn submit_signed_event_with_keys( // ── Tests ─────────────────────────────────────────────────────────────────── #[cfg(test)] -mod tests { - use super::{ - build_profile_event, classify_intercepted_response, effective_agent_relay_url, - extract_retry_in_hint, parse_command_response, relay_http_base_url, - MALFORMED_RESPONSE_MESSAGE, - }; - use serde::Deserialize; - - // ── extract_retry_in_hint ──────────────────────────────────────────────── - - #[test] - fn extracts_hint_from_429_body() { - assert_eq!( - extract_retry_in_hint(r#"{"error":"rate-limited: quota exceeded; retry in 4s"}"#), - Some(4) - ); - } - - #[test] - fn extracts_hint_when_no_json_wrapper() { - assert_eq!(extract_retry_in_hint("retry in 30s"), Some(30)); - } - - #[test] - fn returns_none_when_no_hint_present() { - assert_eq!( - extract_retry_in_hint(r#"{"error":"rate-limited: quota exceeded"}"#), - None - ); - assert_eq!(extract_retry_in_hint(""), None); - } - - #[test] - fn overlong_digit_string_returns_none() { - // A digit sequence that exceeds u64::MAX cannot be parsed; the function - // must return None (→ caller uses the default) rather than panicking. - assert_eq!( - extract_retry_in_hint("retry in 99999999999999999999999s"), - None - ); - } - - // ── relay_error_message: hint capping ──────────────────────────────────── - // - // Verify that an oversized relay hint is capped in the returned message - // string, not just inside `activate_rate_limit()`. This guarantees every - // consumer — including the TS gate via `applyTauriRateLimitIfNeeded` — - // receives the capped value rather than the raw untrusted relay value. - - #[tokio::test] - async fn oversized_hint_is_capped_in_relay_error_message_string() { - use crate::relay_admission::{reset_rate_limit_gate, MAX_HINT_SECONDS, TEST_SERIAL}; - use std::io::{Read as _, Write as _}; - - let _serial = TEST_SERIAL.lock().await; - reset_rate_limit_gate(); - - // Use a std::net listener on a std::thread — the same pattern as the - // relay_admission loopback tests. This avoids two races that cause CI - // failures with tokio::net + into_std(): - // 1. No request read: the client is still sending when the response - // arrives → hyper `UnexpectedMessage`/`Canceled` under load. - // 2. into_std() leaves the socket in nonblocking mode → write_all - // may return WouldBlock and silently drop the response. - let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); - let addr = listener.local_addr().unwrap(); - - // Serve a 429 with a hint far exceeding MAX_HINT_SECONDS (300). - let oversized = 1_000_000u64; - let body = format!(r#"{{"error":"rate-limited: quota exceeded; retry in {oversized}s"}}"#); - let body_len = body.len(); - std::thread::spawn(move || { - if let Ok((mut stream, _)) = listener.accept() { - // Read the request first so the client finishes sending before - // we write the response — mirrors relay_admission.rs pattern. - let mut buf = [0u8; 4096]; - let _ = stream.read(&mut buf); - let response = format!( - "HTTP/1.1 429 Too Many Requests\r\nContent-Type: application/json\r\nContent-Length: {body_len}\r\nConnection: close\r\n\r\n{body}" - ); - let _ = stream.write_all(response.as_bytes()); - let _ = stream.flush(); - } - }); - - let client = reqwest::Client::new(); - let response = client - .get(format!("http://{addr}/")) - .send() - .await - .expect("request must succeed"); - - let msg = super::relay_error_message(response).await; - - // The message must embed the CAPPED hint, not the raw 1 000 000. - assert_eq!( - msg, - format!("relay rate-limited: retry in {MAX_HINT_SECONDS}s"), - "relay_error_message must embed the capped hint, not the raw untrusted value" - ); - assert!( - !msg.contains(&oversized.to_string()), - "raw oversized hint must not appear in the message string" - ); - reset_rate_limit_gate(); - } - - // ── effective_agent_relay_url: legacy pin ignored ───────────────────────── - - #[test] - fn stored_relay_pin_is_ignored() { - // Zero-touch cutover (#2122): a creation-era per-record relay pin is - // parsed and persisted but never consulted — the workspace relay wins. - assert_eq!( - effective_agent_relay_url("wss://relay.other.com", "wss://staging.example.com"), - "wss://staging.example.com" - ); - } - - #[test] - fn empty_relay_resolves_to_workspace() { - // A never-set record resolves to the active workspace relay at read-time, - // so a stale stored default can never make it load-bearing. - assert_eq!( - effective_agent_relay_url("", "wss://staging.example.com"), - "wss://staging.example.com" - ); - } - - #[test] - fn whitespace_only_relay_resolves_to_workspace() { - // Whitespace-only behaves identically — no value survives. - assert_eq!( - effective_agent_relay_url(" ", "wss://staging.example.com"), - "wss://staging.example.com" - ); - } - - // ── relay_http_base_url scheme conversion ──────────────────────────────── - - #[test] - fn loopback_ws_localhost_preserves_authority() { - // Tenant host-binding keys off the HTTP Host/authority. The desktop must - // not rewrite localhost to 127.0.0.1, or local dev HTTP calls target a - // different unmapped community than the WebSocket URL. - assert_eq!( - relay_http_base_url("ws://localhost:3000"), - "http://localhost:3000" - ); - } - - #[test] - fn loopback_trailing_slash_removed_authority_preserved() { - assert_eq!( - relay_http_base_url("ws://localhost:3000/"), - "http://localhost:3000" - ); - } - - #[test] - fn remote_wss_host_unchanged() { - assert_eq!( - relay_http_base_url("wss://relay.example.com"), - "https://relay.example.com" - ); - } - - #[test] - fn loopback_ipv4_literal_unchanged() { - assert_eq!( - relay_http_base_url("ws://127.0.0.1:3000"), - "http://127.0.0.1:3000" - ); - } - - #[test] - fn localhost_substring_host_unchanged() { - assert_eq!( - relay_http_base_url("ws://localhost.evil.com:3000"), - "http://localhost.evil.com:3000" - ); - } - - #[test] - fn loopback_wss_localhost_preserves_authority() { - assert_eq!( - relay_http_base_url("wss://localhost:3000"), - "https://localhost:3000" - ); - } - - // ── classify_intercepted_response ──────────────────────────────────────── - - #[test] - fn intercepted_cloudflare_host_returns_some() { - let result = classify_intercepted_response("sqprod.cloudflareaccess.com", "text/html"); - assert!(result.is_some()); - let msg = result.unwrap(); - assert!( - msg.starts_with("relay unreachable:"), - "should have unreachable prefix" - ); - assert!(msg.contains("Cloudflare"), "should mention Cloudflare"); - } - - #[test] - fn intercepted_cloudflare_apex_host_returns_some() { - // The apex domain itself should also match. - let result = classify_intercepted_response("cloudflareaccess.com", "application/json"); - assert!(result.is_some()); - let msg = result.unwrap(); - assert!(msg.starts_with("relay unreachable:")); - assert!(msg.contains("Cloudflare")); - } - - #[test] - fn intercepted_non_cloudflare_html_returns_some() { - let result = - classify_intercepted_response("proxy.corporate.example", "text/html; charset=utf-8"); - assert!(result.is_some()); - let msg = result.unwrap(); - assert!(msg.starts_with("relay unreachable:")); - } - - #[test] - fn normal_relay_json_returns_none() { - let result = classify_intercepted_response("relay.myapp.example.com", "application/json"); - assert!(result.is_none()); - } - - #[test] - fn content_type_case_insensitive() { - // Uppercase content-type must still be detected. - let result = classify_intercepted_response("proxy.example.com", "TEXT/HTML"); - assert!(result.is_some()); - assert!(result.unwrap().starts_with("relay unreachable:")); - } - - #[test] - fn evil_suffix_does_not_match_cloudflare() { - // A host whose suffix happens to contain the Cloudflare string but is - // not actually a subdomain must NOT match. - let result = classify_intercepted_response( - "notcloudflareaccess.com.evil.example", - "application/json", - ); - assert!( - result.is_none(), - "false suffix match should not trigger Cloudflare branch" - ); - } - - // classify_request_error requires a real reqwest::Error (not publicly - // constructable) — tested indirectly through integration; skipped here. - - // ── parse_json_response malformed-body contract ────────────────────────── - - #[test] - fn malformed_response_message_stays_off_unreachable_bucket() { - // A reached-but-malformed 2xx body is not a connectivity failure. If this - // message ever regains the "relay unreachable:" prefix, the frontend - // classifier would misroute it as unreachable — pin that it never does. - assert!( - !MALFORMED_RESPONSE_MESSAGE.starts_with("relay unreachable:"), - "malformed-response message must not match the unreachable prefix" - ); - } - - // ── parse_command_response ─────────────────────────────────────────────── - - #[derive(Debug, Deserialize, PartialEq)] - struct ChannelCreated { - channel_id: String, - } - - #[test] - fn parse_command_response_decodes_typed_payload() { - let msg = r#"response:{"channel_id":"abc123"}"#; - let parsed: ChannelCreated = parse_command_response(msg).expect("should parse"); - assert_eq!( - parsed, - ChannelCreated { - channel_id: "abc123".to_string() - } - ); - } - - #[test] - fn parse_command_response_accepts_raw_json_fallback() { - // Backward-compat: relays that emit raw JSON (no prefix) still work. - let msg = r#"{"channel_id":"abc"}"#; - let parsed: ChannelCreated = parse_command_response(msg).expect("fallback parse"); - assert_eq!( - parsed, - ChannelCreated { - channel_id: "abc".to_string() - } - ); - } - - #[test] - fn parse_command_response_rejects_invalid_prefixed_json() { - let msg = "response:not-json"; - let result: Result = parse_command_response(msg); - assert!(result.is_err()); - assert!(result.unwrap_err().contains("response parse failed")); - } - - #[test] - fn parse_command_response_rejects_garbage() { - let msg = "totally not json or response"; - let result: Result = parse_command_response(msg); - assert!(result.is_err()); - } - - // ── build_profile_event ────────────────────────────────────────────────── - - /// Generate a valid NIP-OA auth tag JSON string signed by a fresh owner key - /// and addressed to `agent_keys`. - /// - /// Uses `nostr_compat` (nostr 0.36) for the owner keys because - /// `buzz_sdk_pkg::nip_oa::compute_auth_tag` expects nostr 0.36 types. - /// The agent pubkey is bridged via hex encoding. - fn make_valid_auth_tag(agent_keys: &nostr::Keys) -> String { - let owner_keys = nostr::Keys::generate(); - let agent_pubkey_hex = agent_keys.public_key().to_hex(); - let agent_compat_pubkey = - nostr::PublicKey::from_hex(&agent_pubkey_hex).expect("valid hex pubkey should parse"); - buzz_sdk_pkg::nip_oa::compute_auth_tag(&owner_keys, &agent_compat_pubkey, "") - .expect("compute_auth_tag should not fail with distinct keys") - } - - #[test] - fn profile_event_with_valid_auth_tag() { - let agent_keys = nostr::Keys::generate(); - let tag_json = make_valid_auth_tag(&agent_keys); - let event = build_profile_event(&agent_keys, "TestBot", None, Some(&tag_json)) - .expect("should succeed with a valid auth tag"); - - // Exactly one "auth" tag must be present. - let auth_tags: Vec<_> = event - .tags - .iter() - .filter(|t| t.as_slice().first().map(|s| s.as_str()) == Some("auth")) - .collect(); - assert_eq!(auth_tags.len(), 1, "expected exactly 1 auth tag"); - - // Must be a kind:0 (Metadata) event. - assert_eq!(event.kind, nostr::Kind::Metadata); - } - - #[test] - fn profile_event_without_auth_tag() { - let agent_keys = nostr::Keys::generate(); - let event = build_profile_event(&agent_keys, "TestBot", None, None) - .expect("should succeed without an auth tag"); - - // No "auth" tags should be present. - let auth_tags: Vec<_> = event - .tags - .iter() - .filter(|t| t.as_slice().first().map(|s| s.as_str()) == Some("auth")) - .collect(); - assert_eq!(auth_tags.len(), 0, "expected no auth tags"); - - assert_eq!(event.kind, nostr::Kind::Metadata); - } - - #[test] - fn profile_event_rejects_invalid_auth_tag() { - let agent_keys = nostr::Keys::generate(); - // Structurally valid JSON array but with a bogus signature — verification must fail. - let bad_json = format!(r#"["auth","{}","","{}"]"#, "a".repeat(64), "b".repeat(128)); - let result = build_profile_event(&agent_keys, "TestBot", None, Some(&bad_json)); - assert!(result.is_err(), "should reject an invalid auth tag"); - assert!( - result.unwrap_err().contains("verification failed"), - "error message should mention verification failure" - ); - } -} +mod tests; diff --git a/desktop/src-tauri/src/relay/tests.rs b/desktop/src-tauri/src/relay/tests.rs new file mode 100644 index 00000000000..4ae39249328 --- /dev/null +++ b/desktop/src-tauri/src/relay/tests.rs @@ -0,0 +1,615 @@ +//! Unit tests for the relay HTTP/command bridge helpers. +//! Extracted from `relay.rs` to keep that module under the file-size ratchet. + +use super::{ + build_profile_event, classify_intercepted_response, effective_agent_relay_url, + extract_retry_in_hint, parse_command_response, relay_http_base_url, MALFORMED_RESPONSE_MESSAGE, +}; +use serde::Deserialize; + +// ── extract_retry_in_hint ──────────────────────────────────────────────── + +#[test] +fn extracts_hint_from_429_body() { + assert_eq!( + extract_retry_in_hint(r#"{"error":"rate-limited: quota exceeded; retry in 4s"}"#), + Some(4) + ); +} + +#[test] +fn extracts_hint_when_no_json_wrapper() { + assert_eq!(extract_retry_in_hint("retry in 30s"), Some(30)); +} + +#[test] +fn returns_none_when_no_hint_present() { + assert_eq!( + extract_retry_in_hint(r#"{"error":"rate-limited: quota exceeded"}"#), + None + ); + assert_eq!(extract_retry_in_hint(""), None); +} + +#[test] +fn overlong_digit_string_returns_none() { + // A digit sequence that exceeds u64::MAX cannot be parsed; the function + // must return None (→ caller uses the default) rather than panicking. + assert_eq!( + extract_retry_in_hint("retry in 99999999999999999999999s"), + None + ); +} + +// ── relay_error_message: hint capping ──────────────────────────────────── +// +// Verify that an oversized relay hint is capped in the returned message +// string, not just inside `activate_rate_limit()`. This guarantees every +// consumer — including the TS gate via `applyTauriRateLimitIfNeeded` — +// receives the capped value rather than the raw untrusted relay value. + +#[tokio::test] +async fn oversized_hint_is_capped_in_relay_error_message_string() { + use crate::relay_admission::{reset_rate_limit_gate, MAX_HINT_SECONDS, TEST_SERIAL}; + use std::io::{Read as _, Write as _}; + + let _serial = TEST_SERIAL.lock().await; + reset_rate_limit_gate(); + + // Use a std::net listener on a std::thread — the same pattern as the + // relay_admission loopback tests. This avoids two races that cause CI + // failures with tokio::net + into_std(): + // 1. No request read: the client is still sending when the response + // arrives → hyper `UnexpectedMessage`/`Canceled` under load. + // 2. into_std() leaves the socket in nonblocking mode → write_all + // may return WouldBlock and silently drop the response. + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + + // Serve a 429 with a hint far exceeding MAX_HINT_SECONDS (300). + let oversized = 1_000_000u64; + let body = format!(r#"{{"error":"rate-limited: quota exceeded; retry in {oversized}s"}}"#); + let body_len = body.len(); + std::thread::spawn(move || { + if let Ok((mut stream, _)) = listener.accept() { + // Read the request first so the client finishes sending before + // we write the response — mirrors relay_admission.rs pattern. + let mut buf = [0u8; 4096]; + let _ = stream.read(&mut buf); + let response = format!( + "HTTP/1.1 429 Too Many Requests\r\nContent-Type: application/json\r\nContent-Length: {body_len}\r\nConnection: close\r\n\r\n{body}" + ); + let _ = stream.write_all(response.as_bytes()); + let _ = stream.flush(); + } + }); + + let client = reqwest::Client::new(); + let response = client + .get(format!("http://{addr}/")) + .send() + .await + .expect("request must succeed"); + + let msg = super::relay_error_message(response).await; + + // The message must embed the CAPPED hint, not the raw 1 000 000. + assert_eq!( + msg, + format!("relay rate-limited: retry in {MAX_HINT_SECONDS}s"), + "relay_error_message must embed the capped hint, not the raw untrusted value" + ); + assert!( + !msg.contains(&oversized.to_string()), + "raw oversized hint must not appear in the message string" + ); + reset_rate_limit_gate(); +} + +// ── effective_agent_relay_url: legacy pin ignored ───────────────────────── + +#[test] +fn stored_relay_pin_is_ignored() { + // Zero-touch cutover (#2122): a creation-era per-record relay pin is + // parsed and persisted but never consulted — the workspace relay wins. + assert_eq!( + effective_agent_relay_url("wss://relay.other.com", "wss://staging.example.com"), + "wss://staging.example.com" + ); +} + +#[test] +fn empty_relay_resolves_to_workspace() { + // A never-set record resolves to the active workspace relay at read-time, + // so a stale stored default can never make it load-bearing. + assert_eq!( + effective_agent_relay_url("", "wss://staging.example.com"), + "wss://staging.example.com" + ); +} + +#[test] +fn whitespace_only_relay_resolves_to_workspace() { + // Whitespace-only behaves identically — no value survives. + assert_eq!( + effective_agent_relay_url(" ", "wss://staging.example.com"), + "wss://staging.example.com" + ); +} + +// ── relay_http_base_url scheme conversion ──────────────────────────────── + +#[test] +fn loopback_ws_localhost_preserves_authority() { + // Tenant host-binding keys off the HTTP Host/authority. The desktop must + // not rewrite localhost to 127.0.0.1, or local dev HTTP calls target a + // different unmapped community than the WebSocket URL. + assert_eq!( + relay_http_base_url("ws://localhost:3000"), + "http://localhost:3000" + ); +} + +#[test] +fn loopback_trailing_slash_removed_authority_preserved() { + assert_eq!( + relay_http_base_url("ws://localhost:3000/"), + "http://localhost:3000" + ); +} + +#[test] +fn remote_wss_host_unchanged() { + assert_eq!( + relay_http_base_url("wss://relay.example.com"), + "https://relay.example.com" + ); +} + +#[test] +fn loopback_ipv4_literal_unchanged() { + assert_eq!( + relay_http_base_url("ws://127.0.0.1:3000"), + "http://127.0.0.1:3000" + ); +} + +#[test] +fn localhost_substring_host_unchanged() { + assert_eq!( + relay_http_base_url("ws://localhost.evil.com:3000"), + "http://localhost.evil.com:3000" + ); +} + +#[test] +fn loopback_wss_localhost_preserves_authority() { + assert_eq!( + relay_http_base_url("wss://localhost:3000"), + "https://localhost:3000" + ); +} + +// ── classify_intercepted_response ──────────────────────────────────────── + +#[test] +fn intercepted_cloudflare_host_returns_some() { + let result = classify_intercepted_response("sqprod.cloudflareaccess.com", "text/html"); + assert!(result.is_some()); + let msg = result.unwrap(); + assert!( + msg.starts_with("relay unreachable:"), + "should have unreachable prefix" + ); + assert!(msg.contains("Cloudflare"), "should mention Cloudflare"); +} + +#[test] +fn intercepted_cloudflare_apex_host_returns_some() { + // The apex domain itself should also match. + let result = classify_intercepted_response("cloudflareaccess.com", "application/json"); + assert!(result.is_some()); + let msg = result.unwrap(); + assert!(msg.starts_with("relay unreachable:")); + assert!(msg.contains("Cloudflare")); +} + +#[test] +fn intercepted_non_cloudflare_html_returns_some() { + let result = + classify_intercepted_response("proxy.corporate.example", "text/html; charset=utf-8"); + assert!(result.is_some()); + let msg = result.unwrap(); + assert!(msg.starts_with("relay unreachable:")); +} + +#[test] +fn normal_relay_json_returns_none() { + let result = classify_intercepted_response("relay.myapp.example.com", "application/json"); + assert!(result.is_none()); +} + +#[test] +fn content_type_case_insensitive() { + // Uppercase content-type must still be detected. + let result = classify_intercepted_response("proxy.example.com", "TEXT/HTML"); + assert!(result.is_some()); + assert!(result.unwrap().starts_with("relay unreachable:")); +} + +#[test] +fn evil_suffix_does_not_match_cloudflare() { + // A host whose suffix happens to contain the Cloudflare string but is + // not actually a subdomain must NOT match. + let result = + classify_intercepted_response("notcloudflareaccess.com.evil.example", "application/json"); + assert!( + result.is_none(), + "false suffix match should not trigger Cloudflare branch" + ); +} + +// classify_request_error requires a real reqwest::Error (not publicly +// constructable) — tested indirectly through integration; skipped here. + +// ── /query per-request timeout → classified error ──────────────────────── +// +// A stalled `/query` connection (headers never arrive) must not hang the +// caller forever. Both production `/query` builders funnel through +// `send_query_request`, which owns the per-request `.timeout(...)`; this test +// drives that exact helper against a loopback server that accepts the +// connection but never responds. It asserts two things the frontend depends +// on: (1) the helper returns instead of hanging, and (2) the failure is the +// stable `"relay unreachable: request timed out"` classified string. +// +// The outer `tokio::time::timeout` is the regression guard: if the production +// `.timeout(...)` is ever removed from `send_query_request`, this call would +// hang forever, so the guard fires and the test fails fast rather than +// stalling CI. A short 200ms deadline keeps the happy path fast. +#[tokio::test] +async fn stalled_query_request_times_out_with_classified_error() { + use std::io::Read as _; + use std::time::Duration; + + // A listener that accepts the connection and then holds it open without + // ever writing a response — the "headers never arrive" stall. + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + let handle = std::thread::spawn(move || { + if let Ok((mut stream, _)) = listener.accept() { + // Drain the request but deliberately never respond, then hold + // the socket until the client aborts on its own timeout. + let mut buf = [0u8; 4096]; + let _ = stream.read(&mut buf); + std::thread::sleep(Duration::from_secs(2)); + } + }); + + let client = reqwest::Client::new(); + let url = format!("http://{addr}/query"); + let result = tokio::time::timeout( + Duration::from_secs(5), + super::send_query_request( + &client, + &url, + "Nostr test-auth", + None, + b"[]".to_vec(), + Duration::from_millis(200), + ), + ) + .await + .expect( + "send_query_request must honor its per-request timeout and resolve within 5s; \ + if this guard fires, the production .timeout(...) was lost", + ); + + let err = result.expect_err("a stalled /query must surface an error, not succeed"); + assert_eq!( + err, "relay unreachable: request timed out", + "a timed-out /query must surface the stable classified string" + ); + + let _ = handle.join(); +} + +// ── /query body-stall timeout → classified error (not malformed) ───────── +// +// `send()` resolves once response headers arrive, so a relay that returns a +// valid 2xx JSON header block and then stalls the body trips the request +// deadline inside `response.json()` — the branch the pre-header stall above +// cannot reach. That is a connectivity failure, not a malformed body, so it +// must surface the stable "relay unreachable: request timed out" string rather +// than the malformed-response bucket. This drives `send_query_request` against +// a loopback that writes headers promising a body it never sends. +#[tokio::test] +async fn stalled_response_body_times_out_with_classified_error() { + use std::io::{Read as _, Write as _}; + use std::time::Duration; + + // Accept, drain the request, write a complete 2xx JSON header block that + // promises a body (Content-Length), then send nothing and hold the socket + // — the "headers arrive, body stalls" half-open case. + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + let handle = std::thread::spawn(move || { + if let Ok((mut stream, _)) = listener.accept() { + let mut buf = [0u8; 4096]; + let _ = stream.read(&mut buf); + let _ = stream.write_all( + b"HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: 64\r\n\r\n", + ); + let _ = stream.flush(); + // Never write the promised body; hold past the client deadline. + std::thread::sleep(Duration::from_secs(2)); + } + }); + + let client = reqwest::Client::new(); + let url = format!("http://{addr}/query"); + let result = tokio::time::timeout( + Duration::from_secs(5), + super::send_query_request( + &client, + &url, + "Nostr test-auth", + None, + b"[]".to_vec(), + Duration::from_millis(200), + ), + ) + .await + .expect( + "send_query_request must honor its per-request timeout through body \ + consumption and resolve within 5s", + ); + + let err = result.expect_err("a stalled response body must surface an error, not succeed"); + assert_eq!( + err, "relay unreachable: request timed out", + "a body-stall timeout must surface the classified timeout string, not the \ + malformed-response bucket" + ); + + let _ = handle.join(); +} + +// ── /query non-2xx body-stall timeout → classified error (not status) ──── +// +// The 2xx path is not the only body-consuming path. A relay that returns a +// non-success status (500, 429, …) routes through `relay_error_message`, which +// consumes the body via `text()` to extract the structured error field. If the +// relay sends the status headers and then stalls the promised body, that +// consumption trips the same request deadline — and it must surface the stable +// "relay unreachable: request timed out" classification, not a bare +// "relay returned 500" that hides the connectivity failure. This drives +// `send_query_request` against a loopback that writes 500 headers promising a +// body it never sends. +#[tokio::test] +async fn stalled_error_response_body_times_out_with_classified_error() { + use std::io::{Read as _, Write as _}; + use std::time::Duration; + + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + let handle = std::thread::spawn(move || { + if let Ok((mut stream, _)) = listener.accept() { + let mut buf = [0u8; 4096]; + let _ = stream.read(&mut buf); + // 500 status headers promising a body (Content-Length) that never + // arrives — the "error headers arrive, body stalls" half-open case. + let _ = stream.write_all( + b"HTTP/1.1 500 Internal Server Error\r\nContent-Type: application/json\r\nContent-Length: 64\r\n\r\n", + ); + let _ = stream.flush(); + std::thread::sleep(Duration::from_secs(2)); + } + }); + + let client = reqwest::Client::new(); + let url = format!("http://{addr}/query"); + let result = tokio::time::timeout( + Duration::from_secs(5), + super::send_query_request( + &client, + &url, + "Nostr test-auth", + None, + b"[]".to_vec(), + Duration::from_millis(200), + ), + ) + .await + .expect( + "send_query_request must honor its per-request timeout through error-body \ + consumption and resolve within 5s", + ); + + let err = result.expect_err("a stalled error-response body must surface an error, not succeed"); + assert_eq!( + err, "relay unreachable: request timed out", + "a non-2xx body-stall timeout must surface the classified timeout string, not the \ + status bucket" + ); + + let _ = handle.join(); +} + +// ── /query non-stalled 500 → status message (timeout preservation is scoped) ─ +// +// The timeout preservation above must not swallow genuine relay errors: a 500 +// whose body arrives promptly still surfaces as "relay returned 500". This +// pins that `classify_body_timeout` only fires on an actual timeout, so the +// error-classification path stays intact for live relay failures. +#[tokio::test] +async fn non_stalled_error_response_yields_status_message() { + use std::io::{Read as _, Write as _}; + use std::time::Duration; + + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + let handle = std::thread::spawn(move || { + if let Ok((mut stream, _)) = listener.accept() { + let mut buf = [0u8; 4096]; + let _ = stream.read(&mut buf); + // A complete 500 with a non-JSON body delivered immediately. + let body = "internal error"; + let response = format!( + "HTTP/1.1 500 Internal Server Error\r\nContent-Type: text/plain\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", + body.len() + ); + let _ = stream.write_all(response.as_bytes()); + let _ = stream.flush(); + } + }); + + let client = reqwest::Client::new(); + let url = format!("http://{addr}/query"); + let result = tokio::time::timeout( + Duration::from_secs(5), + super::send_query_request( + &client, + &url, + "Nostr test-auth", + None, + b"[]".to_vec(), + Duration::from_millis(200), + ), + ) + .await + .expect("a promptly-served 500 must resolve well within 5s"); + + let err = result.expect_err("a 500 must surface an error, not succeed"); + assert_eq!( + err, "relay returned 500 Internal Server Error", + "a non-stalled 500 must keep its status classification, not be reclassified as a timeout" + ); + + let _ = handle.join(); +} + +// ── parse_json_response malformed-body contract ────────────────────────── + +#[test] +fn malformed_response_message_stays_off_unreachable_bucket() { + // A reached-but-malformed 2xx body is not a connectivity failure. If this + // message ever regains the "relay unreachable:" prefix, the frontend + // classifier would misroute it as unreachable — pin that it never does. + assert!( + !MALFORMED_RESPONSE_MESSAGE.starts_with("relay unreachable:"), + "malformed-response message must not match the unreachable prefix" + ); +} + +// ── parse_command_response ─────────────────────────────────────────────── + +#[derive(Debug, Deserialize, PartialEq)] +struct ChannelCreated { + channel_id: String, +} + +#[test] +fn parse_command_response_decodes_typed_payload() { + let msg = r#"response:{"channel_id":"abc123"}"#; + let parsed: ChannelCreated = parse_command_response(msg).expect("should parse"); + assert_eq!( + parsed, + ChannelCreated { + channel_id: "abc123".to_string() + } + ); +} + +#[test] +fn parse_command_response_accepts_raw_json_fallback() { + // Backward-compat: relays that emit raw JSON (no prefix) still work. + let msg = r#"{"channel_id":"abc"}"#; + let parsed: ChannelCreated = parse_command_response(msg).expect("fallback parse"); + assert_eq!( + parsed, + ChannelCreated { + channel_id: "abc".to_string() + } + ); +} + +#[test] +fn parse_command_response_rejects_invalid_prefixed_json() { + let msg = "response:not-json"; + let result: Result = parse_command_response(msg); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("response parse failed")); +} + +#[test] +fn parse_command_response_rejects_garbage() { + let msg = "totally not json or response"; + let result: Result = parse_command_response(msg); + assert!(result.is_err()); +} + +// ── build_profile_event ────────────────────────────────────────────────── + +/// Generate a valid NIP-OA auth tag JSON string signed by a fresh owner key +/// and addressed to `agent_keys`. +/// +/// Uses `nostr_compat` (nostr 0.36) for the owner keys because +/// `buzz_sdk_pkg::nip_oa::compute_auth_tag` expects nostr 0.36 types. +/// The agent pubkey is bridged via hex encoding. +fn make_valid_auth_tag(agent_keys: &nostr::Keys) -> String { + let owner_keys = nostr::Keys::generate(); + let agent_pubkey_hex = agent_keys.public_key().to_hex(); + let agent_compat_pubkey = + nostr::PublicKey::from_hex(&agent_pubkey_hex).expect("valid hex pubkey should parse"); + buzz_sdk_pkg::nip_oa::compute_auth_tag(&owner_keys, &agent_compat_pubkey, "") + .expect("compute_auth_tag should not fail with distinct keys") +} + +#[test] +fn profile_event_with_valid_auth_tag() { + let agent_keys = nostr::Keys::generate(); + let tag_json = make_valid_auth_tag(&agent_keys); + let event = build_profile_event(&agent_keys, "TestBot", None, Some(&tag_json)) + .expect("should succeed with a valid auth tag"); + + // Exactly one "auth" tag must be present. + let auth_tags: Vec<_> = event + .tags + .iter() + .filter(|t| t.as_slice().first().map(|s| s.as_str()) == Some("auth")) + .collect(); + assert_eq!(auth_tags.len(), 1, "expected exactly 1 auth tag"); + + // Must be a kind:0 (Metadata) event. + assert_eq!(event.kind, nostr::Kind::Metadata); +} + +#[test] +fn profile_event_without_auth_tag() { + let agent_keys = nostr::Keys::generate(); + let event = build_profile_event(&agent_keys, "TestBot", None, None) + .expect("should succeed without an auth tag"); + + // No "auth" tags should be present. + let auth_tags: Vec<_> = event + .tags + .iter() + .filter(|t| t.as_slice().first().map(|s| s.as_str()) == Some("auth")) + .collect(); + assert_eq!(auth_tags.len(), 0, "expected no auth tags"); + + assert_eq!(event.kind, nostr::Kind::Metadata); +} + +#[test] +fn profile_event_rejects_invalid_auth_tag() { + let agent_keys = nostr::Keys::generate(); + // Structurally valid JSON array but with a bogus signature — verification must fail. + let bad_json = format!(r#"["auth","{}","","{}"]"#, "a".repeat(64), "b".repeat(128)); + let result = build_profile_event(&agent_keys, "TestBot", None, Some(&bad_json)); + assert!(result.is_err(), "should reject an invalid auth tag"); + assert!( + result.unwrap_err().contains("verification failed"), + "error message should mention verification failure" + ); +} diff --git a/desktop/src/features/channels/ui/ChannelPane.tsx b/desktop/src/features/channels/ui/ChannelPane.tsx index bccc163ed40..93eb3803f17 100644 --- a/desktop/src/features/channels/ui/ChannelPane.tsx +++ b/desktop/src/features/channels/ui/ChannelPane.tsx @@ -13,6 +13,7 @@ import { useRelaySelfQuery } from "@/features/moderation/hooks"; import { DropZoneOverlay } from "@/features/messages/ui/ComposerAttachments"; import { MessageThreadPanel } from "@/features/messages/ui/MessageThreadPanel"; import { MessageThreadPanelSkeleton } from "@/features/messages/ui/MessageThreadPanelSkeleton"; +import { ThreadRepliesErrorCard } from "@/features/messages/ui/MessageThreadReplyState"; import { MessageTimeline, type MessageTimelineHandle, @@ -95,6 +96,8 @@ export const ChannelPane = React.memo(function ChannelPane({ welcomeKickoffSettingUp = false, messages, threadSummaries, + huddleThreadRepliesError = false, + onRetryHuddleThreadReplies, firstUnreadMessageId = null, unreadCount = 0, canResetThreadPanelWidth, @@ -151,6 +154,8 @@ export const ChannelPane = React.memo(function ChannelPane({ threadHeadMessage, threadMessages, threadMessagesPending = false, + threadMessagesError = false, + onRetryThreadReplies, threadPanelWidthPx, threadScrollTargetId, threadTypingPubkeys, @@ -567,6 +572,11 @@ export const ChannelPane = React.memo(function ChannelPane({ } > {isHuddleTranscript ? null : header} + {isHuddleTranscript && huddleThreadRepliesError ? ( +
+ +
+ ) : null}
; + /** + * A Huddle transcript flattens summarized reply subtrees into the chat + * timeline. When one of those subtree loads fails, this reports the aggregate + * failure so the transcript can surface a non-destructive retry alert instead + * of silently presenting a partial conversation as complete. + */ + huddleThreadRepliesError?: boolean; + onRetryHuddleThreadReplies?: () => void; firstUnreadMessageId?: string | null; unreadCount?: number; canResetThreadPanelWidth: boolean; @@ -170,6 +178,8 @@ export type ChannelPaneProps = { threadAllMessages: TimelineMessage[]; threadMessages: MainTimelineEntry[]; threadMessagesPending?: boolean; + threadMessagesError?: boolean; + onRetryThreadReplies?: () => void; threadPanelWidthPx: number; threadTypingPubkeys: string[]; threadReplyTargetMessage: TimelineMessage | null; diff --git a/desktop/src/features/channels/ui/ChannelScreen.tsx b/desktop/src/features/channels/ui/ChannelScreen.tsx index 68df9bc05c6..06788de56f0 100644 --- a/desktop/src/features/channels/ui/ChannelScreen.tsx +++ b/desktop/src/features/channels/ui/ChannelScreen.tsx @@ -245,7 +245,12 @@ export function ChannelScreen({ const deleteMessageMutation = useDeleteMessageMutation(activeChannel); const editMessageMutation = useEditMessageMutation(activeChannel); const joinChannelMutation = useJoinChannelMutation(activeChannelId); - const { resolvedMessages, threadSummaries } = useHuddleChannelMessages({ + const { + resolvedMessages, + threadSummaries, + threadRepliesError: huddleThreadRepliesError, + onRetryThreadReplies: onRetryHuddleThreadReplies, + } = useHuddleChannelMessages({ activeChannel, isHuddleTranscript, messages: messagesQuery.data ?? EMPTY_RELAY_EVENTS, @@ -886,6 +891,8 @@ export function ChannelScreen({ isTimelineLoading={isTimelineLoading} messages={timelineMessages} threadSummaries={threadSummaries} + huddleThreadRepliesError={huddleThreadRepliesError} + onRetryHuddleThreadReplies={onRetryHuddleThreadReplies} onCancelEdit={handleCancelEdit} onCancelThreadReply={handleCancelThreadReply} onChannelManagementDeleted={handleChannelManagementDeleted} @@ -955,6 +962,10 @@ export function ChannelScreen({ threadHeadMessage={displayedThreadHeadMessage} threadMessages={displayedThreadMessages} threadMessagesPending={threadRepliesQuery.isPending} + threadMessagesError={threadRepliesQuery.isError} + onRetryThreadReplies={() => { + void threadRepliesQuery.refetch(); + }} threadPanelWidthPx={threadPanelWidthPx} threadTypingPubkeys={threadTypingPubkeys} threadReplyTargetMessage={displayedThreadReplyTargetMessage} diff --git a/desktop/src/features/channels/ui/useHuddleChannelMessages.ts b/desktop/src/features/channels/ui/useHuddleChannelMessages.ts index 2a90971ddec..5a3a7c40419 100644 --- a/desktop/src/features/channels/ui/useHuddleChannelMessages.ts +++ b/desktop/src/features/channels/ui/useHuddleChannelMessages.ts @@ -66,5 +66,13 @@ export function useHuddleChannelMessages({ [huddleThreadReplies.events, isHuddleTranscript, resolvedChannelMessages], ); - return { resolvedMessages, threadSummaries }; + return { + resolvedMessages, + threadSummaries, + // A summarized reply subtree failing must not leave the transcript reading + // as complete: surface the aggregate failure so the consumer can show a + // non-destructive retry alert alongside the rows that did load. + threadRepliesError: isHuddleTranscript && huddleThreadReplies.isError, + onRetryThreadReplies: huddleThreadReplies.refetch, + }; } diff --git a/desktop/src/features/messages/combineThreadRepliesResults.test.mjs b/desktop/src/features/messages/combineThreadRepliesResults.test.mjs new file mode 100644 index 00000000000..47beb1c7a83 --- /dev/null +++ b/desktop/src/features/messages/combineThreadRepliesResults.test.mjs @@ -0,0 +1,104 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { combineThreadRepliesResults } from "./useThreadReplies.ts"; + +const CHANNEL_A = "a".repeat(64); +const CHANNEL_B = "b".repeat(64); + +function event(id, createdAt) { + return { + id, + pubkey: "c".repeat(64), + kind: 9, + created_at: createdAt, + content: "reply", + tags: [], + sig: "sig", + }; +} + +function ok(data) { + return { + data, + isPending: false, + isError: false, + error: null, + refetch: () => { + throw new Error("a successful subtree must not be refetched"); + }, + }; +} + +function failed(refetch) { + return { + data: undefined, + isPending: false, + isError: true, + error: new Error("subtree load failed"), + refetch, + }; +} + +function pending() { + return { + data: undefined, + isPending: true, + isError: false, + error: null, + refetch: () => {}, + }; +} + +test("aggregates events across roots in chronological order", () => { + const combined = combineThreadRepliesResults([ + ok([event(CHANNEL_A, 200)]), + ok([event(CHANNEL_B, 100)]), + ]); + assert.deepEqual( + combined.events.map((e) => e.created_at), + [100, 200], + ); + assert.equal(combined.isPending, false); + assert.equal(combined.isError, false); + assert.equal(combined.error, null); +}); + +test("a failed subtree surfaces aggregate error and never silently drops", () => { + // The load-bearing contract: one failed root among successful roots must make + // the aggregate report isError so the consumer can surface a failure instead + // of presenting a partial transcript as complete. + const combined = combineThreadRepliesResults([ + ok([event(CHANNEL_A, 100)]), + failed(() => {}), + ]); + assert.equal(combined.isError, true); + assert.ok(combined.error instanceof Error); + // Successful rows still contribute their events (non-destructive). + assert.equal(combined.events.length, 1); +}); + +test("isPending reflects any still-loading root", () => { + const combined = combineThreadRepliesResults([ok([]), pending()]); + assert.equal(combined.isPending, true); +}); + +test("refetch re-runs only the failed subtrees, not the successful ones", () => { + let failedRefetched = 0; + const combined = combineThreadRepliesResults([ + ok([event(CHANNEL_A, 100)]), + failed(() => { + failedRefetched += 1; + }), + ]); + // ok().refetch throws if called, so a partial-success refetch that touched + // every query would throw here; it must only touch the failed one. + combined.refetch(); + assert.equal(failedRefetched, 1); +}); + +test("all-success aggregate reports no error", () => { + const combined = combineThreadRepliesResults([ok([]), ok([])]); + assert.equal(combined.isError, false); + assert.equal(combined.error, null); +}); diff --git a/desktop/src/features/messages/lib/threadPanel.ts b/desktop/src/features/messages/lib/threadPanel.ts index ebc1b35ae7e..6dc1190910d 100644 --- a/desktop/src/features/messages/lib/threadPanel.ts +++ b/desktop/src/features/messages/lib/threadPanel.ts @@ -542,3 +542,59 @@ export function buildThreadPanelData( expandedReplyIds, ); } + +function hasLaterVisibleSibling( + entries: readonly MainTimelineEntry[], + entryIndex: number, +): boolean { + const depth = entries[entryIndex]?.message.depth; + if (depth == null) { + return false; + } + + for (let index = entryIndex + 1; index < entries.length; index += 1) { + const nextDepth = entries[index].message.depth; + if (nextDepth <= depth) { + return nextDepth === depth; + } + } + + return false; +} + +/** + * Depths at which a vertical thread-branch guide should continue past `message` + * because an ancestor on its path still has a later visible sibling. Pure so + * the branch-guide geometry is unit-tested without the panel. + */ +export function getActiveContinuationDepths({ + ancestors, + entries, + index, + message, +}: { + ancestors: readonly { index: number; message: TimelineMessage }[]; + entries: readonly MainTimelineEntry[]; + index: number; + message: TimelineMessage; +}): number[] { + const depths: number[] = []; + + for (const ancestor of ancestors) { + if (ancestor.message.depth === 0) { + continue; + } + + const childDepth = ancestor.message.depth + 1; + const pathChild = + message.depth === childDepth + ? { index, message } + : ancestors.find((candidate) => candidate.message.depth === childDepth); + + if (pathChild && hasLaterVisibleSibling(entries, pathChild.index)) { + depths.push(ancestor.message.depth); + } + } + + return depths; +} diff --git a/desktop/src/features/messages/lib/threadReplyHighlight.test.mjs b/desktop/src/features/messages/lib/threadReplyHighlight.test.mjs new file mode 100644 index 00000000000..bad72b33395 --- /dev/null +++ b/desktop/src/features/messages/lib/threadReplyHighlight.test.mjs @@ -0,0 +1,86 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { selectThreadRowHighlight } from "./threadReplyHighlight.ts"; + +// The hovered branch spans rows in the half-open range (startIndex, endIndex]. +const branch = { id: "b", depth: 1, startIndex: 2, endIndex: 5 }; + +test("row-highlight: null branch highlights nothing", () => { + assert.deepEqual( + selectThreadRowHighlight({ + branch: null, + index: 3, + messageId: "x", + messageDepth: 2, + showGuides: true, + }), + { + isBranchOwner: false, + isInsideBranch: false, + isDirectChild: false, + lineDepths: undefined, + }, + ); +}); + +test("row-highlight: the branch owner is flagged but is not inside its own range", () => { + const h = selectThreadRowHighlight({ + branch, + index: 2, + messageId: "b", + messageDepth: 1, + showGuides: true, + }); + assert.equal(h.isBranchOwner, true); + // startIndex is excluded, so the owner row itself is not "inside". + assert.equal(h.isInsideBranch, false); + assert.equal(h.lineDepths, undefined); +}); + +test("row-highlight: a direct child inside the branch draws the guide line", () => { + const h = selectThreadRowHighlight({ + branch, + index: 3, + messageId: "c", + messageDepth: 2, + showGuides: true, + }); + assert.equal(h.isInsideBranch, true); + assert.equal(h.isDirectChild, true); + assert.deepEqual(h.lineDepths, [1]); +}); + +test("row-highlight: a deeper descendant is inside but not a direct child", () => { + const h = selectThreadRowHighlight({ + branch, + index: 4, + messageId: "d", + messageDepth: 3, + showGuides: true, + }); + assert.equal(h.isInsideBranch, true); + assert.equal(h.isDirectChild, false); +}); + +test("row-highlight: a row past endIndex is outside the branch", () => { + const h = selectThreadRowHighlight({ + branch, + index: 6, + messageId: "e", + messageDepth: 2, + showGuides: true, + }); + assert.equal(h.isInsideBranch, false); +}); + +test("row-highlight: guides suppressed → no line depths even inside the branch", () => { + const h = selectThreadRowHighlight({ + branch, + index: 3, + messageId: "c", + messageDepth: 2, + showGuides: false, + }); + assert.equal(h.isInsideBranch, true); + assert.equal(h.lineDepths, undefined); +}); diff --git a/desktop/src/features/messages/lib/threadReplyHighlight.ts b/desktop/src/features/messages/lib/threadReplyHighlight.ts new file mode 100644 index 00000000000..0ce446daecb --- /dev/null +++ b/desktop/src/features/messages/lib/threadReplyHighlight.ts @@ -0,0 +1,45 @@ +/** + * The hovered collapse-branch range, or null when nothing is hovered. Rows whose + * index falls inside `(startIndex, endIndex]` belong to the branch. + */ +export type HighlightedThreadBranch = { + id: string; + depth: number; + startIndex: number; + endIndex: number; +} | null; + +/** Per-row branch-highlight flags derived from the hovered collapse branch. */ +export type ThreadRowHighlight = { + isBranchOwner: boolean; + isInsideBranch: boolean; + isDirectChild: boolean; + lineDepths: number[] | undefined; +}; + +/** + * Which highlight decorations a reply row shows for the hovered collapse branch. + * Pure so the index-range logic is unit-testable without rendering the panel. + */ +export function selectThreadRowHighlight({ + branch, + index, + messageId, + messageDepth, + showGuides, +}: { + branch: HighlightedThreadBranch; + index: number; + messageId: string; + messageDepth: number; + showGuides: boolean; +}): ThreadRowHighlight { + const isBranchOwner = branch?.id === messageId; + const isInsideBranch = + branch != null && index > branch.startIndex && index <= branch.endIndex; + const isDirectChild = + isInsideBranch && branch != null && messageDepth === branch.depth + 1; + const lineDepths = + showGuides && isInsideBranch && branch ? [branch.depth] : undefined; + return { isBranchOwner, isInsideBranch, isDirectChild, lineDepths }; +} diff --git a/desktop/src/features/messages/lib/timelineSnapshot.test.mjs b/desktop/src/features/messages/lib/timelineSnapshot.test.mjs index a0374fbe2b0..5334d58875f 100644 --- a/desktop/src/features/messages/lib/timelineSnapshot.test.mjs +++ b/desktop/src/features/messages/lib/timelineSnapshot.test.mjs @@ -14,6 +14,7 @@ import { selectLatestMessageKey, selectTimelineBodySurface, selectTimelineIntroSurface, + selectThreadRepliesSurface, } from "./timelineSnapshot.ts"; // Local-midnight unix-second timestamps so isSameDay (local time) is stable @@ -399,6 +400,126 @@ test("deferred-render: keys the empty decision off the live count, not deferred" assert.equal(selectDeferredListRenderState(0, 1), "pending"); }); +// ── selectThreadRepliesSurface ────────────────────────────────────────────── +// PR-1 defect 2: a terminal thread-load error must NEVER be presented as the +// authoritative "No replies in this branch yet" empty state. These pin the +// paint precedence that gates that in MessageThreadPanel. + +test("thread-surface: pending query paints the skeleton", () => { + assert.equal( + selectThreadRepliesSurface({ + isPending: true, + isError: false, + renderState: "empty", + }), + "skeleton", + ); +}); + +test("thread-surface: terminal error with no data paints error, never empty", () => { + // The core false-empty guard: the load failed (isError) and there is nothing + // cached (renderState "empty"). This MUST be "error" so the UI shows + // "Couldn't load replies" + Retry instead of an authoritative empty thread. + const surface = selectThreadRepliesSurface({ + isPending: false, + isError: true, + renderState: "empty", + }); + assert.equal(surface, "error"); + assert.notEqual(surface, "empty"); +}); + +test("thread-surface: page-2 failure with no committed rows never claims empty", () => { + // A later-page fetch rejects the whole attempt; partial rows are never + // committed, so the deferred+live lists are empty and isError is set. The + // surface must be "error", never "empty" — the thread is not known-empty. + assert.equal( + selectThreadRepliesSurface({ + isPending: false, + isError: true, + renderState: "empty", + }), + "error", + ); +}); + +test("thread-surface: cached rows stay visible even under a load error", () => { + // An error with cached replies (renderState "list") keeps painting the rows + // non-destructively rather than blanking them for the error card. + assert.equal( + selectThreadRepliesSurface({ + isPending: false, + isError: true, + renderState: "list", + }), + "list", + ); +}); + +test("thread-surface: successful empty load paints the empty state", () => { + // No error, genuinely no replies → the real empty affordance is correct. + assert.equal( + selectThreadRepliesSurface({ + isPending: false, + isError: false, + renderState: "empty", + }), + "empty", + ); +}); + +test("thread-surface: retry success renders the reply list", () => { + // After a Retry re-fetch succeeds, isError clears and rows commit + // (renderState "list") → the list body paints, replacing the error card. + assert.equal( + selectThreadRepliesSurface({ + isPending: false, + isError: false, + renderState: "list", + }), + "list", + ); +}); + +test("thread-surface: streaming-in rows paint nothing (pending), not empty", () => { + assert.equal( + selectThreadRepliesSurface({ + isPending: false, + isError: false, + renderState: "pending", + }), + "pending", + ); +}); + +test("thread-surface: huddle transcripts collapse non-list surfaces to pending", () => { + // Huddle transcripts flatten replies into the chat timeline, so they never + // show the skeleton/error/empty affordances — only the list body or nothing. + for (const isError of [false, true]) { + for (const renderState of ["empty", "pending"]) { + assert.equal( + selectThreadRepliesSurface({ + isPending: false, + isError, + renderState, + isHuddleTranscript: true, + }), + "pending", + ); + } + } + // The list body still paints for a transcript with rows. + assert.equal( + selectThreadRepliesSurface({ + isPending: false, + isError: false, + renderState: "list", + isHuddleTranscript: true, + }), + "list", + ); +}); + test("timeline-body-surface: loading and deferred-pending both paint the single static skeleton", () => { assert.equal( selectTimelineBodySurface({ diff --git a/desktop/src/features/messages/lib/timelineSnapshot.ts b/desktop/src/features/messages/lib/timelineSnapshot.ts index 3bfd9349476..4e23fb22453 100644 --- a/desktop/src/features/messages/lib/timelineSnapshot.ts +++ b/desktop/src/features/messages/lib/timelineSnapshot.ts @@ -209,6 +209,74 @@ export function selectTimelineBodySurface({ return renderState; } +/** + * Which surface the thread-reply body should paint, in strict precedence. + * + * Extracted as a pure function so the load-bearing invariant — a terminal fetch + * error must NEVER be shown as the "empty" (no-replies) state — is unit-tested + * without a DOM. The precedence mirrors the JSX branch order in + * `MessageThreadPanel`: + * + * 1. "skeleton" → the query is still pending (first load, no cache) + * 2. "list" → the deferred snapshot has rows; paint them (even under a + * later error, cached replies stay visible non-destructively) + * 3. "error" → the load terminally failed and there is nothing to show; + * paint "Couldn't load replies" + Retry, never the empty state + * 4. "empty" → the load succeeded and the branch is genuinely empty + * 5. "pending" → deferred is empty but the live list has content; paint + * nothing yet (rows are streaming in on the deferred commit) + * + * Huddle transcripts flatten replies into the chat timeline and never show the + * skeleton/error/empty affordances, so their non-list surfaces collapse to + * "pending" (render nothing). + */ +export type ThreadRepliesSurface = + | "skeleton" + | "list" + | "error" + | "empty" + | "pending"; + +export function selectThreadRepliesSurface({ + isPending, + isError, + renderState, + isHuddleTranscript = false, +}: { + isPending: boolean; + isError: boolean; + renderState: DeferredListRenderState; + isHuddleTranscript?: boolean; +}): ThreadRepliesSurface { + const surface = resolveThreadRepliesSurface({ + isPending, + isError, + renderState, + }); + return isHuddleTranscript && surface !== "list" ? "pending" : surface; +} + +function resolveThreadRepliesSurface({ + isPending, + isError, + renderState, +}: { + isPending: boolean; + isError: boolean; + renderState: DeferredListRenderState; +}): ThreadRepliesSurface { + if (isPending) { + return "skeleton"; + } + if (renderState === "list") { + return "list"; + } + if (isError) { + return "error"; + } + return renderState; +} + export type TimelineMessageDelta = "prepend" | "append" | "replace" | "none"; export function classifyTimelineMessageDelta({ diff --git a/desktop/src/features/messages/ui/MessageThreadPanel.tsx b/desktop/src/features/messages/ui/MessageThreadPanel.tsx index d2650c84ac4..01fbcda7ffb 100644 --- a/desktop/src/features/messages/ui/MessageThreadPanel.tsx +++ b/desktop/src/features/messages/ui/MessageThreadPanel.tsx @@ -4,6 +4,7 @@ import { ArrowDown } from "lucide-react"; import { HuddleTranscriptIntro } from "@/features/huddle/components/HuddleTranscriptIntro"; import { buildThreadSummaryFromVisibleEntries, + getActiveContinuationDepths, hasNestedThreadBranches, type MainTimelineEntry, } from "@/features/messages/lib/threadPanel"; @@ -41,12 +42,14 @@ import { import type { ThreadDepthGuideAction } from "./MessageRow"; import { MessageThreadRow } from "./MessageThreadRow"; import { MessageThreadSummaryRow } from "./MessageThreadSummaryRow"; +import { ThreadReplyRegion } from "./MessageThreadReplyState"; import { TypingIndicatorRow } from "./TypingIndicatorRow"; import { UnreadDivider } from "./UnreadDivider"; import { useComposerHeightPadding } from "./useComposerHeightPadding"; import { useStableSendToChannel } from "./useStableSendToChannel"; import { useAnchoredScroll } from "./useAnchoredScroll"; import { selectDeferredListRenderState } from "@/features/messages/lib/timelineSnapshot"; +import { selectThreadRowHighlight } from "@/features/messages/lib/threadReplyHighlight"; type MessageThreadPanelProps = ThreadPanelLayoutProps & { channel: Channel | null; @@ -106,6 +109,10 @@ type MessageThreadPanelProps = ThreadPanelLayoutProps & { threadHead: TimelineMessage | null; threadReplies: MainTimelineEntry[]; threadRepliesPending?: boolean; + /** True when the thread-reply query terminally failed (all retries exhausted). */ + threadRepliesError?: boolean; + /** Retries the failed thread-reply load; wired to the query's `refetch`. */ + onRetryThreadReplies?: () => void; threadUnreadCount?: number; threadReplyUnreadCounts?: ReadonlyMap; threadTypingPubkeys: string[]; @@ -131,57 +138,6 @@ type MessageThreadPanelProps = ThreadPanelLayoutProps & { const EMPTY_THREAD_REPLIES: MainTimelineEntry[] = []; const THREAD_PANEL_SUMMARY_INDENT_OFFSET_REM = 0; -function hasLaterVisibleSibling( - entries: readonly MainTimelineEntry[], - entryIndex: number, -): boolean { - const depth = entries[entryIndex]?.message.depth; - if (depth == null) { - return false; - } - - for (let index = entryIndex + 1; index < entries.length; index += 1) { - const nextDepth = entries[index].message.depth; - if (nextDepth <= depth) { - return nextDepth === depth; - } - } - - return false; -} - -function getActiveContinuationDepths({ - ancestors, - entries, - index, - message, -}: { - ancestors: readonly { index: number; message: TimelineMessage }[]; - entries: readonly MainTimelineEntry[]; - index: number; - message: TimelineMessage; -}): number[] { - const depths: number[] = []; - - for (const ancestor of ancestors) { - if (ancestor.message.depth === 0) { - continue; - } - - const childDepth = ancestor.message.depth + 1; - const pathChild = - message.depth === childDepth - ? { index, message } - : ancestors.find((candidate) => candidate.message.depth === childDepth); - - if (pathChild && hasLaterVisibleSibling(entries, pathChild.index)) { - depths.push(ancestor.message.depth); - } - } - - return depths; -} - export function MessageThreadPanel({ channel, channelId, @@ -233,6 +189,8 @@ export function MessageThreadPanel({ videoReviewPresentation, threadReplies, threadRepliesPending = false, + threadRepliesError = false, + onRetryThreadReplies, threadUnreadCount, threadReplyUnreadCounts, threadTypingPubkeys, @@ -629,199 +587,187 @@ export function MessageThreadPanel({ className={cn(THREAD_PANEL_MESSAGE_GUTTER_CLASS, "pb-3 pt-0")} data-testid="message-thread-replies" > - {threadRepliesPending && !isHuddleTranscript ? ( -
- - -
- ) : repliesRenderState === "list" ? ( - visibleThreadHeadSummary ? ( + (
- + +
- ) : ( -
- {threadReplyRenderItems.map((item) => { - const { - collapseDepthGuideActions, - connectsToVisibleChild, - continuationDepths, - entry, - index, - isContinuation, - } = item; - const showUnreadDivider = - index > 0 && entry.message.id === firstUnreadReplyId; - const isHighlightedBranchOwner = - highlightedBranch?.id === entry.message.id; - const isInsideHighlightedBranch = - highlightedBranch != null && - index > highlightedBranch.startIndex && - index <= highlightedBranch.endIndex; - const isDirectChildOfHighlightedBranch = - isInsideHighlightedBranch && - highlightedBranch != null && - index > highlightedBranch.startIndex && - index <= highlightedBranch.endIndex && - entry.message.depth === highlightedBranch.depth + 1; - const highlightedLineDepths = - shouldShowThreadBranchGuides && - isInsideHighlightedBranch && - highlightedBranch - ? [highlightedBranch.depth] - : undefined; - return ( -
- {showUnreadDivider ? : null} - + visibleThreadHeadSummary ? ( +
+ +
+ ) : ( +
+ {threadReplyRenderItems.map((item) => { + const { + collapseDepthGuideActions, + connectsToVisibleChild, + continuationDepths, + entry, + index, + isContinuation, + } = item; + const showUnreadDivider = + index > 0 && entry.message.id === firstUnreadReplyId; + const highlight = selectThreadRowHighlight({ + branch: highlightedBranch, + index, + messageId: entry.message.id, + messageDepth: entry.message.depth, + showGuides: shouldShowThreadBranchGuides, + }); + return ( +
- {entry.summary ? ( - + {showUnreadDivider ? : null} + - ) : null} -
- ); - })} -
- ) - ) : repliesRenderState === "empty" && !isHuddleTranscript ? ( - // Only show the empty state when the thread is GENUINELY empty. - // Keying off `deferredThreadReplies` would flash "No replies" for a - // frame while a non-empty list streams in on the deferred commit. -
-

- No replies in this branch yet -

-

- Reply in the thread to continue this branch. -

-
- ) : // "pending": deferred list is empty but the live list has content — - // rows are streaming in on the deferred commit. Paint nothing rather - // than flashing the empty state. - null} + {entry.summary ? ( + + ) : null} +
+ ); + })} +
+ ) + } + />
diff --git a/desktop/src/features/messages/ui/MessageThreadReplyState.test.mjs b/desktop/src/features/messages/ui/MessageThreadReplyState.test.mjs new file mode 100644 index 00000000000..f70143f87b1 --- /dev/null +++ b/desktop/src/features/messages/ui/MessageThreadReplyState.test.mjs @@ -0,0 +1,162 @@ +/** + * Mount regressions for the thread reply region, wired through the + * panel-owned ThreadReplyRegion dispatcher. + * + * Bug this pins: a terminal thread-replies fetch error used to fall through to + * the "No replies in this branch yet" empty card, silently presenting a broken + * load as an authoritative empty branch with no recovery. The fix maps a + * terminal error to the retry card and NEVER the empty card, and routes the + * Retry button back to the query's refetch. + * + * Why this component, and why raw inputs: ThreadReplyRegion now owns BOTH the + * surface selection (selectThreadRepliesSurface, also unit-tested against its + * 8-case matrix in timelineSnapshot.test.mjs) AND the surface→content dispatch. + * MessageThreadPanel passes only its raw query/render state — the pending/error + * flags and the deferred vs. live reply counts — so there is no precomputed + * `surface` prop at the panel boundary to statically mis-set. This file mounts + * ThreadReplyRegion and drives the real raw-state→surface→content mapping, so a + * false-empty regression cannot land at either the selection or the dispatch + * with these tests green. Mounting the full panel is infeasible in node:test + * (its Tiptap composer / React Query stack is unavailable, see + * MessageComposerAutoSend.test.mjs); the render callbacks keep that heavy + * construction in the panel and out of this cheap mount. + * + * CI surface: pnpm test (node:test with @testing-library/react over JSDOM). + */ + +import assert from "node:assert/strict"; +import { after, afterEach, before, test } from "node:test"; + +import { JSDOM } from "jsdom"; + +const dom = new JSDOM("", { + url: "http://localhost", +}); + +before(() => { + Object.assign(globalThis, { + document: dom.window.document, + HTMLElement: dom.window.HTMLElement, + IS_REACT_ACT_ENVIRONMENT: true, + window: dom.window, + }); + dom.window.matchMedia = () => ({ + matches: false, + addEventListener() {}, + removeEventListener() {}, + }); +}); + +afterEach(async () => { + const { cleanup } = await import("@testing-library/react"); + cleanup(); +}); + +after(() => dom.window.close()); + +// Sentinels for the two heavy branches the panel owns. If ThreadReplyRegion +// ever routes error/empty/pending through a render callback, these appear where +// a card is expected and the assertions catch it. +const SKELETON_MARK = "SKELETON_BRANCH_MARKER"; +const LIST_MARK = "LIST_BRANCH_MARKER"; + +async function renderRegion(props) { + const { createElement } = await import("react"); + const { render } = await import("@testing-library/react"); + const { ThreadReplyRegion } = await import("./MessageThreadReplyState.tsx"); + return render( + createElement(ThreadReplyRegion, { + isPending: false, + isError: false, + deferredCount: 0, + liveCount: 0, + renderSkeleton: () => createElement("div", null, SKELETON_MARK), + renderList: () => createElement("div", null, LIST_MARK), + ...props, + }), + ); +} + +test("terminal error renders the retry card, never the empty card", async () => { + const { screen } = await import("@testing-library/react"); + // Raw terminal-failure state: not pending, load errored, nothing to show. + await renderRegion({ isError: true, onRetry: () => {} }); + + const card = screen.getByTestId("message-thread-replies-error"); + assert.ok(card, "a terminal error must render the error card"); + // The card appears asynchronously (after the query/retry lifecycle), so it + // must be an alert live region or a screen-reader user never hears it. + assert.equal( + card.getAttribute("role"), + "alert", + "the async error card must be an alert live region for assistive tech", + ); + assert.equal( + document.body.textContent.includes("No replies in this branch yet"), + false, + "a terminal error must NEVER render the empty state", + ); +}); + +test("Retry button invokes the supplied refetch callback", async () => { + const { fireEvent, screen } = await import("@testing-library/react"); + let retryCount = 0; + await renderRegion({ + isError: true, + onRetry: () => { + retryCount += 1; + }, + }); + + fireEvent.click(screen.getByTestId("message-thread-replies-retry")); + + assert.equal(retryCount, 1, "clicking Retry must call the refetch callback"); +}); + +test("genuine empty surface renders the empty card, not the error card", async () => { + const { screen } = await import("@testing-library/react"); + // Load succeeded (no error), branch is genuinely empty. + await renderRegion({}); + + assert.ok( + document.body.textContent.includes("No replies in this branch yet"), + "a genuine empty branch must render the empty card", + ); + assert.equal( + screen.queryByTestId("message-thread-replies-error"), + null, + "a genuine empty branch must NOT render the error card", + ); +}); + +test("pending surface paints nothing", async () => { + // Deferred snapshot is empty but the live list has content: rows are + // streaming in on the deferred commit, so paint nothing yet. + const { container } = await renderRegion({ deferredCount: 0, liveCount: 1 }); + + assert.equal( + container.textContent, + "", + "the pending surface must render nothing while rows stream in", + ); +}); + +test("skeleton surface renders the panel's skeleton branch", async () => { + const { container } = await renderRegion({ isPending: true }); + + assert.equal( + container.textContent, + SKELETON_MARK, + "the skeleton surface must render the panel's skeleton branch", + ); +}); + +test("list surface renders the panel's list branch", async () => { + const { container } = await renderRegion({ deferredCount: 1, liveCount: 1 }); + + assert.equal( + container.textContent, + LIST_MARK, + "the list surface must render the panel's list branch", + ); +}); diff --git a/desktop/src/features/messages/ui/MessageThreadReplyState.tsx b/desktop/src/features/messages/ui/MessageThreadReplyState.tsx new file mode 100644 index 00000000000..a13d46286ef --- /dev/null +++ b/desktop/src/features/messages/ui/MessageThreadReplyState.tsx @@ -0,0 +1,127 @@ +import type { ReactNode } from "react"; + +import { + selectDeferredListRenderState, + selectThreadRepliesSurface, +} from "@/features/messages/lib/timelineSnapshot"; +import { Button } from "@/shared/ui/button"; + +/** + * Terminal empty/error states for the thread reply region. + * + * These are the two non-list, non-loading outcomes of a thread-reply load. They + * live here (rather than inline in `MessageThreadPanel`) so the load-bearing + * distinction between them stays legible: a genuinely empty branch and a failed + * fetch look similar but must never be confused — see `selectThreadRepliesSurface`. + */ + +/** + * A terminal load failure. This must NEVER be painted as the empty state — that + * silently presents a broken fetch as an authoritative "no replies" and offers + * no recovery. Any cached replies still render via the panel's "list" branch, so + * this only surfaces when the failed load left nothing to show. + * + * `role="alert"` (implicit `aria-live="assertive"`, `aria-atomic="true"`) makes + * the asynchronous failure audible to assistive tech: the card appears only after + * the query/retry lifecycle reaches a terminal error, so without a live region a + * screen-reader user parked in the composer never learns the load failed or that + * Retry became available. + */ +export function ThreadRepliesErrorCard({ onRetry }: { onRetry?: () => void }) { + return ( +
+

+ Couldn't load replies +

+

+ The thread history didn't load. Check your connection and try + again. +

+ {onRetry ? ( + + ) : null} +
+ ); +} + +/** + * A branch that genuinely has no replies (the load succeeded and returned none). + * Only ever painted off the committed render state, never the raw deferred list, + * so it can't flash while a non-empty list streams in on the deferred commit. + */ +export function ThreadRepliesEmptyCard() { + return ( +
+

+ No replies in this branch yet +

+

+ Reply in the thread to continue this branch. +

+
+ ); +} + +/** + * The single paint decision for the thread reply region. This unit owns BOTH + * the surface selection (`selectThreadRepliesSurface`, keyed off the same raw + * query/render state the panel already holds) AND the surface→content dispatch. + * Fusing them here removes the last falsifiable seam: the panel passes only its + * raw state — pending/error flags, the deferred vs. live reply counts, and the + * huddle-transcript flag — so there is no precomputed `surface` prop at the + * panel boundary to statically mis-set (e.g. a stray `surface="empty"` that would + * silently restore the false-empty bug on every fetch failure). The load-bearing + * invariant holds by construction: a terminal fetch "error" renders the retry + * card and NEVER the "empty" "No replies" state, while "pending" paints nothing + * (rows stream in on the deferred commit). + * + * The two heavy branches take render callbacks so the panel keeps ownership of + * its skeleton and list construction (Tiptap/React-Query bound, not mountable in + * node:test) without dragging them into this component. The mount test drives + * the real raw-state→surface→content mapping through this exported unit, so both + * the selection and the dispatch are covered under a cheap mount. + */ +export function ThreadReplyRegion({ + isPending, + isError, + deferredCount, + liveCount, + isHuddleTranscript = false, + onRetry, + renderSkeleton, + renderList, +}: { + isPending: boolean; + isError: boolean; + deferredCount: number; + liveCount: number; + isHuddleTranscript?: boolean; + onRetry?: () => void; + renderSkeleton: () => ReactNode; + renderList: () => ReactNode; +}) { + const surface = selectThreadRepliesSurface({ + isPending, + isError, + renderState: selectDeferredListRenderState(deferredCount, liveCount), + isHuddleTranscript, + }); + if (surface === "skeleton") return <>{renderSkeleton()}; + if (surface === "list") return <>{renderList()}; + if (surface === "error") return ; + if (surface === "empty") return ; + return null; +} diff --git a/desktop/src/features/messages/useThreadReplies.ts b/desktop/src/features/messages/useThreadReplies.ts index 25a6b68986b..4d602348f95 100644 --- a/desktop/src/features/messages/useThreadReplies.ts +++ b/desktop/src/features/messages/useThreadReplies.ts @@ -67,6 +67,39 @@ export function useThreadReplies( }); } +/** + * Aggregate a set of per-root thread-reply query results into one view for a + * multi-root consumer. Pure over the results array so the load-bearing + * error-surfacing contract is unit-testable without a live QueryClient. + * + * `isError`/`error` expose aggregate terminal failure so a consumer never + * silently drops a failed reply subtree — the same false-empty class the + * single-root panel guards against. `error` carries the first failed subtree's + * error; `refetch` re-runs only the failed queries so a partial success is not + * needlessly re-fetched. + */ +export function combineThreadRepliesResults( + results: readonly { + data?: RelayEvent[]; + isPending: boolean; + isError: boolean; + error: unknown; + refetch: () => unknown; + }[], +) { + return { + events: sortMessages(results.flatMap((result) => result.data ?? [])), + isPending: results.some((result) => result.isPending), + isError: results.some((result) => result.isError), + error: results.find((result) => result.isError)?.error ?? null, + refetch: () => { + for (const result of results) { + if (result.isError) void result.refetch(); + } + }, + }; +} + /** * Load every summarized reply subtree for a channel-style Huddle transcript. * Ordinary channels keep replies in their thread panels; Huddles flatten those @@ -87,9 +120,6 @@ export function useThreadRepliesForRoots( staleTime: 0, gcTime: 60 * 60 * 1_000, })), - combine: (results) => ({ - events: sortMessages(results.flatMap((result) => result.data ?? [])), - isPending: results.some((result) => result.isPending), - }), + combine: combineThreadRepliesResults, }); } diff --git a/desktop/src/features/projects/ui/ProjectConversationPanel.tsx b/desktop/src/features/projects/ui/ProjectConversationPanel.tsx index 59f3264b025..11344ea574d 100644 --- a/desktop/src/features/projects/ui/ProjectConversationPanel.tsx +++ b/desktop/src/features/projects/ui/ProjectConversationPanel.tsx @@ -279,7 +279,11 @@ export function ProjectConversationPanel({ scrollTargetId={scrollTargetId} threadHead={panelData.threadHead} threadReplies={panelData.visibleReplies} - threadRepliesPending={false} + threadRepliesPending={threadRepliesQuery.isPending} + threadRepliesError={threadRepliesQuery.isError} + onRetryThreadReplies={() => { + void threadRepliesQuery.refetch(); + }} threadTypingPubkeys={[]} widthPx={widthPx} />, diff --git a/desktop/src/features/projects/ui/ProjectsAgentPromptPage.tsx b/desktop/src/features/projects/ui/ProjectsAgentPromptPage.tsx index 9a465d3da0e..f667fcd2a21 100644 --- a/desktop/src/features/projects/ui/ProjectsAgentPromptPage.tsx +++ b/desktop/src/features/projects/ui/ProjectsAgentPromptPage.tsx @@ -37,6 +37,7 @@ import { } from "@/features/messages/lib/useRichTextEditor"; import { FormattingToolbar } from "@/features/messages/ui/FormattingToolbar"; import { MessageThreadTranscript } from "@/features/messages/ui/MessageThreadTranscript"; +import { ThreadRepliesErrorCard } from "@/features/messages/ui/MessageThreadReplyState"; import type { TimelineMessage } from "@/features/messages/types"; import { useThreadRepliesForRoots } from "@/features/messages/useThreadReplies"; import { useProfileQuery, useUsersBatchQuery } from "@/features/profile/hooks"; @@ -363,6 +364,9 @@ export function ConversationThread({ profiles={profiles} renderAfterMessage={renderSubmittedContext} /> + {threadReplies.isError ? ( + + ) : null} {agentWorking.working ? (
diff --git a/desktop/tests/e2e/huddle-thread-load-failure.spec.ts b/desktop/tests/e2e/huddle-thread-load-failure.spec.ts new file mode 100644 index 00000000000..8d3777201a9 --- /dev/null +++ b/desktop/tests/e2e/huddle-thread-load-failure.spec.ts @@ -0,0 +1,182 @@ +import { expect, test, type Page } from "@playwright/test"; + +import { installMockBridge, TEST_IDENTITIES } from "../helpers/bridge"; + +/** + * Consumer-level guard for the false-empty thread-load bug on the Huddle + * transcript (PR #6447, Carl r3). The transcript flattens summarized reply + * subtrees into the chat timeline via `useThreadRepliesForRoots`, whose combine + * now reports an aggregate `isError`/`refetch`. `useHuddleChannelMessages` used + * to read only `.events` and drop that state, so one failed subtree left the + * partial transcript presenting as complete with no warning or recovery. + * + * The combine unit test proves the hook REPORTS failure; it cannot catch a + * consumer discarding it. This drives the REAL Huddle wiring + * (useHuddleChannelMessages -> ChannelScreen -> ChannelPane) through the mock + * bridge: two summarized roots, fail only ONE subtree's fetch at the IPC + * boundary, assert the surviving root's reply still renders AND the retry alert + * appears, then Retry recovers the failed subtree. Dropping the propagation + * turns this red. + */ + +const HUDDLE_CHANNEL_ID = "11111111-1111-4111-8111-111111111111"; +const HUDDLE_PARENT_ID = "9a1657ac-f7aa-5db0-b632-d8bbeb6dfb50"; + +const ROOT_A_CONTENT = "Huddle root A"; +const REPLY_A_CONTENT = "Huddle reply A survives"; +const ROOT_B_CONTENT = "Huddle root B"; +const REPLY_B_CONTENT = "Huddle reply B recovered"; + +async function waitForMockLiveSubscription(page: Page, channelName: string) { + await expect + .poll(() => + page.evaluate( + (name) => + window.__BUZZ_E2E_HAS_MOCK_LIVE_SUBSCRIPTION__?.({ + channelName: name, + }) ?? false, + channelName, + ), + ) + .toBe(true); +} + +// Fail get_thread_replies for exactly one root while the flag names it, letting +// the other subtree and the eventual retry succeed. Wrapping the real +// __TAURI_INTERNALS__.invoke exercises the whole per-root query path with no +// source seam to bypass — the aggregate error and the failed-only refetch are +// production behavior, not a test stub. +async function installPerRootThreadFailureSwitch(page: Page) { + await page.evaluate(() => { + const w = window as typeof window & { + __FAIL_THREAD_ROOT__?: string | null; + __TAURI_INTERNALS__: { + invoke: ( + command: string, + payload: unknown, + options: unknown, + ) => Promise; + }; + }; + w.__FAIL_THREAD_ROOT__ = null; + const original = w.__TAURI_INTERNALS__.invoke.bind(w.__TAURI_INTERNALS__); + w.__TAURI_INTERNALS__.invoke = async (command, payload, options) => { + if ( + command === "get_thread_replies" && + w.__FAIL_THREAD_ROOT__ != null && + (payload as { rootEventId?: string })?.rootEventId === + w.__FAIL_THREAD_ROOT__ + ) { + throw new Error("relay unreachable: request timed out"); + } + return original(command, payload, options); + }; + }); +} + +async function setFailingThreadRoot(page: Page, rootId: string | null) { + await page.evaluate((rootId) => { + ( + window as typeof window & { __FAIL_THREAD_ROOT__?: string | null } + ).__FAIL_THREAD_ROOT__ = rootId; + }, rootId); +} + +test.describe("huddle thread load failure", () => { + test("a failed reply subtree shows the retry alert beside surviving rows; Retry recovers", async ({ + page, + }) => { + await installMockBridge(page, { + windowLabel: `huddle-${HUDDLE_CHANNEL_ID}`, + huddle: { + parentChannelId: HUDDLE_PARENT_ID, + ephemeralChannelId: HUDDLE_CHANNEL_ID, + members: [ + { pubkey: TEST_IDENTITIES.tyler.pubkey, role: "member" }, + { pubkey: TEST_IDENTITIES.alice.pubkey, role: "bot" }, + ], + transcriptionEnabled: true, + }, + }); + await page.goto("/"); + + await expect(page.getByTestId("huddle-transcript-intro")).toBeVisible(); + await installPerRootThreadFailureSwitch(page); + await waitForMockLiveSubscription(page, "huddle"); + + // Seed two summarized roots. Each threaded reply emits a live thread + // summary (descendant_count > 0), so both roots enter the transcript's + // useThreadRepliesForRoots fan-out and each gets its own subtree fetch. Fail + // root B's fetch BEFORE seeding so its first fan-out fetch reaches the + // terminal error while root A resolves — the aggregate reports failure with + // A's reply already merged, exactly the partial-transcript case. + const rootB = "b".repeat(64); + await setFailingThreadRoot(page, rootB); + const seeded = await page.evaluate( + ({ + agentPubkey, + rootAContent, + replyAContent, + rootBContent, + replyBContent, + rootBId, + }) => { + const emit = window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__; + if (!emit) throw new Error("Mock message emitter is not installed."); + const rootA = emit({ channelName: "huddle", content: rootAContent }); + emit({ + channelName: "huddle", + content: replyAContent, + parentEventId: rootA.id, + pubkey: agentPubkey, + }); + emit({ channelName: "huddle", content: rootBContent, id: rootBId }); + emit({ + channelName: "huddle", + content: replyBContent, + parentEventId: rootBId, + pubkey: agentPubkey, + }); + return { rootA: rootA.id }; + }, + { + agentPubkey: TEST_IDENTITIES.alice.pubkey, + rootAContent: ROOT_A_CONTENT, + replyAContent: REPLY_A_CONTENT, + rootBContent: ROOT_B_CONTENT, + replyBContent: REPLY_B_CONTENT, + rootBId: rootB, + }, + ); + expect(seeded.rootA).toBeTruthy(); + + // The load-bearing assertion: root A's reply still renders (non-destructive + // — a failed subtree does not blank the surviving rows) AND root B's failed + // fan-out surfaces the retry alert. That alert is rendered ONLY by the + // Huddle transcript's `huddleThreadRepliesError` propagation; without it the + // partial transcript would present as complete. (Reply rows themselves flow + // through the live channel window, so their presence is not the signal — the + // alert is.) + await expect( + page.getByTestId("message-row").filter({ hasText: REPLY_A_CONTENT }), + ).toBeVisible({ timeout: 15_000 }); + await expect( + page.getByTestId("message-thread-replies-error"), + ).toContainText("Couldn't load replies", { timeout: 15_000 }); + + // Retry with fetches succeeding: the failed-only refetch recovers root B's + // subtree, the aggregate error clears, and the alert is dismissed while both + // replies stay visible. + await setFailingThreadRoot(page, null); + await page.getByTestId("message-thread-replies-retry").click(); + await expect(page.getByTestId("message-thread-replies-error")).toHaveCount( + 0, + ); + await expect( + page.getByTestId("message-row").filter({ hasText: REPLY_B_CONTENT }), + ).toBeVisible(); + await expect( + page.getByTestId("message-row").filter({ hasText: REPLY_A_CONTENT }), + ).toBeVisible(); + }); +}); diff --git a/desktop/tests/e2e/project-conversation-load-failure.spec.ts b/desktop/tests/e2e/project-conversation-load-failure.spec.ts new file mode 100644 index 00000000000..d76bb709ec3 --- /dev/null +++ b/desktop/tests/e2e/project-conversation-load-failure.spec.ts @@ -0,0 +1,192 @@ +import { expect, test, type Page } from "@playwright/test"; + +import { installMockBridge, TEST_IDENTITIES } from "../helpers/bridge"; + +/** + * End-to-end guard for the false-empty thread-load bug on the SECOND producer + * of the shared thread panel: the Projects "channel conversation" panel + * (`ProjectConversationPanel`). PR #6447 wired the query failure state through + * `ChannelScreen`, but the Projects surface calls the same `useThreadReplies` + * and used to hard-code `threadRepliesPending={false}` with no error/retry — so + * a terminal `/query` failure there painted "No replies in this branch yet" + * with no recovery, the exact defect the PR fixed one surface over. + * + * This drives the REAL Projects panel wiring through the mock bridge: open the + * project's Channels tab, open a conversation, fail every `get_thread_replies` + * fetch at the IPC boundary, and assert the error/Retry card renders (never the + * false-empty). Any regression at the Projects call site (e.g. dropping + * `threadRepliesError` again, or a static `isError={false}`) ships a visible + * false-empty and turns this case red. + */ + +const DEFAULT_MOCK_PUBKEY = "deadbeef".repeat(8); +const ROOT_CONTENT = `Projects conversation root ${DEFAULT_MOCK_PUBKEY} buzz`; +const REPLY_CONTENT = "Projects conversation reply body"; + +// The projects surface is a preview feature — opt in before the app mounts. +async function enableProjectsFeature(page: Page) { + await page.addInitScript(() => { + window.localStorage.setItem( + "buzz-feature-overrides-v1", + JSON.stringify({ projects: true }), + ); + }); +} + +// Fail every get_thread_replies fetch at the IPC boundary while the flag is on, +// then let the real mock handler answer once it is cleared. Wrapping +// __TAURI_INTERNALS__.invoke exercises the whole Projects thread-load path — +// the panel's useThreadReplies query, its retry, and the shared region — with +// no source seam to bypass. +async function installThreadFailureSwitch(page: Page) { + await page.evaluate(() => { + const w = window as typeof window & { + __FAIL_THREAD_REPLIES__?: boolean; + __TAURI_INTERNALS__: { + invoke: ( + command: string, + payload: unknown, + options: unknown, + ) => Promise; + }; + }; + w.__FAIL_THREAD_REPLIES__ = false; + const original = w.__TAURI_INTERNALS__.invoke.bind(w.__TAURI_INTERNALS__); + w.__TAURI_INTERNALS__.invoke = async (command, payload, options) => { + if (command === "get_thread_replies" && w.__FAIL_THREAD_REPLIES__) { + throw new Error("relay unreachable: request timed out"); + } + return original(command, payload, options); + }; + }); +} + +async function setThreadRepliesFailing(page: Page, failing: boolean) { + await page.evaluate((failing) => { + ( + window as typeof window & { __FAIL_THREAD_REPLIES__?: boolean } + ).__FAIL_THREAD_REPLIES__ = failing; + }, failing); +} + +test.describe("project conversation load failure", () => { + test("terminal fetch failure shows error card, never false-empty; Retry recovers", async ({ + page, + }) => { + await enableProjectsFeature(page); + await installMockBridge(page); + await page.goto("/", { waitUntil: "domcontentloaded" }); + + // Seed the conversation BEFORE opening general — i.e. before its live + // subscription exists — so the reply lands in the mock store (searchable, + // and returnable by a successful get_thread_replies) but is never live + // pushed into the thread-replies cache. If general were open first, the + // live handler would seed that cache and the panel would render the list + // branch, masking the error card. The root carries the repository discovery + // token so it surfaces as the channel's latest discussion hit (what the + // Channels-tab row opens); its reply omits the token so it never competes + // to be the opened hit. + // + // Wait for the emitter first: emitting before the app boots is a silent + // no-op (the helper is undefined), which would leave the store empty. + await expect + .poll(() => + page.evaluate( + () => + typeof ( + window as typeof window & { + __BUZZ_E2E_EMIT_MOCK_MESSAGE__?: unknown; + } + ).__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function", + ), + ) + .toBe(true); + const rootId = await page.evaluate( + ({ author, rootContent, replyContent }) => { + const now = Math.floor(Date.now() / 1000); + const emit = ( + window as typeof window & { + __BUZZ_E2E_EMIT_MOCK_MESSAGE__: (input: { + channelName: string; + content: string; + parentEventId?: string; + pubkey?: string; + createdAt?: number; + }) => { id: string }; + } + ).__BUZZ_E2E_EMIT_MOCK_MESSAGE__; + const root = emit({ + channelName: "general", + content: rootContent, + pubkey: author, + createdAt: now, + }); + emit({ + channelName: "general", + content: replyContent, + parentEventId: root.id, + pubkey: author, + createdAt: now + 1, + }); + return root.id; + }, + { + author: TEST_IDENTITIES.alice.pubkey, + rootContent: ROOT_CONTENT, + replyContent: REPLY_CONTENT, + }, + ); + expect(rootId).toBeTruthy(); + + // Navigate to the project's Channels tab and open the general conversation. + await page.getByTestId("open-projects-view").click(); + await page.getByTestId("projects-section-projects").click(); + const projectEntry = page + .locator( + '[data-testid="project-card-buzz"], [data-testid="project-row-buzz"]', + ) + .first(); + await expect(projectEntry).toBeVisible({ timeout: 10_000 }); + await projectEntry.click(); + await page.getByRole("tab", { name: "Channels", exact: true }).click(); + const channelRow = page + .getByTestId("project-channel-row") + .filter({ hasText: "#general" }) + .first(); + await expect(channelRow).toBeVisible({ timeout: 10_000 }); + + // Fail every thread-replies fetch, THEN open the conversation: the panel's + // query and its retry both fail, driving the terminal error state with an + // empty reply cache and nothing to fall back to. + await installThreadFailureSwitch(page); + await setThreadRepliesFailing(page, true); + await channelRow.click(); + + const panel = page.getByTestId("project-conversation-panel"); + await expect(panel).toBeVisible(); + + // The load-bearing assertion: a terminal failure paints the error/Retry + // card and NEVER the false-empty "No replies in this branch yet" state. + await expect( + page.getByTestId("message-thread-replies-error"), + ).toContainText("Couldn't load replies", { timeout: 15_000 }); + await expect(page.getByTestId("message-thread-replies-retry")).toHaveText( + "Retry", + ); + await expect(page.getByText("No replies in this branch yet")).toHaveCount( + 0, + ); + + // Retry with fetches succeeding: the reply loads and renders — the error + // card is gone and no false-empty appears. + await setThreadRepliesFailing(page, false); + await page.getByTestId("message-thread-replies-retry").click(); + await expect(page.getByTestId("message-thread-replies-error")).toHaveCount( + 0, + ); + await expect(page.getByText(REPLY_CONTENT)).toBeVisible(); + await expect(page.getByText("No replies in this branch yet")).toHaveCount( + 0, + ); + }); +}); diff --git a/desktop/tests/e2e/thread-load-failure.spec.ts b/desktop/tests/e2e/thread-load-failure.spec.ts new file mode 100644 index 00000000000..291600e736b --- /dev/null +++ b/desktop/tests/e2e/thread-load-failure.spec.ts @@ -0,0 +1,161 @@ +import { expect, test, type Page } from "@playwright/test"; + +import { TEST_IDENTITIES, installMockBridge } from "../helpers/bridge"; + +/** + * End-to-end guard for the false-empty thread-load bug (PR #6447). A terminal + * thread-replies fetch failure must paint the error/Retry card and NEVER the + * "No replies in this branch yet" empty card — the two look similar but a failed + * load presented as an authoritative empty is the user-visible defect. + * + * Unit tests cover `ThreadReplyRegion` in isolation, but they cannot mount the + * full panel (Tiptap/React-Query), so they never observe the production + * panel→region handoff. This spec drives the REAL panel wiring through the mock + * bridge, so any regression at that seam (e.g. a static `isError={false}` at the + * call site) ships a visible false-empty and turns this case red. + */ + +// Fail every get_thread_replies fetch at the IPC boundary while the flag is on, +// then let the real mock handler answer once it is cleared. Wrapping +// __TAURI_INTERNALS__.invoke (installed by the mock bridge) exercises the whole +// thread-load path — query hook, retry, panel, region — exactly as production +// does, with no source seam to bypass. A boolean gate (rather than a failure +// countdown) is deterministic: stray thread-reply prefetches during channel +// open can't drain it, so the panel's own load reliably reaches the terminal +// error state, and clearing the flag makes the Retry fetch reliably succeed. +async function installThreadFailureSwitch(page: Page) { + await page.evaluate(() => { + const w = window as typeof window & { + __FAIL_THREAD_REPLIES__?: boolean; + __TAURI_INTERNALS__: { + invoke: ( + command: string, + payload: unknown, + options: unknown, + ) => Promise; + }; + }; + w.__FAIL_THREAD_REPLIES__ = false; + const original = w.__TAURI_INTERNALS__.invoke.bind(w.__TAURI_INTERNALS__); + w.__TAURI_INTERNALS__.invoke = async (command, payload, options) => { + if (command === "get_thread_replies" && w.__FAIL_THREAD_REPLIES__) { + throw new Error("relay unreachable: request timed out"); + } + return original(command, payload, options); + }; + }); +} + +async function setThreadRepliesFailing(page: Page, failing: boolean) { + await page.evaluate((failing) => { + ( + window as typeof window & { __FAIL_THREAD_REPLIES__?: boolean } + ).__FAIL_THREAD_REPLIES__ = failing; + }, failing); +} + +// Open the welcome thread deterministically: prefer its summary row, but fall +// back to hovering the root message and clicking Reply. The summary row depends +// on the channel-window query having materialized the seeded reply, which can +// lag; the root Reply affordance opens the same thread panel without that race, +// so the panel is reliably open before the error-card assertions run. +async function openWelcomeThread(page: Page) { + const summary = page.locator( + '[data-testid="message-thread-summary"][data-thread-head-id="mock-general-welcome"]', + ); + if (await summary.count()) { + await summary.first().click(); + } else { + const root = page.locator( + '[data-testid="message-row"][data-message-id="mock-general-welcome"]', + ); + await root.hover(); + await root.getByRole("button", { name: "Reply" }).click(); + } + await expect(page.getByTestId("message-thread-panel")).toBeVisible(); +} + +test.describe("thread load failure", () => { + test("terminal fetch failure shows error card, never false-empty; Retry recovers", async ({ + page, + }) => { + await installMockBridge(page); + await page.goto("/"); + + // Seed a reply into the mock store BEFORE opening general, i.e. before its + // live subscription exists. The reply lands in the channel window (so the + // "1 reply" thread summary renders) but is never live-pushed into the + // thread-replies cache — so the thread opens with an empty reply cache and + // the failed get_thread_replies has nothing to fall back to. If it were + // emitted while general was open, the live handler would seed the thread + // cache and the panel would render the list branch, masking the error card. + // + // Wait for the emitter to be installed first: emitting before the app has + // booted is a silent no-op (the helper is undefined), which would leave the + // store empty and the recovery assertion with nothing to render. + await expect + .poll(() => + page.evaluate( + () => + typeof ( + window as typeof window & { + __BUZZ_E2E_EMIT_MOCK_MESSAGE__?: unknown; + } + ).__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function", + ), + ) + .toBe(true); + await page.evaluate((pubkey) => { + ( + window as typeof window & { + __BUZZ_E2E_EMIT_MOCK_MESSAGE__?: (input: { + channelName: string; + content: string; + parentEventId?: string; + pubkey?: string; + createdAt?: number; + }) => unknown; + } + ).__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: "First reply to welcome", + parentEventId: "mock-general-welcome", + pubkey, + createdAt: Math.floor(Date.now() / 1000) - 10, + }); + }, TEST_IDENTITIES.alice.pubkey); + + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + await installThreadFailureSwitch(page); + + // Fail every thread-replies fetch, then open the thread: the panel query and + // its retry both fail, driving the terminal error state. + await setThreadRepliesFailing(page, true); + await openWelcomeThread(page); + + // The load-bearing assertion: a terminal failure paints the error/Retry + // card and NEVER the false-empty "No replies in this branch yet" state. + await expect( + page.getByTestId("message-thread-replies-error"), + ).toContainText("Couldn't load replies", { timeout: 15_000 }); + await expect(page.getByTestId("message-thread-replies-retry")).toHaveText( + "Retry", + ); + await expect(page.getByText("No replies in this branch yet")).toHaveCount( + 0, + ); + + // Retry with fetches succeeding again: the reply loads and renders — the + // error card is gone and no false-empty appears. + await setThreadRepliesFailing(page, false); + await page.getByTestId("message-thread-replies-retry").click(); + await expect(page.getByTestId("message-thread-replies-error")).toHaveCount( + 0, + ); + await expect(page.getByText("First reply to welcome")).toBeVisible(); + await expect(page.getByText("No replies in this branch yet")).toHaveCount( + 0, + ); + }); +});