feat(sessions): show a bulk restart running, and let it be stopped - #144
Merged
Merged
Conversation
"Restart all…" on a large fleet is a minute-scale operation — restarts are strictly sequential and each Claude session waits for its old process to exit — and until now nothing on screen said so and nothing could stop it. On a 55-session setup that is roughly 3½–7 minutes during which the only exit is closing the app. Three parts: **A rail, not the shutdown board.** RestartRail (2px, docked under RestoreRail) plus a RestartPill counter in the toolbar's right stack, driven by SetRestartProgress. ShutdownOverlay only works because OnClosing collapses TerminalGrid first — WebView2 is an HwndHost and composites over any WPF overlay — and collapsing the grid here would black out every live pane for the whole run. A bulk restart doesn't block the user, so it gets the quiet rail; shutdown does, so it keeps the board. Hidden for a single target, so the toolbar ↻ never flashes one. The restart pair is separate from the restore pair rather than a shared rail: the OnLoaded restore loop leaves the UI responsive between awaits, so the quick menu's "Restart all…" is clickable mid-restore and the two loops can genuinely overlap. Peach (#fab387, the shutdown board's "closing…" colour) keeps the two counters apart when both are up. **Stop means "after the current one".** The ⏹ sets _restartCancelRequested, read at the TOP of each iteration. It cannot abandon the session already mid-restart: its VM is out of _vm.Sessions and its PTY is down by then, so returning early would strand it as dormant — the failure path, not a stop. Targets not yet reached keep their live terminals untouched. **The confirmation quotes a duration.** Services/BulkRestartEstimate is WPF-free so the wording is unit-testable at all (the ShellIntegrationPayload / SessionConfigEditor precedent). It costs a Claude target at 4–8s (exit wait 2.3–4.7s + stagger + ~1.4s relaunch) and a plain shell at 1–2s, reports a range rather than false precision, and switches to minutes only once the optimistic bound passes 60s. At 10+ targets it also says the restarts are sequential and names how many are Claude — that count is what makes the estimate look earned rather than arbitrary. Verified: 0-warning build, 609/609 unit tests (12 new). The chrome was rendered against a live --clean instance and sampled by pixel: below the toolbar border at #313244, the restore rail reads #89B4FA and the restart rail #FAB387, each filling to exactly its k/N proportion, with both pills side by side in the toolbar. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDsEfog5kVkT5NmX1Ya5be
An independent review pass found seven things. All were real; six are fixed
here and the seventh is the reason for the biggest change.
**A restart can no longer start during the startup restore.** Both loops spawn
claude.exe and neither one's stagger can see the other's launches, so
overlapping them is the unlocked read-modify-write on ~/.claude.json that
ClaudeLaunchStaggerMs exists to prevent. The restore loop awaits per session
and leaves the quick menu clickable throughout, so "Restart all…" mid-restore
is an ordinary click on a 55-session start. Both RestartSessionAsync and
RestartSessionsAsync now refuse while _restoreInProgress, dropped with a toast
like the existing guard — one session is enough to lose the race.
This is the finding that should not have needed a reviewer: the previous
commit's XAML comment and CLAUDE.md paragraph both cited that overlap as the
*reason* the rails are separate pairs, which wrote a config race down as a
layout question. Both are corrected, and the separate pairs are now justified
on their own terms.
The rest:
- The confirmation promised a stop control that is never rendered. The bulk
entry points confirm whatever the count, but SetRestartProgress hides the
pill at total <= 1 — so at exactly one session the dialog pointed at
something that never appears. Gated on totalTargets > 1.
- RestartStopBtn had no Style, so it fell through to Aero2's default Button
template, whose IsEnabled=false trigger sets Background/BorderBrush/
Foreground with TargetName and outranks both the Transparent TemplateBinding
and the muted brush set in code. The disabled ⏹ would have rendered as a
light-grey #F4F4F4 box inside the dark pill. ToolBtn has no disabled trigger.
- The stop toast counted vanished targets as "finished": close three
not-yet-reached sessions mid-run, then stop, and three of the "finished"
were never restarted. A separate counter now tracks work actually done, and
"not restarted" replaces "left running" (a target an earlier failure dropped
to dormant is neither).
- ConfirmText_SaysTheQueueCanBeStopped asserted Contains("stop"), which the
always-present "...session commands are stopped" satisfied — deleting the
whole affordance sentence left it green. Now asserts "stop the queue".
- DescribeDuration clamped `others` at 0 but never capped `claude` at the
total, so (3 targets, 10 Claude) quoted 40-80 seconds for three sessions.
- The restart-failure MessageBox had no owner, so it could sit behind the main
window while the bulk loop blocked in it — the rail frozen mid-count with
nothing explaining why, which is the exact "reads as a hang" the rail exists
to prevent. Now owned.
- RestartStop_Click guarded only on _restartInProgress, which the
single-session path also holds, with stale _restartDone/_restartTotal from
the previous bulk run. Not reachable (a Collapsed pill does not hit-test),
but that made the handler's correctness rest on a visibility property;
guarded on _restartTotal > 1 instead. _restartInProgress is also now the
last statement before its try.
3 new tests (612 total), build clean at 0 warnings.
Not verified: the disabled ⏹ on screen. The enabled pill and both rails were
confirmed by pixel earlier; the disabled state could not be captured because
Windows' foreground lock blocked every attempt to raise the test window. The
fix is reasoned from the Aero2 template, not seen.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NDsEfog5kVkT5NmX1Ya5be
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
1 participant
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
Restart all…(#140) is strictly sequential, and each Claude session waits for its old process to actually exit before its replacement starts. On a large setup that is a minute-scale operation — measured against a 55-sessionstate.json, roughly 3½–7 minutes — and until now nothing on screen said so, nothing showed progress, and the only way out was closing the app.What
A rail, not the shutdown board.
RestartRail(2px, docked underRestoreRail) plus aRestartPillcounter in the toolbar's right stack, driven bySetRestartProgress.ShutdownOverlaywas the obvious thing to reuse and it is the wrong shape here. It only works becauseOnClosingcollapsesTerminalGridbefore showing it — WebView2 is anHwndHostand composites over any WPF overlay regardless ofPanel.ZIndex— and collapsing the grid during a restart would black out every live pane for the whole run. A bulk restart does not block the user, so it gets the quiet rail; shutdown does, so it keeps the board. Both rail and pill stay hidden for a single target, so the toolbar ↻ never flashes one.The restart pair is separate from the restore pair rather than a shared rail. The
OnLoadedrestore loop leaves the UI responsive between awaits, so the sidebar quick menu's "Restart all…" is clickable while a restore is still running and the two loops can genuinely overlap; one shared rail would need a precedence rule for no benefit. Peach (#fab387, already the shutdown board's "closing…" colour) keeps the two counters legible when both are up.Stop means "after the current one". The
⏹on the pill sets_restartCancelRequested, read at the top of each loop iteration. It deliberately cannot abandon the session already mid-restart: by that point its VM is out of_vm.Sessionsand its PTY is down, so returning early would strand it as dormant — the failure path, not a stop. Targets not yet reached keep their live terminals untouched, and a toast reports how many were skipped. The flag clears in the samefinallyas_restartInProgress.The confirmation quotes a duration. New WPF-free
Services/BulkRestartEstimate, following theShellIntegrationPayload/SessionConfigEditorprecedent so the wording is unit-testable at all. It costs a Claude target at 4–8s (CLAUDE.md's measured 2.3–4.7s exit + stagger + ~1.4s relaunch from theRESTORElog lines) and a plain shell at 1–2s, reports a range rather than false precision, and switches to minutes only once the optimistic bound passes 60s. At or above 10 targets it also states that restarts are sequential and names how many are Claude — that count is what makes the estimate look earned rather than arbitrary.MessageBoxcannot emphasise anything, so scale is carried by the words. Making the warning look like a warning would mean a themed window instead of aMessageBox; that is a deliberate non-goal here and noted inCLAUDE.md.Testing
Built at 0 warnings; 609/609 unit tests pass, 12 of them new in
BulkRestartEstimateTests.The chrome was rendered and verified, not just compiled. A
--cleaninstance was run with a temporary probe callingSetRestartProgress(12, 55)and the framebuffer sampled by pixel at x=400:At x=1500 the peach has ended and only blue remains — each rail fills to exactly its
k/Nproportion. Both pills render side by side, restart left of restore. The probe is not in the diff.Not verified: the
⏹on a real multi-session restart, since driving session creation through UI automation does not currently work on this app (bothInvokePattern.Invoke()and a real mouse click onNewSessionBtnfail to open the dialog while the process reportsResponding=True). The stop path is exercised only by reading it.🤖 Generated with Claude Code
https://claude.ai/code/session_01NDsEfog5kVkT5NmX1Ya5be