fix(startup): run the Claude config gate off the UI thread by AThraen · Pull Request #107 · umage-ai/CodeShellManager

AThraen · 2026-08-28T17:43:00Z

Fixes a regression I introduced in #96 — caught by the instrumentation from #101, on a real restore rather than by reasoning.

The measurement

11 sessions, 70.5s total restore:

gate  sum: 62948ms      <- 89% of restore time
launch sum:  7548ms

Individual gates of 14296ms, 22953ms, 11090ms — against a 2000ms cap. One is 11× over.

So #96 made restore substantially worse than the flat 2s stagger it replaced, which is the opposite of what it claimed. On the machine I originally measured, launch dominated and the gate looked fine; the failure only shows on a machine loaded enough for it to matter.

Cause

WaitForQuiesceAsync is awaited from the restore loop, which runs on the UI thread. Every Task.Delay continuation therefore queues behind whatever the dispatcher is doing — during restore, creating a WebView2 per session. A "50ms" poll takes seconds, and the file-time reads are synchronous I/O on that same thread.

On the thread pool the timer continuations are prompt and the cap holds.

Being precise about what actually fixed it

The post-delay deadline check in this PR is belt-and-braces and does not explain the measurement. The old loop's top-of-loop check already exited after a late delay, so gate=22953ms was one starved continuation, not a missed deadline.

I initially wrote a test asserting the opposite. It passed against the old code too — so it proved nothing, and it's gone rather than left in to look like coverage. Task.Run is the fix.

What is genuinely new and tested: each sleep is now clamped to the remaining budget, so the tail can't overshoot by a whole poll interval. Wait_NeverSleepsPastTheDeadline fails against the old loop.

Check	Result
Unit tests	312/312
App + Tests build	0 errors, 0 warnings

Worth knowing

If this doesn't bring restore back under the old flat-2s baseline on a loaded machine, the adaptive gate isn't earning its complexity and #96 should simply be reverted. The next RESTORE lines in crash.log will say — gate= should now sit near 300ms, and never exceed ~2000ms.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NDsEfog5kVkT5NmX1Ya5be

Fixes a regression I introduced in #96. Caught by the instrumentation added in #101, on a real restore. Measured before this change, 11 sessions, 70.5s total restore: gate sum: 62948ms <- 89% of restore launch sum: 7548ms with individual gates of 14296ms, 22953ms and 11090ms against a 2000ms cap. So #96 made restore substantially WORSE than the flat 2s stagger it replaced, which is the opposite of what it claimed. Cause: WaitForQuiesceAsync is awaited from the restore loop, which runs on the UI thread. Every Task.Delay continuation therefore queues behind whatever the dispatcher is doing — during restore, creating a WebView2 per session — so a "50ms" poll takes seconds, and the file-time reads are synchronous I/O on that same thread. Running it on the thread pool makes the timer continuations prompt and the cap hold. Being precise about what fixed it: the post-delay deadline check added here is belt-and-braces and does NOT explain the measurement. The old loop's top-of-loop check already exited after a late delay, so gate=22953ms was ONE starved continuation rather than a missed deadline. I initially wrote a test asserting otherwise; it passed against the old code too, so it was proving nothing and is gone. Task.Run is the fix. Also clamps each sleep to the remaining budget so the tail can't overshoot by a whole poll interval — that part is genuinely testable and covered. 312/312 pass, 0 warnings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDsEfog5kVkT5NmX1Ya5be

…#110) MonitorExitAsync did `await Task.Run(() => WaitForSingleObject(h, INFINITE))`, which parks one thread-pool thread per live PTY for the whole lifetime of the session — plus one per run-command PTY. With ~25 sessions restoring that is ~25 permanently blocked pool threads, and the pool injects replacements at roughly one per second, so everything else queues behind it. Measured on a real 25-session restore: gate sum: 81379ms of a 135s restore, max 23998ms against a 2000ms cap Most gates were fine (300-900ms); three blew out to 18-24s. Usually-fast, occasionally-catastrophic is thread starvation, not a slow gate. #107 moved the gate onto the pool to escape a saturated UI thread, which fixed the common case but put it behind this queue for the unlucky ones. Also the likely reason shutdown differs so sharply between machines running identical code: 3 waited / 14 force-disposed / 27002ms on a 41-session machine against 10 waited / 0 / 7662ms on a smaller one. RegisterWaitForSingleObject hands the wait to the OS and only consumes a pool thread once the handle signals, so an idle PTY costs nothing. The registration unregisters from inside its own callback, which is the documented one-shot pattern; the duplicated handle is still closed by MonitorExitAsync's finally, so the SafeWaitHandle is constructed with ownsHandle:false. 312/312 pass, 0 warnings. Claude-Session: https://claude.ai/code/session_01NDsEfog5kVkT5NmX1Ya5be Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

