[hooks] Detect the Vercel CLI on Windows - #143
Open
oulay2002 wants to merge 1 commit into
Open
Conversation
The session-start profiler tells every Windows user to install a CLI they already have. Two independent defects, both in the resolve-then-spawn path: 1. `getBinaryPathCandidates` put the bare name ahead of every PATHEXT suffix. npm lays down three entries per global binary in %APPDATA%\npm: `vercel` (a POSIX sh shim), `vercel.CMD` and `vercel.ps1`. The bare shim won, and spawnSync rejects it with ENOENT. 2. Even once `vercel.CMD` is selected, execFileSync cannot run it. Since the fix for CVE-2024-27980, Node refuses .cmd/.bat without `shell: true` (EINVAL). This hit the `npm view vercel version` probe too, so the outdated-version branch was dead code on Windows. Observed on Windows 11, Node 24.18.0, with Vercel CLI 58.9.0 on PATH: (no extension) execFileSync -> ENOENT .CMD execFileSync -> EINVAL .ps1 execFileSync -> EFTYPE Order candidates by what Node can actually spawn (.EXE/.COM/.CMD/.BAT first, remaining PATHEXT entries next, bare name last), and route batch wrappers through the shell with the path quoted, since cmd.exe re-parses the command line and would split an unquoted path on its spaces. Also stop reporting `installed: false` when the binary resolved but the version probe failed - the CLI is on PATH, and advising a reinstall is the one answer that cannot help. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On Windows,
session-start-profileremits this at the top of every session, even when the CLI is installed, onPATH, and authenticated:The agent then acts on it and recommends a reinstall. The outdated-version branch is unreachable on Windows for the same underlying reason, so
needsUpdatenever fires either.Root cause
Two independent defects in
checkVercelCli's resolve-then-spawn path.1. Candidate ordering picks npm's POSIX shim.
getBinaryPathCandidatesput the bare name ahead of everyPATHEXTsuffix:npm lays down three entries per global binary in
%APPDATA%\npm:vercel(anshscript for Git Bash),vercel.CMD, andvercel.ps1.accessSync(path, X_OK)succeeds on all three — on Windows it degrades to an existence check — so the bareshscript wins, andspawnSyncrejects it withENOENT.2.
.cmdcannot be spawned without a shell. Even after resolvingvercel.CMD,execFileSyncfails: since the fix for CVE-2024-27980, Node refuses.cmd/.batwithoutshell: true, withEINVAL. This also silently killed thenpm view vercel versionprobe, sincenpmresolves tonpm.CMD.Measured on Windows 11, Node 24.18.0, Vercel CLI 58.9.0 on
PATH:execFileSyncresultvercel(no extension)ENOENTvercel.CMDEINVALvercel.ps1EFTYPEEvery reachable candidate failed, so
checkVercelClireturned{ installed: false }in all cases.Fix
.EXE/.COM/.CMD/.BATfirst, remainingPATHEXTentries next, bare name last. This also keeps.PS1from outranking a real executable on machines that add it toPATHEXT—spawnSynccannot run it either.shell: true, with the path quoted, sincecmd.exere-parses the command line and would otherwise split an unquoted path on its spaces.installed: falsewhen the binary resolved but the version probe failed. The CLI is onPATH; telling the user to install it is the one answer that cannot help. It now reportsinstalled: truewith no version, which suppresses both messages rather than emitting a wrong one.getBinaryPathCandidatesand the newbinaryNeedsShelltake optionalplatform/pathExtensionsarguments so the Windows behaviour is testable from any host. Runtime behaviour is unchanged — both default toprocess.platformand the module-levelPATHEXTlist.Verification
Hook run exactly as
hooks.jsoninvokes it, before and after:not installed❌%APPDATA%\npmremovednot installed✅not installed✅The negative control matters: the "installed" case must not be silent for the wrong reason. With the fix the npm registry probe also runs for the first time on Windows, correctly reporting 58.9.0 → 59.0.0.
New tests in
hooks/session-start-profiler-binary-resolution.test.ts(8 tests) cover candidate ordering,PATHEXThandling, already-qualified names, and the shell requirement. Confirmed they fail against the current ordering logic and pass with the fix.bun run typecheckexits 0. Full suite: 849 pass / 125 fail before, 857 pass / 125 fail after — the 8 new tests, no change in failures. (The 125 are pre-existing Windows failures onmain, mostly path-separator assumptions in the snapshot and context tests; happy to open a separate issue if useful.)Notes for reviewers
hooks/session-start-profiler.mjsis the committedtsupoutput, rebuilt withbun run build:hooks. Other.mjsfiles were left untouched.CONTRIBUTING.md; let me know if you'd prefer a different branch naming or commit convention.🤖 Generated with Claude Code