Merge pull request #2675 from langbot-app/test/quality-self-duplicates-20261017

test: remove redundant backend checks and migration aliases

.github/workflows/run-tests.yml

@@ -166,12 +166,19 @@ jobs:
               'test_pipeline_admission_postgres': 27,
               'test_rag_document_identity': 34,
               'test_rag_interrupted_ingestion': 22,
-              'test_migration_branch_convergence': 7,
+              'test_migration_branch_convergence': 6,
               'test_runner_timestamps_postgres': 32,
           }
           counts = Counter(case.get('classname', '').rsplit('.', 1)[-1] for case in cases)
           for module, minimum in expected.items():
               assert counts[module] >= minimum, f'{module}: expected at least {minimum} cases, got {counts[module]}'
+          convergence_cases = {
+              case.get('name') for case in cases
+              if case.get('classname', '').rsplit('.', 1)[-1] == 'test_migration_branch_convergence'
+          }
+          assert 'test_empty_database_startup_schema_then_real_migrations[postgresql]' in convergence_cases, (
+              'The fresh-schema migration and idempotency contract must execute'
+          )
           unsuccessful = [
               case.get('name') for case in cases
               if any(case.find(tag) is not None for tag in ('skipped', 'failure', 'error'))

tests/integration/api/test_smoke.py

@@ -164,16 +164,6 @@ class TestHealthEndpoint:
         data = await response.get_json()
         assert data == {'code': 0, 'msg': 'ok', 'resources': {}}
 
-    @pytest.mark.asyncio
-    async def test_healthz_no_auth_required(self, quart_test_client):
-        """
-        /healthz doesn't require authentication.
-
-        Tests that AuthType.NONE endpoints work without headers.
-        """
-        response = await quart_test_client.get('/healthz')
-        assert response.status_code == 200
-
 
 @pytest.mark.usefixtures('mock_circular_import_chain')
 class TestSystemEndpoint:

tests/integration/api/test_user_space_oauth.py

@@ -234,39 +234,6 @@ async def test_redirect_rejects_invalid_callback_shape(space_oauth_api):
     assert all(payload['code'] == 1 for payload in payloads)
 
 
-@pytest.mark.asyncio
-async def test_explicit_server_side_webui_origin_supports_split_dev_server(space_oauth_api):
-    application, client = space_oauth_api
-    application.instance_config.data['api'] = {'webui_url': 'http://localhost:5173'}
-
-    response = await client.get(
-        '/api/v1/user/space/authorize-url',
-        query_string={'redirect_uri': 'http://localhost:5173/auth/space/callback'},
-        headers={'Origin': 'https://irrelevant.example'},
-    )
-
-    assert response.status_code == 200
-    assert (await response.get_json())['code'] == 0
-
-
-@pytest.mark.asyncio
-async def test_server_side_webhook_origin_supports_bundled_ui(space_oauth_api):
-    application, client = space_oauth_api
-    application.instance_config.data['api'] = {
-        'webui_url': '',
-        'webhook_prefix': 'https://langbot.example/base/path',
-    }
-
-    response = await client.get(
-        '/api/v1/user/space/authorize-url',
-        query_string={'redirect_uri': 'https://langbot.example/auth/space/callback'},
-        headers={'Host': 'attacker.example'},
-    )
-
-    assert response.status_code == 200
-    assert (await response.get_json())['code'] == 0
-
-
 @pytest.mark.asyncio
 async def test_login_callback_requires_and_consumes_server_state(space_oauth_api):
     application, client = space_oauth_api

tests/integration/persistence/test_migration_branch_convergence.py

@@ -354,6 +354,9 @@ async def test_empty_database_startup_schema_then_real_migrations(convergence_en
     await run_alembic_upgrade(engine, 'head')
     assert await get_alembic_current(engine) == get_alembic_head()
 
+    await run_alembic_upgrade(engine, 'head')
+    assert await get_alembic_current(engine) == get_alembic_head()
+
 
 def _legacy_shared_certification(**overrides):
     certification = {
@@ -480,18 +483,6 @@ async def test_certification_artifact_digest_backfill_is_safe_and_keeps_legacy_d
             )
 
 
-@pytest.mark.asyncio
-async def test_certification_artifact_digest_backfill_accepts_fresh_current_schema(convergence_engine):
-    engine = convergence_engine
-    async with engine.begin() as conn:
-        await conn.run_sync(Base.metadata.create_all)
-
-    await run_alembic_upgrade(engine, 'head')
-    await run_alembic_upgrade(engine, 'head')
-
-    assert await get_alembic_current(engine) == get_alembic_head()
-
-
 @pytest.mark.asyncio
 async def test_merge_only_downgrade_preserves_both_branch_schemas(convergence_engine):
     engine = convergence_engine

tests/integration/persistence/test_migrations.py

@@ -438,16 +438,3 @@ class TestSQLiteMigrationGetCurrent:
         # No stamp - should return None
         rev = await get_alembic_current(sqlite_engine)
         assert rev is None, f'Expected None for unstamped DB, got {rev}'
-
-    @pytest.mark.asyncio
-    async def test_get_current_after_stamp_returns_revision(self, sqlite_engine):
-        """
-        get_alembic_current returns correct revision after stamp.
-        """
-        async with sqlite_engine.begin() as conn:
-            await conn.run_sync(Base.metadata.create_all)
-
-        await run_alembic_stamp(sqlite_engine, '0001_baseline')
-
-        rev = await get_alembic_current(sqlite_engine)
-        assert rev == '0001_baseline'

tests/integration/persistence/test_migrations_postgres.py

@@ -550,21 +550,6 @@ class TestPostgreSQLMigrationGetCurrent:
         rev = await get_alembic_current(postgres_engine)
         assert rev is None, f'Expected None for unstamped DB, got {rev}'
 
-    @pytest.mark.asyncio
-    async def test_postgres_get_current_after_stamp_returns_revision(
-        self, postgres_engine, clean_tables, clean_alembic_version
-    ):
-        """
-        get_alembic_current returns correct revision after stamp.
-        """
-        async with postgres_engine.begin() as conn:
-            await conn.run_sync(Base.metadata.create_all)
-
-        await run_alembic_stamp(postgres_engine, '0001_baseline')
-
-        rev = await get_alembic_current(postgres_engine)
-        assert rev == '0001_baseline'
-
 
 class TestPostgreSQLWorkspaceMigration:
     """Focused coverage for upgrading a pre-tenancy PostgreSQL instance."""

tests/unit_tests/agent/test_event_first_protocol.py

@@ -332,16 +332,6 @@ class TestRunnerContextProtocolV1:
         assert not hasattr(ctx, 'bootstrap')
 
 
-class TestHostManagedHistoryNotInProtocol:
-    """Test that Host-managed history payloads are not in Protocol v1."""
-
-    def test_messages_not_in_sdk_context_top_level(self):
-        """RunnerContext should not expose top-level history messages."""
-        ctx_fields = RunnerContext.model_fields.keys()
-
-        assert 'messages' not in ctx_fields
-
-
 class TestSDKResultProtocolV1:
     """Test SDK RunnerResult for Protocol v1."""
 

tests/unit_tests/api/service/test_apikey_service.py

@@ -347,11 +347,6 @@ class TestApiKeyServiceVerifyApiKey:
 
         assert await service.verify_api_key('lbk_invalid_key') is False
 
-    async def test_verify_api_key_empty_string(self, api_key_context):
-        _application, service, _context, _engine = api_key_context
-
-        assert await service.verify_api_key('') is False
-
     async def test_verify_api_key_unknown_key(self, api_key_context):
         _application, service, _context, _engine = api_key_context
 

tests/unit_tests/api/service/test_pipeline_migration_pending_interactions.py

@@ -23,12 +23,6 @@ env = existing.env
 NATIVE = 'langbot.pkg.provider.runners.difysvapi'
 
 
-@pytest.fixture(autouse=True)
-async def interaction_schema(env):
-    async with env.engine.begin() as conn:
-        await conn.run_sync(lambda c: AgentInteraction.__table__.create(c, checkfirst=True))
-
-
 async def add_request(env, status='pending', workspace=WS, pipeline='one'):
     async with env.engine.begin() as conn:
         await conn.execute(
@@ -224,11 +218,3 @@ async def test_native_form_arriving_during_cas_rolls_back(env, monkeypatch):
     configs, snapshots = await rows(env)
     assert configs['one'] == SOURCE and not snapshots
     env.ap.pipeline_mgr.publish_pipeline.assert_not_called()
-
-
-# Also exercise the unchanged regression assertions with this module's
-# interaction-schema fixture. The original tests remain in their own module.
-
-for _name, _test in vars(existing).items():
-    if _name.startswith('test_') and callable(_test):
-        globals()['test_existing_' + _name[5:]] = _test

tests/unit_tests/box/test_box_service.py

@@ -1884,21 +1884,6 @@ class TestBuildSkillExecutionMounts:
         with pytest.raises(ValueError, match='no immutable package root'):
             skill_loader.build_execution_mounts(app, query)
 
-    def test_empty_package_root_skipped_even_when_not_shared(self):
-        """An empty package_root is always invalid regardless of topology."""
-        logger = Mock()
-        skills = {
-            'no_root': {
-                'name': 'no_root',
-                'package_root': '',
-                'manifest_path': '',
-                'revision': 'sha256:' + '1' * 64,
-            }
-        }
-        app = self._make_app(logger, skills)
-
-        assert skill_loader.build_execution_mounts(app, make_query()) == []
-
     def test_returns_empty_when_no_skill_manager(self):
         logger = Mock()
         app = make_app(logger)

tests/unit_tests/core/test_stage.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import inspect
 from unittest.mock import MagicMock
 import pytest
 
@@ -125,25 +126,15 @@ class TestBootingStageAbstract:
             # Abstract class has name attribute defined as None
             assert hasattr(BootingStage, 'name')
 
-    @pytest.mark.asyncio
-    async def test_booting_stage_run_signature(self):
-        """run method receives Application parameter."""
+    def test_booting_stage_run_signature(self):
+        """The production abstract stage accepts an Application asynchronously."""
         mocks = self._make_stage_import_mocks()
 
         with isolated_sys_modules(mocks):
             from langbot.pkg.core.stage import BootingStage
 
-            class TestStage(BootingStage):
-                name = 'TestStage'
-
-                async def run(self, ap):
-                    self.ap_received = ap
-
-            stage = TestStage()
-            mock_ap = MagicMock()
-
-            await stage.run(mock_ap)
-            assert stage.ap_received == mock_ap
+            assert list(inspect.signature(BootingStage.run).parameters) == ['self', 'ap']
+            assert inspect.iscoroutinefunction(BootingStage.run)
 
 
 class TestPreregisteredStages:
@@ -160,19 +151,3 @@ class TestPreregisteredStages:
             from langbot.pkg.core.stage import preregistered_stages
 
             assert isinstance(preregistered_stages, dict)
-
-    def test_preregistered_stages_key_is_string(self):
-        """Registry keys are stage names (strings)."""
-        mocks = self._make_stage_import_mocks()
-
-        with isolated_sys_modules(mocks):
-            from langbot.pkg.core.stage import stage_class, preregistered_stages
-
-            preregistered_stages.clear()
-
-            @stage_class('MyStage')
-            class MyStage:
-                pass
-
-            for key in preregistered_stages:
-                assert isinstance(key, str)

tests/unit_tests/persistence/test_database_decorator.py

@@ -13,7 +13,7 @@ from __future__ import annotations
 import sys
 from contextlib import contextmanager
 from typing import Generator
-from unittest.mock import MagicMock, Mock
+from unittest.mock import MagicMock
 
 import pytest
 
@@ -174,40 +174,3 @@ class TestManagerClassDecorator:
         # BaseDatabaseManager has name annotation (type hint)
         # Check __annotations__ for the type hint
         assert 'name' in database.BaseDatabaseManager.__annotations__
-
-    def test_decorated_class_inherits_from_base(self):
-        """Test that decorated class properly inherits BaseDatabaseManager."""
-        database = get_database_module()
-
-        database.preregistered_managers.clear()
-
-        @database.manager_class('test_inherit')
-        class TestChild(database.BaseDatabaseManager):
-            async def initialize(self):
-                pass
-
-        assert issubclass(TestChild, database.BaseDatabaseManager)
-        # Has abstract method requirement satisfied
-        assert hasattr(TestChild, 'initialize')
-
-    def test_decorator_preserves_class_methods(self):
-        """Test that decorator preserves existing class methods."""
-        database = get_database_module()
-
-        database.preregistered_managers.clear()
-
-        @database.manager_class('preserve_test')
-        class ManagerWithMethods(database.BaseDatabaseManager):
-            custom_attr = 'test_value'
-
-            async def initialize(self):
-                pass
-
-            def custom_method(self):
-                return self.custom_attr
-
-        assert ManagerWithMethods.custom_attr == 'test_value'
-        # Create instance to test method (with mock app)
-        mock_app = Mock()
-        instance = ManagerWithMethods(mock_app)
-        assert instance.custom_method() == 'test_value'

tests/unit_tests/plugin/test_connector_pure.py

@@ -147,13 +147,6 @@ class TestParsePluginId:
         assert author == 'myauthor'
         assert name == 'myplugin'
 
-    def test_parse_plugin_id_empty(self):
-        """Empty plugin ID is invalid."""
-        from langbot.pkg.plugin.connector import PluginRuntimeConnector
-
-        with pytest.raises(ValueError):
-            PluginRuntimeConnector._parse_plugin_id('')
-
 
 @pytest.mark.asyncio
 async def test_marketplace_response_reader_is_bounded():

tests/unit_tests/plugin/test_plugin_id_parsing.py

@@ -5,10 +5,6 @@ import pytest
 from langbot.pkg.plugin.connector import PluginRuntimeConnector
 
 
-def test_parse_plugin_id_accepts_author_name():
-    assert PluginRuntimeConnector._parse_plugin_id('langbot/rag-engine') == ('langbot', 'rag-engine')
-
-
 @pytest.mark.parametrize(
     'plugin_id',
     [

tests/unit_tests/storage/test_s3storage.py

@@ -57,14 +57,6 @@ def s3_mock():
 class TestS3StorageProviderInit:
     """Tests for S3StorageProvider initialization."""
 
-    def test_init_stores_app_reference(self):
-        """Test that __init__ stores the Application reference."""
-        s3storage = get_s3storage_module()
-
-        mock_app = Mock()
-        provider = s3storage.S3StorageProvider(mock_app)
-        assert provider.ap is mock_app
-
     def test_init_s3_client_none(self):
         """Test that s3_client starts as None."""
         s3storage = get_s3storage_module()
@@ -232,7 +224,7 @@ class TestS3StorageProviderWithMoto:
 
     @pytest.mark.asyncio
     async def test_delete_dir_recursive_handles_trailing_slash(self, mock_app_with_s3_config, s3_mock):
-        """Test that delete_dir_recursive handles path without trailing slash."""
+        """Test that delete_dir_recursive handles a path with a trailing slash."""
         s3storage = get_s3storage_module()
 
         provider = s3storage.S3StorageProvider(mock_app_with_s3_config)
@@ -241,8 +233,7 @@ class TestS3StorageProviderWithMoto:
         # Save object
         await provider.save('mydir/file.txt', b'data')
 
-        # Delete without trailing slash
-        await provider.delete_dir_recursive('mydir')
+        await provider.delete_dir_recursive('mydir/')
 
         # Verify deleted
         assert await provider.exists('mydir/file.txt') is False

tests/unit_tests/storage/test_storage_manager.py

@@ -9,7 +9,6 @@ from unittest.mock import Mock, AsyncMock, patch
 
 from langbot.pkg.storage.mgr import StorageMgr
 from langbot.pkg.storage.providers.localstorage import LocalStorageProvider
-from langbot.pkg.storage.providers.s3storage import S3StorageProvider
 
 
 class TestStorageMgr:
@@ -21,77 +20,6 @@ class TestStorageMgr:
         storage_mgr = StorageMgr(mock_app)
         assert storage_mgr.ap == mock_app
 
-    @pytest.mark.asyncio
-    async def test_initialize_default_local(self):
-        """Should use local storage by default."""
-        mock_app = Mock()
-        mock_app.instance_config = Mock()
-        mock_app.instance_config.data = {}
-        mock_app.logger = Mock()
-
-        storage_mgr = StorageMgr(mock_app)
-
-        with patch.object(LocalStorageProvider, 'initialize', new_callable=AsyncMock):
-            await storage_mgr.initialize()
-            assert isinstance(storage_mgr.storage_provider, LocalStorageProvider)
-            mock_app.logger.info.assert_called()
-
-    @pytest.mark.asyncio
-    async def test_initialize_with_explicit_local(self):
-        """Should use local storage when explicitly configured."""
-        mock_app = Mock()
-        mock_app.instance_config = Mock()
-        mock_app.instance_config.data = {'storage': {'use': 'local'}}
-        mock_app.logger = Mock()
-
-        storage_mgr = StorageMgr(mock_app)
-
-        with patch.object(LocalStorageProvider, 'initialize', new_callable=AsyncMock):
-            await storage_mgr.initialize()
-            assert isinstance(storage_mgr.storage_provider, LocalStorageProvider)
-
-    @pytest.mark.asyncio
-    async def test_initialize_with_s3(self):
-        """Should use S3 storage when configured."""
-        mock_app = Mock()
-        mock_app.instance_config = Mock()
-        mock_app.instance_config.data = {'storage': {'use': 's3', 's3': {'endpoint_url': 'https://s3.amazonaws.com'}}}
-        mock_app.logger = Mock()
-
-        storage_mgr = StorageMgr(mock_app)
-
-        with patch.object(S3StorageProvider, 'initialize', new_callable=AsyncMock):
-            await storage_mgr.initialize()
-            assert isinstance(storage_mgr.storage_provider, S3StorageProvider)
-
-    @pytest.mark.asyncio
-    async def test_initialize_invalid_type_defaults_to_local(self):
-        """Should default to local storage for invalid storage type."""
-        mock_app = Mock()
-        mock_app.instance_config = Mock()
-        mock_app.instance_config.data = {'storage': {'use': 'invalid_type'}}
-        mock_app.logger = Mock()
-
-        storage_mgr = StorageMgr(mock_app)
-
-        with patch.object(LocalStorageProvider, 'initialize', new_callable=AsyncMock):
-            await storage_mgr.initialize()
-            assert isinstance(storage_mgr.storage_provider, LocalStorageProvider)
-
-    @pytest.mark.asyncio
-    async def test_initialize_calls_provider_initialize(self):
-        """Should call the provider's initialize method."""
-        mock_app = Mock()
-        mock_app.instance_config = Mock()
-        mock_app.instance_config.data = {}
-        mock_app.logger = Mock()
-
-        storage_mgr = StorageMgr(mock_app)
-
-        with patch.object(LocalStorageProvider, 'initialize', new_callable=AsyncMock) as mock_init:
-            await storage_mgr.initialize()
-            mock_init.assert_called_once()
-
     @pytest.mark.asyncio
     async def test_shutdown_delegates_to_active_provider(self):
         storage_mgr = StorageMgr(Mock())