diff --git a/CLAUDE.md b/CLAUDE.md index 8504c28..7a33cac 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -303,14 +303,39 @@ The page-side `mousedown` handler also calls `fitAddon.fit()`, and the initial f - **Close** (`vm.CloseCommand`) → `MainViewModel.OnSessionCloseRequested` → `vm.Dispose()` + remove from `Sessions` + `SessionManager.RemoveSession()`. Session is gone from `state.json`. - **Sleep** (`SleepSession(vm)`) → `vm.Dispose()` + remove from `Sessions` but **keep** the `ShellSession` in `SessionManager` with `IsDormant = true`. A muted dormant sidebar entry replaces the active one. - **Wake** (`WakeSessionAsync(session)`) → re-runs `LaunchSessionAsync(session, restoring: true)` — same path as restore-on-startup. - - **Restart** (`RestartSessionAsync(vm)`) → sleep-style teardown *without* the dormant bookkeeping, then `LaunchSessionAsync(session, restoring: true, removeOnFailure: false)`. The `ShellSession` stays in `SessionManager` (so Id, group, run commands and sidebar slot survive) and never enters the recently-closed ring. Used by "Edit session…" — see below. Both non-default arguments matter: `restoring: true` makes a Claude session resume rather than start a fresh conversation (matching Wake — a restart tears down the same way, so it must recover the same way), and `removeOnFailure: false` stops a bad edit from *deleting* the session, since `LaunchSessionAsync`'s PTY-failure path calls `SessionManager.RemoveSession` — correct for a session that never started, destructive for a relaunch. If the relaunch fails either way, the session falls back to dormant so the row stays visible and fixable instead of leaving a launching placeholder that never resolves. + - **Restart** (`RestartSessionAsync(vm)`) → sleep-style teardown *without* the dormant bookkeeping, then `LaunchSessionAsync(session, restoring: true, removeOnFailure: false)`. The `ShellSession` stays in `SessionManager` (so Id, group, run commands and sidebar slot survive) and never enters the recently-closed ring. Reachable from the **↻ toolbar button and the "Restart" menu entries** (see "Restarting sessions" below) and from "Edit session…". Both non-default arguments matter: `restoring: true` makes a Claude session resume rather than start a fresh conversation (matching Wake — a restart tears down the same way, so it must recover the same way), and `removeOnFailure: false` stops a bad edit from *deleting* the session, since `LaunchSessionAsync`'s PTY-failure path calls `SessionManager.RemoveSession` — correct for a session that never started, destructive for a relaunch. If the relaunch fails either way, the session falls back to dormant so the row stays visible and fixable instead of leaving a launching placeholder that never resolves. A Claude restart also **waits for the old process to actually exit** (`DisposeAndWaitForExitAsync` + a config quiesce) before relaunching. Without that it recreates the concurrent-config-writer race the launch stagger and the shutdown loop both exist to prevent, and `--resume` can read a session index the outgoing process hasn't finalised. Non-Claude sessions skip the wait — they don't touch that file. +6. On app close: `_vm.SaveStateAsync()` flushes `_sessionManager.Sessions` (live + dormant) to `state.json` (unless `--clean`). + **Waiting for a PTY to exit.** Check `PseudoTerminal.HasExited`, never `IsRunning`. `IsRunning` is `_hProcess != IntPtr.Zero` and the handle is only released in `Dispose`, so it stays true for a child that exited on its own — subscribing to `Exited` for one of those waits out the full timeout for an event that already fired. `HasExited` is latched immediately before `Exited` is raised. This is not academic: combined with `ClaudeShutdownBudgetMs`, two stale panes consumed the entire shutdown budget and every remaining *live* Claude session was then force-disposed with no exit wait — the exact opposite of what the budget was for. **`ClaudeShutdownBudgetMs` is sized from measurement (30s).** The original 15s came from the only data available at the time — idle sessions exiting in 460–770ms. Real shutdowns of *busy* sessions measure **2.3–4.7s each**, so nine of them need roughly 30s, and 15s meant force-disposing more than half the fleet on an ordinary close. Waiting is the right trade: a clean exit lets Claude finish writing its config, and `ShutdownOverlay` is already on screen telling the user why. The budget exists to bound a genuinely wedged session, not to hurry a healthy one. If you shrink it, re-measure `exit=` in `crash.log` first — the summary line alone can't distinguish "slow exits" from "waits that aren't returning". -6. On app close: `_vm.SaveStateAsync()` flushes `_sessionManager.Sessions` (live + dormant) to `state.json` (unless `--clean`). + +### Restarting sessions + +Restart closes and reopens a session in place. The use case is picking up a new build of the CLI — `claude` updated on disk — without losing the session, its group, its run commands or its conversation. Five entry points, all routed through `RestartSessionsAsync`: + +| Entry point | Scope | Confirms? | +|---|---|---| +| **↻** on the terminal toolbar (between ⚙ and 💤) | the one session | no | +| **"Restart"** in the per-session right-click menu (above Sleep / Close) | the one session | no | +| **"Restart (N)…"** — same menu, with 2+ sidebar rows selected | the selection | yes | +| **"Restart all…"** in the group right-click menu, next to Sleep all / Close all | live sessions in that group | yes | +| **"Restart all…"** under Bulk actions in the sidebar quick-menu | every live session | yes | + +Multi-select resolves through `MainViewModel.ResolveActionTargets`, exactly like Sleep / Close. The per-session menu is rebuilt on `ContextMenuOpening`, so the count always reflects the live selection. + +Confirmation is driven by the `confirmHeadline` parameter rather than a count test: **null** (the toolbar button and the per-session menu) prompts only for 2+ targets with a generic headline, so a single "Restart" just goes; the bulk entry points pass a scope-naming headline and therefore always prompt, matching their "Close all…" siblings. The prompt defaults to **No**, like "Close all…" — a restart terminates every running process *and* stops in-flight run commands (`vm.Runner.StopAll()`), and a stray Enter shouldn't start a minute-scale operation with no stop button. Dormant sessions are filtered out everywhere — they want Wake, and they pick up a new binary when woken anyway. + +**The guard lives on the entry points, not the restart itself.** `RestartSessionCoreAsync` does the work and holds no guard; `RestartSessionAsync` (single session — used by "Edit session…") and `RestartSessionsAsync` (everything else) each take `_restartInProgress` and call the core. The split exists because the bulk loop holds the flag across all its iterations, so a guard inside the core would reject the loop's own work. Don't call the core from anywhere that doesn't already hold the flag: the edit dialog used to call the restart directly, which meant answering "Restart now?" during a bulk restart put two `claude.exe` instances on `~/.claude.json` at once. A rejected invocation raises a toast rather than failing silently, since a bulk restart runs long enough that a dead-feeling button reads as a bug. + +**Two shutdown checks, and both are needed.** `RestartSessionsAsync` breaks out of its queue when `_isShuttingDown`, and `RestartSessionCoreAsync` checks again *after* its teardown waits and before `LaunchSessionAsync`. The second one is the load-bearing one: the core removes the VM from `_vm.Sessions` **before** the 10s exit wait, so a session that is mid-restart is invisible to `OnClosing`'s `var all = _vm.Sessions.ToList()` snapshot. Relaunching past that point starts a ConPTY child nothing will ever dispose — and session PTYs are started with `useJobObject: false` (only `RunInstance` passes `true`), so no job object cleans up the tree either. The result is an orphaned `claude.exe` outliving the app. Returning early leaves the `ShellSession` in the `SessionManager`, so it is still in `state.json` and simply starts again next launch. + +**Restart restores focus.** The teardown hands `ActiveSession` to `_vm.Sessions.LastOrDefault()` and `LaunchSessionAsync` never claims it back, so the core captures `wasActive` up front and re-resolves the session by id afterwards (the relaunch builds a *new* `SessionViewModel`). Without it, restarting the session you're looking at swaps the visible pane in `LayoutMode.Single` and silently retargets `Ctrl+W` / `F5` at an unrelated session. + +`RestartSessionsAsync` runs its targets **strictly sequentially** — `RestartSessionAsync` waits for each Claude process to exit before starting its replacement, and awaiting those in parallel would put several `claude.exe` instances back on the shared config file at once, which is the race the wait exists to prevent. A `_restartInProgress` flag drops (rather than queues) an overlapping invocation for the same reason — see the guard note above. That sequencing is also why the bulk entry points confirm: restarting a fleet of Claude sessions is a minute-scale operation, not an instant one. Each target is re-resolved per iteration, since every restart replaces the `SessionViewModel` for that id and an earlier one in the loop may have failed into dormant. ## Editing a Session's Configuration diff --git a/src/CodeShellManager/MainWindow.xaml.cs b/src/CodeShellManager/MainWindow.xaml.cs index 9c4f37f..7ef4be3 100644 --- a/src/CodeShellManager/MainWindow.xaml.cs +++ b/src/CodeShellManager/MainWindow.xaml.cs @@ -105,6 +105,10 @@ private enum SortField { None, Name, Folder, LastActive, Branch, Dirty, Repo } private bool _isShuttingDown = false; private bool _shutdownComplete = false; + // Guards RestartSessionsAsync so two restart loops can't interleave — see the comment + // there for why restarts have to stay strictly sequential. + private bool _restartInProgress = false; + public MainWindow() { InitializeComponent(); @@ -2333,6 +2337,16 @@ private System.Windows.Controls.ContextMenu BuildSidebarQuickMenu() }; bulkActions.Items.Add(wakeAllDormant); + // Restart every live session — the "I just updated the claude CLI" button. + // Dormant sessions are left alone; they pick the new binary up when woken. + var restartAllGlobal = new System.Windows.Controls.MenuItem { Header = "Restart all…" }; + restartAllGlobal.Click += async (_, _) => + { + var ids = _vm.Sessions.Select(v => v.Id).ToList(); + await RestartSessionsAsync(ids, $"Restart all {ids.Count} live session(s)?"); + }; + bulkActions.Items.Add(restartAllGlobal); + var sleepAllGlobal = new System.Windows.Controls.MenuItem { Header = "Sleep all" }; sleepAllGlobal.Click += (_, _) => { @@ -2437,6 +2451,7 @@ private System.Windows.Controls.ContextMenu BuildSidebarQuickMenu() int liveCount = _vm.Sessions.Count; int dormantCount = _sessionManager.Sessions.Count(s => s.IsDormant); wakeAllDormant.IsEnabled = dormantCount > 0; + restartAllGlobal.IsEnabled = liveCount > 0; sleepAllGlobal.IsEnabled = liveCount > 0; closeAllGlobal.IsEnabled = liveCount > 0; bulkActions.IsEnabled = liveCount > 0 || dormantCount > 0; @@ -2994,6 +3009,16 @@ private void AddGroupBulkActionItems(System.Windows.Controls.ContextMenu menu, s }; menu.Items.Add(remoteControl); + var restartAll = new System.Windows.Controls.MenuItem { Header = "Restart all…" }; + restartAll.Click += async (_, _) => + { + var ids = _vm.Sessions.Where(v => v.Session.GroupId == groupId) + .Select(v => v.Id).ToList(); + await RestartSessionsAsync(ids, + $"Restart {ids.Count} session(s) in group '{groupName}'?"); + }; + menu.Items.Add(restartAll); + var sleepAll = new System.Windows.Controls.MenuItem { Header = "Sleep all" }; sleepAll.Click += (_, _) => { @@ -3034,6 +3059,7 @@ private void AddGroupBulkActionItems(System.Windows.Controls.ContextMenu menu, s int liveCount = _vm.Sessions.Count(v => v.Session.GroupId == groupId); int dormantCount = _sessionManager.Sessions.Count(s => s.GroupId == groupId && s.IsDormant); remoteControl.IsEnabled = liveCount > 0; + restartAll.IsEnabled = liveCount > 0; sleepAll.IsEnabled = liveCount > 0; wakeAll.IsEnabled = dormantCount > 0; closeAll.IsEnabled = liveCount > 0; @@ -3359,6 +3385,18 @@ private System.Windows.Controls.ContextMenu BuildSessionContextMenu(SessionViewM menu.Items.Add(removeFrom); menu.Items.Add(new System.Windows.Controls.Separator()); + + // Restart — close and reopen in place, keeping the session's Id, group, run commands + // and sidebar slot. The reason this exists: picking up a new build of the CLI (a + // `claude` update) without losing the session or its conversation. + var restartItem = new System.Windows.Controls.MenuItem + { + Header = isMulti ? $"Restart{countSuffix}…" : "Restart", + ToolTip = "Stop and relaunch the terminal; Claude sessions resume their conversation", + }; + restartItem.Click += async (_, _) => await RestartSessionsAsync(targetIds); + menu.Items.Add(restartItem); + var sleepItem = new System.Windows.Controls.MenuItem { Header = $"Sleep{countSuffix}" }; sleepItem.Click += (_, _) => { @@ -4320,6 +4358,23 @@ private Border BuildTerminalWrapper(SessionViewModel vm, WebView2 webView) }; editBtn.Click += async (_, _) => await EditSessionAsync(vm); + // Restart — stop and relaunch this session's terminal in place. Routed through + // RestartSessionsAsync (rather than RestartSessionAsync directly) so it shares the + // re-entrancy guard with the menu paths; a single target skips the confirmation. + var restartBtn = new WpfButton + { + Content = "↻", + ToolTip = "Restart session (stop and relaunch the terminal)", + Background = Brushes.Transparent, + BorderThickness = new Thickness(0), + Foreground = new SolidColorBrush(Color.FromRgb(0xa6, 0xad, 0xc8)), + FontSize = 12, + Cursor = System.Windows.Input.Cursors.Hand, + Padding = new Thickness(4, 2, 4, 2), + Margin = new Thickness(0, 0, 4, 0) + }; + restartBtn.Click += async (_, _) => await RestartSessionsAsync(new[] { vm.Id }); + // Sleep (dormant) button — keeps the session in the sidebar but stops the PTY var sleepBtn = new WpfButton { @@ -4359,6 +4414,7 @@ private Border BuildTerminalWrapper(SessionViewModel vm, WebView2 webView) DockPanel.SetDock(toolbarPsBtn, Dock.Right); DockPanel.SetDock(notesBtn, Dock.Right); DockPanel.SetDock(sleepBtn, Dock.Right); + DockPanel.SetDock(restartBtn, Dock.Right); DockPanel.SetDock(editBtn, Dock.Right); DockPanel.SetDock(chevronBtn, Dock.Right); DockPanel.SetDock(playBtn, Dock.Right); @@ -4372,6 +4428,7 @@ private Border BuildTerminalWrapper(SessionViewModel vm, WebView2 webView) toolbarContent.Children.Add(toolbarPsBtn); toolbarContent.Children.Add(notesBtn); toolbarContent.Children.Add(sleepBtn); + toolbarContent.Children.Add(restartBtn); toolbarContent.Children.Add(editBtn); toolbarContent.Children.Add(chevronBtn); toolbarContent.Children.Add(playBtn); @@ -4721,6 +4778,84 @@ private void EditDormantSession(ShellSession session) _ = _vm.SaveStateAsync(); } + /// + /// Restarts every session in , one after another. Sequential + /// by design: waits for a Claude process to actually + /// exit before starting its replacement, and running those waits concurrently would put + /// several claude.exe instances back on the shared config file at once — the race the + /// wait exists to prevent. The cost is time, so a multi-target restart asks first. + /// + /// + /// First line of the confirmation prompt. When null, the prompt is shown only for 2+ + /// targets with a generic headline — right for the context menu, where a single + /// "Restart" should just go. The bulk entry points ("Restart all") pass a scope-naming + /// headline instead and so always confirm, matching their "Close all…" siblings. + /// + private async Task RestartSessionsAsync( + IReadOnlyList sessionIds, string? confirmHeadline = null) + { + // Overlapping restart loops would defeat the staggering above just as thoroughly as + // a parallel loop would, so a second invocation is dropped rather than queued. The + // toast is the only feedback there would otherwise be: a bulk restart runs for + // minutes, and a click that does nothing at all just reads as a broken button. + if (_restartInProgress) + { + Services.ToastHelper.Show("Restart busy", + "Another restart is still running — try again when it finishes."); + return; + } + + var targets = sessionIds + .Select(id => _vm.Sessions.FirstOrDefault(s => s.Id == id)) + .Where(v => v != null) + .Select(v => v!.Id) + .ToList(); + if (targets.Count == 0) return; + + if (confirmHeadline != null || targets.Count > 1) + { + string headline = confirmHeadline ?? $"Restart {targets.Count} sessions?"; + // Defaults to No, like "Close all…" — this terminates every running process and + // any in-flight run commands, and a stray Enter shouldn't start a minute-scale + // operation there's no way to stop. + var r = MessageBox.Show(this, + headline + Environment.NewLine + Environment.NewLine + + "Each running process is terminated and relaunched in turn, and any running " + + "session commands are stopped. Claude sessions resume their conversation. " + + "They restart one at a time, so this can take a while.", + "Restart sessions", MessageBoxButton.YesNo, MessageBoxImage.Question, + MessageBoxResult.No); + if (r != MessageBoxResult.Yes) return; + } + + _restartInProgress = true; + try + { + foreach (var id in targets) + { + // Abandon the rest of the queue once the window is closing. OnClosing + // snapshots _vm.Sessions and disposes exactly that set, so a relaunch that + // lands after the snapshot spawns a ConPTY child nothing will ever dispose — + // an orphaned claude.exe outliving the app. A bulk restart is minute-scale + // and full of await points, which makes closing the window part-way through + // it an ordinary thing to do rather than a corner case. + if (_isShuttingDown) break; + + // Re-resolve per iteration: each restart replaces the SessionViewModel for + // that id, and an earlier one in this loop may have failed into dormant. + // + // Core, not RestartSessionAsync: this loop already holds _restartInProgress, + // and the guarded entry point would reject its own iterations. + var vm = _vm.Sessions.FirstOrDefault(s => s.Id == id); + if (vm != null) await RestartSessionCoreAsync(vm); + } + } + finally + { + _restartInProgress = false; + } + } + /// /// Tears down a live session's PTY/terminal and relaunches it from the same /// — the sleep/wake teardown without the dormant bookkeeping. @@ -4735,8 +4870,33 @@ private void EditDormantSession(ShellSession session) /// process needs. Null means "use the session's current command". /// private async Task RestartSessionAsync(SessionViewModel vm, string? launchedCommand = null) + { + // The single-session entry point, so it carries the guard. RestartSessionsAsync + // holds the same flag across its whole loop and calls the core directly, which is + // why the guard can't live in the core itself. + if (_restartInProgress) + { + Services.ToastHelper.Show("Restart busy", + "Another restart is still running — try again when it finishes."); + return; + } + _restartInProgress = true; + try { await RestartSessionCoreAsync(vm, launchedCommand); } + finally { _restartInProgress = false; } + } + + /// + /// The restart itself, with no re-entrancy guard of its own — every caller must already + /// hold . + /// + private async Task RestartSessionCoreAsync(SessionViewModel vm, string? launchedCommand = null) { var session = vm.Session; + // Restored after the relaunch: the teardown below hands ActiveSession to whatever + // sits last in the list, and LaunchSessionAsync never claims it back. Without this, + // restarting the session you're looking at swaps the visible pane in Single layout + // and silently retargets Ctrl+W / F5 at an unrelated session. + bool wasActive = ReferenceEquals(_vm.ActiveSession, vm); vm.Runner.StopAll(); if (_selectionAnchorId == vm.Id) _selectionAnchorId = null; @@ -4793,6 +4953,22 @@ private async Task RestartSessionAsync(SessionViewModel vm, string? launchedComm vm.Dispose(); } + // Don't relaunch into a closing app. This has to sit AFTER the waits above, not + // just between queued restarts: the VM was removed from _vm.Sessions before them, + // so OnClosing's `var all = _vm.Sessions.ToList()` snapshot can't see a session + // that is mid-restart. Relaunching past that point starts a ConPTY child that + // nothing will ever dispose — and session PTYs (unlike run commands) are started + // with useJobObject: false, so no job object kills the tree either. The result is + // an orphaned claude.exe outliving the app. + // + // Returning here leaves the ShellSession in the SessionManager, so it is still in + // state.json and simply starts again on the next launch. + if (_isShuttingDown) + { + Log($"Restart of '{session.Name}' abandoned — app is shutting down."); + return; + } + try { // restoring: true so a Claude session resumes its conversation instead of @@ -4825,6 +5001,19 @@ private async Task RestartSessionAsync(SessionViewModel vm, string? launchedComm AddDormantSidebarItem(session); RebuildSidebarOrder(); _ = _vm.SaveStateAsync(); + return; + } + + // Give focus back to the session that had it. The relaunch produced a NEW + // SessionViewModel, so this re-resolves by id rather than reusing the old one. + if (wasActive) + { + var revived = _vm.Sessions.FirstOrDefault(s => s.Id == session.Id); + if (revived != null) + { + _vm.ActiveSession = revived; + UpdateSidebarActiveState(); + } } }