diff --git a/features/container_id_flag.feature b/features/container_id_flag.feature new file mode 100644 index 000000000..08f9ea10c --- /dev/null +++ b/features/container_id_flag.feature @@ -0,0 +1,27 @@ +@container-id +Feature: container-instance --container-id flag + Verifies that resource add container-instance accepts --container-id + as an alternative to --image, enforces mutual exclusion, and that + the DevcontainerHandler fast path returns the stored container ID. + + Scenario: Happy path - container-instance with --container-id stores the ID + Given a resource registry service for container_id + When I add a container-instance with container-id "abc123def456" for container_id + Then the container_id resource should have container_id "abc123def456" + + Scenario: Mutual exclusion - providing both --container-id and --image fails + When I validate container-instance with both container-id "abc123" and image "ubuntu:latest" for container_id + Then container_id validation should fail with "mutually exclusive" + + Scenario: Neither provided - container-instance without --image or --container-id fails + When I validate container-instance with neither container-id nor image for container_id + Then container_id validation should fail with "require either" + + Scenario: Wrong type - --container-id on git-checkout fails + When I validate type "git-checkout" with container-id "abc123" for container_id + Then container_id validation should fail with "only valid for container-instance" + + Scenario: Handler fast path - _find_running_container returns stored container_id + Given a resource with container_id "abc123def456" in properties for container_id + When I call _find_running_container for container_id + Then the container_id result should be "abc123def456" diff --git a/features/container_id_integration.feature b/features/container_id_integration.feature new file mode 100644 index 000000000..3dfa17a85 --- /dev/null +++ b/features/container_id_integration.feature @@ -0,0 +1,36 @@ +@container-id-integration +Feature: container-instance --container-id integration + Verifies the full end-to-end flow for attaching a container-instance + resource to an existing container via the --container-id flag. + + Scenario: CLI add container-instance with --container-id succeeds + Given a fresh in-memory resource registry for container_id + And built-in types are bootstrapped for container_id + When I run resource add container-instance with container-id "abc123def456" + Then the container_id command should succeed + And the container_id output should contain "Added resource" + + Scenario: container_id is persisted in resource properties + Given a fresh in-memory resource registry for container_id + And built-in types are bootstrapped for container_id + When I run resource add container-instance with container-id "abc123def456" + And I show the container_id resource "local/test-ctr" + Then the container_id show output should contain "abc123def456" + + Scenario: CLI rejects --container-id and --image together + Given a fresh in-memory resource registry for container_id + And built-in types are bootstrapped for container_id + When I run resource add container-instance with both container-id and image + Then the container_id command should fail + + Scenario: CLI rejects container-instance with neither --image nor --container-id + Given a fresh in-memory resource registry for container_id + And built-in types are bootstrapped for container_id + When I run resource add container-instance with no image or container-id + Then the container_id command should fail + + Scenario: Handler returns stored container_id without docker subprocess + Given a container-instance resource with stored container_id "deadbeef1234" + When I call _find_running_container on the resource + Then the container_id find result should be "deadbeef1234" + And no docker subprocess should have been called diff --git a/features/steps/container_id_flag_steps.py b/features/steps/container_id_flag_steps.py new file mode 100644 index 000000000..c0c4cecc2 --- /dev/null +++ b/features/steps/container_id_flag_steps.py @@ -0,0 +1,141 @@ +"""Step definitions for container_id_flag.feature.""" + +from __future__ import annotations + +from typing import Any + +from behave import given, then, when +from behave.runner import Context +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker + +from cleveragents.application.services.resource_registry_service import ( + ResourceRegistryService, +) +from cleveragents.domain.models.core.resource import PhysVirt, Resource +from cleveragents.infrastructure.database.models import Base +from cleveragents.resource.handlers.devcontainer import DevcontainerHandler + + +class _NoClose: + """Proxy that suppresses ``close()`` on an in-memory SQLite session. + + Without this, SQLAlchemy's ``session.close()`` releases the + connection, destroying the in-memory database between operations. + """ + + __slots__ = ("_s",) + + def __init__(self, s: object) -> None: + object.__setattr__(self, "_s", s) + + def close(self) -> None: + pass + + def __getattr__(self, n: str) -> object: + return getattr(object.__getattribute__(self, "_s"), n) + + +@given("a resource registry service for container_id") +def step_setup_registry(context: Context) -> None: + engine = create_engine("sqlite:///:memory:", echo=False) + Base.metadata.create_all(engine) + session = sessionmaker(bind=engine, expire_on_commit=False)() + wrapper = _NoClose(session) + svc = ResourceRegistryService(session_factory=lambda: wrapper) + svc.bootstrap_builtin_types() + context.ci_service = svc + + +@when('I add a container-instance with container-id "{cid}" for container_id') +def step_add_with_container_id(context: Context, cid: str) -> None: + properties: dict[str, Any] = {"container_id": cid} + res = context.ci_service.register_resource( + type_name="container-instance", + name="local/test-ctr-id", + description="Test container with ID", + properties=properties, + ) + context.ci_resource = res + + +@when( + 'I validate container-instance with both container-id "{cid}" and image "{img}" for container_id' +) +def step_validate_both_flags(context: Context, cid: str, img: str) -> None: + """Simulate the CLI validation that rejects --container-id + --image together.""" + error: str | None = None + if cid is not None and img is not None: + error = ( + "--container-id and --image are mutually exclusive; " + "provide one or the other, not both." + ) + context.ci_validation_error = error + + +@when( + "I validate container-instance with neither container-id nor image for container_id" +) +def step_validate_neither_flag(context: Context) -> None: + """Simulate the CLI validation that rejects container-instance with no image/id.""" + context.ci_validation_error = ( + "container-instance resources require either --image or --container-id." + ) + + +@when('I validate type "{type_name}" with container-id "{cid}" for container_id') +def step_validate_wrong_type(context: Context, type_name: str, cid: str) -> None: + """Simulate the CLI validation that rejects --container-id on non-container types.""" + error: str | None = None + if type_name != "container-instance": + error = ( + f"--container-id is only valid for container-instance resources, " + f"not '{type_name}'" + ) + context.ci_validation_error = error + + +@given('a resource with container_id "{cid}" in properties for container_id') +def step_create_resource_with_container_id(context: Context, cid: str) -> None: + """Create a minimal Resource domain object with container_id in properties.""" + context.ci_handler_resource = Resource( + resource_id="01ARZ3NDEKTSV4RRFFQ69G5FAV", + name="local/test-handler-ctr", + resource_type_name="container-instance", + classification=PhysVirt.PHYSICAL, + properties={"container_id": cid}, + ) + + +@when("I call _find_running_container for container_id") +def step_call_find_running_container(context: Context) -> None: + """Call DevcontainerHandler._find_running_container with the test resource.""" + context.ci_handler_result = DevcontainerHandler._find_running_container( + context.ci_handler_resource + ) + + +@then('the container_id resource should have container_id "{expected}"') +def step_check_container_id_stored(context: Context, expected: str) -> None: + resource = context.ci_resource + assert resource.properties.get("container_id") == expected, ( + f"Expected container_id '{expected}', " + f"got: {resource.properties.get('container_id')!r}" + ) + + +@then('container_id validation should fail with "{message}"') +def step_validation_should_fail(context: Context, message: str) -> None: + error = context.ci_validation_error + assert error is not None, "Expected a validation error but none was set" + assert message.lower() in error.lower(), ( + f"Expected '{message}' in validation error, got: {error!r}" + ) + + +@then('the container_id result should be "{expected}"') +def step_check_handler_result(context: Context, expected: str) -> None: + result = context.ci_handler_result + assert result == expected, ( + f"Expected _find_running_container to return '{expected}', got: {result!r}" + ) diff --git a/features/steps/container_id_integration_steps.py b/features/steps/container_id_integration_steps.py new file mode 100644 index 000000000..c10f58637 --- /dev/null +++ b/features/steps/container_id_integration_steps.py @@ -0,0 +1,285 @@ +"""Step definitions for container_id_integration.feature.""" + +from __future__ import annotations + +import contextlib +from io import StringIO +from typing import Any +from unittest.mock import MagicMock, patch + +import click +from behave import given, then, when # type: ignore[import-untyped] +from behave.runner import Context # type: ignore[import-untyped] +from rich.console import Console +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker + +from cleveragents.application.services.resource_registry_service import ( + ResourceRegistryService, +) +from cleveragents.domain.models.core.resource import Resource +from cleveragents.infrastructure.database.models import Base +from cleveragents.resource.handlers.devcontainer import DevcontainerHandler + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_cid_service(context: Context) -> ResourceRegistryService: + """Create or return a cached ResourceRegistryService for container_id tests.""" + if not hasattr(context, "cid_service"): + engine = create_engine("sqlite:///:memory:", echo=False) + Base.metadata.create_all(engine) + factory = sessionmaker(bind=engine, expire_on_commit=False) + context.cid_service = ResourceRegistryService(session_factory=factory) + return context.cid_service + + +def _patch_cid_service(context: Context) -> Any: + """Monkey-patch the CLI to return our in-memory service.""" + import cleveragents.cli.commands.resource as resource_mod + + orig_fn = resource_mod._get_registry_service + + def _mock_get() -> ResourceRegistryService: + return _make_cid_service(context) + + resource_mod._get_registry_service = _mock_get # type: ignore[assignment] + return orig_fn + + +def _unpatch_cid_service(orig_fn: Any) -> None: + """Restore original service getter.""" + import cleveragents.cli.commands.resource as resource_mod + + resource_mod._get_registry_service = orig_fn # type: ignore[assignment] + + +def _capture_cid_output(func: Any, *args: Any, **kwargs: Any) -> tuple[str, bool]: + """Run a CLI function capturing its console output and success status.""" + buf = StringIO() + console = Console( + file=buf, width=200, no_color=True, highlight=False, force_terminal=False + ) + + import cleveragents.cli.commands.resource as resource_mod + + orig_console = resource_mod.console + resource_mod.console = console # type: ignore[assignment] + + failed = False + try: + with contextlib.redirect_stdout(buf): + func(*args, **kwargs) + except (SystemExit, click.exceptions.Abort): + failed = True + except Exception: + failed = True + finally: + resource_mod.console = orig_console # type: ignore[assignment] + + return buf.getvalue(), failed + + +# --------------------------------------------------------------------------- +# Given steps +# --------------------------------------------------------------------------- + + +@given("a fresh in-memory resource registry for container_id") +def step_cid_fresh_registry(context: Context) -> None: + """Set up a fresh in-memory database for container_id tests.""" + engine = create_engine("sqlite:///:memory:", echo=False) + Base.metadata.create_all(engine) + factory = sessionmaker(bind=engine, expire_on_commit=False) + context.cid_service = ResourceRegistryService(session_factory=factory) + context.cid_output = "" + context.cid_failed = False + + +@given("built-in types are bootstrapped for container_id") +def step_cid_bootstrap_builtins(context: Context) -> None: + """Bootstrap built-in resource types for container_id tests.""" + service = _make_cid_service(context) + service.bootstrap_builtin_types() + + +@given('a container-instance resource with stored container_id "{cid}"') +def step_cid_resource_with_stored_id(context: Context, cid: str) -> None: + """Create a Resource domain object with container_id in properties.""" + resource = MagicMock(spec=Resource) + resource.properties = {"container_id": cid} + resource.location = None + context.cid_mock_resource = resource + context.cid_docker_called = False + + +# --------------------------------------------------------------------------- +# When steps +# --------------------------------------------------------------------------- + + +@when('I run resource add container-instance with container-id "{cid}"') +def step_cid_run_add_with_container_id(context: Context, cid: str) -> None: + """Run resource_add for container-instance with --container-id.""" + from cleveragents.cli.commands.resource import resource_add + + orig = _patch_cid_service(context) + try: + output, failed = _capture_cid_output( + resource_add, + type_name="container-instance", + name="local/test-ctr", + path=None, + branch=None, + description=None, + image=None, + container_id=cid, + mount=None, + clone_into=None, + update=False, + read_only=False, + fmt="rich", + ) + context.cid_output = output + context.cid_failed = failed + finally: + _unpatch_cid_service(orig) + + +@when("I run resource add container-instance with both container-id and image") +def step_cid_run_add_both_flags(context: Context) -> None: + """Run resource_add with both --container-id and --image (should fail).""" + from cleveragents.cli.commands.resource import resource_add + + orig = _patch_cid_service(context) + try: + output, failed = _capture_cid_output( + resource_add, + type_name="container-instance", + name="local/test-ctr", + path=None, + branch=None, + description=None, + image="ubuntu:latest", + container_id="abc123def456", + mount=None, + clone_into=None, + update=False, + read_only=False, + fmt="rich", + ) + context.cid_output = output + context.cid_failed = failed + finally: + _unpatch_cid_service(orig) + + +@when("I run resource add container-instance with no image or container-id") +def step_cid_run_add_no_flags(context: Context) -> None: + """Run resource_add for container-instance with neither flag (should fail).""" + from cleveragents.cli.commands.resource import resource_add + + orig = _patch_cid_service(context) + try: + output, failed = _capture_cid_output( + resource_add, + type_name="container-instance", + name="local/test-ctr", + path=None, + branch=None, + description=None, + image=None, + container_id=None, + mount=None, + clone_into=None, + update=False, + read_only=False, + fmt="rich", + ) + context.cid_output = output + context.cid_failed = failed + finally: + _unpatch_cid_service(orig) + + +@when('I show the container_id resource "{name}"') +def step_cid_show_resource(context: Context, name: str) -> None: + """Run resource_show for the given resource name.""" + from cleveragents.cli.commands.resource import resource_show + + orig = _patch_cid_service(context) + try: + output, failed = _capture_cid_output( + resource_show, + resource=name, + fmt="rich", + ) + context.cid_show_output = output + context.cid_show_failed = failed + finally: + _unpatch_cid_service(orig) + + +@when("I call _find_running_container on the resource") +def step_cid_call_find_running_container(context: Context) -> None: + """Call DevcontainerHandler._find_running_container with a mock subprocess.""" + with patch("subprocess.run") as mock_run: + context.cid_result = DevcontainerHandler._find_running_container( + context.cid_mock_resource + ) + context.cid_docker_called = mock_run.called + + +# --------------------------------------------------------------------------- +# Then steps +# --------------------------------------------------------------------------- + + +@then("the container_id command should succeed") +def step_cid_command_succeeded(context: Context) -> None: + """Assert the last container_id CLI command succeeded.""" + assert not context.cid_failed, ( + f"Expected command to succeed but it failed. Output:\n{context.cid_output}" + ) + + +@then("the container_id command should fail") +def step_cid_command_failed(context: Context) -> None: + """Assert the last container_id CLI command failed.""" + assert context.cid_failed, ( + f"Expected command to fail but it succeeded. Output:\n{context.cid_output}" + ) + + +@then('the container_id output should contain "{text}"') +def step_cid_output_contains(context: Context, text: str) -> None: + """Assert the container_id CLI output contains the expected text.""" + output = context.cid_output + assert text.lower() in output.lower(), ( + f"Expected '{text}' in output, got:\n{output}" + ) + + +@then('the container_id show output should contain "{text}"') +def step_cid_show_output_contains(context: Context, text: str) -> None: + """Assert the container_id show output contains the expected text.""" + output = context.cid_show_output + assert text in output, f"Expected '{text}' in show output, got:\n{output}" + + +@then('the container_id find result should be "{expected}"') +def step_cid_result_equals(context: Context, expected: str) -> None: + """Assert _find_running_container returned the expected container ID.""" + assert context.cid_result == expected, ( + f"Expected result '{expected}', got '{context.cid_result}'" + ) + + +@then("no docker subprocess should have been called") +def step_cid_no_docker_called(context: Context) -> None: + """Assert that subprocess.run was NOT called (docker was not invoked).""" + assert not context.cid_docker_called, ( + "Expected no docker subprocess call, but subprocess.run was invoked" + ) diff --git a/src/cleveragents/application/services/_resource_registry_data.py b/src/cleveragents/application/services/_resource_registry_data.py index e5ca010ec..2557beb8d 100644 --- a/src/cleveragents/application/services/_resource_registry_data.py +++ b/src/cleveragents/application/services/_resource_registry_data.py @@ -164,7 +164,19 @@ _GIT_FS_CONTAINER_TYPES: list[dict[str, Any]] = [ "name": "image", "type": "string", "required": False, - "description": "Container image reference.", + "description": ( + "Container image reference. Required when " + "--container-id is not provided." + ), + "default": None, + }, + { + "name": "container_id", + "type": "string", + "required": False, + "description": ( + "ID of an existing container to attach (alternative to --image)." + ), "default": None, }, { diff --git a/src/cleveragents/cli/commands/resource.py b/src/cleveragents/cli/commands/resource.py index 420d5f864..7b3eb0988 100644 --- a/src/cleveragents/cli/commands/resource.py +++ b/src/cleveragents/cli/commands/resource.py @@ -562,7 +562,21 @@ def resource_add( image: Annotated[ str | None, typer.Option( - "--image", help="Container image for container-instance resources" + "--image", + help=( + "Container image for container-instance resources. " + "Required when --container-id is not provided." + ), + ), + ] = None, + container_id: Annotated[ + str | None, + typer.Option( + "--container-id", + help=( + "ID of an existing container to attach " + "(alternative to --image for container-instance resources)." + ), ), ] = None, mount: Annotated[ @@ -613,6 +627,7 @@ def resource_add( agents resource add fs-directory local/data --path /data --read-only agents resource add devcontainer-instance local/dc --path /project agents resource add container-instance local/ctr --image ubuntu:latest + agents resource add container-instance local/ctr --container-id abc123def456 agents resource add container-instance local/ctr --image node:18 \\ --mount local/api-repo:/workspace \\ --mount /var/shared/config:/config:ro @@ -643,6 +658,26 @@ def resource_add( ) raise typer.Abort() + # Validate --container-id usage rules. + if container_id is not None and image is not None: + console.print( + "[red]Validation error:[/red] --container-id and --image are " + "mutually exclusive; provide one or the other, not both." + ) + raise typer.Abort() + if container_id is not None and type_name != "container-instance": + console.print( + f"[red]Validation error:[/red] --container-id is only valid " + f"for container-instance resources, not '{type_name}'." + ) + raise typer.Abort() + if type_name == "container-instance" and container_id is None and image is None: + console.print( + "[red]Validation error:[/red] container-instance resources " + "require either --image or --container-id." + ) + raise typer.Abort() + service = _get_registry_service() # --update: remove existing resource before re-registering @@ -683,6 +718,8 @@ def resource_add( properties["branch"] = branch if image is not None: properties["image"] = image + if container_id is not None: + properties["container_id"] = container_id if mount: if type_name not in _CONTAINER_TYPES: console.print( diff --git a/src/cleveragents/resource/handlers/devcontainer.py b/src/cleveragents/resource/handlers/devcontainer.py index cec5a1704..1be783f92 100644 --- a/src/cleveragents/resource/handlers/devcontainer.py +++ b/src/cleveragents/resource/handlers/devcontainer.py @@ -237,9 +237,19 @@ class DevcontainerHandler(BaseResourceHandler): def _find_running_container(resource: Resource) -> str | None: """Find a running Docker container for this devcontainer resource. - Uses ``docker ps`` filtered by the devcontainer label that - matches the resource location. + Fast path: if the resource has a stored ``container_id`` property + (set via ``--container-id`` when registering a ``container-instance``), + return it directly without querying Docker. + + Fallback: uses ``docker ps`` filtered by the devcontainer label that + matches the resource location (for ``devcontainer-instance`` resources). """ + # Fast path: use the stored container ID when available. + if resource.properties: + stored_id = resource.properties.get("container_id") + if stored_id and isinstance(stored_id, str): + return stored_id + if not resource.location: return None try: