Problems with forked repositories

[planetf1]

I setup automerge yesterday on our egeria repository - https://github.com/odpi/egeria

When I submit a PR without the automerge label the automerge action completes successfully, reporting label not found - correct :-)

However this morning I tagged two PRs with the automerge label, and the task failed with:

Run a4b03ef
2
env:
3
GITHUB_TOKEN: ***
4
MERGE_LABELS: automerge,!no-not-merge
5
MERGE_RETRIES: 300
6
MERGE_RETRY_SLEEP: 60000
7
/usr/bin/docker run --name dfb7513390c702f4ef1b1cb9356ddd06b02_ddf008 --label 488dfb --workdir /github/workspace --rm -e GITHUB_TOKEN -e MERGE_LABELS -e MERGE_RETRIES -e MERGE_RETRY_SLEEP -e HOME -e GITHUB_REF -e GITHUB_SHA -e GITHUB_REPOSITORY -e GITHUB_RUN_ID -e GITHUB_RUN_NUMBER -e GITHUB_ACTOR -e GITHUB_WORKFLOW -e GITHUB_HEAD_REF -e GITHUB_BASE_REF -e GITHUB_EVENT_NAME -e GITHUB_WORKSPACE -e GITHUB_ACTION -e GITHUB_EVENT_PATH -e RUNNER_OS -e RUNNER_TOOL_CACHE -e RUNNER_TEMP -e RUNNER_WORKSPACE -e ACTIONS_RUNTIME_URL -e ACTIONS_RUNTIME_TOKEN -e ACTIONS_CACHE_URL -e GITHUB_ACTIONS=true -v "/var/run/docker.sock":"/var/run/docker.sock" -v "/home/runner/work/_temp/_github_home":"/github/home" -v "/home/runner/work/_temp/_github_workflow":"/github/workflow" -v "/home/runner/work/egeria/egeria":"/github/workspace" 488dfb:7513390c702f4ef1b1cb9356ddd06b02
8
INFO Event name: pull_request
9
INFO Updating PR #2463 Improve reliability of CTS notebook esp in containers
10
INFO No update necessary, mergeable_state: clean
11
INFO Merging PR #2463 Improve reliability of CTS notebook esp in containers
12
INFO PR is probably ready: mergeable_state: clean
13
INFO Failed to merge PR: Not Found
14
INFO Retrying after 60000 ms ... (1/300)
15
ERROR Not Found
16
##[error]Docker run failed with exit code 1

This is from: odpi/egeria#2463

The PR was not up to date with master - we do enforce this and normally do manually - I was thinking the action does this?

[pascalgn]

GH doesn't let me clone my own repository, so I cannot quickly test this right now.

Could you possibly adapt your actions workflow file to enable tracing and then try again?

      - name: automerge
        uses: pascalgn/automerge-action@...
        with:
          args: "--trace"

[planetf1]

Thanks - will do so & update here

[planetf1]

Here's a log of a failing run -> https://gist.github.com/3ae5899ae6886ddf64f97abaaeae0126

[pascalgn]

Yeah, that was a bug because I used pullRequest.head... where instead pullRequest.base... should have been used!

This should be fixed in the latest version, you can try it like

    steps:
      - name: automerge
        uses: "pascalgn/automerge-action@7854d3bd607dccdaf0b2c134b699a812c8960213"

[planetf1]

