a73x

9de0fef3

Detect merged patches from the git graph

a73x   2026-03-22 10:12

Commit message
Detect merged patches from the git graph
benches/core_ops.rs
Old New
@@ -78,6 +78,7 @@ fn setup_patches(n: usize) -> (Repository, TempDir) {
78 fixes: None, 78 fixes: None,
79 commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(), 79 commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
80 tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(), 80 tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
81 base_commit: None,
81 }, 82 },
82 clock: 0, 83 clock: 0,
83 }; 84 };
@@ -219,6 +220,7 @@ fn bench_patch_from_ref(c: &mut Criterion) {
219 fixes: None, 220 fixes: None,
220 commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(), 221 commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
221 tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(), 222 tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
223 base_commit: None,
222 }, 224 },
223 clock: 0, 225 clock: 0,
224 }; 226 };
src/cli.rs
Old New
@@ -12,6 +12,38 @@ pub enum SortMode {
12 Alpha, 12 Alpha,
13 } 13 }
14 14
15 /// Trait for items that support list filtering, sorting, and pagination.
16 pub trait Listable {
17 fn is_open(&self) -> bool;
18 fn last_updated(&self) -> &str;
19 fn created_at(&self) -> &str;
20 fn title(&self) -> &str;
21 }
22
23 /// Filter by open status, sort by mode, then apply offset/limit pagination.
24 pub fn filter_sort_paginate<T: Listable>(
25 items: Vec<T>,
26 show_closed: bool,
27 sort: SortMode,
28 offset: Option<usize>,
29 limit: Option<usize>,
30 ) -> Vec<T> {
31 let mut filtered: Vec<T> = items
32 .into_iter()
33 .filter(|item| show_closed || item.is_open())
34 .collect();
35 match sort {
36 SortMode::Recent => filtered.sort_by(|a, b| b.last_updated().cmp(a.last_updated())),
37 SortMode::Created => filtered.sort_by(|a, b| b.created_at().cmp(a.created_at())),
38 SortMode::Alpha => filtered.sort_by(|a, b| a.title().cmp(b.title())),
39 }
40 filtered
41 .into_iter()
42 .skip(offset.unwrap_or(0))
43 .take(limit.unwrap_or(usize::MAX))
44 .collect()
45 }
46
15 #[derive(Parser)] 47 #[derive(Parser)]
16 #[command( 48 #[command(
17 name = "git-collab", 49 name = "git-collab",
@@ -328,11 +360,6 @@ pub enum PatchCmd {
328 #[arg(long)] 360 #[arg(long)]
329 json: bool, 361 json: bool,
330 }, 362 },
331 /// Merge a patch into its base branch
332 Merge {
333 /// Patch ID (prefix match)
334 id: String,
335 },
336 /// Close a patch 363 /// Close a patch
337 Close { 364 Close {
338 /// Patch ID (prefix match) 365 /// Patch ID (prefix match)
src/dag.rs
Old New
@@ -2,8 +2,8 @@ use git2::{ObjectType, Oid, Repository, Sort};
2 2
3 use crate::error::Error; 3 use crate::error::Error;
4 use crate::event::{Action, Event}; 4 use crate::event::{Action, Event};
5 use crate::identity::author_signature; 5 use crate::identity::{author_signature, get_author};
6 use crate::signing::sign_event; 6 use crate::signing::{self, sign_event};
7 7
8 /// The manifest blob content included in every event commit tree. 8 /// The manifest blob content included in every event commit tree.
9 const MANIFEST_JSON: &[u8] = br#"{"version":1,"format":"git-collab"}"#; 9 const MANIFEST_JSON: &[u8] = br#"{"version":1,"format":"git-collab"}"#;
@@ -11,43 +11,72 @@ const MANIFEST_JSON: &[u8] = br#"{"version":1,"format":"git-collab"}"#;
11 /// Maximum allowed size for an event.json blob (1 MB). 11 /// Maximum allowed size for an event.json blob (1 MB).
12 pub const MAX_EVENT_BLOB_SIZE: usize = 1_048_576; 12 pub const MAX_EVENT_BLOB_SIZE: usize = 1_048_576;
13 13
14 /// Walk the entire DAG reachable from `tip` and return the maximum clock value. 14 /// Build an event tree containing event.json, signature, pubkey, and manifest.json blobs.
15 /// Returns 0 if the DAG is empty or all events have clock 0 (pre-migration). 15 /// Returns the tree OID.
16 pub fn max_clock(repo: &Repository, tip: Oid) -> Result<u64, Error> { 16 fn build_event_tree(
17 let mut revwalk = repo.revwalk()?; 17 repo: &Repository,
18 revwalk.set_sorting(Sort::TOPOLOGICAL)?; 18 event_blob: Oid,
19 revwalk.push(tip)?; 19 sig_blob: Oid,
20 pubkey_blob: Oid,
21 manifest_blob: Oid,
22 ) -> Result<Oid, Error> {
23 let mut tb = repo.treebuilder(None)?;
24 tb.insert("event.json", event_blob, 0o100644)?;
25 tb.insert("signature", sig_blob, 0o100644)?;
26 tb.insert("pubkey", pubkey_blob, 0o100644)?;
27 tb.insert("manifest.json", manifest_blob, 0o100644)?;
28 let tree_oid = tb.write()?;
29 Ok(tree_oid)
30 }
20 31
21 let mut max = 0u64; 32 /// Sign an event, serialize it, and build the event tree. Returns the tree OID.
22 for oid_result in revwalk { 33 fn sign_and_build_tree(
23 let oid = oid_result?; 34 repo: &Repository,
24 let commit = repo.find_commit(oid)?; 35 event: &Event,
25 let tree = commit.tree()?; 36 signing_key: &ed25519_dalek::SigningKey,
26 let entry = tree 37 ) -> Result<Oid, Error> {
27 .get_name("event.json") 38 let detached = sign_event(event, signing_key)?;
28 .ok_or_else(|| git2::Error::from_str("missing event.json in commit tree"))?; 39 let event_json = serde_json::to_vec_pretty(event)?;
29
30 if entry.kind() != Some(ObjectType::Blob) {
31 return Err(Error::Git(git2::Error::from_str(
32 "event.json entry is not a blob",
33 )));
34 }
35 40
36 let blob = repo.find_blob(entry.id())?; 41 let event_blob = repo.blob(&event_json)?;
37 let content = blob.content(); 42 let sig_blob = repo.blob(detached.signature.as_bytes())?;
38 if content.len() > MAX_EVENT_BLOB_SIZE { 43 let pubkey_blob = repo.blob(detached.pubkey.as_bytes())?;
39 return Err(Error::PayloadTooLarge { 44 let manifest_blob = repo.blob(MANIFEST_JSON)?;
40 actual: content.len(),
41 limit: MAX_EVENT_BLOB_SIZE,
42 });
43 }
44 45
45 let event: Event = serde_json::from_slice(content)?; 46 build_event_tree(repo, event_blob, sig_blob, pubkey_blob, manifest_blob)
46 if event.clock > max { 47 }
47 max = event.clock; 48
48 } 49 /// Load and deserialize the event from a commit's tree.
50 fn load_event_from_commit(repo: &Repository, oid: Oid) -> Result<Event, Error> {
51 let commit = repo.find_commit(oid)?;
52 let tree = commit.tree()?;
53 let entry = tree
54 .get_name("event.json")
55 .ok_or_else(|| git2::Error::from_str("missing event.json in commit tree"))?;
56
57 if entry.kind() != Some(ObjectType::Blob) {
58 return Err(Error::Git(git2::Error::from_str(
59 "event.json entry is not a blob",
60 )));
49 } 61 }
50 Ok(max) 62
63 let blob = repo.find_blob(entry.id())?;
64 let content = blob.content();
65 if content.len() > MAX_EVENT_BLOB_SIZE {
66 return Err(Error::PayloadTooLarge {
67 actual: content.len(),
68 limit: MAX_EVENT_BLOB_SIZE,
69 });
70 }
71
72 Ok(serde_json::from_slice(content)?)
73 }
74
75 /// Read the clock value from the tip commit of a DAG.
76 /// Since clocks are monotonically increasing, the tip always has the max clock.
77 /// Returns 0 if the event has clock 0 (pre-migration).
78 pub fn max_clock(repo: &Repository, tip: Oid) -> Result<u64, Error> {
79 Ok(load_event_from_commit(repo, tip)?.clock)
51 } 80 }
52 81
53 /// Create an orphan commit (no parents) with the given event. 82 /// Create an orphan commit (no parents) with the given event.
@@ -61,20 +90,7 @@ pub fn create_root_event(
61 let mut event = event.clone(); 90 let mut event = event.clone();
62 event.clock = 1; 91 event.clock = 1;
63 92
64 let detached = sign_event(&event, signing_key)?; 93 let tree_oid = sign_and_build_tree(repo, &event, signing_key)?;
65 let event_json = serde_json::to_vec_pretty(&event)?;
66
67 let event_blob = repo.blob(&event_json)?;
68 let sig_blob = repo.blob(detached.signature.as_bytes())?;
69 let pubkey_blob = repo.blob(detached.pubkey.as_bytes())?;
70 let manifest_blob = repo.blob(MANIFEST_JSON)?;
71
72 let mut tb = repo.treebuilder(None)?;
73 tb.insert("event.json", event_blob, 0o100644)?;
74 tb.insert("signature", sig_blob, 0o100644)?;
75 tb.insert("pubkey", pubkey_blob, 0o100644)?;
76 tb.insert("manifest.json", manifest_blob, 0o100644)?;
77 let tree_oid = tb.write()?;
78 let tree = repo.find_tree(tree_oid)?; 94 let tree = repo.find_tree(tree_oid)?;
79 95
80 let sig = author_signature(&event.author)?; 96 let sig = author_signature(&event.author)?;
@@ -98,32 +114,54 @@ pub fn append_event(
98 let mut event = event.clone(); 114 let mut event = event.clone();
99 event.clock = current_max + 1; 115 event.clock = current_max + 1;
100 116
101 let detached = sign_event(&event, signing_key)?; 117 let tree_oid = sign_and_build_tree(repo, &event, signing_key)?;
102 let event_json = serde_json::to_vec_pretty(&event)?;
103
104 let event_blob = repo.blob(&event_json)?;
105 let sig_blob = repo.blob(detached.signature.as_bytes())?;
106 let pubkey_blob = repo.blob(detached.pubkey.as_bytes())?;
107 let manifest_blob = repo.blob(MANIFEST_JSON)?;
108
109 let mut tb = repo.treebuilder(None)?;
110 tb.insert("event.json", event_blob, 0o100644)?;
111 tb.insert("signature", sig_blob, 0o100644)?;
112 tb.insert("pubkey", pubkey_blob, 0o100644)?;
113 tb.insert("manifest.json", manifest_blob, 0o100644)?;
114 let tree_oid = tb.write()?;
115 let tree = repo.find_tree(tree_oid)?; 118 let tree = repo.find_tree(tree_oid)?;
116 119
117 let sig = author_signature(&event.author)?; 120 let sig = author_signature(&event.author)?;
118 let message = commit_message(&event.action); 121 let message = commit_message(&event.action);
119 122
120 let parent_oid = repo.refname_to_id(ref_name)?; 123 let parent = repo.find_commit(tip)?;
121 let parent = repo.find_commit(parent_oid)?;
122 124
123 let oid = repo.commit(Some(ref_name), &sig, &sig, &message, &tree, &[&parent])?; 125 let oid = repo.commit(Some(ref_name), &sig, &sig, &message, &tree, &[&parent])?;
124 Ok(oid) 126 Ok(oid)
125 } 127 }
126 128
129 /// Build an Event from the given action, filling in timestamp and author
130 /// automatically. The clock field is set to 0 (callers like `create_root_event`
131 /// and `append_event` overwrite it).
132 pub fn build_event(repo: &Repository, action: Action) -> Result<Event, Error> {
133 let author = get_author(repo)?;
134 Ok(Event {
135 timestamp: chrono::Utc::now().to_rfc3339(),
136 author,
137 action,
138 clock: 0,
139 })
140 }
141
142 /// Convenience wrapper: load signing key, build event, and append it to an
143 /// existing DAG ref in one call.
144 pub fn append_action(
145 repo: &Repository,
146 ref_name: &str,
147 action: Action,
148 ) -> Result<Oid, Error> {
149 let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;
150 let event = build_event(repo, action)?;
151 append_event(repo, ref_name, &event, &sk)
152 }
153
154 /// Convenience wrapper: load signing key, build event, and create a root
155 /// (orphan) DAG commit. Returns the new commit OID (entity ID).
156 pub fn create_root_action(
157 repo: &Repository,
158 action: Action,
159 ) -> Result<Oid, Error> {
160 let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;
161 let event = build_event(repo, action)?;
162 create_root_event(repo, &event, &sk)
163 }
164
127 /// Walk the DAG from the given ref in topological order (oldest first). 165 /// Walk the DAG from the given ref in topological order (oldest first).
128 /// Returns (commit_oid, event) pairs. 166 /// Returns (commit_oid, event) pairs.
129 pub fn walk_events(repo: &Repository, ref_name: &str) -> Result<Vec<(Oid, Event)>, Error> { 167 pub fn walk_events(repo: &Repository, ref_name: &str) -> Result<Vec<(Oid, Event)>, Error> {
@@ -135,31 +173,7 @@ pub fn walk_events(repo: &Repository, ref_name: &str) -> Result<Vec<(Oid, Event)
135 let mut events = Vec::new(); 173 let mut events = Vec::new();
136 for oid_result in revwalk { 174 for oid_result in revwalk {
137 let oid = oid_result?; 175 let oid = oid_result?;
138 let commit = repo.find_commit(oid)?; 176 let event = load_event_from_commit(repo, oid)?;
139 let tree = commit.tree()?;
140 let entry = tree
141 .get_name("event.json")
142 .ok_or_else(|| git2::Error::from_str("missing event.json in commit tree"))?;
143
144 // Verify the entry points to a blob, not a tree or commit
145 if entry.kind() != Some(ObjectType::Blob) {
146 return Err(Error::Git(git2::Error::from_str(
147 "event.json entry is not a blob",
148 )));
149 }
150
151 let blob = repo.find_blob(entry.id())?;
152
153 // Check blob size before attempting deserialization
154 let content = blob.content();
155 if content.len() > MAX_EVENT_BLOB_SIZE {
156 return Err(Error::PayloadTooLarge {
157 actual: content.len(),
158 limit: MAX_EVENT_BLOB_SIZE,
159 });
160 }
161
162 let event: Event = serde_json::from_slice(content)?;
163 events.push((oid, event)); 177 events.push((oid, event));
164 } 178 }
165 Ok(events) 179 Ok(events)
@@ -223,19 +237,7 @@ pub fn reconcile(
223 clock: merge_clock, 237 clock: merge_clock,
224 }; 238 };
225 239
226 let detached = sign_event(&merge_event, signing_key)?; 240 let tree_oid = sign_and_build_tree(repo, &merge_event, signing_key)?;
227 let event_json = serde_json::to_vec_pretty(&merge_event)?;
228 let event_blob = repo.blob(&event_json)?;
229 let sig_blob = repo.blob(detached.signature.as_bytes())?;
230 let pubkey_blob = repo.blob(detached.pubkey.as_bytes())?;
231 let manifest_blob = repo.blob(MANIFEST_JSON)?;
232
233 let mut tb = repo.treebuilder(None)?;
234 tb.insert("event.json", event_blob, 0o100644)?;
235 tb.insert("signature", sig_blob, 0o100644)?;
236 tb.insert("pubkey", pubkey_blob, 0o100644)?;
237 tb.insert("manifest.json", manifest_blob, 0o100644)?;
238 let tree_oid = tb.write()?;
239 let tree = repo.find_tree(tree_oid)?; 241 let tree = repo.find_tree(tree_oid)?;
240 242
241 let sig = author_signature(merge_author)?; 243 let sig = author_signature(merge_author)?;
@@ -283,19 +285,7 @@ pub fn migrate_clocks(
283 clock = migrated.clock; // respect existing clocks 285 clock = migrated.clock; // respect existing clocks
284 } 286 }
285 287
286 let detached = sign_event(&migrated, signing_key)?; 288 let tree_oid = sign_and_build_tree(repo, &migrated, signing_key)?;
287 let event_json = serde_json::to_vec_pretty(&migrated)?;
288 let event_blob = repo.blob(&event_json)?;
289 let sig_blob = repo.blob(detached.signature.as_bytes())?;
290 let pubkey_blob = repo.blob(detached.pubkey.as_bytes())?;
291 let manifest_blob = repo.blob(MANIFEST_JSON)?;
292
293 let mut tb = repo.treebuilder(None)?;
294 tb.insert("event.json", event_blob, 0o100644)?;
295 tb.insert("signature", sig_blob, 0o100644)?;
296 tb.insert("pubkey", pubkey_blob, 0o100644)?;
297 tb.insert("manifest.json", manifest_blob, 0o100644)?;
298 let tree_oid = tb.write()?;
299 let tree = repo.find_tree(tree_oid)?; 289 let tree = repo.find_tree(tree_oid)?;
300 290
301 let sig = author_signature(&migrated.author)?; 291 let sig = author_signature(&migrated.author)?;
@@ -333,7 +323,7 @@ fn commit_message(action: &Action) -> String {
333 Action::IssueReopen => "issue: reopen".to_string(), 323 Action::IssueReopen => "issue: reopen".to_string(),
334 Action::PatchCreate { title, .. } => format!("patch: create \"{}\"", title), 324 Action::PatchCreate { title, .. } => format!("patch: create \"{}\"", title),
335 Action::PatchRevision { .. } => "patch: revision".to_string(), 325 Action::PatchRevision { .. } => "patch: revision".to_string(),
336 Action::PatchReview { verdict, .. } => format!("patch: review ({:?})", verdict), 326 Action::PatchReview { verdict, .. } => format!("patch: review ({})", verdict),
337 Action::PatchComment { .. } => "patch: comment".to_string(), 327 Action::PatchComment { .. } => "patch: comment".to_string(),
338 Action::PatchInlineComment { ref file, line, .. } => { 328 Action::PatchInlineComment { ref file, line, .. } => {
339 format!("patch: inline comment on {}:{}", file, line) 329 format!("patch: inline comment on {}:{}", file, line)
src/error.rs
Old New
@@ -37,4 +37,7 @@ pub enum Error {
37 37
38 #[error("invalid ref name: {0}")] 38 #[error("invalid ref name: {0}")]
39 InvalidRefName(String), 39 InvalidRefName(String),
40
41 #[error("ambiguous id prefix '{prefix}': {count} matches")]
42 AmbiguousId { prefix: String, count: usize },
40 } 43 }
src/event.rs
Old New
@@ -67,6 +67,9 @@ pub enum Action {
67 fixes: Option<String>, 67 fixes: Option<String>,
68 commit: String, 68 commit: String,
69 tree: String, 69 tree: String,
70 /// Base branch tip OID at creation time (for merge detection).
71 #[serde(default, skip_serializing_if = "Option::is_none")]
72 base_commit: Option<String>,
70 }, 73 },
71 #[serde(rename = "patch.revision")] 74 #[serde(rename = "patch.revision")]
72 PatchRevision { 75 PatchRevision {
@@ -102,10 +105,41 @@ pub enum Action {
102 Merge, 105 Merge,
103 } 106 }
104 107
105 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] 108 #[derive(Debug, Copy, Clone, Serialize, Deserialize, PartialEq)]
106 pub enum ReviewVerdict { 109 pub enum ReviewVerdict {
107 Approve, 110 Approve,
108 RequestChanges, 111 RequestChanges,
109 Comment, 112 Comment,
110 Reject, 113 Reject,
111 } 114 }
115
116 impl ReviewVerdict {
117 pub fn as_str(&self) -> &'static str {
118 match self {
119 ReviewVerdict::Approve => "approve",
120 ReviewVerdict::RequestChanges => "request-changes",
121 ReviewVerdict::Comment => "comment",
122 ReviewVerdict::Reject => "reject",
123 }
124 }
125 }
126
127 impl std::fmt::Display for ReviewVerdict {
128 fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
129 f.write_str(self.as_str())
130 }
131 }
132
133 impl std::str::FromStr for ReviewVerdict {
134 type Err = String;
135
136 fn from_str(s: &str) -> Result<Self, Self::Err> {
137 match s {
138 "approve" => Ok(ReviewVerdict::Approve),
139 "request-changes" => Ok(ReviewVerdict::RequestChanges),
140 "comment" => Ok(ReviewVerdict::Comment),
141 "reject" => Ok(ReviewVerdict::Reject),
142 other => Err(format!("unknown verdict: {}", other)),
143 }
144 }
145 }
src/issue.rs
Old New
@@ -1,11 +1,9 @@
1 use git2::Repository; 1 use git2::Repository;
2 2
3 use crate::cli::SortMode; 3 use crate::cli::{self, SortMode};
4 use crate::dag; 4 use crate::dag;
5 use crate::event::{Action, Event}; 5 use crate::event::Action;
6 use crate::identity::get_author; 6 use crate::state::{self, IssueState};
7 use crate::signing;
8 use crate::state::{self, IssueState, IssueStatus};
9 7
10 pub fn open( 8 pub fn open(
11 repo: &Repository, 9 repo: &Repository,
@@ -13,19 +11,14 @@ pub fn open(
13 body: &str, 11 body: &str,
14 relates_to: Option<&str>, 12 relates_to: Option<&str>,
15 ) -> Result<String, crate::error::Error> { 13 ) -> Result<String, crate::error::Error> {
16 let sk = signing::load_signing_key(&signing::signing_key_dir()?)?; 14 let oid = dag::create_root_action(
17 let author = get_author(repo)?; 15 repo,
18 let event = Event { 16 Action::IssueOpen {
19 timestamp: chrono::Utc::now().to_rfc3339(),
20 author,
21 action: Action::IssueOpen {
22 title: title.to_string(), 17 title: title.to_string(),
23 body: body.to_string(), 18 body: body.to_string(),
24 relates_to: relates_to.map(|s| s.to_string()), 19 relates_to: relates_to.map(|s| s.to_string()),
25 }, 20 },
26 clock: 0, 21 )?;
27 };
28 let oid = dag::create_root_event(repo, &event, &sk)?;
29 let id = oid.to_string(); 22 let id = oid.to_string();
30 let ref_name = format!("refs/collab/issues/{}", id); 23 let ref_name = format!("refs/collab/issues/{}", id);
31 repo.reference(&ref_name, oid, false, "issue open")?; 24 repo.reference(&ref_name, oid, false, "issue open")?;
@@ -37,6 +30,14 @@ pub struct ListEntry {
37 pub unread: Option<usize>, 30 pub unread: Option<usize>,
38 } 31 }
39 32
33 fn load_issues(repo: &Repository, show_archived: bool) -> Result<Vec<state::IssueState>, crate::error::Error> {
34 if show_archived {
35 state::list_issues_with_archived(repo)
36 } else {
37 state::list_issues(repo)
38 }
39 }
40
40 pub fn list( 41 pub fn list(
41 repo: &Repository, 42 repo: &Repository,
42 show_closed: bool, 43 show_closed: bool,
@@ -45,29 +46,15 @@ pub fn list(
45 offset: Option<usize>, 46 offset: Option<usize>,
46 sort: SortMode, 47 sort: SortMode,
47 ) -> Result<Vec<ListEntry>, crate::error::Error> { 48 ) -> Result<Vec<ListEntry>, crate::error::Error> {
48 let issues = if show_archived { 49 let issues = load_issues(repo, show_archived)?;
49 state::list_issues_with_archived(repo)? 50 let filtered = cli::filter_sort_paginate(issues, show_closed, sort, offset, limit);
50 } else { 51 let entries = filtered
51 state::list_issues(repo)?
52 };
53 let mut entries: Vec<_> = issues
54 .into_iter() 52 .into_iter()
55 .filter(|i| show_closed || i.status == IssueStatus::Open)
56 .map(|issue| { 53 .map(|issue| {
57 let unread = count_unread(repo, &issue.id); 54 let unread = count_unread(repo, &issue.id);
58 ListEntry { issue, unread } 55 ListEntry { issue, unread }
59 }) 56 })
60 .collect(); 57 .collect();
61 match sort {
62 SortMode::Recent => entries.sort_by(|a, b| b.issue.last_updated.cmp(&a.issue.last_updated)),
63 SortMode::Created => entries.sort_by(|a, b| b.issue.created_at.cmp(&a.issue.created_at)),
64 SortMode::Alpha => entries.sort_by(|a, b| a.issue.title.cmp(&b.issue.title)),
65 }
66 let entries = entries
67 .into_iter()
68 .skip(offset.unwrap_or(0))
69 .take(limit.unwrap_or(usize::MAX))
70 .collect();
71 Ok(entries) 58 Ok(entries)
72 } 59 }
73 60
@@ -87,10 +74,7 @@ pub fn list_to_writer(
87 } 74 }
88 for e in &entries { 75 for e in &entries {
89 let i = &e.issue; 76 let i = &e.issue;
90 let status = match i.status { 77 let status = i.status.as_str();
91 IssueStatus::Open => "open",
92 IssueStatus::Closed => "closed",
93 };
94 let labels = if i.labels.is_empty() { 78 let labels = if i.labels.is_empty() {
95 String::new() 79 String::new()
96 } else { 80 } else {
@@ -131,20 +115,8 @@ fn count_unread(repo: &git2::Repository, id: &str) -> Option<usize> {
131 } 115 }
132 116
133 pub fn list_json(repo: &Repository, show_closed: bool, show_archived: bool, sort: SortMode) -> Result<String, crate::error::Error> { 117 pub fn list_json(repo: &Repository, show_closed: bool, show_archived: bool, sort: SortMode) -> Result<String, crate::error::Error> {
134 let issues = if show_archived { 118 let issues = load_issues(repo, show_archived)?;
135 state::list_issues_with_archived(repo)? 119 let filtered = cli::filter_sort_paginate(issues, show_closed, sort, None, None);
136 } else {
137 state::list_issues(repo)?
138 };
139 let mut filtered: Vec<_> = issues
140 .into_iter()
141 .filter(|i| show_closed || i.status == IssueStatus::Open)
142 .collect();
143 match sort {
144 SortMode::Recent => filtered.sort_by(|a, b| b.last_updated.cmp(&a.last_updated)),
145 SortMode::Created => filtered.sort_by(|a, b| b.created_at.cmp(&a.created_at)),
146 SortMode::Alpha => filtered.sort_by(|a, b| a.title.cmp(&b.title)),
147 }
148 Ok(serde_json::to_string_pretty(&filtered)?) 120 Ok(serde_json::to_string_pretty(&filtered)?)
149 } 121 }
150 122
@@ -165,34 +137,18 @@ pub fn show(repo: &Repository, id_prefix: &str) -> Result<IssueState, crate::err
165 } 137 }
166 138
167 pub fn label(repo: &Repository, id_prefix: &str, label: &str) -> Result<(), crate::error::Error> { 139 pub fn label(repo: &Repository, id_prefix: &str, label: &str) -> Result<(), crate::error::Error> {
168 let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;
169 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?; 140 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?;
170 let author = get_author(repo)?; 141 dag::append_action(repo, &ref_name, Action::IssueLabel {
171 let event = Event { 142 label: label.to_string(),
172 timestamp: chrono::Utc::now().to_rfc3339(), 143 })?;
173 author,
174 action: Action::IssueLabel {
175 label: label.to_string(),
176 },
177 clock: 0,
178 };
179 dag::append_event(repo, &ref_name, &event, &sk)?;
180 Ok(()) 144 Ok(())
181 } 145 }
182 146
183 pub fn unlabel(repo: &Repository, id_prefix: &str, label: &str) -> Result<(), crate::error::Error> { 147 pub fn unlabel(repo: &Repository, id_prefix: &str, label: &str) -> Result<(), crate::error::Error> {
184 let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;
185 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?; 148 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?;
186 let author = get_author(repo)?; 149 dag::append_action(repo, &ref_name, Action::IssueUnlabel {
187 let event = Event { 150 label: label.to_string(),
188 timestamp: chrono::Utc::now().to_rfc3339(), 151 })?;
189 author,
190 action: Action::IssueUnlabel {
191 label: label.to_string(),
192 },
193 clock: 0,
194 };
195 dag::append_event(repo, &ref_name, &event, &sk)?;
196 Ok(()) 152 Ok(())
197 } 153 }
198 154
@@ -201,18 +157,10 @@ pub fn assign(
201 id_prefix: &str, 157 id_prefix: &str,
202 assignee: &str, 158 assignee: &str,
203 ) -> Result<(), crate::error::Error> { 159 ) -> Result<(), crate::error::Error> {
204 let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;
205 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?; 160 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?;
206 let author = get_author(repo)?; 161 dag::append_action(repo, &ref_name, Action::IssueAssign {
207 let event = Event { 162 assignee: assignee.to_string(),
208 timestamp: chrono::Utc::now().to_rfc3339(), 163 })?;
209 author,
210 action: Action::IssueAssign {
211 assignee: assignee.to_string(),
212 },
213 clock: 0,
214 };
215 dag::append_event(repo, &ref_name, &event, &sk)?;
216 Ok(()) 164 Ok(())
217 } 165 }
218 166
@@ -221,18 +169,10 @@ pub fn unassign(
221 id_prefix: &str, 169 id_prefix: &str,
222 assignee: &str, 170 assignee: &str,
223 ) -> Result<(), crate::error::Error> { 171 ) -> Result<(), crate::error::Error> {
224 let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;
225 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?; 172 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?;
226 let author = get_author(repo)?; 173 dag::append_action(repo, &ref_name, Action::IssueUnassign {
227 let event = Event { 174 assignee: assignee.to_string(),
228 timestamp: chrono::Utc::now().to_rfc3339(), 175 })?;
229 author,
230 action: Action::IssueUnassign {
231 assignee: assignee.to_string(),
232 },
233 clock: 0,
234 };
235 dag::append_event(repo, &ref_name, &event, &sk)?;
236 Ok(()) 176 Ok(())
237 } 177 }
238 178
@@ -247,35 +187,19 @@ pub fn edit(
247 git2::Error::from_str("at least one of --title or --body must be provided").into(), 187 git2::Error::from_str("at least one of --title or --body must be provided").into(),
248 ); 188 );
249 } 189 }
250 let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;
251 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?; 190 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?;
252 let author = get_author(repo)?; 191 dag::append_action(repo, &ref_name, Action::IssueEdit {
253 let event = Event { 192 title: title.map(|s| s.to_string()),
254 timestamp: chrono::Utc::now().to_rfc3339(), 193 body: body.map(|s| s.to_string()),
255 author, 194 })?;
256 action: Action::IssueEdit {
257 title: title.map(|s| s.to_string()),
258 body: body.map(|s| s.to_string()),
259 },
260 clock: 0,
261 };
262 dag::append_event(repo, &ref_name, &event, &sk)?;
263 Ok(()) 195 Ok(())
264 } 196 }
265 197
266 pub fn comment(repo: &Repository, id_prefix: &str, body: &str) -> Result<(), crate::error::Error> { 198 pub fn comment(repo: &Repository, id_prefix: &str, body: &str) -> Result<(), crate::error::Error> {
267 let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;
268 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?; 199 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?;
269 let author = get_author(repo)?; 200 dag::append_action(repo, &ref_name, Action::IssueComment {
270 let event = Event { 201 body: body.to_string(),
271 timestamp: chrono::Utc::now().to_rfc3339(), 202 })?;
272 author,
273 action: Action::IssueComment {
274 body: body.to_string(),
275 },
276 clock: 0,
277 };
278 dag::append_event(repo, &ref_name, &event, &sk)?;
279 Ok(()) 203 Ok(())
280 } 204 }
281 205
@@ -284,18 +208,10 @@ pub fn close(
284 id_prefix: &str, 208 id_prefix: &str,
285 reason: Option<&str>, 209 reason: Option<&str>,
286 ) -> Result<(), crate::error::Error> { 210 ) -> Result<(), crate::error::Error> {
287 let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;
288 let (ref_name, id) = state::resolve_issue_ref(repo, id_prefix)?; 211 let (ref_name, id) = state::resolve_issue_ref(repo, id_prefix)?;
289 let author = get_author(repo)?; 212 dag::append_action(repo, &ref_name, Action::IssueClose {
290 let event = Event { 213 reason: reason.map(|s| s.to_string()),
291 timestamp: chrono::Utc::now().to_rfc3339(), 214 })?;
292 author,
293 action: Action::IssueClose {
294 reason: reason.map(|s| s.to_string()),
295 },
296 clock: 0,
297 };
298 dag::append_event(repo, &ref_name, &event, &sk)?;
299 // Archive the ref (move to refs/collab/archive/issues/) 215 // Archive the ref (move to refs/collab/archive/issues/)
300 if ref_name.starts_with("refs/collab/issues/") { 216 if ref_name.starts_with("refs/collab/issues/") {
301 state::archive_issue_ref(repo, &id)?; 217 state::archive_issue_ref(repo, &id)?;
@@ -310,15 +226,7 @@ pub fn delete(repo: &Repository, id_prefix: &str) -> Result<String, crate::error
310 } 226 }
311 227
312 pub fn reopen(repo: &Repository, id_prefix: &str) -> Result<(), crate::error::Error> { 228 pub fn reopen(repo: &Repository, id_prefix: &str) -> Result<(), crate::error::Error> {
313 let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;
314 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?; 229 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?;
315 let author = get_author(repo)?; 230 dag::append_action(repo, &ref_name, Action::IssueReopen)?;
316 let event = Event {
317 timestamp: chrono::Utc::now().to_rfc3339(),
318 author,
319 action: Action::IssueReopen,
320 clock: 0,
321 };
322 dag::append_event(repo, &ref_name, &event, &sk)?;
323 Ok(()) 231 Ok(())
324 } 232 }
src/lib.rs
Old New
@@ -20,7 +20,7 @@ use base64::Engine;
20 use cli::{Commands, IdentityCmd, IssueCmd, KeyCmd, PatchCmd}; 20 use cli::{Commands, IdentityCmd, IssueCmd, KeyCmd, PatchCmd};
21 use event::ReviewVerdict; 21 use event::ReviewVerdict;
22 use git2::Repository; 22 use git2::Repository;
23 use state::{IssueStatus, PatchStatus}; 23
24 24
25 /// Check if the reviewer's base ref has moved ahead of the patch's latest revision. 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> { 26 pub fn staleness_warning(repo: &Repository, patch: &state::PatchState) -> Option<String> {
@@ -64,10 +64,7 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
64 } else { 64 } else {
65 for e in &entries { 65 for e in &entries {
66 let i = &e.issue; 66 let i = &e.issue;
67 let status = match i.status { 67 let status = i.status.as_str();
68 IssueStatus::Open => "open",
69 IssueStatus::Closed => "closed",
70 };
71 let labels = if i.labels.is_empty() { 68 let labels = if i.labels.is_empty() {
72 String::new() 69 String::new()
73 } else { 70 } else {
@@ -92,11 +89,7 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
92 return Ok(()); 89 return Ok(());
93 } 90 }
94 let i = issue::show(repo, &id)?; 91 let i = issue::show(repo, &id)?;
95 let status = match i.status { 92 println!("Issue {} [{}]", &i.id[..8], i.status);
96 IssueStatus::Open => "open",
97 IssueStatus::Closed => "closed",
98 };
99 println!("Issue {} [{}]", &i.id[..8], status);
100 println!("Title: {}", i.title); 93 println!("Title: {}", i.title);
101 println!("Author: {} <{}>", i.author.name, i.author.email); 94 println!("Author: {} <{}>", i.author.name, i.author.email);
102 println!("Created: {}", i.created_at); 95 println!("Created: {}", i.created_at);
@@ -217,14 +210,9 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
217 println!("No patches found."); 210 println!("No patches found.");
218 } else { 211 } else {
219 for p in &patches { 212 for p in &patches {
220 let status = match p.status {
221 PatchStatus::Open => "open",
222 PatchStatus::Closed => "closed",
223 PatchStatus::Merged => "merged",
224 };
225 println!( 213 println!(
226 "{:.8} {:6} {} (by {})", 214 "{:.8} {:6} {} (by {})",
227 p.id, status, p.title, p.author.name 215 p.id, p.status, p.title, p.author.name
228 ); 216 );
229 } 217 }
230 } 218 }
@@ -237,13 +225,8 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
237 return Ok(()); 225 return Ok(());
238 } 226 }
239 let p = patch::show(repo, &id)?; 227 let p = patch::show(repo, &id)?;
240 let status = match p.status {
241 PatchStatus::Open => "open",
242 PatchStatus::Closed => "closed",
243 PatchStatus::Merged => "merged",
244 };
245 let rev_count = p.revisions.len(); 228 let rev_count = p.revisions.len();
246 println!("Patch {} [{}] (r{})", &p.id[..8], status, rev_count); 229 println!("Patch {} [{}] (r{})", &p.id[..8], p.status, rev_count);
247 println!("Title: {}", p.title); 230 println!("Title: {}", p.title);
248 println!("Author: {} <{}>", p.author.name, p.author.email); 231 println!("Author: {} <{}>", p.author.name, p.author.email);
249 match p.resolve_head(repo) { 232 match p.resolve_head(repo) {
@@ -289,7 +272,7 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
289 for r in &reviews { 272 for r in &reviews {
290 let rev_label = r.revision.map(|n| format!(" (r{})", n)).unwrap_or_default(); 273 let rev_label = r.revision.map(|n| format!(" (r{})", n)).unwrap_or_default();
291 println!( 274 println!(
292 "\n{} ({:?}) - {}{}:\n{}", 275 "\n{} ({}) - {}{}:\n{}",
293 r.author.name, r.verdict, r.timestamp, rev_label, r.body 276 r.author.name, r.verdict, r.timestamp, rev_label, r.body
294 ); 277 );
295 } 278 }
@@ -350,18 +333,11 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
350 Ok(()) 333 Ok(())
351 } 334 }
352 PatchCmd::Review { id, verdict, body, revision } => { 335 PatchCmd::Review { id, verdict, body, revision } => {
353 let v = match verdict.as_str() { 336 let v: ReviewVerdict = verdict.parse().map_err(|_| {
354 "approve" => ReviewVerdict::Approve, 337 git2::Error::from_str(
355 "request-changes" => ReviewVerdict::RequestChanges, 338 "verdict must be: approve, request-changes, comment, or reject",
356 "comment" => ReviewVerdict::Comment, 339 )
357 "reject" => ReviewVerdict::Reject, 340 })?;
358 _ => {
359 return Err(git2::Error::from_str(
360 "verdict must be: approve, request-changes, comment, or reject",
361 )
362 .into());
363 }
364 };
365 patch::review(repo, &id, v, &body, revision)?; 341 patch::review(repo, &id, v, &body, revision)?;
366 println!("Review submitted."); 342 println!("Review submitted.");
367 Ok(()) 343 Ok(())
@@ -381,11 +357,6 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
381 } 357 }
382 Ok(()) 358 Ok(())
383 } 359 }
384 PatchCmd::Merge { id } => {
385 let p = patch::merge(repo, &id)?;
386 println!("Patch {:.8} merged into {}.", p.id, p.base_ref);
387 Ok(())
388 }
389 PatchCmd::Close { id, reason } => { 360 PatchCmd::Close { id, reason } => {
390 patch::close(repo, &id, reason.as_deref())?; 361 patch::close(repo, &id, reason.as_deref())?;
391 println!("Patch closed."); 362 println!("Patch closed.");
src/log.rs
Old New
@@ -154,7 +154,7 @@ fn action_summary(action: &Action) -> String {
154 Some(b) => format!("revision: {}", truncate(b, 50)), 154 Some(b) => format!("revision: {}", truncate(b, 50)),
155 None => "revision".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),
159 Action::PatchInlineComment { file, line, .. } => format!("comment on {}:{}", file, line), 159 Action::PatchInlineComment { file, line, .. } => format!("comment on {}:{}", file, line),
160 Action::PatchClose { reason } => match reason { 160 Action::PatchClose { reason } => match reason {
src/patch.rs
Old New
@@ -1,6 +1,6 @@
1 use git2::{DiffFormat, Oid, Repository}; 1 use git2::{DiffFormat, Oid, Repository};
2 2
3 use crate::cli::SortMode; 3 use crate::cli::{self, SortMode};
4 use crate::dag; 4 use crate::dag;
5 use crate::error::Error; 5 use crate::error::Error;
6 use crate::event::{Action, Event, ReviewVerdict}; 6 use crate::event::{Action, Event, ReviewVerdict};
@@ -104,13 +104,11 @@ pub fn create(
104 // Get commit and tree OIDs for revision 1 104 // Get commit and tree OIDs for revision 1
105 let commit = repo.find_commit(tip_oid)?; 105 let commit = repo.find_commit(tip_oid)?;
106 let tree_oid = commit.tree()?.id(); 106 let tree_oid = commit.tree()?.id();
107 let base_oid = repo.refname_to_id(&format!("refs/heads/{}", base_ref))?;
107 108
108 let sk = signing::load_signing_key(&signing::signing_key_dir()?)?; 109 let oid = dag::create_root_action(
109 let author = get_author(repo)?; 110 repo,
110 let event = Event { 111 Action::PatchCreate {
111 timestamp: chrono::Utc::now().to_rfc3339(),
112 author,
113 action: Action::PatchCreate {
114 title: title.to_string(), 112 title: title.to_string(),
115 body: body.to_string(), 113 body: body.to_string(),
116 base_ref: base_ref.to_string(), 114 base_ref: base_ref.to_string(),
@@ -118,10 +116,9 @@ pub fn create(
118 fixes: fixes.map(|s| s.to_string()), 116 fixes: fixes.map(|s| s.to_string()),
119 commit: tip_oid.to_string(), 117 commit: tip_oid.to_string(),
120 tree: tree_oid.to_string(), 118 tree: tree_oid.to_string(),
119 base_commit: Some(base_oid.to_string()),
121 }, 120 },
122 clock: 0, 121 )?;
123 };
124 let oid = dag::create_root_event(repo, &event, &sk)?;
125 let id = oid.to_string(); 122 let id = oid.to_string();
126 let ref_name = format!("refs/collab/patches/{}", id); 123 let ref_name = format!("refs/collab/patches/{}", id);
127 repo.reference(&ref_name, oid, false, "patch create")?; 124 repo.reference(&ref_name, oid, false, "patch create")?;
@@ -141,21 +138,7 @@ pub fn list(
141 } else { 138 } else {
142 state::list_patches(repo)? 139 state::list_patches(repo)?
143 }; 140 };
144 let mut filtered: Vec<_> = patches 141 Ok(cli::filter_sort_paginate(patches, show_closed, sort, offset, limit))
145 .into_iter()
146 .filter(|p| show_closed || p.status == PatchStatus::Open)
147 .collect();
148 match sort {
149 SortMode::Recent => filtered.sort_by(|a, b| b.last_updated.cmp(&a.last_updated)),
150 SortMode::Created => filtered.sort_by(|a, b| b.created_at.cmp(&a.created_at)),
151 SortMode::Alpha => filtered.sort_by(|a, b| a.title.cmp(&b.title)),
152 }
153 let filtered = filtered
154 .into_iter()
155 .skip(offset.unwrap_or(0))
156 .take(limit.unwrap_or(usize::MAX))
157 .collect();
158 Ok(filtered)
159 } 142 }
160 143
161 pub fn list_to_writer( 144 pub fn list_to_writer(
@@ -173,15 +156,10 @@ pub fn list_to_writer(
173 return Ok(()); 156 return Ok(());
174 } 157 }
175 for p in &patches { 158 for p in &patches {
176 let status = match p.status {
177 PatchStatus::Open => "open",
178 PatchStatus::Closed => "closed",
179 PatchStatus::Merged => "merged",
180 };
181 writeln!( 159 writeln!(
182 writer, 160 writer,
183 "{:.8} {:6} {} (by {})", 161 "{:.8} {:6} {} (by {})",
184 p.id, status, p.title, p.author.name 162 p.id, p.status, p.title, p.author.name
185 ) 163 )
186 .ok(); 164 .ok();
187 } 165 }
@@ -189,21 +167,8 @@ pub fn list_to_writer(
189 } 167 }
190 168
191 pub fn list_json(repo: &Repository, show_closed: bool, show_archived: bool, sort: SortMode) -> Result<String, crate::error::Error> { 169 pub fn list_json(repo: &Repository, show_closed: bool, show_archived: bool, sort: SortMode) -> Result<String, crate::error::Error> {
192 let patches = if show_archived { 170 let patches = list(repo, show_closed, show_archived, None, None, sort)?;
193 state::list_patches_with_archived(repo)? 171 Ok(serde_json::to_string_pretty(&patches)?)
194 } else {
195 state::list_patches(repo)?
196 };
197 let mut filtered: Vec<_> = patches
198 .into_iter()
199 .filter(|p| show_closed || p.status == PatchStatus::Open)
200 .collect();
201 match sort {
202 SortMode::Recent => filtered.sort_by(|a, b| b.last_updated.cmp(&a.last_updated)),
203 SortMode::Created => filtered.sort_by(|a, b| b.created_at.cmp(&a.created_at)),
204 SortMode::Alpha => filtered.sort_by(|a, b| a.title.cmp(&b.title)),
205 }
206 Ok(serde_json::to_string_pretty(&filtered)?)
207 } 172 }
208 173
209 pub fn show_json(repo: &Repository, id_prefix: &str) -> Result<String, crate::error::Error> { 174 pub fn show_json(repo: &Repository, id_prefix: &str) -> Result<String, crate::error::Error> {
@@ -378,112 +343,6 @@ pub fn revise(
378 Ok(()) 343 Ok(())
379 } 344 }
380 345
381 pub fn merge(repo: &Repository, id_prefix: &str) -> Result<PatchState, crate::error::Error> {
382 let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;
383 let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
384 let mut p = PatchState::from_ref(repo, &ref_name, &id)?;
385
386 if p.status != PatchStatus::Open {
387 return Err(git2::Error::from_str(&format!(
388 "patch is {:?}, can only merge open patches",
389 p.status
390 ))
391 .into());
392 }
393
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
413 let head_oid = p.resolve_head(repo)?;
414 let head_commit = repo.find_commit(head_oid)
415 .map_err(|_| git2::Error::from_str("cannot resolve head commit in patch"))?;
416
417 let base_ref = format!("refs/heads/{}", p.base_ref);
418 let base_oid = repo.refname_to_id(&base_ref)?;
419
420 // Fast-forward: the base must be an ancestor of head
421 let head_oid = head_commit.id();
422 if repo.graph_descendant_of(head_oid, base_oid)? {
423 repo.reference(&base_ref, head_oid, true, "collab: merge patch")?;
424 } else if head_oid == base_oid {
425 // Already at the same point, nothing to do
426 } else {
427 // Not a fast-forward — create a merge commit on the base branch
428 let base_commit = repo.find_commit(base_oid)?;
429 let author = get_author(repo)?;
430 let sig = crate::identity::author_signature(&author)?;
431 let mut index = repo.merge_commits(&base_commit, &head_commit, None)?;
432 if index.has_conflicts() {
433 return Err(git2::Error::from_str(
434 "merge has conflicts; resolve manually then revise the patch",
435 )
436 .into());
437 }
438 let tree_oid = index.write_tree_to(repo)?;
439 let tree = repo.find_tree(tree_oid)?;
440 let msg = format!("Merge patch {:.8}: {}", id, p.title);
441 repo.commit(
442 Some(&base_ref),
443 &sig,
444 &sig,
445 &msg,
446 &tree,
447 &[&base_commit, &head_commit],
448 )?;
449 }
450
451 // Record the merge event in the patch DAG
452 let author = get_author(repo)?;
453 let event = Event {
454 timestamp: chrono::Utc::now().to_rfc3339(),
455 author: author.clone(),
456 action: Action::PatchMerge,
457 clock: 0,
458 };
459 dag::append_event(repo, &ref_name, &event, &sk)?;
460
461 // Archive the patch ref
462 if ref_name.starts_with("refs/collab/patches/") {
463 state::archive_patch_ref(repo, &id)?;
464 }
465
466 // Auto-close linked issue if present
467 if let Some(ref fixes_id) = p.fixes {
468 if let Ok((issue_ref, issue_id)) = state::resolve_issue_ref(repo, fixes_id) {
469 let close_event = Event {
470 timestamp: chrono::Utc::now().to_rfc3339(),
471 author,
472 action: Action::IssueClose {
473 reason: Some(format!("Fixed by patch {:.8}", p.id)),
474 },
475 clock: 0,
476 };
477 dag::append_event(repo, &issue_ref, &close_event, &sk)?;
478 // Archive the issue ref
479 if issue_ref.starts_with("refs/collab/issues/") {
480 state::archive_issue_ref(repo, &issue_id)?;
481 }
482 }
483 }
484
485 Ok(p)
486 }
487 346
488 /// Generate a unified diff between a patch's base branch and head commit. 347 /// Generate a unified diff between a patch's base branch and head commit.
489 pub fn diff( 348 pub fn diff(
@@ -505,25 +364,29 @@ pub fn diff(
505 } 364 }
506 } 365 }
507 366
367 /// Resolve the base tree for diffing: find the merge-base between the base branch
368 /// and the given head OID, falling back to the base branch tip if no merge-base exists.
369 fn resolve_base_tree<'a>(
370 repo: &'a Repository,
371 base_branch: &str,
372 head_oid: Oid,
373 ) -> Result<Option<git2::Tree<'a>>, Error> {
374 let base_ref = format!("refs/heads/{}", base_branch);
375 let base_oid = match repo.refname_to_id(&base_ref) {
376 Ok(oid) => oid,
377 Err(_) => return Ok(None),
378 };
379 let tree_source = repo.merge_base(base_oid, head_oid).unwrap_or(base_oid);
380 Ok(Some(repo.find_commit(tree_source)?.tree()?))
381 }
382
508 /// Generate a diff string from a patch's base and head using three-dot (merge-base) diff. 383 /// Generate a diff string from a patch's base and head using three-dot (merge-base) diff.
509 pub fn generate_diff(repo: &Repository, patch: &state::PatchState) -> Result<String, Error> { 384 pub fn generate_diff(repo: &Repository, patch: &state::PatchState) -> Result<String, Error> {
510 let head_oid = patch.resolve_head(repo)?; 385 let head_oid = patch.resolve_head(repo)?;
511 let head_commit = repo.find_commit(head_oid) 386 let head_commit = repo.find_commit(head_oid)
512 .map_err(|e| Error::Cmd(format!("bad head ref: {}", e)))?; 387 .map_err(|e| Error::Cmd(format!("bad head ref: {}", e)))?;
513 let head_tree = head_commit.tree()?; 388 let head_tree = head_commit.tree()?;
514 389 let base_tree = resolve_base_tree(repo, &patch.base_ref, head_oid)?;
515 let base_ref = format!("refs/heads/{}", patch.base_ref);
516 let base_tree = if let Ok(base_oid) = repo.refname_to_id(&base_ref) {
517 if let Ok(merge_base_oid) = repo.merge_base(base_oid, head_oid) {
518 let merge_base_commit = repo.find_commit(merge_base_oid)?;
519 Some(merge_base_commit.tree()?)
520 } else {
521 let base_commit = repo.find_commit(base_oid)?;
522 Some(base_commit.tree()?)
523 }
524 } else {
525 None
526 };
527 390
528 let git_diff = repo.diff_tree_to_tree(base_tree.as_ref(), Some(&head_tree), None)?; 391 let git_diff = repo.diff_tree_to_tree(base_tree.as_ref(), Some(&head_tree), None)?;
529 format_diff(&git_diff) 392 format_diff(&git_diff)
@@ -538,22 +401,9 @@ fn generate_diff_at_revision(
538 let revision = patch.revisions.iter().find(|r| r.number == rev_number) 401 let revision = patch.revisions.iter().find(|r| r.number == rev_number)
539 .ok_or_else(|| Error::Cmd(format!("revision {} not found", rev_number)))?; 402 .ok_or_else(|| Error::Cmd(format!("revision {} not found", rev_number)))?;
540 403
541 let tree_oid = Oid::from_str(&revision.tree)?; 404 let head_tree = repo.find_tree(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)?; 405 let commit_oid = Oid::from_str(&revision.commit)?;
546 let base_tree = if let Ok(base_oid) = repo.refname_to_id(&base_ref) { 406 let base_tree = resolve_base_tree(repo, &patch.base_ref, commit_oid)?;
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 407
558 let git_diff = repo.diff_tree_to_tree(base_tree.as_ref(), Some(&head_tree), None)?; 408 let git_diff = repo.diff_tree_to_tree(base_tree.as_ref(), Some(&head_tree), None)?;
559 format_diff(&git_diff) 409 format_diff(&git_diff)
@@ -679,18 +529,10 @@ pub fn close(
679 id_prefix: &str, 529 id_prefix: &str,
680 reason: Option<&str>, 530 reason: Option<&str>,
681 ) -> Result<(), crate::error::Error> { 531 ) -> Result<(), crate::error::Error> {
682 let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;
683 let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?; 532 let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
684 let author = get_author(repo)?; 533 dag::append_action(repo, &ref_name, Action::PatchClose {
685 let event = Event { 534 reason: reason.map(|s| s.to_string()),
686 timestamp: chrono::Utc::now().to_rfc3339(), 535 })?;
687 author,
688 action: Action::PatchClose {
689 reason: reason.map(|s| s.to_string()),
690 },
691 clock: 0,
692 };
693 dag::append_event(repo, &ref_name, &event, &sk)?;
694 // Archive the ref (move to refs/collab/archive/patches/) 536 // Archive the ref (move to refs/collab/archive/patches/)
695 if ref_name.starts_with("refs/collab/patches/") { 537 if ref_name.starts_with("refs/collab/patches/") {
696 state::archive_patch_ref(repo, &id)?; 538 state::archive_patch_ref(repo, &id)?;
@@ -698,24 +540,3 @@ pub fn close(
698 Ok(()) 540 Ok(())
699 } 541 }
700 542
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
@@ -1,3 +1,5 @@
1 use std::fmt;
2
1 use git2::{Oid, Repository}; 3 use git2::{Oid, Repository};
2 use serde::{Deserialize, Serialize}; 4 use serde::{Deserialize, Serialize};
3 5
@@ -30,27 +32,12 @@ fn deserialize_oid_option<'de, D: serde::Deserializer<'de>>(d: D) -> Result<Opti
30 } 32 }
31 33
32 fn serialize_verdict<S: serde::Serializer>(v: &ReviewVerdict, s: S) -> Result<S::Ok, S::Error> { 34 fn serialize_verdict<S: serde::Serializer>(v: &ReviewVerdict, s: S) -> Result<S::Ok, S::Error> {
33 let str_val = match v { 35 s.serialize_str(v.as_str())
34 ReviewVerdict::Approve => "approve",
35 ReviewVerdict::RequestChanges => "request-changes",
36 ReviewVerdict::Comment => "comment",
37 ReviewVerdict::Reject => "reject",
38 };
39 s.serialize_str(str_val)
40 } 36 }
41 37
42 fn deserialize_verdict<'de, D: serde::Deserializer<'de>>(d: D) -> Result<ReviewVerdict, D::Error> { 38 fn deserialize_verdict<'de, D: serde::Deserializer<'de>>(d: D) -> Result<ReviewVerdict, D::Error> {
43 let s = String::deserialize(d)?; 39 let s = String::deserialize(d)?;
44 match s.as_str() { 40 s.parse().map_err(serde::de::Error::custom)
45 "approve" => Ok(ReviewVerdict::Approve),
46 "request-changes" => Ok(ReviewVerdict::RequestChanges),
47 "comment" => Ok(ReviewVerdict::Comment),
48 "reject" => Ok(ReviewVerdict::Reject),
49 other => Err(serde::de::Error::custom(format!(
50 "unknown verdict: {}",
51 other
52 ))),
53 }
54 } 41 }
55 42
56 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] 43 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
@@ -60,6 +47,21 @@ pub enum IssueStatus {
60 Closed, 47 Closed,
61 } 48 }
62 49
50 impl IssueStatus {
51 pub fn as_str(&self) -> &'static str {
52 match self {
53 IssueStatus::Open => "open",
54 IssueStatus::Closed => "closed",
55 }
56 }
57 }
58
59 impl fmt::Display for IssueStatus {
60 fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
61 f.write_str(self.as_str())
62 }
63 }
64
63 #[derive(Debug, Clone, Serialize, Deserialize)] 65 #[derive(Debug, Clone, Serialize, Deserialize)]
64 #[allow(dead_code)] 66 #[allow(dead_code)]
65 pub struct Comment { 67 pub struct Comment {
@@ -113,6 +115,22 @@ pub enum PatchStatus {
113 Merged, 115 Merged,
114 } 116 }
115 117
118 impl PatchStatus {
119 pub fn as_str(&self) -> &'static str {
120 match self {
121 PatchStatus::Open => "open",
122 PatchStatus::Closed => "closed",
123 PatchStatus::Merged => "merged",
124 }
125 }
126 }
127
128 impl fmt::Display for PatchStatus {
129 fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
130 f.write_str(self.as_str())
131 }
132 }
133
116 #[derive(Debug, Clone, Serialize, Deserialize)] 134 #[derive(Debug, Clone, Serialize, Deserialize)]
117 pub struct Revision { 135 pub struct Revision {
118 pub number: u32, 136 pub number: u32,
@@ -142,6 +160,9 @@ pub struct PatchState {
142 pub base_ref: String, 160 pub base_ref: String,
143 pub fixes: Option<String>, 161 pub fixes: Option<String>,
144 pub branch: String, 162 pub branch: String,
163 /// Base branch tip OID at patch creation time (None for old patches).
164 #[serde(default, skip_serializing_if = "Option::is_none")]
165 pub base_commit: Option<String>,
145 pub comments: Vec<Comment>, 166 pub comments: Vec<Comment>,
146 pub inline_comments: Vec<InlineComment>, 167 pub inline_comments: Vec<InlineComment>,
147 pub reviews: Vec<Review>, 168 pub reviews: Vec<Review>,
@@ -152,6 +173,36 @@ pub struct PatchState {
152 pub author: Author, 173 pub author: Author,
153 } 174 }
154 175
176 impl crate::cli::Listable for IssueState {
177 fn is_open(&self) -> bool {
178 self.status == IssueStatus::Open
179 }
180 fn last_updated(&self) -> &str {
181 &self.last_updated
182 }
183 fn created_at(&self) -> &str {
184 &self.created_at
185 }
186 fn title(&self) -> &str {
187 &self.title
188 }
189 }
190
191 impl crate::cli::Listable for PatchState {
192 fn is_open(&self) -> bool {
193 self.status == PatchStatus::Open
194 }
195 fn last_updated(&self) -> &str {
196 &self.last_updated
197 }
198 fn created_at(&self) -> &str {
199 &self.created_at
200 }
201 fn title(&self) -> &str {
202 &self.title
203 }
204 }
205
155 impl IssueState { 206 impl IssueState {
156 pub fn from_ref( 207 pub fn from_ref(
157 repo: &Repository, 208 repo: &Repository,
@@ -317,13 +368,44 @@ impl PatchState {
317 Ok((ahead, behind)) 368 Ok((ahead, behind))
318 } 369 }
319 370
371 /// Auto-detect merge: if the patch is still Open and its head is
372 /// reachable from the base branch tip, the user merged it outside of
373 /// git-collab. We compare the current base tip to the base_commit
374 /// recorded at creation time — if it has moved to include the patch
375 /// head, the patch is merged.
376 fn check_auto_merge(&mut self, repo: &Repository) {
377 if self.status != PatchStatus::Open {
378 return;
379 }
380 let Ok(patch_head) = self.resolve_head(repo) else { return };
381 let base_ref = format!("refs/heads/{}", self.base_ref);
382 let Ok(base_tip) = repo.refname_to_id(&base_ref) else { return };
383 let base_moved = self.base_commit.as_ref()
384 .map(|bc| bc != &base_tip.to_string())
385 .unwrap_or(false);
386 let reachable = base_tip == patch_head
387 || repo.graph_descendant_of(base_tip, patch_head).unwrap_or(false);
388 if base_moved && reachable {
389 self.status = PatchStatus::Merged;
390 }
391 }
392
320 pub fn from_ref( 393 pub fn from_ref(
321 repo: &Repository, 394 repo: &Repository,
322 ref_name: &str, 395 ref_name: &str,
323 id: &str, 396 id: &str,
324 ) -> Result<Self, crate::error::Error> { 397 ) -> Result<Self, crate::error::Error> {
325 // Check cache first 398 // Check cache first
326 if let Some(cached) = cache::get_cached_state::<PatchState>(repo, ref_name) { 399 if let Some(mut cached) = cache::get_cached_state::<PatchState>(repo, ref_name) {
400 // The cache may return a stale Open status if the patch was merged
401 // outside of git-collab since the DAG tip hasn't changed.
402 cached.check_auto_merge(repo);
403 if cached.status == PatchStatus::Merged {
404 // Update the cache with the corrected status
405 if let Ok(tip) = repo.refname_to_id(ref_name) {
406 cache::set_cached_state(repo, ref_name, tip, &cached);
407 }
408 }
327 return Ok(cached); 409 return Ok(cached);
328 } 410 }
329 411
@@ -362,6 +444,7 @@ impl PatchState {
362 fixes, 444 fixes,
363 commit, 445 commit,
364 tree, 446 tree,
447 base_commit,
365 } => { 448 } => {
366 let revisions = vec![Revision { 449 let revisions = vec![Revision {
367 number: 1, 450 number: 1,
@@ -378,6 +461,7 @@ impl PatchState {
378 base_ref, 461 base_ref,
379 fixes, 462 fixes,
380 branch, 463 branch,
464 base_commit,
381 comments: Vec::new(), 465 comments: Vec::new(),
382 inline_comments: Vec::new(), 466 inline_comments: Vec::new(),
383 reviews: Vec::new(), 467 reviews: Vec::new(),
@@ -461,17 +545,17 @@ impl PatchState {
461 545
462 if let Some(ref mut s) = state { 546 if let Some(ref mut s) = state {
463 s.last_updated = max_timestamp; 547 s.last_updated = max_timestamp;
548 s.check_auto_merge(repo);
464 } 549 }
465 state.ok_or_else(|| git2::Error::from_str("no PatchCreate event found in DAG").into()) 550 state.ok_or_else(|| git2::Error::from_str("no PatchCreate event found in DAG").into())
466 } 551 }
467 } 552 }
468 553
469 /// Enumerate all collab refs of a given kind, returning (ref_name, id) pairs. 554 /// Enumerate collab refs under a given prefix, returning (ref_name, id) pairs.
470 fn collab_refs( 555 fn refs_under(
471 repo: &Repository, 556 repo: &Repository,
472 kind: &str, 557 prefix: &str,
473 ) -> Result<Vec<(String, String)>, crate::error::Error> { 558 ) -> Result<Vec<(String, String)>, crate::error::Error> {
474 let prefix = format!("refs/collab/{}/", kind);
475 let glob = format!("{}*", prefix); 559 let glob = format!("{}*", prefix);
476 let refs = repo.references_glob(&glob)?; 560 let refs = repo.references_glob(&glob)?;
477 let mut result = Vec::new(); 561 let mut result = Vec::new();
@@ -479,7 +563,7 @@ fn collab_refs(
479 let r = r?; 563 let r = r?;
480 let ref_name = r.name().unwrap_or_default().to_string(); 564 let ref_name = r.name().unwrap_or_default().to_string();
481 let id = ref_name 565 let id = ref_name
482 .strip_prefix(&prefix) 566 .strip_prefix(prefix)
483 .unwrap_or_default() 567 .unwrap_or_default()
484 .to_string(); 568 .to_string();
485 result.push((ref_name, id)); 569 result.push((ref_name, id));
@@ -487,25 +571,18 @@ fn collab_refs(
487 Ok(result) 571 Ok(result)
488 } 572 }
489 573
490 /// Enumerate all collab refs of a given kind under the archive namespace. 574 fn collab_refs(
575 repo: &Repository,
576 kind: &str,
577 ) -> Result<Vec<(String, String)>, crate::error::Error> {
578 refs_under(repo, &format!("refs/collab/{}/", kind))
579 }
580
491 fn collab_archive_refs( 581 fn collab_archive_refs(
492 repo: &Repository, 582 repo: &Repository,
493 kind: &str, 583 kind: &str,
494 ) -> Result<Vec<(String, String)>, crate::error::Error> { 584 ) -> Result<Vec<(String, String)>, crate::error::Error> {
495 let prefix = format!("refs/collab/archive/{}/", kind); 585 refs_under(repo, &format!("refs/collab/archive/{}/", kind))
496 let glob = format!("{}*", prefix);
497 let refs = repo.references_glob(&glob)?;
498 let mut result = Vec::new();
499 for r in refs {
500 let r = r?;
501 let ref_name = r.name().unwrap_or_default().to_string();
502 let id = ref_name
503 .strip_prefix(&prefix)
504 .unwrap_or_default()
505 .to_string();
506 result.push((ref_name, id));
507 }
508 Ok(result)
509 } 586 }
510 587
511 /// Resolve a short ID prefix to a full ref. Searches both active and archive namespaces. 588 /// Resolve a short ID prefix to a full ref. Searches both active and archive namespaces.
src/status.rs
Old New
@@ -80,10 +80,7 @@ pub fn compute(repo: &Repository) -> Result<ProjectStatus, Error> {
80 kind: "issue", 80 kind: "issue",
81 id: issue.id[..8.min(issue.id.len())].to_string(), 81 id: issue.id[..8.min(issue.id.len())].to_string(),
82 title: issue.title.clone(), 82 title: issue.title.clone(),
83 status: match issue.status { 83 status: issue.status.to_string(),
84 IssueStatus::Open => "open".to_string(),
85 IssueStatus::Closed => "closed".to_string(),
86 },
87 created_at: issue.created_at.clone(), 84 created_at: issue.created_at.clone(),
88 }); 85 });
89 } 86 }
@@ -92,11 +89,7 @@ pub fn compute(repo: &Repository) -> Result<ProjectStatus, Error> {
92 kind: "patch", 89 kind: "patch",
93 id: patch.id[..8.min(patch.id.len())].to_string(), 90 id: patch.id[..8.min(patch.id.len())].to_string(),
94 title: patch.title.clone(), 91 title: patch.title.clone(),
95 status: match patch.status { 92 status: patch.status.to_string(),
96 PatchStatus::Open => "open".to_string(),
97 PatchStatus::Closed => "closed".to_string(),
98 PatchStatus::Merged => "merged".to_string(),
99 },
100 created_at: patch.created_at.clone(), 93 created_at: patch.created_at.clone(),
101 }); 94 });
102 } 95 }
src/tui/events.rs
Old New
@@ -205,21 +205,13 @@ pub(crate) fn run_loop(
205 KeyAction::Reload => app.reload(repo), 205 KeyAction::Reload => app.reload(repo),
206 KeyAction::OpenPatchDetail => { 206 KeyAction::OpenPatchDetail => {
207 if let Some(patch) = app.linked_patch_for_selected().cloned() { 207 if let Some(patch) = app.linked_patch_for_selected().cloned() {
208 // Check staleness 208 open_patch_detail(app, repo, patch);
209 let warning = crate::staleness_warning(repo, &patch); 209 }
210 app.patch_revision_idx = patch.revisions.len().saturating_sub(1); 210 }
211 app.patch_interdiff_mode = false; 211 KeyAction::OpenPatchDetailDirect(idx) => {
212 app.patch_scroll = 0; 212 let patch = app.visible_patches().get(idx).cloned().cloned();
213 213 if let Some(patch) = patch {
214 // Generate initial diff (latest revision vs base) 214 open_patch_detail(app, repo, patch);
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 } 215 }
224 } 216 }
225 KeyAction::OpenCommitBrowser => { 217 KeyAction::OpenCommitBrowser => {
@@ -276,6 +268,22 @@ fn generate_patch_diff_for(
276 } 268 }
277 } 269 }
278 270
271 fn open_patch_detail(app: &mut App, repo: &Repository, patch: crate::state::PatchState) {
272 let warning = crate::staleness_warning(repo, &patch);
273 app.patch_revision_idx = patch.revisions.len().saturating_sub(1);
274 app.patch_interdiff_mode = false;
275 app.patch_scroll = 0;
276
277 let diff = generate_patch_diff_for(repo, &patch, app.patch_revision_idx, false);
278 app.patch_diff = diff;
279 app.current_patch = Some(patch);
280 app.mode = ViewMode::PatchDetail;
281
282 if let Some(w) = warning {
283 app.status_msg = Some(w);
284 }
285 }
286
279 fn regenerate_patch_diff(app: &mut App, repo: &Repository) { 287 fn regenerate_patch_diff(app: &mut App, repo: &Repository) {
280 if let Some(ref patch) = app.current_patch { 288 if let Some(ref patch) = app.current_patch {
281 let diff = generate_patch_diff_for( 289 let diff = generate_patch_diff_for(
src/tui/mod.rs
Old New
@@ -79,6 +79,7 @@ mod tests {
79 base_ref: "main".into(), 79 base_ref: "main".into(),
80 fixes: None, 80 fixes: None,
81 branch: format!("feature/{}", id), 81 branch: format!("feature/{}", id),
82 base_commit: None,
82 comments: vec![], 83 comments: vec![],
83 inline_comments: vec![], 84 inline_comments: vec![],
84 reviews: vec![], 85 reviews: vec![],
@@ -274,6 +275,7 @@ mod tests {
274 base_ref: "main".to_string(), 275 base_ref: "main".to_string(),
275 fixes: None, 276 fixes: None,
276 branch: format!("feature/p{:07x}", i), 277 branch: format!("feature/p{:07x}", i),
278 base_commit: None,
277 comments: Vec::new(), 279 comments: Vec::new(),
278 inline_comments: Vec::new(), 280 inline_comments: Vec::new(),
279 reviews: Vec::new(), 281 reviews: Vec::new(),
@@ -408,6 +410,7 @@ mod tests {
408 fixes: None, 410 fixes: None,
409 commit: "abc123".to_string(), 411 commit: "abc123".to_string(),
410 tree: "def456".to_string(), 412 tree: "def456".to_string(),
413 base_commit: None,
411 }; 414 };
412 assert_eq!(action_type_label(&action), "Patch Create"); 415 assert_eq!(action_type_label(&action), "Patch Create");
413 } 416 }
@@ -493,7 +496,7 @@ mod tests {
493 }; 496 };
494 let detail = format_event_detail(&oid, &event); 497 let detail = format_event_detail(&oid, &event);
495 assert!(detail.contains("Patch Review")); 498 assert!(detail.contains("Patch Review"));
496 assert!(detail.contains("Approve")); 499 assert!(detail.contains("approve"));
497 assert!(detail.contains("Looks good!")); 500 assert!(detail.contains("Looks good!"));
498 } 501 }
499 502
@@ -1032,6 +1035,7 @@ mod tests {
1032 base_ref: "main".into(), 1035 base_ref: "main".into(),
1033 fixes: Some("i1".into()), 1036 fixes: Some("i1".into()),
1034 branch: "feature/fix-thing".into(), 1037 branch: "feature/fix-thing".into(),
1038 base_commit: None,
1035 comments: vec![crate::state::Comment { 1039 comments: vec![crate::state::Comment {
1036 author: make_author(), 1040 author: make_author(),
1037 body: "Thread comment".into(), 1041 body: "Thread comment".into(),
src/tui/state.rs
Old New
@@ -2,7 +2,7 @@ use crossterm::event::{KeyCode, KeyModifiers};
2 use git2::{Oid, Repository}; 2 use git2::{Oid, Repository};
3 use ratatui::widgets::ListState; 3 use ratatui::widgets::ListState;
4 4
5 use crate::state::{self, IssueState, IssueStatus, PatchState}; 5 use crate::state::{self, IssueState, IssueStatus, PatchState, PatchStatus};
6 6
7 #[derive(Debug, PartialEq)] 7 #[derive(Debug, PartialEq)]
8 pub(crate) enum Pane { 8 pub(crate) enum Pane {
@@ -25,6 +25,7 @@ pub(crate) enum KeyAction {
25 Reload, 25 Reload,
26 OpenCommitBrowser, 26 OpenCommitBrowser,
27 OpenPatchDetail, 27 OpenPatchDetail,
28 OpenPatchDetailDirect(usize), // index into visible_patches
28 } 29 }
29 30
30 #[derive(Debug, PartialEq, Clone, Copy)] 31 #[derive(Debug, PartialEq, Clone, Copy)]
@@ -52,6 +53,12 @@ impl StatusFilter {
52 } 53 }
53 } 54 }
54 55
56 #[derive(Debug, PartialEq, Clone, Copy)]
57 pub(crate) enum ListMode {
58 Issues,
59 Patches,
60 }
61
55 #[derive(Debug, PartialEq)] 62 #[derive(Debug, PartialEq)]
56 pub(crate) enum InputMode { 63 pub(crate) enum InputMode {
57 Normal, 64 Normal,
@@ -64,6 +71,8 @@ pub(crate) struct App {
64 pub(crate) issues: Vec<IssueState>, 71 pub(crate) issues: Vec<IssueState>,
65 pub(crate) patches: Vec<PatchState>, 72 pub(crate) patches: Vec<PatchState>,
66 pub(crate) list_state: ListState, 73 pub(crate) list_state: ListState,
74 pub(crate) patch_list_state: ListState,
75 pub(crate) list_mode: ListMode,
67 pub(crate) scroll: u16, 76 pub(crate) scroll: u16,
68 pub(crate) pane: Pane, 77 pub(crate) pane: Pane,
69 pub(crate) mode: ViewMode, 78 pub(crate) mode: ViewMode,
@@ -89,10 +98,16 @@ impl App {
89 if !issues.is_empty() { 98 if !issues.is_empty() {
90 list_state.select(Some(0)); 99 list_state.select(Some(0));
91 } 100 }
101 let mut patch_list_state = ListState::default();
102 if !patches.is_empty() {
103 patch_list_state.select(Some(0));
104 }
92 Self { 105 Self {
93 issues, 106 issues,
94 patches, 107 patches,
95 list_state, 108 list_state,
109 patch_list_state,
110 list_mode: ListMode::Issues,
96 scroll: 0, 111 scroll: 0,
97 pane: Pane::ItemList, 112 pane: Pane::ItemList,
98 mode: ViewMode::Details, 113 mode: ViewMode::Details,
@@ -133,8 +148,23 @@ impl App {
133 .collect() 148 .collect()
134 } 149 }
135 150
151 pub(crate) fn visible_patches(&self) -> Vec<&PatchState> {
152 self.patches
153 .iter()
154 .filter(|p| match self.status_filter {
155 StatusFilter::Open => p.status == PatchStatus::Open,
156 StatusFilter::Closed => p.status == PatchStatus::Closed || p.status == PatchStatus::Merged,
157 StatusFilter::All => true,
158 })
159 .filter(|p| self.matches_search(&p.title))
160 .collect()
161 }
162
136 pub(crate) fn visible_count(&self) -> usize { 163 pub(crate) fn visible_count(&self) -> usize {
137 self.visible_issues().len() 164 match self.list_mode {
165 ListMode::Issues => self.visible_issues().len(),
166 ListMode::Patches => self.visible_patches().len(),
167 }
138 } 168 }
139 169
140 pub(crate) fn move_selection(&mut self, delta: i32) { 170 pub(crate) fn move_selection(&mut self, delta: i32) {
@@ -142,13 +172,17 @@ impl App {
142 if len == 0 { 172 if len == 0 {
143 return; 173 return;
144 } 174 }
145 let current = self.list_state.selected().unwrap_or(0); 175 let state = match self.list_mode {
176 ListMode::Issues => &mut self.list_state,
177 ListMode::Patches => &mut self.patch_list_state,
178 };
179 let current = state.selected().unwrap_or(0);
146 let new = if delta > 0 { 180 let new = if delta > 0 {
147 (current + delta as usize).min(len - 1) 181 (current + delta as usize).min(len - 1)
148 } else { 182 } else {
149 current.saturating_sub((-delta) as usize) 183 current.saturating_sub((-delta) as usize)
150 }; 184 };
151 self.list_state.select(Some(new)); 185 state.select(Some(new));
152 self.scroll = 0; 186 self.scroll = 0;
153 } 187 }
154 188
@@ -168,6 +202,7 @@ impl App {
168 match code { 202 match code {
169 KeyCode::Esc => { 203 KeyCode::Esc => {
170 self.mode = ViewMode::Details; 204 self.mode = ViewMode::Details;
205 self.pane = Pane::ItemList;
171 self.current_patch = None; 206 self.current_patch = None;
172 self.patch_diff.clear(); 207 self.patch_diff.clear();
173 self.patch_scroll = 0; 208 self.patch_scroll = 0;
@@ -301,17 +336,34 @@ impl App {
301 match code { 336 match code {
302 KeyCode::Char('q') | KeyCode::Esc => KeyAction::Quit, 337 KeyCode::Char('q') | KeyCode::Esc => KeyAction::Quit,
303 KeyCode::Char('c') if modifiers.contains(KeyModifiers::CONTROL) => KeyAction::Quit, 338 KeyCode::Char('c') if modifiers.contains(KeyModifiers::CONTROL) => KeyAction::Quit,
339 KeyCode::Char('P') => {
340 // Toggle between Issues and Patches list mode
341 self.list_mode = match self.list_mode {
342 ListMode::Issues => ListMode::Patches,
343 ListMode::Patches => ListMode::Issues,
344 };
345 self.scroll = 0;
346 self.pane = Pane::ItemList;
347 self.mode = ViewMode::Details;
348 KeyAction::Continue
349 }
304 KeyCode::Char('c') => { 350 KeyCode::Char('c') => {
305 // Open commit browser: only when in detail pane with an item selected 351 // Open commit browser: only when in detail pane with an item selected
306 if self.pane == Pane::Detail && self.list_state.selected().is_some() { 352 if self.pane == Pane::Detail
353 && self.list_mode == ListMode::Issues
354 && self.list_state.selected().is_some()
355 {
307 KeyAction::OpenCommitBrowser 356 KeyAction::OpenCommitBrowser
308 } else { 357 } else {
309 KeyAction::Continue 358 KeyAction::Continue
310 } 359 }
311 } 360 }
312 KeyCode::Char('p') => { 361 KeyCode::Char('p') => {
313 // Open patch detail: only when in detail pane with a linked patch 362 // Open patch detail: only when in detail pane with a linked patch (issues mode)
314 if self.pane == Pane::Detail && self.linked_patch_for_selected().is_some() { 363 if self.pane == Pane::Detail
364 && self.list_mode == ListMode::Issues
365 && self.linked_patch_for_selected().is_some()
366 {
315 KeyAction::OpenPatchDetail 367 KeyAction::OpenPatchDetail
316 } else { 368 } else {
317 KeyAction::Continue 369 KeyAction::Continue
@@ -342,6 +394,15 @@ impl App {
342 KeyAction::Continue 394 KeyAction::Continue
343 } 395 }
344 KeyCode::Tab | KeyCode::Enter => { 396 KeyCode::Tab | KeyCode::Enter => {
397 // In Patches list mode, entering detail pane opens patch detail directly
398 if self.list_mode == ListMode::Patches && self.pane == Pane::ItemList {
399 if let Some(idx) = self.patch_list_state.selected() {
400 let visible_len = self.visible_patches().len();
401 if idx < visible_len {
402 return KeyAction::OpenPatchDetailDirect(idx);
403 }
404 }
405 }
345 self.pane = match self.pane { 406 self.pane = match self.pane {
346 Pane::ItemList => Pane::Detail, 407 Pane::ItemList => Pane::Detail,
347 Pane::Detail => Pane::ItemList, 408 Pane::Detail => Pane::ItemList,
@@ -351,8 +412,11 @@ impl App {
351 KeyCode::Char('a') => { 412 KeyCode::Char('a') => {
352 self.status_filter = self.status_filter.next(); 413 self.status_filter = self.status_filter.next();
353 let count = self.visible_count(); 414 let count = self.visible_count();
354 self.list_state 415 let state = match self.list_mode {
355 .select(if count > 0 { Some(0) } else { None }); 416 ListMode::Issues => &mut self.list_state,
417 ListMode::Patches => &mut self.patch_list_state,
418 };
419 state.select(if count > 0 { Some(0) } else { None });
356 KeyAction::Continue 420 KeyAction::Continue
357 } 421 }
358 KeyCode::Char('r') => KeyAction::Reload, 422 KeyCode::Char('r') => KeyAction::Reload,
@@ -378,11 +442,23 @@ impl App {
378 if let Ok(patches) = state::list_patches(repo) { 442 if let Ok(patches) = state::list_patches(repo) {
379 self.patches = patches; 443 self.patches = patches;
380 } 444 }
381 let visible_len = self.visible_count(); 445 // Clamp issue list selection
446 let issue_len = self.visible_issues().len();
382 if let Some(sel) = self.list_state.selected() { 447 if let Some(sel) = self.list_state.selected() {
383 if sel >= visible_len { 448 if sel >= issue_len {
384 self.list_state.select(if visible_len > 0 { 449 self.list_state.select(if issue_len > 0 {
385 Some(visible_len - 1) 450 Some(issue_len - 1)
451 } else {
452 None
453 });
454 }
455 }
456 // Clamp patch list selection
457 let patch_len = self.visible_patches().len();
458 if let Some(sel) = self.patch_list_state.selected() {
459 if sel >= patch_len {
460 self.patch_list_state.select(if patch_len > 0 {
461 Some(patch_len - 1)
386 } else { 462 } else {
387 None 463 None
388 }); 464 });
src/tui/widgets.rs
Old New
@@ -5,7 +5,7 @@ use ratatui::widgets::{Block, Borders, List, ListItem, Paragraph, Wrap};
5 use crate::event::{Action, ReviewVerdict}; 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, ListMode, Pane, StatusFilter, ViewMode};
9 9
10 pub(crate) fn action_type_label(action: &Action) -> &str { 10 pub(crate) fn action_type_label(action: &Action) -> &str {
11 match action { 11 match action {
@@ -78,7 +78,7 @@ pub(crate) fn format_event_detail(oid: &Oid, event: &crate::event::Event) -> Str
78 } 78 }
79 } 79 }
80 Action::PatchReview { verdict, body, .. } => { 80 Action::PatchReview { verdict, body, .. } => {
81 detail.push_str(&format!("\nVerdict: {:?}\n", verdict)); 81 detail.push_str(&format!("\nVerdict: {}\n", verdict));
82 if !body.is_empty() { 82 if !body.is_empty() {
83 detail.push_str(&format!("\n{}\n", body)); 83 detail.push_str(&format!("\n{}\n", body));
84 } 84 }
@@ -146,39 +146,73 @@ fn render_list(frame: &mut Frame, app: &mut App, area: Rect) {
146 Style::default().fg(Color::DarkGray) 146 Style::default().fg(Color::DarkGray)
147 }; 147 };
148 148
149 let visible = app.visible_issues(); 149 match app.list_mode {
150 let items: Vec<ListItem> = visible 150 ListMode::Issues => {
151 .iter() 151 let visible = app.visible_issues();
152 .map(|i| { 152 let items: Vec<ListItem> = visible
153 let status = match i.status { 153 .iter()
154 IssueStatus::Open => "open", 154 .map(|i| {
155 IssueStatus::Closed => "closed", 155 let status = i.status.as_str();
156 }; 156 let style = match i.status {
157 let style = match i.status { 157 IssueStatus::Open => Style::default().fg(Color::Green),
158 IssueStatus::Open => Style::default().fg(Color::Green), 158 IssueStatus::Closed => Style::default().fg(Color::Red),
159 IssueStatus::Closed => Style::default().fg(Color::Red), 159 };
160 }; 160 ListItem::new(format!("{:.8} {:6} {}", i.id, status, i.title)).style(style)
161 ListItem::new(format!("{:.8} {:6} {}", i.id, status, i.title)).style(style) 161 })
162 }) 162 .collect();
163 .collect(); 163
164 164 let title = format!("Issues ({})", app.status_filter.label());
165 let title = format!("Issues ({})", app.status_filter.label()); 165
166 166 let list = List::new(items)
167 let list = List::new(items) 167 .block(
168 .block( 168 Block::default()
169 Block::default() 169 .borders(Borders::ALL)
170 .borders(Borders::ALL) 170 .title(title)
171 .title(title) 171 .border_style(border_style),
172 .border_style(border_style), 172 )
173 ) 173 .highlight_style(
174 .highlight_style( 174 Style::default()
175 Style::default() 175 .bg(Color::DarkGray)
176 .bg(Color::DarkGray) 176 .add_modifier(Modifier::BOLD),
177 .add_modifier(Modifier::BOLD), 177 )
178 ) 178 .highlight_symbol("> ");
179 .highlight_symbol("> "); 179
180 180 frame.render_stateful_widget(list, area, &mut app.list_state);
181 frame.render_stateful_widget(list, area, &mut app.list_state); 181 }
182 ListMode::Patches => {
183 let visible = app.visible_patches();
184 let items: Vec<ListItem> = visible
185 .iter()
186 .map(|p| {
187 let status = p.status.as_str();
188 let style = match p.status {
189 PatchStatus::Open => Style::default().fg(Color::Green),
190 PatchStatus::Closed => Style::default().fg(Color::Red),
191 PatchStatus::Merged => Style::default().fg(Color::Cyan),
192 };
193 ListItem::new(format!("{:.8} {:6} {}", p.id, status, p.title)).style(style)
194 })
195 .collect();
196
197 let title = format!("Patches ({})", app.status_filter.label());
198
199 let list = List::new(items)
200 .block(
201 Block::default()
202 .borders(Borders::ALL)
203 .title(title)
204 .border_style(border_style),
205 )
206 .highlight_style(
207 Style::default()
208 .bg(Color::DarkGray)
209 .add_modifier(Modifier::BOLD),
210 )
211 .highlight_symbol("> ");
212
213 frame.render_stateful_widget(list, area, &mut app.patch_list_state);
214 }
215 }
182 } 216 }
183 217
184 fn render_detail(frame: &mut Frame, app: &mut App, area: Rect) { 218 fn render_detail(frame: &mut Frame, app: &mut App, area: Rect) {
@@ -260,31 +294,52 @@ fn render_detail(frame: &mut Frame, app: &mut App, area: Rect) {
260 return; 294 return;
261 } 295 }
262 296
263 let visible = app.visible_issues(); 297 match app.list_mode {
264 let selected_idx = app.list_state.selected().unwrap_or(0); 298 ListMode::Issues => {
265 let content: Text = match visible.get(selected_idx) { 299 let visible = app.visible_issues();
266 Some(issue) => build_issue_detail(issue, &app.patches), 300 let selected_idx = app.list_state.selected().unwrap_or(0);
267 None => Text::raw("No matches for current filter."), 301 let content: Text = match visible.get(selected_idx) {
268 }; 302 Some(issue) => build_issue_detail(issue, &app.patches),
303 None => Text::raw("No matches for current filter."),
304 };
269 305
270 let block = Block::default() 306 let block = Block::default()
271 .borders(Borders::ALL) 307 .borders(Borders::ALL)
272 .title("Issue Details") 308 .title("Issue Details")
273 .border_style(border_style); 309 .border_style(border_style);
274 310
275 let para = Paragraph::new(content) 311 let para = Paragraph::new(content)
276 .block(block) 312 .block(block)
277 .wrap(Wrap { trim: false }) 313 .wrap(Wrap { trim: false })
278 .scroll((app.scroll, 0)); 314 .scroll((app.scroll, 0));
279 315
280 frame.render_widget(para, area); 316 frame.render_widget(para, area);
317 }
318 ListMode::Patches => {
319 let visible = app.visible_patches();
320 let selected_idx = app.patch_list_state.selected().unwrap_or(0);
321 let content: Text = match visible.get(selected_idx) {
322 Some(patch) => build_patch_summary(patch),
323 None => Text::raw("No patches for current filter."),
324 };
325
326 let block = Block::default()
327 .borders(Borders::ALL)
328 .title("Patch Details")
329 .border_style(border_style);
330
331 let para = Paragraph::new(content)
332 .block(block)
333 .wrap(Wrap { trim: false })
334 .scroll((app.scroll, 0));
335
336 frame.render_widget(para, area);
337 }
338 }
281 } 339 }
282 340
283 fn build_issue_detail(issue: &IssueState, patches: &[PatchState]) -> Text<'static> { 341 fn build_issue_detail(issue: &IssueState, patches: &[PatchState]) -> Text<'static> {
284 let status = match issue.status { 342 let status = issue.status.as_str();
285 IssueStatus::Open => "open",
286 IssueStatus::Closed => "closed",
287 };
288 343
289 let mut lines: Vec<Line> = vec![ 344 let mut lines: Vec<Line> = vec![
290 Line::from(vec![ 345 Line::from(vec![
@@ -409,17 +464,78 @@ fn build_issue_detail(issue: &IssueState, patches: &[PatchState]) -> Text<'stati
409 Text::from(lines) 464 Text::from(lines)
410 } 465 }
411 466
467 fn build_patch_summary(patch: &PatchState) -> Text<'static> {
468 let status_str = patch.status.as_str();
469 let status_color = match patch.status {
470 PatchStatus::Open => Color::Green,
471 PatchStatus::Closed => Color::Red,
472 PatchStatus::Merged => Color::Cyan,
473 };
474
475 let mut lines: Vec<Line> = vec![
476 Line::from(vec![
477 Span::styled("Patch ", Style::default().add_modifier(Modifier::BOLD)),
478 Span::styled(
479 format!("{:.8}", patch.id),
480 Style::default()
481 .fg(Color::Yellow)
482 .add_modifier(Modifier::BOLD),
483 ),
484 Span::raw(" "),
485 Span::styled(status_str, Style::default().fg(status_color)),
486 ]),
487 Line::from(vec![
488 Span::styled("Title: ", Style::default().fg(Color::DarkGray)),
489 Span::raw(patch.title.clone()),
490 ]),
491 Line::from(vec![
492 Span::styled("Author: ", Style::default().fg(Color::DarkGray)),
493 Span::raw(format!("{} <{}>", patch.author.name, patch.author.email)),
494 ]),
495 Line::from(vec![
496 Span::styled("Branch: ", Style::default().fg(Color::DarkGray)),
497 Span::raw(patch.branch.clone()),
498 ]),
499 Line::from(vec![
500 Span::styled("Base: ", Style::default().fg(Color::DarkGray)),
501 Span::raw(patch.base_ref.clone()),
502 ]),
503 Line::from(vec![
504 Span::styled("Revisions:", Style::default().fg(Color::DarkGray)),
505 Span::raw(format!(" {}", patch.revisions.len())),
506 ]),
507 ];
508
509 if let Some(ref fixes) = patch.fixes {
510 lines.push(Line::from(vec![
511 Span::styled("Fixes: ", Style::default().fg(Color::DarkGray)),
512 Span::raw(format!("{:.8}", fixes)),
513 ]));
514 }
515
516 if !patch.body.is_empty() {
517 lines.push(Line::raw(""));
518 for l in patch.body.lines() {
519 lines.push(Line::raw(l.to_string()));
520 }
521 }
522
523 lines.push(Line::raw(""));
524 lines.push(Line::styled(
525 "Press Enter to view full detail with diff",
526 Style::default().fg(Color::DarkGray),
527 ));
528
529 Text::from(lines)
530 }
531
412 fn build_patch_detail_text(app: &App) -> Text<'static> { 532 fn build_patch_detail_text(app: &App) -> Text<'static> {
413 let patch = match &app.current_patch { 533 let patch = match &app.current_patch {
414 Some(p) => p, 534 Some(p) => p,
415 None => return Text::raw("No patch loaded."), 535 None => return Text::raw("No patch loaded."),
416 }; 536 };
417 537
418 let status_str = match patch.status { 538 let status_str = patch.status.as_str();
419 PatchStatus::Open => "open",
420 PatchStatus::Closed => "closed",
421 PatchStatus::Merged => "merged",
422 };
423 let status_color = match patch.status { 539 let status_color = match patch.status {
424 PatchStatus::Open => Color::Green, 540 PatchStatus::Open => Color::Green,
425 PatchStatus::Closed => Color::Red, 541 PatchStatus::Closed => Color::Red,
@@ -515,12 +631,7 @@ fn build_patch_detail_text(app: &App) -> Text<'static> {
515 .add_modifier(Modifier::BOLD), 631 .add_modifier(Modifier::BOLD),
516 )); 632 ));
517 for review in &patch.reviews { 633 for review in &patch.reviews {
518 let verdict_str = match review.verdict { 634 let verdict_str = review.verdict.as_str();
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 { 635 let verdict_color = match review.verdict {
525 ReviewVerdict::Approve => Color::Green, 636 ReviewVerdict::Approve => Color::Green,
526 ReviewVerdict::RequestChanges => Color::Yellow, 637 ReviewVerdict::RequestChanges => Color::Yellow,
@@ -700,21 +811,30 @@ fn render_footer(frame: &mut Frame, app: &App, area: Rect) {
700 return; 811 return;
701 } 812 }
702 813
703 let mode_hint = match app.mode { 814 let text = match app.mode {
704 ViewMode::CommitList => " Esc:back", 815 ViewMode::CommitList => " j/k:navigate Enter:detail Esc:back q:quit".to_string(),
705 ViewMode::CommitDetail => " Esc:back j/k:scroll", 816 ViewMode::CommitDetail => " j/k:scroll Esc:back q:quit".to_string(),
706 ViewMode::PatchDetail => " Esc:back j/k:scroll [/]:revision d:interdiff", 817 ViewMode::PatchDetail => " j/k:scroll Esc:back [/]:revision d:interdiff q:quit".to_string(),
707 ViewMode::Details => " c:events p:patch", 818 ViewMode::Details => {
708 }; 819 let list_hint = match app.list_mode {
709 let filter_hint = match app.status_filter { 820 ListMode::Issues => "P:patches",
710 StatusFilter::Open => "a:show all", 821 ListMode::Patches => "P:issues",
711 StatusFilter::All => "a:closed", 822 };
712 StatusFilter::Closed => "a:open only", 823 let filter_hint = match app.status_filter {
824 StatusFilter::Open => "a:show all",
825 StatusFilter::All => "a:closed",
826 StatusFilter::Closed => "a:open only",
827 };
828 let mode_hint = match app.list_mode {
829 ListMode::Issues => " c:events p:patch",
830 ListMode::Patches => " Enter:view patch",
831 };
832 format!(
833 " j/k:navigate Tab:pane {} {}{} /:search r:refresh q:quit",
834 list_hint, filter_hint, mode_hint
835 )
836 }
713 }; 837 };
714 let text = format!(
715 " j/k:navigate Tab:pane {}{} /:search n:new issue o:checkout r:refresh q:quit",
716 filter_hint, mode_hint
717 );
718 let para = Paragraph::new(text).style(Style::default().bg(Color::DarkGray).fg(Color::White)); 838 let para = Paragraph::new(text).style(Style::default().bg(Color::DarkGray).fg(Color::White));
719 frame.render_widget(para, area); 839 frame.render_widget(para, area);
720 } 840 }
tests/adversarial_test.rs
Old New
@@ -544,6 +544,7 @@ fn arb_action() -> impl Strategy<Value = Action> {
544 fixes: None, 544 fixes: None,
545 commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(), 545 commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
546 tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(), 546 tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
547 base_commit: None,
547 } 548 }
548 }), 549 }),
549 proptest::option::of(".*").prop_map(|body| Action::PatchRevision { 550 proptest::option::of(".*").prop_map(|body| Action::PatchRevision {
tests/cli_test.rs
Old New
@@ -382,7 +382,7 @@ fn test_patch_review_approve() {
382 382
383 let out = repo.run_ok(&["patch", "show", &id]); 383 let out = repo.run_ok(&["patch", "show", &id]);
384 assert!(out.contains("LGTM!")); 384 assert!(out.contains("LGTM!"));
385 assert!(out.contains("Approve")); 385 assert!(out.contains("(approve)"));
386 assert!(out.contains("Reviews")); 386 assert!(out.contains("Reviews"));
387 } 387 }
388 388
@@ -402,7 +402,7 @@ fn test_patch_review_request_changes() {
402 ]); 402 ]);
403 403
404 let out = repo.run_ok(&["patch", "show", &id]); 404 let out = repo.run_ok(&["patch", "show", &id]);
405 assert!(out.contains("RequestChanges")); 405 assert!(out.contains("(request-changes)"));
406 assert!(out.contains("Needs error handling")); 406 assert!(out.contains("Needs error handling"));
407 } 407 }
408 408
@@ -489,7 +489,7 @@ fn test_patch_close() {
489 } 489 }
490 490
491 #[test] 491 #[test]
492 fn test_patch_merge_fast_forward() { 492 fn test_patch_auto_detect_merge_on_git_merge() {
493 let repo = TestRepo::new("Alice", "alice@example.com"); 493 let repo = TestRepo::new("Alice", "alice@example.com");
494 494
495 // Create a feature branch ahead of main 495 // Create a feature branch ahead of main
@@ -501,36 +501,15 @@ fn test_patch_merge_fast_forward() {
501 let out = repo.run_ok(&["patch", "create", "-t", "Add feature", "-B", "feature"]); 501 let out = repo.run_ok(&["patch", "create", "-t", "Add feature", "-B", "feature"]);
502 let id = out.trim().strip_prefix("Created patch ").unwrap(); 502 let id = out.trim().strip_prefix("Created patch ").unwrap();
503 503
504 let out = repo.run_ok(&["patch", "merge", id]); 504 // Merge via git directly
505 assert!(out.contains("merged into main")); 505 repo.git(&["merge", "feature"]);
506
507 // Main should now point at the feature commit
508 let main_head = repo.git(&["rev-parse", "main"]).trim().to_string();
509 let feature_head = repo.git(&["rev-parse", "feature"]).trim().to_string();
510 assert_eq!(main_head, feature_head);
511 506
507 // Patch should auto-detect as merged
512 let out = repo.run_ok(&["patch", "show", id]); 508 let out = repo.run_ok(&["patch", "show", id]);
513 assert!(out.contains("[merged]")); 509 assert!(out.contains("[merged]"));
514 } 510 }
515 511
516 #[test] 512 #[test]
517 fn test_patch_cannot_merge_closed() {
518 let repo = TestRepo::new("Alice", "alice@example.com");
519
520 repo.git(&["checkout", "-b", "feature"]);
521 repo.commit_file("f.txt", "f", "feature commit");
522 repo.git(&["checkout", "main"]);
523
524 let out = repo.run_ok(&["patch", "create", "-t", "Will close", "-B", "feature"]);
525 let id = out.trim().strip_prefix("Created patch ").unwrap();
526
527 repo.run_ok(&["patch", "close", id]);
528
529 let err = repo.run_err(&["patch", "merge", id]);
530 assert!(err.contains("can only merge open patches"));
531 }
532
533 #[test]
534 fn test_patch_list_empty() { 513 fn test_patch_list_empty() {
535 let repo = TestRepo::new("Alice", "alice@example.com"); 514 let repo = TestRepo::new("Alice", "alice@example.com");
536 515
@@ -566,30 +545,6 @@ fn test_patch_create_with_fixes() {
566 assert!(out.contains(&issue_id[..8]), "should show linked issue ID"); 545 assert!(out.contains(&issue_id[..8]), "should show linked issue ID");
567 } 546 }
568 547
569 #[test]
570 fn test_patch_merge_auto_closes_linked_issue() {
571 let repo = TestRepo::new("Alice", "alice@example.com");
572
573 let issue_id = repo.issue_open("Crash on startup");
574
575 repo.git(&["checkout", "-b", "fix"]);
576 repo.commit_file("fix.rs", "fixed", "fix crash");
577 repo.git(&["checkout", "main"]);
578
579 let out = repo.run_ok(&[
580 "patch", "create",
581 "-t", "Fix crash",
582 "-B", "fix",
583 "--fixes", &issue_id,
584 ]);
585 let patch_id = out.trim().strip_prefix("Created patch ").unwrap();
586
587 repo.run_ok(&["patch", "merge", patch_id]);
588
589 // Issue should now be closed
590 let out = repo.run_ok(&["issue", "show", &issue_id]);
591 assert!(out.contains("[closed]"), "linked issue should be auto-closed on merge");
592 }
593 548
594 // =========================================================================== 549 // ===========================================================================
595 // Unread tracking 550 // Unread tracking
@@ -759,16 +714,17 @@ fn test_full_patch_review_cycle() {
759 // Approve 714 // Approve
760 repo.run_ok(&["patch", "review", &id, "-v", "approve", "-b", "LGTM now"]); 715 repo.run_ok(&["patch", "review", &id, "-v", "approve", "-b", "LGTM now"]);
761 716
762 // Merge 717 // Merge via git
763 repo.run_ok(&["patch", "merge", &id]); 718 repo.git(&["checkout", "main"]);
719 repo.git(&["merge", "feature"]);
764 720
765 // Verify final state 721 // Verify final state — auto-detected as merged
766 let out = repo.run_ok(&["patch", "show", &id]); 722 let out = repo.run_ok(&["patch", "show", &id]);
767 assert!(out.contains("[merged]")); 723 assert!(out.contains("[merged]"));
768 assert!(out.contains("Added documentation")); 724 assert!(out.contains("Added documentation"));
769 assert!(out.contains("LGTM now")); 725 assert!(out.contains("LGTM now"));
770 assert!(out.contains("RequestChanges")); 726 assert!(out.contains("(request-changes)"));
771 assert!(out.contains("Approve")); 727 assert!(out.contains("(approve)"));
772 assert!(out.contains("Missing doc comment")); 728 assert!(out.contains("Missing doc comment"));
773 assert!(out.contains("Otherwise looks good")); 729 assert!(out.contains("Otherwise looks good"));
774 assert!(out.contains("Inline Comments")); 730 assert!(out.contains("Inline Comments"));
tests/collab_test.rs
Old New
@@ -643,6 +643,7 @@ fn create_branch_patch(
643 fixes: None, 643 fixes: None,
644 commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(), 644 commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
645 tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(), 645 tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
646 base_commit: None,
646 }, 647 },
647 clock: 0, 648 clock: 0,
648 }; 649 };
@@ -915,6 +916,7 @@ fn test_resolve_head_with_oid_string() {
915 fixes: None, 916 fixes: None,
916 commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(), 917 commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
917 tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(), 918 tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
919 base_commit: None,
918 }, 920 },
919 clock: 0, 921 clock: 0,
920 }; 922 };
@@ -929,57 +931,90 @@ fn test_resolve_head_with_oid_string() {
929 } 931 }
930 932
931 // --------------------------------------------------------------------------- 933 // ---------------------------------------------------------------------------
932 // Phase 5: US3 — Merge (T024-T025) 934 // Phase 5: US3 — Merge auto-detection
933 // --------------------------------------------------------------------------- 935 // ---------------------------------------------------------------------------
934 936
935 #[test] 937 #[test]
936 fn test_merge_branch_based_patch() { 938 fn test_auto_detect_merged_patch_via_git_merge() {
937 // T024: merging a branch-based patch performs git merge and updates DAG 939 // When a user merges the patch branch into the base branch manually
940 // (using git merge), PatchState should auto-detect that the patch
941 // is merged without needing `patch merge`.
938 let tmp = TempDir::new().unwrap(); 942 let tmp = TempDir::new().unwrap();
939 let repo = init_repo(tmp.path(), &alice()); 943 let repo = init_repo(tmp.path(), &alice());
940 make_initial_commit(&repo, "main"); 944 make_initial_commit(&repo, "main");
941 let tip = add_commit_on_branch(&repo, "main", "f.rs", b"x"); 945
946 let tip = add_commit_on_branch(&repo, "main", "base.rs", b"base");
942 repo.branch("feat", &repo.find_commit(tip).unwrap(), false).unwrap(); 947 repo.branch("feat", &repo.find_commit(tip).unwrap(), false).unwrap();
943 add_commit_on_branch(&repo, "feat", "feat.rs", b"feature code"); 948 add_commit_on_branch(&repo, "feat", "feat.rs", b"feature code");
944 949
945 let id = patch::create(&repo, "Merge test", "", "main", "feat", None).unwrap(); 950 // Create the patch
951 let id = patch::create(&repo, "Auto-detect test", "", "main", "feat", None).unwrap();
946 952
947 // Merge the patch 953 // Verify it's open
948 patch::merge(&repo, &id).unwrap(); 954 let ref_name = format!("refs/collab/patches/{}", id);
949 955 let state = PatchState::from_ref(&repo, &ref_name, &id).unwrap();
950 // Check that the patch is now merged (archived after merge) 956 assert_eq!(state.status, PatchStatus::Open);
951 let archive_ref = format!("refs/collab/archive/patches/{}", id);
952 let state = PatchState::from_ref(&repo, &archive_ref, &id).unwrap();
953 assert_eq!(state.status, PatchStatus::Merged);
954 957
955 // Check that the base branch now has the feature commit 958 // Manually fast-forward main to feat (simulating `git merge feat`)
956 let base_tip = repo.refname_to_id("refs/heads/main").unwrap();
957 let feat_tip = repo.refname_to_id("refs/heads/feat").unwrap(); 959 let feat_tip = repo.refname_to_id("refs/heads/feat").unwrap();
958 assert!( 960 repo.reference("refs/heads/main", feat_tip, true, "manual merge").unwrap();
959 repo.graph_descendant_of(base_tip, feat_tip).unwrap() 961
960 || base_tip == feat_tip, 962 // Now PatchState should auto-detect that it's merged
961 "base should contain the feature branch" 963 let state = PatchState::from_ref_uncached(&repo, &ref_name, &id).unwrap();
962 ); 964 assert_eq!(state.status, PatchStatus::Merged, "should auto-detect merge");
963 } 965 }
964 966
965 #[test] 967 #[test]
966 fn test_merge_conflicting_patch_reports_error() { 968 fn test_auto_detect_merged_patch_deleted_branch() {
967 // T025: merging a conflicting patch reports error 969 // If the patch branch was deleted after a manual merge,
970 // auto-detection should not crash — patch stays Open.
968 let tmp = TempDir::new().unwrap(); 971 let tmp = TempDir::new().unwrap();
969 let repo = init_repo(tmp.path(), &alice()); 972 let repo = init_repo(tmp.path(), &alice());
970 make_initial_commit(&repo, "main"); 973 make_initial_commit(&repo, "main");
971 let tip = add_commit_on_branch(&repo, "main", "conflict.rs", b"original"); 974
975 let tip = add_commit_on_branch(&repo, "main", "base.rs", b"base");
972 repo.branch("feat", &repo.find_commit(tip).unwrap(), false).unwrap(); 976 repo.branch("feat", &repo.find_commit(tip).unwrap(), false).unwrap();
977 add_commit_on_branch(&repo, "feat", "feat.rs", b"feature code");
973 978
974 // Both branches modify the same file differently 979 let id = patch::create(&repo, "Deleted branch test", "", "main", "feat", None).unwrap();
975 add_commit_on_branch(&repo, "feat", "conflict.rs", b"feature version");
976 add_commit_on_branch(&repo, "main", "conflict.rs", b"main version");
977 980
978 let id = patch::create(&repo, "Conflict test", "", "main", "feat", None).unwrap(); 981 // Delete the feature branch (simulating cleanup after merge)
979 let result = patch::merge(&repo, &id); 982 repo.find_reference("refs/heads/feat").unwrap().delete().unwrap();
980 assert!(result.is_err(), "conflicting merge should fail"); 983
981 let err_msg = result.unwrap_err().to_string(); 984 // Should not crash, patch stays Open (can't verify merge without the branch)
982 assert!(err_msg.contains("conflict"), "error should mention conflicts"); 985 let ref_name = format!("refs/collab/patches/{}", id);
986 let state = PatchState::from_ref_uncached(&repo, &ref_name, &id).unwrap();
987 assert_eq!(state.status, PatchStatus::Open);
988 }
989
990 #[test]
991 fn test_cache_does_not_defeat_auto_detect_merge() {
992 // Regression: from_ref() returned cached Open status even after the
993 // patch branch was merged into main, because the cache hit bypassed
994 // the auto-detect merge logic that only ran in from_ref_uncached().
995 let tmp = TempDir::new().unwrap();
996 let repo = init_repo(tmp.path(), &alice());
997 make_initial_commit(&repo, "main");
998
999 let tip = add_commit_on_branch(&repo, "main", "base.rs", b"base");
1000 repo.branch("feat", &repo.find_commit(tip).unwrap(), false).unwrap();
1001 add_commit_on_branch(&repo, "feat", "feat.rs", b"feature code");
1002
1003 // Create the patch via the high-level API (records base_commit)
1004 let id = patch::create(&repo, "Cache merge test", "", "main", "feat", None).unwrap();
1005 let ref_name = format!("refs/collab/patches/{}", id);
1006
1007 // First call: populates cache, status should be Open
1008 let state = PatchState::from_ref(&repo, &ref_name, &id).unwrap();
1009 assert_eq!(state.status, PatchStatus::Open);
1010
1011 // Manually fast-forward main to feat (simulating `git merge feat`)
1012 let feat_tip = repo.refname_to_id("refs/heads/feat").unwrap();
1013 repo.reference("refs/heads/main", feat_tip, true, "manual merge").unwrap();
1014
1015 // Second call: cache hit (DAG tip unchanged), but should still detect merge
1016 let state2 = PatchState::from_ref(&repo, &ref_name, &id).unwrap();
1017 assert_eq!(state2.status, PatchStatus::Merged, "cached from_ref should detect merge");
983 } 1018 }
984 1019
985 // --------------------------------------------------------------------------- 1020 // ---------------------------------------------------------------------------
tests/common/mod.rs
Old New
@@ -141,6 +141,7 @@ pub fn create_patch(repo: &Repository, author: &Author, title: &str) -> (String,
141 fixes: None, 141 fixes: None,
142 commit: commit_oid.to_string(), 142 commit: commit_oid.to_string(),
143 tree: tree_oid.to_string(), 143 tree: tree_oid.to_string(),
144 base_commit: None,
144 }, 145 },
145 clock: 0, 146 clock: 0,
146 }; 147 };
tests/crdt_test.rs
Old New
@@ -1,42 +1,14 @@
1 mod common; 1 mod common;
2 2
3 use common::{alice, bob, init_repo, test_signing_key};
3 use ed25519_dalek::SigningKey; 4 use ed25519_dalek::SigningKey;
4 use git2::{Oid, Repository}; 5 use git2::{Oid, Repository};
5 use rand_core::OsRng;
6 use tempfile::TempDir; 6 use tempfile::TempDir;
7 7
8 use git_collab::dag; 8 use git_collab::dag;
9 use git_collab::event::{Action, Author, Event}; 9 use git_collab::event::{Action, Event};
10 use git_collab::state::{IssueState, IssueStatus, PatchState, PatchStatus}; 10 use git_collab::state::{IssueState, IssueStatus, PatchState, PatchStatus};
11 11
12 fn alice() -> Author {
13 Author {
14 name: "Alice".to_string(),
15 email: "alice@example.com".to_string(),
16 }
17 }
18
19 fn bob() -> Author {
20 Author {
21 name: "Bob".to_string(),
22 email: "bob@example.com".to_string(),
23 }
24 }
25
26 fn test_sk() -> SigningKey {
27 SigningKey::generate(&mut OsRng)
28 }
29
30 fn init_repo(dir: &std::path::Path) -> Repository {
31 let repo = Repository::init(dir).unwrap();
32 {
33 let mut config = repo.config().unwrap();
34 config.set_str("user.name", "Test").unwrap();
35 config.set_str("user.email", "test@test.com").unwrap();
36 }
37 repo
38 }
39
40 fn read_event_clock(repo: &Repository, oid: Oid) -> u64 { 12 fn read_event_clock(repo: &Repository, oid: Oid) -> u64 {
41 let commit = repo.find_commit(oid).unwrap(); 13 let commit = repo.find_commit(oid).unwrap();
42 let tree = commit.tree().unwrap(); 14 let tree = commit.tree().unwrap();
@@ -51,8 +23,8 @@ fn read_event_clock(repo: &Repository, oid: Oid) -> u64 {
51 #[test] 23 #[test]
52 fn create_root_event_sets_clock_to_1() { 24 fn create_root_event_sets_clock_to_1() {
53 let dir = TempDir::new().unwrap(); 25 let dir = TempDir::new().unwrap();
54 let repo = init_repo(dir.path()); 26 let repo = init_repo(dir.path(), &alice());
55 let sk = test_sk(); 27 let sk = test_signing_key();
56 28
57 let event = Event { 29 let event = Event {
58 timestamp: "2026-01-01T00:00:00Z".to_string(), 30 timestamp: "2026-01-01T00:00:00Z".to_string(),
@@ -72,8 +44,8 @@ fn create_root_event_sets_clock_to_1() {
72 #[test] 44 #[test]
73 fn append_event_increments_clock() { 45 fn append_event_increments_clock() {
74 let dir = TempDir::new().unwrap(); 46 let dir = TempDir::new().unwrap();
75 let repo = init_repo(dir.path()); 47 let repo = init_repo(dir.path(), &alice());
76 let sk = test_sk(); 48 let sk = test_signing_key();
77 49
78 let open_event = Event { 50 let open_event = Event {
79 timestamp: "2026-01-01T00:00:00Z".to_string(), 51 timestamp: "2026-01-01T00:00:00Z".to_string(),
@@ -118,8 +90,8 @@ fn append_event_increments_clock() {
118 #[test] 90 #[test]
119 fn max_clock_returns_highest_clock_in_dag() { 91 fn max_clock_returns_highest_clock_in_dag() {
120 let dir = TempDir::new().unwrap(); 92 let dir = TempDir::new().unwrap();
121 let repo = init_repo(dir.path()); 93 let repo = init_repo(dir.path(), &alice());
122 let sk = test_sk(); 94 let sk = test_signing_key();
123 95
124 let event = Event { 96 let event = Event {
125 timestamp: "2026-01-01T00:00:00Z".to_string(), 97 timestamp: "2026-01-01T00:00:00Z".to_string(),
@@ -155,8 +127,8 @@ fn max_clock_returns_highest_clock_in_dag() {
155 #[test] 127 #[test]
156 fn reconcile_merge_clock_is_max_of_both_branches_plus_one() { 128 fn reconcile_merge_clock_is_max_of_both_branches_plus_one() {
157 let dir = TempDir::new().unwrap(); 129 let dir = TempDir::new().unwrap();
158 let repo = init_repo(dir.path()); 130 let repo = init_repo(dir.path(), &alice());
159 let sk = test_sk(); 131 let sk = test_signing_key();
160 132
161 // Create root event (clock=1) 133 // Create root event (clock=1)
162 let open = Event { 134 let open = Event {
@@ -221,8 +193,8 @@ fn concurrent_issue_close_reopen_higher_clock_wins() {
221 // the OID tiebreaker should produce a deterministic result. 193 // the OID tiebreaker should produce a deterministic result.
222 // But when clocks differ, the higher clock always wins. 194 // But when clocks differ, the higher clock always wins.
223 let dir = TempDir::new().unwrap(); 195 let dir = TempDir::new().unwrap();
224 let repo = init_repo(dir.path()); 196 let repo = init_repo(dir.path(), &alice());
225 let sk = test_sk(); 197 let sk = test_signing_key();
226 198
227 // Create root issue (clock=1) 199 // Create root issue (clock=1)
228 let open = Event { 200 let open = Event {
@@ -280,8 +252,8 @@ fn concurrent_issue_close_reopen_higher_clock_wins() {
280 fn concurrent_issue_same_clock_oid_breaks_tie() { 252 fn concurrent_issue_same_clock_oid_breaks_tie() {
281 // When two status changes have the same clock, higher OID (lexicographic) wins. 253 // When two status changes have the same clock, higher OID (lexicographic) wins.
282 let dir = TempDir::new().unwrap(); 254 let dir = TempDir::new().unwrap();
283 let repo = init_repo(dir.path()); 255 let repo = init_repo(dir.path(), &alice());
284 let sk = test_sk(); 256 let sk = test_signing_key();
285 257
286 // Create root issue (clock=1) 258 // Create root issue (clock=1)
287 let open = Event { 259 let open = Event {
@@ -338,17 +310,12 @@ fn concurrent_issue_same_clock_oid_breaks_tie() {
338 #[test] 310 #[test]
339 fn concurrent_patch_close_merge_higher_clock_wins() { 311 fn concurrent_patch_close_merge_higher_clock_wins() {
340 let dir = TempDir::new().unwrap(); 312 let dir = TempDir::new().unwrap();
341 let repo = init_repo(dir.path()); 313 let repo = init_repo(dir.path(), &alice());
342 let sk = test_sk(); 314 let sk = test_signing_key();
343 315
344 // Need a branch for the patch 316 // Need a branch for the patch
345 let sig = git2::Signature::now("Test", "test@test.com").unwrap(); 317 let main_oid = repo.refname_to_id("refs/heads/main").unwrap();
346 let tree_oid = repo.treebuilder(None).unwrap().write().unwrap(); 318 let initial_commit = repo.find_commit(main_oid).unwrap();
347 let tree = repo.find_tree(tree_oid).unwrap();
348 let initial = repo
349 .commit(Some("refs/heads/main"), &sig, &sig, "initial", &tree, &[])
350 .unwrap();
351 let initial_commit = repo.find_commit(initial).unwrap();
352 repo.branch("test-branch", &initial_commit, false).unwrap(); 319 repo.branch("test-branch", &initial_commit, false).unwrap();
353 320
354 // Create root patch (clock=1) 321 // Create root patch (clock=1)
@@ -363,6 +330,7 @@ fn concurrent_patch_close_merge_higher_clock_wins() {
363 fixes: None, 330 fixes: None,
364 commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(), 331 commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
365 tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(), 332 tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
333 base_commit: None,
366 }, 334 },
367 clock: 0, 335 clock: 0,
368 }; 336 };
@@ -413,8 +381,8 @@ fn concurrent_patch_close_merge_higher_clock_wins() {
413 #[test] 381 #[test]
414 fn migrate_clocks_assigns_sequential_clocks() { 382 fn migrate_clocks_assigns_sequential_clocks() {
415 let dir = TempDir::new().unwrap(); 383 let dir = TempDir::new().unwrap();
416 let repo = init_repo(dir.path()); 384 let repo = init_repo(dir.path(), &alice());
417 let sk = test_sk(); 385 let sk = test_signing_key();
418 386
419 // Create a DAG with events that have clock=0 (simulating pre-migration data) 387 // Create a DAG with events that have clock=0 (simulating pre-migration data)
420 // We need to create commits directly to simulate old format 388 // We need to create commits directly to simulate old format
@@ -462,8 +430,8 @@ fn migrate_clocks_assigns_sequential_clocks() {
462 fn migrate_clocks_on_zero_clock_events() { 430 fn migrate_clocks_on_zero_clock_events() {
463 // Build a DAG with clock=0 events by writing directly (bypassing DAG functions) 431 // Build a DAG with clock=0 events by writing directly (bypassing DAG functions)
464 let dir = TempDir::new().unwrap(); 432 let dir = TempDir::new().unwrap();
465 let repo = init_repo(dir.path()); 433 let repo = init_repo(dir.path(), &alice());
466 let sk = test_sk(); 434 let sk = test_signing_key();
467 435
468 let event1 = Event { 436 let event1 = Event {
469 timestamp: "2026-01-01T00:00:00Z".to_string(), 437 timestamp: "2026-01-01T00:00:00Z".to_string(),
@@ -532,8 +500,8 @@ fn clock_field_survives_serialization_round_trip() {
532 #[test] 500 #[test]
533 fn clock_field_in_dag_round_trip() { 501 fn clock_field_in_dag_round_trip() {
534 let dir = TempDir::new().unwrap(); 502 let dir = TempDir::new().unwrap();
535 let repo = init_repo(dir.path()); 503 let repo = init_repo(dir.path(), &alice());
536 let sk = test_sk(); 504 let sk = test_signing_key();
537 505
538 let event = Event { 506 let event = Event {
539 timestamp: "2026-01-01T00:00:00Z".to_string(), 507 timestamp: "2026-01-01T00:00:00Z".to_string(),
tests/revision_test.rs
Old New
@@ -518,6 +518,7 @@ fn test_concurrent_revision_dedup_after_reconcile() {
518 fixes: None, 518 fixes: None,
519 commit: head.to_string(), 519 commit: head.to_string(),
520 tree: tree_oid.to_string(), 520 tree: tree_oid.to_string(),
521 base_commit: None,
521 }, 522 },
522 clock: 0, 523 clock: 0,
523 }; 524 };
@@ -586,182 +587,5 @@ fn test_concurrent_revision_dedup_after_reconcile() {
586 assert_eq!(state.revisions[1].commit, new_commit.to_string()); 587 assert_eq!(state.revisions[1].commit, new_commit.to_string());
587 } 588 }
588 589
589 // =========================================================================== 590 // (Merge policy tests removed — policy enforcement dropped in favour
590 // D2: Merge policy enforcement (require_approval_on_latest) 591 // of auto-detect merge via git graph.)
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
@@ -212,6 +212,7 @@ fn event_json_uses_namespaced_action_types() {
212 fixes: Some("deadbeef".to_string()), 212 fixes: Some("deadbeef".to_string()),
213 commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(), 213 commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
214 tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(), 214 tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
215 base_commit: None,
215 }, 216 },
216 clock: 0, 217 clock: 0,
217 }; 218 };
tests/sort_test.rs
Old New
@@ -1,9 +1,7 @@
1 mod common; 1 mod common;
2 2
3 use common::{alice, bob, TestRepo}; 3 use common::{alice, bob, init_repo, test_signing_key, TestRepo};
4 use ed25519_dalek::SigningKey;
5 use git2::Repository; 4 use git2::Repository;
6 use rand_core::OsRng;
7 use tempfile::TempDir; 5 use tempfile::TempDir;
8 6
9 use git_collab::cli::SortMode; 7 use git_collab::cli::SortMode;
@@ -11,20 +9,6 @@ use git_collab::dag;
11 use git_collab::event::{Action, Author, Event}; 9 use git_collab::event::{Action, Author, Event};
12 use git_collab::state::{self, IssueState, PatchState}; 10 use git_collab::state::{self, IssueState, PatchState};
13 11
14 fn test_signing_key() -> SigningKey {
15 SigningKey::generate(&mut OsRng)
16 }
17
18 fn init_repo(dir: &std::path::Path, author: &Author) -> Repository {
19 let repo = Repository::init(dir).expect("init repo");
20 {
21 let mut config = repo.config().unwrap();
22 config.set_str("user.name", &author.name).unwrap();
23 config.set_str("user.email", &author.email).unwrap();
24 }
25 repo
26 }
27
28 /// Create an issue with a specific timestamp. Returns (ref_name, id). 12 /// Create an issue with a specific timestamp. Returns (ref_name, id).
29 fn open_issue_at( 13 fn open_issue_at(
30 repo: &Repository, 14 repo: &Repository,
@@ -89,6 +73,7 @@ fn create_patch_at(
89 fixes: None, 73 fixes: None,
90 commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(), 74 commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
91 tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(), 75 tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
76 base_commit: None,
92 }, 77 },
93 clock: 0, 78 clock: 0,
94 }; 79 };
tests/sync_test.rs
Old New
@@ -293,6 +293,7 @@ fn test_patch_review_across_repos() {
293 fixes: None, 293 fixes: None,
294 commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(), 294 commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
295 tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(), 295 tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
296 base_commit: None,
296 }, 297 },
297 clock: 0, 298 clock: 0,
298 }; 299 };
@@ -345,6 +346,7 @@ fn test_concurrent_review_and_revise() {
345 fixes: None, 346 fixes: None,
346 commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(), 347 commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
347 tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(), 348 tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
349 base_commit: None,
348 }, 350 },
349 clock: 0, 351 clock: 0,
350 }; 352 };