Validate the flow-trigger goal for flows that resolve a resource

Flow triggers pointed at the Code Review foundational flow never produced a review. RunService builds a foundational flow's goal from Gitlab::UrlBuilder, and the flow binds that goal to its merge_request_iid tool input, which only accepts an integer.

The workflows API already guards this. !250117 (merged) added goal_validator_resolver, which rejects a goal the flow cannot resolve and normalises a URL to the bare IID - but only the REST endpoint called it. This applies the same validator on the trigger path.

Mention-triggered code review is now refused before dispatch rather than failing after the LLM steps have been billed. The refusal posts a note saying retrying will not help, instead of the generic retry prompt. Making it work needs #619401 (closed).

Found while rolling out merge_request_create_flow_trigger (#618743 (closed)).

Detailed context for AI agents

The failure

On a project with an Ai::FlowTrigger for event type 6 (merge_request), filter action created, whose ai_catalog_item_consumer points at the Code Review foundational flow, every run died on its first step:

Tool build_review_merge_request_context raised validation error
1 validation error for BuildReviewMergeRequestContextInput
merge_request_iid
  Input should be a valid integer, unable to parse string as an integer
  [type=int_parsing, input_value='https://gitlab.com/.../-/merge_requests/111', input_type=str]

The session was created, the "Duo Code Review has started" note was posted by the flow's service account, then the flow errored with no review comments.

Root cause

Ai::FlowTriggers::RunService#catalog_item_user_prompt resolves the goal in this order:

  1. foundational_flow.goal_templates if the flow declares one
  2. GoalTemplates::Base.default_mention_goal for mention events
  3. Gitlab::UrlBuilder.build(resource) for any other event on a foundational flow
  4. the raw user_input otherwise

Code Review declares no goal_templates, so trigger-started runs landed on branch 3 and passed a URL.

Code Review does define a goal_validator_resolver that rejects an unusable goal and normalises a URL to a bare IID, but it was only reachable through validate_goal, whose single caller was the REST endpoint at ee/lib/api/ai/duo_workflows/workflows.rb:265. The trigger path (RunService -> Ai::Catalog::Flows::ExecuteService -> Ai::DuoWorkflows::CreateWorkflowService) never called it, so nothing normalised the URL.

Why the existing entry points work

Ai::DuoWorkflows::CodeReview::ReviewMergeRequestService sets goal: merge_request.iid itself and does not go through RunService. That covers automatic Duo Code Review (auto_duo_code_review_enabled) and assigning GitLab Duo as a reviewer, which is how Code Review runs on gitlab-org/gitlab today - confirmed by inspecting live sessions there, which all carry bare integer goals. Those paths are untouched.

Scope of the bug

Not specific to the created action or to merge_request_create_flow_trigger. Every trigger entry point was affected:

  • created, approved, merged, ready and assign_reviewer hit branch 3 and sent a resource URL.
  • mention hit branch 2: default_mention_goal produces Input: <thread>\nContext: {MergeRequest IID: 123}, which fails the same int_parsing check.

That flag only made it visible, because "review a merge request when it is created" is the first thing anyone wires up.

Approach

validated_goal runs the resolved goal through foundational_flow.validate_goal before dispatch and returns the validator's response, so a normalised goal replaces the original. A flow that declares no validator gets nil back and keeps its goal untouched, which is every flow except Code Review today.

An earlier revision of this MR added an Ai::Catalog::GoalTemplates::CodeReview returning the bare IID instead. Replaced after review feedback: a goal template is another consumer of the overloaded goal, which is exactly what #619401 (closed) wants to remove, and reusing the existing validator means the trigger path cannot start a run the API would have refused. The earlier revision's claim that this option "touches every foundational flow's start path" was wrong - the validator is opt-in per flow.

No feature flag

A flag was originally proposed for this change and dropped after the maintainer asked whether it was needed. The only foundational flow that declares a goal_validator_resolver today is code_review/v1, and its validator accepts a bare IID or a merge request URL, while the flow itself resolves the goal with find_by_iid. The validator is strictly more permissive than the flow, so anything it rejects would have failed on the flow's first step anyway - the flag would have been guarding a path that cannot regress. The rollout issue #626838 (closed) has been closed.

Reporting the rejection

Both error branches of the code_review goal_validator_resolver now set reason: :invalid_goal, and Ai::Messaging::Adapters::GitlabDuoNote#error_text has a matching case. Without a reason, Ai::Messaging::Adapters::Base fell back to :execute_workflow_failed, so a mention-triggered rejection posted "Failed to start the Duo workflow. Please try again." - a retry prompt for a goal that can never validate. A reason on its own would not have helped either: an unmapped one falls through to the else branch, "Something went wrong. Please try again."

The note reads "I can't tell what you want me to act on here. Please try again in a different context." It names no resource type on purpose. The adapter serves every mention-triggered flow, not only Code Review (Notes::PostProcessService routes Duo Developer mentions on issues through it), and merge request wording would have been wrong in the case this fires in most often: the user mentions Duo on a merge request and would have been told to mention Duo on a merge request. error_text only receives the reason symbol, never the validator's message, so the mapping cannot be more specific without widening on_flow_failed and translating the validator strings.

locale/gitlab.pot was updated for the new string.

What this deliberately does not fix

Mention-triggered code review still does not produce a review. The mention goal carries the conversation, not an identifier, so the validator rejects it. Behaviour changes from "starts, bills the LLM steps, then reports The workflow completed but no response was produced" to "refused immediately". Cheaper and faster, but not a working review; that needs the structured resource pointer in #619401 (closed).

Verification

GitLab.com. While rolling out the flag on gitlab-com/create-stage/code-review-ai-experiment-playground, nine trigger-started code_review/v1 sessions across merge requests !109 (merged)-!117 (merged) all failed with a URL goal, covering non-draft, draft, conflicted-at-create, fork and five concurrent creates. On the same merge requests the auto-review sessions succeeded with a bare IID goal, isolating the difference to the goal value.

Local GDK, against real Duo Workflow Service. One trigger per entry point pointed at the Code Review consumer on gitlab-org/gitlab-test, restarting Rails between runs:

trigger before after
created (event 6) goal = MR URL, failed goal = the bare IID, finished, review posted
assign_reviewer (event 2) goal = MR URL, failed goal = the bare IID, finished, review posted
mention (event 0) goal = Input: <thread>..., failed after ~90s of LLM steps refused before dispatch, no session created

All three "before" runs failed with the same int_parsing error, differing only in the value passed.

The two runs below differed only in whether the validator ran, with no source changes between them - the flag existed at the time of testing and was flipped between the runs:

goal validated session goal outcome
no 234 http://gdk.test:3000/.../merge_requests/3280 failed
yes 233 3279 finished

The unvalidated run reproduces master's behaviour exactly.

validate_goal was also exercised directly against each goal shape the trigger path produces: a bare IID passes through unchanged, an MR URL normalises to the IID, and a mention goal is rejected with must be a merge request IID or a full merge request URL.

Specs, green locally:

  • ee/spec/services/ai/flow_triggers/run_service_spec.rb - full file, 149 examples, including the pre-existing assertions that other flows still forward a URL goal untouched
  • ee/spec/models/ai/catalog/foundational_flow_spec.rb - 124 examples
  • ee/spec/services/ai/messaging/adapters/gitlab_duo_note_spec.rb - 33 examples

Five new examples in run_service_spec.rb cover URL normalisation for a validating flow, rejection of an unresolvable goal, that the rejection carries an :invalid_goal reason, that no workflow row is created on rejection, and that a flow without a validator forwards its goal untouched. Two existing examples in foundational_flow_spec.rb are extended to assert the reason. One new row in the #deliver_error table in gitlab_duo_note_spec.rb covers the new note text. RuboCop clean.

Out of scope

  • The structured resource pointer in #619401 (closed).
  • Two findings from the same rollout, neither caused by this: conflicted-at-create merge requests reach a Recommend Reviewers run with no synced code-owner rules, so it assigns nobody; and two triggers on the same event with different composite-identity service accounts starve each other, since container.ai_flow_triggers has no ORDER BY and only the first identity links (see https://gitlab.com/gitlab-org/gitlab/-/issues/612038).
Edited by Marc Shaw

Merge request reports

Loading
Loading