Skip to content

Commit 4742f43

Browse files
committed
refactor: split window IPC by feature, simplify naming
Address review feedback: - Move OAuthCallback out of SecretsManager into src/oauth/oauthCallback.ts - Move WindowBroadcast (the generic primitive) into src/ipc/ - Rename WindowIpc to DuplicateWorkspaceIpc and move it into src/workspace/, since it covers a single workspace-coordination feature, not all window IPC - Use Zod schemas in both DuplicateWorkspaceIpc and OAuthCallback for consistent validation - Tighten tests: the broadcast key-isolation test now also asserts delivery on the matching key, and the stale-request test asserts delivery of fresh requests
1 parent a34b30d commit 4742f43

18 files changed

Lines changed: 412 additions & 351 deletions

src/commands.ts

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,3 @@
1-
import {
2-
type Workspace,
3-
type WorkspaceAgent,
4-
} from "coder/site/src/api/typesGenerated";
51
import * as fs from "node:fs/promises";
62
import * as os from "node:os";
73
import * as path from "node:path";
@@ -13,20 +9,11 @@ import {
139
extractAgents,
1410
workspaceStatusLabel,
1511
} from "./api/api-helper";
16-
import { type CoderApi } from "./api/coderApi";
1712
import * as cliExec from "./core/cliExec";
18-
import { type CliManager } from "./core/cliManager";
19-
import { type ServiceContainer } from "./core/container";
20-
import { type MementoManager } from "./core/mementoManager";
21-
import { type PathResolver } from "./core/pathResolver";
22-
import { type SecretsManager } from "./core/secretsManager";
2313
import { appendVsCodeLogs } from "./core/supportBundleLogs";
24-
import { type DeploymentManager } from "./deployment/deploymentManager";
2514
import { CertificateError } from "./error/certificateError";
2615
import { toError } from "./error/errorUtils";
2716
import { type FeatureSet, featureSetForVersion } from "./featureSet";
28-
import { type Logger } from "./logging/logger";
29-
import { type LoginCoordinator } from "./login/loginCoordinator";
3017
import { withCancellableProgress, withProgress } from "./progress";
3118
import { maybeAskAgent, maybeAskUrl } from "./promptUtils";
3219
import {
@@ -36,13 +23,31 @@ import {
3623
import { resolveCliAuth } from "./settings/cli";
3724
import { toRemoteAuthority, toSafeHost } from "./util";
3825
import { vscodeProposed } from "./vscodeProposed";
39-
import { type PongMessage, type WindowIpc } from "./windowIpc";
4026
import {
4127
AgentTreeItem,
4228
type OpenableTreeItem,
4329
WorkspaceTreeItem,
4430
} from "./workspace/workspacesProvider";
4531

32+
import type {
33+
Workspace,
34+
WorkspaceAgent,
35+
} from "coder/site/src/api/typesGenerated";
36+
37+
import type { CoderApi } from "./api/coderApi";
38+
import type { CliManager } from "./core/cliManager";
39+
import type { ServiceContainer } from "./core/container";
40+
import type { MementoManager } from "./core/mementoManager";
41+
import type { PathResolver } from "./core/pathResolver";
42+
import type { SecretsManager } from "./core/secretsManager";
43+
import type { DeploymentManager } from "./deployment/deploymentManager";
44+
import type { Logger } from "./logging/logger";
45+
import type { LoginCoordinator } from "./login/loginCoordinator";
46+
import type {
47+
DuplicateWorkspaceIpc,
48+
PongMessage,
49+
} from "./workspace/duplicateWorkspaceIpc";
50+
4651
interface OpenOptions {
4752
workspaceOwner?: string;
4853
workspaceName?: string;
@@ -66,7 +71,7 @@ export class Commands {
6671
private readonly secretsManager: SecretsManager;
6772
private readonly cliManager: CliManager;
6873
private readonly loginCoordinator: LoginCoordinator;
69-
private readonly windowIpc: WindowIpc;
74+
private readonly duplicateWorkspaceIpc: DuplicateWorkspaceIpc;
7075

7176
// These will only be populated when actively connected to a workspace and are
7277
// used in commands. Because commands can be executed by the user, it is not
@@ -90,7 +95,7 @@ export class Commands {
9095
this.secretsManager = serviceContainer.getSecretsManager();
9196
this.cliManager = serviceContainer.getCliManager();
9297
this.loginCoordinator = serviceContainer.getLoginCoordinator();
93-
this.windowIpc = serviceContainer.getWindowIpc();
98+
this.duplicateWorkspaceIpc = serviceContainer.getDuplicateWorkspaceIpc();
9499
}
95100

96101
/**
@@ -1083,8 +1088,9 @@ export class Commands {
10831088
// Only set the memento when opening a new folder/window
10841089
await this.mementoManager.setStartupMode("start");
10851090

1086-
// Best-effort: detect other connected windows in the background.
1087-
this.windowIpc
1091+
// Best-effort check for an already-connected window. Runs in the
1092+
// background so it never delays openFolder.
1093+
this.duplicateWorkspaceIpc
10881094
.sendPing(remoteAuthority)
10891095
.then((pong) => {
10901096
if (pong) {
@@ -1117,10 +1123,8 @@ export class Commands {
11171123
return true;
11181124
}
11191125

1120-
/**
1121-
* Non-blocking notification — VS Code may dismiss it at any time,
1122-
* so this must not be awaited.
1123-
*/
1126+
// VS Code may dismiss a non-modal info message without resolving the
1127+
// thenable, so this must not be awaited from the open path.
11241128
private showMultiWindowNotification(
11251129
pong: PongMessage,
11261130
remoteAuthority: string,
@@ -1129,13 +1133,13 @@ export class Commands {
11291133
const openEmptyAction = "Open Without Folder";
11301134
vscode.window
11311135
.showInformationMessage(
1132-
`A window is already connected to this workspace (${pong.folder}).`,
1136+
"This workspace is already open in another window.",
11331137
duplicateAction,
11341138
openEmptyAction,
11351139
)
11361140
.then(async (choice) => {
11371141
if (choice === duplicateAction) {
1138-
await this.windowIpc.sendDuplicate(pong.sessionId);
1142+
await this.duplicateWorkspaceIpc.sendDuplicate(pong.sessionId);
11391143
} else if (choice === openEmptyAction) {
11401144
await vscode.commands.executeCommand("vscode.newWindow", {
11411145
remoteAuthority,

src/core/container.ts

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import * as vscode from "vscode";
22

33
import { CoderApi } from "../api/coderApi";
4-
import { type Logger } from "../logging/logger";
54
import { LoginCoordinator } from "../login/loginCoordinator";
6-
import { WindowIpc } from "../windowIpc";
5+
import { OAuthCallback } from "../oauth/oauthCallback";
6+
import { DuplicateWorkspaceIpc } from "../workspace/duplicateWorkspaceIpc";
77

88
import { CliCredentialManager } from "./cliCredentialManager";
99
import { CliManager } from "./cliManager";
@@ -12,6 +12,8 @@ import { MementoManager } from "./mementoManager";
1212
import { PathResolver } from "./pathResolver";
1313
import { SecretsManager } from "./secretsManager";
1414

15+
import type { Logger } from "../logging/logger";
16+
1517
/**
1618
* Service container for dependency injection.
1719
* Centralizes the creation and management of all core services.
@@ -25,7 +27,8 @@ export class ServiceContainer implements vscode.Disposable {
2527
private readonly cliManager: CliManager;
2628
private readonly contextManager: ContextManager;
2729
private readonly loginCoordinator: LoginCoordinator;
28-
private readonly windowIpc: WindowIpc;
30+
private readonly duplicateWorkspaceIpc: DuplicateWorkspaceIpc;
31+
private readonly oauthCallback: OAuthCallback;
2932

3033
constructor(context: vscode.ExtensionContext) {
3134
this.logger = vscode.window.createOutputChannel("Coder", { log: true });
@@ -65,14 +68,19 @@ export class ServiceContainer implements vscode.Disposable {
6568
this.cliCredentialManager,
6669
);
6770
this.contextManager = new ContextManager(context);
71+
this.oauthCallback = new OAuthCallback(context.secrets, this.logger);
6872
this.loginCoordinator = new LoginCoordinator(
6973
this.secretsManager,
7074
this.mementoManager,
7175
this.logger,
7276
this.cliCredentialManager,
77+
this.oauthCallback,
7378
context.extension.id,
7479
);
75-
this.windowIpc = new WindowIpc(context.secrets, this.logger);
80+
this.duplicateWorkspaceIpc = new DuplicateWorkspaceIpc(
81+
context.secrets,
82+
this.logger,
83+
);
7684
}
7785

7886
getPathResolver(): PathResolver {
@@ -107,8 +115,12 @@ export class ServiceContainer implements vscode.Disposable {
107115
return this.loginCoordinator;
108116
}
109117

110-
getWindowIpc(): WindowIpc {
111-
return this.windowIpc;
118+
getDuplicateWorkspaceIpc(): DuplicateWorkspaceIpc {
119+
return this.duplicateWorkspaceIpc;
120+
}
121+
122+
getOAuthCallback(): OAuthCallback {
123+
return this.oauthCallback;
112124
}
113125

114126
/**

src/core/secretsManager.ts

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { z } from "zod";
22

33
import { DeploymentSchema, type Deployment } from "../deployment/types";
44
import { toSafeHost } from "../util";
5-
import { WindowBroadcast } from "../windowBroadcast";
65

76
import type { OAuth2ClientRegistrationResponse } from "coder/site/src/api/typesGenerated";
87
import type { Memento, SecretStorage, Disposable } from "vscode";
@@ -17,7 +16,6 @@ const DEPLOYMENT_ACCESS_PREFIX = "coder.access.";
1716

1817
type SecretKeyPrefix = typeof SESSION_KEY_PREFIX | typeof OAUTH_CLIENT_PREFIX;
1918

20-
const OAUTH_CALLBACK_KEY = "coder.oauthCallback";
2119
const CURRENT_DEPLOYMENT_KEY = "coder.currentDeployment";
2220
const DEFAULT_MAX_DEPLOYMENTS = 10;
2321

@@ -52,30 +50,12 @@ const SessionAuthSchema = z.object({
5250

5351
export type SessionAuth = z.infer<typeof SessionAuthSchema>;
5452

55-
const OAuthCallbackDataSchema = z.object({
56-
state: z.string(),
57-
code: z.string().nullable(),
58-
error: z.string().nullable(),
59-
});
60-
61-
export type OAuthCallbackData = z.infer<typeof OAuthCallbackDataSchema>;
62-
6353
export class SecretsManager {
64-
public readonly oauthCallback: WindowBroadcast<OAuthCallbackData>;
65-
6654
constructor(
6755
private readonly secrets: SecretStorage,
6856
private readonly memento: Memento,
6957
private readonly logger: Logger,
70-
) {
71-
this.oauthCallback = new WindowBroadcast(
72-
secrets,
73-
OAUTH_CALLBACK_KEY,
74-
(v: unknown): v is OAuthCallbackData =>
75-
OAuthCallbackDataSchema.safeParse(v).success,
76-
logger,
77-
);
78-
}
58+
) {}
7959

8060
private buildKey(prefix: SecretKeyPrefix, safeHostname: string): string {
8161
return `${prefix}${safeHostname || "<legacy>"}`;

src/extension.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -336,19 +336,16 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
336336
const remote = new Remote(serviceContainer, commands, ctx);
337337

338338
// Respond to PINGs and DUPLICATE commands from other windows.
339-
const windowIpc = serviceContainer.getWindowIpc();
339+
const duplicateWorkspaceIpc = serviceContainer.getDuplicateWorkspaceIpc();
340340
ctx.subscriptions.push(
341-
windowIpc.onRequest(async (msg) => {
341+
duplicateWorkspaceIpc.onRequest(async (msg) => {
342342
const currentAuthority = vscodeProposed.env.remoteAuthority;
343343
if (!currentAuthority) {
344344
return;
345345
}
346346

347347
if (msg.type === "ping" && msg.authority === currentAuthority) {
348-
// Only the first folder matters for dedup; multi-root
349-
// workspaces use a different URI scheme.
350-
const folder = vscode.workspace.workspaceFolders?.[0]?.uri.path ?? "";
351-
await windowIpc.sendPong(msg.id, vscode.env.sessionId, folder);
348+
await duplicateWorkspaceIpc.sendPong(msg.id, vscode.env.sessionId);
352349
}
353350

354351
if (
Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,18 @@
11
import type { Disposable, SecretStorage } from "vscode";
2+
import type { ZodType } from "zod";
23

3-
import type { Logger } from "./logging/logger";
4+
import type { Logger } from "../logging/logger";
45

56
/**
6-
* Typed pub/sub over a single SecretStorage key.
7-
*
8-
* SecretStorage.onDidChange fires across all VS Code windows, so each
9-
* WindowBroadcast instance acts as a cross-window channel for messages
10-
* of type T.
7+
* Typed pub/sub over a single SecretStorage key. SecretStorage.onDidChange
8+
* fires across all VS Code windows, so each instance is a cross-window
9+
* channel for messages of type T.
1110
*/
1211
export class WindowBroadcast<T> {
1312
constructor(
1413
private readonly secrets: SecretStorage,
1514
private readonly key: string,
16-
private readonly validate: (value: unknown) => value is T,
15+
private readonly schema: ZodType<T>,
1716
private readonly logger: Logger,
1817
) {}
1918

@@ -31,11 +30,11 @@ export class WindowBroadcast<T> {
3130
if (!raw) {
3231
return;
3332
}
34-
const parsed: unknown = JSON.parse(raw);
35-
if (!this.validate(parsed)) {
33+
const parsed = this.schema.safeParse(JSON.parse(raw));
34+
if (!parsed.success) {
3635
return;
3736
}
38-
await handler(parsed);
37+
await handler(parsed.data);
3938
} catch (err) {
4039
this.logger.error(`Error handling broadcast on ${this.key}`, err);
4140
}

src/login/loginCoordinator.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import type { MementoManager } from "../core/mementoManager";
1919
import type { OAuthTokenData, SecretsManager } from "../core/secretsManager";
2020
import type { Deployment } from "../deployment/types";
2121
import type { Logger } from "../logging/logger";
22+
import type { OAuthCallback } from "../oauth/oauthCallback";
2223

2324
type LoginResult =
2425
| { success: false }
@@ -43,10 +44,12 @@ export class LoginCoordinator implements vscode.Disposable {
4344
private readonly mementoManager: MementoManager,
4445
private readonly logger: Logger,
4546
private readonly cliCredentialManager: CliCredentialManager,
47+
oauthCallback: OAuthCallback,
4648
extensionId: string,
4749
) {
4850
this.oauthAuthorizer = new OAuthAuthorizer(
4951
secretsManager,
52+
oauthCallback,
5053
logger,
5154
extensionId,
5255
);

src/oauth/authorizer.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
1-
import { type AxiosInstance } from "axios";
21
import * as vscode from "vscode";
32

43
import { CoderApi } from "../api/coderApi";
5-
import { type SecretsManager } from "../core/secretsManager";
6-
import { type Deployment } from "../deployment/types";
7-
import { type Logger } from "../logging/logger";
84

95
import {
106
AUTH_GRANT_TYPE,
@@ -21,6 +17,7 @@ import {
2117
toUrlSearchParams,
2218
} from "./utils";
2319

20+
import type { AxiosInstance } from "axios";
2421
import type {
2522
OAuth2AuthorizationServerMetadata,
2623
OAuth2ClientRegistrationRequest,
@@ -30,6 +27,12 @@ import type {
3027
User,
3128
} from "coder/site/src/api/typesGenerated";
3229

30+
import type { SecretsManager } from "../core/secretsManager";
31+
import type { Deployment } from "../deployment/types";
32+
import type { Logger } from "../logging/logger";
33+
34+
import type { OAuthCallback } from "./oauthCallback";
35+
3336
/**
3437
* Handles the OAuth authorization code flow for authenticating with Coder deployments.
3538
* Encapsulates client registration, PKCE challenge, and token exchange.
@@ -39,6 +42,7 @@ export class OAuthAuthorizer implements vscode.Disposable {
3942

4043
constructor(
4144
private readonly secretsManager: SecretsManager,
45+
private readonly oauthCallback: OAuthCallback,
4246
private readonly logger: Logger,
4347
private readonly extensionId: string,
4448
) {}
@@ -247,7 +251,7 @@ export class OAuthAuthorizer implements vscode.Disposable {
247251
timeoutMins * 60 * 1000,
248252
);
249253

250-
const listener = this.secretsManager.oauthCallback.onReceive(
254+
const listener = this.oauthCallback.onReceive(
251255
({ state: callbackState, code, error }) => {
252256
if (callbackState !== state) {
253257
this.logger.warn(

0 commit comments

Comments
 (0)