CI: silent merge check #33145

m3dwards commented at 2:53 PM on August 6, 2025: contributor

In this design there is a new GHA job merged into the main repo that adds check runs to each PR on whether they have a silent merge conflict or not.

The idea is that we have a job that pretty much runs 24/7 triggered every 6 hours or so and set to run for 5.5 hours, it could be set to run on a GHA runner rather than a Cirrus runner. Obviously running on a Cirrus runner (or even multiple runners) would dramatically speed up how many PRs can be checked for silent merge conflicts.

The job uses the Github checks API to add a passing or failing test run to each PR directly. The silent merge check job itself should always pass, it writes pass or fail checks to the PRs.

It will look through all open PRs and discard anything that has a git merge failure or a failing test with the thinking that there is no point checking something that can't be merged anyway. Then it will select PRs that have not had a silent merge check and one by one pull the merge branch of the PR and the full CI job "Previous releases" on that branch.

When all PRs have been checked it will then re-check a PR that was checked the longest time ago.

DrahtBot added the label Tests on Aug 6, 2025

DrahtBot commented at 2:53 PM on August 6, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33145.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	fjahr

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

No conflicts as of last run.

in .github/workflows/silentmerge.yml:39 in 1b5747233f

  34 | +            echo "::notice title=Runner Selection::Using GitHub-hosted runners"
  35 | +          fi
  36 | +
  37 | +  previous-releases:
  38 | +    name: 'Previous releases, depends DEBUG'
  39 | +    if: contains(github.event.label.name, 'PeriodicMergeCICheck')

maflcko commented at 3:09 PM on August 6, 2025:

Seems fine, but currently a vector of task names is accepted, see --task in https://github.com/maflcko/DrahtBot/blob/2038d4d541b89bf2601614b5656d7d30d73e17be/rerun_ci/src/main.rs#L7-L23

It would be good to allow the same somehow?

m3dwards commented at 2:29 PM on August 7, 2025:

Can you elaborate a little more on what this would look like? Would this be different labels for different jobs? As in a label that runs the Previous Releases job and a different label that would run a different job?

maflcko commented at 2:40 PM on August 7, 2025:

I don't know. Not sure if we want to see pull requests spammed with "added and removed" label events (https://github.com/testing-cirrus-runners/bitcoin/pull/19#event-19014793971).

GitHub doesn't make this easy and I wonder if we may want to just spin our own CI to detect silent merge conflicts.

maflcko commented at 2:56 PM on August 7, 2025:

To give some more context: We are now trying to add 200+ lines of code to this repo which have to be maintained, but they cause label spam, and likely aren't sufficient to achieve the goal, because the ci-failed notification will go to the one adding the label and triggering the workflow?

If so, we'd have to create a comment (or other notification) anyway. So if additional glue code is needed, we might as well handle all code externally.

m3dwards commented at 4:14 PM on August 8, 2025:

It's a fair comment. I'll see what the ci-failed notification could look like but you could be right that this approach is less than ideal. @0xB10C has suggested perhaps merging this into the main CI file but might come with a verbose predicate before each job.

maflcko commented at 3:32 PM on August 20, 2025:

Yeah, merging this with the main CI file is certainly better, if possible. However, it still leaves the issue of label spam. Also, it needs to be confirmed to be working correctly (to send the failed-notification to the right person)

Sammie05 commented at 7:54 PM on August 6, 2025: none

Nice work on this! The label-based trigger for PeriodicMergeCICheck is a clean approach.

One thought though since this is driven by label events, do we need to consider cases where multiple labels are applied at once? Would the condition still behave as expected?

Also agree with the suggestion from @maflcko having a vector of task names could as well improve flexibility

maflcko commented at 8:32 AM on August 7, 2025: member

Other stuff to check would be that this works at all and doesn't just re-run an ancient commit, like #32989 (comment)

m3dwards commented at 2:26 PM on August 7, 2025: contributor

