From 88d8219713aad1125c2ab3b1463afc199837bed8 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Mon, 27 Apr 2026 13:37:15 +0000 Subject: [PATCH 1/4] feat: support multiple VS Code windows to the same workspace Add WindowBroadcast, a generic typed pub/sub channel over SecretStorage.onDidChange for transient cross-window messaging. Build WindowIpc on top of two broadcast channels to detect when multiple windows are connected to the same Coder workspace. When a user clicks Connect and another window responds to the PING, a non-blocking notification offers Duplicate Window or Open Without Folder. The entire chain is fire-and-forget; openWorkspace() never blocks on it. Closes #783 Closes #548 --- src/commands.ts | 50 ++++++++++ src/core/container.ts | 7 ++ src/extension.ts | 27 +++++ src/windowBroadcast.ts | 44 ++++++++ src/windowIpc.ts | 161 ++++++++++++++++++++++++++++++ test/unit/windowBroadcast.test.ts | 91 +++++++++++++++++ test/unit/windowIpc.test.ts | 94 +++++++++++++++++ 7 files changed, 474 insertions(+) create mode 100644 src/windowBroadcast.ts create mode 100644 src/windowIpc.ts create mode 100644 test/unit/windowBroadcast.test.ts create mode 100644 test/unit/windowIpc.test.ts diff --git a/src/commands.ts b/src/commands.ts index 3155b87069..a4e14d10cb 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -36,6 +36,7 @@ import { import { resolveCliAuth } from "./settings/cli"; import { toRemoteAuthority, toSafeHost } from "./util"; import { vscodeProposed } from "./vscodeProposed"; +import { type PongMessage, type WindowIpc } from "./windowIpc"; import { AgentTreeItem, type OpenableTreeItem, @@ -65,6 +66,7 @@ export class Commands { private readonly secretsManager: SecretsManager; private readonly cliManager: CliManager; private readonly loginCoordinator: LoginCoordinator; + private readonly windowIpc: WindowIpc; // These will only be populated when actively connected to a workspace and are // used in commands. Because commands can be executed by the user, it is not @@ -88,6 +90,7 @@ export class Commands { this.secretsManager = serviceContainer.getSecretsManager(); this.cliManager = serviceContainer.getCliManager(); this.loginCoordinator = serviceContainer.getLoginCoordinator(); + this.windowIpc = serviceContainer.getWindowIpc(); } /** @@ -1079,6 +1082,19 @@ export class Commands { // Only set the memento when opening a new folder/window await this.mementoManager.setStartupMode("start"); + + // Best-effort: detect other connected windows in the background. + this.windowIpc + .sendPing(remoteAuthority) + .then((pong) => { + if (pong) { + this.showMultiWindowNotification(pong, remoteAuthority); + } + }) + .catch((err: unknown) => { + this.logger.error(`IPC ping failed for ${remoteAuthority}`, err); + }); + if (folderPath) { await vscode.commands.executeCommand( "vscode.openFolder", @@ -1100,6 +1116,40 @@ export class Commands { }); return true; } + + /** + * Non-blocking notification — VS Code may dismiss it at any time, + * so this must not be awaited. + */ + private showMultiWindowNotification( + pong: PongMessage, + remoteAuthority: string, + ): void { + const duplicateAction = "Duplicate Window"; + const openEmptyAction = "Open Without Folder"; + vscode.window + .showInformationMessage( + `A window is already connected to this workspace (${pong.folder}).`, + duplicateAction, + openEmptyAction, + ) + .then(async (choice) => { + if (choice === duplicateAction) { + await this.windowIpc.sendDuplicate(pong.sessionId); + } else if (choice === openEmptyAction) { + await vscode.commands.executeCommand("vscode.newWindow", { + remoteAuthority, + reuseWindow: false, + }); + } + }) + .then(undefined, (err: unknown) => { + this.logger.error( + `Multi-window notification failed for ${remoteAuthority}`, + err, + ); + }); + } } async function openFile(filePath: string): Promise { diff --git a/src/core/container.ts b/src/core/container.ts index ce8ca887e2..335b82ba41 100644 --- a/src/core/container.ts +++ b/src/core/container.ts @@ -3,6 +3,7 @@ import * as vscode from "vscode"; import { CoderApi } from "../api/coderApi"; import { type Logger } from "../logging/logger"; import { LoginCoordinator } from "../login/loginCoordinator"; +import { WindowIpc } from "../windowIpc"; import { CliCredentialManager } from "./cliCredentialManager"; import { CliManager } from "./cliManager"; @@ -24,6 +25,7 @@ export class ServiceContainer implements vscode.Disposable { private readonly cliManager: CliManager; private readonly contextManager: ContextManager; private readonly loginCoordinator: LoginCoordinator; + private readonly windowIpc: WindowIpc; constructor(context: vscode.ExtensionContext) { this.logger = vscode.window.createOutputChannel("Coder", { log: true }); @@ -70,6 +72,7 @@ export class ServiceContainer implements vscode.Disposable { this.cliCredentialManager, context.extension.id, ); + this.windowIpc = new WindowIpc(context.secrets, this.logger); } getPathResolver(): PathResolver { @@ -104,6 +107,10 @@ export class ServiceContainer implements vscode.Disposable { return this.loginCoordinator; } + getWindowIpc(): WindowIpc { + return this.windowIpc; + } + /** * Dispose of all services and clean up resources. */ diff --git a/src/extension.ts b/src/extension.ts index 3b54d1c033..7284b0df42 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -335,6 +335,33 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { const remote = new Remote(serviceContainer, commands, ctx); + // Respond to PINGs and DUPLICATE commands from other windows. + const windowIpc = serviceContainer.getWindowIpc(); + ctx.subscriptions.push( + windowIpc.onRequest(async (msg) => { + const currentAuthority = vscodeProposed.env.remoteAuthority; + if (!currentAuthority) { + return; + } + + if (msg.type === "ping" && msg.authority === currentAuthority) { + // Only the first folder matters for dedup; multi-root + // workspaces use a different URI scheme. + const folder = vscode.workspace.workspaceFolders?.[0]?.uri.path ?? ""; + await windowIpc.sendPong(msg.id, vscode.env.sessionId, folder); + } + + if ( + msg.type === "duplicate" && + msg.targetSessionId === vscode.env.sessionId + ) { + await vscode.commands.executeCommand( + "workbench.action.duplicateWorkspaceInNewWindow", + ); + } + }), + ); + // Since the "onResolveRemoteAuthority:ssh-remote" activation event exists // in package.json we're able to perform actions before the authority is // resolved by the remote SSH extension. diff --git a/src/windowBroadcast.ts b/src/windowBroadcast.ts new file mode 100644 index 0000000000..aa66cc668b --- /dev/null +++ b/src/windowBroadcast.ts @@ -0,0 +1,44 @@ +import type { Disposable, SecretStorage } from "vscode"; + +import type { Logger } from "./logging/logger"; + +/** + * Typed pub/sub over a single SecretStorage key. + * + * SecretStorage.onDidChange fires across all VS Code windows, so each + * WindowBroadcast instance acts as a cross-window channel for messages + * of type T. + */ +export class WindowBroadcast { + constructor( + private readonly secrets: SecretStorage, + private readonly key: string, + private readonly validate: (value: unknown) => value is T, + private readonly logger: Logger, + ) {} + + async send(msg: T): Promise { + await this.secrets.store(this.key, JSON.stringify(msg)); + } + + onReceive(handler: (msg: T) => void | Promise): Disposable { + return this.secrets.onDidChange(async (e) => { + if (e.key !== this.key) { + return; + } + try { + const raw = await this.secrets.get(this.key); + if (!raw) { + return; + } + const parsed: unknown = JSON.parse(raw); + if (!this.validate(parsed)) { + return; + } + await handler(parsed); + } catch (err) { + this.logger.error(`Error handling broadcast on ${this.key}`, err); + } + }); + } +} diff --git a/src/windowIpc.ts b/src/windowIpc.ts new file mode 100644 index 0000000000..4ca7382b28 --- /dev/null +++ b/src/windowIpc.ts @@ -0,0 +1,161 @@ +import crypto from "node:crypto"; + +import { WindowBroadcast } from "./windowBroadcast"; + +import type { Disposable, SecretStorage } from "vscode"; + +import type { Logger } from "./logging/logger"; + +const MESSAGE_MAX_AGE_MS = 5000; +const DEFAULT_PING_TIMEOUT_MS = 1000; + +export interface PingMessage { + type: "ping"; + id: string; + authority: string; + ts: number; +} + +export interface PongMessage { + type: "pong"; + id: string; + sessionId: string; + folder: string; + ts: number; +} + +export interface DuplicateMessage { + type: "duplicate"; + id: string; + targetSessionId: string; + ts: number; +} + +type RequestMessage = PingMessage | DuplicateMessage; + +function isRequestMessage(msg: unknown): msg is RequestMessage { + if (typeof msg !== "object" || msg === null) { + return false; + } + const obj = msg as Record; + if (typeof obj.id !== "string" || typeof obj.ts !== "number") { + return false; + } + if (obj.type === "ping") { + return typeof obj.authority === "string"; + } + if (obj.type === "duplicate") { + return typeof obj.targetSessionId === "string"; + } + return false; +} + +function isPongMessage(msg: unknown): msg is PongMessage { + if (typeof msg !== "object" || msg === null) { + return false; + } + const obj = msg as Record; + return ( + obj.type === "pong" && + typeof obj.id === "string" && + typeof obj.sessionId === "string" && + typeof obj.folder === "string" && + typeof obj.ts === "number" + ); +} + +/** Cross-window IPC built on WindowBroadcast channels. */ +export class WindowIpc { + private readonly requests: WindowBroadcast; + private readonly responses: WindowBroadcast; + + constructor( + secrets: SecretStorage, + private readonly logger: Logger, + ) { + this.requests = new WindowBroadcast( + secrets, + "coder.ipc.req", + isRequestMessage, + logger, + ); + this.responses = new WindowBroadcast( + secrets, + "coder.ipc.res", + isPongMessage, + logger, + ); + } + + /** Send a PING and wait for a PONG within the timeout. */ + sendPing( + authority: string, + timeoutMs = DEFAULT_PING_TIMEOUT_MS, + ): Promise { + const id = crypto.randomUUID(); + + return new Promise((resolve) => { + let settled = false; + + const settle = (result: PongMessage | undefined) => { + if (settled) { + return; + } + settled = true; + clearTimeout(timer); + listener.dispose(); + resolve(result); + }; + + const listener = this.responses.onReceive((msg) => { + if (msg.id === id) { + settle(msg); + } + }); + + const timer = setTimeout(() => settle(undefined), timeoutMs); + + this.requests + .send({ type: "ping", id, authority, ts: Date.now() }) + .then(undefined, (err: unknown) => { + this.logger.error("Failed to send IPC ping", err); + settle(undefined); + }); + }); + } + + async sendPong( + pingId: string, + sessionId: string, + folder: string, + ): Promise { + await this.responses.send({ + type: "pong", + id: pingId, + sessionId, + folder, + ts: Date.now(), + }); + } + + async sendDuplicate(targetSessionId: string): Promise { + await this.requests.send({ + type: "duplicate", + id: crypto.randomUUID(), + targetSessionId, + ts: Date.now(), + }); + } + + /** Listen for incoming requests. Stale messages are ignored. */ + onRequest( + handler: (msg: RequestMessage) => void | Promise, + ): Disposable { + return this.requests.onReceive((msg) => { + if (Date.now() - msg.ts > MESSAGE_MAX_AGE_MS) { + return; + } + return handler(msg); + }); + } +} diff --git a/test/unit/windowBroadcast.test.ts b/test/unit/windowBroadcast.test.ts new file mode 100644 index 0000000000..421c8564ec --- /dev/null +++ b/test/unit/windowBroadcast.test.ts @@ -0,0 +1,91 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +import { WindowBroadcast } from "../../src/windowBroadcast"; +import { InMemorySecretStorage, createMockLogger } from "../mocks/testHelpers"; + +interface TestMessage { + kind: string; + value: number; +} + +function isTestMessage(v: unknown): v is TestMessage { + if (typeof v !== "object" || v === null) { + return false; + } + const obj = v as Record; + return typeof obj.kind === "string" && typeof obj.value === "number"; +} + +function createBroadcast(key = "test.channel") { + const secrets = new InMemorySecretStorage(); + const logger = createMockLogger(); + const broadcast = new WindowBroadcast(secrets, key, isTestMessage, logger); + return { secrets, logger, broadcast }; +} + +describe("WindowBroadcast", () => { + beforeEach(() => vi.useFakeTimers()); + afterEach(() => vi.useRealTimers()); + + it("delivers sent messages to receivers", async () => { + const { broadcast } = createBroadcast(); + const handler = vi.fn(); + broadcast.onReceive(handler); + + await broadcast.send({ kind: "greeting", value: 42 }); + await vi.advanceTimersByTimeAsync(10); + + expect(handler).toHaveBeenCalledWith({ kind: "greeting", value: 42 }); + }); + + it("ignores messages on other keys", async () => { + const { secrets } = createBroadcast("my.key"); + const other = new WindowBroadcast( + secrets, + "other.key", + isTestMessage, + createMockLogger(), + ); + + const handler = vi.fn(); + other.onReceive(handler); + + await secrets.store("my.key", JSON.stringify({ kind: "x", value: 1 })); + await vi.advanceTimersByTimeAsync(10); + + expect(handler).not.toHaveBeenCalled(); + }); + + it("ignores messages that fail validation", async () => { + const { secrets, broadcast } = createBroadcast(); + const handler = vi.fn(); + broadcast.onReceive(handler); + + await secrets.store("test.channel", JSON.stringify({ bad: "shape" })); + await vi.advanceTimersByTimeAsync(10); + + expect(handler).not.toHaveBeenCalled(); + }); + + it("ignores malformed JSON without crashing", async () => { + const { secrets, broadcast } = createBroadcast(); + const handler = vi.fn(); + broadcast.onReceive(handler); + + await secrets.store("test.channel", "{not json"); + await vi.advanceTimersByTimeAsync(10); + + expect(handler).not.toHaveBeenCalled(); + }); + + it("stops delivering after dispose", async () => { + const { broadcast } = createBroadcast(); + const handler = vi.fn(); + broadcast.onReceive(handler).dispose(); + + await broadcast.send({ kind: "late", value: 0 }); + await vi.advanceTimersByTimeAsync(10); + + expect(handler).not.toHaveBeenCalled(); + }); +}); diff --git a/test/unit/windowIpc.test.ts b/test/unit/windowIpc.test.ts new file mode 100644 index 0000000000..dca6451d29 --- /dev/null +++ b/test/unit/windowIpc.test.ts @@ -0,0 +1,94 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +import { WindowIpc } from "../../src/windowIpc"; +import { InMemorySecretStorage, createMockLogger } from "../mocks/testHelpers"; + +function createIpcPair() { + const secrets = new InMemorySecretStorage(); + const logger = createMockLogger(); + return { + secrets, + sender: new WindowIpc(secrets, logger), + receiver: new WindowIpc(secrets, logger), + }; +} + +describe("WindowIpc", () => { + beforeEach(() => vi.useFakeTimers()); + afterEach(() => vi.useRealTimers()); + + describe("sendPing", () => { + it("resolves with PONG when another window responds", async () => { + const { sender, receiver } = createIpcPair(); + receiver.onRequest(async (msg) => { + if (msg.type === "ping") { + await receiver.sendPong(msg.id, "session-abc", "/home/coder/project"); + } + }); + + const promise = sender.sendPing("ssh-remote+my-host", 2000); + await vi.advanceTimersByTimeAsync(50); + + expect(await promise).toMatchObject({ + sessionId: "session-abc", + folder: "/home/coder/project", + }); + }); + + it("resolves undefined when no window responds", async () => { + const { sender } = createIpcPair(); + const promise = sender.sendPing("ssh-remote+my-host", 100); + await vi.advanceTimersByTimeAsync(150); + + expect(await promise).toBeUndefined(); + }); + }); + + it("onRequest ignores stale messages", async () => { + const { secrets, receiver } = createIpcPair(); + const handler = vi.fn(); + receiver.onRequest(handler); + + await secrets.store( + "coder.ipc.req", + JSON.stringify({ + type: "ping", + id: "old", + authority: "ssh-remote+host", + ts: Date.now() - 10_000, + }), + ); + await vi.advanceTimersByTimeAsync(10); + + expect(handler).not.toHaveBeenCalled(); + }); + + it("full ping → pong → duplicate round trip", async () => { + const { secrets, sender } = createIpcPair(); + const duplicated = vi.fn(); + + const windowA = new WindowIpc(secrets, createMockLogger()); + windowA.onRequest(async (msg) => { + if (msg.type === "ping" && msg.authority === "ssh-remote+host") { + await windowA.sendPong(msg.id, "win-a", "/home/coder/app"); + } + if (msg.type === "duplicate" && msg.targetSessionId === "win-a") { + duplicated(); + } + }); + + const promise = sender.sendPing("ssh-remote+host", 2000); + await vi.advanceTimersByTimeAsync(50); + const pong = await promise; + + expect(pong).toMatchObject({ + sessionId: "win-a", + folder: "/home/coder/app", + }); + + await sender.sendDuplicate("win-a"); + await vi.advanceTimersByTimeAsync(10); + + expect(duplicated).toHaveBeenCalledOnce(); + }); +}); From a34b30daa1b925a54b6b57a7b72a15fa764db1ab Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Mon, 27 Apr 2026 15:37:51 +0000 Subject: [PATCH 2/4] refactor: migrate OAuth callback to WindowBroadcast --- src/core/secretsManager.ts | 63 ++++++------------------------ src/oauth/authorizer.ts | 2 +- src/uri/uriHandler.ts | 2 +- test/unit/oauth/authorizer.test.ts | 18 +++++---- test/unit/uri/uriHandler.test.ts | 6 +-- 5 files changed, 29 insertions(+), 62 deletions(-) diff --git a/src/core/secretsManager.ts b/src/core/secretsManager.ts index 7d8d66cfd5..d4764e8d0a 100644 --- a/src/core/secretsManager.ts +++ b/src/core/secretsManager.ts @@ -2,6 +2,7 @@ import { z } from "zod"; import { DeploymentSchema, type Deployment } from "../deployment/types"; import { toSafeHost } from "../util"; +import { WindowBroadcast } from "../windowBroadcast"; import type { OAuth2ClientRegistrationResponse } from "coder/site/src/api/typesGenerated"; import type { Memento, SecretStorage, Disposable } from "vscode"; @@ -57,14 +58,24 @@ const OAuthCallbackDataSchema = z.object({ error: z.string().nullable(), }); -type OAuthCallbackData = z.infer; +export type OAuthCallbackData = z.infer; export class SecretsManager { + public readonly oauthCallback: WindowBroadcast; + constructor( private readonly secrets: SecretStorage, private readonly memento: Memento, private readonly logger: Logger, - ) {} + ) { + this.oauthCallback = new WindowBroadcast( + secrets, + OAUTH_CALLBACK_KEY, + (v: unknown): v is OAuthCallbackData => + OAuthCallbackDataSchema.safeParse(v).success, + logger, + ); + } private buildKey(prefix: SecretKeyPrefix, safeHostname: string): string { return `${prefix}${safeHostname || ""}`; @@ -306,54 +317,6 @@ export class SecretsManager { return safeHostname; } - /** - * Write an OAuth callback result to secrets storage. - * Used for cross-window communication when OAuth callback arrives in a different window. - */ - public async setOAuthCallback(data: OAuthCallbackData): Promise { - const parsed = OAuthCallbackDataSchema.parse(data); - await this.secrets.store(OAUTH_CALLBACK_KEY, JSON.stringify(parsed)); - } - - /** - * Listen for OAuth callback results from any VS Code window. - * The listener receives the state parameter, code (if success), and error (if failed). - */ - public onDidChangeOAuthCallback( - listener: (data: OAuthCallbackData) => void, - ): Disposable { - return this.secrets.onDidChange(async (e) => { - if (e.key !== OAUTH_CALLBACK_KEY) { - return; - } - - const raw = await this.secrets.get(OAUTH_CALLBACK_KEY); - if (!raw) { - return; - } - - let parsed: unknown; - try { - parsed = JSON.parse(raw); - } catch (err) { - this.logger.error("Failed to parse OAuth callback JSON", err); - return; - } - - const result = OAuthCallbackDataSchema.safeParse(parsed); - if (!result.success) { - this.logger.error("Invalid OAuth callback data shape", result.error); - return; - } - - try { - listener(result.data); - } catch (err) { - this.logger.error("Error in onDidChangeOAuthCallback listener", err); - } - }); - } - public getOAuthClientRegistration( safeHostname: string, ): Promise { diff --git a/src/oauth/authorizer.ts b/src/oauth/authorizer.ts index 98e2a11984..f7088b89f8 100644 --- a/src/oauth/authorizer.ts +++ b/src/oauth/authorizer.ts @@ -247,7 +247,7 @@ export class OAuthAuthorizer implements vscode.Disposable { timeoutMins * 60 * 1000, ); - const listener = this.secretsManager.onDidChangeOAuthCallback( + const listener = this.secretsManager.oauthCallback.onReceive( ({ state: callbackState, code, error }) => { if (callbackState !== state) { this.logger.warn( diff --git a/src/uri/uriHandler.ts b/src/uri/uriHandler.ts index 703cae4e97..e2ffdc521c 100644 --- a/src/uri/uriHandler.ts +++ b/src/uri/uriHandler.ts @@ -211,7 +211,7 @@ async function handleOAuthCallback(ctx: UriRouteContext): Promise { } try { - await secretsManager.setOAuthCallback({ state, code, error }); + await secretsManager.oauthCallback.send({ state, code, error }); logger.debug("OAuth callback processed successfully"); } catch (err) { logger.error("Failed to process OAuth callback:", err); diff --git a/test/unit/oauth/authorizer.test.ts b/test/unit/oauth/authorizer.test.ts index ecfed45f9d..4e987a08b9 100644 --- a/test/unit/oauth/authorizer.test.ts +++ b/test/unit/oauth/authorizer.test.ts @@ -83,7 +83,7 @@ function createTestContext() { /** Completes login by sending successful OAuth callback */ const completeLogin = async (state: string) => { - await base.secretsManager.setOAuthCallback({ + await base.secretsManager.oauthCallback.send({ state, code: "code", error: null, @@ -138,7 +138,7 @@ describe("OAuthAuthorizer", () => { const { state } = await waitForBrowserToOpen(); // Set the callback with the correct state (simulate user clicking authorize) - await secretsManager.setOAuthCallback({ + await secretsManager.oauthCallback.send({ state, code: "auth-code-123", error: null, @@ -185,7 +185,7 @@ describe("OAuthAuthorizer", () => { const { authUrl, state } = await waitForBrowserToOpen(); expect(authUrl.searchParams.get("client_id")).toBe("existing-client-id"); - await secretsManager.setOAuthCallback({ + await secretsManager.oauthCallback.send({ state, code: "code", error: null, @@ -225,7 +225,7 @@ describe("OAuthAuthorizer", () => { const { authUrl, state } = await waitForBrowserToOpen(); expect(authUrl.searchParams.get("client_id")).toBe("new-client-id"); - await secretsManager.setOAuthCallback({ + await secretsManager.oauthCallback.send({ state, code: "code", error: null, @@ -267,7 +267,7 @@ describe("OAuthAuthorizer", () => { const { loginPromise, state } = await startLogin(); // Send callback with wrong state - should be ignored - await secretsManager.setOAuthCallback({ + await secretsManager.oauthCallback.send({ state: "wrong-state", code: "code", error: null, @@ -292,7 +292,7 @@ describe("OAuthAuthorizer", () => { setupOAuthRoutes(); const { loginPromise, state } = await startLogin(); - await secretsManager.setOAuthCallback({ + await secretsManager.oauthCallback.send({ state, code: null, error: "access_denied", @@ -307,7 +307,11 @@ describe("OAuthAuthorizer", () => { setupOAuthRoutes(); const { loginPromise, state } = await startLogin(); - await secretsManager.setOAuthCallback({ state, code: null, error: null }); + await secretsManager.oauthCallback.send({ + state, + code: null, + error: null, + }); await expect(loginPromise).rejects.toThrow( "No authorization code received", diff --git a/test/unit/uri/uriHandler.test.ts b/test/unit/uri/uriHandler.test.ts index 849f94e02c..a95ff3370c 100644 --- a/test/unit/uri/uriHandler.test.ts +++ b/test/unit/uri/uriHandler.test.ts @@ -361,7 +361,7 @@ describe("uriHandler", () => { const { handleUri, secretsManager } = createTestContext(); const callbackPromise = new Promise((resolve) => { - secretsManager.onDidChangeOAuthCallback(resolve); + secretsManager.oauthCallback.onReceive(resolve); }); await handleUri( @@ -380,7 +380,7 @@ describe("uriHandler", () => { const { handleUri, secretsManager } = createTestContext(); const callbackPromise = new Promise((resolve) => { - secretsManager.onDidChangeOAuthCallback(resolve); + secretsManager.oauthCallback.onReceive(resolve); }); await handleUri( @@ -399,7 +399,7 @@ describe("uriHandler", () => { const { handleUri, secretsManager } = createTestContext(); let callbackReceived = false; - secretsManager.onDidChangeOAuthCallback(() => { + secretsManager.oauthCallback.onReceive(() => { callbackReceived = true; }); From 4742f4316926ecbd2a1a920e584a7ff570f21770 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Tue, 28 Apr 2026 11:58:58 +0300 Subject: [PATCH 3/4] 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 --- src/commands.ts | 52 ++++---- src/core/container.ts | 24 +++- src/core/secretsManager.ts | 22 +--- src/extension.ts | 9 +- src/{ => ipc}/windowBroadcast.ts | 19 ++- src/login/loginCoordinator.ts | 3 + src/oauth/authorizer.ts | 14 +- src/oauth/oauthCallback.ts | 45 +++++++ src/uri/uriHandler.ts | 4 +- .../duplicateWorkspaceIpc.ts} | 124 ++++++++---------- test/unit/ipc/windowBroadcast.test.ts | 92 +++++++++++++ test/unit/login/loginCoordinator.test.ts | 14 +- test/unit/oauth/authorizer.test.ts | 30 +++-- test/unit/oauth/testUtils.ts | 10 +- test/unit/uri/uriHandler.test.ts | 16 ++- test/unit/windowBroadcast.test.ts | 91 ------------- test/unit/windowIpc.test.ts | 94 ------------- .../workspace/duplicateWorkspaceIpc.test.ts | 100 ++++++++++++++ 18 files changed, 412 insertions(+), 351 deletions(-) rename src/{ => ipc}/windowBroadcast.ts (62%) create mode 100644 src/oauth/oauthCallback.ts rename src/{windowIpc.ts => workspace/duplicateWorkspaceIpc.ts} (53%) create mode 100644 test/unit/ipc/windowBroadcast.test.ts delete mode 100644 test/unit/windowBroadcast.test.ts delete mode 100644 test/unit/windowIpc.test.ts create mode 100644 test/unit/workspace/duplicateWorkspaceIpc.test.ts diff --git a/src/commands.ts b/src/commands.ts index a4e14d10cb..4cdedde1a8 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -1,7 +1,3 @@ -import { - type Workspace, - type WorkspaceAgent, -} from "coder/site/src/api/typesGenerated"; import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; @@ -13,20 +9,11 @@ import { extractAgents, workspaceStatusLabel, } from "./api/api-helper"; -import { type CoderApi } from "./api/coderApi"; import * as cliExec from "./core/cliExec"; -import { type CliManager } from "./core/cliManager"; -import { type ServiceContainer } from "./core/container"; -import { type MementoManager } from "./core/mementoManager"; -import { type PathResolver } from "./core/pathResolver"; -import { type SecretsManager } from "./core/secretsManager"; import { appendVsCodeLogs } from "./core/supportBundleLogs"; -import { type DeploymentManager } from "./deployment/deploymentManager"; import { CertificateError } from "./error/certificateError"; import { toError } from "./error/errorUtils"; import { type FeatureSet, featureSetForVersion } from "./featureSet"; -import { type Logger } from "./logging/logger"; -import { type LoginCoordinator } from "./login/loginCoordinator"; import { withCancellableProgress, withProgress } from "./progress"; import { maybeAskAgent, maybeAskUrl } from "./promptUtils"; import { @@ -36,13 +23,31 @@ import { import { resolveCliAuth } from "./settings/cli"; import { toRemoteAuthority, toSafeHost } from "./util"; import { vscodeProposed } from "./vscodeProposed"; -import { type PongMessage, type WindowIpc } from "./windowIpc"; import { AgentTreeItem, type OpenableTreeItem, WorkspaceTreeItem, } from "./workspace/workspacesProvider"; +import type { + Workspace, + WorkspaceAgent, +} from "coder/site/src/api/typesGenerated"; + +import type { CoderApi } from "./api/coderApi"; +import type { CliManager } from "./core/cliManager"; +import type { ServiceContainer } from "./core/container"; +import type { MementoManager } from "./core/mementoManager"; +import type { PathResolver } from "./core/pathResolver"; +import type { SecretsManager } from "./core/secretsManager"; +import type { DeploymentManager } from "./deployment/deploymentManager"; +import type { Logger } from "./logging/logger"; +import type { LoginCoordinator } from "./login/loginCoordinator"; +import type { + DuplicateWorkspaceIpc, + PongMessage, +} from "./workspace/duplicateWorkspaceIpc"; + interface OpenOptions { workspaceOwner?: string; workspaceName?: string; @@ -66,7 +71,7 @@ export class Commands { private readonly secretsManager: SecretsManager; private readonly cliManager: CliManager; private readonly loginCoordinator: LoginCoordinator; - private readonly windowIpc: WindowIpc; + private readonly duplicateWorkspaceIpc: DuplicateWorkspaceIpc; // These will only be populated when actively connected to a workspace and are // used in commands. Because commands can be executed by the user, it is not @@ -90,7 +95,7 @@ export class Commands { this.secretsManager = serviceContainer.getSecretsManager(); this.cliManager = serviceContainer.getCliManager(); this.loginCoordinator = serviceContainer.getLoginCoordinator(); - this.windowIpc = serviceContainer.getWindowIpc(); + this.duplicateWorkspaceIpc = serviceContainer.getDuplicateWorkspaceIpc(); } /** @@ -1083,8 +1088,9 @@ export class Commands { // Only set the memento when opening a new folder/window await this.mementoManager.setStartupMode("start"); - // Best-effort: detect other connected windows in the background. - this.windowIpc + // Best-effort check for an already-connected window. Runs in the + // background so it never delays openFolder. + this.duplicateWorkspaceIpc .sendPing(remoteAuthority) .then((pong) => { if (pong) { @@ -1117,10 +1123,8 @@ export class Commands { return true; } - /** - * Non-blocking notification — VS Code may dismiss it at any time, - * so this must not be awaited. - */ + // VS Code may dismiss a non-modal info message without resolving the + // thenable, so this must not be awaited from the open path. private showMultiWindowNotification( pong: PongMessage, remoteAuthority: string, @@ -1129,13 +1133,13 @@ export class Commands { const openEmptyAction = "Open Without Folder"; vscode.window .showInformationMessage( - `A window is already connected to this workspace (${pong.folder}).`, + "This workspace is already open in another window.", duplicateAction, openEmptyAction, ) .then(async (choice) => { if (choice === duplicateAction) { - await this.windowIpc.sendDuplicate(pong.sessionId); + await this.duplicateWorkspaceIpc.sendDuplicate(pong.sessionId); } else if (choice === openEmptyAction) { await vscode.commands.executeCommand("vscode.newWindow", { remoteAuthority, diff --git a/src/core/container.ts b/src/core/container.ts index 335b82ba41..243fb3467f 100644 --- a/src/core/container.ts +++ b/src/core/container.ts @@ -1,9 +1,9 @@ import * as vscode from "vscode"; import { CoderApi } from "../api/coderApi"; -import { type Logger } from "../logging/logger"; import { LoginCoordinator } from "../login/loginCoordinator"; -import { WindowIpc } from "../windowIpc"; +import { OAuthCallback } from "../oauth/oauthCallback"; +import { DuplicateWorkspaceIpc } from "../workspace/duplicateWorkspaceIpc"; import { CliCredentialManager } from "./cliCredentialManager"; import { CliManager } from "./cliManager"; @@ -12,6 +12,8 @@ import { MementoManager } from "./mementoManager"; import { PathResolver } from "./pathResolver"; import { SecretsManager } from "./secretsManager"; +import type { Logger } from "../logging/logger"; + /** * Service container for dependency injection. * Centralizes the creation and management of all core services. @@ -25,7 +27,8 @@ export class ServiceContainer implements vscode.Disposable { private readonly cliManager: CliManager; private readonly contextManager: ContextManager; private readonly loginCoordinator: LoginCoordinator; - private readonly windowIpc: WindowIpc; + private readonly duplicateWorkspaceIpc: DuplicateWorkspaceIpc; + private readonly oauthCallback: OAuthCallback; constructor(context: vscode.ExtensionContext) { this.logger = vscode.window.createOutputChannel("Coder", { log: true }); @@ -65,14 +68,19 @@ export class ServiceContainer implements vscode.Disposable { this.cliCredentialManager, ); this.contextManager = new ContextManager(context); + this.oauthCallback = new OAuthCallback(context.secrets, this.logger); this.loginCoordinator = new LoginCoordinator( this.secretsManager, this.mementoManager, this.logger, this.cliCredentialManager, + this.oauthCallback, context.extension.id, ); - this.windowIpc = new WindowIpc(context.secrets, this.logger); + this.duplicateWorkspaceIpc = new DuplicateWorkspaceIpc( + context.secrets, + this.logger, + ); } getPathResolver(): PathResolver { @@ -107,8 +115,12 @@ export class ServiceContainer implements vscode.Disposable { return this.loginCoordinator; } - getWindowIpc(): WindowIpc { - return this.windowIpc; + getDuplicateWorkspaceIpc(): DuplicateWorkspaceIpc { + return this.duplicateWorkspaceIpc; + } + + getOAuthCallback(): OAuthCallback { + return this.oauthCallback; } /** diff --git a/src/core/secretsManager.ts b/src/core/secretsManager.ts index d4764e8d0a..ac3ee6b14c 100644 --- a/src/core/secretsManager.ts +++ b/src/core/secretsManager.ts @@ -2,7 +2,6 @@ import { z } from "zod"; import { DeploymentSchema, type Deployment } from "../deployment/types"; import { toSafeHost } from "../util"; -import { WindowBroadcast } from "../windowBroadcast"; import type { OAuth2ClientRegistrationResponse } from "coder/site/src/api/typesGenerated"; import type { Memento, SecretStorage, Disposable } from "vscode"; @@ -17,7 +16,6 @@ const DEPLOYMENT_ACCESS_PREFIX = "coder.access."; type SecretKeyPrefix = typeof SESSION_KEY_PREFIX | typeof OAUTH_CLIENT_PREFIX; -const OAUTH_CALLBACK_KEY = "coder.oauthCallback"; const CURRENT_DEPLOYMENT_KEY = "coder.currentDeployment"; const DEFAULT_MAX_DEPLOYMENTS = 10; @@ -52,30 +50,12 @@ const SessionAuthSchema = z.object({ export type SessionAuth = z.infer; -const OAuthCallbackDataSchema = z.object({ - state: z.string(), - code: z.string().nullable(), - error: z.string().nullable(), -}); - -export type OAuthCallbackData = z.infer; - export class SecretsManager { - public readonly oauthCallback: WindowBroadcast; - constructor( private readonly secrets: SecretStorage, private readonly memento: Memento, private readonly logger: Logger, - ) { - this.oauthCallback = new WindowBroadcast( - secrets, - OAUTH_CALLBACK_KEY, - (v: unknown): v is OAuthCallbackData => - OAuthCallbackDataSchema.safeParse(v).success, - logger, - ); - } + ) {} private buildKey(prefix: SecretKeyPrefix, safeHostname: string): string { return `${prefix}${safeHostname || ""}`; diff --git a/src/extension.ts b/src/extension.ts index 7284b0df42..cb38ecb3ae 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -336,19 +336,16 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { const remote = new Remote(serviceContainer, commands, ctx); // Respond to PINGs and DUPLICATE commands from other windows. - const windowIpc = serviceContainer.getWindowIpc(); + const duplicateWorkspaceIpc = serviceContainer.getDuplicateWorkspaceIpc(); ctx.subscriptions.push( - windowIpc.onRequest(async (msg) => { + duplicateWorkspaceIpc.onRequest(async (msg) => { const currentAuthority = vscodeProposed.env.remoteAuthority; if (!currentAuthority) { return; } if (msg.type === "ping" && msg.authority === currentAuthority) { - // Only the first folder matters for dedup; multi-root - // workspaces use a different URI scheme. - const folder = vscode.workspace.workspaceFolders?.[0]?.uri.path ?? ""; - await windowIpc.sendPong(msg.id, vscode.env.sessionId, folder); + await duplicateWorkspaceIpc.sendPong(msg.id, vscode.env.sessionId); } if ( diff --git a/src/windowBroadcast.ts b/src/ipc/windowBroadcast.ts similarity index 62% rename from src/windowBroadcast.ts rename to src/ipc/windowBroadcast.ts index aa66cc668b..436b184d7d 100644 --- a/src/windowBroadcast.ts +++ b/src/ipc/windowBroadcast.ts @@ -1,19 +1,18 @@ import type { Disposable, SecretStorage } from "vscode"; +import type { ZodType } from "zod"; -import type { Logger } from "./logging/logger"; +import type { Logger } from "../logging/logger"; /** - * Typed pub/sub over a single SecretStorage key. - * - * SecretStorage.onDidChange fires across all VS Code windows, so each - * WindowBroadcast instance acts as a cross-window channel for messages - * of type T. + * Typed pub/sub over a single SecretStorage key. SecretStorage.onDidChange + * fires across all VS Code windows, so each instance is a cross-window + * channel for messages of type T. */ export class WindowBroadcast { constructor( private readonly secrets: SecretStorage, private readonly key: string, - private readonly validate: (value: unknown) => value is T, + private readonly schema: ZodType, private readonly logger: Logger, ) {} @@ -31,11 +30,11 @@ export class WindowBroadcast { if (!raw) { return; } - const parsed: unknown = JSON.parse(raw); - if (!this.validate(parsed)) { + const parsed = this.schema.safeParse(JSON.parse(raw)); + if (!parsed.success) { return; } - await handler(parsed); + await handler(parsed.data); } catch (err) { this.logger.error(`Error handling broadcast on ${this.key}`, err); } diff --git a/src/login/loginCoordinator.ts b/src/login/loginCoordinator.ts index 0835330b67..d894c3830a 100644 --- a/src/login/loginCoordinator.ts +++ b/src/login/loginCoordinator.ts @@ -19,6 +19,7 @@ import type { MementoManager } from "../core/mementoManager"; import type { OAuthTokenData, SecretsManager } from "../core/secretsManager"; import type { Deployment } from "../deployment/types"; import type { Logger } from "../logging/logger"; +import type { OAuthCallback } from "../oauth/oauthCallback"; type LoginResult = | { success: false } @@ -43,10 +44,12 @@ export class LoginCoordinator implements vscode.Disposable { private readonly mementoManager: MementoManager, private readonly logger: Logger, private readonly cliCredentialManager: CliCredentialManager, + oauthCallback: OAuthCallback, extensionId: string, ) { this.oauthAuthorizer = new OAuthAuthorizer( secretsManager, + oauthCallback, logger, extensionId, ); diff --git a/src/oauth/authorizer.ts b/src/oauth/authorizer.ts index f7088b89f8..7280e74202 100644 --- a/src/oauth/authorizer.ts +++ b/src/oauth/authorizer.ts @@ -1,10 +1,6 @@ -import { type AxiosInstance } from "axios"; import * as vscode from "vscode"; import { CoderApi } from "../api/coderApi"; -import { type SecretsManager } from "../core/secretsManager"; -import { type Deployment } from "../deployment/types"; -import { type Logger } from "../logging/logger"; import { AUTH_GRANT_TYPE, @@ -21,6 +17,7 @@ import { toUrlSearchParams, } from "./utils"; +import type { AxiosInstance } from "axios"; import type { OAuth2AuthorizationServerMetadata, OAuth2ClientRegistrationRequest, @@ -30,6 +27,12 @@ import type { User, } from "coder/site/src/api/typesGenerated"; +import type { SecretsManager } from "../core/secretsManager"; +import type { Deployment } from "../deployment/types"; +import type { Logger } from "../logging/logger"; + +import type { OAuthCallback } from "./oauthCallback"; + /** * Handles the OAuth authorization code flow for authenticating with Coder deployments. * Encapsulates client registration, PKCE challenge, and token exchange. @@ -39,6 +42,7 @@ export class OAuthAuthorizer implements vscode.Disposable { constructor( private readonly secretsManager: SecretsManager, + private readonly oauthCallback: OAuthCallback, private readonly logger: Logger, private readonly extensionId: string, ) {} @@ -247,7 +251,7 @@ export class OAuthAuthorizer implements vscode.Disposable { timeoutMins * 60 * 1000, ); - const listener = this.secretsManager.oauthCallback.onReceive( + const listener = this.oauthCallback.onReceive( ({ state: callbackState, code, error }) => { if (callbackState !== state) { this.logger.warn( diff --git a/src/oauth/oauthCallback.ts b/src/oauth/oauthCallback.ts new file mode 100644 index 0000000000..14bc516602 --- /dev/null +++ b/src/oauth/oauthCallback.ts @@ -0,0 +1,45 @@ +import { z } from "zod"; + +import { WindowBroadcast } from "../ipc/windowBroadcast"; + +import type { Disposable, SecretStorage } from "vscode"; + +import type { Logger } from "../logging/logger"; + +const OAUTH_CALLBACK_KEY = "coder.oauthCallback"; + +const OAuthCallbackDataSchema = z.object({ + state: z.string(), + code: z.string().nullable(), + error: z.string().nullable(), +}); + +export type OAuthCallbackData = z.infer; + +/** + * Forwards OAuth redirect parameters from the URI handler back to the + * window that initiated the login. Required because the redirect may + * land in a different window than the one that started the flow. + */ +export class OAuthCallback { + private readonly broadcast: WindowBroadcast; + + constructor(secrets: SecretStorage, logger: Logger) { + this.broadcast = new WindowBroadcast( + secrets, + OAUTH_CALLBACK_KEY, + OAuthCallbackDataSchema, + logger, + ); + } + + send(data: OAuthCallbackData): Promise { + return this.broadcast.send(data); + } + + onReceive( + handler: (data: OAuthCallbackData) => void | Promise, + ): Disposable { + return this.broadcast.onReceive(handler); + } +} diff --git a/src/uri/uriHandler.ts b/src/uri/uriHandler.ts index e2ffdc521c..6942d1c94f 100644 --- a/src/uri/uriHandler.ts +++ b/src/uri/uriHandler.ts @@ -199,7 +199,7 @@ async function setupDeployment( async function handleOAuthCallback(ctx: UriRouteContext): Promise { const { params, serviceContainer } = ctx; const logger = serviceContainer.getLogger(); - const secretsManager = serviceContainer.getSecretsManager(); + const oauthCallback = serviceContainer.getOAuthCallback(); const code = params.get("code"); const state = params.get("state"); @@ -211,7 +211,7 @@ async function handleOAuthCallback(ctx: UriRouteContext): Promise { } try { - await secretsManager.oauthCallback.send({ state, code, error }); + await oauthCallback.send({ state, code, error }); logger.debug("OAuth callback processed successfully"); } catch (err) { logger.error("Failed to process OAuth callback:", err); diff --git a/src/windowIpc.ts b/src/workspace/duplicateWorkspaceIpc.ts similarity index 53% rename from src/windowIpc.ts rename to src/workspace/duplicateWorkspaceIpc.ts index 4ca7382b28..3b100a39b3 100644 --- a/src/windowIpc.ts +++ b/src/workspace/duplicateWorkspaceIpc.ts @@ -1,71 +1,59 @@ import crypto from "node:crypto"; +import { z } from "zod"; -import { WindowBroadcast } from "./windowBroadcast"; +import { WindowBroadcast } from "../ipc/windowBroadcast"; import type { Disposable, SecretStorage } from "vscode"; -import type { Logger } from "./logging/logger"; +import type { Logger } from "../logging/logger"; const MESSAGE_MAX_AGE_MS = 5000; const DEFAULT_PING_TIMEOUT_MS = 1000; -export interface PingMessage { - type: "ping"; - id: string; - authority: string; - ts: number; -} - -export interface PongMessage { - type: "pong"; - id: string; - sessionId: string; - folder: string; - ts: number; -} - -export interface DuplicateMessage { - type: "duplicate"; - id: string; - targetSessionId: string; - ts: number; -} - -type RequestMessage = PingMessage | DuplicateMessage; - -function isRequestMessage(msg: unknown): msg is RequestMessage { - if (typeof msg !== "object" || msg === null) { - return false; - } - const obj = msg as Record; - if (typeof obj.id !== "string" || typeof obj.ts !== "number") { - return false; - } - if (obj.type === "ping") { - return typeof obj.authority === "string"; - } - if (obj.type === "duplicate") { - return typeof obj.targetSessionId === "string"; - } - return false; -} - -function isPongMessage(msg: unknown): msg is PongMessage { - if (typeof msg !== "object" || msg === null) { - return false; - } - const obj = msg as Record; - return ( - obj.type === "pong" && - typeof obj.id === "string" && - typeof obj.sessionId === "string" && - typeof obj.folder === "string" && - typeof obj.ts === "number" - ); -} - -/** Cross-window IPC built on WindowBroadcast channels. */ -export class WindowIpc { +const REQUEST_KEY = "coder.ipc.req"; +const RESPONSE_KEY = "coder.ipc.res"; + +const PingMessageSchema = z.object({ + type: z.literal("ping"), + id: z.string(), + authority: z.string(), + ts: z.number(), +}); + +const PongMessageSchema = z.object({ + type: z.literal("pong"), + id: z.string(), + sessionId: z.string(), + ts: z.number(), +}); + +const DuplicateMessageSchema = z.object({ + type: z.literal("duplicate"), + id: z.string(), + targetSessionId: z.string(), + ts: z.number(), +}); + +const RequestMessageSchema = z.discriminatedUnion("type", [ + PingMessageSchema, + DuplicateMessageSchema, +]); + +export type PingMessage = z.infer; +export type PongMessage = z.infer; +export type DuplicateMessage = z.infer; +export type RequestMessage = z.infer; + +/** + * Cross-window protocol for the open-workspace flow: + * PING "anyone connected to this authority?" + * PONG "yes, with this sessionId" + * DUPLICATE "sessionId, please duplicate yourself" + * + * The sender shows the prompt locally on first PONG. If the user picks + * Duplicate, it sends DUPLICATE targeted at the responder's sessionId. + */ +export class DuplicateWorkspaceIpc { private readonly requests: WindowBroadcast; private readonly responses: WindowBroadcast; @@ -75,14 +63,14 @@ export class WindowIpc { ) { this.requests = new WindowBroadcast( secrets, - "coder.ipc.req", - isRequestMessage, + REQUEST_KEY, + RequestMessageSchema, logger, ); this.responses = new WindowBroadcast( secrets, - "coder.ipc.res", - isPongMessage, + RESPONSE_KEY, + PongMessageSchema, logger, ); } @@ -124,20 +112,16 @@ export class WindowIpc { }); } - async sendPong( - pingId: string, - sessionId: string, - folder: string, - ): Promise { + async sendPong(pingId: string, sessionId: string): Promise { await this.responses.send({ type: "pong", id: pingId, sessionId, - folder, ts: Date.now(), }); } + /** Ask the window with this sessionId to duplicate itself. */ async sendDuplicate(targetSessionId: string): Promise { await this.requests.send({ type: "duplicate", @@ -147,7 +131,7 @@ export class WindowIpc { }); } - /** Listen for incoming requests. Stale messages are ignored. */ + /** Listen for incoming requests. Stale messages are dropped. */ onRequest( handler: (msg: RequestMessage) => void | Promise, ): Disposable { diff --git a/test/unit/ipc/windowBroadcast.test.ts b/test/unit/ipc/windowBroadcast.test.ts new file mode 100644 index 0000000000..e72a5a6e17 --- /dev/null +++ b/test/unit/ipc/windowBroadcast.test.ts @@ -0,0 +1,92 @@ +import { describe, it, expect, vi } from "vitest"; +import { z } from "zod"; + +import { WindowBroadcast } from "@/ipc/windowBroadcast"; + +import { + InMemorySecretStorage, + createMockLogger, +} from "../../mocks/testHelpers"; + +const TestMessageSchema = z.object({ + kind: z.string(), + value: z.number(), +}); +type TestMessage = z.infer; + +// SecretStorage.onDidChange fires synchronously after store(), but the +// listener body awaits a storage read before invoking the handler. Yield +// once to let those microtasks run. +const flushAsync = () => new Promise((resolve) => setImmediate(resolve)); + +function makeBroadcast(key = "test.channel") { + const secrets = new InMemorySecretStorage(); + const broadcast = new WindowBroadcast( + secrets, + key, + TestMessageSchema, + createMockLogger(), + ); + return { secrets, broadcast }; +} + +describe("WindowBroadcast", () => { + it("delivers messages on its own key and ignores other keys", async () => { + const { secrets, broadcast } = makeBroadcast("my.key"); + const other = new WindowBroadcast( + secrets, + "other.key", + TestMessageSchema, + createMockLogger(), + ); + + const handler = vi.fn(); + broadcast.onReceive(handler); + + await other.send({ kind: "stranger", value: 1 }); + await flushAsync(); + expect(handler).not.toHaveBeenCalled(); + + await broadcast.send({ kind: "self", value: 2 }); + await flushAsync(); + expect(handler).toHaveBeenCalledWith({ kind: "self", value: 2 }); + expect(handler).toHaveBeenCalledTimes(1); + }); + + it("drops messages that fail schema validation", async () => { + const { secrets, broadcast } = makeBroadcast(); + const handler = vi.fn(); + broadcast.onReceive(handler); + + await secrets.store("test.channel", JSON.stringify({ kind: "x" })); + await flushAsync(); + + expect(handler).not.toHaveBeenCalled(); + }); + + it("drops malformed JSON without crashing", async () => { + const { secrets, broadcast } = makeBroadcast(); + const handler = vi.fn(); + broadcast.onReceive(handler); + + await secrets.store("test.channel", "{not json"); + await flushAsync(); + + expect(handler).not.toHaveBeenCalled(); + }); + + it("stops delivering after dispose", async () => { + const { broadcast } = makeBroadcast(); + const handler = vi.fn(); + const subscription = broadcast.onReceive(handler); + + await broadcast.send({ kind: "before", value: 1 }); + await flushAsync(); + expect(handler).toHaveBeenCalledTimes(1); + + subscription.dispose(); + await broadcast.send({ kind: "after", value: 2 }); + await flushAsync(); + expect(handler).toHaveBeenCalledTimes(1); + }); +}); diff --git a/test/unit/login/loginCoordinator.test.ts b/test/unit/login/loginCoordinator.test.ts index be9013f33d..fa6a2aedd3 100644 --- a/test/unit/login/loginCoordinator.test.ts +++ b/test/unit/login/loginCoordinator.test.ts @@ -6,6 +6,7 @@ import { MementoManager } from "@/core/mementoManager"; import { SecretsManager } from "@/core/secretsManager"; import { getHeaders } from "@/headers"; import { LoginCoordinator } from "@/login/loginCoordinator"; +import { OAuthCallback } from "@/oauth/oauthCallback"; import { maybeAskAuthMethod } from "@/promptUtils"; import { @@ -115,6 +116,7 @@ function createTestContext() { const memento = new InMemoryMemento(); const logger = createMockLogger(); const secretsManager = new SecretsManager(secretStorage, memento, logger); + const oauthCallback = new OAuthCallback(secretStorage, logger); const mementoManager = new MementoManager(memento); const mockCredentialManager = createMockCliCredentialManager(); @@ -123,6 +125,7 @@ function createTestContext() { mementoManager, logger, mockCredentialManager, + oauthCallback, "coder.coder-remote", ); @@ -151,6 +154,7 @@ function createTestContext() { mockConfig, userInteraction, secretsManager, + oauthCallback, mementoManager, mockCredentialManager, coordinator, @@ -302,8 +306,13 @@ describe("LoginCoordinator", () => { }); it("logs warning instead of showing dialog for autoLogin", async () => { - const { mockConfig, secretsManager, mementoManager, mockAuthFailure } = - createTestContext(); + const { + mockConfig, + secretsManager, + oauthCallback, + mementoManager, + mockAuthFailure, + } = createTestContext(); mockConfig.set("coder.tlsCertFile", "/path/to/cert.pem"); mockConfig.set("coder.tlsKeyFile", "/path/to/key.pem"); @@ -313,6 +322,7 @@ describe("LoginCoordinator", () => { mementoManager, logger, createMockCliCredentialManager(), + oauthCallback, "coder.coder-remote", ); diff --git a/test/unit/oauth/authorizer.test.ts b/test/unit/oauth/authorizer.test.ts index 4e987a08b9..55560d84e9 100644 --- a/test/unit/oauth/authorizer.test.ts +++ b/test/unit/oauth/authorizer.test.ts @@ -61,6 +61,7 @@ function createTestContext() { const base = createBaseTestContext(); const authorizer = new OAuthAuthorizer( base.secretsManager, + base.oauthCallback, base.logger, EXTENSION_ID, ); @@ -83,7 +84,7 @@ function createTestContext() { /** Completes login by sending successful OAuth callback */ const completeLogin = async (state: string) => { - await base.secretsManager.oauthCallback.send({ + await base.oauthCallback.send({ state, code: "code", error: null, @@ -111,7 +112,8 @@ async function waitForBrowserToOpen(): Promise<{ describe("OAuthAuthorizer", () => { describe("login flow", () => { it("completes full OAuth login flow successfully", async () => { - const { mockAdapter, secretsManager, authorizer } = createTestContext(); + const { mockAdapter, oauthCallback, secretsManager, authorizer } = + createTestContext(); setupAxiosMockRoutes(mockAdapter, { "/.well-known/oauth-authorization-server": @@ -138,7 +140,7 @@ describe("OAuthAuthorizer", () => { const { state } = await waitForBrowserToOpen(); // Set the callback with the correct state (simulate user clicking authorize) - await secretsManager.oauthCallback.send({ + await oauthCallback.send({ state, code: "auth-code-123", error: null, @@ -156,7 +158,8 @@ describe("OAuthAuthorizer", () => { }); it("uses existing client registration when redirect URI matches", async () => { - const { mockAdapter, secretsManager, authorizer } = createTestContext(); + const { mockAdapter, oauthCallback, secretsManager, authorizer } = + createTestContext(); // Pre-store a client registration with matching redirect URI await secretsManager.setOAuthClientRegistration( @@ -185,7 +188,7 @@ describe("OAuthAuthorizer", () => { const { authUrl, state } = await waitForBrowserToOpen(); expect(authUrl.searchParams.get("client_id")).toBe("existing-client-id"); - await secretsManager.oauthCallback.send({ + await oauthCallback.send({ state, code: "code", error: null, @@ -194,7 +197,8 @@ describe("OAuthAuthorizer", () => { }); it("re-registers client when redirect URI has changed", async () => { - const { mockAdapter, secretsManager, authorizer } = createTestContext(); + const { mockAdapter, oauthCallback, secretsManager, authorizer } = + createTestContext(); // Pre-store a client registration with different redirect URI await secretsManager.setOAuthClientRegistration( @@ -225,7 +229,7 @@ describe("OAuthAuthorizer", () => { const { authUrl, state } = await waitForBrowserToOpen(); expect(authUrl.searchParams.get("client_id")).toBe("new-client-id"); - await secretsManager.oauthCallback.send({ + await oauthCallback.send({ state, code: "code", error: null, @@ -260,14 +264,14 @@ describe("OAuthAuthorizer", () => { describe("callback handling", () => { it("ignores callback with wrong state", async () => { - const { secretsManager, setupOAuthRoutes, startLogin, completeLogin } = + const { oauthCallback, setupOAuthRoutes, startLogin, completeLogin } = createTestContext(); setupOAuthRoutes(); const { loginPromise, state } = await startLogin(); // Send callback with wrong state - should be ignored - await secretsManager.oauthCallback.send({ + await oauthCallback.send({ state: "wrong-state", code: "code", error: null, @@ -287,12 +291,12 @@ describe("OAuthAuthorizer", () => { }); it("rejects on OAuth error callback", async () => { - const { secretsManager, setupOAuthRoutes, startLogin } = + const { oauthCallback, setupOAuthRoutes, startLogin } = createTestContext(); setupOAuthRoutes(); const { loginPromise, state } = await startLogin(); - await secretsManager.oauthCallback.send({ + await oauthCallback.send({ state, code: null, error: "access_denied", @@ -302,12 +306,12 @@ describe("OAuthAuthorizer", () => { }); it("rejects when no code is received", async () => { - const { secretsManager, setupOAuthRoutes, startLogin } = + const { oauthCallback, setupOAuthRoutes, startLogin } = createTestContext(); setupOAuthRoutes(); const { loginPromise, state } = await startLogin(); - await secretsManager.oauthCallback.send({ + await oauthCallback.send({ state, code: null, error: null, diff --git a/test/unit/oauth/testUtils.ts b/test/unit/oauth/testUtils.ts index aa64759a91..714c581197 100644 --- a/test/unit/oauth/testUtils.ts +++ b/test/unit/oauth/testUtils.ts @@ -3,6 +3,7 @@ import { vi } from "vitest"; import { SecretsManager } from "@/core/secretsManager"; import { getHeaders } from "@/headers"; +import { OAuthCallback } from "@/oauth/oauthCallback"; import { createMockLogger, @@ -128,6 +129,7 @@ export function createBaseTestContext() { const memento = new InMemoryMemento(); const logger = createMockLogger(); const secretsManager = new SecretsManager(secretStorage, memento, logger); + const oauthCallback = new OAuthCallback(secretStorage, logger); /** Sets up default OAuth routes - use explicit routes when asserting on values */ const setupOAuthRoutes = () => { @@ -140,5 +142,11 @@ export function createBaseTestContext() { }); }; - return { mockAdapter, secretsManager, logger, setupOAuthRoutes }; + return { + mockAdapter, + secretsManager, + oauthCallback, + logger, + setupOAuthRoutes, + }; } diff --git a/test/unit/uri/uriHandler.test.ts b/test/unit/uri/uriHandler.test.ts index a95ff3370c..bd2dd00b8c 100644 --- a/test/unit/uri/uriHandler.test.ts +++ b/test/unit/uri/uriHandler.test.ts @@ -3,6 +3,7 @@ import * as vscode from "vscode"; import { MementoManager } from "@/core/mementoManager"; import { SecretsManager } from "@/core/secretsManager"; +import { OAuthCallback } from "@/oauth/oauthCallback"; import { CALLBACK_PATH } from "@/oauth/utils"; import { maybeAskUrl } from "@/promptUtils"; import { registerUriHandler } from "@/uri/uriHandler"; @@ -67,6 +68,7 @@ function createTestContext() { const memento = new InMemoryMemento(); const logger = createMockLogger(); const secretsManager = new SecretsManager(secretStorage, memento, logger); + const oauthCallback = new OAuthCallback(secretStorage, logger); const loginCoordinator = createMockLoginCoordinator(secretsManager); const mementoManager = new MementoManager(memento); const commands = new MockCommands(); @@ -77,6 +79,7 @@ function createTestContext() { getMementoManager: () => mementoManager, getLoginCoordinator: () => loginCoordinator as unknown as LoginCoordinator, getContextManager: () => new MockContextManager(), + getOAuthCallback: () => oauthCallback, getLogger: () => logger, } as unknown as ServiceContainer; @@ -108,6 +111,7 @@ function createTestContext() { deploymentManager, loginCoordinator, secretsManager, + oauthCallback, logger, showErrorMessage, chatPanelProvider, @@ -358,10 +362,10 @@ describe("uriHandler", () => { } it("stores OAuth callback with code and state", async () => { - const { handleUri, secretsManager } = createTestContext(); + const { handleUri, oauthCallback } = createTestContext(); const callbackPromise = new Promise((resolve) => { - secretsManager.oauthCallback.onReceive(resolve); + oauthCallback.onReceive(resolve); }); await handleUri( @@ -377,10 +381,10 @@ describe("uriHandler", () => { }); it("stores OAuth callback with error", async () => { - const { handleUri, secretsManager } = createTestContext(); + const { handleUri, oauthCallback } = createTestContext(); const callbackPromise = new Promise((resolve) => { - secretsManager.oauthCallback.onReceive(resolve); + oauthCallback.onReceive(resolve); }); await handleUri( @@ -396,10 +400,10 @@ describe("uriHandler", () => { }); it("does not store callback when state is missing", async () => { - const { handleUri, secretsManager } = createTestContext(); + const { handleUri, oauthCallback } = createTestContext(); let callbackReceived = false; - secretsManager.oauthCallback.onReceive(() => { + oauthCallback.onReceive(() => { callbackReceived = true; }); diff --git a/test/unit/windowBroadcast.test.ts b/test/unit/windowBroadcast.test.ts deleted file mode 100644 index 421c8564ec..0000000000 --- a/test/unit/windowBroadcast.test.ts +++ /dev/null @@ -1,91 +0,0 @@ -import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; - -import { WindowBroadcast } from "../../src/windowBroadcast"; -import { InMemorySecretStorage, createMockLogger } from "../mocks/testHelpers"; - -interface TestMessage { - kind: string; - value: number; -} - -function isTestMessage(v: unknown): v is TestMessage { - if (typeof v !== "object" || v === null) { - return false; - } - const obj = v as Record; - return typeof obj.kind === "string" && typeof obj.value === "number"; -} - -function createBroadcast(key = "test.channel") { - const secrets = new InMemorySecretStorage(); - const logger = createMockLogger(); - const broadcast = new WindowBroadcast(secrets, key, isTestMessage, logger); - return { secrets, logger, broadcast }; -} - -describe("WindowBroadcast", () => { - beforeEach(() => vi.useFakeTimers()); - afterEach(() => vi.useRealTimers()); - - it("delivers sent messages to receivers", async () => { - const { broadcast } = createBroadcast(); - const handler = vi.fn(); - broadcast.onReceive(handler); - - await broadcast.send({ kind: "greeting", value: 42 }); - await vi.advanceTimersByTimeAsync(10); - - expect(handler).toHaveBeenCalledWith({ kind: "greeting", value: 42 }); - }); - - it("ignores messages on other keys", async () => { - const { secrets } = createBroadcast("my.key"); - const other = new WindowBroadcast( - secrets, - "other.key", - isTestMessage, - createMockLogger(), - ); - - const handler = vi.fn(); - other.onReceive(handler); - - await secrets.store("my.key", JSON.stringify({ kind: "x", value: 1 })); - await vi.advanceTimersByTimeAsync(10); - - expect(handler).not.toHaveBeenCalled(); - }); - - it("ignores messages that fail validation", async () => { - const { secrets, broadcast } = createBroadcast(); - const handler = vi.fn(); - broadcast.onReceive(handler); - - await secrets.store("test.channel", JSON.stringify({ bad: "shape" })); - await vi.advanceTimersByTimeAsync(10); - - expect(handler).not.toHaveBeenCalled(); - }); - - it("ignores malformed JSON without crashing", async () => { - const { secrets, broadcast } = createBroadcast(); - const handler = vi.fn(); - broadcast.onReceive(handler); - - await secrets.store("test.channel", "{not json"); - await vi.advanceTimersByTimeAsync(10); - - expect(handler).not.toHaveBeenCalled(); - }); - - it("stops delivering after dispose", async () => { - const { broadcast } = createBroadcast(); - const handler = vi.fn(); - broadcast.onReceive(handler).dispose(); - - await broadcast.send({ kind: "late", value: 0 }); - await vi.advanceTimersByTimeAsync(10); - - expect(handler).not.toHaveBeenCalled(); - }); -}); diff --git a/test/unit/windowIpc.test.ts b/test/unit/windowIpc.test.ts deleted file mode 100644 index dca6451d29..0000000000 --- a/test/unit/windowIpc.test.ts +++ /dev/null @@ -1,94 +0,0 @@ -import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; - -import { WindowIpc } from "../../src/windowIpc"; -import { InMemorySecretStorage, createMockLogger } from "../mocks/testHelpers"; - -function createIpcPair() { - const secrets = new InMemorySecretStorage(); - const logger = createMockLogger(); - return { - secrets, - sender: new WindowIpc(secrets, logger), - receiver: new WindowIpc(secrets, logger), - }; -} - -describe("WindowIpc", () => { - beforeEach(() => vi.useFakeTimers()); - afterEach(() => vi.useRealTimers()); - - describe("sendPing", () => { - it("resolves with PONG when another window responds", async () => { - const { sender, receiver } = createIpcPair(); - receiver.onRequest(async (msg) => { - if (msg.type === "ping") { - await receiver.sendPong(msg.id, "session-abc", "/home/coder/project"); - } - }); - - const promise = sender.sendPing("ssh-remote+my-host", 2000); - await vi.advanceTimersByTimeAsync(50); - - expect(await promise).toMatchObject({ - sessionId: "session-abc", - folder: "/home/coder/project", - }); - }); - - it("resolves undefined when no window responds", async () => { - const { sender } = createIpcPair(); - const promise = sender.sendPing("ssh-remote+my-host", 100); - await vi.advanceTimersByTimeAsync(150); - - expect(await promise).toBeUndefined(); - }); - }); - - it("onRequest ignores stale messages", async () => { - const { secrets, receiver } = createIpcPair(); - const handler = vi.fn(); - receiver.onRequest(handler); - - await secrets.store( - "coder.ipc.req", - JSON.stringify({ - type: "ping", - id: "old", - authority: "ssh-remote+host", - ts: Date.now() - 10_000, - }), - ); - await vi.advanceTimersByTimeAsync(10); - - expect(handler).not.toHaveBeenCalled(); - }); - - it("full ping → pong → duplicate round trip", async () => { - const { secrets, sender } = createIpcPair(); - const duplicated = vi.fn(); - - const windowA = new WindowIpc(secrets, createMockLogger()); - windowA.onRequest(async (msg) => { - if (msg.type === "ping" && msg.authority === "ssh-remote+host") { - await windowA.sendPong(msg.id, "win-a", "/home/coder/app"); - } - if (msg.type === "duplicate" && msg.targetSessionId === "win-a") { - duplicated(); - } - }); - - const promise = sender.sendPing("ssh-remote+host", 2000); - await vi.advanceTimersByTimeAsync(50); - const pong = await promise; - - expect(pong).toMatchObject({ - sessionId: "win-a", - folder: "/home/coder/app", - }); - - await sender.sendDuplicate("win-a"); - await vi.advanceTimersByTimeAsync(10); - - expect(duplicated).toHaveBeenCalledOnce(); - }); -}); diff --git a/test/unit/workspace/duplicateWorkspaceIpc.test.ts b/test/unit/workspace/duplicateWorkspaceIpc.test.ts new file mode 100644 index 0000000000..ec8fa8f204 --- /dev/null +++ b/test/unit/workspace/duplicateWorkspaceIpc.test.ts @@ -0,0 +1,100 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +import { DuplicateWorkspaceIpc } from "@/workspace/duplicateWorkspaceIpc"; + +import { + InMemorySecretStorage, + createMockLogger, +} from "../../mocks/testHelpers"; + +function makeIpcPair() { + const secrets = new InMemorySecretStorage(); + return { + secrets, + sender: new DuplicateWorkspaceIpc(secrets, createMockLogger()), + receiver: new DuplicateWorkspaceIpc(secrets, createMockLogger()), + }; +} + +describe("DuplicateWorkspaceIpc", () => { + beforeEach(() => vi.useFakeTimers()); + afterEach(() => vi.useRealTimers()); + + it("returns the PONG when another window responds to a PING", async () => { + const { sender, receiver } = makeIpcPair(); + receiver.onRequest(async (msg) => { + if (msg.type === "ping") { + await receiver.sendPong(msg.id, "session-abc"); + } + }); + + const promise = sender.sendPing("ssh-remote+my-host", 2000); + await vi.advanceTimersByTimeAsync(50); + const pong = await promise; + + expect(pong).toMatchObject({ + type: "pong", + sessionId: "session-abc", + }); + }); + + it("resolves to undefined when no window answers within the timeout", async () => { + const { sender } = makeIpcPair(); + const promise = sender.sendPing("ssh-remote+my-host", 100); + await vi.advanceTimersByTimeAsync(150); + + expect(await promise).toBeUndefined(); + }); + + it("delivers fresh requests but drops stale ones", async () => { + const { secrets, sender, receiver } = makeIpcPair(); + const handler = vi.fn(); + receiver.onRequest(handler); + + // Replay an old request directly through storage. + await secrets.store( + "coder.ipc.req", + JSON.stringify({ + type: "ping", + id: "old", + authority: "ssh-remote+host", + ts: Date.now() - 10_000, + }), + ); + await vi.advanceTimersByTimeAsync(10); + expect(handler).not.toHaveBeenCalled(); + + // A fresh sendDuplicate should be delivered. + await sender.sendDuplicate("session-fresh"); + await vi.advanceTimersByTimeAsync(10); + expect(handler).toHaveBeenCalledWith( + expect.objectContaining({ + type: "duplicate", + targetSessionId: "session-fresh", + }), + ); + }); + + it("supports the full ping → pong → duplicate round trip", async () => { + const { secrets, sender } = makeIpcPair(); + const duplicateReceived = vi.fn(); + + const peer = new DuplicateWorkspaceIpc(secrets, createMockLogger()); + peer.onRequest(async (msg) => { + if (msg.type === "ping" && msg.authority === "ssh-remote+host") { + await peer.sendPong(msg.id, "win-a"); + } else if (msg.type === "duplicate" && msg.targetSessionId === "win-a") { + duplicateReceived(); + } + }); + + const promise = sender.sendPing("ssh-remote+host", 2000); + await vi.advanceTimersByTimeAsync(50); + const pong = await promise; + expect(pong).toMatchObject({ sessionId: "win-a" }); + + await sender.sendDuplicate("win-a"); + await vi.advanceTimersByTimeAsync(10); + expect(duplicateReceived).toHaveBeenCalledOnce(); + }); +}); From 2f53028f68ba954794744f2366c705c2e47a9eb7 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Tue, 28 Apr 2026 15:03:51 +0300 Subject: [PATCH 4/4] refactor: drop unused message timestamp and max-age filter --- CHANGELOG.md | 6 ++++ src/workspace/duplicateWorkspaceIpc.ts | 16 ++-------- .../workspace/duplicateWorkspaceIpc.test.ts | 29 ------------------- 3 files changed, 8 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab29d5747e..b0aba83d84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ ## Unreleased +### Added + +- Opening a workspace that's already connected in another VS Code window now shows a prompt + to **Duplicate Window** (preserving tabs and panels) or **Open Without Folder**, instead of + just focusing the existing window with no way to open a second view of the same workspace. + ### Fixed - The **Coder: Workspace Build** output channel is no longer created when reconnecting to an diff --git a/src/workspace/duplicateWorkspaceIpc.ts b/src/workspace/duplicateWorkspaceIpc.ts index 3b100a39b3..f456e24fda 100644 --- a/src/workspace/duplicateWorkspaceIpc.ts +++ b/src/workspace/duplicateWorkspaceIpc.ts @@ -7,7 +7,6 @@ import type { Disposable, SecretStorage } from "vscode"; import type { Logger } from "../logging/logger"; -const MESSAGE_MAX_AGE_MS = 5000; const DEFAULT_PING_TIMEOUT_MS = 1000; const REQUEST_KEY = "coder.ipc.req"; @@ -17,21 +16,18 @@ const PingMessageSchema = z.object({ type: z.literal("ping"), id: z.string(), authority: z.string(), - ts: z.number(), }); const PongMessageSchema = z.object({ type: z.literal("pong"), id: z.string(), sessionId: z.string(), - ts: z.number(), }); const DuplicateMessageSchema = z.object({ type: z.literal("duplicate"), id: z.string(), targetSessionId: z.string(), - ts: z.number(), }); const RequestMessageSchema = z.discriminatedUnion("type", [ @@ -104,7 +100,7 @@ export class DuplicateWorkspaceIpc { const timer = setTimeout(() => settle(undefined), timeoutMs); this.requests - .send({ type: "ping", id, authority, ts: Date.now() }) + .send({ type: "ping", id, authority }) .then(undefined, (err: unknown) => { this.logger.error("Failed to send IPC ping", err); settle(undefined); @@ -117,7 +113,6 @@ export class DuplicateWorkspaceIpc { type: "pong", id: pingId, sessionId, - ts: Date.now(), }); } @@ -127,19 +122,12 @@ export class DuplicateWorkspaceIpc { type: "duplicate", id: crypto.randomUUID(), targetSessionId, - ts: Date.now(), }); } - /** Listen for incoming requests. Stale messages are dropped. */ onRequest( handler: (msg: RequestMessage) => void | Promise, ): Disposable { - return this.requests.onReceive((msg) => { - if (Date.now() - msg.ts > MESSAGE_MAX_AGE_MS) { - return; - } - return handler(msg); - }); + return this.requests.onReceive(handler); } } diff --git a/test/unit/workspace/duplicateWorkspaceIpc.test.ts b/test/unit/workspace/duplicateWorkspaceIpc.test.ts index ec8fa8f204..e3e7189f2a 100644 --- a/test/unit/workspace/duplicateWorkspaceIpc.test.ts +++ b/test/unit/workspace/duplicateWorkspaceIpc.test.ts @@ -46,35 +46,6 @@ describe("DuplicateWorkspaceIpc", () => { expect(await promise).toBeUndefined(); }); - it("delivers fresh requests but drops stale ones", async () => { - const { secrets, sender, receiver } = makeIpcPair(); - const handler = vi.fn(); - receiver.onRequest(handler); - - // Replay an old request directly through storage. - await secrets.store( - "coder.ipc.req", - JSON.stringify({ - type: "ping", - id: "old", - authority: "ssh-remote+host", - ts: Date.now() - 10_000, - }), - ); - await vi.advanceTimersByTimeAsync(10); - expect(handler).not.toHaveBeenCalled(); - - // A fresh sendDuplicate should be delivered. - await sender.sendDuplicate("session-fresh"); - await vi.advanceTimersByTimeAsync(10); - expect(handler).toHaveBeenCalledWith( - expect.objectContaining({ - type: "duplicate", - targetSessionId: "session-fresh", - }), - ); - }); - it("supports the full ping → pong → duplicate round trip", async () => { const { secrets, sender } = makeIpcPair(); const duplicateReceived = vi.fn();