diff --git a/crates/buzz-db/src/channel.rs b/crates/buzz-db/src/channel.rs index 396244cc9..5508c95ca 100644 --- a/crates/buzz-db/src/channel.rs +++ b/crates/buzz-db/src/channel.rs @@ -332,6 +332,40 @@ pub async fn set_canvas( Ok(()) } +/// Namespace for the per-channel membership advisory lock. Serializes the +/// role-authorization + last-owner-count + write sequences in [`add_member`] +/// and [`remove_member`] against each other. +/// +/// Both functions read an owner COUNT and then write a *different* row than the +/// one they counted, so `READ COMMITTED` snapshot isolation alone permits two +/// concurrent demotions (or a demotion racing a removal) to each observe two +/// owners, each pass, and together leave zero — the exact governance loss the +/// guards exist to prevent. An advisory key rather than `SELECT ... FOR UPDATE` +/// on the channel row: membership is its own contention domain and must not +/// serialize against unrelated channel metadata writers (`update_channel`, +/// `set_topic`, the TTL transition). Distinct key domain from +/// `buzz_channel_ttl:`. +const CHANNEL_MEMBERSHIP_LOCK_NAMESPACE: &str = "buzz_channel_membership:"; + +/// 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. +async fn acquire_channel_membership_lock( + tx: &mut Transaction<'_, Postgres>, + community_id: CommunityId, + channel_id: Uuid, +) -> Result<()> { + sqlx::query("SELECT pg_advisory_xact_lock(hashtextextended($1, 0))") + .bind(format!( + "{CHANNEL_MEMBERSHIP_LOCK_NAMESPACE}{}:{}", + community_id.as_uuid(), + channel_id + )) + .execute(&mut **tx) + .await?; + Ok(()) +} + /// Add a member to a channel. /// /// Role enforcement: @@ -360,6 +394,10 @@ pub async fn add_member( let mut tx = pool.begin().await?; + // First statement: serialize the whole role-check / owner-count / upsert + // sequence against concurrent membership writes on this channel. + acquire_channel_membership_lock(&mut tx, community_id, channel_id).await?; + let channel = get_channel_tx(&mut tx, community_id, channel_id).await?; let effective_role = if channel.visibility == "private" { @@ -411,6 +449,56 @@ pub async fn add_member( } }; + // Changing an *active* member's role is privileged in BOTH directions. + // Demotion is as consequential as promotion: only owners/admins may grant + // elevated roles, so a demoted owner cannot restore themselves. Guarding + // only `role.is_elevated()` above therefore left owner→member demotion + // unauthorized-by-anyone. Re-adding an active member with the role they + // already hold stays idempotent and unguarded — the huddle bot-add and + // kind:9021 join paths rely on that. + // + // Deliberately keyed on the *active* role. A soft-removed row's stored role + // is history, not live authority: `removed_at` says it is no longer in + // force. Reactivation therefore lands at whatever `effective_role` the + // checks above already authorized — `Member` for any unprivileged caller, + // elevated only when a currently-elevated granter asked for it. Inferring + // current authority from a removed row would make soft-deleted ownership a + // resurrection token: an owner removed by another owner could self-rejoin + // via kind:9021 (`Member, None`) and silently regain ownership. + let current_role = get_active_role_tx(&mut tx, community_id, channel_id, pubkey).await?; + if let Some(current_role) = current_role.filter(|r| r != effective_role.as_str()) { + let actor_role = match invited_by { + Some(inviter) => get_active_role_tx(&mut tx, community_id, channel_id, inviter).await?, + None => None, + }; + let actor_role: Option = actor_role.and_then(|r| r.parse().ok()); + if !actor_role.is_some_and(|r| r.is_elevated()) { + return Err(DbError::AccessDenied( + "only owners/admins may change an active member's role".to_string(), + )); + } + + // Defense-in-depth, mirroring `remove_member`: a demotion must not + // strip the channel of its last owner, which would leave nobody able + // to moderate, edit metadata, or re-grant ownership. + if current_role == "owner" && effective_role != MemberRole::Owner { + let row = sqlx::query( + "SELECT COUNT(*) as cnt FROM channel_members \ + WHERE community_id = $1 AND channel_id = $2 AND role = 'owner' AND removed_at IS NULL", + ) + .bind(community_id.as_uuid()) + .bind(channel_id) + .fetch_one(&mut *tx) + .await?; + let owner_count: i64 = row.try_get("cnt")?; + if owner_count <= 1 { + return Err(DbError::AccessDenied( + "cannot demote the last owner — transfer ownership first".to_string(), + )); + } + } + } + sqlx::query( r#" INSERT INTO channel_members (community_id, channel_id, pubkey, role, invited_by) @@ -452,10 +540,19 @@ pub async fn add_member( /// removing themselves. /// /// Returns `Err(DbError::MemberNotFound)` if the target is not an active member. -/// The actor's role check and the UPDATE run inside a transaction to prevent a -/// TOCTOU race where the actor's role changes between the check and the update. -/// The `is_agent_owner` check runs outside the transaction against the main pool -/// because `agent_owner_pubkey` is immutable (set once at token mint). +/// +/// The per-channel membership lock is the transaction's first statement, so the +/// actor's role check, the last-owner count, and the UPDATE are all serialized +/// against concurrent membership writes — otherwise a concurrent demotion of the +/// actor could commit after their role was read and this removal would proceed on +/// a stale elevated role. +/// +/// The `is_agent_owner` lookup deliberately runs *before* the transaction opens: +/// it borrows a second connection from `pool`, and issuing it while holding the +/// lock could deadlock against ourselves on a small pool. That is safe because +/// `agent_owner_pubkey` is immutable — [`crate::user::set_agent_owner`] only +/// updates it when it `IS NULL` (first-mint-wins), so its value cannot change +/// under us and needs no serialization. pub async fn remove_member( pool: &PgPool, community_id: CommunityId, @@ -463,9 +560,24 @@ pub async fn remove_member( pubkey: &[u8], actor_pubkey: &[u8], ) -> Result<()> { + let is_self_remove = pubkey == actor_pubkey; + + // Immutable, and must not be queried while holding the lock (second pool + // connection). Resolved up front so every *mutable* authorization read can + // sit behind the serialization point below. + let actor_is_agent_owner = if is_self_remove { + false + } else { + crate::user::is_agent_owner(pool, community_id, pubkey, actor_pubkey).await? + }; + let mut tx = pool.begin().await?; - let is_self_remove = pubkey == actor_pubkey; + // First statement: serialize the actor-role check, the last-owner count and + // the UPDATE against concurrent membership writes on this channel (same key + // as `add_member`). + acquire_channel_membership_lock(&mut tx, community_id, channel_id).await?; + if !is_self_remove { let actor_role_str = get_active_role_tx(&mut tx, community_id, channel_id, actor_pubkey) .await? @@ -473,11 +585,7 @@ pub async fn remove_member( let actor_role: MemberRole = actor_role_str.parse().map_err(|_| { DbError::InvalidData(format!("invalid role in database: {actor_role_str}")) })?; - // Safe to query outside the transaction: agent_owner_pubkey is immutable - // (set once at token mint, first-mint-wins). - if !actor_role.is_elevated() - && !crate::user::is_agent_owner(pool, community_id, pubkey, actor_pubkey).await? - { + if !actor_role.is_elevated() && !actor_is_agent_owner { return Err(DbError::AccessDenied( "only owners/admins or the agent's owner may remove other members".to_string(), )); @@ -1892,4 +2000,688 @@ mod tests { "random user should not be able to remove someone else's bot" ); } + + /// SECURITY REPRO (Dawn, kind:9000 demotion report): an unprivileged plain + /// member calls add_member with role=Member against the channel OWNER. + /// If this succeeds, add_member has no demotion authorization and no + /// last-owner guard. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn repro_unprivileged_member_can_demote_owner() { + let pool = setup_pool().await; + let community_id = make_test_community(&pool).await; + let community = CommunityId::from_uuid(community_id); + let victim_owner = random_pubkey(); + let attacker = random_pubkey(); + + for pk in [&victim_owner, &attacker] { + ensure_user(&pool, community, pk) + .await + .expect("ensure user"); + } + + let channel = create_test_channel( + &pool, + community_id, + "repro-demote-owner", + ChannelType::Stream, + ChannelVisibility::Open, + None, + &victim_owner, + None, + ) + .await + .expect("create channel"); + + // create_test_channel already seeds the creator as 'owner', mirroring + // create_channel's own INSERT (channel.rs:131-145). + let role_of = |members: Vec, pk: Vec| -> Option { + members.into_iter().find(|m| m.pubkey == pk).map(|m| m.role) + }; + let before = role_of( + get_members(&pool, community, channel.id) + .await + .expect("members"), + victim_owner.clone(), + ); + assert_eq!( + before.as_deref(), + Some("owner"), + "victim must start as owner" + ); + + // Attacker: plain member, not owner/admin. + add_member( + &pool, + community, + channel.id, + &attacker, + MemberRole::Member, + None, + ) + .await + .expect("attacker self-joins open channel"); + + // The attack: attacker is `invited_by` and demotes the owner. + let res = add_member( + &pool, + community, + channel.id, + &victim_owner, + MemberRole::Member, + Some(&attacker), + ) + .await; + + let after = role_of( + get_members(&pool, community, channel.id) + .await + .expect("members"), + victim_owner.clone(), + ); + let owners = get_members(&pool, community, channel.id) + .await + .expect("members") + .into_iter() + .filter(|m| m.role == "owner") + .count(); + + assert!( + res.is_err(), + "unprivileged member must not be able to demote the owner" + ); + assert_eq!(after.as_deref(), Some("owner"), "owner role must survive"); + assert_eq!(owners, 1, "channel must still have its owner"); + } + + /// SECURITY REPRO (Dawn): same demotion on a PRIVATE channel, where the + /// attacker is a plain member. The report claims any member suffices here. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn repro_private_channel_member_can_demote_owner() { + let pool = setup_pool().await; + let community_id = make_test_community(&pool).await; + let community = CommunityId::from_uuid(community_id); + let victim_owner = random_pubkey(); + let attacker = random_pubkey(); + + for pk in [&victim_owner, &attacker] { + ensure_user(&pool, community, pk) + .await + .expect("ensure user"); + } + + let channel = create_test_channel( + &pool, + community_id, + "repro-demote-owner-private", + ChannelType::Stream, + ChannelVisibility::Private, + None, + &victim_owner, + None, + ) + .await + .expect("create private channel"); + + // Owner invites the attacker as a plain member (legitimate). + add_member( + &pool, + community, + channel.id, + &attacker, + MemberRole::Member, + Some(&victim_owner), + ) + .await + .expect("owner invites attacker"); + + // Attack: plain member demotes the owner. + let res = add_member( + &pool, + community, + channel.id, + &victim_owner, + MemberRole::Member, + Some(&attacker), + ) + .await; + + let members = get_members(&pool, community, channel.id) + .await + .expect("members"); + let victim_role = members + .iter() + .find(|m| m.pubkey == victim_owner) + .map(|m| m.role.clone()); + let owners = members.iter().filter(|m| m.role == "owner").count(); + + assert!( + res.is_err(), + "plain member must not be able to demote the owner on a private channel" + ); + assert_eq!( + victim_role.as_deref(), + Some("owner"), + "owner role must survive" + ); + assert_eq!(owners, 1, "channel must still have its owner"); + } + + /// The fix must not break legitimate role management: an owner demoting a + /// co-owner (while another owner remains) must still succeed, and promotion + /// by an owner must still succeed. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn owner_can_still_manage_roles_after_demotion_guard() { + 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 other = random_pubkey(); + + for pk in [&owner, &other] { + ensure_user(&pool, community, pk) + .await + .expect("ensure user"); + } + + let channel = create_test_channel( + &pool, + community_id, + "roles-still-manageable", + ChannelType::Stream, + ChannelVisibility::Open, + None, + &owner, + None, + ) + .await + .expect("create channel"); + + // Owner promotes `other` to owner — allowed (actor is elevated). + add_member( + &pool, + community, + channel.id, + &other, + MemberRole::Owner, + Some(&owner), + ) + .await + .expect("owner may promote to owner"); + + // Owner demotes the co-owner back to member — allowed: actor is elevated + // and another owner remains, so the last-owner guard does not trip. + add_member( + &pool, + community, + channel.id, + &other, + MemberRole::Member, + Some(&owner), + ) + .await + .expect("owner may demote a co-owner while another owner remains"); + + let members = get_members(&pool, community, channel.id) + .await + .expect("members"); + let role_of = |pk: &Vec| { + members + .iter() + .find(|m| &m.pubkey == pk) + .map(|m| m.role.clone()) + }; + assert_eq!(role_of(&other).as_deref(), Some("member")); + assert_eq!(role_of(&owner).as_deref(), Some("owner")); + + // Idempotent re-add at the SAME role must stay unguarded even from a + // non-elevated actor — the huddle bot-add path depends on this. + let bot = random_pubkey(); + ensure_user(&pool, community, &bot) + .await + .expect("ensure bot"); + add_member( + &pool, + community, + channel.id, + &bot, + MemberRole::Bot, + Some(&owner), + ) + .await + .expect("add bot"); + add_member( + &pool, + community, + channel.id, + &bot, + MemberRole::Bot, + Some(&other), + ) + .await + .expect("re-adding at the same role must remain idempotent"); + + // But the last owner cannot be demoted, even by themselves. + let err = add_member( + &pool, + community, + channel.id, + &owner, + MemberRole::Member, + Some(&owner), + ) + .await + .expect_err("last owner must not be demotable"); + println!("last-owner demotion rejected: {err}"); + } + + /// Isolates the actor-authorization guard from the last-owner guard. + /// + /// `repro_unprivileged_member_can_demote_owner` demotes the *sole* owner, so + /// the last-owner guard alone is enough to reject it: stubbing out the actor + /// check leaves that test green and the authorization hole invisible. Here a + /// second owner remains, so the last-owner guard cannot fire and only the + /// actor check stands between an unprivileged member and a co-owner's role. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn unprivileged_member_cannot_demote_a_co_owner() { + 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 co_owner = random_pubkey(); + let attacker = random_pubkey(); + + for pk in [&owner, &co_owner, &attacker] { + ensure_user(&pool, community, pk) + .await + .expect("ensure user"); + } + + let channel = create_test_channel( + &pool, + community_id, + "co-owner-demotion-authz", + ChannelType::Stream, + ChannelVisibility::Open, + None, + &owner, + None, + ) + .await + .expect("create channel"); + + add_member( + &pool, + community, + channel.id, + &co_owner, + MemberRole::Owner, + Some(&owner), + ) + .await + .expect("owner may promote a co-owner"); + + add_member( + &pool, + community, + channel.id, + &attacker, + MemberRole::Member, + None, + ) + .await + .expect("attacker self-joins the open channel"); + + // Two owners remain, so the last-owner guard cannot reject this. Only + // the actor-authorization check can. + let err = add_member( + &pool, + community, + channel.id, + &co_owner, + MemberRole::Member, + Some(&attacker), + ) + .await + .expect_err("an unprivileged member must not demote a co-owner"); + println!("co-owner demotion by unprivileged actor rejected: {err}"); + + let members = get_members(&pool, community, channel.id) + .await + .expect("members"); + let role_of = |pk: &Vec| { + members + .iter() + .find(|m| &m.pubkey == pk) + .map(|m| m.role.clone()) + }; + assert_eq!( + role_of(&co_owner).as_deref(), + Some("owner"), + "co-owner must keep their role" + ); + assert_eq!( + members.iter().filter(|m| m.role == "owner").count(), + 2, + "both owners must survive" + ); + } + + /// Sets up an open channel with exactly two owners, returning + /// `(community, channel_id, owner_a, owner_b)`. + async fn channel_with_two_owners( + pool: &PgPool, + name: &str, + ) -> (CommunityId, Uuid, Vec, Vec) { + let community_id = make_test_community(pool).await; + let community = CommunityId::from_uuid(community_id); + let owner_a = random_pubkey(); + let owner_b = random_pubkey(); + for pk in [&owner_a, &owner_b] { + ensure_user(pool, community, pk).await.expect("ensure user"); + } + + let channel = create_test_channel( + pool, + community_id, + name, + ChannelType::Stream, + ChannelVisibility::Open, + None, + &owner_a, + None, + ) + .await + .expect("create channel"); + + add_member( + pool, + community, + channel.id, + &owner_b, + MemberRole::Owner, + Some(&owner_a), + ) + .await + .expect("promote second owner"); + + (community, channel.id, owner_a, owner_b) + } + + /// 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. + /// + /// Deterministic rather than timing-based: an outer transaction takes the + /// per-channel membership key first, then each membership writer must block + /// until it is released. Verified by mutation — dropping the lock from either + /// function makes that call return immediately and fails this test. + #[tokio::test] + #[ignore] + async fn membership_writes_serialize_on_the_shared_channel_lock() { + let pool = setup_pool().await; + let (community, channel_id, owner_a, owner_b) = + channel_with_two_owners(&pool, "membership-lock-shared").await; + + for label in ["add_member", "remove_member"] { + // Hold the same advisory key an in-tree membership write would take. + let mut holder = pool.begin().await.expect("begin lock holder"); + acquire_channel_membership_lock(&mut holder, community, channel_id) + .await + .expect("holder acquires membership key"); + + let pool2 = pool.clone(); + let (target, actor) = (owner_a.clone(), owner_b.clone()); + let mut writer = tokio::spawn(async move { + match label { + "add_member" => add_member( + &pool2, + community, + channel_id, + &target, + MemberRole::Member, + Some(&actor), + ) + .await + .map(|_| ()), + _ => remove_member(&pool2, community, channel_id, &target, &actor).await, + } + }); + + // While the key is held, the writer must make no progress. + let blocked = + tokio::time::timeout(std::time::Duration::from_millis(750), &mut writer).await; + assert!( + blocked.is_err(), + "{label} completed while the channel membership key was held — \ + it is not serializing on the shared lock" + ); + println!("{label} blocked on the held membership key, as required"); + + // Releasing the key lets it proceed. + holder.rollback().await.expect("release membership key"); + tokio::time::timeout(std::time::Duration::from_secs(10), writer) + .await + .expect("writer must proceed once the key is released") + .expect("writer task panicked") + .expect("writer must succeed after the key is released"); + + // Restore two owners for the next iteration. + add_member( + &pool, + community, + channel_id, + &owner_a, + MemberRole::Owner, + Some(&owner_b), + ) + .await + .expect("restore second owner"); + } + } + + /// Every *mutable* authorization read must sit behind the membership lock. + /// A remover that reads its elevated role before acquiring the lock can be + /// demoted by a concurrent writer and still proceed on the stale role. + /// + /// Deterministic: the holder takes the key, `remove_member` blocks on it, the + /// holder then demotes the remover and commits. Once the key is released the + /// remover must re-read its (now unprivileged) role and be rejected. + #[tokio::test] + #[ignore] + async fn remove_member_rejects_an_actor_demoted_while_it_waited() { + let pool = setup_pool().await; + let (community, channel_id, owner_a, owner_b) = + channel_with_two_owners(&pool, "stale-actor-role").await; + // owner_b removes a plain member, so the last-owner guard is not what + // rejects this — only the actor's own role can. + let victim = random_pubkey(); + ensure_user(&pool, community, &victim) + .await + .expect("ensure victim"); + add_member( + &pool, + community, + channel_id, + &victim, + MemberRole::Member, + Some(&owner_a), + ) + .await + .expect("add victim"); + + let mut holder = pool.begin().await.expect("begin lock holder"); + acquire_channel_membership_lock(&mut holder, community, channel_id) + .await + .expect("holder acquires membership key"); + + let pool2 = pool.clone(); + let (actor, target) = (owner_b.clone(), victim.clone()); + let mut remover = tokio::spawn(async move { + remove_member(&pool2, community, channel_id, &target, &actor).await + }); + + // Must be waiting on the key, not already authorized past it. + assert!( + tokio::time::timeout(std::time::Duration::from_millis(750), &mut remover) + .await + .is_err(), + "remove_member must block on the membership key before authorizing" + ); + + // Demote the waiting actor to a plain member and release the key. + sqlx::query( + "UPDATE channel_members SET role = 'member' \ + WHERE community_id = $1 AND channel_id = $2 AND pubkey = $3", + ) + .bind(community.as_uuid()) + .bind(channel_id) + .bind(&owner_b) + .execute(&mut *holder) + .await + .expect("demote the waiting actor"); + holder.commit().await.expect("commit demotion"); + + let result = tokio::time::timeout(std::time::Duration::from_secs(10), remover) + .await + .expect("remover must proceed once the key is released") + .expect("remover task panicked"); + + let err = result.expect_err("a demoted actor must not remove another member"); + println!("stale-role removal rejected: {err}"); + + // The victim must still be an active member. + let members = get_members(&pool, community, channel_id) + .await + .expect("members"); + assert!( + members.iter().any(|m| m.pubkey == victim), + "victim must not have been removed by a demoted actor" + ); + } + + /// A soft-removed row keeps its stored `role`, but that role is history, + /// not live authority — `removed_at` says it is no longer in force. So + /// reactivation must land at the baseline the caller was authorized for, + /// never at the role the row happens to remember. + /// + /// Regression for the sharper vulnerability the alternative would create: + /// an owner kicked by another owner self-rejoins through the kind:9021 + /// path (`Member`, no inviter) and must come back as a plain member. If + /// `add_member` inferred authority from the removed row, soft-deleted + /// ownership would be a resurrection token. + /// + /// Two owners on purpose, so the last-owner guard can never be what + /// decides the outcome — only role resolution can. + #[tokio::test] + #[ignore] + async fn kicked_owner_rejoins_as_member_not_owner() { + let pool = setup_pool().await; + let (community, channel_id, owner_a, owner_b) = + channel_with_two_owners(&pool, "kicked-owner-rejoin").await; + + // owner_a kicks owner_b (allowed: owner_a remains as the last owner). + remove_member(&pool, community, channel_id, &owner_b, &owner_a) + .await + .expect("an owner may remove another owner"); + + let stored: String = sqlx::query_scalar( + "SELECT role::text FROM channel_members \ + WHERE community_id = $1 AND channel_id = $2 AND pubkey = $3", + ) + .bind(community.as_uuid()) + .bind(channel_id) + .bind(&owner_b) + .fetch_one(&pool) + .await + .expect("stored role survives soft removal"); + assert_eq!( + stored, "owner", + "the removed row still remembers `owner` — which is exactly why \ + authorization must not read it" + ); + + // The kind:9021 self-rejoin path: `Member`, no inviter. + add_member( + &pool, + community, + channel_id, + &owner_b, + MemberRole::Member, + None, + ) + .await + .expect("a removed member may rejoin an open channel"); + + let rejoined = get_member_role(&pool, community, channel_id, &owner_b) + .await + .expect("read role after rejoin"); + assert_eq!( + rejoined.as_deref(), + Some("member"), + "a kicked owner must rejoin at baseline privilege, not regain ownership" + ); + } + + /// The other side of the same boundary: reactivation may reach an elevated + /// role, but only because a *currently* elevated granter asked for it. + #[tokio::test] + #[ignore] + async fn removed_owner_is_restored_only_by_a_current_owner() { + let pool = setup_pool().await; + let (community, channel_id, owner_a, owner_b) = + channel_with_two_owners(&pool, "removed-owner-restore").await; + + remove_member(&pool, community, channel_id, &owner_b, &owner_a) + .await + .expect("an owner may remove another owner"); + + // An unprivileged member cannot re-add them at `owner`. + let rando = random_pubkey(); + ensure_user(&pool, community, &rando) + .await + .expect("ensure rando"); + add_member( + &pool, + community, + channel_id, + &rando, + MemberRole::Member, + None, + ) + .await + .expect("rando self-joins open channel"); + let denied = add_member( + &pool, + community, + channel_id, + &owner_b, + MemberRole::Owner, + Some(&rando), + ) + .await; + assert!( + matches!(denied, Err(DbError::AccessDenied(_))), + "an unprivileged actor must not re-add anyone at `owner`, got {denied:?}" + ); + + // The remaining owner can. + add_member( + &pool, + community, + channel_id, + &owner_b, + MemberRole::Owner, + Some(&owner_a), + ) + .await + .expect("a current owner may restore ownership"); + let restored = get_member_role(&pool, community, channel_id, &owner_b) + .await + .expect("read role after restore"); + assert_eq!(restored.as_deref(), Some("owner")); + } } diff --git a/crates/buzz-db/src/feed.rs b/crates/buzz-db/src/feed.rs index 7cc02d801..511a2a608 100644 --- a/crates/buzz-db/src/feed.rs +++ b/crates/buzz-db/src/feed.rs @@ -34,9 +34,10 @@ use sqlx::{PgPool, QueryBuilder}; use uuid::Uuid; use buzz_core::kind::{ - KIND_FORUM_COMMENT, KIND_FORUM_POST, KIND_JOB_PROGRESS, KIND_JOB_REQUEST, KIND_JOB_RESULT, - KIND_STREAM_MESSAGE, KIND_STREAM_MESSAGE_V2, KIND_STREAM_REMINDER, - KIND_WORKFLOW_APPROVAL_REQUESTED, + KIND_FORUM_COMMENT, KIND_FORUM_POST, KIND_GIT_ISSUE, KIND_GIT_PR_UPDATE, KIND_GIT_PULL_REQUEST, + KIND_GIT_STATUS_CLOSED, KIND_GIT_STATUS_DRAFT, KIND_GIT_STATUS_MERGED, KIND_GIT_STATUS_OPEN, + KIND_JOB_PROGRESS, KIND_JOB_REQUEST, KIND_JOB_RESULT, KIND_STREAM_MESSAGE, + KIND_STREAM_MESSAGE_V2, KIND_STREAM_REMINDER, KIND_TEXT_NOTE, KIND_WORKFLOW_APPROVAL_REQUESTED, }; use buzz_core::{CommunityId, StoredEvent}; @@ -103,7 +104,9 @@ fn build_mentions_query( qb.push(" AND e.deleted_at IS NULL"); qb.push(format!( " AND e.kind IN ({KIND_STREAM_MESSAGE}, {KIND_STREAM_MESSAGE_V2}, \ - {KIND_FORUM_POST}, {KIND_FORUM_COMMENT})" + {KIND_TEXT_NOTE}, {KIND_FORUM_POST}, {KIND_FORUM_COMMENT}, {KIND_GIT_PULL_REQUEST}, \ + {KIND_GIT_PR_UPDATE}, {KIND_GIT_ISSUE}, {KIND_GIT_STATUS_OPEN}, \ + {KIND_GIT_STATUS_MERGED}, {KIND_GIT_STATUS_CLOSED}, {KIND_GIT_STATUS_DRAFT})" )); push_visible_channel_filter(&mut qb, "e.channel_id", accessible_channel_ids); if let Some(s) = since { @@ -252,7 +255,7 @@ mod tests { use nostr::{EventBuilder, Keys, Kind, Tag}; use uuid::Uuid; - const TEST_DB_URL: &str = "postgres://buzz:buzz_dev@localhost:5432/buzz"; + const TEST_DB_URL: &str = "postgres://buzz:buzz_dev@localhost:5432/buzz"; // sadscan:disable np.postgres.1 -- local test-only credentials async fn setup_pool() -> PgPool { let database_url = std::env::var("BUZZ_TEST_DATABASE_URL") @@ -777,6 +780,12 @@ mod tests { sql.contains("AND m.community_id = "), "mentions feed must also bind event_mentions.community_id: {sql}" ); + assert!( + sql.contains(&KIND_GIT_PULL_REQUEST.to_string()) + && sql.contains(&KIND_GIT_ISSUE.to_string()) + && sql.contains(&KIND_TEXT_NOTE.to_string()), + "mentions feed must include Buzz Git roots and comments: {sql}" + ); } #[test] diff --git a/crates/buzz-relay/src/handlers/side_effects.rs b/crates/buzz-relay/src/handlers/side_effects.rs index 3112e9a55..6ff1315ac 100644 --- a/crates/buzz-relay/src/handlers/side_effects.rs +++ b/crates/buzz-relay/src/handlers/side_effects.rs @@ -291,35 +291,39 @@ pub async fn validate_admin_event( match kind { 9000 => { - // Validate role tag if present - let role_str = extract_tag_value(event, "role").unwrap_or_else(|| "member".to_string()); - if role_str.parse::().is_err() { - return Err(anyhow::anyhow!("invalid role: {role_str}")); - } + // An absent role tag means "no role change requested": for an existing + // member that preserves the role they already hold, and only defaults + // to Member for a genuinely new member. Defaulting unconditionally to + // Member made a bare self-targeted PUT_USER silently demote an owner. + let role_str = extract_tag_value(event, "role"); + let requested_role = match role_str { + Some(ref s) => match s.parse::() { + Ok(r) => Some(r), + Err(_) => return Err(anyhow::anyhow!("invalid role: {s}")), + }, + None => None, + }; + + let members = state.db.get_members(tenant.community(), channel_id).await?; + let actor_role: Option = members + .iter() + .find(|m| m.pubkey == actor_bytes) + .and_then(|m| m.role.parse().ok()); // PUT_USER: open channels allow any authenticated user; private channels // require the actor to be an existing member (any role can invite). if channel.visibility == "private" { - let members = state.db.get_members(tenant.community(), channel_id).await?; - let actor_member = members.iter().find(|m| m.pubkey == actor_bytes); - match actor_member { - Some(_) => {} - None => return Err(anyhow::anyhow!("actor not authorized")), + if actor_role.is_none() { + return Err(anyhow::anyhow!("actor not authorized")); } // Only owners/admins may grant elevated roles. - let role: buzz_db::channel::MemberRole = role_str.parse().unwrap(); - if role.is_elevated() { - let actor_role: buzz_db::channel::MemberRole = actor_member - .unwrap() - .role - .parse() - .unwrap_or(buzz_db::channel::MemberRole::Member); - if !actor_role.is_elevated() { - return Err(anyhow::anyhow!( - "only owners/admins may grant elevated roles" - )); - } + if requested_role.is_some_and(|r| r.is_elevated()) + && !actor_role.is_some_and(|r| r.is_elevated()) + { + return Err(anyhow::anyhow!( + "only owners/admins may grant elevated roles" + )); } } @@ -327,6 +331,39 @@ pub async fn validate_admin_event( let target_pubkey = extract_p_tag(event).ok_or_else(|| anyhow::anyhow!("missing p tag"))?; + // Changing an ACTIVE existing member's role is privileged in both + // directions, on every visibility. `get_members` filters + // `removed_at IS NULL`, so a soft-removed row is deliberately not an + // "existing member" here: its stored role is history, not live + // authority, and reactivation is governed by the elevated-granter + // check above rather than by the role the row remembers. + // + // `add_member` is the authority (it also covers the desktop/admin + // callers that skip this validator); rejecting here too means the + // client gets a real error instead of an OK for an event whose side + // effect then fails. Re-adding at the same role stays idempotent — + // the huddle bot-add path relies on that. + if let Some((target, role)) = members + .iter() + .find(|m| m.pubkey == target_pubkey) + .zip(requested_role) + .filter(|(m, role)| m.role != role.as_str()) + { + if !actor_role.is_some_and(|r| r.is_elevated()) { + return Err(anyhow::anyhow!( + "only owners/admins may change an active member's role" + )); + } + if target.role == "owner" + && role != buzz_db::channel::MemberRole::Owner + && members.iter().filter(|m| m.role == "owner").count() <= 1 + { + return Err(anyhow::anyhow!( + "cannot demote the last owner — transfer ownership first" + )); + } + } + // Self-add: always allowed regardless of policy. if target_pubkey == actor_bytes { return Ok(()); @@ -1208,10 +1245,22 @@ async fn handle_put_user( let channel_id = extract_h_tag_channel(event).ok_or_else(|| anyhow::anyhow!("missing h tag"))?; let target_pubkey = extract_p_tag(event).ok_or_else(|| anyhow::anyhow!("missing p tag"))?; - let role_str = extract_tag_value(event, "role").unwrap_or_else(|| "member".to_string()); - let role: MemberRole = role_str - .parse() - .map_err(|_| anyhow::anyhow!("invalid role: {role_str}"))?; + // No role tag = no role change: preserve an existing member's current role and + // fall back to Member only for a new member. Unconditionally defaulting to + // Member let a bare PUT_USER silently demote an existing owner/admin. + let role: MemberRole = match extract_tag_value(event, "role") { + Some(role_str) => role_str + .parse() + .map_err(|_| anyhow::anyhow!("invalid role: {role_str}"))?, + None => state + .db + .get_members(tenant.community(), channel_id) + .await? + .iter() + .find(|m| m.pubkey == target_pubkey) + .and_then(|m| m.role.parse().ok()) + .unwrap_or(MemberRole::Member), + }; let actor_bytes = event.pubkey.to_bytes().to_vec(); diff --git a/crates/buzz-test-client/tests/e2e_relay.rs b/crates/buzz-test-client/tests/e2e_relay.rs index 7a7f7e19f..6f59299ed 100644 --- a/crates/buzz-test-client/tests/e2e_relay.rs +++ b/crates/buzz-test-client/tests/e2e_relay.rs @@ -2475,3 +2475,441 @@ async fn test_reply_ingest_pushes_live_thread_summary() { client.disconnect().await.expect("disconnect"); } + +/// Read a member's authoritative role from the relay-signed kind:39002 member +/// list. The relay's own view of membership, not the client's — a kind:9000 can +/// be `accepted` (stored) while its membership side effect fails, so asserting +/// on the OK alone cannot see a broken write. +async fn member_role(url: &str, keys: &Keys, channel_id: &str, pubkey_hex: &str) -> Option { + let mut ws = BuzzTestClient::connect(url, keys).await.expect("connect"); + let sid = sub_id("members"); + let filter = Filter::new() + .kind(Kind::Custom(39002)) + .custom_tags(SingleLetterTag::lowercase(Alphabet::D), [channel_id]); + ws.subscribe(&sid, vec![filter]) + .await + .expect("subscribe 39002"); + let events = ws + .collect_until_eose(&sid, Duration::from_secs(5)) + .await + .expect("39002 EOSE"); + ws.disconnect().await.ok(); + // Latest 39002 wins; find this pubkey's tag. Shape is + // ["p", pubkey, relay_url, role] (side_effects.rs:1058-1062) — role is + // index 3, not 2. + events.iter().max_by_key(|e| e.created_at).and_then(|e| { + e.tags.iter().find_map(|t| { + let p = t.as_slice(); + (p.len() >= 4 && p[0] == "p" && p[1] == pubkey_hex).then(|| p[3].clone()) + }) + }) +} + +/// SECURITY REPRO (Dawn): can an unprivileged NON-MEMBER demote the owner of an +/// OPEN channel to `member` with a single kind:9000? Asserts the reported +/// vulnerability is FIXED; it fails on vulnerable code. +#[tokio::test] +#[ignore] +async fn test_nip29_put_user_cannot_demote_owner() { + let url = relay_url(); + + let victim_keys = Keys::generate(); + let victim_hex = victim_keys.public_key().to_hex(); + let attacker_keys = Keys::generate(); + + // Victim creates an open channel -> victim is seeded as its owner. + let channel_id = create_test_channel(&victim_keys).await; + + let before = member_role(&url, &victim_keys, &channel_id, &victim_hex).await; + assert_eq!( + before.as_deref(), + Some("owner"), + "victim must start as owner (got {before:?})" + ); + + // The attack: attacker (not a member, not the creator) publishes + // kind:9000 { h=channel, p=victim, role=member }. + let mut ws = BuzzTestClient::connect(&url, &attacker_keys) + .await + .expect("connect as attacker"); + let event = EventBuilder::new(Kind::Custom(9000), "") + .tags([ + Tag::parse(["h", &channel_id]).unwrap(), + Tag::parse(["p", &victim_hex]).unwrap(), + Tag::parse(["role", "member"]).unwrap(), + ]) + .sign_with_keys(&attacker_keys) + .expect("sign kind 9000"); + let ok = ws.send_event(event).await.expect("send kind 9000"); + ws.disconnect().await.ok(); + + let after = member_role(&url, &victim_keys, &channel_id, &victim_hex).await; + println!( + "attacker kind:9000 accepted = {} ({})", + ok.accepted, ok.message + ); + println!("victim role before = {before:?}, after = {after:?}"); + + assert_eq!( + after.as_deref(), + Some("owner"), + "PRIVESC: unprivileged non-member demoted the channel owner to {after:?}" + ); + assert!( + !ok.accepted, + "unprivileged non-member's role-demotion kind:9000 must be rejected" + ); +} + +/// SECURITY REPRO (Dawn), follow-on questions the report asserts but does not test: +/// after the owner is demoted, (a) can the attacker promote itself to owner, and +/// (b) can the ex-owner restore its own role? Both must be rejected — which is +/// exactly what makes the demotion unrecoverable over the relay. +#[tokio::test] +#[ignore] +async fn test_nip29_owner_demotion_recovery_paths() { + let url = relay_url(); + + let victim_keys = Keys::generate(); + let victim_hex = victim_keys.public_key().to_hex(); + let attacker_keys = Keys::generate(); + let attacker_hex = attacker_keys.public_key().to_hex(); + + let channel_id = create_test_channel(&victim_keys).await; + + let put_user = |signer: Keys, target_hex: String, role: &'static str| { + let channel_id = channel_id.clone(); + let url = url.clone(); + async move { + let mut ws = BuzzTestClient::connect(&url, &signer) + .await + .expect("connect"); + // `allow_self_tagging` is REQUIRED: EventBuilder otherwise silently + // drops any `p` tag matching the signer (nostr-0.44.3 + // builder.rs:435-449), which would make self-targeted PUT_USER + // events fail as "missing p tag" and mask the real verdict. + let event = EventBuilder::new(Kind::Custom(9000), "") + .allow_self_tagging() + .tags([ + Tag::parse(["h", &channel_id]).unwrap(), + Tag::parse(["p", &target_hex]).unwrap(), + Tag::parse(["role", role]).unwrap(), + ]) + .sign_with_keys(&signer) + .expect("sign kind 9000"); + let ok = ws.send_event(event).await.expect("send kind 9000"); + ws.disconnect().await.ok(); + ok + } + }; + + // Step 1: strip the owner. + let demote = put_user(attacker_keys.clone(), victim_hex.clone(), "member").await; + println!( + "1. attacker demotes owner -> accepted={} {}", + demote.accepted, demote.message + ); + + // Step 2: attacker tries to make itself owner. + let self_promote = put_user(attacker_keys.clone(), attacker_hex.clone(), "owner").await; + println!( + "2. attacker self->owner -> accepted={} {}", + self_promote.accepted, self_promote.message + ); + + // Step 3: ex-owner tries to restore itself. + let restore = put_user(victim_keys.clone(), victim_hex.clone(), "owner").await; + println!( + "3. ex-owner restores self -> accepted={} {}", + restore.accepted, restore.message + ); + + // `accepted` only means the event was stored — the membership side effect can + // still fail. Read the authoritative roles back from the relay-signed 39002. + let mut ws = BuzzTestClient::connect(&url, &victim_keys) + .await + .expect("connect"); + let sid = sub_id("members-final"); + let filter = Filter::new().kind(Kind::Custom(39002)).custom_tags( + SingleLetterTag::lowercase(Alphabet::D), + [channel_id.as_str()], + ); + ws.subscribe(&sid, vec![filter]) + .await + .expect("subscribe 39002"); + let events = ws + .collect_until_eose(&sid, Duration::from_secs(5)) + .await + .expect("39002 EOSE"); + ws.disconnect().await.ok(); + let latest = events.iter().max_by_key(|e| e.created_at).expect("a 39002"); + let roles: Vec<(String, String)> = latest + .tags + .iter() + .filter_map(|t| { + let p = t.as_slice(); + (p.len() >= 4 && p[0] == "p").then(|| (p[1].clone(), p[3].clone())) + }) + .collect(); + let role_of = |hex: &str| { + roles + .iter() + .find(|(pk, _)| pk == hex) + .map(|(_, r)| r.clone()) + }; + println!("FINAL victim role = {:?}", role_of(&victim_hex)); + println!("FINAL attacker role = {:?}", role_of(&attacker_hex)); + assert_eq!( + role_of(&victim_hex).as_deref(), + Some("owner"), + "owner must retain its role through all three attempts" + ); + assert_eq!( + role_of(&attacker_hex), + None, + "attacker must never gain a role" + ); +} + +/// SECURITY REPRO (Dawn), finding #2: a kind:9000 PUT_USER with NO `role` tag. +/// +/// `handle_put_user` used to default an absent role tag to `member`, so a bare +/// self-targeted PUT_USER silently demoted the sender — no attacker required. +/// `test_nip29_put_user_self_add_bypasses_policy` sends exactly this event but +/// asserts only `ok.accepted`, and side-effect failures are logged rather than +/// surfaced (ingest.rs:2460-2467), so an `accepted` assertion structurally +/// cannot observe the demotion. This asserts the resulting STATE instead. +/// +/// TWO owners on purpose. With a sole owner the last-owner guard rejects the +/// demotion anyway, so the test would pass even with the role-preservation +/// removed — it would be measuring a different defect's fix. A second owner +/// disarms that guard, leaving the absent-role-tag handling as the only thing +/// that can decide the outcome. +#[tokio::test] +#[ignore] +async fn test_nip29_put_user_without_role_tag_preserves_role() { + let url = relay_url(); + + let owner_a = Keys::generate(); + let owner_b = Keys::generate(); + let b_hex = owner_b.public_key().to_hex(); + let channel_id = create_test_channel(&owner_a).await; + + // owner_a promotes owner_b, so the channel has two owners. + let mut ws = BuzzTestClient::connect(&url, &owner_a) + .await + .expect("connect as owner_a"); + let promote = EventBuilder::new(Kind::Custom(9000), "") + .tags([ + Tag::parse(["h", &channel_id]).unwrap(), + Tag::parse(["p", &b_hex]).unwrap(), + Tag::parse(["role", "owner"]).unwrap(), + ]) + .sign_with_keys(&owner_a) + .expect("sign promote"); + let ok = ws.send_event(promote).await.expect("send promote"); + ws.disconnect().await.ok(); + assert!(ok.accepted, "promote rejected: {}", ok.message); + assert_eq!( + member_role(&url, &owner_a, &channel_id, &b_hex) + .await + .as_deref(), + Some("owner"), + "owner_b must be a second owner before the probe" + ); + + // The probe: owner_b sends a bare self-targeted PUT_USER — h + p, no `role`. + // `allow_self_tagging` is required or EventBuilder drops the self `p` tag + // (nostr-0.44.3 builder.rs:435-449) and the event fails as "missing p tag". + let mut ws = BuzzTestClient::connect(&url, &owner_b) + .await + .expect("connect as owner_b"); + let bare = EventBuilder::new(Kind::Custom(9000), "") + .allow_self_tagging() + .tags([ + Tag::parse(["h", &channel_id]).unwrap(), + Tag::parse(["p", &b_hex]).unwrap(), + ]) + .sign_with_keys(&owner_b) + .expect("sign bare put_user"); + let ok = ws.send_event(bare).await.expect("send bare put_user"); + ws.disconnect().await.ok(); + assert!( + ok.accepted, + "self-targeted PUT_USER must stay accepted: {}", + ok.message + ); + + assert_eq!( + member_role(&url, &owner_a, &channel_id, &b_hex) + .await + .as_deref(), + Some("owner"), + "an absent role tag means no role change — it must not demote an owner" + ); +} + +/// SECURITY (Dawn), relay-layer guard isolation — the *validator*, not the DB. +/// +/// The DB guards in `add_member` are what actually stop the privesc, and every +/// other test here asserts the resulting STATE ("the role didn't change"). +/// That makes them structurally blind to `validate_admin_event`: stubbing out +/// either relay-side check leaves the whole nip29 suite green, because the DB +/// still refuses the write and the role is still correct. Verified by mutation +/// — the relay returns `accepted:true` and logs `Side effect failed: access +/// denied: ...` while the state assertion happily passes. +/// +/// The relay guards earn their keep by giving the client an honest +/// `accepted:false` instead of an OK for an event that silently fails after the +/// fact. So these two tests assert `accepted == false` — the one observable +/// only the validator controls — and each is shaped so exactly one guard can +/// fire. +/// +/// Guard under test: "only owners/admins may change an active member's role". +/// TWO owners on purpose, so the last-owner guard cannot fire and take the +/// credit; a plain member targeting a co-owner leaves the actor check as the +/// only thing that can reject. +#[tokio::test] +#[ignore] +async fn test_nip29_relay_rejects_role_change_by_unprivileged_actor() { + let url = relay_url(); + + let owner_a = Keys::generate(); + let owner_b = Keys::generate(); + let b_hex = owner_b.public_key().to_hex(); + let attacker = Keys::generate(); + let attacker_hex = attacker.public_key().to_hex(); + let channel_id = create_test_channel(&owner_a).await; + + // owner_a promotes owner_b -> the channel has two owners. + let mut ws = BuzzTestClient::connect(&url, &owner_a) + .await + .expect("connect as owner_a"); + let promote = EventBuilder::new(Kind::Custom(9000), "") + .tags([ + Tag::parse(["h", &channel_id]).unwrap(), + Tag::parse(["p", &b_hex]).unwrap(), + Tag::parse(["role", "owner"]).unwrap(), + ]) + .sign_with_keys(&owner_a) + .expect("sign promote"); + let ok = ws.send_event(promote).await.expect("send promote"); + ws.disconnect().await.ok(); + assert!(ok.accepted, "promote rejected: {}", ok.message); + + // The attacker joins the open channel as a plain member. + let mut ws = BuzzTestClient::connect(&url, &attacker) + .await + .expect("connect as attacker"); + let join = EventBuilder::new(Kind::Custom(9000), "") + .allow_self_tagging() + .tags([ + Tag::parse(["h", &channel_id]).unwrap(), + Tag::parse(["p", &attacker_hex]).unwrap(), + Tag::parse(["role", "member"]).unwrap(), + ]) + .sign_with_keys(&attacker) + .expect("sign self-join"); + let ok = ws.send_event(join).await.expect("send self-join"); + ws.disconnect().await.ok(); + assert!(ok.accepted, "self-join rejected: {}", ok.message); + assert_eq!( + member_role(&url, &owner_a, &channel_id, &attacker_hex) + .await + .as_deref(), + Some("member"), + "attacker must be an active plain member before the probe" + ); + + // The probe: a plain member demotes a co-owner. Two owners remain, so only + // the actor-authorization guard can reject this. + let mut ws = BuzzTestClient::connect(&url, &attacker) + .await + .expect("connect as attacker"); + let attack = EventBuilder::new(Kind::Custom(9000), "") + .tags([ + Tag::parse(["h", &channel_id]).unwrap(), + Tag::parse(["p", &b_hex]).unwrap(), + Tag::parse(["role", "member"]).unwrap(), + ]) + .sign_with_keys(&attacker) + .expect("sign attack"); + let ok = ws.send_event(attack).await.expect("send attack"); + ws.disconnect().await.ok(); + println!( + "unprivileged co-owner demotion -> accepted={} {}", + ok.accepted, ok.message + ); + + assert!( + !ok.accepted, + "the relay validator must reject an unprivileged actor's role change, \ + not accept it and let the side effect fail silently" + ); + assert_eq!( + member_role(&url, &owner_a, &channel_id, &b_hex) + .await + .as_deref(), + Some("owner"), + "co-owner must keep their role" + ); +} + +/// SECURITY (Dawn), relay-layer guard isolation — see the test above for why +/// `accepted` rather than state is the assertion that matters here. +/// +/// Guard under test: the relay-side last-owner check. The actor is the SOLE +/// owner demoting themselves, so the actor-authorization guard is satisfied +/// (an owner is elevated) and cannot mask the result — only the last-owner +/// check can reject. +#[tokio::test] +#[ignore] +async fn test_nip29_relay_rejects_last_owner_self_demotion() { + let url = relay_url(); + + let owner = Keys::generate(); + let owner_hex = owner.public_key().to_hex(); + let channel_id = create_test_channel(&owner).await; + + assert_eq!( + member_role(&url, &owner, &channel_id, &owner_hex) + .await + .as_deref(), + Some("owner"), + "creator must be the sole owner before the probe" + ); + + // The probe: the sole owner demotes themselves. Elevated actor, so the + // actor check passes; the last-owner guard is the only thing left. + let mut ws = BuzzTestClient::connect(&url, &owner) + .await + .expect("connect as owner"); + let demote = EventBuilder::new(Kind::Custom(9000), "") + .allow_self_tagging() + .tags([ + Tag::parse(["h", &channel_id]).unwrap(), + Tag::parse(["p", &owner_hex]).unwrap(), + Tag::parse(["role", "member"]).unwrap(), + ]) + .sign_with_keys(&owner) + .expect("sign self-demote"); + let ok = ws.send_event(demote).await.expect("send self-demote"); + ws.disconnect().await.ok(); + println!( + "sole-owner self-demotion -> accepted={} {}", + ok.accepted, ok.message + ); + + assert!( + !ok.accepted, + "the relay validator must reject demoting the last owner, not accept \ + it and let the side effect fail silently" + ); + assert_eq!( + member_role(&url, &owner, &channel_id, &owner_hex) + .await + .as_deref(), + Some("owner"), + "the last owner must keep their role" + ); +} diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index ba35481bd..7e316dd54 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -98,6 +98,7 @@ export default defineConfig({ "**/inbox-reactions.spec.ts", "**/send-channel-binding.spec.ts", "**/project-commit-detail.spec.ts", + "**/project-inbox.spec.ts", "**/project-pr-review.spec.ts", "**/persona-model-combobox-screenshots.spec.ts", "**/drafts-screenshots.spec.ts", diff --git a/desktop/src-tauri/src/commands/messages.rs b/desktop/src-tauri/src/commands/messages.rs index 1928d69d2..afe94cdfe 100644 --- a/desktop/src-tauri/src/commands/messages.rs +++ b/desktop/src-tauri/src/commands/messages.rs @@ -68,7 +68,20 @@ pub async fn get_feed( // Mentions: messages that reference me via #p. let mut mention_filter = serde_json::json!({ - "kinds": [9, 40002, 1, 45001, 45003], + "kinds": [ + 9, + 40002, + 1, + 45001, + 45003, + buzz_core_pkg::kind::KIND_GIT_PULL_REQUEST, + buzz_core_pkg::kind::KIND_GIT_PR_UPDATE, + buzz_core_pkg::kind::KIND_GIT_ISSUE, + buzz_core_pkg::kind::KIND_GIT_STATUS_OPEN, + buzz_core_pkg::kind::KIND_GIT_STATUS_MERGED, + buzz_core_pkg::kind::KIND_GIT_STATUS_CLOSED, + buzz_core_pkg::kind::KIND_GIT_STATUS_DRAFT, + ], "#p": [my_pubkey], "limit": cap, }); diff --git a/desktop/src/features/home/lib/inbox.ts b/desktop/src/features/home/lib/inbox.ts index b4c05ef80..a34ea1b66 100644 --- a/desktop/src/features/home/lib/inbox.ts +++ b/desktop/src/features/home/lib/inbox.ts @@ -6,6 +6,10 @@ import { getThreadReference, isBroadcastReply, } from "@/features/messages/lib/threading"; +import { + getProjectInboxReference, + isProjectInboxItem, +} from "@/features/home/lib/projectInbox"; import type { TimelineReaction } from "@/features/messages/types"; import type { Channel, @@ -18,6 +22,7 @@ import { resolveMentionProps } from "@/shared/lib/resolveMentionNames"; export type InboxFilter = | "all" + | "project" | "mention" | "thread" | "needs_action" @@ -29,10 +34,10 @@ export type InboxFilter = export type InboxItem = { avatarUrl: string | null; /** - * Stable conversation identity: `rootId ?? parentId ?? event.id` for the - * thread group. Does NOT change when a new reply advances the representative - * latest event. Use this for lifecycle continuity: scroll gating, draft - * keys, local-reply storage, and selection identity. + * Stable conversation identity: the NIP-10 root for messages, or a + * repository-scoped root for Buzz Git work. Does NOT change when a new reply + * advances the representative latest event. Use this for lifecycle + * continuity: scroll gating, draft keys, local-reply storage, and selection. */ conversationId: string; id: string; @@ -136,7 +141,33 @@ function diffInDays(from: Date, to: Date) { ); } -function feedHeadline(item: FeedItem) { +function tagValue(item: FeedItem, name: string) { + return item.tags.find((tag) => tag[0] === name)?.[1]?.trim() || null; +} + +function projectRootItem(item: FeedItem, groupItems: readonly FeedItem[]) { + return ( + groupItems.find( + (candidate) => candidate.kind === 1618 || candidate.kind === 1621, + ) ?? item + ); +} + +function projectTypeLabel(item: FeedItem) { + if (item.kind === 1618) return "Pull request"; + if (item.kind === 1621) return "Issue"; + return "Project update"; +} + +function feedHeadline(item: FeedItem, groupItems: readonly FeedItem[] = []) { + if (isProjectInboxItem(item)) { + const root = projectRootItem(item, groupItems); + return ( + (tagValue(root, "subject") ?? root.content.trim().split("\n")[0]) || + projectTypeLabel(root) + ); + } + switch (item.kind) { case 40007: return "Reminder"; @@ -242,6 +273,14 @@ function resolveGroupChannel( export function getInboxTypeLabel(item: InboxItem): InboxTypeLabel { const channelName = item.channelLabel; + if (item.groupItems.some(isProjectInboxItem)) { + const root = projectRootItem(item.item, item.groupItems); + return { + text: projectTypeLabel(root), + channelLabel: null, + }; + } + if (item.item.channelType === "dm") { return { text: item.senderLabel ? `DM from ${item.senderLabel}` : "DM", @@ -300,23 +339,50 @@ function categoryPriority(category: FeedItemCategory) { } function getInboxThreadKey(item: FeedItem) { + const projectReference = getProjectInboxReference(item); + if (projectReference) { + return `project:${projectReference.repoAddress}:${projectReference.rootId}`; + } + const thread = getThreadReference(item.tags); return thread.rootId ?? thread.parentId ?? item.id; } +function getStableConversationId(item: FeedItem) { + return getInboxItemConversationId(item); +} + /** - * Returns the stable conversation ID for any FeedItem or relay event: the - * NIP-10 root tag id, falling back to parent-reply tag id, then event id. + * Returns the stable conversation ID for any FeedItem or relay event. Buzz Git + * roots include their repository coordinate; messages use the NIP-10 root, + * parent-reply tag, then event id. * This is the same derivation used by `buildInboxItems` for `conversationId`. */ export function getInboxConversationId( tags: string[][], eventId: string, + kind?: number, ): string { + if (kind !== undefined) { + const projectReference = getProjectInboxReference({ + id: eventId, + kind, + tags, + }); + if (projectReference) { + return `project:${projectReference.repoAddress}:${projectReference.rootId}`; + } + } + const thread = getThreadReference(tags); return thread.rootId ?? thread.parentId ?? eventId; } +/** Returns the stable conversation identity for a complete Inbox feed item. */ +export function getInboxItemConversationId(item: FeedItem) { + return getInboxConversationId(item.tags, item.id, item.kind); +} + function formatInboxTimestamp(unixSeconds: number) { const date = new Date(unixSeconds * 1_000); const now = new Date(); @@ -436,7 +502,7 @@ export function buildInboxItems({ group.items.push(item); group.latestActivityAt = Math.max(group.latestActivityAt, item.createdAt); - if (item.id === threadKey) { + if (item.id === getStableConversationId(item)) { group.rootItem = item; } @@ -447,7 +513,8 @@ export function buildInboxItems({ .sort( ([, left], [, right]) => right.latestActivityAt - left.latestActivityAt, ) - .map(([conversationId, group]) => { + .map(([, group]) => { + const conversationId = getStableConversationId(group.items[0]); const latestItem = group.items.reduce((latest, current) => current.createdAt > latest.createdAt ? current : latest, ); @@ -461,7 +528,7 @@ export function buildInboxItems({ profiles, preferResolvedSelfLabel: true, }); - const subject = feedHeadline(item); + const subject = feedHeadline(item, group.items); const preview = feedPreview(item); const { mentionNames, mentionPubkeysByName } = resolveMentionProps( item.tags, diff --git a/desktop/src/features/home/lib/inboxViewHelpers.ts b/desktop/src/features/home/lib/inboxViewHelpers.ts index e87671b9f..869dbf181 100644 --- a/desktop/src/features/home/lib/inboxViewHelpers.ts +++ b/desktop/src/features/home/lib/inboxViewHelpers.ts @@ -3,6 +3,7 @@ import { type InboxContextMessage, type InboxFilter, } from "@/features/home/lib/inbox"; +import { isProjectInboxItem } from "@/features/home/lib/projectInbox"; import { getChannelIdFromTags, getThreadReference, @@ -39,6 +40,12 @@ export function matchesInboxFilter( ); } + if (filter === "project") { + return [item.item, ...(item.groupItems ?? [])].some( + (groupItem) => groupItem && isProjectInboxItem(groupItem), + ); + } + return item.categories.includes(filter); } diff --git a/desktop/src/features/home/lib/projectInbox.test.mjs b/desktop/src/features/home/lib/projectInbox.test.mjs new file mode 100644 index 000000000..040f54e84 --- /dev/null +++ b/desktop/src/features/home/lib/projectInbox.test.mjs @@ -0,0 +1,235 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { buildInboxItems, getInboxTypeLabel } from "./inbox.ts"; +import { matchesInboxFilter } from "./inboxViewHelpers.ts"; +import { + getProjectInboxReference, + isProjectInboxItem, + resolveProjectInboxWorkItem, +} from "./projectInbox.ts"; + +const OWNER = "a".repeat(64); +const REVIEWER = "b".repeat(64); +const REPO_ADDRESS = `30617:${OWNER}:buzz`; +const PR_ID = "c".repeat(64); +const ISSUE_ID = "d".repeat(64); + +function feedItem(overrides = {}) { + return { + id: PR_ID, + kind: 1618, + pubkey: OWNER, + content: "Inbox support", + createdAt: 1_700_000_000, + channelId: null, + channelName: "", + tags: [ + ["a", REPO_ADDRESS], + ["p", REVIEWER], + ["subject", "Add project work items to Inbox"], + ], + category: "mention", + ...overrides, + }; +} + +const project = { + id: "buzz", + name: "Buzz", + owner: OWNER, + repoAddress: REPO_ADDRESS, +}; + +const pullRequest = { + id: PR_ID, + author: OWNER, + title: "Add project work items to Inbox", +}; + +const issue = { + id: ISSUE_ID, + author: REVIEWER, + title: "Inbox issue", +}; + +test("recognizes project roots and project thread activity", () => { + assert.equal(isProjectInboxItem(feedItem()), true); + assert.equal( + isProjectInboxItem( + feedItem({ + id: "e".repeat(64), + kind: 1, + tags: [ + ["a", REPO_ADDRESS], + ["e", PR_ID, "", "root"], + ["p", REVIEWER], + ], + }), + ), + true, + ); + assert.equal( + isProjectInboxItem( + feedItem({ + kind: 9, + tags: [ + ["a", REPO_ADDRESS], + ["e", PR_ID, "", "root"], + ], + }), + ), + false, + ); +}); + +test("resolves the canonical project root from status and comment events", () => { + assert.deepEqual(getProjectInboxReference(feedItem()), { + repoAddress: REPO_ADDRESS, + rootId: PR_ID, + }); + assert.deepEqual( + getProjectInboxReference( + feedItem({ + id: "f".repeat(64), + kind: 1631, + tags: [ + ["a", REPO_ADDRESS], + ["e", PR_ID, "", "root"], + ], + }), + ), + { + repoAddress: REPO_ADDRESS, + rootId: PR_ID, + }, + ); +}); + +test("matches a selected inbox event to its canonical pull request or issue", () => { + const workItems = { + pullRequests: { + items: [{ project, pullRequest }], + failedSections: [], + }, + issues: { + items: [{ project, issue }], + failedSections: [], + }, + }; + + assert.deepEqual(resolveProjectInboxWorkItem(feedItem(), workItems), { + type: "pull-request", + project, + pullRequest, + }); + assert.deepEqual( + resolveProjectInboxWorkItem( + feedItem({ + id: ISSUE_ID, + kind: 1621, + tags: [ + ["a", REPO_ADDRESS], + ["p", OWNER], + ["subject", "Inbox issue"], + ], + }), + workItems, + ), + { + type: "issue", + project, + issue, + }, + ); +}); + +test("presents project work with its canonical subject and project filter", () => { + const [item] = buildInboxItems({ + feed: { + feed: { + mentions: [feedItem()], + needsAction: [], + activity: [], + agentActivity: [], + }, + meta: { since: 0, total: 1, generatedAt: 1_700_000_000 }, + }, + }); + + assert.equal(item.subject, "Add project work items to Inbox"); + assert.deepEqual(getInboxTypeLabel(item), { + text: "Pull request", + channelLabel: null, + }); + assert.equal(matchesInboxFilter(item, "project"), true); + assert.equal( + matchesInboxFilter( + { + ...item, + item: feedItem({ kind: 9, tags: [["h", "channel-id"]] }), + groupItems: [feedItem({ kind: 9, tags: [["h", "channel-id"]] })], + }, + "project", + ), + false, + ); +}); + +test("groups uppercase NIP-34 pull request updates with their root", () => { + const update = feedItem({ + id: "f".repeat(64), + kind: 1619, + createdAt: 1_700_000_100, + content: "Pushed another commit", + tags: [ + ["a", REPO_ADDRESS], + ["E", PR_ID], + ["p", REVIEWER], + ], + }); + const items = buildInboxItems({ + feed: { + feed: { + mentions: [feedItem(), update], + needsAction: [], + activity: [], + agentActivity: [], + }, + meta: { since: 0, total: 2, generatedAt: 1_700_000_100 }, + }, + }); + + assert.equal(items.length, 1); + assert.equal(items[0].id, update.id); + assert.equal(items[0].subject, "Add project work items to Inbox"); +}); + +test("does not group project events from different repositories", () => { + const otherRepoAddress = `30617:${"e".repeat(64)}:other`; + const items = buildInboxItems({ + feed: { + feed: { + mentions: [ + feedItem(), + feedItem({ + id: "f".repeat(64), + kind: 1619, + tags: [ + ["a", otherRepoAddress], + ["E", PR_ID], + ["p", REVIEWER], + ], + }), + ], + needsAction: [], + activity: [], + agentActivity: [], + }, + meta: { since: 0, total: 2, generatedAt: 1_700_000_100 }, + }, + }); + + assert.equal(items.length, 2); + assert.notEqual(items[0].conversationId, items[1].conversationId); +}); diff --git a/desktop/src/features/home/lib/projectInbox.ts b/desktop/src/features/home/lib/projectInbox.ts new file mode 100644 index 000000000..a22b35521 --- /dev/null +++ b/desktop/src/features/home/lib/projectInbox.ts @@ -0,0 +1,99 @@ +import type { + Project, + ProjectIssue, + ProjectPullRequest, +} from "@/features/projects/hooks"; +import type { ProjectsWorkItemsResult } from "@/features/projects/projectWorkItems"; +import type { FeedItem } from "@/shared/api/types"; +import { + KIND_GIT_ISSUE, + KIND_GIT_PR_UPDATE, + KIND_GIT_PULL_REQUEST, + KIND_GIT_STATUS_CLOSED, + KIND_GIT_STATUS_DRAFT, + KIND_GIT_STATUS_MERGED, + KIND_GIT_STATUS_OPEN, + KIND_TEXT_NOTE, +} from "@/shared/constants/kinds"; + +const PROJECT_ROOT_KINDS = new Set([KIND_GIT_PULL_REQUEST, KIND_GIT_ISSUE]); +const PROJECT_ACTIVITY_KINDS = new Set([ + KIND_TEXT_NOTE, + KIND_GIT_PR_UPDATE, + KIND_GIT_STATUS_OPEN, + KIND_GIT_STATUS_MERGED, + KIND_GIT_STATUS_CLOSED, + KIND_GIT_STATUS_DRAFT, +]); +const REPO_ADDRESS_PATTERN = /^30617:[0-9a-f]{64}:.+$/i; + +export type ProjectInboxWorkItem = + | { + type: "pull-request"; + project: Project; + pullRequest: ProjectPullRequest; + } + | { + type: "issue"; + project: Project; + issue: ProjectIssue; + }; + +function tagValue(item: Pick, name: string) { + return item.tags.find( + (tag) => tag[0] === name && typeof tag[1] === "string" && tag[1].length > 0, + )?.[1]; +} + +/** Returns the canonical Buzz Git repository and root event for an Inbox row. */ +export function getProjectInboxReference( + item: Pick, +): { repoAddress: string; rootId: string } | null { + const repoAddress = tagValue(item, "a"); + if (!repoAddress || !REPO_ADDRESS_PATTERN.test(repoAddress)) { + return null; + } + + if (PROJECT_ROOT_KINDS.has(item.kind)) { + return { repoAddress, rootId: item.id }; + } + + if (!PROJECT_ACTIVITY_KINDS.has(item.kind)) { + return null; + } + + const rootId = tagValue(item, "e") ?? tagValue(item, "E"); + return rootId ? { repoAddress, rootId } : null; +} + +/** Whether a feed event belongs to a Buzz Git pull request or issue thread. */ +export function isProjectInboxItem(item: FeedItem) { + return getProjectInboxReference(item) !== null; +} + +/** Resolves an Inbox event to the current canonical Buzz Git work item. */ +export function resolveProjectInboxWorkItem( + item: FeedItem, + workItems: ProjectsWorkItemsResult | undefined, +): ProjectInboxWorkItem | null { + const reference = getProjectInboxReference(item); + if (!reference || !workItems) { + return null; + } + + const pullRequestEntry = workItems.pullRequests.items.find( + ({ project, pullRequest }) => + project.repoAddress === reference.repoAddress && + pullRequest.id === reference.rootId, + ); + if (pullRequestEntry) { + return { type: "pull-request", ...pullRequestEntry }; + } + + const issueEntry = workItems.issues.items.find( + ({ issue, project }) => + project.repoAddress === reference.repoAddress && + issue.id === reference.rootId, + ); + return issueEntry ? { type: "issue", ...issueEntry } : null; +} diff --git a/desktop/src/features/home/ui/HomeView.tsx b/desktop/src/features/home/ui/HomeView.tsx index 992702f43..2bcb231e9 100644 --- a/desktop/src/features/home/ui/HomeView.tsx +++ b/desktop/src/features/home/ui/HomeView.tsx @@ -14,7 +14,7 @@ import { type InboxReply, buildInboxItems, formatInboxFullTimestamp, - getInboxConversationId, + getInboxItemConversationId, } from "@/features/home/lib/inbox"; import { useInboxSelectionAnchor } from "@/features/home/useInboxSelectionAnchor"; import { @@ -413,7 +413,7 @@ export function HomeView({ // correct row selected (by conversationId) even after the anchor event has // been displaced from groupItems by a newer representative. const latchedConversationId = activeLatchedItem - ? getInboxConversationId(activeLatchedItem.tags, activeLatchedItem.id) + ? getInboxItemConversationId(activeLatchedItem) : null; const selectedConversationId = selectedItemFromAll?.conversationId ?? latchedConversationId; diff --git a/desktop/src/features/home/ui/InboxDetailPane.tsx b/desktop/src/features/home/ui/InboxDetailPane.tsx index 23a174822..5d6d98323 100644 --- a/desktop/src/features/home/ui/InboxDetailPane.tsx +++ b/desktop/src/features/home/ui/InboxDetailPane.tsx @@ -6,6 +6,8 @@ import type { InboxItem, InboxReply, } from "@/features/home/lib/inbox"; +import { getProjectInboxReference } from "@/features/home/lib/projectInbox"; +import { ProjectInboxDetail } from "@/features/home/ui/ProjectInboxDetail"; import { ChannelMembersBar } from "@/features/channels/ui/ChannelMembersBar"; import { useCommunities } from "@/features/communities/useCommunities"; import { formatInboxTypeLabel } from "@/features/home/lib/inbox"; @@ -102,7 +104,23 @@ type InboxDetailPaneProps = { ) => Promise; }; -export function InboxDetailPane({ +/** Routes Inbox selections to their canonical message or Buzz Git detail. */ +export function InboxDetailPane(props: InboxDetailPaneProps) { + if (props.item && getProjectInboxReference(props.item.item)) { + return ( + + ); + } + + return ; +} + +function InboxMessageDetailPane({ agentPubkeys, canDelete, canOpenChannel, diff --git a/desktop/src/features/home/ui/InboxListPane.tsx b/desktop/src/features/home/ui/InboxListPane.tsx index b92008e7a..603ebc2be 100644 --- a/desktop/src/features/home/ui/InboxListPane.tsx +++ b/desktop/src/features/home/ui/InboxListPane.tsx @@ -50,6 +50,7 @@ import { VirtualizedList } from "@/shared/ui/VirtualizedList"; const FILTER_OPTIONS: Array<{ label: string; value: InboxFilter }> = [ { value: "all", label: "All" }, + { value: "project", label: "Projects" }, { value: "mention", label: "Mentions" }, { value: "thread", label: "Threads" }, { value: "needs_action", label: "Needs Action" }, diff --git a/desktop/src/features/home/ui/ProjectInboxDetail.tsx b/desktop/src/features/home/ui/ProjectInboxDetail.tsx new file mode 100644 index 000000000..c3d17e687 --- /dev/null +++ b/desktop/src/features/home/ui/ProjectInboxDetail.tsx @@ -0,0 +1,129 @@ +import { ArrowLeft } from "lucide-react"; + +import { useAppNavigation } from "@/app/navigation/useAppNavigation"; +import type { InboxItem } from "@/features/home/lib/inbox"; +import { resolveProjectInboxWorkItem } from "@/features/home/lib/projectInbox"; +import { ProjectInboxDetailPane } from "@/features/home/ui/ProjectInboxDetailPane"; +import { + useProjectsQuery, + useProjectsWorkItemsQuery, +} from "@/features/projects/hooks"; +import type { UserProfileLookup } from "@/features/profile/lib/identity"; +import { Button } from "@/shared/ui/button"; + +type ProjectInboxDetailProps = { + isSinglePanelView?: boolean; + item: InboxItem; + onBack?: () => void; + profiles?: UserProfileLookup; +}; + +function ProjectInboxStatus({ + message, + onBack, + onRetry, +}: { + message: string; + onBack?: () => void; + onRetry?: () => void; +}) { + return ( +
+ {onBack ? ( +
+ +
+ ) : null} +
+

