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_on never passes internal: to Notes::CreateService, so mention replies default to public.
  • Passing internal: true on the threaded reply alone does not work: Notes::BuildService#new_note enforces one confidentiality state per discussion — when in_reply_to_discussion_id resolves to an existing (necessarily public) discussion, the reply's confidentiality is force-synced to the parent discussion's, silently overriding an explicit internal: param. A forced-internal reply therefore cannot live inside the public mention discussion.

Solution

  • New Ai::Catalog::FoundationalFlow attribute force_internal_mention_replies (default false, mirroring the existing suppress_mention_progress_note opt-in pattern), enabled only for security_review/v1 — developer/v1 mention replies (and the @GitLabDuo surface) are unchanged.
  • EE::Notes::PostProcessService passes the flag into the GitlabDuoNote adapter; it round-trips through the workflow callback context so CallbackWorker deliveries 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 internal visibility 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), because deliver_result reports success from the internal note and CallbackWorker will not retry.
  • The pointer wording is flow-generic, not Security-Review-specific: GitlabDuoNote is a shared adapter and any flow opting into force_internal_mention_replies gets 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-master base.
  • RuboCop on all 6 touched Ruby files — no offenses.
  • locale/gitlab.pot regenerated via tooling/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

Edited by Meir Benayoun

Merge request reports

Loading
Loading