DAP reviewer assignment: confirm service account identity and permissions for trigger-driven runs

Phase A. Needs an answer before the project setting is removed.

Problem — corrected after code verification

The original framing ("bespoke runs as the author, trigger runs as the service account") was imprecise. Verified mechanics:

Attribution does not change. In both paths the reviewer-assignment system note is authored by the service account: the flow's update_merge_request tool call authenticates with a composite OAuth token whose resource_owner_id is the service account (ee/app/services/ai/duo_workflows/create_composite_oauth_access_token_service.rb:55-65), so API-side current_user — and therefore the system note author (app/services/merge_requests/base_service.rb:244-247) — is the service account either way.

What changes is the human in the composite identity. Every ability must be satisfied by both the service account and the scoped human user (app/models/ability.rb:98-105, 187-207 — fail-closed intersection):

Bespoke path Trigger path
Human in composite identity MR author (auto_assign_reviewers_worker.rb:39-42) whoever marked the MR ready (cloud_event.current_user)
Non-human initiator allowed (author can be a bot) refused — run_service.rb:80 returns cannot be triggered by non-human users
Progress notes on the MR none (flow posts its own recommendation comment) "🔄 Processing…" note authored by the service account (Ai::FlowTriggers::CreateNoteService, run_service.rb:46-48)

Consequences:

  1. Bot-marked-ready MRs stop triggering entirely (current_user.human? guard). Additionally the shared worker aborts when the event has no current_user at all (cloud_events_flow_trigger_worker.rb:28-32).
  2. The permission-critical human changes from author to ready-marker. Setting reviewers requires admin_merge_request — Developer+ (app/services/merge_requests/base_service.rb:200-226 filter_reviewer → can_admin_issuable?; config/authz/roles/developer.yml). The service account has Developer via auto-membership (ee/app/services/ai/catalog/item_consumers/create_service.rb:220-231, SERVICE_ACCOUNT_ACCESS_LEVEL = ::Gitlab::Access::DEVELOPER), so the effective constraint is the human. Note the bespoke path already fails silently today for MRs authored by non-Developers; the trigger path may actually widen coverage there (the ready-marker is often a maintainer), while narrowing it for bots.
  3. Failures are silent. When admin_merge_request is denied, filter_reviewer just deletes reviewer_ids from the params — the API returns 200 with the MR unchanged; no error, no note. Hard tool failures land only in the agent session logs. Only start-time failures produce a visible note (CreateNoteService#mark_failed).
  4. New user-visible noise: the trigger path posts a progress note on every fire (the resource is a Noteable); the bespoke path posts nothing until the flow's own recommendation comment. Check whether suppress_mention_progress_note-style suppression should be extended, or the note accepted.

Questions to resolve

  • Is the change of the composite-identity human (author → ready-marker) acceptable for Beta? /cc @phikai
  • Is losing bot-marked-ready merge requests acceptable, and how many real merge requests does that affect? (Query: ready-transition events by author type.)
  • Is the per-fire progress note acceptable, or should it be suppressed for this flow?
  • Confirm the service account provisioning is in place for affected projects: group-level consumer with composite_identity_enforced: true service account (create_service.rb:141-217), Developer membership on the target project (create_service.rb:220-231).

Verification checklist (staging or a flag-enabled production project)

  1. Enable the Recommend Reviewers flow for the group; verify the duo-recommend-reviewers-<group> service account exists and has Developer on the test project.
  2. Create a Merge request > Marked ready trigger; verify Ai::FlowTrigger validation passes (consumer_must_have_active_service_account, flow_trigger.rb:122-126).
  3. Mark an MR ready as a Developer+ human → flow runs, reviewers set, system note authored by the service account.
  4. Mark an MR ready as a Reporter → expect the silent no-op (200, no reviewers, nothing on the MR); confirm where it surfaces (agent session logs) and record it.
  5. Mark an MR ready via a bot/automation → confirm no trigger fires; record the log line.
  6. Compare the MR activity feed against a bespoke-path run for attribution parity.

Acceptance criteria

  • Each question above answered and recorded on this issue
  • Verification checklist executed and results recorded
  • Any permission gap either fixed or documented as a prerequisite in the setup docs
Edited by Marc Shaw