fix(agent): restore raw mode#367
Open
fry69 wants to merge 2 commits into
Open
Conversation
Three fixes for regressions introduced by the raw-mode restoration commit (ce5b70b): 1. agent_prompt_yes_no_ex: handle non-blocking stdin editor_start sets O_NONBLOCK on stdin; fgets() returned NULL immediately (EAGAIN) causing the save prompt to exit without waiting for user input. Temporarily switch to blocking mode around the fgets call. 2. /quit handler: stop editor before prompting editor_restore_terminal_layout resets the scroll region but does not disable raw mode or restore stdin flags. Call editor_stop first so that raw mode is off and stdin is blocking when we prompt the user. Handle AGENT_EXIT_CANCEL by restarting the editor instead of leaving it half-stopped. 3. linenoiseRestoreRawMode: do not corrupt orig_termios The function unconditionally overwrote orig_termios with the current terminal attributes via tcgetattr. While the editor is active those are the raw-mode settings (OPOST cleared). Later, disableRawMode restores this corrupted orig_termios, leaving the terminal in raw mode after editor_stop. puts output then lacks carriage returns, causing garbled display (e.g. /help). Only update orig_termios when rawmode == 0 (child process changed terminal to cooked).
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
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.
Fix terminal freeze after bash child processes
Problem
Running any bash command via the agent's bash tool call (e.g.,
tmux list-sessions) renders the terminal unresponsive to keyboard input. Typed characters don't appear, and the only recovery is to kill the agent process and pressCtrl-L.After killing the agent, the terminal is compressed to the bottom line - the scroll-region escape sequences the agent left behind are still active - confirming the terminal mode was corrupted, not just the agent's input handling.
Root cause
When the agent spawns
sh -c "<cmd>", onlystdout/stderrare redirected to a pipe.stdinis left as the original terminalfd. The shell detectsstdinis a tty and callstcsetattr()to set cooked mode (ICANON,ECHO, etc.), overridinglinenoise's raw mode. After the child exits the terminal stays cooked.linenoisenever re-applies raw mode becauseenableRawMode()is called once at editor startup.In cooked mode the terminal driver buffers input line-by-line. The agent's non-blocking
read()seesEAGAINbecause no complete line is available until Enter is pressed. The user sees nothing happen.Fix (two layers)
stdinto/dev/null(ds4_agent.c:agent_bash_start)The child now opens
/dev/nullanddup2's it ontoSTDIN_FILENObefore exec. This prevents the shell from ever callingtcsetattr()on the controlling terminal.Safety analysis: The bash tool is a non-interactive command runner. The agent never writes to the child's
stdin– it only readsstdout/stderrvia a pipe. The model supplies all data through command arguments, file writes, or shell pipelines (printf ... | nc host port). Commands that expect interactive terminal input (e.g.,ssh,sudo,git commitwithout-m) never worked because the agent has no mechanism to forward keystrokes to the child. The redirect only breaks things that were already broken, and it fixes the terminal freeze for everything else.linenoise.c,ds4_agent.c)A new public function
linenoiseRestoreRawMode()re-applies the same raw-mode flags thatenableRawMode()sets. It re-fetchesorig_termiosviatcgetattr()so it works even if a child process changed the terminal attributes.The worker thread sets a
raw_mode_needs_restoreflag (under the worker mutex) inagent_bash_finalize()whenever a bash job finishes. The UI thread checks this flag at the top of its event loop and callslinenoiseRestoreRawMode()if needed. This provides defense-in-depth for any child process that might still touch/dev/ttydespite thestdinredirect (e.g.,sshopening/dev/ttydirectly).Testing
Before the fix, running tmux list-sessions inside the agent froze the terminal 100% of the time. After the fix, the command runs normally and the terminal stays fully responsive. Repeated testing with various bash commands (including long-running processes and commands that produce ANSI escape sequences) shows no regression.
fix #366
Update: More fixes.
Fix 1 –
agent_prompt_yes_no_ex(non‑blockingstdin)editor_startsetsstdintoO_NONBLOCK. The prompt function used fgets(stdin)which immediately returnsNULL(EAGAIN) when no data is available, causing the function to treat it as "no" and exit without waiting for the user.Change: Save the
O_NONBLOCKflag, temporarily disable it beforefgets, restore it after.Fix 2 –
/quithandler (terminal restoration before exit)The handler called
editor_restore_terminal_layout(scroll‑region reset) but nevereditor_stop, which is the function that restoresstdinflags and disables raw mode. WhenAGENT_EXIT_NOWwas taken (user declines save),exit(0)was called while raw mode was still active andstdinwas non‑blocking, leaving the terminal unusable.Change: Call
editor_stopbefore prompting so that raw mode and non‑blocking are already disabled when we prompt or exit. TheAGENT_EXIT_CANCELcase now restarts the editor instead of leaving it half‑stopped.Fix 3 –
linenoiseRestoreRawMode(corruptedorig_termios)This is the root cause of the garbled /help output. The function unconditionally overwrote the global
orig_termioswith the current terminal attributes viatcgetattr. While the editor is active, those are the raw‑mode settings (withOPOSTcleared). Later, when editor_stop callsdisableRawMode, it restores that corruptedorig_termios— the terminal stays in raw mode. Then put outputs\nwithout carriage return, staggering the lines.Change: Only update
orig_termioswhenrawmode == 0(i.e., a child process actually changed the terminal to cooked). If we are already in raw mode, keep the originalorig_termiosintact sodisableRawModecan still restore proper cooked mode.