Cannot manually close revisions

Hello, it is impossible for us to manually close revisions (for instance after forgetting to specify the revision in the commit message), including for the diff author. The bug is very similar to this one from a few years ago.

We have the issue on our instance and I can reproduce it on a test instance at Phacility. The bug happens both with differential.always-allow-close set to true or false, leading me to believe this is a bug with the feature itself, and not just a configuration mistake.

Thanks in advance for the help! :slightly_smiling_face:

Reproduction Instructions

  1. Create a test instance at https://admin.phacility.com/.
  2. Create a new git repository hosted on it and activate it.
  3. Create a new diff against this repo with the following raw diff
    diff --git a/README b/README
    new file mode 100644
    index 0000000..345e6ae
    --- /dev/null
    +++ b/README
    @@ -0,0 +1 @@
    +Test
    
  4. Try to close the diff and see that the option is not available.
  5. Set differential.always-allow-close to true in the settings and see that the problem still remains.

screen

Phabricator/Arcanist Version

phabricator  26f9ba4684a8d3a41f3a88d25a88e3f276e776e5 (Sun, Aug 11) (branched from 0a3c26998fc985a0cc660fe6a8e92184c6b08f69 on origin) 
arcanist     feb5f4d42c4fe0001e76428e80d5e88262308802 (Jun 22 2019) (branched from d92fa96366c0ed50e4257508148aa75192d4fb1f on origin) 
phutil       8df85007f38ecd06867582fa0539429a3ae83a37 (Wed, Jul 31) (branched from b416093386a225b1d9a2de906899b94cbf4babcb on origin)

Has the revision been “Accepted”? You can only close “Accepted” revisions.

Oh snap, is this a “recent” condition on being able to close a revision? The developer thought that not accepting was the root of the issue, but I was convinced otherwise…
I skimmed the documentation but didn’t find confirmation. Is this stated somewhere?

Sorry for the disturbance in any case, and thanks for the answer.

Oh snap, is this a “recent” condition on being able to close a revision?

I think this behavior was added in Feb 2014, in https://secure.phabricator.com/D8307. See line 347-351 of DifferentialTransactionEditor.php.

It’s possible it’s older than that, but I couldn’t immediately find evidence of the behavior prior to that change.

Is this stated somewhere?

It’s probably not in the documentation, but you’ll get an explicit error if you attempt to apply the action with arc or via the API:

$ arc close-revision D89
Usage Exception: Revision D89 can not be closed. You can only close revisions which have been 'accepted'.