Bail out of a flow trigger run for an unsupported resource type
Ai::FlowTriggers::RunService would happily start a flow for a resource type the flow cannot handle. recommend_reviewers/v1 declares no supported_events, so an @duo mention on an issue started a workflow whose goal was the flow's description sentence bound to merge_request_iid - the run then did nothing.
Foundational flows can now declare supported_resource_types, and RunService bails with a validation error before any workflow is created when the trigger resource is not one of them. A nil declaration means "no restriction", so only recommend_reviewers/v1 changes behaviour.
Behind the bail_flow_trigger_on_unsupported_resource feature flag, disabled by default.
Related to #612121 (closed)
Detailed context for AI agents
Why the existing attributes did not work
Ai::Catalog::FoundationalFlow had no notion of an accepted resource type, and the two adjacent attributes are the wrong axis:
supported_eventsis event-shaped. It isnilforrecommend_reviewers/v1, andnilmeans any event is allowed (Ai::FlowTrigger#supported_events_match_foundational_flow). Narrowing it to merge-request-ish events would still be wrong - amentioncan land on an issue or a merge request, so the event does not tell you the resource.preconditionis aGitlab::FilterEvaluatorfilter run against the webhook payload. Expressing "the resource must be aMergeRequest" there means one rule per payload shape the flow can receive, kept in sync by hand.
What changed
| File | Change |
|---|---|
ee/app/models/ai/catalog/foundational_flow/attributes.rb |
New supported_resource_types attribute. nil means no restriction, [] means no trigger can ever start the flow. |
ee/app/models/ai/catalog/foundational_flow.rb |
#supports_resource?(resource) - true when the attribute is nil, otherwise is_a? against each declared class. |
ee/app/services/ai/flow_triggers/run_service.rb |
Guard as the first check in #validation_error, returning reason: :unsupported_resource_type. |
ee/app/models/ai/catalog/foundational_flow/definitions/recommend_reviewers.rb |
Declares supported_resource_types: [::MergeRequest]. |
ee/app/services/ai/messaging/adapters/gitlab_duo_note.rb |
#error_text branch for the new reason. |
Where the guard sits and why
RunService#validation_error is evaluated inside execute_flow, before start_catalog_workflow / run_workload, so nothing is created. The check runs ahead of the autonomous_trigger? branch: whether a flow can handle a resource is independent of whether a human or a service account pulled the trigger.
Both mention paths handle an error ServiceResponse without raising, so the mention loop in Notes::PostProcessService stays intact and sibling triggers are not re-run on NewNoteWorker retry:
- Adapter path (all foundational flows, including this one):
route_mention_through_adapter?sends them throughAi::Messaging::Adapters::GitlabDuoNote.with_lifecycle_hookscallson_flow_failedwithresponse.reason. The issue description saidCreateNoteServicehandles this - that is no longer the path foundational flows take, which is whyerror_textneeded the new branch. Without it the user would have seen "Something went wrong. Please try again." CreateNoteServicepath (non-foundational catalog flows):mark_failed(note, response.message)flips the progress note to the error text.
Non-mention callers (EE::IssuableBaseService#execute_flow_triggers, Ai::FlowTriggers::EventTriggerService, the CloudEventsFlowTriggerWorker concern, Ai::DailyFlowOnPushWorker) discard the response, so the bail is silent there - correct, since there is no thread to reply into.
Feature flag
bail_flow_trigger_on_unsupported_resource, gitlab_com_derisk, project actor, default off. Rollout issue: #624218
Nothing outside the flag can execute or fail. Two additions sit outside it, and neither is reachable with the flag off:
- The
supported_resource_typesattribute,#supports_resource?and the[::MergeRequest]declaration are inert data whose only reader is the gated#supported_resource?. Nothing serialises the attribute set -SeedFoundationalFlowsServiceassigns an explicit column list, and there is no GraphQL, REST or serializer exposure ofFoundationalFlowattributes. - The adapter's
error_textbranch only fires on:unsupported_resource_type, and the gated guard is the only producer of that reason.
An earlier revision validated the declaration shape in the model. That is now a spec instead, because the validation was the one thing the flag could not cover and its failure mode was worse than no check at all: load_items! is lazy rather than boot-time, and it writes into storage as it iterates, so an invalid definition raises on first access and then leaves storage non-empty. Every later access skips loading and silently returns a truncated catalog - measured at 7 of 13 flows on a GDK, with recommend_reviewers/v1 and five unrelated flows simply absent. The spec catches the same mistake in CI and names the offending flow.
nil resource
A flow that declares supported_resource_types also rejects resource: nil, because every such flow reads the resource to build its goal - GoalTemplates::RecommendReviewers.resolve raises ArgumentError on nil today. Three callers pass nil, but none of them target a flow that declares the attribute.
The two pre-existing MergeRequest guards
Added by !248388 (merged) and both kept:
GoalTemplates::RecommendReviewers.resolve- only reachable fromRunService, so it is dead while the flag is on. Kept as the backstop for the flag-off path; comment updated to say so.additional_context_resolver-resolve_additional_context_foris also called fromAi::DuoWorkflows::CreateAndStartWorkflowService, whichRunServicedoes not gate, so this one is still load-bearing.
Verification
Run locally, all green:
ee/spec/services/ai/flow_triggers/run_service_spec.rb- 148 examplesee/spec/models/ai/catalog/foundational_flow_spec.rb- 116 examplesee/spec/services/ai/messaging/adapters/gitlab_duo_note_spec.rb,ee/spec/models/ai/catalog/goal_templates/recommend_reviewers_spec.rb- 39 examplesee/spec/models/ai/flow_trigger_spec.rb- 114 examplesee/spec/services/ee/notes/post_process_service_spec.rb- 42 examples
New coverage: unsupported resource does not reach Ai::Catalog::Flows::ExecuteService and creates no workflow; the error message and :unsupported_resource_type reason; nil resource; a supported resource still runs; and the flag-off path still runs the unsupported resource.
Manual verification on a GDK
The flag was toggled for real with Feature.enable / Feature.disable in separate processes (no stub_feature_flags), against a real group, project with a repository, AI catalog item, service account, parent and child item consumers, Ai::FlowTrigger, plus a real Issue and MergeRequest:
| resource | flag off | flag on |
|---|---|---|
Issue |
supported_resource? true, validation_error nil, reaches ExecuteService |
false, "cannot be triggered for a Issue resource" / :unsupported_resource_type, 0 workflow rows |
MergeRequest |
true, nil, reaches ExecuteService |
identical - true, nil |
nil |
true, nil, then raises ArgumentError: resource must not be nil |
false, "cannot be triggered for a missing resource" |
The MergeRequest row is byte-identical in both states, so the guard only filters unsupported resources. The nil row also confirms the flag converts a raise into a clean ServiceResponse. Ai::DuoWorkflow.available? is false on a stock GDK, so both states stop at the same Duo permission check downstream of the guard and no run reaches Duo Workflow Service.
One pre-existing example from !248388 (merged) (run_service_spec.rb, "with a template that declines the resource type") asserted that the goal template declines gracefully rather than raising. The guard now stops that run before the template is reached, so the example was moved under stub_feature_flags(bail_flow_trigger_on_unsupported_resource: false) - which is exactly the backstop it was written to cover.
Out of scope
- Declaring
supported_resource_typeson any other flow.fix_pipeline/v1is the obvious next candidate ([::Ci::Pipeline]), but it already constrains itself throughsupported_eventsand aprecondition, so it gains little and would change behaviour for a flow that is mid-rollout. - Removing the goal-template backstop. That belongs in the flag cleanup MR.
Epic: gitlab-org#23019