Other stuff to check would be that this works at all and doesn't just re-run an ancient commit, like #32989 (comment)

Just ran a test to specifically test for this by updating the default (master) branch and adding and removing the label and it correctly picked an updated merge commit based on the PR merged into the updated master.

0xB10C commented at 10:41 AM on August 8, 2025: contributor

Have you considered doing something like the following in .github/workflows/ci.yml to avoid having to maintain separate files?

name: CI

on:
  push:
  pull_request:
    types: [opened, reopened, synchronize, labeled]

jobs:
  conditional-job:
    if: >
      github.event_name == 'push' ||
      (
        github.event_name == 'pull_request' &&
        (
          github.event.action == 'opened' ||
          github.event.action == 'reopened' ||
          github.event.action == 'synchronize' ||
          (
            github.event.action == 'labeled' &&
            contains(github.event.label.name, 'PeriodicMergeCICheck')
          )
        )
      )
    runs-on: ubuntu-latest
    steps:
      - run: echo "Running CI job"

m3dwards commented at 4:10 PM on August 8, 2025: contributor

Have you considered doing something like the following in .github/workflows/ci.yml to avoid having to maintain separate files?

Could be a good shout, seems a bit of a verbose check if that has to be included in each job. I'll have a think about it. This is exactly the type of feedback that's good to consider.

DrahtBot added the label Needs rebase on Sep 3, 2025

maflcko commented at 9:43 AM on September 4, 2025: member

concept ack, but I don't have the slightest idea if this is even possible (or how) with GHA

m3dwards commented at 1:26 PM on September 4, 2025: contributor

Currently working on an alternative approach of one job that loops through PRs, might close this PR in favour of the other one based on feedback.

maflcko commented at 7:29 AM on October 16, 2025: member

Any updates here? Just asking, because it is a bit tedious to manually search for all silent merge conflicts. Example ref: #29136 (comment)

fjahr commented at 4:47 PM on November 21, 2025: contributor

Concept ACK

maflcko commented at 4:52 PM on November 21, 2025: member

I think what could work (hacky, untested):

Setup a separate "silent-merge-check" repo with the desired ci config
Push the selected ci runs to it on the desired schedule
Get the result, and on failure, pass it back to the upstream repo

fanquake commented at 11:40 AM on February 13, 2026: member

@m3dwards are you still working on something here? My understanding is that this is still an issue.

m3dwards commented at 4:21 PM on February 16, 2026: contributor

I haven't been working on it but happy to pick this back up

m3dwards force-pushed on Mar 4, 2026

m3dwards commented at 1:17 PM on March 4, 2026: contributor

I wanted to share my current direction and thinking on this and have pushed some example code and can show some test runs on my fork. I think it's a good point to get some feedback if we think this is a good general direction.

In this design there is a new GHA job merged into the main repo that adds check runs to each PR on whether they have a silent merge conflict or not.

The idea is that we have a job that pretty much runs 24/7 triggered every 6 hours or so and set to run for 5.5 hours, it could be set to run on a GHA runner rather than a Cirrus runner. Obviously running on a Cirrus runner (or even multiple runners) would dramatically speed up how many PRs can be checked for silent merge conflicts.

The job uses the Github checks API to add a passing or failing test run to each PR directly. The silent merge check job itself should always pass, it writes pass or fail checks to the PRs.

It will look through all open PRs and discard anything that has a git merge failure or a failing test with the thinking that there is no point checking something that can't be merged anyway. Then it will select PRs that have not had a silent merge check and one by one pull the merge branch of the PR and the full CI job "Previous releases" on that branch.

When all PRs have been checked it will then re-check a PR that was checked the longest time ago.

Things that can be tweaked:

Could run on multiple runners simultaneously rather than just one
How long the job is allowed to run for (currently 5.5 hours, GHA runners have a limit of 6)
How frequently the job is triggered
What type of runner to run the job on
The ordering of how to select the next PR to check.
What things trigger the job to skip a PR (currently failing tests and merge conflicts)

