chadtmiller.com/knotless
git that lives in your Atmosphere account
1
10
The merge queue is off until a branch's rule turns it on
.changeset/queue-off-by-default.mdnew+7−0
| @@ -0,0 +1,7 @@ | ||
--- | ||
default: minor | ||
--- | ||
| ||
# The merge queue is off until a branch's rule turns it on | ||
| ||
`knotless pull merge --queue` and the `pull_merge` tool's `queue` now refuse a pull unless the branch it lands on is protected with `--queue`. Turn it on with `knotless branch protect <url> <branch> --queue`, and every pull into that branch then lands through the queue. A pull already queued keeps its place. | ||
crates/knotless/src/cli.rs+6−4
| @@ -279,10 +279,12 @@ pull | ||
pull retarget <url> <at-uri> [--base <branch>] [--head <branch>] | ||
point a pull at other branches; the head | ||
only by the pull's author | ||
pull merge <url> <at-uri> --queue put a pull in its branch's merge queue and | ||
return: a runner checks queued pulls in | ||
batches, squashed one commit each on the | ||
branch, and lands each batch that passes | ||
pull merge <url> <at-uri> --queue put a pull in its branch's merge queue, | ||
where the branch is protected with | ||
--queue, and return: a runner checks | ||
queued pulls in batches, squashed one | ||
commit each on the branch, and lands | ||
each batch that passes | ||
pull merge <url> <at-uri> [--keep-branch] [--wait <secs>] [--force] | ||
merge a pull: a merge commit naming it, | ||
or squashed into a protected branch; its | ||
crates/knotless/src/landing.rs+10−4
| @@ -95,7 +95,8 @@ pub struct May { | ||
/// Merge it past its checks: the owner alone. | ||
pub force: bool, | ||
/// Put it in the merge queue, or take it out: those who may merge it, | ||
/// in a repository whose runners land a queue. | ||
/// 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, | ||
/// Change its target branch: those who may close it. | ||
pub base: bool, | ||
| @@ -107,15 +108,20 @@ pub struct May { | ||
} | ||
| ||
/// What `who` may do to `item` on `t`. | ||
pub fn may(t: Facts, item: &Item, who: &str) -> May { | ||
pub fn may(t: Facts, items: &[Item], item: &Item, who: &str) -> May { | ||
let owner = t.owner; | ||
let at = crate::keys::tid(); | ||
let maintainer = forge::allowed(t.record, owner, &item.author, who, "merged", &at); | ||
let runners = !t.record.runners.is_empty(); | ||
let queued = item | ||
.queue | ||
.as_ref() | ||
.is_some_and(|q| q.state == crate::records::queue::QUEUED); | ||
let on = queued || crate::queue::takes(t.record, owner, items, item); | ||
May { | ||
merge: maintainer, | ||
force: who == owner, | ||
queue: maintainer && runners, | ||
queue: maintainer && runners && on, | ||
base: forge::allowed(t.record, owner, &item.author, who, "closed", &at), | ||
head: who == item.author, | ||
rerun: who == owner && runners, | ||
| @@ -207,7 +213,7 @@ pub async fn gauge( | ||
deletes: forge::spent(t, item, items), | ||
base: base.clone(), | ||
head: head.clone(), | ||
may: may(t, item, who), | ||
may: may(t, items, item, who), | ||
queue_blocked: None, | ||
queue_only: t.record.queue_only(branch), | ||
}; | ||
crates/knotless/src/queue.rs+20−1
| @@ -62,6 +62,16 @@ pub fn lands_on(items: &[Item], item: &Item) -> String { | ||
at.base.clone().unwrap_or_default() | ||
} | ||
| ||
/// Whether the merge queue is on for `item`: whether the branch it lands | ||
/// on, the one under its whole stack if it is in one, takes pulls through | ||
/// the queue by its rule. | ||
pub fn takes(record: &RepoRecord, owner: &str, items: &[Item], item: &Item) -> bool { | ||
let chain = forge::stack_of(owner, &record.key, items, &item.uri); | ||
let lowest = chain.last().copied().unwrap_or(item); | ||
let base = lowest.base.as_deref().unwrap_or_default(); | ||
record.queue_only(base) | ||
} | ||
| ||
/// The queued pulls among `items`, in the order they were queued: the open | ||
/// pulls whose latest queue record is `queued`. One its author made a | ||
/// draft again after it was queued waits out of line, and is back in its | ||
| @@ -156,7 +166,9 @@ async fn open_pull(ctx: &Ctx, t: &Target, pull: &str) -> Result<(Vec<Entry>, Ite | ||
| ||
/// Puts a pull at the back of the merge queue of the branch it targets, | ||
/// for a runner of the repository to land: returns at once with its place. | ||
/// One already queued keeps its place. The owner and collaborators may. | ||
/// One already queued keeps its place, whatever the branch's rule says now; | ||
/// any other goes in only where the rule turns the queue on ([`takes`]). | ||
/// The owner and collaborators may. | ||
pub async fn enqueue(ctx: &Ctx, url: &RepoUrl, pull: &str) -> Result<Place> { | ||
let t = Target::open(ctx, url).await?; | ||
let pull = t.named(pull); | ||
| @@ -173,6 +185,13 @@ pub async fn enqueue(ctx: &Ctx, url: &RepoUrl, pull: &str) -> Result<Place> { | ||
} | ||
let mut before = None; | ||
if item.queue.as_ref().is_none_or(|q| q.state != QUEUED) { | ||
let items = forge::assemble(&t.record, t.remote.did(), &entries); | ||
if !takes(&t.record, t.remote.did(), &items, &item) { | ||
let base = item.base.as_deref().unwrap_or_default(); | ||
bail!( | ||
"{base} does not use the merge queue: merge it with `knotless pull merge`, or turn the queue on with `knotless branch protect {url} {base} --queue`" | ||
); | ||
} | ||
let head = admit(ctx, &t, &item).await?; | ||
before = ejected_at(&item, &head); | ||
let at = Ejected { | ||
crates/knotless-agent/src/lib.rs+3−0
| @@ -366,6 +366,9 @@ impl Agent { | ||
let items = forge::list(&self.ctx, url).await?; | ||
let (owner, key) = (t.remote.did(), t.reference().key); | ||
let ejected = items.iter().filter(|i| keeps(i, owner, &key, &t.record)); | ||
// Queueing is refused once its branch's rule has dropped the queue, | ||
// so one brought up to date could not go back in. | ||
let ejected = ejected.filter(|i| knotless::queue::takes(&t.record, owner, &items, i)); | ||
for pull in ejected { | ||
let kept = self.keep(url, &t, pull).await; | ||
let kept = kept.map(|did| did.map(String::from)); | ||
crates/knotless-agent/tests/keep.rs+14−1
| @@ -62,8 +62,12 @@ fn an_agent_keeps_turned_away_pulls_moving() { | ||
let work = owner.work("w", 1); | ||
owner.commit_file(&work, "notes", "one\n"); | ||
owner.git(&work, &["push", "-q", &url, "main"]); | ||
// A runner named, so its pulls may be queued. | ||
// A runner named, and the queue on for main, so its pulls may be queued. | ||
ok(&owner, &["runner", "add", &url, &handle]); | ||
ok( | ||
&owner, | ||
&["branch", "protect", &url, "main", "--merges", "--queue"], | ||
); | ||
// `branch` as main is now, with `file` holding `text`, pushed. | ||
let write = |branch: &str, from: &str, file: &str, text: &str| { | ||
owner.git(&work, &["fetch", "-q", &url, from]); | ||
| @@ -208,8 +212,17 @@ fn an_agent_keeps_turned_away_pulls_moving() { | ||
| ||
// Left turned away at the head it had before it was brought up to | ||
// date, as when queueing it failed: queued again, nothing more said. | ||
// With the queue off for main it is left alone, until it is on again. | ||
eject(&added, &added_at); | ||
wait_until(|| shown(&added).contains("merge queue: ejected")); | ||
ok(&owner, &["branch", "protect", &url, "main", "--undo"]); | ||
once(&opts); | ||
assert!(shown(&added).contains("merge queue: ejected")); | ||
assert_eq!(said(&owner, &added).len(), 1); | ||
ok( | ||
&owner, | ||
&["branch", "protect", &url, "main", "--merges", "--queue"], | ||
); | ||
once(&opts); | ||
wait_until(|| shown(&added).contains("merge queue: queued")); | ||
assert_eq!(said(&owner, &added).len(), 1); | ||
crates/knotless-mcp/src/lib.rs+2−1
| @@ -1199,7 +1199,8 @@ pub struct MergePull { | ||
#[serde(default)] | ||
pub force: bool, | ||
/// Put it in its branch's merge queue instead, and return at once: | ||
/// a runner lands it with the batch it is in once that passes. | ||
/// a runner lands it with the batch it is in once that passes. Refused | ||
/// unless the branch is protected with `queue`. | ||
#[serde(default)] | ||
pub queue: bool, | ||
/// Say whether it would merge now, and why not, and what you may do | ||
crates/knotless-mcp/tests/mcp.rs+11−0
| @@ -1563,6 +1563,17 @@ fn agents_fork_review_and_merge() { | ||
"runner_add", | ||
json!({"repo": repo, "handle": owner.account.handle}), | ||
); | ||
// Nor is there a queue until a rule of the branch turns it on. | ||
let (text, err) = o.call( | ||
"pull_merge", | ||
json!({"repo": repo, "uri": aside, "queue": true}), | ||
); | ||
assert!( | ||
err && text.contains("does not use the merge queue"), | ||
"{text}" | ||
); | ||
let on = json!({"repo": repo, "branch": "main", "merges": true, "queue": true}); | ||
o.json("branch_protect", on); | ||
// Asked first, it would merge: it has no workflows to wait for. Its | ||
// branch is in the fork, so nothing goes with it. | ||
let checked = o.json( | ||
crates/knotless-run/tests/run.rs+7−1
| @@ -3380,6 +3380,9 @@ impl Line { | ||
server, | ||
}; | ||
line.ok(&["runner", "add", &line.url, &handle]); | ||
line.ok(&[ | ||
"branch", "protect", &line.url, "main", "--merges", "--queue", | ||
]); | ||
line | ||
} | ||
| ||
| @@ -3827,6 +3830,7 @@ fn a_merge_queue_lands_in_batches() { | ||
"sidework", | ||
"Into side", | ||
]); | ||
q.ok(&["branch", "protect", &url, "side", "--merges", "--queue"]); | ||
let out = q.ok(&["pull", "merge", &url, &into, "--queue"]); | ||
assert!(out.starts_with("queued into side: 1 of 1\n"), "{out}"); | ||
wait_until(|| q.ok(&["queue", "list", &url]).contains(&into)); | ||
| @@ -4082,7 +4086,9 @@ fn a_merge_queue_waits_for_its_gate() { | ||
gate.set("KNOTLESS_APPVIEW", &q.base); | ||
let handle = gate.account.handle.clone(); | ||
gate.set("KNOTLESS_IDENTIFIER", &handle); | ||
let rules = q.ok(&["branch", "protect", &url, "main", "--gate", &handle]); | ||
let rules = q.ok(&[ | ||
"branch", "protect", &url, "main", "--merges", "--queue", "--gate", &handle, | ||
]); | ||
assert!( | ||
rules.contains(&format!("\tgate {}", gate.account.did)), | ||
"{rules}" | ||
crates/knotless-serve/src/web/settings.rs+4−0
| @@ -54,6 +54,9 @@ pub struct Settings { | ||
#[serde(skip_serializing_if = "Option::is_none")] | ||
default_branch: Option<String>, | ||
rules: Vec<knotless::records::Rule>, | ||
/// Whether it names a runner, which the merge queue needs to land it. | ||
#[serde(skip_serializing_if = "std::ops::Not::not")] | ||
runners: bool, | ||
labels: Vec<knotless::records::Label>, | ||
collaborators: Vec<ActorView>, | ||
triagers: Vec<TriagerView>, | ||
| @@ -146,6 +149,7 @@ pub async fn get_repo_settings( | ||
topics: record.topics, | ||
default_branch: record.default_branch, | ||
rules: record.rules, | ||
runners: !record.runners.is_empty(), | ||
labels: record.labels, | ||
collaborators, | ||
triagers, | ||