diff --git a/docs/swarm/builder.md b/docs/swarm/builder.md index 94705cad..17556cd7 100644 --- a/docs/swarm/builder.md +++ b/docs/swarm/builder.md @@ -225,6 +225,16 @@ is the same case: the old builder has no subscriber for the new subject, so the controller gets no responders at once, and the deploys go out after the builder updates. Each round adds a build and deploy node per failing agent. +When the build belongs to a merged config PR and `nix` exited with an error or hit +its deadline, the controller also comments once on that PR, with its forge +account: the agent, the `internal/meta` commit, `nix`'s last lines (at most 4000 +bytes), and that the deploy waits and the swarm builds it again every round. A comment +carries a marker line with the agent and the commit, and the controller posts +only if the PR holds no comment with that marker, so the retry rounds stay quiet +until `internal/meta` moves. A `no verdict` failure, and a build that isn't a +merged PR's (a new agent, the pin fan-out), leave no comment. When the forge +refuses the comment, the controller logs a warning and the node ends as before. + The controller exports the gauge `swarm.deploys.held`: the agents whose latest build failed, with the label `reason` set to `failed` or `unavailable`. The Grafana alert `hh-deploys-held` fires when the sum stays above zero for 30 minutes, so diff --git a/swarm-controller/src/config_pr.rs b/swarm-controller/src/config_pr.rs index 630f9b28..70a4504b 100644 --- a/swarm-controller/src/config_pr.rs +++ b/swarm-controller/src/config_pr.rs @@ -73,11 +73,13 @@ struct WebhookBranch { sha: Option, } -/// A config PR that was just merged: whose config, and the commit to deploy. +/// A config PR that was just merged: whose config, the commit to deploy, and +/// the PR. #[derive(Debug, PartialEq, Eq)] pub struct MergedConfigPr { pub agent: String, pub rev: String, + pub pr: u64, } /// The merge a verified `ConfigPr` delivery reports, if it reports one. @@ -108,6 +110,7 @@ pub fn merged(body: &[u8]) -> Option { Some(MergedConfigPr { agent: payload.repository.name, rev, + pr: payload.pull_request.number, }) } @@ -373,6 +376,7 @@ mod tests { Some(super::MergedConfigPr { agent: "damocles".to_owned(), rev: "abc123".to_owned(), + pr: 5, }) ); } diff --git a/swarm-controller/src/forge.rs b/swarm-controller/src/forge.rs index 2b89d2d1..062938cd 100644 --- a/swarm-controller/src/forge.rs +++ b/swarm-controller/src/forge.rs @@ -55,6 +55,7 @@ pub mod legacy_tokens; mod meta_repo; pub mod objects; pub mod operator_profiles; +pub(crate) mod pr_comment; pub mod pull_diff; pub mod review_request; pub mod runner_token; diff --git a/swarm-controller/src/forge/pr_comment.rs b/swarm-controller/src/forge/pr_comment.rs new file mode 100644 index 00000000..19a47978 --- /dev/null +++ b/swarm-controller/src/forge/pr_comment.rs @@ -0,0 +1,150 @@ +//! Comments on a pull request, posted once per marker. + +use anyhow::{Context, Result}; +use forgejo_api::structs::{CreateIssueCommentOption, IssueGetCommentsQuery}; + +use super::Client; + +impl Client { + /// Comment `body` on pull request `pr` of `owner/repo` unless a comment + /// there already contains `marker`. Whether a comment was posted. + /// + /// The marker is what the comment carries, so the check holds across a + /// restart and across every caller. It reads the comments first, so two + /// concurrent calls with one marker can both post. + pub(crate) async fn comment_once( + &self, + owner: &str, + repo: &str, + pr: u64, + marker: &str, + body: &str, + ) -> Result { + let index = i64::try_from(pr).context("PR number out of range")?; + let existing = self + .api + .call(|api| { + Box::pin( + api.issue_get_comments( + owner, + repo, + index, + IssueGetCommentsQuery { + since: None, + before: None, + }, + ) + .all(), + ) + }) + .await + .with_context(|| format!("listing the comments on {owner}/{repo}#{pr}"))?; + if existing + .iter() + .any(|c| c.body.as_deref().is_some_and(|b| b.contains(marker))) + { + return Ok(false); + } + let option = CreateIssueCommentOption { + body: body.to_owned(), + updated_at: None, + }; + self.api + .call(|api| { + api.issue_create_comment(owner, repo, index, option.clone()) + .into_future() + }) + .await + .with_context(|| format!("commenting on {owner}/{repo}#{pr}"))?; + Ok(true) + } +} + +#[cfg(test)] +pub(crate) mod tests { + use std::sync::{Arc, Mutex}; + + use axum::http::{Method, StatusCode, Uri}; + + use super::*; + + /// The comments a stand-in forge holds, as the issue path they were + /// posted to and their body. + pub(crate) type Posted = Arc>>; + + /// A stand-in forge that stores the comments posted to any issue path and + /// lists them back. + pub(crate) async fn stand_in() -> (Client, Posted) { + let held: Posted = Arc::default(); + let state = Arc::clone(&held); + let app = axum::Router::new().fallback( + move |method: Method, uri: Uri, body: axum::body::Bytes| { + let state = Arc::clone(&state); + async move { + let path = uri.path().to_owned(); + let mut held = state.lock().unwrap(); + match method.as_str() { + "GET" => { + let list: Vec<_> = held + .iter() + .filter(|(p, _)| *p == path) + .enumerate() + .map( + |(id, (_, body))| serde_json::json!({ "id": id, "body": body }), + ) + .collect(); + let count = list.len().to_string(); + ( + StatusCode::OK, + [("x-total-count", count)], + serde_json::Value::from(list).to_string(), + ) + } + "POST" => { + let sent: serde_json::Value = serde_json::from_slice(&body).unwrap(); + let text = sent["body"].as_str().unwrap().to_owned(); + held.push((path, text.clone())); + ( + StatusCode::CREATED, + [("x-total-count", String::new())], + serde_json::json!({ "id": held.len(), "body": text }).to_string(), + ) + } + _ => ( + StatusCode::NOT_FOUND, + [("x-total-count", String::new())], + r#"{"message":"no"}"#.to_owned(), + ), + } + } + }, + ); + 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::for_test_forge(url), held) + } + + #[tokio::test] + async fn a_marker_is_commented_once_and_a_new_marker_again() { + let (client, held) = stand_in().await; + let once = |marker: &'static str| { + let client = &client; + async move { + client + .comment_once("o", "r", 7, marker, &format!("{marker} text")) + .await + .unwrap() + } + }; + assert!(once("").await, "first comment is posted"); + assert!(!once("").await, "same marker is not posted again"); + assert!(once("").await, "control: another marker posts"); + let held = held.lock().unwrap(); + assert_eq!(held.len(), 2, "{held:?}"); + assert_eq!( + held[0].0, + format!("/api/v1/repos/o/r/{}/7/comments", "issues") + ); + } +} diff --git a/swarm-controller/src/held_deploys.rs b/swarm-controller/src/held_deploys.rs index 0949c8e2..7d9d8078 100644 --- a/swarm-controller/src/held_deploys.rs +++ b/swarm-controller/src/held_deploys.rs @@ -96,6 +96,7 @@ mod tests { .append( SwarmNodeKind::BuildMeta { agent: (*agent).to_owned(), + pr: None, }, Vec::new(), None, diff --git a/swarm-controller/src/jobq.rs b/swarm-controller/src/jobq.rs index f0b5c549..08f73662 100644 --- a/swarm-controller/src/jobq.rs +++ b/swarm-controller/src/jobq.rs @@ -116,7 +116,10 @@ pub(crate) enum SwarmNodeKind { /// /// Carries no rev: it reads the head when it runs, which in most DAGs is /// after the `UpdateMeta` that commits it. - BuildMeta { agent: String }, + /// + /// `pr` is the merged config PR the build belongs to, if any: a red `nix` + /// build is reported on it once. + BuildMeta { agent: String, pr: Option }, /// Re-mint `agent`'s queue secret if it is still stored and old enough. /// See `agent_renewal`. /// @@ -202,10 +205,12 @@ impl hive_jobq_wire::WireNode for SwarmNodeKind { | SwarmNodeKind::MintAgentIdentity { agent } | SwarmNodeKind::MintAgentForgeToken { agent } | SwarmNodeKind::MintAgentMatrixAccount { agent } - | SwarmNodeKind::BuildMeta { agent } | SwarmNodeKind::RenewAgentQueueCredential { agent } => { serde_json::json!({ "agent": agent }) } + SwarmNodeKind::BuildMeta { agent, pr } => { + serde_json::json!({ "agent": agent, "pr": pr }) + } SwarmNodeKind::MintHiveSenderToken { hive } | SwarmNodeKind::MintHiveForgeToken { hive } => serde_json::json!({ "hive": hive }), SwarmNodeKind::MintClassifierForgeToken @@ -414,7 +419,7 @@ async fn run_node_work( SwarmNodeKind::BuildAgent { agent, rev, pr } => { build_agent(&deps, forge, agent, rev, pr).await } - SwarmNodeKind::BuildMeta { agent } => build_meta(&deps, forge, agent).await, + SwarmNodeKind::BuildMeta { agent, pr } => build_meta(&deps, forge, agent, pr).await, SwarmNodeKind::RenewAgentQueueCredential { agent } => renew_queue_credential(&agent).await, SwarmNodeKind::SetAgentWanted { hive, agent } => { set_agent_wanted(deps.wanted.as_deref(), &hive, &agent).await @@ -602,8 +607,9 @@ async fn build_meta( deps: &WorkerDeps, forge: Option>, agent: String, + pr: Option, ) -> hive_jobq::scheduler::Outcome { - meta_build::run(forge.as_deref(), deps.queue.as_ref(), agent).await + meta_build::run(forge.as_deref(), deps.queue.as_ref(), agent, pr).await } /// What an `UpdateMeta` pass decided: the deploys to send, and the placements @@ -746,11 +752,13 @@ fn gated_deploy<'a>( hive: &str, agent: &str, rev: Option, + pr: Option, update: Option, ) -> hive_jobq::builder::NodeRef<'a, SwarmNodeKind, SwarmResourceKind> { let mut build = b .node(SwarmNodeKind::BuildMeta { agent: agent.to_owned(), + pr, }) .needs(SwarmResourceKind::Builder); if let Some(update) = update { @@ -785,7 +793,7 @@ fn declare_fan_out( } let deploys: Vec = placements .iter() - .map(|(hive, agent)| gated_deploy(grown, hive, agent, None, None).guid()) + .map(|(hive, agent)| gated_deploy(grown, hive, agent, None, None, None).guid()) .collect(); if let FanOut::Record { pin } = fan_out { let mut record = grown.node(SwarmNodeKind::RecordFanout { pin: pin.clone() }); @@ -1173,7 +1181,7 @@ pub(crate) async fn queue_merged_config_deploy( state: &AppState, merged: config_pr::MergedConfigPr, ) { - let config_pr::MergedConfigPr { agent, rev } = merged; + let config_pr::MergedConfigPr { agent, rev, pr } = merged; let declared = match read_declarations(state, state.hives.iter()).await { Ok(declared) => declared, Err((hive, e)) => { @@ -1197,7 +1205,7 @@ pub(crate) async fn queue_merged_config_deploy( .jobq .lock() .unwrap_or_else(std::sync::PoisonError::into_inner) - .insert_job(None, |b| merged_deploy_job(b, &hive, &agent, &rev)); + .insert_job(None, |b| merged_deploy_job(b, &hive, &agent, &rev, pr)); match inserted { Ok(_) => tracing::info!(%agent, %rev, %hive, "config-pr merge: deploy queued"), Err(e) => { @@ -1218,12 +1226,13 @@ fn merged_deploy_job( hive: &str, agent: &str, rev: &str, + pr: u64, ) -> Vec { let update_meta = b .node(SwarmNodeKind::UpdateMeta) .needs(SwarmResourceKind::Meta); let root = update_meta.guid(); - let _deploy = gated_deploy(b, hive, agent, Some(rev.to_owned()), Some(root)); + let _deploy = gated_deploy(b, hive, agent, Some(rev.to_owned()), Some(pr), Some(root)); vec![root] } @@ -1347,7 +1356,7 @@ pub(crate) fn declare_agent_job( // queue at all fails this node — but `TriggerDeploy` has nothing to // publish to on such a host either, so the strict edge cancels a node // that could not have succeeded anyway. - let trigger_deploy = gated_deploy(b, hive, agent, None, Some(update_meta.guid())) + let trigger_deploy = gated_deploy(b, hive, agent, None, None, Some(update_meta.guid())) .after_any(mint_identity) .after_any(mint_forge_token) // `after_any` for the same reason: the container reads this token @@ -1561,6 +1570,9 @@ struct SkippedDeploy { hive: String, agent: String, rev: Option, + /// The merged config PR the failed build belonged to, so the build + /// declared again reports on it too. + pr: Option, cause: SkipCause, } @@ -1592,14 +1604,15 @@ fn skipped_deploys( let SwarmNodeKind::TriggerDeploy { hive, agent, rev } = &n.payload else { return None; }; - let failed = |is: fn(&SwarmNodeKind) -> bool| { - n.deps.iter().any(|d| match d { - hive_jobq::Dep::Node { id, .. } => graph.node(*id).is_some_and(|dep| { - is(&dep.payload) && dep.state == hive_jobq::State::Failed - }), - hive_jobq::Dep::Resource { .. } => false, + let dep_nodes = || { + n.deps.iter().filter_map(|d| match d { + hive_jobq::Dep::Node { id, .. } => graph.node(*id), + hive_jobq::Dep::Resource { .. } => None, }) }; + let failed = |is: fn(&SwarmNodeKind) -> bool| { + dep_nodes().any(|dep| is(&dep.payload) && dep.state == hive_jobq::State::Failed) + }; let cause = if failed(|p| matches!(p, SwarmNodeKind::UpdateMeta)) { SkipCause::FailedUpdate } else if failed(|p| matches!(p, SwarmNodeKind::BuildMeta { .. })) { @@ -1607,10 +1620,15 @@ fn skipped_deploys( } else { return None; }; + let pr = dep_nodes().find_map(|dep| match &dep.payload { + SwarmNodeKind::BuildMeta { pr, .. } => *pr, + _ => None, + }); Some(SkippedDeploy { hive: hive.clone(), agent: agent.clone(), rev: rev.clone(), + pr, cause, }) }) @@ -1666,7 +1684,7 @@ fn queue_meta_update( }); let deploys: Vec<_> = skipped .into_iter() - .map(|s| gated_deploy(b, &s.hive, &s.agent, s.rev, update).guid()) + .map(|s| gated_deploy(b, &s.hive, &s.agent, s.rev, s.pr, update).guid()) .collect(); update.map_or(deploys, |update| vec![update]) }) @@ -2294,7 +2312,7 @@ mod tests { } fn merged_dag(b: &TestBuilder) { - let _ = super::merged_deploy_job(b, "pr1ma", "atlas", "abc"); + let _ = super::merged_deploy_job(b, "pr1ma", "atlas", "abc", 7); } /// A deploy needs the meta update to succeed: its edge refuses a failed @@ -2890,7 +2908,7 @@ mod tests { .filter(|n| n.payload.label() == label) .filter(|n| n.state == State::Pending) .map(|n| match &n.payload { - SwarmNodeKind::BuildMeta { agent } + SwarmNodeKind::BuildMeta { agent, .. } | SwarmNodeKind::TriggerDeploy { agent, .. } => agent.clone(), other => panic!("not a build or a deploy: {other:?}"), }) @@ -3811,12 +3829,27 @@ mod tests { nodes.into_iter().map(|(_, rev)| rev).collect() } + /// The `pr` of `agent`'s builds, oldest first. + fn build_prs(sched: &SharedSched, agent: &str) -> Vec> { + let guard = sched.lock().unwrap(); + let mut nodes: Vec<_> = guard + .graph() + .nodes() + .filter_map(|n| match &n.payload { + SwarmNodeKind::BuildMeta { agent: a, pr } if a == agent => Some((n.id, *pr)), + _ => None, + }) + .collect(); + nodes.sort_by_key(|(id, _)| *id); + nodes.into_iter().map(|(_, pr)| pr).collect() + } + fn merged_atlas() -> SharedSched { let sched = fresh_sched(); sched .lock() .unwrap() - .insert_job(None, |b| merged_deploy_job(b, "pr1ma", "atlas", "abc")) + .insert_job(None, |b| merged_deploy_job(b, "pr1ma", "atlas", "abc", 7)) .expect("the graph inserts"); sched } @@ -3865,11 +3898,27 @@ mod tests { .iter() .all(|rev| rev.as_deref() == Some("abc")) ); + // So does the PR it belongs to: every retried build reports on it too. + assert_eq!(build_prs(&sched, "atlas"), [Some(7); 4]); assert!( !queue_meta_update(&sched).expect("tick"), "the success ends the retries" ); } + + /// Only a merged config PR's build carries a PR: the creation DAG's does + /// not, so its failure has nobody to comment on. + #[test] + fn only_a_merged_prs_build_carries_its_pr() { + let sched = merged_atlas(); + sched + .lock() + .unwrap() + .insert_job(None, |b| declare_agent_job(b, "iris", "pr1ma", false)) + .expect("the graph inserts"); + assert_eq!(build_prs(&sched, "atlas"), [Some(7)]); + assert_eq!(build_prs(&sched, "iris"), [None]); + } } #[cfg(test)] diff --git a/swarm-controller/src/meta_build.rs b/swarm-controller/src/meta_build.rs index 29f7d125..16a8d2d3 100644 --- a/swarm-controller/src/meta_build.rs +++ b/swarm-controller/src/meta_build.rs @@ -19,6 +19,11 @@ //! //! Both are built again on every round of the meta timer //! (`jobq::queue_meta_update`) until a build succeeds. +//! +//! A `nix` failure of a merged config PR's build (`pr` set) also leaves one +//! comment on that PR, deduped by a marker in the comment per agent and +//! `internal/meta` rev, so the retry rounds stay quiet. A no-verdict failure +//! says nothing about the config and leaves none. use std::fmt::Display; use std::future::Future; @@ -51,6 +56,15 @@ pub(crate) trait MetaRepo { fn head(&self) -> impl Future>> + Send; /// Make sure the builder's forge account can read the repo. fn grant_builder_read(&self) -> impl Future> + Send; + /// Comment `body` on `pr` of `agent`'s config repo unless a comment there + /// already contains `marker`. Whether a comment was posted. + fn comment_once( + &self, + agent: &str, + pr: u64, + marker: &str, + body: &str, + ) -> impl Future> + Send; } impl MetaRepo for forge::Client { @@ -67,6 +81,14 @@ impl MetaRepo for forge::Client { ) .await } + + async fn comment_once(&self, agent: &str, pr: u64, marker: &str, body: &str) -> Result { + crate::jobq::forge_node( + "build_meta", + forge::Client::comment_once(self, forge::CONFIG_ORG, agent, pr, marker, body), + ) + .await + } } /// Where a build is sent and its closure recorded: the queue in production, a @@ -119,11 +141,55 @@ impl MetaBuilds for async_nats::Client { } } +/// The most of `nix`'s tail a PR comment carries, in bytes: the builder's own cap +/// on the tail it sends. +const COMMENT_TAIL_BYTES: usize = 4000; + +/// The comment a red build leaves on its merged config PR, first line the +/// marker the dedupe looks for. One marker per agent and `internal/meta` rev. +fn failure_comment(agent: &str, meta_rev: &str, error_tail: &str) -> (String, String) { + let marker = format!(""); + let mut start = error_tail.len().saturating_sub(COMMENT_TAIL_BYTES); + while !error_tail.is_char_boundary(start) { + start += 1; + } + let tail = error_tail[start..].replace("```", "'''"); + let body = format!( + "{marker}\nThe swarm could not build `{agent}` from `internal/meta` at `{meta_rev}` \ + after this PR merged. The deploy is held and built again on every round until a \ + build succeeds.\n\nLast lines of `nix`:\n\n```\n{}\n```", + tail.trim_end() + ); + (marker, body) +} + +/// Tell `pr` that its merge's build failed, once per agent and `meta_rev`. A +/// comment that cannot be posted is logged and changes nothing about the node. +async fn report_failure( + head: &impl MetaRepo, + pr: u64, + agent: &str, + meta_rev: &str, + error_tail: &str, +) { + let (marker, body) = failure_comment(agent, meta_rev, error_tail); + match head.comment_once(agent, pr, &marker, &body).await { + Ok(posted) => { + tracing::info!(%agent, pr, %meta_rev, posted, "build_meta: reported the failed build on the PR"); + } + Err(e) => tracing::warn!( + %agent, pr, %meta_rev, error = %format!("{e:#}"), + "build_meta: could not comment on the PR" + ), + } +} + /// Build `agent` from `internal/meta`'s head and record the closure. pub(crate) async fn run( head: Option<&impl MetaRepo>, queue: Option<&impl MetaBuilds>, agent: String, + pr: Option, ) -> Outcome { let Some(head) = head else { return no_verdict( @@ -155,7 +221,12 @@ pub(crate) async fn run( match queue.build(request).await { Err(why) => no_verdict(why), Ok(FromMetaReply::Unavailable { reason }) => no_verdict(reason), - Ok(FromMetaReply::Failed { error_tail }) => Outcome::Failed(error_tail), + Ok(FromMetaReply::Failed { error_tail }) => { + if let Some(pr) = pr { + report_failure(head, pr, &agent, &meta_rev, &error_tail).await; + } + Outcome::Failed(error_tail) + } Ok(FromMetaReply::Built { out_path, system }) => { let closure = AgentClosure { meta_rev, @@ -184,6 +255,8 @@ mod tests { struct FakeHead { head: Result, &'static str>, grant_fails: bool, + /// The forge a PR comment goes to; `None` panics on a comment. + comments: Option>, } impl FakeHead { @@ -191,6 +264,7 @@ mod tests { Self { head, grant_fails: false, + comments: None, } } } @@ -207,6 +281,19 @@ mod tests { anyhow::ensure!(!self.grant_fails, "the forge refused the collaborator"); Ok(()) } + + async fn comment_once( + &self, + agent: &str, + pr: u64, + marker: &str, + body: &str, + ) -> Result { + let client = self.comments.as_ref().expect("no comment expected"); + client + .comment_once(forge::CONFIG_ORG, agent, pr, marker, body) + .await + } } /// A queue that answers every build with `reply`, and keeps what it was @@ -256,7 +343,11 @@ mod tests { } async fn run_with(head: &FakeHead, queue: &FakeQueue) -> Outcome { - run(Some(head), Some(queue), "atlas".to_owned()).await + run(Some(head), Some(queue), "atlas".to_owned(), None).await + } + + async fn run_for_pr(head: &FakeHead, queue: &FakeQueue, pr: u64) -> Outcome { + run(Some(head), Some(queue), "atlas".to_owned(), Some(pr)).await } #[tokio::test] @@ -374,6 +465,7 @@ mod tests { head.as_ref(), with_queue.then_some(&queue), "atlas".to_owned(), + None, ) .await; let Outcome::Failed(error) = outcome else { @@ -384,6 +476,137 @@ mod tests { } } + const REV2: &str = "fedcba9876543210fedcba9876543210fedcba98"; + + fn nix_failed(tail: &str) -> FakeQueue { + FakeQueue::answering(Ok(FromMetaReply::Failed { + error_tail: tail.to_owned(), + })) + } + + async fn head_with_comments( + rev: &'static str, + ) -> (FakeHead, crate::forge::pr_comment::tests::Posted) { + let (client, posted) = crate::forge::pr_comment::tests::stand_in().await; + let head = FakeHead { + comments: Some(std::sync::Arc::new(client)), + ..FakeHead::reading(Ok(Some(rev))) + }; + (head, posted) + } + + /// A `nix` failure of a merged config PR's build is one comment on that PR + /// with the agent, the `internal/meta` rev, `nix`'s tail and the retry. + #[tokio::test] + async fn a_red_build_of_a_merged_pr_comments_once_on_it() { + let (head, posted) = head_with_comments(REV).await; + let outcome = run_for_pr(&head, &nix_failed("error: builder for foo failed"), 7).await; + assert_eq!( + outcome, + Outcome::Failed("error: builder for foo failed".to_owned()) + ); + let posted = posted.lock().unwrap(); + let [(path, body)] = posted.as_slice() else { + panic!("one comment: {posted:?}") + }; + assert_eq!( + path, + &format!("/api/v1/repos/agent-configs/atlas/{}/7/comments", "issues") + ); + assert!(body.contains("`atlas`"), "{body}"); + assert!(body.contains(REV), "{body}"); + assert!(body.contains("error: builder for foo failed"), "{body}"); + assert!(body.contains("held"), "{body}"); + assert!(body.contains("every round"), "{body}"); + } + + /// The timer builds a failed merge again on every round: the same agent and + /// `internal/meta` rev is not commented twice. A new rev is a new failure, + /// so it is. + #[tokio::test] + async fn the_retry_rounds_do_not_comment_again_but_a_new_meta_rev_does() { + let (head, posted) = head_with_comments(REV).await; + for _ in 0..3 { + run_for_pr(&head, &nix_failed("error: boom"), 7).await; + } + let count = posted.lock().unwrap().len(); + assert_eq!(count, 1, "quiet retries"); + + let moved = FakeHead { + comments: head.comments.clone(), + ..FakeHead::reading(Ok(Some(REV2))) + }; + run_for_pr(&moved, &nix_failed("error: boom"), 7).await; + let posted = posted.lock().unwrap(); + assert_eq!(posted.len(), 2, "{posted:?}"); + assert!(posted[1].1.contains(REV2), "{:?}", posted[1]); + } + + /// A build that got no verdict says nothing about the PR's config, so it + /// leaves no comment. The `nix` failure arm is the control. + #[tokio::test] + async fn a_no_verdict_failure_does_not_comment() { + for (name, reply) in [ + ("no responders", Err("no swarm builder".to_owned())), + ( + "unavailable", + Ok(FromMetaReply::Unavailable { + reason: "busy".to_owned(), + }), + ), + ] { + let (head, posted) = head_with_comments(REV).await; + let outcome = run_for_pr(&head, &FakeQueue::answering(reply), 7).await; + assert!( + matches!(outcome, Outcome::Failed(e) if is_no_verdict(&e)), + "{name}" + ); + assert!(posted.lock().unwrap().is_empty(), "{name}"); + } + let (head, posted) = head_with_comments(REV).await; + run_for_pr(&head, &nix_failed("error: boom"), 7).await; + assert_eq!( + posted.lock().unwrap().len(), + 1, + "control: a nix failure comments" + ); + } + + /// A build that is not a merged PR's (the pin fan-out, a created agent) + /// has nobody to comment on. + #[tokio::test] + async fn a_failed_build_that_is_not_from_a_pr_does_not_comment() { + let (head, posted) = head_with_comments(REV).await; + let outcome = run_with(&head, &nix_failed("error: boom")).await; + assert_eq!(outcome, Outcome::Failed("error: boom".to_owned())); + assert!(posted.lock().unwrap().is_empty()); + } + + #[tokio::test] + async fn the_comment_carries_the_end_of_a_long_tail_only() { + let tail = format!("START{}END", "x".repeat(10_000)); + let (head, posted) = head_with_comments(REV).await; + run_for_pr(&head, &nix_failed(&tail), 7).await; + let posted = posted.lock().unwrap(); + let body = &posted[0].1; + assert!(body.contains("END"), "the end of the tail is kept"); + assert!(!body.contains("START"), "the start is cut"); + assert!(body.len() < COMMENT_TAIL_BYTES + 1000, "{}", body.len()); + } + + /// A forge that does not take the comment leaves the node failed as it was. + #[tokio::test] + async fn a_comment_that_cannot_be_posted_does_not_change_the_outcome() { + let head = FakeHead { + comments: Some(std::sync::Arc::new(crate::forge::Client::for_test_forge( + url::Url::parse("http://127.0.0.1:1/").unwrap(), + ))), + ..FakeHead::reading(Ok(Some(REV))) + }; + let outcome = run_for_pr(&head, &nix_failed("error: boom"), 7).await; + assert_eq!(outcome, Outcome::Failed("error: boom".to_owned())); + } + #[test] fn the_repo_path_the_builder_fetches_is_the_one_the_controller_writes() { assert_eq!(