fix(cli): wait for the task tree's graceful shutdown on ctrl-c

crates/vite_cli_snapshots/src/bin/vpt/report_orphan_on_ctrlc.rs

@@ -1,19 +1,25 @@
-/// report-orphan-on-ctrlc `<verdict-file>`
+/// report-orphan-on-ctrlc `<verdict-file>` \[--ignore-interrupt\]
 ///
 /// Simulates a task with a graceful shutdown phase (a vite dev server
 /// closing on Ctrl+C) and records whether `vp run` let that shutdown finish
 /// (issue #2036): emits a "ready" milestone, waits for Ctrl+C, then spends
 /// 300 ms shutting down before exiting cleanly.
 ///
+/// With `--ignore-interrupt` the task instead announces the interrupt (a
+/// printed line plus an "interrupted" milestone, so a test can synchronize
+/// a second Ctrl+C on it) and keeps running forever, like a task whose
+/// shutdown hangs. Its graceful shutdown then never completes; ending it
+/// takes vp's force-kill escalation.
+///
 /// The verdict is written by a watcher process spawned into its own process
 /// group, insulated from whatever signals vp sends the task tree during
 /// teardown (the same way real dev-server grandchildren outlive `vp run`).
 /// The watcher holds the read end of a pipe; the task writes "done" after
 /// its shutdown completes. Pipe EOF without "done" means the task was torn
 /// down mid-shutdown, and the watcher records that in `<verdict-file>`
 /// (atomically, via a temp-file rename) for a follow-up `vpt wait-file`
