Skip to content

fix(relay): stop panic when reacting to a project root or comment - #4973

Open
BradGroux wants to merge 2 commits into
block:mainfrom
BradGroux:fix/relay-reaction-project-root-panic
Open

fix(relay): stop panic when reacting to a project root or comment#4973
BradGroux wants to merge 2 commits into
block:mainfrom
BradGroux:fix/relay-reaction-project-root-panic

Conversation

@BradGroux

@BradGroux BradGroux commented Aug 5, 2026

Copy link
Copy Markdown

What users saw

Tapping a reaction on an issue, pull request, or a comment on one could panic the relay's ingest worker. The reaction row was inserted before the panic, so the original publisher could see a failed request even though the event had already been persisted.

Why it happened

Reactions (NIP-25, kind 7) derive their channel scope from the target event. Project events such as issues and pull requests do not carry an h tag, so derive_reaction_channel returns NoChannel and channel_id is None.

The conformance-trace emission on the reaction path assumed channel_id was always present:

channel: channel_label(channel_id.expect("reaction path has channel")),

That assumption predates reactions on project events. Because persistence completes before the trace action is built, the panic happens after the database write.

What changed

Extracted the three-way (channel_id, was_inserted) match already used by the general message-write path into a shared write_trace_action helper:

  • (Some(ch), true) becomes WriteInsert
  • (Some(ch), false) becomes WriteDuplicate
  • (None, _) becomes WriteInsertGlobal

Both the reaction path and the message-write path now call this helper. Channel-less project reactions use the existing global-write trace vocabulary instead of unwrapping a missing channel.

How this was tested

Added four unit tests covering the complete helper matrix:

  • channel-less insert emits WriteInsertGlobal
  • channel-less duplicate emits WriteInsertGlobal
  • channel-bearing insert emits WriteInsert
  • channel-bearing duplicate emits WriteDuplicate

The channel-less insert is the exact affected path. A normal retry of the same active reaction exits earlier through ReactionEventInsertOutcome::Duplicate; the channel-less duplicate case is defensive coverage for the helper contract rather than a claim about that retry path.

Validation at f611f96e7:

./bin/cargo fmt --all -- --check
cargo test -p buzz-relay --lib handlers::ingest::tests
cargo clippy -p buzz-relay --all-targets -- -D warnings

All 164 ingest tests passed, and formatting and strict Clippy checks completed successfully.

Scope and non-goals

  • Does not change reaction persistence or channel derivation.
  • Does not change duplicate-reaction behavior.
  • Only replaces the unsafe trace-action construction and shares the mapping with the message path.

Closes #4936.

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 block#4936

Co-authored-by: Brad Groux <bradgroux@hotmail.com>
Signed-off-by: Brad Groux <bradgroux@hotmail.com>
Signed-off-by: npub17q2gdupkvswvk5kprwc7plergm4gn295uw6fe4mjyjv53ahuhtnq02jd3f <f01486f036641ccb52c11bb1e0ff2346ea89a8b4e3b49cd772249948f6fcbae6@digitalmeld.communities.buzz.xyz>
@BradGroux
BradGroux requested a review from a team as a code owner August 5, 2026 22:41
Co-authored-by: Brad Groux <bradgroux@hotmail.com>
Signed-off-by: Brad Groux <bradgroux@hotmail.com>
Signed-off-by: Brad Groux <3053586+BradGroux@users.noreply.github.com>
@BradGroux

Copy link
Copy Markdown
Author

Review update: I traced the insert and duplicate control flow, corrected the PR description, and pushed the required formatting cleanup. The functional fix remains narrowly scoped to trace-action construction. A normal retry of the same active reaction exits earlier through ReactionEventInsertOutcome::Duplicate; it does not re-enter the trace helper, so the previous claim that every retry panicked was inaccurate.

At f611f96e7, rustfmt, all 164 relay ingest tests, and strict Clippy pass. I also started the repository-wide test recipe: its non-integration suites advanced successfully, but the integration-service gate could not start because no Docker daemon was available in the review environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

relay: a reaction on a project root (issue/PR) panics the ingest worker

2 participants