Terminal left in cbreak mode (no echo) after conductor run exits

[hertznsk]

Terminal left in cbreak mode (no echo) after conductor run exits

Symptom

After a conductor run <workflow.yaml> completes (exit code 0), the invoking terminal is left with -echo -icanon: typed characters are not displayed (input still works). Recovery requires typing reset or stty sane blindly.

Observed on Linux (kernel pty, tmux 3.7b and bare terminal), conductor-cli v0.1.20 (also reproduced on main @ 7aaa589), Python 3.12. The failure is intermittent — most runs restore the terminal correctly; some runs leave it broken.

Root cause

KeyboardListener._original_settings (the termios attrs to restore on exit) is captured once in start():

# src/conductor/interrupt/listener.py:106-111
try:
    self._original_settings = termios.tcgetattr(sys.stdin.fileno())
except termios.error:
    ...

but the listener lifecycle allows cbreak mode to be re-entered without re-capturing or invalidating the saved attrs:

• suspend() (called around gates/dialogs, e.g. engine/workflow.py _suspend_listener) restores the terminal via tcsetattr(TCSADRAIN, self._original_settings) but keeps _original_settings set (listener.py:157-185);
• resume() re-enters cbreak via tty.setcbreak() without re-reading the current attrs (listener.py:187-221);
• if start() or setcbreak is ever executed while the tty is already in cbreak (double listener instance, or a suspend/resume interleaving), tcgetattr captures the cbreak state itself as the "original".

stop() then "restores" by applying that captured cbreak state, and the atexit fallback is a no-op (_original_settings was already cleared), so the terminal stays at -echo -icanon.

Evidence (instrumented real run)

Monkey-patched termios.tcsetattr/KeyboardListener in a real conductor run of an 11-agent workflow. Broken run:

02:07:11  tcsetattr(0) TCSAFLUSH  lflag=0x8a31   ← start(): entering cbreak (echo/icanon OFF)
02:08:08  tcsetattr(0) TCSADRAIN  lflag=0x8a31   ← stop(): "restore" applies the SAME cbreak lflag
02:08:08  tcgetattr(0) → echo=False icanon=False ← terminal remains broken
02:08:11  ATEXIT → echo=False icanon=False       ← fallback handler is a no-op

Clean run (for comparison) — the restore applies the correct pre-cbreak attrs (lflag=0x8a3b) and the terminal survives:

02:17:27  tcsetattr(0) TCSADRAIN  lflag=0x8a3b   ← stop(): real restore
02:17:27  tcgetattr(0) → echo=True icanon=True

Reproduced 8 times across runs; the deciding factor is whether _original_settings still holds the true pre-cbreak attrs at stop() time.

Minimal unit demonstration of the capture bug:

# second start() while already in cbreak saves the cbreak state as "original"
l1 = KeyboardListener(interrupt_event=ev); await l1.start()   # saves sane attrs
l2 = KeyboardListener(interrupt_event=ev); await l2.start()   # saves cbreak attrs (lflag & ~ECHO & ~ICANON)
await l2.stop()   # "restores" cbreak — terminal stays broken
# → l2._original_settings shows echo=False icanon=False

Why current tests don't catch it

tests/test_interrupt/test_listener.py mocks termios (test_stop_restores_terminal asserts tcsetattr was called with the mock's settings) — it never exercises a real pty, so capturing a degraded state goes unnoticed.

Suggested fixes

1. Re-capture on every cbreak entry. Before each tty.setcbreak() (both start() and resume()), read tcgetattr and store it as the restore target only if the tty is not already in cbreak — or simpler: capture the initial attrs once per process (module-level), never overwrite them.
2. Invalidate on suspend. suspend() should clear _original_settings the same way stop() does, forcing resume() to re-capture.
3. Use TCSANOW instead of TCSADRAIN for restore so it cannot be delayed by output drain.
4. Add an integration test on a real pty (pty.fork()): start → suspend → resume → stop, then tcgetattr and assert ECHO|ICANON are set.

Workaround for users

reset or stty sane restores echo. Alternatively run conductor run --no-interactive when Esc-interrupt is not needed.