Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ require (
github.com/bufbuild/buf v1.30.0
github.com/charmbracelet/huh v0.8.0
github.com/charmbracelet/huh/spinner v0.0.0-20240917123815-c9b2c9cdb7b6
github.com/charmbracelet/x/xpty v0.1.2
github.com/chenzhekl/goply v0.0.0-20190930133256-258c2381defd
github.com/creack/pty v1.1.24
github.com/disintegration/imaging v1.6.2
github.com/docker/go-units v0.5.0
github.com/edaniels/gobag v1.0.7-0.20220607183102-4242cd9e2848
Expand Down Expand Up @@ -181,8 +181,11 @@ require (
github.com/charmbracelet/lipgloss v1.1.0 // indirect
github.com/charmbracelet/x/ansi v0.9.3 // indirect
github.com/charmbracelet/x/cellbuf v0.0.13 // indirect
github.com/charmbracelet/x/conpty v0.1.0 // indirect
github.com/charmbracelet/x/errors v0.0.0-20240508181413-e8d8b6e2de86 // indirect
github.com/charmbracelet/x/exp/strings v0.0.0-20240722160745-212f7b056ed0 // indirect
github.com/charmbracelet/x/term v0.2.1 // indirect
github.com/charmbracelet/x/termios v0.1.1 // indirect
github.com/chewxy/hm v1.0.0 // indirect
github.com/chewxy/math32 v1.0.8 // indirect
github.com/clipperhouse/stringish v0.1.1 // indirect
Expand All @@ -191,6 +194,7 @@ require (
github.com/containerd/console v1.0.5 // indirect
github.com/containerd/stargz-snapshotter/estargz v0.15.1 // indirect
github.com/cpuguy83/go-md2man/v2 v2.0.7 // indirect
github.com/creack/pty v1.1.24 // indirect
github.com/cyphar/filepath-securejoin v0.4.1 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.2.0 // indirect
Expand Down
40 changes: 25 additions & 15 deletions services/shell/builtin/builtin.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"sync"
"time"

"github.com/creack/pty"
"github.com/charmbracelet/x/xpty"
"go.viam.com/utils"

"go.viam.com/rdk/logging"
Expand Down Expand Up @@ -48,24 +48,37 @@ type builtIn struct {
func (svc *builtIn) Shell(ctx context.Context, extra map[string]interface{}) (
chan<- string, chan<- map[string]interface{}, <-chan shell.Output, error,
) {
if runtime.GOOS == "windows" {
return nil, nil, nil, errors.New("shell not supported on windows yet; sorry")
}

defaultShellPath, ok := os.LookupEnv("SHELL")
if !ok {
defaultShellPath = "/bin/sh"
}
shellArgs := []string{"-i"}
shellEnv := []string{"TERM=xterm-256color"}
if runtime.GOOS == "windows" {
// powershell when available, else cmd.exe; inherit the parent environment (nil).
defaultShellPath = "cmd.exe"
if ps, lookErr := exec.LookPath("powershell.exe"); lookErr == nil {
defaultShellPath = ps
}
shellArgs = nil
shellEnv = nil
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

windows doesn't take the -i (interactive shell) or term input using nil here still works


ctxCancel, cancel := context.WithCancel(ctx)
//nolint:gosec
cmd := exec.CommandContext(ctxCancel, defaultShellPath, "-i")
cmd.Env = []string{"TERM=xterm-256color"}
f, err := pty.Start(cmd)
cmd := exec.Command(defaultShellPath, shellArgs...)

@allisonschiang allisonschiang Jun 11, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we pass the ctxCancel into xpty.WaitProcess on line 134 because exec can't launch a conPTY process

see: golang/go#62708

cmd.Env = shellEnv
// xpty gives a unix pty or a Windows ConPTY behind one interface.
f, err := xpty.NewPty(80, 24)
if err != nil {
cancel()
return nil, nil, nil, err
}
if err := f.Start(cmd); err != nil {
cancel()
utils.UncheckedError(f.Close())
return nil, nil, nil, err
}

var sizeLock sync.Mutex
lastSet := time.Time{}
Expand All @@ -92,10 +105,7 @@ func (svc *builtIn) Shell(ctx context.Context, extra map[string]interface{}) (
return
}
lastSet = time.Now()
if err := pty.Setsize(f, &pty.Winsize{
Rows: uint16(rows),
Cols: uint16(cols),
}); err != nil {
if err := f.Resize(int(cols), int(rows)); err != nil {
svc.logger.CErrorw(ctx, "error setting pty window size", "error", err)
}
default:
Expand All @@ -121,8 +131,8 @@ func (svc *builtIn) Shell(ctx context.Context, extra map[string]interface{}) (
utils.PanicCapturingGo(func() {
defer svc.activeBackgroundWorkers.Done()
defer cancel()
if err := cmd.Wait(); err != nil {
svc.logger.CDebugw(ctx, "error waiting for cmd", "error", err)
if err := xpty.WaitProcess(ctxCancel, cmd); err != nil {
svc.logger.CDebugw(ctx, "error waiting for shell process", "error", err)
}
if err := f.Close(); err != nil {
svc.logger.CDebugw(ctx, "error closing pty", "error", err)
Expand Down Expand Up @@ -170,7 +180,7 @@ func (svc *builtIn) Shell(ctx context.Context, extra map[string]interface{}) (
select {
case inputData, ok := <-input:
if ok {
if _, err := f.WriteString(inputData); err != nil {
if _, err := f.Write([]byte(inputData)); err != nil {
svc.logger.CErrorw(ctx, "error writing data", "error", err)
return
}
Expand Down
Loading