From 5b09d2b82ea35da28090a72ad02a092f20e19975 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Thu, 5 Mar 2026 16:39:26 +0300 Subject: [PATCH 1/4] feat: set reconnectionGraceTime to 8h, extract settings helper VS Code's default reconnection grace time of 3 hours is too short for Coder workspaces that go offline overnight. Set 8 hours (28800s) when the user hasn't configured a value. Extract the inline jsonc settings manipulation from remote.ts into a reusable applySettingOverrides() function with dedicated tests. --- src/remote/remote.ts | 90 ++-------- src/remote/userSettings.ts | 98 +++++++++++ test/unit/remote/userSettings.test.ts | 238 ++++++++++++++++++++++++++ 3 files changed, 347 insertions(+), 79 deletions(-) create mode 100644 src/remote/userSettings.ts create mode 100644 test/unit/remote/userSettings.test.ts diff --git a/src/remote/remote.ts b/src/remote/remote.ts index ac7c99ca..ab53ea6e 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -4,7 +4,6 @@ import { type Workspace, type WorkspaceAgent, } from "coder/site/src/api/typesGenerated"; -import * as jsonc from "jsonc-parser"; import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; @@ -60,6 +59,7 @@ import { } from "./sshConfig"; import { SshProcessMonitor } from "./sshProcess"; import { computeSshProperties, sshSupportsSetEnv } from "./sshSupport"; +import { applySettingOverrides, buildSshOverrides } from "./userSettings"; import { WorkspaceStateMachine } from "./workspaceStateMachine"; export interface RemoteDetails extends vscode.Disposable { @@ -459,85 +459,17 @@ export class Remote { const inbox = await Inbox.create(workspace, workspaceClient, this.logger); disposables.push(inbox); - // Do some janky setting manipulation. this.logger.info("Modifying settings..."); - const remotePlatforms = vscodeProposed.workspace - .getConfiguration() - .get>("remote.SSH.remotePlatform", {}); - const connTimeout = vscodeProposed.workspace - .getConfiguration() - .get("remote.SSH.connectTimeout"); - - // We have to directly munge the settings file with jsonc because trying to - // update properly through the extension API hangs indefinitely. Possibly - // VS Code is trying to update configuration on the remote, which cannot - // connect until we finish here leading to a deadlock. We need to update it - // locally, anyway, and it does not seem possible to force that via API. - let settingsContent = "{}"; - try { - settingsContent = await fs.readFile( - this.pathResolver.getUserSettingsPath(), - "utf8", - ); - } catch { - // Ignore! It's probably because the file doesn't exist. - } - - // Add the remote platform for this host to bypass a step where VS Code asks - // the user for the platform. - let mungedPlatforms = false; - if ( - !remotePlatforms[parts.sshHost] || - remotePlatforms[parts.sshHost] !== agent.operating_system - ) { - remotePlatforms[parts.sshHost] = agent.operating_system; - settingsContent = jsonc.applyEdits( - settingsContent, - jsonc.modify( - settingsContent, - ["remote.SSH.remotePlatform"], - remotePlatforms, - {}, - ), - ); - mungedPlatforms = true; - } - - // VS Code ignores the connect timeout in the SSH config and uses a default - // of 15 seconds, which can be too short in the case where we wait for - // startup scripts. For now we hardcode a longer value. Because this is - // potentially overwriting user configuration, it feels a bit sketchy. If - // microsoft/vscode-remote-release#8519 is resolved we can remove this. - const minConnTimeout = 1800; - let mungedConnTimeout = false; - if (!connTimeout || connTimeout < minConnTimeout) { - settingsContent = jsonc.applyEdits( - settingsContent, - jsonc.modify( - settingsContent, - ["remote.SSH.connectTimeout"], - minConnTimeout, - {}, - ), - ); - mungedConnTimeout = true; - } - - if (mungedPlatforms || mungedConnTimeout) { - try { - await fs.writeFile( - this.pathResolver.getUserSettingsPath(), - settingsContent, - ); - } catch (ex) { - // This could be because the user's settings.json is read-only. This is - // the case when using home-manager on NixOS, for example. Failure to - // write here is not necessarily catastrophic since the user will be - // asked for the platform and the default timeout might be sufficient. - mungedPlatforms = mungedConnTimeout = false; - this.logger.warn("Failed to configure settings", ex); - } - } + const overrides = buildSshOverrides( + vscodeProposed.workspace.getConfiguration(), + parts.sshHost, + agent.operating_system, + ); + await applySettingOverrides( + this.pathResolver.getUserSettingsPath(), + overrides, + this.logger, + ); const logDir = this.getLogDir(featureSet); diff --git a/src/remote/userSettings.ts b/src/remote/userSettings.ts new file mode 100644 index 00000000..75ea03bf --- /dev/null +++ b/src/remote/userSettings.ts @@ -0,0 +1,98 @@ +import * as jsonc from "jsonc-parser"; +import * as fs from "node:fs/promises"; + +import type { WorkspaceConfiguration } from "vscode"; + +import type { Logger } from "../logging/logger"; + +export interface SettingOverride { + key: string; + value: unknown; +} + +/** + * Build the list of VS Code setting overrides needed for a remote SSH + * connection to a Coder workspace. + */ +export function buildSshOverrides( + config: Pick, + sshHost: string, + agentOS: string, +): SettingOverride[] { + const overrides: SettingOverride[] = []; + + // Bypass the platform prompt by setting the remote platform for this host. + const remotePlatforms = config.get>( + "remote.SSH.remotePlatform", + {}, + ); + if (remotePlatforms[sshHost] !== agentOS) { + overrides.push({ + key: "remote.SSH.remotePlatform", + value: { ...remotePlatforms, [sshHost]: agentOS }, + }); + } + + // VS Code's default connect timeout of 15s is too short when waiting for + // startup scripts. Enforce a minimum. + const minConnTimeout = 1800; + const connTimeout = config.get("remote.SSH.connectTimeout"); + if (!connTimeout || connTimeout < minConnTimeout) { + overrides.push({ + key: "remote.SSH.connectTimeout", + value: minConnTimeout, + }); + } + + // VS Code's default reconnection grace time (ProtocolConstants.ReconnectionGraceTime) + // is 3 hours (10800s). Coder workspaces commonly go offline overnight, so we + // bump to 8 hours. See https://github.com/microsoft/vscode/blob/main/src/vs/base/parts/ipc/common/ipc.net.ts + if (config.get("remote.SSH.reconnectionGraceTime") === undefined) { + overrides.push({ + key: "remote.SSH.reconnectionGraceTime", + value: 28800, // 8 hours in seconds + }); + } + + return overrides; +} + +/** + * Apply setting overrides to the user's settings.json file. + * + * We munge the file directly with jsonc instead of using the VS Code API + * because the API hangs indefinitely during remote connection setup (likely + * a deadlock from trying to update config on the not-yet-connected remote). + */ +export async function applySettingOverrides( + settingsFilePath: string, + overrides: SettingOverride[], + logger: Logger, +): Promise { + if (overrides.length === 0) { + return false; + } + + let settingsContent = "{}"; + try { + settingsContent = await fs.readFile(settingsFilePath, "utf8"); + } catch { + // File probably doesn't exist yet. + } + + for (const { key, value } of overrides) { + settingsContent = jsonc.applyEdits( + settingsContent, + jsonc.modify(settingsContent, [key], value, {}), + ); + } + + try { + await fs.writeFile(settingsFilePath, settingsContent); + return true; + } catch (ex) { + // Could be read-only (e.g. home-manager on NixOS). Not catastrophic. + logger.warn("Failed to configure settings", ex); + return false; + } +} diff --git a/test/unit/remote/userSettings.test.ts b/test/unit/remote/userSettings.test.ts new file mode 100644 index 00000000..d8cddbea --- /dev/null +++ b/test/unit/remote/userSettings.test.ts @@ -0,0 +1,238 @@ +import { vol } from "memfs"; +import * as fsPromises from "node:fs/promises"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { + applySettingOverrides, + buildSshOverrides, +} from "@/remote/userSettings"; + +import { + MockConfigurationProvider, + createMockLogger, +} from "../../mocks/testHelpers"; + +import type * as fs from "node:fs"; + +vi.mock("node:fs/promises", async () => { + const memfs: { fs: typeof fs } = await vi.importActual("memfs"); + return memfs.fs.promises; +}); + +/** Helper to extract a single override by key from the result. */ +function findOverride( + overrides: Array<{ key: string; value: unknown }>, + key: string, +): unknown { + return overrides.find((o) => o.key === key)?.value; +} + +interface TimeoutCase { + timeout: number | undefined; + label: string; +} + +describe("buildSshOverrides", () => { + describe("remote platform", () => { + it("adds host when missing or OS differs", () => { + const config = new MockConfigurationProvider(); + + // New host is added alongside existing entries. + config.set("remote.SSH.remotePlatform", { "other-host": "darwin" }); + expect( + findOverride( + buildSshOverrides(config, "new-host", "linux"), + "remote.SSH.remotePlatform", + ), + ).toEqual({ "other-host": "darwin", "new-host": "linux" }); + + // Existing host with wrong OS gets corrected. + config.set("remote.SSH.remotePlatform", { "my-host": "windows" }); + expect( + findOverride( + buildSshOverrides(config, "my-host", "linux"), + "remote.SSH.remotePlatform", + ), + ).toEqual({ "my-host": "linux" }); + }); + + it("skips override when host already matches", () => { + const config = new MockConfigurationProvider(); + config.set("remote.SSH.remotePlatform", { "my-host": "linux" }); + expect( + findOverride( + buildSshOverrides(config, "my-host", "linux"), + "remote.SSH.remotePlatform", + ), + ).toBeUndefined(); + }); + }); + + describe("connect timeout", () => { + it.each([ + { timeout: undefined, label: "unset" }, + { timeout: 0, label: "zero" }, + { timeout: 15, label: "below minimum" }, + { timeout: 1799, label: "just under minimum" }, + ])("enforces minimum of 1800 when $label", ({ timeout }) => { + const config = new MockConfigurationProvider(); + if (timeout !== undefined) { + config.set("remote.SSH.connectTimeout", timeout); + } + expect( + findOverride( + buildSshOverrides(config, "host", "linux"), + "remote.SSH.connectTimeout", + ), + ).toBe(1800); + }); + + it.each([ + { timeout: 1800, label: "exactly minimum" }, + { timeout: 3600, label: "above minimum" }, + ])("preserves timeout when $label", ({ timeout }) => { + const config = new MockConfigurationProvider(); + config.set("remote.SSH.connectTimeout", timeout); + expect( + findOverride( + buildSshOverrides(config, "host", "linux"), + "remote.SSH.connectTimeout", + ), + ).toBeUndefined(); + }); + }); + + describe("reconnection grace time", () => { + it("defaults to 8 hours when not configured", () => { + expect( + findOverride( + buildSshOverrides(new MockConfigurationProvider(), "host", "linux"), + "remote.SSH.reconnectionGraceTime", + ), + ).toBe(28800); + }); + + it("preserves any user-configured value", () => { + const config = new MockConfigurationProvider(); + config.set("remote.SSH.reconnectionGraceTime", 3600); + expect( + findOverride( + buildSshOverrides(config, "host", "linux"), + "remote.SSH.reconnectionGraceTime", + ), + ).toBeUndefined(); + }); + }); + + it("produces no overrides when all settings are already correct", () => { + const config = new MockConfigurationProvider(); + config.set("remote.SSH.remotePlatform", { "my-host": "linux" }); + config.set("remote.SSH.connectTimeout", 3600); + config.set("remote.SSH.reconnectionGraceTime", 7200); + expect(buildSshOverrides(config, "my-host", "linux")).toHaveLength(0); + }); +}); + +describe("applySettingOverrides", () => { + const settingsPath = "/settings.json"; + const logger = createMockLogger(); + + beforeEach(() => { + vol.reset(); + }); + + async function readSettings(): Promise> { + const raw = await fsPromises.readFile(settingsPath, "utf8"); + return JSON.parse(raw) as Record; + } + + it("returns false when overrides list is empty", async () => { + expect(await applySettingOverrides(settingsPath, [], logger)).toBe(false); + }); + + it("creates file and applies overrides when file does not exist", async () => { + const ok = await applySettingOverrides( + settingsPath, + [ + { + key: "remote.SSH.remotePlatform", + value: { "coder-gke--main": "linux" }, + }, + { key: "remote.SSH.connectTimeout", value: 1800 }, + { key: "remote.SSH.reconnectionGraceTime", value: 28800 }, + ], + logger, + ); + + expect(ok).toBe(true); + expect(await readSettings()).toMatchObject({ + "remote.SSH.remotePlatform": { "coder-gke--main": "linux" }, + "remote.SSH.connectTimeout": 1800, + "remote.SSH.reconnectionGraceTime": 28800, + }); + }); + + it("preserves existing settings when applying overrides", async () => { + vol.fromJSON({ + [settingsPath]: JSON.stringify({ + "remote.SSH.remotePlatform": { "coder-gke--main": "linux" }, + "remote.SSH.connectTimeout": 15, + }), + }); + + await applySettingOverrides( + settingsPath, + [{ key: "remote.SSH.connectTimeout", value: 1800 }], + logger, + ); + + expect(await readSettings()).toMatchObject({ + "remote.SSH.remotePlatform": { "coder-gke--main": "linux" }, + "remote.SSH.connectTimeout": 1800, + }); + }); + + it("handles JSONC with comments", async () => { + vol.fromJSON({ + [settingsPath]: [ + "{", + " // Platform overrides for remote SSH hosts", + ' "remote.SSH.remotePlatform": { "coder-gke--main": "linux" },', + ' "remote.SSH.connectTimeout": 15', + "}", + ].join("\n"), + }); + + await applySettingOverrides( + settingsPath, + [{ key: "remote.SSH.connectTimeout", value: 1800 }], + logger, + ); + + const raw = await fsPromises.readFile(settingsPath, "utf8"); + expect(raw).toContain("// Platform overrides for remote SSH hosts"); + expect(raw).toContain("1800"); + expect(raw).toContain('"remote.SSH.remotePlatform"'); + }); + + it("returns false and logs warning when write fails", async () => { + vol.fromJSON({ [settingsPath]: "{}" }); + const writeSpy = vi + .spyOn(fsPromises, "writeFile") + .mockRejectedValueOnce(new Error("EACCES: permission denied")); + + const ok = await applySettingOverrides( + settingsPath, + [{ key: "remote.SSH.connectTimeout", value: 1800 }], + logger, + ); + + expect(ok).toBe(false); + expect(logger.warn).toHaveBeenCalledWith( + "Failed to configure settings", + expect.anything(), + ); + + writeSpy.mockRestore(); + }); +}); From ead3b59eb2dcb257a84d70b4ff67ec378d9e8990 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Thu, 12 Mar 2026 22:12:06 +0300 Subject: [PATCH 2/4] feat: add "Apply Recommended SSH Settings" command and set defaults for Cursor/reconnection Add a command palette entry (Coder: Apply Recommended SSH Settings) that writes all four recommended SSH settings globally. Also automatically set serverShutdownTimeout (Cursor) and maxReconnectionAttempts when undefined during connection setup, matching the existing reconnectionGraceTime behavior. --- package.json | 8 +++++ src/commands.ts | 20 +++++++++++ src/extension.ts | 4 +++ src/remote/userSettings.ts | 50 ++++++++++++++++++++------- test/unit/remote/userSettings.test.ts | 14 ++++++++ 5 files changed, 84 insertions(+), 12 deletions(-) diff --git a/package.json b/package.json index bfe0dabf..02c49b40 100644 --- a/package.json +++ b/package.json @@ -320,6 +320,11 @@ "title": "Refresh Tasks", "category": "Coder", "icon": "$(refresh)" + }, + { + "command": "coder.applyRecommendedSettings", + "title": "Apply Recommended SSH Settings", + "category": "Coder" } ], "menus": { @@ -386,6 +391,9 @@ { "command": "coder.tasks.refresh", "when": "false" + }, + { + "command": "coder.applyRecommendedSettings" } ], "view/title": [ diff --git a/src/commands.ts b/src/commands.ts index 3a9ad5f1..99010e43 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -24,6 +24,7 @@ import { type Logger } from "./logging/logger"; import { type LoginCoordinator } from "./login/loginCoordinator"; import { withProgress } from "./progress"; import { maybeAskAgent, maybeAskUrl } from "./promptUtils"; +import { RECOMMENDED_SSH_SETTINGS } from "./remote/userSettings"; import { escapeCommandArg, toRemoteAuthority, toSafeHost } from "./util"; import { vscodeProposed } from "./vscodeProposed"; import { @@ -310,6 +311,25 @@ export class Commands { } } + /** + * Apply recommended SSH settings for reliable Coder workspace connections. + */ + public async applyRecommendedSettings(): Promise { + const config = vscode.workspace.getConfiguration(); + const entries = Object.entries(RECOMMENDED_SSH_SETTINGS); + for (const [key, setting] of entries) { + await config.update( + key, + setting.value, + vscode.ConfigurationTarget.Global, + ); + } + const summary = entries.map(([, s]) => s.label).join(", "); + vscode.window.showInformationMessage( + `Applied recommended SSH settings: ${summary}`, + ); + } + /** * Create a new workspace for the currently logged-in deployment. * diff --git a/src/extension.ts b/src/extension.ts index 9266bf54..4a0db0ad 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -271,6 +271,10 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { "coder.manageCredentials", commands.manageCredentials.bind(commands), ), + vscode.commands.registerCommand( + "coder.applyRecommendedSettings", + commands.applyRecommendedSettings.bind(commands), + ), ); const remote = new Remote(serviceContainer, commands, ctx); diff --git a/src/remote/userSettings.ts b/src/remote/userSettings.ts index 75ea03bf..6defd7ef 100644 --- a/src/remote/userSettings.ts +++ b/src/remote/userSettings.ts @@ -10,6 +10,30 @@ export interface SettingOverride { value: unknown; } +interface RecommendedSetting { + readonly value: number | null; + readonly label: string; +} + +export const RECOMMENDED_SSH_SETTINGS = { + "remote.SSH.connectTimeout": { + value: 1800, + label: "Connect Timeout: 1800s (30 min)", + }, + "remote.SSH.reconnectionGraceTime": { + value: 28800, + label: "Reconnection Grace Time: 28800s (8 hours)", + }, + "remote.SSH.serverShutdownTimeout": { + value: 28800, + label: "Server Shutdown Timeout: 28800s (8 hours)", + }, + "remote.SSH.maxReconnectionAttempts": { + value: null, + label: "Max Reconnection Attempts: max allowed", + }, +} as const satisfies Record; + /** * Build the list of VS Code setting overrides needed for a remote SSH * connection to a Coder workspace. @@ -21,7 +45,7 @@ export function buildSshOverrides( ): SettingOverride[] { const overrides: SettingOverride[] = []; - // Bypass the platform prompt by setting the remote platform for this host. + // Set the remote platform for this host to bypass the platform prompt. const remotePlatforms = config.get>( "remote.SSH.remotePlatform", {}, @@ -33,9 +57,9 @@ export function buildSshOverrides( }); } - // VS Code's default connect timeout of 15s is too short when waiting for - // startup scripts. Enforce a minimum. - const minConnTimeout = 1800; + // Default 15s is too short for startup scripts; enforce a minimum. + const minConnTimeout = + RECOMMENDED_SSH_SETTINGS["remote.SSH.connectTimeout"].value; const connTimeout = config.get("remote.SSH.connectTimeout"); if (!connTimeout || connTimeout < minConnTimeout) { overrides.push({ @@ -44,14 +68,16 @@ export function buildSshOverrides( }); } - // VS Code's default reconnection grace time (ProtocolConstants.ReconnectionGraceTime) - // is 3 hours (10800s). Coder workspaces commonly go offline overnight, so we - // bump to 8 hours. See https://github.com/microsoft/vscode/blob/main/src/vs/base/parts/ipc/common/ipc.net.ts - if (config.get("remote.SSH.reconnectionGraceTime") === undefined) { - overrides.push({ - key: "remote.SSH.reconnectionGraceTime", - value: 28800, // 8 hours in seconds - }); + // Set recommended defaults for settings the user hasn't configured. + const setIfUndefined = [ + "remote.SSH.reconnectionGraceTime", + "remote.SSH.serverShutdownTimeout", + "remote.SSH.maxReconnectionAttempts", + ] as const; + for (const key of setIfUndefined) { + if (config.get(key) === undefined) { + overrides.push({ key, value: RECOMMENDED_SSH_SETTINGS[key].value }); + } } return overrides; diff --git a/test/unit/remote/userSettings.test.ts b/test/unit/remote/userSettings.test.ts index d8cddbea..59192432 100644 --- a/test/unit/remote/userSettings.test.ts +++ b/test/unit/remote/userSettings.test.ts @@ -124,11 +124,25 @@ describe("buildSshOverrides", () => { }); }); + it.each([ + { key: "remote.SSH.serverShutdownTimeout", expected: 28800 }, + { key: "remote.SSH.maxReconnectionAttempts", expected: null }, + ])("defaults $key when not configured", ({ key, expected }) => { + const overrides = buildSshOverrides( + new MockConfigurationProvider(), + "host", + "linux", + ); + expect(findOverride(overrides, key)).toBe(expected); + }); + it("produces no overrides when all settings are already correct", () => { const config = new MockConfigurationProvider(); config.set("remote.SSH.remotePlatform", { "my-host": "linux" }); config.set("remote.SSH.connectTimeout", 3600); config.set("remote.SSH.reconnectionGraceTime", 7200); + config.set("remote.SSH.serverShutdownTimeout", 600); + config.set("remote.SSH.maxReconnectionAttempts", 4); expect(buildSshOverrides(config, "my-host", "linux")).toHaveLength(0); }); }); From 4b6af7f5352cf05f028941a72164dc3e05903e7c Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Mon, 16 Mar 2026 15:53:01 +0300 Subject: [PATCH 3/4] feat: add confirmation dialog, split auto/recommended SSH defaults --- src/commands.ts | 47 ++++++++++++---- src/remote/userSettings.ts | 77 ++++++++++++++++----------- test/unit/remote/userSettings.test.ts | 12 +++++ 3 files changed, 93 insertions(+), 43 deletions(-) diff --git a/src/commands.ts b/src/commands.ts index 99010e43..bc4d9649 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -24,7 +24,10 @@ import { type Logger } from "./logging/logger"; import { type LoginCoordinator } from "./login/loginCoordinator"; import { withProgress } from "./progress"; import { maybeAskAgent, maybeAskUrl } from "./promptUtils"; -import { RECOMMENDED_SSH_SETTINGS } from "./remote/userSettings"; +import { + RECOMMENDED_SSH_SETTINGS, + applySettingOverrides, +} from "./remote/userSettings"; import { escapeCommandArg, toRemoteAuthority, toSafeHost } from "./util"; import { vscodeProposed } from "./vscodeProposed"; import { @@ -315,19 +318,41 @@ export class Commands { * Apply recommended SSH settings for reliable Coder workspace connections. */ public async applyRecommendedSettings(): Promise { - const config = vscode.workspace.getConfiguration(); const entries = Object.entries(RECOMMENDED_SSH_SETTINGS); - for (const [key, setting] of entries) { - await config.update( - key, - setting.value, - vscode.ConfigurationTarget.Global, - ); + const summary = entries.map(([, s]) => s.label).join("\n"); + const confirm = await vscodeProposed.window.showWarningMessage( + "Apply Recommended SSH Settings", + { + useCustom: true, + modal: true, + detail: summary, + }, + "Apply", + ); + if (confirm !== "Apply") { + return; } - const summary = entries.map(([, s]) => s.label).join(", "); - vscode.window.showInformationMessage( - `Applied recommended SSH settings: ${summary}`, + + const overrides = entries.map(([key, setting]) => ({ + key, + value: setting.value, + })); + await applySettingOverrides( + this.pathResolver.getUserSettingsPath(), + overrides, + this.logger, ); + if (this.remoteWorkspaceClient) { + const action = await vscode.window.showInformationMessage( + "Applied recommended SSH settings. Reload the window for changes to take effect.", + "Reload Window", + ); + if (action === "Reload Window") { + await vscode.commands.executeCommand("workbench.action.reloadWindow"); + } + } else { + vscode.window.showInformationMessage("Applied recommended SSH settings."); + } } /** diff --git a/src/remote/userSettings.ts b/src/remote/userSettings.ts index 6defd7ef..1246c1cd 100644 --- a/src/remote/userSettings.ts +++ b/src/remote/userSettings.ts @@ -1,3 +1,4 @@ +import { formatDuration, intervalToDuration } from "date-fns"; import * as jsonc from "jsonc-parser"; import * as fs from "node:fs/promises"; @@ -15,25 +16,45 @@ interface RecommendedSetting { readonly label: string; } +function recommended( + shortName: string, + value: number | null, +): RecommendedSetting { + if (value === null) { + return { value, label: `${shortName}: max allowed` }; + } + const humanized = formatDuration( + intervalToDuration({ start: 0, end: value * 1000 }), + ); + return { value, label: `${shortName}: ${humanized}` }; +} + +/** Applied by the "Apply Recommended SSH Settings" command. */ export const RECOMMENDED_SSH_SETTINGS = { - "remote.SSH.connectTimeout": { - value: 1800, - label: "Connect Timeout: 1800s (30 min)", - }, - "remote.SSH.reconnectionGraceTime": { - value: 28800, - label: "Reconnection Grace Time: 28800s (8 hours)", - }, - "remote.SSH.serverShutdownTimeout": { - value: 28800, - label: "Server Shutdown Timeout: 28800s (8 hours)", - }, - "remote.SSH.maxReconnectionAttempts": { - value: null, - label: "Max Reconnection Attempts: max allowed", - }, + "remote.SSH.connectTimeout": recommended("Connect Timeout", 1800), + "remote.SSH.reconnectionGraceTime": recommended( + "Reconnection Grace Time", + 86400, + ), + "remote.SSH.serverShutdownTimeout": recommended( + "Server Shutdown Timeout", + 86400, + ), + "remote.SSH.maxReconnectionAttempts": recommended( + "Max Reconnection Attempts", + null, + ), } as const satisfies Record; +type SshSettingKey = keyof typeof RECOMMENDED_SSH_SETTINGS; + +/** Defaults set during connection when the user hasn't configured a value. */ +const AUTO_SETUP_DEFAULTS = { + "remote.SSH.reconnectionGraceTime": 28800, // 8h + "remote.SSH.serverShutdownTimeout": 28800, // 8h + "remote.SSH.maxReconnectionAttempts": null, // max allowed +} as const satisfies Partial>; + /** * Build the list of VS Code setting overrides needed for a remote SSH * connection to a Coder workspace. @@ -58,25 +79,17 @@ export function buildSshOverrides( } // Default 15s is too short for startup scripts; enforce a minimum. - const minConnTimeout = - RECOMMENDED_SSH_SETTINGS["remote.SSH.connectTimeout"].value; - const connTimeout = config.get("remote.SSH.connectTimeout"); - if (!connTimeout || connTimeout < minConnTimeout) { - overrides.push({ - key: "remote.SSH.connectTimeout", - value: minConnTimeout, - }); + const connTimeoutKey: SshSettingKey = "remote.SSH.connectTimeout"; + const { value: minConnTimeout } = RECOMMENDED_SSH_SETTINGS[connTimeoutKey]; + const connTimeout = config.get(connTimeoutKey); + if (minConnTimeout && (!connTimeout || connTimeout < minConnTimeout)) { + overrides.push({ key: connTimeoutKey, value: minConnTimeout }); } - // Set recommended defaults for settings the user hasn't configured. - const setIfUndefined = [ - "remote.SSH.reconnectionGraceTime", - "remote.SSH.serverShutdownTimeout", - "remote.SSH.maxReconnectionAttempts", - ] as const; - for (const key of setIfUndefined) { + // Set conservative defaults for settings the user hasn't configured. + for (const [key, value] of Object.entries(AUTO_SETUP_DEFAULTS)) { if (config.get(key) === undefined) { - overrides.push({ key, value: RECOMMENDED_SSH_SETTINGS[key].value }); + overrides.push({ key, value }); } } diff --git a/test/unit/remote/userSettings.test.ts b/test/unit/remote/userSettings.test.ts index 59192432..94845671 100644 --- a/test/unit/remote/userSettings.test.ts +++ b/test/unit/remote/userSettings.test.ts @@ -229,6 +229,18 @@ describe("applySettingOverrides", () => { expect(raw).toContain('"remote.SSH.remotePlatform"'); }); + it("writes null values literally instead of deleting the key", async () => { + const ok = await applySettingOverrides( + settingsPath, + [{ key: "remote.SSH.maxReconnectionAttempts", value: null }], + logger, + ); + + expect(ok).toBe(true); + const raw = await fsPromises.readFile(settingsPath, "utf8"); + expect(raw).toContain('"remote.SSH.maxReconnectionAttempts": null'); + }); + it("returns false and logs warning when write fails", async () => { vol.fromJSON({ [settingsPath]: "{}" }); const writeSpy = vi From 340f560b0e7f611a162bd9c0ea5804f27b93461f Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Mon, 16 Mar 2026 16:08:36 +0300 Subject: [PATCH 4/4] Review comments --- CHANGELOG.md | 3 +++ src/commands.ts | 12 ++++++++++-- src/logging/logger.ts | 1 + src/remote/remote.ts | 15 ++++++++++----- src/remote/userSettings.ts | 8 ++++++-- test/mocks/testHelpers.ts | 1 + test/unit/error/serverCertificateError.test.ts | 1 + test/unit/remote/userSettings.test.ts | 4 ++-- 8 files changed, 34 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e9f3e218..6dcb88c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ ### Added +- Automatically set `reconnectionGraceTime`, `serverShutdownTimeout`, and `maxReconnectionAttempts` + on first connection to prevent disconnects during overnight workspace sleep. +- New **Coder: Apply Recommended SSH Settings** command to overwrite all recommended SSH settings at once. - 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. diff --git a/src/commands.ts b/src/commands.ts index bc4d9649..3357f456 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -337,12 +337,20 @@ export class Commands { key, value: setting.value, })); - await applySettingOverrides( + const ok = await applySettingOverrides( this.pathResolver.getUserSettingsPath(), overrides, this.logger, ); - if (this.remoteWorkspaceClient) { + if (!ok) { + const action = await vscode.window.showErrorMessage( + "Failed to write SSH settings. Check the Coder output for details.", + "Show Output", + ); + if (action === "Show Output") { + this.logger.show(); + } + } else if (this.remoteWorkspaceClient) { const action = await vscode.window.showInformationMessage( "Applied recommended SSH settings. Reload the window for changes to take effect.", "Reload Window", diff --git a/src/logging/logger.ts b/src/logging/logger.ts index 30bf0ec6..0010d758 100644 --- a/src/logging/logger.ts +++ b/src/logging/logger.ts @@ -4,4 +4,5 @@ export interface Logger { info(message: string, ...args: unknown[]): void; warn(message: string, ...args: unknown[]): void; error(message: string, ...args: unknown[]): void; + show(): void; } diff --git a/src/remote/remote.ts b/src/remote/remote.ts index ab53ea6e..d661a013 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -465,11 +465,16 @@ export class Remote { parts.sshHost, agent.operating_system, ); - await applySettingOverrides( - this.pathResolver.getUserSettingsPath(), - overrides, - this.logger, - ); + if (overrides.length > 0) { + const ok = await applySettingOverrides( + this.pathResolver.getUserSettingsPath(), + overrides, + this.logger, + ); + if (ok) { + this.logger.info("Settings modified successfully"); + } + } const logDir = this.getLogDir(featureSet); diff --git a/src/remote/userSettings.ts b/src/remote/userSettings.ts index 1246c1cd..25f11202 100644 --- a/src/remote/userSettings.ts +++ b/src/remote/userSettings.ts @@ -29,7 +29,11 @@ function recommended( return { value, label: `${shortName}: ${humanized}` }; } -/** Applied by the "Apply Recommended SSH Settings" command. */ +/** + * Applied by the "Apply Recommended SSH Settings" command. + * These are more aggressive (24h) than AUTO_SETUP_DEFAULTS (8h) because the + * user is explicitly opting in via the command palette. + */ export const RECOMMENDED_SSH_SETTINGS = { "remote.SSH.connectTimeout": recommended("Connect Timeout", 1800), "remote.SSH.reconnectionGraceTime": recommended( @@ -109,7 +113,7 @@ export async function applySettingOverrides( logger: Logger, ): Promise { if (overrides.length === 0) { - return false; + return true; } let settingsContent = "{}"; diff --git a/test/mocks/testHelpers.ts b/test/mocks/testHelpers.ts index 631a1282..9cc093f4 100644 --- a/test/mocks/testHelpers.ts +++ b/test/mocks/testHelpers.ts @@ -391,6 +391,7 @@ export function createMockLogger(): Logger { info: vi.fn(), warn: vi.fn(), error: vi.fn(), + show: vi.fn(), }; } diff --git a/test/unit/error/serverCertificateError.test.ts b/test/unit/error/serverCertificateError.test.ts index 27710d4b..622ab405 100644 --- a/test/unit/error/serverCertificateError.test.ts +++ b/test/unit/error/serverCertificateError.test.ts @@ -40,6 +40,7 @@ describe("Certificate errors", () => { info: throwingLog, warn: throwingLog, error: throwingLog, + show: () => {}, }; const disposers: Array<() => void> = []; diff --git a/test/unit/remote/userSettings.test.ts b/test/unit/remote/userSettings.test.ts index 94845671..60761c3d 100644 --- a/test/unit/remote/userSettings.test.ts +++ b/test/unit/remote/userSettings.test.ts @@ -160,8 +160,8 @@ describe("applySettingOverrides", () => { return JSON.parse(raw) as Record; } - it("returns false when overrides list is empty", async () => { - expect(await applySettingOverrides(settingsPath, [], logger)).toBe(false); + it("returns true when overrides list is empty", async () => { + expect(await applySettingOverrides(settingsPath, [], logger)).toBe(true); }); it("creates file and applies overrides when file does not exist", async () => {