fix(cli): normalize the event id a reaction query filters on - #5984
Open
Chessing234 wants to merge 7 commits into
Open
fix(cli): normalize the event id a reaction query filters on#5984Chessing234 wants to merge 7 commits into
Chessing234 wants to merge 7 commits into
Conversation
`validate_hex64` is documented as validating a "64-character lowercase hex string" but accepts either case — `is_ascii_hexdigit` does — and every caller passes the string through unchanged. That is harmless where the value reaches a typed field: `ids` and `authors` are parsed into `EventId`/`PublicKey` on the relay, and `buzz-sdk`'s builders already lowercase before they write a tag. It is not harmless in a NIP-01 generic tag filter, which is compared as a string. Add `parse_hex64`, which validates and returns the value lowercased, and correct the older function's doc to say what it does and when each is right. No call sites yet. Signed-off-by: Taksh <takshkothari09@gmail.com>
`buzz messages thread --event <ID>` validated the hex and used it verbatim in two filters. `ids` is parsed into a typed `EventId` on the relay, so case there is harmless — but `#e` is a NIP-01 generic tag filter, matched by string comparison against a tag every event writes lowercase. Paste an id in uppercase and the command returns the root event and none of its replies: a partial thread, printed as if it were the whole thing. Lowercase the id once, at the boundary, and lift the two filters into `thread_filters` so the shape is pinned by a test instead of only by the network. Signed-off-by: Taksh <takshkothari09@gmail.com>
`buzz issues assign` looks up the issue's prior assignment events with an `#e` filter before deciding what to publish. Same string-compared tag filter as the thread query: an uppercase issue id finds no prior assignments, so the command builds its note from an empty history — reporting an assignment against a state it never actually read. The `ids` filter beside it is typed and would still resolve the issue itself, which is what makes the failure quiet. Signed-off-by: Taksh <takshkothari09@gmail.com>
Three commands build this coordinate by hand with `format!`, each after a `validate_hex64` that does not normalize. `GitRepoCoord::to_a_tag_value` — what every published `a` tag is actually built from — lowercases the owner and leaves the repo `d`-tag exactly as given. Put that rule in one place, next to the validators the callers already use. No call sites yet. Signed-off-by: Taksh <takshkothari09@gmail.com>
`buzz patches list`, `buzz issues list` and `buzz pr list` each built
`30617:{repo_owner}:{repo_id}` straight from the argument. Published events
build the same coordinate through `GitRepoCoord::to_a_tag_value`, which
lowercases the owner.
`#a` is a generic tag filter, compared as a raw string, so an owner pasted in
uppercase produces a coordinate that matches nothing: all three commands
print an empty list for a repository that has patches, issues and pull
requests. There is no error — an empty result and a wrong result look
identical here.
Route the three through `repo_coord_a_value` so the query is spelled the way
the tag is written.
Signed-off-by: Taksh <takshkothari09@gmail.com>
`buzz reactions remove` finds the reaction to retract by querying kind:7
events with `#e` set to the target id, then reports what it found. With an id
pasted in uppercase the query returns nothing and the command answers
no reaction with emoji '👍' found for your pubkey on event <ID>
which is a statement about the world, and it is false. The reaction is there;
the query spelled its target differently than the tag does. `reactions get`
prints an empty list for the same reason.
`cmd_add_reaction` normalizes too — it parses the id into a typed `EventId`
so the published tag was already lowercase, but leaving one of the three
spellings unnormalized is how this drifts back.
This is the last raw tag filter in the CLI built from an unnormalized
argument. Every value under `#e`/`#p`/`#a` in `crates/buzz-cli/src/commands`
now comes from a typed key, from `parse_hex64`, or from
`repo_coord_a_value`.
Signed-off-by: Taksh <takshkothari09@gmail.com>
Both reaction commands built the same kind:7 filter inline, once with an author narrowing and once without, so nothing but the network could tell you whether the `#e` value was spelled right. Lift them into `reaction_filter` and assert what goes in: the normalized id under `#e`, and `authors` present only when narrowing to the caller. No behaviour change. Signed-off-by: Taksh <takshkothari09@gmail.com>
themiguelamador
approved these changes
Aug 16, 2026
themiguelamador
left a comment
themiguelamador
left a comment
There was a problem hiding this comment.
Reviewed the full seven-commit stack against current main. The canonicalization matches the relay generic-tag comparison and the SDK repository-coordinate encoding, and the changed call sites preserve typed ids/authors behavior. No findings. Verified cargo test -p buzz-cli (359 passed), cargo fmt --all -- --check, and cargo clippy -p buzz-cli --all-targets -- -D warnings.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Last of the same root cause as #5966 and #5970. No issue filed.
buzz reactions removefinds the reaction to retract by querying kind:7 events with#eset to the target id, then reports what it found.#eis a NIP-01 generic tag filter, compared as a raw string against a tag every event writes lowercase — so with an id pasted in uppercase the query returns nothing and the command answers:That is a statement about the world, and it is false. The reaction is there; the query spelled its target differently than the tag does.
reactions getprints an empty list for the same reason.cmd_add_reactionnormalizes too. It parses the id into a typedEventId, so its published tag was already lowercase and nothing was broken there — but leaving one of the three spellings unnormalized is exactly how this drifts back.The second commit lifts the two inline filters into
reaction_filterand pins what goes into them, so the#evalue is checked by a test rather than only by the network.This finishes the sweep. Every value under
#e/#p/#aincrates/buzz-cli/src/commandsnow comes from a typed key (my_pk,owner.to_hex()), fromparse_hex64, or fromrepo_coord_a_value.Stacked on #5970, which is stacked on #5966. Take them in that order and this branch contains all three; happy to rebase or squash the series if you'd rather have it as one change.
Verified locally on the pinned 1.95.0 toolchain:
cargo test -p buzz-cli— 359 passed, 0 failed (357 on the parent branch, plus the 2 new)cargo clippy --workspace --all-targets -- -D warnings— cleancargo fmt --all -- --check— cleanNote: I'm an outside contributor, so the workflow runs here sit at
action_requireduntil a maintainer approves them; only the DCO check reports on its own.