Fix TUI panic-safety and doc-accuracy gaps found in final QA pass

docs/decisions/sandboxing.md

@@ -30,6 +30,6 @@ the ones that explain the throw.
 That choice belongs to each front end, not to the library. `sendra-core` has no
 `println!` or `eprintln!` anywhere in it — running a script returns the lines it
 printed alongside its verdict, and `sendra-cli` decides they are stderr.
-`sendra-tui` reuses the same crate and puts them somewhere a redrawn frame does
-not wipe out, without the library ever writing over either front end's own
-interface.
+`sendra-tui` does not run `pre_request`/`post_request` scripts at all — see
+[The interactive TUI](../reference/tui.md) — so this choice does not arise
+there.

docs/reference/tui.md

@@ -3,9 +3,10 @@
 `sendra tui` opens a full-screen terminal app for browsing and running the
 requests in a collection (or a single request file), instead of one shell
 invocation per request. It reuses `sendra-core` directly — the same request
-model, YAML loading, environment resolution, scripting, capture and HTTP
-execution `run`/`test` use — so a collection behaves identically whether you
-send it from the TUI or from the command line.
+model, YAML loading, environment resolution, capture and HTTP execution
+`run`/`test` use — with one exception: `pre_request`/`post_request` scripts
+are not run from the TUI (see below). Everything else behaves identically
+whether you send it from the TUI or from the command line.
 
 ## Installing and launching
 
@@ -85,6 +86,13 @@ A request's status, once it has run, shows in the detail pane alongside its
 headers and body. Requests that have never been run yet just show their
 resolved request preview.
 
+A request's `pre_request`/`post_request` scripts, if it has any, are **not**
+run when sent from the TUI — only `sendra run`/`sendra test` run them. Every
+other part of the send pipeline (environment substitution, config header
+merging, auth/query/body resolution, assertions, capture) is identical
+either way; this is the one honest gap, not a silent one. See
+[Scripting](scripting.md).
+
 ## Editing a request
 
 `i` opens the selected request in an edit form covering every part of the

sendra-cli/tests/version_and_help.rs

@@ -0,0 +1,62 @@
+//! Proves `--version` and `--help` work through the compiled `sendra`
+//! binary, including for the `tui` subcommand — clap derives both from
+//! `Cli`'s `#[command(version, about)]` and each `Command` variant's doc
+//! comments (see `sendra-cli/src/cli.rs`), but nothing else in this crate's
+//! test suite spawns the binary with either flag, so a regression that broke
+//! one (a missing `version` attribute, a `Tui` variant losing its doc
+//! comment) would otherwise go unnoticed until someone ran it by hand.
+
+use std::process::Command;
+
+fn run(args: &[&str]) -> (bool, String, String) {
+    let output = Command::new(env!("CARGO_BIN_EXE_sendra"))
+        .args(args)
+        .output()
+        .expect("the compiled sendra binary must launch");
+    (
+        output.status.success(),
+        String::from_utf8(output.stdout).expect("stdout must be valid UTF-8"),
+        String::from_utf8(output.stderr).expect("stderr must be valid UTF-8"),
+    )
+}
+
+#[test]
+fn version_prints_the_binary_name_and_cargo_version() {
+    let (ok, stdout, _) = run(&["--version"]);
+    assert!(ok, "`--version` must exit successfully");
+    assert_eq!(
+        stdout.trim(),
+        format!("sendra {}", env!("CARGO_PKG_VERSION"))
+    );
+
+    // clap also offers the short form.
+    let (ok, stdout_short, _) = run(&["-V"]);
+    assert!(ok, "`-V` must exit successfully");
+    assert_eq!(stdout_short, stdout, "`-V` and `--version` must agree");
+}
+
+#[test]
+fn top_level_help_lists_every_subcommand() {
+    let (ok, stdout, _) = run(&["--help"]);
+    assert!(ok, "`--help` must exit successfully");
+    for subcommand in ["run", "test", "init", "schema", "import", "tui"] {
+        assert!(
+            stdout.contains(subcommand),
+            "top-level --help must list `{subcommand}`: {stdout}"
+        );
+    }
+    assert!(
+        stdout.contains("launch the interactive TUI") || stdout.contains("no subcommand"),
+        "top-level --help must mention the bare-invocation TUI shortcut: {stdout}"
+    );
+}
+
+#[test]
+fn tui_help_documents_its_optional_path() {
+    let (ok, stdout, _) = run(&["tui", "--help"]);
+    assert!(ok, "`sendra tui --help` must exit successfully");
+    assert!(
+        stdout.contains("PATH"),
+        "`sendra tui --help` must document the optional path argument: {stdout}"
+    );
+}

sendra-tui/src/lib.rs

@@ -167,11 +167,20 @@ fn install_panic_hook() {
     }));
 }
 
