Skip to content

fix(desktop): Branch in new chat drops the question and loses the branched session on restart - #71960

Merged
austinpickett merged 2 commits into
NousResearch:mainfrom
alelpoan:fix/branch-loses-question-and-session-v2
Jul 27, 2026
Merged

fix(desktop): Branch in new chat drops the question and loses the branched session on restart#71960
austinpickett merged 2 commits into
NousResearch:mainfrom
alelpoan:fix/branch-loses-question-and-session-v2

Conversation

@alelpoan

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes two related bugs in the Branch in new chat action on an open, live chat:

  1. Branching from an assistant reply created a new chat containing only that reply, with the preceding user question missing.
  2. The branched chat disappeared after restarting the app, resolving as "session not found" on resume.

Both bugs live in the same code path (branchCurrentSession / forkBranch for an open chat):

  • The history slice used messages.slice(at, at + 1) — exactly the clicked message — discarding everything before it, including the question it answered.
  • Branching called the RPC session.create, which only persists a DB row lazily, on the first prompt sent in that session. A branched chat is normally just read, not typed into, so the row never got created — hence "session not found" on the next app restart. This was confirmed to not be a timing/race issue (waited 30-40s before restart with the same result).

The fix switches open-chat branching to the existing, purpose-built RPC session.branch, which persists the session and its history synchronously. session.branch on the backend gained a count parameter so it can fork history up to a specific message instead of always forking the entire transcript, and its response was extended with the fields the frontend expects (stored_session_id, messages, info).

Branching a stored session from the sidebar (branchStoredSession, no live runtime to target) is unchanged and still uses session.create.

Related Issue

Fixes #

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • tui_gateway/server.py: session.branch handler now accepts an optional count param to truncate the parent's live history to the first N messages instead of always forking the full transcript. Response now also includes stored_session_id, message_count, messages, and info for parity with session.create.
  • apps/desktop/src/app/session/hooks/use-session-actions/index.ts:
    • branchCurrentSession: history slice now always starts at 0 instead of at the clicked message's index, so the question before the clicked reply is preserved.
    • forkBranch: added a sourceSessionId parameter; when present (branching an open live chat) it calls session.branch with { session_id, count } instead of session.create.
    • branchCurrentSession now passes activeSessionIdRef.current as sourceSessionId.
    • branchStoredSession now passes null as sourceSessionId, preserving the old session.create path for sidebar branching.
  • tests/tui_gateway/test_protocol.py: added test_session_branch_with_count_truncates_history, asserting count truncates what gets persisted.
  • apps/desktop/src/app/session/hooks/use-session-actions.test.tsx: extended BranchHarness to expose branchCurrentSession; added a test asserting that branching an open chat from a middle message calls session.branch with the correct session_id/count, not session.create.

How to Test

  1. Open a chat with at least two question/answer turns.
  2. Click Branch in new chat on the first assistant reply (not the last one) — before this fix, the new tab showed only that reply; after the fix it shows the question and the reply.
  3. With the new branched tab open, quit the app entirely and relaunch it — before this fix, the branched tab vanished ("session not found"); after the fix it reopens with its history intact.
  4. Regression check: branch a session from the sidebar (right-click → Branch) — this path is untouched and should behave exactly as before.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Windows 11

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

image image

alelpoan added 2 commits July 26, 2026 15:36
…#issue)

- session.branch on the backend now accepts a count param to truncate
  the parent history to the clicked message, instead of always forking
  the entire transcript. Also returns stored_session_id/messages/info
  so the frontend has parity with session.create's response shape.
- branchCurrentSession (open live chat) now slices history from 0
  instead of from the clicked message index, so the question preceding
  an assistant reply is no longer dropped when branching.
- forkBranch now calls session.branch (not session.create) when
  branching an open live chat, since session.create only persists a DB
  row lazily on first prompt - a branched chat that nobody types into
  never got saved, and vanished as 'session not found' on the next
  app restart. branchStoredSession (branching from the sidebar, no
  live runtime) keeps using session.create as before.
- backend: assert session.branch with a count param only persists the
  first N messages of the live history to the new session.
- frontend: BranchHarness now exposes branchCurrentSession; assert
  branching an open chat from a middle message calls session.branch
  with the parent session id and the correct trimmed count, instead of
  session.create.
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) comp/tui Terminal UI (ui-tui/ + tui_gateway/) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 26, 2026

@austinpickett austinpickett left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Code Review Summary — Hermes Agent

Verdict: Approve. Clean root-cause fix for both bugs, no conflicts, tests pass.

What I verified

  • Root cause on both bugs is correct.
    • Dropped question: branchCurrentSession sliced messages.slice(at, at+1) (only the clicked reply). Changing start to 0 keeps everything up to and including the clicked message, so the preceding question survives. Correct.
    • Vanishes on restart: session.create only persists a DB row lazily on first prompt, so a read-only branch never got saved. Switching the open-live-chat path to session.branch (which persists row + history synchronously) is the right fix, and matches how the TUI /branch already persists.
  • Backend count handling is safe and backward-compatible. if isinstance(count, int) and count > 0: history = history[:count] — omitting count or passing 0/negative preserves the old full-transcript fork, so the existing TUI /branch caller is unaffected.
  • Response parity is exact. session.branch now returns session_id, stored_session_id, message_count, messages (via the shared _history_to_messages), and info (via the shared _session_info, fed the freshly-built agent + branched session) — the same shape session.create returns, so the frontend's single SessionCreateResponse consumer path works for both.
  • Both forkBranch call sites updated correctly. branchCurrentSession passes activeSessionIdRef.current (live → session.branch); branchStoredSession passes null (sidebar → unchanged session.create). No other callers.
  • Tests are non-vacuous. Reverting the history[:count] slice makes test_session_branch_with_count_truncates_history fail on the len(append_calls) == 2 assertion — confirmed it actually guards the fix. Backend branch tests 4/4 pass; frontend suite 37/37 pass; app + electron typecheck clean.

