From 749266489acd7170327824e0ecd8f7e5491b293f Mon Sep 17 00:00:00 2001 From: Ferrol Aderholdt Date: Wed, 29 Jul 2026 18:07:11 -0700 Subject: [PATCH] Fix opencode session binding, state accuracy, and TUI CPU burn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five related fixes, all around full-screen agent TUIs: - Two OpenCode sessions in one directory reported identical tokens, context and state: both watchers bound to the newest db session there. Watchers now claim a distinct one and release it when their session goes away — the watcher thread outlived its session and kept stealing the binding from the next opencode started in the same directory. - Sessions got stuck on WAITING indefinitely. A permission dialog is only visible to terminal patterns, so it has to outrank the watcher's Running/Thinking — but it was never given up, and the watcher only re-reported on a change. WAITING now releases when the dialog leaves the screen, when input is sent, or when the watcher reports the turn finished (re-asserted every 5s rather than 30s). Relatedly, a freshly started TUI session sat on RUNNING forever because the idle timeout keyed off the last completed output line, which a TUI that repaints via cursor positioning never produces. - Ever-worsening CPU burn: ~20% of a core per idle OpenCode session and still climbing. The PTY reader's unterminated-line buffer had no bound, and a TUI can repaint for minutes without a newline, so it grew without limit while being ANSI-stripped and regex-scanned every 20ms. The tail is now capped, unchanged tails are not re-scanned, and off-screen sessions skip the screen-change hash. Measured flat at ~8% of a core for two sessions afterwards. - Claude permission dialogs were not detected as WAITING: the dialog is drawn in a box, so lines arrive behind a border glyph, and the box often arrives as one blob with no newlines — neither of which the start-anchored patterns matched. - Removed the WAITING preview row under a waiting session in the status panel. The WAITING state itself is unchanged. Co-Authored-By: Claude --- CHANGELOG.md | 10 ++- src/app.rs | 198 +++++++++++++++++++++++++++++++++---------- src/opencode_log.rs | 174 ++++++++++++++++++++++++++++++-------- src/patterns.rs | 75 ++++++++++++++++- src/session.rs | 199 +++++++++++++++++++++++++++++++++++++------- src/ui.rs | 37 +------- 6 files changed, 548 insertions(+), 145 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b4c37d..c3dc206 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ ## Unreleased +- Fixed two OpenCode sessions in the same directory reporting identical tokens, context and state: both watchers bound to the newest db session in that directory. Each watcher now claims a distinct one, and releases it when its session goes away. +- Fixed OpenCode sessions getting stuck on WAITING indefinitely. A permission dialog is only visible to terminal pattern matching, so it has to outrank the db watcher's Running/Thinking — but it was never given up, and the watcher only re-reported on a *change*. WAITING now releases when the dialog leaves the screen, when input is sent to the session, or when the watcher reports the turn finished (now re-asserted every 5s rather than 30s). +- Fixed a freshly started full-screen TUI session (OpenCode) sitting on RUNNING forever: the idle-timeout check keyed off the last completed output line, which a TUI that repaints via cursor positioning never produces. +- Fixed ever-worsening CPU burn from full-screen TUI sessions — around 20% of a core per idle OpenCode session and still climbing. The PTY reader's unterminated-line buffer had no bound, and a TUI can repaint for minutes without a newline, so the buffer grew without limit while being ANSI-stripped and regex-scanned every 20ms. The tail is now capped, unchanged tails are not re-scanned, and off-screen sessions skip the screen-change hash. +- Fixed Claude permission dialogs not being detected as WAITING: the dialog is drawn inside a box, so every line arrives behind a `│` border, and the whole box often arrives as one blob with no newlines — neither of which the start-anchored patterns matched. +- Removed the WAITING preview row under a waiting session in the status panel; the WAITING state itself is unchanged. - Capability tokens are no longer minted from a silently-failed CSPRNG read. If `/dev/urandom` could not be opened or read the error was discarded and the buffer kept its zero initializer, so every token became 32 zeros — a predictable credential for reattach and for TCP agents. `mint_token` now propagates the failure, which surfaces as a session-spawn error rather than a weak token. Most likely to have bitten agents run inside a container or bubblewrap sandbox with no `/dev` bound. - A panic no longer leaves the terminal unusable. The relay client installs a panic hook that leaves the alternate screen, disables raw mode and mouse reporting, and pops kitty flags before the panic report prints, so the message lands on the normal screen instead of a shell with no echo. `SIGTERM`/`SIGHUP` are now handled the same way as a detach rather than killing the client mid-alternate-screen. - Fixed a crash when the command bar's slash-command popup was open on a short terminal: the popup claimed one row per match (up to 8) without checking how many rows were left above the bar, underflowing the row calculation and panicking inside ratatui's buffer indexing. The popup now yields rows to the bar, and terminals below 20x12 render a "terminal too small" placeholder instead of attempting a layout the solver can't satisfy. @@ -11,7 +17,7 @@ - A pipe's `Summarize` relay no longer stalls indefinitely against an unresponsive endpoint; the request now carries a 60s timeout, matching the bounds the orchestrator paths already set. - Fixed Escape keypresses being swallowed when followed quickly by another key (crossterm merges them into Alt+char): the ESC prefix is now forwarded to the PTY. Most visible in vim, where Esc then `:wq` typed fast left the session in insert mode with `:wq` inserted into the buffer. - `PageUp`/`PageDown` now scroll linkshell's captured scrollback in claude/codex panes (matching the mouse wheel) instead of being sent to the TUI, which ignored them. -- Fixed the whole UI shaking when a codex session flapped in and out of WAITING: the status panel's waiting-preview row now shrinks with a few seconds of hysteresis, breaking the resize→repaint→state-flap feedback loop. +- Fixed the whole UI shaking when a codex session flapped in and out of WAITING: the status panel now shrinks with a few seconds of hysteresis, breaking the resize→repaint→state-flap feedback loop. - Fixed missing token/context stats for resumed codex sessions (`codex resume` or the in-TUI picker): the rollout watcher now also picks up a pre-existing rollout file that starts being written after the session spawns. - Chat pane input: `Home`/`End`/`Delete` now work; `Up`/`Down` recall previously sent messages (draft preserved); typing `/` as the first character opens a filtering command popup (`Up`/`Down` select, `Tab` completes). - Added recursive split panes: any pane can be split side by side (`alt-\`) or top/bottom (`alt--`), repeatedly and in any direction, for arbitrary tiled layouts. `alt-w` closes the focused pane (its sibling reclaims the space), `alt-r` rotates a split, `alt-o` cycles focus. Replaces the previous two-pane toggle; keybinding actions are now `split_pane_right`, `split_pane_down`, `close_pane`, `rotate_split`, `focus_next_pane`. @@ -21,6 +27,6 @@ - Added split panes with independent focus and PTY sizing. - Added a fuzzy command palette and session-name completion. - Added pipe topology summaries and an interactive `pipes` overlay. -- Added WAITING previews and debounced desktop notifications. +- Added debounced desktop notifications. - Added diagnostics, recipes, and configuration documentation. diff --git a/src/app.rs b/src/app.rs index 1842543..83e0974 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1023,6 +1023,10 @@ impl App { // A stopped process can't react to the PTY closing; continue it first // so it actually dies instead of lingering as a stopped orphan. let _ = self.sessions[idx].set_paused(false); + // Retire the OpenCode db watcher so it releases its claim on the db + // session — otherwise it keeps taking it from the next opencode + // started in the same directory. + crate::opencode_log::stop_watcher(self.sessions[idx].id); // Drop the PTY write channel so the background task exits self.sessions[idx].pty_writer = None; self.sessions.remove(idx); @@ -1361,15 +1365,21 @@ impl App { /// especially) repaint continuously with byte-identical frames — gating on /// real change keeps the 60 fps render loop off the CPU while idle. pub fn handle_session_bytes(&mut self, session_id: usize, data: Vec) -> bool { - let mut changed = false; - if let Some(session) = self.sessions.iter_mut().find(|s| s.id == session_id) { - changed = session.process_bytes(&data); - } + // Only an on-screen session can warrant a redraw, so decide visibility + // first and let off-screen sessions skip the screen-diff hash entirely — + // that hash renders the whole formatted screen, and background TUIs + // (opencode) repaint often enough for it to dominate CPU time. + // // While the user is scrolled up, hold their position (tmux-style) // instead of yanking to the tail on every output burst — full-screen // TUIs redraw constantly, which previously made scrollback unusable // for them. Typing returns to the live view (see clear_scroll). - changed && self.session_is_visible(session_id) + let visible = self.session_is_visible(session_id); + let mut changed = false; + if let Some(session) = self.sessions.iter_mut().find(|s| s.id == session_id) { + changed = session.process_bytes(&data, visible); + } + changed } pub fn handle_session_output(&mut self, session_id: usize, line: String) { @@ -1395,11 +1405,7 @@ impl App { // When JSONL is active it owns Thinking/Running/Ready transitions, but it // cannot see permission prompts or question text, so Waiting and Error must // still come from terminal pattern matching. - if !session.ipc_state - || matches!(new_state, SessionState::Waiting | SessionState::Error) - { - session.state = new_state; - } + session.apply_pattern_state(new_state); } if state_before.as_ref() != Some(&session.state) { session.waiting_prompt = if session.state == SessionState::Waiting { @@ -1738,11 +1744,8 @@ impl App { // flip to Running — that requires a complete line. // Even with JSONL active, Waiting and Error must pass through because // JSONL has no record for permission prompts or question text. - if new_state != SessionState::Running - && (!session.ipc_state - || matches!(new_state, SessionState::Waiting | SessionState::Error)) - { - session.state = new_state; + if new_state != SessionState::Running { + session.apply_pattern_state(new_state); } } if state_before.as_ref() != Some(&session.state) { @@ -1782,14 +1785,17 @@ impl App { .find(|s| s.id == session_id) .map(|s| s.state.clone()); if let Some(session) = self.sessions.iter_mut().find(|s| s.id == session_id) { - session.state = state.clone(); - session.waiting_prompt = if state == SessionState::Waiting { + // A pattern-detected permission dialog outranks a watcher's + // Running/Thinking (see apply_watcher_state), so the reported state + // is not always the visible one. + if !session.apply_watcher_state(state.clone()) { + return; + } + session.waiting_prompt = if session.state == SessionState::Waiting { extract_waiting_prompt(&session.output_lines) } else { None }; - session.ipc_state = true; - session.ipc_state_set_at = Some(std::time::Instant::now()); } if old.as_ref() != Some(&state) { match old { @@ -2733,26 +2739,30 @@ impl App { if expired { session.ipc_state = false; session.ipc_state_set_at = None; + session.pattern_waiting = false; let before = session.state.clone(); session.state = SessionState::Ready; tick_ready.push((session.id, before)); } } if !session.ipc_state { - if let Some(last) = session.last_output_at { - let elapsed = last.elapsed(); - if (elapsed > Duration::from_secs(2) - && matches!( - session.state, - SessionState::Running | SessionState::Thinking - )) - || (elapsed > Duration::from_secs(30) - && session.state == SessionState::Waiting) - { - let before = session.state.clone(); - session.state = SessionState::Ready; - tick_ready.push((session.id, before)); - } + // Fall back to the launch time: a full-screen TUI can repaint + // for its whole life without ever completing a line, leaving + // last_output_at unset — and the session parked on the + // STARTING/RUNNING it was born with. + let last = session.last_output_at.unwrap_or(session.started_at); + let elapsed = last.elapsed(); + if (elapsed > Duration::from_secs(2) + && matches!( + session.state, + SessionState::Running | SessionState::Thinking + )) + || (elapsed > Duration::from_secs(30) && session.state == SessionState::Waiting) + { + let before = session.state.clone(); + session.pattern_waiting = false; + session.state = SessionState::Ready; + tick_ready.push((session.id, before)); } } } @@ -4208,18 +4218,16 @@ impl App { // Typing returns the view to the live tail. self.clear_scroll(); if self.broadcast_mode { - let ids: Vec = self + for s in self .sessions - .iter() + .iter_mut() .filter(|s| s.state != SessionState::Dead) - .map(|s| s.id) - .collect(); - for id in ids { - if let Some(s) = self.sessions.iter().find(|s| s.id == id) { - s.write_bytes(data.to_vec()); - } + { + s.note_input_sent(); + s.write_bytes(data.to_vec()); } - } else if let Some(session) = self.active_session() { + } else if let Some(session) = self.active_session_mut() { + session.note_input_sent(); session.write_bytes(data.to_vec()); } } @@ -4850,7 +4858,7 @@ impl App { } }, }; - let Some(s) = self.sessions.iter().find(|s| s.id == id) else { + let Some(s) = self.sessions.iter_mut().find(|s| s.id == id) else { return; }; if s.state != SessionState::Waiting { @@ -4869,6 +4877,7 @@ impl App { (_, true) => b"y\r", (_, false) => b"n\r", }; + s.note_input_sent(); s.write_bytes(bytes.to_vec()); let name = s.name.clone(); if self.last_permission_request == Some(id) { @@ -5566,6 +5575,13 @@ async fn run_pty( let mut reader = read_half; let mut buf = [0u8; 4096]; let mut pending = String::new(); + // Last partial we sent for state inference, and when. A repainting TUI + // re-emits the same tail 50 times a second; re-inferring from identical + // text costs an ANSI strip plus a regex sweep each time and can only + // produce the same answer. Unchanged text is still re-sent once a second so + // a state the app changed elsewhere (an expired override) gets corrected. + let mut last_partial = String::new(); + let mut last_partial_at = std::time::Instant::now(); loop { match tokio::time::timeout(std::time::Duration::from_millis(20), reader.read(&mut buf)) @@ -5595,6 +5611,7 @@ async fn run_pty( pending.push_str(&String::from_utf8_lossy(&data)); let (complete, partial) = split_pty_lines(&pending); pending = partial; + truncate_pending(&mut pending); for line in complete { if tx .send(AppEvent::SessionOutput { session_id, line }) @@ -5606,8 +5623,14 @@ async fn run_pty( } } Ok(Err(_)) => break, - // Timeout: update current-line display without adding to the line buffer + // Timeout: re-infer state from the unterminated tail, which is + // where a TUI's prompts and dialogs live. Err(_) => { + if pending == last_partial + && last_partial_at.elapsed() < std::time::Duration::from_secs(1) + { + continue; + } if tx .send(AppEvent::SessionCurrentLine { session_id, @@ -5618,6 +5641,8 @@ async fn run_pty( { return Ok(()); } + last_partial.clone_from(&pending); + last_partial_at = std::time::Instant::now(); } } } @@ -5626,6 +5651,30 @@ async fn run_pty( Ok(()) } +/// Cap on the unterminated tail the PTY reader carries between chunks. +/// +/// Full-screen TUIs (opencode above all) repaint via cursor positioning and can +/// run for minutes without emitting a single newline or carriage return, so the +/// partial buffer grows without bound — and it is cloned, ANSI-stripped and +/// regex-scanned every 20ms for state inference, which turns into an +/// ever-worsening CPU burn (measured climbing past 20% of a core on one idle +/// opencode session). Only the tail is meaningful for inferring the state of +/// what's on screen now, so drop the rest. Big enough to hold a repainted +/// permission dialog, which Claude sends as one blob. +const MAX_PENDING_BYTES: usize = 8 * 1024; + +/// Trim `pending` to the last [`MAX_PENDING_BYTES`], on a char boundary. +fn truncate_pending(pending: &mut String) { + if pending.len() <= MAX_PENDING_BYTES { + return; + } + let mut cut = pending.len() - MAX_PENDING_BYTES; + while cut < pending.len() && !pending.is_char_boundary(cut) { + cut += 1; + } + pending.drain(..cut); +} + /// Split PTY bytes into complete lines and a leftover partial. /// Handles \r\n (line end), \n (line end), and bare \r (carriage return — /// overwrite current line). A trailing \r is deferred until the next chunk @@ -6025,6 +6074,41 @@ mod tests { assert_eq!(app.sessions[0].waiting_prompt, None); } + #[test] + fn watched_session_holds_waiting_through_the_turn_then_releases_on_ready() { + let mut app = make_app(); + let id = app.spawn_headless_session("agent".into(), None).unwrap(); + + // A db/JSONL watcher owns the ordinary states. + app.handle_ipc_state(id, SessionState::Running); + // A permission dialog only the terminal can see. + app.handle_session_output(id, "Should I apply this change? [y/n]".into()); + assert_eq!(app.sessions[0].state, SessionState::Waiting); + + // The watcher keeps reporting the in-flight turn; the dialog wins. + app.handle_ipc_state(id, SessionState::Running); + assert_eq!(app.sessions[0].state, SessionState::Waiting); + + // Once the turn completes, the dialog is stale and WAITING is released + // — without this the session parked on WAITING indefinitely. + app.handle_ipc_state(id, SessionState::Ready); + assert_eq!(app.sessions[0].state, SessionState::Ready); + assert_eq!(app.sessions[0].waiting_prompt, None); + } + + #[test] + fn answering_a_dialog_lets_watcher_states_flow_again() { + let mut app = make_app(); + let id = app.spawn_headless_session("agent".into(), None).unwrap(); + app.handle_ipc_state(id, SessionState::Running); + app.handle_session_output(id, "Should I apply this change? [y/n]".into()); + assert_eq!(app.sessions[0].state, SessionState::Waiting); + + app.sessions[0].note_input_sent(); + app.handle_ipc_state(id, SessionState::Running); + assert_eq!(app.sessions[0].state, SessionState::Running); + } + #[test] fn notifications_respect_state_age_and_per_session_debounce() { let mut config = Config::default(); @@ -7070,6 +7154,30 @@ mod tests { assert_eq!(resp["session_id"], id); } + #[test] + fn truncate_pending_bounds_a_newline_less_repaint_stream() { + let mut pending = String::new(); + // A TUI repainting via cursor positioning, never ending a line. + for i in 0..4000 { + pending.push_str(&format!("\x1b[H\x1b[2Jframe {i}")); + truncate_pending(&mut pending); + } + assert!(pending.len() <= MAX_PENDING_BYTES); + // The newest frame is what state inference needs, and it survives. + assert!(pending.ends_with("frame 3999")); + + // Multi-byte characters are never split mid-sequence. + let mut wide = "→".repeat(MAX_PENDING_BYTES); + truncate_pending(&mut wide); + assert!(wide.len() <= MAX_PENDING_BYTES); + assert!(wide.chars().all(|c| c == '→')); + + // Short buffers are left exactly as they are. + let mut short = "❯ 1. Yes".to_string(); + truncate_pending(&mut short); + assert_eq!(short, "❯ 1. Yes"); + } + #[test] fn split_pty_lines_handles_lf_crlf_bare_cr_and_deferred_cr() { let (lines, pending) = split_pty_lines("one\ntwo\r\nspinner 1\rspinner 2\r"); @@ -7216,7 +7324,7 @@ mod tests { s.push_output_line(format!("history-{}", i)); } // Enter the alternate screen, like claude/codex/opencode do. - s.process_bytes(b"\x1b[?1049h"); + s.process_bytes(b"\x1b[?1049h", true); assert!(s.screen.screen().alternate_screen()); } // Scrolling a full-screen app walks our captured history… diff --git a/src/opencode_log.rs b/src/opencode_log.rs index 612cb34..7dd5216 100644 --- a/src/opencode_log.rs +++ b/src/opencode_log.rs @@ -4,6 +4,7 @@ /// `session` table and per-message usage JSON on the `message` table; the /// latest assistant message's input-side tokens reflect the current context /// window size. +use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; use std::time::Duration; @@ -30,18 +31,86 @@ fn open_db(path: &Path) -> Option { Some(conn) } +/// Which linkshell session each OpenCode db session is bound to. Two linkshell +/// sessions running `opencode` in the same directory would otherwise both bind +/// to the newest db session and report identical tokens/context/state. +static CLAIMS: std::sync::Mutex>> = std::sync::Mutex::new(None); + +fn claims() -> std::sync::MutexGuard<'static, Option>> { + let mut guard = CLAIMS.lock().unwrap_or_else(|e| e.into_inner()); + guard.get_or_insert_with(HashMap::new); + guard +} + +/// Claim `db_session` for `owner`, unless another linkshell session holds it. +fn claim(db_session: &str, owner: usize) -> bool { + let mut guard = claims(); + let map = guard.as_mut().expect("initialized by claims()"); + match map.get(db_session) { + Some(held) if *held != owner => false, + _ => { + map.retain(|_, held| *held != owner); + map.insert(db_session.to_string(), owner); + true + } + } +} + +fn release(owner: usize) { + let mut guard = claims(); + guard + .as_mut() + .expect("initialized by claims()") + .retain(|_, held| *held != owner); +} + +/// linkshell sessions whose watcher should stop. Watcher threads outlive their +/// session otherwise (they only notice the app shutting down), and a leaked one +/// keeps re-claiming the newest db session away from a freshly started +/// OpenCode in the same directory. Session ids are never reused, so retired +/// entries stay valid for the life of the server. +static RETIRED: std::sync::Mutex>> = std::sync::Mutex::new(None); + +/// Stop the watcher bound to a linkshell session, if any. Called when the +/// session goes away. +pub fn stop_watcher(owner: usize) { + RETIRED + .lock() + .unwrap_or_else(|e| e.into_inner()) + .get_or_insert_with(HashSet::new) + .insert(owner); + release(owner); +} + +fn is_retired(owner: usize) -> bool { + RETIRED + .lock() + .unwrap_or_else(|e| e.into_inner()) + .as_ref() + .is_some_and(|set| set.contains(&owner)) +} + /// Most recently active top-level OpenCode session in `dir` touched since -/// `since_ms` (epoch millis). Subagent sessions carry a parent_id and are -/// excluded — the TUI's status line reflects the parent session. -fn find_session(conn: &Connection, dir: &str, since_ms: i64) -> Option { - conn.query_row( - "SELECT id FROM session \ - WHERE directory = ?1 AND parent_id IS NULL AND time_updated >= ?2 \ - ORDER BY time_updated DESC LIMIT 1", - rusqlite::params![dir, since_ms], - |row| row.get(0), - ) - .ok() +/// `since_ms` (epoch millis) and not already claimed by another linkshell +/// session. Subagent sessions carry a parent_id and are excluded — the TUI's +/// status line reflects the parent session. +fn find_session(conn: &Connection, dir: &str, since_ms: i64, owner: usize) -> Option { + let mut stmt = conn + .prepare( + "SELECT id FROM session \ + WHERE directory = ?1 AND parent_id IS NULL AND time_updated >= ?2 \ + ORDER BY time_updated DESC LIMIT ?3", + ) + .ok()?; + let candidates: Vec = stmt + .query_map( + rusqlite::params![dir, since_ms, crate::session::MAX_SESSIONS as i64], + |row| row.get::<_, String>(0), + ) + .ok()? + .flatten() + .collect(); + candidates.into_iter().find(|id| claim(id, owner)) } /// The session.model column holds JSON like @@ -224,7 +293,7 @@ pub fn spawn_watcher(session_id: usize, cwd: String, tx: tokio::sync::mpsc::Send let mut last_state: Option = None; let mut state_sent_at = std::time::Instant::now(); - while !tx.is_closed() { + 'watch: while !tx.is_closed() && !is_retired(session_id) { std::thread::sleep(Duration::from_millis(1000)); if conn.is_none() { @@ -236,23 +305,26 @@ pub fn spawn_watcher(session_id: usize, cwd: String, tx: tokio::sync::mpsc::Send let Some(c) = conn.as_ref() else { continue }; // Re-query the binding each poll: the user can switch sessions - // inside the OpenCode TUI, and the newest active one wins. - if let Some(id) = find_session(c, &dir, since_ms) { + // inside the OpenCode TUI, and the newest active one we can claim + // wins. + if let Some(id) = find_session(c, &dir, since_ms, session_id) { bound = Some(id); } let Some(id) = bound.as_ref() else { continue }; // State first: it must flow even before any tokens are recorded // (read_stats bails on all-zero sessions). - // Re-assert an unchanged Ready every 30s: ipc_state overrides - // expire (ipc_state_override_timeout_secs, default 60s), and once - // one lapses the pattern path could re-park an idle session on - // Thinking from TUI redraw noise. Only Ready is refreshed — - // re-asserting Running would stomp a pattern-detected Waiting - // (permission dialog) that the DB can't see. + // Re-assert the current state every REFRESH_SECS: ipc_state + // overrides expire (ipc_state_override_timeout_secs, default 60s), + // and once one lapses the pattern path could re-park an idle + // session on Thinking from TUI redraw noise. The refresh is also + // the only thing that releases a stale pattern-detected WAITING + // (Session::apply_watcher_state holds WAITING against a + // Running/Thinking report but yields to Ready), so it must be + // frequent enough not to strand the session for a visible while. + const REFRESH_SECS: u64 = 5; if let Some(state) = read_latest_state(c, id) { - let refresh = state == SessionState::Ready - && state_sent_at.elapsed() > Duration::from_secs(30); + let refresh = state_sent_at.elapsed() > Duration::from_secs(REFRESH_SECS); if last_state.as_ref() != Some(&state) || refresh { if tx .blocking_send(AppEvent::IpcStateOverride { @@ -261,7 +333,7 @@ pub fn spawn_watcher(session_id: usize, cwd: String, tx: tokio::sync::mpsc::Send }) .is_err() { - return; + break 'watch; } last_state = Some(state); state_sent_at = std::time::Instant::now(); @@ -281,7 +353,7 @@ pub fn spawn_watcher(session_id: usize, cwd: String, tx: tokio::sync::mpsc::Send }) .is_err() { - return; + break 'watch; } last_provider = Some(provider); } @@ -296,7 +368,7 @@ pub fn spawn_watcher(session_id: usize, cwd: String, tx: tokio::sync::mpsc::Send }) .is_err() { - return; + break 'watch; } last_model = Some(model); } @@ -310,11 +382,12 @@ pub fn spawn_watcher(session_id: usize, cwd: String, tx: tokio::sync::mpsc::Send }) .is_err() { - return; + break 'watch; } last_stats = Some(stats); } } + release(session_id); }); } @@ -361,20 +434,55 @@ mod tests { .unwrap(); } + // CLAIMS is process-global, so each test uses its own owner ids and its own + // db session ids to stay independent under the parallel test runner. #[test] fn finds_newest_top_level_session_in_directory() { let conn = test_db(); - insert_session(&conn, "ses_old", "/home/u/proj", None, 100); - insert_session(&conn, "ses_new", "/home/u/proj", None, 200); - insert_session(&conn, "ses_sub", "/home/u/proj", Some("ses_new"), 300); - insert_session(&conn, "ses_other", "/home/u/other", None, 400); + insert_session(&conn, "a_old", "/home/u/proj", None, 100); + insert_session(&conn, "a_new", "/home/u/proj", None, 200); + insert_session(&conn, "a_sub", "/home/u/proj", Some("a_new"), 300); + insert_session(&conn, "a_other", "/home/u/other", None, 400); assert_eq!( - find_session(&conn, "/home/u/proj", 150).as_deref(), - Some("ses_new") + find_session(&conn, "/home/u/proj", 150, 900).as_deref(), + Some("a_new") + ); + // Re-binding the same owner keeps the same session, not the runner-up. + assert_eq!( + find_session(&conn, "/home/u/proj", 150, 900).as_deref(), + Some("a_new") ); // Nothing active since the launch timestamp → no binding. - assert!(find_session(&conn, "/home/u/proj", 250).is_none()); + assert!(find_session(&conn, "/home/u/proj", 250, 900).is_none()); + release(900); + } + + #[test] + fn two_watchers_in_one_directory_bind_to_different_sessions() { + let conn = test_db(); + insert_session(&conn, "b_old", "/home/u/two", None, 100); + insert_session(&conn, "b_new", "/home/u/two", None, 200); + + assert_eq!( + find_session(&conn, "/home/u/two", 50, 910).as_deref(), + Some("b_new") + ); + assert_eq!( + find_session(&conn, "/home/u/two", 50, 911).as_deref(), + Some("b_old") + ); + // A third has nothing left to claim rather than duplicating a binding. + assert!(find_session(&conn, "/home/u/two", 50, 912).is_none()); + + // Retiring the first watcher frees its session for the newcomer. + stop_watcher(910); + assert_eq!( + find_session(&conn, "/home/u/two", 50, 912).as_deref(), + Some("b_new") + ); + release(911); + release(912); } #[test] diff --git a/src/patterns.rs b/src/patterns.rs index d6958ba..cb88490 100644 --- a/src/patterns.rs +++ b/src/patterns.rs @@ -36,8 +36,15 @@ impl PatternMatcher { r"⠋|⠙|⠹|⠸|⠼|⠴|⠦|⠧|⠇|⠏|(?i)\b(thinking|working|generating|reasoning)\b\.\.\.|esc to interrupt|ctrl\+c to interrupt", ) .unwrap(), - // idle prompt markers used by local agent TUIs and llama-cli - local_ready: Regex::new(r"^[>❯]\s*$|^\(\S+\)>\s*$").unwrap(), + // Idle prompt markers used by local agent TUIs and llama-cli. + // Border-tolerant and multi-line for the same reason as + // claude_waiting: opencode draws its input box with `│` edges, so + // its idle prompt is `│ > │`, not a bare `>` on its own line. That + // Ready signal is what releases a stale pattern WAITING. + local_ready: Regex::new( + r"(?m)^\s*[│┃|]?\s*[>❯]\s*[│┃|]?\s*$|^\s*\(\S+\)>\s*$", + ) + .unwrap(), // opencode permission dialog ("△ Permission required" with // Allow once / Allow always / Reject options) and aider's // (Y)es/(N)o confirmation prompts. @@ -53,8 +60,15 @@ impl PatternMatcher { // list) — none of which the legacy y/n markers match. The ❯ // selector before a numbered item is required so ordinary // numbered lists in Claude's prose don't read as dialogs. + // + // Those menus are drawn inside a rounded box, so every line arrives + // prefixed by a `│` border — and because Claude repaints via cursor + // positioning rather than newlines, several of them arrive in one + // partial-line blob. Hence `(?m)` plus a border-tolerant start + // anchor; a plain `^\s*` matched neither case and dialogs went + // undetected. claude_waiting: Regex::new( - r"\[y/n\]|\[Y/n\]|\(yes/no\)|Press Enter|continue\?|proceed\?|^\s*❯\s*\d+\.\s|(?i)^\s*do you want\b|(?i)don't ask again", + r"(?m)\[y/n\]|\[Y/n\]|\(yes/no\)|Press Enter|continue\?|proceed\?|(?:^|[│┃|])\s*❯\s*\d+\.\s|(?i)(?:^|[│┃|])\s*do you want\b|(?i)don't ask again", ) .unwrap(), codex_ready: Regex::new(r"codex>\s*$|>\s*$").unwrap(), @@ -410,6 +424,61 @@ mod tests { ); } + #[test] + fn claude_dialogs_inside_a_box_border_are_waiting() { + let matcher = PatternMatcher::new(); + + // Claude Code draws permission dialogs in a rounded box, so each line + // reaches us behind a `│`, and the whole box often arrives as a single + // repaint blob with no line of its own. + for line in [ + "│ Do you want to make this edit to app.rs? │", + "│ ❯ 1. Yes │", + "│ 2. Yes, and don't ask again this session │", + ] { + assert_eq!( + matcher.infer_state(line, BaseKind::Claude), + Some(SessionState::Waiting), + "{line}" + ); + } + + let blob = "╭──────────────────────╮\n\ + │ Edit file │\n\ + │ Do you want to edit? │\n\ + │ ❯ 1. Yes │\n\ + ╰──────────────────────╯"; + assert_eq!( + matcher.infer_state(blob, BaseKind::Claude), + Some(SessionState::Waiting) + ); + + // Prose that merely mentions a numbered plan is still not a dialog. + assert_eq!( + matcher.infer_state("│ 1. First, refactor the parser │", BaseKind::Claude), + Some(SessionState::Running) + ); + } + + #[test] + fn local_agent_boxed_input_prompt_is_ready() { + let matcher = PatternMatcher::new(); + + // opencode's input box: the idle prompt sits between border glyphs. + for line in ["│ > │", "│ ❯ │", "> "] { + assert_eq!( + matcher.infer_state(line, BaseKind::LocalAgent), + Some(SessionState::Ready), + "{line}" + ); + } + // A line with real content next to the prompt is not an idle prompt. + assert_eq!( + matcher.infer_state("│ > fix the parser │", BaseKind::LocalAgent), + Some(SessionState::Running) + ); + } + #[test] fn parse_context_max_reads_llama_nctx_line() { let matcher = PatternMatcher::new(); diff --git a/src/session.rs b/src/session.rs index 8ab5f64..eff0b02 100644 --- a/src/session.rs +++ b/src/session.rs @@ -252,6 +252,16 @@ pub struct Session { pub ipc_state: bool, /// When ipc_state was last set; used to expire stale overrides in handle_tick. pub ipc_state_set_at: Option, + /// The current WAITING state came from terminal pattern matching (a + /// permission dialog / question on screen), not from a watcher or IPC. + /// Dialogs are invisible to the JSONL/db watchers, so this state has to + /// survive their Running/Thinking reports — but it must also be *given + /// up* once the dialog is gone, or the session parks on WAITING forever. + pub pattern_waiting: bool, + /// Last state reported by an authoritative source (JSONL/db watcher or an + /// IPC `state` message). Restored when a pattern-derived WAITING clears, + /// so the session doesn't fall back to a stale terminal-scraped guess. + pub watcher_state: Option, /// Agent group for broadcast addressing and group-triggered pipes. pub group: Option, /// vt100 screen buffer — updated with raw PTY bytes, used for display @@ -355,6 +365,8 @@ impl Session { paused: false, ipc_state: false, ipc_state_set_at: None, + pattern_waiting: false, + watcher_state: None, group: None, screen: vt100::Parser::new(rows, cols, 1000), stats: TokenStats::default(), @@ -378,22 +390,20 @@ impl Session { } } - /// Feed raw PTY bytes into the vt100 screen. Returns `true` if the visible - /// frame changed as a result — callers use this to skip redundant redraws - /// for full-screen TUIs that repaint with byte-identical output. - pub fn process_bytes(&mut self, data: &[u8]) -> bool { + /// Feed raw PTY bytes into the vt100 screen. When `detect_change`, returns + /// `true` if the visible frame changed as a result — callers use this to + /// skip redundant redraws for full-screen TUIs that repaint with + /// byte-identical output. The check hashes the whole formatted screen, so + /// callers that cannot act on the answer (off-screen sessions) pass `false` + /// and always get `false` back. + pub fn process_bytes(&mut self, data: &[u8], detect_change: bool) -> bool { use std::hash::{Hash, Hasher}; // Capture top-line snapshot BEFORE processing, if this session kind // supports alt-screen scrollback capture and we're on the alternate screen. let prev_top = if self.kind.captures_alt_scrollback() && self.screen.screen().alternate_screen() { - self.screen - .screen() - .contents() - .lines() - .next() - .map(|s| s.trim_end().trim_end_matches('\t').to_string()) + self.top_row() } else { None }; @@ -403,13 +413,7 @@ impl Session { // After processing, check if content scrolled (top row changed) if let Some(ref prev) = prev_top { - let current_top: Option = self - .screen - .screen() - .contents() - .lines() - .next() - .map(|s| s.trim_end().trim_end_matches('\t').to_string()); + let current_top: Option = self.top_row(); // If the top line changed, content scrolled upward. The old top row // was pushed off-screen — capture it for scrollback. @@ -428,6 +432,11 @@ impl Session { } } } + if !detect_change { + // Leave last_screen_hash stale on purpose: the first chunk after the + // session becomes visible again then reads as changed and redraws. + return false; + } // contents_formatted is exactly what the display path renders from, so // an unchanged hash means the next frame would be pixel-identical. let mut hasher = std::collections::hash_map::DefaultHasher::new(); @@ -438,6 +447,18 @@ impl Session { changed } + /// Text of the screen's top row. `Screen::rows` is a lazy iterator, so this + /// renders one row — unlike `contents()`, which builds the entire screen + /// into a String and was being called twice per PTY chunk. + fn top_row(&self) -> Option { + let cols = self.screen.screen().size().1; + self.screen + .screen() + .rows(0, cols) + .next() + .map(|s| s.trim_end().trim_end_matches('\t').to_string()) + } + pub fn resize_screen(&mut self, rows: u16, cols: u16) { self.screen.set_size(rows, cols); } @@ -475,6 +496,58 @@ impl Session { lines } + /// Apply a state inferred from terminal output. + /// + /// Terminal patterns are the only source that can see permission dialogs + /// and questions, so WAITING/ERROR from them outrank a watcher's + /// Running/Thinking. The flip side is that a pattern WAITING must release + /// as soon as the screen stops showing the dialog — otherwise a session + /// whose watcher never reports another *change* parks on WAITING forever. + /// + /// Callers working from partial lines drop RUNNING before calling: there, + /// "non-empty text" is too weak a signal to declare a session running. + pub fn apply_pattern_state(&mut self, new_state: SessionState) { + match new_state { + SessionState::Waiting | SessionState::Error => { + self.pattern_waiting = new_state == SessionState::Waiting; + self.state = new_state; + } + // Ready/Thinking are positive evidence that the dialog is gone. + // RUNNING is not: a repaint of an unrelated screen region also + // reads as RUNNING, and clearing on that flickers the dialog away. + SessionState::Ready | SessionState::Thinking if self.pattern_waiting => { + self.pattern_waiting = false; + self.state = self.watcher_state.clone().unwrap_or(new_state); + } + _ if !self.ipc_state => { + self.pattern_waiting = false; + self.state = new_state; + } + _ => {} + } + } + + /// Apply a state from an authoritative source (JSONL/db watcher or IPC). + /// Returns false if the report was *suppressed* rather than applied. + /// Running/Thinking are recorded + /// but not shown while a pattern-detected dialog is up — the watcher cannot + /// see the dialog, and the CLI is genuinely mid-turn *and* blocked on the + /// user. Any other report (Ready/Error/Dead/an explicit Waiting) means the + /// dialog is resolved or superseded. + pub fn apply_watcher_state(&mut self, new_state: SessionState) -> bool { + self.watcher_state = Some(new_state.clone()); + self.ipc_state = true; + self.ipc_state_set_at = Some(Instant::now()); + if self.pattern_waiting + && matches!(new_state, SessionState::Running | SessionState::Thinking) + { + return false; + } + self.pattern_waiting = false; + self.state = new_state; + true + } + /// State text for display and reporting: paused sessions read PAUSED /// regardless of the frozen underlying state. pub fn state_label(&self) -> &str { @@ -577,6 +650,14 @@ impl Session { } } + /// Note that input was sent to this session. Whatever dialog the screen was + /// showing has now been answered, so a pattern-detected WAITING must stop + /// outranking watcher reports — otherwise the session reads WAITING for the + /// whole tool run that follows the approval. + pub fn note_input_sent(&mut self) { + self.pattern_waiting = false; + } + /// Non-blocking send to PTY; silently drops if channel is full or closed pub fn write_bytes(&self, data: Vec) { if let Some(tx) = &self.pty_writer { @@ -772,7 +853,7 @@ mod tests { let mut s = session(SessionKind::Claude); // TUI-style output: enter the alternate screen, then paint with // cursor positioning — no newline-terminated lines ever arrive. - s.process_bytes(b"\x1b[?1049h\x1b[1;1Hfirst row\x1b[2;1Hsecond row"); + s.process_bytes(b"\x1b[?1049h\x1b[1;1Hfirst row\x1b[2;1Hsecond row", true); assert!(s.output_lines.is_empty()); let tail = s.read_tail(50); @@ -814,7 +895,7 @@ mod tests { fn process_bytes_updates_vt100_screen_and_tick_counter() { let mut s = session(SessionKind::Shell); - s.process_bytes(b"hello"); + s.process_bytes(b"hello", true); assert_eq!(s.bytes_since_last_tick, 5); assert_eq!(s.screen.screen().contents().trim(), "hello"); @@ -825,11 +906,70 @@ mod tests { let mut s = session(SessionKind::Shell); // First paint changes the (blank) screen. - assert!(s.process_bytes(b"hello")); + assert!(s.process_bytes(b"hello", true)); // Re-emitting an identical frame (as full-screen TUIs do) is a no-op. - assert!(!s.process_bytes(b"\x1b[1;1Hhello")); + assert!(!s.process_bytes(b"\x1b[1;1Hhello", true)); // Actual new content changes the frame again. - assert!(s.process_bytes(b" world")); + assert!(s.process_bytes(b" world", true)); + } + + #[test] + fn process_bytes_skips_change_detection_when_asked() { + let mut s = session(SessionKind::Shell); + + // Off-screen sessions never report a change (nothing could redraw)... + assert!(!s.process_bytes(b"hello", false)); + // ...and the first check after becoming visible sees the difference. + assert!(s.process_bytes(b"", true)); + } + + #[test] + fn pattern_waiting_outranks_watcher_until_the_dialog_clears() { + let mut s = session(SessionKind::OpenCode); + + // A watcher owns the ordinary states. + assert!(s.apply_watcher_state(SessionState::Running)); + assert_eq!(s.state, SessionState::Running); + + // A dialog the watcher cannot see takes over... + s.apply_pattern_state(SessionState::Waiting); + assert_eq!(s.state, SessionState::Waiting); + + // ...and survives further mid-turn watcher reports, and RUNNING + // repaints of unrelated screen regions. + assert!(!s.apply_watcher_state(SessionState::Running)); + assert_eq!(s.state, SessionState::Waiting); + s.apply_pattern_state(SessionState::Running); + assert_eq!(s.state, SessionState::Waiting); + + // An idle prompt on screen releases it, restoring the watcher's view. + s.apply_pattern_state(SessionState::Ready); + assert_eq!(s.state, SessionState::Running); + assert!(!s.pattern_waiting); + } + + #[test] + fn watcher_ready_releases_a_stale_pattern_waiting() { + let mut s = session(SessionKind::OpenCode); + s.apply_watcher_state(SessionState::Running); + s.apply_pattern_state(SessionState::Waiting); + + // The turn finished, so the dialog is answered no matter what the + // (repaint-noisy) screen still shows. + assert!(s.apply_watcher_state(SessionState::Ready)); + assert_eq!(s.state, SessionState::Ready); + assert!(!s.pattern_waiting); + } + + #[test] + fn pattern_states_apply_freely_without_a_watcher() { + let mut s = session(SessionKind::Aider); + + s.apply_pattern_state(SessionState::Waiting); + assert_eq!(s.state, SessionState::Waiting); + // No watcher state to fall back on: a complete RUNNING line clears it. + s.apply_pattern_state(SessionState::Running); + assert_eq!(s.state, SessionState::Running); } #[test] @@ -1062,17 +1202,20 @@ mod tests { 80, 100, ); - s.process_bytes(b"\x1b[?1049h\x1b[HLine A\r\nLine B\r\nLine C\r\nLine D\r\nLine E"); + s.process_bytes( + b"\x1b[?1049h\x1b[HLine A\r\nLine B\r\nLine C\r\nLine D\r\nLine E", + true, + ); assert!(s.output_lines.is_empty()); // Scroll up by 1: Line A leaves the top, everything shifts up - s.process_bytes(b"\x1b[1S"); + s.process_bytes(b"\x1b[1S", true); assert_eq!(s.output_lines.len(), 1); assert_eq!(s.output_lines.front().unwrap(), "Line A"); // Scroll up by 1 more: Line B leaves the top - s.process_bytes(b"\x1b[1S"); + s.process_bytes(b"\x1b[1S", true); assert_eq!(s.output_lines.len(), 2); assert_eq!(s.output_lines.get(1).unwrap(), "Line B"); @@ -1089,10 +1232,10 @@ mod tests { 80, 100, ); - s.process_bytes(b"\x1b[?1049h\x1b[HHeader\r\nBody 1\r\nFooter"); + s.process_bytes(b"\x1b[?1049h\x1b[HHeader\r\nBody 1\r\nFooter", true); // Repaint with same top line (TUI re-rendering) — should NOT add to output_lines - s.process_bytes(b"\x1b[HHeader\r\nBody 1\r\nFooter"); + s.process_bytes(b"\x1b[HHeader\r\nBody 1\r\nFooter", true); assert!(s.output_lines.is_empty()); } @@ -1107,7 +1250,7 @@ mod tests { 80, 100, ); - s.process_bytes(b"hello world\n"); + s.process_bytes(b"hello world\n", true); assert!(s.output_lines.is_empty()); } } diff --git a/src/ui.rs b/src/ui.rs index 03d280f..3e592b5 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -173,24 +173,15 @@ pub fn draw(f: &mut Frame<'_>, app: &App) -> LayoutInfo { // ── Top-level vertical split ─────────────────────────────────────────── // main output | session bar | status panel - let previews = app - .sessions - .iter() - .filter(|session| { - !session.hidden - && session.state == SessionState::Waiting - && session.waiting_prompt.is_some() - }) - .count() as u16; let orch_row = if app.orchestrator.is_some() || app.orchestrator_session_id.is_some() { 1u16 } else { 0 }; - let desired_status_rows = app.visible_indices().len().max(1) as u16 + 4 + previews + orch_row; + let desired_status_rows = app.visible_indices().len().max(1) as u16 + 4 + orch_row; let capped = desired_status_rows.min((body.height / 3).max(4)); - // Hysteresis so a flapping WAITING preview row can't oscillate the - // pane layout (and with it the sessions' PTY sizes). + // Hysteresis so a changing row count can't oscillate the pane layout + // (and with it the sessions' PTY sizes). let status_rows = app .stabilized_status_rows(capped) .min((body.height / 3).max(4)); @@ -772,28 +763,6 @@ fn draw_status_panel(f: &mut Frame<'_>, app: &App, area: Rect) -> Vec { let line = Paragraph::new(Line::from(spans)); f.render_widget(line, row); - if session.state == SessionState::Waiting { - if let Some(prompt) = &session.waiting_prompt { - if row_y < inner.y + inner.height { - let preview = Rect { - x: inner.x, - y: row_y, - width: inner.width, - height: 1, - }; - f.render_widget( - Paragraph::new(Line::styled( - format!(" ↳ {prompt}"), - Style::default() - .fg(Color::Gray) - .add_modifier(Modifier::DIM | Modifier::ITALIC), - )), - preview, - ); - row_y += 1; - } - } - } } // Orchestrator agent row — after the session rows so the returned click