From 4512df79e6287d6d58e547cb7d05bb8d3b6c4f10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Sun, 13 Sep 2026 09:08:09 -0400 Subject: [PATCH] fix(tui): terminal escapes ride the writer queue and teardown is bounded A terminal that stopped draining the pty parked the writer thread inside its tty write while it held the stderr lock; the next inline stderr write from the event loop (mouse toggle, notification, kitty clear) deadlocked the loop, and /quit hung on the unbounded writer join and the teardown writes. EscapeWriter queues out-of-band escapes on the frame writer: the mouse capture toggle, notifications (title, progress, protocol), the auth cursor, gboom keyboard flags, the wrap clipboard request, and the kitty clears (queued per agent view and merged into the frame's post-flush, nested subagent views included). The $EDITOR/$PAGER handoff aborts when the writer does not drain. WriterThread::join_within and the teardown run under a 2 s grace; after a stall fd 2 is silenced, the exit hints, relaunch and update prints are skipped, and the panic hook keeps only its log report. Ported from upstream 1.0.19 (grok-build EscapeWriter, WriterJoin, run_bounded_teardown), extended to the remaining inline writes. Claude-Session: https://claude.ai/code/session_01RUqV8HVGXcAETyYbZ2tPQ4 --- crates/codegen/kigi-bin/src/main.rs | 26 +- .../kigi-pager-render/src/render/draw.rs | 141 ++++++++++- crates/codegen/kigi-tty-utils/src/lib.rs | 13 + .../src/app/agent_view/interactions.rs | 11 + .../kigi-tui/src/app/agent_view/media.rs | 15 +- .../kigi-tui/src/app/agent_view/mod.rs | 2 + .../kigi-tui/src/app/agent_view/render.rs | 40 ++- .../kigi-tui/src/app/agent_view/session.rs | 1 + crates/codegen/kigi-tui/src/app/app_view.rs | 40 ++- .../kigi-tui/src/app/dispatch/settings/ui.rs | 15 +- .../kigi-tui/src/app/dispatch/task_result.rs | 1 + .../kigi-tui/src/app/dispatch/tests/mod.rs | 6 +- crates/codegen/kigi-tui/src/app/event_loop.rs | 55 ++-- crates/codegen/kigi-tui/src/app/mod.rs | 236 +++++++++++++++--- .../codegen/kigi-tui/src/notifications/mod.rs | 48 ++-- .../kigi-tui/src/notifications/protocol.rs | 60 +++-- .../kigi-tui/src/wrap_clipboard_image.rs | 15 +- 17 files changed, 560 insertions(+), 165 deletions(-) diff --git a/crates/codegen/kigi-bin/src/main.rs b/crates/codegen/kigi-bin/src/main.rs index 2ca9ba7..3f72ef1 100644 --- a/crates/codegen/kigi-bin/src/main.rs +++ b/crates/codegen/kigi-bin/src/main.rs @@ -1596,16 +1596,21 @@ async fn async_main() -> Result<()> { let result = kigi_tui::app::run(args, bg_update_rx).await; kigi_sandbox::flush(); match result { - Ok(true) => { + Ok(exit) if exit.quit_for_update => { let adopted = bg_update_wait.lock().await.take(); - if finish_update_on_exit(adopted, &update_config).await { - eprintln!("Update installed. Run `kigi` to start."); - } else { - eprintln!("Update did not complete. Run `kigi update` to retry."); + let installed = + finish_update_on_exit(adopted, &update_config, exit.terminal_reading).await; + // A terminal that stopped reading at teardown would block these prints. + if exit.terminal_reading { + if installed { + eprintln!("Update installed. Run `kigi` to start."); + } else { + eprintln!("Update did not complete. Run `kigi update` to retry."); + } } Ok(()) } - Ok(false) => Ok(()), + Ok(_) => Ok(()), Err(e) => Err(e), } } @@ -1621,8 +1626,13 @@ async fn async_main() -> Result<()> { async fn finish_update_on_exit( adopted: Option>>, update_config: &UpdateConfig, + terminal_reading: bool, ) -> bool { let run_blocking = |reason: Option| async move { + // The blocking updater prints and its child inherits stderr: not on a stalled terminal. + if !terminal_reading { + return false; + } if let Some(reason) = reason { eprintln!("{reason}"); } @@ -1636,7 +1646,9 @@ async fn finish_update_on_exit( }; match adopted { Some(handle) => { - eprintln!("Waiting for the update download to finish..."); + if terminal_reading { + eprintln!("Waiting for the update download to finish..."); + } match handle.await { Ok(Ok(status)) if status.success() => true, Ok(Ok(status)) => { diff --git a/crates/codegen/kigi-pager-render/src/render/draw.rs b/crates/codegen/kigi-pager-render/src/render/draw.rs index 31ff278..c78072f 100644 --- a/crates/codegen/kigi-pager-render/src/render/draw.rs +++ b/crates/codegen/kigi-pager-render/src/render/draw.rs @@ -146,6 +146,10 @@ impl TermWriter { pub fn writer_sync(&self) -> &WriterSync { &self.sync } + /// A cloneable handle onto this writer's queue for out-of-band escapes. + pub fn escape_writer(&self) -> EscapeWriter { + EscapeWriter::new(self.tx.clone(), self.sync.clone()) + } } impl Write for TermWriter { fn write(&mut self, data: &[u8]) -> std::io::Result { @@ -166,6 +170,56 @@ impl Drop for TermWriter { let _ = self.flush(); } } +/// Out-of-band escapes ride the frame queue (an inline stderr write deadlocks on a stalled tty); drop every clone before `join_within`. +#[derive(Clone)] +pub struct EscapeWriter { + tx: mpsc::Sender>, + sync: WriterSync, +} +impl EscapeWriter { + pub fn new(tx: mpsc::Sender>, sync: WriterSync) -> Self { + Self { tx, sync } + } + /// No writer thread behind it: sends are dropped. For tests and views built before the terminal exists. + pub fn disconnected() -> Self { + let (tx, _rx) = mpsc::channel::>(); + Self { + tx, + sync: WriterSync::new(), + } + } + /// Never blocks; covered by [`WriterSync::wait_drained`]. + pub fn emit(&self, bytes: impl Into>) { + let data: Vec = bytes.into(); + if data.is_empty() { + return; + } + self.sync.mark_queued(); + let _ = self.tx.send(data); + } + /// Winapi-only commands run synchronously: a console API call, not a tty write. + pub fn emit_command(&self, command: impl crossterm::Command) { + #[cfg(windows)] + if !command.is_ansi_code_supported() { + let _ = command.execute_winapi(); + return; + } + let mut ansi = String::new(); + if command.write_ansi(&mut ansi).is_ok() { + self.emit(ansi); + } + } +} +/// Outcome of [`WriterThread::join_within`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum WriterJoin { + /// The thread drained its queue and exited. + Joined, + /// Still running at the deadline (tty blocked, or a sender alive); detached. + TimedOut, +} +/// Teardown waits this long for the writer thread before detaching it, so a stalled tty cannot hang `/quit`. +pub const WRITER_JOIN_GRACE: Duration = Duration::from_secs(2); /// Handle for the background writer thread. /// /// Joining ensures all queued frames have been written to the terminal @@ -174,19 +228,45 @@ pub struct WriterThread { handle: Option>, } impl WriterThread { - /// Block until the writer thread has processed all pending frames and - /// exited. The [`mpsc::Sender`] must be dropped *before* calling this, - /// otherwise the thread will never see the channel close. - pub fn join(mut self) { - if let Some(h) = self.handle.take() { - let _ = h.join(); + /// Wait up to `grace` for the thread to drain and exit, else detach it; every sender (`EscapeWriter` clones too) must be dropped first. + pub fn join_within(mut self, grace: Duration) -> WriterJoin { + let Some(handle) = self.handle.take() else { + return WriterJoin::Joined; + }; + if wait_finished(&handle, grace) { + let _ = handle.join(); + WriterJoin::Joined + } else { + tracing::warn!( + grace_ms = grace.as_millis() as u64, + "term-writer thread still running at teardown; detaching" + ); + WriterJoin::TimedOut + } + } + #[cfg(test)] + fn for_test(handle: std::thread::JoinHandle<()>) -> Self { + Self { + handle: Some(handle), } } } +fn wait_finished(handle: &std::thread::JoinHandle<()>, grace: Duration) -> bool { + let deadline = Instant::now() + grace; + while !handle.is_finished() { + if Instant::now() >= deadline { + return false; + } + std::thread::sleep(Duration::from_millis(1)); + } + true +} impl Drop for WriterThread { fn drop(&mut self) { - if let Some(h) = self.handle.take() { - let _ = h.join(); + if let Some(handle) = self.handle.take() + && wait_finished(&handle, WRITER_JOIN_GRACE) + { + let _ = handle.join(); } } } @@ -197,7 +277,7 @@ impl Drop for WriterThread { /// `BufWriter`. The [`WriterSync`] must be shared with every [`TermWriter`] /// built on the sender so [`WriterSync::wait_drained`] tracks the queue. /// Drop the sender to signal the thread to exit, then call -/// [`WriterThread::join`] to wait for it. +/// [`WriterThread::join_within`] to wait for it. pub fn spawn_writer_thread() -> (mpsc::Sender>, WriterSync, WriterThread) { let (tx, rx) = mpsc::channel::>(); let sync = WriterSync::new(); @@ -570,4 +650,47 @@ mod tests { "Round-tripped bytes do not decode to original UTF-8 string" ); } + /// Frames and escapes share one queue, so the writer sees them in call order. + #[test] + #[cfg(not(windows))] + fn escapes_share_the_frame_queue_in_call_order() { + let (tx, rx) = mpsc::channel::>(); + let sync = WriterSync::new(); + let mut frames = TermWriter::new(tx, sync.clone()); + let escapes = frames.escape_writer(); + frames.write_all(b"frame-1").unwrap(); + frames.flush().unwrap(); + escapes.emit("\x1b[?1000h"); + frames.write_all(b"frame-2").unwrap(); + frames.flush().unwrap(); + escapes.emit_command(crossterm::event::DisableMouseCapture); + let payloads: Vec> = std::iter::from_fn(|| rx.try_recv().ok()).collect(); + assert_eq!(payloads.len(), 4); + assert_eq!(payloads[0], b"frame-1"); + assert_eq!(payloads[1], b"\x1b[?1000h"); + assert_eq!(payloads[2], b"frame-2"); + assert!(payloads[3].starts_with(b"\x1b["), "{:?}", payloads[3]); + assert!(!sync.is_drained(), "four payloads queued, none written"); + } + /// A writer thread parked in a blocked tty write must not hang teardown. + #[test] + fn join_within_times_out_on_a_stuck_writer_thread() { + let (release_tx, release_rx) = mpsc::channel::<()>(); + let handle = std::thread::spawn(move || { + let _ = release_rx.recv(); + }); + let started = Instant::now(); + let outcome = WriterThread::for_test(handle).join_within(Duration::from_millis(50)); + assert_eq!(outcome, WriterJoin::TimedOut); + assert!(started.elapsed() < Duration::from_secs(2)); + let _ = release_tx.send(()); + } + #[test] + fn join_within_joins_a_finished_writer_thread() { + let handle = std::thread::spawn(|| {}); + assert_eq!( + WriterThread::for_test(handle).join_within(Duration::from_secs(5)), + WriterJoin::Joined + ); + } } diff --git a/crates/codegen/kigi-tty-utils/src/lib.rs b/crates/codegen/kigi-tty-utils/src/lib.rs index 2357bb3..de6f894 100644 --- a/crates/codegen/kigi-tty-utils/src/lib.rs +++ b/crates/codegen/kigi-tty-utils/src/lib.rs @@ -585,6 +585,19 @@ pub fn dup_tui_stderr() -> io::Result { } } +/// Point fd 2 at `/dev/null` for the rest of the process: the terminal stopped reading, so any further stderr write would block. +pub fn silence_native_stderr() { + #[cfg(unix)] + { + use std::os::unix::io::AsRawFd; + let Ok(devnull) = std::fs::OpenOptions::new().write(true).open("/dev/null") else { + return; + }; + // SAFETY: devnull is a valid descriptor; fd 2 is a valid target. + unsafe { libc::dup2(devnull.as_raw_fd(), 2) }; + } +} + /// Restore fd 2 to point to the real terminal stderr. /// /// Call this before exiting so that any final messages (panic output, diff --git a/crates/codegen/kigi-tui/src/app/agent_view/interactions.rs b/crates/codegen/kigi-tui/src/app/agent_view/interactions.rs index f8461f7..a10081b 100644 --- a/crates/codegen/kigi-tui/src/app/agent_view/interactions.rs +++ b/crates/codegen/kigi-tui/src/app/agent_view/interactions.rs @@ -1231,6 +1231,17 @@ mod cancel_turn_mouse_tests { use crate::views::modal::{CancelTurnChoice, CancelTurnViewState}; use crossterm::event::{MouseButton, MouseEvent, MouseEventKind}; use ratatui::layout::Rect; + #[test] + fn queued_post_flush_escapes_drain_in_order() { + let mut agent = make_agent(); + agent.queue_post_flush(crate::terminal::overlay::PostFlush::plain("a".into())); + agent.queue_post_flush(crate::terminal::overlay::PostFlush::plain("b".into())); + let drained = agent + .take_pending_post_flush() + .map(|p| p.as_str().to_owned()); + assert_eq!(drained.as_deref(), Some("ab")); + assert!(agent.take_pending_post_flush().is_none()); + } fn make_agent() -> AgentView { let (tx, _rx) = tokio::sync::mpsc::unbounded_channel(); AgentView::new( diff --git a/crates/codegen/kigi-tui/src/app/agent_view/media.rs b/crates/codegen/kigi-tui/src/app/agent_view/media.rs index 26f6654..492a5b2 100644 --- a/crates/codegen/kigi-tui/src/app/agent_view/media.rs +++ b/crates/codegen/kigi-tui/src/app/agent_view/media.rs @@ -22,10 +22,7 @@ impl AgentView { match key.code { KeyCode::Esc | KeyCode::Char('q') => { // Kitty images outlive the dropped viewer state; clear before close. - kigi_shell::util::with_locked_stderr(|stderr| { - let clear = PostFlush::from(overlay::clear_kitty()); - let _ = clear.write_to(stderr); - }); + self.queue_post_flush(PostFlush::from(overlay::clear_kitty())); self.image_viewer = None; self.image_load_rx = None; // The viewer's decoded/re-encoded overlay image (tens of MB @@ -539,10 +536,7 @@ impl AgentView { match key.code { KeyCode::Esc | KeyCode::Char('q') => { // Kitty images outlive the dropped viewer state; clear before close. - kigi_shell::util::with_locked_stderr(|stderr| { - let clear = PostFlush::from(overlay::clear_kitty()); - let _ = clear.write_to(stderr); - }); + self.queue_post_flush(PostFlush::from(overlay::clear_kitty())); self.video_viewer = None; // The viewer's pre-extracted frame set (~50–300 MB for a // typical clip) just dropped; return the pages to the OS. @@ -569,10 +563,7 @@ impl AgentView { match gboom.handle_key(key) { crate::gboom::GboomKeyOutcome::Close => { // Kitty images outlive the dropped game state; clear before close. - kigi_shell::util::with_locked_stderr(|stderr| { - let clear = PostFlush::from(overlay::clear_kitty()); - let _ = clear.write_to(stderr); - }); + self.queue_post_flush(PostFlush::from(overlay::clear_kitty())); self.gboom = None; } crate::gboom::GboomKeyOutcome::Changed => {} diff --git a/crates/codegen/kigi-tui/src/app/agent_view/mod.rs b/crates/codegen/kigi-tui/src/app/agent_view/mod.rs index a00d218..62b625e 100644 --- a/crates/codegen/kigi-tui/src/app/agent_view/mod.rs +++ b/crates/codegen/kigi-tui/src/app/agent_view/mod.rs @@ -1003,6 +1003,8 @@ pub struct AgentView { pub(crate) edit_hl: Option, /// Whether any inline media is currently placed on screen. pub(crate) inline_media_active: bool, + /// Escapes queued for the next frame's post-flush write (see `queue_post_flush`). + pub(crate) pending_post_flush: Option, /// Image IDs that were placed on screen last frame. Used to detect /// images that scrolled off and need their Kitty placements cleared. pub(crate) last_placed_ids: HashSet, diff --git a/crates/codegen/kigi-tui/src/app/agent_view/render.rs b/crates/codegen/kigi-tui/src/app/agent_view/render.rs index 23499c2..c5406a6 100644 --- a/crates/codegen/kigi-tui/src/app/agent_view/render.rs +++ b/crates/codegen/kigi-tui/src/app/agent_view/render.rs @@ -605,13 +605,32 @@ impl AgentView { false, &mut Vec::new(), ); - child_post_flush = post_flush; + // The child's own queued clears (inline image ids) go out before its frame escapes. + child_post_flush = match (child_view.take_pending_post_flush(), post_flush) { + (Some(mut pending), Some(frame)) => { + pending.append(frame); + Some(pending) + } + (pending, frame) => pending.or(frame), + }; } (None, child_post_flush) } pub fn should_show_tip(&mut self) -> bool { false } + /// Queue escapes for the frame's post-flush write: the event loop never writes stderr inline. + pub(crate) fn queue_post_flush(&mut self, escapes: crate::terminal::overlay::PostFlush) { + match self.pending_post_flush.as_mut() { + Some(pending) => pending.append(escapes), + None => self.pending_post_flush = Some(escapes), + } + } + pub(crate) fn take_pending_post_flush( + &mut self, + ) -> Option { + self.pending_post_flush.take() + } /// `area` is the screen region assigned to this agent view. /// When a tracing overlay is visible, this is smaller than `f.area()`. /// @@ -669,20 +688,17 @@ impl AgentView { || self.active_modal.is_some()) { self.inline_media_active = false; - kigi_shell::util::with_locked_stderr(|stderr| { - for &id in self.inline_media_ids.values() { - let clear = crate::terminal::image::clear_kitty_image(id); - let _ = std::io::Write::write_all(stderr, clear.as_bytes()); - } - }); + let mut clears = String::new(); + for &id in self.inline_media_ids.values() { + clears.push_str(&crate::terminal::image::clear_kitty_image(id)); + } + self.queue_post_flush(crate::terminal::overlay::PostFlush::plain(clears)); self.inline_media_ids.clear(); self.inline_media_iterm_emitted.clear(); } if let Some(ref child_sid) = self.active_subagent.clone() { if let Some(esc) = self.take_own_inline_media_clear_escapes() { - kigi_shell::util::with_locked_stderr(|stderr| { - let _ = std::io::Write::write_all(stderr, esc.as_bytes()); - }); + self.queue_post_flush(crate::terminal::overlay::PostFlush::plain(esc)); } return self.draw_subagent_fullscreen( &child_sid.clone(), @@ -695,9 +711,7 @@ impl AgentView { ); } if let Some(esc) = self.take_subagent_inline_media_clear_escapes() { - kigi_shell::util::with_locked_stderr(|stderr| { - let _ = std::io::Write::write_all(stderr, esc.as_bytes()); - }); + self.queue_post_flush(crate::terminal::overlay::PostFlush::plain(esc)); } let appearance = self.scrollback.appearance().clone(); let layout_cfg = &appearance.scrollback.layout; diff --git a/crates/codegen/kigi-tui/src/app/agent_view/session.rs b/crates/codegen/kigi-tui/src/app/agent_view/session.rs index 90c4ab0..13142ad 100644 --- a/crates/codegen/kigi-tui/src/app/agent_view/session.rs +++ b/crates/codegen/kigi-tui/src/app/agent_view/session.rs @@ -185,6 +185,7 @@ impl AgentView { mermaid: None, edit_hl: None, inline_media_active: false, + pending_post_flush: None, last_placed_ids: HashSet::new(), last_terminal_size: (0, 0), terminal_size_stale: false, diff --git a/crates/codegen/kigi-tui/src/app/app_view.rs b/crates/codegen/kigi-tui/src/app/app_view.rs index 796b5b4..8f8a238 100644 --- a/crates/codegen/kigi-tui/src/app/app_view.rs +++ b/crates/codegen/kigi-tui/src/app/app_view.rs @@ -954,6 +954,8 @@ pub struct AppView { /// Whether the pager uses fullscreen (alt-screen) or inline mode. /// Set from the resolved terminal state at startup. pub(crate) screen_mode: super::ScreenMode, + /// Out-of-band escapes ride the frame queue, never an inline stderr write. + pub(crate) escape_writer: crate::render::draw::EscapeWriter, /// Agent Dashboard state. `Some(_)` only when the dashboard view /// is active (`active_view == AgentDashboard`) or recently closed. /// Held outside the `ActiveView` discriminant because `DashboardState` @@ -1022,7 +1024,10 @@ impl AppView { scroll_state: MouseScrollState::default(), scroll_config: ScrollConfig::from_settings(), appearance: AppearanceConfig::default(), - notification_service: NotificationService::new(Default::default()), + notification_service: NotificationService::new( + Default::default(), + crate::render::draw::EscapeWriter::disconnected(), + ), pending_notification_escapes: None, deferred_notification: None, tracing_rx: None, @@ -1054,6 +1059,7 @@ impl AppView { welcome_auth_fallback_rect: None, auth_show_raw_url: false, auth_mouse_disabled: false, + escape_writer: crate::render::draw::EscapeWriter::disconnected(), session_picker_entries: None, session_picker_loading: false, session_picker_state: crate::views::picker::PickerState::with_mode( @@ -3095,9 +3101,8 @@ impl AppView { .hyperlink_capabilities() .osc22_cursor { - kigi_shell::util::with_locked_stderr(|stderr| { - let _ = crossterm::execute!(stderr, crate::terminal::SetDefaultCursor); - }); + self.escape_writer + .emit_command(crate::terminal::SetDefaultCursor); } } let want_mouse_off = self.auth_show_raw_url @@ -3106,17 +3111,15 @@ impl AppView { && matches!(self.auth_state, AuthState::Authenticating { .. }); if want_mouse_off && !self.auth_mouse_disabled { self.auth_mouse_disabled = true; - kigi_shell::util::with_locked_stderr(|stderr| { - let _ = crossterm::execute!(stderr, crossterm::event::DisableMouseCapture); - }); + self.escape_writer + .emit_command(crossterm::event::DisableMouseCapture); #[cfg(windows)] super::win_native_selection::enable_native_selection(); super::MOUSE_CAPTURE_ENABLED.store(false, std::sync::atomic::Ordering::Release); } else if !want_mouse_off && self.auth_mouse_disabled { self.auth_mouse_disabled = false; - kigi_shell::util::with_locked_stderr(|stderr| { - let _ = crossterm::execute!(stderr, crossterm::event::EnableMouseCapture); - }); + self.escape_writer + .emit_command(crossterm::event::EnableMouseCapture); super::MOUSE_CAPTURE_ENABLED.store(true, std::sync::atomic::Ordering::Release); for agent in self.agents.values_mut() { agent.set_sticky_toast_recursive(None); @@ -3450,6 +3453,8 @@ impl AppView { panel.render(full_area, f.buffer_mut()); } let (cursor_pos, post_flush) = result; + let post_flush = + Self::merge_post_flush(agent.take_pending_post_flush(), post_flush); let has_cloud = false; if has_cloud || self.import_claude_modal.is_some() { link_spans.clear(); @@ -3505,7 +3510,7 @@ impl AppView { dashboard, |inner, buf| { if let Some(agent) = agents.get_mut(&agent_id) { - agent.draw( + let (cursor, post_flush) = agent.draw( inner, buf, registry, @@ -3517,6 +3522,13 @@ impl AppView { bundle_state, false, link_spans, + ); + ( + cursor, + Self::merge_post_flush( + agent.take_pending_post_flush(), + post_flush, + ), ) } else { (None, None) @@ -4304,7 +4316,10 @@ pub(crate) mod tests { scroll_state: MouseScrollState::default(), scroll_config: ScrollConfig::default(), appearance: AppearanceConfig::default(), - notification_service: NotificationService::new(Default::default()), + notification_service: NotificationService::new( + Default::default(), + crate::render::draw::EscapeWriter::disconnected(), + ), pending_notification_escapes: None, deferred_notification: None, tracing_rx: None, @@ -4373,6 +4388,7 @@ pub(crate) mod tests { welcome_auth_fallback_rect: None, auth_show_raw_url: false, auth_mouse_disabled: false, + escape_writer: crate::render::draw::EscapeWriter::disconnected(), session_picker_entries: None, session_picker_loading: false, session_picker_state: crate::views::picker::PickerState::with_mode( diff --git a/crates/codegen/kigi-tui/src/app/dispatch/settings/ui.rs b/crates/codegen/kigi-tui/src/app/dispatch/settings/ui.rs index 7347dc5..bbb633a 100644 --- a/crates/codegen/kigi-tui/src/app/dispatch/settings/ui.rs +++ b/crates/codegen/kigi-tui/src/app/dispatch/settings/ui.rs @@ -506,13 +506,14 @@ pub(in crate::app::dispatch) fn dispatch_toggle_mouse_capture(app: &mut AppView) "active_view": format!("{:?}", app.active_view), })), ); - kigi_shell::util::with_locked_stderr(|stderr| { - let _ = if enable { - crossterm::execute!(stderr, crossterm::event::EnableMouseCapture) - } else { - crossterm::execute!(stderr, crossterm::event::DisableMouseCapture) - }; - }); + // Through the writer queue, never inline: see EscapeWriter. + if enable { + app.escape_writer + .emit_command(crossterm::event::EnableMouseCapture); + } else { + app.escape_writer + .emit_command(crossterm::event::DisableMouseCapture); + } // On legacy conhost, DisableMouseCapture restores the *pre-capture* stdin // mode, which may itself have QuickEdit off (per-window profile or a // stale mode from a crashed run) — assert it so "mouse off" actually diff --git a/crates/codegen/kigi-tui/src/app/dispatch/task_result.rs b/crates/codegen/kigi-tui/src/app/dispatch/task_result.rs index 38c80fa..8913992 100644 --- a/crates/codegen/kigi-tui/src/app/dispatch/task_result.rs +++ b/crates/codegen/kigi-tui/src/app/dispatch/task_result.rs @@ -463,6 +463,7 @@ pub(super) fn dispatch_task_result(result: TaskResult, app: &mut AppView) -> Vec None, wrap_text.as_deref(), None, + &app.escape_writer, ); let effects = drain_clipboard_target(&target, app); maybe_show_x11_primary_paste_hint( diff --git a/crates/codegen/kigi-tui/src/app/dispatch/tests/mod.rs b/crates/codegen/kigi-tui/src/app/dispatch/tests/mod.rs index cebe278..ff52880 100644 --- a/crates/codegen/kigi-tui/src/app/dispatch/tests/mod.rs +++ b/crates/codegen/kigi-tui/src/app/dispatch/tests/mod.rs @@ -81,7 +81,10 @@ fn test_app() -> AppView { scroll_state: crate::input::mouse::MouseScrollState::default(), scroll_config: crate::input::mouse::ScrollConfig::default(), appearance: crate::appearance::AppearanceConfig::default(), - notification_service: crate::notifications::NotificationService::new(Default::default()), + notification_service: crate::notifications::NotificationService::new( + Default::default(), + crate::render::draw::EscapeWriter::disconnected(), + ), pending_notification_escapes: None, deferred_notification: None, tracing_rx: None, @@ -153,6 +156,7 @@ fn test_app() -> AppView { welcome_auth_fallback_rect: None, auth_show_raw_url: false, auth_mouse_disabled: false, + escape_writer: crate::render::draw::EscapeWriter::disconnected(), session_picker_entries: None, session_picker_loading: false, session_picker_state: crate::views::picker::PickerState::with_mode( diff --git a/crates/codegen/kigi-tui/src/app/event_loop.rs b/crates/codegen/kigi-tui/src/app/event_loop.rs index 770cb19..e7fc928 100644 --- a/crates/codegen/kigi-tui/src/app/event_loop.rs +++ b/crates/codegen/kigi-tui/src/app/event_loop.rs @@ -222,7 +222,7 @@ fn suspend_for_child( reader_parked: &std::sync::atomic::AtomicBool, input_rx: &mut tokio::sync::mpsc::UnboundedReceiver, run_child: impl FnOnce(), -) -> Option<(u16, u16)> { +) -> std::io::Result> { use std::sync::atomic::Ordering; // Pause the reader thread, then wait for a FRESH park (reader provably out of // crossterm) so the main thread is the sole poll/read caller; bounded so a @@ -237,8 +237,13 @@ fn suspend_for_child( // fullscreen, before LeaveAlternateScreen below — mirroring teardown's // "no late frame after LeaveAlternateScreen" drain). Bounded like the // reader park so a wedged pty can't hang the suspend. + // The alt-screen writes below take the stderr lock a parked writer still holds: abort instead. if !writer_sync.wait_drained(Duration::from_millis(750)) { - tracing::warn!("suspend: frame writer not drained within 750ms; proceeding"); + input_paused.store(false, Ordering::Release); + return Err(std::io::Error::new( + std::io::ErrorKind::TimedOut, + "terminal writer did not drain before suspend", + )); } // Pre-child cursor probe (minimal only — minimal's startup already proved // this terminal answers CPR). Reader is parked, so the reply is ours. @@ -274,7 +279,7 @@ fn suspend_for_child( // lands in the channel, not the tty. while input_rx.try_recv().is_ok() {} input_paused.store(false, Ordering::Release); - moved_cursor + Ok(moved_cursor) } /// Restore the inline/minimal live region after a tty-taking child exited. @@ -351,7 +356,7 @@ fn run_pending_suspends( .or_else(|_| std::env::var("EDITOR")) .unwrap_or_else(|_| "vi".to_string()); let writer_sync = terminal.backend_mut().writer_mut().writer_sync().clone(); - let moved_cursor = suspend_for_child( + let moved_cursor = match suspend_for_child( app.screen_mode, &writer_sync, input_paused, @@ -360,7 +365,15 @@ fn run_pending_suspends( || { let _ = std::process::Command::new(&editor).arg(&path).status(); }, - ); + ) { + Ok(moved) => moved, + Err(error) => { + tracing::warn!(%error, "editor suspend skipped"); + app.pending_agents_modal_refresh = None; + app.show_toast("Editor not opened: the terminal is not draining output"); + return; + } + }; if let Some(tab) = app.pending_agents_modal_refresh.take() && let ActiveView::Agent(id) = app.active_view && let Some(agent) = app.agents.get_mut(&id) @@ -387,7 +400,7 @@ fn run_pending_suspends( .filter(|p| !p.trim().is_empty()) .unwrap_or_else(|| "less".to_string()); let writer_sync = terminal.backend_mut().writer_mut().writer_sync().clone(); - let moved_cursor = suspend_for_child( + let moved_cursor = match suspend_for_child( app.screen_mode, &writer_sync, input_paused, @@ -431,7 +444,15 @@ fn run_pending_suspends( .status(); } }, - ); + ) { + Ok(moved) => moved, + Err(error) => { + tracing::warn!(%error, "pager suspend skipped"); + let _ = std::fs::remove_file(&path); + app.show_toast("Transcript not opened: the terminal is not draining output"); + return; + } + }; let _ = std::fs::remove_file(&path); // The pager owned the screen; re-anchor if it printed inline (cat) and // repaint the full viewport rather than diffing against a screen state @@ -492,6 +513,7 @@ pub(crate) async fn run( // poll the leader roster (see the roster-poll arm below). app.leader_mode = connection.leader_status_rx.is_some(); app.screen_mode = term_state.screen_mode; + app.escape_writer = terminal.backend_mut().writer().escape_writer(); // Agent/dashboard prompts pick the mode up at their creation sites // (`apply_app_scoped_gates` / `ensure_dashboard_state`); the welcome prompt // already exists, so inject here. @@ -737,12 +759,15 @@ pub(crate) async fn run( cursor: compat.cursor.sessions, }; - // Load notification config from [ui.notifications] in config.toml. - if let Some(ref raw) = effective_config { - app.notification_service = crate::notifications::NotificationService::new( - crate::notifications::load_notification_config(raw), - ); - } + // Load notification config from [ui.notifications] in config.toml; the + // service always gets the live escape writer, never AppView::new's stub. + app.notification_service = crate::notifications::NotificationService::new( + effective_config + .as_ref() + .map(crate::notifications::load_notification_config) + .unwrap_or_default(), + app.escape_writer.clone(), + ); // Full layered resolve (env/requirements/remote may beat plain `[ui]`). crate::appearance::cache::set_show_thinking_blocks( @@ -1378,7 +1403,7 @@ pub(crate) async fn run( let want_gboom_keyboard = app.gboom_active(); if want_gboom_keyboard { if !gboom_keyboard_pushed { - super::push_gboom_keyboard_flags(); + super::push_gboom_keyboard_flags(&app.escape_writer); gboom_keyboard_pushed = true; } // Only the active game receives release events; any other open @@ -1386,7 +1411,7 @@ pub(crate) async fn run( // no key down when reopened after a tab/view switch. app.gboom_release_backgrounded_games(); } else if gboom_keyboard_pushed { - super::pop_gboom_keyboard_flags(); + super::pop_gboom_keyboard_flags(&app.escape_writer); gboom_keyboard_pushed = false; // No game is the active input target now (switched to a non-game // view); clear every game's holds for the same reason. diff --git a/crates/codegen/kigi-tui/src/app/mod.rs b/crates/codegen/kigi-tui/src/app/mod.rs index d5a3b16..98d06c7 100644 --- a/crates/codegen/kigi-tui/src/app/mod.rs +++ b/crates/codegen/kigi-tui/src/app/mod.rs @@ -73,23 +73,19 @@ static GBOOM_KEYBOARD_PUSHED: AtomicBool = AtomicBool::new(false); /// release events — required to track several keys held at once. No-op /// unless the Kitty keyboard protocol is active. Balanced by /// [`pop_gboom_keyboard_flags`] (and by `restore_terminal` on teardown). -pub(crate) fn push_gboom_keyboard_flags() { +pub(crate) fn push_gboom_keyboard_flags(writer: &crate::render::draw::EscapeWriter) { if !kitty_flags_pushed() || GBOOM_KEYBOARD_PUSHED.swap(true, Ordering::AcqRel) { return; } let flags = event::KeyboardEnhancementFlags::DISAMBIGUATE_ESCAPE_CODES | event::KeyboardEnhancementFlags::REPORT_EVENT_TYPES | event::KeyboardEnhancementFlags::REPORT_ALL_KEYS_AS_ESCAPE_CODES; - kigi_shell::util::with_locked_stderr(|stderr| { - let _ = execute!(stderr, event::PushKeyboardEnhancementFlags(flags)); - }); + writer.emit_command(event::PushKeyboardEnhancementFlags(flags)); } /// Pop the extra keyboard layer pushed by [`push_gboom_keyboard_flags`]. -pub(crate) fn pop_gboom_keyboard_flags() { +pub(crate) fn pop_gboom_keyboard_flags(writer: &crate::render::draw::EscapeWriter) { if GBOOM_KEYBOARD_PUSHED.swap(false, Ordering::AcqRel) { - kigi_shell::util::with_locked_stderr(|stderr| { - let _ = execute!(stderr, event::PopKeyboardEnhancementFlags); - }); + writer.emit_command(event::PopKeyboardEnhancementFlags); } } /// Tracks whether mouse capture (the five DEC modes enabled by @@ -175,6 +171,7 @@ pub(crate) const MOUSE_OFF_HINT_PROMPT: &str = /// `write()` to stderr / the pty fd, keeping the tokio event loop free /// from pty back-pressure (e.g. when Ghostty is busy with another pane). pub use crate::render::draw::PagerTerminal; +use crate::render::draw::WriterJoin; /// Whether the pager uses the alternate screen (fullscreen) or stays inline. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum ScreenMode { @@ -305,14 +302,19 @@ fn resolve_hunk_tracker_mode( /// pager skips the welcome screen and immediately loads that session (replaying /// its history). Sessions not found locally are restored from remote storage. /// -/// Returns `Ok(true)` when the user accepted a pending update. The caller -/// should print a message telling the user to relaunch `kigi`. +/// How the pager exited: whether the user accepted a pending update (the +/// caller relaunches `kigi`) and whether the terminal was still reading at +/// teardown (when not, further terminal output must be skipped). +pub struct RunExit { + pub quit_for_update: bool, + pub terminal_reading: bool, +} pub async fn run( args: PagerArgs, bg_update_rx: Option< tokio::sync::oneshot::Receiver>, >, -) -> anyhow::Result { +) -> anyhow::Result { kigi_tty_utils::redirect_native_stderr(); let screen_mode_override = screen_mode_relaunch::take_screen_mode_env_override(); let cancel = CancellationToken::new(); @@ -580,33 +582,58 @@ pub async fn run( ) .await; crate::unified_log::flush_blocking().await; - let _ = restore_terminal(terminal, writer_thread, screen_mode); + // After a timed-out writer join the terminal is not reading: exit hints would block on stderr. + let terminal_reading = !matches!( + restore_terminal(terminal, writer_thread, screen_mode), + Ok(WriterJoin::TimedOut) + ); cancel.cancel(); kigi_tty_utils::global_process_scope().kill_all(); match result { Ok(run_result) => { if run_result.quit_for_update { - return Ok(true); + return Ok(RunExit { + quit_for_update: true, + terminal_reading, + }); } if let Some(relaunch) = run_result.relaunch.as_ref() { + if !terminal_reading { + // The replacement would inherit a silenced stderr and never show. + tracing::warn!("screen-mode relaunch skipped: the terminal stopped reading"); + return Ok(RunExit { + quit_for_update: false, + terminal_reading, + }); + } if let Err(e) = screen_mode_relaunch::exec_screen_mode_relaunch( &relaunch.session_id, relaunch.minimal, ) { tracing::error!(error = % e, "screen-mode relaunch failed"); - print_relaunch_failure_hint( - &e, - &relaunch.session_id, - relaunch.minimal, - &mut io::stderr(), - ); + if terminal_reading { + print_relaunch_failure_hint( + &e, + &relaunch.session_id, + relaunch.minimal, + &mut io::stderr(), + ); + } } - return Ok(false); + return Ok(RunExit { + quit_for_update: false, + terminal_reading, + }); } - if let Some(info) = run_result.exit_info { + if let Some(info) = run_result.exit_info + && terminal_reading + { print_exit_resume_hint(&info.session_id, info.minimal, &mut io::stderr()); } - Ok(false) + Ok(RunExit { + quit_for_update: false, + terminal_reading, + }) } Err(e) => Err(e), } @@ -1065,15 +1092,23 @@ fn init_terminal( kigi_crash_handler::disable_terminal_escape_restore(); }) } -/// Drop the terminal (closing the writer mpsc channel) and join the -/// writer thread. After this returns, subsequent direct stderr writes -/// are guaranteed to land strictly after every queued frame. +/// Drop the terminal and join the writer within `WRITER_JOIN_GRACE`; after `TimedOut` the writer may still hold the stderr lock, so callers must not take it unbounded. fn drain_writer_thread_before_teardown( terminal: PagerTerminal, writer_thread: crate::render::draw::WriterThread, -) { +) -> WriterJoin { drop(terminal); - writer_thread.join(); + let join = writer_thread.join_within(crate::render::draw::WRITER_JOIN_GRACE); + if join == WriterJoin::TimedOut { + crate::unified_log::warn( + "term.writer.join_timeout", + None, + Some(serde_json::json!({ + "grace_ms": crate::render::draw::WRITER_JOIN_GRACE.as_millis() as u64 + })), + ); + } + join } /// Inline teardown escape sequences in the canonical order, shared by /// `restore_terminal` and `set_panic_hook` so the on-wire byte order is @@ -1104,7 +1139,12 @@ fn emit_terminal_teardown_sequences(mode: ScreenMode, inline_cursor_row: Option< kigi_shell::util::with_locked_stderr(|stderr| { let _ = execute!(stderr, event::DisableFocusChange); }); - pop_gboom_keyboard_flags(); + // Teardown runs after the writer is gone, so this pop is inline. + if GBOOM_KEYBOARD_PUSHED.swap(false, Ordering::AcqRel) { + kigi_shell::util::with_locked_stderr(|stderr| { + let _ = execute!(stderr, event::PopKeyboardEnhancementFlags); + }); + } if crate::terminal::take_kitty_flags_pushed() { kigi_shell::util::with_locked_stderr(|stderr| { let _ = execute!(stderr, event::PopKeyboardEnhancementFlags); @@ -1134,15 +1174,57 @@ fn emit_terminal_teardown_sequences(mode: ScreenMode, inline_cursor_row: Option< #[cfg(windows)] win_native_selection::restore_stdin_mode(); } -/// Consumes `terminal` and `writer_thread`: queues a final clear in -/// fullscreen mode, drains the writer thread, then emits the inline -/// teardown sequences. The drain ordering guarantees no late frame can -/// land after `LeaveAlternateScreen`. +/// Bound on teardown writes when the stderr lock may be wedged (panic hook; a timed-out writer join). +const TEARDOWN_GRACE: std::time::Duration = std::time::Duration::from_secs(2); +/// Run a best-effort teardown on a helper thread for at most `grace`; on timeout it is detached (the process is exiting), inline if no thread can spawn. +fn run_bounded_teardown(f: impl FnOnce() + Send + 'static, grace: std::time::Duration) -> bool { + let slot = std::sync::Arc::new(parking_lot::Mutex::new(Some(f))); + let (done_tx, done_rx) = std::sync::mpsc::channel::<()>(); + let worker_slot = std::sync::Arc::clone(&slot); + let spawned = std::thread::Builder::new() + .name("bounded-teardown".into()) + .spawn(move || { + if let Some(f) = worker_slot.lock().take() { + f(); + } + let _ = done_tx.send(()); + }); + match spawned { + Ok(_) => done_rx.recv_timeout(grace).is_ok(), + Err(_) => { + if let Some(f) = slot.lock().take() { + f(); + } + true + } + } +} +/// Clear (fullscreen), drain the writer, then emit the teardown sequences; `TimedOut` means the terminal is not reading and further stderr writes may block. fn restore_terminal( + terminal: PagerTerminal, + writer_thread: crate::render::draw::WriterThread, + mode: ScreenMode, +) -> io::Result { + let join = restore_terminal_with( + terminal, + writer_thread, + mode, + drain_writer_thread_before_teardown, + emit_terminal_teardown_sequences, + )?; + if join == WriterJoin::TimedOut { + // Later exit output (hints, relaunch notice, update result, error report) must not block on a stalled tty. + kigi_tty_utils::silence_native_stderr(); + } + Ok(join) +} +fn restore_terminal_with( mut terminal: PagerTerminal, writer_thread: crate::render::draw::WriterThread, mode: ScreenMode, -) -> io::Result<()> { + drain: impl FnOnce(PagerTerminal, crate::render::draw::WriterThread) -> WriterJoin, + teardown: impl FnOnce(ScreenMode, Option) + Send + 'static, +) -> io::Result { if mode.is_fullscreen() { let _ = terminal.clear(); { @@ -1151,14 +1233,24 @@ fn restore_terminal( } } let inline_cursor_row = (!mode.is_fullscreen()).then(|| terminal.viewport_area().bottom()); - drain_writer_thread_before_teardown(terminal, writer_thread); - emit_terminal_teardown_sequences(mode, inline_cursor_row); + let join = drain(terminal, writer_thread); + // A clean join proves past writes landed, not that the terminal still reads: every teardown write is bounded. + let torn_down = run_bounded_teardown(move || teardown(mode, inline_cursor_row), TEARDOWN_GRACE); + if !torn_down { + tracing::warn!( + grace_ms = TEARDOWN_GRACE.as_millis() as u64, + "terminal teardown did not finish in time; detaching" + ); + } drain_pending_events_with_timeout(std::time::Duration::from_millis(10)); let _ = terminal::disable_raw_mode(); signal_handler::mark_restored(); kigi_crash_handler::disable_terminal_escape_restore(); kigi_tty_utils::restore_native_stderr(); - Ok(()) + if join == WriterJoin::TimedOut || !torn_down { + return Ok(WriterJoin::TimedOut); + } + Ok(WriterJoin::Joined) } pub(crate) fn set_terminal_title(title: &str) { let full = terminal_title_string(title); @@ -1183,18 +1275,88 @@ fn terminal_title_string(title: &str) -> String { fn set_panic_hook(mode: ScreenMode) { let hook = panic::take_hook(); panic::set_hook(Box::new(move |info| { - emit_terminal_teardown_sequences(mode, None); + // The writer thread may hold the stderr lock in a blocked tty write. + let torn_down = run_bounded_teardown( + move || emit_terminal_teardown_sequences(mode, None), + TEARDOWN_GRACE, + ); let _ = terminal::disable_raw_mode(); signal_handler::mark_restored(); kigi_crash_handler::disable_terminal_escape_restore(); kigi_tty_utils::restore_native_stderr(); kigi_tty_utils::global_process_scope().kill_all(); + if !torn_down { + // The previous hook prints to stderr, which is not reading; keep the log report only. + kigi_tty_utils::silence_native_stderr(); + tracing::error!(panic = %info, "process panicked while the terminal was stalled"); + return; + } hook(info); })); } #[cfg(test)] mod tests { use super::*; + fn test_terminal_and_writer_thread() -> (PagerTerminal, crate::render::draw::WriterThread) { + let (frame_tx, writer_sync, writer_thread) = crate::render::draw::spawn_writer_thread(); + let backend = + CrosstermBackend::new(crate::render::draw::TermWriter::new(frame_tx, writer_sync)); + let terminal = kigi_ratatui_inline::Terminal::with_options( + backend, + ratatui::TerminalOptions { + viewport: ratatui::Viewport::Fixed(ratatui::layout::Rect::new(0, 0, 80, 10)), + }, + ) + .expect("fixed-viewport terminal"); + (terminal, writer_thread) + } + /// After a timed-out writer join the teardown must be bounded: the writer may hold the stderr lock. + #[test] + fn restore_bounds_teardown_after_a_timed_out_writer_join() { + let (terminal, writer_thread) = test_terminal_and_writer_thread(); + let (release_tx, release_rx) = std::sync::mpsc::channel::<()>(); + let started = std::time::Instant::now(); + let result = restore_terminal_with( + terminal, + writer_thread, + ScreenMode::Inline, + |terminal, writer_thread| { + drop(terminal); + drop(writer_thread); + WriterJoin::TimedOut + }, + move |_, _| { + let _ = release_rx.recv(); + }, + ); + assert_eq!(result.expect("restore"), WriterJoin::TimedOut); + assert!( + started.elapsed() < TEARDOWN_GRACE + std::time::Duration::from_secs(5), + "teardown must be detached at the grace deadline" + ); + let _ = release_tx.send(()); + } + /// A clean join followed by a teardown that never returns still reports a stalled terminal. + #[test] + fn restore_reports_a_stalled_terminal_when_teardown_times_out_after_a_clean_join() { + let (terminal, writer_thread) = test_terminal_and_writer_thread(); + let (release_tx, release_rx) = std::sync::mpsc::channel::<()>(); + let result = restore_terminal_with( + terminal, + writer_thread, + ScreenMode::Inline, + |terminal, writer_thread| { + drop(terminal); + drop(writer_thread); + WriterJoin::Joined + }, + move |_, _| { + let _ = release_rx.recv(); + }, + ); + assert_eq!(result.expect("restore"), WriterJoin::TimedOut); + let _ = release_tx.send(()); + } /// `[ui].cursor_blink` tri-state → startup cursor policy; the `None` /// default must be Inherit (emit nothing). #[test] diff --git a/crates/codegen/kigi-tui/src/notifications/mod.rs b/crates/codegen/kigi-tui/src/notifications/mod.rs index 1e14b19..82e802e 100644 --- a/crates/codegen/kigi-tui/src/notifications/mod.rs +++ b/crates/codegen/kigi-tui/src/notifications/mod.rs @@ -42,10 +42,15 @@ pub struct NotificationService { /// `true` after the first notification; cleared via /// [`clear_permission_notification`] when the queue drains to empty. permission_notified: bool, + /// Notification, title and progress escapes ride the frame queue (see `EscapeWriter`). + escape_writer: crate::render::draw::EscapeWriter, } impl NotificationService { - pub fn new(config: NotificationConfig) -> Self { + pub fn new( + config: NotificationConfig, + escape_writer: crate::render::draw::EscapeWriter, + ) -> Self { let terminal_ctx = crate::terminal::terminal_context(); let protocol = resolve_protocol(config.method, terminal_ctx); let focus_tracker = focus::FocusTracker::new( @@ -64,6 +69,7 @@ impl NotificationService { progress_active: false, progress_last_sent: None, permission_notified: false, + escape_writer, } } @@ -111,6 +117,7 @@ impl NotificationService { &event.title, &event.body, self.terminal_ctx, + &self.escape_writer, ); } } @@ -132,13 +139,7 @@ impl NotificationService { self.clear_progress_into(&mut buf); } - if !buf.is_empty() { - kigi_shell::util::with_locked_stderr(|stderr| { - use std::io::Write; - let _ = stderr.write_all(buf.as_bytes()); - let _ = stderr.flush(); - }); - } + self.escape_writer.emit(buf); } /// Build escape sequences to set the title and progress bar to idle @@ -206,22 +207,9 @@ impl NotificationService { pub fn shutdown(&mut self) { // Reset the tab title back to "kigi" so it doesn't linger on the // last activity label after exit. - let title_esc = self.title_manager.reset(); - kigi_shell::util::with_locked_stderr(|stderr| { - use std::io::Write as _; - let _ = stderr.write_all(title_esc.as_bytes()); - let _ = stderr.flush(); - }); - - let mut buf = String::new(); + let mut buf = self.title_manager.reset(); self.clear_progress_into(&mut buf); - if !buf.is_empty() { - kigi_shell::util::with_locked_stderr(|stderr| { - use std::io::Write as _; - let _ = stderr.write_all(buf.as_bytes()); - let _ = stderr.flush(); - }); - } + self.escape_writer.emit(buf); } /// Returns `true` if a terminal notification for `ApprovalRequired` has @@ -281,6 +269,7 @@ impl NotificationService { progress_active: false, progress_last_sent: None, permission_notified: false, + escape_writer: crate::render::draw::EscapeWriter::disconnected(), } } } @@ -661,6 +650,19 @@ mod tests { assert!(!svc.is_progress_active()); } + /// Shutdown escapes ride the writer queue, never an inline stderr write. + #[test] + fn shutdown_queues_the_title_reset_on_the_writer() { + let (tx, rx) = std::sync::mpsc::channel::>(); + let mut svc = NotificationService::new( + NotificationConfig::default(), + crate::render::draw::EscapeWriter::new(tx, crate::render::draw::WriterSync::new()), + ); + svc.shutdown(); + let payload = rx.try_recv().expect("shutdown must queue its escapes"); + assert!(payload.starts_with(b"\x1b]"), "{payload:?}"); + } + #[test] fn shutdown_clears_active_progress() { let mut svc = NotificationService::new_for_test(NotificationConfig { diff --git a/crates/codegen/kigi-tui/src/notifications/protocol.rs b/crates/codegen/kigi-tui/src/notifications/protocol.rs index b5cf170..5b6ea21 100644 --- a/crates/codegen/kigi-tui/src/notifications/protocol.rs +++ b/crates/codegen/kigi-tui/src/notifications/protocol.rs @@ -1,5 +1,4 @@ use std::borrow::Cow; -use std::io::Write; use crate::notifications::tmux; use crate::terminal::{MultiplexerKind, TerminalContext, TerminalName}; @@ -68,6 +67,7 @@ pub fn emit_notification( title: &str, body: &str, ctx: &TerminalContext, + writer: &crate::render::draw::EscapeWriter, ) { // For body-only protocols (OSC 9, OSC 99), fold the title (session // name) into the body so it's visible. For OSC 777 (Ghostty), the @@ -81,23 +81,13 @@ pub fn emit_notification( NotificationProtocol::None => return, }; + // Notifications fire from the event-loop thread: queued, never an inline stderr write. if ctx.is_tmux_backed() { - let wrapped = tmux::tmux_passthrough(&sequence); - kigi_shell::util::with_locked_stderr(|stderr| { - let _ = stderr.write_all(wrapped.as_bytes()); - let _ = stderr.flush(); - }); + writer.emit(tmux::tmux_passthrough(&sequence)); } else if matches!(protocol, NotificationProtocol::Bel) { - kigi_shell::util::with_locked_stderr(|stderr| { - let _ = stderr.write_all(BEL_BYTE); - let _ = stderr.flush(); - }); + writer.emit(BEL_BYTE); } else { - let bytes = sequence.as_bytes(); - kigi_shell::util::with_locked_stderr(|stderr| { - let _ = stderr.write_all(bytes); - let _ = stderr.flush(); - }); + writer.emit(sequence.as_bytes()); } } @@ -292,31 +282,61 @@ mod tests { #[test] fn emit_none_is_noop() { let ctx = ctx_with_brand(TerminalName::KigiDesktop); - emit_notification(NotificationProtocol::None, "title", "body", &ctx); + emit_notification( + NotificationProtocol::None, + "title", + "body", + &ctx, + &crate::render::draw::EscapeWriter::disconnected(), + ); } #[test] fn emit_bel_does_not_panic() { let ctx = ctx_with_brand(TerminalName::Unknown); - emit_notification(NotificationProtocol::Bel, "", "", &ctx); + emit_notification( + NotificationProtocol::Bel, + "", + "", + &ctx, + &crate::render::draw::EscapeWriter::disconnected(), + ); } #[test] fn emit_osc9_does_not_panic() { let ctx = ctx_with_brand(TerminalName::Iterm2); - emit_notification(NotificationProtocol::Osc9, "title", "body", &ctx); + emit_notification( + NotificationProtocol::Osc9, + "title", + "body", + &ctx, + &crate::render::draw::EscapeWriter::disconnected(), + ); } #[test] fn emit_osc99_does_not_panic() { let ctx = ctx_with_brand(TerminalName::Kitty); - emit_notification(NotificationProtocol::Osc99, "title", "body", &ctx); + emit_notification( + NotificationProtocol::Osc99, + "title", + "body", + &ctx, + &crate::render::draw::EscapeWriter::disconnected(), + ); } #[test] fn emit_osc777_does_not_panic() { let ctx = ctx_with_brand(TerminalName::Ghostty); - emit_notification(NotificationProtocol::Osc777, "title", "body", &ctx); + emit_notification( + NotificationProtocol::Osc777, + "title", + "body", + &ctx, + &crate::render::draw::EscapeWriter::disconnected(), + ); } #[test] diff --git a/crates/codegen/kigi-tui/src/wrap_clipboard_image.rs b/crates/codegen/kigi-tui/src/wrap_clipboard_image.rs index ba827b0..149327a 100644 --- a/crates/codegen/kigi-tui/src/wrap_clipboard_image.rs +++ b/crates/codegen/kigi-tui/src/wrap_clipboard_image.rs @@ -56,13 +56,18 @@ pub fn maybe_request_wrap_host_image( local_image: Option<&ImageData>, local_text: Option<&str>, local_file_urls: Option<&str>, + writer: &crate::render::draw::EscapeWriter, ) -> bool { maybe_request_wrap_host_image_with( osc52_sink_active(), local_image, local_text, local_file_urls, - write_request_osc, + || { + // The reply arrives as a paste event; the request is queued, never an inline write. + writer.emit(request_osc_bytes()); + Ok(()) + }, ) } @@ -83,14 +88,6 @@ fn maybe_request_wrap_host_image_with( emit().is_ok() } -fn write_request_osc() -> std::io::Result<()> { - use std::io::Write; - kigi_shell::util::with_locked_stderr(|stderr| { - stderr.write_all(&request_osc_bytes())?; - stderr.flush() - }) -} - /// Decode wrap host-image paste content (`Event::Paste` payload). /// /// `None` → not wrap magic (caller treats as normal text). Malformed wrap