From f5c6d499046595f2580a3ba7bd75d4621da44d06 Mon Sep 17 00:00:00 2001 From: midagedev Date: Wed, 30 Sep 2026 05:14:59 +0900 Subject: [PATCH] =?UTF-8?q?term:=20Windows=20gets=20a=20real=20PTY=20?= =?UTF-8?q?=E2=80=94=20ConPTY=20under=20a=20Job=20Object=20(GDK-891)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit session_windows.go was an honest refusal (GDK-862): Create returned ErrUnsupportedPlatform and the pane said "not on Windows yet" and meant it. It is an implementation now — CreatePseudoConsole plus a child spawned with PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, cgo-free on x/sys/windows, no new module. Spawn is a direct CreateProcess rather than os/exec: attaching a child to a pseudoconsole needs an attribute list on STARTUPINFOEX, and syscall.SysProcAttr has no field that can carry one. wait() is WaitForSingleObject on the raw handle — the same observable answer, rebuilt. The unix half has no owner of the process tree, which is why it signals a process group and then walks the controlling terminal to find the jobs a shell moved out of it. Windows has an owner, so the ladder maps onto the Job Object instead: hangup closes the pseudoconsole (conhost tears the session down), kill terminates the job, and KILL_ON_JOB_CLOSE backs both up when the handle goes. The child starts CREATE_SUSPENDED and joins the job before its first instruction, so "every descendant is in the job" is a theorem rather than a race AssignProcessToJobObject has to win. That also makes sessionMembers a job query instead of the nil it used to return — enumeration becomes a lookup, with no revoked-device or recycled-pid hazard to guard against. Two unix contracts have no Windows twin and say so where they are read. winsize answers from the last size resize accepted, because ConPTY has no GetPseudoConsoleSize — the unix half trusts only the kernel's read-back, and this one cannot. PID is the shell's pid and nothing else: there is no process group to also be. Handles leak by default here, so the file opens with a ledger naming the single closer of each of the seven and the order they go in, and the control-plane calls sit behind a lock that zeroes each field as it closes — a syscall on a recycled handle value is not the benign EBADF a stale fd is on unix. Runtime proof is the Windows CI job; nothing in this file executes on a developer's macOS. The new step runs the four axes that matter — a command's output makes the round trip, a resize reaches the child, kill takes the grandchild with it, and closing twice is safe — and the last of those is the Job Object contract itself. What could be measured here was measured: both GOOS builds, vet, the full native suite, sourcelint, the staticcheck matrix. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 9 + cmd/gadak/doctor.go | 23 + internal/term/doc.go | 11 +- internal/term/manager.go | 5 +- internal/term/members_windows.go | 20 +- internal/term/session.go | 6 +- internal/term/session_windows.go | 635 ++++++++++++++++++++++++-- internal/term/session_windows_test.go | 243 ++++++++++ internal/term/win32diag.go | 30 ++ 9 files changed, 949 insertions(+), 33 deletions(-) create mode 100644 internal/term/session_windows_test.go create mode 100644 internal/term/win32diag.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5aa6da5a6..56d4d0800 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1223,6 +1223,15 @@ jobs: working-directory: desktop run: go test ./... -run 'Protocol' -count=1 + # GDK-891: the ConPTY contract (spawn roundtrip, resize answered by + # the child, Job-Object tree kill, close idempotence) can only + # execute on Windows — dev machines are macOS. Runs from the repo + # root: internal/term belongs to the root module, unlike the desktop + # step above. + - name: Terminal (ConPTY) tests + if: steps.cifilter.outputs.run == 'true' + run: go test ./internal/term/ -count=1 + # makeappx validates the manifest schema; only an install proves the # identity, the full-trust entry point, the protocol declaration and the # app-execution alias are accepted. The Store re-signs with its own certificate, so here diff --git a/cmd/gadak/doctor.go b/cmd/gadak/doctor.go index 9727d7046..ac9040937 100644 --- a/cmd/gadak/doctor.go +++ b/cmd/gadak/doctor.go @@ -30,6 +30,7 @@ import ( "github.com/midagedev/gadak/internal/skillinstall" "github.com/midagedev/gadak/internal/store" syncer "github.com/midagedev/gadak/internal/sync" + "github.com/midagedev/gadak/internal/term" issuetap "github.com/midagedev/issuetap" ) @@ -181,6 +182,13 @@ type doctorReport struct { // opens, on the unaugmented environment), never a defect. HostEnv doctorHostEnv `json:"host_env"` + // TermWin32 is the ConPTY control plane's last failure — which Win32 + // call, which error code (GDK-891) — so "my pane won't open on + // Windows" has a next question to ask without a debugger. A call name + // and a number only, and empty (none recorded) on every healthy + // serve and every non-Windows build. + TermWin32 string `json:"term_win32,omitempty"` + // localDBSchema is local.db's on-disk PRAGMA user_version when the file // exists — the local half of the `schemas` line (GDK-1967), collected // before any open so the refusal paths report it too. Unexported: the @@ -524,6 +532,7 @@ func collectDoctor() doctorReport { rep.BinaryPath, rep.BinarySignature = collectBuildIdentity() // Independent of the mirror — the early returns below must not lose it. rep.HostEnv = collectHostEnv() + rep.TermWin32 = collectTermWin32() // Agent wiring is independent of the mirror, and the mirror branch below // returns early — collect it first so a user with no mirror still gets the @@ -1323,6 +1332,7 @@ func formatDoctorText(r doctorReport) string { line("go_version", r.GoVersion) line("os", r.OS+"/"+r.Arch) line("hostenv", formatDoctorHostEnv(r.HostEnv)) + line("term-conpty", r.TermWin32) line("profile", r.Profile) line("workspace_kind", r.WorkspaceKind) line("origin_type", r.Workspace.OriginType) @@ -1673,6 +1683,19 @@ func collectHostEnv() doctorHostEnv { } } +// collectTermWin32 is the terminal subsystem's last control-plane failure +// as one paste-safe token pair: the Win32 call's name and its error code +// (GDK-891) — the same counts-and-short-tokens rule the hostenv line +// keeps. Non-Windows builds and healthy serves report none recorded; the +// recorder is ConPTY code. +func collectTermWin32() string { + call, code, ok := term.LastWin32Failure() + if !ok { + return "none recorded" + } + return fmt.Sprintf("%s win32 %d", call, code) +} + // countPathAdded measures what the augmentation contributed: PATH entries // present after it that base did not have. Counts only — the entries // themselves stay out of this paste-safe document. diff --git a/internal/term/doc.go b/internal/term/doc.go index 87465b8b1..a7d455ceb 100644 --- a/internal/term/doc.go +++ b/internal/term/doc.go @@ -70,8 +70,15 @@ // .CloseByToken is how `gadak pairing revoke` reaches a live shell — // see internal/server's watchdog for who calls it. // -// - Windows returns ErrUnsupportedPlatform from Create, naming GDK-861 -// (the ConPTY shape). An honest stub beats a silent one. +// - Windows runs on ConPTY (GDK-891): CreatePseudoConsole plus a child +// spawned with PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, inside a Job +// Object. The unix ladder maps onto the job rather than onto signals — +// hangup closes the pseudoconsole, kill terminates the job, and the +// job owns every descendant, so the tree needs no controlling-terminal +// walk. ErrUnsupportedPlatform survives as the contract for a platform +// that has no PTY at all; no supported GOOS answers with it today. +// Runtime proof is the Windows CI job — the shape cannot be exercised +// on a developer's macOS. // // Snapshot() is the debug surface: per-session id, pid, pids (every // process on the session's controlling terminal), size, attachment diff --git a/internal/term/manager.go b/internal/term/manager.go index 134dc61f0..80483c052 100644 --- a/internal/term/manager.go +++ b/internal/term/manager.go @@ -204,7 +204,10 @@ type Info struct { // (GDK-1192): "matched/N" (read-back agreed on attempt N), // "unverified:" (read-back failed and the set was trusted), // "exhausted" (bounded attempts, error returned), "set-error:", - // or "unsupported" on Windows. Empty before the first resize. Pure + // or, on Windows, "set" — ConPTY has no GetPseudoConsoleSize to read + // the size back with, so there is no read-back branch to name and the + // word says only that ResizePseudoConsole returned (GDK-891). + // Empty before the first resize. Pure // diagnostic — it changes no behaviour, it names it. LastResizeExit string `json:"last_resize_exit,omitempty"` } diff --git a/internal/term/members_windows.go b/internal/term/members_windows.go index 5f5c67169..72044a9ab 100644 --- a/internal/term/members_windows.go +++ b/internal/term/members_windows.go @@ -2,4 +2,22 @@ package term -func sessionMembers(int) []int { return nil } +// sessionMembers answers membersOf's "every process on this session's +// terminal" (the idle verdict and the roster's PIDs column) with a query +// to the session's Job Object — see jobPids. +// +// The unix halves walk the controlling-terminal device because nothing +// there owns the tree; the walk is once-and-remembered, with start-time +// stamps against pid recycling, because the terminal is revoked under it. +// Here the job owns the tree, so the member list is a query instead of a +// walk: no revocation race to survive, no stamps to check. A pid the +// registry does not know (never started, killed, or closed) answers nil, +// which the callers read as "nothing running" — the same fail direction +// as the unix walk coming back empty. +func sessionMembers(shellPID int) []int { + p := lookupProc(shellPID) + if p == nil { + return nil + } + return p.jobPids() +} diff --git a/internal/term/session.go b/internal/term/session.go index 2f7fbcc65..36190c0b0 100644 --- a/internal/term/session.go +++ b/internal/term/session.go @@ -160,8 +160,10 @@ type Session struct { // ID is the session id a socket URL carries. func (s *Session) ID() string { return s.id } -// PID is the shell's process id — also its process-group id, because the -// child is started with Setsid. +// PID is the shell's process id. On unix it is also its process-group id, +// because the child is started with Setsid; Windows has no process group +// at all — there the tree's owner is the session's Job Object, and this is +// the shell's pid and nothing more (GDK-891). func (s *Session) PID() int { return s.pid } // TokenID is the pairing token this session was opened with, empty for a diff --git a/internal/term/session_windows.go b/internal/term/session_windows.go index a8d6ff340..66d71483f 100644 --- a/internal/term/session_windows.go +++ b/internal/term/session_windows.go @@ -2,40 +2,621 @@ package term -import "fmt" - -// Windows has no PTY here yet. ConPTY is a different shape from -// openpty/Setsid — a pseudo-console handle pair plus -// PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, with no process group to signal — -// and it is being spiked separately (GDK-861). An honest refusal beats a -// silent stub that opens something and then behaves unlike every other -// platform: the pane can say "not on Windows yet" and mean it. +import ( + "errors" + "fmt" + "os" + "os/exec" + "sort" + "strings" + "sync" + "sync/atomic" + "unicode/utf16" + "unsafe" + + "golang.org/x/sys/windows" +) + +// ptyProc is the Windows half of a session: a ConPTY, the two pipes that +// carry its bytes, and the shell spawned onto it inside a Job Object. +// +// Spawn is a direct CreateProcess, not os/exec. Attaching a child to a +// pseudoconsole needs PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE riding a +// STARTUPINFOEX attribute list, and syscall.SysProcAttr — the struct +// behind os/exec's SysProcAttr — has no field that can carry one (go1.26 +// src/syscall/exec_windows.go: the struct offers flags, a token, security +// attributes and an inheritable-handle list, nothing more). The attribute +// list is exactly the surface the stdlib does not expose, so this file +// calls CreateProcess the way the Microsoft pseudoconsole sample and +// charmbracelet/x/conpty do, and wait() is WaitForSingleObject instead of +// cmd.Wait — the same observable answer rebuilt on the raw handle. +// +// The unix half signals a process group and, because a job-control shell +// moves jobs out of it, walks the controlling-terminal device to reach +// everyone: nothing on unix owns the tree. Windows has an owner — the Job +// Object. Every process a job member spawns is a member (unless it asks +// for a breakaway, which nothing here does), so the unix ladder becomes: // -// The type keeps the same method set as the unix ptyProc so the shared -// session core compiles unchanged; none of these methods can be reached, -// because startProc never returns a value. -type ptyProc struct{} +// hangup ClosePseudoConsole — conhost tears the session down and hands +// the attached clients the terminal-closed event; the machine's +// SIGHUP. The call is void in Win32 (fire-and-forget into +// conhost), so hangup reports no error: the observable is the +// pipes going quiet and wait() answering. +// kill TerminateJobObject — the whole tree, unconditionally and at +// once. Blunter than unix SIGKILL-to-the-tty-members, but the +// contract is the same one the unix walk exists to keep: no +// orphan survives the session. +// close the job handle, last — JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE is +// the backstop that fires after the ladder above had its turn, +// the moral equivalent of the kernel revoking the master fd. +// +// The child starts CREATE_SUSPENDED and joins the job before its first +// instruction runs, so "every descendant is in the job" is a theorem +// rather than AssignProcessToJobObject winning a race against the shell's +// first spawn. +// +// The handle ledger — Windows leaks by default, so every handle this file +// creates has one named closer: +// +// inPipeRead, outPipeWrite the ends handed to CreatePseudoConsole. +// conhost holds its own duplicates, so ours are redundant from +// the moment that call returns — and keeping outPipeWrite is +// worse than redundant: a write handle we hold open is a write +// handle the read side never sees EOF past. Both close in +// startProc, right after CreateProcess succeeds. +// hpc the pseudoconsole — once, by whichever of hangup() and +// closePTY() reaches it first (hpcOnce). +// attr the attribute list — deleted once, in closePTY, before hpc: +// its buffer references the pseudoconsole and no spawn reads it +// again. +// inPipeWrite, outPipeRead the master ends — in closePTY, after hpc: +// closing the pseudoconsole is what ends a pump parked in Read. +// job the job handle — last in closePTY, after the registry forgets +// this proc, so KILL_ON_JOB_CLOSE fires after the graceful +// ladder and a registry lookup never races a recycled value. +// proc the child process — in wait(), after the exit code is read. +// thread the child's main thread — in startProc, right after Resume. +type ptyProc struct { + procID int + + // endMu is the Windows analogue of the unix ioctlMu: it keeps the + // handle-touching control-plane calls (Write, resize, hangup, kill, + // jobPids) and closePTY off each other's toes. Windows recycles handle + // values aggressively, so a syscall on a just-closed handle is not the + // benign EBADF a stale fd is on unix — it can land on an unrelated + // live object of this process (another session's job, a socket). + // Every handle field is zeroed under the write side as it closes, so + // a reader holding the read side sees either a live handle or a zero. + // + // Read is deliberately not under this lock: the pump parks in ReadFile + // for the session's whole life and would hold the read side forever. + // closePTY un-parks it with CancelIoEx before taking the write side — + // the service os.File's poll teardown provides on unix for free. + endMu sync.RWMutex + hpc windows.Handle + job windows.Handle + inWrite windows.Handle + outRead windows.Handle + proc windows.Handle + attr *windows.ProcThreadAttributeListContainer + // hpcAttr is the heap box the attribute list points at: Update stores + // the lpValue POINTER, not the bytes, so the storage must outlive the + // spawn (charmbracelet/x/conpty keeps a *Handle for the same reason). + hpcAttr *windows.Handle + // cols/rows remember the size the pseudoconsole was last told. There + // is no GetPseudoConsoleSize, so winsize() answers from this record — + // a different epistemic thing than the unix TTYSize; see winsize. + cols, rows uint16 + + // closed flips true the instant closePTY starts, before any handle + // goes: a Write or resize racing the close then maps its failure to + // ErrSessionClosed — the contract every caller checks — instead of a + // raw ERROR_BROKEN_PIPE that leaks the handle's state. Same owner as + // the unix half (GDK-914). + closed atomic.Bool + + waitOnce sync.Once + code int + waitErr error + + closeOnce sync.Once + hpcOnce sync.Once + + // resizeExit names the branch the last resize left by, for Info()'s + // LastResizeExit diagnostic — same field, same reason as unix + // (GDK-1192), with a Windows-specific vocabulary: "set" (the call + // returned nil; no read-back exists on this platform to say more), + // "set-error:", "" before the first resize. + resizeExit atomic.Pointer[string] +} + +// startProc creates the pseudoconsole, the job, and the shell on it. On +// any failure every handle created so far is closed exactly once, in +// reverse creation order — the same order closePTY closes in. func startProc(opts Options) (*ptyProc, error) { - _ = opts - return nil, fmt.Errorf("%w: the terminal needs ConPTY on Windows (GDK-861)", ErrUnsupportedPlatform) + shell := opts.Shell + if shell == "" { + // COMSPEC is the Windows $SHELL: the shell the account is + // configured with, cmd.exe when nothing says otherwise. + shell = os.Getenv("COMSPEC") + } + if shell == "" { + shell = "cmd.exe" + } + // A bare name resolves against PATH and PATHEXT the way os/exec would; + // an absolute path round-trips. A miss keeps the raw string so + // CreateProcess below is the step that names the failure. + if resolved, err := exec.LookPath(shell); err == nil { + shell = resolved + } + // The same environment contract as the unix half, minus the login-shell + // merge: hostenv has no login shell to ask on Windows (its Resolve + // refuses, its Augment passes base through), so the pane runs on the + // serve's environment plus the terminal markers. opts.Env stays last — + // the caller's GADAK_WORKSPACE names the window this pane belongs to, + // and the last duplicate is the one the child sees. + base := os.Environ() + env := append([]string(nil), base...) + env = append(env, "TERM=xterm-256color", "GADAK_TERMINAL=1") + if !envCarries(base, "COLORTERM") { + env = append(env, "COLORTERM=truecolor") + } + env = append(env, opts.Env...) + + cols, rows := opts.Cols, opts.Rows + if cols == 0 { + cols = 80 + } + if rows == 0 { + rows = 24 + } + p := &ptyProc{cols: cols, rows: rows} + + // created collects every handle this function must close if a later + // step fails; the failure path walks it in reverse, the same order + // closePTY uses on the success path. + var created []windows.Handle + fail := func(step string, err error) (*ptyProc, error) { + recordWin32(step, err) + for i := len(created) - 1; i >= 0; i-- { + _ = windows.CloseHandle(created[i]) + } + if p.attr != nil { + p.attr.Delete() + } + return nil, fmt.Errorf("term: start %s: %s: %w", shell, step, err) + } + + sa := &windows.SecurityAttributes{ + Length: uint32(unsafe.Sizeof(windows.SecurityAttributes{})), + InheritHandle: 1, + } + var inRead, outWrite windows.Handle + if err := windows.CreatePipe(&inRead, &p.inWrite, sa, 0); err != nil { + return fail("CreatePipe(input)", err) + } + created = append(created, inRead, p.inWrite) + if err := windows.CreatePipe(&p.outRead, &outWrite, sa, 0); err != nil { + return fail("CreatePipe(output)", err) + } + created = append(created, p.outRead, outWrite) + + if err := windows.CreatePseudoConsole(windows.Coord{X: int16(cols), Y: int16(rows)}, inRead, outWrite, 0, &p.hpc); err != nil { + return fail("CreatePseudoConsole", err) + } + created = append(created, p.hpc) + + var err error + p.attr, err = windows.NewProcThreadAttributeList(1) + if err != nil { + return fail("NewProcThreadAttributeList", err) + } + // The attribute points at hpcAttr's storage, not at the handle value: + // UpdateProcThreadAttribute wants a pointer to the attribute value — + // and a pointer to typed storage is also the shape go vet accepts + // (uintptr→unsafe.Pointer is the conversion it flags). + p.hpcAttr = new(windows.Handle) + *p.hpcAttr = p.hpc + if err = p.attr.Update(windows.PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, unsafe.Pointer(p.hpcAttr), unsafe.Sizeof(*p.hpcAttr)); err != nil { + return fail("ProcThreadAttributeList.Update", err) + } + + if p.job, err = windows.CreateJobObject(nil, nil); err != nil { + return fail("CreateJobObject", err) + } + created = append(created, p.job) + var jobInfo windows.JOBOBJECT_EXTENDED_LIMIT_INFORMATION + jobInfo.BasicLimitInformation.LimitFlags = windows.JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE + if _, err = windows.SetInformationJobObject(p.job, windows.JobObjectExtendedLimitInformation, uintptr(unsafe.Pointer(&jobInfo)), uint32(unsafe.Sizeof(jobInfo))); err != nil { + return fail("SetInformationJobObject", err) + } + + cmdline, err := windows.UTF16PtrFromString(windows.ComposeCommandLine(append([]string{shell}, opts.Args...))) + if err != nil { + return fail("ComposeCommandLine", err) + } + var dir *uint16 + if opts.Dir != "" { + if dir, err = windows.UTF16PtrFromString(opts.Dir); err != nil { + return fail("UTF16PtrFromString(dir)", err) + } + } + siEx := &windows.StartupInfoEx{} + siEx.Cb = uint32(unsafe.Sizeof(*siEx)) + siEx.ProcThreadAttributeList = p.attr.List() + // STARTF_USESTDHANDLES is deliberately not set: the pseudoconsole + // attribute is what supplies the child's std handles, and pointing the + // startup record at (zero) inherited handles would fight it. + pi := new(windows.ProcessInformation) + flags := uint32(windows.CREATE_UNICODE_ENVIRONMENT) | windows.EXTENDED_STARTUPINFO_PRESENT | windows.CREATE_SUSPENDED + if err = windows.CreateProcess(nil, cmdline, nil, nil, false, flags, envBlock(env), dir, &siEx.StartupInfo, pi); err != nil { + return fail("CreateProcess", err) + } + created = append(created, pi.Process) + p.procID = int(pi.ProcessId) + p.proc = pi.Process + // The suspended start means a failed assign or resume leaves a child + // that never ran (or ran unjobbed): kill it explicitly, because + // closing its handle is not a kill. + if err = windows.AssignProcessToJobObject(p.job, pi.Process); err != nil { + _ = windows.TerminateProcess(pi.Process, 1) + return fail("AssignProcessToJobObject", err) + } + if _, err = windows.ResumeThread(pi.Thread); err != nil { + _ = windows.TerminateProcess(pi.Process, 1) + return fail("ResumeThread", err) + } + _ = windows.CloseHandle(pi.Thread) + + // conhost has held its own duplicates of these since CreatePseudoConsole + // returned, so ours are redundant — and ours holding outWrite open is + // worse than redundant: a write handle we keep alive is a write handle + // the read side will never see EOF past (the ledger above). + _ = windows.CloseHandle(inRead) + _ = windows.CloseHandle(outWrite) + + registerProc(p) + return p, nil +} + +// envCarries reports whether env has key at all, empty value included — +// presence, not value, is the not-overwriting rule for COLORTERM. The same +// helper the unix half has, on the platform where this file compiles. +func envCarries(env []string, key string) bool { + for _, kv := range env { + if k, _, ok := strings.Cut(kv, "="); ok && k == key { + return true + } + } + return false +} + +// envBlock builds the UTF-16 environment CreateProcess takes. Duplicates +// collapse case-insensitively with the last winning — os/exec's Windows +// contract, and the reason "the caller's Env is appended last" means +// anything here — and the block is sorted, as the stdlib's is. +func envBlock(env []string) *uint16 { + idx := make(map[string]int, len(env)) + out := make([]string, 0, len(env)) + for _, kv := range env { + k, _, ok := strings.Cut(kv, "=") + if !ok { + continue + } + k = strings.ToLower(k) + if i, dup := idx[k]; dup { + out[i] = kv + continue + } + idx[k] = len(out) + out = append(out, kv) + } + sort.Strings(out) + block := make([]uint16, 0, 64) + for _, kv := range out { + block = append(block, utf16.Encode([]rune(kv))...) + block = append(block, 0) + } + return &append(block, 0)[0] +} + +// Read is the pump's only entry to the output pipe. Not under endMu — see +// the field comment for why a lock held across a parked ReadFile would be +// a deadlock, and how closePTY un-parks it instead. +func (p *ptyProc) Read(b []byte) (int, error) { + var n uint32 + err := windows.ReadFile(p.outRead, b, &n, nil) + return int(n), err +} + +func (p *ptyProc) Write(b []byte) (int, error) { + p.endMu.RLock() + defer p.endMu.RUnlock() + var n uint32 + err := windows.WriteFile(p.inWrite, b, &n, nil) + if err != nil && (p.closed.Load() || p.inWrite == 0) { + return int(n), ErrSessionClosed + } + return int(n), err } -func (p *ptyProc) Read(b []byte) (int, error) { return 0, ErrUnsupportedPlatform } -func (p *ptyProc) Write(b []byte) (int, error) { return 0, ErrUnsupportedPlatform } -func (p *ptyProc) pid() int { return 0 } +func (p *ptyProc) pid() int { return p.procID } + +// resize has neither of the unix retry machines: Windows does not +// interrupt in-flight syscalls with signals (no EINTR to retry — a +// failure is a failure), and there is no size read-back API to verify +// with (no "matched/N" branch to record). What remains of the unix +// contract is the diagnostic: the exit is named, and a size the call +// accepted is remembered as the size the session carries. func (p *ptyProc) resize(cols, rows uint16) error { - _, _ = cols, rows - return ErrUnsupportedPlatform + p.endMu.RLock() + err := p.resizeHeld(cols, rows) + p.endMu.RUnlock() + if err != nil && p.closed.Load() { + return ErrSessionClosed + } + return err } + +// resizeHeld runs under endMu's read side. +func (p *ptyProc) resizeHeld(cols, rows uint16) error { + if p.hpc == 0 { + return ErrSessionClosed + } + if err := windows.ResizePseudoConsole(p.hpc, windows.Coord{X: int16(cols), Y: int16(rows)}); err != nil { + recordWin32("ResizePseudoConsole", err) + p.recordResizeExit("set-error:" + err.Error()) + return err + } + p.cols, p.rows = cols, rows + p.recordResizeExit("set") + return nil +} + +func (p *ptyProc) recordResizeExit(s string) { p.resizeExit.Store(&s) } + +// lastResizeExit reports the recorded exit, "" when no resize has run yet. +func (p *ptyProc) lastResizeExit() string { + if s := p.resizeExit.Load(); s != nil { + return *s + } + return "" +} + +// winsize answers the size the pseudoconsole carries. On unix this is +// TIOCGWINSZ — the kernel's answer, the evidence that a resize the session +// believes it applied is the one the child's tty has. ConPTY has no +// read-back, so this is the last size resize() accepted (the creation size +// before any): our own record, not the machine's. The unix method exists +// to catch "accepted and did nothing" (GDK-1192); this one cannot catch +// that class by construction, and the honest answer is the record. func (p *ptyProc) winsize() (cols, rows uint16, err error) { - return 0, 0, ErrUnsupportedPlatform + p.endMu.RLock() + defer p.endMu.RUnlock() + if p.hpc == 0 { + return 0, 0, ErrSessionClosed + } + return p.cols, p.rows, nil +} + +// hangup is the graceful rung of the ladder — ClosePseudoConsole, the +// machine's "terminal closed". It shares hpcOnce with closePTY so the +// pseudoconsole closes exactly once whichever rung reaches it first; a +// second close would be a close of a recycled handle value. +func (p *ptyProc) hangup() error { + p.endMu.RLock() + defer p.endMu.RUnlock() + p.closeConPTY() + return nil +} + +// kill is the whole job at once. After it the job handle stays open and +// registered (closePTY owns both) but empty: membersOf keeps answering +// through the job, now with nobody in it — "nothing running", the honest +// answer for a terminated tree, and the axis the tree-kill test measures. +// The one path that kills without ever closing (Manager.Create's +// already-closed branch) leaves a registry entry and a job handle behind +// rather than blinding that seam; the unix half leaks the same shape +// there (a dropped *exec.Cmd that nobody Waits). +func (p *ptyProc) kill() error { + p.endMu.RLock() + defer p.endMu.RUnlock() + if p.closed.Load() || p.job == 0 { + return nil + } + if err := windows.TerminateJobObject(p.job, 1); err != nil { + recordWin32("TerminateJobObject", err) + return err + } + return nil +} + +// wait is the answer os/exec's cmd.Wait would have given, rebuilt for the +// direct CreateProcess: block on the process handle, read the exit code, +// close the handle once. Idempotent through waitOnce, like the unix half. +func (p *ptyProc) wait() (int, error) { + p.waitOnce.Do(func() { + ev, err := windows.WaitForSingleObject(p.proc, windows.INFINITE) + if err == nil && ev == windows.WAIT_OBJECT_0 { + var code uint32 + if err = windows.GetExitCodeProcess(p.proc, &code); err != nil { + recordWin32("GetExitCodeProcess", err) + } else { + p.code = int(code) + } + } + p.waitErr = err + if p.proc != 0 { + _ = windows.CloseHandle(p.proc) + p.proc = 0 + } + }) + return p.code, p.waitErr +} + +// closeConPTY closes the pseudoconsole exactly once. Both hangup and +// closePTY arrive here; neither may close it twice. +func (p *ptyProc) closeConPTY() { + p.hpcOnce.Do(func() { + if p.hpc != 0 { + windows.ClosePseudoConsole(p.hpc) + p.hpc = 0 + } + }) +} + +// closePTY runs the ledger's close side, once, in the reference +// implementation's order with the job appended: +// +// - CancelIoEx first, outside endMu: a Write parked on a full input pipe +// holds endMu's read side, and cancelling the parked call is the only +// thing that releases it — taking the write side first would +// deadlock. This is what os.File's poll teardown gives the unix half +// for free (a blocked writer there returns when the fd closes). +// - the registry, before any handle goes: a lookup must never hand back +// a proc whose job handle is about to become a recycled value. +// - the attribute list, before hpc: its buffer references the +// pseudoconsole and no spawn will read it again. +// - the pseudoconsole: the hangup. Closing it is what makes conhost let +// go of the output pipe's write side, which is what ends a pump +// parked in Read. +// - the master pipe ends, after conhost is gone: handle hygiene. +// - the job handle, last: KILL_ON_JOB_CLOSE must not fire before the +// graceful ladder above has had its turn. +func (p *ptyProc) closePTY() error { + var err error + p.closeOnce.Do(func() { + p.closed.Store(true) + // Best effort by design: nothing to cancel is the normal case + // (ERROR_NOT_FOUND), and the closes below are what matter. + _ = windows.CancelIoEx(p.outRead, nil) + _ = windows.CancelIoEx(p.inWrite, nil) + p.endMu.Lock() + defer p.endMu.Unlock() + unregisterProc(p.procID) + if p.attr != nil { + p.attr.Delete() + } + p.closeConPTY() + // Zero before closing: Read does not take endMu (it parks), so a + // racing reader that catches the close before the zero could act + // on a handle value Windows has already handed to another object. + // A reader that sees the zero gets an immediate error instead — + // the pump's normal end. + inW, outR := p.inWrite, p.outRead + p.inWrite, p.outRead = 0, 0 + err = errors.Join(windows.CloseHandle(inW), windows.CloseHandle(outR)) + job := p.job + p.job = 0 + if job != 0 { + err = errors.Join(err, windows.CloseHandle(job)) + } + }) + return err +} + +// jobProcessIdList is JOBOBJECT_BASIC_PROCESS_ID_LIST with room for a +// session's ordinary tree inline; bigger trees retry on the heap below. +type jobProcessIdList struct { + Assigned uint32 + InList uint32 + Pids [64]uintptr +} + +// jobPids is the job's own answer to "who is running in this session's +// tree" — the enumeration the unix half re-derives by walking the +// controlling-terminal device, because unix has no owner of the tree to +// ask. Includes the shell. Nil once the job is gone or closed, and on a +// query failure — which membersOf reads as "nothing running", the same +// fail direction as the unix walk coming back empty. +func (p *ptyProc) jobPids() []int { + p.endMu.RLock() + defer p.endMu.RUnlock() + if p.job == 0 { + return nil + } + var raw jobProcessIdList + var ret uint32 + err := windows.QueryInformationJobObject(p.job, windows.JobObjectBasicProcessIdList, uintptr(unsafe.Pointer(&raw)), uint32(unsafe.Sizeof(raw)), &ret) + if errors.Is(err, windows.ERROR_MORE_DATA) { + // More members than the inline array holds: size the retry on the + // assigned count the first call still filled in. + n := int(raw.Assigned) + if n < len(raw.Pids) { + n = len(raw.Pids) + } + buf := make([]byte, int(unsafe.Offsetof(raw.Pids))+n*int(unsafe.Sizeof(uintptr(0)))) + err = windows.QueryInformationJobObject(p.job, windows.JobObjectBasicProcessIdList, uintptr(unsafe.Pointer(&buf[0])), uint32(len(buf)), &ret) + if err == nil { + big := (*jobProcessIdList)(unsafe.Pointer(&buf[0])) + return pidsToList(unsafe.Slice(&big.Pids[0], big.InList)) + } + } + if err != nil { + recordWin32("QueryInformationJobObject", err) + return nil + } + return pidsToList(raw.Pids[:raw.InList]) +} + +func pidsToList(pids []uintptr) []int { + out := make([]int, 0, len(pids)) + for _, pid := range pids { + if pid != 0 { + out = append(out, int(pid)) + } + } + if len(out) == 0 { + return nil + } + return out +} + +// liveProcs is the pid → proc registry that lets membersOf turn "every +// process on this session's terminal" into a job query: the shell pid is +// the only key the session core passes down. Entries leave at kill (the +// tree is gone) and at closePTY (before the job handle closes, so a +// lookup can never race a recycled handle value). +var liveProcs struct { + sync.Mutex + m map[int]*ptyProc +} + +func registerProc(p *ptyProc) { + liveProcs.Lock() + if liveProcs.m == nil { + liveProcs.m = map[int]*ptyProc{} + } + liveProcs.m[p.procID] = p + liveProcs.Unlock() +} + +func unregisterProc(pid int) { + liveProcs.Lock() + delete(liveProcs.m, pid) + liveProcs.Unlock() +} + +func lookupProc(pid int) *ptyProc { + liveProcs.Lock() + defer liveProcs.Unlock() + return liveProcs.m[pid] } -// lastResizeExit mirrors the unix diagnostic field on the stub: resize can -// never run here, so the answer is fixed. -func (p *ptyProc) lastResizeExit() string { return "unsupported" } -func (p *ptyProc) hangup() error { return ErrUnsupportedPlatform } -func (p *ptyProc) kill() error { return ErrUnsupportedPlatform } -func (p *ptyProc) wait() (int, error) { return -1, ErrUnsupportedPlatform } -func (p *ptyProc) closePTY() error { return ErrUnsupportedPlatform } +// recordWin32 keeps the last control-plane failure — the call's name and +// its Win32 error code, nothing else — for LastWin32Failure, which doctor +// reads. The data plane deliberately does not record: a broken pipe is how +// every session ends, and doctor must not read a normal shutdown as a +// terminal failure. +func recordWin32(call string, err error) { + if err == nil { + return + } + var code uint32 + var eno windows.Errno + if errors.As(err, &eno) { + code = uint32(eno) + } + lastWin32Failure.Store(&win32Failure{call: call, code: code}) +} diff --git a/internal/term/session_windows_test.go b/internal/term/session_windows_test.go new file mode 100644 index 000000000..0558e4350 --- /dev/null +++ b/internal/term/session_windows_test.go @@ -0,0 +1,243 @@ +//go:build windows + +package term + +import ( + "os" + "regexp" + "strconv" + "strings" + "testing" + "time" +) + +// The ConPTY contract, pinned against a real pseudoconsole. These tests +// are the only place this package's Windows half executes — the dev +// machines are macOS, so CI's Desktop Windows job runs them (GDK-891). +// Every test starts through startProc or Manager.Create, which on the +// pre-GDK-891 stub failed with ErrUnsupportedPlatform at the first +// assertion: that stub is the FAIL-first these tests were red against. + +// testWindowsManager is New with a grace short enough to observe. The +// grace is the one contract a test cannot wait out honestly (60s), so +// Config takes it. +func testWindowsManager(t *testing.T) *Manager { + t.Helper() + m := New(Config{Grace: 2 * time.Second}) + t.Cleanup(m.CloseAll) + return m +} + +// comspec names the one shell every Windows image has: $SHELL's Windows +// counterpart, cmd.exe when nothing says otherwise. Pinning the test to +// COMSPEC rather than a developer's setting is the same discipline the +// unix half pins to /bin/sh. +func comspec() string { + if s := os.Getenv("COMSPEC"); s != "" { + return s + } + return "cmd.exe" +} + +// comspecSession starts that shell under a real session. +func comspecSession(t *testing.T, m *Manager, opts Options) *Session { + t.Helper() + if opts.Shell == "" { + opts.Shell = comspec() + } + s, err := m.Create(opts) + if err != nil { + t.Fatalf("Create: %v", err) + } + t.Cleanup(func() { _ = s.Close() }) + return s +} + +// readUntil collects output until want appears or the deadline passes. +// On Done it drains once before failing: a backlog pending at the end is +// still readable through Take, so a marker that arrived in the last chunk +// must not be reported missing. +func readUntil(t *testing.T, a *Attachment, want string, within time.Duration) string { + t.Helper() + var got strings.Builder + deadline := time.After(within) + for { + select { + case <-a.Wake(): + got.Write(a.Take()) + if strings.Contains(got.String(), want) { + return got.String() + } + case <-a.Done(): + got.Write(a.Take()) + if strings.Contains(got.String(), want) { + return got.String() + } + t.Fatalf("waiting for %q; attachment ended with %q", want, got.String()) + case <-deadline: + t.Fatalf("waiting for %q; got %q", want, got.String()) + } + } +} + +// drainFor collects output until the stream has been quiet for the whole +// gap — the "clear the banner and the echoed input" helper the phases +// below run between their commands. +func drainFor(t *testing.T, a *Attachment, gap time.Duration) string { + t.Helper() + var got strings.Builder + quiet := time.NewTimer(gap) + defer quiet.Stop() + for { + select { + case <-a.Wake(): + got.Write(a.Take()) + quiet.Reset(gap) + case <-a.Done(): + got.Write(a.Take()) + return got.String() + case <-quiet.C: + return got.String() + } + } +} + +// ① The roundtrip: a command this test typed comes back as its output. +// The value is assembled from two variables so the marker exists in that +// form only in the shell's OUTPUT — the pane echoes the raw input lines, +// and a marker that appears in them proves nothing about the direction of +// the pipe. +func TestSessionEchoRoundtrip(t *testing.T) { + m := testWindowsManager(t) + s := comspecSession(t, m, Options{}) + a, err := s.Attach() + if err != nil { + t.Fatalf("Attach: %v", err) + } + for _, line := range []string{ + "set gadak_probe_a=con_pty_", + "set gadak_probe_b=roundtrip", + "echo %gadak_probe_a%%gadak_probe_b%", + } { + if _, err := s.Write([]byte(line + "\r")); err != nil { + t.Fatalf("Write(%q): %v", line, err) + } + } + readUntil(t, a, "con_pty_roundtrip", 20*time.Second) +} + +// ② resize, answered by the child itself: `mode con` prints the console +// size the shell sees, which on a healthy pseudoconsole is the size +// Resize just applied — the same axis the unix half pins with stty size. +// winsize() is along for the record it keeps, which is all it can keep: +// ConPTY has no read-back, so the remembered size is the pinned contract +// and the child's answer is the evidence. +func TestResizeChildAnswers(t *testing.T) { + m := testWindowsManager(t) + s := comspecSession(t, m, Options{Cols: 80, Rows: 24}) + a, err := s.Attach() + if err != nil { + t.Fatalf("Attach: %v", err) + } + cols, rows, err := s.TTYSize() + if err != nil || cols != 80 || rows != 24 { + t.Fatalf("initial winsize = %dx%d, %v; want 80x24", cols, rows, err) + } + drainFor(t, a, 500*time.Millisecond) + if err := s.Resize(120, 40); err != nil { + t.Fatalf("Resize: %v", err) + } + if _, err := s.Write([]byte("mode con\r")); err != nil { + t.Fatalf("Write(mode con): %v", err) + } + out := readUntil(t, a, "Columns:", 20*time.Second) + colRe := regexp.MustCompile(`(?im)^\s*Columns:\s+(\d+)`) + lineRe := regexp.MustCompile(`(?im)^\s*Lines:\s+(\d+)`) + cm, lm := colRe.FindStringSubmatch(out), lineRe.FindStringSubmatch(out) + if cm == nil || lm == nil { + t.Fatalf("mode con output had no Columns/Lines pair:\n%s", out) + } + gotCols, _ := strconv.Atoi(cm[1]) + gotRows, _ := strconv.Atoi(lm[1]) + if gotCols != 120 || gotRows != 40 { + t.Fatalf("child reports %dx%d after Resize(120,40);\nmode con output:\n%s", gotCols, gotRows, out) + } + if cols, rows, err = s.TTYSize(); err != nil || cols != 120 || rows != 40 { + t.Fatalf("winsize after resize = %dx%d, %v; want 120x40", cols, rows, err) + } +} + +// ③ The round's core contract: kill takes the child's child with it. The +// unix half pins a HUP-immune grandchild; the Windows machine is one Job +// Object, so the axis is the job's own member list, before (the +// grandchild must be a member — spawned by the shell, no breakaway) and +// after (nobody), with wait() answering in between. +func TestKillTakesTheTreeWithIt(t *testing.T) { + p, err := startProc(Options{Cols: 80, Rows: 24}) + if err != nil { + t.Fatalf("startProc: %v", err) + } + t.Cleanup(func() { _ = p.closePTY() }) + if _, err := p.Write([]byte("start /b ping -n 60 127.0.0.1\r")); err != nil { + t.Fatalf("Write(start): %v", err) + } + var members []int + deadline := time.Now().Add(20 * time.Second) + for time.Now().Before(deadline) { + if members = membersOf(p.pid()); len(members) >= 2 { + break + } + time.Sleep(100 * time.Millisecond) + } + if len(members) < 2 { + t.Fatalf("grandchild never joined the job; members=%v", members) + } + if err := p.kill(); err != nil { + t.Fatalf("kill: %v", err) + } + waited := make(chan error, 1) + go func() { + _, werr := p.wait() + waited <- werr + }() + select { + case werr := <-waited: + if werr != nil { + t.Fatalf("wait after kill: %v", werr) + } + case <-time.After(20 * time.Second): + t.Fatal("wait did not return after kill") + } + deadline = time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + if members = membersOf(p.pid()); len(members) == 0 { + return + } + time.Sleep(100 * time.Millisecond) + } + t.Fatalf("job still has members after kill: %v", members) +} + +// ④ closePTY is safe to run twice, and after hangup: the pseudoconsole +// close is shared between them, so the ladder hangup → closePTY → +// closePTY must not double-close any handle — on Windows a double close +// can land on a recycled handle value, which is somebody else's live +// object. +func TestClosePTYTwiceAfterHangup(t *testing.T) { + p, err := startProc(Options{Cols: 80, Rows: 24}) + if err != nil { + t.Fatalf("startProc: %v", err) + } + if err := p.hangup(); err != nil { + t.Fatalf("hangup: %v", err) + } + if err := p.closePTY(); err != nil { + t.Fatalf("first closePTY: %v", err) + } + if err := p.closePTY(); err != nil { + t.Fatalf("second closePTY: %v", err) + } + if err := p.hangup(); err != nil { + t.Fatalf("hangup after close: %v", err) + } +} diff --git a/internal/term/win32diag.go b/internal/term/win32diag.go new file mode 100644 index 000000000..d0c1c4103 --- /dev/null +++ b/internal/term/win32diag.go @@ -0,0 +1,30 @@ +package term + +import "sync/atomic" + +// win32Failure is the ConPTY control plane's last recorded failure: which +// Win32 call failed and with what error code. A call name and a count — +// the paste-safe rule the doctor banner states; no paths, no error prose. +type win32Failure struct { + call string + code uint32 +} + +// lastWin32Failure is process-wide on purpose: doctor reports on the +// serve, not on one session, and "the terminal subsystem's last error" is +// a serve-shaped question. The recorder (recordWin32, in +// session_windows.go) is Windows-only code; on every other platform this +// stays nil and LastWin32Failure reports nothing. +var lastWin32Failure atomic.Pointer[win32Failure] + +// LastWin32Failure is the diagnostic hook `gadak doctor` reads (GDK-891): +// the name of the Win32 call that last failed in the terminal's control +// plane — creation, resize, teardown — and its error code. ok is false +// when no failure has been recorded, which on non-Windows builds is +// always. +func LastWin32Failure() (call string, code int, ok bool) { + if f := lastWin32Failure.Load(); f != nil { + return f.call, int(f.code), true + } + return "", 0, false +}