The TUI has no job-control handshake: once Codewhale's process group ends up in the background, SIGTTIN suspends it, the terminal keeps the TUI's mouse/paste/raw modes, and an in-flight turn exists only in the checkpoint

[Lujc0523]

Summary

Codewhale's TUI performs a foreground-ownership check exactly once, at startup
(require_foreground_terminal_owner(), called from event_loop.rs:628). There is no
runtime equivalent and no SIGTSTP / SIGTTIN / SIGCONT handler anywhere in the TUI.
So if the TUI's process group becomes non-foreground after startup — by any job-control
route — the outcome is:

1. The background group's next read of the TTY raises SIGTTIN. The whole process group is
   stopped by the kernel, in the middle of an agent turn, with no chance to run any
   cleanup. The shell reports:
   [1] + <pid> suspended (tty input) codewhale resume <session>
2. Because no cleanup runs, every terminal mode the TUI enabled stays enabled: raw mode,
   ?1000h/?1002h/?1003h/?1006h/?1015h mouse reporting, ?2004h bracketed paste, the
   alternate screen. The terminal then "poisons the shell with raw escape reports" — the
   exact failure the code comment at tui/ui/terminal.rs:119-120 warns about — so mouse
   movement / scrolling feeds escape bytes into whatever foreground shell is present.
3. Any child the TUI had just spawned is orphaned or left as a zombie, because the
   stopped parent can never wait() on it (zsh <defunct>, parent = the stopped TUI).
4. The in-flight turn is not in the session file. The session JSON was last written at
   the previous turn boundary (249 messages); the work performed before the stop lives only
   in sessions/checkpoints/<id>.json (265 messages). Whether resume sees those 16
   messages depends on which of the two it reads.

The TUI is not crashed and not deadlocked — it is stopped, and fg revives it. But the
user sees a frozen/garbled terminal, a shell prompt, and no indication that the session is
still alive in the background. From the user's side this reads as "the agent died mid-run".

Environment

• Codewhale 0.9.13 (npm wrapper + native binary built from the codewhale-cli 0.9.13 crate)
• Linux, xfce4-terminal, PTY 271x49, TERM=xterm-256color
• Long-lived session resumed in-session: codewhale resume <session-id>
• Process state read directly from ps / /proc and the shell's own job notification

Observed sequence

The consequence is reproducible and fully evidenced below. The exact job-control action
that first moved the process group out of the foreground is not yet pinned down — the
kernel does not record why a process was stopped, and the shell's earlier suspended
notification (if any) had scrolled away. Candidates that all produce this end state:

• Ctrl+Z followed by bg (resume in background) — the resumed group immediately reads the
  TTY and takes SIGTTIN
• any tcsetpgrp that hands the foreground to another job while Codewhale keeps its input
  pump running
• a second shell or launcher sharing the same controlling TTY

What is established: at the moment of the stop, the process group was not the TTY's
foreground group, and the TUI was reading the TTY (that is the only thing that raises
SIGTTIN).

Expected behavior

Either the TUI never ends up reading a TTY it does not own, or — when it does — it behaves
like every other full-screen terminal program (vim, nano, htop, less): on SIGTSTP/SIGTTIN
it restores the terminal (leave alt screen, disable mouse/paste, disable raw mode), stops
itself, and on SIGCONT re-enters its modes and repaints. The user sees a clean shell while
suspended and an intact TUI after fg.

Actual behavior

• Whole process group T (stopped), wchan: do_signal_stop; shell shows
  suspended (tty input).
• Terminal left in the TUI's modes; SGR/CUP escape fragments (35;37;18N...,
  51;59;26M...) appear appended to shell prompt lines — i.e. the terminal's report stream
  is being echoed by the foreground shell.
• A child process spawned immediately before the stop is left as an unreaped zombie.
• The session file is stale relative to the checkpoint by one full turn.

Evidence

Process state (captured after the incident, while still stopped):

$ ps -o pid,ppid,pgid,sid,tpgid,stat,tty,args -p <tui-pid>,<shell-pid>
    PID    PPID    PGID     SID   TPGID STAT TT       COMMAND
 <tui>  <shell>   <tui> <shell> <shell> Tl   pts/30   node …/codewhale resume <session>
 <bin>   <tui>    <tui> <shell> <shell> Tl   pts/30   …/downloads/codewhale resume <session>
<zomb>   <bin>   <zomb> <shell> <shell>  Z   pts/30   [zsh] <defunct>
 <shell> <login> <shell> <shell> <shell> Ss+  pts/30   zsh

TPGID is the shell, not the TUI: the TUI group is in the background. STAT is Tl.
/proc/<tui>/status reports State: T (stopped) with wchan: do_signal_stop.

Shell job notification (visible on screen):

[1]  + <tui-pid> suspended (tty input)  codewhale resume <session-id>

(tty input) is the shell's rendering of a SIGTTIN stop — distinct from a plain
Ctrl+Z stop, which zsh reports without the (tty input) suffix.

Zombie child: <zsh> Z, PPid = the TUI binary, started in the same second as the
stop and never reaped — consistent with the parent being stopped before it could wait().

Session data split across two files:

sessions/<session>.json             249 messages  mtime 18:21:05   (previous turn boundary)
sessions/checkpoints/<session>.json 265 messages  mtime 18:24:13   (last snapshot before stop)

The last message in the checkpoint is an assistant message carrying two bash tool calls
with no matching tool results — the exact point of interruption. The 16 messages missing
from the session file are one complete round of work.

Terminal modes after the suspend (affected TTY vs. a healthy Codewhale TTY on the same
host, same terminal program):

affected: isig -icanon iexten -echo … opost
healthy : -isig -icanon -iexten -echo … -opost

The affected TTY still carries TUI-left signal/output processing bits. (Both are
-icanon -echo because zsh's ZLE also runs that way, so this comparison is suggestive, not
conclusive on its own.)

Screen text: the shell's own prompt lines are followed by long runs of 256-colour SGR /
cursor-position fragments, which is the "raw escape reports" pattern named in the source
comment quoted below.

Analysis

The codebase already documents this failure mode and defensively guards one moment —
startup:

// codewhale-tui/src/tui/ui/terminal.rs:115-126
/// Refuse to enter terminal modes from a background Unix process group.
///
/// A TTY can still report `isatty(3) == true` after a shell has suspended the
/// process. Reading from that background group triggers `SIGTTIN`; enabling
/// mouse or keyboard protocols before that stop poisons the shell with raw
/// escape reports. Check foreground ownership before the first mode change.
#[cfg(unix)]
pub(crate) fn require_foreground_terminal_owner() -> Result<()> { … }

and the interactive-shell path is outright refused on Unix because it cannot take over the
terminal safely:

// codewhale-tui/src/tools/shell.rs:1700-1708  (Unix)
pub(crate) fn inherited_interactive_terminal_refusal() -> Option<&'static str> {
    Some("Inherited interactive terminal takeover is unavailable on Unix because foreground \
          TTY ownership cannot be transferred safely. …")
}

// codewhale-tui/src/tools/shell.rs:2127-2133
// A new Unix process group that inherits the terminal is not its foreground owner.
// Letting it read stdin triggers SIGTTIN; sharing Codewhale's group instead would
// make cooked-mode Ctrl+C terminate both parent and child. Until this path owns a
// complete POSIX job-control lease, fail closed before spawning.

Both guards are pre-spawn guards. Nothing covers the case where Codewhale's own group
loses the foreground while it is running:

• require_foreground_terminal_owner() has exactly one caller — event_loop.rs:628, in the
  startup path. It is never re-evaluated, and there is no periodic or post-spawn
  tcgetpgrp(STDIN_FILENO) != getpgrp() check.
• emergency_restore_terminal() (tui/ui/terminal.rs:618) exists and does the right thing
  (DisableMouseCapture, disable_raw_mode, …), but is only wired to the panic hook
  (lib.rs:1757-1763) and one early-startup path (lib.rs:767).
• A grep for SIGTSTP|SIGTTIN|SIGCONT|signal_hook across codewhale-tui finds no job
  control handling at all — the only SIGWINCH mention is a comment in
  tui/external_editor.rs. So when the kernel stops the process, no handler runs, and every
  mode the TUI turned on stays on.

