DAP trigger platform: suppress duplicate flow runs on repeated ready events

Related to &23019. No longer blocks the removal of the project setting — see the scope note below.

Problem

Every draft > ready transition publishes MergeRequests::ReadyEvent, and there is no rate limiting or dedup anywhere in the trigger platform. Verified: no ApplicationRateLimiter and no exclusive lease in ee/app/services/ai/flow_triggers/, ee/app/services/ai/catalog/, or the trigger workers — only Sidekiq concurrency_limit -> { 100 } and idempotent!, neither of which dedups distinct events.

So each draft ⇄ ready toggle fires a full workflow execution. For Recommend Reviewers that means:

  • the cost of a complete flow run;
  • a service-account progress note posted on the merge request every time (Ai::FlowTriggers::CreateNoteService, run_service.rb:46-48);
  • repeated reviewer churn.

The bespoke MergeRequests::AutoAssignReviewersWorker bailed before starting any of that. ReviewerAssignment::AssignService documents that the guards live only in the worker and that any new caller must reimplement them (ee/app/services/merge_requests/reviewer_assignment/assign_service.rb:44-51).

This is not specific to Recommend Reviewers. Any flow wired to a repeatable event has the same exposure, which makes it a missing platform feature rather than a per-flow concern.

Scope: this issue is now re-fire suppression only

This issue originally carried a second guard — skip the run when the merge request already has human reviewers. That guard was deliberately dropped. Runs are wanted even when reviewers exist, so the precondition no longer reflects intended behaviour. The implementation was removed and !248391 (closed) was closed.

The follow-on from that decision is tracked separately: the update_merge_request tool reaches PUT /projects/:id/merge_requests/:iid, where reviewer_ids replaces the reviewer list rather than appending to it, so a run against a merge request with hand-picked reviewers can overwrite them. That tool lives in duo-workflow-service; this repository only carries its registry entry.

Implementation

There is one solid precedent to copy: DependencyManagement::SecurityUpdate::TriggerResolveDependencyBumpWorkflowWorker#skip_due_to_workflow_state? (lines 30-31, 67-75) — skip when an active workflow of the same definition already exists for the merge request, and cap total runs (MAX_ITERATIONS = 2).

All the pieces already exist on the model:

  • Ai::DuoWorkflows::Workflow.for_merge_request (workflow.rb:84)
  • .with_workflow_definition (workflow.rb:101)
  • #status_group / GROUPED_STATUSES[:active] = [:created, :running] (workflow.rb:172-179, 649)

Implement it generically rather than as a Recommend Reviewers special case, either in the per-trigger loop (cloud_events_flow_trigger_worker.rb:68-74, before RunService.new at line 78) or early in RunService#execute: skip when an active workflow with the trigger's workflow definition already exists for the resource. A run-count cap per resource is optional and can be a follow-up.

Alternative: accept and document

Viable for this guard, unlike the dropped one — the exposure is cost and note noise rather than a correctness problem. Needs product sign-off.

Specs

  • Ready worker / shared concern: ee/spec/workers/ai/catalog/flows/execute_merge_request_ready_workflow_triggers_worker_spec.rb, which uses the 'a cloud events flow trigger worker' shared examples at ee/spec/support/shared_examples/workers/ai/cloud_events_flow_trigger_worker_shared_examples.rb.
  • If the guard lands in RunService, add coverage to ee/spec/services/ai/flow_triggers/run_service_spec.rb.

Acceptance criteria

  • A second ready event while a workflow of the same definition is active for the same resource does not start a duplicate run, or the cost is explicitly accepted and documented with product sign-off
  • The behaviour applies platform-wide, not only to recommend_reviewers/v1
  • Decision recorded on this issue
Edited by 🤖 GitLab Bot 🤖