From fc50522399c3f1b678ef7e4de691d4df4d1f32cb Mon Sep 17 00:00:00 2001 From: fdegier Date: Fri, 5 Dec 2025 15:27:44 +0100 Subject: [PATCH 1/3] Add Foundational Flow triggers Changelog: added EE: true --- .../ai/duo_workflows/workflow_definition.rb | 21 +++- .../flows/sync_foundational_flows_service.rb | 110 ++++++++++++++++-- .../sync_foundational_flows_service_spec.rb | 83 ++++++++++++- 3 files changed, 196 insertions(+), 18 deletions(-) diff --git a/ee/app/models/ai/duo_workflows/workflow_definition.rb b/ee/app/models/ai/duo_workflows/workflow_definition.rb index 7fd0ddab38bc46..fe0f9885fb26ea 100644 --- a/ee/app/models/ai/duo_workflows/workflow_definition.rb +++ b/ee/app/models/ai/duo_workflows/workflow_definition.rb @@ -17,7 +17,8 @@ class WorkflowDefinition ::Ai::DuoWorkflows::Workflow::AgentPrivileges::READ_WRITE_GITLAB, ::Ai::DuoWorkflows::Workflow::AgentPrivileges::RUN_COMMANDS, ::Ai::DuoWorkflows::Workflow::AgentPrivileges::USE_GIT - ] + ], + triggers: [:review] }, { id: 2, @@ -31,7 +32,8 @@ class WorkflowDefinition ::Ai::DuoWorkflows::Workflow::AgentPrivileges::RUN_COMMANDS, ::Ai::DuoWorkflows::Workflow::AgentPrivileges::USE_GIT ], - environment: "web" + environment: "web", + triggers: [] }, { id: 3, @@ -45,7 +47,20 @@ class WorkflowDefinition ::Ai::DuoWorkflows::Workflow::AgentPrivileges::RUN_COMMANDS, ::Ai::DuoWorkflows::Workflow::AgentPrivileges::USE_GIT ], - environment: "web" + environment: "web", + triggers: [] + }, + { + id: 4, + name: "developer/v1", + pre_approved_agent_privileges: [ + ::Ai::DuoWorkflows::Workflow::AgentPrivileges::READ_WRITE_FILES, + ::Ai::DuoWorkflows::Workflow::AgentPrivileges::READ_ONLY_GITLAB, + ::Ai::DuoWorkflows::Workflow::AgentPrivileges::READ_WRITE_GITLAB, + ::Ai::DuoWorkflows::Workflow::AgentPrivileges::USE_GIT + ], + environment: "web", + triggers: [:assign] } ].freeze diff --git a/ee/app/services/ai/catalog/flows/sync_foundational_flows_service.rb b/ee/app/services/ai/catalog/flows/sync_foundational_flows_service.rb index 55a0c937988014..fbb66564166301 100644 --- a/ee/app/services/ai/catalog/flows/sync_foundational_flows_service.rb +++ b/ee/app/services/ai/catalog/flows/sync_foundational_flows_service.rb @@ -52,20 +52,36 @@ def sync_flows end def create_consumer_for_catalog_item(item) - params = { item: item } + return unless should_create_consumer?(item) + return unless authorized_to_create_consumer?(item) - if container.is_a?(Project) && item.flow? - parent_consumer = container.root_ancestor.configured_ai_catalog_items.for_item(item.id).first + result = create_or_find_consumer(item) + consumer = extract_consumer_from_result(result, item) - return if parent_consumer.nil? + create_trigger_if_needed(consumer, item) if consumer - params[:parent_item_consumer] = parent_consumer - end + result + end - if current_user - return unless Ability.allowed?(current_user, :admin_ai_catalog_item_consumer, container) - return unless Ability.allowed?(current_user, :read_ai_catalog_item, item) - end + def should_create_consumer?(item) + return true unless container.is_a?(Project) + return true unless item.flow? || item.third_party_flow? + + parent_consumer = container.root_ancestor.configured_ai_catalog_items + .find { |c| c.ai_catalog_item_id == item.id } + + parent_consumer.present? + end + + def authorized_to_create_consumer?(item) + return true unless current_user + + Ability.allowed?(current_user, :admin_ai_catalog_item_consumer, container) && + Ability.allowed?(current_user, :read_ai_catalog_item, item) + end + + def create_or_find_consumer(item) + params = build_consumer_params(item) ::Ai::Catalog::ItemConsumers::CreateService.new( container: container, @@ -74,6 +90,76 @@ def create_consumer_for_catalog_item(item) ).execute end + def build_consumer_params(item) + params = { item: item } + + if container.is_a?(Project) && (item.flow? || item.third_party_flow?) + parent_consumer = container.namespace.configured_ai_catalog_items + .find { |c| c.ai_catalog_item_id == item.id } + + params[:parent_item_consumer] = parent_consumer if parent_consumer + end + + params + end + + def extract_consumer_from_result(result, item) + return result.payload[:item_consumer] if result.success? + return find_existing_consumer(item) if item_already_configured?(result) + + nil + end + + def item_already_configured?(result) + result.error? && result.message.include?("Item already configured") + end + + def find_existing_consumer(item) + container.configured_ai_catalog_items.find { |c| c.ai_catalog_item_id == item.id } + end + + def create_trigger_if_needed(consumer, item) + return unless container.is_a?(Project) + return unless item&.foundational_flow_reference.present? # Only create trigger for foundational flows + + create_trigger_for_consumer(consumer, item) + end + + def create_trigger_for_consumer(consumer, item) + service_account = extract_service_account(consumer) + return unless service_account + + trigger_params = build_trigger_params(service_account, consumer, item) + + ::Ai::FlowTriggers::CreateService.new( + project: container, + current_user: current_user + ).execute(trigger_params) + rescue StandardError => e + ::Gitlab::ErrorTracking.track_exception( + e, + item_id: item.id, + project_id: container.id + ) + end + + def extract_service_account(consumer) + if consumer.project.present? + consumer.parent_item_consumer&.service_account + else + consumer.service_account + end + end + + def build_trigger_params(service_account, consumer, item) + { + user_id: service_account.id, + description: "Foundational flow trigger for #{item.name}", + ai_catalog_item_consumer_id: consumer.id, + event_types: default_event_types + } + end + def remove_consumers_not_in(catalog_item_ids) ids_to_remove = foundational_flow_ids - catalog_item_ids @@ -87,6 +173,10 @@ def remove_all_flows def foundational_flow_ids Item.foundational_flow_ids end + + def default_event_types + Ai::FlowTrigger::EVENT_TYPES.values + end end end end diff --git a/ee/spec/services/ai/catalog/flows/sync_foundational_flows_service_spec.rb b/ee/spec/services/ai/catalog/flows/sync_foundational_flows_service_spec.rb index c110270663a4bf..ed466242838f90 100644 --- a/ee/spec/services/ai/catalog/flows/sync_foundational_flows_service_spec.rb +++ b/ee/spec/services/ai/catalog/flows/sync_foundational_flows_service_spec.rb @@ -40,7 +40,8 @@ allow(Ability).to receive(:allowed?).and_return(true) create_service = instance_double(Ai::Catalog::ItemConsumers::CreateService) - allow(create_service).to receive(:execute).and_return(ServiceResponse.success) + allow(create_service).to receive(:execute).and_return( + ServiceResponse.success(payload: { item_consumer: build(:ai_catalog_item_consumer) })) allow(Ai::Catalog::ItemConsumers::CreateService).to receive(:new).and_return(create_service) service.execute @@ -79,7 +80,8 @@ allow(container).to receive(:enabled_flow_catalog_item_ids).and_return([flow1.id]) create_service = instance_double(Ai::Catalog::ItemConsumers::CreateService) - allow(create_service).to receive(:execute).and_return(ServiceResponse.success) + allow(create_service).to receive(:execute).and_return( + ServiceResponse.success(payload: { item_consumer: parent_consumer })) expect(Ai::Catalog::ItemConsumers::CreateService).to receive(:new) .with( @@ -102,6 +104,55 @@ end end + context 'when trigger creation' do + let(:container) { project } + + it 'does not create triggers for group-level consumers' do + allow(container).to receive(:enabled_flow_catalog_item_ids).and_return([flow1.id]) + allow(Ability).to receive(:allowed?).and_return(true) + + create_service = instance_double(Ai::Catalog::ItemConsumers::CreateService) + group_consumer = build(:ai_catalog_item_consumer, group: group, item: flow1) + allow(create_service).to receive(:execute).and_return( + ServiceResponse.success(payload: { item_consumer: group_consumer }) + ) + allow(Ai::Catalog::ItemConsumers::CreateService).to receive(:new).and_return(create_service) + + expect(Ai::FlowTriggers::CreateService).not_to receive(:new) + + described_class.new(container, current_user: user).execute + end + + it 'skips trigger creation when service account is missing' do + parent_consumer = build(:ai_catalog_item_consumer, id: 456, group: group, item: flow1) + allow(parent_consumer).to receive(:service_account).and_return(nil) + + project_consumer = build(:ai_catalog_item_consumer, + id: 123, + project: container, + item: flow1, + parent_item_consumer: parent_consumer + ) + allow(project_consumer).to receive_messages(project: container, parent_item_consumer: parent_consumer) + + container.project_setting.update!(duo_foundational_flows_enabled: true) + allow(container).to receive(:enabled_flow_catalog_item_ids).and_return([flow1.id]) + allow(container.namespace).to receive(:configured_ai_catalog_items).and_return([parent_consumer]) + allow(container).to receive(:remove_foundational_flow_consumers) + allow(Ability).to receive(:allowed?).and_return(true) + + create_service = instance_double(Ai::Catalog::ItemConsumers::CreateService) + allow(create_service).to receive(:execute).and_return( + ServiceResponse.success(payload: { item_consumer: project_consumer }) + ) + allow(Ai::Catalog::ItemConsumers::CreateService).to receive(:new).and_return(create_service) + + expect(Ai::FlowTriggers::CreateService).not_to receive(:new) + + described_class.new(container, current_user: user).execute + end + end + context 'when user does not have permission' do before do container.namespace_settings.update!(duo_foundational_flows_enabled: true) @@ -130,7 +181,8 @@ allow(container).to receive(:enabled_flow_catalog_item_ids).and_return([flow1.id]) create_service = instance_double(Ai::Catalog::ItemConsumers::CreateService) - allow(create_service).to receive(:execute).and_return(ServiceResponse.success) + allow(create_service).to receive(:execute).and_return( + ServiceResponse.success(payload: { item_consumer: build(:ai_catalog_item_consumer) })) allow(Ai::Catalog::ItemConsumers::CreateService).to receive(:new).and_return(create_service) service.execute @@ -139,6 +191,25 @@ end end + context 'when item is already configured' do + before do + container.namespace_settings.update!(duo_foundational_flows_enabled: true) + end + + it 'handles the error gracefully and continues' do + allow(container).to receive(:enabled_flow_catalog_item_ids).and_return([flow1.id]) + allow(Ability).to receive(:allowed?).and_return(true) + + create_service = instance_double(Ai::Catalog::ItemConsumers::CreateService) + error_result = ServiceResponse.error(message: "Item already configured for container") + allow(create_service).to receive(:execute).and_return(error_result) + allow(Ai::Catalog::ItemConsumers::CreateService).to receive(:new).and_return(create_service) + + # The service should handle the error gracefully + expect { service.execute }.not_to raise_error + end + end + context 'when catalog item is not found' do before do container.namespace_settings.update!(duo_foundational_flows_enabled: true) @@ -306,7 +377,8 @@ it 'calls CreateService with correct parameters' do create_service = instance_double(Ai::Catalog::ItemConsumers::CreateService) - allow(create_service).to receive(:execute).and_return(ServiceResponse.success) + allow(create_service).to receive(:execute).and_return( + ServiceResponse.success(payload: { item_consumer: build(:ai_catalog_item_consumer) })) expect(Ai::Catalog::ItemConsumers::CreateService).to receive(:new).with( container: container, @@ -330,7 +402,8 @@ allow(Ability).to receive(:allowed?).and_return(true) create_service = instance_double(Ai::Catalog::ItemConsumers::CreateService) - allow(create_service).to receive(:execute).and_return(ServiceResponse.success) + allow(create_service).to receive(:execute).and_return( + ServiceResponse.success(payload: { item_consumer: build(:ai_catalog_item_consumer) })) allow(Ai::Catalog::ItemConsumers::CreateService).to receive(:new).and_return(create_service) expect(Gitlab::ErrorTracking).to receive(:track_exception) -- GitLab From 95ad8c04f94892d97e880c79e309ca2909457b45 Mon Sep 17 00:00:00 2001 From: Surabhi Suman Date: Thu, 11 Dec 2025 11:51:25 +0530 Subject: [PATCH 2/3] Fetch triggers from workflow_definition This adds flow triggers for foundational flow directly from workflow_definition config and automatically create the triggers when settings change EE: true --- .../ai/duo_workflows/workflow_definition.rb | 25 +++- .../flows/sync_foundational_flows_service.rb | 30 ++-- .../sync_foundational_flows_service_spec.rb | 132 +++++++++++------- 3 files changed, 120 insertions(+), 67 deletions(-) diff --git a/ee/app/models/ai/duo_workflows/workflow_definition.rb b/ee/app/models/ai/duo_workflows/workflow_definition.rb index fe0f9885fb26ea..c228f71e48dd8f 100644 --- a/ee/app/models/ai/duo_workflows/workflow_definition.rb +++ b/ee/app/models/ai/duo_workflows/workflow_definition.rb @@ -18,7 +18,7 @@ class WorkflowDefinition ::Ai::DuoWorkflows::Workflow::AgentPrivileges::RUN_COMMANDS, ::Ai::DuoWorkflows::Workflow::AgentPrivileges::USE_GIT ], - triggers: [:review] + triggers: [] }, { id: 2, @@ -53,6 +53,8 @@ class WorkflowDefinition { id: 4, name: "developer/v1", + foundational_flow_reference: "developer/v1", + description: "GitLab Duo developer", pre_approved_agent_privileges: [ ::Ai::DuoWorkflows::Workflow::AgentPrivileges::READ_WRITE_FILES, ::Ai::DuoWorkflows::Workflow::AgentPrivileges::READ_ONLY_GITLAB, @@ -60,7 +62,24 @@ class WorkflowDefinition ::Ai::DuoWorkflows::Workflow::AgentPrivileges::USE_GIT ], environment: "web", - triggers: [:assign] + triggers: [::Ai::FlowTrigger::EVENT_TYPES[:assign]], + avatar: "gitlab-duo-flow.png" + }, + { + id: 5, + name: "fix_pipeline/v1", + foundational_flow_reference: "fix_pipeline/v1", + description: "GitLab pipeline troubleshooter", + pre_approved_agent_privileges: [ + ::Ai::DuoWorkflows::Workflow::AgentPrivileges::READ_WRITE_FILES, + ::Ai::DuoWorkflows::Workflow::AgentPrivileges::READ_ONLY_GITLAB, + ::Ai::DuoWorkflows::Workflow::AgentPrivileges::READ_WRITE_GITLAB, + ::Ai::DuoWorkflows::Workflow::AgentPrivileges::RUN_COMMANDS, + ::Ai::DuoWorkflows::Workflow::AgentPrivileges::USE_GIT + ], + environment: "web", + triggers: [], + avatar: "fix-pipeline-flow.png" } ].freeze @@ -72,6 +91,8 @@ class WorkflowDefinition attribute :environment, :string, default: "ambient" attribute :foundational_flow_reference, :string attribute :description, :string + attribute :triggers, default: [] + attribute :avatar, :string validates :name, :ai_feature, presence: true diff --git a/ee/app/services/ai/catalog/flows/sync_foundational_flows_service.rb b/ee/app/services/ai/catalog/flows/sync_foundational_flows_service.rb index fbb66564166301..680f66b80495db 100644 --- a/ee/app/services/ai/catalog/flows/sync_foundational_flows_service.rb +++ b/ee/app/services/ai/catalog/flows/sync_foundational_flows_service.rb @@ -65,16 +65,13 @@ def create_consumer_for_catalog_item(item) def should_create_consumer?(item) return true unless container.is_a?(Project) - return true unless item.flow? || item.third_party_flow? - - parent_consumer = container.root_ancestor.configured_ai_catalog_items - .find { |c| c.ai_catalog_item_id == item.id } + parent_consumer = container.root_ancestor.configured_ai_catalog_items.for_item(item.id).first parent_consumer.present? end def authorized_to_create_consumer?(item) - return true unless current_user + return false unless current_user Ability.allowed?(current_user, :admin_ai_catalog_item_consumer, container) && Ability.allowed?(current_user, :read_ai_catalog_item, item) @@ -93,9 +90,8 @@ def create_or_find_consumer(item) def build_consumer_params(item) params = { item: item } - if container.is_a?(Project) && (item.flow? || item.third_party_flow?) - parent_consumer = container.namespace.configured_ai_catalog_items - .find { |c| c.ai_catalog_item_id == item.id } + if container.is_a?(Project) && item.flow? + parent_consumer = find_existing_consumer(item, container.root_ancestor) params[:parent_item_consumer] = parent_consumer if parent_consumer end @@ -105,7 +101,7 @@ def build_consumer_params(item) def extract_consumer_from_result(result, item) return result.payload[:item_consumer] if result.success? - return find_existing_consumer(item) if item_already_configured?(result) + return find_existing_consumer(item, container) if item_already_configured?(result) nil end @@ -114,13 +110,12 @@ def item_already_configured?(result) result.error? && result.message.include?("Item already configured") end - def find_existing_consumer(item) + def find_existing_consumer(item, container) container.configured_ai_catalog_items.find { |c| c.ai_catalog_item_id == item.id } end def create_trigger_if_needed(consumer, item) return unless container.is_a?(Project) - return unless item&.foundational_flow_reference.present? # Only create trigger for foundational flows create_trigger_for_consumer(consumer, item) end @@ -130,6 +125,7 @@ def create_trigger_for_consumer(consumer, item) return unless service_account trigger_params = build_trigger_params(service_account, consumer, item) + return unless trigger_params.present? ::Ai::FlowTriggers::CreateService.new( project: container, @@ -152,11 +148,14 @@ def extract_service_account(consumer) end def build_trigger_params(service_account, consumer, item) + event_types = fetch_event_type_for_flow(item.foundational_flow_reference) + return if event_types.empty? + { user_id: service_account.id, description: "Foundational flow trigger for #{item.name}", ai_catalog_item_consumer_id: consumer.id, - event_types: default_event_types + event_types: event_types } end @@ -174,8 +173,11 @@ def foundational_flow_ids Item.foundational_flow_ids end - def default_event_types - Ai::FlowTrigger::EVENT_TYPES.values + def fetch_event_type_for_flow(foundational_flow_reference) + flow_definition = ::Ai::DuoWorkflows::WorkflowDefinition[foundational_flow_reference] + return [] unless flow_definition.present? && flow_definition.triggers.present? + + flow_definition.triggers end end end diff --git a/ee/spec/services/ai/catalog/flows/sync_foundational_flows_service_spec.rb b/ee/spec/services/ai/catalog/flows/sync_foundational_flows_service_spec.rb index ed466242838f90..dafef2f4f260f4 100644 --- a/ee/spec/services/ai/catalog/flows/sync_foundational_flows_service_spec.rb +++ b/ee/spec/services/ai/catalog/flows/sync_foundational_flows_service_spec.rb @@ -40,8 +40,7 @@ allow(Ability).to receive(:allowed?).and_return(true) create_service = instance_double(Ai::Catalog::ItemConsumers::CreateService) - allow(create_service).to receive(:execute).and_return( - ServiceResponse.success(payload: { item_consumer: build(:ai_catalog_item_consumer) })) + allow(create_service).to receive(:execute).and_return(ServiceResponse.success) allow(Ai::Catalog::ItemConsumers::CreateService).to receive(:new).and_return(create_service) service.execute @@ -76,8 +75,10 @@ end it 'creates consumers with parent consumer for flows' do - parent_consumer = create(:ai_catalog_item_consumer, group: group, item: flow1) + parent_consumer = build_stubbed(:ai_catalog_item_consumer, group: group, item: flow1) allow(container).to receive(:enabled_flow_catalog_item_ids).and_return([flow1.id]) + allow(service).to receive(:should_create_consumer?).and_return(true) + allow(service).to receive(:find_existing_consumer).with(flow1, group).and_return(parent_consumer) create_service = instance_double(Ai::Catalog::ItemConsumers::CreateService) allow(create_service).to receive(:execute).and_return( @@ -106,50 +107,86 @@ context 'when trigger creation' do let(:container) { project } + let(:create_service) { instance_double(Ai::Catalog::ItemConsumers::CreateService) } + let(:flow) do + create(:ai_catalog_item, foundational_flow_reference: 'developer/v1', public: true, + organization: group.organization) + end - it 'does not create triggers for group-level consumers' do - allow(container).to receive(:enabled_flow_catalog_item_ids).and_return([flow1.id]) - allow(Ability).to receive(:allowed?).and_return(true) - - create_service = instance_double(Ai::Catalog::ItemConsumers::CreateService) - group_consumer = build(:ai_catalog_item_consumer, group: group, item: flow1) - allow(create_service).to receive(:execute).and_return( - ServiceResponse.success(payload: { item_consumer: group_consumer }) - ) + before do + container.project_setting.update!(duo_foundational_flows_enabled: true) + allow(container).to receive(:enabled_flow_catalog_item_ids).and_return([flow.id]) + allow(Ability).to receive(:allowed?).with(user, :admin_ai_catalog_item_consumer, container).and_return(true) + allow(Ability).to receive(:allowed?).with(user, :read_ai_catalog_item, flow).and_return(true) allow(Ai::Catalog::ItemConsumers::CreateService).to receive(:new).and_return(create_service) + end - expect(Ai::FlowTriggers::CreateService).not_to receive(:new) + context 'when consumer is group-level' do + before do + group_consumer = build(:ai_catalog_item_consumer, group: group, item: flow) + allow(create_service).to receive(:execute).and_return( + ServiceResponse.success(payload: { item_consumer: group_consumer }) + ) + end - described_class.new(container, current_user: user).execute + it 'does not create triggers' do + expect(Ai::FlowTriggers::CreateService).not_to receive(:new) + + described_class.new(container, current_user: user).execute + end end - it 'skips trigger creation when service account is missing' do - parent_consumer = build(:ai_catalog_item_consumer, id: 456, group: group, item: flow1) - allow(parent_consumer).to receive(:service_account).and_return(nil) + context 'when parent consumer has no service account' do + before do + parent_consumer = create(:ai_catalog_item_consumer, group: group, item: flow) + allow(parent_consumer).to receive(:service_account).and_return(nil) - project_consumer = build(:ai_catalog_item_consumer, - id: 123, - project: container, - item: flow1, - parent_item_consumer: parent_consumer - ) - allow(project_consumer).to receive_messages(project: container, parent_item_consumer: parent_consumer) + project_consumer = build(:ai_catalog_item_consumer, + project: container, + item: flow1, + parent_item_consumer: parent_consumer + ) - container.project_setting.update!(duo_foundational_flows_enabled: true) - allow(container).to receive(:enabled_flow_catalog_item_ids).and_return([flow1.id]) - allow(container.namespace).to receive(:configured_ai_catalog_items).and_return([parent_consumer]) - allow(container).to receive(:remove_foundational_flow_consumers) - allow(Ability).to receive(:allowed?).and_return(true) + allow(create_service).to receive(:execute).and_return( + ServiceResponse.success(payload: { item_consumer: project_consumer }) + ) + end - create_service = instance_double(Ai::Catalog::ItemConsumers::CreateService) - allow(create_service).to receive(:execute).and_return( - ServiceResponse.success(payload: { item_consumer: project_consumer }) - ) - allow(Ai::Catalog::ItemConsumers::CreateService).to receive(:new).and_return(create_service) + it 'does not create triggers' do + expect(Ai::FlowTriggers::CreateService).not_to receive(:new) + + described_class.new(container, current_user: user).execute + end + end + + context 'when parent consumer has service account' do + let(:service_account) { build(:user) } + let(:trigger_service) { instance_double(Ai::FlowTriggers::CreateService) } + + before do + parent_consumer = create(:ai_catalog_item_consumer, group: group, item: flow) + allow(parent_consumer).to receive(:service_account).and_return(service_account) + + project_consumer = build(:ai_catalog_item_consumer, + project: container, + item: flow1, + parent_item_consumer: parent_consumer + ) + + allow(create_service).to receive(:execute).and_return( + ServiceResponse.success(payload: { item_consumer: project_consumer }) + ) + allow(trigger_service).to receive(:execute).and_return(ServiceResponse.success) + end - expect(Ai::FlowTriggers::CreateService).not_to receive(:new) + it 'creates triggers' do + expect(Ai::FlowTriggers::CreateService).to receive(:new) + .and_return(trigger_service) - described_class.new(container, current_user: user).execute + described_class.new(container, current_user: user).execute + + expect(trigger_service).to have_received(:execute) + end end end @@ -177,17 +214,10 @@ let(:current_user) { nil } - it 'creates consumers without permission checks' do + it 'does not create consumers' do allow(container).to receive(:enabled_flow_catalog_item_ids).and_return([flow1.id]) - - create_service = instance_double(Ai::Catalog::ItemConsumers::CreateService) - allow(create_service).to receive(:execute).and_return( - ServiceResponse.success(payload: { item_consumer: build(:ai_catalog_item_consumer) })) - allow(Ai::Catalog::ItemConsumers::CreateService).to receive(:new).and_return(create_service) - + expect(Ai::Catalog::ItemConsumers::CreateService).not_to receive(:new) service.execute - - expect(Ai::Catalog::ItemConsumers::CreateService).to have_received(:new) end end @@ -320,9 +350,11 @@ end it 'looks up parent consumer from root ancestor, not immediate parent' do - # Parent consumer is on the root group, not the subgroup parent_consumer = create(:ai_catalog_item_consumer, group: group, item: flow1) allow(container).to receive(:enabled_flow_catalog_item_ids).and_return([flow1.id]) + allow(group).to receive(:configured_ai_catalog_items).and_return( + Ai::Catalog::ItemConsumer.where(id: parent_consumer.id) + ) create_service = instance_double(Ai::Catalog::ItemConsumers::CreateService) allow(create_service).to receive(:execute).and_return(ServiceResponse.success) @@ -340,8 +372,8 @@ end it 'skips flows when parent consumer does not exist on root ancestor' do - # No parent consumer on root ancestor allow(container).to receive(:enabled_flow_catalog_item_ids).and_return([flow1.id]) + allow(group).to receive_message_chain(:configured_ai_catalog_items, :for_item).and_return([]) expect(Ai::Catalog::ItemConsumers::CreateService).not_to receive(:new) @@ -377,8 +409,7 @@ it 'calls CreateService with correct parameters' do create_service = instance_double(Ai::Catalog::ItemConsumers::CreateService) - allow(create_service).to receive(:execute).and_return( - ServiceResponse.success(payload: { item_consumer: build(:ai_catalog_item_consumer) })) + allow(create_service).to receive(:execute).and_return(ServiceResponse.success) expect(Ai::Catalog::ItemConsumers::CreateService).to receive(:new).with( container: container, @@ -402,8 +433,7 @@ allow(Ability).to receive(:allowed?).and_return(true) create_service = instance_double(Ai::Catalog::ItemConsumers::CreateService) - allow(create_service).to receive(:execute).and_return( - ServiceResponse.success(payload: { item_consumer: build(:ai_catalog_item_consumer) })) + allow(create_service).to receive(:execute).and_return(ServiceResponse.success) allow(Ai::Catalog::ItemConsumers::CreateService).to receive(:new).and_return(create_service) expect(Gitlab::ErrorTracking).to receive(:track_exception) -- GitLab From e129d27d501fa2522bb58abce30a5ec68c95371f Mon Sep 17 00:00:00 2001 From: Surabhi Suman Date: Sat, 13 Dec 2025 16:18:29 +0530 Subject: [PATCH 3/3] Add check for pre-existing triggers --- .../flows/sync_foundational_flows_service.rb | 21 +++--- .../sync_foundational_flows_service_spec.rb | 64 +++++++++++-------- 2 files changed, 49 insertions(+), 36 deletions(-) diff --git a/ee/app/services/ai/catalog/flows/sync_foundational_flows_service.rb b/ee/app/services/ai/catalog/flows/sync_foundational_flows_service.rb index 680f66b80495db..4589bedae55280 100644 --- a/ee/app/services/ai/catalog/flows/sync_foundational_flows_service.rb +++ b/ee/app/services/ai/catalog/flows/sync_foundational_flows_service.rb @@ -66,7 +66,7 @@ def create_consumer_for_catalog_item(item) def should_create_consumer?(item) return true unless container.is_a?(Project) - parent_consumer = container.root_ancestor.configured_ai_catalog_items.for_item(item.id).first + parent_consumer = find_existing_consumer(item, container.root_ancestor) parent_consumer.present? end @@ -131,12 +131,6 @@ def create_trigger_for_consumer(consumer, item) project: container, current_user: current_user ).execute(trigger_params) - rescue StandardError => e - ::Gitlab::ErrorTracking.track_exception( - e, - item_id: item.id, - project_id: container.id - ) end def extract_service_account(consumer) @@ -148,7 +142,7 @@ def extract_service_account(consumer) end def build_trigger_params(service_account, consumer, item) - event_types = fetch_event_type_for_flow(item.foundational_flow_reference) + event_types = fetch_event_type_for_flow(item.foundational_flow_reference, service_account) return if event_types.empty? { @@ -173,11 +167,18 @@ def foundational_flow_ids Item.foundational_flow_ids end - def fetch_event_type_for_flow(foundational_flow_reference) + def fetch_event_type_for_flow(foundational_flow_reference, service_account) flow_definition = ::Ai::DuoWorkflows::WorkflowDefinition[foundational_flow_reference] return [] unless flow_definition.present? && flow_definition.triggers.present? - flow_definition.triggers + flow_definition.triggers.reject { |event| trigger_exists?(service_account, event) } + end + + def trigger_exists?(service_account, event) + event_type = ::Ai::FlowTrigger::EVENT_TYPES.key(event) + return false unless event_type + + container.ai_flow_triggers.triggered_on(event_type).by_users([service_account]).exists? end end end diff --git a/ee/spec/services/ai/catalog/flows/sync_foundational_flows_service_spec.rb b/ee/spec/services/ai/catalog/flows/sync_foundational_flows_service_spec.rb index dafef2f4f260f4..e219d984617f48 100644 --- a/ee/spec/services/ai/catalog/flows/sync_foundational_flows_service_spec.rb +++ b/ee/spec/services/ai/catalog/flows/sync_foundational_flows_service_spec.rb @@ -75,10 +75,9 @@ end it 'creates consumers with parent consumer for flows' do - parent_consumer = build_stubbed(:ai_catalog_item_consumer, group: group, item: flow1) + parent_consumer = create(:ai_catalog_item_consumer, group: group, item: flow1) allow(container).to receive(:enabled_flow_catalog_item_ids).and_return([flow1.id]) - allow(service).to receive(:should_create_consumer?).and_return(true) - allow(service).to receive(:find_existing_consumer).with(flow1, group).and_return(parent_consumer) + allow(group).to receive(:configured_ai_catalog_items).and_return([parent_consumer]) create_service = instance_double(Ai::Catalog::ItemConsumers::CreateService) allow(create_service).to receive(:execute).and_return( @@ -160,32 +159,47 @@ end context 'when parent consumer has service account' do - let(:service_account) { build(:user) } - let(:trigger_service) { instance_double(Ai::FlowTriggers::CreateService) } + let(:service_account) do + create(:user, :service_account, composite_identity_enforced: true, provisioned_by_group: group) + end + + let(:parent_consumer) { create(:ai_catalog_item_consumer, group: group, item: flow) } + let(:project_consumer) do + create(:ai_catalog_item_consumer, project: container, item: flow1, parent_item_consumer: parent_consumer) + end before do - parent_consumer = create(:ai_catalog_item_consumer, group: group, item: flow) allow(parent_consumer).to receive(:service_account).and_return(service_account) - - project_consumer = build(:ai_catalog_item_consumer, - project: container, - item: flow1, - parent_item_consumer: parent_consumer - ) - allow(create_service).to receive(:execute).and_return( ServiceResponse.success(payload: { item_consumer: project_consumer }) ) - allow(trigger_service).to receive(:execute).and_return(ServiceResponse.success) + allow(Ability).to receive(:allowed?).with(user, :admin_service_accounts, group).and_return(true) + allow(group).to receive(:configured_ai_catalog_items).and_return([parent_consumer]) end it 'creates triggers' do - expect(Ai::FlowTriggers::CreateService).to receive(:new) - .and_return(trigger_service) - + expect_next_instance_of(::Ai::FlowTriggers::CreateService) do |instance| + expect(instance).to receive(:execute).with( + hash_including(user_id: service_account.id, ai_catalog_item_consumer_id: project_consumer.id) + ).and_call_original + end described_class.new(container, current_user: user).execute + end + + context 'when trigger already exists' do + before do + create(:ai_flow_trigger, + project: container, + user: service_account, + event_types: [::Ai::FlowTrigger::EVENT_TYPES[:assign]] + ) + end + + it 'does not create a duplicate trigger' do + expect(Ai::FlowTriggers::CreateService).not_to receive(:new) - expect(trigger_service).to have_received(:execute) + described_class.new(container, current_user: user).execute + end end end end @@ -224,18 +238,17 @@ context 'when item is already configured' do before do container.namespace_settings.update!(duo_foundational_flows_enabled: true) + allow(container).to receive(:enabled_flow_catalog_item_ids).and_return([flow1.id]) + allow(Ability).to receive(:allowed?).with(user, :admin_ai_catalog_item_consumer, container).and_return(true) + allow(Ability).to receive(:allowed?).with(user, :read_ai_catalog_item, flow1).and_return(true) end it 'handles the error gracefully and continues' do - allow(container).to receive(:enabled_flow_catalog_item_ids).and_return([flow1.id]) - allow(Ability).to receive(:allowed?).and_return(true) - create_service = instance_double(Ai::Catalog::ItemConsumers::CreateService) error_result = ServiceResponse.error(message: "Item already configured for container") allow(create_service).to receive(:execute).and_return(error_result) allow(Ai::Catalog::ItemConsumers::CreateService).to receive(:new).and_return(create_service) - # The service should handle the error gracefully expect { service.execute }.not_to raise_error end end @@ -350,14 +363,13 @@ end it 'looks up parent consumer from root ancestor, not immediate parent' do + # Parent consumer is on the root group, not the subgroup parent_consumer = create(:ai_catalog_item_consumer, group: group, item: flow1) allow(container).to receive(:enabled_flow_catalog_item_ids).and_return([flow1.id]) - allow(group).to receive(:configured_ai_catalog_items).and_return( - Ai::Catalog::ItemConsumer.where(id: parent_consumer.id) - ) create_service = instance_double(Ai::Catalog::ItemConsumers::CreateService) allow(create_service).to receive(:execute).and_return(ServiceResponse.success) + allow(group).to receive(:configured_ai_catalog_items).and_return([parent_consumer]) expect(Ai::Catalog::ItemConsumers::CreateService).to receive(:new) .with( @@ -372,8 +384,8 @@ end it 'skips flows when parent consumer does not exist on root ancestor' do + # No parent consumer on root ancestor allow(container).to receive(:enabled_flow_catalog_item_ids).and_return([flow1.id]) - allow(group).to receive_message_chain(:configured_ai_catalog_items, :for_item).and_return([]) expect(Ai::Catalog::ItemConsumers::CreateService).not_to receive(:new) -- GitLab