Diff author is allowed to remove blocking package owner reviewer

Ahoy,

As part of my playing around with a webhook for custom review rules (see here for context), I noticed the following:

Description

As the author of a diff that triggers the addition of an owner package as a blocking reviewer (ie, with the settings in the image below), I’m actually free to remove the blocking reviewer and get the diff landed without input from the package owner.

Screenshot 2021-01-21 at 13.40.13

I was expecting the same behaviour as for reviewers added via a herald rule: in that case, if project X was added as a blocking reviewer and I remove it from the diff, herald immediately re-adds it, as illustrated in this screenshot:

Screenshot 2021-01-21 at 13.43.46

The obvious workaround is to add owners via a herald rule, though it makes the Owner package feature less useful, as each package then requires a rule that checks if that owner is involved, and adds it as a reviewer.


Reproduction Instructions

  • On a repo, define an owner package (via <your-phab-instance>/owners/ that requires all changes to be reviewed in a blocking manner for paths /blocked-foo and set the owner to some group G (of which you are not a member)
  • Create a file /blocked-foo/bar.txt and commit, then create a diff
  • Remove the blocking reviewer from the reviewer list
  • Get the diff accepted by someone who is not part of group G
  • Land the diff

Phabricator/Arcanist Version
phabricator 86ad69863930 Oct 19 2020|b2e96df3a3be|
arcanist |ac54d61d7af2 Oct 19 2020


Notes
The account with which I tested this is an admin account, but given phabricator’s philosophy I’m not expecting this to be a factor? If you’d like me to test with a non-admin user I’ll be happy to oblige. Non-admin accounts can remove blocking reviewing owners all the same.

Kindly,
Julien

This is intended behavior.