fix(command): route Windows .cmd shims through PowerShell for pm commands#1498
Open
fix(command): route Windows .cmd shims through PowerShell for pm commands#1498
Conversation
Member
Author
How to use the Graphite Merge QueueAdd the label auto-merge to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Member
Author
|
deps on voidzero-dev/vite-task#368 |
fengmk2
force-pushed
the
pm-features-at-local-cli
branch
from
74c9564 to
46f67b1
Compare
Member
Author
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ 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 fc05b17. Configure here.
fengmk2
commented
Apr 30, 2026
fengmk2
added a commit
that referenced
this pull request
Apr 30, 2026
Per PR review on #1498: every Rust call site of `run_command` / `run_command_with_fspy` already builds envs with uppercase `"PATH"` (via `HashMap::from([("PATH".to_string(), format_path_env(...))])` in vite_install command handlers and vite_pm_cli). The `Path`-keyed scenario the case-insensitive lookup defended against doesn't reach this code path, so the lookup and its Windows-only regression test are dead weight. Use `envs.get("PATH")` directly.
Member
Author
|
@cursor review |
fengmk2
added
test: e2e
There was a problem hiding this comment.
✅ 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 ad7b4bf. Configure here.
fengmk2
commented
Apr 30, 2026
fengmk2
added a commit
that referenced
this pull request
Apr 30, 2026
…ProjectDir Per PR review on #1498: `projectDir?: string` doesn't belong on the canonical `ExecutionResult` in `utils/command.ts` — plain `runCommand` and `runCommandSilently` never produce one. Move it onto a new domain-specific interface `ExecutionWithProjectDir extends ExecutionResult` defined next to `runCommandAndDetectProjectDir` in `create/command.ts` (the function that originates the field). All template executors that produce projectDir now declare `Promise<ExecutionWithProjectDir>`: `executeBundledTemplate`, `executeBuiltinTemplate`, `executeGeneratorScaffold`, `executeMonorepoTemplate`, `executeRemoteTemplate`, `runRemoteTemplateCommand`. `bin.ts:938`'s `result` variable is widened to the same interface so consumers can read `.projectDir`. Type-only refactor — no runtime change.
fengmk2
changed the base branch from
pm-features-at-local-cli
to
graphite-base/1498
fengmk2
added a commit
that referenced
this pull request
Apr 30, 2026
Per PR review on #1498: every Rust call site of `run_command` / `run_command_with_fspy` already builds envs with uppercase `"PATH"` (via `HashMap::from([("PATH".to_string(), format_path_env(...))])` in vite_install command handlers and vite_pm_cli). The `Path`-keyed scenario the case-insensitive lookup defended against doesn't reach this code path, so the lookup and its Windows-only regression test are dead weight. Use `envs.get("PATH")` directly.
fengmk2
added a commit
that referenced
this pull request
Apr 30, 2026
…ProjectDir Per PR review on #1498: `projectDir?: string` doesn't belong on the canonical `ExecutionResult` in `utils/command.ts` — plain `runCommand` and `runCommandSilently` never produce one. Move it onto a new domain-specific interface `ExecutionWithProjectDir extends ExecutionResult` defined next to `runCommandAndDetectProjectDir` in `create/command.ts` (the function that originates the field). All template executors that produce projectDir now declare `Promise<ExecutionWithProjectDir>`: `executeBundledTemplate`, `executeBuiltinTemplate`, `executeGeneratorScaffold`, `executeMonorepoTemplate`, `executeRemoteTemplate`, `runRemoteTemplateCommand`. `bin.ts:938`'s `result` variable is widened to the same interface so consumers can read `.projectDir`. Type-only refactor — no runtime change.
fengmk2
force-pushed
the
graphite-base/1498
branch
from
46f67b1 to
4e511a3
Compare
fengmk2
changed the base branch from
graphite-base/1498
to
vp-create-support-org
fengmk2
added a commit
that referenced
this pull request
May 7, 2026
Per PR review on #1498: every Rust call site of `run_command` / `run_command_with_fspy` already builds envs with uppercase `"PATH"` (via `HashMap::from([("PATH".to_string(), format_path_env(...))])` in vite_install command handlers and vite_pm_cli). The `Path`-keyed scenario the case-insensitive lookup defended against doesn't reach this code path, so the lookup and its Windows-only regression test are dead weight. Use `envs.get("PATH")` directly.
fengmk2
force-pushed
the
graphite-base/1498
branch
from
4e511a3 to
b83605b
Compare
fengmk2
added a commit
that referenced
this pull request
May 7, 2026
…ProjectDir Per PR review on #1498: `projectDir?: string` doesn't belong on the canonical `ExecutionResult` in `utils/command.ts` — plain `runCommand` and `runCommandSilently` never produce one. Move it onto a new domain-specific interface `ExecutionWithProjectDir extends ExecutionResult` defined next to `runCommandAndDetectProjectDir` in `create/command.ts` (the function that originates the field). All template executors that produce projectDir now declare `Promise<ExecutionWithProjectDir>`: `executeBundledTemplate`, `executeBuiltinTemplate`, `executeGeneratorScaffold`, `executeMonorepoTemplate`, `executeRemoteTemplate`, `runRemoteTemplateCommand`. `bin.ts:938`'s `result` variable is widened to the same interface so consumers can read `.projectDir`. Type-only refactor — no runtime change.
fengmk2
added a commit
that referenced
this pull request
May 7, 2026
`rewrite_cmd_to_powershell` previously bailed early with `vp_home()?`, which silently blocked rewrites for `node_modules/.bin/*.cmd` paths whenever `vite_shared::get_vp_home()` failed (e.g. CI containers without `$HOME`, sandboxed shells). The two scopes are documented as architecturally independent — one targets vp-managed shims under `$VP_HOME`, the other targets npm/pnpm/yarn-emitted shims under `node_modules/.bin/` — but the implementation coupled them by short-circuiting on the vp_home lookup. Make `vp_home` optional through `rewrite_in_scope` and `is_in_managed_scope` so an unresolvable `$VP_HOME` only disables the vp-home arm of the `||`; `node_modules/.bin` rewrites still apply. Add a regression test covering the unresolved-vp_home case. Reported by Cursor Bugbot on PR #1498.
Member
Author
|
@cursor review |
There was a problem hiding this comment.
✅ 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 7044ab9. Configure here.
…ands Apply the same .cmd → .ps1 rewrite to `vite_command::run_command` (and `run_command_with_fspy`) so package-manager-routed commands (`vp dlx`, `vp add`, `vp remove`, `vp update`, `vp install`, …) stop spawning `cmd.exe` on Windows and Ctrl+C no longer leaves the terminal in a broken state. The task-layer fix (`vite_task_plan::ps1_shim`) only covered `node_modules/.bin/*.cmd` — too narrow for pm shims like `npm.cmd` / `pnpm.cmd` / `yarn.cmd` which live in `~/.vite-plus/js_runtime/...` and system PATH. The new module drops the `node_modules/.bin` check: if a `.cmd` has a sibling `.ps1`, route through PowerShell. Closes #1489
…bing Both `run_command` and `run_command_with_fspy` were doing the same resolve_bin + rewrite_cmd_to_powershell match. Pull that into a private helper so each call site is one line.
Switch `vite_command::ps1_shim` to depend on the new `vite_powershell` crate (companion change in voidzero-dev/vite-task#368) for the PowerShell host lookup, the fixed argument prefix, and the sibling-`.ps1` discovery. This module now just composes those primitives with vp's own conventions: absolute `.ps1` path in args and `Vec<OsString>` return type to match the spawn API. Bumps the vite-task git rev across all pinned crates to pick up the extraction.
Picks up the test-only AbsolutePathBuf import move from voidzero-dev/vite-task#368 review feedback.
Two minor cleanups from review: - Hoist `OsString` into the existing `std::ffi` `use` so the `resolve_program` return type reads `Vec<OsString>` instead of the inlined `Vec<std::ffi::OsString>`. - Drop the `Arc::<AbsolutePath>::from(...)` turbofish in the `host_arc` test helper — type inference handles it.
On Windows the canonical key is `Path`, and the TS layer's
`prependToPathToEnvs` preserves whatever casing the process started
with when adding the downloaded package-manager bin dir. Hard-coding
`envs.get("PATH")` missed a `Path`-keyed override and silently fell
back to the process PATH — where the prepended shim is absent —
making `vp create` (and any other flow routed through
`runCommandAndDetectProjectDir` / `run_command_with_fspy`) fail with
`Cannot find binary path...` for `npx`/`pnpm`.
Match the case-insensitive lookup `fspy::Command::resolve_program`
already does, so the previously-correct behavior of the fspy path
holds after vp-side resolution moved up a layer in
1b77707 (refactor(command): extract resolve_program to dedupe
ps1-rewrite plumbing).
Adds a Windows-only regression test (`resolve_program_tests`) that
writes a `.exe` into a tempdir and resolves via a `Path`-keyed env
map; without the fix the test fails because resolution falls back to
the process PATH.
Per the Codex adversarial review: the previous implementation rewrote any `.cmd` with a sibling `.ps1` to spawn through `PowerShell -ExecutionPolicy Bypass`. That covers vp's own pm shims but also silently retargets unrelated PATH commands — system tools, globally-installed npm wrappers, third-party CLIs whose `.cmd`/`.ps1` wrappers may not be semantically equivalent — and broadens execution semantics on locked-down hosts that intentionally block unsigned `.ps1` execution. Gate the rewrite on `resolved.starts_with($VP_HOME)`, where `$VP_HOME` is the vp install root (`~/.vite-plus` by default). That covers the legitimate cases (`$VP_HOME/js_runtime/node/<v>/npm.cmd` and `$VP_HOME/package_manager/<pm>/<v>/<pm>/bin/<pm>.cmd`) and leaves anything else alone. `$VP_HOME` is read once via `vite_shared::get_vp_home()` and cached in a `LazyLock`. Adds a regression test (`returns_none_for_cmd_outside_vp_home`) that puts a matching `.cmd`+`.ps1` pair outside the scope and asserts no rewrite happens.
…test Two minor cleanups from review: - Merge the two-sentence "outside `$VP_HOME` is left alone" paragraph into one that focuses on the WHY (avoid silently changing execution semantics for unrelated commands; respect locked-down hosts) instead of restating the trade-off as status-quo prose. - Rename the second `ps1_str` binding in the inside-vp-home test to `ps1_path` so the `PathBuf` and the `&str` derived from it have distinct names.
Extends the rewrite scope so `<...>/node_modules/.bin/*.cmd` is also routed through PowerShell, in addition to paths inside `$VP_HOME`. The structural check is purely path-component based (case-insensitive on `.bin`/`node_modules` to match Windows semantics) — no project locality gate. npm/pnpm/yarn-emitted shims always come with a sibling `.ps1`, so the rewrite stays self-consistent across single-package projects, hoisted monorepos, and globally-installed shims. Anything outside both scopes — system tools, third-party CLIs whose `.cmd`/`.ps1` wrappers may diverge — still keeps its existing `.cmd` path. Adds tests for the node_modules/.bin path and a casing variant (`Node_Modules\.Bin`); removes the now-redundant cwd-locality tests.
Picks up the squash-merged voidzero-dev/vite-task#368 (the `vite_powershell` crate extraction). Replaces the draft-branch SHA `957278df` with the merged-to-main SHA across all 7 vite-task crate pins (fspy, vite_glob, vite_path, vite_powershell, vite_str, vite_task, vite_workspace).
Per PR review on #1498: every Rust call site of `run_command` / `run_command_with_fspy` already builds envs with uppercase `"PATH"` (via `HashMap::from([("PATH".to_string(), format_path_env(...))])` in vite_install command handlers and vite_pm_cli). The `Path`-keyed scenario the case-insensitive lookup defended against doesn't reach this code path, so the lookup and its Windows-only regression test are dead weight. Use `envs.get("PATH")` directly.
…cstring Two cleanups from review: - Trim the "complementary task-layer rewrite" paragraph from the module docstring — readers can `git grep ps1_shim` if they need to find the sibling module in vite-task. - Tighten `rewrite_in_scope`'s `host` parameter from `&Arc<AbsolutePath>` to `&AbsolutePath` so the internal Arc wrapper no longer leaks through the test boundary. Tests now build a plain `AbsolutePathBuf` host via a renamed `host_buf` helper.
… PowerShell deadlock Two intertwined fixes: 1. Drop the stdin pipe in `runCommandSilently` — use `stdio: ['ignore', 'pipe', 'pipe']` instead of `stdio: 'pipe'`. Leaving an open stdin pipe deadlocks any descendant `.ps1` shim whose `$MyInvocation.ExpectingInput` branch reads stdin to EOF before invoking `node` (the npm/pnpm/yarn cmd-shim pwsh template). The `migration-prettier-eslint-combo` snap test on Windows hung because `vp migrate` shells out via `runCommandSilently` to `vp dlx @oxlint/migrate`, and after the .cmd → PowerShell rewrite landed on this branch the inherited stdin pipe was never closed. 2. Consolidate the two duplicate `runCommandSilently` / `runCommand` pairs (`packages/cli/src/utils/command.ts` and `packages/cli/src/create/command.ts`) into one canonical copy in `utils/command.ts`. `create/command.ts` now re-exports them and keeps only the create-specific helpers (`runCommandAndDetectProjectDir`, `getPackageRunner`, `formatDlxCommand`, `prependToPathToEnvs`). Aligns the previously diverging `runCommand` return types (`number` vs `ExecutionResult`) on `Promise<ExecutionResult>` — utils' `runCommand` had no callers so the rename is safe — and prevents the stdin fix from drifting between two copies.
`vp create vite@9.0.5 ... --template react-ts` was failing on Windows
with `vite-plus: failed to execute …\vp.exe`. The chain:
fspy spawns powershell.exe (after the .cmd → .ps1 rewrite)
→ pnpm.ps1 invokes node.exe
→ node.exe is the vite_trampoline shim
→ trampoline tries `Command::new(vp_exe).status()` → fails
fspy uses Microsoft Detours (`DetourUpdateProcessWithDll` in
`crates/fspy/src/windows/mod.rs`) to inject a tracking DLL into spawned
processes and propagate the injection to descendants via the hooked
`CreateProcess`. With the rewrite landed, an extra PowerShell frame
sits between the fspy spawn and the descendant trampolines, and the
injection chain into the trampoline's vp.exe child fails — surfacing
as the "failed to execute" error from `vite_trampoline/src/main.rs`.
Pre-branch the chain went through cmd.exe, where Detours' standard
child-process hook propagates injection cleanly.
Restore `run_command_with_fspy` to its pre-branch form:
`fspy::Command::new(bin_name)`, no upfront `resolve_bin`, no rewrite —
let fspy do its own PATH lookup as before. Also reverts the test
assertion to fspy's native error message.
The fspy path is one-shot and non-interactive (project-dir detection
in `vp create`); the Ctrl+C corruption the rewrite avoids isn't a
concern there. The rewrite stays in place for `run_command`, which
serves all the pm subcommands (`vp dlx`, `vp add`, `vp remove`,
`vp update`, `vp install`, …) where Ctrl+C does matter.
…ProjectDir Per PR review on #1498: `projectDir?: string` doesn't belong on the canonical `ExecutionResult` in `utils/command.ts` — plain `runCommand` and `runCommandSilently` never produce one. Move it onto a new domain-specific interface `ExecutionWithProjectDir extends ExecutionResult` defined next to `runCommandAndDetectProjectDir` in `create/command.ts` (the function that originates the field). All template executors that produce projectDir now declare `Promise<ExecutionWithProjectDir>`: `executeBundledTemplate`, `executeBuiltinTemplate`, `executeGeneratorScaffold`, `executeMonorepoTemplate`, `executeRemoteTemplate`, `runRemoteTemplateCommand`. `bin.ts:938`'s `result` variable is widened to the same interface so consumers can read `.projectDir`. Type-only refactor — no runtime change.
The five re-exports in `create/command.ts` were a leftover from the earlier consolidation pass: - `ExecutionResult`, `RunCommandOptions`, `RunCommandResult` (type re-exports): nobody imported them through `create/command.ts`. Three dead bridges. - `runCommand`, `runCommandSilently` (value re-exports): only `create/templates/remote.ts` consumed them. Migrate that one file to import directly from `'../../utils/command.ts'` and the bridge can go. `create/command.ts` is now strictly a create-flow module: it defines `ExecutionWithProjectDir`, `runCommandAndDetectProjectDir`, `getPackageRunner`, `formatDlxCommand`, `prependToPathToEnvs` — no bridging from utils.
…ettier
(Re-applying after rebase dropped the previous TEMP commit.)
Trim the cli-snap-test matrix to only the failing combo
(windows-latest × shard 3/3) and replace the broad snap-test step
with a targeted run of `new-create-vite-migrates-eslint-prettier`,
plus a manual reproduction that dumps:
- node/pnpm/vp versions and `vp env doctor`
- the post-create my-react-ts/package.json and vite.config.ts
- pnpm's actual `node_modules/.pnpm/*vite-plus-cor*` layout
(to verify the Windows path-length truncation)
- vite-plus-core's package.json and its imports field as JSON
(does it actually define `#module-sync-enabled`?)
- every node_modules file mentioning `module-sync-enabled`
- vp fmt --write rerun with `NODE_OPTIONS=--trace-warnings
--enable-source-maps` for the full ESM trace
- a direct `node --input-type=module -e "import('file://...')"`
of the importer to surface Node's raw stack on the
mixed-separator path
Revert this commit before merging — both the matrix trim and the
debug step are tagged with explicit TEMP: comments.
…-resolution traces
The first debug run (job 74737440965) confirmed:
- vite-plus-core defines `#module-sync-enabled` correctly
- pnpm truncates the dir name to `@voidzero-dev+vite-plus-cor_<hash>`
- Path Node sees has mixed `\` / `/` separators
But the script had a bug: `node -e "require('my-react-ts/...')"`
treated the relative path as a node_modules lookup and failed with
MODULE_NOT_FOUND at the first imports-field probe, which (with set -e
in effect) killed every subsequent group — manual vp fmt --write,
direct Node ESM import, symlink/realpath comparison — before they
could run.
Fixes:
- Remove `set -e`; each `::group::` runs independently.
- Pass paths via `process.argv[1]` so `node -e` never treats them as
module specifiers.
- Use `require('url').pathToFileURL` to build a Windows-correct
`file://` URL for the direct-import attempt.
- Add three new diagnostics:
- Walk Node's resolution from my-react-ts to vite-plus, print path
and realpath.
- Print the exact `#module-sync-enabled` import line in
`vite-plus-core/dist/vite/node/chunks/node.js`.
- Compare the pnpm symlink path with its realpath (mixed-separator
vs normalized).
…sep paths
`tmpdir()` returns a backslash path on Windows
(`C:\Users\runneradmin\AppData\Local\Temp`); concatenating
`${systemTmpDir}/vite-plus-test-${uuid}` produced a mixed-separator
seed that propagated through every downstream test path. When Node's
ESM resolver walked up from a pnpm-nested file
(`.../node_modules/.pnpm/@voidzero-dev+vite-plus-cor_<hash>/.../
chunks/node.js`) looking for the package.json that defines
`#module-sync-enabled`, the mixed `\`/`/` boundaries — combined with
the pnpm junction reparse points adding fresh backslashes — broke
the walk and surfaced as `TypeError [ERR_PACKAGE_IMPORT_NOT_DEFINED]:
Package import specifier "#module-sync-enabled" is not defined`.
This was the deterministic Windows snap-test failure observed on
`new-create-vite-migrates-eslint-prettier`. The TEMP debug
instrumentation confirmed:
- vite-plus-core defines `#module-sync-enabled` correctly
- A manual repro using a clean `${RUNNER_TEMP}/vp-debug-repro` path
(uniform separators after `cd` + `pwd`) makes `vp fmt --write`
exit 0
- Direct `node import('file://D:/.../chunks/node.js')` with
`pathToFileURL`-normalized URL succeeds with `IMPORT OK`
Only the snap-test path (mixed-sep, seeded by this concat) fails.
Fix: use `path.join`, matching every other path operation in this
file.
The previous attempt to fix the Windows mixed-separator path with
`path.join` didn't work because the seed string itself was already
mixed: `fs.realpathSync(tmpdir())` (the JS legacy implementation)
returns `C:\Users/runneradmin/AppData/Local/Temp` on this Windows
runner — backslash for the drive, forward slashes after.
`path.join` only normalizes its template input, not the prefix.
Switch to `fs.realpathSync.native` (libuv `uv_fs_realpath`) which
returns the canonical backslash path on Windows. Also replace the
remaining `${tempTmpDir}/${name}` and `${casesDir}/${name}` concats
in `runTestCase` with `path.join` so a forward slash isn't reintroduced
at the case-dir level.
This should make Node's ESM resolver see uniform-separator paths and
let `#module-sync-enabled` subpath imports inside pnpm-nested
`vite-plus-core` resolve cleanly during `vp fmt --write`.
… Drive Empirical test: in the previous debug run the manual repro on \`D:\\a\\_temp\\vp-debug-repro\` (Dev Drive, ReFS) ran \`vp create\` + \`vp fmt --write\` cleanly while the same flow on \`C:\\Users\\runneradmin\\AppData\\Local\\Temp\\…\` (NTFS) failed with \`ERR_PACKAGE_IMPORT_NOT_DEFINED\` for \`#module-sync-enabled\`. Same Node, same pnpm, same code — only the filesystem differs. Hypothesis: NTFS preserves backslashes in the resolved-target portion when Node walks pnpm's junction reparse points, producing the mixed \`\\\` / \`/\` path that breaks Node's ESM package walk-up. ReFS doesn't. Override TEMP/TMP to point at the Dev Drive only for the targeted snap-test step. If the test now passes, the filesystem hypothesis holds and the long-term fix is to route the snap-test temp dir onto the Dev Drive (or normalize separators in vp's resolver path serialization). If it still fails, the bug is elsewhere.
Permanent fix for the Windows-only `new-create-vite-migrates-eslint-prettier`
snap-test failure. The TEMP-debug experiment confirmed that pointing
the snap-test temp dir at the Dev Drive (ReFS) makes
`vp fmt --write` succeed (`Code formatted`); leaving it on NTFS-on-C:
fails with `ERR_PACKAGE_IMPORT_NOT_DEFINED` for `#module-sync-enabled`.
Root cause: NTFS preserves the junction-target backslashes during
reparse-point resolution, producing mixed `\` / `/` paths that confuse
Node's ESM subpath-import walk-up inside pnpm's nested
`.pnpm/<truncated>/node_modules/<scope>/<pkg>` layout. ReFS resolves
junctions to clean paths.
Changes:
- Add `TEMP,{{ DEV_DRIVE }}/Temp` and `TMP,{{ DEV_DRIVE }}/Temp` to
the `setup-dev-drive` `env-mapping` in the `cli-snap-test` job.
Job-scoped, only applies on Windows (the step is gated on
`runner.os == 'Windows'`); macOS/Linux runners are unaffected.
- Revert the TEMP debug instrumentation: restore the full
3-OS × 3-shard matrix and the original snap-test step that runs
the standard local + global sharded suites.
- Update `new-create-vite-migrates-eslint-prettier/snap.txt` to
include the `Packages: +<variable>` / `+` / `Progress: …, done`
lines that pnpm now emits in CI (independent pnpm-output drift,
surfaced once the resolution issue was fixed).
Strict improvements from earlier debug iterations are kept:
- `fs.realpathSync.native` for `tmpdir()` resolution.
- `path.join` for `tempTmpDir` and `caseTmpDir` construction.
The previous commit pointed TEMP/TMP at the Dev Drive (`E:\Temp`) via `setup-dev-drive`'s env-mapping but didn't create the directory. `setup-dev-drive` only mounts the drive — it doesn't pre-create the target directories. The bootstrap CLI installer immediately `lstat`'d `$TEMP` and failed with ENOENT, killing the job before any snap test could run. Add a small bash step (Windows-only) right after `Setup Dev Drive` that runs `mkdir -p "$TEMP" "$TMP"`.
The vp create + auto-migrate output is long, environment-sensitive, and produces unstable lines (pnpm install progress, dependency counts, scaffold messages from upstream create-vite) that drift between Linux/macOS and Windows runners. Setting `ignoreOutput: true` on the create command lets us assert what we actually care about — that the post-migrate filesystem state and generated configs are correct — via the follow-up `test ! -f`, `cat`, and `node -e` commands. Trim snap.txt to drop the captured create-and-migrate output (59 lines), matching the new ignoreOutput behavior. Verified by running `pnpm -F ./packages/cli snap-test-global new-create-vite-migrates-eslint-prettier` locally — passes cleanly without regenerating the snap.
`rewrite_cmd_to_powershell` previously bailed early with `vp_home()?`, which silently blocked rewrites for `node_modules/.bin/*.cmd` paths whenever `vite_shared::get_vp_home()` failed (e.g. CI containers without `$HOME`, sandboxed shells). The two scopes are documented as architecturally independent — one targets vp-managed shims under `$VP_HOME`, the other targets npm/pnpm/yarn-emitted shims under `node_modules/.bin/` — but the implementation coupled them by short-circuiting on the vp_home lookup. Make `vp_home` optional through `rewrite_in_scope` and `is_in_managed_scope` so an unresolvable `$VP_HOME` only disables the vp-home arm of the `||`; `node_modules/.bin` rewrites still apply. Add a regression test covering the unresolved-vp_home case. Reported by Cursor Bugbot on PR #1498.
The `pnpm`/`npm`/`yarn` `.ps1` wrappers introspect stdin (via `$MyInvocation.ExpectingInput` and similar) and hang or misbehave when stdin is piped or null — i.e. when vite-plus runs in CI, snap tests with `stdio: ['ignore', ...]`, or any scripted invocation. The earlier `migration-prettier-eslint-combo` snap-test timeout was one symptom of this class of bug. The rewrite exists to suppress the `cmd.exe` "Terminate batch job" prompt on Ctrl+C, which is only meaningful when there is a real terminal to corrupt. Gating on `stdin.is_terminal()` keeps the Ctrl+C protection in interactive shells while letting non-interactive spawns go through `.cmd` directly, sidestepping the ps1 stdin hangs. Cache the TTY check once via `LazyLock` (stdin TTY-ness is fixed for the process lifetime, and `build_command` always inherits stdin into children). Thread the bit through `rewrite_in_scope` as a new `is_interactive` parameter for testability, alongside the existing `vp_home`/`host` test seams. Add a regression test covering the non-interactive bypass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Apply the same .cmd → .ps1 rewrite to
vite_command::run_command(andrun_command_with_fspy) so package-manager-routed commands (vp dlx,vp add,vp remove,vp update,vp install, …) stop spawningcmd.exeon Windows and Ctrl+C no longer leaves the terminal in abroken state.
The task-layer fix (
vite_task_plan::ps1_shim) only coverednode_modules/.bin/*.cmd— too narrow for pm shims likenpm.cmd/pnpm.cmd/yarn.cmdwhich live in~/.vite-plus/js_runtime/...and system PATH. The new module drops the
node_modules/.bincheck:if a
.cmdhas a sibling.ps1, route through PowerShell.Closes #1489
Note
Medium Risk
Changes Windows process-spawning behavior for
vite_command::run_commandby rewriting certain.cmdshims to PowerShell, which can affect command execution semantics and CI behavior on Windows. Also tweaks temp directory handling and CI env vars on Windows, potentially impacting snapshot test stability.Overview
Fixes Windows Ctrl+C/terminal corruption for vp-managed package manager shims by adding a
.cmd→ sibling.ps1rewrite path invite_command(resolve_program+ newps1_shimmodule) so eligible commands are spawned viapowershell.exewith a safe arg prefix.Updates the CLI create flow to use a unified
ExecutionResulttype fromutils/command(and returns{ exitCode }objects consistently), while adjustingrunCommandSilentlyto avoid stdin piping that can deadlock PowerShell.ps1wrappers.Hardens Windows snapshot testing by moving
TEMP/TMPonto the ReFS dev drive in CI (and ensuring the directory exists) and by makingsnap-test.tsuserealpathSync.native+path.joinfor temp paths and safer cleanup logging.Bumps
vite-taskgit dependencies (including newvite_powershell) to a newer revision and wiresvite_powershell/vite_sharedintovite_command.Reviewed by Cursor Bugbot for commit 7044ab9. Configure here.