diff --git a/.repository-projection.json b/.repository-projection.json index 40ab7b2..7ace0b6 100644 --- a/.repository-projection.json +++ b/.repository-projection.json @@ -3,11 +3,11 @@ "projection": "borg", "projectionSchemaVersion": 1, "sourceRepository": "dx-corp/mono", - "sourceSha": "0cf84f90bfd2316c7942db9040adf02600a4185f", + "sourceSha": "957fcce869bc4aa2aeca09a23dcb933b23437704", "destinationRepository": "dx-corp/borg", - "priorProjectedBase": "d9fa852ceaab9c0d87678461edf39376ac996ab5", + "priorProjectedBase": "fca0fb4012d288364bc6dd8a20f28537f4829584", "definitionDigest": "78ba8d1b5794c261ff0afba6d99abc10b9297ea0dd94be4397fb1cbf63cf0c6e", "toolDigest": "3236c76d5bddbdab39d448c244f17fb36fa84e10", - "contentDigest": "017dbfc9a5add83f90bc00e037efe2247c8a235c22a751939922c8fa647ba96f", + "contentDigest": "1ea246ccf3856b94f5430e9bb5279ceb192487c91f7388bd972723fd097a3661", "publicationEligible": true } diff --git a/src/janitor.rs b/src/janitor.rs index 1873728..0cf4f27 100644 --- a/src/janitor.rs +++ b/src/janitor.rs @@ -11,7 +11,8 @@ //! superseded runs that already started, and `dedupe_same_sha` cancels queued //! duplicates of a run of the same workflow, commit, event and pull request. //! Duplicate groups never include `workflow_dispatch` or `merge_group` runs, -//! and a running duplicate is never cancelled. The +//! and a running duplicate is never cancelled, even when its workflow still +//! reads `queued`. An unreadable jobs response also preserves the run. The //! janitor is opt-in (`supersede_janitor` in the queen config). Cancelling //! runs needs `actions: write`: the janitor uses the queen's App installation //! token (minted and refreshed on demand) unless the config names a static @@ -97,8 +98,8 @@ pub struct PassReport { /// Superseded push runs left alone because one of their jobs is running. /// GitHub shows a run as `queued` while any job waits for a runner. pub started_push: u32, - /// Runs that changed state between the listing and the cancel and were - /// left alone. + /// Runs left alone when a fresh status, head or active child job + /// contradicted the listing used to select them. pub raced: u32, /// Runs whose cancel GitHub answered with 409 (already finished, or stuck /// in a state GitHub will not cancel). @@ -597,6 +598,19 @@ impl Janitor { report.deferred += 1; continue; } + // A waiting sibling can keep the workflow queued after jobs start. + match self.has_running_job(repo, run.id).await { + Ok(false) => {} + Ok(true) => { + report.raced += 1; + continue; + } + Err(e) => { + tracing::warn!("janitor: {repo} run {}: {e}", run.id); + report.errors += 1; + continue; + } + } let why = format!( "duplicate of run {kept} (workflow {}, head {})", run.workflow_id, run.head_sha diff --git a/tests/janitor.rs b/tests/janitor.rs index 8cc629a..2b4c59f 100644 --- a/tests/janitor.rs +++ b/tests/janitor.rs @@ -294,6 +294,64 @@ async fn queued_duplicates_keep_the_lowest_id() { assert_eq!(r.duplicates, 0, "{r:?}"); } +#[tokio::test] +async fn a_queued_duplicate_with_a_running_job_is_kept() { + // GitHub's workflow stays queued while a sibling job waits for a runner. + // cancel_running must never opt same-head duplicate cleanup into killing it. + for cancel_running in [false, true] { + let h = Harness::start(with(json!({ + "dedupe_same_sha": true, "cancel_running": cancel_running + }))) + .await; + h.state().pulls.insert(10, "sha".into()); + h.add_run(4, "queued", "pull_request", "sha", Some(10)); + h.add_run(9, "queued", "pull_request", "sha", Some(10)); + h.add_run_job(9, "active", "in_progress", Some("runner-1")); + h.add_run_job(9, "waiting", "queued", None); + + let r = h.janitor_pass().await; + assert_eq!( + (r.duplicates, r.dedup_cancelled, r.errors, r.raced), + (1, 0, 0, 1), + "{r:?}" + ); + assert!(h.state().cancelled.is_empty()); + } +} + +#[tokio::test] +async fn an_unreadable_duplicate_job_response_preserves_the_run() { + let h = Harness::start(with(json!({"dedupe_same_sha": true}))).await; + h.state().pulls.insert(10, "sha".into()); + h.add_run(4, "queued", "pull_request", "sha", Some(10)); + h.add_run(9, "queued", "pull_request", "sha", Some(10)); + // An invalid provider response cannot establish that no job is running. + h.state() + .jobs + .push(json!({"id": 90, "run_id": 9, "status": false})); + + let r = h.janitor_pass().await; + assert_eq!((r.dedup_cancelled, r.errors), (0, 1), "{r:?}"); + assert!(h.state().cancelled.is_empty()); +} + +#[tokio::test] +async fn keeping_an_active_duplicate_leaves_the_cancel_budget_for_an_idle_one() { + let h = Harness::start(with(json!({ + "dedupe_same_sha": true, "max_cancels_per_pass": 1 + }))) + .await; + h.state().pulls.insert(10, "sha".into()); + for id in [4, 6, 9] { + h.add_run(id, "queued", "pull_request", "sha", Some(10)); + } + h.add_run_job(6, "active", "in_progress", Some("runner-1")); + + let r = h.janitor_pass().await; + assert_eq!((r.dedup_cancelled, r.deferred, r.raced), (1, 0, 1), "{r:?}"); + assert_eq!(h.state().cancelled, [9]); +} + #[tokio::test] async fn a_running_duplicate_is_kept_and_never_cancelled() { let h = Harness::start(with(json!({"dedupe_same_sha": true}))).await;