Set session socket into environment variable (#6282)

* Avoid spawning code-server with --reuse-window and --new-window

These flags mean the user explicitly wants to open in an existing
instance so if the socket is down it should error rather than try to
spawn code-server normally.

* Set session socket into environment variable

While I was at it I added a CLI flag to override the default.  I also
swapped the default to --user-data-dir.

The value is set on an environment variable so it can be used by the
extension host similar to VSCODE_IPC_HOOK_CLI.

* Add e2e test for opening files externally

CHANGELOG.md

@@ -24,6 +24,16 @@ Code v99.99.999
 
 Code v1.79.2
 
+### Fixed
+
+- Fix being unable to launch multiple instances of code-server for different
+  users.
+
+### Added
+
+- `--session-socket` CLI flag to configure the location of the session socket.
+  By default it will be placed in `--user-data-dir`.
+
 ## [4.14.0](https://github.com/coder/code-server/releases/tag/v4.14.0) - 2023-06-16
 
 Code v1.79.2

patches/store-socket.diff

@@ -15,32 +15,31 @@ Index: code-server/lib/vscode/src/vs/workbench/api/node/extHostExtensionService.
 ===================================================================
 --- code-server.orig/lib/vscode/src/vs/workbench/api/node/extHostExtensionService.ts
 +++ code-server/lib/vscode/src/vs/workbench/api/node/extHostExtensionService.ts
-@@ -2,7 +2,9 @@
+@@ -2,7 +2,7 @@
   *  Copyright (c) Microsoft Corporation. All rights reserved.
   *  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 performance from 'vs/base/common/performance';
  import { createApiFactoryAndRegisterActors } from 'vs/workbench/api/common/extHost.api.impl';
  import { RequireInterceptor } from 'vs/workbench/api/common/extHostRequireInterceptor';
-@@ -17,6 +19,7 @@ import { ExtensionRuntime } from 'vs/wor
+@@ -17,6 +17,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';
  import { ExtHostDiskFileSystemProvider } from 'vs/workbench/api/node/extHostDiskFileSystemProvider';
 
  class NodeModuleRequireInterceptor extends RequireInterceptor {
-@@ -83,6 +86,52 @@ export class ExtHostExtensionService ext
+@@ -83,6 +84,52 @@ export class ExtHostExtensionService ext
  		await interceptor.install();
  		performance.mark('code/extHost/didInitAPI');
 
 +		(async () => {
 +			const socketPath = process.env['VSCODE_IPC_HOOK_CLI'];
-+			if (!socketPath) {
++			const codeServerSocketPath = process.env['CODE_SERVER_SESSION_SOCKET']
++			if (!socketPath || !codeServerSocketPath) {
 +				return;
 +			}
 +			const workspace = this._instaService.invokeFunction((accessor) => {
@@ -52,7 +51,6 @@ Index: code-server/lib/vscode/src/vs/workbench/api/node/extHostExtensionService.
 +				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',
@@ -91,17 +89,15 @@ 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 @@
+@@ -3,6 +3,7 @@
   *  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
+@@ -400,7 +401,28 @@ async function startExtensionHostProcess
  	);
 
  	// rewrite onTerminate-function to be a proper shutdown
@@ -110,11 +106,11 @@ Index: code-server/lib/vscode/src/vs/workbench/api/node/extensionHostProcess.ts
 +		extensionHostMain.terminate(reason);
 +
 +		const socketPath = process.env['VSCODE_IPC_HOOK_CLI'];
-+		if (!socketPath) {
++		const codeServerSocketPath = process.env['CODE_SERVER_SESSION_SOCKET']
++		if (!socketPath || !codeServerSocketPath) {
 +			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,

src/node/app.ts

@@ -9,7 +9,7 @@ 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 { EditorSessionManager, makeEditorSessionManagerServer } from "./vscodeSocket"
 import { handleUpgrade } from "./wsRouter"
 
 type SocketOptions = { socket: string; "socket-mode"?: string }
@@ -88,7 +88,7 @@ export const createApp = async (args: DefaultedArgs): Promise<App> => {
   handleUpgrade(wsRouter, server)
 
   const editorSessionManager = new EditorSessionManager()
-  const editorSessionManagerServer = await makeEditorSessionManagerServer(DEFAULT_SOCKET_PATH, editorSessionManager)
+  const editorSessionManagerServer = await makeEditorSessionManagerServer(args["session-socket"], editorSessionManager)
   const disposeEditorSessionManagerServer = disposer(editorSessionManagerServer)
 
   const dispose = async () => {

src/node/cli.ts

@@ -4,7 +4,7 @@ import { load } from "js-yaml"
 import * as os from "os"
 import * as path from "path"
 import { generateCertificate, generatePassword, humanPath, paths, splitOnFirstEquals } from "./util"
-import { DEFAULT_SOCKET_PATH, EditorSessionManagerClient } from "./vscodeSocket"
+import { EditorSessionManagerClient } from "./vscodeSocket"
 
 export enum Feature {
   // No current experimental features!
@@ -51,6 +51,7 @@ export interface UserProvidedCodeArgs {
   "disable-file-downloads"?: boolean
   "disable-workspace-trust"?: boolean
   "disable-getting-started-override"?: boolean
+  "session-socket"?: string
 }
 
 /**
@@ -160,6 +161,9 @@ export const options: Options<Required<UserProvidedArgs>> = {
       "Disable update check. Without this flag, code-server checks every 6 hours against the latest github release and \n" +
       "then notifies you once every week that a new release is available.",
   },
+  "session-socket": {
+    type: "string",
+  },
   "disable-file-downloads": {
     type: "boolean",
     description:
@@ -459,6 +463,7 @@ export interface DefaultedArgs extends ConfigArgs {
   usingEnvHashedPassword: boolean
   "extensions-dir": string
   "user-data-dir": string
+  "session-socket": string
   /* Positional arguments. */
   _: string[]
 }
@@ -479,6 +484,11 @@ export async function setDefaults(cliArgs: UserProvidedArgs, configArgs?: Config
     args["extensions-dir"] = path.join(args["user-data-dir"], "extensions")
   }
 
+  if (!args["session-socket"]) {
+    args["session-socket"] = path.join(args["user-data-dir"], "code-server-ipc.sock")
+  }
+  process.env.CODE_SERVER_SESSION_SOCKET = args["session-socket"]
+
   // --verbose takes priority over --log and --log takes priority over the
   // environment variable.
   if (args.verbose) {
@@ -739,37 +749,45 @@ function bindAddrFromAllSources(...argsConfig: UserProvidedArgs[]): Addr {
  * existing instance. The arguments here should be the arguments the user
  * explicitly passed on the command line, *NOT DEFAULTS* or the configuration.
  */
-export const shouldOpenInExistingInstance = async (args: UserProvidedArgs): Promise<string | undefined> => {
+export const shouldOpenInExistingInstance = async (
+  args: UserProvidedArgs,
+  sessionSocket: string,
+): Promise<string | undefined> => {
   // Always use the existing instance if we're running from VS Code's terminal.
   if (process.env.VSCODE_IPC_HOOK_CLI) {
     logger.debug("Found VSCODE_IPC_HOOK_CLI")
     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
-  }
+  const client = new EditorSessionManagerClient(sessionSocket)
 
   // If these flags are set then assume the user is trying to open in an
-  // existing instance since these flags have no effect otherwise.
+  // existing instance since these flags have no effect otherwise.  That means
+  // if there is no existing instance we should error rather than falling back
+  // to spawning code-server normally.
   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 await client.getConnectedSocketPath(paths[0])
+    const socketPath = await client.getConnectedSocketPath(paths[0])
+    if (!socketPath) {
+      throw new Error(`No opened code-server instances found to handle ${paths[0]}`)
+    }
+    return socketPath
   }
 
   // It's possible the user is trying to spawn another instance of code-server.
   // 1. Check if any unrelated flags are set (this should only run when
   //    code-server is invoked exactly like this: `code-server my-file`).
   // 2. That a file or directory was passed.
   // 3. That the socket is active.
+  // 4. That an instance exists to handle the path (implied by #3).
   if (Object.keys(args).length === 1 && typeof args._ !== "undefined" && args._.length > 0) {
+    if (!(await client.canConnect())) {
+      return undefined
+    }
     const socketPath = await client.getConnectedSocketPath(paths[0])
     if (socketPath) {
       logger.debug("Found existing code-server socket")

src/node/entry.ts

@@ -51,7 +51,7 @@ async function entry(): Promise<void> {
     return runCodeCli(args)
   }
 
-  const socketPath = await shouldOpenInExistingInstance(cliArgs)
+  const socketPath = await shouldOpenInExistingInstance(cliArgs, args["session-socket"])
   if (socketPath) {
     logger.debug("Trying to open in existing instance")
     return openInExistingInstance(args, socketPath)

src/node/vscodeSocket.ts

@@ -4,10 +4,7 @@ import * as http from "http"
 import * as path from "path"
 import { HttpCode } from "../common/http"
 import { listen } from "./app"
-import { canConnect, paths } from "./util"
-
-// Socket path of the daemonized code-server instance.
-export const DEFAULT_SOCKET_PATH = path.join(paths.data, `code-server-ipc.sock`)
+import { canConnect } from "./util"
 
 export interface EditorSessionEntry {
   workspace: {

test/e2e/models/CodeServer.ts

@@ -117,40 +117,26 @@ export class CodeServer {
    * directories.
    */
   private async spawn(): Promise<CodeServerProcess> {
-    // This will be used both as the workspace and data directory to ensure
-    // instances don't bleed into each other.
     const dir = await this.createWorkspace()
-
+    const args = await this.argsWithDefaults([
+      "--auth",
+      "none",
+      // The workspace to open.
+      ...(this.args.includes("--ignore-last-opened") ? [] : [dir]),
+      ...this.args,
+      // Using port zero will spawn on a random port.
+      "--bind-addr",
+      "127.0.0.1:0",
+    ])
     return new Promise((resolve, reject) => {
-      const args = [
-        this.entry,
-        "--extensions-dir",
-        path.join(dir, "extensions"),
-        "--auth",
-        "none",
-        // The workspace to open.
-        ...(this.args.includes("--ignore-last-opened") ? [] : [dir]),
-        ...this.args,
-        // Using port zero will spawn on a random port.
-        "--bind-addr",
-        "127.0.0.1:0",
-        // Setting the XDG variables would be easier and more thorough but the
-        // modules we import ignores those variables for non-Linux operating
-        // systems so use these flags instead.
-        "--config",
-        path.join(dir, "config.yaml"),
-        "--user-data-dir",
-        dir,
-      ]
       this.logger.debug("spawning `node " + args.join(" ") + "`")
       const proc = cp.spawn("node", args, {
         cwd: path.join(__dirname, "../../.."),
         env: {
           ...process.env,
           ...this.env,
-          // Set to empty string to prevent code-server from
-          // using the existing instance when running the e2e tests
-          // from an integrated terminal.
+          // Prevent code-server from using the existing instance when running
+          // the e2e tests from an integrated terminal.
           VSCODE_IPC_HOOK_CLI: "",
           PASSWORD,
         },
@@ -173,11 +159,15 @@ export class CodeServer {
         reject(error)
       })
 
+      // Tracks when the HTTP and session servers are ready.
+      let httpAddress: string | undefined
+      let sessionAddress: string | undefined
+
       let resolved = false
       proc.stdout.setEncoding("utf8")
       onLine(proc, (line) => {
         // As long as we are actively getting input reset the timer.  If we stop
-        // getting input and still have not found the address the timer will
+        // getting input and still have not found the addresses the timer will
         // reject.
         timer.reset()
 
@@ -186,20 +176,69 @@ export class CodeServer {
         if (resolved) {
           return
         }
-        const match = line.trim().match(/HTTPS? server listening on (https?:\/\/[.:\d]+)\/?$/)
+
+        let match = line.trim().match(/HTTPS? server listening on (https?:\/\/[.:\d]+)\/?$/)
         if (match) {
-          // Cookies don't seem to work on IP address so swap to localhost.
+          // Cookies don't seem to work on IP addresses so swap to localhost.
           // TODO: Investigate whether this is a bug with code-server.
-          const address = match[1].replace("127.0.0.1", "localhost")
-          this.logger.debug(`spawned on ${address}`)
+          httpAddress = match[1].replace("127.0.0.1", "localhost")
+        }
+
+        match = line.trim().match(/Session server listening on (.+)$/)
+        if (match) {
+          sessionAddress = match[1]
+        }
+
+        if (typeof httpAddress !== "undefined" && typeof sessionAddress !== "undefined") {
           resolved = true
           timer.dispose()
-          resolve({ process: proc, address })
+          this.logger.debug(`code-server is ready: ${httpAddress} ${sessionAddress}`)
+          resolve({ process: proc, address: httpAddress })
         }
       })
     })
   }
 
+  /**
+   * Execute a short-lived command.
+   */
+  async run(args: string[]): Promise<void> {
+    args = await this.argsWithDefaults(args)
+    this.logger.debug("executing `node " + args.join(" ") + "`")
+    await util.promisify(cp.exec)("node " + args.join(" "), {
+      cwd: path.join(__dirname, "../../.."),
+      env: {
+        ...process.env,
+        ...this.env,
+        // Prevent code-server from using the existing instance when running
+        // the e2e tests from an integrated terminal.
+        VSCODE_IPC_HOOK_CLI: "",
+      },
+    })
+  }
+
+  /**
+   * Combine arguments with defaults.
+   */
+  private async argsWithDefaults(args: string[]): Promise<string[]> {
+    // This will be used both as the workspace and data directory to ensure
+    // instances don't bleed into each other.
+    const dir = await this.workspaceDir
+    return [
+      this.entry,
+      "--extensions-dir",
+      path.join(dir, "extensions"),
+      ...args,
+      // Setting the XDG variables would be easier and more thorough but the
+      // modules we import ignores those variables for non-Linux operating
+      // systems so use these flags instead.
+      "--config",
+      path.join(dir, "config.yaml"),
+      "--user-data-dir",
+      dir,
+    ]
+  }
+
   /**
    * Close the code-server process.
    */
@@ -364,6 +403,13 @@ export class CodeServerPage {
     await this.waitForTab(file)
   }
 
+  /**
+   * Open a file through an external command.
+   */
+  async openFileExternally(file: string) {
+    await this.codeServer.run(["--reuse-window", file])
+  }
+
   /**
    * Wait for a tab to open for the specified file.
    */