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: 2enum value so old rows still deserialize to a label. - The feature flag
dap_powered_recommend_reviewerswas turned off on gprd and gstg on 2026-09-02 (rollout issue #602418 (closed)). It wastype: 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:
EE::MergeRequests::CreateService(line 38) andEE::MergeRequests::UpdateService(line 126) enqueuedAutoAssignReviewersWorkeron every merge request create and every draft-to-ready transition.- The worker loaded the merge request, checked draft status, loaded reviewers, then called
AssignService. AssignServiceran a licensed-feature check and aset_merge_request_metadatapolicy check, thenStrategyFactory.buildreturned nil becauseSTRATEGIESmaps onlycode_owners, and it returnedskipped('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
UPDATEat 38.8 seconds and flagged it for exceeding the 100ms query timing guideline. There is no index onreviewer_assignment_strategy, so it seq-scansproject_settings(amediumtable, 10-50 GB perdb/docs/project_settings.yml). - Production has a 15 second
statement_timeoutthat a rawexecutedoes not disable, so the statement would not have survived there. Fixing that meant eitherdisable_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.rbandapp/models/users/group_callout.rbdelete the key and leave a# N removed in <MR link>comment;app/models/merge_request_diff.rbkeeps deprecatedstate_machinestates 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#deserializeismapping.key(value)), andserializepassesnilstraight through. The column issmallint 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 fordap_poweredand false fornil.
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: thedap_poweredcontext previously asserted the worker delegated toAssignServiceand assigned nobody. It now assertsAssignServiceis never constructed.ee/spec/services/merge_requests/reviewer_assignment/assign_service_spec.rb: thedap_poweredcontext previously expected'No strategy available'. Called directly it now returns'Feature disabled', andStrategyFactory.buildis never reached.ee/spec/models/ee/project_setting_spec.rb: new context assertingreviewer_auto_assignment_enabled?is false fordap_powered.
Verification performed (all local, all passing)
ee/spec/models/ee/project_setting_spec.rb- 219 examplesspec/models/project_setting_spec.rbandee/spec/views/projects/settings/merge_requests/_reviewer_auto_assignment_settings.html.haml_spec.rb- 95 examplesee/spec/workers/merge_requests/auto_assign_reviewers_worker_spec.rb- 9 examplesee/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_poweredenum value. - Any change to the settings UI, the REST API allow-list, or
StrategyFactory.GET /projects/:idstill exposes"reviewer_assignment_strategy": "dap_powered"for those 6 projects; the write allow-list already rejects it.