Skip to content

fix(desktop): preserve full thread context when opening reply notifications - #7452

Open
JohnRichardEnders wants to merge 1 commit into
block:mainfrom
JohnRichardEnders:fix/thread-notification-context
Open

fix(desktop): preserve full thread context when opening reply notifications#7452
JohnRichardEnders wants to merge 1 commit into
block:mainfrom
JohnRichardEnders:fix/thread-notification-context

Conversation

@JohnRichardEnders

Copy link
Copy Markdown

Summary

Fix reply notifications opening a thread containing only the clicked reply instead of its root and surrounding conversation.

Notification/search navigation caches a display-only RelayEvent with no reply tags. ChannelRouteScreen inserted that projection before fetching the actual event. useChannelRouteTarget consequently classified it as a root, selected the reply as the thread head, and consumed the route target. Hydrating the real reply afterward did not repair the already-accepted selection.

Defer inserting the cached projection until the existing authoritative event/root/ancestor fetch completes. Merge fetched events first, and retain the projection only as a fallback when the event cannot be fetched. Existing timeline rows remain mounted after the route target clears.

Duplicate check

Searched issues/PRs for notification thread navigation. Closest prior fix is merged #790, which carries the thread root through notification routing. This fixes a later projection/hydration race in that route path, not another notification transport implementation. #6695 concerns displaying replies in the channel timeline.

Testing

  • Added a deterministic browser regression that holds the clicked reply's get_event lookup, clicks a real mock desktop notification, then releases hydration. It requires the root, sibling reply, intermediate parent, and nested target in the thread panel.
  • Before the fix: regression fails because the thread head is Nested notification reply.
  • After the fix: regression passes; full navigation spec has 20 passing cases and one pre-existing skipped case.
  • Existing notification settings drive Inbox badge and desktop alerts integration case passes, including the cache-only notification fallback.
  • Typecheck, Biome checks on the changed files, and file-size checks pass.
  • Visually verified notification activation in local Chromium using the existing mock Tauri/relay bridge: root, parent, sibling, and nested reply all visible.

No relay protocol, notification payload, or installed-app configuration changes.

@JohnRichardEnders
JohnRichardEnders requested a review from a team as a code owner September 7, 2026 15:04
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

🔐 Codex Security Review

Status: review required for the current range.

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

Signed-off-by: John R. Enders <92937551+JohnRichardEnders@users.noreply.github.com>
@JohnRichardEnders
JohnRichardEnders force-pushed the fix/thread-notification-context branch from 922aab2 to 031df39 Compare September 7, 2026 15:12
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.

1 participant