From da5aaf2f4eb2d1708c5412814f14cf82284785c1 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Thu, 5 Mar 2026 17:17:32 +0300 Subject: [PATCH 1/4] feat: fall back to extension storage for proxy log directory When coder.proxyLogDirectory is not configured, use the extension's global storage log path instead of discarding logs. This makes troubleshooting easier without requiring manual configuration. --- package.json | 2 +- src/commands.ts | 66 +++++++++++++++------------------------- src/core/pathResolver.ts | 20 ++++++++++-- src/remote/remote.ts | 12 ++------ 4 files changed, 45 insertions(+), 55 deletions(-) diff --git a/package.json b/package.json index 6adb26ee..88dbcd4b 100644 --- a/package.json +++ b/package.json @@ -102,7 +102,7 @@ "default": "" }, "coder.proxyLogDirectory": { - "markdownDescription": "If set, the Coder CLI will output extra SSH information into this directory, which can be helpful for debugging connectivity issues.", + "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.", "type": "string", "default": "" }, diff --git a/src/commands.ts b/src/commands.ts index d6b08e14..43e244e8 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -151,51 +151,33 @@ export class Commands { return this.openFile(this.workspaceLogPath); } - const logDir = vscode.workspace - .getConfiguration() - .get("coder.proxyLogDirectory"); - if (logDir) { - try { - const files = await fs.readdir(logDir); - // Sort explicitly since fs.readdir order is platform-dependent - const logFiles = files - .filter((f) => f.endsWith(".log")) - .sort((a, b) => a.localeCompare(b)) - .reverse(); - - if (logFiles.length === 0) { - vscode.window.showInformationMessage( - "No log files found in the configured log directory.", - ); - return; - } + const logDir = this.pathResolver.getProxyLogPath(); + try { + const files = await fs.readdir(logDir); + // Sort explicitly since fs.readdir order is platform-dependent + const logFiles = files + .filter((f) => f.endsWith(".log")) + .sort((a, b) => a.localeCompare(b)) + .reverse(); + + if (logFiles.length === 0) { + vscode.window.showInformationMessage( + "No log files found in the log directory.", + ); + return; + } - const selected = await vscode.window.showQuickPick(logFiles, { - title: "Select a log file to view", - }); + const selected = await vscode.window.showQuickPick(logFiles, { + title: "Select a log file to view", + }); - if (selected) { - await this.openFile(path.join(logDir, selected)); - } - } catch (error) { - vscode.window.showErrorMessage( - `Failed to read log directory: ${error instanceof Error ? error.message : String(error)}`, - ); + if (selected) { + await this.openFile(path.join(logDir, selected)); } - } else { - vscode.window - .showInformationMessage( - "No logs available. Make sure to set coder.proxyLogDirectory to get logs.", - "Open Settings", - ) - .then((action) => { - if (action === "Open Settings") { - vscode.commands.executeCommand( - "workbench.action.openSettings", - "coder.proxyLogDirectory", - ); - } - }); + } catch (error) { + vscode.window.showErrorMessage( + `Failed to read log directory: ${error instanceof Error ? error.message : String(error)}`, + ); } } diff --git a/src/core/pathResolver.ts b/src/core/pathResolver.ts index 8c320088..5f528b2f 100644 --- a/src/core/pathResolver.ts +++ b/src/core/pathResolver.ts @@ -1,6 +1,8 @@ import * as path from "node:path"; import * as vscode from "vscode"; +import { expandPath } from "../util"; + export class PathResolver { constructor( private readonly basePath: string, @@ -51,16 +53,28 @@ export class PathResolver { } /** - * Return the path where log data from the connection is stored. + * Return the default path where log data from the connection is stored. * * The CLI will write files here named after the process PID. - * - * Note: This directory is not currently used. */ 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 configured || this.getLogPath(); + } + /** * Get the path to the user's settings.json file. * diff --git a/src/remote/remote.ts b/src/remote/remote.ts index e51001ad..bb8297db 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -46,7 +46,6 @@ import { OAuthSessionManager } from "../oauth/sessionManager"; import { AuthorityPrefix, escapeCommandArg, - expandPath, parseRemoteAuthority, } from "../util"; import { vscodeProposed } from "../vscodeProposed"; @@ -702,7 +701,8 @@ export class Remote { /** * Return the --log-dir argument value for the ProxyCommand. It may be an - * empty string if the setting is not set or the cli does not support it. + * empty string if the CLI does not support it. Falls back to extension + * storage when the user setting is not configured. * * Value defined in the "coder.sshFlags" setting is not considered. */ @@ -710,13 +710,7 @@ export class Remote { if (!featureSet.proxyLogDirectory) { return ""; } - // If the proxyLogDirectory is not set in the extension settings we don't send one. - return expandPath( - String( - vscode.workspace.getConfiguration().get("coder.proxyLogDirectory") ?? - "", - ).trim(), - ); + return this.pathResolver.getProxyLogPath(); } /** From 0bef2e29dea8f8496f29f468d84b06ad7dbb4a07 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Thu, 5 Mar 2026 17:22:07 +0300 Subject: [PATCH 2/4] test: add tests for getProxyLogPath log directory resolution Covers configured path, empty/whitespace/unset fallback to default, and tilde expansion. --- test/unit/core/pathResolver.test.ts | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/test/unit/core/pathResolver.test.ts b/test/unit/core/pathResolver.test.ts index 2930fb7e..c7a39082 100644 --- a/test/unit/core/pathResolver.test.ts +++ b/test/unit/core/pathResolver.test.ts @@ -1,5 +1,5 @@ import * as path from "path"; -import { beforeEach, describe, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { PathResolver } from "@/core/pathResolver"; @@ -28,6 +28,32 @@ describe("PathResolver", () => { expectPathsEqual(pathResolver.getUrlPath(""), path.join(basePath, "url")); }); + describe("getProxyLogPath", () => { + const defaultLogPath = path.join(basePath, "log"); + + it.each([ + { setting: "/custom/log/dir", expected: "/custom/log/dir" }, + { setting: "", expected: defaultLogPath }, + { setting: " ", expected: defaultLogPath }, + { setting: undefined, expected: defaultLogPath }, + ])( + "should return $expected when setting is '$setting'", + ({ setting, expected }) => { + if (setting !== undefined) { + mockConfig.set("coder.proxyLogDirectory", setting); + } + expectPathsEqual(pathResolver.getProxyLogPath(), expected); + }, + ); + + it("should expand tilde in configured path", () => { + mockConfig.set("coder.proxyLogDirectory", "~/logs"); + const result = pathResolver.getProxyLogPath(); + expect(result).not.toContain("~"); + expect(result).toContain("logs"); + }); + }); + describe("getBinaryCachePath", () => { it("should use custom binary destination when configured", () => { mockConfig.set("coder.binaryDestination", "/custom/binary/path"); From c730b241ea54e136f67da65dcf5b8339fdb5ceae Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Mon, 9 Mar 2026 13:15:29 +0300 Subject: [PATCH 3/4] 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. --- package.json | 2 +- src/core/pathResolver.ts | 52 ++++++++++++++++------------- src/remote/remote.ts | 5 ++- test/unit/core/pathResolver.test.ts | 44 +++++++++++++++++++++++- 4 files changed, 74 insertions(+), 29 deletions(-) diff --git a/package.json b/package.json index 88dbcd4b..e37f1ef8 100644 --- a/package.json +++ b/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": "" }, diff --git a/src/core/pathResolver.ts b/src/core/pathResolver.ts index 5f528b2f..f02d078f 100644 --- a/src/core/pathResolver.ts +++ b/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("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(setting)?.trim() ?? "", + ); + const resolved = + fromSetting || expandPath(process.env[envVar]?.trim() ?? ""); + return resolved ? path.normalize(resolved) : ""; + } } diff --git a/src/remote/remote.ts b/src/remote/remote.ts index bb8297db..a3f59aef 100644 --- a/src/remote/remote.ts +++ b/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. */ diff --git a/test/unit/core/pathResolver.test.ts b/test/unit/core/pathResolver.test.ts index c7a39082..6962a633 100644 --- a/test/unit/core/pathResolver.test.ts +++ b/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", ""); From 52f29181595432951d92e0ecf954e85b53b0ea8a Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Wed, 11 Mar 2026 14:51:22 +0300 Subject: [PATCH 4/4] Handle review comments --- CHANGELOG.md | 6 ++++++ src/commands.ts | 30 ++++++++++++++++++++++-------- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ebd22e56..57e08bf3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ ## Unreleased +### Added + +- Proxy log directory now defaults to the extension's global storage when `coder.proxyLogDirectory` + is not set, so SSH connection logs are always captured without manual configuration. Also respects + the `CODER_SSH_LOG_DIR` environment variable as a fallback. + ## [v1.14.0-pre](https://github.com/coder/vscode-coder/releases/tag/v1.14.0-pre) 2026-03-06 ### Added diff --git a/src/commands.ts b/src/commands.ts index 43e244e8..3a9ad5f1 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -148,12 +148,12 @@ export class Commands { public async viewLogs(): Promise { if (this.workspaceLogPath) { // Return the connected deployment's log file. - return this.openFile(this.workspaceLogPath); + return openFile(this.workspaceLogPath); } const logDir = this.pathResolver.getProxyLogPath(); try { - const files = await fs.readdir(logDir); + const files = await readdirOrEmpty(logDir); // Sort explicitly since fs.readdir order is platform-dependent const logFiles = files .filter((f) => f.endsWith(".log")) @@ -172,7 +172,7 @@ export class Commands { }); if (selected) { - await this.openFile(path.join(logDir, selected)); + await openFile(path.join(logDir, selected)); } } catch (error) { vscode.window.showErrorMessage( @@ -181,11 +181,6 @@ export class Commands { } } - private async openFile(filePath: string): Promise { - const uri = vscode.Uri.file(filePath); - await vscode.window.showTextDocument(uri); - } - /** * Log out and clear stored credentials, requiring re-authentication on next login. */ @@ -768,3 +763,22 @@ export class Commands { }); } } + +async function openFile(filePath: string): Promise { + const uri = vscode.Uri.file(filePath); + await vscode.window.showTextDocument(uri); +} + +/** + * Read a directory's entries, returning an empty array if it does not exist. + */ +async function readdirOrEmpty(dirPath: string): Promise { + try { + return await fs.readdir(dirPath); + } catch (err) { + if (err instanceof Error && "code" in err && err.code === "ENOENT") { + return []; + } + throw err; + } +}