fix: create a new pty for exec.Cmd

cmd.go

@@ -2,11 +2,8 @@ package wish
 
 import (
 	"context"
-	"fmt"
 	"io"
 	"os/exec"
-	"runtime"
-	"time"
 
 	tea "github.com/charmbracelet/bubbletea"
 	"github.com/charmbracelet/ssh"
@@ -18,14 +15,7 @@ import (
 // itself.
 func CommandContext(ctx context.Context, s ssh.Session, name string, args ...string) *Cmd {
 	cmd := exec.CommandContext(ctx, name, args...)
-	pty, _, ok := s.Pty()
-	if !ok {
-		cmd.Stdin, cmd.Stdout, cmd.Stderr = s, s, s
-		return &Cmd{cmd: cmd}
-	}
-
-	cmd.Env = append(cmd.Environ(), "SSH_TTY="+pty.Name(), fmt.Sprintf("TERM=%s", pty.Term))
-	return &Cmd{cmd, &pty}
+	return &Cmd{s, cmd}
 }
 
 // Command sets stdin, stdout, and stderr to the current session's PTY slave.
@@ -40,8 +30,8 @@ func Command(s ssh.Session, name string, args ...string) *Cmd {
 
 // Cmd wraps a *exec.Cmd and a ssh.Pty so a command can be properly run.
 type Cmd struct {
-	cmd *exec.Cmd
-	pty *ssh.Pty
+	sess ssh.Session
+	cmd  *exec.Cmd
 }
 
 // SetDir set the underlying exec.Cmd env.
@@ -61,29 +51,12 @@ func (c *Cmd) SetDir(dir string) {
 
 // Run runs the program and waits for it to finish.
 func (c *Cmd) Run() error {
-	if c.pty == nil {
+	ppty, winCh, ok := c.sess.Pty()
+	if !ok {
+		c.cmd.Stdin, c.cmd.Stdout, c.cmd.Stderr = c.sess, c.sess, c.sess
 		return c.cmd.Run()
 	}
-
-	if err := c.pty.Start(c.cmd); err != nil {
-		return err
-	}
-
-	if runtime.GOOS == "windows" {
-		start := time.Now()
-		for c.cmd.ProcessState == nil {
-			if time.Since(start) > time.Second*10 {
-				return fmt.Errorf("could not start process")
-			}
-			time.Sleep(100 * time.Millisecond)
-		}
-		if !c.cmd.ProcessState.Success() {
-			return fmt.Errorf("process failed: exit %d", c.cmd.ProcessState.ExitCode())
-		}
-		return nil
-	}
-
-	return c.cmd.Wait()
+	return c.doRun(ppty, winCh)
 }
 
 var _ tea.ExecCommand = &Cmd{}

cmd_darwin.go

@@ -0,0 +1,96 @@
+//go:build darwin
+// +build darwin
+
+package wish
+
+import (
+	"errors"
+	"io"
+
+	"github.com/charmbracelet/log"
+	"github.com/charmbracelet/ssh"
+	"github.com/creack/pty"
+	"github.com/muesli/cancelreader"
+	"golang.org/x/term"
+)
+
+// on macOS, the slave pty is killed once exec finishes.
+// since we're using it for the ssh session, this would render
+// the pty and the session unusable.
+// so, we need to create another pty, and run the Cmd on it instead.
+func (c *Cmd) doRun(ppty ssh.Pty, winCh <-chan ssh.Window) error {
+	done := make(chan struct{}, 1)
+	go func() {
+		<-done
+		close(done)
+	}()
+	ptmxClose := make(chan struct{}, 1)
+	ptmx, err := pty.Start(c.cmd)
+	if err != nil {
+		return err
+	}
+	defer func() {
+		if err := ptmx.Close(); err != nil {
+			log.Warn("could not close pty", "err", err)
+		}
+		ptmxClose <- struct{}{}
+		close(ptmxClose)
+	}()
+
+	// setup resizes
+	go func() {
+		for {
+			select {
+			case <-ptmxClose:
+				return
+			case w := <-winCh:
+				log.Infof("resize %d %d", w.Height, w.Width)
+				if err := pty.Setsize(ptmx, &pty.Winsize{
+					Rows: uint16(w.Height),
+					Cols: uint16(w.Width),
+				}); err != nil {
+					log.Warn("could not set term size", "err", err)
+				}
+			}
+		}
+	}()
+	if err := pty.InheritSize(ppty.Slave, ptmx); err != nil {
+		return err
+	}
+
+	// put the ssh session's pty in raw mode
+	oldState, err := term.MakeRaw(int(ppty.Slave.Fd()))
+	if err != nil {
+		return err
+	}
+	defer func() {
+		if err := term.Restore(int(ppty.Slave.Fd()), oldState); err != nil {
+			log.Error("could not restore terminal", "err", err)
+		}
+	}()
+
+	// we'll need to be able to cancel the reader, otherwise the copy
+	// from ptmx will eat the next keypress after the exec exits.
+	cancelSlave, err := cancelreader.NewReader(ppty.Slave)
+	if err != nil {
+		return err
+	}
+	defer func() { cancelSlave.Cancel() }()
+
+	// sync io
+	go func() {
+		defer func() { done <- struct{}{} }()
+		if _, err := io.Copy(ptmx, cancelSlave); err != nil {
+			if errors.Is(err, io.EOF) || errors.Is(err, cancelreader.ErrCanceled) {
+				// safe to ignore
+				return
+			}
+			log.Warn("failed to copy", "err", err)
+		}
+	}()
+	if _, err := io.Copy(ppty.Slave, ptmx); err != nil && !errors.Is(err, io.EOF) {
+		return err
+	}
+
+	return c.cmd.Wait()
+}

cmd_test.go

@@ -5,6 +5,7 @@ import (
 	"runtime"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/charmbracelet/ssh"
 	"github.com/charmbracelet/wish/testsession"
@@ -33,12 +34,18 @@ func TestCommandNoPty(t *testing.T) {
 }
 
 func TestCommandPty(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip()
+	}
 	tmp := t.TempDir()
 	srv := &ssh.Server{
 		Handler: func(s ssh.Session) {
 			runEcho(s, "hello")
 			runEnv(s, []string{"HELLO=world"})
 			runPwd(s, tmp)
+			// for some reason sometimes on macos github action runners,
+			// it cuts parts of the output.
+			time.Sleep(100 * time.Millisecond)
 		},
 	}
 	if err := ssh.AllocatePty()(srv); err != nil {
@@ -64,6 +71,9 @@ func TestCommandPty(t *testing.T) {
 }
 
 func TestCommandPtyError(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip()
+	}
 	srv := &ssh.Server{
 		Handler: func(s ssh.Session) {
 			if err := Command(s, "nopenopenope").Run(); err != nil {

cmd_unix.go

@@ -0,0 +1,13 @@
+//go:build dragonfly || freebsd || linux || netbsd || openbsd || solaris
+// +build dragonfly freebsd linux netbsd openbsd solaris
+
+package wish
+
+import "github.com/charmbracelet/ssh"
+
+func (c *Cmd) doRun(ppty ssh.Pty, _ <-chan ssh.Window) error {
+	if err := ppty.Start(c.cmd); err != nil {
+		return err
+	}
+	return c.cmd.Wait()
+}

cmd_windows.go

@@ -0,0 +1,29 @@
+//go:build windows
+// +build windows
+
+package wish
+
+import (
+	"fmt"
+	"time"
+
+	"github.com/charmbracelet/ssh"
+)
+
+func (c *Cmd) doRun(ppty ssh.Pty, _ <-chan ssh.Window) error {
+	if err := ppty.Start(c.cmd); err != nil {
+		return err
+	}
+
+	start := time.Now()
+	for c.cmd.ProcessState == nil {
+		if time.Since(start) > time.Second*10 {
+			return fmt.Errorf("could not start process")
+		}
+		time.Sleep(100 * time.Millisecond)
+	}
+	if !c.cmd.ProcessState.Success() {
+		return fmt.Errorf("process failed: exit %d", c.cmd.ProcessState.ExitCode())
+	}
+	return nil
+}

go.mod

@@ -8,13 +8,16 @@ require (
 	github.com/charmbracelet/lipgloss v0.9.1
 	github.com/charmbracelet/log v0.3.1
 	github.com/charmbracelet/ssh v0.0.0-20240118173142-6d7cf11c8371
+	github.com/creack/pty v1.1.21
 	github.com/go-git/go-git/v5 v5.11.0
 	github.com/google/go-cmp v0.6.0
 	github.com/hashicorp/golang-lru/v2 v2.0.7
 	github.com/matryer/is v1.4.1
+	github.com/muesli/cancelreader v0.2.2
 	github.com/muesli/termenv v0.15.2
 	golang.org/x/crypto v0.18.0
 	golang.org/x/sync v0.6.0
+	golang.org/x/term v0.16.0
 	golang.org/x/time v0.5.0
 )
 
@@ -28,7 +31,6 @@ require (
 	github.com/charmbracelet/x/exp/term v0.0.0-20240117031359-6e25c76a1efe // indirect
 	github.com/cloudflare/circl v1.3.7 // indirect
 	github.com/containerd/console v1.0.4-0.20230313162750-1ae8d489ac81 // indirect
-	github.com/creack/pty v1.1.21 // indirect
 	github.com/cyphar/filepath-securejoin v0.2.4 // indirect
 	github.com/emirpasic/gods v1.18.1 // indirect
 	github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect
@@ -42,7 +44,6 @@ require (
 	github.com/mattn/go-localereader v0.0.1 // indirect
 	github.com/mattn/go-runewidth v0.0.15 // indirect
 	github.com/muesli/ansi v0.0.0-20211018074035-2e021307bc4b // indirect
-	github.com/muesli/cancelreader v0.2.2 // indirect
 	github.com/muesli/reflow v0.3.0 // indirect
 	github.com/pjbgf/sha1cd v0.3.0 // indirect
 	github.com/rivo/uniseg v0.2.0 // indirect
@@ -54,7 +55,6 @@ require (
 	golang.org/x/mod v0.13.0 // indirect
 	golang.org/x/net v0.19.0 // indirect
 	golang.org/x/sys v0.16.0 // indirect
-	golang.org/x/term v0.16.0 // indirect
 	golang.org/x/text v0.14.0 // indirect
 	golang.org/x/tools v0.14.0 // indirect
 	gopkg.in/warnings.v0 v0.1.2 // indirect