The same "terminal handoff is the caller's responsibility and one caller forgot" shape as
the external-editor input-pump issue (/hooks edit / Ctrl+Shift+O), but in the opposite
direction: there the TUI kept competing for input; here the TUI loses its claim on the
terminal entirely and the kernel does the stopping.

The split session state is the second half of the problem. persist_pending_work_checkpoint
(tui/ui/session_state.rs:80) writes PersistRequest::SaveCheckpoint → checkpoints/<id>.json
for in-progress work, while the main sessions/<id>.json is written at turn boundaries. A
stop that lands mid-turn therefore leaves the only copy of the turn in the checkpoint.

Impact

• Silent loss of a turn's visibility. A stop mid-turn leaves the session file one turn
  behind. If the user gives up and kills the process, or resumes through a path that reads
  the session file rather than the checkpoint, that work disappears.
• Terminal left poisoned. Mouse reports and bracketed-paste bytes are delivered into an
  ordinary shell. At minimum this spews escape garbage; at worst the shell accumulates
  content the user never typed.
• Misleading symptom. The session is alive and fg-recoverable, but it is indistinguishable
  from a crash or a hang. The user's rational next step is to kill it — which is precisely the
  action that discards the unflushed turn.
• Unreaped children. Any child alive at stop time becomes a zombie (or an orphan if the
  parent is later killed), so process-group signalling designed to clean up children has
  nothing to run against.
• Reproducible on demand with any terminal editor or long-running shell child present at the
  moment of the stop; no special configuration is required.

Suggested fix

1. Install a job-control handshake (the durable fix). Handle SIGTSTP, SIGTTIN, and
   SIGCONT: on stop, run the same teardown as emergency_restore_terminal(), then
   raise(SIGSTOP) (or re-raise the original signal with the default disposition restored);
   on SIGCONT, re-enable modes and force a full repaint. This is what every other
   full-screen TUI does, and it makes the failure impossible rather than merely detected.
