Close and reopen a proposal through the API and oh 71e65fcf95ab
PATCH /proposals/{n} takes {"state": "closed"} or {"state": "open"},
by the proposal's author or anyone with triage access, as the page's Close
and Reopen buttons do; oh proposal close N [-m MSG] and oh proposal
reopen N use it. Proposals whose work reaches the target by a push are
already marked landed (#141).
closes #140
- Author
- Andy McCall @andymccall <[email protected]>
- When
- · 2026-10-13 07:02 UTC
- Signature
- ✔ signed by Andy McCall
- Hash
71e65fcf95ab37246392ea0013ba4b53be8a9b804b5887bd555b5cecb10a41fa- Depends on
- 1f1eb06a9009 2291f8cc9c96 4d2e44d79b03 5c6e92fd8d71 62ceb2eec18c 739a763a558c 76f262e1a666 7d2eb590051f 93be9ce4653e ac713df7b38b b64e2537d702 bf1ab3458db1 d23464c0ac60 de505df5cf8d ef20fdc14e8b
- Depended on by
- 041a945f55f8 09ffa7b600a9 2a8b621676b4 d943160dafeb acc85ac0f70b
- In bookmarks
- api-unknown-fields cancel-landed-runs close-proposal-140 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
- 13 atoms, 0 tree operations, 7 files
Browse the tree at this change
modified crates/hub-api/src/lib.rs +4 −0
| … | 933 unchanged lines |
||
| 934 | 934 | /// The whole set of labels, by name (triage); oxygenhub#80. |
|
| 935 | 935 | #[serde(default, skip_serializing_if = "Option::is_none")] |
|
| 936 | 936 | pub labels: Option<Vec<String>>, |
|
| 937 | + | /// `closed` or `open` (reopen), by its author or with triage access, |
|
| 938 | + | /// as the page's Close and Reopen do (oxygenhub#140). |
|
| 939 | + | #[serde(default, skip_serializing_if = "Option::is_none")] |
|
| 940 | + | pub state: Option<String>, |
|
| 937 | 941 | } |
|
| 938 | 942 | ||
| 939 | 943 | /// `PATCH /repos/{owner}/{name}/releases/{tag}`: what to change; fields |
|
| … | 204 unchanged lines |
modified crates/hub-api/ts/index.d.ts +3 −0
| … | 390 unchanged lines |
||
| 391 | 391 | export type ProposalEdit = { |
|
| 392 | 392 | body?: string | null; |
|
| 393 | 393 | labels?: string[] | null; |
|
| 394 | + | state?: string | null; |
|
| 394 | 395 | title?: string | null; |
|
| 395 | 396 | }; |
|
| 396 | 397 | ||
| … | 46 unchanged lines |
||
| 443 | 444 | export type ProjectEdit = { |
|
| 444 | 445 | closed?: boolean | null; |
|
| 445 | 446 | description?: string | null; |
|
| 447 | + | owner?: string | null; |
|
| 446 | 448 | private?: boolean | null; |
|
| 447 | 449 | title?: string | null; |
|
| 448 | 450 | }; |
|
| … | 89 unchanged lines |
||
| 538 | 540 | finished_ms?: number | null; |
|
| 539 | 541 | log_lines: number; |
|
| 540 | 542 | name: string; |
|
| 543 | + | placed?: string; |
|
| 541 | 544 | runner: string; |
|
| 542 | 545 | started_ms?: number | null; |
|
| 543 | 546 | status: string; |
|
| … | 34 unchanged lines |
modified crates/hub/src/api/mod.rs +22 −0
| … | 906 unchanged lines |
||
| 907 | 907 | let names: Vec<&str> = want.iter().map(|l| l.name.as_str()).collect(); |
|
| 908 | 908 | db::proposals::add_event(&app.db, p.id, &me.username, "set labels", &names.join(", ")).await.ok(); |
|
| 909 | 909 | } |
|
| 910 | + | if let Some(want) = body.state.as_deref() { |
|
| 911 | + | if Some(me.id) != p.author_id && role < Role::Triage { |
|
| 912 | + | return Err(ApiError( |
|
| 913 | + | StatusCode::FORBIDDEN, |
|
| 914 | + | "only the proposal's author or someone with triage access can close or reopen it".into(), |
|
| 915 | + | )); |
|
| 916 | + | } |
|
| 917 | + | let new_state = match (want, p.state.as_str()) { |
|
| 918 | + | ("closed", "open" | "draft") => "closed", |
|
| 919 | + | ("open", "closed") => "open", |
|
| 920 | + | (w, now) if w == now => now, |
|
| 921 | + | ("closed" | "open", now) => { |
|
| 922 | + | return Err(ApiError(StatusCode::CONFLICT, format!("a proposal that is {now} cannot become {want}"))) |
|
| 923 | + | } |
|
| 924 | + | _ => return Err(ApiError(StatusCode::BAD_REQUEST, "state is closed or open".into())), |
|
| 925 | + | }; |
|
| 926 | + | if new_state != p.state { |
|
| 927 | + | db::proposals::set_state(&app.db, p.id, new_state, "").await?; |
|
| 928 | + | db::proposals::add_event(&app.db, p.id, &me.username, new_state, "").await.ok(); |
|
| 929 | + | crate::hooks::emit(&app, &row, &format!("proposal.{new_state}"), &me.username, serde_json::json!({ "number": number })).await; |
|
| 930 | + | } |
|
| 931 | + | } |
|
| 910 | 932 | let p = db::proposals::get(&app.db, row.id, number).await?.ok_or_else(|| not_found("no such proposal"))?; |
|
| 911 | 933 | let source_url = match p.source_repo_id { |
|
| 912 | 934 | Some(id) => db::repos::get_by_id(&app.db, id).await?.map(|r| format!("{}/{}", app.config.base_url(), r.slug())), |
|
| … | 1448 unchanged lines |
modified crates/hub/tests/proposals.rs +55 −0
| … | 544 unchanged lines |
||
| 545 | 545 | } |
|
| 546 | 546 | assert_eq!((p.state.as_str(), p.landed_via.as_str()), ("landed", "push"), "{p:?}"); |
|
| 547 | 547 | } |
|
| 548 | + | ||
| 549 | + | /// #140: a proposal is closed and reopened through the API, as the page's |
|
| 550 | + | /// buttons do, by its author or someone with triage access. |
|
| 551 | + | #[tokio::test(flavor = "multi_thread")] |
|
| 552 | + | async fn a_proposal_is_closed_and_reopened_through_the_api() { |
|
| 553 | + | let h = hub().await; |
|
| 554 | + | let app = &h.app; |
|
| 555 | + | let andy = register(app, "andy").await; |
|
| 556 | + | let repo = RepoName::parse("andy/demo").unwrap(); |
|
| 557 | + | app.repos.create(&repo, None).unwrap(); |
|
| 558 | + | db::repos::create(&app.db, "andy", "demo", "", hub_api::Visibility::Public).await.unwrap(); |
|
| 559 | + | let work = tempfile::tempdir().unwrap(); |
|
| 560 | + | let mut client = Repo::init(work.path()).unwrap(); |
|
| 561 | + | client.create_identity("Andy McCall", "[email protected]", oxy_core::repo::IdentityScope::User).unwrap(); |
|
| 562 | + | client.config.set("remote.origin.url", &format!("{}/andy/demo", h.base)).unwrap(); |
|
| 563 | + | client.config.set("remote.origin.token", "site").unwrap(); |
|
| 564 | + | client.save_config().unwrap(); |
|
| 565 | + | write(work.path(), "a.txt", b"one\n"); |
|
| 566 | + | client.snapshot("t").unwrap(); |
|
| 567 | + | let base = client.commit("base", &[]).unwrap(); |
|
| 568 | + | assert!(client.push("origin", false).unwrap().iter().all(|r| r.ok)); |
|
| 569 | + | client.bookmark_create("feature", Some(&[base])).unwrap(); |
|
| 570 | + | client.bookmark_switch("feature", false).unwrap(); |
|
| 571 | + | write(work.path(), "a.txt", b"one\ntwo\n"); |
|
| 572 | + | client.snapshot("t").unwrap(); |
|
| 573 | + | client.commit("add two", &[]).unwrap(); |
|
| 574 | + | assert!(client.push_bookmark("origin", "feature", false).unwrap().iter().all(|r| r.ok)); |
|
| 575 | + | let preview = get(app, "/andy/demo/proposals/new?source=feature&target=main", Some(&andy)).await; |
|
| 576 | + | let csrf = csrf_of(&preview.body); |
|
| 577 | + | let r = post(app, "/andy/demo/proposals/new", Some(&andy), &format!("_csrf={csrf}&title=Two&body=&target=main&source=feature")).await; |
|
| 578 | + | assert_eq!(r.status, StatusCode::SEE_OTHER, "{}", r.body); |
|
| 579 | + | ||
| 580 | + | let patch = |json: &'static str, bearer: Option<&'static str>| async move { |
|
| 581 | + | let mut req = |
|
| 582 | + | Request::builder().method("PATCH").uri("/api/v1/repos/andy/demo/proposals/1").header(header::CONTENT_TYPE, "application/json"); |
|
| 583 | + | if let Some(b) = bearer { |
|
| 584 | + | req = req.header(header::AUTHORIZATION, format!("Bearer {b}")); |
|
| 585 | + | } |
|
| 586 | + | let res = oxygenhub::router(app.clone()).oneshot(req.body(Body::from(json)).unwrap()).await.unwrap(); |
|
| 587 | + | let st = res.status(); |
|
| 588 | + | (st, String::from_utf8_lossy(&res.into_body().collect().await.unwrap().to_bytes()).into_owned()) |
|
| 589 | + | }; |
|
| 590 | + | let state = || async { db::proposals::get(&app.db, 1, 1).await.unwrap().unwrap().state }; |
|
| 591 | + | // The site token acts for the first administrator, andy. |
|
| 592 | + | let (st, body) = patch(r#"{"state":"closed"}"#, Some("site")).await; |
|
| 593 | + | assert_eq!(st, StatusCode::OK, "{body}"); |
|
| 594 | + | assert_eq!(state().await, "closed"); |
|
| 595 | + | assert_eq!(patch(r#"{"state":"closed"}"#, Some("site")).await.0, StatusCode::OK, "closing a closed proposal changes nothing"); |
|
| 596 | + | let (st, _) = patch(r#"{"state":"open"}"#, Some("site")).await; |
|
| 597 | + | assert_eq!(st, StatusCode::OK); |
|
| 598 | + | assert_eq!(state().await, "open"); |
|
| 599 | + | assert_eq!(patch(r#"{"state":"landed"}"#, Some("site")).await.0, StatusCode::BAD_REQUEST, "landing has its own endpoint"); |
|
| 600 | + | assert_ne!(patch(r#"{"state":"closed"}"#, None).await.0, StatusCode::OK, "not without a token"); |
|
| 601 | + | assert_eq!(state().await, "open"); |
|
| 602 | + | } |
modified crates/oh/src/main.rs +29 −1
| … | 248 unchanged lines |
||
| 249 | 249 | Checks { number: i64 }, |
|
| 250 | 250 | /// Land a proposal on its target, as the page's Land button does |
|
| 251 | 251 | Land { number: i64 }, |
|
| 252 | + | /// Close a proposal without landing it, with an optional comment |
|
| 253 | + | Close { |
|
| 254 | + | number: i64, |
|
| 255 | + | /// Say why, as a comment before it closes |
|
| 256 | + | #[arg(long, short = 'm')] |
|
| 257 | + | message: Option<String>, |
|
| 258 | + | }, |
|
| 259 | + | /// Reopen a closed proposal |
|
| 260 | + | Reopen { number: i64 }, |
|
| 252 | 261 | } |
|
| 253 | 262 | ||
| 254 | 263 | #[derive(Subcommand)] |
|
| … | 563 unchanged lines |
||
| 818 | 827 | if title.is_none() && body.is_none() { |
|
| 819 | 828 | bail!("say what to change: --title, --body or --body-file"); |
|
| 820 | 829 | } |
|
| 821 | - | let edit = hub_api::ProposalEdit { title, body, labels: None }; |
|
| 830 | + | let edit = hub_api::ProposalEdit { title, body, labels: None, state: None }; |
|
| 822 | 831 | let p: Proposal = c.send("PATCH", &format!("{base}/proposals/{number}"), &edit)?; |
|
| 823 | 832 | if json { |
|
| 824 | 833 | println!("{}", serde_json::to_string(&p)?); |
|
| … | 27 unchanged lines |
||
| 852 | 861 | println!("{}", serde_json::to_string(&p)?); |
|
| 853 | 862 | } else { |
|
| 854 | 863 | println!("{} landed #{} on {}", ui::ok("✔"), p.number, p.target); |
|
| 864 | + | } |
|
| 865 | + | } |
|
| 866 | + | ProposalCmd::Close { number, message } => { |
|
| 867 | + | if let Some(body) = message { |
|
| 868 | + | c.send_text("POST", &format!("{base}/proposals/{number}/comments"), &NewComment { body, path: None, line: None })?; |
|
| 869 | + | } |
|
| 870 | + | let p: Proposal = c.send("PATCH", &format!("{base}/proposals/{number}"), &serde_json::json!({ "state": "closed" }))?; |
|
| 871 | + | if json { |
|
| 872 | + | println!("{}", serde_json::to_string(&p)?); |
|
| 873 | + | } else { |
|
| 874 | + | println!("{} closed #{} {}", ui::ok("✔"), p.number, p.title); |
|
| 875 | + | } |
|
| 876 | + | } |
|
| 877 | + | ProposalCmd::Reopen { number } => { |
|
| 878 | + | let p: Proposal = c.send("PATCH", &format!("{base}/proposals/{number}"), &serde_json::json!({ "state": "open" }))?; |
|
| 879 | + | if json { |
|
| 880 | + | println!("{}", serde_json::to_string(&p)?); |
|
| 881 | + | } else { |
|
| 882 | + | println!("{} reopened #{} {}", ui::ok("✔"), p.number, p.title); |
|
| 855 | 883 | } |
|
| 856 | 884 | } |
|
| 857 | 885 | ProposalCmd::Checks { number } => { |
|
| … | 414 unchanged lines |
modified docs/oh.md +2 −0
| … | 46 unchanged lines |
||
| 47 | 47 | | `oh proposal create -t TITLE --from BOOKMARK [--into BOOKMARK] [--draft]` | Propose a bookmark's changes | |
|
| 48 | 48 | | `oh proposal checks N` | Checks on the projected state | |
|
| 49 | 49 | | `oh proposal land N` | Land it on its target, as the page's Land button does | |
|
| 50 | + | | `oh proposal close N [-m MSG]` | Close it without landing, with an optional comment saying why | |
|
| 51 | + | | `oh proposal reopen N` | Reopen a closed proposal | |
|
| 50 | 52 | | `oh run list`, `oh run view N` | OxygenFlow runs and their jobs | |
|
| 51 | 53 | | `oh run start [--bookmark B \| --change HASH] [--watch]` | Run the pipeline without a push | |
|
| 52 | 54 | | `oh run watch N` | Follow a run; exit 0 if it passes, 1 if not | |
|
| … | 14 unchanged lines |
modified docs/spec.md +1 −1
| … | 362 unchanged lines |
||
| 363 | 363 | ||
| 364 | 364 | ### 7.4 Reviews and approvals |
|
| 365 | 365 | ||
| 366 | - | The author or a writer can **ask for a review** from anyone who can read the repository (not the author), under Reviewers on the proposal page or with `POST /api/v1/repos/{o}/{n}/proposals/{number}/reviewers` and `{"reviewers": [...]}`; `DELETE .../reviewers/{username}` withdraws a request. Each person asked gets a notification (`proposal.review_requested`), the timeline records it, the proposal's JSON lists `requested_reviewers`, and a review from that person answers the request. Every review notifies the proposal's author (`proposal.review`). An AI assistant the repository may use (§13.4) can be asked the same way; its answer is a `comment` review recorded via the assistant under the asker's name. |
|
| 366 | + | Its author, or anyone with triage access, closes and reopens a proposal on its page or with `PATCH /api/v1/repos/{o}/{n}/proposals/{number}` and `{"state": "closed"}` or `{"state": "open"}` (`oh proposal close N [-m MSG]`, `oh proposal reopen N`; #140); asking for the state it already has changes nothing, and landing has its own endpoint. The author or a writer can **ask for a review** from anyone who can read the repository (not the author), under Reviewers on the proposal page or with `POST /api/v1/repos/{o}/{n}/proposals/{number}/reviewers` and `{"reviewers": [...]}`; `DELETE .../reviewers/{username}` withdraws a request. Each person asked gets a notification (`proposal.review_requested`), the timeline records it, the proposal's JSON lists `requested_reviewers`, and a review from that person answers the request. Every review notifies the proposal's author (`proposal.review`). An AI assistant the repository may use (§13.4) can be asked the same way; its answer is a `comment` review recorded via the assistant under the asker's name. |
|
| 367 | 367 | ||
| 368 | 368 | A review is `comment`, `request changes`, or `approve`. An approval MUST be backed by a **seal attestation** (spec §14.4, `kind = seal`) over the proposal's change hashes, signed with a key linked to the reviewer's account. Private keys never enter the browser, so the reviewer signs from a client: `oxy seal --proposal <url>` or `oxy seal <hashes>` (§18, a new command; the seal object already exists in the model) signs and pushes the seal; the hub matches the seal to the proposal and records the approval. The page shows the command ready to copy. An approval not yet backed by a seal is shown as "approval pending signature" and does not count towards requirements. |
|
| 369 | 369 | ||
| … | 349 unchanged lines |