diff --git a/.github/ISSUE_TEMPLATE/bug-report.md b/.github/ISSUE_TEMPLATE/bug-report.md index e383313452..c951650eb1 100644 --- a/.github/ISSUE_TEMPLATE/bug-report.md +++ b/.github/ISSUE_TEMPLATE/bug-report.md @@ -4,6 +4,10 @@ about: Report a reproducible bug in Buzz labels: bug --- +> [!IMPORTANT] +> Do not include security vulnerabilities in a public issue. [Report them +> privately through a GitHub security advisory](https://github.com/block/buzz/security/advisories/new). + **Describe the bug** A clear and concise description of what the bug is. diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml index 0086358db1..67bfbe0ce4 100644 --- a/.github/ISSUE_TEMPLATE/config.yml +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -1 +1,5 @@ blank_issues_enabled: true +contact_links: + - name: Report a security vulnerability + url: https://github.com/block/buzz/security/advisories/new + about: Report security vulnerabilities privately to the Buzz maintainers. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9e69c9287e..eb07bd1e4b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -87,6 +87,10 @@ jobs: scripts/test-mobile-release-candidate-publisher.sh - name: Mobile worktree identity contract run: scripts/test-mobile-worktree-overrides.sh + - name: Rust cache contract + run: | + scripts/test-rust-cache-contract.sh + scripts/test-rust-cache-contract-regressions.sh - name: File size policy run: just file-size-check @@ -101,7 +105,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 - - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2 + - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2.9.1 with: save-if: ${{ github.event_name != 'pull_request' }} - name: Format check @@ -123,8 +127,9 @@ jobs: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 - uses: rui314/setup-mold@9c9c13bf4c3f1adef0cc596abc155580bcb04444 # v1 - - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2 + - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2.9.1 with: + key: sherpa-cache-v1 save-if: ${{ github.event_name != 'pull_request' }} - name: Install cargo-nextest uses: taiki-e/install-action@0fd46367812ee04360509b4169d9f659d6892bb2 # v2.79.15 @@ -147,7 +152,7 @@ jobs: fetch-depth: 2 - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 - uses: rui314/setup-mold@9c9c13bf4c3f1adef0cc596abc155580bcb04444 # v1 - - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2 + - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2.9.1 with: workspaces: desktop/src-tauri save-if: ${{ github.event_name != 'pull_request' }} @@ -340,7 +345,7 @@ jobs: key: relay-artifacts-${{ runner.os }}-${{ hashFiles('crates/**', 'migrations/**', 'Dockerfile', 'Cargo.toml', 'Cargo.lock', 'rust-toolchain.toml', '.cargo/config.toml', '.github/workflows/ci.yml') }} - uses: rui314/setup-mold@9c9c13bf4c3f1adef0cc596abc155580bcb04444 # v1 if: steps.relay-artifacts-cache.outputs.cache-hit != 'true' - - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2 + - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2.9.1 if: steps.relay-artifacts-cache.outputs.cache-hit != 'true' with: workspaces: | @@ -674,6 +679,19 @@ jobs: VALUES ('00000000-0000-4000-8000-00000000c0de', 'localhost:3000') ON CONFLICT (lower(host)) DO NOTHING ;" + - name: Replaceable persistence PostgreSQL tests + # Transaction, concurrency, and mention-index coverage for the + # replaceable-event store seam. These tests require real Postgres and + # are ignored by the infrastructure-free unit-test job. + run: | + filter='package(buzz-db) and test(/tests::(parameterized_|concurrent_parameterized_)/)' + cargo nextest run \ + --archive-file target/ci/backend-integration-tests.tar.zst \ + -E "${filter}" \ + --run-ignored ignored-only + env: + DATABASE_URL: postgres://buzz:${{ env.BUZZ_TEST_POSTGRES_PASSWORD }}@localhost:5432/buzz + TEST_DATABASE_URL: postgres://buzz:${{ env.BUZZ_TEST_POSTGRES_PASSWORD }}@localhost:5432/buzz - name: Start relay run: | chmod +x ./target/ci/buzz-relay @@ -775,7 +793,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 - - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2 + - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2.9.1 with: save-if: ${{ github.event_name != 'pull_request' }} # Reuse the relay + git-credential-nostr built by Desktop E2E Relay @@ -964,7 +982,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 - - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2 + - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2.9.1 with: key: cross-${{ matrix.target }} save-if: ${{ github.event_name != 'pull_request' }} @@ -1005,7 +1023,7 @@ jobs: # toolchain (1.95.0 + clippy via profile = default) comes from the # repo-root rust-toolchain.toml, which the runner's preinstalled rustup # honors on demand; the host triple already is x86_64-pc-windows-msvc. - - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2 + - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2.9.1 with: workspaces: | . @@ -1082,7 +1100,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 - - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2 + - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2.9.1 with: workspaces: desktop/src-tauri save-if: ${{ github.event_name != 'pull_request' }} diff --git a/.github/workflows/mesh-lifecycle.yml b/.github/workflows/mesh-lifecycle.yml index b5f9660dfe..c699439980 100644 --- a/.github/workflows/mesh-lifecycle.yml +++ b/.github/workflows/mesh-lifecycle.yml @@ -55,7 +55,7 @@ jobs: - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 - - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2 + - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2.9.1 with: save-if: ${{ github.event_name != 'pull_request' }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index cc8147515a..187604fcf4 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -511,7 +511,7 @@ jobs: - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 - - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2 + - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2.9.1 with: workspaces: desktop/src-tauri lookup-only: true diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index db0aea637f..0247b570a2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -6,6 +6,10 @@ get from zero to a merged pull request. If you have questions that aren't answered here, [open an issue](https://github.com/block/buzz/issues/new). +If you believe you found a security vulnerability, do not open a public issue. +Follow our [security policy](SECURITY.md) to submit a private vulnerability +report instead. + --- ## Table of Contents diff --git a/SECURITY.md b/SECURITY.md index 45202b10fb..add36efee4 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -2,16 +2,25 @@ ## Reporting a Vulnerability -**Please do not report security vulnerabilities through public GitHub issues.** +**Do not report security vulnerabilities through a public GitHub issue, pull +request, discussion, or other public channel.** -If you discover a security vulnerability in Buzz, please report it by emailing -**buzz@block.xyz**. Include as much detail as possible: +Use GitHub's private vulnerability reporting form instead: + +**[Report a vulnerability privately](https://github.com/block/buzz/security/advisories/new)** + +Submitting the form starts a private security advisory with the Buzz +maintainers. Use that advisory for vulnerability details, follow-up questions, +and coordinated remediation. Include as much detail as possible: - A description of the vulnerability and its potential impact - Steps to reproduce or a proof-of-concept (if available) - The affected version(s) or commit range - Any suggested mitigations you've identified +If GitHub's private reporting form is unavailable to you, email +**buzz@block.xyz** and do not include vulnerability details in a public issue. + You will receive an acknowledgment within **48 hours**. We aim to provide a full response — including a timeline for a fix — within **7 days** of initial contact. We'll keep you informed as we work toward a resolution. @@ -121,4 +130,6 @@ We use `cargo audit` in CI to scan for known vulnerabilities in dependencies. ## Disclosure Policy We follow [coordinated disclosure](https://en.wikipedia.org/wiki/Coordinated_vulnerability_disclosure). -Reporters will be credited unless they request anonymity. +We use the private advisory to coordinate validation, remediation, and +disclosure with the reporter. Reporters will be credited unless they request +anonymity. diff --git a/crates/buzz-db/src/channel.rs b/crates/buzz-db/src/channel.rs index 109a9367d7..a1890adb56 100644 --- a/crates/buzz-db/src/channel.rs +++ b/crates/buzz-db/src/channel.rs @@ -625,7 +625,7 @@ pub async fn lock_member_snapshot( // this key before INSERT; migration 0032 then takes the membership key in // the INSERT trigger. Taking both in that order avoids mixed-version // duplicate heads without introducing a lock-order inversion. - let replacement_lock = crate::event_replacement_lock_key( + let replacement_lock = crate::replaceable::event_replacement_lock_key( community_id, 39002, relay_pubkey, diff --git a/crates/buzz-db/src/event.rs b/crates/buzz-db/src/event.rs index 5d682d7843..dcfc97c844 100644 --- a/crates/buzz-db/src/event.rs +++ b/crates/buzz-db/src/event.rs @@ -6,7 +6,7 @@ use chrono::{DateTime, Utc}; use nostr::Event; -use sqlx::{PgPool, Postgres, QueryBuilder, Row, Transaction}; +use sqlx::{PgConnection, PgPool, Postgres, QueryBuilder, Row, Transaction}; use uuid::Uuid; use buzz_core::kind::{ @@ -275,6 +275,30 @@ pub async fn insert_event( community_id: CommunityId, event: &Event, channel_id: Option, +) -> Result<(StoredEvent, bool)> { + let mut connection = pool.acquire().await?; + insert_event_on(&mut connection, community_id, event, channel_id).await +} + +/// Insert a Nostr event in a caller-owned PostgreSQL transaction. +/// +/// This is the transaction-composition seam for callers that must keep the +/// event insert open while performing related work. The caller owns commit or +/// rollback. +pub async fn insert_event_in_transaction( + tx: &mut Transaction<'_, Postgres>, + community_id: CommunityId, + event: &Event, + channel_id: Option, +) -> Result<(StoredEvent, bool)> { + insert_event_on(tx.as_mut(), community_id, event, channel_id).await +} + +async fn insert_event_on( + connection: &mut PgConnection, + community_id: CommunityId, + event: &Event, + channel_id: Option, ) -> Result<(StoredEvent, bool)> { let kind_u16 = event.kind.as_u16(); let kind_u32 = u32::from(kind_u16); @@ -317,7 +341,7 @@ pub async fn insert_event( .bind(channel_id) .bind(d_tag.as_deref()) .bind(not_before) - .execute(pool) + .execute(connection) .await?; let was_inserted = result.rows_affected() > 0; @@ -1607,6 +1631,31 @@ mod tests { id } + #[tokio::test] + #[ignore = "requires Postgres"] + async fn event_insert_in_existing_transaction_rolls_back_with_caller() { + let pool = setup_pool().await; + let community_uuid = make_test_community(&pool).await; + let community = CommunityId::from_uuid(community_uuid); + let event = make_text_event("caller-owned transaction"); + + let mut tx = pool.begin().await.expect("begin event insert transaction"); + let (_, was_inserted) = insert_event_in_transaction(&mut tx, community, &event, None) + .await + .expect("insert event in caller transaction"); + assert!(was_inserted); + tx.rollback().await.expect("roll back event insert"); + + let persisted: i64 = + sqlx::query_scalar("SELECT count(*) FROM events WHERE community_id = $1 AND id = $2") + .bind(community_uuid) + .bind(event.id.as_bytes().as_slice()) + .fetch_one(&pool) + .await + .expect("count rolled-back event"); + assert_eq!(persisted, 0); + } + #[tokio::test] #[ignore = "requires Postgres"] async fn event_insert_ttl_trigger_handles_permanent_ephemeral_duplicate_and_activation_race() { diff --git a/crates/buzz-db/src/lib.rs b/crates/buzz-db/src/lib.rs index 98b802696f..e4cc14c3f4 100644 --- a/crates/buzz-db/src/lib.rs +++ b/crates/buzz-db/src/lib.rs @@ -8,6 +8,21 @@ //! - Events table is partitioned by month on `created_at`. //! - No FK references to partitioned tables. //! - Uses `sqlx::query()` (runtime) not `sqlx::query!()` (compile-time). +//! +//! ## Runtime and store ownership +//! This crate intentionally keeps database runtime and Buzz domain persistence +//! together while maintaining an internal boundary between them: +//! +//! - Runtime concerns own pool construction, writer/replica routing, transaction +//! creation, session invariants, metrics, and health support. +//! - Store concerns own domain-specific SQL, row mapping, locking, mutation +//! rules, indexes, and focused persistence tests. +//! +//! Transaction-required store operations accept [`sqlx::Transaction`] so their +//! composition requirement is visible in the type. Private connection helpers +//! are reserved for SQL primitives that are valid on any same-session +//! connection. New domains should prove this boundary incrementally instead of +//! exposing raw pools or introducing broad store traits. /// Explicit deployment-global admin report reads. pub mod admin_moderation; @@ -45,6 +60,8 @@ pub mod reaction; pub mod relay_invite; /// Relay-level membership persistence (NIP-43). pub mod relay_members; +/// Replaceable-event persistence and coordinate locking. +pub mod replaceable; /// Replica freshness fence for keyset-cursor read routing. pub mod replica_fence; /// Thread metadata persistence. @@ -68,37 +85,12 @@ use uuid::Uuid; use buzz_core::{CommunityId, StoredEvent}; -pub(crate) fn event_replacement_lock_key( - community_id: CommunityId, - kind: i32, - pubkey: &[u8], - coordinate: Option<&[u8]>, -) -> i64 { - let mut hash: u64 = 0xcbf29ce484222325; - let kind_bytes = kind.to_le_bytes(); - for bytes in [ - community_id.as_uuid().as_bytes().as_slice(), - kind_bytes.as_slice(), - pubkey, - ] { - for byte in bytes { - hash ^= *byte as u64; - hash = hash.wrapping_mul(0x100000001b3); - } - } - if let Some(coordinate) = coordinate { - for byte in coordinate { - hash ^= *byte as u64; - hash = hash.wrapping_mul(0x100000001b3); - } - } - hash as i64 -} - /// Extract p-tag mentions from an event and insert into the `event_mentions` table. /// -/// Called after event insertion. Failures are logged but do not block event storage. -/// Uses `INSERT ... ON CONFLICT DO NOTHING` so duplicate inserts are silently skipped. +/// This pool-owning wrapper propagates failures to its caller. Replacement writes +/// use the transaction-bound helper below so event storage and mention indexing +/// commit or roll back together. Duplicate inserts are silently skipped with +/// `INSERT ... ON CONFLICT DO NOTHING`. pub async fn insert_mentions( pool: &PgPool, community_id: CommunityId, @@ -4896,7 +4888,7 @@ impl Db { .ok_or(DbError::InvalidTimestamp(created_at_secs))?; // Collisions only cause extra serialization; they cannot change behavior. - let lock_key = event_replacement_lock_key( + let lock_key = replaceable::event_replacement_lock_key( community_id, kind_i32, pubkey_bytes.as_slice(), @@ -5079,8 +5071,12 @@ impl Db { let kind_i32 = buzz_core::kind::KIND_NIP43_MEMBERSHIP_LIST as i32; let pubkey_bytes = relay_keypair.public_key().to_bytes(); - let lock_key = - event_replacement_lock_key(community_id, kind_i32, pubkey_bytes.as_slice(), None); + let lock_key = replaceable::event_replacement_lock_key( + community_id, + kind_i32, + pubkey_bytes.as_slice(), + None, + ); let mut tx = self.pool.begin().await?; @@ -5187,243 +5183,6 @@ impl Db { member_count, )) } - - /// Atomically replace a NIP-33 parameterized replaceable event (kind 30000–39999). - /// - /// Keeps only the event with the highest `created_at` per `(kind, pubkey, d_tag)`. - /// Same-second ties are broken by lowest event `id` (deterministic ordering). - /// The entire check → retire old payload → insert runs in a single transaction - /// with an advisory lock to prevent concurrent-insert races. NIP-RS read-state - /// coordinates hard-delete the superseded payload and preserve a compact - /// ordering watermark. Buzz mesh status coordinates also hard-delete their - /// superseded heartbeat payload because only the live head has product - /// value; other NIP-33 kinds retain soft-deleted history. - /// - /// **Channel policy:** NIP-33 replacement keys on `(kind, pubkey, d_tag)` globally — - /// `channel_id` is NOT part of the replacement key. This matches the Nostr spec: - /// an author's parameterized replaceable event is a single global resource identified - /// by its d-tag, regardless of which channel it was submitted to. The `channel_id` - /// parameter is stored on the new row for query scoping but does not affect replacement. - /// - /// Note: `replace_addressable_event()` keys on `channel_id` because it serves - /// relay-signed NIP-29 group metadata (kind 39000–39002) where the relay is the - /// author and channel_id distinguishes groups. User-submitted NIP-33 events use - /// this function instead, where the author's pubkey + d-tag is the natural key. - #[datastore_span(name = "replace_parameterized_event", system = "postgresql")] - pub async fn replace_parameterized_event( - &self, - community_id: CommunityId, - event: &nostr::Event, - d_tag: &str, - channel_id: Option, - ) -> Result<(StoredEvent, bool)> { - let kind_i32 = buzz_core::kind::event_kind_i32(event); - let pubkey_bytes = event.pubkey.to_bytes(); - let created_at_secs = event.created_at.as_secs() as i64; - let created_at = chrono::DateTime::from_timestamp(created_at_secs, 0) - .ok_or(DbError::InvalidTimestamp(created_at_secs))?; - - let lock_key = event_replacement_lock_key( - community_id, - kind_i32, - pubkey_bytes.as_slice(), - Some(d_tag.as_bytes()), - ); - - let mut tx = self.pool.begin().await?; - - sqlx::query("SELECT pg_advisory_xact_lock($1)") - .bind(lock_key) - .execute(&mut *tx) - .await?; - - let d_tag_count = event - .tags - .iter() - .filter(|tag| tag.as_slice().first().is_some_and(|part| part == "d")) - .count(); - let has_exact_d_tag = event.tags.iter().any(|tag| { - let parts = tag.as_slice(); - parts.len() >= 2 && parts[0] == "d" && parts[1] == d_tag - }); - let read_state_t_tag_count = event - .tags - .iter() - .filter(|tag| { - let parts = tag.as_slice(); - parts.len() == 2 && parts[0] == "t" && parts[1] == "read-state" - }) - .count(); - let is_nip_rs = kind_i32 == buzz_core::kind::KIND_READ_STATE as i32 - && d_tag_count == 1 - && has_exact_d_tag - && d_tag.strip_prefix("read-state:").is_some_and(|slot| { - slot.len() == 32 - && slot - .bytes() - .all(|byte| byte.is_ascii_hexdigit() && !byte.is_ascii_uppercase()) - }) - && read_state_t_tag_count == 1; - let is_buzz_mesh_status = kind_i32 == buzz_core::kind::KIND_BOOKMARK_SET as i32 - && d_tag.starts_with("buzz-mesh-member-status:") - && event.tags.iter().any(|tag| { - let parts = tag.as_slice(); - parts.len() == 2 && parts[0] == "k" && parts[1] == "buzz-mesh-status" - }); - let hard_delete_superseded = is_nip_rs || is_buzz_mesh_status; - - // Check the live head and, for NIP-RS, the compact historical ordering - // watermark. The watermark remains after a NIP-09 coordinate deletion, - // preventing a previously accepted signed blob from being resurrected. - let existing: Option<(chrono::DateTime, Vec)> = sqlx::query_as( - "SELECT created_at, id FROM events \ - WHERE community_id = $1 AND kind = $2 AND pubkey = $3 AND d_tag = $4 AND deleted_at IS NULL \ - ORDER BY created_at DESC, id ASC LIMIT 1", - ) - .bind(community_id.as_uuid()) - .bind(kind_i32) - .bind(pubkey_bytes.as_slice()) - .bind(d_tag) - .fetch_optional(&mut *tx) - .await?; - let watermark: Option<(chrono::DateTime, Vec)> = if is_nip_rs { - sqlx::query_as( - "SELECT created_at, event_id FROM parameterized_event_watermarks \ - WHERE community_id = $1 AND kind = $2 AND pubkey = $3 AND d_tag = $4", - ) - .bind(community_id.as_uuid()) - .bind(kind_i32) - .bind(pubkey_bytes.as_slice()) - .bind(d_tag) - .fetch_optional(&mut *tx) - .await? - } else { - None - }; - - // Stale-write protection: reject if either durable ordering source - // dominates the incoming tuple. Equal timestamps use lowest event id. - let incoming_id = event.id.as_bytes().as_slice(); - let dominated = - existing - .iter() - .chain(watermark.iter()) - .any(|(accepted_ts, accepted_id)| { - created_at < *accepted_ts - || (created_at == *accepted_ts && incoming_id >= accepted_id.as_slice()) - }); - if dominated { - tx.rollback().await?; - let received_at = chrono::Utc::now(); - return Ok(( - StoredEvent::with_received_at(event.clone(), received_at, channel_id, false), - false, - )); - } - - if existing.is_some() { - if is_nip_rs { - // Migration 0011 rejects regex-coordinate hard deletes from - // pre-fix writers. Authorize only this corrected NIP-RS delete, - // transaction-locally so pooled connections cannot leak it. - sqlx::query("SELECT set_config('buzz.nip_rs_hard_delete', 'on', true)") - .execute(&mut *tx) - .await?; - } - let statement = if hard_delete_superseded { - "DELETE FROM events \ - WHERE community_id = $1 AND kind = $2 AND pubkey = $3 AND d_tag = $4 AND deleted_at IS NULL" - } else { - "UPDATE events SET deleted_at = NOW() \ - WHERE community_id = $1 AND kind = $2 AND pubkey = $3 AND d_tag = $4 AND deleted_at IS NULL" - }; - sqlx::query(statement) - .bind(community_id.as_uuid()) - .bind(kind_i32) - .bind(pubkey_bytes.as_slice()) - .bind(d_tag) - .execute(&mut *tx) - .await?; - - if hard_delete_superseded { - if let Some((_, existing_id)) = &existing { - // Event first, mentions second: migration 0009's live-event - // fence uses this global lock order to avoid deadlocks. - sqlx::query( - "DELETE FROM event_mentions WHERE community_id = $1 AND event_id = $2", - ) - .bind(community_id.as_uuid()) - .bind(existing_id) - .execute(&mut *tx) - .await?; - } - } - } - - // Insert the new event inside the transaction. - let sig_bytes = event.sig.serialize(); - let tags_json = serde_json::to_value(&event.tags)?; - let received_at = chrono::Utc::now(); - - let insert_result = sqlx::query( - "INSERT INTO events (community_id, id, pubkey, created_at, kind, tags, content, sig, received_at, channel_id, d_tag, not_before) \ - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) \ - ON CONFLICT DO NOTHING", - ) - .bind(community_id.as_uuid()) - .bind(event.id.as_bytes().as_slice()) - .bind(pubkey_bytes.as_slice()) - .bind(created_at) - .bind(kind_i32) - .bind(&tags_json) - .bind(&event.content) - .bind(sig_bytes.as_slice()) - .bind(received_at) - .bind(channel_id) - .bind(d_tag) - .bind(event::extract_not_before(event)) - .execute(&mut *tx) - .await?; - - let was_inserted = insert_result.rows_affected() > 0; - if !was_inserted { - tx.rollback().await?; - return Ok(( - StoredEvent::with_received_at(event.clone(), received_at, channel_id, false), - false, - )); - } - - if is_nip_rs { - sqlx::query( - "INSERT INTO parameterized_event_watermarks \ - (community_id, kind, pubkey, d_tag, created_at, event_id) \ - VALUES ($1, $2, $3, $4, $5, $6) \ - ON CONFLICT (community_id, kind, pubkey, d_tag) DO UPDATE SET \ - created_at = EXCLUDED.created_at, event_id = EXCLUDED.event_id", - ) - .bind(community_id.as_uuid()) - .bind(kind_i32) - .bind(pubkey_bytes.as_slice()) - .bind(d_tag) - .bind(created_at) - .bind(incoming_id) - .execute(&mut *tx) - .await?; - } - - tx.commit().await?; - - // Mentions are a denormalized index — safe outside the transaction. - if let Err(e) = crate::insert_mentions(&self.pool, community_id, event, channel_id).await { - tracing::warn!(event_id = %event.id, "Failed to insert mentions: {e}"); - } - - Ok(( - StoredEvent::with_received_at(event.clone(), received_at, channel_id, true), - true, - )) - } } /// A full API token record. @@ -6035,6 +5794,456 @@ mod tests { assert_eq!(live, 0, "watermark must block stale resurrection"); } + #[tokio::test] + #[ignore = "requires Postgres"] + async fn nip_rs_transaction_operation_restores_hard_delete_opt_in() { + use nostr::{EventBuilder, Keys, Kind, Tag, Timestamp}; + + let db = setup_db().await; + let community = CommunityId::from_uuid(make_community(&db.pool).await); + let keys = Keys::generate(); + let base = Timestamp::now().as_secs(); + let replace_d_tag = format!("read-state:{}", "b".repeat(32)); + let victim_d_tag = format!("read-state:{}", "c".repeat(32)); + let event = |d_tag: &str, content: &str, timestamp: u64| { + EventBuilder::new( + Kind::Custom(buzz_core::kind::KIND_READ_STATE as u16), + content, + ) + .tags(vec![ + Tag::parse(["d", d_tag]).expect("d tag"), + Tag::parse(["t", "read-state"]).expect("t tag"), + ]) + .custom_created_at(Timestamp::from(timestamp)) + .sign_with_keys(&keys) + .expect("sign read state") + }; + let old = event(&replace_d_tag, "old", base); + let new = event(&replace_d_tag, "new", base + 1); + let victim = event(&victim_d_tag, "victim", base); + + assert!( + db.replace_parameterized_event(community, &old, &replace_d_tag, None) + .await + .expect("insert old head") + .1 + ); + assert!( + db.replace_parameterized_event(community, &victim, &victim_d_tag, None) + .await + .expect("insert victim head") + .1 + ); + + let mut tx = db + .begin_transaction() + .await + .expect("begin caller transaction"); + let result = db + .replace_parameterized_event_in_transaction( + &mut tx, + community, + &new, + &replace_d_tag, + None, + replaceable::ParameterizedReplacePrecondition::Unconditional, + ) + .await + .expect("replace inside caller transaction"); + assert_eq!( + result.status, + replaceable::ParameterizedReplaceStatus::Inserted + ); + + let leaked: Option = sqlx::query_scalar( + "SELECT NULLIF(current_setting('buzz.nip_rs_hard_delete', true), '')", + ) + .fetch_one(&mut *tx) + .await + .expect("read hard-delete opt-in after replacement"); + assert_ne!(leaked.as_deref(), Some("on")); + + let unauthorized = sqlx::query( + "DELETE FROM events WHERE community_id=$1 AND kind=30078 \ + AND pubkey=$2 AND d_tag=$3 AND deleted_at IS NULL", + ) + .bind(community.as_uuid()) + .bind(keys.public_key().to_bytes()) + .bind(&victim_d_tag) + .execute(&mut *tx) + .await; + assert!( + unauthorized.is_err(), + "replacement opt-in must not authorize later caller SQL" + ); + tx.rollback().await.expect("roll back caller transaction"); + } + + #[tokio::test] + #[ignore = "requires Postgres"] + async fn parameterized_replacement_in_existing_transaction_honors_revision_and_rollback() { + use nostr::{EventBuilder, Keys, Kind, Tag, Timestamp}; + + let db = setup_db().await; + let community = CommunityId::from_uuid(make_community(&db.pool).await); + let keys = Keys::generate(); + let d_tag = format!("transactional-project-{}", Uuid::new_v4().simple()); + let base = Timestamp::now().as_secs(); + let event = |content: &str, timestamp: u64| { + EventBuilder::new(Kind::Custom(buzz_core::kind::KIND_PROJECT as u16), content) + .tags(vec![Tag::parse(["d", d_tag.as_str()]).expect("d tag")]) + .custom_created_at(Timestamp::from(timestamp)) + .sign_with_keys(&keys) + .expect("sign project") + }; + let old = event("old", base); + let new = event("new", base + 1); + + assert!( + db.replace_parameterized_event(community, &old, &d_tag, None) + .await + .expect("insert old head") + .1 + ); + + let mut tx = db.begin_transaction().await.expect("begin replacement tx"); + let outcome = db + .replace_parameterized_event_in_transaction( + &mut tx, + community, + &new, + &d_tag, + None, + replaceable::ParameterizedReplacePrecondition::ExpectedRevision( + old.id.as_bytes().as_slice(), + ), + ) + .await + .expect("replace inside caller transaction"); + assert_eq!( + outcome.status, + replaceable::ParameterizedReplaceStatus::Inserted + ); + tx.rollback().await.expect("roll back replacement tx"); + + let live_id: Vec = sqlx::query_scalar( + "SELECT id FROM events WHERE community_id=$1 AND kind=$2 AND pubkey=$3 \ + AND d_tag=$4 AND deleted_at IS NULL", + ) + .bind(community.as_uuid()) + .bind(buzz_core::kind::KIND_PROJECT as i32) + .bind(keys.public_key().to_bytes()) + .bind(&d_tag) + .fetch_one(&db.pool) + .await + .expect("load live head after rollback"); + assert_eq!(live_id, old.id.as_bytes().to_vec()); + + let mut tx = db + .begin_transaction() + .await + .expect("begin stale revision tx"); + let mismatch = db + .replace_parameterized_event_in_transaction( + &mut tx, + community, + &new, + &d_tag, + None, + replaceable::ParameterizedReplacePrecondition::ExpectedRevision( + [0x42; 32].as_slice(), + ), + ) + .await + .expect("evaluate stale revision"); + assert_eq!( + mismatch.status, + replaceable::ParameterizedReplaceStatus::RevisionMismatch + ); + tx.rollback().await.expect("roll back stale revision tx"); + + let missing_d_tag = format!("missing-project-{}", Uuid::new_v4().simple()); + let missing = EventBuilder::new( + Kind::Custom(buzz_core::kind::KIND_PROJECT as u16), + "missing", + ) + .tags(vec![ + Tag::parse(["d", missing_d_tag.as_str()]).expect("missing d tag") + ]) + .custom_created_at(Timestamp::from(base + 2)) + .sign_with_keys(&keys) + .expect("sign missing project"); + let mut tx = db + .begin_transaction() + .await + .expect("begin missing revision tx"); + let missing_result = db + .replace_parameterized_event_in_transaction( + &mut tx, + community, + &missing, + &missing_d_tag, + None, + replaceable::ParameterizedReplacePrecondition::ExpectedRevision( + [0x24; 32].as_slice(), + ), + ) + .await + .expect("evaluate missing revision"); + assert_eq!( + missing_result.status, + replaceable::ParameterizedReplaceStatus::RevisionMissing + ); + tx.rollback().await.expect("roll back missing revision tx"); + } + + #[tokio::test] + #[ignore = "requires Postgres"] + async fn parameterized_replacement_rolls_back_when_mention_indexing_fails() { + use nostr::{EventBuilder, Keys, Kind, Tag, Timestamp}; + + let admin = PgPool::connect(&admin_url().await) + .await + .expect("connect admin"); + let (pool, scratch_name) = create_scratch_db(&admin, "atomic_parameterized").await; + let db = Db::from_pool(pool.clone()); + let community = CommunityId::from_uuid(make_community(&pool).await); + let keys = Keys::generate(); + let mentioned = Keys::generate().public_key().to_hex(); + let d_tag = format!("mention-project-{}", Uuid::new_v4().simple()); + let tags = || { + vec![ + Tag::parse(["d", d_tag.as_str()]).expect("d tag"), + Tag::parse(["p", mentioned.as_str()]).expect("p tag"), + ] + }; + let base = Timestamp::now().as_secs(); + let old = EventBuilder::new(Kind::Custom(buzz_core::kind::KIND_PROJECT as u16), "old") + .tags(tags()) + .custom_created_at(Timestamp::from(base)) + .sign_with_keys(&keys) + .expect("sign old project"); + let new = EventBuilder::new(Kind::Custom(buzz_core::kind::KIND_PROJECT as u16), "new") + .tags(tags()) + .custom_created_at(Timestamp::from(base + 1)) + .sign_with_keys(&keys) + .expect("sign new project"); + + assert!( + db.replace_parameterized_event(community, &old, &d_tag, None) + .await + .expect("insert old project") + .1 + ); + sqlx::query( + "CREATE FUNCTION reject_test_mention() RETURNS trigger AS $$ \ + BEGIN RAISE EXCEPTION 'injected mention failure'; END; \ + $$ LANGUAGE plpgsql", + ) + .execute(&pool) + .await + .expect("create failure function"); + sqlx::query( + "CREATE TRIGGER reject_test_mention BEFORE INSERT ON event_mentions \ + FOR EACH ROW EXECUTE FUNCTION reject_test_mention()", + ) + .execute(&pool) + .await + .expect("install failure injection"); + + let mut tx = db + .begin_transaction() + .await + .expect("begin caller transaction"); + let error = db + .replace_parameterized_event_in_transaction( + &mut tx, + community, + &new, + &d_tag, + None, + replaceable::ParameterizedReplacePrecondition::Unconditional, + ) + .await + .expect_err("mention failure must fail replacement"); + assert!(error.to_string().contains("injected mention failure")); + + let probe: i32 = sqlx::query_scalar("SELECT 1") + .fetch_one(&mut *tx) + .await + .expect("inner failure must leave caller transaction usable"); + assert_eq!(probe, 1); + + let live_id: Vec = sqlx::query_scalar( + "SELECT id FROM events WHERE community_id=$1 AND kind=$2 AND pubkey=$3 \ + AND d_tag=$4 AND deleted_at IS NULL", + ) + .bind(community.as_uuid()) + .bind(buzz_core::kind::KIND_PROJECT as i32) + .bind(keys.public_key().to_bytes()) + .bind(&d_tag) + .fetch_one(&mut *tx) + .await + .expect("load live project after failed indexing"); + assert_eq!(live_id, old.id.as_bytes().to_vec()); + let new_rows: i64 = + sqlx::query_scalar("SELECT count(*) FROM events WHERE community_id=$1 AND id=$2") + .bind(community.as_uuid()) + .bind(new.id.as_bytes().as_slice()) + .fetch_one(&mut *tx) + .await + .expect("count rolled-back project"); + assert_eq!(new_rows, 0); + tx.commit().await.expect("commit usable caller transaction"); + + drop_scratch_db(&admin, pool, &scratch_name).await; + } + + #[tokio::test] + #[ignore = "requires Postgres"] + async fn parameterized_duplicate_restores_live_head_inside_caller_transaction() { + use nostr::{EventBuilder, Keys, Kind, Tag, Timestamp}; + + let db = setup_db().await; + let community = CommunityId::from_uuid(make_community(&db.pool).await); + let keys = Keys::generate(); + let d_tag = format!("duplicate-project-{}", Uuid::new_v4().simple()); + let base = Timestamp::now().as_secs(); + let event = |content: &str, timestamp: u64| { + EventBuilder::new(Kind::Custom(buzz_core::kind::KIND_PROJECT as u16), content) + .tags(vec![Tag::parse(["d", d_tag.as_str()]).expect("d tag")]) + .custom_created_at(Timestamp::from(timestamp)) + .sign_with_keys(&keys) + .expect("sign project") + }; + let old = event("old-live-head", base); + let duplicate = event("soft-deleted-duplicate", base + 1); + + assert!( + db.replace_parameterized_event(community, &duplicate, &d_tag, None) + .await + .expect("insert future duplicate") + .1 + ); + sqlx::query("UPDATE events SET deleted_at=NOW() WHERE community_id=$1 AND id=$2") + .bind(community.as_uuid()) + .bind(duplicate.id.as_bytes().as_slice()) + .execute(&db.pool) + .await + .expect("soft-delete duplicate row"); + + let mut seed_tx = db + .begin_transaction() + .await + .expect("begin seed transaction"); + let (_, was_inserted) = + event::insert_event_in_transaction(&mut seed_tx, community, &old, None) + .await + .expect("insert older live head"); + assert!(was_inserted); + seed_tx.commit().await.expect("commit older live head"); + + let mut tx = db + .begin_transaction() + .await + .expect("begin caller transaction"); + let result = db + .replace_parameterized_event_in_transaction( + &mut tx, + community, + &duplicate, + &d_tag, + None, + replaceable::ParameterizedReplacePrecondition::Unconditional, + ) + .await + .expect("evaluate soft-deleted duplicate"); + assert_eq!( + result.status, + replaceable::ParameterizedReplaceStatus::Duplicate + ); + + let live_id: Vec = sqlx::query_scalar( + "SELECT id FROM events WHERE community_id=$1 AND kind=$2 AND pubkey=$3 \ + AND d_tag=$4 AND deleted_at IS NULL", + ) + .bind(community.as_uuid()) + .bind(buzz_core::kind::KIND_PROJECT as i32) + .bind(keys.public_key().to_bytes()) + .bind(&d_tag) + .fetch_one(&mut *tx) + .await + .expect("caller transaction remains usable after duplicate"); + assert_eq!(live_id, old.id.as_bytes().to_vec()); + tx.rollback().await.expect("roll back caller transaction"); + } + + #[tokio::test] + #[ignore = "requires Postgres"] + async fn concurrent_parameterized_replacement_keeps_deterministic_head() { + use nostr::{EventBuilder, Keys, Kind, Tag, Timestamp}; + + let db = setup_db().await; + let community = CommunityId::from_uuid(make_community(&db.pool).await); + let keys = Keys::generate(); + let d_tag = format!("concurrent-project-{}", Uuid::new_v4().simple()); + let created_at = Timestamp::now().as_secs(); + let event = |content: &str, timestamp: u64| { + EventBuilder::new(Kind::Custom(buzz_core::kind::KIND_PROJECT as u16), content) + .tags(vec![Tag::parse(["d", d_tag.as_str()]).expect("d tag")]) + .custom_created_at(Timestamp::from(timestamp)) + .sign_with_keys(&keys) + .expect("sign project") + }; + let first = event("first", created_at); + let second = event("second", created_at); + let expected = if first.id.as_bytes() < second.id.as_bytes() { + &first + } else { + &second + }; + + let (first_result, second_result) = tokio::join!( + db.replace_parameterized_event(community, &first, &d_tag, None), + db.replace_parameterized_event(community, &second, &d_tag, None), + ); + let first_inserted = first_result.expect("first concurrent writer").1; + let second_inserted = second_result.expect("second concurrent writer").1; + assert!( + first_inserted || second_inserted, + "at least one concurrent writer must insert", + ); + + let live_ids: Vec> = sqlx::query_scalar( + "SELECT id FROM events WHERE community_id=$1 AND kind=$2 AND pubkey=$3 \ + AND d_tag=$4 AND deleted_at IS NULL", + ) + .bind(community.as_uuid()) + .bind(buzz_core::kind::KIND_PROJECT as i32) + .bind(keys.public_key().to_bytes()) + .bind(&d_tag) + .fetch_all(&db.pool) + .await + .expect("load concurrent live head"); + assert_eq!(live_ids, vec![expected.id.as_bytes().to_vec()]); + + assert!( + !db.replace_parameterized_event(community, expected, &d_tag, None) + .await + .expect("replay winning event") + .1, + "replaying the live event must be idempotent", + ); + let stale = event("stale", created_at.saturating_sub(1)); + assert!( + !db.replace_parameterized_event(community, &stale, &d_tag, None) + .await + .expect("submit stale event") + .1, + "an older event must not replace the live head", + ); + } + #[tokio::test] #[ignore = "requires Postgres"] async fn mesh_status_replacement_keeps_one_physical_row() { diff --git a/crates/buzz-db/src/replaceable.rs b/crates/buzz-db/src/replaceable.rs new file mode 100644 index 0000000000..aa57a51666 --- /dev/null +++ b/crates/buzz-db/src/replaceable.rs @@ -0,0 +1,417 @@ +//! Replaceable-event persistence and coordinate locking. + +use buzz_core::{CommunityId, StoredEvent}; +use buzz_datastore_tracing::datastore_span; +use chrono::{DateTime, Utc}; +use sqlx::{Acquire, Postgres, Transaction}; +use uuid::Uuid; + +use crate::{Db, DbError, Result}; + +/// Result category for a parameterized-replaceable event write. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum ParameterizedReplaceStatus { + /// The incoming event was inserted as the coordinate's live head. + Inserted, + /// The exact event was already accepted. + Duplicate, + /// A newer event, or lower-ID same-second event, already dominates it. + Superseded, + /// A requested current revision has no live coordinate head. + RevisionMissing, + /// The live coordinate head differs from the requested revision. + RevisionMismatch, + /// An exact replay was required, but the event is not the live head. + ReplayOnlyMiss, +} + +/// Structural precondition for a parameterized-replaceable write. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum ParameterizedReplacePrecondition<'a> { + /// Apply normal NIP-33 ordering without a revision precondition. + Unconditional, + /// Require the live head to match this validated event ID. + ExpectedRevision(&'a [u8]), + /// Accept only an exact live-head replay and perform no mutation otherwise. + ExactReplayOnly, +} + +/// Result of a transaction-bound parameterized-replaceable event write. +#[derive(Clone, Debug)] +pub struct ParameterizedReplaceResult { + /// Stored representation of the submitted event. + pub event: StoredEvent, + /// Whether and why the coordinate accepted the event. + pub status: ParameterizedReplaceStatus, +} + +impl ParameterizedReplaceResult { + fn new( + event: &nostr::Event, + received_at: DateTime, + channel_id: Option, + status: ParameterizedReplaceStatus, + ) -> Self { + Self { + event: StoredEvent::with_received_at( + event.clone(), + received_at, + channel_id, + status == ParameterizedReplaceStatus::Inserted, + ), + status, + } + } +} + +/// Derive the transaction-scoped advisory-lock key for an event coordinate. +/// +/// Hash collisions only add serialization; the SQL predicates still determine +/// which rows are read or changed. +pub(crate) fn event_replacement_lock_key( + community_id: CommunityId, + kind: i32, + pubkey: &[u8], + coordinate: Option<&[u8]>, +) -> i64 { + let mut hash: u64 = 0xcbf29ce484222325; + let kind_bytes = kind.to_le_bytes(); + for bytes in [ + community_id.as_uuid().as_bytes().as_slice(), + kind_bytes.as_slice(), + pubkey, + ] { + for byte in bytes { + hash ^= *byte as u64; + hash = hash.wrapping_mul(0x100000001b3); + } + } + if let Some(coordinate) = coordinate { + for byte in coordinate { + hash ^= *byte as u64; + hash = hash.wrapping_mul(0x100000001b3); + } + } + hash as i64 +} + +/// Replace a parameterized event in a caller-owned transaction. +/// +/// This function acquires a transaction-scoped advisory lock but never commits +/// or rolls back the outer transaction. The typed precondition can require the +/// current live head to have an exact event ID or restrict the operation to an +/// idempotent replay. +async fn replace_parameterized_event_in_transaction_impl( + tx: &mut Transaction<'_, Postgres>, + community_id: CommunityId, + event: &nostr::Event, + d_tag: &str, + channel_id: Option, + precondition: ParameterizedReplacePrecondition<'_>, +) -> Result { + let kind_i32 = buzz_core::kind::event_kind_i32(event); + let pubkey_bytes = event.pubkey.to_bytes(); + let created_at_secs = event.created_at.as_secs() as i64; + let created_at = DateTime::from_timestamp(created_at_secs, 0) + .ok_or(DbError::InvalidTimestamp(created_at_secs))?; + let received_at = Utc::now(); + + let lock_key = event_replacement_lock_key( + community_id, + kind_i32, + pubkey_bytes.as_slice(), + Some(d_tag.as_bytes()), + ); + sqlx::query("SELECT pg_advisory_xact_lock($1)") + .bind(lock_key) + .execute(&mut **tx) + .await?; + + let d_tag_count = event + .tags + .iter() + .filter(|tag| tag.as_slice().first().is_some_and(|part| part == "d")) + .count(); + let has_exact_d_tag = event.tags.iter().any(|tag| { + let parts = tag.as_slice(); + parts.len() >= 2 && parts[0] == "d" && parts[1] == d_tag + }); + let read_state_t_tag_count = event + .tags + .iter() + .filter(|tag| { + let parts = tag.as_slice(); + parts.len() == 2 && parts[0] == "t" && parts[1] == "read-state" + }) + .count(); + let is_nip_rs = kind_i32 == buzz_core::kind::KIND_READ_STATE as i32 + && d_tag_count == 1 + && has_exact_d_tag + && d_tag.strip_prefix("read-state:").is_some_and(|slot| { + slot.len() == 32 + && slot + .bytes() + .all(|byte| byte.is_ascii_hexdigit() && !byte.is_ascii_uppercase()) + }) + && read_state_t_tag_count == 1; + let is_buzz_mesh_status = kind_i32 == buzz_core::kind::KIND_BOOKMARK_SET as i32 + && d_tag.starts_with("buzz-mesh-member-status:") + && event.tags.iter().any(|tag| { + let parts = tag.as_slice(); + parts.len() == 2 && parts[0] == "k" && parts[1] == "buzz-mesh-status" + }); + let hard_delete_superseded = is_nip_rs || is_buzz_mesh_status; + + let existing: Option<(DateTime, Vec)> = sqlx::query_as( + "SELECT created_at, id FROM events \ + WHERE community_id = $1 AND kind = $2 AND pubkey = $3 AND d_tag = $4 AND deleted_at IS NULL \ + ORDER BY created_at DESC, id ASC LIMIT 1", + ) + .bind(community_id.as_uuid()) + .bind(kind_i32) + .bind(pubkey_bytes.as_slice()) + .bind(d_tag) + .fetch_optional(&mut **tx) + .await?; + let watermark: Option<(DateTime, Vec)> = if is_nip_rs { + sqlx::query_as( + "SELECT created_at, event_id FROM parameterized_event_watermarks \ + WHERE community_id = $1 AND kind = $2 AND pubkey = $3 AND d_tag = $4", + ) + .bind(community_id.as_uuid()) + .bind(kind_i32) + .bind(pubkey_bytes.as_slice()) + .bind(d_tag) + .fetch_optional(&mut **tx) + .await? + } else { + None + }; + + let incoming_id = event.id.as_bytes().as_slice(); + if existing + .as_ref() + .is_some_and(|(_, existing_id)| existing_id.as_slice() == incoming_id) + || watermark + .as_ref() + .is_some_and(|(_, event_id)| event_id.as_slice() == incoming_id) + { + return Ok(ParameterizedReplaceResult::new( + event, + received_at, + channel_id, + ParameterizedReplaceStatus::Duplicate, + )); + } + + if precondition == ParameterizedReplacePrecondition::ExactReplayOnly { + return Ok(ParameterizedReplaceResult::new( + event, + received_at, + channel_id, + ParameterizedReplaceStatus::ReplayOnlyMiss, + )); + } + + if let ParameterizedReplacePrecondition::ExpectedRevision(expected_revision) = precondition { + let status = match existing.as_ref() { + None => Some(ParameterizedReplaceStatus::RevisionMissing), + Some((_, existing_id)) if existing_id.as_slice() != expected_revision => { + Some(ParameterizedReplaceStatus::RevisionMismatch) + } + Some(_) => None, + }; + if let Some(status) = status { + return Ok(ParameterizedReplaceResult::new( + event, + received_at, + channel_id, + status, + )); + } + } + + let dominated = existing + .iter() + .chain(watermark.iter()) + .any(|(accepted_ts, accepted_id)| { + created_at < *accepted_ts + || (created_at == *accepted_ts && incoming_id >= accepted_id.as_slice()) + }); + if dominated { + return Ok(ParameterizedReplaceResult::new( + event, + received_at, + channel_id, + ParameterizedReplaceStatus::Superseded, + )); + } + + let mut savepoint = tx.begin().await?; + if existing.is_some() { + let previous_nip_rs_hard_delete: Option = if is_nip_rs { + sqlx::query_scalar( + "SELECT NULLIF(current_setting('buzz.nip_rs_hard_delete', true), '')", + ) + .fetch_one(&mut *savepoint) + .await? + } else { + None + }; + if is_nip_rs { + sqlx::query("SELECT set_config('buzz.nip_rs_hard_delete', 'on', true)") + .execute(&mut *savepoint) + .await?; + } + let statement = if hard_delete_superseded { + "DELETE FROM events \ + WHERE community_id = $1 AND kind = $2 AND pubkey = $3 AND d_tag = $4 AND deleted_at IS NULL" + } else { + "UPDATE events SET deleted_at = NOW() \ + WHERE community_id = $1 AND kind = $2 AND pubkey = $3 AND d_tag = $4 AND deleted_at IS NULL" + }; + sqlx::query(statement) + .bind(community_id.as_uuid()) + .bind(kind_i32) + .bind(pubkey_bytes.as_slice()) + .bind(d_tag) + .execute(&mut *savepoint) + .await?; + + if is_nip_rs { + let previous_value = previous_nip_rs_hard_delete.as_deref().unwrap_or_default(); + sqlx::query("SELECT set_config('buzz.nip_rs_hard_delete', $1, true)") + .bind(previous_value) + .execute(&mut *savepoint) + .await?; + } + + if hard_delete_superseded { + if let Some((_, existing_id)) = &existing { + sqlx::query("DELETE FROM event_mentions WHERE community_id = $1 AND event_id = $2") + .bind(community_id.as_uuid()) + .bind(existing_id) + .execute(&mut *savepoint) + .await?; + } + } + } + + let sig_bytes = event.sig.serialize(); + let tags_json = serde_json::to_value(&event.tags)?; + let insert_result = sqlx::query( + "INSERT INTO events (community_id, id, pubkey, created_at, kind, tags, content, sig, received_at, channel_id, d_tag, not_before) \ + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) \ + ON CONFLICT DO NOTHING", + ) + .bind(community_id.as_uuid()) + .bind(incoming_id) + .bind(pubkey_bytes.as_slice()) + .bind(created_at) + .bind(kind_i32) + .bind(&tags_json) + .bind(&event.content) + .bind(sig_bytes.as_slice()) + .bind(received_at) + .bind(channel_id) + .bind(d_tag) + .bind(crate::event::extract_not_before(event)) + .execute(&mut *savepoint) + .await?; + + if insert_result.rows_affected() == 0 { + savepoint.rollback().await?; + return Ok(ParameterizedReplaceResult::new( + event, + received_at, + channel_id, + ParameterizedReplaceStatus::Duplicate, + )); + } + + if is_nip_rs { + sqlx::query( + "INSERT INTO parameterized_event_watermarks \ + (community_id, kind, pubkey, d_tag, created_at, event_id) \ + VALUES ($1, $2, $3, $4, $5, $6) \ + ON CONFLICT (community_id, kind, pubkey, d_tag) DO UPDATE SET \ + created_at = EXCLUDED.created_at, event_id = EXCLUDED.event_id", + ) + .bind(community_id.as_uuid()) + .bind(kind_i32) + .bind(pubkey_bytes.as_slice()) + .bind(d_tag) + .bind(created_at) + .bind(incoming_id) + .execute(&mut *savepoint) + .await?; + } + + crate::insert_mentions_in_transaction(&mut savepoint, community_id, event, channel_id).await?; + savepoint.commit().await?; + + Ok(ParameterizedReplaceResult::new( + event, + received_at, + channel_id, + ParameterizedReplaceStatus::Inserted, + )) +} + +impl Db { + /// Replace a NIP-33 event inside a caller-owned transaction. + /// + /// The caller owns commit or rollback. Requiring [`Transaction`] here and + /// in the internal state machine makes the advisory-lock contract explicit. + pub async fn replace_parameterized_event_in_transaction( + &self, + tx: &mut Transaction<'_, Postgres>, + community_id: CommunityId, + event: &nostr::Event, + d_tag: &str, + channel_id: Option, + precondition: ParameterizedReplacePrecondition<'_>, + ) -> Result { + replace_parameterized_event_in_transaction_impl( + tx, + community_id, + event, + d_tag, + channel_id, + precondition, + ) + .await + } + + /// Atomically replace a NIP-33 parameterized replaceable event. + /// + /// Replacement keys on `(kind, pubkey, d_tag)` across channels. The + /// highest timestamp wins; same-second ties use the lowest event ID. + #[datastore_span(name = "replace_parameterized_event", system = "postgresql")] + pub async fn replace_parameterized_event( + &self, + community_id: CommunityId, + event: &nostr::Event, + d_tag: &str, + channel_id: Option, + ) -> Result<(StoredEvent, bool)> { + let mut tx = self.pool.begin().await?; + let result = self + .replace_parameterized_event_in_transaction( + &mut tx, + community_id, + event, + d_tag, + channel_id, + ParameterizedReplacePrecondition::Unconditional, + ) + .await?; + let was_inserted = result.status == ParameterizedReplaceStatus::Inserted; + if was_inserted { + tx.commit().await?; + } else { + tx.rollback().await?; + } + Ok((result.event, was_inserted)) + } +} diff --git a/crates/buzz-relay/src/handlers/command_executor.rs b/crates/buzz-relay/src/handlers/command_executor.rs index d8569a7a86..007db43ffd 100644 --- a/crates/buzz-relay/src/handlers/command_executor.rs +++ b/crates/buzz-relay/src/handlers/command_executor.rs @@ -105,8 +105,9 @@ async fn persist_command_event( event: &Event, channel_id_override: Option, ) -> Result { - let channel_id = channel_id_override.or_else(|| extract_channel_id(event)); + use buzz_db::replaceable::{ParameterizedReplacePrecondition, ParameterizedReplaceStatus}; + let channel_id = channel_id_override.or_else(|| extract_channel_id(event)); let mut tx = db .begin_transaction() .await @@ -118,22 +119,8 @@ async fn persist_command_event( IngestError::Rejected(format!("restricted: community writes are fenced: {error}")) })?; - // INSERT with ON CONFLICT DO NOTHING — idempotency guard. - let id_bytes = event.id.as_bytes(); - let pubkey_bytes = event.pubkey.to_bytes(); - let sig_bytes = event.sig.serialize(); - let tags_json = serde_json::to_value(&event.tags) - .map_err(|e| IngestError::Internal(format!("error: serialize tags: {e}")))?; - let kind_i32 = event.kind.as_u16() as i32; - let created_at_secs = event.created_at.as_secs() as i64; - let created_at = chrono::DateTime::from_timestamp(created_at_secs, 0).ok_or_else(|| { - IngestError::Rejected(format!("invalid: bad timestamp {created_at_secs}")) - })?; - let received_at = chrono::Utc::now(); - - // Extract d_tag for parameterized replaceable kinds (NIP-33). let d_tag = buzz_db::event::extract_d_tag(event); - if let Some(ref d_tag) = d_tag { + if let Some(d_tag) = d_tag.as_deref() { if d_tag.len() > buzz_db::event::D_TAG_MAX_LEN { return Err(IngestError::Rejected(format!( "invalid: d tag too long ({} bytes, max {})", @@ -142,130 +129,81 @@ async fn persist_command_event( ))); } - // Command kinds normally use plain insert semantics, but workflow - // definitions are NIP-33 events. Serialize writers for the same - // coordinate and reject stale writes before executing the domain - // mutation, otherwise old updates can overwrite newer workflow state. - let lock_key = { - let mut h: u64 = 0xcbf29ce484222325; - for b in tenant.community().as_uuid().as_bytes() { - h ^= *b as u64; - h = h.wrapping_mul(0x100000001b3); - } - for b in kind_i32.to_le_bytes() { - h ^= b as u64; - h = h.wrapping_mul(0x100000001b3); - } - for b in pubkey_bytes.as_slice() { - h ^= *b as u64; - h = h.wrapping_mul(0x100000001b3); - } - for b in d_tag.as_bytes() { - h ^= *b as u64; - h = h.wrapping_mul(0x100000001b3); - } - h as i64 + let kind = event.kind.as_u16() as i32; + let (expected_revision, revision_error) = match parse_expected_workflow_revision( + kind, + extract_tag(event, "expected-revision").as_deref(), + ) { + Ok(expected_revision) => (expected_revision, None), + Err(error) => (None, Some(error)), }; - - sqlx::query("SELECT pg_advisory_xact_lock($1)") - .bind(lock_key) - .execute(tx.as_mut()) - .await - .map_err(|e| IngestError::Internal(format!("error: lock event coordinate: {e}")))?; - - let existing: Option<(chrono::DateTime, Vec)> = sqlx::query_as( - "SELECT created_at, id FROM events \ - WHERE community_id = $1 AND kind = $2 AND pubkey = $3 AND d_tag = $4 AND deleted_at IS NULL \ - ORDER BY created_at DESC, id ASC LIMIT 1", - ) - .bind(tenant.community().as_uuid()) - .bind(kind_i32) - .bind(pubkey_bytes.as_slice()) - .bind(d_tag) - .fetch_optional(tx.as_mut()) - .await - .map_err(|e| IngestError::Internal(format!("error: query event coordinate: {e}")))?; - - let incoming_id = event.id.as_bytes().as_slice(); - if existing - .as_ref() - .is_some_and(|(_, existing_id)| existing_id.as_slice() == incoming_id) - { - return Ok(PersistResult::Duplicate); - } - - let expected_revision = extract_tag(event, "expected-revision"); - validate_workflow_revision( - kind_i32, - expected_revision.as_deref(), - existing.as_ref().map(|(_, id)| id.as_slice()), - )?; - if let Some((existing_ts, existing_id)) = existing { - let dominated = created_at < existing_ts - || (created_at == existing_ts && incoming_id >= existing_id.as_slice()); - if dominated { - if kind_i32 == KIND_WORKFLOW_DEF as i32 && expected_revision.is_some() { - return Err(IngestError::Rejected( - "conflict: workflow update was superseded; refresh and try again".into(), - )); - } - return Ok(PersistResult::Duplicate); - } - - sqlx::query( - "UPDATE events SET deleted_at = NOW() \ - WHERE community_id = $1 AND kind = $2 AND pubkey = $3 AND d_tag = $4 AND deleted_at IS NULL", + let precondition = if revision_error.is_some() { + ParameterizedReplacePrecondition::ExactReplayOnly + } else if let Some(expected_revision) = expected_revision.as_deref() { + ParameterizedReplacePrecondition::ExpectedRevision(expected_revision) + } else { + ParameterizedReplacePrecondition::Unconditional + }; + let result = db + .replace_parameterized_event_in_transaction( + &mut tx, + tenant.community(), + event, + d_tag, + channel_id, + precondition, ) - .bind(tenant.community().as_uuid()) - .bind(kind_i32) - .bind(pubkey_bytes.as_slice()) - .bind(d_tag) - .execute(tx.as_mut()) .await - .map_err(|e| IngestError::Internal(format!("error: replace old event: {e}")))?; - } + .map_err(|e| { + IngestError::Internal(format!("error: replace parameterized event: {e}")) + })?; + + return match result.status { + ParameterizedReplaceStatus::Inserted => Ok(PersistResult::Inserted(tx)), + ParameterizedReplaceStatus::Duplicate => Ok(PersistResult::Duplicate), + ParameterizedReplaceStatus::Superseded + if kind == KIND_WORKFLOW_DEF as i32 && expected_revision.is_some() => + { + Err(IngestError::Rejected( + "conflict: workflow update was superseded; refresh and try again".into(), + )) + } + ParameterizedReplaceStatus::Superseded => Ok(PersistResult::Duplicate), + ParameterizedReplaceStatus::RevisionMissing => Err(IngestError::Rejected( + "conflict: workflow revision does not exist".into(), + )), + ParameterizedReplaceStatus::RevisionMismatch => Err(IngestError::Rejected( + "conflict: workflow changed since it was loaded".into(), + )), + ParameterizedReplaceStatus::ReplayOnlyMiss => match revision_error { + Some(error) => Err(error), + None => Err(IngestError::Internal( + "error: replay-only replacement lacked a revision error".into(), + )), + }, + }; } - let result = sqlx::query( - r#" - INSERT INTO events (community_id, id, pubkey, created_at, kind, tags, content, sig, received_at, channel_id, d_tag) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) - ON CONFLICT DO NOTHING - "#, - ) - .bind(tenant.community().as_uuid()) - .bind(id_bytes.as_slice()) - .bind(pubkey_bytes.as_slice()) - .bind(created_at) - .bind(kind_i32) - .bind(&tags_json) - .bind(&event.content) - .bind(sig_bytes.as_slice()) - .bind(received_at) - .bind(channel_id) - .bind(d_tag.as_deref()) - .execute(tx.as_mut()) - .await - .map_err(|e| IngestError::Internal(format!("error: insert event: {e}")))?; - - if result.rows_affected() == 0 { - // Duplicate — rollback (implicit on drop) and signal idempotent success. - Ok(PersistResult::Duplicate) - } else { + let (_, was_inserted) = + buzz_db::event::insert_event_in_transaction(&mut tx, tenant.community(), event, channel_id) + .await + .map_err(|e| IngestError::Internal(format!("error: insert event: {e}")))?; + if was_inserted { Ok(PersistResult::Inserted(tx)) + } else { + Ok(PersistResult::Duplicate) } } -fn validate_workflow_revision( +fn parse_expected_workflow_revision( kind: i32, expected_revision: Option<&str>, - existing_id: Option<&[u8]>, -) -> Result<(), IngestError> { +) -> Result>, IngestError> { if kind != KIND_WORKFLOW_DEF as i32 { - return Ok(()); + return Ok(None); } - let expected_id = expected_revision + expected_revision .map(|expected| { let id = hex::decode(expected).map_err(|_| { IngestError::Rejected("invalid: bad expected workflow revision".into()) @@ -277,18 +215,7 @@ fn validate_workflow_revision( } Ok(id) }) - .transpose()?; - - match (expected_id.as_deref(), existing_id) { - (None, _) => Ok(()), - (Some(_), None) => Err(IngestError::Rejected( - "conflict: workflow revision does not exist".into(), - )), - (Some(expected), Some(existing)) if expected != existing => Err(IngestError::Rejected( - "conflict: workflow changed since it was loaded".into(), - )), - (Some(_), Some(_)) => Ok(()), - } + .transpose() } /// Extract all `p` tag values (hex pubkeys) from an event. @@ -425,7 +352,7 @@ async fn handle_dm_open( .await .map_err(|e| IngestError::Internal(format!("error: db open_dm: {e}")))?; - // Commit: event + mutation succeeded atomically. + // Finalize the idempotency record after the separate mutation succeeds. tx.commit() .await .map_err(|e| IngestError::Internal(format!("error: commit transaction: {e}")))?; @@ -586,7 +513,7 @@ async fn handle_dm_add_member( .await .map_err(|e| IngestError::Internal(format!("error: db open_dm: {e}")))?; - // Commit: event + mutation succeeded atomically. + // Finalize the idempotency record after the separate mutation succeeds. tx.commit() .await .map_err(|e| IngestError::Internal(format!("error: commit transaction: {e}")))?; @@ -691,7 +618,7 @@ async fn handle_dm_hide( .await .map_err(|e| IngestError::Internal(format!("error: db hide_dm: {e}")))?; - // Commit: event + mutation succeeded atomically. + // Finalize the idempotency record after the separate mutation succeeds. tx.commit() .await .map_err(|e| IngestError::Internal(format!("error: commit transaction: {e}")))?; @@ -994,7 +921,7 @@ async fn handle_workflow_trigger( .await .map_err(|e| IngestError::Internal(format!("error: db create_workflow_run: {e}")))?; - // Commit: event + run creation succeeded atomically. + // Finalize the idempotency record after the separate run creation succeeds. tx.commit() .await .map_err(|e| IngestError::Internal(format!("error: commit transaction: {e}")))?; @@ -1169,7 +1096,7 @@ async fn handle_approval_grant( )); } - // Commit: event + approval update succeeded atomically. + // Finalize the idempotency record after the separate approval update succeeds. tx.commit() .await .map_err(|e| IngestError::Internal(format!("error: commit transaction: {e}")))?; @@ -1280,7 +1207,7 @@ async fn handle_approval_deny( )); } - // Commit: event + approval denial succeeded atomically. + // Finalize the idempotency record after the separate approval denial succeeds. tx.commit() .await .map_err(|e| IngestError::Internal(format!("error: commit transaction: {e}")))?; @@ -1489,57 +1416,40 @@ mod tests { .expect("workflow event") } - fn rejection_message(result: Result<(), IngestError>) -> String { + fn rejection_message(result: Result>, IngestError>) -> String { match result { Err(IngestError::Rejected(message)) => message, Err(IngestError::AuthFailed(message)) => panic!("unexpected auth failure: {message}"), Err(IngestError::Internal(message)) => panic!("unexpected internal failure: {message}"), - Ok(()) => panic!("expected revision validation to fail"), + Ok(_) => panic!("expected revision parsing to fail"), } } #[test] - fn workflow_revision_accepts_create_and_matching_update() { - let existing = [0x42; 32]; - assert!(validate_workflow_revision(KIND_WORKFLOW_DEF as i32, None, None).is_ok()); - assert!(validate_workflow_revision( - KIND_WORKFLOW_DEF as i32, - Some(&hex::encode(existing)), - Some(&existing), - ) - .is_ok()); + fn workflow_revision_parser_accepts_create_and_valid_update() { + let revision = [0x42; 32]; + assert_eq!( + parse_expected_workflow_revision(KIND_WORKFLOW_DEF as i32, None) + .expect("tagless workflow"), + None + ); + assert_eq!( + parse_expected_workflow_revision( + KIND_WORKFLOW_DEF as i32, + Some(&hex::encode(revision)), + ) + .expect("valid revision"), + Some(revision.to_vec()) + ); } #[test] - fn workflow_revision_rejects_stale_and_malformed_updates() { - let existing = [0x42; 32]; - let stale = [0x24; 32]; - assert_eq!( - rejection_message(validate_workflow_revision( - KIND_WORKFLOW_DEF as i32, - Some(&hex::encode(stale)), - Some(&existing), - )), - "conflict: workflow changed since it was loaded", - ); - assert!( - validate_workflow_revision(KIND_WORKFLOW_DEF as i32, None, Some(&existing)).is_ok(), - "tagless legacy workflow updates remain compatible during rollout", - ); + fn workflow_revision_parser_rejects_malformed_values() { for malformed in ["not-hex", "42"] { assert_eq!( - rejection_message(validate_workflow_revision( + rejection_message(parse_expected_workflow_revision( KIND_WORKFLOW_DEF as i32, Some(malformed), - Some(&existing), - )), - "invalid: bad expected workflow revision", - ); - assert_eq!( - rejection_message(validate_workflow_revision( - KIND_WORKFLOW_DEF as i32, - Some(malformed), - None, )), "invalid: bad expected workflow revision", ); @@ -1547,14 +1457,11 @@ mod tests { } #[test] - fn workflow_revision_rejects_update_for_missing_coordinate() { + fn revision_tag_does_not_change_other_command_kinds() { assert_eq!( - rejection_message(validate_workflow_revision( - KIND_WORKFLOW_DEF as i32, - Some(&hex::encode([0x42; 32])), - None, - )), - "conflict: workflow revision does not exist", + parse_expected_workflow_revision(KIND_DM_OPEN as i32, Some("not-hex")) + .expect("non-workflow revision tag"), + None ); } @@ -1567,6 +1474,25 @@ mod tests { let created_at = Timestamp::now().as_secs(); let create = workflow_event(&keys, workflow_id, created_at, None, "create"); + let missing_revision = hex::encode([0x24; 32]); + let missing_revision_update = workflow_event( + &keys, + Uuid::new_v4(), + created_at, + Some(&missing_revision), + "missing-revision", + ); + let error = match persist_command_event(&db, &tenant, &missing_revision_update, None).await + { + Err(error) => error, + Ok(_) => panic!("missing revision must not create a workflow"), + }; + assert!(matches!( + error, + IngestError::Rejected(ref message) + if message == "conflict: workflow revision does not exist" + )); + let PersistResult::Inserted(tx) = persist_command_event(&db, &tenant, &create, None) .await .expect("persist create") @@ -1621,6 +1547,23 @@ mod tests { PersistResult::Duplicate )); + let stale_revision_update = workflow_event( + &keys, + workflow_id, + created_at + 1, + Some(&create_revision), + "stale-revision", + ); + let error = match persist_command_event(&db, &tenant, &stale_revision_update, None).await { + Err(error) => error, + Ok(_) => panic!("stale revision must not replace the current workflow"), + }; + assert!(matches!( + error, + IngestError::Rejected(ref message) + if message == "conflict: workflow changed since it was loaded" + )); + let error = match persist_command_event(&db, &tenant, &dominated_update, None).await { Err(error) => error, Ok(_) => panic!("distinct dominated CAS update must not report duplicate success"), @@ -1632,8 +1575,55 @@ mod tests { )); } - #[test] - fn revision_tag_does_not_change_other_command_kinds() { - assert!(validate_workflow_revision(KIND_DM_OPEN as i32, Some("not-hex"), None).is_ok()); + #[tokio::test] + #[ignore = "requires Postgres"] + async fn workflow_persistence_replays_legacy_malformed_revision_before_validation() { + let (db, tenant) = persistence_test_context().await; + let keys = Keys::generate(); + let workflow_id = Uuid::new_v4(); + let created_at = Timestamp::now().as_secs(); + let legacy = workflow_event( + &keys, + workflow_id, + created_at, + Some("not-hex"), + "legacy-malformed", + ); + + let mut tx = db.begin_transaction().await.expect("begin legacy seed"); + let (_, was_inserted) = buzz_db::event::insert_event_in_transaction( + &mut tx, + tenant.community(), + &legacy, + extract_channel_id(&legacy), + ) + .await + .expect("seed legacy workflow event"); + assert!(was_inserted); + tx.commit().await.expect("commit legacy seed"); + + assert!(matches!( + persist_command_event(&db, &tenant, &legacy, None) + .await + .expect("exact legacy replay must remain idempotent"), + PersistResult::Duplicate + )); + + let distinct = workflow_event( + &keys, + workflow_id, + created_at + 1, + Some("not-hex"), + "distinct-malformed", + ); + let error = match persist_command_event(&db, &tenant, &distinct, None).await { + Err(error) => error, + Ok(_) => panic!("distinct malformed revision must remain rejected"), + }; + assert!(matches!( + error, + IngestError::Rejected(ref message) + if message == "invalid: bad expected workflow revision" + )); } } diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index 9099beff69..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", @@ -86,6 +87,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/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-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/agents/ui/OtherSetupAgentMarker.tsx b/desktop/src/features/agents/ui/OtherSetupAgentMarker.tsx index e653943469..19ef31bd75 100644 --- a/desktop/src/features/agents/ui/OtherSetupAgentMarker.tsx +++ b/desktop/src/features/agents/ui/OtherSetupAgentMarker.tsx @@ -1,7 +1,6 @@ import { Cloud } from "lucide-react"; import { cn } from "@/shared/lib/cn"; -import { Badge } from "@/shared/ui/badge"; const OTHER_SETUP_LABEL = "From another Buzz setup"; @@ -13,18 +12,14 @@ export function OtherSetupAgentMarker({ testId?: string; }) { return ( - + ); } 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/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/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/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 ? ( 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/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) ? (