Delete bookmarks: a button for writers, pushed deletions checked e271a37b629a
The bookmarks page gains a delete button for writers on every bookmark but the default. It does what oxy push --delete does, through the same checks (so it is refused under a trust policy), and is logged on the pushes page and sent to webhooks as a push. The hub tells the protocol server its default bookmark, so a pushed deletion of it is refused whatever the repository's own config says. Pinned to the oxygen that adds deletion. closes #24
- Author
- Andy McCall @andymccall <[email protected]>
- When
- · 2026-09-28 22:41 UTC
- Signature
- ✔ signed by Andy McCall
- Hash
e271a37b629a7c18c4409e2e7333342b2f189316e2da1fdf7c2db6aec818472c- Depends on
- 19bb31ff1f5c 380996db84e5 3d1e74cdac76 5e9dbc57bc31 9008bf6c2b8c 93be9ce4653e 9ff27da517fe b64e2537d702 d23464c0ac60
- Depended on by
- 351cf57e5608 a7aa76ac52d7 e5b97ac399e7 c8684d44d549 66fd9d6b1452 4f6925337848 760a7dd30d6f 9a065bc0ce33
- In bookmarks
- api-unknown-fields cancel-landed-runs clear-waiting close-proposal-140 diffusion-pin-bookmark draft-ready image-check job-waits main oh-comment-body-file proposal-list-fast run-one-job wiki-says-so
- In tags
- v0.1.0 v0.2.0 v0.3.0 v0.3.1 v0.4.0 v0.4.1 v0.4.2 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
- 15 atoms, 0 tree operations, 7 files
Browse the tree at this change
modified crates/hub/src/protocol.rs +5 −0
| … | 156 unchanged lines |
||
| 157 | 157 | } |
|
| 158 | 158 | } |
|
| 159 | 159 | ||
| 160 | + | fn default_bookmark(&self, name: &str) -> Option<String> { |
|
| 161 | + | let parsed = RepoName::parse(name)?; |
|
| 162 | + | self.row(&parsed).ok().map(|r| r.default_bookmark) |
|
| 163 | + | } |
|
| 164 | + | ||
| 160 | 165 | fn after_push(&self, name: &str, report: &PushReport, headers: &HeaderMap) { |
|
| 161 | 166 | let Some(parsed) = RepoName::parse(name) else { return }; |
|
| 162 | 167 | let Ok(row) = self.row(&parsed) else { return }; |
|
| … | 94 unchanged lines |
modified crates/hub/src/web/mod.rs +1 −0
| … | 107 unchanged lines |
||
| 108 | 108 | .route("/{owner}/{name}/compare", get(history::compare_index)) |
|
| 109 | 109 | .route("/{owner}/{name}/compare/{spec}", get(history::compare)) |
|
| 110 | 110 | .route("/{owner}/{name}/bookmarks", get(refs::bookmarks)) |
|
| 111 | + | .route("/{owner}/{name}/bookmarks/delete", post(refs::delete_bookmark)) |
|
| 111 | 112 | .route("/{owner}/{name}/tags", get(refs::tags)) |
|
| 112 | 113 | .route("/{owner}/{name}/conflicts", get(refs::conflicts)) |
|
| 113 | 114 | .route("/{owner}/{name}/trust", get(refs::trust)) |
|
| … | 464 unchanged lines |
modified crates/hub/src/web/refs.rs +109 −7
| 1 | 1 | //! Bookmarks, tags, conflicts, trust and pushes pages. |
|
| 2 | 2 | ||
| 3 | - | use super::{filters, lookup, Error, Page, RepoShell, Shell}; |
|
| 4 | - | use crate::auth::Viewer; |
|
| 3 | + | use super::settings::{csrf, require}; |
|
| 4 | + | use super::{filters, lookup, lookup_role, Error, Page, RepoShell, Shell}; |
|
| 5 | + | use crate::auth::{Access, Role, Viewer}; |
|
| 5 | 6 | use crate::repos::view; |
|
| 6 | 7 | use crate::{db, Shared}; |
|
| 7 | 8 | use askama::Template; |
|
| 8 | - | use axum::extract::{Path, State}; |
|
| 9 | + | use axum::extract::{Form, Path, State}; |
|
| 10 | + | use serde::Deserialize; |
|
| 9 | 11 | ||
| 10 | 12 | #[derive(Template)] |
|
| 11 | 13 | #[template(path = "bookmarks.html")] |
|
| … | 1 unchanged lines |
||
| 13 | 15 | shell: Shell, |
|
| 14 | 16 | repo: RepoShell, |
|
| 15 | 17 | bookmarks: Vec<view::BookmarkView>, |
|
| 18 | + | /// Whether the viewer may delete bookmarks (write access). |
|
| 19 | + | can_delete: bool, |
|
| 20 | + | notice: Option<String>, |
|
| 21 | + | error: Option<String>, |
|
| 16 | 22 | } |
|
| 17 | 23 | ||
| 24 | + | async fn bookmarks_page( |
|
| 25 | + | app: &Shared, |
|
| 26 | + | shell: Shell, |
|
| 27 | + | repo: crate::repos::RepoName, |
|
| 28 | + | row: db::repos::RepoRow, |
|
| 29 | + | role: Role, |
|
| 30 | + | notice: Option<String>, |
|
| 31 | + | error: Option<String>, |
|
| 32 | + | ) -> Result<Page<BookmarksPage>, Error> { |
|
| 33 | + | let default = row.default_bookmark.clone(); |
|
| 34 | + | let bookmarks = |
|
| 35 | + | app.repos.read(&repo, move |r, cache| view::bookmarks(r, cache, &default)).await.map_err(|e| Error::from_repo(shell.clone(), e))?; |
|
| 36 | + | let can_delete = shell.user.is_some() && role >= Role::Write; |
|
| 37 | + | Ok(Page(BookmarksPage { shell, repo: RepoShell::new(app, &row, "bookmarks").with_role(role), bookmarks, can_delete, notice, error })) |
|
| 38 | + | } |
|
| 39 | + | ||
| 18 | 40 | pub async fn bookmarks( |
|
| 19 | 41 | State(app): State<Shared>, |
|
| 20 | 42 | viewer: Viewer, |
|
| … | 1 unchanged lines |
||
| 22 | 44 | ) -> Result<Page<BookmarksPage>, Error> { |
|
| 23 | 45 | let shell = Shell::new(&app, &format!("/{owner}/{name}/bookmarks"), &viewer); |
|
| 24 | 46 | let (repo, row, role) = lookup(&app, &shell, &viewer, &owner, &name).await?; |
|
| 25 | - | let default = row.default_bookmark.clone(); |
|
| 26 | - | let bookmarks = |
|
| 27 | - | app.repos.read(&repo, move |r, cache| view::bookmarks(r, cache, &default)).await.map_err(|e| Error::from_repo(shell.clone(), e))?; |
|
| 28 | - | Ok(Page(BookmarksPage { shell, repo: RepoShell::new(&app, &row, "bookmarks").with_role(role), bookmarks })) |
|
| 47 | + | bookmarks_page(&app, shell, repo, row, role, None, None).await |
|
| 48 | + | } |
|
| 49 | + | ||
| 50 | + | #[derive(Deserialize)] |
|
| 51 | + | pub struct DeleteBookmarkForm { |
|
| 52 | + | #[serde(rename = "_csrf")] |
|
| 53 | + | csrf_token: String, |
|
| 54 | + | bookmark: String, |
|
| 55 | + | } |
|
| 56 | + | ||
| 57 | + | /// Delete a bookmark from the web (hub spec §6.4): what `oxy push --delete` |
|
| 58 | + | /// does, through the same checks. Never the default bookmark; refused under |
|
| 59 | + | /// a trust policy, which needs a signed attestation only a client can make. |
|
| 60 | + | pub async fn delete_bookmark( |
|
| 61 | + | State(app): State<Shared>, |
|
| 62 | + | viewer: Viewer, |
|
| 63 | + | Path((owner, name)): Path<(String, String)>, |
|
| 64 | + | Form(f): Form<DeleteBookmarkForm>, |
|
| 65 | + | ) -> Result<Page<BookmarksPage>, Error> { |
|
| 66 | + | let shell = Shell::new(&app, &format!("/{owner}/{name}/bookmarks"), &viewer); |
|
| 67 | + | let me = require(&viewer, &shell)?; |
|
| 68 | + | let (repo, row, role) = lookup_role(&app, &shell, &viewer, &owner, &name, Access::Write).await?; |
|
| 69 | + | csrf(&viewer, &f.csrf_token, &shell)?; |
|
| 70 | + | let bookmark = f.bookmark.trim().to_string(); |
|
| 71 | + | if bookmark == row.default_bookmark { |
|
| 72 | + | let msg = format!("{bookmark} is the default bookmark; choose another default in Settings first."); |
|
| 73 | + | return bookmarks_page(&app, shell, repo, row, role, None, Some(msg)).await; |
|
| 74 | + | } |
|
| 75 | + | let target = bookmark.clone(); |
|
| 76 | + | let outcome = app |
|
| 77 | + | .repos |
|
| 78 | + | .write(&repo, move |r, _| { |
|
| 79 | + | let Some(state) = r.view()?.bookmarks.get(&target).copied() else { |
|
| 80 | + | return Ok(Err(format!("There is no bookmark {target}."))); |
|
| 81 | + | }; |
|
| 82 | + | let update = oxy_core::protocol::RefUpdate { |
|
| 83 | + | name: target.clone(), |
|
| 84 | + | old_state: state, |
|
| 85 | + | new_state: oxy_core::hash::ObjectRef::ZERO, |
|
| 86 | + | force: true, |
|
| 87 | + | attestation: None, |
|
| 88 | + | }; |
|
| 89 | + | let report = r.receive_push_report(Default::default(), vec![update], None, vec![])?; |
|
| 90 | + | Ok(match report.results.first() { |
|
| 91 | + | Some(res) if res.ok => Ok(state.to_hex()), |
|
| 92 | + | Some(res) => Err(res.reason.clone()), |
|
| 93 | + | None => Err("nothing happened".into()), |
|
| 94 | + | }) |
|
| 95 | + | }) |
|
| 96 | + | .await |
|
| 97 | + | .map_err(|e| Error::from_repo(shell.clone(), e))?; |
|
| 98 | + | let old_state = match outcome { |
|
| 99 | + | Ok(s) => s, |
|
| 100 | + | Err(msg) => return bookmarks_page(&app, shell, repo, row, role, None, Some(format!("Could not delete {bookmark}: {msg}"))).await, |
|
| 101 | + | }; |
|
| 102 | + | let result = db::repos::PushOutcome { |
|
| 103 | + | reference: bookmark.clone(), |
|
| 104 | + | ok: true, |
|
| 105 | + | reason: "deleted on the web".into(), |
|
| 106 | + | old_state: old_state.clone(), |
|
| 107 | + | new_state: oxy_core::hash::ObjectRef::ZERO.to_hex(), |
|
| 108 | + | heads: 0, |
|
| 109 | + | changes: 0, |
|
| 110 | + | }; |
|
| 111 | + | db::repos::record_push(&app.db, row.id, Some((me.account.id, &me.account.username)), &[result]).await.ok(); |
|
| 112 | + | crate::hooks::emit( |
|
| 113 | + | &app, |
|
| 114 | + | &row, |
|
| 115 | + | "push", |
|
| 116 | + | &me.account.username, |
|
| 117 | + | serde_json::json!({ "updates": [{ "reference": bookmark, "old_state": old_state, "new_state": oxy_core::hash::ObjectRef::ZERO.to_hex(), "heads": 0, "changes": 0, "deleted": true }], "via": "web" }), |
|
| 118 | + | ) |
|
| 119 | + | .await; |
|
| 120 | + | db::accounts::audit(&app.db, Some(&me.account), "bookmark.deleted", &format!("{owner}/{name}"), &bookmark).await.ok(); |
|
| 121 | + | bookmarks_page( |
|
| 122 | + | &app, |
|
| 123 | + | shell, |
|
| 124 | + | repo, |
|
| 125 | + | row, |
|
| 126 | + | role, |
|
| 127 | + | Some(format!("Deleted {bookmark}. Its changes stay while anything else reaches them.")), |
|
| 128 | + | None, |
|
| 129 | + | ) |
|
| 130 | + | .await |
|
| 29 | 131 | } |
|
| 30 | 132 | ||
| 31 | 133 | #[derive(Template)] |
|
| … | 71 unchanged lines |
modified crates/hub/tests/api.rs +88 −0
| … | 756 unchanged lines |
||
| 757 | 757 | assert_eq!(row().await.flow, ""); |
|
| 758 | 758 | assert!(set("sometimes").await.body.contains("Pick on, off")); |
|
| 759 | 759 | } |
|
| 760 | + | ||
| 761 | + | /// Deleting bookmarks (hub spec §6.4): pushed deletions and the delete |
|
| 762 | + | /// button, never the repository's default bookmark as the hub sets it. |
|
| 763 | + | #[tokio::test(flavor = "multi_thread")] |
|
| 764 | + | async fn bookmarks_are_deleted_by_push_and_on_the_web() { |
|
| 765 | + | let h = hub().await; |
|
| 766 | + | let app = &h.app; |
|
| 767 | + | let andy = call(app, "POST", "/register", None, None, None, Some("username=andy&email=a%40example.com&password=correct-horse-battery")) |
|
| 768 | + | .await |
|
| 769 | + | .cookie |
|
| 770 | + | .unwrap(); |
|
| 771 | + | let page = call(app, "GET", "/new", None, Some(&andy), None, None).await; |
|
| 772 | + | call(app, "POST", "/new", None, Some(&andy), None, Some(&format!("_csrf={}&owner=andy&name=demo", csrf_of(&page.body)))).await; |
|
| 773 | + | let url = format!("{}/andy/demo", h.base); |
|
| 774 | + | let work = tempfile::tempdir().unwrap(); |
|
| 775 | + | let (w, u) = (work.path().to_path_buf(), url.clone()); |
|
| 776 | + | let client = tokio::task::spawn_blocking(move || { |
|
| 777 | + | let mut c = Repo::init(&w).unwrap(); |
|
| 778 | + | c.config.set("remote.origin.url", &u).unwrap(); |
|
| 779 | + | c.config.set("remote.origin.token", "site").unwrap(); |
|
| 780 | + | c.save_config().unwrap(); |
|
| 781 | + | write(&w, "a.txt", b"a\n"); |
|
| 782 | + | c.commit("a", &[]).unwrap(); |
|
| 783 | + | assert!(c.push("origin", false).unwrap()[0].ok); |
|
| 784 | + | for b in ["develop", "feature/x", "stray"] { |
|
| 785 | + | c.bookmark_create(b, None).unwrap(); |
|
| 786 | + | assert!(c.push_bookmark("origin", b, false).unwrap()[0].ok); |
|
| 787 | + | } |
|
| 788 | + | // A pushed deletion. |
|
| 789 | + | let r = c.push_delete("origin", "feature/x").unwrap(); |
|
| 790 | + | assert!(r[0].ok, "{r:?}"); |
|
| 791 | + | c |
|
| 792 | + | }) |
|
| 793 | + | .await |
|
| 794 | + | .unwrap(); |
|
| 795 | + | let page = call(app, "GET", "/andy/demo/bookmarks", None, Some(&andy), None, None).await; |
|
| 796 | + | assert!(!page.body.contains(">feature/x<") && page.body.contains(">stray<"), "{}", page.body); |
|
| 797 | + | assert!(page.body.contains("/andy/demo/bookmarks/delete"), "writers see delete buttons"); |
|
| 798 | + | let anon = call(app, "GET", "/andy/demo/bookmarks", None, None, None, None).await; |
|
| 799 | + | assert!(!anon.body.contains("/bookmarks/delete")); |
|
| 800 | + | ||
| 801 | + | // The hub's default, not the repository's config, is protected: make |
|
| 802 | + | // develop the default, then main can go and develop cannot. |
|
| 803 | + | let row = oxygenhub::db::repos::get(&app.db, "andy", "demo").await.unwrap().unwrap(); |
|
| 804 | + | oxygenhub::db::repos::set_default_bookmark(&app.db, row.id, "develop").await.unwrap(); |
|
| 805 | + | let client = tokio::task::spawn_blocking(move || { |
|
| 806 | + | let r = client.push_delete("origin", "develop").unwrap(); |
|
| 807 | + | assert!(!r[0].ok && r[0].reason.contains("default bookmark"), "{r:?}"); |
|
| 808 | + | let r = client.push_delete("origin", "main").unwrap(); |
|
| 809 | + | assert!(r[0].ok, "{r:?}"); |
|
| 810 | + | client |
|
| 811 | + | }) |
|
| 812 | + | .await |
|
| 813 | + | .unwrap(); |
|
| 814 | + | drop(client); |
|
| 815 | + | ||
| 816 | + | // On the web: a writer deletes stray; the default is refused. |
|
| 817 | + | let form = |b: &'static str| { |
|
| 818 | + | let (app, andy) = (app.clone(), andy.clone()); |
|
| 819 | + | async move { |
|
| 820 | + | let page = call(&app, "GET", "/andy/demo/bookmarks", None, Some(&andy), None, None).await; |
|
| 821 | + | call( |
|
| 822 | + | &app, |
|
| 823 | + | "POST", |
|
| 824 | + | "/andy/demo/bookmarks/delete", |
|
| 825 | + | None, |
|
| 826 | + | Some(&andy), |
|
| 827 | + | None, |
|
| 828 | + | Some(&format!("_csrf={}&bookmark={b}", csrf_of(&page.body))), |
|
| 829 | + | ) |
|
| 830 | + | .await |
|
| 831 | + | } |
|
| 832 | + | }; |
|
| 833 | + | let r = form("stray").await; |
|
| 834 | + | assert!(r.body.contains("Deleted stray"), "{}", r.body); |
|
| 835 | + | assert!(!r.body.contains(">stray<")); |
|
| 836 | + | let r = form("develop").await; |
|
| 837 | + | assert!(r.body.contains("is the default bookmark"), "{}", r.body); |
|
| 838 | + | let r = form("nope").await; |
|
| 839 | + | assert!(r.body.contains("There is no bookmark nope"), "{}", r.body); |
|
| 840 | + | let pushes = call(app, "GET", "/andy/demo/pushes", None, Some(&andy), None, None).await; |
|
| 841 | + | assert!(pushes.body.contains("deleted on the web"), "logged like a push"); |
|
| 842 | + | // Anonymous visitors cannot. |
|
| 843 | + | assert_ne!( |
|
| 844 | + | call(app, "POST", "/andy/demo/bookmarks/delete", None, None, None, Some("_csrf=x&bookmark=develop")).await.status, |
|
| 845 | + | StatusCode::OK |
|
| 846 | + | ); |
|
| 847 | + | } |
replaced deps/oxygenflow +0 −0
No line changes.
modified docs/spec.md +1 −1
| … | 244 unchanged lines |
||
| 245 | 245 | | `/{owner}/{name}/changes/{ref}` | History (§6.7) | |
|
| 246 | 246 | | `/{owner}/{name}/change/{hash}` | Change page (§6.8) | |
|
| 247 | 247 | | `/{owner}/{name}/compare/{a}...{b}` | Changes in `b` not in `a`, and the diff between the two states. Each side is picked from the bookmarks and tags, or typed as a change or state; a swap link reverses them. When both are bookmarks, `b` has changes `a` lacks and the viewer can write, **Propose this** opens a new proposal of `b` into `a` (or links the one already open). `/compare` alone compares the most recently changed other bookmark against the default one; `/compare?base=…&head=…` is the pickers' form | |
|
| 248 | - | | `/{owner}/{name}/bookmarks` | Bookmarks with heads, ahead/behind the default, last update, attestation status | |
|
| 248 | + | | `/{owner}/{name}/bookmarks` | Bookmarks with heads, ahead/behind the default, last update, attestation status. Writers delete any bookmark but the default one: what `oxy push --delete` does, through the same checks (refused under a trust policy, which needs a signed attestation only a client can make), logged on the pushes page and sent to webhooks as a push. A pushed deletion of the default bookmark (the hub's setting, not the repository's config) is refused | |
|
| 249 | 249 | | `/{owner}/{name}/conflicts` | Every bookmark whose state carries conflicts, each with its regions (§6.10) | |
|
| 250 | 250 | | `/{owner}/{name}/proposals`, `/proposal/{n}` | §7. The list filters by state and by `q` (`author:`, `label:`, `target:` and words), sorts by recently updated (default), newest or oldest, and pages 25 at a time | |
|
| 251 | 251 | | `/{owner}/{name}/issues`, `/issue/{n}` | §8. The list filters by state and by `q` (`author:`, `label:`, `assignee:`, `milestone:` and words), sorts by newest (pinned first, default), oldest, recently updated or most comments, and pages 25 at a time. The Proposals and Issues tabs show how many are open | |
|
| … | 326 unchanged lines |
modified templates/bookmarks.html +4 −1
| … | 4 unchanged lines |
||
| 5 | 5 | {% block tabs %}{% call m::tabs(repo, "bookmarks") %}{% endblock %} |
|
| 6 | 6 | {% block content %} |
|
| 7 | 7 | <div class="head"><h1>Bookmarks <small>{{ bookmarks.len() }}</small></h1></div> |
|
| 8 | + | {% if let Some(n) = notice %}<p class="note">{{ n }}</p>{% endif %} |
|
| 9 | + | {% if let Some(e) = error %}<p class="error">{{ e }}</p>{% endif %} |
|
| 8 | 10 | <div class="table"><table> |
|
| 9 | 11 | <tr><th>Bookmark</th><th>Heads</th><th>Changes</th><th>Latest</th><th></th></tr> |
|
| 10 | 12 | {% for b in bookmarks %} |
|
| … | 2 unchanged lines |
||
| 13 | 15 | <td>{% for h in b.heads %}<a class="chip" href="{{ repo.url }}/change/{{ h }}">{{ h|short }}</a> {% endfor %}{% if b.heads.len() > 1 %}<span class="tag warn">{{ b.heads.len() }} heads</span>{% endif %}</td> |
|
| 14 | 16 | <td>{{ b.changes }}</td> |
|
| 15 | 17 | <td><time title="{{ b.latest_ms|when }}">{{ b.latest_ms|ago }}</time></td> |
|
| 16 | - | <td class="small"><a href="{{ repo.url }}/changes/{{ b.name }}">history</a>{% if !b.default %} · <a href="{{ repo.url }}/compare/{{ repo.default_bookmark }}...{{ b.name }}">compare with {{ repo.default_bookmark }}</a>{% endif %}</td> |
|
| 18 | + | <td class="small"><a href="{{ repo.url }}/changes/{{ b.name }}">history</a>{% if !b.default %} · <a href="{{ repo.url }}/compare/{{ repo.default_bookmark }}...{{ b.name }}">compare with {{ repo.default_bookmark }}</a>{% if can_delete %} |
|
| 19 | + | <form class="inline" method="post" action="{{ repo.url }}/bookmarks/delete"><input type="hidden" name="_csrf" value="{{ shell.csrf }}"><input type="hidden" name="bookmark" value="{{ b.name }}"><button type="submit" class="small danger-btn" onclick="return confirm('Delete the bookmark {{ b.name }}? Its changes stay while anything else reaches them.')">DELETE</button></form>{% endif %}{% endif %}</td> |
|
| 17 | 20 | </tr> |
|
| 18 | 21 | {% endfor %} |
|
| 19 | 22 | </table></div> |
|
| … | 1 unchanged lines |