Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bubbletea ExecProcess within a wish session #196

Closed
robinovitch61 opened this issue Dec 1, 2023 · 20 comments · Fixed by #197
Closed

Bubbletea ExecProcess within a wish session #196

robinovitch61 opened this issue Dec 1, 2023 · 20 comments · Fixed by #197
Assignees
Labels
bug Something isn't working

Comments

@robinovitch61
Copy link

robinovitch61 commented Dec 1, 2023

I have a bubble tea application where users can invoke an ExecProcess command to do something like enter vim. When I host the application using wish, the execprocess functionality breaks. For example:

package main

// An example Bubble Tea server. This will put an ssh session into alt screen
// and continually print up to date terminal information.

import (
	"context"
	"errors"
	"fmt"
	"os"
	"os/exec"
	"os/signal"
	"syscall"
	"time"

	tea "github.com/charmbracelet/bubbletea"
	"github.com/charmbracelet/log"
	"github.com/charmbracelet/ssh"
	"github.com/charmbracelet/wish"
	bm "github.com/charmbracelet/wish/bubbletea"
	lm "github.com/charmbracelet/wish/logging"
)

const (
	host = "localhost"
	port = 23234
)

func main() {
	s, err := wish.NewServer(
		wish.WithAddress(fmt.Sprintf("%s:%d", host, port)),
		wish.WithMiddleware(
			bm.Middleware(teaHandler),
			lm.Middleware(),
		),
	)
	if err != nil {
		log.Error("could not start server", "error", err)
	}

	done := make(chan os.Signal, 1)
	signal.Notify(done, os.Interrupt, syscall.SIGINT, syscall.SIGTERM)
	log.Info("Starting SSH server", "host", host, "port", port)
	go func() {
		if err = s.ListenAndServe(); err != nil && !errors.Is(err, ssh.ErrServerClosed) {
			log.Error("could not start server", "error", err)
			done <- nil
		}
	}()

	<-done
	log.Info("Stopping SSH server")
	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
	defer func() { cancel() }()
	if err := s.Shutdown(ctx); err != nil && !errors.Is(err, ssh.ErrServerClosed) {
		log.Error("could not stop server", "error", err)
	}
}

// You can wire any Bubble Tea model up to the middleware with a function that
// handles the incoming ssh.Session. Here we just grab the terminal info and
// pass it to the new model. You can also return tea.ProgramOptions (such as
// tea.WithAltScreen) on a session by session basis.
func teaHandler(s ssh.Session) (tea.Model, []tea.ProgramOption) {
	pty, _, active := s.Pty()
	if !active {
		wish.Fatalln(s, "no active terminal, skipping")
		return nil, nil
	}
	m := model{
		term:   pty.Term,
		width:  pty.Window.Width,
		height: pty.Window.Height,
	}
	return m, []tea.ProgramOption{tea.WithAltScreen()}
}

// Just a generic tea.Model to demo terminal information of ssh.
type model struct {
	term   string
	width  int
	height int
}

func (m model) Init() tea.Cmd {
	return nil
}

type VimFinishedMsg struct{ err error }

func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
	switch msg := msg.(type) {
	case tea.WindowSizeMsg:
		m.height = msg.Height
		m.width = msg.Width
	case tea.KeyMsg:
		switch msg.String() {
		case "e":
			c := exec.Command("vim", "file.txt")
			cmd := tea.ExecProcess(c, func(err error) tea.Msg {
				return VimFinishedMsg{err: err}
			})
			return m, cmd
		case "q", "ctrl+c":
			return m, tea.Quit
		}
	}
	return m, nil
}

func (m model) View() string {
	s := "Your term is %s\n"
	s += "Your window size is x: %d y: %d\n\n"
	s += "Press 'q' to quit\n"
	return fmt.Sprintf(s, m.term, m.width, m.height)
}

