Platform-aware default shell + Windows recording fixes

Default recording shell is now platform-aware (config::default_shell), used by
both Config::with_defaults() and the menu's runtime fallback:

  * Unix: $SHELL, falling back to /bin/sh.
  * Windows: pwsh.exe when PowerShell 7 is on PATH, else powershell.exe.
    Previously with_defaults() read $SHELL on every platform, so Windows got an
    empty default and fell back to /bin/sh, which does not exist there.

pwsh is preferred because aterm forwards its environment to the child: launched
from pwsh, the inherited PSModulePath points at PowerShell 7's modules, and
handing that to a Windows PowerShell 5.1 child makes it load PS7 module versions
incompatible with 5.1 — PSReadLine and even core modules fail to load ("Cannot
load PSReadline module"). Launching pwsh keeps the shell and its PSModulePath
consistent. The unused per-platform default_shell() helpers are removed.

Windows console handling for the recording (no-op on Unix):

  * TerminalModeGuard enables ENABLE_VIRTUAL_TERMINAL_INPUT (and clears the
    cooked-input flags) so special keys — arrows, Home/End, function keys — are
    captured as VT escape sequences and forwarded to the child; ReadConsoleW
    otherwise returns only typed characters and drops them. It also enables
    ENABLE_VIRTUAL_TERMINAL_PROCESSING + DISABLE_NEWLINE_AUTO_RETURN so the
    child's VT output (colours, cursor motion) renders instead of printing escape
    bytes. The exact prior console modes are restored on drop.

  * StdinPoller's reader thread gates each blocking console read behind a 50ms
    WaitForSingleObject on the console input handle, so it re-checks its stop
    flag and exits on its own at teardown. This keeps the thread from outliving
    the recording and stealing the post-recording menu's first keystroke, and
    avoids an un-interruptible read that would otherwise hang the menu until a
    keypress.

These Windows console APIs have no safe-wrapper equivalent (the way rustix
covers the Unix syscalls), so the crate-wide forbid(unsafe_code) is relaxed to
deny(unsafe_code) with a single scoped, documented #[allow(unsafe_code)] on
recorder::windows; each FFI call has a SAFETY comment. Adds windows-sys as a
cfg(windows) dependency.

Verified: 164 tests pass; clippy clean on the native and x86_64-pc-windows-gnu
targets. Windows runtime behavior confirmed by the reporter (PSReadLine loads,
special keys work, exit drops straight to the menu).

Author: jrozner

Cargo.lock

@@ -37,5 +37,14 @@ url = "2.5.8"
 rustix = { version = "1.1.4", features = ["event"] }
 signal-hook = "0.4.4"
 
+# Console VT input/output modes for ConPTY forwarding and clean stdin-reader
+# shutdown after recording; see src/recorder/windows.rs.
+[target.'cfg(windows)'.dependencies]
+windows-sys = { version = "0.61", features = [
+    "Win32_Foundation",
+    "Win32_System_Console",
+    "Win32_System_Threading",
+] }
+
 [dev-dependencies]
 httpmock = "0.8.3"

Cargo.toml

@@ -75,7 +75,7 @@ The config file follows the XDG standard and lives at
 | Config file key  | CLI flag                  | Meaning                                                            |
 | ---------------- | ------------------------- | ----------------------------------------------------------------- |
 | `outputDir`      |                           | Where to store recording files (defaults to the XDG data dir)     |
-| `recordingShell` | `-s`, `--shell`           | Shell to launch (defaults to `$SHELL`)                            |
+| `recordingShell` | `-s`, `--shell`           | Shell to launch (defaults to `$SHELL` on Unix; on Windows, `pwsh` if installed, else `powershell.exe`) |
 | `operationSlug`  | `--operation`             | Operation to upload to (can also be selected before recording)    |
 | `apiURL`         |                           | Where the ASHIRT backend service is located                       |
 | `accessKey`      |                           | Access Key for the backend (created in the frontend)              |

README.md

@@ -157,13 +157,14 @@ struct ConfigFile {
 impl Config {
     /// Returns the built-in defaults — the lowest-precedence source.
     ///
-    /// Mirrors the Go `TermRecorderConfigWithDefaults`: schema version `1`, the
-    /// recording shell taken from the `SHELL` environment variable, and the
+    /// Mirrors the Go `TermRecorderConfigWithDefaults`: schema version `1`, a
+    /// platform-appropriate recording shell (see [`default_shell`] — `$SHELL` on
+    /// Unix, PowerShell on Windows, preferring `pwsh` when installed), and the
     /// output base defaulting to aterm's per-user XDG data directory.
     pub fn with_defaults() -> Self {
         Config {
             config_version: 1,
-            recording_shell: std::env::var("SHELL").unwrap_or_default(),
+            recording_shell: default_shell(),
             // Recordings are *data*, so base them under aterm's per-user XDG
             // data directory by default (`~/.local/share/aterm` on Linux,
             // honouring `$XDG_DATA_HOME`; the platform data dir on Windows;
@@ -459,6 +460,54 @@ fn default_output_dir() -> String {
     }
 }
 
+/// Returns the platform-appropriate default recording shell.
+///
+/// * Unix (Linux/macOS): the user's login shell from `$SHELL`, falling back to
+///   `/bin/sh` when it is unset or empty. This is what "pull whatever the
+///   `SHELL` environment variable is" means at configuration time.
+/// * Windows: PowerShell 7 (`pwsh.exe`) when it is on `PATH`, otherwise Windows
+///   PowerShell (`powershell.exe`). Windows has no `$SHELL`, and `%COMSPEC%`
+///   points at the legacy `cmd.exe`, so PowerShell is the saner modern default.
+///   `pwsh` is preferred because aterm forwards its environment to the child:
+///   when launched from `pwsh`, the inherited `PSModulePath` points at
+///   PowerShell 7's modules, which breaks module loading (PSReadLine included)
+///   in a Windows PowerShell 5.1 child. Launching `pwsh` keeps the shell and its
+///   `PSModulePath` consistent; `powershell.exe` is the always-present fallback.
+///
+/// Always returns a non-empty value, so it doubles as the last-resort fallback
+/// when no shell is configured (see [`crate::menu`]).
+pub fn default_shell() -> String {
+    #[cfg(windows)]
+    {
+        if executable_on_path("pwsh.exe") {
+            "pwsh.exe".to_string()
+        } else {
+            "powershell.exe".to_string()
+        }
+    }
+    #[cfg(not(windows))]
+    {
+        match std::env::var("SHELL") {
+            Ok(shell) if !shell.trim().is_empty() => shell,
+            _ => "/bin/sh".to_string(),
+        }
+    }
+}
+
+/// Returns whether `exe` is found in any directory on `PATH`.
+///
+/// A minimal `which`-style lookup (no extra dependency) used to prefer
+/// `pwsh.exe` over `powershell.exe` only when PowerShell 7 is actually
+/// installed. `PATH` is split with the platform separator via
+/// [`std::env::split_paths`].
+#[cfg(windows)]
+fn executable_on_path(exe: &str) -> bool {
+    let Some(path) = std::env::var_os("PATH") else {
+        return false;
+    };
+    std::env::split_paths(&path).any(|dir| dir.join(exe).is_file())
+}
+
 /// Returns the platform configuration directory for aterm.
 ///
 /// A plain `aterm` directory under the per-user config base:
@@ -803,6 +852,38 @@ mod tests {
         fs::remove_dir(&dir).ok();
     }
 
+    /// The default shell is always a non-empty, platform-appropriate value, so
+    /// `with_defaults` never seeds an empty `recording_shell` and the menu
+    /// fallback always has something to launch.
+    #[test]
+    fn default_shell_is_non_empty_and_platform_appropriate() {
+        let shell = default_shell();
+        assert!(!shell.trim().is_empty(), "default shell must be non-empty");
+
+        // Windows prefers `pwsh.exe` when on PATH, else `powershell.exe`.
+        #[cfg(windows)]
+        assert!(
+            shell == "pwsh.exe" || shell == "powershell.exe",
+            "Windows default shell must be pwsh.exe or powershell.exe, got {shell}"
+        );
+
+        // On Unix the default is `$SHELL` when set, else `/bin/sh`.
+        #[cfg(not(windows))]
+        match std::env::var("SHELL") {
+            Ok(s) if !s.trim().is_empty() => assert_eq!(
+                shell, s,
+                "Unix default shell must come from $SHELL when set"
+            ),
+            _ => assert_eq!(
+                shell, "/bin/sh",
+                "Unix default shell must fall back to /bin/sh"
+            ),
+        }
+
+        // `with_defaults` seeds the same value.
+        assert_eq!(Config::with_defaults().recording_shell, shell);
+    }
+
     #[test]
     fn config_path_ends_with_expected_segments() {
         // Don't assert the platform prefix, just the trailing structure.

src/config.rs

@@ -12,9 +12,13 @@
 //!     `anyhow::Result` is used ONLY at the command boundary ([`app::run`] and
 //!     `main`). See [`config`] for a worked thiserror example and [`app`] for the
 //!     anyhow boundary.
-//!   * No `unsafe`: the crate is `#![forbid(unsafe_code)]`. Platform syscalls go
-//!     through safe wrappers (e.g. `rustix` on Unix).
-#![forbid(unsafe_code)]
+//!   * No `unsafe` by default: the crate is `#![deny(unsafe_code)]`. Platform
+//!     syscalls go through safe wrappers (e.g. `rustix` on Unix). The one
+//!     exception is the Windows console FFI in [`recorder::windows`] — VT
+//!     console-mode setup and cancelling the blocking stdin read — which has no
+//!     safe-wrapper equivalent and opts in via a scoped, documented
+//!     `#[allow(unsafe_code)]` on that module alone.
+#![deny(unsafe_code)]
 
 pub mod app;
 pub mod asciicast;

src/lib.rs

@@ -666,17 +666,16 @@ fn start_recording(
     Ok(())
 }
 
-/// Picks the shell to record: the configured shell, falling back to `$SHELL`,
-/// then `/bin/sh`. The configured value is normally already seeded from `$SHELL`
-/// by [`Config::with_defaults`]; this keeps a sensible last resort.
+/// Picks the shell to record: the configured shell, falling back to the
+/// platform default ([`crate::config::default_shell`] — `$SHELL` on Unix,
+/// PowerShell on Windows). The configured value is normally already seeded with
+/// that same default by [`Config::with_defaults`]; this keeps a sensible last
+/// resort when the config carries an empty shell.
 fn recording_shell(config: &Config) -> String {
     if !config.recording_shell.trim().is_empty() {
         return config.recording_shell.clone();
     }
-    match std::env::var("SHELL") {
-        Ok(shell) if !shell.trim().is_empty() => shell,
-        _ => "/bin/sh".to_string(),
-    }
+    crate::config::default_shell()
 }
 
 #[cfg(test)]

src/menu.rs

@@ -131,6 +131,10 @@ impl PtySession {
             .map_err(|e| RecorderError::OpenPty(e.to_string()))?;
 
         let mut cmd = CommandBuilder::new(shell);
+        // Forward the current environment verbatim. `std::env::vars()` only yields
+        // variables that are actually set, so `TERM`/`SHELL` are passed through
+        // when present (e.g. a Git Bash / MSYS / WSL-interop launch) and simply
+        // absent otherwise — native Windows consoles define neither.
         for (key, value) in std::env::vars() {
             cmd.env(key, value);
         }
@@ -454,6 +458,11 @@ pub fn record_session<W: Write + Send + 'static>(
     // Raw mode is entered only after the PTY is up so an early failure leaves the
     // terminal untouched. The guard restores cooked mode on any exit path.
     let _raw = RawModeGuard::enable()?;
+    // Platform terminal-mode setup, restored on drop: on Windows this enables the
+    // console's virtual-terminal input/output so host key presses reach the
+    // ConPTY child as VT sequences and its VT output renders; on Unix it is a
+    // no-op (the PTY already does this). Dropped after teardown, before the menu.
+    let _term_mode = platform::TerminalModeGuard::install()?;
     let mut resize_watcher = platform::ResizeWatcher::install()?;
     let mut stdin_poller = platform::StdinPoller::new()?;
 

src/recorder/mod.rs

@@ -18,9 +18,21 @@ use rustix::io::Errno;
 
 use super::StdinRead;
 
-/// Returns the user's preferred shell (`$SHELL`), falling back to `/bin/sh`.
-pub fn default_shell() -> String {
-    std::env::var("SHELL").unwrap_or_else(|_| "/bin/sh".to_string())
+/// No-op terminal-mode guard on Unix.
+///
+/// Unix PTYs already deliver raw VT input and render VT output without any extra
+/// console-mode setup, so the cooked-mode clearing done by the shared
+/// [`RawModeGuard`](super::RawModeGuard) is sufficient. This type exists only so
+/// the shared recorder can install platform terminal-mode setup uniformly; its
+/// Windows counterpart configures the console's virtual-terminal modes.
+#[must_use = "kept symmetric with the Windows guard; bind it to a variable"]
+pub struct TerminalModeGuard;
+
+impl TerminalModeGuard {
+    /// Installs nothing; succeeds unconditionally.
+    pub fn install() -> io::Result<Self> {
+        Ok(Self)
+    }
 }
 
 /// Detects terminal resizes via the `SIGWINCH` signal.