Merge pull request #2099 from cdr/open-in

Make opening in an existing instance work outside code-server

ci/build/test-standalone-release.sh

@@ -15,8 +15,7 @@ main() {
   ./release-standalone/bin/code-server --extensions-dir "$EXTENSIONS_DIR" --install-extension ms-python.python
   local installed_extensions
   installed_extensions="$(./release-standalone/bin/code-server --extensions-dir "$EXTENSIONS_DIR" --list-extensions 2>&1)"
-  if [[ $installed_extensions != *"info  Using config file ~/.config/code-server/config.yaml
+  if [[ $installed_extensions != "ms-python.python" ]]; then
-ms-python.python" ]]; then
     echo "Unexpected output from listing extensions:"
     echo "$installed_extensions"
     exit 1

ci/dev/vscode.patch

@@ -3035,6 +3035,31 @@ index b3c89e51cfc25a53293a352a2a8ad50d5f26d595..e21abe4e13bc25a5b72f556bbfb61085
  registerSingleton(IExtHostTerminalService, ExtHostTerminalService);
  registerSingleton(IExtHostTunnelService, ExtHostTunnelService);
 +registerSingleton(IExtHostNodeProxy, class extends NotImplementedProxy<IExtHostNodeProxy>(String(IExtHostNodeProxy)) { whenReady = Promise.resolve(); });
+diff --git a/src/vs/workbench/api/node/extHostCLIServer.ts b/src/vs/workbench/api/node/extHostCLIServer.ts
+index 7cae126cc0f804273850933468690e0f9f10a5b8..08c2aa5cdae3f3d06bb08b7055dc7e7def260132 100644
+--- a/src/vs/workbench/api/node/extHostCLIServer.ts
++++ b/src/vs/workbench/api/node/extHostCLIServer.ts
+@@ -11,6 +11,8 @@ import { IWindowOpenable, IOpenWindowOptions } from 'vs/platform/windows/common/
+ import { URI } from 'vs/base/common/uri';
+ import { hasWorkspaceFileExtension } from 'vs/platform/workspaces/common/workspaces';
+ import { ILogService } from 'vs/platform/log/common/log';
++import { join } from 'vs/base/common/path';
++import { tmpdir } from 'os';
+ 
+ export interface OpenCommandPipeArgs {
+ 	type: 'open';
+@@ -54,6 +56,11 @@ export class CLIServer {
+ 	private async setup(): Promise<string> {
+ 		this._ipcHandlePath = generateRandomPipeName();
+ 
++		// NOTE@coder: Write this out so we can get the most recent path.
++		fs.promises.writeFile(join(tmpdir(), "vscode-ipc"), this._ipcHandlePath).catch((error) => {
++			this.logService.error(error);
++		});
++
+ 		try {
+ 			this._server.listen(this.ipcHandlePath);
+ 			this._server.on('error', err => this.logService.error(err));
 diff --git a/src/vs/workbench/api/worker/extHost.worker.services.ts b/src/vs/workbench/api/worker/extHost.worker.services.ts
 index 3843fdec386edc09a1d361b63de892a04e0070ed..8aac4df527857e964798362a69f5591bef07c165 100644
 --- a/src/vs/workbench/api/worker/extHost.worker.services.ts

src/node/cli.ts

@@ -5,7 +5,7 @@ import * as os from "os"
 import * as path from "path"
 import { Args as VsArgs } from "../../lib/vscode/src/vs/server/ipc"
 import { AuthType } from "./http"
-import { generatePassword, humanPath, paths } from "./util"
+import { canConnect, generatePassword, humanPath, paths } from "./util"
 
 export class Optional<T> {
   public constructor(public readonly value?: T) {}
@@ -152,12 +152,12 @@ const options: Options<Required<Args>> = {
   "new-window": {
     type: "boolean",
     short: "n",
-    description: "Force to open a new window. (use with open-in)",
+    description: "Force to open a new window.",
   },
   "reuse-window": {
     type: "boolean",
     short: "r",
-    description: "Force to open a file or folder in an already opened window. (use with open-in)",
+    description: "Force to open a file or folder in an already opened window.",
   },
 
   locale: { type: "string" },
@@ -327,6 +327,21 @@ export const parse = (
 
   logger.debug("parsed command line", field("args", args))
 
+  return args
+}
+
+export async function setDefaults(args: Args): Promise<Args> {
+  args = { ...args }
+
+  if (!args["user-data-dir"]) {
+    await copyOldMacOSDataDir()
+    args["user-data-dir"] = paths.data
+  }
+
+  if (!args["extensions-dir"]) {
+    args["extensions-dir"] = path.join(args["user-data-dir"], "extensions")
+  }
+
   // --verbose takes priority over --log and --log takes priority over the
   // environment variable.
   if (args.verbose) {
@@ -369,21 +384,6 @@ export const parse = (
   return args
 }
 
-export async function setDefaults(args: Args): Promise<Args> {
-  args = { ...args }
-
-  if (!args["user-data-dir"]) {
-    await copyOldMacOSDataDir()
-    args["user-data-dir"] = paths.data
-  }
-
-  if (!args["extensions-dir"]) {
-    args["extensions-dir"] = path.join(args["user-data-dir"], "extensions")
-  }
-
-  return args
-}
-
 async function defaultConfigFile(): Promise<string> {
   return `bind-addr: 127.0.0.1:8080
 auth: password
@@ -410,10 +410,6 @@ export async function readConfigFile(configPath?: string): Promise<Args> {
     logger.info(`Wrote default config file to ${humanPath(configPath)}`)
   }
 
-  if (!process.env.CODE_SERVER_PARENT_PID && !process.env.VSCODE_IPC_HOOK_CLI) {
-    logger.info(`Using config file ${humanPath(configPath)}`)
-  }
-
   const configFile = await fs.readFile(configPath)
   const config = yaml.safeLoad(configFile.toString(), {
     filename: configPath,
@@ -496,3 +492,52 @@ async function copyOldMacOSDataDir(): Promise<void> {
     await fs.copy(oldDataDir, paths.data)
   }
 }
+
+export const shouldRunVsCodeCli = (args: Args): boolean => {
+  return !!args["list-extensions"] || !!args["install-extension"] || !!args["uninstall-extension"]
+}
+
+/**
+ * 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
+ * explicitly passed on the command line, not defaults or the configuration.
+ */
+export const shouldOpenInExistingInstance = async (args: Args): Promise<string | undefined> => {
+  // Always use the existing instance if we're running from VS Code's terminal.
+  if (process.env.VSCODE_IPC_HOOK_CLI) {
+    return process.env.VSCODE_IPC_HOOK_CLI
+  }
+
+  const readSocketPath = async (): Promise<string | undefined> => {
+    try {
+      return await fs.readFile(path.join(os.tmpdir(), "vscode-ipc"), "utf8")
+    } catch (error) {
+      if (error.code !== "ENOENT") {
+        throw error
+      }
+    }
+    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 Args] ? prev + 1 : prev
+  }, 0)
+  if (openInFlagCount > 0) {
+    return readSocketPath()
+  }
+
+  // It's possible the user is trying to spawn another instance of code-server.
+  // Check if any unrelated flags are set (check against one because `_` always
+  // exists), that a file or directory was passed, and that the socket is
+  // active.
+  if (Object.keys(args).length === 1 && args._.length > 0) {
+    const socketPath = await readSocketPath()
+    if (socketPath && (await canConnect(socketPath))) {
+      return socketPath
+    }
+  }
+
+  return undefined
+}

src/node/entry.ts

@@ -11,19 +11,21 @@ import { ProxyHttpProvider } from "./app/proxy"
 import { StaticHttpProvider } from "./app/static"
 import { UpdateHttpProvider } from "./app/update"
 import { VscodeHttpProvider } from "./app/vscode"
-import { Args, bindAddrFromAllSources, optionDescriptions, parse, readConfigFile, setDefaults } from "./cli"
+import {
+  Args,
+  bindAddrFromAllSources,
+  optionDescriptions,
+  parse,
+  readConfigFile,
+  setDefaults,
+  shouldOpenInExistingInstance,
+  shouldRunVsCodeCli,
+} from "./cli"
 import { coderCloudBind } from "./coder-cloud"
 import { AuthType, HttpServer, HttpServerOptions } from "./http"
 import { loadPlugins } from "./plugin"
 import { generateCertificate, hash, humanPath, open } from "./util"
-import { ipcMain, wrap } from "./wrapper"
+import { ipcMain, WrapperProcess } from "./wrapper"
-
-process.on("uncaughtException", (error) => {
-  logger.error(`Uncaught exception: ${error.message}`)
-  if (typeof error.stack !== "undefined") {
-    logger.error(error.stack)
-  }
-})
 
 let pkg: { version?: string; commit?: string } = {}
 try {
@@ -35,6 +37,86 @@ try {
 const version = pkg.version || "development"
 const commit = pkg.commit || "development"
 
+export const runVsCodeCli = (args: Args): void => {
+  logger.debug("forking vs code cli...")
+  const vscode = cp.fork(path.resolve(__dirname, "../../lib/vscode/out/vs/server/fork"), [], {
+    env: {
+      ...process.env,
+      CODE_SERVER_PARENT_PID: process.pid.toString(),
+    },
+  })
+  vscode.once("message", (message: any) => {
+    logger.debug("got message from VS Code", field("message", message))
+    if (message.type !== "ready") {
+      logger.error("Unexpected response waiting for ready response", field("type", message.type))
+      process.exit(1)
+    }
+    const send: CliMessage = { type: "cli", args }
+    vscode.send(send)
+  })
+  vscode.once("error", (error) => {
+    logger.error("Got error from VS Code", field("error", error))
+    process.exit(1)
+  })
+  vscode.on("exit", (code) => process.exit(code || 0))
+}
+
+export const openInExistingInstance = async (args: Args, socketPath: string): Promise<void> => {
+  const pipeArgs: OpenCommandPipeArgs & { fileURIs: string[] } = {
+    type: "open",
+    folderURIs: [],
+    fileURIs: [],
+    forceReuseWindow: args["reuse-window"],
+    forceNewWindow: args["new-window"],
+  }
+
+  const isDir = async (path: string): Promise<boolean> => {
+    try {
+      const st = await fs.stat(path)
+      return st.isDirectory()
+    } catch (error) {
+      return false
+    }
+  }
+
+  for (let i = 0; i < args._.length; i++) {
+    const fp = path.resolve(args._[i])
+    if (await isDir(fp)) {
+      pipeArgs.folderURIs.push(fp)
+    } else {
+      pipeArgs.fileURIs.push(fp)
+    }
+  }
+
+  if (pipeArgs.forceNewWindow && pipeArgs.fileURIs.length > 0) {
+    logger.error("--new-window can only be used with folder paths")
+    process.exit(1)
+  }
+
+  if (pipeArgs.folderURIs.length === 0 && pipeArgs.fileURIs.length === 0) {
+    logger.error("Please specify at least one file or folder")
+    process.exit(1)
+  }
+
+  const vscode = http.request(
+    {
+      path: "/",
+      method: "POST",
+      socketPath,
+    },
+    (response) => {
+      response.on("data", (message) => {
+        logger.debug("got message from VS Code", field("message", message.toString()))
+      })
+    },
+  )
+  vscode.on("error", (error: unknown) => {
+    logger.error("got error from VS Code", field("error", error))
+  })
+  vscode.write(JSON.stringify(pipeArgs))
+  vscode.end()
+}
+
 const main = async (args: Args, configArgs: Args): Promise<void> => {
   if (args.link) {
     // If we're being exposed to the cloud, we listen on a random address and disable auth.
@@ -99,13 +181,15 @@ const main = async (args: Args, configArgs: Args): Promise<void> => {
 
   await loadPlugins(httpServer, args)
 
-  ipcMain().onDispose(() => {
+  ipcMain.onDispose(() => {
     httpServer.dispose().then((errors) => {
       errors.forEach((error) => logger.error(error.message))
     })
   })
 
   logger.info(`code-server ${version} ${commit}`)
+  logger.info(`Using config file ${humanPath(args.config)}`)
+
   const serverAddress = await httpServer.listen()
   logger.info(`HTTP server listening on ${serverAddress}`)
 
@@ -139,7 +223,9 @@ const main = async (args: Args, configArgs: Args): Promise<void> => {
   if (serverAddress && !options.socket && args.open) {
     // The web socket doesn't seem to work if browsing with 0.0.0.0.
     const openAddress = serverAddress.replace(/:\/\/0.0.0.0/, "://localhost")
-    await open(openAddress).catch(console.error)
+    await open(openAddress).catch((error: Error) => {
+      logger.error("Failed to open", field("address", openAddress), field("error", error))
+    })
     logger.info(`Opened ${openAddress}`)
   }
 
@@ -148,27 +234,27 @@ const main = async (args: Args, configArgs: Args): Promise<void> => {
       await coderCloudBind(serverAddress!, args.link.value)
     } catch (err) {
       logger.error(err.message)
-      ipcMain().exit(1)
+      ipcMain.exit(1)
     }
   }
 }
 
 async function entry(): Promise<void> {
-  const tryParse = async (): Promise<[Args, Args]> => {
+  const cliArgs = parse(process.argv.slice(2))
-    try {
+  const configArgs = await readConfigFile(cliArgs.config)
-      const cliArgs = parse(process.argv.slice(2))
+  // This prioritizes the flags set in args over the ones in the config file.
-      const configArgs = await readConfigFile(cliArgs.config)
+  let args = Object.assign(configArgs, cliArgs)
-      // This prioritizes the flags set in args over the ones in the config file.
+  args = await setDefaults(args)
-      let args = Object.assign(configArgs, cliArgs)
+
-      args = await setDefaults(args)
+  // There's no need to check flags like --help or to spawn in an existing
-      return [args, configArgs]
+  // instance for the child process because these would have already happened in
-    } catch (error) {
+  // the parent and the child wouldn't have been spawned.
-      console.error(error.message)
+  if (ipcMain.isChild) {
-      process.exit(1)
+    await ipcMain.handshake()
-    }
+    ipcMain.preventExit()
+    return main(args, configArgs)
   }
 
-  const [args, configArgs] = await tryParse()
   if (args.help) {
     console.log("code-server", version, commit)
     console.log("")
@@ -178,7 +264,10 @@ async function entry(): Promise<void> {
     optionDescriptions().forEach((description) => {
       console.log("", description)
     })
-  } else if (args.version) {
+    return
+  }
+
+  if (args.version) {
     if (args.json) {
       console.log({
         codeServer: version,
@@ -188,83 +277,23 @@ async function entry(): Promise<void> {
     } else {
       console.log(version, commit)
     }
-    process.exit(0)
+    return
-  } else if (args["list-extensions"] || args["install-extension"] || args["uninstall-extension"]) {
-    logger.debug("forking vs code cli...")
-    const vscode = cp.fork(path.resolve(__dirname, "../../lib/vscode/out/vs/server/fork"), [], {
-      env: {
-        ...process.env,
-        CODE_SERVER_PARENT_PID: process.pid.toString(),
-      },
-    })
-    vscode.once("message", (message: any) => {
-      logger.debug("Got message from VS Code", field("message", message))
-      if (message.type !== "ready") {
-        logger.error("Unexpected response waiting for ready response")
-        process.exit(1)
-      }
-      const send: CliMessage = { type: "cli", args }
-      vscode.send(send)
-    })
-    vscode.once("error", (error) => {
-      logger.error(error.message)
-      process.exit(1)
-    })
-    vscode.on("exit", (code) => process.exit(code || 0))
-  } else if (process.env.VSCODE_IPC_HOOK_CLI) {
-    const pipeArgs: OpenCommandPipeArgs = {
-      type: "open",
-      folderURIs: [],
-      forceReuseWindow: args["reuse-window"],
-      forceNewWindow: args["new-window"],
-    }
-    const isDir = async (path: string): Promise<boolean> => {
-      try {
-        const st = await fs.stat(path)
-        return st.isDirectory()
-      } catch (error) {
-        return false
-      }
-    }
-    for (let i = 0; i < args._.length; i++) {
-      const fp = path.resolve(args._[i])
-      if (await isDir(fp)) {
-        pipeArgs.folderURIs.push(fp)
-      } else {
-        if (!pipeArgs.fileURIs) {
-          pipeArgs.fileURIs = []
-        }
-        pipeArgs.fileURIs.push(fp)
-      }
-    }
-    if (pipeArgs.forceNewWindow && pipeArgs.fileURIs && pipeArgs.fileURIs.length > 0) {
-      logger.error("new-window can only be used with folder paths")
-      process.exit(1)
-    }
-    if (pipeArgs.folderURIs.length === 0 && (!pipeArgs.fileURIs || pipeArgs.fileURIs.length === 0)) {
-      logger.error("Please specify at least one file or folder argument")
-      process.exit(1)
-    }
-    const vscode = http.request(
-      {
-        path: "/",
-        method: "POST",
-        socketPath: process.env["VSCODE_IPC_HOOK_CLI"],
-      },
-      (res) => {
-        res.on("data", (message) => {
-          logger.debug("Got message from VS Code", field("message", message.toString()))
-        })
-      },
-    )
-    vscode.on("error", (err) => {
-      logger.debug("Got error from VS Code", field("error", err))
-    })
-    vscode.write(JSON.stringify(pipeArgs))
-    vscode.end()
-  } else {
-    wrap(() => main(args, configArgs))
   }
+
+  if (shouldRunVsCodeCli(args)) {
+    return runVsCodeCli(args)
+  }
+
+  const socketPath = await shouldOpenInExistingInstance(cliArgs)
+  if (socketPath) {
+    return openInExistingInstance(args, socketPath)
+  }
+
+  const wrapper = new WrapperProcess(require("../../package.json").version)
+  return wrapper.start()
 }
 
-entry()
+entry().catch((error) => {
+  logger.error(error.message)
+  ipcMain.exit(error)
+})

src/node/socket.ts

@@ -4,7 +4,7 @@ import * as path from "path"
 import * as tls from "tls"
 import { Emitter } from "../common/emitter"
 import { generateUuid } from "../common/util"
-import { tmpdir } from "./util"
+import { canConnect, tmpdir } from "./util"
 
 /**
  * Provides a way to proxy a TLS socket. Can be used when you need to pass a
@@ -89,17 +89,6 @@ export class SocketProxyProvider {
   }
 
   public async findFreeSocketPath(basePath: string, maxTries = 100): Promise<string> {
-    const canConnect = (path: string): Promise<boolean> => {
-      return new Promise((resolve) => {
-        const socket = net.connect(path)
-        socket.once("error", () => resolve(false))
-        socket.once("connect", () => {
-          socket.destroy()
-          resolve(true)
-        })
-      })
-    }
-
     let i = 0
     let path = basePath
     while ((await canConnect(path)) && i < maxTries) {

src/node/util.ts

@@ -2,6 +2,7 @@ import * as cp from "child_process"
 import * as crypto from "crypto"
 import envPaths from "env-paths"
 import * as fs from "fs-extra"
+import * as net from "net"
 import * as os from "os"
 import * as path from "path"
 import * as util from "util"
@@ -246,3 +247,17 @@ export function pathToFsPath(path: string, keepDriveLetterCasing = false): strin
   }
   return value
 }
+
+/**
+ * Return a promise that resolves with whether the socket path is active.
+ */
+export function canConnect(path: string): Promise<boolean> {
+  return new Promise((resolve) => {
+    const socket = net.connect(path)
+    socket.once("error", () => resolve(false))
+    socket.once("connect", () => {
+      socket.destroy()
+      resolve(true)
+    })
+  })
+}

src/node/wrapper.ts

@@ -32,19 +32,13 @@ export class IpcMain {
   public readonly onMessage = this._onMessage.event
   private readonly _onDispose = new Emitter<NodeJS.Signals | undefined>()
   public readonly onDispose = this._onDispose.event
-  public readonly processExit: (code?: number) => never
+  public readonly processExit: (code?: number) => never = process.exit
 
-  public constructor(public readonly parentPid?: number) {
+  public constructor(private readonly parentPid?: number) {
     process.on("SIGINT", () => this._onDispose.emit("SIGINT"))
     process.on("SIGTERM", () => this._onDispose.emit("SIGTERM"))
     process.on("exit", () => this._onDispose.emit(undefined))
 
-    // Ensure we control when the process exits.
-    this.processExit = process.exit
-    process.exit = function (code?: number) {
-      logger.warn(`process.exit() was prevented: ${code || "unknown code"}.`)
-    } as (code?: number) => never
-
     this.onDispose((signal) => {
       // Remove listeners to avoid possibly triggering disposal again.
       process.removeAllListeners()
@@ -71,6 +65,19 @@ export class IpcMain {
     }
   }
 
+  /**
+   * Ensure we control when the process exits.
+   */
+  public preventExit(): void {
+    process.exit = function (code?: number) {
+      logger.warn(`process.exit() was prevented: ${code || "unknown code"}.`)
+    } as (code?: number) => never
+  }
+
+  public get isChild(): boolean {
+    return typeof this.parentPid !== "undefined"
+  }
+
   public exit(error?: number | ProcessError): never {
     if (error && typeof error !== "number") {
       this.processExit(typeof error.code === "number" ? error.code : 1)
@@ -127,17 +134,12 @@ export class IpcMain {
   }
 }
 
-let _ipcMain: IpcMain
+/**
-export const ipcMain = (): IpcMain => {
+ * Channel for communication between the child and parent processes.
-  if (!_ipcMain) {
+ */
-    _ipcMain = new IpcMain(
+export const ipcMain = new IpcMain(
-      typeof process.env.CODE_SERVER_PARENT_PID !== "undefined"
+  typeof process.env.CODE_SERVER_PARENT_PID !== "undefined" ? parseInt(process.env.CODE_SERVER_PARENT_PID) : undefined,
-        ? parseInt(process.env.CODE_SERVER_PARENT_PID)
+)
-        : undefined,
-    )
-  }
-  return _ipcMain
-}
 
 export interface WrapperOptions {
   maxMemory?: number
@@ -162,14 +164,11 @@ export class WrapperProcess {
     this.logStdoutStream = rfs.createStream(path.join(paths.data, "coder-logs", "code-server-stdout.log"), opts)
     this.logStderrStream = rfs.createStream(path.join(paths.data, "coder-logs", "code-server-stderr.log"), opts)
 
-    ipcMain().onDispose(() => {
+    ipcMain.onDispose(() => {
-      if (this.process) {
+      this.disposeChild()
-        this.process.removeAllListeners()
-        this.process.kill()
-      }
     })
 
-    ipcMain().onMessage((message) => {
+    ipcMain.onMessage((message) => {
       switch (message.type) {
         case "relaunch":
           logger.info(`Relaunching: ${this.currentVersion} -> ${message.version}`)
@@ -181,55 +180,65 @@ export class WrapperProcess {
           break
       }
     })
-
-    process.on("SIGUSR1", async () => {
-      logger.info("Received SIGUSR1; hotswapping")
-      this.relaunch()
-    })
   }
 
-  private async relaunch(): Promise<void> {
+  private disposeChild(): void {
     this.started = undefined
     if (this.process) {
       this.process.removeAllListeners()
       this.process.kill()
     }
+  }
+
+  private async relaunch(): Promise<void> {
+    this.disposeChild()
     try {
       await this.start()
     } catch (error) {
       logger.error(error.message)
-      ipcMain().exit(typeof error.code === "number" ? error.code : 1)
+      ipcMain.exit(typeof error.code === "number" ? error.code : 1)
     }
   }
 
   public start(): Promise<void> {
-    if (!this.started) {
+    // If we have a process then we've already bound this.
-      this.started = this.spawn().then((child) => {
+    if (!this.process) {
-        // Log both to stdout and to the log directory.
+      process.on("SIGUSR1", async () => {
-        if (child.stdout) {
+        logger.info("Received SIGUSR1; hotswapping")
-          child.stdout.pipe(this.logStdoutStream)
+        this.relaunch()
-          child.stdout.pipe(process.stdout)
-        }
-        if (child.stderr) {
-          child.stderr.pipe(this.logStderrStream)
-          child.stderr.pipe(process.stderr)
-        }
-        logger.debug(`spawned inner process ${child.pid}`)
-        ipcMain()
-          .handshake(child)
-          .then(() => {
-            child.once("exit", (code) => {
-              logger.debug(`inner process ${child.pid} exited unexpectedly`)
-              ipcMain().exit(code || 0)
-            })
-          })
-        this.process = child
       })
     }
+    if (!this.started) {
+      this.started = this._start()
+    }
     return this.started
   }
 
-  private async spawn(): Promise<cp.ChildProcess> {
+  private async _start(): Promise<void> {
+    const child = this.spawn()
+    this.process = child
+
+    // Log both to stdout and to the log directory.
+    if (child.stdout) {
+      child.stdout.pipe(this.logStdoutStream)
+      child.stdout.pipe(process.stdout)
+    }
+    if (child.stderr) {
+      child.stderr.pipe(this.logStderrStream)
+      child.stderr.pipe(process.stderr)
+    }
+
+    logger.debug(`spawned inner process ${child.pid}`)
+
+    await ipcMain.handshake(child)
+
+    child.once("exit", (code) => {
+      logger.debug(`inner process ${child.pid} exited unexpectedly`)
+      ipcMain.exit(code || 0)
+    })
+  }
+
+  private spawn(): cp.ChildProcess {
     // Flags to pass along to the Node binary.
     let nodeOptions = `${process.env.NODE_OPTIONS || ""} ${(this.options && this.options.nodeOptions) || ""}`
     if (!/max_old_space_size=(\d+)/g.exec(nodeOptions)) {
@@ -251,23 +260,13 @@ export class WrapperProcess {
 // It's possible that the pipe has closed (for example if you run code-server
 // --version | head -1). Assume that means we're done.
 if (!process.stdout.isTTY) {
-  process.stdout.on("error", () => ipcMain().exit())
+  process.stdout.on("error", () => ipcMain.exit())
 }
 
-export const wrap = (fn: () => Promise<void>): void => {
+// Don't let uncaught exceptions crash the process.
-  if (ipcMain().parentPid) {
+process.on("uncaughtException", (error) => {
-    ipcMain()
+  logger.error(`Uncaught exception: ${error.message}`)
-      .handshake()
+  if (typeof error.stack !== "undefined") {
-      .then(() => fn())
+    logger.error(error.stack)
-      .catch((error: ProcessError): void => {
-        logger.error(error.message)
-        ipcMain().exit(error)
-      })
-  } else {
-    const wrapper = new WrapperProcess(require("../../package.json").version)
-    wrapper.start().catch((error) => {
-      logger.error(error.message)
-      ipcMain().exit(error)
-    })
   }
-}
+})

test/cli.test.ts

@@ -1,20 +1,31 @@
-import { logger, Level } from "@coder/logger"
+import { Level, logger } from "@coder/logger"
 import * as assert from "assert"
+import * as fs from "fs-extra"
+import * as net from "net"
+import * as os from "os"
 import * as path from "path"
-import { parse } from "../src/node/cli"
+import { Args, parse, setDefaults, shouldOpenInExistingInstance } from "../src/node/cli"
+import { paths, tmpdir } from "../src/node/util"
 
-describe("cli", () => {
+type Mutable<T> = {
+  -readonly [P in keyof T]: T[P]
+}
+
+describe("parser", () => {
   beforeEach(() => {
     delete process.env.LOG_LEVEL
   })
 
-  // The parser will always fill these out.
+  // The parser should not set any defaults so the caller can determine what
+  // values the user actually set. These are only set after explicitly calling
+  // `setDefaults`.
   const defaults = {
-    _: [],
+    "extensions-dir": path.join(paths.data, "extensions"),
+    "user-data-dir": paths.data,
   }
 
   it("should set defaults", () => {
-    assert.deepEqual(parse([]), defaults)
+    assert.deepEqual(parse([]), { _: [] })
   })
 
   it("should parse all available options", () => {
@@ -69,7 +80,7 @@ describe("cli", () => {
         help: true,
         host: "0.0.0.0",
         json: true,
-        log: "trace",
+        log: "error",
         open: true,
         port: 8081,
         socket: path.resolve("mumble"),
@@ -83,19 +94,20 @@ describe("cli", () => {
 
   it("should work with short options", () => {
     assert.deepEqual(parse(["-vvv", "-v"]), {
-      ...defaults,
+      _: [],
-      log: "trace",
       verbose: true,
       version: true,
     })
-    assert.equal(process.env.LOG_LEVEL, "trace")
-    assert.equal(logger.level, Level.Trace)
   })
 
-  it("should use log level env var", () => {
+  it("should use log level env var", async () => {
+    const args = parse([])
+    assert.deepEqual(args, { _: [] })
+
     process.env.LOG_LEVEL = "debug"
-    assert.deepEqual(parse([]), {
+    assert.deepEqual(await setDefaults(args), {
       ...defaults,
+      _: [],
       log: "debug",
       verbose: false,
     })
@@ -103,8 +115,9 @@ describe("cli", () => {
     assert.equal(logger.level, Level.Debug)
 
     process.env.LOG_LEVEL = "trace"
-    assert.deepEqual(parse([]), {
+    assert.deepEqual(await setDefaults(args), {
       ...defaults,
+      _: [],
       log: "trace",
       verbose: true,
     })
@@ -113,9 +126,16 @@ describe("cli", () => {
   })
 
   it("should prefer --log to env var and --verbose to --log", async () => {
+    let args = parse(["--log", "info"])
+    assert.deepEqual(args, {
+      _: [],
+      log: "info",
+    })
+
     process.env.LOG_LEVEL = "debug"
-    assert.deepEqual(parse(["--log", "info"]), {
+    assert.deepEqual(await setDefaults(args), {
       ...defaults,
+      _: [],
       log: "info",
       verbose: false,
     })
@@ -123,17 +143,26 @@ describe("cli", () => {
     assert.equal(logger.level, Level.Info)
 
     process.env.LOG_LEVEL = "trace"
-    assert.deepEqual(parse(["--log", "info"]), {
+    assert.deepEqual(await setDefaults(args), {
       ...defaults,
+      _: [],
       log: "info",
       verbose: false,
     })
     assert.equal(process.env.LOG_LEVEL, "info")
     assert.equal(logger.level, Level.Info)
 
+    args = parse(["--log", "info", "--verbose"])
+    assert.deepEqual(args, {
+      _: [],
+      log: "info",
+      verbose: true,
+    })
+
     process.env.LOG_LEVEL = "warn"
-    assert.deepEqual(parse(["--log", "info", "--verbose"]), {
+    assert.deepEqual(await setDefaults(args), {
       ...defaults,
+      _: [],
       log: "trace",
       verbose: true,
     })
@@ -141,9 +170,12 @@ describe("cli", () => {
     assert.equal(logger.level, Level.Trace)
   })
 
-  it("should ignore invalid log level env var", () => {
+  it("should ignore invalid log level env var", async () => {
     process.env.LOG_LEVEL = "bogus"
-    assert.deepEqual(parse([]), defaults)
+    assert.deepEqual(await setDefaults(parse([])), {
+      _: [],
+      ...defaults,
+    })
   })
 
   it("should error if value isn't provided", () => {
@@ -166,7 +198,7 @@ describe("cli", () => {
 
   it("should not error if the value is optional", () => {
     assert.deepEqual(parse(["--cert"]), {
-      ...defaults,
+      _: [],
       cert: {
         value: undefined,
       },
@@ -177,7 +209,7 @@ describe("cli", () => {
     assert.throws(() => parse(["--socket", "--socket-path-value"]), /--socket requires a value/)
     // If you actually had a path like this you would do this instead:
     assert.deepEqual(parse(["--socket", "./--socket-path-value"]), {
-      ...defaults,
+      _: [],
       socket: path.resolve("--socket-path-value"),
     })
     assert.throws(() => parse(["--cert", "--socket-path-value"]), /Unknown option --socket-path-value/)
@@ -185,7 +217,6 @@ describe("cli", () => {
 
   it("should allow positional arguments before options", () => {
     assert.deepEqual(parse(["foo", "test", "--auth", "none"]), {
-      ...defaults,
       _: ["foo", "test"],
       auth: "none",
     })
@@ -193,12 +224,85 @@ describe("cli", () => {
 
   it("should support repeatable flags", () => {
     assert.deepEqual(parse(["--proxy-domain", "*.coder.com"]), {
-      ...defaults,
+      _: [],
       "proxy-domain": ["*.coder.com"],
     })
     assert.deepEqual(parse(["--proxy-domain", "*.coder.com", "--proxy-domain", "test.com"]), {
-      ...defaults,
+      _: [],
       "proxy-domain": ["*.coder.com", "test.com"],
     })
   })
 })
+
+describe("cli", () => {
+  let args: Mutable<Args> = { _: [] }
+  const testDir = path.join(tmpdir, "tests/cli")
+  const vscodeIpcPath = path.join(os.tmpdir(), "vscode-ipc")
+
+  before(async () => {
+    await fs.remove(testDir)
+    await fs.mkdirp(testDir)
+  })
+
+  beforeEach(async () => {
+    delete process.env.VSCODE_IPC_HOOK_CLI
+    args = { _: [] }
+    await fs.remove(vscodeIpcPath)
+  })
+
+  it("should use existing if inside code-server", async () => {
+    process.env.VSCODE_IPC_HOOK_CLI = "test"
+    assert.strictEqual(await shouldOpenInExistingInstance(args), "test")
+
+    args.port = 8081
+    args._.push("./file")
+    assert.strictEqual(await shouldOpenInExistingInstance(args), "test")
+  })
+
+  it("should use existing if --reuse-window is set", async () => {
+    args["reuse-window"] = true
+    assert.strictEqual(await shouldOpenInExistingInstance(args), undefined)
+
+    await fs.writeFile(vscodeIpcPath, "test")
+    assert.strictEqual(await shouldOpenInExistingInstance(args), "test")
+
+    args.port = 8081
+    assert.strictEqual(await shouldOpenInExistingInstance(args), "test")
+  })
+
+  it("should use existing if --new-window is set", async () => {
+    args["new-window"] = true
+    assert.strictEqual(await shouldOpenInExistingInstance(args), undefined)
+
+    await fs.writeFile(vscodeIpcPath, "test")
+    assert.strictEqual(await shouldOpenInExistingInstance(args), "test")
+
+    args.port = 8081
+    assert.strictEqual(await shouldOpenInExistingInstance(args), "test")
+  })
+
+  it("should use existing if no unrelated flags are set, has positional, and socket is active", async () => {
+    assert.strictEqual(await shouldOpenInExistingInstance(args), undefined)
+
+    args._.push("./file")
+    assert.strictEqual(await shouldOpenInExistingInstance(args), undefined)
+
+    const socketPath = path.join(testDir, "socket")
+    await fs.writeFile(vscodeIpcPath, socketPath)
+    assert.strictEqual(await shouldOpenInExistingInstance(args), undefined)
+
+    await new Promise((resolve) => {
+      const server = net.createServer(() => {
+        // Close after getting the first connection.
+        server.close()
+      })
+      server.once("listening", () => resolve(server))
+      server.listen(socketPath)
+    })
+
+    assert.strictEqual(await shouldOpenInExistingInstance(args), socketPath)
+
+    args.port = 8081
+    assert.strictEqual(await shouldOpenInExistingInstance(args), undefined)
+  })
+})