From f6e6617a9dcc2308d5039f8afaab974b49fb9577 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Mon, 24 Aug 2026 16:13:03 -0400 Subject: [PATCH 1/9] fix(desktop): bound thread /query and surface load errors, not false-empty (#6447) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Long threads in the desktop app sometimes never load (stuck on a skeleton until you close and reopen the panel), and a failed load silently renders as "No replies in this branch yet" — presenting a broken fetch as an authoritative empty thread with no way to recover. This fixes both, the two IMPORTANT findings from the thread-load investigation. ## Defect 1 — unbounded `/query` request The shared `reqwest::Client` in `relay.rs` sets no timeout, and neither `/query` request builder set a per-request `.timeout(...)`. A stalled or half-open connection (headers or body never arrive) leaves the request pending forever, so a thread-history load hangs on the skeleton indefinitely. Fix: a 30s per-request deadline on both `/query` builders, funnelled through one `send_query_request` helper so the timeout can never be applied to one builder and dropped from the other. Scoped per-request rather than client-level because the same client also serves STT/TTS model downloads, builderlab auth, and the media proxy — a client-level timeout would cut those off. The deadline sits above the 25s WS `HISTORY_TIMEOUT_MS` so a slow-but-live relay isn't cut off before the WebSocket path would be. A timeout surfaces through `classify_request_error` as the stable `"relay unreachable: request timed out"` string. `send()` resolves as soon as response headers arrive, so a relay that returns headers and then stalls the body trips the deadline during body consumption, not at `send()` — and that consumption happens on two paths: `parse_json_response` for 2xx, and `relay_error_message` for a non-success status (500/429/…). Both paths route their body-consumption error through one shared `classify_body_timeout` helper so they can't drift: a stalled body surfaces the stable `"relay unreachable: request timed out"` string on either path rather than the malformed-response bucket (2xx) or a bare `"relay returned 500"` status label (non-2xx). A genuinely non-stalled error still keeps its status classification. ## Defect 2 — terminal error painted as empty `ChannelScreen` consumed only `isPending`/`data` from the thread-replies query. Once React Query exhausted its one retry, `isPending` was false and the zero-length data fell through `selectDeferredListRenderState` to the `"empty"` state — indistinguishable from a genuinely empty branch, with no retry affordance. Fix: plumb `isError` + `refetch` through `ChannelScreen` → `ChannelPane` → `MessageThreadPanel`. A pure `selectThreadRepliesSurface` helper decides the paint in strict precedence — the load-bearing invariant is that a terminal error **never** resolves to `"empty"`, and cached replies stay visible non-destructively under a later error (the error card only surfaces when there is nothing to show). The panel renders an explicit "Couldn't load replies" + Retry card (testids `message-thread-replies-error` / `message-thread-replies-retry`). `ProjectConversationPanel` is a second producer of the same shared panel and used to hard-code `threadRepliesPending={false}` with no error/retry, so a failed load in a Projects conversation still painted the false-empty. It now propagates the same `isPending`/`isError`/`refetch` from its `useThreadReplies` query. The multi-root `useThreadRepliesForRoots` hook (the Huddle transcript and Projects-agent conversation surfaces) had the same gap in its `useQueries` `combine`: it returned only `{ events, isPending }`, so a failed reply subtree contributed zero rows and vanished. The combine is now a pure, unit-testable `combineThreadRepliesResults` that exposes aggregate `isError`/`error` plus a `refetch` that re-runs only the failed subtrees. Both multi-root consumers render the shared "Couldn't load replies" + Retry card when a subtree fails: the Projects-agent conversation after its transcript, and the Huddle transcript as a non-destructive banner above the timeline. `useHuddleChannelMessages` used to read only `.events` and discard the aggregate state, so one summarized root failing left the flattened transcript presenting as complete; it now propagates `threadRepliesError`/`onRetryThreadReplies` through `ChannelScreen` into `ChannelPane`, where successful rows stay visible and `onRetry` re-runs only the failed subtrees. The error card carries `role="alert"` so its asynchronous appearance is announced to assistive tech — without a live region a screen-reader user parked in the composer never learns the load failed or that Retry became available. ## Tests - `stalled_query_request_times_out_with_classified_error` — a loopback server that never responds; asserts the stable classified timeout string. - `stalled_response_body_times_out_with_classified_error` — a loopback server that writes valid 2xx JSON headers then stalls the body past the deadline; asserts the classified timeout string, not the malformed bucket. - `stalled_error_response_body_times_out_with_classified_error` — a loopback that writes `500` headers promising a body it never sends; asserts the classified timeout string rather than the `500` status label. - `non_stalled_error_response_yields_status_message` — a promptly-served `500` still surfaces `"relay returned 500 Internal Server Error"`, pinning that timeout preservation is scoped to actual timeouts. - `selectThreadRepliesSurface` — pending→skeleton, terminal error→error (never empty), page-2 failure never empty, cached rows stay visible under error, successful-empty→empty, retry-success→list, streaming→pending, and huddle-transcript collapse. - `MessageThreadReplyState` mounted test — terminal error renders the error card (asserting `role="alert"`), never the empty card. - `combineThreadRepliesResults` — multi-root aggregation/order, a failed subtree surfaces the aggregate error and never drops rows, aggregate pending, refetch re-runs only failed queries, all-success yields no error. - `thread-load-failure.spec.ts` (smoke E2E) — binds the real channel-thread panel wiring: forces a terminal `get_thread_replies` failure at the IPC boundary, asserts the error card renders (never the false-empty) and Retry recovers. - `project-conversation-load-failure.spec.ts` (smoke E2E) — the same guard for the Projects conversation producer, driven through the Projects Channels-tab row. - `huddle-thread-load-failure.spec.ts` (smoke E2E) — the consumer-level guard the combine unit test can't provide: drives the real Huddle wiring (`useHuddleChannelMessages` → `ChannelScreen` → `ChannelPane`) with two summarized roots, fails one subtree's fetch at the IPC boundary, asserts the surviving root's reply stays visible while the retry alert surfaces, then Retry recovers the failed subtree and clears the alert. ## Structure To stay under the desktop file-size ratchet, `relay.rs`'s inline test module moved to `relay/tests.rs`, and two pure pieces were extracted from the panel: the empty/error reply cards (`MessageThreadReplyState`) and the per-row branch-highlight derivation (`selectThreadRowHighlight`). --------- Signed-off-by: Will Pfleger Co-authored-by: Duncan --- desktop/playwright.config.ts | 3 + desktop/src-tauri/src/relay.rs | 501 +++----------- desktop/src-tauri/src/relay/tests.rs | 615 ++++++++++++++++++ .../src/features/channels/ui/ChannelPane.tsx | 12 + .../features/channels/ui/ChannelPane.types.ts | 10 + .../features/channels/ui/ChannelScreen.tsx | 13 +- .../channels/ui/useHuddleChannelMessages.ts | 10 +- .../combineThreadRepliesResults.test.mjs | 104 +++ .../src/features/messages/lib/threadPanel.ts | 56 ++ .../lib/threadReplyHighlight.test.mjs | 86 +++ .../messages/lib/threadReplyHighlight.ts | 45 ++ .../messages/lib/timelineSnapshot.test.mjs | 121 ++++ .../features/messages/lib/timelineSnapshot.ts | 68 ++ .../messages/ui/MessageThreadPanel.tsx | 394 +++++------ .../ui/MessageThreadReplyState.test.mjs | 162 +++++ .../messages/ui/MessageThreadReplyState.tsx | 127 ++++ .../src/features/messages/useThreadReplies.ts | 38 +- .../projects/ui/ProjectConversationPanel.tsx | 6 +- .../projects/ui/ProjectsAgentPromptPage.tsx | 4 + .../e2e/huddle-thread-load-failure.spec.ts | 182 ++++++ .../project-conversation-load-failure.spec.ts | 192 ++++++ desktop/tests/e2e/thread-load-failure.spec.ts | 161 +++++ 22 files changed, 2273 insertions(+), 637 deletions(-) create mode 100644 desktop/src-tauri/src/relay/tests.rs create mode 100644 desktop/src/features/messages/combineThreadRepliesResults.test.mjs create mode 100644 desktop/src/features/messages/lib/threadReplyHighlight.test.mjs create mode 100644 desktop/src/features/messages/lib/threadReplyHighlight.ts create mode 100644 desktop/src/features/messages/ui/MessageThreadReplyState.test.mjs create mode 100644 desktop/src/features/messages/ui/MessageThreadReplyState.tsx create mode 100644 desktop/tests/e2e/huddle-thread-load-failure.spec.ts create mode 100644 desktop/tests/e2e/project-conversation-load-failure.spec.ts create mode 100644 desktop/tests/e2e/thread-load-failure.spec.ts diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index 9099beff69..69250c4b53 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 bd3fefb125..f408ef2afd 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 0000000000..4ae3924932 --- /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 dbdb2ed234..4086a4e1c1 100644 --- a/desktop/src/features/channels/ui/ChannelPane.tsx +++ b/desktop/src/features/channels/ui/ChannelPane.tsx @@ -14,6 +14,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, @@ -97,6 +98,8 @@ export const ChannelPane = React.memo(function ChannelPane({ welcomeKickoffSettingUp = false, messages, threadSummaries, + huddleThreadRepliesError = false, + onRetryHuddleThreadReplies, firstUnreadMessageId = null, unreadCount = 0, canResetThreadPanelWidth, @@ -153,6 +156,8 @@ export const ChannelPane = React.memo(function ChannelPane({ threadHeadMessage, threadMessages, threadMessagesPending = false, + threadMessagesError = false, + onRetryThreadReplies, threadPanelWidthPx, threadScrollTargetId, threadTypingPubkeys, @@ -624,6 +629,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; @@ -163,6 +171,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 aac93d7f8d..240a9ad70c 100644 --- a/desktop/src/features/channels/ui/ChannelScreen.tsx +++ b/desktop/src/features/channels/ui/ChannelScreen.tsx @@ -254,7 +254,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, @@ -896,6 +901,8 @@ export function ChannelScreen({ isTimelineLoading={isTimelineLoading} messages={timelineMessages} threadSummaries={threadSummaries} + huddleThreadRepliesError={huddleThreadRepliesError} + onRetryHuddleThreadReplies={onRetryHuddleThreadReplies} onCancelEdit={handleCancelEdit} onCancelThreadReply={handleCancelThreadReply} onChannelManagementDeleted={handleChannelManagementDeleted} @@ -967,6 +974,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 2a90971dde..5a3a7c4041 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 0000000000..47beb1c7a8 --- /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 ebc1b35ae7..6dc1190910 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 0000000000..bad72b3339 --- /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 0000000000..0ce446daec --- /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 a0374fbe2b..5334d58875 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 3bfd934947..4e23fb2245 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 d2650c84ac..01fbcda7ff 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 0000000000..f70143f87b --- /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 0000000000..a13d46286e --- /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 25a6b68986..4d602348f9 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 59f3264b02..11344ea574 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 9a465d3da0..f667fcd2a2 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 0000000000..8d3777201a --- /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 0000000000..d76bb709ec --- /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 0000000000..291600e736 --- /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, + ); + }); +}); From f79d346a178408661fcad85122364ac2ad7e9cb2 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Mon, 24 Aug 2026 14:55:05 -0700 Subject: [PATCH 2/9] fix(composer): wrap Buzz chip labels without orphaning icons (#6581) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Category:** fix **User Impact:** Long Buzz link chips now wrap without overflowing in composers and sent messages; icon-bearing stubs use at most five graphemes when no earlier separator exists, so some sent chips attach the icon to a shorter prefix than before. Labels over 48 graphemes are visibly truncated in the composer while their full identity remains available to assistive technology and in the tooltip. **Problem:** Long repository, issue, pull request, and channel chip labels could orphan their icon or overflow narrow composers and sent messages. **Solution:** Keep the icon with a bounded, grapheme-safe leading fragment while allowing the remaining text to break anywhere; cap visible labels at 48 graphemes without changing the full tooltip or accessible identity.
File changes **desktop/src/features/messages/lib/composerMessageLinkNode.ts** Splits composer chip content into an icon-bearing leading fragment and a freely wrapping remainder while preserving the semantic label and link metadata. **desktop/src/features/messages/lib/composerMessageLinkNode.test.mjs** Updates renderer assertions for the fragment structure and verifies every supported Buzz link kind retains the intended visible label. **desktop/src/shared/styles/globals/composer.css** Keeps ordinary composer mention decorations inline while relying on the existing shared markdown chip wrapping rules for Buzz links. **desktop/src/shared/ui/mentionChip.ts** Centralizes grapheme-aware leading-fragment boundaries and label truncation so composer and sent chips share the same visible identity. **desktop/src/shared/ui/markdown/BuzzLinkChip.tsx** Uses the shared grapheme-aware boundary when rendering sent-message chip fragments. **desktop/tests/e2e/navigation.spec.ts** Covers increasing wrap depth across constrained widths, icon attachment, the sent-message wrap, accessible labeling, and tooltip positioning over both edge fragments.
## Reproduction steps 1. Open a desktop channel and paste a Buzz link with a long repository or channel name into the composer. 2. Narrow the composer until the chip spans two or more lines. 3. Confirm the label breaks mid-string while the icon remains attached to the first label fragment. 4. Send the message, hover both the first and last rendered fragments, and confirm the tooltip follows the hovered fragment. ## Screenshots **Before — the icon drops onto a separate line from its chip label** ![Composer chip with an orphaned icon before the fix](https://d24qwcpro867f5.cloudfront.net/repos/buzz/prs/6581/00-before-inline-chip-icon-wrap.png) **After — the icon stays attached while the remaining label wraps** The same long repository chip at three composer widths. Its label gains line breaks as space contracts, while the icon remains attached to the leading fragment. **420px — one line** ![Long repository chip on one line at 420px](https://d24qwcpro867f5.cloudfront.net/repos/buzz/prs/6581/01-wide-420px.png) **210px — two lines** ![Long repository chip on two lines at 210px](https://d24qwcpro867f5.cloudfront.net/repos/buzz/prs/6581/02-medium-210px.png) **150px — three lines** ![Long repository chip on three lines at 150px](https://d24qwcpro867f5.cloudfront.net/repos/buzz/prs/6581/03-narrow-150px.png) --------- Signed-off-by: Taylor Ho Co-authored-by: Carl --- .../lib/composerMessageLinkNode.test.mjs | 57 ++++- .../messages/lib/composerMessageLinkNode.ts | 45 +++- .../src/shared/styles/globals/composer.css | 10 +- .../src/shared/styles/globals/markdown.css | 5 + desktop/src/shared/ui/markdown.test.mjs | 6 +- .../src/shared/ui/markdown/BuzzLinkChip.tsx | 22 +- .../useInlineTooltipPosition.test.mjs | 147 +++++++++++++ .../ui/markdown/useInlineTooltipPosition.ts | 49 ++++- desktop/src/shared/ui/mentionChip.ts | 41 +++- desktop/tests/e2e/navigation.spec.ts | 200 +++++++++++++++++- 10 files changed, 545 insertions(+), 37 deletions(-) create mode 100644 desktop/src/shared/ui/markdown/useInlineTooltipPosition.test.mjs diff --git a/desktop/src/features/messages/lib/composerMessageLinkNode.test.mjs b/desktop/src/features/messages/lib/composerMessageLinkNode.test.mjs index 3695275762..57212aeafe 100644 --- a/desktop/src/features/messages/lib/composerMessageLinkNode.test.mjs +++ b/desktop/src/features/messages/lib/composerMessageLinkNode.test.mjs @@ -19,6 +19,7 @@ const CHANNEL_MESSAGE_ID = "a".repeat(64); const CHANNEL_MESSAGE_HREF = `buzz://channel/${CHANNEL_ID}/${CHANNEL_MESSAGE_ID}`; const OWNER = "a".repeat(64); const REPO_HREF = `buzz://repo?owner=${OWNER}&d=buzz-world`; +const PROJECT_HREF = `buzz://project?owner=${OWNER}&d=buzz-world`; const ISSUE_ID = "b".repeat(64); const ISSUE_HREF = `buzz://issue?id=${ISSUE_ID}&owner=${OWNER}&d=buzz-world`; const PR_ID = "c".repeat(64); @@ -188,6 +189,10 @@ test("markdown parsing stops message links before emphasis delimiters", () => { assert.deepEqual(token.meta, { channelName: "general", href: HREF }); }); +function renderedChipLabel(rendered) { + return `${rendered[2][2]}${rendered[3]}`; +} + test("composer node uses the sent-message chip presentation", () => { const node = { attrs: { channelName: "general", href: HREF }, @@ -209,7 +214,44 @@ test("composer node uses the sent-message chip presentation", () => { assert.equal(rendered[1]["data-buzz-link"], ""); // Channel label only — no event hash, so the chip does not change width when // the draft is sent and the rendered chip resolves its metadata. - assert.equal(rendered[2], "general"); + assert.match(rendered[1].class, /wrapping-inline-chip/); + assert.match(rendered[2][1].class, /inline-chip-leading-fragment/); + assert.equal(renderedChipLabel(rendered), "general"); +}); + +test("composer node truncates and preserves grapheme-safe leading fragments", () => { + const render = ComposerMessageLinkNode.configure({ + resolveChannelName: () => undefined, + }).config.renderHTML; + assert.ok(render); + + const longName = `relay-${"observability".repeat(5)}`; + const longRendered = render.call( + { options: { resolveChannelName: () => undefined } }, + { + node: { attrs: { channelName: longName, href: HREF } }, + HTMLAttributes: {}, + }, + ); + assert.equal(renderedChipLabel(longRendered), `${longName.slice(0, 47)}…`); + + for (const [label, expectedLeading] of [ + ["🇺🇸channel", "🇺🇸chan"], + ["e\u0301quipe", "e\u0301quip"], + ["relaytoolsobservabilityconsole-main", "relay"], + [" leading-space", ""], + ]) { + const rendered = render.call( + { options: { resolveChannelName: () => undefined } }, + { + node: { attrs: { channelName: label, href: HREF } }, + HTMLAttributes: {}, + }, + ); + assert.equal(rendered[2][2], expectedLeading); + assert.match(rendered[2][1].class, /inline-chip-with-icon/); + assert.equal(renderedChipLabel(rendered), label); + } }); test("composer node renders channel and entity chip presentations", () => { @@ -227,24 +269,29 @@ test("composer node renders channel and entity chip presentations", () => { const channel = render(CHANNEL_HREF); assert.equal(channel[1]["data-channel-deep-link"], ""); assert.match(channel[1].class, /inline-chip-icon-channel/); - assert.equal(channel[2], "general"); + assert.equal(renderedChipLabel(channel), "general"); const repo = render(REPO_HREF); assert.equal(repo[1]["data-buzz-link-kind"], "repo"); assert.match(repo[1].class, /inline-chip-icon-repo/); - assert.equal(repo[2], "buzz-world"); + assert.equal(renderedChipLabel(repo), "buzz-world"); + + const project = render(PROJECT_HREF); + assert.equal(project[1]["data-buzz-link-kind"], "project"); + assert.match(project[1].class, /inline-chip-icon-project/); + assert.equal(renderedChipLabel(project), "buzz-world"); const issue = render(ISSUE_HREF); assert.equal(issue[1]["data-buzz-link-kind"], "issue"); assert.match(issue[1].class, /inline-chip-icon-issue/); // Repository name only — the rendered chip never widens into the issue // title, so the composer must not widen into the event hash either. - assert.equal(issue[2], "buzz-world"); + assert.equal(renderedChipLabel(issue), "buzz-world"); const pullRequest = render(PR_HREF); assert.equal(pullRequest[1]["data-buzz-link-kind"], "pr"); assert.match(pullRequest[1].class, /inline-chip-icon-pr/); - assert.equal(pullRequest[2], "buzz-world"); + assert.equal(renderedChipLabel(pullRequest), "buzz-world"); }); test("markdown rendering stores identity in attributes, not visible id text", () => { diff --git a/desktop/src/features/messages/lib/composerMessageLinkNode.ts b/desktop/src/features/messages/lib/composerMessageLinkNode.ts index 9587c312ca..59b15c78ba 100644 --- a/desktop/src/features/messages/lib/composerMessageLinkNode.ts +++ b/desktop/src/features/messages/lib/composerMessageLinkNode.ts @@ -12,8 +12,11 @@ import { } from "@/shared/lib/entityLink"; import { inlineChipIconClasses, + inlineChipLeadingEnd, type InlineChipIconKind, MENTION_CHIP_BASE_CLASSES, + truncateInlineChipLabel, + WRAPPING_INLINE_CHIP_CLASSES, } from "@/shared/ui/mentionChip"; import { buildChannelLink, parseChannelLink } from "./channelLink"; import { getMessageLinkLabel } from "./messageLinkLabel"; @@ -285,6 +288,38 @@ function composerLinkPresentation( }; } +function wrappingComposerChipContent( + label: string, + icon: InlineChipIconKind, +): { leading: [string, Record, string]; remainder: string } { + const leadingEnd = inlineChipLeadingEnd(label); + if (!leadingEnd) { + return { + leading: [ + "span", + { + "aria-hidden": "true", + class: `inline-chip-leading-fragment ${inlineChipIconClasses(icon)}`, + }, + "", + ], + remainder: label, + }; + } + + return { + leading: [ + "span", + { + "aria-hidden": "true", + class: `inline-chip-leading-fragment ${inlineChipIconClasses(icon)}`, + }, + label.slice(0, leadingEnd), + ], + remainder: label.slice(leadingEnd), + }; +} + export const ComposerMessageLinkNode = Node.create({ name: COMPOSER_MESSAGE_LINK_NODE_NAME, @@ -328,11 +363,16 @@ export const ComposerMessageLinkNode = String(node.attrs.channelName ?? ""), this.options.resolveChannelName, ); + const visibleLabel = truncateInlineChipLabel(presentation.label); + const content = wrappingComposerChipContent( + visibleLabel, + presentation.icon, + ); return [ "span", mergeAttributes(HTMLAttributes, { "aria-label": presentation.ariaLabel, - class: `${MENTION_CHIP_BASE_CLASSES} ${inlineChipIconClasses(presentation.icon)} cursor-text`, + class: `${MENTION_CHIP_BASE_CLASSES} ${WRAPPING_INLINE_CHIP_CLASSES} ${inlineChipIconClasses(presentation.icon)} cursor-text`, "data-buzz-link": "", "data-channel-name": presentation.channelName, "data-composer-buzz-link": "", @@ -340,7 +380,8 @@ export const ComposerMessageLinkNode = ...presentation.dataAttributes, title: presentation.ariaLabel, }), - presentation.label, + content.leading, + content.remainder, ]; }, diff --git a/desktop/src/shared/styles/globals/composer.css b/desktop/src/shared/styles/globals/composer.css index fbbcdf026c..781ac8a196 100644 --- a/desktop/src/shared/styles/globals/composer.css +++ b/desktop/src/shared/styles/globals/composer.css @@ -256,11 +256,11 @@ Body copy is text-sm; inline code labels sit one step down (text-xs). All chip variants share one box height so mono labels stay balanced. - Rendered-message chips use inline-flex. The same decoration in the - composer pulls the caret into the chip and swallows the trailing - space after an @mention, so typing "hello" after @quinn becomes - "@quinnhello". Keep composer chips inline; leave ::before icons in - place. */ + Plain-text mention decorations stay inline because inline-flex pulls the + caret into their decorated range and swallows the trailing space after an + @mention. Composer Buzz links are atom nodes, but their labels may still + fragment between characters; only the icon and leading label fragment stay + together. */ .rich-text-composer .tiptap .mention-chip { display: inline; min-height: 0; diff --git a/desktop/src/shared/styles/globals/markdown.css b/desktop/src/shared/styles/globals/markdown.css index e443357ae8..9d0c382ac3 100644 --- a/desktop/src/shared/styles/globals/markdown.css +++ b/desktop/src/shared/styles/globals/markdown.css @@ -211,6 +211,11 @@ white-space: nowrap; } +.message-markdown + .inline-chip-leading-fragment.inline-chip-with-icon:empty::after { + content: "\200b"; +} + .message-markdown .inline-chip-leading-fragment.inline-chip-with-icon::before { display: block; left: 0; diff --git a/desktop/src/shared/ui/markdown.test.mjs b/desktop/src/shared/ui/markdown.test.mjs index 8136ff9f08..6cd15c69b4 100644 --- a/desktop/src/shared/ui/markdown.test.mjs +++ b/desktop/src/shared/ui/markdown.test.mjs @@ -1121,7 +1121,7 @@ test("bare Buzz permalinks render cohesive icon-prefixed chips", () => { assert.equal((html.match(/data-channel-deep-link=""/g) ?? []).length, 1); assert.match(html, /inline-chip-icon-channel/); assert.match(html, /wrapping-inline-chip/); - assert.match(html, /inline-chip-leading-fragment[^>]*>e]*>engin { ); assert.equal( - (html.match(/inline-chip-leading-fragment[^>]*>5<\/span>80ca78b/g) ?? []) + (html.match(/inline-chip-leading-fragment[^>]*>580ca<\/span>78b/g) ?? []) .length, 2, ); @@ -1332,7 +1332,7 @@ test("channel references replace the authored hash with the channel icon", () => assert.match(html, /inline-chip-icon-channel/); assert.match(html, /wrapping-inline-chip/); - assert.match(html, /inline-chip-leading-fragment[^>]*>e]*>engin]+>/g, ""), /engineering/); assert.doesNotMatch(html, />#engineering + ); } diff --git a/desktop/tests/e2e/mentions.spec.ts b/desktop/tests/e2e/mentions.spec.ts index ad1acf87c3..8f0300af16 100644 --- a/desktop/tests/e2e/mentions.spec.ts +++ b/desktop/tests/e2e/mentions.spec.ts @@ -410,19 +410,25 @@ test("duplicate owned agents preserve provenance and exact pubkey selection", as 0, ); await expect(relayRow).toContainText("agent"); - await expect( - relayRow.getByTestId("mention-agent-provenance"), - ).toHaveAttribute("aria-label", "From another Buzz setup"); - await expect( - relayRow.getByText("Other setup", { exact: true }), - ).toBeVisible(); + const relayProvenanceMarker = relayRow.getByTestId( + "mention-agent-provenance", + ); + await expect(relayProvenanceMarker).toHaveAttribute( + "aria-label", + "From another Buzz setup", + ); + await expect(relayProvenanceMarker).toHaveAttribute( + "title", + "From another Buzz setup", + ); + await expect(relayProvenanceMarker).toBeVisible(); + await expect(relayProvenanceMarker).toHaveText(""); + await expect(relayProvenanceMarker.locator("svg")).toBeVisible(); await expect(managedRow).not.toContainText("managed by you"); await expect(relayRow).not.toContainText("managed by you"); await page.setViewportSize({ width: 760, height: 640 }); - await expect( - relayRow.getByText("Other setup", { exact: true }), - ).toBeVisible(); + await expect(relayProvenanceMarker).toBeVisible(); const rowBox = await relayRow.boundingBox(); const dropdownBox = await dropdown.boundingBox(); expect(rowBox).not.toBeNull(); @@ -495,7 +501,8 @@ test("duplicate owned agents preserve provenance and exact pubkey selection", as "aria-label", "From another Buzz setup", ); - await expect(remoteSidebarMarker).toHaveText("Other setup"); + await expect(remoteSidebarMarker).toHaveText(""); + await expect(remoteSidebarMarker.locator("svg")).toBeVisible(); const remoteSidebarRow = page.getByTestId(`sidebar-member-${relayPubkey}`); const localSidebarMarker = page.getByTestId( `sidebar-member-agent-provenance-${managedPubkey}`, From c5166f2164035ca96787daee6528d5dc04c4a02e Mon Sep 17 00:00:00 2001 From: morgmart <98432065+morgmart@users.noreply.github.com> Date: Mon, 24 Aug 2026 16:28:51 -0700 Subject: [PATCH 6/9] feat(desktop): simplify the message action rail (#6529) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem Quick-reaction shortcuts make the message action rail visually noisy. Reaction access should remain first-class, but through one predictable Add reaction action rather than several learned emoji shortcuts. ## Change The rail is now: **Add reaction · Reply · Copy link · More** - Remove all quick-reaction shortcut buttons and their divider from the message action rail. - Preserve Add reaction as the first action, including its existing picker, tooltip, accessible name, keyboard behavior, reaction behavior, and feedback. - Keep the existing Link2 Copy link action, shared copy handler, More-menu entry, eligibility guards, and success feedback unchanged. - Keep reveal, positioning, responsiveness, styling, and remaining menu paths unchanged. The now-unused quick-reaction rendering component and imports were removed from `MessageActionBar`; shared reaction learning remains intact for other reaction surfaces. ## Tests Focused smoke coverage verifies: - zero `React with …` shortcut buttons; - exact ordered rail: `Open reactions → Reply → Copy link → More actions`; - the rail and More-menu paths emit the same canonical thread-aware `buzz://message` URL; - existing success feedback; - pending and huddle rows omit both copy-link surfaces; - the action bar stays within the open thread panel. The zero-shortcut contract was mutation-checked by restoring the prior production action bar: the focused test failed causally with expected 0 versus received 3 quick-reaction buttons. ## Validation At `4f062e0d60b0f0c16b6862cdea7137c287060947`: - `pnpm exec biome check src/features/messages/ui/MessageActionBar.tsx tests/e2e/message-copy-link.spec.ts` — passed. - `pnpm exec tsc --noEmit` — passed. - `pnpm test` — 5,432 passed, 0 failed. - `pnpm build:e2e` — passed; existing dynamic-import and chunk-size warnings only. - `pnpm exec playwright test tests/e2e/message-copy-link.spec.ts --project=smoke` — 2 passed. - Pre-push `file-size-check`, `desktop-check`, `desktop-typecheck`, and `desktop-test` — passed. Vogue’s design review: **SHIP** — reaction remains discoverable and accessible as the visible first action; the extra click is an intentional efficiency tradeoff for the simpler hierarchy. --------- Signed-off-by: Trace (Engineer) <9d485ff0c62915e08a801162c195c7ef096f6f6143925407255cea2656c4b8a4@buzz.block.builderlab.xyz> Signed-off-by: morgmart <98432065+morgmart@users.noreply.github.com> Signed-off-by: Rivet Co-authored-by: Trace (Engineer) <9d485ff0c62915e08a801162c195c7ef096f6f6143925407255cea2656c4b8a4@buzz.block.builderlab.xyz> Co-authored-by: Rivet --- desktop/playwright.config.ts | 1 + .../features/messages/ui/MessageActionBar.tsx | 141 +++++--------- .../e2e/channel-activity-popover.spec.ts | 9 +- desktop/tests/e2e/custom-emoji.spec.ts | 21 +- desktop/tests/e2e/inbox-reactions.spec.ts | 10 +- desktop/tests/e2e/message-copy-link.spec.ts | 182 ++++++++++++++++++ desktop/tests/e2e/reaction-order.spec.ts | 25 +-- 7 files changed, 268 insertions(+), 121 deletions(-) create mode 100644 desktop/tests/e2e/message-copy-link.spec.ts diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index 69250c4b53..be15c75587 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -38,6 +38,7 @@ export default defineConfig({ "**/invites-settings-screenshots.spec.ts", "**/messaging.spec.ts", "**/message-feedback-snapshots.spec.ts", + "**/message-copy-link.spec.ts", "**/custom-emoji.spec.ts", "**/profile-custom-emoji-status.spec.ts", "**/custom-emoji-ui.spec.ts", diff --git a/desktop/src/features/messages/ui/MessageActionBar.tsx b/desktop/src/features/messages/ui/MessageActionBar.tsx index 4fcf0f067a..9c490edb07 100644 --- a/desktop/src/features/messages/ui/MessageActionBar.tsx +++ b/desktop/src/features/messages/ui/MessageActionBar.tsx @@ -18,7 +18,6 @@ import { toast } from "sonner"; import { buildMessageLink } from "@/features/messages/lib/messageLink"; import { EmojiPicker } from "@/features/custom-emoji/ui/EmojiPicker"; -import { useCustomEmoji } from "@/features/custom-emoji/hooks"; import { getThreadReference } from "@/features/messages/lib/threading"; import { ReportMessageDialog } from "@/features/moderation/ui/ReportMessageDialog"; import { MessageModerationMenuItems } from "@/features/moderation/ui/MessageModerationMenuItems"; @@ -26,15 +25,9 @@ import type { TimelineMessage, TimelineReaction, } from "@/features/messages/types"; -import { - recordQuickReactionEmoji, - useQuickReactionEmojis, -} from "@/features/messages/ui/useQuickReactionEmojis"; -import { reactionEmojiUrl } from "@/shared/api/customEmoji"; +import { recordQuickReactionEmoji } from "@/features/messages/ui/useQuickReactionEmojis"; import { cn } from "@/shared/lib/cn"; import { copyTextToClipboard } from "@/shared/lib/clipboard"; -import { emojiDisplayName } from "@/shared/lib/emojiName"; -import { rewriteRelayUrl } from "@/shared/lib/mediaUrl"; import { KIND_HUDDLE_STARTED } from "@/shared/constants/kinds"; import { Button } from "@/shared/ui/button"; import { HashArrowIn } from "@/shared/ui/icons"; @@ -53,6 +46,32 @@ import { Tooltip, TooltipContent, TooltipTrigger } from "@/shared/ui/tooltip"; const ACTION_BUTTON_CLASS = "h-8 w-8 rounded-full p-0"; const ACTION_ICON_CLASS = "!h-4 !w-4"; +/** Copying a message link is offered from both the hover action bar and the + * More menu; both paths share this exact link-building + toast behavior. */ +function copyMessageLink(channelId: string, message: TimelineMessage) { + const { rootId } = getThreadReference(message.tags ?? []); + const link = buildMessageLink({ + channelId, + messageId: message.id, + threadRootId: rootId, + }); + copyTextToClipboard(link, "Link copied to clipboard"); +} + +/** Gate shared by every copy-link surface: pending sends have no delivered + * event to link to, huddle system rows aren't linkable, and callers without + * a channelId (e.g. inbox preview rows) can't build the link. */ +function canCopyMessageLink( + message: TimelineMessage, + channelId: string | null | undefined, +): channelId is string { + return ( + !message.pending && + message.kind !== KIND_HUDDLE_STARTED && + Boolean(channelId) + ); +} + function MoreActionsMenu({ channelId, message, @@ -242,17 +261,11 @@ function MoreActionsMenu({ ) : null} - {hasCopyActions && channelId ? ( + {canCopyMessageLink(message, channelId) ? ( { - const { rootId } = getThreadReference(message.tags ?? []); - const link = buildMessageLink({ - channelId, - messageId: message.id, - threadRootId: rootId, - }); - copyTextToClipboard(link, "Link copied to clipboard"); + copyMessageLink(channelId, message); }} > @@ -316,51 +329,6 @@ function MoreActionsMenu({ ); } -function QuickReactionButton({ - customEmojiUrl, - emoji, - onSelect, -}: { - customEmojiUrl?: string; - emoji: string; - onSelect: (emoji: string) => void; -}) { - const displayName = emojiDisplayName(emoji); - const mediaUrl = customEmojiUrl ? rewriteRelayUrl(customEmojiUrl) : null; - - return ( - - - - - {displayName} - - ); -} - -function isCustomEmojiShortcode(emoji: string) { - return emoji.startsWith(":") && emoji.endsWith(":"); -} - export const MessageActionBar = React.memo(function MessageActionBar({ channelId, message, @@ -404,20 +372,6 @@ export const MessageActionBar = React.memo(function MessageActionBar({ }) { const [isReactionPickerOpen, setIsReactionPickerOpen] = React.useState(false); const [isDropdownOpen, setIsDropdownOpen] = React.useState(false); - const customEmoji = useCustomEmoji(); - const quickReactionEmojis = useQuickReactionEmojis(4, customEmoji); - const quickReactionItems = React.useMemo( - () => - quickReactionEmojis - .map((emoji) => ({ - customEmojiUrl: reactionEmojiUrl(emoji, customEmoji), - emoji, - })) - .filter( - (item) => !isCustomEmojiShortcode(item.emoji) || item.customEmojiUrl, - ), - [customEmoji, quickReactionEmojis], - ); const hasReplyAction = Boolean(onReply); const hasReactionAction = Boolean(onReactionSelect); @@ -482,22 +436,6 @@ export const MessageActionBar = React.memo(function MessageActionBar({ >
- {hasReactionAction && quickReactionItems.length > 0 ? ( - <> -
- {quickReactionItems.map(({ customEmojiUrl, emoji }) => ( - - ))} -
-
- - ) : null} - {hasReactionAction ? ( ) : null} + {canCopyMessageLink(message, channelId) ? ( + + + + + Copy link + + ) : null} + {hasMoreMenuActions ? ( { await rootRow.hover(); const actionBar = page.getByTestId(`message-action-bar-${root.id}`); await expect(actionBar).toBeVisible(); - await actionBar - .getByRole("button", { name: /^React with / }) - .first() - .click(); + await actionBar.getByRole("button", { name: "Open reactions" }).click(); + const picker = page.locator("em-emoji-picker"); + await expect(picker).toBeVisible(); + await picker.locator("input[type='search']").fill("thumbs up"); + await picker.getByRole("button", { name: "👍" }).first().click(); await expect( rootRow.getByRole("button", { name: /^Toggle .* reaction$/ }), ).toBeVisible(); diff --git a/desktop/tests/e2e/custom-emoji.spec.ts b/desktop/tests/e2e/custom-emoji.spec.ts index 16ca3900ec..9a4bf7f07c 100644 --- a/desktop/tests/e2e/custom-emoji.spec.ts +++ b/desktop/tests/e2e/custom-emoji.spec.ts @@ -285,7 +285,7 @@ async function quickReactionStorageContains( }, emoji); } -test("message quick reaction tray stays neutral after selecting a tray emoji", async ({ +test("message reaction action stays neutral after selecting from the picker", async ({ page, }) => { await openGeneral(page); @@ -294,19 +294,18 @@ test("message quick reaction tray stays neutral after selecting a tray emoji", a await expect(row).toBeVisible(); await row.hover(); - const quickReactionButton = row.getByRole("button", { - name: "React with :+1:", - }); - await expect(quickReactionButton).toBeVisible(); - await quickReactionButton.click(); + const reactionTrigger = messageReactionTrigger(row); + await expect(reactionTrigger).toBeVisible(); + await reactionTrigger.click(); + const picker = page.locator("em-emoji-picker"); + await expect(picker).toBeVisible(); + await picker.locator("input[type='search']").fill("thumbs up"); + await picker.getByRole("button", { name: "👍" }).first().click(); await expect(row.getByLabel("Toggle 👍 reaction")).toBeVisible(); await row.hover(); - await expect(quickReactionButton).not.toHaveAttribute("aria-pressed", "true"); - await expect(quickReactionButton).not.toHaveClass(SELECTED_ACTION_CLASS); - await expect(messageReactionTrigger(row)).not.toHaveClass( - SELECTED_ACTION_CLASS, - ); + await expect(reactionTrigger).not.toHaveAttribute("aria-pressed", "true"); + await expect(reactionTrigger).not.toHaveClass(SELECTED_ACTION_CLASS); }); test("emoji picker keeps Frequently used live within the app session", async ({ diff --git a/desktop/tests/e2e/inbox-reactions.spec.ts b/desktop/tests/e2e/inbox-reactions.spec.ts index 1b7451cbcc..2385be0f6c 100644 --- a/desktop/tests/e2e/inbox-reactions.spec.ts +++ b/desktop/tests/e2e/inbox-reactions.spec.ts @@ -114,7 +114,7 @@ test("inbox reaction on a thread-reply mention persists after refetch", async ({ }, ); - // Open the inbox item and react via the hover action bar's quick reaction. + // Open the inbox item and react through Add reaction and the picker. const item = page.getByTestId(`home-inbox-item-${replyEvent.id}`); await item.click(); const detail = page.getByTestId("home-inbox-detail"); @@ -152,9 +152,11 @@ test("inbox reaction on a thread-reply mention persists after refetch", async ({ } expect(actionBarBox.y).toBeGreaterThanOrEqual(selectedMessageBox.y); - await selectedMessage - .getByRole("button", { name: "React with :+1:" }) - .click(); + await actionBar.getByRole("button", { name: "Open reactions" }).click(); + const picker = page.locator("em-emoji-picker"); + await expect(picker).toBeVisible(); + await picker.locator("input[type='search']").fill("thumbs up"); + await picker.getByRole("button", { name: "👍" }).first().click(); // The pill must appear AND persist: the post-toggle refetch replaces the // optimistic state with fetched reaction events. Give the refetch time to diff --git a/desktop/tests/e2e/message-copy-link.spec.ts b/desktop/tests/e2e/message-copy-link.spec.ts new file mode 100644 index 0000000000..a7ef2f10c6 --- /dev/null +++ b/desktop/tests/e2e/message-copy-link.spec.ts @@ -0,0 +1,182 @@ +import { expect, test, type Locator, type Page } from "@playwright/test"; + +import { KIND_HUDDLE_STARTED } from "../../src/shared/constants/kinds"; +import { installMockBridge } from "../helpers/bridge"; + +const GENERAL_CHANNEL_ID = "9a1657ac-f7aa-5db0-b632-d8bbeb6dfb50"; + +async function latestClipboardWrite(page: Page) { + return page.evaluate(() => + (window.__BUZZ_E2E_COMMAND_LOG__ ?? []).findLast( + ({ command }) => command === "copy_text_to_clipboard", + ), + ); +} + +async function expectCopyLinkUnavailable(row: Locator, messageId: string) { + await row.hover(); + await expect(row.getByTestId(`copy-link-message-${messageId}`)).toHaveCount( + 0, + ); + + const moreActions = row.getByTestId(`more-actions-${messageId}`); + if (await moreActions.count()) { + await moreActions.click({ force: true }); + await expect( + row.page().getByTestId(`copy-message-link-${messageId}`), + ).toHaveCount(0); + await row.page().keyboard.press("Escape"); + } +} + +test.beforeEach(async ({ page }) => { + await page.context().grantPermissions(["clipboard-read", "clipboard-write"], { + origin: "http://127.0.0.1:4173", + }); + await installMockBridge(page); +}); + +test("message action rail copies the same canonical thread link as More", async ({ + page, +}) => { + await page.setViewportSize({ width: 900, height: 700 }); + await page.goto("/"); + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + await expect + .poll(() => + page.evaluate( + () => typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function", + ), + ) + .toBe(true); + + const { replyId, rootId } = await page.evaluate(() => { + const emit = window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__; + if (!emit) throw new Error("Mock message emitter is unavailable."); + const root = emit({ + channelName: "general", + content: "Copy-link regression root", + id: "a".repeat(64), + }); + const reply = emit({ + channelName: "general", + content: "Copy-link regression reply", + id: "b".repeat(64), + parentEventId: root.id, + }); + return { replyId: reply.id, rootId: root.id }; + }); + + await page + .locator( + `[data-testid="message-thread-summary"][data-thread-head-id="${rootId}"]`, + ) + .click(); + const threadPanel = page.getByTestId("message-thread-panel"); + const replyRow = threadPanel.locator(`[data-message-id="${replyId}"]`); + await expect(replyRow).toContainText("Copy-link regression reply"); + await replyRow.hover(); + + const actionBar = replyRow.getByTestId(`message-action-bar-${replyId}`); + const quickReactions = actionBar.getByRole("button", { + name: /^React with /, + }); + await expect(quickReactions).toHaveCount(0); + const orderedActionNames = await actionBar + .getByRole("button") + .evaluateAll((buttons) => + buttons.map((button) => button.getAttribute("aria-label")), + ); + expect(orderedActionNames).toEqual([ + "Open reactions", + "Reply", + "Copy link", + "More actions", + ]); + + const copyLink = actionBar.getByTestId(`copy-link-message-${replyId}`); + await expect(copyLink).toHaveAccessibleName("Copy link"); + await copyLink.hover(); + await expect(page.getByRole("tooltip", { name: "Copy link" })).toBeVisible(); + + const expectedLink = `buzz://message?channel=${GENERAL_CHANNEL_ID}&id=${replyId}&thread=${rootId}`; + await copyLink.click(); + await expect + .poll(async () => (await latestClipboardWrite(page))?.payload.text) + .toBe(expectedLink); + await expect( + page.locator("[data-sonner-toast]").filter({ + hasText: "Link copied to clipboard", + }), + ).toBeVisible(); + + await actionBar.getByTestId(`more-actions-${replyId}`).click(); + await page.getByTestId(`copy-message-link-${replyId}`).click(); + await expect + .poll(async () => { + const writes = (await page.evaluate(() => + (window.__BUZZ_E2E_COMMAND_LOG__ ?? []).filter( + ({ command }) => command === "copy_text_to_clipboard", + ), + )) as Array<{ payload: unknown }>; + return writes.map(({ payload }) => payload); + }) + .toEqual([{ text: expectedLink }, { text: expectedLink }]); + + const [barBox, panelBox] = await Promise.all([ + actionBar.boundingBox(), + threadPanel.boundingBox(), + ]); + expect(barBox).not.toBeNull(); + expect(panelBox).not.toBeNull(); + if (!barBox || !panelBox) throw new Error("Message action bounds missing."); + expect(barBox.x).toBeGreaterThanOrEqual(panelBox.x); + expect(barBox.x + barBox.width).toBeLessThanOrEqual( + panelBox.x + panelBox.width, + ); +}); + +test("pending and huddle rows omit both copy-link surfaces", async ({ + page, +}) => { + await page.goto("/"); + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + await expect + .poll(() => + page.evaluate( + () => typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function", + ), + ) + .toBe(true); + + const { huddleId, pendingId } = await page.evaluate((huddleKind) => { + const emit = window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__; + if (!emit) throw new Error("Mock message emitter is unavailable."); + const pending = emit({ + channelName: "general", + content: "Pending copy-link regression", + id: "c".repeat(64), + pending: true, + }); + const huddle = emit({ + channelName: "general", + content: JSON.stringify({ + ephemeral_channel_id: "10000000-0000-4000-8000-000000000001", + }), + id: "d".repeat(64), + kind: huddleKind, + }); + return { huddleId: huddle.id, pendingId: pending.id }; + }, KIND_HUDDLE_STARTED); + + await expectCopyLinkUnavailable( + page.locator(`[data-message-id="${pendingId}"]`), + pendingId, + ); + await expectCopyLinkUnavailable( + page.locator(`[data-message-id="${huddleId}"]`), + huddleId, + ); +}); diff --git a/desktop/tests/e2e/reaction-order.spec.ts b/desktop/tests/e2e/reaction-order.spec.ts index 34cf071e77..b37ce56dd1 100644 --- a/desktop/tests/e2e/reaction-order.spec.ts +++ b/desktop/tests/e2e/reaction-order.spec.ts @@ -64,16 +64,19 @@ async function getBodyToReactionGap( return Math.round(reactions.y - (body.y + body.height)); } -/** Click a quick-reaction tray button by emoji (hover → click tray button). */ -async function addQuickReaction( +/** Add a reaction through the preserved message action and emoji picker. */ +async function addReaction( + page: import("@playwright/test").Page, row: import("@playwright/test").Locator, emoji: string, - label: string, + search: string, ) { await row.hover(); - const btn = row.getByRole("button", { name: `React with ${label}` }); - await expect(btn).toBeVisible(); - await btn.click(); + await row.getByRole("button", { name: "Open reactions" }).click(); + const picker = page.locator("em-emoji-picker"); + await expect(picker).toBeVisible(); + await picker.locator("input[type='search']").fill(search); + await picker.getByRole("button", { name: emoji }).first().click(); // Wait for the optimistic pill to appear before continuing. await expect( row @@ -97,11 +100,11 @@ test("reaction pills render left-to-right in the order reactions were added", as // Add three reactions in order: 👍 → ❤️ → 😂. // Wait >1 s between each so mock bridge timestamps differ by at least 1 Unix // second — the formatter sorts by created_at, so distinct seconds matter. - await addQuickReaction(row, "👍", ":+1:"); + await addReaction(page, row, "👍", "thumbs up"); await page.waitForTimeout(1100); - await addQuickReaction(row, "❤️", ":heart:"); + await addReaction(page, row, "❤️", "red heart"); await page.waitForTimeout(1100); - await addQuickReaction(row, "😂", ":joy:"); + await addReaction(page, row, "😂", "face with tears of joy"); const pills = await getPillOrder(row); expect(pills).toEqual(["👍", "❤️", "😂"]); @@ -136,9 +139,9 @@ test("a later emoji that accrues more reactors stays to the right of an earlier // since the key invariant is positional stability: even if ❤️ had higher // count it must stay right of 👍. // We use 🎉 (added second) and verify it stays right of 👍 (added first). - await addQuickReaction(row, "👍", ":+1:"); + await addReaction(page, row, "👍", "thumbs up"); await page.waitForTimeout(1100); - await addQuickReaction(row, "🎉", ":tada:"); + await addReaction(page, row, "🎉", "party popper"); // Both pills present in chronological order: 👍 left, 🎉 right. const afterAdd = await getPillOrder(row); From e760c51820b2103d965c22b44254678e10fb689a Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Mon, 24 Aug 2026 17:40:23 -0700 Subject: [PATCH 7/9] feat(workflows): discover trigger filter values (#6712) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Category:** improvement **User Impact:** Workflow authors can discover people and messages while configuring trigger filters, then see readable enriched labels instead of raw identifiers. **Problem:** Author and message filters required users to know and paste raw public keys or event IDs, and configured workflows surfaced those opaque values afterward. **Solution:** Add network-backed pickers and presentation enrichment while keeping deterministic local public-key and event-ID fallbacks authoritative whenever discovery is unavailable or untrusted. Related issue: none found.
File changes **desktop/src/features/workflows/ui/WorkflowAuthorPicker.tsx** Adds channel-aware author discovery, profile search, keyboard navigation, loading states, and deterministic public-key fallback selection. **desktop/src/features/workflows/ui/WorkflowCard.tsx** Uses enriched trigger presentation when building the workflow card’s readable summary. **desktop/src/features/workflows/ui/WorkflowDialog.tsx** Keeps Escape scoped to an active filter picker before allowing the inspector or dialog to close. **desktop/src/features/workflows/ui/WorkflowFormBuilder.tsx** Threads channel context into trigger filters and renders enriched author/message summaries in the workflow sequence. **desktop/src/features/workflows/ui/WorkflowMessagePicker.tsx** Adds paged channel-history discovery, message search, exact event lookup, profile labels, keyboard navigation, and bounded results. **desktop/src/features/workflows/ui/WorkflowRichTriggerDescription.tsx** Renders compact author identity details and loading presentation inside trigger summaries. **desktop/src/features/workflows/ui/WorkflowTriggerConditions.tsx** Connects author and message filter accordions to their pickers while preserving selected and excluded condition semantics. **desktop/src/features/workflows/ui/useWorkflowAuthorPresentation.ts** Resolves configured author keys to trusted display labels with deterministic fallbacks. **desktop/src/features/workflows/ui/useWorkflowTriggerPresentation.ts** Enriches configured message IDs only after validating the fetched event and channel. **desktop/src/features/workflows/ui/workflowAuthorCandidates.test.mjs** Covers author candidate normalization, ordering, deduplication, and fallback behavior. **desktop/src/features/workflows/ui/workflowAuthorCandidates.ts** Builds stable author candidates from channel members, profiles, and raw public keys. **desktop/src/features/workflows/ui/workflowConditionExpression.ts** Allows message IDs to participate in basic trigger-filter parsing. **desktop/src/features/workflows/ui/workflowDefinition.ts** Accepts enriched trigger text when generating workflow card labels. **desktop/src/features/workflows/ui/workflowMessageCandidates.test.mjs** Covers event validation, source merging, deterministic ordering, and exact-lookup enrichment boundaries. **desktop/src/features/workflows/ui/workflowMessageCandidates.ts** Validates message candidates by event kind, channel, and exact event ID before permitting enrichment. **desktop/src/features/workflows/ui/workflowTriggerDescription.test.mjs** Covers readable selected/excluded author and message descriptions plus loading fallbacks. **desktop/src/features/workflows/ui/workflowTriggerDescription.ts** Builds concise enriched trigger descriptions while retaining stable raw-ID fallbacks. **desktop/tests/e2e/workflow-local-controls.spec.ts** Exercises picker discovery, selection toggles, Escape ownership, bounded scrolling, and enriched workflow summaries.
## Reproduction steps 1. Open Workflows and create a workflow for a channel with members and message history. 2. Choose **Reaction Added** as the trigger and expand **Author**. 3. Confirm channel members and fetched profile results are discoverable, searchable, and keyboard accessible; choose one. 4. Expand **Message**, confirm recent channel messages appear in a bounded list, and choose one. 5. Toggle either selected filter between **is** and **is not**, then collapse the inspector and confirm the sequence summary stays readable. 6. Add a send-message step and create the workflow; confirm its card uses the resolved author and message labels. 7. Repeat while discovery is unavailable and confirm raw public keys/event IDs remain selectable and authoritative. ## Screenshots ### Author discovery ![Author picker showing discoverable channel members and profile labels](https://raw.githubusercontent.com/block/buzz/df47bdab841e0f52eb1f4ea9ac4e70aa16d240e0/pr-6712--01-author-discovery.png) ### Message discovery ![Message picker showing bounded channel history discovery](https://raw.githubusercontent.com/block/buzz/df47bdab841e0f52eb1f4ea9ac4e70aa16d240e0/pr-6712--02-message-discovery.png) ### Selected filter summaries ![Workflow builder showing readable selected author and message filters](https://raw.githubusercontent.com/block/buzz/df47bdab841e0f52eb1f4ea9ac4e70aa16d240e0/pr-6712--03-selected-filter-summaries.png) ### Enriched workflow card ![Workflow card showing enriched author and message labels](https://raw.githubusercontent.com/block/buzz/df47bdab841e0f52eb1f4ea9ac4e70aa16d240e0/pr-6712--04-enriched-workflow-card.png) --------- Signed-off-by: Taylor Ho Co-authored-by: Carl Co-authored-by: Princess Donut <0366ccd5ee09c2779a9d6bd6683daa17c16a508a51f6a7e7314018dab8fdc49b@buzz.block.builderlab.xyz> Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz> --- desktop/src-tauri/src/commands/messages.rs | 19 +- .../src/commands/messages/event_batch.rs | 128 +++++ desktop/src-tauri/src/lib.rs | 1 + .../workflows/ui/WorkflowAuthorPicker.tsx | 361 ++++++++++++++ .../features/workflows/ui/WorkflowCard.tsx | 20 +- .../features/workflows/ui/WorkflowDialog.tsx | 7 + .../workflows/ui/WorkflowFormBuilder.tsx | 34 +- .../workflows/ui/WorkflowMessagePicker.tsx | 469 ++++++++++++++++++ .../ui/WorkflowRichTriggerDescription.tsx | 53 ++ .../ui/WorkflowTriggerConditions.tsx | 220 +++++++- .../features/workflows/ui/WorkflowsView.tsx | 7 + .../ui/useWorkflowAuthorPresentation.ts | 57 +++ ...seWorkflowListAuthorPresentations.test.mjs | 30 ++ .../ui/useWorkflowListAuthorPresentations.ts | 60 +++ ...eWorkflowListMessagePresentations.test.mjs | 67 +++ .../ui/useWorkflowListMessagePresentations.ts | 92 ++++ .../ui/useWorkflowTriggerPresentation.ts | 70 +++ .../ui/workflowAuthorCandidates.test.mjs | 146 ++++++ .../workflows/ui/workflowAuthorCandidates.ts | 130 +++++ .../ui/workflowConditionExpression.ts | 4 +- .../workflows/ui/workflowDefinition.ts | 2 +- .../ui/workflowMessageCandidates.test.mjs | 218 ++++++++ .../workflows/ui/workflowMessageCandidates.ts | 139 ++++++ .../ui/workflowTriggerDescription.test.mjs | 63 ++- .../ui/workflowTriggerDescription.ts | 47 ++ desktop/src/shared/api/tauri.ts | 6 +- desktop/src/shared/api/tauriEvents.ts | 14 + desktop/src/testing/e2eBridge.ts | 9 + .../tests/e2e/workflow-local-controls.spec.ts | 355 ++++++++++++- 29 files changed, 2761 insertions(+), 67 deletions(-) create mode 100644 desktop/src-tauri/src/commands/messages/event_batch.rs create mode 100644 desktop/src/features/workflows/ui/WorkflowAuthorPicker.tsx create mode 100644 desktop/src/features/workflows/ui/WorkflowMessagePicker.tsx create mode 100644 desktop/src/features/workflows/ui/WorkflowRichTriggerDescription.tsx create mode 100644 desktop/src/features/workflows/ui/useWorkflowAuthorPresentation.ts create mode 100644 desktop/src/features/workflows/ui/useWorkflowListAuthorPresentations.test.mjs create mode 100644 desktop/src/features/workflows/ui/useWorkflowListAuthorPresentations.ts create mode 100644 desktop/src/features/workflows/ui/useWorkflowListMessagePresentations.test.mjs create mode 100644 desktop/src/features/workflows/ui/useWorkflowListMessagePresentations.ts create mode 100644 desktop/src/features/workflows/ui/useWorkflowTriggerPresentation.ts create mode 100644 desktop/src/features/workflows/ui/workflowAuthorCandidates.test.mjs create mode 100644 desktop/src/features/workflows/ui/workflowAuthorCandidates.ts create mode 100644 desktop/src/features/workflows/ui/workflowMessageCandidates.test.mjs create mode 100644 desktop/src/features/workflows/ui/workflowMessageCandidates.ts create mode 100644 desktop/src/shared/api/tauriEvents.ts diff --git a/desktop/src-tauri/src/commands/messages.rs b/desktop/src-tauri/src/commands/messages.rs index 5de2d32b80..1e221b6bd1 100644 --- a/desktop/src-tauri/src/commands/messages.rs +++ b/desktop/src-tauri/src/commands/messages.rs @@ -396,23 +396,8 @@ pub async fn get_channel_messages_before( }) } -#[tauri::command] -pub async fn get_event(event_id: String, state: State<'_, AppState>) -> Result { - let events = query_relay( - &state, - &[serde_json::json!({ - "ids": [event_id], - "kinds": [0, 1, 3, 5, 7, 9, 30078, 40002, 40003, 40008, 40099, 40100, 45001, 45003, buzz_core_pkg::kind::KIND_HUDDLE_STARTED], - "limit": 1 - })], - ) - .await?; - - let ev = events - .first() - .ok_or_else(|| "event not found".to_string())?; - serde_json::to_string(ev).map_err(|e| format!("serialize event: {e}")) -} +mod event_batch; +pub use event_batch::{get_event, get_events}; // ── Writes ────────────────────────────────────────────────────────────────── diff --git a/desktop/src-tauri/src/commands/messages/event_batch.rs b/desktop/src-tauri/src/commands/messages/event_batch.rs new file mode 100644 index 0000000000..1bb5174868 --- /dev/null +++ b/desktop/src-tauri/src/commands/messages/event_batch.rs @@ -0,0 +1,128 @@ +use std::collections::HashSet; + +use tauri::State; + +use crate::{app_state::AppState, relay::query_relay}; + +// The relay clamps a single filter to this many events. Keep exact-ID reads in +// chunks so a large workflow list cannot silently lose late presentations. +const EVENT_QUERY_CHUNK_SIZE: usize = 1_000; + +const GET_EVENT_KINDS: [u32; 15] = [ + 0, + 1, + 3, + 5, + 7, + 9, + 30078, + 40002, + 40003, + 40008, + 40099, + 40100, + 45001, + 45003, + buzz_core_pkg::kind::KIND_HUDDLE_STARTED, +]; + +#[tauri::command] +pub async fn get_event(event_id: String, state: State<'_, AppState>) -> Result { + let events = query_relay( + &state, + &[serde_json::json!({ + "ids": [event_id], + "kinds": GET_EVENT_KINDS, + "limit": 1 + })], + ) + .await?; + + let event = events + .first() + .ok_or_else(|| "event not found".to_string())?; + serde_json::to_string(event).map_err(|error| format!("serialize event: {error}")) +} + +/// Resolve many exact event IDs in relay-sized chunks. Callers still validate +/// event kind, channel scope, and requested ID before using presentation data. +fn normalized_event_id_chunks(event_ids: Vec) -> Vec> { + let mut seen_ids = HashSet::new(); + let event_ids = event_ids + .into_iter() + .map(|event_id| event_id.trim().to_ascii_lowercase()) + .filter(|event_id| event_id.len() == 64 && event_id.chars().all(|c| c.is_ascii_hexdigit())) + .filter(|event_id| seen_ids.insert(event_id.clone())) + .collect::>(); + event_ids + .chunks(EVENT_QUERY_CHUNK_SIZE) + .map(<[String]>::to_vec) + .collect() +} + +#[tauri::command] +pub async fn get_events( + event_ids: Vec, + state: State<'_, AppState>, +) -> Result, String> { + let event_id_chunks = normalized_event_id_chunks(event_ids); + if event_id_chunks.is_empty() { + return Ok(Vec::new()); + } + + let mut events_by_id = std::collections::HashMap::new(); + for event_ids in event_id_chunks { + let events = query_relay( + &state, + &[serde_json::json!({ + "ids": event_ids, + "kinds": GET_EVENT_KINDS, + "limit": event_ids.len() + })], + ) + .await?; + for event in events { + events_by_id.entry(event.id).or_insert(event); + } + } + + events_by_id + .into_values() + .map(|event| { + serde_json::to_value(event).map_err(|error| format!("serialize event: {error}")) + }) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn keeps_exact_relay_ceiling_in_one_chunk() { + let chunks = normalized_event_id_chunks( + (0..EVENT_QUERY_CHUNK_SIZE) + .map(|index| format!("{index:064x}")) + .collect(), + ); + + assert_eq!(chunks.len(), 1); + assert_eq!(chunks[0].len(), EVENT_QUERY_CHUNK_SIZE); + } + + #[test] + fn normalizes_deduplicates_and_keeps_ids_beyond_relay_ceiling() { + let last_id = format!("{:064x}", EVENT_QUERY_CHUNK_SIZE); + let mut event_ids = (0..=EVENT_QUERY_CHUNK_SIZE) + .map(|index| format!("{index:064X}")) + .collect::>(); + event_ids.extend(["not-an-event-id".to_string(), format!(" {last_id} ")]); + + let chunks = normalized_event_id_chunks(event_ids); + + assert_eq!(chunks.iter().map(Vec::len).sum::(), 1_001); + assert_eq!(chunks.len(), 2); + assert_eq!(chunks[0].len(), EVENT_QUERY_CHUNK_SIZE); + assert_eq!(chunks[1], [last_id]); + } +} diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 428aa4d2a7..613040b809 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -652,6 +652,7 @@ pub fn run() { add_reaction, remove_reaction, get_event, + get_events, show_native_notification, #[cfg(target_os = "macos")] macos_notifications::take_pending_activations, diff --git a/desktop/src/features/workflows/ui/WorkflowAuthorPicker.tsx b/desktop/src/features/workflows/ui/WorkflowAuthorPicker.tsx new file mode 100644 index 0000000000..bcf0b209ff --- /dev/null +++ b/desktop/src/features/workflows/ui/WorkflowAuthorPicker.tsx @@ -0,0 +1,361 @@ +import { Check, LoaderCircle, Search } from "lucide-react"; +import * as React from "react"; + +import { useChannelMembersQuery } from "@/features/channels/hooks"; +import { useRelayMembersQuery } from "@/features/community-members/hooks"; +import { + useFlattenedUserSearchResults, + useInfiniteUserSearchQuery, + useUsersBatchQuery, +} from "@/features/profile/hooks"; +import { resolveUserLabel } from "@/features/profile/lib/identity"; +import { cn } from "@/shared/lib/cn"; +import { truncatePubkey } from "@/shared/lib/pubkey"; +import { Input } from "@/shared/ui/input"; +import { UserAvatar } from "@/shared/ui/UserAvatar"; +import { + enrichAuthorCandidates, + filterAuthorCandidatePage, + mergeAuthorCandidateSources, + nextWorkflowAuthorIndex, + parseDirectAuthorInput, + type WorkflowAuthorCandidate, +} from "./workflowAuthorCandidates"; + +const PAGE_SIZE = 50; + +export function WorkflowAuthorPicker({ + channelId, + disabled, + id, + onChange, + onEscape, + value, +}: { + channelId?: string | null; + disabled?: boolean; + id: string; + onChange: (pubkey: string) => void; + onEscape?: () => void; + value: string; +}) { + const pickerRef = React.useRef(null); + const optionRefs = React.useRef(new Map()); + const [query, setQuery] = React.useState(""); + const [activeIndex, setActiveIndex] = React.useState(null); + const [columnCount, setColumnCount] = React.useState(2); + const trimmedQuery = query.trim(); + const deferredQuery = React.useDeferredValue(trimmedQuery); + const normalizedValue = parseDirectAuthorInput(value); + const channelMembersQuery = useChannelMembersQuery(channelId ?? null); + const relayMembersQuery = useRelayMembersQuery(true); + const directoryQuery = useInfiniteUserSearchQuery(deferredQuery, { + allowEmpty: true, + limit: PAGE_SIZE, + }); + const directoryResults = useFlattenedUserSearchResults(directoryQuery.data); + const directPubkey = parseDirectAuthorInput(deferredQuery); + + const baseCandidates = React.useMemo( + () => + mergeAuthorCandidateSources([ + normalizedValue ? [{ pubkey: normalizedValue }] : [], + directPubkey ? [{ pubkey: directPubkey }] : [], + channelMembersQuery.data ?? [], + relayMembersQuery.data ?? [], + directoryResults, + ]), + [ + channelMembersQuery.data, + directPubkey, + directoryResults, + normalizedValue, + relayMembersQuery.data, + ], + ); + const candidatePage = React.useMemo( + () => + filterAuthorCandidatePage( + baseCandidates, + deferredQuery, + directPubkey, + PAGE_SIZE, + ), + [baseCandidates, deferredQuery, directPubkey], + ); + const profileQuery = useUsersBatchQuery( + candidatePage.map(({ pubkey }) => pubkey), + ); + const candidates = React.useMemo( + () => + enrichAuthorCandidates(candidatePage, profileQuery.data?.profiles ?? {}), + [candidatePage, profileQuery.data?.profiles], + ); + const visibleCandidates = candidates; + const listId = `${id}-list`; + + React.useEffect(() => { + if (activeIndex !== null && activeIndex >= visibleCandidates.length) { + setActiveIndex( + visibleCandidates.length > 0 ? visibleCandidates.length - 1 : null, + ); + } + }, [activeIndex, visibleCandidates.length]); + + React.useEffect(() => { + const picker = pickerRef.current; + if (!picker) return; + const updateColumnCount = () => + setColumnCount(picker.clientWidth >= 544 ? 3 : 2); + updateColumnCount(); + const observer = new ResizeObserver(updateColumnCount); + observer.observe(picker); + return () => observer.disconnect(); + }, []); + + React.useEffect(() => { + if (activeIndex === null) return; + const candidate = visibleCandidates[activeIndex]; + if (!candidate) return; + optionRefs.current + .get(candidate.pubkey) + ?.scrollIntoView({ block: "nearest", inline: "nearest" }); + }, [activeIndex, visibleCandidates]); + + const loading = + (channelMembersQuery.isLoading || + relayMembersQuery.isLoading || + directoryQuery.isLoading) && + visibleCandidates.length === 0; + const failed = + channelMembersQuery.isError || + relayMembersQuery.isError || + directoryQuery.isError; + + function moveActive(delta: number) { + setActiveIndex((current) => + nextWorkflowAuthorIndex(current, delta, visibleCandidates.length), + ); + } + + return ( +
+
+ + { + setQuery(event.target.value); + setActiveIndex(null); + }} + onKeyDown={(event) => { + const currentQuery = event.currentTarget.value.trim(); + const delta = + event.key === "ArrowDown" + ? columnCount + : event.key === "ArrowUp" + ? -columnCount + : event.key === "ArrowRight" + ? 1 + : event.key === "ArrowLeft" + ? -1 + : 0; + if (delta) { + event.preventDefault(); + moveActive(delta); + } else if ( + event.key === "Enter" && + currentQuery === deferredQuery && + visibleCandidates[activeIndex ?? 0] + ) { + event.preventDefault(); + const candidate = visibleCandidates[activeIndex ?? 0]; + onChange( + candidate.pubkey === normalizedValue ? "" : candidate.pubkey, + ); + } else if (event.key === "Escape") { + event.preventDefault(); + event.stopPropagation(); + if (query) { + setQuery(""); + setActiveIndex(null); + } else { + onEscape?.(); + } + } + }} + placeholder="Search people or paste a public key…" + role="combobox" + spellCheck={false} + value={query} + /> +
+ +
{ + const list = event.currentTarget; + if ( + list.scrollHeight - list.scrollTop - list.clientHeight < 64 && + directoryQuery.hasNextPage && + !directoryQuery.isFetchingNextPage + ) { + void directoryQuery.fetchNextPage(); + } + }} + role="listbox" + > + {visibleCandidates.map((candidate, index) => ( + { + setActiveIndex(null); + onChange( + candidate.pubkey === normalizedValue ? "" : candidate.pubkey, + ); + }} + optionRef={(node) => { + if (node) optionRefs.current.set(candidate.pubkey, node); + else optionRefs.current.delete(candidate.pubkey); + }} + selected={candidate.pubkey === normalizedValue} + /> + ))} + {loading ? ( +

+ Loading authors… +

+ ) : visibleCandidates.length === 0 ? ( +

+ {failed ? "Couldn’t load authors." : "No authors found."} +

+ ) : null} + {failed ? ( + + ) : null} + {directoryQuery.hasNextPage ? ( + + ) : null} +
+
+ ); +} + +function AuthorOption({ + active, + candidate, + disabled, + id, + onSelect, + optionRef, + selected, +}: { + active: boolean; + candidate: WorkflowAuthorCandidate; + disabled?: boolean; + id: string; + onSelect: () => void; + optionRef: (node: HTMLButtonElement | null) => void; + selected: boolean; +}) { + const label = resolveUserLabel({ + fallbackName: candidate.displayName, + profiles: { + [candidate.pubkey]: { + displayName: candidate.displayName, + avatarUrl: candidate.avatarUrl, + nip05Handle: candidate.nip05Handle, + ownerPubkey: candidate.ownerPubkey, + isAgent: candidate.isAgent, + }, + }, + pubkey: candidate.pubkey, + }); + return ( + + ); +} diff --git a/desktop/src/features/workflows/ui/WorkflowCard.tsx b/desktop/src/features/workflows/ui/WorkflowCard.tsx index 48ce8d2db6..8556424f87 100644 --- a/desktop/src/features/workflows/ui/WorkflowCard.tsx +++ b/desktop/src/features/workflows/ui/WorkflowCard.tsx @@ -24,14 +24,20 @@ import { getWorkflowActionTiles, getWorkflowCardLabel, getWorkflowTriggerEmoji, + getWorkflowTriggerConfig, getWorkflowTriggerType, } from "./workflowDefinition"; import { StatusEmoji } from "@/features/user-status/ui/StatusEmoji"; +import type { WorkflowCardAuthorPresentation } from "./useWorkflowListAuthorPresentations"; +import type { WorkflowMessagePresentation } from "./useWorkflowListMessagePresentations"; +import { workflowTriggerDescription } from "./workflowTriggerDescription"; type WorkflowCardProps = { workflow: Workflow; + authorPresentation?: WorkflowCardAuthorPresentation; channelName?: string; isTogglingEnabled?: boolean; + messagePresentation?: WorkflowMessagePresentation; onView: (workflow: Workflow) => void; onTrigger: (workflowId: string) => void; onToggleEnabled: (workflow: Workflow) => void; @@ -189,8 +195,10 @@ function ActionTileStack({ export function WorkflowCard({ workflow, + authorPresentation, channelName, isTogglingEnabled = false, + messagePresentation, onView, onTrigger, onToggleEnabled, @@ -201,7 +209,17 @@ export function WorkflowCard({ const [triggerAnimationSequence, setTriggerAnimationSequence] = React.useState(0); const isEnabled = getWorkflowEnabled(workflow.definition); - const cardLabel = getWorkflowCardLabel(workflow.definition); + const configuredTrigger = getWorkflowTriggerConfig(workflow.definition); + const cardLabel = getWorkflowCardLabel(workflow.definition, { + triggerDescription: configuredTrigger + ? workflowTriggerDescription(configuredTrigger, { + authorLabel: authorPresentation?.label ?? undefined, + authorLoading: authorPresentation?.loading, + messageLabel: messagePresentation?.messageLabel ?? undefined, + messageLoading: messagePresentation?.messageLoading, + }) + : undefined, + }); const triggerType = getWorkflowTriggerType(workflow.definition); const actionTiles = getWorkflowActionTiles(workflow.definition); const triggerEmoji = getWorkflowTriggerEmoji(workflow.definition); diff --git a/desktop/src/features/workflows/ui/WorkflowDialog.tsx b/desktop/src/features/workflows/ui/WorkflowDialog.tsx index 3f8655a3aa..198e0fe91c 100644 --- a/desktop/src/features/workflows/ui/WorkflowDialog.tsx +++ b/desktop/src/features/workflows/ui/WorkflowDialog.tsx @@ -557,6 +557,13 @@ export function WorkflowDialog({ { + if ( + event.target instanceof HTMLElement && + event.target.closest("[data-workflow-filter-picker-search]") + ) { + event.preventDefault(); + return; + } if (formBuilderRef.current?.closeInspector()) { event.preventDefault(); event.stopPropagation(); diff --git a/desktop/src/features/workflows/ui/WorkflowFormBuilder.tsx b/desktop/src/features/workflows/ui/WorkflowFormBuilder.tsx index 58e4b4a960..8ac97402aa 100644 --- a/desktop/src/features/workflows/ui/WorkflowFormBuilder.tsx +++ b/desktop/src/features/workflows/ui/WorkflowFormBuilder.tsx @@ -30,8 +30,10 @@ import { Switch } from "@/shared/ui/switch"; import { Textarea } from "@/shared/ui/textarea"; import { reactionConditionValue } from "./workflowReactionCondition"; import { WorkflowTriggerConditions } from "./WorkflowTriggerConditions"; +import { WorkflowRichTriggerDescription } from "./WorkflowRichTriggerDescription"; +import { useWorkflowTriggerPresentation } from "./useWorkflowTriggerPresentation"; +import { compactWorkflowTriggerDescription } from "./workflowTriggerDescription"; import { workflowStepDescription } from "./workflowStepDescription"; -import { workflowTriggerDescription } from "./workflowTriggerDescription"; import { WorkflowScheduleFields } from "./WorkflowScheduleFields"; import { WorkflowStepCard } from "./WorkflowStepCard"; import { @@ -67,12 +69,14 @@ function TriggerConfigFields({ trigger, onConditionDraftsChange, onUpdate, + workflowChannelId, }: { conditionDrafts: ParsedConditionExpression[] | null; disabled?: boolean; trigger: TriggerConfig; onConditionDraftsChange: (drafts: ParsedConditionExpression[] | null) => void; onUpdate: (trigger: TriggerConfig) => void; + workflowChannelId?: string | null; }) { switch (trigger.on) { case "message_posted": @@ -94,6 +98,7 @@ function TriggerConfigFields({ } triggerType={trigger.on} value={reactionConditionValue(trigger)} + workflowChannelId={workflowChannelId} /> ); case "webhook": @@ -220,7 +225,7 @@ function WorkflowNode({ terminal, title, }: { - description: string; + description: React.ReactNode; disabled?: boolean; icon?: React.ReactNode; label: string; @@ -614,10 +619,23 @@ export const WorkflowFormBuilder = React.forwardRef< ) ?.value.trim(); }, [formState.trigger]); - const triggerDescription = workflowTriggerDescription(formState.trigger); - const visibleTriggerDescription = triggerEmoji - ? "Reaction added" - : triggerDescription; + const triggerPresentation = useWorkflowTriggerPresentation( + formState.trigger, + workflowChannelId, + ); + const triggerDescription = triggerPresentation.description; + const compactTriggerDescription = compactWorkflowTriggerDescription( + triggerDescription, + triggerEmoji, + ); + const visibleTriggerDescription = ( + + ); const TriggerIcon = { diff_posted: GitPullRequest, message_posted: MessageSquare, @@ -889,6 +907,7 @@ export const WorkflowFormBuilder = React.forwardRef< > {selectedNode.type === "trigger" ? ( -
+
) : selectedStep ? ( diff --git a/desktop/src/features/workflows/ui/WorkflowMessagePicker.tsx b/desktop/src/features/workflows/ui/WorkflowMessagePicker.tsx new file mode 100644 index 0000000000..6639d85b0f --- /dev/null +++ b/desktop/src/features/workflows/ui/WorkflowMessagePicker.tsx @@ -0,0 +1,469 @@ +import { useInfiniteQuery, useQueries, useQuery } from "@tanstack/react-query"; +import { Check, LoaderCircle, Search } from "lucide-react"; +import * as React from "react"; + +import { parseChannelWindowResponse } from "@/features/messages/lib/channelWindowResponse"; +import { useUsersBatchQuery } from "@/features/profile/hooks"; +import { + resolveUserLabel, + type UserProfileLookup, +} from "@/features/profile/lib/identity"; +import { useSearchMessagesQuery } from "@/features/search/hooks"; +import { getChannelWindowEvents } from "@/shared/api/channelWindow"; +import { getEventById } from "@/shared/api/tauri"; +import type { ChannelPageCursor } from "@/shared/api/types"; +import { cn } from "@/shared/lib/cn"; +import { Input } from "@/shared/ui/input"; +import { UserAvatar } from "@/shared/ui/UserAvatar"; +import { + mergeMessageCandidateSources, + normalizeMessageEventId, + type WorkflowMessageCandidate, + validatedWorkflowMessageCandidate, + validateWorkflowMessageSearchResults, +} from "./workflowMessageCandidates"; + +const PAGE_SIZE = 25; + +function truncateContent(content: string | null): string { + const normalized = content?.trim().replaceAll(/\s+/g, " ") ?? ""; + if (!normalized) return "No message body"; + return normalized.length > 120 + ? `${normalized.slice(0, 117)}...` + : normalized; +} + +function formatTimestamp(unixSeconds: number | null): string | null { + if (unixSeconds === null) return null; + return new Intl.DateTimeFormat(undefined, { + month: "short", + day: "numeric", + hour: "numeric", + minute: "2-digit", + }).format(new Date(unixSeconds * 1_000)); +} + +export function WorkflowMessagePicker({ + channelId, + disabled, + id, + onChange, + onEscape, + value, +}: { + channelId?: string | null; + disabled?: boolean; + id: string; + onChange: (messageId: string) => void; + onEscape?: () => void; + value: string; +}) { + const optionRefs = React.useRef(new Map()); + const [query, setQuery] = React.useState(""); + const [activeIndex, setActiveIndex] = React.useState(null); + const trimmedQuery = query.trim(); + const deferredQuery = React.useDeferredValue(trimmedQuery); + const normalizedQuery = deferredQuery.toLowerCase(); + const selectedId = normalizeMessageEventId(value); + const directId = normalizeMessageEventId(query); + const lookupId = directId ?? selectedId; + + const historyQuery = useInfiniteQuery({ + enabled: Boolean(channelId), + initialPageParam: null as ChannelPageCursor | null, + queryKey: ["workflow-message-picker", channelId], + queryFn: async ({ pageParam }) => { + if (!channelId) throw new Error("Choose a channel first."); + return parseChannelWindowResponse( + await getChannelWindowEvents(channelId, pageParam, PAGE_SIZE), + channelId, + pageParam, + ); + }, + getNextPageParam: (lastPage) => lastPage.nextCursor ?? undefined, + staleTime: 30_000, + }); + const searchQuery = useSearchMessagesQuery(deferredQuery, { + channelId: channelId ?? undefined, + enabled: Boolean(channelId && normalizedQuery && !directId), + limit: 30, + minimumQueryLength: 1, + }); + const exactQuery = useQuery({ + enabled: Boolean(channelId && lookupId), + queryKey: ["workflow-message-picker-exact", channelId, lookupId], + queryFn: () => getEventById(lookupId ?? ""), + retry: false, + staleTime: 60_000, + }); + + const selectedFallback = selectedId + ? [{ id: selectedId, pubkey: null, content: null, createdAt: null }] + : []; + const directFallback = directId + ? [{ id: directId, pubkey: null, content: null, createdAt: null }] + : []; + const historyCandidates = React.useMemo( + () => + (historyQuery.data?.pages ?? []).flatMap((page) => + page.rows.flatMap(({ event }) => { + const candidate = channelId + ? validatedWorkflowMessageCandidate(event, { channelId }) + : null; + return candidate ? [candidate] : []; + }), + ), + [channelId, historyQuery.data?.pages], + ); + const searchHitIds = React.useMemo( + () => [ + ...new Set( + (searchQuery.data?.hits ?? []).flatMap((hit) => { + const eventId = normalizeMessageEventId(hit.eventId); + return eventId ? [eventId] : []; + }), + ), + ], + [searchQuery.data?.hits], + ); + const searchEventQueries = useQueries({ + queries: searchHitIds.map((eventId) => ({ + enabled: Boolean(channelId), + queryKey: ["workflow-message-picker-search-event", channelId, eventId], + queryFn: () => getEventById(eventId), + retry: false, + staleTime: 60_000, + })), + }); + const searchCandidates = validateWorkflowMessageSearchResults( + searchHitIds.map((requestedId, index) => ({ + requestedId, + event: searchEventQueries[index]?.data, + })), + channelId ?? "", + ); + const exactCandidate = React.useMemo(() => { + if (!channelId || !lookupId) return null; + return validatedWorkflowMessageCandidate(exactQuery.data, { + channelId, + requestedId: lookupId, + }); + }, [channelId, exactQuery.data, lookupId]); + const allCandidates = React.useMemo(() => { + // Preserve the relay-provided history/search order. Exact lookups and raw-ID + // fallbacks only fill gaps; selecting an existing row must not move it. + return mergeMessageCandidateSources([ + historyCandidates, + searchCandidates, + exactCandidate ? [exactCandidate] : [], + selectedFallback, + directFallback, + ]); + }, [ + directFallback, + exactCandidate, + historyCandidates, + searchCandidates, + selectedFallback, + ]); + const visibleCandidates = React.useMemo(() => { + if (directId) return allCandidates.filter(({ id }) => id === directId); + if (!normalizedQuery) return allCandidates; + return allCandidates.filter( + (candidate) => + candidate.id.includes(normalizedQuery) || + candidate.content?.toLowerCase().includes(normalizedQuery), + ); + }, [allCandidates, directId, normalizedQuery]); + const profilePubkeys = React.useMemo( + () => [ + ...new Set( + visibleCandidates.flatMap(({ pubkey }) => (pubkey ? [pubkey] : [])), + ), + ], + [visibleCandidates], + ); + const profilesQuery = useUsersBatchQuery(profilePubkeys); + const listId = `${id}-list`; + + React.useEffect(() => { + if (activeIndex !== null && activeIndex >= visibleCandidates.length) { + setActiveIndex( + visibleCandidates.length > 0 ? visibleCandidates.length - 1 : null, + ); + } + }, [activeIndex, visibleCandidates.length]); + React.useEffect(() => { + if (activeIndex === null) return; + const candidate = visibleCandidates[activeIndex]; + if (!candidate) return; + optionRefs.current + .get(candidate.id) + ?.scrollIntoView({ block: "nearest", inline: "nearest" }); + }, [activeIndex, visibleCandidates]); + + const searchEventsFetching = searchEventQueries.some( + ({ isFetching }) => isFetching, + ); + const searchEventsFailed = searchEventQueries.some(({ isError }) => isError); + const loading = + (historyQuery.isLoading || + searchQuery.isFetching || + searchEventsFetching || + exactQuery.isFetching) && + visibleCandidates.length === 0; + const failed = + historyQuery.isError || + searchQuery.isError || + searchEventsFailed || + exactQuery.isError; + const invalidDirectResult = Boolean( + directId && lookupId === directId && exactQuery.data && !exactCandidate, + ); + + return ( +
+
+ + { + setQuery(event.target.value); + setActiveIndex(null); + }} + onKeyDown={(event) => { + const currentQuery = event.currentTarget.value.trim(); + if (event.key === "ArrowDown" || event.key === "ArrowUp") { + event.preventDefault(); + if (visibleCandidates.length > 0) { + setActiveIndex((current) => { + const startingIndex = current ?? -1; + return ( + (startingIndex + + (event.key === "ArrowDown" ? 1 : -1) + + visibleCandidates.length) % + visibleCandidates.length + ); + }); + } + } else if (event.key === "Enter") { + const currentDirectId = normalizeMessageEventId(currentQuery); + if (currentQuery !== deferredQuery && !currentDirectId) return; + const candidate = currentDirectId + ? allCandidates.find(({ id }) => id === currentDirectId) + : visibleCandidates[activeIndex ?? 0]; + if (!candidate) return; + event.preventDefault(); + if (invalidDirectResult) return; + onChange(candidate.id === selectedId ? "" : candidate.id); + } else if (event.key === "Escape") { + event.preventDefault(); + event.stopPropagation(); + if (query) { + setQuery(""); + setActiveIndex(null); + } else { + onEscape?.(); + } + } + }} + placeholder={ + channelId + ? "Search messages or paste a message ID…" + : "Choose a channel first" + } + role="combobox" + spellCheck={false} + value={query} + /> + {historyQuery.isFetching || + searchQuery.isFetching || + searchEventsFetching || + exactQuery.isFetching ? ( + + ) : null} +
+ {invalidDirectResult ? ( +

+ That message is not available in this channel. +

+ ) : null} +
{ + const list = event.currentTarget; + if ( + !normalizedQuery && + list.scrollHeight - list.scrollTop - list.clientHeight < 64 && + historyQuery.hasNextPage && + !historyQuery.isFetchingNextPage + ) { + void historyQuery.fetchNextPage(); + } + }} + role="listbox" + > + {visibleCandidates.map((candidate, index) => ( + { + setActiveIndex(null); + if (!invalidDirectResult) { + onChange(candidate.id === selectedId ? "" : candidate.id); + } + }} + optionRef={(node) => { + if (node) optionRefs.current.set(candidate.id, node); + else optionRefs.current.delete(candidate.id); + }} + profiles={profilesQuery.data?.profiles} + selected={candidate.id === selectedId} + /> + ))} + {loading ? ( +

+ Loading messages… +

+ ) : visibleCandidates.length === 0 ? ( +

+ {failed + ? "Couldn’t load messages." + : normalizedQuery + ? "No messages found." + : "No messages yet."} +

+ ) : null} + {failed ? ( + + ) : null} + {!normalizedQuery && historyQuery.hasNextPage ? ( + + ) : null} +
+
+ ); +} + +function MessageOption({ + active, + candidate, + disabled, + id, + onSelect, + optionRef, + profiles, + selected, +}: { + active: boolean; + candidate: WorkflowMessageCandidate; + disabled?: boolean; + id: string; + onSelect: () => void; + optionRef: (node: HTMLButtonElement | null) => void; + profiles?: UserProfileLookup; + selected: boolean; +}) { + const author = candidate.pubkey + ? resolveUserLabel({ profiles, pubkey: candidate.pubkey }) + : "Selected message"; + const profile = candidate.pubkey + ? profiles?.[candidate.pubkey.toLowerCase()] + : undefined; + const timestamp = formatTimestamp(candidate.createdAt); + return ( + + ); +} diff --git a/desktop/src/features/workflows/ui/WorkflowRichTriggerDescription.tsx b/desktop/src/features/workflows/ui/WorkflowRichTriggerDescription.tsx new file mode 100644 index 0000000000..50eb5ef672 --- /dev/null +++ b/desktop/src/features/workflows/ui/WorkflowRichTriggerDescription.tsx @@ -0,0 +1,53 @@ +import { LoaderCircle } from "lucide-react"; + +import { UserAvatar } from "@/shared/ui/UserAvatar"; +import { splitWorkflowAuthorDescription } from "./workflowTriggerDescription"; + +export function WorkflowRichTriggerDescription({ + avatarUrl, + description, + label, + loading, +}: { + avatarUrl?: string | null; + description: string; + label?: string | null; + loading?: boolean; +}) { + if (loading) { + return ( + + {description} + + + ); + } + + const segments = label + ? splitWorkflowAuthorDescription(description, label) + : null; + if (!label || !segments) return description; + + const { prefix, suffix } = segments; + return ( + + {prefix ? {prefix} : null} + + + {label} + {suffix ? ` ${suffix}` : ""} + + + ); +} diff --git a/desktop/src/features/workflows/ui/WorkflowTriggerConditions.tsx b/desktop/src/features/workflows/ui/WorkflowTriggerConditions.tsx index 39caec253a..78f6e30606 100644 --- a/desktop/src/features/workflows/ui/WorkflowTriggerConditions.tsx +++ b/desktop/src/features/workflows/ui/WorkflowTriggerConditions.tsx @@ -3,8 +3,12 @@ import * as React from "react"; import { cn } from "@/shared/lib/cn"; import { Input } from "@/shared/ui/input"; +import { UserAvatar } from "@/shared/ui/UserAvatar"; import { Tabs, TabsList, TabsTrigger } from "@/shared/ui/tabs"; +import { WorkflowAuthorPicker } from "./WorkflowAuthorPicker"; import { WorkflowEmojiField } from "./WorkflowEmojiField"; +import { WorkflowMessagePicker } from "./WorkflowMessagePicker"; +import { useWorkflowTriggerPresentation } from "./useWorkflowTriggerPresentation"; import { FieldLabel } from "./workflowFormPrimitives"; import { buildConditionExpressions, @@ -45,12 +49,81 @@ function compact(value: string): string { : `${trimmed.slice(0, 11)}…${trimmed.slice(-6)}`; } +function fieldUsesFullHeightPicker(field: string): boolean { + return field === "trigger_author" || field === "trigger_message_id"; +} + function fieldPlaceholder(field: string): string { if (field === "trigger_author") return "64-character hex pubkey"; if (field === "trigger_message_id") return "64-character hex event ID"; return "e.g. deploy"; } +function ExclusionStrike() { + return ( +
) : ( -
+
{fields.map((field) => { const existing = conditions.find( (condition) => condition.field === field.value, @@ -179,8 +279,32 @@ export function WorkflowTriggerConditions({ const summary = existing ? `${OPERATOR_LABELS[condition.operator]}${condition.value ? ` ${compact(condition.value)}` : ""}` : "Any"; + const authorSummary = + existing && + field.value === "trigger_author" && + triggerPresentation.pubkey && + triggerPresentation.label + ? triggerPresentation.label + : null; + const messageSummary = + existing && + field.value === "trigger_message_id" && + triggerPresentation.messageId && + triggerPresentation.messageLabel + ? triggerPresentation.messageLabel + .trim() + .replaceAll(/\s+/g, " ") + : null; return ( -
+
{expanded ? ( -
+
Match
@@ -258,6 +417,51 @@ export function WorkflowTriggerConditions({ } value={condition.value} /> + ) : field.value === "trigger_author" ? ( + collapsePicker(field.value)} + onChange={(pubkey) => + updateConditions( + pubkey + ? [ + ...conditions.filter( + (item) => item.field !== field.value, + ), + { ...condition, value: pubkey }, + ] + : conditions.filter( + (item) => item.field !== field.value, + ), + ) + } + value={condition.value} + /> + ) : field.value === "trigger_message_id" ? ( + collapsePicker(field.value)} + onChange={(messageId) => + updateConditions( + messageId + ? [ + ...conditions.filter( + (item) => item.field !== field.value, + ), + { ...condition, value: messageId }, + ] + : conditions.filter( + (item) => item.field !== field.value, + ), + ) + } + value={condition.value} + /> ) : (
) ) : null} - {existing ? ( + {existing && !fieldUsesFullHeightPicker(field.value) ? (