Resolve foundational flow additional context and goal in DAP trigger path

The DAP trigger path cannot actually run the Recommend Reviewers flow. Two gaps: Ai::FlowTriggers::RunService never calls the flow's additional_context_resolver, so the flow receives no reviewer data to reason over; and it sends the merge request URL as the goal where the flow config binds context:goal as a bare merge_request_iid. This MR closes both, adds a goal template, and guards the resolver to MergeRequest.

Neither gap is latent — nothing is feature-flagged, so any group with beta flows enabled can configure a Merge request > Marked ready trigger today and get empty runs. The bespoke dap_powered path is untouched; removing it is later epic phases.

  • Closes #607673 (closed) and #607674 (closed)
  • Epic: gitlab-org#23019
  • Specs: the four touched spec files pass — 376 examples, 0 failures. RuboCop clean.
  • End-to-end verification on the playground project — to be done together after review, not attempted from CI
Detailed context for AI agents

The two gaps

Gap 1 — no additional context. The flow needs reviewer_data (current reviewers, approval rules, ranked candidates) to do anything useful. That payload is produced by the flow item's additional_context_resolver, but Ai::FlowTriggers::RunService#additional_context only built context for Ci::Pipeline resources and never called the resolver. A trigger-driven run therefore received nothing to reason over.

Gap 2 — wrong goal. The trigger path sent Gitlab::UrlBuilder.build(resource) (the MR URL) as the goal, but the flow config binds context:goal as merge_request_iid in every component, so the flow needs the bare iid. The model may have salvaged the iid from the URL string, which left create_merge_request_note / update_merge_request tool calls relying on LLM parsing where the bespoke path passed exact data.

Why this is safe and small

The delivery mechanism already exists end to end. Both the bespoke path and the trigger path converge on Ai::DuoWorkflows::StartWorkflowService, which serializes additional_context into the DUO_WORKFLOW_ADDITIONAL_CONTEXT_CONTENT workload variable — the bespoke path proves it in production today. The only missing pieces were the resolver call and a goal template.

FoundationalFlow#resolve_additional_context_for returns [] when a flow declares no resolver, so every other foundational flow is a no-op. Pipeline additional context is byte-for-byte unchanged, just extracted into pipeline_additional_context.

Changes

  1. RunService#additional_context now merges pipeline context (when the resource is a pipeline) with the flow's resolved context (when the flow declares a resolver), returning nil when neither applies.
  2. New Ai::Catalog::GoalTemplates::RecommendReviewers, declared on recommend_reviewers/v1, returns the bare iid for every event type when the resource is a merge request. It deliberately never returns user_input — for mention events that carries conversation text, not the iid, and this flow has no conversational path. For any other resource type it returns nil rather than raising: Flows::ExecuteService falls back to the flow description, whereas raising would abort the unrescued mention loop in Notes::PostProcessService and re-run sibling triggers on NewNoteWorker retry.
  3. Both the goal template and the recommend_reviewers/v1 additional_context_resolver are guarded on MergeRequest. This flow declares no supported_events, which means any event type is allowed, so a mention trigger could fire it for an Issue. Issue has no reviewers, so the builder would raise NoMethodError in the trigger worker; and an issue iid bound as context:goal would silently point the flow at an unrelated merge request in the same project instead of failing. Not part of the original issue scope; found while verifying the change and fixed here because this MR is what makes the path reachable.
  4. Latent bug fixed in passing: StartWorkflowService#serialized_flow_additional_context filtered reserved categories with envelope[:Category] (symbol key) while every producer builds string-keyed "Category" envelopes, so the guard never dropped anything. It now matches both key types, and no longer mutates the caller-provided array (the previous delete_if + << mutated @params[:additional_context] in place).

Spec coverage

The other new specs exercise RunService#additional_context and #catalog_item_user_prompt directly, which is the convention already in that file but does not prove either value reaches the flow. run_service_spec therefore also asserts that Ai::Catalog::Flows::ExecuteService receives the bare iid as user_prompt and the reviewer_data envelope as additional_context for a recommend_reviewers/v1 trigger run. Reverting both source changes was confirmed to fail that assertion on both halves — the goal arrives as the merge request URL and additional_context is nil — so it is load-bearing for the whole change. The two start_workflow_service_spec additions were checked the same way.

No gitlab-ai-gateway change is needed — verified against recommend_reviewers/1.0.0.yml, which reads only context:goal, context:project_id, and context:inputs.reviewer_data. The merge_request_id param that start_catalog_workflow passes separately is never consumed by the flow config.

No feature flag

Deliberate epic decision: Phase A is revertible by MR revert and the bespoke path is untouched. See the risk table in the epic.

Coordination note

Unmerged commit 769b3540cf38 (Paulo Martins, security-review custom instructions) adds another one-off special case to #additional_context for security_review/v1. The generic resolver approach subsumes it — security_review/v1 can declare an additional_context_resolver calling CodeReview::CustomInstructionsContextBuilder instead of stacking a second special case. Worth coordinating before both land; nothing in this MR blocks that change.

Prior art

!242137 (closed) — never merged; same idea, via a hook that predates the additional_context_resolver now on master.

Out of scope

  • The bespoke path (AutoAssignReviewersWorker, RecommendReviewers::ExecuteService, the dap_powered enum/setting/API) — later epic phases
  • Re-fire suppression for repeated ready events — #607907 (closed). The reviewers-present skip that issue also used to cover was dropped by decision: runs are wanted even when reviewers exist.
  • Catalog feature-flag gating — #607676 (closed)

Testing

bundle exec rspec ee/spec/services/ai/flow_triggers/run_service_spec.rb \
  ee/spec/models/ai/catalog/goal_templates/ \
  ee/spec/models/ai/catalog/foundational_flow_spec.rb \
  ee/spec/services/ai/duo_workflows/start_workflow_service_spec.rb

437 examples, 0 failures. RuboCop clean on all changed files.

Each new behavioural example was verified to fail without the corresponding production change (the resolver, goal-template, and string-key examples all fail on master; the no-op examples pass both ways by design, since they assert unchanged behaviour).

MR acceptance checklist

  • Tests added for the new behaviour and for the unchanged paths
  • Follows the code review guidelines
  • No user-facing documentation change needed (no UI, API, or configuration surface changes)
  • No migration or database review needed
Edited by Marc Shaw

Merge request reports

Loading
Loading