Skip to content

fix: prefer .ps1 over .cmd shim on Windows#345

Open
fengmk2 wants to merge 13 commits intomainfrom
fix/prefer-ps1-over-cmd-shim
Open

fix: prefer .ps1 over .cmd shim on Windows#345
fengmk2 wants to merge 13 commits intomainfrom
fix/prefer-ps1-over-cmd-shim

Conversation

@fengmk2
Copy link
Copy Markdown
Member

@fengmk2 fengmk2 commented Apr 18, 2026

Summary

Spawning a node_modules/.bin/*.cmd shim on Windows forces the OS to start cmd.exe, which intercepts Ctrl+C with a "Terminate batch job (Y/N)?" prompt and leaves the terminal in a corrupt state (backspace prints ^H, Ctrl+C prints ^C, input becomes sluggish).

When which() resolves a .cmd shim under node_modules/.bin/ and the system has pwsh.exe or powershell.exe available, the planner substitutes the sibling .ps1 path so the task graph records the script that actually runs. At spawn time, any .ps1 program is invoked via powershell.exe -File, so Ctrl+C propagates cleanly without the intermediate cmd.exe hop. If no PowerShell host is available, the substitution is skipped — the original .cmd path is preserved so the task still runs (matching pre-PR behavior).

Architecture

The logic is split across two layers so task metadata and cache keys reflect the real script, while the PowerShell host hop stays a spawn-time concern. Both layers consult the same POWERSHELL_HOST cache so plan-time and spawn-time decisions can never disagree.

Plan layer — vite_task_plan::ps1_shim

  • powershell_host() -> Option<&'static Arc<AbsolutePath>> — cached via LazyLock, prefers pwsh.exe over powershell.exe, None when neither is on PATH (or not on Windows). Owned by the plan crate but exposed as a pub accessor so the spawn layer can reuse the same cache.
  • prefer_ps1_sibling(AbsolutePathBuf) -> AbsolutePathBuf runs inside which() right after which::which_in resolves a binary. Gated on powershell_host().is_some(): if no host is available, the .cmd is kept untouched so we don't hand CreateProcess a .ps1 it can't execute.
  • On Windows with host + sibling .ps1: substitutes node_modules/.bin/vite.cmd with node_modules/.bin/vite.ps1. SpawnCommand.program_path and SpawnFingerprint reference the .ps1 directly; cache keys stay portable (workspace-relative).
  • .cmd/.bin/node_modules comparisons are case-insensitive.
  • No-op on non-Windows (compile-time const fn passthrough).

Spawn layer — vite_task::session::execute::powershell

  • wrap_ps1_with_powershell(&program, &args) -> Option<(program, args)>.
  • If the program ends in .ps1, wraps the spawn as powershell.exe -NoProfile -NoLogo -ExecutionPolicy Bypass -File <ps1> <args>, using the same host as the plan layer.
  • Option return lets the caller reuse the original Arcs on the passthrough path — no clones for non-.ps1 spawns.

Scope (what is not touched)

The plan-time substitution fires only when all of the following hold:

  1. Extension is .cmd (case-insensitive), and
  2. The file lives under node_modules/.bin/ (case-insensitive path-component check), and
  3. A sibling .ps1 exists, and
  4. pwsh.exe or powershell.exe is on PATH.

Only .cmd is considered because npm/pnpm/yarn's cmd-shim (the tool that generates the triplets) emits .cmd/.ps1/POSIX — never .bat.

Everything else is left alone:

  • .exe / .com / .bat binaries
  • System tools (C:\Windows\System32\where.cmd, chcp.com, etc.)
  • User-installed .cmd wrappers outside node_modules/.bin/
  • Any .cmd without a sibling .ps1
  • Stripped-down Windows installs with no PowerShell host on PATH (fall back to .cmd)

Closes voidzero-dev/vite-plus#1176

Test plan

  • cargo clippy --workspace --all-targets clean on macOS
  • cargo test --workspace passes on macOS — 4 ps1_shim tests + 2 powershell tests run cross-platform via injected host
  • Windows CI green
  • Manual verification on Windows: reproduce with https://github.com/Curtion/report-vite-plus-1 after bumping vite-task in vite-plus; Ctrl+C during vp run dev should leave the terminal clean
  • Manual verification: confirm pwsh.exe is preferred when both it and powershell.exe are on PATH
  • Manual verification: on a Windows box with PATH stripped of PowerShell, confirm .cmd is still used and tasks still run (regression guard for the Cursor-flagged "no host" case)

@fengmk2 fengmk2 self-assigned this Apr 18, 2026
fengmk2 added a commit that referenced this pull request Apr 18, 2026
Address review feedback on #345:
- Extract the 5 fixed PowerShell prefix flags to a `const POWERSHELL_PREFIX`
- Build `new_args` via iterator chain instead of manual Vec pushes
- Drop the no-op `let _ = (&program_path, &args);` on non-Windows
fengmk2 added a commit to voidzero-dev/vite-plus that referenced this pull request Apr 18, 2026
Pulls in voidzero-dev/vite-task#345 which prefers .ps1 shims over .cmd
on Windows to avoid the "Terminate batch job (Y/N)?" prompt and terminal
corruption on Ctrl+C during `vp run dev`.

Closes #1176
@fengmk2

This comment was marked as resolved.

fengmk2 added a commit to voidzero-dev/vite-plus that referenced this pull request Apr 18, 2026
Pulls in voidzero-dev/vite-task#345 which prefers .ps1 shims over .cmd
on Windows to avoid the "Terminate batch job (Y/N)?" prompt and terminal
corruption on Ctrl+C during `vp run dev`.

Closes #1176
Comment thread crates/vite_task_plan/src/plan.rs Outdated
@fengmk2

This comment was marked as resolved.

fengmk2 added a commit to voidzero-dev/vite-plus that referenced this pull request Apr 18, 2026
Picks up the cache-portability fix for voidzero-dev/vite-task#345
(PowerShell rewrite moved from plan layer to spawn layer).
fengmk2 added a commit to voidzero-dev/vite-plus that referenced this pull request Apr 18, 2026
Picks up voidzero-dev/vite-task#345 fix for missing `which` dep on
Windows after the module move.
cursor[bot]

This comment was marked as resolved.

@fengmk2
Copy link
Copy Markdown
Member Author

fengmk2 commented Apr 18, 2026

@cursor review

fengmk2 added a commit to voidzero-dev/vite-plus that referenced this pull request Apr 18, 2026
Pulls in voidzero-dev/vite-task#345 which prefers .ps1 shims over .cmd
on Windows to avoid the "Terminate batch job (Y/N)?" prompt and terminal
corruption on Ctrl+C during `vp run dev`.

Closes #1176
fengmk2 added a commit to voidzero-dev/vite-plus that referenced this pull request Apr 18, 2026
Picks up the cache-portability fix for voidzero-dev/vite-task#345
(PowerShell rewrite moved from plan layer to spawn layer).
fengmk2 added a commit to voidzero-dev/vite-plus that referenced this pull request Apr 18, 2026
Picks up voidzero-dev/vite-task#345 fix for missing `which` dep on
Windows after the module move.
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 4f14ebe. Configure here.

@fengmk2 fengmk2 changed the title fix(plan): prefer .ps1 over .cmd shim on Windows fix(spawn): prefer .ps1 over .cmd shim on Windows Apr 18, 2026
@fengmk2
Copy link
Copy Markdown
Member Author

fengmk2 commented Apr 18, 2026

before

image

after

image

fengmk2 added 8 commits April 18, 2026 22:24
Spawning a .cmd shim (e.g. `vite.cmd`) from any shell forces Windows to
start `cmd.exe`, which intercepts Ctrl+C with a "Terminate batch job
(Y/N)?" prompt and leaves the terminal in a corrupt state. When the
resolved binary is `.cmd`/`.bat` and a sibling `.ps1` exists, route the
spawn through `powershell.exe -File` (preferring `pwsh.exe`) so Ctrl+C
propagates cleanly without the cmd.exe hop.

Closes voidzero-dev/vite-plus#1176
Address review feedback on #345:
- Extract the 5 fixed PowerShell prefix flags to a `const POWERSHELL_PREFIX`
- Build `new_args` via iterator chain instead of manual Vec pushes
- Drop the no-op `let _ = (&program_path, &args);` on non-Windows
Cursor review flagged that applying the rewrite at plan time leaks
absolute `.ps1` paths and `powershell.exe` into `SpawnFingerprint`,
breaking cache key portability across machines, CI, and repo moves
(the cache was designed around relative paths).

Move the rewrite to the spawn layer (`session/execute/spawn.rs`) so
the fingerprint still keys on the original tool's relative path and
args, while the actual spawn still routes through PowerShell.
The `.cmd`→`.ps1` rewrite moved from `vite_task_plan` (where `which`
was already a dep) to `vite_task` without bringing the dep along.
Windows CI failed with E0433. Only needed at `cfg(windows)`.
- Return Option<(Arc, Arc)>; caller reuses original references on the
  passthrough path, dropping two unconditional Arc clones per spawn
- Inline the `imp` submodule: two cfg-gated definitions replace the
  `cfg_attr(expect(missing_const_for_fn))` workaround
- Use eq_ignore_ascii_case to drop the lowercase String allocation
- Gate pure rewrite on `any(windows, test)` so tests run on every OS
System or user-installed .cmd/.bat files elsewhere on PATH (e.g.,
`C:\Windows\System32\where.cmd`) must not be silently rerouted through
PowerShell — the triplet shape (.cmd + .ps1 + POSIX) is only guaranteed
for npm/pnpm/yarn shims. Check the last two path components before
rewriting.

Adds a negative test for a .cmd outside node_modules/.bin.
- Compare `.bin` / `node_modules` path components case-insensitively,
  matching the existing case-insensitive `.cmd`/`.bat` check and npm's
  de-facto lowercase naming (robust to symlinks/mounts)
- Condense the scope-check comment to one line to match the repo style
- Narrow `#[expect(clippy::disallowed_types)]` from the whole test
  module to just the two helpers that actually bridge std paths
@fengmk2 fengmk2 force-pushed the fix/prefer-ps1-over-cmd-shim branch from 5ce6c62 to 3e1fc38 Compare April 18, 2026 14:25
@fengmk2
Copy link
Copy Markdown
Member Author

fengmk2 commented Apr 18, 2026

@cursor review

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 3e1fc38. Configure here.

…t spawn

Record the script that actually runs in task metadata instead of the
`.cmd` shim that never spawns:

- New `vite_task_plan::ps1_shim::prefer_ps1_sibling` substitutes
  `node_modules/.bin/*.cmd` (or `.bat`) with the sibling `.ps1` when
  resolving binaries via `which()`. Applied case-insensitively and only
  under `node_modules/.bin` so system tools stay untouched.
- `SpawnCommand.program_path` / `SpawnFingerprint` now reference the
  `.ps1` directly; cache keys stay portable because the path is still
  relative to the workspace.
- `vite_task::session::execute::powershell` shrinks to just the
  powershell wrapping: if a program ends in `.ps1`, wrap the spawn as
  `powershell.exe -NoProfile -NoLogo -ExecutionPolicy Bypass -File <ps1> <args>`.
  No more filesystem stat or path-shape check at spawn time.
- Tests split accordingly: sibling selection is covered in ps1_shim;
  spawn tests only verify the powershell wrap.
- Swap `cfg_attr(not(windows), expect(missing_const_for_fn))` for two
  cfg-gated function definitions, matching the pattern already used in
  `session::execute::powershell`
- Move `#[expect(clippy::disallowed_types)]` from the test module down
  to the two helper functions that actually bridge tempdir's std paths
@fengmk2 fengmk2 changed the title fix(spawn): prefer .ps1 over .cmd shim on Windows fix: route node_modules/.bin .cmd shims through PowerShell on Windows Apr 18, 2026
@fengmk2
Copy link
Copy Markdown
Member Author

fengmk2 commented Apr 18, 2026

@cursor review

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 26021ca. Configure here.

@fengmk2 fengmk2 changed the title fix: route node_modules/.bin .cmd shims through PowerShell on Windows refactor: prefer .ps1 over .cmd shim on Windows Apr 18, 2026
@fengmk2 fengmk2 changed the title refactor: prefer .ps1 over .cmd shim on Windows fix: prefer .ps1 over .cmd shim on Windows Apr 18, 2026
Comment thread crates/vite_task_plan/src/ps1_shim.rs Outdated
npm/pnpm/yarn's cmd-shim only emits `.cmd`/`.ps1`/POSIX triplets —
`.bat` never shows up in a `node_modules/.bin` shim. The extra check
was noise.
@fengmk2
Copy link
Copy Markdown
Member Author

fengmk2 commented Apr 19, 2026

@cursor review

@fengmk2 fengmk2 marked this pull request as ready for review April 19, 2026 07:32
@fengmk2 fengmk2 requested a review from branchseer April 19, 2026 07:33
Comment thread crates/vite_task_plan/src/ps1_shim.rs
fengmk2 added 2 commits April 19, 2026 15:45
Cursor flagged: `prefer_ps1_sibling` was substituting `.cmd` → `.ps1`
unconditionally at plan time. If neither `pwsh.exe` nor `powershell.exe`
is on PATH, spawn-time `wrap_ps1_with_powershell` returns `None`,
and `CreateProcess` then tries to run the raw `.ps1` and fails. Before
this PR the `.cmd` would have worked (with the Ctrl+C bug, but at
least it ran).

Gate the plan-time substitution on PowerShell host availability:

- Move `POWERSHELL_HOST` from spawn layer into `ps1_shim` and expose it
  via `pub fn powershell_host()`. One cached lookup shared by both
  layers — plan decides only when spawn will actually be able to
  invoke the `.ps1`.
- `prefer_ps1_sibling` returns the input unchanged when `powershell_host()`
  is `None`, so PowerShell-less Windows systems keep using the `.cmd`
  path (same behavior as before this PR).
- `session::execute::powershell` reuses the shared host, dropping its
  local `LazyLock` and the now-unused `which` dep on `vite_task`.
@fengmk2
Copy link
Copy Markdown
Member Author

fengmk2 commented Apr 19, 2026

@cursor review

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit be08530. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pressing Ctrl+C after vp run dev, pnpm run dev, or npm run dev leaves the terminal in a broken state on Windows

1 participant