{message}

+ {onRetry ? ( + + ) : null} +
+
+ ); +} + +/** Resolves and renders the live Buzz Git object selected from Inbox. */ +export function ProjectInboxDetail({ + isSinglePanelView = false, + item, + onBack, + profiles, +}: ProjectInboxDetailProps) { + const { goProject } = useAppNavigation(); + const projectsQuery = useProjectsQuery(); + const projectsWorkItemsQuery = useProjectsWorkItemsQuery( + projectsQuery.data ?? [], + ); + const workItem = resolveProjectInboxWorkItem( + item.item, + projectsWorkItemsQuery.data, + ); + + if (!workItem) { + const error = projectsQuery.error ?? projectsWorkItemsQuery.error; + const isLoading = + projectsQuery.isLoading || projectsWorkItemsQuery.isLoading; + return ( + { + void projectsQuery.refetch(); + void projectsWorkItemsQuery.refetch(); + } + : undefined + } + /> + ); + } + + const failedSections = + workItem.type === "pull-request" + ? projectsWorkItemsQuery.data?.pullRequests.failedSections + : projectsWorkItemsQuery.data?.issues.failedSections; + if (failedSections && failedSections.length > 0) { + return ( + void projectsWorkItemsQuery.refetch()} + /> + ); + } + + return ( + { + const workItemId = + workItem.type === "pull-request" + ? { pullRequestId: workItem.pullRequest.id } + : { issueId: workItem.issue.id }; + void goProject(workItem.project.id, workItemId); + }} + profiles={profiles} + workItem={workItem} + /> + ); +} diff --git a/desktop/src/features/home/ui/ProjectInboxDetailPane.tsx b/desktop/src/features/home/ui/ProjectInboxDetailPane.tsx new file mode 100644 index 000000000..5b80c13cb --- /dev/null +++ b/desktop/src/features/home/ui/ProjectInboxDetailPane.tsx @@ -0,0 +1,178 @@ +import { ArrowLeft, ExternalLink } from "lucide-react"; +import * as React from "react"; + +import { useCommunities } from "@/features/communities/useCommunities"; +import type { ProjectInboxWorkItem } from "@/features/home/lib/projectInbox"; +import { ProjectIssueDetail } from "@/features/projects/ui/ProjectIssuesPanel"; +import { + ProjectPullRequestDetail, + PullRequestDetailHeader, + PullRequestMetaRail, +} from "@/features/projects/ui/ProjectPullRequestsPanel"; +import { + resolveUserLabel, + type UserProfileLookup, +} from "@/features/profile/lib/identity"; +import { openProjectMergeRecoveryTerminal } from "@/shared/api/projectGit"; +import { useElementWidth } from "@/shared/hooks/use-mobile"; +import { TopChromeInsetHeader } from "@/shared/layout/TopChromeInsetHeader"; +import { cn } from "@/shared/lib/cn"; +import { normalizePubkey } from "@/shared/lib/pubkey"; +import { Button } from "@/shared/ui/button"; +import { UserAvatar } from "@/shared/ui/UserAvatar"; + +type ProjectInboxDetailPaneProps = { + isSinglePanelView?: boolean; + onBack?: () => void; + onOpenProject: () => void; + profiles?: UserProfileLookup; + workItem: ProjectInboxWorkItem; +}; + +/** Renders a canonical Buzz Git work item with its existing project actions. */ +export function ProjectInboxDetailPane({ + isSinglePanelView = false, + onBack, + onOpenProject, + profiles, + workItem, +}: ProjectInboxDetailPaneProps) { + const { activeCommunity } = useCommunities(); + const [detailContentRef, detailContentWidth] = + useElementWidth(); + const showSideRail = detailContentWidth >= 760; + const authorPubkey = + workItem.type === "pull-request" + ? workItem.pullRequest.author + : workItem.issue.author; + const authorLabel = resolveUserLabel({ profiles, pubkey: authorPubkey }); + const authorAvatarUrl = + profiles?.[normalizePubkey(authorPubkey)]?.avatarUrl ?? null; + const inboxTitle = `${authorLabel} sent you ${ + workItem.type === "pull-request" ? "a pull request" : "an issue" + }`; + const handleOpenMergeRecoveryTerminal = React.useCallback( + async (input: { + expectedCommit: string; + sourceBranch: string; + sourceCloneUrl: string; + targetBranch: string; + }) => { + if (workItem.type !== "pull-request") { + throw new Error("Merge recovery is only available for pull requests."); + } + const targetCloneUrl = workItem.project.cloneUrls[0]; + if (!targetCloneUrl) { + throw new Error("This project has no clone URL."); + } + return openProjectMergeRecoveryTerminal({ + ...input, + projectDtag: workItem.project.dtag, + reposDir: activeCommunity?.reposDir, + targetCloneUrl, + }); + }, + [activeCommunity?.reposDir, workItem], + ); + + return ( +
+ +
+
+
+ {isSinglePanelView && onBack ? ( + + ) : null} + +

+ {inboxTitle} +

+
+ +
+
+ + +
+
+
+ {workItem.type === "pull-request" ? ( +
+
+ + +
+ +
+ ) : ( + + )} +
+
+
+
+ ); +} diff --git a/desktop/src/features/onboarding/ui/MachineOnboardingFlow.tsx b/desktop/src/features/onboarding/ui/MachineOnboardingFlow.tsx index d7fc9071d..ca87c7663 100644 --- a/desktop/src/features/onboarding/ui/MachineOnboardingFlow.tsx +++ b/desktop/src/features/onboarding/ui/MachineOnboardingFlow.tsx @@ -171,7 +171,11 @@ export function MachineOnboardingFlow({ onClick={() => void loadFreshIdentity()} type="button" > - {isPending ? "Saving identity…" : "Create a new identity key"} + {isPending + ? "Loading identity…" + : selectedPubkey + ? "Continue setup" + : "Create a new identity key"} diff --git a/desktop/src/features/projects/ui/ProjectFeedRow.tsx b/desktop/src/features/projects/ui/ProjectFeedRow.tsx index ae5a47798..4747d0351 100644 --- a/desktop/src/features/projects/ui/ProjectFeedRow.tsx +++ b/desktop/src/features/projects/ui/ProjectFeedRow.tsx @@ -7,6 +7,7 @@ import type * as React from "react"; * trailing cluster (hash, id, comment count) on the right. */ export function ProjectFeedRow({ + eventId, meta, onOpen, statusIcon, @@ -14,6 +15,7 @@ export function ProjectFeedRow({ title, trailing, }: { + eventId?: string; meta: React.ReactNode; onOpen?: () => void; statusIcon?: React.ReactNode; @@ -24,6 +26,7 @@ export function ProjectFeedRow({ return (
diff --git a/desktop/src/features/projects/ui/ProjectIssuesPanel.tsx b/desktop/src/features/projects/ui/ProjectIssuesPanel.tsx index b3215aa11..104000e55 100644 --- a/desktop/src/features/projects/ui/ProjectIssuesPanel.tsx +++ b/desktop/src/features/projects/ui/ProjectIssuesPanel.tsx @@ -15,6 +15,7 @@ import { } from "@/features/profile/lib/identity"; import { relativeTime } from "@/features/projects/lib/projectsViewHelpers"; import type { ChannelMember } from "@/shared/api/types"; +import { cn } from "@/shared/lib/cn"; import { normalizePubkey } from "@/shared/lib/pubkey"; import { ProjectFeedRow, @@ -158,14 +159,17 @@ function IssueRow({ ); } -function IssueDetail({ +/** Full issue conversation and comment composer. */ +export function ProjectIssueDetail({ issue, profiles, project, + stackMetaRail = false, }: { issue: ProjectIssue; profiles?: UserProfileLookup; project: Project; + stackMetaRail?: boolean; }) { const commentMutation = useCreateProjectIssueCommentMutation(project); const authorLabel = resolveUserLabel({ profiles, pubkey: issue.author }); @@ -198,7 +202,12 @@ function IssueDetail({ ); return ( -
+
@@ -253,7 +262,11 @@ function IssueDetail({
- +
); } @@ -263,16 +276,23 @@ function IssueDetail({ function IssueMetaRail({ issue, profiles, + stacked = false, }: { issue: ProjectIssue; profiles?: UserProfileLookup; + stacked?: boolean; }) { const authorProfile = profiles?.[normalizePubkey(issue.author)]; const authorLabel = resolveUserLabel({ profiles, pubkey: issue.author }); const status = issueStatusVisual(issue.status); return ( -