Mostly restore command-line parity

Restore most everything and remove the added server arguments.  This
will let us add and remove options after later so we can contain the
number of breaking changes.

To accomplish this a hard separation is added between the CLI arguments
and the server arguments.

The separation between user-provided arguments and arguments with
defaults is also made more clear.

The extra directory flags have been left out as they were buggy and
should be implemented upstream although I think there are better
solutions anyway.  locale and install-source are unsupported with the
web remote and are left removed.  It is unclear whether they were used
before anyway.

Some restored flags still need to have their behavior re-implemented.

Author: code-asher

src/node/app.ts

@@ -44,7 +44,7 @@ const listen = (server: http.Server, { host, port, socket }: ListenOptions) => {
       server.listen(socket, onListen)
     } else {
       // [] is the correct format when using :: but Node errors with them.
-      server.listen(parseInt(port, 10), host.replace(/^\[|\]$/g, ""), onListen)
+      server.listen(port, host.replace(/^\[|\]$/g, ""), onListen)
     }
   })
 }

src/node/cli.ts

@@ -41,7 +41,7 @@ async function entry(): Promise<void> {
     return
   }
 
-  const cliArgs = await parse(process.argv.slice(2))
+  const cliArgs = parse(process.argv.slice(2))
   const configArgs = await readConfigFile(cliArgs.config)
   const args = await setDefaults(cliArgs, configArgs)
 
@@ -74,12 +74,14 @@ async function entry(): Promise<void> {
     return
   }
 
