Skip to content

Commit 837df11

Browse files
authored
feat: fall back to extension storage for proxy log directory (#827)
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. - Add getProxyLogPath() to PathResolver with setting / CODER_SSH_LOG_DIR / global storage fallback chain - Extract shared resolveOverride() for consistent setting+env resolution across getBinaryCachePath and getProxyLogPath - Simplify viewLogs to use getProxyLogPath() and handle missing directories gracefully - Add tests for log path resolution, env var precedence, and tilde expansion Fixes #824
1 parent 26e8363 commit 837df11

File tree

6 files changed

+155
-74
lines changed

6 files changed

+155
-74
lines changed

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,12 @@
22

33
## Unreleased
44

5+
### Added
6+
7+
- Proxy log directory now defaults to the extension's global storage when `coder.proxyLogDirectory`
8+
is not set, so SSH connection logs are always captured without manual configuration. Also respects
9+
the `CODER_SSH_LOG_DIR` environment variable as a fallback.
10+
511
## [v1.14.0-pre](https://github.com/coder/vscode-coder/releases/tag/v1.14.0-pre) 2026-03-06
612

713
### Added

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@
102102
"default": ""
103103
},
104104
"coder.proxyLogDirectory": {
105-
"markdownDescription": "If set, the Coder CLI will output extra SSH information into this directory, which can be helpful for debugging connectivity issues.",
105+
"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.",
106106
"type": "string",
107107
"default": ""
108108
},

src/commands.ts

Lines changed: 44 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -148,62 +148,39 @@ export class Commands {
148148
public async viewLogs(): Promise<void> {
149149
if (this.workspaceLogPath) {
150150
// Return the connected deployment's log file.
151-
return this.openFile(this.workspaceLogPath);
151+
return openFile(this.workspaceLogPath);
152152
}
153153

154-
const logDir = vscode.workspace
155-
.getConfiguration()
156-
.get<string>("coder.proxyLogDirectory");
157-
if (logDir) {
158-
try {
159-
const files = await fs.readdir(logDir);
160-
// Sort explicitly since fs.readdir order is platform-dependent
161-
const logFiles = files
162-
.filter((f) => f.endsWith(".log"))
163-
.sort((a, b) => a.localeCompare(b))
164-
.reverse();
165-
166-
if (logFiles.length === 0) {
167-
vscode.window.showInformationMessage(
168-
"No log files found in the configured log directory.",
169-
);
170-
return;
171-
}
154+
const logDir = this.pathResolver.getProxyLogPath();
155+
try {
156+
const files = await readdirOrEmpty(logDir);
157+
// Sort explicitly since fs.readdir order is platform-dependent
158+
const logFiles = files
159+
.filter((f) => f.endsWith(".log"))
160+
.sort((a, b) => a.localeCompare(b))
161+
.reverse();
162+
163+
if (logFiles.length === 0) {
164+
vscode.window.showInformationMessage(
165+
"No log files found in the log directory.",
166+
);
167+
return;
168+
}
172169

173-
const selected = await vscode.window.showQuickPick(logFiles, {
174-
title: "Select a log file to view",
175-
});
170+
const selected = await vscode.window.showQuickPick(logFiles, {
171+
title: "Select a log file to view",
172+
});
176173

177-
if (selected) {
178-
await this.openFile(path.join(logDir, selected));
179-
}
180-
} catch (error) {
181-
vscode.window.showErrorMessage(
182-
`Failed to read log directory: ${error instanceof Error ? error.message : String(error)}`,
183-
);
174+
if (selected) {
175+
await openFile(path.join(logDir, selected));
184176
}
185-
} else {
186-
vscode.window
187-
.showInformationMessage(
188-
"No logs available. Make sure to set coder.proxyLogDirectory to get logs.",
189-
"Open Settings",
190-
)
191-
.then((action) => {
192-
if (action === "Open Settings") {
193-
vscode.commands.executeCommand(
194-
"workbench.action.openSettings",
195-
"coder.proxyLogDirectory",
196-
);
197-
}
198-
});
177+
} catch (error) {
178+
vscode.window.showErrorMessage(
179+
`Failed to read log directory: ${error instanceof Error ? error.message : String(error)}`,
180+
);
199181
}
200182
}
201183

202-
private async openFile(filePath: string): Promise<void> {
203-
const uri = vscode.Uri.file(filePath);
204-
await vscode.window.showTextDocument(uri);
205-
}
206-
207184
/**
208185
* Log out and clear stored credentials, requiring re-authentication on next login.
209186
*/
@@ -786,3 +763,22 @@ export class Commands {
786763
});
787764
}
788765
}
766+
767+
async function openFile(filePath: string): Promise<void> {
768+
const uri = vscode.Uri.file(filePath);
769+
await vscode.window.showTextDocument(uri);
770+
}
771+
772+
/**
773+
* Read a directory's entries, returning an empty array if it does not exist.
774+
*/
775+
async function readdirOrEmpty(dirPath: string): Promise<string[]> {
776+
try {
777+
return await fs.readdir(dirPath);
778+
} catch (err) {
779+
if (err instanceof Error && "code" in err && err.code === "ENOENT") {
780+
return [];
781+
}
782+
throw err;
783+
}
784+
}