If I don't run this with wish, e.g. replace main with the following contents:

	if _, err := tea.NewProgram(model{}).Run(); err != nil {
		fmt.Println("uh oh:", err)
		os.Exit(1)
	}

Then the exec process works fine.

Any ideas on if and how this might be possible to fix?

robinovitch61 added a commit to robinovitch61/wander that referenced this issue Dec 1, 2023
Fixes #107

This is a big change that does the following:
- replaces the bespoke exec implementation with an impl inspired from the nomad cli, using the client directly
- add a wander exec command
- therefore tls is supported
- improves general exec experience (can move cursor, does not strip ansi escape sequences, etc)
- BREAKAGE: exec with wander serve breaks. Unfortunately with this impl, tea.ExecProcess is used, which is currently charmbracelet/wish#196
@caarlos0 caarlos0 self-assigned this Dec 4, 2023
@caarlos0 caarlos0 added the bug Something isn't working label Dec 4, 2023
@caarlos0
Copy link
Member

caarlos0 commented Dec 4, 2023

Hey, thanks for the issue!

Opened #197, if you can try it out.

PS: it'll not work on windows :|

@robinovitch61
Copy link
Author

robinovitch61 commented Dec 4, 2023

Hi @caarlos0 , thanks so much for hopping on this! It almost works perfectly, but I'm finding that if I want to return to the application that shows "Press 'e' to edit" after exiting vim, I can't. E.g. in the example your PR is adding, comment out:

	case VimFinishedMsg:
		//if msg.err != nil {
		//	m.err = msg.err
		//	return m, tea.Quit
		//}
	}

Then run the app, enter and exit vim, then terminal becomes non responsive

@caarlos0
Copy link
Member

caarlos0 commented Dec 4, 2023

no problem!

to do that, you'll need to change it to return m, nil instead of commenting it all out.

also updated the PR if you wanna check it out

@robinovitch61
Copy link
Author

robinovitch61 commented Dec 4, 2023

no problem!

to do that, you'll need to change it to return m, nil instead of commenting it all out.

also updated the PR if you wanna check it out

unfortunately i'm still getting the terminal freezing on :wq rather than returning back to the "Press..." prompt as I'd expect. Short demo here - I have to kill the tmux pane at the end as the terminal becomes non-responsive

2023-12-04_13-47-32.mp4

@caarlos0
Copy link
Member

caarlos0 commented Dec 5, 2023

ah, I see what's going on:

	case VimFinishedMsg:
		m.err = msg.err
		return m, nil

this should fix it..

@robinovitch61
Copy link
Author

	case VimFinishedMsg:
		m.err = msg.err
		return m, nil

no dice :/ demo with updated version:

2023-12-04_18-38-24.mp4

@caarlos0
Copy link
Member

caarlos0 commented Dec 7, 2023

I'm unable to repro this... are you running the full example from #197?

If so, which terminal and contents of $TERM and $SHELL you have?

@robinovitch61
Copy link
Author

robinovitch61 commented Dec 7, 2023

I'm unable to repro this... are you running the full example from #197?

If so, which terminal and contents of $TERM and $SHELL you have?

Thanks for trying! I am also unable to repro in a docker container as follows (it works there):

dockerized :wq returns without hanging:
❯ docker run -ti golang:1.21 bash
root@b5a486264de3:/go# git clone https://github.com/charmbracelet/wish.git
Cloning into 'wish'...
remote: Enumerating objects: 1473, done.
remote: Counting objects: 100% (911/911), done.
remote: Compressing objects: 100% (404/404), done.
remote: Total 1473 (delta 658), reused 630 (delta 474), pack-reused 562
Receiving objects: 100% (1473/1473), 425.46 KiB | 2.82 MiB/s, done.
Resolving deltas: 100% (860/860), done.
root@b5a486264de3:/go# cd wish
root@b5a486264de3:/go/wish# git checkout using-pty
branch 'using-pty' set up to track 'origin/using-pty'.
Switched to a new branch 'using-pty'
root@b5a486264de3:/go/wish# git log | head
commit 9ba7e364763978fbbfcdb91909457c917c34a655
Author: Carlos Alexandro Becker <[email protected]>
Date:   Mon Dec 4 22:19:39 2023 -0300

    docs: fix example

