15bc9a3c

Make the dashboard a review surface, and let it admit it is old

a73x 2026-08-12 17:52

Commit message

Make the dashboard a review surface, and let it admit it is old

The TUI wrote exactly one kind of event: `n` opened an issue. You could
read a patch, walk its revisions, view diffs and interdiffs, read every
review comment — and then had to quit to say anything about what you had
just read. For a tool whose thesis is that review should preserve the
correspondence between feedback and the change answering it, the review
surface could not leave a comment.

It also never noticed anything. The event loop polled for keystrokes, not
for repository changes, so the screen showed whatever was true when `r`
was last pressed, with nothing on it admitting that. Leave it open beside
working agents — the way it is meant to be used — and it looks perfectly
healthy while being an hour out of date.

The review loop, on the patch detail pane, anchored to what is on screen:

  c  comment on the diff line under the cursor, or on the patch
  R  review with a verdict: approve / request-changes / comment
  x  mark the comment under the cursor answered, or withdraw the claim
  a  show the change that answered it
  o  check the patch out

Prose goes through $EDITOR, as `git commit` does and as the CLI already
did — no second, worse text input gets written here, and the one that
existed for issue bodies is gone. The buffer is seeded with the hunk
being commented on, as `#` lines, so suspending does not cost the writer
the thing they were writing about; those lines come back out, and an
empty message aborts and says so. No $EDITOR is a sentence on the status
line, never a hang. The screen is restored by a `Drop`, so a non-zero
exit or a killed editor gives the terminal back either way.

Inline comments go through `patch::comment`'s `--at` path, so they
inherit its validation against the revision the comment anchors to. The
anchors themselves come from the diff machinery rather than a second
reading of it: `format_diff` now renders rows that already carry them,
and `--line-numbers` is one way of printing those rows.

Refresh notifies rather than reloads. A two-second timer compares
`refs/collab/**` tips — a ref read, not a fold — and raises a banner
counting what arrived; the reader presses `r` when they are ready, so
rows never move under someone mid-comment. An `as of HH:MM` marker is
always there, which is the whole difference between a screen that is
silently wrong and one that is merely old. Reloading re-baselines the
snapshot, so the dashboard's own writes are not announced back as news,
and `refs/collab/local/` is excluded so a read marker is not mistaken
for someone speaking.

`o` no longer checks out `PatchState.branch`. That field has been
provenance only since revision refs landed, and on agent-produced
patches it names a worktree that is long gone; it now runs the same
`patch::checkout` the CLI does, against the latest revision's commit,
which a revision ref pins and which therefore always resolves.

Fixes 094b055b. Fixes 59ab40e8.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

README.md

Old		New
@@ -178,7 +178,17 @@ $ git-collab patch review a1b2c3d4 --verdict request-changes -b "see inline"
178	```	178	```
179		179
180	`git-collab dashboard` opens a TUI over the same data if you would rather browse	180	`git-collab dashboard` opens a TUI over the same data if you would rather browse
181	than type.	181	than type — and it is a review surface, not only a browser. Open a patch and
		182	the review loop is under your hands: `c` comments on the diff line the cursor
		183	is on (or on the patch, anywhere else), `R` records a verdict, `x` marks a
		184	comment answered or withdraws the claim, `a` shows the change that answered it,
		185	and `o` checks the latest revision out. Every body is composed in `$EDITOR`,
		186	seeded with the hunk you are writing about; as with `git commit`, `#` lines are
		187	dropped and an empty message aborts.
		188
		189	The dashboard never reloads under you. A header says `as of HH:MM`, and when an
		190	agent lands something while you are reading, a banner says how many events
		191	arrived — you reload with `r` when you are ready.
182		192
183	## Writing and correcting prose	193	## Writing and correcting prose
184		194

src/patch.rs

Old		New
@@ -1202,6 +1202,16 @@ fn generate_diff_at_revision(
1202	rev_number: u32,	1202	rev_number: u32,
1203	opts: &DiffOpts,	1203	opts: &DiffOpts,
1204	) -> Result<String, Error> {	1204	) -> Result<String, Error> {
		1205	let git_diff = revision_diff(repo, patch, rev_number, opts)?;
		1206	format_diff(&git_diff, opts)
		1207	}
		1208
		1209	fn revision_diff<'r>(
		1210	repo: &'r Repository,
		1211	patch: &PatchState,
		1212	rev_number: u32,
		1213	opts: &DiffOpts,
		1214	) -> Result<git2::Diff<'r>, Error> {
1205	let revision = patch	1215	let revision = patch
1206	.revisions	1216	.revisions
1207	.iter()	1217	.iter()
@@ -1215,12 +1225,43 @@ fn generate_diff_at_revision(
1215	None => resolve_base_tree(repo, &patch.base_ref, commit_oid)?,	1225	None => resolve_base_tree(repo, &patch.base_ref, commit_oid)?,
1216	};	1226	};
1217		1227
1218	let git_diff = repo.diff_tree_to_tree(	1228	Ok(repo.diff_tree_to_tree(
1219	base_tree.as_ref(),	1229	base_tree.as_ref(),
1220	Some(&head_tree),	1230	Some(&head_tree),
1221	Some(&mut diff_options(opts)),	1231	Some(&mut diff_options(opts)),
1222	)?;	1232	)?)
1223	format_diff(&git_diff, opts)	1233	}
		1234
		1235	/// The rows of the diff a reviewer is looking at, each carrying the anchor
		1236	/// `patch comment --at` accepts for it.
		1237	///
		1238	/// Exists so the dashboard can offer "comment on this line" without inventing
		1239	/// a second notion of which lines are anchorable. `interdiff_from` selects the
		1240	/// interdiff view; the note an interdiff prepends is dropped, since it is
		1241	/// prose for a terminal rather than a line of the change.
		1242	pub fn diff_rows_for(
		1243	repo: &Repository,
		1244	patch: &PatchState,
		1245	rev_number: u32,
		1246	interdiff_from: Option<u32>,
		1247	opts: &DiffOpts,
		1248	) -> Result<Vec<DiffRow>, Error> {
		1249	match interdiff_from {
		1250	Some(from) => {
		1251	let (note, git_diff) = interdiff_diff(repo, patch, from, rev_number, opts)?;
		1252	let mut rows = Vec::new();
		1253	for line in note.lines() {
		1254	rows.push(DiffRow {
		1255	text: format!("{}\n", line),
		1256	body: false,
		1257	anchor: None,
		1258	});
		1259	}
		1260	rows.extend(diff_rows(&git_diff)?);
		1261	Ok(rows)
		1262	}
		1263	None => diff_rows(&revision_diff(repo, patch, rev_number, opts)?),
		1264	}
1224	}	1265	}
1225		1266
1226	/// Compute the interdiff between two revisions: what the author changed in	1267	/// Compute the interdiff between two revisions: what the author changed in
@@ -1254,6 +1295,21 @@ pub fn interdiff(
1254	to_rev: u32,	1295	to_rev: u32,
1255	opts: &DiffOpts,	1296	opts: &DiffOpts,
1256	) -> Result<String, Error> {	1297	) -> Result<String, Error> {
		1298	let (note, git_diff) = interdiff_diff(repo, patch, from_rev, to_rev, opts)?;
		1299	Ok(note + &format_diff(&git_diff, opts)?)
		1300	}
		1301
		1302	/// The note and the libgit2 diff an interdiff renders from.
		1303	///
		1304	/// Split out of [`interdiff`] so the dashboard can take the rows — and with
		1305	/// them the anchors — rather than a rendered string it would have to parse.
		1306	fn interdiff_diff<'r>(
		1307	repo: &'r Repository,
		1308	patch: &PatchState,
		1309	from_rev: u32,
		1310	to_rev: u32,
		1311	opts: &DiffOpts,
		1312	) -> Result<(String, git2::Diff<'r>), Error> {
1257	let from = find_revision(patch, from_rev)?;	1313	let from = find_revision(patch, from_rev)?;
1258	let to = find_revision(patch, to_rev)?;	1314	let to = find_revision(patch, to_rev)?;
1259		1315
@@ -1269,13 +1325,13 @@ pub fn interdiff(
1269		1325
1270	let from_tree = repo.find_tree(Oid::from_str(&from.tree)?)?;	1326	let from_tree = repo.find_tree(Oid::from_str(&from.tree)?)?;
1271	let to_tree = repo.find_tree(Oid::from_str(&to.tree)?)?;	1327	let to_tree = repo.find_tree(Oid::from_str(&to.tree)?)?;
1272	let plain = \|note: String\| -> Result<String, Error> {	1328	let plain = \|note: String\| -> Result<(String, git2::Diff<'r>), Error> {
1273	let git_diff = repo.diff_tree_to_tree(	1329	let git_diff = repo.diff_tree_to_tree(
1274	Some(&from_tree),	1330	Some(&from_tree),
1275	Some(&to_tree),	1331	Some(&to_tree),
1276	Some(&mut diff_options(opts)),	1332	Some(&mut diff_options(opts)),
1277	)?;	1333	)?;
1278	Ok(note + &format_diff(&git_diff, opts)?)	1334	Ok((note, git_diff))
1279	};	1335	};
1280		1336
1281	let (older, newer) = if from_rev < to_rev {	1337	let (older, newer) = if from_rev < to_rev {
@@ -1353,7 +1409,7 @@ pub fn interdiff(
1353	} else {	1409	} else {
1354	repo.diff_tree_to_index(Some(&from_tree), Some(&replayed), Some(&mut git_opts))?	1410	repo.diff_tree_to_index(Some(&from_tree), Some(&replayed), Some(&mut git_opts))?
1355	};	1411	};
1356	Ok(note + &format_diff(&git_diff, opts)?)	1412	Ok((note, git_diff))
1357	}	1413	}
1358		1414
1359	fn find_revision(patch: &PatchState, number: u32) -> Result<&state::Revision, Error> {	1415	fn find_revision(patch: &PatchState, number: u32) -> Result<&state::Revision, Error> {
@@ -1425,11 +1481,39 @@ fn format_diff(git_diff: &git2::Diff, opts: &DiffOpts) -> Result<String, Error>
1425	let buf = stats.to_buf(git2::DiffStatsFormat::FULL, 80)?;	1481	let buf = stats.to_buf(git2::DiffStatsFormat::FULL, 80)?;
1426	return Ok(String::from_utf8_lossy(&buf).into_owned());	1482	return Ok(String::from_utf8_lossy(&buf).into_owned());
1427	}	1483	}
		1484	Ok(render_rows(&diff_rows(git_diff)?, opts.line_numbers))
		1485	}
1428		1486
1429	// `(gutter, text)`. `None` marks a line with no anchor; a header line	1487	/// One row of a rendered diff, and the anchor a comment on it would carry.
1430	// carries no gutter at all, so it is held separately from "anchorable but	1488	///
1431	// not on the new side".	1489	/// The dashboard needs the anchor of the line under its cursor, and printing a
1432	let mut rows: Vec<(Option<Option<String>>, String)> = Vec::new();	1490	/// gutter and parsing it back would be a second anchoring path — one that
		1491	/// could disagree with `--line-numbers` about which lines are anchorable at
		1492	/// all. So the rows are the shared representation and the gutter is one way of
		1493	/// rendering them.
		1494	#[derive(Debug, Clone, PartialEq)]
		1495	pub struct DiffRow {
		1496	/// The row as it prints, newline included, with no gutter.
		1497	pub text: String,
		1498	/// Whether this is a diff body line (`+`, `-`, ` `) rather than a header
		1499	/// or hunk marker. Only body rows take the gutter column.
		1500	pub body: bool,
		1501	/// The `path:line` a comment here would anchor to. `Some` only on new-side
		1502	/// lines: a deleted line is not in the revision's tree, so nothing could
		1503	/// anchor to it and `patch comment` would reject an anchor offered there.
		1504	pub anchor: Option<(String, u32)>,
		1505	}
		1506
		1507	impl DiffRow {
		1508	/// The gutter token, as `patch comment --at` accepts it.
		1509	fn anchor_token(&self) -> Option<String> {
		1510	self.anchor.as_ref().map(\|(p, n)\| format!("{}:{}", p, n))
		1511	}
		1512	}
		1513
		1514	/// Walk a libgit2 diff into rows.
		1515	fn diff_rows(git_diff: &git2::Diff) -> Result<Vec<DiffRow>, Error> {
		1516	let mut rows: Vec<DiffRow> = Vec::new();
1433	git_diff.print(DiffFormat::Patch, \|delta, _hunk, line\| {	1517	git_diff.print(DiffFormat::Patch, \|delta, _hunk, line\| {
1434	if rows.len() >= 5000 {	1518	if rows.len() >= 5000 {
1435	return true;	1519	return true;
@@ -1444,50 +1528,63 @@ fn format_diff(git_diff: &git2::Diff, opts: &DiffOpts) -> Result<String, Error>
1444	if let Ok(content) = std::str::from_utf8(line.content()) {	1528	if let Ok(content) = std::str::from_utf8(line.content()) {
1445	text.push_str(content);	1529	text.push_str(content);
1446	}	1530	}
1447	let gutter = if opts.line_numbers && prefix.is_some() {	1531	let anchor = if prefix.is_some() {
1448	Some(line.new_lineno().and_then(\|n\| {	1532	line.new_lineno().and_then(\|n\| {
1449	delta	1533	delta
1450	.new_file()	1534	.new_file()
1451	.path()	1535	.path()
1452	.map(\|p\| format!("{}:{}", p.display(), n))	1536	.map(\|p\| (p.display().to_string(), n))
1453	}))	1537	})
1454	} else {	1538	} else {
1455	None	1539	None
1456	};	1540	};
1457	rows.push((gutter, text));	1541	rows.push(DiffRow {
		1542	text,
		1543	body: prefix.is_some(),
		1544	anchor,
		1545	});
1458	true	1546	true
1459	})?;	1547	})?;
1460		1548
1461	let truncated = rows.len() >= 5000;	1549	if rows.len() >= 5000 {
1462	let width = rows	1550	rows.push(DiffRow {
1463	.iter()	1551	text: "\n[truncated at 5000 lines]".to_string(),
1464	.filter_map(\|(g, _)\| g.as_ref().and_then(\|a\| a.as_ref()).map(\|s\| s.chars().count()))	1552	body: false,
1465	.max()	1553	anchor: None,
1466	.unwrap_or(0);	1554	});
		1555	}
		1556	Ok(rows)
		1557	}
		1558
		1559	/// Render rows back to a unified diff, optionally with the anchor gutter.
		1560	fn render_rows(rows: &[DiffRow], line_numbers: bool) -> String {
		1561	let width = if line_numbers {
		1562	rows.iter()
		1563	.filter_map(\|r\| r.anchor_token().map(\|s\| s.chars().count()))
		1564	.max()
		1565	.unwrap_or(0)
		1566	} else {
		1567	0
		1568	};
1467		1569
1468	let mut output = String::new();	1570	let mut output = String::new();
1469	for (gutter, text) in &rows {	1571	for row in rows {
1470	match gutter {	1572	if line_numbers && row.body {
1471	Some(Some(anchor)) => {	1573	match row.anchor_token() {
1472	output.push_str(anchor);	1574	Some(anchor) => {
1473	for _ in anchor.chars().count()..width {	1575	output.push_str(&anchor);
		1576	for _ in anchor.chars().count()..width {
		1577	output.push(' ');
		1578	}
1474	output.push(' ');	1579	output.push(' ');
1475	}	1580	}
1476	output.push(' ');	1581	// Anchorable position, no new-side line: keep the body aligned.
		1582	None => output.push_str(&" ".repeat(width + 1)),
1477	}	1583	}
1478	// Anchorable position, no new-side line: keep the body aligned.
1479	Some(None) => output.push_str(&" ".repeat(width + 1)),
1480	// Headers and hunk markers stay flush left.
1481	None => {}
1482	}	1584	}
1483	output.push_str(text);	1585	output.push_str(&row.text);
1484	}	1586	}
1485		1587	output
1486	if truncated {
1487	output.push_str("\n[truncated at 5000 lines]");
1488	}
1489
1490	Ok(output)
1491	}	1588	}
1492		1589
1493	/// Patch log: list all revisions with timestamps and file-change summaries.	1590	/// Patch log: list all revisions with timestamps and file-change summaries.

src/tui/events.rs

@@ @@ -1,28 +1,40 @@ @@
+use std::io::{self, stdout};
+use std::time::{Duration, Instant};
+use crossterm::event::{self, Event, KeyCode};
+use crossterm::terminal::{self, EnterAlternateScreen, LeaveAlternateScreen};
+use crossterm::ExecutableCommand;
+use git2::Repository;
+use ratatui::prelude::*;
+use ratatui::widgets::ListState;
+use crate::error::Error;
+use crate::event::ReviewVerdict;
+use crate::issue as issue_mod;
+use crate::patch as patch_mod;
+use super::state::{App, InputMode, KeyAction, ViewMode};
+use super::widgets::{ui, RowTarget};
+/// How often the collab tips are compared. The loop already wakes every 100ms
+/// for input, so this costs a ref read every twentieth wake-up next to a
+/// redraw that was happening anyway.
+const TIP_POLL: Duration = Duration::from_secs(2);
+pub(crate) fn run_loop(
+    terminal: &mut Terminal<CrosstermBackend<io::Stdout>>,
+    app: &mut App,
+    repo: &Repository,
+) -> Result<(), Error> {
+    let mut last_poll = Instant::now();
+    loop {
+        terminal.draw(|frame| ui(frame, app, Some(repo)))?;
+        if last_poll.elapsed() >= TIP_POLL {
+            last_poll = Instant::now();
+            app.poll_tips(repo);
+        }
+        if event::poll(Duration::from_millis(100))? {
+            if let Event::Key(key) = event::read()? {
+                app.status_msg = None; // clear status on any keypress
@@ @@ -60,68 +72,31 @@ pub(crate) fn run_loop( @@
+                                app.input_mode = InputMode::Normal;
+                                app.input_buf.clear();
+                            // The title is one line and is a prompt, so it
+                            // stays here. The body is markdown and is prose,
+                            // so it goes where prose goes: there is no second,
+                            // worse text editor in this program any more.
+                            KeyCode::Enter => {
+                                let title = app.input_buf.trim().to_string();
+                                app.input_mode = InputMode::Normal;
+                                app.input_buf.clear();
+                                if title.is_empty() {
+                                    continue;
+                                app.create_title = title.clone();
+                                let seed = comment_seed(&format!("New issue: {}", title), vec![]);
+                                match compose(terminal, &seed) {
+                                    Ok(Composed::Body(body)) => create_issue(app, repo, &body),
+                                    Ok(Composed::Aborted) => {
+                                        app.create_title.clear();
+                                        app.status_msg =
+                                            Some("Aborting issue: empty message.".to_string());
+                                    Err(e) => {
+                                        app.create_title.clear();
+                                        app.status_msg = Some(e.to_string());
+                            KeyCode::Backspace => {
+                                app.input_buf.pop();
@@ @@ -133,6 +108,19 @@ pub(crate) fn run_loop( @@
+                        continue;
+                    InputMode::ReviewVerdict => {
+                        let verdict = match key.code {
+                            KeyCode::Char('a') => Some(ReviewVerdict::Approve),
+                            KeyCode::Char('r') => Some(ReviewVerdict::RequestChanges),
+                            KeyCode::Char('c') => Some(ReviewVerdict::Comment),
+                            _ => None,
+                        };
+                        app.input_mode = InputMode::Normal;
+                        if let Some(verdict) = verdict {
+                            submit_review(terminal, app, repo, verdict);
+                        }
+                        continue;
+                    }
+                    InputMode::Normal => {}
@@ @@ -152,46 +140,7 @@ pub(crate) fn run_loop( @@
+                            continue;
+                        KeyCode::Char('o') => {
+                            return checkout_and_leave(app, repo);
+                        _ => {}
@@ @@ -233,6 +182,10 @@ pub(crate) fn run_loop( @@
+                    KeyAction::Comment => submit_comment(terminal, app, repo),
+                    KeyAction::ToggleResolve => toggle_resolve(app, repo),
+                    KeyAction::ShowAnswers => show_answers(app, repo),
+                    KeyAction::Checkout => return checkout_and_leave(app, repo),
+                    KeyAction::Continue => {}
@@ @@ -240,32 +193,329 @@ pub(crate) fn run_loop( @@
+// ── Writing from the dashboard ──────────────────────────────────────────────
+/// The result of asking the user for prose.
+enum Composed {
+    Body(String),
+    /// Nothing was left after the `#` lines came out — the same "aborting due
+    /// to empty message" `git commit` gives, and for the same reason: an
+    /// accident recorded in an append-only DAG is permanent.
+    Aborted,
+}
+/// Leave the alternate screen for as long as this lives.
+///
+/// A `Drop` rather than a pair of calls so the screen comes back however the
+/// editor ends — a non-zero exit, a signal, or a panic on the way out. The
+/// terminal belongs to the user, and handing it back is not conditional on
+/// the write succeeding.
+struct Suspended;
+impl Suspended {
+    fn enter() -> Result<Self, Error> {
+        terminal::disable_raw_mode()?;
+        stdout().execute(LeaveAlternateScreen)?;
+        Ok(Suspended)
+    }
+}
+impl Drop for Suspended {
+    fn drop(&mut self) {
+        let _ = stdout().execute(EnterAlternateScreen);
+        let _ = terminal::enable_raw_mode();
+    }
+}
+/// Suspend the dashboard, compose a body in `$EDITOR`, and come back.
+///
+/// The same convention as `git commit`, and the same one the CLI settled on:
+/// no second text editor gets written here. `#` lines are stripped on the way
+/// back, because the buffer is seeded with the thing being commented on — a
+/// reviewer should not lose the hunk they were reading in order to write about
+/// it. That does mean a `#` heading cannot be written from this surface; the
+/// CLI, which seeds nothing, keeps them.
+fn compose(
+    terminal: &mut Terminal<CrosstermBackend<io::Stdout>>,
+    seed: &str,
+) -> Result<Composed, Error> {
+    // Checked before anything moves: an editor that does not exist must be a
+    // sentence on the status line, never a hang and never a silent no-op.
+    if crate::editor::resolve_editor().is_none() {
+        return Err(Error::Cmd(
+            "No editor configured: set $EDITOR or $VISUAL, or use `git-collab patch comment` \
+             with --body / --body-file."
+                .to_string(),
+        ));
+    }
+    let composed = {
+        let _screen = Suspended::enter()?;
+        crate::editor::compose(seed)
+    };
+    // Whatever happened out there, the screen is ours again and holds someone
+    // else's output.
+    terminal.clear()?;
+    let body = strip_comments(&composed?);
+    if body.trim().is_empty() {
+        return Ok(Composed::Aborted);
+    }
+    Ok(Composed::Body(body))
+}
+/// Drop the `#` lines the buffer was seeded with.
+fn strip_comments(text: &str) -> String {
+    let kept: Vec<&str> = text
+        .lines()
+        .filter(|l| !l.trim_start().starts_with('#'))
+        .collect();
+    kept.join("\n")
+}
+/// Seed an editor buffer with what is being written about.
+fn comment_seed(headline: &str, context: Vec<String>) -> String {
+    let mut seed = format!(
+        "# {}\n# Lines starting with # are ignored; an empty message aborts.\n\n\n",
+        headline
+    );
+    if !context.is_empty() {
+        seed.push_str("\n# --- the lines you are commenting on ---\n");
+        for line in context {
+            seed.push_str(&format!("# {}\n", line));
+        }
+    }
+    seed
+}
+/// The rows around the cursor, as `#` context for the editor buffer.
+fn cursor_context(app: &App) -> Vec<String> {
+    let start = app.patch_cursor.saturating_sub(5);
+    let end = (app.patch_cursor + 4).min(app.patch_rows.len());
+    app.patch_rows[start..end]
+        .iter()
+        .enumerate()
+        .map(|(i, row)| {
+            let marker = if start + i == app.patch_cursor {
+                ">"
+            } else {
+                " "
+            };
+            format!("{}{}", marker, row.line)
+        })
+        .collect()
+}
+fn create_issue(app: &mut App, repo: &Repository, body: &str) {
+    let title = app.create_title.clone();
+    match issue_mod::open(repo, &title, body, None) {
+        Ok(id) => {
+            app.reload(repo);
+            app.status_msg = Some(format!("Issue created: {}", app.issue_abbrev.of(&id)));
+        }
+        Err(e) => {
+            app.status_msg = Some(format!("Error creating issue: {}", e));
+        }
+    }
+    app.input_mode = InputMode::Normal;
+    app.input_buf.clear();
+    app.create_title.clear();
+}
+fn submit_comment(
+    terminal: &mut Terminal<CrosstermBackend<io::Stdout>>,
+    app: &mut App,
+    repo: &Repository,
+) {
+    let Some(patch) = app.current_patch.clone() else {
+        return;
+    };
+    let short = app.patch_abbrev.of(&patch.id).to_string();
+    let revision = patch.revisions.get(app.patch_revision_idx).map(|r| r.number);
+    // Inline when the cursor is on a line of the diff, on the thread
+    // otherwise. Both go through `patch::comment`, which is the CLI's own
+    // `--at` path: the anchor is checked against the revision the comment
+    // names, not against whatever the working tree happens to hold.
+    let (headline, anchor) = match app.cursor_target() {
+        RowTarget::Diff { file, line } => (
+            format!("Commenting on {}:{} of patch {}", file, line, short),
+            Some((file, line)),
+        ),
+        _ => (
+            format!("Commenting on patch {}: {}", short, patch.title),
+            None,
+        ),
+    };
+    let seed = comment_seed(&headline, cursor_context(app));
+    match compose(terminal, &seed) {
+        Ok(Composed::Body(body)) => {
+            let args = crate::body::BodyArgs::new(Some(&body), None);
+            let result = match &anchor {
+                Some((file, line)) => patch_mod::comment(
+                    repo,
+                    &patch.id,
+                    &args,
+                    Some(file),
+                    Some(*line),
+                    revision,
+                    false,
+                ),
+                None => patch_mod::comment(repo, &patch.id, &args, None, None, None, false),
+            };
+            match result {
+                Ok(report) => {
+                    reload_patch(app, repo);
+                    app.status_msg = Some(report.placement.to_string());
+                }
+                Err(e) => app.status_msg = Some(format!("Comment failed: {}", e)),
+            }
+        }
+        Ok(Composed::Aborted) => {
+            app.status_msg = Some("Aborting comment: empty message.".to_string())
+        }
+        Err(e) => app.status_msg = Some(e.to_string()),
+    }
+}
+fn submit_review(
+    terminal: &mut Terminal<CrosstermBackend<io::Stdout>>,
+    app: &mut App,
+    repo: &Repository,
+    verdict: ReviewVerdict,
+) {
+    let Some(patch) = app.current_patch.clone() else {
+        return;
+    };
+    let short = app.patch_abbrev.of(&patch.id).to_string();
+    let revision = patch.revisions.get(app.patch_revision_idx).map(|r| r.number);
+    let seed = comment_seed(
+        &format!(
+            "Review of patch {}: {}\n# Verdict: {}",
+            short, patch.title, verdict
+        ),
+        Vec::new(),
+    );
+    match compose(terminal, &seed) {
+        Ok(Composed::Body(body)) => {
+            match patch_mod::review(repo, &patch.id, verdict, &body, revision) {
+                Ok(_) => {
+                    reload_patch(app, repo);
+                    app.status_msg = Some(format!("Review recorded: {}", verdict));
+                }
+                Err(e) => app.status_msg = Some(format!("Review failed: {}", e)),
+            }
+        }
+        Ok(Composed::Aborted) => {
+            app.status_msg = Some("Aborting review: empty message.".to_string())
+        }
+        Err(e) => app.status_msg = Some(e.to_string()),
+    }
+}
+fn toggle_resolve(app: &mut App, repo: &Repository) {
+    let Some(patch) = app.current_patch.clone() else {
+        return;
+    };
+    let RowTarget::Comment { oid, resolved } = app.cursor_target() else {
+        app.status_msg =
+            Some("Move to an inline comment to mark it answered (j/k).".to_string());
+        return;
+    };
+    let outcome = if resolved {
+        patch_mod::unresolve(repo, &patch.id, &oid).map(|_| "reopened")
+    } else {
+        patch_mod::resolve(repo, &patch.id, &oid, None).map(|_| "resolved")
+    };
+    match outcome {
+        Ok(what) => {
+            reload_patch(app, repo);
+            app.status_msg = Some(format!("Comment {:.8} {}", oid, what));
+        }
+        Err(e) => app.status_msg = Some(format!("Could not update the comment: {}", e)),
+    }
+}
+fn show_answers(app: &mut App, repo: &Repository) {
+    let Some(patch) = app.current_patch.clone() else {
+        return;
+    };
+    let RowTarget::Comment { oid, .. } = app.cursor_target() else {
+        app.status_msg = Some("Move to an inline comment to see what answered it.".to_string());
+        return;
+    };
+    match patch_mod::diff(repo, &patch.id, None, None, Some(&oid), &Default::default()) {
+        Ok(text) => {
+            app.set_diff_text(&text);
+            app.patch_answers = Some(oid);
+            app.patch_cursor = 0;
+            app.patch_scroll = 0;
+            app.rebuild_patch_rows();
+        }
+        Err(e) => app.status_msg = Some(format!("{}", e)),
+    }
+}
+/// Leave the dashboard for a checkout of the patch's latest revision.
+///
+/// `patch::checkout` is the same code `git-collab patch checkout` runs: it
+/// takes the latest revision's commit, which a revision ref pins and which
+/// therefore always resolves, and leaves a branch behind with instructions for
+/// getting back. The old behaviour checked out `PatchState.branch`, a
+/// provenance field naming a worktree that on an agent-produced patch no
+/// longer exists.
+fn checkout_and_leave(app: &mut App, repo: &Repository) -> Result<(), Error> {
+    let Some(id) = app.checkout_target() else {
+        app.status_msg = Some("No patch to check out here.".to_string());
+        return Ok(());
+    };
+    terminal::disable_raw_mode()?;
+    stdout().execute(LeaveAlternateScreen)?;
+    match patch_mod::checkout(repo, &id) {
+        Ok(report) => patch_mod::report_checkout(repo, &report),
+        Err(e) => eprintln!("Checkout failed: {}", e),
+    }
+    Ok(())
+}
+/// Re-read the patch on screen after the dashboard wrote to it.
+///
+/// `reload` re-baselines the tip snapshot as part of its job, which is what
+/// keeps the staleness banner from announcing the reader's own comment back to
+/// them as somebody else's news.
+fn reload_patch(app: &mut App, repo: &Repository) {
+    app.reload(repo);
+    if let Some(id) = app.current_patch.as_ref().map(|p| p.id.clone()) {
+        if let Some(fresh) = app.patches.iter().find(|p| p.id == id).cloned() {
+            app.patch_revision_idx = app
+                .patch_revision_idx
+                .min(fresh.revisions.len().saturating_sub(1));
+            app.current_patch = Some(fresh);
+        }
+    }
+    regenerate_patch_diff(app, repo);
+}
+// ── Reading ─────────────────────────────────────────────────────────────────
+fn generate_patch_diff_for(
+    repo: &Repository,
+    patch: &crate::state::PatchState,
+    rev_idx: usize,
+    interdiff_mode: bool,
+) -> Result<Vec<patch_mod::DiffRow>, Error> {
+    if patch.revisions.is_empty() {
+        return Err(Error::Cmd("(no revisions)".to_string()));
+    let rev = &patch.revisions[rev_idx];
+    let from = if interdiff_mode && rev_idx > 0 {
+        Some(patch.revisions[rev_idx - 1].number)
+    } else {
+        None
+    };
+    patch_mod::diff_rows_for(repo, patch, rev.number, from, &Default::default())
+fn open_patch_detail(app: &mut App, repo: &Repository, patch: crate::state::PatchState) {
+    app.patch_revision_idx = patch.revisions.len().saturating_sub(1);
+    app.patch_interdiff_mode = false;
+    app.patch_scroll = 0;
+    app.patch_cursor = 0;
+    app.patch_answers = None;
+    app.current_patch = Some(patch);
+    app.mode = ViewMode::PatchDetail;
+    regenerate_patch_diff(app, repo);
+    if let Some(w) = warning {
+        app.status_msg = Some(w);
+fn regenerate_patch_diff(app: &mut App, repo: &Repository) {
+    if let Some(ref patch) = app.current_patch {
+        match generate_patch_diff_for(repo, patch, app.patch_revision_idx, app.patch_interdiff_mode)
+        {
+            Ok(rows) => app.patch_diff = rows,
+            Err(e) => app.set_diff_text(&format!("(error generating diff: {})", e)),
+        }
+    }
+    app.rebuild_patch_rows();
+}
+#[cfg(test)]
+mod tests {
+    use super::*;
+    /// The seeded context comes back out, so what the reviewer wrote is what
+    /// gets recorded — not the hunk they were reading.
+    #[test]
+    fn context_lines_do_not_reach_the_body() {
+        let stripped = strip_comments(
+            "# Commenting on src/lib.rs:3\n# Lines starting with # are ignored.\n\nwhy?\n\n# ---\n#  let x = 1;\n",
+        );
+        assert_eq!(stripped.trim(), "why?");
+    }
+    /// The same rule `git commit` uses: an untouched buffer is an abort, not a
+    /// comment made of hash marks.
+    #[test]
+    fn an_untouched_buffer_is_blank() {
+        let seed = comment_seed("Commenting on src/lib.rs:3", vec![" let x = 1;".to_string()]);
+        assert!(strip_comments(&seed).trim().is_empty());
+    }
+    /// Only a leading `#` is a comment marker; a `#` inside a line is prose.
+    #[test]
+    fn a_hash_inside_a_line_survives() {
+        assert_eq!(strip_comments("issue #12 is the same bug"), "issue #12 is the same bug");
+    }
+    /// The buffer says what is being written about and where the cursor was,
+    /// so suspending does not cost the reviewer the thing they were reading.
+    #[test]
+    fn the_seed_carries_the_hunk() {
+        let seed = comment_seed(
+            "Commenting on src/lib.rs:3",
+            vec![" fn a() {".to_string(), "+    b();".to_string()],
+        );
+        assert!(seed.starts_with("# Commenting on src/lib.rs:3\n"));
+        assert!(seed.contains("#  fn a() {\n"));
+        assert!(seed.contains("# +    b();\n"));

src/tui/mod.rs

Old		New
@@ -1,5 +1,6 @@
1	mod events;	1	mod events;
2	mod state;	2	mod state;
		3	mod tips;
3	mod widgets;	4	mod widgets;
4		5
5	use std::io::stdout;	6	use std::io::stdout;
@@ -25,6 +26,9 @@ pub fn run(repo: &Repository) -> Result<(), Error> {
25	let patch_abbrev = crate::abbrev::for_patches(repo);	26	let patch_abbrev = crate::abbrev::for_patches(repo);
26		27
27	let mut app = App::new(issues, patches, issue_abbrev, patch_abbrev);	28	let mut app = App::new(issues, patches, issue_abbrev, patch_abbrev);
		29	// Where the refs stood when the lists above were folded. Without this the
		30	// first tip poll would report the whole repository as news.
		31	app.rebaseline(repo);
28		32
29	terminal::enable_raw_mode()?;	33	terminal::enable_raw_mode()?;
30	stdout().execute(EnterAlternateScreen)?;	34	stdout().execute(EnterAlternateScreen)?;
@@ -199,24 +203,6 @@ mod tests {
199	}	203	}
200		204
201	#[test]	205	#[test]
202	fn test_create_title_transitions_to_body() {
203	let mut app = test_app();
204	app.input_mode = InputMode::CreateTitle;
205	app.input_buf = "My new issue".into();
206
207	// Simulate Enter with non-empty title
208	let title = app.input_buf.trim().to_string();
209	assert!(!title.is_empty());
210	app.create_title = title;
211	app.input_buf.clear();
212	app.input_mode = InputMode::CreateBody;
213
214	assert_eq!(app.input_mode, InputMode::CreateBody);
215	assert_eq!(app.create_title, "My new issue");
216	assert!(app.input_buf.is_empty());
217	}
218
219	#[test]
220	fn test_empty_title_dismissed() {	206	fn test_empty_title_dismissed() {
221	let mut app = test_app();	207	let mut app = test_app();
222	app.input_mode = InputMode::CreateTitle;	208	app.input_mode = InputMode::CreateTitle;
@@ -1223,7 +1209,7 @@ mod tests {
1223	app.patch_scroll = 5;	1209	app.patch_scroll = 5;
1224	app.patch_revision_idx = 1;	1210	app.patch_revision_idx = 1;
1225	app.patch_interdiff_mode = true;	1211	app.patch_interdiff_mode = true;
1226	app.patch_diff = "some diff".into();	1212	app.set_diff_text("some diff");
1227		1213
1228	let result = app.handle_key(	1214	let result = app.handle_key(
1229	crossterm::event::KeyCode::Esc,	1215	crossterm::event::KeyCode::Esc,
@@ -1249,45 +1235,76 @@ mod tests {
1249	assert_eq!(result, KeyAction::Quit);	1235	assert_eq!(result, KeyAction::Quit);
1250	}	1236	}
1251		1237
		1238	/// `j`/`k` move a cursor now, not the viewport. The pane has to have a
		1239	/// selected line for `c` and `x` to mean "this one", and a scroll offset
		1240	/// with no cursor cannot say which line a reader is looking at.
1252	#[test]	1241	#[test]
1253	fn test_patch_detail_scroll() {	1242	fn test_patch_detail_cursor_moves_line_by_line() {
1254	let mut app = test_app();	1243	let mut app = test_app();
1255	app.mode = ViewMode::PatchDetail;	1244	app.mode = ViewMode::PatchDetail;
1256	app.current_patch = Some(make_patch_with_revisions());	1245	app.current_patch = Some(make_patch_with_revisions());
1257	app.patch_scroll = 0;	1246	app.rebuild_patch_rows();
1258		1247
1259	app.handle_key(	1248	app.handle_key(
1260	crossterm::event::KeyCode::Char('j'),	1249	crossterm::event::KeyCode::Char('j'),
1261	crossterm::event::KeyModifiers::empty(),	1250	crossterm::event::KeyModifiers::empty(),
1262	);	1251	);
1263	assert_eq!(app.patch_scroll, 1);	1252	assert_eq!(app.patch_cursor, 1);
1264	app.handle_key(	1253	app.handle_key(
1265	crossterm::event::KeyCode::Char('j'),	1254	crossterm::event::KeyCode::Char('j'),
1266	crossterm::event::KeyModifiers::empty(),	1255	crossterm::event::KeyModifiers::empty(),
1267	);	1256	);
1268	assert_eq!(app.patch_scroll, 2);	1257	assert_eq!(app.patch_cursor, 2);
1269	app.handle_key(	1258	app.handle_key(
1270	crossterm::event::KeyCode::Char('k'),	1259	crossterm::event::KeyCode::Char('k'),
1271	crossterm::event::KeyModifiers::empty(),	1260	crossterm::event::KeyModifiers::empty(),
1272	);	1261	);
1273	assert_eq!(app.patch_scroll, 1);	1262	assert_eq!(app.patch_cursor, 1);
		1263	// Two lines down in a pane twenty lines tall is still on screen, so
		1264	// nothing has scrolled: the text under a reader must not jump.
		1265	assert_eq!(app.patch_scroll, 0);
1274	}	1266	}
1275		1267
		1268	/// The cursor clamps to the last row rather than running off the end.
1276	#[test]	1269	#[test]
1277	fn test_patch_detail_page_scroll() {	1270	fn test_patch_detail_cursor_clamps_at_the_end() {
1278	let mut app = test_app();	1271	let mut app = test_app();
1279	app.mode = ViewMode::PatchDetail;	1272	app.mode = ViewMode::PatchDetail;
1280	app.patch_scroll = 0;	1273	app.current_patch = Some(make_patch_with_revisions());
		1274	app.rebuild_patch_rows();
		1275	let last = app.patch_rows.len() - 1;
		1276
		1277	for _ in 0..500 {
		1278	app.handle_key(
		1279	crossterm::event::KeyCode::Char('j'),
		1280	crossterm::event::KeyModifiers::empty(),
		1281	);
		1282	}
		1283	assert_eq!(app.patch_cursor, last);
		1284	}
		1285
		1286	/// Once the cursor leaves the visible rows, the view follows it — and by
		1287	/// exactly as much as it has to.
		1288	#[test]
		1289	fn test_patch_detail_scroll_follows_the_cursor() {
		1290	let mut app = test_app();
		1291	app.mode = ViewMode::PatchDetail;
		1292	app.current_patch = Some(make_patch_with_revisions());
		1293	app.set_diff_text(&(0..100).map(\|i\| format!(" line {}\n", i)).collect::<String>());
		1294	app.rebuild_patch_rows();
		1295	app.patch_viewport = 10;
1281		1296
1282	app.handle_key(	1297	app.handle_key(
1283	crossterm::event::KeyCode::PageDown,	1298	crossterm::event::KeyCode::PageDown,
1284	crossterm::event::KeyModifiers::empty(),	1299	crossterm::event::KeyModifiers::empty(),
1285	);	1300	);
1286	assert_eq!(app.patch_scroll, 20);	1301	assert_eq!(app.patch_cursor, 20);
		1302	assert_eq!(app.patch_scroll, 11);
1287	app.handle_key(	1303	app.handle_key(
1288	crossterm::event::KeyCode::PageUp,	1304	crossterm::event::KeyCode::PageUp,
1289	crossterm::event::KeyModifiers::empty(),	1305	crossterm::event::KeyModifiers::empty(),
1290	);	1306	);
		1307	assert_eq!(app.patch_cursor, 0);
1291	assert_eq!(app.patch_scroll, 0);	1308	assert_eq!(app.patch_scroll, 0);
1292	}	1309	}
1293		1310
@@ -1361,7 +1378,8 @@ mod tests {
1361	app.mode = ViewMode::PatchDetail;	1378	app.mode = ViewMode::PatchDetail;
1362	app.current_patch = Some(make_patch_with_revisions());	1379	app.current_patch = Some(make_patch_with_revisions());
1363	app.patch_revision_idx = 1;	1380	app.patch_revision_idx = 1;
1364	app.patch_diff = "+added line\n-removed line\n context".into();	1381	app.set_diff_text("+added line\n-removed line\n context");
		1382	app.rebuild_patch_rows();
1365		1383
1366	let buf = render_app(&mut app);	1384	let buf = render_app(&mut app);
1367	assert_buffer_contains(&buf, "Patch Detail");	1385	assert_buffer_contains(&buf, "Patch Detail");
@@ -1375,7 +1393,8 @@ mod tests {
1375	let mut app = make_app(3, 0);	1393	let mut app = make_app(3, 0);
1376	app.mode = ViewMode::PatchDetail;	1394	app.mode = ViewMode::PatchDetail;
1377	app.current_patch = Some(make_patch_with_revisions());	1395	app.current_patch = Some(make_patch_with_revisions());
1378	app.patch_diff = String::new();	1396	app.patch_diff.clear();
		1397	app.rebuild_patch_rows();
1379		1398
1380	let buf = render_app(&mut app);	1399	let buf = render_app(&mut app);
1381	assert_buffer_contains(&buf, "needs-review");	1400	assert_buffer_contains(&buf, "needs-review");
@@ -1387,7 +1406,8 @@ mod tests {
1387	app.mode = ViewMode::PatchDetail;	1406	app.mode = ViewMode::PatchDetail;
1388	app.current_patch = Some(make_patch_with_revisions());	1407	app.current_patch = Some(make_patch_with_revisions());
1389	app.patch_revision_idx = 1;	1408	app.patch_revision_idx = 1;
1390	app.patch_diff = String::new();	1409	app.patch_diff.clear();
		1410	app.rebuild_patch_rows();
1391		1411
1392	let buf = render_app(&mut app);	1412	let buf = render_app(&mut app);
1393	assert_buffer_contains(&buf, "Reviews");	1413	assert_buffer_contains(&buf, "Reviews");
@@ -1395,15 +1415,27 @@ mod tests {
1395	assert_buffer_contains(&buf, "LGTM");	1415	assert_buffer_contains(&buf, "LGTM");
1396	}	1416	}
1397		1417
		1418	/// The review loop has to be discoverable, so those keys come first and
		1419	/// survive an eighty-column terminal. The rest is on the same line and
		1420	/// clips there, which is why it is ordered the way it is.
1398	#[test]	1421	#[test]
1399	fn test_render_patch_detail_footer() {	1422	fn test_render_patch_detail_footer() {
1400	let mut app = make_app(3, 0);	1423	let mut app = make_app(3, 0);
1401	app.mode = ViewMode::PatchDetail;	1424	app.mode = ViewMode::PatchDetail;
1402	let buf = render_app(&mut app);	1425	let buf = render_app(&mut app);
		1426	assert_buffer_contains(&buf, "c:comment");
		1427	assert_buffer_contains(&buf, "R:review");
		1428	assert_buffer_contains(&buf, "x:resolve");
		1429	assert_buffer_contains(&buf, "a:answers");
1403	assert_buffer_contains(&buf, "Esc:back");	1430	assert_buffer_contains(&buf, "Esc:back");
1404	assert_buffer_contains(&buf, "[/]:revision");	1431
1405	// "d:interdiff" may be truncated at 80 cols, check prefix	1432	let backend = TestBackend::new(120, 24);
1406	assert_buffer_contains(&buf, "d:inter");	1433	let mut terminal = Terminal::new(backend).unwrap();
		1434	terminal.draw(\|frame\| ui(frame, &mut app, None)).unwrap();
		1435	let wide = terminal.backend().buffer().clone();
		1436	assert_buffer_contains(&wide, "[/]:revision");
		1437	assert_buffer_contains(&wide, "d:interdiff");
		1438	assert_buffer_contains(&wide, "o:checkout");
1407	}	1439	}
1408		1440
1409	#[test]	1441	#[test]
@@ -1411,6 +1443,7 @@ mod tests {
1411	let mut app = make_app(3, 0);	1443	let mut app = make_app(3, 0);
1412	app.mode = ViewMode::PatchDetail;	1444	app.mode = ViewMode::PatchDetail;
1413	app.current_patch = None;	1445	app.current_patch = None;
		1446	app.rebuild_patch_rows();
1414		1447
1415	let buf = render_app(&mut app);	1448	let buf = render_app(&mut app);
1416	assert_buffer_contains(&buf, "No patch loaded");	1449	assert_buffer_contains(&buf, "No patch loaded");
@@ -1444,8 +1477,9 @@ mod tests {
1444	// Simulate what events.rs does	1477	// Simulate what events.rs does
1445	app.current_patch = Some(make_patch_with_revisions());	1478	app.current_patch = Some(make_patch_with_revisions());
1446	app.patch_revision_idx = 1;	1479	app.patch_revision_idx = 1;
1447	app.patch_diff = "diff content".into();	1480	app.set_diff_text("diff content");
1448	app.mode = ViewMode::PatchDetail;	1481	app.mode = ViewMode::PatchDetail;
		1482	app.rebuild_patch_rows();
1449		1483
1450	// Navigate revisions	1484	// Navigate revisions
1451	app.handle_key(	1485	app.handle_key(
@@ -1461,12 +1495,12 @@ mod tests {
1461	);	1495	);
1462	assert!(app.patch_interdiff_mode);	1496	assert!(app.patch_interdiff_mode);
1463		1497
1464	// Scroll	1498	// Walk a line down
1465	app.handle_key(	1499	app.handle_key(
1466	crossterm::event::KeyCode::Char('j'),	1500	crossterm::event::KeyCode::Char('j'),
1467	crossterm::event::KeyModifiers::empty(),	1501	crossterm::event::KeyModifiers::empty(),
1468	);	1502	);
1469	assert_eq!(app.patch_scroll, 1);	1503	assert_eq!(app.patch_cursor, 1);
1470		1504
1471	// Escape back	1505	// Escape back
1472	app.handle_key(	1506	app.handle_key(
@@ -1477,6 +1511,162 @@ mod tests {
1477	assert!(app.current_patch.is_none());	1511	assert!(app.current_patch.is_none());
1478	}	1512	}
1479		1513
		1514	// ── The review surface ───────────────────────────────────────────────
		1515
		1516	/// A `+` line of the diff carries the anchor `patch comment --at` accepts,
		1517	/// so `c` there is an inline comment and not a thread one. The anchor is
		1518	/// the one the diff machinery computed, not one reconstructed here.
		1519	#[test]
		1520	fn test_a_diff_line_carries_its_anchor() {
		1521	let mut app = make_app(0, 0);
		1522	app.mode = ViewMode::PatchDetail;
		1523	app.current_patch = Some(make_patch_with_revisions());
		1524	app.patch_diff = vec![
		1525	crate::patch::DiffRow {
		1526	text: "@@ -1,2 +1,3 @@\n".into(),
		1527	body: false,
		1528	anchor: None,
		1529	},
		1530	crate::patch::DiffRow {
		1531	text: "+let x = 1;\n".into(),
		1532	body: true,
		1533	anchor: Some(("src/main.rs".to_string(), 42)),
		1534	},
		1535	];
		1536	app.rebuild_patch_rows();
		1537
		1538	let anchored: Vec<&RowTarget> = app
		1539	.patch_rows
		1540	.iter()
		1541	.map(\|r\| &r.target)
		1542	.filter(\|t\| matches!(t, RowTarget::Diff { .. }))
		1543	.collect();
		1544	assert_eq!(
		1545	anchored,
		1546	vec![&RowTarget::Diff {
		1547	file: "src/main.rs".to_string(),
		1548	line: 42
		1549	}]
		1550	);
		1551	}
		1552
		1553	/// A deleted line is not in the revision's tree, so it offers no anchor —
		1554	/// the same rule `patch diff --line-numbers` follows. `c` there falls back
		1555	/// to a thread comment rather than proposing an anchor that would be
		1556	/// rejected.
		1557	#[test]
		1558	fn test_a_deleted_line_offers_no_anchor() {
		1559	let mut app = make_app(0, 0);
		1560	app.mode = ViewMode::PatchDetail;
		1561	app.current_patch = Some(make_patch_with_revisions());
		1562	app.patch_diff = vec![crate::patch::DiffRow {
		1563	text: "-gone\n".into(),
		1564	body: true,
		1565	anchor: None,
		1566	}];
		1567	app.rebuild_patch_rows();
		1568
		1569	assert!(app
		1570	.patch_rows
		1571	.iter()
		1572	.all(\|r\| !matches!(r.target, RowTarget::Diff { .. })));
		1573	}
		1574
		1575	/// `x` needs to name a comment, and it has to name it the way `patch show`
		1576	/// does, because a reviewer reads the id off one surface and types it into
		1577	/// the other.
		1578	#[test]
		1579	fn test_an_inline_comment_row_carries_the_id_patch_show_prints() {
		1580	let mut app = make_app(0, 0);
		1581	app.mode = ViewMode::PatchDetail;
		1582	let patch = make_patch_with_revisions();
		1583	let expected = patch.inline_comments[0].commit_id.to_string();
		1584	app.current_patch = Some(patch);
		1585	app.rebuild_patch_rows();
		1586
		1587	let found = app
		1588	.patch_rows
		1589	.iter()
		1590	.find_map(\|r\| match &r.target {
		1591	RowTarget::Comment { oid, resolved } => Some((oid.clone(), *resolved)),
		1592	_ => None,
		1593	})
		1594	.expect("an inline comment row");
		1595	assert_eq!(found, (expected.clone(), false));
		1596	// And the same eight characters are on screen, in brackets.
		1597	let buf = render_app(&mut app);
		1598	assert_buffer_contains(&buf, &format!("[{}]", &expected[..8]));
		1599	}
		1600
		1601	/// Nothing else on the pane is a comment or a line of a file, so `c` there
		1602	/// is a thread comment and `x` there has nothing to act on.
		1603	#[test]
		1604	fn test_prose_rows_have_no_target() {
		1605	let mut app = make_app(0, 0);
		1606	app.mode = ViewMode::PatchDetail;
		1607	app.current_patch = Some(make_patch_with_revisions());
		1608	app.rebuild_patch_rows();
		1609
		1610	app.patch_cursor = 1; // the Title: line
		1611	assert_eq!(app.cursor_target(), RowTarget::None);
		1612	}
		1613
		1614	/// `o` names a patch, never a branch. `PatchState.branch` is provenance —
		1615	/// on an agent-produced patch it names a worktree that is long gone — and
		1616	/// checking it out was the failure this replaces.
		1617	#[test]
		1618	fn test_checkout_target_is_a_patch_id_not_a_branch() {
		1619	let mut app = test_app();
		1620	app.list_mode = ListMode::Patches;
		1621	app.patch_list_state.select(Some(0));
		1622
		1623	assert_eq!(app.checkout_target(), Some("p1".to_string()));
		1624	}
		1625
		1626	#[test]
		1627	fn test_checkout_target_follows_the_open_patch() {
		1628	let mut app = test_app();
		1629	app.mode = ViewMode::PatchDetail;
		1630	app.current_patch = Some(make_patch_with_revisions());
		1631	assert_eq!(app.checkout_target(), Some("deadbeef".to_string()));
		1632	}
		1633
		1634	// ── Staleness ────────────────────────────────────────────────────────
		1635
		1636	/// Unconditional, whether or not anything changed. This alone is what
		1637	/// turns a stale screen from silently wrong into merely old.
		1638	#[test]
		1639	fn test_render_says_when_it_was_loaded() {
		1640	let mut app = make_app(3, 0);
		1641	let buf = render_app(&mut app);
		1642	assert_buffer_contains(&buf, &format!("as of {}", app.loaded_at.format("%H:%M")));
		1643	}
		1644
		1645	#[test]
		1646	fn test_no_banner_when_nothing_moved() {
		1647	let mut app = make_app(3, 0);
		1648	app.new_events = 0;
		1649	let buf = render_app(&mut app);
		1650	let text = buffer_to_string(&buf);
		1651	assert!(!text.contains("new event"), "{}", text);
		1652	}
		1653
		1654	/// The count is of events rather than of objects: "4 new events" is honest
		1655	/// and costs a walk of what moved, where "2 issues changed" would cost the
		1656	/// fold the whole design avoids.
		1657	#[test]
		1658	fn test_banner_counts_events() {
		1659	let mut app = make_app(3, 0);
		1660	app.new_events = 4;
		1661	let buf = render_app(&mut app);
		1662	assert_buffer_contains(&buf, "4 new events");
		1663	assert_buffer_contains(&buf, "r to reload");
		1664
		1665	app.new_events = 1;
		1666	let buf = render_app(&mut app);
		1667	assert_buffer_contains(&buf, "1 new event ");
		1668	}
		1669
1480	#[test]	1670	#[test]
1481	fn test_linked_patch_for_selected() {	1671	fn test_linked_patch_for_selected() {
1482	let mut app = test_app();	1672	let mut app = test_app();

src/tui/state.rs

Old		New
@@ -1,3 +1,4 @@
		1	use chrono::{DateTime, Local};
1	use crossterm::event::{KeyCode, KeyModifiers};	2	use crossterm::event::{KeyCode, KeyModifiers};
2	use git2::{Oid, Repository};	3	use git2::{Oid, Repository};
3	use ratatui::widgets::ListState;	4	use ratatui::widgets::ListState;
@@ -5,6 +6,9 @@ use ratatui::widgets::ListState;
5	use crate::abbrev::Abbrev;	6	use crate::abbrev::Abbrev;
6	use crate::state::{self, IssueState, IssueStatus, PatchState, PatchStatus};	7	use crate::state::{self, IssueState, IssueStatus, PatchState, PatchStatus};
7		8
		9	use super::tips::Tips;
		10	use super::widgets::{DetailRow, RowTarget};
		11
8	#[derive(Debug, PartialEq)]	12	#[derive(Debug, PartialEq)]
9	pub(crate) enum Pane {	13	pub(crate) enum Pane {
10	ItemList,	14	ItemList,
@@ -27,6 +31,16 @@ pub(crate) enum KeyAction {
27	OpenCommitBrowser,	31	OpenCommitBrowser,
28	OpenPatchDetail,	32	OpenPatchDetail,
29	OpenPatchDetailDirect(usize), // index into visible_patches	33	OpenPatchDetailDirect(usize), // index into visible_patches
		34	/// Comment on the row under the cursor: inline when it is a line of the
		35	/// diff, on the patch's thread otherwise.
		36	Comment,
		37	/// Claim, or withdraw the claim, that the comment under the cursor was
		38	/// answered.
		39	ToggleResolve,
		40	/// Show the change that answered the comment under the cursor.
		41	ShowAnswers,
		42	/// Check out the patch's latest revision for local browsing.
		43	Checkout,
30	}	44	}
31		45
32	#[derive(Debug, PartialEq, Clone, Copy)]	46	#[derive(Debug, PartialEq, Clone, Copy)]
@@ -65,7 +79,9 @@ pub(crate) enum InputMode {
65	Normal,	79	Normal,
66	Search,	80	Search,
67	CreateTitle,	81	CreateTitle,
68	CreateBody,	82	/// `R` was pressed: waiting for the verdict, which is the one part of a
		83	/// review too small to be worth an editor round trip.
		84	ReviewVerdict,
69	}	85	}
70		86
71	pub(crate) struct App {	87	pub(crate) struct App {
@@ -99,10 +115,33 @@ pub(crate) struct App {
99	pub(crate) event_list_state: ListState,	115	pub(crate) event_list_state: ListState,
100	// Patch detail view state	116	// Patch detail view state
101	pub(crate) current_patch: Option<PatchState>,	117	pub(crate) current_patch: Option<PatchState>,
102	pub(crate) patch_diff: String,	118	/// The diff on screen, row by row, each carrying the `--at` anchor a
		119	/// comment on it would use. Rows rather than a string because the cursor
		120	/// has to be able to say which line of which file it is sitting on.
		121	pub(crate) patch_diff: Vec<crate::patch::DiffRow>,
		122	/// Every line the patch detail pane renders, with what a write key would
		123	/// act on. Built when the view is built rather than during a draw: the key
		124	/// handler needs it, and the key handler never renders.
		125	pub(crate) patch_rows: Vec<DetailRow>,
		126	/// Which of `patch_rows` the cursor is on.
		127	pub(crate) patch_cursor: usize,
		128	/// Rows the detail pane last had room for, so the scroll can follow the
		129	/// cursor. Only the renderer knows this; it is recorded on the way past.
		130	pub(crate) patch_viewport: u16,
		131	/// When set, the diff on screen is the change that answered this comment
		132	/// rather than the revision's own diff.
		133	pub(crate) patch_answers: Option<String>,
103	pub(crate) patch_scroll: u16,	134	pub(crate) patch_scroll: u16,
104	pub(crate) patch_revision_idx: usize,	135	pub(crate) patch_revision_idx: usize,
105	pub(crate) patch_interdiff_mode: bool,	136	pub(crate) patch_interdiff_mode: bool,
		137	/// When the lists on screen were folded, and where the collab refs stood
		138	/// at that moment. Together these are the difference between a screen that
		139	/// is merely old and one that is silently wrong.
		140	pub(crate) loaded_at: DateTime<Local>,
		141	pub(crate) tips: Tips,
		142	/// Events that landed since `tips` was taken. Never acted on by itself —
		143	/// it raises a banner, and the reader reloads when they are ready.
		144	pub(crate) new_events: usize,
106	}	145	}
107		146
108	impl App {	147	impl App {
@@ -140,13 +179,91 @@ impl App {
140	event_history: Vec::new(),	179	event_history: Vec::new(),
141	event_list_state: ListState::default(),	180	event_list_state: ListState::default(),
142	current_patch: None,	181	current_patch: None,
143	patch_diff: String::new(),	182	patch_diff: Vec::new(),
		183	patch_rows: Vec::new(),
		184	patch_cursor: 0,
		185	patch_viewport: 20,
		186	patch_answers: None,
144	patch_scroll: 0,	187	patch_scroll: 0,
145	patch_revision_idx: 0,	188	patch_revision_idx: 0,
146	patch_interdiff_mode: false,	189	patch_interdiff_mode: false,
		190	loaded_at: Local::now(),
		191	tips: Tips::default(),
		192	new_events: 0,
		193	}
		194	}
		195
		196	/// Take the diff on screen from plain text, with no anchors.
		197	///
		198	/// For the cases that are prose rather than a diff — an error from the
		199	/// diff machinery, or a patch with no revisions. Nothing in them is a line
		200	/// of a file, so nothing in them is anchorable.
		201	pub(crate) fn set_diff_text(&mut self, text: &str) {
		202	self.patch_diff = text
		203	.lines()
		204	.map(\|l\| crate::patch::DiffRow {
		205	text: format!("{}\n", l),
		206	body: false,
		207	anchor: None,
		208	})
		209	.collect();
		210	}
		211
		212	/// Rebuild the patch detail pane's rows from the state behind them.
		213	///
		214	/// Called whenever the patch, the revision, the diff mode or the comments
		215	/// change — never from a draw. The key handler acts on these rows, so they
		216	/// have to be the rows the reader was looking at when they pressed the
		217	/// key, not the rows a subsequent redraw would have produced.
		218	pub(crate) fn rebuild_patch_rows(&mut self) {
		219	self.patch_rows = super::widgets::build_patch_detail_rows(self);
		220	if self.patch_cursor >= self.patch_rows.len() {
		221	self.patch_cursor = self.patch_rows.len().saturating_sub(1);
		222	}
		223	self.follow_cursor();
		224	}
		225
		226	/// The row the cursor is on in the patch detail pane.
		227	pub(crate) fn cursor_target(&self) -> RowTarget {
		228	self.patch_rows
		229	.get(self.patch_cursor)
		230	.map(\|r\| r.target.clone())
		231	.unwrap_or_default()
		232	}
		233
		234	fn move_cursor(&mut self, delta: i32) {
		235	let len = self.patch_rows.len();
		236	if len == 0 {
		237	self.patch_cursor = 0;
		238	return;
		239	}
		240	self.patch_cursor = if delta > 0 {
		241	(self.patch_cursor + delta as usize).min(len - 1)
		242	} else {
		243	self.patch_cursor.saturating_sub((-delta) as usize)
		244	};
		245	self.follow_cursor();
		246	}
		247
		248	/// Keep the cursor on screen without moving the view any further than it
		249	/// has to: a reader walking a diff should not have the text jump.
		250	fn follow_cursor(&mut self) {
		251	let height = self.patch_viewport.max(1) as usize;
		252	let top = self.patch_scroll as usize;
		253	if self.patch_cursor < top {
		254	self.patch_scroll = self.patch_cursor as u16;
		255	} else if self.patch_cursor >= top + height {
		256	self.patch_scroll = (self.patch_cursor + 1 - height) as u16;
147	}	257	}
148	}	258	}
149		259
		260	/// Reset what a change of revision or diff mode invalidates.
		261	fn reset_patch_view(&mut self) {
		262	self.patch_scroll = 0;
		263	self.patch_cursor = 0;
		264	self.patch_answers = None;
		265	}
		266
150	pub(crate) fn matches_search(&self, title: &str) -> bool {	267	pub(crate) fn matches_search(&self, title: &str) -> bool {
151	if self.search_query.is_empty() {	268	if self.search_query.is_empty() {
152	return true;	269	return true;
@@ -227,9 +344,10 @@ impl App {
227	self.pane = Pane::ItemList;	344	self.pane = Pane::ItemList;
228	self.current_patch = None;	345	self.current_patch = None;
229	self.patch_diff.clear();	346	self.patch_diff.clear();
230	self.patch_scroll = 0;	347	self.patch_rows.clear();
231	self.patch_revision_idx = 0;	348	self.patch_revision_idx = 0;
232	self.patch_interdiff_mode = false;	349	self.patch_interdiff_mode = false;
		350	self.reset_patch_view();
233	return KeyAction::Continue;	351	return KeyAction::Continue;
234	}	352	}
235	KeyCode::Char('q') => return KeyAction::Quit,	353	KeyCode::Char('q') => return KeyAction::Quit,
@@ -237,27 +355,36 @@ impl App {
237	return KeyAction::Quit;	355	return KeyAction::Quit;
238	}	356	}
239	KeyCode::Char('j') \| KeyCode::Down => {	357	KeyCode::Char('j') \| KeyCode::Down => {
240	self.patch_scroll = self.patch_scroll.saturating_add(1);	358	self.move_cursor(1);
241	return KeyAction::Continue;	359	return KeyAction::Continue;
242	}	360	}
243	KeyCode::Char('k') \| KeyCode::Up => {	361	KeyCode::Char('k') \| KeyCode::Up => {
244	self.patch_scroll = self.patch_scroll.saturating_sub(1);	362	self.move_cursor(-1);
245	return KeyAction::Continue;	363	return KeyAction::Continue;
246	}	364	}
247	KeyCode::PageDown => {	365	KeyCode::PageDown => {
248	self.patch_scroll = self.patch_scroll.saturating_add(20);	366	self.move_cursor(20);
249	return KeyAction::Continue;	367	return KeyAction::Continue;
250	}	368	}
251	KeyCode::PageUp => {	369	KeyCode::PageUp => {
252	self.patch_scroll = self.patch_scroll.saturating_sub(20);	370	self.move_cursor(-20);
		371	return KeyAction::Continue;
		372	}
		373	// ── The review loop ──────────────────────────────────────
		374	KeyCode::Char('c') => return KeyAction::Comment,
		375	KeyCode::Char('R') => {
		376	self.input_mode = InputMode::ReviewVerdict;
253	return KeyAction::Continue;	377	return KeyAction::Continue;
254	}	378	}
		379	KeyCode::Char('x') => return KeyAction::ToggleResolve,
		380	KeyCode::Char('a') => return KeyAction::ShowAnswers,
		381	KeyCode::Char('o') => return KeyAction::Checkout,
255	KeyCode::Char(']') => {	382	KeyCode::Char(']') => {
256	if let Some(ref patch) = self.current_patch {	383	if let Some(ref patch) = self.current_patch {
257	let max = patch.revisions.len().saturating_sub(1);	384	let max = patch.revisions.len().saturating_sub(1);
258	if self.patch_revision_idx < max {	385	if self.patch_revision_idx < max {
259	self.patch_revision_idx += 1;	386	self.patch_revision_idx += 1;
260	self.patch_scroll = 0;	387	self.reset_patch_view();
261	return KeyAction::Reload; // signal to regenerate diff	388	return KeyAction::Reload; // signal to regenerate diff
262	}	389	}
263	}	390	}
@@ -266,14 +393,14 @@ impl App {
266	KeyCode::Char('[') => {	393	KeyCode::Char('[') => {
267	if self.patch_revision_idx > 0 {	394	if self.patch_revision_idx > 0 {
268	self.patch_revision_idx -= 1;	395	self.patch_revision_idx -= 1;
269	self.patch_scroll = 0;	396	self.reset_patch_view();
270	return KeyAction::Reload; // signal to regenerate diff	397	return KeyAction::Reload; // signal to regenerate diff
271	}	398	}
272	return KeyAction::Continue;	399	return KeyAction::Continue;
273	}	400	}
274	KeyCode::Char('d') => {	401	KeyCode::Char('d') => {
275	self.patch_interdiff_mode = !self.patch_interdiff_mode;	402	self.patch_interdiff_mode = !self.patch_interdiff_mode;
276	self.patch_scroll = 0;	403	self.reset_patch_view();
277	return KeyAction::Reload; // signal to regenerate diff	404	return KeyAction::Reload; // signal to regenerate diff
278	}	405	}
279	_ => return KeyAction::Continue,	406	_ => return KeyAction::Continue,
@@ -446,6 +573,27 @@ impl App {
446	}	573	}
447	}	574	}
448		575
		576	/// The patch `o` would check out, whichever view is on screen.
		577	///
		578	/// An id, never a branch name. `PatchState.branch` has been provenance
		579	/// only since revision refs landed — it records where the patch was
		580	/// authored, and on an agent-produced patch that worktree is long gone —
		581	/// so checking it out failed on exactly the patches this tool is for. The
		582	/// latest revision's commit is pinned by a revision ref and always
		583	/// resolves.
		584	pub(crate) fn checkout_target(&self) -> Option<String> {
		585	if self.mode == ViewMode::PatchDetail {
		586	return self.current_patch.as_ref().map(\|p\| p.id.clone());
		587	}
		588	match self.list_mode {
		589	ListMode::Issues => self.linked_patch_for_selected().map(\|p\| p.id.clone()),
		590	ListMode::Patches => {
		591	let idx = self.patch_list_state.selected()?;
		592	self.visible_patches().get(idx).map(\|p\| p.id.clone())
		593	}
		594	}
		595	}
		596
449	pub(crate) fn selected_item_id(&self) -> Option<String> {	597	pub(crate) fn selected_item_id(&self) -> Option<String> {
450	let idx = self.list_state.selected()?;	598	let idx = self.list_state.selected()?;
451	let visible = self.visible_issues();	599	let visible = self.visible_issues();
@@ -457,7 +605,32 @@ impl App {
457	Some(format!("refs/collab/issues/{}", id))	605	Some(format!("refs/collab/issues/{}", id))
458	}	606	}
459		607
		608	/// Take the tips as the new baseline, and note when.
		609	///
		610	/// Called from [`App::reload`] rather than only from the timer, and that
		611	/// is the whole of the "your own comment is not news" rule: every write
		612	/// the dashboard makes is followed by a reload, and a reload starts
		613	/// counting again from where the repository stands now.
		614	pub(crate) fn rebaseline(&mut self, repo: &Repository) {
		615	self.tips = Tips::read(repo);
		616	self.new_events = 0;
		617	self.loaded_at = Local::now();
		618	}
		619
		620	/// Compare the collab tips against the baseline, without folding anything.
		621	pub(crate) fn poll_tips(&mut self, repo: &Repository) {
		622	let now = Tips::read(repo);
		623	if now == self.tips {
		624	return;
		625	}
		626	// Count against the baseline, not against the last tick, so the banner
		627	// says how far behind the screen is rather than how much arrived in
		628	// the last two seconds.
		629	self.new_events = self.tips.events_since(repo, &now);
		630	}
		631
460	pub(crate) fn reload(&mut self, repo: &Repository) {	632	pub(crate) fn reload(&mut self, repo: &Repository) {
		633	self.rebaseline(repo);
461	if let Ok(issues) = state::list_issues(repo) {	634	if let Ok(issues) = state::list_issues(repo) {
462	self.issues = issues;	635	self.issues = issues;
463	}	636	}

src/tui/tips.rs

@@ @@ -0,0 +1,173 @@ @@
+//! Noticing that somebody else wrote, without reading what they wrote.
+//!
+//! The dashboard used to show whatever was true the last time `r` was pressed,
+//! with nothing on screen admitting it. The fix is not to reload on a timer:
+//! this is a review surface, and rows appearing, vanishing or reordering while
+//! someone is halfway through a comment is worse than being briefly out of
+//! date. So the timer only *compares tips* — a ref read — and when they move,
+//! a banner says so and the reader presses `r` when they are ready.
+//!
+//! Deliberately not a fold. `list_issues`/`list_patches` walk every DAG in the
+//! repository; doing that every couple of seconds to discover that nothing
+//! changed is the cost this exists to avoid.
+use git2::{Oid, Repository};
+/// Where every collab DAG stood at a moment in time.
+#[derive(Debug, Default, Clone, PartialEq)]
+pub(crate) struct Tips {
+    /// `(ref name, tip)`, sorted by name so two snapshots compare directly.
+    entries: Vec<(String, Oid)>,
+}
+impl Tips {
+    /// Read the current tips. Purely a ref read: nothing is folded, nothing is
+    /// written, and a repository that cannot be enumerated yields an empty
+    /// snapshot rather than an error the dashboard would have to render.
+    pub(crate) fn read(repo: &Repository) -> Self {
+        let mut entries = Vec::new();
+        if let Ok(refs) = repo.references_glob("refs/collab/**") {
+            for r in refs.flatten() {
+                let Some(name) = r.name() else { continue };
+                // `refs/collab/local/` is this clone's own bookkeeping — the
+                // `seen/` read markers `patch show` moves, and the migration
+                // parking spots. Counting those would make reading a patch in
+                // another terminal look like somebody else said something.
+                if name.starts_with("refs/collab/local/") {
+                    continue;
+                }
+                let Some(oid) = r.target() else { continue };
+                entries.push((name.to_string(), oid));
+            }
+        }
+        entries.sort();
+        Tips { entries }
+    }
+    /// How many events arrived since `self` was taken.
+    ///
+    /// Only walked for refs that actually moved, which is the whole point of
+    /// comparing tips first: on the common tick nothing moved and this is not
+    /// called at all. A ref that appeared since the snapshot contributes its
+    /// whole history, which for a new issue or patch is the one or two events
+    /// that created it.
+    pub(crate) fn events_since(&self, repo: &Repository, now: &Tips) -> usize {
+        let mut count = 0usize;
+        for (name, tip) in &now.entries {
+            match self.entries.binary_search_by(|(n, _)| n.as_str().cmp(name)) {
+                Ok(idx) => {
+                    let was = self.entries[idx].1;
+                    if was == *tip {
+                        continue;
+                    }
+                    count += match repo.graph_ahead_behind(*tip, was) {
+                        Ok((ahead, _behind)) => ahead.max(1),
+                        // Unrelated histories, or an object this clone lacks:
+                        // the ref moved, so something happened. Say one rather
+                        // than nothing.
+                        Err(_) => 1,
+                    };
+                }
+                Err(_) => count += walk_len(repo, *tip),
+            }
+        }
+        count
+    }
+}
+/// Number of commits reachable from `tip`, or 1 when it cannot be walked.
+fn walk_len(repo: &Repository, tip: Oid) -> usize {
+    let Ok(mut walk) = repo.revwalk() else {
+        return 1;
+    };
+    if walk.push(tip).is_err() {
+        return 1;
+    }
+    walk.count().max(1)
+}
+#[cfg(test)]
+mod tests {
+    use super::*;
+    /// A repo with one commit, and a helper for pointing collab refs at it.
+    fn scratch_repo() -> (tempfile::TempDir, Repository, Oid) {
+        let dir = tempfile::TempDir::new().unwrap();
+        let repo = Repository::init(dir.path()).unwrap();
+        let first = {
+            let sig = git2::Signature::now("t", "t@t").unwrap();
+            let mut index = repo.index().unwrap();
+            let tree_oid = index.write_tree().unwrap();
+            let tree = repo.find_tree(tree_oid).unwrap();
+            repo.commit(Some("HEAD"), &sig, &sig, "one", &tree, &[])
+                .unwrap()
+        };
+        (dir, repo, first)
+    }
+    #[test]
+    fn a_moved_ref_counts_the_events_that_moved_it() {
+        let (_dir, repo, first) = scratch_repo();
+        repo.reference("refs/collab/issues/abc", first, true, "seed")
+            .unwrap();
+        let before = Tips::read(&repo);
+        let sig = git2::Signature::now("t", "t@t").unwrap();
+        let tree = repo.find_commit(first).unwrap().tree().unwrap();
+        let parent = repo.find_commit(first).unwrap();
+        let second = repo
+            .commit(None, &sig, &sig, "two", &tree, &[&parent])
+            .unwrap();
+        repo.reference("refs/collab/issues/abc", second, true, "move")
+            .unwrap();
+        let after = Tips::read(&repo);
+        assert_ne!(before, after);
+        assert_eq!(before.events_since(&repo, &after), 1);
+    }
+    /// The `seen/` markers `patch show` moves are this clone's own
+    /// bookkeeping. Counting them would make reading a patch in another
+    /// terminal look like somebody else had said something.
+    #[test]
+    fn local_bookkeeping_refs_are_not_news() {
+        let (_dir, repo, first) = scratch_repo();
+        repo.reference("refs/collab/issues/abc", first, true, "seed")
+            .unwrap();
+        let before = Tips::read(&repo);
+        repo.reference(
+            "refs/collab/local/seen/patches/abc",
+            first,
+            true,
+            "mark seen",
+        )
+        .unwrap();
+        let after = Tips::read(&repo);
+        assert_eq!(before, after, "a local marker showed up as a tip");
+        assert_eq!(before.events_since(&repo, &after), 0);
+    }
+    /// A ref that did not exist at the baseline contributes its history — for
+    /// a new issue, the one event that opened it.
+    #[test]
+    fn a_new_ref_counts_as_news() {
+        let (_dir, repo, first) = scratch_repo();
+        let before = Tips::read(&repo);
+        repo.reference("refs/collab/issues/abc", first, true, "seed")
+            .unwrap();
+        let after = Tips::read(&repo);
+        assert_eq!(before.events_since(&repo, &after), 1);
+    }
+    #[test]
+    fn an_unchanged_repository_is_not_news() {
+        let (_dir, repo, first) = scratch_repo();
+        repo.reference("refs/collab/issues/abc", first, true, "seed")
+            .unwrap();
+        let before = Tips::read(&repo);
+        let after = Tips::read(&repo);
+        assert_eq!(before.events_since(&repo, &after), 0);
+    }
+}

src/tui/widgets.rs

Old		New
@@ -205,25 +205,112 @@ pub(crate) fn format_event_detail(
205	detail	205	detail
206	}	206	}
207		207
		208	/// What a write key acts on when the cursor is on a given row.
		209	#[derive(Debug, Clone, PartialEq, Default)]
		210	pub(crate) enum RowTarget {
		211	/// Prose, a heading, a revision — nothing a comment anchors to.
		212	#[default]
		213	None,
		214	/// A new-side line of the diff, carrying the anchor `patch comment --at`
		215	/// accepts for it.
		216	Diff { file: String, line: u32 },
		217	/// An inline comment, named by the id `patch show` prints in brackets.
		218	Comment { oid: String, resolved: bool },
		219	}
		220
		221	/// A rendered line of the patch detail pane, and what it stands for.
		222	///
		223	/// Built once when the view is built rather than during a draw: the key
		224	/// handler has to know what the cursor is on, and the key handler never draws.
		225	#[derive(Debug, Clone)]
		226	pub(crate) struct DetailRow {
		227	pub(crate) line: Line<'static>,
		228	pub(crate) target: RowTarget,
		229	}
		230
		231	impl DetailRow {
		232	fn plain(line: Line<'static>) -> Self {
		233	DetailRow {
		234	line,
		235	target: RowTarget::None,
		236	}
		237	}
		238	}
		239
		240	/// Accumulator for the detail pane.
		241	///
		242	/// Most of the pane is prose with nothing to act on, so `push` takes a bare
		243	/// line; the two places the cursor can do something — an inline comment and a
		244	/// new-side line of the diff — say so with `push_target`.
		245	struct Rows(Vec<DetailRow>);
		246
		247	impl Rows {
		248	fn new() -> Self {
		249	Rows(Vec::new())
		250	}
		251
		252	fn push(&mut self, line: Line<'static>) {
		253	self.0.push(DetailRow::plain(line));
		254	}
		255
		256	fn push_target(&mut self, line: Line<'static>, target: RowTarget) {
		257	self.0.push(DetailRow { line, target });
		258	}
		259	}
		260
208	pub(crate) fn ui(frame: &mut Frame, app: &mut App, repo: Option<&Repository>) {	261	pub(crate) fn ui(frame: &mut Frame, app: &mut App, repo: Option<&Repository>) {
209	let chunks = Layout::default()	262	let chunks = Layout::default()
210	.direction(Direction::Vertical)	263	.direction(Direction::Vertical)
211	.constraints([Constraint::Min(1), Constraint::Length(1)])	264	.constraints([
		265	Constraint::Length(1),
		266	Constraint::Min(1),
		267	Constraint::Length(1),
		268	])
212	.split(frame.area());	269	.split(frame.area());
213		270
214	let main_area = chunks[0];	271	let header_area = chunks[0];
215	let footer_area = chunks[1];	272	let main_area = chunks[1];
		273	let footer_area = chunks[2];
216		274
217	let panes = Layout::default()	275	let panes = Layout::default()
218	.direction(Direction::Horizontal)	276	.direction(Direction::Horizontal)
219	.constraints([Constraint::Percentage(35), Constraint::Percentage(65)])	277	.constraints([Constraint::Percentage(35), Constraint::Percentage(65)])
220	.split(main_area);	278	.split(main_area);
221		279
		280	render_header(frame, app, header_area);
222	render_list(frame, app, panes[0]);	281	render_list(frame, app, panes[0]);
223	render_detail(frame, app, panes[1], repo);	282	render_detail(frame, app, panes[1], repo);
224	render_footer(frame, app, footer_area);	283	render_footer(frame, app, footer_area);
225	}	284	}
226		285
		286	/// When the screen was loaded, and whether anything has happened since.
		287	///
		288	/// The timestamp is unconditional. That is the point: a dashboard left open
		289	/// beside working agents used to show last hour's repository and look
		290	/// perfectly healthy, and a screen that admits it is old is merely old.
		291	fn render_header(frame: &mut Frame, app: &App, area: Rect) {
		292	let mut spans = vec![
		293	Span::styled(" DASHBOARD", Style::default().add_modifier(Modifier::BOLD)),
		294	Span::styled(
		295	format!(" as of {}", app.loaded_at.format("%H:%M")),
		296	Style::default().fg(Color::DarkGray),
		297	),
		298	];
		299	if app.new_events > 0 {
		300	spans.push(Span::styled(
		301	format!(
		302	" ● {} new event{} · r to reload",
		303	app.new_events,
		304	if app.new_events == 1 { "" } else { "s" }
		305	),
		306	Style::default()
		307	.fg(Color::Yellow)
		308	.add_modifier(Modifier::BOLD),
		309	));
		310	}
		311	frame.render_widget(Paragraph::new(Line::from(spans)), area);
		312	}
		313
227	fn render_list(frame: &mut Frame, app: &mut App, area: Rect) {	314	fn render_list(frame: &mut Frame, app: &mut App, area: Rect) {
228	let border_style = if app.pane == Pane::ItemList {	315	let border_style = if app.pane == Pane::ItemList {
229	Style::default().fg(Color::Yellow)	316	Style::default().fg(Color::Yellow)
@@ -321,14 +408,40 @@ fn render_detail(frame: &mut Frame, app: &mut App, area: Rect, repo: Option<&Rep
321		408
322	// Handle patch detail mode	409	// Handle patch detail mode
323	if app.mode == ViewMode::PatchDetail {	410	if app.mode == ViewMode::PatchDetail {
324	let content = build_patch_detail_text(app);	411	// The rows are built by the key handler, not here, so that `c` and `x`
		412	// act on the same row the reader is looking at. A draw that rebuilt
		413	// them could disagree with the last keypress.
		414	//
		415	// Recording the height is the one thing only the renderer knows, and
		416	// the scroll needs it to follow the cursor. It moves no ref and reads
		417	// no object: a draw stays a draw.
		418	app.patch_viewport = area.height.saturating_sub(2);
		419	let cursor = app.patch_cursor;
		420	let lines: Vec<Line> = app
		421	.patch_rows
		422	.iter()
		423	.enumerate()
		424	.map(\|(i, row)\| {
		425	if i == cursor {
		426	row.line.clone().style(
		427	Style::default()
		428	.bg(Color::DarkGray)
		429	.add_modifier(Modifier::BOLD),
		430	)
		431	} else {
		432	row.line.clone()
		433	}
		434	})
		435	.collect();
325	let block = Block::default()	436	let block = Block::default()
326	.borders(Borders::ALL)	437	.borders(Borders::ALL)
327	.title("Patch Detail")	438	.title("Patch Detail")
328	.border_style(border_style);	439	.border_style(border_style);
329	let para = Paragraph::new(content)	440	// No wrapping: a wrapped line occupies more rows than it has, which
		441	// would put the cursor's highlight on a different line than the one
		442	// `c` is about to comment on.
		443	let para = Paragraph::new(Text::from(lines))
330	.block(block)	444	.block(block)
331	.wrap(Wrap { trim: false })
332	.scroll((app.patch_scroll, 0));	445	.scroll((app.patch_scroll, 0));
333	frame.render_widget(para, area);	446	frame.render_widget(para, area);
334	return;	447	return;
@@ -681,10 +794,12 @@ fn build_patch_summary(
681	Text::from(lines)	794	Text::from(lines)
682	}	795	}
683		796
684	fn build_patch_detail_text(app: &App) -> Text<'static> {	797	/// Build every row of the patch detail pane, with what a write key would act
		798	/// on for each.
		799	pub(crate) fn build_patch_detail_rows(app: &App) -> Vec<DetailRow> {
685	let patch = match &app.current_patch {	800	let patch = match &app.current_patch {
686	Some(p) => p,	801	Some(p) => p,
687	None => return Text::raw("No patch loaded."),	802	None => return vec![DetailRow::plain(Line::raw("No patch loaded."))],
688	};	803	};
689		804
690	let status_str = patch.status.as_str();	805	let status_str = patch.status.as_str();
@@ -694,49 +809,48 @@ fn build_patch_detail_text(app: &App) -> Text<'static> {
694	PatchStatus::Merged => Color::Cyan,	809	PatchStatus::Merged => Color::Cyan,
695	};	810	};
696		811
697	let mut lines: Vec<Line> = vec![	812	let mut rows = Rows::new();
698	Line::from(vec![	813	rows.push(Line::from(vec![
699	Span::styled("Patch ", Style::default().add_modifier(Modifier::BOLD)),	814	Span::styled("Patch ", Style::default().add_modifier(Modifier::BOLD)),
700	Span::styled(	815	Span::styled(
701	app.patch_abbrev.of(&patch.id).to_string(),	816	app.patch_abbrev.of(&patch.id).to_string(),
702	Style::default()	817	Style::default()
703	.fg(Color::Yellow)	818	.fg(Color::Yellow)
704	.add_modifier(Modifier::BOLD),	819	.add_modifier(Modifier::BOLD),
705	),	820	),
706	Span::raw(" "),	821	Span::raw(" "),
707	Span::styled(status_str, Style::default().fg(status_color)),	822	Span::styled(status_str, Style::default().fg(status_color)),
708	]),	823	]));
709	Line::from(vec![	824	rows.push(Line::from(vec![
710	Span::styled("Title: ", Style::default().fg(Color::DarkGray)),	825	Span::styled("Title: ", Style::default().fg(Color::DarkGray)),
711	Span::raw(patch.title.clone()),	826	Span::raw(patch.title.clone()),
712	]),	827	]));
713	Line::from(vec![	828	rows.push(Line::from(vec![
714	Span::styled("Author: ", Style::default().fg(Color::DarkGray)),	829	Span::styled("Author: ", Style::default().fg(Color::DarkGray)),
715	Span::raw(format!("{} <{}>", patch.author.name, patch.author.email)),	830	Span::raw(format!("{} <{}>", patch.author.name, patch.author.email)),
716	]),	831	]));
717	Line::from(vec![	832	rows.push(Line::from(vec![
718	Span::styled("Branch: ", Style::default().fg(Color::DarkGray)),	833	Span::styled("Branch: ", Style::default().fg(Color::DarkGray)),
719	Span::raw(patch.branch.clone()),	834	Span::raw(patch.branch.clone()),
720	]),	835	]));
721	Line::from(vec![	836	rows.push(Line::from(vec![
722	Span::styled("Base: ", Style::default().fg(Color::DarkGray)),	837	Span::styled("Base: ", Style::default().fg(Color::DarkGray)),
723	Span::raw(patch.base_ref.clone()),	838	Span::raw(patch.base_ref.clone()),
724	]),	839	]));
725	Line::from(vec![	840	rows.push(Line::from(vec![
726	Span::styled("Revisions:", Style::default().fg(Color::DarkGray)),	841	Span::styled("Revisions:", Style::default().fg(Color::DarkGray)),
727	Span::raw(format!(" {}", patch.revisions.len())),	842	Span::raw(format!(" {}", patch.revisions.len())),
728	]),	843	]));
729	];
730		844
731	if !patch.labels.is_empty() {	845	if !patch.labels.is_empty() {
732	lines.push(Line::from(vec![	846	rows.push(Line::from(vec![
733	Span::styled("Labels: ", Style::default().fg(Color::DarkGray)),	847	Span::styled("Labels: ", Style::default().fg(Color::DarkGray)),
734	Span::raw(patch.labels.join(", ")),	848	Span::raw(patch.labels.join(", ")),
735	]));	849	]));
736	}	850	}
737		851
738	if let Some(ref fixes) = patch.fixes {	852	if let Some(ref fixes) = patch.fixes {
739	lines.push(Line::from(vec![	853	rows.push(Line::from(vec![
740	Span::styled("Fixes: ", Style::default().fg(Color::DarkGray)),	854	Span::styled("Fixes: ", Style::default().fg(Color::DarkGray)),
741	Span::raw(app.issue_abbrev.of(fixes).to_string()),	855	Span::raw(app.issue_abbrev.of(fixes).to_string()),
742	]));	856	]));
@@ -745,8 +859,8 @@ fn build_patch_detail_text(app: &App) -> Text<'static> {
745	// Staleness warning (stored in status_msg or computed text)	859	// Staleness warning (stored in status_msg or computed text)
746	if let Some(ref warning) = app.status_msg {	860	if let Some(ref warning) = app.status_msg {
747	if warning.contains("behind") {	861	if warning.contains("behind") {
748	lines.push(Line::raw(""));	862	rows.push(Line::raw(""));
749	lines.push(Line::styled(	863	rows.push(Line::styled(
750	warning.clone(),	864	warning.clone(),
751	Style::default()	865	Style::default()
752	.fg(Color::Yellow)	866	.fg(Color::Yellow)
@@ -757,8 +871,8 @@ fn build_patch_detail_text(app: &App) -> Text<'static> {
757		871
758	// Revision list	872	// Revision list
759	if !patch.revisions.is_empty() {	873	if !patch.revisions.is_empty() {
760	lines.push(Line::raw(""));	874	rows.push(Line::raw(""));
761	lines.push(Line::styled(	875	rows.push(Line::styled(
762	"--- Revisions ---",	876	"--- Revisions ---",
763	Style::default()	877	Style::default()
764	.fg(Color::Magenta)	878	.fg(Color::Magenta)
@@ -774,7 +888,7 @@ fn build_patch_detail_text(app: &App) -> Text<'static> {
774	" "	888	" "
775	};	889	};
776	let label = if i == 0 { " (initial)" } else { "" };	890	let label = if i == 0 { " (initial)" } else { "" };
777	lines.push(Line::from(vec![Span::raw(format!(	891	rows.push(Line::from(vec![Span::raw(format!(
778	"{}r{} {} {}{}",	892	"{}r{} {} {}{}",
779	marker, rev.number, short, rev.timestamp, label	893	marker, rev.number, short, rev.timestamp, label
780	))]));	894	))]));
@@ -783,8 +897,8 @@ fn build_patch_detail_text(app: &App) -> Text<'static> {
783		897
784	// Reviews	898	// Reviews
785	if !patch.reviews.is_empty() {	899	if !patch.reviews.is_empty() {
786	lines.push(Line::raw(""));	900	rows.push(Line::raw(""));
787	lines.push(Line::styled(	901	rows.push(Line::styled(
788	"--- Reviews ---",	902	"--- Reviews ---",
789	Style::default()	903	Style::default()
790	.fg(Color::Blue)	904	.fg(Color::Blue)
@@ -802,7 +916,7 @@ fn build_patch_detail_text(app: &App) -> Text<'static> {
802	.revision	916	.revision
803	.map(\|r\| format!(" (r{})", r))	917	.map(\|r\| format!(" (r{})", r))
804	.unwrap_or_default();	918	.unwrap_or_default();
805	lines.push(Line::from(vec![	919	rows.push(Line::from(vec![
806	Span::styled(	920	Span::styled(
807	review.author.name.clone(),	921	review.author.name.clone(),
808	Style::default().add_modifier(Modifier::BOLD),	922	Style::default().add_modifier(Modifier::BOLD),
@@ -814,7 +928,7 @@ fn build_patch_detail_text(app: &App) -> Text<'static> {
814	]));	928	]));
815	if !review.body.is_empty() {	929	if !review.body.is_empty() {
816	for l in review.body.lines() {	930	for l in review.body.lines() {
817	lines.push(Line::raw(format!(" {}", l)));	931	rows.push(Line::raw(format!(" {}", l)));
818	}	932	}
819	}	933	}
820	}	934	}
@@ -822,8 +936,8 @@ fn build_patch_detail_text(app: &App) -> Text<'static> {
822		936
823	// Inline comments	937	// Inline comments
824	if !patch.inline_comments.is_empty() {	938	if !patch.inline_comments.is_empty() {
825	lines.push(Line::raw(""));	939	rows.push(Line::raw(""));
826	lines.push(Line::styled(	940	rows.push(Line::styled(
827	"--- Inline Comments ---",	941	"--- Inline Comments ---",
828	Style::default()	942	Style::default()
829	.fg(Color::Blue)	943	.fg(Color::Blue)
@@ -834,34 +948,53 @@ fn build_patch_detail_text(app: &App) -> Text<'static> {
834	.revision	948	.revision
835	.map(\|r\| format!(" (r{})", r))	949	.map(\|r\| format!(" (r{})", r))
836	.unwrap_or_default();	950	.unwrap_or_default();
837	lines.push(Line::from(vec![	951	// The id is the one `patch show` prints in brackets and
838	Span::styled(	952	// `resolve_comment` takes as a prefix, at the same eight
839	ic.author.name.clone(),	953	// characters. The two surfaces have to agree, because a reviewer
840	Style::default().add_modifier(Modifier::BOLD),	954	// reads it off one and types it into the other.
841	),	955	let resolved = ic.resolved.is_some();
842	Span::raw(format!(" {}:{}", ic.file, ic.line)),	956	rows.push_target(
843	Span::raw(rev_label),	957	Line::from(vec![
844	edited_span(ic.edited),	958	Span::styled(
845	non_blocking_span(ic.non_blocking),	959	ic.author.name.clone(),
846	]));	960	Style::default().add_modifier(Modifier::BOLD),
		961	),
		962	Span::raw(format!(" {}:{}", ic.file, ic.line)),
		963	Span::raw(rev_label),
		964	Span::styled(
		965	format!(" [{:.8}]", ic.commit_id),
		966	Style::default().fg(Color::DarkGray),
		967	),
		968	Span::styled(
		969	if resolved { " resolved" } else { "" },
		970	Style::default().fg(Color::Green),
		971	),
		972	edited_span(ic.edited),
		973	non_blocking_span(ic.non_blocking),
		974	]),
		975	RowTarget::Comment {
		976	oid: ic.commit_id.to_string(),
		977	resolved,
		978	},
		979	);
847	for l in body_lines(&ic.body, ic.deleted) {	980	for l in body_lines(&ic.body, ic.deleted) {
848	lines.push(Line::raw(format!(" {}", l)));	981	rows.push(Line::raw(format!(" {}", l)));
849	}	982	}
850	}	983	}
851	}	984	}
852		985
853	// Thread comments	986	// Thread comments
854	if !patch.comments.is_empty() {	987	if !patch.comments.is_empty() {
855	lines.push(Line::raw(""));	988	rows.push(Line::raw(""));
856	lines.push(Line::styled(	989	rows.push(Line::styled(
857	"--- Comments ---",	990	"--- Comments ---",
858	Style::default()	991	Style::default()
859	.fg(Color::Blue)	992	.fg(Color::Blue)
860	.add_modifier(Modifier::BOLD),	993	.add_modifier(Modifier::BOLD),
861	));	994	));
862	for c in &patch.comments {	995	for c in &patch.comments {
863	lines.push(Line::raw(""));	996	rows.push(Line::raw(""));
864	lines.push(Line::from(vec![	997	rows.push(Line::from(vec![
865	Span::styled(	998	Span::styled(
866	c.author.name.clone(),	999	c.author.name.clone(),
867	Style::default().add_modifier(Modifier::BOLD),	1000	Style::default().add_modifier(Modifier::BOLD),
@@ -873,13 +1006,23 @@ fn build_patch_detail_text(app: &App) -> Text<'static> {
873	edited_span(c.edited),	1006	edited_span(c.edited),
874	]));	1007	]));
875	for l in body_lines(&c.body, c.deleted) {	1008	for l in body_lines(&c.body, c.deleted) {
876	lines.push(Line::raw(format!(" {}", l)));	1009	rows.push(Line::raw(format!(" {}", l)));
877	}	1010	}
878	}	1011	}
879	}	1012	}
880		1013
881	// Diff header	1014	// Diff header
882	lines.push(Line::raw(""));	1015	rows.push(Line::raw(""));
		1016	if let Some(ref comment) = app.patch_answers {
		1017	rows.push(Line::styled(
		1018	format!("--- What answered [{:.8}] ---", comment),
		1019	Style::default()
		1020	.fg(Color::Green)
		1021	.add_modifier(Modifier::BOLD),
		1022	));
		1023	push_diff_rows(&mut rows, app);
		1024	return rows.0;
		1025	}
883	let diff_mode = if app.patch_interdiff_mode {	1026	let diff_mode = if app.patch_interdiff_mode {
884	let rev_idx = app.patch_revision_idx;	1027	let rev_idx = app.patch_revision_idx;
885	if rev_idx > 0 {	1028	if rev_idx > 0 {
@@ -901,36 +1044,51 @@ fn build_patch_detail_text(app: &App) -> Text<'static> {
901	.unwrap_or(1);	1044	.unwrap_or(1);
902	format!("--- Diff r{} vs base ---", rev_num)	1045	format!("--- Diff r{} vs base ---", rev_num)
903	};	1046	};
904	lines.push(Line::styled(	1047	rows.push(Line::styled(
905	diff_mode,	1048	diff_mode,
906	Style::default()	1049	Style::default()
907	.fg(Color::Green)	1050	.fg(Color::Green)
908	.add_modifier(Modifier::BOLD),	1051	.add_modifier(Modifier::BOLD),
909	));	1052	));
910		1053
911	// Diff content	1054	push_diff_rows(&mut rows, app);
		1055	rows.0
		1056	}
		1057
		1058	/// Render the diff, carrying each new-side row's anchor onto the row the
		1059	/// cursor will land on.
		1060	fn push_diff_rows(rows: &mut Rows, app: &App) {
912	if app.patch_diff.is_empty() {	1061	if app.patch_diff.is_empty() {
913	lines.push(Line::raw("(no diff available)"));	1062	rows.push(Line::raw("(no diff available)"));
914	} else {	1063	return;
915	for l in app.patch_diff.lines() {	1064	}
916	let style = if l.starts_with('+') {	1065	for row in &app.patch_diff {
917	Style::default().fg(Color::Green)	1066	let text = row.text.trim_end_matches(['\n', '\r']).to_string();
918	} else if l.starts_with('-') {	1067	let style = if text.starts_with("diff ") {
919	Style::default().fg(Color::Red)	1068	Style::default()
920	} else if l.starts_with("@@") {	1069	.fg(Color::Yellow)
921	Style::default().fg(Color::Cyan)	1070	.add_modifier(Modifier::BOLD)
922	} else if l.starts_with("diff ") {	1071	} else if text.starts_with("@@") {
923	Style::default()	1072	Style::default().fg(Color::Cyan)
924	.fg(Color::Yellow)	1073	} else if text.starts_with('+') {
925	.add_modifier(Modifier::BOLD)	1074	Style::default().fg(Color::Green)
926	} else {	1075	} else if text.starts_with('-') {
927	Style::default()	1076	Style::default().fg(Color::Red)
928	};	1077	} else {
929	lines.push(Line::styled(l.to_string(), style));	1078	Style::default()
		1079	};
		1080	let line = Line::styled(text, style);
		1081	match &row.anchor {
		1082	Some((file, number)) => rows.push_target(
		1083	line,
		1084	RowTarget::Diff {
		1085	file: file.clone(),
		1086	line: *number,
		1087	},
		1088	),
		1089	None => rows.push(line),
930	}	1090	}
931	}	1091	}
932
933	Text::from(lines)
934	}	1092	}
935		1093
936	fn render_footer(frame: &mut Frame, app: &App, area: Rect) {	1094	fn render_footer(frame: &mut Frame, app: &App, area: Rect) {
@@ -955,22 +1113,17 @@ fn render_footer(frame: &mut Frame, app: &App, area: Rect) {
955	} else {	1113	} else {
956	&app.input_buf	1114	&app.input_buf
957	};	1115	};
958	let text = format!(" New issue - Title: {}_", display);	1116	let text = format!(" New issue - Title: {}_ (Enter: write the body)", display);
959	let style = Style::default().bg(Color::Green).fg(Color::Black);	1117	let style = Style::default().bg(Color::Green).fg(Color::Black);
960	let para = Paragraph::new(text).style(style);	1118	let para = Paragraph::new(text).style(style);
961	frame.render_widget(para, area);	1119	frame.render_widget(para, area);
962	return;	1120	return;
963	}	1121	}
964	InputMode::CreateBody => {	1122	InputMode::ReviewVerdict => {
965	let max_len = (area.width as usize).saturating_sub(21);	1123	let para = Paragraph::new(
966	let display = if app.input_buf.len() > max_len {	1124	" Review verdict: a:approve r:request-changes c:comment Esc:cancel",
967	&app.input_buf[app.input_buf.len() - max_len..]	1125	)
968	} else {	1126	.style(Style::default().bg(Color::Blue).fg(Color::White));
969	&app.input_buf
970	};
971	let text = format!(" New issue - Body: {}_ (Esc: skip)", display);
972	let style = Style::default().bg(Color::Green).fg(Color::Black);
973	let para = Paragraph::new(text).style(style);
974	frame.render_widget(para, area);	1127	frame.render_widget(para, area);
975	return;	1128	return;
976	}	1129	}
@@ -988,8 +1141,13 @@ fn render_footer(frame: &mut Frame, app: &App, area: Rect) {
988	let text = match app.mode {	1141	let text = match app.mode {
989	ViewMode::CommitList => " j/k:navigate Enter:detail Esc:back q:quit".to_string(),	1142	ViewMode::CommitList => " j/k:navigate Enter:detail Esc:back q:quit".to_string(),
990	ViewMode::CommitDetail => " j/k:scroll Esc:back q:quit".to_string(),	1143	ViewMode::CommitDetail => " j/k:scroll Esc:back q:quit".to_string(),
		1144	// Ordered by what a reviewer reaches for, because this is one line and
		1145	// a narrow terminal clips the tail: the review loop first, then the
		1146	// way out, then the things you can also get to by other means.
991	ViewMode::PatchDetail => {	1147	ViewMode::PatchDetail => {
992	" j/k:scroll Esc:back [/]:revision d:interdiff q:quit".to_string()	1148	" j/k:line c:comment R:review x:resolve a:answers Esc:back q:quit \
		1149	[/]:revision d:interdiff o:checkout"
		1150	.to_string()
993	}	1151	}
994	ViewMode::Details => {	1152	ViewMode::Details => {
995	let list_hint = match app.list_mode {	1153	let list_hint = match app.list_mode {

tests/common/mod.rs

@@ @@ -19,6 +19,52 @@ use git_collab::dag; @@
+use git_collab::event::{Action, Author, Event, ReviewVerdict};
+use git_collab::signing;
+/// Strip terminal control sequences from captured pty output.
+///
+/// A TUI redraw is escape sequences interleaved with the cells that changed,
+/// so `contains("as of")` on the raw bytes is a coin toss: the words are there
+/// but a cursor move may sit between them. Removing the control sequences
+/// leaves the characters that were actually put on the screen, in order.
+pub fn screen_text(bytes: &[u8]) -> String {
+    let text = String::from_utf8_lossy(bytes);
+    let mut out = String::with_capacity(text.len());
+    let mut chars = text.chars().peekable();
+    while let Some(c) = chars.next() {
+        if c != '\u{1b}' {
+            out.push(c);
+            continue;
+        }
+        match chars.next() {
+            // CSI: parameters and intermediates, then a final byte in @..~.
+            Some('[') => {
+                for c in chars.by_ref() {
+                    if ('\u{40}'..='\u{7e}').contains(&c) {
+                        break;
+                    }
+                }
+            }
+            // OSC: runs to BEL or ST.
+            Some(']') => {
+                while let Some(c) = chars.next() {
+                    if c == '\u{7}' {
+                        break;
+                    }
+                    if c == '\u{1b}' && chars.peek() == Some(&'\\') {
+                        chars.next();
+                        break;
+                    }
+                }
+            }
+            // Two-character sequences (ESC =, ESC >, ESC (B, ...).
+            Some('(') | Some(')') => {
+                chars.next();
+            }
+            _ => {}
+        }
+    }
+    out
+}
+// ===========================================================================
+// Library-level helpers (for collab_test / sync_test)
+// ===========================================================================
@@ @@ -623,6 +669,73 @@ impl TestRepo { @@
+    /// Run `git-collab dashboard` in a pseudo-terminal and drive its stdin from
+    /// a callback, so a test can interleave keystrokes with pauses and with
+    /// writes made by another process.
+    ///
+    /// The smoke helper above writes every key at once and waits for the exit,
+    /// which cannot express "open the dashboard, let something else move a ref,
+    /// then look at the screen". `extra_env` is applied last, so a test can set
+    /// `EDITOR` to a script and exercise the suspend path with no human.
+    pub fn run_dashboard_driven<F>(
+        &self,
+        cols: u16,
+        rows: u16,
+        extra_env: &[(&str, &str)],
+        drive: F,
+    ) -> Output
+    where
+        F: FnOnce(&mut std::process::ChildStdin),
+    {
+        let mut command = Command::new("script");
+        self.apply_env(&mut command);
+        for (k, v) in extra_env {
+            command.env(k, v);
+        }
+        let mut child = command
+            .args([
+                "-qec",
+                &format!(
+                    "stty cols {} rows {}; {} dashboard",
+                    cols,
+                    rows,
+                    env!("CARGO_BIN_EXE_git-collab")
+                ),
+                "/dev/null",
+            ])
+            .current_dir(self.dir.path())
+            .stdin(Stdio::piped())
+            .stdout(Stdio::piped())
+            .stderr(Stdio::piped())
+            .spawn()
+            .expect("failed to launch dashboard in pty");
+        {
+            let mut stdin = child.stdin.take().expect("dashboard stdin piped");
+            // Let the dashboard reach its first draw before anything is typed:
+            // keys delivered before the event loop starts are read by the
+            // terminal, not by the program under test.
+            thread::sleep(Duration::from_millis(600));
+            drive(&mut stdin);
+        }
+        let deadline = Instant::now() + Duration::from_secs(30);
+        loop {
+            if child.try_wait().expect("failed to poll dashboard process").is_some() {
+                return child
+                    .wait_with_output()
+                    .expect("failed to collect dashboard output");
+            }
+            if Instant::now() >= deadline {
+                let _ = child.kill();
+                return child
+                    .wait_with_output()
+                    .expect("failed to collect timed out dashboard output");
+            }
+            thread::sleep(Duration::from_millis(20));
+        }
+    }
+    /// Open an issue and return the 8-char short ID.
+    pub fn issue_open(&self, title: &str) -> String {
+        let out = self.run_ok(&["issue", "open", "-t", title]);

tests/tui_review_test.rs

@@ @@ -0,0 +1,460 @@ @@
+//! The dashboard as a review surface, driven through a real pty.
+//!
+//! Two properties are only true end to end and so are only testable here: that
+//! suspending to `$EDITOR` leaves the alternate screen, runs a process, comes
+//! back and records what the process wrote; and that a ref another process
+//! moved is noticed while the dashboard is sitting there. Both are checked by
+//! looking at the collab DAG afterwards rather than at the screen, because the
+//! screen can lie and the DAG cannot.
+mod common;
+use common::{screen_text, TestRepo};
+use std::io::Write;
+use std::thread;
+use std::time::Duration;
+/// A fake `$EDITOR` that replaces the buffer with `body` and exits cleanly.
+fn editor_writing(repo: &TestRepo, name: &str, body: &str) -> String {
+    repo.write_script(
+        name,
+        &format!("#!/bin/sh\ncat > \"$1\" <<'GITCOLLABEOF'\n{}\nGITCOLLABEOF\n", body),
+    )
+}
+/// Seed a repo with one patch that has a real revision to comment on.
+fn repo_with_patch() -> (TestRepo, String) {
+    let repo = TestRepo::new("Reviewer", "reviewer@example.com");
+    repo.commit_file("src/lib.rs", "one\ntwo\nthree\nfour\n", "seed");
+    repo.git(&["checkout", "-b", "feature/x"]);
+    repo.commit_file("src/lib.rs", "one\ntwo changed\nthree\nfour\nfive\n", "work");
+    let out = repo.run_ok(&["patch", "create", "-t", "A patch to review", "-B", "feature/x"]);
+    let id = out
+        .trim()
+        .strip_prefix("Created patch ")
+        .expect("patch create output")
+        .to_string();
+    repo.git(&["checkout", "main"]);
+    (repo, id)
+}
+// ── Staleness (59ab40e8) ────────────────────────────────────────────────────
+/// The property that turns the old behaviour from a limitation into a bug:
+/// a screen that is out of date has to say when it was loaded.
+#[test]
+fn dashboard_always_shows_when_it_was_loaded() {
+    let repo = TestRepo::new("Reader", "reader@example.com");
+    repo.issue_open("something to look at");
+    let out = repo.run_dashboard_smoke_sized("q", 100, 24);
+    let text = screen_text(&out.stdout);
+    assert!(
+        text.contains("as of "),
+        "no 'as of' marker on the dashboard:\n{}",
+        text
+    );
+}
+/// A ref another process moves while the dashboard sits there must be
+/// announced. The dashboard must not reload under the reader — the banner is
+/// the whole feature — so the assertion is on the banner, not on the list.
+#[test]
+fn an_external_write_raises_the_banner() {
+    let repo = TestRepo::new("Reader", "reader@example.com");
+    repo.issue_open("the one already on screen");
+    let out = repo.run_dashboard_driven(100, 24, &[], |stdin| {
+        // The dashboard is up and has folded what exists. Now be the agent
+        // that lands something while the reviewer is reading. `repo.run_ok`
+        // targets the temp repo and its isolated home — running the binary by
+        // hand here would write to whatever directory the test happens to be
+        // started from.
+        thread::sleep(Duration::from_millis(300));
+        repo.issue_open("landed while you were reading");
+        thread::sleep(Duration::from_millis(4000));
+        let _ = stdin.write_all(b"q");
+    });
+    let text = screen_text(&out.stdout);
+    assert!(
+        text.contains("new event"),
+        "no staleness banner after an external write:\n{}",
+        text
+    );
+}
+/// `n` writes its body through `$EDITOR` too — there is no second, worse text
+/// input left in the program.
+#[test]
+fn n_composes_the_issue_body_in_the_editor() {
+    let repo = TestRepo::new("Reader", "reader@example.com");
+    let editor = editor_writing(&repo, "issue-body.sh", "the long markdown body");
+    repo.run_dashboard_driven(100, 24, &[("EDITOR", &editor)], |stdin| {
+        let _ = stdin.write_all(b"nA new issue\r");
+        thread::sleep(Duration::from_millis(2000));
+        let _ = stdin.write_all(b"q");
+    });
+    let listed = repo.run_ok(&["issue", "list"]);
+    let id = listed
+        .split_whitespace()
+        .next()
+        .expect("one issue was opened");
+    let shown = repo.run_ok(&["issue", "show", id]);
+    assert!(
+        shown.contains("A new issue") && shown.contains("the long markdown body"),
+        "the editor's body did not reach the issue:\n{}",
+        shown
+    );
+}
+/// The reader's own comment must not be announced back to them as news. The
+/// write reloads, and that reload has to re-baseline the tip snapshot.
+#[test]
+fn the_dashboards_own_write_does_not_raise_the_banner() {
+    let repo = TestRepo::new("Reader", "reader@example.com");
+    repo.issue_open("existing");
+    let editor = editor_writing(&repo, "issue-editor.sh", "a body for it");
+    let out = repo.run_dashboard_driven(100, 24, &[("EDITOR", &editor)], |stdin| {
+        // `n`, a title, Enter — which opens the editor for the body.
+        let _ = stdin.write_all(b"nmine\r");
+        thread::sleep(Duration::from_millis(4000));
+        let _ = stdin.write_all(b"q");
+    });
+    let text = screen_text(&out.stdout);
+    // Vacuously true if nothing was written, so check the write landed first.
+    let issues = repo.run_ok(&["issue", "list"]);
+    assert!(
+        issues.contains("mine"),
+        "the dashboard never made the write this is about:\n{}",
+        issues
+    );
+    assert!(
+        !text.contains("new event"),
+        "the dashboard announced its own write as external:\n{}",
+        text
+    );
+}
+// ── Writing from the review surface (094b055b) ──────────────────────────────
+/// The whole point of the issue: read a patch and say something about it
+/// without quitting. The body arrives through `$EDITOR`.
+#[test]
+fn r_records_a_review_composed_in_the_editor() {
+    let (repo, id) = repo_with_patch();
+    let editor = editor_writing(&repo, "review-editor.sh", "this reads well");
+    repo.run_dashboard_driven(100, 40, &[("EDITOR", &editor)], |stdin| {
+        // Switch to the patch list, open the patch, review, approve.
+        let _ = stdin.write_all(b"P");
+        thread::sleep(Duration::from_millis(300));
+        let _ = stdin.write_all(b"\r");
+        thread::sleep(Duration::from_millis(500));
+        let _ = stdin.write_all(b"R");
+        thread::sleep(Duration::from_millis(300));
+        let _ = stdin.write_all(b"a");
+        thread::sleep(Duration::from_millis(2000));
+        let _ = stdin.write_all(b"q");
+    });
+    let shown = repo.run_ok(&["patch", "show", &id]);
+    assert!(
+        shown.contains("this reads well") && shown.contains("approve"),
+        "no review recorded:\n{}",
+        shown
+    );
+}
+/// `c` on a line of the diff anchors the comment there, through the same
+/// `--at file:line` path the CLI uses — so the anchor is checked against the
+/// revision the comment names, and lands as an inline comment rather than a
+/// thread one.
+#[test]
+fn c_on_a_diff_line_records_an_inline_comment() {
+    let (repo, id) = repo_with_patch();
+    let editor = editor_writing(&repo, "comment-editor.sh", "why the rename?");
+    repo.run_dashboard_driven(100, 40, &[("EDITOR", &editor)], |stdin| {
+        let _ = stdin.write_all(b"P");
+        thread::sleep(Duration::from_millis(300));
+        let _ = stdin.write_all(b"\r");
+        thread::sleep(Duration::from_millis(500));
+        // Walk down to a line of the diff body. The detail pane opens with the
+        // cursor at the top, and the header, revision list and diff header sit
+        // above the first anchorable line.
+        let _ = stdin.write_all(b"jjjjjjjjjjjjjjjj");
+        thread::sleep(Duration::from_millis(400));
+        let _ = stdin.write_all(b"c");
+        thread::sleep(Duration::from_millis(2000));
+        let _ = stdin.write_all(b"q");
+    });
+    let shown = repo.run_ok(&["patch", "show", &id]);
+    assert!(
+        shown.contains("why the rename?"),
+        "no comment recorded:\n{}",
+        shown
+    );
+    assert!(
+        shown.contains("src/lib.rs:"),
+        "the comment did not land on a line of the file:\n{}",
+        shown
+    );
+}
+/// A body that is empty once the `#` context is stripped aborts the write, the
+/// way `git commit` does, and nothing is appended.
+#[test]
+fn a_comment_only_body_aborts_and_records_nothing() {
+    let (repo, id) = repo_with_patch();
+    // An editor that leaves the seeded `#` lines exactly as they were.
+    let editor = repo.write_script("noop-editor.sh", "#!/bin/sh\nexit 0\n");
+    let out = repo.run_dashboard_driven(100, 40, &[("EDITOR", &editor)], |stdin| {
+        let _ = stdin.write_all(b"P");
+        thread::sleep(Duration::from_millis(300));
+        let _ = stdin.write_all(b"\r");
+        thread::sleep(Duration::from_millis(500));
+        let _ = stdin.write_all(b"c");
+        thread::sleep(Duration::from_millis(2000));
+        let _ = stdin.write_all(b"q");
+    });
+    let shown = repo.run_ok(&["patch", "show", &id]);
+    assert!(
+        !shown.contains("--- Comments ---") && !shown.contains("--- Inline Comments ---"),
+        "an aborted comment was recorded anyway:\n{}",
+        shown
+    );
+    // ...and the reader is told, rather than left wondering whether the key
+    // did anything at all.
+    let text = screen_text(&out.stdout);
+    assert!(
+        text.contains("Aborting"),
+        "nothing on screen said the comment was abandoned:\n{}",
+        text
+    );
+}
+/// An editor that dies mid-write must leave the DAG alone and the dashboard
+/// usable — the screen is restored and the next key still works.
+#[test]
+fn an_editor_killed_mid_write_records_nothing() {
+    let (repo, id) = repo_with_patch();
+    let editor = repo.write_script(
+        "dying-editor.sh",
+        "#!/bin/sh\nprintf 'half a thought' > \"$1\"\nkill -TERM $$\n",
+    );
+    let out = repo.run_dashboard_driven(100, 40, &[("EDITOR", &editor)], |stdin| {
+        let _ = stdin.write_all(b"P");
+        thread::sleep(Duration::from_millis(300));
+        let _ = stdin.write_all(b"\r");
+        thread::sleep(Duration::from_millis(500));
+        let _ = stdin.write_all(b"c");
+        thread::sleep(Duration::from_millis(2000));
+        let _ = stdin.write_all(b"q");
+    });
+    let shown = repo.run_ok(&["patch", "show", &id]);
+    assert!(
+        !shown.contains("half a thought"),
+        "a killed editor's buffer was recorded:\n{}",
+        shown
+    );
+    let text = screen_text(&out.stdout);
+    assert!(
+        text.contains("Editor exited"),
+        "the dashboard did not say the editor died:\n{}",
+        text
+    );
+    // The screen came back, or `q` would never have been read and the harness
+    // would have had to kill the process.
+    assert!(
+        out.status.success(),
+        "the dashboard did not exit cleanly after a killed editor"
+    );
+}
+/// `x` claims the comment under the cursor was answered, and `x` again
+/// withdraws the claim. Both go through `PatchState::resolve_comment`, which
+/// is why the id on screen is the one the CLI takes.
+#[test]
+fn x_resolves_and_reopens_the_comment_under_the_cursor() {
+    let (repo, id) = repo_with_patch();
+    repo.run_ok(&[
+        "patch",
+        "comment",
+        &id,
+        "--at",
+        "src/lib.rs:2",
+        "-b",
+        "this line worries me",
+    ]);
+    // The detail pane opens with the cursor on row 0. Above the inline comment
+    // sit six header rows, a blank, the Revisions heading, one revision, a
+    // blank and the Inline Comments heading — eleven rows.
+    repo.run_dashboard_driven(100, 40, &[], |stdin| {
+        let _ = stdin.write_all(b"P");
+        thread::sleep(Duration::from_millis(300));
+        let _ = stdin.write_all(b"\r");
+        thread::sleep(Duration::from_millis(500));
+        let _ = stdin.write_all(b"jjjjjjjjjjj");
+        thread::sleep(Duration::from_millis(400));
+        let _ = stdin.write_all(b"x");
+        thread::sleep(Duration::from_millis(1500));
+        let _ = stdin.write_all(b"q");
+    });
+    let shown = repo.run_ok(&["patch", "show", &id]);
+    assert!(
+        shown.contains("--- Resolved"),
+        "`x` did not mark the comment answered:\n{}",
+        shown
+    );
+    // The same key withdraws the claim. The row moved into the Resolved
+    // section, one line further down.
+    repo.run_dashboard_driven(100, 40, &[], |stdin| {
+        let _ = stdin.write_all(b"P");
+        thread::sleep(Duration::from_millis(300));
+        let _ = stdin.write_all(b"\r");
+        thread::sleep(Duration::from_millis(500));
+        let _ = stdin.write_all(b"jjjjjjjjjjj");
+        thread::sleep(Duration::from_millis(400));
+        let _ = stdin.write_all(b"x");
+        thread::sleep(Duration::from_millis(1500));
+        let _ = stdin.write_all(b"q");
+    });
+    let shown = repo.run_ok(&["patch", "show", &id]);
+    assert!(
+        !shown.contains("--- Resolved"),
+        "`x` did not withdraw the claim:\n{}",
+        shown
+    );
+}
+/// `a` shows the change that answered the comment under the cursor: the
+/// correspondence between feedback and the revision answering it, which is the
+/// thing this project exists to keep. The view is the one `--answers` already
+/// built; the dashboard only points it at the selected comment.
+#[test]
+fn a_shows_what_answered_the_comment_under_the_cursor() {
+    let (repo, id) = repo_with_patch();
+    repo.run_ok(&[
+        "patch",
+        "comment",
+        &id,
+        "--at",
+        "src/lib.rs:2",
+        "-b",
+        "this line worries me",
+    ]);
+    // A revision that answers it, then the claim that it did.
+    repo.git(&["checkout", "feature/x"]);
+    repo.commit_file(
+        "src/lib.rs",
+        "one\ntwo reworded\nthree\nfour\nfive\n",
+        "address the comment",
+    );
+    repo.run_ok(&["patch", "revise", &id, "-b", "reworded"]);
+    repo.git(&["checkout", "main"]);
+    let shown = repo.run_ok(&["patch", "show", &id]);
+    let comment_id = shown
+        .lines()
+        .find(|l| l.contains("src/lib.rs:2"))
+        .and_then(|l| l.split('[').nth(1))
+        .and_then(|s| s.split(']').next())
+        .expect("patch show prints the comment id in brackets")
+        .to_string();
+    repo.run_ok(&["patch", "resolve", &id, &comment_id]);
+    let out = repo.run_dashboard_driven(100, 40, &[], |stdin| {
+        let _ = stdin.write_all(b"P");
+        thread::sleep(Duration::from_millis(300));
+        let _ = stdin.write_all(b"\r");
+        thread::sleep(Duration::from_millis(500));
+        // Six header rows, blank, Revisions heading, two revisions, blank,
+        // the Resolved-comment heading — twelve rows down to the comment.
+        let _ = stdin.write_all(b"jjjjjjjjjjjj");
+        thread::sleep(Duration::from_millis(400));
+        let _ = stdin.write_all(b"a");
+        thread::sleep(Duration::from_millis(1500));
+        let _ = stdin.write_all(b"q");
+    });
+    // Matched without the interior spaces a redraw is free to skip: ratatui
+    // repaints only the cells that changed, so a blank between two words may
+    // never be written at all.
+    let text = screen_text(&out.stdout);
+    assert!(
+        text.contains("answered [") && text.contains("two reworded"),
+        "`a` did not show the change that answered the comment:\n{}",
+        text
+    );
+}
+/// `o` checks out the latest revision's commit, which a revision ref pins.
+/// The branch the patch was authored on is deleted here on purpose: that is
+/// the state every agent-produced patch is in, and checking `PatchState.branch`
+/// out is what used to fail on them.
+#[test]
+fn o_checks_out_the_revision_even_with_the_branch_gone() {
+    let (repo, id) = repo_with_patch();
+    let head = repo.git(&["rev-parse", "feature/x"]).trim().to_string();
+    repo.git(&["branch", "-D", "feature/x"]);
+    let out = repo.run_dashboard_driven(100, 40, &[], |stdin| {
+        let _ = stdin.write_all(b"P");
+        thread::sleep(Duration::from_millis(300));
+        let _ = stdin.write_all(b"o");
+        thread::sleep(Duration::from_millis(1500));
+    });
+    let at = repo.git(&["rev-parse", "HEAD"]).trim().to_string();
+    assert_eq!(
+        at,
+        head,
+        "not on the latest revision's commit:\n{}",
+        screen_text(&out.stdout)
+    );
+    let branch = repo.git(&["rev-parse", "--abbrev-ref", "HEAD"]);
+    assert!(
+        branch.trim().starts_with("collab/"),
+        "expected a collab/ branch for patch {}, got {}",
+        id,
+        branch
+    );
+}
+/// No editor configured is a message naming the alternatives, never a hang and
+/// never a silent no-op.
+#[test]
+fn no_editor_configured_says_so() {
+    let (repo, _id) = repo_with_patch();
+    let out = repo.run_dashboard_driven(100, 40, &[("EDITOR", ""), ("VISUAL", "")], |stdin| {
+        let _ = stdin.write_all(b"P");
+        thread::sleep(Duration::from_millis(300));
+        let _ = stdin.write_all(b"\r");
+        thread::sleep(Duration::from_millis(500));
+        let _ = stdin.write_all(b"c");
+        thread::sleep(Duration::from_millis(1000));
+        let _ = stdin.write_all(b"q");
+    });
+    let text = screen_text(&out.stdout);
+    assert!(
+        text.contains("$EDITOR") || text.contains("EDITOR"),
+        "no message naming the editor variables:\n{}",
+        text
+    );
+    assert!(
+        out.status.success(),
+        "the dashboard hung or died with no editor configured"
+    );
+}