* 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>

Support drag/drop files #8

Description

Activity

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions