`/hooks edit` hands the terminal to $EDITOR without pausing the TUI input thread — keystrokes get split between the editor and the composer

[Lujc0523]

Summary

/hooks edit (also /hooks add, /hooks new) launches $VISUAL/$EDITOR on
.codewhale/hooks.toml without pausing the TUI terminal-input thread. The TUI keeps
reading stdin while the editor owns the terminal, so the user's keystrokes are split
between the two readers. The result is that the editor cannot be quit interactively
(Esc, :q!, Enter never arrive intact), the TUI simultaneously swallows the same
keystrokes into the composer, and the session looks completely frozen.

Every other place that hands the terminal to a child does perform the handshake
(Ctrl+Shift+O composer editor, interactive shell commands). This one path was missed.

Environment

• codewhale 0.9.13 (npm wrapper 0.9.13 + native binary built from the codewhale-cli
0.9.13 crate; codewhale --version → codewhale 0.9.13)
• Linux, xfce4-terminal, PTY 271x49, TERM=xterm-256color
• $VISUAL and $EDITOR both unset → the documented vi fallback is used
• Editor observed: vi (vim), which creates .hooks.toml.swp in .codewhale/

Steps to reproduce

1. Start the TUI in a trusted workspace.
2. Type /hooks edit and press Enter.
3. vi opens on <workspace>/.codewhale/hooks.toml (template is seeded on first use).
4. Press Esc, then :q!, then Enter.

Expected behavior

vi exits, control returns to the TUI, the Hooks screen reports "Hooks unchanged." — the
normal external-editor round trip.

Actual behavior

• The editor does not exit. Nothing the user types is enough to leave it.
• The keystrokes are partially delivered: in a controlled run, : and ! landed in
  vi's command line, while Esc and Enter were consumed by the TUI. The editor ends up
  in an unusable half-state (command line open, no way to confirm or cancel).
• The TUI consumes the rest, so fragments of the user's editing are submitted as composer
  input. In the reported session, ~/.deepseek/composer_history.txt contained the entries
  q, !!, q sitting between two genuine prompts — consistent with the pieces of a
  :q! quit attempt being captured by the TUI (including its Enter) instead of reaching
  the editor.
• The session appears hung. The only reliable recovery is to kill the editor (or the
  session) from a different terminal — which the user cannot do if the TUI is their only
  terminal.

Root cause

The TUI reads stdin on a dedicated pump thread, TerminalInputPump:

• codewhale-tui/src/tui/ui/terminal_input.rs:77 — spawn_parts() spawns the
  codewhale-terminal-input thread.
• codewhale-tui/src/tui/ui/terminal_input.rs:90-98 — the loop only stops reading when
  thread_paused is set; otherwise it runs event::poll(...) and event::read()
  continuously, consuming keystrokes that a foreground child should own.
• codewhale-tui/src/tui/ui/terminal_input.rs:199 — pause_for_child_terminal() is the
  only mechanism that makes the pump yield the terminal (it waits for paused_ack, with a
  500 ms timeout).

with_suspended_tui() (codewhale-tui/src/tui/external_editor.rs:200) handles only
crossterm state — disable_raw_mode() at :222, leave_alt_screen() at :224, and the
mirror calls at :231/:233. It has no access to the pump and therefore never pauses it.
Suspending raw mode alone is not sufficient: the pump thread keeps calling event::read()
on the same tty regardless of the terminal's mode.

The two external-editor call sites differ:

Correct — composer editor (Ctrl+Shift+O), codewhale-tui/src/tui/ui/event_loop.rs:

// :6505
let editor_result = terminal_input.pause_for_child_terminal().and_then(|()| {
    let result = prepare_terminal_input_handoff(&terminal_input, &mut pending_terminal_events)
        .and_then(|ready| {
            if ready {
                crate::tui::external_editor::spawn_editor_for_input(/* :6512 */ )
            } else { /* refuse handoff on pending Esc / Ctrl+C */ }
        });
    terminal_input.resume_after_child_terminal();   // :6526
    result
});

This does all three things: pause the pump, drain buffered input so it cannot leak into the
child (prepare_terminal_input_handoff, codewhale-tui/src/tui/ui/terminal.rs:44), and
refuse the handoff if a cancellation key is pending. The same pattern is used for
interactive shells (event_loop.rs:3191).

Broken — /hooks edit, codewhale-tui/src/tui/ui/apply.rs:

