diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 20a8742..8eee1ef 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -176,6 +176,15 @@ jobs: - name: Verify dependencies run: go mod verify + # The PTY service is the one package whose behaviour is genuinely + # per-platform: creack/pty on Unix, ConPTY on Windows, and a termination + # path that has no SIGHUP to send on Windows at all. The Test job above + # only ever exercises the Linux half, so a ConPTY regression would reach + # a release having passed every gate. This runs the same tests `make + # test` runs, on each OS in the matrix. + - name: Smoke-test the PTY service on this platform + run: go test -race -shuffle=on -count=1 ./internal/pty/... + - name: Install the Wails CLI run: go install github.com/wailsapp/wails/v2/cmd/wails@v2.13.0 diff --git a/go.mod b/go.mod index 5ba73e2..467f001 100644 --- a/go.mod +++ b/go.mod @@ -6,11 +6,15 @@ go 1.26.5 // otherwise pull into this module's build, test, coverage and lint denominator. ignore frontend/node_modules -require github.com/wailsapp/wails/v2 v2.13.0 +require ( + github.com/aymanbagabas/go-pty v0.2.3 + github.com/wailsapp/wails/v2 v2.13.0 +) require ( git.sr.ht/~jackmordaunt/go-toast/v2 v2.0.3 // indirect github.com/bep/debounce v1.2.1 // indirect + github.com/creack/pty v1.1.24 // indirect github.com/go-ole/go-ole v1.3.0 // indirect github.com/godbus/dbus/v5 v5.1.0 // indirect github.com/google/uuid v1.6.0 // indirect @@ -29,6 +33,7 @@ require ( github.com/rivo/uniseg v0.4.7 // indirect github.com/samber/lo v1.49.1 // indirect github.com/tkrajina/go-reflector v0.5.8 // indirect + github.com/u-root/u-root v0.16.0 // indirect github.com/valyala/bytebufferpool v1.0.0 // indirect github.com/valyala/fasttemplate v1.2.2 // indirect github.com/wailsapp/go-webview2 v1.0.22 // indirect diff --git a/go.sum b/go.sum index b664a33..589108a 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,11 @@ git.sr.ht/~jackmordaunt/go-toast/v2 v2.0.3 h1:N3IGoHHp9pb6mj1cbXbuaSXV/UMKwmbKLf53nQmtqMA= git.sr.ht/~jackmordaunt/go-toast/v2 v2.0.3/go.mod h1:QtOLZGz8olr4qH2vWK0QH0w0O4T9fEIjMuWpKUsH7nc= +github.com/aymanbagabas/go-pty v0.2.3 h1:hsqcTIUV8I4iTSh3HQl61CR2wh0YPS6gHOYLhAfWu/E= +github.com/aymanbagabas/go-pty v0.2.3/go.mod h1:GLkgQovzqN5A1xMB79yHWiG1rhcquZCjkwKQGKFPdPg= github.com/bep/debounce v1.2.1 h1:v67fRdBA9UQu2NhLFXrSg0Brw7CexQekrBwDMM8bzeY= github.com/bep/debounce v1.2.1/go.mod h1:H8yggRPQKLUhUoqrJC1bO2xNya7vanpDl7xR3ISbCJ0= +github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s= +github.com/creack/pty v1.1.24/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/go-ole/go-ole v1.3.0 h1:Dt6ye7+vXGIKZ7Xtk4s6/xVdGDQynvom7xCFEdWr6uE= @@ -12,6 +16,8 @@ github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg= github.com/gorilla/websocket v1.5.3/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= +github.com/hugelgupf/vmtest v0.0.0-20240307030256-5d9f3d34a58d h1:nP8SfQJqruIVSWYJTuYc37jLHEY1Z0fF+zKSrs3K/C8= +github.com/hugelgupf/vmtest v0.0.0-20240307030256-5d9f3d34a58d/go.mod h1:B63hDJMhTupLWCHwopAyEo7wRFowx9kOc8m8j1sfOqE= github.com/jchv/go-winloader v0.0.0-20210711035445-715c2860da7e h1:Q3+PugElBCf4PFpxhErSzU3/PY5sFL5Z6rfv4AbGAck= github.com/jchv/go-winloader v0.0.0-20210711035445-715c2860da7e/go.mod h1:alcuEEnZsY1WQsagKhZDsoPCRoOijYqhZvPwLG0kzVs= github.com/labstack/echo/v4 v4.13.3 h1:pwhpCPrTl5qry5HRdM5FwdXnhXSLSY+WE+YQSeCaafY= @@ -51,6 +57,10 @@ github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/tkrajina/go-reflector v0.5.8 h1:yPADHrwmUbMq4RGEyaOUpz2H90sRsETNVpjzo3DLVQQ= github.com/tkrajina/go-reflector v0.5.8/go.mod h1:ECbqLgccecY5kPmPmXg1MrHW585yMcDkVl6IvJe64T4= +github.com/u-root/gobusybox/src v0.0.0-20250101170133-2e884e4509c7 h1:dtiVT4SeBUc/vHtwI2HjDZN+FCKTstQBxugIxJEGo9g= +github.com/u-root/gobusybox/src v0.0.0-20250101170133-2e884e4509c7/go.mod h1:PW3wGFCHjdHxAhra5FKvcARbCGqGfentYuPKmuhv8DY= +github.com/u-root/u-root v0.16.0 h1:wY40O83MBVks97+Is0WlFlOPSwKQMIrWP9R1IsrExg8= +github.com/u-root/u-root v0.16.0/go.mod h1:yL/XdSSW27PdGLgUh4MNRBy54mKM+TBLzpwiB4nwj90= github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6KllzawFIhcdPw= github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= github.com/valyala/fasttemplate v1.2.2 h1:lxLXG0uE3Qnshl9QyaK6XJxMXlQZELvChBOCmQD0Loo= @@ -63,9 +73,13 @@ github.com/wailsapp/wails/v2 v2.13.0 h1:S7OgXWpj72V91unF8iDWJKbcS9ZpwCT3R0QVru4v github.com/wailsapp/wails/v2 v2.13.0/go.mod h1:nVr/wSIEZ7xxKPkzK65mjpKpaOPQI2k4pvLwGR/i4kc= golang.org/x/crypto v0.51.0 h1:IBPXwPfKxY7cWQZ38ZCIRPI50YLeevDLlLnyC5wRGTI= golang.org/x/crypto v0.51.0/go.mod h1:8AdwkbraGNABw2kOX6YFPs3WM22XqI4EXEd8g+x7Oc8= +golang.org/x/mod v0.35.0 h1:Ww1D637e6Pg+Zb2KrWfHQUnH2dQRLBQyAtpr/haaJeM= +golang.org/x/mod v0.35.0/go.mod h1:+GwiRhIInF8wPm+4AoT6L0FA1QWAad3OMdTRx4tFYlU= golang.org/x/net v0.0.0-20210505024714-0287a6fb4125/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.54.0 h1:2zJIZAxAHV/OHCDTCOHAYehQzLfSXuf/5SoL/Dv6w/w= golang.org/x/net v0.54.0/go.mod h1:Sj4oj8jK6XmHpBZU/zWHw3BV3abl4Kvi+Ut7cQcY+cQ= +golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= +golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.0.0-20200810151505-1b9f1253b3ed/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -75,9 +89,13 @@ golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.44.0 h1:ildZl3J4uzeKP07r2F++Op7E9B29JRUy+a27EibtBTQ= golang.org/x/sys v0.44.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= +golang.org/x/term v0.43.0 h1:S4RLU2sB31O/NCl+zFN9Aru9A/Cq2aqKpTZJ6B+DwT4= +golang.org/x/term v0.43.0/go.mod h1:lrhlHNdQJHO+1qVYiHfFKVuVioJIheAc3fBSMFYEIsk= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc= golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/tools v0.44.0 h1:UP4ajHPIcuMjT1GqzDWRlalUEoY+uzoZKnhOjbIPD2c= +golang.org/x/tools v0.44.0/go.mod h1:KA0AfVErSdxRZIsOVipbv3rQhVXTnlU6UhKxHd1seDI= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/godobject_budget_test.go b/godobject_budget_test.go index b8ddfd1..e0ba952 100644 --- a/godobject_budget_test.go +++ b/godobject_budget_test.go @@ -27,7 +27,13 @@ const ( // loose fields where one owner struct belonged. If raising it by more than // one per service, the question to answer in review is why the service is // not one handle. - maxAppFields = 1 + // + // 1 -> 2 in #2: the PTY service arrives as a single *pty.Manager handle. + // Every terminal behavior — create, write, resize, kill, scrollback — + // lives in internal/pty and is reached through that one field, so this is + // the one-handle-per-service case the paragraph above describes, not + // state accumulating on the coordinator. + maxAppFields = 2 // maxAppMethods caps methods with an App receiver, counting value and // pointer receivers alike. Pinned at today's actual with zero slack. diff --git a/internal/app/app.go b/internal/app/app.go index 3e3d79f..91bd5b4 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -5,12 +5,14 @@ package app import ( + "context" "embed" "github.com/wailsapp/wails/v2/pkg/options" "github.com/wailsapp/wails/v2/pkg/options/assetserver" "github.com/txn2/m6t/internal/buildinfo" + "github.com/txn2/m6t/internal/pty" ) // Window geometry. m6t is a single-window app: the three-pane project @@ -34,13 +36,19 @@ var windowBackground = &options.RGBA{R: 22, G: 24, B: 29, A: 1} // asset server) out of it. type App struct { info buildinfo.Info + + // terminals owns the PTY sessions behind the embedded terminal. It is one + // handle rather than loose state because the app composes services, it + // does not implement them: everything the terminal does lives in + // internal/pty and is reached through here. + terminals *pty.Manager } // newApp builds the binding. It is unexported because Options is the only // supported way to construct the application: an App that is not bound into // the window options is unreachable from the frontend. func newApp() *App { - return &App{info: buildinfo.Get()} + return &App{info: buildinfo.Get(), terminals: pty.New()} } // Version reports the build identity to the frontend, which shows it in the @@ -54,6 +62,8 @@ func (a *App) Version() buildinfo.Info { // bound methods, embedded assets — is covered by tests rather than asserted in // a comment. func Options(assets embed.FS) *options.App { + application := newApp() + return &options.App{ Title: windowTitle, Width: windowWidth, @@ -62,6 +72,14 @@ func Options(assets embed.FS) *options.App { MinHeight: windowMinHeight, AssetServer: &assetserver.Options{Assets: assets}, BackgroundColour: windowBackground, - Bind: []any{newApp()}, + Bind: []any{application}, + + // PTYs are backend-owned and outlive every window in the app, so the + // only thing that ends them is the app ending. Without this hook, + // quitting m6t would leave the user's shells — and whatever they were + // running — orphaned behind it. + OnShutdown: func(context.Context) { + application.terminals.Shutdown() + }, } } diff --git a/internal/app/app_test.go b/internal/app/app_test.go index d5f9748..2dc2c6d 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -1,10 +1,14 @@ package app import ( + "context" "embed" + "errors" + "runtime" "testing" "github.com/txn2/m6t/internal/buildinfo" + "github.com/txn2/m6t/internal/pty" ) //go:embed testdata @@ -75,6 +79,40 @@ func TestOptionsBindOnlyTheApp(t *testing.T) { } } +// PTYs are backend-owned and outlive every window, so the app quitting is the +// only thing that ends them. Without the shutdown hook, closing m6t would +// orphan the user's shells and whatever they were running. +func TestShutdownTerminatesEveryTerminalSession(t *testing.T) { + opts := Options(testAssets) + + application, ok := opts.Bind[0].(*App) + if !ok { + t.Fatalf("Bind[0] is %T, want *App", opts.Bind[0]) + } + if opts.OnShutdown == nil { + t.Fatal("OnShutdown is nil; quitting the app would leave its PTY sessions running") + } + + id, err := application.terminals.Create(pty.Options{Command: longRunningCommand()}) + if err != nil { + t.Fatalf("creating a terminal session: %v", err) + } + + opts.OnShutdown(context.Background()) + + if _, err := application.terminals.Attach(id); !errors.Is(err, pty.ErrNoSuchSession) { + t.Errorf("session %s survived shutdown: Attach error = %v, want ErrNoSuchSession", id, err) + } +} + +// longRunningCommand returns an argv that stays alive until it is killed. +func longRunningCommand() []string { + if runtime.GOOS == "windows" { + return []string{"cmd.exe", "/c", "ping -n 61 127.0.0.1 >NUL"} + } + return []string{"/bin/sh", "-c", "sleep 60"} +} + func TestEachCallBuildsItsOwnBinding(t *testing.T) { first := Options(testAssets).Bind[0] second := Options(testAssets).Bind[0] diff --git a/internal/pty/hangup_unix.go b/internal/pty/hangup_unix.go new file mode 100644 index 0000000..c48d983 --- /dev/null +++ b/internal/pty/hangup_unix.go @@ -0,0 +1,42 @@ +//go:build !windows + +package pty + +import ( + "fmt" + "os" + "syscall" +) + +// hangup asks the child to end the way closing a terminal window does, giving +// a shell the chance to run its exit traps. +func hangup(proc *os.Process) error { + return signalGroup(proc, syscall.SIGHUP) +} + +// forceKill ends the child unconditionally. It is the follow-up for a child +// that ignored the hangup. +func forceKill(proc *os.Process) error { + return signalGroup(proc, syscall.SIGKILL) +} + +// signalGroup signals the child's whole process group, falling back to the +// child alone if the group cannot be determined. +// +// The group is the point. go-pty starts the child in its own session, so a +// shell that has spawned `claude` or `vim` is the leader of a group containing +// them. Signaling only the shell would reap the shell and orphan everything +// it started — which is exactly the "no zombie processes" failure this is +// written to avoid. +func signalGroup(proc *os.Process, sig syscall.Signal) error { + if pgid, err := syscall.Getpgid(proc.Pid); err == nil { + if err := syscall.Kill(-pgid, sig); err != nil { + return fmt.Errorf("signaling process group %d: %w", pgid, err) + } + return nil + } + if err := proc.Signal(sig); err != nil { + return fmt.Errorf("signaling process %d: %w", proc.Pid, err) + } + return nil +} diff --git a/internal/pty/hangup_windows.go b/internal/pty/hangup_windows.go new file mode 100644 index 0000000..74a3df1 --- /dev/null +++ b/internal/pty/hangup_windows.go @@ -0,0 +1,28 @@ +//go:build windows + +package pty + +import ( + "fmt" + "os" +) + +// hangup ends the child. +// +// Windows has no SIGHUP. A console application is ended by closing its +// pseudoconsole or by terminating the process outright, and go-pty owns the +// pseudoconsole handle, so termination is the honest option here. The +// consequence is that the killGrace window in session.kill buys a Windows +// child nothing — it is already gone when the grace period would have started. +func hangup(proc *os.Process) error { + if err := proc.Kill(); err != nil { + return fmt.Errorf("terminating process %d: %w", proc.Pid, err) + } + return nil +} + +// forceKill ends the child unconditionally. On Windows there is nothing +// gentler than hangup to escalate from, so the two are the same call. +func forceKill(proc *os.Process) error { + return hangup(proc) +} diff --git a/internal/pty/manager.go b/internal/pty/manager.go new file mode 100644 index 0000000..98ee771 --- /dev/null +++ b/internal/pty/manager.go @@ -0,0 +1,129 @@ +package pty + +import ( + "fmt" + "strconv" + "sync" +) + +// Manager owns the live PTY sessions. m6t holds one for the whole +// application: PTYs are backend-owned and survive project-tab switches +// (DESIGN.md §3.2), so their lifetime is the app's, not any window's. +// +// A Manager is safe for concurrent use. +type Manager struct { + mu sync.Mutex + sessions map[SessionID]*session + lastID uint64 +} + +// New returns a Manager holding no sessions. +func New() *Manager { + return &Manager{sessions: make(map[SessionID]*session)} +} + +// Create starts a session and returns its identifier. The session runs until +// its child exits, and stays registered after that until Kill or Shutdown +// drops it. +func (m *Manager) Create(opts Options) (SessionID, error) { + s, err := start(opts) + if err != nil { + return "", err + } + + m.mu.Lock() + m.lastID++ + id := SessionID("pty-" + strconv.FormatUint(m.lastID, 10)) + m.sessions[id] = s + m.mu.Unlock() + + go s.run() + + return id, nil +} + +// Attach returns a consumer's view of a session: its scrollback, its +// subsequent output and its exit status. +func (m *Manager) Attach(id SessionID) (Attachment, error) { + s, err := m.lookup(id) + if err != nil { + return Attachment{}, err + } + return s.attach(), nil +} + +// Write sends input to a session's child. +func (m *Manager) Write(id SessionID, p []byte) error { + s, err := m.lookup(id) + if err != nil { + return err + } + return s.write(p) +} + +// Resize changes the window size a session's child sees. A zero dimension is +// replaced by the default rather than passed through, because a terminal of +// zero columns is not a size any child can render at. +func (m *Manager) Resize(id SessionID, cols, rows uint16) error { + s, err := m.lookup(id) + if err != nil { + return err + } + width, height := size(cols, rows) + return s.resize(width, height) +} + +// Kill terminates a session's child and forgets the session. It returns once +// the child has been reaped. Killing an already-exited session is valid and +// simply drops it. +func (m *Manager) Kill(id SessionID) error { + m.mu.Lock() + s, ok := m.sessions[id] + delete(m.sessions, id) + m.mu.Unlock() + + if !ok { + return fmt.Errorf("session %s: %w", id, ErrNoSuchSession) + } + s.kill() + return nil +} + +// Shutdown terminates every session and returns once they have all been +// reaped. It is what the application calls on the way out, and the reason +// closing m6t does not leave orphaned shells behind. +// +// Sessions are killed concurrently: each one may wait out killGrace, and +// serializing that would multiply the app's shutdown time by the number of +// open terminals. +func (m *Manager) Shutdown() { + m.mu.Lock() + live := make([]*session, 0, len(m.sessions)) + for id, s := range m.sessions { + live = append(live, s) + delete(m.sessions, id) + } + m.mu.Unlock() + + var wg sync.WaitGroup + wg.Add(len(live)) + for _, s := range live { + go func() { + defer wg.Done() + s.kill() + }() + } + wg.Wait() +} + +// lookup resolves an identifier to its session. +func (m *Manager) lookup(id SessionID) (*session, error) { + m.mu.Lock() + defer m.mu.Unlock() + + s, ok := m.sessions[id] + if !ok { + return nil, fmt.Errorf("session %s: %w", id, ErrNoSuchSession) + } + return s, nil +} diff --git a/internal/pty/pty.go b/internal/pty/pty.go new file mode 100644 index 0000000..de1ecc1 --- /dev/null +++ b/internal/pty/pty.go @@ -0,0 +1,173 @@ +// Package pty owns the pseudo-terminal sessions behind m6t's embedded +// terminal (DESIGN.md §3.2, §8). It spawns the user's real login shell +// attached to a real PTY, keeps a bounded scrollback per session, and hands +// output to whichever consumers are attached. +// +// The package knows nothing about transports, terminal tabs or the Wails +// bridge. It takes an argv slice and a window size and returns an identifier; +// the loopback stream server and the UI are built on top of that seam rather +// than inside it, which is what lets either change without touching process +// lifecycle. +// +// Ownership contract: a session outlives its child process. When the child +// exits, the session records the status, notifies attached consumers and stops +// producing output, but stays registered so a consumer can still attach and +// replay the final scrollback — the "your shell exited" state a terminal tab +// shows. The owner drops the session with Kill, and Shutdown drops all of +// them. Nothing else removes a session from a Manager. +package pty + +import ( + "errors" + "os" + "time" +) + +const ( + // defaultCols and defaultRows size a session whose caller did not say. + // They are the classic terminal geometry, chosen because a shell that + // starts at 0x0 renders its prompt at the wrong width until the first + // resize arrives. + defaultCols = 80 + defaultRows = 24 + + // scrollbackBytes bounds the per-session replay buffer. It is what a + // consumer attaching late receives, and it is the reason a session with + // nobody reading it cannot grow without limit. + scrollbackBytes = 256 << 10 + + // readChunkBytes sizes one read from the PTY master. Large enough that a + // screenful of output arrives in one chunk, small enough that the first + // byte of a prompt is not held back waiting for a full buffer. + readChunkBytes = 32 << 10 + + // consumerQueue is how many chunks a single consumer may fall behind + // before its chunks start being dropped. Dropping is deliberate: the + // alternative is back-pressure onto the shell, which would let a stalled + // WebSocket freeze the user's terminal. + consumerQueue = 64 + + // killGrace is how long a child gets between the hangup and the SIGKILL. + // It is the window in which a shell runs its exit traps and flushes; a + // shell that ignores the hangup does not get to hold up app shutdown. + killGrace = 2 * time.Second + + // drainGrace bounds the wait for the output pump to see EOF after the + // child exits. Normally EOF arrives immediately; a grandchild still + // holding the slave end open is what this timeout exists for. + drainGrace = 250 * time.Millisecond + + // terminalType is the TERM the child sees. xterm.js implements the + // xterm-256color capabilities (DESIGN.md §8), so claiming anything else + // would misreport what the frontend can render. + terminalType = "xterm-256color" + + // windowsGOOS is the runtime.GOOS value for Windows. + windowsGOOS = "windows" +) + +// ErrNoSuchSession reports an operation naming a session the Manager does not +// hold — one never created, or one already killed. +var ErrNoSuchSession = errors.New("no such pty session") + +// SessionID identifies one session within a Manager. It is opaque: callers +// pass it back, they do not parse it. +type SessionID string + +// Options describes a session to create. The zero value is valid and starts +// the user's login shell at the default size in the process's own working +// directory. +type Options struct { + // Cwd is the child's working directory. Empty means inherit the + // application's. + Cwd string + + // Env holds additional environment entries in "KEY=value" form. They are + // appended to the application's own environment, so a caller can override + // an inherited variable by naming it again. + Env []string + + // Cols and Rows are the initial window size. Zero means the default. + Cols uint16 + Rows uint16 + + // Command is the argv to run. Empty means the user's login shell, which + // is what a terminal tab wants; the project's "run Claude Code" action + // (DESIGN.md §6) is what passes an explicit argv. + Command []string +} + +// Exit reports how a session's child process ended. +type Exit struct { + // Code is the child's exit status, or -1 when it was terminated by a + // signal rather than exiting on its own. -1 is what a killed session + // reports on every platform, which is why the signal number is not + // carried here: it has no meaning on Windows. + Code int +} + +// Attachment is one consumer's view of a session: what the session has already +// produced, what it produces next, and how it ended. +type Attachment struct { + // Replay is the scrollback at the moment of attaching — up to + // scrollbackBytes of the most recent output. It is a fresh copy the + // consumer owns. + Replay []byte + + // Chunks carries output produced after the attach. It is closed when the + // child exits. + // + // A consumer that stops reading does not stall the child: once it is + // consumerQueue chunks behind, further chunks are dropped rather than + // queued. Dropping loses bytes in the middle of the stream, so a consumer + // that cares about rendering fidelity — a terminal does — should treat a + // resumed read as a reason to redraw from a fresh Attach rather than + // assume continuity. + // + // Chunks are read-only and shared: every consumer of the same session + // receives the same backing array for a given chunk. Modifying one would + // corrupt what the others see. + Chunks <-chan []byte + + // Exited receives the exit status exactly once and is then closed. + // Attaching to an already-exited session yields a channel that already + // holds the status, so a late consumer never blocks here. + Exited <-chan Exit +} + +// shellFor returns the argv of the user's login shell for the named platform. +// +// The environment lookup and GOOS are parameters rather than reads of the +// ambient process so this decision is testable for every platform from any +// platform — the alternative is a rule that only the machine running the test +// ever exercises. +func shellFor(goos string, lookupEnv func(string) string) []string { + if goos == windowsGOOS { + return []string{"powershell.exe"} + } + if shell := lookupEnv("SHELL"); shell != "" { + return []string{shell} + } + return []string{"/bin/sh"} +} + +// environ builds the child's environment: everything the application has, then +// TERM, then the caller's additions. Later entries win, so a caller can +// override TERM and TERM overrides an inherited one. +func environ(extra []string) []string { + env := os.Environ() + env = append(env, "TERM="+terminalType) + return append(env, extra...) +} + +// size resolves the requested window size, substituting the default for a +// dimension the caller left at zero. +func size(cols, rows uint16) (width, height uint16) { + if cols == 0 { + cols = defaultCols + } + if rows == 0 { + rows = defaultRows + } + return cols, rows +} diff --git a/internal/pty/pty_test.go b/internal/pty/pty_test.go new file mode 100644 index 0000000..85a9a7b --- /dev/null +++ b/internal/pty/pty_test.go @@ -0,0 +1,683 @@ +package pty + +import ( + "errors" + "path/filepath" + "runtime" + "slices" + "strings" + "testing" + "time" + + xpty "github.com/aymanbagabas/go-pty" +) + +// readTimeout bounds every wait on a real child process. Generous, because +// these tests spawn shells on shared CI runners; a wait that hits it is a +// genuine failure, not a slow machine. +const readTimeout = 20 * time.Second + +// shell returns an argv running the given script through the platform's shell. +// The two scripts must do the same thing; they are separate because cmd.exe is +// not sh. +func shell(unixScript, windowsScript string) []string { + if runtime.GOOS == windowsGOOS { + return []string{"cmd.exe", "/c", windowsScript} + } + return []string{"/bin/sh", "-c", unixScript} +} + +// interactiveShell returns an argv for a bare shell reading from the terminal. +func interactiveShell() []string { + if runtime.GOOS == windowsGOOS { + return []string{"cmd.exe"} + } + return []string{"/bin/sh"} +} + +// requireSession returns the live session behind an ID. Tests reach past the +// Manager deliberately: waiting on the child directly is what lets them assert +// on lifecycle without an attached consumer changing the thing being measured. +func requireSession(t *testing.T, m *Manager, id SessionID) *session { + t.Helper() + s, err := m.lookup(id) + if err != nil { + t.Fatalf("looking up %s: %v", id, err) + } + return s +} + +// waitForExit blocks until the session's child has been reaped. +func waitForExit(t *testing.T, s *session) { + t.Helper() + select { + case <-s.done: + case <-time.After(readTimeout): + t.Fatal("timed out waiting for the child to exit") + } +} + +// readUntil collects output from an attachment until it contains want. +func readUntil(t *testing.T, a Attachment, want string) { + t.Helper() + var seen strings.Builder + seen.Write(a.Replay) + if strings.Contains(seen.String(), want) { + return + } + + deadline := time.After(readTimeout) + for { + select { + case chunk, open := <-a.Chunks: + if !open { + t.Fatalf("the session ended before producing %q; output was %q", want, seen.String()) + } + seen.Write(chunk) + if strings.Contains(seen.String(), want) { + return + } + case <-deadline: + t.Fatalf("timed out waiting for %q; output was %q", want, seen.String()) + } + } +} + +// awaitExit returns the exit status an attachment reports. +func awaitExit(t *testing.T, a Attachment) Exit { + t.Helper() + select { + case e := <-a.Exited: + return e + case <-time.After(readTimeout): + t.Fatal("timed out waiting for the exit event") + return Exit{} + } +} + +// The acceptance path from the issue: run a command, read its output, resize, +// kill, observe the exit event, and leave no unreaped process behind. +func TestSessionStreamsOutputResizesAndDiesWhenKilled(t *testing.T) { + m := New() + id, err := m.Create(Options{ + Command: shell("echo hello; sleep 60", "echo hello & ping -n 61 127.0.0.1 >NUL"), + Cols: 100, + Rows: 40, + }) + if err != nil { + t.Fatalf("Create: %v", err) + } + + s := requireSession(t, m, id) + attached, err := m.Attach(id) + if err != nil { + t.Fatalf("Attach: %v", err) + } + + readUntil(t, attached, "hello") + + if err := m.Resize(id, 120, 50); err != nil { + t.Errorf("Resize: %v", err) + } + + if err := m.Kill(id); err != nil { + t.Fatalf("Kill: %v", err) + } + + // Kill returns only once the child is reaped: a populated ProcessState is + // the proof there is no zombie left behind it. + if s.cmd.ProcessState == nil { + t.Error("Kill returned before the child was reaped; the process was left unwaited") + } + if got := awaitExit(t, attached); got.Code == 0 { + t.Errorf("exit code = 0 after a kill, want a non-zero or signaled status") + } + if _, err := m.Attach(id); !errors.Is(err, ErrNoSuchSession) { + t.Errorf("Attach after Kill: error = %v, want ErrNoSuchSession", err) + } +} + +func TestExitStatusCarriesTheChildsExitCode(t *testing.T) { + m := New() + id, err := m.Create(Options{Command: shell("exit 7", "exit 7")}) + if err != nil { + t.Fatalf("Create: %v", err) + } + t.Cleanup(func() { _ = m.Kill(id) }) + + attached, err := m.Attach(id) + if err != nil { + t.Fatalf("Attach: %v", err) + } + + if got := awaitExit(t, attached); got.Code != 7 { + t.Errorf("exit = %+v, want Code 7", got) + } +} + +// The child must never be throttled by the absence of a reader. This runs a +// command that produces more output than the scrollback holds with nothing +// attached at all, and checks it ran to completion anyway. +func TestChildRunsToCompletionWithNoConsumerAttached(t *testing.T) { + if runtime.GOOS == windowsGOOS { + t.Skip("the bulk-output script is POSIX sh; the Windows path is covered by the spawn/echo/kill tests") + } + + // 400 lines of ~1KB each: comfortably more than scrollbackBytes, and + // built without seq/awk so it runs on any POSIX shell. + const marker = "BULK-OUTPUT-COMPLETE" + script := "s=" + strings.Repeat("x", 64) + "; s=$s$s$s$s; s=$s$s$s$s; " + + "i=0; while [ $i -lt 400 ]; do echo \"$s\"; i=$((i+1)); done; echo " + marker + + m := New() + id, err := m.Create(Options{Command: shell(script, "")}) + if err != nil { + t.Fatalf("Create: %v", err) + } + t.Cleanup(func() { _ = m.Kill(id) }) + + // Nothing is attached while the child runs; wait on the child itself. + waitForExit(t, requireSession(t, m, id)) + + attached, err := m.Attach(id) + if err != nil { + t.Fatalf("Attach after exit: %v", err) + } + replay := string(attached.Replay) + + if !strings.Contains(replay, marker) { + t.Error("the child did not reach the end of its output; it was blocked by having no reader") + } + if len(attached.Replay) != scrollbackBytes { + t.Errorf("scrollback holds %d bytes, want it filled to %d", len(attached.Replay), scrollbackBytes) + } + if got := awaitExit(t, attached); got.Code != 0 { + t.Errorf("exit = %+v, want Code 0", got) + } +} + +// A consumer that stops reading must be dropped from, not block the child. +func TestSlowConsumerDoesNotStallTheChild(t *testing.T) { + if runtime.GOOS == windowsGOOS { + t.Skip("the bulk-output script is POSIX sh; the Windows path is covered by the spawn/echo/kill tests") + } + + script := "s=" + strings.Repeat("x", 64) + "; s=$s$s$s$s; s=$s$s$s$s; " + + "i=0; while [ $i -lt 400 ]; do echo \"$s\"; i=$((i+1)); done" + + m := New() + id, err := m.Create(Options{Command: shell(script, "")}) + if err != nil { + t.Fatalf("Create: %v", err) + } + t.Cleanup(func() { _ = m.Kill(id) }) + + // Attached and deliberately never read: the queue fills, chunks are + // dropped, and the child still has to finish. + if _, err := m.Attach(id); err != nil { + t.Fatalf("Attach: %v", err) + } + + waitForExit(t, requireSession(t, m, id)) +} + +func TestWriteReachesTheChild(t *testing.T) { + m := New() + // The shell echoes what is typed, so the command text must not contain + // the answer — otherwise the echo alone would satisfy the assertion. + id, err := m.Create(Options{Command: interactiveShell()}) + if err != nil { + t.Fatalf("Create: %v", err) + } + t.Cleanup(func() { _ = m.Kill(id) }) + + attached, err := m.Attach(id) + if err != nil { + t.Fatalf("Attach: %v", err) + } + + input := "expr 6 \\* 7\n" + if runtime.GOOS == windowsGOOS { + input = "set /a 6*7\r\n" + } + if err := m.Write(id, []byte(input)); err != nil { + t.Fatalf("Write: %v", err) + } + + readUntil(t, attached, "42") +} + +func TestAttachAfterExitReplaysScrollbackAndReportsTheStatus(t *testing.T) { + m := New() + id, err := m.Create(Options{Command: shell("echo persisted; exit 3", "echo persisted & exit 3")}) + if err != nil { + t.Fatalf("Create: %v", err) + } + t.Cleanup(func() { _ = m.Kill(id) }) + + waitForExit(t, requireSession(t, m, id)) + + attached, err := m.Attach(id) + if err != nil { + t.Fatalf("Attach after exit: %v", err) + } + if !strings.Contains(string(attached.Replay), "persisted") { + t.Errorf("replay = %q, want it to contain the output produced before the exit", attached.Replay) + } + // A late consumer must not hang waiting for output that will never come. + if _, open := <-attached.Chunks; open { + t.Error("Chunks delivered a value on an exited session; it must be closed") + } + if got := awaitExit(t, attached); got.Code != 3 { + t.Errorf("exit = %+v, want Code 3", got) + } +} + +func TestUnknownSessionIsRejectedByEveryMethod(t *testing.T) { + m := New() + const missing SessionID = "pty-does-not-exist" + + if _, err := m.Attach(missing); !errors.Is(err, ErrNoSuchSession) { + t.Errorf("Attach: error = %v, want ErrNoSuchSession", err) + } + if err := m.Write(missing, []byte("x")); !errors.Is(err, ErrNoSuchSession) { + t.Errorf("Write: error = %v, want ErrNoSuchSession", err) + } + if err := m.Resize(missing, 80, 24); !errors.Is(err, ErrNoSuchSession) { + t.Errorf("Resize: error = %v, want ErrNoSuchSession", err) + } + if err := m.Kill(missing); !errors.Is(err, ErrNoSuchSession) { + t.Errorf("Kill: error = %v, want ErrNoSuchSession", err) + } +} + +func TestCreateFailsWhenTheCommandCannotStart(t *testing.T) { + m := New() + + id, err := m.Create(Options{Command: []string{"m6t-no-such-binary-exists-here"}}) + if err == nil { + t.Fatalf("Create succeeded with id %q for a binary that does not exist", id) + } + if id != "" { + t.Errorf("Create returned id %q alongside an error, want the empty id", id) + } + // A failed start must leave nothing registered. + if len(m.sessions) != 0 { + t.Errorf("Manager holds %d sessions after a failed Create, want 0", len(m.sessions)) + } +} + +func TestCreateRunsTheChildInTheRequestedDirectory(t *testing.T) { + dir := t.TempDir() + // macOS reports /var as a symlink to /private/var, so compare against the + // resolved path rather than the one t.TempDir handed back. + resolved, err := filepath.EvalSymlinks(dir) + if err != nil { + t.Fatalf("resolving %s: %v", dir, err) + } + + m := New() + id, err := m.Create(Options{Cwd: dir, Command: shell("pwd", "cd")}) + if err != nil { + t.Fatalf("Create: %v", err) + } + t.Cleanup(func() { _ = m.Kill(id) }) + + attached, err := m.Attach(id) + if err != nil { + t.Fatalf("Attach: %v", err) + } + + readUntil(t, attached, filepath.Base(resolved)) +} + +// A bare command name must resolve against PATH even when the session has a +// working directory, because every project terminal has one and the Windows +// default shell is the bare name "powershell.exe". +// +// This is a regression test. go-pty resolves a bare name relative to Cmd.Dir +// on Windows, so before start() called exec.LookPath, creating a session with +// a Cwd looked for the shell inside the project directory and failed with +// "file does not exist" — meaning no terminal could ever open on Windows. +func TestCreateResolvesABareCommandNameAgainstPathNotTheWorkingDirectory(t *testing.T) { + bare := "sh" + if runtime.GOOS == windowsGOOS { + bare = "cmd.exe" + } + + m := New() + // An empty directory: anything resolved relative to it cannot exist. + id, err := m.Create(Options{Cwd: t.TempDir(), Command: []string{bare}}) + if err != nil { + t.Fatalf("Create with a bare command name and a Cwd: %v", err) + } + t.Cleanup(func() { _ = m.Kill(id) }) + + if _, err := m.Attach(id); err != nil { + t.Fatalf("Attach: %v", err) + } +} + +func TestCreateReportsAShellThatIsNotOnPath(t *testing.T) { + m := New() + + _, err := m.Create(Options{Command: []string{"m6t-definitely-not-on-path"}}) + if err == nil { + t.Fatal("Create succeeded for a binary that is not on PATH") + } + if !strings.Contains(err.Error(), "m6t-definitely-not-on-path") { + t.Errorf("error %q does not name the binary that could not be found", err) + } +} + +func TestCreateExportsTheCallersEnvironment(t *testing.T) { + m := New() + id, err := m.Create(Options{ + Env: []string{"M6T_TEST_VAR=session-scoped-value"}, + Command: shell("echo $M6T_TEST_VAR", "echo %M6T_TEST_VAR%"), + }) + if err != nil { + t.Fatalf("Create: %v", err) + } + t.Cleanup(func() { _ = m.Kill(id) }) + + attached, err := m.Attach(id) + if err != nil { + t.Fatalf("Attach: %v", err) + } + + readUntil(t, attached, "session-scoped-value") +} + +func TestShutdownKillsEverySession(t *testing.T) { + m := New() + stayAlive := shell("sleep 60", "ping -n 61 127.0.0.1 >NUL") + + sessions := make([]*session, 0, 3) + ids := make([]SessionID, 0, 3) + for range 3 { + id, err := m.Create(Options{Command: stayAlive}) + if err != nil { + t.Fatalf("Create: %v", err) + } + ids = append(ids, id) + sessions = append(sessions, requireSession(t, m, id)) + } + + m.Shutdown() + + if len(m.sessions) != 0 { + t.Errorf("Manager holds %d sessions after Shutdown, want 0", len(m.sessions)) + } + for i, s := range sessions { + if s.cmd.ProcessState == nil { + t.Errorf("session %s was not reaped by Shutdown", ids[i]) + } + } +} + +// The hangup is a request; SIGKILL is what makes it a guarantee. A shell that +// traps SIGHUP — which a user's rc file is free to do — must not be able to +// hold the app open past killGrace. +func TestKillEscalatesToSigkillWhenTheChildIgnoresTheHangup(t *testing.T) { + if runtime.GOOS == windowsGOOS { + t.Skip("Windows has no SIGHUP to ignore; hangup terminates the child outright") + } + + const trapReady = "TRAP-INSTALLED" + + m := New() + // The trailing echo stops sh exec-ing into sleep, so the process holding + // the ignored disposition is the shell itself. + id, err := m.Create(Options{ + Command: shell(`trap "" HUP; echo `+trapReady+`; sleep 60; echo done`, ""), + }) + if err != nil { + t.Fatalf("Create: %v", err) + } + s := requireSession(t, m, id) + + // Kill only once the trap is actually installed. Signaling earlier races + // the shell's own startup, and SIGHUP at default disposition kills it + // instantly — which would pass a test of the escalation path without ever + // exercising it. + attached, err := m.Attach(id) + if err != nil { + t.Fatalf("Attach: %v", err) + } + readUntil(t, attached, trapReady) + + start := time.Now() + if err := m.Kill(id); err != nil { + t.Fatalf("Kill: %v", err) + } + elapsed := time.Since(start) + + if s.cmd.ProcessState == nil { + t.Fatal("Kill returned before the child was reaped") + } + if elapsed < killGrace { + t.Errorf("Kill returned after %v, before the %v grace elapsed; the child was never given "+ + "its chance to exit cleanly", elapsed, killGrace) + } + if elapsed > killGrace+readTimeout { + t.Errorf("Kill took %v; a child ignoring the hangup must still die at the grace deadline", elapsed) + } +} + +// Once a session's PTY is closed, input and resize must report that rather +// than silently succeeding against a dead terminal. +func TestWriteAndResizeFailOnceTheSessionHasBeenKilled(t *testing.T) { + m := New() + id, err := m.Create(Options{Command: shell("sleep 60", "ping -n 61 127.0.0.1 >NUL")}) + if err != nil { + t.Fatalf("Create: %v", err) + } + + s := requireSession(t, m, id) + if err := m.Kill(id); err != nil { + t.Fatalf("Kill: %v", err) + } + + if err := s.write([]byte("input\n")); err == nil { + t.Error("write to a killed session succeeded; it must report the closed terminal") + } + if err := s.resize(100, 40); err == nil { + t.Error("resize of a killed session succeeded; it must report the closed terminal") + } +} + +func TestShutdownOnAnEmptyManagerIsSafe(t *testing.T) { + m := New() + m.Shutdown() + + // Shutdown must leave the Manager usable rather than in a state where the + // session map has been nilled out from under the next Create. + if _, err := m.Create(Options{Command: shell("exit 0", "exit 0")}); err != nil { + t.Errorf("Create after Shutdown on an empty Manager: %v", err) + } +} + +func TestSessionIDsAreUnique(t *testing.T) { + m := New() + t.Cleanup(m.Shutdown) + + seen := map[SessionID]bool{} + for range 3 { + id, err := m.Create(Options{Command: shell("exit 0", "exit 0")}) + if err != nil { + t.Fatalf("Create: %v", err) + } + if seen[id] { + t.Fatalf("Create returned the already-issued id %q", id) + } + seen[id] = true + } +} + +func TestShellForPicksTheUsersLoginShell(t *testing.T) { + tests := []struct { + name string + goos string + env map[string]string + want []string + }{ + { + name: "unix uses SHELL when it is set", + goos: "darwin", + env: map[string]string{"SHELL": "/usr/bin/fish"}, + want: []string{"/usr/bin/fish"}, + }, + { + name: "unix falls back to /bin/sh when SHELL is unset", + goos: "linux", + env: map[string]string{}, + want: []string{"/bin/sh"}, + }, + { + name: "unix falls back when SHELL is set but empty", + goos: "linux", + env: map[string]string{"SHELL": ""}, + want: []string{"/bin/sh"}, + }, + { + name: "windows uses PowerShell and ignores SHELL", + goos: windowsGOOS, + env: map[string]string{"SHELL": "/usr/bin/fish"}, + want: []string{"powershell.exe"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := shellFor(tt.goos, func(k string) string { return tt.env[k] }) + if strings.Join(got, " ") != strings.Join(tt.want, " ") { + t.Errorf("shellFor(%q) = %v, want %v", tt.goos, got, tt.want) + } + }) + } +} + +func TestEnvironInheritsTheProcessEnvironmentAndSetsTerm(t *testing.T) { + t.Setenv("M6T_INHERITED_MARKER", "inherited") + + env := environ([]string{"EXTRA=added"}) + + want := map[string]string{ + "M6T_INHERITED_MARKER": "inherited", + "TERM": terminalType, + "EXTRA": "added", + } + for key, value := range want { + if !slices.Contains(env, key+"="+value) { + t.Errorf("environ() is missing %s=%s; got %v", key, value, env) + } + } +} + +// A caller's own entry must win over the inherited one, which is what makes +// Options.Env an override rather than a suggestion. +func TestEnvironLetsTheCallerOverrideAnInheritedVariable(t *testing.T) { + t.Setenv("M6T_OVERRIDDEN", "from-process") + + env := environ([]string{"M6T_OVERRIDDEN=from-caller"}) + + last := "" + for _, entry := range env { + if strings.HasPrefix(entry, "M6T_OVERRIDDEN=") { + last = entry + } + } + if last != "M6T_OVERRIDDEN=from-caller" { + t.Errorf("the last M6T_OVERRIDDEN entry is %q, want the caller's value to win", last) + } +} + +func TestEnvironOverridesTermForTheCaller(t *testing.T) { + t.Setenv("TERM", "dumb") + + env := environ(nil) + + last := "" + for _, entry := range env { + if strings.HasPrefix(entry, "TERM=") { + last = entry + } + } + if last != "TERM="+terminalType { + t.Errorf("the last TERM entry is %q, want TERM=%s", last, terminalType) + } +} + +func TestSizeSubstitutesDefaultsForZeroDimensions(t *testing.T) { + tests := []struct { + name string + cols, rows uint16 + wantCols, wantRows uint16 + }{ + {"both given", 120, 50, 120, 50}, + {"both zero", 0, 0, defaultCols, defaultRows}, + {"zero columns only", 0, 50, defaultCols, 50}, + {"zero rows only", 120, 0, 120, defaultRows}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotCols, gotRows := size(tt.cols, tt.rows) + if gotCols != tt.wantCols || gotRows != tt.wantRows { + t.Errorf("size(%d, %d) = (%d, %d), want (%d, %d)", + tt.cols, tt.rows, gotCols, gotRows, tt.wantCols, tt.wantRows) + } + }) + } +} + +func TestResizeRejectsNothingAndAppliesDefaults(t *testing.T) { + m := New() + id, err := m.Create(Options{Command: shell("sleep 60", "ping -n 61 127.0.0.1 >NUL")}) + if err != nil { + t.Fatalf("Create: %v", err) + } + t.Cleanup(func() { _ = m.Kill(id) }) + + // A zero dimension must not reach the PTY, which cannot render at 0. + if err := m.Resize(id, 0, 0); err != nil { + t.Errorf("Resize with zero dimensions: %v", err) + } +} + +// A child that never ran leaves no ProcessState to read a code from. -1 is +// what Exit documents for "did not exit on its own", and the alternative — a +// zero-valued Exit — would report a clean exit for a session that failed. +func TestExitStatusReportsMinusOneWithoutAProcessState(t *testing.T) { + if got := exitStatus(&xpty.Cmd{}, errors.New("wait failed")); got.Code != -1 { + t.Errorf("exit = %+v, want Code -1 when the child produced no state", got) + } + if got := exitStatus(&xpty.Cmd{}, nil); got.Code != 0 { + t.Errorf("exit = %+v, want Code 0 when the wait succeeded with no state", got) + } +} + +func TestDefaultShellIsUsedWhenNoCommandIsGiven(t *testing.T) { + if runtime.GOOS == windowsGOOS { + t.Skip("PowerShell startup is too slow to gate this test on; the Unix path pins the behavior") + } + t.Setenv("SHELL", "/bin/sh") + + m := New() + id, err := m.Create(Options{}) + if err != nil { + t.Fatalf("Create with no command: %v", err) + } + t.Cleanup(func() { _ = m.Kill(id) }) + + attached, err := m.Attach(id) + if err != nil { + t.Fatalf("Attach: %v", err) + } + if err := m.Write(id, []byte("expr 6 \\* 7\n")); err != nil { + t.Fatalf("Write: %v", err) + } + + readUntil(t, attached, "42") +} diff --git a/internal/pty/ring.go b/internal/pty/ring.go new file mode 100644 index 0000000..c871e21 --- /dev/null +++ b/internal/pty/ring.go @@ -0,0 +1,57 @@ +package pty + +// ring is a fixed-capacity byte buffer holding the most recent bytes written +// to it. +// +// It is the mechanism behind the "a session survives having nobody reading it" +// rule. Writing never blocks and never fails: the alternative — an unbounded +// buffer — turns a terminal nobody is watching into a memory leak, and +// back-pressure onto the PTY turns it into a frozen shell. Dropping the oldest +// output is the only option that costs the user nothing they were still +// looking at. +// +// ring is not safe for concurrent use; the session owning it holds the lock. +type ring struct { + buf []byte + start int // index of the oldest byte held + size int // bytes currently held +} + +// newRing returns a ring holding at most capacity bytes. capacity must be +// positive. +func newRing(capacity int) *ring { + return &ring{buf: make([]byte, capacity)} +} + +// write appends p, evicting the oldest bytes when there is not enough room. +func (r *ring) write(p []byte) { + capacity := len(r.buf) + + // A single write larger than the whole buffer makes every byte already + // held unreachable, so only p's own tail survives. + if len(p) >= capacity { + copy(r.buf, p[len(p)-capacity:]) + r.start, r.size = 0, capacity + return + } + + end := (r.start + r.size) % capacity + n := copy(r.buf[end:], p) + copy(r.buf, p[n:]) + + if overflow := r.size + len(p) - capacity; overflow > 0 { + r.start = (r.start + overflow) % capacity + r.size = capacity + return + } + r.size += len(p) +} + +// snapshot returns a copy of the held bytes, oldest first. The caller owns the +// result; the ring keeps writing into its own storage. +func (r *ring) snapshot() []byte { + out := make([]byte, r.size) + n := copy(out, r.buf[r.start:min(r.start+r.size, len(r.buf))]) + copy(out[n:], r.buf[:r.size-n]) + return out +} diff --git a/internal/pty/ring_test.go b/internal/pty/ring_test.go new file mode 100644 index 0000000..dd5bf94 --- /dev/null +++ b/internal/pty/ring_test.go @@ -0,0 +1,115 @@ +package pty + +import ( + "bytes" + "strings" + "testing" +) + +func TestRingHoldsWhatFitsInOrder(t *testing.T) { + r := newRing(8) + r.write([]byte("abc")) + r.write([]byte("de")) + + if got := string(r.snapshot()); got != "abcde" { + t.Errorf("snapshot = %q, want %q", got, "abcde") + } +} + +func TestRingEvictsOldestOnceFull(t *testing.T) { + r := newRing(4) + r.write([]byte("abcd")) + r.write([]byte("ef")) + + // "ab" is the oldest and must be the part that went. + if got := string(r.snapshot()); got != "cdef" { + t.Errorf("snapshot = %q, want %q", got, "cdef") + } +} + +func TestRingWrapsAroundTheEndOfItsStorage(t *testing.T) { + r := newRing(5) + r.write([]byte("abcd")) + r.write([]byte("efg")) // wraps past the end of the backing array + + if got := string(r.snapshot()); got != "cdefg" { + t.Errorf("snapshot = %q, want %q", got, "cdefg") + } +} + +func TestRingKeepsOnlyTheTailOfAnOversizedWrite(t *testing.T) { + r := newRing(4) + r.write([]byte("xy")) + r.write([]byte("abcdefgh")) + + if got := string(r.snapshot()); got != "efgh" { + t.Errorf("snapshot = %q, want %q", got, "efgh") + } +} + +func TestRingSnapshotDoesNotAliasTheBuffer(t *testing.T) { + r := newRing(4) + r.write([]byte("abcd")) + + snap := r.snapshot() + r.write([]byte("wxyz")) + + if got := string(snap); got != "abcd" { + t.Errorf("snapshot changed to %q after a later write; it must be a copy", got) + } +} + +func TestRingNeverExceedsItsCapacity(t *testing.T) { + const capacity = 16 + r := newRing(capacity) + for range 100 { + r.write([]byte("0123456789")) + } + + snap := r.snapshot() + if len(snap) != capacity { + t.Errorf("snapshot is %d bytes, want the capacity %d", len(snap), capacity) + } + // 1000 bytes of a repeating 10-byte cycle: the last 16 end "...6789". + if !bytes.HasSuffix(snap, []byte("6789")) { + t.Errorf("snapshot %q does not end with the most recent bytes", snap) + } +} + +func TestRingEmptySnapshotIsEmpty(t *testing.T) { + if got := newRing(8).snapshot(); len(got) != 0 { + t.Errorf("a ring nothing was written to snapshots as %q, want empty", got) + } +} + +// The scrollback is what a late consumer replays, so its capacity is a product +// decision (DESIGN.md §8), not an implementation detail free to drift. +func TestScrollbackIsTheDocumentedSize(t *testing.T) { + if want := 256 * 1024; scrollbackBytes != want { + t.Errorf("scrollbackBytes = %d, want %d", scrollbackBytes, want) + } +} + +func TestRingHandlesAWriteExactlyItsCapacity(t *testing.T) { + r := newRing(4) + r.write([]byte("ab")) + r.write([]byte("wxyz")) + + if got := string(r.snapshot()); got != "wxyz" { + t.Errorf("snapshot = %q, want %q", got, "wxyz") + } +} + +func TestRingAccumulatesAcrossManySmallWrites(t *testing.T) { + r := newRing(1024) + var want strings.Builder + for i := range 50 { + chunk := strings.Repeat(string(rune('a'+i%26)), 3) + r.write([]byte(chunk)) + want.WriteString(chunk) + } + + if got := string(r.snapshot()); got != want.String() { + t.Errorf("snapshot = %q, want %q", got, want.String()) + } +} diff --git a/internal/pty/session.go b/internal/pty/session.go new file mode 100644 index 0000000..ca312fd --- /dev/null +++ b/internal/pty/session.go @@ -0,0 +1,238 @@ +package pty + +import ( + "bytes" + "fmt" + "os" + "os/exec" + "runtime" + "sync" + "time" + + xpty "github.com/aymanbagabas/go-pty" +) + +// consumer is one attached reader's delivery queue. +type consumer struct { + chunks chan []byte + exited chan Exit +} + +// session is one running PTY and the child process attached to it. +type session struct { + pty xpty.Pty + cmd *xpty.Cmd + + // done closes when the child has been reaped and the exit status + // published. Waiting on it is how kill knows there is no process left. + done chan struct{} + + mu sync.Mutex + scrollback *ring + consumers []*consumer + exited bool + exit Exit +} + +// start opens a PTY and launches opts.Command in it, or the user's login shell +// when no command is given. The returned session is live but not yet pumping +// output; the caller starts run. +func start(opts Options) (*session, error) { + terminal, err := xpty.New() + if err != nil { + return nil, fmt.Errorf("opening pty: %w", err) + } + + // Size before starting so the child reads the right geometry from its + // first prompt onward rather than drawing once at the wrong width. + cols, rows := size(opts.Cols, opts.Rows) + if err := terminal.Resize(int(cols), int(rows)); err != nil { + _ = terminal.Close() + return nil, fmt.Errorf("sizing pty: %w", err) + } + + argv := opts.Command + if len(argv) == 0 { + argv = shellFor(runtime.GOOS, os.Getenv) + } + + // Resolve against PATH before handing the name over. go-pty resolves a + // bare name relative to Cmd.Dir on Windows, so a session started with a + // Cwd — which every project terminal is — would look for powershell.exe + // inside the project directory and fail. Resolving here also turns "the + // shell is not installed" into an error at Create rather than a session + // that exists and immediately dies. + binary, err := exec.LookPath(argv[0]) + if err != nil { + _ = terminal.Close() + return nil, fmt.Errorf("locating %q: %w", argv[0], err) + } + + cmd := terminal.Command(binary, argv[1:]...) + cmd.Dir = opts.Cwd + cmd.Env = environ(opts.Env) + if err := cmd.Start(); err != nil { + _ = terminal.Close() + return nil, fmt.Errorf("starting %q: %w", argv[0], err) + } + + return &session{ + pty: terminal, + cmd: cmd, + done: make(chan struct{}), + scrollback: newRing(scrollbackBytes), + }, nil +} + +// run pumps output until the child exits, then publishes the exit status. It +// owns the session's lifetime and returns only once the child is reaped. +func (s *session) run() { + defer close(s.done) + + drained := make(chan struct{}) + go func() { + defer close(drained) + s.pump() + }() + + waitErr := s.cmd.Wait() + + // The child is gone, but output it wrote before exiting may still be in + // the pipe. Give the pump a moment to see EOF on its own so that last + // screenful reaches consumers ahead of the exit event; close the master + // only if something downstream is holding the slave end open. + select { + case <-drained: + case <-time.After(drainGrace): + } + _ = s.pty.Close() + <-drained + + s.finish(exitStatus(s.cmd, waitErr)) +} + +// pump reads the PTY master until it fails, which is how a closed or +// hung-up terminal reports that there is nothing more to read. +func (s *session) pump() { + buf := make([]byte, readChunkBytes) + for { + n, err := s.pty.Read(buf) + if n > 0 { + s.broadcast(buf[:n]) + } + if err != nil { + return + } + } +} + +// broadcast records a chunk in the scrollback and offers it to every attached +// consumer. A consumer whose queue is full has the chunk dropped: the child +// must never wait on a reader. +func (s *session) broadcast(chunk []byte) { + s.mu.Lock() + defer s.mu.Unlock() + + s.scrollback.write(chunk) + if len(s.consumers) == 0 { + return + } + + // pump reuses its read buffer, so consumers get a copy. One copy is + // shared across them because nothing downstream may modify it. + out := bytes.Clone(chunk) + for _, c := range s.consumers { + select { + case c.chunks <- out: + default: + } + } +} + +// finish publishes the exit status to every attached consumer and marks the +// session as ended, so later attaches report the status instead of waiting for +// output that will never come. +func (s *session) finish(e Exit) { + s.mu.Lock() + defer s.mu.Unlock() + + s.exited, s.exit = true, e + for _, c := range s.consumers { + close(c.chunks) + c.exited <- e + close(c.exited) + } + s.consumers = nil +} + +// attach registers a consumer and returns its view of the session. The +// scrollback snapshot and the registration happen under one lock, so no chunk +// is both replayed and delivered, and none is missed between the two. +func (s *session) attach() Attachment { + s.mu.Lock() + defer s.mu.Unlock() + + c := &consumer{ + chunks: make(chan []byte, consumerQueue), + exited: make(chan Exit, 1), + } + if s.exited { + close(c.chunks) + c.exited <- s.exit + close(c.exited) + } else { + s.consumers = append(s.consumers, c) + } + + return Attachment{Replay: s.scrollback.snapshot(), Chunks: c.chunks, Exited: c.exited} +} + +// write sends input to the child. +func (s *session) write(p []byte) error { + if _, err := s.pty.Write(p); err != nil { + return fmt.Errorf("writing to pty: %w", err) + } + return nil +} + +// resize changes the window size the child sees. +func (s *session) resize(cols, rows uint16) error { + if err := s.pty.Resize(int(cols), int(rows)); err != nil { + return fmt.Errorf("resizing pty: %w", err) + } + return nil +} + +// kill terminates the child and returns once it has been reaped, so a caller +// that returns has left no process behind. +// +// A hangup goes first: that is what closing a terminal window does, and it +// gives the shell its chance to run exit traps. A shell that is still alive +// after killGrace gets SIGKILL. Signal errors are not reported because the +// only ordinary cause is the child having already exited, which is the +// outcome being asked for. +func (s *session) kill() { + if proc := s.cmd.Process; proc != nil { + _ = hangup(proc) + select { + case <-s.done: + return + case <-time.After(killGrace): + } + _ = forceKill(proc) + } + <-s.done +} + +// exitStatus reads the child's status off the finished command. A child killed +// by a signal has no exit code of its own, and ExitCode reports -1 for it, +// which is the value Exit documents for that case. +func exitStatus(cmd *xpty.Cmd, waitErr error) Exit { + if cmd.ProcessState != nil { + return Exit{Code: cmd.ProcessState.ExitCode()} + } + if waitErr != nil { + return Exit{Code: -1} + } + return Exit{Code: 0} +} diff --git a/package_budget_test.go b/package_budget_test.go index 50875ca..3b9e0d6 100644 --- a/package_budget_test.go +++ b/package_budget_test.go @@ -50,6 +50,10 @@ var structuralPins = map[string]packagePin{ loc: 150, exported: 2, why: "link-time build identity; a dependency root importing nothing first-party", }, + "internal/pty": { + loc: 750, exported: 7, + why: "PTY service: session lifecycle, scrollback and platform termination for the embedded terminal", + }, } // locCeilingNote explains why the LOC ceilings carry headroom while every other @@ -58,19 +62,26 @@ var structuralPins = map[string]packagePin{ // A LOC ceiling set to a package's exact current line count is not a ratchet, // it is a freeze: one more line of doc comment in buildinfo.go would fail the // build. The number has to represent the size at which a package stops being -// readable in one sitting, and m6t cannot measure that yet — the backend -// services that will be the real packages (git, pty, kube, helm; DESIGN.md -// §3.2) land in #2 and #5. So these figures are seeded as policy: +// readable in one sitting. +// +// internal/pty is the first ceiling here set from a real measurement rather +// than policy. It landed with #2 at 644 lines across six files, and 750 is +// that plus room for the follow-up fixes a new service attracts — not enough +// room for a second service to move in alongside it. The stream server that +// consumes it (#3) is its own package, so this number should hold. +// +// The rest are still seeded as policy, because the services that would let +// them be measured (git, kube, helm; DESIGN.md §3.2) land in #5: // // - 200 for internal/app and 150 for buildinfo — roughly 3x and 2x today's // size, so ordinary work does not trip the gate while a package that // doubles again arrives in review as a decomposition question. // - 60 for the root: main.go does one thing and must keep doing only that. // -// Re-pin against real measurements once #2 and #5 land. No other ceiling here +// Re-pin those against real measurements once #5 lands. No other ceiling here // needs the caveat: counts of packages, files and exported names do not grow // through ordinary editing. -const locCeilingNote = "LOC ceilings are policy-seeded; re-pin after #2/#5 (see locCeilingNote)" +const locCeilingNote = "internal/pty is measured; the other LOC ceilings are policy-seeded pending #5 (see locCeilingNote)" // maxFilesPerPackage stops a package from escaping its LOC budget by fanning // the same code across many small files. diff --git a/package_graph_test.go b/package_graph_test.go index b6c1fe8..e9dd4ef 100644 --- a/package_graph_test.go +++ b/package_graph_test.go @@ -49,12 +49,21 @@ func TestNoDeadPackages(t *testing.T) { // reviewer gets to ask whether the new dependency belongs. func TestImportGraphIsPinned(t *testing.T) { // The composition root imports the binding layer; the binding layer reads - // build identity. buildinfo imports nothing first-party — it is a leaf, and - // depguard pins that independently. + // build identity and composes the backend services. buildinfo imports + // nothing first-party — it is a leaf, and depguard pins that + // independently. + // + // internal/app -> internal/pty is the first service edge (#2). It belongs + // because the binding layer is where services are composed: the App owns + // the Manager so that quitting the app can end the PTY sessions, which + // nothing else is positioned to do. The edge runs one way only — pty + // imports no first-party package at all, which is what keeps it usable + // from the stream server (#3) without dragging the Wails layer in. want := map[string][]string{ rootPackageDir: {"internal/app"}, - "internal/app": {"internal/buildinfo"}, + "internal/app": {"internal/buildinfo", "internal/pty"}, "internal/buildinfo": {}, + "internal/pty": {}, } graph := firstPartyImports(t)