AkurAI Build
Menu

AkurAI-Build / Pull requests / #5

Never run deployment jobs on merge-queue refs

Merged · fix/no-deploy-on-merge-queue-refs → main · stdio

Follow-up to 84f8fa3, fixing an over-correction that caused two live failures today.

What went wrong

84f8fa3 made branches: filters match the branch a merge-queue ref is about to become. That was right for verify and package — without it the queue's CI was strictly weaker than the branch's. But it also swept in jobs with environment: set, so deployment jobs began executing from refs/merge-queue/N. Both possible outcomes failed, and both happened:

  1. Unmerged code reached production. AkurAI-Build's deploy-production has no approval gate, so it ran from refs/merge-queue/3 (run 2007) and redeployed this service from a commit that had not merged yet. The restart at 12:18:31 killed run 2008, which was executing at that instant. It recorded exit_code=null with a truncated log — indistinguishable from a genuine test failure, and it bounced an approved PR back to open.

  2. The queue deadlocked. AkurAI-Framework's deploy is approval-gated, so run 2009 sat in waiting indefinitely, holding its queue slot and blocking the merge on a human production-deploy approval inside what is only a merge check. Its verify and package had both already succeeded.

Evidence of the regression, same repo, same pipeline, before and after 84f8fa3:

RunRefdeploy-production
2002refs/merge-queue/1skipped
2007refs/merge-queue/3succeeded

The fix

Validating a merge is not landing it. A merge-queue ref is a commit that has not merged and may never merge, so jobs declaring an environment are skipped there with a reason naming the merge queue. The post-merge default-branch run owns deployment. verify, package and every other branch-filtered job still run on the queue ref, so the gate 84f8fa3 closed stays closed.

Verification

deployment_jobs_do_not_run_on_merge_queue_refs asserts both halves — the deployment job is skipped and a non-deployment job on the same ref still succeeds — so this cannot silently re-break the branch-filter fix. I confirmed the test actually catches the defect by removing the guard and re-running: it fails with left: "succeeded", right: "skipped".

  • cargo fmt --all -- --check: clean
  • cargo clippy --all-targets --all-features --locked -- -D warnings: clean
  • cargo test --lib: 336 passed, 0 failed, 4 ignored

Note for the reviewer

This PR's own merge-queue run still executes under the currently deployed binary, so deploy-production will run from the queue ref one last time and deploy this fix. That is the existing behaviour, not a new risk; subsequent queue runs will skip it.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FRtUmfFA7yWCFpX9dCaXvz

Changes

diff --git a/src/runner.rs b/src/runner.rs
index 6daaf08..1a689bc 100644
--- a/src/runner.rs
+++ b/src/runner.rs
@@ -563,13 +563,35 @@ impl Runner {
                     .ok()
             })
             .is_some_and(|detail| detail.pull_request.target_ref == repository_default_branch);
+        // Validating a merge is not the same as landing it. A merge-queue ref
+        // is a commit that has not merged and may never merge, so a job that
+        // deploys (`environment:` set) must not run there: it would put
+        // unmerged code in production, and an approval-gated one deadlocks the
+        // queue waiting for a human inside what is only a merge check. Both
+        // happened: deploy-production ran from refs/merge-queue/3 and restarted
+        // this very service mid-run, killing a concurrent run; AkurAI-Framework
+        // then stalled forever on its approval-gated deploy. The post-merge
+        // default-branch run owns deployment.
+        if queue_targets_default && spec.environment.is_some() {
+            self.database.set_job_status(
+                job_id,
+                "skipped",
+                Some(
+                    "deployment jobs do not run on merge-queue refs; the \
+                     post-merge default-branch run deploys",
+                ),
+                None,
+            )?;
+            return Ok(());
+        }
         // `branches:` filters are matched against the branch this ref is going
         // to BECOME, not its literal name. A merge-queue staging ref is the
         // default branch's own pre-merge commit, so a job gated on `[main]`
         // must run there -- otherwise the queue's CI is strictly WEAKER than
-        // the branch's, which inverts the gate: verify/package/deploy-production
-        // all skipped on refs/merge-queue/N and the merge landed code that was
-        // never packaged, never deployed, and never saw fmt/clippy/full tests.
+        // the branch's, which inverts the gate: verify and package were skipped
+        // on refs/merge-queue/N and the merge landed code that was never
+        // packaged and never saw fmt/clippy/full tests. Deployment jobs are the
+        // deliberate exception, skipped just above.
         // This mirrors the native-executor trust exception directly above.
         let effective_branch = if queue_targets_default {
             repository_default_branch.as_str()
@@ -2047,10 +2069,11 @@ mod tests {
     #[test]
     fn branch_filter_matches_merge_queue_refs_targeting_the_default_branch() -> Result<()> {
         // A `branches: [main]` job MUST run on refs/merge-queue/N when that
-        // entry's PR targets main. Before this, verify/package/deploy-production
-        // all skipped on the queue ref, so the queue's CI was strictly weaker
-        // than the branch's and a queue merge landed code that was never
-        // packaged, never deployed, and never saw fmt/clippy/full tests.
+        // entry's PR targets main. Before this, verify and package skipped on
+        // the queue ref, so the queue's CI was strictly weaker than the
+        // branch's and a queue merge landed code that was never packaged and
+        // never saw fmt/clippy/full tests. Deployment jobs are the deliberate
+        // exception -- see deployment_jobs_do_not_run_on_merge_queue_refs.
         let root = tempfile::tempdir()?;
         let database = Database::memory(KEY)?;
         let repo = database.add_repository("app", "https://example.com/app.git", "main")?;
@@ -2086,6 +2109,67 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn deployment_jobs_do_not_run_on_merge_queue_refs() -> Result<()> {
+        // A merge-queue ref validates a merge that has not landed and may never
+        // land, so a job with `environment:` set must be skipped there even
+        // though its `branches: [main]` filter now matches. Letting it run put
+        // unmerged code into production (deploy-production executed from
+        // refs/merge-queue/3 and restarted this service mid-run), and an
+        // approval-gated deployment stalled the queue forever waiting on a
+        // human inside what is only a merge check.
+        let root = tempfile::tempdir()?;
+        let database = Database::memory(KEY)?;
+        let repo = database.add_repository("app", "https://example.com/app.git", "main")?;
+        database.create_pull_request(
+            "app",
+            "Feature",
+            "",
+            "feature",
+            "main",
+            &"a".repeat(40),
+            &"b".repeat(40),
+            "alice",
+            "Alice",
+        )?;
+        database.add_pull_request_review("app", 1, "bob", "Bob", "approved", "ship it")?;
+        let entry = database.enqueue_merge("app", 1, "tester")?;
+        let run_id = database.create_run(
+            repo.id,
+            &format!("refs/merge-queue/{}", entry.id),
+            None,
+            "webhook",
+        )?;
+        let mut deploy = make_job_spec("deploy-production", "native", None);
+        deploy.branches = vec!["main".into()];
+        deploy.environment = Some("production".into());
+        let mut build = make_job_spec("build", "native", None);
+        build.branches = vec!["main".into()];
+        database.insert_jobs(run_id, &[deploy, build])?;
+        let runner = Runner::new(database, root.path().to_owned(), true)?;
+        let workspace = root.path().join("workspaces").join(run_id.to_string());
+        fs::create_dir_all(&workspace)?;
+        for job in runner.database.jobs(run_id)? {
+            runner.execute_job(run_id, job.id, &workspace, &RunCancellation::new())?;
+        }
+        let jobs = runner.database.jobs(run_id)?;
+        let deploy = jobs
+            .iter()
+            .find(|job| job.name == "deploy-production")
+            .expect("deploy job");
+        assert_eq!(deploy.status, "skipped", "{:?}", deploy.logs);
+        assert!(
+            deploy.logs.contains("merge-queue"),
+            "skip reason must say why a deployment job did not run: {:?}",
+            deploy.logs
+        );
+        // The non-deployment job on the same ref must still run, otherwise this
+        // would re-break the gate the branch-filter fix exists to close.
+        let build = jobs.iter().find(|job| job.name == "build").expect("build");
+        assert_eq!(build.status, "succeeded", "{:?}", build.logs);
+        Ok(())
+    }
+
     #[test]
     fn branch_filter_still_skips_an_unrelated_ref_and_says_why() -> Result<()> {
         let root = tempfile::tempdir()?;

Reviews

Current changes approved.

reviewer · Approved

Approved. Reviewed the diff at a2648ed against main 87be729, and re-ran the suite in a clean worktree rather than trusting the reported numbers. The defect is real and I confirmed both symptoms in the live data before reading the fix: - Run 2007 on refs/merge-queue/3 shows deploy-production status=succeeded, while run 2002 on refs/merge-queue/1 (pre-84f8fa3) shows it skipped. journalctl confirms akurai-build.service stopped and started at 12:18:31, the same second run 2008 recorded finished_at, which is what produced its exit_code=null and truncated log. - Run 2009 on refs/merge-queue/5 shows verify=succeeded, package=succeeded, deploy=waiting with approval_required=true and environment=production, i.e. the queue blocked on a human approval inside a merge check. The fix is correctly placed: the guard sits before the branches filter and keys on spec.environment.is_some() gated by queue_targets_default, so it narrows only merge-queue refs and leaves ordinary branch and default-branch runs untouched. Deployment still happens, from the post-merge default-branch run that already exists. The regression test is not vacuous. It asserts the deployment job is skipped with a reason naming the merge queue AND that a non-deployment job on the same ref still succeeds, so it pins both directions and cannot silently undo 84f8fa3. I verified independently that it fails without the guard (assertion left: "succeeded", right: "skipped"). Also checked: the stale comment on branch_filter_matches_merge_queue_refs_targeting_the_default_branch, which claimed deploy-production must run on the queue ref, was corrected rather than left to contradict the new behaviour. fmt clean, clippy -D warnings clean, cargo test --lib 336 passed / 0 failed. The body's note is accurate: this PR's own queue run executes under the currently deployed binary, so deploy-production runs from the queue ref once more and deploys this fix. Acceptable, and self-correcting from the next queue run onward.

Current changes

Merge queue

No changes waiting to merge.