// :2075
AppAction::EditProjectHooks => {
    edit_project_hooks_from_tui(terminal, app, config);   // no terminal_input available
}

// :2550
fn edit_project_hooks_from_tui(terminal: &mut AppTerminal, app: &mut App, config: &Config) {
    // ...
    // :2565
    let outcome = crate::tui::external_editor::spawn_editor_for_path(
        terminal,
        app.use_alt_screen(),
        app.use_mouse_capture,
        app.use_bracketed_paste,
        &path,
    );
}

Neither the action handler nor edit_project_hooks_from_tui has a &TerminalInputPump.
The signature makes the handshake structurally impossible at this layer, so it can never be
added by accident — it was simply never wired up.

Note the same asymmetry exists between spawn_editor_for_input (used by the correct path)
and spawn_editor_for_path (used by the broken one): both call with_suspended_tui, so
both look correct, but the pause lives in the caller and only one caller has it.

Controlled reproduction

Run on the reporter's machine against 0.9.13, in an isolated workspace so no real project
files were touched. Full script: codewhale-issue-experiment.py (attached alongside this
report). It allocates its own PTY, runs a throwaway codewhale, drives the UI, and checks
ps for the spawned vi.

Observed output (abridged):

>>> workspace-trust prompt detected; pressing 1 (trust this workspace)
>>> typing: /hooks edit (char by char)
[check 1] vi processes: ['3984267 3981902 Sl+  pts/34   vi /tmp/.../work/.codewhale/hooks.toml']

>>> injecting exit keys: ESC : q ! ENTER (spread out)
[check 2] vi processes after exit keys: ['3984267 3981902 Sl+  pts/34   vi /tmp/.../work/.codewhale/hooks.toml']

RESULT: vi STILL RUNNING -> keystrokes swallowed (reproduced)

The final screen dump of the PTY shows vi's status line ending in ::! — the : and !
of the exit sequence reached the editor, but Esc and Enter did not. That split is the
signature of two readers competing for one tty, and it is reproducible on demand.

Impact

• /hooks edit is unusable with any terminal editor (vi, vim, nano, emacs -nw),
  which is exactly the fallback the resolver picks when $VISUAL/$EDITOR are unset
  (external_editor.rs:40-50).
• The failure mode is maximally confusing: the editor partially responds, so it does not
  look like a missing handoff — it looks like a hung agent.
• The user's typed characters are captured as composer input, so failed quit attempts are
  submitted as messages to the model.
• Recovery requires a second terminal. On a single-terminal setup the whole session is
  lost.

Related path with the same defect

codewhale-tui/src/tui/history.rs:2937 (try_open_file_at_line, invoked on mouse click of a
path:line in tool output via codewhale-tui/src/tui/mouse_ui.rs:1680) spawns $EDITOR
with Command::new(&editor).spawn() at :2963 and never suspends the TUI at all — not even
raw mode or the alt screen. Its fallback is also vim. Terminal editors launched from that
path compete for the same tty while the TUI keeps rendering.

Companion defect: the same missing invariant, in the other direction

There is a mirror image of this bug, reported separately in
codewhale-issue-sigttin-suspend-poisons-terminal.md: the TUI's own process group can end
up in the background, at which point its next read of the tty raises SIGTTIN and the kernel
stops the whole group mid-turn. Because the TUI installs no SIGTSTP/SIGTTIN/SIGCONT
handler, the very modes this file's code path also enables — raw mode, mouse capture,
bracketed paste, the alternate screen — are never torn down, and the terminal goes on to
"poison the shell with raw escape reports"
(codewhale-tui/src/tui/ui/terminal.rs:119-120).

The two defects are opposite failures of the same thing. Here the TUI keeps competing for a
tty that a child owns; there the TUI loses its claim on the tty entirely and is stopped by
the kernel. In both cases terminal ownership is implemented per call-site rather than as
one enforced invariant — every "hand the terminal to a child" and every "take it back"
decision is made by whichever call site remembers to make it.

Test gap

There are unit tests for the handshake primitives themselves —
codewhale-tui/src/tui/ui/tests.rs:25635
(terminal_input_child_pause_drains_codewhale_events_before_editor_handoff) and :25686
(terminal_input_handoff_preserves_pending_cancellation_keys) — but nothing asserts that
each editor entry point uses them. spawn_editor_for_path and
edit_project_hooks_from_tui have no test coverage at all, which is why the missing pairing
went unnoticed.

Suggested fix

Items 1–4 each close this bug; item 5 is what keeps it closed.

1. Thread &TerminalInputPump down to edit_project_hooks_from_tui and wrap the
   spawn_editor_for_path call in the same pause → prepare_terminal_input_handoff → resume sequence used at event_loop.rs:6505. Extract that sequence into one helper so
   both call sites share it.
2. Move the pause into with_suspended_tui by giving it access to the pump, so every
   caller is correct by construction and the "caller must remember to pause" invariant
   disappears.
3. Add a regression test that enumerates external-editor entry points and asserts each one
   pauses the pump before spawning — or a PTY-level integration test in the style of the
   reproduction script above.
4. Whatever the fix, consider documenting that $EDITOR must be a GUI editor or one that
   returns immediately (code --wait, subl -w), since that is currently the only safe
   configuration for the terminal-editor paths.
5. Reduce all of the above to one invariant instead of more call-site fixes: a single
   "terminal ownership" abstraction that owns raw mode, the mouse/paste/alt-screen modes and
   the input pump together, and that every path must pass through — handing the terminal to a
   child, stopping for a job-control signal, resuming, or exiting. Items 1–2 fix this entry
   point; only this generalises to the companion defect above, and to the next call site
   nobody has written yet.

Reporter notes

• The .hooks.toml.swp swap file is left behind when the editor is killed, and the
  template file itself is unchanged; no data was lost in the reported incident.
• Killing the editor from another terminal (kill -TERM <vi pid>) does restore the TUI, so
  the TUI side is not itself deadlocked — it is blocked in wait() on the child while its
  input pump keeps stealing keystrokes.

[github-actions]

Thanks @Lujc0523 for the report.

This issue is staying open for maintainer triage. CodeWhale gets better because people bring us real edge cases from real machines, providers, regions, and workflows.

If you can add a reproduction, logs, version output, screenshots, or the provider/model involved, that makes it much easier for us to verify and harvest the fix. Maintainers may comment /lgtmi to mark recurring issue reporters as approved so this intake note is skipped next time.

[Hmbown]

Confirmed, and the diagnosis holds: /hooks edit reaches spawn_editor_for_path → run_editor_on_path, which calls cmd.status() with stdin inherited, while the codewhale-terminal-input pump keeps calling event::poll/event::read on the same tty. Two readers, one tty, and the kernel hands each keystroke to whichever read() lands first — exactly the :/! to vi, Esc/Enter to the composer split you measured.

One correction, because it changes the size and shape of the fix.

There is no shared handshake helper that this path forgot to call

The report says every other place that hands the terminal to a child performs the handshake, and this one was missed. The primitives are shared — pause_for_child_terminal / resume_after_child_terminal (terminal_input.rs:199/:223) and prepare_terminal_input_handoff (terminal.rs:44) — but the composition is written inline, twice, by two different mechanisms:

• the composer editor (Ctrl+Shift+O) does pause → drain → spawn → resume inline at event_loop.rs:6485-6509;
• engine-driven interactive children use a different protocol entirely: the tool layer sends Event::PauseEvents with an ack (InteractiveTerminalGuard::engage, core/engine/tool_execution.rs:105-148) and the event loop does the pause/drain in a handler at event_loop.rs:3168-3258.

So there is no single helper to call. The fix has to create one.

And there is a scope problem behind it

The input pump is a local variable of the event-loop function (let mut terminal_input = TerminalInputPump::spawn()?, event_loop.rs:1450). It is not on App, not on AppTerminal, and not passed down. /hooks edit dispatches AppAction::EditProjectHooks into apply.rs, which carries &mut AppTerminal but no pump handle — so that layer performs the terminal-mode half (with_suspended_tui) and cannot perform the reader half, no matter how carefully it is written.

That is also why with_suspended_tui's doc comment ("Hand the terminal to a child") states an invariant the function has no way to enforce, and why both editor entry points look correct on inspection while only one is.

The fix we have scoped: add the missing with_child_terminal helper next to prepare_terminal_input_handoff, rewrite the inline composer-editor block to use it (so the abstraction deletes caller code rather than adding a third way), and make /hooks edit a deferred request the event-loop body runs where the pump is in scope — the precedent already exists at event_loop.rs:4713.

There is a second broken site

try_open_file_at_line (history.rs:2937) has the same gap and one more: it uses .spawn() rather than .status() (history.rs:2963), so the TUI does not even wait for the child. It is called from mouse_ui.rs:1635, another layer with no pump handle, so it needs the same deferral. Worth fixing in the same slice rather than discovering it later.

