Panic surface: unreachable!() clusters and mutex-poison cascade #116

Status — partially changed, still open (reviewed 2026-08-22)

The terminal was split into PTY and grid components, the big chat.rs was split into chat/mod.rs + chat/model.rs + components, and several unrelated crash paths were hardened — but the invariant/panic sweep itself is not complete.

Current evidence on main (e8a5f1cc)

1. Invariant aborts

   • 8 production unreachable!() sites remain (test-module sites excluded): 5 in crates/ui/src/chat/mod.rs (302, 313, 375, 513, 778), plus crates/ui/src/chat/components/subagent.rs:39, crates/core/src/session.rs:1095, and crates/runtime/src/app/orchestrate.rs:931.
   • The earlier crates/agent/src/acp.rs unreachable!() cluster is gone, and the count is down from 13; the remaining sites still turn presentation- and session-state assumptions into process aborts.

2. Mutex poisoning

   • std::sync::Mutex with lock().unwrap() remains widespread: crates/agent/src/acp.rs (37 call sites), crates/term/src/grid_emulator.rs (9), crates/term/src/pty.rs (5), crates/term/src/pty_info.rs (1).
   • A panic while holding any of these shared locks can cascade into later UI/reader-thread panics.

3. Child-pipe assumptions

   • ACP launch still uses expect("piped stdin/stdout/stderr") after spawning the agent (crates/agent/src/acp.rs:327-329), and ACP terminal creation has the same stdout/stderr assumptions (acp.rs:2423-2424).
   • These should surface structured start/terminal errors rather than aborting.

Direction

• Replace externally reachable unreachable!() branches with logged fallback/error events; keep debug_assert! for developer visibility.
• Use non-poisoning mutexes where appropriate, or recover poison explicitly when continuing is safe.
• Convert child-pipe expect calls and counter-overflow expects into propagated errors.
• Add targeted tests that inject unexpected variants, poisoned locks, and missing child pipes.

Acceptance criteria

No provider output, recoverable process-launch failure, or terminal worker panic can abort the application solely through these clusters.

Effort: M · Risk: LOW–MED