diff --git a/frontend/packages/swarm-ui/src/pages/review-tryout/ReviewTryOutPage.tsx b/frontend/packages/swarm-ui/src/pages/review-tryout/ReviewTryOutPage.tsx index 07bd804b..25c3a763 100644 --- a/frontend/packages/swarm-ui/src/pages/review-tryout/ReviewTryOutPage.tsx +++ b/frontend/packages/swarm-ui/src/pages/review-tryout/ReviewTryOutPage.tsx @@ -37,11 +37,10 @@ import { ClassificationEditor } from "./ClassificationEditor.js"; import { TryOutResult } from "./TryOutResult.js"; import { draftFromRule, - draftProblem, - duplicateName, hasTypedText, prNumber, repoProblem, + rowsProblem, toBody, type ClassificationDraft, type OpenPrAnswer, @@ -218,15 +217,10 @@ export function ReviewTryOutPage() { }; }, [runId]); - // What keeps the rows from going to the repo; a PR number is not needed. - const rowsProblem = - repoProblem(repo) ?? - drafts.map(draftProblem).find((p) => p !== null) ?? - duplicateName(drafts) ?? - null; + const openPrProblem = rowsProblem(repo, drafts); const problem = (prNumber(pr) === null ? "a PR number is a whole number above 0" : null) ?? - rowsProblem; + openPrProblem; async function run(e: Event) { e.preventDefault(); @@ -410,7 +404,7 @@ export function ReviewTryOutPage() { diff --git a/frontend/packages/swarm-ui/src/pages/review-tryout/TryOutTypes.ts b/frontend/packages/swarm-ui/src/pages/review-tryout/TryOutTypes.ts index c8233a85..0195ac1f 100644 --- a/frontend/packages/swarm-ui/src/pages/review-tryout/TryOutTypes.ts +++ b/frontend/packages/swarm-ui/src/pages/review-tryout/TryOutTypes.ts @@ -150,6 +150,20 @@ export function duplicateName(drafts: ClassificationDraft[]): string | null { return null; } +/** What keeps the rows from going to the repo as a PR, or null: the repo, then + * each row, then a shared name. Unlike a run, it needs no PR number. */ +export function rowsProblem( + repo: string, + drafts: ClassificationDraft[], +): string | null { + return ( + repoProblem(repo) ?? + drafts.map(draftProblem).find((p) => p !== null) ?? + duplicateName(drafts) ?? + null + ); +} + export function toBody(d: ClassificationDraft): ClassificationBody { return { name: d.name.trim(), diff --git a/swarm-controller/src/forge/classifier_account.rs b/swarm-controller/src/forge/classifier_account.rs index df4b03e8..aff3ec3d 100644 --- a/swarm-controller/src/forge/classifier_account.rs +++ b/swarm-controller/src/forge/classifier_account.rs @@ -181,44 +181,7 @@ impl Client { repo: &str, commit: BranchCommit, ) -> Result<()> { - use base64::Engine as _; - - let branch = commit.branch.clone(); - self.as_classifier() - .await? - .repo_change_files( - owner, - repo, - ChangeFilesOptions { - author: Some(Identity { - name: Some(commit.author.name), - email: Some(commit.author.email), - }), - branch: Some(commit.base), - committer: None, - dates: None, - files: vec![ChangeFileOperation { - content: Some( - base64::engine::general_purpose::STANDARD.encode(&commit.content), - ), - from_path: None, - operation: if commit.sha.is_some() { - ChangeFileOperationOperation::Update - } else { - ChangeFileOperationOperation::Create - }, - path: commit.path, - sha: commit.sha, - }], - force_overwrite_new_branch: Some(false), - message: Some(commit.message), - new_branch: Some(commit.branch), - signoff: None, - }, - ) - .await - .with_context(|| format!("pushing {LOGIN}'s branch {branch} to {owner}/{repo}"))?; - Ok(()) + push_branch(&self.as_classifier().await?, owner, repo, commit).await } /// The forge account `login` as a commit author: its full name, or its @@ -338,6 +301,47 @@ impl Client { } } +/// Make `commit` with `forge`'s identity as committer, through the contents +/// API. +async fn push_branch(forge: &Forgejo, owner: &str, repo: &str, commit: BranchCommit) -> Result<()> { + use base64::Engine as _; + + let branch = commit.branch.clone(); + forge + .repo_change_files( + owner, + repo, + ChangeFilesOptions { + author: Some(Identity { + name: Some(commit.author.name), + email: Some(commit.author.email), + }), + branch: Some(commit.base), + committer: None, + dates: None, + files: vec![ChangeFileOperation { + content: Some( + base64::engine::general_purpose::STANDARD.encode(&commit.content), + ), + from_path: None, + operation: if commit.sha.is_some() { + ChangeFileOperationOperation::Update + } else { + ChangeFileOperationOperation::Create + }, + path: commit.path, + sha: commit.sha, + }], + force_overwrite_new_branch: Some(false), + message: Some(commit.message), + new_branch: Some(commit.branch), + signoff: None, + }, + ) + .await + .with_context(|| format!("pushing {LOGIN}'s branch {branch} to {owner}/{repo}"))?; + Ok(()) +} /// Check the account now and every [`RECONCILE_INTERVAL`] after, and call /// `enqueue`, which inserts a `MintClassifierForgeToken` node, when it needs /// a job. A pass that fails is logged and retried on the next tick. @@ -419,6 +423,86 @@ mod tests { assert!(lockdown_patch(&seen, LOGIN).is_none()); } + /// The one request `push_branch` sends for `sha`, as a stand-in forge + /// saw it: method, path and JSON body. + async fn pushed(sha: Option<&str>) -> (String, String, serde_json::Value) { + use axum::http::{Method, StatusCode, Uri, header::CONTENT_TYPE}; + + let seen = Arc::new(std::sync::Mutex::new(Vec::new())); + let recorded = Arc::clone(&seen); + let app = axum::Router::new().fallback( + move |method: Method, uri: Uri, body: axum::body::Bytes| { + let recorded = Arc::clone(&recorded); + async move { + recorded.lock().unwrap().push(( + method.to_string(), + uri.path().to_owned(), + serde_json::from_slice(&body).unwrap_or_default(), + )); + ( + StatusCode::CREATED, + [(CONTENT_TYPE, "application/json")], + r#"{"commit":{"sha":"c1"}}"#, + ) + } + }, + ); + 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 }); + let forge = Forgejo::new(Auth::Token("stub-token"), url).unwrap(); + let commit = BranchCommit { + base: "main".to_owned(), + branch: "review-classifier/alice-0a1b2c3d".to_owned(), + path: ".hyperhive/review-classifier.yml".to_owned(), + sha: sha.map(str::to_owned), + content: b"classifications: []\n".to_vec(), + message: "review classifier: add no-ads".to_owned(), + author: CommitAuthor { + name: "Alice".to_owned(), + email: "alice@example.test".to_owned(), + }, + }; + push_branch(&forge, "o", "r", commit).await.expect("pushed"); + let mut seen = seen.lock().unwrap(); + assert_eq!(seen.len(), 1, "{seen:?}"); + seen.remove(0) + } + + #[tokio::test] + async fn a_push_is_one_commit_on_a_new_branch_authored_by_the_operator() { + let (method, path, body) = pushed(Some("b1")).await; + assert_eq!(method, "POST"); + assert_eq!(path, "/api/v1/repos/o/r/contents"); + assert_eq!(body["branch"], "main"); + assert_eq!(body["new_branch"], "review-classifier/alice-0a1b2c3d"); + assert_eq!(body["force_overwrite_new_branch"], false); + assert_eq!(body["message"], "review classifier: add no-ads"); + assert_eq!( + body["author"], + serde_json::json!({ "name": "Alice", "email": "alice@example.test" }) + ); + assert_eq!(body["committer"], serde_json::Value::Null); + assert_eq!( + body["files"], + serde_json::json!([{ + "operation": "update", + "path": ".hyperhive/review-classifier.yml", + "sha": "b1", + // base64 of "classifications: []\n" + "content": "Y2xhc3NpZmljYXRpb25zOiBbXQo=", + "from_path": null, + }]) + ); + } + + #[tokio::test] + async fn a_push_without_a_blob_sha_creates_the_file() { + let (_, _, body) = pushed(None).await; + assert_eq!(body["files"][0]["operation"], "create"); + assert_eq!(body["files"][0]["sha"], serde_json::Value::Null); + } + #[tokio::test] async fn a_commit_author_is_the_accounts_name_and_email_or_none() { use super::super::tests::stub_forge;