Approve gets re-requested occasionally

[vojvodics]

Describe the bug

I'm unsure if this is a bug with our config, or in gitstream.
We have a rule to require multiple approves if the PR is > than some estimated time. And also a rule that automatically assigns a review for a person based on the files changed and who created a PR:

  complex_review:
    if:
      - {{ calc.etr >= 5 }}
    run:
      - action: set-required-approvals@v1
        args:
          approvals: {{ 3 if (calc.etr >= 10) else 2 }}

  {% for item in teams %}
  assign_reviewer_files_{{ loop.index }}:
    if:
      - {{ files | match(list=item.files) | some }}
      - {{ item.team_members | reject(term=pr.author) | reject(list=pr.reviewers) | length >= 1 }}
    run:
      - action: add-reviewers@v1
        args:
          reviewers: {{ item.team_members | reject(term=pr.author) | reject(list=pr.reviewers) }}
  {% endfor %}

  {% for item in teams %}
  assign_team_reviewer_{{ loop.index }}:
    if:
      - {{ item.team_members | includes(term=pr.author) }}
      - {{ item.team_members | reject(term=pr.author) | reject(list=pr.reviewers) | length >= 1 }}
    run:
      - action: add-reviewers@v1
        args:
          reviewers: {{ item.team_members | reject(term=pr.author) | reject(list=pr.reviewers) }}
  {% endfor %}

// reject is added because gitstream would fail if it required a review from a person that created a PR and also codeowners who github adds automatically.

The issue doesn't occur on every PR, but sometimes if the PR is >= 10min, gitstream bot re-requests a review from the last person that approved the PR:

This causes the approve to never be accepted and an infinite loop.

To Reproduce

Provided above, not sure if I can create a reproduction as it requires a bit more complex setup than usual

Expected behavior

Approve shouldn't be re-requested

[vim-zz]

Hi, I'm still looking at this issue, but just from a quick look, you can also use fail_on_error: false in add-reviewers, and then you probably don't need the reject filter on the action itself.

[vojvodics]

@vim-zz thanks. Would gitstream still request a review from others? E.g. I have 3 people in the array, one of which is a codeowner - would the other 2 get a request for a review or would it skip everything?

[vim-zz]

It shouls still assign the other 2

[tom-moore]

Running into this also with a simpler rule. It's a bit annoying as when the review is re-requested it wipes out the 'team' review showing as finished on the PR. The required approval check still passes as an individual from the team has still approved but the re-request and pending team approval on the PR looks a bit strange.

For us, it seems to actually be happening to all our PRs that I can see. A member of the team approves and then the team gets re-requested. It does not occur if someone who is not in the required/assigned team approves.
We just put this new rule into place in the last couple of days.
Potentially adding a check for the rule to see if the team has already approved before running it could work but not sure if that's a good idea.

Not that the reason we're doing add-reviewers then require-reviewers with auto_assign false is because I thought that might have been causing it and was an attempt to fix it. Was the same behaviour with just a require-reviewers in our first rule below and also happens with the second rule (only using add-reviewers).

  qa_required_approval:
    if:
      - {{ not is.hotfix }} 
      - {{ is.qa_required }}
    run:
      - action: add-reviewers@v1
        args:
          reviewers: [<org>/qa]
      - action: require-reviewers@v1
        args:
          reviewers: [<org>/qa]
          also_assign: false
  qa_approval:
    if:
      - {{ not is.hotfix }} 
      - {{ not is.qa_required }}
    run:
      - action: add-reviewers@v1
        args:
          reviewers: [<org>/qa]