A comment is added to third party PRs (with a different origin repo) instructing a maintainer to add the comment /run-acceptance-tests. This does run a workflow via command dispatch in the background but has two issues:
- It doesn't check out the correct code (it runs on the default branch).
- It doesn't report back success to the PR to unblock merging.
(2) has never worked as this is a limitation of the command dispatch via repository_dispatch triggers. Repository dispatch always runs against the default branch and has no option to run elsewhere. Therefore the job statuses will never report back to the correct PR or commit automatically.
(1) used to work, but it appears got broken when we moved away from each job being defined in a single file and moved to re-usable workflows (around June 2024 #942). This used to work by having a special case when checking out the code to specify the commit hash via an env variable which was populated when in repository_dispatch mode from the triggering payload.
Possible solutions
1. Pass the SHA explicitly into all workflows
The most direct fix for (1) is to avoid ever using the github context for the current SHA. Most actions we use have the option for the SHA to be specified manually (but fallback to the default github context).
We would then have a switch in the run-acceptance-tests workflow to check if we're in repository_dispatch mode and pull the SHA from the payload as we used to do.
Interestingly, this will also now fix (2) because we've since moved to the explicit sentinel status action on which we can specify the SHA, and this is the only check required to merge the PR.
Drawbacks
This is tricky to prove that we've added overrides to all the correct places that require the current SHA. It's also possible that some actions are hard coded to use the SHA from the context and can't be overridden (including our versioning action, though we can fix this). Furthermore, this is very prone to breaking again in the future as the workflows evolve and people forget to specify the SHA from the input explicitly on some steps which could result in unexpected failures.
2. Change command dispatch to use workflow_dispatch
Workflow dispatch is the same mechanism used when running a job manually from the actions tab. This has the added features of having a ref parameter which allows the job to be run for a specific branch or commit. This would naturally operate on the correct SHA and report back all steps progress to the PR, avoiding manually overriding the SHA in every step.
Challenges
Command dispatch has since also be leveraged for the /release command. It appears it's not possible to specify the type of dispatch per command so we'd need to also rework the release workflow which is heavily reliant on the repository dispatch payload which is not available when using workflow dispatch.
A side question: do we still need the /release command? Is this a workflow we still encourage or do we all use the label based release process now?
It's possible we might be able to have two separate command dispatch handlers - one for each target command, but I'm not yet sure on if they would error if an unrecognised command was attempted.
Furthermore, this whole command dispatch process isn't currently implemented for 3rd party providers as it requires personal access tokens to be configured and therefore we need a process that works for them also.
3. Remove command dispatch and update instructions
It would be trivial to update the run-acceptance-tests workflow to allow workflow_dispatch. This allows it to be triggered in the GitHub actions UI. We could then update the instructions comment to link the maintainer to where to click the run button rather than implementing a whole complicated command dispatch mechanism.
When we originally wrote the command dispatch mechanism there was not a clickable UI for running actions directly which therefore mandated this complex setup, but perhaps it's a worthwhile simplification given the issues raised above.
4. Remove all custom process for third parties
At the time we implemented the original command dispatch setup, GitHub only had the option to approve first-time contributors. Once approved once, a third party could then execute workflows on subsequent PRs without approval.
This is no longer the case - see the docs on approving workflows for forks). There is now the option to always require approval before running workflows from third parties. This can be enforced at the organisation level.
Proposed steps:
- Enforce approval for every run from a third party at an organization level.
- Delete the conditionals based on
github.event.pull_request.head.repo.full_name
- Delete the auto-commenting on PRs from third parties.
- Delete the command dispatch.
A comment is added to third party PRs (with a different origin repo) instructing a maintainer to add the comment
/run-acceptance-tests. This does run a workflow via command dispatch in the background but has two issues:(2) has never worked as this is a limitation of the command dispatch via repository_dispatch triggers. Repository dispatch always runs against the default branch and has no option to run elsewhere. Therefore the job statuses will never report back to the correct PR or commit automatically.
(1) used to work, but it appears got broken when we moved away from each job being defined in a single file and moved to re-usable workflows (around June 2024 #942). This used to work by having a special case when checking out the code to specify the commit hash via an env variable which was populated when in repository_dispatch mode from the triggering payload.
Possible solutions
1. Pass the SHA explicitly into all workflows
The most direct fix for (1) is to avoid ever using the
githubcontext for the current SHA. Most actions we use have the option for the SHA to be specified manually (but fallback to the default github context).We would then have a switch in the
run-acceptance-testsworkflow to check if we're inrepository_dispatchmode and pull the SHA from the payload as we used to do.Interestingly, this will also now fix (2) because we've since moved to the explicit sentinel status action on which we can specify the SHA, and this is the only check required to merge the PR.
Drawbacks
This is tricky to prove that we've added overrides to all the correct places that require the current SHA. It's also possible that some actions are hard coded to use the SHA from the context and can't be overridden (including our versioning action, though we can fix this). Furthermore, this is very prone to breaking again in the future as the workflows evolve and people forget to specify the SHA from the input explicitly on some steps which could result in unexpected failures.
2. Change command dispatch to use workflow_dispatch
Workflow dispatch is the same mechanism used when running a job manually from the actions tab. This has the added features of having a
refparameter which allows the job to be run for a specific branch or commit. This would naturally operate on the correct SHA and report back all steps progress to the PR, avoiding manually overriding the SHA in every step.Challenges
Command dispatch has since also be leveraged for the
/releasecommand. It appears it's not possible to specify the type of dispatch per command so we'd need to also rework the release workflow which is heavily reliant on the repository dispatch payload which is not available when using workflow dispatch.It's possible we might be able to have two separate command dispatch handlers - one for each target command, but I'm not yet sure on if they would error if an unrecognised command was attempted.
Furthermore, this whole command dispatch process isn't currently implemented for 3rd party providers as it requires personal access tokens to be configured and therefore we need a process that works for them also.
3. Remove command dispatch and update instructions
It would be trivial to update the run-acceptance-tests workflow to allow workflow_dispatch. This allows it to be triggered in the GitHub actions UI. We could then update the instructions comment to link the maintainer to where to click the run button rather than implementing a whole complicated command dispatch mechanism.
When we originally wrote the command dispatch mechanism there was not a clickable UI for running actions directly which therefore mandated this complex setup, but perhaps it's a worthwhile simplification given the issues raised above.
4. Remove all custom process for third parties
At the time we implemented the original command dispatch setup, GitHub only had the option to approve first-time contributors. Once approved once, a third party could then execute workflows on subsequent PRs without approval.
This is no longer the case - see the docs on approving workflows for forks). There is now the option to always require approval before running workflows from third parties. This can be enforced at the organisation level.
Proposed steps:
github.event.pull_request.head.repo.full_name