Fix Windows CLI shim execution#445
Conversation
|
Opened the matching bug report as #446 for the ugig bug-fix trail. This PR fixes the Windows .cmd shim execution issue. |
Greptile SummaryThis PR fixes Windows CLI shim execution by routing bare command names through
Confidence Score: 3/5The Windows dispatch logic is sound for typical usage, but The packages/core/src/exec.ts — specifically the Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["exec(cmd, args, opts)"] --> B{shouldUseWindowsCmd?}
B -- "win32 + bare name\n(no / \\ , not .exe/.com)" --> C["spawn('cmd.exe', ['/d','/s','/c', cmd, ...args])"]
B -- "non-Windows or\npath / .exe / .com" --> D["spawn(cmd, args)"]
C --> E[child process runs via cmd.exe\n.cmd shim resolves on PATH]
D --> F[child process runs directly]
E & F --> G{exitCode / error?}
G -- ENOENT --> H["reject: 'command not found: cmd'"]
G -- "exitCode !== 0\n(throwOnNonZero)" --> I["reject: cmd failed"]
G -- exitCode 0 --> J["resolve: ExecResult"]
K["ensureCli(cmd, hint, log)"] --> L["exec(cmd, ['--version'], throwOnNonZero:false)"]
L --> M{exitCode !== 0?}
M -- yes --> N["throw 'command not found: cmd'"]
M -- no --> O["CLI present — return"]
N --> P["catch: log error\nthrow 'cmd not installed. hint'"]
Reviews (4): Last reviewed commit: "Address Windows cmd argument handling" | Re-trigger Greptile |
|
Fixed the Greptile review items in 99b9eee: restored stdin to ignore via explicit stdio, kept stdout/stderr piped, and hardened Windows shell quoting for trailing backslashes before quotes. Re-ran packages/docs/pandoc/src/index.test.ts (5 passed), @profullstack/sh1pt-core typecheck, and @profullstack/sh1pt-docs-pandoc typecheck. |
|
Fixed the latest Greptile finding in commit bb071a9: �nsureCli now treats non-zero --version exits as missing, with coverage for the Windows 9009-style case. I also limited Windows shell dispatch to bare commands so absolute .exe paths avoid shell quoting/percent-expansion concerns. Re-ran packages/core/src/exec.test.ts + packages/docs/pandoc/src/index.test.ts (7 passed), plus core and docs-pandoc typechecks. |
|
Fixed the latest Windows argument-handling concern in e8b8dea: bare Windows commands now run through cmd.exe /d /s /c with the command arguments passed separately, so we no longer build one custom-quoted command string that can corrupt percent-wrapped args or trailing backslashes. I also updated the regression test so it exercises a bare .cmd command on PATH and verifies both %SH1PT_EXEC_LITERAL% and a trailing-backslash path survive.\n\nVerification run locally:\n- npx --yes pnpm@9.12.0 exec vitest run packages/core/src/exec.test.ts packages/docs/pandoc/src/index.test.ts -> 7 passed\n- npx --yes pnpm@9.12.0 --filter @profullstack/sh1pt-core typecheck -> passed\n- npx --yes pnpm@9.12.0 --filter @profullstack/sh1pt-docs-pandoc typecheck -> passed |
| const result = await exec(cmd, ['--version'], { log: () => {}, throwOnNonZero: false }); | ||
| if (result.exitCode !== 0) throw new Error(`command not found: ${cmd}`); |
There was a problem hiding this comment.
ensureCli now rejects installed CLIs that exit non-zero for --version
On Linux (where spawn still directly executes the binary), any CLI that returns a non-zero exit code from --version — even though it is installed and working — will now be thrown as command not found: <cmd> and ultimately reported as "not installed". Before this PR, only ENOENT triggered that branch on Linux; a non-zero exit was silently ignored and treated as "present". The broader exitCode !== 0 guard is necessary on Windows (where cmd.exe swallows ENOENT and returns 9009), but applying it unconditionally on Linux changes the contract for every caller. Consider limiting the exitCode check to process.platform === 'win32' so the Linux path retains its original ENOENT-only semantics.
Summary
exec()commands through the Windows shell on Windows so.cmdshims such asnpx,vsce,jsr, andpandocresolve correctlyWhy
On Windows,
child_process.spawn('npx', ...)andspawn('pandoc', ...)fail withENOENTin this environment because command shims are exposed as.cmdfiles. That means target/doc adapters that rely on external CLIs cannot execute even when the CLI is installed on PATH.Validation
pnpm exec vitest run packages/docs/pandoc/src/index.test.ts-> 5 passedpnpm --filter @profullstack/sh1pt-core typecheckpnpm --filter @profullstack/sh1pt-docs-pandoc typecheckFor the ugig bug-fix bounty: this fixes a Windows CLI execution bug found while testing the repository locally.