fix: drop a redundant usize cast in the git panel

CLAUDE.md

@@ -41,6 +41,7 @@ Keep these boundaries clean; they're the seams for testing and for future work:
 - `git/` — runs git commands, parses porcelain/diff output into typed structs. No TUI types leak in here.
 - `diff/` — diff model: files, hunks, lines, intra-line word diff. Pure data + transforms; heavily unit-tested.
 - `annotate/` — annotation model, persistence, stdout serialization.
+- `clipboard.rs` — the one seam around `arboard`, shared by `ui`'s in-app copy gesture (`U` off a PR) and `main`'s on-quit presentation. Caches its handle for the process lifetime: on X11 the clipboard is served by the owning process, so a handle created and dropped per copy would leave the reviewer with nothing while the TUI is still running.
 - `lsp/` — server lifecycle + the three requests. Must be fully async and never block the render loop; missing/slow servers degrade silently.
 - `ui/` — ratatui widgets, layout, event loop, keymap. Keymap is data (remappable), not hardcoded match arms scattered through widgets.
 - `review/` — per-file review-status model (spec 08 Unit 3, docs/specs/08-spec-branch-review-mode/08-spec-branch-review-mode.md): pure `ReviewStatus` tri-state and transition functions, no TUI types. Persistence (`review-state.json`, blob-SHA reconciliation) lands as a `review::store` submodule in spec 08 task 4.0.

README.md

