feat(bundle): low-hanging fruit + tests, refactor, docs by AThraen · Pull Request #63 · umage-ai/CodeShellManager

AThraen · 2026-05-17T07:42:30Z

Summary

e386863 — Original bundle of fixes for issues Add 3x3 grid layout (9-pane) #51, Run command output contains raw escape sequences (e.g. [?9001h) #53, Run commands: choose between bare process and PowerShell host #54, Run commands: open a URL in the default browser after command completes #55, Race in PseudoTerminal handle close vs MonitorExitAsync wait #39, Restore last closed session / suggest recent closed sessions when creating a new session #46 (pre-session work).
c4b41b5 — Adds 83 unit tests across AlertDetector, ColorService, OutputIndexer, SearchService. All four services were previously untested. Test count went from ~50 to 206.
9d8ece9 — Extracts IPseudoTerminal interface so RunInstance / SessionRunner can be tested with a fake. Adds 15 new SessionRunner lifecycle tests. No behavioral change to ConPTY code; production WPF wiring untouched (existing public ctors preserved).
1ec2b4b — Expands CLAUDE.md Services table from 6 to 20 rows; adds a new ## Per-Session Notes section documenting the notes feature (was advertised in README, missing from CLAUDE.md). Inline comment on AlertDetector's 1.5s idle timer.
6fec182 — Fixes the pre-existing WiX manifest CI check (app.png was on main but missing from installer/CodeShellManager.wxs).

Test plan

dotnet test tests/CodeShellManager.Tests/CodeShellManager.Tests.csproj — 206 tests pass on this branch
dotnet build src/CodeShellManager/CodeShellManager.csproj — clean
CI green

🤖 Generated with Claude Code

Six low-effort / high-impact changes bundled on one branch: #53 Run-command output: strip CSI private-mode (e.g. ESC[?9001h) and OSC ST. The shared regex in RunInstance + OutputIndexer now accepts `?` in CSI params and recognises both BEL and ESC\ as OSC terminators. #54 RunCommandItem.Mode — Process (default, cmd /c) vs PowerShell (pwsh -EncodedCommand UTF-16LE base64; falls back to powershell.exe). SSH runs ignore Mode — remote always goes through bash. #55 RunCommandItem.PostRunUrl — opens URL via ShellExecute on exit code 0. Editor dialog grows Mode + URL columns; dialog widened 720→900. #51 LayoutMode.ThreeByThree — new 3×3 (9-pane) grid layout and toolbar btn. #39 PseudoTerminal MonitorExitAsync now waits on a DuplicateHandle so the Dispose path can close _hProcess without racing the wait. Eliminates the Win32 UB that could fire Exited before the child actually exited. #46 Recently-closed ring buffer (cap 10, persisted to state.json): - new Models/RecentlyClosedEntry.cs - Ctrl+Shift+T pops & reopens the newest entry (browser convention) - duplicate-session moves to Ctrl+Alt+T - "Recently closed" list at the top of the New Session dialog - FromSession deep-copies RunCommands with fresh Ids - --clean mode skips push, matching SaveStateAsync semantics Verification • dotnet build: 0 errors, 30 pre-existing warnings • dotnet test: 90/90 pass (was 82; +8 new tests covering BuildPwshArgs, RecentlyClosedEntry.FromSession, Subtitle, RunCommandItem defaults) CLAUDE.md updated: new keybindings, Recently Closed section, RunCommandItem data model with Mode + PostRunUrl semantics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…SearchService 83 tests across four services that were untested: - AlertDetector: regex patterns for tool-approval / input-required prompts, ANSI stripping, idle-timer cancellation, NotifyUserInteracted - ColorService: FNV-1a determinism, palette distribution, hex format, case-insensitivity - OutputIndexer: CSI/OSC/keypad ANSI sequence stripping, CR handling - SearchService: FTS5 search, notes, history, prune/clear, usage stats (per-test temp SQLite for isolation)

- New IPseudoTerminal interface with the minimum surface RunInstance uses (DataReceived, Exited, ExitCode, Start) - PseudoTerminal implements it; no behavioral change to ConPTY code - RunInstance and SessionRunner gain an internal ctor accepting Func<IPseudoTerminal> factory; production ctors preserved so the WPF UI is untouched - 15 new SessionRunnerTests using a hand-rolled FakePseudoTerminal — covers Run/Stop/Dismiss lifecycle, kill-and-restart semantics, state transitions, and the 1MB output-buffer cap

