Remove the unused policy_violation field from the cached MR widget

What does this MR do and why?

This removes the unused policy_violation field from the cached MR widget response and the EE code that computes it. The frontend stopped reading the value in September 2022, and the widget state it used to drive became unreachable in March 2024 when the old policy mergeability check was removed. Computing it still cost real work on every poll: about 220,000 ReactiveCachingWorker jobs a day, counted against Code Review's Sidekiq apdex even though the underlying work belongs to Software Composition Analysis. This is an internal endpoint used only by the GitLab frontend, so dropping the key is not a public API change, but the deleted code lives in Software Composition Analysis's domain and should be flagged to that group.

Detailed context for AI agents

Root cause

Projects::MergeRequests::ContentController#cached_widget is the ETag-cached half of the MR widget poll, served through MergeRequestPollCachedWidgetEntity. The EE extension added a policy_violation field, backed by EE::MergeRequest#has_denied_policies?, which called compare_reports(::Ci::CompareLicenseScanningReportsService). That method goes through ReactiveCaching, so every read renews the cache's alive key, a cold cache enqueues a ReactiveCachingWorker job, and a ReactiveCaching::LowUrgencyWorker keeps refreshing the cache every minute for its 10 minute lifetime after the last poll. It also ran DB queries on every cache miss for diff_head_pipeline, Gitlab::LicenseScanning.scanner_for_pipeline(...).results_available?, and an approval rule lookup.

Production Kibana over 24 hours showed 220,363 ReactiveCachingWorker jobs with meta.feature_category = code_review_workflow, caller Projects::MergeRequests::ContentController#cached_widget, and related_class = MergeRequest. That is about 85% of the Code Review slice of this worker. Because the worker is not_owned, this counts against Code Review's Sidekiq apdex for a computation that belongs to Software Composition Analysis.

The field was added in July 2020 (aa3d11cf788b, "Prevent merge if denied licenses are found") when denied licenses were a merge blocker rendered as a widget state.

Evidence the field is unused

  • Frontend: the EE store copied the value once, this.policyViolation = data.policy_violation, in ee/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js (line 60). Nothing ever read this.policyViolation.
  • ee/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js mapped a policyViolation state to the mr-widget-policy-violation component ("Merge blocked: denied licenses must be removed."). Neither the CE nor the EE get_state_key.js ever returned that state, so the component could never mount.
  • The store value was last read in get_state_key.js until September 2022, when "Update merge widget to use detailed merge status field" (dcd78ef7ae95) replaced if (this.policyViolation) with detailedMergeStatus === POLICIES_DENIED.
  • The only code path that produced that state used detailedMergeStatus === POLICIES_DENIED, from the mergeability check, not from this field. That path was removed in March 2024 by "Finish removing the policy mergeability check" (!146278 (merged)). Denied-license blocking is now handled by approval rules and the security_policy_violations merge check.
  • A repository-wide search on master for policy_violation, policyViolation, has_denied_policies, has_approved_license_check, license_scanning_feature_available, the component name and its translated string found only the code removed here plus two stale allow(...).to receive(:has_denied_policies?) stubs in ee/spec/controllers/ee/projects/merge_requests_controller_spec.rb, which are removed too.
  • Backend: has_denied_policies? had exactly one caller, the entity. has_approved_license_check? and license_scanning_feature_available? each had exactly one caller, has_denied_policies?.

What is removed

  • ee/app/serializers/ee/merge_request_poll_cached_widget_entity.rb: the policy_violation exposure.
  • ee/app/models/ee/merge_request.rb: has_denied_policies?, has_approved_license_check?, license_scanning_feature_available?.
  • ee/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js: the policyViolation assignment.
  • ee/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js: the stateKey.policyViolation export (now empty, so removed) and the policyViolation component mapping. geoSecondaryNode stays.
  • ee/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue: the import and registration of MrWidgetPolicyViolation.
  • Deleted: ee/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_policy_violation.vue and its Jest spec.
  • ee/spec/models/merge_request_spec.rb: the #has_denied_policies? and #license_scanning_feature_available? describe blocks.
  • ee/spec/serializers/ee/merge_request_poll_cached_widget_entity_spec.rb: the "includes policy violation status" example.
  • ee/spec/fixtures/api/schemas/entities/merge_request_poll_cached_widget.json: the policy_violation property; the EE schema now only references the CE schema.
  • locale/gitlab.pot: the "Merge blocked: denied licenses must be removed." string, regenerated with tooling/bin/gettext_extractor.

Verification

  • bundle exec rspec ee/spec/serializers/ee/merge_request_poll_cached_widget_entity_spec.rb: 19 examples, 0 failures.
  • bundle exec rspec ee/spec/controllers/ee/projects/merge_requests/content_controller_spec.rb (validates the cached_widget JSON against the schema): 1 example, 0 failures.
  • Jest: ee/spec/frontend/vue_merge_request_widget/mr_widget_options_spec.js and ee/spec/frontend/vue_merge_request_widget/stores/mr_widget_store_spec.js: 32 passed, 2 skipped (pre-existing skips).
  • bundle exec rspec ee/spec/controllers/ee/projects/merge_requests_controller_spec.rb: 105 examples, 0 failures.
  • A throwaway request spec called GET /:project/-/merge_requests/:iid/cached_widget.json on an EE merge request with CycloneDX reports, a head pipeline and license scanning licensed, listing every Sidekiq job enqueued. On master it enqueues exactly one job: ReactiveCachingWorker["MergeRequest", id, "Ci::CompareLicenseScanningReportsService"] with meta.feature_category = code_review_workflow. On this branch it enqueues nothing, still returns 200, and the JSON no longer contains policy_violation.
  • RuboCop, ESLint, and Prettier are clean on the changed files. The pre-push lefthook (eslint, rubocop, dependency cruiser, unused-code-linter, jsonlint) passed.

Alternatives considered and rejected

  • Re-attributing the job to Software Composition Analysis by wrapping the enqueue in a feature category context. Tried in an earlier draft, !256681 (closed) (now closed). Rejected because the work has no consumer, so moving it between groups would still burn about 220,000 jobs a day for nothing.
  • Hardcoding a feature category on the worker class, !254437 (closed). Rejected because it overrides caller attribution for every other consumer of the shared worker.

Out of scope

  • The SAST report query attribution, handled by the companion MR !256683 (merged).
  • EE::Group#billable_members_count seat count jobs enqueued from MR pages, about 3% of the same slice.

References

Screenshots or screen recordings

No visible UI change. The removed component could not render.

How to set up and validate locally

  1. Open an MR in a project with license scanning results on the head pipeline and confirm the widget still renders and polls. In the network tab, cached_widget.json returns 200 and no longer contains policy_violation.
  2. Check the Sidekiq log while polling. No ReactiveCachingWorker jobs with meta.related_class=MergeRequest and caller cached_widget are enqueued.
  3. Run the two RSpec files and two Jest files listed under Verification above.

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.

Edited by Marc Shaw

Merge request reports

Loading
Loading