a73x

11193927

Show live issue claims in the web UI

a73x   2026-09-06 08:09

Commit message
Show live issue claims in the web UI
src/server/http/mod.rs
Old New
@@ -20,6 +20,9 @@ use std::sync::Arc;
20 pub struct AppState { 20 pub struct AppState {
21 pub repos_dir: PathBuf, 21 pub repos_dir: PathBuf,
22 pub site_title: String, 22 pub site_title: String,
23 /// The collaboration database, read (never written) by these handlers to
24 /// show who has claimed what.
25 pub collab_db: PathBuf,
23 } 26 }
24 27
25 pub fn router(state: AppState) -> Router { 28 pub fn router(state: AppState) -> Router {
src/server/http/repo/issues.rs
Old New
@@ -5,6 +5,29 @@ use axum::response::{IntoResponse, Response};
5 5
6 use super::{collab_counts, internal_error, not_found, open_repo, AppState, CommentView}; 6 use super::{collab_counts, internal_error, not_found, open_repo, AppState, CommentView};
7 use crate::http::timestamp::Timestamp; 7 use crate::http::timestamp::Timestamp;
8 use crate::leases;
9
10 /// Every live claim in a repository, as `issue id -> (holder, expiry)`.
11 ///
12 /// A missing or unreadable lease database is treated as "no claims": the
13 /// claim is a hint on a page, and a server that has never served a
14 /// `collab-lease` verb has no database at all. Failing the page over an
15 /// absent hint would be the wrong trade.
16 fn claims_for(
17 state: &AppState,
18 entry: &crate::repos::RepoEntry,
19 ) -> std::collections::HashMap<String, (String, Option<i64>)> {
20 let key = leases::repo_key(&state.repos_dir, &entry.path);
21 let now = chrono::Utc::now().timestamp();
22 leases::open(&state.collab_db)
23 .and_then(|conn| leases::list(&conn, &key, now))
24 .map(|rows| {
25 rows.into_iter()
26 .map(|l| (l.issue_id, (l.holder, l.expires_at)))
27 .collect()
28 })
29 .unwrap_or_default()
30 }
8 31
9 /// Which issues to include in the list view. Defaults to `Open` so the list 32 /// Which issues to include in the list view. Defaults to `Open` so the list
10 /// agrees with the nav badge, which only ever counts open issues. Closed and 33 /// agrees with the nav badge, which only ever counts open issues. Closed and
@@ -48,6 +71,9 @@ pub struct IssueListItem {
48 pub author: String, 71 pub author: String,
49 pub labels: String, 72 pub labels: String,
50 pub updated: Timestamp, 73 pub updated: Timestamp,
74 /// Who holds a live claim on this issue, or empty when nobody does — the
75 /// same string-or-empty convention `labels` uses.
76 pub claimed_by: String,
51 } 77 }
52 78
53 #[derive(Debug)] 79 #[derive(Debug)]
@@ -60,6 +86,10 @@ pub struct IssueDetailView {
60 pub assignees: String, 86 pub assignees: String,
61 pub close_reason: Option<String>, 87 pub close_reason: Option<String>,
62 pub comments: Vec<CommentView>, 88 pub comments: Vec<CommentView>,
89 /// Who holds a live claim, or empty when nobody does.
90 pub claimed_by: String,
91 /// When the claim lapses, or empty for an open-ended one (an assignment).
92 pub claim_expires: String,
63 } 93 }
64 94
65 #[derive(askama::Template, askama_web::WebTemplate)] 95 #[derive(askama::Template, askama_web::WebTemplate)]
@@ -93,12 +123,13 @@ pub async fn issues(
93 Query(query): Query<IssuesQuery>, 123 Query(query): Query<IssuesQuery>,
94 State(state): State<Arc<AppState>>, 124 State(state): State<Arc<AppState>>,
95 ) -> Response { 125 ) -> Response {
96 let (_entry, repo) = match open_repo(&state, &repo_name) { 126 let (entry, repo) = match open_repo(&state, &repo_name) {
97 Ok(r) => r, 127 Ok(r) => r,
98 Err(resp) => return resp, 128 Err(resp) => return resp,
99 }; 129 };
100 130
101 let (open_patches, open_issues) = collab_counts(&repo); 131 let (open_patches, open_issues) = collab_counts(&repo);
132 let claims = claims_for(&state, &entry);
102 133
103 let filter = IssueListFilter::from_query(query.filter.as_deref()); 134 let filter = IssueListFilter::from_query(query.filter.as_deref());
104 135
@@ -133,6 +164,10 @@ pub async fn issues(
133 .map(|i| { 164 .map(|i| {
134 let id = i.id.clone(); 165 let id = i.id.clone();
135 let short_id = abbrev.of(&id).to_string(); 166 let short_id = abbrev.of(&id).to_string();
167 let claimed_by = claims
168 .get(&id)
169 .map(|(holder, _)| holder.clone())
170 .unwrap_or_default();
136 IssueListItem { 171 IssueListItem {
137 short_id, 172 short_id,
138 id, 173 id,
@@ -141,6 +176,7 @@ pub async fn issues(
141 author: i.author.name, 176 author: i.author.name,
142 labels: i.labels.join(", "), 177 labels: i.labels.join(", "),
143 updated: Timestamp::new(i.last_updated), 178 updated: Timestamp::new(i.last_updated),
179 claimed_by,
144 } 180 }
145 }) 181 })
146 .collect(); 182 .collect();
@@ -172,7 +208,7 @@ pub async fn issue_detail(
172 Path((repo_name, issue_id)): Path<(String, String)>, 208 Path((repo_name, issue_id)): Path<(String, String)>,
173 State(state): State<Arc<AppState>>, 209 State(state): State<Arc<AppState>>,
174 ) -> Response { 210 ) -> Response {
175 let (_entry, repo) = match open_repo(&state, &repo_name) { 211 let (entry, repo) = match open_repo(&state, &repo_name) {
176 Ok(r) => r, 212 Ok(r) => r,
177 Err(resp) => return resp, 213 Err(resp) => return resp,
178 }; 214 };
@@ -189,6 +225,20 @@ pub async fn issue_detail(
189 Err(_) => return internal_error(&state, "Failed to load issue state."), 225 Err(_) => return internal_error(&state, "Failed to load issue state."),
190 }; 226 };
191 227
228 // The one issue's claim, asked for directly rather than filtered out of
229 // the whole repository's list.
230 let claim = leases::open(&state.collab_db)
231 .and_then(|conn| {
232 leases::current(
233 &conn,
234 &leases::repo_key(&state.repos_dir, &entry.path),
235 &full_id,
236 chrono::Utc::now().timestamp(),
237 )
238 })
239 .ok()
240 .flatten();
241
192 let issue = IssueDetailView { 242 let issue = IssueDetailView {
193 title: is.title, 243 title: is.title,
194 body: is.body, 244 body: is.body,
@@ -197,6 +247,13 @@ pub async fn issue_detail(
197 labels: is.labels.join(", "), 247 labels: is.labels.join(", "),
198 assignees: is.assignees.join(", "), 248 assignees: is.assignees.join(", "),
199 close_reason: is.close_reason, 249 close_reason: is.close_reason,
250 claimed_by: claim.as_ref().map(|l| l.holder.clone()).unwrap_or_default(),
251 claim_expires: claim
252 .as_ref()
253 .and_then(|l| l.expires_at)
254 .and_then(|s| chrono::DateTime::from_timestamp(s, 0))
255 .map(|dt| dt.to_rfc3339_opts(chrono::SecondsFormat::Secs, true))
256 .unwrap_or_default(),
200 comments: is 257 comments: is
201 .comments 258 .comments
202 .into_iter() 259 .into_iter()
src/server/http/templates/issue_detail.html
Old New
@@ -8,6 +8,9 @@
8 <span class="status-{{ issue.status }}">{{ issue.status }}</span> 8 <span class="status-{{ issue.status }}">{{ issue.status }}</span>
9 &nbsp; by <strong>{{ issue.author }}</strong> 9 &nbsp; by <strong>{{ issue.author }}</strong>
10 </p> 10 </p>
11 {% if !issue.claimed_by.is_empty() %}
12 <p style="color: #666;">Claimed by: <strong>{{ issue.claimed_by }}</strong>{% if !issue.claim_expires.is_empty() %} <span class="mono">(expires {{ issue.claim_expires }})</span>{% endif %}</p>
13 {% endif %}
11 {% if !issue.labels.is_empty() %} 14 {% if !issue.labels.is_empty() %}
12 <p style="color: #666;">Labels: {{ issue.labels }}</p> 15 <p style="color: #666;">Labels: {{ issue.labels }}</p>
13 {% endif %} 16 {% endif %}
src/server/http/templates/issues.html
Old New
@@ -27,6 +27,7 @@
27 <th>Status</th> 27 <th>Status</th>
28 <th>Title</th> 28 <th>Title</th>
29 <th>Author</th> 29 <th>Author</th>
30 <th>Claimed by</th>
30 <th>Labels</th> 31 <th>Labels</th>
31 <th>Updated</th> 32 <th>Updated</th>
32 </tr> 33 </tr>
@@ -38,6 +39,7 @@
38 <td><span class="status-{{ i.status }}">{{ i.status }}</span></td> 39 <td><span class="status-{{ i.status }}">{{ i.status }}</span></td>
39 <td><a href="/{{ repo_name }}/issues/{{ i.id }}">{{ i.title }}</a></td> 40 <td><a href="/{{ repo_name }}/issues/{{ i.id }}">{{ i.title }}</a></td>
40 <td style="color: #666;">{{ i.author }}</td> 41 <td style="color: #666;">{{ i.author }}</td>
42 <td style="color: #666;">{{ i.claimed_by }}</td>
41 <td style="color: #666;">{{ i.labels }}</td> 43 <td style="color: #666;">{{ i.labels }}</td>
42 <td class="mono timestamp" style="color: #666;" title="{{ i.updated.full }}">{{ i.updated.short }}</td> 44 <td class="mono timestamp" style="color: #666;" title="{{ i.updated.full }}">{{ i.updated.short }}</td>
43 </tr> 45 </tr>
src/server/leases.rs
Old New
@@ -92,6 +92,23 @@ pub enum Release {
92 NotHolder { holder: String }, 92 NotHolder { holder: String },
93 } 93 }
94 94
95 /// The `repo` column value for a repository on disk: its path relative to
96 /// `repos_dir` (so `…/repos/myrepo.git` becomes `myrepo.git`).
97 ///
98 /// One function because two callers need the identical string and they reach
99 /// it from different directions — the SSH verb from a resolved exec path, the
100 /// web UI from a `RepoEntry`. Deriving it twice is how they would drift, and
101 /// the symptom would be a claim that exists over SSH but is invisible on the
102 /// page. Relative rather than absolute so the rows survive `repos_dir`
103 /// moving.
104 pub fn repo_key(repos_dir: &Path, repo_path: &Path) -> String {
105 repo_path
106 .strip_prefix(repos_dir)
107 .unwrap_or(repo_path)
108 .to_string_lossy()
109 .to_string()
110 }
111
95 /// Open (creating if needed) the lease database and ensure the schema. 112 /// Open (creating if needed) the lease database and ensure the schema.
96 pub fn open(path: &Path) -> rusqlite::Result<Connection> { 113 pub fn open(path: &Path) -> rusqlite::Result<Connection> {
97 let conn = Connection::open(path)?; 114 let conn = Connection::open(path)?;
@@ -542,6 +559,24 @@ mod tests {
542 } 559 }
543 560
544 #[test] 561 #[test]
562 fn repo_key_is_relative_to_repos_dir() {
563 assert_eq!(
564 repo_key(Path::new("/srv/git"), Path::new("/srv/git/myrepo.git")),
565 "myrepo.git"
566 );
567 assert_eq!(
568 repo_key(Path::new("/srv/git"), Path::new("/srv/git/org/sub.git")),
569 "org/sub.git"
570 );
571 // A path outside repos_dir cannot be made relative; keep it whole
572 // rather than inventing a key that could collide.
573 assert_eq!(
574 repo_key(Path::new("/srv/git"), Path::new("/elsewhere/x.git")),
575 "/elsewhere/x.git"
576 );
577 }
578
579 #[test]
545 fn open_creates_file_and_schema() { 580 fn open_creates_file_and_schema() {
546 let dir = tempfile::tempdir().unwrap(); 581 let dir = tempfile::tempdir().unwrap();
547 let path = dir.path().join("collab.db"); 582 let path = dir.path().join("collab.db");
src/server/main.rs
Old New
@@ -230,6 +230,7 @@ async fn main() {
230 let app_state = http::AppState { 230 let app_state = http::AppState {
231 repos_dir: config.repos_dir.clone(), 231 repos_dir: config.repos_dir.clone(),
232 site_title: config.site_title.clone(), 232 site_title: config.site_title.clone(),
233 collab_db: config.collab_db_path(),
233 }; 234 };
234 let router = http::router(app_state); 235 let router = http::router(app_state);
235 236
src/server/ssh/session.rs
Old New
@@ -437,14 +437,9 @@ impl SshHandler {
437 _ => principal.to_string(), 437 _ => principal.to_string(),
438 }; 438 };
439 439
440 // The repo key leases are stored under. `entry_for_path` already 440 // The repo key leases are stored under — the same derivation the web
441 // proved this path is a repo under repos_dir; the relative form keeps 441 // UI uses, so a claim made here is the claim shown there.
442 // rows portable if repos_dir moves. 442 let repo_key = crate::leases::repo_key(&self.config.repos_dir, resolved_path);
443 let repo_key = resolved_path
444 .strip_prefix(&self.config.repos_dir)
445 .unwrap_or(resolved_path)
446 .to_string_lossy()
447 .to_string();
448 443
449 // Acquiring points at a specific issue, so the issue has to exist and 444 // Acquiring points at a specific issue, so the issue has to exist and
450 // be open: leasing work nobody can do is a bug we should refuse, not 445 // be open: leasing work nobody can do is a bug we should refuse, not
tests/web_rendering_test.rs
Old New
@@ -412,3 +412,96 @@ fn rendering_pages_moves_no_refs_and_appends_no_events() {
412 "rendering pages changed the repository's refs" 412 "rendering pages changed the repository's refs"
413 ); 413 );
414 } 414 }
415
416 /// A live claim belongs on the page: a claim nobody can see is a claim people
417 /// work around. Both the list and the detail view show the holder, and an
418 /// expired one shows nobody — `refs/collab/*` says nothing about either, so
419 /// this is the only place the lease store and the UI meet.
420 #[test]
421 fn a_live_claim_shows_on_the_issue_pages_and_an_expired_one_does_not() {
422 let harness = ServerHarness::new("render-claims");
423 harness.push_head();
424 let (_ref_name, full_id) = common::open_issue(
425 &harness.work_repo_git2(),
426 &common::alice(),
427 "An issue to claim",
428 );
429 harness.push_collab_refs();
430 let name = harness.repo_name().to_string();
431
432 let unclaimed_list = harness.get_ok(&format!("/{name}/issues"));
433 assert!(
434 unclaimed_list.body.contains("Claimed by"),
435 "the column should exist even with no claims"
436 );
437
438 let acquired = harness.ssh_exec(&format!(
439 "collab-lease acquire '{name}.git' '{full_id}' --ttl 600"
440 ));
441 assert!(acquired.status.success());
442 let holder =
443 serde_json::from_str::<serde_json::Value>(String::from_utf8_lossy(&acquired.stdout).trim())
444 .unwrap()["holder"]
445 .as_str()
446 .unwrap()
447 .to_string();
448
449 let list = harness.get_ok(&format!("/{name}/issues"));
450 assert!(
451 list.body.contains(&holder),
452 "the list should name the holder {holder}: {}",
453 list.body
454 );
455
456 let detail = harness.get_ok(&format!("/{name}/issues/{full_id}"));
457 assert!(
458 detail.body.contains("Claimed by:") && detail.body.contains(&holder),
459 "the detail page should name the holder: {}",
460 detail.body
461 );
462 assert!(
463 detail.body.contains("expires"),
464 "a ttl claim should say when it lapses: {}",
465 detail.body
466 );
467
468 // A 1-second lease, waited out: the row survives (it carries the fencing
469 // token) but the page must stop showing a claim.
470 harness.ssh_exec(&format!("collab-lease release '{name}.git' '{full_id}'"));
471 harness.ssh_exec(&format!(
472 "collab-lease acquire '{name}.git' '{full_id}' --ttl 1"
473 ));
474 std::thread::sleep(std::time::Duration::from_millis(1200));
475
476 let expired = harness.get_ok(&format!("/{name}/issues/{full_id}"));
477 assert!(
478 !expired.body.contains("Claimed by:"),
479 "an expired claim must not be shown: {}",
480 expired.body
481 );
482 }
483
484 /// An open-ended claim (no `--ttl`) is an assignment, so it must not claim to
485 /// expire.
486 #[test]
487 fn an_open_ended_claim_shows_no_expiry() {
488 let harness = ServerHarness::new("render-assigned");
489 harness.push_head();
490 let (_ref_name, full_id) = common::open_issue(
491 &harness.work_repo_git2(),
492 &common::alice(),
493 "An assigned issue",
494 );
495 harness.push_collab_refs();
496 let name = harness.repo_name().to_string();
497
498 harness.ssh_exec(&format!("collab-lease acquire '{name}.git' '{full_id}'"));
499
500 let detail = harness.get_ok(&format!("/{name}/issues/{full_id}"));
501 assert!(detail.body.contains("Claimed by:"));
502 assert!(
503 !detail.body.contains("expires"),
504 "an open-ended claim must not print an expiry: {}",
505 detail.body
506 );
507 }