CI: silent merge check by m3dwards · Pull Request #33145 · bitcoin/bitcoin

m3dwards · 2025-08-06T14:53:08Z

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 · 2025-08-06T14:53:13Z

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  into the comment that the bot should ignore.

Conflicts

No conflicts as of last run.

maflcko · 2025-08-06T15:09:17Z

+  previous-releases:
+    name: 'Previous releases, depends DEBUG'
+    if: contains(github.event.label.name, 'PeriodicMergeCICheck')


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?

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?

I don't know. Not sure if we want to see pull requests spammed with "added and removed" label events (testing-cirrus-runners#19 (comment)).

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

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.

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.

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 · 2025-08-06T19:54:22Z

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 · 2025-08-07T08:32:04Z

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

m3dwards · 2025-08-07T14:26:27Z

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

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 · 2025-08-08T16:10:54Z

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.

maflcko · 2025-09-04T09:43:59Z

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

m3dwards · 2025-09-04T13:26:33Z

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 · 2025-10-16T07:29:45Z

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

fjahr · 2025-11-21T16:47:32Z

Concept ACK

maflcko · 2025-11-21T16:52:52Z

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 · 2026-02-13T11:40:11Z

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

m3dwards · 2026-02-16T16:21:04Z

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

m3dwards · 2026-03-04T13:17:43Z

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: m3dwards#6
A PR that failed the silent merge check: m3dwards#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:

@willcl-ark @maflcko interested in your opinion on this design and if it's worth pursuing further and polishing up.

maflcko

lgtm, thx!

maflcko · 2026-05-06T07:57:42Z

+
+on:
+  schedule:
+    - cron: "0 0 * * 0"  # Sunday 00:00 UTC


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

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

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

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

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

maflcko · 2026-05-06T08:12:12Z

+    repo_root = Path(__file__).resolve().parent.parent
+    os.chdir(repo_root)
+    start_time = time.monotonic()
+    max_runtime_seconds = int(timedelta(hours=5, minutes=30).total_seconds())


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 · 2026-05-20T14:49:20Z

are you still working on this?

m3dwards · 2026-05-20T14:58:43Z

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 · 2026-05-20T15:05:37Z

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

m3dwards · 2026-05-20T16:21:05Z

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

DrahtBot · 2026-05-20T22:09:48Z

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

Hints

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.

willcl-ark

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...

willcl-ark · 2026-05-21T09:57:47Z

+    needs: [runners]
+    runs-on: ubuntu-latest
+    permissions:
+      checks: write


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

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

willcl-ark · 2026-05-21T09:58:38Z

+from datetime import datetime, timedelta
+from pathlib import Path
+
+MAX_RUNTIME = timedelta()


Is this intentional? surely we'd want here

timedelta(hours=5, minutes=30)

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

Yup, refactor / copy paste error.

maflcko · 2026-05-21T10:23:23Z

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 · 2026-05-21T14:58:24Z

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 · 2026-05-21T15:33:27Z

functional tests

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

m3dwards · 2026-05-21T15:39:01Z

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 · 2026-05-21T15:47:54Z

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 · 2026-05-27T05:22:54Z

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

This was fixed in #35348

m3dwards · 2026-05-28T18:54:51Z

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.

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.

maflcko

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

maflcko · 2026-06-18T07:45:41Z

+            print(f"PR #{pr['number']} has merge conflicts against current master, skipping.")
+            continue
+
+        ci_result = run([".github/ci-test-each-commit-exec.py"], check=False,


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?

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.

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?

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

m3dwards · 2026-06-18T14:55:45Z

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.

DrahtBot added the Tests label Aug 6, 2025

maflcko reviewed Aug 6, 2025

View reviewed changes

DrahtBot mentioned this pull request Aug 7, 2025

ci: Migrate CI to hosted Cirrus Runners #32989

Merged

0xB10C reviewed Aug 8, 2025

View reviewed changes

maflcko mentioned this pull request Sep 1, 2025

Revisiting us self-hosting parts of our CI #31965

Closed

DrahtBot added the Needs rebase label Sep 3, 2025

maflcko mentioned this pull request Nov 21, 2025

test: assumeutxo: add missing tests in wallet_assumeutxo.py #30455

Merged

maflcko mentioned this pull request Jan 23, 2026

refactor: Avoid copies by using const references or by move-construction #31650

Merged

m3dwards force-pushed the 250806-ci-silent-merge branch from 1b57472 to 358e4a6 Compare March 4, 2026 12:55

m3dwards force-pushed the 250806-ci-silent-merge branch from 358e4a6 to 996bd18 Compare March 4, 2026 13:23

DrahtBot added the CI failed label Mar 4, 2026

m3dwards force-pushed the 250806-ci-silent-merge branch from 996bd18 to ef3ea76 Compare March 4, 2026 13:27

DrahtBot removed CI failed Needs rebase labels Mar 4, 2026

maflcko approved these changes May 6, 2026

View reviewed changes

Comment thread .github/workflows/silent-merge-check.yml

maflcko reviewed May 6, 2026

View reviewed changes

m3dwards force-pushed the 250806-ci-silent-merge branch 2 times, most recently from d1ac08f to 0165078 Compare May 20, 2026 22:09

DrahtBot added the CI failed label May 20, 2026

DrahtBot removed the CI failed label May 20, 2026

willcl-ark reviewed May 21, 2026

View reviewed changes

m3dwards force-pushed the 250806-ci-silent-merge branch from 0165078 to 393dc8a Compare May 28, 2026 18:52

m3dwards force-pushed the 250806-ci-silent-merge branch from 393dc8a to 532b083 Compare June 17, 2026 21:31

maflcko reviewed Jun 18, 2026

View reviewed changes

m3dwards marked this pull request as ready for review June 18, 2026 14:55

Conversation

m3dwards commented Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sammie05 commented Aug 6, 2025

Uh oh!

maflcko commented Aug 7, 2025

Uh oh!

m3dwards commented Aug 7, 2025

Uh oh!

0xB10C left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

m3dwards commented Aug 8, 2025

Uh oh!

maflcko commented Sep 4, 2025

Uh oh!

m3dwards commented Sep 4, 2025

Uh oh!

maflcko commented Oct 16, 2025

Uh oh!

fjahr commented Nov 21, 2025

Uh oh!

maflcko commented Nov 21, 2025

Uh oh!

fanquake commented Feb 13, 2026

Uh oh!

m3dwards commented Feb 16, 2026

Uh oh!

m3dwards commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko commented May 20, 2026

Uh oh!

m3dwards commented May 20, 2026

Uh oh!

maflcko commented May 20, 2026

Uh oh!

m3dwards commented May 20, 2026

Uh oh!

DrahtBot commented May 20, 2026

Uh oh!

willcl-ark left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

m3dwards commented Aug 6, 2025 •

edited

Loading

DrahtBot commented Aug 6, 2025 •

edited

Loading

0xB10C left a comment •

edited

Loading

m3dwards commented Mar 4, 2026 •

edited

Loading