From 728dbc3f761e1611079e32fb3b56e1ccf420be5f Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Wed, 1 Oct 2025 11:35:08 +0300 Subject: [PATCH 1/3] Fix subscriptions, components, and watchers not disposed on extension deactivation --- src/api/coderApi.ts | 42 +-- src/commands.ts | 4 +- src/core/container.ts | 2 +- src/extension.ts | 140 ++++++--- src/remote/remote.ts | 453 ++++++++++++++-------------- src/workspace/workspacesProvider.ts | 36 ++- 6 files changed, 360 insertions(+), 317 deletions(-) diff --git a/src/api/coderApi.ts b/src/api/coderApi.ts index 6c6c0faf..1d73ef00 100644 --- a/src/api/coderApi.ts +++ b/src/api/coderApi.ts @@ -7,7 +7,7 @@ import { type Workspace, type WorkspaceAgent, } from "coder/site/src/api/typesGenerated"; -import { type WorkspaceConfiguration } from "vscode"; +import * as vscode from "vscode"; import { type ClientOptions } from "ws"; import { CertificateError } from "../error"; @@ -33,17 +33,12 @@ import { createHttpAgent } from "./utils"; const coderSessionTokenHeader = "Coder-Session-Token"; -type WorkspaceConfigurationProvider = () => WorkspaceConfiguration; - /** * Unified API class that includes both REST API methods from the base Api class * and WebSocket methods for real-time functionality. */ export class CoderApi extends Api { - private constructor( - private readonly output: Logger, - private readonly configProvider: WorkspaceConfigurationProvider, - ) { + private constructor(private readonly output: Logger) { super(); } @@ -55,15 +50,14 @@ export class CoderApi extends Api { baseUrl: string, token: string | undefined, output: Logger, - configProvider: WorkspaceConfigurationProvider, ): CoderApi { - const client = new CoderApi(output, configProvider); + const client = new CoderApi(output); client.setHost(baseUrl); if (token) { client.setSessionToken(token); } - setupInterceptors(client, baseUrl, output, configProvider); + setupInterceptors(client, baseUrl, output); return client; } @@ -127,7 +121,7 @@ export class CoderApi extends Api { coderSessionTokenHeader ] as string | undefined; - const httpAgent = createHttpAgent(this.configProvider()); + const httpAgent = createHttpAgent(vscode.workspace.getConfiguration()); const webSocket = new OneWayWebSocket({ location: baseUrl, ...configs, @@ -174,14 +168,13 @@ function setupInterceptors( client: CoderApi, baseUrl: string, output: Logger, - configProvider: WorkspaceConfigurationProvider, ): void { - addLoggingInterceptors(client.getAxiosInstance(), output, configProvider); + addLoggingInterceptors(client.getAxiosInstance(), output); client.getAxiosInstance().interceptors.request.use(async (config) => { const headers = await getHeaders( baseUrl, - getHeaderCommand(configProvider()), + getHeaderCommand(vscode.workspace.getConfiguration()), output, ); // Add headers from the header command. @@ -192,7 +185,7 @@ function setupInterceptors( // Configure proxy and TLS. // Note that by default VS Code overrides the agent. To prevent this, set // `http.proxySupport` to `on` or `off`. - const agent = createHttpAgent(configProvider()); + const agent = createHttpAgent(vscode.workspace.getConfiguration()); config.httpsAgent = agent; config.httpAgent = agent; config.proxy = false; @@ -209,38 +202,35 @@ function setupInterceptors( ); } -function addLoggingInterceptors( - client: AxiosInstance, - logger: Logger, - configProvider: WorkspaceConfigurationProvider, -) { +function addLoggingInterceptors(client: AxiosInstance, logger: Logger) { client.interceptors.request.use( (config) => { const configWithMeta = config as RequestConfigWithMeta; configWithMeta.metadata = createRequestMeta(); - logRequest(logger, configWithMeta, getLogLevel(configProvider())); + logRequest(logger, configWithMeta, getLogLevel()); return config; }, (error: unknown) => { - logError(logger, error, getLogLevel(configProvider())); + logError(logger, error, getLogLevel()); return Promise.reject(error); }, ); client.interceptors.response.use( (response) => { - logResponse(logger, response, getLogLevel(configProvider())); + logResponse(logger, response, getLogLevel()); return response; }, (error: unknown) => { - logError(logger, error, getLogLevel(configProvider())); + logError(logger, error, getLogLevel()); return Promise.reject(error); }, ); } -function getLogLevel(cfg: WorkspaceConfiguration): HttpClientLogLevel { - const logLevelStr = cfg +function getLogLevel(): HttpClientLogLevel { + const logLevelStr = vscode.workspace + .getConfiguration() .get( "coder.httpClientLogLevel", HttpClientLogLevel[HttpClientLogLevel.BASIC], diff --git a/src/commands.ts b/src/commands.ts index 462010ba..bd4071cc 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -260,9 +260,7 @@ export class Commands { token: string, isAutologin: boolean, ): Promise<{ user: User; token: string } | null> { - const client = CoderApi.create(url, token, this.logger, () => - vscode.workspace.getConfiguration(), - ); + const client = CoderApi.create(url, token, this.logger); if (!needToken(vscode.workspace.getConfiguration())) { try { const user = await client.getAuthenticatedUser(); diff --git a/src/core/container.ts b/src/core/container.ts index f820bb0d..72f28088 100644 --- a/src/core/container.ts +++ b/src/core/container.ts @@ -11,7 +11,7 @@ import { SecretsManager } from "./secretsManager"; * Service container for dependency injection. * Centralizes the creation and management of all core services. */ -export class ServiceContainer { +export class ServiceContainer implements vscode.Disposable { private readonly logger: vscode.LogOutputChannel; private readonly pathResolver: PathResolver; private readonly mementoManager: MementoManager; diff --git a/src/extension.ts b/src/extension.ts index 982342eb..decdb3a9 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -57,6 +57,8 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { } const serviceContainer = new ServiceContainer(ctx, vscodeProposed); + ctx.subscriptions.push(serviceContainer); + const output = serviceContainer.getLogger(); const mementoManager = serviceContainer.getMementoManager(); const secretsManager = serviceContainer.getSecretsManager(); @@ -72,7 +74,6 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { url || "", await secretsManager.getSessionToken(), output, - () => vscode.workspace.getConfiguration(), ); const myWorkspacesProvider = new WorkspaceProvider( @@ -81,33 +82,47 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { output, 5, ); + ctx.subscriptions.push(myWorkspacesProvider); + const allWorkspacesProvider = new WorkspaceProvider( WorkspaceQuery.All, client, output, ); + ctx.subscriptions.push(allWorkspacesProvider); // createTreeView, unlike registerTreeDataProvider, gives us the tree view API // (so we can see when it is visible) but otherwise they have the same effect. const myWsTree = vscode.window.createTreeView(MY_WORKSPACES_TREE_ID, { treeDataProvider: myWorkspacesProvider, }); + ctx.subscriptions.push(myWsTree); myWorkspacesProvider.setVisibility(myWsTree.visible); - myWsTree.onDidChangeVisibility((event) => { - myWorkspacesProvider.setVisibility(event.visible); - }); + myWsTree.onDidChangeVisibility( + (event) => { + myWorkspacesProvider.setVisibility(event.visible); + }, + undefined, + ctx.subscriptions, + ); const allWsTree = vscode.window.createTreeView(ALL_WORKSPACES_TREE_ID, { treeDataProvider: allWorkspacesProvider, }); + ctx.subscriptions.push(allWsTree); allWorkspacesProvider.setVisibility(allWsTree.visible); - allWsTree.onDidChangeVisibility((event) => { - allWorkspacesProvider.setVisibility(event.visible); - }); + allWsTree.onDidChangeVisibility( + (event) => { + allWorkspacesProvider.setVisibility(event.visible); + }, + undefined, + ctx.subscriptions, + ); // Handle vscode:// URIs. - vscode.window.registerUriHandler({ + const uriHandler = vscode.window.registerUriHandler({ handleUri: async (uri) => { + const cliManager = serviceContainer.getCliManager(); const params = new URLSearchParams(uri.query); if (uri.path === "/open") { const owner = params.get("owner"); @@ -253,59 +268,89 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { } }, }); - - const cliManager = serviceContainer.getCliManager(); + ctx.subscriptions.push(uriHandler); // Register globally available commands. Many of these have visibility // controlled by contexts, see `when` in the package.json. const commands = new Commands(serviceContainer, client); - vscode.commands.registerCommand("coder.login", commands.login.bind(commands)); - vscode.commands.registerCommand( - "coder.logout", - commands.logout.bind(commands), + ctx.subscriptions.push( + vscode.commands.registerCommand( + "coder.login", + commands.login.bind(commands), + ), ); - vscode.commands.registerCommand("coder.open", commands.open.bind(commands)); - vscode.commands.registerCommand( - "coder.openDevContainer", - commands.openDevContainer.bind(commands), + ctx.subscriptions.push( + vscode.commands.registerCommand( + "coder.logout", + commands.logout.bind(commands), + ), ); - vscode.commands.registerCommand( - "coder.openFromSidebar", - commands.openFromSidebar.bind(commands), + ctx.subscriptions.push( + vscode.commands.registerCommand("coder.open", commands.open.bind(commands)), ); - vscode.commands.registerCommand( - "coder.openAppStatus", - commands.openAppStatus.bind(commands), + ctx.subscriptions.push( + vscode.commands.registerCommand( + "coder.openDevContainer", + commands.openDevContainer.bind(commands), + ), ); - vscode.commands.registerCommand( - "coder.workspace.update", - commands.updateWorkspace.bind(commands), + ctx.subscriptions.push( + vscode.commands.registerCommand( + "coder.openFromSidebar", + commands.openFromSidebar.bind(commands), + ), ); - vscode.commands.registerCommand( - "coder.createWorkspace", - commands.createWorkspace.bind(commands), + ctx.subscriptions.push( + vscode.commands.registerCommand( + "coder.openAppStatus", + commands.openAppStatus.bind(commands), + ), ); - vscode.commands.registerCommand( - "coder.navigateToWorkspace", - commands.navigateToWorkspace.bind(commands), + ctx.subscriptions.push( + vscode.commands.registerCommand( + "coder.workspace.update", + commands.updateWorkspace.bind(commands), + ), ); - vscode.commands.registerCommand( - "coder.navigateToWorkspaceSettings", - commands.navigateToWorkspaceSettings.bind(commands), + ctx.subscriptions.push( + vscode.commands.registerCommand( + "coder.createWorkspace", + commands.createWorkspace.bind(commands), + ), ); - vscode.commands.registerCommand("coder.refreshWorkspaces", () => { - myWorkspacesProvider.fetchAndRefresh(); - allWorkspacesProvider.fetchAndRefresh(); - }); - vscode.commands.registerCommand( - "coder.viewLogs", - commands.viewLogs.bind(commands), + ctx.subscriptions.push( + vscode.commands.registerCommand( + "coder.navigateToWorkspace", + commands.navigateToWorkspace.bind(commands), + ), + ); + ctx.subscriptions.push( + vscode.commands.registerCommand( + "coder.navigateToWorkspaceSettings", + commands.navigateToWorkspaceSettings.bind(commands), + ), + ); + ctx.subscriptions.push( + vscode.commands.registerCommand("coder.refreshWorkspaces", () => { + myWorkspacesProvider.fetchAndRefresh(); + allWorkspacesProvider.fetchAndRefresh(); + }), + ); + ctx.subscriptions.push( + vscode.commands.registerCommand( + "coder.viewLogs", + commands.viewLogs.bind(commands), + ), ); - vscode.commands.registerCommand("coder.searchMyWorkspaces", async () => - showTreeViewSearch(MY_WORKSPACES_TREE_ID), + ctx.subscriptions.push( + vscode.commands.registerCommand("coder.searchMyWorkspaces", async () => + showTreeViewSearch(MY_WORKSPACES_TREE_ID), + ), ); - vscode.commands.registerCommand("coder.searchAllWorkspaces", async () => - showTreeViewSearch(ALL_WORKSPACES_TREE_ID), + ctx.subscriptions.push( + vscode.commands.registerCommand("coder.searchAllWorkspaces", async () => + showTreeViewSearch(ALL_WORKSPACES_TREE_ID), + ), ); // Since the "onResolveRemoteAuthority:ssh-remote" activation event exists @@ -325,6 +370,7 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { isFirstConnect, ); if (details) { + ctx.subscriptions.push(details); // Authenticate the plugin client which is used in the sidebar to display // workspaces belonging to this deployment. client.setHost(details.url); diff --git a/src/remote/remote.ts b/src/remote/remote.ts index baf7b28c..e6ebb8a0 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -276,12 +276,7 @@ export class Remote { // break this connection. We could force close the remote session or // disallow logging out/in altogether, but for now just use a separate // client to remain unaffected by whatever the plugin is doing. - const workspaceClient = CoderApi.create( - baseUrlRaw, - token, - this.logger, - () => vscode.workspace.getConfiguration(), - ); + const workspaceClient = CoderApi.create(baseUrlRaw, token, this.logger); // Store for use in commands. this.commands.workspaceRestClient = workspaceClient; @@ -398,256 +393,262 @@ export class Remote { } const disposables: vscode.Disposable[] = []; - // Register before connection so the label still displays! - disposables.push( - this.registerLabelFormatter( + try { + // Register before connection so the label still displays! + let labelFormatterDisposable = this.registerLabelFormatter( remoteAuthority, workspace.owner_name, workspace.name, - ), - ); - - // If the workspace is not in a running state, try to get it running. - if (workspace.latest_build.status !== "running") { - const updatedWorkspace = await this.maybeWaitForRunning( - workspaceClient, - workspace, - parts.label, - binaryPath, - featureSet, - firstConnect, ); - if (!updatedWorkspace) { - // User declined to start the workspace. + disposables.push(labelFormatterDisposable); + + // If the workspace is not in a running state, try to get it running. + if (workspace.latest_build.status !== "running") { + const updatedWorkspace = await this.maybeWaitForRunning( + workspaceClient, + workspace, + parts.label, + binaryPath, + featureSet, + firstConnect, + ); + if (!updatedWorkspace) { + // User declined to start the workspace. + await this.closeRemote(); + return; + } + workspace = updatedWorkspace; + } + this.commands.workspace = workspace; + + // Pick an agent. + this.logger.info(`Finding agent for ${workspaceName}...`); + const agents = extractAgents(workspace.latest_build.resources); + const gotAgent = await this.commands.maybeAskAgent(agents, parts.agent); + if (!gotAgent) { + // User declined to pick an agent. await this.closeRemote(); return; } - workspace = updatedWorkspace; - } - this.commands.workspace = workspace; - - // Pick an agent. - this.logger.info(`Finding agent for ${workspaceName}...`); - const agents = extractAgents(workspace.latest_build.resources); - const gotAgent = await this.commands.maybeAskAgent(agents, parts.agent); - if (!gotAgent) { - // User declined to pick an agent. - await this.closeRemote(); - return; - } - let agent = gotAgent; // Reassign so it cannot be undefined in callbacks. - this.logger.info(`Found agent ${agent.name} with status`, agent.status); - - // Do some janky setting manipulation. - this.logger.info("Modifying settings..."); - const remotePlatforms = this.vscodeProposed.workspace - .getConfiguration() - .get>("remote.SSH.remotePlatform", {}); - const connTimeout = this.vscodeProposed.workspace - .getConfiguration() - .get("remote.SSH.connectTimeout"); - - // We have to directly munge the settings file with jsonc because trying to - // update properly through the extension API hangs indefinitely. Possibly - // VS Code is trying to update configuration on the remote, which cannot - // connect until we finish here leading to a deadlock. We need to update it - // locally, anyway, and it does not seem possible to force that via API. - let settingsContent = "{}"; - try { - settingsContent = await fs.readFile( - this.pathResolver.getUserSettingsPath(), - "utf8", - ); - } catch { - // Ignore! It's probably because the file doesn't exist. - } - - // Add the remote platform for this host to bypass a step where VS Code asks - // the user for the platform. - let mungedPlatforms = false; - if ( - !remotePlatforms[parts.host] || - remotePlatforms[parts.host] !== agent.operating_system - ) { - remotePlatforms[parts.host] = agent.operating_system; - settingsContent = jsonc.applyEdits( - settingsContent, - jsonc.modify( - settingsContent, - ["remote.SSH.remotePlatform"], - remotePlatforms, - {}, - ), - ); - mungedPlatforms = true; - } + let agent = gotAgent; // Reassign so it cannot be undefined in callbacks. + this.logger.info(`Found agent ${agent.name} with status`, agent.status); + + // Do some janky setting manipulation. + this.logger.info("Modifying settings..."); + const remotePlatforms = this.vscodeProposed.workspace + .getConfiguration() + .get>("remote.SSH.remotePlatform", {}); + const connTimeout = this.vscodeProposed.workspace + .getConfiguration() + .get("remote.SSH.connectTimeout"); + + // We have to directly munge the settings file with jsonc because trying to + // update properly through the extension API hangs indefinitely. Possibly + // VS Code is trying to update configuration on the remote, which cannot + // connect until we finish here leading to a deadlock. We need to update it + // locally, anyway, and it does not seem possible to force that via API. + let settingsContent = "{}"; + try { + settingsContent = await fs.readFile( + this.pathResolver.getUserSettingsPath(), + "utf8", + ); + } catch { + // Ignore! It's probably because the file doesn't exist. + } - // VS Code ignores the connect timeout in the SSH config and uses a default - // of 15 seconds, which can be too short in the case where we wait for - // startup scripts. For now we hardcode a longer value. Because this is - // potentially overwriting user configuration, it feels a bit sketchy. If - // microsoft/vscode-remote-release#8519 is resolved we can remove this. - const minConnTimeout = 1800; - let mungedConnTimeout = false; - if (!connTimeout || connTimeout < minConnTimeout) { - settingsContent = jsonc.applyEdits( - settingsContent, - jsonc.modify( + // Add the remote platform for this host to bypass a step where VS Code asks + // the user for the platform. + let mungedPlatforms = false; + if ( + !remotePlatforms[parts.host] || + remotePlatforms[parts.host] !== agent.operating_system + ) { + remotePlatforms[parts.host] = agent.operating_system; + settingsContent = jsonc.applyEdits( settingsContent, - ["remote.SSH.connectTimeout"], - minConnTimeout, - {}, - ), - ); - mungedConnTimeout = true; - } + jsonc.modify( + settingsContent, + ["remote.SSH.remotePlatform"], + remotePlatforms, + {}, + ), + ); + mungedPlatforms = true; + } - if (mungedPlatforms || mungedConnTimeout) { - try { - await fs.writeFile( - this.pathResolver.getUserSettingsPath(), + // VS Code ignores the connect timeout in the SSH config and uses a default + // of 15 seconds, which can be too short in the case where we wait for + // startup scripts. For now we hardcode a longer value. Because this is + // potentially overwriting user configuration, it feels a bit sketchy. If + // microsoft/vscode-remote-release#8519 is resolved we can remove this. + const minConnTimeout = 1800; + let mungedConnTimeout = false; + if (!connTimeout || connTimeout < minConnTimeout) { + settingsContent = jsonc.applyEdits( settingsContent, + jsonc.modify( + settingsContent, + ["remote.SSH.connectTimeout"], + minConnTimeout, + {}, + ), ); - } catch (ex) { - // This could be because the user's settings.json is read-only. This is - // the case when using home-manager on NixOS, for example. Failure to - // write here is not necessarily catastrophic since the user will be - // asked for the platform and the default timeout might be sufficient. - mungedPlatforms = mungedConnTimeout = false; - this.logger.warn("Failed to configure settings", ex); + mungedConnTimeout = true; } - } - // Watch the workspace for changes. - const monitor = new WorkspaceMonitor( - workspace, - workspaceClient, - this.logger, - this.vscodeProposed, - ); - disposables.push(monitor); - disposables.push( - monitor.onChange.event((w) => (this.commands.workspace = w)), - ); + if (mungedPlatforms || mungedConnTimeout) { + try { + await fs.writeFile( + this.pathResolver.getUserSettingsPath(), + settingsContent, + ); + } catch (ex) { + // This could be because the user's settings.json is read-only. This is + // the case when using home-manager on NixOS, for example. Failure to + // write here is not necessarily catastrophic since the user will be + // asked for the platform and the default timeout might be sufficient. + mungedPlatforms = mungedConnTimeout = false; + this.logger.warn("Failed to configure settings", ex); + } + } - // Watch coder inbox for messages - const inbox = new Inbox(workspace, workspaceClient, this.logger); - disposables.push(inbox); + // Watch the workspace for changes. + const monitor = new WorkspaceMonitor( + workspace, + workspaceClient, + this.logger, + this.vscodeProposed, + ); + disposables.push(monitor); + disposables.push( + monitor.onChange.event((w) => (this.commands.workspace = w)), + ); - // Wait for the agent to connect. - if (agent.status === "connecting") { - this.logger.info(`Waiting for ${workspaceName}/${agent.name}...`); - await vscode.window.withProgress( - { - title: "Waiting for the agent to connect...", - location: vscode.ProgressLocation.Notification, - }, - async () => { - await new Promise((resolve) => { - const updateEvent = monitor.onChange.event((workspace) => { - if (!agent) { - return; - } - const agents = extractAgents(workspace.latest_build.resources); - const found = agents.find((newAgent) => { - return newAgent.id === agent.id; + // Watch coder inbox for messages + const inbox = new Inbox(workspace, workspaceClient, this.logger); + disposables.push(inbox); + + // Wait for the agent to connect. + if (agent.status === "connecting") { + this.logger.info(`Waiting for ${workspaceName}/${agent.name}...`); + await vscode.window.withProgress( + { + title: "Waiting for the agent to connect...", + location: vscode.ProgressLocation.Notification, + }, + async () => { + await new Promise((resolve) => { + const updateEvent = monitor.onChange.event((workspace) => { + if (!agent) { + return; + } + const agents = extractAgents(workspace.latest_build.resources); + const found = agents.find((newAgent) => { + return newAgent.id === agent.id; + }); + if (!found) { + return; + } + agent = found; + if (agent.status === "connecting") { + return; + } + updateEvent.dispose(); + resolve(); }); - if (!found) { - return; - } - agent = found; - if (agent.status === "connecting") { - return; - } - updateEvent.dispose(); - resolve(); }); - }); - }, - ); - this.logger.info(`Agent ${agent.name} status is now`, agent.status); - } + }, + ); + this.logger.info(`Agent ${agent.name} status is now`, agent.status); + } - // Make sure the agent is connected. - // TODO: Should account for the lifecycle state as well? - if (agent.status !== "connected") { - const result = await this.vscodeProposed.window.showErrorMessage( - `${workspaceName}/${agent.name} ${agent.status}`, - { - useCustom: true, - modal: true, - detail: `The ${agent.name} agent failed to connect. Try restarting your workspace.`, - }, - ); - if (!result) { - await this.closeRemote(); + // Make sure the agent is connected. + // TODO: Should account for the lifecycle state as well? + if (agent.status !== "connected") { + const result = await this.vscodeProposed.window.showErrorMessage( + `${workspaceName}/${agent.name} ${agent.status}`, + { + useCustom: true, + modal: true, + detail: `The ${agent.name} agent failed to connect. Try restarting your workspace.`, + }, + ); + if (!result) { + await this.closeRemote(); + return; + } + await this.reloadWindow(); return; } - await this.reloadWindow(); - return; - } - - const logDir = this.getLogDir(featureSet); - // This ensures the Remote SSH extension resolves the host to execute the - // Coder binary properly. - // - // If we didn't write to the SSH config file, connecting would fail with - // "Host not found". - try { - this.logger.info("Updating SSH config..."); - await this.updateSSHConfig( - workspaceClient, - parts.label, - parts.host, - binaryPath, - logDir, - featureSet, - ); - } catch (error) { - this.logger.warn("Failed to configure SSH", error); - throw error; - } + const logDir = this.getLogDir(featureSet); - // TODO: This needs to be reworked; it fails to pick up reconnects. - this.findSSHProcessID().then(async (pid) => { - if (!pid) { - // TODO: Show an error here! - return; - } - disposables.push(this.showNetworkUpdates(pid)); - if (logDir) { - const logFiles = await fs.readdir(logDir); - const logFileName = logFiles - .reverse() - .find( - (file) => file === `${pid}.log` || file.endsWith(`-${pid}.log`), - ); - this.commands.workspaceLogPath = logFileName - ? path.join(logDir, logFileName) - : undefined; - } else { - this.commands.workspaceLogPath = undefined; + // This ensures the Remote SSH extension resolves the host to execute the + // Coder binary properly. + // + // If we didn't write to the SSH config file, connecting would fail with + // "Host not found". + try { + this.logger.info("Updating SSH config..."); + await this.updateSSHConfig( + workspaceClient, + parts.label, + parts.host, + binaryPath, + logDir, + featureSet, + ); + } catch (error) { + this.logger.warn("Failed to configure SSH", error); + throw error; } - }); - // Register the label formatter again because SSH overrides it! - disposables.push( - vscode.extensions.onDidChange(() => { - disposables.push( - this.registerLabelFormatter( + // TODO: This needs to be reworked; it fails to pick up reconnects. + this.findSSHProcessID().then(async (pid) => { + if (!pid) { + // TODO: Show an error here! + return; + } + disposables.push(this.showNetworkUpdates(pid)); + if (logDir) { + const logFiles = await fs.readdir(logDir); + const logFileName = logFiles + .reverse() + .find( + (file) => file === `${pid}.log` || file.endsWith(`-${pid}.log`), + ); + this.commands.workspaceLogPath = logFileName + ? path.join(logDir, logFileName) + : undefined; + } else { + this.commands.workspaceLogPath = undefined; + } + }); + + // Register the label formatter again because SSH overrides it! + disposables.push( + vscode.extensions.onDidChange(() => { + // Dispose previous label formatter + labelFormatterDisposable.dispose(); + labelFormatterDisposable = this.registerLabelFormatter( remoteAuthority, workspace.owner_name, workspace.name, agent.name, - ), - ); - }), - ); + ); + disposables.push(labelFormatterDisposable); + }), + ); - disposables.push( - ...this.createAgentMetadataStatusBar(agent, workspaceClient), - ); + disposables.push( + ...this.createAgentMetadataStatusBar(agent, workspaceClient), + ); + } catch (ex) { + // Whatever error happens, make sure we clean up the disposables in case of failure + disposables.forEach((d) => d.dispose()); + throw ex; + } this.logger.info("Remote setup complete"); diff --git a/src/workspace/workspacesProvider.ts b/src/workspace/workspacesProvider.ts index 915ef32a..b83e4f84 100644 --- a/src/workspace/workspacesProvider.ts +++ b/src/workspace/workspacesProvider.ts @@ -34,12 +34,12 @@ export enum WorkspaceQuery { * abort polling until fetchAndRefresh() is called again. */ export class WorkspaceProvider - implements vscode.TreeDataProvider + implements vscode.TreeDataProvider, vscode.Disposable { // Undefined if we have never fetched workspaces before. private workspaces: WorkspaceTreeItem[] | undefined; - private agentWatchers: Record = - {}; + private agentWatchers: Map = + new Map(); private timeout: NodeJS.Timeout | undefined; private fetching = false; private visible = false; @@ -121,7 +121,7 @@ export class WorkspaceProvider return this.fetch(); } - const oldWatcherIds = Object.keys(this.agentWatchers); + const oldWatcherIds = [...this.agentWatchers.keys()]; const reusedWatcherIds: string[] = []; // TODO: I think it might make more sense for the tree items to contain @@ -132,23 +132,23 @@ export class WorkspaceProvider const agents = extractAllAgents(resp.workspaces); agents.forEach((agent) => { // If we have an existing watcher, re-use it. - if (this.agentWatchers[agent.id]) { + const oldWatcher = this.agentWatchers.get(agent.id); + if (oldWatcher) { reusedWatcherIds.push(agent.id); - return this.agentWatchers[agent.id]; + } else { + // Otherwise create a new watcher. + const watcher = createAgentMetadataWatcher(agent.id, this.client); + watcher.onChange(() => this.refresh()); + this.agentWatchers.set(agent.id, watcher); } - // Otherwise create a new watcher. - const watcher = createAgentMetadataWatcher(agent.id, this.client); - watcher.onChange(() => this.refresh()); - this.agentWatchers[agent.id] = watcher; - return watcher; }); } // Dispose of watchers we ended up not reusing. oldWatcherIds.forEach((id) => { if (!reusedWatcherIds.includes(id)) { - this.agentWatchers[id].dispose(); - delete this.agentWatchers[id]; + this.agentWatchers.get(id)?.dispose(); + this.agentWatchers.delete(id); } }); @@ -244,7 +244,7 @@ export class WorkspaceProvider return Promise.resolve(agentTreeItems); } else if (element instanceof AgentTreeItem) { - const watcher = this.agentWatchers[element.agent.id]; + const watcher = this.agentWatchers.get(element.agent.id); if (watcher?.error) { return Promise.resolve([new ErrorTreeItem(watcher.error)]); } @@ -305,6 +305,14 @@ export class WorkspaceProvider } return Promise.resolve(this.workspaces || []); } + + dispose() { + this.cancelPendingRefresh(); + for (const watcher of this.agentWatchers.values()) { + watcher.dispose(); + } + this.agentWatchers.clear(); + } } /** From 9e08d641f464b8ff25a3e9bf3881847b2e175c86 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Wed, 1 Oct 2025 17:57:28 +0300 Subject: [PATCH 2/3] tsconfig simplification --- test/tsconfig.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/tsconfig.json b/test/tsconfig.json index ece5f0b1..1be61bbd 100644 --- a/test/tsconfig.json +++ b/test/tsconfig.json @@ -6,5 +6,5 @@ "@/*": ["src/*"] } }, - "include": ["**/*", "../src/**/*"] + "include": [".", "../src"] } From 994865961850e60498b2f2b87b48c9a574b24363 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Fri, 3 Oct 2025 11:39:00 +0300 Subject: [PATCH 3/3] Review comments --- src/extension.ts | 26 -------------------------- src/remote/remote.ts | 8 +++----- 2 files changed, 3 insertions(+), 31 deletions(-) diff --git a/src/extension.ts b/src/extension.ts index decdb3a9..e069c3a3 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -278,76 +278,50 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { "coder.login", commands.login.bind(commands), ), - ); - ctx.subscriptions.push( vscode.commands.registerCommand( "coder.logout", commands.logout.bind(commands), ), - ); - ctx.subscriptions.push( vscode.commands.registerCommand("coder.open", commands.open.bind(commands)), - ); - ctx.subscriptions.push( vscode.commands.registerCommand( "coder.openDevContainer", commands.openDevContainer.bind(commands), ), - ); - ctx.subscriptions.push( vscode.commands.registerCommand( "coder.openFromSidebar", commands.openFromSidebar.bind(commands), ), - ); - ctx.subscriptions.push( vscode.commands.registerCommand( "coder.openAppStatus", commands.openAppStatus.bind(commands), ), - ); - ctx.subscriptions.push( vscode.commands.registerCommand( "coder.workspace.update", commands.updateWorkspace.bind(commands), ), - ); - ctx.subscriptions.push( vscode.commands.registerCommand( "coder.createWorkspace", commands.createWorkspace.bind(commands), ), - ); - ctx.subscriptions.push( vscode.commands.registerCommand( "coder.navigateToWorkspace", commands.navigateToWorkspace.bind(commands), ), - ); - ctx.subscriptions.push( vscode.commands.registerCommand( "coder.navigateToWorkspaceSettings", commands.navigateToWorkspaceSettings.bind(commands), ), - ); - ctx.subscriptions.push( vscode.commands.registerCommand("coder.refreshWorkspaces", () => { myWorkspacesProvider.fetchAndRefresh(); allWorkspacesProvider.fetchAndRefresh(); }), - ); - ctx.subscriptions.push( vscode.commands.registerCommand( "coder.viewLogs", commands.viewLogs.bind(commands), ), - ); - ctx.subscriptions.push( vscode.commands.registerCommand("coder.searchMyWorkspaces", async () => showTreeViewSearch(MY_WORKSPACES_TREE_ID), ), - ); - ctx.subscriptions.push( vscode.commands.registerCommand("coder.searchAllWorkspaces", async () => showTreeViewSearch(ALL_WORKSPACES_TREE_ID), ), diff --git a/src/remote/remote.ts b/src/remote/remote.ts index e6ebb8a0..2a286ab4 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -400,7 +400,9 @@ export class Remote { workspace.owner_name, workspace.name, ); - disposables.push(labelFormatterDisposable); + disposables.push({ + dispose: () => labelFormatterDisposable.dispose(), + }); // If the workspace is not in a running state, try to get it running. if (workspace.latest_build.status !== "running") { @@ -637,11 +639,7 @@ export class Remote { workspace.name, agent.name, ); - disposables.push(labelFormatterDisposable); }), - ); - - disposables.push( ...this.createAgentMetadataStatusBar(agent, workspaceClient), ); } catch (ex) {