commit 46447eec1602bd08b41eda0015bccb8bd4781823
Author: Carlos Alexandro Becker <[email protected]>
Date:   Mon Dec 4 17:28:21 2023 -0300

root@b5a486264de3:/go/wish# cd examples/wish-exec/
root@b5a486264de3:/go/wish/examples/wish-exec# go run . &
[1] 20
root@b5a486264de3:/go/wish/examples/wish-exec# go: downloading github.com/charmbracelet/bubbletea v0.24.3-0.20231204180345-7e60f93c11a6
go: downloading github.com/charmbracelet/log v0.3.1
go: downloading github.com/charmbracelet/ssh v0.0.0-20231203183338-c29875a2932c
go: downloading github.com/charmbracelet/keygen v0.5.0
go: downloading golang.org/x/crypto v0.16.0
go: downloading github.com/aymanbagabas/go-pty v0.2.1
go: downloading github.com/charmbracelet/lipgloss v0.9.1
go: downloading github.com/muesli/termenv v0.15.2
go: downloading github.com/go-logfmt/logfmt v0.6.0
go: downloading github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be
go: downloading github.com/containerd/console v1.0.4-0.20230313162750-1ae8d489ac81
go: downloading github.com/mattn/go-isatty v0.0.18
go: downloading github.com/mattn/go-localereader v0.0.1
go: downloading github.com/muesli/ansi v0.0.0-20211018074035-2e021307bc4b
go: downloading github.com/muesli/cancelreader v0.2.2
go: downloading github.com/muesli/reflow v0.3.0
go: downloading golang.org/x/sync v0.5.0
go: downloading golang.org/x/term v0.15.0
go: downloading github.com/creack/pty v1.1.15
go: downloading github.com/u-root/u-root v0.11.0
go: downloading golang.org/x/sys v0.15.0
go: downloading github.com/mattn/go-runewidth v0.0.15
go: downloading github.com/aymanbagabas/go-osc52/v2 v2.0.1
go: downloading github.com/lucasb-eyer/go-colorful v1.2.0
go: downloading github.com/rivo/uniseg v0.2.0
2023/12/07 17:36:20 INFO Starting SSH server host=localhost port=23235

root@b5a486264de3:/go/wish/examples/wish-exec# ssh localhost -p 23235
The authenticity of host '[localhost]:23235 ([127.0.0.1]:23235)' can't be established.
ED25519 key fingerprint is SHA256:OQHPMGGhFKDd03upvg0fxLgNcDcB7YpejykI0vsCInU.
This key is not known by any other names.
Are you sure you want to continue connecting (yes/no/[fingerprint])? yes
Warning: Permanently added '[localhost]:23235' (ED25519) to the list of known hosts.
2023/12/07 17:36:51 pty allocated: /dev/pts/2
                                             2023/12/07 17:36:51 INFO root connect 127.0.0.1:38460 false [] xterm 238 58
                                                                                                                        2023/12/07 17:36:53 INFO 127.0.0.1:38460 disconnect 2.546413376s
                                                                                                                                                                                        Connection to localhost closed.
root@b5a486264de3:/go/wish/examples/wish-exec# apt update && apt install vim
Get:1 http://deb.debian.org/debian bookworm InRelease [151 kB]
Get:2 http://deb.debian.org/debian bookworm-updates InRelease [52.1 kB]
Get:3 http://deb.debian.org/debian-security bookworm-security InRelease [48.0 kB]
Get:4 http://deb.debian.org/debian bookworm/main arm64 Packages [8681 kB]
Get:5 http://deb.debian.org/debian bookworm-updates/main arm64 Packages [6672 B]
Get:6 http://deb.debian.org/debian-security bookworm-security/main arm64 Packages [103 kB]
Fetched 9042 kB in 2s (5456 kB/s)
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
1 package can be upgraded. Run 'apt list --upgradable' to see it.
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
The following additional packages will be installed:
  libgpm2 libsodium23 vim-common vim-runtime xxd
