Add revision-aware review with interdiffs

CLAUDE.md

Old		New
@@ -8,6 +8,7 @@ Auto-generated from all feature plans. Last updated: 2026-03-21
8	- Rust 2021 edition + ratatui 0.30, crossterm 0.29, git2 0.19 (004-dashboard-filtering)	8	- Rust 2021 edition + ratatui 0.30, crossterm 0.29, git2 0.19 (004-dashboard-filtering)
9	- N/A (ephemeral filter state, no persistence) (004-dashboard-filtering)	9	- N/A (ephemeral filter state, no persistence) (004-dashboard-filtering)
10	- Rust 2021 edition + git2 0.19, clap 4 (derive), serde/serde_json 1, chrono 0.4, ed25519-dalek 2 (012-patch-branch-refactor)	10	- Rust 2021 edition + git2 0.19, clap 4 (derive), serde/serde_json 1, chrono 0.4, ed25519-dalek 2 (012-patch-branch-refactor)
		11	- Git object database (collab DAG refs under `.git/refs/collab/`) (015-gerrit-style-patchsets)
11		12
12	- Rust 2021 edition + git2 0.19, clap 4, serde/serde_json 1, chrono 0.4, thiserror 2. New: `ed25519-dalek`, `rand`, `base64` (001-gpg-event-signing)	13	- Rust 2021 edition + git2 0.19, clap 4, serde/serde_json 1, chrono 0.4, thiserror 2. New: `ed25519-dalek`, `rand`, `base64` (001-gpg-event-signing)
13		14
@@ -27,9 +28,9 @@ cargo test [ONLY COMMANDS FOR ACTIVE TECHNOLOGIES][ONLY COMMANDS FOR ACTIVE TECH
27	Rust 2021 edition: Follow standard conventions	28	Rust 2021 edition: Follow standard conventions
28		29
29	## Recent Changes	30	## Recent Changes
		31	- 015-gerrit-style-patchsets: Added Rust 2021 edition + git2 0.19, clap 4 (derive), serde/serde_json 1, chrono 0.4, ed25519-dalek 2
30	- 012-patch-branch-refactor: Added Rust 2021 edition + git2 0.19, clap 4 (derive), serde/serde_json 1, chrono 0.4, ed25519-dalek 2	32	- 012-patch-branch-refactor: Added Rust 2021 edition + git2 0.19, clap 4 (derive), serde/serde_json 1, chrono 0.4, ed25519-dalek 2
31	- 004-dashboard-filtering: Added Rust 2021 edition + ratatui 0.30, crossterm 0.29, git2 0.19	33	- 004-dashboard-filtering: Added Rust 2021 edition + ratatui 0.30, crossterm 0.29, git2 0.19
32	- 003-key-trust-allowlist: Added Rust 2021 edition + git2 0.19, clap 4 (derive), ed25519-dalek 2, base64 0.22, serde/serde_json 1, dirs 5, thiserror 2
33		34
34		35
35	<!-- MANUAL ADDITIONS START -->	36	<!-- MANUAL ADDITIONS START -->

benches/core_ops.rs

Old		New
@@ -76,6 +76,8 @@ fn setup_patches(n: usize) -> (Repository, TempDir) {
76	base_ref: "main".to_string(),	76	base_ref: "main".to_string(),
77	branch: format!("feature-{}", i),	77	branch: format!("feature-{}", i),
78	fixes: None,	78	fixes: None,
		79	commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
		80	tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
79	},	81	},
80	clock: 0,	82	clock: 0,
81	};	83	};
@@ -215,6 +217,8 @@ fn bench_patch_from_ref(c: &mut Criterion) {
215	base_ref: "main".to_string(),	217	base_ref: "main".to_string(),
216	branch: format!("feature-bench-{}", count),	218	branch: format!("feature-bench-{}", count),
217	fixes: None,	219	fixes: None,
		220	commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
		221	tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
218	},	222	},
219	clock: 0,	223	clock: 0,
220	};	224	};

src/cli.rs

Old		New
@@ -266,11 +266,20 @@ pub enum PatchCmd {
266	/// Output as JSON	266	/// Output as JSON
267	#[arg(long)]	267	#[arg(long)]
268	json: bool,	268	json: bool,
		269	/// Show only comments/reviews from this revision
		270	#[arg(long)]
		271	revision: Option<u32>,
269	},	272	},
270	/// Show diff between base and head	273	/// Show diff between base and head
271	Diff {	274	Diff {
272	/// Patch ID (prefix match)	275	/// Patch ID (prefix match)
273	id: String,	276	id: String,
		277	/// Show historical diff for a specific revision against the base
		278	#[arg(long)]
		279	revision: Option<u32>,
		280	/// Show interdiff between two revisions (N M or just N for N..latest)
		281	#[arg(long, num_args = 1..=2)]
		282	between: Option<Vec<u32>>,
274	},	283	},
275	/// Comment on a patch (use --file and --line for inline comments)	284	/// Comment on a patch (use --file and --line for inline comments)
276	Comment {	285	Comment {
@@ -285,6 +294,9 @@ pub enum PatchCmd {
285	/// Line number for inline comment	294	/// Line number for inline comment
286	#[arg(short, long)]	295	#[arg(short, long)]
287	line: Option<u32>,	296	line: Option<u32>,
		297	/// Target revision for inline comment
		298	#[arg(long)]
		299	revision: Option<u32>,
288	},	300	},
289	/// Review a patch	301	/// Review a patch
290	Review {	302	Review {
@@ -296,15 +308,26 @@ pub enum PatchCmd {
296	/// Review body	308	/// Review body
297	#[arg(short, long)]	309	#[arg(short, long)]
298	body: String,	310	body: String,
		311	/// Target revision for review
		312	#[arg(long)]
		313	revision: Option<u32>,
299	},	314	},
300	/// Revise a patch (record a revision note)	315	/// Revise a patch (record a new revision snapshot)
301	Revise {	316	Revise {
302	/// Patch ID (prefix match)	317	/// Patch ID (prefix match)
303	id: String,	318	id: String,
304	/// Updated description	319	/// Revision description
305	#[arg(short, long)]	320	#[arg(short, long)]
306	body: Option<String>,	321	body: Option<String>,
307	},	322	},
		323	/// Show revision log for a patch
		324	Log {
		325	/// Patch ID (prefix match)
		326	id: String,
		327	/// Output as JSON
		328	#[arg(long)]
		329	json: bool,
		330	},
308	/// Merge a patch into its base branch	331	/// Merge a patch into its base branch
309	Merge {	332	Merge {
310	/// Patch ID (prefix match)	333	/// Patch ID (prefix match)

src/dag.rs

Old		New
@@ -332,7 +332,7 @@ fn commit_message(action: &Action) -> String {
332	Action::IssueClose { .. } => "issue: close".to_string(),	332	Action::IssueClose { .. } => "issue: close".to_string(),
333	Action::IssueReopen => "issue: reopen".to_string(),	333	Action::IssueReopen => "issue: reopen".to_string(),
334	Action::PatchCreate { title, .. } => format!("patch: create \"{}\"", title),	334	Action::PatchCreate { title, .. } => format!("patch: create \"{}\"", title),
335	Action::PatchRevise { .. } => "patch: revise".to_string(),	335	Action::PatchRevision { .. } => "patch: revision".to_string(),
336	Action::PatchReview { verdict, .. } => format!("patch: review ({:?})", verdict),	336	Action::PatchReview { verdict, .. } => format!("patch: review ({:?})", verdict),
337	Action::PatchComment { .. } => "patch: comment".to_string(),	337	Action::PatchComment { .. } => "patch: comment".to_string(),
338	Action::PatchInlineComment { ref file, line, .. } => {	338	Action::PatchInlineComment { ref file, line, .. } => {

src/event.rs

Old		New
@@ -65,15 +65,21 @@ pub enum Action {
65	branch: String,	65	branch: String,
66	#[serde(default, skip_serializing_if = "Option::is_none")]	66	#[serde(default, skip_serializing_if = "Option::is_none")]
67	fixes: Option<String>,	67	fixes: Option<String>,
		68	commit: String,
		69	tree: String,
68	},	70	},
69	#[serde(rename = "patch.revise")]	71	#[serde(rename = "patch.revision")]
70	PatchRevise {	72	PatchRevision {
		73	commit: String,
		74	tree: String,
		75	#[serde(default, skip_serializing_if = "Option::is_none")]
71	body: Option<String>,	76	body: Option<String>,
72	},	77	},
73	#[serde(rename = "patch.review")]	78	#[serde(rename = "patch.review")]
74	PatchReview {	79	PatchReview {
75	verdict: ReviewVerdict,	80	verdict: ReviewVerdict,
76	body: String,	81	body: String,
		82	revision: u32,
77	},	83	},
78	#[serde(rename = "patch.comment")]	84	#[serde(rename = "patch.comment")]
79	PatchComment {	85	PatchComment {
@@ -84,6 +90,7 @@ pub enum Action {
84	file: String,	90	file: String,
85	line: u32,	91	line: u32,
86	body: String,	92	body: String,
		93	revision: u32,
87	},	94	},
88	#[serde(rename = "patch.close")]	95	#[serde(rename = "patch.close")]
89	PatchClose {	96	PatchClose {
@@ -100,4 +107,5 @@ pub enum ReviewVerdict {
100	Approve,	107	Approve,
101	RequestChanges,	108	RequestChanges,
102	Comment,	109	Comment,
		110	Reject,
103	}	111	}

src/lib.rs

Old		New
@@ -22,6 +22,27 @@ use event::ReviewVerdict;
22	use git2::Repository;	22	use git2::Repository;
23	use state::{IssueStatus, PatchStatus};	23	use state::{IssueStatus, PatchStatus};
24		24
		25	/// Check if the reviewer's base ref has moved ahead of the patch's latest revision.
		26	pub fn staleness_warning(repo: &Repository, patch: &state::PatchState) -> Option<String> {
		27	let latest_commit = &patch.revisions.last()?.commit;
		28	let commit_oid = git2::Oid::from_str(latest_commit).ok()?;
		29	let base_ref = format!("refs/heads/{}", patch.base_ref);
		30	let base_tip = repo.refname_to_id(&base_ref).ok()?;
		31	let merge_base = repo.merge_base(commit_oid, base_tip).ok()?;
		32	if merge_base == base_tip {
		33	return None;
		34	}
		35	let (ahead, _) = repo.graph_ahead_behind(base_tip, merge_base).ok()?;
		36	if ahead == 0 {
		37	return None;
		38	}
		39	let mb_short = &merge_base.to_string()[..8];
		40	Some(format!(
		41	"\u{26a0} Based on {}@{} ({} commits behind your {})",
		42	patch.base_ref, mb_short, ahead, patch.base_ref
		43	))
		44	}
		45
25	pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {	46	pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
26	match cli.command {	47	match cli.command {
27	Commands::Init => sync::init(repo),	48	Commands::Init => sync::init(repo),
@@ -209,7 +230,7 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
209	}	230	}
210	Ok(())	231	Ok(())
211	}	232	}
212	PatchCmd::Show { id, json } => {	233	PatchCmd::Show { id, json, revision } => {
213	if json {	234	if json {
214	let output = patch::show_json(repo, &id)?;	235	let output = patch::show_json(repo, &id)?;
215	println!("{}", output);	236	println!("{}", output);
@@ -221,7 +242,8 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
221	PatchStatus::Closed => "closed",	242	PatchStatus::Closed => "closed",
222	PatchStatus::Merged => "merged",	243	PatchStatus::Merged => "merged",
223	};	244	};
224	println!("Patch {} [{}]", &p.id[..8], status);	245	let rev_count = p.revisions.len();
		246	println!("Patch {} [{}] (r{})", &p.id[..8], status, rev_count);
225	println!("Title: {}", p.title);	247	println!("Title: {}", p.title);
226	println!("Author: {} <{}>", p.author.name, p.author.email);	248	println!("Author: {} <{}>", p.author.name, p.author.email);
227	match p.resolve_head(repo) {	249	match p.resolve_head(repo) {
@@ -240,27 +262,55 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
240	if let Some(ref fixes) = p.fixes {	262	if let Some(ref fixes) = p.fixes {
241	println!("Fixes: {:.8}", fixes);	263	println!("Fixes: {:.8}", fixes);
242	}	264	}
		265	// Staleness warning
		266	if let Some(warning) = staleness_warning(repo, &p) {
		267	eprintln!("{}", warning);
		268	}
		269	// Show revisions
		270	if !p.revisions.is_empty() {
		271	println!("\n--- Revisions ---");
		272	for rev in &p.revisions {
		273	let short = if rev.commit.len() >= 8 { &rev.commit[..8] } else { &rev.commit };
		274	let body_display = rev.body.as_deref().map(|b| format!(" {}", b)).unwrap_or_default();
		275	println!(" r{}: {} ({}){}", rev.number, short, rev.timestamp, body_display);
		276	}
		277	}
243	if !p.body.is_empty() {	278	if !p.body.is_empty() {
244	println!("\n{}", p.body);	279	println!("\n{}", p.body);
245	}	280	}
246	if !p.reviews.is_empty() {	281	// Filter reviews by revision if requested
		282	let reviews: Vec<_> = if let Some(rev) = revision {
		283	p.reviews.iter().filter(|r| r.revision == Some(rev)).collect()
		284	} else {
		285	p.reviews.iter().collect()
		286	};
		287	if !reviews.is_empty() {
247	println!("\n--- Reviews ---");	288	println!("\n--- Reviews ---");
248	for r in &p.reviews {	289	for r in &reviews {
		290	let rev_label = r.revision.map(|n| format!(" (r{})", n)).unwrap_or_default();
249	println!(	291	println!(
250	"\n{} ({:?}) - {}:\n{}",	292	"\n{} ({:?}) - {}{}:\n{}",
251	r.author.name, r.verdict, r.timestamp, r.body	293	r.author.name, r.verdict, r.timestamp, rev_label, r.body
252	);	294	);
253	}	295	}
254	}	296	}
255	if !p.inline_comments.is_empty() {	297	// Filter inline comments by revision if requested
		298	let inline_comments: Vec<_> = if let Some(rev) = revision {
		299	p.inline_comments.iter().filter(|c| c.revision == Some(rev)).collect()
		300	} else {
		301	p.inline_comments.iter().collect()
		302	};
		303	if !inline_comments.is_empty() {
256	println!("\n--- Inline Comments ---");	304	println!("\n--- Inline Comments ---");
257	for c in &p.inline_comments {	305	for c in &inline_comments {
		306	let rev_label = c.revision.map(|n| format!(" r{}", n)).unwrap_or_default();
258	println!(	307	println!(
259	"\n{} on {}:{} ({}):\n {}",	308	"\n{} on {}:{} ({}{}):\n {}",
260	c.author.name, c.file, c.line, c.timestamp, c.body	309	c.author.name, c.file, c.line, c.timestamp, rev_label, c.body
261	);	310	);
262	}	311	}
263	}	312	}
		313	// Thread comments always shown
264	if !p.comments.is_empty() {	314	if !p.comments.is_empty() {
265	println!("\n--- Comments ---");	315	println!("\n--- Comments ---");
266	for c in &p.comments {	316	for c in &p.comments {
@@ -269,8 +319,18 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
269	}	319	}
270	Ok(())	320	Ok(())
271	}	321	}
272	PatchCmd::Diff { id } => {	322	PatchCmd::Diff { id, revision, between } => {
273	let diff = patch::diff(repo, &id)?;	323	if revision.is_some() && between.is_some() {
		324	return Err(error::Error::Cmd(
		325	"--revision and --between are mutually exclusive".to_string(),
		326	));
		327	}
		328	let between_pair = between.map(|v| {
		329	let from = v[0];
		330	let to = v.get(1).copied();
		331	(from, to)
		332	});
		333	let diff = patch::diff(repo, &id, revision, between_pair)?;
274	if diff.is_empty() {	334	if diff.is_empty() {
275	println!("No diff available (commits may be identical).");	335	println!("No diff available (commits may be identical).");
276	} else {	336	} else {
@@ -283,24 +343,26 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
283	body,	343	body,
284	file,	344	file,
285	line,	345	line,
		346	revision,
286	} => {	347	} => {
287	patch::comment(repo, &id, &body, file.as_deref(), line)?;	348	patch::comment(repo, &id, &body, file.as_deref(), line, revision)?;
288	println!("Comment added.");	349	println!("Comment added.");
289	Ok(())	350	Ok(())
290	}	351	}
291	PatchCmd::Review { id, verdict, body } => {	352	PatchCmd::Review { id, verdict, body, revision } => {
292	let v = match verdict.as_str() {	353	let v = match verdict.as_str() {
293	"approve" => ReviewVerdict::Approve,	354	"approve" => ReviewVerdict::Approve,
294	"request-changes" => ReviewVerdict::RequestChanges,	355	"request-changes" => ReviewVerdict::RequestChanges,
295	"comment" => ReviewVerdict::Comment,	356	"comment" => ReviewVerdict::Comment,
		357	"reject" => ReviewVerdict::Reject,
296	_ => {	358	_ => {
297	return Err(git2::Error::from_str(	359	return Err(git2::Error::from_str(
298	"verdict must be: approve, request-changes, or comment",	360	"verdict must be: approve, request-changes, comment, or reject",
299	)	361	)
300	.into());	362	.into());
301	}	363	}
302	};	364	};
303	patch::review(repo, &id, v, &body)?;	365	patch::review(repo, &id, v, &body, revision)?;
304	println!("Review submitted.");	366	println!("Review submitted.");
305	Ok(())	367	Ok(())
306	}	368	}
@@ -309,6 +371,16 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
309	println!("Patch revised.");	371	println!("Patch revised.");
310	Ok(())	372	Ok(())
311	}	373	}
		374	PatchCmd::Log { id, json } => {
		375	let p = patch::patch_log(repo, &id)?;
		376	if json {
		377	let output = patch::patch_log_json(&p)?;
		378	println!("{}", output);
		379	} else {
		380	patch::patch_log_to_writer(repo, &p, &mut std::io::stdout())?;
		381	}
		382	Ok(())
		383	}
312	PatchCmd::Merge { id } => {	384	PatchCmd::Merge { id } => {
313	let p = patch::merge(repo, &id)?;	385	let p = patch::merge(repo, &id)?;
314	println!("Patch {:.8} merged into {}.", p.id, p.base_ref);	386	println!("Patch {:.8} merged into {}.", p.id, p.base_ref);

src/log.rs

Old		New
@@ -116,7 +116,7 @@ fn action_type_name(action: &Action) -> String {
116	Action::IssueUnassign { .. } => "IssueUnassign".to_string(),	116	Action::IssueUnassign { .. } => "IssueUnassign".to_string(),
117	Action::IssueReopen => "IssueReopen".to_string(),	117	Action::IssueReopen => "IssueReopen".to_string(),
118	Action::PatchCreate { .. } => "PatchCreate".to_string(),	118	Action::PatchCreate { .. } => "PatchCreate".to_string(),
119	Action::PatchRevise { .. } => "PatchRevise".to_string(),	119	Action::PatchRevision { .. } => "PatchRevision".to_string(),
120	Action::PatchReview { .. } => "PatchReview".to_string(),	120	Action::PatchReview { .. } => "PatchReview".to_string(),
121	Action::PatchComment { .. } => "PatchComment".to_string(),	121	Action::PatchComment { .. } => "PatchComment".to_string(),
122	Action::PatchInlineComment { .. } => "PatchInlineComment".to_string(),	122	Action::PatchInlineComment { .. } => "PatchInlineComment".to_string(),
@@ -150,9 +150,9 @@ fn action_summary(action: &Action) -> String {
150	Action::IssueUnassign { assignee } => format!("unassign \"{}\"", assignee),	150	Action::IssueUnassign { assignee } => format!("unassign \"{}\"", assignee),
151	Action::IssueReopen => "reopen".to_string(),	151	Action::IssueReopen => "reopen".to_string(),
152	Action::PatchCreate { title, .. } => format!("create \"{}\"", title),	152	Action::PatchCreate { title, .. } => format!("create \"{}\"", title),
153	Action::PatchRevise { body, .. } => match body {	153	Action::PatchRevision { body, .. } => match body {
154	Some(b) => format!("revise: {}", truncate(b, 50)),	154	Some(b) => format!("revision: {}", truncate(b, 50)),
155	None => "revise".to_string(),	155	None => "revision".to_string(),
156	},	156	},
157	Action::PatchReview { verdict, .. } => format!("review: {:?}", verdict),	157	Action::PatchReview { verdict, .. } => format!("review: {:?}", verdict),
158	Action::PatchComment { body } => truncate(body, 60),	158	Action::PatchComment { body } => truncate(body, 60),

src/patch.rs

Old		New
@@ -1,4 +1,4 @@
1	use git2::{DiffFormat, Repository};	1	use git2::{DiffFormat, Oid, Repository};
2		2
3	use crate::cli::SortMode;	3	use crate::cli::SortMode;
4	use crate::dag;	4	use crate::dag;
@@ -6,7 +6,69 @@ use crate::error::Error;
6	use crate::event::{Action, Event, ReviewVerdict};	6	use crate::event::{Action, Event, ReviewVerdict};
7	use crate::identity::get_author;	7	use crate::identity::get_author;
8	use crate::signing;	8	use crate::signing;
9	use crate::state::{self, PatchState, PatchStatus};	9	use crate::state::{self, PatchState, PatchStatus, Revision};
		10
		11	/// Auto-detect whether the branch tip has changed since the last recorded revision.
		12	/// If it has, append a PatchRevision event and return the new `Revision` so callers
		13	/// can update their in-memory `PatchState` without re-walking the DAG.
		14	fn auto_detect_revision(
		15	repo: &Repository,
		16	ref_name: &str,
		17	patch: &PatchState,
		18	sk: &ed25519_dalek::SigningKey,
		19	) -> Result<Option<Revision>, Error> {
		20	let current_rev = patch.revisions.last().map(|r| r.number).unwrap_or(0);
		21
		22	// Try to resolve the branch tip
		23	let tip_oid = match patch.resolve_head(repo) {
		24	Ok(oid) => oid,
		25	Err(_) => return Ok(None), // Branch deleted or unavailable
		26	};
		27
		28	let last_commit = patch.revisions.last().map(|r| r.commit.as_str()).unwrap_or("");
		29	let tip_hex = tip_oid.to_string();
		30
		31	if tip_hex == last_commit {
		32	return Ok(None);
		33	}
		34
		35	// Branch tip changed — insert a PatchRevision event
		36	let commit = repo.find_commit(tip_oid)?;
		37	let tree_oid = commit.tree()?.id();
		38	let author = get_author(repo)?;
		39	let timestamp = chrono::Utc::now().to_rfc3339();
		40	let event = Event {
		41	timestamp: timestamp.clone(),
		42	author,
		43	action: Action::PatchRevision {
		44	commit: tip_hex.clone(),
		45	tree: tree_oid.to_string(),
		46	body: None,
		47	},
		48	clock: 0,
		49	};
		50	dag::append_event(repo, ref_name, &event, sk)?;
		51	Ok(Some(Revision {
		52	number: current_rev + 1,
		53	commit: tip_hex,
		54	tree: tree_oid.to_string(),
		55	body: None,
		56	timestamp,
		57	}))
		58	}
		59
		60	/// Auto-detect revision changes and update the in-memory PatchState in place.
		61	fn auto_detect_and_update(
		62	repo: &Repository,
		63	ref_name: &str,
		64	patch: &mut PatchState,
		65	sk: &ed25519_dalek::SigningKey,
		66	) -> Result<(), Error> {
		67	if let Some(rev) = auto_detect_revision(repo, ref_name, patch, sk)? {
		68	patch.revisions.push(rev);
		69	}
		70	Ok(())
		71	}
10		72
11	pub fn create(	73	pub fn create(
12	repo: &Repository,	74	repo: &Repository,
@@ -23,9 +85,9 @@ pub fn create(
23	));	85	));
24	}	86	}
25		87
26	// Verify branch exists	88	// Verify branch exists and get tip
27	let branch_ref = format!("refs/heads/{}", branch);	89	let branch_ref = format!("refs/heads/{}", branch);
28	repo.refname_to_id(&branch_ref)	90	let tip_oid = repo.refname_to_id(&branch_ref)
29	.map_err(|e| crate::error::Error::Cmd(format!("branch '{}' not found: {}", branch, e)))?;	91	.map_err(|e| crate::error::Error::Cmd(format!("branch '{}' not found: {}", branch, e)))?;
30		92
31	// Check for duplicate: scan open patches for matching branch	93	// Check for duplicate: scan open patches for matching branch
@@ -39,6 +101,10 @@ pub fn create(
39	}	101	}
40	}	102	}
41		103
		104	// Get commit and tree OIDs for revision 1
		105	let commit = repo.find_commit(tip_oid)?;
		106	let tree_oid = commit.tree()?.id();
		107
42	let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;	108	let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;
43	let author = get_author(repo)?;	109	let author = get_author(repo)?;
44	let event = Event {	110	let event = Event {
@@ -50,6 +116,8 @@ pub fn create(
50	base_ref: base_ref.to_string(),	116	base_ref: base_ref.to_string(),
51	branch: branch.to_string(),	117	branch: branch.to_string(),
52	fixes: fixes.map(|s| s.to_string()),	118	fixes: fixes.map(|s| s.to_string()),
		119	commit: tip_oid.to_string(),
		120	tree: tree_oid.to_string(),
53	},	121	},
54	clock: 0,	122	clock: 0,
55	};	123	};
@@ -155,26 +223,53 @@ pub fn comment(
155	body: &str,	223	body: &str,
156	file: Option<&str>,	224	file: Option<&str>,
157	line: Option<u32>,	225	line: Option<u32>,
		226	target_revision: Option<u32>,
158	) -> Result<(), crate::error::Error> {	227	) -> Result<(), crate::error::Error> {
159	let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;	228	let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;
160	let (ref_name, _id) = state::resolve_patch_ref(repo, id_prefix)?;	229	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
		230	let mut patch = PatchState::from_ref(repo, &ref_name, &id)?;
		231
		232	// Auto-detect revision (runs for all comment types to record branch changes)
		233	auto_detect_and_update(repo, &ref_name, &mut patch, &sk)?;
		234
161	let author = get_author(repo)?;	235	let author = get_author(repo)?;
162		236
163	let action = match (file, line) {	237	let action = match (file, line) {
164	(Some(f), Some(l)) => Action::PatchInlineComment {	238	(Some(f), Some(l)) => {
165	file: f.to_string(),	239	// Determine revision for the inline comment
166	line: l,	240	let rev = if let Some(target) = target_revision {
167	body: body.to_string(),	241	// Validate target revision exists
168	},	242	if !patch.revisions.iter().any(|r| r.number == target) {
		243	return Err(Error::Cmd(format!("revision {} not found", target)));
		244	}
		245	target
		246	} else {
		247	patch.revisions.last().map(|r| r.number).unwrap_or(1)
		248	};
		249	Action::PatchInlineComment {
		250	file: f.to_string(),
		251	line: l,
		252	body: body.to_string(),
		253	revision: rev,
		254	}
		255	}
169	(Some(_), None) | (None, Some(_)) => {	256	(Some(_), None) | (None, Some(_)) => {
170	return Err(git2::Error::from_str(	257	return Err(git2::Error::from_str(
171	"--file and --line must both be provided for inline comments",	258	"--file and --line must both be provided for inline comments",
172	)	259	)
173	.into());	260	.into());
174	}	261	}
175	(None, None) => Action::PatchComment {	262	(None, None) => {
176	body: body.to_string(),	263	// Thread comment — not revision-anchored
177	},	264	if target_revision.is_some() {
		265	return Err(Error::Cmd(
		266	"thread comments are not revision-scoped; use --file and --line for inline comments".to_string(),
		267	));
		268	}
		269	Action::PatchComment {
		270	body: body.to_string(),
		271	}
		272	}
178	};	273	};
179		274
180	let event = Event {	275	let event = Event {
@@ -192,20 +287,55 @@ pub fn review(
192	id_prefix: &str,	287	id_prefix: &str,
193	verdict: ReviewVerdict,	288	verdict: ReviewVerdict,
194	body: &str,	289	body: &str,
		290	target_revision: Option<u32>,
195	) -> Result<(), crate::error::Error> {	291	) -> Result<(), crate::error::Error> {
196	let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;	292	let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;
197	let (ref_name, _id) = state::resolve_patch_ref(repo, id_prefix)?;	293	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
		294	let mut patch = PatchState::from_ref(repo, &ref_name, &id)?;
		295
		296	// Auto-detect revision
		297	auto_detect_and_update(repo, &ref_name, &mut patch, &sk)?;
		298
		299	let rev = if let Some(target) = target_revision {
		300	if !patch.revisions.iter().any(|r| r.number == target) {
		301	return Err(Error::Cmd(format!("revision {} not found", target)));
		302	}
		303	target
		304	} else {
		305	patch.revisions.last().map(|r| r.number).unwrap_or(1)
		306	};
		307
		308	let is_reject = verdict == ReviewVerdict::Reject;
198	let author = get_author(repo)?;	309	let author = get_author(repo)?;
199	let event = Event {	310	let event = Event {
200	timestamp: chrono::Utc::now().to_rfc3339(),	311	timestamp: chrono::Utc::now().to_rfc3339(),
201	author,	312	author: author.clone(),
202	action: Action::PatchReview {	313	action: Action::PatchReview {
203	verdict,	314	verdict,
204	body: body.to_string(),	315	body: body.to_string(),
		316	revision: rev,
205	},	317	},
206	clock: 0,	318	clock: 0,
207	};	319	};
208	dag::append_event(repo, &ref_name, &event, &sk)?;	320	dag::append_event(repo, &ref_name, &event, &sk)?;
		321
		322	// A reject verdict also closes the patch
		323	if is_reject {
		324	let close_event = Event {
		325	timestamp: chrono::Utc::now().to_rfc3339(),
		326	author,
		327	action: Action::PatchClose {
		328	reason: Some(format!("Rejected: {}", body)),
		329	},
		330	clock: 0,
		331	};
		332	dag::append_event(repo, &ref_name, &close_event, &sk)?;
		333	// Archive the ref
		334	if ref_name.starts_with("refs/collab/patches/") {
		335	state::archive_patch_ref(repo, &id)?;
		336	}
		337	}
		338
209	Ok(())	339	Ok(())
210	}	340	}
211		341
@@ -215,12 +345,31 @@ pub fn revise(
215	body: Option<&str>,	345	body: Option<&str>,
216	) -> Result<(), crate::error::Error> {	346	) -> Result<(), crate::error::Error> {
217	let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;	347	let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;
218	let (ref_name, _id) = state::resolve_patch_ref(repo, id_prefix)?;	348	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
		349	let patch = PatchState::from_ref(repo, &ref_name, &id)?;
		350
		351	// Resolve current branch tip
		352	let tip_oid = patch.resolve_head(repo)?;
		353	let tip_hex = tip_oid.to_string();
		354	let last_commit = patch.revisions.last().map(|r| r.commit.as_str()).unwrap_or("");
		355
		356	if tip_hex == last_commit {
		357	let current_rev = patch.revisions.last().map(|r| r.number).unwrap_or(0);
		358	return Err(Error::Cmd(format!(
		359	"no changes since revision {}",
		360	current_rev
		361	)));
		362	}
		363
		364	let commit = repo.find_commit(tip_oid)?;
		365	let tree_oid = commit.tree()?.id();
219	let author = get_author(repo)?;	366	let author = get_author(repo)?;
220	let event = Event {	367	let event = Event {
221	timestamp: chrono::Utc::now().to_rfc3339(),	368	timestamp: chrono::Utc::now().to_rfc3339(),
222	author,	369	author,
223	action: Action::PatchRevise {	370	action: Action::PatchRevision {
		371	commit: tip_hex,
		372	tree: tree_oid.to_string(),
224	body: body.map(|s| s.to_string()),	373	body: body.map(|s| s.to_string()),
225	},	374	},
226	clock: 0,	375	clock: 0,
@@ -232,7 +381,7 @@ pub fn revise(
232	pub fn merge(repo: &Repository, id_prefix: &str) -> Result<PatchState, crate::error::Error> {	381	pub fn merge(repo: &Repository, id_prefix: &str) -> Result<PatchState, crate::error::Error> {
233	let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;	382	let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;
234	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;	383	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
235	let p = PatchState::from_ref(repo, &ref_name, &id)?;	384	let mut p = PatchState::from_ref(repo, &ref_name, &id)?;
236		385
237	if p.status != PatchStatus::Open {	386	if p.status != PatchStatus::Open {
238	return Err(git2::Error::from_str(&format!(	387	return Err(git2::Error::from_str(&format!(
@@ -242,7 +391,25 @@ pub fn merge(repo: &Repository, id_prefix: &str) -> Result<PatchState, crate::er
242	.into());	391	.into());
243	}	392	}
244		393
245	// Resolve the head commit: use branch tip for branch-based patches, stored OID otherwise	394	// Auto-detect revision before merge
		395	auto_detect_and_update(repo, &ref_name, &mut p, &sk)?;
		396
		397	// Check merge policy
		398	let config = read_collab_config(repo);
		399	if config.require_approval_on_latest {
		400	let latest_rev = p.revisions.last().map(|r| r.number).unwrap_or(0);
		401	let has_approval = p.reviews.iter().any(|r| {
		402	r.verdict == ReviewVerdict::Approve && r.revision == Some(latest_rev)
		403	});
		404	if !has_approval {
		405	return Err(Error::Cmd(format!(
		406	"merge requires approval on the latest revision (revision {})",
		407	latest_rev
		408	)));
		409	}
		410	}
		411
		412	// Resolve the head commit
246	let head_oid = p.resolve_head(repo)?;	413	let head_oid = p.resolve_head(repo)?;
247	let head_commit = repo.find_commit(head_oid)	414	let head_commit = repo.find_commit(head_oid)
248	.map_err(|_| git2::Error::from_str("cannot resolve head commit in patch"))?;	415	.map_err(|_| git2::Error::from_str("cannot resolve head commit in patch"))?;
@@ -319,10 +486,23 @@ pub fn merge(repo: &Repository, id_prefix: &str) -> Result<PatchState, crate::er
319	}	486	}
320		487
321	/// Generate a unified diff between a patch's base branch and head commit.	488	/// Generate a unified diff between a patch's base branch and head commit.
322	pub fn diff(repo: &Repository, id_prefix: &str) -> Result<String, Error> {	489	pub fn diff(
		490	repo: &Repository,
		491	id_prefix: &str,
		492	revision: Option<u32>,
		493	between: Option<(u32, Option<u32>)>,
		494	) -> Result<String, Error> {
323	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;	495	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
324	let p = PatchState::from_ref(repo, &ref_name, &id)?;	496	let p = PatchState::from_ref(repo, &ref_name, &id)?;
325	generate_diff(repo, &p)	497
		498	if let Some((from, to)) = between {
		499	let to_rev = to.unwrap_or_else(|| p.revisions.last().map(|r| r.number).unwrap_or(1));
		500	interdiff(repo, &p, from, to_rev)
		501	} else if let Some(rev) = revision {
		502	generate_diff_at_revision(repo, &p, rev)
		503	} else {
		504	generate_diff(repo, &p)
		505	}
326	}	506	}
327		507
328	/// Generate a diff string from a patch's base and head using three-dot (merge-base) diff.	508	/// Generate a diff string from a patch's base and head using three-dot (merge-base) diff.
@@ -346,12 +526,67 @@ pub fn generate_diff(repo: &Repository, patch: &state::PatchState) -> Result<Str
346	};	526	};
347		527
348	let git_diff = repo.diff_tree_to_tree(base_tree.as_ref(), Some(&head_tree), None)?;	528	let git_diff = repo.diff_tree_to_tree(base_tree.as_ref(), Some(&head_tree), None)?;
		529	format_diff(&git_diff)
		530	}
		531
		532	/// Generate a diff for a specific revision against the base branch (historical full diff).
		533	fn generate_diff_at_revision(
		534	repo: &Repository,
		535	patch: &PatchState,
		536	rev_number: u32,
		537	) -> Result<String, Error> {
		538	let revision = patch.revisions.iter().find(|r| r.number == rev_number)
		539	.ok_or_else(|| Error::Cmd(format!("revision {} not found", rev_number)))?;
		540
		541	let tree_oid = Oid::from_str(&revision.tree)?;
		542	let head_tree = repo.find_tree(tree_oid)?;
		543
		544	let base_ref = format!("refs/heads/{}", patch.base_ref);
		545	let commit_oid = Oid::from_str(&revision.commit)?;
		546	let base_tree = if let Ok(base_oid) = repo.refname_to_id(&base_ref) {
		547	if let Ok(merge_base_oid) = repo.merge_base(base_oid, commit_oid) {
		548	let merge_base_commit = repo.find_commit(merge_base_oid)?;
		549	Some(merge_base_commit.tree()?)
		550	} else {
		551	let base_commit = repo.find_commit(base_oid)?;
		552	Some(base_commit.tree()?)
		553	}
		554	} else {
		555	None
		556	};
		557
		558	let git_diff = repo.diff_tree_to_tree(base_tree.as_ref(), Some(&head_tree), None)?;
		559	format_diff(&git_diff)
		560	}
		561
		562	/// Compute the interdiff between two revisions.
		563	pub fn interdiff(
		564	repo: &Repository,
		565	patch: &PatchState,
		566	from_rev: u32,
		567	to_rev: u32,
		568	) -> Result<String, Error> {
		569	let from = patch.revisions.iter().find(|r| r.number == from_rev)
		570	.ok_or_else(|| Error::Cmd(format!("revision {} not found", from_rev)))?;
		571	let to = patch.revisions.iter().find(|r| r.number == to_rev)
		572	.ok_or_else(|| Error::Cmd(format!("revision {} not found", to_rev)))?;
		573
		574	let from_tree_oid = Oid::from_str(&from.tree)?;
		575	let to_tree_oid = Oid::from_str(&to.tree)?;
		576	let from_tree = repo.find_tree(from_tree_oid)?;
		577	let to_tree = repo.find_tree(to_tree_oid)?;
		578
		579	let git_diff = repo.diff_tree_to_tree(Some(&from_tree), Some(&to_tree), None)?;
		580	format_diff(&git_diff)
		581	}
349		582
		583	/// Format a git2::Diff as a unified diff string.
		584	fn format_diff(git_diff: &git2::Diff) -> Result<String, Error> {
350	let mut output = String::new();	585	let mut output = String::new();
351	let mut lines = 0usize;	586	let mut lines = 0usize;
352	git_diff.print(DiffFormat::Patch, |_delta, _hunk, line| {	587	git_diff.print(DiffFormat::Patch, |_delta, _hunk, line| {
353	if lines >= 5000 {	588	if lines >= 5000 {
354	return true; // stop appending but don't abort libgit2	589	return true;
355	}	590	}
356	let prefix = match line.origin() {	591	let prefix = match line.origin() {
357	'+' => "+",	592	'+' => "+",
@@ -374,6 +609,65 @@ pub fn generate_diff(repo: &Repository, patch: &state::PatchState) -> Result<Str
374	Ok(output)	609	Ok(output)
375	}	610	}
376		611
		612	/// Patch log: list all revisions with timestamps and file-change summaries.
		613	pub fn patch_log(
		614	repo: &Repository,
		615	id_prefix: &str,
		616	) -> Result<PatchState, Error> {
		617	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
		618	PatchState::from_ref(repo, &ref_name, &id)
		619	}
		620
		621	pub fn patch_log_to_writer(
		622	repo: &Repository,
		623	patch: &PatchState,
		624	writer: &mut dyn std::io::Write,
		625	) -> Result<(), Error> {
		626	if patch.revisions.is_empty() {
		627	writeln!(writer, "No revisions recorded.")?;
		628	return Ok(());
		629	}
		630
		631	for (i, rev) in patch.revisions.iter().enumerate() {
		632	let short_oid = if rev.commit.len() >= 8 { &rev.commit[..8] } else { &rev.commit };
		633	let label = if i == 0 { " (initial)" } else { "" };
		634	let body_display = rev.body.as_deref().map(|b| format!(" \"{}\"", b)).unwrap_or_default();
		635
		636	// Compute file-change summary between consecutive revisions
		637	let file_summary = if i > 0 {
		638	let prev = &patch.revisions[i - 1];
		639	match (Oid::from_str(&prev.tree), Oid::from_str(&rev.tree)) {
		640	(Ok(from_oid), Ok(to_oid)) => {
		641	if let (Ok(from_tree), Ok(to_tree)) = (repo.find_tree(from_oid), repo.find_tree(to_oid)) {
		642	if let Ok(diff) = repo.diff_tree_to_tree(Some(&from_tree), Some(&to_tree), None) {
		643	let stats = diff.stats().ok();
		644	stats.map(|s| format!(" {} file(s) changed, +{} -{}", s.files_changed(), s.insertions(), s.deletions())).unwrap_or_default()
		645	} else {
		646	String::new()
		647	}
		648	} else {
		649	String::new()
		650	}
		651	}
		652	_ => String::new(),
		653	}
		654	} else {
		655	String::new()
		656	};
		657
		658	writeln!(
		659	writer,
		660	"r{} {} {}{}{}{}",
		661	rev.number, short_oid, rev.timestamp, label, file_summary, body_display
		662	)?;
		663	}
		664	Ok(())
		665	}
		666
		667	pub fn patch_log_json(patch: &PatchState) -> Result<String, Error> {
		668	Ok(serde_json::to_string_pretty(&patch.revisions)?)
		669	}
		670
377	pub fn delete(repo: &Repository, id_prefix: &str) -> Result<String, crate::error::Error> {	671	pub fn delete(repo: &Repository, id_prefix: &str) -> Result<String, crate::error::Error> {
378	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;	672	let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
379	repo.find_reference(&ref_name)?.delete()?;	673	repo.find_reference(&ref_name)?.delete()?;
@@ -404,3 +698,24 @@ pub fn close(
404	Ok(())	698	Ok(())
405	}	699	}
406		700
		701	/// Collab config (from refs/collab/config).
		702	#[derive(Default)]
		703	struct CollabConfig {
		704	require_approval_on_latest: bool,
		705	}
		706
		707	fn read_collab_config(repo: &Repository) -> CollabConfig {
		708	(|| -> Option<CollabConfig> {
		709	let tip = repo.refname_to_id("refs/collab/config").ok()?;
		710	let tree = repo.find_commit(tip).ok()?.tree().ok()?;
		711	let blob = repo.find_blob(tree.get_name("config.json")?.id()).ok()?;
		712	let val: serde_json::Value = serde_json::from_slice(blob.content()).ok()?;
		713	Some(CollabConfig {
		714	require_approval_on_latest: val
		715	.pointer("/merge/require_approval_on_latest")
		716	.and_then(|v| v.as_bool())
		717	.unwrap_or(false),
		718	})
		719	})()
		720	.unwrap_or_default()
		721	}

src/state.rs

Old		New
@@ -34,6 +34,7 @@ fn serialize_verdict<S: serde::Serializer>(v: &ReviewVerdict, s: S) -> Result<S:
34	ReviewVerdict::Approve => "approve",	34	ReviewVerdict::Approve => "approve",
35	ReviewVerdict::RequestChanges => "request-changes",	35	ReviewVerdict::RequestChanges => "request-changes",
36	ReviewVerdict::Comment => "comment",	36	ReviewVerdict::Comment => "comment",
		37	ReviewVerdict::Reject => "reject",
37	};	38	};
38	s.serialize_str(str_val)	39	s.serialize_str(str_val)
39	}	40	}
@@ -44,6 +45,7 @@ fn deserialize_verdict<'de, D: serde::Deserializer<'de>>(d: D) -> Result<ReviewV
44	"approve" => Ok(ReviewVerdict::Approve),	45	"approve" => Ok(ReviewVerdict::Approve),
45	"request-changes" => Ok(ReviewVerdict::RequestChanges),	46	"request-changes" => Ok(ReviewVerdict::RequestChanges),
46	"comment" => Ok(ReviewVerdict::Comment),	47	"comment" => Ok(ReviewVerdict::Comment),
		48	"reject" => Ok(ReviewVerdict::Reject),
47	other => Err(serde::de::Error::custom(format!(	49	other => Err(serde::de::Error::custom(format!(
48	"unknown verdict: {}",	50	"unknown verdict: {}",
49	other	51	other
@@ -98,6 +100,8 @@ pub struct Review {
98	pub verdict: ReviewVerdict,	100	pub verdict: ReviewVerdict,
99	pub body: String,	101	pub body: String,
100	pub timestamp: String,	102	pub timestamp: String,
		103	#[serde(default)]
		104	pub revision: Option<u32>,
101	}	105	}
102		106
103	#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]	107	#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
@@ -110,12 +114,23 @@ pub enum PatchStatus {
110	}	114	}
111		115
112	#[derive(Debug, Clone, Serialize, Deserialize)]	116	#[derive(Debug, Clone, Serialize, Deserialize)]
		117	pub struct Revision {
		118	pub number: u32,
		119	pub commit: String,
		120	pub tree: String,
		121	pub body: Option<String>,
		122	pub timestamp: String,
		123	}
		124
		125	#[derive(Debug, Clone, Serialize, Deserialize)]
113	pub struct InlineComment {	126	pub struct InlineComment {
114	pub author: Author,	127	pub author: Author,
115	pub file: String,	128	pub file: String,
116	pub line: u32,	129	pub line: u32,
117	pub body: String,	130	pub body: String,
118	pub timestamp: String,	131	pub timestamp: String,
		132	#[serde(default)]
		133	pub revision: Option<u32>,
119	}	134	}
120		135
121	#[derive(Debug, Clone, Serialize, Deserialize)]	136	#[derive(Debug, Clone, Serialize, Deserialize)]
@@ -130,6 +145,7 @@ pub struct PatchState {
130	pub comments: Vec<Comment>,	145	pub comments: Vec<Comment>,
131	pub inline_comments: Vec<InlineComment>,	146	pub inline_comments: Vec<InlineComment>,
132	pub reviews: Vec<Review>,	147	pub reviews: Vec<Review>,
		148	pub revisions: Vec<Revision>,
133	pub created_at: String,	149	pub created_at: String,
134	#[serde(default)]	150	#[serde(default)]
135	pub last_updated: String,	151	pub last_updated: String,
@@ -344,7 +360,16 @@ impl PatchState {
344	base_ref,	360	base_ref,
345	branch,	361	branch,
346	fixes,	362	fixes,
		363	commit,
		364	tree,
347	} => {	365	} => {
		366	let revisions = vec![Revision {
		367	number: 1,
		368	commit: commit.clone(),
		369	tree,
		370	body: None,
		371	timestamp: event.timestamp.clone(),
		372	}];
348	state = Some(PatchState {	373	state = Some(PatchState {
349	id: id.to_string(),	374	id: id.to_string(),
350	title,	375	title,
@@ -356,25 +381,36 @@ impl PatchState {
356	comments: Vec::new(),	381	comments: Vec::new(),
357	inline_comments: Vec::new(),	382	inline_comments: Vec::new(),
358	reviews: Vec::new(),	383	reviews: Vec::new(),
		384	revisions,
359	created_at: event.timestamp.clone(),	385	created_at: event.timestamp.clone(),
360	last_updated: String::new(),	386	last_updated: String::new(),
361	author: event.author.clone(),	387	author: event.author.clone(),
362	});	388	});
363	}	389	}
364	Action::PatchRevise { body } => {	390	Action::PatchRevision { commit, tree, body } => {
365	if let Some(ref mut s) = state {	391	if let Some(ref mut s) = state {
366	if let Some(b) = body {	392	// Dedup by commit OID — skip if already seen
367	s.body = b;	393	let already_seen = s.revisions.iter().any(|r| r.commit == commit);
		394	if !already_seen {
		395	let number = s.revisions.len() as u32 + 1;
		396	s.revisions.push(Revision {
		397	number,
		398	commit,
		399	tree,
		400	body,
		401	timestamp: event.timestamp.clone(),
		402	});
368	}	403	}
369	}	404	}
370	}	405	}
371	Action::PatchReview { verdict, body } => {	406	Action::PatchReview { verdict, body, revision } => {
372	if let Some(ref mut s) = state {	407	if let Some(ref mut s) = state {
373	s.reviews.push(Review {	408	s.reviews.push(Review {
374	author: event.author.clone(),	409	author: event.author.clone(),
375	verdict,	410	verdict,
376	body,	411	body,
377	timestamp: event.timestamp.clone(),	412	timestamp: event.timestamp.clone(),
		413	revision: Some(revision),
378	});	414	});
379	}	415	}
380	}	416	}
@@ -388,7 +424,7 @@ impl PatchState {
388	});	424	});
389	}	425	}
390	}	426	}
391	Action::PatchInlineComment { file, line, body } => {	427	Action::PatchInlineComment { file, line, body, revision } => {
392	if let Some(ref mut s) = state {	428	if let Some(ref mut s) = state {
393	s.inline_comments.push(InlineComment {	429	s.inline_comments.push(InlineComment {
394	author: event.author.clone(),	430	author: event.author.clone(),
@@ -396,6 +432,7 @@ impl PatchState {
396	line,	432	line,
397	body,	433	body,
398	timestamp: event.timestamp.clone(),	434	timestamp: event.timestamp.clone(),
		435	revision: Some(revision),
399	});	436	});
400	}	437	}
401	}	438	}

src/status.rs

Old		New
@@ -68,6 +68,7 @@ pub fn compute(repo: &Repository) -> Result<ProjectStatus, Error> {
68	ReviewVerdict::Approve => patches_approved += 1,	68	ReviewVerdict::Approve => patches_approved += 1,
69	ReviewVerdict::RequestChanges => patches_changes_requested += 1,	69	ReviewVerdict::RequestChanges => patches_changes_requested += 1,
70	ReviewVerdict::Comment => {}	70	ReviewVerdict::Comment => {}
		71	ReviewVerdict::Reject => {}
71	}	72	}
72	}	73	}
73	}	74	}

src/tui/events.rs

Old		New
@@ -10,6 +10,7 @@ use ratatui::widgets::ListState;
10		10
11	use crate::error::Error;	11	use crate::error::Error;
12	use crate::issue as issue_mod;	12	use crate::issue as issue_mod;
		13	use crate::patch as patch_mod;
13		14
14	use super::state::{App, InputMode, KeyAction, ViewMode};	15	use super::state::{App, InputMode, KeyAction, ViewMode};
15	use super::widgets::ui;	16	use super::widgets::ui;
@@ -197,7 +198,30 @@ pub(crate) fn run_loop(
197		198
198	match app.handle_key(key.code, key.modifiers) {	199	match app.handle_key(key.code, key.modifiers) {
199	KeyAction::Quit => return Ok(()),	200	KeyAction::Quit => return Ok(()),
		201	KeyAction::Reload if app.mode == ViewMode::PatchDetail => {
		202	// Regenerate diff for current revision/interdiff state
		203	regenerate_patch_diff(app, repo);
		204	}
200	KeyAction::Reload => app.reload(repo),	205	KeyAction::Reload => app.reload(repo),
		206	KeyAction::OpenPatchDetail => {
		207	if let Some(patch) = app.linked_patch_for_selected().cloned() {
		208	// Check staleness
		209	let warning = crate::staleness_warning(repo, &patch);
		210	app.patch_revision_idx = patch.revisions.len().saturating_sub(1);
		211	app.patch_interdiff_mode = false;
		212	app.patch_scroll = 0;
		213
		214	// Generate initial diff (latest revision vs base)
		215	let diff = generate_patch_diff_for(repo, &patch, app.patch_revision_idx, false);
		216	app.patch_diff = diff;
		217	app.current_patch = Some(patch);
		218	app.mode = ViewMode::PatchDetail;
		219
		220	if let Some(w) = warning {
		221	app.status_msg = Some(w);
		222	}
		223	}
		224	}
201	KeyAction::OpenCommitBrowser => {	225	KeyAction::OpenCommitBrowser => {
202	if let Some(ref_name) = app.selected_ref_name() {	226	if let Some(ref_name) = app.selected_ref_name() {
203	match crate::dag::walk_events(repo, &ref_name) {	227	match crate::dag::walk_events(repo, &ref_name) {
@@ -223,3 +247,43 @@ pub(crate) fn run_loop(
223	}	247	}
224	}	248	}
225	}	249	}
		250
		251	fn generate_patch_diff_for(
		252	repo: &Repository,
		253	patch: &crate::state::PatchState,
		254	rev_idx: usize,
		255	interdiff_mode: bool,
		256	) -> String {
		257	if patch.revisions.is_empty() {
		258	return "(no revisions)".to_string();
		259	}
		260
		261	let rev = &patch.revisions[rev_idx];
		262
		263	if interdiff_mode && rev_idx > 0 {
		264	let from_rev = patch.revisions[rev_idx - 1].number;
		265	let to_rev = rev.number;
		266	match patch_mod::interdiff(repo, patch, from_rev, to_rev) {
		267	Ok(d) => d,
		268	Err(e) => format!("(error generating interdiff: {})", e),
		269	}
		270	} else {
		271	// Diff at specific revision vs base
		272	match patch_mod::diff(repo, &patch.id, Some(rev.number), None) {
		273	Ok(d) => d,
		274	Err(e) => format!("(error generating diff: {})", e),
		275	}
		276	}
		277	}
		278
		279	fn regenerate_patch_diff(app: &mut App, repo: &Repository) {
		280	if let Some(ref patch) = app.current_patch {
		281	let diff = generate_patch_diff_for(
		282	repo,
		283	patch,
		284	app.patch_revision_idx,
		285	app.patch_interdiff_mode,
		286	);
		287	app.patch_diff = diff;
		288	}
		289	}

src/tui/mod.rs

Old		New
@@ -82,6 +82,7 @@ mod tests {
82	comments: vec![],	82	comments: vec![],
83	inline_comments: vec![],	83	inline_comments: vec![],
84	reviews: vec![],	84	reviews: vec![],
		85	revisions: vec![],
85	created_at: String::new(),	86	created_at: String::new(),
86	last_updated: String::new(),	87	last_updated: String::new(),
87	author: make_author(),	88	author: make_author(),
@@ -276,6 +277,7 @@ mod tests {
276	comments: Vec::new(),	277	comments: Vec::new(),
277	inline_comments: Vec::new(),	278	inline_comments: Vec::new(),
278	reviews: Vec::new(),	279	reviews: Vec::new(),
		280	revisions: Vec::new(),
279	created_at: "2026-01-01T00:00:00Z".to_string(),	281	created_at: "2026-01-01T00:00:00Z".to_string(),
280	last_updated: "2026-01-01T00:00:00Z".to_string(),	282	last_updated: "2026-01-01T00:00:00Z".to_string(),
281	author: test_author(),	283	author: test_author(),
@@ -404,6 +406,8 @@ mod tests {
404	base_ref: "main".to_string(),	406	base_ref: "main".to_string(),
405	branch: "feature/test".to_string(),	407	branch: "feature/test".to_string(),
406	fixes: None,	408	fixes: None,
		409	commit: "abc123".to_string(),
		410	tree: "def456".to_string(),
407	};	411	};
408	assert_eq!(action_type_label(&action), "Patch Create");	412	assert_eq!(action_type_label(&action), "Patch Create");
409	}	413	}
@@ -413,6 +417,7 @@ mod tests {
413	let action = Action::PatchReview {	417	let action = Action::PatchReview {
414	verdict: ReviewVerdict::Approve,	418	verdict: ReviewVerdict::Approve,
415	body: "lgtm".to_string(),	419	body: "lgtm".to_string(),
		420	revision: 1,
416	};	421	};
417	assert_eq!(action_type_label(&action), "Patch Review");	422	assert_eq!(action_type_label(&action), "Patch Review");
418	}	423	}
@@ -423,6 +428,7 @@ mod tests {
423	file: "src/main.rs".to_string(),	428	file: "src/main.rs".to_string(),
424	line: 42,	429	line: 42,
425	body: "nit".to_string(),	430	body: "nit".to_string(),
		431	revision: 1,
426	};	432	};
427	assert_eq!(action_type_label(&action), "Inline Comment");	433	assert_eq!(action_type_label(&action), "Inline Comment");
428	}	434	}
@@ -481,6 +487,7 @@ mod tests {
481	action: Action::PatchReview {	487	action: Action::PatchReview {
482	verdict: ReviewVerdict::Approve,	488	verdict: ReviewVerdict::Approve,
483	body: "Looks good!".to_string(),	489	body: "Looks good!".to_string(),
		490	revision: 1,
484	},	491	},
485	clock: 0,	492	clock: 0,
486	};	493	};
@@ -1012,4 +1019,368 @@ mod tests {
1012	let buf = render_app(&mut app);	1019	let buf = render_app(&mut app);
1013	assert_buffer_contains(&buf, "Error loading events");	1020	assert_buffer_contains(&buf, "Error loading events");
1014	}	1021	}
		1022
		1023	// ── Patch detail view tests ─────────────────────────────────────────
		1024
		1025	fn make_patch_with_revisions() -> PatchState {
		1026	use crate::state::Revision;
		1027	PatchState {
		1028	id: "deadbeef".into(),
		1029	title: "Fix the thing".into(),
		1030	body: "Detailed description".into(),
		1031	status: PatchStatus::Open,
		1032	base_ref: "main".into(),
		1033	fixes: Some("i1".into()),
		1034	branch: "feature/fix-thing".into(),
		1035	comments: vec![crate::state::Comment {
		1036	author: make_author(),
		1037	body: "Thread comment".into(),
		1038	timestamp: "2026-01-05T00:00:00Z".into(),
		1039	commit_id: Oid::from_str("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").unwrap(),
		1040	}],
		1041	inline_comments: vec![crate::state::InlineComment {
		1042	author: make_author(),
		1043	file: "src/main.rs".into(),
		1044	line: 42,
		1045	body: "Nit: rename this".into(),
		1046	timestamp: "2026-01-03T00:00:00Z".into(),
		1047	revision: Some(1),
		1048	}],
		1049	reviews: vec![crate::state::Review {
		1050	author: make_author(),
		1051	verdict: ReviewVerdict::Approve,
		1052	body: "LGTM".into(),
		1053	timestamp: "2026-01-04T00:00:00Z".into(),
		1054	revision: Some(2),
		1055	}],
		1056	revisions: vec![
		1057	Revision {
		1058	number: 1,
		1059	commit: "aaaa1111aaaa1111aaaa1111aaaa1111aaaa1111".into(),
		1060	tree: "bbbb1111bbbb1111bbbb1111bbbb1111bbbb1111".into(),
		1061	body: None,
		1062	timestamp: "2026-01-01T00:00:00Z".into(),
		1063	},
		1064	Revision {
		1065	number: 2,
		1066	commit: "aaaa2222aaaa2222aaaa2222aaaa2222aaaa2222".into(),
		1067	tree: "bbbb2222bbbb2222bbbb2222bbbb2222bbbb2222".into(),
		1068	body: Some("Addressed review comments".into()),
		1069	timestamp: "2026-01-02T00:00:00Z".into(),
		1070	},
		1071	],
		1072	created_at: "2026-01-01T00:00:00Z".into(),
		1073	last_updated: "2026-01-04T00:00:00Z".into(),
		1074	author: make_author(),
		1075	}
		1076	}
		1077
		1078	#[test]
		1079	fn test_p_key_opens_patch_detail_when_linked_patch_exists() {
		1080	let mut app = test_app();
		1081	// Issue i1 has a linked patch (p1 has fixes=None, so we need to set it)
		1082	app.patches[0].fixes = Some("i1".into());
		1083	app.pane = Pane::Detail;
		1084	app.list_state.select(Some(0)); // selects issue i1
		1085
		1086	let result = app.handle_key(
		1087	crossterm::event::KeyCode::Char('p'),
		1088	crossterm::event::KeyModifiers::empty(),
		1089	);
		1090	assert_eq!(result, KeyAction::OpenPatchDetail);
		1091	}
		1092
		1093	#[test]
		1094	fn test_p_key_noop_when_no_linked_patch() {
		1095	let mut app = test_app();
		1096	// No patches fix any issues by default
		1097	app.pane = Pane::Detail;
		1098	app.list_state.select(Some(0));
		1099
		1100	let result = app.handle_key(
		1101	crossterm::event::KeyCode::Char('p'),
		1102	crossterm::event::KeyModifiers::empty(),
		1103	);
		1104	assert_eq!(result, KeyAction::Continue);
		1105	}
		1106
		1107	#[test]
		1108	fn test_p_key_noop_in_item_list_pane() {
		1109	let mut app = test_app();
		1110	app.patches[0].fixes = Some("i1".into());
		1111	app.pane = Pane::ItemList;
		1112	app.list_state.select(Some(0));
		1113
		1114	let result = app.handle_key(
		1115	crossterm::event::KeyCode::Char('p'),
		1116	crossterm::event::KeyModifiers::empty(),
		1117	);
		1118	assert_eq!(result, KeyAction::Continue);
		1119	}
		1120
		1121	#[test]
		1122	fn test_patch_detail_esc_returns_to_details() {
		1123	let mut app = test_app();
		1124	app.current_patch = Some(make_patch_with_revisions());
		1125	app.mode = ViewMode::PatchDetail;
		1126	app.patch_scroll = 5;
		1127	app.patch_revision_idx = 1;
		1128	app.patch_interdiff_mode = true;
		1129	app.patch_diff = "some diff".into();
		1130
		1131	let result = app.handle_key(
		1132	crossterm::event::KeyCode::Esc,
		1133	crossterm::event::KeyModifiers::empty(),
		1134	);
		1135	assert_eq!(result, KeyAction::Continue);
		1136	assert_eq!(app.mode, ViewMode::Details);
		1137	assert!(app.current_patch.is_none());
		1138	assert!(app.patch_diff.is_empty());
		1139	assert_eq!(app.patch_scroll, 0);
		1140	assert_eq!(app.patch_revision_idx, 0);
		1141	assert!(!app.patch_interdiff_mode);
		1142	}
		1143
		1144	#[test]
		1145	fn test_patch_detail_q_quits() {
		1146	let mut app = test_app();
		1147	app.mode = ViewMode::PatchDetail;
		1148	let result = app.handle_key(
		1149	crossterm::event::KeyCode::Char('q'),
		1150	crossterm::event::KeyModifiers::empty(),
		1151	);
		1152	assert_eq!(result, KeyAction::Quit);
		1153	}
		1154
		1155	#[test]
		1156	fn test_patch_detail_scroll() {
		1157	let mut app = test_app();
		1158	app.mode = ViewMode::PatchDetail;
		1159	app.current_patch = Some(make_patch_with_revisions());
		1160	app.patch_scroll = 0;
		1161
		1162	app.handle_key(
		1163	crossterm::event::KeyCode::Char('j'),
		1164	crossterm::event::KeyModifiers::empty(),
		1165	);
		1166	assert_eq!(app.patch_scroll, 1);
		1167	app.handle_key(
		1168	crossterm::event::KeyCode::Char('j'),
		1169	crossterm::event::KeyModifiers::empty(),
		1170	);
		1171	assert_eq!(app.patch_scroll, 2);
		1172	app.handle_key(
		1173	crossterm::event::KeyCode::Char('k'),
		1174	crossterm::event::KeyModifiers::empty(),
		1175	);
		1176	assert_eq!(app.patch_scroll, 1);
		1177	}
		1178
		1179	#[test]
		1180	fn test_patch_detail_page_scroll() {
		1181	let mut app = test_app();
		1182	app.mode = ViewMode::PatchDetail;
		1183	app.patch_scroll = 0;
		1184
		1185	app.handle_key(
		1186	crossterm::event::KeyCode::PageDown,
		1187	crossterm::event::KeyModifiers::empty(),
		1188	);
		1189	assert_eq!(app.patch_scroll, 20);
		1190	app.handle_key(
		1191	crossterm::event::KeyCode::PageUp,
		1192	crossterm::event::KeyModifiers::empty(),
		1193	);
		1194	assert_eq!(app.patch_scroll, 0);
		1195	}
		1196
		1197	#[test]
		1198	fn test_patch_detail_revision_navigation() {
		1199	let mut app = test_app();
		1200	app.mode = ViewMode::PatchDetail;
		1201	app.current_patch = Some(make_patch_with_revisions());
		1202	app.patch_revision_idx = 0;
		1203
		1204	// Navigate forward
		1205	let result = app.handle_key(
		1206	crossterm::event::KeyCode::Char(']'),
		1207	crossterm::event::KeyModifiers::empty(),
		1208	);
		1209	assert_eq!(result, KeyAction::Reload);
		1210	assert_eq!(app.patch_revision_idx, 1);
		1211	assert_eq!(app.patch_scroll, 0);
		1212
		1213	// Can't go past last revision
		1214	let result = app.handle_key(
		1215	crossterm::event::KeyCode::Char(']'),
		1216	crossterm::event::KeyModifiers::empty(),
		1217	);
		1218	assert_eq!(result, KeyAction::Continue);
		1219	assert_eq!(app.patch_revision_idx, 1);
		1220
		1221	// Navigate backward
		1222	let result = app.handle_key(
		1223	crossterm::event::KeyCode::Char('['),
		1224	crossterm::event::KeyModifiers::empty(),
		1225	);
		1226	assert_eq!(result, KeyAction::Reload);
		1227	assert_eq!(app.patch_revision_idx, 0);
		1228
		1229	// Can't go below 0
		1230	let result = app.handle_key(
		1231	crossterm::event::KeyCode::Char('['),
		1232	crossterm::event::KeyModifiers::empty(),
		1233	);
		1234	assert_eq!(result, KeyAction::Continue);
		1235	assert_eq!(app.patch_revision_idx, 0);
		1236	}
		1237
		1238	#[test]
		1239	fn test_patch_detail_interdiff_toggle() {
		1240	let mut app = test_app();
		1241	app.mode = ViewMode::PatchDetail;
		1242	app.current_patch = Some(make_patch_with_revisions());
		1243	app.patch_interdiff_mode = false;
		1244
		1245	let result = app.handle_key(
		1246	crossterm::event::KeyCode::Char('d'),
		1247	crossterm::event::KeyModifiers::empty(),
		1248	);
		1249	assert_eq!(result, KeyAction::Reload);
		1250	assert!(app.patch_interdiff_mode);
		1251	assert_eq!(app.patch_scroll, 0);
		1252
		1253	let result = app.handle_key(
		1254	crossterm::event::KeyCode::Char('d'),
		1255	crossterm::event::KeyModifiers::empty(),
		1256	);
		1257	assert_eq!(result, KeyAction::Reload);
		1258	assert!(!app.patch_interdiff_mode);
		1259	}
		1260
		1261	#[test]
		1262	fn test_render_patch_detail() {
		1263	let mut app = make_app(3, 0);
		1264	app.mode = ViewMode::PatchDetail;
		1265	app.current_patch = Some(make_patch_with_revisions());
		1266	app.patch_revision_idx = 1;
		1267	app.patch_diff = "+added line\n-removed line\n context".into();
		1268
		1269	let buf = render_app(&mut app);
		1270	assert_buffer_contains(&buf, "Patch Detail");
		1271	assert_buffer_contains(&buf, "Fix the thing");
		1272	assert_buffer_contains(&buf, "deadbeef");
		1273	assert_buffer_contains(&buf, "feature/fix-thing");
		1274	}
		1275
		1276	#[test]
		1277	fn test_render_patch_detail_shows_reviews() {
		1278	let mut app = make_app(3, 0);
		1279	app.mode = ViewMode::PatchDetail;
		1280	app.current_patch = Some(make_patch_with_revisions());
		1281	app.patch_revision_idx = 1;
		1282	app.patch_diff = String::new();
		1283
		1284	let buf = render_app(&mut app);
		1285	assert_buffer_contains(&buf, "Reviews");
		1286	assert_buffer_contains(&buf, "approve");
		1287	assert_buffer_contains(&buf, "LGTM");
		1288	}
		1289
		1290	#[test]
		1291	fn test_render_patch_detail_footer() {
		1292	let mut app = make_app(3, 0);
		1293	app.mode = ViewMode::PatchDetail;
		1294	let buf = render_app(&mut app);
		1295	assert_buffer_contains(&buf, "Esc:back");
		1296	assert_buffer_contains(&buf, "[/]:revision");
		1297	// "d:interdiff" may be truncated at 80 cols, check prefix
		1298	assert_buffer_contains(&buf, "d:inter");
		1299	}
		1300
		1301	#[test]
		1302	fn test_render_patch_detail_no_patch_loaded() {
		1303	let mut app = make_app(3, 0);
		1304	app.mode = ViewMode::PatchDetail;
		1305	app.current_patch = None;
		1306
		1307	let buf = render_app(&mut app);
		1308	assert_buffer_contains(&buf, "No patch loaded");
		1309	}
		1310
		1311	#[test]
		1312	fn test_patch_detail_ctrl_c_quits() {
		1313	let mut app = test_app();
		1314	app.mode = ViewMode::PatchDetail;
		1315	let result = app.handle_key(
		1316	crossterm::event::KeyCode::Char('c'),
		1317	crossterm::event::KeyModifiers::CONTROL,
		1318	);
		1319	assert_eq!(result, KeyAction::Quit);
		1320	}
		1321
		1322	#[test]
		1323	fn test_full_patch_detail_flow() {
		1324	let mut app = test_app();
		1325	app.patches[0].fixes = Some("i1".into());
		1326	app.pane = Pane::Detail;
		1327	app.list_state.select(Some(0));
		1328
		1329	// Press p to open patch detail
		1330	let action = app.handle_key(
		1331	crossterm::event::KeyCode::Char('p'),
		1332	crossterm::event::KeyModifiers::empty(),
		1333	);
		1334	assert_eq!(action, KeyAction::OpenPatchDetail);
		1335
		1336	// Simulate what events.rs does
		1337	app.current_patch = Some(make_patch_with_revisions());
		1338	app.patch_revision_idx = 1;
		1339	app.patch_diff = "diff content".into();
		1340	app.mode = ViewMode::PatchDetail;
		1341
		1342	// Navigate revisions
		1343	app.handle_key(
		1344	crossterm::event::KeyCode::Char('['),
		1345	crossterm::event::KeyModifiers::empty(),
		1346	);
		1347	assert_eq!(app.patch_revision_idx, 0);
		1348
		1349	// Toggle interdiff
		1350	app.handle_key(
		1351	crossterm::event::KeyCode::Char('d'),
		1352	crossterm::event::KeyModifiers::empty(),
		1353	);
		1354	assert!(app.patch_interdiff_mode);
		1355
		1356	// Scroll
		1357	app.handle_key(
		1358	crossterm::event::KeyCode::Char('j'),
		1359	crossterm::event::KeyModifiers::empty(),
		1360	);
		1361	assert_eq!(app.patch_scroll, 1);
		1362
		1363	// Escape back
		1364	app.handle_key(
		1365	crossterm::event::KeyCode::Esc,
		1366	crossterm::event::KeyModifiers::empty(),
		1367	);
		1368	assert_eq!(app.mode, ViewMode::Details);
		1369	assert!(app.current_patch.is_none());
		1370	}
		1371
		1372	#[test]
		1373	fn test_linked_patch_for_selected() {
		1374	let mut app = test_app();
		1375	app.patches[0].fixes = Some("i1".into());
		1376	app.list_state.select(Some(0));
		1377
		1378	let patch = app.linked_patch_for_selected();
		1379	assert!(patch.is_some());
		1380	assert_eq!(patch.unwrap().title, "Login fix patch");
		1381
		1382	// Second issue has no linked patch
		1383	app.list_state.select(Some(1));
		1384	assert!(app.linked_patch_for_selected().is_none());
		1385	}
1015	}	1386	}

src/tui/state.rs

Old		New
@@ -15,6 +15,7 @@ pub(crate) enum ViewMode {
15	Details,	15	Details,
16	CommitList,	16	CommitList,
17	CommitDetail,	17	CommitDetail,
		18	PatchDetail,
18	}	19	}
19		20
20	#[derive(Debug, PartialEq)]	21	#[derive(Debug, PartialEq)]
@@ -23,6 +24,7 @@ pub(crate) enum KeyAction {
23	Quit,	24	Quit,
24	Reload,	25	Reload,
25	OpenCommitBrowser,	26	OpenCommitBrowser,
		27	OpenPatchDetail,
26	}	28	}
27		29
28	#[derive(Debug, PartialEq, Clone, Copy)]	30	#[derive(Debug, PartialEq, Clone, Copy)]
@@ -73,6 +75,12 @@ pub(crate) struct App {
73	pub(crate) status_msg: Option<String>,	75	pub(crate) status_msg: Option<String>,
74	pub(crate) event_history: Vec<(Oid, crate::event::Event)>,	76	pub(crate) event_history: Vec<(Oid, crate::event::Event)>,
75	pub(crate) event_list_state: ListState,	77	pub(crate) event_list_state: ListState,
		78	// Patch detail view state
		79	pub(crate) current_patch: Option<PatchState>,
		80	pub(crate) patch_diff: String,
		81	pub(crate) patch_scroll: u16,
		82	pub(crate) patch_revision_idx: usize,
		83	pub(crate) patch_interdiff_mode: bool,
76	}	84	}
77		85
78	impl App {	86	impl App {
@@ -96,6 +104,11 @@ impl App {
96	status_msg: None,	104	status_msg: None,
97	event_history: Vec::new(),	105	event_history: Vec::new(),
98	event_list_state: ListState::default(),	106	event_list_state: ListState::default(),
		107	current_patch: None,
		108	patch_diff: String::new(),
		109	patch_scroll: 0,
		110	patch_revision_idx: 0,
		111	patch_interdiff_mode: false,
99	}	112	}
100	}	113	}
101		114
@@ -139,7 +152,77 @@ impl App {
139	self.scroll = 0;	152	self.scroll = 0;
140	}	153	}
141		154
		155	/// Find the first linked patch for the currently selected issue.
		156	pub(crate) fn linked_patch_for_selected(&self) -> Option<&PatchState> {
		157	let idx = self.list_state.selected()?;
		158	let visible = self.visible_issues();
		159	let issue = visible.get(idx)?;
		160	self.patches
		161	.iter()
		162	.find(|p| p.fixes.as_deref() == Some(&issue.id))
		163	}
		164
142	pub(crate) fn handle_key(&mut self, code: KeyCode, modifiers: KeyModifiers) -> KeyAction {	165	pub(crate) fn handle_key(&mut self, code: KeyCode, modifiers: KeyModifiers) -> KeyAction {
		166	// Handle PatchDetail mode
		167	if self.mode == ViewMode::PatchDetail {
		168	match code {
		169	KeyCode::Esc => {
		170	self.mode = ViewMode::Details;
		171	self.current_patch = None;
		172	self.patch_diff.clear();
		173	self.patch_scroll = 0;
		174	self.patch_revision_idx = 0;
		175	self.patch_interdiff_mode = false;
		176	return KeyAction::Continue;
		177	}
		178	KeyCode::Char('q') => return KeyAction::Quit,
		179	KeyCode::Char('c') if modifiers.contains(KeyModifiers::CONTROL) => {
		180	return KeyAction::Quit;
		181	}
		182	KeyCode::Char('j') | KeyCode::Down => {
		183	self.patch_scroll = self.patch_scroll.saturating_add(1);
		184	return KeyAction::Continue;
		185	}
		186	KeyCode::Char('k') | KeyCode::Up => {
		187	self.patch_scroll = self.patch_scroll.saturating_sub(1);
		188	return KeyAction::Continue;
		189	}
		190	KeyCode::PageDown => {
		191	self.patch_scroll = self.patch_scroll.saturating_add(20);
		192	return KeyAction::Continue;
		193	}
		194	KeyCode::PageUp => {
		195	self.patch_scroll = self.patch_scroll.saturating_sub(20);
		196	return KeyAction::Continue;
		197	}
		198	KeyCode::Char(']') => {
		199	if let Some(ref patch) = self.current_patch {
		200	let max = patch.revisions.len().saturating_sub(1);
		201	if self.patch_revision_idx < max {
		202	self.patch_revision_idx += 1;
		203	self.patch_scroll = 0;
		204	return KeyAction::Reload; // signal to regenerate diff
		205	}
		206	}
		207	return KeyAction::Continue;
		208	}
		209	KeyCode::Char('[') => {
		210	if self.patch_revision_idx > 0 {
		211	self.patch_revision_idx -= 1;
		212	self.patch_scroll = 0;
		213	return KeyAction::Reload; // signal to regenerate diff
		214	}
		215	return KeyAction::Continue;
		216	}
		217	KeyCode::Char('d') => {
		218	self.patch_interdiff_mode = !self.patch_interdiff_mode;
		219	self.patch_scroll = 0;
		220	return KeyAction::Reload; // signal to regenerate diff
		221	}
		222	_ => return KeyAction::Continue,
		223	}
		224	}
		225
143	// Handle CommitDetail mode first	226	// Handle CommitDetail mode first
144	if self.mode == ViewMode::CommitDetail {	227	if self.mode == ViewMode::CommitDetail {
145	match code {	228	match code {
@@ -226,6 +309,14 @@ impl App {
226	KeyAction::Continue	309	KeyAction::Continue
227	}	310	}
228	}	311	}
		312	KeyCode::Char('p') => {
		313	// Open patch detail: only when in detail pane with a linked patch
		314	if self.pane == Pane::Detail && self.linked_patch_for_selected().is_some() {
		315	KeyAction::OpenPatchDetail
		316	} else {
		317	KeyAction::Continue
		318	}
		319	}
229	KeyCode::Char('j') | KeyCode::Down => {	320	KeyCode::Char('j') | KeyCode::Down => {
230	if self.pane == Pane::ItemList {	321	if self.pane == Pane::ItemList {
231	self.move_selection(1);	322	self.move_selection(1);

src/tui/widgets.rs

Old		New
@@ -2,7 +2,7 @@ use git2::Oid;
2	use ratatui::prelude::*;	2	use ratatui::prelude::*;
3	use ratatui::widgets::{Block, Borders, List, ListItem, Paragraph, Wrap};	3	use ratatui::widgets::{Block, Borders, List, ListItem, Paragraph, Wrap};
4		4
5	use crate::event::Action;	5	use crate::event::{Action, ReviewVerdict};
6	use crate::state::{IssueState, IssueStatus, PatchState, PatchStatus};	6	use crate::state::{IssueState, IssueStatus, PatchState, PatchStatus};
7		7
8	use super::state::{App, InputMode, Pane, StatusFilter, ViewMode};	8	use super::state::{App, InputMode, Pane, StatusFilter, ViewMode};
@@ -14,7 +14,7 @@ pub(crate) fn action_type_label(action: &Action) -> &str {
14	Action::IssueClose { .. } => "Issue Close",	14	Action::IssueClose { .. } => "Issue Close",
15	Action::IssueReopen => "Issue Reopen",	15	Action::IssueReopen => "Issue Reopen",
16	Action::PatchCreate { .. } => "Patch Create",	16	Action::PatchCreate { .. } => "Patch Create",
17	Action::PatchRevise { .. } => "Patch Revise",	17	Action::PatchRevision { .. } => "Patch Revision",
18	Action::PatchReview { .. } => "Patch Review",	18	Action::PatchReview { .. } => "Patch Review",
19	Action::PatchComment { .. } => "Patch Comment",	19	Action::PatchComment { .. } => "Patch Comment",
20	Action::PatchInlineComment { .. } => "Inline Comment",	20	Action::PatchInlineComment { .. } => "Inline Comment",
@@ -68,20 +68,22 @@ pub(crate) fn format_event_detail(oid: &Oid, event: &crate::event::Event) -> Str
68	detail.push_str(&format!("\n{}\n", body));	68	detail.push_str(&format!("\n{}\n", body));
69	}	69	}
70	}	70	}
71	Action::PatchRevise { body } => {	71	Action::PatchRevision { commit, tree, body } => {
		72	detail.push_str(&format!("\nCommit: {}\n", commit));
		73	detail.push_str(&format!("Tree: {}\n", tree));
72	if let Some(b) = body {	74	if let Some(b) = body {
73	if !b.is_empty() {	75	if !b.is_empty() {
74	detail.push_str(&format!("\n{}\n", b));	76	detail.push_str(&format!("\n{}\n", b));
75	}	77	}
76	}	78	}
77	}	79	}
78	Action::PatchReview { verdict, body } => {	80	Action::PatchReview { verdict, body, .. } => {
79	detail.push_str(&format!("\nVerdict: {:?}\n", verdict));	81	detail.push_str(&format!("\nVerdict: {:?}\n", verdict));
80	if !body.is_empty() {	82	if !body.is_empty() {
81	detail.push_str(&format!("\n{}\n", body));	83	detail.push_str(&format!("\n{}\n", body));
82	}	84	}
83	}	85	}
84	Action::PatchInlineComment { file, line, body } => {	86	Action::PatchInlineComment { file, line, body, .. } => {
85	detail.push_str(&format!("\nFile: {}:{}\n", file, line));	87	detail.push_str(&format!("\nFile: {}:{}\n", file, line));
86	if !body.is_empty() {	88	if !body.is_empty() {
87	detail.push_str(&format!("\n{}\n", body));	89	detail.push_str(&format!("\n{}\n", body));
@@ -186,6 +188,21 @@ fn render_detail(frame: &mut Frame, app: &mut App, area: Rect) {
186	Style::default().fg(Color::DarkGray)	188	Style::default().fg(Color::DarkGray)
187	};	189	};
188		190
		191	// Handle patch detail mode
		192	if app.mode == ViewMode::PatchDetail {
		193	let content = build_patch_detail_text(app);
		194	let block = Block::default()
		195	.borders(Borders::ALL)
		196	.title("Patch Detail")
		197	.border_style(border_style);
		198	let para = Paragraph::new(content)
		199	.block(block)
		200	.wrap(Wrap { trim: false })
		201	.scroll((app.patch_scroll, 0));
		202	frame.render_widget(para, area);
		203	return;
		204	}
		205
189	// Handle commit browser modes	206	// Handle commit browser modes
190	if app.mode == ViewMode::CommitList {	207	if app.mode == ViewMode::CommitList {
191	let items: Vec<ListItem> = app	208	let items: Vec<ListItem> = app
@@ -392,6 +409,245 @@ fn build_issue_detail(issue: &IssueState, patches: &[PatchState]) -> Text<'stati
392	Text::from(lines)	409	Text::from(lines)
393	}	410	}
394		411
		412	fn build_patch_detail_text(app: &App) -> Text<'static> {
		413	let patch = match &app.current_patch {
		414	Some(p) => p,
		415	None => return Text::raw("No patch loaded."),
		416	};
		417
		418	let status_str = match patch.status {
		419	PatchStatus::Open => "open",
		420	PatchStatus::Closed => "closed",
		421	PatchStatus::Merged => "merged",
		422	};
		423	let status_color = match patch.status {
		424	PatchStatus::Open => Color::Green,
		425	PatchStatus::Closed => Color::Red,
		426	PatchStatus::Merged => Color::Cyan,
		427	};
		428
		429	let mut lines: Vec<Line> = vec![
		430	Line::from(vec![
		431	Span::styled("Patch ", Style::default().add_modifier(Modifier::BOLD)),
		432	Span::styled(
		433	format!("{:.8}", patch.id),
		434	Style::default()
		435	.fg(Color::Yellow)
		436	.add_modifier(Modifier::BOLD),
		437	),
		438	Span::raw(" "),
		439	Span::styled(status_str, Style::default().fg(status_color)),
		440	]),
		441	Line::from(vec![
		442	Span::styled("Title: ", Style::default().fg(Color::DarkGray)),
		443	Span::raw(patch.title.clone()),
		444	]),
		445	Line::from(vec![
		446	Span::styled("Author: ", Style::default().fg(Color::DarkGray)),
		447	Span::raw(format!("{} <{}>", patch.author.name, patch.author.email)),
		448	]),
		449	Line::from(vec![
		450	Span::styled("Branch: ", Style::default().fg(Color::DarkGray)),
		451	Span::raw(patch.branch.clone()),
		452	]),
		453	Line::from(vec![
		454	Span::styled("Base: ", Style::default().fg(Color::DarkGray)),
		455	Span::raw(patch.base_ref.clone()),
		456	]),
		457	Line::from(vec![
		458	Span::styled("Revisions:", Style::default().fg(Color::DarkGray)),
		459	Span::raw(format!(" {}", patch.revisions.len())),
		460	]),
		461	];
		462
		463	if let Some(ref fixes) = patch.fixes {
		464	lines.push(Line::from(vec![
		465	Span::styled("Fixes: ", Style::default().fg(Color::DarkGray)),
		466	Span::raw(format!("{:.8}", fixes)),
		467	]));
		468	}
		469
		470	// Staleness warning (stored in status_msg or computed text)
		471	if let Some(ref warning) = app.status_msg {
		472	if warning.contains("behind") {
		473	lines.push(Line::raw(""));
		474	lines.push(Line::styled(
		475	warning.clone(),
		476	Style::default().fg(Color::Yellow).add_modifier(Modifier::BOLD),
		477	));
		478	}
		479	}
		480
		481	// Revision list
		482	if !patch.revisions.is_empty() {
		483	lines.push(Line::raw(""));
		484	lines.push(Line::styled(
		485	"--- Revisions ---",
		486	Style::default()
		487	.fg(Color::Magenta)
		488	.add_modifier(Modifier::BOLD),
		489	));
		490	for (i, rev) in patch.revisions.iter().enumerate() {
		491	let short = if rev.commit.len() >= 8 {
		492	&rev.commit[..8]
		493	} else {
		494	&rev.commit
		495	};
		496	let marker = if i == app.patch_revision_idx {
		497	"> "
		498	} else {
		499	" "
		500	};
		501	let label = if i == 0 { " (initial)" } else { "" };
		502	lines.push(Line::from(vec![
		503	Span::raw(format!("{}r{} {} {}{}", marker, rev.number, short, rev.timestamp, label)),
		504	]));
		505	}
		506	}
		507
		508	// Reviews
		509	if !patch.reviews.is_empty() {
		510	lines.push(Line::raw(""));
		511	lines.push(Line::styled(
		512	"--- Reviews ---",
		513	Style::default()
		514	.fg(Color::Blue)
		515	.add_modifier(Modifier::BOLD),
		516	));
		517	for review in &patch.reviews {
		518	let verdict_str = match review.verdict {
		519	ReviewVerdict::Approve => "approve",
		520	ReviewVerdict::RequestChanges => "request-changes",
		521	ReviewVerdict::Comment => "comment",
		522	ReviewVerdict::Reject => "reject",
		523	};
		524	let verdict_color = match review.verdict {
		525	ReviewVerdict::Approve => Color::Green,
		526	ReviewVerdict::RequestChanges => Color::Yellow,
		527	ReviewVerdict::Comment => Color::White,
		528	ReviewVerdict::Reject => Color::Red,
		529	};
		530	let rev_label = review
		531	.revision
		532	.map(|r| format!(" (r{})", r))
		533	.unwrap_or_default();
		534	lines.push(Line::from(vec![
		535	Span::styled(
		536	review.author.name.clone(),
		537	Style::default().add_modifier(Modifier::BOLD),
		538	),
		539	Span::raw(format!(" {} ", review.timestamp)),
		540	Span::styled(verdict_str, Style::default().fg(verdict_color)),
		541	Span::raw(rev_label),
		542	]));
		543	if !review.body.is_empty() {
		544	for l in review.body.lines() {
		545	lines.push(Line::raw(format!(" {}", l)));
		546	}
		547	}
		548	}
		549	}
		550
		551	// Inline comments
		552	if !patch.inline_comments.is_empty() {
		553	lines.push(Line::raw(""));
		554	lines.push(Line::styled(
		555	"--- Inline Comments ---",
		556	Style::default()
		557	.fg(Color::Blue)
		558	.add_modifier(Modifier::BOLD),
		559	));
		560	for ic in &patch.inline_comments {
		561	let rev_label = ic
		562	.revision
		563	.map(|r| format!(" (r{})", r))
		564	.unwrap_or_default();
		565	lines.push(Line::from(vec![
		566	Span::styled(
		567	ic.author.name.clone(),
		568	Style::default().add_modifier(Modifier::BOLD),
		569	),
		570	Span::raw(format!(" {}:{}", ic.file, ic.line)),
		571	Span::raw(rev_label),
		572	]));
		573	for l in ic.body.lines() {
		574	lines.push(Line::raw(format!(" {}", l)));
		575	}
		576	}
		577	}
		578
		579	// Thread comments
		580	if !patch.comments.is_empty() {
		581	lines.push(Line::raw(""));
		582	lines.push(Line::styled(
		583	"--- Comments ---",
		584	Style::default()
		585	.fg(Color::Blue)
		586	.add_modifier(Modifier::BOLD),
		587	));
		588	for c in &patch.comments {
		589	lines.push(Line::raw(""));
		590	lines.push(Line::from(vec![
		591	Span::styled(
		592	c.author.name.clone(),
		593	Style::default().add_modifier(Modifier::BOLD),
		594	),
		595	Span::styled(
		596	format!(" ({})", c.timestamp),
		597	Style::default().fg(Color::DarkGray),
		598	),
		599	]));
		600	for l in c.body.lines() {
		601	lines.push(Line::raw(format!(" {}", l)));
		602	}
		603	}
		604	}
		605
		606	// Diff header
		607	lines.push(Line::raw(""));
		608	let diff_mode = if app.patch_interdiff_mode {
		609	let rev_idx = app.patch_revision_idx;
		610	if rev_idx > 0 {
		611	let from_rev = patch.revisions.get(rev_idx - 1).map(|r| r.number).unwrap_or(0);
		612	let to_rev = patch.revisions.get(rev_idx).map(|r| r.number).unwrap_or(0);
		613	format!("--- Interdiff r{} -> r{} ---", from_rev, to_rev)
		614	} else {
		615	"--- Diff vs base (no previous revision) ---".to_string()
		616	}
		617	} else {
		618	let rev_num = patch.revisions.get(app.patch_revision_idx).map(|r| r.number).unwrap_or(1);
		619	format!("--- Diff r{} vs base ---", rev_num)
		620	};
		621	lines.push(Line::styled(
		622	diff_mode,
		623	Style::default()
		624	.fg(Color::Green)
		625	.add_modifier(Modifier::BOLD),
		626	));
		627
		628	// Diff content
		629	if app.patch_diff.is_empty() {
		630	lines.push(Line::raw("(no diff available)"));
		631	} else {
		632	for l in app.patch_diff.lines() {
		633	let style = if l.starts_with('+') {
		634	Style::default().fg(Color::Green)
		635	} else if l.starts_with('-') {
		636	Style::default().fg(Color::Red)
		637	} else if l.starts_with("@@") {
		638	Style::default().fg(Color::Cyan)
		639	} else if l.starts_with("diff ") {
		640	Style::default().fg(Color::Yellow).add_modifier(Modifier::BOLD)
		641	} else {
		642	Style::default()
		643	};
		644	lines.push(Line::styled(l.to_string(), style));
		645	}
		646	}
		647
		648	Text::from(lines)
		649	}
		650
395	fn render_footer(frame: &mut Frame, app: &App, area: Rect) {	651	fn render_footer(frame: &mut Frame, app: &App, area: Rect) {
396	match app.input_mode {	652	match app.input_mode {
397	InputMode::Search => {	653	InputMode::Search => {
@@ -447,7 +703,8 @@ fn render_footer(frame: &mut Frame, app: &App, area: Rect) {
447	let mode_hint = match app.mode {	703	let mode_hint = match app.mode {
448	ViewMode::CommitList => " Esc:back",	704	ViewMode::CommitList => " Esc:back",
449	ViewMode::CommitDetail => " Esc:back j/k:scroll",	705	ViewMode::CommitDetail => " Esc:back j/k:scroll",
450	ViewMode::Details => " c:events",	706	ViewMode::PatchDetail => " Esc:back j/k:scroll [/]:revision d:interdiff",
		707	ViewMode::Details => " c:events p:patch",
451	};	708	};
452	let filter_hint = match app.status_filter {	709	let filter_hint = match app.status_filter {
453	StatusFilter::Open => "a:show all",	710	StatusFilter::Open => "a:show all",

tests/adversarial_test.rs

Old		New
@@ -542,9 +542,15 @@ fn arb_action() -> impl Strategy<Value = Action> {
542	base_ref,	542	base_ref,
543	branch,	543	branch,
544	fixes: None,	544	fixes: None,
		545	commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
		546	tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
545	}	547	}
546	}),	548	}),
547	proptest::option::of(".*").prop_map(|body| Action::PatchRevise { body }),	549	proptest::option::of(".*").prop_map(|body| Action::PatchRevision {
		550	commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
		551	tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
		552	body,
		553	}),
548	(".*",).prop_map(|(body,)| Action::PatchComment { body }),	554	(".*",).prop_map(|(body,)| Action::PatchComment { body }),
549	Just(Action::PatchMerge),	555	Just(Action::PatchMerge),
550	Just(Action::Merge),	556	Just(Action::Merge),

tests/collab_test.rs

Old		New
@@ -332,7 +332,9 @@ fn test_patch_review_workflow() {
332	let event = Event {	332	let event = Event {
333	timestamp: now(),	333	timestamp: now(),
334	author: alice(),	334	author: alice(),
335	action: Action::PatchRevise {	335	action: Action::PatchRevision {
		336	commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
		337	tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
336	body: Some("Updated implementation".to_string()),	338	body: Some("Updated implementation".to_string()),
337	},	339	},
338	clock: 0,	340	clock: 0,
@@ -343,7 +345,9 @@ fn test_patch_review_workflow() {
343		345
344	let state = PatchState::from_ref(&repo, &ref_name, &id).unwrap();	346	let state = PatchState::from_ref(&repo, &ref_name, &id).unwrap();
345	assert_eq!(state.reviews.len(), 2);	347	assert_eq!(state.reviews.len(), 2);
346	assert_eq!(state.body, "Updated implementation");	348	// PatchRevision body is on the revision, not the patch body
		349	assert_eq!(state.revisions.len(), 2); // revision 1 from create + revision 2 from PatchRevision
		350	assert_eq!(state.revisions[1].body.as_deref(), Some("Updated implementation"));
347	}	351	}
348		352
349	#[test]	353	#[test]
@@ -637,6 +641,8 @@ fn create_branch_patch(
637	base_ref: base_ref.to_string(),	641	base_ref: base_ref.to_string(),
638	branch: branch.to_string(),	642	branch: branch.to_string(),
639	fixes: None,	643	fixes: None,
		644	commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
		645	tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
640	},	646	},
641	clock: 0,	647	clock: 0,
642	};	648	};
@@ -850,7 +856,9 @@ fn test_patch_create_with_head_commit_field_deserializes() {
850	"title": "Agent patch",	856	"title": "Agent patch",
851	"body": "from worktree",	857	"body": "from worktree",
852	"base_ref": "main",	858	"base_ref": "main",
853	"head_commit": "abc123def456"	859	"head_commit": "abc123def456",
		860	"commit": "abc123",
		861	"tree": "def456"
854	}	862	}
855	}"#;	863	}"#;
856	let event: Event = serde_json::from_str(json).unwrap();	864	let event: Event = serde_json::from_str(json).unwrap();
@@ -872,7 +880,9 @@ fn test_patch_create_with_branch_field_still_works() {
872	"title": "Normal patch",	880	"title": "Normal patch",
873	"body": "",	881	"body": "",
874	"base_ref": "main",	882	"base_ref": "main",
875	"branch": "feature-branch"	883	"branch": "feature-branch",
		884	"commit": "abc123",
		885	"tree": "def456"
876	}	886	}
877	}"#;	887	}"#;
878	let event: Event = serde_json::from_str(json).unwrap();	888	let event: Event = serde_json::from_str(json).unwrap();
@@ -903,6 +913,8 @@ fn test_resolve_head_with_oid_string() {
903	base_ref: "main".to_string(),	913	base_ref: "main".to_string(),
904	branch: tip.to_string(),	914	branch: tip.to_string(),
905	fixes: None,	915	fixes: None,
		916	commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
		917	tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
906	},	918	},
907	clock: 0,	919	clock: 0,
908	};	920	};

tests/common/mod.rs

Old		New
@@ -45,7 +45,8 @@ pub fn setup_signing_key(config_dir: &Path) {
45	git_collab::signing::generate_keypair(config_dir).expect("generate test keypair");	45	git_collab::signing::generate_keypair(config_dir).expect("generate test keypair");
46	}	46	}
47		47
48	/// Create a non-bare repo in a directory with user identity configured.	48	/// Create a non-bare repo in a directory with user identity configured
		49	/// and an initial empty commit on `main`.
49	pub fn init_repo(dir: &Path, author: &Author) -> Repository {	50	pub fn init_repo(dir: &Path, author: &Author) -> Repository {
50	let repo = Repository::init(dir).expect("init repo");	51	let repo = Repository::init(dir).expect("init repo");
51	{	52	{
@@ -53,6 +54,13 @@ pub fn init_repo(dir: &Path, author: &Author) -> Repository {
53	config.set_str("user.name", &author.name).unwrap();	54	config.set_str("user.name", &author.name).unwrap();
54	config.set_str("user.email", &author.email).unwrap();	55	config.set_str("user.email", &author.email).unwrap();
55	}	56	}
		57	// Create initial empty commit so that HEAD and refs/heads/main exist
		58	{
		59	let sig = git2::Signature::now(&author.name, &author.email).unwrap();
		60	let tree_oid = repo.treebuilder(None).unwrap().write().unwrap();
		61	let tree = repo.find_tree(tree_oid).unwrap();
		62	repo.commit(Some("refs/heads/main"), &sig, &sig, "initial", &tree, &[]).unwrap();
		63	}
56	repo	64	repo
57	}	65	}
58		66
@@ -117,6 +125,11 @@ pub fn reopen_issue(repo: &Repository, ref_name: &str, author: &Author) {
117	/// Create a patch using DAG primitives. Returns (ref_name, id).	125	/// Create a patch using DAG primitives. Returns (ref_name, id).
118	pub fn create_patch(repo: &Repository, author: &Author, title: &str) -> (String, String) {	126	pub fn create_patch(repo: &Repository, author: &Author, title: &str) -> (String, String) {
119	let sk = test_signing_key();	127	let sk = test_signing_key();
		128	// Get branch tip for commit/tree fields
		129	let head = repo.head().unwrap();
		130	let commit_oid = head.target().unwrap();
		131	let commit = repo.find_commit(commit_oid).unwrap();
		132	let tree_oid = commit.tree().unwrap().id();
120	let event = Event {	133	let event = Event {
121	timestamp: now(),	134	timestamp: now(),
122	author: author.clone(),	135	author: author.clone(),
@@ -126,6 +139,8 @@ pub fn create_patch(repo: &Repository, author: &Author, title: &str) -> (String,
126	base_ref: "main".to_string(),	139	base_ref: "main".to_string(),
127	branch: "test-branch".to_string(),	140	branch: "test-branch".to_string(),
128	fixes: None,	141	fixes: None,
		142	commit: commit_oid.to_string(),
		143	tree: tree_oid.to_string(),
129	},	144	},
130	clock: 0,	145	clock: 0,
131	};	146	};
@@ -145,6 +160,7 @@ pub fn add_review(repo: &Repository, ref_name: &str, author: &Author, verdict: R
145	action: Action::PatchReview {	160	action: Action::PatchReview {
146	verdict,	161	verdict,
147	body: "review comment".to_string(),	162	body: "review comment".to_string(),
		163	revision: 1,
148	},	164	},
149	clock: 0,	165	clock: 0,
150	};	166	};

tests/crdt_test.rs

Old		New
@@ -361,6 +361,8 @@ fn concurrent_patch_close_merge_higher_clock_wins() {
361	base_ref: "main".to_string(),	361	base_ref: "main".to_string(),
362	branch: "test-branch".to_string(),	362	branch: "test-branch".to_string(),
363	fixes: None,	363	fixes: None,
		364	commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
		365	tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
364	},	366	},
365	clock: 0,	367	clock: 0,
366	};	368	};

tests/revision_test.rs

Old		New
@@ -0,0 +1,767 @@
		1	mod common;
		2
		3	use common::TestRepo;
		4	use common::{alice, bob, init_repo, test_signing_key, now};
		5
		6	use git_collab::dag;
		7	use git_collab::event::{Action, Event};
		8	use git_collab::state::PatchState;
		9
		10	use tempfile::TempDir;
		11
		12	// ===========================================================================
		13	// Revision recording on patch create
		14	// ===========================================================================
		15
		16	#[test]
		17	fn test_patch_create_records_revision_1() {
		18	let repo = TestRepo::new("Alice", "alice@example.com");
		19	let id = repo.patch_create("Feature X");
		20
		21	let out = repo.run_ok(&["patch", "show", &id, "--json"]);
		22	let json: serde_json::Value = serde_json::from_str(&out).unwrap();
		23	let revisions = json["revisions"].as_array().unwrap();
		24	assert_eq!(revisions.len(), 1);
		25	assert_eq!(revisions[0]["number"], 1);
		26	assert!(!revisions[0]["commit"].as_str().unwrap().is_empty());
		27	assert!(!revisions[0]["tree"].as_str().unwrap().is_empty());
		28	}
		29
		30	// ===========================================================================
		31	// Auto-detect revision on comment
		32	// ===========================================================================
		33
		34	#[test]
		35	fn test_auto_detect_revision_on_comment() {
		36	let repo = TestRepo::new("Alice", "alice@example.com");
		37
		38	// Create feature branch and patch
		39	repo.git(&["checkout", "-b", "feat-auto"]);
		40	repo.commit_file("v1.txt", "v1", "initial commit");
		41	let out = repo.run_ok(&["patch", "create", "-t", "Auto-detect test", "-B", "feat-auto"]);
		42	let id = out.trim().strip_prefix("Created patch ").unwrap().to_string();
		43
		44	// Push a new commit to the branch
		45	repo.git(&["checkout", "feat-auto"]);
		46	repo.commit_file("v2.txt", "v2", "second commit");
		47	repo.git(&["checkout", "main"]);
		48
		49	// Comment should auto-insert a PatchRevision first
		50	repo.run_ok(&["patch", "comment", &id, "-b", "Looks interesting"]);
		51
		52	let out = repo.run_ok(&["patch", "show", &id, "--json"]);
		53	let json: serde_json::Value = serde_json::from_str(&out).unwrap();
		54	let revisions = json["revisions"].as_array().unwrap();
		55	assert_eq!(revisions.len(), 2, "should have 2 revisions (create + auto-detect)");
		56	assert_eq!(revisions[0]["number"], 1);
		57	assert_eq!(revisions[1]["number"], 2);
		58	}
		59
		60	#[test]
		61	fn test_no_revision_when_branch_unchanged() {
		62	let repo = TestRepo::new("Alice", "alice@example.com");
		63
		64	repo.git(&["checkout", "-b", "feat-no-change"]);
		65	repo.commit_file("v1.txt", "v1", "initial commit");
		66	let out = repo.run_ok(&["patch", "create", "-t", "No change test", "-B", "feat-no-change"]);
		67	let id = out.trim().strip_prefix("Created patch ").unwrap().to_string();
		68
		69	// Comment WITHOUT branch change — no new revision
		70	repo.run_ok(&["patch", "comment", &id, "-b", "Just a thought"]);
		71
		72	let out = repo.run_ok(&["patch", "show", &id, "--json"]);
		73	let json: serde_json::Value = serde_json::from_str(&out).unwrap();
		74	let revisions = json["revisions"].as_array().unwrap();
		75	assert_eq!(revisions.len(), 1, "should still have only 1 revision");
		76	}
		77
		78	// ===========================================================================
		79	// Auto-detect revision on review
		80	// ===========================================================================
		81
		82	#[test]
		83	fn test_auto_detect_revision_on_review() {
		84	let repo = TestRepo::new("Alice", "alice@example.com");
		85
		86	repo.git(&["checkout", "-b", "feat-review"]);
		87	repo.commit_file("v1.txt", "v1", "initial commit");
		88	let out = repo.run_ok(&["patch", "create", "-t", "Review test", "-B", "feat-review"]);
		89	let id = out.trim().strip_prefix("Created patch ").unwrap().to_string();
		90
		91	// Push a new commit
		92	repo.git(&["checkout", "feat-review"]);
		93	repo.commit_file("v2.txt", "v2", "second commit");
		94	repo.git(&["checkout", "main"]);
		95
		96	// Review should auto-insert a PatchRevision
		97	repo.run_ok(&["patch", "review", &id, "-v", "approve", "-b", "LGTM"]);
		98
		99	let out = repo.run_ok(&["patch", "show", &id, "--json"]);
		100	let json: serde_json::Value = serde_json::from_str(&out).unwrap();
		101	let revisions = json["revisions"].as_array().unwrap();
		102	assert_eq!(revisions.len(), 2);
		103
		104	// Review should be anchored to revision 2
		105	let reviews = json["reviews"].as_array().unwrap();
		106	assert_eq!(reviews.len(), 1);
		107	assert_eq!(reviews[0]["revision"], 2);
		108	}
		109
		110	// ===========================================================================
		111	// Explicit revise
		112	// ===========================================================================
		113
		114	#[test]
		115	fn test_revise_creates_revision() {
		116	let repo = TestRepo::new("Alice", "alice@example.com");
		117
		118	repo.git(&["checkout", "-b", "feat-revise"]);
		119	repo.commit_file("v1.txt", "v1", "initial commit");
		120	let out = repo.run_ok(&["patch", "create", "-t", "Revise test", "-B", "feat-revise"]);
		121	let id = out.trim().strip_prefix("Created patch ").unwrap().to_string();
		122
		123	// Push new commit
		124	repo.git(&["checkout", "feat-revise"]);
		125	repo.commit_file("v2.txt", "v2", "second commit");
		126	repo.git(&["checkout", "main"]);
		127
		128	repo.run_ok(&["patch", "revise", &id, "-b", "Addressed feedback"]);
		129
		130	let out = repo.run_ok(&["patch", "show", &id, "--json"]);
		131	let json: serde_json::Value = serde_json::from_str(&out).unwrap();
		132	let revisions = json["revisions"].as_array().unwrap();
		133	assert_eq!(revisions.len(), 2);
		134	assert_eq!(revisions[1]["body"].as_str().unwrap(), "Addressed feedback");
		135	}
		136
		137	#[test]
		138	fn test_revise_rejects_when_unchanged() {
		139	let repo = TestRepo::new("Alice", "alice@example.com");
		140
		141	repo.git(&["checkout", "-b", "feat-revise-err"]);
		142	repo.commit_file("v1.txt", "v1", "initial commit");
		143	let out = repo.run_ok(&["patch", "create", "-t", "Revise error test", "-B", "feat-revise-err"]);
		144	let id = out.trim().strip_prefix("Created patch ").unwrap().to_string();
		145
		146	// No new commit — revise should fail
		147	let err = repo.run_err(&["patch", "revise", &id]);
		148	assert!(err.contains("no changes since revision"));
		149	}
		150
		151	// ===========================================================================
		152	// Inline comments anchored to revisions
		153	// ===========================================================================
		154
		155	#[test]
		156	fn test_inline_comment_anchored_to_revision() {
		157	let repo = TestRepo::new("Alice", "alice@example.com");
		158
		159	repo.git(&["checkout", "-b", "feat-inline"]);
		160	repo.commit_file("src/lib.rs", "fn hello() {}", "initial");
		161	let out = repo.run_ok(&["patch", "create", "-t", "Inline test", "-B", "feat-inline"]);
		162	let id = out.trim().strip_prefix("Created patch ").unwrap().to_string();
		163
		164	// Comment on revision 1
		165	repo.run_ok(&[
		166	"patch", "comment", &id, "-b", "nit: naming", "-f", "src/lib.rs", "-l", "1",
		167	]);
		168
		169	let out = repo.run_ok(&["patch", "show", &id, "--json"]);
		170	let json: serde_json::Value = serde_json::from_str(&out).unwrap();
		171	let inline = json["inline_comments"].as_array().unwrap();
		172	assert_eq!(inline.len(), 1);
		173	assert_eq!(inline[0]["revision"], 1);
		174	}
		175
		176	#[test]
		177	fn test_inline_comment_explicit_revision() {
		178	let repo = TestRepo::new("Alice", "alice@example.com");
		179
		180	repo.git(&["checkout", "-b", "feat-inline-rev"]);
		181	repo.commit_file("lib.rs", "fn a() {}", "v1");
		182	let out = repo.run_ok(&["patch", "create", "-t", "Inline rev test", "-B", "feat-inline-rev"]);
		183	let id = out.trim().strip_prefix("Created patch ").unwrap().to_string();
		184
		185	// Push v2
		186	repo.git(&["checkout", "feat-inline-rev"]);
		187	repo.commit_file("lib.rs", "fn b() {}", "v2");
		188	repo.git(&["checkout", "main"]);
		189
		190	// Auto-detect by commenting
		191	repo.run_ok(&["patch", "comment", &id, "-b", "trigger revision"]);
		192
		193	// Now explicitly target revision 1
		194	repo.run_ok(&[
		195	"patch", "comment", &id, "-b", "old nit", "-f", "lib.rs", "-l", "1", "--revision", "1",
		196	]);
		197
		198	let out = repo.run_ok(&["patch", "show", &id, "--json"]);
		199	let json: serde_json::Value = serde_json::from_str(&out).unwrap();
		200	let inline = json["inline_comments"].as_array().unwrap();
		201	assert_eq!(inline.len(), 1);
		202	assert_eq!(inline[0]["revision"], 1);
		203	}
		204
		205	#[test]
		206	fn test_thread_comment_rejects_revision_flag() {
		207	let repo = TestRepo::new("Alice", "alice@example.com");
		208
		209	repo.git(&["checkout", "-b", "feat-thread"]);
		210	repo.commit_file("x.txt", "x", "init");
		211	let out = repo.run_ok(&["patch", "create", "-t", "Thread test", "-B", "feat-thread"]);
		212	let id = out.trim().strip_prefix("Created patch ").unwrap().to_string();
		213
		214	let err = repo.run_err(&["patch", "comment", &id, "-b", "note", "--revision", "1"]);
		215	assert!(err.contains("thread comments are not revision-scoped"));
		216	}
		217
		218	// ===========================================================================
		219	// patch show --revision N filters
		220	// ===========================================================================
		221
		222	#[test]
		223	fn test_show_revision_filter() {
		224	let repo = TestRepo::new("Alice", "alice@example.com");
		225
		226	repo.git(&["checkout", "-b", "feat-show-rev"]);
		227	repo.commit_file("a.txt", "a", "v1");
		228	let out = repo.run_ok(&["patch", "create", "-t", "Show rev test", "-B", "feat-show-rev"]);
		229	let id = out.trim().strip_prefix("Created patch ").unwrap().to_string();
		230
		231	// Comment on r1
		232	repo.run_ok(&[
		233	"patch", "comment", &id, "-b", "r1 comment", "-f", "a.txt", "-l", "1",
		234	]);
		235
		236	// Push v2 and comment on r2
		237	repo.git(&["checkout", "feat-show-rev"]);
		238	repo.commit_file("b.txt", "b", "v2");
		239	repo.git(&["checkout", "main"]);
		240	repo.run_ok(&[
		241	"patch", "comment", &id, "-b", "r2 comment", "-f", "b.txt", "-l", "1",
		242	]);
		243
		244	// Show --revision 1: should show r1 comment, not r2
		245	let out = repo.run_ok(&["patch", "show", &id, "--revision", "1"]);
		246	assert!(out.contains("r1 comment"));
		247	assert!(!out.contains("r2 comment"));
		248
		249	// Show --revision 2: should show r2 comment, not r1
		250	let out = repo.run_ok(&["patch", "show", &id, "--revision", "2"]);
		251	assert!(out.contains("r2 comment"));
		252	assert!(!out.contains("r1 comment"));
		253	}
		254
		255	// ===========================================================================
		256	// Interdiff between revisions
		257	// ===========================================================================
		258
		259	#[test]
		260	fn test_interdiff_between_revisions() {
		261	let repo = TestRepo::new("Alice", "alice@example.com");
		262
		263	repo.git(&["checkout", "-b", "feat-interdiff"]);
		264	repo.commit_file("a.txt", "hello", "v1");
		265	let out = repo.run_ok(&["patch", "create", "-t", "Interdiff test", "-B", "feat-interdiff"]);
		266	let id = out.trim().strip_prefix("Created patch ").unwrap().to_string();
		267
		268	// Push v2
		269	repo.git(&["checkout", "feat-interdiff"]);
		270	repo.commit_file("b.txt", "world", "v2");
		271	repo.git(&["checkout", "main"]);
		272
		273	// Create revision 2 via revise
		274	repo.run_ok(&["patch", "revise", &id]);
		275
		276	// Interdiff between r1 and r2 should show b.txt added
		277	let out = repo.run_ok(&["patch", "diff", &id, "--between", "1", "2"]);
		278	assert!(out.contains("b.txt"));
		279	}
		280
		281	#[test]
		282	fn test_interdiff_single_arg_means_n_to_latest() {
		283	let repo = TestRepo::new("Alice", "alice@example.com");
		284
		285	repo.git(&["checkout", "-b", "feat-between-single"]);
		286	repo.commit_file("a.txt", "hello", "v1");
		287	let out = repo.run_ok(&["patch", "create", "-t", "Between single test", "-B", "feat-between-single"]);
		288	let id = out.trim().strip_prefix("Created patch ").unwrap().to_string();
		289
		290	// Push v2
		291	repo.git(&["checkout", "feat-between-single"]);
		292	repo.commit_file("c.txt", "new", "v2");
		293	repo.git(&["checkout", "main"]);
		294	repo.run_ok(&["patch", "revise", &id]);
		295
		296	// --between 1 (single arg) = 1..latest
		297	let out = repo.run_ok(&["patch", "diff", &id, "--between", "1"]);
		298	assert!(out.contains("c.txt"));
		299	}
		300
		301	#[test]
		302	fn test_interdiff_nonexistent_revision_errors() {
		303	let repo = TestRepo::new("Alice", "alice@example.com");
		304
		305	repo.git(&["checkout", "-b", "feat-bad-rev"]);
		306	repo.commit_file("a.txt", "a", "v1");
		307	let out = repo.run_ok(&["patch", "create", "-t", "Bad rev test", "-B", "feat-bad-rev"]);
		308	let id = out.trim().strip_prefix("Created patch ").unwrap().to_string();
		309
		310	let err = repo.run_err(&["patch", "diff", &id, "--between", "1", "2"]);
		311	assert!(err.contains("revision 2 not found"));
		312	}
		313
		314	#[test]
		315	fn test_diff_revision_flag_shows_historical_diff() {
		316	let repo = TestRepo::new("Alice", "alice@example.com");
		317
		318	repo.git(&["checkout", "-b", "feat-hist"]);
		319	repo.commit_file("a.txt", "hello", "v1");
		320	let out = repo.run_ok(&["patch", "create", "-t", "Hist diff test", "-B", "feat-hist"]);
		321	let id = out.trim().strip_prefix("Created patch ").unwrap().to_string();
		322
		323	// Push v2 (adds b.txt)
		324	repo.git(&["checkout", "feat-hist"]);
		325	repo.commit_file("b.txt", "world", "v2");
		326	repo.git(&["checkout", "main"]);
		327	repo.run_ok(&["patch", "revise", &id]);
		328
		329	// --revision 1 should show only a.txt
		330	let out = repo.run_ok(&["patch", "diff", &id, "--revision", "1"]);
		331	assert!(out.contains("a.txt"));
		332	assert!(!out.contains("b.txt"));
		333	}
		334
		335	#[test]
		336	fn test_diff_revision_and_between_mutually_exclusive() {
		337	let repo = TestRepo::new("Alice", "alice@example.com");
		338
		339	repo.git(&["checkout", "-b", "feat-mutex"]);
		340	repo.commit_file("a.txt", "a", "v1");
		341	let out = repo.run_ok(&["patch", "create", "-t", "Mutex test", "-B", "feat-mutex"]);
		342	let id = out.trim().strip_prefix("Created patch ").unwrap().to_string();
		343
		344	let err = repo.run_err(&["patch", "diff", &id, "--revision", "1", "--between", "1"]);
		345	assert!(err.contains("mutually exclusive"));
		346	}
		347
		348	// ===========================================================================
		349	// Patch log
		350	// ===========================================================================
		351
		352	#[test]
		353	fn test_patch_log() {
		354	let repo = TestRepo::new("Alice", "alice@example.com");
		355
		356	repo.git(&["checkout", "-b", "feat-log"]);
		357	repo.commit_file("a.txt", "hello", "v1");
		358	let out = repo.run_ok(&["patch", "create", "-t", "Log test", "-B", "feat-log"]);
		359	let id = out.trim().strip_prefix("Created patch ").unwrap().to_string();
		360
		361	// Push v2
		362	repo.git(&["checkout", "feat-log"]);
		363	repo.commit_file("b.txt", "world", "v2");
		364	repo.git(&["checkout", "main"]);
		365	repo.run_ok(&["patch", "revise", &id, "-b", "added b.txt"]);
		366
		367	// Push v3
		368	repo.git(&["checkout", "feat-log"]);
		369	repo.commit_file("c.txt", "!", "v3");
		370	repo.git(&["checkout", "main"]);
		371	repo.run_ok(&["patch", "revise", &id]);
		372
		373	let out = repo.run_ok(&["patch", "log", &id]);
		374	assert!(out.contains("r1"));
		375	assert!(out.contains("(initial)"));
		376	assert!(out.contains("r2"));
		377	assert!(out.contains("added b.txt"));
		378	assert!(out.contains("r3"));
		379	}
		380
		381	#[test]
		382	fn test_patch_log_json() {
		383	let repo = TestRepo::new("Alice", "alice@example.com");
		384
		385	repo.git(&["checkout", "-b", "feat-log-json"]);
		386	repo.commit_file("a.txt", "a", "v1");
		387	let out = repo.run_ok(&["patch", "create", "-t", "Log JSON test", "-B", "feat-log-json"]);
		388	let id = out.trim().strip_prefix("Created patch ").unwrap().to_string();
		389
		390	let out = repo.run_ok(&["patch", "log", &id, "--json"]);
		391	let json: serde_json::Value = serde_json::from_str(&out).unwrap();
		392	let revisions = json.as_array().unwrap();
		393	assert_eq!(revisions.len(), 1);
		394	assert_eq!(revisions[0]["number"], 1);
		395	}
		396
		397	// ===========================================================================
		398	// Reviews anchored to revisions
		399	// ===========================================================================
		400
		401	#[test]
		402	fn test_review_explicit_revision() {
		403	let repo = TestRepo::new("Alice", "alice@example.com");
		404
		405	repo.git(&["checkout", "-b", "feat-rev-explicit"]);
		406	repo.commit_file("a.txt", "a", "v1");
		407	let out = repo.run_ok(&["patch", "create", "-t", "Rev review", "-B", "feat-rev-explicit"]);
		408	let id = out.trim().strip_prefix("Created patch ").unwrap().to_string();
		409
		410	// Push v2
		411	repo.git(&["checkout", "feat-rev-explicit"]);
		412	repo.commit_file("b.txt", "b", "v2");
		413	repo.git(&["checkout", "main"]);
		414	repo.run_ok(&["patch", "revise", &id]);
		415
		416	// Review targeting revision 1
		417	repo.run_ok(&[
		418	"patch", "review", &id, "-v", "request-changes", "-b", "fix r1", "--revision", "1",
		419	]);
		420
		421	let out = repo.run_ok(&["patch", "show", &id, "--json"]);
		422	let json: serde_json::Value = serde_json::from_str(&out).unwrap();
		423	let reviews = json["reviews"].as_array().unwrap();
		424	assert_eq!(reviews.len(), 1);
		425	assert_eq!(reviews[0]["revision"], 1);
		426	}
		427
		428	#[test]
		429	fn test_review_shows_revision_context_in_show() {
		430	let repo = TestRepo::new("Alice", "alice@example.com");
		431
		432	repo.git(&["checkout", "-b", "feat-rev-show"]);
		433	repo.commit_file("a.txt", "a", "v1");
		434	let out = repo.run_ok(&["patch", "create", "-t", "Review show", "-B", "feat-rev-show"]);
		435	let id = out.trim().strip_prefix("Created patch ").unwrap().to_string();
		436
		437	repo.run_ok(&["patch", "review", &id, "-v", "approve", "-b", "LGTM"]);
		438
		439	let out = repo.run_ok(&["patch", "show", &id]);
		440	assert!(out.contains("(r1)"));
		441	}
		442
		443	// ===========================================================================
		444	// Dedup by commit OID
		445	// ===========================================================================
		446
		447	#[test]
		448	fn test_revision_dedup_by_commit_oid() {
		449	// Two comments with no branch change should not create duplicate revisions
		450	let repo = TestRepo::new("Alice", "alice@example.com");
		451
		452	repo.git(&["checkout", "-b", "feat-dedup"]);
		453	repo.commit_file("a.txt", "a", "v1");
		454	let out = repo.run_ok(&["patch", "create", "-t", "Dedup test", "-B", "feat-dedup"]);
		455	let id = out.trim().strip_prefix("Created patch ").unwrap().to_string();
		456
		457	// Push v2
		458	repo.git(&["checkout", "feat-dedup"]);
		459	repo.commit_file("b.txt", "b", "v2");
		460	repo.git(&["checkout", "main"]);
		461
		462	// Two comments — first should auto-detect revision, second should not create another
		463	repo.run_ok(&["patch", "comment", &id, "-b", "first"]);
		464	repo.run_ok(&["patch", "comment", &id, "-b", "second"]);
		465
		466	let out = repo.run_ok(&["patch", "show", &id, "--json"]);
		467	let json: serde_json::Value = serde_json::from_str(&out).unwrap();
		468	let revisions = json["revisions"].as_array().unwrap();
		469	assert_eq!(revisions.len(), 2, "should have exactly 2 revisions, not 3");
		470	}
		471
		472	// ===========================================================================
		473	// Revision count in show output
		474	// ===========================================================================
		475
		476	#[test]
		477	fn test_show_displays_revision_count() {
		478	let repo = TestRepo::new("Alice", "alice@example.com");
		479
		480	repo.git(&["checkout", "-b", "feat-count"]);
		481	repo.commit_file("a.txt", "a", "v1");
		482	let out = repo.run_ok(&["patch", "create", "-t", "Count test", "-B", "feat-count"]);
		483	let id = out.trim().strip_prefix("Created patch ").unwrap().to_string();
		484
		485	let out = repo.run_ok(&["patch", "show", &id]);
		486	assert!(out.contains("(r1)"));
		487	}
		488
		489	// ===========================================================================
		490	// D1: Concurrent PatchRevision dedup after DAG reconciliation
		491	// ===========================================================================
		492
		493	#[test]
		494	fn test_concurrent_revision_dedup_after_reconcile() {
		495	// Two collaborators independently detect the same branch change and
		496	// create PatchRevision events with the same commit OID. After DAG
		497	// reconciliation, only one revision boundary should exist.
		498	let tmp = TempDir::new().unwrap();
		499	let repo = init_repo(tmp.path(), &alice());
		500
		501	// Create a branch with a commit
		502	let head = repo.head().unwrap().target().unwrap();
		503	let head_commit = repo.find_commit(head).unwrap();
		504	repo.branch("feat-concurrent", &head_commit, false).unwrap();
		505
		506	let sk = test_signing_key();
		507
		508	// Create a patch (revision 1)
		509	let tree_oid = head_commit.tree().unwrap().id();
		510	let create_event = Event {
		511	timestamp: now(),
		512	author: alice(),
		513	action: Action::PatchCreate {
		514	title: "Concurrent test".to_string(),
		515	body: "".to_string(),
		516	base_ref: "main".to_string(),
		517	branch: "feat-concurrent".to_string(),
		518	fixes: None,
		519	commit: head.to_string(),
		520	tree: tree_oid.to_string(),
		521	},
		522	clock: 0,
		523	};
		524	let patch_oid = dag::create_root_event(&repo, &create_event, &sk).unwrap();
		525	let id = patch_oid.to_string();
		526	let ref_name = format!("refs/collab/patches/{}", id);
		527	repo.reference(&ref_name, patch_oid, false, "test").unwrap();
		528
		529	// Push a new commit on the branch
		530	let sig = git2::Signature::now("Alice", "alice@example.com").unwrap();
		531	let blob = repo.blob(b"new content").unwrap();
		532	let mut tb = repo.treebuilder(Some(&head_commit.tree().unwrap())).unwrap();
		533	tb.insert("new.txt", blob, 0o100644).unwrap();
		534	let new_tree_oid = tb.write().unwrap();
		535	let new_tree = repo.find_tree(new_tree_oid).unwrap();
		536	let new_commit = repo.commit(
		537	Some("refs/heads/feat-concurrent"),
		538	&sig, &sig, "new commit", &new_tree, &[&head_commit],
		539	).unwrap();
		540
		541	let root_tip = repo.refname_to_id(&ref_name).unwrap();
		542
		543	// Alice appends a PatchRevision for the new commit
		544	let rev_event = Event {
		545	timestamp: now(),
		546	author: alice(),
		547	action: Action::PatchRevision {
		548	commit: new_commit.to_string(),
		549	tree: new_tree_oid.to_string(),
		550	body: None,
		551	},
		552	clock: 0,
		553	};
		554	dag::append_event(&repo, &ref_name, &rev_event, &sk).unwrap();
		555	let alice_tip = repo.refname_to_id(&ref_name).unwrap();
		556
		557	// Reset ref back to root, then Bob also appends same PatchRevision
		558	repo.reference(&ref_name, root_tip, true, "reset for bob").unwrap();
		559	let rev_event_bob = Event {
		560	timestamp: now(),
		561	author: bob(),
		562	action: Action::PatchRevision {
		563	commit: new_commit.to_string(),
		564	tree: new_tree_oid.to_string(),
		565	body: None,
		566	},
		567	clock: 0,
		568	};
		569	dag::append_event(&repo, &ref_name, &rev_event_bob, &sk).unwrap();
		570	let bob_tip = repo.refname_to_id(&ref_name).unwrap();
		571
		572	// Reconcile: create a remote ref pointing to bob's tip, local to alice's tip
		573	let remote_ref = format!("refs/collab/sync/origin/patches/{}", id);
		574	repo.reference(&remote_ref, bob_tip, true, "remote").unwrap();
		575	repo.reference(&ref_name, alice_tip, true, "restore alice").unwrap();
		576	dag::reconcile(&repo, &ref_name, &remote_ref, &alice(), &sk).unwrap();
		577
		578	// Materialize and verify: should have exactly 2 revisions (create + 1 deduped)
		579	let state = PatchState::from_ref(&repo, &ref_name, &id).unwrap();
		580	assert_eq!(
		581	state.revisions.len(), 2,
		582	"duplicate PatchRevision events with same commit OID should be deduped to one revision"
		583	);
		584	assert_eq!(state.revisions[0].number, 1);
		585	assert_eq!(state.revisions[1].number, 2);
		586	assert_eq!(state.revisions[1].commit, new_commit.to_string());
		587	}
		588
		589	// ===========================================================================
		590	// D2: Merge policy enforcement (require_approval_on_latest)
		591	// ===========================================================================
		592
		593	#[test]
		594	fn test_merge_policy_require_approval_on_latest() {
		595	let tmp = TempDir::new().unwrap();
		596	let repo = init_repo(tmp.path(), &alice());
		597	let sk = test_signing_key();
		598
		599	// Create a branch with a commit
		600	let head = repo.head().unwrap().target().unwrap();
		601	let head_commit = repo.find_commit(head).unwrap();
		602	repo.branch("feat-policy", &head_commit, false).unwrap();
		603
		604	// Add a commit on the branch so it differs from main
		605	let sig = git2::Signature::now("Alice", "alice@example.com").unwrap();
		606	let blob = repo.blob(b"feature code").unwrap();
		607	let mut tb = repo.treebuilder(Some(&head_commit.tree().unwrap())).unwrap();
		608	tb.insert("feature.txt", blob, 0o100644).unwrap();
		609	let feat_tree_oid = tb.write().unwrap();
		610	let feat_tree = repo.find_tree(feat_tree_oid).unwrap();
		611	let feat_commit = repo.commit(
		612	Some("refs/heads/feat-policy"),
		613	&sig, &sig, "add feature", &feat_tree, &[&head_commit],
		614	).unwrap();
		615
		616	// Create patch (revision 1)
		617	let create_event = Event {
		618	timestamp: now(),
		619	author: alice(),
		620	action: Action::PatchCreate {
		621	title: "Policy test".to_string(),
		622	body: "".to_string(),
		623	base_ref: "main".to_string(),
		624	branch: "feat-policy".to_string(),
		625	fixes: None,
		626	commit: feat_commit.to_string(),
		627	tree: feat_tree_oid.to_string(),
		628	},
		629	clock: 0,
		630	};
		631	let patch_oid = dag::create_root_event(&repo, &create_event, &sk).unwrap();
		632	let patch_id = patch_oid.to_string();
		633	let patch_ref = format!("refs/collab/patches/{}", patch_id);
		634	repo.reference(&patch_ref, patch_oid, false, "test").unwrap();
		635
		636	// Approve on revision 1
		637	let review_event = Event {
		638	timestamp: now(),
		639	author: bob(),
		640	action: Action::PatchReview {
		641	verdict: git_collab::event::ReviewVerdict::Approve,
		642	body: "LGTM".to_string(),
		643	revision: 1,
		644	},
		645	clock: 0,
		646	};
		647	dag::append_event(&repo, &patch_ref, &review_event, &sk).unwrap();
		648
		649	// Push revision 2 (new commit on branch)
		650	let blob2 = repo.blob(b"v2 code").unwrap();
		651	let parent = repo.find_commit(feat_commit).unwrap();
		652	let mut tb2 = repo.treebuilder(Some(&parent.tree().unwrap())).unwrap();
		653	tb2.insert("v2.txt", blob2, 0o100644).unwrap();
		654	let v2_tree_oid = tb2.write().unwrap();
		655	let v2_tree = repo.find_tree(v2_tree_oid).unwrap();
		656	let v2_commit = repo.commit(
		657	Some("refs/heads/feat-policy"),
		658	&sig, &sig, "v2", &v2_tree, &[&parent],
		659	).unwrap();
		660
		661	// Record revision 2
		662	let rev_event = Event {
		663	timestamp: now(),
		664	author: alice(),
		665	action: Action::PatchRevision {
		666	commit: v2_commit.to_string(),
		667	tree: v2_tree_oid.to_string(),
		668	body: None,
		669	},
		670	clock: 0,
		671	};
		672	dag::append_event(&repo, &patch_ref, &rev_event, &sk).unwrap();
		673
		674	// Write merge policy config: require_approval_on_latest = true
		675	let config_json = br#"{"merge":{"require_approval_on_latest":true}}"#;
		676	let config_blob = repo.blob(config_json).unwrap();
		677	let mut ctb = repo.treebuilder(None).unwrap();
		678	ctb.insert("config.json", config_blob, 0o100644).unwrap();
		679	let config_tree_oid = ctb.write().unwrap();
		680	let config_tree = repo.find_tree(config_tree_oid).unwrap();
		681	repo.commit(
		682	Some("refs/collab/config"),
		683	&sig, &sig, "set merge policy", &config_tree, &[],
		684	).unwrap();
		685
		686	// Try to merge via the library — should fail because approval is on revision 1, not 2
		687	let result = git_collab::patch::merge(&repo, &patch_id[..8]);
		688	assert!(result.is_err(), "merge should fail when approval is not on latest revision");
		689	let err_msg = result.unwrap_err().to_string();
		690	assert!(
		691	err_msg.contains("merge requires approval on the latest revision"),
		692	"error message should mention latest revision requirement, got: {}",
		693	err_msg
		694	);
		695	}
		696
		697	#[test]
		698	fn test_merge_policy_passes_when_approved_on_latest() {
		699	let tmp = TempDir::new().unwrap();
		700	let repo = init_repo(tmp.path(), &alice());
		701	let sk = test_signing_key();
		702
		703	// Create a branch with a commit
		704	let head = repo.head().unwrap().target().unwrap();
		705	let head_commit = repo.find_commit(head).unwrap();
		706	repo.branch("feat-policy-ok", &head_commit, false).unwrap();
		707
		708	let sig = git2::Signature::now("Alice", "alice@example.com").unwrap();
		709	let blob = repo.blob(b"feature code").unwrap();
		710	let mut tb = repo.treebuilder(Some(&head_commit.tree().unwrap())).unwrap();
		711	tb.insert("feature.txt", blob, 0o100644).unwrap();
		712	let feat_tree_oid = tb.write().unwrap();
		713	let feat_tree = repo.find_tree(feat_tree_oid).unwrap();
		714	let feat_commit = repo.commit(
		715	Some("refs/heads/feat-policy-ok"),
		716	&sig, &sig, "add feature", &feat_tree, &[&head_commit],
		717	).unwrap();
		718
		719	// Create patch
		720	let create_event = Event {
		721	timestamp: now(),
		722	author: alice(),
		723	action: Action::PatchCreate {
		724	title: "Policy pass test".to_string(),
		725	body: "".to_string(),
		726	base_ref: "main".to_string(),
		727	branch: "feat-policy-ok".to_string(),
		728	fixes: None,
		729	commit: feat_commit.to_string(),
		730	tree: feat_tree_oid.to_string(),
		731	},
		732	clock: 0,
		733	};
		734	let patch_oid = dag::create_root_event(&repo, &create_event, &sk).unwrap();
		735	let patch_id = patch_oid.to_string();
		736	let patch_ref = format!("refs/collab/patches/{}", patch_id);
		737	repo.reference(&patch_ref, patch_oid, false, "test").unwrap();
		738
		739	// Approve on revision 1 (which IS the latest)
		740	let review_event = Event {
		741	timestamp: now(),
		742	author: bob(),
		743	action: Action::PatchReview {
		744	verdict: git_collab::event::ReviewVerdict::Approve,
		745	body: "LGTM".to_string(),
		746	revision: 1,
		747	},
		748	clock: 0,
		749	};
		750	dag::append_event(&repo, &patch_ref, &review_event, &sk).unwrap();
		751
		752	// Write merge policy config
		753	let config_json = br#"{"merge":{"require_approval_on_latest":true}}"#;
		754	let config_blob = repo.blob(config_json).unwrap();
		755	let mut ctb = repo.treebuilder(None).unwrap();
		756	ctb.insert("config.json", config_blob, 0o100644).unwrap();
		757	let config_tree_oid = ctb.write().unwrap();
		758	let config_tree = repo.find_tree(config_tree_oid).unwrap();
		759	repo.commit(
		760	Some("refs/collab/config"),
		761	&sig, &sig, "set merge policy", &config_tree, &[],
		762	).unwrap();
		763
		764	// Merge should succeed — approval is on latest revision (1)
		765	let result = git_collab::patch::merge(&repo, &patch_id[..8]);
		766	assert!(result.is_ok(), "merge should succeed when approval is on latest revision: {:?}", result.err());
		767	}

tests/signing_test.rs

Old		New
@@ -210,6 +210,8 @@ fn event_json_uses_namespaced_action_types() {
210	base_ref: "main".to_string(),	210	base_ref: "main".to_string(),
211	branch: "feature/fix-bug".to_string(),	211	branch: "feature/fix-bug".to_string(),
212	fixes: Some("deadbeef".to_string()),	212	fixes: Some("deadbeef".to_string()),
		213	commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
		214	tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
213	},	215	},
214	clock: 0,	216	clock: 0,
215	};	217	};

tests/sort_test.rs

Old		New
@@ -87,6 +87,8 @@ fn create_patch_at(
87	base_ref: "main".to_string(),	87	base_ref: "main".to_string(),
88	branch: format!("branch-{}", title.replace(' ', "-")),	88	branch: format!("branch-{}", title.replace(' ', "-")),
89	fixes: None,	89	fixes: None,
		90	commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
		91	tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
90	},	92	},
91	clock: 0,	93	clock: 0,
92	};	94	};

tests/sync_test.rs

Old		New
@@ -291,6 +291,8 @@ fn test_patch_review_across_repos() {
291	base_ref: "main".to_string(),	291	base_ref: "main".to_string(),
292	branch: "feature/x".to_string(),	292	branch: "feature/x".to_string(),
293	fixes: None,	293	fixes: None,
		294	commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
		295	tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
294	},	296	},
295	clock: 0,	297	clock: 0,
296	};	298	};
@@ -312,6 +314,7 @@ fn test_patch_review_across_repos() {
312	action: Action::PatchReview {	314	action: Action::PatchReview {
313	verdict: ReviewVerdict::Approve,	315	verdict: ReviewVerdict::Approve,
314	body: "LGTM!".to_string(),	316	body: "LGTM!".to_string(),
		317	revision: 1,
315	},	318	},
316	clock: 0,	319	clock: 0,
317	};	320	};
@@ -340,6 +343,8 @@ fn test_concurrent_review_and_revise() {
340	base_ref: "main".to_string(),	343	base_ref: "main".to_string(),
341	branch: "feature/wip".to_string(),	344	branch: "feature/wip".to_string(),
342	fixes: None,	345	fixes: None,
		346	commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
		347	tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
343	},	348	},
344	clock: 0,	349	clock: 0,
345	};	350	};
@@ -356,7 +361,9 @@ fn test_concurrent_review_and_revise() {
356	let revise_event = Event {	361	let revise_event = Event {
357	timestamp: now(),	362	timestamp: now(),
358	author: alice(),	363	author: alice(),
359	action: Action::PatchRevise {	364	action: Action::PatchRevision {
		365	commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
		366	tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
360	body: Some("Updated description".to_string()),	367	body: Some("Updated description".to_string()),
361	},	368	},
362	clock: 0,	369	clock: 0,
@@ -370,6 +377,7 @@ fn test_concurrent_review_and_revise() {
370	action: Action::PatchReview {	377	action: Action::PatchReview {
371	verdict: ReviewVerdict::RequestChanges,	378	verdict: ReviewVerdict::RequestChanges,
372	body: "Needs work".to_string(),	379	body: "Needs work".to_string(),
		380	revision: 1,
373	},	381	},
374	clock: 0,	382	clock: 0,
375	};	383	};
@@ -457,15 +465,6 @@ fn test_unsigned_event_sync_rejected() {
457	// Set up Alice's repo with an unsigned event directly via git2	465	// Set up Alice's repo with an unsigned event directly via git2
458	let alice_dir = TempDir::new().unwrap();	466	let alice_dir = TempDir::new().unwrap();
459	let alice_repo = common::init_repo(alice_dir.path(), &alice());	467	let alice_repo = common::init_repo(alice_dir.path(), &alice());
460	// Create initial commit so repo is not empty
461	{
462	let sig = git2::Signature::now("Alice", "alice@example.com").unwrap();
463	let tree_oid = alice_repo.treebuilder(None).unwrap().write().unwrap();
464	let tree = alice_repo.find_tree(tree_oid).unwrap();
465	alice_repo
466	.commit(Some("refs/heads/main"), &sig, &sig, "init", &tree, &[])
467	.unwrap();
468	}
469		468
470	// Create an unsigned event	469	// Create an unsigned event
471	let event = Event {	470	let event = Event {
@@ -507,14 +506,6 @@ fn test_tampered_event_sync_rejected() {
507	// Set up a repo with a tampered event	506	// Set up a repo with a tampered event
508	let dir = TempDir::new().unwrap();	507	let dir = TempDir::new().unwrap();
509	let repo = common::init_repo(dir.path(), &alice());	508	let repo = common::init_repo(dir.path(), &alice());
510	// Create initial commit
511	{
512	let sig = git2::Signature::now("Alice", "alice@example.com").unwrap();
513	let tree_oid = repo.treebuilder(None).unwrap().write().unwrap();
514	let tree = repo.find_tree(tree_oid).unwrap();
515	repo.commit(Some("refs/heads/main"), &sig, &sig, "init", &tree, &[])
516	.unwrap();
517	}
518		509
519	// Create a tampered event (signed then modified)	510	// Create a tampered event (signed then modified)
520	let event = Event {	511	let event = Event {