fix: set stdout in exec when not tty is available

[ivanvc]

Currently, Exec will force the stdout to the TTY. However, if there's no TTY available, the stdout should be just the program stdout.

I found this behavior while using Exec in a Wish application. The application would hang, as p.output.TTY() is nil.

[jonbretman]

Might this also fix an issue I'm seeing using a CLI that uses Bubbletea in a build image (AWS Codebuild). I get:

panic: open /dev/tty: no such device or address

[hopefulTex]

How does setting the output to p.output differ from the (nil)file returned by p.output.TTY()?
If the goal is to output to stdout then would using os.Stdout work similarly to setting Stderr below?

[ivanvc]

Hi @hopefulTex!

As background, I'm doing this from a Wish (SSH) application. I debugged Wish, Bubble Tea, and termenv to see where would be the best place for this fix. With this in mind, let me answer your questions.

How does setting the output to p.output differ from the (nil)file returned by p.output.TTY()?

If the output is nil, the client won't display anything on the screen.

If the goal is to output to stdout then would using os.Stdout work similarly to setting Stderr below?

I'm afraid the goal is not to output to os.Stdout. Doing so would show the output of the execution on the server's screen rather than the client's.

The underlying problem is that Bubble Tea's option WithOutput(...) will convert the output (ssh.Session which implements the io.Writer interface) into a termenv.Output. Then, termenv's output.TTY() will return nil as tty doesn't implement the termenv.File interface (ssh.Session doesn't define Fd() uintptr, which makes sense as it doesn't have a file descriptor).

So, the possible places to fix this issue are:

1. Wish's Bubble Tea middleware. Wrap the output in a struct that defines a dummy FD() function. But, I would discard this option, as it just instructs to use bubbletea.WithOutput(...).
2. Bubble Tea's WithOutput() option, check if the output is an ssh.Session. If so, could you do the wrapping defined in 1? But, it doesn't seem to be the right place as it would add a dependency on charmbracelet/ssh. Or, checking if the output is an ssh.Session, and then not creating a termenv.Output (but it would lose termenv's features).
3. In termenv's output.TTY(), checking if tty is an ssh.Session. However, it has the same issue as in the previous option.
4. Finally, the last option I can think of would be to define the dummy Fd() uintptr method in (ssh.session)[https://github.com/charmbracelet/ssh/blob/master/session.go]. To be honest, this would be another option, but I'm torn because defining a dummy file descriptor doesn't sound like a good thing to do.

Yet another option would be to create a Bubble that receives the ssh.Session (as I wouldn't have access to the program input/output as they are not exported). But, the bug would still happen if someone else wants to execute a remote command using a Bubble Tea application with Wish.

I created two sample applications that reproduce the issue. One doesn't allocate a pty, so the command (journalctl -f) just shows the output to the client. While the other one starts an interactive bash session: https://gist.github.com/ivanvc/cb4580c02e8bb04d17ef286c9183098b

[ivanvc]

@jonbretman, I think that issue may not be related. Seems like what you're using is not allocating a PTY (or TTY). In my case, the screen gets blank, as the output is nil, and there's nothing to display (the output won't get to the client's screen). I can take a look at your code and check if it's a related issue. You can also replace bubbletea in go.mod with my branch and check if it fixes the issue:

require (                                                                                                 
...
        github.com/charmbracelet/bubbletea v0.24.2  
...
)

replace github.com/charmbracelet/bubbletea => github.com/ivanvc/bubbletea v0.0.0-20230608001803-18eff18d9537