Skip to content

fix(relay): stop multi-channel HTTP query from narrowing to one UUID - #5307

Open
BradGroux wants to merge 1 commit into
block:mainfrom
BradGroux:fix/relay-multi-h-query-narrowing
Open

fix(relay): stop multi-channel HTTP query from narrowing to one UUID#5307
BradGroux wants to merge 1 commit into
block:mainfrom
BradGroux:fix/relay-multi-h-query-narrowing

Conversation

@BradGroux

Copy link
Copy Markdown
Contributor

Summary

A POST /query filter with multiple #h values silently returned events from only the lexicographically smallest channel UUID, ignoring all other listed channels. The desktop Workflows screen issues exactly this shape — one filter with all member-channel IDs — so workflows in any channel other than the smallest UUID were invisible.

The single-filter extract_channel_id_from_filter returned the first parseable UUID from the BTreeSet-ordered #h values, which pinned the SQL to channel_id = <that uuid>. The multi-filter variant extract_channel_id_from_filters already had the correct guard — return None when more than one distinct channel appears — but the HTTP bridge path (build_event_query_from_filter) used the single-filter variant without that guard.

The fix gives extract_channel_id_from_filter the same multi-channel semantics as the plural variant: return None when the filter carries more than one distinct channel UUID. apply_access_scope_to_query then replaces the single-channel pin with the caller's full accessible-channel IN-list, and the existing per-event post-check (filters_match + accessible-channel check) keeps results correct.

Related issue

Fixes #5053.

Testing

  • 4 new unit tests covering the single-filter extraction: single channel, multiple distinct channels (returns None), no channel tag (returns None), and duplicate same channel (returns the channel).
  • Full buzz-relay library test suite: 863 passed, 8 failed (all PoolTimedOut — database-dependent media/admin tests that fail identically on clean main without a Postgres connection).
  • cargo fmt --check and cargo clippy -- -D warnings passed for buzz-relay.

@BradGroux
BradGroux requested a review from a team as a code owner August 8, 2026 08:37
@BradGroux

Copy link
Copy Markdown
Contributor Author

This is the third recurring review + rebase pass for the open BradGroux PRs on block/buzz.

What this PR does

Fixes a relay bug where extract_channel_id_from_filter would return the lexicographically smallest channel UUID from a multi-value #h tag (due to BTreeSet iteration order) instead of recognizing that multiple distinct channels were requested. When a filter carries multiple distinct #h values, the function now returns None, causing apply_access_scope_to_query to fall back to the caller's accessible-channel list instead of pinning to one arbitrary UUID.

The implementation iterates the tag values, tracks the first UUID found, and returns None if a second distinct UUID is encountered. Duplicate UUIDs are handled correctly (return the single UUID).

Correctness

  • The three-way logic (no channel → None, single channel → Some(id), multiple distinct channels → None) is correct and well-tested.
  • Four new unit tests cover: single channel, multi-distinct-value, no channel, and duplicate channel.
  • The refactor from manual iter() loop to filter.generic_tags.get(&h_tag) is cleaner and uses the proper SingleLetterTag API.
  • No new dependencies. No unwrap/expect in production paths (test-only expect is acceptable).

Rebase result

Already based on current main (02f640bc4). No rebase needed — 0 commits behind.

  • Head SHA: c3c0836406141e1f9c30a3ecc69f703319a679fb (unchanged)
  • Mergeable: ✅ MERGEABLE
  • CI: DCO ✅, Semgrep OSS ✅, zizmor ✅

No code changes were made — this was a review + rebase pass only.

@BradGroux
BradGroux force-pushed the fix/relay-multi-h-query-narrowing branch from c3c0836 to a63a022 Compare August 9, 2026 10:40
@BradGroux

Copy link
Copy Markdown
Contributor Author

Portfolio review update (2026-08-09)

I rechecked the relay query narrowing behavior against current main. The multi-h filter issue is still present without this change, and no newer main commit or competing PR supersedes the fix.

I rebased the branch onto 5bf78671f45178f8de02ba18d3d321cbbf19cd1f. The reviewed patch remains equivalent after the rebase and the branch merges cleanly. The helper coverage remains the relevant regression proof; upstream checks are rerunning for the rewritten head.

@BradGroux
BradGroux force-pushed the fix/relay-multi-h-query-narrowing branch from a63a022 to 36b2858 Compare August 11, 2026 03:01
@BradGroux

Copy link
Copy Markdown
Contributor Author

Portfolio rebase and review update (2026-08-11)

Rebased onto main at 7e6e9c547 (2026-08-10). No conflicts.

Still needed: The multi-#h query narrowing bug is still present in extract_channel_id_from_filter. No newer main commit addresses this — the function still returns the first parseable UUID from BTreeSet-ordered #h values, pinning SQL to one channel when a filter lists multiple channels.

Verification: cargo check -p buzz-relay passes cleanly. The PR adds four unit tests covering single-channel, multi-distinct-channel (returns None), no-channel, and duplicate-channel cases.

@EmminiX

EmminiX commented Aug 11, 2026

