fix: avoid spurious 'restart terminal' prompts on every window reload (#1648)

* fix: avoid spurious 'restart terminal' prompts on window reload

VS Code shows a 'restart terminal to apply environment variable changes'
prompt whenever an extension's EnvironmentVariableCollection differs from
what is already applied to running terminals. The previous No-Config
Debug activation flow called collection.clear() followed by replace() /
append() unconditionally, which VS Code always interprets as a change
even when the resulting values are byte-identical, so the prompt fired
on every window reload.

Refactor the activation to diff-aware helpers (applyReplaceIfChanged,
applyAppendIfChanged, deleteIfPresent) that only write to the collection
when the existing mutator's type, value, or options actually differ from
what we want. The hot path on a normal reload becomes a sequence of
no-op get() comparisons, so VS Code never observes a change and the
prompt no longer appears.

Additional notes:
* Compare normalized EnvironmentVariableMutatorOptions
  (applyAtProcessCreation / applyAtShellIntegration) so future options
  changes still trigger a write.
* When the Java Language Server cannot resolve a Java home, keep the
  previously stored VSCODE_JAVA_EXEC instead of deleting it, to avoid
  churn caused by transient startup failures.
* Explicitly clean up legacy keys (currently JAVA_TOOL_OPTIONS) that
  older versions of this extension used to set but no longer manage.
* Set a human-readable description on the collection so users can see
  the source in VS Code's environment variable UI.

Fixes #1647

* fix: drop unused legacy env var cleanup per review feedback

JAVA_TOOL_OPTIONS has never been written to the per-extension EnvironmentVariableCollection by this codebase (verified via 'git log -S JAVA_TOOL_OPTIONS'). The legacy cleanup loop was therefore dead code, and could be misread as touching user-managed env vars (it cannot — the collection is per-extension and only sees mutators this extension wrote).

* refactor: drop unused deleteIfPresent helper

Production code no longer calls deleteIfPresent after the legacy cleanup loop was removed. Drop the helper and its tests rather than carry a YAGNI export — easy to reintroduce if a real caller appears.

Author: wenytang-ms

src/envVarSync.ts

@@ -0,0 +1,95 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license.
+
+import * as vscode from "vscode";
+
+/**
+ * Helpers that update a {@link vscode.EnvironmentVariableCollection} only when
+ * the resulting mutation would actually change the collection.
+ *
+ * Background: VS Code shows a "Restart terminal to apply environment variable
+ * changes" prompt whenever an extension's collection differs from what has
+ * already been applied to running terminals. Calling `replace()` / `append()`
+ * with the same value, or calling `clear()` followed by re-adding identical
+ * entries, still counts as a change and re-triggers the prompt on every
+ * window reload. See issue #1647.
+ *
+ * These helpers compare the existing mutator (type + value + options) against
+ * the desired one and skip the write entirely when they match.
+ */
+
+const DEFAULT_OPTIONS: Required<vscode.EnvironmentVariableMutatorOptions> = {
+    applyAtProcessCreation: true,
+    applyAtShellIntegration: false,
+};
+
+function normalizeOptions(
+    options?: vscode.EnvironmentVariableMutatorOptions,
+): Required<vscode.EnvironmentVariableMutatorOptions> {
+    return { ...DEFAULT_OPTIONS, ...options };
+}
+
+function sameOptions(
+    existing: vscode.EnvironmentVariableMutator,
+    desired?: vscode.EnvironmentVariableMutatorOptions,
+): boolean {
+    const e = normalizeOptions(existing.options);
+    const d = normalizeOptions(desired);
+    return e.applyAtProcessCreation === d.applyAtProcessCreation
+        && e.applyAtShellIntegration === d.applyAtShellIntegration;
+}
+
+/**
+ * Calls `collection.replace(variable, value, options)` only when the existing
+ * mutator (if any) does not already match.
+ *
+ * @returns `true` if the collection was actually written to.
+ */
+export function applyReplaceIfChanged(
+    collection: vscode.EnvironmentVariableCollection,
+    variable: string,
+    value: string,
+    options?: vscode.EnvironmentVariableMutatorOptions,
+): boolean {
+    const existing = collection.get(variable);
+    if (existing
+        && existing.type === vscode.EnvironmentVariableMutatorType.Replace
+        && existing.value === value
+        && sameOptions(existing, options)) {
+        return false;
+    }
+    if (options) {
+        collection.replace(variable, value, options);
+    } else {
+        collection.replace(variable, value);
+    }
+    return true;
+}
+
+/**
+ * Calls `collection.append(variable, value, options)` only when the existing
+ * mutator (if any) does not already match.
+ *
+ * @returns `true` if the collection was actually written to.
+ */
+export function applyAppendIfChanged(
+    collection: vscode.EnvironmentVariableCollection,
+    variable: string,
+    value: string,
+    options?: vscode.EnvironmentVariableMutatorOptions,
+): boolean {
+    const existing = collection.get(variable);
+    if (existing
+        && existing.type === vscode.EnvironmentVariableMutatorType.Append
+        && existing.value === value
+        && sameOptions(existing, options)) {
+        return false;
+    }
+    if (options) {
+        collection.append(variable, value, options);
+    } else {
+        collection.append(variable, value);
+    }
+    return true;
+}
+

src/noConfigDebugInit.ts

@@ -9,6 +9,9 @@ import * as vscode from 'vscode';
 import { sendInfo, sendError } from "vscode-extension-telemetry-wrapper";
 import { getJavaHome } from "./utility";
 import { buildNoConfigPathAppendValue } from "./pathUtil";
+import { applyAppendIfChanged, applyReplaceIfChanged } from "./envVarSync";
+
+const ENV_VAR_COLLECTION_DESCRIPTION = "Java No-Config Debug";
 
 /**
  * Registers the configuration-less debugging setup for the extension.
@@ -21,7 +24,7 @@ import { buildNoConfigPathAppendValue } from "./pathUtil";
  *
  * Environment Variables:
  * - `VSCODE_JDWP_ADAPTER_ENDPOINTS`: Path to the file containing the debugger adapter endpoint.
- * - `JAVA_TOOL_OPTIONS`: JDWP configuration for automatic debugging.
+ * - `VSCODE_JAVA_EXEC`: Path to the java executable from the Java Language Server (when available).
  * - `PATH`: Appends the path to the noConfigScripts directory.
  */
 export async function registerNoConfigDebug(
@@ -69,30 +72,39 @@ export async function registerNoConfigDebug(
         }
     }
 
-    // clear the env var collection to remove any existing env vars
-    collection.clear();
+    // Surface a description in VS Code's environment variable UI so users can
+    // see which extension is contributing these variables.
+    if (collection.description !== ENV_VAR_COLLECTION_DESCRIPTION) {
+        collection.description = ENV_VAR_COLLECTION_DESCRIPTION;
+    }
 
-    // Add env var for VSCODE_JDWP_ADAPTER_ENDPOINTS
+    // Apply our managed variables using diff-aware helpers. On a typical
+    // window reload the values are unchanged and these calls are no-ops, so
+    // VS Code does not prompt the user to restart their existing terminals.
+    // See issue #1647.
+    //
     // Note: We do NOT set JAVA_TOOL_OPTIONS globally to avoid affecting all Java processes
     // (javac, maven, gradle, language server, etc.). Instead, JAVA_TOOL_OPTIONS is set
     // only in the debugjava wrapper scripts (debugjava.ps1, debugjava.bat, debugjava)
-    collection.replace('VSCODE_JDWP_ADAPTER_ENDPOINTS', tempFilePath);
+    applyReplaceIfChanged(collection, 'VSCODE_JDWP_ADAPTER_ENDPOINTS', tempFilePath);
 
     // Try to get Java executable from Java Language Server
-    // This ensures we use the same Java version as the project is compiled with
+    // This ensures we use the same Java version as the project is compiled with.
+    // If detection fails or returns nothing, we deliberately keep any previously
+    // set VSCODE_JAVA_EXEC to avoid churn from transient startup failures.
     try {
         const javaHome = await getJavaHome();
         if (javaHome) {
             const javaExec = path.join(javaHome, 'bin', 'java');
-            collection.replace('VSCODE_JAVA_EXEC', javaExec);
+            applyReplaceIfChanged(collection, 'VSCODE_JAVA_EXEC', javaExec);
         }
     } catch (error) {
         // If we can't get Java from Language Server, that's okay
         // The wrapper script will fall back to JAVA_HOME or PATH
     }
 
     const noConfigScriptsDir = path.join(extPath, 'bundled', 'scripts', 'noConfigScripts');
-    collection.append('PATH', buildNoConfigPathAppendValue(noConfigScriptsDir));
+    applyAppendIfChanged(collection, 'PATH', buildNoConfigPathAppendValue(noConfigScriptsDir));
 
     // create file system watcher for the debuggerAdapterEndpointFolder for when the communication port is written
     const fileSystemWatcher = vscode.workspace.createFileSystemWatcher(

test/envVarSync.test.ts

@@ -0,0 +1,213 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license.
+
+import * as assert from "assert";
+import * as vscode from "vscode";
+
+import {
+    applyAppendIfChanged,
+    applyReplaceIfChanged,
+} from "../src/envVarSync";
+
+interface FakeMutator {
+    type: vscode.EnvironmentVariableMutatorType;
+    value: string;
+    options: vscode.EnvironmentVariableMutatorOptions;
+}
+
+interface FakeCollection extends vscode.EnvironmentVariableCollection {
+    __calls: { replace: number; append: number; delete: number };
+}
+
+function createFakeCollection(): FakeCollection {
+    const store = new Map<string, FakeMutator>();
+    const calls = { replace: 0, append: 0, delete: 0 };
+
+    const collection = {
+        persistent: true,
+        description: undefined as string | vscode.MarkdownString | undefined,
+        get(name: string): FakeMutator | undefined {
+            return store.get(name);
+        },
+        replace(name: string, value: string, options?: vscode.EnvironmentVariableMutatorOptions): void {
+            calls.replace += 1;
+            store.set(name, {
+                type: vscode.EnvironmentVariableMutatorType.Replace,
+                value,
+                options: { applyAtProcessCreation: true, applyAtShellIntegration: false, ...options },
+            });
+        },
+        append(name: string, value: string, options?: vscode.EnvironmentVariableMutatorOptions): void {
+            calls.append += 1;
+            store.set(name, {
+                type: vscode.EnvironmentVariableMutatorType.Append,
+                value,
+                options: { applyAtProcessCreation: true, applyAtShellIntegration: false, ...options },
+            });
+        },
+        prepend(name: string, value: string, options?: vscode.EnvironmentVariableMutatorOptions): void {
+            store.set(name, {
+                type: vscode.EnvironmentVariableMutatorType.Prepend,
+                value,
+                options: { applyAtProcessCreation: true, applyAtShellIntegration: false, ...options },
+            });
+        },
+        delete(name: string): void {
+            calls.delete += 1;
+            store.delete(name);
+        },
+        clear(): void {
+            store.clear();
+        },
+        forEach(callback: (variable: string, mutator: FakeMutator, collection: any) => void): void {
+            store.forEach((mutator, variable) => callback(variable, mutator, collection));
+        },
+        getScoped(): vscode.EnvironmentVariableCollection {
+            return collection as unknown as vscode.EnvironmentVariableCollection;
+        },
+        *[Symbol.iterator](): IterableIterator<[string, FakeMutator]> {
+            yield* store.entries();
+        },
+        __calls: calls,
+    };
+
+    return collection as unknown as FakeCollection;
+}
+
+suite("envVarSync", () => {
+    suite("applyReplaceIfChanged", () => {
+        test("writes when variable is missing", () => {
+            const c = createFakeCollection();
+            const changed = applyReplaceIfChanged(c, "FOO", "bar");
+            assert.strictEqual(changed, true);
+            assert.strictEqual(c.__calls.replace, 1);
+            assert.strictEqual(c.get("FOO")!.value, "bar");
+            assert.strictEqual(c.get("FOO")!.type, vscode.EnvironmentVariableMutatorType.Replace);
+        });
+
+        test("is a no-op when value and type already match (default options)", () => {
+            const c = createFakeCollection();
+            applyReplaceIfChanged(c, "FOO", "bar");
+            c.__calls.replace = 0;
+
+            const changed = applyReplaceIfChanged(c, "FOO", "bar");
+
+            assert.strictEqual(changed, false);
+            assert.strictEqual(c.__calls.replace, 0);
+        });
+
+        test("writes when value differs", () => {
+            const c = createFakeCollection();
+            applyReplaceIfChanged(c, "FOO", "bar");
+            c.__calls.replace = 0;
+
+            const changed = applyReplaceIfChanged(c, "FOO", "baz");
+
+            assert.strictEqual(changed, true);
+            assert.strictEqual(c.__calls.replace, 1);
+            assert.strictEqual(c.get("FOO")!.value, "baz");
+        });
+
+        test("overrides an existing Append mutator", () => {
+            const c = createFakeCollection();
+            applyAppendIfChanged(c, "FOO", "bar");
+            c.__calls.replace = 0;
+
+            const changed = applyReplaceIfChanged(c, "FOO", "bar");
+
+            assert.strictEqual(changed, true);
+            assert.strictEqual(c.__calls.replace, 1);
+            assert.strictEqual(c.get("FOO")!.type, vscode.EnvironmentVariableMutatorType.Replace);
+        });
+
+        test("writes when options differ from existing mutator", () => {
+            const c = createFakeCollection();
+            applyReplaceIfChanged(c, "FOO", "bar", { applyAtProcessCreation: true, applyAtShellIntegration: false });
+            c.__calls.replace = 0;
+
+            const changed = applyReplaceIfChanged(c, "FOO", "bar", { applyAtProcessCreation: false, applyAtShellIntegration: false });
+
+            assert.strictEqual(changed, true);
+            assert.strictEqual(c.__calls.replace, 1);
+        });
+
+        test("treats omitted options as the documented defaults", () => {
+            const c = createFakeCollection();
+            applyReplaceIfChanged(c, "FOO", "bar", { applyAtProcessCreation: true, applyAtShellIntegration: false });
+            c.__calls.replace = 0;
+
+            const changed = applyReplaceIfChanged(c, "FOO", "bar");
+
+            assert.strictEqual(changed, false);
+            assert.strictEqual(c.__calls.replace, 0);
+        });
+    });
+
+    suite("applyAppendIfChanged", () => {
+        test("writes when variable is missing", () => {
+            const c = createFakeCollection();
+            const changed = applyAppendIfChanged(c, "PATH", ";C:\\extra");
+            assert.strictEqual(changed, true);
+            assert.strictEqual(c.__calls.append, 1);
+            assert.strictEqual(c.get("PATH")!.value, ";C:\\extra");
+            assert.strictEqual(c.get("PATH")!.type, vscode.EnvironmentVariableMutatorType.Append);
+        });
+
+        test("is a no-op when value already matches", () => {
+            const c = createFakeCollection();
+            applyAppendIfChanged(c, "PATH", ";C:\\extra");
+            c.__calls.append = 0;
+
+            const changed = applyAppendIfChanged(c, "PATH", ";C:\\extra");
+
+            assert.strictEqual(changed, false);
+            assert.strictEqual(c.__calls.append, 0);
+        });
+
+        test("writes when value differs", () => {
+            const c = createFakeCollection();
+            applyAppendIfChanged(c, "PATH", ";C:\\extra");
+            c.__calls.append = 0;
+
+            const changed = applyAppendIfChanged(c, "PATH", ";C:\\other");
+
+            assert.strictEqual(changed, true);
+            assert.strictEqual(c.__calls.append, 1);
+            assert.strictEqual(c.get("PATH")!.value, ";C:\\other");
+        });
+
+        test("overrides an existing Replace mutator on the same variable", () => {
+            const c = createFakeCollection();
+            applyReplaceIfChanged(c, "PATH", ";C:\\extra");
+            c.__calls.append = 0;
+
+            const changed = applyAppendIfChanged(c, "PATH", ";C:\\extra");
+
+            assert.strictEqual(changed, true);
+            assert.strictEqual(c.__calls.append, 1);
+            assert.strictEqual(c.get("PATH")!.type, vscode.EnvironmentVariableMutatorType.Append);
+        });
+    });
+
+    suite("regression: issue #1647 - repeated activations stay quiet", () => {
+        test("re-applying the same set of variables does not touch the collection", () => {
+            const c = createFakeCollection();
+
+            // Simulate first activation.
+            applyReplaceIfChanged(c, "VSCODE_JDWP_ADAPTER_ENDPOINTS", "/tmp/endpoint-abc.txt");
+            applyReplaceIfChanged(c, "VSCODE_JAVA_EXEC", "/opt/jdk/bin/java");
+            applyAppendIfChanged(c, "PATH", ":/ext/bundled/scripts/noConfigScripts");
+
+            const callsAfterFirst = { ...c.__calls };
+            assert.deepStrictEqual(callsAfterFirst, { replace: 2, append: 1, delete: 0 });
+
+            // Simulate a window reload: same values, same order.
+            applyReplaceIfChanged(c, "VSCODE_JDWP_ADAPTER_ENDPOINTS", "/tmp/endpoint-abc.txt");
+            applyReplaceIfChanged(c, "VSCODE_JAVA_EXEC", "/opt/jdk/bin/java");
+            applyAppendIfChanged(c, "PATH", ":/ext/bundled/scripts/noConfigScripts");
+
+            // Nothing should have been written on the second pass.
+            assert.deepStrictEqual(c.__calls, callsAfterFirst);
+        });
+    });
+});