From 17516d2783e0529e1da9f983211ee4c4d4224f20 Mon Sep 17 00:00:00 2001 From: Agent Zero Date: Sun, 23 Aug 2026 16:39:09 +0200 Subject: [PATCH] fix(arch-043,arch-052): deterministic system tenant lookup; async-safe file metadata --- app/config.py | 4 + app/core/db/__init__.py | 17 +++ app/core/storage.py | 52 +++++++-- app/plugins/builtins/automation/plugin.py | 17 +-- tests/test_arch_block_a.py | 131 ++++++++++++++++++++++ 5 files changed, 201 insertions(+), 20 deletions(-) create mode 100644 tests/test_arch_block_a.py diff --git a/app/config.py b/app/config.py index 3fe487e..acac5e9 100644 --- a/app/config.py +++ b/app/config.py @@ -107,6 +107,10 @@ class Settings(BaseSettings): rate_limit_webhook_max: int = 100 # incoming webhooks rate_limit_webhook_window: int = 60 # 1 minute + # System tenant — used by seeding/plugins that need a well-known default + # tenant (must match scripts/seed_admin.py slug). + system_tenant_slug: str = "default" + # LLM Cost Overrun Protection (B.17) llm_monthly_budget_usd: float = 100.0 # per-tenant monthly LLM budget llm_hard_cutoff: bool = True # block LLM calls when budget exceeded diff --git a/app/core/db/__init__.py b/app/core/db/__init__.py index a069556..694b37a 100644 --- a/app/core/db/__init__.py +++ b/app/core/db/__init__.py @@ -355,6 +355,23 @@ async def close_engine() -> None: _migration_session_factory = None +async def get_system_tenant(db: AsyncSession): + """Return the well-known system tenant, or ``None`` if it does not exist. + + Resolves by configured slug (``settings.system_tenant_slug``, default + ``"default"`` as created by ``scripts/seed_admin.py``) instead of an + arbitrary first row, so multi-tenant databases stay deterministic. + """ + from sqlalchemy import select + + from app.config import get_settings + from app.models.tenant import Tenant # lazy: models import this module's Base + + slug = get_settings().system_tenant_slug + result = await db.execute(select(Tenant).where(Tenant.slug == slug).limit(1)) + return result.scalar_one_or_none() + + def reset_engine_for_testing(engine: AsyncEngine) -> async_sessionmaker[AsyncSession]: """Replace all global engines with a test engine. Returns a session factory. diff --git a/app/core/storage.py b/app/core/storage.py index bfcb117..9a2a408 100644 --- a/app/core/storage.py +++ b/app/core/storage.py @@ -501,11 +501,38 @@ async def save_with_metadata( } +async def get_file_metadata_async(path: str) -> dict[str, Any]: + """Awaitable variant of :func:`get_file_metadata` (ARCH-052). + + Safe to call from inside a running event loop — never creates a + nested one. For local storage this is plain filesystem access; for + S3 and other async backends the backend's ``exists()`` is awaited. + """ + backend = get_storage_backend() + if isinstance(backend, LocalStorage): + full_path = backend._full_path(path) + if not os.path.exists(full_path): + return {"size": None, "modified": None, "exists": False} + stat = os.stat(full_path) + return { + "size": stat.st_size, + "modified": stat.st_mtime, + "exists": True, + } + # S3 or other async backends — await the backend directly + if not await backend.exists(path): + return {"size": None, "modified": None, "exists": False} + return {"size": None, "modified": None, "exists": True} + + def get_file_metadata(path: str) -> dict[str, Any]: """Read metadata of a stored file without loading its content. - Works with the *local* storage backend. For S3, use the S3 client - ``stat_object`` API directly. + Works with the *local* storage backend without touching the event + loop. For S3 and other async-only backends this drives the check + through ``asyncio.run``; calling it from inside a running event loop + raises ``RuntimeError`` — use :func:`get_file_metadata_async` there + instead (ARCH-052). Parameters ---------- @@ -530,14 +557,15 @@ def get_file_metadata(path: str) -> dict[str, Any]: "modified": stat.st_mtime, "exists": True, } - # S3 or other backends — fall back to exists() check - import asyncio as _asyncio - - loop = _asyncio.new_event_loop() + # Async-only backend outside a running loop is fine; inside one we + # must never build a nested event loop. try: - exists = loop.run_until_complete(backend.exists(path)) - if not exists: - return {"size": None, "modified": None, "exists": False} - return {"size": None, "modified": None, "exists": True} - finally: - loop.close() + asyncio.get_running_loop() + except RuntimeError: + pass + else: + raise RuntimeError( + "get_file_metadata() cannot be used with async storage backends " + "inside a running event loop — use get_file_metadata_async()" + ) + return asyncio.run(get_file_metadata_async(path)) diff --git a/app/plugins/builtins/automation/plugin.py b/app/plugins/builtins/automation/plugin.py index 10114b8..6fe45b3 100644 --- a/app/plugins/builtins/automation/plugin.py +++ b/app/plugins/builtins/automation/plugin.py @@ -246,11 +246,12 @@ class AutomationPlugin(BasePlugin): from app.plugins.builtins.automation.prebuilt.report_agent import create_report_agent from sqlalchemy import select as sa_select - # Get first tenant + admin user for seeding + # Get system tenant + admin user for seeding (ARCH-043: + # deterministic slug lookup instead of arbitrary first row) + from app.core.db import get_system_tenant from app.models.user import User - from app.models.tenant import Tenant - tenant_result = await db.execute(sa_select(Tenant).limit(1)) - tenant = tenant_result.scalar_one_or_none() + + tenant = await get_system_tenant(db) if tenant: user_result = await db.execute( sa_select(User) @@ -418,16 +419,16 @@ class AutomationPlugin(BasePlugin): from another plugin's manifest. Uses plugin name prefixing for conflict resolution.""" from sqlalchemy import select - # Get default tenant_id from the first tenant in the DB - from app.models.tenant import Tenant + # Get system tenant for contributions (ARCH-043: deterministic slug + # lookup instead of arbitrary first row) + from app.core.db import get_system_tenant from app.plugins.builtins.automation.models import AutomationCronJob from app.plugins.builtins.automation.services import ( AgentService, AutomationService, CronJobService, ) - tenant_result = await db.execute(select(Tenant).limit(1)) - tenant = tenant_result.scalar_one_or_none() + tenant = await get_system_tenant(db) default_tenant_id = tenant.id if tenant else None if default_tenant_id is None: logger.warning("No tenant found — skipping plugin contributions registration") diff --git a/tests/test_arch_block_a.py b/tests/test_arch_block_a.py new file mode 100644 index 0000000..e8f720c --- /dev/null +++ b/tests/test_arch_block_a.py @@ -0,0 +1,131 @@ +# Regression tests for Block A fixes: ARCH-043 (system tenant) and ARCH-052 (storage metadata). + +from __future__ import annotations + +import asyncio +import uuid + +import pytest + + +# --- ARCH-043: get_system_tenant resolves by configured slug --- + + +class TestGetSystemTenant: + @pytest.mark.asyncio + async def test_resolves_by_slug_not_first_row(self, db_session): + from sqlalchemy import select + + from app.config import get_settings + from app.core.db import get_system_tenant + from app.models.tenant import Tenant + + settings = get_settings() + slug = settings.system_tenant_slug + + # Insert a decoy first and the real system tenant second, + # so a limit(1)-style query would pick the wrong row. + decoy = Tenant(name='Decoy Org', slug=f'decoy-{uuid.uuid4().hex[:8]}') + db_session.add(decoy) + system = Tenant(name='Default Org', slug=slug) + db_session.add(system) + await db_session.commit() + + try: + tenant = await get_system_tenant(db_session) + assert tenant is not None + assert tenant.slug == slug + # sanity: decoy really is the first row + first = (await db_session.execute(select(Tenant).limit(1))).scalar_one() + assert first.id != tenant.id + finally: + await db_session.delete(decoy) + await db_session.delete(system) + await db_session.commit() + + @pytest.mark.asyncio + async def test_returns_none_when_missing(self, db_session): + from sqlalchemy import delete + + from app.core.db import get_system_tenant + from app.models.tenant import Tenant + + await db_session.execute(delete(Tenant)) + await db_session.commit() + + tenant = await get_system_tenant(db_session) + assert tenant is None + + +# --- ARCH-052: async metadata without nested event loops --- + + +@pytest.fixture +def tmp_storage(tmp_path, monkeypatch): + from app.core.storage import LocalStorage, reset_storage_backend + + monkeypatch.setenv('STORAGE_PATH', str(tmp_path)) + monkeypatch.setenv('STORAGE_BACKEND', 'local') + reset_storage_backend() + backend = LocalStorage(base_path=str(tmp_path)) + yield backend + reset_storage_backend() + + +class TestGetFileMetadataAsync: + def _use_local(self, tmp_storage, monkeypatch): + monkeypatch.setenv('STORAGE_PATH', str(tmp_storage.base_path)) + monkeypatch.setenv('STORAGE_BACKEND', 'local') + from app.core.storage import reset_storage_backend + + reset_storage_backend() + import app.core.storage as storage_mod + + storage_mod._storage_backend = tmp_storage + + def teardown_method(self): + from app.core.storage import reset_storage_backend + + reset_storage_backend() + + @pytest.mark.asyncio + async def test_awaitable_inside_running_loop(self, tmp_storage, monkeypatch): + self._use_local(tmp_storage, monkeypatch) + + from app.core.storage import get_file_metadata_async + + await tmp_storage.save('meta/async.txt', b'async metadata') + + meta = await get_file_metadata_async('meta/async.txt') + assert meta['exists'] is True + assert meta['size'] == len(b'async metadata') + assert meta['modified'] is not None + + @pytest.mark.asyncio + async def test_non_existing_inside_running_loop(self, tmp_storage, monkeypatch): + self._use_local(tmp_storage, monkeypatch) + + from app.core.storage import get_file_metadata_async + + meta = await get_file_metadata_async('nonexistent/async.txt') + assert meta['exists'] is False + assert meta['size'] is None + assert meta['modified'] is None + + def test_sync_wrapper_still_works(self, tmp_storage, monkeypatch): + self._use_local(tmp_storage, monkeypatch) + + from app.core.storage import get_file_metadata + + asyncio.run(tmp_storage.save('meta/sync.txt', b'sync metadata')) + meta = get_file_metadata('meta/sync.txt') + assert meta['exists'] is True + assert meta['size'] == len(b'sync metadata') + + def test_sync_wrapper_non_existing(self, tmp_storage, monkeypatch): + self._use_local(tmp_storage, monkeypatch) + + from app.core.storage import get_file_metadata + + meta = get_file_metadata('nonexistent/sync.txt') + assert meta == {'size': None, 'modified': None, 'exists': False}