Skip to content

fix(cli): fail a thread read whose event is in another channel - #5923

Open
Chessing234 wants to merge 4 commits into
block:mainfrom
Chessing234:fix/cli-thread-channel-mismatch
Open

fix(cli): fail a thread read whose event is in another channel#5923
Chessing234 wants to merge 4 commits into
block:mainfrom
Chessing234:fix/cli-thread-channel-mismatch

Conversation

@Chessing234

Copy link
Copy Markdown
Contributor

Fixes #5800.

messages thread ORs two filters: replies scoped by #h and #e, and the root selected by ids alone. A --channel that does not match the event therefore returns the root and none of its replies, and exits 0 — which is byte-for-byte what a thread nobody answered looks like. The reporter's case is the dangerous one: an operator reads "no replies" and re-dispatches work that was already answered in the channel the event actually lives in.

Writes already refuse the same mismatch (ingest.rs: "parent event belongs to a different channel"); this makes the read agree, and names both UUIDs so the fix is a copy-paste away.

A root the relay did not return is left alone, so a genuinely missing event still prints an empty array rather than turning into an error, and a root with no h tag (nothing in this path publishes one, but the guard is cheap) passes through.

Verified locally: cargo test -p buzz-cli --lib (354 passed, 5 new), cargo clippy -p buzz-cli --all-targets clean, cargo fmt --all --check clean. Not run: a live relay — the shape of the two filters and the h-tag placement are read from messages.rs and the issue's own transcript.

`messages thread` ORs a reply filter scoped by `#h` with a root filter that
selects by id alone. A `--channel` that does not match the event therefore
returns the root and none of its replies, and exits 0 — indistinguishable
from a thread nobody answered, which is a conclusion an operator will act
on.

Reject the mismatch with the channel the event is actually in. Writes
already refuse the same thing ("parent event belongs to a different
channel"); this makes the read agree. A root the relay did not return is
left alone, so a genuinely missing event still prints nothing.

Signed-off-by: Taksh <takshkothari09@gmail.com>
A root in the requested channel passes, a root from another channel is
rejected with both UUIDs named, a reply's own h tag does not decide the
outcome, and a missing or untagged root still passes through.

Signed-off-by: Taksh <takshkothari09@gmail.com>
@Chessing234
Chessing234 requested a review from a team as a code owner August 15, 2026 03:51

@themiguelamador themiguelamador left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new channel check still has two correctness gaps:

  • It compares UUID text byte-for-byte, so an uppercase but valid --channel value falsely mismatches the lowercase UUID stored in the event.
  • A returned root with no valid h tag passes through, even though the command cannot establish that the root belongs to the requested channel. (A genuinely missing root still needs to preserve the existing empty-result behavior.)

I fixed both in commit c9ae3c674: channel IDs are parsed and compared as UUIDs, event tag extraction follows the relay's first-parseable-h behavior, and a returned root without a valid channel now fails closed.

Verified with all 356 buzz-cli library tests, strict Clippy, rustfmt, and git diff --check.

@Chessing234

Copy link
Copy Markdown
Contributor Author

Thanks — both fixed, in e8e17f8 and 45f7138. Complear/buzz 404s for me (gh api repos/Complear/buzz, and fetching c9ae3c674, both fail), so these are written from your description.

UUID comparison. event_channel_id now returns a parsed Uuid and the comparison is by value. It mirrors buzz_sdk::extract_channel_id — first parseable h tag rather than first h tag, so a junk tag ahead of the real one does not hide it — with a comment saying why the walk is duplicated (that helper takes nostr::Event; this path holds serde_json::Value).

One thing worth flagging: comparing by value would have been half a fix, so I went one step further than your description. The uppercase --channel is also wrong in the #h filter, not just in the comparison — h tags are lowercase hyphenated on the wire (NIP-PL h_grammar: "uuid-v4-lowercase", crates/buzz-relay/src/nip11.rs:215), so an uppercase argument matches no replies at the relay. Fixing only the comparison would have converted the false error into the silent "nobody answered" thread this PR exists to remove. So cmd_get_thread now canonicalizes once (parse_uuidhyphenated()) and uses that for the filter and the check. Same for the {braced} and unhyphenated spellings Uuid::parse_str accepts — there is a test over all three.

Root with no valid h tag. Now fails closed with event <id> carries no readable channel (h) tag, …, covering both no h tag at all and an h tag that is not a UUID. A root the relay did not return is still passed through, so a genuinely missing event keeps printing an empty array rather than erroring — the test for that is unchanged.

Verified on the pushed head: cargo test -p buzz-cli --lib 358 passed (was 354; 4 net new), cargo clippy -p buzz-cli --all-targets -- -D warnings clean, cargo fmt --all -- --check clean, git diff --check clean. Still not run: a live relay.

The review points out the check compared UUID text byte-for-byte, so an
uppercase but valid --channel falsely mismatched the lowercase UUID in the
event's h tag.

`event_channel_id` now returns a parsed `Uuid`, mirroring
`buzz_sdk::extract_channel_id` — first *parseable* h tag, so a junk tag
ahead of the real one does not hide it — and the comparison is by value.

The argument is also canonicalized before it reaches the relay. Comparing by
value alone would have been half a fix: `Uuid::parse_str` accepts uppercase,
braced and unhyphenated spellings, but h tags are lowercase hyphenated on the
wire (NIP-PL `h_grammar: uuid-v4-lowercase`), so a non-canonical --channel
still matched no replies in the `#h` filter — turning the false error back
into the silent "nobody answered" thread this PR exists to remove.

357 buzz-cli lib tests pass.

Signed-off-by: Taksh <takshkothari09@gmail.com>
The review points out a returned root with no valid h tag passed through,
even though the command cannot then establish that the root belongs to the
requested channel — printing it with no replies is the same silence the
check exists to remove.

It is now a usage error naming the event. A root the relay did not return at
all still passes through, so a genuinely missing event keeps printing an
empty array rather than turning into an error.

358 buzz-cli lib tests pass; clippy --all-targets -D warnings and fmt clean.

Signed-off-by: Taksh <takshkothari09@gmail.com>
@Chessing234
Chessing234 force-pushed the fix/cli-thread-channel-mismatch branch from 45f7138 to 7300e69 Compare August 16, 2026 16:45
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.

buzz messages thread silently returns partial result for wrong channel

2 participants