Skip to content

fix: always prepend separator when appending noConfigScripts to PATH (#1637)#1641

Open
wenytang-ms wants to merge 2 commits intomainfrom
fix/path-separator-noconfig
Open

fix: always prepend separator when appending noConfigScripts to PATH (#1637)#1641
wenytang-ms wants to merge 2 commits intomainfrom
fix/path-separator-noconfig

Conversation

@wenytang-ms
Copy link
Copy Markdown
Contributor

@wenytang-ms wenytang-ms commented May 7, 2026

Fixes #1637

Problem

vscode.EnvironmentVariableCollection.append('PATH', value) does literal string concatenation — it does not insert a path separator. The current code in src/noConfigDebugInit.ts tries to compensate by checking process.env.PATH:

const currentPath = process.env.PATH || '';
const needsSeparator = currentPath.length > 0 && !currentPath.endsWith(pathSeparator);
const pathValueToAppend = needsSeparator ? `` : noConfigScriptsDir;
collection.append('PATH', pathValueToAppend);

But process.env.PATH is the extension host's PATH, which is not necessarily the PATH used by the integrated terminal (it can be modified by terminal.integrated.env. or by other extensions' env-var collections). When the extension host PATH ends with ; but the resolved terminal PATH does not, needsSeparator is same, and the directory gets appended with no separator — collapsing it into the last entry of the user's PATH.

That is exactly what the user reports in #1637:

...;C:\Program Files\jreleaser\c:\Users\REDACTED\.vscode\extensions\vscjava.vscode-java-debug-0.59.0\bundled\scripts\noConfigScripts

Fix

Always prepend the separator. If the user's PATH already ends with one, the resulting double separator just creates an empty entry, which both Windows and POSIX shells handle safely.

Verification

  • npx tsc -p . --noEmit
  • npm run tslint

EnvironmentVariableCollection.append() performs a literal string concatenation
and does not insert a path separator. The previous heuristic checked
process.env.PATH on the extension host and skipped the separator when that PATH
already ended with one. However, the PATH used by the integrated terminal can
differ from the extension host's PATH, so this check could incorrectly drop the
separator and glue the noConfigScripts directory onto the last entry of the
user's PATH (e.g. C:\Program Files\jreleaser\c:\Users\...\noConfigScripts).

Always prepend the separator. A trailing empty PATH entry (if the user's PATH
already ended with one) is harmless on both Windows and POSIX shells.

Fixes #1637
@wenytang-ms wenytang-ms force-pushed the fix/path-separator-noconfig branch from bffed36 to bd3bc78 Compare May 7, 2026 08:46
Extract the PATH append-value computation into a vscode-free
src/pathUtil.ts module so it can be unit-tested in plain Node mocha.

Add test/pathUtil.test.ts covering:
- Correct separator on Windows (;), Linux (:), and macOS (:).
- The appended value always starts with a path separator (regression
  guard for #1637).
- The scripts directory remains a standalone PATH entry when the user's
  PATH last entry has no trailing separator (the exact scenario reported
  in #1637, e.g. C:\Program Files\jreleaser\\).
- A trailing separator on the user's PATH only produces a harmless empty
  entry, never glues another entry onto our scripts dir.
- The scripts directory is preserved verbatim at the end of the value.

All 9 tests pass under plain mocha; the file is also picked up by the
existing Electron-based test runner via the **/**.test.js glob.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

Extension appending to PATH without delimiter.

2 participants