Avoid synchronous mergeability check on widget GET requests
widget.jsonis the last of the 10 GET endpoints from #628132 that still writesmerge_statusduring the request. It callsmergeable?, directly and through the auto-merge strategies, and that runs the mergeability check synchronously.- Behind the new
enqueue_widget_mergeability_checkflag, the widget enqueues the check instead, like the other 9 endpoints since !256999 (merged). widget.jsonno longer exposesmergeable. The frontend now derives merge-immediately availability fromdetailedMergeStatusinstead, so this needs a frontend review.
Detailed context for AI agents
Background: INC-12449 showed that GET endpoints writing to the primary database are dangerous. The detect_writes_on_get analyzer allowlists the affected endpoints in lib/gitlab/database/query_analyzers/prevent_writes_on_get.rb until each is fixed. !256999 (merged) (merged) moved the checking status write out of the GET request and into MergeRequestMergeabilityCheckWorker, behind the flag mark_mergeability_checking_in_worker (rollout #630354). That removed the write from 9 of the 10 endpoints named in the work item.
Remaining problem: Projects::MergeRequests::ContentController#widget still writes. MergeRequestPollWidgetEntity calls MergeRequest#mergeable? in two places:
- the
mergeablefield, directly. - the
available_auto_merge_strategiesfield, viaAutoMergeService#available_strategies, which calls the merge-when-checks-pass and merge-train strategies'availability_details, which in turn callmergeable?.
Either path runs the mergeability check synchronously: it writes the verdict (can_be_merged / cannot_be_merged), takes a row lock, and reloads the merge head diff, all inside the GET request.
Fix: MergeRequest#with_async_mergeability_check { ... } sets an internal flag for the duration of the block, and mergeable? passes async: !!@async_mergeability_check to check_mergeability. ContentController#widget calls merge_request.with_async_mergeability_check { serializer(MergeRequestPollWidgetEntity) } when the new flag enqueue_widget_mergeability_check is on (type gitlab_com_derisk, default off, actor = project). There is no public writer, so a caller cannot leave async mode switched on by accident. This came from reviewer feedback.
Alternative rejected: threading an explicit async argument through AutoMergeService and the CE/EE strategy availability_details overrides (about 7 files). Rejected in favor of the block approach to keep the blast radius small.
Frontend change: a new commit removes the mergeable field from MergeRequestPollWidgetEntity (widget.json). Its only consumer was isMergeAllowed in app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js, which gates the "Merge immediately" dropdown in the ready-to-merge state. isMergeAllowed is now a getter that returns detailedMergeStatus === 'MERGEABLE'. detailedMergeStatus already comes from the GraphQL state query and the mergeRequestMergeStatusUpdated subscription, and DetailedMergeStatusService returns :mergeable only when the same set of checks that mergeable? runs all pass, so the value is equivalent.
Reasons: (a) with the flag on, widget.json reported mergeable: false for an unchecked MR until the worker wrote the verdict, and the frontend only refetched widget.json on the state poll, which the merge_widget_stop_polling flag (rollout #579437) disables, so the "Merge immediately" option could stay hidden until reload. Deriving it from the subscription fixes that. (b) it removes one of the two mergeable? callers in the entity. The remaining caller is available_auto_merge_strategies, which is why the block in the controller is still needed.
Also removed: the mergeable property from the JSON schema fixture spec/fixtures/api/schemas/entities/merge_request_poll_widget.json, the entity spec for #mergeable, and the controller spec now expects check_mergeability once instead of twice. A new store spec covers the getter for MERGEABLE and non-MERGEABLE statuses.
This frontend change is not behind the feature flag and needs a frontend review.
Flag independence: this flag works on its own, but a fully write-free widget GET needs both enqueue_widget_mergeability_check and mark_mergeability_checking_in_worker on. With only the new flag on, async_execute still writes the checking status inside the request.
UX effect with the flag on: while an MR is unchecked, the widget may offer auto-merge in place of an immediate merge until the worker writes the verdict. That write fires the mergeRequestMergeStatusUpdated GraphQL subscription, which updates detailedMergeStatus, availableAutoMergeStrategies, autoMergeEnabled, and commitCount in the store. With the frontend change in this MR, isMergeAllowed follows detailedMergeStatus too, so no widget.json refetch is needed.
Known gap: the GraphQL subscription is only set up after the state query first returns. If the worker finishes before that, the event is missed, and the auto-merge strategies from the initial widget.json stay until the next poll or reload. This gap exists on master today for the strategies and is not made worse by this MR. It is out of scope here.
Verification:
- New request spec for the GET widget endpoint: flag on keeps status
uncheckedand enqueues the worker; flag off lets the status settle tocan_be_mergedduring the request. New model spec formergeable?inside the block. - Throwaway request spec exercising all 10 endpoints from the work item against an
uncheckedMR: with both flags on, 0merge_statusUPDATEs and 0 row locks on every endpoint, and the widget endpoint makes no writes at all. With flags off, the old writes remain, confirming the spec exercises the write path. The new widget request spec fails without the controller change. - Jest specs:
mr_widget_store_spec(CE and EE),mr_widget_ready_to_merge_spec(CE and EE), all passing. - RSpec:
spec/serializers/merge_request_poll_widget_entity_spec.rb,spec/controllers/projects/merge_requests/content_controller_spec.rb,spec/requests/projects/merge_requests/content_spec.rb, 35 examples, 0 failures.
Rollout: #630708
Follow-up: once both flags are default-on, remove the merge request endpoints from ALLOWED_ENDPOINTS in lib/gitlab/database/query_analyzers/prevent_writes_on_get.rb.
No changelog entry, since the flag is default off.