fix(cli): wait for the task's graceful shutdown on ctrl-c#2065
Closed
forehalo wants to merge 9 commits into
Closed
fix(cli): wait for the task's graceful shutdown on ctrl-c#2065forehalo wants to merge 9 commits into
forehalo wants to merge 9 commits into
Conversation
✅ Deploy Preview for viteplus-preview canceled.
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses stray/odd OSC terminal sequences after interrupting vp run with Ctrl+C by ensuring the parent process preserves/restores terminal state and temporarily ignores SIGINT while awaiting the child process. It applies this guard to the global CLI’s JS delegation path so Ctrl+C primarily affects the spawned JS CLI process instead of terminating the Rust parent mid-cleanup.
Changes:
- Route JS CLI delegation through
vite_command::execute_with_terminal_guardinstead ofCommand::status(). - Extend the Unix terminal-state guard to also ignore and then restore the process
SIGINTdisposition while the child runs. - Enable the
nixsignalfeature for non-Windows builds.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/vite_global_cli/src/js_executor.rs | Uses execute_with_terminal_guard when delegating to JS entrypoints so terminal cleanup runs even after Ctrl+C. |
| crates/vite_command/src/lib.rs | Updates terminal guard behavior to ignore/restore SIGINT on Unix while awaiting the child process. |
| crates/vite_command/Cargo.toml | Adds nix signal feature to support sigaction usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Member
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60cb19524f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
fengmk2
added
the
preview-build
Member
|
@forehalo I will make some improvements based on your fixes and add snapshot tests to ensure continuous verification. |
fengmk2
added a commit
that referenced
this pull request
Jul 6, 2026
…rl-c Builds on the terminal-guard approach from #2065 with three adjustments: Swallowing moves out of the guard's sigaction and into the wait loop as a signal handler. SIG_IGN survives exec into the entire child tree, so any task without its own SIGINT handler (a plain script, a shell) would become uninterruptible; handler dispositions reset to default on exec and leave children untouched. It also applies when stdin is not a terminal, where the guard's SIG_IGN never installed at all. A second Ctrl+C force-kills the child, so a task that ignores the first interrupt cannot make vp unstoppable. Without it, killing a stuck task requires an external signal. Children killed by a signal now exit with the conventional 128 + signal code (130 for SIGINT) instead of a generic failure. Refs #2036
Killing a vp run task with Ctrl+C in fish leaves stray escape sequences on the next prompt: fish probes the terminal (OSC 11, CSI 6n, DA1) at every prompt draw, and vp run tears the task down within milliseconds of Ctrl+C instead of waiting for its graceful shutdown, so the dying task tree's late terminal writes and mode restores race the shell's probes and the replies land as garbage input. The new run_ctrlc_teardown fixture pins the vp-side defect: a dev-server-like vpt payload whose shutdown takes 300 ms records, via a watcher process in its own process group that survives vp's teardown, whether that shutdown was allowed to finish. verdict.txt currently reads "task was torn down before its graceful shutdown finished"; a fix should flip it to "task completed its graceful shutdown". Also adds vpt wait-file, a generic helper that polls until a file exists and prints it, for asserting on files written by a step's surviving process tree. Refs voidzero-dev#2036
…rl-c Builds on the terminal-guard approach from voidzero-dev#2065 with three adjustments: Swallowing moves out of the guard's sigaction and into the wait loop as a signal handler. SIG_IGN survives exec into the entire child tree, so any task without its own SIGINT handler (a plain script, a shell) would become uninterruptible; handler dispositions reset to default on exec and leave children untouched. It also applies when stdin is not a terminal, where the guard's SIG_IGN never installed at all. A second Ctrl+C force-kills the child, so a task that ignores the first interrupt cannot make vp unstoppable. Without it, killing a stuck task requires an external signal. Children killed by a signal now exit with the conventional 128 + signal code (130 for SIGINT) instead of a generic failure. Refs voidzero-dev#2036
…scalation The run_ctrlc_teardown snapshot flips from "task was torn down before its graceful shutdown finished" to "task completed its graceful shutdown" and now guards the waiting behavior. A second case covers the escalation: a task that ignores the interrupt (vpt report-orphan-on-ctrlc --ignore-interrupt) keeps vp waiting after the first Ctrl+C and is force-killed by the second, with vp exiting 137 and the surviving watcher recording the unfinished shutdown. The "interrupted" milestone between the two Ctrl+C presses keeps them from coalescing into a single signal.
fengmk2
changed the title
Member
|
@codex review |
fengmk2
added
preview-build
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9425c4fda
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Review follow-ups on the Ctrl+C guard: Forward a targeted interrupt to the child when stdin is not a terminal: a supervisor, timeout, or CI cancellation signals only this process, and swallowing it left the task running with vp apparently hung. With a terminal the foreground group already delivered it, so forwarding would double it (a second SIGINT makes vite exit immediately instead of gracefully). Make the second Ctrl+C kill the whole process group when this process is its own group leader (a shell job leader), dying with it, instead of SIGKILLing only the intermediate node process and leaving the stuck task tree alive; the terminal state is restored first. When sharing the parent's group (a supervisor), only the direct child is killed, so the supervisor is never taken down. The escalation also prints a warning so the force quit is visible. Re-raise SIGINT after a swallowed interrupt once the child has exited, so calling shells see a died-of-SIGINT status and abort scripts and loops (wait-and-cooperative-exit); previously vp exited with the child's plain status and a Ctrl+C'd `vp run` inside a script let the script continue. Restore the pre-guard SIGINT disposition after the child exits: tokio's handler registration is process-global and permanent, which left the process (including the node local CLI) immune to Ctrl+C for its remaining lifetime. Register the signal stream before spawning so a registration failure cannot leak an unwaited child. Reuse shim::exec::exit_code_from_status (128 + signal) for vp run and vpr exit codes instead of a duplicated mapping in main.rs and a plain code().unwrap_or(1) in vpr; give the fixture's wait-file steps headroom over the watcher's 10 s safety cap; share the ctrl-c bootstrap (including the Windows CONSOLE_IGNORE_CTRL_C workaround) between the vpt payloads; refresh the stale vpt header comment. Refs voidzero-dev#2036
The local JS/NAPI path collapsed signal-death statuses to exit code 1 with status.code().unwrap_or(1) before returning across the NAPI boundary, so interrupting a local vp dev/preview reported 1 instead of 130 (and force kills 1 instead of 137). vite_command now exposes exit_code_from_status (128 + signal on unix) as the single implementation; the binding's execution paths and the global CLI's shim helper both delegate to it. Refs voidzero-dev#2036
The field is regenerated by the packages build from the local vendored vite clone; a stale clone in the build environment rewrote it to 8.1.2 and it was committed accidentally. The branch pin is 8.1.3.
Member
|
@forehalo Sorry, My changes are too extensive, I will close this, please help to resubmit. |
forehalo
pushed a commit
to forehalo/vite-plus
that referenced
this pull request
Jul 7, 2026
…rl-c Builds on the terminal-guard approach from voidzero-dev#2065 with three adjustments: Swallowing moves out of the guard's sigaction and into the wait loop as a signal handler. SIG_IGN survives exec into the entire child tree, so any task without its own SIGINT handler (a plain script, a shell) would become uninterruptible; handler dispositions reset to default on exec and leave children untouched. It also applies when stdin is not a terminal, where the guard's SIG_IGN never installed at all. A second Ctrl+C force-kills the child, so a task that ignores the first interrupt cannot make vp unstoppable. Without it, killing a stuck task requires an external signal. Children killed by a signal now exit with the conventional 128 + signal code (130 for SIGINT) instead of a generic failure. Refs voidzero-dev#2036 (cherry picked from commit bb5239e)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2036
On Ctrl+C,
vp(the PTY session leader) exited within milliseconds while the task tree (vite dev server, managed node) was still shutting down. The dying tree's late terminal writes and mode restores raced the next shell prompt, so fish's prompt probes (OSC 11, CSI 6n, DA1) were answered into a terminal nobody owned and the replies showed up as^[]11;rgb:...^[[9;1R^[[?62;22;52con the prompt.vpnow runs delegated commands behind a terminal guard: it saves and restores the terminal state, swallows SIGINT while the child runs (with a signal handler rather thanSIG_IGN, so children without their own SIGINT handling stay interruptible), and lets the child's own exit decide whenvpreturns. A second Ctrl+C force-kills the child so a stuck task cannot makevpunstoppable, and children killed by a signal exit with the conventional128 + signalcode.PTY snapshot tests pin both behaviors: waiting for the task's graceful shutdown, and the double Ctrl+C escalation. Verified with real fish 4.8 driven in a scripted PTY: the stray sequences no longer appear after killing
vp run dev.