fix: seal child-process stdin on Windows (first-run hang) (#74)

* fix: seal child-process stdin on Windows (first-run hang)

mcpp's first-run flow on Windows was hanging at xlings / xim / curl / git
grandchildren that block on terminal stdin, forcing users to press Enter
repeatedly to advance bootstrap and toolchain install.

Root cause: process::seal_stdin was a no-op on Windows, and
install_with_progress's direct-install path deliberately bypassed it.
The POSIX side has had </dev/null sealing since PR #55 (macOS xcrun hang
fix); Windows never received the equivalent fix. PR #57 only suppressed
stdout/stderr noise (>/dev/null 2>&1) and did not touch stdin.

Changes:
  - process.cppm: seal_stdin now appends "<NUL" on Windows (matches POSIX
    behavior). All capture / run_silent / run_streaming / run_passthrough
    callers gain the protection automatically.
  - xlings.cppm: install_with_progress's direct path explicitly appends
    "<NUL" on Windows. POSIX keeps the original behavior conservatively.
  - shell.cppm: silent_redirect docstring corrected — it never touched
    stdin, that's seal_stdin's job. Implementation unchanged.

Regression coverage:
  - tests/unit/test_process_seal_stdin.cpp — deterministic reproduction
    test. Rebinds the test process's own stdin to an open, empty,
    never-closing pipe, then calls run_silent / capture / run_streaming
    with a child that reads stdin (more on Windows, cat on POSIX).
    Without the fix the child would block forever waiting on our pipe;
    with the fix it reads NUL / /dev/null and exits immediately. 5-second
    upper bound (real runs complete in <100ms).
  - ci-windows.yml — adds a step that launches mcpp via
    System.Diagnostics.Process with RedirectStandardInput=$true (parent
    holds the child's stdin open but never writes). Runs mcpp --version,
    mcpp build, mcpp run. Without the fix, any grandchild reading stdin
    blocks → step times out → CI fails. With the fix → all complete.

* ci(windows): convert MCPP_SELF MSYS path → Windows path; rename pwsh $Args

Two issues with the regression step from the previous commit (both showed
up only on the actual Windows runner, not in local validation):

1. MCPP_SELF was set in an earlier bash step via `pwd` (git-bash) so the
   value is MSYS-style (e.g. /d/a/mcpp/...). Bash steps tolerate it but
   pwsh's `&` operator can't exec it ("not recognized as a name of a
   cmdlet, function, script file, or executable program"). Convert via
   cygpath -w before use.

2. `$Args` is a PowerShell automatic variable inside function scope; a
   `param([string]$Args)` does not bind cleanly. Renamed to $McppArgs
   to avoid the collision (also updated call sites).

Author: Sunrisepeak

.github/workflows/ci-windows.yml

@@ -90,6 +90,92 @@ jobs:
           export MCPP_VENDORED_XLINGS=$(cygpath -w "$USERPROFILE/.xlings/subos/default/bin/xlings.exe")
           "$MCPP_SELF" test
 
+      # Regression test for the Windows first-run "press Enter to advance" hang.
+      # Launches mcpp with an OPEN, EMPTY, never-closing stdin pipe. Without
+      # seal_stdin's Windows fix, any grandchild that reads stdin would inherit
+      # our pipe and block forever — caught by the timeout below. With the fix,
+      # every subprocess stdin is redirected from NUL → no possibility of hang.
+      - name: "Regression: mcpp survives open-empty-stdin (Windows hang fix)"
+        shell: pwsh
+        timeout-minutes: 15
+        env:
+          MCPP_VENDORED_XLINGS: ${{ env.XLINGS_BIN }}
+        run: |
+          $ErrorActionPreference = 'Stop'
+
+          # MCPP_SELF was set in a bash step as an MSYS-style path
+          # (e.g. /d/a/mcpp/...). PowerShell can't exec that — convert it
+          # to a native Windows path via the git-bash cygpath that ships
+          # on the runner.
+          $mcppExe = (& 'C:\Program Files\Git\usr\bin\cygpath.exe' -w $env:MCPP_SELF).Trim()
+          Write-Host "Resolved MCPP_SELF (Windows form): $mcppExe"
+          if (-not (Test-Path $mcppExe)) {
+              throw "MCPP_SELF after cygpath not found: $mcppExe"
+          }
+
+          $tmp = Join-Path $env:RUNNER_TEMP ("stdin-hang-test-" + [guid]::NewGuid().ToString('N'))
+          New-Item -ItemType Directory -Path $tmp | Out-Null
+          Set-Location $tmp
+          & $mcppExe new hello_stdin
+          Set-Location hello_stdin
+
+          function Invoke-McppWithOpenStdin {
+              param([string]$McppPath, [string]$McppArgs, [int]$TimeoutSeconds = 300)
+
+              $psi = [System.Diagnostics.ProcessStartInfo]::new()
+              $psi.FileName               = $McppPath
+              $psi.Arguments              = $McppArgs
+              $psi.WorkingDirectory       = (Get-Location).Path
+              $psi.UseShellExecute        = $false
+              $psi.RedirectStandardInput  = $true   # parent holds child's stdin
+              $psi.RedirectStandardOutput = $true
+              $psi.RedirectStandardError  = $true
+              # By default the child inherits the parent's env (we did not
+              # touch $psi.Environment) so MCPP_VENDORED_XLINGS / PATH / etc.
+              # propagate.
+
+              $p = [System.Diagnostics.Process]::Start($psi)
+
+              # Async-drain stdout/stderr so a full output buffer doesn't
+              # itself deadlock the child (separate failure mode from the
+              # stdin hang we're testing).
+              $stdoutTask = $p.StandardOutput.ReadToEndAsync()
+              $stderrTask = $p.StandardError.ReadToEndAsync()
+
+              # NEVER write or close $p.StandardInput — the pipe stays open
+              # and empty for the lifetime of the child. Any grandchild that
+              # reads stdin will block on this pipe → caught by WaitForExit.
+
+              if (-not $p.WaitForExit($TimeoutSeconds * 1000)) {
+                  try { $p.Kill($true) } catch {}
+                  Write-Host "----- captured stdout -----"
+                  Write-Host $stdoutTask.Result
+                  Write-Host "----- captured stderr -----"
+                  Write-Host $stderrTask.Result
+                  throw "REGRESSION: 'mcpp $McppArgs' HUNG with open-empty stdin after ${TimeoutSeconds}s. The Windows seal_stdin fix is not effective."
+              }
+
+              Write-Host "----- stdout -----"
+              Write-Host $stdoutTask.Result
+              Write-Host "----- stderr -----"
+              Write-Host $stderrTask.Result
+
+              if ($p.ExitCode -ne 0) {
+                  throw "'mcpp $McppArgs' exited with code $($p.ExitCode) (no hang, but failed)."
+              }
+          }
+
+          Write-Host '=== T1: mcpp --version (sanity, fast path) ==='
+          Invoke-McppWithOpenStdin -McppPath $mcppExe -McppArgs '--version' -TimeoutSeconds 30
+
+          Write-Host '=== T2: mcpp build (full bootstrap + toolchain + dep resolve + compile) ==='
+          Invoke-McppWithOpenStdin -McppPath $mcppExe -McppArgs 'build' -TimeoutSeconds 600
+
+          Write-Host '=== T3: mcpp run (post-build run path) ==='
+          Invoke-McppWithOpenStdin -McppPath $mcppExe -McppArgs 'run' -TimeoutSeconds 120
+
+          Write-Host 'SUCCESS: mcpp completes with open-empty stdin → Windows seal_stdin fix verified.'
+
       - name: E2E suite
         shell: bash
         run: |

src/platform/process.cppm

@@ -1,10 +1,14 @@
 // mcpp.platform.process — platform-aware process runner.
 //
 // Centralises all popen/system usage so callers do not scatter #if _WIN32
-// guards or duplicate the popen-read loop.  On POSIX, all functions
-// automatically redirect stdin from /dev/null to prevent interactive
-// prompts from child processes (fixes macOS first-run hangs where xcrun
-// or xcode-select would block waiting for user input).
+// guards or duplicate the popen-read loop.  All functions automatically
+// seal stdin (redirect from /dev/null on POSIX, from NUL on Windows) to
+// prevent interactive prompts from child processes:
+//   - POSIX: fixes macOS first-run hangs where xcrun / xcode-select would
+//     block waiting for user input.
+//   - Windows: fixes first-run hangs where xlings / xim / curl / git child
+//     processes would block on terminal stdin, forcing the user to press
+//     Enter repeatedly to advance bootstrap / toolchain install.
 //
 // Entry points:
 //   capture        — run a command, capture stdout
@@ -73,12 +77,16 @@ namespace mcpp::platform::process {
 
 namespace {
 
-// On POSIX, append "< /dev/null" to prevent child processes from reading
-// stdin.  This fixes macOS first-run hangs where tools like xcrun or
-// xcode-select block waiting for user input.
+// Append a non-interactive stdin redirect to prevent child processes from
+// blocking on terminal input.
+//   - POSIX:  "< /dev/null"  — fixes macOS xcrun / xcode-select hangs.
+//   - Windows: "<NUL"        — fixes xlings / xim / curl / git hangs on
+//                              first-run toolchain install (user otherwise
+//                              had to press Enter repeatedly to advance).
+// `cmd.exe` accepts `<NUL` as a redirect for an immediately-EOF stdin.
 std::string seal_stdin(std::string_view cmd) {
 #if defined(_WIN32)
-    return std::string(cmd);
+    return std::string(cmd) + " <NUL";
 #else
     return std::string(cmd) + " </dev/null";
 #endif

src/platform/shell.cppm

@@ -20,7 +20,10 @@ export namespace mcpp::platform::shell {
 // Platform-aware shell argument quoting.
 std::string quote(std::string_view s);
 
-// Full silent redirect (stdin + stdout + stderr → /dev/null).
+// Silent redirect — stdout + stderr → /dev/null (or NUL on Windows).
+// stdin is NOT touched here; that's the responsibility of
+// mcpp::platform::process::seal_stdin, which is auto-applied by capture /
+// run_silent / run_streaming on all platforms.
 #if defined(_WIN32)
 constexpr std::string_view silent_redirect = ">nul 2>&1";
 #else

src/xlings.cppm

@@ -681,9 +681,16 @@ int install_with_progress(const Env& env, std::string_view target,
     {
         auto directCmd = build_command_prefix(env) +
             std::format(" install {} -y {}", target, mcpp::platform::shell::silent_redirect);
-        // Use std::system() directly — do NOT redirect stdin via </dev/null
-        // because xlings may need stdin for subprocess coordination during
-        // large package extraction.
+        // Windows: explicitly seal stdin (<NUL) so xlings and any grandchildren
+        // (curl, git, 7z, etc.) cannot block on a terminal read. The earlier
+        // comment claimed POSIX must keep stdin open for "subprocess
+        // coordination" — that's never been observed in practice on Linux/macOS,
+        // and on Windows it caused users to press Enter repeatedly during first-
+        // run toolchain install. POSIX keeps the original behavior to stay
+        // conservative.
+        if constexpr (mcpp::platform::is_windows) {
+            directCmd += " <NUL";
+        }
         int directRc = mcpp::platform::process::extract_exit_code(
             std::system(directCmd.c_str()));
         if (directRc == 0) return 0;

tests/unit/test_process_seal_stdin.cpp

@@ -0,0 +1,164 @@
+// Regression test for the Windows first-run hang where xlings / xim / curl /
+// git child processes blocked on terminal stdin, forcing the user to press
+// Enter repeatedly to advance bootstrap / toolchain install.
+//
+// `mcpp::platform::process::{capture, run_silent, run_streaming}` MUST seal
+// stdin so any child reading stdin gets immediate EOF, not a blocking read.
+//   POSIX:   appends "</dev/null"
+//   Windows: appends "<NUL"
+//
+// Test strategy: rebind this test process's own stdin to an open, empty,
+// never-closing pipe. Then invoke run_silent / capture with a child that
+// reads stdin. Without seal_stdin, the child would inherit our pipe stdin
+// and block forever; the gtest runner would then hang until CI timeout.
+// With the fix, the child reads from NUL / /dev/null and exits immediately.
+
+#include <gtest/gtest.h>
+#include <chrono>
+
+#if defined(_WIN32)
+#include <windows.h>
+#include <io.h>
+#include <fcntl.h>
+#else
+#include <fcntl.h>
+#include <unistd.h>
+#endif
+
+import std;
+import mcpp.platform.process;
+
+namespace {
+
+// Maximum seconds a sealed-stdin command may take before we declare it
+// "hung". Real child runs (cat / more reading from EOF stdin) complete in
+// well under 100ms on any modern machine, so 5s is a very generous bound.
+constexpr int kMaxSealedSeconds = 5;
+
+// RAII: rebind STDIN to an open, empty, never-closing pipe for the duration
+// of one test. Restores the original stdin on destruction.
+class OpenEmptyStdinScope {
+public:
+    OpenEmptyStdinScope() {
+#if defined(_WIN32)
+        SECURITY_ATTRIBUTES sa{};
+        sa.nLength = sizeof(sa);
+        sa.bInheritHandle = TRUE;
+        if (!CreatePipe(&hRead_, &hWrite_, &sa, 0)) {
+            std::abort();
+        }
+        // Make the read end inheritable (already is via sa, but be explicit).
+        SetHandleInformation(hRead_, HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT);
+
+        // Save the original stdin (both Win32 handle + CRT fd) so we can
+        // restore in the destructor.
+        origStdinHandle_ = GetStdHandle(STD_INPUT_HANDLE);
+        origStdinFd_     = _dup(0);
+
+        // Bind the pipe-read-end as our process's stdin at both layers.
+        SetStdHandle(STD_INPUT_HANDLE, hRead_);
+        int newFd = _open_osfhandle(reinterpret_cast<intptr_t>(hRead_),
+                                    _O_RDONLY | _O_BINARY);
+        if (newFd >= 0) {
+            _dup2(newFd, 0);
+            _close(newFd);  // _dup2 keeps a reference; we're done with newFd
+        }
+#else
+        if (::pipe(pipeFds_) != 0) std::abort();
+        origStdinFd_ = ::dup(0);
+        ::dup2(pipeFds_[0], 0);
+#endif
+    }
+
+    ~OpenEmptyStdinScope() {
+#if defined(_WIN32)
+        // Restore original stdin.
+        if (origStdinFd_ >= 0) {
+            _dup2(origStdinFd_, 0);
+            _close(origStdinFd_);
+        }
+        SetStdHandle(STD_INPUT_HANDLE, origStdinHandle_);
+        if (hWrite_) CloseHandle(hWrite_);
+        if (hRead_)  CloseHandle(hRead_);
+#else
+        if (origStdinFd_ >= 0) {
+            ::dup2(origStdinFd_, 0);
+            ::close(origStdinFd_);
+        }
+        ::close(pipeFds_[0]);
+        ::close(pipeFds_[1]);
+#endif
+    }
+
+    OpenEmptyStdinScope(const OpenEmptyStdinScope&) = delete;
+    OpenEmptyStdinScope& operator=(const OpenEmptyStdinScope&) = delete;
+
+private:
+#if defined(_WIN32)
+    HANDLE hRead_  = nullptr;
+    HANDLE hWrite_ = nullptr;          // intentionally never written → reader blocks
+    HANDLE origStdinHandle_ = nullptr;
+    int    origStdinFd_     = -1;
+#else
+    int pipeFds_[2]    = {-1, -1};      // intentionally never written → reader blocks
+    int origStdinFd_   = -1;
+#endif
+};
+
+// A child command that reads stdin to EOF and exits.
+// With seal_stdin in effect → stdin is NUL / /dev/null → child exits immediately.
+// Without seal_stdin AND with an open-empty parent stdin → child blocks forever.
+constexpr std::string_view kStdinReaderCmd =
+#if defined(_WIN32)
+    "more >nul 2>&1"
+#else
+    "cat >/dev/null"
+#endif
+    ;
+
+template <class F>
+double time_seconds(F&& fn) {
+    auto t0 = std::chrono::steady_clock::now();
+    fn();
+    auto t1 = std::chrono::steady_clock::now();
+    return std::chrono::duration<double>(t1 - t0).count();
+}
+
+} // namespace
+
+// run_silent: must seal stdin so the child does not inherit our pipe stdin
+// and block forever.
+TEST(ProcessSealStdin, RunSilentDoesNotHangWhenParentStdinIsOpenPipe) {
+    OpenEmptyStdinScope scope;
+    double elapsed = time_seconds([] {
+        (void)mcpp::platform::process::run_silent(kStdinReaderCmd);
+    });
+    EXPECT_LT(elapsed, static_cast<double>(kMaxSealedSeconds))
+        << "run_silent took " << elapsed
+        << "s — child blocked on stdin → seal_stdin is broken or not applied";
+}
+
+// capture: must also seal stdin (it shares seal_stdin with run_silent).
+TEST(ProcessSealStdin, CaptureDoesNotHangWhenParentStdinIsOpenPipe) {
+    OpenEmptyStdinScope scope;
+    double elapsed = time_seconds([] {
+        (void)mcpp::platform::process::capture(kStdinReaderCmd);
+    });
+    EXPECT_LT(elapsed, static_cast<double>(kMaxSealedSeconds))
+        << "capture took " << elapsed
+        << "s — child blocked on stdin → seal_stdin is broken or not applied";
+}
+
+// run_streaming: same property — children spawned via popen-streaming must
+// not inherit a live stdin.
+TEST(ProcessSealStdin, RunStreamingDoesNotHangWhenParentStdinIsOpenPipe) {
+    OpenEmptyStdinScope scope;
+    double elapsed = time_seconds([] {
+        (void)mcpp::platform::process::run_streaming(
+            kStdinReaderCmd,
+            [](std::string_view) {});
+    });
+    EXPECT_LT(elapsed, static_cast<double>(kMaxSealedSeconds))
+        << "run_streaming took " << elapsed
+        << "s — child blocked on stdin → seal_stdin is broken or not applied";
+}