Suggested packages:
  gpm ctags vim-doc vim-scripts
The following NEW packages will be installed:
  libgpm2 libsodium23 vim vim-common vim-runtime xxd
0 upgraded, 6 newly installed, 0 to remove and 1 not upgraded.
Need to get 8791 kB of archives.
After this operation, 42.0 MB of additional disk space will be used.
Do you want to continue? [Y/n] Y
Get:1 http://deb.debian.org/debian bookworm/main arm64 vim-common all 2:9.0.1378-2 [124 kB]
Get:2 http://deb.debian.org/debian bookworm/main arm64 libgpm2 arm64 1.20.7-10+b1 [14.4 kB]
Get:3 http://deb.debian.org/debian bookworm/main arm64 libsodium23 arm64 1.0.18-1 [119 kB]
Get:4 http://deb.debian.org/debian bookworm/main arm64 vim-runtime all 2:9.0.1378-2 [7025 kB]
Get:5 http://deb.debian.org/debian bookworm/main arm64 vim arm64 2:9.0.1378-2 [1424 kB]
Get:6 http://deb.debian.org/debian bookworm/main arm64 xxd arm64 2:9.0.1378-2 [83.4 kB]
Fetched 8791 kB in 1s (8837 kB/s)
debconf: delaying package configuration, since apt-utils is not installed
Selecting previously unselected package vim-common.
(Reading database ... 15633 files and directories currently installed.)
Preparing to unpack .../0-vim-common_2%3a9.0.1378-2_all.deb ...
Unpacking vim-common (2:9.0.1378-2) ...
Selecting previously unselected package libgpm2:arm64.
Preparing to unpack .../1-libgpm2_1.20.7-10+b1_arm64.deb ...
Unpacking libgpm2:arm64 (1.20.7-10+b1) ...
Selecting previously unselected package libsodium23:arm64.
Preparing to unpack .../2-libsodium23_1.0.18-1_arm64.deb ...
Unpacking libsodium23:arm64 (1.0.18-1) ...
Selecting previously unselected package vim-runtime.
Preparing to unpack .../3-vim-runtime_2%3a9.0.1378-2_all.deb ...
Adding 'diversion of /usr/share/vim/vim90/doc/help.txt to /usr/share/vim/vim90/doc/help.txt.vim-tiny by vim-runtime'
Adding 'diversion of /usr/share/vim/vim90/doc/tags to /usr/share/vim/vim90/doc/tags.vim-tiny by vim-runtime'
Unpacking vim-runtime (2:9.0.1378-2) ...
Selecting previously unselected package vim.
Preparing to unpack .../4-vim_2%3a9.0.1378-2_arm64.deb ...
Unpacking vim (2:9.0.1378-2) ...
Selecting previously unselected package xxd.
Preparing to unpack .../5-xxd_2%3a9.0.1378-2_arm64.deb ...
Unpacking xxd (2:9.0.1378-2) ...
Setting up libsodium23:arm64 (1.0.18-1) ...
Setting up libgpm2:arm64 (1.20.7-10+b1) ...
Setting up xxd (2:9.0.1378-2) ...
Setting up vim-common (2:9.0.1378-2) ...
Setting up vim-runtime (2:9.0.1378-2) ...
Setting up vim (2:9.0.1378-2) ...
update-alternatives: using /usr/bin/vim.basic to provide /usr/bin/editor (editor) in auto mode
update-alternatives: using /usr/bin/vim.basic to provide /usr/bin/ex (ex) in auto mode
update-alternatives: using /usr/bin/vim.basic to provide /usr/bin/rview (rview) in auto mode
update-alternatives: using /usr/bin/vim.basic to provide /usr/bin/rvim (rvim) in auto mode
update-alternatives: using /usr/bin/vim.basic to provide /usr/bin/vi (vi) in auto mode
update-alternatives: using /usr/bin/vim.basic to provide /usr/bin/view (view) in auto mode
update-alternatives: using /usr/bin/vim.basic to provide /usr/bin/vim (vim) in auto mode
update-alternatives: using /usr/bin/vim.basic to provide /usr/bin/vimdiff (vimdiff) in auto mode
Processing triggers for libc-bin (2.36-9+deb12u3) ...
root@b5a486264de3:/go/wish/examples/wish-exec# ssh localhost -p 23235
2023/12/07 17:37:05 pty allocated: /dev/pts/3
                                             2023/12/07 17:37:05 INFO root connect 127.0.0.1:53914 false [] xterm 238 58
                                                                                                                        2023/12/07 17:37:09 INFO 127.0.0.1:53914 disconnect 3.66838621s
                                                                                                                                                                                       Connection to localhost closed.
