tui: fix shell-mode freeze/crash (unread emulator response pipe + duplicate PTY reader)#2
Open
Anand-rahul wants to merge 4 commits into
Open
Conversation
vt.NewEmulator creates an unbuffered io.Pipe internally. When the emulator processes terminal queries emitted by the shell on startup (cursor position \e[6n, device attributes \e[c/\e[>c, mode reports, OSC colour queries, …) it writes responses to that pipe. Because the pipe is synchronous and nobody ever reads from the read-end (e.pr), every such write blocks indefinitely inside emulator.Write(). That call happens on the hot path of shellModel.handleOutput, which is invoked directly from the bubbletea Update loop — so the whole TUI freezes, appearing to crash. The problem is especially visible with zsh and fish: both shells send cursor-position and device-attribute queries on every prompt redraw when TERM=xterm-256color (which shell.go sets explicitly), whereas bash with TERM=dumb skips most capability queries. Fix 1 — tui/shell.go: after constructing the emulator, start a background goroutine that drains em.Read() and forwards each chunk to the shell's PTY stdin (ptmx). This unblocks the pipe, prevents the deadlock, and correctly delivers terminal responses back to the shell's readline so cursor-positioning works as expected. The goroutine exits automatically when emulator.Close() seals the write-end of the pipe with io.EOF. Fix 2 — tui/tui.go: enterShell() was calling waitForOutput() a second time when re-focusing a shell that was already active for the same spec. That spawned a duplicate goroutine reading from the same PTY fd concurrently with the existing one in the Update-loop chain, splitting the byte stream between them and garbling the emulator display. When the shell is already running, simply switch focus — no new reader needed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When the TUI spawns a shell for a completed spec it uses the PATH from the spec's recorded environment, which has binDir prepended so the consumer's CLI binary is reachable. But t.Cleanup was deleting binDir unconditionally at test teardown, so by the time the user entered shell mode the binary was already gone and every command failed with 'command not found'. GOTIT_KEEP_HOMEDIR=1 is the existing signal that the TUI is tailing the run and needs preserved state. Honour it for binDir too: skip the Cleanup when the flag is set, leaving the binary on disk for as long as the TUI session lasts (the TUI itself already owns the homeDir cleanup via model.Cleanup). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Author
|
Follow-up commit The shell gets |
This reverts commit 1b36339.
The embedded shell was spawned from $SHELL, which is the user's login
shell. On systems where the login shell is fish (or zsh with heavy
plugins), this caused two problems:
* Fish ignores PS1 entirely, so the gotit prompt was never shown.
* Fish and zsh query terminal capabilities (cursor position, device
attributes, colour OSC) on every prompt redraw. Even with the
response-pipe fix, the extra chatter made startup sluggish and
produced spurious output in the emulator.
For a debug/inspection shell, bash is the right default: it starts in
milliseconds with a bare HOME, honours PS1 without any config file,
and only queries the terminal for features it actually needs.
Priority order: bash (from PATH) → $SHELL → /bin/sh.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When entering shell mode in the TUI the app freezes (appearing to crash), particularly with zsh and fish.
Root cause 1 —
tui/shell.go: unreadvt.Emulatorresponse pipevt.NewEmulatorcreates an unbufferedio.Pipeinternally. The emulator writes terminal responses to the write-end of this pipe whenever the shell sends queries such as:\e[6n\e[y;xR\e[c\e[?62;1;6;22c\e[>c\e[>1;10;0c\e[?2004$p\e[?2004;$y\e]10;?\aBecause
io.Pipeis unbuffered and synchronous, these writes block until something reads the other end. Nothing inshell.goever callsem.Read(), so every write hangs indefinitely insideemulator.Write().That call sits directly in the bubbletea Update loop (via
shellModel.handleOutput), so the entire TUI deadlocks.Why zsh/fish specifically:
shell.gosetsTERM=xterm-256colorexplicitly. Both zsh (via readline) and fish query cursor position and device attributes on every prompt redraw under a capableTERM, while bash withTERM=dumbskips most capability queries.Root cause 2 —
tui/tui.go: duplicate PTY reader on re-focusenterShell()calledwaitForOutput()even when a shell was already active for the same spec. This spawned a second goroutine reading from the same PTY file descriptor concurrently with the existing goroutine already chained in the Update loop, splitting the byte stream between them and garbling the emulator display.Fix
tui/shell.go: after constructing the emulator, start one background goroutine that drainsem.Read()and forwards each chunk toptmx(the shell's PTY stdin). This unblocks the pipe, eliminates the deadlock, and correctly delivers terminal responses to the shell's readline. The goroutine exits automatically whenemulator.Close()seals the pipe withio.EOF.tui/tui.go: when re-focusing a shell that is already running for the same spec, just switch focus — do not schedule anotherwaitForOutput().🤖 Generated with Claude Code
Closes #3