From e59afa2dc38843c937d364e140e047924d31c2f1 Mon Sep 17 00:00:00 2001 From: npub17q2gdupkvswvk5kprwc7plergm4gn295uw6fe4mjyjv53ahuhtnq02jd3f Date: Wed, 5 Aug 2026 17:40:57 -0500 Subject: [PATCH 1/2] fix(relay): stop panic when reacting to a project root or comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A kind:7 reaction targeting a project event (kind 1621 issue, 1618 PR, or a kind-1 comment on one) carries no h tag, so derive_reaction_channel returns NoChannel and channel_id is None. The reaction path's conformance trace emission asserted channel_id was always Some via expect(), which panicked the tokio worker. The reaction row was already stored before the panic, so retries hit the duplicate branch and panicked again — head-of-line blocking for durable-queue clients like buzz-acp. Extract the three-way (channel_id, was_inserted) match into a shared write_trace_action helper used by both the reaction path and the general message-write path, so the two seams cannot diverge. Add regression tests for all four combinations. Refs #4936 Co-authored-by: Brad Groux Signed-off-by: Brad Groux Signed-off-by: npub17q2gdupkvswvk5kprwc7plergm4gn295uw6fe4mjyjv53ahuhtnq02jd3f --- crates/buzz-relay/src/handlers/ingest.rs | 142 ++++++++++++++++------- 1 file changed, 102 insertions(+), 40 deletions(-) diff --git a/crates/buzz-relay/src/handlers/ingest.rs b/crates/buzz-relay/src/handlers/ingest.rs index cd9f20b5f4..3ac3bfef76 100644 --- a/crates/buzz-relay/src/handlers/ingest.rs +++ b/crates/buzz-relay/src/handlers/ingest.rs @@ -195,6 +195,42 @@ fn emit_product_feedback_success( ); } +/// Build the abstract write [`TraceAction`] for a persist outcome. +/// +/// `channel_id.is_some()` distinguishes channel-bearing writes +/// (Insert/Duplicate) from channel-less writes (InsertGlobal); +/// `was_inserted` distinguishes accepted-new writes from no-op-on-conflict +/// duplicates. This mirrors the spec's three-way split at the `WriteInsert` +/// (line 514), `WriteInsertGlobal` (line 559), and `WriteDuplicate` +/// (line 606) seams. +/// +/// Shared by the reaction path and the general message-write path so the +/// two seams cannot diverge — see #4936 for the bug that motivated +/// extracting this helper. +fn write_trace_action( + event: &Event, + channel_id: Option, + was_inserted: bool, +) -> TraceAction { + let claimed = claimed_community_from_event(event); + match (channel_id, was_inserted) { + (Some(ch), true) => TraceAction::WriteInsert { + msg_id: msg_id_label(event.id.as_bytes()), + channel: channel_label(ch), + claimed_community: claimed, + }, + (Some(ch), false) => TraceAction::WriteDuplicate { + msg_id: msg_id_label(event.id.as_bytes()), + channel: channel_label(ch), + claimed_community: claimed, + }, + (None, _) => TraceAction::WriteInsertGlobal { + msg_id: msg_id_label(event.id.as_bytes()), + claimed_community: claimed, + }, + } +} + /// Increment the rejection counter with a bounded reason and transport label. /// /// Shared by the WS `EVENT` handler and the HTTP `POST /events` handler so @@ -2764,25 +2800,14 @@ async fn ingest_event_inner( }; let pubkey_hex = auth.pubkey().to_hex(); - // Spec WriteInsert (line 514) / WriteDuplicate (line 606): emit - // the abstract write action. The persist API returns - // `was_inserted` (true → Insert, false → Duplicate). This branch - // is the reaction path; channel_id is always Some here, so - // WriteInsertGlobal does not apply. - let claimed = claimed_community_from_event(&event); - let action = if was_inserted { - TraceAction::WriteInsert { - msg_id: msg_id_label(event.id.as_bytes()), - channel: channel_label(channel_id.expect("reaction path has channel")), - claimed_community: claimed, - } - } else { - TraceAction::WriteDuplicate { - msg_id: msg_id_label(event.id.as_bytes()), - channel: channel_label(channel_id.expect("reaction path has channel")), - claimed_community: claimed, - } - }; + // Spec WriteInsert (line 514) / WriteInsertGlobal (line 559) / + // WriteDuplicate (line 606): emit the abstract write action. A + // reaction on a project event (kind 1621 issue, 1618 PR, or a + // kind-1 comment on one) carries no `h` tag, so + // `derive_reaction_channel` returns `NoChannel` and `channel_id` + // is `None`. The shared `write_trace_action` helper mirrors the + // message write path's three-way split — see #4936. + let action = write_trace_action(&event, channel_id, was_inserted); emit(tracer, action, state_for_request(tenant, auth.pubkey())); dispatch_persistent_event( tenant, @@ -2900,31 +2925,12 @@ async fn ingest_event_inner( let pubkey_hex = auth.pubkey().to_hex(); // Spec WriteInsert (line 514) / WriteInsertGlobal (line 559) / // WriteDuplicate (line 606): emit the abstract write at the trailing - // dispatch site. `channel_id.is_some()` distinguishes channel-bearing - // (Insert/Duplicate) from channel-less (InsertGlobal); `was_inserted` - // distinguishes accepted-new (Insert/Global) from no-op-on-conflict - // (Duplicate). The WriteInsertGlobal duplicate case is not modeled + // dispatch site. The WriteInsertGlobal duplicate case is not modeled // separately in the spec (channel-less duplicates collapse to the // same observation shape as channel-less inserts at this seam); // see docs/spec/MultiTenantRelay.tla lines 559-595. { - let claimed = claimed_community_from_event(&event); - let action = match (channel_id, was_inserted) { - (Some(ch), true) => TraceAction::WriteInsert { - msg_id: msg_id_label(event.id.as_bytes()), - channel: channel_label(ch), - claimed_community: claimed, - }, - (Some(ch), false) => TraceAction::WriteDuplicate { - msg_id: msg_id_label(event.id.as_bytes()), - channel: channel_label(ch), - claimed_community: claimed, - }, - (None, _) => TraceAction::WriteInsertGlobal { - msg_id: msg_id_label(event.id.as_bytes()), - claimed_community: claimed, - }, - }; + let action = write_trace_action(&event, channel_id, was_inserted); emit(tracer, action, state_for_request(tenant, auth.pubkey())); } dispatch_persistent_event( @@ -4919,4 +4925,60 @@ mod tests { Some(&1) ); } + + // --- write_trace_action regression tests (#4936) --- + // A reaction on a project event (kind 1621 issue, 1618 PR, or a + // kind-1 comment on one) carries no `h` tag, so `derive_reaction_channel` + // returns `NoChannel` and `channel_id` is `None`. The old code + // panicked on `channel_id.expect("reaction path has channel")`; the + // shared `write_trace_action` helper must emit `WriteInsertGlobal` + // instead. + + fn make_reaction_event() -> Event { + EventBuilder::new(Kind::Custom(KIND_REACTION as u16), "+") + .sign_with_keys(&nostr::Keys::generate()) + .expect("sign reaction") + } + + #[test] + fn write_trace_action_channel_less_insert_emits_write_insert_global() { + let event = make_reaction_event(); + let action = write_trace_action(&event, None, true); + assert!( + matches!(action, TraceAction::WriteInsertGlobal { .. }), + "channel-less insert must emit WriteInsertGlobal, got {action:?}" + ); + } + + #[test] + fn write_trace_action_channel_less_duplicate_emits_write_insert_global() { + let event = make_reaction_event(); + let action = write_trace_action(&event, None, false); + assert!( + matches!(action, TraceAction::WriteInsertGlobal { .. }), + "channel-less duplicate must emit WriteInsertGlobal, got {action:?}" + ); + } + + #[test] + fn write_trace_action_channel_bearing_insert_emits_write_insert() { + let event = make_reaction_event(); + let ch = Uuid::new_v4(); + let action = write_trace_action(&event, Some(ch), true); + assert!( + matches!(action, TraceAction::WriteInsert { .. }), + "channel-bearing insert must emit WriteInsert, got {action:?}" + ); + } + + #[test] + fn write_trace_action_channel_bearing_duplicate_emits_write_duplicate() { + let event = make_reaction_event(); + let ch = Uuid::new_v4(); + let action = write_trace_action(&event, Some(ch), false); + assert!( + matches!(action, TraceAction::WriteDuplicate { .. }), + "channel-bearing duplicate must emit WriteDuplicate, got {action:?}" + ); + } } From f611f96e7e51028b0f08114cd4501624f587abca Mon Sep 17 00:00:00 2001 From: Brad Groux <3053586+BradGroux@users.noreply.github.com> Date: Wed, 5 Aug 2026 18:26:39 -0500 Subject: [PATCH 2/2] chore(relay): format reaction trace helper Co-authored-by: Brad Groux Signed-off-by: Brad Groux Signed-off-by: Brad Groux <3053586+BradGroux@users.noreply.github.com> --- crates/buzz-relay/src/handlers/ingest.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/crates/buzz-relay/src/handlers/ingest.rs b/crates/buzz-relay/src/handlers/ingest.rs index 3ac3bfef76..fdba396a91 100644 --- a/crates/buzz-relay/src/handlers/ingest.rs +++ b/crates/buzz-relay/src/handlers/ingest.rs @@ -207,11 +207,7 @@ fn emit_product_feedback_success( /// Shared by the reaction path and the general message-write path so the /// two seams cannot diverge — see #4936 for the bug that motivated /// extracting this helper. -fn write_trace_action( - event: &Event, - channel_id: Option, - was_inserted: bool, -) -> TraceAction { +fn write_trace_action(event: &Event, channel_id: Option, was_inserted: bool) -> TraceAction { let claimed = claimed_community_from_event(event); match (channel_id, was_inserted) { (Some(ch), true) => TraceAction::WriteInsert {