\"Mentioned in commit\" system note attributed to wrong user due to merge train push identity
## Summary When a merge request is merged via the merge train, the automatically generated "mentioned in commit" system note on the MR can be attributed to the **wrong user** — specifically, a user who had no involvement in the MR. ### Observed behavior MR [!227515](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/227515) was merged by **Igor Drozdov** via the merge train. The resulting merge commit is `5e5ac3dd`. A system note was created on the MR: > **Brendan Lynch** mentioned in commit 5e5ac3dd Brendan Lynch had **no involvement** in this MR — not as author, reviewer, assignee, or commenter. The note should have been attributed to Igor Drozdov (the merge user) or Marc Shaw (the MR author). ### Expected behavior The "mentioned in commit" system note should be attributed to either the merge user or the commit's git author — not an unrelated user. ## Root Cause Analysis ### How cross-reference notes are created The call chain for "mentioned in commit" system notes is: ``` PostReceiveWorker (identifies the PUSHING USER from git hook identity) → Git::ProcessRefChangesService (current_user = pushing user) → Git::BranchPushService (current_user = pushing user) → Git::BranchHooksService#enqueue_process_commit_messages → ProcessCommitWorker.perform_in(project_id, current_user.id, commit_hash) → commit.create_cross_references!(user) # user = pushing user → SystemNoteService.cross_reference(ref, commit, author) → Note.create(author: PUSHING_USER) ``` The **author** of the system note is set to the **git pushing user** (the user whose identity is sent to Gitaly for the `ff_merge` operation), **not** the commit's git author. ### The merge train flow 1. `MergeTrains::RefreshService#execute` ([`ee/app/services/merge_trains/refresh_service.rb:24-26`](ee/app/services/merge_trains/refresh_service.rb#L24-26)) iterates over all cars, creating `RefreshMergeRequestService.new(car.target_project, car.user, ...)` 2. `RefreshMergeRequestService#merge!` ([`ee/app/services/merge_trains/refresh_merge_request_service.rb:158`](ee/app/services/merge_trains/refresh_merge_request_service.rb#L158)) calls `MergeService.new(current_user: merge_request.merge_user)` 3. `FromTrainRef#execute_git_merge!` ([`ee/app/services/merge_requests/merge_strategies/from_train_ref.rb:41`](ee/app/services/merge_requests/merge_strategies/from_train_ref.rb#L41)) calls `repository.ff_merge(current_user, ...)` — the `current_user` is `merge_request.merge_user` 4. Gitaly performs the ff-merge, triggering the post-receive hook with this user's identity Each merge train merge is a separate git push to the target branch, each triggering its own `PostReceiveWorker`. ### The bug The `ProcessCommitWorker` ([`app/workers/process_commit_worker.rb:56`](app/workers/process_commit_worker.rb#L56)) passes the **pushing user** to `commit.create_cross_references!(user)`, overriding the default `self.author` (the commit's git author from git metadata). Additionally, `ProcessCommitWorker` has `deduplicate :until_executed`. If overlapping merge train pushes or subsequent pushes to master process the same commit `5e5ac3dd`, and the first worker to execute happens to have a **different user_id** (e.g., from a concurrent merge train refresh triggered by a different user's activity), the cross-reference note is attributed to that user. ### Key code locations | File | Line | Issue | |------|------|-------| | `app/workers/process_commit_worker.rb` | [L33-56](app/workers/process_commit_worker.rb#L33-56) | Uses `user_id` (the **pusher**) as the cross-reference author, not the commit's git author | | `app/services/git/branch_hooks_service.rb` | [L136-142](app/services/git/branch_hooks_service.rb#L136-142) | Passes `current_user.id` (pushing user) to `ProcessCommitWorker` | | `app/models/concerns/mentionable.rb` | [L137](app/models/concerns/mentionable.rb#L137) | `create_cross_references!` defaults to `self.author` but `ProcessCommitWorker` overrides with pushing user | | `ee/app/services/merge_trains/refresh_service.rb` | [L24-26](ee/app/services/merge_trains/refresh_service.rb#L24-26) | Uses `car.user` as `current_user` for the refresh service | | `ee/app/services/merge_trains/refresh_merge_request_service.rb` | [L158](ee/app/services/merge_trains/refresh_merge_request_service.rb#L158) | Uses `merge_request.merge_user` for `MergeService` | ### Potential fixes 1. **Use the commit's git author** for cross-reference notes instead of the pushing user. In `ProcessCommitWorker`, change `commit.create_cross_references!(user, closed_issues)` to use the default `commit.create_cross_references!(commit.author || user, closed_issues)` — falling back to the pushing user only when the commit author can't be resolved to a GitLab user. 2. **Use the merge commit author** specifically in the merge train context. Ensure the Gitaly push identity always matches the merge commit author for merge train merges. 3. **Investigate the deduplication/race condition** in `ProcessCommitWorker` that allows a later push by a different user to process the same commit and create the note under the wrong identity.
issue