mergeRequestSetReviewers silently ignores unresolvable reviewer usernames

Bug

The mergeRequestSetReviewers GraphQL mutation reports success (errors: []) when passed reviewer usernames that don't resolve to a visible user, without actually updating the merge request's reviewers.

Root cause

UsersFinder.new(current_user, username: usernames).execute in app/graphql/mutations/merge_requests/set_reviewers.rb#reviewer_ids silently omits any username that doesn't resolve — no error recorded. With operationMode: APPEND, the "new" reviewer ID set then equals the existing one. MergeRequests::UpdateReviewersService#execute (app/services/merge_requests/update_reviewers_service.rb:17) detects no change and takes its early return. The mutation returns success with an empty errors array.

Related, same mechanism: on instances without the multiple_merge_request_reviewers licensed feature, reviewer_ids truncates to .first(1) (the EE override at ee/app/services/ee/merge_requests/update_reviewers_service.rb:10 skips the truncation only when licensed). Because APPEND orders existing reviewers before new ones, the truncation keeps the existing reviewer and drops the one being added — another silent no-change. Minor in practice: the caller below requires Premium or above.

Not affected, for reference: a username that resolves to a real user who cannot read the merge request is already handled correctly — MergeRequests::UpdateService#new_user_ids records a model error that surfaces through errors.

Impact

The Duo Agent Platform recommend_reviewers flow calls this mutation through a new add_merge_request_reviewers tool (gitlab-org/modelops/applied-ml/code-suggestions/ai-assist!6040 (merged)). Because the mutation cannot report failure, the tool can only echo back the usernames it was given. The flow posts its summary comment naming the reviewer before the assignment step runs, so a slightly wrong username produces a merge request with a comment claiming a reviewer was added and no reviewer actually added — the same symptom as #603172, which that MR is fixing.

Any GraphQL API caller is affected, not just the Duo Agent Platform.

Proposed fix

Validate the requested usernames up front in Mutations::MergeRequests::SetReviewers and return the unresolvable ones through the mutation's existing errors array. Apply no change at all if any username is unresolvable (atomic, not partial), so callers get a predictable result.

Why reuse errors instead of adding a new payload field:

• field :errors (app/graphql/mutations/base_mutation.rb:15) already declares scopes: [:api, :read_api, :ai_workflows], so it is readable by the ai_workflows-scoped token the flow carries.
• A new field would need its own scopes declaration. Both the payload's merge_request field and Types::MergeRequestType#reviewers (app/graphql/types/merge_request_type.rb:247) default to scopes: [:api, :read_api], which is why the flow cannot simply read the resulting reviewers back.
• It fixes the behaviour for every caller, not only the Duo Agent Platform.

Once shipped, the duo-workflow-service tool can stop echoing its input and report what was actually assigned.

Behaviour change to weigh in review

Callers that currently pass unresolvable usernames and get silence would start receiving an error in errors. The reviewer sidebar uses this mutation (app/assets/javascripts/merge_requests/components/reviewers/queries/set_reviewers.mutation.graphql) but only ever sends usernames chosen from a picker, so no practical impact is expected there.

Steps to reproduce

Against a merge request that already has one reviewer:

mutation {
  mergeRequestSetReviewers(input: {
    projectPath: "group/project"
    iid: "1"
    reviewerUsernames: ["does_not_exist"]
    operationMode: APPEND
  }) {
    errors
    mergeRequest {
      reviewers {
        nodes { username }
      }
    }
  }
}

Observed: errors: [], and reviewers unchanged — does_not_exist was never added, and nothing signals that to the caller.