root@b5a486264de3:/go/wish/examples/wish-exec#

Here are details of my environment where it doesn't work:

~/projects/wish/examples/wish-exec using-pty
❯ git log | head
commit 9ba7e364763978fbbfcdb91909457c917c34a655
Author: Carlos Alexandro Becker <[email protected]>
Date:   Mon Dec 4 22:19:39 2023 -0300

    docs: fix example
❯ printenv | rg 'TERM|SHELL'
TERM_SESSION_ID=w0t0p0:74FB7D50-7353-428B-AAB9-8C67B8923493
LC_TERMINAL_VERSION=3.4.22
ITERM_PROFILE=Default
SHELL=/bin/zsh
TERM_PROGRAM_VERSION=3.4.22
TERM_PROGRAM=iTerm.app
LC_TERMINAL=iTerm2
COLORTERM=truecolor
TERM=xterm-256color
ITERM_SESSION_ID=w0t0p0:74FB7D50-7353-428B-AAB9-8C67B8923493
  • using iterm2, but mac terminal also didn't work
image

I've tried various things to make it work in my environment, but none have worked yet, including:

  • running unset TERM_PROGRAM TERM_PROGRAM_VERSION TERM_SESSION_ID SHELL && export TERM=xterm before running the wish app and before ssh'ing in
  • trying with go versions 1.20 as well as 1.21
  • trying inside of and outside of tmux

@caarlos0
Copy link
Member

caarlos0 commented Dec 7, 2023

Does it work if you run with other editor, for example, nano?

@robinovitch61
Copy link
Author

robinovitch61 commented Dec 7, 2023

Does it work if you run with other editor, for example, nano?

same thing in nano, it hangs after exit

image

@robinovitch61
Copy link
Author

Another data point: running go run . on the docker host and then running this in a container:

docker run -ti -p 23235:23235 golang:1.20
ssh host.docker.internal -p 23235

successfully connects to the app hosted outside docker from inside docker, but still hangs on exit. Was wondering if maybe it was the version/configuration of ssh that was doing it, but doesn't seem to be the case.

@caarlos0
Copy link
Member

caarlos0 commented Dec 7, 2023

Thanks! I'll try to repro this again later today 🙏

@caarlos0
Copy link
Member

caarlos0 commented Dec 7, 2023

ok, on macos I can repro it all the time

added this:

	case VimFinishedMsg:
		m.err = msg.err
		if m.err != nil {
			log.Error("vim finished", "error", m.err)
		}
		return m, nil

and got the actual issue:

2023/12/07 18:35:32 ERRO vim finished error="error entering raw mode: inappropriate ioctl for device"

now... why this happens, I dunno yet

cc/ @aymanbagabas

