On Windows, `pty.New()` was creating a `ConPTY` (`PseudoConsole`) even
when no process would be attached. `ConPTY` requires a real process to
function correctly - without one, the pipe handles become invalid
intermittently, causing flaky test failures like `read |0: The handle is
invalid.`
This affected tests using the `ptytest.New()` + `Attach()` pattern for
in-process CLI testing.
The fix splits Windows PTY creation into two paths:
- `newPty()` now returns a simple pipe-based PTY for the `Attach()` use
case
- `newConPty()` creates a real `ConPTY`, called by `Start()` when a
process will be attached
AFAICT this will result in no change in behaviour outside of tests.
Fixescoder/internal#1277
_Disclaimer: investigated and implemented by Claude Opus 4.5, reviewed
by me._
---------
Signed-off-by: Danny Kopping <danny@coder.com>
Fixes all our Go file imports to match the preferred spec that we've _mostly_ been using. For example:
```
import (
"context"
"time"
"github.com/prometheus/client_golang/prometheus"
"golang.org/x/xerrors"
"gopkg.in/natefinch/lumberjack.v2"
"cdr.dev/slog/v3"
"github.com/coder/coder/v2/codersdk/agentsdk"
"github.com/coder/serpent"
)
```
3 groups: standard library, 3rd partly libs, Coder libs.
This PR makes the change across the codebase. The PR in the stack above modifies our formatting to maintain this state of affairs, and is a separate PR so it's possible to review that one in detail.
Resolved an invalid parameter error (-2147024809) during PTY creation on
Windows 11 22H2 (but not only) when connecting via JetBrains Toolbox
which spawns the native SSH client with `-tt` forcing PTY allocation
even though there is no "terminal" on the client side to query its size.
CreatePseudoConsole doesn't accept a 0x0 (zero width and zero height)
console size and unfortunately, there is NO explicit documentation in
the official Microsoft documentation that states the minimum valid
values or explicitly prohibits 0x0.
Looking at real-world implementations in the search results, all
examples use reasonable non-zero values.
I tested this with a local Windows VM registered to dev.coder.com i.e.
externally managed workspace.
Fixes: #20468
* feat: Add workspace agent for SSH
This adds the initial agent that supports TTY
and execution over SSH. It functions across MacOS,
Windows, and Linux.
This does not handle the coderd interaction yet,
but does setup a simple path forward.
* Fix pty tests on Windows
* Fix log race
* Lock around dial error to fix log output
* Fix context return early
* fix: Leaking yamux session after HTTP handler is closed
Closes#317. We depended on the context canceling the yamux connection,
but this isn't a sync operation. Explicitly calling close ensures the
handler waits for yamux to complete before exit.
* Lock around close return
* Force failure with log
* Fix failed handler
* Upgrade dep
* Fix defer inside loops
* Fix context cancel for HTTP requests
* Fix resize
* Initial agent
* fix: Use buffered reader in peer to fix ShortBuffer
This prevents a io.ErrShortBuffer from occurring when the byte
slice being read is smaller than the chunks sent from the opposite
pipe.
This makes sense for unordered connections, where transmission is
not guarunteed, but does not make sense for TCP-like connections.
We use a bufio.Reader when ordered to ensure data isn't lost.
* SSH server works!
* Start Windows support
* Something works
* Refactor pty package to support Windows spawn
* SSH server now works on Windows
* Fix non-Windows
* Fix Linux PTY render
* FIx linux build tests
* Remove agent and wintest
* Add test for Windows resize
* Fix linting errors
* Add Windows environment variables
* Add strings import
* Add comment for attrs
* Add goleak
* Add require import
Danny Kopping