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

src/noConfigDebugInit.ts

@@ -8,6 +8,7 @@ import * as vscode from 'vscode';
 
 import { sendInfo, sendError } from "vscode-extension-telemetry-wrapper";
 import { getJavaHome } from "./utility";
+import { buildNoConfigPathAppendValue } from "./pathUtil";
 
 /**
  * Registers the configuration-less debugging setup for the extension.
@@ -91,14 +92,7 @@ export async function registerNoConfigDebug(
     }
 
     const noConfigScriptsDir = path.join(extPath, 'bundled', 'scripts', 'noConfigScripts');
-    const pathSeparator = process.platform === 'win32' ? ';' : ':';
-
-    // Check if the current PATH already ends with a path separator to avoid double separators
-    const currentPath = process.env.PATH || '';
-    const needsSeparator = currentPath.length > 0 && !currentPath.endsWith(pathSeparator);
-    const pathValueToAppend = needsSeparator ? `${pathSeparator}${noConfigScriptsDir}` : noConfigScriptsDir;
-
-    collection.append('PATH', pathValueToAppend);
+    collection.append('PATH', buildNoConfigPathAppendValue(noConfigScriptsDir));
 
     // create file system watcher for the debuggerAdapterEndpointFolder for when the communication port is written
     const fileSystemWatcher = vscode.workspace.createFileSystemWatcher(

src/pathUtil.ts

@@ -0,0 +1,33 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license.
+
+/**
+ * Builds the value to append to PATH for the noConfigScripts directory.
+ *
+ * `vscode.EnvironmentVariableCollection.append()` performs literal string
+ * concatenation and does NOT insert a path separator. We always prepend one so
+ * we never glue our directory onto the last entry of the user's PATH (e.g.
+ * `...;C:\Program Files\jreleaser\c:\Users\...\noConfigScripts`).
+ *
+ * We cannot rely on `process.env.PATH` ending with a separator: the integrated
+ * terminal's PATH may differ from the extension host's PATH (it can be modified
+ * by `terminal.integrated.env.*` or by other extensions' env-var collections).
+ *
+ * A leading separator is always safe: if the resolved PATH already ends with
+ * one, the resulting empty PATH entry is harmless on both Windows and POSIX
+ * shells.
+ *
+ * This module has no `vscode` import so it can be unit-tested in plain Node.
+ *
+ * @param scriptsDir absolute path to the noConfigScripts directory
+ * @param platform   the target platform; defaults to `process.platform`. Made
+ *                   injectable so unit tests can exercise both Windows and
+ *                   POSIX behavior on a single host.
+ */
+export function buildNoConfigPathAppendValue(
+    scriptsDir: string,
+    platform: NodeJS.Platform = process.platform,
+): string {
+    const pathSeparator = platform === 'win32' ? ';' : ':';
+    return `${pathSeparator}${scriptsDir}`;
+}

test/pathUtil.test.ts

@@ -0,0 +1,98 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license.
+
+import * as assert from "assert";
+
+import { buildNoConfigPathAppendValue } from "../src/pathUtil";
+
+// Regression tests for issue #1637: the extension was appending its
+// noConfigScripts directory to PATH without a separator on some terminal
+// PATH configurations, gluing it onto the last entry of the user's PATH.
+suite("buildNoConfigPathAppendValue", () => {
+
+    const winDir = "C:\\Users\\me\\.vscode\\extensions\\vscjava.vscode-java-debug-0.59.0\\bundled\\scripts\\noConfigScripts";
+    const posixDir = "/home/me/.vscode/extensions/vscjava.vscode-java-debug-0.59.0/bundled/scripts/noConfigScripts";
+
+    test("uses ';' as separator on Windows", () => {
+        const result = buildNoConfigPathAppendValue(winDir, "win32");
+        assert.strictEqual(result, `;${winDir}`);
+    });
+
+    test("uses ':' as separator on Linux", () => {
+        const result = buildNoConfigPathAppendValue(posixDir, "linux");
+        assert.strictEqual(result, `:${posixDir}`);
+    });
+
+    test("uses ':' as separator on macOS", () => {
+        const result = buildNoConfigPathAppendValue(posixDir, "darwin");
+        assert.strictEqual(result, `:${posixDir}`);
+    });
+
+    test("always starts with a path separator (Windows)", () => {
+        const result = buildNoConfigPathAppendValue(winDir, "win32");
+        assert.ok(result.startsWith(";"), `expected leading ';', got: ${result}`);
+    });
+
+    test("always starts with a path separator (POSIX)", () => {
+        const result = buildNoConfigPathAppendValue(posixDir, "linux");
+        assert.ok(result.startsWith(":"), `expected leading ':', got: ${result}`);
+    });
+
+    test("never collapses scriptsDir into the previous PATH entry on Windows", () => {
+        // Simulates the exact scenario from issue #1637: a user PATH whose
+        // last entry has no trailing separator. After append, the script dir
+        // must not be glued onto 'jreleaser\'.
+        const userPath = "C:\\foo;C:\\Program Files\\jreleaser\\";
+        const finalPath = userPath + buildNoConfigPathAppendValue(winDir, "win32");
+
+        const entries = finalPath.split(";");
+        assert.ok(
+            entries.includes("C:\\Program Files\\jreleaser\\"),
+            `expected 'jreleaser\\' to remain a standalone PATH entry, got entries: ${JSON.stringify(entries)}`,
+        );
+        assert.ok(
+            entries.includes(winDir),
+            `expected scripts dir to be a standalone PATH entry, got entries: ${JSON.stringify(entries)}`,
+        );
+    });
+
+    test("never collapses scriptsDir into the previous PATH entry on POSIX", () => {
+        const userPath = "/usr/bin:/opt/jreleaser/bin";
+        const finalPath = userPath + buildNoConfigPathAppendValue(posixDir, "linux");
+
+        const entries = finalPath.split(":");
+        assert.ok(
+            entries.includes("/opt/jreleaser/bin"),
+            `expected '/opt/jreleaser/bin' to remain a standalone PATH entry, got entries: ${JSON.stringify(entries)}`,
+        );
+        assert.ok(
+            entries.includes(posixDir),
+            `expected scripts dir to be a standalone PATH entry, got entries: ${JSON.stringify(entries)}`,
+        );
+    });
+
+    test("yields only an empty (harmless) entry when the user's PATH already ends with a separator", () => {
+        // If the resolved terminal PATH already ends with ';', append produces
+        // ';;'. The empty middle entry is ignored by Windows and (in this
+        // position) effectively a no-op on POSIX shells.
+        const userPath = "C:\\foo;C:\\bar;";
+        const finalPath = userPath + buildNoConfigPathAppendValue(winDir, "win32");
+
+        const entries = finalPath.split(";");
+        // The scripts dir must still be a standalone, valid entry.
+        assert.ok(
+            entries.includes(winDir),
+            `expected scripts dir to remain standalone, got entries: ${JSON.stringify(entries)}`,
+        );
+        // No real entry should be merged with our scripts dir.
+        assert.ok(
+            !entries.some((e) => e !== winDir && e.endsWith(winDir)),
+            `no entry should be glued to scripts dir, got entries: ${JSON.stringify(entries)}`,
+        );
+    });
+
+    test("scriptsDir appears unchanged at the end of the appended value", () => {
+        const result = buildNoConfigPathAppendValue(winDir, "win32");
+        assert.ok(result.endsWith(winDir), `expected value to end with scriptsDir, got: ${result}`);
+    });
+});