- Services table grew from 6 to 20 rows; all files under src/CodeShellManager/Services/ are now represented (was ~30% coverage) - New "## Per-Session Notes" section documenting the project_notes SQLite table, lazy load + 1s debounce save, FTS5 search integration, and the folder-key consequence - One-line inline comment on AlertDetector's 1500ms idle constant explaining the choice (long enough for Claude's prompt redraw bursts to settle, short enough to feel responsive)

Copilot

Pull request overview

This PR bundles several productivity and reliability improvements for CodeShellManager: run-command enhancements, session recovery, layout/boot UX updates, ConPTY/ANSI cleanup, expanded tests, docs, and installer packaging.

Changes:

Adds PowerShell/post-run URL support for run commands and recently-closed session recovery.
Adds 3×3 layout, terminal boot/shutdown overlays, ConPTY wait-handle changes, and ANSI stripping updates.
Expands unit tests and CLAUDE documentation; updates WiX asset manifest.

Reviewed changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
`tests/CodeShellManager.Tests/SessionRunnerTests.cs`	Adds lifecycle tests for `SessionRunner`/`RunInstance` via fake PTY.
`tests/CodeShellManager.Tests/SearchServiceTests.cs`	Adds SearchService, notes, history, pruning, and usage-stats tests.
`tests/CodeShellManager.Tests/RunInstanceTests.cs`	Adds PowerShell encoded-command argument test.
`tests/CodeShellManager.Tests/RunCommandItemTests.cs`	Adds defaults test for new run-command fields.
`tests/CodeShellManager.Tests/RecentlyClosedEntryTests.cs`	Adds tests for recently-closed session snapshots.
`tests/CodeShellManager.Tests/OutputIndexerTests.cs`	Adds ANSI stripping regex tests.
`tests/CodeShellManager.Tests/ColorServiceTests.cs`	Adds ColorService palette/hash tests.
`tests/CodeShellManager.Tests/AlertDetectorTests.cs`	Adds AlertDetector behavior tests.
`src/CodeShellManager/Views/SessionRunCommandsDialog.xaml.cs`	Persists run mode and post-run URL from editor rows.
`src/CodeShellManager/Views/SessionRunCommandsDialog.xaml`	Adds mode selector and post-run URL UI.
`src/CodeShellManager/Views/NewSessionDialog.xaml.cs`	Adds recently-closed entry selection support.
`src/CodeShellManager/Views/NewSessionDialog.xaml`	Adds recently-closed list UI.
`src/CodeShellManager/ViewModels/MainViewModel.cs`	Adds 3×3 layout enum and recently-closed ring buffer helpers.
`src/CodeShellManager/Terminal/TerminalBridge.cs`	Adds boot overlay messaging and background-color handling.
`src/CodeShellManager/Terminal/PseudoTerminal.cs`	Adds `IPseudoTerminal` implementation and duplicate wait handle logic.
`src/CodeShellManager/Terminal/OutputIndexer.cs`	Expands ANSI/OSC stripping regex.
`src/CodeShellManager/Terminal/IPseudoTerminal.cs`	Introduces PTY abstraction for testing.
`src/CodeShellManager/Services/SessionRunner.cs`	Adds injectable PTY factory seam.
`src/CodeShellManager/Services/RunInstance.cs`	Adds PowerShell mode, post-run URL handling, PTY factory, and ANSI updates.
`src/CodeShellManager/Services/AlertDetector.cs`	Documents idle timer behavior.
`src/CodeShellManager/Models/RunCommandItem.cs`	Adds `RunMode` and `PostRunUrl`.
`src/CodeShellManager/Models/RecentlyClosedEntry.cs`	Adds snapshot model for closed sessions.
`src/CodeShellManager/Models/AppState.cs`	Persists recently-closed ring buffer.
`src/CodeShellManager/MainWindow.xaml.cs`	Wires reopen, 3×3 layout, boot context, shutdown overlay, and shortcut changes.
`src/CodeShellManager/MainWindow.xaml`	Adds 3×3 layout button and shutdown overlay.
`src/CodeShellManager/Assets/terminal.html`	Adds terminal boot overlay markup/styles.
`src/CodeShellManager/Assets/terminal-transparent.html`	Adds boot overlay to transparent terminal page.
`src/CodeShellManager/Assets/terminal-init.js`	Handles boot overlay WebView messages.
`installer/CodeShellManager.wxs`	Adds `app.png` to WiX manifest.
`CLAUDE.md`	Expands service docs, run-command docs, notes docs, and shortcuts.

