Treat the retired dap_powered strategy as disabled

A project setting stuck on the retired dap_powered reviewer assignment strategy currently still counts as "enabled" in ProjectSetting#reviewer_auto_assignment_enabled?, because the check only excludes disabled. That sends every merge request create and every draft-to-ready transition on the 6 affected projects through a real Sidekiq worker and a real policy check, before failing to find a matching strategy and skipping. This MR changes the predicate to an allow-list so dap_powered is treated as disabled up front, with no user-visible behaviour change.

It supersedes !248480 (closed), which reset the 6 affected rows via a post-deploy migration. That approach is dropped in favour of this smaller code-only fix; see details for why.

Detailed context for AI agents

Background

dap_powered was a bespoke project setting for DAP-powered reviewer recommendation, replaced by the standard Duo Agent Platform flow-trigger setup.

  • Both write paths were removed in !248476 (merged) (2026-08-20), so the value is no longer selectable in the settings UI or the REST API.
  • The bespoke code path was deleted in !253170 (merged) (merged 2026-09-07), which deliberately kept the dap_powered: 2 enum value so old rows still deserialize to a label.
  • The feature flag dap_powered_recommend_reviewers was turned off on gprd and gstg on 2026-09-02 (rollout issue #602418 (closed)). It was type: wip, default_enabled: false, so it was never on by default anywhere.
  • Rows created before the write paths were removed still hold the value: 6 rows on GitLab.com, all internal projects. Self-managed instances are expected to have none, since the flag was never default-enabled.
  • Tracking issue: #607678 (closed).

The actual problem being fixed

A dap_powered row was inert in outcome but not free in work. Because 'dap_powered' != 'disabled' was true:

  1. EE::MergeRequests::CreateService (line 38) and EE::MergeRequests::UpdateService (line 126) enqueued AutoAssignReviewersWorker on every merge request create and every draft-to-ready transition.
  2. The worker loaded the merge request, checked draft status, loaded reviewers, then called AssignService.
  3. AssignService ran a licensed-feature check and a set_merge_request_metadata policy check, then StrategyFactory.build returned nil because STRATEGIES maps only code_owners, and it returned skipped('No strategy available').

So: a real Sidekiq job and a real policy evaluation per merge request event, with no outcome. Nothing was ever broken by this, it was pure wasted work.

Secondary effect resolved by the same change: the settings checkbox already rendered unchecked (it uses checked_value: 'code_owners'), so the column disagreed with the UI before this fix.

What this replaces

Supersedes !248480 (closed), which added a post-deploy migration to reset the 6 rows from dap_powered (2) to disabled (0). That migration was dropped because:

  • The migration testing pipeline measured the single UPDATE at 38.8 seconds and flagged it for exceeding the 100ms query timing guideline. There is no index on reviewer_assignment_strategy, so it seq-scans project_settings (a medium table, 10-50 GB per db/docs/project_settings.yml).
  • Production has a 15 second statement_timeout that a raw execute does not disable, so the statement would not have survived there. Fixing that meant either disable_statement_timeout (needs database maintainer sign-off) or batching (roughly 2-4 minutes of post-deploy runtime).
  • That is a large amount of production database work, plus a database maintainer's review, to clean 6 rows on internal projects.
  • The rows are also self-healing on any instance: the project Merge requests settings page has unchecked_value: 'disabled', so any save clears a stale row.
  • Making the code ignore the value achieves the same behavioural end state, on every instance, with no data change and no database review.

Consequence worth stating plainly: the 6 rows keep the value 2 in the database indefinitely. They are inert. If someone later wants the data tidy, a settings page save or a separate cleanup can do it.

On keeping the enum value

dap_powered: 2 stays mapped in REVIEWER_ASSIGNMENT_STRATEGIES, with a comment marking it deprecated. Rationale:

  • There is no documented project-wide rule for retiring enum values. Two patterns exist in the codebase: app/models/users/callout.rb and app/models/users/group_callout.rb delete the key and leave a # N removed in <MR link> comment; app/models/merge_request_diff.rb keeps deprecated state_machine states listed with a comment saying the values may still occur in the database.
  • Since the rows are not being reset, the value persists forever, so removing the mapping frees nothing. GitLab does not reuse enum integers either way.
  • Removing the mapping would make Rails deserialize those rows to nil (ActiveRecord::Enum::EnumType#deserialize is mapping.key(value)), and serialize passes nil straight through. The column is smallint DEFAULT 0 NOT NULL, so any save that dirtied the attribute on one of those rows would attempt to write NULL.
  • The new predicate is written as an allow-list specifically so it stays correct either way: reviewer_assignment_code_owners? is false for dap_powered and false for nil.

The change

app/models/project_setting.rb, reviewer_auto_assignment_enabled?:

before: reviewer_auto_assignment_available? && reviewer_assignment_strategy != 'disabled'
after:  reviewer_auto_assignment_available? && reviewer_assignment_code_owners?

Plus a comment on REVIEWER_ASSIGNMENT_STRATEGIES marking dap_powered: 2 as deprecated. 4 files total, including specs.

Spec changes

Three existing specs encoded the old late bail and now encode the early one:

  • ee/spec/workers/merge_requests/auto_assign_reviewers_worker_spec.rb: the dap_powered context previously asserted the worker delegated to AssignService and assigned nobody. It now asserts AssignService is never constructed.
  • ee/spec/services/merge_requests/reviewer_assignment/assign_service_spec.rb: the dap_powered context previously expected 'No strategy available'. Called directly it now returns 'Feature disabled', and StrategyFactory.build is never reached.
  • ee/spec/models/ee/project_setting_spec.rb: new context asserting reviewer_auto_assignment_enabled? is false for dap_powered.

Verification performed (all local, all passing)

  • ee/spec/models/ee/project_setting_spec.rb - 219 examples
  • spec/models/project_setting_spec.rb and ee/spec/views/projects/settings/merge_requests/_reviewer_auto_assignment_settings.html.haml_spec.rb - 95 examples
  • ee/spec/workers/merge_requests/auto_assign_reviewers_worker_spec.rb - 9 examples
  • ee/spec/services/merge_requests/reviewer_assignment/assign_service_spec.rb - 22 examples
  • RuboCop clean on all 4 changed files

Out of scope

  • Resetting the 6 rows. No migration, no console data change.
  • Removing the dap_powered enum value.
  • Any change to the settings UI, the REST API allow-list, or StrategyFactory. GET /projects/:id still exposes "reviewer_assignment_strategy": "dap_powered" for those 6 projects; the write allow-list already rejects it.
Edited by Marc Shaw

Merge request reports

Loading
Loading