From 67045ebbfbca087d7d5f51978174d94ec86c8b94 Mon Sep 17 00:00:00 2001 From: Duncan Date: Mon, 17 Aug 2026 20:22:33 -0400 Subject: [PATCH] fix(workflows): share NIP-10 marker parser for reply validity parity event_is_reply accepted any value in the reply marker's event-id slot, but ingest only honors a marker when the id is exactly 64 ASCII-hex chars. A tag like ["e","bad","","reply"] is stored top-level by ingest yet read as a reply by the workflow, so a trigger_is_reply == false workflow wrongly skipped it. Extract the root/reply marker parse into a shared buzz_core::nip10::parse_thread_markers, gated on the same 64-hex validity. Both the relay ingest resolver and the workflow trigger_is_reply predicate now call it, so a malformed id can no longer diverge the two. Also replaces an argument-free format! in the live e2e probe with a plain string. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- crates/buzz-core/src/lib.rs | 2 + crates/buzz-core/src/nip10.rs | 118 +++++++++++++++++++++ crates/buzz-relay/src/handlers/ingest.rs | 20 +--- crates/buzz-test-client/tests/e2e_relay.rs | 5 +- crates/buzz-workflow/src/lib.rs | 66 ++++++++++-- 5 files changed, 182 insertions(+), 29 deletions(-) create mode 100644 crates/buzz-core/src/nip10.rs diff --git a/crates/buzz-core/src/lib.rs b/crates/buzz-core/src/lib.rs index 7424915c8..36dc772da 100644 --- a/crates/buzz-core/src/lib.rs +++ b/crates/buzz-core/src/lib.rs @@ -26,6 +26,8 @@ pub mod invite; pub mod kind; /// Network utilities — SSRF-safe IP classification. pub mod network; +/// NIP-10 thread-marker parsing — shared `root`/`reply` marker resolver. +pub mod nip10; /// Agent observer frame helpers. pub mod observer; /// NIP-AB device pairing — crypto primitives, message types, and errors. diff --git a/crates/buzz-core/src/nip10.rs b/crates/buzz-core/src/nip10.rs new file mode 100644 index 000000000..fbeb3eddc --- /dev/null +++ b/crates/buzz-core/src/nip10.rs @@ -0,0 +1,118 @@ +//! Shared NIP-10 thread-marker parsing. +//! +//! One parser for the `root`/`reply` markers on an event's `e` tags, so every +//! consumer reads ancestry the same way. The relay ingest resolver +//! (`resolve_nip10_thread_meta`) and the workflow `trigger_is_reply` predicate +//! both call this — a second hand-rolled copy is exactly how the two drifted on +//! marker semantics and on id-validity. +//! +//! Validity mirrors ingest: a marker counts only when its event id is exactly +//! 64 ASCII-hex characters. A malformed id (e.g. `["e","bad","","reply"]`) is +//! ignored, never treated as a thread link. + +/// The `root` and `reply` event ids parsed from an event's NIP-10 `e` tags. +/// +/// Each is `Some(id_hex)` only when a marker of that kind carried a valid +/// 64-hex event id. The last valid occurrence of each marker wins, matching +/// the relay resolver's single-pass overwrite. +#[derive(Debug, Default, Clone, PartialEq, Eq)] +pub struct ThreadMarkers { + /// Event id from a valid `["e", <64-hex>, , "root"]` tag. + pub root: Option, + /// Event id from a valid `["e", <64-hex>, , "reply"]` tag. + pub reply: Option, +} + +/// Return true when `id` is exactly 64 ASCII-hex characters — the shape a +/// Nostr event id must have to be a real thread link. +fn is_event_id_hex(id: &str) -> bool { + id.len() == 64 && id.chars().all(|c| c.is_ascii_hexdigit()) +} + +/// Parse the NIP-10 `root`/`reply` markers from an event's tags. +/// +/// Only `e` tags with a marker (`parts.len() >= 4`) and a valid 64-hex event id +/// are considered; everything else is ignored. +pub fn parse_thread_markers(tags: &nostr::Tags) -> ThreadMarkers { + let mut markers = ThreadMarkers::default(); + for tag in tags.iter() { + let parts = tag.as_slice(); + if parts.len() >= 4 && parts[0] == "e" && is_event_id_hex(&parts[1]) { + match parts[3].as_str() { + "root" => markers.root = Some(parts[1].to_string()), + "reply" => markers.reply = Some(parts[1].to_string()), + _ => {} + } + } + } + markers +} + +#[cfg(test)] +mod tests { + use super::*; + use nostr::{EventBuilder, Keys, Kind, Tag}; + + fn markers_for(tags: Vec) -> ThreadMarkers { + let event = EventBuilder::new(Kind::Custom(9), "") + .tags(tags) + .sign_with_keys(&Keys::generate()) + .expect("sign"); + parse_thread_markers(&event.tags) + } + + fn id() -> String { + "a".repeat(64) + } + + #[test] + fn no_e_tags_yields_no_markers() { + assert_eq!(markers_for(vec![]), ThreadMarkers::default()); + } + + #[test] + fn root_and_reply_both_parsed() { + let m = markers_for(vec![ + Tag::parse(["e", &id(), "", "root"]).unwrap(), + Tag::parse(["e", &"b".repeat(64), "", "reply"]).unwrap(), + ]); + assert_eq!(m.root.as_deref(), Some(id().as_str())); + assert_eq!(m.reply.as_deref(), Some("b".repeat(64).as_str())); + } + + #[test] + fn reply_only_marker_parsed() { + let m = markers_for(vec![Tag::parse(["e", &id(), "", "reply"]).unwrap()]); + assert_eq!(m.reply.as_deref(), Some(id().as_str())); + assert!(m.root.is_none()); + } + + #[test] + fn bare_e_tag_without_marker_is_ignored() { + let m = markers_for(vec![Tag::parse(["e", &id()]).unwrap()]); + assert_eq!(m, ThreadMarkers::default()); + } + + #[test] + fn malformed_id_is_ignored_for_both_markers() { + // Ingest gates the marker on a valid 64-hex id; a malformed id is not a + // thread link, so neither marker is set. + let m = markers_for(vec![ + Tag::parse(["e", "bad", "", "reply"]).unwrap(), + Tag::parse(["e", "also-bad", "", "root"]).unwrap(), + ]); + assert_eq!(m, ThreadMarkers::default()); + } + + #[test] + fn valid_root_with_malformed_reply_is_top_level() { + // A valid root but a malformed reply id: reply is ignored, so this is + // top-level to ingest (root-only) and must be so here too. + let m = markers_for(vec![ + Tag::parse(["e", &id(), "", "root"]).unwrap(), + Tag::parse(["e", "bad", "", "reply"]).unwrap(), + ]); + assert_eq!(m.root.as_deref(), Some(id().as_str())); + assert!(m.reply.is_none()); + } +} diff --git a/crates/buzz-relay/src/handlers/ingest.rs b/crates/buzz-relay/src/handlers/ingest.rs index 1de9824d2..6acfd1fc2 100644 --- a/crates/buzz-relay/src/handlers/ingest.rs +++ b/crates/buzz-relay/src/handlers/ingest.rs @@ -724,23 +724,9 @@ pub(crate) async fn resolve_nip10_thread_meta( channel_id: Uuid, state: &AppState, ) -> Result, String> { - let mut root_hex: Option = None; - let mut reply_hex: Option = None; - - for tag in event.tags.iter() { - let parts = tag.as_slice(); - if parts.len() >= 4 && parts[0] == "e" { - let hex_val = &parts[1]; - let marker = &parts[3]; - if hex_val.len() == 64 && hex_val.chars().all(|c| c.is_ascii_hexdigit()) { - match marker.as_str() { - "root" => root_hex = Some(hex_val.to_string()), - "reply" => reply_hex = Some(hex_val.to_string()), - _ => {} - } - } - } - } + let markers = buzz_core::nip10::parse_thread_markers(&event.tags); + let root_hex = markers.root; + let reply_hex = markers.reply; if root_hex.is_none() && reply_hex.is_none() { return Ok(None); diff --git a/crates/buzz-test-client/tests/e2e_relay.rs b/crates/buzz-test-client/tests/e2e_relay.rs index 18ea05825..b119d2677 100644 --- a/crates/buzz-test-client/tests/e2e_relay.rs +++ b/crates/buzz-test-client/tests/e2e_relay.rs @@ -2679,8 +2679,7 @@ async fn test_workflow_reply_in_thread_pushes_live_thread_summary() { // A message_posted workflow that replies in-thread, but only to NEW // top-level messages (`trigger_is_reply == false`) — so it cannot recurse // on the reply it just posted. - let yaml = format!( - "name: reply-bot\n\ + let yaml = "name: reply-bot\n\ description: F3 live probe\n\ trigger:\n\ \x20 on: message_posted\n\ @@ -2691,7 +2690,7 @@ async fn test_workflow_reply_in_thread_pushes_live_thread_summary() { \x20 action: send_message\n\ \x20 text: \"auto-reply\"\n\ \x20 reply_in_thread: true\n" - ); + .to_string(); let def = EventBuilder::new(Kind::Custom(30620), yaml) .tags([ Tag::parse(["d", &Uuid::new_v4().to_string()]).unwrap(), diff --git a/crates/buzz-workflow/src/lib.rs b/crates/buzz-workflow/src/lib.rs index 3bc0e409c..eccfe06d0 100644 --- a/crates/buzz-workflow/src/lib.rs +++ b/crates/buzz-workflow/src/lib.rs @@ -1021,16 +1021,17 @@ pub fn build_trigger_context(event: &buzz_core::StoredEvent) -> executor::Trigge } } -/// True when an event is a threaded reply — it carries a NIP-10 `e` tag marked -/// `reply`. A `root` marker alone is not enough: the ingest resolver treats -/// `(root=Some, reply=None)` as top-level (see `resolve_nip10_thread_meta`), so -/// a `message_posted` filter of `trigger_is_reply == false` fires on those and -/// on messages with no NIP-10 markers at all. +/// True when an event is a threaded reply — it carries a valid NIP-10 `reply` +/// marker. Delegates to the shared [`buzz_core::nip10`] parser so this stays in +/// lockstep with ingest's `resolve_nip10_thread_meta`: a `root` marker alone is +/// top-level, and a marker with a malformed (non-64-hex) event id is ignored by +/// ingest, so it must not flip `trigger_is_reply` either — else a +/// `trigger_is_reply == false` workflow would skip a message ingest stored as a +/// new top-level post. fn event_is_reply(event: &nostr::Event) -> bool { - event.tags.iter().any(|tag| { - let parts = tag.as_slice(); - parts.len() >= 4 && parts[0] == "e" && parts[3] == "reply" - }) + buzz_core::nip10::parse_thread_markers(&event.tags) + .reply + .is_some() } /// Pure authority decision for [`WorkflowEngine::check_owner_authority`]. @@ -1670,6 +1671,53 @@ steps: assert!(!ctx.is_reply, "unmarked e-tag must not count as a reply"); } + #[test] + fn build_trigger_context_is_reply_false_for_malformed_reply_id() { + // Ingest gates a marker on a valid 64-hex event id; a malformed reply + // id is not a thread link, so ingest stores the event top-level. The + // predicate must agree, or `trigger_is_reply == false` would skip it. + use nostr::{EventBuilder, Keys, Kind, Tag}; + use uuid::Uuid; + let keys = Keys::generate(); + let event = EventBuilder::new(Kind::Custom(9), "malformed reply marker") + .tags([Tag::parse(["e", "bad", "", "reply"]).expect("reply tag")]) + .sign_with_keys(&keys) + .expect("sign"); + let stored = buzz_core::StoredEvent::new(event, Some(Uuid::new_v4())); + let ctx = build_trigger_context(&stored); + assert!( + !ctx.is_reply, + "a malformed reply id is ignored by ingest, so it is top-level" + ); + } + + #[test] + fn build_trigger_context_is_reply_false_for_valid_root_malformed_reply() { + // A valid `root` marker but a malformed `reply` id: ingest ignores the + // reply and stores the event as root-only, i.e. top-level. The predicate + // must not flip to reply on the malformed marker. + use nostr::{EventBuilder, Keys, Kind, Tag}; + use uuid::Uuid; + let root = Keys::generate(); + let root_event = EventBuilder::new(Kind::Custom(9), "root") + .sign_with_keys(&root) + .expect("sign root"); + let keys = Keys::generate(); + let event = EventBuilder::new(Kind::Custom(9), "valid root, malformed reply") + .tags([ + Tag::parse(["e", &root_event.id.to_hex(), "", "root"]).expect("root tag"), + Tag::parse(["e", "bad", "", "reply"]).expect("reply tag"), + ]) + .sign_with_keys(&keys) + .expect("sign"); + let stored = buzz_core::StoredEvent::new(event, Some(Uuid::new_v4())); + let ctx = build_trigger_context(&stored); + assert!( + !ctx.is_reply, + "a valid root with a malformed reply id is top-level to ingest" + ); + } + #[test] fn build_trigger_context_reaction_event() { let (stored, target_id_hex) = make_reaction_event();