diff --git a/CHANGELOG.md b/CHANGELOG.md index 63d19920..bf4373be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,24 @@ real world uses." ### Fixed +- **TUI input no longer freezes while long-running shell jobs + flood stdout** (#1299, harvested from PR #1494 by + **@CrepuscularIRIS / autoghclaw**). The job-panel refresh path + was calling `full_output()` from inside the `ShellManager` + mutex, which cloned the entire accumulated stdout/stderr buffer + every 2.5 seconds. For browser-automation or large-build jobs + the buffer grew unboundedly; cloning held the mutex for + O(total_bytes) time, starving the `crossterm::event::poll` loop + and dropping keystrokes. The refresh now reads only the last + `max_tail_chars * 4` bytes under the lock (lock hold time is + O(1) regardless of total output volume) and decodes those into + a tail string for display. `stdout_len` / `stderr_len` still + report the true total byte counts so no caller invariant + breaks. Also tightens `take_delta_from_buffer` to slice + `[cursor..total]` inside the lock guard instead of cloning the + whole buffer first, and skips UTF-8 continuation bytes at + `tail_start` so `from_utf8_lossy` never emits a leading U+FFFD + in the job panel. - **`@`-mention truncation no longer splits multi-byte UTF-8 sequences** (#1441, harvested from PR #1495 by **@CrepuscularIRIS / autoghclaw**). When `@`-mentioning a file diff --git a/crates/tui/src/tools/shell.rs b/crates/tui/src/tools/shell.rs index d567ed03..99fa2bcd 100644 --- a/crates/tui/src/tools/shell.rs +++ b/crates/tui/src/tools/shell.rs @@ -469,7 +469,17 @@ impl BackgroundShell { } fn job_snapshot(&self) -> ShellJobSnapshot { - let (stdout_full, stderr_full, stdout_len, stderr_len) = self.full_output(); + // Use tail_from_buffer instead of full_output so we never clone the + // entire accumulated stdout/stderr for display purposes. full_output + // is O(total_bytes_written), which caused the ShellManager mutex to be + // held for an arbitrarily long time during list_jobs() calls from the + // TUI event loop — freezing input handling on long automation runs. + let (stdout_len, stdout_tail) = tail_from_buffer(&self.stdout_buffer, 1200); + let (stderr_len, stderr_tail) = self + .stderr_buffer + .as_ref() + .map(|buf| tail_from_buffer(buf, 1200)) + .unwrap_or((0, String::new())); ShellJobSnapshot { id: self.id.clone(), job_id: self.id.clone(), @@ -478,8 +488,8 @@ impl BackgroundShell { status: self.status.clone(), exit_code: self.exit_code, elapsed_ms: u64::try_from(self.started_at.elapsed().as_millis()).unwrap_or(u64::MAX), - stdout_tail: tail_text(&stdout_full, 1200), - stderr_tail: tail_text(&stderr_full, 1200), + stdout_tail, + stderr_tail, stdout_len, stderr_len, stdin_available: self.stdin.is_some() && self.status == ShellStatus::Running, @@ -1370,11 +1380,36 @@ impl ShellManager { } fn take_delta_from_buffer(buffer: &Arc>>, cursor: &mut usize) -> (Vec, usize) { - let data = buffer.lock().map(|d| d.clone()).unwrap_or_default(); - let start = (*cursor).min(data.len()); - let delta = data[start..].to_vec(); - *cursor = data.len(); - (delta, data.len()) + let guard = buffer.lock().unwrap_or_else(|e| e.into_inner()); + let total = guard.len(); + let start = (*cursor).min(total); + // Clone only the unread portion (the delta), not the entire accumulated buffer. + // Long-running processes can produce megabytes of output; cloning the full + // buffer on every poll held the ShellManager mutex for O(total_bytes) time. + let delta = guard[start..].to_vec(); + *cursor = total; + (delta, total) +} + +/// Read only the tail of a byte buffer and return (total_len, tail_string). +/// +/// Avoids cloning the full buffer when only a trailing excerpt is needed +/// (e.g. for the job-panel display). `max_tail_chars` is in Unicode scalar +/// values; we read at most `max_tail_chars * 4` bytes from the end to account +/// for multi-byte UTF-8 sequences. +fn tail_from_buffer(buffer: &Arc>>, max_tail_chars: usize) -> (usize, String) { + let guard = buffer.lock().unwrap_or_else(|e| e.into_inner()); + let total = guard.len(); + // Over-estimate byte count (4 bytes per char worst case for UTF-8). + let mut tail_start = total.saturating_sub(max_tail_chars.saturating_mul(4)); + // Snap forward to the next valid UTF-8 codepoint boundary so we don't + // pass a slice beginning with continuation bytes (0x80–0xBF) to + // from_utf8_lossy, which would emit a leading U+FFFD replacement char. + while tail_start < total && (guard[tail_start] & 0xC0) == 0x80 { + tail_start += 1; + } + let tail_str = String::from_utf8_lossy(&guard[tail_start..]).into_owned(); + (total, tail_text(&tail_str, max_tail_chars)) } fn tail_text(text: &str, max_chars: usize) -> String {