Prefer matching editor sessions when opening files. (coder#6191)

Signed-off-by: Sean Lee <[email protected]>
typoniels · Jun 14, 2023 · fb73742 · fb73742
1 parent ccb0d3a
commit fb73742
Show file tree

Hide file tree

Showing 9 changed files with 785 additions and 130 deletions.
diff --git a/patches/store-socket.diff b/patches/store-socket.diff
@@ -1,4 +1,4 @@
-Store a static reference to the IPC socket
+Store the IPC socket with workspace metadata.
 
 This lets us use it to open files inside code-server from outside of
 code-server.
@@ -9,6 +9,8 @@ To test this:
 
 It should open in your existing code-server instance.
 
+When the extension host is terminated, the socket is unregistered.
+
 Index: code-server/lib/vscode/src/vs/workbench/api/node/extHostExtensionService.ts
 ===================================================================
 --- code-server.orig/lib/vscode/src/vs/workbench/api/node/extHostExtensionService.ts
@@ -18,20 +20,114 @@ Index: code-server/lib/vscode/src/vs/workbench/api/node/extHostExtensionService.
   *  Licensed under the MIT License. See License.txt in the project root for license information.
   *--------------------------------------------------------------------------------------------*/
 -
-+import { promises as fs } from 'fs';
-+import * as os from 'os'
++import * as os from 'os';
++import * as _http from 'http';
 +import * as path from 'vs/base/common/path';
  import * as performance from 'vs/base/common/performance';
  import { createApiFactoryAndRegisterActors } from 'vs/workbench/api/common/extHost.api.impl';
  import { RequireInterceptor } from 'vs/workbench/api/common/extHostRequireInterceptor';
-@@ -72,6 +74,10 @@ export class ExtHostExtensionService ext
- 		if (this._initData.remote.isRemote && this._initData.remote.authority) {
- 			const cliServer = this._instaService.createInstance(CLIServer);
- 			process.env['VSCODE_IPC_HOOK_CLI'] = cliServer.ipcHandlePath;
-+
-+			fs.writeFile(path.join(os.tmpdir(), 'vscode-ipc'), cliServer.ipcHandlePath).catch((error) => {
-+				this._logService.error(error);
+@@ -17,6 +19,7 @@ import { ExtensionRuntime } from 'vs/wor
+ import { CLIServer } from 'vs/workbench/api/node/extHostCLIServer';
+ import { realpathSync } from 'vs/base/node/extpath';
+ import { ExtHostConsoleForwarder } from 'vs/workbench/api/node/extHostConsoleForwarder';
++import { IExtHostWorkspace } from '../common/extHostWorkspace';
+
+ class NodeModuleRequireInterceptor extends RequireInterceptor {
+
+@@ -79,6 +82,52 @@ export class ExtHostExtensionService ext
+ 		await interceptor.install();
+ 		performance.mark('code/extHost/didInitAPI');
+
++		(async () => {
++			const socketPath = process.env['VSCODE_IPC_HOOK_CLI'];
++			if (!socketPath) {
++				return;
++			}
++			const workspace = this._instaService.invokeFunction((accessor) => {
++				const workspaceService = accessor.get(IExtHostWorkspace);
++				return workspaceService.workspace;
++			});
++			const entry = {
++				workspace,
++				socketPath
++			};
++			const message = JSON.stringify({entry});
++			const codeServerSocketPath = path.join(os.tmpdir(), 'code-server-ipc.sock');
++			await new Promise<void>((resolve, reject) => {
++				const opts: _http.RequestOptions = {
++					path: '/add-session',
++					socketPath: codeServerSocketPath,
++					method: 'POST',
++					headers: {
++						'content-type': 'application/json',
++					}
++				};
++				const req = _http.request(opts, (res) => {
++					res.on('error', reject);
++					res.on('end', () => {
++						try {
++							if (res.statusCode === 200) {
++								resolve();
++							} else {
++								reject(new Error('Unexpected status code: ' + res.statusCode));
++							}
++						} catch (e: unknown) {
++							reject(e);
++						}
++					});
++				});
++				req.on('error', reject);
++				req.write(message);
++				req.end();
 +			});
- 		}
++		})().catch(error => {
++			this._logService.error(error);
++		});
++
+ 		// Do this when extension service exists, but extensions are not being activated yet.
+ 		const configProvider = await this._extHostConfiguration.getConfigProvider();
+ 		await connectProxyResolver(this._extHostWorkspace, configProvider, this, this._logService, this._mainThreadTelemetryProxy, this._initData);
+Index: code-server/lib/vscode/src/vs/workbench/api/node/extensionHostProcess.ts
+===================================================================
+--- code-server.orig/lib/vscode/src/vs/workbench/api/node/extensionHostProcess.ts
++++ code-server/lib/vscode/src/vs/workbench/api/node/extensionHostProcess.ts
+@@ -3,6 +3,9 @@
+  *  Licensed under the MIT License. See License.txt in the project root for license information.
+  *--------------------------------------------------------------------------------------------*/
+
++import * as os from 'os';
++import * as _http from 'http';
++import * as path from 'vs/base/common/path';
+ import * as nativeWatchdog from 'native-watchdog';
+ import * as net from 'net';
+ import * as minimist from 'minimist';
+@@ -400,7 +403,28 @@ async function startExtensionHostProcess
+ 	);
+
+ 	// rewrite onTerminate-function to be a proper shutdown
+-	onTerminate = (reason: string) => extensionHostMain.terminate(reason);
++	onTerminate = (reason: string) => {
++		extensionHostMain.terminate(reason);
++
++		const socketPath = process.env['VSCODE_IPC_HOOK_CLI'];
++		if (!socketPath) {
++			return;
++		}
++		const message = JSON.stringify({socketPath});
++		const codeServerSocketPath = path.join(os.tmpdir(), 'code-server-ipc.sock');
++		const opts: _http.RequestOptions = {
++			path: '/delete-session',
++			socketPath: codeServerSocketPath,
++			method: 'POST',
++			headers: {
++				'content-type': 'application/json',
++				'accept': 'application/json'
++			}
++		};
++		const req = _http.request(opts);
++		req.write(message);
++		req.end();
++	};
+ }
 
- 		// Module loading tricks
+ startExtensionHostProcess().catch((err) => console.log(err));
diff --git a/src/node/app.ts b/src/node/app.ts
@@ -9,9 +9,11 @@ import * as util from "../common/util"
 import { DefaultedArgs } from "./cli"
 import { disposer } from "./http"
 import { isNodeJSErrnoException } from "./util"
+import { DEFAULT_SOCKET_PATH, EditorSessionManager, makeEditorSessionManagerServer } from "./vscodeSocket"
 import { handleUpgrade } from "./wsRouter"
 
-type ListenOptions = Pick<DefaultedArgs, "socket-mode" | "socket" | "port" | "host">
+type SocketOptions = { socket: string; "socket-mode"?: string }
+type ListenOptions = DefaultedArgs | SocketOptions
 
 export interface App extends Disposable {
   /** Handles regular HTTP requests. */
@@ -20,12 +22,18 @@ export interface App extends Disposable {
   wsRouter: Express
   /** The underlying HTTP server. */
   server: http.Server
+  /** Handles requests to the editor session management API. */
+  editorSessionManagerServer: http.Server
 }
 
-export const listen = async (server: http.Server, { host, port, socket, "socket-mode": mode }: ListenOptions) => {
-  if (socket) {
+const isSocketOpts = (opts: ListenOptions): opts is SocketOptions => {
+  return !!(opts as SocketOptions).socket || !(opts as DefaultedArgs).host
+}
+
+export const listen = async (server: http.Server, opts: ListenOptions) => {
+  if (isSocketOpts(opts)) {
     try {
-      await fs.unlink(socket)
+      await fs.unlink(opts.socket)
     } catch (error: any) {
       handleArgsSocketCatchError(error)
     }
@@ -38,18 +46,20 @@ export const listen = async (server: http.Server, { host, port, socket, "socket-
       server.on("error", (err) => util.logError(logger, "http server error", err))
       resolve()
     }
-    if (socket) {
-      server.listen(socket, onListen)
+    if (isSocketOpts(opts)) {
+      server.listen(opts.socket, onListen)
     } else {
       // [] is the correct format when using :: but Node errors with them.
-      server.listen(port, host.replace(/^\[|\]$/g, ""), onListen)
+      server.listen(opts.port, opts.host.replace(/^\[|\]$/g, ""), onListen)
     }
   })
 
   // NOTE@jsjoeio: we need to chmod after the server is finished
   // listening. Otherwise, the socket may not have been created yet.
-  if (socket && mode) {
-    await fs.chmod(socket, mode)
+  if (isSocketOpts(opts)) {
+    if (opts["socket-mode"]) {
+      await fs.chmod(opts.socket, opts["socket-mode"])
+    }
   }
 }
 
@@ -70,14 +80,22 @@ export const createApp = async (args: DefaultedArgs): Promise<App> => {
       )
     : http.createServer(router)
 
-  const dispose = disposer(server)
+  const disposeServer = disposer(server)
 
   await listen(server, args)
 
   const wsRouter = express()
   handleUpgrade(wsRouter, server)
 
-  return { router, wsRouter, server, dispose }
+  const editorSessionManager = new EditorSessionManager()
+  const editorSessionManagerServer = await makeEditorSessionManagerServer(DEFAULT_SOCKET_PATH, editorSessionManager)
+  const disposeEditorSessionManagerServer = disposer(editorSessionManagerServer)
+
+  const dispose = async () => {
+    await Promise.all([disposeServer(), disposeEditorSessionManagerServer()])
+  }
+
+  return { router, wsRouter, server, dispose, editorSessionManagerServer }
 }
 
 /**

diff --git a/src/node/cli.ts b/src/node/cli.ts
@@ -3,17 +3,8 @@ import { promises as fs } from "fs"
 import { load } from "js-yaml"
 import * as os from "os"
 import * as path from "path"
-import {
-  canConnect,
-  generateCertificate,
-  generatePassword,
-  humanPath,
-  paths,
-  isNodeJSErrnoException,
-  splitOnFirstEquals,
-} from "./util"
-
-const DEFAULT_SOCKET_PATH = path.join(os.tmpdir(), "vscode-ipc")
+import { generateCertificate, generatePassword, humanPath, paths, splitOnFirstEquals } from "./util"
+import { DEFAULT_SOCKET_PATH, EditorSessionManagerClient } from "./vscodeSocket"
 
 export enum Feature {
   // No current experimental features!
@@ -591,9 +582,7 @@ export async function setDefaults(cliArgs: UserProvidedArgs, configArgs?: Config
   }
   args["proxy-domain"] = finalProxies
 
-  if (typeof args._ === "undefined") {
-    args._ = []
-  }
+  args._ = getResolvedPathsFromArgs(args)
 
   return {
     ...args,
@@ -602,6 +591,10 @@ export async function setDefaults(cliArgs: UserProvidedArgs, configArgs?: Config
   } as DefaultedArgs // TODO: Technically no guarantee this is fulfilled.
 }
 
+export function getResolvedPathsFromArgs(args: UserProvidedArgs): string[] {
+  return (args._ ?? []).map((p) => path.resolve(p))
+}
+
 /**
  * Helper function to return the default config file.
  *
@@ -741,27 +734,6 @@ function bindAddrFromAllSources(...argsConfig: UserProvidedArgs[]): Addr {
   return addr
 }
 
-/**
- * Reads the socketPath based on path passed in.
- *
- * The one usually passed in is the DEFAULT_SOCKET_PATH.
- *
- * If it can't read the path, it throws an error and returns undefined.
- */
-export async function readSocketPath(path: string): Promise<string | undefined> {
-  try {
-    return await fs.readFile(path, "utf8")
-  } catch (error) {
-    // If it doesn't exist, we don't care.
-    // But if it fails for some reason, we should throw.
-    // We want to surface that to the user.
-    if (!isNodeJSErrnoException(error) || error.code !== "ENOENT") {
-      throw error
-    }
-  }
-  return undefined
-}
-
 /**
  * Determine if it looks like the user is trying to open a file or folder in an
  * existing instance. The arguments here should be the arguments the user
@@ -774,14 +746,22 @@ export const shouldOpenInExistingInstance = async (args: UserProvidedArgs): Prom
     return process.env.VSCODE_IPC_HOOK_CLI
   }
 
+  const paths = getResolvedPathsFromArgs(args)
+  const client = new EditorSessionManagerClient(DEFAULT_SOCKET_PATH)
+
+  // If we can't connect to the socket then there's no existing instance.
+  if (!(await client.canConnect())) {
+    return undefined
+  }
+
   // If these flags are set then assume the user is trying to open in an
   // existing instance since these flags have no effect otherwise.
   const openInFlagCount = ["reuse-window", "new-window"].reduce((prev, cur) => {
     return args[cur as keyof UserProvidedArgs] ? prev + 1 : prev
   }, 0)
   if (openInFlagCount > 0) {
     logger.debug("Found --reuse-window or --new-window")
-    return readSocketPath(DEFAULT_SOCKET_PATH)
+    return await client.getConnectedSocketPath(paths[0])
   }
 
   // It's possible the user is trying to spawn another instance of code-server.
@@ -790,8 +770,8 @@ export const shouldOpenInExistingInstance = async (args: UserProvidedArgs): Prom
   // 2. That a file or directory was passed.
   // 3. That the socket is active.
   if (Object.keys(args).length === 1 && typeof args._ !== "undefined" && args._.length > 0) {
-    const socketPath = await readSocketPath(DEFAULT_SOCKET_PATH)
-    if (socketPath && (await canConnect(socketPath))) {
+    const socketPath = await client.getConnectedSocketPath(paths[0])
+    if (socketPath) {
       logger.debug("Found existing code-server socket")
       return socketPath
     }

diff --git a/src/node/main.ts b/src/node/main.ts
@@ -1,7 +1,6 @@
 import { field, logger } from "@coder/logger"
 import http from "http"
 import * as os from "os"
-import path from "path"
 import { Disposable } from "../common/emitter"
 import { plural } from "../common/util"
 import { createApp, ensureAddress } from "./app"
@@ -70,9 +69,8 @@ export const openInExistingInstance = async (args: DefaultedArgs, socketPath: st
     forceNewWindow: args["new-window"],
     gotoLineMode: true,
   }
-  const paths = args._ || []
-  for (let i = 0; i < paths.length; i++) {
-    const fp = path.resolve(paths[i])
+  for (let i = 0; i < args._.length; i++) {
+    const fp = args._[i]
     if (await isDirectory(fp)) {
       pipeArgs.folderURIs.push(fp)
     } else {
@@ -123,10 +121,12 @@ export const runCodeServer = async (
   const app = await createApp(args)
   const protocol = args.cert ? "https" : "http"
   const serverAddress = ensureAddress(app.server, protocol)
+  const sessionServerAddress = app.editorSessionManagerServer.address()
   const disposeRoutes = await register(app, args)
 
   logger.info(`Using config file ${humanPath(os.homedir(), args.config)}`)
   logger.info(`${protocol.toUpperCase()} server listening on ${serverAddress.toString()}`)
+  logger.info(`Session server listening on ${sessionServerAddress?.toString()}`)
 
   if (args.auth === AuthType.Password) {
     logger.info("  - Authentication is enabled")