src/core/pathResolver.ts

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import * as path from "node:path";
22
import * as vscode from "vscode";
33

4+
import { expandPath } from "../util";
5+
46
export class PathResolver {
57
constructor(
68
private readonly basePath: string,
@@ -30,15 +32,12 @@ export class PathResolver {
3032
* The caller must ensure this directory exists before use.
3133
*/
3234
public getBinaryCachePath(safeHostname: string): string {
33-
const settingPath = vscode.workspace
34-
.getConfiguration()
35-
.get<string>("coder.binaryDestination")
36-
?.trim();
37-
const binaryPath =
38-
settingPath || process.env.CODER_BINARY_DESTINATION?.trim();
39-
return binaryPath
40-
? path.normalize(binaryPath)
41-
: path.join(this.getGlobalConfigDir(safeHostname), "bin");
35+
return (
36+
PathResolver.resolveOverride(
37+
"coder.binaryDestination",
38+
"CODER_BINARY_DESTINATION",
39+
) || path.join(this.getGlobalConfigDir(safeHostname), "bin")
40+
);
4241
}
4342

4443
/**
@@ -51,14 +50,19 @@ export class PathResolver {
5150
}
5251

5352
/**
54-
* Return the path where log data from the connection is stored.
53+
* Return the proxy log directory from the `coder.proxyLogDirectory` setting
54+
* or the `CODER_SSH_LOG_DIR` environment variable, falling back to the `log`
55+
* subdirectory inside the extension's global storage path.
5556
*
5657
* The CLI will write files here named after the process PID.
57-
*
58-
* Note: This directory is not currently used.
5958
*/
60-
public getLogPath(): string {
61-
return path.join(this.basePath, "log");
59+
public getProxyLogPath(): string {
60+
return (
61+
PathResolver.resolveOverride(
62+
"coder.proxyLogDirectory",
63+
"CODER_SSH_LOG_DIR",
64+
) || path.join(this.basePath, "log")
65+
);
6266
}
6367

6468
/**
@@ -117,4 +121,18 @@ export class PathResolver {
117121
public getCodeLogDir(): string {
118122
return this.codeLogPath;
119123
}
124+
125+
/**
126+
* Read a path from a VS Code setting then an environment variable, returning
127+
* the first non-empty value after trimming, tilde/variable expansion, and
128+
* normalization. Returns an empty string when neither source provides a path.
129+
*/
130+
private static resolveOverride(setting: string, envVar: string): string {
131+
const fromSetting = expandPath(
132+
vscode.workspace.getConfiguration().get<string>(setting)?.trim() ?? "",
133+
);
134+
const resolved =
135+
fromSetting || expandPath(process.env[envVar]?.trim() ?? "");
136+
return resolved ? path.normalize(resolved) : "";
137+
}
120138
}

src/remote/remote.ts

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ import { OAuthSessionManager } from "../oauth/sessionManager";
4646
import {
4747
AuthorityPrefix,
4848
escapeCommandArg,
49-
expandPath,
5049
parseRemoteAuthority,
5150
} from "../util";
5251
import { vscodeProposed } from "../vscodeProposed";
@@ -701,22 +700,16 @@ export class Remote {
701700
}
702701

703702
/**
704-
* Return the --log-dir argument value for the ProxyCommand. It may be an
705-
* empty string if the setting is not set or the cli does not support it.
703+
* Return the --log-dir argument value for the ProxyCommand, or an empty
704+
* string when the CLI does not support it.
706705
*
707706
* Value defined in the "coder.sshFlags" setting is not considered.
708707
*/
709708
private getLogDir(featureSet: FeatureSet): string {
710709
if (!featureSet.proxyLogDirectory) {
711710
return "";
712711
}
713-
// If the proxyLogDirectory is not set in the extension settings we don't send one.
714-
return expandPath(
715-
String(
716-
vscode.workspace.getConfiguration().get("coder.proxyLogDirectory") ??
717-
"",
718-
).trim(),
719-
);
712+
return this.pathResolver.getProxyLogPath();
720713
}
721714

722715
/**

test/unit/core/pathResolver.test.ts

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as path from "path";
2-
import { beforeEach, describe, it, vi } from "vitest";
2+
import { beforeEach, describe, expect, it, vi } from "vitest";
33

44
import { PathResolver } from "@/core/pathResolver";
55

@@ -28,6 +28,60 @@ describe("PathResolver", () => {
2828
expectPathsEqual(pathResolver.getUrlPath(""), path.join(basePath, "url"));
2929
});
3030

31+
describe("getProxyLogPath", () => {
32+
const defaultLogPath = path.join(basePath, "log");
33+
34+
it.each([
35+
{ setting: "/custom/log/dir", expected: "/custom/log/dir" },
36+
{ setting: "", expected: defaultLogPath },
37+
{ setting: " ", expected: defaultLogPath },
38+
{ setting: undefined, expected: defaultLogPath },
39+
])(
40+
"should return $expected when setting is '$setting'",
41+
({ setting, expected }) => {
42+
if (setting !== undefined) {
43+
mockConfig.set("coder.proxyLogDirectory", setting);
44+
}
45+
expectPathsEqual(pathResolver.getProxyLogPath(), expected);
46+
},
47+
);
48+
49+
it("should expand tilde and ${userHome} in configured path", () => {
50+
mockConfig.set("coder.proxyLogDirectory", "~/logs");
51+
expect(pathResolver.getProxyLogPath()).not.toContain("~");
52+
53+
mockConfig.set("coder.proxyLogDirectory", "${userHome}/logs");
54+
expect(pathResolver.getProxyLogPath()).not.toContain("${userHome}");
55+
});
56+
57+
it("should normalize configured path", () => {
58+
mockConfig.set("coder.proxyLogDirectory", "/custom/../log/./dir");
59+
expectPathsEqual(pathResolver.getProxyLogPath(), "/log/dir");
60+
});
61+
62+
it("should use CODER_SSH_LOG_DIR environment variable with proper precedence", () => {
63+
// Use the global storage when the environment variable and setting are unset/blank
64+
vi.stubEnv("CODER_SSH_LOG_DIR", "");
65+
mockConfig.set("coder.proxyLogDirectory", "");
66+
expectPathsEqual(pathResolver.getProxyLogPath(), defaultLogPath);
67+
68+
// Test environment variable takes precedence over global storage
69+
vi.stubEnv("CODER_SSH_LOG_DIR", " /env/log/path ");
70+
expectPathsEqual(pathResolver.getProxyLogPath(), "/env/log/path");
71+
72+
// Test setting takes precedence over environment variable
73+
mockConfig.set("coder.proxyLogDirectory", " /setting/log/path ");
74+
expectPathsEqual(pathResolver.getProxyLogPath(), "/setting/log/path");
75+
});
76+
77+
it("should expand tilde in CODER_SSH_LOG_DIR", () => {
78+
vi.stubEnv("CODER_SSH_LOG_DIR", "~/logs");
79+
const result = pathResolver.getProxyLogPath();
80+
expect(result).not.toContain("~");
81+
expect(result).toContain("logs");
82+
});
83+
});
84+
3185
describe("getBinaryCachePath", () => {
3286
it("should use custom binary destination when configured", () => {
3387
mockConfig.set("coder.binaryDestination", "/custom/binary/path");
@@ -54,6 +108,20 @@ describe("PathResolver", () => {
54108
);
55109
});
56110

111+
it("should expand tilde in configured path", () => {
112+
mockConfig.set("coder.binaryDestination", "~/bin");
113+
const result = pathResolver.getBinaryCachePath("deployment");
114+
expect(result).not.toContain("~");
115+
expect(result).toContain("bin");
116+
});
117+
118+
it("should expand tilde in CODER_BINARY_DESTINATION", () => {
119+
vi.stubEnv("CODER_BINARY_DESTINATION", "~/bin");
120+
const result = pathResolver.getBinaryCachePath("deployment");
121+
expect(result).not.toContain("~");
122+
expect(result).toContain("bin");
123+
});
124+
57125
it("should use CODER_BINARY_DESTINATION environment variable with proper precedence", () => {
58126
// Use the global storage when the environment variable and setting are unset/blank
59127
vi.stubEnv("CODER_BINARY_DESTINATION", "");

0 commit comments

Comments
 (0)