From 24ec6a468ec9d0d425ee58fbfc4d416412c446ad Mon Sep 17 00:00:00 2001 From: Wes Date: Thu, 20 Aug 2026 21:22:41 -0600 Subject: [PATCH 01/33] Repair stale large channel roster snapshots (#6251) ## Summary - detect relay-authored NIP-29 kind 39002 roster snapshots truncated by the former 1,000-member query cap and repair stale large rosters during relay startup - serialize canonical roster capture and replacement with membership writes, preserving tenant, channel, signer, pubkey, and role boundaries through mixed-version deployments - install migration 0032's fail-closed roster fence on the partitioned events table and verify its catalog shape plus behavior before opening relay listeners ## Rollout Migration 0032 is a hard schema-before-code compatibility boundary. Apply migrations before rolling this relay version. Startup refuses to open listeners when the parent/partition triggers are missing, disabled, mis-shaped, or behaviorally inert. For large installations, prefer `buzz-admin migrate` and monitor lock acquisition as documented in the chart README. ## Validation Exact head: `bcbba271f54bc0046a6683007e5a2b70403a11d5` - rebased onto `569308c23c9c2bf620dd3a9a5e4baecbcfa22e16`; the nine-file feature patch is byte-identical to pre-rebase head `be8ea0084f4d4c78c7c2550baad4399e4df8ce73` - pre-push hook passed at exact head: branch-skew, file-size, full Rust unit suite, Desktop Tauri clippy, and Desktop Tauri tests - `cargo fmt --all -- --check` - `cargo test -p buzz-relay group_members_snapshot_keeps_members_past_one_thousand -- --nocapture` - focused CI-mode Playwright regression: `selected relay agents revoked after the invite prompt cause no side effects` passed at exact head - prior exact-patch validation: `large_roster_reconciliation_candidates_respect_snapshot_count_and_signer`, mixed-writer locking/rollback, migration admission, partition trigger coverage, and desired-schema parity regressions ## Review Independent DB/relay review found no blocking issues in the exact feature patch. The concurrency fence holds the established replacement and membership locks on one transaction/connection through replacement; failures roll back both soft-delete and insert. Reconciliation remains tenant/channel/signer scoped and validates exact normalized pubkey-plus-role membership. The prior red Desktop shard was unrelated to this backend-only diff: its mocked mention test exercises no relay, database, or migration path. It reproduced as a timing flake on the old head, passed on retry/base, and now passes locally after rebasing onto current main. --------- Signed-off-by: Wes Co-authored-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz> --- crates/buzz-db/src/channel.rs | 644 ++++++++++++++++++ crates/buzz-db/src/lib.rs | 410 ++++++++++- crates/buzz-db/src/migration.rs | 75 +- .../buzz-relay/src/handlers/side_effects.rs | 133 +++- crates/buzz-relay/src/main.rs | 25 + deploy/charts/buzz/README.md | 2 + .../0032_channel_roster_snapshot_fence.sql | 76 +++ schema/schema.sql | 79 +++ scripts/attach-schema-partitions.sql | 11 +- 9 files changed, 1423 insertions(+), 32 deletions(-) create mode 100644 migrations/0032_channel_roster_snapshot_fence.sql diff --git a/crates/buzz-db/src/channel.rs b/crates/buzz-db/src/channel.rs index 8035ab58adb..109a9367d7a 100644 --- a/crates/buzz-db/src/channel.rs +++ b/crates/buzz-db/src/channel.rs @@ -347,6 +347,129 @@ pub async fn set_canvas( /// `buzz_channel_ttl:`. const CHANNEL_MEMBERSHIP_LOCK_NAMESPACE: &str = "buzz_channel_membership:"; +/// Verify that migration 0032's roster fence is active on the partitioned +/// `events` parent and every attached partition. +/// +/// New roster publishers depend on this database-side guard to serialize with +/// legacy publishers during a rolling deployment. If the migration has not +/// been applied, publishing with the new lock protocol would falsely appear +/// safe while an old pod could still overwrite it with stale membership. +pub async fn verify_channel_roster_fence_catalog<'e>( + executor: impl sqlx::PgExecutor<'e>, +) -> Result<()> { + // tgtype bits: 1 = ROW, 2 = BEFORE, 4 = INSERT, 16 = UPDATE, 64 = INSTEAD. + // Required: ROW + BEFORE + INSERT set; UPDATE + INSTEAD clear. + let missing: Vec = sqlx::query_scalar( + r#" + SELECT n.nspname || '.' || c.relname + FROM ( + SELECT 'public.events'::regclass AS oid + UNION ALL + SELECT inhrelid FROM pg_inherits WHERE inhparent = 'public.events'::regclass + ) rels + JOIN pg_class c ON c.oid = rels.oid + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE NOT EXISTS ( + SELECT 1 FROM pg_trigger t + WHERE t.tgrelid = rels.oid + AND t.tgname = 'trg_events_guard_channel_roster_snapshot' + AND t.tgfoid = to_regprocedure('public.guard_channel_roster_snapshot()') + AND t.tgenabled IN ('O', 'A') + AND t.tgtype & 1 = 1 -- row-level + AND t.tgtype & 2 = 2 -- BEFORE + AND t.tgtype & 4 = 4 -- fires on INSERT + AND t.tgtype & 16 = 0 -- not UPDATE + AND t.tgtype & 64 = 0 -- not INSTEAD OF + ) + "#, + ) + .fetch_all(executor) + .await?; + if !missing.is_empty() { + return Err(DbError::InvalidData(format!( + "channel roster fence trigger missing, disabled, or mis-shaped on: {}", + missing.join(", ") + ))); + } + Ok(()) +} + +/// Prove migration 0032's roster fence semantics through the live writer pool. +/// +/// The catalog check cannot detect a no-op or otherwise corrupted trigger +/// function. This rolled-back probe verifies that a canonical empty roster is +/// accepted while a stale roster member is rejected with `check_violation`. +pub async fn verify_channel_roster_fence_behavior(pool: &sqlx::PgPool) -> Result<()> { + let mut tx = pool.begin().await?; + let community_id = Uuid::new_v4(); + let channel_id = Uuid::new_v4(); + sqlx::query("INSERT INTO communities (id, host) VALUES ($1, $2)") + .bind(community_id) + .bind(format!( + "roster-fence-verify-{}.invalid", + community_id.simple() + )) + .execute(&mut *tx) + .await?; + + let insert = |id: Vec, tags: serde_json::Value| { + sqlx::query( + "INSERT INTO events (community_id, id, pubkey, created_at, kind, tags, content, sig, received_at, channel_id, d_tag) \ + VALUES ($1, $2, $3, NOW(), 39002, $4, '', $5, NOW(), $6, $7)", + ) + .bind(community_id) + .bind(id) + .bind(vec![0u8; 32]) + .bind(tags) + .bind(vec![0u8; 64]) + .bind(channel_id) + .bind(channel_id.to_string()) + }; + + insert( + vec![0u8; 32], + serde_json::json!([["d", channel_id.to_string()]]), + ) + .execute(&mut *tx) + .await + .map_err(|error| { + DbError::InvalidData(format!( + "channel roster fence rejected a canonical probe roster: {error}" + )) + })?; + + sqlx::query("SAVEPOINT roster_fence_probe") + .execute(&mut *tx) + .await?; + let stale = insert( + vec![1u8; 32], + serde_json::json!([ + ["d", channel_id.to_string()], + ["p", hex::encode([2u8; 32]), "", "member"] + ]), + ) + .execute(&mut *tx) + .await; + match stale { + Err(sqlx::Error::Database(error)) if error.code().as_deref() == Some("23514") => {} + Ok(_) => { + return Err(DbError::InvalidData( + "channel roster fence is inert: a stale probe roster was accepted".into(), + )); + } + Err(error) => { + return Err(DbError::InvalidData(format!( + "channel roster fence probe failed unexpectedly: {error}" + ))); + } + } + sqlx::query("ROLLBACK TO SAVEPOINT roster_fence_probe") + .execute(&mut *tx) + .await?; + tx.rollback().await?; + Ok(()) +} + /// Take the per-channel membership lock. MUST be the first statement in the /// transaction that then reads roles/owner counts and writes membership, so the /// whole check-then-write sequence is atomic against a concurrent one. @@ -366,6 +489,179 @@ async fn acquire_channel_membership_lock( Ok(()) } +/// An active member roster captured while holding the channel's membership +/// serialization lock on one writer connection. +pub struct LockedMemberSnapshot { + /// Canonical active members captured behind the lock. + pub members: Vec, + community_id: CommunityId, + channel_id: Uuid, + relay_pubkey: Vec, + tx: Transaction<'static, Postgres>, +} + +impl LockedMemberSnapshot { + /// Return the newest relay-authored member snapshot timestamp using this + /// guard's existing connection. + pub async fn latest_member_event_timestamp( + &mut self, + community_id: CommunityId, + channel_id: Uuid, + relay_pubkey: &[u8], + ) -> Result> { + let value: Option> = sqlx::query_scalar( + "SELECT created_at FROM events WHERE community_id = $1 AND kind = 39002 AND pubkey = $2 AND channel_id = $3 AND deleted_at IS NULL ORDER BY created_at DESC, id ASC LIMIT 1", + ) + .bind(community_id.as_uuid()) + .bind(relay_pubkey) + .bind(channel_id) + .fetch_optional(&mut *self.tx) + .await?; + Ok(value.map(|timestamp| timestamp.timestamp() as u64)) + } + + /// Replace the relay-authored member snapshot on this guard's existing + /// connection. The membership lock therefore spans capture and replacement + /// without a nested pool checkout. + pub async fn replace_member_event( + &mut self, + community_id: CommunityId, + channel_id: Uuid, + event: &nostr::Event, + ) -> Result<(buzz_core::StoredEvent, bool)> { + if community_id != self.community_id + || channel_id != self.channel_id + || event.pubkey.to_bytes().as_slice() != self.relay_pubkey.as_slice() + { + return Err(DbError::InvalidData( + "member snapshot replacement does not match its locked coordinate".into(), + )); + } + let kind = buzz_core::kind::event_kind_i32(event); + if kind != 39002 { + return Err(DbError::InvalidData( + "member snapshot replacement requires kind 39002".into(), + )); + } + let pubkey = 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 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 channel_id = $4 AND deleted_at IS NULL ORDER BY created_at DESC, id ASC LIMIT 1", + ) + .bind(community_id.as_uuid()) + .bind(kind) + .bind(pubkey.as_slice()) + .bind(channel_id) + .fetch_optional(&mut *self.tx) + .await?; + let incoming_id = event.id.as_bytes().as_slice(); + if let Some((existing_ts, existing_id)) = existing { + if created_at < existing_ts + || (created_at == existing_ts && incoming_id >= existing_id.as_slice()) + { + return Ok(( + buzz_core::StoredEvent::with_received_at( + event.clone(), + Utc::now(), + Some(channel_id), + false, + ), + false, + )); + } + } + sqlx::query("UPDATE events SET deleted_at = NOW() WHERE community_id = $1 AND kind = $2 AND pubkey = $3 AND channel_id = $4 AND deleted_at IS NULL") + .bind(community_id.as_uuid()).bind(kind).bind(pubkey.as_slice()).bind(channel_id) + .execute(&mut *self.tx).await?; + let received_at = Utc::now(); + let tags = serde_json::to_value(&event.tags)?; + let sig = event.sig.serialize(); + let inserted = sqlx::query("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(community_id.as_uuid()).bind(event.id.as_bytes().as_slice()) + .bind(pubkey.as_slice()).bind(created_at).bind(kind).bind(tags) + .bind(&event.content).bind(sig.as_slice()).bind(received_at).bind(channel_id) + .bind(crate::event::extract_d_tag(event)).execute(&mut *self.tx).await?; + if inserted.rows_affected() == 0 { + return Err(DbError::InvalidData( + "member snapshot event id already exists".into(), + )); + } + crate::insert_mentions_in_transaction(&mut self.tx, community_id, event, Some(channel_id)) + .await?; + Ok(( + buzz_core::StoredEvent::with_received_at( + event.clone(), + received_at, + Some(channel_id), + true, + ), + true, + )) + } + + /// Commit the replacement and release the membership lock. + pub async fn release(self) -> Result<()> { + self.tx.commit().await?; + Ok(()) + } +} + +/// Capture all active members while holding the same per-channel lock used by +/// membership writers. +/// +/// The returned guard must remain alive through publication. This prevents a +/// rolling relay from publishing an older roster after a concurrent add or +/// remove has committed and published newer membership state. +pub async fn lock_member_snapshot( + pool: &PgPool, + community_id: CommunityId, + channel_id: Uuid, + relay_pubkey: &[u8], +) -> Result { + let mut tx = pool.begin().await?; + // Match the canonical replacement writer's lock order. Old binaries take + // 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( + community_id, + 39002, + relay_pubkey, + Some(channel_id.as_bytes()), + ); + sqlx::query("SELECT pg_advisory_xact_lock($1)") + .bind(replacement_lock) + .execute(&mut *tx) + .await?; + acquire_channel_membership_lock(&mut tx, community_id, channel_id).await?; + let rows = sqlx::query( + r#" + SELECT cm.channel_id, cm.pubkey, cm.role::text AS role, cm.joined_at, cm.invited_by, cm.removed_at + FROM channel_members cm + JOIN channels c ON cm.community_id = c.community_id AND cm.channel_id = c.id AND c.deleted_at IS NULL + WHERE cm.community_id = $1 AND cm.channel_id = $2 AND cm.removed_at IS NULL + ORDER BY cm.joined_at ASC + "#, + ) + .bind(community_id.as_uuid()) + .bind(channel_id) + .fetch_all(&mut *tx) + .await?; + let members = rows + .into_iter() + .map(row_to_member_record) + .collect::>>()?; + Ok(LockedMemberSnapshot { + members, + community_id, + channel_id, + relay_pubkey: relay_pubkey.to_vec(), + tx, + }) +} + /// Add a member to a channel. /// /// Role enforcement: @@ -781,6 +1077,81 @@ pub async fn get_accessible_channel_ids( .collect() } +/// A large channel whose canonical active-member count may need its legacy +/// discovery snapshot repaired. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct LargeChannelRoster { + /// Community that owns the channel. + pub community_id: CommunityId, + /// Canonical host for the owning community. + pub host: String, + /// Channel whose roster snapshot differs from canonical membership. + pub channel_id: Uuid, + /// Canonical active-member count. + pub member_count: i64, +} + +/// Returns active channels whose canonical roster exceeds `minimum_members`. +/// +/// This is an internal cross-community maintenance read. Callers must preserve +/// the returned community id when reading or rewriting discovery state. +pub async fn list_large_channel_rosters_needing_reconciliation( + pool: &PgPool, + minimum_members: i64, + relay_pubkey: &[u8], +) -> Result> { + let rows = sqlx::query( + r#" + WITH large_rosters AS ( + SELECT cm.community_id, cm.channel_id, COUNT(*) AS member_count + FROM channel_members cm + JOIN channels ch + ON ch.community_id = cm.community_id + AND ch.id = cm.channel_id + AND ch.deleted_at IS NULL + WHERE cm.removed_at IS NULL + GROUP BY cm.community_id, cm.channel_id + HAVING COUNT(*) > $1 + ) + SELECT lr.community_id, community.host, lr.channel_id, lr.member_count + FROM large_rosters lr + JOIN communities community ON community.id = lr.community_id + JOIN LATERAL ( + SELECT roster.tags + FROM events roster + WHERE roster.community_id = lr.community_id + AND roster.channel_id = lr.channel_id + AND roster.kind = 39002 + AND roster.pubkey = $2 + AND roster.deleted_at IS NULL + ORDER BY roster.created_at DESC, roster.id ASC + LIMIT 1 + ) live_roster ON true + WHERE lr.member_count <> ( + SELECT COUNT(*) + FROM jsonb_array_elements(live_roster.tags) tag + WHERE tag->>0 = 'p' + ) + ORDER BY lr.community_id, lr.channel_id + "#, + ) + .bind(minimum_members) + .bind(relay_pubkey) + .fetch_all(pool) + .await?; + + rows.into_iter() + .map(|row| { + Ok(LargeChannelRoster { + community_id: CommunityId::from_uuid(row.try_get("community_id")?), + host: row.try_get("host")?, + channel_id: row.try_get("channel_id")?, + member_count: row.try_get("member_count")?, + }) + }) + .collect() +} + /// Lists channels in a community, optionally filtered by visibility string. pub async fn list_channels( pool: &PgPool, @@ -1535,6 +1906,7 @@ mod tests { use super::*; use crate::user::{ensure_user, set_agent_owner}; use nostr::Keys; + use sqlx::postgres::PgPoolOptions; const TEST_DB_URL: &str = "postgres://buzz:buzz_dev@localhost:5432/buzz"; // sadscan:disable np.postgres.1 -- local test-only credentials @@ -2016,6 +2388,188 @@ mod tests { ); } + #[tokio::test] + #[ignore = "requires Postgres"] + async fn large_roster_reconciliation_candidates_respect_snapshot_count_and_signer() { + let pool = setup_pool().await; + let community_id = make_test_community(&pool).await; + let community = CommunityId::from_uuid(community_id); + let creator = random_pubkey(); + let relay_pubkey = random_pubkey(); + let other_relay_pubkey = random_pubkey(); + let channel = create_test_channel( + &pool, + community_id, + "stale-large-roster", + ChannelType::Stream, + ChannelVisibility::Open, + None, + &creator, + None, + ) + .await + .expect("create test channel"); + + let extra_members = 1_500; + sqlx::query( + r#" + INSERT INTO channel_members (community_id, channel_id, pubkey, role, joined_at) + SELECT $1, $2, decode(lpad(to_hex(n), 64, '0'), 'hex'), 'member', + NOW() + (n || ' seconds')::interval + FROM generate_series(1, $3) n + "#, + ) + .bind(community_id) + .bind(channel.id) + .bind(extra_members) + .execute(&pool) + .await + .expect("insert large roster"); + + let stale_tags: Vec = + std::iter::once(serde_json::json!(["d", channel.id.to_string()])) + .chain((0..1_000).map(|n| serde_json::json!(["p", format!("{n:064x}")]))) + .collect(); + let complete_tags: Vec = + std::iter::once(serde_json::json!(["d", channel.id.to_string()])) + .chain((0..1_501).map(|n| serde_json::json!(["p", format!("{n:064x}")]))) + .collect(); + + // Insert canonical-looking history first, then corrupt the newest row + // with UPDATE to model a stale snapshot that predates migration 0032's + // INSERT fence. New stale snapshots cannot be inserted once that fence + // is deployed. + sqlx::query( + r#" + INSERT INTO events + (community_id, id, pubkey, created_at, kind, tags, content, sig, channel_id, d_tag) + VALUES + ($1, $2, $3, NOW() - INTERVAL '1 minute', 39002, $4, '', $5, $6, $7), + ($1, $8, $3, NOW(), 39002, $4, '', $5, $6, $7) + "#, + ) + .bind(community_id) + .bind(random_pubkey()) + .bind(&relay_pubkey) + .bind(serde_json::Value::Array(complete_tags.clone())) + .bind(vec![0u8; 64]) + .bind(channel.id) + .bind(channel.id.to_string()) + .bind(random_pubkey()) + .execute(&pool) + .await + .expect("insert historical duplicate snapshots"); + sqlx::query( + "UPDATE events SET tags = $1 WHERE community_id = $2 AND channel_id = $3 \ + AND kind = 39002 AND pubkey = $4 AND created_at = (SELECT MAX(created_at) \ + FROM events WHERE community_id = $2 AND channel_id = $3 AND kind = 39002 AND pubkey = $4)", + ) + .bind(serde_json::Value::Array(stale_tags)) + .bind(community_id) + .bind(channel.id) + .bind(&relay_pubkey) + .execute(&pool) + .await + .expect("simulate pre-fence stale live snapshot"); + + // The same channel UUID in another tenant is deliberately valid. A + // complete snapshot there must not mask this tenant's stale head. + let other_community_id = make_test_community(&pool).await; + sqlx::query( + r#" + INSERT INTO channels + (id, community_id, name, channel_type, visibility, created_by) + VALUES ($1, $2, 'same-id-complete-roster', 'stream', 'open', $3) + "#, + ) + .bind(channel.id) + .bind(other_community_id) + .bind(&creator) + .execute(&pool) + .await + .expect("insert same channel id in other tenant"); + sqlx::query( + r#" + INSERT INTO channel_members (community_id, channel_id, pubkey, role, joined_at) + SELECT $1, $2, decode(lpad(to_hex(n), 64, '0'), 'hex'), 'member', + NOW() + (n || ' seconds')::interval + FROM generate_series(0, 1500) n + "#, + ) + .bind(other_community_id) + .bind(channel.id) + .execute(&pool) + .await + .expect("insert complete other-tenant roster"); + sqlx::query( + r#" + INSERT INTO events + (community_id, id, pubkey, created_at, kind, tags, content, sig, channel_id, d_tag) + VALUES ($1, $2, $3, NOW(), 39002, $4, '', $5, $6, $7) + "#, + ) + .bind(other_community_id) + .bind(random_pubkey()) + .bind(&relay_pubkey) + .bind(serde_json::Value::Array(complete_tags.clone())) + .bind(vec![0u8; 64]) + .bind(channel.id) + .bind(channel.id.to_string()) + .execute(&pool) + .await + .expect("insert complete other-tenant snapshot"); + + // Put the stale channel behind the 1,000 newest channels that the old + // list_channels-based sweep could see. This set-based scan has no such + // pagination ceiling. + sqlx::query( + r#" + INSERT INTO channels + (id, community_id, name, channel_type, visibility, created_by, created_at) + SELECT gen_random_uuid(), $1, 'newer-decoy-' || n, 'stream', 'open', $2, + NOW() + (n || ' seconds')::interval + FROM generate_series(1, 1000) n + "#, + ) + .bind(community_id) + .bind(&creator) + .execute(&pool) + .await + .expect("insert channels beyond old list ceiling"); + + let candidates = + list_large_channel_rosters_needing_reconciliation(&pool, 1_000, &relay_pubkey) + .await + .expect("find stale snapshot"); + assert_eq!(candidates.len(), 1); + assert_eq!(candidates[0].community_id, community); + assert_eq!(candidates[0].channel_id, channel.id); + assert_eq!(candidates[0].member_count, 1_501); + + let other_signer_candidates = + list_large_channel_rosters_needing_reconciliation(&pool, 1_000, &other_relay_pubkey) + .await + .expect("other signer is isolated from relay-authored snapshot"); + assert!(other_signer_candidates.is_empty()); + + sqlx::query( + "UPDATE events SET tags = $1, created_at = NOW() + INTERVAL '1 minute' WHERE community_id = $2 AND channel_id = $3 AND kind = 39002 AND pubkey = $4 AND created_at = (SELECT MAX(created_at) FROM events WHERE community_id = $2 AND channel_id = $3 AND kind = 39002 AND pubkey = $4 AND deleted_at IS NULL)", + ) + .bind(serde_json::Value::Array(complete_tags)) + .bind(community_id) + .bind(channel.id) + .bind(&relay_pubkey) + .execute(&pool) + .await + .expect("complete snapshot"); + + let converged = + list_large_channel_rosters_needing_reconciliation(&pool, 1_000, &relay_pubkey) + .await + .expect("check converged snapshot"); + assert!(converged.is_empty()); + } + /// A random non-admin, non-owner user cannot remove someone else's bot. #[tokio::test] #[ignore = "requires Postgres"] @@ -2499,6 +3053,96 @@ mod tests { (community, channel.id, owner_a, owner_b) } + /// A captured roster holds the same lock as membership writers until the + /// publisher explicitly releases it. This is the freshness fence used by + /// rolling-deploy reconciliation. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn locked_member_snapshot_blocks_post_capture_membership_mutation() { + let pool = setup_pool().await; + let community_id = make_test_community(&pool).await; + let community = CommunityId::from_uuid(community_id); + let owner = random_pubkey(); + let newcomer = random_pubkey(); + let channel = create_test_channel( + &pool, + community_id, + "snapshot-freshness-fence", + ChannelType::Stream, + ChannelVisibility::Open, + None, + &owner, + None, + ) + .await + .expect("create channel"); + + let snapshot_pool = PgPoolOptions::new() + .max_connections(1) + .acquire_timeout(std::time::Duration::from_secs(1)) + .connect(TEST_DB_URL) + .await + .expect("connect one-connection pool"); + let relay_keys = Keys::generate(); + let mut snapshot = lock_member_snapshot( + &snapshot_pool, + community, + channel.id, + &relay_keys.public_key().to_bytes(), + ) + .await + .expect("capture locked roster"); + assert_eq!(snapshot.members.len(), 1); + let event = nostr::EventBuilder::new(nostr::Kind::Custom(39002), "") + .tags(vec![ + nostr::Tag::parse(["d", &channel.id.to_string()]).expect("d tag"), + nostr::Tag::parse(["p", &hex::encode(&owner)]).expect("p tag"), + ]) + .sign_with_keys(&relay_keys) + .expect("sign roster"); + let (_, inserted) = snapshot + .replace_member_event(community, channel.id, &event) + .await + .expect("replace roster on held connection"); + assert!(inserted); + + let mut contender = pool.begin().await.expect("begin membership writer"); + let acquired: bool = + sqlx::query_scalar("SELECT pg_try_advisory_xact_lock(hashtextextended($1, 0))") + .bind(format!( + "{CHANNEL_MEMBERSHIP_LOCK_NAMESPACE}{}:{}", + community.as_uuid(), + channel.id + )) + .fetch_one(&mut *contender) + .await + .expect("try membership writer lock"); + assert!( + !acquired, + "membership mutation must wait until the captured roster is published" + ); + contender.rollback().await.expect("rollback contender"); + + snapshot.release().await.expect("release snapshot fence"); + add_member( + &pool, + community, + channel.id, + &newcomer, + MemberRole::Member, + None, + ) + .await + .expect("membership mutation after publication"); + assert_eq!( + get_members(&pool, community, channel.id) + .await + .expect("fresh roster") + .len(), + 2 + ); + } + /// The lock must be shared with `remove_member`: a demotion racing an owner /// removal goes through a separate count/update path, so both must serialize /// on the same key or they can jointly empty the owner set. diff --git a/crates/buzz-db/src/lib.rs b/crates/buzz-db/src/lib.rs index 330525d310d..3ff230f9503 100644 --- a/crates/buzz-db/src/lib.rs +++ b/crates/buzz-db/src/lib.rs @@ -68,7 +68,7 @@ use uuid::Uuid; use buzz_core::{CommunityId, StoredEvent}; -fn event_replacement_lock_key( +pub(crate) fn event_replacement_lock_key( community_id: CommunityId, kind: i32, pubkey: &[u8], @@ -2390,6 +2390,24 @@ impl Db { channel::set_canvas(&self.pool, community_id, channel_id, canvas).await } + /// Verify the mixed-version channel-roster database fence end to end. + #[datastore_span(name = "verify_channel_roster_fence", system = "postgresql")] + pub async fn verify_channel_roster_fence(&self) -> Result<()> { + channel::verify_channel_roster_fence_catalog(&self.pool).await?; + channel::verify_channel_roster_fence_behavior(&self.pool).await + } + + /// Capture the active roster while holding the membership-writer lock. + #[datastore_span(name = "lock_member_snapshot", system = "postgresql")] + pub async fn lock_member_snapshot( + &self, + community_id: CommunityId, + channel_id: Uuid, + relay_pubkey: &[u8], + ) -> Result { + channel::lock_member_snapshot(&self.pool, community_id, channel_id, relay_pubkey).await + } + /// Adds a member to a channel. #[datastore_span(name = "add_member", system = "postgresql")] pub async fn add_member( @@ -2476,6 +2494,24 @@ impl Db { channel::get_accessible_channel_ids(&self.pool, community_id, pubkey).await } + /// Returns large active-channel rosters whose relay-authored snapshots differ. + #[datastore_span( + name = "list_large_channel_rosters_needing_reconciliation", + system = "postgresql" + )] + pub async fn list_large_channel_rosters_needing_reconciliation( + &self, + minimum_members: i64, + relay_pubkey: &[u8], + ) -> Result> { + channel::list_large_channel_rosters_needing_reconciliation( + &self.pool, + minimum_members, + relay_pubkey, + ) + .await + } + /// Lists channels, optionally filtered by visibility. #[datastore_span(name = "list_channels", system = "postgresql")] pub async fn list_channels( @@ -5480,6 +5516,113 @@ mod tests { id } + #[tokio::test] + #[ignore = "requires Postgres"] + async fn unmigrated_roster_fence_blocks_startup_until_0032_is_applied() { + let admin = PgPool::connect(&admin_url().await) + .await + .expect("connect admin"); + let (pool, scratch_name) = + create_scratch_db_through(&admin, "roster_fence_unmigrated", Some(31)).await; + let db = Db::from_pool(pool.clone()); + + let error = db + .verify_channel_roster_fence() + .await + .expect_err("pre-0032 schema must block roster publishers"); + assert!( + error.to_string().contains("channel roster fence trigger"), + "startup gate must report the missing schema fence: {error}" + ); + let rows_before: i64 = sqlx::query_scalar("SELECT count(*) FROM events WHERE kind = 39002") + .fetch_one(&pool) + .await + .expect("count pre-migration rosters"); + assert_eq!( + rows_before, 0, + "failed startup gate must not publish a roster" + ); + + migration::run_migrations(&pool) + .await + .expect("apply migration 0032"); + db.verify_channel_roster_fence() + .await + .expect("0032 must open the startup gate"); + + drop_scratch_db(&admin, pool, &scratch_name).await; + } + + #[tokio::test] + #[ignore = "requires Postgres"] + async fn channel_roster_fence_behavior_verification_detects_inert_function() { + let admin = PgPool::connect(&admin_url().await) + .await + .expect("connect admin"); + let (pool, scratch_name) = create_scratch_db(&admin, "roster_fence_inert").await; + let db = Db::from_pool(pool.clone()); + + sqlx::raw_sql( + "CREATE OR REPLACE FUNCTION guard_channel_roster_snapshot() \ + RETURNS TRIGGER AS $$ BEGIN RETURN NEW; END; $$ LANGUAGE plpgsql;", + ) + .execute(&pool) + .await + .expect("replace roster fence with inert body"); + let error = db + .verify_channel_roster_fence() + .await + .expect_err("inert roster fence must fail closed"); + assert!( + error + .to_string() + .contains("stale probe roster was accepted"), + "behavior probe must identify inert semantics: {error}" + ); + + drop_scratch_db(&admin, pool, &scratch_name).await; + } + + #[tokio::test] + #[ignore = "requires Postgres"] + async fn channel_roster_fence_catalog_verification_fails_closed() { + let admin = PgPool::connect(&admin_url().await) + .await + .expect("connect admin"); + let (pool, scratch_name) = create_scratch_db(&admin, "roster_fence_catalog").await; + let db = Db::from_pool(pool.clone()); + + db.verify_channel_roster_fence() + .await + .expect("migrated roster fence must verify"); + + let child: String = sqlx::query_scalar( + "SELECT n.nspname || '.' || c.relname \ + FROM pg_inherits i JOIN pg_class c ON c.oid = i.inhrelid \ + JOIN pg_namespace n ON n.oid = c.relnamespace \ + WHERE i.inhparent = 'public.events'::regclass ORDER BY i.inhrelid LIMIT 1", + ) + .fetch_one(&pool) + .await + .expect("load event partition"); + sqlx::query(sqlx::AssertSqlSafe(format!( + "ALTER TABLE {child} DISABLE TRIGGER trg_events_guard_channel_roster_snapshot" + ))) + .execute(&pool) + .await + .expect("disable partition roster trigger"); + let error = db + .verify_channel_roster_fence() + .await + .expect_err("disabled partition roster fence must fail closed"); + assert!( + error.to_string().contains(&child), + "verification must identify the unfenced partition: {error}" + ); + + drop_scratch_db(&admin, pool, &scratch_name).await; + } + #[tokio::test] #[ignore = "requires Postgres"] async fn addressable_replacement_rolls_back_when_mention_indexing_fails() { @@ -5493,13 +5636,14 @@ mod tests { let community_uuid = Uuid::new_v4(); let channel = Uuid::new_v4(); let keys = Keys::generate(); - seed_community_channel(&pool, community_uuid, channel, &keys).await; + let owner_keys = Keys::generate(); + seed_community_channel(&pool, community_uuid, channel, &owner_keys).await; let community = CommunityId::from_uuid(community_uuid); - let member = Keys::generate().public_key().to_hex(); + let member = owner_keys.public_key().to_hex(); let tags = || { vec![ Tag::parse(["d", channel.to_string().as_str()]).expect("d tag"), - Tag::parse(["p", member.as_str(), "", "member"]).expect("p tag"), + Tag::parse(["p", member.as_str(), "", "owner"]).expect("p tag"), ] }; let base = Timestamp::now().as_secs(); @@ -5561,6 +5705,238 @@ mod tests { drop_scratch_db(&admin, pool, &scratch_name).await; } + #[tokio::test] + #[ignore = "requires Postgres"] + async fn stale_legacy_roster_cannot_replace_new_locked_snapshot() { + use nostr::{EventBuilder, Keys, Kind, Tag, Timestamp}; + + let admin = PgPool::connect(&admin_url().await) + .await + .expect("connect admin"); + let (setup_pool, scratch_name) = create_scratch_db(&admin, "mixed_roster_writer").await; + let base_url = admin_url().await; + let slash = base_url.rfind('/').expect("database URL has path segment"); + let scratch_url = format!("{}/{}", &base_url[..slash], scratch_name); + let pool = PgPoolOptions::new() + .max_connections(1) + .acquire_timeout(Duration::from_secs(1)) + .connect(&scratch_url) + .await + .expect("connect one-connection scratch pool"); + setup_pool.close().await; + let db = Db::from_pool(pool.clone()); + let community_uuid = Uuid::new_v4(); + let community = CommunityId::from_uuid(community_uuid); + let channel = Uuid::new_v4(); + let relay_keys = Keys::generate(); + let owner_keys = Keys::generate(); + let owner = owner_keys.public_key().to_bytes(); + seed_community_channel(&pool, community_uuid, channel, &owner_keys).await; + + // This is the old pod's unlocked capture A. It remains in process memory + // while a role-only canonical mutation advances and the new pod publishes B. + let base = Timestamp::now().as_secs(); + let roster = |members: &[(&[u8], &str)], timestamp| { + let tags = + std::iter::once(Tag::parse(["d", channel.to_string().as_str()]).expect("d tag")) + .chain(members.iter().map(|(member, role)| { + Tag::parse(["p", hex::encode(member).as_str(), "", *role]).expect("p tag") + })) + .collect::>(); + EventBuilder::new(Kind::Custom(39002), "") + .tags(tags) + .custom_created_at(Timestamp::from(timestamp)) + .sign_with_keys(&relay_keys) + .expect("sign roster") + }; + + let newcomer = Keys::generate().public_key().to_bytes(); + sqlx::query( + "INSERT INTO channel_members (community_id, channel_id, pubkey, role, invited_by) \ + VALUES ($1, $2, $3, 'member', $4)", + ) + .bind(community_uuid) + .bind(channel) + .bind(newcomer.as_slice()) + .bind(owner.as_slice()) + .execute(&pool) + .await + .expect("seed member before legacy capture"); + let stale_a = roster( + &[(owner.as_slice(), "owner"), (newcomer.as_slice(), "member")], + base + 2, + ); + + sqlx::query( + "UPDATE channel_members SET role = 'admin' \ + WHERE community_id = $1 AND channel_id = $2 AND pubkey = $3", + ) + .bind(community_uuid) + .bind(channel) + .bind(newcomer.as_slice()) + .execute(&pool) + .await + .expect("commit newer canonical role"); + + let relay_pubkey = relay_keys.public_key().to_bytes(); + let mut snapshot = db + .lock_member_snapshot(community, channel, &relay_pubkey) + .await + .expect("new writer captures locked roster B"); + let fresh_b = roster( + &[(owner.as_slice(), "owner"), (newcomer.as_slice(), "admin")], + base + 1, + ); + assert!( + snapshot + .replace_member_event(community, channel, &fresh_b) + .await + .expect("new writer publishes B") + .1 + ); + snapshot + .release() + .await + .expect("commit B and release locks"); + + // The legacy canonical path takes the replacement key, soft-deletes B, + // then attempts its newer-timestamp stale A. Migration 0032 rejects the + // INSERT; transaction rollback must restore B. A one-connection pool + // proves the lock order does not turn this compatibility path into a + // self-deadlock. + let error = tokio::time::timeout( + Duration::from_secs(3), + db.replace_addressable_event(community, &stale_a, Some(channel)), + ) + .await + .expect("legacy replacement must not deadlock") + .expect_err("stale captured roster A must be rejected"); + assert!( + matches!( + error, + DbError::Sqlx(sqlx::Error::Database(ref db_error)) + if db_error.code().as_deref() == Some("23514") + ), + "expected roster fence check violation, got {error:?}" + ); + + let live_ids: Vec> = sqlx::query_scalar( + "SELECT id FROM events WHERE community_id=$1 AND channel_id=$2 \ + AND kind=39002 AND pubkey=$3 AND deleted_at IS NULL", + ) + .bind(community_uuid) + .bind(channel) + .bind(relay_pubkey.as_slice()) + .fetch_all(&pool) + .await + .expect("load live roster heads"); + assert_eq!(live_ids, vec![fresh_b.id.as_bytes().to_vec()]); + let stale_rows: i64 = + sqlx::query_scalar("SELECT count(*) FROM events WHERE community_id=$1 AND id=$2") + .bind(community_uuid) + .bind(stale_a.id.as_bytes().as_slice()) + .fetch_one(&pool) + .await + .expect("count rejected stale roster"); + assert_eq!(stale_rows, 0, "stale roster insert must roll back"); + + drop_scratch_db(&admin, pool, &scratch_name).await; + } + + #[tokio::test] + #[ignore = "requires Postgres"] + async fn desired_schema_rejects_stale_legacy_roster_role() { + use nostr::{EventBuilder, Keys, Kind, Tag, Timestamp}; + + let admin = PgPool::connect(&admin_url().await) + .await + .expect("connect admin"); + let scratch_name = format!("schema_roster_role_{}", Uuid::new_v4().simple()); + sqlx::query(sqlx::AssertSqlSafe(format!( + "CREATE DATABASE {scratch_name}" + ))) + .execute(&admin) + .await + .expect("create desired-schema scratch db"); + let base_url = admin_url().await; + let slash = base_url.rfind('/').expect("database URL has path segment"); + let scratch_url = format!("{}/{}", &base_url[..slash], scratch_name); + let pool = PgPoolOptions::new() + .max_connections(1) + .connect(&scratch_url) + .await + .expect("connect desired-schema scratch db"); + sqlx::raw_sql(include_str!("../../../schema/schema.sql")) + .execute(&pool) + .await + .expect("apply desired-state schema"); + + let db = Db::from_pool(pool.clone()); + let community_uuid = Uuid::new_v4(); + let community = CommunityId::from_uuid(community_uuid); + let channel = Uuid::new_v4(); + let relay_keys = Keys::generate(); + let owner_keys = Keys::generate(); + let owner = owner_keys.public_key().to_bytes(); + seed_community_channel(&pool, community_uuid, channel, &owner_keys).await; + let member = Keys::generate().public_key().to_bytes(); + sqlx::query( + "INSERT INTO channel_members (community_id, channel_id, pubkey, role, invited_by) \ + VALUES ($1, $2, $3, 'admin', $4)", + ) + .bind(community_uuid) + .bind(channel) + .bind(member.as_slice()) + .bind(owner.as_slice()) + .execute(&pool) + .await + .expect("seed canonical admin"); + + let roster = |role: &str, timestamp| { + EventBuilder::new(Kind::Custom(39002), "") + .tags(vec![ + Tag::parse(["d", channel.to_string().as_str()]).expect("d tag"), + Tag::parse(["p", hex::encode(owner).as_str(), "", "owner"]) + .expect("owner p tag"), + Tag::parse(["p", hex::encode(member).as_str(), "", role]) + .expect("member p tag"), + ]) + .custom_created_at(Timestamp::from(timestamp)) + .sign_with_keys(&relay_keys) + .expect("sign roster") + }; + let base = Timestamp::now().as_secs(); + let fresh = roster("admin", base); + assert!( + db.replace_addressable_event(community, &fresh, Some(channel)) + .await + .expect("publish canonical role") + .1 + ); + let stale = roster("member", base + 1); + let error = db + .replace_addressable_event(community, &stale, Some(channel)) + .await + .expect_err("desired-state fence must reject stale role"); + assert!(matches!( + error, + DbError::Sqlx(sqlx::Error::Database(ref db_error)) + if db_error.code().as_deref() == Some("23514") + )); + let live_id: Vec = sqlx::query_scalar( + "SELECT id FROM events WHERE community_id=$1 AND channel_id=$2 \ + AND kind=39002 AND deleted_at IS NULL", + ) + .bind(community_uuid) + .bind(channel) + .fetch_one(&pool) + .await + .expect("load desired-state live roster"); + assert_eq!(live_id, fresh.id.as_bytes().to_vec()); + + drop_scratch_db(&admin, pool, &scratch_name).await; + } + #[tokio::test] #[ignore = "requires Postgres"] async fn nip_rs_replacement_hard_deletes_payload_and_watermark_rejects_replay() { @@ -6917,9 +7293,12 @@ mod tests { std::env::var("TEST_DATABASE_URL").unwrap_or_else(|_| TEST_DB_URL.into()) } - /// Create a fresh scratch database on the same server and run migrations. - /// Returns (pool, db_name); callers should `drop_scratch_db` when done. - async fn create_scratch_db(admin: &PgPool, prefix: &str) -> (PgPool, String) { + /// Create a fresh scratch database on the same server and optionally run migrations. + async fn create_scratch_db_through( + admin: &PgPool, + prefix: &str, + target: Option, + ) -> (PgPool, String) { let name = format!("{}_{}", prefix, Uuid::new_v4().simple()); sqlx::query(sqlx::AssertSqlSafe(format!("CREATE DATABASE {name}"))) .execute(admin) @@ -6934,12 +7313,23 @@ mod tests { let pool = PgPool::connect(&scratch_url) .await .expect("connect scratch db"); - migration::run_migrations(&pool) - .await - .expect("migrate scratch db"); + match target { + Some(target) => migration::run_migrations_through(&pool, target) + .await + .expect("migrate scratch db through target"), + None => migration::run_migrations(&pool) + .await + .expect("migrate scratch db"), + } (pool, name) } + /// Create a fresh scratch database on the same server and run all migrations. + /// Returns (pool, db_name); callers should `drop_scratch_db` when done. + async fn create_scratch_db(admin: &PgPool, prefix: &str) -> (PgPool, String) { + create_scratch_db_through(admin, prefix, None).await + } + async fn drop_scratch_db(admin: &PgPool, pool: PgPool, name: &str) { pool.close().await; let _ = sqlx::query(sqlx::AssertSqlSafe(format!( diff --git a/crates/buzz-db/src/migration.rs b/crates/buzz-db/src/migration.rs index be87faa1ac2..94c7aea2faf 100644 --- a/crates/buzz-db/src/migration.rs +++ b/crates/buzz-db/src/migration.rs @@ -32,6 +32,20 @@ pub async fn run_migrations(pool: &PgPool) -> Result<()> { .await } +#[cfg(test)] +pub(crate) async fn run_migrations_through(pool: &PgPool, target: i64) -> Result<()> { + with_exclusive_schema_destruction_lock(pool, |mut conn| async move { + let outcome = async { + reject_legacy_nip_rs_cardinality_ambiguity(&mut conn).await?; + MIGRATOR.run_to(target, &mut conn).await?; + Ok(()) + } + .await; + (conn, outcome) + }) + .await +} + async fn run_migrations_locked(conn: &mut PgConnection) -> Result<()> { reject_legacy_nip_rs_cardinality_ambiguity(conn).await?; MIGRATOR.run(&mut *conn).await?; @@ -43,6 +57,7 @@ async fn run_migrations_locked(conn: &mut PgConnection) -> Result<()> { // guard, so migration fails closed if any is missing. (The fence probe // re-runs this same check at startup on non-migrating relays.) crate::replica_fence::verify_floor_guard_catalog(&mut *conn).await?; + crate::channel::verify_channel_roster_fence_catalog(&mut *conn).await?; Ok(()) } @@ -625,7 +640,7 @@ mod tests { let mut migrations: Vec<_> = MIGRATOR.iter().collect(); migrations.sort_by_key(|migration| migration.version); - assert_eq!(migrations.len(), 31); + assert_eq!(migrations.len(), 32); assert_eq!(migrations[0].version, 1); assert_eq!(&*migrations[0].description, "initial schema"); assert!(migrations[0] @@ -1036,6 +1051,36 @@ mod tests { assert_eq!(migrations[29].version, 30); let deletion_recovery = migrations[29].sql.as_str(); assert!(deletion_recovery.contains("SET LOCAL lock_timeout = '5s'")); + + // Mixed-version channel-roster fence: old canonical replacement writers + // acquire their replacement key before INSERT; this trigger then takes + // the membership key and validates the exact active pubkey/role p-tag set. + assert_eq!(migrations[31].version, 32); + let roster_fence = migrations[31].sql.as_str(); + assert!(roster_fence.contains("CREATE TRIGGER trg_events_guard_channel_roster_snapshot")); + assert!(roster_fence.contains("NEW.kind <> 39002")); + assert!(roster_fence.contains("'buzz_channel_membership:'")); + assert!(roster_fence.contains("cm.removed_at IS NULL")); + assert!(roster_fence.contains("cm.role::text")); + assert!(roster_fence.contains("jsonb_array_length(roster_tag.tag_json) <> 4")); + assert!(roster_fence.contains("roster_tag.tag_json->>3")); + assert!(roster_fence.contains("snapshot_members IS DISTINCT FROM canonical_members")); + assert!(roster_fence.contains("ERRCODE = '23514'")); + + // Fresh desired-state bootstrap must install the identical executable + // fence as migration 0032. CI and isolated relay startup use schema.sql + // without running migrations, so drift reopens rolling-deploy races. + fn extract_roster_fence(sql: &str) -> &str { + let fence_start = "CREATE OR REPLACE FUNCTION guard_channel_roster_snapshot()"; + let fence_end = " FOR EACH ROW EXECUTE FUNCTION guard_channel_roster_snapshot();"; + let start = sql.find(fence_start).expect("roster fence function"); + let relative_end = sql[start..].find(fence_end).expect("roster fence trigger"); + &sql[start..start + relative_end + fence_end.len()] + } + assert_eq!( + extract_roster_fence(roster_fence), + extract_roster_fence(desired_schema) + ); } #[test] @@ -1224,6 +1269,7 @@ mod tests { // Build the needles so this test's own source never matches them. let migrate_macro = ["sqlx", "::migrate!"].concat(); let migrator_run = ["MIGRATOR", ".run("].concat(); + let migrator_run_to = ["MIGRATOR", ".run_to("].concat(); let manifest_dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")); let this_file = manifest_dir.join("src/migration.rs"); @@ -1250,23 +1296,24 @@ mod tests { rust_sources(crates_dir, &mut files); for path in &files { let source = std::fs::read_to_string(path).expect("read rust source"); - let (macro_hits, run_hits) = ( + let (macro_hits, run_hits, run_to_hits) = ( count(&source, &migrate_macro), count(&source, &migrator_run), + count(&source, &migrator_run_to), ); if *path == this_file { assert_eq!( - (macro_hits, run_hits), - (1, 1), - "migration.rs must embed the migrator once and run it exactly once, \ - inside the locked wrapper" + (macro_hits, run_hits, run_to_hits), + (1, 1, 1), + "migration.rs must embed the migrator once, run it once in production, \ + and expose exactly one test-only bounded run" ); } else if *path == push_gateway_exception { continue; } else { assert_eq!( - (macro_hits, run_hits), - (0, 0), + (macro_hits, run_hits, run_to_hits), + (0, 0, 0), "{} embeds or runs a SQLx migrator outside the schema/destruction \ lock contract; route migration execution through \ buzz_db migration::run_migrations", @@ -1289,13 +1336,23 @@ mod tests { .find("async fn with_exclusive_schema_destruction_lock") .expect("exclusive lock wrapper"); let run_site = source.find(&migrator_run).expect("migrator run site"); + let run_to_site = source + .find(&migrator_run_to) + .expect("bounded test migrator run site"); assert!( source[entry..locked].contains("with_exclusive_schema_destruction_lock("), "run_migrations must delegate through the exclusive schema/destruction lock" ); assert!( run_site > locked && run_site < wrapper, - "the migrator run site must live inside run_migrations_locked" + "the production migrator run site must live inside run_migrations_locked" + ); + assert!( + run_to_site > entry + && run_to_site < locked + && source[entry..run_to_site].contains("#[cfg(test)]") + && source[entry..run_to_site].contains("with_exclusive_schema_destruction_lock("), + "the bounded migrator run must remain test-only and use the exclusive lock wrapper" ); assert!( source[wrapper..].contains("pg_advisory_lock($1)") diff --git a/crates/buzz-relay/src/handlers/side_effects.rs b/crates/buzz-relay/src/handlers/side_effects.rs index f2b58937ab5..89595fbee17 100644 --- a/crates/buzz-relay/src/handlers/side_effects.rs +++ b/crates/buzz-relay/src/handlers/side_effects.rs @@ -1049,6 +1049,55 @@ fn group_members_tags(group_id: &str, members: &[MemberRecord]) -> anyhow::Resul Ok(tags) } +async fn store_group_members_event( + tenant: &TenantContext, + state: &Arc, + channel_id: Uuid, + member_snapshot: &mut buzz_db::channel::LockedMemberSnapshot, +) -> anyhow::Result> { + let group_id = channel_id.to_string(); + let tags = group_members_tags(&group_id, &member_snapshot.members)?; + let relay_pubkey = state.relay_keypair.public_key().to_bytes(); + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_secs(); + let ts = member_snapshot + .latest_member_event_timestamp(tenant.community(), channel_id, &relay_pubkey) + .await? + .map(|timestamp| timestamp + 1) + .unwrap_or(now) + .max(now); + let event = EventBuilder::new(Kind::Custom(KIND_NIP29_GROUP_MEMBERS as u16), "") + .tags(tags) + .custom_created_at(nostr::Timestamp::from(ts)) + .sign_with_keys(&state.relay_keypair) + .map_err(|error| anyhow::anyhow!("failed to sign member snapshot: {error}"))?; + let (stored, inserted) = member_snapshot + .replace_member_event(tenant.community(), channel_id, &event) + .await?; + Ok(inserted.then_some(stored)) +} + +async fn dispatch_group_members_event( + tenant: &TenantContext, + state: &Arc, + stored: Option, + relay_pubkey_hex: &str, +) { + if let Some(stored) = stored { + dispatch_persistent_event( + tenant, + state, + &stored, + KIND_NIP29_GROUP_MEMBERS, + relay_pubkey_hex, + None, + ) + .await; + } +} + /// Emit NIP-29 group discovery events (39000, 39001, 39002) signed by the relay keypair. /// Called after group creation, metadata changes, or membership changes. /// Events are stored channel-scoped (`channel_id = Some(...)`) so that existing @@ -1151,18 +1200,18 @@ pub async fn emit_group_discovery_events( .await?; } - { - let tags = group_members_tags(&group_id, &members)?; - emit_addressable_discovery_event( - tenant, - state, - channel_id, - KIND_NIP29_GROUP_MEMBERS, - tags, - &relay_pubkey_hex, - ) + // Re-capture membership behind the writer lock immediately before the + // authoritative 39002 replacement. Metadata/admin snapshots retain their + // existing behavior; only membership publication needs this freshness fence. + let relay_pubkey = state.relay_keypair.public_key().to_bytes(); + let mut member_snapshot = state + .db + .lock_member_snapshot(tenant.community(), channel_id, &relay_pubkey) .await?; - } + let stored_members = + store_group_members_event(tenant, state, channel_id, &mut member_snapshot).await?; + member_snapshot.release().await?; + dispatch_group_members_event(tenant, state, stored_members, &relay_pubkey_hex).await; Ok(()) } @@ -3052,6 +3101,68 @@ pub async fn publish_nip43_member_removed( publish_nip43_delta(tenant, state, 8001, target_pubkey_hex, "member-removed").await } +/// Repair legacy kind:39002 snapshots truncated by the former 1,000-member +/// database cap. +/// +/// The scan is deliberately limited to canonical rosters above that boundary, +/// so normal-sized channels and already-correct large snapshots incur no +/// rewrites. Community identity travels with every candidate; a shared relay +/// never resolves a channel against a neighboring tenant. +pub async fn reconcile_large_channel_member_snapshots( + state: &Arc, +) -> anyhow::Result { + const LEGACY_ROSTER_LIMIT: i64 = 1_000; + + let relay_pubkey = state.relay_keypair.public_key(); + let candidates = state + .db + .list_large_channel_rosters_needing_reconciliation( + LEGACY_ROSTER_LIMIT, + &relay_pubkey.to_bytes(), + ) + .await?; + let relay_pubkey_hex = relay_pubkey.to_hex(); + let mut reconciled = 0usize; + + for candidate in candidates { + let result = async { + let channel_id = candidate.channel_id; + // Hold the membership-writer lock from roster capture through + // replacement. Otherwise a rolling deployment can publish stale + // roster A after another relay commits and publishes roster B. + let mut member_snapshot = state + .db + .lock_member_snapshot(candidate.community_id, channel_id, &relay_pubkey.to_bytes()) + .await?; + let tenant = TenantContext::resolved(candidate.community_id, candidate.host.clone()); + let stored_members = + store_group_members_event(&tenant, state, channel_id, &mut member_snapshot).await?; + member_snapshot.release().await?; + dispatch_group_members_event(&tenant, state, stored_members, &relay_pubkey_hex).await; + Ok::(true) + } + .await; + + match result { + Ok(true) => reconciled += 1, + Ok(false) => {} + Err(error) => { + metrics::counter!("buzz_channel_roster_reconciliation_failures_total").increment(1); + warn!( + community_id = %candidate.community_id, + host = %candidate.host, + channel_id = %candidate.channel_id, + %error, + "large channel roster reconciliation failed" + ); + } + } + } + + metrics::counter!("buzz_channel_roster_reconciliations_total").increment(reconciled as u64); + Ok(reconciled) +} + /// Reconcile channels that exist in the DB but don't have kind:39000 events. /// /// This handles the case where channels were created via direct SQL inserts diff --git a/crates/buzz-relay/src/main.rs b/crates/buzz-relay/src/main.rs index 3584e1849d1..566b684f830 100644 --- a/crates/buzz-relay/src/main.rs +++ b/crates/buzz-relay/src/main.rs @@ -534,6 +534,31 @@ async fn main() -> anyhow::Result<()> { ); } + match state.db.verify_channel_roster_fence().await { + Ok(()) => { + info!("Channel roster fence verified"); + } + Err(error) => { + error!(%error, "Channel roster fence validation failed"); + return Err(anyhow::anyhow!( + "Channel roster fence is unsafe; apply or repair migration 0032 before starting this relay: {error}" + )); + } + } + + // Repair legacy NIP-29 channel rosters that were persisted while the + // canonical member query still truncated at 1,000 rows. Validation above + // makes migration 0032 a code/schema compatibility gate before the new + // replacement protocol or listener can serve traffic. + match buzz_relay::handlers::side_effects::reconcile_large_channel_member_snapshots(&state).await + { + Ok(count) if count > 0 => info!(count, "large channel member snapshots repaired"), + Ok(_) => {} + Err(error) => { + tracing::warn!(%error, "large channel member snapshot startup reconciliation failed") + } + } + // NIP-43: reconcile the event-backed roster for every provisioned // community before opening the listener. `relay_members` is canonical; // this repairs pre-snapshot communities and any publication that failed diff --git a/deploy/charts/buzz/README.md b/deploy/charts/buzz/README.md index 86989676604..30cee4f4063 100644 --- a/deploy/charts/buzz/README.md +++ b/deploy/charts/buzz/README.md @@ -205,6 +205,8 @@ default so long-lived WebSocket connections have time to drain. Schema migrations are embedded in the relay binary via `sqlx::migrate!` and run at startup, gated by `BUZZ_AUTO_MIGRATE` (default `true`). Multiple replicas race-safely behind a Postgres advisory lock. `helm upgrade` is the entire upgrade procedure. +Migration 0032 is a hard compatibility boundary for relay versions that publish repaired channel rosters. The relay verifies the roster-fence trigger catalog and behavior before opening listeners and refuses to start if 0032 is missing or inert. Apply migrations before rolling the relay; for large installations, prefer a controlled `buzz-admin migrate` job with PostgreSQL lock monitoring before the code rollout. + If you prefer decoupling migrations from serving, set `migrate.autoMigrate=false`. **In that mode the chart does not run migrations for you** — you own running `buzz-admin migrate` (separate Pod / one-shot Job) against the database before every `helm install` / `helm upgrade`. Readiness probes only verify DB connectivity, not schema freshness, so a pod will appear healthy against an unmigrated schema and fail under load. A pre-upgrade Helm Job for this is on the chart roadmap; the values knob `migrate.preUpgradeJob.enabled` is reserved. ## Backups diff --git a/migrations/0032_channel_roster_snapshot_fence.sql b/migrations/0032_channel_roster_snapshot_fence.sql new file mode 100644 index 00000000000..cdc7bc4b93e --- /dev/null +++ b/migrations/0032_channel_roster_snapshot_fence.sql @@ -0,0 +1,76 @@ +-- Prevent mixed-version relay pods from publishing a stale NIP-29 member +-- snapshot after a newer canonical roster has been committed. +-- +-- Old binaries already serialize kind 39002 replacement on the replacement +-- advisory key. This trigger adds the channel-membership key at INSERT time, +-- after that canonical key, and validates every p tag against the current +-- active membership set and roles. New binaries take both keys in the same +-- order before capture and replacement. Thus old and new writers remain +-- compatible during a rolling deploy. +CREATE OR REPLACE FUNCTION guard_channel_roster_snapshot() +RETURNS TRIGGER AS $$ +DECLARE + canonical_members TEXT[]; + snapshot_members TEXT[]; +BEGIN + IF NEW.kind <> 39002 OR NEW.channel_id IS NULL THEN + RETURN NEW; + END IF; + + PERFORM pg_advisory_xact_lock(hashtextextended( + 'buzz_channel_membership:' || NEW.community_id::text || ':' || NEW.channel_id::text, + 0 + )); + + SELECT COALESCE( + array_agg(encode(cm.pubkey, 'hex') || ':' || cm.role::text ORDER BY cm.pubkey), + ARRAY[]::TEXT[] + ) + INTO canonical_members + FROM channel_members cm + WHERE cm.community_id = NEW.community_id + AND cm.channel_id = NEW.channel_id + AND cm.removed_at IS NULL; + + -- A roster is canonical only when every p tag uses the emitted four-field + -- shape, contains a 32-byte hex pubkey and valid authoritative role, has no + -- duplicate members, and exactly matches the active membership rows. + IF EXISTS ( + SELECT 1 + FROM jsonb_array_elements(NEW.tags) AS roster_tag(tag_json) + WHERE roster_tag.tag_json->>0 = 'p' + AND ( + jsonb_array_length(roster_tag.tag_json) <> 4 + OR COALESCE(roster_tag.tag_json->>1, '') !~ '^[0-9a-fA-F]{64}$' + OR roster_tag.tag_json->>2 <> '' + OR COALESCE(roster_tag.tag_json->>3, '') NOT IN ('owner', 'admin', 'bot', 'member', 'guest') + ) + ) THEN + RAISE EXCEPTION 'kind 39002 roster contains an invalid p tag' + USING ERRCODE = '23514'; + END IF; + + SELECT COALESCE( + array_agg( + lower((roster_tag.tag_json->>1)) || ':' || (roster_tag.tag_json->>3) + ORDER BY decode((roster_tag.tag_json->>1), 'hex') + ), + ARRAY[]::TEXT[] + ) + INTO snapshot_members + FROM jsonb_array_elements(NEW.tags) AS roster_tag(tag_json) + WHERE roster_tag.tag_json->>0 = 'p'; + + IF snapshot_members IS DISTINCT FROM canonical_members THEN + RAISE EXCEPTION 'kind 39002 roster does not match canonical channel membership' + USING ERRCODE = '23514'; + END IF; + + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +DROP TRIGGER IF EXISTS trg_events_guard_channel_roster_snapshot ON events; +CREATE TRIGGER trg_events_guard_channel_roster_snapshot + BEFORE INSERT ON events + FOR EACH ROW EXECUTE FUNCTION guard_channel_roster_snapshot(); diff --git a/schema/schema.sql b/schema/schema.sql index 9ef7bc0a4b8..6e14e6be1bf 100644 --- a/schema/schema.sql +++ b/schema/schema.sql @@ -990,6 +990,85 @@ AFTER INSERT ON events DEFERRABLE INITIALLY DEFERRED FOR EACH ROW EXECUTE FUNCTION refresh_channel_ttl_after_event_insert(); +-- Channel roster snapshot fence (keep in sync with migrations/0032). +-- Prevent mixed-version relay pods from publishing a stale NIP-29 member +-- snapshot after a newer canonical roster has been committed. +-- +-- Old binaries already serialize kind 39002 replacement on the replacement +-- advisory key. This trigger adds the channel-membership key at INSERT time, +-- after that canonical key, and validates every p tag against the current +-- active membership set and roles. New binaries take both keys in the same +-- order before capture and replacement. Thus old and new writers remain +-- compatible during a rolling deploy. +CREATE OR REPLACE FUNCTION guard_channel_roster_snapshot() +RETURNS TRIGGER AS $$ +DECLARE + canonical_members TEXT[]; + snapshot_members TEXT[]; +BEGIN + IF NEW.kind <> 39002 OR NEW.channel_id IS NULL THEN + RETURN NEW; + END IF; + + PERFORM pg_advisory_xact_lock(hashtextextended( + 'buzz_channel_membership:' || NEW.community_id::text || ':' || NEW.channel_id::text, + 0 + )); + + SELECT COALESCE( + array_agg(encode(cm.pubkey, 'hex') || ':' || cm.role::text ORDER BY cm.pubkey), + ARRAY[]::TEXT[] + ) + INTO canonical_members + FROM channel_members cm + WHERE cm.community_id = NEW.community_id + AND cm.channel_id = NEW.channel_id + AND cm.removed_at IS NULL; + + -- A roster is canonical only when every p tag uses the emitted four-field + -- shape, contains a 32-byte hex pubkey and valid authoritative role, has no + -- duplicate members, and exactly matches the active membership rows. + IF EXISTS ( + SELECT 1 + FROM jsonb_array_elements(NEW.tags) AS roster_tag(tag_json) + WHERE roster_tag.tag_json->>0 = 'p' + AND ( + jsonb_array_length(roster_tag.tag_json) <> 4 + OR COALESCE(roster_tag.tag_json->>1, '') !~ '^[0-9a-fA-F]{64}$' + OR roster_tag.tag_json->>2 <> '' + OR COALESCE(roster_tag.tag_json->>3, '') NOT IN ('owner', 'admin', 'bot', 'member', 'guest') + ) + ) THEN + RAISE EXCEPTION 'kind 39002 roster contains an invalid p tag' + USING ERRCODE = '23514'; + END IF; + + SELECT COALESCE( + array_agg( + lower((roster_tag.tag_json->>1)) || ':' || (roster_tag.tag_json->>3) + ORDER BY decode((roster_tag.tag_json->>1), 'hex') + ), + ARRAY[]::TEXT[] + ) + INTO snapshot_members + FROM jsonb_array_elements(NEW.tags) AS roster_tag(tag_json) + WHERE roster_tag.tag_json->>0 = 'p'; + + IF snapshot_members IS DISTINCT FROM canonical_members THEN + RAISE EXCEPTION 'kind 39002 roster does not match canonical channel membership' + USING ERRCODE = '23514'; + END IF; + + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +DROP TRIGGER IF EXISTS trg_events_guard_channel_roster_snapshot ON events; +CREATE TRIGGER trg_events_guard_channel_roster_snapshot + BEFORE INSERT ON events + FOR EACH ROW EXECUTE FUNCTION guard_channel_roster_snapshot(); + + -- Replica-fence floor guard (keep in sync with migrations/0021). A deferred -- constraint trigger re-checks, inside COMMIT processing, that channel-bearing -- event rows are no older than `buzz.created_at_floor` seconds before commit diff --git a/scripts/attach-schema-partitions.sql b/scripts/attach-schema-partitions.sql index 5837676f842..a67bb706b1b 100644 --- a/scripts/attach-schema-partitions.sql +++ b/scripts/attach-schema-partitions.sql @@ -16,12 +16,12 @@ BEGIN ) THEN -- pgschema may copy parent triggers onto standalone children. Drop -- those copies before ATTACH; PostgreSQL recreates inherited parent - -- triggers while attaching and rejects same-named child triggers - -- (both the push-match trigger and the replica-fence floor guard). + -- triggers while attaching and rejects same-named child triggers. DROP TRIGGER IF EXISTS events_enqueue_push_match ON events_p_past; DROP TRIGGER IF EXISTS events_refresh_channel_ttl ON events_p_past; DROP TRIGGER IF EXISTS events_created_at_floor ON events_p_past; DROP TRIGGER IF EXISTS community_write_fence_events ON events_p_past; + DROP TRIGGER IF EXISTS trg_events_guard_channel_roster_snapshot ON events_p_past; ALTER TABLE events ATTACH PARTITION events_p_past FOR VALUES FROM (MINVALUE) TO ('2026-01-01'); END IF; @@ -35,6 +35,7 @@ BEGIN DROP TRIGGER IF EXISTS events_refresh_channel_ttl ON events_p2026_01; DROP TRIGGER IF EXISTS events_created_at_floor ON events_p2026_01; DROP TRIGGER IF EXISTS community_write_fence_events ON events_p2026_01; + DROP TRIGGER IF EXISTS trg_events_guard_channel_roster_snapshot ON events_p2026_01; ALTER TABLE events ATTACH PARTITION events_p2026_01 FOR VALUES FROM ('2026-01-01') TO ('2026-02-01'); END IF; @@ -48,6 +49,7 @@ BEGIN DROP TRIGGER IF EXISTS events_refresh_channel_ttl ON events_p2026_02; DROP TRIGGER IF EXISTS events_created_at_floor ON events_p2026_02; DROP TRIGGER IF EXISTS community_write_fence_events ON events_p2026_02; + DROP TRIGGER IF EXISTS trg_events_guard_channel_roster_snapshot ON events_p2026_02; ALTER TABLE events ATTACH PARTITION events_p2026_02 FOR VALUES FROM ('2026-02-01') TO ('2026-03-01'); END IF; @@ -61,6 +63,7 @@ BEGIN DROP TRIGGER IF EXISTS events_refresh_channel_ttl ON events_p2026_03; DROP TRIGGER IF EXISTS events_created_at_floor ON events_p2026_03; DROP TRIGGER IF EXISTS community_write_fence_events ON events_p2026_03; + DROP TRIGGER IF EXISTS trg_events_guard_channel_roster_snapshot ON events_p2026_03; ALTER TABLE events ATTACH PARTITION events_p2026_03 FOR VALUES FROM ('2026-03-01') TO ('2026-04-01'); END IF; @@ -74,6 +77,7 @@ BEGIN DROP TRIGGER IF EXISTS events_refresh_channel_ttl ON events_p2026_04; DROP TRIGGER IF EXISTS events_created_at_floor ON events_p2026_04; DROP TRIGGER IF EXISTS community_write_fence_events ON events_p2026_04; + DROP TRIGGER IF EXISTS trg_events_guard_channel_roster_snapshot ON events_p2026_04; ALTER TABLE events ATTACH PARTITION events_p2026_04 FOR VALUES FROM ('2026-04-01') TO ('2026-05-01'); END IF; @@ -87,6 +91,7 @@ BEGIN DROP TRIGGER IF EXISTS events_refresh_channel_ttl ON events_p2026_05; DROP TRIGGER IF EXISTS events_created_at_floor ON events_p2026_05; DROP TRIGGER IF EXISTS community_write_fence_events ON events_p2026_05; + DROP TRIGGER IF EXISTS trg_events_guard_channel_roster_snapshot ON events_p2026_05; ALTER TABLE events ATTACH PARTITION events_p2026_05 FOR VALUES FROM ('2026-05-01') TO ('2026-06-01'); END IF; @@ -100,6 +105,7 @@ BEGIN DROP TRIGGER IF EXISTS events_refresh_channel_ttl ON events_p2026_06; DROP TRIGGER IF EXISTS events_created_at_floor ON events_p2026_06; DROP TRIGGER IF EXISTS community_write_fence_events ON events_p2026_06; + DROP TRIGGER IF EXISTS trg_events_guard_channel_roster_snapshot ON events_p2026_06; ALTER TABLE events ATTACH PARTITION events_p2026_06 FOR VALUES FROM ('2026-06-01') TO ('2026-07-01'); END IF; @@ -113,6 +119,7 @@ BEGIN DROP TRIGGER IF EXISTS events_refresh_channel_ttl ON events_p_future; DROP TRIGGER IF EXISTS events_created_at_floor ON events_p_future; DROP TRIGGER IF EXISTS community_write_fence_events ON events_p_future; + DROP TRIGGER IF EXISTS trg_events_guard_channel_roster_snapshot ON events_p_future; ALTER TABLE events ATTACH PARTITION events_p_future FOR VALUES FROM ('2026-07-01') TO (MAXVALUE); END IF; From 9891e64f6b8358d78aa85f2ba248310d58b51ec0 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Fri, 21 Aug 2026 21:07:36 +1000 Subject: [PATCH 02/33] fix(desktop): clarify add agents channel action (#6374) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Create agent" button takes you to a UI to **invite** your existing agents to a channel. Rewording the button to make this clear. Before Screenshot 2026-08-21 at 1 58 40 pm After (Sorry about different elements being in hover state in the screenshots) Screenshot 2026-08-21 at 1 58 16 pm ## Summary - Clarify that the empty-channel intro action adds existing agents to the channel. - Assert the exact action title and description in the existing E2E coverage while preserving the separate Welcome create-agent flow. ### Related issue None found. ### Testing - `../node_modules/.bin/biome check src/features/channels/ui/useChannelIntro.tsx tests/e2e/channels.spec.ts` passed. - `./node_modules/.bin/tsc && ./node_modules/.bin/vite build --mode e2e` passed. - The isolated Playwright smoke case `empty channel shows intro actions` passed (1/1) after installing the repo-pinned Chromium. - `env -u BUZZ_AGENT_PROVIDER just ci` passed. - Screenshots not captured; this is a copy-only UI change. --------- Signed-off-by: Matt Toohey --- desktop/src/features/channels/ui/useChannelIntro.tsx | 4 ++-- desktop/tests/e2e/channels.spec.ts | 11 +++++++++-- desktop/tests/e2e/tooltip-semantics.spec.ts | 2 +- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/desktop/src/features/channels/ui/useChannelIntro.tsx b/desktop/src/features/channels/ui/useChannelIntro.tsx index c22bbff2c28..19f2da0edbc 100644 --- a/desktop/src/features/channels/ui/useChannelIntro.tsx +++ b/desktop/src/features/channels/ui/useChannelIntro.tsx @@ -91,9 +91,9 @@ export function useChannelIntro({ if (!activeChannel.archivedAt && activeChannel.isMember) { if (onAddAgent) { actions.push({ - description: "Add an agent here.", + description: "Bring them in.", icon: , - label: "Create agent", + label: "Add agents", onClick: onAddAgent, testId: "channel-intro-action-create-agent", }); diff --git a/desktop/tests/e2e/channels.spec.ts b/desktop/tests/e2e/channels.spec.ts index 8a32a436c32..52d91dea3ed 100644 --- a/desktop/tests/e2e/channels.spec.ts +++ b/desktop/tests/e2e/channels.spec.ts @@ -1746,8 +1746,15 @@ test("empty channel shows intro actions", async ({ page }) => { await expect( page.getByTestId("channel-intro-action-create-channel"), ).toHaveCount(0); + const addAgentsAction = page.getByTestId("channel-intro-action-create-agent"); + await expect(addAgentsAction).toBeVisible(); await expect( - page.getByTestId("channel-intro-action-create-agent"), + addAgentsAction.getByText("Add agents", { exact: true }), + ).toBeVisible(); + await expect( + addAgentsAction.getByText("Bring them in.", { + exact: true, + }), ).toBeVisible(); await expect( page.getByTestId("channel-intro-action-add-people"), @@ -1767,7 +1774,7 @@ test("empty channel shows intro actions", async ({ page }) => { await page.keyboard.press("Escape"); await expect(page.getByTestId("members-sidebar")).not.toBeVisible(); - await page.getByTestId("channel-intro-action-create-agent").click(); + await addAgentsAction.click(); await expect(page.getByRole("heading", { name: "Add agents" })).toBeVisible(); await page.keyboard.press("Escape"); diff --git a/desktop/tests/e2e/tooltip-semantics.spec.ts b/desktop/tests/e2e/tooltip-semantics.spec.ts index 73306f025cf..90ccd061bfc 100644 --- a/desktop/tests/e2e/tooltip-semantics.spec.ts +++ b/desktop/tests/e2e/tooltip-semantics.spec.ts @@ -142,7 +142,7 @@ for (const theme of THEMES) { await expect(row).toBeVisible(); await expectMutedSupportingText( row.getByRole("button", { name: "Open channel general" }), - /Public channel · Active \d+[mhdw] ago/, + /Public channel · Active (just now|\d+[mhdw] ago)/, ); await page.mouse.move(0, 0); await expectMutedSupportingText( From 9b32e055fed45864e1982f3d99c5402ba35cd8a6 Mon Sep 17 00:00:00 2001 From: thomaspblock Date: Fri, 21 Aug 2026 08:45:55 -0400 Subject: [PATCH 03/33] polish(desktop): finish Projects navigation and context chrome (#6429) ## Summary After #6396, Projects still split chrome across the workspace header, a copy-link control, and a labeled Actions group that mixed people, create, and metadata. This PR finishes that surface: the right-hand context box is unlabeled actions plus a Details group, people stacks and contribution heatmaps are gone from that box, Create review sits with Create task, and the top chrome is terminal / chat / info with no copy-link. Sent project context collapses to a pill, and review file diffs keep the last good git view instead of flashing empty while queries refetch. This also lands the remaining navigation polish that followed Part 3: overview and list presentation, readme and commit layout, and opening the latest matching conversation from the Channels tab without leaving the project. ### Related issue N/A. Related: #6396 ## Testing - Walked Files, Tasks, Reviews, task/review detail, overview tabs, and chrome chat vs info in the running desktop app - Pre-push: desktop typecheck, unit tests, Tauri checks, and file-size gate passed - Updated Projects smoke specs for the new context groups, Create review, chrome order, and removed copy-link control - Merged current `origin/main`; one conflict in discussion-channel rows kept conversation-panel navigation and took main's bounded channel-name lookup ## Post-Deploy Monitoring & Validation - validate Projects workspace chrome, context box, and review file diffs in the first staging Desktop session - healthy signals: context box shows unlabeled actions then Details, chat toggle sits between terminal and info, review diffs stay populated across selection changes - failure signals: missing Create review, restored heatmap/people in the context box, or empty Files Changed while the review is still selected; mitigate by reverting this PR --------- Signed-off-by: Thomas Petersen --- desktop/src-tauri/src/lib.rs | 1 - .../lib/mentionHighlightExtension.test.mjs | 148 +++++ .../messages/lib/mentionHighlightExtension.ts | 434 ++++++++++++- .../messages/lib/useRichTextEditor.ts | 29 +- .../messages/ui/MessageThreadTranscript.tsx | 24 +- .../lib/projectDetailAgentContext.test.mjs | 41 +- .../projects/lib/projectDetailAgentContext.ts | 28 +- .../lib/projectReviewDisplay.test.mjs | 181 ++++++ .../projects/lib/projectReviewDisplay.ts | 101 +++ .../ui/AgentContextPayloadPreview.test.mjs | 23 +- .../ui/AgentContextPayloadPreview.tsx | 15 +- .../projects/ui/DiscussionChannels.tsx | 125 ++-- .../projects/ui/IssueAssigneesRow.tsx | 4 +- .../projects/ui/ProjectAgentChatPanel.tsx | 3 +- .../ProjectAgentSubmittedContextPill.test.mjs | 61 ++ .../ui/ProjectAgentSubmittedContextPill.tsx | 39 ++ .../src/features/projects/ui/ProjectCards.tsx | 43 +- .../projects/ui/ProjectCommitCopyButton.tsx | 17 +- .../projects/ui/ProjectCommitDetailPanel.tsx | 162 ++--- .../projects/ui/ProjectDetailChrome.tsx | 11 - .../projects/ui/ProjectDetailFeedPanels.tsx | 6 +- .../projects/ui/ProjectDetailScreen.tsx | 166 ++--- .../projects/ui/ProjectDetailSection.tsx | 19 +- .../projects/ui/ProjectEntityListRow.tsx | 11 +- .../ProjectPullRequestFilesChangedPanel.tsx | 18 +- .../projects/ui/ProjectPullRequestsPanel.tsx | 138 ++-- .../projects/ui/ProjectReadmePanel.tsx | 6 +- .../ui/ProjectRepositoryActionsPanel.tsx | 595 ++++++++---------- .../ui/ProjectRepositoryLatestCommitRow.tsx | 41 ++ .../projects/ui/ProjectRepositoryPanel.tsx | 46 +- .../projects/ui/ProjectRightPanelControls.tsx | 9 +- .../projects/ui/ProjectSectionHeader.tsx | 15 +- .../projects/ui/ProjectSelectableGroup.tsx | 49 +- .../ProjectWorkItemCommunicationActions.tsx | 9 +- .../ui/ProjectWorkItemContextActions.tsx | 14 +- .../ui/ProjectWorkItemContextDetails.tsx | 21 + .../projects/ui/ProjectWorkItemGroup.tsx | 4 +- .../projects/ui/ProjectWorkItemRow.tsx | 88 +-- .../projects/ui/ProjectWorkspaceTabs.tsx | 88 +-- .../projects/ui/ProjectsAgentPromptPage.tsx | 69 +- .../projects/ui/ProjectsChannelsList.tsx | 1 + .../projects/ui/ProjectsContributionGraph.tsx | 170 ----- .../projects/ui/ProjectsIssuesList.tsx | 1 + .../projects/ui/ProjectsListHeaderBar.tsx | 56 +- .../ui/ProjectsOverviewChromeActions.tsx | 62 ++ .../projects/ui/ProjectsOverviewPanel.tsx | 41 +- .../projects/ui/ProjectsOverviewRail.tsx | 9 - .../projects/ui/ProjectsPullRequestsList.tsx | 1 + .../src/features/projects/ui/ProjectsView.tsx | 88 +-- .../projects/ui/PullRequestMetaRail.tsx | 24 +- .../projects/ui/PullRequestReviewersRow.tsx | 116 ++-- .../projects/ui/PullRequestsPanelSurface.tsx | 62 ++ .../features/projects/ui/RepositoryCards.tsx | 28 +- .../ui/projectsOverviewContext.test.mjs | 15 +- .../projects/ui/projectsOverviewContext.ts | 36 +- .../ui/useRetainedProjectGitViews.test.mjs | 492 +++++++++++++++ .../projects/ui/useRetainedProjectGitViews.ts | 218 +++++++ .../src/shared/styles/globals/composer.css | 12 +- .../styles/globals/composerCaret.test.mjs | 18 + desktop/src/shared/ui/markdown.tsx | 41 +- .../ui/markdown/ChannelDeepLink.test.mjs | 11 + .../src/shared/ui/markdown/ImageMosaic.tsx | 28 + desktop/src/shared/ui/markdown/types.ts | 7 + desktop/src/shared/ui/markdownUtils.ts | 1 + desktop/tests/e2e/mentions.spec.ts | 83 ++- .../tests/e2e/project-commit-detail.spec.ts | 51 +- .../tests/e2e/project-issue-comments.spec.ts | 16 + desktop/tests/e2e/project-pr-review.spec.ts | 490 +++++++++++++-- .../tests/e2e/projects-v3-screenshots.spec.ts | 285 ++++++--- desktop/tests/e2e/sidebar.spec.ts | 41 ++ 70 files changed, 3932 insertions(+), 1474 deletions(-) create mode 100644 desktop/src/features/projects/lib/projectReviewDisplay.test.mjs create mode 100644 desktop/src/features/projects/lib/projectReviewDisplay.ts create mode 100644 desktop/src/features/projects/ui/ProjectAgentSubmittedContextPill.test.mjs create mode 100644 desktop/src/features/projects/ui/ProjectAgentSubmittedContextPill.tsx create mode 100644 desktop/src/features/projects/ui/ProjectRepositoryLatestCommitRow.tsx delete mode 100644 desktop/src/features/projects/ui/ProjectsContributionGraph.tsx create mode 100644 desktop/src/features/projects/ui/ProjectsOverviewChromeActions.tsx create mode 100644 desktop/src/features/projects/ui/PullRequestsPanelSurface.tsx create mode 100644 desktop/src/features/projects/ui/useRetainedProjectGitViews.test.mjs create mode 100644 desktop/src/features/projects/ui/useRetainedProjectGitViews.ts create mode 100644 desktop/src/shared/styles/globals/composerCaret.test.mjs create mode 100644 desktop/src/shared/ui/markdown/ImageMosaic.tsx diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index c120ac12679..e1b8a9551b1 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -914,7 +914,6 @@ pub fn run() { RunEvent::Exit => { shut_down_app(app_handle, &run_shutdown_done); app_handle.state::().release(); - #[cfg(all(feature = "mesh-llm", target_os = "macos"))] if restart_requested.load(Ordering::SeqCst) { relaunch_after_mesh_shutdown(app_handle); diff --git a/desktop/src/features/messages/lib/mentionHighlightExtension.test.mjs b/desktop/src/features/messages/lib/mentionHighlightExtension.test.mjs index 164225fd80d..2747bdb1864 100644 --- a/desktop/src/features/messages/lib/mentionHighlightExtension.test.mjs +++ b/desktop/src/features/messages/lib/mentionHighlightExtension.test.mjs @@ -1,9 +1,19 @@ import assert from "node:assert/strict"; import test from "node:test"; +import { getSchema } from "@tiptap/core"; +import StarterKit from "@tiptap/starter-kit"; + import { + assignMentionHighlightNames, buildHighlightPatterns, + createMentionCaretSettlement, findHighlightMatches, + insertPosForMentionTextInput, + mentionTextInputInsertPos, + positionAfterArrowLeftThroughMentionSpace, + selectionAfterMentionTrailingSpace, + shouldAdvanceMentionCaret, } from "./mentionHighlightExtension.ts"; // ── buildHighlightPatterns ──────────────────────────────────────────── @@ -164,3 +174,141 @@ test("#general should NOT match inside #generally (trailing word boundary)", () const matches = findHighlightMatches("#generally", patterns); assert.equal(matches.length, 0); }); + +const schema = getSchema([ + StarterKit.configure({ + heading: false, + trailingNode: false, + link: false, + }), +]); +const paragraph = (...content) => schema.nodes.paragraph.create(null, content); +const text = (value) => schema.text(value); +const document = (...content) => schema.nodes.doc.create(null, content); + +test("selectionAfterMentionTrailingSpace steps past the space after @Name", () => { + const doc = document(paragraph(text("@quinn "))); + const spacePos = 1 + "@quinn".length; + assert.equal(selectionAfterMentionTrailingSpace(doc, spacePos), spacePos + 1); + assert.equal( + selectionAfterMentionTrailingSpace(doc, spacePos + 1), + spacePos + 1, + ); +}); + +test("selectionAfterMentionTrailingSpace leaves a caret inside the mention name", () => { + const doc = document(paragraph(text("@quinn "))); + assert.equal(selectionAfterMentionTrailingSpace(doc, 4), 4); +}); + +test("selectionAfterMentionTrailingSpace does not move without a trailing space", () => { + const doc = document(paragraph(text("@quinn"))); + const end = 1 + "@quinn".length; + assert.equal(selectionAfterMentionTrailingSpace(doc, end), end); +}); + +test("shouldAdvanceMentionCaret restores a remap while this editor is settling", () => { + assert.equal( + shouldAdvanceMentionCaret({ + from: 7, + next: 8, + settling: true, + docChanged: false, + }), + true, + ); +}); + +test("shouldAdvanceMentionCaret does not steal ArrowLeft after settlement is cancelled", () => { + assert.equal( + shouldAdvanceMentionCaret({ + from: 7, + next: 8, + settling: false, + docChanged: false, + }), + false, + ); +}); + +test("shouldAdvanceMentionCaret still advances after a document change", () => { + assert.equal( + shouldAdvanceMentionCaret({ + from: 7, + next: 8, + settling: false, + docChanged: true, + }), + true, + ); +}); + +test("createMentionCaretSettlement keeps two editors independent", () => { + const composerA = createMentionCaretSettlement(); + const composerB = createMentionCaretSettlement(); + composerA.arm(8); + assert.equal(composerB.peek(), null); + composerB.arm(12); + composerA.cancel(); + assert.equal(composerA.peek(), null); + assert.equal(composerB.peek(), 12); +}); + +test("insertPosForMentionTextInput redirects a caret at the chip edge", () => { + const doc = document(paragraph(text("@quinn "))); + const spacePos = 1 + "@quinn".length; + assert.equal( + insertPosForMentionTextInput(doc, spacePos, spacePos), + spacePos + 1, + ); + assert.equal( + insertPosForMentionTextInput(doc, spacePos + 1, spacePos + 1), + null, + ); +}); + +test("insertPosForMentionTextInput keeps a selected trailing space", () => { + const doc = document(paragraph(text("@quinn "))); + const spacePos = 1 + "@quinn".length; + assert.equal( + insertPosForMentionTextInput(doc, spacePos, spacePos + 1), + spacePos + 1, + ); +}); + +test("mentionTextInputInsertPos honors a deliberate caret after settlement", () => { + const doc = document(paragraph(text("@bob "))); + const spacePos = 1 + "@bob".length; + assert.equal(mentionTextInputInsertPos(doc, spacePos, spacePos, false), null); + assert.equal( + mentionTextInputInsertPos(doc, spacePos, spacePos, true), + spacePos + 1, + ); +}); + +test("positionAfterArrowLeftThroughMentionSpace steps onto the token end", () => { + const doc = document(paragraph(text("@bob "))); + const afterSpace = 1 + "@bob".length + 1; + assert.equal( + positionAfterArrowLeftThroughMentionSpace(doc, afterSpace), + afterSpace - 1, + ); + assert.equal( + positionAfterArrowLeftThroughMentionSpace(doc, afterSpace - 1), + null, + ); +}); + +test("assignMentionHighlightNames skips an unchanged list", () => { + const storage = { names: ["bob"], agentNames: [], channelNames: [] }; + assert.equal(assignMentionHighlightNames(storage, ["bob"], [], []), false); +}); + +test("assignMentionHighlightNames updates when a new mention is added", () => { + const storage = { names: ["bob"], agentNames: [], channelNames: [] }; + assert.equal( + assignMentionHighlightNames(storage, ["bob", "quinn"], [], []), + true, + ); + assert.deepEqual(storage.names, ["bob", "quinn"]); +}); diff --git a/desktop/src/features/messages/lib/mentionHighlightExtension.ts b/desktop/src/features/messages/lib/mentionHighlightExtension.ts index a8d3ef8ff0a..f20feef26da 100644 --- a/desktop/src/features/messages/lib/mentionHighlightExtension.ts +++ b/desktop/src/features/messages/lib/mentionHighlightExtension.ts @@ -1,5 +1,11 @@ import { Extension } from "@tiptap/core"; -import { Plugin, PluginKey, type Transaction } from "@tiptap/pm/state"; +import type { Node as ProseMirrorNode } from "@tiptap/pm/model"; +import { + Plugin, + PluginKey, + TextSelection, + type Transaction, +} from "@tiptap/pm/state"; import { Decoration, DecorationSet } from "@tiptap/pm/view"; import { @@ -9,6 +15,238 @@ import { export const mentionHighlightKey = new PluginKey("mentionHighlight"); +export type MentionCaretSettlement = { + arm: (pos: number) => void; + peek: () => number | null; + cancel: () => void; +}; + +export function createMentionCaretSettlement(): MentionCaretSettlement { + let pos: number | null = null; + return { + arm(nextPos: number) { + pos = nextPos; + }, + peek() { + return pos; + }, + cancel() { + pos = null; + }, + }; +} + +/** + * Whether to move an empty caret from `from` to `next` after a mention + * trailing space. Settlement is per editor: autocomplete arms it, and + * ArrowLeft/click cancel it so we do not steal an intentional caret. + */ +export function shouldAdvanceMentionCaret({ + from, + next, + settling, + docChanged, +}: { + from: number; + next: number; + settling: boolean; + docChanged: boolean; +}): boolean { + return next !== from && (settling || docChanged); +} + +/** + * Where to insert typed text when the caret (or a one-character selection) + * sits on the trailing space after an `@name` / `#channel` token. + * A selected trailing space would otherwise be replaced, producing + * `@bobhello`. + */ +export function insertPosForMentionTextInput( + doc: ProseMirrorNode, + from: number, + to: number, +): number | null { + const next = selectionAfterMentionTrailingSpace(doc, from); + if (from === to) { + return next === from ? null : next; + } + if (to === next && next === from + 1) { + return next; + } + return null; +} + +/** + * Redirect chip-edge typing only while autocomplete is settling. After a + * deliberate ArrowLeft or chip click, honor the caret so `x` lands in the + * token (`@bobx`) instead of after the space (`@bob x`). + */ +export function mentionTextInputInsertPos( + doc: ProseMirrorNode, + from: number, + to: number, + settling: boolean, +): number | null { + if (!settling) return null; + return insertPosForMentionTextInput(doc, from, to); +} + +/** Caret just after a mention trailing space: ArrowLeft lands on the token end. */ +export function positionAfterArrowLeftThroughMentionSpace( + doc: ProseMirrorNode, + from: number, +): number | null { + if (from <= 0) return null; + const chipEnd = from - 1; + if (selectionAfterMentionTrailingSpace(doc, chipEnd) === from) { + return chipEnd; + } + return null; +} + +export function setDomCaretAtPos( + view: { + domAtPos: (pos: number) => { node: Node; offset: number }; + root: Document | ShadowRoot; + }, + pos: number, +): void { + if (typeof document === "undefined") return; + let mapped: { node: Node; offset: number }; + try { + mapped = view.domAtPos(pos); + } catch { + return; + } + const range = document.createRange(); + try { + range.setStart(mapped.node, mapped.offset); + } catch { + return; + } + range.collapse(true); + const root = view.root; + const selection = + "getSelection" in root && typeof root.getSelection === "function" + ? root.getSelection() + : window.getSelection(); + if (!selection) return; + selection.removeAllRanges(); + selection.addRange(range); +} + +export function reassertMentionCaretAfterFocus(view: { + state: { + doc: ProseMirrorNode; + selection: { empty: boolean; from: number }; + tr: Transaction; + }; + dispatch: (tr: Transaction) => void; + domAtPos: (pos: number) => { node: Node; offset: number }; + root: Document | ShadowRoot; +}): void { + if (!view.state.selection.empty) return; + const from = view.state.selection.from; + const next = selectionAfterMentionTrailingSpace(view.state.doc, from); + if (next !== from) { + view.dispatch( + view.state.tr.setSelection(TextSelection.create(view.state.doc, next)), + ); + } + setDomCaretAtPos(view, view.state.selection.from); +} + +export type MentionHighlightStorage = { + names: string[]; + agentNames: string[]; + channelNames: string[]; +}; + +function sameNameList(current: string[], next: string[]): boolean { + return ( + current.length === next.length && + current.every((name, index) => name === next[index]) + ); +} + +export function assignMentionHighlightNames( + storage: MentionHighlightStorage, + names: string[], + agentNames: string[], + channelNames: string[], +): boolean { + if ( + sameNameList(storage.names, names) && + sameNameList(storage.agentNames, agentNames) && + sameNameList(storage.channelNames, channelNames) + ) { + return false; + } + storage.names = names; + storage.agentNames = agentNames; + storage.channelNames = channelNames; + return true; +} + +export function mentionHighlightStorage(editor: { + storage: object; +}): MentionHighlightStorage | undefined { + if (!("mentionHighlight" in editor.storage)) return undefined; + return editor.storage.mentionHighlight as MentionHighlightStorage; +} + +export function settleAutocompleteMentionInsert( + editor: { storage: object }, + tr: Transaction, + text: string, +): void { + const storage = mentionHighlightStorage(editor); + const mentionInsert = /(?:^|[\s(])([@#])([^\s]+) $/.exec(text); + if (!mentionInsert) return; + const prefix = mentionInsert[1]; + const label = mentionInsert[2]; + if (storage) { + const known = [ + ...storage.names, + ...storage.agentNames, + ...storage.channelNames, + ]; + if (!known.some((name) => name.toLowerCase() === label.toLowerCase())) { + if (prefix === "#") { + storage.channelNames = [...storage.channelNames, label]; + } else { + storage.names = [...storage.names, label]; + } + } + } + tr.setMeta(mentionHighlightKey, true); +} + +export function syncMentionHighlightFromProps( + editor: { + storage: object; + state: { tr: Transaction }; + view: { dispatch: (tr: Transaction) => void }; + }, + names: string[] | undefined, + agentNames: string[] | undefined, + channelNames: string[] | undefined, +): void { + const storage = mentionHighlightStorage(editor); + if ( + !storage || + !assignMentionHighlightNames( + storage, + names ?? [], + agentNames ?? [], + channelNames ?? [], + ) + ) { + return; + } + editor.view.dispatch(editor.state.tr.setMeta(mentionHighlightKey, true)); +} + /** * TipTap extension that applies inline `mention-chip` decorations * to `@Name` and `#channel-name` patterns in the document. @@ -29,6 +267,7 @@ export const MentionHighlightExtension = Extension.create({ addProseMirrorPlugins() { const extension = this; + const settlement = createMentionCaretSettlement(); return [ new Plugin({ @@ -43,6 +282,16 @@ export const MentionHighlightExtension = Extension.create({ ); }, apply(tr, oldDecorations) { + if ( + tr.getMeta(mentionHighlightKey) && + tr.selection.empty && + (tr.docChanged || settlement.peek() !== null) + ) { + settlement.arm( + selectionAfterMentionTrailingSpace(tr.doc, tr.selection.from), + ); + } + // Names/channels changed — full rebuild required. if (tr.getMeta(mentionHighlightKey)) { return buildDecorations( @@ -85,10 +334,131 @@ export const MentionHighlightExtension = Extension.create({ return oldDecorations.map(tr.mapping, tr.doc); }, }, + appendTransaction(transactions, _oldState, newState) { + if (!newState.selection.empty) { + settlement.cancel(); + return null; + } + const from = newState.selection.from; + const next = selectionAfterMentionTrailingSpace(newState.doc, from); + if ( + !shouldAdvanceMentionCaret({ + from, + next, + settling: settlement.peek() !== null, + docChanged: transactions.some((tr) => tr.docChanged), + }) + ) { + return null; + } + return newState.tr.setSelection( + TextSelection.create(newState.doc, next), + ); + }, + view() { + let applying = false; + return { + update(view) { + if (applying || settlement.peek() === null) return; + if (!view.state.selection.empty) { + settlement.cancel(); + return; + } + const from = view.state.selection.from; + const next = selectionAfterMentionTrailingSpace( + view.state.doc, + from, + ); + if (next !== from) { + applying = true; + try { + view.dispatch( + view.state.tr.setSelection( + TextSelection.create(view.state.doc, next), + ), + ); + } finally { + applying = false; + } + } + setDomCaretAtPos(view, view.state.selection.from); + }, + destroy() { + settlement.cancel(); + }, + }; + }, props: { decorations(state) { return this.getState(state) ?? DecorationSet.empty; }, + handleTextInput(view, from, to, text) { + const insertAt = mentionTextInputInsertPos( + view.state.doc, + from, + to, + settlement.peek() !== null, + ); + if (insertAt == null) { + settlement.cancel(); + return false; + } + const tr = view.state.tr.insertText(text, insertAt); + const caret = tr.mapping.map(insertAt, 1); + tr.setSelection(TextSelection.create(tr.doc, caret)); + view.dispatch(tr); + settlement.cancel(); + setDomCaretAtPos(view, caret); + return true; + }, + handleKeyDown(view, event) { + if ( + event.key === "ArrowRight" || + event.key === "ArrowUp" || + event.key === "ArrowDown" || + event.key === "Home" || + event.key === "End" + ) { + settlement.cancel(); + return false; + } + if (event.key !== "ArrowLeft" || !view.state.selection.empty) { + return false; + } + settlement.cancel(); + const chipEnd = positionAfterArrowLeftThroughMentionSpace( + view.state.doc, + view.state.selection.from, + ); + if (chipEnd == null) return false; + view.dispatch( + view.state.tr.setSelection( + TextSelection.create(view.state.doc, chipEnd), + ), + ); + setDomCaretAtPos(view, chipEnd); + return true; + }, + handleClick(view, pos, event) { + const target = event.target; + const onChip = + target instanceof Element && + Boolean(target.closest(".mention-chip")); + settlement.cancel(); + if (!onChip) return false; + const chipEnd = positionAfterArrowLeftThroughMentionSpace( + view.state.doc, + pos, + ); + if (chipEnd == null) return false; + view.dispatch( + view.state.tr.setSelection( + TextSelection.create(view.state.doc, chipEnd), + ), + ); + setDomCaretAtPos(view, chipEnd); + return true; + }, }, }), ]; @@ -136,6 +506,28 @@ export function buildHighlightPatterns( return patterns; } +/** + * If `pos` sits at the end of an `@name` / `#channel` token and the next + * character is a space, return the position after that space. + * + * Autocomplete inserts `@Name ` then chip decorations wrap the token. The + * browser can map the caret back to the chip edge, so the next keystroke + * lands before the space (`@quinnhello`). Callers use this to keep typing + * after the token. + */ +export function selectionAfterMentionTrailingSpace( + doc: ProseMirrorNode, + pos: number, +): number { + if (pos < 0 || pos >= doc.content.size) return pos; + const nextChar = doc.textBetween(pos, pos + 1, "\n", "\0"); + if (nextChar !== " ") return pos; + const lookbehind = Math.min(pos, 80); + const before = doc.textBetween(pos - lookbehind, pos, "\n", "\0"); + if (!/(?:^|[\s(])[@#][^\s]+$/.test(before)) return pos; + return pos + 1; +} + /** * Find all highlight matches in a text string given a set of patterns. * Returns an array of { from, to } offsets relative to the text start. @@ -310,25 +702,41 @@ function addMatchesForPatterns( while (match !== null) { const from = position + match.index; const to = from + match[0].length; + const outsideEnd = { inclusiveEnd: false }; if (options?.hidePrefix && /^[@#]/.test(match[0])) { decorations.push( - Decoration.inline(from, from + 1, { - class: "mention-prefix-hidden", - spellcheck: "false", - }), + Decoration.inline( + from, + from + 1, + { + class: "mention-prefix-hidden", + spellcheck: "false", + }, + outsideEnd, + ), ); decorations.push( - Decoration.inline(from + 1, to, { - class: className, - spellcheck: "false", - }), + Decoration.inline( + from + 1, + to, + { + class: className, + spellcheck: "false", + }, + outsideEnd, + ), ); } else { decorations.push( - Decoration.inline(from, to, { - class: className, - spellcheck: "false", - }), + Decoration.inline( + from, + to, + { + class: className, + spellcheck: "false", + }, + outsideEnd, + ), ); } match = pattern.exec(text); diff --git a/desktop/src/features/messages/lib/useRichTextEditor.ts b/desktop/src/features/messages/lib/useRichTextEditor.ts index f812eb91156..36a75f32e5d 100644 --- a/desktop/src/features/messages/lib/useRichTextEditor.ts +++ b/desktop/src/features/messages/lib/useRichTextEditor.ts @@ -24,7 +24,9 @@ import { MESSAGE_MARKDOWN_CLASS } from "@/shared/ui/mentionChip"; import { MentionHighlightExtension, - mentionHighlightKey, + reassertMentionCaretAfterFocus, + settleAutocompleteMentionInsert, + syncMentionHighlightFromProps, } from "./mentionHighlightExtension"; import { CUSTOM_EMOJI_NODE_NAME } from "./customEmojiNode"; import { useComposerCustomEmoji } from "./useComposerCustomEmoji"; @@ -688,24 +690,15 @@ export function useRichTextEditor({ }, [editor, placeholder]); // Keep mention/channel-highlight decorations in sync with known names. - // NOTE: We use `editor.storage.mentionHighlight` (the mutable storage object - // shared with the ProseMirror plugin closure) rather than finding the - // extension instance via extensionManager — the instance's `.storage` getter - // returns a fresh spread-copy on every access, so mutations are silently lost. + // Mutate `editor.storage.mentionHighlight`; the extension getter copies storage. React.useEffect(() => { if (!editor) return; - // biome-ignore lint/suspicious/noExplicitAny: TipTap's Storage type doesn't include dynamic extension keys - const storage = (editor.storage as any).mentionHighlight as - | { names: string[]; agentNames: string[]; channelNames: string[] } - | undefined; - if (storage) { - storage.names = mentionNames ?? []; - storage.agentNames = agentMentionNames ?? []; - storage.channelNames = channelNames ?? []; - // Force the plugin to re-decorate by dispatching a metadata transaction. - const { tr } = editor.state; - editor.view.dispatch(tr.setMeta(mentionHighlightKey, true)); - } + syncMentionHighlightFromProps( + editor, + mentionNames, + agentMentionNames, + channelNames, + ); }, [editor, mentionNames, agentMentionNames, channelNames]); // Custom-emoji set changes: re-resolve the `src` attr on any existing @@ -872,8 +865,10 @@ export function useRichTextEditor({ // "Position N out of range".) const cursorPM = tr.mapping.map(toPM); tr.setSelection(TextSelection.create(tr.doc, cursorPM)); + settleAutocompleteMentionInsert(editor, tr, text); editor.view.dispatch(tr); editor.view.focus(); + reassertMentionCaretAfterFocus(editor.view); }, [editor, customEmojiWiring.resolveUrl], ); diff --git a/desktop/src/features/messages/ui/MessageThreadTranscript.tsx b/desktop/src/features/messages/ui/MessageThreadTranscript.tsx index fceda286578..7460f9aad3c 100644 --- a/desktop/src/features/messages/ui/MessageThreadTranscript.tsx +++ b/desktop/src/features/messages/ui/MessageThreadTranscript.tsx @@ -21,6 +21,7 @@ type MessageThreadTranscriptProps = { remove: boolean, ) => Promise; profiles?: UserProfileLookup; + renderAfterMessage?: (message: TimelineMessage) => React.ReactNode; testId?: string; }; @@ -36,6 +37,7 @@ export function MessageThreadTranscript({ messages, onToggleReaction, profiles, + renderAfterMessage, testId = "message-thread-transcript", }: MessageThreadTranscriptProps) { const renderItems = React.useMemo(() => { @@ -59,16 +61,18 @@ export function MessageThreadTranscript({ data-testid={testId} > {renderItems.map(({ isContinuation, message }) => ( - + + + {renderAfterMessage?.(message)} + ))} ); diff --git a/desktop/src/features/projects/lib/projectDetailAgentContext.test.mjs b/desktop/src/features/projects/lib/projectDetailAgentContext.test.mjs index 7febdf8b678..36066132c42 100644 --- a/desktop/src/features/projects/lib/projectDetailAgentContext.test.mjs +++ b/desktop/src/features/projects/lib/projectDetailAgentContext.test.mjs @@ -6,6 +6,7 @@ import { buildProjectSelectionAgentContext, buildProjectsOverviewAgentContext, projectDetailAgentContextBlock, + splitProjectDetailAgentContext, stripProjectDetailAgentContext, untrustedPromptValue, withProjectSelectionAgentContext, @@ -215,8 +216,44 @@ test("selected project context enforces a final serialization budget", () => { }); test("strips hidden page context from the displayed user message", () => { - const content = `Explain this file${projectDetailAgentContextBlock( + const payload = projectDetailAgentContextBlock( buildProjectDetailAgentContext(base), - )}`; + ); + const content = `Explain this file${payload}`; assert.equal(stripProjectDetailAgentContext(content), "Explain this file"); + assert.deepEqual(splitProjectDetailAgentContext(content), { + context: payload.trim(), + message: "Explain this file", + }); +}); + +test("leaves ordinary messages unchanged without inventing context", () => { + assert.deepEqual(splitProjectDetailAgentContext("A normal message"), { + context: null, + message: "A normal message", + }); +}); + +test("splits only the final appended context marker", () => { + const userMessage = + "Discuss this literal example:\n---\nCurrent Buzz project page:\nnot appended"; + const payload = projectDetailAgentContextBlock( + buildProjectDetailAgentContext(base), + ); + assert.deepEqual(splitProjectDetailAgentContext(`${userMessage}${payload}`), { + context: payload.trim(), + message: userMessage, + }); +}); + +test("splits workspace repository context for the shared conversation view", () => { + const payload = + '\n---\nWorkspace repositories:\n- "Buzz" (address: "owner:buzz")'; + assert.deepEqual( + splitProjectDetailAgentContext(`Compare the repos${payload}`), + { + context: payload.trim(), + message: "Compare the repos", + }, + ); }); diff --git a/desktop/src/features/projects/lib/projectDetailAgentContext.ts b/desktop/src/features/projects/lib/projectDetailAgentContext.ts index 0194bad3e18..97142157007 100644 --- a/desktop/src/features/projects/lib/projectDetailAgentContext.ts +++ b/desktop/src/features/projects/lib/projectDetailAgentContext.ts @@ -5,6 +5,12 @@ import { } from "./projectSelection.ts"; const PROJECT_PAGE_CONTEXT_MARKER = "Current Buzz project page:"; +/** Marker for the repository set appended by the full Projects agent page. */ +export const PROJECT_WORKSPACE_CONTEXT_MARKER = "Workspace repositories:"; +const PROJECT_AGENT_CONTEXT_MARKERS = [ + PROJECT_PAGE_CONTEXT_MARKER, + PROJECT_WORKSPACE_CONTEXT_MARKER, +]; const MAX_OVERVIEW_CONTEXT_ITEMS = 200; const MAX_OVERVIEW_CONTEXT_FIELD_LENGTH = 180; const MAX_SELECTION_CONTEXT_ITEMS = 100; @@ -323,8 +329,24 @@ function overviewContextField(value: string | null | undefined) { return normalizedPromptValue(value, MAX_OVERVIEW_CONTEXT_FIELD_LENGTH); } +export function splitProjectDetailAgentContext(content: string): { + context: string | null; + message: string; +} { + const markerIndex = Math.max( + ...PROJECT_AGENT_CONTEXT_MARKERS.map((marker) => + content.lastIndexOf(`---\n${marker}`), + ), + ); + if (markerIndex === -1) { + return { context: null, message: content }; + } + return { + context: content.slice(markerIndex).trim(), + message: content.slice(0, markerIndex).replace(/\n+$/, ""), + }; +} + export function stripProjectDetailAgentContext(content: string) { - const markerIndex = content.indexOf(`---\n${PROJECT_PAGE_CONTEXT_MARKER}`); - if (markerIndex === -1) return content; - return content.slice(0, markerIndex).replace(/\n+$/, ""); + return splitProjectDetailAgentContext(content).message; } diff --git a/desktop/src/features/projects/lib/projectReviewDisplay.test.mjs b/desktop/src/features/projects/lib/projectReviewDisplay.test.mjs new file mode 100644 index 00000000000..2f89decab42 --- /dev/null +++ b/desktop/src/features/projects/lib/projectReviewDisplay.test.mjs @@ -0,0 +1,181 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + currentPullRequestForSelection, + projectReviewFilesChangedBody, + retainLatestByKey, + reviewDiffWorkspaceBranch, + shouldReplaceRetainedPullRequest, +} from "./projectReviewDisplay.ts"; + +test("retainLatestByKey keeps the previous value when shouldReplace is false", () => { + const cache = { current: { key: "pr-1", value: { files: [1] } } }; + + const retained = retainLatestByKey( + cache, + "pr-1", + { files: [] }, + (next, previous) => + next.files.length > 0 ? true : previous.files.length === 0, + ); + + assert.deepEqual(retained, { files: [1] }); + assert.deepEqual(cache.current.value, { files: [1] }); +}); + +test("retainLatestByKey takes a new key immediately", () => { + const cache = { current: { key: "pr-1", value: { files: [1] } } }; + + const retained = retainLatestByKey( + cache, + "pr-2", + { files: [] }, + (next) => next.files.length > 0, + ); + + assert.deepEqual(retained, { files: [] }); +}); + +test("an explicit selected review does not fall back to another identity", () => { + const selected = { id: "pr-a" }; + const branchReview = { id: "pr-branch" }; + + assert.equal( + currentPullRequestForSelection({ + fallback: branchReview, + pullRequests: [selected, branchReview], + selectedPullRequestId: "pr-a", + }), + selected, + ); + assert.equal( + currentPullRequestForSelection({ + fallback: branchReview, + pullRequests: [branchReview], + selectedPullRequestId: "pr-a", + }), + null, + ); + assert.equal( + currentPullRequestForSelection({ + fallback: branchReview, + pullRequests: [branchReview], + selectedPullRequestId: null, + }), + branchReview, + ); +}); + +test("retained review identity stays aligned with the diff-query identity across fetch phases", () => { + const reviewA = { id: "pr-a" }; + const renderedCache = { current: { key: "repo:pr-a", value: reviewA } }; + const diffQueryCache = { current: { key: "repo:pr-a", value: reviewA } }; + + const renderedDuringFetch = retainLatestByKey( + renderedCache, + "repo:pr-a", + currentPullRequestForSelection({ + fallback: { id: "pr-branch" }, + pullRequests: [], + selectedPullRequestId: "pr-a", + }), + (next) => shouldReplaceRetainedPullRequest(next, true), + ); + const diffDuringFetch = retainLatestByKey( + diffQueryCache, + "repo:pr-a", + currentPullRequestForSelection({ + fallback: { id: "pr-branch" }, + pullRequests: [], + selectedPullRequestId: "pr-a", + }), + (next) => shouldReplaceRetainedPullRequest(next, true), + ); + assert.equal(renderedDuringFetch, reviewA); + assert.equal(diffDuringFetch, reviewA); + assert.equal(renderedDuringFetch.id, diffDuringFetch.id); + + const renderedAfterComplete = retainLatestByKey( + renderedCache, + "repo:pr-a", + currentPullRequestForSelection({ + fallback: { id: "pr-branch" }, + pullRequests: [], + selectedPullRequestId: "pr-a", + }), + (next) => shouldReplaceRetainedPullRequest(next, false), + ); + const diffAfterComplete = retainLatestByKey( + diffQueryCache, + "repo:pr-a", + currentPullRequestForSelection({ + fallback: { id: "pr-branch" }, + pullRequests: [], + selectedPullRequestId: "pr-a", + }), + (next) => shouldReplaceRetainedPullRequest(next, false), + ); + assert.equal(renderedAfterComplete, null); + assert.equal(diffAfterComplete, null); +}); + +test("review files stay mounted when a populated diff races an unavailable snapshot", () => { + assert.equal( + projectReviewFilesChangedBody({ + hasPopulatedDiff: true, + hasSelectedPullRequest: true, + repositoryUnavailable: true, + }), + "files", + ); +}); + +test("review files can show unavailable before a diff exists", () => { + assert.equal( + projectReviewFilesChangedBody({ + hasPopulatedDiff: false, + hasSelectedPullRequest: true, + repositoryUnavailable: true, + }), + "unavailable", + ); +}); + +test("review files render the panel for a selected review when the repo is available", () => { + assert.equal( + projectReviewFilesChangedBody({ + hasPopulatedDiff: false, + hasSelectedPullRequest: true, + repositoryUnavailable: false, + }), + "files", + ); +}); + +test("review diffs stay on the target branch, not the head or picker branch", () => { + assert.equal( + reviewDiffWorkspaceBranch({ + activeBranch: "variation/bees", + defaultBranch: "main", + pullRequest: { targetBranch: "main" }, + }), + "main", + ); + assert.equal( + reviewDiffWorkspaceBranch({ + activeBranch: "variation/bees", + defaultBranch: "main", + pullRequest: { targetBranch: null }, + }), + "main", + ); + assert.equal( + reviewDiffWorkspaceBranch({ + activeBranch: "variation/bees", + defaultBranch: "main", + pullRequest: null, + }), + "variation/bees", + ); +}); diff --git a/desktop/src/features/projects/lib/projectReviewDisplay.ts b/desktop/src/features/projects/lib/projectReviewDisplay.ts new file mode 100644 index 00000000000..d7ba721238c --- /dev/null +++ b/desktop/src/features/projects/lib/projectReviewDisplay.ts @@ -0,0 +1,101 @@ +type RetainLatestByKeyCache = { + current: { + key: string; + value: T; + }; +}; + +/** + * Keep the latest accepted value for a stable key across transient empties. + * A new key always takes `value` immediately (real navigation). + */ +export function retainLatestByKey( + cache: RetainLatestByKeyCache, + key: string, + value: T, + shouldReplace: (next: T, previous: T) => boolean, +): T { + if (cache.current.key !== key) { + cache.current = { key, value }; + return value; + } + if (shouldReplace(value, cache.current.value)) { + cache.current.value = value; + } + return cache.current.value; +} + +/** + * Keep a selected review through transient empty refetches. Once the fetch + * is idle, accept the completed result — including null — so the rendered + * identity can match the diff-query identity. + */ +export function shouldReplaceRetainedPullRequest( + next: unknown, + isFetching: boolean, +): boolean { + return Boolean(next) || !isFetching; +} + +/** + * Resolve the current review for a selection. An explicit ID that is missing + * from the list is `null` so a completed refetch can clear it; pass + * `fallback` only when no ID is selected (branch auto-select). + */ +export function currentPullRequestForSelection({ + fallback = null, + pullRequests, + selectedPullRequestId, +}: { + fallback?: T | null; + pullRequests: readonly T[] | undefined; + selectedPullRequestId: string | null; +}): T | null { + if (selectedPullRequestId) { + return ( + pullRequests?.find((item) => item.id === selectedPullRequestId) ?? null + ); + } + return fallback; +} + +/** + * Which body to render under a review's Files changed section. + * A populated diff must keep the files panel mounted even when the repository + * snapshot briefly looks unavailable — swapping in the unavailable placeholder + * is the files-section flicker. + */ +export function projectReviewFilesChangedBody({ + hasPopulatedDiff, + hasSelectedPullRequest, + repositoryUnavailable, +}: { + hasPopulatedDiff: boolean; + hasSelectedPullRequest: boolean; + repositoryUnavailable: boolean; +}): "files" | "unavailable" | null { + if (hasSelectedPullRequest && (hasPopulatedDiff || !repositoryUnavailable)) { + return "files"; + } + if (repositoryUnavailable) return "unavailable"; + return null; +} + +/** + * Workspace branch used to fetch a review diff. + * A selected review is `target...head`; do not key that query on the head + * branch or the workspace picker, or Files changed will swap with the + * default-branch snapshot. + */ +export function reviewDiffWorkspaceBranch({ + activeBranch, + defaultBranch, + pullRequest, +}: { + activeBranch: string | null | undefined; + defaultBranch: string | null | undefined; + pullRequest: { targetBranch: string | null } | null | undefined; +}): string | null | undefined { + if (!pullRequest) return activeBranch; + return pullRequest.targetBranch || defaultBranch || activeBranch; +} diff --git a/desktop/src/features/projects/ui/AgentContextPayloadPreview.test.mjs b/desktop/src/features/projects/ui/AgentContextPayloadPreview.test.mjs index 749d5eb5011..6229c775389 100644 --- a/desktop/src/features/projects/ui/AgentContextPayloadPreview.test.mjs +++ b/desktop/src/features/projects/ui/AgentContextPayloadPreview.test.mjs @@ -33,7 +33,7 @@ afterEach(async () => { after(() => dom.window.close()); -async function renderPreview(payload) { +async function renderPreview(payload, options = {}) { const { createElement } = await import("react"); const { render } = await import("@testing-library/react"); const { AgentContextPayloadPreview } = await import( @@ -41,8 +41,9 @@ async function renderPreview(payload) { ); return render( createElement(AgentContextPayloadPreview, { + iconOnly: options.iconOnly, payload, - triggerLabel: "Context", + triggerLabel: options.triggerLabel ?? "Context", }), ); } @@ -83,6 +84,24 @@ test("discloses the exact appended payload before send, adversarial metadata inc assert.equal(screen.queryByTestId("agent-context-preview"), null); }); +test("supports a subtle icon-only disclosure without losing its accessible name", async () => { + const { fireEvent, screen } = await import("@testing-library/react"); + await renderPreview("Exact context", { + iconOnly: true, + triggerLabel: "Preview message context", + }); + + const trigger = screen.getByRole("button", { + name: "Preview message context", + }); + assert.equal(trigger.textContent?.trim(), ""); + fireEvent.click(trigger); + assert.equal( + screen.getByTestId("agent-context-preview-payload").textContent, + "Exact context", + ); +}); + test("renders nothing when there is no payload to append", async () => { const { screen } = await import("@testing-library/react"); await renderPreview(""); diff --git a/desktop/src/features/projects/ui/AgentContextPayloadPreview.tsx b/desktop/src/features/projects/ui/AgentContextPayloadPreview.tsx index d6db47b8686..550cadd86aa 100644 --- a/desktop/src/features/projects/ui/AgentContextPayloadPreview.tsx +++ b/desktop/src/features/projects/ui/AgentContextPayloadPreview.tsx @@ -1,6 +1,7 @@ import { Info } from "lucide-react"; import * as React from "react"; +import { cn } from "@/shared/lib/cn"; import { Button } from "@/shared/ui/button"; /** @@ -13,9 +14,11 @@ import { Button } from "@/shared/ui/button"; * inspects here is byte-identical to what gets signed under their key. */ export function AgentContextPayloadPreview({ + iconOnly = false, payload, triggerLabel, }: { + iconOnly?: boolean; payload: string; triggerLabel: string; }) { @@ -25,8 +28,14 @@ export function AgentContextPayloadPreview({ return (
{open ? (
{ + const byChannel = new Map(); + for (const hit of hits) { + if (hit.channelId && !byChannel.has(hit.channelId)) { + byChannel.set(hit.channelId, hit); + } + } + return byChannel; + }, [hits]); +} + +/** + * Shared row-click behavior: land on the latest matching message — in the + * side conversation panel when one is mounted — instead of jumping straight + * to the channel. Forum content opens in place (the panel renders chat + * threads only), and channels with no quotable hit fall back to plain + * channel navigation. + */ +function openDiscussionHit({ + channelId, + goChannel, + latestHit, + openSearchHit, + panel, +}: { + channelId: string; + goChannel: (channelId: string) => unknown; + latestHit: SearchHit | undefined; + openSearchHit: (hit: SearchHit) => unknown; + panel: { openConversation: (hit: SearchHit) => void } | null; +}) { + if (!latestHit) { + void goChannel(channelId); + return; + } + const opensForum = + latestHit.kind === KIND_FORUM_POST || latestHit.kind === KIND_FORUM_COMMENT; + if (!panel || opensForum) { + void openSearchHit(latestHit); + return; + } + panel.openConversation(latestHit); +} + /** Channel display name, preferring the hit's name, then bounded metadata, * then a short id so inaccessible/renamed channels still render something. */ function useChannelNameLookup(channelIds: readonly string[]) { @@ -141,19 +188,10 @@ export function DiscussedInChannels({ { enabled: visible.length > 0 }, ); const profiles = profilesQuery.data?.profiles; - // Hits are sorted newest first, so the first hit per channel is the one a - // click should land on (and the one worth quoting). The origin channel has - // no such hit: the `h` tag proves only the channel, so its row navigates - // to the channel without claiming any particular message. - const latestHitByChannel = React.useMemo(() => { - const byChannel = new Map(); - for (const hit of hits) { - if (hit.channelId && !byChannel.has(hit.channelId)) { - byChannel.set(hit.channelId, hit); - } - } - return byChannel; - }, [hits]); + // The origin channel has no quotable hit: the `h` tag proves only the + // channel, so its row navigates to the channel without claiming any + // particular message. + const latestHitByChannel = useLatestHitByChannel(hits); if (channels.length === 0) return null; const hiddenCount = channels.length - visible.length; @@ -173,21 +211,14 @@ export function DiscussedInChannels({ {visible.map((channel) => { const latestHit = latestHitByChannel.get(channel.id); const name = channelName(channel.id, channel.name); - const opensForum = - latestHit != null && - (latestHit.kind === KIND_FORUM_POST || - latestHit.kind === KIND_FORUM_COMMENT); - const openConversation = () => { - if (!latestHit) { - void goChannel(channel.id); - return; - } - if (!projectConversationPanel || opensForum) { - void openSearchHit(latestHit); - return; - } - projectConversationPanel.openConversation(latestHit); - }; + const openConversation = () => + openDiscussionHit({ + channelId: channel.id, + goChannel, + latestHit, + openSearchHit, + panel: projectConversationPanel, + }); return (
@@ -336,7 +367,9 @@ function DiscussionNameList({ /** * Full-width channel list for the workspace "Channels" tab: every channel * where the repository (or its PRs/issues) is linked in chat, with the - * people who discussed it there. + * people who discussed it there. Clicking a row opens the latest matching + * conversation in the side panel (whose header still jumps to the channel) + * rather than leaving the project view. */ export function DiscussionChannelsPanel({ query, @@ -345,13 +378,17 @@ export function DiscussionChannelsPanel({ query: string; repositoryName: string; }) { - const { channels, isLoading, isTruncated } = useDiscussionChannels(query); - const { goChannel } = useAppNavigation(); + const { channels, hits, isLoading, isTruncated } = + useDiscussionChannels(query); + const { goChannel, openSearchHit } = useAppNavigation(); + const projectConversationPanel = useProjectConversationPanel(); + const latestHitByChannel = useLatestHitByChannel(hits); const channelIds = React.useMemo( () => channels.map((channel) => channel.id), [channels], ); const channelName = useChannelNameLookup(channelIds); + const profilesQuery = useUsersBatchQuery( channels.flatMap((channel) => channel.participants), { enabled: channels.length > 0 }, @@ -363,7 +400,10 @@ export function DiscussionChannelsPanel({ } if (channels.length === 0) { return ( -

+

No channels reference this repository yet. Paste its link (or a review or task link) in a channel and it will show up here.

@@ -379,10 +419,11 @@ export function DiscussionChannelsPanel({ ); return ( -
+
    {channels.map((channel) => { const name = channelName(channel.id, channel.name); + const latestHit = latestHitByChannel.get(channel.id); return (
  • } - onClick={() => void goChannel(channel.id)} + onClick={() => + openDiscussionHit({ + channelId: channel.id, + goChannel, + latestHit, + openSearchHit, + panel: projectConversationPanel, + }) + } people={channel.participants} peopleTestId="project-channel-participants" profiles={profiles} @@ -411,7 +460,11 @@ export function DiscussionChannelsPanel({ }} testId="project-channel-row" title={`#${name}`} - titleAttr={`Open #${name}`} + titleAttr={ + latestHit + ? `Open the latest conversation in #${name}` + : `Open #${name}` + } />
    • ); diff --git a/desktop/src/features/projects/ui/IssueAssigneesRow.tsx b/desktop/src/features/projects/ui/IssueAssigneesRow.tsx index 4da5123814a..74130447270 100644 --- a/desktop/src/features/projects/ui/IssueAssigneesRow.tsx +++ b/desktop/src/features/projects/ui/IssueAssigneesRow.tsx @@ -267,7 +267,9 @@ export function IssueAssigneesRow({ {canSelfAssign && viewer ? ( + {open ? ( +
    + {payload} +
    + ) : null} +
+ ); + }, +); diff --git a/desktop/src/features/projects/ui/ProjectCards.tsx b/desktop/src/features/projects/ui/ProjectCards.tsx index b8c3ffd273d..68e067e0d1b 100644 --- a/desktop/src/features/projects/ui/ProjectCards.tsx +++ b/desktop/src/features/projects/ui/ProjectCards.tsx @@ -20,10 +20,8 @@ import type { ProjectActivitySummary, } from "@/features/projects/hooks"; import { - formatExactTimestamp, getProjectUpdatedAt, listRowDescription, - relativeTime, } from "@/features/projects/lib/projectsViewHelpers"; import type { ProjectRepoUnavailableReason } from "@/features/projects/lib/projectRepoAvailability"; import { projectShareLink } from "@/features/projects/lib/projectShareLinks"; @@ -55,39 +53,6 @@ import { ProjectEntityListRow } from "./ProjectEntityListRow"; import { PROJECT_GRID_CARD_BODY_CLASS } from "./projectGridCardStyles"; import { ProjectListRowMenu } from "./ProjectListRowMenu"; -function ProjectUpdatedLabel({ - profiles, - project, - summary, -}: { - profiles?: UserProfileLookup; - project: Project; - summary: ProjectActivitySummary | undefined; -}) { - const updatedAt = getProjectUpdatedAt(project, summary); - const latestCommit = summary?.latestCommit; - const authorLabel = latestCommit?.author - ? resolveUserLabel({ profiles, pubkey: latestCommit.author }) - : null; - - return ( - - - - {relativeTime(updatedAt)} - - - - {latestCommit - ? `${latestCommit.title || latestCommit.commit.slice(0, 7)}${ - authorLabel ? ` · ${authorLabel}` : "" - } · ${formatExactTimestamp(latestCommit.createdAt)}` - : `Created ${formatExactTimestamp(project.createdAt)}`} - - - ); -} - export function ProjectPeopleStack({ pubkeys, profiles, @@ -540,12 +505,7 @@ export function ProjectGridCard({
-
- +
{repositoryCount} } + affiliationClassName="w-auto" affiliationTestId="projects-row-context" affiliationTitle={`${repositoryCount} ${ repositoryCount === 1 ? "repository" : "repositories" diff --git a/desktop/src/features/projects/ui/ProjectCommitCopyButton.tsx b/desktop/src/features/projects/ui/ProjectCommitCopyButton.tsx index 0930146e2db..faadaf5d2c4 100644 --- a/desktop/src/features/projects/ui/ProjectCommitCopyButton.tsx +++ b/desktop/src/features/projects/ui/ProjectCommitCopyButton.tsx @@ -15,12 +15,16 @@ export function CopyTextButton({ text: string; }) { const [copied, setCopied] = React.useState(false); - const handleCopy = React.useCallback(() => { - void writeTextToClipboard(text).then(() => { - setCopied(true); - setTimeout(() => setCopied(false), 2_000); - }); - }, [text]); + const handleCopy = React.useCallback( + (event: React.MouseEvent) => { + event.stopPropagation(); + void writeTextToClipboard(text).then(() => { + setCopied(true); + setTimeout(() => setCopied(false), 2_000); + }); + }, + [text], + ); return ( - {open ?
{children}
: null} + {open ? ( +
{children}
+ ) : null} ); } diff --git a/desktop/src/features/projects/ui/ProjectEntityListRow.tsx b/desktop/src/features/projects/ui/ProjectEntityListRow.tsx index 10cd1f87a41..d0769e31e50 100644 --- a/desktop/src/features/projects/ui/ProjectEntityListRow.tsx +++ b/desktop/src/features/projects/ui/ProjectEntityListRow.tsx @@ -131,6 +131,7 @@ export function ProjectEntitySelectControl({ export function ProjectEntityListRow({ affiliation, + affiliationClassName, affiliationTestId, affiliationTitle, beforeDate, @@ -158,6 +159,7 @@ export function ProjectEntityListRow({ trailing, }: { affiliation?: React.ReactNode; + affiliationClassName?: string; affiliationTestId?: string; affiliationTitle?: string; beforeDate?: React.ReactNode; @@ -292,7 +294,10 @@ export function ProjectEntityListRow({ ) : null} {affiliation ? ( {count != null ? ( - + {count} {countSuffix} diff --git a/desktop/src/features/projects/ui/ProjectPullRequestFilesChangedPanel.tsx b/desktop/src/features/projects/ui/ProjectPullRequestFilesChangedPanel.tsx index 105d20f10ee..72884a33ed3 100644 --- a/desktop/src/features/projects/ui/ProjectPullRequestFilesChangedPanel.tsx +++ b/desktop/src/features/projects/ui/ProjectPullRequestFilesChangedPanel.tsx @@ -755,8 +755,10 @@ export function ProjectPullRequestFilesChangedPanel({ } export function ProjectDiffFilesPanel({ + className, error, diff, + fileTreeClassName, isLoading, embedded = false, focusedAnchor, @@ -764,6 +766,10 @@ export function ProjectDiffFilesPanel({ inlineComments, subjectLabel, }: { + /** Extra classes for the file-tree/diff grid container. */ + className?: string; + /** Overrides the file tree's default `max-h-96` cap, e.g. for full-height layouts. */ + fileTreeClassName?: string; error: unknown; diff: ProjectRepoDiff | null | undefined; isLoading: boolean; @@ -814,11 +820,11 @@ export function ProjectDiffFilesPanel({ } }, [filteredFiles, selectedPath]); - if (isLoading) { + if (isLoading && !diff) { return ; } - if (error) { + if (error && !diff) { const message = errorMessage(error); return (
@@ -876,7 +883,12 @@ export function ProjectDiffFilesPanel({ />
-