Merge pull request #18 from code4mk/windows-support

Split foreground_pid by platform: ConPTY has no process groups

Author: mostafa6765

docs/WINDOWS-SUPPORT.md

@@ -141,10 +141,13 @@ by accident, and it breaks the moment an NSIS `.exe` is published alongside it.
 
 - **"A command is running" check.** `PtyInstance::foreground_pid()` compares the
   tty's foreground process group against the shell's pid. ConPTY has no process
-  groups, so this returns `None` — which the code already treats as idle. The
-  practical effect is that the confirmation before restarting to install an
-  update won't fire on Windows. Safe direction (a missed prompt, not a wrong
-  action), but it should be documented rather than discovered.
+  groups at all, and `portable-pty` reflects that by declaring
+  `process_group_leader` only under `#[cfg(unix)]` — so this is a *compile*
+  error on Windows, not a method that returns `None`. `foreground_pid` is now
+  two implementations, and the Windows one returns `None`, which callers already
+  read as idle. The practical effect is that the confirmation before restarting
+  to install an update won't fire on Windows: a missed prompt rather than a
+  wrong action.
 - **Spec storage.** `~/.figyterm` becomes `%USERPROFILE%\.figyterm`, which works
   but isn't idiomatic — `%APPDATA%\figyterm` is. Same call as the XDG question on
   Linux, and the same answer: low priority.

docs/WINDOWS-TASKS.md

@@ -9,8 +9,17 @@ Windows · `[ ]` todo
 Everything below marked `[x]` was written and type-checked, but **nothing has run
 on Windows**. The Rust Windows branches were compiled for
 `x86_64-pc-windows-msvc` in isolation (the full crate can't cross-check here —
-`ring` needs a Windows C toolchain), and the path logic is covered by unit tests
-that deliberately run on any host. §8 is the part that still needs a real box.
+`ring`, via the updater plugin, needs a Windows C toolchain), and the path logic
+is covered by unit tests that deliberately run on any host. §8 is the part that
+still needs a real box.
+
+That isolation has a known blind spot, and it has already cost one round trip:
+it checks the code *this port wrote*, not pre-existing code that the Windows
+target newly compiles. `foreground_pid()` calling a `#[cfg(unix)]` trait method
+was found by a real Windows build, not by anything here. A crate-wide audit for
+other unix-only APIs (`std::os::unix`, `libc`, `nix`, `PermissionsExt`, …) came
+back empty, so that was the only one — but the first real build is still the
+authority.
 
 ---
 
@@ -83,8 +92,9 @@ that deliberately run on any host. §8 is the part that still needs a real box.
       table (it shares Linux's `Ctrl+Shift` scheme).
 - [x] **`RELEASING.md`** — the Windows job and which artifact the updater uses.
 - [x] **Document the degradations** — the "a command is running" guard can't fire
-      on Windows (ConPTY has no process groups, so `foreground_pid()` always
-      returns `None`, which reads as idle).
+      on Windows: ConPTY has no process groups, and `portable-pty` declares
+      `process_group_leader` under `#[cfg(unix)]`, so `foreground_pid()` is
+      `#[cfg]`-split and returns `None` there. Callers read that as idle.
 
 ## 8. Verification
 

src-tauri/src/terminal/pty.rs

@@ -151,7 +151,9 @@ pub struct PtyInstance {
     pub master: Arc<Mutex<Box<dyn MasterPty + Send>>>,
     pub writer: Arc<Mutex<Box<dyn Write + Send>>>,
     /// PID of the login shell itself. When the tty's foreground process group
-    /// leader differs from this, the user is running something.
+    /// leader differs from this, the user is running something. Unread on
+    /// Windows, where there are no process groups to compare it against.
+    #[cfg_attr(windows, allow(dead_code))]
     shell_pid: Option<u32>,
     shutdown: Arc<Mutex<bool>>,
 }
@@ -257,13 +259,28 @@ impl PtyInstance {
     /// `None` means idle (or undeterminable, which is treated as idle: a false
     /// "busy" would block updating forever, while a false "idle" only costs the
     /// user a confirmation they'd have clicked through anyway).
+    #[cfg(unix)]
     pub fn foreground_pid(&self) -> Option<u32> {
         let shell_pid = self.shell_pid?;
         let master = self.master.lock().ok()?;
         let leader = master.process_group_leader()? as u32;
         (leader != shell_pid).then_some(leader)
     }
 
+    /// Always `None` on Windows.
+    ///
+    /// The question doesn't exist there: ConPTY has no process groups, and
+    /// `portable-pty` reflects that by declaring `process_group_leader` only
+    /// under `#[cfg(unix)]` — so this isn't a stub for something unimplemented,
+    /// it's the whole answer the platform can give.
+    ///
+    /// Callers read `None` as idle, which errs the safe way: the update flow
+    /// simply won't name a running command before restarting.
+    #[cfg(windows)]
+    pub fn foreground_pid(&self) -> Option<u32> {
+        None
+    }
+
     pub fn write(&self, data: &[u8]) -> Result<(), String> {
         let mut writer = self.writer.lock().map_err(|e| e.to_string())?;
         writer.write_all(data).map_err(|e| format!("Write failed: {}", e))?;