+/// Unwinds its own partial progress on failure, rather than relying on the
+/// caller: `run` calls this with `?`, which on a mid-setup error propagates
+/// immediately, before `run`'s own `restore_terminal()` (guarding
+/// `event_loop`) is ever reached, so raw mode and the alternate screen have
+/// to be left exactly as this function found them on every `Err` path, not
+/// just the first one.
 fn init_terminal() -> io::Result<Terminal<CrosstermBackend<Stdout>>> {
     enable_raw_mode()?;
     let mut stdout = io::stdout();
-    execute!(stdout, EnterAlternateScreen)?;
-    Terminal::new(CrosstermBackend::new(stdout))
+    if let Err(err) = execute!(stdout, EnterAlternateScreen) {
+        let _ = disable_raw_mode();
+        return Err(err);
+    }
+    Terminal::new(CrosstermBackend::new(stdout)).inspect_err(|_| restore_terminal())
 }
 
 /// The only place allowed to touch crossterm event types directly — translates

sendra-tui/src/oauth_login.rs

@@ -57,17 +57,43 @@ pub enum LoginOutcome {
 /// message, and every path through [`run_login`] below reaches its one
 /// `on_complete` call rather than leaving the caller's waiting state stuck
 /// forever.
+///
+/// **Always calls `on_complete` exactly once, even if `run_login` panics** —
+/// wrapped in `catch_unwind` the same way [`crate::run_request::spawn`]
+/// wraps `execute`, so a bug anywhere in the login pipeline cannot unwind
+/// this thread out from under `on_complete` and leave the caller's
+/// `OAuthLoginState` stuck at `WaitingForBrowser` forever.
 pub fn spawn(
     oauth: OAuthAuth,
     cache: Arc<OAuthTokenCache>,
     on_complete: impl FnOnce(LoginOutcome) + Send + 'static,
 ) {
     std::thread::spawn(move || {
-        let outcome = run_login(&oauth, &cache);
+        let outcome =
+            login_catching_panics(std::panic::AssertUnwindSafe(|| run_login(&oauth, &cache)));
         on_complete(outcome);
     });
 }
 
+/// Runs `f`, catching any panic and turning it into `LoginOutcome::Failed`
+/// instead of letting it unwind past this point — the mechanism [`spawn`]
+/// relies on to guarantee `on_complete` always runs exactly once, panic or
+/// not. Factored out from `spawn` itself, the same way
+/// [`crate::run_request::run_catching_panics`] is, so this recovery
+/// behavior is directly unit-testable with a closure that deliberately
+/// panics, rather than only reachable by getting a real bug to misfire
+/// somewhere inside `run_login`'s real network/browser pipeline.
+fn login_catching_panics(
+    f: impl FnOnce() -> LoginOutcome + std::panic::UnwindSafe,
+) -> LoginOutcome {
+    std::panic::catch_unwind(f).unwrap_or_else(|payload| {
+        LoginOutcome::Failed(format!(
+            "the login thread panicked: {}",
+            crate::run_request::panic_payload_message(&payload)
+        ))
+    })
+}
+
 /// The flow itself: PKCE pair and CSRF `state` generated fresh for this one
 /// attempt, a local listener bound to `oauth.redirect_uri`, the browser
 /// opened to `oauth.authorization_url`, then a wait — bounded by
@@ -290,6 +316,25 @@ mod tests {
     use std::io::Read;
     use std::net::TcpStream;
 
+    /// Proof requirement for `spawn`'s doc comment: a panic anywhere in
+    /// `run_login` must still produce a `LoginOutcome` rather than unwinding
+    /// past `spawn`'s thread closure, the same guarantee
+    /// `run_request::spawn` already has (and is tested for) — see that
+    /// crate's `a_panic_on_the_run_thread_still_completes_instead_of_hanging_forever`.
+    #[test]
+    fn a_panic_in_run_login_still_produces_a_failed_outcome_instead_of_hanging_forever() {
+        let outcome = login_catching_panics(|| panic!("simulated bug in the login pipeline"));
+        match outcome {
+            LoginOutcome::Failed(message) => {
+                assert!(
+                    message.contains("simulated bug in the login pipeline"),
+                    "the panic message must survive into the outcome: {message}"
+                );
+            }
+            LoginOutcome::Success => panic!("a panic must never read as a successful login"),
+        }
+    }
+
     #[test]
     fn redirect_bind_addr_resolves_loopback_host_and_port() {
         let addr = redirect_bind_addr("http://127.0.0.1:8899/callback").expect("valid loopback");

sendra-tui/src/run_request.rs

@@ -181,7 +181,7 @@ fn run_catching_panics(f: impl FnOnce() -> RunOutcome + std::panic::UnwindSafe)
 /// genuinely is one, while the explicit `&*payload` deref downcasts
 /// correctly. Verified with a minimal repro outside this crate before
 /// settling on this signature, not assumed.
-fn panic_payload_message(payload: &Box<dyn std::any::Any + Send>) -> String {
+pub(crate) fn panic_payload_message(payload: &Box<dyn std::any::Any + Send>) -> String {
     let payload: &(dyn std::any::Any + Send) = &**payload;
     if let Some(message) = payload.downcast_ref::<&str>() {
         (*message).to_string()