Page MenuHomePhabricator

One of our audits is displaying comments from another audit
Open, Needs TriagePublic

Description

There appears to be some kind of corruption happening with one of our audits. It is displaying comments from another audit.
Looks like the entire transaction with all the comments from one of the engineers got applied to the wrong context. In another instance that same engineer typed up a bunch of inline comments on an audit and had them vanish when he committed them (presumably to surface on another audit somewhere?)

I'm not sure what diagnostic information would be useful here. Please let me know what I can provide.

phabricator 70463e8a16d4cbe0c83b23b9328c4a359072c506 (Thu, Jun 23)
arcanist 2374403e8f80a8fb16cdc1ce2843f932632e8cf0 (Fri, Jun 17)
phutil 51c179b4c000de34d85a62188b5f9efb28afba73 (Fri, Jun 17)

Event Timeline

Some more info I dug up:

  • There are 2 commits that appear to have flipped comments, i.e. the comments that appear on audit1 actually apply to audit2 and vice versa.
  • These commits are sequential and were pushed simultaneously (i.e. the author made multiple commits on master locally before git pushing them all)
  • In both cases only comments made by the author appear out of place.
epriestley added a subscriber: epriestley.

Bug reports MUST include reproduction steps that allow us to reproduce the issue locally. Without this information, we can not move forward.

See Contributing Bug Reports and Providing Reproduction Steps for help writing an actionable bug report.

This actually is a lot worse than I originally thought.
All inline comments that are made on an audit are actually applied to the audit I was previously commenting on (like an off by 1 error). I've isolated the following things:

  • This happens across repositories (i.e. if I have multiple repositories, commenting on one commit and then switching to another repository's commit and commenting on it will produce a comment on the first commit)
  • This happens for all users
  • This happens regardless of audit status - i.e. commits with audit requested exhibit this as much as commits that don't require audit.
  • Submitting an empty comment first thing before adding any content to a commit works around this problem - subsequently added commits appear as expected.
  • The broken state can be easily detected by unavailability of the "hover state" of the overall commit field - pressing "z" does nothing

I've been trying to isolate what's really going on by making inline comments containing SHA of the commit on which they're made. Then I look for that comment in audit_transaction_comment. The one that disappears looks like it has a different source and a NULL transaction:

mysql> select * from audit_transaction_comment where content like "0599cecf1051";
+-----+--------------------------------+--------------------------------+--------------------------------+------------+--------------------------------+----------------+--------------+---------------------------------+-----------+-------------+--------------+--------------------------------+--------+-----------+------------+------------+------------+------------+--------------------+-----------------+
| id  | phid                           | transactionPHID                | authorPHID                     | viewPolicy | editPolicy                     | commentVersion | content      | contentSource                   | isDeleted | dateCreated | dateModified | commitPHID                     | pathID | isNewFile | lineNumber | lineLength | fixedState | hasReplies | replyToCommentPHID | legacyCommentID |
+-----+--------------------------------+--------------------------------+--------------------------------+------------+--------------------------------+----------------+--------------+---------------------------------+-----------+-------------+--------------+--------------------------------+--------+-----------+------------+------------+------------+------------+--------------------+-----------------+
| 229 | PHID-XCMT-ogw546sbw4xiuqpmx5hw | NULL                           | PHID-USER-773aibb7k3psylpc4z2q | public     | PHID-USER-773aibb7k3psylpc4z2q |              1 | 0599cecf1051 | {"source":"legacy","params":[]} |         0 |  1468372865 |   1468372865 | PHID-CMIT-eqww2shcxoitc47n2puu | 439868 |         1 |        122 |          2 | NULL       |          0 | NULL               |            NULL |
| 230 | PHID-XCMT-4cggxcyfxdddebgu7akn | PHID-XACT-CMIT-pni4svuvmkg42lw | PHID-USER-773aibb7k3psylpc4z2q | public     | PHID-USER-773aibb7k3psylpc4z2q |              1 | 0599cecf1051 | {"source":"web","params":[]}    |         0 |  1468372880 |   1468372882 | PHID-CMIT-xl5ktx4hpnpzsbqawwhw | 439868 |         1 |        121 |          1 | NULL       |          0 | NULL               |            NULL |
+-----+--------------------------------+--------------------------------+--------------------------------+------------+--------------------------------+----------------+--------------+---------------------------------+-----------+-------------+--------------+--------------------------------+--------+-----------+------------+------------+------------+------------+--------------------+-----------------+
2 rows in set (0.00 sec)

This is, by the way, on a brand new install that's not even a month old.
I tried to reproduce it on another install that is older and I'm not having any luck.

You can use a free instance at http://phacility.com/ if you need a clean, up to date place to test privately.

I'm unable to reproduce on a test install right now, but then again, I stopped reproducing on my install after rebooting the server. It looks like something happens eventually to cause this to break and I start getting these broken comments.
Looks like every time I get a broken one it doesn't have a transaction PHID associated with it and all comments submitted together have the same wrong associated commitPHID.

I'll keep digging around to see what else I can come up with, but I'd appreciate some pointers, if you have any.

Sounds similar to T11092, which was recently resolved.

Looks like that fix affects Differential, but not Diffusion? After reading through that bug, I can totally reproduce this on demand in Audit
I have a test instance here that reproduces this problem (both actively able to reproduce and has commits in the database that are exhibiting the behavior already): https://test-kdauzhhtarbi.phacility.com/rLIBdde2f74f2793f0216f0d76618ed335a9c802cfec, but I reproduced it reliably on 3 separate instances now.

Here are steps to reproduce:

  1. Open a commit that has not been commented on or otherwise edited in any way, let's call it rT1 (this unedited part seems key)
  2. Open the Conpherence sidebar, it doesn't matter what is in it, just as long as it is open
  3. Create an inline comment.
  4. Submit.

The comment will disappear. If you were previously looking at another commit in the same state (let's call it rT2) the rT1's comment will now appear as unsubmitted on rT2 (at the bottom of the review). The will be marked as "Not Visible" and trying to delete them causes a corruption in the UI.

Looks like there are 2 ways to defeat the repro steps:

  • Close the Conpherence panel
  • Submit an empty comment on the commit, which puts it into some stable state.

Please let me know if you need me to create another bug for this since this one was closed for lack of repro steps.

Thanks for looking into this further.