Skip to content

fix(desktop): preserve membership refresh during an in-flight channel fetch - #7306

Open
nathansmithopenclaw-alt wants to merge 2 commits into
block:mainfrom
nathansmithopenclaw-alt:membership-refresh-inflight
Open

nathansmithopenclaw-alt wants to merge 2 commits into
block:mainfrom
nathansmithopenclaw-alt:membership-refresh-inflight

Conversation

@nathansmithopenclaw-alt

Copy link
Copy Markdown

Summary

Keep the Desktop joined-channel list live when a membership notification
arrives while get_channels is already in flight.

  • Route kind 44100 / 44101 channel-list invalidation through the existing
    refreshChannelsWhenIdle helper.
  • Coalesce bursts with the existing trailing-debounce utility.
  • Re-arm while the channel query is fetching, then invalidate exactly once
    after it becomes idle.
  • Keep the affected channel's detail and member-list invalidations immediate.
  • Cancel the pending debounce when the identity-scoped hook unmounts.

Problem

useMembershipNotifications currently invalidates channelsQueryKey
immediately. If an older get_channels request is already running, the
membership signal can lose this ordering:

  1. get_channels starts and reads the pre-membership list.
  2. A kind 44100 or 44101 arrives and invalidates the query.
  3. The older request settles with pre-event data and clears the dirty state.
  4. The Channels / Forums sidebar remains stale until the next polling,
    browse, or reconnect refresh.

The repository already documents and solves this race for ordinary live
channel traffic in refreshChannelsWhenIdle. This change applies the same
boundary to membership-driven discovery.

Scope

This is intentionally client-only and narrow. It does not:

  • change relay membership or NIP-29 event emission;
  • add a global kind 39002 subscription (those discovery events are
    channel-scoped);
  • change terminal CLOSED retry policy;
  • restore broad focus-triggered query refetch; or
  • add a second manual refresh control.

User impact

When another member adds or removes the signed-in user, the Desktop channel
navigation converges immediately after any already-running channel fetch
settles instead of remaining stale for up to the next one-minute poll. The same
shared cache update also refreshes downstream channel-bound subscriptions,
search/reference resolution, unread state, and navigation labels.

Testing

  • Regression: deliver a membership event while the channel query reports
    one active fetch; prove channel detail and member-list keys invalidate
    immediately while the channel-list invalidation waits for idle, then
    lands exactly once.
  • Full Desktop unit suite.
  • Desktop typecheck.
  • Sidebar channel-list E2E suite.
  • Biome check for the changed source and regression test.
  • git diff --check.

Related: #6713

Signed-off-by: nathansmithopenclaw-alt <273889179+nathansmithopenclaw-alt@users.noreply.github.com>
@nathansmithopenclaw-alt
nathansmithopenclaw-alt requested a review from a team as a code owner September 3, 2026 21:47
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

🔐 Codex Security Review

Status: review required for the current range.

The current range is 3c7f288c60d67df78577b237e27c3dfc8831aaa1...8034ca922f9a41db2a545624d844d0f4a213b10c.
A new review must complete for this exact range. When manual authorization
is required, a Block organization member must comment exactly
@buzz-security-review 8034ca922f9a41db2a545624d844d0f4a213b10c to authorize a new review.
Any previous review applies only to its recorded range.

@Chessing234 Chessing234 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

deferring the list invalidate while get_channels is in flight (and re-arming) is exactly the membership/list race. test covers the busy→idle handoff.

@Chessing234 Chessing234 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the idle helper is the right idea, but isFetching({ queryKey: channelsQueryKey }) is a prefix match, and this handler also kicks detail/members first — so those child fetches keep the gate closed and delay the list refresh even when get_channels is idle. please use exact: true for the list query (and cover that in the test without wall-clock sleeps).

`isFetching({ queryKey: channelsQueryKey })` matches by prefix, so an
in-flight ["channels", id, "detail"] or ["channels", id, "members"] fetch
counted as the channel list being busy. The membership handler kicks both
of those immediately after arming the debounce, so the gate was held
closed by the very fetches the handler itself started and re-armed
indefinitely under membership churn. Pass `exact: true` so only
get_channels holds the gate.

Drive the test against a real QueryClient instead of replacing
`isFetching` wholesale: a stub cannot exercise prefix-vs-exact filter
semantics, which is the behaviour at issue. Time is driven by node:test
fake timers, so the file no longer sleeps on the wall clock.

Signed-off-by: nathansmithopenclaw-alt <273889179+nathansmithopenclaw-alt@users.noreply.github.com>
@nathansmithopenclaw-alt

Copy link
Copy Markdown
Author

Good catch — fixed in 8034ca9. isFetching({ queryKey: channelsQueryKey }) is a prefix match, so the ["channels", id, "detail"] and ["channels", id, "members"] fetches that this same handler kicks off were counted as the list being busy; the gate was held closed by the very fetches it started and re-armed indefinitely under membership churn. It now passes exact: true, so only get_channels holds it. You were also right that the old test could not have caught this: it replaced queryClient.isFetching wholesale, so prefix-vs-exact filter semantics were never exercised. The test now runs against a real QueryClient with isFetching untouched — one case parks a real members fetch in flight (deferred promise) while the list query is idle and asserts the list refresh still lands, with a fixture-sanity assertion that the prefix and exact filters genuinely disagree in that state so it cannot pass vacuously; the busy→idle case now parks a real get_channels fetch rather than a stub. Both drive the 500 ms debounce with node:test fake timers, so the wall-clock sleeps are gone.

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.

2 participants