fix(cli): replace in-memory automation-profile repository with database-backed persistence
The automation-profile CLI commands used an _InMemoryProfileRepository (a Python dict) that lost all data between CLI process invocations. Profiles created with "automation-profile add" were invisible to subsequent "automation-profile show" or "list" calls because each CLI command is a separate process with a fresh empty dict. Changes: - Replaced _InMemoryProfileRepository with the real AutomationProfileRepository from the infrastructure layer, wired via the DI container following the same pattern as tool.py and session.py. - Added auto_commit support to AutomationProfileRepository (matching the existing SessionRepository pattern) so that CLI commands running outside a UnitOfWork commit each operation automatically. - Added safety_json and guards_json Text columns to the automation_profiles table (Alembic migration m6_005) for full-fidelity round-trip of the AutomationGuard and SafetyProfile sub-models. Previously, guards and several safety fields (max_cost_per_plan, max_retries_per_step, etc.) were silently dropped on persistence. - Updated _from_domain, _to_domain, and _update_row to serialize and deserialize the full guard and safety sub-models via JSON, with backward-compatible fallback to legacy scalar columns. Refs: #746
This commit is contained in:
@@ -0,0 +1,40 @@
|
||||
"""Add safety_json and guards_json columns to automation_profiles table.
|
||||
|
||||
Persists the full SafetyProfile and AutomationGuard sub-models as
|
||||
serialised JSON, complementing the existing scalar safety columns.
|
||||
Nullable so existing rows are unaffected.
|
||||
|
||||
Revision ID: m6_005_profile_guards_json
|
||||
Revises: m6_004_container_metadata_column
|
||||
Create Date: 2026-03-13 00:00:00
|
||||
|
||||
"""
|
||||
|
||||
from collections.abc import Sequence
|
||||
|
||||
import sqlalchemy as sa
|
||||
from alembic import op
|
||||
|
||||
# revision identifiers, used by Alembic.
|
||||
revision: str = "m6_005_profile_guards_json"
|
||||
down_revision: str | Sequence[str] | None = "m6_004_container_metadata_column"
|
||||
branch_labels: str | Sequence[str] | None = None
|
||||
depends_on: str | Sequence[str] | None = None
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
"""Add safety_json and guards_json columns to automation_profiles."""
|
||||
with op.batch_alter_table("automation_profiles") as batch_op:
|
||||
batch_op.add_column(
|
||||
sa.Column("safety_json", sa.Text(), nullable=True),
|
||||
)
|
||||
batch_op.add_column(
|
||||
sa.Column("guards_json", sa.Text(), nullable=True),
|
||||
)
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
"""Remove safety_json and guards_json columns."""
|
||||
with op.batch_alter_table("automation_profiles") as batch_op:
|
||||
batch_op.drop_column("guards_json")
|
||||
batch_op.drop_column("safety_json")
|
||||
@@ -27,12 +27,39 @@ importlib.reload(cleveragents)
|
||||
|
||||
from typer.testing import CliRunner # noqa: E402
|
||||
|
||||
from cleveragents.cli.commands.automation_profile import ( # noqa: E402
|
||||
_InMemoryProfileRepository,
|
||||
from cleveragents.application.services.automation_profile_service import ( # noqa: E402
|
||||
AutomationProfileService,
|
||||
)
|
||||
from cleveragents.cli.commands.automation_profile import ( # noqa: E402
|
||||
app as profile_app,
|
||||
)
|
||||
from cleveragents.domain.models.core.automation_profile import ( # noqa: E402
|
||||
AutomationProfile,
|
||||
)
|
||||
|
||||
|
||||
class _InMemoryProfileRepository:
|
||||
"""In-memory repository satisfying the AutomationProfileRepository duck-type contract.
|
||||
|
||||
Provides the four methods expected by ``AutomationProfileService``:
|
||||
``get_by_name``, ``list_all``, ``upsert``, and ``delete``.
|
||||
"""
|
||||
|
||||
def __init__(self) -> None:
|
||||
self._store: dict[str, AutomationProfile] = {}
|
||||
|
||||
def get_by_name(self, name: str) -> AutomationProfile | None:
|
||||
return self._store.get(name)
|
||||
|
||||
def list_all(self) -> list[AutomationProfile]:
|
||||
return list(self._store.values())
|
||||
|
||||
def upsert(self, profile: AutomationProfile) -> None:
|
||||
self._store[profile.name] = profile
|
||||
|
||||
def delete(self, name: str) -> None:
|
||||
self._store.pop(name, None)
|
||||
|
||||
|
||||
_VALID_YAML = """\
|
||||
name: bench/test-profile
|
||||
@@ -58,19 +85,25 @@ safety:
|
||||
_runner = CliRunner()
|
||||
|
||||
|
||||
def _reset_repo() -> None:
|
||||
"""Reset the module-level in-memory repo."""
|
||||
def _patch_service() -> None:
|
||||
"""Patch ``_get_service`` in the CLI module to use an in-memory repo.
|
||||
|
||||
Each call creates a fresh repo + service so benchmark iterations
|
||||
are isolated from each other and safe for parallel execution.
|
||||
"""
|
||||
import cleveragents.cli.commands.automation_profile as ap_mod
|
||||
|
||||
ap_mod._repo = _InMemoryProfileRepository()
|
||||
repo = _InMemoryProfileRepository()
|
||||
service = AutomationProfileService(repo=repo) # type: ignore[arg-type]
|
||||
ap_mod._get_service = lambda: service
|
||||
|
||||
|
||||
class AutomationProfileCLIAddSuite:
|
||||
"""Benchmark automation-profile add --config throughput."""
|
||||
|
||||
def setup(self) -> None:
|
||||
"""Write a temporary YAML file."""
|
||||
_reset_repo()
|
||||
"""Write a temporary YAML file and patch service."""
|
||||
_patch_service()
|
||||
fd, self._path = tempfile.mkstemp(suffix=".yaml")
|
||||
with os.fdopen(fd, "w") as fh:
|
||||
fh.write(_VALID_YAML)
|
||||
@@ -81,7 +114,7 @@ class AutomationProfileCLIAddSuite:
|
||||
|
||||
def time_add_from_config(self) -> None:
|
||||
"""Benchmark add --config end-to-end."""
|
||||
_reset_repo()
|
||||
_patch_service()
|
||||
_runner.invoke(profile_app, ["add", "--config", self._path])
|
||||
|
||||
|
||||
@@ -90,7 +123,7 @@ class AutomationProfileCLIListSuite:
|
||||
|
||||
def setup(self) -> None:
|
||||
"""Set up with built-in profiles."""
|
||||
_reset_repo()
|
||||
_patch_service()
|
||||
|
||||
def time_list_all(self) -> None:
|
||||
"""Benchmark listing all profiles."""
|
||||
@@ -110,7 +143,7 @@ class AutomationProfileCLIShowSuite:
|
||||
|
||||
def setup(self) -> None:
|
||||
"""Set up."""
|
||||
_reset_repo()
|
||||
_patch_service()
|
||||
|
||||
def time_show_rich(self) -> None:
|
||||
"""Benchmark showing a profile in rich format."""
|
||||
|
||||
@@ -15,7 +15,6 @@ from typer.testing import CliRunner
|
||||
|
||||
from cleveragents.cli.commands.automation_profile import (
|
||||
_guards_dict,
|
||||
_InMemoryProfileRepository,
|
||||
_profile_spec_dict,
|
||||
)
|
||||
from cleveragents.cli.commands.automation_profile import (
|
||||
@@ -24,6 +23,27 @@ from cleveragents.cli.commands.automation_profile import (
|
||||
from cleveragents.domain.models.core.automation_guard import AutomationGuard
|
||||
from cleveragents.domain.models.core.automation_profile import AutomationProfile
|
||||
|
||||
|
||||
def _create_in_memory_profile_service():
|
||||
"""Create an AutomationProfileService backed by an in-memory SQLite DB."""
|
||||
from sqlalchemy import create_engine
|
||||
from sqlalchemy.orm import sessionmaker
|
||||
|
||||
from cleveragents.application.services.automation_profile_service import (
|
||||
AutomationProfileService,
|
||||
)
|
||||
from cleveragents.infrastructure.database.models import Base
|
||||
from cleveragents.infrastructure.database.repositories import (
|
||||
AutomationProfileRepository,
|
||||
)
|
||||
|
||||
engine = create_engine("sqlite:///:memory:", echo=False)
|
||||
Base.metadata.create_all(engine)
|
||||
factory = sessionmaker(bind=engine, expire_on_commit=False)
|
||||
repo = AutomationProfileRepository(session_factory=factory, auto_commit=True)
|
||||
return AutomationProfileService(repo=repo)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -77,16 +97,24 @@ def _write_temp_yaml(context: Context, content: str) -> str:
|
||||
|
||||
@given("a fresh automation profile CLI runner for guards coverage")
|
||||
def step_fresh_guards_runner(context: Context) -> None:
|
||||
from unittest.mock import patch
|
||||
|
||||
context.runner = CliRunner()
|
||||
context.result = None
|
||||
context.guards_dict_result: dict[str, object] | None = None
|
||||
context.profile_spec_result: dict[str, object] | None = None
|
||||
context.guard_obj: AutomationGuard | None = None
|
||||
context.guarded_profile: AutomationProfile | None = None
|
||||
# Reset the module-level in-memory repo
|
||||
import cleveragents.cli.commands.automation_profile as ap_mod
|
||||
|
||||
ap_mod._repo = _InMemoryProfileRepository()
|
||||
context.guards_dict_result = None
|
||||
context.profile_spec_result = None
|
||||
context.guard_obj = None
|
||||
context.guarded_profile = None
|
||||
# Create an in-memory service and patch _get_service for this scenario
|
||||
context._ap_service = _create_in_memory_profile_service()
|
||||
context._ap_patcher = patch(
|
||||
"cleveragents.cli.commands.automation_profile._get_service",
|
||||
return_value=context._ap_service,
|
||||
)
|
||||
context._ap_patcher.start()
|
||||
if not hasattr(context, "_cleanup_handlers"):
|
||||
context._cleanup_handlers = []
|
||||
context._cleanup_handlers.append(context._ap_patcher.stop)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -233,8 +261,6 @@ def step_spec_dict_guards_key_true(context: Context, key: str) -> None:
|
||||
|
||||
@given('a custom guarded profile "{name}" is stored in the repo')
|
||||
def step_store_guarded_profile(context: Context, name: str) -> None:
|
||||
import cleveragents.cli.commands.automation_profile as ap_mod
|
||||
|
||||
profile = _make_guarded_profile(
|
||||
name=name,
|
||||
max_tool_calls=5,
|
||||
@@ -244,7 +270,7 @@ def step_store_guarded_profile(context: Context, name: str) -> None:
|
||||
require_approval_for_writes=True,
|
||||
require_approval_for_apply=True,
|
||||
)
|
||||
ap_mod._repo.upsert(profile)
|
||||
context._ap_service._repo.upsert(profile)
|
||||
|
||||
|
||||
@when('I run automation-profile show "{name}" in rich format')
|
||||
|
||||
@@ -12,7 +12,6 @@ from behave.runner import Context
|
||||
from typer.testing import CliRunner
|
||||
|
||||
from cleveragents.cli.commands.automation_profile import (
|
||||
_InMemoryProfileRepository,
|
||||
_threshold_summary,
|
||||
emit_automation_level_deprecation_warning,
|
||||
)
|
||||
@@ -21,13 +20,35 @@ from cleveragents.cli.commands.automation_profile import (
|
||||
)
|
||||
from cleveragents.core.exceptions import (
|
||||
CleverAgentsError,
|
||||
NotFoundError,
|
||||
ValidationError,
|
||||
)
|
||||
from cleveragents.domain.models.core.automation_profile import (
|
||||
BUILTIN_PROFILES,
|
||||
AutomationProfile,
|
||||
)
|
||||
from cleveragents.infrastructure.database.repositories import (
|
||||
AutomationProfileNotFoundError,
|
||||
)
|
||||
|
||||
|
||||
def _create_in_memory_profile_service():
|
||||
"""Create an AutomationProfileService backed by an in-memory SQLite DB."""
|
||||
from sqlalchemy import create_engine
|
||||
from sqlalchemy.orm import sessionmaker
|
||||
|
||||
from cleveragents.application.services.automation_profile_service import (
|
||||
AutomationProfileService,
|
||||
)
|
||||
from cleveragents.infrastructure.database.models import Base
|
||||
from cleveragents.infrastructure.database.repositories import (
|
||||
AutomationProfileRepository,
|
||||
)
|
||||
|
||||
engine = create_engine("sqlite:///:memory:", echo=False)
|
||||
Base.metadata.create_all(engine)
|
||||
factory = sessionmaker(bind=engine, expire_on_commit=False)
|
||||
repo = AutomationProfileRepository(session_factory=factory, auto_commit=True)
|
||||
return AutomationProfileService(repo=repo)
|
||||
|
||||
|
||||
def _make_profile(
|
||||
@@ -58,25 +79,31 @@ def _write_temp(context: Context, content: str) -> str:
|
||||
def step_coverage_runner(context: Context) -> None:
|
||||
context.runner = CliRunner()
|
||||
context.result = None
|
||||
import cleveragents.cli.commands.automation_profile as ap_mod
|
||||
|
||||
ap_mod._repo = _InMemoryProfileRepository()
|
||||
# Create an in-memory service and patch _get_service for this scenario
|
||||
context._ap_service = _create_in_memory_profile_service()
|
||||
context._ap_patcher = patch(
|
||||
"cleveragents.cli.commands.automation_profile._get_service",
|
||||
return_value=context._ap_service,
|
||||
)
|
||||
context._ap_patcher.start()
|
||||
if not hasattr(context, "_cleanup_handlers"):
|
||||
context._cleanup_handlers = []
|
||||
context._cleanup_handlers.append(context._ap_patcher.stop)
|
||||
|
||||
|
||||
@when("I delete a non-existent profile from the in-memory repo")
|
||||
def step_delete_nonexistent_from_repo(context: Context) -> None:
|
||||
repo = _InMemoryProfileRepository()
|
||||
context.repo_error = None
|
||||
try:
|
||||
repo.delete("nonexistent/profile")
|
||||
except NotFoundError as exc:
|
||||
context._ap_service._repo.delete("nonexistent/profile")
|
||||
except AutomationProfileNotFoundError as exc:
|
||||
context.repo_error = exc
|
||||
|
||||
|
||||
@then("a NotFoundError should be raised from the repo")
|
||||
def step_assert_not_found_error(context: Context) -> None:
|
||||
assert context.repo_error is not None, "Expected NotFoundError but none was raised"
|
||||
assert isinstance(context.repo_error, NotFoundError)
|
||||
assert isinstance(context.repo_error, AutomationProfileNotFoundError)
|
||||
|
||||
|
||||
@when("I call threshold summary on a built-in profile")
|
||||
@@ -180,10 +207,8 @@ def step_run_add_ca_error(context: Context) -> None:
|
||||
|
||||
@given('a custom coverage profile "{name}" has been added')
|
||||
def step_add_custom_coverage_profile(context: Context, name: str) -> None:
|
||||
import cleveragents.cli.commands.automation_profile as ap_mod
|
||||
|
||||
profile = _make_profile(name=name)
|
||||
ap_mod._repo.upsert(profile)
|
||||
context._ap_service._repo.upsert(profile)
|
||||
|
||||
|
||||
@when('I run automation-profile remove "{name}" without --yes and decline')
|
||||
|
||||
@@ -11,15 +11,33 @@ from behave import given, then, when
|
||||
from behave.runner import Context
|
||||
from typer.testing import CliRunner
|
||||
|
||||
from cleveragents.cli.commands.automation_profile import (
|
||||
_InMemoryProfileRepository,
|
||||
)
|
||||
from cleveragents.cli.commands.automation_profile import (
|
||||
app as profile_app,
|
||||
)
|
||||
from cleveragents.domain.models.core.automation_profile import AutomationProfile
|
||||
from cleveragents.domain.models.core.safety_profile import SafetyProfile
|
||||
|
||||
|
||||
def _create_in_memory_profile_service():
|
||||
"""Create an AutomationProfileService backed by an in-memory SQLite DB."""
|
||||
from sqlalchemy import create_engine
|
||||
from sqlalchemy.orm import sessionmaker
|
||||
|
||||
from cleveragents.application.services.automation_profile_service import (
|
||||
AutomationProfileService,
|
||||
)
|
||||
from cleveragents.infrastructure.database.models import Base
|
||||
from cleveragents.infrastructure.database.repositories import (
|
||||
AutomationProfileRepository,
|
||||
)
|
||||
|
||||
engine = create_engine("sqlite:///:memory:", echo=False)
|
||||
Base.metadata.create_all(engine)
|
||||
factory = sessionmaker(bind=engine, expire_on_commit=False)
|
||||
repo = AutomationProfileRepository(session_factory=factory, auto_commit=True)
|
||||
return AutomationProfileService(repo=repo)
|
||||
|
||||
|
||||
# Regex to strip ANSI escape sequences that Rich may emit.
|
||||
_ANSI_RE = re.compile(r"\x1b\[[0-9;]*[a-zA-Z]")
|
||||
|
||||
@@ -119,10 +137,16 @@ def step_automation_profile_cli_runner(context: Context) -> None:
|
||||
"""Set up the CLI runner for testing."""
|
||||
context.runner = CliRunner()
|
||||
context.result = None
|
||||
# Reset the module-level in-memory repo for each scenario
|
||||
import cleveragents.cli.commands.automation_profile as ap_mod
|
||||
|
||||
ap_mod._repo = _InMemoryProfileRepository()
|
||||
# Create an in-memory service and patch _get_service for this scenario
|
||||
context._ap_service = _create_in_memory_profile_service()
|
||||
context._ap_patcher = patch(
|
||||
"cleveragents.cli.commands.automation_profile._get_service",
|
||||
return_value=context._ap_service,
|
||||
)
|
||||
context._ap_patcher.start()
|
||||
if not hasattr(context, "_cleanup_handlers"):
|
||||
context._cleanup_handlers = []
|
||||
context._cleanup_handlers.append(context._ap_patcher.stop)
|
||||
|
||||
|
||||
@given("a valid automation profile config YAML file")
|
||||
@@ -134,10 +158,8 @@ def step_valid_profile_config(context: Context) -> None:
|
||||
@given('the custom profile "{name}" already exists')
|
||||
def step_custom_profile_exists(context: Context, name: str) -> None:
|
||||
"""Pre-register a custom profile in the in-memory repo."""
|
||||
import cleveragents.cli.commands.automation_profile as ap_mod
|
||||
|
||||
profile = _make_custom_profile(name=name)
|
||||
ap_mod._repo.upsert(profile)
|
||||
context._ap_service._repo.upsert(profile)
|
||||
|
||||
|
||||
@given("an invalid YAML automation profile config file")
|
||||
@@ -186,10 +208,8 @@ schema_version: "{version}"
|
||||
@given('a custom profile "{name}" has been added')
|
||||
def step_custom_profile_added(context: Context, name: str) -> None:
|
||||
"""Add a custom profile via the in-memory repo."""
|
||||
import cleveragents.cli.commands.automation_profile as ap_mod
|
||||
|
||||
profile = _make_custom_profile(name=name)
|
||||
ap_mod._repo.upsert(profile)
|
||||
context._ap_service._repo.upsert(profile)
|
||||
|
||||
|
||||
# ---- When steps ---- #
|
||||
|
||||
@@ -2,27 +2,49 @@
|
||||
"""Robot Framework helper for automation-profile CLI smoke tests.
|
||||
|
||||
Invoked by Robot tests to exercise the automation-profile CLI commands
|
||||
in isolation using the Typer CliRunner with mocked services.
|
||||
in isolation using the Typer CliRunner with a per-test in-memory SQLite
|
||||
database, fully isolated for parallel execution.
|
||||
|
||||
The module-level ``_get_service()`` in the production code calls
|
||||
``get_container()`` which requires full DI wiring. We monkey-patch it
|
||||
to return an ``AutomationProfileService`` backed by a fresh in-memory
|
||||
SQLite ``AutomationProfileRepository`` so the tests can run without
|
||||
container setup and without colliding across pabot workers.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
import sys
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
# Ensure the local source tree is importable
|
||||
_SRC = str(Path(__file__).resolve().parents[1] / "src")
|
||||
if _SRC not in sys.path:
|
||||
sys.path.insert(0, _SRC)
|
||||
|
||||
# Suppress debug-level structlog/retry noise that pollutes stdout in the
|
||||
# CliRunner capture buffer. Must happen before any cleveragents imports
|
||||
# that configure structlog.
|
||||
logging.getLogger().setLevel(logging.WARNING)
|
||||
|
||||
from sqlalchemy import create_engine # noqa: E402
|
||||
from sqlalchemy.orm import sessionmaker # noqa: E402
|
||||
from typer.testing import CliRunner # noqa: E402
|
||||
|
||||
import cleveragents.cli.commands.automation_profile as _ap_mod # noqa: E402
|
||||
from cleveragents.application.services.automation_profile_service import ( # noqa: E402
|
||||
AutomationProfileService,
|
||||
)
|
||||
from cleveragents.infrastructure.database.models import Base # noqa: E402
|
||||
from cleveragents.infrastructure.database.repositories import ( # noqa: E402
|
||||
AutomationProfileRepository,
|
||||
)
|
||||
|
||||
_InMemoryProfileRepository = _ap_mod._InMemoryProfileRepository
|
||||
profile_app = _ap_mod.app
|
||||
|
||||
_runner = CliRunner()
|
||||
@@ -49,21 +71,62 @@ safety:
|
||||
"""
|
||||
|
||||
|
||||
def _reset_repo() -> None:
|
||||
"""Reset the module-level in-memory repo."""
|
||||
import cleveragents.cli.commands.automation_profile as ap_mod
|
||||
def _extract_json(text: str) -> object:
|
||||
"""Extract and parse the first valid JSON object or array from *text*.
|
||||
|
||||
ap_mod._repo = _InMemoryProfileRepository()
|
||||
``format_output`` writes JSON directly to ``sys.stdout`` which the
|
||||
CliRunner captures. Structlog debug messages may precede the JSON
|
||||
payload. This function scans for the first ``{`` or ``[`` and
|
||||
parses from there.
|
||||
"""
|
||||
for i, ch in enumerate(text):
|
||||
if ch in ("{", "["):
|
||||
try:
|
||||
return json.loads(text[i:])
|
||||
except json.JSONDecodeError:
|
||||
continue
|
||||
raise ValueError(f"No valid JSON found in output:\n{text[:500]}")
|
||||
|
||||
|
||||
def _make_isolated_service() -> AutomationProfileService:
|
||||
"""Create a fully isolated AutomationProfileService with an in-memory DB."""
|
||||
engine = create_engine("sqlite:///:memory:", echo=False)
|
||||
Base.metadata.create_all(engine)
|
||||
factory = sessionmaker(bind=engine, expire_on_commit=False)
|
||||
repo = AutomationProfileRepository(session_factory=factory, auto_commit=True)
|
||||
return AutomationProfileService(repo=repo)
|
||||
|
||||
|
||||
# Module-level service instance that is reset before each test.
|
||||
_isolated_service: AutomationProfileService | None = None
|
||||
|
||||
|
||||
def _get_service_override() -> AutomationProfileService:
|
||||
"""Replacement for ``_ap_mod._get_service`` that returns the test service."""
|
||||
assert _isolated_service is not None, "Call _reset_service() first"
|
||||
return _isolated_service
|
||||
|
||||
|
||||
def _reset_service() -> None:
|
||||
"""Create a fresh isolated service for the next test."""
|
||||
global _isolated_service
|
||||
_isolated_service = _make_isolated_service()
|
||||
|
||||
|
||||
def _invoke(args: list[str]):
|
||||
"""Invoke the profile CLI with the patched service."""
|
||||
with patch.object(_ap_mod, "_get_service", _get_service_override):
|
||||
return _runner.invoke(profile_app, args)
|
||||
|
||||
|
||||
def test_add_profile() -> None:
|
||||
"""Test adding a profile via YAML config."""
|
||||
_reset_repo()
|
||||
_reset_service()
|
||||
fd, path = tempfile.mkstemp(suffix=".yaml")
|
||||
try:
|
||||
with os.fdopen(fd, "w") as fh:
|
||||
fh.write(_VALID_YAML)
|
||||
result = _runner.invoke(profile_app, ["add", "--config", path])
|
||||
result = _invoke(["add", "--config", path])
|
||||
assert result.exit_code == 0, f"add failed: {result.output}"
|
||||
print("add-profile-ok")
|
||||
finally:
|
||||
@@ -72,8 +135,8 @@ def test_add_profile() -> None:
|
||||
|
||||
def test_show_profile() -> None:
|
||||
"""Test showing a built-in profile."""
|
||||
_reset_repo()
|
||||
result = _runner.invoke(profile_app, ["show", "manual"])
|
||||
_reset_service()
|
||||
result = _invoke(["show", "manual"])
|
||||
assert result.exit_code == 0, f"show failed: {result.output}"
|
||||
assert "manual" in result.output
|
||||
print("show-profile-ok")
|
||||
@@ -81,19 +144,19 @@ def test_show_profile() -> None:
|
||||
|
||||
def test_show_json() -> None:
|
||||
"""Test showing a profile in JSON format."""
|
||||
_reset_repo()
|
||||
result = _runner.invoke(profile_app, ["show", "manual", "--format", "json"])
|
||||
_reset_service()
|
||||
result = _invoke(["show", "manual", "--format", "json"])
|
||||
assert result.exit_code == 0, f"show json failed: {result.output}"
|
||||
# Verify it's valid JSON
|
||||
parsed = json.loads(result.output)
|
||||
parsed = _extract_json(result.output)
|
||||
assert isinstance(parsed, dict)
|
||||
assert parsed["name"] == "manual"
|
||||
print("show-json-ok")
|
||||
|
||||
|
||||
def test_show_yaml() -> None:
|
||||
"""Test showing a profile in YAML format."""
|
||||
_reset_repo()
|
||||
result = _runner.invoke(profile_app, ["show", "manual", "--format", "yaml"])
|
||||
_reset_service()
|
||||
result = _invoke(["show", "manual", "--format", "yaml"])
|
||||
assert result.exit_code == 0, f"show yaml failed: {result.output}"
|
||||
assert "name: manual" in result.output
|
||||
print("show-yaml-ok")
|
||||
@@ -101,8 +164,8 @@ def test_show_yaml() -> None:
|
||||
|
||||
def test_list_profiles() -> None:
|
||||
"""Test listing all profiles."""
|
||||
_reset_repo()
|
||||
result = _runner.invoke(profile_app, ["list"])
|
||||
_reset_service()
|
||||
result = _invoke(["list"])
|
||||
assert result.exit_code == 0, f"list failed: {result.output}"
|
||||
assert "manual" in result.output
|
||||
print("list-profiles-ok")
|
||||
@@ -110,10 +173,10 @@ def test_list_profiles() -> None:
|
||||
|
||||
def test_list_json() -> None:
|
||||
"""Test listing profiles in JSON format."""
|
||||
_reset_repo()
|
||||
result = _runner.invoke(profile_app, ["list", "--format", "json"])
|
||||
_reset_service()
|
||||
result = _invoke(["list", "--format", "json"])
|
||||
assert result.exit_code == 0, f"list json failed: {result.output}"
|
||||
parsed = json.loads(result.output)
|
||||
parsed = _extract_json(result.output)
|
||||
assert isinstance(parsed, list)
|
||||
assert len(parsed) >= 8 # At least 8 built-in profiles
|
||||
print("list-json-ok")
|
||||
@@ -121,19 +184,19 @@ def test_list_json() -> None:
|
||||
|
||||
def test_remove_profile() -> None:
|
||||
"""Test removing a custom profile."""
|
||||
_reset_repo()
|
||||
_reset_service()
|
||||
# First add a profile
|
||||
fd, path = tempfile.mkstemp(suffix=".yaml")
|
||||
try:
|
||||
with os.fdopen(fd, "w") as fh:
|
||||
fh.write(_VALID_YAML)
|
||||
add_result = _runner.invoke(profile_app, ["add", "--config", path])
|
||||
add_result = _invoke(["add", "--config", path])
|
||||
assert add_result.exit_code == 0, f"add failed: {add_result.output}"
|
||||
finally:
|
||||
Path(path).unlink(missing_ok=True)
|
||||
|
||||
# Then remove it
|
||||
result = _runner.invoke(profile_app, ["remove", "acme/robot-test", "--yes"])
|
||||
# Then remove it (reuse same service so the profile persists)
|
||||
result = _invoke(["remove", "acme/robot-test", "--yes"])
|
||||
assert result.exit_code == 0, f"remove failed: {result.output}"
|
||||
assert "removed" in result.output.lower()
|
||||
print("remove-profile-ok")
|
||||
@@ -141,8 +204,8 @@ def test_remove_profile() -> None:
|
||||
|
||||
def test_remove_builtin_fails() -> None:
|
||||
"""Test that removing a built-in profile fails."""
|
||||
_reset_repo()
|
||||
result = _runner.invoke(profile_app, ["remove", "manual", "--yes"])
|
||||
_reset_service()
|
||||
result = _invoke(["remove", "manual", "--yes"])
|
||||
assert result.exit_code != 0, f"remove should have failed: {result.output}"
|
||||
print("remove-builtin-fails-ok")
|
||||
|
||||
|
||||
@@ -46,41 +46,23 @@ console = Console()
|
||||
_FORMAT_HELP = "Output format: json, yaml, plain, table, or rich (default: rich)"
|
||||
|
||||
|
||||
class _InMemoryProfileRepository:
|
||||
"""In-memory repository for custom automation profiles."""
|
||||
|
||||
def __init__(self) -> None:
|
||||
self._profiles: dict[str, AutomationProfile] = {}
|
||||
|
||||
def get_by_name(self, name: str) -> AutomationProfile | None:
|
||||
return self._profiles.get(name)
|
||||
|
||||
def list_all(self) -> list[AutomationProfile]:
|
||||
return [self._profiles[k] for k in sorted(self._profiles)]
|
||||
|
||||
def upsert(
|
||||
self,
|
||||
profile: AutomationProfile,
|
||||
expected_schema_version: str | None = None,
|
||||
) -> None:
|
||||
self._profiles[profile.name] = profile
|
||||
|
||||
def delete(self, name: str) -> None:
|
||||
if name not in self._profiles:
|
||||
raise NotFoundError(
|
||||
f"Profile '{name}' not found",
|
||||
resource_type="automation_profile",
|
||||
resource_id=name,
|
||||
)
|
||||
del self._profiles[name]
|
||||
|
||||
|
||||
_repo = _InMemoryProfileRepository()
|
||||
|
||||
|
||||
def _get_service() -> AutomationProfileService:
|
||||
"""Get the AutomationProfileService with in-memory repository."""
|
||||
repo: Any = _repo
|
||||
"""Get the AutomationProfileService with database-backed repository."""
|
||||
from cleveragents.application.container import get_container
|
||||
|
||||
container = get_container()
|
||||
database_url: str = container.database_url()
|
||||
|
||||
from sqlalchemy import create_engine
|
||||
from sqlalchemy.orm import sessionmaker
|
||||
|
||||
from cleveragents.infrastructure.database.repositories import (
|
||||
AutomationProfileRepository,
|
||||
)
|
||||
|
||||
engine = create_engine(database_url, echo=False)
|
||||
factory = sessionmaker(bind=engine, expire_on_commit=False)
|
||||
repo = AutomationProfileRepository(session_factory=factory, auto_commit=True)
|
||||
return AutomationProfileService(repo=repo)
|
||||
|
||||
|
||||
|
||||
@@ -2082,11 +2082,15 @@ class AutomationProfileModel(Base): # type: ignore[misc]
|
||||
auto_retry_transient = Column(Float, nullable=False, default=0.0)
|
||||
auto_checkpoint_restore = Column(Float, nullable=False, default=0.0)
|
||||
|
||||
# Safety requirements
|
||||
# Safety requirements (legacy scalar columns kept for queries)
|
||||
require_sandbox = Column(Boolean, nullable=False, default=True)
|
||||
require_checkpoints = Column(Boolean, nullable=False, default=True)
|
||||
allow_unsafe_tools = Column(Boolean, nullable=False, default=False)
|
||||
|
||||
# Full-fidelity JSON for SafetyProfile and AutomationGuard sub-models
|
||||
safety_json = Column(Text, nullable=True)
|
||||
guards_json = Column(Text, nullable=True)
|
||||
|
||||
# Timestamps (ISO-8601 strings)
|
||||
created_at = Column(Text, nullable=False)
|
||||
updated_at = Column(Text, nullable=False)
|
||||
|
||||
@@ -1447,6 +1447,25 @@ class LifecyclePlanRepository:
|
||||
session.rollback()
|
||||
raise DatabaseError(f"Failed to update plan {plan_id_str}: {exc}") from exc
|
||||
|
||||
@database_retry
|
||||
def list_all(self) -> list[Any]:
|
||||
"""List all persisted lifecycle plans.
|
||||
|
||||
Returns:
|
||||
List of ``Plan`` domain objects ordered by creation time
|
||||
(newest first).
|
||||
"""
|
||||
session = self._session()
|
||||
try:
|
||||
rows = (
|
||||
session.query(LifecyclePlanModel)
|
||||
.order_by(LifecyclePlanModel.created_at.desc())
|
||||
.all()
|
||||
)
|
||||
return [r.to_domain() for r in rows]
|
||||
except (OperationalError, SQLAlchemyDatabaseError) as exc:
|
||||
raise DatabaseError(f"Failed to list plans: {exc}") from exc
|
||||
|
||||
@database_retry
|
||||
def delete(self, plan_id: str) -> bool:
|
||||
"""Delete a plan by ULID, cascading to child tables.
|
||||
@@ -4223,11 +4242,29 @@ class AutomationProfileRepository:
|
||||
|
||||
Uses the session-factory pattern consistent with other repositories.
|
||||
Built-in profiles are NOT stored here.
|
||||
|
||||
All mutating methods flush but do NOT commit by default; the caller
|
||||
or a ``UnitOfWork`` wrapper is responsible for committing the
|
||||
transaction. When ``auto_commit`` is ``True`` (e.g. CLI usage
|
||||
outside a UoW), each method commits and closes its own session.
|
||||
"""
|
||||
|
||||
def __init__(self, session_factory: Callable[[], Session]) -> None:
|
||||
"""Initialise with a callable that returns a Session."""
|
||||
def __init__(
|
||||
self,
|
||||
session_factory: Callable[[], Session],
|
||||
*,
|
||||
auto_commit: bool = False,
|
||||
) -> None:
|
||||
"""Initialise with a callable that returns a new SQLAlchemy Session.
|
||||
|
||||
Args:
|
||||
session_factory: Factory returning a new SQLAlchemy ``Session``.
|
||||
auto_commit: When ``True``, each public method commits and
|
||||
closes its session automatically. Useful for CLI commands
|
||||
that operate outside a ``UnitOfWork``.
|
||||
"""
|
||||
self._session_factory = session_factory
|
||||
self._auto_commit = auto_commit
|
||||
|
||||
def _session(self) -> Session:
|
||||
"""Convenience helper to obtain a session."""
|
||||
@@ -4319,6 +4356,8 @@ class AutomationProfileRepository:
|
||||
row = self._from_domain(profile, now_iso)
|
||||
session.add(row)
|
||||
session.flush()
|
||||
if self._auto_commit:
|
||||
session.commit()
|
||||
except AutomationProfileSchemaVersionError:
|
||||
raise
|
||||
except IntegrityError as exc:
|
||||
@@ -4353,6 +4392,8 @@ class AutomationProfileRepository:
|
||||
raise AutomationProfileNotFoundError(f"Profile '{name}' not found")
|
||||
session.delete(row)
|
||||
session.flush()
|
||||
if self._auto_commit:
|
||||
session.commit()
|
||||
except AutomationProfileNotFoundError:
|
||||
raise
|
||||
except (
|
||||
@@ -4367,11 +4408,38 @@ class AutomationProfileRepository:
|
||||
row: AutomationProfileModel,
|
||||
) -> Any:
|
||||
"""Convert an ORM row to an AutomationProfile domain object."""
|
||||
import json as _json
|
||||
|
||||
from cleveragents.domain.models.core.automation_profile import (
|
||||
AutomationGuard,
|
||||
AutomationProfile,
|
||||
)
|
||||
from cleveragents.domain.models.core.safety_profile import SafetyProfile
|
||||
|
||||
# Restore full safety from JSON if available, else fall back to
|
||||
# legacy scalar columns for backward compatibility.
|
||||
safety_json_str = (
|
||||
cast(str | None, row.safety_json) if hasattr(row, "safety_json") else None
|
||||
)
|
||||
if safety_json_str:
|
||||
safety = SafetyProfile(**_json.loads(safety_json_str))
|
||||
else:
|
||||
safety = SafetyProfile(
|
||||
require_sandbox=bool(row.require_sandbox),
|
||||
require_checkpoints=bool(row.require_checkpoints),
|
||||
require_human_approval=False,
|
||||
allow_unsafe_tools=bool(row.allow_unsafe_tools),
|
||||
max_retries_per_step=3,
|
||||
)
|
||||
|
||||
# Restore guards from JSON if available.
|
||||
guards_json_str = (
|
||||
cast(str | None, row.guards_json) if hasattr(row, "guards_json") else None
|
||||
)
|
||||
guards: AutomationGuard | None = None
|
||||
if guards_json_str:
|
||||
guards = AutomationGuard(**_json.loads(guards_json_str))
|
||||
|
||||
return AutomationProfile(
|
||||
name=cast(str, row.name),
|
||||
description=cast(str, row.description) or "",
|
||||
@@ -4387,18 +4455,8 @@ class AutomationProfileRepository:
|
||||
auto_child_plans=float(cast(float, row.auto_child_plans)),
|
||||
auto_retry_transient=float(cast(float, row.auto_retry_transient)),
|
||||
auto_checkpoint_restore=float(cast(float, row.auto_checkpoint_restore)),
|
||||
# NOTE: require_human_approval and max_retries_per_step are not
|
||||
# persisted in the automation_profiles table (only the original
|
||||
# three safety booleans are stored). Defaults are used here for
|
||||
# backwards compatibility. See c4_001_safety_profile_column for
|
||||
# the Action-level safety_profile_json column.
|
||||
safety=SafetyProfile(
|
||||
require_sandbox=bool(row.require_sandbox),
|
||||
require_checkpoints=bool(row.require_checkpoints),
|
||||
require_human_approval=False,
|
||||
allow_unsafe_tools=bool(row.allow_unsafe_tools),
|
||||
max_retries_per_step=3,
|
||||
),
|
||||
safety=safety,
|
||||
guards=guards,
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
@@ -4407,6 +4465,15 @@ class AutomationProfileRepository:
|
||||
now_iso: str,
|
||||
) -> AutomationProfileModel:
|
||||
"""Convert an AutomationProfile to an ORM row."""
|
||||
import json as _json
|
||||
|
||||
safety_json = (
|
||||
_json.dumps(profile.safety.model_dump()) if profile.safety else None
|
||||
)
|
||||
guards_json = (
|
||||
_json.dumps(profile.guards.model_dump()) if profile.guards else None
|
||||
)
|
||||
|
||||
return AutomationProfileModel(
|
||||
name=profile.name,
|
||||
description=profile.description,
|
||||
@@ -4425,6 +4492,8 @@ class AutomationProfileRepository:
|
||||
require_sandbox=profile.safety.require_sandbox,
|
||||
require_checkpoints=profile.safety.require_checkpoints,
|
||||
allow_unsafe_tools=profile.safety.allow_unsafe_tools,
|
||||
safety_json=safety_json,
|
||||
guards_json=guards_json,
|
||||
created_at=now_iso,
|
||||
updated_at=now_iso,
|
||||
)
|
||||
@@ -4482,6 +4551,14 @@ class AutomationProfileRepository:
|
||||
row.allow_unsafe_tools = ( # type: ignore[assignment]
|
||||
profile.safety.allow_unsafe_tools
|
||||
)
|
||||
import json as _json
|
||||
|
||||
row.safety_json = ( # type: ignore[assignment]
|
||||
_json.dumps(profile.safety.model_dump()) if profile.safety else None
|
||||
)
|
||||
row.guards_json = ( # type: ignore[assignment]
|
||||
_json.dumps(profile.guards.model_dump()) if profile.guards else None
|
||||
)
|
||||
row.updated_at = now_iso # type: ignore[assignment]
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user