Skip to content

fix(desktop): open a branched session in the main workspace, not just a tile - #93461

Closed
chelsealong wants to merge 2 commits into
NousResearch:mainfrom
chelsealong:fix/desktop-branch-opens-main-workspace-93444
Closed

chelsealong wants to merge 2 commits into
NousResearch:mainfrom
chelsealong:fix/desktop-branch-opens-main-workspace-93444

Conversation

@chelsealong

Copy link
Copy Markdown

Summary

Fixes #93444. Creating a branch from the session you're currently viewing
(right-click → "Branch") only added a new row in the left sidebar — the main
workspace area stayed on the parent session, so branching looked like it did
nothing.

Root cause

In apps/desktop/src/app/session/hooks/use-session-actions/index.ts,
forkBranch ended by opening the branch as a session-tile and deliberately
leaving the primary selection on the parent (introduced in 6326b30c93 /
#69750, to fix a different bug: branching stealing focus from the parent
chat).

In the default layout there is no visible tile pane, so openSessionTile(..., 'center') + revealTreePane(...) had almost no visible effect — the only
observable result was the new sidebar row. It's worse in the most common
case: branching the session you're currently viewing. openSessionTile
short-circuits when the target is already $selectedStoredSessionId, so the
tile-open is a straight no-op there.

Fix

forkBranch is shared by every branch entry point (message GitFork button,
/branch and /fork, sidebar right-click, tile Branch) — some of those
branch the session currently open in the main pane, others branch an
unrelated session from the sidebar while looking at something else. Loading
the branch as primary unconditionally (an earlier version of this PR) would
have reintroduced #69750 for the latter case: branching a different
session from the sidebar would yank the active view away from whatever the
user was looking at.

So the fix only takes over the main pane when the session being branched is
the one already selected:

if (parentStoredId !== null && selectedStoredSessionIdRef.current === parentStoredId) {
  await resumeSession(routedSessionId)
} else {
  openSessionTile(routedSessionId, 'center')
  patchSessionTile(routedSessionId, { runtimeId: branched.session_id })
  revealTreePane(`session-tile:${routedSessionId}`)
}

resumeSession is the canonical "load session as primary" path — it sets
$selectedStoredSessionId (which drives the route to the main workspace) and
closes any tile with the same id (a session is either the main thread or a
tile, never both). It also reuses the runtime already warm-cached by
forkBranch's preceding ensureSessionState/updateSessionState calls
(runtimeIdByStoredSessionIdRef / sessionStateByRuntimeIdRef), so this
doesn't cost an extra resume RPC beyond what the old tile-open path already
did implicitly. When the branched session isn't the one currently selected,
behavior is unchanged from before this PR: it opens as its own tile.

Test

Updated use-session-actions.test.tsx's
branchStoredSession desktop source tagging suite:

  • The existing test now explicitly sets the harness's
    selectedStoredSessionId to the branched parent (the "currently-open
    chat" case) and asserts the branch becomes primary and does not exist as a
    tile.
  • Added a new test, "keeps the current view when branching a different
    session from the sidebar (does not reintroduce fix(desktop): open branched chat in a new tab and switch to it #69750)": the harness is
    selected on stored-other, and a different session, stored-parent, is
    branched (the sidebar/background path). Asserts $selectedStoredSessionId
    stays on stored-other and the branch opens as a tile instead.

Confirmed the new test fails without the source fix (i.e. against the
previous, unconditional-resumeSession version of this PR):

$ git show HEAD~1:apps/desktop/src/app/session/hooks/use-session-actions/index.ts \
    > apps/desktop/src/app/session/hooks/use-session-actions/index.ts
$ npx vitest run src/app/session/hooks/use-session-actions.test.tsx

 FAIL  src/app/session/hooks/use-session-actions.test.tsx > branchStoredSession desktop source tagging
   > keeps the current view when branching a different session from the sidebar (does not reintroduce #69750)
AssertionError: expected 'branch-stored' to be 'stored-other'
 Test Files  1 failed (1)
      Tests  1 failed | 57 passed (58)

$ git checkout HEAD -- apps/desktop/src/app/session/hooks/use-session-actions/index.ts

And passes with the fix restored:

$ npx vitest run src/app/session/hooks/use-session-actions.test.tsx
 Test Files  1 passed (1)
      Tests  58 passed (58)

Full session-hooks suite (43 files, 665 tests) also passes:

$ npx vitest run src/app/session
 Test Files  43 passed (43)
      Tests  665 passed (665)

tsc -p apps/desktop --noEmit and eslint on both changed files are clean.

AI assistance disclosure

This change was developed with AI assistance (Claude Code), including
locating the relevant code, implementing the fix, and updating/verifying the
test.

… a tile

forkBranch ended by opening the branch as a session-tile and leaving the
primary selection on the parent (NousResearch#69750). In the default layout there is
no visible tile pane, so branching only added a sidebar row with no
feedback in the main area — and openSessionTile no-ops when the target
is already the selected session, the common case of branching the chat
you're viewing.

Load the branch as the primary session via resumeSession instead, which
reuses the runtime already warm-cached by forkBranch's
ensureSessionState/updateSessionState calls, so it doesn't cost an extra
resume RPC.

Fixes NousResearch#93444
…ssion

forkBranch was unconditionally routing every branched session into the
main pane via resumeSession, including sidebar/background branches of
a session the user isn't currently viewing. That reintroduces the
NousResearch#69750 focus-stealing bug for that path: branching a different session
from the sidebar yanked the active view away from whatever was open.
Only take over the main pane when the branch's parent is the session
already selected; otherwise keep opening it as its own tile.
@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 24, 2026

@zhongwater123 zhongwater123 left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI-generated comment: This comment was produced automatically by AI and may be misleading. Please independently verify the cited evidence.

Verdict
No correctness issue found in the reviewed scope; no author code change is requested.
Author action: The required CI check still needs a rerun or separate baseline fix before merge.
Independent delta: A complementary mutation exercised the selected-parent boundary, and an exact base/head comparison isolated the red required check from this patch.

Incremental evidence

Base/head: 608a56ed7f50926ecfae1db39447b280a0bc4d1e → 2f5be252409ee7c143703e5ea36f55f9436a48e9.

  • The PR already mutates to unconditional resumeSession to protect the background-session path. I instead replaced the final guard with the pre-fix always-open-tile behavior; only the selected-parent regression failed (1 failed, 57 passed: expected branch-stored, received stored-parent). Restoring head returned to 58/58. Together, the two mutations exercise both sides of the guard.
  • Required CI fails in unchanged apps/desktop/src/app/skills/index.test.tsx. Running that file on exact base and exact head produced the same 11/11 failures; the changed session-action file passed 58/58, so I do not attribute the red check to this patch.

Coverage limits

This independent run used Windows 11 / Node 22.18.0. I did not independently rerun the author's 665-test session-hooks suite or a packaged Electron UI. The required check remains red and still has to be rerun or resolved before merge.

@teknium1

Copy link
Copy Markdown
Collaborator

Merged via #93663 with your commits cherry-picked (authorship preserved). Thanks @chelsealong!

@teknium1 teknium1 closed this Aug 24, 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.

desktop: branched session opens only in sidebar, not in main workspace (default layout)

4 participants