…t shutdown (#111) The adaptive config-watching gate from #96 could not hold its own 2000ms cap on the launch path. Measured across three runs on a 10-session machine: pre-#107 gate sum ~61s max 22953ms #107 gate sum ~62s max 31378ms #110 gate sum ~34s max 12574ms Two follow-up fixes — #107 moving it off the UI thread, #110 removing the blocked-thread-per-PTY behind it — roughly halved the cost but never bounded it. Three attempts is enough. What it bought when it worked was ~1.2s per session. Across 30 samples: about a third were pinned at ~2000ms (no write observed, waited the full cap — i.e. identical to a flat delay with more machinery), about a third were genuine wins, and the rest ran 5-31s. A mechanism whose best case saves ~1.2s and whose worst case costs 12-31s is not worth keeping on a path the user waits through at every launch. Predictable beats occasionally-clever here. Both launch paths go back to Task.Delay(staggerMs). KEPT at shutdown, where the data says the opposite: cfgSettle measures a consistent ~304ms against the flat 1000ms, on every run and both machines. The machine is quiet at shutdown, so polling is reliable. Same mechanism, different contention, different answer — the comments say so at both sites. #110 stays regardless: parking a thread-pool thread per PTY is wrong on its own merits and it halved this. Net for a 10-Claude-session restore: a bounded ~20s of stagger instead of ~34s with 12-second outliers. 312/312 pass, 0 warnings. Claude-Session: https://claude.ai/code/session_01NDsEfog5kVkT5NmX1Ya5be Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

@font-face

An independent verification of the six fixes confirmed four outright and found three problems, one of which silently defeated the fix it was checking. FUNCTIONAL — RestartSessionAsync branched on the WRONG command. EditSessionAsync calls SessionConfigEditor.Apply BEFORE RestartSessionAsync, and Apply mutates session.Command in place. So the branch read the NEW command: editing a Claude session to something else tore down the running claude.exe with a bare Dispose, no exit wait and no config quiesce — exactly the ~/.claude.json race the fix was written to close, still open on the one edit that needed it. The old command is now captured pre-Apply and passed in. UX — the sidebar row vanished for up to 11s during a Claude restart. The row is removed early, and the new exit wait was inserted BEFORE the placeholder that exists to stop the row blinking out. Placeholder moved above the wait. PERF — PwshLocator.Executable is a Lazy first forced from PseudoTerminal.Start, which runs on the UI thread. With the Store-alias probe behind it that is up to ~7s of frozen window on first launch, and a realistic ~1s during restore. This is the same class of UI-thread stall #107 and #110 spent four rounds removing, and I reintroduced it. Now warmed on the pool in OnLoaded, so the Lazy is already resolved before any session starts. Also from the verification: - HasExited is now a volatile field. It was safe as an auto-property only because the field-like Exited event's add accessor is an Interlocked CAS and supplied the fence — too subtle to rely on for a future reader that polls. - The setOptions re-fit uses requestAnimationFrame. Neither fonts API works here: fonts.ready is permanently resolved after page load, and fonts.load only matches CSS-connected FontFace objects — there are no @font-face rules and the families are OS-installed, so it resolved having matched nothing. rAF fires after the style change is applied and measured. - Corrected the PwshLocator class doc, which still argued against the probe the code now performs, and a comment in ApplyEditMode that described NameBox being blanked and repaired. Verified correct without change: HasExited latching on every exit path (including the DuplicateHandle early return), the guard/re-check ordering, the edit-mode early return, the folder validation branch and placement, and the WaitForHandleAsync lock (no inline dispatch, so no deadlock; Dispose strictly after Unregister). 314/314 pass, 0 warnings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDsEfog5kVkT5NmX1Ya5be

@font-face

* fix: six findings from the v0.7.0 release review Independent review of v0.6.0..HEAD raised 10 findings. This fixes the six that are release-blocking or self-inflicted; #8 (DbGate per-line throughput) and #9 (File.Replace portability) are deferred by agreement. 1. HIGH — DisposeAndWaitForExitAsync guarded on pty.IsRunning, which is `_hProcess != IntPtr.Zero` and only cleared in Dispose. It does NOT mean the child is alive, so a pane whose claude already exited (user typed `exit`, or it crashed) subscribed to Exited AFTER MonitorExitAsync had already raised it, then waited the full 10s for an event that never comes. That compounded with the 15s shutdown budget added in #101: two stale panes consume the whole allowance and every remaining LIVE Claude session is then force-disposed with no exit wait — losing exactly the ~/.claude.json serialization the loop exists to provide. A slow shutdown became a correctness risk. Added PseudoTerminal.HasExited, latched before Exited is raised so a subscriber can never observe "not exited" for a dead process. Caller guards on it and re-checks after subscribing to close the in-between window. 2. MEDIUM — RestartSessionAsync disposed the old session and relaunched immediately, recreating the concurrent-config-writer race that the launch stagger and shutdown loop both exist to prevent, and making --resume read a session index the outgoing process had not finalised. Claude sessions now wait for real exit plus config quiesce; non-Claude keep the cheap teardown. 3. MEDIUM — SessionType_Changed cleared NameBox unconditionally. In edit mode that silently wrote Name = "" (AutoFillName has nothing to refill from for a remote session). Now returns early in edit mode. 4. MEDIUM — no working-folder validation in edit mode. An empty folder was persisted and LaunchSessionAsync silently fell back to %USERPROFILE%, so a session appeared reconfigured while running somewhere else, with git info and accent colour keyed off an empty path. Flipping Remote -> Local hit this every time. Validated in edit mode only; create mode keeps the useful home-folder default. 5. MEDIUM — self-inflicted in #105. IsRunnable rejected any zero-byte reparse point to skip Store App Execution Alias stubs, but a WORKING Store install of PowerShell 7 is exactly that shape. Store-PS7 users were silently downgraded to 5.1, losing the profile functions that are the entire reason for preferring pwsh — on every session launch, since the locators merged. Traded a rare failure for a common one. Now: metadata fast path for ordinary executables (no spawn), and an actual bounded execution probe only for the ambiguous alias shape. 6. LOW — self-inflicted in #110. RegisterWaitForSingleObject's callback can run before the assignment to `registration` completes when the handle is already signalled, leaving the wait unregistered and disposing the event under a live registration. Published through a lock with a once-only release flag. Also corrected a stale comment in MainViewModel describing a mouse-report filter that no longer exists, and added the _sessionUiVersion bump in RestartSessionAsync that the field's own doc says belongs there. 314/314 pass, 0 warnings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDsEfog5kVkT5NmX1Ya5be * docs: record the HasExited rule, the Claude restart wait, and why an alias shape is ambiguous Follow-up to the six release-review fixes. - Waiting for a PTY: check HasExited, never IsRunning. Written down because the distinction is invisible at the call site and getting it wrong turned the shutdown budget into a correctness risk rather than a latency guard. - RestartSessionAsync waits for real exit + config quiesce for Claude sessions. - PwshLocator: a zero-byte reparse point is AMBIGUOUS, not bad. A working Store install of PowerShell 7 is the same shape as a dead stub, so only that case is settled by probing. Recorded because rejecting the shape looks obviously correct and silently downgraded Store users. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDsEfog5kVkT5NmX1Ya5be * fix(profile): don't let a malformed colour scheme abort a session launch From the release security audit — reported below its severity bar, but it is a real robustness bug. ApplyProfileOverrides called JsonSerializer.Deserialize<JsonElement> on ProfileColorSchemeJson with no guard. That value is normally produced by SchemeMapper, but ImportExportService deserializes a whole AppState from any file the user opens, so it can be arbitrary. A JsonException there propagates out of the launch path and takes down an otherwise fine session. Now caught and logged; the session launches with the default palette. Losing a theme is survivable, failing to start is not. The audit found nothing at or above its bar across the whole v0.6.0..HEAD diff: the PostRunUrl scheme guard holds (no parse divergence between Uri and ShellExecute), SQL parameterisation is intact including the FTS5 and LIKE paths, and nothing is interpolated into the page — host->page traffic is exclusively PostWebMessageAsString of serialized JSON, with no ExecuteScriptAsync or NavigateToString anywhere. One hardening suggestion deliberately NOT taken: PwshLocator validates the absolute path from where.exe then returns the bare name for CreateProcess to re-resolve. Returning the validated path would close a currently-unreachable search-order gap, but the value is interpolated into a command line by BuildCmdLine without quoting, so an absolute path containing spaces would break every wrapped session. Not worth that risk immediately before a release for a gap the audit itself judged unreachable. 314/314 pass, 0 warnings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDsEfog5kVkT5NmX1Ya5be * fix: address the verification pass on the release-review fixes An independent verification of the six fixes confirmed four outright and found three problems, one of which silently defeated the fix it was checking. FUNCTIONAL — RestartSessionAsync branched on the WRONG command. EditSessionAsync calls SessionConfigEditor.Apply BEFORE RestartSessionAsync, and Apply mutates session.Command in place. So the branch read the NEW command: editing a Claude session to something else tore down the running claude.exe with a bare Dispose, no exit wait and no config quiesce — exactly the ~/.claude.json race the fix was written to close, still open on the one edit that needed it. The old command is now captured pre-Apply and passed in. UX — the sidebar row vanished for up to 11s during a Claude restart. The row is removed early, and the new exit wait was inserted BEFORE the placeholder that exists to stop the row blinking out. Placeholder moved above the wait. PERF — PwshLocator.Executable is a Lazy first forced from PseudoTerminal.Start, which runs on the UI thread. With the Store-alias probe behind it that is up to ~7s of frozen window on first launch, and a realistic ~1s during restore. This is the same class of UI-thread stall #107 and #110 spent four rounds removing, and I reintroduced it. Now warmed on the pool in OnLoaded, so the Lazy is already resolved before any session starts. Also from the verification: - HasExited is now a volatile field. It was safe as an auto-property only because the field-like Exited event's add accessor is an Interlocked CAS and supplied the fence — too subtle to rely on for a future reader that polls. - The setOptions re-fit uses requestAnimationFrame. Neither fonts API works here: fonts.ready is permanently resolved after page load, and fonts.load only matches CSS-connected FontFace objects — there are no @font-face rules and the families are OS-installed, so it resolved having matched nothing. rAF fires after the style change is applied and measured. - Corrected the PwshLocator class doc, which still argued against the probe the code now performs, and a comment in ApplyEditMode that described NameBox being blanked and repaired. Verified correct without change: HasExited latching on every exit path (including the DuplicateHandle early return), the guard/re-check ordering, the edit-mode early return, the folder validation branch and placement, and the WaitForHandleAsync lock (no inline dispatch, so no deadlock; Dispose strictly after Unregister). 314/314 pass, 0 warnings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDsEfog5kVkT5NmX1Ya5be * fix(startup): await the pwsh warm-up instead of racing it Third verification pass: corrections A (restart command branch) and B (sidebar placeholder) verified correct; C was incomplete. Starting the warm task in OnLoaded is necessary but not sufficient. PwshLocator's Lazy uses ExecutionAndPublication, so a UI thread reaching .Value while the pool thread is still inside the factory takes the Lazy's monitor and blocks for the REMAINING factory duration. That converts a guaranteed stall into a raced one and shrinks it by the head start — it does not remove it. Residual was quantified at up to ~5-6.5s in the pathological case (slow where.exe plus a live Store alias paying a full PS7 cold start). PublicationOnly would not have fixed it either: the UI thread would stop blocking on the monitor and simply run its own copy of the factory, paying the same cost. The task is now awaited before the restore loop. Awaiting yields rather than blocking, so the window stays responsive, and Task.WhenAny with an 8s ceiling — above Resolve's own 7s bound of 2s where.exe + 5s alias probe — means a wedged probe delays restore rather than preventing it. In the ordinary MSI case Resolve finishes in ~10-50ms and this is a no-op. Also dropped a superseded comment in terminal-init.js: the old fonts.load rationale was left above the new requestAnimationFrame one and directly contradicted it. Verified correct without change in this pass: launchedCommand is captured genuinely pre-Apply with no re-mutation before use and only one call site exists; the placeholder is added exactly once and, critically, still AFTER the _sessionUi and _vm.Sessions removals so RebuildSidebarOrder's Resolve takes the placeholder branch rather than a stale live item; HasExited is latched on all three exit paths through the outer finally; and the rAF re-fit cannot throw. 314/314 pass, 0 warnings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDsEfog5kVkT5NmX1Ya5be --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

AThraen merged commit aa24e69 into main Aug 29, 2026
1 check passed

AThraen deleted the fix/claude-gate-overshoot branch August 29, 2026 18:48

This was referenced Aug 29, 2026

fix(terminal): make clicking a pane activate it, from the page side #108

Merged

perf(pty): wait for process exit without holding a thread-pool thread #110

Merged

revert(startup): flat Claude launch stagger; keep the adaptive gate at shutdown #111

Merged

AThraen mentioned this pull request Sep 6, 2026

perf: session restore on startup is fully sequential (~131s for 25 sessions; launch cost, not sleeps) #82

Open

Navigation Menu

Uh oh!

fix(startup): run the Claude config gate off the UI thread - #107

fix(startup): run the Claude config gate off the UI thread#107
AThraen merged 1 commit into
mainfrom
fix/claude-gate-overshoot

AThraen commented Aug 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

AThraen commented Aug 28, 2026

The measurement

Cause

Being precise about what actually fixed it

Worth knowing

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant