fix(arch-043,arch-052): deterministic system tenant lookup; async-safe file metadata
Check Cross-Plugin Imports / check (push) Has been cancelled
Check Cross-Plugin Imports / check (push) Has been cancelled
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
+40
-12
@@ -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))
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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}
|
||||
Reference in New Issue
Block a user