From 7665ee776fe44ff0c65edb36e382298d8a5eecbb Mon Sep 17 00:00:00 2001 From: Allan Thraen Date: Sun, 6 Sep 2026 15:37:40 +0200 Subject: [PATCH] feat(ux): determinate restore rail, and a shutdown board that is actually visible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two waiting states, treated differently on purpose. STARTUP — restore rail A 25-session restore runs ~131s with per-session launch cost swinging 12x (0.6s to 10.9s, measured from crash.log). There is no steady rate to extrapolate from, so an indeterminate spinner reads identically at session 2 and session 22. RestoreRail (2px, under the toolbar) plus a "7 / 25 restoring" pill give the aggregate k-of-N; per-session state stays on the existing placeholder sidebar rows. Restore does NOT block the user — placeholder rows mean you can click into a session the moment it lands — so this stays deliberately out of the way. Nothing modal. The counter advances after the try/catch, not inside it, so a session that fails to restore still moves the rail. Otherwise one bad session strands it short of full, which reads as exactly the hang it is meant to rule out. SHUTDOWN — the overlay was invisible, and that is the real bug Reported as "more like 1 line - thats hard to see. no spinner or anything". The markup looked correct: a Grid sibling with Panel.ZIndex="100" and a Storyboard-rotated arc dead centre. WebView2 is an HwndHost. A native child window is composited by the OS on top of everything WPF renders, and Panel.ZIndex does not enter into it. The overlay was being drawn BEHIND every terminal pane. What reached the user was scrim leaking through the few-pixel gaps between panes — hence a stray line and no spinner, because the spinner sat centred over a pane. The codebase already documents HwndHost for input (#108, three separate call sites). Nobody had written down the output half, which is the more expensive one to rediscover because the code looks right. Fix: collapse TerminalGrid before showing the overlay. Safe — every pane is about to be disposed, and a frozen terminal is worth nothing mid-shutdown. With it visible, it earns being more than a spinner. The board names every session and ticks it off (· pending, ◐ closing, ✓ clean, ⨯ force-disposed), with elapsed time per session, a k/N bar, and a budget bar against ClaudeShutdownBudgetMs. Force-disposed sessions are marked rather than hidden: that is the case a user most wants to see and it used to happen completely silently. ShutdownHint escalates with elapsed time to explain why the wait is long. Explanatory, not jokey — it has to read well on the four-hundredth shutdown, which is the argument against the "be patient" copy I originally sketched. Also here: - Board list scrollbar restyled (scoped to that ScrollViewer). Any setup past ~14 sessions scrolls, so the stock system-coloured bar was on screen for most shutdowns, not an edge case. - Board skipped entirely at zero sessions, so --clean runs don't flash a full-window "0 / 0". - Corrected a stale comment claiming the adaptive config gate is "still used at SHUTDOWN". #117 removed it from both paths. Verified: layout rendered offscreen via RenderTargetBitmap against the real markup extracted from MainWindow.xaml (9 rows and 27 rows, to confirm the card caps and the list scrolls). The airspace fix itself needs a live run — it cannot be proven headlessly, since it is about OS compositing. 304/304 tests pass, 0 warnings. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NDsEfog5kVkT5NmX1Ya5be --- CLAUDE.md | 45 ++++- src/CodeShellManager/MainWindow.xaml | 156 ++++++++++++--- src/CodeShellManager/MainWindow.xaml.cs | 244 ++++++++++++++++++++++-- 3 files changed, 405 insertions(+), 40 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 100008c..32bbce9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -143,7 +143,7 @@ The accent comes from the **live VM**, not the `Border.Tag` stashed at build tim Both (2) and (3) **must come from the page**, and this is the part that is easy to get wrong twice: -- **WebView2 is an `HwndHost`.** Mouse input landing on hosted native content raises **no** WPF routed events, tunnelling `Preview*` ones included. A `PreviewMouseLeftButtonDown` on the host Border only ever fires for the thin ring around the terminal (#108). +- **WebView2 is an `HwndHost`.** Mouse input landing on hosted native content raises **no** WPF routed events, tunnelling `Preview*` ones included. A `PreviewMouseLeftButtonDown` on the host Border only ever fires for the thin ring around the terminal (#108). The same fact bites on the way out too — WPF content cannot be *drawn* over a pane either, whatever `Panel.ZIndex` says. See "Session Spinners". - **xterm's `onData` is not "the user typed".** It also carries replies the terminal generates itself — device attributes (`ESC[?1;2c`), cursor-position reports, OSC colour replies, focus in/out (`ESC[I`/`ESC[O`) — plus mouse reports when the app enables tracking. Filtering those by inspecting the bytes cannot work; a device-attribute reply is not distinguishable from typing by shape. xterm knows internally (`triggerDataEvent`'s `wasUserInput`) but does not expose it on `onData`. `onKey` is the only honest source (#106). The page-side `mousedown` handler also calls `fitAddon.fit()`, and the initial fit is re-run on `document.fonts.ready`. xterm derives its column count from the *measured advance width* of the font, so a fit that runs before the font loads computes the wrong `cols` and tells the PTY a width that doesn't match what is drawn — text then overlaps mid-line. The `ResizeObserver` cannot catch that, because the element size never changed, only the glyph metrics (#113). @@ -342,9 +342,46 @@ Two overlays cover launch and shutdown so the user sees progress instead of a bl **Launch overlay (per session)** lives in `Assets/terminal.html` and `Assets/terminal-transparent.html` as a CSS-animated rotating SVG arc with a phase label. Visible by default; `TerminalBridge` posts `setBootState` after `NavigationCompleted` (label = `Starting {cmd}…` for local, `Connecting to {host}…` for SSH; accent = session color) and `bootDone` on the first PTY byte (via `OnPtyData → PostBootDoneIfNeeded`, race-safe via `Interlocked.CompareExchange`). An 8-second fallback timer scheduled in `NavCompleted` also calls `PostBootDoneIfNeeded` so silent sessions and slow SSH handshakes don't lock the user out of the pane. -**Shutdown overlay (app-level)** is a `Grid x:Name="ShutdownOverlay"` on `MainWindow.xaml` with a `Storyboard`-rotated `Path`. `OnClosing` shows it then `await Dispatcher.InvokeAsync(() => {}, DispatcherPriority.Background)` to force a render pass before the existing synchronous session-disposal loop blocks the UI thread. - -Full design: `docs/superpowers/specs/2026-05-16-session-spinners-design.md`. +**You cannot draw WPF content over a terminal pane.** WebView2 is an `HwndHost`, and a +native child window is composited by the OS *on top of* everything WPF renders — +`Panel.ZIndex` does not enter into it. This is the same `HwndHost` fact recorded under +"What makes a session active", but for **output** rather than input, and it is the more +expensive half to rediscover: the code looks correct, the overlay is genuinely in the tree +with `Panel.ZIndex="100"`, and it simply does not appear. + +The original centred shutdown spinner was invisible for exactly this reason. All the user +ever saw was scrim leaking through the few-pixel gaps *between* panes — reported, fairly, as +"more like 1 line, hard to see, no spinner". Anything full-window must therefore either +collapse `TerminalGrid` first (what `OnClosing` does) or live in the toolbar/sidebar chrome, +which no `HwndHost` covers. + +**Restore rail (startup, app-level)** — `RestoreRail` (a 2px `ProgressBar`, `FlatBar` style) +docked under the toolbar plus a `RestorePill` counter in the toolbar's right stack, both +driven by `SetRestoreProgress(done, total)` from the `OnLoaded` restore loop and hidden +outside it. Determinate on purpose: a 25-session restore runs ~131s with per-session cost +swinging 12×, so there is no rate to extrapolate and a spinner reads identically at session +2 and session 22. Placed in the toolbar because that is above the airspace problem. + +The counter advances *after* the `try`/`catch` around `LaunchSessionAsync`, so a session that +fails to restore still moves the rail — otherwise one bad session strands it short of full, +which reads as a hang. + +**Shutdown board (app-level)** — `ShutdownOverlay` is now a card listing every session with a +per-row glyph (`·` pending → `◐` closing → `✓` clean / `⨯` force-disposed), elapsed time, an +overall `k / N` bar, and a budget bar running against `ClaudeShutdownBudgetMs`. Built by +`BuildShutdownBoard`, updated in place by `MarkShutdownRow` / `SetShutdownProgress` / +`SetShutdownBudget`. + +Force-disposed sessions are **marked, not hidden** — that is the case a user most wants to +see, and it used to happen silently. `ShutdownHint` escalates with elapsed time to explain +*why* the wait is long; keep it explanatory rather than jokey, since it has to still read +well on the four-hundredth shutdown. The board is skipped entirely when there are no +sessions, so `--clean` runs don't get a full-window flash of "0 / 0". + +Full design: `docs/superpowers/specs/2026-05-16-session-spinners-design.md`. Option +comparison behind the current design: the "Waiting States" artifact (Quiet Rail for startup, +Restore Board for shutdown — the two paths deliberately differ, because restore does not +block the user and shutdown does). ## Search diff --git a/src/CodeShellManager/MainWindow.xaml b/src/CodeShellManager/MainWindow.xaml index e602ed5..23d711d 100644 --- a/src/CodeShellManager/MainWindow.xaml +++ b/src/CodeShellManager/MainWindow.xaml @@ -64,6 +64,28 @@ + + + @@ -91,6 +113,16 @@ + + + + + @@ -152,6 +184,19 @@ + + + - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/CodeShellManager/MainWindow.xaml.cs b/src/CodeShellManager/MainWindow.xaml.cs index 19264e2..de82ffc 100644 --- a/src/CodeShellManager/MainWindow.xaml.cs +++ b/src/CodeShellManager/MainWindow.xaml.cs @@ -313,6 +313,15 @@ private async void OnLoaded(object sender, RoutedEventArgs e) // Wall clock for the whole restore, so per-session timings are comparable and // the total is visible in crash.log (issue #82). var restoreClock = System.Diagnostics.Stopwatch.StartNew(); + + // Determinate restore progress. A 25-session restore runs ~131s with + // per-session cost swinging 12x, so there is no rate to extrapolate from and + // an indeterminate spinner reads the same at session 2 as at session 22. + // The rail and the pill are the only aggregate signal; per-session state stays + // on the placeholder sidebar rows. + int restoreTotal = saved.Count(x => !x.IsDormant), restoreDone = 0; + SetRestoreProgress(restoreDone, restoreTotal); + foreach (var s in saved) { if (s.IsDormant) continue; @@ -330,9 +339,10 @@ private async void OnLoaded(object sender, RoutedEventArgs e) // with more machinery. Predictable beats occasionally-clever on a path // the user waits through at every launch. // - // The gate is still used at SHUTDOWN, where the machine is quiet and it - // measures a consistent ~304ms against this same flat 1000ms — see - // OnClosing. Different contention, different answer. + // #117 removed it from the shutdown path too. The reasoning for keeping it + // there — "the machine is quiet at shutdown, so polling is reliable" — was + // falsified by measurement: a real run logged cfgSettle=8731ms against a + // 1000ms cap. Same disease, same fix. Both paths now use a flat delay. long gateStart = restoreClock.ElapsedMilliseconds; if (isClaude && lastWasClaude && staggerMs > 0) await Task.Delay(staggerMs); @@ -359,8 +369,14 @@ private async void OnLoaded(object sender, RoutedEventArgs e) $"launch={restoreClock.ElapsedMilliseconds - launchStart}ms " + $"total={restoreClock.ElapsedMilliseconds}ms"); + // Counted here, not in the try, so a session that failed to restore still + // advances the rail — otherwise one bad session strands it short of full + // and it reads as a hang. + SetRestoreProgress(++restoreDone, restoreTotal); + lastWasClaude = isClaude; } + SetRestoreProgress(restoreTotal, restoreTotal, finished: true); if (webView2AccessDenied.Count > 0) { MessageBox.Show( @@ -381,6 +397,166 @@ private async void OnLoaded(object sender, RoutedEventArgs e) } } + /// + /// Drives the restore rail + toolbar pill. Both are hidden outside a restore, and + /// hidden entirely when there is nothing to restore — a rail that flashes full for + /// one frame on a single-session start is noise, not feedback. + /// + private void SetRestoreProgress(int done, int total, bool finished = false) + { + if (total <= 0 || finished) + { + RestoreRail.Visibility = Visibility.Collapsed; + RestorePill.Visibility = Visibility.Collapsed; + return; + } + + RestoreRail.Maximum = total; + RestoreRail.Value = done; + RestoreRail.Visibility = Visibility.Visible; + RestorePillText.Text = $"{done} / {total} restoring"; + RestorePill.Visibility = Visibility.Visible; + } + + // ── Shutdown board ────────────────────────────────────────────────────── + // Row handles, so each session's line can be updated in place as it closes. + private readonly Dictionary _shutdownRows = new(); + + /// Builds one row per session, all pending, in disposal order. + private void BuildShutdownBoard(IReadOnlyList sessions) + { + ShutdownList.Children.Clear(); + _shutdownRows.Clear(); + + foreach (var vm in sessions) + { + var grid = new Grid { Margin = new Thickness(0, 1, 0, 1) }; + grid.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(3) }); + grid.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(18) }); + grid.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) }); + grid.ColumnDefinitions.Add(new ColumnDefinition { Width = GridLength.Auto }); + + var accent = (Color)ColorConverter.ConvertFromString(vm.AccentColor ?? "#6c7086"); + var stripe = new Border + { + Background = new SolidColorBrush(Color.FromArgb(0x99, accent.R, accent.G, accent.B)), + CornerRadius = new CornerRadius(2), + Height = 14 + }; + Grid.SetColumn(stripe, 0); + + var glyph = new TextBlock + { + Text = "·", + Foreground = new SolidColorBrush(Color.FromRgb(0x6c, 0x70, 0x86)), + FontFamily = new FontFamily("Consolas"), + FontSize = 11, + HorizontalAlignment = HorizontalAlignment.Center, + VerticalAlignment = VerticalAlignment.Center + }; + Grid.SetColumn(glyph, 1); + + var name = new TextBlock + { + Text = string.IsNullOrWhiteSpace(vm.Name) ? vm.Command : vm.Name, + Foreground = new SolidColorBrush(Color.FromRgb(0x6c, 0x70, 0x86)), + FontFamily = new FontFamily("Segoe UI"), + FontSize = 11.5, + TextTrimming = TextTrimming.CharacterEllipsis, + VerticalAlignment = VerticalAlignment.Center, + Margin = new Thickness(2, 0, 8, 0) + }; + Grid.SetColumn(name, 2); + + var time = new TextBlock + { + Text = "", + Foreground = new SolidColorBrush(Color.FromRgb(0x58, 0x5b, 0x70)), + FontFamily = new FontFamily("Consolas"), + FontSize = 10, + VerticalAlignment = VerticalAlignment.Center + }; + Grid.SetColumn(time, 3); + + grid.Children.Add(stripe); + grid.Children.Add(glyph); + grid.Children.Add(name); + grid.Children.Add(time); + + var row = new Border + { + Padding = new Thickness(6, 3, 8, 3), + CornerRadius = new CornerRadius(4), + Background = Brushes.Transparent, + Child = grid + }; + + ShutdownList.Children.Add(row); + _shutdownRows[vm.Id] = (row, glyph, time); + // Name brush is shared with the glyph lookup below via the row's Tag so + // MarkShutdownRow can brighten it without re-walking the visual tree. + row.Tag = name; + } + + SetShutdownProgress(0, sessions.Count); + SetShutdownBudget(0); + } + + /// Updates one session's row in place. Unknown ids are ignored. + private void MarkShutdownRow(string sessionId, string glyph, string hex, string? time, bool active = false) + { + if (!_shutdownRows.TryGetValue(sessionId, out var r)) return; + + var brush = new SolidColorBrush((Color)ColorConverter.ConvertFromString(hex)); + r.Glyph.Text = glyph; + r.Glyph.Foreground = brush; + if (time != null) r.Time.Text = time; + + r.Row.Background = active + ? new SolidColorBrush(Color.FromRgb(0x25, 0x25, 0x39)) + : Brushes.Transparent; + + if (r.Row.Tag is TextBlock name) + name.Foreground = new SolidColorBrush(active + ? Color.FromRgb(0xcd, 0xd6, 0xf4) + : Color.FromRgb(0xa6, 0xad, 0xc8)); + + // Keep the session being waited on visible in a 25-row list. + if (active) r.Row.BringIntoView(); + } + + private void SetShutdownProgress(int done, int total) + { + ShutdownProgress.Maximum = Math.Max(1, total); + ShutdownProgress.Value = done; + ShutdownCount.Text = $"{done} / {total}"; + } + + /// + /// Draws the elapsed share of , and escalates the + /// hint line with time. The escalation explains *why* the wait is long rather than + /// being chatty — it has to still read well on the four-hundredth shutdown. + /// + private void SetShutdownBudget(long elapsedMs) + { + double used = Math.Min(1.0, elapsedMs / (double)ClaudeShutdownBudgetMs); + ShutdownBudgetBar.Value = used; + ShutdownBudgetBar.Foreground = new SolidColorBrush( + used >= 1.0 ? Color.FromRgb(0xf3, 0x8b, 0xa8) + : used > 0.6 ? Color.FromRgb(0xfa, 0xb3, 0x87) + : Color.FromRgb(0xa6, 0xe3, 0xa1)); + + double left = (ClaudeShutdownBudgetMs - elapsedMs) / 1000.0; + ShutdownBudgetLeft.Text = left <= 0 ? "budget spent" : $"{left:0}s left"; + + ShutdownHint.Text = elapsedMs switch + { + < 8000 => "Letting each session exit cleanly…", + < 20000 => "Claude writes its session state on exit — worth the wait.", + _ => "Some sessions are slow to finish. Closing them shortly." + }; + } + // Detects WebView2 user-data folder access-denied, which surfaces as // UnauthorizedAccessException from CoreWebView2Environment.CreateAsync / // CreateCoreWebView2ControllerAsync when another process is holding the @@ -5273,26 +5449,52 @@ protected override async void OnClosing(System.ComponentModel.CancelEventArgs e) if (_isShuttingDown) return; _isShuttingDown = true; - // Show the shutdown overlay so the user sees progress while sessions tear down. - // The yield lets WPF render the overlay before the synchronous disposal below blocks - // the UI thread; without it, the overlay would only paint after Close() is reached. - ShutdownOverlay.Visibility = Visibility.Visible; - await Dispatcher.InvokeAsync(() => { }, - System.Windows.Threading.DispatcherPriority.Background); + var all = _vm.Sessions.ToList(); + + // Collapse the terminal area before showing the overlay. This is load-bearing, not + // tidying: WebView2 is an HwndHost, and a native child window composites OVER all + // WPF-rendered content no matter what Panel.ZIndex claims. The overlay used to be + // drawn *behind* every terminal pane, so the centred spinner was invisible and the + // only thing that reached the user was scrim leaking through the few-pixel gaps + // between panes — which reads as a stray line, not a shutdown indicator. + // + // Safe to collapse: every pane here is about to be disposed, and a frozen terminal + // is worth nothing during shutdown anyway. + // + // Skipped when there is nothing to close: with no sessions the teardown is + // instant, and the board would be a full-window flash showing "0 / 0". + if (all.Count > 0) + { + TerminalGrid.Visibility = Visibility.Collapsed; + BuildShutdownBoard(all); + ShutdownOverlay.Visibility = Visibility.Visible; + // The yield lets WPF render the overlay before the synchronous disposal below + // blocks the UI thread; without it the board would only paint at Close(). + await Dispatcher.InvokeAsync(() => { }, + System.Windows.Threading.DispatcherPriority.Background); + } _windowStateTimer.Stop(); if (_windowStateReady) _vm.UpdateWindowState(WindowState, Left, Top, Width, Height); await _vm.SaveStateAsync(); - var all = _vm.Sessions.ToList(); - // Non-Claude sessions don't fight over ~/.claude.json — dispose them in parallel. + int boardDone = 0, boardTotal = all.Count; foreach (var vm in all) { if (!ClaudeSessionService.IsClaudeCommand(vm.Command)) + { vm.Dispose(); + MarkShutdownRow(vm.Id, "✓", "#a6e3a1", "closed"); + SetShutdownProgress(++boardDone, boardTotal); + } } + // These are synchronous, so nothing has repainted yet — yield once so the board + // shows them as closed rather than jumping when the first Claude session lands. + if (boardDone > 0) + await Dispatcher.InvokeAsync(() => { }, + System.Windows.Threading.DispatcherPriority.Background); // Claude rewrites ~/.claude.json on exit without locking, so two claude.exe // processes flushing simultaneously can corrupt it. Dispose claude sessions one @@ -5323,14 +5525,25 @@ await Dispatcher.InvokeAsync(() => { }, $"disposing '{vm.Name}' without waiting"); try { vm.Dispose(); } catch { } skippedWait++; + // Marked, not hidden. Force-disposal is the case where a user most wants + // to know which session didn't get to exit cleanly. + MarkShutdownRow(vm.Id, "⨯", "#f38ba8", "forced"); + SetShutdownProgress(++boardDone, boardTotal); + SetShutdownBudget(shutdownClock.ElapsedMilliseconds); continue; } + MarkShutdownRow(vm.Id, "◐", "#fab387", "closing…", active: true); + SetShutdownBudget(shutdownClock.ElapsedMilliseconds); + long t0 = shutdownClock.ElapsedMilliseconds; await DisposeAndWaitForExitAsync(vm, timeoutMs: Math.Min(10000, remainingBudget)); long exitMs = shutdownClock.ElapsedMilliseconds - t0; disposed++; + MarkShutdownRow(vm.Id, "✓", "#a6e3a1", $"{exitMs / 1000.0:0.0}s"); + SetShutdownProgress(++boardDone, boardTotal); + // The exit wait above is on the process handle, but Claude's config write can // still be in flight when the handle closes — hence a flat post-exit pause. // @@ -5359,6 +5572,15 @@ await Dispatcher.InvokeAsync(() => { }, Log($"SHUTDOWN complete: {disposed} waited, {skippedWait} force-disposed, " + $"{shutdownClock.ElapsedMilliseconds}ms total"); + // Last paint before the DB close and the reclose below. Without it the board's + // final row stays mid-flight on screen for the remainder of teardown. + SetShutdownBudget(shutdownClock.ElapsedMilliseconds); + ShutdownHint.Text = skippedWait > 0 + ? $"Closed {disposed}, force-closed {skippedWait}. Saving index…" + : "All sessions closed. Saving index…"; + await Dispatcher.InvokeAsync(() => { }, + System.Windows.Threading.DispatcherPriority.Background); + // OutputIndexer.Dispose now drains its worker first, but SqliteConnection.Close // has been observed to throw NRE internally on shutdown — swallow + log so it // doesn't escape as an unhandled exception during application exit.