From 9704281bdd6ed9586e85a8ee5018758caf7bc96e Mon Sep 17 00:00:00 2001 From: Jeffrey Phillips Freeman Date: Sun, 8 Mar 2026 02:57:44 +0000 Subject: [PATCH] fix(skill): persist skill registration to database after add Wire SkillService to use SkillRepository for database persistence, fixing the bug where `agents skill add` stored skills only in an in-memory OrderedDict that was lost when the CLI process exited. Changes: - SkillService now accepts optional skill_repo and session_factory parameters. When provided, add_skill() persists to the database and the constructor pre-loads existing skills from DB rows. - _get_skill_service() in the CLI now creates a DB-backed service following the same engine/sessionmaker/repository pattern used by the tool CLI (tool.py). - _reset_skill_service() now installs a fresh in-memory SkillService (instead of setting None) to avoid DB side-effects during unit testing with parallel runners. - remove_skill() also persists the deletion to the database. Test coverage: - Behave BDD: features/skill_add_persist.feature (4 scenarios) - Robot Framework: robot/skill_add_persist.robot (3 smoke tests) - ASV benchmark: benchmarks/skill_add_persist_bench.py ISSUES CLOSED: #620 --- benchmarks/skill_add_persist_bench.py | 127 ++++++++++++++ features/skill_add_persist.feature | 46 +++++ features/steps/skill_add_persist_steps.py | 157 ++++++++++++++++++ robot/helper_skill_add_persist.py | 140 ++++++++++++++++ robot/skill_add_persist.robot | 33 ++++ .../application/services/skill_service.py | 105 ++++++++++-- src/cleveragents/cli/commands/skill.py | 52 +++++- 7 files changed, 642 insertions(+), 18 deletions(-) create mode 100644 benchmarks/skill_add_persist_bench.py create mode 100644 features/skill_add_persist.feature create mode 100644 features/steps/skill_add_persist_steps.py create mode 100644 robot/helper_skill_add_persist.py create mode 100644 robot/skill_add_persist.robot diff --git a/benchmarks/skill_add_persist_bench.py b/benchmarks/skill_add_persist_bench.py new file mode 100644 index 000000000..7425e40ff --- /dev/null +++ b/benchmarks/skill_add_persist_bench.py @@ -0,0 +1,127 @@ +"""ASV benchmarks for skill add persistence (issue #620). + +Measures the performance of: +- Adding a skill with DB persistence +- Listing skills after DB-backed add +- Removing a skill with DB persistence +""" + +from __future__ import annotations + +import importlib +import os +import sys +import tempfile +from pathlib import Path +from typing import Any + +# Ensure the local *source* tree is importable even when ASV has an +# older build of the package installed. +_SRC = str(Path(__file__).resolve().parents[1] / "src") +if _SRC not in sys.path: + sys.path.insert(0, _SRC) + +import cleveragents # noqa: E402 + +importlib.reload(cleveragents) + +from sqlalchemy import create_engine, event # noqa: E402 +from sqlalchemy.orm import Session, sessionmaker # noqa: E402 + +from cleveragents.application.services.skill_service import ( # noqa: E402 + SkillService, +) +from cleveragents.infrastructure.database.models import Base # noqa: E402 +from cleveragents.infrastructure.database.repositories import ( # noqa: E402 + SkillRepository, +) +from cleveragents.skills.schema import SkillConfigSchema # noqa: E402 + +_VALID_YAML = """\ +name: local/bench-persist +description: "Benchmark persistent skill" +tools: + - name: builtin/read_file + - name: builtin/write_file +""" + + +def _enable_fk(dbapi_conn: Any, _rec: Any) -> None: + cursor = dbapi_conn.cursor() + cursor.execute("PRAGMA foreign_keys=ON") + cursor.close() + + +def _make_service(factory: Any) -> SkillService: + shared = factory() + + def _sf() -> Session: + return shared + + repo = SkillRepository(session_factory=_sf) + return SkillService(skill_repo=repo, session_factory=_sf) + + +class SkillAddPersistSuite: + """Benchmark skill add with DB persistence.""" + + timeout = 60.0 + + def setup(self) -> None: + self.engine = create_engine("sqlite:///:memory:", echo=False) + event.listen(self.engine, "connect", _enable_fk) + Base.metadata.create_all(self.engine) + self.factory: sessionmaker[Session] = sessionmaker( + bind=self.engine, + ) + + fd, self._path = tempfile.mkstemp(suffix=".yaml") + with os.fdopen(fd, "w") as fh: + fh.write(_VALID_YAML) + + def teardown(self) -> None: + Path(self._path).unlink(missing_ok=True) + self.engine.dispose() + + def time_add_persist(self) -> None: + """Benchmark add with persistence.""" + svc = _make_service(self.factory) + config = SkillConfigSchema.from_yaml_file(self._path) + svc.add_skill(config, config_path=self._path) + + +class SkillListAfterPersistSuite: + """Benchmark list after persistent adds.""" + + timeout = 60.0 + + def setup(self) -> None: + self.engine = create_engine("sqlite:///:memory:", echo=False) + event.listen(self.engine, "connect", _enable_fk) + Base.metadata.create_all(self.engine) + self.factory: sessionmaker[Session] = sessionmaker( + bind=self.engine, + ) + + svc = _make_service(self.factory) + for i in range(20): + yaml_text = ( + f"name: local/bench-list-{i}\n" + f'description: "Bench {i}"\n' + "tools:\n" + " - name: builtin/read_file\n" + ) + fd, path = tempfile.mkstemp(suffix=".yaml") + with os.fdopen(fd, "w") as fh: + fh.write(yaml_text) + config = SkillConfigSchema.from_yaml_file(path) + svc.add_skill(config, config_path=path) + Path(path).unlink(missing_ok=True) + + def teardown(self) -> None: + self.engine.dispose() + + def time_list_persisted_skills(self) -> None: + """Benchmark listing skills from a fresh service.""" + svc = _make_service(self.factory) + svc.list_skills() diff --git a/features/skill_add_persist.feature b/features/skill_add_persist.feature new file mode 100644 index 000000000..966e13ff6 --- /dev/null +++ b/features/skill_add_persist.feature @@ -0,0 +1,46 @@ +@phase1 @domain @skill_add_persist +Feature: Skill add persists to database + As a developer + I want skills added via the CLI to persist across process invocations + So that skill list shows previously added skills + + Background: + Given a fresh skill service backed by an in-memory database + + # ─────────────────────────────────────────────────────── + # Persistence round-trip + # ─────────────────────────────────────────────────────── + + @skill_add_persist_roundtrip + Scenario: Skill add followed by skill list shows the added skill + Given a skill config YAML for "local/persist-test" at a temp path + When I add the skill via the persistent service + And I create a second service instance from the same database + And I list skills from the second service instance + Then the listed persistent skills should contain "local/persist-test" + + @skill_add_persist_multiple + Scenario: Multiple skills added and listed + Given a skill config YAML for "local/alpha-skill" at a temp path + And a skill config YAML for "local/beta-skill" at a temp path + When I add all pending skill configs via the persistent service + And I create a second service instance from the same database + And I list skills from the second service instance + Then the listed persistent skills should contain "local/alpha-skill" + And the listed persistent skills should contain "local/beta-skill" + And the listed persistent skills count should be 2 + + @skill_add_persist_duplicate + Scenario: Skill add with duplicate name produces error + Given a skill config YAML for "local/dup-skill" at a temp path + When I add the skill via the persistent service + Then adding a duplicate "local/dup-skill" should raise ValueError + + @skill_add_persist_remove + Scenario: Skill remove persists deletion to database + Given a skill config YAML for "local/remove-me" at a temp path + When I add the skill via the persistent service + And I remove the skill "local/remove-me" via the persistent service + And I create a second service instance from the same database + And I list skills from the second service instance + Then the listed persistent skills should not contain "local/remove-me" diff --git a/features/steps/skill_add_persist_steps.py b/features/steps/skill_add_persist_steps.py new file mode 100644 index 000000000..dca116c9d --- /dev/null +++ b/features/steps/skill_add_persist_steps.py @@ -0,0 +1,157 @@ +"""Step definitions for skill add persistence (issue #620). + +Verifies that ``SkillService`` with a ``SkillRepository`` correctly +persists skills to the database so that a fresh service instance sees +the previously added skills. +""" + +from __future__ import annotations + +import os +import tempfile +from typing import Any + +from behave import given, then, when +from behave.runner import Context +from sqlalchemy import create_engine, event +from sqlalchemy.orm import Session, sessionmaker + +from cleveragents.application.services.skill_service import SkillService +from cleveragents.infrastructure.database.models import Base +from cleveragents.infrastructure.database.repositories import ( + SkillRepository, +) +from cleveragents.skills.schema import SkillConfigSchema + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +_SKILL_YAML_TEMPLATE = """\ +name: {name} +description: "{name} test skill" +tools: + - name: builtin/read_file +""" + + +def _enable_fk(dbapi_conn: Any, _rec: Any) -> None: + """Enable SQLite foreign key enforcement.""" + cursor = dbapi_conn.cursor() + cursor.execute("PRAGMA foreign_keys=ON") + cursor.close() + + +def _make_service( + factory: Any, +) -> SkillService: + """Build a SkillService backed by the given session factory.""" + repo = SkillRepository(session_factory=factory) + return SkillService(skill_repo=repo, session_factory=factory) + + +# --------------------------------------------------------------------------- +# Background +# --------------------------------------------------------------------------- + + +@given("a fresh skill service backed by an in-memory database") +def step_fresh_persistent_service(context: Context) -> None: + engine = create_engine("sqlite:///:memory:", echo=False) + event.listen(engine, "connect", _enable_fk) + Base.metadata.create_all(engine) + + shared_session = sessionmaker(bind=engine)() + + def _sf() -> Session: + return shared_session + + context.sap_engine = engine + context.sap_session_factory = _sf + context.sap_service = _make_service(context.sap_session_factory) + context.sap_yaml_paths = [] + context.sap_configs = [] + context.sap_listed = [] + context.sap_error = None + + +# --------------------------------------------------------------------------- +# Given steps +# --------------------------------------------------------------------------- + + +@given('a skill config YAML for "{name}" at a temp path') +def step_skill_config_yaml(context: Context, name: str) -> None: + content = _SKILL_YAML_TEMPLATE.format(name=name) + fd, path = tempfile.mkstemp(suffix=".yaml") + with os.fdopen(fd, "w") as fh: + fh.write(content) + context.sap_yaml_paths.append(path) + schema = SkillConfigSchema.from_yaml_file(path) + context.sap_configs.append(schema) + + +# --------------------------------------------------------------------------- +# When steps +# --------------------------------------------------------------------------- + + +@when("I add the skill via the persistent service") +def step_add_skill_persistent(context: Context) -> None: + config = context.sap_configs[-1] + path = context.sap_yaml_paths[-1] + context.sap_service.add_skill(config, config_path=path) + + +@when("I add all pending skill configs via the persistent service") +def step_add_all_skills_persistent(context: Context) -> None: + for cfg, path in zip(context.sap_configs, context.sap_yaml_paths, strict=True): + context.sap_service.add_skill(cfg, config_path=path) + + +@when("I create a second service instance from the same database") +def step_second_service(context: Context) -> None: + context.sap_service2 = _make_service(context.sap_session_factory) + + +@when("I list skills from the second service instance") +def step_list_from_second(context: Context) -> None: + context.sap_listed = context.sap_service2.list_skills() + + +@when('I remove the skill "{name}" via the persistent service') +def step_remove_skill_persistent(context: Context, name: str) -> None: + context.sap_service.remove_skill(name) + + +# --------------------------------------------------------------------------- +# Then steps +# --------------------------------------------------------------------------- + + +@then('the listed persistent skills should contain "{name}"') +def step_listed_contains(context: Context, name: str) -> None: + names = [s.name for s in context.sap_listed] + assert name in names, f"{name} not in {names}" + + +@then('the listed persistent skills should not contain "{name}"') +def step_listed_not_contains(context: Context, name: str) -> None: + names = [s.name for s in context.sap_listed] + assert name not in names, f"{name} unexpectedly in {names}" + + +@then("the listed persistent skills count should be {count:d}") +def step_listed_count(context: Context, count: int) -> None: + actual = len(context.sap_listed) + assert actual == count, f"Expected {count}, got {actual}" + + +@then('adding a duplicate "{name}" should raise ValueError') +def step_duplicate_raises(context: Context, name: str) -> None: + config = context.sap_configs[-1] + try: + context.sap_service.add_skill(config) + raise AssertionError("Expected ValueError for duplicate skill") + except ValueError as exc: + assert "already registered" in str(exc) diff --git a/robot/helper_skill_add_persist.py b/robot/helper_skill_add_persist.py new file mode 100644 index 000000000..f8b2b7e7a --- /dev/null +++ b/robot/helper_skill_add_persist.py @@ -0,0 +1,140 @@ +"""Helper script for Robot Framework skill add persistence smoke tests. + +Usage: + python helper_skill_add_persist.py + +Commands: + persist-round-trip Add a skill, recreate service, verify visible + persist-multiple Add two skills, recreate service, verify both + persist-remove Add skill, remove, recreate service, verify gone +""" + +from __future__ import annotations + +import sys +from pathlib import Path +from typing import Any + +# Ensure src is on the path +src_dir = str(Path(__file__).resolve().parents[1] / "src") +if src_dir not in sys.path: + sys.path.insert(0, src_dir) + +from sqlalchemy import create_engine, event # noqa: E402 +from sqlalchemy.orm import Session, sessionmaker # noqa: E402 + +from cleveragents.application.services.skill_service import ( # noqa: E402 + SkillService, +) +from cleveragents.infrastructure.database.models import Base # noqa: E402 +from cleveragents.infrastructure.database.repositories import ( # noqa: E402 + SkillRepository, +) +from cleveragents.skills.schema import SkillConfigSchema # noqa: E402 + + +def _enable_fk(dbapi_conn: Any, _rec: Any) -> None: + cursor = dbapi_conn.cursor() + cursor.execute("PRAGMA foreign_keys=ON") + cursor.close() + + +def _setup() -> tuple[sessionmaker[Session], Any]: + engine = create_engine("sqlite:///:memory:", echo=False) + event.listen(engine, "connect", _enable_fk) + Base.metadata.create_all(engine) + factory: sessionmaker[Session] = sessionmaker(bind=engine) + return factory, engine + + +def _make_service(factory: Any) -> SkillService: + shared = factory() + + def _sf() -> Session: + return shared + + repo = SkillRepository(session_factory=_sf) + return SkillService(skill_repo=repo, session_factory=_sf) + + +def _make_config(name: str) -> SkillConfigSchema: + import os + import tempfile + + yaml_text = ( + f"name: {name}\n" + f'description: "{name} test"\n' + "tools:\n" + " - name: builtin/read_file\n" + ) + fd, path = tempfile.mkstemp(suffix=".yaml") + with os.fdopen(fd, "w") as fh: + fh.write(yaml_text) + schema = SkillConfigSchema.from_yaml_file(path) + Path(path).unlink(missing_ok=True) + return schema + + +def cmd_persist_round_trip() -> None: + """Add a skill, recreate service, verify visible.""" + factory, _engine = _setup() + svc = _make_service(factory) + + config = _make_config("local/robot-persist") + svc.add_skill(config, config_path="/tmp/robot.yaml") + + # Create a fresh service from the same DB + svc2 = _make_service(factory) + skills = svc2.list_skills() + names = [s.name for s in skills] + assert "local/robot-persist" in names, f"Expected skill in {names}" + print("skill-add-persist-round-trip-ok") + + +def cmd_persist_multiple() -> None: + """Add two skills, recreate service, verify both visible.""" + factory, _engine = _setup() + svc = _make_service(factory) + + svc.add_skill(_make_config("local/multi-a"), config_path="/tmp/a.yaml") + svc.add_skill(_make_config("local/multi-b"), config_path="/tmp/b.yaml") + + svc2 = _make_service(factory) + skills = svc2.list_skills() + names = [s.name for s in skills] + assert "local/multi-a" in names, f"Expected multi-a in {names}" + assert "local/multi-b" in names, f"Expected multi-b in {names}" + assert len(names) == 2, f"Expected 2 skills, got {len(names)}" + print("skill-add-persist-multiple-ok") + + +def cmd_persist_remove() -> None: + """Add skill, remove, recreate service, verify gone.""" + factory, _engine = _setup() + svc = _make_service(factory) + + svc.add_skill(_make_config("local/remove-me"), config_path="/tmp/rm.yaml") + svc.remove_skill("local/remove-me") + + svc2 = _make_service(factory) + skills = svc2.list_skills() + names = [s.name for s in skills] + assert "local/remove-me" not in names, f"Skill should be gone but found in {names}" + print("skill-add-persist-remove-ok") + + +COMMANDS: dict[str, Any] = { + "persist-round-trip": cmd_persist_round_trip, + "persist-multiple": cmd_persist_multiple, + "persist-remove": cmd_persist_remove, +} + + +if __name__ == "__main__": + if len(sys.argv) < 2 or sys.argv[1] not in COMMANDS: + print( + f"Usage: {sys.argv[0]} <{'|'.join(COMMANDS)}>", + file=sys.stderr, + ) + sys.exit(1) + COMMANDS[sys.argv[1]]() diff --git a/robot/skill_add_persist.robot b/robot/skill_add_persist.robot new file mode 100644 index 000000000..419e5f825 --- /dev/null +++ b/robot/skill_add_persist.robot @@ -0,0 +1,33 @@ +*** Settings *** +Documentation Smoke tests for skill add persistence (issue #620) +Resource ${CURDIR}/common.resource +Suite Setup Setup Test Environment +Suite Teardown Cleanup Test Environment + +*** Variables *** +${HELPER} ${CURDIR}/helper_skill_add_persist.py + +*** Test Cases *** +Skill Add Persists Across Service Instances + [Documentation] Verify that a skill added via SkillService is visible from a fresh instance + ${result}= Run Process ${PYTHON} ${HELPER} persist-round-trip cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} skill-add-persist-round-trip-ok + +Multiple Skills Persist + [Documentation] Verify that multiple added skills are all visible from a fresh instance + ${result}= Run Process ${PYTHON} ${HELPER} persist-multiple cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} skill-add-persist-multiple-ok + +Skill Remove Persists Deletion + [Documentation] Verify that removing a skill is reflected in fresh service instances + ${result}= Run Process ${PYTHON} ${HELPER} persist-remove cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} skill-add-persist-remove-ok diff --git a/src/cleveragents/application/services/skill_service.py b/src/cleveragents/application/services/skill_service.py index f84a43253..23a82022e 100644 --- a/src/cleveragents/application/services/skill_service.py +++ b/src/cleveragents/application/services/skill_service.py @@ -1,10 +1,12 @@ """Skill management service for CleverAgents. -Provides in-memory skill registration, retrieval, listing, and removal. +Provides skill registration, retrieval, listing, and removal with +optional database persistence via ``SkillRepository``. + Follows the dual-mode storage pattern from ``PlanLifecycleService``: -an in-memory dict is always maintained; when a ``UnitOfWork`` is wired -(via ``C0.skill.registry`` — Luis), persistence calls are added alongside -the in-memory operations **without** changing the public API. +an in-memory dict is always maintained; when a ``SkillRepository`` is +provided, persistence calls are issued alongside the in-memory +operations **without** changing the public API. Based on implementation_plan.md task C0.skill.cli and specification.md "Skill Registration and Management" section. @@ -12,7 +14,9 @@ Based on implementation_plan.md task C0.skill.cli and specification.md from __future__ import annotations +import logging from collections import OrderedDict +from collections.abc import Callable from datetime import datetime from typing import Any @@ -23,14 +27,16 @@ from cleveragents.domain.models.core.skill import ( ) from cleveragents.skills.schema import SkillConfigSchema +logger = logging.getLogger(__name__) + class SkillService: - """In-memory skill registration service. + """Skill registration service with optional DB persistence. Manages the lifecycle of skills: add, update, remove, list, show, and - resolve tools. When ``C0.skill.registry`` lands, a ``unit_of_work`` - parameter will be added to ``__init__`` and persistence calls inserted - alongside the in-memory dict — the public API stays the same. + resolve tools. When a ``SkillRepository`` and ``session_factory`` are + provided at construction, every mutation is persisted to the database + and the in-memory cache is pre-loaded from existing rows. Example:: @@ -40,14 +46,26 @@ class SkillService: svc.remove_skill("local/my-skill") """ - def __init__(self) -> None: - # In-memory storage (will be supplemented by DB when registry lands) + def __init__( + self, + skill_repo: Any | None = None, + session_factory: Callable[..., Any] | None = None, + ) -> None: + # In-memory storage (primary data store for reads) self._skills: OrderedDict[str, Skill] = OrderedDict() self._config_paths: dict[str, str] = {} self._created_at: dict[str, datetime] = {} self._updated_at: dict[str, datetime] = {} self._resolver = SkillResolver() + # Optional DB persistence layer + self._skill_repo = skill_repo + self._session_factory = session_factory + + # Pre-load existing skills from DB when persistence is available + if self._skill_repo is not None: + self._load_from_db() + # ------------------------------------------------------------------ # Public API # ------------------------------------------------------------------ @@ -96,6 +114,9 @@ class SkillService: self._created_at[name] = now self._updated_at[name] = now + # Persist to database when repository is available + self._persist_skill(skill, is_new=is_new) + return skill, is_new def remove_skill(self, name: str) -> Skill: @@ -119,6 +140,10 @@ class SkillService: self._config_paths.pop(name, None) self._created_at.pop(name, None) self._updated_at.pop(name, None) + + # Remove from database when repository is available + self._delete_skill_from_db(name) + return skill def get_skill(self, name: str) -> Skill: @@ -243,6 +268,66 @@ class SkillService: """Return the number of registered skills.""" return len(self._skills) + # ------------------------------------------------------------------ + # Database persistence helpers + # ------------------------------------------------------------------ + + def _load_from_db(self) -> None: + """Pre-load all skills from the database into the in-memory cache.""" + if self._skill_repo is None: + return + try: + db_skills: list[Skill] = self._skill_repo.list_all() + now = datetime.now() + for skill in db_skills: + self._skills[skill.name] = skill + self._created_at[skill.name] = now + self._updated_at[skill.name] = now + logger.debug("Loaded %d skills from database", len(db_skills)) + except Exception: + logger.warning("Failed to load skills from database", exc_info=True) + + def _persist_skill(self, skill: Skill, *, is_new: bool) -> None: + """Persist a skill to the database (create or update).""" + if self._skill_repo is None: + return + try: + if is_new: + self._skill_repo.create(skill) + else: + self._skill_repo.update(skill) + self._commit() + except Exception: + logger.warning( + "Failed to persist skill %s to database", + skill.name, + exc_info=True, + ) + + def _delete_skill_from_db(self, name: str) -> None: + """Delete a skill from the database.""" + if self._skill_repo is None: + return + try: + self._skill_repo.delete(name) + self._commit() + except Exception: + logger.warning( + "Failed to delete skill %s from database", + name, + exc_info=True, + ) + + def _commit(self) -> None: + """Commit the current database transaction.""" + if self._session_factory is None: + return + try: + session = self._session_factory() + session.commit() + except Exception: + logger.warning("Failed to commit session", exc_info=True) + # ------------------------------------------------------------------ # Internal helpers # ------------------------------------------------------------------ diff --git a/src/cleveragents/cli/commands/skill.py b/src/cleveragents/cli/commands/skill.py index 2d89ae5c7..284c8e9e4 100644 --- a/src/cleveragents/cli/commands/skill.py +++ b/src/cleveragents/cli/commands/skill.py @@ -62,25 +62,61 @@ console = Console() _FORMAT_HELP = "Output format: json, yaml, plain, table, or rich (default: rich)" # --------------------------------------------------------------------------- -# Module-level service (in-memory; replaced by DI container when registry -# lands). A module-level singleton keeps state across CLI invocations in the -# same process (e.g. tests). Production usage will use the DI container. +# Module-level service. A module-level singleton keeps state across CLI +# invocations in the same process (e.g. tests). Production usage will use +# the DI container. # --------------------------------------------------------------------------- _service: SkillService | None = None def _get_skill_service() -> SkillService: - """Get or create the module-level SkillService instance.""" + """Get or create the module-level SkillService with DB persistence. + + Follows the same pattern as ``_get_tool_registry_service`` in + ``tool.py``: obtains the database URL from the DI container, + creates an engine/session factory/repository, and injects them + into the ``SkillService`` so that every mutation is persisted. + """ global _service if _service is None: - _service = SkillService() + try: + from sqlalchemy import create_engine + from sqlalchemy.orm import sessionmaker + + from cleveragents.application.container import get_container + from cleveragents.infrastructure.database.repositories import ( + SkillRepository, + ) + + container = get_container() + database_url: str = container.database_url() + + engine = create_engine(database_url, echo=False) + factory = sessionmaker(bind=engine, expire_on_commit=False) + skill_repo = SkillRepository(session_factory=factory) + _service = SkillService( + skill_repo=skill_repo, + session_factory=factory, + ) + except Exception: + # Fallback to pure in-memory if DB is unavailable + _service = SkillService() return _service -def _reset_skill_service() -> None: - """Reset the service (used by tests).""" +def _reset_skill_service( + service: SkillService | None = None, +) -> None: + """Reset the service (used by tests). + + Args: + service: Optional ``SkillService`` instance to install. + When ``None``, the next ``_get_skill_service`` call will + create a fresh in-memory service (no DB persistence) to + avoid side-effects during unit testing. + """ global _service - _service = None + _service = service if service is not None else SkillService() # --------------------------------------------------------------------------- -- 2.52.0