Sincere thanks for addressing that - out of interest I wonder if it would be reasonable to use use the latest ? presumably opascalgn/automerge-action? Would you recommend that? (new to github actions, and I guess there's no confirmed version yet?)

[pascalgn]

I would definitely recommend against using latest. We did that once accidentally for another action (provided by GH) and the API of that action changed, without us noticing, breaking our build.

If you want, you can point to the releases (which is also what GH is recommending, e.g. when looking at the action on the marketplace). However, I would recommend to always point to the full SHA, so that you can be reasonably safe that the version doesn't change (accidentally or on purpose)

[planetf1]

Thanks for the clarification.

I tried a new automerge & hit a problem:

2020-01-27T09:38:11.7325143Z INFO  Updating PR #2496 Add notes on issue management & update autoclosing
2020-01-27T09:38:11.7325338Z INFO  No update necessary, mergeable_state: clean
2020-01-27T09:38:11.7325613Z INFO  Merging PR #2496 Add notes on issue management & update autoclosing
2020-01-27T09:38:11.7325809Z INFO  PR is probably ready: mergeable_state: clean
2020-01-27T09:38:11.7325970Z INFO  Failed to merge PR: Resource not accessible by integration

I posted the full trace to https://gist.github.com/04e7f4103cbd4346cb207504165ab107

I'm not sure if this is a followon or not.

I didn't see any evidence of the automerge rebasing the PR - is it intended to do this?

[pascalgn]

I'm afraid that's a general problem with GH actions and forks. From the docs:

When you create a pull request from a forked repository to the base repository, GitHub sends the pull_request event to the base repository and no pull request events occur on the forked repository.
[...]
The permissions for the GITHUB_TOKEN in forked repositories is read-only.

As I understand it, this means:

1. The action is not triggered in the forked repository, so it cannot rebase the branch there (as it never runs)
2. The action is triggered in the base repository, but it cannot rebase the branch, because the branch belongs to the fork and the action only has read-only access to the fork

The docs also state

Workflows don't run on forked repositories by default. You must enable GitHub Actions in the Actions tab of the forked repository.

So if you enable actions for the fork, the action could be triggered by the push event (this should get sent to the fork) and rebase should happen.

Merging should work correctly, as the pull_request event is sent to the base repository (which has the correct permissions to do the actual merging)

[planetf1]

Ah! Thanks for the explanation.

Yes our process is all centered around forks (fairly common open source model).

In our case dependabot is the one bot that actually creates branches in the main repo -- and it is able therefore to auto rebase & merge

Although I could enable updates on my repo, it won't scale as a general solution. I'll probably have to think again how we want to handle this.

Thanks for the time though.

[pascalgn]

Yes our process is all centered around forks (fairly common open source model).

Yes, that's why I think it makes sense to support this case correctly.

I'm not entirely sure if it's really required or even desired to update branches in the forks. When I fork a repository, I would not expect some automation to update my branch. That could be one of the reasons why GH requires actions to be explicitly enabled.

If you want the PR to be up-to-date before merging (which makes sense IMO), maybe it could be enough to indicate that it's behind. Currently this is made visible via the failed automerge check, but if that's not enough, one could think about adding a comment like "Cannot merge, as branch is behind". I think the permissions should be enough for this

[planetf1]

Though just indicating with a message - whilst it helps the user understand why it isn't merged, I don't think it completely addresses the problem that occurs in forked repos where github protections require branches to be up to date before merging.(understand why of course)

If we have a backlog of 11 PRs to merge (like this morning) I have to chose my first one, click 'update branch' on github (of course also in cli, or could be done by owner going a pull or rebase & push). wait for checks to complete (30 mins - build/tests). Then I can merge.

In the meantime if anyone else has done the same and remerged I have to start again, since the 'update' will need to be done again, then another 30 mins of checks.

So with the automerge behaviour described we might get the first one through but no more. So it could eliminate that 30 min period of checking before merging, but what it can't do is mark a whole set of PRs and get them effectively queued up for merging.

[GMNGeoffrey]

So to be clear about the current support for forks, if I set this up to just do auto-merging and do not set "Require branches to be up to date before merging", will it work for merging a PR coming from a fork?

Something like this:

automerge.yml

name: automerge	
on:	
  pull_request:	
    types:	
      - labeled	
      - unlabeled	
      - synchronize	
      - opened	
      - edited	
      - ready_for_review	
      - reopened	
      - unlocked	
  pull_request_review:	
    types:	
      - submitted	
  check_suite:	
    types:	
      - completed	
  status: {}	
jobs:	
  automerge:	
    runs-on: ubuntu-18.04
    steps:	
      - name: automerge	
        uses: "pascalgn/automerge-action@135f0bdb927d9807b5446f7ca9ecc2c51de03c4a"	
        env:	
          GITHUB_TOKEN: "${{ secrets.GITHUB_WRITE_ACCESS_TOKEN }}"  # personal access token
          MERGE_LABELS: "automerge-squash,!wip"	
          MERGE_METHOD: "squash"	
          MERGE_FORKS: true
          MERGE_COMMIT_MESSAGE: "pull-request-title-and-description"

[pascalgn]

Yes, that should work, see some comment above

Merging should work correctly, as the pull_request event is sent to the base repository (which has the correct permissions to do the actual merging)

[yelizariev]

I don't think that it will work -- secrets are not available in forks, so you cannot use custom token

[michaelbeaumont]

It seems like this ultimately stems from the mismatch between "this code is safe to run workflows on with a secret token because it's already in the main repo" vs "it's safe to run because I added a label that implies it's allowed to be in the repo (but I need that secret token to make it so)". It appears to be a failure of Github Actions but the problem isn't easy to solve using the generic "events trigger some arbitrary code" workflow interface.

I would suppose this trust implication (label from maintainers -> safe to run workflows with secrets and/or merge) needs to be special cased in Github (or some interface for expressing the idea "if such and such is true, this code is safe" needs to be introduced). Alternatively, treat a PR action not as either "running in a fork" or "running in base" but rather "running with this base and this fork, take into account what the base says about PRs to itself"

But would creating a workflow that just runs periodically to loop over open PRs and their labels solve this problem until then?

[planetf1]

I think a periodic/scheduled workflow would be a suitable alternative. Certainly for me the main value I see in automerge is avoiding the need to rebuild, wait for a 30min cycle, go to merge, find another one has got in and start again. I'm not really so bothered if it takes 30 mins or a day, it's more the automation and reducing manual effort for PRs we think are good to go.

Indeed allowing to run only in less active hours could be seen as a feature (avoiding interaction with any more urgent fixes being manually driven)!

[pascalgn]

I would suppose this trust implication (label from maintainers -> safe to run workflows with secrets and/or merge) needs to be special cased in Github

Yes, it's nice that this action is useful for people, but when you really think about it, it should actually be a built-in feature, as it already is in GitLab or Azure DevOps (if I'm not mistaken). Now this action supports much more obscure workflows then initially thought necessary, but I still hope that GH will at least implement the core functionality (merge when all checks pass) some day.

I think a periodic/scheduled workflow would be a suitable alternative.

Yes, I think that's a good idea. I even thought about switching our workflow to "scheduled," as the action will otherwise be triggered unnecessarily many times. Checking every 15 or 30 minutes or so, based on build speed, should be fine. I haven't tested it much with the "schedule" event trigger, but it should already work

[michaelbeaumont]

Well, well! Certainly relevant: github/roadmap#107

[michaelbeaumont]

Hmm it seems check_suite/check_run events for forked repositories are also useless, since they don't contain a reference to the pull request.