diff --git a/src/langbot/pkg/api/http/service/bot.py b/src/langbot/pkg/api/http/service/bot.py index 42bb35338..316ac02a7 100644 --- a/src/langbot/pkg/api/http/service/bot.py +++ b/src/langbot/pkg/api/http/service/bot.py @@ -137,7 +137,16 @@ class BotService: bot = await self.get_bot(context, bot_data['uuid'], include_secret=True) - await self.ap.platform_mgr.load_bot(context, bot) + try: + await self.ap.platform_mgr.load_bot(context, bot) + except Exception: + # The bot row was already inserted above; without this rollback a + # failing adapter constructor (e.g. a missing optional credential + # key) would leave a permanently disabled orphan bot in the DB. + await self.ap.persistence_mgr.execute_async( + sqlalchemy.delete(persistence_bot.Bot).where(persistence_bot.Bot.uuid == bot_data['uuid']) + ) + raise return bot_data['uuid'] diff --git a/tests/unit_tests/api/service/test_bot_service.py b/tests/unit_tests/api/service/test_bot_service.py index 55869fc0a..fd809165e 100644 --- a/tests/unit_tests/api/service/test_bot_service.py +++ b/tests/unit_tests/api/service/test_bot_service.py @@ -12,6 +12,7 @@ import pytest from unittest.mock import AsyncMock, MagicMock, Mock, patch from types import SimpleNamespace import json +import sqlalchemy import uuid from langbot.pkg.api.http.service.bot import BotService @@ -449,10 +450,58 @@ class TestBotServiceCreateBot: insert_statement = ap.persistence_mgr.execute_async.await_args_list[1].args[0] insert_values = insert_statement.compile().params assert insert_values['workspace_uuid'] == WORKSPACE_UUID - assert insert_values['use_pipeline_uuid'] == 'default-pipeline-uuid' - assert insert_values['use_pipeline_name'] == 'Default Pipeline' assert bot_uuid is not None # Verify UUID was returned + async def test_create_bot_rolls_back_insert_when_load_bot_fails(self): + """Deletes the inserted row when the adapter fails to load. + + Regression: a failing adapter constructor (e.g. KeyError on a missing + optional credential key) used to leave a permanently disabled orphan + bot in the DB — the insert was already committed and the HTTP layer + surfaced a 500 without any cleanup. + """ + # Setup + ap = SimpleNamespace() + ap.persistence_mgr = SimpleNamespace() + ap.instance_config = SimpleNamespace() + ap.instance_config.data = {'system': {'limitation': {'max_bots': -1}}} + ap.platform_mgr = SimpleNamespace() + ap.platform_mgr.load_bot = AsyncMock(side_effect=KeyError('token')) + + pipeline_result = Mock() + pipeline_result.first = Mock(return_value=None) + bot_result = Mock() + bot_result.first = Mock(return_value=_create_mock_bot()) + + executed_statements = [] + + async def mock_execute(query): + executed_statements.append(query) + if len(executed_statements) <= 2: + return pipeline_result # 1: limitation bots query, 2: pipeline query + if len(executed_statements) == 3: + return Mock() # insert + return bot_result # get_bot after insert + + ap.persistence_mgr.execute_async = AsyncMock(side_effect=mock_execute) + ap.persistence_mgr.serialize_model = Mock(return_value={'uuid': 'new-uuid', 'name': 'New Bot'}) + + service = BotService(ap) + + # Execute & Verify: the adapter error propagates + with pytest.raises(KeyError, match='token'): + await service.create_bot( + WORKSPACE_UUID, {'name': 'New Bot', 'adapter': 'telegram', 'adapter_config': {}} + ) + + # And the inserted row is rolled back via a DELETE on the new uuid + # (no limitation query runs because max_bots=-1) + assert len(executed_statements) == 4 # pipeline select, insert, bot select, delete + delete_statement = executed_statements[-1] + assert isinstance(delete_statement, sqlalchemy.sql.dml.Delete) + compiled = delete_statement.compile() + assert compiled.params['uuid_1'] is not None + class TestBotServiceUpdateBot: """Tests for update_bot method."""