Skip to content

fix(acp): resolve Windows ENOENT when spawning ACP agent process#400

Merged
zts212653 merged 4 commits intozts212653:mainfrom
Lygggggg:fix/acp-windows-enoent
Apr 10, 2026
Merged

fix(acp): resolve Windows ENOENT when spawning ACP agent process#400
zts212653 merged 4 commits intozts212653:mainfrom
Lygggggg:fix/acp-windows-enoent

Conversation

@Lygggggg
Copy link
Copy Markdown
Contributor

@Lygggggg Lygggggg commented Apr 9, 2026

AcpClient.initialize() used bare child_process.spawn() which cannot find npm-global .cmd shims on Windows. Reuse the same 3-tier fallback from cli-spawn.ts: resolve .cmd → underlying .js script, then Git Bash shell, then cmd.exe shell. Skipped when custom spawnFn is injected (test path unchanged).

PR Type

  • 🐛 Patch — Bug fix, typo, test gap (no Feature Doc needed)
  • Feature — New capability or behavior change (requires Feature Doc)
  • 📋 Protocol — Rules, skills, workflow changes (the doc IS the contribution)

Related Issue

Closes #401

What

  • Modified AcpClient.ts to add Windows .cmd shim resolution using the existing 3-tier fallback from cli-spawn-win.ts
  • Added regression test acp-client-windows-spawn.test.js covering the default spawn path on Windows

Why

On Windows, npm-global CLI tools like gemini are installed as .cmd shim scripts. child_process.spawn() without shell: true cannot locate .cmd files, causing ENOENT. The non-ACP path (cli-spawn.ts) already handled this; AcpClient did not.

Tradeoff

Could have used shell: true universally, but that introduces shell injection risk and is inconsistent with the existing strategy in cli-spawn-win.ts.

Test Evidence

$ pnpm check
✔ All checks passed

$ pnpm lint
✔ No type errors

$ node --test test/acp/acp-client-windows-spawn.test.js test/acp/acp-client.test.js
▶ AcpClient Windows default spawn path
  ✔ resolves .cmd shim to node + script.js when spawnFn is omitted (52ms)
▶ AcpClient Windows default spawn path (1 test, 1 pass)

▶ AcpClient
  ... (26 tests, 26 pass)

ℹ tests 27
ℹ suites 2
ℹ pass 27
ℹ fail 0

AcpClient.initialize() used bare child_process.spawn() which cannot
find npm-global .cmd shims on Windows. Reuse the same 3-tier fallback
from cli-spawn.ts: resolve .cmd → underlying .js script, then Git Bash
shell, then cmd.exe shell. Skipped when custom spawnFn is injected
(test path unchanged).

Fixes: spawn gemini ENOENT on Windows

[宪宪/Opus-46🐾]
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Lygggggg Lygggggg requested a review from zts212653 as a code owner April 9, 2026 11:12
Copy link
Copy Markdown
Owner

Thanks for the fix. I checked the ACP path against our current Windows spawn helpers, and the bug itself looks real: AcpClient was still using a bare child_process.spawn(...), while the non-ACP CLI path had already added Windows shim handling for .cmd wrappers.

The fix direction here also looks right. Reusing the same 3-step Windows fallback (resolve shim -> Git Bash -> cmd.exe) is consistent with our existing cli-spawn-win.ts logic.

That said, I don’t think we should merge this PR as-is yet:

  1. There is no open accepted issue linked to this PR. The historical Windows ENOENT discussion in #64 documents the same root cause, but that issue is already closed, and this PR body still leaves Closes # empty.
  2. This change touches a branch that is not currently covered by AcpClient tests: the Windows + no-custom-spawnFn path inside initialize().
  3. The PR also has no CI / test evidence attached yet, so the maintainer-side merge gate is still incomplete.

What I’d suggest before merge:

  • reopen #64 or open a small follow-up issue specifically for the ACP-side Windows ENOENT path, then link it here;
  • add one regression test that exercises the Windows ACP spawn branch in AcpClient.initialize();
  • fill in the test evidence / checks in the PR body.

Once those are in, this looks like a good candidate to land.

— 缅因猫-gpt5.4

zts212653 and others added 2 commits April 9, 2026 20:19
…2653#401)

Covers the default spawn path (no custom spawnFn) in AcpClient.initialize()
on Windows: verifies .cmd shim resolution rewrites spawn to node + script.js.

Co-Authored-By: 砚砚/Codex [砚砚/Codex🐾] <noreply@anthropic.com>
Co-Authored-By: 宪宪/Opus-46 [宪宪/Opus-46🐾] <noreply@anthropic.com>
Copy link
Copy Markdown
Owner

@zts212653 zts212653 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug 确认真实:AcpClient 在 Windows 下确实漏了 .cmd shim fallback。这轮已经补了回归测试,并且我补推了 lint fixup;等当前 CI 全绿后就可以合。

@zts212653 zts212653 merged commit 0735883 into zts212653:main Apr 10, 2026
5 checks passed
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.

fix(acp): AcpClient Windows ENOENT — .cmd shim not resolved

2 participants