[b84330808]

Fixes #8089

As suggested in the issue, this adds --wait to code-server's own script, so code-server --wait --reuse-window works as GIT_EDITOR even when that script is the one on PATH instead of remote-cli. It also works from a shell outside the integrated terminal, through the session socket.

It does what VS Code's remote-cli does: create a marker file, send it as waitMarkerFilePath with the open request, and return once Code deletes it after the files are closed. Like --reuse-window, --wait only opens in an existing instance and errors if there is none. -w is already --welcome-text, so there's no short flag.

Added unit tests for the wait loop (with a fake socket standing in for Code's) and for --wait not falling back to starting a server. I haven't run it against a full build.

[code-asher]

Thank you for the PR!

[code-asher]

I think in the error case we should actually throw the error instead of logging it, so the wait command does not exit with 0 despite the request having failed.

With other commands it would not matter as much if we exit with 0 despite failing to make the request, but I think it will be more important with --wait otherwise git might think the edit was done even though it failed.

I think it could look something like this (untested):

try {
  await new Promise((resolve, reject) => {
    const vscode = http.request(
      {
        path: "/",
        method: "POST",
        socketPath,
      },
      (response) => {
        response.on("data", (message) => {
          logger.debug("got message from Code", field("message", message.toString()))
        })
        response.on("end", resolve)
      },
    )
    vscode.on("error", reject)
    vscode.write(JSON.stringify(pipeArgs))
    vscode.end()
  })
} catch (error) {
  // Try to clean up the marker, but do not let removal errors override the original error.
  if (marker) {
    try {
      await fs.rm(marker)
    } catch (error) {
      if (!isNodeJSErrnoException(error) || error.code !== "ENOENT") {
        logger.error("failed to clean up wait marker", error)
      }
    }
  }
  throw error
}

// Code deletes the marker file once the opened files are closed.
while (marker && await isFile(marker)) {
  await newPromise((resolve) => setTimeout(resolve, 1000))
}

[b84330808]

Done, it now rejects on request errors and rethrows after trying to remove the marker, so --wait exits non-zero. I followed your sketch, so the other commands now wait for the response and fail on errors too.

[code-asher]

We should use tmpdir from constants instead of os.tmpdir() (it is basically the same but uses a code-server sub-directory).

Also maybe we could use generateUuid() from common/util.

So something like marker = path.join(tmpdir, generateUuid(16))

[b84330808]

Done: path.join(tmpdir, generateUuid(16)). I also mkdir tmpdir first, since nothing else creates it in the CLI process before this runs.

[code-asher]

This would add an extra constant 1.5 seconds to the test run time, instead could we perhaps use Jest's timer mocks? jest.useFakeTimers() and jest.runAllTimers() should do the trick I think.

[b84330808]

Switched to fake timers. The test runs the poll timer as soon as it's scheduled, so it takes about 25 ms now.

[code-asher]

Similarly here, after removing we would do jest.runAllTimers() again to make it resolve instantly.

[b84330808]

Done, the removal is followed by running the pending timer.

[code-asher]

There is no need to clean up the file because everything in the tmpdir gets deleted as part of the test cleanup afterward.

[b84330808]

Removed the cleanup.