feat(sessions): restart a session without losing it - #140
Merged
Merged
Conversation
RestartSessionAsync already did the right thing — tear down the PTY and relaunch from the same ShellSession, keeping Id, group, run commands and sidebar slot, waiting for a Claude process to actually exit first — but it was only reachable from the "this change needs a restart" prompt in the edit dialog. There was no way to just close-and-reopen a session, which is what you want after updating the claude CLI on disk. Adds "Restart" to the per-session context menu, above Sleep / Close, and "Restart (N)…" when several rows are selected. RestartSessionsAsync runs the targets strictly sequentially. Awaiting the restarts in parallel would put several claude.exe instances back on the shared config file at once, which is the exact race the per-session exit wait exists to prevent; _restartInProgress drops an overlapping invocation rather than queueing it for the same reason. Sequential Claude restarts take real time, so N > 1 confirms first and a single restart doesn't. Targets are re-resolved per iteration, since each restart replaces the SessionViewModel for that id and an earlier one in the loop may have failed into dormant. Dormant sessions are filtered out — those want Wake, not Restart. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NsYm9iLc2aRnRDfV2uZRQX
…ions Rounds out the restart action with the three entry points the context menu was missing: - ↻ on the terminal toolbar, between ⚙ and 💤, so the lifecycle buttons (restart / sleep / close) sit together. - "Restart all…" in the group right-click menu, beside Sleep all / Wake all / Close all, disabled with the others when the group has nothing live. - "Restart all…" under Bulk actions in the sidebar quick-menu — the "I just updated the claude CLI" button. Dormant sessions are left alone; they pick the new binary up when woken. All of them route through RestartSessionsAsync, so they inherit the sequential execution and the _restartInProgress guard rather than each growing its own loop. The toolbar button goes through the list overload for the same reason, even though it only ever has one target. Confirmation is now driven by a confirmHeadline parameter instead of a bare count test. Null means "prompt only for 2+ targets, generic wording", which is right for the toolbar button and the per-session menu, where a single Restart should just go. The bulk entries pass a headline naming their scope and therefore always prompt, matching their "Close all…" siblings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NsYm9iLc2aRnRDfV2uZRQX
…starts OnClosing snapshots _vm.Sessions into `all` and disposes exactly that set. Nothing in the restart path consulted _isShuttingDown, so a relaunch landing after that snapshot created a ConPTY child that nothing would ever dispose — an orphaned claude.exe outliving the app. The window existed before this PR, but it was one session wide and only reachable from the edit dialog. A bulk restart is minute-scale and full of await points (a 10s exit wait plus a stagger per Claude session), which makes closing the window part-way through it an ordinary thing to do rather than a corner case. Bail out of the remaining queue instead. The narrower race — the single restart already in flight when OnClosing takes its snapshot — is unchanged and pre-existing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NsYm9iLc2aRnRDfV2uZRQX
Five fixes from a review of #140. The first two are real races; the rest are behaviour the new entry points made reachable enough to matter. The shutdown guard added in ec72fb9 only covered the gap BETWEEN queued restarts, which misses the case that actually bites. RestartSessionCoreAsync removes the VM from _vm.Sessions before its 10s exit wait, so a session that is mid-restart is invisible to OnClosing's `var all = _vm.Sessions.ToList()` snapshot; the continuation then relaunches into an app that has already saved state and disposed everything. Session PTYs use useJobObject: false (only RunInstance passes true), so nothing kills the tree — an orphaned claude.exe outliving the app. Click ↻ on a Claude session, close the window during the exit wait, and you had one. The check now also sits after the waits and before the relaunch, which is the only placement that covers the single-target toolbar path at all. EditSessionAsync called RestartSessionAsync directly and so bypassed _restartInProgress entirely, despite the comment and CLAUDE.md both claiming the flag serialized restarts. Answering "Restart now?" during a bulk restart interleaved two teardown/relaunch cycles and put two claude.exe instances on ~/.claude.json at once — the exact race the sequencing exists to prevent. The guard moves onto the entry points (RestartSessionAsync / RestartSessionsAsync) with the work in an unguarded RestartSessionCoreAsync, since the bulk loop holds the flag across its own iterations and would otherwise reject them. Also: restart now restores ActiveSession, because teardown hands it to LastOrDefault() and LaunchSessionAsync never claims it back — restarting the session you were looking at swapped the visible pane in Single layout and retargeted Ctrl+W / F5 at an unrelated session. The bulk confirmation defaults to No like "Close all…" and now mentions that run commands are stopped. And a guard rejection raises a toast instead of doing nothing at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NsYm9iLc2aRnRDfV2uZRQX
The sidebar quick-menu's Opened handler sets IsEnabled on every other bulk action — Wake all dormant, Sleep all, Close all — but "Restart all…" was added without one, so it stayed enabled unconditionally. With only dormant sessions the Bulk actions submenu is still enabled (liveCount > 0 || dormantCount > 0), so the entry rendered as clickable and then no-opped: RestartSessionsAsync returns on targets.Count == 0 before it confirms. The group context menu's "Restart all…" already does this (restartAll.IsEnabled = liveCount > 0); this is the global one catching up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDsEfog5kVkT5NmX1Ya5be
2 of 11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
There was no way to close-and-reopen a session in place. If you updated the
claudeCLI on disk, your options were to close the session (losing it fromstate.json, and its group and run commands with it) or to sleep and wake it. What you actually want is "same session, new process".The primitive already existed —
RestartSessionAsynctears down the PTY and relaunches from the sameShellSession, keeping Id, group, run commands and sidebar slot, and never entering the recently-closed ring. It was just unreachable except via the "this change needs a restart" prompt in the edit-session dialog. This PR exposes it.What
Five entry points, all routed through a new
RestartSessionsAsyncwrapper:Multi-select resolves through
MainViewModel.ResolveActionTargets, the same as Sleep / Close, and the menu is rebuilt onContextMenuOpeningso the count always matches the live selection.Notes for review
Restarts are strictly sequential, by design.
RestartSessionAsyncwaits for a Claude process to actually exit before starting its replacement. Awaiting those in parallel would put severalclaude.exeinstances back on the shared config file at once — precisely the race that wait exists to prevent. A_restartInProgressflag drops (rather than queues) an overlapping invocation for the same reason. The cost is wall-clock time, which is why the bulk paths confirm first.Confirmation is driven by a
confirmHeadlineparameter, not a count test. Null means "prompt only for 2+ targets, generic wording" — right for the toolbar button and the single-session menu entry, where a Restart should just go. The bulk entries pass a headline naming their scope and therefore always prompt, matchingClose all…. Without that split, "Restart all" on a one-session group would have fired silently.Restarted Claude sessions resume their conversation (
restoring: true, inherited from the existing restart path) — the right default for the update-the-CLI case: new binary, same conversation.Dormant sessions are filtered out at every entry point. They want Wake, and they pick up a new binary when woken anyway.
Testing
Build clean at 0 warnings; 597 unit tests pass.
Verified by build and unit tests only — not driven in a live app, so the ↻ glyph's rendering and the toolbar spacing are unconfirmed on screen. Worth a quick look before merge:
dotnet run --project src/CodeShellManager/CodeShellManager.csproj -- --clean.CLAUDE.mdgains a "Restarting sessions" section under Session Lifecycle. Two pre-existing lifecycle notes (Waiting for a PTY to exit,ClaudeShutdownBudgetMs) moved above the new heading — the new section had otherwise been inserted between them and the numbered list they belong to.🤖 Generated with Claude Code
https://claude.ai/code/session_01NsYm9iLc2aRnRDfV2uZRQX