Not started tonight: ui/event_loop.rs and ui/terminal.rs have uncommitted work from another agent in this workspace, and this fix has to edit both. Queued rather than done, to avoid clobbering their in-flight changes.

Thank you for the controlled run that showed which keys went where — that is what made the two-readers conclusion unambiguous rather than a guess.

[Hmbown]

claiming: taking suggestion 2 rather than 1 — moving the pump pause inside with_suspended_tui, so every external-editor entry point is correct by construction instead of /hooks edit growing its own copy of the handshake.

Files: crates/tui/src/tui/ui/terminal_input.rs (a process-scoped gate on the one pump, plus an RAII pause guard), crates/tui/src/tui/ui.rs (re-export), crates/tui/src/tui/external_editor.rs (with_suspended_tui acquires it first and releases it last), and a regression test in crates/tui/src/tui/ui/tests.rs.

Not in this slice: history.rs try_open_file_at_line, which you flagged under "Related path with the same defect". It fire-and-forget spawn()s $EDITOR without waiting at all, so it needs a different shape than a pause — I will file it separately rather than let it be buried by closing this.

[Hmbown]

Fixed by dd5d7e4fa on fix/lost-work-batch-a-20260915 (local branch — not pushed; pushing is founder authority).

Shape: your suggestion 2, not 1. Threading &TerminalInputPump into edit_project_hooks_from_tui means threading it and the pending-event queue through apply_command_result, which has twelve call sites, to fix one of them — and it leaves the next entry point to remember the rule. Instead the pause moved into with_suspended_tui, which both editor entry points already go through:

• crates/tui/src/tui/ui/terminal_input.rs — a process-scoped gate on the pump's paused/paused_ack flags, published by spawn/install_parts and retracted on detach/drop (retraction is Arc::ptr_eq-guarded so a detached wedged thread cannot unpublish its replacement), plus pause_terminal_input_for_child() -> io::Result<ChildTerminalInputPause>, an RAII guard. One stdin, one pump, so the singleton is by construction rather than convention.
• crates/tui/src/tui/external_editor.rs — with_suspended_tui takes the guard as step 0, before it touches any terminal mode, and drops it as step 4, after the modes are restored. It fails closed: a pump that will not acknowledge means the handoff would reproduce this exact defect, so the editor does not run.

Tests (crates/tui/src/tui/ui/tests.rs):

test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 12734 filtered out; finished in 0.55s

(child_terminal_pause_stops_the_pump_reading_and_resumes_it_on_drop, child_terminal_pause_refuses_the_handoff_when_the_pump_never_acknowledges, plus the three pre-existing pump tests, which still pass.)

test result: ok. 12 passed; 0 failed; 0 ignored; 0 measured; 12727 filtered out; finished in 0.02s

(external_editor::.)

cargo fmt --all -- --check clean; CI's clippy line (-D warnings with the uninlined_format_args/too_many_arguments/unnecessary_map_or allows) clean on -p codewhale-tui --all-targets --all-features.

What I could not verify, stated plainly. The new tests drive a simulated pump loop, not the crossterm one — spawning the real pump needs an interactive terminal, and building an AppTerminal in a test needs a tty for backend.size(). So what is pinned is the guard's contract (stops reading, resumes on drop, refuses on no-ack) and the fact that it now sits on the only path to the child. What is not pinned by a test is the end-to-end /hooks edit round trip; your PTY script is still the only thing that proves that, and I did not run it. Your suggestion 3 — a PTY-level integration test — remains worth doing.

Suggestion 5 is not done. This unifies the input pump with the terminal modes inside one function, which is most of "hand the terminal to a child". It does nothing for the other direction: no SIGTSTP/SIGTTIN/SIGCONT handler, so the companion defect stands.

Known limitations, written into the code beside the behaviour: the gate stops the pump reading but cannot drain input the pump already buffered, and cannot refuse the handoff on a pending Esc/Ctrl+C — both need the receiver and the event loop's pending queue, so prepare_terminal_input_handoff stays at the call sites that have them. The composer path still does all three; /hooks edit now does the one that matters for this bug.

The related path you flagged is not fixed: history.rs try_open_file_at_line fire-and-forget spawn()s $EDITOR and never waits, so there is no point at which a pause could end. Filed as #6235 rather than left buried under this closure.