Copy link
Copy Markdown

Confirming this from a self-hosted relay, with two pieces of evidence I have not seen in the other threads on this bug.

The relay already documents the behaviour this PR restores

filter_fully_pushable and extract_channel_id_from_filter live ~50 lines apart in the same file and disagree.

crates/buzz-relay/src/handlers/req.rs:802-808:

"h" => {
    // Single #h is pushed as channel_id; multi-#h is not.
    if tag_values.len() > 1 {
        return false;
    }
}

crates/buzz-relay/src/handlers/req.rs:858-869 (current main, be48ce98):

fn extract_channel_id_from_filter(filter: &Filter) -> Option<uuid::Uuid> {
    for (tag_key, tag_values) in filter.generic_tags.iter() {
        ...
                if let Ok(id) = val.parse::<uuid::Uuid>() {
                    return Some(id);

So the COUNT path correctly refuses to push multi-#h, while the query path pushes the first UUID it meets. Since generic_tags is set-backed, "first" is effectively the lexicographically smallest channel UUID, not request order.

Three other call sites already implement the guard this PR adds, which is why I read it as a missing guard rather than a design question:

  • extract_channel_id_from_filters (req.rs:1029) returns None on multiple distinct UUIDs, with a doc comment explaining exactly why.
  • filter_to_query_params pushes #p only when values.len() == 1.
  • crates/buzz-relay/src/api/bridge.rs:1685-1698, the NIP-50 search path, does the fuller thing: it parses every #h value and intersects the set with accessible_channels, skipping the filter when none survive. A workflow query never reaches it only because that loop continues unless filter.search is set.

That last one is worth highlighting: the project has already written the correct multi-#h handling once.

Reproduction on real data

Self-hosted relay, one account, kind:30620 rows straight from Postgres:

channel distinct workflows events
2757e3d4-… 2 6
eeacc5bb-… 1 1
f8809de6-… 2 3

Five workflows across three channels. Desktop's get_channels_workflows batches all member channels into one #h filter, 2757e3d4 sorts first, and the Workflows screen shows 2 of 5. Per-channel buzz workflows list --channel <id> returns all of them correctly, which is what makes it look like the data is missing when it is only unreachable.

The batching itself is correct and documented in desktop/src-tauri/src/commands/workflows.rs: "A nostr #h filter matches ANY of its listed values, so one query with all channel ids returns the same set." That is NIP-01. It became load-bearing when the screen moved from N per-channel queries to one batched query.

One honest caveat on this fix

Returning None makes apply_access_scope_to_query set channel_ids to the caller's whole accessible set, and bridge.rs:1299 then re-applies the real filter via filters_match, so results are correct. But the SQL page (buzz-db/src/event.rs, DEFAULT_MAX_PAGE_LIMIT = 1000) is filled from the wider set before that narrowing. So a caller with more than a page of accessible events in the matched kinds can still lose requested rows off the end. That is the same LIMIT-before-post-filter hazard the #p pushdown comment in filter_to_query_params already calls out.

Not a blocker for this PR, which is a correct minimal fix and strictly better than the status quo. Flagging it so the follow-up is on the record: pushing the requested set into EventQuery.channel_ids and intersecting with accessible channels, exactly as the search path already does, would remove the hazard entirely.

Duplicates

This is reported in #2385 (oldest), #4419, #4659, #4804 and #5053, with #4446 and #5339 as alternative fixes. Consolidating on #2385 plus this PR would probably make the state of it easier to see.

@BradGroux

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough confirmation and the real-world reproduction — this is the best evidence on this bug I've seen in any of the threads.

Verified against current main (be48ce98)

Every claim in the comment checks out:

1. The two functions disagree. filter_fully_pushable (line 802) refuses multi-#h (returns false), while extract_channel_id_from_filter (line 858) returns the first UUID from BTreeSet iteration order — lexicographic, not request order. This is the bug.

2. Three other call sites already guard against multi-#h:

  • extract_channel_id_from_filters (line 1029) returns None on multiple distinct UUIDs with a doc comment explaining why.
  • filter_to_query_params (line 944) pushes #p only when values.len() == 1.
  • bridge.rs line 1693 (handle_bridge_search) intersects every #h value with accessible_channels and skips the filter when none survive. This is the most complete handling in the codebase.

3. The bridge path is also fixed by this PR. The Desktop Workflows screen goes through POST /queryquery_events_authedbuild_event_query_from_filterextract_channel_id_from_filter. With this PR, that function returns None for multi-#h. Then apply_access_scope_to_query sets channel_ids to the caller's accessible set, and filters_match re-applies the full #h filter as post-filtering. Results are correct.

On the LIMIT-before-post-filter caveat

This is an honest and accurate observation. Returning None widens the SQL candidate set to all accessible channels before filters_match narrows it. The DEFAULT_MAX_PAGE_LIMIT (1000) means a caller with >1000 accessible-channel events in the matched kinds could lose rows past the page boundary.

The caveat is not a blocker for this PR because:

  • The status quo (pinning to one channel) is strictly worse — it returns zero workflows for every channel except the lexicographically smallest one, not just truncated results.
  • The #p pushdown comment in filter_to_query_params already calls out the same LIMIT-before-post-filter hazard, so this is a known class of issue in the codebase, not a new one.
  • The search path in bridge.rs:1693 already shows the complete fix: push the requested #h set into EventQuery.channel_ids and intersect with accessible channels at the SQL level. That eliminates the post-filter narrowing entirely.

The follow-up EmminiX describes (push the requested set into channel_ids and intersect with accessible channels, exactly as the search path does) is the right next step. It would require extending filter_to_query_params to accept multiple channel IDs instead of a single Option<Uuid>, or adding a separate channel_ids field to the EventQuery that's populated alongside channel_id. Either way, that's a follow-up PR, not a reason to block this one.

On duplicate consolidation

Agreed that consolidating on #2385 plus this PR would make the state easier to see. The five open issues (#2385, #4419, #4659, #4804, #5053) all describe the same root cause. The two other open fix PRs (#4446 by @Trevongit and #5339 by @joaoh8282) both fix the same extract_channel_id_from_filter function with the same multi-distinct-UUID guard. #4446 also changes the Desktop side to send N single-#h filters instead of one multi-#h filter, which works around the bug at the client layer — that's a reasonable belt-and-suspenders approach but it changes the query shape from 1 relay round-trip to N, which is why the relay-side fix here is the better primary fix.

This PR is a correct minimal fix that is strictly better than the status quo. The LIMIT caveat is real but belongs in a follow-up.

@BradGroux
BradGroux force-pushed the fix/relay-multi-h-query-narrowing branch from 36b2858 to 51a7417 Compare August 12, 2026 02:55
@BradGroux

Copy link
Copy Markdown
Contributor Author

Rebased onto latest main (4b35706, the 0.5.10 release). No conflicts — crates/buzz-relay/src/handlers/req.rs had no recent changes that overlapped.

Reviewed for accuracy and continued relevance:

  • The multi-channel HTTP query narrowing bug is still present in main — no recent commit touches extract_channel_id_from_filter or the HTTP bridge query path.
  • The fix (use extract_channel_id_from_filters with the multi-channel guard in build_event_query_from_filter) is still the correct approach.
  • The Workflows screen still issues the multi-#h filter shape that triggers the bug.

The PR is mergeable and ready for review.

@BradGroux
BradGroux force-pushed the fix/relay-multi-h-query-narrowing branch from 51a7417 to e7a79cc Compare August 13, 2026 05:13
@BradGroux

Copy link
Copy Markdown
Contributor Author

Portfolio rebase and review update (2026-08-13)

Rebased onto main at a96af8952 (2026-08-13). No conflicts.

Still needed

Yes. The multi-channel HTTP query narrowing fix is not superseded. Two other open PRs from other contributors still target the same bug — #4446 (client-side workaround sending N single-#h filters) and #5339 (same guard in extract_channel_id_from_filter). Neither has been merged. This PR remains the correct minimal server-side fix.

The LIMIT caveat documented in the previous review still applies: returning None for multi-#h widens the SQL candidate set before post-filtering, which could truncate at DEFAULT_MAX_PAGE_LIMIT. Not a blocker — the status quo returns zero workflows for every channel except the smallest UUID. The complete fix (push #h set into channel_ids and intersect at SQL level, as the search path already does) remains a follow-up.

Verification

cargo check clean. All 14 portfolio PRs confirmed MERGEABLE on GitHub.

A POST /query filter with multiple #h values silently returned events
from only the lexicographically smallest channel UUID, ignoring all
others. The single-filter extract_channel_id_from_filter returned the
first parseable UUID from the BTreeSet-ordered #h values, which pinned
the SQL to a single channel. The multi-filter variant
extract_channel_id_from_filters already had the correct guard — return
None when more than one distinct channel appears — but the HTTP bridge
path used the single-filter variant without that guard.

When extract_channel_id_from_filter returns None,
apply_access_scope_to_query replaces the single-channel pin with the
caller's full accessible-channel IN-list, and the existing per-event
post-check keeps results correct.

Closes block#5053.

Co-authored-by: Brad Groux <brad@digitalmeld.com>
Signed-off-by: Brad Groux <brad@digitalmeld.com>
@BradGroux
BradGroux force-pushed the fix/relay-multi-h-query-narrowing branch from e7a79cc to ab4ec58 Compare August 15, 2026 03:01
@BradGroux

Copy link
Copy Markdown
Contributor Author

Rebase review — 2026-08-15

Rebased onto current main at 82f7ed153 (2026-08-15). 41 new commits landed since the last review point (a96af8952, 2026-08-13). This PR rebased cleanly with no conflicts.

Still-needed assessment

Checked all 41 new commits for overlap. None supersede this PR. The PR remains the correct fix for its subject area.

Verification

  • cargo check not required — no Rust conflicts
  • GitHub confirms MERGEABLE status

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.

HTTP bridge /query with multiple #h values silently narrows to the lexicographically smallest channel UUID — Workflows screen shows "No workflows yet"

2 participants