wip: unfinished review fix-up (interrupted by the session usage limit)

Uncommitted edits from the fix-up stage, saved so nothing is lost. Not
compiled or tested after these edits; the next pass reviews and finishes them.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01R9rJEoMuUSRWznU6QjkE7h

crates/tui/src/commands/groups/session/structcopy.rs

@@ -163,19 +163,17 @@ fn execute_structcopy(app: &mut App, arg: Option<&str>) -> CommandResult {
         // the default clipboard path never prints the payload.
         return CommandResult::message(json);
     }
-    // `requires_terminal_paste()` is true only for an SSH session with no
-    // forwarded display, where the sole transport is the terminal client
-    // itself. That write is queued on a background writer, so a successful
-    // return means "accepted for transport", not "in the clipboard".
-    let terminal_client = app.clipboard.requires_terminal_paste();
+    // A terminal write (OSC 52 / tmux) is queued on a background writer and
+    // never acknowledged, so it reports "queued", not "in the clipboard".
+    // That covers SSH without a display and a native clipboard that failed.
     let bytes = json.len();
-    match app.clipboard.write_text(&json) {
-        Ok(()) if terminal_client => CommandResult::message(
+    match app.clipboard.write_text_status(&json) {
+        Ok(crate::tui::clipboard::CopyTransport::Terminal) => CommandResult::message(
             tr(app.ui_locale, MessageId::CmdStructcopyClipboardQueued)
                 .replace("{kind}", &label)
                 .replace("{bytes}", &bytes.to_string()),
         ),
-        Ok(()) => CommandResult::message(
+        Ok(crate::tui::clipboard::CopyTransport::Native) => CommandResult::message(
             tr(app.ui_locale, MessageId::CmdStructcopyClipboardAccepted)
                 .replace("{kind}", &label)
                 .replace("{bytes}", &bytes.to_string()),
@@ -2753,7 +2751,9 @@ mod tests {
         let tmpdir = TempDir::new().expect("tempdir");
         let mut app = test_app(&tmpdir);
         seed_transcript(&mut app);
-        app.clipboard = ClipboardHandler::for_test(true, false);
+        // SSH with no display: the write goes to the terminal writer, as in
+        // production, and the receipt follows the transport that took it.
+        app.clipboard = ClipboardHandler::terminal_only_for_test();
         assert!(app.clipboard.requires_terminal_paste());
 
         let result = execute_structcopy(&mut app, Some("turn 1"));

crates/tui/src/tui/context_menu.rs

@@ -7,7 +7,7 @@
 use std::cell::Cell;
 use std::time::Instant;
 
-use crossterm::event::{KeyCode, KeyEvent, MouseButton, MouseEvent, MouseEventKind};
+use crossterm::event::{KeyCode, KeyEvent, KeyModifiers, MouseButton, MouseEvent, MouseEventKind};
 use ratatui::{
     buffer::Buffer,
     layout::Rect,
@@ -421,7 +421,13 @@ impl ModalView for ContextMenuView {
                 ViewAction::None
             }
             KeyCode::Enter => self.activate(self.selected, true),
-            KeyCode::Char(c) => {
+            // A hint is the bare letter: Ctrl+C or Alt+Y must not run the
+            // row that happens to carry `c` or `y`.
+            KeyCode::Char(c)
+                if !key.modifiers.intersects(
+                    KeyModifiers::CONTROL | KeyModifiers::ALT | KeyModifiers::SUPER,
+                ) =>
+            {
                 let key = c.to_string();
                 match self.entries.iter().position(|entry| entry.hint == key) {
                     Some(idx) => self.activate(idx, false),
@@ -799,6 +805,13 @@ mod tests {
             view.handle_key(key(KeyCode::Char('x'))),
             ViewAction::None
         ));
+        // A modified letter is a different shortcut, not the row's hint.
+        for modifiers in [KeyModifiers::CONTROL, KeyModifiers::ALT] {
+            assert!(matches!(
+                view.handle_key(KeyEvent::new(KeyCode::Char('y'), modifiers)),
+                ViewAction::None
+            ));
+        }
     }
 
     /// Entries past the ninth had no key at all. Each now gets a spare

crates/tui/src/tui/history.rs

@@ -2983,9 +2983,8 @@ pub(crate) fn first_file_line_reference(text: &str, workspace: &Path) -> Option<
 /// Accepts the forms tools and models print: `src/a.rs:12`, `src/a.rs:12:5`,
 /// `--> src/a.rs:12:5` (rustc), `` `src/a.rs:12` `` and a reference inside a
 /// sentence or brackets. The text is model output, so it is not trusted to
-/// name a file: every candidate goes through
-/// [`crate::snapshot::workspace_relative_path`] — the gate file-revert already
-/// trusts — which refuses `..` and absolute paths outside the workspace.
+/// name a file: every candidate goes through [`workspace_file`], which
+/// refuses `..`, absolute paths outside the workspace and links.
 pub(crate) fn file_line_reference(line: &str, workspace: &Path) -> Option<(PathBuf, u32)> {
     line.split_whitespace().find_map(|token| {
         // Leading `.` stays: `./src/a.rs` is relative, not `/src/a.rs`.
@@ -2999,12 +2998,37 @@ pub(crate) fn file_line_reference(line: &str, workspace: &Path) -> Option<(PathB
             return None;
         }
         let path_str = path_str.strip_prefix("./").unwrap_or(path_str);
-        let relative = crate::snapshot::workspace_relative_path(workspace, path_str)?;
-        let absolute = workspace.join(relative);
-        absolute.is_file().then_some((absolute, line_no))
+        workspace_file(workspace, path_str).map(|absolute| (absolute, line_no))
     })
 }
 
+/// The regular file `raw` names inside `workspace`, as
+/// `workspace.join(relative)`, or `None`.
+///
+/// [`crate::snapshot::workspace_relative_path`] checks only the text, and
+/// `is_file()` follows links, so a link inside the workspace (`vendor -> /`,
+/// `notes.md -> ~/.ssh/config`) used to pass. Each part below the workspace
+/// is read with `symlink_metadata` and a link is refused, as
+/// `runtime_api::workspace::confined_directory` does, and the resolved path
+/// must still sit under the resolved workspace. Callers that act later check
+/// again at that point: this is a check at one moment, not a lock.
+pub(crate) fn workspace_file(workspace: &Path, raw: &str) -> Option<PathBuf> {
+    let relative = crate::snapshot::workspace_relative_path(workspace, raw)?;
+    let mut path = workspace.to_path_buf();
+    let mut is_file = false;
+    for component in relative.components() {
+        path.push(component);
+        let metadata = std::fs::symlink_metadata(&path).ok()?;
+        if crate::plugins::metadata_is_link_or_reparse(&metadata) {
+            return None;
+        }
+        is_file = metadata.is_file();
+    }
+    let resolved_workspace = workspace.canonicalize().ok()?;
+    let inside = path.canonicalize().ok()?.starts_with(&resolved_workspace);
+    (is_file && inside).then_some(path)
+}
+
 /// Split `path:N`, `path:N:C` or `path:N:text` at the first all-digit
 /// segment after the path. Earlier colons stay in the path (`C:\x.rs:3`).
 fn split_path_line(token: &str) -> Option<(&str, u32)> {

crates/tui/src/tui/history/tests.rs

@@ -2751,6 +2751,42 @@ fn file_line_reference_refuses_paths_outside_the_workspace() {
     );
 }
 
+/// A link inside the workspace passed the text-only check and `is_file()`
+/// followed it, so `vendor -> <outside>` or `notes.md -> <outside file>` in
+/// model output offered "Open in editor" on a file outside the workspace.
+#[cfg(unix)]
+#[test]
+fn file_line_reference_refuses_links_out_of_the_workspace() {
+    let root = tempfile::tempdir().unwrap();
+    let workspace = root.path().join("ws");
+    let outside = root.path().join("outside");
+    std::fs::create_dir_all(workspace.join("src")).unwrap();
+    std::fs::create_dir_all(&outside).unwrap();
+    std::fs::write(outside.join("secret.rs"), "secret\n").unwrap();
+    std::fs::write(workspace.join("src/in.rs"), "fn a() {}\n").unwrap();
+    std::os::unix::fs::symlink(&outside, workspace.join("vendor")).unwrap();
+    std::os::unix::fs::symlink(outside.join("secret.rs"), workspace.join("notes.rs")).unwrap();
+    // A link that stays inside is still a link: refused, not followed.
+    std::os::unix::fs::symlink(workspace.join("src"), workspace.join("alias")).unwrap();
+
+    for line in ["vendor/secret.rs:1", "notes.rs:1", "alias/in.rs:1"] {
+        assert_eq!(
+            super::file_line_reference(line, &workspace),
+            None,
+            "{line:?}"
+        );
+    }
+    assert_eq!(
+        super::workspace_file(&workspace, "src"),
+        None,
+        "a directory"
+    );
+    assert_eq!(
+        super::file_line_reference("src/in.rs:2", &workspace),
+        Some((workspace.join("src/in.rs"), 2))
+    );
+}
+
 #[test]
 fn first_file_line_reference_skips_unresolvable_and_malformed_rows() {
     let dir = tempfile::tempdir().unwrap();

crates/tui/src/tui/mouse_ui.rs

@@ -1724,8 +1724,9 @@ fn push_work_row_entries(app: &App, mouse: MouseEvent, entries: &mut Vec<Context
 }
 
 /// The workspace file a right-click on a transcript cell points at: the first
-/// `path:line` on the clicked line, else the first in the cell. Both are
-/// confined to the workspace by `history::file_line_reference`.
+/// `path:line` on the clicked line, else the first in the cell. Both must be
+/// regular files inside the workspace, reached without links
+/// (`history::workspace_file`); `open_file_in_editor` checks again.
 fn context_menu_file_reference(
     app: &App,
     mouse: MouseEvent,
@@ -1890,6 +1891,19 @@ pub(crate) fn open_file_in_editor(
     path: &std::path::Path,
     line: u32,
 ) {
+    // The menu checked this path when it was built; a link can be swapped in
+    // before the click, so check again right before the editor gets it.
+    let Some(path) = path
+        .to_str()
+        .and_then(|raw| crate::tui::history::workspace_file(&app.workspace, raw))
+    else {
+        app.status_message = Some(format!(
+            "Did not open {}: it is no longer a file inside the workspace",
+            path.display()
+        ));
+        return;
+    };
+    let path = path.as_path();
     let outcome = crate::tui::external_editor::spawn_editor_for_path(
         terminal,
         app.use_alt_screen(),

crates/tui/src/tui/ui/event_loop.rs

@@ -5958,12 +5958,9 @@ pub(crate) async fn run_event_loop(
                 {
                     let sel = app.selected_text();
                     if !sel.is_empty() {
-                        if app.clipboard.write_text(&sel).is_ok() {
-                            app.push_status_toast(
-                                "Copied to clipboard",
-                                StatusToastLevel::Info,
-                                None,
-                            );
+                        if let Ok(transport) = app.clipboard.write_text_status(&sel) {
+                            let receipt = copy_receipt(app, transport, "Copied to clipboard");
+                            app.push_status_toast(receipt, StatusToastLevel::Info, None);
                             app.clear_selection();
                         } else {
                             app.push_status_toast("Copy failed", StatusToastLevel::Error, None);
@@ -6701,13 +6698,11 @@ pub(crate) async fn run_event_loop(
                     // When the composer is empty (transcript focus) →
                     // copy the focused cell text to the system clipboard.
                     if app.input.is_empty() && app.view_stack.is_empty() {
-                        if copy_focused_cell(app) {
-                            app.push_status_toast(
-                                "Copied to clipboard",
-                                StatusToastLevel::Info,
-                                Some(2_000),
-                            );
-                        } else {
+                        // `copy_focused_cell` leaves its own receipt, which
+                        // names the transport; a toast here said "Copied"
+                        // even when only the terminal was asked to copy.
+                        app.status_message = None;
+                        if !copy_focused_cell(app) && app.status_message.is_none() {
                             app.status_message = Some("No transcript cell to copy".to_string());
                         }
                     } else {
@@ -6717,11 +6712,28 @@ pub(crate) async fn run_event_loop(
                 KeyCode::Char('x') if key.modifiers.contains(KeyModifiers::CONTROL) => {
                     let sel = app.selected_text();
                     if !sel.is_empty() {
-                        if app.clipboard.write_text(&sel).is_ok() {
-                            app.push_status_toast("Cut to clipboard", StatusToastLevel::Info, None);
-                            app.delete_selection();
-                        } else {
-                            app.push_status_toast("Cut failed", StatusToastLevel::Error, None);
+                        // Cut deletes only after a native clipboard confirmed
+                        // the copy. A terminal (OSC 52 / tmux) copy is never
+                        // confirmed, so the text stays where it is.
+                        match app.clipboard.write_text_status(&sel) {
+                            Ok(crate::tui::clipboard::CopyTransport::Native) => {
+                                app.push_status_toast(
+                                    "Cut to clipboard",
+                                    StatusToastLevel::Info,
+                                    None,
+                                );
+                                app.delete_selection();
+                            }
+                            Ok(crate::tui::clipboard::CopyTransport::Terminal) => {
+                                app.push_status_toast(
+                                    "Sent to the terminal clipboard; kept the text because terminals do not confirm copies",
+                                    StatusToastLevel::Info,
+                                    None,
+                                );
+                            }
+                            Err(_) => {
+                                app.push_status_toast("Cut failed", StatusToastLevel::Error, None);
+                            }
                         }
                     }
                 }

crates/tui/src/tui/ui/handlers.rs

@@ -1500,10 +1500,11 @@ pub(crate) async fn handle_view_events(
             ViewEvent::CopyToClipboard { text, label } => {
                 if text.is_empty() {
                     app.status_message = Some(format!("{label} is empty"));
-                } else if app.clipboard.write_text(&text).is_ok() {
-                    app.status_message = Some(format!("{label} copied"));
                 } else {
-                    app.status_message = Some(format!("Copy failed ({label})"));
+                    app.status_message = Some(match app.clipboard.write_text_status(&text) {
+                        Ok(transport) => copy_receipt(app, transport, format!("{label} copied")),
+                        Err(_) => format!("Copy failed ({label})"),
+                    });
                 }
             }
             ViewEvent::ApprovalDecision {