Merged system notes displayed as manually merged when it is not

As discussed in #436061 (comment 1861719243), it seems that the merge state change system notes sometimes display merged manually instead of linking to the related MR that triggered the merge.

It's displayed like this:

When it should be displayed like this:

I believe it's due to the execution timing of the UpdateMergeRequestsWorker which gets triggered by the initial merge commit here. It won't find the MR that has just been merged as it hasn't been marked as merged yet. I believe that background job sometimes gets executed earlier than the execution of MergeRequests::PostMergeService as we commit first and process the post merge service. I'm not sure how often this happens, but I noticed this happening a couple of times. I suppose it depends on the workload of the background workers.

I think it'd be tricky to fix this timing issue as we need to make sure the git commit succeeds first before we mark them as merged. It'd be best if we can do that somehow though.

One way to work around this timing issue would be to update the merged manually to include link to the project commit where we list MRs that include the commit as well. It'd be similar to the commit link within the merge details that links to something like 95a5e7dc. At least, that provides a way to see all MRs that contains the commit.

That link already exists in the merge details widget like below, but maybe it's helpful to link to that instead of saying merged manually incorrectly occasionally?

MRs that include this commit will be listed like this so it could be more helpful.

added backend bugtransient groupcode review labels

set weight to 2

assigned to @dskim_gitlab

marked this issue as related to #436061 (closed)

added typebug label

added devopscreate sectiondev labels

mentioned in issue #436061 (closed)

@dskim_gitlab I'd like to explore if we can fix the raciness of the note generation so that it consistently provides the other MR. If we can't find a good solution, I think the commit option is a reasonable fall back... but let's spend some time trying to solve the underlying cause first.

@phikai Yeah, that makes sense. I agree let's spend some time first to see if we can find a simple enough way to guarantee the execution order.

Should we swap it with one my assign issues for 17.0 or do you prefer to us work on it in the following milestone?

@dskim_gitlab I think %17.1 is fine to re-evaluate. Will also give us (maybe) a better sense of how common this is.

cc: @francoisrose so we can revisit for planning

@phikai Sure, that sounds good to me

changed milestone to %17.1

added twtriaged label

added UI text label

changed milestone to %17.2

added severity3 label

Moving to %17.3 due to low capacity, see create-stage#13209 (comment 1942985106)

changed milestone to %17.3

Moving to %Awaiting further demand, we've postponed this a few times already, and it doesn't seem pressing.

changed milestone to %Awaiting further demand

unassigned @dskim_gitlab

mentioned in issue #455133 (closed)

marked #455133 (closed) as a duplicate of this issue

marked this issue as related to #455133 (closed)

changed milestone to %Backlog

mentioned in issue #508613 (closed)

This has been surfaced as a potential audit complication by our Quality team. We would greatly appreciate this being fixed to have a cleaner story to tell regarding software pedigree.

marked #508613 (closed) as a duplicate of this issue

marked this issue as related to #508613 (closed)

This customer (internal link) is also experiencing this issue and concerning that this is being reported in the Peer Review dashboards as false positives as no approvals took place.

added customer label

marked this issue as related to #512551