Remove the dap_powered reviewer assignment strategy from project settings and the API
Reviewer assignment with the Duo Agent Platform has two configuration surfaces for one feature. This removes one of them - the dap_powered option on the reviewer_assignment_strategy project setting - leaving the flow trigger as the only setup path. Requested by @phikai in gitlab-org#22675 (comment 3562765776).
The settings radio group disappears (the section collapses to the code-owners checkbox for every project) and PUT /projects/:id now returns 400 for dap_powered. Only the write path closes: the enum value, the worker branch, and existing rows are deliberately left alone so a rollback stays a plain revert, which means GET can still return dap_powered and the response docs now say so.
- Closes #607677 (closed) · Epic: gitlab-org#23019
- Phase A predecessor !248388 (merged) is merged
- Confirmed the settings page renders correctly for a project that still holds
dap_powered - One behaviour change for existing
dap_poweredprojects: saving that settings section now resets them todisabled. Detail below - Sequence with the docs MR !248483 (merged) (#607680 (closed)): once the setting is gone the trigger is the only setup path, and the docs still describe the setting
Detailed context for AI agents
What changes
| Surface | Change |
|---|---|
| Settings page | The strategy radio group is gone; the section collapses to the code-owners checkbox for every project |
| REST API | values: whitelist narrowed to disabled/code_owners, so PUT /projects/:id returns 400 for dap_powered |
doc/api/projects.md |
dap_powered dropped from the writable value lists; the four GET response tables note it can still be returned; removal history note on the update table plus a {{< history >}} entry |
| OpenAPI | dap_powered dropped from the openapi_v3.yaml enum: block. openapi_v2.yaml deliberately untouched, see below |
locale/gitlab.pot |
Six now-orphaned strings removed |
reviewerAssignmentStrategy was never exposed over GraphQL, so there's no GraphQL change. The controller needs no change either - the permitted attribute stays for code_owners.
Deliberately out of scope
So that rolling this back stays a plain revert and existing rows keep working:
- the
dap_powered: 2enum value inapp/models/project_setting.rb - the DAP branch in
MergeRequests::AutoAssignReviewersWorker - the existing rows themselves
Those come in later phases, after the trigger path is verified in production.
Behaviour change for API clients
The REST whitelist was never feature-flag gated - any project with the code_owners licensed feature could set dap_powered, regardless of dap_powered_recommend_reviewers. Rejecting the value is therefore a real behaviour change for API clients, which is why this carries a changelog entry even though the UI-side feature is Beta and flag gated.
Projects already stored as dap_powered are untouched and keep working through the bespoke path.
Why openapi_v2.yaml is untouched
doc/api/openapi/_index.md:25 states it plainly:
The OpenAPI 2.0 specification (
openapi_v2.yaml) is deprecated and no longer receives updates. Use the OpenAPI 3.0 specification (openapi_v3.yaml) instead.
An earlier revision of this MR hand-edited the v2 file and asked for a second opinion on it. That was wrong on two counts: the file's own header says "auto-generated by a script, please do not edit this file directly", and per the policy above it should not be updated at all. It has been reverted to match master exactly, so dap_powered remains in the v2 enum. That is intentional and consistent with every other API change since the deprecation.
Supporting evidence, measured on pristine master at 67b89edeaf34 with no MR changes applied:
rake gitlab:openapi:v2:check_docsalready fails- regenerating produces 3205 changed lines (1677 insertions, 1528 deletions), none of which mention
reviewer_assignment_strategyordap_powered
Neither CI nor the pre-push hook gates v2: the openapi_docs hook globs {doc/api/openapi/openapi_v3.yaml,*.rb} and the only openapi references in .gitlab/ci/rules.gitlab-ci.yml are to openapi_v3.yaml. Nothing is broken there, that is just what a frozen file looks like.
openapi_v3.yaml was properly regenerated - it needed a schema hash rename (RequestBody_2913c4f9f47a -> RequestBody_c4a49d9828d1) that a hand edit would have missed. Re-verified after rebase: the pre-image hash is still current on master, and rake gitlab:openapi:v3:check_docs reports "OpenAPI v3 documentation is up to date".
GitLab Duo review findings
All three addressed:
- GET response tables were inaccurate. Correct finding. Existing rows keep the value, so
GETcan still returndap_poweredwhile only writes are rejected. The four response tables (Get single project, List all projects, List user projects, List user contributed projects) now say the attribute can also returndap_poweredfor projects configured before 19.4. - Removal history note was on the wrong table. Correct finding, verified against
ee/lib/ee/api/helpers/projects_helpers.rb:159-attrs.delete(:reviewer_assignment_strategy)unlessparams[:id].present?, and the param lives only inoptional_update_params_ee. The attribute is not settable on create at all, so the note moved from the create table to the update table, plus a new{{< history >}}entry under "Update a project". openapi_v2.yamlhand edit needs verification. Resolved by removing the change: v2 is a deprecated, frozen file that should not be updated at all. See above.
Verification
Settings page render, for a project that still holds dap_powered (local GDK, project row set to dap_powered and dap_powered_recommend_reviewers_available? true - the exact condition that previously rendered the radio group):
- section renders,
Reviewer assignment strategylegend absent, zero radio inputs - zero occurrences of
dap_poweredanywhere in the response body - only the checkbox plus its hidden
disabledcompanion field render, with nocheckedattribute
Specs - all green, 11 examples:
bundle exec rspec ee/spec/views/projects/settings/merge_requests/_reviewer_auto_assignment_settings.html.haml_spec.rb # 6 examples
bundle exec rspec ee/spec/requests/api/projects_spec.rb:2174 ee/spec/requests/api/projects_spec.rb:910 # 5 examplesRuboCop, haml-lint, markdownlint and Vale all clean. The new dap_powered -> 400 example was confirmed to fail with the production change stashed and pass with it.
Other notes
- The
dap_powered: 2enum value still exists, sobuild(:project_setting, reviewer_assignment_strategy: :dap_powered)remains valid in specs. A view spec example asserts such a project now renders an unchecked checkbox - that's the honest current behaviour and it documents what the later phase's data reset is for. - !248391 (closed) was closed - the guard it added was dropped by decision.
doc/api/projects.md:2369still saysreviewer_assignment_strategywas introduced in 19.1 while the attribute tables say 19.2. That is master's own inconsistency, untouched here.
Existing dap_powered projects
The rows, the enum value and the worker branch are all untouched, so a project already on dap_powered with the flag on keeps working exactly as before. There is one behaviour change though, and it is the only one that touches existing users.
The settings section now renders the standard Rails checkbox pair: a hidden disabled field plus an unchecked code_owners checkbox. Confirmed in the rendered page during verification:
<input name="project[project_setting_attributes][reviewer_assignment_strategy]" type="hidden" value="disabled" autocomplete="off" />
<input class="custom-control-input" type="checkbox" value="code_owners" name="project[project_setting_attributes][reviewer_assignment_strategy]" id="project_project_setting_attributes_reviewer_assignment_strategy" />The controller permits the attribute (ee/app/controllers/ee/projects/settings/merge_requests_controller.rb:38), so if the owner of a dap_powered project opens Settings > Merge requests and selects Save changes on that section without touching anything, the row flips from dap_powered to disabled and the feature is silently lost. Before this MR the radio group rendered dap_powered as the checked option, so saving preserved it.
That is the same end state Phase C (#607678 (closed)) intends, just user triggered at unpredictable times rather than done deliberately in one migration. Flagged rather than fixed: preserving the value would mean carrying a hidden field for a value the UI no longer offers, which works against the point of having a single setup path. Worth a decision from @phikai on whether the gradual reset is acceptable for Beta or Phase C should land close behind this.
Why this is not gated on the remaining Phase A issues
The epic text says Phase A must land before the setting is removed. That was written when Phase A still had two functional gaps, both since closed by !248388 (merged). Of what remains:
- #607675 (closed) is a decision to write down (which human sits in the composite identity on the trigger path), not a code dependency. It changes no line of this diff.
- #607676 (closed) is a decision about whether to add a catalog gate; its own description says it can be closed if ungated Beta availability is acceptable.
- #607907 (closed) states explicitly that it no longer blocks setting removal.
This MR also closes a foot-gun rather than opening one. ee/app/workers/merge_requests/auto_assign_reviewers_worker.rb:23-37 branches on reviewer_assignment_dap_powered? and then returns early unless dap_powered_recommend_reviewers_available?, so the code-owners else branch never runs either. A project with dap_powered set and the flag off therefore gets no reviewer assignment at all, silently. Because the REST whitelist was never flag gated, any project with the code_owners licensed feature could reach that state.
MR acceptance checklist
- Tests added for the behaviour change
- Follows the code review guidelines
- API documentation updated
- No migration or database review needed (no schema or data change in this MR)