From 2cd4dd87091242b43268b682cc6716b5db1be2ec Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Mon, 21 Jul 2025 10:23:16 +0200 Subject: [PATCH] Prevent recursion when invoking AI This can result in stack overflow errors for missing commands. https://fizzy.37signals.com/5986089/collections/7/cards/1135 Follow up to https://github.com/basecamp/fizzy/pull/766 --- app/models/command/ai/parser.rb | 4 +++- app/models/command/parser.rb | 17 +++++++++++++++-- test/models/command/ai/parser_test.rb | 11 +++++++++++ 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/app/models/command/ai/parser.rb b/app/models/command/ai/parser.rb index 4121a9de3..151ab00d5 100644 --- a/app/models/command/ai/parser.rb +++ b/app/models/command/ai/parser.rb @@ -34,7 +34,9 @@ class Command::Ai::Parser end def commands_from_query(normalized_query, context) - parser = Command::Parser.new(context) + # The query should only contain supported /commands. If that's not the case, + # we don't want to fall back to AI again (potential stack overflow). + parser = Command::Parser.new(context, fall_back_to_ai: false) if command_lines = normalized_query[:commands].presence command_lines.collect { parser.parse(it) } end diff --git a/app/models/command/parser.rb b/app/models/command/parser.rb index 0bf21abde..a0555824a 100644 --- a/app/models/command/parser.rb +++ b/app/models/command/parser.rb @@ -3,8 +3,9 @@ class Command::Parser delegate :user, :cards, :filter, :script_name, to: :context - def initialize(context) + def initialize(context, fall_back_to_ai: true) @context = context + @fall_back_to_ai = fall_back_to_ai end def parse(string) @@ -17,6 +18,10 @@ class Command::Parser end private + def fall_back_to_ai? + @fall_back_to_ai + end + def as_plain_text(string) ActionText::Content.new(string).to_plain_text end @@ -100,7 +105,7 @@ class Command::Parser elsif card = single_card_from(string) Command::GoToCard.new(card_id: card.id) else - Command::Ai::Parser.new(context).parse(string) + parse_with_fallback(string) end end @@ -114,4 +119,12 @@ class Command::Parser def single_card_from(string) user.accessible_cards.find_by_id(string) end + + def parse_with_fallback(string) + if fall_back_to_ai? + Command::Ai::Parser.new(context).parse(string) + else + Command::Search.new(terms: string) + end + end end diff --git a/test/models/command/ai/parser_test.rb b/test/models/command/ai/parser_test.rb index 821edd965..771456f2f 100644 --- a/test/models/command/ai/parser_test.rb +++ b/test/models/command/ai/parser_test.rb @@ -25,4 +25,15 @@ class Command::Ai::ParserTest < ActionDispatch::IntegrationTest assert_equal [ collections(:writebook).id.to_s ], params[:collection_ids] assert_equal [ tags(:web).id.to_s ], params[:tag_ids] end + + test "if the AI translator emitted an unknown command, this will result in a regular search (not on AI recursion)" do + Command::Ai::Translator.any_instance.stubs(:translate).returns(commands: [ "/missing" ]) + + command = parse_command "assign @kevin and close" + assert command.commands.one? + + search_command = command.commands.first + assert search_command.is_a?(Command::Search) + assert_equal "/missing", search_command.terms + end end