diff --git a/docs/debugging.md b/docs/debugging.md index e362628..63c5a7b 100644 --- a/docs/debugging.md +++ b/docs/debugging.md @@ -118,6 +118,17 @@ For history that outlives the pod, use [VictoriaLogs](providers.md#log-provider- ## Debug containers and pods +When a pod shell fails, sofka keeps the exit status and the last 16 KiB of +captured standard error in a dialog until you dismiss it. After the command +exits, sofka reads available error output before applying a 100 ms deadline to +an idle pipe. It reads at most 1 MiB after exit so a descendant that keeps +writing cannot block the interface. If the runtime reports a +missing `sh`, press `d` to open the debug image prompt for the same target. +The image is shown before creation. No debug container is created until you +accept the prompt and any required guardrail confirmation. Canceling returns +to the original error. Read-only mode and the `debug` guardrail still apply. +Permission errors and connection failures do not trigger this recovery offer. + `:debug` on a **pod** attaches a temporary ephemeral debug container with `kubectl debug`. sofka prompts for the image (prefilled from `[debug]`). An empty `command` starts an interactive shell (bash if the image has it, else sh), like diff --git a/docs/features.md b/docs/features.md index 064bd30..70db7de 100644 --- a/docs/features.md +++ b/docs/features.md @@ -535,6 +535,9 @@ concurrent drains, and full kubectl drain parity are outside this feature. - **PVC explore** (`x` on a PVC, or `:pvc-explore`) - a two-pane browser over a volume's contents, with `s` for a shell inside it. See [PVC explore](#pvc-explore). +- **Shell failure recovery** keeps command errors visible until dismissed. + A missing shell offers the built-in debug image prompt for the same target, + without plugins. Creation requires explicit acceptance and obeys guardrails. - **Ephemeral debug containers** and **node debug pods** (`:debug`). See [Debug containers and pods](debugging.md#debug-containers-and-pods). - **Logs** (`l`) - combined logs for marked pods, per-container on a pod, or aggregated across all matching diff --git a/docs/keys.md b/docs/keys.md index f1aff56..0bb74e0 100644 --- a/docs/keys.md +++ b/docs/keys.md @@ -326,3 +326,11 @@ See [Create a plugin package](plugin-authoring.md). `PgUp` and `PgDn` scroll text that does not fit in the popup. The action keys stay visible. In an input popup, typing moves the view to the cursor. + +## Command failures + +A failed interactive command opens an error dialog. `esc` or `enter` dismisses +it; `PgUp` and `PgDn` scroll the output. If a pod shell failed because `sh` is +missing, `d` opens the debug image prompt for the same pod and container. +Accept the image to start the debug container, subject to the configured +guardrails. Canceling the prompt returns to the original error. diff --git a/src/app/actions.rs b/src/app/actions.rs index f6b1552..984f644 100644 --- a/src/app/actions.rs +++ b/src/app/actions.rs @@ -754,6 +754,28 @@ impl App { if self.deny_readonly() { return; } + let container = container.or_else(|| { + let obj = self.store.get(&format!("{ns}/{pod}"))?; + let containers = obj.data.pointer("/spec/containers")?.as_array()?; + let default = obj + .metadata + .annotations + .as_ref() + .and_then(|a| a.get("kubectl.kubernetes.io/default-container")); + default + .filter(|name| { + containers + .iter() + .any(|c| c["name"].as_str() == Some(name.as_str())) + }) + .cloned() + .or_else(|| containers.first()?.get("name")?.as_str().map(str::to_owned)) + }); + self.shell_target = Some(ShellTarget { + ns: ns.clone(), + pod: pod.clone(), + container: container.clone(), + }); self.note_action("shell", format!("{pod} in {ns}")); let mut argv = self.kubectl_base(); argv.extend(["exec".into(), "-it".into(), "-n".into(), ns, pod]); @@ -788,8 +810,19 @@ impl App { }; let name = obj.metadata.name.clone().unwrap_or_default(); let ns = obj.metadata.namespace.clone().unwrap_or_default(); - // A debug container is a mutation of the pod — let guardrails gate it, - // with no default confirmation (like shell). + self.request_debug_target(ns, name, target, None); + } + + pub(super) fn request_debug_target( + &mut self, + ns: String, + name: String, + target: Option, + recovery: Option>, + ) { + if self.deny_readonly() { + return; + } let targets = [(name.clone(), ns.clone())]; if self .guard("debug", "pods", &targets, ConfirmLevel::None) @@ -806,10 +839,47 @@ impl App { ns, pod: name, target, + recovery, }); self.mode = Mode::Prompt; } + pub(super) fn confirm_debug( + &mut self, + ns: String, + pod: String, + target: Option, + image: String, + recovery: Option>, + ) { + if self.deny_readonly() { + if recovery.is_some() { + self.retain_recovery_error(); + } + return; + } + let targets = [(pod.clone(), ns.clone())]; + let Some(level) = self.guard("debug", "pods", &targets, ConfirmLevel::None) else { + if recovery.is_some() { + self.retain_recovery_error(); + } + return; + }; + let label = format!("Start debug container in {ns}/{pod} with image {image}?"); + self.begin_guarded( + ConfirmAction::Debug { + ns, + pod: pod.clone(), + target, + image, + recovery, + }, + label, + level, + pod, + ); + } + /// Launch `kubectl debug` for the ephemeral container: suspends the TUI and /// shells out interactively, exactly like exec/attach. The ephemeral /// container persists on the pod (Kubernetes can't remove it) until the pod @@ -820,6 +890,7 @@ impl App { pod: String, target: Option, image: String, + recovery: Option>, ) { let tgt = target .as_deref() @@ -846,7 +917,10 @@ impl App { argv.push("--".into()); argv.extend(self.debug.command.clone()); } - self.pending = Some(Suspend::Shell(argv)); + self.pending = Some(match recovery { + Some(failure) => Suspend::Recovery { argv, failure }, + None => Suspend::Shell(argv), + }); } /// `:debug` on a node — preview and confirm the host access a node debug diff --git a/src/app/command_failure.rs b/src/app/command_failure.rs new file mode 100644 index 0000000..17a5049 --- /dev/null +++ b/src/app/command_failure.rs @@ -0,0 +1,100 @@ +use super::*; + +#[derive(Clone)] +pub struct ShellTarget { + pub ns: String, + pub pod: String, + pub container: Option, +} + +#[derive(Clone)] +pub struct CommandFailure { + pub message: String, + original_message: String, + pub target: Option, +} + +pub(crate) fn missing_shell(message: &str) -> bool { + message.lines().any(|line| { + let line = line.to_ascii_lowercase(); + (line.contains("exec: \"sh\"") || line.contains("exec: \"/bin/sh\"")) + && (line.contains("executable file not found") + || line.contains("no such file or directory")) + }) +} + +impl App { + pub fn handle_command_result( + &mut self, + target: Option, + result: std::io::Result<()>, + recovery: Option>, + ) { + match result { + Ok(()) => { + self.command_failure = None; + self.flash = "Command completed.".into(); + self.flash_err = false; + } + Err(error) => { + let message = crate::ui::strip_ansi_if_present(&error.to_string()).into_owned(); + let target = target.filter(|_| missing_shell(&message)); + let (message, original_message, target) = match recovery { + Some(previous) => ( + format!( + "{}\n\nRecovery failed:\n{message}", + previous.original_message + ), + previous.original_message, + previous.target, + ), + None => (message.clone(), message, target), + }; + self.command_failure = Some(CommandFailure { + message, + original_message, + target, + }); + self.popup_scroll = 0; + self.flash = "Command failed.".into(); + self.flash_err = true; + } + } + } + + pub fn command_failure_visible(&self) -> bool { + self.command_failure.is_some() && !matches!(self.mode, Mode::Prompt | Mode::Confirm) + } + + pub(super) fn retain_recovery_error(&mut self) { + if let Some(failure) = &mut self.command_failure { + failure.message.push_str(&format!("\n\n{}", self.flash)); + } + } + + pub(super) fn key_command_failure(&mut self, key: KeyEvent) { + match key.code { + KeyCode::Esc | KeyCode::Enter => self.command_failure = None, + KeyCode::PageUp | KeyCode::Up => { + self.popup_scroll = self.popup_scroll.saturating_sub(self.popup_viewport.max(1)); + } + KeyCode::PageDown | KeyCode::Down => { + self.popup_scroll = self + .popup_scroll + .saturating_add(self.popup_viewport.max(1)) + .min(self.popup_max_scroll); + } + KeyCode::Char('d') => { + let target = self.command_failure.as_ref().and_then(|f| f.target.clone()); + if let Some(target) = target { + let recovery = self.command_failure.clone().map(Box::new); + self.request_debug_target(target.ns, target.pod, target.container, recovery); + if self.mode != Mode::Prompt { + self.retain_recovery_error(); + } + } + } + _ => {} + } + } +} diff --git a/src/app/input.rs b/src/app/input.rs index ae3da65..51b5a2f 100644 --- a/src/app/input.rs +++ b/src/app/input.rs @@ -17,6 +17,10 @@ impl App { // ----- key handling -------------------------------------------------- pub fn handle_key(&mut self, key: KeyEvent) -> Result<()> { + if self.command_failure_visible() { + self.key_command_failure(key); + return Ok(()); + } let action = self.keymap.action(self.key_scope(), &key); if action == Some(Action::PluginActivity) { self.toggle_plugin_activity(); diff --git a/src/app/mod.rs b/src/app/mod.rs index 23a091a..97b2665 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -239,6 +239,10 @@ pub enum Mode { /// command (exec, edit, port-forward), then resume. pub enum Suspend { Shell(Vec), + Recovery { + argv: Vec, + failure: Box, + }, } /// A `kubectl port-forward` running in the background (not `Suspend::Shell` @@ -373,6 +377,13 @@ enum ConfirmAction { }, /// Delete the node debugger pods sofka launched this session (`:debug-clean`). CleanupDebuggers, + Debug { + ns: String, + pod: String, + target: Option, + image: String, + recovery: Option>, + }, /// Create a temporary pod that mounts a PVC nothing else mounts, so it can /// be browsed or shelled into. PvcHelper { @@ -483,6 +494,7 @@ enum PromptKind { ns: String, pod: String, target: Option, + recovery: Option>, }, /// File-transfer path prompts (`t` on a pod), asked in two steps: the /// source path first (`src` is `None`), then the destination with the @@ -2260,6 +2272,8 @@ pub struct App { event_task: Option>, pub pending: Option, + pub shell_target: Option, + pub command_failure: Option, /// Mode to return to when leaving a transient view (logs/detail/diff). return_mode: Mode, /// Row key (ns/name) selected when a transient view was opened, restored on @@ -2546,6 +2560,8 @@ impl App { event_gen: 0, event_task: None, pending: None, + shell_target: None, + command_failure: None, return_mode: Mode::Table, return_selection: None, should_quit: false, @@ -2625,6 +2641,8 @@ impl App { } mod actions; +mod command_failure; +pub use command_failure::{CommandFailure, ShellTarget}; mod adjacent; mod argocd; mod authz; diff --git a/src/app/mouse.rs b/src/app/mouse.rs index 346aa4f..57d3e60 100644 --- a/src/app/mouse.rs +++ b/src/app/mouse.rs @@ -69,6 +69,9 @@ impl App { /// logs, documents, pickers) without a second navigation code path; /// clicks are table-specific (select a row, sort by a header). pub fn handle_mouse(&mut self, m: MouseEvent) -> Result<()> { + if self.command_failure_visible() { + return Ok(()); + } if self.plugin_activity_visible() { let code = match m.kind { MouseEventKind::ScrollUp => KeyCode::Up, diff --git a/src/app/overlays.rs b/src/app/overlays.rs index 22eb3b5..803d5cc 100644 --- a/src/app/overlays.rs +++ b/src/app/overlays.rs @@ -146,6 +146,13 @@ impl App { } => { self.do_node_debug(node, image, namespace, profile); } + ConfirmAction::Debug { + ns, + pod, + target, + image, + recovery, + } => self.do_debug(ns, pod, target, image, recovery), ConfirmAction::CleanupDebuggers => { self.do_cleanup_debuggers(); } @@ -337,11 +344,16 @@ impl App { self.do_set_image(ns, name, plural, container, input); } } - Some(PromptKind::Debug { ns, pod, target }) => { + Some(PromptKind::Debug { + ns, + pod, + target, + recovery, + }) => { if input.is_empty() { self.flash_warn("no debug image given"); } else { - self.do_debug(ns, pod, target, input); + self.confirm_debug(ns, pod, target, input, recovery); } } // The transfer prompts chain: source path first, then the diff --git a/src/app/tests.rs b/src/app/tests.rs index 467dec1..46608d2 100644 --- a/src/app/tests.rs +++ b/src/app/tests.rs @@ -12136,6 +12136,7 @@ async fn debug_from_container_picker_pins_target() { "a".into(), Some("app".into()), "busybox:latest".into(), + None, ); let Some(Suspend::Shell(argv)) = app.pending.take() else { panic!("expected a debug shell"); @@ -32959,3 +32960,202 @@ async fn native_describe_key_preserves_secret_token_exception() { app.handle_key(press(KeyCode::Esc)).unwrap(); } } + +fn complete_failed_shell(app: &mut App, error: &str) { + assert!(matches!(app.pending.take(), Some(Suspend::Shell(_)))); + let target = app.shell_target.take(); + app.handle_command_result(target, Err(std::io::Error::other(error.to_owned())), None); + app.after_suspend(); +} + +const MISSING_SHELL: &str = + "OCI runtime exec failed: exec: \"sh\": executable file not found in $PATH"; + +#[tokio::test] +async fn missing_shell_keeps_error_and_offers_debug_for_original_container() { + let (mut app, _rx) = app_with_pod(); + apply( + &mut app, + json!({"apiVersion":"v1", "kind":"Pod", + "metadata":{"name":"a", "namespace":"default", "annotations":{ + "kubectl.kubernetes.io/default-container":"worker"}}, + "spec":{"containers":[{"name":"web"},{"name":"worker"}]}}), + ); + app.handle_key(press(KeyCode::Char('s'))).unwrap(); + let Some(Suspend::Shell(argv)) = &app.pending else { + panic!("no shell"); + }; + assert!(argv.windows(2).any(|v| v == ["-c", "worker"])); + complete_failed_shell(&mut app, MISSING_SHELL); + app.expire_flash(); + let mut terminal = ratatui::Terminal::new(ratatui::backend::TestBackend::new(110, 35)).unwrap(); + terminal.draw(|f| crate::ui::draw(f, &mut app)).unwrap(); + let screen = terminal + .backend() + .buffer() + .content + .chunks(110) + .map(|row| row.iter().map(|cell| cell.symbol()).collect::()) + .collect::>() + .join("\n"); + assert!(screen.contains("Start debug container"), "{screen}"); + assert!(app.command_failure_visible()); + app.handle_key(press(KeyCode::Char('x'))).unwrap(); + assert!(app.command_failure_visible()); + app.handle_key(press(KeyCode::Char('d'))).unwrap(); + assert_eq!(app.mode, Mode::Prompt); + assert_eq!(app.prompt_input, app.debug.image); + assert!(app.pending.is_none()); + app.handle_key(press(KeyCode::Esc)).unwrap(); + assert!(app.command_failure_visible()); + assert!( + app.command_failure + .as_ref() + .unwrap() + .message + .contains(MISSING_SHELL) + ); + app.handle_key(press(KeyCode::Char('d'))).unwrap(); + app.handle_key(press(KeyCode::Enter)).unwrap(); + let Some(Suspend::Recovery { argv, failure }) = app.pending.take() else { + panic!("no debug command"); + }; + assert!(argv.iter().any(|a| a == "--target=worker")); + assert!(argv.windows(2).any(|v| v == ["default", "a"])); + app.handle_command_result( + None, + Err(std::io::Error::other("debug containers are forbidden")), + Some(failure), + ); + let message = &app.command_failure.as_ref().unwrap().message; + assert!(message.contains(MISSING_SHELL) && message.contains("debug containers are forbidden")); + app.handle_key(press(KeyCode::Char('d'))).unwrap(); + app.handle_key(press(KeyCode::Enter)).unwrap(); + app.pending.take(); + app.handle_command_result(None, Ok(()), None); + assert!(app.command_failure.is_none()); +} + +#[tokio::test] +async fn shell_failure_recovery_retains_errors_when_readonly_or_guardrail_blocks() { + for readonly in [true, false] { + let (mut app, _rx) = app_with_pod(); + app.handle_key(press(KeyCode::Char('s'))).unwrap(); + complete_failed_shell(&mut app, MISSING_SHELL); + if readonly { + app.readonly = true; + } else { + app.guardrails = vec![crate::config::Guardrail { + actions: vec!["debug".into()], + deny: true, + ..Default::default() + }]; + } + app.handle_key(press(KeyCode::Char('d'))).unwrap(); + assert!(app.command_failure_visible()); + let message = &app.command_failure.as_ref().unwrap().message; + assert!(message.contains(MISSING_SHELL)); + assert!( + message.contains(if readonly { "read-only" } else { "guardrail" }), + "{message}" + ); + assert!(app.pending.is_none()); + app.handle_key(press(KeyCode::Esc)).unwrap(); + assert!(app.command_failure.is_none()); + } +} + +#[tokio::test] +async fn shell_debug_recovery_obeys_typed_confirmation() { + let (mut app, _rx) = app_with_pod(); + app.guardrails = vec![crate::config::Guardrail { + actions: vec!["debug".into()], + confirmation: Some("type-resource-name".into()), + ..Default::default() + }]; + app.handle_key(press(KeyCode::Char('s'))).unwrap(); + complete_failed_shell(&mut app, MISSING_SHELL); + app.handle_key(press(KeyCode::Char('d'))).unwrap(); + app.handle_key(press(KeyCode::Enter)).unwrap(); + assert_eq!(app.mode, Mode::Prompt); + assert!(app.pending.is_none()); + app.handle_key(press(KeyCode::Char('a'))).unwrap(); + app.handle_key(press(KeyCode::Enter)).unwrap(); + assert!(app.pending.is_some()); +} + +#[tokio::test] +async fn other_shell_failures_do_not_offer_debug_and_success_has_no_dialog() { + for error in [ + "Error from server (Forbidden): pods/exec is forbidden", + "connection refused", + "exit status 127", + "sh: ls: not found", + ] { + let (mut app, _rx) = app_with_pod(); + app.handle_key(press(KeyCode::Char('s'))).unwrap(); + complete_failed_shell(&mut app, error); + assert!(app.command_failure_visible()); + assert!(app.command_failure.as_ref().unwrap().target.is_none()); + app.handle_key(press(KeyCode::Char('d'))).unwrap(); + assert!(app.pending.is_none()); + app.handle_key(press(KeyCode::Enter)).unwrap(); + assert!(app.command_failure.is_none()); + } + let (mut app, _rx) = app_with_pod(); + app.handle_key(press(KeyCode::Char('s'))).unwrap(); + app.pending.take(); + let target = app.shell_target.take(); + app.handle_command_result(target, Ok(()), None); + assert!(!app.flash_err); + assert!(app.command_failure.is_none()); +} + +#[tokio::test] +async fn unrelated_command_failure_replaces_pending_shell_recovery() { + let (mut app, _rx) = app_with_pod(); + app.handle_key(press(KeyCode::Char('s'))).unwrap(); + complete_failed_shell(&mut app, MISSING_SHELL); + app.handle_key(press(KeyCode::Char('d'))).unwrap(); + assert_eq!(app.mode, Mode::Prompt); + assert!(app.command_failure.is_some()); + + app.handle_command_result( + None, + Err(std::io::Error::other("unrelated command failed")), + None, + ); + app.handle_key(press(KeyCode::Esc)).unwrap(); + let failure = app.command_failure.as_ref().unwrap(); + assert_eq!(failure.message, "unrelated command failed"); + assert!(failure.target.is_none()); + app.handle_key(press(KeyCode::Char('d'))).unwrap(); + assert!(app.pending.is_none()); + assert!(app.command_failure_visible()); +} + +#[tokio::test] +async fn unrelated_missing_shell_uses_its_own_target() { + let (mut app, _rx) = app_with_pod(); + app.handle_key(press(KeyCode::Char('s'))).unwrap(); + complete_failed_shell(&mut app, MISSING_SHELL); + app.handle_key(press(KeyCode::Char('d'))).unwrap(); + app.handle_command_result( + Some(ShellTarget { + ns: "other".into(), + pod: "new-pod".into(), + container: Some("worker".into()), + }), + Err(std::io::Error::other(MISSING_SHELL)), + None, + ); + app.handle_key(press(KeyCode::Esc)).unwrap(); + app.handle_key(press(KeyCode::Char('d'))).unwrap(); + app.handle_key(press(KeyCode::Enter)).unwrap(); + let Some(Suspend::Recovery { argv, failure }) = app.pending.take() else { + panic!("no recovery command"); + }; + assert!(argv.windows(2).any(|v| v == ["other", "new-pod"])); + assert!(argv.iter().any(|a| a == "--target=worker")); + assert_eq!(failure.target.as_ref().unwrap().pod, "new-pod"); +} diff --git a/src/main.rs b/src/main.rs index cb99ef7..1b4bd6e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1009,17 +1009,14 @@ fn dispatch( /// its shell is requested from a message, not from the keystroke that asked /// for it). fn take_suspend(terminal: &mut ratatui::DefaultTerminal, app: &mut App, captured: bool) { - if let Some(app::Suspend::Shell(argv)) = app.pending.take() { - match terminal::suspend_and_run(terminal, &argv, captured) { - Ok(()) => { - app.flash = format!("ran: {}", argv.join(" ")); - app.flash_err = false; - } - Err(error) => { - app.flash = format!("cannot run command: {error}"); - app.flash_err = true; - } - } + if let Some(command) = app.pending.take() { + let (argv, recovery) = match command { + app::Suspend::Shell(argv) => (argv, None), + app::Suspend::Recovery { argv, failure } => (argv, Some(failure)), + }; + let target = app.shell_target.take(); + let result = terminal::suspend_and_run(terminal, &argv, captured); + app.handle_command_result(target, result, recovery); app.after_suspend(); terminal_title::set(app.terminal_title().as_deref()); } diff --git a/src/terminal.rs b/src/terminal.rs index c5931a5..813e646 100644 --- a/src/terminal.rs +++ b/src/terminal.rs @@ -1,4 +1,5 @@ -use std::io; +use std::io::{self, Write}; +use std::process::Stdio; use crossterm::event::{DisableMouseCapture, EnableMouseCapture}; use crossterm::terminal::{ @@ -23,9 +24,7 @@ pub fn suspend_and_run( } let _ = disable_raw_mode(); let _ = crossterm::execute!(io::stdout(), LeaveAlternateScreen, crossterm::cursor::Show); - let result = std::process::Command::new(&argv[0]) - .args(&argv[1..]) - .status(); + let result = run_command(argv); // Set the modes directly. ratatui::init would install another panic hook. let _ = enable_raw_mode(); let _ = crossterm::execute!(io::stdout(), EnterAlternateScreen); @@ -33,7 +32,108 @@ pub fn suspend_and_run( let _ = crossterm::execute!(io::stdout(), EnableMouseCapture); } let _ = terminal.clear(); - result.map(|_| ()) + result +} + +const ERROR_LIMIT: usize = 16 * 1024; + +fn run_command(argv: &[String]) -> io::Result<()> { + std::thread::scope(|scope| { + scope + .spawn(|| { + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build()? + .block_on(run_command_async(argv)) + }) + .join() + .map_err(|_| io::Error::other("Command runner failed."))? + }) +} + +async fn run_command_async(argv: &[String]) -> io::Result<()> { + use tokio::io::AsyncReadExt; + let mut child = tokio::process::Command::new(&argv[0]) + .args(&argv[1..]) + .stdin(Stdio::inherit()) + .stdout(Stdio::inherit()) + .stderr(Stdio::piped()) + .spawn()?; + let mut stderr = child.stderr.take().expect("piped stderr"); + let mut tail = Vec::new(); + let mut buffer = [0; 4096]; + let mut closed = false; + let status = loop { + tokio::select! { + biased; + result = child.wait() => break result?, + result = stderr.read(&mut buffer), if !closed => { + match result { + Ok(0) => closed = true, + Ok(n) => record_stderr(&mut tail, &mut io::stderr(), &buffer[..n]), + Err(error) if error.kind() == io::ErrorKind::Interrupted => {}, + Err(_) => closed = true, + } + } + } + }; + if !closed { + drain_stderr( + &mut stderr, + &mut tail, + &mut io::stderr(), + tokio::time::Instant::now() + std::time::Duration::from_millis(100), + ) + .await; + } + if status.success() { + Ok(()) + } else { + Err(io::Error::other(format!( + "Command failed ({status}).\n{}", + String::from_utf8_lossy(&tail).trim() + ))) + } +} + +fn record_stderr(tail: &mut Vec, output: &mut impl Write, bytes: &[u8]) { + let _ = output.write_all(bytes); + tail.extend_from_slice(bytes); + if tail.len() > ERROR_LIMIT { + tail.drain(..tail.len() - ERROR_LIMIT); + } +} + +async fn drain_stderr( + stderr: &mut (impl tokio::io::AsyncRead + Unpin), + tail: &mut Vec, + output: &mut impl Write, + deadline: tokio::time::Instant, +) { + use tokio::io::AsyncReadExt; + let mut buffer = [0; 4096]; + // Also bound descendants that keep writing after the command exits. + let mut remaining: usize = 1024 * 1024; + while remaining > 0 { + let capacity = remaining.min(buffer.len()); + tokio::select! { + biased; + // Read available bytes before an expired timer. Cooperative task + // budgets must not make a ready pipe appear empty during this drain. + result = tokio::task::unconstrained(stderr.read(&mut buffer[..capacity])) => { + match result { + Ok(0) => break, + Ok(n) => { + record_stderr(tail, output, &buffer[..n]); + remaining -= n; + } + Err(error) if error.kind() == io::ErrorKind::Interrupted => {}, + Err(_) => break, + } + } + _ = tokio::time::sleep_until(deadline) => break, + } + } } #[cfg(unix)] diff --git a/src/terminal/tests.rs b/src/terminal/tests.rs index 8f6602f..3d1bd36 100644 --- a/src/terminal/tests.rs +++ b/src/terminal/tests.rs @@ -150,7 +150,14 @@ impl Drop for Session { #[test] fn terminal_plugin_restores_tui_after_exit_interrupt_quit_and_spawn_error() { - for case in ["exit", "interrupt", "quit", "missing"] { + for case in [ + "exit", + "failure", + "descendant", + "interrupt", + "quit", + "missing", + ] { let mut session = Session::start(case); for _ in 0..2 { session.expect("TUI_READY"); @@ -202,10 +209,24 @@ fn terminal_plugin_command() { } println!("PLUGIN_READY"); io::stdout().flush().unwrap(); - if case == "exit" { + if matches!(case.as_str(), "exit" | "failure" | "descendant") { let mut answer = String::new(); io::stdin().read_line(&mut answer).unwrap(); assert_eq!(answer.trim_end(), "done"); + if case == "descendant" { + let _child = Command::new("sleep") + .arg("30") + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .spawn() + .unwrap(); + eprintln!("descendant still has stderr open"); + std::process::exit(7); + } + if case == "failure" { + eprintln!("exec: \"sh\": executable file not found in $PATH"); + std::process::exit(7); + } } else { std::thread::sleep(Duration::from_secs(30)); panic!("terminal interrupt did not stop the command"); @@ -247,7 +268,12 @@ async fn terminal_plugin_child() { panic!("plugin did not queue a terminal command: {}", app.flash); }; let result = suspend_and_run(&mut terminal, &argv, captured); - assert_eq!(result.is_err(), case == "missing"); + assert_eq!(result.is_err(), case != "exit"); + if case == "failure" { + let error = result.unwrap_err().to_string(); + assert!(error.contains("executable file not found"), "{error}"); + assert!(error.contains("7"), "{error}"); + } app.after_suspend(); assert!(crossterm::terminal::is_raw_mode_enabled().unwrap()); for (signal, previous) in [libc::SIGINT, libc::SIGQUIT].into_iter().zip(&before) { @@ -282,3 +308,52 @@ async fn terminal_plugin_child() { ratatui::restore(); println!("ALL_DONE"); } + +#[tokio::test] +async fn stderr_drain_keeps_ready_tail_after_deadline() { + let final_error = b"exec: \"sh\": executable file not found in $PATH\n"; + let mut bytes = vec![b'x'; ERROR_LIMIT * 4]; + bytes.extend_from_slice(final_error); + let mut reader = bytes.as_slice(); + let mut tail = Vec::new(); + let mut output = Vec::new(); + drain_stderr( + &mut reader, + &mut tail, + &mut output, + tokio::time::Instant::now(), + ) + .await; + assert_eq!(output, bytes); + assert_eq!(tail.len(), ERROR_LIMIT); + assert!(tail.ends_with(final_error)); +} + +#[tokio::test] +async fn stderr_drain_stops_for_idle_or_continuously_writing_descendants() { + let (mut reader, _held_open) = tokio::io::duplex(4096); + let mut tail = Vec::new(); + tokio::time::timeout( + Duration::from_secs(1), + drain_stderr( + &mut reader, + &mut tail, + &mut io::sink(), + tokio::time::Instant::now(), + ), + ) + .await + .unwrap(); + assert!(tail.is_empty()); + let mut reader = tokio::io::repeat(b'x'); + let mut output = Vec::new(); + drain_stderr( + &mut reader, + &mut tail, + &mut output, + tokio::time::Instant::now(), + ) + .await; + assert_eq!(output.len(), 1024 * 1024); + assert_eq!(tail.len(), ERROR_LIMIT); +} diff --git a/src/ui.rs b/src/ui.rs index a2e3267..c7748f6 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -114,6 +114,9 @@ pub fn resize( pub fn draw(frame: &mut Frame, app: &mut App) { draw_base(frame, app); draw_plugin_activity(frame, app); + if app.command_failure_visible() { + draw_text_popup(frame, app, false); + } } fn draw_plugin_activity(frame: &mut Frame, app: &mut App) { @@ -2802,6 +2805,10 @@ fn build_help(app: &App, width: usize) -> (Vec>, String) { ":journal · :audit", "session-local log of the mutating actions you've taken", )); + lines.push(bind( + "Command error", + "esc dismiss · PgUp/PgDn scroll · d debug if shell is missing", + )); lines.push(bind( ":debug", "pod: ephemeral debug container · node: privileged debug pod", @@ -4210,7 +4217,19 @@ fn draw_text_popup(frame: &mut Frame, app: &mut App, input: bool) { centered_rect_with_min(50, 20, 56, 7, bounds) }; let color = if input { theme::peach() } else { theme::red() }; - let (label, scope, title, hint) = if input { + let (label, scope, title, hint) = if app.command_failure_visible() { + let failure = app.command_failure.as_ref().unwrap(); + ( + &failure.message, + "confirm", + " Command failed ", + if failure.target.is_some() { + "d Start debug container · esc/enter dismiss · PgUp/PgDn scroll".into() + } else { + "esc/enter dismiss · PgUp/PgDn scroll".into() + }, + ) + } else if input { ( &app.prompt_label, "prompt",