fix(sh): escape /C so MSYS doesn't drop the cmd switch

[zkochan]

Summary

The sh shim generated for a .cmd / .bat target runs the script via:

exec cmd /C "<path>" "$@"

Under Git Bash (MSYS), the path-conversion layer that runs when bash spawns a native Win32 process rewrites arguments matching POSIX-path heuristics. A bare /C is treated as a path and rewritten to C:\. cmd.exe then never sees the /C flag — it starts interactively and reads the remainder of the calling script as input until EOF.

Prefixing with // is the MSYS escape: //C survives the translation and reaches cmd.exe as /C. The cmd shim is unaffected (cmd.exe doesn't path-translate its own args), so the change is scoped to generateShShim.

Repro

In a Git Bash shell on Windows after this PR is reverted, any cmd-shim-wrapped batch script reproduces the bug. Concretely, in a pnpm Windows CI run:

# pn is a sh shim that does: exec cmd /C "...pn.cmd" "$@"
$ pn exec node -v
Microsoft Windows [Version 10.0.26100.32690]
(c) Microsoft Corporation. All rights reserved.

D:\a\pnpm\pnpm>

(That's cmd.exe's interactive banner and prompt. The next line of the calling script is then fed to cmd as input.)

After live-patching the shim with sed 's,/C ,//C ,', the same call returns v22.13.0 as expected. Full diagnostic + proof: https://github.com/pnpm/pnpm/actions/runs/25701484608/job/75462777365

Test plan

• Existing unit/e2e tests pass (node --test test/test.js test/e2e.test.js, 58/58)
• Snapshot updated to reflect the /C → //C change on the two relevant sh-shim lines (no other snapshots touched)
• End-to-end validated on Windows CI by patching the live shim with sed — pn exec node -v returns the expected version after the patch
• Once released, bump @zkochan/cmd-shim in pnpm and re-run the Windows test job

Context

Bug introduced in #46 (which added the /C switch). #48 expanded MSYS detection in the same shim but didn't touch the args. Affects any .cmd / .bat target invoked via the sh shim from Git Bash / MSYS / Cygwin.

---

Written by an agent (Claude Code, claude-opus-4-7).

Summary by CodeRabbit

• Bug Fixes

  • Ensure shims preserve cmd.exe switches under MSYS/Git Bash by escaping bare /C and /K when targeting cmd, so wrapped commands run reliably.

• Tests

  • Updated snapshots to reflect escaped switch behavior.
  • Added a Windows/Git Bash regression test to verify shims preserve cmd switches and produce expected output.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f86a2a60-e366-40b2-82c4-4ecdbdb35fa7

📥 Commits

Reviewing files that changed from the base of the PR and between 47c17a8 and 51031bb.

📒 Files selected for processing (1)

• src/index.ts

📜 Recent review details

🔇 Additional comments (2)

src/index.ts (2)

327-334: Solid MSYS switch-escape helper.

This correctly targets bare /C and /K tokens while preserving surrounding spacing, so it fixes MSYS translation without altering unrelated args.

---

445-455: Good runtime scoping for escape application.

Conditioning the rewrite on cmd/cmd.exe keeps the Git Bash fix in place while avoiding argument mutation for non-cmd shims.

---

📝 Walkthrough

Walkthrough

Adds MSYS/Git Bash escaping for cmd.exe switches: a new helper rewrites /C and /K to //C///K, integrates it into sh shim generation, and updates snapshot and e2e tests to verify behavior under Bash/MSYS.

Changes

MSYS Command Switch Escaping

