Skip to content

fix(web): show forked session in sidebar immediately#174

Merged
saucam merged 2 commits into
mainfrom
fix/fork-sidebar-update
Jul 14, 2026
Merged

fix(web): show forked session in sidebar immediately#174
saucam merged 2 commits into
mainfrom
fix/fork-sidebar-update

Conversation

@saucam

@saucam saucam commented Jul 14, 2026

Copy link
Copy Markdown
Owner

Problem

Forking a session from the web UI did nothing visible — the new fork never appeared in the left sidebar and wasn't focused. Reproduced by forking a session (including forking an already-forked session, e.g. "Exelexis (fork)").

Root cause

request() resolves to the daemon's response.ok envelope ({type, requestId, data}), not the bare payload. ForkButton.doFork checked "id" in data on the envelope, but the fork's SessionInfo is nested at .data. So the check was always false → mergeSession/focusSession never fired → the fork never entered the store and the sidebar stayed unchanged.

NewSessionModal has the same dead primary branch but survives via a refreshSessions() fallback; doFork had no fallback, so forks silently vanished.

The existing fork tests mocked request to resolve the unwrapped SessionInfo, which is why they passed while prod was broken.

Fix

  • doFork unwraps .data from the envelope before merging + focusing, with a refreshSessions() fallback for an older daemon / unexpected shape (mirrors NewSessionModal).
  • Fork test mocks now resolve the real response.ok envelope, and the default-fork test asserts the fork lands in the store (getSession("fork-1")) and becomes focused (focusedSession()?.id === "fork-1"), and that the fallback was not taken — so the shape mismatch can't regress.

Scope note (follow-up, not this PR)

A brand-new session reaches other attached clients only via their own session.list refresh — the daemon broadcasts session.info_update only to clients already attached to a session. This is symmetric between create and fork (not a fork regression). A workspace-level "session created" broadcast so a second tab/TUI updates live is a separate enhancement.

Verification

  • web: 220/220 tests pass (32 in SessionControls incl. the new regression assertion).
  • tsc --noEmit: clean.

🤖 Generated with Claude Code

doFork read the result of request() as the bare SessionInfo, but
request() resolves to the daemon's `response.ok` envelope
({type, requestId, data}) — the fork's SessionInfo is at `.data`. So the
`"id" in data` check was always false: mergeSession/focusSession never
fired and the new fork never entered the store, leaving the sidebar
unchanged (this also hit forking an already-forked session).

Unwrap `.data` before merging + focusing, with a refreshSessions()
fallback for an older daemon (mirrors NewSessionModal). Fork test mocks
now resolve the real response.ok envelope and assert the fork lands in
the store and becomes focused, so the shape mismatch can't regress.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

🤖 Gemini code review

This PR fixes a bug where newly created session forks do not appear in the sidebar or get focused immediately. The issue was caused by request() returning an envelope object (response.ok) with the SessionInfo nested inside the data property, whereas the codebase previously expected SessionInfo to be the top-level returned value. This PR fixes the handler to properly extract the data property, adds a fallback to refresh the sessions list for older daemons, and updates the tests to correctly reflect the structure of the API response.

Findings: 🔴 0 · 🟠 0 · 🟡 0 · 🟢 0


Tokens spent · ⬆️ Input: 2,570 · ⬇️ Output: 126 · Σ Total: 3,375
Total may be higher due to thinking token counts.

Exercise the older-daemon path (response.ok with no `.data`) so both
branches of doFork's result handling are covered and codecov patch stays
green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.90%. Comparing base (f4139b7) to head (166bd25).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #174   +/-   ##
=======================================
  Coverage   84.90%   84.90%           
=======================================
  Files         106      106           
  Lines       18678    18678           
=======================================
  Hits        15858    15858           
  Misses       2820     2820           
Flag Coverage Δ
daemon 84.90% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@saucam saucam merged commit 948efe6 into main Jul 14, 2026
4 checks passed
@saucam saucam mentioned this pull request Jul 14, 2026
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