feat(cli): add --container-id flag to agents resource add for container-instance
CI / typecheck (pull_request) Successful in 44s
CI / security (pull_request) Successful in 53s
CI / quality (pull_request) Successful in 32s
CI / lint (pull_request) Successful in 3m28s
CI / helm (pull_request) Successful in 23s
CI / build (pull_request) Successful in 54s
CI / unit_tests (pull_request) Failing after 6m21s
CI / docker (pull_request) Has been skipped
CI / e2e_tests (pull_request) Failing after 16m7s
CI / integration_tests (pull_request) Successful in 22m39s
CI / coverage (pull_request) Successful in 10m36s
CI / status-check (pull_request) Failing after 1s
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Successful in 56m9s

Implemented an explicit --container-id option as an alternative to --image
for container-instance resources and added comprehensive validation, data
storage, and fast-path container resolution.

- Added --container-id option to resource_add() in
  src/cleveragents/cli/commands/resource.py as an alternative to --image
  for container-instance resources
- Enforced mutual exclusion: --container-id and --image are mutually
  exclusive; container-instance requires one or the other
- Restricted --container-id to container-instance only (not
  devcontainer-instance)
- Persisted container_id in resource properties when provided
- Updated DevcontainerHandler._find_running_container() in
  src/cleveragents/resource/handlers/devcontainer.py to use the stored
  container_id as a fast path before falling back to docker label search
- Added container_id to container-instance cli_args in
  src/cleveragents/application/services/_resource_registry_data.py
- Created features/container_id_flag.feature and
  features/steps/container_id_flag_steps.py with 5 unit test scenarios
- Created features/container_id_integration.feature and
  features/steps/container_id_integration_steps.py with 5 integration
  test scenarios

Key design decisions:
- Used isinstance(stored_id, str) guard in handler for Pyright type narrowing
- Validation raises typer.Abort() for clean CLI error handling
- No behavioral changes for devcontainer-instance resources

ISSUES CLOSED: #2598
This commit is contained in:
2026-04-05 04:22:48 +00:00
parent bbff42ac9a
commit 565e7918a3
7 changed files with 552 additions and 4 deletions
+27
View File
@@ -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"
+36
View File
@@ -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
+141
View File
@@ -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}"
)
@@ -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"
)
@@ -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,
},
{
+38 -1
View File
@@ -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(
@@ -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: