Skip to content

Commit ee02756

Browse files
committed
Add WebSocket reconnection on configuration changes
WebSockets now automatically reconnect when relevant settings change (proxy, TLS, header command, etc.). Previously, only credential changes (host, and token) triggered reconnection.
1 parent 1b6a65d commit ee02756

File tree

7 files changed

+274
-42
lines changed

7 files changed

+274
-42
lines changed

src/api/coderApi.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
import * as vscode from "vscode";
1717
import { type ClientOptions } from "ws";
1818

19+
import { watchConfigurationChanges } from "../configWatcher";
1920
import { CertificateError } from "../error";
2021
import { getHeaderCommand, getHeaders } from "../headers";
2122
import { EventStreamLogger } from "../logging/eventStreamLogger";
@@ -51,6 +52,21 @@ import { createHttpAgent } from "./utils";
5152

5253
const coderSessionTokenHeader = "Coder-Session-Token";
5354

55+
/**
56+
* Configuration settings that affect WebSocket connections.
57+
* Changes to these settings will trigger WebSocket reconnection.
58+
*/
59+
const webSocketConfigSettings = [
60+
"coder.headerCommand",
61+
"coder.insecure",
62+
"coder.tlsCertFile",
63+
"coder.tlsKeyFile",
64+
"coder.tlsCaFile",
65+
"coder.tlsAltHost",
66+
"http.proxy",
67+
"coder.proxyBypass",
68+
] as const;
69+
5470
/**
5571
* Unified API class that includes both REST API methods from the base Api class
5672
* and WebSocket methods for real-time functionality.
@@ -59,9 +75,11 @@ export class CoderApi extends Api implements vscode.Disposable {
5975
private readonly reconnectingSockets = new Set<
6076
ReconnectingWebSocket<never>
6177
>();
78+
private readonly configWatcher: vscode.Disposable;
6279

6380
private constructor(private readonly output: Logger) {
6481
super();
82+
this.configWatcher = this.watchConfigChanges();
6583
}
6684

6785
/**
@@ -133,12 +151,34 @@ export class CoderApi extends Api implements vscode.Disposable {
133151
* This clears handlers and prevents reconnection.
134152
*/
135153
dispose(): void {
154+
this.configWatcher.dispose();
136155
for (const socket of this.reconnectingSockets) {
137156
socket.close();
138157
}
139158
this.reconnectingSockets.clear();
140159
}
141160

161+
/**
162+
* Watch for configuration changes that affect WebSocket connections.
163+
* When any watched setting changes, all active WebSockets are reconnected.
164+
*/
165+
private watchConfigChanges(): vscode.Disposable {
166+
const settings = webSocketConfigSettings.map((setting) => ({
167+
setting,
168+
getValue: () => vscode.workspace.getConfiguration().get(setting),
169+
}));
170+
return watchConfigurationChanges(settings, () => {
171+
if (this.reconnectingSockets.size > 0) {
172+
this.output.info(
173+
`Configuration changed, reconnecting ${this.reconnectingSockets.size} WebSocket(s)`,
174+
);
175+
for (const socket of this.reconnectingSockets) {
176+
socket.reconnect();
177+
}
178+
}
179+
});
180+
}
181+
142182
watchInboxNotifications = async (
143183
watchTemplates: string[],
144184
watchTargets: string[],

src/configWatcher.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import { isDeepStrictEqual } from "node:util";
2+
import * as vscode from "vscode";
3+
4+
export interface WatchedSetting {
5+
setting: string;
6+
getValue: () => unknown;
7+
}
8+
9+
/**
10+
* Watch for configuration changes and invoke a callback when values change.
11+
* Only fires when actual values change, not just when settings are touched.
12+
*/
13+
export function watchConfigurationChanges(
14+
settings: WatchedSetting[],
15+
onChange: (changedSettings: string[]) => void,
16+
): vscode.Disposable {
17+
const appliedValues = new Map(settings.map((s) => [s.setting, s.getValue()]));
18+
19+
return vscode.workspace.onDidChangeConfiguration((e) => {
20+
const changedSettings: string[] = [];
21+
22+
for (const { setting, getValue } of settings) {
23+
if (!e.affectsConfiguration(setting)) {
24+
continue;
25+
}
26+
27+
const newValue = getValue();
28+
29+
if (!isDeepStrictEqual(newValue, appliedValues.get(setting))) {
30+
changedSettings.push(setting);
31+
appliedValues.set(setting, newValue);
32+
}
33+
}
34+
35+
if (changedSettings.length > 0) {
36+
onChange(changedSettings);
37+
}
38+
});
39+
}

src/remote/remote.ts

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import * as jsonc from "jsonc-parser";
88
import * as fs from "node:fs/promises";
99
import * as os from "node:os";
1010
import * as path from "node:path";
11-
import { isDeepStrictEqual } from "node:util";
1211
import * as semver from "semver";
1312
import * as vscode from "vscode";
1413

@@ -23,6 +22,7 @@ import { CoderApi } from "../api/coderApi";
2322
import { needToken } from "../api/utils";
2423
import { getGlobalFlags, getGlobalFlagsRaw, getSshFlags } from "../cliConfig";
2524
import { type Commands } from "../commands";
25+
import { watchConfigurationChanges } from "../configWatcher";
2626
import { type CliManager } from "../core/cliManager";
2727
import * as cliUtils from "../core/cliUtils";
2828
import { type ServiceContainer } from "../core/container";
@@ -854,29 +854,10 @@ export class Remote {
854854
getValue: () => unknown;
855855
}>,
856856
): vscode.Disposable {
857-
// Capture applied values at setup time
858-
const appliedValues = new Map(
859-
settings.map((s) => [s.setting, s.getValue()]),
860-
);
861-
862-
return vscode.workspace.onDidChangeConfiguration((e) => {
863-
const changedTitles: string[] = [];
864-
865-
for (const { setting, title, getValue } of settings) {
866-
if (!e.affectsConfiguration(setting)) {
867-
continue;
868-
}
869-
870-
const newValue = getValue();
857+
const titleMap = new Map(settings.map((s) => [s.setting, s.title]));
871858

872-
if (!isDeepStrictEqual(newValue, appliedValues.get(setting))) {
873-
changedTitles.push(title);
874-
}
875-
}
876-
877-
if (changedTitles.length === 0) {
878-
return;
879-
}
859+
return watchConfigurationChanges(settings, (changedSettings) => {
860+
const changedTitles = changedSettings.map((s) => titleMap.get(s)!);
880861

881862
const message =
882863
changedTitles.length === 1

test/mocks/testHelpers.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@ export class MockConfigurationProvider {
2020

2121
/**
2222
* Set a configuration value that will be returned by vscode.workspace.getConfiguration().get()
23+
* Automatically fires onDidChangeConfiguration event (emulating VS Code behavior).
2324
*/
2425
set(key: string, value: unknown): void {
2526
this.config.set(key, value);
27+
this.fireConfigChangeEvent(key);
2628
}
2729

2830
/**
@@ -42,6 +44,23 @@ export class MockConfigurationProvider {
4244
this.config.clear();
4345
}
4446

47+
/**
48+
* Fire configuration change event for a specific setting.
49+
*/
50+
private fireConfigChangeEvent(setting: string): void {
51+
const fireEvent = (
52+
vscode.workspace as typeof vscode.workspace & {
53+
__fireDidChangeConfiguration: (
54+
e: vscode.ConfigurationChangeEvent,
55+
) => void;
56+
}
57+
).__fireDidChangeConfiguration;
58+
59+
fireEvent({
60+
affectsConfiguration: (section: string) => section === setting,
61+
});
62+
}
63+
4564
/**
4665
* Setup the vscode.workspace.getConfiguration mock to return our values
4766
*/
@@ -63,7 +82,7 @@ export class MockConfigurationProvider {
6382
}),
6483
inspect: vi.fn(),
6584
update: vi.fn((key: string, value: unknown) => {
66-
this.config.set(getFullKey(key), value);
85+
this.set(getFullKey(key), value);
6786
return Promise.resolve();
6887
}),
6988
};

test/mocks/vscode.runtime.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,10 @@ export const workspace = {
120120
onDidChangeWorkspaceFolders: onDidChangeWorkspaceFolders.event,
121121

122122
// test-only triggers:
123-
__fireDidChangeConfiguration: onDidChangeConfiguration.fire,
124-
__fireDidChangeWorkspaceFolders: onDidChangeWorkspaceFolders.fire,
123+
__fireDidChangeConfiguration: (e: unknown) =>
124+
onDidChangeConfiguration.fire(e),
125+
__fireDidChangeWorkspaceFolders: (e: unknown) =>
126+
onDidChangeWorkspaceFolders.fire(e),
125127
};
126128

127129
export const env = {

test/unit/api/coderApi.test.ts

Lines changed: 70 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,11 @@ describe("CoderApi", () => {
8787
mockConfig.set("coder.httpClientLogLevel", "BASIC");
8888
});
8989

90+
afterEach(() => {
91+
// Dispose any api created during the test to clean up config watchers
92+
api?.dispose();
93+
});
94+
9095
describe("HTTP Interceptors", () => {
9196
it("adds custom headers and HTTP agent to requests", async () => {
9297
const mockAgent = new ProxyAgent();
@@ -429,24 +434,24 @@ describe("CoderApi", () => {
429434
});
430435
});
431436

432-
describe("Reconnection on Host/Token Changes", () => {
433-
const setupAutoOpeningWebSocket = () => {
434-
const sockets: Array<Partial<Ws>> = [];
435-
vi.mocked(Ws).mockImplementation((url: string | URL) => {
436-
const mockWs = createMockWebSocket(String(url), {
437-
on: vi.fn((event, handler) => {
438-
if (event === "open") {
439-
setImmediate(() => handler());
440-
}
441-
return mockWs as Ws;
442-
}),
443-
});
444-
sockets.push(mockWs);
445-
return mockWs as Ws;
437+
const setupAutoOpeningWebSocket = () => {
438+
const sockets: Array<Partial<Ws>> = [];
439+
vi.mocked(Ws).mockImplementation((url: string | URL) => {
440+
const mockWs = createMockWebSocket(String(url), {
441+
on: vi.fn((event, handler) => {
442+
if (event === "open") {
443+
setImmediate(() => handler());
444+
}
445+
return mockWs as Ws;
446+
}),
446447
});
447-
return sockets;
448-
};
448+
sockets.push(mockWs);
449+
return mockWs as Ws;
450+
});
451+
return sockets;
452+
};
449453

454+
describe("Reconnection on Host/Token Changes", () => {
450455
it("triggers reconnection when session token changes", async () => {
451456
const sockets = setupAutoOpeningWebSocket();
452457
api = createApi(CODER_URL, AXIOS_TOKEN);
@@ -593,6 +598,55 @@ describe("CoderApi", () => {
593598
expect(sockets[0].close).toHaveBeenCalled();
594599
});
595600
});
601+
602+
describe("Configuration Change Reconnection", () => {
603+
const tick = () => new Promise((resolve) => setImmediate(resolve));
604+
605+
it("reconnects sockets when watched config value changes", async () => {
606+
mockConfig.set("coder.insecure", false);
607+
const sockets = setupAutoOpeningWebSocket();
608+
api = createApi(CODER_URL, AXIOS_TOKEN);
609+
await api.watchAgentMetadata(AGENT_ID);
610+
611+
mockConfig.set("coder.insecure", true);
612+
await tick();
613+
614+
expect(sockets[0].close).toHaveBeenCalledWith(
615+
1000,
616+
"Replacing connection",
617+
);
618+
expect(sockets).toHaveLength(2);
619+
});
620+
621+
it.each([
622+
["unchanged value", "coder.insecure", false],
623+
["unrelated setting", "unrelated.setting", "new-value"],
624+
])("does not reconnect for %s", async (_desc, key, value) => {
625+
mockConfig.set("coder.insecure", false);
626+
const sockets = setupAutoOpeningWebSocket();
627+
api = createApi(CODER_URL, AXIOS_TOKEN);
628+
await api.watchAgentMetadata(AGENT_ID);
629+
630+
mockConfig.set(key, value);
631+
await tick();
632+
633+
expect(sockets[0].close).not.toHaveBeenCalled();
634+
expect(sockets).toHaveLength(1);
635+
});
636+
637+
it("stops watching after dispose", async () => {
638+
mockConfig.set("coder.insecure", false);
639+
const sockets = setupAutoOpeningWebSocket();
640+
api = createApi(CODER_URL, AXIOS_TOKEN);
641+
await api.watchAgentMetadata(AGENT_ID);
642+
643+
api.dispose();
644+
mockConfig.set("coder.insecure", true);
645+
await tick();
646+
647+
expect(sockets).toHaveLength(1);
648+
});
649+
});
596650
});
597651

598652
const mockAdapterImpl = vi.hoisted(() => (config: Record<string, unknown>) => {

0 commit comments

Comments
 (0)