diff --git a/.release/desktop-candidate.json b/.release/desktop-candidate.json index 2c06a06c6a..e668efa7b2 100644 --- a/.release/desktop-candidate.json +++ b/.release/desktop-candidate.json @@ -1,10 +1,10 @@ { "schema": 2, - "version": "0.5.14", - "base_sha": "1b3dbcaaea882eeea90359c1db02e306d2f4f50a", - "previous_tag": "desktop-v0.5.13", - "previous_base_sha": "09768100ec3420f0aa7cd278bd00fe0baab5de8d", - "previous_merge_sha": "51beba603886d34e751349d12b33c0c5aeb92c28", - "tag": "desktop-v0.5.14", - "commit_count": 1 + "version": "0.5.15", + "base_sha": "7f61cf431af1d8f0480a0baf525881a12f2be7f2", + "previous_tag": "desktop-v0.5.14", + "previous_base_sha": "1b3dbcaaea882eeea90359c1db02e306d2f4f50a", + "previous_merge_sha": "82f7ed1532f50e0d28afca5580ed522f1c2ef1ca", + "tag": "desktop-v0.5.15", + "commit_count": 18 } diff --git a/CHANGELOG.md b/CHANGELOG.md index 9248c6fbd7..8b701ec258 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,33 @@ # Changelog +## v0.5.15 + +### Desktop and shared changes + +- Preserve managed agent mentions during relay errors ([#6167](https://github.com/block/buzz/pull/6167)) ([`7f61cf431af1d8f0480a0baf525881a12f2be7f2`](https://github.com/block/buzz/commit/7f61cf431af1d8f0480a0baf525881a12f2be7f2)) +- fix(workflows): preserve multi-channel listing semantics ([#6009](https://github.com/block/buzz/pull/6009)) ([`f7a01bda7b1bf95cdbc9dc21bb69970955b14ecc`](https://github.com/block/buzz/commit/f7a01bda7b1bf95cdbc9dc21bb69970955b14ecc)) +- fix(desktop): align preview sidebar row styling ([#6163](https://github.com/block/buzz/pull/6163)) ([`439c03749182495ee09f85a73423dd17e7ccda61`](https://github.com/block/buzz/commit/439c03749182495ee09f85a73423dd17e7ccda61)) +- fix(desktop): repair dropped team membership links at boot and on edit ([#5904](https://github.com/block/buzz/pull/5904)) ([`57feca2f20bb3434d70ce770b9ed98b1c1472332`](https://github.com/block/buzz/commit/57feca2f20bb3434d70ce770b9ed98b1c1472332)) +- Rename Bumble agent to Pollen ([#5864](https://github.com/block/buzz/pull/5864)) ([`076081bfc646f8fdf8ff9dc6e00843b5bdae0ad0`](https://github.com/block/buzz/commit/076081bfc646f8fdf8ff9dc6e00843b5bdae0ad0)) +- fix(desktop): resolve agent profiles through one archive-aware selector ([#5706](https://github.com/block/buzz/pull/5706)) ([`d12d82577818a95babac4d30cf242c46124feb5e`](https://github.com/block/buzz/commit/d12d82577818a95babac4d30cf242c46124feb5e)) +- feat(workflows): add responsive library card actions ([#6008](https://github.com/block/buzz/pull/6008)) ([`edc4a09aaa41c29e2495a28247c895febaf6587d`](https://github.com/block/buzz/commit/edc4a09aaa41c29e2495a28247c895febaf6587d)) +- fix(desktop): enforce shared agent access across devices ([#6086](https://github.com/block/buzz/pull/6086)) ([`f716eef437dcf91994518b8df7f581e86bb51748`](https://github.com/block/buzz/commit/f716eef437dcf91994518b8df7f581e86bb51748)) +- feat(model-capabilities): drive model capabilities and labels from one manifest ([#5597](https://github.com/block/buzz/pull/5597)) ([`1b7e5ac1be641f5ecc2b2a0ba37a1dc400e073c9`](https://github.com/block/buzz/commit/1b7e5ac1be641f5ecc2b2a0ba37a1dc400e073c9)) +- fix(desktop): hide the offcanvas-collapsed sidebar so it stops painting over the community rail ([#5947](https://github.com/block/buzz/pull/5947)) ([`78cbffeb64c01220e705adf0aa9690fdbd0d7a37`](https://github.com/block/buzz/commit/78cbffeb64c01220e705adf0aa9690fdbd0d7a37)) + +### Other repository changes + +- Remove Startup Recovery section in base prompt ([#6161](https://github.com/block/buzz/pull/6161)) ([`f64899e5d17df4c928ea415a5f42052120edaecb`](https://github.com/block/buzz/commit/f64899e5d17df4c928ea415a5f42052120edaecb)) +- fix(cli): keep project replacement timestamps at or after wall clock ([#5666](https://github.com/block/buzz/pull/5666)) ([`a282e0643fe0f14ace4d9b57ead99d0635e38995`](https://github.com/block/buzz/commit/a282e0643fe0f14ace4d9b57ead99d0635e38995)) +- Remove GitHub security advisory commitment ([#6144](https://github.com/block/buzz/pull/6144)) ([`85bacea52b8359999f22c6ac07207a130809c488`](https://github.com/block/buzz/commit/85bacea52b8359999f22c6ac07207a130809c488)) +- fix(acp): gate relay-signed workflow messages on their attributed author ([#6129](https://github.com/block/buzz/pull/6129)) ([`54f11219efe6b2617ba74d1ef8701fb5413956d8`](https://github.com/block/buzz/commit/54f11219efe6b2617ba74d1ef8701fb5413956d8)) +- fix(acp): replace Goose native system prompt ([#5964](https://github.com/block/buzz/pull/5964)) ([`5b3f0375a26843d73b29b55cc2f3c313bd857ccb`](https://github.com/block/buzz/commit/5b3f0375a26843d73b29b55cc2f3c313bd857ccb)) +- docs: refresh agent development guidance ([#6049](https://github.com/block/buzz/pull/6049)) ([`f956e6fe06a76e50cbd8fba1a162482e752e7f1a`](https://github.com/block/buzz/commit/f956e6fe06a76e50cbd8fba1a162482e752e7f1a)) +- feat(mobile): require device authentication for identity export ([#5116](https://github.com/block/buzz/pull/5116)) ([`d8281b9c93395f15d55091b131bb2747a0a3da8a`](https://github.com/block/buzz/commit/d8281b9c93395f15d55091b131bb2747a0a3da8a)) +- Polish mobile message threads and composer ([#5645](https://github.com/block/buzz/pull/5645)) ([`69107dc3bfecbb80cc5f5b8bb6a7647ad054ce57`](https://github.com/block/buzz/commit/69107dc3bfecbb80cc5f5b8bb6a7647ad054ce57)) + +[Compare desktop-v0.5.14...desktop-v0.5.15](https://github.com/block/buzz/compare/desktop-v0.5.14...desktop-v0.5.15) + ## v0.5.14 ### Desktop and shared changes diff --git a/crates/buzz-acp/src/base_prompt.md b/crates/buzz-acp/src/base_prompt.md index 83b9357171..a746e21762 100644 --- a/crates/buzz-acp/src/base_prompt.md +++ b/crates/buzz-acp/src/base_prompt.md @@ -87,13 +87,6 @@ All replies and delegations — including task assignments to other agents — g - Use top-level channel-visible posts for milestones teammates must act on: picked up, blocked + need input, PR up, done. - Praise in public; correct in the work, not the person. -## Startup Recovery - -1. `buzz feed get` — surface pending mentions and action items. Filter by type: `mentions`, `needs_action`, `activity`, `agent_activity`. -2. `buzz messages get --channel ` on assigned channels — catch up on recent history. -3. Check `AGENTS.md` in your working directory for team context. -4. Check `RESEARCH/`, `GUIDES/`, `PLANS/` before searching externally. Use `buzz messages search --query "..."` for cross-channel keyword lookups. - ## Workspace Layout Your persistent workspace is in your working directory: diff --git a/crates/buzz-cli/src/commands/projects.rs b/crates/buzz-cli/src/commands/projects.rs index 32056bc699..00e6f3efb9 100644 --- a/crates/buzz-cli/src/commands/projects.rs +++ b/crates/buzz-cli/src/commands/projects.rs @@ -4,8 +4,10 @@ //! 1. Fetch the caller's own live head via `kinds:[30621] + authors:[self] + #d:[slug]`. //! 2. Mutate the tag set (strip `auth`, apply change). //! 3. Re-validate the full envelope through Layer A before submitting. -//! 4. Set `created_at = head.created_at + 1` (never wall-clock) to avoid -//! overwriting a concurrently advancing head. +//! 4. Set `created_at = max(client_now, head.created_at + 1)` so the +//! replacement dominates the observed head and uses wall clock for +//! ordinary stale heads. Unusually future heads may still hit the relay's +//! timestamp-drift guard until time advances. //! //! Limitations recorded in this phase: //! - Relay hints are read-preserved but not authored (`--repo` carries @@ -136,13 +138,17 @@ async fn submit_project( // ── Build helpers ───────────────────────────────────────────────────────────── -/// Advance the `created_at` counter off an observed head. -fn next_timestamp(head: &Event) -> Result { - head.created_at +/// Choose the later of client wall clock and the instant after the observed head. +/// +/// The relay remains authoritative for timestamp drift: a sufficiently future +/// head can require a timestamp that the relay will temporarily reject. +fn next_timestamp(head: &Event, now: Timestamp) -> Result { + let after_head = head + .created_at .as_secs() .checked_add(1) - .map(Timestamp::from) - .ok_or_else(|| CliError::Other("project timestamp cannot be advanced".into())) + .ok_or_else(|| CliError::Other("project timestamp cannot be advanced".into()))?; + Ok(Timestamp::from(after_head.max(now.as_secs()))) } /// Strip `auth` from a tag list and pass the resulting envelope through @@ -312,7 +318,7 @@ pub async fn cmd_add_repo( let head = fetch_own_project(client, slug) .await? .ok_or_else(|| CliError::NotFound(format!("project {slug:?} not found")))?; - let next_ts = next_timestamp(&head)?; + let next_ts = next_timestamp(&head, Timestamp::now())?; // Build the new tag set: keep existing tags (including hinted members), // append new members only if not already present (by coordinate). @@ -366,7 +372,7 @@ pub async fn cmd_remove_repo( let head = fetch_own_project(client, slug) .await? .ok_or_else(|| CliError::NotFound(format!("project {slug:?} not found")))?; - let next_ts = next_timestamp(&head)?; + let next_ts = next_timestamp(&head, Timestamp::now())?; // Verify all requested repos exist in the project. let existing_coords: std::collections::HashSet = head @@ -458,7 +464,7 @@ pub async fn cmd_update( let head = fetch_own_project(client, slug) .await? .ok_or_else(|| CliError::NotFound(format!("project {slug:?} not found")))?; - let next_ts = next_timestamp(&head)?; + let next_ts = next_timestamp(&head, Timestamp::now())?; // Build the new tag set. For each singleton metadata field: // - setter present: replace value (strip old, append new) @@ -515,7 +521,7 @@ pub async fn cmd_update( /// /// Head-based and verified: /// 1. Fetch own live head — `NotFound` if absent. -/// 2. Build tombstone at `head.created_at + 1`. +/// 2. Build tombstone at `max(client_now, head.created_at + 1)`. /// 3. Submit. /// 4. Re-query the coordinate; if a newer head survived → `Conflict`. pub async fn cmd_delete(client: &BuzzClient, slug: &str) -> Result<(), CliError> { @@ -524,7 +530,7 @@ pub async fn cmd_delete(client: &BuzzClient, slug: &str) -> Result<(), CliError> let head = fetch_own_project(client, slug) .await? .ok_or_else(|| CliError::NotFound(format!("project {slug:?} not found")))?; - let next_ts = next_timestamp(&head)?; + let next_ts = next_timestamp(&head, Timestamp::now())?; let pubkey_hex = client.keys().public_key().to_hex(); let tombstone = build_delete_addressable(KIND_PROJECT, &pubkey_hex, slug) @@ -1003,31 +1009,51 @@ mod tests { // ── next_timestamp ordering ─────────────────────────────────────────────── - /// `next_timestamp` must return `head.created_at + 1` regardless of the wall - /// clock. NIP-MP Deletion rule: a tombstone older than the live head does - /// NOT remove it, so we must advance strictly off the observed head — never - /// use wall-clock time, which could be behind a head that was bumped - /// multiple times in the same second. - #[test] - fn next_timestamp_returns_head_plus_one_when_head_is_ahead_of_wall_clock() { - // Build a minimal signed event with a created_at far in the future. + fn project_head_at(created_at: u64) -> Event { let keys = nostr::Keys::generate(); - let far_future_ts = Timestamp::from(9_999_999_999u64); // year 2286 let tags = vec![ make_test_tag(&["d", "platform"]), make_test_tag(&["a", &format!("30617:{OWNER_HEX}:buzz")]), ]; - let builder = rebuild_project("", tags, far_future_ts).expect("valid head envelope"); - let head = builder.sign_with_keys(&keys).expect("sign"); - // Verify the event actually has our future timestamp. - assert_eq!(head.created_at, far_future_ts); + rebuild_project("", tags, Timestamp::from(created_at)) + .expect("valid head envelope") + .sign_with_keys(&keys) + .expect("sign") + } - // next_timestamp must return far_future + 1, not now(). - let next = next_timestamp(&head).expect("no overflow"); - assert_eq!( - next.as_secs(), - far_future_ts.as_secs() + 1, - "tombstone must be strictly after head, even when head is far in the future" + #[test] + fn next_timestamp_uses_later_of_wall_clock_and_after_head() { + let cases = [ + ("stale head", 100, 1_000, 1_000), + ("head equal to now", 1_000, 1_000, 1_001), + ("future head", 1_500, 1_000, 1_501), + ("last timestamp inside future boundary", 1_899, 1_000, 1_900), + ( + "future boundary cannot be dominated inside the window", + 1_900, + 1_000, + 1_901, + ), + ]; + + for (name, head_ts, now, expected) in cases { + let head = project_head_at(head_ts); + let next = next_timestamp(&head, Timestamp::from(now)).expect("no overflow"); + + assert_eq!(next.as_secs(), expected, "case: {name}"); + } + } + + #[test] + fn next_timestamp_rejects_overflowing_head() { + let head = project_head_at(u64::MAX); + + let err = next_timestamp(&head, Timestamp::from(1_000u64)) + .expect_err("maximum timestamp cannot be advanced"); + + assert!( + matches!(err, CliError::Other(ref message) if message == "project timestamp cannot be advanced"), + "unexpected error: {err}" ); } diff --git a/crates/buzz-core/src/filter.rs b/crates/buzz-core/src/filter.rs index 1671f76224..32e3a7ad16 100644 --- a/crates/buzz-core/src/filter.rs +++ b/crates/buzz-core/src/filter.rs @@ -184,6 +184,28 @@ mod tests { )); } + #[test] + fn h_tag_multi_value_filter_matches_any_channel() { + let channel_a = uuid::Uuid::new_v4(); + let channel_b = uuid::Uuid::new_v4(); + let stored = stored_with_tag(Tag::parse(["h", &channel_b.to_string()]).unwrap()); + let filter = Filter::new().custom_tags( + nostr::SingleLetterTag::lowercase(nostr::Alphabet::H), + [channel_a.to_string(), channel_b.to_string()], + ); + + assert!(filters_match(&[filter], &stored)); + } + + #[test] + fn empty_h_tag_filter_matches_nothing() { + let channel_id = uuid::Uuid::new_v4(); + let stored = stored_with_tag(Tag::parse(["h", &channel_id.to_string()]).unwrap()); + let filter: Filter = serde_json::from_value(serde_json::json!({ "#h": [] })).unwrap(); + + assert!(!filters_match(&[filter], &stored)); + } + #[test] fn h_tag_fallback_uses_stored_channel_id() { // Reactions (kind:7) and deletions (kind:5) don't carry h-tags — diff --git a/crates/buzz-db/src/event.rs b/crates/buzz-db/src/event.rs index db15057171..5d682d7843 100644 --- a/crates/buzz-db/src/event.rs +++ b/crates/buzz-db/src/event.rs @@ -70,10 +70,17 @@ pub struct EventQuery { /// Restrict results to events with an `e` tag referencing any of these event IDs (hex). /// Uses JSONB containment (`tags @> ...`) against the `tags` column. pub e_tags: Option>, - /// Restrict results to events in any of these channels, while retaining - /// channel-less global events. Applied before SQL `LIMIT` so access-filtered - /// historical pages have exact exhaustion semantics. + /// Restrict results to events in any of these channels. By default, + /// channel-less global events are retained so this can enforce a viewer's + /// accessible-channel scope without hiding global events. Set + /// [`EventQuery::channel_ids_include_global`] to `false` for an explicit + /// multi-channel `#h` filter, which must match only requested channels. + /// Applied before SQL `LIMIT` so access- and filter-scoped historical pages + /// have exact exhaustion semantics. pub channel_ids: Option>, + /// Whether [`EventQuery::channel_ids`] also retains channel-less global + /// events. Defaults to `true` for access-scope queries. + pub channel_ids_include_global: bool, /// Override the default page clamp ([`DEFAULT_MAX_PAGE_LIMIT`]). Used by /// the COUNT fallback path, which needs to fetch all matching events for /// post-filter counting. When None, the default clamp applies. @@ -122,6 +129,7 @@ impl EventQuery { ids: None, e_tags: None, channel_ids: None, + channel_ids_include_global: true, max_limit: None, shared_gated_reader: None, } @@ -404,20 +412,24 @@ pub(crate) async fn query_events_on( qb.push(format!(" AND {col_prefix}channel_id IS NULL")); } - // Multi-channel IN pushdown: restrict to events in any of these channels - // OR global events (channel_id IS NULL). Used by NIP-45 COUNT to enforce - // channel access at the SQL level without fetching all rows. + // Multi-channel IN pushdown. Access-scope queries retain global events; + // explicit multi-value #h filters do not. // - // SECURITY: Some(empty vec) means "user has access to NO channels" — - // only global events (channel_id IS NULL) should be returned. + // SECURITY: Some(empty vec) means "match no channels". Access-scope + // queries still retain globals; explicit #h queries match nothing. if let Some(ref ch_ids) = q.channel_ids { if ch_ids.is_empty() { - // No channel access — only global (non-channel) events visible. - qb.push(format!(" AND {col_prefix}channel_id IS NULL")); + if q.channel_ids_include_global { + qb.push(format!(" AND {col_prefix}channel_id IS NULL")); + } else { + qb.push(" AND FALSE"); + } } else { - qb.push(format!( - " AND ({col_prefix}channel_id IS NULL OR {col_prefix}channel_id IN (" - )); + qb.push(" AND ("); + if q.channel_ids_include_global { + qb.push(format!("{col_prefix}channel_id IS NULL OR ")); + } + qb.push(format!("{col_prefix}channel_id IN (")); let mut sep = qb.separated(", "); for ch in ch_ids { sep.push_bind(*ch); @@ -670,15 +682,21 @@ pub(crate) async fn count_events_on(conn: &mut sqlx::PgConnection, q: &EventQuer qb.push(format!(" AND {col_prefix}channel_id IS NULL")); } - // Multi-channel IN pushdown for COUNT: restrict to accessible channels + global. - // SECURITY: Some(empty vec) = no channel access → global events only. + // Multi-channel IN pushdown for COUNT. Access-scope queries retain global + // events; explicit multi-value #h filters do not. if let Some(ref ch_ids) = q.channel_ids { if ch_ids.is_empty() { - qb.push(format!(" AND {col_prefix}channel_id IS NULL")); + if q.channel_ids_include_global { + qb.push(format!(" AND {col_prefix}channel_id IS NULL")); + } else { + qb.push(" AND FALSE"); + } } else { - qb.push(format!( - " AND ({col_prefix}channel_id IS NULL OR {col_prefix}channel_id IN (" - )); + qb.push(" AND ("); + if q.channel_ids_include_global { + qb.push(format!("{col_prefix}channel_id IS NULL OR ")); + } + qb.push(format!("{col_prefix}channel_id IN (")); let mut sep = qb.separated(", "); for ch in ch_ids { sep.push_bind(*ch); @@ -1878,6 +1896,70 @@ mod tests { .expect("sign timestamped event") } + #[tokio::test] + #[ignore = "requires Postgres"] + async fn explicit_multi_channel_scope_is_applied_before_historical_page_limit() { + let pool = setup_pool().await; + let community_uuid = make_test_community(&pool).await; + let community = CommunityId::from_uuid(community_uuid); + let channel_a = make_test_channel(&pool, community_uuid, None).await; + let channel_b = make_test_channel(&pool, community_uuid, None).await; + let unrelated_c = make_test_channel(&pool, community_uuid, None).await; + let base = 1_800_000_000; + + let older_a = make_event_at(39_000, "older requested A", base + 1); + insert_event(&pool, community, &older_a, Some(channel_a)) + .await + .expect("insert requested A candidate"); + let requested_b = make_event_at(39_000, "requested B", base + 2); + insert_event(&pool, community, &requested_b, Some(channel_b)) + .await + .expect("insert requested B candidate"); + let newer_c = make_event_at(39_000, "newer unrelated C", base + 3); + insert_event(&pool, community, &newer_c, Some(unrelated_c)) + .await + .expect("insert unrelated C candidate"); + let global = make_event_at(39_000, "global candidate", base + 4); + insert_event(&pool, community, &global, None) + .await + .expect("insert global candidate"); + + let events = query_events( + &pool, + &EventQuery { + kinds: Some(vec![39_000]), + channel_ids: Some(vec![channel_a, channel_b]), + channel_ids_include_global: false, + limit: Some(1), + ..EventQuery::for_community(community) + }, + ) + .await + .expect("query explicit multi-channel page"); + + assert_eq!(events.len(), 1); + assert_eq!( + events[0].event.id, requested_b.id, + "newer unrelated channel C must not consume the requested A/B limit" + ); + + let partial_authorization_count = count_events( + &pool, + &EventQuery { + kinds: Some(vec![39_000]), + channel_ids: Some(vec![channel_a]), + channel_ids_include_global: false, + ..EventQuery::for_community(community) + }, + ) + .await + .expect("count one authorized channel from a multi-channel request"); + assert_eq!( + partial_authorization_count, 1, + "partial authorization must exclude requested B, unrelated C, and global rows" + ); + } + #[tokio::test] #[ignore = "requires Postgres"] async fn access_scope_is_applied_before_historical_page_limit() { diff --git a/crates/buzz-relay/src/api/bridge.rs b/crates/buzz-relay/src/api/bridge.rs index 0856c85cf3..8fdea4b3c0 100644 --- a/crates/buzz-relay/src/api/bridge.rs +++ b/crates/buzz-relay/src/api/bridge.rs @@ -981,6 +981,8 @@ async fn query_events_authed( .map(|v| serde_json::from_value(v.clone())) .collect::>() .map_err(|e| api_error(StatusCode::BAD_REQUEST, &format!("invalid filters: {e}")))?; + crate::handlers::req::extract_channel_ids_from_filters_limited(&filters) + .map_err(|()| api_error(StatusCode::BAD_REQUEST, "too many explicit channels"))?; // P-gated kinds (gift wraps, member notifications, observer frames) require // the caller's own pubkey in the #p tag — same enforcement as WS REQ handler. @@ -1005,10 +1007,18 @@ async fn query_events_authed( } // Get channels this user can access — same enforcement as WS REQ handler. - let accessible_channels = state + let mut accessible_channels = state .get_accessible_channel_ids_cached(tenant.community(), &pubkey_bytes) .await .map_err(|e| internal_error(&format!("channel access lookup: {e}")))?; + repair_requested_channel_access( + state, + tenant, + &filters, + &pubkey_bytes, + &mut accessible_channels, + ) + .await?; if filters.iter().any(|f| f.search.is_some()) { if has_mixed_search_filters(&filters) { @@ -1234,8 +1244,9 @@ async fn query_events_authed( tenant.community(), ) .await; - crate::handlers::req::apply_access_scope_to_query( + crate::handlers::req::apply_channel_scope_to_query( &mut query, + filter, extract_channel_from_filter(filter), &accessible_channels, ); @@ -1324,6 +1335,39 @@ async fn query_events_authed( Ok(Json(Value::Array(events))) } +async fn repair_requested_channel_access( + state: &AppState, + tenant: &TenantContext, + filters: &[nostr::Filter], + pubkey_bytes: &[u8], + accessible_channels: &mut Vec, +) -> Result<(), (StatusCode, Json)> { + for filter in filters { + let Some(requested) = + crate::handlers::req::extract_channel_ids_from_filters(std::slice::from_ref(filter)) + else { + continue; + }; + for channel_id in requested { + if accessible_channels.contains(&channel_id) { + continue; + } + let is_member = state + .db + .is_member(tenant.community(), channel_id, pubkey_bytes) + .await + .map_err(|e| internal_error(&format!("channel membership confirmation: {e}")))?; + crate::handlers::req::resolve_request_local_access( + accessible_channels, + channel_id, + true, + Some(is_member), + ); + } + } + Ok(()) +} + /// Count events via HTTP bridge (NIP-98 auth). Returns `{"count": N}`. /// /// Enforces channel access: only counts events in channels the user can access. @@ -1415,6 +1459,8 @@ async fn count_events_authed( let filters: Vec = serde_json::from_slice(body) .map_err(|e| api_error(StatusCode::BAD_REQUEST, &format!("invalid filters: {e}")))?; + crate::handlers::req::extract_channel_ids_from_filters_limited(&filters) + .map_err(|()| api_error(StatusCode::BAD_REQUEST, "too many explicit channels"))?; // P-gated kinds enforcement — same as WS REQ and /query. let authed_pubkey_hex = pubkey.to_hex(); @@ -1438,10 +1484,18 @@ async fn count_events_authed( } // Get channels this user can access. - let accessible_channels = state + let mut accessible_channels = state .get_accessible_channel_ids_cached(tenant.community(), &pubkey_bytes) .await .map_err(|e| internal_error(&format!("channel access lookup: {e}")))?; + repair_requested_channel_access( + state, + tenant, + &filters, + &pubkey_bytes, + &mut accessible_channels, + ) + .await?; let mut total: u64 = 0; for filter in &filters { @@ -1463,9 +1517,19 @@ async fn count_events_authed( crate::handlers::req::filter_can_match_shared_gated_kinds(filter); // If filter targets a specific channel, verify access. - if let Some(ch_id) = extract_channel_from_filter(filter) { - if !accessible_channels.contains(&ch_id) { - continue; // Skip filters targeting inaccessible channels. + if crate::handlers::req::extract_channel_ids_from_filters(std::slice::from_ref(filter)) + .is_some() + { + let ch_id = extract_channel_from_filter(filter); + let requested = crate::handlers::req::extract_channel_ids_from_filters( + std::slice::from_ref(filter), + ) + .unwrap_or_default(); + if !requested + .iter() + .any(|channel_id| accessible_channels.contains(channel_id)) + { + continue; } // Channel is accessible — count with pushability check. let mut query = crate::handlers::req::build_event_query_from_filter( @@ -1475,6 +1539,12 @@ async fn count_events_authed( tenant.community(), ) .await; + crate::handlers::req::apply_channel_scope_to_query( + &mut query, + filter, + ch_id, + &accessible_channels, + ); // Shared-gated visibility pushdown: same as REQ and /query paths, so // the fallback's query_events call doesn't over-fetch private rows. if needs_shared_gate_filtering { diff --git a/crates/buzz-relay/src/connection.rs b/crates/buzz-relay/src/connection.rs index c37421e7e8..5fcfe70b91 100644 --- a/crates/buzz-relay/src/connection.rs +++ b/crates/buzz-relay/src/connection.rs @@ -24,7 +24,6 @@ use crate::state::{ run_registered_community_connection, AppState, CommunityConnectionControl, CommunityDisconnectReason, }; -use buzz_pubsub::EventTopic; /// Maximum time a new socket may hold a connection slot without completing NIP-42 auth. const AUTH_TIMEOUT: Duration = Duration::from_secs(5); @@ -287,10 +286,18 @@ async fn handle_active_connection( let _ = auth_timeout_task.await; for removed in state.sub_registry.remove_connection(conn.conn_id) { - state - .pubsub - .release_topic(&conn.tenant, topic_for_subscription(removed.channel_id)) - .await; + if removed.scope.is_global() { + state + .pubsub + .release_topic(&conn.tenant, buzz_pubsub::EventTopic::Global) + .await; + } + for &channel_id in removed.scope.channel_ids() { + state + .pubsub + .release_topic(&conn.tenant, buzz_pubsub::EventTopic::Channel(channel_id)) + .await; + } } state.conn_manager.deregister(conn.conn_id); if let AuthState::Authenticated(ref auth_ctx) = *conn.auth_state.read().await { @@ -729,13 +736,6 @@ fn send_admission_result( } } -fn topic_for_subscription(channel_id: Option) -> EventTopic { - match channel_id { - Some(channel_id) => EventTopic::Channel(channel_id), - None => EventTopic::Global, - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/crates/buzz-relay/src/handlers/close.rs b/crates/buzz-relay/src/handlers/close.rs index d8ad1aa51f..86f3d0da79 100644 --- a/crates/buzz-relay/src/handlers/close.rs +++ b/crates/buzz-relay/src/handlers/close.rs @@ -5,7 +5,6 @@ use tracing::debug; use crate::connection::ConnectionState; use crate::protocol::RelayMessage; use crate::state::AppState; -use buzz_pubsub::EventTopic; /// Handle a CLOSE command — remove the subscription and send CLOSED acknowledgement. pub async fn handle_close(sub_id: String, conn: Arc, state: Arc) { @@ -16,20 +15,21 @@ pub async fn handle_close(sub_id: String, conn: Arc, state: Arc // Deregister from the fan-out index before sending CLOSED so no new // messages are routed to this sub after the client's CLOSE is acknowledged. if let Some(removed) = state.sub_registry.remove_subscription(conn_id, &sub_id) { - state - .pubsub - .release_topic(&conn.tenant, topic_for_subscription(removed.channel_id)) - .await; + if removed.scope.is_global() { + state + .pubsub + .release_topic(&conn.tenant, buzz_pubsub::EventTopic::Global) + .await; + } + for &channel_id in removed.scope.channel_ids() { + state + .pubsub + .release_topic(&conn.tenant, buzz_pubsub::EventTopic::Channel(channel_id)) + .await; + } } conn.send(RelayMessage::closed(&sub_id, "")); debug!(conn_id = %conn_id, sub_id = %sub_id, "Subscription closed"); } - -fn topic_for_subscription(channel_id: Option) -> EventTopic { - match channel_id { - Some(channel_id) => EventTopic::Channel(channel_id), - None => EventTopic::Global, - } -} diff --git a/crates/buzz-relay/src/handlers/count.rs b/crates/buzz-relay/src/handlers/count.rs index 3eeab5e807..938674301e 100644 --- a/crates/buzz-relay/src/handlers/count.rs +++ b/crates/buzz-relay/src/handlers/count.rs @@ -13,20 +13,8 @@ use crate::handlers::req::{ use crate::protocol::RelayMessage; use crate::state::AppState; -/// Extract a channel UUID from a single filter's `#h` tag. -fn extract_channel_from_filter(filter: &Filter) -> Option { - let h_tag = nostr::SingleLetterTag::lowercase(nostr::Alphabet::H); - filter.generic_tags.get(&h_tag).and_then(|vs| { - if vs.len() == 1 { - vs.iter().next()?.parse::().ok() - } else { - None - } - }) -} - /// Handle a COUNT message: require auth, enforce channel access, execute filters, -/// return aggregate count. +/// and return the aggregate count. pub async fn handle_count( sub_id: String, filters: Vec, @@ -75,6 +63,23 @@ pub async fn handle_count( return; } + let requested_channel_sets = + match super::req::extract_channel_ids_from_filters_limited(&filters) { + Ok(_) => filters + .iter() + .map(|filter| { + super::req::extract_channel_ids_from_filters(std::slice::from_ref(filter)) + }) + .collect::>(), + Err(()) => { + conn.send(RelayMessage::closed( + &sub_id, + "restricted: too many explicit channels", + )); + return; + } + }; + // Get channels this user can access — same enforcement as WS REQ handler. let mut accessible_channels = match state .get_accessible_channel_ids_cached(conn.tenant.community(), &pubkey_bytes) @@ -98,7 +103,7 @@ pub async fn handle_count( // For each filter, count matching events with channel access enforcement. let mut total: u64 = 0; - for filter in &filters { + for (filter, requested_channels) in filters.iter().zip(requested_channel_sets) { // Determine if this filter can match author-only kinds — if so, the // fast-path count_events() cannot be used because it doesn't do // per-event author filtering. @@ -117,38 +122,50 @@ pub async fn handle_count( let needs_result_gated_filtering = filter_can_match_result_gated_kinds(filter) && !result_gated_count_safe_for_pushdown(filter, &authed_pubkey_hex); - if let Some(ch_id) = extract_channel_from_filter(filter) { - // Filter targets a specific channel — verify access. Mirrors the WS - // REQ handler: a cache-negative may be a stale miss on a non-writer - // pod, so confirm uncached and repair the Vec request-locally via - // `super::req::resolve_request_local_access` (so a just-added channel - // is counted, and any later filter on the same channel sees it too). - let db_is_member = if accessible_channels.contains(&ch_id) { - None - } else { - match state - .db - .is_member(conn.tenant.community(), ch_id, &pubkey_bytes) - .await - { - Ok(member) => Some(member), - Err(e) => { - warn!(sub_id = %sub_id, "Channel membership confirmation failed: {e}"); - conn.send(RelayMessage::closed(&sub_id, "error: database error")); - return; - } + if let Some(requested_channels) = requested_channels { + for &ch_id in &requested_channels { + if accessible_channels.contains(&ch_id) { + continue; } - }; - if !super::req::resolve_request_local_access( - &mut accessible_channels, - ch_id, - token_channel_ids + let token_allows = token_channel_ids .as_deref() - .is_none_or(|allowed| allowed.contains(&ch_id)), - db_is_member, - ) { - continue; // Skip filters targeting inaccessible channels. + .is_none_or(|allowed| allowed.contains(&ch_id)); + let db_is_member = if token_allows { + match state + .db + .is_member(conn.tenant.community(), ch_id, &pubkey_bytes) + .await + { + Ok(member) => Some(member), + Err(e) => { + warn!(sub_id = %sub_id, "Channel membership confirmation failed: {e}"); + conn.send(RelayMessage::closed(&sub_id, "error: database error")); + return; + } + } + } else { + None + }; + super::req::resolve_request_local_access( + &mut accessible_channels, + ch_id, + token_allows, + db_is_member, + ); } + let authorized_requested: Vec<_> = requested_channels + .iter() + .copied() + .filter(|channel_id| accessible_channels.contains(channel_id)) + .collect(); + if authorized_requested.is_empty() { + continue; + } + // Preserve the original explicit multi-channel shape even when + // authorization narrows it to one channel. The helper must write + // that intersection into `channel_ids`; synthesizing `Some(A)` here + // would leave a query built from multi-#h completely unscoped. + let ch_id = (requested_channels.len() == 1).then_some(authorized_requested[0]); // Channel is accessible — count with pushability check. let mut query = super::req::build_event_query_from_filter( filter, @@ -157,6 +174,12 @@ pub async fn handle_count( conn.tenant.community(), ) .await; + super::req::apply_channel_scope_to_query( + &mut query, + filter, + ch_id, + &accessible_channels, + ); // Shared-gated visibility pushdown: pre-filter the fallback // query_events candidate page before ORDER/LIMIT. if needs_shared_gate_filtering { diff --git a/crates/buzz-relay/src/handlers/req.rs b/crates/buzz-relay/src/handlers/req.rs index fd7deadf51..250fb4f9b9 100644 --- a/crates/buzz-relay/src/handlers/req.rs +++ b/crates/buzz-relay/src/handlers/req.rs @@ -33,6 +33,14 @@ const MAX_SUBSCRIPTIONS: usize = 1024; /// `buffer_unordered`), so dedupe/trace/error semantics are unchanged. pub(crate) const FILTER_QUERY_CONCURRENCY: usize = 4; +/// Maximum aggregate number of explicit `#h` values accepted in one REQ, +/// COUNT, HTTP `/query`, or HTTP `/count` request. +/// +/// Explicit channels may each require an uncached membership lookup and, for a +/// live WS subscription, a registry entry plus Redis topic retain. Bound the +/// values before any of that request-amplified work begins. +pub(crate) const MAX_EXPLICIT_CHANNEL_VALUES: usize = 128; + // Guard: keep the bound a small fraction of any sane Postgres pool size. // Raising it past this range requires re-running the relay bench and // reconsidering pool contention (see docs above). Compile-time — violating @@ -85,6 +93,18 @@ pub async fn handle_req( } }; + let channel_id = extract_channel_id_from_filters(&filters); + let requested_channel_ids = match extract_channel_ids_from_filters_limited(&filters) { + Ok(ids) => ids, + Err(()) => { + conn.send(RelayMessage::closed( + &sub_id, + "restricted: too many explicit channels", + )); + return; + } + }; + let mut accessible_channels = if filters_are_nip43_membership_only(&filters) { metrics::counter!("buzz_req_global_access_resolution_skips_total", "kind" => "13534") .increment(1); @@ -106,8 +126,6 @@ pub async fn handle_req( accessible_channels.retain(|channel_id| allowed.contains(channel_id)); } - let channel_id = extract_channel_id_from_filters(&filters); - // Build the conformance `AbstractState` once at request entry. The // `Option` only goes `None` on malformed pubkey bytes (already a // separate failure path elsewhere); on the hot read path this is @@ -126,50 +144,70 @@ pub async fn handle_req( // `resolve_request_local_access`). Running this ahead of the search branch // is what fixes the search false-miss: a `#h=` search would // otherwise be scoped against the stale vector and return empty. - if let Some(ch_id) = channel_id { - let token_allows = token_channel_ids - .as_deref() - .is_none_or(|allowed| allowed.contains(&ch_id)); - let db_is_member = if !token_allows || accessible_channels.contains(&ch_id) { - None - } else { - match state - .db - .is_member(conn.tenant.community(), ch_id, &pubkey_bytes) - .await - { - Ok(member) => { - if let Some(state_snap) = trace_state.as_ref() { - crate::conformance::record_req_authcheck( - &state.tracer, - state_snap, - ch_id, - member, - ); + if let Some(requested) = requested_channel_ids.as_ref() { + for &ch_id in requested { + let token_allows = token_channel_ids + .as_deref() + .is_none_or(|allowed| allowed.contains(&ch_id)); + let db_is_member = if !token_allows || accessible_channels.contains(&ch_id) { + None + } else { + match state + .db + .is_member(conn.tenant.community(), ch_id, &pubkey_bytes) + .await + { + Ok(member) => { + if let Some(state_snap) = trace_state.as_ref() { + crate::conformance::record_req_authcheck( + &state.tracer, + state_snap, + ch_id, + member, + ); + } + Some(member) + } + Err(e) => { + warn!(conn_id = %conn_id, "Channel membership confirmation failed: {e}"); + conn.send(RelayMessage::closed(&sub_id, "error: database error")); + return; } - Some(member) } - Err(e) => { - warn!(conn_id = %conn_id, "Channel membership confirmation failed: {e}"); - conn.send(RelayMessage::closed(&sub_id, "error: database error")); - return; - } - } - }; - if !resolve_request_local_access( - &mut accessible_channels, - ch_id, - token_allows, - db_is_member, - ) { - conn.send(RelayMessage::closed( - &sub_id, - "restricted: not a channel member", - )); - return; + }; + // An OR filter may include inaccessible channels; retain every + // authorized requested channel and silently omit the others. + resolve_request_local_access( + &mut accessible_channels, + ch_id, + token_allows, + db_is_member, + ); } } + let authorized_requested_channels = requested_channel_ids.as_ref().map(|requested| { + requested + .iter() + .copied() + .filter(|channel_id| accessible_channels.contains(channel_id)) + .collect::>() + }); + // Partial authorization preserves NIP-01 OR semantics by omitting only + // inaccessible branches. If no valid requested channel survives, retain the + // established single-channel contract: reject instead of registering a + // subscription that can never produce an event or a terminal notice. + if authorized_requested_channels + .as_ref() + .is_some_and(|authorized| authorized.is_empty()) + { + conn.send(RelayMessage::closed( + &sub_id, + "restricted: not a channel member", + )); + return; + } + // Applied BEFORE the NIP-50 search branch so that an authenticated member // cannot use `{"search":"...","kinds":[30174]}` (or similar for p-gated // kinds) to harvest indexed-but-globally-stored sensitive events. Search @@ -236,23 +274,39 @@ pub async fn handle_req( subs.insert(sub_id.clone(), filters.clone()); } - let replaced = state.sub_registry.register_scoped( - conn.tenant.community(), - conn_id, - sub_id.clone(), - filters.clone(), - channel_id, - ); + let replaced = if let Some(channel_ids) = authorized_requested_channels.as_ref() { + state.sub_registry.register_channels_scoped( + conn.tenant.community(), + conn_id, + sub_id.clone(), + filters.clone(), + channel_ids.clone(), + ) + } else { + state.sub_registry.register_scoped( + conn.tenant.community(), + conn_id, + sub_id.clone(), + filters.clone(), + None, + ) + }; if let Some(replaced) = replaced { + release_subscription_topics(&state, &conn.tenant, &replaced.scope).await; + } + if let Some(channel_ids) = authorized_requested_channels.as_ref() { + for &channel_id in channel_ids { + state + .pubsub + .retain_topic(&conn.tenant, EventTopic::Channel(channel_id)) + .await; + } + } else { state .pubsub - .release_topic(&conn.tenant, topic_for_subscription(replaced.channel_id)) + .retain_topic(&conn.tenant, EventTopic::Global) .await; } - state - .pubsub - .retain_topic(&conn.tenant, topic_for_subscription(channel_id)) - .await; debug!(conn_id = %conn_id, sub_id = %sub_id, "Subscription registered"); @@ -288,7 +342,12 @@ pub async fn handle_req( }; let mut params = filter_to_query_params(filter, per_filter_channel, conn.tenant.community()); - apply_access_scope_to_query(&mut params, per_filter_channel, &accessible_channels); + apply_channel_scope_to_query( + &mut params, + filter, + per_filter_channel, + &accessible_channels, + ); // Shared-gated visibility pushdown: set reader bytes so query_events // appends the SQL visibility clause before ORDER/LIMIT, preventing // newer private events from starving older shared ones off the page. @@ -785,11 +844,11 @@ pub(crate) fn count_fallback_exceeded(candidate_count: usize) -> bool { /// an exact count without post-filtering. /// /// Pushed constraints: kinds, authors (single or multi), ids, since, until, -/// channel_id (#h single), #p (single), #d (single, NIP-33-only kinds), #e (any), -/// channel_ids (injected by caller). +/// authorized channel scope (#h single or multi, injected by caller), #p (single), +/// #d (single, NIP-33-only kinds), #e (any). /// -/// Anything else (multi-#p, #t, #a, search, multi-#h, #d on non-NIP-33) -/// requires post-filtering and cannot use the fast COUNT path. +/// Anything else (multi-#p, #t, #a, search, #d on non-NIP-33) requires +/// post-filtering and cannot use the fast COUNT path. pub fn filter_fully_pushable(filter: &Filter) -> bool { // Check if filter exclusively targets NIP-33 kinds (needed for #d pushability). let is_nip33_only = filter.kinds.as_ref().is_some_and(|ks| { @@ -803,10 +862,8 @@ pub fn filter_fully_pushable(filter: &Filter) -> bool { let key = tag_key.to_string(); match key.as_str() { "h" => { - // Single #h is pushed as channel_id; multi-#h is not. - if tag_values.len() > 1 { - return false; - } + // The caller pushes the complete authorized #h set through + // EventQuery::channel_id/channel_ids before invoking COUNT. } "p" => { // Single #p is pushed via event_mentions join; multi is not. @@ -854,19 +911,20 @@ fn filters_are_nip43_membership_only(filters: &[Filter]) -> bool { }) } -/// Extract a channel UUID from a single filter's `#h` tag. +/// Extract the single channel UUID from a filter's `#h` tag. +/// +/// A multi-value `#h` filter has NIP-01 OR semantics, so it cannot be reduced +/// to one `EventQuery::channel_id` without dropping matches from the other +/// channels. Return `None` in that case and let the caller apply the accessible +/// channel set in SQL before the full filter is evaluated in Rust. fn extract_channel_id_from_filter(filter: &Filter) -> Option { - for (tag_key, tag_values) in filter.generic_tags.iter() { - let key = tag_key.to_string(); - if key == "h" { - for val in tag_values { - if let Ok(id) = val.parse::() { - return Some(id); - } - } - } + let h_tag = nostr::SingleLetterTag::lowercase(nostr::Alphabet::H); + let values = filter.generic_tags.get(&h_tag)?; + if values.len() != 1 { + return None; } - None + + values.iter().next()?.parse::().ok() } /// Convert a single NIP-01 filter into an [`EventQuery`] for the database. @@ -1002,30 +1060,96 @@ fn filter_to_query_params( } } -/// Push the caller's authorized channel set into logically global historical -/// queries so SQL `LIMIT` counts visible rows. Channel-less events remain in -/// scope by `EventQuery::channel_ids` contract; an explicit single-channel -/// filter keeps its narrower `channel_id` predicate. -pub(crate) fn apply_access_scope_to_query( +/// Push channel constraints into SQL before `LIMIT`. +/// +/// A valid multi-value `#h` is narrowed to the requested channels the reader +/// may access. Invalid values are ignored, and an empty authorized result is an +/// explicit match-nothing scope rather than a global query. Filters without +/// `#h` retain the full accessible-channel scope plus global events. +pub(crate) fn apply_channel_scope_to_query( query: &mut EventQuery, + filter: &Filter, channel_id: Option, accessible_channels: &[uuid::Uuid], ) { - if channel_id.is_none() { + if channel_id.is_some() { + return; + } + + let h_tag = nostr::SingleLetterTag::lowercase(nostr::Alphabet::H); + if let Some(values) = filter.generic_tags.get(&h_tag) { + query.channel_ids = Some( + values + .iter() + .filter_map(|value| value.parse::().ok()) + .filter(|requested| accessible_channels.contains(requested)) + .collect(), + ); + query.channel_ids_include_global = false; + } else { query.channel_ids = Some(accessible_channels.to_vec()); } } -/// Extract a single channel UUID from filter generic tags, or `None` if the -/// subscription is logically global. +/// Extract the complete channel set when every filter is explicitly #h-scoped. +/// `None` means at least one filter is community-global. /// -/// Checks the `"h"` tag key — channel-scoped subscriptions use `#h = `. -/// -/// Returns `None` when: -/// - Any filter has no channel tag (that filter matches all channels → global sub), or -/// - Multiple distinct channel UUIDs appear across filters (can't index under one channel). -/// -/// Callers that receive `None` treat the subscription as global (slow-path fan-out). +/// The aggregate value count is checked before UUID parsing or membership I/O; +/// duplicate and malformed values still consume the request budget. +pub(crate) fn extract_channel_ids_from_filters_limited( + filters: &[Filter], +) -> Result>, ()> { + let h_tag = nostr::SingleLetterTag::lowercase(nostr::Alphabet::H); + let value_count = filters.iter().try_fold(0usize, |count, filter| { + let additional = filter + .generic_tags + .get(&h_tag) + .map_or(0, |values| values.len()); + count.checked_add(additional).ok_or(()) + })?; + if value_count > MAX_EXPLICIT_CHANNEL_VALUES { + return Err(()); + } + + Ok(extract_channel_ids_from_filters(filters)) +} + +/// Extract the complete channel set without applying the aggregate request budget. +/// Callers that can trigger I/O must validate first with +/// [`extract_channel_ids_from_filters_limited`]. +pub(crate) fn extract_channel_ids_from_filters(filters: &[Filter]) -> Option> { + let h_tag = nostr::SingleLetterTag::lowercase(nostr::Alphabet::H); + let mut channel_ids = Vec::new(); + for filter in filters { + let values = filter.generic_tags.get(&h_tag)?; + for value in values { + if let Ok(channel_id) = value.parse::() { + if !channel_ids.contains(&channel_id) { + channel_ids.push(channel_id); + } + } + } + } + Some(channel_ids) +} + +async fn release_subscription_topics( + state: &AppState, + tenant: &TenantContext, + scope: &crate::subscription::SubscriptionScope, +) { + if scope.is_global() { + state.pubsub.release_topic(tenant, EventTopic::Global).await; + } else { + for &channel_id in scope.channel_ids() { + state + .pubsub + .release_topic(tenant, EventTopic::Channel(channel_id)) + .await; + } + } +} + fn extract_channel_id_from_filters(filters: &[Filter]) -> Option { let mut found_id: Option = None; for f in filters { @@ -1289,13 +1413,6 @@ pub(crate) fn author_only_filters_authorized(filters: &[Filter], authed_pubkey_h }) } -fn topic_for_subscription(channel_id: Option) -> EventTopic { - match channel_id { - Some(channel_id) => EventTopic::Channel(channel_id), - None => EventTopic::Global, - } -} - #[cfg(test)] mod tests { use super::*; @@ -1308,7 +1425,7 @@ mod tests { uuid::Uuid::new_v4(), )); - apply_access_scope_to_query(&mut query, None, &accessible); + apply_channel_scope_to_query(&mut query, &Filter::new(), None, &accessible); assert_eq!(query.channel_ids.as_deref(), Some(accessible.as_slice())); } @@ -1322,7 +1439,7 @@ mod tests { )); query.channel_id = Some(channel); - apply_access_scope_to_query(&mut query, Some(channel), &accessible); + apply_channel_scope_to_query(&mut query, &Filter::new(), Some(channel), &accessible); assert!(query.channel_ids.is_none()); assert_eq!(query.channel_id, Some(channel)); @@ -1551,6 +1668,171 @@ mod tests { assert_eq!(extract_channel_id_from_filters(&filters), Some(channel_id)); } + #[test] + fn extract_channel_id_from_multi_value_filter_returns_none() { + let channel_a = uuid::Uuid::new_v4(); + let channel_b = uuid::Uuid::new_v4(); + let filter: Filter = serde_json::from_value(serde_json::json!({ + "#h": [channel_a.to_string(), channel_b.to_string()], + })) + .unwrap(); + + assert_eq!(extract_channel_id_from_filter(&filter), None); + assert_eq!( + filter_to_query_params( + &filter, + extract_channel_id_from_filter(&filter), + buzz_core::tenant::CommunityId::from_uuid(uuid::Uuid::nil()), + ) + .channel_id, + None, + "multi-channel OR filters must not be narrowed to their first channel", + ); + } + + #[test] + fn valid_channel_union_survives_malformed_or_empty_explicit_siblings() { + let valid = uuid::Uuid::new_v4(); + for sibling in [ + serde_json::json!({"#h": ["not-a-uuid"]}), + serde_json::json!({"#h": []}), + ] { + let filters = [ + filter_with_channel(valid), + serde_json::from_value(sibling).expect("parse sibling filter"), + ]; + assert_eq!( + extract_channel_ids_from_filters(&filters), + Some(vec![valid]), + ); + } + + let malformed_only: Filter = + serde_json::from_value(serde_json::json!({"#h": ["not-a-uuid"]})) + .expect("parse malformed filter"); + assert_eq!( + extract_channel_ids_from_filters(&[malformed_only]), + Some(Vec::new()), + "malformed-only explicit scope must remain match-nothing, never global", + ); + } + + #[test] + fn explicit_channel_limit_is_aggregate_and_counts_every_value() { + let channel_values = |count: usize| { + (0..count) + .map(|_| uuid::Uuid::new_v4().to_string()) + .collect::>() + }; + let at_limit: Filter = serde_json::from_value(serde_json::json!({ + "#h": channel_values(MAX_EXPLICIT_CHANNEL_VALUES), + })) + .unwrap(); + assert!(extract_channel_ids_from_filters_limited(&[at_limit]).is_ok()); + + let first: Filter = serde_json::from_value(serde_json::json!({ + "#h": channel_values(MAX_EXPLICIT_CHANNEL_VALUES), + })) + .unwrap(); + let duplicate_over_limit: Filter = serde_json::from_value(serde_json::json!({ + "#h": [uuid::Uuid::nil().to_string()], + })) + .unwrap(); + assert_eq!( + extract_channel_ids_from_filters_limited(&[first, duplicate_over_limit]), + Err(()), + ); + + let global_then_over_limit = [ + Filter::new(), + serde_json::from_value(serde_json::json!({ + "#h": channel_values(MAX_EXPLICIT_CHANNEL_VALUES + 1), + })) + .unwrap(), + ]; + assert_eq!( + extract_channel_ids_from_filters_limited(&global_then_over_limit), + Err(()), + "a global filter must not hide an over-limit explicit filter", + ); + } + + #[test] + fn multi_value_h_scope_intersects_access_before_limit() { + let channel_a = uuid::Uuid::new_v4(); + let channel_b = uuid::Uuid::new_v4(); + let unrelated_c = uuid::Uuid::new_v4(); + let unauthorized = uuid::Uuid::new_v4(); + let filter: Filter = serde_json::from_value(serde_json::json!({ + "#h": [ + channel_a.to_string(), + channel_b.to_string(), + unauthorized.to_string(), + "not-a-uuid" + ], + "limit": 1 + })) + .unwrap(); + let mut query = filter_to_query_params( + &filter, + extract_channel_id_from_filter(&filter), + buzz_core::tenant::CommunityId::from_uuid(uuid::Uuid::nil()), + ); + + apply_channel_scope_to_query( + &mut query, + &filter, + None, + &[channel_a, channel_b, unrelated_c], + ); + + let scoped_channels = query.channel_ids.expect("explicit channel scope"); + assert_eq!(scoped_channels.len(), 2); + assert!(scoped_channels.contains(&channel_a)); + assert!(scoped_channels.contains(&channel_b)); + assert!(!query.channel_ids_include_global); + assert_eq!(query.limit, Some(1)); + } + + #[test] + fn multi_value_h_scope_remains_explicit_when_only_one_channel_is_authorized() { + let authorized = uuid::Uuid::new_v4(); + let unauthorized = uuid::Uuid::new_v4(); + let filter: Filter = serde_json::from_value(serde_json::json!({ + "#h": [authorized.to_string(), unauthorized.to_string()], + })) + .unwrap(); + let mut query = filter_to_query_params( + &filter, + extract_channel_id_from_filter(&filter), + buzz_core::tenant::CommunityId::from_uuid(uuid::Uuid::nil()), + ); + + apply_channel_scope_to_query(&mut query, &filter, None, &[authorized]); + + assert_eq!(query.channel_id, None); + assert_eq!(query.channel_ids, Some(vec![authorized])); + assert!(!query.channel_ids_include_global); + } + + #[test] + fn empty_or_unauthorized_h_scope_matches_nothing() { + for values in [serde_json::json!([]), serde_json::json!(["not-a-uuid"])] { + let filter: Filter = + serde_json::from_value(serde_json::json!({ "#h": values })).unwrap(); + let mut query = filter_to_query_params( + &filter, + None, + buzz_core::tenant::CommunityId::from_uuid(uuid::Uuid::nil()), + ); + + apply_channel_scope_to_query(&mut query, &filter, None, &[uuid::Uuid::new_v4()]); + + assert_eq!(query.channel_ids, Some(Vec::new())); + assert!(!query.channel_ids_include_global); + } + } + #[test] fn test_extract_channel_id_mixed_channels_returns_none() { let channel_a = uuid::Uuid::new_v4(); diff --git a/crates/buzz-relay/src/handlers/side_effects.rs b/crates/buzz-relay/src/handlers/side_effects.rs index 0dc6cbd503..282ea77657 100644 --- a/crates/buzz-relay/src/handlers/side_effects.rs +++ b/crates/buzz-relay/src/handlers/side_effects.rs @@ -116,20 +116,24 @@ async fn evict_conn_channel_subscriptions( if let Some(subscriptions) = state.conn_manager.subscriptions_for(conn_id) { let mut conn_subscriptions = subscriptions.lock().await; - for (sub_id, _) in &removed { - conn_subscriptions.remove(sub_id); + for update in &removed { + if update.removed { + conn_subscriptions.remove(&update.sub_id); + } } } - for (sub_id, removed_scope) in removed { + for update in removed { state .pubsub - .release_topic(tenant, topic_for_subscription(removed_scope.channel_id)) + .release_topic(tenant, buzz_pubsub::EventTopic::Channel(channel_id)) .await; - let _ = state.conn_manager.send_to( - conn_id, - RelayMessage::closed(&sub_id, "restricted: channel access revoked"), - ); + if update.removed { + let _ = state.conn_manager.send_to( + conn_id, + RelayMessage::closed(&update.sub_id, "restricted: channel access revoked"), + ); + } } } @@ -3367,13 +3371,6 @@ pub async fn publish_nipia_unarchived( .await } -fn topic_for_subscription(channel_id: Option) -> EventTopic { - match channel_id { - Some(channel_id) => EventTopic::Channel(channel_id), - None => EventTopic::Global, - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/crates/buzz-relay/src/subscription.rs b/crates/buzz-relay/src/subscription.rs index 7a62188d3a..3a82ea27f5 100644 --- a/crates/buzz-relay/src/subscription.rs +++ b/crates/buzz-relay/src/subscription.rs @@ -13,7 +13,39 @@ pub type ConnId = Uuid; /// Subscription identifier — the client-supplied string from a REQ message. pub type SubId = String; /// Stored subscription entry: filters paired with server-resolved community and optional channel scope. -pub type SubEntry = (Vec, CommunityId, Option); +pub type SubEntry = (Vec, CommunityId, SubscriptionScope); + +/// Server-resolved live-routing scope for a subscription. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SubscriptionScope { + /// Community-global events only. + Global, + /// Events from any of these authorized channels. + Channels(Vec), +} + +impl SubscriptionScope { + fn matches_channel(&self, channel_id: Option) -> bool { + match (self, channel_id) { + (Self::Global, None) => true, + (Self::Channels(channels), Some(channel_id)) => channels.contains(&channel_id), + _ => false, + } + } + + /// Return the channels retained by this routing scope. + pub fn channel_ids(&self) -> &[Uuid] { + match self { + Self::Global => &[], + Self::Channels(channels) => channels, + } + } + + /// Whether this routing scope retains the community-global topic. + pub fn is_global(&self) -> bool { + matches!(self, Self::Global) + } +} /// Index key combining a channel and event kind for O(1) fan-out lookups. #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -32,12 +64,21 @@ struct GlobalPKindIndexKey { } /// A removed subscription's server-resolved routing scope. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct RemovedSubscription { /// Server-resolved community this subscription belonged to. pub community_id: CommunityId, - /// Tenant-local channel scope; `None` means the community-global topic. - pub channel_id: Option, + /// Server-resolved topics retained by the removed subscription. + pub scope: SubscriptionScope, +} + +/// Result of removing one revoked channel from a live subscription scope. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ChannelSubscriptionUpdate { + /// Client-supplied subscription identifier. + pub sub_id: SubId, + /// Whether no authorized channels remain and the subscription was removed. + pub removed: bool, } /// Thread-safe registry of active subscriptions with targeted in-memory fan-out indexes. @@ -73,42 +114,77 @@ impl SubscriptionRegistry { sub_id: SubId, filters: Vec, channel_id: Option, + ) -> Option { + let scope = channel_id + .map(|channel_id| SubscriptionScope::Channels(vec![channel_id])) + .unwrap_or(SubscriptionScope::Global); + self.register_with_scope(community_id, conn_id, sub_id, filters, scope) + } + + /// Register a subscription under every authorized requested channel. + pub fn register_channels_scoped( + &self, + community_id: CommunityId, + conn_id: ConnId, + sub_id: SubId, + filters: Vec, + channel_ids: Vec, + ) -> Option { + self.register_with_scope( + community_id, + conn_id, + sub_id, + filters, + SubscriptionScope::Channels(channel_ids), + ) + } + + fn register_with_scope( + &self, + community_id: CommunityId, + conn_id: ConnId, + sub_id: SubId, + filters: Vec, + scope: SubscriptionScope, ) -> Option { let removed = self.remove_subscription(conn_id, &sub_id); - self.subs - .entry(conn_id) - .or_default() - .insert(sub_id.clone(), (filters.clone(), community_id, channel_id)); + self.subs.entry(conn_id).or_default().insert( + sub_id.clone(), + (filters.clone(), community_id, scope.clone()), + ); metrics::gauge!("buzz_subscriptions_active").increment(1.0); - if let Some(ch_id) = channel_id { - match extract_kinds_from_filters(&filters) { - None => { - // At least one filter has no `kinds` constraint — wildcard, - // this sub wants all kinds in this channel. - self.channel_wildcard_index - .entry((community_id, ch_id)) - .or_default() - .push((conn_id, sub_id.clone())); - } - Some(kinds) if kinds.is_empty() => { - // All filters had explicit empty kinds lists (`kinds: []`). - // Per NIP-01, `kinds: []` means "match no kinds" — this - // subscription will never receive any events. Do not index it - // anywhere; `filters_match` will reject all events at fan-out. - } - Some(kinds) => { - for kind in kinds { - let key = IndexKey { - channel_id: ch_id, - kind, - }; - self.channel_kind_index - .entry((community_id, key)) + if let SubscriptionScope::Channels(channel_ids) = &scope { + for ch_id in channel_ids { + let ch_id = *ch_id; + match extract_kinds_from_filters(&filters) { + None => { + // At least one filter has no `kinds` constraint — wildcard, + // this sub wants all kinds in this channel. + self.channel_wildcard_index + .entry((community_id, ch_id)) .or_default() .push((conn_id, sub_id.clone())); } + Some(kinds) if kinds.is_empty() => { + // All filters had explicit empty kinds lists (`kinds: []`). + // Per NIP-01, `kinds: []` means "match no kinds" — this + // subscription will never receive any events. Do not index it + // anywhere; `filters_match` will reject all events at fan-out. + } + Some(kinds) => { + for kind in kinds { + let key = IndexKey { + channel_id: ch_id, + kind, + }; + self.channel_kind_index + .entry((community_id, key)) + .or_default() + .push((conn_id, sub_id.clone())); + } + } } } } else { @@ -177,16 +253,16 @@ impl SubscriptionRegistry { F: FnOnce(), { let mut conn_subs = self.subs.get_mut(&conn_id)?; - let (filters, community_id, channel_id) = conn_subs.remove(sub_id)?; + let (filters, community_id, scope) = conn_subs.remove(sub_id)?; after_remove(); - self.remove_from_index(conn_id, sub_id, &filters, community_id, channel_id); + self.remove_from_index(conn_id, sub_id, &filters, community_id, &scope); drop(conn_subs); metrics::gauge!("buzz_subscriptions_active").decrement(1.0); Some(RemovedSubscription { community_id, - channel_id, + scope, }) } @@ -195,11 +271,11 @@ impl SubscriptionRegistry { let mut removed = Vec::new(); if let Some((_, conn_subs)) = self.subs.remove(&conn_id) { let count = conn_subs.len(); - for (sub_id, (filters, community_id, channel_id)) in &conn_subs { - self.remove_from_index(conn_id, sub_id, filters, *community_id, *channel_id); + for (sub_id, (filters, community_id, scope)) in &conn_subs { + self.remove_from_index(conn_id, sub_id, filters, *community_id, scope); removed.push(RemovedSubscription { community_id: *community_id, - channel_id: *channel_id, + scope: scope.clone(), }); } metrics::gauge!("buzz_subscriptions_active").decrement(count as f64); @@ -207,34 +283,58 @@ impl SubscriptionRegistry { removed } - /// Remove all subscriptions on `conn_id` scoped to `channel_id` in one community. + /// Remove one revoked channel from every matching subscription in a community. + /// Multi-channel subscriptions are re-indexed with their remaining scope; + /// subscriptions with no channels left are removed entirely. pub fn remove_channel_subscriptions_scoped( &self, community_id: CommunityId, conn_id: ConnId, channel_id: Uuid, - ) -> Vec<(SubId, RemovedSubscription)> { + ) -> Vec { let sub_ids: Vec = self .subs .get(&conn_id) .map(|conn_subs| { conn_subs .iter() - .filter_map(|(sub_id, (_, sub_community_id, sub_channel_id))| { - (*sub_community_id == community_id && *sub_channel_id == Some(channel_id)) - .then_some(sub_id.clone()) + .filter_map(|(sub_id, (_, sub_community_id, scope))| { + (*sub_community_id == community_id + && scope.channel_ids().contains(&channel_id)) + .then_some(sub_id.clone()) }) .collect() }) .unwrap_or_default(); - sub_ids - .into_iter() - .filter_map(|sub_id| { - let removed = self.remove_subscription(conn_id, &sub_id)?; - Some((sub_id, removed)) - }) - .collect() + let mut updates = Vec::with_capacity(sub_ids.len()); + for sub_id in sub_ids { + let Some(mut conn_subs) = self.subs.get_mut(&conn_id) else { + break; + }; + let Some((filters, _, scope)) = conn_subs.get_mut(&sub_id) else { + continue; + }; + let filters = filters.clone(); + let SubscriptionScope::Channels(channel_ids) = scope else { + continue; + }; + channel_ids.retain(|candidate| *candidate != channel_id); + let removed = channel_ids.is_empty(); + self.remove_from_index( + conn_id, + &sub_id, + &filters, + community_id, + &SubscriptionScope::Channels(vec![channel_id]), + ); + if removed { + conn_subs.remove(&sub_id); + metrics::gauge!("buzz_subscriptions_active").decrement(1.0); + } + updates.push(ChannelSubscriptionUpdate { sub_id, removed }); + } + updates } /// Test-only convenience wrapper preserving the original single-tenant test API. @@ -242,7 +342,8 @@ impl SubscriptionRegistry { pub fn remove_channel_subscriptions(&self, conn_id: ConnId, channel_id: Uuid) -> Vec { self.remove_channel_subscriptions_scoped(test_community(), conn_id, channel_id) .into_iter() - .map(|(sub_id, _)| sub_id) + .filter(|update| update.removed) + .map(|update| update.sub_id) .collect() } @@ -441,12 +542,12 @@ impl SubscriptionRegistry { seen: &mut HashSet<(ConnId, SubId)>, ) { if let Some(conn_subs) = self.subs.get(&conn_id) { - if let Some((filters, sub_community_id, sub_channel_id)) = conn_subs.get(sub_id) { + if let Some((filters, sub_community_id, scope)) = conn_subs.get(sub_id) { // Candidate snapshots can become stale while a same-ID replacement // moves the subscription. Re-check its authoritative scope before // matching so an old index entry cannot deliver across scopes. if *sub_community_id == community_id - && *sub_channel_id == event.channel_id + && scope.matches_channel(event.channel_id) && filters_match(filters, event) { let entry = (conn_id, sub_id.to_string()); @@ -466,42 +567,45 @@ impl SubscriptionRegistry { sub_id: &str, filters: &[Filter], community_id: CommunityId, - channel_id: Option, + scope: &SubscriptionScope, ) { - if let Some(ch_id) = channel_id { - match extract_kinds_from_filters(filters) { - // None = wildcard (at least one filter had no kinds constraint). - None => { - // Was in wildcard index. - if let Some(mut entries) = - self.channel_wildcard_index.get_mut(&(community_id, ch_id)) - { - entries.retain(|(cid, sid)| !(*cid == conn_id && sid == sub_id)); - if entries.is_empty() { - drop(entries); - self.channel_wildcard_index.remove(&(community_id, ch_id)); - } - } - } - Some(kinds) if kinds.is_empty() => { - // `kinds: []` subscriptions are never indexed (they match nothing), - // so there is nothing to remove here. - } - Some(kinds) => { - // Was in kind-specific index. - for kind in kinds { - let key = IndexKey { - channel_id: ch_id, - kind, - }; - if let Some(mut entries) = self - .channel_kind_index - .get_mut(&(community_id, key.clone())) + if let SubscriptionScope::Channels(channel_ids) = scope { + for ch_id in channel_ids { + let ch_id = *ch_id; + match extract_kinds_from_filters(filters) { + // None = wildcard (at least one filter had no kinds constraint). + None => { + // Was in wildcard index. + if let Some(mut entries) = + self.channel_wildcard_index.get_mut(&(community_id, ch_id)) { entries.retain(|(cid, sid)| !(*cid == conn_id && sid == sub_id)); if entries.is_empty() { drop(entries); - self.channel_kind_index.remove(&(community_id, key)); + self.channel_wildcard_index.remove(&(community_id, ch_id)); + } + } + } + Some(kinds) if kinds.is_empty() => { + // `kinds: []` subscriptions are never indexed (they match nothing), + // so there is nothing to remove here. + } + Some(kinds) => { + // Was in kind-specific index. + for kind in kinds { + let key = IndexKey { + channel_id: ch_id, + kind, + }; + if let Some(mut entries) = self + .channel_kind_index + .get_mut(&(community_id, key.clone())) + { + entries.retain(|(cid, sid)| !(*cid == conn_id && sid == sub_id)); + if entries.is_empty() { + drop(entries); + self.channel_kind_index.remove(&(community_id, key)); + } } } } @@ -686,6 +790,49 @@ mod tests { assert_eq!(matches[0].1, sub_id); } + #[test] + fn multi_channel_subscription_fans_out_only_requested_channels() { + let registry = SubscriptionRegistry::new(); + let conn_id = Uuid::new_v4(); + let channel_a = Uuid::new_v4(); + let channel_b = Uuid::new_v4(); + let unrelated = Uuid::new_v4(); + let sub_id = "multi-channel".to_string(); + let filters = vec![Filter::new() + .kind(Kind::TextNote) + .custom_tag( + SingleLetterTag::lowercase(Alphabet::H), + channel_a.to_string(), + ) + .custom_tag( + SingleLetterTag::lowercase(Alphabet::H), + channel_b.to_string(), + )]; + + registry.register_channels_scoped( + test_community(), + conn_id, + sub_id.clone(), + filters, + vec![channel_a, channel_b], + ); + + assert_eq!( + registry.fan_out(&make_stored_event(Kind::TextNote, Some(channel_a))), + vec![(conn_id, sub_id.clone())] + ); + assert_eq!( + registry.fan_out(&make_stored_event(Kind::TextNote, Some(channel_b))), + vec![(conn_id, sub_id)] + ); + assert!(registry + .fan_out(&make_stored_event(Kind::TextNote, Some(unrelated))) + .is_empty()); + assert!(registry + .fan_out(&make_stored_event(Kind::TextNote, None)) + .is_empty()); + } + #[test] fn test_subscription_registry_remove() { let registry = SubscriptionRegistry::new(); @@ -1688,6 +1835,54 @@ mod tests { ); } + #[test] + fn revoking_one_channel_keeps_multi_channel_subscription_live() { + let registry = SubscriptionRegistry::new(); + let community = CommunityId::from_uuid(Uuid::from_u128(0xaaaa)); + let conn = Uuid::new_v4(); + let channel_a = Uuid::new_v4(); + let channel_b = Uuid::new_v4(); + let filters = vec![Filter::new().kind(Kind::TextNote)]; + registry.register_channels_scoped( + community, + conn, + "multi".to_string(), + filters, + vec![channel_a, channel_b], + ); + + let updates = registry.remove_channel_subscriptions_scoped(community, conn, channel_a); + assert_eq!( + updates, + vec![ChannelSubscriptionUpdate { + sub_id: "multi".to_string(), + removed: false, + }] + ); + assert!(registry + .fan_out_scoped( + community, + &make_stored_event(Kind::TextNote, Some(channel_a)) + ) + .is_empty()); + assert_eq!( + registry.fan_out_scoped( + community, + &make_stored_event(Kind::TextNote, Some(channel_b)) + ), + vec![(conn, "multi".to_string())] + ); + + let updates = registry.remove_channel_subscriptions_scoped(community, conn, channel_b); + assert_eq!( + updates, + vec![ChannelSubscriptionUpdate { + sub_id: "multi".to_string(), + removed: true, + }] + ); + } + #[test] fn per_community_subscriptions_snapshot_is_correctly_scoped() { // Verify that per_community_subscriptions() returns the correct diff --git a/crates/buzz-test-client/tests/e2e_relay.rs b/crates/buzz-test-client/tests/e2e_relay.rs index 5d5ad8916c..882cbabfe2 100644 --- a/crates/buzz-test-client/tests/e2e_relay.rs +++ b/crates/buzz-test-client/tests/e2e_relay.rs @@ -72,8 +72,9 @@ fn nip98_post_header(keys: &Keys, url: &str, body: &str) -> String { } async fn e2e_db_pool() -> sqlx::Pool { - let database_url = std::env::var("DATABASE_URL") - .unwrap_or_else(|_| "postgres://buzz:buzz_dev@localhost:5432/buzz".to_string()); + let database_url = std::env::var("DATABASE_URL").unwrap_or_else(|_| { + "postgres://buzz:buzz_dev@localhost:5432/buzz".to_string() // sadscan:disable np.postgres.1 + }); sqlx::postgres::PgPoolOptions::new() .max_connections(1) .connect(&database_url) @@ -721,6 +722,81 @@ async fn test_stored_events_returned_before_eose() { client.disconnect().await.expect("disconnect"); } +/// An explicit `#h` branch that cannot match must not cancel a valid OR sibling. +/// The valid channel remains usable for historical delivery and live fan-out; +/// malformed-only requests still close because no authorized UUID survives. +#[tokio::test] +#[ignore] +async fn test_valid_channel_survives_malformed_or_empty_h_sibling() { + let url = relay_url(); + let kind: u16 = 9; + let keys = Keys::generate(); + let channel = create_test_channel(&keys).await; + let mut client = BuzzTestClient::connect(&url, &keys).await.expect("connect"); + + for (label, sibling) in [ + ( + "malformed", + serde_json::json!({"kinds": [kind], "#h": ["not-a-uuid"]}), + ), + ("empty", serde_json::json!({"kinds": [kind], "#h": []})), + ] { + let historical = format!("{label}-historical-{}", Uuid::new_v4()); + let ok = client + .send_text_message(&keys, &channel, &historical, kind) + .await + .expect("send historical event"); + assert!(ok.accepted, "historical event rejected: {}", ok.message); + + let valid = Filter::new() + .kind(Kind::Custom(kind)) + .custom_tags(SingleLetterTag::lowercase(Alphabet::H), [channel.as_str()]); + let sibling: Filter = serde_json::from_value(sibling).expect("parse sibling filter"); + let sid = sub_id(label); + client + .subscribe(&sid, vec![valid, sibling]) + .await + .expect("subscribe"); + + let events = client + .collect_until_eose(&sid, Duration::from_secs(5)) + .await + .expect("valid sibling history followed by EOSE"); + assert!( + events.iter().any(|event| event.content == historical), + "valid sibling history missing for {label} #h branch: {events:?}", + ); + + let live = format!("{label}-live-{}", Uuid::new_v4()); + let ok = client + .send_text_message(&keys, &channel, &live, kind) + .await + .expect("send live event"); + assert!(ok.accepted, "live event rejected: {}", ok.message); + let message = client + .recv_event(Duration::from_secs(5)) + .await + .expect("receive post-EOSE live event"); + match message { + RelayMessage::Event { + subscription_id, + event, + } => { + assert_eq!(subscription_id, sid); + assert_eq!(event.content, live); + } + other => panic!("expected live EVENT for {label} sibling, got {other:?}"), + } + + client + .close_subscription(&sid) + .await + .expect("close subscription"); + } + + client.disconnect().await.expect("disconnect"); +} + /// Ephemeral events (kind 20000–29999) must be accepted but not persisted. #[tokio::test] #[ignore] diff --git a/desktop/package.json b/desktop/package.json index 39e93d8a98..963fe643d6 100644 --- a/desktop/package.json +++ b/desktop/package.json @@ -1,7 +1,7 @@ { "name": "buzz", "private": true, - "version": "0.5.14", + "version": "0.5.15", "type": "module", "scripts": { "dev": "vite", diff --git a/desktop/src-tauri/Cargo.lock b/desktop/src-tauri/Cargo.lock index 887e1282ff..7d5a3f67dc 100644 --- a/desktop/src-tauri/Cargo.lock +++ b/desktop/src-tauri/Cargo.lock @@ -1081,7 +1081,7 @@ dependencies = [ [[package]] name = "buzz-desktop" -version = "0.5.14" +version = "0.5.15" dependencies = [ "anyhow", "arboard", diff --git a/desktop/src-tauri/Cargo.toml b/desktop/src-tauri/Cargo.toml index 527690df14..6d14b04cf4 100644 --- a/desktop/src-tauri/Cargo.toml +++ b/desktop/src-tauri/Cargo.toml @@ -7,7 +7,7 @@ members = ["crates/buzz-terminal"] [package] name = "buzz-desktop" -version = "0.5.14" +version = "0.5.15" description = "Buzz desktop app" authors = ["you"] edition = "2021" diff --git a/desktop/src-tauri/src/commands/personas/inbound.rs b/desktop/src-tauri/src/commands/personas/inbound.rs index 5c38373f7c..42f720915a 100644 --- a/desktop/src-tauri/src/commands/personas/inbound.rs +++ b/desktop/src-tauri/src/commands/personas/inbound.rs @@ -255,8 +255,14 @@ fn reconcile_inbound_persona_event_blocking( } KIND_TEAM => { let mut teams = load_teams(&app)?; - apply_inbound_team(&mut teams, d_tag, team_content_from_event(&event)?); - save_teams(&app, &teams)?; + commit_inbound_team( + &mut teams, + d_tag, + team_content_from_event(&event)?, + |teams| save_teams(&app, teams), + || load_managed_agents(&app), + |records| save_managed_agents(&app, records), + )?; } KIND_MANAGED_AGENT => { let mut agents = load_managed_agents(&app)?; @@ -584,6 +590,53 @@ fn apply_inbound_managed_agent( false } +/// In-memory core of the inbound `KIND_TEAM` reconcile: capture the matched +/// team's roster *before* applying the inbound projection, apply it, persist +/// teams authoritatively, then propagate the prior→current membership delta to +/// live instances best-effort — the same binding semantics the local +/// create/update commands use. Without this, a 30176 team edit from another +/// device lands on `teams.json` but never touches `ManagedAgentRecord.team_id`: +/// an added persona's running instances stay unbound (member in roster, not in +/// behavior) and a removed persona's instances keep drawing the old team's +/// instructions at spawn until restart. +/// +/// A no-match insert has no prior roster, so its whole roster is the added +/// delta — symmetric with `commit_team_create`. Injected persistence keeps it +/// `AppHandle`-free so the prior-roster capture and delta direction are +/// unit-testable; a `persist_teams` error propagates, agent IO is best-effort +/// (mirrors the local command path: the authoritative team write already +/// landed, and boot repair is the designed retry for a stale binding). +fn commit_inbound_team( + teams: &mut Vec, + d_tag: String, + inbound: TeamEventContent, + persist_teams: impl FnOnce(&[TeamRecord]) -> Result<(), String>, + load_agents: impl FnOnce() -> Result, String>, + save_agents: impl FnOnce(&[ManagedAgentRecord]) -> Result<(), String>, +) -> Result<(), String> { + let team_id = d_tag.clone(); + let previous_persona_ids = teams + .iter() + .find(|record| record.id == team_id) + .map(|record| record.persona_ids.clone()) + .unwrap_or_default(); + apply_inbound_team(teams, d_tag, inbound); + let current_persona_ids = teams + .iter() + .find(|record| record.id == team_id) + .map(|record| record.persona_ids.clone()) + .unwrap_or_default(); + persist_teams(teams)?; + crate::commands::teams::propagate_membership_best_effort( + &team_id, + &previous_persona_ids, + ¤t_persona_ids, + load_agents, + save_agents, + ); + Ok(()) +} + /// Merge an inbound kind:30176 team projection into the local set. /// /// Matches the local record whose `id` equals the event's d-tag (the d-tag IS diff --git a/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs b/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs index e7834f8231..c7235bd034 100644 --- a/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs +++ b/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs @@ -546,6 +546,176 @@ fn inbound_team_no_match_inserts_idempotently() { assert_eq!(teams.len(), 2, "re-receive of inserted team no-ops"); } +// ── Inbound team → membership propagation (commit_inbound_team wiring) ───── + +use std::cell::RefCell; + +/// A running instance of `persona_id`, optionally bound to a team. +fn team_instance(seed: char, persona_id: &str, team_id: Option<&str>) -> ManagedAgentRecord { + let mut record = local_agent(); + record.pubkey = seed.to_string().repeat(64); + record.name = persona_id.to_string(); + record.persona_id = Some(persona_id.to_string()); + record.team_id = team_id.map(str::to_string); + record +} + +/// An inbound team edit that ADDS a persona must bind that persona's unbound +/// running instances to the team — exactly like a local `update_team`. Without +/// the propagation wiring the instance stays unbound (member in roster, not in +/// behavior) until restart. +#[test] +fn inbound_team_add_binds_unbound_instance_through_wiring() { + let mut teams = vec![local_team()]; + teams[0].persona_ids = vec!["p-existing".to_string()]; + let existing = vec![ + team_instance('a', "p-added", None), + team_instance('b', "p-existing", Some(TEAM_ID)), + ]; + let saved = RefCell::new(None); + + commit_inbound_team( + &mut teams, + TEAM_ID.to_string(), + TeamEventContent { + name: "Team".to_string(), + description: None, + instructions: None, + persona_ids: Some(vec!["p-existing".to_string(), "p-added".to_string()]), + }, + |_| Ok(()), + || Ok(existing.clone()), + |records| { + *saved.borrow_mut() = Some(records.to_vec()); + Ok(()) + }, + ) + .expect("inbound add succeeds"); + + let saved = saved + .borrow() + .clone() + .expect("add must save the agent store"); + assert_eq!( + saved[0].team_id.as_deref(), + Some(TEAM_ID), + "the added persona's unbound instance is bound to the team" + ); + assert_eq!( + saved[1].team_id.as_deref(), + Some(TEAM_ID), + "an instance already on the team is untouched" + ); +} + +/// An inbound team edit that REMOVES a persona ("keep agents") must detach that +/// persona's instances bound to this team, so a kept instance stops drawing the +/// team's instructions at spawn. +#[test] +fn inbound_team_removal_detaches_instance_through_wiring() { + let mut teams = vec![local_team()]; + teams[0].persona_ids = vec!["p-removed".to_string()]; + let existing = vec![team_instance('a', "p-removed", Some(TEAM_ID))]; + let saved = RefCell::new(None); + + commit_inbound_team( + &mut teams, + TEAM_ID.to_string(), + TeamEventContent { + name: "Team".to_string(), + description: None, + instructions: None, + persona_ids: Some(vec![]), + }, + |_| Ok(()), + || Ok(existing.clone()), + |records| { + *saved.borrow_mut() = Some(records.to_vec()); + Ok(()) + }, + ) + .expect("inbound removal succeeds"); + + let saved = saved + .borrow() + .clone() + .expect("removal must save the agent store"); + assert_eq!( + saved[0].team_id, None, + "the removed persona's instance is detached from the team" + ); +} + +/// An inbound edit that omits `persona_ids` (a pre-always-publish client) +/// preserves local membership, so the delta is empty and no instance is +/// re-pointed — a metadata-only inbound edit must not disturb bindings. +#[test] +fn inbound_team_omitted_roster_leaves_bindings_untouched() { + let mut teams = vec![local_team()]; + teams[0].persona_ids = vec!["p-a".to_string()]; + let existing = vec![team_instance('a', "p-a", None)]; + let saved = RefCell::new(None); + + commit_inbound_team( + &mut teams, + TEAM_ID.to_string(), + team_content_omitting_optional_fields("Renamed"), + |_| Ok(()), + || Ok(existing.clone()), + |records| { + *saved.borrow_mut() = Some(records.to_vec()); + Ok(()) + }, + ) + .expect("inbound metadata-only edit succeeds"); + + assert!( + saved.borrow().is_none(), + "an empty membership delta writes nothing to the agent store" + ); +} + +/// A failing agent-store write after the authoritative `save_teams` is +/// swallowed: the inbound reconcile still succeeds (boot repair is the retry), +/// so a secondary-store hiccup never aborts an inbound event whose team write +/// already landed. +#[test] +fn inbound_team_swallows_agent_store_failure() { + let mut teams = vec![local_team()]; + teams[0].persona_ids = vec![]; + commit_inbound_team( + &mut teams, + TEAM_ID.to_string(), + TeamEventContent { + name: "Team".to_string(), + description: None, + instructions: None, + persona_ids: Some(vec!["p-added".to_string()]), + }, + |_| Ok(()), + || Err("agent store unreadable".to_string()), + |_| Ok(()), + ) + .expect("inbound reconcile swallows secondary-store failure"); +} + +/// A `persist_teams` error propagates — the authoritative team write failing is +/// a real reconcile failure, unlike best-effort agent IO. +#[test] +fn inbound_team_propagates_persist_teams_error() { + let mut teams = vec![local_team()]; + let err = commit_inbound_team( + &mut teams, + TEAM_ID.to_string(), + team_content("Team"), + |_| Err("disk full".to_string()), + || Ok(vec![]), + |_| Ok(()), + ) + .expect_err("a failed team persist must propagate"); + assert_eq!(err, "disk full"); +} + // ── Tombstone (kind:5) consume ──────────────────────────────────────────── fn deletion_event(coord: &str) -> nostr::Event { diff --git a/desktop/src-tauri/src/commands/teams.rs b/desktop/src-tauri/src/commands/teams.rs index 4377ddaa43..e17c5bdb24 100644 --- a/desktop/src-tauri/src/commands/teams.rs +++ b/desktop/src-tauri/src/commands/teams.rs @@ -4,8 +4,9 @@ use uuid::Uuid; use crate::{ app_state::AppState, managed_agents::{ - delete_team_with_cascade, ensure_persona_ids_are_active, load_personas, load_teams, - save_teams, try_regenerate_nest, CreateTeamRequest, TeamRecord, UpdateTeamRequest, + delete_team_with_cascade, ensure_persona_ids_are_active, load_managed_agents, + load_personas, load_teams, save_managed_agents, save_teams, try_regenerate_nest, + CreateTeamRequest, TeamRecord, UpdateTeamRequest, }, util::now_iso, }; @@ -25,6 +26,174 @@ fn trim_optional(value: Option) -> Option { }) } +/// Propagate a team's membership *change* to its members' already-running +/// instances, best-effort. Loads the agent store, applies the roster delta via +/// [`apply_team_membership_delta`], and re-saves only when something changed; +/// any load/save error is logged and swallowed. Called after the authoritative +/// `save_teams` succeeds — the team already exists on disk and boot repair is +/// the designed retry for a stale/unset binding, so a secondary-store hiccup +/// must not fail a command whose team write already landed (a UI retry would +/// then mint a duplicate team). +/// +/// `load_agents`/`save_agents` are injected so the command wiring (prior-roster +/// capture, delta direction, and this best-effort policy) is unit-testable +/// without an `AppHandle`; the commands pass the real store IO. +/// +/// Shared with the inbound reconcile path (`commands::personas::inbound`): a +/// 30176 team edit arriving from another device must bind/detach instances the +/// same way a local edit does, so both call this one wrapper. +pub(in crate::commands) fn propagate_membership_best_effort( + team_id: &str, + previous_persona_ids: &[String], + current_persona_ids: &[String], + load_agents: impl FnOnce() -> Result, String>, + save_agents: impl FnOnce(&[crate::managed_agents::ManagedAgentRecord]) -> Result<(), String>, +) { + let result = (|| -> Result<(), String> { + let mut records = load_agents()?; + if apply_team_membership_delta( + &mut records, + team_id, + previous_persona_ids, + current_persona_ids, + ) { + save_agents(&records)?; + } + Ok(()) + })(); + if let Err(e) = result { + eprintln!("buzz-desktop: team-membership-propagate: {e}"); + } +} + +/// In-memory core of [`create_team`]: push the built team, persist teams +/// authoritatively, then propagate its whole roster (no prior members ⇒ the +/// whole roster is the added delta) to live instances best-effort. Decoupled +/// from the `AppHandle` shell via injected persistence so the create wiring is +/// unit-testable. A `persist_teams` error propagates; agent IO is best-effort. +fn commit_team_create( + teams: &mut Vec, + team: TeamRecord, + persist_teams: impl FnOnce(&[TeamRecord]) -> Result<(), String>, + load_agents: impl FnOnce() -> Result, String>, + save_agents: impl FnOnce(&[crate::managed_agents::ManagedAgentRecord]) -> Result<(), String>, +) -> Result { + teams.push(team.clone()); + persist_teams(teams)?; + propagate_membership_best_effort(&team.id, &[], &team.persona_ids, load_agents, save_agents); + Ok(team) +} + +/// In-memory core of [`update_team`]: mutate the matching team, capturing its +/// roster *before* the edit, persist teams authoritatively, then propagate the +/// prior→current delta to live instances best-effort. The prior-roster capture +/// and its use as the delta baseline live here — not at a command call site — +/// so a miswire to the wrong baseline is caught by a test. Injected persistence +/// keeps it `AppHandle`-free; a `persist_teams` error propagates, agent IO is +/// best-effort. Returns the updated team. +#[allow(clippy::too_many_arguments)] +fn commit_team_update( + teams: &mut [TeamRecord], + id: &str, + name: String, + description: Option, + instructions: Option, + persona_ids: Vec, + now: String, + persist_teams: impl FnOnce(&[TeamRecord]) -> Result<(), String>, + load_agents: impl FnOnce() -> Result, String>, + save_agents: impl FnOnce(&[crate::managed_agents::ManagedAgentRecord]) -> Result<(), String>, +) -> Result { + let team = teams + .iter_mut() + .find(|record| record.id == id) + .ok_or_else(|| format!("team {id} not found"))?; + + // Capture the pre-edit roster before mutation: the propagation delta + // (added → backfill, removed → detach) is computed against it. + let previous_persona_ids = team.persona_ids.clone(); + team.name = name; + team.description = description; + team.instructions = instructions; + team.persona_ids = persona_ids; + team.updated_at = now; + + let updated = team.clone(); + persist_teams(teams)?; + propagate_membership_best_effort( + &updated.id, + &previous_persona_ids, + &updated.persona_ids, + load_agents, + save_agents, + ); + Ok(updated) +} + +/// Pure core of the membership propagation: apply the roster delta to `records` +/// in place and report whether anything changed. Decoupled from the store IO so +/// the binding rules are unit-testable. +/// +/// Two directions, keyed on the delta between the pre-edit and post-edit +/// rosters: +/// +/// - **Added** (`current` but not `previous`): backfill `team_id` on the +/// persona's *unbound* instances, so an added persona spawns with the team's +/// instructions (`spawn_snapshot::effective_team_instructions` keys on +/// `record.team_id`). Only an unset field is set — a shared persona keeps an +/// existing binding — and an explicit add is legitimate binding evidence even +/// when the persona belongs to several teams. +/// - **Removed** (`previous` but not `current`): clear `team_id` on instances +/// bound to *this* team, so a "keep agents" removal stops feeding a kept +/// instance the instructions of a team it no longer belongs to. Bindings to +/// other teams are untouched. +/// +/// Delta-scoping is what keeps a metadata-only edit inert: with no roster +/// change both sets are empty and no instance is re-pointed — a shared unbound +/// persona is not silently bound to whichever team was last edited. `create` +/// has no prior roster, so it passes an empty `previous` and the whole roster is +/// "added" (the pre-fix whole-roster backfill). A persona both removed and +/// re-added in one edit appears in neither set (set difference, not +/// operation order), so its binding is left as-is. +fn apply_team_membership_delta( + records: &mut [crate::managed_agents::ManagedAgentRecord], + team_id: &str, + previous_persona_ids: &[String], + current_persona_ids: &[String], +) -> bool { + let added: Vec<&str> = current_persona_ids + .iter() + .filter(|id| !previous_persona_ids.iter().any(|p| p == *id)) + .map(String::as_str) + .collect(); + let removed: Vec<&str> = previous_persona_ids + .iter() + .filter(|id| !current_persona_ids.iter().any(|p| p == *id)) + .map(String::as_str) + .collect(); + if added.is_empty() && removed.is_empty() { + return false; + } + + let mut changed = false; + for record in records.iter_mut() { + if record.pubkey.is_empty() { + continue; + } + let Some(persona_id) = record.persona_id.as_deref() else { + continue; + }; + if record.team_id.is_none() && added.contains(&persona_id) { + record.team_id = Some(team_id.to_string()); + changed = true; + } else if record.team_id.as_deref() == Some(team_id) && removed.contains(&persona_id) { + record.team_id = None; + changed = true; + } + } + changed +} + /// Retain a freshly authored team event in the local store, flagged for relay /// sync. Called inside a command's `managed_agents_store_lock`-held body after /// `save_teams`; the background flush loop publishes it out-of-band. @@ -171,8 +340,13 @@ pub async fn create_team(input: CreateTeamRequest, app: AppHandle) -> Result Result Result) -> ManagedAgentRecord { + let mut record = serde_json::from_value::(serde_json::json!({ + "pubkey": seed.to_string().repeat(64), + "name": persona_id, + "persona_id": persona_id, + "relay_url": "ws://localhost:3000", + "acp_command": "buzz-acp", + "agent_command": "goose", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "system_prompt": "prompt", + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z", + })) + .unwrap(); + record.team_id = team_id.map(str::to_string); + record + } + + fn ids(list: &[&str]) -> Vec { + list.iter().map(|s| s.to_string()).collect() + } + + /// A metadata-only edit (no roster change) never re-points an instance — + /// including an unbound instance of a persona this team shares with another. + #[test] + fn metadata_only_edit_leaves_bindings_untouched() { + let mut records = vec![instance('a', "duncan", None)]; + let roster = ids(&["duncan"]); + assert!(!apply_team_membership_delta( + &mut records, + "team-a", + &roster, + &roster + )); + assert_eq!(records[0].team_id, None); + } + + /// Only the *added* persona's unbound instance is bound; an untouched member + /// already present in the previous roster is not re-pointed. + #[test] + fn added_persona_backfills_only_its_unbound_instance() { + let mut records = vec![ + instance('a', "duncan", None), + instance('b', "paul", Some("team-b")), + ]; + assert!(apply_team_membership_delta( + &mut records, + "team-a", + &ids(&["paul"]), + &ids(&["paul", "duncan"]), + )); + assert_eq!(records[0].team_id.as_deref(), Some("team-a")); + // Paul was already on the team and bound elsewhere — untouched. + assert_eq!(records[1].team_id.as_deref(), Some("team-b")); + } + + /// An added persona binds even when shared across teams: an explicit add is + /// legitimate evidence (unlike the boot-repair's order-blind case). + #[test] + fn added_shared_persona_binds_to_the_edited_team() { + let mut records = vec![instance('a', "duncan", None)]; + assert!(apply_team_membership_delta( + &mut records, + "team-a", + &[], + &ids(&["duncan"]), + )); + assert_eq!(records[0].team_id.as_deref(), Some("team-a")); + } + + /// Removing a persona ("keep agents") clears its binding to *this* team so a + /// kept instance stops drawing the team's instructions at spawn. + #[test] + fn removed_persona_detaches_instance_bound_to_this_team() { + let mut records = vec![instance('a', "duncan", Some("team-a"))]; + assert!(apply_team_membership_delta( + &mut records, + "team-a", + &ids(&["duncan"]), + &[], + )); + assert_eq!(records[0].team_id, None); + } + + /// Removal only clears a binding pointing at *this* team — an instance of + /// the same persona bound to a different team is left alone. + #[test] + fn removed_persona_leaves_other_team_binding_untouched() { + let mut records = vec![instance('a', "duncan", Some("team-b"))]; + assert!(!apply_team_membership_delta( + &mut records, + "team-a", + &ids(&["duncan"]), + &[], + )); + assert_eq!(records[0].team_id.as_deref(), Some("team-b")); + } + + /// A minimal owner-authored team record for wiring tests. + fn team(id: &str, persona_ids: &[&str]) -> TeamRecord { + TeamRecord { + id: id.to_string(), + name: id.to_string(), + description: None, + instructions: None, + persona_ids: ids(persona_ids), + is_builtin: false, + source_dir: None, + is_symlink: false, + symlink_target: None, + version: None, + created_at: "2026-01-01T00:00:00Z".to_string(), + updated_at: "2026-01-01T00:00:00Z".to_string(), + } + } + + /// Records the injected store IO a commit performs, so a test can assert + /// the wiring saved (or deliberately did not) the agent store. + #[derive(Default)] + struct StoreSpy { + saved: Option>, + } + + /// Metadata-only `update_team` must pass the TRUE prior roster into the + /// delta, so an unchanged roster is an empty delta and no agent write fires. + /// The `&previous_persona_ids` → `&[]` miswire would drop the prior roster, + /// making the whole roster look "added" and re-pointing the unbound instance. + #[test] + fn commit_team_update_uses_true_prior_roster() { + let mut teams = vec![team("team-a", &["duncan"])]; + let existing = vec![instance('a', "duncan", None)]; + let spy = RefCell::new(StoreSpy::default()); + + let updated = commit_team_update( + &mut teams, + "team-a", + "Team A".to_string(), + None, + Some("new instructions".to_string()), + ids(&["duncan"]), + "2026-02-02T00:00:00Z".to_string(), + |_| Ok(()), + || Ok(existing.clone()), + |records| { + spy.borrow_mut().saved = Some(records.to_vec()); + Ok(()) + }, + ) + .expect("metadata-only update succeeds"); + + assert_eq!(updated.instructions.as_deref(), Some("new instructions")); + // Empty delta ⇒ nothing changed ⇒ no save (the true-prior-roster gate). + assert!( + spy.borrow().saved.is_none(), + "metadata-only edit must not write the agent store" + ); + } + + /// Removing a persona from the roster must reach the detach branch through + /// the command wiring: the instance bound to this team is cleared and saved. + #[test] + fn commit_team_update_removal_detaches_through_wiring() { + let mut teams = vec![team("team-a", &["duncan"])]; + let existing = vec![instance('a', "duncan", Some("team-a"))]; + let spy = RefCell::new(StoreSpy::default()); + + commit_team_update( + &mut teams, + "team-a", + "team-a".to_string(), + None, + None, + ids(&[]), + "2026-02-02T00:00:00Z".to_string(), + |_| Ok(()), + || Ok(existing.clone()), + |records| { + spy.borrow_mut().saved = Some(records.to_vec()); + Ok(()) + }, + ) + .expect("removal update succeeds"); + + let saved = spy.borrow().saved.clone().expect("detach must save"); + assert_eq!(saved[0].team_id, None, "removed persona detaches from team"); + } + + /// `create_team` has no prior roster, so its whole roster is the added delta: + /// the unbound instance of a listed persona is bound through the wiring. + #[test] + fn commit_team_create_treats_full_roster_as_added() { + let mut teams: Vec = Vec::new(); + let existing = vec![instance('a', "duncan", None)]; + let spy = RefCell::new(StoreSpy::default()); + + let created = commit_team_create( + &mut teams, + team("team-a", &["duncan"]), + |_| Ok(()), + || Ok(existing.clone()), + |records| { + spy.borrow_mut().saved = Some(records.to_vec()); + Ok(()) + }, + ) + .expect("create succeeds"); + + assert_eq!(created.id, "team-a"); + let saved = spy.borrow().saved.clone().expect("backfill must save"); + assert_eq!( + saved[0].team_id.as_deref(), + Some("team-a"), + "whole roster is the added delta on create" + ); + } + + /// A failing secondary agent write after successful `save_teams` is + /// swallowed: both commits still return the persisted team. Otherwise a UI + /// retry of a create whose team already landed would mint a duplicate. + #[test] + fn commit_returns_ok_when_agent_save_fails() { + let mut teams: Vec = Vec::new(); + let created = commit_team_create( + &mut teams, + team("team-a", &["duncan"]), + |_| Ok(()), + || Ok(vec![instance('a', "duncan", None)]), + |_| Err("disk full".to_string()), + ) + .expect("create swallows secondary-store failure"); + assert_eq!(created.id, "team-a"); + + let mut teams = vec![team("team-a", &["duncan"])]; + let updated = commit_team_update( + &mut teams, + "team-a", + "team-a".to_string(), + None, + None, + ids(&[]), + "2026-02-02T00:00:00Z".to_string(), + |_| Ok(()), + || Err("agent store unreadable".to_string()), + |_| Ok(()), + ) + .expect("update swallows secondary-store failure"); + assert_eq!(updated.persona_ids, Vec::::new()); + } +} + #[tauri::command] pub async fn delete_team(id: String, app: AppHandle) -> Result<(), String> { use tauri::Manager; diff --git a/desktop/src-tauri/src/commands/workflows.rs b/desktop/src-tauri/src/commands/workflows.rs index 2e77645f85..c4e5d38c8b 100644 --- a/desktop/src-tauri/src/commands/workflows.rs +++ b/desktop/src-tauri/src/commands/workflows.rs @@ -1,3 +1,5 @@ +use std::collections::HashSet; + use serde::Serialize; use serde_json::Value; use tauri::State; @@ -103,34 +105,73 @@ pub async fn get_channel_workflows( Ok(events.iter().map(workflow_from_event).collect()) } -/// Fetch workflows across many channels in a single relay round-trip. +// Keep this aligned with the relay's aggregate explicit-`#h` request bound. +// Each filter below carries exactly one explicit value so old relays retain the +// known-compatible shape while current relays cannot reject large memberships. +const WORKFLOW_QUERY_CHANNEL_BATCH_SIZE: usize = 128; + +/// Fetch workflows across many channels using bounded relay round-trips. /// /// The Workflows overview screen previously issued one `get_channel_workflows` /// query per member channel (`Promise.all` fanout in `WorkflowsView`), i.e. N -/// relay POSTs. A nostr `#h` filter matches ANY of its listed values, so one -/// query with all channel ids returns the same set. Each `WorkflowWire` carries -/// its own `channel_id` (from the event's `h` tag), so the frontend can still -/// group results by channel. Neither this nor the per-channel command sets a -/// `limit`, so batching does not change result completeness. +/// relay POSTs. This sends one single-channel filter per channel, in requests of +/// at most 128 filters. Using one multi-value `#h` filter is equivalent under +/// NIP-01, but older relays incorrectly narrowed that shape to its first +/// channel. Each `WorkflowWire` carries its own `channel_id` (from the event's +/// `h` tag), so the frontend can still group results by channel. Neither this +/// nor the per-channel command sets a `limit`, so batching does not change +/// result completeness. Results are deduplicated by signed event ID in case a +/// caller supplies duplicate channel IDs. #[tauri::command] pub async fn get_channels_workflows( channel_ids: Vec, state: State<'_, AppState>, ) -> Result, String> { - if channel_ids.is_empty() { - return Ok(Vec::new()); + let filter_batches = channel_workflow_filter_batches(channel_ids)?; + let mut seen_event_ids = HashSet::new(); + let mut workflows = Vec::new(); + + for filters in filter_batches { + let events = query_relay(&state, &filters).await?; + append_unique_workflows(&mut workflows, &mut seen_event_ids, &events); } - let events = query_relay( - &state, - &[serde_json::json!({ - "kinds": [30620], - "#h": channel_ids, - })], - ) - .await?; + Ok(workflows) +} - Ok(events.iter().map(workflow_from_event).collect()) +fn append_unique_workflows( + workflows: &mut Vec, + seen_event_ids: &mut HashSet, + events: &[nostr::Event], +) { + workflows.extend( + events + .iter() + .filter(|event| seen_event_ids.insert(event.id)) + .map(workflow_from_event), + ); +} + +fn channel_workflow_filter_batches(channel_ids: Vec) -> Result>, String> { + let filters = channel_workflow_filters(channel_ids)?; + Ok(filters + .chunks(WORKFLOW_QUERY_CHANNEL_BATCH_SIZE) + .map(<[Value]>::to_vec) + .collect()) +} + +fn channel_workflow_filters(channel_ids: Vec) -> Result, String> { + channel_ids + .into_iter() + .map(|channel_id| { + let channel_id = uuid::Uuid::parse_str(channel_id.trim()) + .map_err(|_| "invalid channel id".to_string())?; + Ok(serde_json::json!({ + "kinds": [30620], + "#h": [channel_id.to_string()], + })) + }) + .collect() } #[tauri::command] diff --git a/desktop/src-tauri/src/commands/workflows_tests.rs b/desktop/src-tauri/src/commands/workflows_tests.rs index 4adbd52177..8522d233c3 100644 --- a/desktop/src-tauri/src/commands/workflows_tests.rs +++ b/desktop/src-tauri/src/commands/workflows_tests.rs @@ -192,6 +192,81 @@ fn workflow_wire_serializes_with_snake_case_keys() { } } +#[test] +fn multi_channel_workflow_query_uses_one_filter_per_channel() { + let other_channel = "33333333-3333-3333-3333-333333333333"; + let filters = channel_workflow_filters(vec![CHAN.to_string(), other_channel.to_string()]) + .expect("valid channels"); + + assert_eq!(filters.len(), 2); + assert_eq!( + filters[0], + serde_json::json!({ + "kinds": [30620], + "#h": [CHAN], + }) + ); + assert_eq!( + filters[1], + serde_json::json!({ + "kinds": [30620], + "#h": [other_channel], + }) + ); +} + +#[test] +fn workflow_queries_batch_above_relay_explicit_channel_limit() { + let channel_ids = (0..WORKFLOW_QUERY_CHANNEL_BATCH_SIZE + 1) + .map(|index| uuid::Uuid::from_u128(index as u128 + 1).to_string()) + .collect(); + let batches = channel_workflow_filter_batches(channel_ids).expect("valid channels"); + + assert_eq!(batches.len(), 2); + assert_eq!(batches[0].len(), WORKFLOW_QUERY_CHANNEL_BATCH_SIZE); + assert_eq!(batches[1].len(), 1); + assert!(batches.iter().flatten().all(|filter| filter["#h"] + .as_array() + .is_some_and(|values| values.len() == 1))); +} + +#[test] +fn workflow_query_results_are_deduplicated_by_event_id() { + let first = wf_event(WF, CHAN, YAML); + let second_workflow = "33333333-3333-3333-3333-333333333333"; + let second = wf_event(second_workflow, CHAN, YAML); + let mut workflows = Vec::new(); + let mut seen_event_ids = HashSet::new(); + + append_unique_workflows( + &mut workflows, + &mut seen_event_ids, + &[first.clone(), second.clone()], + ); + append_unique_workflows(&mut workflows, &mut seen_event_ids, &[first, second]); + + assert_eq!(workflows.len(), 2); + assert_eq!(workflows[0].id, WF); + assert_eq!(workflows[1].id, second_workflow); +} + +#[test] +fn channel_workflow_filters_reject_malformed_or_blank_channel_ids() { + for channel_id in ["not-a-uuid", "", " "] { + let error = channel_workflow_filters(vec![channel_id.to_string()]) + .expect_err("malformed channel id must fail before querying the relay"); + assert_eq!(error, "invalid channel id"); + } +} + +#[test] +fn channel_workflow_filters_accepts_empty_input() { + assert_eq!( + channel_workflow_filters(Vec::new()).expect("empty input is valid"), + Vec::::new() + ); +} + #[test] fn trigger_response_uses_persisted_run_id_contract() { let wire = trigger_wire_from_message( diff --git a/desktop/src-tauri/src/commands/workspace.rs b/desktop/src-tauri/src/commands/workspace.rs index a32fd2c0e0..89e6e29cec 100644 --- a/desktop/src-tauri/src/commands/workspace.rs +++ b/desktop/src-tauri/src/commands/workspace.rs @@ -233,16 +233,39 @@ pub async fn apply_workspace( // Adopt whatever the pre-scoping release left queued in the global // retention database BEFORE the scoped reconcile and flush run, so // stranded tombstones and archive requests publish on this boot - // instead of being abandoned by the storage cutover. + // instead of being abandoned by the storage cutover. Best-effort: + // it is not a prerequisite for the superseding head — the team leg + // below builds the repaired roster's head fresh from disk with a + // monotonic `created_at` regardless of what the legacy copy left. migrate_legacy_retention_into(&restore_app, &scope); - crate::event_sync::spawn_event_sync( + // Await the reconcile to completion — do NOT spawn it — and + // propagate its failure. The boot migration may have repaired team + // membership on disk; the frontend starts inbound history replay + // the moment `useCommunityInit` observes the applied workspace, and + // an old relay team head could otherwise win that race and overwrite + // the repaired `persona_ids`. The team leg is fatal (see + // `run_event_sync`): only its success durably retains the corrected + // head with a superseding `monotonic_created_at`, so + // `retain_inbound_event`'s equal/older guard rejects the stale head. + // On failure we return `Err` — the command reports failure, + // `useCommunityInit` never exposes the community, and inbound replay + // never starts against an un-superseded disk state. + crate::event_sync::run_event_sync_blocking( restore_app.clone(), scope.owner_keys, scope.db_path, ) + .await?; } Err(error) => { - eprintln!("buzz-desktop: scoped event-sync unavailable after workspace apply: {error}"); + // Scope resolution is a prerequisite for establishing the + // superseding head, so its failure is fatal for the same reason: + // without a scope we cannot retain the repaired roster ahead of an + // inbound replay. Fail the command rather than silently opening the + // inbound lane. + return Err(format!( + "scoped event-sync unavailable after workspace apply: {error}" + )); } } diff --git a/desktop/src-tauri/src/event_sync.rs b/desktop/src-tauri/src/event_sync.rs index ee8e0d8b10..93990f2b24 100644 --- a/desktop/src-tauri/src/event_sync.rs +++ b/desktop/src-tauri/src/event_sync.rs @@ -13,32 +13,44 @@ use std::path::Path; /// `sync_team_personas` wrote in [`crate::migration::run_boot_migrations`] /// (see its `# Ordering` guard). Event signing needs the resolved owner keys, /// so this runs after identity resolution, not in the boot migrations. -pub fn run_event_sync(app: &tauri::AppHandle, owner_keys: &nostr::Keys, db_path: &Path) { +pub fn run_event_sync( + app: &tauri::AppHandle, + owner_keys: &nostr::Keys, + db_path: &Path, +) -> Result<(), String> { + // Persona and agent legs stay best-effort: they log and swallow, and their + // failure does not undo the boot team-membership repair. The team leg is + // fatal — it establishes the superseding local head (a monotonic + // `created_at`) that lets `retain_inbound_event`'s equal/older guard reject + // a stale relay roster. If it fails, the caller must not let the frontend + // expose the community and start inbound replay against an un-superseded + // disk state. migrate_personas_to_events(app, owner_keys, db_path); - migrate_teams_to_events(app, owner_keys, db_path); + migrate_teams_to_events(app, owner_keys, db_path)?; crate::managed_agents::reconcile::reconcile_agents_to_events(app, owner_keys, db_path); + Ok(()) } -/// Spawn the best-effort event reconcile off the synchronous Tauri setup path. +/// Run the scoped event reconcile to completion on the blocking pool. /// -/// The owner keys are cloned before spawning so the task never touches the -/// `AppState::keys` mutex. The reconcile itself is still synchronous JSON, -/// SQLite, and signing work, so it runs on the blocking pool rather than an -/// async worker. -pub fn spawn_event_sync( +/// Callers that must not let downstream work observe a not-yet-retained disk +/// state (e.g. `apply_workspace` before the frontend can start inbound history +/// replay) await this so the repaired local heads are durably retained — with a +/// superseding `monotonic_created_at` — before an old relay head can race in. +/// The owner keys are moved in so the task never touches the `AppState::keys` +/// mutex; the reconcile itself is synchronous JSON/SQLite/signing work, so it +/// runs on the blocking pool rather than an async worker. +/// +/// Returns `Err` if the task fails to join or the fatal team leg errors, so the +/// caller can withhold community exposure until the superseding head is durable. +pub async fn run_event_sync_blocking( app: tauri::AppHandle, owner_keys: nostr::Keys, db_path: std::path::PathBuf, -) { - tauri::async_runtime::spawn(async move { - if let Err(e) = tauri::async_runtime::spawn_blocking(move || { - run_event_sync(&app, &owner_keys, &db_path); - }) +) -> Result<(), String> { + tauri::async_runtime::spawn_blocking(move || run_event_sync(&app, &owner_keys, &db_path)) .await - { - eprintln!("buzz-desktop: event-sync: spawn_blocking failed: {e}"); - } - }); + .map_err(|e| format!("event-sync: spawn_blocking failed: {e}"))? } /// Reconcile `personas.json` into the persona-event retention store. @@ -219,21 +231,23 @@ fn migrate_personas_in_dir_at( /// /// Must run after the persisted identity is resolved (it signs each event with /// the owner's keys). -pub fn migrate_teams_to_events(app: &tauri::AppHandle, keys: &nostr::Keys, db_path: &Path) { +pub fn migrate_teams_to_events( + app: &tauri::AppHandle, + keys: &nostr::Keys, + db_path: &Path, +) -> Result<(), String> { use crate::managed_agents::managed_agents_base_dir; - let Ok(base_dir) = managed_agents_base_dir(app) else { - return; - }; + let base_dir = managed_agents_base_dir(app) + .map_err(|e| format!("team-event-migration: base dir unavailable: {e}"))?; match migrate_teams_in_dir_at(&base_dir, keys, db_path) { - Ok(0) => {} + Ok(0) => Ok(()), Ok(migrated) => { eprintln!("buzz-desktop: team-event-migration: {migrated} teams migrated to retention"); + Ok(()) } - Err(e) => { - eprintln!("buzz-desktop: team-event-migration: {e}"); - } + Err(e) => Err(format!("team-event-migration: {e}")), } } diff --git a/desktop/src-tauri/src/event_sync_team_events_tests.rs b/desktop/src-tauri/src/event_sync_team_events_tests.rs index 0f7ab52bf5..b1a56b0661 100644 --- a/desktop/src-tauri/src/event_sync_team_events_tests.rs +++ b/desktop/src-tauri/src/event_sync_team_events_tests.rs @@ -133,3 +133,132 @@ fn migrate_teams_no_file_is_noop() { let keys = nostr::Keys::generate(); assert_eq!(migrate_teams_in_dir(base.path(), &keys).unwrap(), 0); } + +/// Error-contract for the fatal team leg. `run_event_sync` propagates a team +/// leg failure via `?`, and `apply_workspace` returns that `Err` so the +/// frontend never exposes the community against an un-superseded disk state. +/// This proves the leg genuinely surfaces failure (rather than logging and +/// swallowing) on an unreadable store — the precondition that made the +/// propagation load-bearing. +#[test] +fn migrate_teams_surfaces_error_on_unparseable_store() { + let base = tempfile::tempdir().unwrap(); + std::fs::write(base.path().join("teams.json"), "{ not valid json").unwrap(); + let keys = nostr::Keys::generate(); + assert!(migrate_teams_in_dir(base.path(), &keys).is_err()); +} + +/// Build a signed inbound team head at an explicit `created_at`, mirroring a +/// relay replay of a stale, pre-namespacing roster. +fn stale_inbound_head( + keys: &nostr::Keys, + id: &str, + bare_persona_ids: &[&str], + created_at: i64, +) -> crate::managed_agents::retention::RetainedEvent { + use crate::managed_agents::{team_events::build_team_event, TeamRecord}; + use buzz_core_pkg::kind::KIND_TEAM; + use nostr::JsonUtil; + + let record = TeamRecord { + id: id.to_string(), + name: "Sietch Tabr".to_string(), + description: None, + instructions: None, + persona_ids: bare_persona_ids.iter().map(|s| s.to_string()).collect(), + is_builtin: false, + source_dir: None, + is_symlink: false, + symlink_target: None, + version: None, + created_at: "2025-01-01T00:00:00Z".to_string(), + updated_at: "2025-01-01T00:00:00Z".to_string(), + }; + let event = build_team_event(&record) + .unwrap() + .custom_created_at(nostr::Timestamp::from(created_at as u64)) + .sign_with_keys(keys) + .unwrap(); + crate::managed_agents::retention::RetainedEvent { + kind: KIND_TEAM, + pubkey: keys.public_key().to_hex(), + d_tag: id.to_string(), + content: event.content.to_string(), + created_at: event.created_at.as_secs() as i64, + raw_event: event.as_json(), + pending_sync: false, + } +} + +/// Finding-1 retention-precedence guarantee. This proves the *mechanic* the +/// awaited-reconcile ordering relies on — it does not itself exercise +/// `apply_workspace` (an `AppHandle`-level path). Given the boot reconcile has +/// retained the repaired namespaced roster with a monotonic `created_at` +/// (reconcile-first), a stale relay head replayed afterward is older, so +/// `retain_inbound_event` skips it and the repaired roster stays. The +/// inbound-first lane is the counterfactual the ordering closes: with no +/// repaired head retained yet, the very same stale head is applied and restores +/// bare membership. Retention order is the only difference between the lanes; +/// `apply_workspace` awaiting the reconcile (see `commands/workspace.rs`) is +/// what forces the reconcile-first order in production. +#[test] +fn reconcile_first_makes_stale_inbound_team_head_lose() { + use crate::managed_agents::retention::{ + get_retained_event, open_retention_db, retain_inbound_event, InboundOutcome, + }; + use buzz_core_pkg::kind::KIND_TEAM; + + let keys = nostr::Keys::generate(); + let pubkey = keys.public_key().to_hex(); + let repaired = serde_json::json!([{ + "id": "sietch-tabr", + "name": "Sietch Tabr", + "persona_ids": ["sietch-tabr:thufir", "sietch-tabr:paul", "sietch-tabr:duncan"], + "is_builtin": false, + "created_at": "2025-01-01T00:00:00Z", + "updated_at": "2025-01-01T00:00:00Z" + }]); + let bare = ["thufir", "paul", "duncan"]; + + // Reconcile-first lane (the fix): the awaited boot reconcile retains the + // repaired namespaced roster with a monotonic `created_at`; a stale relay + // head replayed afterward is older, so `retain_inbound_event` skips it and + // the retained roster stays repaired. + let ordered = tempfile::tempdir().unwrap(); + let ordered_db = ordered.path().join("retention.db"); + write_base_teams(ordered.path(), &repaired); + assert_eq!( + migrate_teams_in_dir_at(ordered.path(), &keys, &ordered_db).unwrap(), + 1 + ); + let conn = open_retention_db(&ordered_db).unwrap(); + let repaired_head = get_retained_event(&conn, KIND_TEAM, &pubkey, "sietch-tabr") + .unwrap() + .unwrap(); + let stale = stale_inbound_head(&keys, "sietch-tabr", &bare, repaired_head.created_at - 1); + assert_eq!( + retain_inbound_event(&conn, &stale).unwrap(), + InboundOutcome::Skipped + ); + let head = get_retained_event(&conn, KIND_TEAM, &pubkey, "sietch-tabr") + .unwrap() + .unwrap(); + assert!(head.content.contains("sietch-tabr:thufir")); + assert!(!head.content.contains("\"thufir\"")); + + // Inbound-first lane (the race the fix closes): with no repaired head + // retained yet, the very same stale relay head is applied, restoring the + // bare pre-namespacing roster. Ordering is the only difference. + let raced = tempfile::tempdir().unwrap(); + let raced_db = raced.path().join("retention.db"); + let raced_conn = open_retention_db(&raced_db).unwrap(); + let stale = stale_inbound_head(&keys, "sietch-tabr", &bare, repaired_head.created_at - 1); + assert_eq!( + retain_inbound_event(&raced_conn, &stale).unwrap(), + InboundOutcome::Applied + ); + let head = get_retained_event(&raced_conn, KIND_TEAM, &pubkey, "sietch-tabr") + .unwrap() + .unwrap(); + assert!(head.content.contains("\"thufir\"")); +} diff --git a/desktop/src-tauri/src/managed_agents/mod.rs b/desktop/src-tauri/src/managed_agents/mod.rs index c6ccd3709c..16234aa3d6 100644 --- a/desktop/src-tauri/src/managed_agents/mod.rs +++ b/desktop/src-tauri/src/managed_agents/mod.rs @@ -39,6 +39,7 @@ pub(crate) mod spawn_snapshot; pub(crate) mod storage; pub(crate) mod team_events; mod team_repair; +pub(crate) use team_repair::team_persona_key; mod teams; mod types; diff --git a/desktop/src-tauri/src/migration.rs b/desktop/src-tauri/src/migration.rs index 0baba6456b..1e22d7aaec 100644 --- a/desktop/src-tauri/src/migration.rs +++ b/desktop/src-tauri/src/migration.rs @@ -149,8 +149,7 @@ fn run_boot_migrations_inner(app: &tauri::AppHandle, reset_completed: bool) { // ensures the dev nest boots with the correct workspace on its first launch, // matching what the prod nest had configured. Skip-if-dest-exists so it is // idempotent and never clobbers a value the dev nest already set explicitly. - // Uses the composed helper so the gate + migration run through the same - // code path that the behavioral test exercises. + // Uses the composed helper so gate + migration share the tested code path. if let (Some(home), Some(dev_nest)) = (dirs::home_dir(), crate::managed_agents::nest_dir()) { maybe_migrate_dev_repos_dir(is_dev, reset_completed, &home, &dev_nest); } @@ -181,11 +180,12 @@ fn run_boot_migrations_inner(app: &tauri::AppHandle, reset_completed: bool) { strip_baked_team_instructions(app); refresh_builtin_agent_avatars(app); // B5: manufacture definitions for standalone agents AFTER the fold (so - // pre-existing definition slugs are present for collision checks) and - // before event sync republishes — the backfilled link is what flips the - // 30177 projection to its slim shape. + // pre-existing definition slugs exist for collision checks) and before event + // sync republishes — the backfilled link flips the 30177 projection. backfill_standalone_agents(app); - detach_directory_backed_teams(app); + // Repair dropped team↔member links, then detach directory-backed teams, + // gated on a clean repair so a failure preserves `source_dir` for a retry. + team_membership::repair_then_detach_teams(app); reconcile_provider_mcp_commands(app); reconcile_databricks_v1_to_v2(app); materialize_agent_runtimes(app); @@ -1373,8 +1373,8 @@ use fold::load_persona_runtimes; mod backfill; pub use backfill::backfill_standalone_agents; mod detach; -pub use detach::detach_directory_backed_teams; mod pollen; +mod team_membership; pub(crate) use pollen::*; mod team_suffix; pub use team_suffix::strip_baked_team_instructions; diff --git a/desktop/src-tauri/src/migration/detach.rs b/desktop/src-tauri/src/migration/detach.rs index 9f746e479f..7912365331 100644 --- a/desktop/src-tauri/src/migration/detach.rs +++ b/desktop/src-tauri/src/migration/detach.rs @@ -9,10 +9,12 @@ use crate::managed_agents::{ManagedAgentRecord, TeamRecord}; /// Lift pack instructions into `TeamRecord.instructions` and detach /// directory-backed teams from their source directories. /// -/// Runs on app launch if any `TeamRecord` still has `source_dir` set. -/// Both output files are written atomically (temp-file + rename), so a crash -/// mid-write leaves the previous version intact and the migration can safely -/// retry on next boot. +/// Core logic, decoupled from the Tauri `AppHandle` for testing. +/// +/// Runs on app launch (gated on a clean team-membership repair) if any +/// `TeamRecord` still has `source_dir` set. Both output files are written +/// atomically (temp-file + rename), so a crash mid-write leaves the previous +/// version intact and the migration can safely retry on next boot. /// /// Steps (written last so the idempotency gate stays open until both files /// are committed): @@ -24,18 +26,6 @@ use crate::managed_agents::{ManagedAgentRecord, TeamRecord}; /// `instructions` if the field is not already set. /// 4. Clear `source_dir`, `is_symlink`, `symlink_target`, `version` on each /// directory-backed `TeamRecord`. -pub fn detach_directory_backed_teams(app: &tauri::AppHandle) { - let Ok(base_dir) = crate::managed_agents::managed_agents_base_dir(app) else { - return; - }; - match detach_directory_backed_teams_in_dir(&base_dir) { - Ok(0) => {} - Ok(n) => eprintln!("buzz-desktop: detach-dir-teams: detached {n} directory-backed team(s)"), - Err(e) => eprintln!("buzz-desktop: detach-dir-teams: {e}"), - } -} - -/// Core logic, decoupled from the Tauri `AppHandle` for testing. /// /// `base_dir` is the managed-agents base directory (`/agents/`). /// Returns the number of teams detached (0 = nothing to do). diff --git a/desktop/src-tauri/src/migration/team_membership.rs b/desktop/src-tauri/src/migration/team_membership.rs new file mode 100644 index 0000000000..632714f3f9 --- /dev/null +++ b/desktop/src-tauri/src/migration/team_membership.rs @@ -0,0 +1,353 @@ +//! Repair team↔member links that a membership edit failed to propagate. +//! +//! Two independent defects, both rooted in a team-membership change not +//! reaching the records that depend on it, are healed in one pass over +//! `teams.json` + `managed-agents.json`: +//! +//! 1. **Stale `persona_ids`.** Team records written before persona ids were +//! namespaced hold bare slugs (`thufir`) instead of the namespaced id +//! (`sietch-tabr:thufir`). Nothing rewrites them, and the interactive save +//! path (`ensure_persona_ids_are_active`) *drops* an id it cannot resolve — +//! silently shrinking the team. This migration rewrites a stale id to the +//! persona it names whenever that persona is unambiguous, and — unlike the +//! save path — never drops one it cannot resolve. +//! +//! 2. **Orphaned or stale instance `team_id`.** Team instructions are injected +//! at spawn by matching `record.team_id` +//! (`spawn_snapshot::effective_team_instructions`), so an instance's binding +//! must track its persona's membership. Two ways it drifts: adding a persona +//! to a team does not backfill `team_id` on that persona's already-running +//! instances (a member in the roster but not in behavior), and removing a +//! persona while keeping its agents leaves the binding pointing at a team +//! that no longer lists it (still drawing that team's instructions). This +//! backfills an unset binding and heals a stale one — always on the same +//! single-team evidence rule, never guessing across teams. +//! +//! The stale-id rewrite is strictly additive (rewrite-or-leave); the binding +//! repair converges to a fixed point (bound-to-a-listing-team or unbound), so a +//! second boot is a clean no-op either way. Runs BEFORE +//! `detach_directory_backed_teams` so a not-yet-detached directory-backed team +//! can still be scoped by its `source_dir`, and before any UI save can drop an +//! unresolvable id. + +use std::collections::HashMap; +use std::path::Path; + +use crate::managed_agents::{team_persona_key, ManagedAgentRecord, TeamRecord}; + +/// Repair stale team `persona_ids`/instance `team_id`, then detach +/// directory-backed teams — but only when the repair succeeded. +/// +/// `repair` clears no `source_dir`; the downstream detach does. A stale bare +/// slug shared across source teams is disambiguated by `source_dir`, so if +/// repair fails (its backup or write errored) and detach still ran, the next +/// boot would see only ambiguous candidates and the original membership-loss +/// path recurs. Gating detach on a clean repair preserves `source_dir` as retry +/// evidence for that boot; the next boot retries repair and, once clean, +/// detaches. +pub(super) fn repair_then_detach_teams(app: &tauri::AppHandle) { + let Ok(base_dir) = crate::managed_agents::managed_agents_base_dir(app) else { + return; + }; + orchestrate_repair_then_detach( + || repair_team_membership_in_dir(&base_dir), + || super::detach::detach_directory_backed_teams_in_dir(&base_dir), + ); +} + +/// Gate `detach` on a successful `repair`: run detach only when repair returned +/// `Ok`. Injected ops keep the gate `AppHandle`-free so a failing repair's +/// skip-detach behavior is unit-testable without a filesystem fault. +fn orchestrate_repair_then_detach( + repair: impl FnOnce() -> Result, + detach: impl FnOnce() -> Result, +) { + match repair() { + Ok(repaired) => { + if repaired > 0 { + eprintln!("buzz-desktop: team-membership-repair: repaired {repaired} record(s)"); + } + match detach() { + Ok(0) => {} + Ok(n) => { + eprintln!( + "buzz-desktop: detach-dir-teams: detached {n} directory-backed team(s)" + ) + } + Err(e) => eprintln!("buzz-desktop: detach-dir-teams: {e}"), + } + } + Err(e) => eprintln!( + "buzz-desktop: team-membership-repair: {e} — skipping directory-backed detach this \ + boot to preserve source_dir for a clean-repair retry" + ), + } +} + +/// Core logic, decoupled from the Tauri `AppHandle` for testing. +/// +/// `base_dir` is the managed-agents base directory (`/agents/`). +/// Returns the number of records changed across both files (0 = nothing to do, +/// nothing written, so a re-run is a clean no-op). +pub(super) fn repair_team_membership_in_dir(base_dir: &Path) -> Result { + let teams_path = base_dir.join("teams.json"); + let agents_path = base_dir.join("managed-agents.json"); + + // Definitions and teams both live in these two files; without either there + // is nothing to link. + if !teams_path.exists() || !agents_path.exists() { + return Ok(0); + } + + let teams_content = std::fs::read_to_string(&teams_path) + .map_err(|e| format!("failed to read teams.json: {e}"))?; + let mut teams: Vec = serde_json::from_str(&teams_content) + .map_err(|e| format!("failed to parse teams.json: {e}"))?; + + let agents_content = std::fs::read_to_string(&agents_path) + .map_err(|e| format!("failed to read managed-agents.json: {e}"))?; + let mut agents: Vec = serde_json::from_str(&agents_content) + .map_err(|e| format!("failed to parse managed-agents.json: {e}"))?; + + let rewrites = rewrite_stale_persona_ids(&mut teams, &agents); + let backfills = backfill_instance_team_ids(&teams, &mut agents); + + if rewrites == 0 && backfills == 0 { + return Ok(0); + } + + // Pre-migration backups, both taken BEFORE either live store write: the + // stated contract is a full recovery pair even if a crash lands between the + // two writes, so neither store may be rewritten until both pristine backups + // exist. A stale bare slug shared across source teams is disambiguated by + // `source_dir`, which the downstream detach clears — so the pristine + // pre-repair `teams.json` is the evidence a retry needs. Each backup is + // created once (create-new), so a re-run after a partial failure never + // overwrites the pristine copy with a half-migrated snapshot. + if rewrites > 0 { + let bak = crate::util::resolved_backup_path( + &teams_path, + "teams.json.pre-team-membership-repair.bak", + ); + crate::util::create_restricted_backup_once(&bak, teams_content.as_bytes()) + .map_err(|e| format!("failed to write teams.json backup: {e}"))?; + } + if backfills > 0 { + let bak = crate::util::resolved_backup_path( + &agents_path, + "managed-agents.json.pre-team-membership-repair.bak", + ); + crate::util::create_restricted_backup_once(&bak, agents_content.as_bytes()) + .map_err(|e| format!("failed to write managed-agents.json backup: {e}"))?; + } + + if rewrites > 0 { + let payload = serde_json::to_vec_pretty(&teams) + .map_err(|e| format!("failed to serialize teams.json: {e}"))?; + crate::managed_agents::atomic_write_json(&teams_path, &payload)?; + } + + if backfills > 0 { + // Restricted: this store can carry plaintext agent nsecs on a + // keyringless host (SECURITY.md:90). + let payload = serde_json::to_vec_pretty(&agents) + .map_err(|e| format!("failed to serialize managed-agents.json: {e}"))?; + crate::managed_agents::atomic_write_json_restricted(&agents_path, &payload)?; + } + + Ok(rewrites + backfills) +} + +/// Set of persona ids that resolve to a definition — the definition records are +/// the key-less unified-store entries (`pubkey == ""`); their `slug` is the id +/// a team references. +fn resolvable_ids(agents: &[ManagedAgentRecord]) -> Vec<&str> { + agents + .iter() + .filter(|r| r.pubkey.is_empty()) + .filter_map(|r| r.slug.as_deref()) + .collect() +} + +/// Rewrite each team's stale `persona_ids` to the persona they name, when +/// unambiguous. Returns the number of ids rewritten. +/// +/// An id is *stale* when no definition slug equals it. Its repair target is the +/// definition whose `source_team_persona_slug` equals the stale id — i.e. the +/// bare slug is the pre-namespacing form of that persona's namespaced slug. The +/// rewrite happens only when exactly one such definition exists (optionally +/// scoped to the team's source team); zero or many candidates leave the id +/// untouched, which is strictly safer than the save path that drops it. +fn rewrite_stale_persona_ids(teams: &mut [TeamRecord], agents: &[ManagedAgentRecord]) -> usize { + let resolvable = resolvable_ids(agents); + let definitions: Vec<&ManagedAgentRecord> = + agents.iter().filter(|r| r.pubkey.is_empty()).collect(); + + let mut rewritten = 0usize; + for team in teams.iter_mut() { + // Scope candidate personas to this team's source team when derivable: + // a directory-backed team keys off its source_dir name; a detached team + // keys off the unique source_team of its already-resolvable members. + let scope = team_source_scope(team, &definitions); + for id in team.persona_ids.iter_mut() { + if resolvable.contains(&id.as_str()) { + continue; + } + let candidates: Vec<&&ManagedAgentRecord> = definitions + .iter() + .filter(|d| d.source_team_persona_slug.as_deref() == Some(id.as_str())) + .filter(|d| match scope.as_deref() { + Some(team_key) => d.source_team.as_deref() == Some(team_key), + None => true, + }) + .collect(); + let [only] = candidates.as_slice() else { + eprintln!( + "buzz-desktop: team-membership-repair: team {:?}: leaving unresolvable \ + persona id {:?} ({} candidate(s))", + team.id, + id, + candidates.len() + ); + continue; + }; + if let Some(slug) = only.slug.as_deref() { + *id = slug.to_string(); + rewritten += 1; + } + } + } + rewritten +} + +/// The source-team key that scopes a team's persona candidates, or `None` when +/// it cannot be derived (matching then falls back to a global unique slug). +/// +/// Directory-backed teams use `team_persona_key` (the pack manifest id). A +/// detached team (`source_dir` cleared) has no such key, so we infer it from +/// the unique `source_team` among its members that already resolve. +fn team_source_scope(team: &TeamRecord, definitions: &[&ManagedAgentRecord]) -> Option { + if team.source_dir.is_some() { + return Some(team_persona_key(team).to_string()); + } + let mut source_teams: Vec<&str> = team + .persona_ids + .iter() + .filter_map(|id| { + definitions + .iter() + .find(|d| d.slug.as_deref() == Some(id.as_str())) + .and_then(|d| d.source_team.as_deref()) + }) + .collect(); + source_teams.sort_unstable(); + source_teams.dedup(); + match source_teams.as_slice() { + [only] => Some((*only).to_string()), + _ => None, + } +} + +/// Repair instance `team_id` against the current rosters. Returns the number of +/// instances changed. +/// +/// Two directions, both conservative and evidence-gated: +/// +/// - **Unbound → bound (backfill).** An instance whose persona is a team member +/// but whose own `team_id` is unset is bound to that team, so it spawns with +/// the team's instructions. Only when the persona belongs to *exactly one* +/// team — a persona spanning several teams has no evidence selecting one +/// (JSON team order is not ownership), so it is left unbound and logged. +/// - **Stale binding → cleared or re-pointed.** An instance bound to a team +/// whose roster no longer lists its persona (a "keep agents" removal left the +/// binding behind, so the kept instance keeps drawing that team's +/// instructions at spawn) is healed: re-pointed when the persona now belongs +/// to exactly one *other* team (same single-evidence rule), otherwise unbound +/// and logged. A binding whose team still lists the persona is authoritative +/// and never touched. +/// +/// Idempotent: after a repair every instance is either bound to a team that +/// lists it or unbound with no single-team evidence, so a second pass is a +/// no-op. +fn backfill_instance_team_ids(teams: &[TeamRecord], agents: &mut [ManagedAgentRecord]) -> usize { + // persona_id → the sole team referencing it, or None once a *distinct* + // second team is seen (ambiguous → never used as binding evidence). A + // persona listed twice within one team is not ambiguity — duplicates are + // not prohibited at the storage boundary (`ensure_persona_ids_are_active` + // checks existence only; create/update/inbound persist the vector + // unchanged), so poisoning on a same-team repeat would strand a + // legitimately single-team instance. + let mut persona_to_team: HashMap<&str, Option<&str>> = HashMap::new(); + // Team ids that exist in the store, and the (team_id, persona_id) pairs they + // list. A binding is *stale* only when its team still exists but no longer + // lists the persona — a binding to an absent team is left alone (it already + // degrades to no instructions via `effective_team_instructions`, and a + // deleted team is not this repair's concern). + let mut team_ids: std::collections::HashSet<&str> = std::collections::HashSet::new(); + let mut membership: std::collections::HashSet<(&str, &str)> = std::collections::HashSet::new(); + for team in teams { + team_ids.insert(team.id.as_str()); + for persona_id in &team.persona_ids { + membership.insert((team.id.as_str(), persona_id.as_str())); + persona_to_team + .entry(persona_id.as_str()) + .and_modify(|slot| { + if slot.is_some_and(|seen| seen != team.id.as_str()) { + *slot = None; + } + }) + .or_insert(Some(team.id.as_str())); + } + } + + let mut repaired = 0usize; + for agent in agents.iter_mut() { + if agent.pubkey.is_empty() { + continue; + } + let Some(persona_id) = agent.persona_id.as_deref() else { + continue; + }; + match agent.team_id.as_deref() { + // Live binding, or a binding to an absent team: leave it. A binding + // is only stale when its team exists and dropped the persona. + Some(bound) + if !team_ids.contains(bound) || membership.contains(&(bound, persona_id)) => {} + // Stale binding: the still-present bound team dropped this persona. + // Re-point on single-team evidence, else unbind — never guess. + Some(_) => match persona_to_team.get(persona_id) { + Some(Some(team_id)) => { + agent.team_id = Some((*team_id).to_string()); + repaired += 1; + } + _ => { + eprintln!( + "buzz-desktop: team-membership-repair: unbinding instance {:?} — persona \ + {persona_id:?} left its team's roster with no single-team successor", + agent.pubkey + ); + agent.team_id = None; + repaired += 1; + } + }, + // Unbound: backfill on single-team evidence. + None => match persona_to_team.get(persona_id) { + Some(Some(team_id)) => { + agent.team_id = Some((*team_id).to_string()); + repaired += 1; + } + Some(None) => eprintln!( + "buzz-desktop: team-membership-repair: leaving instance {:?} unbound — persona \ + {persona_id:?} spans multiple teams", + agent.pubkey + ), + None => {} + }, + } + } + repaired +} + +#[cfg(test)] +#[path = "team_membership_tests.rs"] +mod tests; diff --git a/desktop/src-tauri/src/migration/team_membership_tests.rs b/desktop/src-tauri/src/migration/team_membership_tests.rs new file mode 100644 index 0000000000..d284d56423 --- /dev/null +++ b/desktop/src-tauri/src/migration/team_membership_tests.rs @@ -0,0 +1,625 @@ +use super::repair_team_membership_in_dir; +use crate::migration::test_support::{ + read_agents_json, read_teams_json, write_agents_json, write_teams_json, +}; +use std::path::{Path, PathBuf}; + +fn base(dir: &Path) -> PathBuf { + dir.join("agents") +} + +/// A key-less definition record: `pubkey == ""`, persona id == `slug`. +/// `source_team` is the manifest id; `source_team_persona_slug` is the +/// pre-namespacing bare slug a stale team id would carry. +fn definition(slug: &str, source_team: &str, bare_slug: &str) -> serde_json::Value { + serde_json::json!({ + "name": slug, + "pubkey": "", + "relay_url": "ws://localhost:3000", + "acp_command": "buzz-acp", + "agent_command": "goose", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "parallelism": 4, + "system_prompt": "prompt", + "model": "gpt-x", + "provider": "openai", + "env_vars": {}, + "start_on_app_launch": true, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z", + "slug": slug, + "source_team": source_team, + "source_team_persona_slug": bare_slug, + }) +} + +/// A standalone definition with no team provenance (persona id == slug). +fn standalone_definition(slug: &str) -> serde_json::Value { + serde_json::json!({ + "name": slug, + "pubkey": "", + "relay_url": "ws://localhost:3000", + "acp_command": "buzz-acp", + "agent_command": "goose", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "parallelism": 4, + "system_prompt": "prompt", + "model": "gpt-x", + "provider": "openai", + "env_vars": {}, + "start_on_app_launch": true, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z", + "slug": slug, + }) +} + +/// A running instance record: `pubkey` set, linked to a persona by `persona_id`. +fn instance(pubkey_seed: char, persona_id: &str, team_id: Option<&str>) -> serde_json::Value { + let mut record = serde_json::json!({ + "name": persona_id, + "pubkey": pubkey_seed.to_string().repeat(64), + "relay_url": "ws://localhost:3000", + "acp_command": "buzz-acp", + "agent_command": "goose", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "parallelism": 4, + "system_prompt": "prompt", + "model": "gpt-x", + "provider": "openai", + "env_vars": {}, + "start_on_app_launch": true, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z", + "persona_id": persona_id, + }); + record["team_id"] = match team_id { + Some(id) => serde_json::json!(id), + None => serde_json::Value::Null, + }; + record +} + +fn team(id: &str, persona_ids: &[&str]) -> serde_json::Value { + serde_json::json!({ + "id": id, + "name": "Sietch Tabr", + "description": null, + "persona_ids": persona_ids, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z", + }) +} + +fn team_persona_ids(dir: &Path, id: &str) -> Vec { + read_teams_json(dir) + .into_iter() + .find(|t| t["id"] == id) + .unwrap()["persona_ids"] + .as_array() + .unwrap() + .iter() + .map(|v| v.as_str().unwrap().to_string()) + .collect() +} + +fn instance_team_id(dir: &Path, pubkey_seed: char) -> Option { + read_agents_json(dir) + .into_iter() + .find(|r| r["pubkey"].as_str() == Some(&pubkey_seed.to_string().repeat(64))) + .unwrap()["team_id"] + .as_str() + .map(str::to_string) +} + +const TEAM_ID: &str = "ab5c038c-1b12-46e2-8283-d6f7c0606fce"; +const ST: &str = "com.wpfleger.sietch-tabr"; + +/// Will's pre-fix store: the team holds four bare pre-namespacing ids plus one +/// resolvable standalone id. Each bare id names exactly one team persona, so +/// all four are rewritten to their namespaced slug and the standalone id is +/// left untouched — the class the save path silently drops. +#[test] +fn rewrites_bare_ids_to_namespaced_slugs() { + let dir = tempfile::tempdir().unwrap(); + write_teams_json( + dir.path(), + &serde_json::json!([team( + TEAM_ID, + &["369695d6", "thufir", "paul", "duncan", "alia"] + )]), + ); + write_agents_json( + dir.path(), + &serde_json::json!([ + standalone_definition("369695d6"), + definition("sietch-tabr:thufir", ST, "thufir"), + definition("sietch-tabr:paul", ST, "paul"), + definition("sietch-tabr:duncan", ST, "duncan"), + definition("sietch-tabr:alia", ST, "alia"), + ]), + ); + + assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 4); + assert_eq!( + team_persona_ids(dir.path(), TEAM_ID), + vec![ + "369695d6", + "sietch-tabr:thufir", + "sietch-tabr:paul", + "sietch-tabr:duncan", + "sietch-tabr:alia", + ] + ); +} + +/// A directory-backed team scopes candidates by its `source_dir` name (the pack +/// manifest id), so a bare slug that appears under two different source teams is +/// disambiguated to the one this team is sourced from. +#[test] +fn scopes_candidates_by_source_dir_for_directory_backed_team() { + let dir = tempfile::tempdir().unwrap(); + let mut t = team(TEAM_ID, &["thufir"]); + t["source_dir"] = serde_json::json!(format!("/packs/{ST}")); + write_teams_json(dir.path(), &serde_json::json!([t])); + write_agents_json( + dir.path(), + &serde_json::json!([ + definition("sietch-tabr:thufir", ST, "thufir"), + // A collision: a different team also has a persona whose bare slug + // is "thufir". Without source scoping this would be ambiguous. + definition("other:thufir", "com.other.pack", "thufir"), + ]), + ); + + assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 1); + assert_eq!( + team_persona_ids(dir.path(), TEAM_ID), + vec!["sietch-tabr:thufir"] + ); +} + +/// A bare id that names two personas with no usable scope is ambiguous: the +/// migration leaves it in place (strictly safer than the save path, which drops +/// it) and the file is not rewritten. +#[test] +fn leaves_ambiguous_id_in_place_without_writing() { + let dir = tempfile::tempdir().unwrap(); + // Detached team (no source_dir) with a single stale member => no resolvable + // sibling to infer a source-team scope from. + write_teams_json(dir.path(), &serde_json::json!([team(TEAM_ID, &["thufir"])])); + write_agents_json( + dir.path(), + &serde_json::json!([ + definition("sietch-tabr:thufir", ST, "thufir"), + definition("other:thufir", "com.other.pack", "thufir"), + ]), + ); + let before = std::fs::read_to_string(base(dir.path()).join("teams.json")).unwrap(); + + assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 0); + assert_eq!(team_persona_ids(dir.path(), TEAM_ID), vec!["thufir"]); + assert_eq!( + std::fs::read_to_string(base(dir.path()).join("teams.json")).unwrap(), + before, + "an ambiguous-only store is never rewritten" + ); + assert!( + !base(dir.path()) + .join("teams.json.pre-team-membership-repair.bak") + .exists(), + "no backup when nothing is repaired" + ); +} + +/// A detached team infers its source-team scope from the unique `source_team` +/// among its already-resolvable members, so a bare id is disambiguated even +/// without a `source_dir`. +#[test] +fn infers_scope_from_resolvable_siblings_when_detached() { + let dir = tempfile::tempdir().unwrap(); + write_teams_json( + dir.path(), + &serde_json::json!([team(TEAM_ID, &["sietch-tabr:paul", "thufir"])]), + ); + write_agents_json( + dir.path(), + &serde_json::json!([ + definition("sietch-tabr:paul", ST, "paul"), + definition("sietch-tabr:thufir", ST, "thufir"), + definition("other:thufir", "com.other.pack", "thufir"), + ]), + ); + + assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 1); + assert_eq!( + team_persona_ids(dir.path(), TEAM_ID), + vec!["sietch-tabr:paul", "sietch-tabr:thufir"] + ); +} + +/// Backfill sets `team_id` on an instance whose persona is a team member but +/// whose own `team_id` is null (the Gurney case), and leaves an already-bound +/// instance untouched (a persona shared across teams keeps its binding). +#[test] +fn backfills_null_team_id_but_never_re_points_a_bound_instance() { + let dir = tempfile::tempdir().unwrap(); + write_teams_json( + dir.path(), + &serde_json::json!([team(TEAM_ID, &["sietch-tabr:gurney", "sietch-tabr:hayt"])]), + ); + write_agents_json( + dir.path(), + &serde_json::json!([ + definition("sietch-tabr:gurney", ST, "gurney"), + definition("sietch-tabr:hayt", ST, "hayt"), + instance('g', "sietch-tabr:gurney", None), + instance('h', "sietch-tabr:hayt", Some("other-team")), + ]), + ); + + assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 1); + assert_eq!(instance_team_id(dir.path(), 'g').as_deref(), Some(TEAM_ID)); + assert_eq!( + instance_team_id(dir.path(), 'h').as_deref(), + Some("other-team"), + "an already-bound instance is never re-pointed" + ); +} + +/// A legacy unbound instance whose persona belongs to *two* teams is left +/// unbound: JSON team order is not ownership evidence, and the product permits +/// one persona under multiple teams with distinct instructions. Its team +/// sibling — a persona in only one team — is still backfilled in the same pass. +#[test] +fn leaves_unbound_instance_of_a_multi_team_persona_unbound() { + let dir = tempfile::tempdir().unwrap(); + write_teams_json( + dir.path(), + &serde_json::json!([ + team(TEAM_ID, &["sietch-tabr:duncan", "sietch-tabr:paul"]), + team("other-team", &["sietch-tabr:duncan"]), + ]), + ); + write_agents_json( + dir.path(), + &serde_json::json!([ + definition("sietch-tabr:duncan", ST, "duncan"), + definition("sietch-tabr:paul", ST, "paul"), + instance('d', "sietch-tabr:duncan", None), + instance('p', "sietch-tabr:paul", None), + ]), + ); + + // Only Paul (single-team) is backfilled; Duncan (two teams) stays unbound. + assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 1); + assert_eq!(instance_team_id(dir.path(), 'd'), None); + assert_eq!(instance_team_id(dir.path(), 'p').as_deref(), Some(TEAM_ID)); +} + +/// A persona listed twice within a *single* team is not ambiguity — the storage +/// boundary does not dedupe `persona_ids`. Its unbound instance is still bound +/// to that one team; only a *distinct* second team poisons the entry. +#[test] +fn same_team_duplicate_persona_id_still_backfills() { + let dir = tempfile::tempdir().unwrap(); + write_teams_json( + dir.path(), + &serde_json::json!([team(TEAM_ID, &["sietch-tabr:duncan", "sietch-tabr:duncan"])]), + ); + write_agents_json( + dir.path(), + &serde_json::json!([ + definition("sietch-tabr:duncan", ST, "duncan"), + instance('d', "sietch-tabr:duncan", None), + ]), + ); + + assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 1); + assert_eq!(instance_team_id(dir.path(), 'd').as_deref(), Some(TEAM_ID)); +} + +/// A stale binding — the bound team no longer lists the instance's persona (a +/// "keep agents" removal left it behind) — is cleared when no other single team +/// claims the persona, so the kept instance stops drawing that team's +/// instructions at spawn. +#[test] +fn clears_stale_binding_when_persona_left_its_team() { + let dir = tempfile::tempdir().unwrap(); + // The team no longer lists gurney; the instance is still bound to it. + write_teams_json( + dir.path(), + &serde_json::json!([team(TEAM_ID, &["sietch-tabr:paul"])]), + ); + write_agents_json( + dir.path(), + &serde_json::json!([ + definition("sietch-tabr:gurney", ST, "gurney"), + definition("sietch-tabr:paul", ST, "paul"), + instance('g', "sietch-tabr:gurney", Some(TEAM_ID)), + ]), + ); + + assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 1); + assert_eq!(instance_team_id(dir.path(), 'g'), None); +} + +/// A stale binding is *re-pointed* — not merely cleared — when the persona now +/// belongs to exactly one other team, matching the single-evidence backfill +/// rule. +#[test] +fn repoints_stale_binding_to_the_sole_successor_team() { + let dir = tempfile::tempdir().unwrap(); + // gurney left TEAM_ID but is the sole member of other-team. + write_teams_json( + dir.path(), + &serde_json::json!([ + team(TEAM_ID, &["sietch-tabr:paul"]), + team("other-team", &["sietch-tabr:gurney"]), + ]), + ); + write_agents_json( + dir.path(), + &serde_json::json!([ + definition("sietch-tabr:gurney", ST, "gurney"), + definition("sietch-tabr:paul", ST, "paul"), + instance('g', "sietch-tabr:gurney", Some(TEAM_ID)), + ]), + ); + + assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 1); + assert_eq!( + instance_team_id(dir.path(), 'g').as_deref(), + Some("other-team") + ); +} + +/// A binding whose team still lists the persona is authoritative — a repair pass +/// leaves it untouched even when that persona also belongs to another team. +#[test] +fn leaves_live_binding_untouched_for_multi_team_persona() { + let dir = tempfile::tempdir().unwrap(); + write_teams_json( + dir.path(), + &serde_json::json!([ + team(TEAM_ID, &["sietch-tabr:duncan"]), + team("other-team", &["sietch-tabr:duncan"]), + ]), + ); + write_agents_json( + dir.path(), + &serde_json::json!([ + definition("sietch-tabr:duncan", ST, "duncan"), + instance('d', "sietch-tabr:duncan", Some(TEAM_ID)), + ]), + ); + + assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 0); + assert_eq!(instance_team_id(dir.path(), 'd').as_deref(), Some(TEAM_ID)); +} + +/// A store that needs no repair is a clean no-op: `Ok(0)`, no write, no backup. +#[test] +fn clean_store_is_a_no_op() { + let dir = tempfile::tempdir().unwrap(); + write_teams_json( + dir.path(), + &serde_json::json!([team(TEAM_ID, &["sietch-tabr:paul"])]), + ); + write_agents_json( + dir.path(), + &serde_json::json!([ + definition("sietch-tabr:paul", ST, "paul"), + instance('p', "sietch-tabr:paul", Some(TEAM_ID)), + ]), + ); + let teams_before = std::fs::read_to_string(base(dir.path()).join("teams.json")).unwrap(); + let agents_before = + std::fs::read_to_string(base(dir.path()).join("managed-agents.json")).unwrap(); + + assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 0); + assert_eq!( + std::fs::read_to_string(base(dir.path()).join("teams.json")).unwrap(), + teams_before + ); + assert_eq!( + std::fs::read_to_string(base(dir.path()).join("managed-agents.json")).unwrap(), + agents_before + ); +} + +/// The full repair is idempotent: a second boot over the already-repaired store +/// finds nothing to do and does not write. +#[test] +fn second_run_is_a_no_op() { + let dir = tempfile::tempdir().unwrap(); + write_teams_json(dir.path(), &serde_json::json!([team(TEAM_ID, &["thufir"])])); + write_agents_json( + dir.path(), + &serde_json::json!([ + definition("sietch-tabr:thufir", ST, "thufir"), + instance('t', "sietch-tabr:thufir", None), + ]), + ); + + assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 2); + let teams_after = std::fs::read_to_string(base(dir.path()).join("teams.json")).unwrap(); + let agents_after = + std::fs::read_to_string(base(dir.path()).join("managed-agents.json")).unwrap(); + + assert_eq!( + repair_team_membership_in_dir(&base(dir.path())).unwrap(), + 0, + "second run finds nothing" + ); + assert_eq!( + std::fs::read_to_string(base(dir.path()).join("teams.json")).unwrap(), + teams_after + ); + assert_eq!( + std::fs::read_to_string(base(dir.path()).join("managed-agents.json")).unwrap(), + agents_after + ); +} + +#[test] +fn missing_store_is_a_no_op() { + let dir = tempfile::tempdir().unwrap(); + std::fs::create_dir_all(base(dir.path())).unwrap(); + assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 0); +} + +#[test] +fn unparseable_store_errors_without_writing() { + let dir = tempfile::tempdir().unwrap(); + std::fs::create_dir_all(base(dir.path())).unwrap(); + let teams_path = base(dir.path()).join("teams.json"); + std::fs::write(&teams_path, "{ not json").unwrap(); + write_agents_json(dir.path(), &serde_json::json!([])); + + let err = repair_team_membership_in_dir(&base(dir.path())).unwrap_err(); + assert!(err.contains("failed to parse"), "unexpected error: {err}"); + assert_eq!( + std::fs::read_to_string(&teams_path).unwrap(), + "{ not json", + "a corrupt store is left for manual recovery" + ); +} + +/// The teams.json backup captures the pre-migration bytes and is written once. +#[test] +fn writes_teams_backup_once() { + let dir = tempfile::tempdir().unwrap(); + write_teams_json(dir.path(), &serde_json::json!([team(TEAM_ID, &["thufir"])])); + write_agents_json( + dir.path(), + &serde_json::json!([definition("sietch-tabr:thufir", ST, "thufir")]), + ); + let bak = base(dir.path()).join("teams.json.pre-team-membership-repair.bak"); + + assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 1); + let bak_content = std::fs::read_to_string(&bak).unwrap(); + assert!( + bak_content.contains("\"thufir\""), + "backup holds the pre-migration stale id" + ); +} + +/// Both pristine backups are created BEFORE either live store is rewritten, so +/// a crash between the two writes still leaves a full recovery pair (Carl's +/// backup-contract finding). A stale bare slug on the team (drives the teams +/// rewrite) plus an unbound instance (drives the agents backfill) exercises +/// both stores; each backup must hold the pre-migration bytes. +#[test] +fn both_backups_precede_either_live_write() { + let dir = tempfile::tempdir().unwrap(); + write_teams_json(dir.path(), &serde_json::json!([team(TEAM_ID, &["thufir"])])); + write_agents_json( + dir.path(), + &serde_json::json!([ + definition("sietch-tabr:thufir", ST, "thufir"), + instance('t', "sietch-tabr:thufir", None), + ]), + ); + let teams_bak = base(dir.path()).join("teams.json.pre-team-membership-repair.bak"); + let agents_bak = base(dir.path()).join("managed-agents.json.pre-team-membership-repair.bak"); + + assert_eq!(repair_team_membership_in_dir(&base(dir.path())).unwrap(), 2); + + // teams.json backup holds the stale bare slug (pre-rewrite bytes). + let teams_bak_content = std::fs::read_to_string(&teams_bak).unwrap(); + assert!( + teams_bak_content.contains("\"thufir\"") + && !teams_bak_content.contains("sietch-tabr:thufir"), + "teams backup captures pre-rewrite bytes" + ); + // managed-agents.json backup holds the null binding (pre-backfill bytes). + let agents_bak_content = std::fs::read_to_string(&agents_bak).unwrap(); + assert!( + agents_bak_content.contains("\"team_id\": null"), + "agents backup captures pre-backfill bytes" + ); +} + +// ── repair→detach orchestration gate (Carl's finding #2) ────────────────── + +use super::orchestrate_repair_then_detach; +use std::cell::Cell; + +/// A failed repair must SKIP the directory-backed detach: detach clears +/// `source_dir`, the disambiguating evidence a clean-repair retry needs, so +/// running it after a repair error would let the original membership-loss path +/// recur on the next boot. +#[test] +fn failed_repair_skips_detach() { + let detach_ran = Cell::new(false); + orchestrate_repair_then_detach( + || Err("repair write failed".to_string()), + || { + detach_ran.set(true); + Ok(0) + }, + ); + assert!( + !detach_ran.get(), + "detach must not run when repair failed — source_dir is preserved for retry" + ); +} + +/// A successful repair runs detach, whether or not the repair changed anything +/// (a clean-store boot with directory-backed teams still needs detaching). +#[test] +fn successful_repair_runs_detach() { + let detach_ran = Cell::new(false); + orchestrate_repair_then_detach( + || Ok(0), + || { + detach_ran.set(true); + Ok(1) + }, + ); + assert!( + detach_ran.get(), + "detach runs after a clean repair even when repair changed nothing" + ); +} + +/// End-to-end discriminating proof: a failed repair must leave a +/// directory-backed team's `source_dir` intact, because the gate skips the real +/// detach op that would otherwise clear it. The store here is fully valid — so +/// detach WOULD succeed and strip `source_dir` if the gate let it run — which +/// is what makes this catch a gate that runs detach unconditionally. +#[test] +fn failed_repair_preserves_source_dir_against_real_detach() { + let dir = tempfile::tempdir().unwrap(); + let base_dir = base(dir.path()); + let mut t = team(TEAM_ID, &["sietch-tabr:thufir"]); + t["source_dir"] = serde_json::json!(format!("/packs/{ST}")); + write_teams_json(dir.path(), &serde_json::json!([t])); + write_agents_json( + dir.path(), + &serde_json::json!([definition("sietch-tabr:thufir", ST, "thufir")]), + ); + + orchestrate_repair_then_detach( + || Err("repair backup write failed".to_string()), + || super::super::detach::detach_directory_backed_teams_in_dir(&base_dir), + ); + + let source_dir = read_teams_json(dir.path()) + .into_iter() + .find(|t| t["id"] == TEAM_ID) + .unwrap()["source_dir"] + .clone(); + assert_eq!( + source_dir, + serde_json::json!(format!("/packs/{ST}")), + "a failed repair must preserve source_dir — detach never ran to clear it" + ); +} diff --git a/desktop/src-tauri/src/migration_test_support.rs b/desktop/src-tauri/src/migration_test_support.rs index 64a428949b..b68415c6b5 100644 --- a/desktop/src-tauri/src/migration_test_support.rs +++ b/desktop/src-tauri/src/migration_test_support.rs @@ -29,3 +29,17 @@ pub(crate) fn read_personas_json(dir: &Path) -> Vec { let content = std::fs::read_to_string(dir.join("agents/personas.json")).unwrap(); serde_json::from_str(&content).unwrap() } + +pub(crate) fn write_teams_json(dir: &Path, records: &serde_json::Value) { + std::fs::create_dir_all(dir.join("agents")).unwrap(); + std::fs::write( + dir.join("agents/teams.json"), + serde_json::to_vec_pretty(records).unwrap(), + ) + .unwrap(); +} + +pub(crate) fn read_teams_json(dir: &Path) -> Vec { + let content = std::fs::read_to_string(dir.join("agents/teams.json")).unwrap(); + serde_json::from_str(&content).unwrap() +} diff --git a/desktop/src-tauri/tauri.conf.json b/desktop/src-tauri/tauri.conf.json index 2f85c5d517..dd7ab08e06 100644 --- a/desktop/src-tauri/tauri.conf.json +++ b/desktop/src-tauri/tauri.conf.json @@ -1,7 +1,7 @@ { "$schema": "https://schema.tauri.app/config/2", "productName": "Buzz", - "version": "0.5.14", + "version": "0.5.15", "identifier": "xyz.block.buzz.app", "build": { "beforeDevCommand": { diff --git a/desktop/src/features/messages/lib/agentMentionRevalidation.test.mjs b/desktop/src/features/messages/lib/agentMentionRevalidation.test.mjs index 214c9a950f..d5a5727855 100644 --- a/desktop/src/features/messages/lib/agentMentionRevalidation.test.mjs +++ b/desktop/src/features/messages/lib/agentMentionRevalidation.test.mjs @@ -7,6 +7,7 @@ const CURRENT = "a".repeat(64); const AGENT = "b".repeat(64); const HUMAN = "c".repeat(64); const OTHER_OWNER = "d".repeat(64); +const LOCAL_AGENT = "e".repeat(64); function options(refetchOwnerProfiles) { return { @@ -49,6 +50,62 @@ test("owner-only revalidation admits an agent only from a fresh same-owner proof assert.deepEqual(result, [HUMAN, AGENT]); }); +test("fresh managed evidence survives unrelated relay authorization errors", async () => { + const result = await revalidateAgentMentionPubkeys({ + ...options(async () => { + throw new Error("owner profiles unavailable"); + }), + pubkeys: [HUMAN, LOCAL_AGENT], + agentPubkeys: new Set([LOCAL_AGENT]), + refetchManagedAgents: async () => ({ + data: [{ pubkey: LOCAL_AGENT }], + error: null, + }), + refetchRelayAgents: async () => ({ + data: undefined, + error: new Error("relay directory unavailable"), + }), + }); + + assert.deepEqual(result, [HUMAN, LOCAL_AGENT]); +}); + +test("relay-only agents still fail closed when relay discovery fails", async () => { + const result = await revalidateAgentMentionPubkeys({ + ...options(async () => ({ + profiles: { [AGENT]: { ownerPubkey: CURRENT } }, + missing: [], + })), + refetchRelayAgents: async () => ({ + data: undefined, + error: new Error("relay directory unavailable"), + }), + }); + + assert.deepEqual(result, [HUMAN]); +}); + +test("mixed evidence preserves only fresh managed agents and humans", async () => { + const result = await revalidateAgentMentionPubkeys({ + ...options(async () => ({ + profiles: { [AGENT]: { ownerPubkey: CURRENT } }, + missing: [LOCAL_AGENT], + })), + pubkeys: [HUMAN, LOCAL_AGENT, AGENT], + agentPubkeys: new Set([LOCAL_AGENT, AGENT]), + refetchManagedAgents: async () => ({ + data: [{ pubkey: LOCAL_AGENT }], + error: null, + }), + refetchRelayAgents: async () => ({ + data: undefined, + error: new Error("relay directory unavailable"), + }), + }); + + assert.deepEqual(result, [HUMAN, LOCAL_AGENT]); +}); + for (const [name, refetchOwnerProfiles] of [ ["revoked owner proof", async () => ({ profiles: {}, missing: [AGENT] })], [ diff --git a/desktop/src/features/messages/lib/agentMentionRevalidation.ts b/desktop/src/features/messages/lib/agentMentionRevalidation.ts index d6c665ba67..1e6b3a7d66 100644 --- a/desktop/src/features/messages/lib/agentMentionRevalidation.ts +++ b/desktop/src/features/messages/lib/agentMentionRevalidation.ts @@ -57,14 +57,13 @@ export async function revalidateAgentMentionPubkeys({ ? refetchOwnerProfiles([...requestedAgentPubkeys]).catch(() => null) : Promise.resolve(null), ]); + const relayDirectoryReady = + relayResult.error === null && relayResult.data !== undefined; if ( - managedResult.error !== null || - relayResult.error !== null || - managedResult.data === undefined || - relayResult.data === undefined || ownerOnly === undefined || ownerPolicyError !== null || - (ownerOnly && ownerProfiles === null) + managedResult.error !== null || + managedResult.data === undefined ) { return filterAdmittedMentionPubkeys(pubkeys, agentPubkeys, new Set()); } @@ -76,23 +75,28 @@ export async function revalidateAgentMentionPubkeys({ currentPubkey, eligibilityScope, managedAgentPubkeys: managedPubkeys, - relayAgents: relayResult.data, + relayAgents: relayDirectoryReady ? relayResult.data : [], sharedChannelIds, }); const admittedPubkeys = new Set( - [...agentPubkeys].filter( - (pubkey) => + [...agentPubkeys].filter((pubkey) => { + const isManagedAgent = managedPubkeys.has(normalizePubkey(pubkey)); + const directoryReady = + isManagedAgent || + (relayDirectoryReady && (!ownerOnly || ownerProfiles !== null)); + return ( getAgentMentionAdmission({ isAgent: true, - isManagedAgent: managedPubkeys.has(pubkey), + isManagedAgent, pubkey, ownerPubkey: ownerProfiles?.profiles[pubkey]?.ownerPubkey, currentPubkey, mentionableAgentPubkeys: mentionablePubkeys, - directoryReady: true, + directoryReady, ownerOnly, - }) === "allow", - ), + }) === "allow" + ); + }), ); return filterAdmittedMentionPubkeys(pubkeys, agentPubkeys, admittedPubkeys); } diff --git a/desktop/src/features/sidebar/ui/AppSidebarPinnedHeader.tsx b/desktop/src/features/sidebar/ui/AppSidebarPinnedHeader.tsx index d0e54ecdcb..bfcb0371a2 100644 --- a/desktop/src/features/sidebar/ui/AppSidebarPinnedHeader.tsx +++ b/desktop/src/features/sidebar/ui/AppSidebarPinnedHeader.tsx @@ -103,7 +103,7 @@ export function AppSidebarPrimaryMenu({ data-tauri-drag-region data-testid="sidebar-primary-menu" > - + - - - Inbox - + + Inbox {homeBadgeCount > 0 ? ( - - - Agents - + + Agents diff --git a/desktop/src/shared/styles/globals/components.css b/desktop/src/shared/styles/globals/components.css index 6dd0054669..b1fa1d9f1e 100644 --- a/desktop/src/shared/styles/globals/components.css +++ b/desktop/src/shared/styles/globals/components.css @@ -1,4 +1,13 @@ @layer components { + /* Primary navigation rows share one inactive hierarchy, including preview + * features that can appear between the permanent destinations. Keeping the + * treatment on the menu prevents newly gated rows from silently rendering + * at stronger emphasis than Inbox and Agents. */ + .sidebar-primary-menu [data-sidebar="menu-button"]:not([data-active="true"]) + > :is(svg, [data-sidebar="menu-label"]) { + opacity: 0.8; + } + .buzz-side-panel-enter { animation: buzz-side-panel-enter 260ms cubic-bezier(0.32, 0.72, 0, 1) both; transform-origin: right center; diff --git a/desktop/tests/e2e/badge.spec.ts b/desktop/tests/e2e/badge.spec.ts index 98bb2dadef..f1b618b0d6 100644 --- a/desktop/tests/e2e/badge.spec.ts +++ b/desktop/tests/e2e/badge.spec.ts @@ -105,6 +105,40 @@ test("selected Inbox and Agents rows keep their highlight without bold text", as await expect(agents).toHaveCSS("font-weight", "400"); }); +test("primary navigation rows share the same inactive emphasis", async ({ + page, +}) => { + await page.goto("/"); + await page.getByTestId("channel-general").click(); + + const primaryMenu = page.getByTestId("sidebar-primary-menu"); + const inactiveRows = [ + primaryMenu.getByRole("button", { name: "Inbox", exact: true }), + page.getByTestId("open-pulse-view"), + page.getByTestId("open-projects-view"), + page.getByTestId("open-agents-view"), + page.getByTestId("open-workflows-view"), + ]; + + for (const row of inactiveRows) { + await expect(row).toHaveAttribute("data-active", "false"); + await expect(row.locator("[data-sidebar=menu-label]")).toHaveCSS( + "opacity", + "0.8", + ); + await expect(row.locator("svg")).toHaveCSS("opacity", "0.8"); + } + + const pulse = page.getByTestId("open-pulse-view"); + await pulse.click(); + await expect(pulse).toHaveAttribute("data-active", "true"); + await expect(pulse.locator("[data-sidebar=menu-label]")).toHaveCSS( + "opacity", + "1", + ); + await expect(pulse.locator("svg")).toHaveCSS("opacity", "1"); +}); + test("hovering a channel keeps its text color", async ({ page }) => { await page.goto("/"); const channel = page.getByTestId("channel-engineering"); diff --git a/desktop/tests/e2e/mentions.spec.ts b/desktop/tests/e2e/mentions.spec.ts index fc4dd8c546..4efd6dd7a3 100644 --- a/desktop/tests/e2e/mentions.spec.ts +++ b/desktop/tests/e2e/mentions.spec.ts @@ -1214,6 +1214,52 @@ test("relay-only allowlisted agents emit a p tag when sent", async ({ .toContain(ALLOWLIST_RELAY_AGENT_PUBKEY); }); +test("managed agents keep their p tag when relay discovery fails before send", async ({ + page, +}) => { + await installMockBridge(page, { + managedAgents: [ + { + pubkey: ALLOWLIST_RELAY_AGENT_PUBKEY, + name: "quinn", + status: "running", + }, + ], + relayAgents: [ + { + pubkey: ALLOWLIST_RELAY_AGENT_PUBKEY, + name: "quinn", + respondTo: "allowlist", + respondToAllowlist: [MOCK_VIEWER_PUBKEY], + channelNames: ["general"], + }, + ], + }); + await page.goto("/"); + await page.getByTestId("channel-general").click(); + + const input = page.getByTestId("message-input"); + await input.fill("@quinn"); + const quinnRow = autocomplete(page).locator("button", { hasText: "quinn" }); + await expect(quinnRow).toBeVisible(); + await quinnRow.click(); + await expect(input).toHaveText("@quinn "); + await page.keyboard.type("hello"); + await expect(input).toHaveText("@quinn hello"); + + await page.evaluate(() => { + window.__BUZZ_E2E__.mock ??= {}; + window.__BUZZ_E2E__.mock.relayAgentListErrors = Array(5).fill( + "mock unrelated relay directory failure", + ); + }); + await page.getByTestId("send-message").click(); + + await expect + .poll(() => readOutgoingMentionPubkeys(page, "@quinn hello")) + .toContain(ALLOWLIST_RELAY_AGENT_PUBKEY); +}); + test("selected relay agents revoked before send emit no p tag", async ({ page, }) => {