Merge pull request 'fix(cli): implement --mount flag on resource add container-instance' (#1134) from bugfix/m8-container-mount-flag into master

Reviewed-on: #1134
Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>

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

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"

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"

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: "
+                    "<resource>:<path> or <host-path>:<path>:<mode>. "
+                    "Use --mount flag (repeatable) on the CLI."
+                ),
+                "default": None,
+            },
         ],
         "parent_types": ["git-checkout", "fs-directory"],
         "child_types": [],

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-name>:<container-path>`` — resource-reference mount (rw)
+        - ``<host-path>:<container-path>:<mode>`` — 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 "
+        "<source>:<target> or <source>:<target>:<mode>"
+    )
+
+
 @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>:<path> (resource-ref, rw) or "
+                "<host-path>:<container-path>:<mode> (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"