From 870b0fdea5644b79170154993d8084a2273ad750 Mon Sep 17 00:00:00 2001 From: Keeyan Nejad Date: Sun, 14 Sep 2025 15:07:51 +0100 Subject: [PATCH 1/3] Validate released flows have at least 1 step --- .../ai/catalog/concerns/flow_version.rb | 16 +++++++-- ee/spec/factories/ai/catalog/item_versions.rb | 6 +++- .../models/ai/catalog/flow_version.rb | 36 +++++++++++++++++++ locale/gitlab.pot | 3 ++ 4 files changed, 58 insertions(+), 3 deletions(-) diff --git a/ee/app/models/ai/catalog/concerns/flow_version.rb b/ee/app/models/ai/catalog/concerns/flow_version.rb index 7eb32477dec740..6b6df7380685f6 100644 --- a/ee/app/models/ai/catalog/concerns/flow_version.rb +++ b/ee/app/models/ai/catalog/concerns/flow_version.rb @@ -7,16 +7,28 @@ module FlowVersion extend ActiveSupport::Concern included do + validate :validate_released_version_has_steps, if: -> { released? } + def delete_no_longer_used_dependencies dependencies.where.not(dependency_id: dependency_ids).delete_all end def dependency_ids - # Check to ensure item is a flow and not an agent, so we don't have to load item for item.flow? - return unless definition['steps'] + return unless definition_for_flow? && definition['steps'].present? definition['steps'].pluck('agent_id').uniq.compact # rubocop:disable Database/AvoidUsingPluckWithoutLimit -- Not ActiveRecord end + + def validate_released_version_has_steps + return unless definition_for_flow? && definition['steps'].empty? + + errors.add(:definition, s_('AICatalog|must have at least one step')) + end + + # A shortcut to avoid checking item.flow? which would require loading `item` + def definition_for_flow? + definition.key?('steps') + end end end end diff --git a/ee/spec/factories/ai/catalog/item_versions.rb b/ee/spec/factories/ai/catalog/item_versions.rb index 6b1bda42c61bfc..2323abd4fdcfa4 100644 --- a/ee/spec/factories/ai/catalog/item_versions.rb +++ b/ee/spec/factories/ai/catalog/item_versions.rb @@ -28,9 +28,13 @@ trait :for_flow do item { association :ai_catalog_flow } definition do + agent_id = (Ai::Catalog::Item.find_by(item_type: :agent) || create(:ai_catalog_agent)).id # rubocop:disable RSpec/FactoryBot/InlineAssociation -- Not used for an association + { 'triggers' => [1], - 'steps' => [] + 'steps' => [ + { 'agent_id' => agent_id, 'current_version_id' => 1, 'pinned_version_prefix' => nil } + ] } end end diff --git a/ee/spec/support/shared_examples/models/ai/catalog/flow_version.rb b/ee/spec/support/shared_examples/models/ai/catalog/flow_version.rb index ce1366458142dd..f9cbeb6b865b14 100644 --- a/ee/spec/support/shared_examples/models/ai/catalog/flow_version.rb +++ b/ee/spec/support/shared_examples/models/ai/catalog/flow_version.rb @@ -5,6 +5,42 @@ let_it_be(:agent2) { create(:ai_catalog_item, :agent) } let_it_be(:agent3) { create(:ai_catalog_item, :agent) } + describe 'validations' do + it 'does not allow a released version without steps' do + version = build( + :ai_catalog_item_version, + :released, + :for_flow, + 'definition' => { 'steps' => [], 'triggers' => [] } + ) + + expect(version).not_to be_valid + expect(version.errors[:definition]).to include(s_('AICatalog|must have at least one step')) + end + + it 'allow a released version with steps' do + version = build(:ai_catalog_item_version, :released, :for_flow) + + expect(version).to be_valid + end + + it 'allows an unreleased version to have no steps' do + version = build( + :ai_catalog_item_version, + :for_flow, + 'definition' => { 'steps' => [], 'triggers' => [] } + ) + + expect(version).to be_valid + end + + it 'allows a released agent version to not have steps' do + version = build(:ai_catalog_agent_version, :for_agent, :released) + + expect(version).to be_valid + end + end + describe '#delete_no_longer_used_dependencies' do let_it_be(:flow_version) do create( diff --git a/locale/gitlab.pot b/locale/gitlab.pot index ba0835f58f9537..e02e47b94e1250 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2811,6 +2811,9 @@ msgstr "" msgid "AICatalog|is private to another project" msgstr "" +msgid "AICatalog|must have at least one step" +msgstr "" + msgid "AICatalog|organization must match the item consumer's organization" msgstr "" -- GitLab From 9b1a07ca71bfa541b4b5c74123b0246cb1c5805b Mon Sep 17 00:00:00 2001 From: Keeyan Nejad Date: Mon, 15 Sep 2025 09:08:56 +0100 Subject: [PATCH 2/3] Address feedback - Use latest version for item version agent step - Check `definition_for_flow?` in validation - Change error message --- ee/app/models/ai/catalog/concerns/flow_version.rb | 6 +++--- ee/spec/factories/ai/catalog/item_versions.rb | 4 ++-- .../shared_examples/models/ai/catalog/flow_version.rb | 2 +- locale/gitlab.pot | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/ee/app/models/ai/catalog/concerns/flow_version.rb b/ee/app/models/ai/catalog/concerns/flow_version.rb index 6b6df7380685f6..e7c9bb89b4a7d2 100644 --- a/ee/app/models/ai/catalog/concerns/flow_version.rb +++ b/ee/app/models/ai/catalog/concerns/flow_version.rb @@ -7,7 +7,7 @@ module FlowVersion extend ActiveSupport::Concern included do - validate :validate_released_version_has_steps, if: -> { released? } + validate :validate_released_version_has_steps, if: -> { released? && definition_for_flow? } def delete_no_longer_used_dependencies dependencies.where.not(dependency_id: dependency_ids).delete_all @@ -20,9 +20,9 @@ def dependency_ids end def validate_released_version_has_steps - return unless definition_for_flow? && definition['steps'].empty? + return unless definition['steps'].empty? - errors.add(:definition, s_('AICatalog|must have at least one step')) + errors.add(:definition, s_('AICatalog|must have at least one node')) end # A shortcut to avoid checking item.flow? which would require loading `item` diff --git a/ee/spec/factories/ai/catalog/item_versions.rb b/ee/spec/factories/ai/catalog/item_versions.rb index 2323abd4fdcfa4..e48a9ee23a6531 100644 --- a/ee/spec/factories/ai/catalog/item_versions.rb +++ b/ee/spec/factories/ai/catalog/item_versions.rb @@ -28,12 +28,12 @@ trait :for_flow do item { association :ai_catalog_flow } definition do - agent_id = (Ai::Catalog::Item.find_by(item_type: :agent) || create(:ai_catalog_agent)).id # rubocop:disable RSpec/FactoryBot/InlineAssociation -- Not used for an association + agent = Ai::Catalog::Item.find_by(item_type: :agent) || create(:ai_catalog_agent) # rubocop:disable RSpec/FactoryBot/InlineAssociation -- Not used for an association { 'triggers' => [1], 'steps' => [ - { 'agent_id' => agent_id, 'current_version_id' => 1, 'pinned_version_prefix' => nil } + { 'agent_id' => agent.id, 'current_version_id' => agent.latest_version.id, 'pinned_version_prefix' => nil } ] } end diff --git a/ee/spec/support/shared_examples/models/ai/catalog/flow_version.rb b/ee/spec/support/shared_examples/models/ai/catalog/flow_version.rb index f9cbeb6b865b14..82d07cccfdd9ff 100644 --- a/ee/spec/support/shared_examples/models/ai/catalog/flow_version.rb +++ b/ee/spec/support/shared_examples/models/ai/catalog/flow_version.rb @@ -15,7 +15,7 @@ ) expect(version).not_to be_valid - expect(version.errors[:definition]).to include(s_('AICatalog|must have at least one step')) + expect(version.errors[:definition]).to include(s_('AICatalog|must have at least one node')) end it 'allow a released version with steps' do diff --git a/locale/gitlab.pot b/locale/gitlab.pot index e02e47b94e1250..8053685915e924 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2811,7 +2811,7 @@ msgstr "" msgid "AICatalog|is private to another project" msgstr "" -msgid "AICatalog|must have at least one step" +msgid "AICatalog|must have at least one node" msgstr "" msgid "AICatalog|organization must match the item consumer's organization" -- GitLab From d798c37f447271747f238e662e50e067b35597c5 Mon Sep 17 00:00:00 2001 From: Keeyan Nejad Date: Mon, 15 Sep 2025 14:34:02 +0100 Subject: [PATCH 3/3] Remove spec for when flow has no versions This case is no longer possible since latest_version is required --- .../duo_workflow_payload_builder/experimental_spec.rb | 7 ------- 1 file changed, 7 deletions(-) diff --git a/ee/spec/lib/ai/catalog/duo_workflow_payload_builder/experimental_spec.rb b/ee/spec/lib/ai/catalog/duo_workflow_payload_builder/experimental_spec.rb index f1ecfc331b69ea..85c38e6fd14af8 100644 --- a/ee/spec/lib/ai/catalog/duo_workflow_payload_builder/experimental_spec.rb +++ b/ee/spec/lib/ai/catalog/duo_workflow_payload_builder/experimental_spec.rb @@ -70,13 +70,6 @@ end describe '#build' do - context 'when flow has no versions' do - let_it_be(:empty_flow) { create(:ai_catalog_flow, project: project) } - let_it_be(:builder) { described_class.new(empty_flow) } - - it_behaves_like 'invalid flow configuration' - end - context 'when flow has no agents' do let_it_be(:empty_steps_definition) { { 'triggers' => [1], 'steps' => [] } } let_it_be(:empty_steps_flow) { create(:ai_catalog_flow, project: project) } -- GitLab