Skip to content

TUI: unguarded raiser-side multiplexer calls in action handlers can crash the app #143

Description

@pbean

Context

CodeRabbit round 2 on #137 found _attach_to_target calling runs.attach_target_argv() unguarded; 8ed66e8 fixed that one site (except MultiplexerError → self.notify(..., severity="error")). The class of bug is broader than the one call, though: any raiser-side multiplexer call inside a Textual action handler crashes the whole TUI when it raises — an unhandled exception in an action tears the app down and re-raises out of run(). On the tmux backend this is mostly theoretical (its argv-building methods can't raise), but herdr methods do real server round-trips and raise HerdrError whenever the server died / timed out / a workspace was torn down between the pre-gates and the call.

Known remaining crash path

action_attachlaunch.session_exists(session) (src/bmad_loop/tui/app.py:356) → has_session, which is raiser-side on transport failure (pinned by test_seam_methods_never_leak_raw_subprocess_error in tests/test_herdr_backend.py). A herdr server dying between the _mux_missing() PATH probe and this call crashes the TUI — exactly the trigger the 8ed66e8 guard handles one line further down.

What's already safe (no work needed)

  • Sentinel-side seam methods (select_window, set_window_option/unset_window_option, list_windows, kill_window, kill_session, switch_client, current_pane_id, list_sessions, session_options, …) degrade instead of raising, so select_ctl_window, set_return_pane, ctl_window, kill_ctl_window, the cleanup action, etc. are fine as-is.
  • The launch/resume/resolve flows route through launch.start_detached / _ensure_ctl_session, which already wrap MultiplexerErrorLaunchError, and the actions catch LaunchError.
  • The CLI needs nothing: cli.main's top-level backstop already surfaces error: <msg> + rc 1 (pinned by test_attach_multiplexer_error_surfaces_clean_error).

Proposed shape

A systematic "the TUI never crashes on MultiplexerError" pass over tui/app.py:

  1. Guard the known site (session_exists in action_attach) the same way as 8ed66e8, or hoist a small helper (e.g. _mux_guarded(fn) mirroring the existing _guarded) so action handlers convert MultiplexerError → error toast uniformly.
  2. Audit the remaining action handlers for raiser-side seam calls (has_session, list_window_ids, window_alive, new_*, send_text, attach_target_argv) reached outside a LaunchError/MultiplexerError wrapper; the raiser/sentinel split in the seam-leak test is the checklist.
  3. Optionally pin the convention with a test in tests/test_tui_app.py per guarded action (the test_attach_multiplexer_error_notifies recipe: monkeypatch the seam call to raise, drive the binding, assert the toast lands and the app survives).

Follow-up from the round-2 review discussion on #137 (see the resolved tui/app.py thread there). Depends on the #136#137#141#142 stack landing; the audit should run against the post-#142 names (_mux_missing, mux_available).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions