fix(lsp/process): prefer PATHEXT extensions over extensionless name (#4262)#5
Open
YOMXXX wants to merge 1 commit into
Open
fix(lsp/process): prefer PATHEXT extensions over extensionless name (#4262)#5YOMXXX wants to merge 1 commit into
YOMXXX wants to merge 1 commit into
Conversation
…n Windows (#4262) `getWindowsPathExtensions` returned `["", ...extensions, ".exe", ".cmd", ".bat"]` — with the empty string first. On Windows, when a wrapper-script-style LSP server ships both an extensionless shebang script and a `.bat`/`.cmd` companion (jdtls is the canonical case), `resolveWindowsCommand` matched the extensionless file first because it tried `""` before `.BAT`/`.CMD`. `isWindowsShellShim` then returned false (the resolved path doesn't end in `.bat`/`.cmd`), so Node spawned the extensionless Python script directly. Windows ignores shebangs, so the spawn failed with UV_ENOENT (-4058) and the LSP server never came up. Move the empty string to the end of the array so PATHEXT extensions get checked first — matching cmd.exe's lookup order. The wrapper wins, isWindowsShellShim returns true, and node spawns through `cmd.exe /d /s /c jdtls.bat ...`. Includes a regression test that drops both files (`jdtls` + `jdtls.bat`) into a tmp dir and asserts the resolver picks the wrapper.
Author
|
Cross-reference: this PR is part of a roadmap covering 12 PRs across both oh-my-openagent and this submodule. See the rollup at code-yeongyu/oh-my-openagent#4293 — it groups bug fixes, regression locks, and new features with a suggested review order. Direct link: code-yeongyu/oh-my-openagent#4293 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes code-yeongyu/oh-my-openagent#4262.
Root cause
getWindowsPathExtensionsreturned the candidate extension list with an empty string at the front:```ts
return [...new Set(["", ...extensions, ".exe", ".cmd", ".bat"])];
```
On Windows, when an LSP server ships both an extensionless shebang script and a `.bat`/`.cmd` companion — `jdtls` is the canonical case — `resolveWindowsCommand`'s inner loop tried the empty string first, matched the extensionless file via `existsSync`, and returned its full path.
`isWindowsShellShim` then returned `false` (the resolved path doesn't end with `.bat`/`.cmd`), so Node spawned the extensionless Python script directly. Windows can't honor shebangs, so spawn failed with `UV_ENOENT` (`-4058`) and the language server never came up.
Fix
Move the empty string to the end of the array so PATHEXT extensions get checked first — matching `cmd.exe`'s actual lookup order:
```ts
return [...new Set([...extensions, ".exe", ".cmd", ".bat", ""])];
```
Now `jdtls.bat` wins, `isWindowsShellShim` returns true, and the wrapper goes through `cmd.exe /d /s /c jdtls.bat ...` — which is what jdtls needs to set `JAVA_HOME` correctly.
Test plan
Summary by cubic
Prefer Windows
PATHEXTextensions over extensionless names when resolving LSP binaries. Ensuresjdtlsand similar servers run via their.bat/.cmdwrapper instead of the shebang script, avoiding UV_ENOENT.getWindowsPathExtensionsto try PATHEXT first and the empty string last, matchingcmd.exelookup; prevents spawning extensionless scripts.createSpawnCommand(["jdtls", "--stdio"], "win32", ...)resolves tocmd.exe /d /s /c jdtls.bat.Written for commit 4023034. Summary will update on new commits. Review in cubic