I suspect this will not reproduce for us given the reproduction instructions. The code is structured like this:

foreach (objects as object) {
  foreach (accounts as account) {
    try {
      make_comment();
      make_link();
      break; // <<< Important!
    } catch (Exception) {
      phlog(...);
    }
  }
}

Particularly, note the break;.

The intended meaning of this block is "try each account until one succeeds; when it does, stop trying".

I would guess that what you are actually seeing is that an exception in make_link() causing the make_comment() code to fire again, incorrectly.

You could verify this by turning off link posting. That is, if you follow these steps:

• Revert to clean master.
• Disable the "post a link" option in the JIRA adapter.
• Make an update to an object with multiple subscriber accounts, per reproduction instructions here.

...I suspect you will see only one comment post.

In this vein, the immediate error arising while creating the link is probably somewhere in bin/phd log. Resolving this error would likely resolve the problem.

Our behavior here is also incorrect, but the assumption here and in D18001 (that we didn't consider this case and that the code makes no effort to account for it) isn't correct either, and thus the resolution in D18001 may not be the best solution available to us.

If I'm right about this, I'd like to understand what the actual error occurring on link posting is before choosing a solution. We probably can not identify this error on our own, given only the instructions and information you've provided.

If I'm not right about this, I'd like to understand why the code isn't already working as intended before choosing a solution. From a plain reading of the code, the break; means it should not exhibit the behavior you describe.

We can make an effort to confirm that we can't reproduce this, but setup is a pain and I'm reasonably sure the effort will be fruitless, so this is difficult to prioritize.

All that makes a lot of sense, I will try what you suggest and see if that was the source of the evil.

Indeed there are the predicted error messages in the phd log. I will try to explore more and see what is wrong. It is probably related to the issue I reported (T12726), likely some of the fields are incorrectly created by arcanist in the first place.

I will revert to master and see if the behaviour happens only with reviews created by the "broken windows user" that I talk about on the other issue.

[24-May-2017 11:08:41 Europe/Madrid] [2017-05-24 11:08:41] EXCEPTION: (HTTPFutureHTTPResponseStatus) [HTTP/403] 
{"errorMessages":["Issue linking is currently disabled."],"errors":{}} at [<phutil>/src/future/http/BaseHTTPFuture.php:339]
[24-May-2017 11:08:41 Europe/Madrid] arcanist(head=master, ref.master=3c4735795a29), phabricator(head=master, ref.master=ef839192aa5a), phutil(head=master, ref.master=a900d7b63e95)
[24-May-2017 11:08:41 Europe/Madrid]   #0 <#2> BaseHTTPFuture::parseRawHTTPResponse(string) called at [<phutil>/src/future/http/HTTPSFuture.php:418]
[24-May-2017 11:08:41 Europe/Madrid]   #1 <#2> HTTPSFuture::isReady() called at [<phutil>/src/future/Future.php:37]
[24-May-2017 11:08:41 Europe/Madrid]   #2 <#2> Future::resolve() called at [<phutil>/src/future/http/BaseHTTPFuture.php:279]
[24-May-2017 11:08:41 Europe/Madrid]   #3 <#2> BaseHTTPFuture::resolvex() called at [<phutil>/src/future/oauth/PhutilOAuth1Future.php:279]
[24-May-2017 11:08:41 Europe/Madrid]   #4 <#2> PhutilOAuth1Future::resolveJSON() called at [<phabricator>/src/applications/doorkeeper/worker/DoorkeeperJIRAFeedWorker.php:250]
[24-May-2017 11:08:41 Europe/Madrid]   #5 <#2> DoorkeeperJIRAFeedWorker::postLink(PhabricatorExternalAccount, string) called at [<phabricator>/src/applications/doorkeeper/worker/DoorkeeperJIRAFeedWorker.php:104]
[24-May-2017 11:08:41 Europe/Madrid]   #6 phlog(HTTPFutureHTTPResponseStatus) called at [<phabricator>/src/applications/doorkeeper/worker/DoorkeeperJIRAFeedWorker.php:110]
[24-May-2017 11:08:41 Europe/Madrid]   #7 DoorkeeperJIRAFeedWorker::publishFeedStory() called at [<phabricator>/src/applications/doorkeeper/worker/DoorkeeperFeedWorker.php:180]
[24-May-2017 11:08:41 Europe/Madrid]   #8 DoorkeeperFeedWorker::doWork() called at [<phabricator>/src/infrastructure/daemon/workers/PhabricatorWorker.php:123]
[24-May-2017 11:08:41 Europe/Madrid]   #9 PhabricatorWorker::executeTask() called at [<phabricator>/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php:171]
[24-May-2017 11:08:41 Europe/Madrid]   #10 PhabricatorWorkerActiveTask::executeTask() called at [<phabricator>/src/infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php:22]
[24-May-2017 11:08:41 Europe/Madrid]   #11 PhabricatorTaskmasterDaemon::run() called at [<phutil>/src/daemon/PhutilDaemon.php:219]
[24-May-2017 11:08:41 Europe/Madrid]   #12 PhutilDaemon::execute() called at [<phutil>/scripts/daemon/exec/exec_daemon.php:131]

Our behavior here is also legitimately wrong, and we can look at fixing this upsteam once we understand what's actually going wrong. I think if you dig in a bit you'll find a set of reproduction steps that are something like this:

• Create two accounts, A and B.
• Create a JIRA adapter with "Comment" and "Link" enabled.
• Link A and B to JIRA.
• <<<In JIRA, go toggle some setting which disables linking for those users, or disables linking for the entire JIRA install, or whatever else.>>>
• Add A and B as reviewers on change R (then post a comment or whatever).
• Observe: Comments post twice (what happens with links?)

We need to know what the missing step is -- it might be a JIRA setting like "Disable linking"; it might be an account setting like "Prevent this user from linking tickets to external objects"; it might be an issue setting like not giving the user permission on the issue; or it might be something else like you haven't paid Atlassian enough Link Tax. I'm not familiar enough with JIRA to offer much of a guess.

If that's roughly what's happened, we can look at solutions based on what the underlying issue is -- maybe testing for settings, permissions, etc.

Next steps:

• We're waiting for the missing JIRA configuration, described above, that will let us reproduce this issue and move forward.
• If we don't receive the missing step, we can't move forward. We'll close this report after it spends a few more days in limbo.
• If you identify the missing step in the future (maybe you just don't have time now, or can't figure it out after some digging) feel free to file a new report at that time, even if we close this one.