Skip to content

Commit 6d96b8c

Browse files
committed
Harden TUI shutdown, bind Ctrl+W / Ctrl+K, restore cursor on mode toggle
1 parent 56ff1e0 commit 6d96b8c

8 files changed

Lines changed: 159 additions & 9 deletions

File tree

CHANGELOG.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,19 @@ All notable changes to Sofos are documented in this file.
44

55
## [Unreleased]
66

7+
### Added
8+
9+
- **`Ctrl+W` and `Ctrl+K` are now bound in the input box.** `Ctrl+W` deletes the word behind the cursor (the readline / bash / zsh / fish binding) and `Ctrl+K` deletes from the cursor to the end of the line. `Ctrl+U` (delete to start of line) is unchanged.
10+
711
### Fixed
812

13+
- **`Ctrl+C` while the slash-command popup is open dismisses the popup.** It used to fall through to the outer handler and quit the session, which surprised users who only wanted to bail out of the suggestion list. Ctrl+C with the popup closed still requests shutdown.
14+
- **The cursor shape now follows the active mode after `/safe` and `/normal`.** Toggling safe mode mid-session previously updated the status line but left the cursor in its old shape; the cursor now switches to the safe-mode underscore on `/safe` and back to the default block on `/normal`, matching the startup behaviour.
15+
- **A panic anywhere inside the TUI now restores the terminal before the backtrace prints.** A process-wide panic hook disables raw mode, disables bracketed paste, pops the keyboard enhancement flags, and shows the cursor through a real-tty handle that bypasses the output-capture pipe — so even a panic that strikes before the local Drop chain runs leaves the user with a usable shell.
16+
- **Quitting while a confirmation modal is on screen no longer hangs the worker.** The event loop now sends the modal's default answer back before tearing down, unblocking the worker thread that was parked waiting for a reply so the `join` on shutdown completes promptly.
17+
- **Tool output without newlines no longer freezes the UI.** A captured pipe that delivered a multi-kilobyte blob with no `\n` used to keep the reader thread waiting silently; sofos now flushes the partial line to the UI every 4 KB and continues reading.
18+
- **History rows render correctly on Windows ConPTY.** The cursor save/restore escapes (`ESC [ s` / `ESC [ u`) that legacy ConPTY silently drops have been replaced by relative `MoveUp` / `MoveDown` motion, so wrapped history lines now repaint reliably across Windows terminals.
19+
920
- **Anthropic decode failures now name the provider and show what came back.** A non-streaming response that doesn't match the expected JSON shape used to surface as the generic "HTTP request failed: error decoding response body" with no provider context; sofos now reads the body as text first and includes a redacted preview in the error so a misconfigured proxy is obvious at a glance.
1021
- **Cache-cost numbers settle correctly on turns where server-side compaction lands late.** Anthropic emits the final `cache_read_input_tokens` and `cache_creation_input_tokens` only on the trailing `message_delta` event in those cases; sofos now refreshes both totals when they appear there, so the cost summary picks up the cache-creation premium instead of under-reporting.
1122
- **OpenAI refusals reach the conversation.** A `{type: "refusal", refusal: "..."}` block used to be dropped silently, surfacing as "Assistant returned an empty response"; sofos now lifts the refusal text into the visible response so both the user and the next turn see what the model said.

