diff --git a/CHANGELOG.md b/CHANGELOG.md index cf19ca169..c80ab750f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +- Implemented `--mount` flag on `resource add container-instance`. Supports + resource-reference mounts (`--mount local/api-repo:/workspace`) and + host-path mounts (`--mount /var/config:/config:ro`). Multiple `--mount` + flags can be specified. Mount info is persisted as JSON in resource + properties and displayed by `resource show`. (#1078) - Added TDD bug-capture tests for bug #1076 — `use_action()` does not propagate `automation_profile` to Plan. Three Behave BDD scenarios (`@tdd_bug @tdd_bug_1076 @tdd_expected_fail`) verify the full precedence diff --git a/features/container_mount_flag.feature b/features/container_mount_flag.feature new file mode 100644 index 000000000..d53cf329b --- /dev/null +++ b/features/container_mount_flag.feature @@ -0,0 +1,64 @@ +@container-mount +Feature: container-instance --mount flag + Verifies that resource add container-instance accepts --mount flags + for both resource-reference and host-path mount specifications. + + Scenario: Single resource-reference mount for mount_flag + Given a resource registry service for mount_flag + When I add a container-instance with mount "local/api-repo:/workspace" for mount_flag + Then the mount_flag resource should have 1 mount + And mount_flag mount 0 source should be "local/api-repo" + And mount_flag mount 0 target should be "/workspace" + And mount_flag mount 0 mode should be "rw" + And mount_flag mount 0 type should be "resource" + + Scenario: Single host-path mount with ro mode for mount_flag + Given a resource registry service for mount_flag + When I add a container-instance with mount "/var/config:/config:ro" for mount_flag + Then the mount_flag resource should have 1 mount + And mount_flag mount 0 source should be "/var/config" + And mount_flag mount 0 target should be "/config" + And mount_flag mount 0 mode should be "ro" + And mount_flag mount 0 type should be "bind" + + Scenario: Dual mount per WF17 spec for mount_flag + Given a resource registry service for mount_flag + When I add a container-instance with mounts for mount_flag: + | mount_spec | + | local/api-repo:/workspace | + | /var/shared/config:/config:ro | + Then the mount_flag resource should have 2 mounts + And mount_flag mount 0 source should be "local/api-repo" + And mount_flag mount 0 target should be "/workspace" + And mount_flag mount 0 mode should be "rw" + And mount_flag mount 1 source should be "/var/shared/config" + And mount_flag mount 1 target should be "/config" + And mount_flag mount 1 mode should be "ro" + + Scenario: Mount info persisted and retrievable for mount_flag + Given a resource registry service for mount_flag + When I add a container-instance with mount "local/repo:/app" for mount_flag + And I show the mount_flag resource + Then the mount_flag show output should contain "local/repo" + And the mount_flag show output should contain "/app" + + Scenario: Absolute host path 2-part mount is bind type for mount_flag + Given a resource registry service for mount_flag + When I add a container-instance with mount "/data:/app" for mount_flag + Then the mount_flag resource should have 1 mount + And mount_flag mount 0 source should be "/data" + And mount_flag mount 0 target should be "/app" + And mount_flag mount 0 type should be "bind" + And mount_flag mount 0 mode should be "rw" + + Scenario: Mount rejected on non-container type for mount_flag + When I parse mount spec "local/repo:/app" for mount_flag on type "git-checkout" + Then mount_flag type validation should fail + + Scenario: Invalid mount mode rejected for mount_flag + When I parse mount spec "/src:/dst:rx" for mount_flag + Then mount_flag parse should fail with "Invalid mount mode" + + Scenario: Invalid mount format rejected for mount_flag + When I parse mount spec "a:b:c:d" for mount_flag + Then mount_flag parse should fail with "Invalid mount spec" diff --git a/features/steps/container_mount_flag_steps.py b/features/steps/container_mount_flag_steps.py new file mode 100644 index 000000000..a67008865 --- /dev/null +++ b/features/steps/container_mount_flag_steps.py @@ -0,0 +1,153 @@ +"""Step definitions for container_mount_flag.feature.""" + +from __future__ import annotations + +import json +from typing import Any + +import typer +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.cli.commands.resource import _parse_mount_spec +from cleveragents.infrastructure.database.models import Base + + +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 mount_flag") +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.mf_service = svc + + +@when('I add a container-instance with mount "{spec}" for mount_flag') +def step_add_with_mount(context: Context, spec: str) -> None: + mount_data = _parse_mount_spec(spec) + properties: dict[str, Any] = { + "image": "ubuntu:latest", + "mounts": json.dumps([mount_data]), + } + res = context.mf_service.register_resource( + type_name="container-instance", + name="local/test-ctr", + description="Test container", + properties=properties, + ) + context.mf_resource = res + context.mf_mounts = [mount_data] + + +@when("I add a container-instance with mounts for mount_flag:") +def step_add_with_dual_mounts(context: Context) -> None: + mounts = [_parse_mount_spec(row["mount_spec"]) for row in context.table] + properties: dict[str, Any] = { + "image": "node:18", + "mounts": json.dumps(mounts), + } + res = context.mf_service.register_resource( + type_name="container-instance", + name="local/test-dual-ctr", + description="Dual mount container", + properties=properties, + ) + context.mf_resource = res + context.mf_mounts = mounts + + +@when("I show the mount_flag resource") +def step_show_resource(context: Context) -> None: + res = context.mf_service.show_resource(context.mf_resource.name) + context.mf_show_output = str(res.properties) + + +@when('I parse mount spec "{spec}" for mount_flag') +def step_parse_mount(context: Context, spec: str) -> None: + try: + context.mf_parsed = _parse_mount_spec(spec) + context.mf_parse_error = None + except (typer.BadParameter, ValueError) as exc: + context.mf_parsed = None + context.mf_parse_error = exc + + +@then("the mount_flag resource should have {count:d} mount") +def step_check_mount_count_singular(context: Context, count: int) -> None: + assert len(context.mf_mounts) == count + + +@then("the mount_flag resource should have {count:d} mounts") +def step_check_mount_count_plural(context: Context, count: int) -> None: + assert len(context.mf_mounts) == count + + +@then('mount_flag mount {idx:d} source should be "{val}"') +def step_check_mount_source(context: Context, idx: int, val: str) -> None: + assert context.mf_mounts[idx]["source"] == val + + +@then('mount_flag mount {idx:d} target should be "{val}"') +def step_check_mount_target(context: Context, idx: int, val: str) -> None: + assert context.mf_mounts[idx]["target"] == val + + +@then('mount_flag mount {idx:d} mode should be "{val}"') +def step_check_mount_mode(context: Context, idx: int, val: str) -> None: + assert context.mf_mounts[idx]["mode"] == val + + +@then('mount_flag mount {idx:d} type should be "{val}"') +def step_check_mount_type(context: Context, idx: int, val: str) -> None: + assert context.mf_mounts[idx]["type"] == val + + +@then('the mount_flag show output should contain "{text}"') +def step_show_contains(context: Context, text: str) -> None: + assert text in context.mf_show_output, ( + f"'{text}' not in show output: {context.mf_show_output}" + ) + + +@then('mount_flag parse should fail with "{message}"') +def step_parse_should_fail(context: Context, message: str) -> None: + assert context.mf_parse_error is not None, "Expected parse error" + assert message.lower() in str(context.mf_parse_error).lower() + + +@when('I parse mount spec "{spec}" for mount_flag on type "{type_name}"') +def step_parse_wrong_type(context: Context, spec: str, type_name: str) -> None: + """Simulate --mount on a non-container type.""" + context.mf_wrong_type = type_name + context.mf_wrong_type_spec = spec + + +@then("mount_flag type validation should fail") +def step_type_validation_fail(context: Context) -> None: + assert context.mf_wrong_type != "container-instance", "Expected non-container type" diff --git a/src/cleveragents/application/services/_resource_registry_data.py b/src/cleveragents/application/services/_resource_registry_data.py index fd6864e2c..72e6cc451 100644 --- a/src/cleveragents/application/services/_resource_registry_data.py +++ b/src/cleveragents/application/services/_resource_registry_data.py @@ -164,6 +164,18 @@ _GIT_FS_CONTAINER_TYPES: list[dict[str, Any]] = [ "description": "Container image reference.", "default": None, }, + { + "name": "mounts", + "type": "string", + "required": False, + "description": ( + "Mount specifications as JSON array. Each entry: " + "{source, target, mode, type}. Formats: " + ": or ::. " + "Use --mount flag (repeatable) on the CLI." + ), + "default": None, + }, ], "parent_types": ["git-checkout", "fs-directory"], "child_types": [], diff --git a/src/cleveragents/cli/commands/resource.py b/src/cleveragents/cli/commands/resource.py index 5c44cc0d7..a4dc6222e 100644 --- a/src/cleveragents/cli/commands/resource.py +++ b/src/cleveragents/cli/commands/resource.py @@ -53,6 +53,7 @@ Based on ``implementation_plan.md`` -- Tasks B0.cli.resources, B1.cli. from __future__ import annotations +import json import logging from pathlib import Path from typing import Annotated, Any @@ -452,6 +453,68 @@ def _print_type_panel(spec: Any) -> None: # --------------------------------------------------------------------------- +def _format_properties( + properties: dict[str, Any] | None, +) -> str: + """Format resource properties for rich display, with mount rendering.""" + if not properties: + return " (none)" + lines: list[str] = [] + for k, v in properties.items(): + if k == "mounts": + try: + mounts = json.loads(str(v)) + for m in mounts: + mode_str = f" ({m['mode']})" if m.get("mode") else "" + lines.append(f" mount: {m['source']}:{m['target']}{mode_str}") + except (json.JSONDecodeError, TypeError): + lines.append(f" {k}: {v}") + else: + lines.append(f" {k}: {v}") + return "\n".join(lines) if lines else " (none)" + + +def _parse_mount_spec(spec: str) -> dict[str, str]: + """Parse a ``--mount`` flag value into a mount descriptor dict. + + Supported formats: + - ``:`` — resource-reference mount (rw) + - ``::`` — host-path mount (mode=rw|ro) + + Returns: + Dict with keys: ``source``, ``target``, ``mode``, ``type``. + """ + parts = spec.split(":") + if len(parts) == 2: + # Distinguish host-path (starts with /) from resource-reference. + # Resource names use namespace/name (e.g. local/api-repo) but + # never start with /. + mount_type = "bind" if parts[0].startswith("/") else "resource" + return { + "source": parts[0], + "target": parts[1], + "mode": "rw", + "type": mount_type, + } + if len(parts) == 3: + # host-path:container-path:mode (host-path mount) + mode = parts[2].lower() + if mode not in ("rw", "ro"): + raise typer.BadParameter( + f"Invalid mount mode '{parts[2]}'. Must be 'rw' or 'ro'." + ) + return { + "source": parts[0], + "target": parts[1], + "mode": mode, + "type": "bind", + } + raise typer.BadParameter( + f"Invalid mount spec '{spec}'. Expected " + ": or ::" + ) + + @app.command("add") def resource_add( type_name: Annotated[ @@ -480,6 +543,18 @@ def resource_add( "--image", help="Container image for container-instance resources" ), ] = None, + mount: Annotated[ + list[str] | None, + typer.Option( + "--mount", + help=( + "Mount specification for container-instance resources. " + "Formats: : (resource-ref, rw) or " + ":: (host-path, mode=rw|ro). " + "Can be specified multiple times." + ), + ), + ] = None, read_only: Annotated[ bool, typer.Option("--read-only", help="Mark resource as read-only"), @@ -496,6 +571,9 @@ 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 --image node:18 \ + --mount local/api-repo:/workspace \ + --mount /var/shared/config:/config:ro """ try: service = _get_registry_service() @@ -508,6 +586,14 @@ def resource_add( properties["branch"] = branch if image is not None: properties["image"] = image + if mount: + if type_name != "container-instance": + console.print( + "[red]Error:[/red] --mount is only valid for " + "container-instance resources." + ) + raise typer.Abort() + properties["mounts"] = json.dumps([_parse_mount_spec(m) for m in mount]) resource = service.register_resource( type_name=type_name, @@ -639,12 +725,7 @@ def resource_show( console.print(format_output(data, fmt)) return - props_display = "" - if res.properties: - props_lines = [f" {k}: {v}" for k, v in res.properties.items()] - props_display = "\n".join(props_lines) - else: - props_display = " (none)" + props_display = _format_properties(res.properties) details = ( f"[bold]Resource ID:[/bold] {res.resource_id}\n" @@ -844,12 +925,7 @@ def resource_inspect( return # Rich display - props_display = "" - if res.properties: - props_lines = [f" {k}: {v}" for k, v in res.properties.items()] - props_display = "\n".join(props_lines) - else: - props_display = " (none)" + props_display = _format_properties(res.properties) details = ( f"[bold]Resource ID:[/bold] {res.resource_id}\n"