From b251901249bfc38c191b42368b304f5c4d095e7d Mon Sep 17 00:00:00 2001 From: quinn Date: Tue, 18 Aug 2026 10:49:39 +0000 Subject: [PATCH] =?UTF-8?q?DIVE-3560:=20fix=20buzz=20dms=20list=20?= =?UTF-8?q?=E2=80=94=20query=20relay-emitted=20kind:39000,=20not=20the=20n?= =?UTF-8?q?ever-emitted=20kind:41001?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `dms list` queried {kinds:[41001], "#p":[me]}. KIND_DM_CREATED (41001) is declared in buzz-core but no code path in this repo ever emits it, so the verb returned [] for every account on every relay while channels list, channels members and messages get all saw the same conversation. An empty list rather than an error is why this read as "no DMs exist" instead of "my predicate matched nothing" — and it is why the buzz plugin's inbound DM bridge (5dive-plugins #44) discovers zero DM targets on a real box. The relay-emitted truth is kind:39000 NIP-29 group metadata, which emit_group_discovery_events tags with t= plus one p tag per participant for DMs. kind:39000 is absent from required_scope_for_kind's allowlist, so client ingest rejects it as "restricted: unknown event kind" — only the relay can author one. t=dm is therefore relay-attested channel_type, not the attacker-controlled name/membership heuristic rejected on this row, which matters because consumers skip the @-mention test on the DM path. - dm_list_filter(): extracted so a regression to a kind nothing emits is a red test, not a silent empty list. - parse_query_events(): a relay error object is an error, not an empty inbox. The old unwrap_or_default() made a dead relay indistinguishable from no DMs. - dms_from_group_metadata(): keeps only t=dm with a non-empty d tag that lists us; a valueless ["t"] tag no longer masks a later ["t","dm"]; newest revision per channel wins; sorted newest first. - Annotated KIND_DM_CREATED as reserved-and-unemitted. Grading: 358/0 buzz-cli tests (9 new), 249/0 buzz-core, clippy clean, fmt clean. 9 mutants all killed, each asserted to have applied, baseline restored byte-identical: accept any channel_type; drop the participant re-check; accept an empty uuid; degrade a relay error to []; keep the oldest revision; query 41001 again (it survived until dm_list_filter was extracted and tested); valueless-tag masking; unscoped #p; limit cap removed. NOT VERIFIED: the e2e arm test_dms_list_query_shape_returns_the_dm (with a kind:41001 negative control) is #[ignore] per repo convention and needs a running relay. No relay, no docker and no reachable box from this seat, so it is unexecuted here. --- crates/buzz-cli/src/commands/dms.rs | 277 +++++++++++++++--- crates/buzz-core/src/kind.rs | 7 + .../tests/e2e_nostr_interop.rs | 87 ++++++ 3 files changed, 338 insertions(+), 33 deletions(-) diff --git a/crates/buzz-cli/src/commands/dms.rs b/crates/buzz-cli/src/commands/dms.rs index 589e4118270..1e2b42aeaeb 100644 --- a/crates/buzz-cli/src/commands/dms.rs +++ b/crates/buzz-cli/src/commands/dms.rs @@ -4,47 +4,135 @@ use crate::client::{extract_d_tag, normalize_write_response, BuzzClient}; use crate::error::CliError; use crate::validate::{parse_uuid, sdk_err, validate_hex64}; -/// List DM conversations by querying kind:41001 (relay-confirmed DMs) filtered by our pubkey. +/// List DM conversations for our pubkey. +/// +/// WIRE FACT (DIVE-3560): kind:41001 (`KIND_DM_CREATED`) is declared in +/// `buzz-core::kind` but **no code path anywhere in this repo ever emits it** — +/// the relay creates a DM in `handle_dm_open` and then publishes NIP-29 group +/// discovery events, never a 41001. The previous implementation queried +/// `{kinds:[41001], "#p":[me]}`, so it matched nothing and printed `[]` for +/// every account on every relay, while `channels list`, `channels members` and +/// `messages get` all saw the same conversation. An empty list, not an error, +/// is why this read as "no DMs exist" rather than "my predicate is wrong". +/// +/// The relay-emitted truth is kind:39000 (NIP-29 group metadata), which +/// `emit_group_discovery_events` tags with `t=` and, for DMs +/// only, one `p` tag per participant. We filter on `t == "dm"`. +/// +/// TRUST: kind:39000 is not in `required_scope_for_kind`'s allowlist, so client +/// ingest rejects it with "restricted: unknown event kind" — only the relay can +/// author one. `t=dm` is therefore relay-attested channel_type, not a +/// client-controlled name or membership heuristic. That distinction is the +/// whole point: consumers (the buzz plugin's inbound bridge) skip the +/// @-mention test on the DM path, so whatever marks a conversation as a DM is +/// what authorises unsolicited input into a session. pub async fn cmd_list_dms(client: &BuzzClient, limit: Option) -> Result<(), CliError> { let my_pk = client.keys().public_key().to_hex(); - let limit = limit.unwrap_or(50).min(200); - let filter = serde_json::json!({ - "kinds": [41001], + let resp = client.query(&dm_list_filter(&my_pk, limit)).await?; + let events = parse_query_events(&resp)?; + let dms = dms_from_group_metadata(&events, &my_pk); + let output = serde_json::to_string(&dms).unwrap_or_default(); + println!("{output}"); + Ok(()) +} + +/// The relay filter `dms list` sends. +/// +/// Kept separate so a regression to a kind nothing emits (see above) is a red +/// unit test rather than a silent empty list on every account. +fn dm_list_filter(my_pk: &str, limit: Option) -> serde_json::Value { + serde_json::json!({ + "kinds": [buzz_sdk::kind::KIND_NIP29_GROUP_METADATA], "#p": [my_pk], - "limit": limit - }); - let resp = client.query(&filter).await?; - let events: Vec = serde_json::from_str(&resp).unwrap_or_default(); - let dms: Vec = events - .iter() - .map(|e| { - let dm_id = extract_d_tag(e); - let participants: Vec = e - .get("tags") - .and_then(|t| t.as_array()) - .map(|tags| { - tags.iter() - .filter_map(|tag| { - let arr = tag.as_array()?; - if arr.first()?.as_str()? == "p" { - arr.get(1)?.as_str().map(|s| s.to_string()) - } else { - None - } - }) - .collect() - }) - .unwrap_or_default(); + "limit": limit.unwrap_or(50).min(200), + }) +} + +/// Parse a relay query response into an event array. +/// +/// A relay error is an object (`{"error":"network_error", ...}`), not an array. +/// The old code ran it through `unwrap_or_default()` and printed `[]`, which is +/// indistinguishable from "you have no DMs" — the caller cannot tell a dead +/// relay from an empty inbox. Fail loudly instead. +fn parse_query_events(resp: &str) -> Result, CliError> { + match serde_json::from_str::(resp) { + Ok(serde_json::Value::Array(events)) => Ok(events), + Ok(serde_json::Value::Object(obj)) => { + let msg = obj + .get("error") + .and_then(|v| v.as_str()) + .unwrap_or("unexpected object response"); + Err(CliError::Other(format!("relay query failed: {msg}"))) + } + Ok(_) => Err(CliError::Other( + "relay query returned a non-array response".into(), + )), + Err(e) => Err(CliError::Other(format!( + "relay query returned unparseable JSON: {e}" + ))), + } +} + +/// Project relay-signed kind:39000 group metadata events onto the DM list shape. +/// +/// Keeps an event only when it is a DM (`t=dm`), carries a non-empty `d` tag +/// (the channel uuid callers pass to `messages get/send --channel`), and lists +/// us among its participants. The `#p` filter already asks the relay for that +/// last condition; re-checking locally means a relay that ignores or widens the +/// filter cannot widen a consumer's poll set. +fn dms_from_group_metadata(events: &[serde_json::Value], my_pk: &str) -> Vec { + let mut out: Vec<(String, Vec, u64)> = Vec::new(); + for e in events { + let tags: Vec<&Vec> = e + .get("tags") + .and_then(|t| t.as_array()) + .map(|tags| tags.iter().filter_map(|t| t.as_array()).collect()) + .unwrap_or_default(); + + // Take the first tag with this name that actually carries a value: a + // valueless `["t"]` earlier in the list must not mask a later + // `["t","dm"]`, or a malformed tag would silently hide a real DM. + let tag_value = |name: &str| -> Option<&str> { + tags.iter() + .filter(|a| a.first().and_then(|v| v.as_str()) == Some(name)) + .find_map(|a| a.get(1).and_then(|v| v.as_str())) + }; + + if tag_value("t") != Some("dm") { + continue; + } + let dm_id = extract_d_tag(e); + if dm_id.is_empty() { + continue; + } + let participants: Vec = tags + .iter() + .filter(|a| a.first().and_then(|v| v.as_str()) == Some("p")) + .filter_map(|a| a.get(1).and_then(|v| v.as_str()).map(str::to_string)) + .collect(); + if !participants.iter().any(|p| p == my_pk) { + continue; + } + let created_at = e.get("created_at").and_then(|v| v.as_u64()).unwrap_or(0); + + // kind:39000 is addressable (replaceable per `d` tag), but a relay may + // still serve more than one revision. Keep the newest per dm_id. + match out.iter_mut().find(|(id, _, _)| id == &dm_id) { + Some(slot) if slot.2 < created_at => *slot = (dm_id, participants, created_at), + Some(_) => {} + None => out.push((dm_id, participants, created_at)), + } + } + out.sort_by_key(|(_, _, created_at)| std::cmp::Reverse(*created_at)); + out.into_iter() + .map(|(dm_id, participants, created_at)| { serde_json::json!({ "dm_id": dm_id, "participants": participants, - "created_at": e.get("created_at").and_then(|v| v.as_u64()).unwrap_or(0), + "created_at": created_at, }) }) - .collect(); - let output = serde_json::to_string(&dms).unwrap_or_default(); - println!("{output}"); - Ok(()) + .collect() } /// Open a DM with one or more users — sign and submit a kind:41010 event with a d-tag. @@ -134,3 +222,126 @@ pub async fn dispatch(cmd: crate::DmsCmd, client: &BuzzClient) -> Result<(), Cli DmsCmd::Hide { channel } => cmd_hide_dm(client, &channel).await, } } + +#[cfg(test)] +mod tests { + use super::{dm_list_filter, dms_from_group_metadata, parse_query_events}; + use serde_json::json; + + const ME: &str = "aa11"; + const THEM: &str = "bb22"; + + fn meta(d: &str, t: &str, ps: &[&str], created_at: u64) -> serde_json::Value { + let mut tags = vec![json!(["d", d]), json!(["name", "DM"]), json!(["t", t])]; + for p in ps { + tags.push(json!(["p", p])); + } + json!({"kind": 39000, "created_at": created_at, "tags": tags}) + } + + #[test] + fn lists_a_dm_we_participate_in() { + let events = vec![meta("chan-1", "dm", &[ME, THEM], 100)]; + let dms = dms_from_group_metadata(&events, ME); + assert_eq!(dms.len(), 1, "expected the DM, got {dms:?}"); + assert_eq!(dms[0]["dm_id"], "chan-1"); + assert_eq!(dms[0]["participants"], json!([ME, THEM])); + assert_eq!(dms[0]["created_at"], 100); + } + + #[test] + fn drops_non_dm_channel_types() { + // The whole defect class: a public channel must never be reported as a + // DM, because consumers skip the @-mention test on the DM path. + for t in ["stream", "forum", "", "DM"] { + let events = vec![meta("chan-1", t, &[ME, THEM], 100)]; + assert!( + dms_from_group_metadata(&events, ME).is_empty(), + "channel_type {t:?} was reported as a DM" + ); + } + } + + #[test] + fn drops_events_with_no_t_tag_at_all() { + let events = vec![json!({ + "kind": 39000, + "created_at": 100, + "tags": [["d", "chan-1"], ["p", ME], ["hidden"]], + })]; + assert!(dms_from_group_metadata(&events, ME).is_empty()); + } + + #[test] + fn drops_dms_we_are_not_a_participant_of() { + let events = vec![meta("chan-1", "dm", &[THEM, "cc33"], 100)]; + assert!( + dms_from_group_metadata(&events, ME).is_empty(), + "a relay that ignores the #p filter must not widen the poll set" + ); + } + + #[test] + fn drops_an_empty_or_missing_channel_uuid() { + let events = vec![ + meta("", "dm", &[ME, THEM], 100), + json!({"kind": 39000, "created_at": 100, "tags": [["t", "dm"], ["p", ME]]}), + ]; + assert!(dms_from_group_metadata(&events, ME).is_empty()); + } + + #[test] + fn keeps_the_newest_revision_per_channel_and_sorts_desc() { + let events = vec![ + meta("chan-1", "dm", &[ME, THEM], 100), + meta("chan-1", "dm", &[ME, THEM, "cc33"], 300), + meta("chan-2", "dm", &[ME, THEM], 200), + ]; + let dms = dms_from_group_metadata(&events, ME); + assert_eq!(dms.len(), 2, "duplicate revisions not collapsed: {dms:?}"); + assert_eq!(dms[0]["dm_id"], "chan-1"); + assert_eq!(dms[0]["created_at"], 300); + assert_eq!(dms[0]["participants"], json!([ME, THEM, "cc33"])); + assert_eq!(dms[1]["dm_id"], "chan-2"); + } + + #[test] + fn tolerates_malformed_tags() { + let events = vec![json!({ + "kind": 39000, + "created_at": 100, + "tags": [["d", "chan-1"], "not-an-array", ["t"], ["p"], ["t", "dm"], ["p", ME]], + })]; + let dms = dms_from_group_metadata(&events, ME); + assert_eq!(dms.len(), 1); + assert_eq!(dms[0]["participants"], json!([ME])); + } + + #[test] + fn a_relay_error_object_is_an_error_not_an_empty_list() { + let err = parse_query_events(r#"{"error":"network_error","url":"https://x/query"}"#) + .expect_err("a relay error must not read as an empty inbox"); + assert!( + err.to_string().contains("network_error"), + "error lost the relay's reason: {err}" + ); + assert!(parse_query_events("nope").is_err()); + assert!(parse_query_events("[]") + .expect("empty array is valid") + .is_empty()); + } + + #[test] + fn the_filter_asks_for_relay_signed_group_metadata_addressed_to_us() { + let f = dm_list_filter(ME, None); + assert_eq!( + f["kinds"], + json!([39000]), + "kind:41001 is emitted by no code path — querying it returns [] for every account" + ); + assert_eq!(f["#p"], json!([ME]), "must be scoped to our own pubkey"); + assert_eq!(f["limit"], 50); + assert_eq!(dm_list_filter(ME, Some(9))["limit"], 9); + assert_eq!(dm_list_filter(ME, Some(9999))["limit"], 200, "limit capped"); + } +} diff --git a/crates/buzz-core/src/kind.rs b/crates/buzz-core/src/kind.rs index 3c6f1d5913d..be9a05bf053 100644 --- a/crates/buzz-core/src/kind.rs +++ b/crates/buzz-core/src/kind.rs @@ -510,6 +510,13 @@ pub const KIND_DM_ADD_MEMBER: u32 = 41011; /// Hide DM from sidebar. pub const KIND_DM_HIDE: u32 = 41012; /// A new direct-message conversation was created. +/// +/// RESERVED, NOT EMITTED (DIVE-3560): no code path in this repo ever builds a +/// kind:41001 event. `handle_dm_open` creates the channel and then publishes +/// NIP-29 group discovery (kind:39000 with `t=dm` and one `p` tag per +/// participant) — that is the event to query when listing a user's DMs. +/// `buzz dms list` once queried this kind and returned an empty list for every +/// account on every relay. Do not consume it without adding an emitter first. pub const KIND_DM_CREATED: u32 = 41001; // Agent job protocol (43000–43999) diff --git a/crates/buzz-test-client/tests/e2e_nostr_interop.rs b/crates/buzz-test-client/tests/e2e_nostr_interop.rs index fce78776764..4d2f23734b8 100644 --- a/crates/buzz-test-client/tests/e2e_nostr_interop.rs +++ b/crates/buzz-test-client/tests/e2e_nostr_interop.rs @@ -752,6 +752,93 @@ async fn test_nip17_gift_wrap_recipient_receives() { client_b.disconnect().await.expect("disconnect B"); } +/// DIVE-3560: the query shape `buzz dms list` uses must actually return the DM. +/// +/// kind:41001 (`KIND_DM_CREATED`) is declared but never emitted by any code path, +/// so the old `{kinds:[41001], "#p":[me]}` query returned `[]` for every account. +/// This asserts the replacement — `{kinds:[39000], "#p":[me]}` — returns the DM +/// carrying the `t=dm` tag and the `d` tag consumers pass to `messages get`. +#[tokio::test] +#[ignore] +async fn test_dms_list_query_shape_returns_the_dm() { + let url = relay_url(); + let keys_a = Keys::generate(); + let keys_b = Keys::generate(); + let a_pubkey_hex = keys_a.public_key().to_hex(); + let b_pubkey_hex = keys_b.public_key().to_hex(); + + let channel_id = create_dm(&keys_a, &b_pubkey_hex).await; + + let mut client_a = BuzzTestClient::connect(&url, &keys_a) + .await + .expect("client A connect"); + + // Negative control: the kind the old implementation queried yields nothing, + // so an empty result here is the defect, not an artifact of the fixture. + let sid_old = sub_id("dms-list-41001"); + let old_filter = Filter::new().kind(Kind::Custom(41001)).custom_tag( + SingleLetterTag::lowercase(Alphabet::P), + a_pubkey_hex.as_str(), + ); + client_a + .subscribe(&sid_old, vec![old_filter]) + .await + .expect("subscribe 41001"); + let old_events = client_a + .collect_until_eose(&sid_old, Duration::from_secs(10)) + .await + .expect("41001 EOSE"); + assert!( + old_events.is_empty(), + "kind:41001 is emitted by nothing; got {} events — the premise of this fix changed", + old_events.len() + ); + + let sid_new = sub_id("dms-list-39000"); + let new_filter = Filter::new().kind(Kind::Custom(39000)).custom_tag( + SingleLetterTag::lowercase(Alphabet::P), + a_pubkey_hex.as_str(), + ); + client_a + .subscribe(&sid_new, vec![new_filter]) + .await + .expect("subscribe 39000"); + let new_events = client_a + .collect_until_eose(&sid_new, Duration::from_secs(10)) + .await + .expect("39000 EOSE"); + + let dm = new_events + .iter() + .find(|e| { + e.tags.iter().any(|t| { + let p = t.as_slice(); + p.len() >= 2 && p[0] == "d" && p[1] == channel_id + }) + }) + .unwrap_or_else(|| { + panic!("no kind:39000 for DM {channel_id} in a #p query addressed to its participant") + }); + + let has_dm_type = dm.tags.iter().any(|t| { + let p = t.as_slice(); + p.len() >= 2 && p[0] == "t" && p[1] == "dm" + }); + assert!( + has_dm_type, + "kind:39000 for a DM missing t=dm — the only relay-attested DM marker. tags: {:?}", + dm.tags + ); + + let lists_b = dm.tags.iter().any(|t| { + let p = t.as_slice(); + p.len() >= 2 && p[0] == "p" && p[1] == b_pubkey_hex + }); + assert!(lists_b, "kind:39000 for a DM missing the other participant"); + + client_a.disconnect().await.expect("disconnect"); +} + /// Create a DM via REST, then subscribe as a participant to verify discovery events. /// Verify: kind:39000 event received with `hidden` and `private` tags. /// Verify: kind:44100 membership notification received.