-/// step. Nothing is printed after the milestone, so the terminal snapshot
-/// stays deterministic no matter how quickly the task tree is killed.
+/// step. Nothing else is printed, so the terminal snapshot stays
+/// deterministic no matter how quickly the task tree is killed.
 #[cfg(unix)]
 pub fn run(args: &[String]) -> Result<(), Box<dyn std::error::Error>> {
     if args.first().is_some_and(|arg| arg == "--watch") {
@@ -22,8 +28,17 @@ pub fn run(args: &[String]) -> Result<(), Box<dyn std::error::Error>> {
 
     use std::{io::Write as _, os::fd::OwnedFd, sync::Arc, time::Duration};
 
-    let Some(verdict_path) = args.first() else {
-        return Err("Usage: vpt report-orphan-on-ctrlc <verdict-file>".into());
+    let mut verdict_path = None;
+    let mut ignore_interrupt = false;
+    for arg in args {
+        if arg == "--ignore-interrupt" {
+            ignore_interrupt = true;
+        } else {
+            verdict_path = Some(arg);
+        }
+    }
+    let Some(verdict_path) = verdict_path else {
+        return Err("Usage: vpt report-orphan-on-ctrlc <verdict-file> [--ignore-interrupt]".into());
     };
 
     let (pipe_read, pipe_write): (OwnedFd, OwnedFd) = nix::unistd::pipe()?;
@@ -61,6 +76,17 @@ pub fn run(args: &[String]) -> Result<(), Box<dyn std::error::Error>> {
     pty_terminal_test_client::mark_milestone("ready");
 
     ctrlc_once_lock.wait();
+    if ignore_interrupt {
+        // A task whose shutdown hangs: announce the interrupt so the test
+        // can synchronize the second Ctrl+C on it, then never finish. The
+        // "done" marker below is unreachable, so the watcher records the
+        // eventual force-kill as an unfinished shutdown.
+        println!("ignoring interrupt; still running");
+        pty_terminal_test_client::mark_milestone("interrupted");
+        loop {
+            std::thread::park();
+        }
+    }
     // The graceful shutdown a real dev server performs after SIGINT. A
     // buggy vp tears the task down milliseconds into this window, so the
     // "done" marker below never gets written.

crates/vite_cli_snapshots/tests/cli_snapshots/README.md

@@ -124,7 +124,8 @@ is identical on every platform:
 self-tests), `vpt check-tty`, `vpt read-stdin`, `vpt exit <code>`,
 `vpt exit-on-ctrlc`, `vpt report-orphan-on-ctrlc` (records via a watcher
 process that survives `vp run` whether the task got to finish its graceful
-shutdown), `vpt barrier`. `vpt wait-file <path> [timeout-ms]` polls until a
+shutdown; `--ignore-interrupt` simulates a task whose shutdown hangs),
+`vpt barrier`. `vpt wait-file <path> [timeout-ms]` polls until a
 file exists and prints it, for asserting on files written by a previous
 step's surviving process tree.
 

crates/vite_cli_snapshots/tests/cli_snapshots/fixtures/run_ctrlc_teardown/package.json

@@ -2,6 +2,7 @@
   "name": "run-ctrlc-teardown",
   "private": true,
   "scripts": {
-    "dev": "vpt report-orphan-on-ctrlc verdict.txt"
+    "dev": "vpt report-orphan-on-ctrlc verdict.txt",
+    "stuck-dev": "vpt report-orphan-on-ctrlc verdict.txt --ignore-interrupt"
   }
 }

crates/vite_cli_snapshots/tests/cli_snapshots/fixtures/run_ctrlc_teardown/snapshots.toml

@@ -1,35 +1,54 @@
-# Reproduces https://github.com/voidzero-dev/vite-plus/issues/2036: killing a
-# `vp run` task with Ctrl+C leaves stray escape sequences on the next fish
-# prompt. fish probes the terminal (OSC 11, CSI 6n, DA1) every time it draws
-# a prompt; after `vp run` + Ctrl+C, the task's process tree (vite dev server,
-# managed node) is still shutting down when the shell takes the terminal back,
-# and its late terminal writes and mode restores race the shell's probes, so
-# the terminal's replies land as garbage input on the prompt
-# (`^[]11;rgb:...^[\^[[9;1R^[[?62;22;52c`). Plain `vite dev` is clean because
-# the shell waits for vite itself, which finishes its terminal cleanup before
-# exiting.
+# Guards the fix for https://github.com/voidzero-dev/vite-plus/issues/2036:
+# killing a `vp run` task with Ctrl+C used to leave stray escape sequences on
+# the next fish prompt. fish probes the terminal (OSC 11, CSI 6n, DA1) every
+# time it draws a prompt; vp used to die of SIGINT within milliseconds (it is
+# the PTY session leader, so the whole session got SIGHUP'd) while the task's
+# process tree (vite dev server, managed node) was still shutting down, and
+# the dying tree's late terminal writes and mode restores raced the shell's
+# probes, turning their replies into garbage input
+# (`^[]11;rgb:...^[\^[[9;1R^[[?62;22;52c`).
 #
-# The vp-side defect this case pins: on Ctrl+C, `vp run` tears the terminal
-# and the task down within milliseconds instead of letting the task shut
-# down gracefully. The payload simulates a dev-server-like task whose
-# shutdown takes 300 ms; a watcher process outside vp's reach records to
-# verdict.txt whether that shutdown was allowed to finish.
+# `vp run` must instead swallow the interrupt and wait for the task tree's
+# graceful shutdown before handing the terminal back. The payload simulates a
+# dev-server-like task whose shutdown takes 300 ms; a watcher process in its
+# own process group (it survives any teardown signals, like real dev-server
+# grandchildren do) records to verdict.txt whether that shutdown finished.
 
 [[case]]
-name = "run_ctrlc_exits_before_task_shutdown"
+name = "run_ctrlc_waits_for_task_shutdown"
 vp = "global"
 skip-platforms = ["windows"]
 comment = """
-Issue #2036: on Ctrl+C, `vp run` tears the task down immediately instead of
-waiting for its graceful shutdown, and whatever the dying task tree still
-does to the terminal races the next shell prompt. verdict.txt records the
-current buggy behavior; a fix should flip it to
-"task completed its graceful shutdown".
+Issue #2036: Ctrl+C on `vp run dev` must let the task finish its graceful
+shutdown before vp exits and the shell takes the terminal back. The verdict
+below flips to "task was torn down before its graceful shutdown finished"
+when vp abandons the task mid-shutdown again.
 """
 steps = [
   { argv = ["vp", "run", "dev"], continue-on-failure = true, interactions = [
     { "expect-milestone" = "ready" },
     { "write-key" = "ctrl-c" },
   ] },
-  { argv = ["vpt", "wait-file", "verdict.txt"], comment = "The verdict the task wrote after `vp run` already exited." },
+  { argv = ["vpt", "wait-file", "verdict.txt"], comment = "The verdict recorded by the task's watcher process." },
+]
+
+[[case]]
+name = "run_double_ctrlc_force_kills_stuck_task"
+vp = "global"
+skip-platforms = ["windows"]
+comment = """
+The escape hatch for the waiting behavior above: a task that ignores the
+interrupt must not make vp unstoppable. The second Ctrl+C force-kills the
+task, so vp exits with the kill signal's code and the watcher records an
+unfinished shutdown. The "interrupted" milestone between the two Ctrl+C
+presses keeps them from coalescing into a single signal.
+"""
+steps = [
+  { argv = ["vp", "run", "stuck-dev"], continue-on-failure = true, interactions = [
+    { "expect-milestone" = "ready" },
+    { "write-key" = "ctrl-c" },
+    { "expect-milestone" = "interrupted" },
+    { "write-key" = "ctrl-c" },
+  ] },
+  { argv = ["vpt", "wait-file", "verdict.txt"], comment = "The verdict recorded by the task's watcher process." },
 ]

crates/vite_cli_snapshots/tests/cli_snapshots/fixtures/run_ctrlc_teardown/snapshots/run_ctrlc_waits_for_task_shutdown.md

@@ -0,0 +1,33 @@
+# run_ctrlc_waits_for_task_shutdown
+
+Issue #2036: Ctrl+C on `vp run dev` must let the task finish its graceful
+shutdown before vp exits and the shell takes the terminal back. The verdict
+below flips to "task was torn down before its graceful shutdown finished"
+when vp abandons the task mid-shutdown again.
+
+## `vp run dev`
+
+**→ expect-milestone:** `ready`
+
+```
+VITE+ - The Unified Toolchain for the Web
+
+$ vpt report-orphan-on-ctrlc verdict.txt ⊘ cache disabled
+```
+
+**← write-key:** `ctrl-c`
+
+```
+VITE+ - The Unified Toolchain for the Web
+
+$ vpt report-orphan-on-ctrlc verdict.txt ⊘ cache disabled
+
+```
+
+## `vpt wait-file verdict.txt`
+
+The verdict recorded by the task's watcher process.
+
+```
+task completed its graceful shutdown
+```

crates/vite_cli_snapshots/tests/cli_snapshots/fixtures/run_ctrlc_teardown/snapshots/run_double_ctrlc_force_kills_stuck_task.md

@@ -0,0 +1,48 @@
+# run_double_ctrlc_force_kills_stuck_task
+
+The escape hatch for the waiting behavior above: a task that ignores the
+interrupt must not make vp unstoppable. The second Ctrl+C force-kills the
+task, so vp exits with the kill signal's code and the watcher records an
+unfinished shutdown. The "interrupted" milestone between the two Ctrl+C
+presses keeps them from coalescing into a single signal.
+
+## `vp run stuck-dev`
+
+**Exit code:** 137
+
+**→ expect-milestone:** `ready`
+
+```
+VITE+ - The Unified Toolchain for the Web
+
+$ vpt report-orphan-on-ctrlc verdict.txt --ignore-interrupt ⊘ cache disabled
+```
+
+**← write-key:** `ctrl-c`
+
+**→ expect-milestone:** `interrupted`
+
+```
+VITE+ - The Unified Toolchain for the Web
+
+$ vpt report-orphan-on-ctrlc verdict.txt --ignore-interrupt ⊘ cache disabled
+ignoring interrupt; still running
+```
+
+**← write-key:** `ctrl-c`
+
+```
+VITE+ - The Unified Toolchain for the Web
+
+$ vpt report-orphan-on-ctrlc verdict.txt --ignore-interrupt ⊘ cache disabled
+ignoring interrupt; still running
+
+```
+
+## `vpt wait-file verdict.txt`
+
+The verdict recorded by the task's watcher process.
+
+```
+task was torn down before its graceful shutdown finished
+```

crates/vite_command/Cargo.toml

@@ -9,7 +9,7 @@ rust-version.workspace = true
 
 [dependencies]
 fspy = { workspace = true }
-tokio = { workspace = true }
+tokio = { workspace = true, features = ["macros", "process", "signal"] }
 tokio-util = { workspace = true }
 tracing = { workspace = true }
 vite_error = { workspace = true }

crates/vite_command/src/lib.rs

@@ -86,31 +86,71 @@ pub fn build_command(bin_path: &AbsolutePath, cwd: &AbsolutePath) -> Command {
     cmd
 }
 
-/// Execute a command while preserving terminal state.
+/// Execute a command while preserving terminal state and waiting through
+/// Ctrl+C.
 ///
-/// This prevents escape sequences from appearing in the prompt when the child process
-/// is interrupted (e.g., via Ctrl+C) while the terminal is in a non-standard state.
+/// The terminal delivers an interrupt to the whole foreground process
+/// group; the child's tree owns the graceful shutdown, and this process
+/// must outlive it, or the terminal is handed back to the shell while the
+/// dying tree still writes to it and restores terminal modes, which is how
+/// the stray escape sequences of issue #2036 ended up on the prompt.
+/// Interrupts are therefore swallowed while the child runs, and the
+/// child's own exit decides when this returns. Swallowing uses a signal
+/// handler rather than `SIG_IGN`: an ignored disposition survives exec
+/// into the entire child tree, which would make tasks without their own
+/// SIGINT handler uninterruptible. A second interrupt force-kills the
+/// child so a task that ignores the first one cannot make the CLI
+/// unstoppable.
 ///
-/// On Unix, saves the terminal state before spawning the child process and restores
-/// it after the child exits. On Windows, this is a simple pass-through.
+/// On Unix, the terminal state saved before spawning is restored after
+/// the child exits, so a child killed mid-raw-mode cannot leave the
+/// terminal broken for the next prompt.
 pub async fn execute_with_terminal_guard(mut cmd: Command) -> Result<ExitStatus, Error> {
     #[cfg(unix)]
-    {
+    let _guard = {
         use nix::libc::STDIN_FILENO;
+        TerminalStateGuard::save(STDIN_FILENO)
+    };
 
-        // Save terminal state before spawning child
-        let _guard = TerminalStateGuard::save(STDIN_FILENO);
+    let mut child = cmd.spawn().map_err(|e| Error::Anyhow(e.into()))?;
 
-        // Spawn and wait for child - guard will restore terminal state on drop
-        let mut child = cmd.spawn().map_err(|e| Error::Anyhow(e.into()))?;
-        child.wait().await.map_err(|e| Error::Anyhow(e.into()))
+    #[cfg(unix)]
+    {
+        let mut sigint = tokio::signal::unix::signal(tokio::signal::unix::SignalKind::interrupt())
+            .map_err(|e| Error::Anyhow(e.into()))?;
+        let mut interrupts = 0u32;
+        loop {
+            tokio::select! {
+                status = child.wait() => return status.map_err(|e| Error::Anyhow(e.into())),
+                _ = sigint.recv() => {
+                    interrupts += 1;
+                    if interrupts >= 2 {
+                        let _ = child.start_kill();
+                    }
+                }
+            }
+        }
     }
 
-    #[cfg(not(unix))]
+    #[cfg(windows)]
     {
-        let mut child = cmd.spawn().map_err(|e| Error::Anyhow(e.into()))?;
-        child.wait().await.map_err(|e| Error::Anyhow(e.into()))
+        let mut ctrl_c = tokio::signal::windows::ctrl_c().map_err(|e| Error::Anyhow(e.into()))?;
+        let mut interrupts = 0u32;
+        loop {
+            tokio::select! {
+                status = child.wait() => return status.map_err(|e| Error::Anyhow(e.into())),
+                _ = ctrl_c.recv() => {
+                    interrupts += 1;
+                    if interrupts >= 2 {
+                        let _ = child.start_kill();
+                    }
+                }
+            }
+        }
     }
+
+    #[cfg(not(any(unix, windows)))]
+    child.wait().await.map_err(|e| Error::Anyhow(e.into()))
 }
 
 /// Build a `tokio::process::Command` for shell execution.

crates/vite_global_cli/src/js_executor.rs

@@ -295,8 +295,7 @@ impl JsExecutor {
         let mut cmd = Self::create_js_command(&node_binary, &bin_prefix);
         cmd.arg(entry_point.as_path()).args(args).current_dir(project_path.as_path());
 
-        let status = cmd.status().await?;
-        Ok(status)
+        Ok(vite_command::execute_with_terminal_guard(cmd).await?)
     }
 
     /// Delegate to local or global vite-plus CLI using the CLI's own runtime.
@@ -352,9 +351,8 @@ impl JsExecutor {
         bin_prefix: &AbsolutePath,
         args: &[String],
     ) -> Result<ExitStatus, Error> {
-        let mut cmd = self.prepare_js_entry(project_path, node_binary, bin_prefix, args)?;
-        let status = cmd.status().await?;
-        Ok(status)
+        let cmd = self.prepare_js_entry(project_path, node_binary, bin_prefix, args)?;
+        Ok(vite_command::execute_with_terminal_guard(cmd).await?)
     }
 
     /// Like [`run_js_entry`], but returns `Output`.