Skip to content

Don't pass duplicate /d flag in Windows cmd use-on-cd hook - #1579

Open
emerson-d-lopes wants to merge 1 commit into
Schniz:masterfrom
emerson-d-lopes:fix/cmd-cd-duplicate-d-flag
Open

Don't pass duplicate /d flag in Windows cmd use-on-cd hook#1579
emerson-d-lopes wants to merge 1 commit into
Schniz:masterfrom
emerson-d-lopes:fix/cmd-cd-duplicate-d-flag

Conversation

@emerson-d-lopes

Copy link
Copy Markdown

Fixes #1556

Problem

The generated cd.cmd always runs cd /d %*. When the user supplies their own /d (e.g. cd /d D:\), the flag is passed twice and cmd fails with:

The syntax of the command is incorrect.

Fix

Add /d only when the first argument isn't already /d:

if /i "%~1" == "/d" cd %*
if /i not "%~1" == "/d" cd /d %*

The shape is deliberate — each alternative fails in a non-obvious way (all verified empirically on Windows 11 cmd.exe):

  • No parenthesized if/else blocks: %* may contain )cd /d C:\Program Files (x86) inside a ( ... ) block closes the block early and breaks the command. The current single-line form handles it; a block would regress it.
  • No setlocal: endlocal reverts directory changes, which would break cd entirely.
  • No goto/labels: the file is stored with LF endings and embedded byte-exact via include_bytes!; cmd's label scanning is unreliable with LF-only line endings. The two guarded ifs avoid labels altogether.
  • First-argument check only (rather than scanning all args): native cd itself only accepts /d before the path (cd C:\ /d fails with "The system cannot find the path specified"), so positions after the first never work anyway and are passed through to native cd to produce its normal error.

Testing

WinCmd e2e tests are currently skip-listed (e2e/describe.ts), so I verified the behavior directly on Windows 11 with a 15-case matrix against real cmd.exe, comparing old script vs new script vs native cd:

Case Old New
cd /d Y:\ (cross-drive via subst, dir contains .node-version) ❌ syntax error ✅ cd works + fnm use --silent-if-unchanged fires
cd /D Y:\ (uppercase)
cd .., cd \, cd (no args) ✅ unchanged
cd "C:\Program Files" / unquoted cd C:\Program Files ✅ unchanged
cd C:\Program Files (x86) and cd /d C:\Program Files (x86) ✅ / ❌ ✅ / ✅
nonexistent dir stays put + native error unchanged
.node-version / .nvmrc / FNM_VERSION_FILE_STRATEGY=recursive triggers ✅ unchanged (verified with a stubbed fnm on PATH)
environment variable leakage into the session none none (fix uses no variables)

Also verified:

  • The LF-endings variant of the script (what release binaries embed) passes the same cases.
  • cargo test — 28/28 pass, including shell::windows_cmd::tests::use_on_cd_quotes_macro_path.
  • Built fnm.exe env --use-on-cd --shell cmd generates the fixed script; re-ran key cases against the generated file.
  • Added e2e/repros/issue-1556.test.ts per AGENTS.md (WinCmd-only; it self-skips today but will cover this once WinCmd e2e is enabled). Compiles under jest, prettier clean.
  • Changeset included (patch).

🤖 Generated with Claude Code

https://claude.ai/code/session_01U5TQ1y3yBEDx73tYTHoUBj

The generated cd.cmd always prepended /d, so a user-supplied /d was
passed twice and `cd /d D:\` failed with "The syntax of the command
is incorrect". Only add /d when the first argument isn't already /d.

The two guarded single-line ifs are deliberate: parenthesized blocks
break on paths containing ")" (e.g. "C:\Program Files (x86)"), goto
labels misbehave with LF line endings (the file is stored with LF and
embedded via include_bytes!), and setlocal would revert the directory
change on endlocal.

Fixes Schniz#1556

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U5TQ1y3yBEDx73tYTHoUBj
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.

cd /d D:\ doesn't work

1 participant