Our agentssh package handles running commands in a pseudoterminal, and we've had several problems with not returning command output.

The implementation prior to this PR seems to behave how we want it to, (at least on Linux), but it

1. uses needlessly complex concurrency and synchronization
2. incorrectly explains why it works
3. doesn't fix the problem on Windows

This leaves us in a precarious situation where if someone tries to fix or enhance this function, they'll misunderstand how and why the current code works.

This PR attempts to address these problems.

This was the subject of several fixes: #6777 #6833 and #6862

The initial bug was that the SSH handler runs io.Copy() in a goroutine to copy the command output to the SSH session, but we didn't wait for this copy to finish before closing down the session, causing a race that sometimes lost output.

However, simply waiting for the io.Copy() to finish would result in a deadlock because of another, much more subtle bug in the way pseudoterminals are handled by the OS.

Linux/macOS

The current code makes an unnecessary call to the low-level syscall dup to duplicate a file descriptor. I'll try to explain what was happening and why it was bad:

On POSIX systems, the way pseudo-TTYs work is that open a pair of files, which I'll refer to as the pty and the tty to match the terminology in our code (Linux itself calls them ptm and pts). The tty acts just like a "real" teletype device for any program interacting with it --- and when we launch commands, we pass the tty as all three of the stdio file descriptors (stdin, stdout, stderr) to the child process.

After forking/exec'ing to start the child process this is what things looked like:

go process
PTY:
  - pty  ------------------------------> pty <--- linux kernel ----> tty <---- child process
  - tty  ------------------------------------------------------------^

Notice that our go process kept the tty file open. What happens is when you read from the pty, and there is no data available to read, it blocks until either

1. there is data to read
2. all file descriptors to the tty are closed

We kept an open file descriptor to the tty so even after the child process exited, we never hit condition 2, until we called Close() on our PTY object, which then closed both pty and tty at the same time.

The current code gets around this by calling dup on the pty file descriptor, so we end up with two references to the PTY.

go process
SSH Handler:
  - stdout ------------------------------+
PTY:                                     V
  - pty  ------------------------------> pty <--- linux kernel ----> tty <---- child process
  - tty  ------------------------------------------------------------^

The explanation given for this is that it allows us to close stdin and stdout separately, but that's a misunderstanding. The child process interacts only with the tty for both stdin and stdout --- duplicating the pty doesn't change this.

However, this does allow us to break the deadlock, because calling Close() on the PTY closes tty, but since we have the duplicated file, we can still read the output as long as the child process is still alive or there is unread buffered data in the kernel. But, this is needlessly wasteful of file descriptors (which are a limited kernel resource), and essentially uses a kernel primitive to accomplish the task of closing tty without closing pty.

The essential features of this PR on Linux/macOS are to

1. Close the tty immediately after creating the child process - this allows us to safely read all the output from the child process without fear of deadlocking.
2. Fix the SSH handler to simply call io.Copy() to copy output, then grab the exit code and return --- no extra file descriptors, wait groups, or synchronization.

Windows

Instead of a single read-write file descriptor, Windows uses a pair of pipes (which are unidirectional). On Windows, though, the pseudoconsole is hosted by a separate process, called the Console Host (conhost.exe), rather than being handled entirely in-kernel like on Linux/macOS. The lifecycle of the pseudoterminal is explicit via a Handle returned on creation. So on Windows, we create 5 handles:

1. Input Read Pipe
2. Input Write Pipe
3. Output Read Pipe
4. Output Write Pipe
5. Console

In the current implementation, we keep all 5 handles open until the PTY is closed, at which point we close all 5. This then causes a race for a reader trying to read all the data before the pipe gets closed.

Docs for PseudoConsoles explain that once you pass the Input Read Pipe and Output Write Pipe to the pseudoconsole host, you should close them.

On Windows we also have to close the Pseudoconsole itself to unblock our output reads, since the Pseudoconsole is a separate process from the child application, and it has an explicit lifetime. So, in this PR, we also add code that closes the Pseudoconsole as soon as the child application exits (for PTYs attached to commands). Crucially, we leave the Output Read Pipe open, so that we can still read any data buffered in the OS.

Other features

• split the PTY interface in 2, since there are still use cases in the code where we create a PTY but don't start a child process. So, we have one interface when we are interacting with a child, and a different interface when we hold both ends of the PTY -- this is done for safety so that you can't attempt to read from the tty after we've closed it.
• add a unit test to check that we clean up orphaned processes when the SSH client disconnects
• add a unit test to check for truncated output from commands started in a PTY