chadtmiller.com/knotless
git that lives in your Atmosphere account
1
10
Errors on the pages name their controls, never a command
.changeset/refusals-in-their-readers-words.mdnew+7−0
| @@ -0,0 +1,7 @@ | ||
--- | ||
default: patch | ||
--- | ||
| ||
# Refusals say what to do in their reader's words | ||
| ||
A refusal ends with what to do about it, after a full stop: on the command line as a command, over MCP as a tool, and on the pages as the control that does it, leaving out what only the command line can. The runner no longer writes how to approve a pull into the reason it records for ejecting one that is not reviewed as its branch asks. Some wordings moved to fit: a failed merge gate ends `read their logs with …`, and `not logged in as <did>` no longer names `KNOTLESS_PASSWORD`. | ||
crates/knotless/src/approvals.rs+15−15
| @@ -24,6 +24,7 @@ | ||
| ||
use crate::codeowners::{self, CodeOwners}; | ||
use crate::env::Ctx; | ||
use crate::fix::{Fix, Refused}; | ||
use crate::forge::{Facts, Item}; | ||
use crate::git::Git; | ||
use crate::identity; | ||
| @@ -314,16 +315,10 @@ pub async fn changed(git: &Git, base: &str, head: &str) -> Result<Vec<String>> { | ||
Ok(names.lines().map(str::to_string).collect()) | ||
} | ||
| ||
/// How a refusal ([`refusal`]) starts to say what to do about it, for a | ||
/// client that says it in its own words to find. | ||
pub const HOW: &str = "Anyone but its author who may write approves it with `knotless comment add "; | ||
/// How it ends to say so. | ||
pub const FORCE: &str = "the owner, its author or not, merges past it with `--force`"; | ||
| ||
/// Why `pull` on `url` may not merge into `branch` as reviewed, or none | ||
/// when it may: what it lacks, the approvals that do not count, and what | ||
/// to do, last. | ||
pub fn refusal(url: &RepoUrl, pull: &str, branch: &str, standing: &Standing) -> Option<String> { | ||
/// when it may: what it lacks and the approvals that do not count, then | ||
/// who approves it, or merges past it. | ||
pub fn refusal(url: &RepoUrl, pull: &str, branch: &str, standing: &Standing) -> Option<Refused> { | ||
let missing = standing.missing(); | ||
if missing.is_empty() { | ||
return None; | ||
| @@ -331,12 +326,17 @@ pub fn refusal(url: &RepoUrl, pull: &str, branch: &str, standing: &Standing) -> | ||
let uncounted: String = standing | ||
.uncounted() | ||
.iter() | ||
.map(|line| format!(" {line}.")) | ||
.map(|line| format!(". {line}")) | ||
.collect(); | ||
Some(format!( | ||
"{pull} is not reviewed as {branch} asks: {}.{uncounted} {HOW}{url} {pull} <text> --approve`; {FORCE}", | ||
let why = format!( | ||
"{pull} is not reviewed as {branch} asks: {}{uncounted}", | ||
missing.join("; ") | ||
)) | ||
); | ||
let approve = Fix::Approve { | ||
url: url.to_string(), | ||
pull: pull.to_string(), | ||
}; | ||
Some(Refused::new(why, [approve, Fix::Force])) | ||
} | ||
| ||
#[cfg(test)] | ||
| @@ -405,7 +405,7 @@ mod tests { | ||
] | ||
); | ||
let url = RepoUrl::parse("knotless://alice.test/r").unwrap(); | ||
let why = refusal(&url, "at://p", "main", &fresh).unwrap(); | ||
let why = refusal(&url, "at://p", "main", &fresh).unwrap().to_string(); | ||
let said = format!( | ||
"main asks: reviews 1 of 2. did:alice's approval does not count: {STALE}. did:stranger's" | ||
); | ||
| @@ -566,7 +566,7 @@ mod tests { | ||
); | ||
assert_eq!(s.missing(), ["owners of web/: @alice or @bob"]); | ||
let url = RepoUrl::parse("knotless://alice.test/r").unwrap(); | ||
let why = refusal(&url, "at://p", "main", &s).unwrap(); | ||
let why = refusal(&url, "at://p", "main", &s).unwrap().to_string(); | ||
assert!( | ||
why.starts_with( | ||
"at://p is not reviewed as main asks: owners of web/: @alice or @bob. " | ||
crates/knotless/src/auto_merge.rs+15−6
| @@ -17,6 +17,7 @@ | ||
//! ([`crate::queue::landing_refusal`]). | ||
| ||
use crate::env::Ctx; | ||
use crate::fix::{Fix, Refused}; | ||
use crate::forge::{self, AutoMerge, Item, Target}; | ||
use crate::git::Git; | ||
use crate::push::Update; | ||
| @@ -96,19 +97,27 @@ pub async fn set(ctx: &Ctx, url: &RepoUrl, pull: &str) -> Result<String> { | ||
); | ||
} | ||
if t.record.runners.is_empty() { | ||
bail!( | ||
"{url} names no runners, and a runner merges a pull once its checks pass: merge it with `knotless pull merge`, or name one with `knotless runner add {url} <handle>`" | ||
); | ||
let why = | ||
format!("{url} names no runners, and a runner merges a pull once its checks pass"); | ||
let runner = Fix::AddRunner { | ||
url: url.to_string(), | ||
}; | ||
bail!(Refused::new(why, [Fix::Merge, runner])); | ||
} | ||
let (_, item) = crate::queue::open_pull(ctx, &t, pull).await?; | ||
if item.draft { | ||
bail!("{}", forge::drafted(url, pull)); | ||
bail!(forge::drafted(url, pull)); | ||
} | ||
let base = item.base.as_deref().unwrap_or_default(); | ||
if t.record.queue_only(base) { | ||
bail!( | ||
"{base} takes pulls through the merge queue, which holds one until its checks pass: queue it with `knotless pull merge {url} {pull} --queue`" | ||
let why = format!( | ||
"{base} takes pulls through the merge queue, which holds one until its checks pass" | ||
); | ||
let queue = Fix::Queue { | ||
url: url.to_string(), | ||
pull: pull.to_string(), | ||
}; | ||
bail!(Refused::new(why, [queue])); | ||
} | ||
let sides = forge::sides(ctx, &t, &item).await?; | ||
let value = record(&t, pull, SET, Some((&item, &sides.head))); | ||
crates/knotless/src/cli.rs+3−2
| @@ -677,7 +677,7 @@ async fn org(ctx: &Ctx, args: &mut Args<'_>, out: &mut dyn Write) -> Result<()> | ||
said.push(format!( | ||
"You've left {org}: you can no longer push to its repositories." | ||
)); | ||
said.extend(stuck.map(|why| format!("warning: {org}'s gateway did not take you out of its spaces, so you may still read its private repositories until it does, which `knotless org leave {org}` asks again: {why}"))); | ||
said.extend(stuck.map(|why| format!("warning: {org}'s gateway did not take you out of its spaces, so you may still read its private repositories until it does, which `knotless org leave {org}` asks again: {why:#}"))); | ||
} | ||
"members" => { | ||
let asked = words.first().copied().or(named); | ||
| @@ -2174,7 +2174,8 @@ async fn command( | ||
bail!("--check takes none of --queue, --keep-branch, --wait and --force"); | ||
} | ||
let me = session::me(ctx).await?; | ||
let found = crate::landing::mergeable(ctx, &url, pull, &me.did).await?; | ||
let words = crate::fix::Words::Cli; | ||
let found = crate::landing::mergeable(ctx, &url, pull, &me.did, words).await?; | ||
return said_mergeable(out, &found); | ||
} | ||
if queue { | ||
crates/knotless/src/fix.rsnew+304−0
| @@ -0,0 +1,304 @@ | ||
//! What to do about a refusal, which each client says in its own words: | ||
//! the command line as a command to run, MCP as a tool to call, and the | ||
//! pages (and the iOS app) as the control that does it there, or not at | ||
//! all where they have none. A refusal names what to do as a [`Fix`], | ||
//! never as a command in its text, so the pages never tell anyone to run | ||
//! one. | ||
| ||
/// Whom a refusal is said to. | ||
#[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
pub enum Words { | ||
/// The command line, and logs: commands to run. | ||
Cli, | ||
/// MCP: tools to call. | ||
Tools, | ||
/// The pages and the app: controls to use. | ||
Pages, | ||
} | ||
| ||
/// Something to do about a refusal, by whom. | ||
#[derive(Debug, Clone, PartialEq, Eq)] | ||
pub enum Fix { | ||
/// Anyone but a pull's author who may write approves it. | ||
Approve { url: String, pull: String }, | ||
/// The owner merges a pull past its checks and reviews. | ||
Force, | ||
/// A draft's author marks it ready. | ||
Ready { url: String, pull: String }, | ||
/// Merge a pull directly. | ||
Merge, | ||
/// Put a pull in the merge queue. | ||
Queue { url: String, pull: String }, | ||
/// Turn the merge queue on for a branch. | ||
UseQueue { url: String, branch: String }, | ||
/// Name the account a runner reports as. | ||
AddRunner { url: String }, | ||
/// The owner asks a pull's runners to run its checks. | ||
Rerun { url: String, pull: String }, | ||
/// Wait for checks still running, or not yet run. | ||
Wait, | ||
/// Read a pull's checks' logs. | ||
Logs { url: String, pull: String }, | ||
/// The owner defines a label. | ||
DefineLabel, | ||
/// Delegate your writes to a gateway. | ||
Delegate, | ||
/// Whoever holds an organization's login delegates its writes. | ||
DelegateOrg { org: String }, | ||
/// Join an organization in public. | ||
JoinPublic { org: String }, | ||
/// Show a hidden membership of an organization. | ||
ShowMembership { org: String }, | ||
/// Log in again. | ||
Login, | ||
} | ||
| ||
impl Fix { | ||
/// Who does what, and how, in `words`: none where they have no way to. | ||
pub fn said(&self, words: Words) -> Option<String> { | ||
match words { | ||
Words::Cli => Some(format!("{} with {}", self.who(), self.command())), | ||
Words::Tools => Some(format!("{} with {}", self.who(), self.tool())), | ||
Words::Pages => self.control().map(str::to_string), | ||
} | ||
} | ||
| ||
/// Who does what, for the command line and MCP. | ||
fn who(&self) -> &'static str { | ||
match self { | ||
Fix::Approve { .. } => "anyone but its author who may write approves it", | ||
Fix::Force => "the owner, its author or not, merges past it", | ||
Fix::Ready { .. } => "its author marks it ready", | ||
Fix::Merge => "merge it", | ||
Fix::Queue { .. } => "queue it", | ||
Fix::UseQueue { .. } => "turn the merge queue on", | ||
Fix::AddRunner { .. } => "name the account a runner reports as", | ||
Fix::Rerun { .. } => "the owner asks for them", | ||
Fix::Wait => "wait for them", | ||
Fix::Logs { .. } => "read their logs", | ||
Fix::DefineLabel => "the owner defines them", | ||
Fix::Delegate => "delegate your writes to one first", | ||
Fix::DelegateOrg { .. } => "whoever holds its login delegates its writes", | ||
Fix::JoinPublic { .. } => "join it in public", | ||
Fix::ShowMembership { .. } => "show your membership", | ||
Fix::Login => "log in", | ||
} | ||
} | ||
| ||
/// The command that does it. | ||
fn command(&self) -> String { | ||
match self { | ||
Fix::Approve { url, pull } => { | ||
format!("`knotless comment add {url} {pull} <text> --approve`") | ||
} | ||
Fix::Force => "`--force`".into(), | ||
Fix::Ready { url, pull } => format!("`knotless pull edit {url} {pull} --ready`"), | ||
Fix::Merge => "`knotless pull merge`".into(), | ||
Fix::Queue { url, pull } => format!("`knotless pull merge {url} {pull} --queue`"), | ||
Fix::UseQueue { url, branch } => { | ||
format!("`knotless branch protect {url} {branch} --queue`") | ||
} | ||
Fix::AddRunner { url } => format!("`knotless runner add {url} <handle>`"), | ||
Fix::Rerun { url, pull } => format!("`knotless run rerun {url} --pull {pull}`"), | ||
Fix::Wait => "`--wait <secs>`".into(), | ||
Fix::Logs { url, pull } => format!("`knotless run list {url} --pull {pull} --log`"), | ||
Fix::DefineLabel => "`knotless label define`".into(), | ||
Fix::Delegate => "`knotless account delegate`".into(), | ||
Fix::DelegateOrg { org } => format!("`knotless account delegate {org}`"), | ||
Fix::JoinPublic { org } | Fix::ShowMembership { org } => { | ||
format!("`knotless org join {org} --public`") | ||
} | ||
Fix::Login => "`knotless login <handle>`".into(), | ||
} | ||
} | ||
| ||
/// The tool that does it. | ||
fn tool(&self) -> &'static str { | ||
match self { | ||
Fix::Approve { .. } => "the comment_add tool and approve", | ||
Fix::Force => "the pull_merge tool and force", | ||
Fix::Ready { .. } => "the pull_edit tool and ready", | ||
Fix::Merge => "the pull_merge tool", | ||
Fix::Queue { .. } => "the pull_merge tool and queue", | ||
Fix::UseQueue { .. } => "the branch_protect tool and queue", | ||
Fix::AddRunner { .. } => "the runner_add tool", | ||
Fix::Rerun { .. } => "the run_rerun tool and pull", | ||
Fix::Wait => "the pull_merge tool and wait", | ||
Fix::Logs { .. } => "the run_list tool, pull and log", | ||
Fix::DefineLabel => "the label_define tool", | ||
Fix::Delegate | Fix::DelegateOrg { .. } => "the account_delegate tool", | ||
Fix::JoinPublic { .. } | Fix::ShowMembership { .. } => "the org_join tool and public", | ||
Fix::Login => "`knotless login <handle>` in a terminal", | ||
} | ||
} | ||
| ||
/// What does it on the pages, if anything does. | ||
fn control(&self) -> Option<&'static str> { | ||
Some(match self { | ||
Fix::Approve { .. } => { | ||
"anyone but its author who may write approves it with Approve, under Review" | ||
} | ||
Fix::Force | Fix::Wait | Fix::Logs { .. } => return None, | ||
Fix::Ready { .. } => "its author marks it ready with Ready for review", | ||
Fix::Merge => "merge it with Merge pull", | ||
Fix::Queue { .. } => "queue it with Add to merge queue", | ||
Fix::UseQueue { .. } => { | ||
"turn the merge queue on with Require the merge queue, under Branches in the repository's settings" | ||
} | ||
Fix::AddRunner { .. } => { | ||
"name the account a runner reports as with Add runner, under Actions in the repository's settings" | ||
} | ||
Fix::Rerun { .. } => "the owner asks for them with Re-run", | ||
Fix::DefineLabel => { | ||
"the owner defines them with New label, under Labels in the repository's settings" | ||
} | ||
Fix::Delegate => { | ||
"delegate your writes to one first with Delegate writes to this server, under Delegated writes in your settings" | ||
} | ||
Fix::DelegateOrg { .. } => { | ||
"whoever holds its login delegates its writes with Delegate writes to this server, under Delegated writes in its settings" | ||
} | ||
Fix::JoinPublic { .. } => "join it in public with Join publicly", | ||
Fix::ShowMembership { .. } => { | ||
"show your membership with Show my membership, on its members page" | ||
} | ||
Fix::Login => "log in again", | ||
}) | ||
} | ||
} | ||
| ||
/// A refusal: why, then what to do about it, which [`std::fmt::Display`] | ||
/// says as the command line does, and [`Refused::said`] in any | ||
/// [`Words`]. | ||
#[derive(Debug, Clone, PartialEq, Eq)] | ||
pub struct Refused { | ||
/// What stands in the way, said the same to everyone. | ||
pub why: String, | ||
pub fixes: Vec<Fix>, | ||
} | ||
| ||
impl Refused { | ||
pub fn new(why: impl Into<String>, fixes: impl IntoIterator<Item = Fix>) -> Refused { | ||
Refused { | ||
why: why.into(), | ||
fixes: fixes.into_iter().collect(), | ||
} | ||
} | ||
| ||
/// Why, then what to do, each that `words` has a way to. | ||
pub fn said(&self, words: Words) -> String { | ||
let fixes: Vec<String> = self.fixes.iter().filter_map(|f| f.said(words)).collect(); | ||
let fixes = fixes.join("; "); | ||
let mut chars = fixes.chars(); | ||
match chars.next() { | ||
None => self.why.clone(), | ||
Some(first) => format!("{}. {}{}", self.why, first.to_uppercase(), chars.as_str()), | ||
} | ||
} | ||
} | ||
| ||
impl std::fmt::Display for Refused { | ||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { | ||
f.write_str(&self.said(Words::Cli)) | ||
} | ||
} | ||
| ||
impl std::error::Error for Refused {} | ||
| ||
/// `err` as `{err:#}` says it, but with what to do about the refusal in it, | ||
/// if it holds one, said in `words`. | ||
pub fn said(err: &anyhow::Error, words: Words) -> String { | ||
let all = format!("{err:#}"); | ||
match err.downcast_ref::<Refused>() { | ||
Some(refused) => all.replacen(&refused.to_string(), &refused.said(words), 1), | ||
None => all, | ||
} | ||
} | ||
| ||
#[cfg(test)] | ||
mod tests { | ||
use super::*; | ||
use anyhow::Context; | ||
| ||
fn every() -> Vec<Fix> { | ||
let (url, pull) = ("knotless://a.test/r".to_string(), "at://p".to_string()); | ||
let org = "o.test".to_string(); | ||
vec![ | ||
Fix::Approve { | ||
url: url.clone(), | ||
pull: pull.clone(), | ||
}, | ||
Fix::Force, | ||
Fix::Ready { | ||
url: url.clone(), | ||
pull: pull.clone(), | ||
}, | ||
Fix::Merge, | ||
Fix::Queue { | ||
url: url.clone(), | ||
pull: pull.clone(), | ||
}, | ||
Fix::UseQueue { | ||
url: url.clone(), | ||
branch: "main".into(), | ||
}, | ||
Fix::AddRunner { url: url.clone() }, | ||
Fix::Rerun { | ||
url: url.clone(), | ||
pull: pull.clone(), | ||
}, | ||
Fix::Wait, | ||
Fix::Logs { url, pull }, | ||
Fix::DefineLabel, | ||
Fix::Delegate, | ||
Fix::DelegateOrg { org: org.clone() }, | ||
Fix::JoinPublic { org: org.clone() }, | ||
Fix::ShowMembership { org }, | ||
Fix::Login, | ||
] | ||
} | ||
| ||
#[test] | ||
fn the_pages_name_no_command() { | ||
for fix in every() { | ||
let command = fix.said(Words::Cli).unwrap(); | ||
assert!(command.contains(" with `"), "{command}"); | ||
let tool = fix.said(Words::Tools).unwrap(); | ||
let named = tool.contains(" tool") || fix == Fix::Login; | ||
assert!(named && !tool.contains("--"), "{tool}"); | ||
if let Some(page) = fix.said(Words::Pages) { | ||
assert!(!page.contains('`') && !page.contains("--"), "{page}"); | ||
assert!(!page.contains("knotless "), "{page}"); | ||
} | ||
} | ||
} | ||
| ||
#[test] | ||
fn a_refusal_says_what_to_do_its_readers_can() { | ||
let url = "knotless://a.test/r".to_string(); | ||
let refused = Refused::new("r names no runners", [Fix::Merge, Fix::AddRunner { url }]); | ||
assert_eq!( | ||
refused.to_string(), | ||
"r names no runners. Merge it with `knotless pull merge`; name the account a runner reports as with `knotless runner add knotless://a.test/r <handle>`" | ||
); | ||
// Only what the pages can do is said there, and with nothing, only why. | ||
let forced = Refused::new("checks failed", [Fix::Force, Fix::Merge]); | ||
assert_eq!( | ||
forced.said(Words::Pages), | ||
"checks failed. Merge it with Merge pull" | ||
); | ||
let only = Refused::new("checks failed", [Fix::Force]); | ||
assert_eq!(only.said(Words::Pages), "checks failed"); | ||
// Under context, a cause or the context itself. | ||
let err = anyhow::Error::from(forced.clone()).context("merging"); | ||
assert_eq!( | ||
said(&err, Words::Pages), | ||
"merging: checks failed. Merge it with Merge pull" | ||
); | ||
let held: anyhow::Result<()> = Err(anyhow::anyhow!("no token")); | ||
let err = held.context(only).unwrap_err(); | ||
assert_eq!(said(&err, Words::Pages), "checks failed: no token"); | ||
let plain = anyhow::anyhow!("no such repository"); | ||
assert_eq!(said(&plain, Words::Pages), "no such repository"); | ||
} | ||
} | ||
crates/knotless/src/forge.rs+41−27
| @@ -19,6 +19,7 @@ use crate::checks::{self, Standing}; | ||
use crate::closing; | ||
use crate::env::Ctx; | ||
use crate::fetch; | ||
use crate::fix::{Fix, Refused}; | ||
use crate::gateway; | ||
use crate::git::Git; | ||
use crate::identity; | ||
| @@ -2065,10 +2066,8 @@ pub async fn label( | ||
let mut defined = Vec::new(); | ||
for name in names { | ||
let Some(label) = t.record.label(name) else { | ||
bail!( | ||
"{}: no label {name}: the owner defines them with `knotless label define`", | ||
t.url | ||
); | ||
let why = format!("{}: no label {name}", t.url); | ||
bail!(Refused::new(why, [Fix::DefineLabel])); | ||
}; | ||
defined.push(label); | ||
} | ||
| @@ -2331,10 +2330,12 @@ pub struct Gate { | ||
} | ||
| ||
/// Why a draft pull neither merges nor goes in the merge queue. | ||
pub(crate) fn drafted(url: &RepoUrl, pull: &str) -> String { | ||
format!( | ||
"{pull} is a draft: its author marks it ready with `knotless pull edit {url} {pull} --ready`" | ||
) | ||
pub(crate) fn drafted(url: &RepoUrl, pull: &str) -> Refused { | ||
let ready = Fix::Ready { | ||
url: url.to_string(), | ||
pull: pull.to_string(), | ||
}; | ||
Refused::new(format!("{pull} is a draft"), [ready]) | ||
} | ||
| ||
/// Merges a pull: a merge commit, naming the pull, even where a | ||
| @@ -2370,14 +2371,18 @@ pub async fn merge( | ||
bail!("{pull} is {}", item.state); | ||
} | ||
if item.draft { | ||
bail!("{}", drafted(target, pull)); | ||
bail!(drafted(target, pull)); | ||
} | ||
let into = item.base.as_deref().unwrap_or_default(); | ||
if t.record.queue_only(into) && !gate.force { | ||
if t.record.runners.is_empty() { | ||
bail!( | ||
"{into} takes pulls only through the merge queue, and {target} names no runners to land it: name one with `knotless runner add {target} <handle>`, or, as its owner, merge with --force" | ||
let why = format!( | ||
"{into} takes pulls only through the merge queue, and {target} names no runners to land it" | ||
); | ||
let runner = Fix::AddRunner { | ||
url: target.to_string(), | ||
}; | ||
bail!(Refused::new(why, [runner, Fix::Force])); | ||
} | ||
return Err(QueueOnly(into.to_string()).into()); | ||
} | ||
| @@ -2404,7 +2409,7 @@ pub async fn merge( | ||
false => approvals::of(ctx, t.facts(), &item, git, base, head).await?, | ||
}; | ||
if let Some(why) = reviewed.and_then(|s| approvals::refusal(target, pull, branch, &s)) { | ||
bail!("{why}"); | ||
bail!(why); | ||
} | ||
if !(gate.force || t.record.runners.is_empty()) { | ||
let at = match fast { | ||
| @@ -2515,7 +2520,7 @@ async fn checked( | ||
} | ||
let left = deadline.saturating_duration_since(Instant::now()); | ||
if !pending || left.is_zero() { | ||
bail!("{why}"); | ||
bail!(why); | ||
} | ||
tokio::time::sleep(left.min(every)).await; | ||
} | ||
| @@ -2529,7 +2534,7 @@ pub(crate) fn refusal( | ||
sides: &Sides, | ||
standing: &Standing, | ||
found: &[checks::Check], | ||
) -> Option<String> { | ||
) -> Option<Refused> { | ||
let short = |sha: &str| sha.get(..12).unwrap_or(sha).to_string(); | ||
let (head, base) = (&sides.head, &sides.base); | ||
let branch = sides.refname.trim_start_matches("refs/heads/"); | ||
| @@ -2538,22 +2543,27 @@ pub(crate) fn refusal( | ||
short(head), | ||
short(base) | ||
); | ||
let why = match standing { | ||
let (url, pull) = (url.to_string(), pull.to_string()); | ||
let (why, mut fixes) = match standing { | ||
Standing::Passed => return None, | ||
Standing::Failed(failed) => { | ||
let named: Vec<_> = failed | ||
.iter() | ||
.map(|(w, how)| format!("{w} ({how})")) | ||
.collect(); | ||
format!( | ||
let why = format!( | ||
"{pull} did not pass its checks at {at}: {}", | ||
named.join(", ") | ||
) | ||
); | ||
(why, Vec::new()) | ||
} | ||
Standing::Running(names) => { | ||
let why = format!( | ||
"{pull}'s checks at {at} are still running: {}", | ||
names.join(", ") | ||
); | ||
(why, vec![Fix::Wait]) | ||
} | ||
Standing::Running(names) => format!( | ||
"{pull}'s checks at {at} are still running: {}; `--wait <secs>` waits for them", | ||
names.join(", ") | ||
), | ||
Standing::Stale(names) => { | ||
// The newest run of one of them, at some other head or base. | ||
let older = found.iter().map(|c| &c.record).find(|r| { | ||
| @@ -2570,15 +2580,19 @@ pub(crate) fn refusal( | ||
format!("the newest are of {branch} at {}", short(tip)) | ||
} | ||
}; | ||
format!( | ||
"{pull} has no checks at {at} of {}: {then}. It is not checked yet: its runners run a pull by the owner or a collaborator when it moves, and anyone else's once the owner runs it with `knotless run rerun {url} --pull {pull}`; `--wait <secs>` waits for it", | ||
let why = format!( | ||
"{pull} has no checks at {at} of {}: {then}. It is not checked yet: its runners run a pull by the owner or a collaborator when it moves, and anyone else's once the owner asks", | ||
names.join(", ") | ||
) | ||
); | ||
let rerun = Fix::Rerun { | ||
url: url.clone(), | ||
pull: pull.clone(), | ||
}; | ||
(why, vec![rerun, Fix::Wait]) | ||
} | ||
}; | ||
Some(format!( | ||
"{why}. Their logs: `knotless run list {url} --pull {pull} --log`; the owner merges past them with `--force`" | ||
)) | ||
fixes.extend([Fix::Logs { url, pull }, Fix::Force]); | ||
Some(Refused::new(why, fixes)) | ||
} | ||
| ||
/// The head branch of `item`, once merged, when nothing needs it any more: | ||
crates/knotless/src/landing.rs+47−22
| @@ -7,6 +7,7 @@ | ||
use crate::approvals; | ||
use crate::checks::{self, Standing}; | ||
use crate::env::Ctx; | ||
use crate::fix::{Refused, Words}; | ||
use crate::forge::{self, Facts, Item, Sides, Target}; | ||
use crate::git::Git; | ||
use crate::mirror::Mirror; | ||
| @@ -179,10 +180,16 @@ pub fn may(t: Facts, items: &[Item], item: &Item, who: &str) -> May { | ||
} | ||
| ||
/// Whether the open pull `pull` on `target` can merge now, as | ||
/// [`forge::merge`] with no wait would decide, and what `who` may do to it. | ||
/// It reads what the merge reads, the pull's checks among them, and writes | ||
/// nothing. | ||
pub async fn mergeable(ctx: &Ctx, target: &RepoUrl, pull: &str, who: &str) -> Result<Mergeable> { | ||
/// [`forge::merge`] with no wait would decide, and what `who` may do to it, | ||
/// why not said in `words`. It reads what the merge reads, the pull's | ||
/// checks among them, and writes nothing. | ||
pub async fn mergeable( | ||
ctx: &Ctx, | ||
target: &RepoUrl, | ||
pull: &str, | ||
who: &str, | ||
words: Words, | ||
) -> Result<Mergeable> { | ||
let t = Target::open(ctx, target).await?; | ||
let pull = t.named(pull); | ||
let pull = pull.as_str(); | ||
| @@ -204,24 +211,27 @@ pub async fn mergeable(ctx: &Ctx, target: &RepoUrl, pull: &str, who: &str) -> Re | ||
checks::list(ctx, &t.url, filter, false).await? | ||
} | ||
}; | ||
Ok(decide(ctx, t.facts(), &item, &sides, gauged, &checks, who).await) | ||
Ok(decide(ctx, t.facts(), &item, &sides, gauged, &checks, who, words).await) | ||
} | ||
| ||
/// Why `who` may not merge on `t`: a hidden member of the organization it | ||
/// is, that merging takes a public membership, in the words a merge would | ||
/// refuse with; anyone else, that only its owner and collaborators do. | ||
async fn not_theirs(ctx: &Ctx, t: Facts<'_>, who: &str) -> String { | ||
async fn not_theirs(ctx: &Ctx, t: Facts<'_>, who: &str) -> Refused { | ||
let me = crate::session::me(ctx) | ||
.await | ||
.ok() | ||
.filter(|me| me.did == who); | ||
if let Some(me) = me | ||
&& let Err(e) = crate::org::needs_public(ctx, t, &me, "merging").await | ||
&& let Err(refused) = crate::org::needs_public(ctx, t, &me, "merging").await | ||
{ | ||
return e.to_string(); | ||
return refused; | ||
} | ||
let target = t.url; | ||
format!("you may not merge it: only the owner and collaborators of {target} merge its pulls") | ||
let why = format!( | ||
"you may not merge it: only the owner and collaborators of {target} merge its pulls" | ||
); | ||
Refused::new(why, []) | ||
} | ||
| ||
/// Where an open pull stands before its checks are read, as [`gauge`] | ||
| @@ -229,6 +239,8 @@ async fn not_theirs(ctx: &Ctx, t: Facts<'_>, who: &str) -> String { | ||
/// when it waits for none, as when it conflicts. | ||
pub struct Gauged { | ||
found: Mergeable, | ||
/// Why it may not merge, so far. | ||
refused: Option<Refused>, | ||
pub workflows: Vec<String>, | ||
} | ||
| ||
| @@ -268,8 +280,9 @@ pub async fn gauge( | ||
queue_only: t.record.queue_only(branch), | ||
auto_merge: AutoMerged::of(t, item, head), | ||
}; | ||
let none = |found| Gauged { | ||
let none = |found, refused| Gauged { | ||
found, | ||
refused, | ||
workflows: Vec::new(), | ||
}; | ||
let since = forge::since_of(git, item, base, head).await?; | ||
| @@ -277,37 +290,43 @@ pub async fn gauge( | ||
Ok(tree) => tree, | ||
Err(files) => { | ||
found.state = "conflicting"; | ||
found.reason = Some(format!("merging conflicts in {}", files.join(", "))); | ||
let why = format!("merging conflicts in {}", files.join(", ")); | ||
found.conflicts = files.into_iter().map(|path| Conflict { path }).collect(); | ||
return Ok(none(found)); | ||
return Ok(none(found, Some(Refused::new(why, [])))); | ||
} | ||
}; | ||
let notes = | ||
|all: Vec<String>| -> Vec<Note> { all.into_iter().map(|text| Note { text }).collect() }; | ||
let mut refused = None; | ||
if let Some(s) = approvals::of(ctx, t, item, git, base, head).await? { | ||
found.reviews = if s.passed() { "passed" } else { "blocked" }; | ||
found.reason = approvals::refusal(t.url, pull, branch, &s); | ||
refused = approvals::refusal(t.url, pull, branch, &s); | ||
found.reviews_missing = notes(s.missing()); | ||
found.reviews_uncounted = notes(s.uncounted()); | ||
found.code_owners_warnings = notes(s.warnings); | ||
} | ||
if found.reason.is_some() { | ||
if refused.is_some() { | ||
found.state = "blocked"; | ||
} | ||
if t.record.runners.is_empty() { | ||
return Ok(none(found)); | ||
return Ok(none(found, refused)); | ||
} | ||
let at = match git.is_ancestor(base, head).await? { | ||
true => head.clone(), | ||
false => forge::checked_merge(git, &tree, base, head).await?, | ||
}; | ||
let workflows = workflow::on_pull(git, &at, &sides.refname).await?; | ||
Ok(Gauged { found, workflows }) | ||
Ok(Gauged { | ||
found, | ||
refused, | ||
workflows, | ||
}) | ||
} | ||
| ||
/// What [`gauge`] found of the open pull `item` on `t`, with where `checks`, | ||
/// the pull's own newest first, leave it, and who may merge it: as | ||
/// [`mergeable`] says. | ||
/// [`mergeable`] says, why not in `words`. | ||
#[allow(clippy::too_many_arguments)] | ||
pub async fn decide( | ||
ctx: &Ctx, | ||
t: Facts<'_>, | ||
| @@ -316,9 +335,11 @@ pub async fn decide( | ||
gauged: Gauged, | ||
checks: &[checks::Check], | ||
who: &str, | ||
words: Words, | ||
) -> Mergeable { | ||
let Gauged { | ||
mut found, | ||
mut refused, | ||
workflows, | ||
} = gauged; | ||
if !workflows.is_empty() { | ||
| @@ -326,22 +347,26 @@ pub async fn decide( | ||
found.checks = standing.name(); | ||
// A merge refuses for its reviews before it reads its checks. | ||
let checked = forge::refusal(t.url, &item.uri, sides, &standing, checks); | ||
found.reason = found.reason.or(checked); | ||
refused = refused.or(checked); | ||
if standing != Standing::Passed { | ||
found.state = "blocked"; | ||
} | ||
} | ||
if item.draft { | ||
found.state = "draft"; | ||
found.reason = Some(forge::drafted(t.url, &item.uri)); | ||
refused = Some(forge::drafted(t.url, &item.uri)); | ||
} | ||
if !found.may.merge { | ||
let not = not_theirs(ctx, t, who).await; | ||
found.reason = Some(match found.reason { | ||
Some(why) => format!("{why}; and {not}"), | ||
refused = Some(match refused { | ||
Some(why) => { | ||
let fixes = why.fixes.into_iter().chain(not.fixes); | ||
Refused::new(format!("{}; and {}", why.why, not.why), fixes) | ||
} | ||
None => not, | ||
}); | ||
} | ||
found.reason = refused.map(|r| r.said(words)); | ||
found | ||
} | ||
| ||
| @@ -386,7 +411,7 @@ pub async fn gate_refusal( | ||
let why = "protected branch: changes land on it only through reviewed pulls; push to another branch and open a pull"; | ||
return Ok(reviewed.then(|| why.to_string())); | ||
}; | ||
let found = mergeable(ctx, &t.url, &pull, pusher).await?; | ||
let found = mergeable(ctx, &t.url, &pull, pusher, Words::Cli).await?; | ||
let blocked = found.reason.filter(|_| found.state != "clean"); | ||
Ok(blocked.map(|why| format!("{said}: {why}"))) | ||
} | ||
crates/knotless/src/lib.rs+1−0
| @@ -28,6 +28,7 @@ pub mod drisl; | ||
pub mod emails; | ||
pub mod env; | ||
pub mod fetch; | ||
pub mod fix; | ||
pub mod forge; | ||
pub mod gateway; | ||
pub mod git; | ||
crates/knotless/src/manage.rs+15−8
| @@ -2,6 +2,7 @@ | ||
//! `knotless` command and the MCP server both do. | ||
| ||
use crate::env::Ctx; | ||
use crate::fix::{Fix, Refused}; | ||
use crate::gateway; | ||
use crate::identity; | ||
use crate::keys; | ||
| @@ -387,7 +388,9 @@ pub async fn list(ctx: &Ctx, owner: &str) -> Result<Vec<Listed>> { | ||
} | ||
| ||
/// Why sharing for writing needs a gateway, which the owner delegates to. | ||
const NO_GATEWAY: &str = "collaborators push through a gateway: delegate your writes to one first (`knotless account delegate`, or the `account_delegate` tool)"; | ||
fn undelegated() -> Refused { | ||
Refused::new("collaborators push through a gateway", [Fix::Delegate]) | ||
} | ||
| ||
/// Lets `who` read a private repository, and with `write`, push to any | ||
/// repository through its gateway, once its owner has delegated their | ||
| @@ -408,11 +411,13 @@ pub async fn share_delegating<W: std::io::Write + ?Sized>( | ||
out: &mut W, | ||
) -> Result<Option<String>> { | ||
let shared = set_access(ctx, url, who, Some(write)).await; | ||
let undelegated = shared.as_ref().is_err_and(|e| e.to_string() == NO_GATEWAY); | ||
if !undelegated { | ||
let delegating = shared | ||
.as_ref() | ||
.is_err_and(|e| e.downcast_ref() == Some(&undelegated())); | ||
if !delegating { | ||
return shared.map(|()| None); | ||
} | ||
let gateway = gateway::appview(ctx).await.context(NO_GATEWAY)?; | ||
let gateway = gateway::appview(ctx).await.context(undelegated())?; | ||
let owner = Remote::open(ctx, url).await?; | ||
let me = session::ensure(ctx, &owner.identity).await?; | ||
gateway::delegate(ctx, &me, &gateway, out).await?; | ||
| @@ -497,7 +502,7 @@ async fn set_access(ctx: &Ctx, url: &RepoUrl, who: &str, access: Option<bool>) - | ||
match gateway::delegated(ctx, remote.did()) { | ||
Some(gateway) => record.gateway = Some(gateway), | ||
None if record.gateway.is_some() => {} | ||
None => bail!(NO_GATEWAY), | ||
None => bail!(undelegated()), | ||
} | ||
} | ||
let written = vec![remote::update( | ||
| @@ -934,9 +939,11 @@ impl Asking { | ||
bail!("{url}: no such repository"); | ||
}; | ||
if repo.value.runners.is_empty() { | ||
bail!( | ||
"{url} names no runners to ask: name the account a runner reports as with `knotless runner add {url} <handle>`" | ||
); | ||
let why = format!("{url} names no runners to ask"); | ||
let runner = Fix::AddRunner { | ||
url: url.to_string(), | ||
}; | ||
bail!(Refused::new(why, [runner])); | ||
} | ||
// A branch or tag may be named bare, as git takes it; a ref the | ||
// repository does not have would be asked of no runner. | ||
crates/knotless/src/org.rs+24−16
| @@ -31,6 +31,7 @@ | ||
| ||
use crate::auth; | ||
use crate::env::Ctx; | ||
use crate::fix::{Fix, Refused}; | ||
use crate::forge::Facts; | ||
use crate::gateway; | ||
use crate::identity::{self, Identity}; | ||
| @@ -457,7 +458,12 @@ fn pusher(did: String, shared: bool) -> OrgMemberRecord { | ||
/// there take a public membership, as those reading it cannot see a hidden | ||
/// one, and would pass over what they did. An appview that does not answer | ||
/// refuses nothing. | ||
pub async fn needs_public(ctx: &Ctx, t: Facts<'_>, me: &Session, what: &str) -> Result<()> { | ||
pub async fn needs_public( | ||
ctx: &Ctx, | ||
t: Facts<'_>, | ||
me: &Session, | ||
what: &str, | ||
) -> std::result::Result<(), Refused> { | ||
let did = me.did.as_str(); | ||
if t.private || did == t.owner || t.record.is_collaborator(did) { | ||
return Ok(()); | ||
| @@ -466,9 +472,11 @@ pub async fn needs_public(ctx: &Ctx, t: Facts<'_>, me: &Session, what: &str) -> | ||
let hidden = |r: &&Roster| r.entry(did).is_some_and(|e| e.standing == Standing::Hidden); | ||
if let Some(roster) = roster.as_ref().filter(hidden) { | ||
let name = &roster.name; | ||
bail!( | ||
"{what} on {name}'s public repositories takes a public membership, as those reading them cannot see a hidden one: `knotless org join {name} --public`" | ||
let why = format!( | ||
"{what} on {name}'s public repositories takes a public membership, as those reading them cannot see a hidden one" | ||
); | ||
let show = Fix::ShowMembership { org: name.clone() }; | ||
return Err(Refused::new(why, [show])); | ||
} | ||
Ok(()) | ||
} | ||
| @@ -854,7 +862,7 @@ pub async fn admit_as( | ||
let own = Own::open(ctx, org).await?; | ||
let name = own.roster.name.clone(); | ||
let Some(entry) = own.roster.entry(who).cloned() else { | ||
bail!("{name} has not invited you: ask one of its admins to (`knotless org add`)"); | ||
bail!("{name} has not invited you: ask one of its admins to invite you"); | ||
}; | ||
// Read at once: their join record, wherever they wrote it, and its | ||
// record of them, in public and in its space. | ||
| @@ -877,9 +885,7 @@ pub async fn admit_as( | ||
let visibility = match (open_join, hidden) { | ||
(_, true) => Visibility::Hidden, | ||
(true, false) => Visibility::Public, | ||
(false, false) => bail!( | ||
"you have not joined {name}: write your join record first (`knotless org join {name}`)" | ||
), | ||
(false, false) => bail!("you have not joined {name}: join it first"), | ||
}; | ||
if entry.joined_at.is_none() && lapsed(&entry.invited_at, entry.expires_at.as_deref()) { | ||
bail!("your invitation to {name} lapsed: ask one of its admins to invite you again"); | ||
| @@ -955,11 +961,9 @@ async fn ask<T: serde::de::DeserializeOwned>( | ||
body: Value, | ||
) -> Result<T> { | ||
let Some(gateway) = &roster.gateway else { | ||
bail!( | ||
"{} acts through no gateway: whoever holds its login runs `knotless account delegate {}`", | ||
roster.name, | ||
roster.name | ||
); | ||
let why = format!("{} acts through no gateway", roster.name); | ||
let org = roster.name.clone(); | ||
bail!(Refused::new(why, [Fix::DelegateOrg { org }])); | ||
}; | ||
let (xrpc, auth) = gateway::client(ctx, me, gateway, lxm).await?; | ||
xrpc.procedure(lxm, Some(&body), Some(&auth)).await | ||
| @@ -1146,10 +1150,14 @@ pub async fn join(ctx: &Ctx, org: &str, visibility: Visibility) -> Result<Joined | ||
}; | ||
let shut = wrote.as_ref().is_err_and(space::refused); | ||
if visibility == Visibility::Hidden && shut { | ||
bail!( | ||
"{} has no invitation for you in its space, or its PDS has no spaces, where a hidden membership is kept: ask its admins, or join with --public", | ||
let why = format!( | ||
"{} has no invitation for you in its space, or its PDS has no spaces, where a hidden membership is kept: ask its admins", | ||
roster.name | ||
); | ||
let public = Fix::JoinPublic { | ||
org: roster.name.clone(), | ||
}; | ||
bail!(Refused::new(why, [public])); | ||
} | ||
wrote?; | ||
let body = json!({"org": org_id.did, "visibility": visibility}); | ||
| @@ -1172,7 +1180,7 @@ pub async fn join(ctx: &Ctx, org: &str, visibility: Visibility) -> Result<Joined | ||
/// organization's gateway take them out of its spaces. Answers why it could | ||
/// not, if it could not: leaving again asks again, as it does to decline an | ||
/// invitation. | ||
pub async fn leave(ctx: &Ctx, org: &str) -> Result<(String, Option<String>)> { | ||
pub async fn leave(ctx: &Ctx, org: &str) -> Result<(String, Option<anyhow::Error>)> { | ||
let me = session::me(ctx).await?; | ||
let mine = identity::resolve(ctx, &me.did).await?; | ||
let org_id = identity::resolve(ctx, org).await?; | ||
| @@ -1205,7 +1213,7 @@ pub async fn leave(ctx: &Ctx, org: &str) -> Result<(String, Option<String>)> { | ||
ask(ctx, &me, &roster, gateway::LEAVE_ORG, body).await | ||
} | ||
.await; | ||
Ok((name, asked.err().map(|e| format!("{e:#}")))) | ||
Ok((name, asked.err())) | ||
} | ||
| ||
/// The organizations `who` (yours when none) is a member of, by DID, as the | ||