Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ PTY (ConPTY) → PseudoTerminal → TerminalBridge → WebView2 (xterm.js)
| `SessionConfigEditor` | Diffs/applies a `SessionConfigDraft` onto a `ShellSession`; decides whether the change needs a PTY restart |
| `DbGate` | Serializes every use of the shared `output.db` `SqliteConnection`. That one connection is handed to `SearchService` *and* to every `OutputIndexer`, and it is not thread-safe — concurrent create/dispose corrupts its internal command list. Acquire as `using var _ = await DbGate.AcquireAsync();` at the top of anything touching it. See issue #102 |
| `PwshLocator` | Single answer to "pwsh or powershell?", shared by `RunInstance` (run commands) and `PseudoTerminal.BuildCmdLine` (session wrapper) so they can't disagree. Resolves via `where.exe`. An ordinary executable is accepted from metadata alone (no spawn). A **zero-byte reparse point is ambiguous, not bad**: that shape is a Store App Execution Alias, and a *working* Store install of PowerShell 7 looks identical to a stub left by an uninstalled app — so only that case is settled by actually probing execution. Rejecting the shape outright silently downgraded Store-PS7 users to 5.1 on every session launch |
| `ClaudeConfigGate` | Watches Claude's config file settle. **Only used at shutdown.** The same mechanism was tried on the launch path and reverted (#111) — see the Claude launch stagger note below |
| `ToastHelper` | Tray balloon notifications |
| `SessionRunner` | Per-session owner of `RunInstance` dictionary (run commands runtime) |
| `RunInstance` | One headless PTY-backed run with ANSI-stripped output buffer |
Expand Down Expand Up @@ -164,6 +163,8 @@ The page-side `mousedown` handler also calls `fitAddon.fit()`, and the initial f
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.

**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`).

## Editing a Session's Configuration
Expand Down Expand Up @@ -360,7 +361,7 @@ Persisted in `state.json`. Key settings:

**Do not replace this with an adaptive wait again.** That was tried (#96), watched the config file settle instead of sleeping a fixed 2s, and was reverted in #111 after three attempts to make it hold its cap. Measured on a real restore it produced gates of 12574ms, 22953ms and 31378ms against a 2000ms cap. Two follow-ups helped without bounding it: #107 moved it off the UI thread, #110 removed a thread-pool thread that `PseudoTerminal` was parking per PTY.

The reason it could never work is worth recording: the restore loop periodically stalls for seconds at a time under load, and *any* timer's continuation absorbs that stall. After the revert, a plain `Task.Delay(2000)` still logged `gate=36339ms`. The gate was never slow — it was a stopwatch measuring someone else's freeze. The gate is still used at **shutdown**, where the machine is quiet and it measures a consistent ~304ms against the flat 1000ms.
The reason it could never work is worth recording: the restore loop periodically stalls for seconds at a time under load, and *any* timer's continuation absorbs that stall. After the revert, a plain `Task.Delay(2000)` still logged `gate=36339ms`. The gate was never slow — it was a stopwatch measuring someone else's freeze. The gate is now gone from BOTH paths. It was kept at shutdown on the reasoning that "the machine is quiet there, so polling is reliable" — measurement falsified that: a real run logged `cfgSettle=8731ms` against a 1000ms cap, 56% of the entire shutdown budget in one session, which is what forced the rest of the fleet to be killed without a wait. Same disease, same fix: flat delay.
- `ShowGitBranch` — show `⎇ branch` in sidebar
- `ShowTerminalStatusDot` — show status dot in terminal toolbar
- `SidebarActionIconsMode` — `OnHover` (default) / `Always` / `Hidden`. Controls the per-row `➕ 💤 ✕` button stack in the sidebar. `Hidden` collapses the panel and reclaims the horizontal space; `OnHover` keeps the panel laid out (no text shift on hover) but transparent + non-interactive until the row is hovered. Rename / Open in Explorer / Open PowerShell here remain reachable via the right-click context menu in all modes, and the terminal toolbar's `✕` is unconditional.
Expand Down
75 changes: 26 additions & 49 deletions src/CodeShellManager/MainWindow.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4405,10 +4405,10 @@ private async Task RestartSessionAsync(SessionViewModel vm, string? launchedComm
// Non-Claude sessions don't touch that file, so they keep the cheap teardown.
if (ClaudeSessionService.IsClaudeCommand(launchedCommand ?? session.Command))
{
DateTime? cfgBefore = ClaudeConfigGate.LastWriteUtcOrNull(_claudeConfigPath);
await DisposeAndWaitForExitAsync(vm, timeoutMs: 10000);
await WaitForClaudeConfigQuiesceAsync(
cfgBefore, Math.Min(_vm.Settings.ClaudeLaunchStaggerMs, 1000));
// Flat pause, same as shutdown — the adaptive gate couldn't hold its cap on
// either path. See the shutdown loop for the measurement.
await Task.Delay(Math.Min(_vm.Settings.ClaudeLaunchStaggerMs, 1000));
}
else
{
Expand Down Expand Up @@ -5326,24 +5326,28 @@ await Dispatcher.InvokeAsync(() => { },
continue;
}

// Baseline before the process is signalled, so the gate below can see the
// shutdown write land.
DateTime? cfgBefore = ClaudeConfigGate.LastWriteUtcOrNull(_claudeConfigPath);

long t0 = shutdownClock.ElapsedMilliseconds;
await DisposeAndWaitForExitAsync(vm, timeoutMs: Math.Min(10000, remainingBudget));
long exitMs = shutdownClock.ElapsedMilliseconds - t0;
disposed++;

// 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 post-exit pause. This
// used to be a flat sleep of up to 1s per session (20s across 20 sessions)
// justified as belt-and-braces. Now it waits for the write to actually settle
// and returns as soon as it has, capped at the same 1s so the worst case is
// unchanged (issue #82).
// still be in flight when the handle closes — hence a flat post-exit pause.
//
// This was an adaptive config-watching gate. #111 reverted that on the launch
// path but KEPT it here, on the reasoning that "the machine is quiet at
// shutdown, so polling is reliable". Measurement falsified that: with sessions
// actively dying, shutdown is not quiet either, and a real run logged
// cfgSettle=8731ms against this 1000ms cap — 8.7x over, and 56% of the whole
// shutdown budget spent in one session, which is what forced the rest to be
// killed without a wait.
//
// The gate's typical ~300ms beats a flat 1000ms right up until it doesn't, and
// the tail is what costs. Recomputing that run with a flat pause gives 8969ms
// instead of 15506ms, with nothing force-disposed. Predictable wins.
long q0 = shutdownClock.ElapsedMilliseconds;
if (postExitMs > 0)
await WaitForClaudeConfigQuiesceAsync(cfgBefore, Math.Min(postExitMs, 1000));
await Task.Delay(Math.Min(postExitMs, 1000));

// Per-session timing so the exit-vs-config-settle split is known rather than
// guessed at. #82 asked for this before optimising further.
Expand Down Expand Up @@ -5399,13 +5403,6 @@ await Dispatcher.InvokeAsync(() => { },
/// disposes the VM. Used for claude sessions on app close so consecutive
/// <c>~/.claude.json</c> writes can't overlap.
/// </summary>
/// <summary>
/// Claude's config file, resolved once. Honours CLAUDE_CONFIG_DIR — with that set
/// the file lives inside it, not at %USERPROFILE%\.claude.json, and watching the
/// wrong one means always waiting the full cap.
/// </summary>
private readonly string _claudeConfigPath = ClaudeConfigGate.ResolveConfigFile();

/// <summary>
/// Total time budget for waiting on Claude sessions to exit at shutdown (issue #82).
///
Expand All @@ -5414,36 +5411,16 @@ await Dispatcher.InvokeAsync(() => { },
/// remaining sessions are disposed without waiting; the job object still kills the
/// process tree, we just stop watching.
///
/// 15s is chosen to comfortably cover a normal fleet (measured exits are well under
/// a second each) while capping the pathological case at something a user will sit
/// through.
/// </summary>
private const int ClaudeShutdownBudgetMs = 15000;

/// <summary>
/// Blocks until Claude's config file has been written and gone quiet, or
/// <paramref name="capMs"/> elapses. Replaces a flat <c>Task.Delay(staggerMs)</c>
/// between consecutive Claude launches (issue #82).
/// Sized from measurement, not taste. The original 15000 was set when the only data
/// available showed ~460-770ms exits on an idle fleet. Real shutdowns of *busy*
/// sessions measure 2.3-4.7s each, so nine of them need roughly 30s — and 15s meant
/// force-disposing over half the fleet on an ordinary close.
///
/// Waiting is the right trade here: a clean exit lets Claude finish writing its
/// config, and the shutdown overlay already tells the user what is happening. The
/// budget exists to bound a genuinely wedged session, not to rush a healthy one.
/// </summary>
private Task WaitForClaudeConfigQuiesceAsync(DateTime? baseline, int capMs) =>
// Task.Run is the actual fix for the overshoot, not just tidiness.
//
// This is awaited from the restore loop, which runs on the UI thread. Left there,
// every Task.Delay continuation queues behind whatever the dispatcher is doing —
// and during restore that's creating a WebView2 per session. A "50ms" poll then
// takes seconds, and the file-time reads are synchronous I/O on the same thread.
// Measured on a real restore before this change: gate=22953ms against a 2000ms
// cap, and 63s of a 70s restore spent in here — far worse than the flat 2s stagger
// this replaced.
//
// On the thread pool the timer continuations are prompt and the cap holds.
Task.Run(() => ClaudeConfigGate.WaitForQuiesceAsync(
baseline,
() => ClaudeConfigGate.LastWriteUtcOrNull(_claudeConfigPath),
() => DateTime.UtcNow,
Task.Delay,
TimeSpan.FromMilliseconds(capMs),
ClaudeConfigGate.DefaultQuietFor));
private const int ClaudeShutdownBudgetMs = 30000;

private static async Task DisposeAndWaitForExitAsync(SessionViewModel vm, int timeoutMs)
{
Expand Down
115 changes: 0 additions & 115 deletions src/CodeShellManager/Services/ClaudeConfigGate.cs

This file was deleted.

Loading