src/repl/mod.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use crate::error::{Result, SofosError};
1919
use crate::mcp::McpManager;
2020
use crate::session::{HistoryManager, SessionState};
2121
use crate::tools::ToolExecutor;
22-
use crate::ui::{UI, set_safe_mode_cursor_style};
22+
use crate::ui::{UI, set_normal_mode_cursor_style, set_safe_mode_cursor_style};
2323
use colored::Colorize;
2424
use std::path::PathBuf;
2525
use std::sync::atomic::{AtomicBool, Ordering};
@@ -582,6 +582,10 @@ impl Repl {
582582
self.safe_mode = true;
583583
self.tool_executor.set_safe_mode(true);
584584
self.refresh_available_tools();
585+
// Switch the cursor to the safe-mode shape so the visual
586+
// affordance matches the new mode. Best-effort: a failed
587+
// SGR write here is purely cosmetic.
588+
let _ = set_safe_mode_cursor_style();
585589

586590
self.session_state
587591
.conversation
@@ -612,6 +616,9 @@ impl Repl {
612616
self.safe_mode = false;
613617
self.tool_executor.set_safe_mode(false);
614618
self.refresh_available_tools();
619+
// Restore the default cursor shape so the visual affordance
620+
// matches the new mode.
621+
let _ = set_normal_mode_cursor_style();
615622

616623
self.session_state
617624
.conversation

src/repl/tui/event_loop.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,18 @@ pub(super) async fn event_loop(
221221
UiEvent::WorkerShutdown(summary) => {
222222
app.exit_summary = Some(summary);
223223
app.should_quit = true;
224+
// Unblock a worker that is parked on `reply_rx.recv()`
225+
// for a confirmation modal. Without this, the
226+
// `worker_handle.thread.join()` in `mod.rs` would
227+
// hang forever: the worker is waiting for the
228+
// modal's responder, but the event loop is about
229+
// to drop `app` (and the responder with it) only
230+
// after the join returns. Sending the default
231+
// index back unblocks the worker exactly as if
232+
// the user had pressed Esc.
233+
if let Some(prompt) = app.confirmation.take() {
234+
let _ = prompt.responder.send(prompt.default_index);
235+
}
224236
// Drain any pending output events before tearing down —
225237
// the stderr/stdout reader threads are a different mpsc
226238
// sender than the worker, so a pre-shutdown

src/repl/tui/input.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,15 @@ pub(super) fn handle_idle_key(
7878
KeyCode::Char('u') if ctrl => {
7979
app.textarea.delete_line_by_head();
8080
}
81+
// Ctrl+W deletes the previous word, matching bash/zsh/readline.
82+
KeyCode::Char('w') if ctrl => {
83+
app.textarea.delete_word();
84+
}
85+
// Ctrl+K deletes from the cursor to the end of the line, also
86+
// standard readline. Mirrors Ctrl+U on the trailing side.
87+
KeyCode::Char('k') if ctrl => {
88+
app.textarea.delete_line_by_end();
89+
}
8190
// Alt+Up / Alt+Down cycle previously-submitted messages
8291
// without shadowing the textarea's own Up/Down cursor keys.
8392
KeyCode::Up if alt && !ctrl => {
@@ -153,6 +162,16 @@ fn handle_slash_popup_key(
153162
app.slash_popup.dismiss(&snapshot);
154163
true
155164
}
165+
// Ctrl+C while the popup is open dismisses the popup rather
166+
// than quitting the session — the user is mid-input and almost
167+
// certainly meant to bail out of the suggestion list, not to
168+
// exit. Outside the popup, Ctrl+C keeps its shutdown behaviour
169+
// through the outer handler.
170+
KeyCode::Char('c') if ctrl => {
171+
let snapshot = app.input_text();
172+
app.slash_popup.dismiss(&snapshot);
173+
true
174+
}
156175
KeyCode::Tab if bare => {
157176
apply_selected_command(app);
158177
true

src/repl/tui/mod.rs

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ pub mod worker;
2626
use std::fs::OpenOptions;
2727
use std::sync::atomic::AtomicBool;
2828
use std::sync::mpsc as std_mpsc;
29-
use std::sync::{Arc, Mutex};
29+
use std::sync::{Arc, Mutex, OnceLock};
3030
use std::time::Duration;
3131

3232
use ratatui::backend::CrosstermBackend;
@@ -43,6 +43,42 @@ use input::spawn_input_reader;
4343
use keymap::install_confirm_handler;
4444
use output::OutputCapture;
4545

46+
/// Real-tty handle saved by [`install_panic_hook`] so the hook can
47+
/// route restoration sequences past any `OutputCapture` redirection
48+
/// still in place at panic time. `OnceLock` because the hook is
49+
/// installed once per process and the handle never needs to change
50+
/// afterwards.
51+
static PANIC_TTY: OnceLock<Mutex<std::fs::File>> = OnceLock::new();
52+
53+
/// Install a process-global panic hook that restores the terminal
54+
/// before chaining to the previous hook. Best-effort on every step:
55+
/// raw mode comes off, bracketed paste is disabled, kitty keyboard
56+
/// enhancement flags are popped, and the cursor is shown — written
57+
/// through [`PANIC_TTY`] (when set) so output reaches the real
58+
/// terminal even if `OutputCapture` has redirected stdout into a
59+
/// pipe. Chaining to the previous hook keeps the backtrace flowing
60+
/// to stderr so the user can still file a useful bug report.
61+
fn install_panic_hook() {
62+
static INIT: std::sync::Once = std::sync::Once::new();
63+
INIT.call_once(|| {
64+
let prev = std::panic::take_hook();
65+
std::panic::set_hook(Box::new(move |info| {
66+
if let Some(tty_mutex) = PANIC_TTY.get() {
67+
if let Ok(mut tty) = tty_mutex.lock() {
68+
let _ = crossterm::execute!(
69+
&mut *tty,
70+
crossterm::event::PopKeyboardEnhancementFlags,
71+
crossterm::event::DisableBracketedPaste,
72+
crossterm::cursor::Show,
73+
);
74+
}
75+
}
76+
let _ = crossterm::terminal::disable_raw_mode();
77+
prev(info);
78+
}));
79+
});
80+
}
81+
4682
/// Background tick cadence — frame rate vs. event-loop responsiveness
4783
/// tradeoff. ~11 Hz is fast enough that streamed output looks fluid
4884
/// without burning CPU on a quiet conversation.
@@ -197,6 +233,14 @@ pub fn run(mut repl: Repl) -> Result<()> {
197233
#[cfg(windows)]
198234
let tty = OpenOptions::new().write(true).open("CONOUT$")?;
199235
let tty_for_backend = tty.try_clone()?;
236+
// Stash a real-tty handle for the panic hook so a panic between
237+
// here and the normal teardown can still write disable-raw-mode +
238+
// disable-bracketed-paste + show-cursor through a writer that
239+
// bypasses the upcoming output-capture redirection.
240+
if let Ok(tty_for_panic) = tty.try_clone() {
241+
let _ = PANIC_TTY.set(Mutex::new(tty_for_panic));
242+
}
243+
install_panic_hook();
200244

201245
let (ui_tx, ui_rx) = mpsc::unbounded_channel::<UiEvent>();
202246
let (job_tx, job_rx) = std_mpsc::channel::<Job>();

src/repl/tui/output.rs

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,51 @@ fn pipe_writer_into_fd(writer: PipeWriter) -> std::io::Result<libc::c_int> {
245245
}
246246
}
247247

248+
/// Soft cap on a single captured "line" emitted to the UI when the
249+
/// writer never delivers a `\n`. Without it a tool that streams a
250+
/// large blob (a base64 attachment, a one-line JSON dump) would
251+
/// freeze the UI for the lifetime of the read — the reader thread
252+
/// sits in `read_until` waiting for a newline that never arrives.
253+
/// Beyond this, we send the partial buffer and continue reading.
254+
const READER_CHUNK_BYTES: usize = 4 * 1024;
255+
256+
/// Read until `\n` OR until `cap` bytes have been accumulated into
257+
/// `line`, whichever comes first. Returns the number of new bytes
258+
/// pushed onto `line`. `0` means EOF.
259+
fn read_until_or_cap(
260+
buf: &mut BufReader<PipeReader>,
261+
delim: u8,
262+
line: &mut Vec<u8>,
263+
cap: usize,
264+
) -> std::io::Result<usize> {
265+
let start = line.len();
266+
loop {
267+
let available = match buf.fill_buf() {
268+
Ok(slice) => slice,
269+
Err(ref e) if e.kind() == std::io::ErrorKind::Interrupted => continue,
270+
Err(e) => return Err(e),
271+
};
272+
if available.is_empty() {
273+
return Ok(line.len() - start);
274+
}
275+
if let Some(pos) = available.iter().position(|b| *b == delim) {
276+
line.extend_from_slice(&available[..=pos]);
277+
buf.consume(pos + 1);
278+
return Ok(line.len() - start);
279+
}
280+
let remaining_cap = cap.saturating_sub(line.len() - start);
281+
if remaining_cap == 0 {
282+
return Ok(line.len() - start);
283+
}
284+
let take = available.len().min(remaining_cap);
285+
line.extend_from_slice(&available[..take]);
286+
buf.consume(take);
287+
if line.len() - start >= cap {
288+
return Ok(line.len() - start);
289+
}
290+
}
291+
}
292+
248293
fn spawn_reader(
249294
reader: PipeReader,
250295
kind: OutputKind,
@@ -268,7 +313,7 @@ fn spawn_reader(
268313
let mut state = SgrState::default();
269314
loop {
270315
line.clear();
271-
match buf.read_until(b'\n', &mut line) {
316+
match read_until_or_cap(&mut buf, b'\n', &mut line, READER_CHUNK_BYTES) {
272317
Ok(0) => break,
273318
Ok(_) => {
274319
// Log the *raw* bytes — including the `\n` /

src/repl/tui/scrollback.rs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,7 @@ use crossterm::Command;
3737
use crossterm::cursor::MoveDown;
3838
use crossterm::cursor::MoveTo;
3939
use crossterm::cursor::MoveToColumn;
40-
use crossterm::cursor::RestorePosition;
41-
use crossterm::cursor::SavePosition;
40+
use crossterm::cursor::MoveUp;
4241
use crossterm::queue;
4342
use crossterm::style::Color as CColor;
4443
use crossterm::style::Colors;
@@ -258,13 +257,19 @@ fn emit_history_line<W: Write>(writer: &mut W, line: &Line, wrap_width: usize) -
258257
queue!(writer, MoveToColumn(0))?;
259258

260259
let physical_rows = (line.width().max(1).div_ceil(wrap_width)) as u16;
261-
if physical_rows > 1 {
262-
queue!(writer, SavePosition)?;
263-
for _ in 1..physical_rows {
260+
let extra_rows = physical_rows.saturating_sub(1);
261+
if extra_rows > 0 {
262+
// Walk DOWN to pre-clear each continuation row, then walk back
263+
// UP the same amount. Pure relative motion — `MoveUp` plus
264+
// `MoveToColumn(0)` — instead of `Save/RestorePosition` (`ESC [
265+
// s` / `ESC [ u`), which Windows ConPTY in legacy mode silently
266+
// drops and modern ConPTY can land outside the active DECSTBM
267+
// scroll region.
268+
for _ in 0..extra_rows {
264269
queue!(writer, MoveDown(1), MoveToColumn(0))?;
265270
queue!(writer, Clear(ClearType::UntilNewLine))?;
266271
}
267-
queue!(writer, RestorePosition)?;
272+
queue!(writer, MoveUp(extra_rows), MoveToColumn(0))?;
268273
}
269274
queue!(
270275
writer,

src/ui/mod.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,3 +341,10 @@ fn set_cursor_style(style: SetCursorStyle) -> io::Result<()> {
341341
pub fn set_safe_mode_cursor_style() -> io::Result<()> {
342342
set_cursor_style(SetCursorStyle::BlinkingUnderScore)
343343
}
344+
345+
/// Reset the terminal cursor to the default blinking block used in
346+
/// normal (non-safe) mode. Mirror of [`set_safe_mode_cursor_style`]
347+
/// so the `/normal` toggle can put the cursor shape back.
348+
pub fn set_normal_mode_cursor_style() -> io::Result<()> {
349+
set_cursor_style(SetCursorStyle::DefaultUserShape)
350+
}

0 commit comments

Comments
 (0)