diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5aa6da5a..56d4d080 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 9727d704..ac904093 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 87465b8b..a7d455ce 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 134dc61f..80483c05 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 5f5c6716..72044a9a 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 2f7fbcc6..36190c0b 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 a8d6ff34..66d71483 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 00000000..0558e435 --- /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 00000000..d0c1c410 --- /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 +}