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

Duplicate approvals #213

Closed
JacobHayes opened this issue Mar 1, 2023 · 8 comments · Fixed by #214
Closed

Duplicate approvals #213

JacobHayes opened this issue Mar 1, 2023 · 8 comments · Fixed by #214

Comments

@JacobHayes
Copy link

After making a change to an approved path, every subsequent commit triggers a new approval. The logs say it is looking for existing reviews, but seems to not find the last one.

Screenshot 2023-02-28 at 19 17 13

Logs from the second run:

Fetching user, pull request information, and existing reviews
Current user is github-actions[bot]
Commit SHA is AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
Pull request #123 has not been approved yet, creating approving review
Approved pull request #123

workflow config:

name: Auto approval
on:
  pull_request:
    branches:
      - main
    paths:
      - 'some-dir/**'
jobs:
  auto-approve:
    runs-on: ubuntu-latest
    permissions:
      pull-requests: write
    steps:
      - uses: hmarr/auto-approve-action@v3
        with:
          review-message: "Automatic approval of ..."

Perhaps there's a missing permission that is preventing it from fetching existing reviews?

@hmarr
Copy link
Owner

hmarr commented Mar 8, 2023

I believe the default behaviour is to re-approve when new commits are pushed to the branch (commit_id == prHead):

const prHead = pr.head.sha;
core.info(`Commit SHA is ${prHead}`);
const alreadyReviewed = reviews.some(
({ user, commit_id, state }) =>
user?.login === login && commit_id == prHead && state === "APPROVED"
);

I assume that's so that re-reviews occur when they are required by branch protections:

image

However, the state === "APPROVED" check should cover that case, as when that box is checked, existing reviews transition to DISMISSED when new commits are pushed.

I'll try removing the check in a PR.

@hmarr
Copy link
Owner

hmarr commented Mar 9, 2023

Released in v3.2.0

@pfuhrmann
Copy link

pfuhrmann commented Mar 15, 2023

I do not believe this works correctly now.

I have DISMISSED reviews, and the action is not re-approving PRs (it states that the PR was already approved by the user, which is true, but the review is dismissed and it seems to not detect that state).

Previously it worked correctly (albeit yes, it was duplicating sometimes, but I did not mind that).

@hmarr
Copy link
Owner

hmarr commented Mar 15, 2023

Thanks @pfuhrmann. It looks like it's possible to have old APPROVED reviews with newer DISMISSED reviews, and only the latest review for a given user counts towards a PR's review state, which is why this is happening. I've opened #216 to fix that.

@hmarr
Copy link
Owner

hmarr commented Mar 15, 2023

@pfuhrmann if you could confirm that you're seeing the same thing (old APPROVED reviews with newer DISMISSED reviews for a given user), that'd be helpful.

You can see all the reviews by running gh api repos/{LOGIN}/{REPO}/pulls/{PR-NUMBER}/reviews if you have the gh cli installed.

@pfuhrmann
Copy link

pfuhrmann commented Mar 15, 2023

Hi @hmarr, thanks for coming back to me so quickly!

This is what I see on the PR (redacted):

[
  {
    "id": 1340392942,
    "node_id": "PRR_kwDOH65R-s5P5MXu",
    "user": {
      "login": "github-actions[bot]",
    },
    "body": "Auto approved (low risk update)! :+1: :rocket:",
    "state": "APPROVED",
    "submitted_at": "2023-03-14T22:41:54Z",
    "commit_id": "2e0af399a670ff3971253f69539274b1dedf13a1"
  },
  {
    "id": 1340392945,
    "node_id": "PRR_kwDOH65R-s5P5MXx",
    "user": {
      "login": "github-actions[bot]",
    },
    "body": "Auto approved (low risk update)! :+1: :rocket:",
    "state": "DISMISSED",
    "submitted_at": "2023-03-14T22:41:55Z",
    "commit_id": "2e0af399a670ff3971253f69539274b1dedf13a1"
  }
]

The full output from action is:

Fetching user, pull request information, and existing reviews
Current user is github-actions[bot]
Commit SHA is 2968fc1548df680b12c727068ad3f9075c56294f
Current user already approved pull request #77, nothing to do

I assume this is the state you wrote about previously, i.e. old APPROVED reviews with newer DISMISSED reviews. Seem's DISMISSED PR submitted_at is 1s later than APPROVED. Probably an edge case, and can happen if there are multiple actions running in parallel?

@hmarr
Copy link
Owner

hmarr commented Mar 15, 2023

Great, yeah that does look like the case I described. I've merged the PR and released v3.2.1 — hopefully that fixes the issue for you!

@pfuhrmann
Copy link

@hmarr I can confirm, all our automated PRs got merged so this seems resolved. Thanks!

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 a pull request may close this issue.

3 participants