Fix Windows npm shim PTY launches

[anst-dev]

No description provided.

[greptile-apps]

Greptile Summary

This PR fixes Windows PTY launches for npm-installed binaries (e.g., claude, codex) that are exposed as .cmd shims. Previously, these shims were wrapped through cmd.exe, which caused issues with long argument strings hitting the Windows command-line length limit.

• New resolveWindowsNpmShimLaunch: Reads the .cmd shim file, parses out the real target (a native .exe/.com or a Node .js script), and returns the target binary + original args — bypassing cmd.exe entirely.
• resolveWindowsBinaryPath refactor: Extracts the full resolved path alongside the extension so shim parsing has a concrete file path to read from, without duplicating the PATH-search logic.
• Test fixes: Three pre-existing Windows PTY tests now set PATH=\"\" to prevent accidental shim resolution against the host environment, making them properly hermetic.

Confidence Score: 4/5

Safe to merge; the shim-detection path is additive and falls back gracefully to the existing cmd.exe launch when it can't parse the shim.

The core logic is sound and well-covered by real-file integration tests. The case-insensitive regex combined with case-sensitive string replacement in resolveDp0Path is a latent inconsistency, and there is no accessibility check on the target binary before returning it from resolveWindowsNpmShimLaunch, which could cause an opaque spawn error instead of a clean fallback. Neither issue affects the happy path that npm shims actually use.

src/core/windows-cmd-launch.ts — specifically the resolveDp0Path and resolveWindowsNpmShimLaunch functions

Important Files Changed

Filename	Overview
src/core/windows-cmd-launch.ts	Refactors resolveWindowsBinaryExtension into resolveWindowsBinaryPath (returns full path + extension), adds npm shim parsing via extractWindowsNpmShimTarget and resolveDp0Path, and exports resolveWindowsNpmShimLaunch. Case-sensitivity mismatch in resolveDp0Path and a missing accessibility check on the resolved target are minor robustness gaps.
src/terminal/pty-session.ts	Adds resolveWindowsNpmShimLaunch as a first-pass check before the existing cmd-shell wrapping logic; falls back correctly when the shim check returns null. Logic is clean and well-guarded.
test/runtime/core/windows-cmd-launch.test.ts	Adds two integration tests for resolveWindowsNpmShimLaunch covering the native-exe and node-script shim patterns; uses real temp files for hermetic coverage.
test/runtime/terminal/pty-session.test.ts	Adds an integration test for shim-launch in PtySession and fixes three existing Windows tests by setting PATH="" to prevent accidental shim resolution against the host environment.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["PtySession.spawn(binary, args)"] --> B["resolveWindowsNpmShimLaunch()"]
    B --> C{win32?}
    C -- No --> D[return null]
    C -- Yes --> E["resolveWindowsBinaryPath()"]
    E --> F{Found .cmd?}
    F -- No --> D
    F -- Yes --> G["extractWindowsNpmShimTarget()\nreadFileSync shim"]
    G --> H{Match regex\npattern?}
    H -- No match --> D
    H -- ".exe/.com" pattern --> I["return { binary: target.exe, args }"]
    H -- "%_prog% .js" pattern --> J["return { binary: node, args: [target.js, ...args] }"]
    D --> K{windowsShimLaunch\nis null?}
    K -- Yes --> L["shouldUseWindowsCmdLaunch()"]
    L --> M{Needs cmd.exe\nwrapping?}
    M -- Yes --> N["spawn cmd.exe /d /s /c ..."]
    M -- No --> O["spawn binary directly"]
    K -- No --> P["spawn resolved shim target"]
    I --> P
    J --> P

Loading

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
src/core/windows-cmd-launch.ts:103-106
**Case-insensitive regex, case-sensitive path replacement**

The regex in `extractWindowsNpmShimTarget` uses the `/i` flag, so it matches `%DP0%\...` or `%Dp0%\...` in the file content and captures the text as-is. However, `resolveDp0Path` calls `replaceAll("%dp0%\\", ...)` and `replaceAll("%dp0%/", ...)`, which are case-sensitive. If the captured value uses any casing other than lowercase (e.g., `%DP0%\node.exe`), neither `replaceAll` matches and `path.resolve()` is called on the raw `%DP0%` string, producing a path relative to CWD instead of the shim directory. npm consistently generates lowercase `%dp0%`, so this is unlikely in practice, but the mismatch between the regex and the replacement is fragile.

### Issue 2 of 2
src/core/windows-cmd-launch.ts:185-188
No accessibility check on the resolved target binary. If the `.cmd` shim exists but the `.exe` it points to has been deleted or moved, `resolveWindowsNpmShimLaunch` returns a non-null result and `pty.spawn` is called with a non-existent binary, throwing an opaque native error. A quick `canAccessPath` guard here preserves the existing fallback path via `shouldUseWindowsCmdLaunch`.

```suggestion
	const targetExtension = extname(target).toLowerCase();
	if (targetExtension === ".exe" || targetExtension === ".com") {
		return canAccessPath(target) ? { binary: target, args } : null;
	}
```

_{Reviews (1): Last reviewed commit: "Fix Windows npm shim PTY launches" | Re-trigger Greptile}

[anst-dev]

Addressed both review comments:

• Made %dp0% path replacement case-insensitive to match the shim parsing regex.
• Added canAccessPath guard before directly launching resolved .exe/.com targets.

Added regression tests for mixed-case %DP0% and missing executable fallback.

Validation:

• npm run test -- test/runtime/core/windows-cmd-launch.test.ts test/runtime/terminal/pty-session.test.ts
• npm run typecheck
• npx @biomejs/biome check src/core/windows-cmd-launch.ts src/terminal/pty-session.ts test/runtime/core/windows-cmd-launch.test.ts test/runtime/terminal/pty-session.test.ts