Force internal notes for SRF mention-triggered replies
What does this MR do and why?
Problem
@-mention-triggered Security Review Flow replies are posted as regular public notes, so on a public project the full security review — findings included — is publicly readable. Live evidence: !244428 (comment 3559697422) (internal: false). Reviewer-assignment-triggered review notes are already internal today — visibility currently depends on the trigger, not on policy.
Root cause
Ai::Messaging::Adapters::GitlabDuoNote#create_note_onnever passesinternal:toNotes::CreateService, so mention replies default to public.- Passing
internal: trueon the threaded reply alone does not work:Notes::BuildService#new_noteenforces one confidentiality state per discussion — whenin_reply_to_discussion_idresolves to an existing (necessarily public) discussion, the reply's confidentiality is force-synced to the parent discussion's, silently overriding an explicitinternal:param. A forced-internal reply therefore cannot live inside the public mention discussion.
Solution
- New
Ai::Catalog::FoundationalFlowattributeforce_internal_mention_replies(defaultfalse, mirroring the existingsuppress_mention_progress_noteopt-in pattern), enabled only forsecurity_review/v1—developer/v1mention replies (and the@GitLabDuosurface) are unchanged. EE::Notes::PostProcessServicepasses the flag into theGitlabDuoNoteadapter; it round-trips through the workflow callback context soCallbackWorkerdeliveries honor it.- Scoped to non-private projects. On a private project the noteable is already members-only, so forcing the reply internal buys no additional confidentiality and only costs the threaded-reply UX — those projects keep a normal threaded reply. Projects with
internalvisibility still force it, since any signed-in user on the instance can read them. - When the mention itself is already an internal note, the discussion's confidentiality already matches, so the reply is threaded into it — one note, in the right thread, no pointer.
- Only a forced-internal reply to a public discussion is split: the review goes to a new internal note on the noteable, and the mention discussion gets a short, content-free public pointer reply linking to it, with distinct success and error wording. The pointer is posted only once the internal note actually persisted, so a failed internal note never leaves a dangling pointer and no findings ever reach the public thread. If the pointer itself fails to persist it is logged with the noteable and internal-note ids (
Gitlab::AppLogger.warn), becausedeliver_resultreports success from the internal note andCallbackWorkerwill not retry. - The pointer wording is flow-generic, not Security-Review-specific:
GitlabDuoNoteis a shared adapter and any flow opting intoforce_internal_mention_repliesgets these strings.
How to test this locally (GDK)
CI covers the unit level. What it does not cover is the real note-creation path through a running instance, which is what this section is for. Behaviour is decided by exactly two inputs — the project's visibility, and whether the mention note is internal — so three cases are sufficient.
Prerequisites: a GDK with Duo enabled and the Security Review Flow available, and an SRF service account provisioned for the group (duo-security-review-*). Post the mention as a developer, not root.
1. Public project — the reply must split
- Create a public project in that group, and an MR in it.
- As a developer, post a note mentioning the SRF service account.
- Expect: a new internal note carrying the review (its own discussion,
internal: true), plus a short public pointer reply threaded into your mention, linking to the internal note and containing no findings.
2. Private project — the reply must stay threaded, with no pointer
- Same steps in a private project.
- Expect: a single public reply threaded into your mention discussion. No separate internal note, no pointer.
3. Public project, mention posted inside an internal note — one threaded internal reply
- In the project from (1), use Add internal note and mention the service account there.
- Expect: exactly one internal reply threaded into that same discussion. No separate note, no pointer.
Observed on a GDK running this branch (real projects, real service-account member, real Notes::CreateService path):
| case | notes created | outcome |
|---|---|---|
| 1 public project, public mention | 2 | internal: true review in its own discussion + public pointer threaded into the mention, linking #note_<id>, carrying no findings |
| 2 private project, public mention | 1 | public reply threaded into the mention, carrying the review. No internal note, no pointer |
| 3 public project, internal mention | 1 | internal: true reply threaded into the mention discussion. No pointer |
One GDK gotcha worth knowing before you start: project_authorizations is denormalized and refreshed by Sidekiq, so if rails-background-jobs is not running, adding the service account to the project appears to do nothing (max_member_access stays 0), mark_note_as_internal is denied, and internal: true silently degrades to a public note. Either run Sidekiq, or force it with AuthorizedProjectUpdate::ProjectRecalculateService.new(project).execute.
Console check for any of the three:
mr = Project.find_by_full_path('<group>/<project>').merge_requests.find_by_iid(<iid>)
mr.notes.order(:id).last(3).map { |n| [n.id, n.internal, n.discussion_id, n.note[0, 60]] }Verification
ee/spec/services/ai/messaging/adapters/gitlab_duo_note_spec.rb+ee/spec/services/ee/notes/post_process_service_spec.rb— 82 examples, 0 failures on a fresh current-masterbase.- RuboCop on all 6 touched Ruby files — no offenses.
locale/gitlab.potregenerated viatooling/bin/gettext_extractor— exactly the two pointer strings.- The pointer-failure spec was confirmed to fail without the production change, so it exercises the behaviour rather than passing vacuously.
Closes https://gitlab.com/gitlab-org/gitlab/-/work_items/606308