@@ -25,8 +25,8 @@ brew install sdavisde/tap/redquill
 
 2. Run `redquill` in the git repo you want to review
 3. Press \` to open the git panel, and `?` for help — it opens on a "This context" view scoped to wherever you pressed it from (plus a curated list of common workflows), with the full reference one `Tab` away. Pause after `g` or `z` and the footer shows what keys can follow.
-4. When viewing the diff, press `c` to leave a comment. When the session ends, your comments are copied to the clipboard so you can paste them straight into an agent.
-5. Reviewing a teammate's pull request? Press `R` to open the Review launcher's Pull Requests tab — see [`docs/forge-setup.md`](docs/forge-setup.md) for supported providers and setup.
+4. When viewing the diff, press `c` to leave a comment and `U` to hand the review off — your comments are copied to the clipboard as markdown, ready to paste straight into an agent, without leaving redquill. Quitting a plain session copies them too.
+5. Reviewing a teammate's pull request? Press `R` to open the Review launcher's Pull Requests tab — see [`docs/forge-setup.md`](docs/forge-setup.md) for supported providers and setup. There `U` submits the review to the forge instead, behind a confirm modal where you pick the verdict.
 
 ## Documentation
 

src/clipboard.rs

@@ -0,0 +1,70 @@
+//! The system clipboard: one thin seam around `arboard`, shared by the two
+//! places redquill hands the reviewer their annotations — the in-app
+//! `submit-forge-review` gesture on a non-PR target (see
+//! `ui::forge_submit`'s copy path) and `main`'s on-quit presentation.
+//!
+//! # Why the handle is cached
+//!
+//! On X11 the clipboard is *served by the owning process*: the content lives
+//! as long as some live `arboard::Clipboard` keeps announcing ownership, and
+//! evaporates when the last one drops. `main`'s exit-time copy can get away
+//! with a throwaway handle because the process is about to end either way
+//! (the documented X11 caveat). A copy made from inside the running TUI
+//! cannot — a handle created and dropped per keypress would leave the
+//! reviewer with an empty clipboard while redquill is still on screen, which
+//! is the exact case the in-app gesture exists to serve.
+//!
+//! So the handle is created lazily on first use and kept for the process
+//! lifetime, in a `thread_local` rather than on `App`: `arboard::Clipboard`
+//! is not `Sync`, both call sites run on the main thread, and keeping it out
+//! of `App` avoids threading a non-`Sync` field through a struct whose other
+//! members are all plain data. A failed creation is not cached — a clipboard
+//! that was unavailable at one keypress (no display server yet, a
+//! transiently busy selection owner) may be available at the next.
+//!
+//! # Error contract
+//!
+//! Failures surface as [`ClipboardError`] for the caller to report; nothing
+//! here degrades silently, because a copy the reviewer asked for and did not
+//! get is exactly the thing they must be told about. `main` falls back to
+//! writing the markdown on stdout; the TUI reports the error in the footer
+//! (it has no fallback — stdout belongs to the annotation format and writing
+//! there mid-render would corrupt the screen).
+
+use std::cell::RefCell;
+
+/// A clipboard write that didn't land, carrying `arboard`'s own message —
+/// the reason is always environmental (no display server, no clipboard
+/// backend compiled in, a selection owner that refused), never something the
+/// caller passed in, so there is nothing to distinguish beyond the text.
+#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)]
+#[error("{0}")]
+pub struct ClipboardError(String);
+
+thread_local! {
+    /// The cached handle (see the module doc). `None` until the first
+    /// successful creation.
+    static HANDLE: RefCell<Option<arboard::Clipboard>> = const { RefCell::new(None) };
+}
+
+/// Copies `text` to the system clipboard, reusing this thread's cached
+/// handle (creating it on first use) so the content survives for as long as
+/// the process runs.
+pub fn copy(text: &str) -> Result<(), ClipboardError> {
+    HANDLE.with(|cell| {
+        let mut slot = cell.borrow_mut();
+        if slot.is_none() {
+            // Not cached on failure: see the module doc.
+            *slot = Some(arboard::Clipboard::new().map_err(|e| ClipboardError(e.to_string()))?);
+        }
+        let Some(clipboard) = slot.as_mut() else {
+            // Unreachable in practice — the block above either filled the
+            // slot or returned — but written as a fallback rather than an
+            // `expect`, per this repo's no-panic rule.
+            return Err(ClipboardError("clipboard unavailable".to_string()));
+        };
+        clipboard
+            .set_text(text.to_owned())
+            .map_err(|e| ClipboardError(e.to_string()))
+    })
+}

src/lib.rs

@@ -4,6 +4,7 @@
 //! integration tests exercise them directly.
 
 pub mod annotate;
+pub mod clipboard;
 pub mod config;
 pub mod diff;
 pub mod forge;

src/main.rs

@@ -270,6 +270,12 @@ fn run_tui(config: &RunConfig) -> anyhow::Result<()> {
 
     let mut app = App::with_git(snapshot, target.clone(), Box::new(runner.clone()));
     if let DiffTarget::Review { branch, .. } = &target {
+        // Recorded before `discovered` moves into `set_review_origin_ops`:
+        // pausing or finishing the review re-roots the app back onto this
+        // root's working tree rather than exiting (see
+        // `ui::end_review::App::leave_review_session`), so a `--review`
+        // launch needs it just as much as a launcher-started session does.
+        app.set_review_origin_root(discovered.root().to_path_buf());
         // Load + reconcile this branch's persisted progress before the
         // first render, so `Accepted`/`Deferred` files start collapsed and a
         // stale `Accepted` file starts marked `ChangedSinceAccepted` and
@@ -322,6 +328,11 @@ fn run_tui(config: &RunConfig) -> anyhow::Result<()> {
     ));
     let outcome = ui::run(&mut app)?;
 
+    // Only a non-review session's `q` ever reaches here with `Emit` (see
+    // `QuitOutcome`): a review's own annotations belong to its persisted
+    // entry and, for a PR/MR, to the forge submit flow, and are cleared from
+    // `app.annotations` as the review is left — so nothing a review produced
+    // can arrive here to be presented.
     if let QuitOutcome::Emit = outcome {
         let markdown = render_markdown(&app.annotations);
         present_annotations(&markdown, app.annotations.len(), config.output.as_deref())?;
@@ -354,7 +365,7 @@ fn present_annotations(
     if count == 0 {
         return Ok(());
     }
-    match copy_to_clipboard(markdown) {
+    match redquill::clipboard::copy(markdown) {
         Ok(()) => {
             let noun = if count == 1 {
                 "annotation"
@@ -373,15 +384,6 @@ fn present_annotations(
     Ok(())
 }
 
-/// Copies `text` to the system clipboard, returning any backend error so the
-/// caller can fall back. Kept as a thin seam around `arboard` so the fallback
-/// policy lives in one place ([`present_annotations`]).
-fn copy_to_clipboard(text: &str) -> anyhow::Result<()> {
-    let mut clipboard = arboard::Clipboard::new()?;
-    clipboard.set_text(text.to_owned())?;
-    Ok(())
-}
-
 fn main() -> anyhow::Result<()> {
     let cli = Cli::parse();
     let config = RunConfig::from(cli);

src/ui/annotation_export.rs

@@ -0,0 +1,211 @@
+//! Handing the reviewer their annotations without quitting: the clipboard
+//! half of the `submit-forge-review` gesture (`U`).
+//!
+//! A forge PR review submits to the forge; every other target — the working
+//! tree, `--staged`, a commit or range, a local branch review with no PR
+//! behind it — has nowhere to submit, so the same key copies the annotations
+//! to the clipboard in the public markdown format (see
+//! [`crate::annotate::render_markdown`], the same bytes `main` presents on
+//! quit). Nothing is consumed or cleared by a copy: the reviewer can keep
+//! annotating and copy again, and the same annotations still reach `main`'s
+//! on-quit presentation. See [`super::forge_submit::App::open_submit_forge`]
+//! for the branch that picks between the two destinations.
+//!
+//! The copy is synchronous. It is a user-initiated one-shot on a cached
+//! clipboard handle (see [`crate::clipboard`]) rather than a per-tick or
+//! per-keystroke cost, so it doesn't belong on a background thread the way
+//! git subprocesses and state saves do.
+
+use crate::annotate::render_markdown;
+
+use super::app::App;
+
+/// The footer line for a copy attempt: `count` annotations were offered and
+/// `error` is the clipboard's own message if the write failed.
+///
+/// Pure and separate from the gesture so the one thing that actually matters
+/// here is testable without a system clipboard: a failed copy must never read
+/// as a successful one. A reviewer who is told their review is on the
+/// clipboard, and then pastes an empty buffer into an agent prompt, has lost
+/// the review — so "clipboard unavailable" has to be as visible as the
+/// success line.
+pub(super) fn copy_status_message(count: usize, error: Option<&str>) -> String {
+    match (count, error) {
+        (0, _) => "no annotations to copy".to_string(),
+        (_, Some(e)) => format!("clipboard unavailable ({e}) \u{2014} nothing copied"),
+        (1, None) => "copied 1 annotation to the clipboard".to_string(),
+        (n, None) => format!("copied {n} annotations to the clipboard"),
+    }
+}
+
+impl App {
+    /// Copies every annotation in this session to the clipboard as markdown
+    /// and reports the outcome in the footer. A no-op (beyond the footer
+    /// line) with nothing to copy.
+    ///
+    /// There is no stdout fallback here, unlike `main`'s on-quit
+    /// presentation: stdout carries the annotation format for other programs
+    /// to parse, and writing to it mid-render would corrupt the screen. A
+    /// clipboard failure is reported and nothing else happens — the reviewer
+    /// still has the on-quit path, and the annotations are untouched.
+    pub(super) fn copy_annotations_to_clipboard(&mut self) {
+        self.copy_annotations_with(|markdown| {
+            crate::clipboard::copy(markdown).map_err(|e| e.to_string())
+        });
+    }
+
+    /// [`App::copy_annotations_to_clipboard`]'s body, with the clipboard
+    /// write injected — the seam this repo puts in front of every external
+    /// service, kept as a plain closure rather than a trait because there is
+    /// exactly one operation and no state behind it.
+    ///
+    /// It also means the tests never touch the developer's real clipboard:
+    /// `cargo test` runs on a machine whose clipboard belongs to a person,
+    /// not to the suite, so a test that exercised the live path would be a
+    /// host side effect of the kind this repo's tempdir rules exist to
+    /// prevent. `copy` is not called at all when there is nothing to copy.
+    fn copy_annotations_with(&mut self, copy: impl FnOnce(&str) -> Result<(), String>) {
+        let count = self.annotations.len();
+        if count == 0 {
+            self.set_status_message(copy_status_message(0, None));
+            return;
+        }
+        let markdown = render_markdown(&self.annotations);
+        let error = copy(&markdown).err();
+        self.set_status_message(copy_status_message(count, error.as_deref()));
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::annotate::{Classification, Target};
+    use crate::diff::FileDiff;
+    use crate::git::RawFilePatch;
+    use std::cell::RefCell;
+
+    /// "… to the clipboard" is the phrase that constitutes the promise; a
+    /// message without it is not claiming the annotations got there.
+    fn claims_success(message: &str) -> bool {
+        message.contains("to the clipboard")
+    }
+
+    /// The guardrail: whichever way a copy goes, the footer must not tell the
+    /// reviewer their annotations are on the clipboard unless they are.
+    #[test]
+    fn a_failed_or_empty_copy_never_reads_as_a_successful_one() {
+        let ok = copy_status_message(3, None);
+        assert!(claims_success(&ok), "got {ok:?}");
+        assert!(ok.contains('3'), "the count must be reported: {ok:?}");
+
+        let failed = copy_status_message(3, Some("no display server"));
+        assert!(
+            !claims_success(&failed),
+            "a failed copy must not read as a success: {failed:?}"
+        );
+        assert!(
+            failed.contains("no display server"),
+            "the clipboard's own reason must surface: {failed:?}"
+        );
+
+        let empty = copy_status_message(0, None);
+        assert!(
+            !claims_success(&empty),
+            "an empty set must not claim a copy: {empty:?}"
+        );
+    }
+
+    fn app_with_annotations(bodies: &[&str]) -> App {
+        let raw = "\
+diff --git a/src/a.rs b/src/a.rs
+index 111..222 100644
+--- a/src/a.rs
++++ b/src/a.rs
+@@ -1,2 +1,2 @@
+ fn main() {
+-    old();
++    new();
+";
+        let file = FileDiff::from_patch(&RawFilePatch {
+            path: "src/a.rs".to_string(),
+            old_path: None,
+            raw: raw.to_string(),
+            is_binary: false,
+        })
+        .unwrap();
+        let mut app = App::new(vec![file]);
+        for body in bodies {
+            app.annotations
+                .add(Target::file("src/a.rs"), Classification::Question, *body)
+                .unwrap();
+        }
+        app
+    }
+
+    /// What reaches the clipboard is the public markdown format over the
+    /// whole annotation set — the same bytes `main` presents on quit, since
+    /// the point of the gesture is to hand an agent a review without quitting
+    /// first. The defect this catches is the copy shipping a partial set (a
+    /// stray `unpublished()` filter, say) or some other rendering.
+    #[test]
+    fn the_copy_hands_over_the_public_markdown_for_every_annotation() {
+        let mut app = app_with_annotations(&["first note", "second note"]);
+        let seen = RefCell::new(None);
+
+        app.copy_annotations_with(|markdown| {
+            *seen.borrow_mut() = Some(markdown.to_string());
+            Ok(())
+        });
+
+        let copied = seen.into_inner().expect("the clipboard write must run");
+        assert_eq!(copied, crate::annotate::render_markdown(&app.annotations));
+        assert!(copied.contains("first note"), "got {copied:?}");
+        assert!(copied.contains("second note"), "got {copied:?}");
+        assert!(
+            app.status_message.as_deref().is_some_and(claims_success),
+            "got {:?}",
+            app.status_message
+        );
+        assert_eq!(
+            app.annotations.len(),
+            2,
+            "a copy must not consume the annotations — the reviewer keeps working"
+        );
+    }
+
+    /// Nothing to copy means the clipboard is never touched at all, rather
+    /// than an empty string replacing whatever the reviewer had on it.
+    #[test]
+    fn an_empty_annotation_set_never_reaches_the_clipboard() {
+        let mut app = app_with_annotations(&[]);
+        let called = RefCell::new(false);
+
+        app.copy_annotations_with(|_| {
+            *called.borrow_mut() = true;
+            Ok(())
+        });
+
+        assert!(!called.into_inner(), "the clipboard must be left alone");
+        assert!(
+            app.status_message
+                .as_deref()
+                .is_some_and(|m| !claims_success(m)),
+            "got {:?}",
+            app.status_message
+        );
+    }
+
+    /// A clipboard that refuses is reported, not swallowed — the reviewer
+    /// must not paste an empty buffer into an agent prompt believing the
+    /// review went with it.
+    #[test]
+    fn a_clipboard_failure_surfaces_instead_of_reading_as_success() {
+        let mut app = app_with_annotations(&["a note"]);
+
+        app.copy_annotations_with(|_| Err("no display server".to_string()));
+
+        let message = app.status_message.as_deref().unwrap_or_default();
+        assert!(!claims_success(message), "got {message:?}");
+        assert!(message.contains("no display server"), "got {message:?}");
+    }
+}