kindly remind on the github token restriction will limit the usability of this action

[hanxiao]

First, Great work!

Please be careful when you continue developing this action. For a pull request coming from a fork (which is where many opensource projects' contribution coming from and thats why people need a CLA bot), Github Action grants only a read access on GITHUB_TOKEN; and PAT secrets are not available on forks. These two restrictions will greatly limit the usage of this action. We already encountered this problem: jina-ai/serve#417

Please refer to this issue for the same problem: actions/labeler#12 A lot of users adapt to this Github action and later have to remove it from their CICD pipeline. You can scroll it down and see a lot of PRs related to that issue are just for removing it from the pipeline.

A good way to test it is let some user fork your repo and submit a pull request from their fork.

[ibakshay]

Hi @hanxiao, First, I am really sorry for responding so late.
Thank you very much for taking the time to report this design issue with the GitHub Actions platform. You are absolutely right. CLA bot will be mostly required only for the pull requests coming from the forks.

We are aware of this issue and I myself have raised this issue multiple times in several discussions. In one of the discussion, the GitHub team told that they are working on a fix and they will come up with a solution to address this problem with PR coming from forks.
I have already mentioned in the readme file that this action won't work for pull requests coming from forks (see below screenshot)

I really hope that the GitHub team somehow fixes this issue so that we can continue with the development of this action. Please feel free to raise a pull request If you can find a workaround solution to make this action work for PRs coming from forks.

[Naatan]

Couldn't you just write at merge time? Granted accepted CLA's that never had their PR merged would have to re-accept, but that's better than this github action being entirely useless for its intended use-case.

[ghuntley]

Nice idea @Naatan!

[ibakshay]

Hi @Naatan, Could you please elaborate on your proposal ?.
Do you mean to write the CLA signatures when the pull request is merged?

[hanxiao]

btw, reducing the frequency of comment posting from the bot is probably something @ibakshay may consider?

• right now if a PR comes from a fork, and the contributor does not sign CLA, the bot can detect that but fails to post any comment on the thread of this PR, as the bot has read-only access to everything.
• now the maintainer could join the thread in and reply recheckcla to grant the bot write access. Comments are now posted on the thread. Everything works fine.
• next time when the same contributor (signed) makes a PR again, the bot still fails. However, notice that the reason of the failure is purely because of the lack of access of posting "everyone has signed CLA", which I believe is unnecessary (the green check mark in the CICD status will indicate the success of this action anyway).
• the same happens, if you hit rerun the workflow on Github action, the bot fails despite everyone has signed the CLA, this is again due to the unnecessary comment posting.

Hence, removing unnecessary comment posting especially when CLA pass will improve the usability of this bot.

Example:

https://github.com/jina-ai/jina/pull/470/checks?check_run_id=768552510

Let me clarify the user journey here.

• If the cla-bot fails, then it must fail due to the lack of CLA signing, nothing else. Red cross on the Github action status.
• Maintainer notice the red across, join in the thread and type recheckcla.
• If the cla-bot success, nothing need to be posted, green checkmark tells everything. Maintainer does not need to take any action.

[Naatan]

Hi @Naatan, Could you please elaborate on your proposal ?.
Do you mean to write the CLA signatures when the pull request is merged?

Exactly. Obviously not ideal, but with the limitations that exist I think that's the best you can do.

[ibakshay]

Hi @hanxiao, Thank you very much for the detailed write up and proposal..

now the maintainer could join the thread in and reply recheckcla to grant the bot write access. Comments are now posted on the thread. Everything works fine. next time when the same contributor (signed) makes a PR again, the bot still fails. However, notice that the reason of the failure is purely because of the lack of access of posting "everyone has signed CLA", which I believe is unnecessary (the green check mark in the CICD status will indicate the success of this action anyway).

Awesome insights 💯. I never thought about this. You are absolutely right. Unnecessary comment posting from bot is not required at all if all the committers have already signed the CLA. I will roll out a release soon to remove the unnecessary comment from this bot. Also, Feel free to raise a pull request for your proposal :).

[ibakshay]

Exactly. Obviously not ideal, but with the limitations that exist I think that's the best you can do.

I believe pull_request.closed also belongs to pull_request event. Unfortunately, there is no write access for any type of pull_request events coming from the forks. For getting more insights, you can read this thread. I hope GitHub finds a solution soon..

[domenkozar]

The plan sounds really good! I'd note that the first time the check fails, it could print what the user should do in the logs. Currently it only says it failed to post a comment (it should probably check if it has access first and fail with a better error).

[ibakshay]

Hello @ghuntley @hanxiao @domenkozar @Naatan :)

Today, GitHub Actions rolled out a series of updates, and Pull Requests from forks are supported 😄 . We have to make use of this new event pull_request_target instead of pull_request event. So, this action will be fully usable from now on.. I will roll out a new release this week which will reflect these changes :).

[ibakshay]

Hello again @ghuntley @hanxiao @domenkozar @Naatan :)

Today I drafted a new release v2.0.0-alpha and in this new release, Pull request coming from forked repository is supported with many other features, improvements and bug fixes :) .

[domenkozar]

Thank you! I will upgrade today/tomorrow.

[domenkozar]

@ibakshay how come PAT is now required?

[ibakshay]

We, unfortunately, need a PAT to access this API to re-run the previously failed CLA PR status checks when the contributor has signed the CLA. I have also asked about this thing, in the GitHub community forum

[ibakshay]

Before, we need to manually re-run the previously failed CLA PR status check. So, PAT was not required.

[domenkozar]

Wouldn't it be possible for the action now to push a commit to PR that would trigger the build?

EDIT: nevermind, action wouldn't run.

[ibakshay]

Hi @domenkozar, I am sorry for the late response. I was on vacation 🌴.

Yes, you are right. Push event from the GitHub Acton wouldn't trigger the action workflows.
I even created a ticket in the GitHub Community discussion regarding this issue and come to know that, it is not possible :/ .
You can view my ticket in the GitHub Community forum here