Comments suppressed due to low confidence (1)

src/CodeShellManager/MainWindow.xaml.cs:4742

The shortcut path also pops the recent entry before knowing whether reopening worked. Because launch failures are swallowed inside LaunchSessionAsync, a failed Ctrl+Shift+T reopen loses the saved entry with no way to retry. Consider peeking first and removing only after a successful launch, or pushing it back if reopen fails.

        if (key == Key.T && mods == (ModifierKeys.Control | ModifierKeys.Shift))
        {
            var entry = _vm.PopMostRecentlyClosed();
            if (entry != null) _ = ReopenClosedSessionAsync(entry);
            return true;

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

AThraen · 2026-05-17T09:46:14Z

+    /// </summary>
+    public string? PostRunUrl { get; set; }
+
    /// <summary>


Fixed in 8f31faf: DuplicateSessionAsync now copies Mode and PostRunUrl onto the cloned run-commands.

AThraen · 2026-05-17T09:46:24Z

+        if (!DuplicateHandle(GetCurrentProcess(), _hProcess, GetCurrentProcess(),
+                out IntPtr waitHandle, 0, false, DUPLICATE_SAME_ACCESS))
+        {
+            Log($"DuplicateHandle failed: {Marshal.GetLastWin32Error()}");
+            return;


Deferring — verified this race is in pre-existing code on main (MonitorExitAsync/Dispose weren't touched by anything in this PR), and the fix (duplicate the handle synchronously in Start or guard _hProcess with a lock) is non-trivial. I'll open a separate follow-up PR rather than scope-creep this one.

AThraen · 2026-05-17T09:46:29Z

+                probe.WaitForExit(2000);
+                _pwshResolved = "pwsh.exe";
+                return _pwshResolved;


Fixed in 8f31faf: ResolvePwsh now requires both WaitForExit(2000) == true AND probe.ExitCode == 0 before caching pwsh.exe; hung probes get Kill(entireProcessTree: true) and we fall back to powershell.exe.

AThraen · 2026-05-17T09:46:33Z

+        if (dialog.SelectedRecentlyClosed != null)
+        {
+            var entry = dialog.SelectedRecentlyClosed;
+            _vm.RemoveRecentlyClosed(entry);


Fixed in 8f31faf: added MainViewModel.PeekMostRecentlyClosed() and a ReopenAndRemoveOnSuccessAsync wrapper in MainWindow. Both call sites (the dialog path at line 416 and the Ctrl+Shift+T handler at line 4740) now peek the entry, attempt the reopen, and only call RemoveRecentlyClosed if the session is still in SessionManager.Sessions afterward (LaunchSessionAsync's failure path removes it, so membership is a reliable success signal).

AThraen · 2026-05-17T09:46:36Z

+    // Mirrors the strip regex in OutputIndexer.AnsiPattern — keep them in sync.
+    // The `?` inside [?0-9;]* covers CSI private-mode sequences like ESC[?9001h
+    // (ConPTY's Win32 input-mode bootstrap), ESC[?1049h (alt screen), etc.
+    [GeneratedRegex(@"\x1B\[[?0-9;]*[mGKHFJABCDsuhl]|\x1B\].*?(?:\x07|\x1B\\)|\x1B[=>]|\r", RegexOptions.Compiled)]


Deferring — the chunk-boundary case is real but uncommon (CSI sequences are typically 3-12 chars so they rarely split; OSC is more vulnerable but bounded). A correct fix needs a pending-suffix buffer that holds back from the trailing \x1B if it doesn't have a complete sequence after it, plus a max-holdback cap to bound memory. Better as a focused follow-up PR than mixed into this bundle.

AThraen · 2026-05-17T09:46:59Z

+      <Component Directory="AssetsFolder" Guid="*">
+        <File Source="$(var.PublishDir)\Assets\app.png" KeyPath="yes" />
+      </Component>


Fixed in 8f31faf: added <Content Include="Assets\app.png"> with CopyToOutputDirectory=PreserveNewest and ExcludeFromSingleFile=true to CodeShellManager.csproj, matching the pattern for app.ico and the other assets.

AThraen · 2026-05-17T09:47:02Z

+/// AlertDetector's only public surface for triggering an alert is <see cref="AlertDetector.Feed"/>,
+/// which starts a hardcoded 1500ms <see cref="System.Threading.Timer"/>. There is no clock-injection
+/// hook, and the regexes are private statics — so to assert on the regex behavior we have to let
+/// the real timer fire.
+///
+/// To stay deterministic we subscribe to <see cref="AlertDetector.AlertRaised"/> with a
+/// <see cref="ManualResetEventSlim"/> and wait with a generous timeout. The wait completes
+/// as soon as the event fires (typically ~1.5s after Feed) — it is NOT a blind Thread.Sleep.
+/// A short negative-case timeout (200ms past the idle threshold) is used when asserting that


Not fixing — this was a deliberate trade-off when the tests were written. AlertDetector uses a hardcoded new System.Threading.Timer(OnIdle, null, 1500, ...); adding an injectable clock would mean a production change just to speed up tests. The tests use ManualResetEventSlim.Wait(1800) so they unblock the instant the timer fires (typically ~1.5s) rather than sleeping the full duration. Total runtime for the file is ~10s on a workstation; acceptable for the coverage it gives. Will revisit if CI starts to flake.

AThraen · 2026-05-17T09:47:05Z

+        // Open post-run URL on success only. ShellExecute hands the URL to the
+        // OS default browser. We swallow exceptions because there's nothing
+        // useful to surface from the PTY exit callback's thread.
+        if (State == RunState.ExitedOk && !string.IsNullOrWhiteSpace(PostRunUrl))
+        {
+            try { Process.Start(new ProcessStartInfo(PostRunUrl) { UseShellExecute = true }); }
+            catch { /* invalid URL or no associated handler — caller can't do anything */ }


Fixed in 8f31faf: failures are now logged to %AppData%\CodeShellManager\crash.log via a new LogPostRunUrlFailure helper. Still no toast because (a) we're on the PTY-exit callback thread and would need a marshal to UI, and (b) post-run URLs are usually fire-and-forget convenience — a toast on every URL failure would be noisy. The log entry gives enough to diagnose if a user reports the issue.

AThraen · 2026-05-17T09:47:08Z

+/// How the run-command's command line is launched.
+///   Process:    cmd /c "<cmd>"  — bare ConPTY child, the historical default.
+///   PowerShell: pwsh.exe -EncodedCommand <b64>  (falls back to powershell.exe if pwsh is missing)
+///               needed when the command relies on pipes, $env:, cmdlets, or other PS syntax.


Pushing back — the doc on line 8 already says Process: cmd /c "<cmd>" — bare ConPTY child, which accurately describes the implementation. "Bare ConPTY child" here refers to cmd being the single child under ConPTY (vs PowerShell mode wrapping in pwsh.exe). The phrasing could be sharper, but the doc is not misleading about cmd /c. The combobox tooltip in SessionRunCommandsDialog.xaml was wrong and is fixed in 8f31faf.

AThraen · 2026-05-17T09:47:10Z

+                                          Background="#313244" Foreground="#cdd6f4" BorderBrush="#45475a"
+                                          Padding="6,3" Margin="0,0,8,0"
+                                          VerticalContentAlignment="Center"
+                                          ToolTip="Process = direct ConPTY child. PowerShell = wrapped in pwsh -EncodedCommand.">


Fixed in 8f31faf: tooltip now reads Process = cmd /c <cmd> (bare ConPTY child). PowerShell = pwsh -EncodedCommand (use for pipes, $env:, cmdlets). — matches the column-header tooltip on line 56 and the actual implementation.

Fixes the eight issues identified as actionable in the PR review: - DuplicateSessionAsync now copies the Mode and PostRunUrl fields onto the cloned run-commands so duplicated sessions don't silently drop PowerShell mode and post-run URLs. - ResolvePwsh checks WaitForExit AND ExitCode before caching pwsh.exe, and kills probes that haven't exited so a hung probe can't poison the cache. - Recently-closed reopen now removes the entry from the ring only on launch success — adds PeekMostRecentlyClosed and a ReopenAndRemoveOnSuccessAsync wrapper that checks SessionManager membership as the success signal. - Boot overlay has an 8s fallback (PostBootDoneIfNeeded fires from a Task.Delay scheduled in NavCompleted) so silent sessions don't get stuck behind the spinner indefinitely. - SearchServiceTests seeds session_history rows with explicit unix timestamps instead of Task.Delay(5), eliminating clock-resolution flakiness on CI. - CodeShellManager.csproj copies Assets\app.png to publish output so the WiX manifest entry from the prior commit isn't dangling at install time. - PostRunUrl failures now log to crash.log instead of being silently swallowed. - SessionRunCommandsDialog Mode-combobox tooltip now accurately says "cmd /c <cmd>" instead of claiming Process mode is a direct ConPTY child. Tests: 206/206 pass.

…nd the overlay Addresses Copilot review feedback on the parallel PR #63: `bootDone` was only posted on the first PTY byte, so a child process that writes nothing (or exits without output) would leave the spinner up indefinitely. NavCompleted now also schedules a Task.Delay(8000) → PostBootDoneIfNeeded; the existing Interlocked.CompareExchange guard makes this idempotent if the first byte beat the timer.

- CodeShellManager.csproj: set MetadataUpdaterSupport=false in Debug to disable Hot Reload. F5 currently crashes with System.ExecutionEngineException immediately after Microsoft.Extensions.DotNetDeltaApplier.dll loads under .NET 10.0.8 + VS 18. Disabling metadata updates makes the delta applier a no-op so debugging works. Remove when the runtime bug is fixed upstream. - MainWindow.OnLoaded: --clean mode now also clears in-memory groups, not just sessions. SaveStateAsync is a no-op in --clean so this doesn't touch state.json; it just means leftover groups from prior debug sessions don't bleed into the isolated run. - CLAUDE.md: clarify the --clean description so it matches the new behavior.

Adds MainViewModel.ClearRecentlyClosed() and calls it from the --clean block in MainWindow.OnLoaded. SaveStateAsync is a no-op in --clean mode so this only affects in-memory state; state.json is untouched. Matches the existing session/group clearing behavior so --clean gives a truly blank slate.

Adds a shared VS launch profile so any contributor on .NET 10.0.8 + VS 18 doesn't hit the System.ExecutionEngineException crash that the Hot Reload delta applier triggers at F5 startup. "hotReloadEnabled": false stops VS from injecting Microsoft.Extensions.DotNetDeltaApplier.dll in the first place — the real fix. Csproj's MetadataUpdaterSupport=false remains as a belt-and-braces runtime switch for anyone with a divergent local profile. commandLineArgs="--clean" is also in the profile, matching the documented debug-isolation default in CLAUDE.md. Remove both when the .NET 10 / VS 18 runtime bug is fixed upstream.

AThraen and others added 12 commits May 16, 2026 11:44

feat(spinner): add boot overlay markup + CSS to xterm host pages

9de6332

feat(spinner): add setBootState/bootDone handlers in terminal-init.js

6a3b8d8

feat(spinner): add SetBootContext + post setBootState in TerminalBridge

9a77e22

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

fix(spinner): suppress CS0169 on _bootDoneFlag pending Task 4

d980f09

feat(spinner): show launching overlay until first PTY byte

b6fed0e

feat(spinner): add ShutdownOverlay grid to MainWindow.xaml

e5974b0

feat(spinner): show ShutdownOverlay during OnClosing teardown

7fdffc9

fix(installer): add app.png to WiX manifest (CI verify-manifest check)

6fec182

AThraen requested a review from Copilot May 17, 2026 07:44

Copilot started reviewing on behalf of AThraen May 17, 2026 07:44 View session

Copilot AI reviewed May 17, 2026

View reviewed changes

AThraen added 3 commits May 17, 2026 12:06

AThraen merged commit cf8020c into main May 17, 2026
1 check passed

AThraen deleted the feat/bundle-low-hanging-fruit branch May 17, 2026 11:51

This was referenced May 17, 2026

feat: session start/shutdown spinners #62

Closed

docs: spinner design spec + doc refresh #64

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(bundle): low-hanging fruit + tests, refactor, docs#63

feat(bundle): low-hanging fruit + tests, refactor, docs#63
AThraen merged 16 commits into
mainfrom
feat/bundle-low-hanging-fruit

AThraen commented May 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

AThraen May 17, 2026

Uh oh!

AThraen May 17, 2026

Uh oh!

AThraen May 17, 2026

Uh oh!

AThraen May 17, 2026

Uh oh!

AThraen May 17, 2026

Uh oh!

AThraen May 17, 2026

Uh oh!

AThraen May 17, 2026

Uh oh!

AThraen May 17, 2026

Uh oh!

AThraen May 17, 2026

Uh oh!

AThraen May 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

AThraen commented May 17, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants