a73x

422b0460

Tidy up server handlers and test coverage

a73x   2026-04-04 16:52

Commit message
Tidy up server handlers and test coverage
benches/core_ops.rs
Old New
@@ -131,16 +131,12 @@ fn bench_list_issues(c: &mut Criterion) {
131 let mut group = c.benchmark_group("list_issues"); 131 let mut group = c.benchmark_group("list_issues");
132 for count in [10, 100, 1000] { 132 for count in [10, 100, 1000] {
133 let (repo, _dir) = setup_issues(count); 133 let (repo, _dir) = setup_issues(count);
134 group.bench_with_input( 134 group.bench_with_input(BenchmarkId::from_parameter(count), &count, |b, _| {
135 BenchmarkId::from_parameter(count), 135 b.iter(|| {
136 &count, 136 let issues = git_collab::state::list_issues(&repo).unwrap();
137 |b, _| { 137 assert_eq!(issues.len(), count);
138 b.iter(|| { 138 });
139 let issues = git_collab::state::list_issues(&repo).unwrap(); 139 });
140 assert_eq!(issues.len(), count);
141 });
142 },
143 );
144 } 140 }
145 group.finish(); 141 group.finish();
146 } 142 }
@@ -149,16 +145,12 @@ fn bench_list_patches(c: &mut Criterion) {
149 let mut group = c.benchmark_group("list_patches"); 145 let mut group = c.benchmark_group("list_patches");
150 for count in [10, 100, 1000] { 146 for count in [10, 100, 1000] {
151 let (repo, _dir) = setup_patches(count); 147 let (repo, _dir) = setup_patches(count);
152 group.bench_with_input( 148 group.bench_with_input(BenchmarkId::from_parameter(count), &count, |b, _| {
153 BenchmarkId::from_parameter(count), 149 b.iter(|| {
154 &count, 150 let patches = git_collab::state::list_patches(&repo).unwrap();
155 |b, _| { 151 assert_eq!(patches.len(), count);
156 b.iter(|| { 152 });
157 let patches = git_collab::state::list_patches(&repo).unwrap(); 153 });
158 assert_eq!(patches.len(), count);
159 });
160 },
161 );
162 } 154 }
163 group.finish(); 155 group.finish();
164 } 156 }
@@ -167,17 +159,13 @@ fn bench_walk_events(c: &mut Criterion) {
167 let mut group = c.benchmark_group("walk_events"); 159 let mut group = c.benchmark_group("walk_events");
168 for count in [10, 100, 500] { 160 for count in [10, 100, 500] {
169 let (repo, ref_name, _id, _dir) = setup_issue_with_comments(count); 161 let (repo, ref_name, _id, _dir) = setup_issue_with_comments(count);
170 group.bench_with_input( 162 group.bench_with_input(BenchmarkId::from_parameter(count), &count, |b, _| {
171 BenchmarkId::from_parameter(count), 163 b.iter(|| {
172 &count, 164 let events = dag::walk_events(&repo, &ref_name).unwrap();
173 |b, _| { 165 // 1 open + N comments
174 b.iter(|| { 166 assert_eq!(events.len(), count + 1);
175 let events = dag::walk_events(&repo, &ref_name).unwrap(); 167 });
176 // 1 open + N comments 168 });
177 assert_eq!(events.len(), count + 1);
178 });
179 },
180 );
181 } 169 }
182 group.finish(); 170 group.finish();
183 } 171 }
@@ -186,16 +174,12 @@ fn bench_issue_from_ref(c: &mut Criterion) {
186 let mut group = c.benchmark_group("issue_from_ref"); 174 let mut group = c.benchmark_group("issue_from_ref");
187 for count in [10, 100, 500] { 175 for count in [10, 100, 500] {
188 let (repo, ref_name, id, _dir) = setup_issue_with_comments(count); 176 let (repo, ref_name, id, _dir) = setup_issue_with_comments(count);
189 group.bench_with_input( 177 group.bench_with_input(BenchmarkId::from_parameter(count), &count, |b, _| {
190 BenchmarkId::from_parameter(count), 178 b.iter(|| {
191 &count, 179 let state = IssueState::from_ref(&repo, &ref_name, &id).unwrap();
192 |b, _| { 180 assert_eq!(state.comments.len(), count);
193 b.iter(|| { 181 });
194 let state = IssueState::from_ref(&repo, &ref_name, &id).unwrap(); 182 });
195 assert_eq!(state.comments.len(), count);
196 });
197 },
198 );
199 } 183 }
200 group.finish(); 184 group.finish();
201 } 185 }
@@ -241,16 +225,12 @@ fn bench_patch_from_ref(c: &mut Criterion) {
241 dag::append_event(&repo, &ref_name, &event, &sk).unwrap(); 225 dag::append_event(&repo, &ref_name, &event, &sk).unwrap();
242 } 226 }
243 227
244 group.bench_with_input( 228 group.bench_with_input(BenchmarkId::from_parameter(count), &count, |b, _| {
245 BenchmarkId::from_parameter(count), 229 b.iter(|| {
246 &count, 230 let state = PatchState::from_ref(&repo, &ref_name, &id).unwrap();
247 |b, _| { 231 assert_eq!(state.comments.len(), count);
248 b.iter(|| { 232 });
249 let state = PatchState::from_ref(&repo, &ref_name, &id).unwrap(); 233 });
250 assert_eq!(state.comments.len(), count);
251 });
252 },
253 );
254 } 234 }
255 group.finish(); 235 group.finish();
256 } 236 }
build.rs
Old New
@@ -6,9 +6,7 @@ include!("src/cli.rs");
6 6
7 fn main() { 7 fn main() {
8 let out = PathBuf::from( 8 let out = PathBuf::from(
9 env::var("MAN_OUT_DIR").unwrap_or_else(|_| { 9 env::var("MAN_OUT_DIR").unwrap_or_else(|_| env::var("OUT_DIR").expect("OUT_DIR not set")),
10 env::var("OUT_DIR").expect("OUT_DIR not set")
11 }),
12 ); 10 );
13 11
14 let cmd = <Cli as clap::CommandFactory>::command(); 12 let cmd = <Cli as clap::CommandFactory>::command();
@@ -21,8 +19,7 @@ fn generate_manpages(cmd: &clap::Command, out: &PathBuf) {
21 let mut buf = Vec::new(); 19 let mut buf = Vec::new();
22 man.render(&mut buf).expect("failed to render man page"); 20 man.render(&mut buf).expect("failed to render man page");
23 fs::create_dir_all(out).expect("failed to create man output dir"); 21 fs::create_dir_all(out).expect("failed to create man output dir");
24 fs::write(out.join(format!("{name}.1")), buf) 22 fs::write(out.join(format!("{name}.1")), buf).expect("failed to write man page");
25 .expect("failed to write man page");
26 23
27 for sub in cmd.get_subcommands() { 24 for sub in cmd.get_subcommands() {
28 if sub.is_hide_set() { 25 if sub.is_hide_set() {
src/cache.rs
Old New
@@ -26,10 +26,7 @@ struct CacheEntry {
26 /// 26 ///
27 /// Returns `Some(state)` if a cache file exists and its stored tip OID matches 27 /// Returns `Some(state)` if a cache file exists and its stored tip OID matches
28 /// the current ref tip. Returns `None` on any mismatch, missing file, or error. 28 /// the current ref tip. Returns `None` on any mismatch, missing file, or error.
29 pub fn get_cached_state<T: DeserializeOwned>( 29 pub fn get_cached_state<T: DeserializeOwned>(repo: &Repository, ref_name: &str) -> Option<T> {
30 repo: &Repository,
31 ref_name: &str,
32 ) -> Option<T> {
33 let current_tip = repo.refname_to_id(ref_name).ok()?; 30 let current_tip = repo.refname_to_id(ref_name).ok()?;
34 let path = cache_dir(repo).join(sanitize_ref_name(ref_name)); 31 let path = cache_dir(repo).join(sanitize_ref_name(ref_name));
35 let data = fs::read_to_string(&path).ok()?; 32 let data = fs::read_to_string(&path).ok()?;
@@ -43,12 +40,7 @@ pub fn get_cached_state<T: DeserializeOwned>(
43 /// Write a cached state for the given ref, keyed by the current tip OID. 40 /// Write a cached state for the given ref, keyed by the current tip OID.
44 /// 41 ///
45 /// Silently ignores errors (cache is best-effort). 42 /// Silently ignores errors (cache is best-effort).
46 pub fn set_cached_state<T: Serialize>( 43 pub fn set_cached_state<T: Serialize>(repo: &Repository, ref_name: &str, tip_oid: Oid, state: &T) {
47 repo: &Repository,
48 ref_name: &str,
49 tip_oid: Oid,
50 state: &T,
51 ) {
52 let dir = cache_dir(repo); 44 let dir = cache_dir(repo);
53 if fs::create_dir_all(&dir).is_err() { 45 if fs::create_dir_all(&dir).is_err() {
54 return; 46 return;
src/dag.rs
Old New
@@ -141,11 +141,7 @@ pub fn build_event(repo: &Repository, action: Action) -> Result<Event, Error> {
141 141
142 /// Convenience wrapper: load signing key, build event, and append it to an 142 /// Convenience wrapper: load signing key, build event, and append it to an
143 /// existing DAG ref in one call. 143 /// existing DAG ref in one call.
144 pub fn append_action( 144 pub fn append_action(repo: &Repository, ref_name: &str, action: Action) -> Result<Oid, Error> {
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()?)?; 145 let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;
150 let event = build_event(repo, action)?; 146 let event = build_event(repo, action)?;
151 append_event(repo, ref_name, &event, &sk) 147 append_event(repo, ref_name, &event, &sk)
@@ -153,10 +149,7 @@ pub fn append_action(
153 149
154 /// Convenience wrapper: load signing key, build event, and create a root 150 /// Convenience wrapper: load signing key, build event, and create a root
155 /// (orphan) DAG commit. Returns the new commit OID (entity ID). 151 /// (orphan) DAG commit. Returns the new commit OID (entity ID).
156 pub fn create_root_action( 152 pub fn create_root_action(repo: &Repository, action: Action) -> Result<Oid, Error> {
157 repo: &Repository,
158 action: Action,
159 ) -> Result<Oid, Error> {
160 let sk = signing::load_signing_key(&signing::signing_key_dir()?)?; 153 let sk = signing::load_signing_key(&signing::signing_key_dir()?)?;
161 let event = build_event(repo, action)?; 154 let event = build_event(repo, action)?;
162 create_root_event(repo, &event, &sk) 155 create_root_event(repo, &event, &sk)
src/editor.rs
Old New
@@ -27,9 +27,8 @@ fn resolve_editor_from(visual: Option<&str>, editor: Option<&str>) -> Option<Str
27 /// The editor string is split on whitespace to support editors like `code --wait`. 27 /// The editor string is split on whitespace to support editors like `code --wait`.
28 /// The command is invoked as: `<editor...> +{line} {file}`. 28 /// The command is invoked as: `<editor...> +{line} {file}`.
29 pub fn open_editor_at(file: &str, line: u32) -> Result<(), Error> { 29 pub fn open_editor_at(file: &str, line: u32) -> Result<(), Error> {
30 let editor_str = resolve_editor().ok_or_else(|| { 30 let editor_str = resolve_editor()
31 Error::Cmd("No editor configured. Set $EDITOR or $VISUAL.".to_string()) 31 .ok_or_else(|| Error::Cmd("No editor configured. Set $EDITOR or $VISUAL.".to_string()))?;
32 })?;
33 32
34 open_editor_at_with(file, line, &editor_str) 33 open_editor_at_with(file, line, &editor_str)
35 } 34 }
@@ -167,10 +166,7 @@ mod tests {
167 166
168 #[test] 167 #[test]
169 fn test_find_comment_at_scroll_all_below() { 168 fn test_find_comment_at_scroll_all_below() {
170 let comments = vec![ 169 let comments = vec![("a.rs".to_string(), 1, 30), ("b.rs".to_string(), 2, 40)];
171 ("a.rs".to_string(), 1, 30),
172 ("b.rs".to_string(), 2, 40),
173 ];
174 // Scroll at 10, all comments are below 170 // Scroll at 10, all comments are below
175 let result = find_comment_at_scroll(&comments, 10); 171 let result = find_comment_at_scroll(&comments, 10);
176 assert_eq!(result, None); 172 assert_eq!(result, None);
@@ -180,20 +176,14 @@ mod tests {
180 fn test_find_comment_at_scroll_first_wins_on_tie() { 176 fn test_find_comment_at_scroll_first_wins_on_tie() {
181 // Two comments at same rendered position -- first in list wins 177 // Two comments at same rendered position -- first in list wins
182 // because our > comparison doesn't replace when equal. 178 // because our > comparison doesn't replace when equal.
183 let comments = vec![ 179 let comments = vec![("a.rs".to_string(), 1, 10), ("b.rs".to_string(), 2, 10)];
184 ("a.rs".to_string(), 1, 10),
185 ("b.rs".to_string(), 2, 10),
186 ];
187 let result = find_comment_at_scroll(&comments, 10); 180 let result = find_comment_at_scroll(&comments, 10);
188 assert_eq!(result, Some(("a.rs".to_string(), 1))); 181 assert_eq!(result, Some(("a.rs".to_string(), 1)));
189 } 182 }
190 183
191 #[test] 184 #[test]
192 fn test_find_comment_at_scroll_scroll_at_zero() { 185 fn test_find_comment_at_scroll_scroll_at_zero() {
193 let comments = vec![ 186 let comments = vec![("a.rs".to_string(), 1, 0), ("b.rs".to_string(), 2, 5)];
194 ("a.rs".to_string(), 1, 0),
195 ("b.rs".to_string(), 2, 5),
196 ];
197 let result = find_comment_at_scroll(&comments, 0); 187 let result = find_comment_at_scroll(&comments, 0);
198 assert_eq!(result, Some(("a.rs".to_string(), 1))); 188 assert_eq!(result, Some(("a.rs".to_string(), 1)));
199 } 189 }
@@ -219,8 +209,7 @@ mod tests {
219 209
220 #[test] 210 #[test]
221 fn test_open_editor_at_file_not_found() { 211 fn test_open_editor_at_file_not_found() {
222 let result = 212 let result = open_editor_at_with("/tmp/nonexistent_file_for_test_abc123xyz.txt", 1, "true");
223 open_editor_at_with("/tmp/nonexistent_file_for_test_abc123xyz.txt", 1, "true");
224 assert!(result.is_err()); 213 assert!(result.is_err());
225 let err_msg = format!("{}", result.unwrap_err()); 214 let err_msg = format!("{}", result.unwrap_err());
226 assert!( 215 assert!(
src/event.rs
Old New
@@ -26,34 +26,22 @@ pub enum Action {
26 relates_to: Option<String>, 26 relates_to: Option<String>,
27 }, 27 },
28 #[serde(rename = "issue.comment")] 28 #[serde(rename = "issue.comment")]
29 IssueComment { 29 IssueComment { body: String },
30 body: String,
31 },
32 #[serde(rename = "issue.close")] 30 #[serde(rename = "issue.close")]
33 IssueClose { 31 IssueClose { reason: Option<String> },
34 reason: Option<String>,
35 },
36 #[serde(rename = "issue.edit")] 32 #[serde(rename = "issue.edit")]
37 IssueEdit { 33 IssueEdit {
38 title: Option<String>, 34 title: Option<String>,
39 body: Option<String>, 35 body: Option<String>,
40 }, 36 },
41 #[serde(rename = "issue.label")] 37 #[serde(rename = "issue.label")]
42 IssueLabel { 38 IssueLabel { label: String },
43 label: String,
44 },
45 #[serde(rename = "issue.unlabel")] 39 #[serde(rename = "issue.unlabel")]
46 IssueUnlabel { 40 IssueUnlabel { label: String },
47 label: String,
48 },
49 #[serde(rename = "issue.assign")] 41 #[serde(rename = "issue.assign")]
50 IssueAssign { 42 IssueAssign { assignee: String },
51 assignee: String,
52 },
53 #[serde(rename = "issue.unassign")] 43 #[serde(rename = "issue.unassign")]
54 IssueUnassign { 44 IssueUnassign { assignee: String },
55 assignee: String,
56 },
57 #[serde(rename = "issue.reopen")] 45 #[serde(rename = "issue.reopen")]
58 IssueReopen, 46 IssueReopen,
59 #[serde(rename = "patch.create", alias = "PatchCreate")] 47 #[serde(rename = "patch.create", alias = "PatchCreate")]
@@ -85,9 +73,7 @@ pub enum Action {
85 revision: u32, 73 revision: u32,
86 }, 74 },
87 #[serde(rename = "patch.comment")] 75 #[serde(rename = "patch.comment")]
88 PatchComment { 76 PatchComment { body: String },
89 body: String,
90 },
91 #[serde(rename = "patch.inline_comment")] 77 #[serde(rename = "patch.inline_comment")]
92 PatchInlineComment { 78 PatchInlineComment {
93 file: String, 79 file: String,
@@ -96,9 +82,7 @@ pub enum Action {
96 revision: u32, 82 revision: u32,
97 }, 83 },
98 #[serde(rename = "patch.close")] 84 #[serde(rename = "patch.close")]
99 PatchClose { 85 PatchClose { reason: Option<String> },
100 reason: Option<String>,
101 },
102 #[serde(rename = "patch.merge")] 86 #[serde(rename = "patch.merge")]
103 PatchMerge, 87 PatchMerge,
104 #[serde(rename = "collab.merge")] 88 #[serde(rename = "collab.merge")]
src/identity.rs
Old New
@@ -80,10 +80,7 @@ pub fn remove_alias(repo: &Repository, email: &str) -> Result<(), Error> {
80 let before = aliases.len(); 80 let before = aliases.len();
81 aliases.retain(|a| a != email); 81 aliases.retain(|a| a != email);
82 if aliases.len() == before { 82 if aliases.len() == before {
83 return Err(Error::Cmd(format!( 83 return Err(Error::Cmd(format!("alias '{}' not found", email)));
84 "alias '{}' not found",
85 email
86 )));
87 } 84 }
88 save_aliases(repo, &aliases)?; 85 save_aliases(repo, &aliases)?;
89 Ok(()) 86 Ok(())
@@ -100,8 +97,7 @@ pub fn whoami(repo: &Repository) -> Result<String, Error> {
100 match signing::signing_key_dir() { 97 match signing::signing_key_dir() {
101 Ok(config_dir) => match signing::load_verifying_key(&config_dir) { 98 Ok(config_dir) => match signing::load_verifying_key(&config_dir) {
102 Ok(vk) => { 99 Ok(vk) => {
103 let pubkey_b64 = 100 let pubkey_b64 = base64::engine::general_purpose::STANDARD.encode(vk.to_bytes());
104 base64::engine::general_purpose::STANDARD.encode(vk.to_bytes());
105 lines.push(format!("Signing key: {}", pubkey_b64)); 101 lines.push(format!("Signing key: {}", pubkey_b64));
106 } 102 }
107 Err(_) => { 103 Err(_) => {
src/issue.rs
Old New
@@ -30,7 +30,10 @@ pub struct ListEntry {
30 pub unread: Option<usize>, 30 pub unread: Option<usize>,
31 } 31 }
32 32
33 fn load_issues(repo: &Repository, show_archived: bool) -> Result<Vec<state::IssueState>, crate::error::Error> { 33 fn load_issues(
34 repo: &Repository,
35 show_archived: bool,
36 ) -> Result<Vec<state::IssueState>, crate::error::Error> {
34 if show_archived { 37 if show_archived {
35 state::list_issues_with_archived(repo) 38 state::list_issues_with_archived(repo)
36 } else { 39 } else {
@@ -106,15 +109,18 @@ fn count_unread(repo: &git2::Repository, id: &str) -> Option<usize> {
106 } 109 }
107 110
108 let mut revwalk = repo.revwalk().ok()?; 111 let mut revwalk = repo.revwalk().ok()?;
109 revwalk 112 revwalk.set_sorting(git2::Sort::TOPOLOGICAL).ok()?;
110 .set_sorting(git2::Sort::TOPOLOGICAL)
111 .ok()?;
112 revwalk.push(tip).ok()?; 113 revwalk.push(tip).ok()?;
113 revwalk.hide(seen_oid).ok()?; 114 revwalk.hide(seen_oid).ok()?;
114 Some(revwalk.count()) 115 Some(revwalk.count())
115 } 116 }
116 117
117 pub fn list_json(repo: &Repository, show_closed: bool, show_archived: bool, sort: SortMode) -> Result<String, crate::error::Error> { 118 pub fn list_json(
119 repo: &Repository,
120 show_closed: bool,
121 show_archived: bool,
122 sort: SortMode,
123 ) -> Result<String, crate::error::Error> {
118 let issues = load_issues(repo, show_archived)?; 124 let issues = load_issues(repo, show_archived)?;
119 let filtered = cli::filter_sort_paginate(issues, show_closed, sort, None, None); 125 let filtered = cli::filter_sort_paginate(issues, show_closed, sort, None, None);
120 Ok(serde_json::to_string_pretty(&filtered)?) 126 Ok(serde_json::to_string_pretty(&filtered)?)
@@ -138,17 +144,25 @@ pub fn show(repo: &Repository, id_prefix: &str) -> Result<IssueState, crate::err
138 144
139 pub fn label(repo: &Repository, id_prefix: &str, label: &str) -> Result<(), crate::error::Error> { 145 pub fn label(repo: &Repository, id_prefix: &str, label: &str) -> Result<(), crate::error::Error> {
140 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?; 146 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?;
141 dag::append_action(repo, &ref_name, Action::IssueLabel { 147 dag::append_action(
142 label: label.to_string(), 148 repo,
143 })?; 149 &ref_name,
150 Action::IssueLabel {
151 label: label.to_string(),
152 },
153 )?;
144 Ok(()) 154 Ok(())
145 } 155 }
146 156
147 pub fn unlabel(repo: &Repository, id_prefix: &str, label: &str) -> Result<(), crate::error::Error> { 157 pub fn unlabel(repo: &Repository, id_prefix: &str, label: &str) -> Result<(), crate::error::Error> {
148 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?; 158 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?;
149 dag::append_action(repo, &ref_name, Action::IssueUnlabel { 159 dag::append_action(
150 label: label.to_string(), 160 repo,
151 })?; 161 &ref_name,
162 Action::IssueUnlabel {
163 label: label.to_string(),
164 },
165 )?;
152 Ok(()) 166 Ok(())
153 } 167 }
154 168
@@ -158,9 +172,13 @@ pub fn assign(
158 assignee: &str, 172 assignee: &str,
159 ) -> Result<(), crate::error::Error> { 173 ) -> Result<(), crate::error::Error> {
160 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?; 174 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?;
161 dag::append_action(repo, &ref_name, Action::IssueAssign { 175 dag::append_action(
162 assignee: assignee.to_string(), 176 repo,
163 })?; 177 &ref_name,
178 Action::IssueAssign {
179 assignee: assignee.to_string(),
180 },
181 )?;
164 Ok(()) 182 Ok(())
165 } 183 }
166 184
@@ -170,9 +188,13 @@ pub fn unassign(
170 assignee: &str, 188 assignee: &str,
171 ) -> Result<(), crate::error::Error> { 189 ) -> Result<(), crate::error::Error> {
172 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?; 190 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?;
173 dag::append_action(repo, &ref_name, Action::IssueUnassign { 191 dag::append_action(
174 assignee: assignee.to_string(), 192 repo,
175 })?; 193 &ref_name,
194 Action::IssueUnassign {
195 assignee: assignee.to_string(),
196 },
197 )?;
176 Ok(()) 198 Ok(())
177 } 199 }
178 200
@@ -188,18 +210,26 @@ pub fn edit(
188 ); 210 );
189 } 211 }
190 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?; 212 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?;
191 dag::append_action(repo, &ref_name, Action::IssueEdit { 213 dag::append_action(
192 title: title.map(|s| s.to_string()), 214 repo,
193 body: body.map(|s| s.to_string()), 215 &ref_name,
194 })?; 216 Action::IssueEdit {
217 title: title.map(|s| s.to_string()),
218 body: body.map(|s| s.to_string()),
219 },
220 )?;
195 Ok(()) 221 Ok(())
196 } 222 }
197 223
198 pub fn comment(repo: &Repository, id_prefix: &str, body: &str) -> Result<(), crate::error::Error> { 224 pub fn comment(repo: &Repository, id_prefix: &str, body: &str) -> Result<(), crate::error::Error> {
199 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?; 225 let (ref_name, _id) = state::resolve_issue_ref(repo, id_prefix)?;
200 dag::append_action(repo, &ref_name, Action::IssueComment { 226 dag::append_action(
201 body: body.to_string(), 227 repo,
202 })?; 228 &ref_name,
229 Action::IssueComment {
230 body: body.to_string(),
231 },
232 )?;
203 Ok(()) 233 Ok(())
204 } 234 }
205 235
@@ -209,9 +239,13 @@ pub fn close(
209 reason: Option<&str>, 239 reason: Option<&str>,
210 ) -> Result<(), crate::error::Error> { 240 ) -> Result<(), crate::error::Error> {
211 let (ref_name, id) = state::resolve_issue_ref(repo, id_prefix)?; 241 let (ref_name, id) = state::resolve_issue_ref(repo, id_prefix)?;
212 dag::append_action(repo, &ref_name, Action::IssueClose { 242 dag::append_action(
213 reason: reason.map(|s| s.to_string()), 243 repo,
214 })?; 244 &ref_name,
245 Action::IssueClose {
246 reason: reason.map(|s| s.to_string()),
247 },
248 )?;
215 // Archive the ref (move to refs/collab/archive/issues/) 249 // Archive the ref (move to refs/collab/archive/issues/)
216 if ref_name.starts_with("refs/collab/issues/") { 250 if ref_name.starts_with("refs/collab/issues/") {
217 state::archive_issue_ref(repo, &id)?; 251 state::archive_issue_ref(repo, &id)?;
src/lib.rs
Old New
@@ -8,8 +8,8 @@ pub mod identity;
8 pub mod issue; 8 pub mod issue;
9 pub mod log; 9 pub mod log;
10 pub mod patch; 10 pub mod patch;
11 pub mod state;
12 pub mod signing; 11 pub mod signing;
12 pub mod state;
13 pub mod status; 13 pub mod status;
14 pub mod sync; 14 pub mod sync;
15 pub mod sync_lock; 15 pub mod sync_lock;
@@ -21,7 +21,6 @@ use cli::{Commands, IdentityCmd, IssueCmd, KeyCmd, PatchCmd};
21 use event::ReviewVerdict; 21 use event::ReviewVerdict;
22 use git2::Repository; 22 use git2::Repository;
23 23
24
25 /// Check if the reviewer's base ref has moved ahead of the patch's latest revision. 24 /// 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> { 25 pub fn staleness_warning(repo: &Repository, patch: &state::PatchState) -> Option<String> {
27 let latest_commit = &patch.revisions.last()?.commit; 26 let latest_commit = &patch.revisions.last()?.commit;
@@ -47,12 +46,23 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
47 match cli.command { 46 match cli.command {
48 Commands::Init => sync::init(repo), 47 Commands::Init => sync::init(repo),
49 Commands::Issue(cmd) => match cmd { 48 Commands::Issue(cmd) => match cmd {
50 IssueCmd::Open { title, body, relates_to } => { 49 IssueCmd::Open {
50 title,
51 body,
52 relates_to,
53 } => {
51 let id = issue::open(repo, &title, &body, relates_to.as_deref())?; 54 let id = issue::open(repo, &title, &body, relates_to.as_deref())?;
52 println!("Opened issue {:.8}", id); 55 println!("Opened issue {:.8}", id);
53 Ok(()) 56 Ok(())
54 } 57 }
55 IssueCmd::List { all, archived, limit, offset, json, sort } => { 58 IssueCmd::List {
59 all,
60 archived,
61 limit,
62 offset,
63 json,
64 sort,
65 } => {
56 if json { 66 if json {
57 let output = issue::list_json(repo, all, archived, sort)?; 67 let output = issue::list_json(repo, all, archived, sort)?;
58 println!("{}", output); 68 println!("{}", output);
@@ -171,7 +181,9 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
171 } else { 181 } else {
172 // Default to current branch 182 // Default to current branch
173 let head_ref = repo.head().map_err(|_| { 183 let head_ref = repo.head().map_err(|_| {
174 error::Error::Cmd("cannot determine current branch (detached HEAD?)".to_string()) 184 error::Error::Cmd(
185 "cannot determine current branch (detached HEAD?)".to_string(),
186 )
175 })?; 187 })?;
176 if head_ref.is_branch() { 188 if head_ref.is_branch() {
177 let name = head_ref.shorthand().ok_or_else(|| { 189 let name = head_ref.shorthand().ok_or_else(|| {
@@ -199,7 +211,14 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
199 println!("Created patch {:.8}", id); 211 println!("Created patch {:.8}", id);
200 Ok(()) 212 Ok(())
201 } 213 }
202 PatchCmd::List { all, archived, limit, offset, json, sort } => { 214 PatchCmd::List {
215 all,
216 archived,
217 limit,
218 offset,
219 json,
220 sort,
221 } => {
203 if json { 222 if json {
204 let output = patch::list_json(repo, all, archived, sort)?; 223 let output = patch::list_json(repo, all, archived, sort)?;
205 println!("{}", output); 224 println!("{}", output);
@@ -233,8 +252,15 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
233 Ok(_) => { 252 Ok(_) => {
234 println!("Branch: {} -> {}", p.branch, p.base_ref); 253 println!("Branch: {} -> {}", p.branch, p.base_ref);
235 if let Ok((ahead, behind)) = p.staleness(repo) { 254 if let Ok((ahead, behind)) = p.staleness(repo) {
236 let freshness = if behind == 0 { "up-to-date" } else { "outdated" }; 255 let freshness = if behind == 0 {
237 println!("Commits: {} ahead, {} behind ({})", ahead, behind, freshness); 256 "up-to-date"
257 } else {
258 "outdated"
259 };
260 println!(
261 "Commits: {} ahead, {} behind ({})",
262 ahead, behind, freshness
263 );
238 } 264 }
239 } 265 }
240 Err(_) => { 266 Err(_) => {
@@ -253,9 +279,20 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
253 if !p.revisions.is_empty() { 279 if !p.revisions.is_empty() {
254 println!("\n--- Revisions ---"); 280 println!("\n--- Revisions ---");
255 for rev in &p.revisions { 281 for rev in &p.revisions {
256 let short = if rev.commit.len() >= 8 { &rev.commit[..8] } else { &rev.commit }; 282 let short = if rev.commit.len() >= 8 {
257 let body_display = rev.body.as_deref().map(|b| format!(" {}", b)).unwrap_or_default(); 283 &rev.commit[..8]
258 println!(" r{}: {} ({}){}", rev.number, short, rev.timestamp, body_display); 284 } else {
285 &rev.commit
286 };
287 let body_display = rev
288 .body
289 .as_deref()
290 .map(|b| format!(" {}", b))
291 .unwrap_or_default();
292 println!(
293 " r{}: {} ({}){}",
294 rev.number, short, rev.timestamp, body_display
295 );
259 } 296 }
260 } 297 }
261 if !p.body.is_empty() { 298 if !p.body.is_empty() {
@@ -263,14 +300,18 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
263 } 300 }
264 // Filter reviews by revision if requested 301 // Filter reviews by revision if requested
265 let reviews: Vec<_> = if let Some(rev) = revision { 302 let reviews: Vec<_> = if let Some(rev) = revision {
266 p.reviews.iter().filter(|r| r.revision == Some(rev)).collect() 303 p.reviews
304 .iter()
305 .filter(|r| r.revision == Some(rev))
306 .collect()
267 } else { 307 } else {
268 p.reviews.iter().collect() 308 p.reviews.iter().collect()
269 }; 309 };
270 if !reviews.is_empty() { 310 if !reviews.is_empty() {
271 println!("\n--- Reviews ---"); 311 println!("\n--- Reviews ---");
272 for r in &reviews { 312 for r in &reviews {
273 let rev_label = r.revision.map(|n| format!(" (r{})", n)).unwrap_or_default(); 313 let rev_label =
314 r.revision.map(|n| format!(" (r{})", n)).unwrap_or_default();
274 println!( 315 println!(
275 "\n{} ({}) - {}{}:\n{}", 316 "\n{} ({}) - {}{}:\n{}",
276 r.author.name, r.verdict, r.timestamp, rev_label, r.body 317 r.author.name, r.verdict, r.timestamp, rev_label, r.body
@@ -279,7 +320,10 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
279 } 320 }
280 // Filter inline comments by revision if requested 321 // Filter inline comments by revision if requested
281 let inline_comments: Vec<_> = if let Some(rev) = revision { 322 let inline_comments: Vec<_> = if let Some(rev) = revision {
282 p.inline_comments.iter().filter(|c| c.revision == Some(rev)).collect() 323 p.inline_comments
324 .iter()
325 .filter(|c| c.revision == Some(rev))
326 .collect()
283 } else { 327 } else {
284 p.inline_comments.iter().collect() 328 p.inline_comments.iter().collect()
285 }; 329 };
@@ -302,7 +346,11 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
302 } 346 }
303 Ok(()) 347 Ok(())
304 } 348 }
305 PatchCmd::Diff { id, revision, between } => { 349 PatchCmd::Diff {
350 id,
351 revision,
352 between,
353 } => {
306 if revision.is_some() && between.is_some() { 354 if revision.is_some() && between.is_some() {
307 return Err(error::Error::Cmd( 355 return Err(error::Error::Cmd(
308 "--revision and --between are mutually exclusive".to_string(), 356 "--revision and --between are mutually exclusive".to_string(),
@@ -332,7 +380,12 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
332 println!("Comment added."); 380 println!("Comment added.");
333 Ok(()) 381 Ok(())
334 } 382 }
335 PatchCmd::Review { id, verdict, body, revision } => { 383 PatchCmd::Review {
384 id,
385 verdict,
386 body,
387 revision,
388 } => {
336 let v: ReviewVerdict = verdict.parse().map_err(|_| { 389 let v: ReviewVerdict = verdict.parse().map_err(|_| {
337 git2::Error::from_str( 390 git2::Error::from_str(
338 "verdict must be: approve, request-changes, comment, or reject", 391 "verdict must be: approve, request-changes, comment, or reject",
@@ -387,8 +440,7 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
387 } 440 }
388 441
389 let vk = signing::generate_keypair(&config_dir)?; 442 let vk = signing::generate_keypair(&config_dir)?;
390 let pubkey_b64 = 443 let pubkey_b64 = base64::engine::general_purpose::STANDARD.encode(vk.to_bytes());
391 base64::engine::general_purpose::STANDARD.encode(vk.to_bytes());
392 println!("Signing key generated."); 444 println!("Signing key generated.");
393 println!("Public key: {}", pubkey_b64); 445 println!("Public key: {}", pubkey_b64);
394 Ok(()) 446 Ok(())
@@ -521,7 +573,10 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> {
521 .map(|l| format!(" ({})", l)) 573 .map(|l| format!(" ({})", l))
522 .unwrap_or_default(); 574 .unwrap_or_default();
523 let scope = if global { " (global)" } else { "" }; 575 let scope = if global { " (global)" } else { "" };
524 println!("Removed trusted key{}: {}{}", scope, removed.pubkey, label_display); 576 println!(
577 "Removed trusted key{}: {}{}",
578 scope, removed.pubkey, label_display
579 );
525 Ok(()) 580 Ok(())
526 } 581 }
527 }, 582 },
src/patch.rs
Old New
@@ -25,7 +25,11 @@ fn auto_detect_revision(
25 Err(_) => return Ok(None), // Branch deleted or unavailable 25 Err(_) => return Ok(None), // Branch deleted or unavailable
26 }; 26 };
27 27
28 let last_commit = patch.revisions.last().map(|r| r.commit.as_str()).unwrap_or(""); 28 let last_commit = patch
29 .revisions
30 .last()
31 .map(|r| r.commit.as_str())
32 .unwrap_or("");
29 let tip_hex = tip_oid.to_string(); 33 let tip_hex = tip_oid.to_string();
30 34
31 if tip_hex == last_commit { 35 if tip_hex == last_commit {
@@ -87,7 +91,8 @@ pub fn create(
87 91
88 // Verify branch exists and get tip 92 // Verify branch exists and get tip
89 let branch_ref = format!("refs/heads/{}", branch); 93 let branch_ref = format!("refs/heads/{}", branch);
90 let tip_oid = repo.refname_to_id(&branch_ref) 94 let tip_oid = repo
95 .refname_to_id(&branch_ref)
91 .map_err(|e| crate::error::Error::Cmd(format!("branch '{}' not found: {}", branch, e)))?; 96 .map_err(|e| crate::error::Error::Cmd(format!("branch '{}' not found: {}", branch, e)))?;
92 97
93 // Check for duplicate: scan open patches for matching branch 98 // Check for duplicate: scan open patches for matching branch
@@ -138,7 +143,13 @@ pub fn list(
138 } else { 143 } else {
139 state::list_patches(repo)? 144 state::list_patches(repo)?
140 }; 145 };
141 Ok(cli::filter_sort_paginate(patches, show_closed, sort, offset, limit)) 146 Ok(cli::filter_sort_paginate(
147 patches,
148 show_closed,
149 sort,
150 offset,
151 limit,
152 ))
142 } 153 }
143 154
144 pub fn list_to_writer( 155 pub fn list_to_writer(
@@ -166,7 +177,12 @@ pub fn list_to_writer(
166 Ok(()) 177 Ok(())
167 } 178 }
168 179
169 pub fn list_json(repo: &Repository, show_closed: bool, show_archived: bool, sort: SortMode) -> Result<String, crate::error::Error> { 180 pub fn list_json(
181 repo: &Repository,
182 show_closed: bool,
183 show_archived: bool,
184 sort: SortMode,
185 ) -> Result<String, crate::error::Error> {
170 let patches = list(repo, show_closed, show_archived, None, None, sort)?; 186 let patches = list(repo, show_closed, show_archived, None, None, sort)?;
171 Ok(serde_json::to_string_pretty(&patches)?) 187 Ok(serde_json::to_string_pretty(&patches)?)
172 } 188 }
@@ -316,7 +332,11 @@ pub fn revise(
316 // Resolve current branch tip 332 // Resolve current branch tip
317 let tip_oid = patch.resolve_head(repo)?; 333 let tip_oid = patch.resolve_head(repo)?;
318 let tip_hex = tip_oid.to_string(); 334 let tip_hex = tip_oid.to_string();
319 let last_commit = patch.revisions.last().map(|r| r.commit.as_str()).unwrap_or(""); 335 let last_commit = patch
336 .revisions
337 .last()
338 .map(|r| r.commit.as_str())
339 .unwrap_or("");
320 340
321 if tip_hex == last_commit { 341 if tip_hex == last_commit {
322 let current_rev = patch.revisions.last().map(|r| r.number).unwrap_or(0); 342 let current_rev = patch.revisions.last().map(|r| r.number).unwrap_or(0);
@@ -343,7 +363,6 @@ pub fn revise(
343 Ok(()) 363 Ok(())
344 } 364 }
345 365
346
347 /// Generate a unified diff between a patch's base branch and head commit. 366 /// Generate a unified diff between a patch's base branch and head commit.
348 pub fn diff( 367 pub fn diff(
349 repo: &Repository, 368 repo: &Repository,
@@ -383,7 +402,8 @@ fn resolve_base_tree<'a>(
383 /// Generate a diff string from a patch's base and head using three-dot (merge-base) diff. 402 /// Generate a diff string from a patch's base and head using three-dot (merge-base) diff.
384 pub fn generate_diff(repo: &Repository, patch: &state::PatchState) -> Result<String, Error> { 403 pub fn generate_diff(repo: &Repository, patch: &state::PatchState) -> Result<String, Error> {
385 let head_oid = patch.resolve_head(repo)?; 404 let head_oid = patch.resolve_head(repo)?;
386 let head_commit = repo.find_commit(head_oid) 405 let head_commit = repo
406 .find_commit(head_oid)
387 .map_err(|e| Error::Cmd(format!("bad head ref: {}", e)))?; 407 .map_err(|e| Error::Cmd(format!("bad head ref: {}", e)))?;
388 let head_tree = head_commit.tree()?; 408 let head_tree = head_commit.tree()?;
389 let base_tree = resolve_base_tree(repo, &patch.base_ref, head_oid)?; 409 let base_tree = resolve_base_tree(repo, &patch.base_ref, head_oid)?;
@@ -398,7 +418,10 @@ fn generate_diff_at_revision(
398 patch: &PatchState, 418 patch: &PatchState,
399 rev_number: u32, 419 rev_number: u32,
400 ) -> Result<String, Error> { 420 ) -> Result<String, Error> {
401 let revision = patch.revisions.iter().find(|r| r.number == rev_number) 421 let revision = patch
422 .revisions
423 .iter()
424 .find(|r| r.number == rev_number)
402 .ok_or_else(|| Error::Cmd(format!("revision {} not found", rev_number)))?; 425 .ok_or_else(|| Error::Cmd(format!("revision {} not found", rev_number)))?;
403 426
404 let head_tree = repo.find_tree(Oid::from_str(&revision.tree)?)?; 427 let head_tree = repo.find_tree(Oid::from_str(&revision.tree)?)?;
@@ -416,9 +439,15 @@ pub fn interdiff(
416 from_rev: u32, 439 from_rev: u32,
417 to_rev: u32, 440 to_rev: u32,
418 ) -> Result<String, Error> { 441 ) -> Result<String, Error> {
419 let from = patch.revisions.iter().find(|r| r.number == from_rev) 442 let from = patch
443 .revisions
444 .iter()
445 .find(|r| r.number == from_rev)
420 .ok_or_else(|| Error::Cmd(format!("revision {} not found", from_rev)))?; 446 .ok_or_else(|| Error::Cmd(format!("revision {} not found", from_rev)))?;
421 let to = patch.revisions.iter().find(|r| r.number == to_rev) 447 let to = patch
448 .revisions
449 .iter()
450 .find(|r| r.number == to_rev)
422 .ok_or_else(|| Error::Cmd(format!("revision {} not found", to_rev)))?; 451 .ok_or_else(|| Error::Cmd(format!("revision {} not found", to_rev)))?;
423 452
424 let from_tree_oid = Oid::from_str(&from.tree)?; 453 let from_tree_oid = Oid::from_str(&from.tree)?;
@@ -460,10 +489,7 @@ fn format_diff(git_diff: &git2::Diff) -> Result<String, Error> {
460 } 489 }
461 490
462 /// Patch log: list all revisions with timestamps and file-change summaries. 491 /// Patch log: list all revisions with timestamps and file-change summaries.
463 pub fn patch_log( 492 pub fn patch_log(repo: &Repository, id_prefix: &str) -> Result<PatchState, Error> {
464 repo: &Repository,
465 id_prefix: &str,
466 ) -> Result<PatchState, Error> {
467 let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?; 493 let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
468 PatchState::from_ref(repo, &ref_name, &id) 494 PatchState::from_ref(repo, &ref_name, &id)
469 } 495 }
@@ -479,19 +505,40 @@ pub fn patch_log_to_writer(
479 } 505 }
480 506
481 for (i, rev) in patch.revisions.iter().enumerate() { 507 for (i, rev) in patch.revisions.iter().enumerate() {
482 let short_oid = if rev.commit.len() >= 8 { &rev.commit[..8] } else { &rev.commit }; 508 let short_oid = if rev.commit.len() >= 8 {
509 &rev.commit[..8]
510 } else {
511 &rev.commit
512 };
483 let label = if i == 0 { " (initial)" } else { "" }; 513 let label = if i == 0 { " (initial)" } else { "" };
484 let body_display = rev.body.as_deref().map(|b| format!(" \"{}\"", b)).unwrap_or_default(); 514 let body_display = rev
515 .body
516 .as_deref()
517 .map(|b| format!(" \"{}\"", b))
518 .unwrap_or_default();
485 519
486 // Compute file-change summary between consecutive revisions 520 // Compute file-change summary between consecutive revisions
487 let file_summary = if i > 0 { 521 let file_summary = if i > 0 {
488 let prev = &patch.revisions[i - 1]; 522 let prev = &patch.revisions[i - 1];
489 match (Oid::from_str(&prev.tree), Oid::from_str(&rev.tree)) { 523 match (Oid::from_str(&prev.tree), Oid::from_str(&rev.tree)) {
490 (Ok(from_oid), Ok(to_oid)) => { 524 (Ok(from_oid), Ok(to_oid)) => {
491 if let (Ok(from_tree), Ok(to_tree)) = (repo.find_tree(from_oid), repo.find_tree(to_oid)) { 525 if let (Ok(from_tree), Ok(to_tree)) =
492 if let Ok(diff) = repo.diff_tree_to_tree(Some(&from_tree), Some(&to_tree), None) { 526 (repo.find_tree(from_oid), repo.find_tree(to_oid))
527 {
528 if let Ok(diff) =
529 repo.diff_tree_to_tree(Some(&from_tree), Some(&to_tree), None)
530 {
493 let stats = diff.stats().ok(); 531 let stats = diff.stats().ok();
494 stats.map(|s| format!(" {} file(s) changed, +{} -{}", s.files_changed(), s.insertions(), s.deletions())).unwrap_or_default() 532 stats
533 .map(|s| {
534 format!(
535 " {} file(s) changed, +{} -{}",
536 s.files_changed(),
537 s.insertions(),
538 s.deletions()
539 )
540 })
541 .unwrap_or_default()
495 } else { 542 } else {
496 String::new() 543 String::new()
497 } 544 }
@@ -530,13 +577,16 @@ pub fn close(
530 reason: Option<&str>, 577 reason: Option<&str>,
531 ) -> Result<(), crate::error::Error> { 578 ) -> Result<(), crate::error::Error> {
532 let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?; 579 let (ref_name, id) = state::resolve_patch_ref(repo, id_prefix)?;
533 dag::append_action(repo, &ref_name, Action::PatchClose { 580 dag::append_action(
534 reason: reason.map(|s| s.to_string()), 581 repo,
535 })?; 582 &ref_name,
583 Action::PatchClose {
584 reason: reason.map(|s| s.to_string()),
585 },
586 )?;
536 // Archive the ref (move to refs/collab/archive/patches/) 587 // Archive the ref (move to refs/collab/archive/patches/)
537 if ref_name.starts_with("refs/collab/patches/") { 588 if ref_name.starts_with("refs/collab/patches/") {
538 state::archive_patch_ref(repo, &id)?; 589 state::archive_patch_ref(repo, &id)?;
539 } 590 }
540 Ok(()) 591 Ok(())
541 } 592 }
542
src/server/config.rs
Old New
@@ -53,9 +53,18 @@ site_title = "my repos"
53 "#; 53 "#;
54 let config = ServerConfig::from_toml(toml).unwrap(); 54 let config = ServerConfig::from_toml(toml).unwrap();
55 assert_eq!(config.repos_dir, PathBuf::from("/srv/git")); 55 assert_eq!(config.repos_dir, PathBuf::from("/srv/git"));
56 assert_eq!(config.http_bind, "127.0.0.1:3000".parse::<SocketAddr>().unwrap()); 56 assert_eq!(
57 assert_eq!(config.ssh_bind, "127.0.0.1:2222".parse::<SocketAddr>().unwrap()); 57 config.http_bind,
58 assert_eq!(config.authorized_keys, PathBuf::from("/etc/git-collab-server/authorized_keys")); 58 "127.0.0.1:3000".parse::<SocketAddr>().unwrap()
59 );
60 assert_eq!(
61 config.ssh_bind,
62 "127.0.0.1:2222".parse::<SocketAddr>().unwrap()
63 );
64 assert_eq!(
65 config.authorized_keys,
66 PathBuf::from("/etc/git-collab-server/authorized_keys")
67 );
59 assert_eq!(config.site_title, "my repos"); 68 assert_eq!(config.site_title, "my repos");
60 } 69 }
61 70
@@ -67,8 +76,14 @@ authorized_keys = "/keys"
67 "#; 76 "#;
68 let config = ServerConfig::from_toml(toml).unwrap(); 77 let config = ServerConfig::from_toml(toml).unwrap();
69 assert_eq!(config.repos_dir, PathBuf::from("/srv/git")); 78 assert_eq!(config.repos_dir, PathBuf::from("/srv/git"));
70 assert_eq!(config.http_bind, "0.0.0.0:8080".parse::<SocketAddr>().unwrap()); 79 assert_eq!(
71 assert_eq!(config.ssh_bind, "0.0.0.0:2222".parse::<SocketAddr>().unwrap()); 80 config.http_bind,
81 "0.0.0.0:8080".parse::<SocketAddr>().unwrap()
82 );
83 assert_eq!(
84 config.ssh_bind,
85 "0.0.0.0:2222".parse::<SocketAddr>().unwrap()
86 );
72 assert_eq!(config.site_title, "git-collab"); 87 assert_eq!(config.site_title, "git-collab");
73 } 88 }
74 89
src/server/http/git_http.rs
Old New
@@ -35,7 +35,13 @@ pub async fn info_refs(
35 35
36 let entry = match crate::repos::resolve(&state.repos_dir, repo_name) { 36 let entry = match crate::repos::resolve(&state.repos_dir, repo_name) {
37 Some(e) => e, 37 Some(e) => e,
38 None => return (StatusCode::NOT_FOUND, format!("Repository '{}' not found.", repo_name)).into_response(), 38 None => {
39 return (
40 StatusCode::NOT_FOUND,
41 format!("Repository '{}' not found.", repo_name),
42 )
43 .into_response()
44 }
39 }; 45 };
40 46
41 let git_dir = if entry.bare { 47 let git_dir = if entry.bare {
@@ -53,7 +59,11 @@ pub async fn info_refs(
53 Ok(o) => o, 59 Ok(o) => o,
54 Err(e) => { 60 Err(e) => {
55 tracing::error!("Failed to spawn git upload-pack: {}", e); 61 tracing::error!("Failed to spawn git upload-pack: {}", e);
56 return (StatusCode::INTERNAL_SERVER_ERROR, "Failed to run git upload-pack").into_response(); 62 return (
63 StatusCode::INTERNAL_SERVER_ERROR,
64 "Failed to run git upload-pack",
65 )
66 .into_response();
57 } 67 }
58 }; 68 };
59 69
@@ -81,7 +91,10 @@ pub async fn info_refs(
81 ( 91 (
82 StatusCode::OK, 92 StatusCode::OK,
83 [ 93 [
84 ("Content-Type", "application/x-git-upload-pack-advertisement"), 94 (
95 "Content-Type",
96 "application/x-git-upload-pack-advertisement",
97 ),
85 ("Cache-Control", "no-cache"), 98 ("Cache-Control", "no-cache"),
86 ], 99 ],
87 body, 100 body,
@@ -98,7 +111,13 @@ pub async fn upload_pack(
98 111
99 let entry = match crate::repos::resolve(&state.repos_dir, repo_name) { 112 let entry = match crate::repos::resolve(&state.repos_dir, repo_name) {
100 Some(e) => e, 113 Some(e) => e,
101 None => return (StatusCode::NOT_FOUND, format!("Repository '{}' not found.", repo_name)).into_response(), 114 None => {
115 return (
116 StatusCode::NOT_FOUND,
117 format!("Repository '{}' not found.", repo_name),
118 )
119 .into_response()
120 }
102 }; 121 };
103 122
104 let git_dir = if entry.bare { 123 let git_dir = if entry.bare {
@@ -118,14 +137,22 @@ pub async fn upload_pack(
118 Ok(c) => c, 137 Ok(c) => c,
119 Err(e) => { 138 Err(e) => {
120 tracing::error!("Failed to spawn git upload-pack: {}", e); 139 tracing::error!("Failed to spawn git upload-pack: {}", e);
121 return (StatusCode::INTERNAL_SERVER_ERROR, "Failed to run git upload-pack").into_response(); 140 return (
141 StatusCode::INTERNAL_SERVER_ERROR,
142 "Failed to run git upload-pack",
143 )
144 .into_response();
122 } 145 }
123 }; 146 };
124 147
125 if let Some(mut stdin) = child.stdin.take() { 148 if let Some(mut stdin) = child.stdin.take() {
126 if let Err(e) = stdin.write_all(&body).await { 149 if let Err(e) = stdin.write_all(&body).await {
127 tracing::error!("Failed to write to git upload-pack stdin: {}", e); 150 tracing::error!("Failed to write to git upload-pack stdin: {}", e);
128 return (StatusCode::INTERNAL_SERVER_ERROR, "Failed to communicate with git upload-pack").into_response(); 151 return (
152 StatusCode::INTERNAL_SERVER_ERROR,
153 "Failed to communicate with git upload-pack",
154 )
155 .into_response();
129 } 156 }
130 } 157 }
131 158
@@ -146,21 +173,14 @@ pub async fn upload_pack(
146 return (StatusCode::INTERNAL_SERVER_ERROR, "git upload-pack failed").into_response(); 173 return (StatusCode::INTERNAL_SERVER_ERROR, "git upload-pack failed").into_response();
147 } 174 }
148 175
149 let mut response = ( 176 let mut response = (StatusCode::OK, output.stdout).into_response();
150 StatusCode::OK,
151 output.stdout,
152 )
153 .into_response();
154 177
155 let headers = response.headers_mut(); 178 let headers = response.headers_mut();
156 headers.insert( 179 headers.insert(
157 "Content-Type", 180 "Content-Type",
158 HeaderValue::from_static("application/x-git-upload-pack-result"), 181 HeaderValue::from_static("application/x-git-upload-pack-result"),
159 ); 182 );
160 headers.insert( 183 headers.insert("Cache-Control", HeaderValue::from_static("no-cache"));
161 "Cache-Control",
162 HeaderValue::from_static("no-cache"),
163 );
164 184
165 response 185 response
166 } 186 }
src/server/http/mod.rs
Old New
@@ -1,11 +1,11 @@
1 pub mod git_http; 1 pub mod git_http;
2 pub mod repo_list;
3 pub mod repo; 2 pub mod repo;
3 pub mod repo_list;
4 4
5 use std::path::PathBuf;
6 use std::sync::Arc;
7 use axum::extract::DefaultBodyLimit; 5 use axum::extract::DefaultBodyLimit;
8 use axum::Router; 6 use axum::Router;
7 use std::path::PathBuf;
8 use std::sync::Arc;
9 9
10 #[derive(Debug, Clone)] 10 #[derive(Debug, Clone)]
11 pub struct AppState { 11 pub struct AppState {
@@ -19,17 +19,32 @@ pub fn router(state: AppState) -> Router {
19 .route("/", axum::routing::get(repo_list::handler)) 19 .route("/", axum::routing::get(repo_list::handler))
20 .route("/{repo_name}", axum::routing::get(repo::overview)) 20 .route("/{repo_name}", axum::routing::get(repo::overview))
21 .route("/{repo_name}/commits", axum::routing::get(repo::commits)) 21 .route("/{repo_name}/commits", axum::routing::get(repo::commits))
22 .route("/{repo_name}/commits/{ref_name}", axum::routing::get(repo::commits_ref)) 22 .route(
23 "/{repo_name}/commits/{ref_name}",
24 axum::routing::get(repo::commits_ref),
25 )
23 .route("/{repo_name}/tree", axum::routing::get(repo::tree_root)) 26 .route("/{repo_name}/tree", axum::routing::get(repo::tree_root))
24 .route("/{repo_name}/tree/{*rest}", axum::routing::get(repo::tree)) 27 .route("/{repo_name}/tree/{*rest}", axum::routing::get(repo::tree))
25 .route("/{repo_name}/blob/{*rest}", axum::routing::get(repo::blob)) 28 .route("/{repo_name}/blob/{*rest}", axum::routing::get(repo::blob))
26 .route("/{repo_name}/diff/{oid}", axum::routing::get(repo::diff)) 29 .route("/{repo_name}/diff/{oid}", axum::routing::get(repo::diff))
27 .route("/{repo_name}/patches", axum::routing::get(repo::patches)) 30 .route("/{repo_name}/patches", axum::routing::get(repo::patches))
28 .route("/{repo_name}/patches/{id}", axum::routing::get(repo::patch_detail)) 31 .route(
32 "/{repo_name}/patches/{id}",
33 axum::routing::get(repo::patch_detail),
34 )
29 .route("/{repo_name}/issues", axum::routing::get(repo::issues)) 35 .route("/{repo_name}/issues", axum::routing::get(repo::issues))
30 .route("/{repo_name}/issues/{id}", axum::routing::get(repo::issue_detail)) 36 .route(
31 .route("/{repo_dot_git}/info/refs", axum::routing::get(git_http::info_refs)) 37 "/{repo_name}/issues/{id}",
32 .route("/{repo_dot_git}/git-upload-pack", axum::routing::post(git_http::upload_pack) 38 axum::routing::get(repo::issue_detail),
33 .layer(DefaultBodyLimit::max(git_http::UPLOAD_PACK_BODY_LIMIT))) 39 )
40 .route(
41 "/{repo_dot_git}/info/refs",
42 axum::routing::get(git_http::info_refs),
43 )
44 .route(
45 "/{repo_dot_git}/git-upload-pack",
46 axum::routing::post(git_http::upload_pack)
47 .layer(DefaultBodyLimit::max(git_http::UPLOAD_PACK_BODY_LIMIT)),
48 )
34 .with_state(shared) 49 .with_state(shared)
35 } 50 }
src/server/http/repo/commits.rs
Old New
@@ -3,7 +3,7 @@ use std::sync::Arc;
3 use axum::extract::{Path, State}; 3 use axum::extract::{Path, State};
4 use axum::response::{IntoResponse, Response}; 4 use axum::response::{IntoResponse, Response};
5 5
6 use super::{AppState, OverviewCommit, collab_counts, head_branch_name, list_branches, open_repo}; 6 use super::{collab_counts, head_branch_name, list_branches, open_repo, AppState, OverviewCommit};
7 7
8 #[derive(askama::Template, askama_web::WebTemplate)] 8 #[derive(askama::Template, askama_web::WebTemplate)]
9 #[template(path = "commits.html")] 9 #[template(path = "commits.html")]
@@ -48,7 +48,13 @@ fn commits_for_ref(repo: &git2::Repository, ref_name: &str, limit: usize) -> Vec
48 .single() 48 .single()
49 .map(|dt| dt.format("%Y-%m-%d").to_string()) 49 .map(|dt| dt.format("%Y-%m-%d").to_string())
50 .unwrap_or_default(); 50 .unwrap_or_default();
51 Some(OverviewCommit { id, short_id, summary, author, date }) 51 Some(OverviewCommit {
52 id,
53 short_id,
54 summary,
55 author,
56 date,
57 })
52 }) 58 })
53 .collect() 59 .collect()
54 } 60 }
src/server/http/repo/issues.rs
Old New
@@ -3,7 +3,7 @@ use std::sync::Arc;
3 use axum::extract::{Path, State}; 3 use axum::extract::{Path, State};
4 use axum::response::{IntoResponse, Response}; 4 use axum::response::{IntoResponse, Response};
5 5
6 use super::{AppState, CommentView, collab_counts, internal_error, not_found, open_repo}; 6 use super::{collab_counts, internal_error, not_found, open_repo, AppState, CommentView};
7 7
8 #[derive(Debug)] 8 #[derive(Debug)]
9 pub struct IssueListItem { 9 pub struct IssueListItem {
@@ -50,10 +50,7 @@ pub struct IssueDetailTemplate {
50 pub issue: IssueDetailView, 50 pub issue: IssueDetailView,
51 } 51 }
52 52
53 pub async fn issues( 53 pub async fn issues(Path(repo_name): Path<String>, State(state): State<Arc<AppState>>) -> Response {
54 Path(repo_name): Path<String>,
55 State(state): State<Arc<AppState>>,
56 ) -> Response {
57 let (_entry, repo) = match open_repo(&state, &repo_name) { 54 let (_entry, repo) = match open_repo(&state, &repo_name) {
58 Ok(r) => r, 55 Ok(r) => r,
59 Err(resp) => return resp, 56 Err(resp) => return resp,
@@ -120,11 +117,15 @@ pub async fn issue_detail(
120 labels: is.labels.join(", "), 117 labels: is.labels.join(", "),
121 assignees: is.assignees.join(", "), 118 assignees: is.assignees.join(", "),
122 close_reason: is.close_reason, 119 close_reason: is.close_reason,
123 comments: is.comments.into_iter().map(|c| CommentView { 120 comments: is
124 author: c.author.name, 121 .comments
125 body: c.body, 122 .into_iter()
126 timestamp: c.timestamp, 123 .map(|c| CommentView {
127 }).collect(), 124 author: c.author.name,
125 body: c.body,
126 timestamp: c.timestamp,
127 })
128 .collect(),
128 }; 129 };
129 130
130 IssueDetailTemplate { 131 IssueDetailTemplate {
src/server/http/repo/mod.rs
Old New
@@ -1,16 +1,16 @@
1 mod overview;
2 mod commits; 1 mod commits;
3 mod tree;
4 mod diff; 2 mod diff;
5 mod patches;
6 mod issues; 3 mod issues;
4 mod overview;
5 mod patches;
6 mod tree;
7 7
8 pub use overview::overview;
9 pub use commits::{commits, commits_ref}; 8 pub use commits::{commits, commits_ref};
10 pub use tree::{tree_root, tree, blob};
11 pub use diff::diff; 9 pub use diff::diff;
12 pub use patches::{patches, patch_detail}; 10 pub use issues::{issue_detail, issues};
13 pub use issues::{issues, issue_detail}; 11 pub use overview::overview;
12 pub use patches::{patch_detail, patches};
13 pub use tree::{blob, tree, tree_root};
14 14
15 use axum::http::StatusCode; 15 use axum::http::StatusCode;
16 use axum::response::{IntoResponse, Response}; 16 use axum::response::{IntoResponse, Response};
@@ -109,7 +109,13 @@ fn recent_commits(repo: &git2::Repository, limit: usize) -> Vec<OverviewCommit>
109 .single() 109 .single()
110 .map(|dt| dt.format("%Y-%m-%d").to_string()) 110 .map(|dt| dt.format("%Y-%m-%d").to_string())
111 .unwrap_or_default(); 111 .unwrap_or_default();
112 Some(OverviewCommit { id, short_id, summary, author, date }) 112 Some(OverviewCommit {
113 id,
114 short_id,
115 summary,
116 author,
117 date,
118 })
113 }) 119 })
114 .collect() 120 .collect()
115 } 121 }
@@ -146,15 +152,26 @@ fn head_branch_name(repo: &git2::Repository) -> String {
146 } 152 }
147 } 153 }
148 // HEAD is unborn or detached — pick the first local branch 154 // HEAD is unborn or detached — pick the first local branch
149 list_branches(repo).into_iter().next().unwrap_or_else(|| "HEAD".to_string()) 155 list_branches(repo)
156 .into_iter()
157 .next()
158 .unwrap_or_else(|| "HEAD".to_string())
150 } 159 }
151 160
152 /// Open a repo by name, returning the entry and git2::Repository or an error Response. 161 /// Open a repo by name, returning the entry and git2::Repository or an error Response.
153 #[allow(clippy::result_large_err)] 162 #[allow(clippy::result_large_err)]
154 fn open_repo(state: &AppState, repo_name: &str) -> Result<(crate::repos::RepoEntry, git2::Repository), Response> { 163 fn open_repo(
164 state: &AppState,
165 repo_name: &str,
166 ) -> Result<(crate::repos::RepoEntry, git2::Repository), Response> {
155 let entry = match crate::repos::resolve(&state.repos_dir, repo_name) { 167 let entry = match crate::repos::resolve(&state.repos_dir, repo_name) {
156 Some(e) => e, 168 Some(e) => e,
157 None => return Err(not_found(state, format!("Repository '{}' not found.", repo_name))), 169 None => {
170 return Err(not_found(
171 state,
172 format!("Repository '{}' not found.", repo_name),
173 ))
174 }
158 }; 175 };
159 176
160 let repo = match crate::repos::open(&entry) { 177 let repo = match crate::repos::open(&entry) {
src/server/http/repo/overview.rs
Old New
@@ -3,7 +3,7 @@ use std::sync::Arc;
3 use axum::extract::{Path, State}; 3 use axum::extract::{Path, State};
4 use axum::response::{IntoResponse, Response}; 4 use axum::response::{IntoResponse, Response};
5 5
6 use super::{AppState, OverviewCommit, collab_counts, open_repo, recent_commits}; 6 use super::{collab_counts, open_repo, recent_commits, AppState, OverviewCommit};
7 7
8 #[derive(Debug)] 8 #[derive(Debug)]
9 pub struct OverviewPatch { 9 pub struct OverviewPatch {
src/server/http/repo/patches.rs
Old New
@@ -3,7 +3,7 @@ use std::sync::Arc;
3 use axum::extract::{Path, State}; 3 use axum::extract::{Path, State};
4 use axum::response::{IntoResponse, Response}; 4 use axum::response::{IntoResponse, Response};
5 5
6 use super::{AppState, CommentView, collab_counts, internal_error, not_found, open_repo}; 6 use super::{collab_counts, internal_error, not_found, open_repo, AppState, CommentView};
7 7
8 #[derive(Debug)] 8 #[derive(Debug)]
9 pub struct PatchListItem { 9 pub struct PatchListItem {
@@ -148,32 +148,48 @@ pub async fn patch_detail(
148 author: ps.author.name, 148 author: ps.author.name,
149 branch: ps.branch, 149 branch: ps.branch,
150 base_ref: ps.base_ref, 150 base_ref: ps.base_ref,
151 revisions: ps.revisions.into_iter().map(|r| RevisionView { 151 revisions: ps
152 number: r.number, 152 .revisions
153 commit: r.commit, 153 .into_iter()
154 timestamp: r.timestamp, 154 .map(|r| RevisionView {
155 body: r.body, 155 number: r.number,
156 }).collect(), 156 commit: r.commit,
157 reviews: ps.reviews.into_iter().map(|r| ReviewView { 157 timestamp: r.timestamp,
158 author: r.author.name, 158 body: r.body,
159 verdict: r.verdict.as_str().to_string(), 159 })
160 body: r.body, 160 .collect(),
161 timestamp: r.timestamp, 161 reviews: ps
162 revision: r.revision, 162 .reviews
163 }).collect(), 163 .into_iter()
164 inline_comments: ps.inline_comments.into_iter().map(|ic| InlineCommentView { 164 .map(|r| ReviewView {
165 author: ic.author.name, 165 author: r.author.name,
166 file: ic.file, 166 verdict: r.verdict.as_str().to_string(),
167 line: ic.line, 167 body: r.body,
168 body: ic.body, 168 timestamp: r.timestamp,
169 timestamp: ic.timestamp, 169 revision: r.revision,
170 revision: ic.revision, 170 })
171 }).collect(), 171 .collect(),
172 comments: ps.comments.into_iter().map(|c| CommentView { 172 inline_comments: ps
173 author: c.author.name, 173 .inline_comments
174 body: c.body, 174 .into_iter()
175 timestamp: c.timestamp, 175 .map(|ic| InlineCommentView {
176 }).collect(), 176 author: ic.author.name,
177 file: ic.file,
178 line: ic.line,
179 body: ic.body,
180 timestamp: ic.timestamp,
181 revision: ic.revision,
182 })
183 .collect(),
184 comments: ps
185 .comments
186 .into_iter()
187 .map(|c| CommentView {
188 author: c.author.name,
189 body: c.body,
190 timestamp: c.timestamp,
191 })
192 .collect(),
177 }; 193 };
178 194
179 PatchDetailTemplate { 195 PatchDetailTemplate {
src/server/http/repo/tree.rs
Old New
@@ -3,7 +3,9 @@ use std::sync::Arc;
3 use axum::extract::{Path, State}; 3 use axum::extract::{Path, State};
4 use axum::response::{IntoResponse, Response}; 4 use axum::response::{IntoResponse, Response};
5 5
6 use super::{AppState, collab_counts, head_branch_name, internal_error, list_branches, not_found, open_repo}; 6 use super::{
7 collab_counts, head_branch_name, internal_error, list_branches, not_found, open_repo, AppState,
8 };
7 9
8 #[derive(Debug)] 10 #[derive(Debug)]
9 pub struct TreeEntry { 11 pub struct TreeEntry {
@@ -52,7 +54,11 @@ fn split_ref_path(input: &str) -> (String, String) {
52 } 54 }
53 let parts: Vec<&str> = input.splitn(2, '/').collect(); 55 let parts: Vec<&str> = input.splitn(2, '/').collect();
54 let ref_name = parts[0].to_string(); 56 let ref_name = parts[0].to_string();
55 let path = if parts.len() > 1 { parts[1].to_string() } else { String::new() }; 57 let path = if parts.len() > 1 {
58 parts[1].to_string()
59 } else {
60 String::new()
61 };
56 (ref_name, path) 62 (ref_name, path)
57 } 63 }
58 64
@@ -65,7 +71,11 @@ fn build_tree_response(
65 ) -> Response { 71 ) -> Response {
66 let (open_patches, open_issues) = collab_counts(repo); 72 let (open_patches, open_issues) = collab_counts(repo);
67 let branches = list_branches(repo); 73 let branches = list_branches(repo);
68 let ref_name = if ref_name == "HEAD" { head_branch_name(repo) } else { ref_name }; 74 let ref_name = if ref_name == "HEAD" {
75 head_branch_name(repo)
76 } else {
77 ref_name
78 };
69 79
70 let obj = match repo.revparse_single(&ref_name) { 80 let obj = match repo.revparse_single(&ref_name) {
71 Ok(o) => o, 81 Ok(o) => o,
@@ -109,22 +119,28 @@ fn build_tree_response(
109 format!("{}/{}", path, name) 119 format!("{}/{}", path, name)
110 }; 120 };
111 let is_dir = e.kind() == Some(git2::ObjectType::Tree); 121 let is_dir = e.kind() == Some(git2::ObjectType::Tree);
112 TreeEntry { name, full_path, is_dir } 122 TreeEntry {
123 name,
124 full_path,
125 is_dir,
126 }
113 }) 127 })
114 .collect(); 128 .collect();
115 129
116 entries.sort_by(|a, b| { 130 entries.sort_by(|a, b| match (a.is_dir, b.is_dir) {
117 match (a.is_dir, b.is_dir) { 131 (true, false) => std::cmp::Ordering::Less,
118 (true, false) => std::cmp::Ordering::Less, 132 (false, true) => std::cmp::Ordering::Greater,
119 (false, true) => std::cmp::Ordering::Greater, 133 _ => a.name.cmp(&b.name),
120 _ => a.name.cmp(&b.name),
121 }
122 }); 134 });
123 135
124 let (show_parent, parent_path) = if path.is_empty() { 136 let (show_parent, parent_path) = if path.is_empty() {
125 (false, String::new()) 137 (false, String::new())
126 } else { 138 } else {
127 let parent = path.rfind('/').map(|i| &path[..i]).unwrap_or("").to_string(); 139 let parent = path
140 .rfind('/')
141 .map(|i| &path[..i])
142 .unwrap_or("")
143 .to_string();
128 (true, parent) 144 (true, parent)
129 }; 145 };
130 146
@@ -181,7 +197,11 @@ pub async fn blob(
181 let (open_patches, open_issues) = collab_counts(&repo); 197 let (open_patches, open_issues) = collab_counts(&repo);
182 let branches = list_branches(&repo); 198 let branches = list_branches(&repo);
183 let (ref_name, file_path) = split_ref_path(&rest); 199 let (ref_name, file_path) = split_ref_path(&rest);
184 let ref_name = if ref_name == "HEAD" { head_branch_name(&repo) } else { ref_name }; 200 let ref_name = if ref_name == "HEAD" {
201 head_branch_name(&repo)
202 } else {
203 ref_name
204 };
185 205
186 let obj = match repo.revparse_single(&ref_name) { 206 let obj = match repo.revparse_single(&ref_name) {
187 Ok(o) => o, 207 Ok(o) => o,
src/server/http/repo_list.rs
Old New
@@ -1,6 +1,6 @@
1 use std::sync::Arc;
2 use axum::extract::State; 1 use axum::extract::State;
3 use axum::response::IntoResponse; 2 use axum::response::IntoResponse;
3 use std::sync::Arc;
4 4
5 use super::AppState; 5 use super::AppState;
6 6
@@ -187,10 +187,17 @@ mod tests {
187 let repo_path = tmp.path().join("repo"); 187 let repo_path = tmp.path().join("repo");
188 188
189 git(tmp.path(), &["init", "repo"]); 189 git(tmp.path(), &["init", "repo"]);
190 std::fs::write(repo_path.join(".git").join("description"), "Local repo description\n").unwrap(); 190 std::fs::write(
191 repo_path.join(".git").join("description"),
192 "Local repo description\n",
193 )
194 .unwrap();
191 195
192 let entry = make_non_bare_entry(&repo_path); 196 let entry = make_non_bare_entry(&repo_path);
193 assert_eq!(read_local_description(&entry), Some("Local repo description".to_string())); 197 assert_eq!(
198 read_local_description(&entry),
199 Some("Local repo description".to_string())
200 );
194 assert_eq!(read_description(&entry), "Local repo description"); 201 assert_eq!(read_description(&entry), "Local repo description");
195 } 202 }
196 } 203 }
src/server/repos.rs
Old New
@@ -23,10 +23,21 @@ pub fn discover(repos_dir: &Path) -> Result<Vec<RepoEntry>, std::io::Error> {
23 let dir_name = entry.file_name().to_string_lossy().to_string(); 23 let dir_name = entry.file_name().to_string_lossy().to_string();
24 24
25 if path.join("HEAD").is_file() { 25 if path.join("HEAD").is_file() {
26 let name = dir_name.strip_suffix(".git").unwrap_or(&dir_name).to_string(); 26 let name = dir_name
27 entries.push(RepoEntry { name, path, bare: true }); 27 .strip_suffix(".git")
28 .unwrap_or(&dir_name)
29 .to_string();
30 entries.push(RepoEntry {
31 name,
32 path,
33 bare: true,
34 });
28 } else if path.join(".git").is_dir() { 35 } else if path.join(".git").is_dir() {
29 entries.push(RepoEntry { name: dir_name, path, bare: false }); 36 entries.push(RepoEntry {
37 name: dir_name,
38 path,
39 bare: false,
40 });
30 } 41 }
31 } 42 }
32 43
@@ -54,8 +65,8 @@ pub fn open(entry: &RepoEntry) -> Result<git2::Repository, git2::Error> {
54 #[cfg(test)] 65 #[cfg(test)]
55 mod tests { 66 mod tests {
56 use super::*; 67 use super::*;
57 use tempfile::TempDir;
58 use std::process::Command; 68 use std::process::Command;
69 use tempfile::TempDir;
59 70
60 fn init_bare(parent: &Path, name: &str) { 71 fn init_bare(parent: &Path, name: &str) {
61 Command::new("git") 72 Command::new("git")
src/server/ssh/auth.rs
Old New
@@ -20,7 +20,11 @@ pub fn parse_authorized_keys(content: &str) -> Vec<AuthorizedKey> {
20 let key_type = parts.next()?.to_string(); 20 let key_type = parts.next()?.to_string();
21 let key_data = parts.next()?.to_string(); 21 let key_data = parts.next()?.to_string();
22 let comment = parts.next().map(|s| s.to_string()); 22 let comment = parts.next().map(|s| s.to_string());
23 Some(AuthorizedKey { key_type, key_data, comment }) 23 Some(AuthorizedKey {
24 key_type,
25 key_data,
26 comment,
27 })
24 }) 28 })
25 .collect() 29 .collect()
26 } 30 }
@@ -31,7 +35,8 @@ pub fn load_authorized_keys(path: &Path) -> Result<Vec<AuthorizedKey>, std::io::
31 } 35 }
32 36
33 pub fn is_authorized(keys: &[AuthorizedKey], key_type: &str, key_data: &str) -> bool { 37 pub fn is_authorized(keys: &[AuthorizedKey], key_type: &str, key_data: &str) -> bool {
34 keys.iter().any(|k| k.key_type == key_type && k.key_data == key_data) 38 keys.iter()
39 .any(|k| k.key_type == key_type && k.key_data == key_data)
35 } 40 }
36 41
37 #[cfg(test)] 42 #[cfg(test)]
src/server/ssh/mod.rs
Old New
@@ -6,8 +6,8 @@ use std::path::Path;
6 use std::sync::Arc; 6 use std::sync::Arc;
7 7
8 use async_trait::async_trait; 8 use async_trait::async_trait;
9 use russh_keys::key::KeyPair;
10 use russh::server::Server as _; 9 use russh::server::Server as _;
10 use russh_keys::key::KeyPair;
11 use tracing::{error, info}; 11 use tracing::{error, info};
12 12
13 use session::{SshHandler, SshServerConfig}; 13 use session::{SshHandler, SshServerConfig};
src/server/ssh/session.rs
Old New
@@ -83,9 +83,7 @@ pub fn resolve_repo_path(repos_dir: &Path, requested: &str) -> Option<PathBuf> {
83 } 83 }
84 // If the path exists, canonicalize both to catch symlink escapes 84 // If the path exists, canonicalize both to catch symlink escapes
85 if full.exists() { 85 if full.exists() {
86 if let (Ok(canon_repos), Ok(canon_full)) = 86 if let (Ok(canon_repos), Ok(canon_full)) = (repos_dir.canonicalize(), full.canonicalize()) {
87 (repos_dir.canonicalize(), full.canonicalize())
88 {
89 if !canon_full.starts_with(&canon_repos) { 87 if !canon_full.starts_with(&canon_repos) {
90 return None; 88 return None;
91 } 89 }
@@ -104,8 +102,9 @@ fn ensure_repo_exists_for_command(git_cmd: &str, repo_path: &Path) -> Result<boo
104 } 102 }
105 103
106 if let Some(parent) = repo_path.parent() { 104 if let Some(parent) = repo_path.parent() {
107 std::fs::create_dir_all(parent) 105 std::fs::create_dir_all(parent).map_err(|e| {
108 .map_err(|e| git2::Error::from_str(&format!("failed to create repo parent dir: {e}")))?; 106 git2::Error::from_str(&format!("failed to create repo parent dir: {e}"))
107 })?;
109 } 108 }
110 109
111 git2::Repository::init_bare(repo_path)?; 110 git2::Repository::init_bare(repo_path)?;
@@ -178,16 +177,17 @@ impl Handler for SshHandler {
178 177
179 info!("Exec request: {}", command_str); 178 info!("Exec request: {}", command_str);
180 179
181 let (git_cmd, repo_path) = match parse_git_command(command_str).map(|(c, p)| (c.to_owned(), p.to_owned())) { 180 let (git_cmd, repo_path) =
182 Some((c, p)) => (c, p), 181 match parse_git_command(command_str).map(|(c, p)| (c.to_owned(), p.to_owned())) {
183 None => { 182 Some((c, p)) => (c, p),
184 warn!("Rejected exec request: not a valid git command"); 183 None => {
185 session.exit_status_request(channel, 1); 184 warn!("Rejected exec request: not a valid git command");
186 session.eof(channel); 185 session.exit_status_request(channel, 1);
187 session.close(channel); 186 session.eof(channel);
188 return Ok(()); 187 session.close(channel);
189 } 188 return Ok(());
190 }; 189 }
190 };
191 191
192 let resolved_path = match resolve_repo_path(&self.config.repos_dir, &repo_path) { 192 let resolved_path = match resolve_repo_path(&self.config.repos_dir, &repo_path) {
193 Some(p) => p, 193 Some(p) => p,
@@ -206,7 +206,10 @@ impl Handler for SshHandler {
206 info!("Created bare repo for receive-pack: {:?}", resolved_path); 206 info!("Created bare repo for receive-pack: {:?}", resolved_path);
207 } 207 }
208 Ok(false) => { 208 Ok(false) => {
209 warn!("Rejected exec request: repo path does not exist: {:?}", resolved_path); 209 warn!(
210 "Rejected exec request: repo path does not exist: {:?}",
211 resolved_path
212 );
210 session.exit_status_request(channel, 1); 213 session.exit_status_request(channel, 1);
211 session.eof(channel); 214 session.eof(channel);
212 session.close(channel); 215 session.close(channel);
@@ -230,7 +233,9 @@ impl Handler for SshHandler {
230 let handle = session.handle(); 233 let handle = session.handle();
231 let git_cmd_owned = git_cmd; 234 let git_cmd_owned = git_cmd;
232 tokio::spawn(async move { 235 tokio::spawn(async move {
233 if let Err(e) = run_git_command(handle, channel, &git_cmd_owned, &resolved_path, rx).await { 236 if let Err(e) =
237 run_git_command(handle, channel, &git_cmd_owned, &resolved_path, rx).await
238 {
234 error!("Git subprocess error: {}", e); 239 error!("Git subprocess error: {}", e);
235 } 240 }
236 }); 241 });
@@ -362,7 +367,10 @@ mod tests {
362 fn reject_traversal() { 367 fn reject_traversal() {
363 let repos_dir = Path::new("/srv/git"); 368 let repos_dir = Path::new("/srv/git");
364 assert_eq!(resolve_repo_path(repos_dir, "/../etc/passwd"), None); 369 assert_eq!(resolve_repo_path(repos_dir, "/../etc/passwd"), None);
365 assert_eq!(resolve_repo_path(repos_dir, "/repo/../../../etc/shadow"), None); 370 assert_eq!(
371 resolve_repo_path(repos_dir, "/repo/../../../etc/shadow"),
372 None
373 );
366 assert_eq!(resolve_repo_path(repos_dir, ".."), None); 374 assert_eq!(resolve_repo_path(repos_dir, ".."), None);
367 } 375 }
368 376
src/signing.rs
Old New
@@ -166,10 +166,7 @@ pub fn sign_event(event: &Event, signing_key: &SigningKey) -> Result<DetachedSig
166 /// 166 ///
167 /// Returns `Missing` if signature or pubkey fields are empty, 167 /// Returns `Missing` if signature or pubkey fields are empty,
168 /// `Valid` if the signature checks out, `Invalid` otherwise. 168 /// `Valid` if the signature checks out, `Invalid` otherwise.
169 pub fn verify_detached( 169 pub fn verify_detached(event: &Event, sig: &DetachedSignature) -> Result<VerifyStatus, Error> {
170 event: &Event,
171 sig: &DetachedSignature,
172 ) -> Result<VerifyStatus, Error> {
173 if sig.signature.is_empty() || sig.pubkey.is_empty() { 170 if sig.signature.is_empty() || sig.pubkey.is_empty() {
174 return Ok(VerifyStatus::Missing); 171 return Ok(VerifyStatus::Missing);
175 } 172 }
src/state.rs
Old New
@@ -26,7 +26,9 @@ fn deserialize_oid<'de, D: serde::Deserializer<'de>>(d: D) -> Result<Oid, D::Err
26 fn deserialize_oid_option<'de, D: serde::Deserializer<'de>>(d: D) -> Result<Option<Oid>, D::Error> { 26 fn deserialize_oid_option<'de, D: serde::Deserializer<'de>>(d: D) -> Result<Option<Oid>, D::Error> {
27 let opt: Option<String> = Option::deserialize(d)?; 27 let opt: Option<String> = Option::deserialize(d)?;
28 match opt { 28 match opt {
29 Some(s) => Oid::from_str(&s).map(Some).map_err(serde::de::Error::custom), 29 Some(s) => Oid::from_str(&s)
30 .map(Some)
31 .map_err(serde::de::Error::custom),
30 None => Ok(None), 32 None => Ok(None),
31 } 33 }
32 } 34 }
@@ -98,7 +100,10 @@ pub struct IssueState {
98 #[derive(Debug, Clone, Serialize, Deserialize)] 100 #[derive(Debug, Clone, Serialize, Deserialize)]
99 pub struct Review { 101 pub struct Review {
100 pub author: Author, 102 pub author: Author,
101 #[serde(serialize_with = "serialize_verdict", deserialize_with = "deserialize_verdict")] 103 #[serde(
104 serialize_with = "serialize_verdict",
105 deserialize_with = "deserialize_verdict"
106 )]
102 pub verdict: ReviewVerdict, 107 pub verdict: ReviewVerdict,
103 pub body: String, 108 pub body: String,
104 pub timestamp: String, 109 pub timestamp: String,
@@ -243,7 +248,11 @@ impl IssueState {
243 max_timestamp = event.timestamp.clone(); 248 max_timestamp = event.timestamp.clone();
244 } 249 }
245 match event.action { 250 match event.action {
246 Action::IssueOpen { title, body, relates_to } => { 251 Action::IssueOpen {
252 title,
253 body,
254 relates_to,
255 } => {
247 state = Some(IssueState { 256 state = Some(IssueState {
248 id: id.to_string(), 257 id: id.to_string(),
249 title, 258 title,
@@ -350,10 +359,7 @@ impl PatchState {
350 // Fall back to branch name lookup 359 // Fall back to branch name lookup
351 let ref_name = format!("refs/heads/{}", self.branch); 360 let ref_name = format!("refs/heads/{}", self.branch);
352 repo.refname_to_id(&ref_name).map_err(|e| { 361 repo.refname_to_id(&ref_name).map_err(|e| {
353 crate::error::Error::Cmd(format!( 362 crate::error::Error::Cmd(format!("branch '{}' not found: {}", self.branch, e))
354 "branch '{}' not found: {}",
355 self.branch, e
356 ))
357 }) 363 })
358 } 364 }
359 365
@@ -377,14 +383,22 @@ impl PatchState {
377 if self.status != PatchStatus::Open { 383 if self.status != PatchStatus::Open {
378 return; 384 return;
379 } 385 }
380 let Ok(patch_head) = self.resolve_head(repo) else { return }; 386 let Ok(patch_head) = self.resolve_head(repo) else {
387 return;
388 };
381 let base_ref = format!("refs/heads/{}", self.base_ref); 389 let base_ref = format!("refs/heads/{}", self.base_ref);
382 let Ok(base_tip) = repo.refname_to_id(&base_ref) else { return }; 390 let Ok(base_tip) = repo.refname_to_id(&base_ref) else {
383 let base_moved = self.base_commit.as_ref() 391 return;
392 };
393 let base_moved = self
394 .base_commit
395 .as_ref()
384 .map(|bc| bc != &base_tip.to_string()) 396 .map(|bc| bc != &base_tip.to_string())
385 .unwrap_or(false); 397 .unwrap_or(false);
386 let reachable = base_tip == patch_head 398 let reachable = base_tip == patch_head
387 || repo.graph_descendant_of(base_tip, patch_head).unwrap_or(false); 399 || repo
400 .graph_descendant_of(base_tip, patch_head)
401 .unwrap_or(false);
388 if base_moved && reachable { 402 if base_moved && reachable {
389 self.status = PatchStatus::Merged; 403 self.status = PatchStatus::Merged;
390 } 404 }
@@ -487,7 +501,11 @@ impl PatchState {
487 } 501 }
488 } 502 }
489 } 503 }
490 Action::PatchReview { verdict, body, revision } => { 504 Action::PatchReview {
505 verdict,
506 body,
507 revision,
508 } => {
491 if let Some(ref mut s) = state { 509 if let Some(ref mut s) = state {
492 s.reviews.push(Review { 510 s.reviews.push(Review {
493 author: event.author.clone(), 511 author: event.author.clone(),
@@ -508,7 +526,12 @@ impl PatchState {
508 }); 526 });
509 } 527 }
510 } 528 }
511 Action::PatchInlineComment { file, line, body, revision } => { 529 Action::PatchInlineComment {
530 file,
531 line,
532 body,
533 revision,
534 } => {
512 if let Some(ref mut s) = state { 535 if let Some(ref mut s) = state {
513 s.inline_comments.push(InlineComment { 536 s.inline_comments.push(InlineComment {
514 author: event.author.clone(), 537 author: event.author.clone(),
@@ -656,7 +679,9 @@ pub fn list_patches(repo: &Repository) -> Result<Vec<PatchState>, crate::error::
656 679
657 /// List all issue refs (active + archived) and return their materialized state. 680 /// List all issue refs (active + archived) and return their materialized state.
658 /// Deduplicates by ID, preferring the archived version (which has the final state). 681 /// Deduplicates by ID, preferring the archived version (which has the final state).
659 pub fn list_issues_with_archived(repo: &Repository) -> Result<Vec<IssueState>, crate::error::Error> { 682 pub fn list_issues_with_archived(
683 repo: &Repository,
684 ) -> Result<Vec<IssueState>, crate::error::Error> {
660 let mut seen = std::collections::HashSet::new(); 685 let mut seen = std::collections::HashSet::new();
661 let mut items = Vec::new(); 686 let mut items = Vec::new();
662 687
@@ -680,7 +705,9 @@ pub fn list_issues_with_archived(repo: &Repository) -> Result<Vec<IssueState>, c
680 705
681 /// List all patch refs (active + archived) and return their materialized state. 706 /// List all patch refs (active + archived) and return their materialized state.
682 /// Deduplicates by ID, preferring the archived version (which has the final state). 707 /// Deduplicates by ID, preferring the archived version (which has the final state).
683 pub fn list_patches_with_archived(repo: &Repository) -> Result<Vec<PatchState>, crate::error::Error> { 708 pub fn list_patches_with_archived(
709 repo: &Repository,
710 ) -> Result<Vec<PatchState>, crate::error::Error> {
684 let mut seen = std::collections::HashSet::new(); 711 let mut seen = std::collections::HashSet::new();
685 let mut items = Vec::new(); 712 let mut items = Vec::new();
686 713
src/status.rs
Old New
@@ -110,7 +110,11 @@ pub fn compute(repo: &Repository) -> Result<ProjectStatus, Error> {
110 110
111 impl fmt::Display for ProjectStatus { 111 impl fmt::Display for ProjectStatus {
112 fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { 112 fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
113 writeln!(f, "Issues: {} open, {} closed", self.issues_open, self.issues_closed)?; 113 writeln!(
114 f,
115 "Issues: {} open, {} closed",
116 self.issues_open, self.issues_closed
117 )?;
114 writeln!( 118 writeln!(
115 f, 119 f,
116 "Patches: {} open, {} merged, {} closed", 120 "Patches: {} open, {} merged, {} closed",
src/sync.rs
Old New
@@ -25,7 +25,10 @@ pub fn validate_collab_ref_id(id: &str) -> Result<(), Error> {
25 id.len() 25 id.len()
26 ))); 26 )));
27 } 27 }
28 if !id.chars().all(|c| c.is_ascii_hexdigit() && !c.is_ascii_uppercase()) { 28 if !id
29 .chars()
30 .all(|c| c.is_ascii_hexdigit() && !c.is_ascii_uppercase())
31 {
29 return Err(Error::InvalidRefName(format!( 32 return Err(Error::InvalidRefName(format!(
30 "ref ID must contain only lowercase hex characters [0-9a-f], got {:?}", 33 "ref ID must contain only lowercase hex characters [0-9a-f], got {:?}",
31 id 34 id
@@ -338,7 +341,9 @@ pub fn sync(repo: &Repository, remote_name: &str) -> Result<(), Error> {
338 .map(|r| { 341 .map(|r| {
339 ( 342 (
340 r.ref_name.clone(), 343 r.ref_name.clone(),
341 r.error.clone().unwrap_or_else(|| "unknown error".to_string()), 344 r.error
345 .clone()
346 .unwrap_or_else(|| "unknown error".to_string()),
342 ) 347 )
343 }) 348 })
344 .collect(); 349 .collect();
@@ -356,10 +361,7 @@ pub fn sync(repo: &Repository, remote_name: &str) -> Result<(), Error> {
356 361
357 let succeeded = sync_result.succeeded().len(); 362 let succeeded = sync_result.succeeded().len();
358 let total = sync_result.results.len(); 363 let total = sync_result.results.len();
359 return Err(Error::PartialSync { 364 return Err(Error::PartialSync { succeeded, total });
360 succeeded,
361 total,
362 });
363 } 365 }
364 } 366 }
365 367
@@ -414,7 +416,9 @@ fn sync_resume(
414 .map(|r| { 416 .map(|r| {
415 ( 417 (
416 r.ref_name.clone(), 418 r.ref_name.clone(),
417 r.error.clone().unwrap_or_else(|| "unknown error".to_string()), 419 r.error
420 .clone()
421 .unwrap_or_else(|| "unknown error".to_string()),
418 ) 422 )
419 }) 423 })
420 .collect(); 424 .collect();
@@ -430,10 +434,7 @@ fn sync_resume(
430 434
431 let succeeded = sync_result.succeeded().len(); 435 let succeeded = sync_result.succeeded().len();
432 let total = sync_result.results.len(); 436 let total = sync_result.results.len();
433 Err(Error::PartialSync { 437 Err(Error::PartialSync { succeeded, total })
434 succeeded,
435 total,
436 })
437 } 438 }
438 } 439 }
439 440
@@ -496,7 +497,8 @@ fn reconcile_refs(
496 // Apply trust checking 497 // Apply trust checking
497 let results = trust::check_trust(&results, &trust_policy); 498 let results = trust::check_trust(&results, &trust_policy);
498 499
499 if matches!(trust_policy, trust::TrustPolicy::Unconfigured) && !warned_unconfigured { 500 if matches!(trust_policy, trust::TrustPolicy::Unconfigured) && !warned_unconfigured
501 {
500 eprintln!("warning: no trusted keys configured — all valid signatures accepted. Run 'collab key add --self' to start."); 502 eprintln!("warning: no trusted keys configured — all valid signatures accepted. Run 'collab key add --self' to start.");
501 warned_unconfigured = true; 503 warned_unconfigured = true;
502 } 504 }
@@ -519,10 +521,7 @@ fn reconcile_refs(
519 } 521 }
520 } 522 }
521 Err(e) => { 523 Err(e) => {
522 eprintln!( 524 eprintln!(" Failed to verify {} {:.8}: {}", kind, id, e);
523 " Failed to verify {} {:.8}: {}",
524 kind, id, e
525 );
526 continue; 525 continue;
527 } 526 }
528 } 527 }
src/trust.rs
Old New
@@ -41,9 +41,9 @@ pub fn global_trusted_keys_path() -> Result<PathBuf, Error> {
41 41
42 /// Validate that a string is a valid base64-encoded 32-byte Ed25519 public key. 42 /// Validate that a string is a valid base64-encoded 32-byte Ed25519 public key.
43 pub fn validate_pubkey(pubkey: &str) -> Result<(), Error> { 43 pub fn validate_pubkey(pubkey: &str) -> Result<(), Error> {
44 let bytes = STANDARD 44 let bytes = STANDARD.decode(pubkey.trim()).map_err(|e| {
45 .decode(pubkey.trim()) 45 Error::Verification(format!("invalid public key: base64 decode failed: {}", e))
46 .map_err(|e| Error::Verification(format!("invalid public key: base64 decode failed: {}", e)))?; 46 })?;
47 if bytes.len() != 32 { 47 if bytes.len() != 32 {
48 return Err(Error::Verification(format!( 48 return Err(Error::Verification(format!(
49 "invalid public key: expected 32 bytes, got {}", 49 "invalid public key: expected 32 bytes, got {}",
@@ -51,8 +51,12 @@ pub fn validate_pubkey(pubkey: &str) -> Result<(), Error> {
51 ))); 51 )));
52 } 52 }
53 let key_bytes: [u8; 32] = bytes.try_into().unwrap(); 53 let key_bytes: [u8; 32] = bytes.try_into().unwrap();
54 VerifyingKey::from_bytes(&key_bytes) 54 VerifyingKey::from_bytes(&key_bytes).map_err(|e| {
55 .map_err(|e| Error::Verification(format!("invalid public key: not a valid Ed25519 point: {}", e)))?; 55 Error::Verification(format!(
56 "invalid public key: not a valid Ed25519 point: {}",
57 e
58 ))
59 })?;
56 Ok(()) 60 Ok(())
57 } 61 }
58 62
@@ -139,7 +143,10 @@ pub fn load_trust_policy(repo: &Repository) -> Result<TrustPolicy, Error> {
139 } 143 }
140 144
141 /// Load trust policy with an explicit global path (for testing). 145 /// Load trust policy with an explicit global path (for testing).
142 pub fn load_trust_policy_with_global(repo: &Repository, global_path: Option<&Path>) -> Result<TrustPolicy, Error> { 146 pub fn load_trust_policy_with_global(
147 repo: &Repository,
148 global_path: Option<&Path>,
149 ) -> Result<TrustPolicy, Error> {
143 let repo_path = trusted_keys_path(repo); 150 let repo_path = trusted_keys_path(repo);
144 let repo_exists = repo_path.exists(); 151 let repo_exists = repo_path.exists();
145 let global_exists = global_path.is_some_and(|p| p.exists()); 152 let global_exists = global_path.is_some_and(|p| p.exists());
@@ -173,7 +180,11 @@ pub fn is_key_trusted(keys: &[TrustedKey], pubkey: &str) -> bool {
173 /// 180 ///
174 /// Creates the parent directory and file if needed. 181 /// Creates the parent directory and file if needed.
175 /// Returns `true` if the key was added, `false` if it was already present. 182 /// Returns `true` if the key was added, `false` if it was already present.
176 pub fn save_trusted_key_to_file(path: &Path, pubkey: &str, label: Option<&str>) -> Result<bool, Error> { 183 pub fn save_trusted_key_to_file(
184 path: &Path,
185 pubkey: &str,
186 label: Option<&str>,
187 ) -> Result<bool, Error> {
177 // Ensure directory exists 188 // Ensure directory exists
178 if let Some(parent) = path.parent() { 189 if let Some(parent) = path.parent() {
179 fs::create_dir_all(parent)?; 190 fs::create_dir_all(parent)?;
@@ -203,7 +214,11 @@ pub fn save_trusted_key_to_file(path: &Path, pubkey: &str, label: Option<&str>)
203 /// 214 ///
204 /// Creates the `.git/collab/` directory and file if needed. 215 /// Creates the `.git/collab/` directory and file if needed.
205 /// Returns `true` if the key was added, `false` if it was already trusted. 216 /// Returns `true` if the key was added, `false` if it was already trusted.
206 pub fn save_trusted_key(repo: &Repository, pubkey: &str, label: Option<&str>) -> Result<bool, Error> { 217 pub fn save_trusted_key(
218 repo: &Repository,
219 pubkey: &str,
220 label: Option<&str>,
221 ) -> Result<bool, Error> {
207 let path = trusted_keys_path(repo); 222 let path = trusted_keys_path(repo);
208 save_trusted_key_to_file(&path, pubkey, label) 223 save_trusted_key_to_file(&path, pubkey, label)
209 } 224 }
@@ -242,9 +257,7 @@ pub fn remove_trusted_key_from_file(path: &Path, pubkey: &str) -> Result<Trusted
242 new_lines.push(line.to_string()); 257 new_lines.push(line.to_string());
243 continue; 258 continue;
244 } 259 }
245 let key_part = trimmed.split_once(' ') 260 let key_part = trimmed.split_once(' ').map(|(k, _)| k).unwrap_or(trimmed);
246 .map(|(k, _)| k)
247 .unwrap_or(trimmed);
248 if key_part != pubkey { 261 if key_part != pubkey {
249 new_lines.push(line.to_string()); 262 new_lines.push(line.to_string());
250 } 263 }
@@ -310,8 +323,8 @@ pub fn check_trust(
310 #[cfg(test)] 323 #[cfg(test)]
311 mod tests { 324 mod tests {
312 use super::*; 325 use super::*;
313 use tempfile::TempDir;
314 use git2::Oid; 326 use git2::Oid;
327 use tempfile::TempDir;
315 328
316 fn test_repo() -> (TempDir, Repository) { 329 fn test_repo() -> (TempDir, Repository) {
317 let dir = TempDir::new().unwrap(); 330 let dir = TempDir::new().unwrap();
@@ -342,7 +355,11 @@ mod tests {
342 let result = validate_pubkey("not-valid-base64!!!"); 355 let result = validate_pubkey("not-valid-base64!!!");
343 assert!(result.is_err()); 356 assert!(result.is_err());
344 let err = result.unwrap_err().to_string(); 357 let err = result.unwrap_err().to_string();
345 assert!(err.contains("base64"), "error should mention base64: {}", err); 358 assert!(
359 err.contains("base64"),
360 "error should mention base64: {}",
361 err
362 );
346 } 363 }
347 364
348 #[test] 365 #[test]
@@ -352,7 +369,11 @@ mod tests {
352 let result = validate_pubkey(&short); 369 let result = validate_pubkey(&short);
353 assert!(result.is_err()); 370 assert!(result.is_err());
354 let err = result.unwrap_err().to_string(); 371 let err = result.unwrap_err().to_string();
355 assert!(err.contains("32 bytes"), "error should mention 32 bytes: {}", err); 372 assert!(
373 err.contains("32 bytes"),
374 "error should mention 32 bytes: {}",
375 err
376 );
356 } 377 }
357 378
358 #[test] 379 #[test]
@@ -511,7 +532,10 @@ mod tests {
511 #[test] 532 #[test]
512 fn check_trust_configured_with_trusted_key_returns_valid() { 533 fn check_trust_configured_with_trusted_key_returns_valid() {
513 let pk = valid_test_pubkey(); 534 let pk = valid_test_pubkey();
514 let keys = vec![TrustedKey { pubkey: pk.clone(), label: None }]; 535 let keys = vec![TrustedKey {
536 pubkey: pk.clone(),
537 label: None,
538 }];
515 let results = vec![make_result(VerifyStatus::Valid, Some(&pk))]; 539 let results = vec![make_result(VerifyStatus::Valid, Some(&pk))];
516 let checked = check_trust(&results, &TrustPolicy::Configured(keys)); 540 let checked = check_trust(&results, &TrustPolicy::Configured(keys));
517 assert_eq!(checked[0].status, VerifyStatus::Valid); 541 assert_eq!(checked[0].status, VerifyStatus::Valid);
@@ -521,7 +545,10 @@ mod tests {
521 fn check_trust_configured_with_untrusted_key_returns_untrusted() { 545 fn check_trust_configured_with_untrusted_key_returns_untrusted() {
522 let pk_trusted = valid_test_pubkey(); 546 let pk_trusted = valid_test_pubkey();
523 let pk_untrusted = valid_test_pubkey(); 547 let pk_untrusted = valid_test_pubkey();
524 let keys = vec![TrustedKey { pubkey: pk_trusted, label: None }]; 548 let keys = vec![TrustedKey {
549 pubkey: pk_trusted,
550 label: None,
551 }];
525 let results = vec![make_result(VerifyStatus::Valid, Some(&pk_untrusted))]; 552 let results = vec![make_result(VerifyStatus::Valid, Some(&pk_untrusted))];
526 let checked = check_trust(&results, &TrustPolicy::Configured(keys)); 553 let checked = check_trust(&results, &TrustPolicy::Configured(keys));
527 assert_eq!(checked[0].status, VerifyStatus::Untrusted); 554 assert_eq!(checked[0].status, VerifyStatus::Untrusted);
@@ -580,7 +607,10 @@ mod tests {
580 let other_pk = valid_test_pubkey(); 607 let other_pk = valid_test_pubkey();
581 let result = remove_trusted_key(&repo, &other_pk); 608 let result = remove_trusted_key(&repo, &other_pk);
582 assert!(result.is_err()); 609 assert!(result.is_err());
583 assert!(result.unwrap_err().to_string().contains("not in the trusted keys list")); 610 assert!(result
611 .unwrap_err()
612 .to_string()
613 .contains("not in the trusted keys list"));
584 } 614 }
585 615
586 #[test] 616 #[test]
@@ -629,8 +659,14 @@ mod tests {
629 TrustPolicy::Configured(keys) => { 659 TrustPolicy::Configured(keys) => {
630 assert_eq!(keys.len(), 2, "should have both global and repo keys"); 660 assert_eq!(keys.len(), 2, "should have both global and repo keys");
631 let pubkeys: Vec<&str> = keys.iter().map(|k| k.pubkey.as_str()).collect(); 661 let pubkeys: Vec<&str> = keys.iter().map(|k| k.pubkey.as_str()).collect();
632 assert!(pubkeys.contains(&pk_global.as_str()), "should contain global key"); 662 assert!(
633 assert!(pubkeys.contains(&pk_repo.as_str()), "should contain repo key"); 663 pubkeys.contains(&pk_global.as_str()),
664 "should contain global key"
665 );
666 assert!(
667 pubkeys.contains(&pk_repo.as_str()),
668 "should contain repo key"
669 );
634 } 670 }
635 _ => panic!("expected Configured"), 671 _ => panic!("expected Configured"),
636 } 672 }
@@ -775,6 +811,9 @@ mod tests {
775 let other_pk = valid_test_pubkey(); 811 let other_pk = valid_test_pubkey();
776 let result = remove_trusted_key_from_file(&global_path, &other_pk); 812 let result = remove_trusted_key_from_file(&global_path, &other_pk);
777 assert!(result.is_err()); 813 assert!(result.is_err());
778 assert!(result.unwrap_err().to_string().contains("not in the trusted keys list")); 814 assert!(result
815 .unwrap_err()
816 .to_string()
817 .contains("not in the trusted keys list"));
779 } 818 }
780 } 819 }
src/tui/events.rs
Old New
@@ -89,8 +89,7 @@ pub(crate) fn run_loop(
89 match issue_mod::open(repo, &title, "", None) { 89 match issue_mod::open(repo, &title, "", None) {
90 Ok(id) => { 90 Ok(id) => {
91 app.reload(repo); 91 app.reload(repo);
92 app.status_msg = 92 app.status_msg = Some(format!("Issue created: {:.8}", id));
93 Some(format!("Issue created: {:.8}", id));
94 } 93 }
95 Err(e) => { 94 Err(e) => {
96 app.status_msg = 95 app.status_msg =
@@ -107,8 +106,7 @@ pub(crate) fn run_loop(
107 match issue_mod::open(repo, &title, &body, None) { 106 match issue_mod::open(repo, &title, &body, None) {
108 Ok(id) => { 107 Ok(id) => {
109 app.reload(repo); 108 app.reload(repo);
110 app.status_msg = 109 app.status_msg = Some(format!("Issue created: {:.8}", id));
111 Some(format!("Issue created: {:.8}", id));
112 } 110 }
113 Err(e) => { 111 Err(e) => {
114 app.status_msg = 112 app.status_msg =
@@ -161,9 +159,7 @@ pub(crate) fn run_loop(
161 .find(|p| p.fixes.as_deref() == Some(&issue.id)) 159 .find(|p| p.fixes.as_deref() == Some(&issue.id))
162 .map(|p| p.branch.clone()) 160 .map(|p| p.branch.clone())
163 // Fall back to closing commit 161 // Fall back to closing commit
164 .or_else(|| { 162 .or_else(|| issue.closed_by.map(|oid| oid.to_string()))
165 issue.closed_by.map(|oid| oid.to_string())
166 })
167 }) 163 })
168 }; 164 };
169 if let Some(head) = checkout_target { 165 if let Some(head) = checkout_target {
@@ -187,8 +183,7 @@ pub(crate) fn run_loop(
187 } 183 }
188 return Ok(()); 184 return Ok(());
189 } else { 185 } else {
190 app.status_msg = 186 app.status_msg = Some("No linked patch to check out".to_string());
191 Some("No linked patch to check out".to_string());
192 } 187 }
193 continue; 188 continue;
194 } 189 }
@@ -227,8 +222,7 @@ pub(crate) fn run_loop(
227 app.scroll = 0; 222 app.scroll = 0;
228 } 223 }
229 Err(e) => { 224 Err(e) => {
230 app.status_msg = 225 app.status_msg = Some(format!("Error loading events: {}", e));
231 Some(format!("Error loading events: {}", e));
232 } 226 }
233 } 227 }
234 } 228 }
src/tui/mod.rs
Old New
@@ -868,10 +868,7 @@ mod tests {
868 fn test_selected_ref_name_issues() { 868 fn test_selected_ref_name_issues() {
869 let app = make_app(3, 0); 869 let app = make_app(3, 0);
870 let ref_name = app.selected_ref_name(); 870 let ref_name = app.selected_ref_name();
871 assert_eq!( 871 assert_eq!(ref_name, Some("refs/collab/issues/00000000".to_string()));
872 ref_name,
873 Some("refs/collab/issues/00000000".to_string())
874 );
875 } 872 }
876 873
877 #[test] 874 #[test]
src/tui/state.rs
Old New
@@ -153,7 +153,9 @@ impl App {
153 .iter() 153 .iter()
154 .filter(|p| match self.status_filter { 154 .filter(|p| match self.status_filter {
155 StatusFilter::Open => p.status == PatchStatus::Open, 155 StatusFilter::Open => p.status == PatchStatus::Open,
156 StatusFilter::Closed => p.status == PatchStatus::Closed || p.status == PatchStatus::Merged, 156 StatusFilter::Closed => {
157 p.status == PatchStatus::Closed || p.status == PatchStatus::Merged
158 }
157 StatusFilter::All => true, 159 StatusFilter::All => true,
158 }) 160 })
159 .filter(|p| self.matches_search(&p.title)) 161 .filter(|p| self.matches_search(&p.title))
src/tui/widgets.rs
Old New
@@ -83,7 +83,9 @@ pub(crate) fn format_event_detail(oid: &Oid, event: &crate::event::Event) -> Str
83 detail.push_str(&format!("\n{}\n", body)); 83 detail.push_str(&format!("\n{}\n", body));
84 } 84 }
85 } 85 }
86 Action::PatchInlineComment { file, line, body, .. } => { 86 Action::PatchInlineComment {
87 file, line, body, ..
88 } => {
87 detail.push_str(&format!("\nFile: {}:{}\n", file, line)); 89 detail.push_str(&format!("\nFile: {}:{}\n", file, line));
88 if !body.is_empty() { 90 if !body.is_empty() {
89 detail.push_str(&format!("\n{}\n", body)); 91 detail.push_str(&format!("\n{}\n", body));
@@ -120,10 +122,7 @@ pub(crate) fn format_event_detail(oid: &Oid, event: &crate::event::Event) -> Str
120 pub(crate) fn ui(frame: &mut Frame, app: &mut App) { 122 pub(crate) fn ui(frame: &mut Frame, app: &mut App) {
121 let chunks = Layout::default() 123 let chunks = Layout::default()
122 .direction(Direction::Vertical) 124 .direction(Direction::Vertical)
123 .constraints([ 125 .constraints([Constraint::Min(1), Constraint::Length(1)])
124 Constraint::Min(1),
125 Constraint::Length(1),
126 ])
127 .split(frame.area()); 126 .split(frame.area());
128 127
129 let main_area = chunks[0]; 128 let main_area = chunks[0];
@@ -390,10 +389,7 @@ fn build_issue_detail(issue: &IssueState, patches: &[PatchState]) -> Text<'stati
390 if let Some(ref oid) = issue.closed_by { 389 if let Some(ref oid) = issue.closed_by {
391 lines.push(Line::from(vec![ 390 lines.push(Line::from(vec![
392 Span::styled("Commit: ", Style::default().fg(Color::DarkGray)), 391 Span::styled("Commit: ", Style::default().fg(Color::DarkGray)),
393 Span::styled( 392 Span::styled(format!("{:.8}", oid), Style::default().fg(Color::Cyan)),
394 format!("{:.8}", oid),
395 Style::default().fg(Color::Cyan),
396 ),
397 ])); 393 ]));
398 } 394 }
399 395
@@ -417,10 +413,7 @@ fn build_issue_detail(issue: &IssueState, patches: &[PatchState]) -> Text<'stati
417 PatchStatus::Merged => ("merged", Color::Cyan), 413 PatchStatus::Merged => ("merged", Color::Cyan),
418 }; 414 };
419 lines.push(Line::from(vec![ 415 lines.push(Line::from(vec![
420 Span::styled( 416 Span::styled(format!("{:.8}", p.id), Style::default().fg(Color::Yellow)),
421 format!("{:.8}", p.id),
422 Style::default().fg(Color::Yellow),
423 ),
424 Span::raw(" "), 417 Span::raw(" "),
425 Span::styled(status.0, Style::default().fg(status.1)), 418 Span::styled(status.0, Style::default().fg(status.1)),
426 Span::raw(format!(" {}", p.title)), 419 Span::raw(format!(" {}", p.title)),
@@ -589,7 +582,9 @@ fn build_patch_detail_text(app: &App) -> Text<'static> {
589 lines.push(Line::raw("")); 582 lines.push(Line::raw(""));
590 lines.push(Line::styled( 583 lines.push(Line::styled(
591 warning.clone(), 584 warning.clone(),
592 Style::default().fg(Color::Yellow).add_modifier(Modifier::BOLD), 585 Style::default()
586 .fg(Color::Yellow)
587 .add_modifier(Modifier::BOLD),
593 )); 588 ));
594 } 589 }
595 } 590 }
@@ -615,9 +610,10 @@ fn build_patch_detail_text(app: &App) -> Text<'static> {
615 " " 610 " "
616 }; 611 };
617 let label = if i == 0 { " (initial)" } else { "" }; 612 let label = if i == 0 { " (initial)" } else { "" };
618 lines.push(Line::from(vec![ 613 lines.push(Line::from(vec![Span::raw(format!(
619 Span::raw(format!("{}r{} {} {}{}", marker, rev.number, short, rev.timestamp, label)), 614 "{}r{} {} {}{}",
620 ])); 615 marker, rev.number, short, rev.timestamp, label
616 ))]));
621 } 617 }
622 } 618 }
623 619
@@ -719,14 +715,22 @@ fn build_patch_detail_text(app: &App) -> Text<'static> {
719 let diff_mode = if app.patch_interdiff_mode { 715 let diff_mode = if app.patch_interdiff_mode {
720 let rev_idx = app.patch_revision_idx; 716 let rev_idx = app.patch_revision_idx;
721 if rev_idx > 0 { 717 if rev_idx > 0 {
722 let from_rev = patch.revisions.get(rev_idx - 1).map(|r| r.number).unwrap_or(0); 718 let from_rev = patch
719 .revisions
720 .get(rev_idx - 1)
721 .map(|r| r.number)
722 .unwrap_or(0);
723 let to_rev = patch.revisions.get(rev_idx).map(|r| r.number).unwrap_or(0); 723 let to_rev = patch.revisions.get(rev_idx).map(|r| r.number).unwrap_or(0);
724 format!("--- Interdiff r{} -> r{} ---", from_rev, to_rev) 724 format!("--- Interdiff r{} -> r{} ---", from_rev, to_rev)
725 } else { 725 } else {
726 "--- Diff vs base (no previous revision) ---".to_string() 726 "--- Diff vs base (no previous revision) ---".to_string()
727 } 727 }
728 } else { 728 } else {
729 let rev_num = patch.revisions.get(app.patch_revision_idx).map(|r| r.number).unwrap_or(1); 729 let rev_num = patch
730 .revisions
731 .get(app.patch_revision_idx)
732 .map(|r| r.number)
733 .unwrap_or(1);
730 format!("--- Diff r{} vs base ---", rev_num) 734 format!("--- Diff r{} vs base ---", rev_num)
731 }; 735 };
732 lines.push(Line::styled( 736 lines.push(Line::styled(
@@ -748,7 +752,9 @@ fn build_patch_detail_text(app: &App) -> Text<'static> {
748 } else if l.starts_with("@@") { 752 } else if l.starts_with("@@") {
749 Style::default().fg(Color::Cyan) 753 Style::default().fg(Color::Cyan)
750 } else if l.starts_with("diff ") { 754 } else if l.starts_with("diff ") {
751 Style::default().fg(Color::Yellow).add_modifier(Modifier::BOLD) 755 Style::default()
756 .fg(Color::Yellow)
757 .add_modifier(Modifier::BOLD)
752 } else { 758 } else {
753 Style::default() 759 Style::default()
754 }; 760 };
@@ -805,8 +811,8 @@ fn render_footer(frame: &mut Frame, app: &App, area: Rect) {
805 811
806 // Show status message if present 812 // Show status message if present
807 if let Some(ref msg) = app.status_msg { 813 if let Some(ref msg) = app.status_msg {
808 let para = 814 let para = Paragraph::new(format!(" {}", msg))
809 Paragraph::new(format!(" {}", msg)).style(Style::default().bg(Color::Yellow).fg(Color::Black)); 815 .style(Style::default().bg(Color::Yellow).fg(Color::Black));
810 frame.render_widget(para, area); 816 frame.render_widget(para, area);
811 return; 817 return;
812 } 818 }
@@ -814,7 +820,9 @@ fn render_footer(frame: &mut Frame, app: &App, area: Rect) {
814 let text = match app.mode { 820 let text = match app.mode {
815 ViewMode::CommitList => " j/k:navigate Enter:detail Esc:back q:quit".to_string(), 821 ViewMode::CommitList => " j/k:navigate Enter:detail Esc:back q:quit".to_string(),
816 ViewMode::CommitDetail => " j/k:scroll Esc:back q:quit".to_string(), 822 ViewMode::CommitDetail => " j/k:scroll Esc:back q:quit".to_string(),
817 ViewMode::PatchDetail => " j/k:scroll Esc:back [/]:revision d:interdiff q:quit".to_string(), 823 ViewMode::PatchDetail => {
824 " j/k:scroll Esc:back [/]:revision d:interdiff q:quit".to_string()
825 }
818 ViewMode::Details => { 826 ViewMode::Details => {
819 let list_hint = match app.list_mode { 827 let list_hint = match app.list_mode {
820 ListMode::Issues => "P:patches", 828 ListMode::Issues => "P:patches",
tests/adversarial_test.rs
Old New
@@ -110,7 +110,10 @@ fn missing_type_field_returns_error() {
110 let json = br#"{"timestamp":"t","author":{"name":"a","email":"e"}}"#; 110 let json = br#"{"timestamp":"t","author":{"name":"a","email":"e"}}"#;
111 let (_tmp, repo, ref_name) = repo_with_blob(json); 111 let (_tmp, repo, ref_name) = repo_with_blob(json);
112 let result = dag::walk_events(&repo, &ref_name); 112 let result = dag::walk_events(&repo, &ref_name);
113 assert!(result.is_err(), "missing action/type field should return Err"); 113 assert!(
114 result.is_err(),
115 "missing action/type field should return Err"
116 );
114 } 117 }
115 118
116 #[test] 119 #[test]
@@ -306,7 +309,8 @@ fn dag_with_one_corrupted_commit_in_middle() {
306 .unwrap(); 309 .unwrap();
307 let mut tb = repo.treebuilder(None).unwrap(); 310 let mut tb = repo.treebuilder(None).unwrap();
308 tb.insert("event.json", good_blob, 0o100644).unwrap(); 311 tb.insert("event.json", good_blob, 0o100644).unwrap();
309 tb.insert("manifest.json", manifest_blob2, 0o100644).unwrap(); 312 tb.insert("manifest.json", manifest_blob2, 0o100644)
313 .unwrap();
310 let tree_oid = tb.write().unwrap(); 314 let tree_oid = tb.write().unwrap();
311 let tree = repo.find_tree(tree_oid).unwrap(); 315 let tree = repo.find_tree(tree_oid).unwrap();
312 let parent2 = repo.find_commit(oid2).unwrap(); 316 let parent2 = repo.find_commit(oid2).unwrap();
@@ -383,10 +387,7 @@ fn blob_just_over_limit_returns_error() {
383 let content = vec![b'x'; MAX_EVENT_BLOB_SIZE + 1]; 387 let content = vec![b'x'; MAX_EVENT_BLOB_SIZE + 1];
384 let (_tmp, repo, ref_name) = repo_with_blob(&content); 388 let (_tmp, repo, ref_name) = repo_with_blob(&content);
385 let result = dag::walk_events(&repo, &ref_name); 389 let result = dag::walk_events(&repo, &ref_name);
386 assert!( 390 assert!(result.is_err(), "blob just over limit should return Err");
387 result.is_err(),
388 "blob just over limit should return Err"
389 );
390 } 391 }
391 392
392 // =========================================================================== 393 // ===========================================================================
@@ -410,11 +411,7 @@ fn ref_id_with_control_chars_rejected() {
410 for ch in &['\n', '\r', '\t'] { 411 for ch in &['\n', '\r', '\t'] {
411 let id = format!("abcdef1234567890abcdef1234567890abcdef1{}", ch); 412 let id = format!("abcdef1234567890abcdef1234567890abcdef1{}", ch);
412 let result = validate_collab_ref_id(&id); 413 let result = validate_collab_ref_id(&id);
413 assert!( 414 assert!(result.is_err(), "control char {:?} should be rejected", ch);
414 result.is_err(),
415 "control char {:?} should be rejected",
416 ch
417 );
418 } 415 }
419 } 416 }
420 417
@@ -431,8 +428,9 @@ fn ref_id_wrong_length_rejected() {
431 assert!(result.is_err(), "6-char ID should be rejected"); 428 assert!(result.is_err(), "6-char ID should be rejected");
432 429
433 // Too long 430 // Too long
434 let result = 431 let result = validate_collab_ref_id(
435 validate_collab_ref_id("abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef12345678ab"); 432 "abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef12345678ab",
433 );
436 assert!(result.is_err(), "80-char ID should be rejected"); 434 assert!(result.is_err(), "80-char ID should be rejected");
437 } 435 }
438 436
@@ -522,16 +520,17 @@ use proptest::prelude::*;
522 520
523 /// Generate an arbitrary Author 521 /// Generate an arbitrary Author
524 fn arb_author() -> impl Strategy<Value = Author> { 522 fn arb_author() -> impl Strategy<Value = Author> {
525 ("[a-zA-Z0-9 ]{0,50}", "[a-zA-Z0-9@.]{0,50}").prop_map(|(name, email)| Author { 523 ("[a-zA-Z0-9 ]{0,50}", "[a-zA-Z0-9@.]{0,50}").prop_map(|(name, email)| Author { name, email })
526 name,
527 email,
528 })
529 } 524 }
530 525
531 /// Generate an arbitrary Action variant 526 /// Generate an arbitrary Action variant
532 fn arb_action() -> impl Strategy<Value = Action> { 527 fn arb_action() -> impl Strategy<Value = Action> {
533 prop_oneof![ 528 prop_oneof![
534 (".*", ".*").prop_map(|(title, body)| Action::IssueOpen { title, body, relates_to: None }), 529 (".*", ".*").prop_map(|(title, body)| Action::IssueOpen {
530 title,
531 body,
532 relates_to: None
533 }),
535 ".*".prop_map(|body| Action::IssueComment { body }), 534 ".*".prop_map(|body| Action::IssueComment { body }),
536 proptest::option::of(".*").prop_map(|reason| Action::IssueClose { reason }), 535 proptest::option::of(".*").prop_map(|reason| Action::IssueClose { reason }),
537 Just(Action::IssueReopen), 536 Just(Action::IssueReopen),
@@ -661,7 +660,8 @@ fn merge_commit_with_one_corrupted_parent() {
661 .unwrap(); 660 .unwrap();
662 let mut tb = repo.treebuilder(None).unwrap(); 661 let mut tb = repo.treebuilder(None).unwrap();
663 tb.insert("event.json", bad_blob, 0o100644).unwrap(); 662 tb.insert("event.json", bad_blob, 0o100644).unwrap();
664 tb.insert("manifest.json", manifest_blob2, 0o100644).unwrap(); 663 tb.insert("manifest.json", manifest_blob2, 0o100644)
664 .unwrap();
665 let tree_oid = tb.write().unwrap(); 665 let tree_oid = tb.write().unwrap();
666 let tree = repo.find_tree(tree_oid).unwrap(); 666 let tree = repo.find_tree(tree_oid).unwrap();
667 let corrupted_oid = repo 667 let corrupted_oid = repo
@@ -682,7 +682,8 @@ fn merge_commit_with_one_corrupted_parent() {
682 .unwrap(); 682 .unwrap();
683 let mut tb = repo.treebuilder(None).unwrap(); 683 let mut tb = repo.treebuilder(None).unwrap();
684 tb.insert("event.json", merge_blob, 0o100644).unwrap(); 684 tb.insert("event.json", merge_blob, 0o100644).unwrap();
685 tb.insert("manifest.json", manifest_blob3, 0o100644).unwrap(); 685 tb.insert("manifest.json", manifest_blob3, 0o100644)
686 .unwrap();
686 let tree_oid = tb.write().unwrap(); 687 let tree_oid = tb.write().unwrap();
687 let tree = repo.find_tree(tree_oid).unwrap(); 688 let tree = repo.find_tree(tree_oid).unwrap();
688 689
tests/cache_test.rs
Old New
@@ -1,6 +1,8 @@
1 mod common; 1 mod common;
2 2
3 use common::{alice, bob, add_comment, close_issue, init_repo, open_issue, create_patch, add_review}; 3 use common::{
4 add_comment, add_review, alice, bob, close_issue, create_patch, init_repo, open_issue,
5 };
4 use git_collab::cache; 6 use git_collab::cache;
5 use git_collab::event::ReviewVerdict; 7 use git_collab::event::ReviewVerdict;
6 use git_collab::state::{IssueState, IssueStatus, PatchState, PatchStatus}; 8 use git_collab::state::{IssueState, IssueStatus, PatchState, PatchStatus};
tests/cli_test.rs
Old New
@@ -425,13 +425,7 @@ fn test_patch_revise() {
425 repo.commit_file("v2.txt", "v2", "version 2"); 425 repo.commit_file("v2.txt", "v2", "version 2");
426 repo.git(&["checkout", "main"]); 426 repo.git(&["checkout", "main"]);
427 427
428 let out = repo.run_ok(&[ 428 let out = repo.run_ok(&["patch", "revise", &id, "-b", "Updated implementation"]);
429 "patch",
430 "revise",
431 &id,
432 "-b",
433 "Updated implementation",
434 ]);
435 assert!(out.contains("Patch revised")); 429 assert!(out.contains("Patch revised"));
436 430
437 let out = repo.run_ok(&["patch", "show", &id]); 431 let out = repo.run_ok(&["patch", "show", &id]);
@@ -532,10 +526,14 @@ fn test_patch_create_with_fixes() {
532 repo.git(&["checkout", "main"]); 526 repo.git(&["checkout", "main"]);
533 527
534 let out = repo.run_ok(&[ 528 let out = repo.run_ok(&[
535 "patch", "create", 529 "patch",
536 "-t", "Fix login bug", 530 "create",
537 "-B", "fix", 531 "-t",
538 "--fixes", &issue_id, 532 "Fix login bug",
533 "-B",
534 "fix",
535 "--fixes",
536 &issue_id,
539 ]); 537 ]);
540 let patch_id = out.trim().strip_prefix("Created patch ").unwrap(); 538 let patch_id = out.trim().strip_prefix("Created patch ").unwrap();
541 539
@@ -545,7 +543,6 @@ fn test_patch_create_with_fixes() {
545 assert!(out.contains(&issue_id[..8]), "should show linked issue ID"); 543 assert!(out.contains(&issue_id[..8]), "should show linked issue ID");
546 } 544 }
547 545
548
549 // =========================================================================== 546 // ===========================================================================
550 // Unread tracking 547 // Unread tracking
551 // =========================================================================== 548 // ===========================================================================
@@ -628,11 +625,31 @@ fn test_status_command_reports_summary_and_recent_items() {
628 repo.patch_create("Status patch"); 625 repo.patch_create("Status patch");
629 626
630 let out = repo.run_ok(&["status"]); 627 let out = repo.run_ok(&["status"]);
631 assert!(out.contains("Issues: 1 open, 0 closed"), "unexpected status output: {}", out); 628 assert!(
632 assert!(out.contains("Patches: 1 open, 0 merged, 0 closed"), "unexpected status output: {}", out); 629 out.contains("Issues: 1 open, 0 closed"),
633 assert!(out.contains("Recently updated:"), "unexpected status output: {}", out); 630 "unexpected status output: {}",
634 assert!(out.contains("[issue]") && out.contains("Status bug"), "unexpected status output: {}", out); 631 out
635 assert!(out.contains("[patch]") && out.contains("Status patch"), "unexpected status output: {}", out); 632 );
633 assert!(
634 out.contains("Patches: 1 open, 0 merged, 0 closed"),
635 "unexpected status output: {}",
636 out
637 );
638 assert!(
639 out.contains("Recently updated:"),
640 "unexpected status output: {}",
641 out
642 );
643 assert!(
644 out.contains("[issue]") && out.contains("Status bug"),
645 "unexpected status output: {}",
646 out
647 );
648 assert!(
649 out.contains("[patch]") && out.contains("Status patch"),
650 "unexpected status output: {}",
651 out
652 );
636 } 653 }
637 654
638 #[test] 655 #[test]
@@ -643,15 +660,44 @@ fn test_log_command_shows_recent_collab_events_and_limit() {
643 repo.patch_create("Logged patch"); 660 repo.patch_create("Logged patch");
644 661
645 let out = repo.run_ok(&["log"]); 662 let out = repo.run_ok(&["log"]);
646 assert!(out.contains("IssueOpen issue"), "unexpected log output: {}", out); 663 assert!(
647 assert!(out.contains("IssueComment issue"), "unexpected log output: {}", out); 664 out.contains("IssueOpen issue"),
648 assert!(out.contains("PatchCreate patch"), "unexpected log output: {}", out); 665 "unexpected log output: {}",
649 assert!(out.contains("open \"Logged issue\""), "unexpected log output: {}", out); 666 out
650 assert!(out.contains("Logged comment"), "unexpected log output: {}", out); 667 );
651 assert!(out.contains("create \"Logged patch\""), "unexpected log output: {}", out); 668 assert!(
669 out.contains("IssueComment issue"),
670 "unexpected log output: {}",
671 out
672 );
673 assert!(
674 out.contains("PatchCreate patch"),
675 "unexpected log output: {}",
676 out
677 );
678 assert!(
679 out.contains("open \"Logged issue\""),
680 "unexpected log output: {}",
681 out
682 );
683 assert!(
684 out.contains("Logged comment"),
685 "unexpected log output: {}",
686 out
687 );
688 assert!(
689 out.contains("create \"Logged patch\""),
690 "unexpected log output: {}",
691 out
692 );
652 693
653 let limited = repo.run_ok(&["log", "-n", "2"]); 694 let limited = repo.run_ok(&["log", "-n", "2"]);
654 assert_eq!(limited.lines().count(), 2, "expected exactly 2 log lines, got: {}", limited); 695 assert_eq!(
696 limited.lines().count(),
697 2,
698 "expected exactly 2 log lines, got: {}",
699 limited
700 );
655 } 701 }
656 702
657 // =========================================================================== 703 // ===========================================================================
@@ -697,7 +743,14 @@ fn test_full_patch_review_cycle() {
697 repo.commit_file("feature.rs", "fn hello() {}", "v1 of feature"); 743 repo.commit_file("feature.rs", "fn hello() {}", "v1 of feature");
698 repo.git(&["checkout", "main"]); 744 repo.git(&["checkout", "main"]);
699 745
700 let out = repo.run_ok(&["patch", "create", "-t", "Add hello function", "-B", "feature"]); 746 let out = repo.run_ok(&[
747 "patch",
748 "create",
749 "-t",
750 "Add hello function",
751 "-B",
752 "feature",
753 ]);
701 let id = out 754 let id = out
702 .trim() 755 .trim()
703 .strip_prefix("Created patch ") 756 .strip_prefix("Created patch ")
@@ -740,13 +793,7 @@ fn test_full_patch_review_cycle() {
740 ); 793 );
741 repo.git(&["checkout", "main"]); 794 repo.git(&["checkout", "main"]);
742 795
743 repo.run_ok(&[ 796 repo.run_ok(&["patch", "revise", &id, "-b", "Added documentation"]);
744 "patch",
745 "revise",
746 &id,
747 "-b",
748 "Added documentation",
749 ]);
750 797
751 // Approve 798 // Approve
752 repo.run_ok(&["patch", "review", &id, "-v", "approve", "-b", "LGTM now"]); 799 repo.run_ok(&["patch", "review", &id, "-v", "approve", "-b", "LGTM now"]);
@@ -782,7 +829,10 @@ fn test_init_key_creates_key_files() {
782 let repo_dir = tempfile::TempDir::new().unwrap(); 829 let repo_dir = tempfile::TempDir::new().unwrap();
783 common::git_cmd(repo_dir.path(), &["init", "-b", "main"]); 830 common::git_cmd(repo_dir.path(), &["init", "-b", "main"]);
784 common::git_cmd(repo_dir.path(), &["config", "user.name", "Alice"]); 831 common::git_cmd(repo_dir.path(), &["config", "user.name", "Alice"]);
785 common::git_cmd(repo_dir.path(), &["config", "user.email", "alice@example.com"]); 832 common::git_cmd(
833 repo_dir.path(),
834 &["config", "user.email", "alice@example.com"],
835 );
786 common::git_cmd(repo_dir.path(), &["commit", "--allow-empty", "-m", "init"]); 836 common::git_cmd(repo_dir.path(), &["commit", "--allow-empty", "-m", "init"]);
787 837
788 // Run init-key with overridden HOME 838 // Run init-key with overridden HOME
@@ -802,12 +852,21 @@ fn test_init_key_creates_key_files() {
802 stdout, 852 stdout,
803 stderr 853 stderr
804 ); 854 );
805 assert!(stdout.contains("Signing key generated"), "should print success message"); 855 assert!(
856 stdout.contains("Signing key generated"),
857 "should print success message"
858 );
806 assert!(stdout.contains("Public key:"), "should print public key"); 859 assert!(stdout.contains("Public key:"), "should print public key");
807 860
808 // Key files should exist 861 // Key files should exist
809 assert!(config_dir.join("signing-key").exists(), "private key file should exist"); 862 assert!(
810 assert!(config_dir.join("signing-key.pub").exists(), "public key file should exist"); 863 config_dir.join("signing-key").exists(),
864 "private key file should exist"
865 );
866 assert!(
867 config_dir.join("signing-key.pub").exists(),
868 "public key file should exist"
869 );
811 870
812 // Run init-key again without --force: should fail 871 // Run init-key again without --force: should fail
813 let output = std::process::Command::new(env!("CARGO_BIN_EXE_git-collab")) 872 let output = std::process::Command::new(env!("CARGO_BIN_EXE_git-collab"))
tests/collab_test.rs
Old New
@@ -156,7 +156,8 @@ fn test_concurrent_comments_create_fork_and_reconcile() {
156 repo.reference(&ref_name, alice_tip, true, "restore alice tip") 156 repo.reference(&ref_name, alice_tip, true, "restore alice tip")
157 .unwrap(); 157 .unwrap();
158 158
159 let (merge_oid, outcome) = dag::reconcile(&repo, &ref_name, &remote_ref, &alice(), &test_signing_key()).unwrap(); 159 let (merge_oid, outcome) =
160 dag::reconcile(&repo, &ref_name, &remote_ref, &alice(), &test_signing_key()).unwrap();
160 assert_eq!(outcome, dag::ReconcileOutcome::Merge); 161 assert_eq!(outcome, dag::ReconcileOutcome::Merge);
161 162
162 let merge_commit = repo.find_commit(merge_oid).unwrap(); 163 let merge_commit = repo.find_commit(merge_oid).unwrap();
@@ -264,7 +265,8 @@ fn test_fast_forward_reconcile() {
264 repo.reference(&remote_ref, ahead_tip, true, "remote ahead") 265 repo.reference(&remote_ref, ahead_tip, true, "remote ahead")
265 .unwrap(); 266 .unwrap();
266 267
267 let (result, outcome) = dag::reconcile(&repo, &ref_name, &remote_ref, &alice(), &test_signing_key()).unwrap(); 268 let (result, outcome) =
269 dag::reconcile(&repo, &ref_name, &remote_ref, &alice(), &test_signing_key()).unwrap();
268 assert_eq!(result, ahead_tip, "should fast-forward to remote tip"); 270 assert_eq!(result, ahead_tip, "should fast-forward to remote tip");
269 assert_eq!(outcome, dag::ReconcileOutcome::FastForward); 271 assert_eq!(outcome, dag::ReconcileOutcome::FastForward);
270 272
@@ -286,7 +288,8 @@ fn test_no_op_when_already_in_sync() {
286 let remote_ref = format!("refs/collab/sync/origin/issues/{}", id); 288 let remote_ref = format!("refs/collab/sync/origin/issues/{}", id);
287 repo.reference(&remote_ref, tip, true, "same tip").unwrap(); 289 repo.reference(&remote_ref, tip, true, "same tip").unwrap();
288 290
289 let (result, outcome) = dag::reconcile(&repo, &ref_name, &remote_ref, &alice(), &test_signing_key()).unwrap(); 291 let (result, outcome) =
292 dag::reconcile(&repo, &ref_name, &remote_ref, &alice(), &test_signing_key()).unwrap();
290 assert_eq!(result, tip); 293 assert_eq!(result, tip);
291 assert_eq!(outcome, dag::ReconcileOutcome::AlreadyCurrent); 294 assert_eq!(outcome, dag::ReconcileOutcome::AlreadyCurrent);
292 } 295 }
@@ -306,7 +309,8 @@ fn test_local_ahead_no_merge() {
306 repo.reference(&remote_ref, root_oid, true, "remote behind") 309 repo.reference(&remote_ref, root_oid, true, "remote behind")
307 .unwrap(); 310 .unwrap();
308 311
309 let (result, outcome) = dag::reconcile(&repo, &ref_name, &remote_ref, &alice(), &test_signing_key()).unwrap(); 312 let (result, outcome) =
313 dag::reconcile(&repo, &ref_name, &remote_ref, &alice(), &test_signing_key()).unwrap();
310 assert_eq!(result, local_tip, "local should stay ahead"); 314 assert_eq!(result, local_tip, "local should stay ahead");
311 assert_eq!(outcome, dag::ReconcileOutcome::LocalAhead); 315 assert_eq!(outcome, dag::ReconcileOutcome::LocalAhead);
312 } 316 }
@@ -347,7 +351,10 @@ fn test_patch_review_workflow() {
347 assert_eq!(state.reviews.len(), 2); 351 assert_eq!(state.reviews.len(), 2);
348 // PatchRevision body is on the revision, not the patch body 352 // PatchRevision body is on the revision, not the patch body
349 assert_eq!(state.revisions.len(), 2); // revision 1 from create + revision 2 from PatchRevision 353 assert_eq!(state.revisions.len(), 2); // revision 1 from create + revision 2 from PatchRevision
350 assert_eq!(state.revisions[1].body.as_deref(), Some("Updated implementation")); 354 assert_eq!(
355 state.revisions[1].body.as_deref(),
356 Some("Updated implementation")
357 );
351 } 358 }
352 359
353 #[test] 360 #[test]
@@ -535,7 +542,9 @@ fn test_signed_event_in_dag() {
535 assert!(matches!(event.action, Action::IssueOpen { .. })); 542 assert!(matches!(event.action, Action::IssueOpen { .. }));
536 543
537 // signature and pubkey should be separate blobs 544 // signature and pubkey should be separate blobs
538 let sig_entry = tree.get_name("signature").expect("signature blob should exist"); 545 let sig_entry = tree
546 .get_name("signature")
547 .expect("signature blob should exist");
539 let pk_entry = tree.get_name("pubkey").expect("pubkey blob should exist"); 548 let pk_entry = tree.get_name("pubkey").expect("pubkey blob should exist");
540 let sig_blob = repo.find_blob(sig_entry.id()).unwrap(); 549 let sig_blob = repo.find_blob(sig_entry.id()).unwrap();
541 let pk_blob = repo.find_blob(pk_entry.id()).unwrap(); 550 let pk_blob = repo.find_blob(pk_entry.id()).unwrap();
@@ -545,11 +554,19 @@ fn test_signed_event_in_dag() {
545 assert!(!pk_str.is_empty(), "pubkey should be present"); 554 assert!(!pk_str.is_empty(), "pubkey should be present");
546 555
547 // manifest.json should exist 556 // manifest.json should exist
548 let manifest_entry = tree.get_name("manifest.json").expect("manifest.json should exist"); 557 let manifest_entry = tree
558 .get_name("manifest.json")
559 .expect("manifest.json should exist");
549 let manifest_blob = repo.find_blob(manifest_entry.id()).unwrap(); 560 let manifest_blob = repo.find_blob(manifest_entry.id()).unwrap();
550 let manifest_str = std::str::from_utf8(manifest_blob.content()).unwrap(); 561 let manifest_str = std::str::from_utf8(manifest_blob.content()).unwrap();
551 assert!(manifest_str.contains("\"version\":1"), "manifest should contain version"); 562 assert!(
552 assert!(manifest_str.contains("\"format\":\"git-collab\""), "manifest should contain format"); 563 manifest_str.contains("\"version\":1"),
564 "manifest should contain version"
565 );
566 assert!(
567 manifest_str.contains("\"format\":\"git-collab\""),
568 "manifest should contain format"
569 );
553 570
554 // Verify the detached signature 571 // Verify the detached signature
555 let detached = DetachedSignature { 572 let detached = DetachedSignature {
@@ -557,7 +574,11 @@ fn test_signed_event_in_dag() {
557 pubkey: pk_str.to_string(), 574 pubkey: pk_str.to_string(),
558 }; 575 };
559 let status = signing::verify_detached(&event, &detached).unwrap(); 576 let status = signing::verify_detached(&event, &detached).unwrap();
560 assert_eq!(status, VerifyStatus::Valid, "signature should verify as valid"); 577 assert_eq!(
578 status,
579 VerifyStatus::Valid,
580 "signature should verify as valid"
581 );
561 582
562 // walk_events should still extract the Event correctly 583 // walk_events should still extract the Event correctly
563 let events = dag::walk_events(&repo, &ref_name).unwrap(); 584 let events = dag::walk_events(&repo, &ref_name).unwrap();
@@ -597,7 +618,9 @@ fn make_initial_commit(repo: &git2::Repository, branch: &str) -> git2::Oid {
597 tb.insert("README.md", blob, 0o100644).unwrap(); 618 tb.insert("README.md", blob, 0o100644).unwrap();
598 let tree_oid = tb.write().unwrap(); 619 let tree_oid = tb.write().unwrap();
599 let tree = repo.find_tree(tree_oid).unwrap(); 620 let tree = repo.find_tree(tree_oid).unwrap();
600 let oid = repo.commit(None, &sig, &sig, "initial commit", &tree, &[]).unwrap(); 621 let oid = repo
622 .commit(None, &sig, &sig, "initial commit", &tree, &[])
623 .unwrap();
601 let ref_name = format!("refs/heads/{}", branch); 624 let ref_name = format!("refs/heads/{}", branch);
602 repo.reference(&ref_name, oid, true, "init branch").unwrap(); 625 repo.reference(&ref_name, oid, true, "init branch").unwrap();
603 // Set HEAD to this branch 626 // Set HEAD to this branch
@@ -606,7 +629,12 @@ fn make_initial_commit(repo: &git2::Repository, branch: &str) -> git2::Oid {
606 } 629 }
607 630
608 /// Add a commit on the given branch, returns the new OID. 631 /// Add a commit on the given branch, returns the new OID.
609 fn add_commit_on_branch(repo: &git2::Repository, branch: &str, filename: &str, content: &[u8]) -> git2::Oid { 632 fn add_commit_on_branch(
633 repo: &git2::Repository,
634 branch: &str,
635 filename: &str,
636 content: &[u8],
637 ) -> git2::Oid {
610 let ref_name = format!("refs/heads/{}", branch); 638 let ref_name = format!("refs/heads/{}", branch);
611 let parent_oid = repo.refname_to_id(&ref_name).unwrap(); 639 let parent_oid = repo.refname_to_id(&ref_name).unwrap();
612 let parent = repo.find_commit(parent_oid).unwrap(); 640 let parent = repo.find_commit(parent_oid).unwrap();
@@ -620,7 +648,15 @@ fn add_commit_on_branch(repo: &git2::Repository, branch: &str, filename: &str, c
620 let tree_oid = tb.write().unwrap(); 648 let tree_oid = tb.write().unwrap();
621 let tree = repo.find_tree(tree_oid).unwrap(); 649 let tree = repo.find_tree(tree_oid).unwrap();
622 650
623 repo.commit(Some(&ref_name), &sig, &sig, &format!("add {}", filename), &tree, &[&parent]).unwrap() 651 repo.commit(
652 Some(&ref_name),
653 &sig,
654 &sig,
655 &format!("add {}", filename),
656 &tree,
657 &[&parent],
658 )
659 .unwrap()
624 } 660 }
625 661
626 /// Create a branch-based patch using DAG primitives. 662 /// Create a branch-based patch using DAG primitives.
@@ -650,7 +686,8 @@ fn create_branch_patch(
650 let oid = dag::create_root_event(repo, &event, &sk).unwrap(); 686 let oid = dag::create_root_event(repo, &event, &sk).unwrap();
651 let id = oid.to_string(); 687 let id = oid.to_string();
652 let patch_ref = format!("refs/collab/patches/{}", id); 688 let patch_ref = format!("refs/collab/patches/{}", id);
653 repo.reference(&patch_ref, oid, false, "test branch patch").unwrap(); 689 repo.reference(&patch_ref, oid, false, "test branch patch")
690 .unwrap();
654 (patch_ref, id) 691 (patch_ref, id)
655 } 692 }
656 693
@@ -667,9 +704,15 @@ fn test_resolve_head_branch_based() {
667 make_initial_commit(&repo, "main"); 704 make_initial_commit(&repo, "main");
668 let feature_tip = add_commit_on_branch(&repo, "main", "feature.rs", b"fn feature() {}"); 705 let feature_tip = add_commit_on_branch(&repo, "main", "feature.rs", b"fn feature() {}");
669 // Create the feature branch at the current tip 706 // Create the feature branch at the current tip
670 repo.branch("feature/test", &repo.find_commit(feature_tip).unwrap(), false).unwrap(); 707 repo.branch(
671 708 "feature/test",
672 let (patch_ref, id) = create_branch_patch(&repo, &alice(), "Test patch", "feature/test", "main"); 709 &repo.find_commit(feature_tip).unwrap(),
710 false,
711 )
712 .unwrap();
713
714 let (patch_ref, id) =
715 create_branch_patch(&repo, &alice(), "Test patch", "feature/test", "main");
673 let state = PatchState::from_ref(&repo, &patch_ref, &id).unwrap(); 716 let state = PatchState::from_ref(&repo, &patch_ref, &id).unwrap();
674 717
675 // resolve_head should return the branch tip 718 // resolve_head should return the branch tip
@@ -689,17 +732,24 @@ fn test_resolve_head_deleted_branch_error() {
689 732
690 make_initial_commit(&repo, "main"); 733 make_initial_commit(&repo, "main");
691 let tip = add_commit_on_branch(&repo, "main", "f.rs", b"x"); 734 let tip = add_commit_on_branch(&repo, "main", "f.rs", b"x");
692 repo.branch("ephemeral", &repo.find_commit(tip).unwrap(), false).unwrap(); 735 repo.branch("ephemeral", &repo.find_commit(tip).unwrap(), false)
736 .unwrap();
693 737
694 let (patch_ref, id) = create_branch_patch(&repo, &alice(), "Ephemeral patch", "ephemeral", "main"); 738 let (patch_ref, id) =
739 create_branch_patch(&repo, &alice(), "Ephemeral patch", "ephemeral", "main");
695 740
696 // Delete the branch 741 // Delete the branch
697 let mut branch = repo.find_branch("ephemeral", git2::BranchType::Local).unwrap(); 742 let mut branch = repo
743 .find_branch("ephemeral", git2::BranchType::Local)
744 .unwrap();
698 branch.delete().unwrap(); 745 branch.delete().unwrap();
699 746
700 let state = PatchState::from_ref(&repo, &patch_ref, &id).unwrap(); 747 let state = PatchState::from_ref(&repo, &patch_ref, &id).unwrap();
701 let result = state.resolve_head(&repo); 748 let result = state.resolve_head(&repo);
702 assert!(result.is_err(), "resolve_head should error when branch is deleted"); 749 assert!(
750 result.is_err(),
751 "resolve_head should error when branch is deleted"
752 );
703 } 753 }
704 754
705 #[test] 755 #[test]
@@ -709,7 +759,8 @@ fn test_staleness_up_to_date() {
709 759
710 make_initial_commit(&repo, "main"); 760 make_initial_commit(&repo, "main");
711 let tip = add_commit_on_branch(&repo, "main", "f.rs", b"x"); 761 let tip = add_commit_on_branch(&repo, "main", "f.rs", b"x");
712 repo.branch("feat", &repo.find_commit(tip).unwrap(), false).unwrap(); 762 repo.branch("feat", &repo.find_commit(tip).unwrap(), false)
763 .unwrap();
713 // Add one commit on the feature branch 764 // Add one commit on the feature branch
714 add_commit_on_branch(&repo, "feat", "feat.rs", b"feature code"); 765 add_commit_on_branch(&repo, "feat", "feat.rs", b"feature code");
715 766
@@ -728,7 +779,8 @@ fn test_staleness_outdated() {
728 779
729 make_initial_commit(&repo, "main"); 780 make_initial_commit(&repo, "main");
730 let tip = add_commit_on_branch(&repo, "main", "f.rs", b"x"); 781 let tip = add_commit_on_branch(&repo, "main", "f.rs", b"x");
731 repo.branch("feat", &repo.find_commit(tip).unwrap(), false).unwrap(); 782 repo.branch("feat", &repo.find_commit(tip).unwrap(), false)
783 .unwrap();
732 // Add commit on feature branch 784 // Add commit on feature branch
733 add_commit_on_branch(&repo, "feat", "feat.rs", b"feature code"); 785 add_commit_on_branch(&repo, "feat", "feat.rs", b"feature code");
734 // Advance main by 2 commits 786 // Advance main by 2 commits
@@ -756,7 +808,8 @@ fn test_create_patch_from_branch_populates_branch_field() {
756 let repo = init_repo(tmp.path(), &alice()); 808 let repo = init_repo(tmp.path(), &alice());
757 make_initial_commit(&repo, "main"); 809 make_initial_commit(&repo, "main");
758 let tip = add_commit_on_branch(&repo, "main", "f.rs", b"x"); 810 let tip = add_commit_on_branch(&repo, "main", "f.rs", b"x");
759 repo.branch("feature/foo", &repo.find_commit(tip).unwrap(), false).unwrap(); 811 repo.branch("feature/foo", &repo.find_commit(tip).unwrap(), false)
812 .unwrap();
760 add_commit_on_branch(&repo, "feature/foo", "feat.rs", b"feat"); 813 add_commit_on_branch(&repo, "feature/foo", "feat.rs", b"feat");
761 814
762 let id = patch::create(&repo, "My patch", "desc", "main", "feature/foo", None).unwrap(); 815 let id = patch::create(&repo, "My patch", "desc", "main", "feature/foo", None).unwrap();
@@ -776,7 +829,8 @@ fn test_create_duplicate_patch_for_same_branch_returns_error() {
776 let repo = init_repo(tmp.path(), &alice()); 829 let repo = init_repo(tmp.path(), &alice());
777 make_initial_commit(&repo, "main"); 830 make_initial_commit(&repo, "main");
778 let tip = add_commit_on_branch(&repo, "main", "f.rs", b"x"); 831 let tip = add_commit_on_branch(&repo, "main", "f.rs", b"x");
779 repo.branch("feature/dup", &repo.find_commit(tip).unwrap(), false).unwrap(); 832 repo.branch("feature/dup", &repo.find_commit(tip).unwrap(), false)
833 .unwrap();
780 834
781 // First creation should succeed 835 // First creation should succeed
782 patch::create(&repo, "First", "", "main", "feature/dup", None).unwrap(); 836 patch::create(&repo, "First", "", "main", "feature/dup", None).unwrap();
@@ -785,7 +839,10 @@ fn test_create_duplicate_patch_for_same_branch_returns_error() {
785 let result = patch::create(&repo, "Second", "", "main", "feature/dup", None); 839 let result = patch::create(&repo, "Second", "", "main", "feature/dup", None);
786 assert!(result.is_err(), "duplicate branch patch should fail"); 840 assert!(result.is_err(), "duplicate branch patch should fail");
787 let err_msg = result.unwrap_err().to_string(); 841 let err_msg = result.unwrap_err().to_string();
788 assert!(err_msg.contains("feature/dup"), "error should mention the branch name"); 842 assert!(
843 err_msg.contains("feature/dup"),
844 "error should mention the branch name"
845 );
789 } 846 }
790 847
791 #[test] 848 #[test]
@@ -796,9 +853,15 @@ fn test_create_patch_from_base_branch_returns_error() {
796 make_initial_commit(&repo, "main"); 853 make_initial_commit(&repo, "main");
797 854
798 let result = patch::create(&repo, "Bad patch", "", "main", "main", None); 855 let result = patch::create(&repo, "Bad patch", "", "main", "main", None);
799 assert!(result.is_err(), "creating patch from base branch should fail"); 856 assert!(
857 result.is_err(),
858 "creating patch from base branch should fail"
859 );
800 let err_msg = result.unwrap_err().to_string(); 860 let err_msg = result.unwrap_err().to_string();
801 assert!(err_msg.contains("base branch"), "error should mention base branch"); 861 assert!(
862 err_msg.contains("base branch"),
863 "error should mention base branch"
864 );
802 } 865 }
803 866
804 // --------------------------------------------------------------------------- 867 // ---------------------------------------------------------------------------
@@ -812,7 +875,8 @@ fn test_patch_show_up_to_date_staleness() {
812 let repo = init_repo(tmp.path(), &alice()); 875 let repo = init_repo(tmp.path(), &alice());
813 make_initial_commit(&repo, "main"); 876 make_initial_commit(&repo, "main");
814 let tip = add_commit_on_branch(&repo, "main", "f.rs", b"x"); 877 let tip = add_commit_on_branch(&repo, "main", "f.rs", b"x");
815 repo.branch("feat", &repo.find_commit(tip).unwrap(), false).unwrap(); 878 repo.branch("feat", &repo.find_commit(tip).unwrap(), false)
879 .unwrap();
816 add_commit_on_branch(&repo, "feat", "feat.rs", b"code"); 880 add_commit_on_branch(&repo, "feat", "feat.rs", b"code");
817 881
818 let (patch_ref, id) = create_branch_patch(&repo, &alice(), "Up to date", "feat", "main"); 882 let (patch_ref, id) = create_branch_patch(&repo, &alice(), "Up to date", "feat", "main");
@@ -828,7 +892,8 @@ fn test_patch_show_outdated_staleness() {
828 let repo = init_repo(tmp.path(), &alice()); 892 let repo = init_repo(tmp.path(), &alice());
829 make_initial_commit(&repo, "main"); 893 make_initial_commit(&repo, "main");
830 let tip = add_commit_on_branch(&repo, "main", "f.rs", b"x"); 894 let tip = add_commit_on_branch(&repo, "main", "f.rs", b"x");
831 repo.branch("feat", &repo.find_commit(tip).unwrap(), false).unwrap(); 895 repo.branch("feat", &repo.find_commit(tip).unwrap(), false)
896 .unwrap();
832 add_commit_on_branch(&repo, "feat", "feat.rs", b"code"); 897 add_commit_on_branch(&repo, "feat", "feat.rs", b"code");
833 // Advance main 898 // Advance main
834 add_commit_on_branch(&repo, "main", "m1.rs", b"m1"); 899 add_commit_on_branch(&repo, "main", "m1.rs", b"m1");
@@ -944,7 +1009,8 @@ fn test_auto_detect_merged_patch_via_git_merge() {
944 make_initial_commit(&repo, "main"); 1009 make_initial_commit(&repo, "main");
945 1010
946 let tip = add_commit_on_branch(&repo, "main", "base.rs", b"base"); 1011 let tip = add_commit_on_branch(&repo, "main", "base.rs", b"base");
947 repo.branch("feat", &repo.find_commit(tip).unwrap(), false).unwrap(); 1012 repo.branch("feat", &repo.find_commit(tip).unwrap(), false)
1013 .unwrap();
948 add_commit_on_branch(&repo, "feat", "feat.rs", b"feature code"); 1014 add_commit_on_branch(&repo, "feat", "feat.rs", b"feature code");
949 1015
950 // Create the patch 1016 // Create the patch
@@ -957,11 +1023,16 @@ fn test_auto_detect_merged_patch_via_git_merge() {
957 1023
958 // Manually fast-forward main to feat (simulating `git merge feat`) 1024 // Manually fast-forward main to feat (simulating `git merge feat`)
959 let feat_tip = repo.refname_to_id("refs/heads/feat").unwrap(); 1025 let feat_tip = repo.refname_to_id("refs/heads/feat").unwrap();
960 repo.reference("refs/heads/main", feat_tip, true, "manual merge").unwrap(); 1026 repo.reference("refs/heads/main", feat_tip, true, "manual merge")
1027 .unwrap();
961 1028
962 // Now PatchState should auto-detect that it's merged 1029 // Now PatchState should auto-detect that it's merged
963 let state = PatchState::from_ref_uncached(&repo, &ref_name, &id).unwrap(); 1030 let state = PatchState::from_ref_uncached(&repo, &ref_name, &id).unwrap();
964 assert_eq!(state.status, PatchStatus::Merged, "should auto-detect merge"); 1031 assert_eq!(
1032 state.status,
1033 PatchStatus::Merged,
1034 "should auto-detect merge"
1035 );
965 } 1036 }
966 1037
967 #[test] 1038 #[test]
@@ -973,13 +1044,17 @@ fn test_auto_detect_merged_patch_deleted_branch() {
973 make_initial_commit(&repo, "main"); 1044 make_initial_commit(&repo, "main");
974 1045
975 let tip = add_commit_on_branch(&repo, "main", "base.rs", b"base"); 1046 let tip = add_commit_on_branch(&repo, "main", "base.rs", b"base");
976 repo.branch("feat", &repo.find_commit(tip).unwrap(), false).unwrap(); 1047 repo.branch("feat", &repo.find_commit(tip).unwrap(), false)
1048 .unwrap();
977 add_commit_on_branch(&repo, "feat", "feat.rs", b"feature code"); 1049 add_commit_on_branch(&repo, "feat", "feat.rs", b"feature code");
978 1050
979 let id = patch::create(&repo, "Deleted branch test", "", "main", "feat", None).unwrap(); 1051 let id = patch::create(&repo, "Deleted branch test", "", "main", "feat", None).unwrap();
980 1052
981 // Delete the feature branch (simulating cleanup after merge) 1053 // Delete the feature branch (simulating cleanup after merge)
982 repo.find_reference("refs/heads/feat").unwrap().delete().unwrap(); 1054 repo.find_reference("refs/heads/feat")
1055 .unwrap()
1056 .delete()
1057 .unwrap();
983 1058
984 // Should not crash, patch stays Open (can't verify merge without the branch) 1059 // Should not crash, patch stays Open (can't verify merge without the branch)
985 let ref_name = format!("refs/collab/patches/{}", id); 1060 let ref_name = format!("refs/collab/patches/{}", id);
@@ -997,7 +1072,8 @@ fn test_cache_does_not_defeat_auto_detect_merge() {
997 make_initial_commit(&repo, "main"); 1072 make_initial_commit(&repo, "main");
998 1073
999 let tip = add_commit_on_branch(&repo, "main", "base.rs", b"base"); 1074 let tip = add_commit_on_branch(&repo, "main", "base.rs", b"base");
1000 repo.branch("feat", &repo.find_commit(tip).unwrap(), false).unwrap(); 1075 repo.branch("feat", &repo.find_commit(tip).unwrap(), false)
1076 .unwrap();
1001 add_commit_on_branch(&repo, "feat", "feat.rs", b"feature code"); 1077 add_commit_on_branch(&repo, "feat", "feat.rs", b"feature code");
1002 1078
1003 // Create the patch via the high-level API (records base_commit) 1079 // Create the patch via the high-level API (records base_commit)
@@ -1010,11 +1086,16 @@ fn test_cache_does_not_defeat_auto_detect_merge() {
1010 1086
1011 // Manually fast-forward main to feat (simulating `git merge feat`) 1087 // Manually fast-forward main to feat (simulating `git merge feat`)
1012 let feat_tip = repo.refname_to_id("refs/heads/feat").unwrap(); 1088 let feat_tip = repo.refname_to_id("refs/heads/feat").unwrap();
1013 repo.reference("refs/heads/main", feat_tip, true, "manual merge").unwrap(); 1089 repo.reference("refs/heads/main", feat_tip, true, "manual merge")
1090 .unwrap();
1014 1091
1015 // Second call: cache hit (DAG tip unchanged), but should still detect merge 1092 // Second call: cache hit (DAG tip unchanged), but should still detect merge
1016 let state2 = PatchState::from_ref(&repo, &ref_name, &id).unwrap(); 1093 let state2 = PatchState::from_ref(&repo, &ref_name, &id).unwrap();
1017 assert_eq!(state2.status, PatchStatus::Merged, "cached from_ref should detect merge"); 1094 assert_eq!(
1095 state2.status,
1096 PatchStatus::Merged,
1097 "cached from_ref should detect merge"
1098 );
1018 } 1099 }
1019 1100
1020 // --------------------------------------------------------------------------- 1101 // ---------------------------------------------------------------------------
@@ -1028,7 +1109,8 @@ fn test_branch_push_auto_reflects_in_patch() {
1028 let repo = init_repo(tmp.path(), &alice()); 1109 let repo = init_repo(tmp.path(), &alice());
1029 make_initial_commit(&repo, "main"); 1110 make_initial_commit(&repo, "main");
1030 let tip = add_commit_on_branch(&repo, "main", "f.rs", b"x"); 1111 let tip = add_commit_on_branch(&repo, "main", "f.rs", b"x");
1031 repo.branch("feat", &repo.find_commit(tip).unwrap(), false).unwrap(); 1112 repo.branch("feat", &repo.find_commit(tip).unwrap(), false)
1113 .unwrap();
1032 add_commit_on_branch(&repo, "feat", "v1.rs", b"version 1"); 1114 add_commit_on_branch(&repo, "feat", "v1.rs", b"version 1");
1033 1115
1034 let (patch_ref, id) = create_branch_patch(&repo, &alice(), "Auto revise", "feat", "main"); 1116 let (patch_ref, id) = create_branch_patch(&repo, &alice(), "Auto revise", "feat", "main");
@@ -1154,7 +1236,16 @@ fn capture_issue_list(
1154 offset: Option<usize>, 1236 offset: Option<usize>,
1155 ) -> String { 1237 ) -> String {
1156 let mut buf = Vec::new(); 1238 let mut buf = Vec::new();
1157 issue::list_to_writer(repo, show_closed, false, limit, offset, git_collab::cli::SortMode::Recent, &mut buf).unwrap(); 1239 issue::list_to_writer(
1240 repo,
1241 show_closed,
1242 false,
1243 limit,
1244 offset,
1245 git_collab::cli::SortMode::Recent,
1246 &mut buf,
1247 )
1248 .unwrap();
1158 String::from_utf8(buf).unwrap() 1249 String::from_utf8(buf).unwrap()
1159 } 1250 }
1160 1251
@@ -1166,7 +1257,16 @@ fn capture_patch_list(
1166 offset: Option<usize>, 1257 offset: Option<usize>,
1167 ) -> String { 1258 ) -> String {
1168 let mut buf = Vec::new(); 1259 let mut buf = Vec::new();
1169 patch::list_to_writer(repo, show_closed, false, limit, offset, git_collab::cli::SortMode::Recent, &mut buf).unwrap(); 1260 patch::list_to_writer(
1261 repo,
1262 show_closed,
1263 false,
1264 limit,
1265 offset,
1266 git_collab::cli::SortMode::Recent,
1267 &mut buf,
1268 )
1269 .unwrap();
1170 String::from_utf8(buf).unwrap() 1270 String::from_utf8(buf).unwrap()
1171 } 1271 }
1172 1272
@@ -1379,7 +1479,9 @@ fn test_issue_list_json_output() {
1379 open_issue(&repo, &alice(), "Issue one"); 1479 open_issue(&repo, &alice(), "Issue one");
1380 open_issue(&repo, &bob(), "Issue two"); 1480 open_issue(&repo, &bob(), "Issue two");
1381 1481
1382 let json_str = git_collab::issue::list_json(&repo, false, false, git_collab::cli::SortMode::Recent).unwrap(); 1482 let json_str =
1483 git_collab::issue::list_json(&repo, false, false, git_collab::cli::SortMode::Recent)
1484 .unwrap();
1383 let value: serde_json::Value = serde_json::from_str(&json_str).unwrap(); 1485 let value: serde_json::Value = serde_json::from_str(&json_str).unwrap();
1384 let arr = value.as_array().unwrap(); 1486 let arr = value.as_array().unwrap();
1385 assert_eq!(arr.len(), 2); 1487 assert_eq!(arr.len(), 2);
@@ -1399,12 +1501,16 @@ fn test_issue_list_json_filters_closed() {
1399 close_issue(&repo, &ref2, &alice()); 1501 close_issue(&repo, &ref2, &alice());
1400 1502
1401 // Without --all, only open issues 1503 // Without --all, only open issues
1402 let json_str = git_collab::issue::list_json(&repo, false, false, git_collab::cli::SortMode::Recent).unwrap(); 1504 let json_str =
1505 git_collab::issue::list_json(&repo, false, false, git_collab::cli::SortMode::Recent)
1506 .unwrap();
1403 let value: serde_json::Value = serde_json::from_str(&json_str).unwrap(); 1507 let value: serde_json::Value = serde_json::from_str(&json_str).unwrap();
1404 assert_eq!(value.as_array().unwrap().len(), 1); 1508 assert_eq!(value.as_array().unwrap().len(), 1);
1405 1509
1406 // With --all, both 1510 // With --all, both
1407 let json_str = git_collab::issue::list_json(&repo, true, false, git_collab::cli::SortMode::Recent).unwrap(); 1511 let json_str =
1512 git_collab::issue::list_json(&repo, true, false, git_collab::cli::SortMode::Recent)
1513 .unwrap();
1408 let value: serde_json::Value = serde_json::from_str(&json_str).unwrap(); 1514 let value: serde_json::Value = serde_json::from_str(&json_str).unwrap();
1409 assert_eq!(value.as_array().unwrap().len(), 2); 1515 assert_eq!(value.as_array().unwrap().len(), 2);
1410 } 1516 }
@@ -1431,7 +1537,9 @@ fn test_patch_list_json_output() {
1431 create_patch(&repo, &alice(), "Patch one"); 1537 create_patch(&repo, &alice(), "Patch one");
1432 create_patch(&repo, &bob(), "Patch two"); 1538 create_patch(&repo, &bob(), "Patch two");
1433 1539
1434 let json_str = git_collab::patch::list_json(&repo, false, false, git_collab::cli::SortMode::Recent).unwrap(); 1540 let json_str =
1541 git_collab::patch::list_json(&repo, false, false, git_collab::cli::SortMode::Recent)
1542 .unwrap();
1435 let value: serde_json::Value = serde_json::from_str(&json_str).unwrap(); 1543 let value: serde_json::Value = serde_json::from_str(&json_str).unwrap();
1436 let arr = value.as_array().unwrap(); 1544 let arr = value.as_array().unwrap();
1437 assert_eq!(arr.len(), 2); 1545 assert_eq!(arr.len(), 2);
@@ -1543,4 +1651,3 @@ fn test_reconcile_outcome_local_ahead() {
1543 assert_eq!(outcome, dag::ReconcileOutcome::LocalAhead); 1651 assert_eq!(outcome, dag::ReconcileOutcome::LocalAhead);
1544 assert_eq!(oid, local_tip); 1652 assert_eq!(oid, local_tip);
1545 } 1653 }
1546
tests/common/mod.rs
Old New
@@ -187,7 +187,8 @@ pub fn init_repo(dir: &Path, author: &Author) -> Repository {
187 let sig = git2::Signature::now(&author.name, &author.email).unwrap(); 187 let sig = git2::Signature::now(&author.name, &author.email).unwrap();
188 let tree_oid = repo.treebuilder(None).unwrap().write().unwrap(); 188 let tree_oid = repo.treebuilder(None).unwrap().write().unwrap();
189 let tree = repo.find_tree(tree_oid).unwrap(); 189 let tree = repo.find_tree(tree_oid).unwrap();
190 repo.commit(Some("refs/heads/main"), &sig, &sig, "initial", &tree, &[]).unwrap(); 190 repo.commit(Some("refs/heads/main"), &sig, &sig, "initial", &tree, &[])
191 .unwrap();
191 } 192 }
192 repo 193 repo
193 } 194 }
@@ -315,7 +316,11 @@ impl TestRepo {
315 git_with_env(dir.path(), &["init", "-b", "main"], &env); 316 git_with_env(dir.path(), &["init", "-b", "main"], &env);
316 git_with_env(dir.path(), &["config", "user.name", name], &env); 317 git_with_env(dir.path(), &["config", "user.name", name], &env);
317 git_with_env(dir.path(), &["config", "user.email", email], &env); 318 git_with_env(dir.path(), &["config", "user.email", email], &env);
318 git_with_env(dir.path(), &["commit", "--allow-empty", "-m", "initial"], &env); 319 git_with_env(
320 dir.path(),
321 &["commit", "--allow-empty", "-m", "initial"],
322 &env,
323 );
319 env.ensure_signing_key(); 324 env.ensure_signing_key();
320 325
321 TestRepo { dir, env } 326 TestRepo { dir, env }
@@ -400,11 +405,15 @@ impl TestRepo {
400 let deadline = Instant::now() + Duration::from_secs(5); 405 let deadline = Instant::now() + Duration::from_secs(5);
401 loop { 406 loop {
402 if let Some(_status) = child.try_wait().expect("failed to poll dashboard process") { 407 if let Some(_status) = child.try_wait().expect("failed to poll dashboard process") {
403 return child.wait_with_output().expect("failed to collect dashboard output"); 408 return child
409 .wait_with_output()
410 .expect("failed to collect dashboard output");
404 } 411 }
405 if Instant::now() >= deadline { 412 if Instant::now() >= deadline {
406 let _ = child.kill(); 413 let _ = child.kill();
407 return child.wait_with_output().expect("failed to collect timed out dashboard output"); 414 return child
415 .wait_with_output()
416 .expect("failed to collect timed out dashboard output");
408 } 417 }
409 thread::sleep(Duration::from_millis(20)); 418 thread::sleep(Duration::from_millis(20));
410 } 419 }
@@ -422,7 +431,9 @@ impl TestRepo {
422 /// Create a patch from a new branch. Returns the 8-char short ID. 431 /// Create a patch from a new branch. Returns the 8-char short ID.
423 pub fn patch_create(&self, title: &str) -> String { 432 pub fn patch_create(&self, title: &str) -> String {
424 // Create a unique branch for this patch 433 // Create a unique branch for this patch
425 let sanitized = title.replace(|c: char| !c.is_alphanumeric(), "-").to_lowercase(); 434 let sanitized = title
435 .replace(|c: char| !c.is_alphanumeric(), "-")
436 .to_lowercase();
426 let branch_name = format!("test/{}", sanitized); 437 let branch_name = format!("test/{}", sanitized);
427 self.git(&["checkout", "-b", &branch_name]); 438 self.git(&["checkout", "-b", &branch_name]);
428 self.commit_file( 439 self.commit_file(
@@ -490,7 +501,10 @@ impl ServerHarness {
490 std::fs::create_dir_all(&repos_dir).unwrap(); 501 std::fs::create_dir_all(&repos_dir).unwrap();
491 502
492 let bare_repo_dir = repos_dir.join(format!("{repo_name}.git")); 503 let bare_repo_dir = repos_dir.join(format!("{repo_name}.git"));
493 git_cmd(root.path(), &["init", "--bare", bare_repo_dir.to_str().unwrap()]); 504 git_cmd(
505 root.path(),
506 &["init", "--bare", bare_repo_dir.to_str().unwrap()],
507 );
494 508
495 let work_repo = TestRepo::new("Alice", "alice@example.com"); 509 let work_repo = TestRepo::new("Alice", "alice@example.com");
496 work_repo.git(&["remote", "add", "origin", bare_repo_dir.to_str().unwrap()]); 510 work_repo.git(&["remote", "add", "origin", bare_repo_dir.to_str().unwrap()]);
@@ -560,8 +574,12 @@ impl ServerHarness {
560 } 574 }
561 575
562 pub fn get(&self, path: &str) -> HttpResponse { 576 pub fn get(&self, path: &str) -> HttpResponse {
563 let mut stream = TcpStream::connect(self.http_addr) 577 let mut stream = TcpStream::connect(self.http_addr).unwrap_or_else(|e| {
564 .unwrap_or_else(|e| panic!("failed to connect to http server on {}: {}", self.http_addr, e)); 578 panic!(
579 "failed to connect to http server on {}: {}",
580 self.http_addr, e
581 )
582 });
565 stream 583 stream
566 .write_all( 584 .write_all(
567 format!( 585 format!(
@@ -598,7 +616,10 @@ impl ServerHarness {
598 } 616 }
599 617
600 if Instant::now() >= deadline { 618 if Instant::now() >= deadline {
601 panic!("timed out waiting for git-collab-server on {}", self.http_addr); 619 panic!(
620 "timed out waiting for git-collab-server on {}",
621 self.http_addr
622 );
602 } 623 }
603 624
604 thread::sleep(Duration::from_millis(50)); 625 thread::sleep(Duration::from_millis(50));
tests/completions_test.rs
Old New
@@ -7,9 +7,17 @@ fn completions_bash_produces_output() {
7 .args(["completions", "bash"]) 7 .args(["completions", "bash"])
8 .output() 8 .output()
9 .expect("failed to run binary"); 9 .expect("failed to run binary");
10 assert!(output.status.success(), "stderr: {}", String::from_utf8_lossy(&output.stderr)); 10 assert!(
11 output.status.success(),
12 "stderr: {}",
13 String::from_utf8_lossy(&output.stderr)
14 );
11 let stdout = String::from_utf8_lossy(&output.stdout); 15 let stdout = String::from_utf8_lossy(&output.stdout);
12 assert!(stdout.contains("complete"), "bash completions should contain 'complete': {}", stdout); 16 assert!(
17 stdout.contains("complete"),
18 "bash completions should contain 'complete': {}",
19 stdout
20 );
13 } 21 }
14 22
15 /// Verify `completions zsh` parses successfully and produces output 23 /// Verify `completions zsh` parses successfully and produces output
@@ -19,7 +27,11 @@ fn completions_zsh_produces_output() {
19 .args(["completions", "zsh"]) 27 .args(["completions", "zsh"])
20 .output() 28 .output()
21 .expect("failed to run binary"); 29 .expect("failed to run binary");
22 assert!(output.status.success(), "stderr: {}", String::from_utf8_lossy(&output.stderr)); 30 assert!(
31 output.status.success(),
32 "stderr: {}",
33 String::from_utf8_lossy(&output.stderr)
34 );
23 let stdout = String::from_utf8_lossy(&output.stdout); 35 let stdout = String::from_utf8_lossy(&output.stdout);
24 assert!(!stdout.is_empty(), "zsh completions should not be empty"); 36 assert!(!stdout.is_empty(), "zsh completions should not be empty");
25 } 37 }
@@ -31,9 +43,17 @@ fn completions_fish_produces_output() {
31 .args(["completions", "fish"]) 43 .args(["completions", "fish"])
32 .output() 44 .output()
33 .expect("failed to run binary"); 45 .expect("failed to run binary");
34 assert!(output.status.success(), "stderr: {}", String::from_utf8_lossy(&output.stderr)); 46 assert!(
47 output.status.success(),
48 "stderr: {}",
49 String::from_utf8_lossy(&output.stderr)
50 );
35 let stdout = String::from_utf8_lossy(&output.stdout); 51 let stdout = String::from_utf8_lossy(&output.stdout);
36 assert!(stdout.contains("complete"), "fish completions should contain 'complete': {}", stdout); 52 assert!(
53 stdout.contains("complete"),
54 "fish completions should contain 'complete': {}",
55 stdout
56 );
37 } 57 }
38 58
39 /// Verify completions work without a git repo (run from /tmp) 59 /// Verify completions work without a git repo (run from /tmp)
@@ -44,5 +64,9 @@ fn completions_work_without_git_repo() {
44 .current_dir(std::env::temp_dir()) 64 .current_dir(std::env::temp_dir())
45 .output() 65 .output()
46 .expect("failed to run binary"); 66 .expect("failed to run binary");
47 assert!(output.status.success(), "completions should work outside a git repo: {}", String::from_utf8_lossy(&output.stderr)); 67 assert!(
68 output.status.success(),
69 "completions should work outside a git repo: {}",
70 String::from_utf8_lossy(&output.stderr)
71 );
48 } 72 }
tests/crdt_test.rs
Old New
@@ -338,8 +338,7 @@ fn concurrent_patch_close_merge_higher_clock_wins() {
338 let local_ref = "refs/collab/patches/test-patch"; 338 let local_ref = "refs/collab/patches/test-patch";
339 let remote_ref = "refs/collab/sync/patches/test-patch"; 339 let remote_ref = "refs/collab/sync/patches/test-patch";
340 repo.reference(local_ref, root_oid, false, "test").unwrap(); 340 repo.reference(local_ref, root_oid, false, "test").unwrap();
341 repo.reference(remote_ref, root_oid, false, "test") 341 repo.reference(remote_ref, root_oid, false, "test").unwrap();
342 .unwrap();
343 342
344 // Local: close (clock=2) 343 // Local: close (clock=2)
345 let close = Event { 344 let close = Event {
tests/editor_test.rs
Old New
@@ -217,7 +217,11 @@ fn e2e_open_editor_at_with_multi_word_editor() {
217 let path = tmp.path().to_str().unwrap(); 217 let path = tmp.path().to_str().unwrap();
218 218
219 let result = open_editor_at(path, 42); 219 let result = open_editor_at(path, 42);
220 assert!(result.is_ok(), "Expected Ok with multi-word editor, got {:?}", result); 220 assert!(
221 result.is_ok(),
222 "Expected Ok with multi-word editor, got {:?}",
223 result
224 );
221 225
222 restore_env("VISUAL", old_visual); 226 restore_env("VISUAL", old_visual);
223 restore_env("EDITOR", old_editor); 227 restore_env("EDITOR", old_editor);
tests/identity_test.rs
Old New
@@ -67,7 +67,10 @@ fn test_identity_alias_add_multiple() {
67 repo.run_ok(&["identity", "alias", "alice@personal.org"]); 67 repo.run_ok(&["identity", "alias", "alice@personal.org"]);
68 let out = repo.run_ok(&["identity", "list"]); 68 let out = repo.run_ok(&["identity", "list"]);
69 assert!(out.contains("alice@work.com"), "should list first alias"); 69 assert!(out.contains("alice@work.com"), "should list first alias");
70 assert!(out.contains("alice@personal.org"), "should list second alias"); 70 assert!(
71 out.contains("alice@personal.org"),
72 "should list second alias"
73 );
71 } 74 }
72 75
73 #[test] 76 #[test]
@@ -102,7 +105,10 @@ fn test_identity_list_shows_primary_and_aliases() {
102 let repo = TestRepo::new("Alice", "alice@example.com"); 105 let repo = TestRepo::new("Alice", "alice@example.com");
103 repo.run_ok(&["identity", "alias", "alice@work.com"]); 106 repo.run_ok(&["identity", "alias", "alice@work.com"]);
104 let out = repo.run_ok(&["identity", "list"]); 107 let out = repo.run_ok(&["identity", "list"]);
105 assert!(out.contains("alice@example.com"), "should show primary email"); 108 assert!(
109 out.contains("alice@example.com"),
110 "should show primary email"
111 );
106 assert!(out.contains("alice@work.com"), "should show alias"); 112 assert!(out.contains("alice@work.com"), "should show alias");
107 } 113 }
108 114
tests/revision_test.rs
Old New
@@ -1,7 +1,7 @@
1 mod common; 1 mod common;
2 2
3 use common::TestRepo; 3 use common::TestRepo;
4 use common::{alice, bob, init_repo, test_signing_key, now}; 4 use common::{alice, bob, init_repo, now, test_signing_key};
5 5
6 use git_collab::dag; 6 use git_collab::dag;
7 use git_collab::event::{Action, Event}; 7 use git_collab::event::{Action, Event};
@@ -38,8 +38,19 @@ fn test_auto_detect_revision_on_comment() {
38 // Create feature branch and patch 38 // Create feature branch and patch
39 repo.git(&["checkout", "-b", "feat-auto"]); 39 repo.git(&["checkout", "-b", "feat-auto"]);
40 repo.commit_file("v1.txt", "v1", "initial commit"); 40 repo.commit_file("v1.txt", "v1", "initial commit");
41 let out = repo.run_ok(&["patch", "create", "-t", "Auto-detect test", "-B", "feat-auto"]); 41 let out = repo.run_ok(&[
42 let id = out.trim().strip_prefix("Created patch ").unwrap().to_string(); 42 "patch",
43 "create",
44 "-t",
45 "Auto-detect test",
46 "-B",
47 "feat-auto",
48 ]);
49 let id = out
50 .trim()
51 .strip_prefix("Created patch ")
52 .unwrap()
53 .to_string();
43 54
44 // Push a new commit to the branch 55 // Push a new commit to the branch
45 repo.git(&["checkout", "feat-auto"]); 56 repo.git(&["checkout", "feat-auto"]);
@@ -52,7 +63,11 @@ fn test_auto_detect_revision_on_comment() {
52 let out = repo.run_ok(&["patch", "show", &id, "--json"]); 63 let out = repo.run_ok(&["patch", "show", &id, "--json"]);
53 let json: serde_json::Value = serde_json::from_str(&out).unwrap(); 64 let json: serde_json::Value = serde_json::from_str(&out).unwrap();
54 let revisions = json["revisions"].as_array().unwrap(); 65 let revisions = json["revisions"].as_array().unwrap();
55 assert_eq!(revisions.len(), 2, "should have 2 revisions (create + auto-detect)"); 66 assert_eq!(
67 revisions.len(),
68 2,
69 "should have 2 revisions (create + auto-detect)"
70 );
56 assert_eq!(revisions[0]["number"], 1); 71 assert_eq!(revisions[0]["number"], 1);
57 assert_eq!(revisions[1]["number"], 2); 72 assert_eq!(revisions[1]["number"], 2);
58 } 73 }
@@ -63,8 +78,19 @@ fn test_no_revision_when_branch_unchanged() {
63 78
64 repo.git(&["checkout", "-b", "feat-no-change"]); 79 repo.git(&["checkout", "-b", "feat-no-change"]);
65 repo.commit_file("v1.txt", "v1", "initial commit"); 80 repo.commit_file("v1.txt", "v1", "initial commit");
66 let out = repo.run_ok(&["patch", "create", "-t", "No change test", "-B", "feat-no-change"]); 81 let out = repo.run_ok(&[
67 let id = out.trim().strip_prefix("Created patch ").unwrap().to_string(); 82 "patch",
83 "create",
84 "-t",
85 "No change test",
86 "-B",
87 "feat-no-change",
88 ]);
89 let id = out
90 .trim()
91 .strip_prefix("Created patch ")
92 .unwrap()
93 .to_string();
68 94
69 // Comment WITHOUT branch change — no new revision 95 // Comment WITHOUT branch change — no new revision
70 repo.run_ok(&["patch", "comment", &id, "-b", "Just a thought"]); 96 repo.run_ok(&["patch", "comment", &id, "-b", "Just a thought"]);
@@ -86,7 +112,11 @@ fn test_auto_detect_revision_on_review() {
86 repo.git(&["checkout", "-b", "feat-review"]); 112 repo.git(&["checkout", "-b", "feat-review"]);
87 repo.commit_file("v1.txt", "v1", "initial commit"); 113 repo.commit_file("v1.txt", "v1", "initial commit");
88 let out = repo.run_ok(&["patch", "create", "-t", "Review test", "-B", "feat-review"]); 114 let out = repo.run_ok(&["patch", "create", "-t", "Review test", "-B", "feat-review"]);
89 let id = out.trim().strip_prefix("Created patch ").unwrap().to_string(); 115 let id = out
116 .trim()
117 .strip_prefix("Created patch ")
118 .unwrap()
119 .to_string();
90 120
91 // Push a new commit 121 // Push a new commit
92 repo.git(&["checkout", "feat-review"]); 122 repo.git(&["checkout", "feat-review"]);
@@ -118,7 +148,11 @@ fn test_revise_creates_revision() {
118 repo.git(&["checkout", "-b", "feat-revise"]); 148 repo.git(&["checkout", "-b", "feat-revise"]);
119 repo.commit_file("v1.txt", "v1", "initial commit"); 149 repo.commit_file("v1.txt", "v1", "initial commit");
120 let out = repo.run_ok(&["patch", "create", "-t", "Revise test", "-B", "feat-revise"]); 150 let out = repo.run_ok(&["patch", "create", "-t", "Revise test", "-B", "feat-revise"]);
121 let id = out.trim().strip_prefix("Created patch ").unwrap().to_string(); 151 let id = out
152 .trim()
153 .strip_prefix("Created patch ")
154 .unwrap()
155 .to_string();
122 156
123 // Push new commit 157 // Push new commit
124 repo.git(&["checkout", "feat-revise"]); 158 repo.git(&["checkout", "feat-revise"]);
@@ -140,8 +174,19 @@ fn test_revise_rejects_when_unchanged() {
140 174
141 repo.git(&["checkout", "-b", "feat-revise-err"]); 175 repo.git(&["checkout", "-b", "feat-revise-err"]);
142 repo.commit_file("v1.txt", "v1", "initial commit"); 176 repo.commit_file("v1.txt", "v1", "initial commit");
143 let out = repo.run_ok(&["patch", "create", "-t", "Revise error test", "-B", "feat-revise-err"]); 177 let out = repo.run_ok(&[
144 let id = out.trim().strip_prefix("Created patch ").unwrap().to_string(); 178 "patch",
179 "create",
180 "-t",
181 "Revise error test",
182 "-B",
183 "feat-revise-err",
184 ]);
185 let id = out
186 .trim()
187 .strip_prefix("Created patch ")
188 .unwrap()
189 .to_string();
145 190
146 // No new commit — revise should fail 191 // No new commit — revise should fail
147 let err = repo.run_err(&["patch", "revise", &id]); 192 let err = repo.run_err(&["patch", "revise", &id]);
@@ -159,11 +204,23 @@ fn test_inline_comment_anchored_to_revision() {
159 repo.git(&["checkout", "-b", "feat-inline"]); 204 repo.git(&["checkout", "-b", "feat-inline"]);
160 repo.commit_file("src/lib.rs", "fn hello() {}", "initial"); 205 repo.commit_file("src/lib.rs", "fn hello() {}", "initial");
161 let out = repo.run_ok(&["patch", "create", "-t", "Inline test", "-B", "feat-inline"]); 206 let out = repo.run_ok(&["patch", "create", "-t", "Inline test", "-B", "feat-inline"]);
162 let id = out.trim().strip_prefix("Created patch ").unwrap().to_string(); 207 let id = out
208 .trim()
209 .strip_prefix("Created patch ")
210 .unwrap()
211 .to_string();
163 212
164 // Comment on revision 1 213 // Comment on revision 1
165 repo.run_ok(&[ 214 repo.run_ok(&[
166 "patch", "comment", &id, "-b", "nit: naming", "-f", "src/lib.rs", "-l", "1", 215 "patch",
216 "comment",
217 &id,
218 "-b",
219 "nit: naming",
220 "-f",
221 "src/lib.rs",
222 "-l",
223 "1",
167 ]); 224 ]);
168 225
169 let out = repo.run_ok(&["patch", "show", &id, "--json"]); 226 let out = repo.run_ok(&["patch", "show", &id, "--json"]);
@@ -179,8 +236,19 @@ fn test_inline_comment_explicit_revision() {
179 236
180 repo.git(&["checkout", "-b", "feat-inline-rev"]); 237 repo.git(&["checkout", "-b", "feat-inline-rev"]);
181 repo.commit_file("lib.rs", "fn a() {}", "v1"); 238 repo.commit_file("lib.rs", "fn a() {}", "v1");
182 let out = repo.run_ok(&["patch", "create", "-t", "Inline rev test", "-B", "feat-inline-rev"]); 239 let out = repo.run_ok(&[
183 let id = out.trim().strip_prefix("Created patch ").unwrap().to_string(); 240 "patch",
241 "create",
242 "-t",
243 "Inline rev test",
244 "-B",
245 "feat-inline-rev",
246 ]);
247 let id = out
248 .trim()
249 .strip_prefix("Created patch ")
250 .unwrap()
251 .to_string();
184 252
185 // Push v2 253 // Push v2
186 repo.git(&["checkout", "feat-inline-rev"]); 254 repo.git(&["checkout", "feat-inline-rev"]);
@@ -192,7 +260,17 @@ fn test_inline_comment_explicit_revision() {
192 260
193 // Now explicitly target revision 1 261 // Now explicitly target revision 1
194 repo.run_ok(&[ 262 repo.run_ok(&[
195 "patch", "comment", &id, "-b", "old nit", "-f", "lib.rs", "-l", "1", "--revision", "1", 263 "patch",
264 "comment",
265 &id,
266 "-b",
267 "old nit",
268 "-f",
269 "lib.rs",
270 "-l",
271 "1",
272 "--revision",
273 "1",
196 ]); 274 ]);
197 275
198 let out = repo.run_ok(&["patch", "show", &id, "--json"]); 276 let out = repo.run_ok(&["patch", "show", &id, "--json"]);
@@ -209,7 +287,11 @@ fn test_thread_comment_rejects_revision_flag() {
209 repo.git(&["checkout", "-b", "feat-thread"]); 287 repo.git(&["checkout", "-b", "feat-thread"]);
210 repo.commit_file("x.txt", "x", "init"); 288 repo.commit_file("x.txt", "x", "init");
211 let out = repo.run_ok(&["patch", "create", "-t", "Thread test", "-B", "feat-thread"]); 289 let out = repo.run_ok(&["patch", "create", "-t", "Thread test", "-B", "feat-thread"]);
212 let id = out.trim().strip_prefix("Created patch ").unwrap().to_string(); 290 let id = out
291 .trim()
292 .strip_prefix("Created patch ")
293 .unwrap()
294 .to_string();
213 295
214 let err = repo.run_err(&["patch", "comment", &id, "-b", "note", "--revision", "1"]); 296 let err = repo.run_err(&["patch", "comment", &id, "-b", "note", "--revision", "1"]);
215 assert!(err.contains("thread comments are not revision-scoped")); 297 assert!(err.contains("thread comments are not revision-scoped"));
@@ -225,12 +307,31 @@ fn test_show_revision_filter() {
225 307
226 repo.git(&["checkout", "-b", "feat-show-rev"]); 308 repo.git(&["checkout", "-b", "feat-show-rev"]);
227 repo.commit_file("a.txt", "a", "v1"); 309 repo.commit_file("a.txt", "a", "v1");
228 let out = repo.run_ok(&["patch", "create", "-t", "Show rev test", "-B", "feat-show-rev"]); 310 let out = repo.run_ok(&[
229 let id = out.trim().strip_prefix("Created patch ").unwrap().to_string(); 311 "patch",
312 "create",
313 "-t",
314 "Show rev test",
315 "-B",
316 "feat-show-rev",
317 ]);
318 let id = out
319 .trim()
320 .strip_prefix("Created patch ")
321 .unwrap()
322 .to_string();
230 323
231 // Comment on r1 324 // Comment on r1
232 repo.run_ok(&[ 325 repo.run_ok(&[
233 "patch", "comment", &id, "-b", "r1 comment", "-f", "a.txt", "-l", "1", 326 "patch",
327 "comment",
328 &id,
329 "-b",
330 "r1 comment",
331 "-f",
332 "a.txt",
333 "-l",
334 "1",
234 ]); 335 ]);
235 336
236 // Push v2 and comment on r2 337 // Push v2 and comment on r2
@@ -238,7 +339,15 @@ fn test_show_revision_filter() {
238 repo.commit_file("b.txt", "b", "v2"); 339 repo.commit_file("b.txt", "b", "v2");
239 repo.git(&["checkout", "main"]); 340 repo.git(&["checkout", "main"]);
240 repo.run_ok(&[ 341 repo.run_ok(&[
241 "patch", "comment", &id, "-b", "r2 comment", "-f", "b.txt", "-l", "1", 342 "patch",
343 "comment",
344 &id,
345 "-b",
346 "r2 comment",
347 "-f",
348 "b.txt",
349 "-l",
350 "1",
242 ]); 351 ]);
243 352
244 // Show --revision 1: should show r1 comment, not r2 353 // Show --revision 1: should show r1 comment, not r2
@@ -262,8 +371,19 @@ fn test_interdiff_between_revisions() {
262 371
263 repo.git(&["checkout", "-b", "feat-interdiff"]); 372 repo.git(&["checkout", "-b", "feat-interdiff"]);
264 repo.commit_file("a.txt", "hello", "v1"); 373 repo.commit_file("a.txt", "hello", "v1");
265 let out = repo.run_ok(&["patch", "create", "-t", "Interdiff test", "-B", "feat-interdiff"]); 374 let out = repo.run_ok(&[
266 let id = out.trim().strip_prefix("Created patch ").unwrap().to_string(); 375 "patch",
376 "create",
377 "-t",
378 "Interdiff test",
379 "-B",
380 "feat-interdiff",
381 ]);
382 let id = out
383 .trim()
384 .strip_prefix("Created patch ")
385 .unwrap()
386 .to_string();
267 387
268 // Push v2 388 // Push v2
269 repo.git(&["checkout", "feat-interdiff"]); 389 repo.git(&["checkout", "feat-interdiff"]);
@@ -284,8 +404,19 @@ fn test_interdiff_single_arg_means_n_to_latest() {
284 404
285 repo.git(&["checkout", "-b", "feat-between-single"]); 405 repo.git(&["checkout", "-b", "feat-between-single"]);
286 repo.commit_file("a.txt", "hello", "v1"); 406 repo.commit_file("a.txt", "hello", "v1");
287 let out = repo.run_ok(&["patch", "create", "-t", "Between single test", "-B", "feat-between-single"]); 407 let out = repo.run_ok(&[
288 let id = out.trim().strip_prefix("Created patch ").unwrap().to_string(); 408 "patch",
409 "create",
410 "-t",
411 "Between single test",
412 "-B",
413 "feat-between-single",
414 ]);
415 let id = out
416 .trim()
417 .strip_prefix("Created patch ")
418 .unwrap()
419 .to_string();
289 420
290 // Push v2 421 // Push v2
291 repo.git(&["checkout", "feat-between-single"]); 422 repo.git(&["checkout", "feat-between-single"]);
@@ -304,8 +435,19 @@ fn test_interdiff_nonexistent_revision_errors() {
304 435
305 repo.git(&["checkout", "-b", "feat-bad-rev"]); 436 repo.git(&["checkout", "-b", "feat-bad-rev"]);
306 repo.commit_file("a.txt", "a", "v1"); 437 repo.commit_file("a.txt", "a", "v1");
307 let out = repo.run_ok(&["patch", "create", "-t", "Bad rev test", "-B", "feat-bad-rev"]); 438 let out = repo.run_ok(&[
308 let id = out.trim().strip_prefix("Created patch ").unwrap().to_string(); 439 "patch",
440 "create",
441 "-t",
442 "Bad rev test",
443 "-B",
444 "feat-bad-rev",
445 ]);
446 let id = out
447 .trim()
448 .strip_prefix("Created patch ")
449 .unwrap()
450 .to_string();
309 451
310 let err = repo.run_err(&["patch", "diff", &id, "--between", "1", "2"]); 452 let err = repo.run_err(&["patch", "diff", &id, "--between", "1", "2"]);
311 assert!(err.contains("revision 2 not found")); 453 assert!(err.contains("revision 2 not found"));
@@ -318,7 +460,11 @@ fn test_diff_revision_flag_shows_historical_diff() {
318 repo.git(&["checkout", "-b", "feat-hist"]); 460 repo.git(&["checkout", "-b", "feat-hist"]);
319 repo.commit_file("a.txt", "hello", "v1"); 461 repo.commit_file("a.txt", "hello", "v1");
320 let out = repo.run_ok(&["patch", "create", "-t", "Hist diff test", "-B", "feat-hist"]); 462 let out = repo.run_ok(&["patch", "create", "-t", "Hist diff test", "-B", "feat-hist"]);
321 let id = out.trim().strip_prefix("Created patch ").unwrap().to_string(); 463 let id = out
464 .trim()
465 .strip_prefix("Created patch ")
466 .unwrap()
467 .to_string();
322 468
323 // Push v2 (adds b.txt) 469 // Push v2 (adds b.txt)
324 repo.git(&["checkout", "feat-hist"]); 470 repo.git(&["checkout", "feat-hist"]);
@@ -339,7 +485,11 @@ fn test_diff_revision_and_between_mutually_exclusive() {
339 repo.git(&["checkout", "-b", "feat-mutex"]); 485 repo.git(&["checkout", "-b", "feat-mutex"]);
340 repo.commit_file("a.txt", "a", "v1"); 486 repo.commit_file("a.txt", "a", "v1");
341 let out = repo.run_ok(&["patch", "create", "-t", "Mutex test", "-B", "feat-mutex"]); 487 let out = repo.run_ok(&["patch", "create", "-t", "Mutex test", "-B", "feat-mutex"]);
342 let id = out.trim().strip_prefix("Created patch ").unwrap().to_string(); 488 let id = out
489 .trim()
490 .strip_prefix("Created patch ")
491 .unwrap()
492 .to_string();
343 493
344 let err = repo.run_err(&["patch", "diff", &id, "--revision", "1", "--between", "1"]); 494 let err = repo.run_err(&["patch", "diff", &id, "--revision", "1", "--between", "1"]);
345 assert!(err.contains("mutually exclusive")); 495 assert!(err.contains("mutually exclusive"));
@@ -356,7 +506,11 @@ fn test_patch_log() {
356 repo.git(&["checkout", "-b", "feat-log"]); 506 repo.git(&["checkout", "-b", "feat-log"]);
357 repo.commit_file("a.txt", "hello", "v1"); 507 repo.commit_file("a.txt", "hello", "v1");
358 let out = repo.run_ok(&["patch", "create", "-t", "Log test", "-B", "feat-log"]); 508 let out = repo.run_ok(&["patch", "create", "-t", "Log test", "-B", "feat-log"]);
359 let id = out.trim().strip_prefix("Created patch ").unwrap().to_string(); 509 let id = out
510 .trim()
511 .strip_prefix("Created patch ")
512 .unwrap()
513 .to_string();
360 514
361 // Push v2 515 // Push v2
362 repo.git(&["checkout", "feat-log"]); 516 repo.git(&["checkout", "feat-log"]);
@@ -384,8 +538,19 @@ fn test_patch_log_json() {
384 538
385 repo.git(&["checkout", "-b", "feat-log-json"]); 539 repo.git(&["checkout", "-b", "feat-log-json"]);
386 repo.commit_file("a.txt", "a", "v1"); 540 repo.commit_file("a.txt", "a", "v1");
387 let out = repo.run_ok(&["patch", "create", "-t", "Log JSON test", "-B", "feat-log-json"]); 541 let out = repo.run_ok(&[
388 let id = out.trim().strip_prefix("Created patch ").unwrap().to_string(); 542 "patch",
543 "create",
544 "-t",
545 "Log JSON test",
546 "-B",
547 "feat-log-json",
548 ]);
549 let id = out
550 .trim()
551 .strip_prefix("Created patch ")
552 .unwrap()
553 .to_string();
389 554
390 let out = repo.run_ok(&["patch", "log", &id, "--json"]); 555 let out = repo.run_ok(&["patch", "log", &id, "--json"]);
391 let json: serde_json::Value = serde_json::from_str(&out).unwrap(); 556 let json: serde_json::Value = serde_json::from_str(&out).unwrap();
@@ -404,8 +569,19 @@ fn test_review_explicit_revision() {
404 569
405 repo.git(&["checkout", "-b", "feat-rev-explicit"]); 570 repo.git(&["checkout", "-b", "feat-rev-explicit"]);
406 repo.commit_file("a.txt", "a", "v1"); 571 repo.commit_file("a.txt", "a", "v1");
407 let out = repo.run_ok(&["patch", "create", "-t", "Rev review", "-B", "feat-rev-explicit"]); 572 let out = repo.run_ok(&[
408 let id = out.trim().strip_prefix("Created patch ").unwrap().to_string(); 573 "patch",
574 "create",
575 "-t",
576 "Rev review",
577 "-B",
578 "feat-rev-explicit",
579 ]);
580 let id = out
581 .trim()
582 .strip_prefix("Created patch ")
583 .unwrap()
584 .to_string();
409 585
410 // Push v2 586 // Push v2
411 repo.git(&["checkout", "feat-rev-explicit"]); 587 repo.git(&["checkout", "feat-rev-explicit"]);
@@ -415,7 +591,15 @@ fn test_review_explicit_revision() {
415 591
416 // Review targeting revision 1 592 // Review targeting revision 1
417 repo.run_ok(&[ 593 repo.run_ok(&[
418 "patch", "review", &id, "-v", "request-changes", "-b", "fix r1", "--revision", "1", 594 "patch",
595 "review",
596 &id,
597 "-v",
598 "request-changes",
599 "-b",
600 "fix r1",
601 "--revision",
602 "1",
419 ]); 603 ]);
420 604
421 let out = repo.run_ok(&["patch", "show", &id, "--json"]); 605 let out = repo.run_ok(&["patch", "show", &id, "--json"]);
@@ -431,8 +615,19 @@ fn test_review_shows_revision_context_in_show() {
431 615
432 repo.git(&["checkout", "-b", "feat-rev-show"]); 616 repo.git(&["checkout", "-b", "feat-rev-show"]);
433 repo.commit_file("a.txt", "a", "v1"); 617 repo.commit_file("a.txt", "a", "v1");
434 let out = repo.run_ok(&["patch", "create", "-t", "Review show", "-B", "feat-rev-show"]); 618 let out = repo.run_ok(&[
435 let id = out.trim().strip_prefix("Created patch ").unwrap().to_string(); 619 "patch",
620 "create",
621 "-t",
622 "Review show",
623 "-B",
624 "feat-rev-show",
625 ]);
626 let id = out
627 .trim()
628 .strip_prefix("Created patch ")
629 .unwrap()
630 .to_string();
436 631
437 repo.run_ok(&["patch", "review", &id, "-v", "approve", "-b", "LGTM"]); 632 repo.run_ok(&["patch", "review", &id, "-v", "approve", "-b", "LGTM"]);
438 633
@@ -452,7 +647,11 @@ fn test_revision_dedup_by_commit_oid() {
452 repo.git(&["checkout", "-b", "feat-dedup"]); 647 repo.git(&["checkout", "-b", "feat-dedup"]);
453 repo.commit_file("a.txt", "a", "v1"); 648 repo.commit_file("a.txt", "a", "v1");
454 let out = repo.run_ok(&["patch", "create", "-t", "Dedup test", "-B", "feat-dedup"]); 649 let out = repo.run_ok(&["patch", "create", "-t", "Dedup test", "-B", "feat-dedup"]);
455 let id = out.trim().strip_prefix("Created patch ").unwrap().to_string(); 650 let id = out
651 .trim()
652 .strip_prefix("Created patch ")
653 .unwrap()
654 .to_string();
456 655
457 // Push v2 656 // Push v2
458 repo.git(&["checkout", "feat-dedup"]); 657 repo.git(&["checkout", "feat-dedup"]);
@@ -480,7 +679,11 @@ fn test_show_displays_revision_count() {
480 repo.git(&["checkout", "-b", "feat-count"]); 679 repo.git(&["checkout", "-b", "feat-count"]);
481 repo.commit_file("a.txt", "a", "v1"); 680 repo.commit_file("a.txt", "a", "v1");
482 let out = repo.run_ok(&["patch", "create", "-t", "Count test", "-B", "feat-count"]); 681 let out = repo.run_ok(&["patch", "create", "-t", "Count test", "-B", "feat-count"]);
483 let id = out.trim().strip_prefix("Created patch ").unwrap().to_string(); 682 let id = out
683 .trim()
684 .strip_prefix("Created patch ")
685 .unwrap()
686 .to_string();
484 687
485 let out = repo.run_ok(&["patch", "show", &id]); 688 let out = repo.run_ok(&["patch", "show", &id]);
486 assert!(out.contains("(r1)")); 689 assert!(out.contains("(r1)"));
@@ -530,14 +733,22 @@ fn test_concurrent_revision_dedup_after_reconcile() {
530 // Push a new commit on the branch 733 // Push a new commit on the branch
531 let sig = git2::Signature::now("Alice", "alice@example.com").unwrap(); 734 let sig = git2::Signature::now("Alice", "alice@example.com").unwrap();
532 let blob = repo.blob(b"new content").unwrap(); 735 let blob = repo.blob(b"new content").unwrap();
533 let mut tb = repo.treebuilder(Some(&head_commit.tree().unwrap())).unwrap(); 736 let mut tb = repo
737 .treebuilder(Some(&head_commit.tree().unwrap()))
738 .unwrap();
534 tb.insert("new.txt", blob, 0o100644).unwrap(); 739 tb.insert("new.txt", blob, 0o100644).unwrap();
535 let new_tree_oid = tb.write().unwrap(); 740 let new_tree_oid = tb.write().unwrap();
536 let new_tree = repo.find_tree(new_tree_oid).unwrap(); 741 let new_tree = repo.find_tree(new_tree_oid).unwrap();
537 let new_commit = repo.commit( 742 let new_commit = repo
538 Some("refs/heads/feat-concurrent"), 743 .commit(
539 &sig, &sig, "new commit", &new_tree, &[&head_commit], 744 Some("refs/heads/feat-concurrent"),
540 ).unwrap(); 745 &sig,
746 &sig,
747 "new commit",
748 &new_tree,
749 &[&head_commit],
750 )
751 .unwrap();
541 752
542 let root_tip = repo.refname_to_id(&ref_name).unwrap(); 753 let root_tip = repo.refname_to_id(&ref_name).unwrap();
543 754
@@ -556,7 +767,8 @@ fn test_concurrent_revision_dedup_after_reconcile() {
556 let alice_tip = repo.refname_to_id(&ref_name).unwrap(); 767 let alice_tip = repo.refname_to_id(&ref_name).unwrap();
557 768
558 // Reset ref back to root, then Bob also appends same PatchRevision 769 // Reset ref back to root, then Bob also appends same PatchRevision
559 repo.reference(&ref_name, root_tip, true, "reset for bob").unwrap(); 770 repo.reference(&ref_name, root_tip, true, "reset for bob")
771 .unwrap();
560 let rev_event_bob = Event { 772 let rev_event_bob = Event {
561 timestamp: now(), 773 timestamp: now(),
562 author: bob(), 774 author: bob(),
@@ -572,14 +784,17 @@ fn test_concurrent_revision_dedup_after_reconcile() {
572 784
573 // Reconcile: create a remote ref pointing to bob's tip, local to alice's tip 785 // Reconcile: create a remote ref pointing to bob's tip, local to alice's tip
574 let remote_ref = format!("refs/collab/sync/origin/patches/{}", id); 786 let remote_ref = format!("refs/collab/sync/origin/patches/{}", id);
575 repo.reference(&remote_ref, bob_tip, true, "remote").unwrap(); 787 repo.reference(&remote_ref, bob_tip, true, "remote")
576 repo.reference(&ref_name, alice_tip, true, "restore alice").unwrap(); 788 .unwrap();
789 repo.reference(&ref_name, alice_tip, true, "restore alice")
790 .unwrap();
577 dag::reconcile(&repo, &ref_name, &remote_ref, &alice(), &sk).unwrap(); 791 dag::reconcile(&repo, &ref_name, &remote_ref, &alice(), &sk).unwrap();
578 792
579 // Materialize and verify: should have exactly 2 revisions (create + 1 deduped) 793 // Materialize and verify: should have exactly 2 revisions (create + 1 deduped)
580 let state = PatchState::from_ref(&repo, &ref_name, &id).unwrap(); 794 let state = PatchState::from_ref(&repo, &ref_name, &id).unwrap();
581 assert_eq!( 795 assert_eq!(
582 state.revisions.len(), 2, 796 state.revisions.len(),
797 2,
583 "duplicate PatchRevision events with same commit OID should be deduped to one revision" 798 "duplicate PatchRevision events with same commit OID should be deduped to one revision"
584 ); 799 );
585 assert_eq!(state.revisions[0].number, 1); 800 assert_eq!(state.revisions[0].number, 1);
tests/server_behavior_test.rs
Old New
@@ -95,7 +95,10 @@ fn missing_repository_and_missing_objects_return_not_found() {
95 assert!(missing_issue.body.contains("missing-issue")); 95 assert!(missing_issue.body.contains("missing-issue"));
96 assert!(missing_issue.body.contains("not found")); 96 assert!(missing_issue.body.contains("not found"));
97 97
98 let missing_blob = harness.get(&format!("/{}/blob/main/src/missing.rs", harness.repo_name())); 98 let missing_blob = harness.get(&format!(
99 "/{}/blob/main/src/missing.rs",
100 harness.repo_name()
101 ));
99 assert!(missing_blob.status_line.contains("404")); 102 assert!(missing_blob.status_line.contains("404"));
100 assert!(missing_blob.body.contains("src/missing.rs")); 103 assert!(missing_blob.body.contains("src/missing.rs"));
101 assert!(missing_blob.body.contains("not found")); 104 assert!(missing_blob.body.contains("not found"));
tests/signing_test.rs
Old New
@@ -107,7 +107,10 @@ fn sign_event_produces_nonempty_signature_and_pubkey() {
107 let event = make_event(); 107 let event = make_event();
108 let detached = sign_event(&event, &sk).unwrap(); 108 let detached = sign_event(&event, &sk).unwrap();
109 109
110 assert!(!detached.signature.is_empty(), "signature should not be empty"); 110 assert!(
111 !detached.signature.is_empty(),
112 "signature should not be empty"
113 );
111 assert!(!detached.pubkey.is_empty(), "pubkey should not be empty"); 114 assert!(!detached.pubkey.is_empty(), "pubkey should not be empty");
112 115
113 // Verify they are valid base64 116 // Verify they are valid base64
@@ -174,7 +177,10 @@ fn canonical_json_deterministic() {
174 let bytes1 = canonical_json(&event).unwrap(); 177 let bytes1 = canonical_json(&event).unwrap();
175 let bytes2 = canonical_json(&event).unwrap(); 178 let bytes2 = canonical_json(&event).unwrap();
176 179
177 assert_eq!(bytes1, bytes2, "canonical_json should produce identical output"); 180 assert_eq!(
181 bytes1, bytes2,
182 "canonical_json should produce identical output"
183 );
178 } 184 }
179 185
180 #[test] 186 #[test]
@@ -218,12 +224,20 @@ fn event_json_uses_namespaced_action_types() {
218 }; 224 };
219 225
220 let json = serde_json::to_string(&event).unwrap(); 226 let json = serde_json::to_string(&event).unwrap();
221 assert!(json.contains("\"type\":\"patch.create\""), "action type should be namespaced: {}", json); 227 assert!(
228 json.contains("\"type\":\"patch.create\""),
229 "action type should be namespaced: {}",
230 json
231 );
222 232
223 // Round-trip 233 // Round-trip
224 let deserialized: Event = serde_json::from_str(&json).unwrap(); 234 let deserialized: Event = serde_json::from_str(&json).unwrap();
225 match deserialized.action { 235 match deserialized.action {
226 Action::PatchCreate { ref title, ref fixes, .. } => { 236 Action::PatchCreate {
237 ref title,
238 ref fixes,
239 ..
240 } => {
227 assert_eq!(title, "Fix bug"); 241 assert_eq!(title, "Fix bug");
228 assert_eq!(fixes.as_deref(), Some("deadbeef")); 242 assert_eq!(fixes.as_deref(), Some("deadbeef"));
229 } 243 }
tests/sort_test.rs
Old New
@@ -35,13 +35,7 @@ fn open_issue_at(
35 } 35 }
36 36
37 /// Add a comment with a specific timestamp to an issue. 37 /// Add a comment with a specific timestamp to an issue.
38 fn add_comment_at( 38 fn add_comment_at(repo: &Repository, ref_name: &str, author: &Author, body: &str, timestamp: &str) {
39 repo: &Repository,
40 ref_name: &str,
41 author: &Author,
42 body: &str,
43 timestamp: &str,
44 ) {
45 let sk = test_signing_key(); 39 let sk = test_signing_key();
46 let event = Event { 40 let event = Event {
47 timestamp: timestamp.to_string(), 41 timestamp: timestamp.to_string(),
@@ -147,7 +141,8 @@ fn test_issue_default_sort_by_recency() {
147 assert_eq!(issues.len(), 2); 141 assert_eq!(issues.len(), 2);
148 142
149 // Default sort = recent: issue A (last_updated=2025-12) should come first 143 // Default sort = recent: issue A (last_updated=2025-12) should come first
150 let entries = git_collab::issue::list(&repo, true, false, None, None, SortMode::Recent).unwrap(); 144 let entries =
145 git_collab::issue::list(&repo, true, false, None, None, SortMode::Recent).unwrap();
151 assert_eq!(entries.len(), 2); 146 assert_eq!(entries.len(), 2);
152 assert_eq!(entries[0].issue.title, "Alpha issue"); 147 assert_eq!(entries[0].issue.title, "Alpha issue");
153 assert_eq!(entries[1].issue.title, "Beta issue"); 148 assert_eq!(entries[1].issue.title, "Beta issue");
@@ -172,7 +167,8 @@ fn test_issue_sort_by_created() {
172 let (_, _) = open_issue_at(&repo, &alice(), "Beta issue", "2025-06-01T00:00:00Z"); 167 let (_, _) = open_issue_at(&repo, &alice(), "Beta issue", "2025-06-01T00:00:00Z");
173 168
174 // Sort by created: B (2025-06) comes first (descending) 169 // Sort by created: B (2025-06) comes first (descending)
175 let entries = git_collab::issue::list(&repo, true, false, None, None, SortMode::Created).unwrap(); 170 let entries =
171 git_collab::issue::list(&repo, true, false, None, None, SortMode::Created).unwrap();
176 assert_eq!(entries.len(), 2); 172 assert_eq!(entries.len(), 2);
177 assert_eq!(entries[0].issue.title, "Beta issue"); 173 assert_eq!(entries[0].issue.title, "Beta issue");
178 assert_eq!(entries[1].issue.title, "Alpha issue"); 174 assert_eq!(entries[1].issue.title, "Alpha issue");
@@ -201,8 +197,7 @@ fn test_patch_last_updated_populated() {
201 let dir = TempDir::new().unwrap(); 197 let dir = TempDir::new().unwrap();
202 let repo = init_repo(dir.path(), &alice()); 198 let repo = init_repo(dir.path(), &alice());
203 199
204 let (ref_name, id) = 200 let (ref_name, id) = create_patch_at(&repo, &alice(), "Test patch", "2025-01-01T00:00:00Z");
205 create_patch_at(&repo, &alice(), "Test patch", "2025-01-01T00:00:00Z");
206 add_patch_comment_at( 201 add_patch_comment_at(
207 &repo, 202 &repo,
208 &ref_name, 203 &ref_name,
@@ -234,7 +229,8 @@ fn test_patch_default_sort_by_recency() {
234 // Patch B: created later, never updated 229 // Patch B: created later, never updated
235 let (_, _) = create_patch_at(&repo, &alice(), "Beta patch", "2025-06-01T00:00:00Z"); 230 let (_, _) = create_patch_at(&repo, &alice(), "Beta patch", "2025-06-01T00:00:00Z");
236 231
237 let patches = git_collab::patch::list(&repo, true, false, None, None, SortMode::Recent).unwrap(); 232 let patches =
233 git_collab::patch::list(&repo, true, false, None, None, SortMode::Recent).unwrap();
238 assert_eq!(patches.len(), 2); 234 assert_eq!(patches.len(), 2);
239 assert_eq!(patches[0].title, "Alpha patch"); 235 assert_eq!(patches[0].title, "Alpha patch");
240 assert_eq!(patches[1].title, "Beta patch"); 236 assert_eq!(patches[1].title, "Beta patch");
@@ -256,7 +252,8 @@ fn test_patch_sort_by_created() {
256 252
257 let (_, _) = create_patch_at(&repo, &alice(), "Beta patch", "2025-06-01T00:00:00Z"); 253 let (_, _) = create_patch_at(&repo, &alice(), "Beta patch", "2025-06-01T00:00:00Z");
258 254
259 let patches = git_collab::patch::list(&repo, true, false, None, None, SortMode::Created).unwrap(); 255 let patches =
256 git_collab::patch::list(&repo, true, false, None, None, SortMode::Created).unwrap();
260 assert_eq!(patches.len(), 2); 257 assert_eq!(patches.len(), 2);
261 assert_eq!(patches[0].title, "Beta patch"); 258 assert_eq!(patches[0].title, "Beta patch");
262 assert_eq!(patches[1].title, "Alpha patch"); 259 assert_eq!(patches[1].title, "Alpha patch");
@@ -293,18 +290,34 @@ fn test_cli_issue_list_sort_flag() {
293 let out = repo.run_ok(&["issue", "list"]); 290 let out = repo.run_ok(&["issue", "list"]);
294 let lines: Vec<&str> = out.lines().collect(); 291 let lines: Vec<&str> = out.lines().collect();
295 assert_eq!(lines.len(), 2); 292 assert_eq!(lines.len(), 2);
296 assert!(lines[0].contains("Apple"), "expected Apple first, got: {}", out); 293 assert!(
294 lines[0].contains("Apple"),
295 "expected Apple first, got: {}",
296 out
297 );
297 298
298 // Alpha sort should show Apple first (alphabetical) 299 // Alpha sort should show Apple first (alphabetical)
299 let out = repo.run_ok(&["issue", "list", "--sort", "alpha"]); 300 let out = repo.run_ok(&["issue", "list", "--sort", "alpha"]);
300 let lines: Vec<&str> = out.lines().collect(); 301 let lines: Vec<&str> = out.lines().collect();
301 assert!(lines[0].contains("Apple"), "expected Apple first in alpha sort, got: {}", out); 302 assert!(
302 assert!(lines[1].contains("Zebra"), "expected Zebra second in alpha sort, got: {}", out); 303 lines[0].contains("Apple"),
304 "expected Apple first in alpha sort, got: {}",
305 out
306 );
307 assert!(
308 lines[1].contains("Zebra"),
309 "expected Zebra second in alpha sort, got: {}",
310 out
311 );
303 312
304 // Created sort should show Apple first (most recently created) 313 // Created sort should show Apple first (most recently created)
305 let out = repo.run_ok(&["issue", "list", "--sort", "created"]); 314 let out = repo.run_ok(&["issue", "list", "--sort", "created"]);
306 let lines: Vec<&str> = out.lines().collect(); 315 let lines: Vec<&str> = out.lines().collect();
307 assert!(lines[0].contains("Apple"), "expected Apple first in created sort, got: {}", out); 316 assert!(
317 lines[0].contains("Apple"),
318 "expected Apple first in created sort, got: {}",
319 out
320 );
308 } 321 }
309 322
310 #[test] 323 #[test]
@@ -319,8 +332,16 @@ fn test_cli_patch_list_sort_flag() {
319 let out = repo.run_ok(&["patch", "list", "--sort", "alpha"]); 332 let out = repo.run_ok(&["patch", "list", "--sort", "alpha"]);
320 let lines: Vec<&str> = out.lines().collect(); 333 let lines: Vec<&str> = out.lines().collect();
321 assert_eq!(lines.len(), 2); 334 assert_eq!(lines.len(), 2);
322 assert!(lines[0].contains("Apple"), "expected Apple first in alpha sort, got: {}", out); 335 assert!(
323 assert!(lines[1].contains("Zebra"), "expected Zebra second in alpha sort, got: {}", out); 336 lines[0].contains("Apple"),
337 "expected Apple first in alpha sort, got: {}",
338 out
339 );
340 assert!(
341 lines[1].contains("Zebra"),
342 "expected Zebra second in alpha sort, got: {}",
343 out
344 );
324 } 345 }
325 346
326 #[test] 347 #[test]
tests/status_test.rs
Old New
@@ -5,7 +5,7 @@ use tempfile::TempDir;
5 use git_collab::event::ReviewVerdict; 5 use git_collab::event::ReviewVerdict;
6 use git_collab::status; 6 use git_collab::status;
7 7
8 use common::{alice, bob, create_patch, init_repo, open_issue, close_issue, add_review}; 8 use common::{add_review, alice, bob, close_issue, create_patch, init_repo, open_issue};
9 9
10 // Additional helpers not in common/mod.rs 10 // Additional helpers not in common/mod.rs
11 11
@@ -117,7 +117,11 @@ fn test_status_recent_items() {
117 117
118 let status = status::compute(&repo).unwrap(); 118 let status = status::compute(&repo).unwrap();
119 assert_eq!(status.recent_items.len(), 2); 119 assert_eq!(status.recent_items.len(), 2);
120 let titles: Vec<&str> = status.recent_items.iter().map(|i| i.title.as_str()).collect(); 120 let titles: Vec<&str> = status
121 .recent_items
122 .iter()
123 .map(|i| i.title.as_str())
124 .collect();
121 assert!(titles.contains(&"Recent issue")); 125 assert!(titles.contains(&"Recent issue"));
122 assert!(titles.contains(&"Recent patch")); 126 assert!(titles.contains(&"Recent patch"));
123 } 127 }
tests/sync_lock_test.rs
Old New
@@ -63,7 +63,10 @@ fn test_acquire_creates_lockfile() {
63 63
64 let lock = SyncLock::acquire(&repo).unwrap(); 64 let lock = SyncLock::acquire(&repo).unwrap();
65 65
66 assert!(lock.lock_path.exists(), "lockfile should exist after acquire"); 66 assert!(
67 lock.lock_path.exists(),
68 "lockfile should exist after acquire"
69 );
67 70
68 let content = fs::read_to_string(&lock.lock_path).unwrap(); 71 let content = fs::read_to_string(&lock.lock_path).unwrap();
69 let info = SyncLockInfo::from_json(&content).unwrap(); 72 let info = SyncLockInfo::from_json(&content).unwrap();
@@ -177,7 +180,11 @@ fn test_acquire_succeeds_with_dead_pid() {
177 write_lockfile(&collab, 999999, &chrono::Utc::now().to_rfc3339()); 180 write_lockfile(&collab, 999999, &chrono::Utc::now().to_rfc3339());
178 181
179 let lock = SyncLock::acquire(&repo); 182 let lock = SyncLock::acquire(&repo);
180 assert!(lock.is_ok(), "acquire should succeed with dead PID: {:?}", lock.err()); 183 assert!(
184 lock.is_ok(),
185 "acquire should succeed with dead PID: {:?}",
186 lock.err()
187 );
181 188
182 // The new lock should be ours 189 // The new lock should be ours
183 let lock = lock.unwrap(); 190 let lock = lock.unwrap();
@@ -231,12 +238,20 @@ fn test_format_lock_age() {
231 let now = chrono::Utc::now(); 238 let now = chrono::Utc::now();
232 let three_sec_ago = (now - chrono::Duration::seconds(3)).to_rfc3339(); 239 let three_sec_ago = (now - chrono::Duration::seconds(3)).to_rfc3339();
233 let result = format_lock_age(&three_sec_ago); 240 let result = format_lock_age(&three_sec_ago);
234 assert!(result.contains("second"), "expected 'second' in: {}", result); 241 assert!(
242 result.contains("second"),
243 "expected 'second' in: {}",
244 result
245 );
235 assert!(result.contains("ago"), "expected 'ago' in: {}", result); 246 assert!(result.contains("ago"), "expected 'ago' in: {}", result);
236 247
237 let two_min_ago = (now - chrono::Duration::minutes(2)).to_rfc3339(); 248 let two_min_ago = (now - chrono::Duration::minutes(2)).to_rfc3339();
238 let result = format_lock_age(&two_min_ago); 249 let result = format_lock_age(&two_min_ago);
239 assert!(result.contains("minute"), "expected 'minute' in: {}", result); 250 assert!(
251 result.contains("minute"),
252 "expected 'minute' in: {}",
253 result
254 );
240 255
241 // Invalid timestamp should fall back gracefully 256 // Invalid timestamp should fall back gracefully
242 let result = format_lock_age("not-a-timestamp"); 257 let result = format_lock_age("not-a-timestamp");
tests/sync_test.rs
Old New
@@ -231,7 +231,8 @@ fn test_cli_sync_partial_failure_can_resume_successfully() {
231 assert!(stderr.contains("Sync partially failed: 1 of 2 refs pushed.")); 231 assert!(stderr.contains("Sync partially failed: 1 of 2 refs pushed."));
232 assert!(stderr.contains(&format!("refs/collab/issues/{}", id1))); 232 assert!(stderr.contains(&format!("refs/collab/issues/{}", id1)));
233 233
234 let state = sync::SyncState::load(&cluster.alice_repo()).expect("partial sync should save state"); 234 let state =
235 sync::SyncState::load(&cluster.alice_repo()).expect("partial sync should save state");
235 assert_eq!(state.pending_refs.len(), 1); 236 assert_eq!(state.pending_refs.len(), 1);
236 assert!(state.pending_refs[0].0.contains(&id1)); 237 assert!(state.pending_refs[0].0.contains(&id1));
237 238
@@ -685,7 +686,12 @@ fn test_reconciliation_merge_commit_is_signed() {
685 let bob_ref = format!("refs/collab/issues/{}", id); 686 let bob_ref = format!("refs/collab/issues/{}", id);
686 687
687 // Both add comments — creating divergent history 688 // Both add comments — creating divergent history
688 add_comment(&alice_repo, &alice_ref, &alice(), "Alice's divergent comment"); 689 add_comment(
690 &alice_repo,
691 &alice_ref,
692 &alice(),
693 "Alice's divergent comment",
694 );
689 add_comment(&bob_repo, &bob_ref, &bob(), "Bob's divergent comment"); 695 add_comment(&bob_repo, &bob_ref, &bob(), "Bob's divergent comment");
690 696
691 // Bob pushes his comment to remote 697 // Bob pushes his comment to remote
@@ -753,7 +759,10 @@ fn test_reconciliation_merge_commit_is_signed() {
753 // Read pubkey from separate blob 759 // Read pubkey from separate blob
754 let pk_entry = tree.get_name("pubkey").expect("pubkey blob should exist"); 760 let pk_entry = tree.get_name("pubkey").expect("pubkey blob should exist");
755 let pk_blob = alice_repo.find_blob(pk_entry.id()).unwrap(); 761 let pk_blob = alice_repo.find_blob(pk_entry.id()).unwrap();
756 let commit_pubkey = std::str::from_utf8(pk_blob.content()).unwrap().trim().to_string(); 762 let commit_pubkey = std::str::from_utf8(pk_blob.content())
763 .unwrap()
764 .trim()
765 .to_string();
757 766
758 assert_eq!( 767 assert_eq!(
759 commit_pubkey, syncing_pubkey, 768 commit_pubkey, syncing_pubkey,
@@ -761,9 +770,14 @@ fn test_reconciliation_merge_commit_is_signed() {
761 ); 770 );
762 771
763 // Read signature from separate blob and verify 772 // Read signature from separate blob and verify
764 let sig_entry = tree.get_name("signature").expect("signature blob should exist"); 773 let sig_entry = tree
774 .get_name("signature")
775 .expect("signature blob should exist");
765 let sig_blob = alice_repo.find_blob(sig_entry.id()).unwrap(); 776 let sig_blob = alice_repo.find_blob(sig_entry.id()).unwrap();
766 let sig_str = std::str::from_utf8(sig_blob.content()).unwrap().trim().to_string(); 777 let sig_str = std::str::from_utf8(sig_blob.content())
778 .unwrap()
779 .trim()
780 .to_string();
767 781
768 let detached = signing::DetachedSignature { 782 let detached = signing::DetachedSignature {
769 signature: sig_str, 783 signature: sig_str,
@@ -947,7 +961,11 @@ fn test_resume_retries_only_failed_refs() {
947 961
948 // Resume sync should succeed 962 // Resume sync should succeed
949 let result = sync::sync(&alice_repo, "origin"); 963 let result = sync::sync(&alice_repo, "origin");
950 assert!(result.is_ok(), "resume sync should succeed: {:?}", result.err()); 964 assert!(
965 result.is_ok(),
966 "resume sync should succeed: {:?}",
967 result.err()
968 );
951 969
952 // State should be cleared 970 // State should be cleared
953 assert!( 971 assert!(
@@ -1043,7 +1061,11 @@ fn test_no_resume_without_state_file() {
1043 1061
1044 // No sync state file — should run full flow 1062 // No sync state file — should run full flow
1045 let result = sync::sync(&alice_repo, "origin"); 1063 let result = sync::sync(&alice_repo, "origin");
1046 assert!(result.is_ok(), "normal sync should succeed: {:?}", result.err()); 1064 assert!(
1065 result.is_ok(),
1066 "normal sync should succeed: {:?}",
1067 result.err()
1068 );
1047 } 1069 }
1048 1070
1049 // T018b [US2]: Resume cleans up stale sync refs 1071 // T018b [US2]: Resume cleans up stale sync refs
tests/trust_test.rs
Old New
@@ -126,7 +126,13 @@ fn test_key_remove_existing() {
126 repo.run_ok(&["key", "add", "--self", "--label", "Alice"]); 126 repo.run_ok(&["key", "add", "--self", "--label", "Alice"]);
127 127
128 let list = repo.run_ok(&["key", "list"]); 128 let list = repo.run_ok(&["key", "list"]);
129 let pubkey = list.lines().next().unwrap().split_whitespace().next().unwrap(); 129 let pubkey = list
130 .lines()
131 .next()
132 .unwrap()
133 .split_whitespace()
134 .next()
135 .unwrap();
130 136
131 let out = repo.run_ok(&["key", "remove", pubkey]); 137 let out = repo.run_ok(&["key", "remove", pubkey]);
132 assert!( 138 assert!(
@@ -148,7 +154,11 @@ fn test_key_remove_nonexistent_errors() {
148 // Add a key first so the file exists 154 // Add a key first so the file exists
149 repo.run_ok(&["key", "add", "--self"]); 155 repo.run_ok(&["key", "add", "--self"]);
150 156
151 let err = repo.run_err(&["key", "remove", "YWJjZGVmZ2hpamtsbW5vcHFyc3R1dnd4eXoxMjM0NTY="]); 157 let err = repo.run_err(&[
158 "key",
159 "remove",
160 "YWJjZGVmZ2hpamtsbW5vcHFyc3R1dnd4eXoxMjM0NTY=",
161 ]);
152 assert!( 162 assert!(
153 err.contains("not in the trusted keys list"), 163 err.contains("not in the trusted keys list"),
154 "should say not found, got: {}", 164 "should say not found, got: {}",
@@ -167,7 +177,14 @@ fn test_key_add_then_remove_then_readd() {
167 repo.run_ok(&["key", "add", "--self", "--label", "Original"]); 177 repo.run_ok(&["key", "add", "--self", "--label", "Original"]);
168 178
169 let list = repo.run_ok(&["key", "list"]); 179 let list = repo.run_ok(&["key", "list"]);
170 let pubkey = list.lines().next().unwrap().split_whitespace().next().unwrap().to_string(); 180 let pubkey = list
181 .lines()
182 .next()
183 .unwrap()
184 .split_whitespace()
185 .next()
186 .unwrap()
187 .to_string();
171 188
172 // Remove 189 // Remove
173 repo.run_ok(&["key", "remove", &pubkey]); 190 repo.run_ok(&["key", "remove", &pubkey]);