diff --git a/CLAUDE.md b/CLAUDE.md index 022f113..c6984e1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1017,7 +1017,7 @@ Each session can have a list of "run commands" — labelled command lines invoke **Data:** `ShellSession.RunCommands: List { Id, Label, CommandLine, IsDefault, Mode, PostRunUrl }`. Exactly one item has `IsDefault=true`; see `RunCommandItem.EnsureSingleDefault`. Persisted to `state.json`. - **`Mode`** (`RunMode.Process` default / `RunMode.PowerShell`) — `Process` runs through `cmd /c` as before; `PowerShell` wraps the command line in `pwsh.exe -NonInteractive -NoLogo -ExecutionPolicy Bypass -EncodedCommand ` (falls back to `powershell.exe` if `pwsh` isn't on PATH). SSH and WSL parents ignore `Mode` — those runs always go through a POSIX shell (`ssh … bash -c` / `wsl.exe … -e -lc`, where the WSL shell is probed per-distro and falls back to `sh` where bash is absent). Use PowerShell when the command relies on pipes (`|`), redirection (`>`), `$env:` variables, or cmdlets. -- **`PostRunUrl`** (`string?`, default `null`) — when set and the run exits with code 0, `Process.Start` opens the URL via `UseShellExecute=true` (default browser). No health-check polling. The value is gated by `RunInstance.IsLaunchableUrl` first: **only absolute `http`/`https` URLs are launched.** ShellExecute would otherwise run a local exe, a `.ps1`, a UNC path or any registered protocol handler, and this fires automatically with no confirmation — and `ImportExportService` deserializes a whole `AppState` (run commands included) from any JSON file the user points at, so the stored value is not trusted. Rejections and launch failures both append to `crash.log`; neither pops UI, since this runs on the PTY-exit callback thread. Scheme-less input (`localhost:5173`) is rejected rather than guessed at. +- **`PostRunUrl`** (`string?`, default `null`) — opened once in the default browser (`UseShellExecute=true`), by whichever comes first: **a loopback URL starts accepting TCP connections** while the run is alive, or the run **exits 0**. The readiness half exists because a URL is almost always a dev server's (`npm run dev`), and a server never exits — with only the exit-0 rule the URL never opened. The watcher (`RunInstance.StartReadyWatcher`) polls every 500ms for up to 2 minutes, connects to every address of the host in parallel (`localhost` is `::1` *and* `127.0.0.1`, servers bind one, and a refused loopback connect costs ~2s on Windows), and stops on exit, Stop or Dispose. It is **loopback-only and skipped for SSH parents**: a remote URL (a deploy target) is reachable before the command has done anything, and an SSH run's server listens on the remote machine's loopback. Those keep the exit-0 rule. Tested in `PostRunUrlReadinessTests`, against real listeners for the probe. The value is gated by `RunInstance.IsLaunchableUrl` first: **only absolute `http`/`https` URLs are launched.** ShellExecute would otherwise run a local exe, a `.ps1`, a UNC path or any registered protocol handler, and this fires automatically with no confirmation — and `ImportExportService` deserializes a whole `AppState` (run commands included) from any JSON file the user points at, so the stored value is not trusted. Rejections and launch failures both append to `crash.log`; neither pops UI, since this runs on the PTY-exit callback thread. Scheme-less input (`localhost:5173`) is rejected rather than guessed at. **Templates:** `RunCommandTemplatesService.SeedFor(folder)` detects project type (top-level scan, first-match: dotnet → cargo → node → python → make) and returns a seed list with fresh Ids. Templates are *copied* onto new sessions at creation time; subsequent edits don't propagate back. SSH sessions skip detection (empty list). diff --git a/src/CodeShellManager/Services/RunInstance.cs b/src/CodeShellManager/Services/RunInstance.cs index 5971571..5bc1c15 100644 --- a/src/CodeShellManager/Services/RunInstance.cs +++ b/src/CodeShellManager/Services/RunInstance.cs @@ -1,6 +1,10 @@ using System; using System.Diagnostics; using System.IO; +using System.Net; +using System.Net.Sockets; +using System.Threading; +using System.Threading.Tasks; using System.Text; using System.Text.RegularExpressions; using CodeShellManager.Models; @@ -50,6 +54,22 @@ public partial class RunInstance : ObservableObject, IDisposable private long _totalAppended; private bool _disposed; + // PostRunUrl is opened at most once per run: by the readiness watcher (a server that has + // started listening) or by a successful exit (a one-shot command), whichever comes first. + private int _urlOpened; + private CancellationTokenSource? _readyCts; + + /// How long the readiness watcher keeps probing a loopback URL before giving up. + internal TimeSpan ReadyTimeout { get; set; } = TimeSpan.FromMinutes(2); + internal TimeSpan ReadyPollInterval { get; set; } = TimeSpan.FromMilliseconds(500); + + /// Test seam: "is something listening behind this URL?". Default: a TCP connect. + internal Func> ReadyProbe { get; set; } = IsListeningAsync; + + /// Test seam: opens the validated URL. Default: ShellExecute → default browser. + internal Action UrlLauncher { get; set; } = + static url => Process.Start(new ProcessStartInfo(url) { UseShellExecute = true }); + public event Action? OutputChanged; public event Action? StateChanged; @@ -136,6 +156,7 @@ public void Start(ShellSession parent) } _pty.Start(command, args, workDir, cols: 200, rows: 50, useJobObject: true); + StartReadyWatcher(parent); } catch (Exception ex) { @@ -211,26 +232,127 @@ private void OnPtyExited() State = ExitCode == 0 ? RunState.ExitedOk : RunState.ExitedFailed; StateChanged?.Invoke(); - // Open post-run URL on success only. ShellExecute hands the URL to the - // OS default browser. We can't pop UI from the PTY-exit callback thread, - // so failures are logged to crash.log for diagnosability rather than silenced. - if (State == RunState.ExitedOk && !string.IsNullOrWhiteSpace(PostRunUrl)) + _readyCts?.Cancel(); + + // A one-shot command (build, deploy) opens its URL when it succeeds. A server never + // exits, so for that case the readiness watcher started in Start() opens it instead. + if (State == RunState.ExitedOk) OpenPostRunUrlOnce(); + } + + /// + /// A run command with a PostRunUrl is usually a dev server (`npm run dev`, `node serve.js`), + /// and a server never exits — so "open the URL when the command exits 0" meant the URL never + /// opened at all. While the run is alive, probe the URL and open it once something is + /// listening there. + /// + /// Only for loopback URLs, and not for SSH parents. A remote URL (a deploy target) is + /// reachable before the command has done anything, so probing it would open the page before + /// the deploy finished — those keep the exit-0 rule. For an SSH parent the server listens on + /// the REMOTE machine's loopback, which a local probe cannot see. + /// + private void StartReadyWatcher(ShellSession parent) + { + if (parent.Kind == SessionKind.Ssh) return; + if (!TryGetLaunchableUrl(PostRunUrl, out string? safeUrl)) return; + var uri = new Uri(safeUrl!); + if (!IsLoopbackHost(uri)) return; + + var cts = new CancellationTokenSource(); + _readyCts = cts; + var probe = ReadyProbe; + var timeout = ReadyTimeout; + var interval = ReadyPollInterval; + _ = Task.Run(async () => { - if (!TryGetLaunchableUrl(PostRunUrl, out string? safeUrl)) + var deadline = DateTime.UtcNow + timeout; + try { - LogPostRunUrl(PostRunUrl, "rejected — only http and https URLs are opened"); - return; + while (!cts.IsCancellationRequested && DateTime.UtcNow < deadline) + { + bool ready; + try { ready = await probe(uri, cts.Token).ConfigureAwait(false); } + catch (OperationCanceledException) { return; } + catch { ready = false; } + if (cts.IsCancellationRequested) return; + if (ready) { OpenPostRunUrlOnce(); return; } + await Task.Delay(interval, cts.Token).ConfigureAwait(false); + } } - // safeUrl, not PostRunUrl: launch the string the validator actually inspected. - // Uri.TryCreate accepts and internally escapes characters that the raw string - // still contains — a quote or space survives into ShellExecute, which expands it - // into the handler's registered `shell\open\command` template. Modern browsers - // pass --single-argument and are unaffected; other registered handlers may not - // be. Validating one string and launching a different one is the bug class, - // regardless of who is currently immune to it. - try { Process.Start(new ProcessStartInfo(safeUrl!) { UseShellExecute = true }); } - catch (Exception ex) { LogPostRunUrl(PostRunUrl, ex.Message); } + catch (OperationCanceledException) { } + }); + } + + /// localhost, 127.0.0.0/8, [::1] — and 0.0.0.0 / [::], which dev servers print. + internal static bool IsLoopbackHost(Uri uri) + { + if (uri.IsLoopback) return true; + return IPAddress.TryParse(uri.Host.Trim('[', ']'), out var ip) + && (ip.Equals(IPAddress.Any) || ip.Equals(IPAddress.IPv6Any)); + } + + private static async Task IsListeningAsync(Uri uri, CancellationToken ct) + { + string host = uri.Host.Trim('[', ']'); + if (host == "0.0.0.0") host = "127.0.0.1"; + else if (host == "::") host = "::1"; + + // "localhost" is ::1 AND 127.0.0.1, and a server usually binds only one of them (Vite on + // Node 17+ binds ::1, many others 127.0.0.1). Probe every address in parallel: on + // Windows a refused loopback connect takes ~2s (SYN retries), so trying them in turn + // inside one timeout never reached the second address. + IPAddress[] addresses; + try + { + addresses = IPAddress.TryParse(host, out var literal) + ? [literal] + : await Dns.GetHostAddressesAsync(host, ct).ConfigureAwait(false); + } + catch (SocketException) { return false; } + + var attempts = new Task[addresses.Length]; + for (int i = 0; i < addresses.Length; i++) + attempts[i] = ConnectsAsync(new IPEndPoint(addresses[i], uri.Port), ct); + foreach (bool ok in await Task.WhenAll(attempts).ConfigureAwait(false)) + if (ok) return true; + return false; + } + + private static async Task ConnectsAsync(IPEndPoint endpoint, CancellationToken ct) + { + using var socket = new Socket(endpoint.AddressFamily, SocketType.Stream, ProtocolType.Tcp); + using var attempt = CancellationTokenSource.CreateLinkedTokenSource(ct); + attempt.CancelAfter(TimeSpan.FromSeconds(3)); + try + { + await socket.ConnectAsync(endpoint, attempt.Token).ConfigureAwait(false); + return true; + } + catch (OperationCanceledException) { return false; } + catch (SocketException) { return false; } + } + + private void OpenPostRunUrlOnce() + { + if (string.IsNullOrWhiteSpace(PostRunUrl)) return; + if (Interlocked.Exchange(ref _urlOpened, 1) != 0) return; + + // ShellExecute hands the URL to the OS default browser. This runs on the PTY-exit + // callback thread or the watcher's pool thread, so failures go to crash.log for + // diagnosability rather than popping UI. + if (!TryGetLaunchableUrl(PostRunUrl, out string? safeUrl)) + { + LogPostRunUrl(PostRunUrl, "rejected — only http and https URLs are opened"); + return; } + // safeUrl, not PostRunUrl: launch the string the validator actually inspected. + // Uri.TryCreate accepts and internally escapes characters that the raw string + // still contains — a quote or space survives into ShellExecute, which expands it + // into the handler's registered `shell\open\command` template. Modern browsers + // pass --single-argument and are unaffected; other registered handlers may not + // be. Validating one string and launching a different one is the bug class, + // regardless of who is currently immune to it. + try { UrlLauncher(safeUrl!); } + catch (Exception ex) { LogPostRunUrl(PostRunUrl, ex.Message); } } /// @@ -390,6 +512,7 @@ public void Dispose() { if (_disposed) return; _disposed = true; + _readyCts?.Cancel(); if (_pty != null) { _pty.DataReceived -= OnPtyData; diff --git a/src/CodeShellManager/Views/SessionRunCommandsDialog.xaml b/src/CodeShellManager/Views/SessionRunCommandsDialog.xaml index 496b253..e3445c1 100644 --- a/src/CodeShellManager/Views/SessionRunCommandsDialog.xaml +++ b/src/CodeShellManager/Views/SessionRunCommandsDialog.xaml @@ -106,7 +106,7 @@ Background="#313244" Foreground="#cdd6f4" BorderBrush="#45475a" CaretBrush="#cdd6f4" Padding="6,4" Margin="0,0,8,0" VerticalContentAlignment="Center" - ToolTip="Optional. Opens in the default browser when the command exits 0. Must be a full http:// or https:// URL — e.g. http://localhost:5173, not localhost:5173."/> + ToolTip="Optional. Opens in the default browser once — for a localhost URL as soon as the server starts listening there, otherwise when the command exits 0. Must be a full http:// or https:// URL — e.g. http://localhost:5173, not localhost:5173."/> +public class PostRunUrlReadinessTests +{ + private sealed class FakePty : IPseudoTerminal + { + public int? ExitCode { get; private set; } +#pragma warning disable CS0067 // never raised: these tests don't need output + public event Action? DataReceived; +#pragma warning restore CS0067 + public event Action? Exited; + public void Start(string command, string args, string workingDirectory, + int cols = 220, int rows = 50, bool useJobObject = false) { } + public void Exit(int code) { ExitCode = code; Exited?.Invoke(); } + public void Dispose() { } + } + + private static readonly TimeSpan Wait = TimeSpan.FromSeconds(5); + private static readonly TimeSpan Short = TimeSpan.FromMilliseconds(250); + + private static (RunInstance run, FakePty pty, BlockingCollection opened) Make( + string? url, Func>? probe = null) + { + var pty = new FakePty(); + var run = new RunInstance( + new RunCommandItem { Id = "r", Label = "Run", CommandLine = "npm run dev", PostRunUrl = url }, + () => pty); + var opened = new BlockingCollection(); + run.UrlLauncher = opened.Add; + run.ReadyPollInterval = TimeSpan.FromMilliseconds(10); + if (probe != null) run.ReadyProbe = probe; + return (run, pty, opened); + } + + private static ShellSession Local() => new() { WorkingFolder = Environment.CurrentDirectory }; + + [Fact] + public void Server_OpensWhenListening_WithoutExiting() + { + int calls = 0; + var (run, _, opened) = Make("http://localhost:5173", + (_, _) => Task.FromResult(Interlocked.Increment(ref calls) >= 3)); + run.Start(Local()); + + Assert.True(opened.TryTake(out var url, Wait)); + Assert.Equal("http://localhost:5173/", url); + Assert.Equal(RunState.Running, run.State); + run.Dispose(); + } + + [Fact] + public void Server_ThenExitZero_OpensOnlyOnce() + { + var (run, pty, opened) = Make("http://127.0.0.1:4377", (_, _) => Task.FromResult(true)); + run.Start(Local()); + Assert.True(opened.TryTake(out _, Wait)); + pty.Exit(0); + Assert.False(opened.TryTake(out _, Short)); + } + + [Fact] + public void OneShot_ExitZero_Opens() + { + var (run, pty, opened) = Make("http://localhost:8000", (_, _) => Task.FromResult(false)); + run.Start(Local()); + pty.Exit(0); + Assert.True(opened.TryTake(out _, Wait)); + } + + [Fact] + public void ExitNonZero_BeforeReady_DoesNotOpen() + { + var (run, pty, opened) = Make("http://localhost:8000", (_, _) => Task.FromResult(false)); + run.Start(Local()); + pty.Exit(1); + Assert.False(opened.TryTake(out _, Short)); + } + + [Fact] + public void Stop_BeforeReady_DoesNotOpenLater() + { + int ready = 0; + var (run, _, opened) = Make("http://localhost:8000", + (_, _) => Task.FromResult(Volatile.Read(ref ready) == 1)); + run.Start(Local()); + run.Stop(); + Volatile.Write(ref ready, 1); + Assert.False(opened.TryTake(out _, Short)); + } + + [Fact] + public void RemoteUrl_IsNotProbed_OpensOnlyOnExitZero() + { + int calls = 0; + var (run, pty, opened) = Make("https://example.com/deployed", + (_, _) => { Interlocked.Increment(ref calls); return Task.FromResult(true); }); + run.Start(Local()); + Assert.False(opened.TryTake(out _, Short)); + Assert.Equal(0, Volatile.Read(ref calls)); + pty.Exit(0); + Assert.True(opened.TryTake(out _, Wait)); + } + + [Fact] + public void SshParent_IsNotProbed() + { + int calls = 0; + var (run, _, opened) = Make("http://localhost:5173", + (_, _) => { Interlocked.Increment(ref calls); return Task.FromResult(true); }); + run.Start(new ShellSession { Kind = SessionKind.Ssh, SshHost = "box", SshUser = "u" }); + Assert.False(opened.TryTake(out _, Short)); + Assert.Equal(0, Volatile.Read(ref calls)); + run.Dispose(); + } + + [Fact] + public void NonHttpUrl_NeverProbedOrOpened() + { + int calls = 0; + var (run, pty, opened) = Make(@"C:\Windows\System32\calc.exe", + (_, _) => { Interlocked.Increment(ref calls); return Task.FromResult(true); }); + run.Start(Local()); + pty.Exit(0); + Assert.False(opened.TryTake(out _, Short)); + Assert.Equal(0, Volatile.Read(ref calls)); + } + + [Theory] + [InlineData("http://localhost:5173", true)] + [InlineData("http://127.0.0.1:4377", true)] + [InlineData("http://127.5.0.1/", true)] + [InlineData("http://[::1]:8080/", true)] + [InlineData("http://0.0.0.0:8000", true)] + [InlineData("http://192.168.1.10:8000", false)] + [InlineData("https://example.com", false)] + [InlineData("http://localhost.example.com", false)] + public void IsLoopbackHost(string url, bool expected) + => Assert.Equal(expected, RunInstance.IsLoopbackHost(new Uri(url))); + + [Theory] + [InlineData("127.0.0.1", "http://localhost:{0}")] // IPv4-only server, reached via "localhost" + [InlineData("::1", "http://localhost:{0}")] // Vite on Node 17+ binds ::1 only + [InlineData("127.0.0.1", "http://0.0.0.0:{0}")] + public void DefaultProbe_DetectsRealListener(string bindAddress, string urlFormat) + { + var listener = new TcpListener(IPAddress.Parse(bindAddress), 0); + listener.Start(); + try + { + int port = ((IPEndPoint)listener.LocalEndpoint).Port; + var pty = new FakePty(); + var run = new RunInstance(new RunCommandItem + { + Id = "r", Label = "Run", CommandLine = "x", + PostRunUrl = string.Format(urlFormat, port), + }, () => pty); + var opened = new BlockingCollection(); + run.UrlLauncher = opened.Add; + run.Start(Local()); + Assert.True(opened.TryTake(out _, Wait)); + run.Dispose(); + } + finally { listener.Stop(); } + } + + [Fact] + public void DefaultProbe_NothingListening_DoesNotOpen() + { + // Grab a free port, then release it so nothing listens there. + var l = new TcpListener(IPAddress.Loopback, 0); + l.Start(); + int port = ((IPEndPoint)l.LocalEndpoint).Port; + l.Stop(); + + var pty = new FakePty(); + var run = new RunInstance(new RunCommandItem + { + Id = "r", Label = "Run", CommandLine = "x", PostRunUrl = $"http://127.0.0.1:{port}", + }, () => pty); + var opened = new BlockingCollection(); + run.UrlLauncher = opened.Add; + run.ReadyPollInterval = TimeSpan.FromMilliseconds(20); + run.Start(Local()); + Assert.False(opened.TryTake(out _, TimeSpan.FromMilliseconds(600))); + run.Dispose(); + } +}