Issues close from changes landing on the default bookmark, however they arrive acc85ac0f70b
"Closes #N" closed an issue only when a push imported the change onto the default bookmark. Landing on the hub is not a push, and a client's oxy proposal land moves main to changes the hub already holds, so landed proposals closed nothing (Metal Gear 2 closed them by hand). Now every change in the default bookmark's new state and not in its old one counts, on a push and on a hub landing, and a landed proposal's description closes issues too. closes #151
- Author
- Andy McCall @andymccall <[email protected]>
- When
- · 2026-10-13 12:15 UTC
- Signature
- ✔ signed by Andy McCall
- Hash
acc85ac0f70b2a5de2d5efb962968443e22475b977c2c19b6aec98baf14a2300- Depends on
- 18e381ea88c2 1f1eb06a9009 3f4b0ea6eabf 71e65fcf95ab 757195ad7a5d 78001759604c 93be9ce4653e b64e2537d702 c79a1111dd56 d23464c0ac60 de505df5cf8d
- Depended on by
- dd0788b756f2 86a69b53aff0 2a8b621676b4 c08cec372c81 f7bcee49bf24 d943160dafeb
- In bookmarks
- api-unknown-fields cancel-landed-runs diffusion-pin-bookmark draft-ready main oh-comment-body-file proposal-list-fast
- In tags
- v0.5.0 v0.5.1 v0.5.2 v0.6.0 v0.6.1 v0.7.0 v0.8.0 v0.9.0 v0.9.1 v0.9.11 v0.9.15 v0.9.16 v0.9.19 v0.9.2 v0.9.20 v0.9.21 v0.9.22 v0.9.23 v0.9.24 v0.9.3 v0.9.4 v0.9.5 v0.9.6 v0.9.7 v0.9.8 v0.9.9
- Edits
- 7 atoms, 0 tree operations, 5 files
Browse the tree at this change
modified crates/hub/src/protocol.rs +15 −1
| … | 323 unchanged lines |
||
| 324 | 324 | } |
|
| 325 | 325 | let app = app_hint.lock().unwrap().as_ref().and_then(|w| w.upgrade()); |
|
| 326 | 326 | if let Some(app) = app { |
|
| 327 | - | crate::web::issues::close_from_messages(&app, &row, &messages).await; |
|
| 327 | + | // Issues close from every change newly on the default |
|
| 328 | + | // bookmark, not only those this push imported: a client's |
|
| 329 | + | // landing moves main to changes the hub already held. |
|
| 330 | + | if let Some(r) = results.iter().find(|r| r.ok && r.reference == row.default_bookmark) { |
|
| 331 | + | let new_heads = moved.iter().find(|(b, _, _)| *b == row.default_bookmark).map(|(_, _, h)| h.clone()).unwrap_or_default(); |
|
| 332 | + | let old = r.old_state.clone(); |
|
| 333 | + | let old_heads = crate::repos::RepoName::new(&row.owner, &row.name); |
|
| 334 | + | let old_heads = match (old_heads, oxy_core::hash::ObjectRef::from_hex(&old)) { |
|
| 335 | + | (Some(n), Some(s)) => app.repos.read(&n, move |repo, _| Ok(repo.store().get_state(&s).map(|st| st.heads).unwrap_or_default())).await.unwrap_or_default(), |
|
| 336 | + | _ => Vec::new(), |
|
| 337 | + | }; |
|
| 338 | + | crate::web::issues::close_from_arrived(&app, &row, old_heads, new_heads, None).await; |
|
| 339 | + | } else { |
|
| 340 | + | crate::web::issues::close_from_messages(&app, &row, &messages).await; |
|
| 341 | + | } |
|
| 328 | 342 | let actor = pusher.as_ref().map(|(_, n)| n.clone()).unwrap_or_default(); |
|
| 329 | 343 | crate::web::issues::references_from_messages(&app, &row, &imported, &on, &actor).await; |
|
| 330 | 344 | if results.iter().any(|r| r.ok && r.reference == row.default_bookmark) { |
|
| … | 63 unchanged lines |
modified crates/hub/src/web/issues.rs +31 −0
| … | 1017 unchanged lines |
||
| 1018 | 1018 | /// Close issues a landed change's message names (`closes #12`), when the |
|
| 1019 | 1019 | /// landing reached the default bookmark. Called from the push hook and |
|
| 1020 | 1020 | /// the hub's own landing. |
|
| 1021 | + | /// Close the issues that the changes newly on the default bookmark say |
|
| 1022 | + | /// they close: those in `new_heads`'s state and not in `old_heads`'s, |
|
| 1023 | + | /// however they got there (a push, a landing on the hub, a client's |
|
| 1024 | + | /// `oxy proposal land` of changes the hub already held). `extra` adds |
|
| 1025 | + | /// text that closes as well, such as a landed proposal's description. |
|
| 1026 | + | pub async fn close_from_arrived( |
|
| 1027 | + | app: &Shared, |
|
| 1028 | + | row: &db::repos::RepoRow, |
|
| 1029 | + | old_heads: Vec<oxy_core::hash::ChangeHash>, |
|
| 1030 | + | new_heads: Vec<oxy_core::hash::ChangeHash>, |
|
| 1031 | + | extra: Option<(String, String)>, |
|
| 1032 | + | ) { |
|
| 1033 | + | let Some(name) = crate::repos::RepoName::new(&row.owner, &row.name) else { return }; |
|
| 1034 | + | let mut messages: Vec<(String, String)> = app |
|
| 1035 | + | .repos |
|
| 1036 | + | .read(&name, move |r, cache| { |
|
| 1037 | + | let new = cache.summary(r, &new_heads)?; |
|
| 1038 | + | let old = if old_heads.is_empty() { None } else { cache.summary(r, &old_heads).ok() }; |
|
| 1039 | + | Ok(new |
|
| 1040 | + | .changes |
|
| 1041 | + | .iter() |
|
| 1042 | + | .filter(|h| !old.as_ref().is_some_and(|o| o.changes.contains(*h))) |
|
| 1043 | + | .filter_map(|h| r.store().get_change_meta(h).ok().map(|c| (h.to_hex(), c.message))) |
|
| 1044 | + | .collect()) |
|
| 1045 | + | }) |
|
| 1046 | + | .await |
|
| 1047 | + | .unwrap_or_default(); |
|
| 1048 | + | messages.extend(extra); |
|
| 1049 | + | close_from_messages(app, row, &messages).await; |
|
| 1050 | + | } |
|
| 1051 | + | ||
| 1021 | 1052 | pub async fn close_from_messages(app: &Shared, row: &db::repos::RepoRow, messages: &[(String, String)]) { |
|
| 1022 | 1053 | for (hash, message) in messages { |
|
| 1023 | 1054 | for n in render::refs::closes(message) { |
|
| … | 27 unchanged lines |
modified crates/hub/src/web/proposals.rs +6 −0
| … | 1226 unchanged lines |
||
| 1227 | 1227 | ) |
|
| 1228 | 1228 | .await |
|
| 1229 | 1229 | .ok(); |
|
| 1230 | + | // A landing on the default bookmark closes the issues its changes (and |
|
| 1231 | + | // its description) say they close, as a push there would. |
|
| 1232 | + | if p.target == row.default_bookmark { |
|
| 1233 | + | let body = (format!("proposal #{number}"), p.body.clone()); |
|
| 1234 | + | crate::web::issues::close_from_arrived(app, row, loaded.target_heads.clone(), loaded.heads.clone(), Some(body)).await; |
|
| 1235 | + | } |
|
| 1230 | 1236 | // Diffusion (#150): a landing moves the target as a push would, so the |
|
| 1231 | 1237 | // pin index follows it, and the consumers of a moved default bookmark |
|
| 1232 | 1238 | // are bumped. |
|
| … | 486 unchanged lines |
modified crates/hub/tests/proposals.rs +89 −0
| … | 599 unchanged lines |
||
| 600 | 600 | assert_ne!(patch(r#"{"state":"closed"}"#, None).await.0, StatusCode::OK, "not without a token"); |
|
| 601 | 601 | assert_eq!(state().await, "open"); |
|
| 602 | 602 | } |
|
| 603 | + | ||
| 604 | + | /// Metal Gear 2's finding: a landed proposal's "Closes #N" closed nothing, |
|
| 605 | + | /// because only changes a push imports were read. Changes that arrive on |
|
| 606 | + | /// the default bookmark close issues however they get there. |
|
| 607 | + | #[tokio::test(flavor = "multi_thread")] |
|
| 608 | + | async fn closes_in_a_landed_change_closes_the_issue() { |
|
| 609 | + | let h = hub().await; |
|
| 610 | + | let app = &h.app; |
|
| 611 | + | let andy = register(app, "andy").await; |
|
| 612 | + | let repo = RepoName::parse("andy/demo").unwrap(); |
|
| 613 | + | app.repos.create(&repo, None).unwrap(); |
|
| 614 | + | db::repos::create(&app.db, "andy", "demo", "", hub_api::Visibility::Public).await.unwrap(); |
|
| 615 | + | let json = |method: &'static str, path: String, body: &'static str| async move { |
|
| 616 | + | let res = oxygenhub::router(app.clone()) |
|
| 617 | + | .oneshot( |
|
| 618 | + | Request::builder() |
|
| 619 | + | .method(method) |
|
| 620 | + | .uri(path) |
|
| 621 | + | .header(header::AUTHORIZATION, "Bearer site") |
|
| 622 | + | .header(header::CONTENT_TYPE, "application/json") |
|
| 623 | + | .body(Body::from(body)) |
|
| 624 | + | .unwrap(), |
|
| 625 | + | ) |
|
| 626 | + | .await |
|
| 627 | + | .unwrap(); |
|
| 628 | + | let st = res.status(); |
|
| 629 | + | (st, String::from_utf8_lossy(&res.into_body().collect().await.unwrap().to_bytes()).into_owned()) |
|
| 630 | + | }; |
|
| 631 | + | for t in ["one", "two"] { |
|
| 632 | + | let body: &'static str = if t == "one" { r#"{"title":"one"}"# } else { r#"{"title":"two"}"# }; |
|
| 633 | + | let (st, b) = json("POST", "/api/v1/repos/andy/demo/issues".into(), body).await; |
|
| 634 | + | assert_eq!(st, StatusCode::CREATED, "{b}"); |
|
| 635 | + | } |
|
| 636 | + | let work = tempfile::tempdir().unwrap(); |
|
| 637 | + | let mut client = Repo::init(work.path()).unwrap(); |
|
| 638 | + | client.create_identity("Andy McCall", "[email protected]", oxy_core::repo::IdentityScope::User).unwrap(); |
|
| 639 | + | client.config.set("remote.origin.url", &format!("{}/andy/demo", h.base)).unwrap(); |
|
| 640 | + | client.config.set("remote.origin.token", "site").unwrap(); |
|
| 641 | + | client.save_config().unwrap(); |
|
| 642 | + | write(work.path(), "a.txt", b"one\n"); |
|
| 643 | + | client.snapshot("t").unwrap(); |
|
| 644 | + | let base = client.commit("base", &[]).unwrap(); |
|
| 645 | + | assert!(client.push("origin", false).unwrap().iter().all(|r| r.ok)); |
|
| 646 | + | client.bookmark_create("feature", Some(&[base])).unwrap(); |
|
| 647 | + | client.bookmark_switch("feature", false).unwrap(); |
|
| 648 | + | write(work.path(), "b.txt", b"b\n"); |
|
| 649 | + | client.snapshot("t").unwrap(); |
|
| 650 | + | client.commit("add b\n\nCloses #1", &[]).unwrap(); |
|
| 651 | + | assert!(client.push_bookmark("origin", "feature", false).unwrap().iter().all(|r| r.ok)); |
|
| 652 | + | let state = |n: i64| async move { db::issues::get(&app.db, 1, n).await.unwrap().unwrap().state }; |
|
| 653 | + | assert_eq!(state(1).await, "open", "a push to another bookmark closes nothing"); |
|
| 654 | + | ||
| 655 | + | // Landed on the hub: #1 closes. |
|
| 656 | + | let preview = get(app, "/andy/demo/proposals/new?source=feature&target=main", Some(&andy)).await; |
|
| 657 | + | let csrf = csrf_of(&preview.body); |
|
| 658 | + | let r = post(app, "/andy/demo/proposals/new", Some(&andy), &format!("_csrf={csrf}&title=B&body=&target=main&source=feature")).await; |
|
| 659 | + | assert_eq!(r.status, StatusCode::SEE_OTHER, "{}", r.body); |
|
| 660 | + | let number = db::proposals::list(&app.db, 1, &["open"]).await.unwrap()[0].number; |
|
| 661 | + | let (st, b) = json("POST", format!("/api/v1/repos/andy/demo/proposals/{number}/land"), "{}").await; |
|
| 662 | + | assert_eq!(st, StatusCode::OK, "{b}"); |
|
| 663 | + | assert_eq!(state(1).await, "closed", "landing closes what the change says"); |
|
| 664 | + | ||
| 665 | + | // A client moving main to a change the hub already holds: #2 closes. |
|
| 666 | + | client.bookmark_create("other", Some(&[base])).unwrap(); |
|
| 667 | + | client.bookmark_switch("other", false).unwrap(); |
|
| 668 | + | write(work.path(), "c.txt", b"c\n"); |
|
| 669 | + | client.snapshot("t").unwrap(); |
|
| 670 | + | let c = client.commit("add c, closes #2", &[]).unwrap(); |
|
| 671 | + | assert!(client.push_bookmark("origin", "other", false).unwrap().iter().all(|r| r.ok)); |
|
| 672 | + | assert_eq!(state(2).await, "open"); |
|
| 673 | + | let (_, list) = json("GET", "/api/v1/repos/andy/demo/bookmarks".into(), "").await; |
|
| 674 | + | let list: Vec<serde_json::Value> = serde_json::from_str(&list).unwrap(); |
|
| 675 | + | let main = list.iter().find(|b| b["name"] == "main").unwrap(); |
|
| 676 | + | let mut heads: Vec<oxy_core::hash::ChangeHash> = |
|
| 677 | + | main["heads"].as_array().unwrap().iter().map(|h| oxy_core::hash::ChangeHash::from_hex(h.as_str().unwrap()).unwrap()).collect(); |
|
| 678 | + | client.fetch("origin").unwrap(); |
|
| 679 | + | heads.push(c); |
|
| 680 | + | client.bookmark_set("main", &heads).unwrap(); |
|
| 681 | + | assert!(client.push_bookmark("origin", "main", false).unwrap().iter().all(|r| r.ok)); |
|
| 682 | + | let mut s2 = state(2).await; |
|
| 683 | + | for _ in 0..100 { |
|
| 684 | + | if s2 == "closed" { |
|
| 685 | + | break; |
|
| 686 | + | } |
|
| 687 | + | tokio::time::sleep(std::time::Duration::from_millis(30)).await; |
|
| 688 | + | s2 = state(2).await; |
|
| 689 | + | } |
|
| 690 | + | assert_eq!(s2, "closed", "a change arriving by push closes it, though the hub held it already"); |
|
| 691 | + | } |
modified docs/spec.md +1 −1
| … | 412 unchanged lines |
||
| 413 | 413 | ||
| 414 | 414 | **The chain of work.** A proposal shows the issues it closes (`closes`, `fixes` or `resolves #N` in its description or in any of its changes' messages) in a strip under its title: each issue with its state, then the proposal, then its target bookmark, marked landed once it is. A Closes list sits in its sidebar. An issue shows the proposals that close it, by the same rule, with their states. So the path from a report to the change that landed it can be followed from either end. |
|
| 415 | 415 | ||
| 416 | - | Relative links and images in issue, comment and proposal text resolve against the repository's default bookmark, and in release notes against the release's tag. Cross references: `#12` links an issue or proposal; `owner/name#12` across repositories; a 12-hex-or-longer token links a change when it resolves; `closes #12` in a change message closes the issue when the change lands on the default bookmark ("closed by change"). Any other `#12` in a pushed change's message, on any bookmark, puts "referenced by change <hash> on <bookmark>" on that issue's or proposal's timeline, the hash linking to the change; other repositories' `owner/name#12` are left alone. |
|
| 416 | + | Relative links and images in issue, comment and proposal text resolve against the repository's default bookmark, and in release notes against the release's tag. Cross references: `#12` links an issue or proposal; `owner/name#12` across repositories; a 12-hex-or-longer token links a change when it resolves; `closes #12` in a change message closes the issue when the change lands on the default bookmark ("closed by change"), however it arrives: every change in the default bookmark's new state and not in its old one counts, whether a push imported it, a client's `oxy proposal land` moved main to changes the hub already held, or the hub landed a proposal (whose description closes issues too); #151. Any other `#12` in a pushed change's message, on any bookmark, puts "referenced by change <hash> on <bookmark>" on that issue's or proposal's timeline, the hash linking to the change; other repositories' `owner/name#12` are left alone. |
|
| 417 | 417 | ||
| 418 | 418 | Import from GitHub or GitLab is a 1.x item. |
|
| 419 | 419 | ||
| … | 333 unchanged lines |