Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue #5080: [Bug]: lint-fix.yml github action doesn't work on a branch not from this repo #5081

Merged
merged 1 commit into from
Nov 16, 2024

Conversation

openhands-agent
Copy link
Contributor

@openhands-agent openhands-agent commented Nov 16, 2024

This pull request fixes #5080.

The issue has been successfully resolved. The original problem was that the GitHub Action was failing when trying to check out code from PRs originating from forks because it couldn't properly access the source repository. The fix implemented adds the correct repository parameter to the actions/checkout action, specifically using ${{ github.event.pull_request.head.repo.full_name }} which dynamically references the source repository of the PR.

This solution:

  1. Addresses the core issue by ensuring the action can access code from forked repositories
  2. Still maintains compatibility with PRs from the same repository
  3. Fixes the specific error where the action couldn't find the branch checkout-a-non-main-branch-option

The PR reviewer should know that this is a straightforward configuration change to the GitHub Actions workflow that improves the robustness of the lint-fix action by making it work with PRs from both internal branches and external forks. No code changes were required, only workflow configuration updates.

Automatic fix generated by OpenHands 🙌


To run this PR locally, use the following command:

docker run -it --rm   -p 3000:3000   -v /var/run/docker.sock:/var/run/docker.sock   --add-host host.docker.internal:host-gateway   -e SANDBOX_RUNTIME_CONTAINER_IMAGE=docker.all-hands.dev/all-hands-ai/runtime:a8ab4bf-nikolaik   --name openhands-app-a8ab4bf   docker.all-hands.dev/all-hands-ai/openhands:a8ab4bf

@neubig neubig marked this pull request as ready for review November 16, 2024 04:27
@neubig neubig enabled auto-merge (squash) November 16, 2024 04:27
@tobitege
Copy link
Collaborator

This shouldn't run on forks. People have their forks' own potential clones and this may interfere unwantedly with their work.
In addition, this mostly won't be able to commit anyways as security wouldn't be set up for this cross-repo access.

@neubig neubig disabled auto-merge November 16, 2024 04:48
@neubig
Copy link
Contributor

neubig commented Nov 16, 2024

Thanks for the comment @tobitege , but could you elaborate? This will only be run if we set the lint-fix label, and it can only be pushed to their forks' branch if they check the box saying maintainers can push to their branch. So I think the danger of allowing this is probably minimal?

@tobitege
Copy link
Collaborator

Thanks for the comment @tobitege , but could you elaborate? This will only be run if we set the lint-fix label, and it can only be pushed to their forks' branch if they check the box saying maintainers can push to their branch. So I think the danger of allowing this is probably minimal?

I just don't like this personally, and it's another thing that isn't obvious to a user/contributor and needs documentation.

@tobitege tobitege merged commit f7652bd into main Nov 16, 2024
13 checks passed
@tobitege tobitege deleted the openhands-fix-issue-5080 branch November 16, 2024 05:55
@neubig
Copy link
Contributor

neubig commented Nov 18, 2024

OK, thanks for approving anyway @tobitege !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: lint-fix.yml github action doesn't work on a branch not from this repo
3 participants