chadtmiller.com/knotless
git that lives in your Atmosphere account
1
10
The reviewer watches every repository an owner holds
.changeset/review-owner.mdnew+7−0
| @@ -0,0 +1,7 @@ | ||
--- | ||
default: minor | ||
--- | ||
| ||
# The reviewer can watch every repository an account holds | ||
| ||
`knotless-agent review --owner <handle>` reviews the pulls of every public repository that account holds, listed again at each read, so a repository made later is reviewed without restarting the agent. Repeat `--owner` for more accounts, and name repositories besides as before. | ||
.knotless/workflows/deploy.yml+3−2
| @@ -54,8 +54,9 @@ env: | ||
KNOTLESS_TRIAGE_ARGS: --model clef --scopes kind,area --discover | ||
# Reviews pulls with Claude Code, as review.knotless.dev, once | ||
# /etc/knotless/review.env holds its logins (deploy/deploy.sh): every | ||
# open pull by the owner or a collaborator, asked or not (--all). | ||
KNOTLESS_REVIEW_REPOS: chadtmiller.com/knotless | ||
# open pull by the owner or a collaborator, asked or not (--all), in | ||
# every repository the owner holds (--owner). | ||
KNOTLESS_REVIEW_REPOS: --owner chadtmiller.com | ||
KNOTLESS_REVIEW_ARGS: --all | ||
steps: | ||
- name: tools | ||
crates/knotless-agent/src/lib.rs+101−16
| @@ -18,6 +18,10 @@ | ||
//! With `--all` it reviews every open pull that is not a draft, asked or | ||
//! not, as a GitHub workflow that reviews each pull does. | ||
//! | ||
//! With `--owner <handle|did>` it watches, besides the repositories named, | ||
//! every public repository that account holds: listed from its PDS at | ||
//! each read, so one made meanwhile is watched from then on. | ||
//! | ||
//! A pull is reviewed once at each head, at most `--rounds` times, and only | ||
//! one its repository's owner or a collaborator opened: anyone may ask any | ||
//! account for a review, and each spends what the command costs its | ||
| @@ -27,14 +31,16 @@ | ||
//! Jetstream is only the doorbell, as it is for knotless-triage: a pull | ||
//! written about a watched repository, or a ref pushed by one whose pull | ||
//! asks it, sends it to read the repository again once `--settle` has | ||
//! passed; it reads every repository on connecting, and every `--sweep` | ||
//! besides. `--once` reads each once, and stops. | ||
//! passed, as does a repository record an owner it watches writes; it | ||
//! reads every repository on connecting, and every `--sweep` besides. | ||
//! `--once` reads each once, and stops. | ||
| ||
use anyhow::{Context, Result, bail}; | ||
use knotless::env::Ctx; | ||
use knotless::forge::{self, Item, LineComment, Target}; | ||
use knotless::jetstream; | ||
use knotless::records::{CommentRecord, PullHead, RepoRecord, nsid}; | ||
use knotless::records::{CommentRecord, PullHead, RepoRecord, RepoRef, nsid}; | ||
use knotless::remote::Remote; | ||
use knotless::session; | ||
use knotless::telemetry::{self, Fields, Level}; | ||
use knotless::url::{RepoUrl, SCHEME}; | ||
| @@ -49,7 +55,7 @@ use tokio::sync::Notify; | ||
/// What a repository asks of its reviews, on the branch a pull merges into. | ||
const REVIEW_MD: &str = ".knotless/review.md"; | ||
| ||
const USAGE: &str = "usage: knotless-agent review --command <command> [--all] [--rounds <n>] [--timeout <secs>] [--jetstream <wss://host>] [--sweep <secs>] [--settle <secs>] [--once] <handle>/<name>..."; | ||
const USAGE: &str = "usage: knotless-agent review --command <command> [--all] [--rounds <n>] [--timeout <secs>] [--jetstream <wss://host>] [--sweep <secs>] [--settle <secs>] [--once] [--owner <handle|did>]... [<handle>/<name>...]"; | ||
| ||
#[derive(Clone)] | ||
pub struct Options { | ||
| @@ -68,6 +74,8 @@ pub struct Options { | ||
pub settle: Duration, | ||
pub once: bool, | ||
pub repos: Vec<String>, | ||
/// The accounts whose every repository it watches, by handle or DID. | ||
pub owners: Vec<String>, | ||
} | ||
| ||
/// The options `args` give. | ||
| @@ -85,6 +93,7 @@ pub fn parse_args(args: &[String]) -> Result<Options, String> { | ||
settle: Duration::from_secs(5), | ||
once: false, | ||
repos: Vec::new(), | ||
owners: Vec::new(), | ||
}; | ||
let mut it = args.iter().skip(2); | ||
while let Some(flag) = it.next() { | ||
| @@ -101,12 +110,15 @@ pub fn parse_args(args: &[String]) -> Result<Options, String> { | ||
"--sweep" => opts.sweep = secs(flag, value()?)?, | ||
"--settle" => opts.settle = secs(flag, value()?)?, | ||
"--once" => opts.once = true, | ||
"--owner" => opts.owners.push(value()?), | ||
other if other.starts_with('-') => return Err(format!("unexpected {other}\n{USAGE}")), | ||
repo => opts.repos.push(repo.trim_start_matches(SCHEME).to_string()), | ||
} | ||
} | ||
if opts.command.is_empty() || opts.repos.is_empty() { | ||
return Err(format!("name its command and a repository\n{USAGE}")); | ||
if opts.command.is_empty() || (opts.repos.is_empty() && opts.owners.is_empty()) { | ||
return Err(format!( | ||
"name its command and a repository, or --owner\n{USAGE}" | ||
)); | ||
} | ||
Ok(opts) | ||
} | ||
| @@ -168,9 +180,12 @@ struct Reviewer { | ||
opts: Options, | ||
/// The agent's DID. | ||
me: String, | ||
/// The repositories it watches, by owner and key: what records about | ||
/// The repositories it was named, by owner and key: what records about | ||
/// each name. | ||
repos: BTreeMap<(String, String), RepoUrl>, | ||
/// Each `--owner`'s DID and repositories, as last listed, by the owner | ||
/// as given. | ||
owned: Mutex<BTreeMap<String, Owned>>, | ||
/// Those whose pulls ask it for a review: a ref they push may move one. | ||
authors: Mutex<BTreeSet<String>>, | ||
/// The head it last reviewed each pull at, by the pull: the appview | ||
| @@ -192,13 +207,14 @@ pub async fn run(opts: Options, ctx: Ctx) -> Result<()> { | ||
opts, | ||
me, | ||
repos, | ||
owned: Mutex::default(), | ||
authors: Mutex::default(), | ||
posted: Mutex::default(), | ||
door: Notify::new(), | ||
}); | ||
if r.opts.once { | ||
for url in r.repos.values() { | ||
r.sweep(url).await?; | ||
for url in r.watched().await { | ||
r.sweep(&url).await?; | ||
} | ||
return Ok(()); | ||
} | ||
| @@ -208,13 +224,65 @@ pub async fn run(opts: Options, ctx: Ctx) -> Result<()> { | ||
_ = r.door.notified() => tokio::time::sleep(r.opts.settle).await, | ||
_ = tokio::time::sleep(r.opts.sweep) => {} | ||
} | ||
for url in r.repos.values() { | ||
failed(url, r.sweep(url).await); | ||
for url in r.watched().await { | ||
failed(&url, r.sweep(&url).await); | ||
} | ||
} | ||
} | ||
| ||
/// An `--owner`'s repositories, as last listed. | ||
#[derive(Default)] | ||
struct Owned { | ||
did: String, | ||
repos: BTreeMap<(String, String), RepoUrl>, | ||
} | ||
| ||
impl Reviewer { | ||
/// Every repository it watches: those it was named, and each owner's, | ||
/// listed again now. An owner whose listing fails keeps its last. | ||
async fn watched(&self) -> Vec<RepoUrl> { | ||
for owner in &self.opts.owners { | ||
match self.list(owner).await { | ||
Ok(listed) => { | ||
self.owned.lock().unwrap().insert(owner.clone(), listed); | ||
} | ||
Err(err) => { | ||
let f = Fields { | ||
did: Some(owner), | ||
error: Some(format!("{err:#}")), | ||
..Default::default() | ||
}; | ||
telemetry::note(Level::WARN, "listing an owner's repositories failed", &f); | ||
} | ||
} | ||
} | ||
let mut all = self.repos.clone(); | ||
for owned in self.owned.lock().unwrap().values() { | ||
for (key, url) in &owned.repos { | ||
all.entry(key.clone()).or_insert_with(|| url.clone()); | ||
} | ||
} | ||
all.into_values().collect() | ||
} | ||
| ||
/// `owner`'s public repositories, from its PDS. | ||
async fn list(&self, owner: &str) -> Result<Owned> { | ||
let url = RepoUrl::parse(&format!("{SCHEME}{owner}/_"))?; | ||
let remote = Remote::connect(&self.ctx, &url).await?; | ||
let did = remote.did().to_string(); | ||
let repos = remote.list_repos().await?.into_iter().map(|r| { | ||
let url = RepoUrl { | ||
authority: did.clone(), | ||
name: r.value.name, | ||
}; | ||
((did.clone(), r.value.key), url) | ||
}); | ||
Ok(Owned { | ||
repos: repos.collect(), | ||
did, | ||
}) | ||
} | ||
| ||
/// Reviews each of `url`'s pulls waiting for it. | ||
async fn sweep(&self, url: &RepoUrl) -> Result<()> { | ||
let t = Target::open(&self.ctx, url).await?; | ||
| @@ -323,18 +391,25 @@ impl Reviewer { | ||
Ok(branch.and_then(|r| r.value.target.clone())) | ||
} | ||
| ||
/// Whether `event` writes a pull about a repository it watches, or a | ||
/// ref of one whose pull asks it for a review. | ||
/// Whether `event` writes a pull about a repository it watches, a | ||
/// ref of one whose pull asks it for a review, or a repository record | ||
/// of an owner it watches. | ||
fn rings(&self, event: &jetstream::Event) -> bool { | ||
let did = event.did.as_deref().unwrap_or_default(); | ||
let authors = self.authors.lock().unwrap(); | ||
let owned = self.owned.lock().unwrap(); | ||
let owner = |did: &str| owned.values().any(|o| o.did == did); | ||
event.ops.iter().any(|op| match op.collection.as_str() { | ||
nsid::PULL => (op.record.as_ref()).is_some_and(|r| { | ||
self.repos | ||
.keys() | ||
.any(|(owner, key)| forge::is_about(r, owner, key)) | ||
let about = serde_json::from_value::<RepoRef>(r["repo"].clone()); | ||
about.is_ok_and(|about| owner(about.did())) | ||
|| self | ||
.repos | ||
.keys() | ||
.any(|(owner, key)| forge::is_about(r, owner, key)) | ||
}), | ||
nsid::REF => authors.contains(did), | ||
nsid::REPO => owner(did), | ||
_ => false, | ||
}) | ||
} | ||
| @@ -443,12 +518,22 @@ mod tests { | ||
assert!(opts.all); | ||
assert_eq!((opts.rounds, opts.timeout.as_secs()), (3, 1200)); | ||
assert!(!opts.once); | ||
let opts = parse_args(&args( | ||
"knotless-agent review --command ./r --owner a.example --owner did:plc:b", | ||
)) | ||
.unwrap(); | ||
assert!(opts.repos.is_empty()); | ||
assert_eq!(opts.owners, ["a.example", "did:plc:b"]); | ||
for (bad, says) in [ | ||
("knotless-agent", "usage:"), | ||
("knotless-agent label --command ./r a.example/p", "usage:"), | ||
("knotless-agent review a.example/p", "name its command"), | ||
("knotless-agent review --command ./r", "name its command"), | ||
("knotless-agent review --command", "--command takes a value"), | ||
( | ||
"knotless-agent review --command ./r --owner", | ||
"--owner takes a value", | ||
), | ||
("knotless-agent review --rounds 0", "--rounds takes a count"), | ||
("knotless-agent review --rounds x", "--rounds takes a count"), | ||
( | ||
crates/knotless-agent/tests/review.rs+40−0
| @@ -248,6 +248,7 @@ fn an_agent_reviews_pulls() { | ||
settle: Duration::ZERO, | ||
once: true, | ||
repos: vec![format!("{}/proj", owner.account.handle)], | ||
owners: Vec::new(), | ||
}; | ||
let once = |opts: &Options| { | ||
rt.block_on(knotless_agent::run(opts.clone(), common::ctx(&bot))) | ||
| @@ -404,6 +405,44 @@ fn an_agent_reviews_pulls() { | ||
assert_eq!(asked(&bot, "Unasked"), 1); | ||
assert_eq!(reviews(&bot, &unasked).0.len(), 1); | ||
assert_eq!(asked(&bot, "Draft one"), 0); | ||
| ||
// With --owner, it watches every repository the owner holds, one made | ||
// after it started too: a repository record the owner writes sends it | ||
// to list them again. An owner it cannot list is passed by. | ||
let js = jetstream(&rt); | ||
let owners = Options { | ||
once: false, | ||
jetstream: js.url.clone(), | ||
sweep: Duration::from_secs(600), | ||
repos: Vec::new(), | ||
owners: vec![ | ||
"did:web:nobody.invalid".into(), | ||
owner.account.handle.clone(), | ||
], | ||
..opts.clone() | ||
}; | ||
let before = asked(&bot, "Broken"); | ||
rt.spawn(knotless_agent::run(owners, common::ctx(&bot))); | ||
wait_until(|| asked(&bot, "Broken") > before); | ||
let later = owner.url("later"); | ||
owner.git(&work, &["push", "-q", &later, "main"]); | ||
owner.commit_file(&work, "f", "later\n"); | ||
owner.git(&work, &["push", "-q", &later, "HEAD:refs/heads/late"]); | ||
owner.git(&work, &["reset", "-q", "--hard", "HEAD~1"]); | ||
let args = ["pull", &later, "main", &later, "late", "Late", "--reviewer"]; | ||
let late = ok(&owner, &[&args[..], &[me.as_str()]].concat()); | ||
let rkey = late.rsplit('/').next().unwrap(); | ||
wait_until(|| owner.cli(&["list", &later], b"").1.contains(rkey)); | ||
js.send(&did, "dev.knotless.repo", "later", serde_json::json!({})); | ||
wait_until(|| reviews(&bot, &late).0.len() == 1); | ||
// A pull about any of the owner's repositories rings for it too. | ||
wait_until(|| ok(&owner, &["show", &later, &late]).contains("changes")); | ||
owner.commit_file(&work, "f", "later again\n"); | ||
owner.git(&work, &["push", "-q", "-f", &later, "HEAD:refs/heads/late"]); | ||
owner.git(&work, &["reset", "-q", "--hard", "HEAD~1"]); | ||
let record = owner.record("dev.knotless.pull", rkey); | ||
js.send(&did, "dev.knotless.pull", rkey, record); | ||
wait_until(|| reviews(&bot, &late).0.len() == 2); | ||
} | ||
| ||
/// A private repository's pulls are reviewed as a public one's, once the | ||
| @@ -472,6 +511,7 @@ fn an_agent_reviews_a_private_repositorys_pulls() { | ||
settle: Duration::ZERO, | ||
once: true, | ||
repos: vec![format!("{owned}/hidden")], | ||
owners: Vec::new(), | ||
}; | ||
rt.block_on(knotless_agent::run(opts, common::ctx(&bot))) | ||
.unwrap(); | ||
deploy/deploy.sh+2−1
| @@ -62,7 +62,8 @@ | ||
# kind,area`, say. | ||
# | ||
# knotless-agent's reviewer too, if KNOTLESS_REVIEW_REPOS names | ||
# repositories whose pulls it reviews, once /etc/knotless/review.env is on | ||
# repositories whose pulls it reviews (or `--owner <handle>`, for all of | ||
# an account's), once /etc/knotless/review.env is on | ||
# the host: a deploy with KNOTLESS_REVIEW_IDENTIFIER, | ||
# KNOTLESS_REVIEW_PASSWORD (an app password) and Claude's login, | ||
# CLAUDE_CODE_OAUTH_TOKEN or ANTHROPIC_API_KEY, writes it, and one | ||
deploy/README.md+1−1
| @@ -354,7 +354,7 @@ docker pull knotless.dev/chadtmiller.com/knotless/knotless-server | ||
| `pages` | `node /app/build` | | ||
| `run` | `knotless-run --secrets … --state … --work … --caches …` | | ||
| `triage` | `knotless-triage --decide <url> --jetstream <url> <repos>` | | ||
| `review` | `knotless-agent review --command knotless-claude-review --jetstream <url> <repos>` | | ||
| `review` | `knotless-agent review --command knotless-claude-review --jetstream <url> <repos>`, where the repositories may be `--owner <handle>` | | ||
| ||
knotless.dev hands those out, with `--releases | ||
knotless://chadtmiller.com/knotless` in docker-compose.yml: each file is | ||
docs/guide/reviews.md+2−2
| @@ -48,7 +48,7 @@ Use a new account, not your own. The owner's approvals count toward required rev | ||
knotless-agent review --command knotless-claude-review --once alice.example.com/my-project | ||
``` | ||
| ||
Name as many repositories as you like, each as `<handle>/<name>`. With no pull asking for it yet, it reads the repository and exits without writing anything. A review takes Claude a minute or two. | ||
Name as many repositories as you like, each as `<handle>/<name>`, or `--owner alice.example.com` for every public repository that account holds, including those it makes later. With no pull asking for it yet, it reads the repository and exits without writing anything. A review takes Claude a minute or two. | ||
| ||
6. Run it without `--once` to keep it watching. See [Keep it running](#keep-it-running). | ||
| ||
| @@ -157,7 +157,7 @@ A reviewer that serves other people's repositories needs an API key. Anthropic's | ||
| ||
Run the same command as in step 5 without `--once`, under whatever keeps a service running on your machine, such as systemd or launchd. Give it the environment variables of step 5. It reconnects to Jetstream when the connection drops. | ||
| ||
The knotless server image holds everything the reviewer needs. Run `knotless-agent review --command knotless-claude-review <repositories>` in it with the same environment. | ||
The knotless server image holds everything the reviewer needs. Run `knotless-agent review --command knotless-claude-review <repositories>` (or `--owner <handle>`) in it with the same environment. | ||
| ||
On the host that serves knotless.dev, the compose file has a `review` service that does this beside knotless-serve and reads pulls from it. [The deploy README](../../deploy/README.md) says how a deploy turns it on. To review private repositories there, put the reviewer's DID in `KNOTLESS_BOT_DIDS` and log it in at `/oauth/bot`, as the runner and the triager do, for the OAuth session that reads them. | ||
| ||
docs/reference/cli.md+9−7
| @@ -541,9 +541,9 @@ record written has it read that repository's, to find one that names it. | ||
| ||
## knotless-agent | ||
| ||
`knotless-agent review --command <command> [flags] <handle>/<name>...` | ||
reviews the open pulls of the repositories named that ask its account for | ||
a review, acting as `KNOTLESS_IDENTIFIER` with `KNOTLESS_PASSWORD`, and | ||
`knotless-agent review --command <command> [flags] [<handle>/<name>...]` | ||
reviews the open pulls of the repositories named, and of every repository | ||
an `--owner` holds, that ask its account for a review, acting as `KNOTLESS_IDENTIFIER` with `KNOTLESS_PASSWORD`, and | ||
reads public repositories' pulls through `KNOTLESS_APPVIEW`. [Reviews by | ||
Claude](../guide/reviews.md) sets one up, and [review | ||
agents](../design/collaboration.md#review-agents) says why it works as it | ||
| @@ -559,6 +559,7 @@ does. | ||
| `--sweep <secs>` | 300 | how often every repository is read, whatever events say | | ||
| `--settle <secs>` | 5 | how long after an event the repositories are read, for the appview to index the pull | | ||
| `--once` | off | read each repository once, and stop | | ||
| `--owner <handle\|did>` | | also watch every public repository this account holds, listed from its PDS at each read, so one it makes later is watched too. Repeat it for more accounts. An owner whose listing fails is logged as `listing an owner's repositories failed`, and its last listing stands | | ||
| ||
It reviews a pull that is open and not a draft, that its owner, a | ||
collaborator or a member of the owner's organization opened, and whose | ||
| @@ -592,15 +593,16 @@ Claude's `--model`, | ||
`KNOTLESS_MCP` for the `knotless-mcp` to run and `CLAUDE` for the `claude`. | ||
| ||
A usage error exits 2 and prints the usage, as `name its command and a | ||
repository`; an error at run time exits 1, as `knotless-agent: <error>`. | ||
repository, or --owner`; an error at run time exits 1, as `knotless-agent: <error>`. | ||
Watching, it stops at the start if a repository cannot be opened; | ||
afterwards one that cannot be read is logged as `reading the repository | ||
failed`, and it goes on. With `--once`, any such error stops it. | ||
| ||
It follows Jetstream as knotless-triage does. It reads every repository | ||
on connecting; a pull written about a watched repository, or a ref pushed | ||
by an account whose pull asks it, has it read the repositories again | ||
after `--settle`. | ||
on connecting; a pull written about a watched repository or any of an | ||
owner's, a repository record an owner writes, or a ref pushed by an | ||
account whose pull asks it, has it read the repositories again after | ||
`--settle`. | ||
| ||
## Environment | ||