-  if (await shouldSpawnCliProcess(args)) {
+  if (shouldSpawnCliProcess(args)) {
+    logger.debug("Found VS Code arguments; spawning VS Code CLI")
     return runVsCodeCli(args)
   }
 
   const socketPath = await shouldOpenInExistingInstance(cliArgs)
   if (socketPath) {
+    logger.debug("Trying to open in existing instance")
     return openInExistingInstance(args, socketPath)
   }
 

src/node/entry.ts

@@ -4,20 +4,21 @@ import path from "path"
 import { Disposable } from "../common/emitter"
 import { plural } from "../common/util"
 import { createApp, ensureAddress } from "./app"
-import { AuthType, DefaultedArgs, Feature } from "./cli"
+import { AuthType, DefaultedArgs, Feature, UserProvidedArgs } from "./cli"
 import { coderCloudBind } from "./coder_cloud"
 import { commit, version } from "./constants"
 import { register } from "./routes"
 import { humanPath, isFile, loadAMDModule, open } from "./util"
 
-export const shouldSpawnCliProcess = async (args: CodeServerLib.ServerParsedArgs): Promise<boolean> => {
+/**
+ * Return true if the user passed an extension-related VS Code flag.
+ */
+export const shouldSpawnCliProcess = (args: UserProvidedArgs): boolean => {
   return (
-    !args["start-server"] &&
-    (!!args["list-extensions"] ||
-      !!args["install-extension"] ||
-      !!args["install-builtin-extension"] ||
-      !!args["uninstall-extension"] ||
-      !!args["locate-extension"])
+    !!args["list-extensions"] ||
+    !!args["install-extension"] ||
+    !!args["uninstall-extension"] ||
+    !!args["locate-extension"]
   )
 }
 
@@ -33,7 +34,11 @@ export const runVsCodeCli = async (args: DefaultedArgs): Promise<void> => {
   const spawnCli = await loadAMDModule<CodeServerLib.SpawnCli>("vs/server/remoteExtensionHostAgent", "spawnCli")
 
   try {
-    await spawnCli(args)
+    await spawnCli({
+      ...args,
+      // For some reason VS Code takes the port as a string.
+      port: typeof args.port !== "undefined" ? args.port.toString() : undefined,
+    })
   } catch (error: any) {
     logger.error("Got error from VS Code", error)
   }
@@ -49,8 +54,9 @@ export const openInExistingInstance = async (args: DefaultedArgs, socketPath: st
     forceReuseWindow: args["reuse-window"],
     forceNewWindow: args["new-window"],
   }
-  for (let i = 0; i < args._.length; i++) {
-    const fp = path.resolve(args._[i])
+  const paths = args._ || []
+  for (let i = 0; i < paths.length; i++) {
+    const fp = path.resolve(paths[i])
     if (await isFile(fp)) {
       pipeArgs.fileURIs.push(fp)
     } else {

src/node/main.ts

@@ -19,9 +19,10 @@ export const createVSServerRouter = async (args: DefaultedArgs): Promise<VSServe
   )
 
   const codeServerMain = await createVSServer(null, {
-    ...args,
-    protocol: args["cert"] || args.link ? "https:" : "http:",
     connectionToken: "0000",
+    ...args,
+    // For some reason VS Code takes the port as a string.
+    port: typeof args.port !== "undefined" ? args.port.toString() : undefined,
   })
 
   const router = express.Router()

src/node/routes/vscode.ts

@@ -4,14 +4,14 @@ import * as http from "http"
 import * as https from "https"
 import * as path from "path"
 import { createApp, ensureAddress, handleArgsSocketCatchError, handleServerError } from "../../../src/node/app"
-import { createDefaultArgs, OptionalString, setDefaults } from "../../../src/node/cli"
+import { OptionalString, setDefaults } from "../../../src/node/cli"
 import { generateCertificate } from "../../../src/node/util"
 import { getAvailablePort, tmpdir } from "../../utils/helpers"
 
 describe("createApp", () => {
   let spy: jest.SpyInstance
   let unlinkSpy: jest.SpyInstance
-  let port: string
+  let port: number
   let tmpDirPath: string
   let tmpFilePath: string
 
@@ -29,7 +29,7 @@ describe("createApp", () => {
     // then you can spy on those modules methods, like unlink.
     // See: https://github.com/aelbore/esbuild-jest/issues/26#issuecomment-893763840
     unlinkSpy = jest.spyOn(promises, "unlink")
-    port = (await getAvailablePort()).toString()
+    port = await getAvailablePort()
   })
 
   afterEach(() => {
@@ -44,7 +44,6 @@ describe("createApp", () => {
 
   it("should return an Express app, a WebSockets Express app and an http server", async () => {
     const defaultArgs = await setDefaults({
-      ...createDefaultArgs(),
       port,
     })
     const app = await createApp(defaultArgs)
@@ -61,7 +60,6 @@ describe("createApp", () => {
 
   it("should handle error events on the server", async () => {
     const defaultArgs = await setDefaults({
-      ...createDefaultArgs(),
       port,
     })
 
@@ -82,9 +80,8 @@ describe("createApp", () => {
   it("should reject errors that happen before the server can listen", async () => {
     // We listen on an invalid port
     // causing the app to reject the Promise called at startup
-    const port = "2"
+    const port = 2
     const defaultArgs = await setDefaults({
-      ...createDefaultArgs(),
       port,
     })
 
@@ -105,7 +102,6 @@ describe("createApp", () => {
   it("should unlink a socket before listening on the socket", async () => {
     await promises.writeFile(tmpFilePath, "")
     const defaultArgs = await setDefaults({
-      ...createDefaultArgs(),
       socket: tmpFilePath,
     })
 
@@ -119,7 +115,6 @@ describe("createApp", () => {
     const testCertificate = await generateCertificate("localhost")
     const cert = new OptionalString(testCertificate.cert)
     const defaultArgs = await setDefaults({
-      ...createDefaultArgs(),
       port,
       cert,
       ["cert-key"]: testCertificate.certKey,

test/unit/node/app.test.ts

@@ -34,7 +34,7 @@ describe("plugin", () => {
         _: [],
         auth: AuthType.None,
         host: "localhost",
-        port: "8080",
+        port: 8080,
         "proxy-domain": [],
         config: "~/.config/code-server/config.yaml",
         verbose: false,

test/unit/node/cli.test.ts

@@ -5,8 +5,8 @@ import * as httpserver from "./httpserver"
 export async function setup(argv: string[], configFile?: string): Promise<httpserver.HttpServer> {
   argv = ["--bind-addr=localhost:0", "--log=warn", ...argv]
 
-  const cliArgs = await parse(argv)
-  const configArgs = await parseConfigFile(configFile || "", "test/integration.ts")
+  const cliArgs = parse(argv)
+  const configArgs = parseConfigFile(configFile || "", "test/integration.ts")
   const args = await setDefaults(cliArgs, configArgs)
 
   const server = await runCodeServer(args)