diff --git a/features/steps/tdd_resource_remove_children_guard_steps.py b/features/steps/tdd_resource_remove_children_guard_steps.py new file mode 100644 index 000000000..0deabcb39 --- /dev/null +++ b/features/steps/tdd_resource_remove_children_guard_steps.py @@ -0,0 +1,29 @@ +"""Step definitions for resource removal linked-children guard TDD tests.""" + +from __future__ import annotations + +from behave import given # type: ignore[import-untyped] +from behave.runner import Context # type: ignore[import-untyped] + +from cleveragents.application.services.resource_registry_service import ( + ResourceRegistryService, +) + +from features.steps import resource_cli_steps as resource_cli + + +def _get_cli_service(context: Context) -> ResourceRegistryService: + """Ensure the resource CLI service is available for linking operations.""" + + try: + return context.resource_cli_service # type: ignore[attr-defined] + except AttributeError: + return resource_cli._make_service(context) + + +@given('the resource registry CLI links child "{child}" under parent "{parent}"') +def step_link_child(context: Context, child: str, parent: str) -> None: + """Link a child resource beneath a parent using the shared CLI service.""" + + service = _get_cli_service(context) + service.link_child(parent, child) diff --git a/features/tdd_resource_remove_children_guard.feature b/features/tdd_resource_remove_children_guard.feature new file mode 100644 index 000000000..b040e8476 --- /dev/null +++ b/features/tdd_resource_remove_children_guard.feature @@ -0,0 +1,13 @@ +@tdd_issue +@tdd_issue_6886 +Feature: Resource removal guard respects manual child links + + Scenario: Prevent deleting a resource that still has linked children + Given a fresh in-memory resource registry + And built-in types are bootstrapped + And I run resource add "git-checkout" "local/test-repo" with path "demo_repo" + And I run resource add "fs-directory" "local/test-repo/src" with path "demo_repo/src" + And the resource registry CLI links child "local/test-repo/src" under parent "local/test-repo" + When I run resource remove "local/test-repo" with yes flag + Then the resource command should fail + And the resource output should contain "link(s) still reference" diff --git a/robot/helper_tdd_resource_remove_children_guard.py b/robot/helper_tdd_resource_remove_children_guard.py new file mode 100644 index 000000000..9da7a60a2 --- /dev/null +++ b/robot/helper_tdd_resource_remove_children_guard.py @@ -0,0 +1,169 @@ + +"""Helper script for tdd_resource_remove_children_guard.robot integration tests.""" + +from __future__ import annotations + +import contextlib +import os +import sys +from pathlib import Path +from typing import Iterator, NoReturn + +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker +from sqlalchemy.pool import StaticPool +from typer.testing import CliRunner + +ROOT: Path = Path(__file__).resolve().parents[1] +SRC_DIR: str = str(ROOT / "src") +if SRC_DIR not in sys.path: + sys.path.insert(0, SRC_DIR) + +from cleveragents.application.services.resource_registry_service import ( + ResourceRegistryService, +) +from cleveragents.core.exceptions import NotFoundError +from cleveragents.infrastructure.database.models import Base +from cleveragents.cli.commands.resource import app as resource_app + +RUNNER: CliRunner = CliRunner() + + +def _fail(message: str) -> NoReturn: + """Exit the helper with an error message and status 1.""" + + print(message, file=sys.stderr) + raise SystemExit(1) + + +def _make_service() -> ResourceRegistryService: + """Create an isolated in-memory ResourceRegistryService instance.""" + + engine = create_engine( + "sqlite:///:memory:", + echo=False, + connect_args={"check_same_thread": False}, + poolclass=StaticPool, + ) + Base.metadata.create_all(engine) + factory = sessionmaker(bind=engine, expire_on_commit=False) + service = ResourceRegistryService(session_factory=factory) + service.bootstrap_builtin_types() + return service + + +@contextlib.contextmanager +def _patched_service(service: ResourceRegistryService) -> Iterator[None]: + """Patch the CLI hook so it returns *service* during the context.""" + + from unittest.mock import patch + + target = "cleveragents.cli.commands.resource._get_registry_service" + with patch(target, return_value=service): + yield + + +def _register_sample_resources(service: ResourceRegistryService) -> tuple[str, str]: + """Register parent/child resources and return their names.""" + + parent_name = "local/test-repo" + child_name = "local/test-repo/src" + + service.register_resource( + "git-checkout", + name=parent_name, + location="demo_repo", + ) + service.register_resource( + "fs-directory", + name=child_name, + location="demo_repo/src", + ) + return parent_name, child_name + + +def _invoke(service: ResourceRegistryService, *args: str): + """Invoke the resource CLI with the provided *args* under the patched service.""" + + env = os.environ.copy() + env.setdefault("NO_COLOR", "1") + + with _patched_service(service): + return RUNNER.invoke(resource_app, list(args), env=env) + + +def _scenario_block_parent_removal() -> None: + """Verify the guard blocks removal when DAG links exist.""" + + service = _make_service() + parent_name, child_name = _register_sample_resources(service) + service.link_child(parent_name, child_name) + + result = _invoke(service, "remove", "--yes", parent_name) + + if result.exit_code == 0: + _fail( + "resource remove succeeded unexpectedly despite existing DAG links." + ) + if "link(s) still reference" not in result.stdout: + _fail( + "resource remove did not report remaining links when guard triggered.\n" + f"STDOUT:\n{result.stdout}\nSTDERR:\n{result.stderr}" + ) + + try: + service.show_resource(parent_name) + except NotFoundError as exc: + _fail(f"Parent resource was deleted despite guard: {exc}") + + print("tdd-resource-remove-guard-parent-blocks-ok") + + +def _scenario_unlink_then_remove() -> None: + """Verify removal succeeds once DAG links are removed.""" + + service = _make_service() + parent_name, child_name = _register_sample_resources(service) + service.link_child(parent_name, child_name) + service.unlink_child(parent_name, child_name) + + result = _invoke(service, "remove", "--yes", parent_name) + + if result.exit_code != 0: + _fail( + "resource remove should succeed after unlinking children.\n" + f"STDOUT:\n{result.stdout}\nSTDERR:\n{result.stderr}" + ) + if "Removed resource:" not in result.stdout: + _fail( + "resource remove did not report successful deletion after unlinking.\n" + f"STDOUT:\n{result.stdout}\nSTDERR:\n{result.stderr}" + ) + + try: + service.show_resource(parent_name) + except NotFoundError: + print("tdd-resource-remove-guard-unlink-then-remove-ok") + return + _fail("Parent resource is still present after successful removal.") + + +def main(argv: list[str]) -> int: + """Entry point dispatcher for helper subcommands.""" + + if len(argv) < 2: + _fail("Missing subcommand: expected 'block-parent-removal' or 'unlink-then-remove'.") + + command = argv[1] + if command == "block-parent-removal": + _scenario_block_parent_removal() + return 0 + if command == "unlink-then-remove": + _scenario_unlink_then_remove() + return 0 + + _fail(f"Unknown subcommand: {command}") + + +if __name__ == "__main__": + raise SystemExit(main(sys.argv)) diff --git a/robot/tdd_resource_remove_children_guard.robot b/robot/tdd_resource_remove_children_guard.robot new file mode 100644 index 000000000..7e42fbdeb --- /dev/null +++ b/robot/tdd_resource_remove_children_guard.robot @@ -0,0 +1,39 @@ + +*** Settings *** +Documentation TDD Bug #6886 — resource removal guard must block linked resources +... Integration regression tests ensuring `agents resource remove` +... refuses to delete resources that still participate in the +... Resource DAG and succeeds after links are removed. +Resource ${CURDIR}/common.resource +Suite Setup Setup Test Environment +Suite Teardown Cleanup Test Environment + +*** Variables *** +${HELPER} ${CURDIR}/helper_tdd_resource_remove_children_guard.py + +*** Test Cases *** +TDD Resource Remove Blocks Linked Parent + [Documentation] Ensure the CLI refuses to delete a parent resource while + ... it retains linked children, reproducing bug #6886. + [Tags] tdd_issue tdd_issue_6886 + ${result}= Run Process ${PYTHON} ${HELPER} block-parent-removal + ... cwd=${WORKSPACE} + ... timeout=60s + ... on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tdd-resource-remove-guard-parent-blocks-ok + +TDD Resource Remove Allows Delete After Unlink + [Documentation] Verify removal succeeds after unlinking children so the + ... guard does not over-protect once DAG edges are cleared. + [Tags] tdd_issue tdd_issue_6886 + ${result}= Run Process ${PYTHON} ${HELPER} unlink-then-remove + ... cwd=${WORKSPACE} + ... timeout=60s + ... on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tdd-resource-remove-guard-unlink-then-remove-ok diff --git a/src/cleveragents/cli/commands/resource.py b/src/cleveragents/cli/commands/resource.py index 1dc7457ef..54ccf40b0 100644 --- a/src/cleveragents/cli/commands/resource.py +++ b/src/cleveragents/cli/commands/resource.py @@ -1377,23 +1377,23 @@ def resource_remove( session = service._session() try: from cleveragents.infrastructure.database.models import ( - ResourceEdgeModel, + ResourceLinkModel, ResourceModel, ) - # Check for edges - edge_count: int = ( - session.query(ResourceEdgeModel) + # Check for existing DAG links + link_count: int = ( + session.query(ResourceLinkModel) .filter( - (ResourceEdgeModel.parent_id == res.resource_id) - | (ResourceEdgeModel.child_id == res.resource_id) + (ResourceLinkModel.parent_id == res.resource_id) + | (ResourceLinkModel.child_id == res.resource_id) ) .count() ) - if edge_count > 0: + if link_count > 0: console.print( f"[red]Cannot remove resource '{res.name or res.resource_id}': " - f"{edge_count} edge(s) still reference it.[/red]" + f"{link_count} link(s) still reference it.[/red]" ) raise typer.Abort()