Skip to content

fix(desktop): open the thread panel for bare ?thread= deep links - #158

Merged
oscarlehuu merged 4 commits into
mainfrom
devin/fix-153-bare-thread-deeplink
Aug 11, 2026
Merged

fix(desktop): open the thread panel for bare ?thread= deep links#158
oscarlehuu merged 4 commits into
mainfrom
devin/fix-153-bare-thread-deeplink

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Aug 11, 2026

Copy link
Copy Markdown

Based on devin/fix-132-thread-summary-e2e (PR #156), targeting main. The
thread-orientation specs that cover this bug only run truthfully with #156's
mock-bridge harness fix, so this branch includes it. Review the top commit;
#156's six commits land with it or before it.

Summary

A bare ?thread=<id> deep link never opened the thread panel. The reported
mechanism (useChannelRouteTarget returning early when targetMessageId is
null) is real but not what breaks the link — instrumenting the failing scenario
showed the panel does open and is then torn down:

  1. ?thread= is mapped to panel state, so openThreadHeadId is the linked head
    from the first render (<ref_snippet file="desktop/src/features/channels/ui/useChannelPanelHistoryState.ts" />).
  2. ChannelRouteScreen fetches that head plus its ancestors asynchronously and
    splices them into the timeline.
  3. Before those events land, useThreadTargetSync sees an open head with no
    message and a settled timeline (isTimelineLoading === false) and treats it
    as deleted: it clears thread from the URL, so the route target disappears
    too and the spliced events arrive with nothing left to open.

So the missing piece is not a second target input — the panel state already
names the right head, which for a bare thread link is the root itself — but the
fact that "head not in the timeline" was read as "head does not exist" while the
route was still resolving it. No new URL parameter or route shape is introduced.

openThreadHeadId = <id>            // from ?thread=, render 1
head missing && !timelineLoading   // -> was: teardown, URL param cleared
                                   //    now: hold, iff <id> is the head the
                                   //         route is still resolving
route fetch settles (events + flag committed in the same render)
head present  -> panel stays open on the resolved root
head absent   -> existing teardown runs (deleted / bogus id)
  • shouldHoldMissingThreadHead() in useThreadTargetSync holds teardown only
    when the missing open head is the head the route is resolving, and only
    until resolution settles. An unrelated missing head still closes immediately,
    and an invalid id closes once the fetch settles instead of leaving a permanent
    skeleton.
  • ChannelRouteScreen derives isRouteTargetResolving during render rather
    than setting it from the fetch effect: ChannelScreen is a child, so its
    effects run before the route's fetch effect, and an effect-set flag would
    still be false on the commit that first sees a new target in the URL (that
    ordering hole is exactly the teardown window). The resolved events and the end
    of the resolve are committed in one continuation, so no consumer ever sees
    "settled, but events not yet spliced".
  • No timers, no timeout changes, no spec changes.

useChannelRouteTarget's ?messageId= semantics are untouched: its early
return only skips main-timeline centering/ancestor expansion, which a bare
thread link does not ask for (the panel derives ancestry and the top-level
anchor from the head via threadOrientation).

Related issue

Fixes #153. Filed by PR #156; no other duplicates found.

Testing

RED before the fix (tests/e2e/thread-orientation.spec.ts, on this branch's
base):

✘ 03-nested-head-keeps-top-level-anchor-and-shows-ancestry   (message-thread-panel never visible)
✘ 04-depth-2-rootId-equals-parentId-still-anchors-top-level  (thread-breadcrumb never contains #general)
2 failed, 2 passed

GREEN after the fix:

✓ 01-breadcrumb-shows-channel-and-navigates-to-anchor
✓ 02-timeline-anchor-state-and-viewing-thread-pill
✓ 03-nested-head-keeps-top-level-anchor-and-shows-ancestry
✓ 04-depth-2-rootId-equals-parentId-still-anchors-top-level
4 passed (10.8s)

Neighbouring deep-link forms (?messageId= + ?thread= in
persistent-agent-audience.spec.ts:37, panel-state URL round-trips in
navigation.spec.ts, buzz://message?...goChannel(channelId, {messageId, threadRootId}) in useMessageDeepLinks): navigation +
persistent-agent-audience + thread-orientation run together — base 22
passed / 2 failed (the two above) / 1 skipped, this branch 25 passed / 0 failed
/ 1 skipped.

Unit: three focused cases for the hold predicate (holds the matching head while
resolving; releases once settled; never holds an unrelated head) in
desktop/src/features/channels/useThreadTargetSync.test.mjs. Gates:
pnpm --filter buzz check, pnpm --filter buzz typecheck,
pnpm --filter buzz test (5049 passed, 1 environment-gated skip, 0 failed).

Full --project=smoke suite, all 4 shards, before vs after — before is this
branch's base, PR #156 head
(run 31458263391),
after is this branch
(run 31460257139),
same workflow and sharding:

shard before after delta
1 8 failed, 2 flaky 8 failed, 1 flaky ±0 (set differs only by flakes: channel-composer-overflow:217 + community-rail:1088 before, channel-browser:399 after)
2 0 failed 0 failed ±0
3 4 failed 4 failed identical set
4 9 failed, 4 flaky 7 failed, 1 flaky −2: exactly thread-orientation 03 and 04

No failure appears after the fix that was not already failing before it, and the
only removed failures are the two this PR targets. The red lanes themselves are
preexisting on main: the latest main run
(31362178966) has
Desktop Smoke E2E (1) and (3) failing and (4) cancelled at the timeout
(#109), and channels.spec.ts:500 (#110) fails in shard 1 there too. Local
xvfb runs of shards 1–3 on this branch agree on the failing families
(10 / 0 / 7 failures in 629s / 567s / 640s; the local extras over CI are the
usual xvfb-only flakes such as messaging.spec.ts:1300 and
onboarding-agent-defaults.spec.ts:878, none in the thread deep-link path).

No visual change beyond the panel now opening, which the specs assert.
docs/crew/STATE.md records the fixed deep-link behavior. No new decision was
needed, so no D-0xx number was taken.

Link to Devin session: https://app.devin.ai/sessions/9ac4ed023b79405a824070632a15449e
Requested by: @oscarlehuu

@oscarlehuu oscarlehuu self-assigned this Aug 11, 2026
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

devin-ai-integration Bot and others added 4 commits August 11, 2026 23:53
Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Oscar Le <oscar.lehuu@gmail.com>
Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Oscar Le <oscar.lehuu@gmail.com>
A bare `?thread=<id>` deep link commits the head to panel state on the same render, but the head and its ancestors are only fetched afterwards by the channel route. `useThreadTargetSync` saw an open head with no message and a settled timeline, so it tore the panel down before the fetched events landed.

Hold that teardown while the channel route is resolving that exact head: `ChannelRouteScreen` derives `isRouteTargetResolving` during render (a consumer's effects run before the route's own fetch effect, so an effect-set flag is still false on the commit that first sees the new target) and commits the resolved events together with the end of the resolve. The hold is keyed to the head's identity and released when resolution settles, so unrelated missing heads and deleted or bogus ids still close the panel.

Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Oscar Le <oscar.lehuu@gmail.com>
Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Oscar Le <oscar.lehuu@gmail.com>
@oscarlehuu
oscarlehuu force-pushed the devin/fix-153-bare-thread-deeplink branch from 70ae39f to 5fa1a25 Compare August 11, 2026 14:23
@oscarlehuu
oscarlehuu merged commit 55b858e into main Aug 11, 2026
8 of 12 checks passed
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.

Bare ?thread=<id> deep link resolves the event and ancestors but never opens the thread panel

1 participant