fix(windows): resolve and execute CLI shims safely

[FabricioCasali]

Summary

• make executable lookup follow Windows PATHEXT, using F_OK instead of the meaningless Windows X_OK check
• execute npm .cmd shims through %SystemRoot%\System32\cmd.exe /d /s /v:off /c without enabling shell: true
• preserve structured argv through the two cmd.exe parses used by npm shims that forward %*, with delayed expansion disabled
• keep stdin as a direct byte stream instead of routing it through PowerShell's text pipeline
• route direct Claude, Codex, Grok, commit-agent, context-link, account-daemon, usage, and GitHub CLI subprocesses through the invocation helper, including its hidden/verbatim spawn options
• fail closed for .bat, .ps1, CR/LF/NUL in argv, missing cmd.exe, and command lines above the conservative 8,100-character limit
• add real Windows .cmd shim fixtures and include them in the quality-windows test set

Why

Windows CLI discovery previously searched only for extensionless names. Tools installed as claude.exe, gh.exe, or npm-generated .cmd shims were therefore invisible, so capability probes returned unknown values and agent nodes were created without a resumable session id.

Resolving PATHEXT fixes discovery, but Node cannot execute a .cmd file directly. shell: true would make prompts and other user-controlled arguments executable shell syntax. The first revision used npm's sibling .ps1 shim; measuring its real $input | & node ... path on Windows PowerShell 5.1 showed that it corrupts Unicode and line endings on stdin. The implementation now invokes the .cmd shim through a hidden cmd.exe, escapes every argv element for both parser passes, disables delayed expansion, and leaves stdin connected directly to the child.

The delivery tests cover empty arguments, spaces, quotes, trailing backslashes, backslashes before quotes, &|<>()@^, %PATH%, !delayed!, Unicode, a shim path containing & and !, a hostile commit prompt, and byte-exact multiline Unicode stdin.

Testing

Executed on Windows:

• npm run typecheck
• npm run build
• npm run host:build
• exact quality-windows Vitest command: 36 files passed, 552 tests passed, 7 skipped
• focused executable/consumer suites: 8 files passed, 62 tests passed, 1 skipped
• final real-shim matrix after adding the special-character path: 19 tests passed
• git diff --check
• real-device npm-installed Claude resume flow: passed

The first Windows gate run hit one timeout in src/core/pty-session-host.test.ts; that test passed in isolation and the complete gate passed on the immediate repeat.

GitHub Actions created CI and Security runs for commit a7e70a26; both are action_required and await maintainer approval. The Windows results above are local, not CI results.

Surfaces

• Desktop: fixes native Windows CLI discovery and execution.
• Server Edition: uses the same core helper; POSIX behavior remains unchanged.
• Mobile: not applicable because this does not change the transport protocol or mobile UI.

Fixes #559

[eneskirca]

Reviewed on an isolated worktree at b3d8eb4a, merged with current main. This is the right shape for the decision left open in #559, and it is a better answer than either option in that thread: it is not a shell:true carve-out, and it does not need to understand npm's shim internals. Full suite on the merge is green (761 files / 10330 tests), typecheck green, fs-atomic.guard, both no-electron guards and line-endings.guard green.

What I could verify, and did