2. Re-check foreground ownership at runtime, not just at startup. After every child spawn
   and on a low-frequency timer, compare tcgetpgrp(STDIN_FILENO) with getpgrp(). On
   mismatch, pause the terminal input pump and surface an explicit message ("this session is
   in the background — run fg") instead of being stopped by the kernel with no diagnostics.
   Reuse validate_foreground_process_group so the message stays consistent with the startup
   one.
3. Make the checkpoint the recovery source of truth. Have resume prefer the checkpoint
   when it is newer than the session file, and tell the user how many messages came from it.
   Alternatively, flush the in-flight turn into the session file when a stop or a
   child-spawn is detected.
4. Extend cleanup coverage beyond the panic hook: SIGTERM/SIGHUP should also run
   emergency_restore_terminal() and reap children, so a killed session does not leave the
   terminal in the same state.

How to confirm you hit this (diagnostic recipe)

# 1. Is the TUI stopped rather than dead?  STAT `T` and a foreground pgid that is not yours.
ps -o pid,ppid,pgid,tpgid,stat,tty,args -p "$(pgrep -f 'codewhale resume' | head -1)"
cat /proc/<pid>/status | grep -E '^(State|SigPnd|ShdPnd)'

# 2. Unreaped children of the stopped TUI:
ps -eo pid,ppid,stat,args | awk '$3 ~ /^Z/'

# 3. Was the turn flushed? Compare message counts and mtimes.
ls -l --time-style=full-iso ~/.codewhale/sessions/<id>.json \
                       ~/.codewhale/sessions/checkpoints/<id>.json

# 4. Recover without losing the turn (do NOT kill):
#    in the terminal that owns the session ->  fg

Test gap

There is a unit test for the startup guard
(tui/ui.rs:360, tui_launch_preflight_rejects_background_process_group) but nothing
asserts runtime behaviour:

• no test that the TUI survives a SIGTSTP/SIGTTIN/SIGCONT cycle with the terminal
  restored (termios compared before/after, and mouse/paste/alt-screen re-enabled on resume);
• no test that emergency_restore_terminal() runs on anything other than the panic path;
• no test that a checkpoint newer than the session file is what resume loads.

A PTY-level integration test in the style of the external-editor reproduction script would
cover all three: start a TUI on its own PTY, move its foreground group away, send SIGTTIN,
assert the process stops and that the PTY's modes were restored, then SIGCONT and assert
repaint.

Workaround

• Recover the running session in place with fg in the owning terminal — the process is
  stopped, not dead, and the in-flight turn is still in memory.
• If the session is abandoned, run reset (or stty sane) in that terminal to clear the
  leftover modes; otherwise mouse movement keeps feeding escape bytes into the shell.
• Avoid backgrounding the Codewhale job (Ctrl+Z + bg) on a TTY you intend to keep using.

Reporter notes

• No data was lost in the reported incident: the session file parses cleanly, all 149
  tool calls in the persisted portion are paired, and the 16 unflushed messages are intact in
  the checkpoint. Only the two tool results that were in flight are missing (both were
  read-only inspection commands, trivially re-runnable).
• The stopped session was recoverable after the fact with fg; nothing in the TUI itself was
  deadlocked. The damage was to the terminal state and to session-file freshness.
• Companion report: codewhale-issue-hooks-editor-input-contention.md — /hooks edit hands
  the terminal to $EDITOR without pausing the TUI input pump, so keystrokes are split
  between two readers. That bug and this one are mirror images: there the TUI keeps a claim
  on the terminal it should have released, here it loses a claim it should have kept. Both
  follow from terminal ownership being implemented per call-site rather than as one enforced
  invariant, and each report now cross-references the other.
• A screen capture of the affected terminal is available on request: it shows the frozen TUI
  frame, the shell's suspended (tty input) job notification, and the escape fragments
  appended to the shell prompt.

[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]

This is a very good report — the process-state evidence, the /proc reads and the checkpoint-vs-session-file split are all confirmed. Three things hold exactly as you wrote them:

• require_foreground_terminal_owner() runs once at startup and has no runtime equivalent; there is no SIGTSTP/SIGTTIN/SIGCONT handler anywhere in the TUI.
• No cleanup runs on a kernel stop, so raw mode, the mouse modes, bracketed paste and the alternate screen all stay enabled, which is the shell poisoning.
• The in-flight turn lives only in the checkpoint, not the session file.

We went to verify the mechanism before writing the fix, and two of the load-bearing details are inverted. Posting them because the fix you sketched would land on the wrong line.

SIGTTIN is raised in event::poll(), not event::read()

The report says the poll/mio call on the tty is harmless and the read(2) inside event::read() is what the kernel answers with SIGTTIN. It is the other way round:

event::poll        (crossterm event.rs:204)
  -> poll_internal                 (event.rs:258)
    -> InternalEventReader::poll   (read.rs:41)
      -> event_source.try_read()   (read.rs:64)
        -> tty_fd.read(&buf)       (mio.rs:96)   <-- the tty read(2) lives here

event::read() then pops from crossterm's internal queue, which poll has already filled (read.rs:85-87, 99-107), so when poll returns Ok(true) the read() on terminal_input.rs:98 performs no tty syscall at all. The stop happens at terminal_input.rs:96.

So any signal handler, parking lever, or PTY assertion has to target the poll call site.

The stop happens while holding crossterm's global reader mutex — and that constrains the fix

crossterm serializes all reading behind one global INTERNAL_EVENT_READER mutex (event.rs:151-163), acquired in poll_internal at event.rs:264 before try_read. So the stopped pump is stopped holding it.

Two consequences:

1. restart_detached() does not create a second SIGTTIN-taking reader. The new pump's timed try_lock_internal_event_reader_for fails, poll_internal returns Ok(false) (event.rs:267), and control lands in the heartbeat branch (terminal_input.rs:101-113) which calls mark_alive(). That masks the stall detector rather than amplifying it. The recurrence you saw after bg comes from the old thread retrying its restarted read — detach_current_thread (terminal_input.rs:243-247) only sets a flag a stopped thread can never observe.

2. A "park the reader" handshake cannot park a thread already stopped inside the lock. pause_for_child_terminal (terminal_input.rs:199) works for a cooperative handoff, but it cannot rescue this case. The pump has to be stopped from entering poll while backgrounded — a check before the poll call, not a park after the fact. That constraint is the main reason this is bigger than adding a signal handler.

Still open

Whether the event loop reaches event_loop.rs:4493 at all during a background-stop cycle is not established from reading alone; on SIGCONT all threads resume and the pump re-stops the process, so the ordering needs a real repro rather than more code reading.

None of this changes your conclusion that the handshake is missing and that the terminal is left poisoned — that part is solid and is what we will fix. It changes where the fix goes. Thank you for the detail; it is what made tracing this tractable.

[Hmbown]

Follow-up on the fix design, after taking a second opinion on the job-control mechanics. Three things change what should be built, and one of them is a separate bug worth fixing on its own.

1. A foreground guard cannot be the fix — only a mitigation

The obvious fix is to check ownership before polling: tcgetpgrp(ctty_fd) (i.e. TIOCGPGRP) against getpgrp(), and skip the poll when they differ.

That is check-then-act and the race cannot be closed: the shell can background the group between the check and the read(2) inside poll. The read(2) itself is the atomic test for foreground — nothing else is. So the guard reduces how often SIGTTIN is taken; it never prevents it. It also does nothing for a thread already stopped inside poll holding crossterm's INTERNAL_EVENT_READER.

Also worth pinning down when this is written: the guard must query the controlling terminal, the same one crossterm's tty_fd uses. Using stdin is wrong when stdin is redirected.

2. Catching or ignoring SIGTTIN is the wrong direction

It is tempting to dodge the stop by handling or ignoring SIGTTIN. What actually happens: with SIGTTIN ignored or blocked (or in an orphaned process group), a background read from the controlling terminal sends no signal and read fails with EIO — no key bytes either way. With a handler installed you also get no input.

So it converts a clean stop into an error or a hot spin, breaks the shell contract that a backgrounded job stops, and fixes nothing about the mode leak this issue reports. Ignoring SIGTTIN is a daemon pattern; a full-screen TUI wants the opposite.

3. The correct shape is the vim/less suspend dance

Suspend (on SIGTSTP, and on SIGTTIN/SIGTTOU when they mean "you lost the terminal"):

1. Leave alternate screen; disable mouse ?1000/1002/1003/1006/1015; disable bracketed paste ?2004; restore saved termios; restore cursor; flush.
2. Reset SIGTSTP to SIG_DFL and unblock it.
3. raise(SIGTSTP) — this, not the kernel's default action mid-read, is what stops you.
4. Execution resumes at this point: reinstall handlers, and re-enter TUI modes only if foreground.

Resume: the shell does tcsetpgrp, never you. On SIGCONT, compare tcgetpgrp() to getpgrp(); if still background (plain bg), touch the terminal at all. Only when foreground: re-enter raw/alt-screen/mouse/paste, handle the resize, full repaint — the shell's output has clobbered the screen.

Ordering hazards, roughly in the order they bite:

1. Restore before stop. Restoring after the stop never runs — that is precisely this bug.
2. SIG_DFL before raise, or the handler recurses and never stops.
3. No terminal writes while backgrounded — that earns SIGTTOU and a second stop, or steals the shell's output.
4. Flush before stopping, or escapes interleave with shell output.
5. Async-signal-safety and thread choice. termios writes, ANSI output and repaint from inside a handler — or from a thread other than the one inside crossterm's mutex — is a deadlock/torn-output hazard. A dedicated signal thread (signalfd/sigwait) that arranges for the stop to happen outside the INTERNAL_EVENT_READER critical section is the shape to aim at. The exact safe coordination with crossterm's global mutex is the hard part and is not established.
6. Handling only SIGTSTP. The stop here arrives via SIGTTIN on a background read, so a SIGTSTP-only handler leaves this exact path broken.

4. A separate bug: the pump reports health when its reader is wedged

terminal_input.rs:96-113:

match event::poll(TERMINAL_INPUT_POLL_INTERVAL) {
    Ok(true)  => { /* read, refresh heartbeat */ }
    Ok(false) => { /* send Heartbeat -> mark_alive() */ }
}

Ok(false) means one of two very different things: no input arrived, or the timed attempt to acquire INTERNAL_EVENT_READER failed because another thread is inside it. The second is "the reader is wedged", and it is reported as health.

That is a liveness lie independent of job control: it masks every pump stall — deadlock, starvation, and this SIGTTIN stop alike. And the restart-detached path manufactures the condition, because a second pump can never win a mutex held by a stopped thread, so it heartbeats forever while the real reader is dead.

Worth fixing on its own, ahead of the signal work: only heartbeat on a cycle that actually attempted the read, and treat lock-contention Ok(false) as stall evidence rather than health. That gives a real tripwire while the longer suspend/restore work is done. It does not fix the mode leak, so (3) is still required.

Verified against this tree: the Ok(false) heartbeat branch is exactly as quoted. The signal-handling design above is standard practice rather than something measured here, and the crossterm-mutex coordination in hazard 5 is the part that still needs proving with a real repro.

[Hmbown]

One correction to point 4 above, found while scoping it — the heartbeat fix is real but it cannot be implemented the way I described.

crossterm::event::poll returns std::io::Result<bool> (crossterm 0.29.0, event.rs:204). It gives the caller no way to distinguish "the timeout elapsed with no input" from "the timed attempt to acquire INTERNAL_EVENT_READER failed". Both surface as Ok(false), and both take roughly the full timeout, so timing does not separate them either. So "only heartbeat on a cycle that actually attempted the read" is not something the pump can decide from what crossterm tells it.

What is still actionable, and narrower: a pump created by restart_detached knows it is a restart. If the previous pump is stopped inside the mutex, the new one can never win it, so every Ok(false) it sees is contention rather than idle — and it heartbeats anyway, which is what manufactures the false liveness. That pump should not be allowed to report health until it has successfully read at least one event; until then its Ok(false) is evidence of a wedged reader, not of an idle terminal.

That is a small change, and it stays true regardless of how the signal-handling work in point 3 lands.

Not starting it here: restart_detached is defined in terminal_input.rs but its only call sites are event_loop.rs:3324 and :4637, and event_loop.rs currently carries another agent's uncommitted work in this workspace. Queued rather than risk clobbering it.

(The alternative — upstreaming a crossterm API that separates contention from idle — is the more honest fix and much slower. Worth noting as the reason this is awkward: the liveness signal we need is one the dependency deliberately hides.)

[Hmbown]

Landed in 95bc5c008 — the stated expected behavior is implemented and proven on a real PTY.

What shipped: tui/ui/job_control_guard.rs — one async-signal-safe handler for SIGTSTP + SIGTTIN that writes the shared restore table (alt screen, mouse modes, bracketed paste, kitty stack), restores termios from the cooked snapshot with tcsetattr(TCSANOW), then raise(SIGSTOP) — never returns, so the SIGTTIN path cannot become the EIO dead-pump. SIGCONT is a single atomic store; the rebuild (raw + alt + modes + full repaint + "Resumed after suspend") runs on the event loop's next iteration in normal context, gated on foreground ownership (a plain bg must not re-steal the tty) and on the child-handoff pause. SIGTTOU is ignored at install time — without it, the background restore's tcsetattr stops the process inside the handler. Kill switch: CODEWHALE_DISABLE_JOB_CONTROL_GUARD=1.

Real-PTY evidence (built binary, artifacts/agent-reports/plans/6169-pty-smoke.py):

boot alt_on=True
SIGTSTP -> WIFSTOPPED=True stopsig=17 (SIGSTOP: the handler reached raise, not a default stop)
  restore bytes on the wire: alt_off=True mouse_off=True paste_off=True
SIGCONT -> repaint bytes, alt re-entered, process alive, shell sentinel on screen
SMOKE: PASS

Plus unit coverage for the state machine, bit ordering, and the byte-table reuse symmetry; fmt/check clean.

Deferred, filed as #6308: the keyboard Ctrl+Z binding (raw mode delivers byte 0x1A, not SIGTSTP) and the in-repo cucumber PTY test (the standalone smoke above proves the shape; #6308 asks for it in CI, and is where the background-SIGTTIN path gets driven live end-to-end).

The design constraints from the maintainer comments were honored as stated: no SIGTTIN ignore/daemon trick (the handler stops), no pre-poll tcgetpgrp guard (racy; the read(2) is the atomic test), and cleanup never needs crossterm's reader lock (raw write + tcsetattr only).