chadtmiller.com/knotless
git that lives in your Atmosphere account
1
10
Queueing a pull again at the head it was ejected at says so
.changeset/queue-same-head.mdnew+7−0
| @@ -0,0 +1,7 @@ | ||
--- | ||
default: patch | ||
--- | ||
| ||
# Queueing a pull again at the head it was ejected at says so | ||
| ||
`knotless merge --queue` says when the pull is going back into the queue at the very head the queue last ejected, and why it was ejected: unless that was not the pull's doing, it will be ejected again. The MCP `merge` tool answers it as `queued.ejectedBefore`, and `dev.knotless.queuePull` as `ejectedBefore`. | ||
crates/knotless/src/cli.rs+22−2
| @@ -23,6 +23,24 @@ use serde_json::json; | ||
use std::io::Write; | ||
use std::path::PathBuf; | ||
| ||
/// Says so when `at` is a pull queued again at the very head the queue | ||
/// last ejected: unless that was not its doing, it will be again. | ||
fn ejected_before( | ||
out: &mut dyn Write, | ||
url: &RepoUrl, | ||
pull: &str, | ||
at: &crate::queue::Place, | ||
) -> Result<()> { | ||
if let Some(why) = &at.ejected_before { | ||
// Said first: a `?` on a line of its own counts as one for coverage. | ||
let said = format!( | ||
"This is the head the queue last ejected, as {why}. Unless that was not the pull's doing, it will be ejected again: fix it, or take it out with `knotless queue {url} --leave {pull}`." | ||
); | ||
writeln!(out, "{said}")?; | ||
} | ||
Ok(()) | ||
} | ||
| ||
fn runtime() -> tokio::runtime::Runtime { | ||
tokio::runtime::Builder::new_current_thread() | ||
.enable_all() | ||
| @@ -2105,8 +2123,9 @@ async fn command( | ||
bail!("--queue takes none of --keep-branch, --wait and --force"); | ||
} | ||
let at = crate::queue::enqueue(ctx, &url, pull).await?; | ||
let (n, of, base) = (at.position, at.of, at.base); | ||
let (n, of, base) = (&at.position, &at.of, &at.base); | ||
writeln!(out, "queued into {base}: {n} of {of}")?; | ||
ejected_before(out, &url, pull, &at)?; | ||
let next = format!( | ||
"A runner lands it once its batch passes its checks. `knotless queue {url}` shows where it is; `knotless show {url} {pull}`, whether it merged or was ejected and why. Ejected, read its checks' logs before queueing it again." | ||
); | ||
| @@ -2116,9 +2135,10 @@ async fn command( | ||
let merged = match crate::queue::merge_or_queue(ctx, &url, pull, keep, gate).await? { | ||
crate::queue::Landed::Merged(merged) => merged, | ||
crate::queue::Landed::Queued(at) => { | ||
let (n, of, base) = (at.position, at.of, at.base); | ||
let (n, of, base) = (&at.position, &at.of, &at.base); | ||
let said = format!("{base} takes pulls through the merge queue: queued"); | ||
writeln!(out, "{said}, {n} of {of}")?; | ||
ejected_before(out, &url, pull, &at)?; | ||
let next = format!( | ||
"A runner lands it once its batch passes its checks. `knotless queue {url}` shows where it is; as the owner, --force merges it at once." | ||
); | ||
crates/knotless/src/queue.rs+22−2
| @@ -59,6 +59,10 @@ pub struct Place { | ||
pub base: String, | ||
pub position: usize, | ||
pub of: usize, | ||
/// Queued again at the very head the queue last ejected, why it was: | ||
/// unless that was not the pull's doing, it will be again. | ||
#[serde(rename = "ejectedBefore", skip_serializing_if = "Option::is_none")] | ||
pub ejected_before: Option<String>, | ||
} | ||
| ||
/// `pull`'s place among `items`, if it is queued. | ||
| @@ -71,7 +75,12 @@ fn place(items: &[Item], pull: &str) -> Option<Place> { | ||
.collect(); | ||
let position = into.iter().position(|i| i.uri == pull)? + 1; | ||
let of = into.len(); | ||
Some(Place { base, position, of }) | ||
Some(Place { | ||
base, | ||
position, | ||
of, | ||
ejected_before: None, | ||
}) | ||
} | ||
| ||
/// Me, if I may put pulls in `t`'s merge queue: its owner or a | ||
| @@ -137,8 +146,10 @@ pub async fn enqueue(ctx: &Ctx, url: &RepoUrl, pull: &str) -> Result<Place> { | ||
if item.draft { | ||
bail!("{}", forge::drafted(url, pull)); | ||
} | ||
let mut before = None; | ||
if item.queue.as_ref().is_none_or(|q| q.state != QUEUED) { | ||
let head = admit(ctx, &t, &item).await?; | ||
before = ejected_at(&item, &head); | ||
let at = Ejected { | ||
head: Some(&head), | ||
..Ejected::default() | ||
| @@ -149,7 +160,16 @@ pub async fn enqueue(ctx: &Ctx, url: &RepoUrl, pull: &str) -> Result<Place> { | ||
entries.push(Entry { uri, author, value }); | ||
} | ||
let items = forge::assemble(&t.record, t.remote.did(), &entries); | ||
Ok(place(&items, pull).expect("a pull just queued is in the queue")) | ||
let mut at = place(&items, pull).expect("a pull just queued is in the queue"); | ||
at.ejected_before = before; | ||
Ok(at) | ||
} | ||
| ||
/// Why the queue last ejected `item`, when it did so at `head`. | ||
fn ejected_at(item: &Item, head: &str) -> Option<String> { | ||
let last = item.queue.as_ref()?; | ||
let same = last.state == EJECTED && last.commit.as_deref() == Some(head); | ||
same.then(|| last.reason.clone().unwrap_or_default()) | ||
} | ||
| ||
/// What a plain merge did: merged the pull, or, into a branch that takes | ||
crates/knotless-mcp/src/lib.rs+1−1
| @@ -2256,7 +2256,7 @@ impl Server { | ||
} | ||
| ||
#[tool( | ||
description = "Merge an open pull request: a fast-forward when possible, else a merge commit; into a branch protected as linear, one squashed commit. Fails on conflicts. In a repository that names runners, only once every workflow that runs on the pull has passed at its head merged with the target branch as it is now: it refuses, naming what failed, what is still running, or the older head or base its checks are of, and checks with pull and log shows why. With wait, it waits up to that many seconds for checks running or not yet run, and the owner's merge asks the runners once to run a head and base none has; force, the owner's alone, merges whatever they say. Into a branch protected with queue (list_rules), it queues the pull instead, as queue does, and answers queued with why, unless the owner forces it. Its head branch, when in this repository, is deleted with it unless keep_branch or another open pull names it. The owner and collaborators may. Into the repository's default branch, it closes each open issue on it that the pull's title or body names right after fixes, closes or resolves (or fix, fixed, close, closed, resolve, resolved), by its page URL (https://<appview>/<owner>/<repo>/issue/<did>/<rkey>) or at:// URI, returning them as closed; the merge queue does the same as it lands. With queue, it puts the pull in its branch's merge queue instead and returns at once with its place (queued: position of of): the quickest way to land, as a runner checks queued pulls together a batch at a time, and there is no need to wait. Queue a pull once it builds, though: one that does not fails its whole batch, and every pull in it waits while the queue halves the batch to find it. Merge the target branch into the pull's branch just before queueing: one that conflicts with it is refused. Ejected, read why with show_item and the failed checks' logs with checks (pull and log), fix it, and queue it again; queue the same head again only when the log shows the failure is not the pull's, such as a test it does not touch that fails other pulls too. One that conflicts only with a pull ahead of it waits for that pull to land or leave, and is ejected then if it conflicts with the branch: merge the branch into it. Ejected again for the same reason, stop and say so rather than queue it once more. merge_queue shows where it is, and show_item whether it merged, or where it stands (standing.said: Queued, 3rd of 5, running its checks or ready for the next batch; In a batch of 4 being checked; Landing; or why it was ejected and which checks failed, also under item.queue.reason and item.queue.checks). Into a branch protected with reviews, it refuses until the pull is reviewed as the rule asks (see protect_branch), naming what is missing. With check, it merges nothing and says whether it would now: state clean, conflicting (conflicts, the paths) or blocked by its checks (checks: passed, failed, running, pending, or none when the repository names no runners) or its reviews (reviews: passed, blocked, or none when its branch asks for none; reviewsMissing, what they lack, as reviews 1 of 2 or owners of web/: @alice or @bob; reviewsUncounted, the approvals that do not count and why; codeOwnersWarnings, the CODEOWNERS lines skipped and why); reason, why not, in the words a merge would refuse with, and that you may not merge it when you may not; method (fastForward, merge or squash); deletes, the branch it would delete; and may, what your part in the repository lets you do to it (merge, force, queue, base and head as retarget_pull changes them, rerun): not whether it would work now, which state and reason say. A blocked pull's merge is refused, the owner's too, unless the owner forces it. queueOnly, when its branch takes pulls only through the merge queue. Merged, it answers the commit its branch is at now (sha), how (method: fastForward, merge or squash), the branch it deleted (deleted) or why it kept the head branch (kept: asked to, in another repository such as a fork, the branch it merges into or the default branch, or named by another open pull), and the issues it closed." | ||
description = "Merge an open pull request: a fast-forward when possible, else a merge commit; into a branch protected as linear, one squashed commit. Fails on conflicts. In a repository that names runners, only once every workflow that runs on the pull has passed at its head merged with the target branch as it is now: it refuses, naming what failed, what is still running, or the older head or base its checks are of, and checks with pull and log shows why. With wait, it waits up to that many seconds for checks running or not yet run, and the owner's merge asks the runners once to run a head and base none has; force, the owner's alone, merges whatever they say. Into a branch protected with queue (list_rules), it queues the pull instead, as queue does, and answers queued with why, unless the owner forces it. Its head branch, when in this repository, is deleted with it unless keep_branch or another open pull names it. The owner and collaborators may. Into the repository's default branch, it closes each open issue on it that the pull's title or body names right after fixes, closes or resolves (or fix, fixed, close, closed, resolve, resolved), by its page URL (https://<appview>/<owner>/<repo>/issue/<did>/<rkey>) or at:// URI, returning them as closed; the merge queue does the same as it lands. With queue, it puts the pull in its branch's merge queue instead and returns at once with its place (queued: position of of), and, queued again at the very head the queue last ejected, why it was (queued.ejectedBefore), as it will be again unless that was not the pull's doing: the quickest way to land, as a runner checks queued pulls together a batch at a time, and there is no need to wait. Queue a pull once it builds, though: one that does not fails its whole batch, and every pull in it waits while the queue halves the batch to find it. Merge the target branch into the pull's branch just before queueing: one that conflicts with it is refused. Ejected, read why with show_item and the failed checks' logs with checks (pull and log), fix it, and queue it again; queue the same head again only when the log shows the failure is not the pull's, such as a test it does not touch that fails other pulls too. One that conflicts only with a pull ahead of it waits for that pull to land or leave, and is ejected then if it conflicts with the branch: merge the branch into it. Ejected again for the same reason, stop and say so rather than queue it once more. merge_queue shows where it is, and show_item whether it merged, or where it stands (standing.said: Queued, 3rd of 5, running its checks or ready for the next batch; In a batch of 4 being checked; Landing; or why it was ejected and which checks failed, also under item.queue.reason and item.queue.checks). Into a branch protected with reviews, it refuses until the pull is reviewed as the rule asks (see protect_branch), naming what is missing. With check, it merges nothing and says whether it would now: state clean, conflicting (conflicts, the paths) or blocked by its checks (checks: passed, failed, running, pending, or none when the repository names no runners) or its reviews (reviews: passed, blocked, or none when its branch asks for none; reviewsMissing, what they lack, as reviews 1 of 2 or owners of web/: @alice or @bob; reviewsUncounted, the approvals that do not count and why; codeOwnersWarnings, the CODEOWNERS lines skipped and why); reason, why not, in the words a merge would refuse with, and that you may not merge it when you may not; method (fastForward, merge or squash); deletes, the branch it would delete; and may, what your part in the repository lets you do to it (merge, force, queue, base and head as retarget_pull changes them, rerun): not whether it would work now, which state and reason say. A blocked pull's merge is refused, the owner's too, unless the owner forces it. queueOnly, when its branch takes pulls only through the merge queue. Merged, it answers the commit its branch is at now (sha), how (method: fastForward, merge or squash), the branch it deleted (deleted) or why it kept the head branch (kept: asked to, in another repository such as a fork, the branch it merges into or the default branch, or named by another open pull), and the issues it closed." | ||
)] | ||
async fn merge( | ||
&self, | ||
crates/knotless-run/src/queue.rs+9−1
| @@ -141,6 +141,9 @@ pub(crate) struct Line { | ||
flights: Vec<Flight>, | ||
suspects: Vec<String>, | ||
failed: (String, Vec<FailedCheck>), | ||
/// The head of each pull in the batch that failed, by its URI: the | ||
/// head the suspect left alone is ejected at. | ||
failed_heads: HashMap<String, String>, | ||
done: HashMap<String, String>, | ||
flown: HashMap<String, String>, | ||
} | ||
| @@ -389,7 +392,9 @@ impl Runner { | ||
let item = queued.iter().find(|i| &i.uri == lone); | ||
let item = item.expect("a suspect is queued").clone(); | ||
let (why, checks) = line.failed.clone(); | ||
self.eject(on, line, &item, None, &why, &checks).await?; | ||
let head = line.failed_heads.get(lone).cloned(); | ||
self.eject(on, line, &item, head.as_deref(), &why, &checks) | ||
.await?; | ||
line.suspects.clear(); | ||
} | ||
continue; | ||
| @@ -410,6 +415,9 @@ impl Runner { | ||
} | ||
all => { | ||
line.suspects = all.iter().map(|s| s.pull.uri.clone()).collect(); | ||
line.failed_heads = (all.iter()) | ||
.map(|s| (s.pull.uri.clone(), s.head.clone())) | ||
.collect(); | ||
line.failed = (format!("{why}. {HALVED}"), checks); | ||
} | ||
} | ||
crates/knotless-run/tests/run.rs+10−0
| @@ -3058,6 +3058,16 @@ fn a_merge_queue_lands_in_batches() { | ||
line.contains(&format!("-\tmain\tejected\t{x}\tx\n\t")), | ||
"{line}" | ||
); | ||
// Queued again at the head it was ejected at, it is told so, and why. | ||
let again = q.ok(&["merge", &url, &x, "--queue"]); | ||
assert!( | ||
again.contains( | ||
"This is the head the queue last ejected, as its batch failed its checks at " | ||
), | ||
"{again}" | ||
); | ||
assert!(again.contains(&format!("--leave {x}`")), "{again}"); | ||
q.ok(&["queue", &url, "--leave", &x]); | ||
| ||
// One alone that fails is ejected at the head it was: here, as a | ||
// workflow it brings does not load. | ||
docs/reference/api.md+1−1
| @@ -702,7 +702,7 @@ private repository's are made and read as the member logged in. | ||
| --- | --- | | ||
| `POST /xrpc/dev.knotless.mergePull` | `{repo, pull, keepBranch}`: merges it as `knotless merge` does, waiting for no checks, answering `{commit, deleted, closed: [{uri}]}`; 400 into a branch that takes pulls only through the merge queue, which `queuePull` queues it into | | ||
| `POST /xrpc/dev.knotless.revertPull` | `{repo, pull, reason}`: as the owner or a collaborator, reverts a merged pull as `knotless 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 merge --queue`, answering `{base, position, of}`; 400 while the queue lands through this server's gateway and the owner has not delegated writes to it | | ||
| `POST /xrpc/dev.knotless.queuePull` | `{repo, pull}`: puts it in its branch's merge queue, as `knotless merge --queue`, answering `{base, position, of}`, and `ejectedBefore`, why, when queued again at the very head the queue last ejected; 400 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.retargetPull` | `{repo, pull, base, head}`: points it at other branches, either or both, as `knotless 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 rerun`, answering `{uri}`, the `dev.knotless.rerun` record | | ||
docs/reference/cli.md+1−1
| @@ -166,7 +166,7 @@ What they print: | ||
| `list` | a line per item: `<kind>`, `<state>`, `<uri>`, `<title>`, tab-separated | | ||
| `show` | `<state> <kind> by <author>: <title>`, then `draft` for a draft pull, `review requested: <handles>` for one that asks for reviews, then `closed by <pull>` for an issue a pull closed, `mentioned in <pull>` for each pull whose title or body names an issue, oldest first, ending `, to close it` where it names it after a closing keyword, `reverts <pull>` for a revert, `reverted by <pull>` for a merged pull whose revert merged, then for a pull each reviewer's standing review as `review: <reviewer> <verdict> at <commit>` (the first 12 characters of the head it looked at), then for an open pull in or ejected from the merge queue `merge queue: <state> at <createdAt>` and where it stands, and for one ejected as checks failed `logs: <command>`, the `knotless checks` that reads their logs, then the body and each comment as `<author> [<verdict>]: <body>`, where a comment on lines says ` on <path>:<lines>[:old] (<uri>[, resolved])` before the colon, and an answer in a thread ` answering <uri>` | | ||
| `merge` | the commit the branch is now at, then `deleted <branch>` and a `closed <issue>` line per issue it closed; queued instead, `<base> takes pulls through the merge queue: queued, <n> of <of>`, then where to follow it | | ||
| `merge --queue` | `queued into <base>: <n> of <of>`, then where to follow it, and to read an ejection's logs before queueing again | | ||
| `merge --queue` | `queued into <base>: <n> of <of>`; queued again at the head the queue last ejected, a line saying so and why; then where to follow it, and to read an ejection's logs before queueing again | | ||
| `merge --check` | `<state>`, `<method>` and `checks <checks>`, and `reviews <passed or blocked>` when its branch asks for reviews, tab-separated, then a `missing <what>` line for each thing its reviews lack (`reviews 1 of 2`, `owners of web/: @alice or @bob`, `changes asked by @carol`; owners who are the pull's author alone add `, its author alone, whose own approval never counts, so web/ needs another owner in CODEOWNERS`), an `uncounted <why>` line for each approval that does not count (`@dan's approval does not count: only the owner's and collaborators' approvals count`), a `warning <what>` line for each CODEOWNERS line skipped, a `conflict <path>` line per file that conflicts, `deletes <branch>` for the branch it would delete, `may <what>`, `queue only` when its branch takes pulls only through the merge queue, and last why it cannot merge, in the words `merge` would refuse with, and that the caller may not merge it when it may not | | ||
| `queue` | a line per pull: `<position>`, `<base>`, `<state>`, `<uri>`, `<title>`, tab-separated, where the state is `queued`, `pre-flight <workflows>` while the [pre-flights](workflows.md#the-merge-queue) it waits for run, comma-separated, or `<checking or landing> at <commit>`; an ejected pull has `-` and `ejected`, and a line after it with the reason; `the merge queue is empty` if it is | | ||
| `queue --json` | each pull waiting, with its `place` and `batch`, or the `preflight` workflows it waits for | | ||
docs/reference/mcp.md+3−1
| @@ -72,7 +72,9 @@ or else why it kept it (asked to, in another repository such as a fork, | ||
the branch merged into or the default branch, or named by another open | ||
pull), and the issues it closed. | ||
With `queue: true`, `merge` puts the pull in its branch's merge queue | ||
instead and answers `{"queued": {base, position, of}}` at once; it then | ||
instead and answers `{"queued": {base, position, of}}` at once, with | ||
`ejectedBefore`, why, when it is queued again at the very head the queue | ||
last ejected; it then | ||
ignores `wait`, `force` and `keep_branch`. Its description tells an | ||
agent to queue a pull once it builds, with the target branch merged in; | ||
once ejected, to read why and the failed checks' logs, fix it and queue | ||
lexicons/dev/knotless/queuePull.json+2−1
| @@ -25,7 +25,8 @@ | ||
"properties": { | ||
"base": { "type": "string", "description": "The branch whose queue it is in." }, | ||
"position": { "type": "integer", "minimum": 1, "description": "Its place in that queue, from 1." }, | ||
"of": { "type": "integer", "minimum": 1, "description": "How many are queued there." } | ||
"of": { "type": "integer", "minimum": 1, "description": "How many are queued there." }, | ||
"ejectedBefore": { "type": "string", "description": "Queued again at the very head the queue last ejected, why it was: unless that was not the pull's doing, it will be ejected again." } | ||
} | ||
} | ||
}, | ||