Some back of the envelope maths with the "Previous Releases" job taking 10/15 minutes with a primed cache on a Cirrus runner and 300 open mergeable PRs (probably overestimate) the job would be checking each PR roughly every 4 days.

Please see a run of the job on my fork here: https://github.com/m3dwards/bitcoin/actions/runs/22670080789 A PR that passed the silent merge check: https://github.com/m3dwards/bitcoin/pull/6 A PR that failed the silent merge check: https://github.com/m3dwards/bitcoin/pull/5 (I modified my master branch to have a change that would silently fail against that PR)

This is what a failing check would look like in the PR:

<img width="908" height="261" alt="Screenshot 2026-03-04 at 13 15 41" src="https://github.com/user-attachments/assets/ba862d7d-59e8-40c0-94b6-d6cc1f91b08e" /> @willcl-ark @maflcko interested in your opinion on this design and if it's worth pursuing further and polishing up.

m3dwards force-pushed on Mar 4, 2026

DrahtBot added the label CI failed on Mar 4, 2026

m3dwards force-pushed on Mar 4, 2026

DrahtBot removed the label CI failed on Mar 4, 2026

DrahtBot removed the label Needs rebase on Mar 4, 2026

in .github/workflows/silent-merge-check.yml:38 in ef3ea7636e outdated

  33 | +            echo "::notice title=Runner Selection::Using Cirrus Runners"
  34 | +          else
  35 | +            echo "provider=gha" >> "$GITHUB_OUTPUT"
  36 | +            echo "::notice title=Runner Selection::Using GitHub-hosted runners"
  37 | +          fi
  38 | +  verify-prs:

maflcko commented at 7:41 AM on May 6, 2026:

nit: Forgot to set the timeout to 6h for this task?

maflcko approved

maflcko commented at 7:41 AM on May 6, 2026: member

lgtm, thx!

in .github/workflows/silent-merge-check.yml:9 in ef3ea7636e

   0 | @@ -0,0 +1,72 @@
   1 | +# Copyright (c) 2023-present The Bitcoin Core developers
   2 | +# Distributed under the MIT software license, see the accompanying
   3 | +# file COPYING or https://opensource.org/license/mit.
   4 | +
   5 | +name: Silent Merge Periodic Check
   6 | +
   7 | +on:
   8 | +  schedule:
   9 | +    - cron: "0 0 * * 0"  # Sunday 00:00 UTC

maflcko commented at 7:57 AM on May 6, 2026:

should this not be - cron: "0 */6 * * *"?

m3dwards commented at 8:05 AM on May 6, 2026:

Most likely, definitely the intention was to run every 6 hours for 5.5 hours.

maflcko commented at 9:31 AM on May 6, 2026:

Actually, an odd minute may be better to avoid the peak time around minute 0:

- cron: "37 */6 * * *"

m3dwards commented at 4:11 PM on May 20, 2026:

https://www.upworthy.com/pick-a-random-number-between-100-you-probably-chose-37-and-there-s-a-big-reason-for-that/

in .github/silent-merge-check.py:115 in ef3ea7636e

 110 | +        sys.exit("GITHUB_REPOSITORY is not set")
 111 | +
 112 | +    repo_root = Path(__file__).resolve().parent.parent
 113 | +    os.chdir(repo_root)
 114 | +    start_time = time.monotonic()
 115 | +    max_runtime_seconds = int(timedelta(hours=5, minutes=30).total_seconds())

maflcko commented at 8:12 AM on May 6, 2026:

nit: I think this could even be 3 hours (with a timeout and cron of 6 hours), to get a pull re-run every week, which should be enough?

maflcko commented at 2:49 PM on May 20, 2026: member

are you still working on this?

m3dwards commented at 2:58 PM on May 20, 2026: contributor

are you still working on this?

yep, was waiting until the new CI provider was picked and merged but I guess there isn't any dependency there.

maflcko commented at 3:05 PM on May 20, 2026: member

Yeah, this runs-on: ubuntu-latest (GHA), so there shouldn't be any blocker

m3dwards commented at 4:21 PM on May 20, 2026: contributor

More concerned with the cache actions changing but can deal with that if and when it happens.

m3dwards force-pushed on May 20, 2026

DrahtBot added the label CI failed on May 20, 2026

DrahtBot commented at 10:09 PM on May 20, 2026: contributor

🚧 At least one of the CI tasks failed. <sub>Task lint: https://github.com/bitcoin/bitcoin/actions/runs/26192340097/job/77063624905</sub> <sub>LLM reason (✨ experimental): CI failed due to a Python lint (ruff) error: datetime.timezone was imported but unused in .github/silent-merge-check.py.</sub>

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

DrahtBot removed the label CI failed on May 20, 2026

in .github/workflows/silent-merge-check.yml:44 in 0165078fbe

  39 | +  verify-prs:
  40 | +    name: 'Verify open PRs for silent merge conflicts'
  41 | +    needs: [runners]
  42 | +    runs-on: ubuntu-latest
  43 | +    permissions:
  44 | +      checks: write

willcl-ark commented at 9:57 AM on May 21, 2026:

I think running the entire job with this permission would let a PR do some annoying things on GH as they have an authenticated write token to play with in run_all.sh?

Probably should just have this token in a seperate step or job where it's needed

m3dwards commented at 3:17 PM on May 21, 2026:

I'll have a think about this as you are right, the PR authors code will have access to the checks write token.

in .github/silent-merge-check.py:15 in 0165078fbe

  10 | +import sys
  11 | +import time
  12 | +from datetime import datetime, timedelta
  13 | +from pathlib import Path
  14 | +
  15 | +MAX_RUNTIME = timedelta()

willcl-ark commented at 9:58 AM on May 21, 2026:

Is this intentional? surely we'd want here

timedelta(hours=5, minutes=30)

or read from an env var (from the main workflow)

m3dwards commented at 3:15 PM on May 21, 2026:

Yup, refactor / copy paste error.

in .github/silent-merge-test.py:98 in 0165078fbe outdated

  93 | +
  94 | +    max_runtime_seconds = int(MAX_RUNTIME.total_seconds())
  95 | +    start_time = time.monotonic()
  96 | +
  97 | +    print("Fetching open PRs...")
  98 | +    open_prs = get_open_prs(repo)

willcl-ark commented at 10:05 AM on May 21, 2026:

Even though these have a mergable field, I think it's bunk :( :

core/worktrees/pr-33145 on  pr-33145 [$] via △ v4.1.2 via 🐍 v3.13.12 via ❄️  impure (nix-shell-env) took 57s
❯ gh api 'repos/bitcoin/bitcoin/pulls/35342' --jq '{number, mergeable, mergeable_state, keys: (keys | map(select(test("merge"))))}'
{
  "keys": [
    "auto_merge",
    "merge_commit_sha",
    "mergeable",
    "mergeable_state",
    "merged",
    "merged_at",
    "merged_by"
  ],
  "mergeable": true,
  "mergeable_state": "blocked",
  "number": 35342
}

core/worktrees/pr-33145 on  pr-33145 [$] via △ v4.1.2 via 🐍 v3.13.12 via ❄️  impure (nix-shell-env)
❯ gh api 'repos/bitcoin/bitcoin/pulls?state=open&per_page=1' --jq '.[0] | {number, mergeable, mergeable_state, keys: (keys | map(select(test("merge"))))}'
{
  "keys": [
    "auto_merge",
    "merge_commit_sha",
    "merged_at"
  ],
  "mergeable": null,
  "mergeable_state": null,
  "number": 35342
}

perhaps we need to iterate each pr individually

maflcko commented at 10:18 AM on May 21, 2026:

Jup, you'll need to call something like get_pull_mergeable in https://github.com/maflcko/DrahtBot/blob/6084f507bc64af4bfd5b3c33ebf725a57c66320a/rerun_ci/src/main.rs#L86

m3dwards commented at 3:05 PM on May 21, 2026:

Thanks for the heads up, that's a good catch. Will just check each PR individually, won't add much extra time.

willcl-ark commented at 10:07 AM on May 21, 2026: member

I feel like I've forgotten context here (sorry), but why can this job not just do something like:

check out bitcoin/bitcoin:master
fetch PR head
attempt the merge locally
run a fixed build command, something like:

rm -Rf build
cmake -B build --parallel
cmake --build build

# optionally
ctest --test-dir build

publish the result?

this could probably be done on a bare runner host too, negating need for docker et al. I think it would probably be faster as well...

maflcko commented at 10:23 AM on May 21, 2026: member

I feel like I've forgotten context here (sorry), but why can this job not just do something like:
* check out bitcoin/bitcoin:master

* fetch PR head

* attempt the merge locally

* run a fixed build command, something like:
rm -Rf build
cmake -B build --parallel
cmake --build build

# optionally
ctest --test-dir build
* publish the result?
this could probably be done on a bare runner host too, negating need for docker et al. I think it would probably be faster as well...

Yeah, if that is less code, that works, too. For reference, except for rm -rf, you can just call python3 .github/ci-test-each-commit-exec.py to do all of the cmake stuff.

m3dwards commented at 2:58 PM on May 21, 2026: contributor

I feel like I've forgotten context here (sorry), but why can this job not just do something like:

check out bitcoin/bitcoin:master

fetch PR head

attempt the merge locally

run a fixed build command, something like:
rm -Rf build
cmake -B build --parallel
cmake --build build

# optionally
ctest --test-dir build
publish the result?

this could probably be done on a bare runner host too, negating need for docker et al. I think it would probably be faster as well...

I'm open to changing it to this but this is more code than it currently is which is simply calling ./ci/test_run_all.sh. I also quite like the idea that it's currently running the functional tests because a failure could be logical rather than just a compilation error.

maflcko commented at 3:33 PM on May 21, 2026: member

functional tests

.github/ci-test-each-commit-exec.py does run them, see the prior comment.

m3dwards commented at 3:39 PM on May 21, 2026: contributor

functional tests

.github/ci-test-each-commit-exec.py does run them, see the prior comment.

I'm not really understanding the advantages of this over ./ci/test_run_all.sh. We don't want to check each commit with this job, only the merge commit right?

maflcko commented at 3:47 PM on May 21, 2026: member

The benefit would be that it doesn't require docker, so I think the three uses: ./.github/actions can be dropped. .github/ci-test-each-commit-exec.py only tests a single commit, the name can be changed, if needed.

But either way is fine. It should be trivial to adjust later, if needed.

maflcko commented at 5:22 AM on May 27, 2026: member

More concerned with the cache actions changing but can deal with that if and when it happens.

This was fixed in https://github.com/bitcoin/bitcoin/pull/35348

m3dwards force-pushed on May 28, 2026

m3dwards commented at 6:54 PM on May 28, 2026: contributor

Pushed latest changes to show direction. Running a bunch of tests on my fork. Will switch this PR to ready to review when the local testing is complete. It's just a bit time consuming as you have to do multiple full CI runs on a bunch of fake PRs.

m3dwards force-pushed on Jun 17, 2026

in .github/silent-merge-test.py:192 in 532b083663

 187 | +        if merge_result.returncode != 0:
 188 | +            run(["git", "merge", "--abort"], check=False)
 189 | +            print(f"PR #{pr['number']} has merge conflicts against current master, skipping.")
 190 | +            continue
 191 | +
 192 | +        ci_result = run([".github/ci-test-each-commit-exec.py"], check=False,

maflcko commented at 7:45 AM on June 18, 2026:

Just checked my rerun_ci.sh script, and realized I was also running --task 'lint'. lint should be fast, so it could also be run here?

m3dwards commented at 3:49 PM on June 18, 2026:

Do you have a link to that script? To run lint as it is in ci.yml would require pulling docker in right and all the docker caches etc etc.

maflcko commented at 4:00 PM on June 18, 2026:

Well the script is https://github.com/maflcko/DrahtBot/blob/2038d4d541b89bf2601614b5656d7d30d73e17be/rerun_ci/src/main.rs#L15

I think you can just call ci/lint.py, because docker is already installed and docker has a built-in cache that works out of the box without having to pull anything?

m3dwards commented at 9:19 PM on June 18, 2026:

I've run experiments today adding lint and it dramatically slowed down how long per PR it takes to process. Right now it's shockingly fast at about 5-7 minutes per PR with a warm ccache. Adding lint took this to 35 minutes per PR. I did expect the first one to be slower but expected the second PR checked to be faster as the image would already have been built?

I think I'm of the mind to just leave lint out as I wouldn't expect a silent merge issue to be often caught by the lint checker and it just slows things down and adds more complexity.

That said, if you are keen to have it, I can keep working to try and figure out why it's so slow and if we can do better caching etc.

Just for reference here is a 2 PR run that was done with lint (1 hour 16 minutes): https://github.com/m3dwards/bitcoin/actions/runs/27784000378

And one done without lint (23 minutes): https://github.com/m3dwards/bitcoin/actions/runs/27787578648

maflcko commented at 10:12 AM on June 19, 2026:

Ok, fair to leave it for later.

Also, looking at the logs, it could be a bit tedious to find the failure, as it is hidden in the middle of a long log? Also, there are no timestamps on any of the lines at all?

Probably fine, but maybe the CDash instance can be used to submit each result, so that each result is stand-alone?

m3dwards commented at 6:09 PM on June 22, 2026:

I can add timestamps.

The logs are only printed when a PR fails but if multiple PRs fail which I would think would be unlikely then yes you would have to differentiate between the PRs and search for your PR number in the log output. I don't think this is very likely though as silent merge issues should be rare-ish?

Is that good enough for this PR as it's probably mostly the same as existing CI.

I am happy to look at adding results to CDash unless you think with only printing results when a PR fails is sufficient, or that it could be added as a followup.

maflcko commented at 7:46 AM on June 18, 2026: member

This is still in Draft. What is the status here?

m3dwards commented at 2:55 PM on June 18, 2026: contributor

This is still in Draft. What is the status here?

I think pretty much ready for review. I'm still running some tests in the background and I can add the lint job you mentioned.

m3dwards marked this as ready for review on Jun 18, 2026

maflcko commented at 10:17 AM on June 19, 2026: member

lgtm. Seems fine to put the code in this repo and call it inside this CI. An alternative would be to have the code under DrahtBot and run it there.

ci: add silent merge check

Adds a periodic CI workflow that runs every 6 hours to detect silent
merge failures across all open PRs. For each PR, it fetches the
pre-built merge commit (pull/<n>/merge), runs a build and the tests,
and posts the result as a check run against the PR's head SHA. PRs
that have never been checked are prioritised (oldest first),followed
by those with the oldest existing check result.

ef35e6c84c

m3dwards force-pushed on Jun 26, 2026

m3dwards commented at 3:30 PM on July 1, 2026: contributor

Added timestamps to the log messages if they do not already have them (such as some of the functional test messages). Timestamps were also only added to the failure log as all other log messages are timestamped properly by GHA.

The logs are now collapsed into their own section meaning it should be much easier to find the log messages when something is wrong. For this reason and because the functional tests aren't integrated with ctest at the moment I have left out CDash integration either for a follow up or for never. @willcl-ark prototyped CDash here https://github.com/bitcoin/bitcoin/compare/master...willcl-ark:bitcoin:pr-33145 but because of the above I have chosen not to add this to this PR.

Example of the logs being collapsed: