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/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/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/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 4c786fd9dd..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]] @@ -225,3 +225,50 @@ 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"] +# 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`. +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 }, +] + +[[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 = ["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 = "vp_run_vpx_script_ctrl_c_cleanup" +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"], 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_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_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..1b1c8c32d4 --- /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,23 @@ +# 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 +``` + +## `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..1b1c8c32d4 --- /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,23 @@ +# 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 +``` + +## `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_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 9ec12e651c..69ebf982eb 100644 --- a/crates/vp_command/src/lib.rs +++ b/crates/vp_command/src/lib.rs @@ -126,25 +126,17 @@ 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 { + // 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))] - { - 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. @@ -214,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) } @@ -408,6 +403,49 @@ impl Drop for TerminalStateGuard { } } +/// While alive, Ctrl+C and Ctrl+Break do not end this process, as `TerminalStateGuard` +/// ignores SIGINT on Unix: the console sends them to the child too, and this process +/// waits for the child to finish its cleanup and exits with its code. A handler routine, +/// 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(()); + +#[cfg(windows)] +unsafe extern "system" { + fn SetConsoleCtrlHandler( + handler: Option 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. + unsafe { SetConsoleCtrlHandler(Some(Self::ignore), 1) }; + Self(()) + } +} + +#[cfg(windows)] +impl Drop for ConsoleCtrlGuard { + fn drop(&mut self) { + // SAFETY: removes the routine install() registered; a no-op if it was not. + unsafe { SetConsoleCtrlHandler(Some(Self::ignore), 0) }; + } +} + #[cfg(test)] mod tests { use tempfile::{TempDir, tempdir}; diff --git a/crates/vp_global_cli/src/commands/vpx_script.rs b/crates/vp_global_cli/src/commands/vpx_script.rs index 29b3c975e5..2a5f8d5187 100644 --- a/crates/vp_global_cli/src/commands/vpx_script.rs +++ b/crates/vp_global_cli/src/commands/vpx_script.rs @@ -143,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 { @@ -230,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 c8d50ebb80..09bcb2135b 100644 --- a/crates/vp_global_cli/src/commands/vpx_script_cases.json +++ b/crates/vp_global_cli/src/commands/vpx_script_cases.json @@ -327,6 +327,101 @@ "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": "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/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/packages/cli/src/vpx-script.ts b/packages/cli/src/vpx-script.ts index 3550dde07f..17d8bbf71d 100644 --- a/packages/cli/src/vpx-script.ts +++ b/packages/cli/src/vpx-script.ts @@ -139,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; @@ -250,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 6242e82e19..c633457a17 100644 --- a/rfcs/vpx-script-execution.md +++ b/rfcs/vpx-script-execution.md @@ -69,6 +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 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 @@ -232,6 +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 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. @@ -294,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), 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 @@ -348,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 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` 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