Skip to content

fix(desktop): route to the branched session so its window renders - #98086

Closed
hutao562 wants to merge 1 commit into
NousResearch:mainfrom
hutao562:fix/branch-window-route
Closed

hutao562 wants to merge 1 commit into
NousResearch:mainfrom
hutao562:fix/branch-window-route

Conversation

@hutao562

Copy link
Copy Markdown

fix(desktop): route to the branched session so its window renders

Symptom

Clicking a bubble's branch action (the GitFork "branch in new chat" button, or any path that forks the open chat into a child session) creates the branch correctly, but the branched session's window shows a blank spinner until the user switches to some other session and switches back — only then does the content render.

Root cause

forkBranch (use-session-actions/index.ts) takes over the main pane like this:

if (parentStoredId !== null && selectedStoredSessionIdRef.current === parentStoredId) {
  await resumeSession(routedSessionId)
}

resumeSession swaps the selection ($selectedStoredSessionId / selectedStoredSessionIdRef) but never touches the URL — its replaceRoute parameter is accepted and ignored, and the function body contains no navigate call at all.

So after a branch the route still points at the PARENT (/:parent-id) while the selection points at the CHILD. On every render:

  • isRouteSessionMismatch(routed=parent, selected=child) → true (chat/index.tsx, route-session-state.ts)
  • ChatRuntimeBoundary gets suppressMessages = true → the transcript is suppressed
  • routedSessionIsLoading → the session loader never retires
  • use-route-resume even sees stuckOnRoutedSession (route ≠ selection) and can re-resume the parent, yanking the view back

Switching away and back works because a sidebar click routes through openSession, which does call navigate(sessionRoute(id)) — that's the only thing the branch path was missing.

Fix

One line in the main-pane takeover path of forkBranch, plus the navigate dependency:

if (parentStoredId !== null && selectedStoredSessionIdRef.current === parentStoredId) {
  await resumeSession(routedSessionId)
  navigate(sessionRoute(routedSessionId), { replace: true })
}

Route and selection now land atomically, so the mismatch suppression never engages.

The background-branch path (branching a session that is NOT currently open) is intentionally untouched: it opens a tile, which carries no route, and navigating there would reintroduce the #69750 focus-stealing bug.

Testing

  • New regression test: routes the main workspace to the branch session (branch window renders without a manual away-and-back) — asserts navigate is called with sessionRoute('branch-stored') + { replace: true } after a main-pane branch.
  • use-session-actions.test.tsx: 85 passed (was 84)
  • Adjacent surface (src/app/session/hooks/ + src/app/chat/): 144 files, 1557 tests passed
  • tsc -p tsconfig.json --noEmit: clean
  • eslint on both touched files: clean

Clicking a bubble's branch action (or "branch in new chat") opens the
child session, but its window stayed blank on a spinner until the user
switched to another session and back.

Root cause: forkBranch takes over the main pane by calling
resumeSession(routedSessionId), but resumeSession swaps only the
selection — its replaceRoute parameter is accepted and never used, and
resumeSession itself never navigates. The URL kept pointing at the
PARENT session while the selection pointed at the CHILD, so
isRouteSessionMismatch stayed true on every render: ChatRuntimeBoundary
suppressed the transcript (suppressMessages) and routedSessionIsLoading
never retired. The away-and-back "fix" worked because a sidebar click
routes through openSession, which navigates.

Fix: navigate to the branch session (replace: true) right after
resumeSession in the main-pane takeover path, so route and selection
land atomically. The background-branch path is untouched — it opens a
tile, which carries no route (and NousResearch#69750 focus-stealing stays fixed).
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 29, 2026
@alt-glitch

Copy link
Copy Markdown

This was generated by AI during triage.

Related: #95992 addresses the same forkBranch route/selection mismatch by navigating before resumeSession; this PR navigates after it. Please choose one ordering and avoid merging both.

@hutao562

Copy link
Copy Markdown
Author

@alt-glitch Thanks for the triage cross-link — I hadn't seen #95992 when I opened this. Having compared the two:

Closing this in favor of #95992 (@mashenchina-max, opened 2 days earlier, same root cause, same fix site, tests included).

For what it's worth, I independently audited both orderings before closing, and #95992's ordering (navigate before resumeSession) is the more robust one, so this closure isn't just deference to prior art:

  • Navigate-first: the URL lands on the child at click time; during the resume window the loader honestly shows under the child's route. On a terminal resume failure the user is already on the child's route, resumeFailedSessionId === routedSessionId holds, and the bounded auto-retry in use-route-resume recovers in place.
  • Navigate-after (this PR): if resumeSession fails mid-flight, the route is still on the parent while the selection already points at the child. stuckOnRoutedSession then fires a spurious re-resume of the parent, yanking the user back, and the failed child's retry latch is armed for a session nobody is viewing — the bounded retry never services it.

One heads-up for #95992's review: it touches the same lines of use-session-actions/index.ts as #98025 (branch truncation by row id), so whichever merges second will need a trivial rebase — no semantic conflict, the two fixes are independent (data correctness vs route convergence).

@hutao562 hutao562 closed this Aug 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants