chadtmiller.com/knotless
git that lives in your Atmosphere account
1
10
The pages set a pull to merge when its checks pass
Only what this pull adds. Pulls can be set to merge by themselves once their checks pass landed as f319561, so its changes do not show here again.
.changeset/pages-merge-when-checks-pass.mdnew+7−0
| @@ -0,0 +1,7 @@ | ||
--- | ||
default: minor | ||
--- | ||
| ||
# The pages set a pull to merge when its checks pass | ||
| ||
A pull's merge box offers Merge when checks pass while its checks run or fail, says who set it, and cancels it. | ||
crates/knotless/src/landing.rs+51−0
| @@ -73,6 +73,46 @@ pub struct Mergeable { | ||
/// it instead, unless its owner forces it. | ||
#[serde(skip_serializing_if = "std::ops::Not::not")] | ||
pub queue_only: bool, | ||
/// Whether it is set to merge by itself once its checks pass, if it | ||
/// ever was and that was not cancelled by someone ([`AutoMerged`]). | ||
#[serde(skip_serializing_if = "Option::is_none")] | ||
pub auto_merge: Option<AutoMerged>, | ||
} | ||
| ||
/// Where a pull's merge when checks pass stands, as | ||
/// `dev.knotless.getMergeStatus#autoMerge`. | ||
#[derive(Debug, Serialize, PartialEq)] | ||
#[serde(rename_all = "camelCase")] | ||
pub struct AutoMerged { | ||
/// `set`; or `pushed` or `retargeted`, cancelled by a push from | ||
/// someone who may not merge it or by moving it to another branch. | ||
pub state: &'static str, | ||
/// Who set it, by DID. | ||
pub by: String, | ||
/// For `pushed`, the owner of the head repository pushed to, by DID. | ||
#[serde(skip_serializing_if = "Option::is_none")] | ||
pub pushed_by: Option<String>, | ||
} | ||
| ||
impl AutoMerged { | ||
/// Where `item`'s stands, its head at `head`, as | ||
/// [`crate::auto_merge::standing`] says. | ||
pub fn of(t: Facts, item: &Item, head: &str) -> Option<AutoMerged> { | ||
use crate::auto_merge::Standing; | ||
let (state, set, pushed_by) = | ||
match crate::auto_merge::standing(t.record, t.owner, item, head) { | ||
Standing::Off => return None, | ||
Standing::Set(set) => ("set", set, None), | ||
Standing::Pushed { set, by } => ("pushed", set, Some(by)), | ||
Standing::Retargeted(set) => ("retargeted", set, None), | ||
}; | ||
let by = set.by.clone(); | ||
Some(AutoMerged { | ||
state, | ||
by, | ||
pushed_by, | ||
}) | ||
} | ||
} | ||
| ||
/// A line of text, as `dev.knotless.getMergeStatus#note`. | ||
| @@ -98,6 +138,11 @@ pub struct May { | ||
/// in a repository whose runners land a queue, while the branch it | ||
/// lands on takes pulls through the queue or it is already queued. | ||
pub queue: bool, | ||
/// Set it to merge by itself once its checks pass: those who may merge | ||
/// it, in a repository that names runners, into a branch with no merge | ||
/// queue. Its author may cancel that too, as `head` says. | ||
#[serde(rename = "autoMerge")] | ||
pub auto_merge: bool, | ||
/// Change its target branch: those who may close it. | ||
pub base: bool, | ||
/// Change its head branch: its author. | ||
| @@ -122,6 +167,11 @@ pub fn may(t: Facts, items: &[Item], item: &Item, who: &str) -> May { | ||
merge: maintainer, | ||
force: who == owner, | ||
queue: maintainer && runners && on, | ||
auto_merge: maintainer | ||
&& runners | ||
&& !t | ||
.record | ||
.queue_only(item.base.as_deref().unwrap_or_default()), | ||
base: forge::allowed(t.record, owner, &item.author, who, "closed", &at), | ||
head: who == item.author, | ||
rerun: who == owner && runners, | ||
| @@ -216,6 +266,7 @@ pub async fn gauge( | ||
may: may(t, items, item, who), | ||
queue_blocked: None, | ||
queue_only: t.record.queue_only(branch), | ||
auto_merge: AutoMerged::of(t, item, head), | ||
}; | ||
let none = |found| Gauged { | ||
found, | ||
crates/knotless-mcp/tests/mcp.rs+1−1
| @@ -1596,7 +1596,7 @@ fn agents_fork_review_and_merge() { | ||
assert_eq!(checked["checks"], "none", "{checked}"); | ||
assert_eq!(checked["method"], "merge", "{checked}"); | ||
assert_eq!(checked.get("deletes"), None, "{checked}"); | ||
let may = json!({"merge": true, "force": true, "queue": true, "base": true, "head": false, "rerun": true}); | ||
let may = json!({"merge": true, "force": true, "queue": true, "autoMerge": false, "base": true, "head": false, "rerun": true}); | ||
assert_eq!(checked["may"], may); | ||
let queued = o.json( | ||
"pull_merge", | ||
crates/knotless-serve/src/web/landing.rs+44−2
| @@ -1,6 +1,6 @@ | ||
//! Landing pulls and running their checks, from the pages: | ||
//! `dev.knotless.getMergeStatus`, `mergePull`, `revertPull`, `queuePull`, | ||
//! `leaveQueue`, `retargetPull`, `requestRerun`, `listWorkflows`, | ||
//! `leaveQueue`, `setAutoMerge`, `retargetPull`, `requestRerun`, `listWorkflows`, | ||
//! `listRunners`, `addRunner` and `removeRunner`. Each write is made as the | ||
//! account logged in to the pages, with the session the server keeps for | ||
//! it ([`crate::login`]), through the same code as `knotless pull merge`, | ||
| @@ -78,7 +78,14 @@ pub async fn get_merge_status( | ||
if found.may.queue && undelegated(&app, &repo) { | ||
found.queue_blocked = Some("undelegated"); | ||
} | ||
Ok(answer(found)) | ||
// Who set it, and who pushed since, as the pages show an account. | ||
let mut said = serde_json::json!(found); | ||
for key in ["by", "pushedBy"] { | ||
if let Some(did) = said["autoMerge"][key].as_str().map(str::to_string) { | ||
said["autoMerge"][key] = serde_json::json!(super::actor::actor(&app, &did).await); | ||
} | ||
} | ||
Ok(answer(said)) | ||
} | ||
| ||
/// Whether the open pull `pull` on `repo` can merge now, as | ||
| @@ -271,6 +278,41 @@ pub async fn queue_pull( | ||
Ok(answer(place)) | ||
} | ||
| ||
#[derive(Deserialize)] | ||
pub struct AutoMergeInput { | ||
repo: String, | ||
pull: String, | ||
#[serde(default)] | ||
undo: bool, | ||
} | ||
| ||
/// `dev.knotless.setAutoMerge`: as `knotless pull merge --auto`, or with | ||
/// undo `--auto --undo`. | ||
pub async fn set_auto_merge( | ||
State(app): State<Arc<App>>, | ||
Extension(viewer): Extension<Viewer>, | ||
headers: HeaderMap, | ||
axum::Json(input): axum::Json<AutoMergeInput>, | ||
) -> Result<Response> { | ||
let act = "set a pull to merge when its checks pass"; | ||
let (ctx, repo) = acting(&app, viewer, &headers, act, &input.repo).await?; | ||
let url = url_of(&repo); | ||
let commit = match input.undo { | ||
true => knotless::auto_merge::cancel(&ctx, &url, &input.pull) | ||
.await | ||
.map(|()| None), | ||
false => knotless::auto_merge::set(&ctx, &url, &input.pull) | ||
.await | ||
.map(Some), | ||
}; | ||
let commit = commit.map_err(refused)?; | ||
// Its author's cancelling is in their repo, which freshening skips. | ||
let _ = freshen(&app, &repo).await; | ||
super::threads::reindex(&app, &repo, &input.pull).await?; | ||
let said = commit.map(|commit| serde_json::json!({"commit": commit})); | ||
Ok(answer(said.unwrap_or(serde_json::json!({})))) | ||
} | ||
| ||
/// `dev.knotless.leaveQueue`: as `knotless queue leave`. | ||
pub async fn leave_queue( | ||
State(app): State<Arc<App>>, | ||
crates/knotless-serve/src/web.rs+1−0
| @@ -245,6 +245,7 @@ fn landing_routes() -> Router<Arc<App>> { | ||
.route("/xrpc/dev.knotless.mergePull", post(merge_pull)) | ||
.route("/xrpc/dev.knotless.revertPull", post(revert_pull)) | ||
.route("/xrpc/dev.knotless.queuePull", post(queue_pull)) | ||
.route("/xrpc/dev.knotless.setAutoMerge", post(set_auto_merge)) | ||
.route("/xrpc/dev.knotless.leaveQueue", post(leave_queue)) | ||
.route("/xrpc/dev.knotless.retargetPull", post(retarget_pull)) | ||
.route("/xrpc/dev.knotless.requestRerun", post(request_rerun)) | ||
crates/knotless-serve/tests/forge.rs+6−0
| @@ -2856,6 +2856,12 @@ fn merge_when_checks_pass_is_set_by_who_may_merge() { | ||
contrib.account.handle, owner.account.handle | ||
); | ||
says(&owner, &f, &pushed); | ||
// Gauged for the pages as its push left it: cancelled. | ||
let (_, checked, _) = owner.knotless(&["pull", "merge", &url, &f, "--check"], b""); | ||
assert!( | ||
!checked.contains("merges when its checks pass"), | ||
"{checked}" | ||
); | ||
let undone = ok(&contrib, &["pull", "merge", &url, &f, "--auto", "--undo"]); | ||
assert!(undone.starts_with("cancelled"), "{undone}"); | ||
wait_until(|| !ok(&owner, &["pull", "show", &url, &f]).contains("merge when checks pass")); | ||
crates/knotless-serve/tests/gateway.rs+29−2
| @@ -1989,7 +1989,7 @@ fn landing_pulls_from_the_pages() { | ||
assert_eq!(ask("getMergeStatus", ¬hing, me).0, 404); | ||
let (code, _, said) = status(me); | ||
assert_eq!(code, 200, "{said}"); | ||
let may = json!({"merge": true, "force": true, "queue": false, "base": true, "head": true, "rerun": false}); | ||
let may = json!({"merge": true, "force": true, "queue": false, "autoMerge": false, "base": true, "head": true, "rerun": false}); | ||
assert_eq!( | ||
(&said["state"], &said["checks"], &said["method"]), | ||
(&json!("clean"), &json!("none"), &json!("merge")), | ||
| @@ -2001,7 +2001,7 @@ fn landing_pulls_from_the_pages() { | ||
); | ||
assert_eq!(said["may"], may); | ||
// Anyone else may do nothing to it. | ||
let none = json!({"merge": false, "force": false, "queue": false, "base": false, "head": false, "rerun": false}); | ||
let none = json!({"merge": false, "force": false, "queue": false, "autoMerge": false, "base": false, "head": false, "rerun": false}); | ||
let theirs = status(them).2; | ||
assert_eq!(theirs["may"], none); | ||
// And says why it would not merge for them. | ||
| @@ -2173,6 +2173,33 @@ fn landing_pulls_from_the_pages() { | ||
let nowhere = json!({"repo": repo, "pull": pull, "base": "nope"}); | ||
assert_eq!(write("retargetPull", me, nowhere).0, 400); | ||
| ||
// Set to merge when its checks pass, by its owner alone: the page says | ||
// by whom, until it moves to another branch, and again once back. | ||
assert_eq!(status(me).2["may"]["autoMerge"], true); | ||
assert!(status(me).2.get("autoMerge").is_none()); | ||
assert_eq!(write("setAutoMerge", them, one.clone()).0, 400); | ||
let (code, _, set) = write("setAutoMerge", me, one.clone()); | ||
assert_eq!(code, 200, "{set}"); | ||
assert_eq!(set["commit"], status(me).2["head"], "{set}"); | ||
let auto = || status(me).2["autoMerge"].clone(); | ||
assert_eq!(auto()["state"], "set"); | ||
assert_eq!(auto()["by"]["handle"], owner.account.handle.as_str()); | ||
let next = json!({"repo": repo, "pull": pull, "base": "next"}); | ||
assert_eq!(write("retargetPull", me, next).2, json!({})); | ||
assert_eq!(auto()["state"], "retargeted"); | ||
let back = json!({"repo": repo, "pull": pull, "base": "main"}); | ||
assert_eq!(write("retargetPull", me, back).2, json!({})); | ||
assert_eq!(auto()["state"], "set"); | ||
// Cancelled, by its owner: it was not someone else's to cancel. | ||
let undo = json!({"repo": repo, "pull": pull, "undo": true}); | ||
assert_eq!(write("setAutoMerge", them, undo.clone()).0, 400); | ||
assert_eq!( | ||
write("setAutoMerge", me, undo.clone()), | ||
(200, Vec::new(), json!({})) | ||
); | ||
assert_eq!(auto(), Value::Null); | ||
assert_eq!(write("setAutoMerge", me, undo).0, 400); | ||
| ||
// With no runner named, it merges at once, as the page shows. | ||
let (code, _, named) = write("removeRunner", me, runner); | ||
assert_eq!((code, named), (200, json!({"runners": []}))); | ||
docs/reference/api.md+10−2
| @@ -783,6 +783,7 @@ private repository's are made and read as the member logged in. | ||
| `POST /xrpc/dev.knotless.revertPull` | `{repo, pull, reason}`: as the owner or a collaborator, reverts a merged pull as `knotless pull revert` does, answering `{uri, branch, commit}`, `uri` the pull it opens, which is indexed at once; 400 on a pull not merged, or whose undoing conflicts. `getThread`'s `viewer.revert` says who may | | ||
| `POST /xrpc/dev.knotless.queuePull` | `{repo, pull}`: puts it in its branch's merge queue, as `knotless pull merge --queue`, answering `{base, position, of}`, and `ejectedBefore`, why, when queued again at the very head the queue last ejected; 400 when the branch it lands on has no `queue` rule, and while the queue lands through this server's gateway and the owner has not delegated writes to it | | ||
| `POST /xrpc/dev.knotless.leaveQueue` | `{repo, pull}`: takes it out, as `knotless queue leave`, answering `{}` | | ||
| `POST /xrpc/dev.knotless.setAutoMerge` | `{repo, pull, undo}`: sets it to merge by itself once its checks pass, as `knotless pull merge --auto`, answering `{commit}`, the head it was set at; with `undo`, cancels that, as `--auto --undo`, answering `{}`. 400 into a branch that takes pulls only through the merge queue, in a repository that names no runners, and when it is not set to cancel | | ||
| `POST /xrpc/dev.knotless.retargetPull` | `{repo, pull, base, head}`: points it at other branches, either or both, as `knotless pull retarget`, answering `{}` | | ||
| `POST /xrpc/dev.knotless.requestRerun` | `{repo, pull}` or `{repo, ref}`, with `workflow`, and at a ref `env: [{name, value}]`: as the owner, asks the runners to run them again, as `knotless run rerun`, answering `{uri}`, the `dev.knotless.rerun` record | | ||
| `POST /xrpc/dev.knotless.addRunner` | `{repo, runner}`, a handle or DID: as the owner, names it a runner, as `knotless runner add`, answering `{runners, warning}`, the warning when a private repository could not tell it | | ||
| @@ -805,7 +806,13 @@ from older servers); | ||
that would work now: `merge` and `queue` (owner and | ||
collaborators, the queue only with runners), `force` (owner), `base` | ||
(those who may close it), `head` (its author), `rerun` (owner, with | ||
runners); and `queueBlocked: "undelegated"` when the asker may queue it but | ||
runners), `autoMerge`, setting it to merge when its checks pass (owner | ||
and collaborators, with runners, into a branch with no `queue` rule; | ||
its author may cancel it, as `head` says); `autoMerge`, once someone set | ||
it so: `state` (`set`; `pushed`, cancelled by a push from someone who | ||
may not merge it, `pushedBy` the account they pushed as; or | ||
`retargeted`, cancelled by its moving to another branch) and `by`, who | ||
set it, each an account as the pages show one; and `queueBlocked: "undelegated"` when the asker may queue it but | ||
no runner the repository names may push, so its queue lands through this | ||
server's gateway, and the owner has not delegated writes to it; and | ||
`queueOnly: true` when its branch takes pulls only through the merge | ||
| @@ -814,7 +821,8 @@ queue, where `mergePull` refuses it and the pages queue it with | ||
for none), `reviewsMissing: [{text}]`, what they lack, | ||
`reviewsUncounted: [{text}]`, the approvals that do not count and why, and | ||
`codeOwnersWarnings: [{text}]`, the CODEOWNERS lines skipped, as `knotless | ||
pull merge --check` prints them. A merge, queue or retarget is indexed at once, so the page after | ||
pull merge --check` prints them. A merge, queue, retarget, or merge when | ||
checks pass set or cancelled is indexed at once, so the page after | ||
shows it. | ||
| ||
`ref` and `path` together are split at the longest branch or tag name that | ||
harness/e2e/flows.spec.mjs+50−0
| @@ -2,6 +2,9 @@ | ||
// write shown at once, kept in the PDS, and shown again once the page is | ||
// read anew, from the index. | ||
| ||
import { writeFileSync } from 'node:fs' | ||
import { tmpdir } from 'node:os' | ||
import { join } from 'node:path' | ||
import { expect, test } from '@playwright/test' | ||
import { | ||
world, | ||
| @@ -911,6 +914,53 @@ test('asking for changes, then dismissing the review, as the owner', async () => | ||
) | ||
}) | ||
| ||
test('setting a pull to merge when its checks pass, and cancelling it', async () => { | ||
// bob's pull from his fork has no checks, so merges now: he adds one | ||
// no runner runs here, and alice, who owns the repository, sets it to | ||
// merge once it passes. | ||
const workflow = join(tmpdir(), 'pull-test.yml') | ||
writeFileSync( | ||
workflow, | ||
'on: {pull: {base: [refs/heads/main]}}\nsteps:\n - name: test\n run: cargo test\n', | ||
) | ||
knotless( | ||
'bob', | ||
'repo', | ||
'commit', | ||
'knotless://bob.test/tally', | ||
'-b', | ||
'words', | ||
'-m', | ||
'Test each pull', | ||
'--put', | ||
'.knotless/workflows/test.yml', | ||
workflow, | ||
) | ||
await go(page, proposed) | ||
const box = page.locator('.merge-box') | ||
await anew(page, (expect) => | ||
expect(box.getByRole('button', { name: 'Merge when checks pass' })).toBeVisible(), | ||
) | ||
await box.getByRole('button', { name: 'Merge when checks pass' }).click() | ||
await expect(box).toContainText('Merges when checks pass') | ||
await expect(box).toContainText('Set by alice.test.') | ||
const set = async () => | ||
(await records('alice', 'dev.knotless.autoMerge')).map((r) => r.value.state) | ||
await expect.poll(set).toEqual(['set']) | ||
await anew(page, (expect) => | ||
expect(page.locator('.merge-box')).toContainText('Merges when checks pass'), | ||
) | ||
// Cancelled, it merges only when someone merges it. | ||
await page.locator('.merge-box').getByRole('button', { name: 'Cancel merge' }).click() | ||
await expect( | ||
page.locator('.merge-box').getByRole('button', { name: 'Merge when checks pass' }), | ||
).toBeVisible() | ||
await expect.poll(async () => (await set()).sort()).toEqual(['cancelled', 'set']) | ||
await anew(page, (expect) => | ||
expect(page.locator('.merge-box')).not.toContainText('Merges when checks pass'), | ||
) | ||
}) | ||
| ||
test('merging into a protected branch', async () => { | ||
// alice's main is protected and linear: the pull is squashed into one commit. | ||
await go(page, world.views['pull-open']) | ||
lexicons/dev/knotless/getMergeStatus.json+16−0
| @@ -45,6 +45,7 @@ | ||
"base": { "type": "string", "description": "The commit its target branch is at, in hex." }, | ||
"head": { "type": "string", "description": "The commit its head branch is at, in hex." }, | ||
"may": { "type": "ref", "ref": "#permissions" }, | ||
"autoMerge": { "type": "ref", "ref": "#autoMerge" }, | ||
"queueBlocked": { | ||
"type": "string", | ||
"knownValues": ["undelegated"], | ||
| @@ -95,6 +96,20 @@ | ||
"path": { "type": "string", "description": "From the repository's root." } | ||
} | ||
}, | ||
"autoMerge": { | ||
"type": "object", | ||
"description": "Where the pull's merge when checks pass stands, if it was ever set and not cancelled by someone: a runner the repository names merges it once it would merge.", | ||
"required": ["state", "by"], | ||
"properties": { | ||
"state": { | ||
"type": "string", | ||
"knownValues": ["set", "pushed", "retargeted"], | ||
"description": "set while it stands; pushed once someone who may not merge it pushed to its head since, and retargeted once it moved to another branch, either of which cancels it." | ||
}, | ||
"by": { "type": "ref", "ref": "dev.knotless.defs#actorView", "description": "Who set it." }, | ||
"pushedBy": { "type": "ref", "ref": "dev.knotless.defs#actorView", "description": "For pushed, the owner of the head repository pushed to." } | ||
} | ||
}, | ||
"permissions": { | ||
"type": "object", | ||
"description": "What the account asking may do to the pull: what its part in the repository lets it do, not whether that would work now. state and reason say that: a merge refuses a blocked pull, the owner's merge too, unless the owner forces it.", | ||
| @@ -104,6 +119,7 @@ | ||
"force": { "type": "boolean", "description": "Merge it past its checks: the owner alone." }, | ||
"queue": { "type": "boolean", "description": "Put it in the merge queue or take it out: those who may merge it, in a repository whose runners land a queue, while the branch it lands on takes pulls through the queue or it is already queued." }, | ||
"base": { "type": "boolean", "description": "Change its target branch: those who may close it." }, | ||
"autoMerge": { "type": "boolean", "description": "Set it to merge by itself once its checks pass, with dev.knotless.setAutoMerge: those who may merge it, in a repository that names runners, into a branch with no merge queue. Cancelling that, those and its author may. Absent is false." }, | ||
"head": { "type": "boolean", "description": "Change its head branch: its author." }, | ||
"rerun": { "type": "boolean", "description": "Ask the runners to run its checks again: the owner, in a repository that names runners." } | ||
} | ||