PowerShell 7.6.5 on Linux (no Windows here, so read these as evidence about PowerShell's parser, not about the Windows delivery path):

• -File arguments are literal. $(Get-Date), a & b, has "quote", '', trail\, an embedded newline and -NotAParam all arrive in $args byte-for-byte, count 8. No expression evaluation, no shell operators. Your safety argument holds.
• $args splatted to a native process (npm's & node cli.js $args shape) preserves argv exactly, empty string included.
• A two-round-trip JSON-RPC exchange streams through an npm-shaped $input | & node … shim without deadlocking — so codex app-server is not structurally blocked by the extra process.

context-link.windows.test.ts passing 'session & untouched' through a real .ps1 is the right kind of test, and git-service.windows.test.ts asserting recorded.path is untouched is a nice catch. I also checked claude-accounts-service.ts, the one remaining findInLoginPath + execFileP site you did not touch: it returns early on process.platform !== 'darwin', so it is genuinely not a gap.

Blocking

1. The GIT_ENV hunk is a stale re-fix and conflicts with main. Your branch predates #630, which extracted src/core/git-env.ts and already omits the PATH key on win32 (issue #583) — main reads const GIT_ENV = gitEnv(). Drop the hunk on rebase and keep main's line; the PR-body bullet "preserve the inherited Windows PATH for GitHub CLI processes" is already true without this PR. This is the only real conflict (CLAUDE.md is an adjacent-bullet conflict, keep both).

2. windowsHide is missing on every new PowerShell spawn. Node's default is false, and every other Windows-facing spawn in this repo sets it — session-host-launcher.ts, session-host/process-tree.ts, bootstrap-windows, ssh-setup-runner.ts, and commit-message.ts's own spawn, which you kept. claude-cli.ts, grok-cli.ts, codex-identity-caps.ts, usage/codex-usage.ts, main/codex-accounts.ts, context-link.ts and git-service.ts now start powershell.exe from a windowless GUI process, once per probe. That is a console flash per probe on Windows. One option each.

Please measure or narrow

3. The stdin leg is untested, and it is the default commit-message path. planAgent only puts the prompt in argv on the custom {prompt} path; for claude and codex it goes on stdin (commit-message.ts:163,168,172). npm's real .ps1 relays stdin as $input | & node …, i.e. through PowerShell's pipeline. Measured here on pwsh 7: content survives, but CRLF is normalized to LF and a trailing newline is appended. On Windows PowerShell 5.1 $OutputEncoding defaults to ASCII (documented; I cannot measure it here), which would replace every non-ASCII byte of the prompt — and the prompt is a staged diff. codex-usage.windows.test.ts does not cover this: its fixture reads [Console]::In directly, deliberately bypassing the shape npm actually ships. Please add a fixture whose .ps1 uses $input | & node … and assert byte fidelity of a non-ASCII, no-trailing-newline prompt — and say in the PR what the answer was.

4. The argv case that motivates the whole design has no delivery test. The {prompt} path puts a whole user prompt into argv, and Windows PowerShell 5.1's argument quoting is the classic breakage area. The existing coverage is a builder assertion ('value & untouched' in exec-path.test.ts) plus a short session id in context-link.windows.test.ts. One commit-message Windows fixture over a hostile prompt — spaces, ", an embedded newline, %PATH%, a trailing backslash, and an empty string — would turn the safety claim into a measurement.

5. CI has never run on this branch. Zero workflow runs on fix/559-windows-cmd-shims (for comparison, #623's branch has four), and there is no action_required run to approve. Your three new fixtures are describe.skipIf(process.platform !== 'win32'), so quality-windows is the only place they execute — the PR's central evidence is currently unverified by CI. Worth re-triggering.

Small

6. ghInvocation([]) as an availability probe reads oddly — it builds a full invocation to answer a boolean, and on Windows costs two fs.existsSync calls per status(). A named ghRunnable() would say what it means. The semantics are right, though: on a .cmd-only gh, ghAvailable becoming false is the honest answer.

7. -ExecutionPolicy Bypass is the correct call (npm and npx do the same, and the .ps1 is the sibling of a .cmd that PATH resolution already chose), but a machine/GPO ExecutionPolicy overrides the parameter, and a locked-down corporate Windows will hit that. The degrade is fine — probes fall back to unknown, commit-message surfaces extractError — but it belongs in the CLAUDE.md bullet, next to a line saying this deliberately does not fall back to pwsh.

Ordering

#623 also rewrites this region of git-service.ts (it moves ghPath() into src/core/gh-path.ts). It merges clean with main today and you need a rebase anyway, so it should land first: your ghInvocation then stays where it is and only swaps its import to ./gh-path. I composed both locally — one three-line resolution, typecheck and tests green.

On #559

This answers the decision that issue was held open for, for both consumers enes named. It does not close it yet: no Windows run on an npm (.cmd) install has confirmed the --resume id comes back — ezwep's verified reboot run was on the native .exe installer, and this PR reports typecheck/build/vitest but no device repro. That run plus items 1–4 is what closes it.

[FabricioCasali]

Thanks for the detailed review. I rebased onto current main and pushed the follow-up as 2be60ce5.

The measurements changed the implementation rather than merely adding coverage:

1. The stale GIT_ENV re-fix is gone. git-service.ts keeps current main's gitEnv() and the extracted ghPath().
2. The PowerShell path was removed entirely. Every .cmd invocation now goes through %SystemRoot%\System32\cmd.exe /d /s /v:off /c with windowsVerbatimArguments: true and windowsHide: true; all consumers propagate those options.
3. A real npm-shaped .ps1 fixture ($input | & node ...) failed byte-fidelity assertions under Windows PowerShell 5.1: Unicode and line endings were not preserved. That is why stdin now remains connected directly to the .cmd child. The replacement fixture passes multiline Unicode input byte-for-byte with no added trailing newline.
4. The argv delivery tests now run through real npm-shaped .cmd shims and cover empty args, spaces, quotes, trailing backslashes, backslashes before quotes, &|<>()@^, %PATH%, !delayed!, Unicode, and a shim path containing & and !. commit-message.windows also delivers a hostile custom prompt through the real shim rather than only asserting the builder output.
5. The branch update created CI and Security runs, but both are currently action_required and need maintainer approval:
   • CI: https://github.com/eneskirca/nodeterm/actions/runs/33907625718
   • Security: https://github.com/eneskirca/nodeterm/actions/runs/33907625756

Local Windows verification at 2be60ce5:

• npm run typecheck
• npm run build
• npm run host:build
• exact quality-windows command: 36 files passed, 552 tests passed, 7 skipped
• focused executable/consumer suites: 8 files passed, 62 tests passed, 1 skipped
• final real-shim matrix: 19/19 passed
• git diff --check

The first Windows gate run had one timeout in pty-session-host.test.ts; it passed in isolation and the full gate passed on the immediate repeat.

The ExecutionPolicy/GPO caveat no longer applies because the implementation no longer launches PowerShell. I left the non-blocking ghInvocation([]) naming cleanup unchanged to keep this follow-up focused on the execution boundary.

One gap remains explicit: this machine's installed Claude is the native claude.exe, not an npm .cmd install, so I have not claimed the final real-device npm --resume reproduction. The npm-shaped delivery boundary is now exercised on Windows, but that end-to-end CLI run still needs an npm-installed Claude environment.

[FabricioCasali]

Follow-up device validation completed: the end-to-end resume flow with an npm-installed Claude CLI passed on Windows. I have added that result to the PR testing notes. The fix remains published at 2be60ce5.

[FabricioCasali]

Implementation complete at a7e70a26. The final non-blocking review note is addressed: ghRunnable() now names the GitHub CLI availability probe at all three call sites without changing its fail-closed semantics. Focused Windows tests passed (20/20), typecheck passed, and git diff --check passed. The PR now uses Fixes #559 because the npm-installed Claude resume flow was also verified successfully on the Windows device.