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

Delete branch option does not appear to respect forks #881

Closed
MichaelJJ opened this issue Dec 13, 2022 · 8 comments
Closed

Delete branch option does not appear to respect forks #881

MichaelJJ opened this issue Dec 13, 2022 · 8 comments
Assignees
Labels
bug Something isn't working

Comments

@MichaelJJ
Copy link

MichaelJJ commented Dec 13, 2022

Describe your issue

It appears that the delete branch option does not identify if the branch is from a fork and will attempt to delete a branch with the same name in the parent repository.

Further context

As an example, if I fork a repository, the PR branch will be my-fork:main. If delete branch is true, then the stale action will attempt to delete branch main in the parent (primary) repo.

The stale action should check if the closed PR branch is a fork and either skip branch deletion, or attempt to delete the branch in the fork.

Looking at the code https://github.com/actions/stale/blob/main/src/classes/issues-processor.ts#L923 the action is simply taking pullRequest.head.ref which is simply the branch name and does not give an indication if this is a fork repo branch. The payload appears to have several ways to detect if this branch is from a fork, including pullRequest.head.repo.fork: true or checking pullRequest.head.repo.owner.

@MichaelJJ MichaelJJ added the bug Something isn't working label Dec 13, 2022
@IvanZosimov
Copy link
Contributor

Hi, @MichaelJJ 👋 Thanks for the bug report, we will take a look at it and get back to you with updates!

@dsame
Copy link
Contributor

dsame commented Jan 23, 2023

Hello @MichaelJJ,

While trying to reproduce the problem it occurred to me i did not understand the issue well.

the branch is from a fork according to the description literally means a branch of the other repo used to create a PR

So can the desired behaviour be said as "do not attempt to delete the branches for the staled PR"?

@MichaelJJ
Copy link
Author

I believe the simplest fix would be to check if the PR head is a fork repository, skip branch deletion for that PR even if the option is enabled.

Here is an example:

I fork a repo, and commit changes to a branch named "test-branch" on the Fork. I create a Pull Request from Fork branch "test" to the parent (original) repo.

The Pull Request becomes stale, and I have the delete branch option enabled. When the stale job runs, it closes the Pull Request in the parent (original) repo and then attempts to delete a branch named "test-branch" in Repo A, even though the PR branch originates from a fork.

If you look at the code here: https://github.com/actions/stale/blob/main/src/classes/issues-processor.ts#L923 it is simply looking for the PR branch name, and should also be checking if the PR originates from a fork. If my PR head is "ForkA:test-branch", the code pullRequest.head.ref will simply return test-branch, which may result in deleting a branch that should not be removed.

@dsame
Copy link
Contributor

dsame commented Jan 24, 2023

@MichaelJJ , and here we are
https://github.com/actions/stale/pull/913/files#diff-93e32a46e13f11c2066be36506b0f153d7da2b6b8d787da9177c10692ce50fc5R926

The idea is: delete the branch if

  • pullRequest.head.repo === null -- PR's branch is not forked
    or
  • pullRequest.head.repo.full_name === '${context.repo.owner}/${context.repo.repo}' - PR is from the same repo

Do you think we are on the same page?

@MichaelJJ
Copy link
Author

I think that would cover it, the branch would only be deleted if the PR head is the repo, and not a fork. Thanks!

@dsame
Copy link
Contributor

dsame commented Jan 26, 2023

@MichaelJJ The related PR merged and can be used immediately with

- uses: actions/stale@main

@dsame
Copy link
Contributor

dsame commented Feb 2, 2023

Hello @MichaelJJ , i am closing the issue because the related PR has been merged, but please feel free to reopen it or create new one if the problem still exists.

@dsame dsame closed this as completed Feb 2, 2023
@panticmilos
Copy link
Contributor

hi @MichaelJJ, we have released a new major version of action, that is including a fix for this issue.

Could you confirm everything works as expected?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants