vinzenz/hyperhive
vinzenz/hyperhive
Watch
0
0
Fork
You've already forked hyperhive
 0
Code Activity

swarm-controller: read review rules through the git tree and blob API

Browse source 
The rules file is now read with the existing forgejo-api client: the
commit's root tree, then the `.hyperhive` tree, then the
`review-classifier.{yml,json}` blob, base64-decoded. Every request takes
single-segment path parameters, so nothing depends on how a nested path
is escaped, and the controller keeps one HTTP transport to the forge.
The second reqwest client, `Client::with_token` and `raw_file_url` are
gone.

A tree the forge truncated is an error, not a missing file. A 404 on a
tree or blob doesn't come back as a forge 404, so only the repo lookup's
404 reads as "no such repo".

The `is_repo_not_found` doc comment is back on its function.

Part of #5096.
This commit is contained in:
atlas 2026-10-12 08:47:07 +02:00 • committed by mara
commit d52e97f376
5 changed files with 202 additions and 147 deletions
Download patch file Download diff file Expand all files Collapse all files

6
swarm-controller/Cargo.toml
View file

@ -22,9 +22,9 @@ axum.workspace = true
# boundary (see #3306's design discussion: forcing that split now, over a
# few idempotent CRUD-ish calls, is premature plumbing).
forgejo-api.workspace = true
# Only for base64-encoding file content for `forge.rs`'s
# `repo_change_files` calls — forgejo's content API takes base64, never
# raw bytes.
# `forge.rs` encodes file content for its `repo_change_files` calls and
# decodes blobs it reads: forgejo's content and blob APIs carry base64,
# never raw bytes.
base64.workspace = true
# `agent_identity.rs` mints the per-agent queue secret, the one value in this
# tree this daemon invents rather than receives. Straight from the kernel's

333
swarm-controller/src/forge.rs
View file

@ -23,10 +23,10 @@ use forgejo_api::structs::{
AddCollaboratorOption, AddCollaboratorOptionPermission, ChangeFileOperation,
ChangeFileOperationOperation, ChangeFilesOptions, CreateBranchProtectionOption,
CreateHookOption, CreateHookOptionConfig, CreateHookOptionType, CreateRepoOption,
CreateUserOption, EditUserOption, IssueListIssuesQuery, IssueListIssuesQueryState,
IssueListIssuesQueryType, RepoDownloadPullDiffOrPatchQuery, RepoGetContentsQuery,
RepoGetPullRequestCommitsQuery, RepoListPullRequestsQuery, RepoListPullRequestsQueryState,
RepoSearchQuery, StateType,
CreateUserOption, EditUserOption, GetTreeQuery, GitEntry, IssueListIssuesQuery,
IssueListIssuesQueryState, IssueListIssuesQueryType, RepoDownloadPullDiffOrPatchQuery,
RepoGetContentsQuery, RepoGetPullRequestCommitsQuery, RepoListPullRequestsQuery,
RepoListPullRequestsQueryState, RepoSearchQuery, StateType,
};
use forgejo_api::{ApiErrorKind, Auth, Forgejo, ForgejoError};
use futures_util::{StreamExt as _, TryStreamExt as _};
@ -184,10 +184,6 @@ async fn fetch_token() -> Result<String> {
/// Typed forgejo client, built once at startup from env.
pub struct Client {
api: Forgejo,
/// The same token as `api`, for the one request `api` can't shape:
/// forgejo-api escapes every `/` in a file path, so [`raw_file_url`]
/// builds that path itself.
http: reqwest::Client,
/// The forge's base URL, kept so a client authenticated as someone
/// else can be built against the same forge (see
/// [`agent_token`]'s read-back).
@ -327,28 +323,11 @@ impl Client {
/// host has not published it yet, or the client failing to build.
async fn connect(url: &url::Url) -> Result<Self> {
let token = fetch_token().await?;
Self::with_token(&token, url.clone())
}
/// A client for `url` holding `token`.
///
/// # Errors
/// A token that is not a valid header value, or a client failing to
/// build.
fn with_token(token: &str, url: url::Url) -> Result<Self> {
let api = Forgejo::new(Auth::Token(token), url.clone()).context("build forgejo client")?;
let mut auth = reqwest::header::HeaderValue::try_from(format!("token {token}"))
.context("the forge token is not a valid header value")?;
auth.set_sensitive(true);
let http = reqwest::Client::builder()
.default_headers(
[(reqwest::header::AUTHORIZATION, auth)]
.into_iter()
.collect(),
)
.build()
.context("build forge HTTP client")?;
Ok(Self { api, http, url })
let api = Forgejo::new(Auth::Token(&token), url.clone()).context("build forgejo client")?;
Ok(Self {
api,
url: url.clone(),
})
}
/// Creation options for an empty repo defaulting to `main`.
@ -1139,9 +1118,14 @@ impl Client {
/// read at the branch's head commit. `None` when the default branch has
/// no commit: an empty repo, or a default branch that doesn't exist.
///
/// The files come through the git tree and blob API, one path segment
/// per request: forgejo-api escapes the `/` of a nested path in its
/// file endpoints.
///
/// # Errors
/// A forge failure; a [`ForgejoError`] 404 underneath comes only from the
/// first call, and means the repo does not exist ([`is_repo_not_found`]).
/// A forge failure, or a tree the forge truncated. A [`ForgejoError`] 404
/// underneath comes only from the first call, and means the repo does
/// not exist ([`is_repo_not_found`]).
pub async fn default_branch_files(
&self,
owner: &str,
@ -1170,9 +1154,10 @@ impl Client {
.commit
.and_then(|c| c.id)
.with_context(|| format!("branch {branch} of {owner}/{repo} names no commit"))?;
let mut trees = HashMap::new();
let mut files = Vec::new();
for path in paths {
if let Some(bytes) = self.raw_file(owner, repo, path, &commit).await? {
if let Some(bytes) = self.file_at(owner, repo, &commit, path, &mut trees).await? {
files.push(((*path).to_owned(), bytes));
}
}
@ -1183,26 +1168,88 @@ impl Client {
}))
}
/// The file at `path` in `owner/repo` at `rev`, or `None` on a 404.
async fn raw_file(
/// The file at `path` in `owner/repo` at `commit`, or `None` when no
/// blob is there. `trees` holds the tree listings already read, by sha.
async fn file_at(
&self,
owner: &str,
repo: &str,
commit: &str,
path: &str,
rev: &str,
trees: &mut HashMap<String, Vec<GitEntry>>,
) -> Result<Option<Vec<u8>>> {
let what = || format!("read {path} of {owner}/{repo} at {rev}");
let response = self
.http
.get(raw_file_url(&self.url, owner, repo, path, rev)?)
.send()
.await
.with_context(what)?;
match response.status() {
StatusCode::NOT_FOUND => Ok(None),
s if s.is_success() => Ok(Some(response.bytes().await.with_context(what)?.to_vec())),
s => Err(anyhow::anyhow!("the forge answered {s}")).with_context(what),
let mut tree = commit.to_owned();
let mut segments = path.split('/').peekable();
while let Some(name) = segments.next() {
let kind = if segments.peek().is_some() {
"tree"
} else {
"blob"
};
let entries = self.tree(owner, repo, &tree, trees).await?;
let Some(sha) = entries
.iter()
.find(|e| e.path.as_deref() == Some(name) && e.r#type.as_deref() == Some(kind))
.and_then(|e| e.sha.clone())
else {
return Ok(None);
};
if kind == "blob" {
return self.blob(owner, repo, &sha).await.map(Some);
}
tree = sha;
}
Ok(None)
}
/// The entries of tree `sha` (or of a commit's root tree), read once
/// into `trees`.
async fn tree<'t>(
&self,
owner: &str,
repo: &str,
sha: &str,
trees: &'t mut HashMap<String, Vec<GitEntry>>,
) -> Result<&'t [GitEntry]> {
if !trees.contains_key(sha) {
let listing = self
.api
.get_tree(owner, repo, sha, GetTreeQuery::default())
.await
.map_err(|e| anyhow::anyhow!("read tree {sha} of {owner}/{repo}: {e}"))?;
if listing.truncated.unwrap_or(false) {
anyhow::bail!(
"the forge truncated tree {sha} of {owner}/{repo}, so a missing entry can't \
be told from one past the cut"
);
}
trees.insert(sha.to_owned(), listing.tree.unwrap_or_default());
}
Ok(&trees[sha])
}
/// The bytes of blob `sha`.
async fn blob(&self, owner: &str, repo: &str, sha: &str) -> Result<Vec<u8>> {
use base64::Engine as _;
let blob = self
.api
.get_blob(owner, repo, sha)
.await
.map_err(|e| anyhow::anyhow!("read blob {sha} of {owner}/{repo}: {e}"))?;
anyhow::ensure!(
blob.encoding.as_deref() == Some("base64"),
"blob {sha} of {owner}/{repo} is not base64 but {:?}",
blob.encoding
);
let content: String = blob
.content
.unwrap_or_default()
.split_ascii_whitespace()
.collect();
base64::engine::general_purpose::STANDARD
.decode(content)
.with_context(|| format!("decode blob {sha} of {owner}/{repo}"))
}
/// Register the swarm-wide hooks against this controller, so a real
@ -1564,31 +1611,6 @@ fn folds_into_success(e: &ForgejoError, existing: bool) -> bool {
/// Used by [`crate::issue_report::get_issue_report`] to tell "no such repo"
/// (404, retrying won't help) apart from every other forge failure (503,
/// might clear up) — see that module for why the distinction matters.
/// The API URL of the file at `path` in `owner/repo` at `rev`, on the forge
/// at `base`. Each segment of `path` is escaped on its own and the segments
/// are joined with a literal `/`, so no proxy or router in front of the forge
/// has to decode a `%2F`. Like forgejo-api's own requests, it ignores any
/// path `base` has.
///
/// # Errors
/// A `base` that can't carry a path, such as a `data:` URL.
fn raw_file_url(
base: &url::Url,
owner: &str,
repo: &str,
path: &str,
rev: &str,
) -> Result<url::Url> {
let mut url = base.clone();
url.path_segments_mut()
.map_err(|()| anyhow::anyhow!("forge URL {base} can't carry a path"))?
.clear()
.extend(["api", "v1", "repos", owner, repo, "raw"])
.extend(path.split('/'));
url.query_pairs_mut().clear().append_pair("ref", rev);
Ok(url)
}
pub(crate) fn is_repo_not_found(e: &ForgejoError) -> bool {
match e {
ForgejoError::ApiError(api) => match api.error_kind() {
@ -1615,7 +1637,8 @@ fn conflict_error() -> ForgejoError {
impl Client {
/// A client for a test's own forge at `url`, holding a placeholder token.
pub(crate) fn for_test_forge(url: url::Url) -> Self {
Self::with_token("stub-token", url).unwrap()
let api = Forgejo::new(Auth::Token("stub-token"), url.clone()).unwrap();
Self { api, url }
}
}
@ -1862,7 +1885,8 @@ mod tests {
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let url = url::Url::parse(&format!("http://{}/", listener.local_addr().unwrap())).unwrap();
tokio::spawn(async move { axum::serve(listener, app).await });
(Client::with_token("stub-token", url).unwrap(), seen)
let api = Forgejo::new(Auth::Token("stub-token"), url.clone()).unwrap();
(Client { api, url }, seen)
}
/// The `PATCH` [`Client::ensure_agent_user`] sent, if any.
@ -2097,51 +2121,50 @@ mod tests {
assert_eq!(methods, ["GET"]);
}
#[test]
fn a_raw_file_url_keeps_the_slash_between_path_segments() {
let base = url::Url::parse("https://forge.example/").unwrap();
let url = raw_file_url(
&base,
"o",
"r",
".hyperhive/review-classifier.yml",
"def456",
)
.unwrap();
assert_eq!(
url.as_str(),
"https://forge.example/api/v1/repos/o/r/raw/.hyperhive/review-classifier.yml?ref=def456"
);
// A segment's own reserved characters are still escaped.
let odd = raw_file_url(&base, "o", "r", "a b/c?d#e", "x").unwrap();
assert_eq!(odd.path(), "/api/v1/repos/o/r/raw/a%20b/c%3Fd%23e");
}
const RULES_YML: &str = ".hyperhive/review-classifier.yml";
const RULES_JSON: &str = ".hyperhive/review-classifier.json";
/// A stand-in forge holding `o/r` with default branch `main` at
/// `def456`, where only `.hyperhive/review-classifier.yml` exists, under
/// that literal path. `branch` is the status the branch lookup answers.
/// Records every request's path and query.
async fn rules_forge(branch: u16) -> (Client, Arc<std::sync::Mutex<Vec<String>>>) {
/// `def456`. The branch lookup answers `branch`, and the commit's root
/// tree answers `root` (a 404 for `None`). The `.hyperhive` tree holds
/// only `review-classifier.yml`. Records every request's path.
async fn rules_forge(
branch: u16,
root: Option<&'static str>,
) -> (Client, Arc<std::sync::Mutex<Vec<String>>>) {
use axum::http::{StatusCode, Uri};
use base64::Engine as _;
let blob = format!(
r#"{{"sha":"b1","encoding":"base64","content":"{}"}}"#,
base64::engine::general_purpose::STANDARD.encode("classifications: []\n")
);
let seen = Arc::new(std::sync::Mutex::new(Vec::new()));
let recorded = Arc::clone(&seen);
let app = axum::Router::new().fallback(move |uri: Uri| {
let recorded = Arc::clone(&recorded);
let (recorded, blob) = (Arc::clone(&recorded), blob.clone());
async move {
recorded.lock().unwrap().push(uri.to_string());
recorded.lock().unwrap().push(uri.path().to_owned());
let (status, body) = match uri.path() {
"/api/v1/repos/o/r" => (200, r#"{"default_branch":"main","empty":false}"#),
"/api/v1/repos/o/r/branches/main" if branch == 200 => {
(200, r#"{"name":"main","commit":{"id":"def456"}}"#)
"/api/v1/repos/o/r" => {
(200, r#"{"default_branch":"main","empty":false}"#.to_owned())
}
"/api/v1/repos/o/r/branches/main" => (branch, r#"{"message":"no"}"#),
"/api/v1/repos/o/r/raw/.hyperhive/review-classifier.yml"
if uri.query() == Some("ref=def456") =>
{
(200, "classifications: []\n")
"/api/v1/repos/o/r/branches/main" if branch == 200 => (
200,
r#"{"name":"main","commit":{"id":"def456"}}"#.to_owned(),
),
"/api/v1/repos/o/r/branches/main" => (branch, r#"{"message":"no"}"#.to_owned()),
"/api/v1/repos/o/r/git/trees/def456" if root.is_some() => {
(200, root.unwrap_or_default().to_owned())
}
_ => (404, r#"{"message":"not found"}"#),
"/api/v1/repos/o/r/git/trees/t1" => (
200,
r#"{"sha":"t1","truncated":false,"tree":[
{"path":"review-classifier.yml","type":"blob","sha":"b1"}]}"#
.to_owned(),
),
"/api/v1/repos/o/r/git/blobs/b1" => (200, blob),
_ => (404, r#"{"message":"not found"}"#.to_owned()),
};
(StatusCode::from_u16(status).unwrap(), body)
}
@ -2149,21 +2172,20 @@ mod tests {
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let url = url::Url::parse(&format!("http://{}/", listener.local_addr().unwrap())).unwrap();
tokio::spawn(async move { axum::serve(listener, app).await });
(Client::with_token("stub-token", url).unwrap(), seen)
let api = Forgejo::new(Auth::Token("stub-token"), url.clone()).unwrap();
(Client { api, url }, seen)
}
/// A root tree with the `.hyperhive` tree and a blob beside it.
const ROOT: &str = r#"{"sha":"def456","truncated":false,"tree":[
{"path":"README.md","type":"blob","sha":"b0"},
{"path":".hyperhive","type":"tree","sha":"t1"}]}"#;
#[tokio::test]
async fn default_branch_files_reads_each_path_at_the_branch_head() {
let (client, seen) = rules_forge(200).await;
async fn default_branch_files_reads_each_path_through_the_trees_at_the_branch_head() {
let (client, seen) = rules_forge(200, Some(ROOT)).await;
let read = client
.default_branch_files(
"o",
"r",
&[
".hyperhive/review-classifier.yml",
".hyperhive/review-classifier.json",
],
)
.default_branch_files("o", "r", &[RULES_YML, RULES_JSON])
.await
.unwrap()
.expect("the branch has a commit");
@ -2171,47 +2193,74 @@ mod tests {
assert_eq!(read.commit, "def456");
assert_eq!(
read.files,
[(
".hyperhive/review-classifier.yml".to_owned(),
b"classifications: []\n".to_vec()
)]
);
assert!(
seen.lock().unwrap().iter().all(|p| !p.contains("%2F")),
"{seen:?}"
[(RULES_YML.to_owned(), b"classifications: []\n".to_vec())]
);
// Each tree is read once for both paths.
let seen = seen.lock().unwrap();
let reads = |p: &str| seen.iter().filter(|s| *s == p).count();
assert_eq!(reads("/api/v1/repos/o/r/git/trees/def456"), 1, "{seen:?}");
assert_eq!(reads("/api/v1/repos/o/r/git/trees/t1"), 1, "{seen:?}");
}
#[tokio::test]
async fn a_repo_without_the_directory_has_no_files() {
let root = r#"{"sha":"def456","truncated":false,"tree":[
{"path":"README.md","type":"blob","sha":"b0"},
{"path":".hyperhive","type":"blob","sha":"b2"}]}"#;
let (client, _) = rules_forge(200, Some(root)).await;
let read = client
.default_branch_files("o", "r", &[RULES_YML])
.await
.unwrap()
.expect("the branch has a commit");
assert_eq!(read.files, []);
}
#[tokio::test]
async fn a_truncated_tree_is_an_error_not_a_missing_file() {
let root = r#"{"sha":"def456","truncated":true,"tree":[]}"#;
let (client, _) = rules_forge(200, Some(root)).await;
let err = client
.default_branch_files("o", "r", &[RULES_YML])
.await
.unwrap_err();
assert!(format!("{err:#}").contains("truncated"), "{err:#}");
}
#[tokio::test]
async fn a_missing_default_branch_is_no_commit_not_no_repo() {
let (client, _) = rules_forge(404).await;
let (client, _) = rules_forge(404, Some(ROOT)).await;
let read = client
.default_branch_files("o", "r", &[".hyperhive/review-classifier.yml"])
.default_branch_files("o", "r", &[RULES_YML])
.await
.unwrap();
assert_eq!(read, None);
}
/// Only the repo lookup's 404 reads as "no such repo"; a failed branch
/// read or a tree the forge doesn't have is some other failure.
#[tokio::test]
async fn an_unknown_repo_is_a_forge_404_and_a_failed_branch_read_is_not() {
let (client, _) = rules_forge(500).await;
async fn only_an_unknown_repo_is_a_forge_404() {
let no_repo = |e: &anyhow::Error| {
e.downcast_ref::<ForgejoError>()
.is_some_and(is_repo_not_found)
};
let (client, _) = rules_forge(500, Some(ROOT)).await;
let unknown = client
.default_branch_files("o", "elsewhere", &[])
.await
.unwrap_err();
let fe = unknown
.downcast_ref::<ForgejoError>()
.expect("a forge error");
assert!(is_repo_not_found(fe), "{unknown:#}");
let failed = client
assert!(no_repo(&unknown), "{unknown:#}");
let branch = client
.default_branch_files("o", "r", &[])
.await
.unwrap_err();
assert!(
!failed
.downcast_ref::<ForgejoError>()
.is_some_and(is_repo_not_found),
"{failed:#}"
);
assert!(!no_repo(&branch), "{branch:#}");
let (client, _) = rules_forge(200, None).await;
let tree = client
.default_branch_files("o", "r", &[RULES_YML])
.await
.unwrap_err();
assert!(!no_repo(&tree), "{tree:#}");
}
}

4
swarm-controller/src/forge/docs_grant.rs
View file

@ -206,6 +206,7 @@ mod tests {
use axum::extract::{Path, State};
use axum::http::{StatusCode, header::CONTENT_TYPE};
use forgejo_api::{Auth, Forgejo};
use super::*;
@ -254,7 +255,8 @@ mod tests {
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let url = url::Url::parse(&format!("http://{}/", listener.local_addr().unwrap())).unwrap();
tokio::spawn(async move { axum::serve(listener, app).await });
(Client::with_token("stub-token", url).unwrap(), docs)
let api = Forgejo::new(Auth::Token("stub-token"), url.clone()).unwrap();
(Client { api, url }, docs)
}
async fn check(

5
swarm-controller/src/forge/legacy_tokens.rs
View file

@ -113,6 +113,8 @@ mod tests {
use std::collections::BTreeMap;
use std::sync::{Arc, Mutex};
use forgejo_api::{Auth, Forgejo};
use super::super::agent_token::{AGENT_TOKEN_NAME, MintReason};
use super::*;
@ -227,7 +229,8 @@ mod tests {
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let url = url::Url::parse(&format!("http://{}/", listener.local_addr().unwrap())).unwrap();
tokio::spawn(async move { axum::serve(listener, app).await });
(Client::with_token("stub-token", url).unwrap(), deletes)
let api = Forgejo::new(Auth::Token("stub-token"), url.clone()).unwrap();
(Client { api, url }, deletes)
}
const MIX: [&str; 5] = [

1
swarm-controller/src/review_classifier.rs
View file

@ -1232,6 +1232,7 @@ classifications:
assert!(detail.contains(says), "{query}: {detail}");
}
}
#[tokio::test]
async fn rules_without_forge_access_on_this_host_are_503() {
let (state, _) = crate::tests::state_with_roster();