From 7c9f6ed9bc64f8dc40dbd0d4b0bf81c1949273c7 Mon Sep 17 00:00:00 2001 From: lodar Date: Thu, 20 Aug 2026 02:26:41 +0000 Subject: [PATCH] fix(cli): `dms list` must query the relay-emitted kind:39000, not kind: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 reads as "no DMs exist" instead of "my predicate matched nothing". 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 rather than a client-supplied name/membership heuristic, 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. Adds an end-to-end interop arm in buzz-test-client that pins the wire kind, so the relay and the CLI cannot drift apart again. Signed-off-by: lodar --- 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..177de05fcd4 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: 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..c9925c83eed 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: 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..d0e39641bb7 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"); } +/// 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.