Check protected branch before reading CODEOWNERS

What does this MR do and why?

• Checks the protected branch setting before reading the CODEOWNERS file when deciding if a code owner rule needs approval.
• The file read is a Gitaly call. It is now skipped when the target branch does not require code owner approval, because optional sections do not matter there.
• The result is the same in every case. Only the number of Gitaly calls changes.
• This does not fix the gitlab-org/gitlab incident, because its master branch requires code owner approval. It is a quick win for other projects.

Detailed context for AI agents

The change

ApprovalWrappedCodeOwnerRule#branch_requires_code_owner_approval? had two guards after the availability check: section_optional? first, then ProtectedBranch.branch_requires_code_owner_approval?. This MR swaps them and drops the separate availability check, which ProtectedBranch.branch_requires_code_owner_approval? already does.

• section_optional? calls Gitlab::CodeOwners.optional_section?. That fetches and parses the CODEOWNERS file from Gitaly (a GetBlobs RPC). It is memoized only per request.
• ProtectedBranch.branch_requires_code_owner_approval? is a database check. It is memoized per request per branch in SafeRequestStore.
• The method is now ProtectedBranch.branch_requires_code_owner_approval? && !section_optional?, with a short comment that CODEOWNERS is checked last because it reads from Gitaly.

Why it matters

The method is reached from ApprovalState#approved? through:

• the approval mergeability check (MergeRequests::Mergeability::CheckApprovedService)
• the approvals API and the merge request widget
• the CI_MERGE_REQUEST_APPROVED CI variable, built while a runner requests a job (Ci::RegisterJobService#present_build!)
• ApprovalRules::FinalizeService

Every merge request with code owner rules paid one CODEOWNERS fetch and parse on each uncached evaluation, even when the target branch does not require code owner approval.

Why no feature flag

It is a pure reorder of two independent guards. The method returns the same boolean for every input, so there is nothing to roll out or roll back.

Edge cases

• section_optional? can return nil. The old code treated nil as "not optional". The new code does the same (!nil is true). Unchanged.
• When a section is optional, the protected branch database check now runs where it was previously skipped. It is cheap and memoized.
• A Gitaly error on this path now only surfaces when the branch requires code owner approval.

Relationship to the issue

The issue is #616383. It is about a Gitaly deadline on this CODEOWNERS fetch failing CI job delivery as scheduler_failure. This MR does not fix that for gitlab-org/gitlab, because its master branch requires code owner approval, so the fetch still happens there.

Out of scope

The bigger fix is to store section optionality on the rule at sync time. It is being discussed on the issue. It is harder because:

• approval_merge_request_rules is over the database table size limit. New columns need an exception or a side table.
• It changes how merges are enforced.

Specs changed

• ee/spec/models/approval_wrapped_code_owner_rule_spec.rb: new example that the CODEOWNERS file is not read when the target branch does not require code owner approval. It failed before the change. The existing example "checks the rule is in an optional codeowners section" now stubs the protected branch check to true, so the read still happens.
• ee/spec/services/approval_rules/finalize_service_spec.rb: "reads CODEOWNERS before opening the transaction" now creates a protected branch that requires code owner approval. The CODEOWNERS read still happens and the ordering is still checked. Without that setup it fails on the new code.

Verification

• Local specs passed: approval_wrapped_code_owner_rule_spec, finalize_service_spec (37 examples), approval_state_spec, check_approved_service_spec, sync_code_owner_approval_rules_spec, refresh/approval_service_spec.
• ee/spec/models/merge_requests/approval_grouped_code_owner_rule_spec.rb has one order-dependent failure locally. It reproduces the same way with the unchanged code, so it is pre-existing and unrelated.
• RuboCop is clean.
• Manual check on a local GDK. Setup: a project with a sectioned CODEOWNERS ([Backend] required, ^[Docs] optional), a merge request touching both, a merge request pipeline and a project runner. Gitaly calls were counted with a fresh request store per call.
  • Target branch does not require code owner approval. Before: CI_MERGE_REQUEST_APPROVED, CheckApprovedService and the approval state each made 1 Gitaly call (the CODEOWNERS fetch). After: 0 calls, same results (variable "true", check success, 0 approvals required).
  • Target branch requires code owner approval. Unchanged: 1 call each, not approved, 1 approval required.
  • Separately, on the unchanged code, injecting GRPC::DeadlineExceeded on the CODEOWNERS fetch made all three raise. It also made Ci::RegisterJobService drop the job as scheduler_failure, which reproduces the mechanism in the issue.

References

• Related to #616383

Screenshots or screen recordings

Not applicable, backend-only change with no UI.

How to set up and validate locally

1. Use a project with a CODEOWNERS file that has sections, and a target branch whose protected branch settings do not require code owner approval.

2. Open a merge request that changes files covered by CODEOWNERS.

3. In a Rails console, count Gitaly calls with a fresh request store:

   mr = MergeRequest.find(<id>)
   
   Gitlab::SafeRequestStore.ensure_request_store do
     mr.approval_state.approved?
     Gitlab::GitalyClient.get_request_count
   end

4. Expect 1 on master and 0 on this branch. With code owner approval required on the target branch, both return 1 (reload the merge request first).

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.