From f8ccfcf05a8974f3104345cfa570c1ebb37894c8 Mon Sep 17 00:00:00 2001 From: "MK (fengmk2)" Date: Wed, 21 Oct 2026 20:29:02 +0800 Subject: [PATCH 1/6] fix(vpx): read a bare `--experimental-config-file` as taking no value Node.js 24.19, 26.1, and later read a bare `--experimental-config-file` as `node.config.json`. Script detection took the next argument as its value, so it misplaced the end of the Node.js options: `--watch` after the script started the watcher, and `--watch`, a watcher option, or `--tsconfig` after the flag was taken as the config file. Both CLIs now read the flag without a value, as those releases do; the `=` form names a config file on every release. Co-Authored-By: Claude Opus 5.5 --- .../command_vpx_script/config-file/.env | 1 + .../command_vpx_script/config-file/a/name.ts | 1 + .../command_vpx_script/config-file/alias.ts | 2 + .../command_vpx_script/config-file/b/name.ts | 1 + .../command_vpx_script/config-file/cli.json | 1 + .../command_vpx_script/config-file/env.json | 1 + .../command_vpx_script/config-file/main.ts | 1 + .../config-file/node.config.json | 1 + .../command_vpx_script/snapshots.toml | 14 +++++ .../vpx_script_bare_config_flag.global.md | 31 +++++++++++ .../vpx_script_bare_config_flag.local.md | 31 +++++++++++ .../vp_global_cli/src/commands/vpx_script.rs | 5 +- .../src/commands/vpx_script_cases.json | 52 ++++++++++++++++++- packages/cli/src/vpx-script.ts | 7 ++- rfcs/vpx-script-execution.md | 5 +- 15 files changed, 149 insertions(+), 5 deletions(-) create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/.env create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/a/name.ts create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/alias.ts create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/b/name.ts create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/cli.json create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/env.json create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/main.ts create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/node.config.json create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_bare_config_flag.global.md create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_bare_config_flag.local.md diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/.env b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/.env new file mode 100644 index 0000000000..6d1bd43da5 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/.env @@ -0,0 +1 @@ +TS_NODE_PROJECT=env.json diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/a/name.ts b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/a/name.ts new file mode 100644 index 0000000000..78fb57fdc3 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/a/name.ts @@ -0,0 +1 @@ +export const name: string = 'cli.json'; diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/alias.ts b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/alias.ts new file mode 100644 index 0000000000..b6ec521f35 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/alias.ts @@ -0,0 +1,2 @@ +import { name } from '@lib/name'; +console.log('tsconfig', name); diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/b/name.ts b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/b/name.ts new file mode 100644 index 0000000000..33ff6ca205 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/b/name.ts @@ -0,0 +1 @@ +export const name: string = 'env.json'; diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/cli.json b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/cli.json new file mode 100644 index 0000000000..cca23127c0 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/cli.json @@ -0,0 +1 @@ +{ "compilerOptions": { "paths": { "@lib/*": ["./a/*"] } } } diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/env.json b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/env.json new file mode 100644 index 0000000000..d1e13e897f --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/env.json @@ -0,0 +1 @@ +{ "compilerOptions": { "paths": { "@lib/*": ["./b/*"] } } } diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/main.ts b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/main.ts new file mode 100644 index 0000000000..8fe06ee94a --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/main.ts @@ -0,0 +1 @@ +console.log('args', JSON.stringify(process.argv.slice(2)), 'config', process.title === 'vpx-config'); diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/node.config.json b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/node.config.json new file mode 100644 index 0000000000..f59b79769f --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/node.config.json @@ -0,0 +1 @@ +{"nodeOptions":{"title":"vpx-config"}} diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots.toml index 4c786fd9dd..5ec5a1a77a 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots.toml @@ -225,3 +225,17 @@ cwd = "signals" env = { TMPDIR = "/tmp" } comment = "A SIGINT sent to the vpx --watch PID reaches the script once, and so does a SIGINT to its whole process group, as Ctrl+C sends it." steps = [["node", "watch.mjs"]] + +[[case]] +name = "vpx_script_bare_config_flag" +vp = ["local", "global"] +cwd = "config-file" +# The first release where the project-local bin sees the bare flag: before 26.12, the +# Node.js running the bin rewrites it to `--experimental-config-file=node.config.json`. +env = { VP_NODE_VERSION = "26.12.0" } +comment = "From Node.js 24.19 and 26.1, a bare --experimental-config-file reads node.config.json and takes no value, so the arguments after it keep their meaning." +steps = [ + { argv = ["vpx", "--experimental-config-file", "./main.ts", "--watch"], comment = "--watch after the script belongs to the script" }, + { argv = ["vpx", "--experimental-config-file", "--tsconfig", "cli.json", "--env-file=.env", "./alias.ts"], comment = "--tsconfig is still vpx's, and wins over the TS_NODE_PROJECT in .env" }, + { argv = ["vpx", "--experimental-config-file", "--watch", "--include", "extra.json"], comment = "--watch still selects the watcher, which takes --include and reports the missing entry", continue-on-failure = true }, +] diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_bare_config_flag.global.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_bare_config_flag.global.md new file mode 100644 index 0000000000..4451816391 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_bare_config_flag.global.md @@ -0,0 +1,31 @@ +# vpx_script_bare_config_flag + +From Node.js 24.19 and 26.1, a bare --experimental-config-file reads node.config.json and takes no value, so the arguments after it keep their meaning. + +## `vpx --experimental-config-file ./main.ts --watch` + +--watch after the script belongs to the script + +``` +args ["--watch"] config true +``` + +## `vpx --experimental-config-file --tsconfig cli.json --env-file=.env ./alias.ts` + +--tsconfig is still vpx's, and wins over the TS_NODE_PROJECT in .env + +``` +tsconfig cli.json +``` + +## `vpx --experimental-config-file --watch --include extra.json` + +--watch still selects the watcher, which takes --include and reports the missing entry + +**Exit code:** 1 + +``` +vpx --watch: missing +Usage: vpx --watch [options] [node options] [--] [script args...] +Run "vpx --watch --help" for the options. +``` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_bare_config_flag.local.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_bare_config_flag.local.md new file mode 100644 index 0000000000..4451816391 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_bare_config_flag.local.md @@ -0,0 +1,31 @@ +# vpx_script_bare_config_flag + +From Node.js 24.19 and 26.1, a bare --experimental-config-file reads node.config.json and takes no value, so the arguments after it keep their meaning. + +## `vpx --experimental-config-file ./main.ts --watch` + +--watch after the script belongs to the script + +``` +args ["--watch"] config true +``` + +## `vpx --experimental-config-file --tsconfig cli.json --env-file=.env ./alias.ts` + +--tsconfig is still vpx's, and wins over the TS_NODE_PROJECT in .env + +``` +tsconfig cli.json +``` + +## `vpx --experimental-config-file --watch --include extra.json` + +--watch still selects the watcher, which takes --include and reports the missing entry + +**Exit code:** 1 + +``` +vpx --watch: missing +Usage: vpx --watch [options] [node options] [--] [script args...] +Run "vpx --watch --help" for the options. +``` diff --git a/crates/vp_global_cli/src/commands/vpx_script.rs b/crates/vp_global_cli/src/commands/vpx_script.rs index 29b3c975e5..2cefc93ce3 100644 --- a/crates/vp_global_cli/src/commands/vpx_script.rs +++ b/crates/vp_global_cli/src/commands/vpx_script.rs @@ -36,6 +36,10 @@ const TS_EXTENSIONS: &[&str] = &[".ts", ".mts", ".cts", ".tsx"]; /// Node.js options that take a required value, which may come as the next argument: /// those `node --help` lists as `--option=...` on Node.js 22, 24, and 26. Best effort: /// a missing entry only moves the missing-file check for the script to Node.js. +/// +/// Not `--experimental-config-file`: Node.js 24.19, 26.1, and later read it bare as +/// `node.config.json`. Older releases take the next argument as the config file, which +/// `--experimental-config-file=` names on every release. const NODE_OPTIONS_WITH_VALUE: &[&str] = &[ "-C", "-e", @@ -58,7 +62,6 @@ const NODE_OPTIONS_WITH_VALUE: &[&str] = &[ "--env-file", "--env-file-if-exists", "--eval", - "--experimental-config-file", "--experimental-default-type", "--experimental-loader", "--experimental-package-map", diff --git a/crates/vp_global_cli/src/commands/vpx_script_cases.json b/crates/vp_global_cli/src/commands/vpx_script_cases.json index c8d50ebb80..1e2b482f40 100644 --- a/crates/vp_global_cli/src/commands/vpx_script_cases.json +++ b/crates/vp_global_cli/src/commands/vpx_script_cases.json @@ -23,7 +23,6 @@ "--env-file", "--env-file-if-exists", "--eval", - "--experimental-config-file", "--experimental-default-type", "--experimental-loader", "--experimental-package-map", @@ -327,6 +326,57 @@ "watch": true } }, + { + "name": "a bare --experimental-config-file takes no value, so a --watch after the script belongs to the script", + "files": { + "main.ts": "" + }, + "args": ["--experimental-config-file", "main.ts", "--watch"], + "expect": { + "script": ["--experimental-config-file", "main.ts", "--watch"] + } + }, + { + "name": "a bare --experimental-config-file does not take --watch or a watcher option", + "files": { + "main.ts": "" + }, + "args": ["--experimental-config-file", "--watch", "--include", "extra.json", "main.ts"], + "expect": { + "script": ["--experimental-config-file", "--include", "extra.json", "main.ts"], + "watch": true + } + }, + { + "name": "a bare --experimental-config-file does not take --tsconfig", + "files": { + "main.ts": "" + }, + "args": [ + "--watch", + "--experimental-config-file", + "--tsconfig", + "cli.json", + "--env-file=.env", + "main.ts" + ], + "expect": { + "script": ["--experimental-config-file", "--env-file=.env", "main.ts"], + "tsconfig": "cli.json", + "watch": true + } + }, + { + "name": "--experimental-config-file= names a config file on every Node.js", + "files": { + "main.ts": "" + }, + "args": ["--experimental-config-file=custom.json", "--watch", "main.ts", "--watch"], + "expect": { + "script": ["--experimental-config-file=custom.json", "main.ts", "--watch"], + "watch": true + } + }, { "name": "the value of --import is not the script", "files": { diff --git a/packages/cli/src/vpx-script.ts b/packages/cli/src/vpx-script.ts index 3550dde07f..d62f6c098d 100644 --- a/packages/cli/src/vpx-script.ts +++ b/packages/cli/src/vpx-script.ts @@ -17,7 +17,11 @@ import path from 'node:path'; export const SCRIPT_EXTENSIONS = ['.ts', '.mts', '.cts', '.tsx', '.js', '.mjs', '.cjs', '.jsx']; const TS_EXTENSIONS = new Set(['.ts', '.mts', '.cts', '.tsx']); -/** Node.js options that take a required value, which may come as the next argument (best effort). */ +/** + * Node.js options that take a required value, which may come as the next argument (best effort). + * Not `--experimental-config-file`, which Node.js 24.19, 26.1, and later read bare as + * `node.config.json`; see `NODE_OPTIONS_WITH_VALUE` in the global CLI. + */ export const NODE_OPTIONS_WITH_VALUE = new Set([ '-C', '-e', @@ -40,7 +44,6 @@ export const NODE_OPTIONS_WITH_VALUE = new Set([ '--env-file', '--env-file-if-exists', '--eval', - '--experimental-config-file', '--experimental-default-type', '--experimental-loader', '--experimental-package-map', diff --git a/rfcs/vpx-script-execution.md b/rfcs/vpx-script-execution.md index 6242e82e19..912babddec 100644 --- a/rfcs/vpx-script-execution.md +++ b/rfcs/vpx-script-execution.md @@ -69,6 +69,8 @@ if T starts with '-': # Node.js invocation; a leading optio skipping the value of options that take one (every `--option=...` that `node --help` lists on 22, 24, and 26: --import, --require, --env-file, -r, …, and the watcher's --include, --exclude, --ignore, --kill-signal, --kill-timeout) + a bare --experimental-config-file takes no value, as Node.js 24.19, 26.1, and later + read it (node.config.json), on every release after -e, --eval, -p, -pe, --print, --run, or --test, no token is a script: Node.js runs that code or package script, or takes test files and globs option names match with `_` for `-` (--redirect_warnings), as Node.js accepts @@ -232,6 +234,7 @@ Files: `packages/cli/src/script-preload.ts`, `script-register.ts`, `script-esm-h - On the off-thread releases (22.18 to 22.22.2, 24.11.0), a `--require` preload also runs in the `module.register()` hooks thread, so it runs twice. tsx has the same behavior. - On the off-thread releases, starting the hooks thread makes Node.js open stdout and stderr, which sets a pipe non-blocking, so `fs.writeSync(1, …)` to a slow reader can fail with `EAGAIN`. `process.stdout.write()` is not affected. tsx has the same behavior. - A `--require` or `--import` in `NODE_OPTIONS` also runs in the local bin's own Node.js before it execs the script, so it runs twice, as under the `tsx` CLI. The global `vpx` runs it once. +- A bare `--experimental-config-file` takes no value in detection, as Node.js 24.19, 26.1, and later read it. Older releases take the next argument as the config file, so there `--experimental-config-file custom.json --watch ./a.ts` leaves `--watch` to Node.js; `--experimental-config-file=custom.json` works on every release. - `vpx --watch` puts its Ctrl+C ack socket in `TMPDIR`. When that path would pass 100 bytes (the `sun_path` limit), the watcher runs without the ack, so a Ctrl+C while it is not the terminal's foreground job reaches a script that handles `SIGINT` twice, as it would without the loader's ack. - On in-thread releases without nodejs/node#62920 (22.22.3+, 24.11.1 to 24.17, 26.0, 26.1), `require()` inside a CommonJS module whose source another load hook supplied can fail. That needs a second loader, which is unsupported. - When a tool `fork()`s the local bin, the bin spawns the script and ignores `SIGINT`, so `kill -INT ` does not stop the script; Ctrl+C does, through the process group. Forwarding it would deliver Ctrl+C twice; tsx avoids that with its own IPC channel. @@ -294,7 +297,7 @@ Other errors, including transform errors, come from Node.js and oxc-node unchang ## Testing - Unit tests in `vpx_script.rs`, `vpx.rs`, and `packages/cli/src/__tests__/vpx-script.spec.ts` run the shared case table, check that the Rust and TypeScript lists match, and cover the local bin's package-mode decision and which binaries it execs without `/bin/sh` (`canExecDirectly`), the loader fallback to the global install and its real path, the Node.js range (equal to `engines.node`), the sync-hooks cutoff, and the helper mapping. `patch-oxc-node.spec.ts`, `sync-remote-deps.spec.ts`, and `toolchain.spec.ts` cover the vendoring. -- Snapshot fixtures under `crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/` run in both flavors, except one local-only case for a binary that is not installed, and the global shim and the local bin record identical snapshots. `command_vpx_script` pins Node.js 22.18.0 (the `module.register()` path) and covers syntax, resolution and `--tsconfig`, arguments and Node.js options, errors, module formats with `fork()`, `Worker`, and `--test`, TypeScript dependencies, tsconfig-driven transforms and JSX, shebangs, `--watch` (an import edit, a tsconfig edit, a created file, both `--tsconfig` positions with an env file naming another config, the tsconfig edit on 24.12.0 for the `registerHooks()` path, its help and errors, and Ctrl+C to the process group, acked once), IPC, and signals sent to the PID of `vpx` (Unix), file names with `#` and `%`, and package mode. `command_vpx_script_node22` (22.23.2), `_node24` (24.12.0), and `_node26` (26.5.0) cover the `registerHooks()` path, including CommonJS output that requires runtime helpers the project cannot resolve; `_old_node` (20.18.0) covers the version error. `shim_system_first_fallback` checks that script mode follows `vp node` in system-first mode. Windows runs every case except watch mode, shebangs, IPC, and signals, which a `.cmd` shim cannot carry, and runs its own installed-binary case with a `.cmd` binary. +- Snapshot fixtures under `crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/` run in both flavors, except one local-only case for a binary that is not installed, and the global shim and the local bin record identical snapshots. `command_vpx_script` pins Node.js 22.18.0 (the `module.register()` path) and covers syntax, resolution and `--tsconfig`, arguments and Node.js options, errors, module formats with `fork()`, `Worker`, and `--test`, TypeScript dependencies, tsconfig-driven transforms and JSX, shebangs, `--watch` (an import edit, a tsconfig edit, a created file, both `--tsconfig` positions with an env file naming another config, the tsconfig edit on 24.12.0 for the `registerHooks()` path, its help and errors, and Ctrl+C to the process group, acked once), a bare `--experimental-config-file` on 26.12.0 (a script argument named `--watch`, then `--tsconfig` and the watcher after the flag), IPC, and signals sent to the PID of `vpx` (Unix), file names with `#` and `%`, and package mode. `command_vpx_script_node22` (22.23.2), `_node24` (24.12.0), and `_node26` (26.5.0) cover the `registerHooks()` path, including CommonJS output that requires runtime helpers the project cannot resolve; `_old_node` (20.18.0) covers the version error. `shim_system_first_fallback` checks that script mode follows `vp node` in system-first mode. Windows runs every case except watch mode, shebangs, IPC, and signals, which a `.cmd` shim cannot carry, and runs its own installed-binary case with a `.cmd` binary. ## Performance From 39f18a00e3d120ffb500894285613ad9f42b13be Mon Sep 17 00:00:00 2001 From: "MK (fengmk2)" Date: Wed, 21 Oct 2026 20:42:30 +0800 Subject: [PATCH 2/6] fix(cli): wait for the child's Ctrl+C cleanup on Windows On Unix, `vp` ignores SIGINT while it waits for a child, so a Ctrl+C reaches the child and `vp` exits once the child has cleaned up. On Windows, `vp.exe` had no handler, so the console killed it with STATUS_CONTROL_C_EXIT while the child was still cleaning up, and the `vpx.exe` trampoline and `vp run` reported that early exit. `vp_command::ConsoleCtrlGuard` handles Ctrl+C and Ctrl+Break while `spawn_tool` and `execute_with_terminal_guard` wait, as the trampoline does. A handler routine, unlike a NULL handler, is not inherited, so the child still gets the event. Co-Authored-By: Claude Opus 5.5 --- crates/vp_command/src/lib.rs | 52 ++++++++++++++++++++++++++- crates/vp_global_cli/src/shim/exec.rs | 4 +++ rfcs/vpx-script-execution.md | 2 +- 3 files changed, 56 insertions(+), 2 deletions(-) diff --git a/crates/vp_command/src/lib.rs b/crates/vp_command/src/lib.rs index 9ec12e651c..142d9d9c7f 100644 --- a/crates/vp_command/src/lib.rs +++ b/crates/vp_command/src/lib.rs @@ -126,7 +126,8 @@ pub fn build_command(bin_path: &AbsolutePath, cwd: &AbsolutePath) -> Command { /// is interrupted (e.g., via Ctrl+C) while the terminal is in a non-standard state. /// /// 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. +/// it after the child exits, and ignores SIGINT meanwhile. On Windows, Ctrl+C is ignored +/// meanwhile (`ConsoleCtrlGuard`), so this process waits for the child's own cleanup. pub async fn execute_with_terminal_guard(mut cmd: Command) -> Result { #[cfg(unix)] { @@ -142,6 +143,8 @@ pub async fn execute_with_terminal_guard(mut cmd: Command) -> Result i32>, + add: i32, + ) -> i32; +} + +#[cfg(windows)] +impl ConsoleCtrlGuard { + const CTRL_BREAK_EVENT: u32 = 1; + const CTRL_C_EVENT: u32 = 0; + + /// Handle Ctrl+C and Ctrl+Break by doing nothing; leave other events, such as closing + /// the console, to the next handler. + unsafe extern "system" fn ignore(ctrl_type: u32) -> i32 { + i32::from(ctrl_type == Self::CTRL_C_EVENT || ctrl_type == Self::CTRL_BREAK_EVENT) + } + + #[must_use] + pub fn install() -> Self { + // SAFETY: registers a handler routine that only returns a value. + let installed = unsafe { SetConsoleCtrlHandler(Some(Self::ignore), 1) } != 0; + Self { installed } + } +} + +#[cfg(windows)] +impl Drop for ConsoleCtrlGuard { + fn drop(&mut self) { + if self.installed { + // SAFETY: removes the handler routine that install() registered. + unsafe { SetConsoleCtrlHandler(Some(Self::ignore), 0) }; + } + } +} + #[cfg(test)] mod tests { use tempfile::{TempDir, tempdir}; diff --git a/crates/vp_global_cli/src/shim/exec.rs b/crates/vp_global_cli/src/shim/exec.rs index 3969c7a8c2..aaeed0171b 100644 --- a/crates/vp_global_cli/src/shim/exec.rs +++ b/crates/vp_global_cli/src/shim/exec.rs @@ -37,6 +37,10 @@ pub fn spawn_tool(path: &AbsolutePath, args: &[String], env: ToolPathEnv) -> i32 }; cmd.args(args).envs(env.into_envs()); sync_child_pwd(&mut cmd); + // Ctrl+C reaches the child too; wait for its cleanup instead of dying first, so the + // caller sees the child's exit. + #[cfg(windows)] + let _ctrl = vp_command::ConsoleCtrlGuard::install(); match cmd.status() { Ok(status) => exit_code_from_status(status), Err(e) => { diff --git a/rfcs/vpx-script-execution.md b/rfcs/vpx-script-execution.md index 912babddec..5b0d41c8b2 100644 --- a/rfcs/vpx-script-execution.md +++ b/rfcs/vpx-script-execution.md @@ -351,7 +351,7 @@ Package mode through the global `vpx` is unchanged. What changes: ## Open Items -- Windows Ctrl+C through the `vp.exe` spawn-and-wait path is not verified. +- Windows Ctrl+C through the `vp.exe` spawn-and-wait path: `vp.exe` handles Ctrl+C and Ctrl+Break while it waits for the child (`vp_command::ConsoleCtrlGuard`, as the `vpx.exe` trampoline and `TerminalStateGuard` on Unix do), so the global `vpx` and `vp run` exit after the script's own cleanup. No snapshot covers it yet. ## Future Enhancements From a30cdaacf6832f5c1835a4dee363ce7218c7d5c9 Mon Sep 17 00:00:00 2001 From: "MK (fengmk2)" Date: Wed, 21 Oct 2026 21:13:45 +0800 Subject: [PATCH 3/6] test(cli): cover Ctrl+C cleanup through the global vpx and vp run A script cleans up for 300 ms after SIGINT. The global `vpx` and `vp run` must exit after it, with its code; on Windows, `vp.exe` used to exit at once with STATUS_CONTROL_C_EXIT. Co-Authored-By: Claude Opus 5.5 --- .../command_vpx_script/ctrl-c/ctrlc.ts | 12 +++++++++ .../command_vpx_script/ctrl-c/milestone.ts | 7 +++++ .../command_vpx_script/ctrl-c/package.json | 11 ++++++++ .../command_vpx_script/snapshots.toml | 27 +++++++++++++++++++ .../vp_run_vpx_script_ctrl_c_cleanup.md | 23 ++++++++++++++++ .../snapshots/vpx_script_ctrl_c_cleanup.md | 17 ++++++++++++ rfcs/vpx-script-execution.md | 2 +- 7 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/ctrl-c/ctrlc.ts create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/ctrl-c/milestone.ts create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/ctrl-c/package.json create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vp_run_vpx_script_ctrl_c_cleanup.md create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_ctrl_c_cleanup.md diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/ctrl-c/ctrlc.ts b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/ctrl-c/ctrlc.ts new file mode 100644 index 0000000000..6864234af8 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/ctrl-c/ctrlc.ts @@ -0,0 +1,12 @@ +// Cleans up for 300 ms after Ctrl+C before it exits, so a CLI that exits first shows. +import { emitMilestone } from './milestone.ts'; + +const timer = setInterval(() => {}, 1000); +process.on('SIGINT', () => { + console.log('SIGINT: cleaning up'); + setTimeout(() => { + console.log('cleaned up'); + clearInterval(timer); + }, 300); +}); +emitMilestone('ready'); diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/ctrl-c/milestone.ts b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/ctrl-c/milestone.ts new file mode 100644 index 0000000000..43fe091321 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/ctrl-c/milestone.ts @@ -0,0 +1,7 @@ +// Emits a runner milestone (a window title update), so a step can wait for it. +export function emitMilestone(name: string): void { + setImmediate(() => { + const encodedName = Buffer.from(name).toString('base64url'); + process.stdout.write(`\x1b]2;vt-pty-terminal-test:${'0'.repeat(32)}:${encodedName}\x1b\\`); + }); +} diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/ctrl-c/package.json b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/ctrl-c/package.json new file mode 100644 index 0000000000..48c80fba28 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/ctrl-c/package.json @@ -0,0 +1,11 @@ +{ + "name": "command-vpx-script-ctrl-c", + "private": true, + "type": "module", + "scripts": { + "cleanup": "vpx ./ctrlc.ts" + }, + "engines": { + "node": "22.18.0" + } +} diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots.toml index 5ec5a1a77a..21e83d2074 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots.toml @@ -239,3 +239,30 @@ steps = [ { argv = ["vpx", "--experimental-config-file", "--tsconfig", "cli.json", "--env-file=.env", "./alias.ts"], comment = "--tsconfig is still vpx's, and wins over the TS_NODE_PROJECT in .env" }, { argv = ["vpx", "--experimental-config-file", "--watch", "--include", "extra.json"], comment = "--watch still selects the watcher, which takes --include and reports the missing entry", continue-on-failure = true }, ] + +[[case]] +name = "vpx_script_ctrl_c_cleanup" +# The global vpx: on Windows it waits for the script through `vp.exe`, which Ctrl+C +# used to end at once with STATUS_CONTROL_C_EXIT (0xC000013A). +vp = "global" +cwd = "ctrl-c" +comment = "Ctrl+C reaches the script, and vpx exits after the script's SIGINT cleanup, with its exit code." +steps = [ + { argv = ["vpx", "./ctrlc.ts"], interactions = [ + { "expect-milestone" = "ready" }, + { "write-key" = "ctrl-c" }, + ] }, +] + +[[case]] +name = "vp_run_vpx_script_ctrl_c_cleanup" +# `vp run` waits for the local CLI through `vp.exe` on Windows too. +vp = "global" +cwd = "ctrl-c" +comment = "Ctrl+C under vp run reaches the script that vpx runs, and vp run exits after the script's SIGINT cleanup." +steps = [ + { argv = ["vp", "run", "cleanup"], continue-on-failure = true, interactions = [ + { "expect-milestone" = "ready" }, + { "write-key" = "ctrl-c" }, + ] }, +] diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vp_run_vpx_script_ctrl_c_cleanup.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vp_run_vpx_script_ctrl_c_cleanup.md new file mode 100644 index 0000000000..d37a21d802 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vp_run_vpx_script_ctrl_c_cleanup.md @@ -0,0 +1,23 @@ +# vp_run_vpx_script_ctrl_c_cleanup + +Ctrl+C under vp run reaches the script that vpx runs, and vp run exits after the script's SIGINT cleanup. + +## `vp run cleanup` + +**→ expect-milestone:** `ready` + +``` +VITE+ - The Unified Toolchain for the Web + +$ vpx ./ctrlc.ts ⊘ cache disabled +``` + +**← write-key:** `ctrl-c` + +``` +VITE+ - The Unified Toolchain for the Web + +$ vpx ./ctrlc.ts ⊘ cache disabled +SIGINT: cleaning up +cleaned up +``` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_ctrl_c_cleanup.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_ctrl_c_cleanup.md new file mode 100644 index 0000000000..41abf42056 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_ctrl_c_cleanup.md @@ -0,0 +1,17 @@ +# vpx_script_ctrl_c_cleanup + +Ctrl+C reaches the script, and vpx exits after the script's SIGINT cleanup, with its exit code. + +## `vpx ./ctrlc.ts` + +**→ expect-milestone:** `ready` + +``` +``` + +**← write-key:** `ctrl-c` + +``` +SIGINT: cleaning up +cleaned up +``` diff --git a/rfcs/vpx-script-execution.md b/rfcs/vpx-script-execution.md index 5b0d41c8b2..eff9e12a7a 100644 --- a/rfcs/vpx-script-execution.md +++ b/rfcs/vpx-script-execution.md @@ -351,7 +351,7 @@ Package mode through the global `vpx` is unchanged. What changes: ## Open Items -- Windows Ctrl+C through the `vp.exe` spawn-and-wait path: `vp.exe` handles Ctrl+C and Ctrl+Break while it waits for the child (`vp_command::ConsoleCtrlGuard`, as the `vpx.exe` trampoline and `TerminalStateGuard` on Unix do), so the global `vpx` and `vp run` exit after the script's own cleanup. No snapshot covers it yet. +- Windows Ctrl+C through the `vp.exe` spawn-and-wait path: `vp.exe` handles Ctrl+C and Ctrl+Break while it waits for the child (`vp_command::ConsoleCtrlGuard`, as the `vpx.exe` trampoline and `TerminalStateGuard` on Unix do), so the global `vpx` and `vp run` exit after the script's own cleanup. `vpx_script_ctrl_c_cleanup` and `vp_run_vpx_script_ctrl_c_cleanup` check that the CLI exits after a 300 ms `SIGINT` cleanup, with the script's code. ## Future Enhancements From 5a21ee4274b2504badee23a23c9b9e98da1ad361 Mon Sep 17 00:00:00 2001 From: "MK (fengmk2)" Date: Wed, 21 Oct 2026 21:23:38 +0800 Subject: [PATCH 4/6] fix(vpx): take a separate .json path after `--experimental-config-file` Reading a bare `--experimental-config-file` as taking no value broke the separate-path form that Node.js 22.x, 24.18 and earlier, and 26.0 take: `custom.json` was taken for the script, so `--tsconfig` after it was left to the watcher, where a TS_NODE_PROJECT from an env file overrode it, and `--watch --include` after it reached Node.js. Detection now takes the next argument as the config file when it ends in `.json`, the same on every release: a `.json` file cannot be the script either way. Co-Authored-By: Claude Opus 5.5 --- .../config-file/custom.json | 1 + .../config-file/watch-tsconfig.mjs | 24 ++++++++++ .../command_vpx_script/snapshots.toml | 15 +++++++ .../vpx_script_config_file_path.global.md | 31 +++++++++++++ .../vpx_script_config_file_path.local.md | 31 +++++++++++++ .../vp_global_cli/src/commands/vpx_script.rs | 19 +++++--- .../src/commands/vpx_script_cases.json | 45 +++++++++++++++++++ packages/cli/src/vpx-script.ts | 28 +++++++----- rfcs/vpx-script-execution.md | 9 ++-- 9 files changed, 183 insertions(+), 20 deletions(-) create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/custom.json create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/watch-tsconfig.mjs create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_config_file_path.global.md create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_config_file_path.local.md diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/custom.json b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/custom.json new file mode 100644 index 0000000000..2ab413bcfa --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/custom.json @@ -0,0 +1 @@ +{ "nodeOptions": { "title": "vpx-custom-config" } } diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/watch-tsconfig.mjs b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/watch-tsconfig.mjs new file mode 100644 index 0000000000..f5a0d242a9 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/watch-tsconfig.mjs @@ -0,0 +1,24 @@ +// Runs `vpx --watch` with `--tsconfig` after a separate config file path, prints the +// tsconfig the script used, and stops the watcher. +import { spawn } from 'node:child_process'; +import { createInterface } from 'node:readline'; + +const args = ['--watch', '--experimental-config-file', 'custom.json', '--tsconfig', 'cli.json']; +// Passed here: on this script's command line, Node.js would read `--env-file` for it too. +const child = spawn('vpx', [...args, '--env-file=.env', './alias.ts'], { + detached: true, + stdio: ['ignore', 'pipe', 'inherit'], +}); +const timer = setTimeout(() => { + console.log('timeout: the script printed no tsconfig'); + process.kill(-child.pid, 'SIGKILL'); + process.exit(1); +}, 20_000); +for await (const line of createInterface({ input: child.stdout })) { + if (line.startsWith('tsconfig ')) { + console.log(`vpx ${args.join(' ')} --env-file=.env ./alias.ts: ${line}`); + clearTimeout(timer); + process.kill(-child.pid, 'SIGTERM'); + break; + } +} diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots.toml index 21e83d2074..9b0ad581a9 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots.toml @@ -240,6 +240,21 @@ steps = [ { argv = ["vpx", "--experimental-config-file", "--watch", "--include", "extra.json"], comment = "--watch still selects the watcher, which takes --include and reports the missing entry", continue-on-failure = true }, ] +[[case]] +name = "vpx_script_config_file_path" +vp = ["local", "global"] +skip-platforms = ["windows"] +cwd = "config-file" +# The fixture's Node.js, 22.18.0, takes the next argument as the config file. Node.js +# warns that the flag is experimental, with a PID, in the launcher too. +env = { NODE_NO_WARNINGS = "1" } +comment = "Before Node.js 24.19 and 26.1, --experimental-config-file takes a separate .json path, so --tsconfig, --watch, and a watcher option after it keep their meaning." +steps = [ + { argv = ["vpx", "--experimental-config-file", "custom.json", "--tsconfig", "cli.json", "--env-file=.env", "./alias.ts"], comment = "--tsconfig after the path wins over the TS_NODE_PROJECT in .env" }, + { argv = ["node", "watch-tsconfig.mjs"], comment = "The same under --watch, given before the flag" }, + { argv = ["vpx", "--experimental-config-file", "custom.json", "--watch", "--include", "extra.json"], comment = "--watch after the path selects the watcher, which takes --include and reports the missing entry", continue-on-failure = true }, +] + [[case]] name = "vpx_script_ctrl_c_cleanup" # The global vpx: on Windows it waits for the script through `vp.exe`, which Ctrl+C diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_config_file_path.global.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_config_file_path.global.md new file mode 100644 index 0000000000..bdc76d233f --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_config_file_path.global.md @@ -0,0 +1,31 @@ +# vpx_script_config_file_path + +Before Node.js 24.19 and 26.1, --experimental-config-file takes a separate .json path, so --tsconfig, --watch, and a watcher option after it keep their meaning. + +## `vpx --experimental-config-file custom.json --tsconfig cli.json --env-file=.env ./alias.ts` + +--tsconfig after the path wins over the TS_NODE_PROJECT in .env + +``` +tsconfig cli.json +``` + +## `node watch-tsconfig.mjs` + +The same under --watch, given before the flag + +``` +vpx --watch --experimental-config-file custom.json --tsconfig cli.json --env-file=.env ./alias.ts: tsconfig cli.json +``` + +## `vpx --experimental-config-file custom.json --watch --include extra.json` + +--watch after the path selects the watcher, which takes --include and reports the missing entry + +**Exit code:** 1 + +``` +vpx --watch: missing +Usage: vpx --watch [options] [node options] [--] [script args...] +Run "vpx --watch --help" for the options. +``` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_config_file_path.local.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_config_file_path.local.md new file mode 100644 index 0000000000..bdc76d233f --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_config_file_path.local.md @@ -0,0 +1,31 @@ +# vpx_script_config_file_path + +Before Node.js 24.19 and 26.1, --experimental-config-file takes a separate .json path, so --tsconfig, --watch, and a watcher option after it keep their meaning. + +## `vpx --experimental-config-file custom.json --tsconfig cli.json --env-file=.env ./alias.ts` + +--tsconfig after the path wins over the TS_NODE_PROJECT in .env + +``` +tsconfig cli.json +``` + +## `node watch-tsconfig.mjs` + +The same under --watch, given before the flag + +``` +vpx --watch --experimental-config-file custom.json --tsconfig cli.json --env-file=.env ./alias.ts: tsconfig cli.json +``` + +## `vpx --experimental-config-file custom.json --watch --include extra.json` + +--watch after the path selects the watcher, which takes --include and reports the missing entry + +**Exit code:** 1 + +``` +vpx --watch: missing +Usage: vpx --watch [options] [node options] [--] [script args...] +Run "vpx --watch --help" for the options. +``` diff --git a/crates/vp_global_cli/src/commands/vpx_script.rs b/crates/vp_global_cli/src/commands/vpx_script.rs index 2cefc93ce3..2a5f8d5187 100644 --- a/crates/vp_global_cli/src/commands/vpx_script.rs +++ b/crates/vp_global_cli/src/commands/vpx_script.rs @@ -36,10 +36,6 @@ const TS_EXTENSIONS: &[&str] = &[".ts", ".mts", ".cts", ".tsx"]; /// Node.js options that take a required value, which may come as the next argument: /// those `node --help` lists as `--option=...` on Node.js 22, 24, and 26. Best effort: /// a missing entry only moves the missing-file check for the script to Node.js. -/// -/// Not `--experimental-config-file`: Node.js 24.19, 26.1, and later read it bare as -/// `node.config.json`. Older releases take the next argument as the config file, which -/// `--experimental-config-file=` names on every release. const NODE_OPTIONS_WITH_VALUE: &[&str] = &[ "-C", "-e", @@ -62,6 +58,7 @@ const NODE_OPTIONS_WITH_VALUE: &[&str] = &[ "--env-file", "--env-file-if-exists", "--eval", + "--experimental-config-file", "--experimental-default-type", "--experimental-loader", "--experimental-package-map", @@ -146,6 +143,17 @@ fn lists_option(options: &[&str], name: &str) -> bool { }) } +/// Whether the option `name`, given without `=value`, takes `next` as its value. Node.js +/// 24.19, 26.1, and later read a bare `--experimental-config-file` as `node.config.json`, +/// and older releases take the next argument as the config file, so that argument is its +/// value only when it is a `.json` file, which cannot be the script either way. +fn takes_next_arg(name: &str, next: &str) -> bool { + if lists_option(&["--experimental-config-file"], name) { + return next.ends_with(".json") && !next.starts_with('-'); + } + lists_option(NODE_OPTIONS_WITH_VALUE, name) || lists_option(WATCH_OPTIONS_WITH_VALUE, name) +} + /// A `vpx` invocation that runs Node.js with the script loader. #[derive(Debug, PartialEq, Eq)] pub struct ScriptInvocation { @@ -233,9 +241,8 @@ pub fn detect( entry &= !lists_option(NODE_OPTIONS_WITHOUT_ENTRY, name); node_args.push(arg.clone()); if name.len() == arg.len() - && (lists_option(NODE_OPTIONS_WITH_VALUE, name) - || lists_option(WATCH_OPTIONS_WITH_VALUE, name)) && let Some(value) = positional.get(index + 1) + && takes_next_arg(name, value) { node_args.push(value.clone()); index += 1; diff --git a/crates/vp_global_cli/src/commands/vpx_script_cases.json b/crates/vp_global_cli/src/commands/vpx_script_cases.json index 1e2b482f40..09bcb2135b 100644 --- a/crates/vp_global_cli/src/commands/vpx_script_cases.json +++ b/crates/vp_global_cli/src/commands/vpx_script_cases.json @@ -23,6 +23,7 @@ "--env-file", "--env-file-if-exists", "--eval", + "--experimental-config-file", "--experimental-default-type", "--experimental-loader", "--experimental-package-map", @@ -377,6 +378,50 @@ "watch": true } }, + { + "name": "a separate .json path after --experimental-config-file is its value, as older Node.js reads it", + "files": { + "main.ts": "" + }, + "args": [ + "--watch", + "--experimental-config-file", + "custom.json", + "--tsconfig", + "cli.json", + "--env-file=.env", + "main.ts" + ], + "expect": { + "script": ["--experimental-config-file", "custom.json", "--env-file=.env", "main.ts"], + "tsconfig": "cli.json", + "watch": true + } + }, + { + "name": "--watch and a watcher option after a separate .json config path keep their meaning", + "files": { + "main.ts": "" + }, + "args": [ + "--experimental-config-file", + "custom.json", + "--watch", + "--include", + "extra.json", + "main.ts" + ], + "expect": { + "script": [ + "--experimental-config-file", + "custom.json", + "--include", + "extra.json", + "main.ts" + ], + "watch": true + } + }, { "name": "the value of --import is not the script", "files": { diff --git a/packages/cli/src/vpx-script.ts b/packages/cli/src/vpx-script.ts index d62f6c098d..17d8bbf71d 100644 --- a/packages/cli/src/vpx-script.ts +++ b/packages/cli/src/vpx-script.ts @@ -17,11 +17,7 @@ import path from 'node:path'; export const SCRIPT_EXTENSIONS = ['.ts', '.mts', '.cts', '.tsx', '.js', '.mjs', '.cjs', '.jsx']; const TS_EXTENSIONS = new Set(['.ts', '.mts', '.cts', '.tsx']); -/** - * Node.js options that take a required value, which may come as the next argument (best effort). - * Not `--experimental-config-file`, which Node.js 24.19, 26.1, and later read bare as - * `node.config.json`; see `NODE_OPTIONS_WITH_VALUE` in the global CLI. - */ +/** Node.js options that take a required value, which may come as the next argument (best effort). */ export const NODE_OPTIONS_WITH_VALUE = new Set([ '-C', '-e', @@ -44,6 +40,7 @@ export const NODE_OPTIONS_WITH_VALUE = new Set([ '--env-file', '--env-file-if-exists', '--eval', + '--experimental-config-file', '--experimental-default-type', '--experimental-loader', '--experimental-package-map', @@ -142,6 +139,20 @@ function nodeOptionName(arg: string): string { return name.startsWith('--') ? `--${name.slice(2).replaceAll('_', '-')}` : name; } +/** + * Whether the option `name`, given without `=value`, takes `next` as its value. Node.js + * 24.19, 26.1, and later read a bare `--experimental-config-file` as `node.config.json`, + * and older releases take the next argument as the config file, so that argument is its + * value only when it is a `.json` file, which cannot be the script either way. Mirrors + * `takes_next_arg` in the global CLI. + */ +function takesNextArg(name: string, next: string): boolean { + if (name === '--experimental-config-file') { + return next.endsWith('.json') && !next.startsWith('-'); + } + return NODE_OPTIONS_WITH_VALUE.has(name) || WATCH_OPTIONS_WITH_VALUE.has(name); +} + export interface VpxFlags { packages: string[]; shellMode: boolean; @@ -253,11 +264,8 @@ export function detectScript(positional: string[], cwd: string): ScriptInvocatio watch ||= name === '--watch-path'; entry &&= !NODE_OPTIONS_WITHOUT_ENTRY.has(name); nodeArgs.push(arg); - if ( - !arg.includes('=') && - (NODE_OPTIONS_WITH_VALUE.has(name) || WATCH_OPTIONS_WITH_VALUE.has(name)) && - positional[index + 1] !== undefined - ) { + const next = positional[index + 1]; + if (!arg.includes('=') && next !== undefined && takesNextArg(name, next)) { nodeArgs.push(positional[++index]); } } diff --git a/rfcs/vpx-script-execution.md b/rfcs/vpx-script-execution.md index eff9e12a7a..3369c4ea70 100644 --- a/rfcs/vpx-script-execution.md +++ b/rfcs/vpx-script-execution.md @@ -69,8 +69,9 @@ if T starts with '-': # Node.js invocation; a leading optio skipping the value of options that take one (every `--option=...` that `node --help` lists on 22, 24, and 26: --import, --require, --env-file, -r, …, and the watcher's --include, --exclude, --ignore, --kill-signal, --kill-timeout) - a bare --experimental-config-file takes no value, as Node.js 24.19, 26.1, and later - read it (node.config.json), on every release + a bare --experimental-config-file takes the next token only when it ends in .json: + older releases take a separate config path, and 24.19, 26.1, and later read + the bare flag as node.config.json; a .json file cannot be the script after -e, --eval, -p, -pe, --print, --run, or --test, no token is a script: Node.js runs that code or package script, or takes test files and globs option names match with `_` for `-` (--redirect_warnings), as Node.js accepts @@ -234,7 +235,7 @@ Files: `packages/cli/src/script-preload.ts`, `script-register.ts`, `script-esm-h - On the off-thread releases (22.18 to 22.22.2, 24.11.0), a `--require` preload also runs in the `module.register()` hooks thread, so it runs twice. tsx has the same behavior. - On the off-thread releases, starting the hooks thread makes Node.js open stdout and stderr, which sets a pipe non-blocking, so `fs.writeSync(1, …)` to a slow reader can fail with `EAGAIN`. `process.stdout.write()` is not affected. tsx has the same behavior. - A `--require` or `--import` in `NODE_OPTIONS` also runs in the local bin's own Node.js before it execs the script, so it runs twice, as under the `tsx` CLI. The global `vpx` runs it once. -- A bare `--experimental-config-file` takes no value in detection, as Node.js 24.19, 26.1, and later read it. Older releases take the next argument as the config file, so there `--experimental-config-file custom.json --watch ./a.ts` leaves `--watch` to Node.js; `--experimental-config-file=custom.json` works on every release. +- A bare `--experimental-config-file` takes the next argument in detection only when it ends in `.json`, the same on every Node.js release: older releases take a separate config path, and 24.19, 26.1, and later read the bare flag as `node.config.json`. On an older release, a separate config path that does not end in `.json` is taken for the script, so `--watch` and `--tsconfig` after it are not vpx's; `--experimental-config-file=` works on every release. - `vpx --watch` puts its Ctrl+C ack socket in `TMPDIR`. When that path would pass 100 bytes (the `sun_path` limit), the watcher runs without the ack, so a Ctrl+C while it is not the terminal's foreground job reaches a script that handles `SIGINT` twice, as it would without the loader's ack. - On in-thread releases without nodejs/node#62920 (22.22.3+, 24.11.1 to 24.17, 26.0, 26.1), `require()` inside a CommonJS module whose source another load hook supplied can fail. That needs a second loader, which is unsupported. - When a tool `fork()`s the local bin, the bin spawns the script and ignores `SIGINT`, so `kill -INT ` does not stop the script; Ctrl+C does, through the process group. Forwarding it would deliver Ctrl+C twice; tsx avoids that with its own IPC channel. @@ -297,7 +298,7 @@ Other errors, including transform errors, come from Node.js and oxc-node unchang ## Testing - Unit tests in `vpx_script.rs`, `vpx.rs`, and `packages/cli/src/__tests__/vpx-script.spec.ts` run the shared case table, check that the Rust and TypeScript lists match, and cover the local bin's package-mode decision and which binaries it execs without `/bin/sh` (`canExecDirectly`), the loader fallback to the global install and its real path, the Node.js range (equal to `engines.node`), the sync-hooks cutoff, and the helper mapping. `patch-oxc-node.spec.ts`, `sync-remote-deps.spec.ts`, and `toolchain.spec.ts` cover the vendoring. -- Snapshot fixtures under `crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/` run in both flavors, except one local-only case for a binary that is not installed, and the global shim and the local bin record identical snapshots. `command_vpx_script` pins Node.js 22.18.0 (the `module.register()` path) and covers syntax, resolution and `--tsconfig`, arguments and Node.js options, errors, module formats with `fork()`, `Worker`, and `--test`, TypeScript dependencies, tsconfig-driven transforms and JSX, shebangs, `--watch` (an import edit, a tsconfig edit, a created file, both `--tsconfig` positions with an env file naming another config, the tsconfig edit on 24.12.0 for the `registerHooks()` path, its help and errors, and Ctrl+C to the process group, acked once), a bare `--experimental-config-file` on 26.12.0 (a script argument named `--watch`, then `--tsconfig` and the watcher after the flag), IPC, and signals sent to the PID of `vpx` (Unix), file names with `#` and `%`, and package mode. `command_vpx_script_node22` (22.23.2), `_node24` (24.12.0), and `_node26` (26.5.0) cover the `registerHooks()` path, including CommonJS output that requires runtime helpers the project cannot resolve; `_old_node` (20.18.0) covers the version error. `shim_system_first_fallback` checks that script mode follows `vp node` in system-first mode. Windows runs every case except watch mode, shebangs, IPC, and signals, which a `.cmd` shim cannot carry, and runs its own installed-binary case with a `.cmd` binary. +- Snapshot fixtures under `crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/` run in both flavors, except one local-only case for a binary that is not installed, and the global shim and the local bin record identical snapshots. `command_vpx_script` pins Node.js 22.18.0 (the `module.register()` path) and covers syntax, resolution and `--tsconfig`, arguments and Node.js options, errors, module formats with `fork()`, `Worker`, and `--test`, TypeScript dependencies, tsconfig-driven transforms and JSX, shebangs, `--watch` (an import edit, a tsconfig edit, a created file, both `--tsconfig` positions with an env file naming another config, the tsconfig edit on 24.12.0 for the `registerHooks()` path, its help and errors, and Ctrl+C to the process group, acked once), a bare `--experimental-config-file` on 26.12.0 and a separate `.json` config path on 22.18.0 (a script argument named `--watch`, then `--tsconfig` and the watcher after the flag or its path), IPC, and signals sent to the PID of `vpx` (Unix), file names with `#` and `%`, and package mode. `command_vpx_script_node22` (22.23.2), `_node24` (24.12.0), and `_node26` (26.5.0) cover the `registerHooks()` path, including CommonJS output that requires runtime helpers the project cannot resolve; `_old_node` (20.18.0) covers the version error. `shim_system_first_fallback` checks that script mode follows `vp node` in system-first mode. Windows runs every case except watch mode, shebangs, IPC, and signals, which a `.cmd` shim cannot carry, and runs its own installed-binary case with a `.cmd` binary. ## Performance From 3c4ca2820eed4a4d30b32d4d615c0ade3286f29e Mon Sep 17 00:00:00 2001 From: "MK (fengmk2)" Date: Wed, 21 Oct 2026 21:30:51 +0800 Subject: [PATCH 5/6] test(cli): run the direct vpx Ctrl+C case on Unix only On Windows, the snapshot runner starts steps with Ctrl+C ignored, which children inherit, and only vite-task clears it, so the script under a direct `vpx` never got Ctrl+C and the step timed out. The `vp run` case passes on Windows and covers the global vpx there as its task. Co-Authored-By: Claude Opus 5.5 --- .../fixtures/command_vpx_script/snapshots.toml | 9 ++++++--- rfcs/vpx-script-execution.md | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots.toml index 9b0ad581a9..d1e4170372 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots.toml @@ -257,9 +257,11 @@ steps = [ [[case]] name = "vpx_script_ctrl_c_cleanup" -# The global vpx: on Windows it waits for the script through `vp.exe`, which Ctrl+C -# used to end at once with STATUS_CONTROL_C_EXIT (0xC000013A). vp = "global" +# On Windows, the runner starts steps with Ctrl+C ignored, which children inherit, and +# nothing between it and the script clears that, so the script never gets Ctrl+C. vite-task +# clears it, so `vp_run_vpx_script_ctrl_c_cleanup` covers the global vpx on Windows. +skip-platforms = ["windows"] cwd = "ctrl-c" comment = "Ctrl+C reaches the script, and vpx exits after the script's SIGINT cleanup, with its exit code." steps = [ @@ -271,7 +273,8 @@ steps = [ [[case]] name = "vp_run_vpx_script_ctrl_c_cleanup" -# `vp run` waits for the local CLI through `vp.exe` on Windows too. +# On Windows, the global vpx under `vp run` waits for the script through `vp.exe`, which +# Ctrl+C used to end at once with STATUS_CONTROL_C_EXIT (0xC000013A). vp = "global" cwd = "ctrl-c" comment = "Ctrl+C under vp run reaches the script that vpx runs, and vp run exits after the script's SIGINT cleanup." diff --git a/rfcs/vpx-script-execution.md b/rfcs/vpx-script-execution.md index 3369c4ea70..b660b38e69 100644 --- a/rfcs/vpx-script-execution.md +++ b/rfcs/vpx-script-execution.md @@ -352,7 +352,7 @@ Package mode through the global `vpx` is unchanged. What changes: ## Open Items -- Windows Ctrl+C through the `vp.exe` spawn-and-wait path: `vp.exe` handles Ctrl+C and Ctrl+Break while it waits for the child (`vp_command::ConsoleCtrlGuard`, as the `vpx.exe` trampoline and `TerminalStateGuard` on Unix do), so the global `vpx` and `vp run` exit after the script's own cleanup. `vpx_script_ctrl_c_cleanup` and `vp_run_vpx_script_ctrl_c_cleanup` check that the CLI exits after a 300 ms `SIGINT` cleanup, with the script's code. +- Windows Ctrl+C through the `vp.exe` spawn-and-wait path: `vp.exe` handles Ctrl+C and Ctrl+Break while it waits for the child (`vp_command::ConsoleCtrlGuard`, as the `vpx.exe` trampoline and `TerminalStateGuard` on Unix do), so the global `vpx` and `vp run` exit after the script's own cleanup. `vpx_script_ctrl_c_cleanup` and `vp_run_vpx_script_ctrl_c_cleanup` check that the CLI exits after a 300 ms `SIGINT` cleanup, with the script's code. On Windows only the `vp run` one runs: the snapshot runner starts steps with Ctrl+C ignored, which only vite-task clears, so the global vpx there is covered as `vp run`'s task. ## Future Enhancements From d66511f59429fe58b811704d55179c1311e7177c Mon Sep 17 00:00:00 2001 From: "MK (fengmk2)" Date: Wed, 21 Oct 2026 22:03:41 +0800 Subject: [PATCH 6/6] refactor(cli): simplify the Ctrl+C guard and the new snapshot fixtures - `execute_with_terminal_guard` picks its guard per platform around one spawn and wait, and `ConsoleCtrlGuard` no longer tracks whether it was installed: removing a routine that was not added is a no-op. - `run_command`, which runs `vp install` and the `vpx` dlx fallback, waits out the child's Ctrl+C cleanup on Windows as well. - The separate config path under `--watch` is a `verify-watch.mjs` scenario instead of a third driver script; the direct `vpx` Ctrl+C case, which only ran on Unix, where the `vp run` case covers the same path, is gone; and the bare config flag case, whose detection has no Windows code, skips the Node.js 26.12 download on Windows. Co-Authored-By: Claude Opus 5.5 --- .../config-file/watch-tsconfig.mjs | 24 ----------- .../command_vpx_script/snapshots.toml | 30 +++++--------- .../vpx_script_config_file_path.global.md | 8 ---- .../vpx_script_config_file_path.local.md | 8 ---- .../snapshots/vpx_script_ctrl_c_cleanup.md | 17 -------- .../snapshots/vpx_script_watch.global.md | 6 +-- .../snapshots/vpx_script_watch.local.md | 6 +-- .../watch/paths/custom.json | 1 + .../command_vpx_script/watch/verify-watch.mjs | 16 ++++++-- crates/vp_command/src/lib.rs | 40 +++++++------------ rfcs/vpx-script-execution.md | 2 +- 11 files changed, 44 insertions(+), 114 deletions(-) delete mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/watch-tsconfig.mjs delete mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_ctrl_c_cleanup.md create mode 100644 crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/watch/paths/custom.json diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/watch-tsconfig.mjs b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/watch-tsconfig.mjs deleted file mode 100644 index f5a0d242a9..0000000000 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/config-file/watch-tsconfig.mjs +++ /dev/null @@ -1,24 +0,0 @@ -// Runs `vpx --watch` with `--tsconfig` after a separate config file path, prints the -// tsconfig the script used, and stops the watcher. -import { spawn } from 'node:child_process'; -import { createInterface } from 'node:readline'; - -const args = ['--watch', '--experimental-config-file', 'custom.json', '--tsconfig', 'cli.json']; -// Passed here: on this script's command line, Node.js would read `--env-file` for it too. -const child = spawn('vpx', [...args, '--env-file=.env', './alias.ts'], { - detached: true, - stdio: ['ignore', 'pipe', 'inherit'], -}); -const timer = setTimeout(() => { - console.log('timeout: the script printed no tsconfig'); - process.kill(-child.pid, 'SIGKILL'); - process.exit(1); -}, 20_000); -for await (const line of createInterface({ input: child.stdout })) { - if (line.startsWith('tsconfig ')) { - console.log(`vpx ${args.join(' ')} --env-file=.env ./alias.ts: ${line}`); - clearTimeout(timer); - process.kill(-child.pid, 'SIGTERM'); - break; - } -} diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots.toml index d1e4170372..07ddd0997e 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots.toml @@ -193,7 +193,7 @@ steps = [ ["node", "verify-watch.mjs", "tsconfig", "--watch"], ["node", "verify-watch.mjs", "created", "--watch"], { argv = ["node", "verify-watch.mjs", "pinned", "--tsconfig", "tsconfig.b.json", "--watch"], comment = "--tsconfig as a vpx option" }, - { argv = ["node", "verify-watch.mjs", "pinned-env", "--watch", "--tsconfig", "tsconfig.b.json"], comment = "--tsconfig among the Node.js options wins over a TS_NODE_PROJECT from an env file" }, + { argv = ["node", "verify-watch.mjs", "pinned-config", "--watch"], comment = "--watch before a separate config file path, and --tsconfig among the Node.js options after it, which wins over a TS_NODE_PROJECT from an env file" }, ] [[case]] @@ -229,6 +229,9 @@ steps = [["node", "watch.mjs"]] [[case]] name = "vpx_script_bare_config_flag" vp = ["local", "global"] +# Detection has no Windows-specific code, and the shared case table runs there; this saves +# a Node.js download on Windows. +skip-platforms = ["windows"] cwd = "config-file" # The first release where the project-local bin sees the bare flag: before 26.12, the # Node.js running the bin rewrites it to `--experimental-config-file=node.config.json`. @@ -251,35 +254,20 @@ env = { NODE_NO_WARNINGS = "1" } comment = "Before Node.js 24.19 and 26.1, --experimental-config-file takes a separate .json path, so --tsconfig, --watch, and a watcher option after it keep their meaning." steps = [ { argv = ["vpx", "--experimental-config-file", "custom.json", "--tsconfig", "cli.json", "--env-file=.env", "./alias.ts"], comment = "--tsconfig after the path wins over the TS_NODE_PROJECT in .env" }, - { argv = ["node", "watch-tsconfig.mjs"], comment = "The same under --watch, given before the flag" }, { argv = ["vpx", "--experimental-config-file", "custom.json", "--watch", "--include", "extra.json"], comment = "--watch after the path selects the watcher, which takes --include and reports the missing entry", continue-on-failure = true }, ] -[[case]] -name = "vpx_script_ctrl_c_cleanup" -vp = "global" -# On Windows, the runner starts steps with Ctrl+C ignored, which children inherit, and -# nothing between it and the script clears that, so the script never gets Ctrl+C. vite-task -# clears it, so `vp_run_vpx_script_ctrl_c_cleanup` covers the global vpx on Windows. -skip-platforms = ["windows"] -cwd = "ctrl-c" -comment = "Ctrl+C reaches the script, and vpx exits after the script's SIGINT cleanup, with its exit code." -steps = [ - { argv = ["vpx", "./ctrlc.ts"], interactions = [ - { "expect-milestone" = "ready" }, - { "write-key" = "ctrl-c" }, - ] }, -] - [[case]] name = "vp_run_vpx_script_ctrl_c_cleanup" -# On Windows, the global vpx under `vp run` waits for the script through `vp.exe`, which -# Ctrl+C used to end at once with STATUS_CONTROL_C_EXIT (0xC000013A). vp = "global" +# On Windows, the global vpx waits for the script through `vp.exe`, which Ctrl+C used to +# end at once with STATUS_CONTROL_C_EXIT (0xC000013A). The runner starts steps with Ctrl+C +# ignored, which children inherit; vite-task clears that, so the script under `vp run` +# gets Ctrl+C there, where it would not under a direct `vpx`. cwd = "ctrl-c" comment = "Ctrl+C under vp run reaches the script that vpx runs, and vp run exits after the script's SIGINT cleanup." steps = [ - { argv = ["vp", "run", "cleanup"], continue-on-failure = true, interactions = [ + { argv = ["vp", "run", "cleanup"], interactions = [ { "expect-milestone" = "ready" }, { "write-key" = "ctrl-c" }, ] }, diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_config_file_path.global.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_config_file_path.global.md index bdc76d233f..1b1c8c32d4 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_config_file_path.global.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_config_file_path.global.md @@ -10,14 +10,6 @@ Before Node.js 24.19 and 26.1, --experimental-config-file takes a separate .json tsconfig cli.json ``` -## `node watch-tsconfig.mjs` - -The same under --watch, given before the flag - -``` -vpx --watch --experimental-config-file custom.json --tsconfig cli.json --env-file=.env ./alias.ts: tsconfig cli.json -``` - ## `vpx --experimental-config-file custom.json --watch --include extra.json` --watch after the path selects the watcher, which takes --include and reports the missing entry diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_config_file_path.local.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_config_file_path.local.md index bdc76d233f..1b1c8c32d4 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_config_file_path.local.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_config_file_path.local.md @@ -10,14 +10,6 @@ Before Node.js 24.19 and 26.1, --experimental-config-file takes a separate .json tsconfig cli.json ``` -## `node watch-tsconfig.mjs` - -The same under --watch, given before the flag - -``` -vpx --watch --experimental-config-file custom.json --tsconfig cli.json --env-file=.env ./alias.ts: tsconfig cli.json -``` - ## `vpx --experimental-config-file custom.json --watch --include extra.json` --watch after the path selects the watcher, which takes --include and reports the missing entry diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_ctrl_c_cleanup.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_ctrl_c_cleanup.md deleted file mode 100644 index 41abf42056..0000000000 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_ctrl_c_cleanup.md +++ /dev/null @@ -1,17 +0,0 @@ -# vpx_script_ctrl_c_cleanup - -Ctrl+C reaches the script, and vpx exits after the script's SIGINT cleanup, with its exit code. - -## `vpx ./ctrlc.ts` - -**→ expect-milestone:** `ready` - -``` -``` - -**← write-key:** `ctrl-c` - -``` -SIGINT: cleaning up -cleaned up -``` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_watch.global.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_watch.global.md index 12bf601e4e..840fd4a054 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_watch.global.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_watch.global.md @@ -28,10 +28,10 @@ vpx --watch restarted after the file an import looked for was created vpx --tsconfig tsconfig.b.json --watch restarted after an edit to the tsconfig it was given ``` -## `node verify-watch.mjs pinned-env --watch --tsconfig tsconfig.b.json` +## `node verify-watch.mjs pinned-config --watch` ---tsconfig among the Node.js options wins over a TS_NODE_PROJECT from an env file +--watch before a separate config file path, and --tsconfig among the Node.js options after it, which wins over a TS_NODE_PROJECT from an env file ``` -vpx --watch --tsconfig tsconfig.b.json --env-file=.env.tsconfig restarted after an edit to the tsconfig it was given +vpx --watch --experimental-config-file custom.json --tsconfig tsconfig.b.json --env-file=.env.tsconfig restarted after an edit to the tsconfig it was given ``` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_watch.local.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_watch.local.md index 12bf601e4e..840fd4a054 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_watch.local.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/snapshots/vpx_script_watch.local.md @@ -28,10 +28,10 @@ vpx --watch restarted after the file an import looked for was created vpx --tsconfig tsconfig.b.json --watch restarted after an edit to the tsconfig it was given ``` -## `node verify-watch.mjs pinned-env --watch --tsconfig tsconfig.b.json` +## `node verify-watch.mjs pinned-config --watch` ---tsconfig among the Node.js options wins over a TS_NODE_PROJECT from an env file +--watch before a separate config file path, and --tsconfig among the Node.js options after it, which wins over a TS_NODE_PROJECT from an env file ``` -vpx --watch --tsconfig tsconfig.b.json --env-file=.env.tsconfig restarted after an edit to the tsconfig it was given +vpx --watch --experimental-config-file custom.json --tsconfig tsconfig.b.json --env-file=.env.tsconfig restarted after an edit to the tsconfig it was given ``` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/watch/paths/custom.json b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/watch/paths/custom.json new file mode 100644 index 0000000000..2ab413bcfa --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/watch/paths/custom.json @@ -0,0 +1 @@ +{ "nodeOptions": { "title": "vpx-custom-config" } } diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/watch/verify-watch.mjs b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/watch/verify-watch.mjs index 0522683160..92ba9fba80 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/watch/verify-watch.mjs +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_vpx_script/watch/verify-watch.mjs @@ -46,9 +46,19 @@ const SCENARIOS = { done: 'restarted after the file an import looked for was created', }, }; -// `pinned` with an env file whose TS_NODE_PROJECT names `tsconfig.json`. The scenario -// passes it: on this script's command line Node.js would read it for this script too. -SCENARIOS['pinned-env'] = { ...SCENARIOS.pinned, args: ['--env-file=.env.tsconfig'] }; +// `pinned` with a separate config file path before `--tsconfig`, and an env file whose +// TS_NODE_PROJECT names `tsconfig.json`. The scenario passes them: on this script's command +// line, Node.js would read the config file and the env file for this script too. +SCENARIOS['pinned-config'] = { + ...SCENARIOS.pinned, + args: [ + '--experimental-config-file', + 'custom.json', + '--tsconfig', + 'tsconfig.b.json', + '--env-file=.env.tsconfig', + ], +}; const [name, ...vpxArgs] = process.argv.slice(2); const scenario = SCENARIOS[name]; diff --git a/crates/vp_command/src/lib.rs b/crates/vp_command/src/lib.rs index 142d9d9c7f..69ebf982eb 100644 --- a/crates/vp_command/src/lib.rs +++ b/crates/vp_command/src/lib.rs @@ -129,25 +129,14 @@ pub fn build_command(bin_path: &AbsolutePath, cwd: &AbsolutePath) -> Command { /// it after the child exits, and ignores SIGINT meanwhile. On Windows, Ctrl+C is ignored /// meanwhile (`ConsoleCtrlGuard`), so this process waits for the child's own cleanup. pub async fn execute_with_terminal_guard(mut cmd: Command) -> Result { + // Save terminal state and ignore SIGINT (Unix) or Ctrl+C (Windows) until the child exits. #[cfg(unix)] - { - use nix::libc::STDIN_FILENO; - - // Save terminal state and ignore sigint signal for current process before spawning child - let _guard = TerminalStateGuard::save(STDIN_FILENO); - - // 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())) - } + let _guard = TerminalStateGuard::save(nix::libc::STDIN_FILENO); + #[cfg(windows)] + let _guard = ConsoleCtrlGuard::install(); - #[cfg(not(unix))] - { - #[cfg(windows)] - let _ctrl = ConsoleCtrlGuard::install(); - let mut child = cmd.spawn().map_err(|e| Error::Anyhow(e.into()))?; - child.wait().await.map_err(|e| Error::Anyhow(e.into())) - } + let mut child = cmd.spawn().map_err(|e| Error::Anyhow(e.into()))?; + child.wait().await.map_err(|e| Error::Anyhow(e.into())) } /// Build a `tokio::process::Command` for shell execution. @@ -217,6 +206,9 @@ where ); let mut cmd = build_command(&program, cwd); cmd.args(&prefix_args).args(&args).envs(envs); + // Wait out the child's own Ctrl+C cleanup; see `ConsoleCtrlGuard`. + #[cfg(windows)] + let _guard = ConsoleCtrlGuard::install(); let status = cmd.status().await?; Ok(status) } @@ -417,9 +409,7 @@ impl Drop for TerminalStateGuard { /// unlike `SetConsoleCtrlHandler(NULL, TRUE)`, is not inherited, so the child still gets /// the event. The `vpx.exe` trampoline does the same. #[cfg(windows)] -pub struct ConsoleCtrlGuard { - installed: bool, -} +pub struct ConsoleCtrlGuard(()); #[cfg(windows)] unsafe extern "system" { @@ -443,18 +433,16 @@ impl ConsoleCtrlGuard { #[must_use] pub fn install() -> Self { // SAFETY: registers a handler routine that only returns a value. - let installed = unsafe { SetConsoleCtrlHandler(Some(Self::ignore), 1) } != 0; - Self { installed } + unsafe { SetConsoleCtrlHandler(Some(Self::ignore), 1) }; + Self(()) } } #[cfg(windows)] impl Drop for ConsoleCtrlGuard { fn drop(&mut self) { - if self.installed { - // SAFETY: removes the handler routine that install() registered. - unsafe { SetConsoleCtrlHandler(Some(Self::ignore), 0) }; - } + // SAFETY: removes the routine install() registered; a no-op if it was not. + unsafe { SetConsoleCtrlHandler(Some(Self::ignore), 0) }; } } diff --git a/rfcs/vpx-script-execution.md b/rfcs/vpx-script-execution.md index b660b38e69..c633457a17 100644 --- a/rfcs/vpx-script-execution.md +++ b/rfcs/vpx-script-execution.md @@ -352,7 +352,7 @@ Package mode through the global `vpx` is unchanged. What changes: ## Open Items -- Windows Ctrl+C through the `vp.exe` spawn-and-wait path: `vp.exe` handles Ctrl+C and Ctrl+Break while it waits for the child (`vp_command::ConsoleCtrlGuard`, as the `vpx.exe` trampoline and `TerminalStateGuard` on Unix do), so the global `vpx` and `vp run` exit after the script's own cleanup. `vpx_script_ctrl_c_cleanup` and `vp_run_vpx_script_ctrl_c_cleanup` check that the CLI exits after a 300 ms `SIGINT` cleanup, with the script's code. On Windows only the `vp run` one runs: the snapshot runner starts steps with Ctrl+C ignored, which only vite-task clears, so the global vpx there is covered as `vp run`'s task. +- Windows Ctrl+C through the `vp.exe` spawn-and-wait path: `vp.exe` handles Ctrl+C and Ctrl+Break while it waits for the child (`vp_command::ConsoleCtrlGuard` in `spawn_tool`, `run_command`, and `execute_with_terminal_guard`, as the `vpx.exe` trampoline and `TerminalStateGuard` on Unix do), so the global `vpx` and `vp run` exit after the script's own cleanup. `vp_run_vpx_script_ctrl_c_cleanup` checks that the global vpx and `vp run` exit after a 300 ms `SIGINT` cleanup, with the script's code. It runs the script as a `vp run` task because the snapshot runner starts steps with Ctrl+C ignored, which only vite-task clears. ## Future Enhancements