Notes (non-blocking)

  • branched.messages in the response is not consumed by forkBranch (it builds state from the local branchMessages), so messages/message_count are there purely for session.create parity. Harmless and intentional.

Verified in an isolated worktree off the PR head: tests/tui_gateway/test_protocol.py -k branch, use-session-actions.test.tsx, tsc -p . + tsc -p tsconfig.electron.json.

@austinpickett austinpickett left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Code Review Summary — Hermes Agent

Verdict: Approve. Clean root-cause fix for both bugs, no conflicts, tests pass.

What I verified

  • Root cause on both bugs is correct.
    • Dropped question: branchCurrentSession sliced messages.slice(at, at+1) (only the clicked reply). Changing start to 0 keeps everything up to and including the clicked message, so the preceding question survives. Correct.
    • Vanishes on restart: session.create only persists a DB row lazily on first prompt, so a read-only branch never got saved. Switching the open-live-chat path to session.branch (which persists row + history synchronously) is the right fix, and matches how the TUI /branch already persists.
  • Backend count handling is safe and backward-compatible. if isinstance(count, int) and count > 0: history = history[:count] — omitting count or passing 0/negative preserves the old full-transcript fork, so the existing TUI /branch caller is unaffected.
  • Response parity is exact. session.branch now returns session_id, stored_session_id, message_count, messages (via the shared _history_to_messages), and info (via the shared _session_info, fed the freshly-built agent + branched session) — the same shape session.create returns, so the frontend's single SessionCreateResponse consumer path works for both.
  • Both forkBranch call sites updated correctly. branchCurrentSession passes activeSessionIdRef.current (live → session.branch); branchStoredSession passes null (sidebar → unchanged session.create). No other callers.
  • Tests are non-vacuous. Reverting the history[:count] slice makes test_session_branch_with_count_truncates_history fail on the len(append_calls) == 2 assertion — confirmed it actually guards the fix. Backend branch tests 4/4 pass; frontend suite 37/37 pass; app + electron typecheck clean.

Notes (non-blocking)

  • branched.messages in the response is not consumed by forkBranch (it builds state from the local branchMessages), so messages/message_count are there purely for session.create parity. Harmless and intentional.

Verified in an isolated worktree off the PR head: tests/tui_gateway/test_protocol.py -k branch, use-session-actions.test.tsx, tsc -p . + tsc -p tsconfig.electron.json.

@austinpickett
austinpickett merged commit 820a808 into NousResearch:main Jul 27, 2026
50 checks passed
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…nched session on restart (NousResearch#71960)

* fix: Branch in new chat loses the question and the branched session (#issue)

- session.branch on the backend now accepts a count param to truncate
  the parent history to the clicked message, instead of always forking
  the entire transcript. Also returns stored_session_id/messages/info
  so the frontend has parity with session.create's response shape.
- branchCurrentSession (open live chat) now slices history from 0
  instead of from the clicked message index, so the question preceding
  an assistant reply is no longer dropped when branching.
- forkBranch now calls session.branch (not session.create) when
  branching an open live chat, since session.create only persists a DB
  row lazily on first prompt - a branched chat that nobody types into
  never got saved, and vanished as 'session not found' on the next
  app restart. branchStoredSession (branching from the sidebar, no
  live runtime) keeps using session.create as before.

* test: cover session.branch count truncation and open-chat branching

- backend: assert session.branch with a count param only persists the
  first N messages of the live history to the new session.
- frontend: BranchHarness now exposes branchCurrentSession; assert
  branching an open chat from a middle message calls session.branch
  with the parent session id and the correct trimmed count, instead of
  session.create.
33hodl pushed a commit to 33hodl/hermes-agent that referenced this pull request Aug 12, 2026
…nched session on restart (NousResearch#71960)

* fix: Branch in new chat loses the question and the branched session (#issue)

- session.branch on the backend now accepts a count param to truncate
  the parent history to the clicked message, instead of always forking
  the entire transcript. Also returns stored_session_id/messages/info
  so the frontend has parity with session.create's response shape.
- branchCurrentSession (open live chat) now slices history from 0
  instead of from the clicked message index, so the question preceding
  an assistant reply is no longer dropped when branching.
- forkBranch now calls session.branch (not session.create) when
  branching an open live chat, since session.create only persists a DB
  row lazily on first prompt - a branched chat that nobody types into
  never got saved, and vanished as 'session not found' on the next
  app restart. branchStoredSession (branching from the sidebar, no
  live runtime) keeps using session.create as before.

* test: cover session.branch count truncation and open-chat branching

- backend: assert session.branch with a count param only persists the
  first N messages of the live history to the new session.
- frontend: BranchHarness now exposes branchCurrentSession; assert
  branching an open chat from a middle message calls session.branch
  with the parent session id and the correct trimmed count, instead of
  session.create.
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/*) comp/tui Terminal UI (ui-tui/ + tui_gateway/) 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