From 72cd40cd9e153e0d325c6a3bd81c8a8f90b3fa57 Mon Sep 17 00:00:00 2001 From: CleverThis Date: Fri, 10 Apr 2026 09:12:43 +0000 Subject: [PATCH] fix(resource): enforce resource removal guard Switch the guard to consult ResourceLinkModel so linked DAG edges prevent parent deletion. Adds Behave and Robot regression coverage to capture the edge cases introduced by issue #6886. ISSUES CLOSED: #6886 --- ...dd_resource_remove_children_guard_steps.py | 29 +++ ...tdd_resource_remove_children_guard.feature | 13 ++ ...lper_tdd_resource_remove_children_guard.py | 169 ++++++++++++++++++ .../tdd_resource_remove_children_guard.robot | 39 ++++ src/cleveragents/cli/commands/resource.py | 16 +- 5 files changed, 258 insertions(+), 8 deletions(-) create mode 100644 features/steps/tdd_resource_remove_children_guard_steps.py create mode 100644 features/tdd_resource_remove_children_guard.feature create mode 100644 robot/helper_tdd_resource_remove_children_guard.py create mode 100644 robot/tdd_resource_remove_children_guard.robot 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()