Layer / File(s)	Summary
MSYS Command Switch Escaping Helper src/index.ts	New escapeMsysCmdSwitches(args) applies a regex to prefix standalone /C and /K with an extra / producing //C///K.
Shell Shim Argument Processing src/index.ts	generateShShim now derives args by applying escapeMsysCmdSwitches to opts.args instead of using raw opts.args directly.
Snapshot Test Verification test/test.js.snapshot	bat.shim snapshot updated to show //C instead of /C in cmd invocation for both "$basedir/cmd" and fallback cmd paths.
E2E Test Import test/e2e.test.js	Adds spawnSync import from node:child_process.
Windows / Git Bash Regression Test test/e2e.test.js	Adds a regression test that runs the shim in Git Bash, asserts the wrapped .cmd output (HELLO_FROM_CMD) is present and that the interactive cmd.exe banner is not emitted.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A rabbit hops through bash and shell,
I add a slash so switches dwell,
From /C to //C they safely pass,
No MSYS trick will drop the task,
Hooray—no more lost flags! 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main fix: escaping the /C switch to prevent MSYS from dropping it in cmd.exe invocations from sh shims.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/msys-path-conv

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/index.ts`:
- Around line 445-452: The code currently calls escapeMsysCmdSwitches(opts.args
|| '') unconditionally, which can rewrite legitimate non-cmd arguments; change
this so escaping is only applied when we are actually invoking a cmd-style
runtime (e.g. .cmd/.bat shims or cmd.exe). Concretely, detect the cmd runtime
before escaping (for example via the resolved shim/target extension or an
existing runtime indicator) and only call escapeMsysCmdSwitches when that check
is true; leave args as opts.args otherwise. Reference escapeMsysCmdSwitches,
opts.args, and the sh/runtime determination (e.g. shNodePath /
normalizePathEnvVar or the code path that identifies .cmd/.bat targets) to
locate where to insert the conditional.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4afe6277-acad-4dd4-9c0b-bd8cd61a6692

📥 Commits

Reviewing files that changed from the base of the PR and between 902cd8a and ae1c082.

📒 Files selected for processing (2)

• src/index.ts
• test/test.js.snapshot

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: test (windows-latest, 22)
• GitHub Check: test (windows-latest, 24)

🔇 Additional comments (2)

src/index.ts (1)

327-334: MSYS switch escaping helper looks correct

Nice addition — the token-level replacement for bare /C and /K is clear and matches the intended MSYS passthrough behavior.

test/test.js.snapshot (1)

15-17: Snapshot update matches the sh-shim behavior change

The /C → //C expectations here correctly reflect the new MSYS-safe sh shim output.

[coderabbitai]

🧹 Nitpick comments (1)

test/e2e.test.js (1)

46-49: ⚡ Quick win

Add a timeout to spawnSync to avoid potential CI hangs.

At Line 46, this test exercises a path that can become interactive on regression. A timeout keeps the suite from stalling indefinitely.

Proposed patch

     const r = spawnSync(bash, ['--noprofile', '--norc', shim], {
       encoding: 'utf8',
       stdio: ['ignore', 'pipe', 'pipe'],
+      timeout: 15_000,
+      killSignal: 'SIGKILL',
     })

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/e2e.test.js` around lines 46 - 49, The spawnSync invocation may hang in
CI; update the options passed to spawnSync (the call creating r with variables
spawnSync, bash, shim) to include a timeout (e.g., timeout in milliseconds) so
the child is forcibly terminated after a reasonable period and the test fails
fast; add the timeout property to the options object (and optionally set
killSignal if desired) so the synchronous spawn cannot block the test runner
indefinitely.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@test/e2e.test.js`:
- Around line 46-49: The spawnSync invocation may hang in CI; update the options
passed to spawnSync (the call creating r with variables spawnSync, bash, shim)
to include a timeout (e.g., timeout in milliseconds) so the child is forcibly
terminated after a reasonable period and the test fails fast; add the timeout
property to the options object (and optionally set killSignal if desired) so the
synchronous spawn cannot block the test runner indefinitely.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9a53b34c-040d-4f22-bfb1-509c917e6a20

📥 Commits

Reviewing files that changed from the base of the PR and between ae1c082 and 47c17a8.

📒 Files selected for processing (1)

• test/e2e.test.js

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: test (windows-latest, 22)
• GitHub Check: test (windows-latest, 24)

🔇 Additional comments (1)

test/e2e.test.js (1)

51-57: Nice regression assertions.

The positive check for HELLO_FROM_CMD plus the negative check for the interactive cmd banner gives a clear signal that the MSYS switch escaping is working.