From fe7e2dc62f44e0a0db84f6eaec282e31a81b9045 Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Wed, 5 Aug 2026 17:43:40 +0900 Subject: [PATCH] test: use sqlite3 session in test_agent_config_service (#38764) --- api/services/agent_config_service.py | 32 +- .../services/test_agent_config_service.py | 545 +++++++++++------- 2 files changed, 345 insertions(+), 232 deletions(-) diff --git a/api/services/agent_config_service.py b/api/services/agent_config_service.py index ded7570eb0f..fdc13eca2d8 100644 --- a/api/services/agent_config_service.py +++ b/api/services/agent_config_service.py @@ -16,6 +16,7 @@ from __future__ import annotations import io import urllib.parse import zipfile +from collections.abc import Callable from dataclasses import dataclass from enum import StrEnum from operator import itemgetter @@ -28,7 +29,7 @@ from sqlalchemy.exc import DataError, SQLAlchemyError from sqlalchemy.orm import Session from core.app.file_access.controller import DatabaseFileAccessController -from core.db.session_factory import session_factory +from core.db.session_factory import session_factory as default_session_factory from core.tools.tool_file_manager import ToolFileManager from extensions.ext_storage import storage from factories import file_factory @@ -114,18 +115,29 @@ class ConfigDownload: class AgentConfigService: - """Read and update Agent Soul-backed config assets for one version target.""" + """Read and update Agent Soul-backed config assets for one version target. + + The service owns the lifecycle of its database sessions. Callers may inject + a session creator for an alternate engine; production defaults to the + application-wide session factory. + """ PREVIEW_MAX_BYTES = 64 * 1024 + _session_factory: Callable[[], Session] + def __init__( self, *, tool_file_manager: ToolFileManager | None = None, skill_normalize_service: ConfigSkillNormalizeService | None = None, + session_factory: Callable[[], Session] | None = None, ) -> None: + """Initialize external collaborators and the service-owned session creator.""" + self._tool_files = tool_file_manager or ToolFileManager() self._skill_normalizer = skill_normalize_service or ConfigSkillNormalizeService() + self._session_factory = session_factory or default_session_factory.create_session def resolve_target( self, @@ -136,7 +148,7 @@ class AgentConfigService: config_version_kind: AgentConfigVersionKind, user_id: str | None = None, ) -> AgentConfigTarget: - with session_factory.create_session() as session: + with self._session_factory() as session: target = self._resolve_target_in_session( session, tenant_id=tenant_id, @@ -494,7 +506,7 @@ class AgentConfigService: filename: str, surface: AgentConfigMutationSurface, ) -> dict[str, object]: - with session_factory.create_session() as session: + with self._session_factory() as session: target = self._resolve_target_in_session( session, tenant_id=tenant_id, @@ -579,7 +591,7 @@ class AgentConfigService: config_version_kind: AgentConfigVersionKind, upload_file_id: str, ) -> dict[str, object]: - with session_factory.create_session() as session: + with self._session_factory() as session: target = self._resolve_target_in_session( session, tenant_id=tenant_id, @@ -615,7 +627,7 @@ class AgentConfigService: payload: ConfigPushPayload, surface: AgentConfigMutationSurface, ) -> dict[str, object]: - with session_factory.create_session() as session: + with self._session_factory() as session: target = self._resolve_target_in_session( session, tenant_id=tenant_id, @@ -709,7 +721,7 @@ class AgentConfigService: env_text: str, surface: AgentConfigMutationSurface, ) -> dict[str, object]: - with session_factory.create_session() as session: + with self._session_factory() as session: target = self._resolve_target_in_session( session, tenant_id=tenant_id, @@ -756,7 +768,7 @@ class AgentConfigService: note: str, surface: AgentConfigMutationSurface, ) -> dict[str, object]: - with session_factory.create_session() as session: + with self._session_factory() as session: target = self._resolve_target_in_session( session, tenant_id=tenant_id, @@ -1339,7 +1351,7 @@ class AgentConfigService: return file_ref.file_id def _load_tool_file_bytes(self, *, tenant_id: str, file_id: str) -> tuple[bytes, str | None]: - with session_factory.create_session() as session: + with self._session_factory() as session: tool_file = session.scalar(select(ToolFile).where(ToolFile.id == file_id, ToolFile.tenant_id == tenant_id)) if tool_file is None: raise AgentConfigServiceError("config_skill_not_found", "config skill payload is missing", status_code=404) @@ -1352,7 +1364,7 @@ class AgentConfigService: file_kind: Literal["upload_file", "tool_file"], file_id: str, ) -> tuple[bytes, str | None, str | None]: - with session_factory.create_session() as session: + with self._session_factory() as session: if file_kind == "tool_file": tool_file = session.scalar( select(ToolFile).where(ToolFile.id == file_id, ToolFile.tenant_id == tenant_id) diff --git a/api/tests/unit_tests/services/test_agent_config_service.py b/api/tests/unit_tests/services/test_agent_config_service.py index 4b1313be7c6..09cf7d90b8c 100644 --- a/api/tests/unit_tests/services/test_agent_config_service.py +++ b/api/tests/unit_tests/services/test_agent_config_service.py @@ -4,11 +4,22 @@ from __future__ import annotations import io import zipfile +from datetime import datetime from types import SimpleNamespace -from unittest.mock import MagicMock, patch +from unittest.mock import patch import pytest +from sqlalchemy.orm import Session, sessionmaker +from extensions.storage.storage_type import StorageType +from models.agent import ( + Agent, + AgentConfigDraft, + AgentConfigDraftType, + AgentConfigSnapshot, + AgentScope, + AgentSource, +) from models.agent_config_entities import ( AgentConfigFileRefConfig, AgentConfigSkillRefConfig, @@ -16,6 +27,9 @@ from models.agent_config_entities import ( AgentFileRefConfig, AgentSoulConfig, ) +from models.enums import CreatorUserRole +from models.model import UploadFile +from models.tools import ToolFile from services.agent.skill_package_service import SkillPackageError from services.agent_config_service import ( AgentConfigService, @@ -27,16 +41,20 @@ from services.agent_config_service import ( ) MODULE = "services.agent_config_service" -TENANT = "tenant-1" -AGENT = "agent-1" -USER = "user-1" +TENANT = "11111111-1111-1111-1111-111111111111" +OTHER_TENANT = "22222222-2222-2222-2222-222222222222" +AGENT = "33333333-3333-3333-3333-333333333333" +USER = "44444444-4444-4444-4444-444444444444" +END_USER = "55555555-5555-5555-5555-555555555555" +SNAPSHOT = "66666666-6666-6666-6666-666666666666" +DRAFT = "77777777-7777-7777-7777-777777777777" +BUILD_DRAFT = "88888888-8888-8888-8888-888888888888" +TOOL_FILE = "99999999-9999-9999-9999-999999999999" +SKILL_FILE = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" +UPLOAD_FILE = "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb" +NORMALIZED_SKILL_FILE = "cccccccc-cccc-cccc-cccc-cccccccccccc" - -def _session_cm(session: MagicMock) -> MagicMock: - context_manager = MagicMock() - context_manager.__enter__.return_value = session - context_manager.__exit__.return_value = None - return context_manager +AGENT_CONFIG_TABLES = (Agent, AgentConfigDraft, AgentConfigSnapshot) def _soul(**updates) -> AgentSoulConfig: @@ -45,6 +63,62 @@ def _soul(**updates) -> AgentSoulConfig: return AgentSoulConfig.model_validate(payload) +def _agent(*, tenant_id: str = TENANT) -> Agent: + return Agent( + id=AGENT, + tenant_id=tenant_id, + name="Config Agent", + scope=AgentScope.ROSTER, + source=AgentSource.ROSTER, + ) + + +def _draft( + *, + version_id: str = DRAFT, + draft_type: AgentConfigDraftType = AgentConfigDraftType.DRAFT, + account_id: str | None = None, + soul: AgentSoulConfig | None = None, +) -> AgentConfigDraft: + return AgentConfigDraft( + id=version_id, + tenant_id=TENANT, + agent_id=AGENT, + draft_type=draft_type, + account_id=account_id, + draft_owner_key=account_id or "", + config_snapshot=soul or _soul(), + ) + + +def _snapshot(*, soul: AgentSoulConfig | None = None) -> AgentConfigSnapshot: + return AgentConfigSnapshot( + id=SNAPSHOT, + tenant_id=TENANT, + agent_id=AGENT, + version=1, + config_snapshot=soul or _soul(), + ) + + +def _service(sqlite_session: Session) -> AgentConfigService: + """Bind service-owned sessions to the current test's isolated SQLite engine.""" + + return AgentConfigService( + session_factory=sessionmaker(bind=sqlite_session.get_bind(), expire_on_commit=False), + ) + + +def _persist_target( + sqlite_session: Session, + version: AgentConfigDraft | AgentConfigSnapshot, + *, + tenant_id: str = TENANT, +) -> None: + sqlite_session.add_all([_agent(tenant_id=tenant_id), version]) + sqlite_session.commit() + + def _version(*, version_id: str = "version-1", snapshot: AgentSoulConfig | None = None) -> SimpleNamespace: agent_soul = snapshot or _soul() return SimpleNamespace( @@ -83,301 +157,312 @@ def _zip_bytes(members: dict[str, bytes]) -> bytes: @pytest.mark.parametrize( - ("kind", "user_id", "version_row", "expected_writable"), + ("kind", "user_id", "version_id", "expected_writable"), [ - (AgentConfigVersionKind.SNAPSHOT, None, _version(version_id="snapshot-1"), False), - (AgentConfigVersionKind.DRAFT, USER, _version(version_id="draft-1"), False), - (AgentConfigVersionKind.BUILD_DRAFT, USER, _version(version_id="build-draft-1"), True), + (AgentConfigVersionKind.SNAPSHOT, None, SNAPSHOT, False), + (AgentConfigVersionKind.DRAFT, USER, DRAFT, False), + (AgentConfigVersionKind.BUILD_DRAFT, USER, BUILD_DRAFT, True), ], ) +@pytest.mark.parametrize("sqlite_session", [AGENT_CONFIG_TABLES], indirect=True) def test_resolve_target_supports_snapshot_draft_and_build_draft( kind: AgentConfigVersionKind, user_id: str | None, - version_row: SimpleNamespace, + version_id: str, expected_writable: bool, + sqlite_session: Session, ) -> None: - session = MagicMock() - session.scalar.side_effect = [AGENT, version_row] - service = AgentConfigService() - - with patch(f"{MODULE}.session_factory.create_session", return_value=_session_cm(session)): - target = service.resolve_target( - tenant_id=TENANT, - agent_id=AGENT, - config_version_id=version_row.id, - config_version_kind=kind, - user_id=user_id, + if kind == AgentConfigVersionKind.SNAPSHOT: + version = _snapshot() + else: + version = _draft( + version_id=version_id, + draft_type=( + AgentConfigDraftType.DEBUG_BUILD + if kind == AgentConfigVersionKind.BUILD_DRAFT + else AgentConfigDraftType.DRAFT + ), + account_id=USER if kind == AgentConfigVersionKind.BUILD_DRAFT else None, ) + _persist_target(sqlite_session, version) + + target = _service(sqlite_session).resolve_target( + tenant_id=TENANT, + agent_id=AGENT, + config_version_id=version_id, + config_version_kind=kind, + user_id=user_id, + ) assert target.agent_id == AGENT - assert target.version_id == version_row.id + assert target.version_id == version_id assert target.kind == kind assert target.writable is expected_writable + assert target.agent_soul == _soul() -def test_resolve_target_requires_user_for_build_draft() -> None: - session = MagicMock() - session.scalar.side_effect = [AGENT] - service = AgentConfigService() +@pytest.mark.parametrize("sqlite_session", [AGENT_CONFIG_TABLES], indirect=True) +def test_resolve_target_requires_user_for_build_draft(sqlite_session: Session) -> None: + _persist_target( + sqlite_session, + _draft( + version_id=BUILD_DRAFT, + draft_type=AgentConfigDraftType.DEBUG_BUILD, + account_id=USER, + ), + ) - with patch(f"{MODULE}.session_factory.create_session", return_value=_session_cm(session)): - with pytest.raises(AgentConfigServiceError, match="user_id is required") as exc_info: - service.resolve_target( - tenant_id=TENANT, - agent_id=AGENT, - config_version_id="build-draft-1", - config_version_kind=AgentConfigVersionKind.BUILD_DRAFT, - ) + with pytest.raises(AgentConfigServiceError, match="user_id is required") as exc_info: + _service(sqlite_session).resolve_target( + tenant_id=TENANT, + agent_id=AGENT, + config_version_id=BUILD_DRAFT, + config_version_kind=AgentConfigVersionKind.BUILD_DRAFT, + ) assert exc_info.value.code == "missing_user_id" @pytest.mark.parametrize( - ("first_scalar", "expected_code"), + ("agent_tenant_id", "expected_code"), [ - (None, "agent_not_found"), - (AGENT, "config_version_not_found"), + (OTHER_TENANT, "agent_not_found"), + (TENANT, "config_version_not_found"), ], ) -def test_resolve_target_maps_missing_agent_and_version(first_scalar: str | None, expected_code: str) -> None: - session = MagicMock() - if first_scalar is None: - session.scalar.return_value = None - else: - session.scalar.side_effect = [first_scalar, None] - service = AgentConfigService() +@pytest.mark.parametrize("sqlite_session", [AGENT_CONFIG_TABLES], indirect=True) +def test_resolve_target_maps_missing_agent_and_version( + agent_tenant_id: str, + expected_code: str, + sqlite_session: Session, +) -> None: + sqlite_session.add(_agent(tenant_id=agent_tenant_id)) + sqlite_session.commit() - with patch(f"{MODULE}.session_factory.create_session", return_value=_session_cm(session)): - with pytest.raises(AgentConfigServiceError) as exc_info: - service.resolve_target( - tenant_id=TENANT, - agent_id=AGENT, - config_version_id="missing", - config_version_kind=AgentConfigVersionKind.SNAPSHOT, - user_id=USER, - ) + with pytest.raises(AgentConfigServiceError) as exc_info: + _service(sqlite_session).resolve_target( + tenant_id=TENANT, + agent_id=AGENT, + config_version_id=SNAPSHOT, + config_version_kind=AgentConfigVersionKind.SNAPSHOT, + user_id=USER, + ) assert exc_info.value.code == expected_code -def test_push_rejects_non_build_draft_writes() -> None: - session = MagicMock() - service = AgentConfigService() +@pytest.mark.parametrize("sqlite_session", [AGENT_CONFIG_TABLES], indirect=True) +def test_push_rejects_non_build_draft_writes(sqlite_session: Session) -> None: + _persist_target(sqlite_session, _draft(soul=_soul(config_note="before"))) - with ( - patch(f"{MODULE}.session_factory.create_session", return_value=_session_cm(session)), - patch.object( - service, - "_resolve_target_in_session", - return_value=_target(kind=AgentConfigVersionKind.DRAFT, writable=False), - ), - ): - with pytest.raises(AgentConfigServiceError, match="build drafts") as exc_info: - service.push( - tenant_id=TENANT, - agent_id=AGENT, - user_id=USER, - config_version_id="draft-1", - config_version_kind=AgentConfigVersionKind.DRAFT, - payload=ConfigPushPayload(note="ignored"), - ) - - assert exc_info.value.code == "config_not_writable" - session.commit.assert_not_called() - - -def test_push_for_console_allows_shared_draft_mutations() -> None: - session = MagicMock() - service = AgentConfigService() - target = _target(kind=AgentConfigVersionKind.DRAFT, writable=False, soul=_soul(config_note="before")) - - with ( - patch(f"{MODULE}.session_factory.create_session", return_value=_session_cm(session)), - patch.object(service, "_resolve_target_in_session", return_value=target), - ): - manifest = service.push_for_console( + with pytest.raises(AgentConfigServiceError, match="build drafts") as exc_info: + _service(sqlite_session).push( tenant_id=TENANT, agent_id=AGENT, user_id=USER, - config_version_id="draft-1", + config_version_id=DRAFT, config_version_kind=AgentConfigVersionKind.DRAFT, - payload=ConfigPushPayload(note="after"), + payload=ConfigPushPayload(note="ignored"), ) + assert exc_info.value.code == "config_not_writable" + sqlite_session.expire_all() + persisted = sqlite_session.get(AgentConfigDraft, DRAFT) + assert persisted is not None + assert persisted.config_snapshot.config_note == "before" + + +@pytest.mark.parametrize("sqlite_session", [AGENT_CONFIG_TABLES], indirect=True) +def test_push_for_console_allows_shared_draft_mutations(sqlite_session: Session) -> None: + _persist_target(sqlite_session, _draft(soul=_soul(config_note="before"))) + + manifest = _service(sqlite_session).push_for_console( + tenant_id=TENANT, + agent_id=AGENT, + user_id=USER, + config_version_id=DRAFT, + config_version_kind=AgentConfigVersionKind.DRAFT, + payload=ConfigPushPayload(note="after"), + ) + assert manifest["note"] == "after" - assert target.version.config_snapshot.config_note == "after" - session.commit.assert_called_once() + sqlite_session.expire_all() + persisted = sqlite_session.get(AgentConfigDraft, DRAFT) + assert persisted is not None + assert persisted.config_snapshot.config_note == "after" -def test_push_accepts_tenant_scoped_tool_file_sources_from_different_upload_owner() -> None: - session = MagicMock() - service = AgentConfigService() - target = _target( - kind=AgentConfigVersionKind.BUILD_DRAFT, - writable=True, - soul=_soul( - config_skills=[{"name": "alpha", "file_id": "", "is_missing": True}], - config_files=[{"name": "guide.txt", "file_kind": "tool_file", "file_id": "", "is_missing": True}], +@pytest.mark.parametrize("sqlite_session", [(*AGENT_CONFIG_TABLES, ToolFile)], indirect=True) +def test_push_accepts_tenant_scoped_tool_file_sources_from_different_upload_owner( + sqlite_session: Session, +) -> None: + _persist_target( + sqlite_session, + _draft( + version_id=BUILD_DRAFT, + draft_type=AgentConfigDraftType.DEBUG_BUILD, + account_id=USER, ), ) - file_source = SimpleNamespace( - id="tool-file-file", + file_source = ToolFile( tenant_id=TENANT, - user_id="end-user-1", + user_id=END_USER, + conversation_id=None, size=7, mimetype="text/plain", file_key="file-key", name="guide.txt", ) - skill_source = SimpleNamespace( - id="tool-file-skill", + file_source.id = TOOL_FILE + skill_source = ToolFile( tenant_id=TENANT, - user_id="end-user-1", + user_id=END_USER, + conversation_id=None, size=123, mimetype="application/zip", file_key="skill-key", name="alpha.zip", ) + skill_source.id = SKILL_FILE + sqlite_session.add_all([file_source, skill_source]) + sqlite_session.commit() skill_ref = AgentConfigSkillRefConfig( name="alpha", description="Alpha skill", - file_id="normalized-skill-file", + file_id=NORMALIZED_SKILL_FILE, size=321, mime_type="application/zip", ) + service = _service(sqlite_session) with ( - patch(f"{MODULE}.session_factory.create_session", return_value=_session_cm(session)), - patch.object(service, "_resolve_target_in_session", return_value=target), - patch.object(service, "_require_tool_file_source", side_effect=[file_source, skill_source]) as require_source, patch(f"{MODULE}.storage.load_once", return_value=b"skill-archive"), - patch.object(service._skill_normalizer, "normalize", return_value=(skill_ref, object())), + patch.object( + service._skill_normalizer, + "normalize", + return_value=(skill_ref, object()), + ), ): manifest = service.push( tenant_id=TENANT, agent_id=AGENT, user_id=USER, - config_version_id="build-draft-1", + config_version_id=BUILD_DRAFT, config_version_kind=AgentConfigVersionKind.BUILD_DRAFT, payload=ConfigPushPayload.model_validate( { - "files": [{"name": "guide.txt", "file_ref": {"kind": "tool_file", "id": "tool-file-file"}}], - "skills": [{"name": "alpha", "file_ref": {"kind": "tool_file", "id": "tool-file-skill"}}], + "files": [{"name": "guide.txt", "file_ref": {"kind": "tool_file", "id": TOOL_FILE}}], + "skills": [{"name": "alpha", "file_ref": {"kind": "tool_file", "id": SKILL_FILE}}], } ), ) - assert [call.args for call in require_source.call_args_list] == [(session,), (session,)] - assert [call.kwargs for call in require_source.call_args_list] == [ - {"tenant_id": TENANT, "file_id": "tool-file-file"}, - {"tenant_id": TENANT, "file_id": "tool-file-skill"}, - ] files = manifest["files"] skills = manifest["skills"] assert isinstance(files, dict) assert isinstance(skills, dict) - assert files["items"][0]["file_id"] == "tool-file-file" - assert files["items"][0]["is_missing"] is False - assert skills["items"][0]["file_id"] == "normalized-skill-file" - assert skills["items"][0]["is_missing"] is False - session.commit.assert_called_once() + assert files["items"][0]["file_id"] == TOOL_FILE + assert skills["items"][0]["file_id"] == NORMALIZED_SKILL_FILE + sqlite_session.expire_all() + persisted = sqlite_session.get(AgentConfigDraft, BUILD_DRAFT) + assert persisted is not None + assert persisted.config_snapshot.config_files[0].file_id == TOOL_FILE + assert persisted.config_snapshot.config_skills[0].file_id == NORMALIZED_SKILL_FILE + persisted_source = sqlite_session.get(ToolFile, TOOL_FILE) + assert persisted_source is not None + assert persisted_source.user_id == END_USER -def test_push_file_for_console_rejects_snapshot_writes() -> None: - session = MagicMock() - service = AgentConfigService() +@pytest.mark.parametrize("sqlite_session", [AGENT_CONFIG_TABLES], indirect=True) +def test_push_file_for_console_rejects_snapshot_writes(sqlite_session: Session) -> None: + _persist_target(sqlite_session, _snapshot()) - with ( - patch(f"{MODULE}.session_factory.create_session", return_value=_session_cm(session)), - patch.object( - service, - "_resolve_target_in_session", - return_value=_target(kind=AgentConfigVersionKind.SNAPSHOT, writable=False), - ), - ): - with pytest.raises(AgentConfigServiceError, match="editable drafts") as exc_info: - service.push_file_for_console( - tenant_id=TENANT, - agent_id=AGENT, - user_id=USER, - config_version_id="snapshot-1", - config_version_kind=AgentConfigVersionKind.SNAPSHOT, - upload_file_id="upload-1", - ) - - assert exc_info.value.code == "config_not_writable" - - -def test_push_file_for_console_uses_service_owned_upload_lookup_and_naming() -> None: - session = MagicMock() - service = AgentConfigService() - target = _target( - kind=AgentConfigVersionKind.DRAFT, - writable=False, - soul=_soul(config_files=[{"name": "guide.txt", "file_kind": "upload_file", "file_id": "", "is_missing": True}]), - ) - upload_file = SimpleNamespace( - id="upload-1", - name="guide.txt", - size=7, - hash="sha256:abc", - mime_type="text/plain", - ) - - with ( - patch(f"{MODULE}.session_factory.create_session", return_value=_session_cm(session)), - patch.object(service, "_resolve_target_in_session", return_value=target), - patch.object(service, "_require_console_upload_file_source", return_value=upload_file), - ): - response = service.push_file_for_console( + with pytest.raises(AgentConfigServiceError, match="editable drafts") as exc_info: + _service(sqlite_session).push_file_for_console( tenant_id=TENANT, agent_id=AGENT, user_id=USER, - config_version_id="draft-1", - config_version_kind=AgentConfigVersionKind.DRAFT, - upload_file_id="upload-1", + config_version_id=SNAPSHOT, + config_version_kind=AgentConfigVersionKind.SNAPSHOT, + upload_file_id=UPLOAD_FILE, ) + assert exc_info.value.code == "config_not_writable" + sqlite_session.expire_all() + persisted = sqlite_session.get(AgentConfigSnapshot, SNAPSHOT) + assert persisted is not None + assert persisted.config_snapshot.config_files == [] + + +@pytest.mark.parametrize("sqlite_session", [(*AGENT_CONFIG_TABLES, UploadFile)], indirect=True) +def test_push_file_for_console_uses_service_owned_upload_lookup_and_naming(sqlite_session: Session) -> None: + _persist_target(sqlite_session, _draft()) + upload_file = UploadFile( + tenant_id=TENANT, + storage_type=StorageType.LOCAL, + key="uploads/guide.txt", + name="guide.txt", + size=7, + extension="txt", + mime_type="text/plain", + created_by_role=CreatorUserRole.ACCOUNT, + created_by=USER, + created_at=datetime(2025, 1, 1), + used=False, + hash="sha256:abc", + ) + upload_file.id = UPLOAD_FILE + sqlite_session.add(upload_file) + sqlite_session.commit() + + response = _service(sqlite_session).push_file_for_console( + tenant_id=TENANT, + agent_id=AGENT, + user_id=USER, + config_version_id=DRAFT, + config_version_kind=AgentConfigVersionKind.DRAFT, + upload_file_id=UPLOAD_FILE, + ) + assert response == { "file": { "id": "guide.txt", "name": "guide.txt", - "file_id": "upload-1", + "file_id": UPLOAD_FILE, "is_missing": False, "size": 7, "hash": "sha256:abc", "mime_type": "text/plain", }, "config_version": { - "id": "version-1", + "id": DRAFT, "kind": "draft", "writable": True, }, } - session.commit.assert_called_once() + sqlite_session.expire_all() + persisted = sqlite_session.get(AgentConfigDraft, DRAFT) + assert persisted is not None + assert persisted.config_snapshot.config_files[0].file_id == UPLOAD_FILE -def test_upload_skill_for_console_maps_package_validation_failures() -> None: - session = MagicMock() - service = AgentConfigService() - target = _target(kind=AgentConfigVersionKind.DRAFT, writable=False) +@pytest.mark.parametrize("sqlite_session", [AGENT_CONFIG_TABLES], indirect=True) +def test_upload_skill_for_console_maps_package_validation_failures(sqlite_session: Session) -> None: + _persist_target(sqlite_session, _draft()) + service = _service(sqlite_session) message = "skill package must contain exactly one skill; multiple skill folders in one archive are not supported" - with ( - patch(f"{MODULE}.session_factory.create_session", return_value=_session_cm(session)), - patch.object(service, "_resolve_target_in_session", return_value=target), - patch.object( - service._skill_normalizer, - "normalize", - side_effect=SkillPackageError("files_outside_skill_root", message, status_code=400), - ), + with patch.object( + service._skill_normalizer, + "normalize", + side_effect=SkillPackageError("files_outside_skill_root", message, status_code=400), ): with pytest.raises(AgentConfigServiceError, match="exactly one skill") as exc_info: service.upload_skill_for_console( tenant_id=TENANT, agent_id=AGENT, user_id=USER, - config_version_id="draft-1", + config_version_id=DRAFT, config_version_kind=AgentConfigVersionKind.DRAFT, content=b"bad-archive", filename="skills.zip", @@ -386,15 +471,19 @@ def test_upload_skill_for_console_maps_package_validation_failures() -> None: assert exc_info.value.code == "files_outside_skill_root" assert exc_info.value.message == message assert exc_info.value.status_code == 400 - session.commit.assert_not_called() + sqlite_session.expire_all() + persisted = sqlite_session.get(AgentConfigDraft, DRAFT) + assert persisted is not None + assert persisted.config_snapshot.config_skills == [] -def test_apply_skill_updates_rejects_non_tool_file_refs() -> None: +@pytest.mark.parametrize("sqlite_session", [()], indirect=True) +def test_apply_skill_updates_rejects_non_tool_file_refs(sqlite_session: Session) -> None: service = AgentConfigService() with pytest.raises(AgentConfigServiceError, match="tool files") as exc_info: service._apply_skill_updates( - MagicMock(), + sqlite_session, tenant_id=TENANT, user_id=USER, current=[], @@ -415,7 +504,12 @@ def test_apply_skill_updates_rejects_non_tool_file_refs() -> None: ("invalid_archive", "stored tool file is not a valid skill archive"), ], ) -def test_apply_skill_updates_maps_normalizer_failures(error_code: str, message: str) -> None: +@pytest.mark.parametrize("sqlite_session", [()], indirect=True) +def test_apply_skill_updates_maps_normalizer_failures( + error_code: str, + message: str, + sqlite_session: Session, +) -> None: service = AgentConfigService() tool_file = SimpleNamespace(name="alpha.zip", file_key="tool-files/alpha.zip") @@ -430,7 +524,7 @@ def test_apply_skill_updates_maps_normalizer_failures(error_code: str, message: ): with pytest.raises(AgentConfigServiceError, match=message) as exc_info: service._apply_skill_updates( - MagicMock(), + sqlite_session, tenant_id=TENANT, user_id=USER, current=[], @@ -549,37 +643,44 @@ def test_manifest_uses_items_shape_without_download_urls() -> None: } -def test_manifest_preserves_missing_config_assets_and_pull_rejects_them() -> None: +@pytest.mark.parametrize("sqlite_session", [AGENT_CONFIG_TABLES], indirect=True) +def test_manifest_preserves_missing_config_assets_and_pull_rejects_them(sqlite_session: Session) -> None: soul = _soul( config_skills=[{"name": "alpha", "file_id": "", "is_missing": True}], config_files=[{"name": "guide.txt", "file_kind": "upload_file", "file_id": "", "is_missing": True}], ) - target = _target(kind=AgentConfigVersionKind.DRAFT, writable=False, soul=soul) - service = AgentConfigService() + _persist_target(sqlite_session, _draft(soul=soul)) + service = _service(sqlite_session) + target = service.resolve_target( + tenant_id=TENANT, + agent_id=AGENT, + config_version_id=DRAFT, + config_version_kind=AgentConfigVersionKind.DRAFT, + user_id=USER, + ) manifest = service._manifest_for_target(target) assert manifest["skills"]["items"][0]["is_missing"] is True # type: ignore[index] assert manifest["files"]["items"][0]["is_missing"] is True # type: ignore[index] - with patch.object(service, "resolve_target", return_value=target): - with pytest.raises(AgentConfigServiceError) as skill_error: - service.pull_skill( - tenant_id=TENANT, - agent_id=AGENT, - config_version_id="draft-1", - config_version_kind=AgentConfigVersionKind.DRAFT, - name="alpha", - user_id=USER, - ) - with pytest.raises(AgentConfigServiceError) as file_error: - service.pull_file( - tenant_id=TENANT, - agent_id=AGENT, - config_version_id="draft-1", - config_version_kind=AgentConfigVersionKind.DRAFT, - name="guide.txt", - user_id=USER, - ) + with pytest.raises(AgentConfigServiceError) as skill_error: + service.pull_skill( + tenant_id=TENANT, + agent_id=AGENT, + config_version_id=DRAFT, + config_version_kind=AgentConfigVersionKind.DRAFT, + name="alpha", + user_id=USER, + ) + with pytest.raises(AgentConfigServiceError) as file_error: + service.pull_file( + tenant_id=TENANT, + agent_id=AGENT, + config_version_id=DRAFT, + config_version_kind=AgentConfigVersionKind.DRAFT, + name="guide.txt", + user_id=USER, + ) assert (skill_error.value.code, skill_error.value.status_code) == ("config_skill_missing", 409) assert (file_error.value.code, file_error.value.status_code) == ("config_file_missing", 409)