@aymanbagabas
Copy link
Member

aymanbagabas commented Dec 7, 2023

2023/12/07 18:35:32 ERRO vim finished error="error entering raw mode: inappropriate ioctl for device"

now... why this happens, I dunno yet

Digging into this using this diff on top of the example from #197, I was able to run Vim fine. However here, I believe both Bubble Tea and Vim are trying to acquire the terminal resulting in this error. When you run tea.Exec, the shell handles the context switching between processes. But here, we're not running a shell on the SSH session and running the program directly in the Pty. What you could do, but feels hacky, is open a new Pty terminal, run Vim on it, and copy I/O to the Bubble Tea Pty.

diff --git a/examples/wish-exec/main.go b/examples/wish-exec/main.go
index 3df70cd2d250..9ebc06446593 100644
--- a/examples/wish-exec/main.go
+++ b/examples/wish-exec/main.go
@@ -16,7 +16,6 @@ import (
 	"github.com/charmbracelet/log"
 	"github.com/charmbracelet/ssh"
 	"github.com/charmbracelet/wish"
-	bm "github.com/charmbracelet/wish/bubbletea"
 	lm "github.com/charmbracelet/wish/logging"
 )
 
@@ -30,7 +29,8 @@ func main() {
 		wish.WithAddress(fmt.Sprintf("%s:%d", host, port)),
 		ssh.AllocatePty(),
 		wish.WithMiddleware(
-			bm.Middleware(teaHandler),
+			// bm.Middleware(teaHandler),
+			myHandler,
 			lm.Middleware(),
 		),
 	)
@@ -57,6 +57,21 @@ func main() {
 	}
 }
 
+func myHandler(sh ssh.Handler) ssh.Handler {
+	return func(s ssh.Session) {
+		ppty, _, active := s.Pty()
+		if !active {
+			wish.Fatalln(s, "no active terminal, skipping")
+			return
+		}
+		c := ppty.Pty.Command("vim", "file.txt")
+		if err := c.Run(); err != nil {
+			wish.Fatalln(s, "vim failed", err)
+			return
+		}
+	}
+}
+
 func teaHandler(s ssh.Session) (tea.Model, []tea.ProgramOption) {
 	_, _, active := s.Pty()
 	if !active {
@@ -94,6 +109,9 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 		}
 	case VimFinishedMsg:
 		m.err = msg.err
+		if m.err != nil {
+			log.Error("vim finished", "error", m.err)
+		}
 		return m, nil
 	}
 

@caarlos0
Copy link
Member

caarlos0 commented Dec 8, 2023

yep, that was my conclusion too... although for some reason I can't repro the issue on Linux, only on macOS.

Maybe we could provide some sort of helper to do that?

@robinovitch61
Copy link
Author

@caarlos0 and @aymanbagabas , let me know if there are any actions I can take to help validate the ideas of a fix here. Thanks for your help so far!

@robinovitch61
Copy link
Author

Gentle bump on this - do y'all think you might plausibly get to a fix here sometime this month? If not I'll release v1.0 of my app anyway with a caveat about slightly limited functionality when using via the ssh app!

@dezren39
Copy link
Contributor

dezren39 commented Jan 2, 2024

Windows of course, (basically) doesn't have a TTY, but they sort-of recently got a PTY. ConPTY may be able suffice, maybe one can spawn a conpty like in #197 but I don't know the details. Leaving this note just in case someone else investigates.

@caarlos0
Copy link
Member

caarlos0 commented Jan 4, 2024

Gentle bump on this - do y'all think you might plausibly get to a fix here sometime this month? If not I'll release v1.0 of my app anyway with a caveat about slightly limited functionality when using via the ssh app!

unlikely, I haven't figured out how to fix this on macOS yet.

since this will be breaking change, we'll also need to plan it properly 🙏

@robinovitch61
Copy link
Author

Hey, it works! Thanks so much @caarlos0 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants