refactor: extract resolveOverride for consistent setting/env path resolution

Add CODER_SSH_LOG_DIR env var fallback for proxy log directory, apply
expandPath consistently to both settings and env vars in getBinaryCachePath
and getProxyLogPath, and inline the removed getLogPath method.

Author: EhabY

package.json

@@ -102,7 +102,7 @@
 					"default": ""
 				},
 				"coder.proxyLogDirectory": {
-					"markdownDescription": "Directory where the Coder CLI outputs SSH connection logs for debugging. When not configured, logs are stored in the extension's global storage directory.",
+					"markdownDescription": "Directory where the Coder CLI outputs SSH connection logs for debugging. Defaults to the value of `CODER_SSH_LOG_DIR` if not set, otherwise the extension's global storage directory.",
 					"type": "string",
 					"default": ""
 				},

src/core/pathResolver.ts

@@ -32,15 +32,12 @@ export class PathResolver {
 	 * The caller must ensure this directory exists before use.
 	 */
 	public getBinaryCachePath(safeHostname: string): string {
-		const settingPath = vscode.workspace
-			.getConfiguration()
-			.get<string>("coder.binaryDestination")
-			?.trim();
-		const binaryPath =
-			settingPath || process.env.CODER_BINARY_DESTINATION?.trim();
-		return binaryPath
-			? path.normalize(binaryPath)
-			: path.join(this.getGlobalConfigDir(safeHostname), "bin");
+		return (
+			PathResolver.resolveOverride(
+				"coder.binaryDestination",
+				"CODER_BINARY_DESTINATION",
+			) || path.join(this.getGlobalConfigDir(safeHostname), "bin")
+		);
 	}
 
 	/**
@@ -53,26 +50,19 @@ export class PathResolver {
 	}
 
 	/**
-	 * Return the default path where log data from the connection is stored.
+	 * Return the proxy log directory from the `coder.proxyLogDirectory` setting
+	 * or the `CODER_SSH_LOG_DIR` environment variable, falling back to the `log`
+	 * subdirectory inside the extension's global storage path.
 	 *
 	 * The CLI will write files here named after the process PID.
 	 */
-	public getLogPath(): string {
-		return path.join(this.basePath, "log");
-	}
-
-	/**
-	 * Return the proxy log directory from user settings, falling back to the
-	 * default log path in extension storage.
-	 */
 	public getProxyLogPath(): string {
-		const configured = expandPath(
-			String(
-				vscode.workspace.getConfiguration().get("coder.proxyLogDirectory") ??
-					"",
-			).trim(),
+		return (
+			PathResolver.resolveOverride(
+				"coder.proxyLogDirectory",
+				"CODER_SSH_LOG_DIR",
+			) || path.join(this.basePath, "log")
 		);
-		return configured || this.getLogPath();
 	}
 
 	/**
@@ -131,4 +121,18 @@ export class PathResolver {
 	public getCodeLogDir(): string {
 		return this.codeLogPath;
 	}
+
+	/**
+	 * Read a path from a VS Code setting then an environment variable, returning
+	 * the first non-empty value after trimming, tilde/variable expansion, and
+	 * normalization. Returns an empty string when neither source provides a path.
+	 */
+	private static resolveOverride(setting: string, envVar: string): string {
+		const fromSetting = expandPath(
+			vscode.workspace.getConfiguration().get<string>(setting)?.trim() ?? "",
+		);
+		const resolved =
+			fromSetting || expandPath(process.env[envVar]?.trim() ?? "");
+		return resolved ? path.normalize(resolved) : "";
+	}
 }

src/remote/remote.ts

@@ -700,9 +700,8 @@ export class Remote {
 	}
 
 	/**
-	 * Return the --log-dir argument value for the ProxyCommand. It may be an
-	 * empty string if the CLI does not support it. Falls back to extension
-	 * storage when the user setting is not configured.
+	 * Return the --log-dir argument value for the ProxyCommand, or an empty
+	 * string when the CLI does not support it.
 	 *
 	 * Value defined in the "coder.sshFlags" setting is not considered.
 	 */

test/unit/core/pathResolver.test.ts

@@ -46,8 +46,36 @@ describe("PathResolver", () => {
 			},
 		);
 
-		it("should expand tilde in configured path", () => {
+		it("should expand tilde and ${userHome} in configured path", () => {
 			mockConfig.set("coder.proxyLogDirectory", "~/logs");
+			expect(pathResolver.getProxyLogPath()).not.toContain("~");
+
+			mockConfig.set("coder.proxyLogDirectory", "${userHome}/logs");
+			expect(pathResolver.getProxyLogPath()).not.toContain("${userHome}");
+		});
+
+		it("should normalize configured path", () => {
+			mockConfig.set("coder.proxyLogDirectory", "/custom/../log/./dir");
+			expectPathsEqual(pathResolver.getProxyLogPath(), "/log/dir");
+		});
+
+		it("should use CODER_SSH_LOG_DIR environment variable with proper precedence", () => {
+			// Use the global storage when the environment variable and setting are unset/blank
+			vi.stubEnv("CODER_SSH_LOG_DIR", "");
+			mockConfig.set("coder.proxyLogDirectory", "");
+			expectPathsEqual(pathResolver.getProxyLogPath(), defaultLogPath);
+
+			// Test environment variable takes precedence over global storage
+			vi.stubEnv("CODER_SSH_LOG_DIR", "   /env/log/path   ");
+			expectPathsEqual(pathResolver.getProxyLogPath(), "/env/log/path");
+
+			// Test setting takes precedence over environment variable
+			mockConfig.set("coder.proxyLogDirectory", "  /setting/log/path  ");
+			expectPathsEqual(pathResolver.getProxyLogPath(), "/setting/log/path");
+		});
+
+		it("should expand tilde in CODER_SSH_LOG_DIR", () => {
+			vi.stubEnv("CODER_SSH_LOG_DIR", "~/logs");
 			const result = pathResolver.getProxyLogPath();
 			expect(result).not.toContain("~");
 			expect(result).toContain("logs");
@@ -80,6 +108,20 @@ describe("PathResolver", () => {
 			);
 		});
 
+		it("should expand tilde in configured path", () => {
+			mockConfig.set("coder.binaryDestination", "~/bin");
+			const result = pathResolver.getBinaryCachePath("deployment");
+			expect(result).not.toContain("~");
+			expect(result).toContain("bin");
+		});
+
+		it("should expand tilde in CODER_BINARY_DESTINATION", () => {
+			vi.stubEnv("CODER_BINARY_DESTINATION", "~/bin");
+			const result = pathResolver.getBinaryCachePath("deployment");
+			expect(result).not.toContain("~");
+			expect(result).toContain("bin");
+		});
+
 		it("should use CODER_BINARY_DESTINATION environment variable with proper precedence", () => {
 			// Use the global storage when the environment variable and setting are unset/blank
 			vi.stubEnv("CODER_BINARY_DESTINATION", "");