fix(resources): allow agents resource stop to stop container-instance and devcontainer-instance resources
CI / lint (pull_request) Successful in 20s
CI / typecheck (pull_request) Successful in 1m17s
CI / quality (pull_request) Successful in 40s
CI / security (pull_request) Successful in 1m4s
CI / build (pull_request) Successful in 31s
CI / helm (pull_request) Successful in 23s
CI / unit_tests (pull_request) Successful in 6m52s
CI / e2e_tests (pull_request) Successful in 17m9s
CI / integration_tests (pull_request) Successful in 23m1s
CI / coverage (pull_request) Successful in 10m39s
CI / docker (pull_request) Successful in 1m21s
CI / status-check (pull_request) Successful in 1s
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Successful in 58m45s
CI / lint (pull_request) Successful in 20s
CI / typecheck (pull_request) Successful in 1m17s
CI / quality (pull_request) Successful in 40s
CI / security (pull_request) Successful in 1m4s
CI / build (pull_request) Successful in 31s
CI / helm (pull_request) Successful in 23s
CI / unit_tests (pull_request) Successful in 6m52s
CI / e2e_tests (pull_request) Successful in 17m9s
CI / integration_tests (pull_request) Successful in 23m1s
CI / coverage (pull_request) Successful in 10m39s
CI / docker (pull_request) Successful in 1m21s
CI / status-check (pull_request) Successful in 1s
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Successful in 58m45s
Add container-instance to _STOPPABLE_TYPES so that agents resource stop accepts both container-instance and devcontainer-instance resources, as required by the specification. The stop_container function already uses docker stop with the container_id from the lifecycle tracker, which works generically for both resource types. - Add container-instance to _STOPPABLE_TYPES frozenset - Update resource_stop() docstring to reflect both stoppable types - Update devcontainer_cleanup.feature: F19 scenario now expects success for container-instance stop (was incorrectly expecting rejection) - Add TDD feature file and step definitions for issue #2588 regression guard ISSUES CLOSED: #2588
This commit is contained in:
@@ -135,14 +135,14 @@ Feature: Devcontainer Session Cleanup and CLI Commands
|
||||
Then the devcontainer CLI exit code should be 0
|
||||
And the CLI rebuild mock should have been called once
|
||||
|
||||
# ── F5/F19: container-instance is NOT stoppable via CLI ─────
|
||||
# ── Issue #2588: container-instance IS stoppable via CLI ────
|
||||
|
||||
Scenario: CLI stop rejects container-instance type (F19 fix)
|
||||
Scenario: CLI stop accepts container-instance type (issue #2588 fix)
|
||||
Given a mock resource service with container-instance "local/ctr-stop" id "01TESTLIFECYCLE0000000370" at "/workspace/project"
|
||||
And an active container "01TESTLIFECYCLE0000000370" with container ID "ctr-ci-stop"
|
||||
When I invoke CLI resource stop "local/ctr-stop"
|
||||
Then the devcontainer CLI exit code should be non-zero
|
||||
And the CLI output should contain "not a stoppable"
|
||||
Then the CLI exit code should be 0
|
||||
And the CLI output should contain "Stopped"
|
||||
|
||||
# ── F6-r6: CLI handler-level error paths ────────────────────
|
||||
|
||||
|
||||
@@ -0,0 +1,165 @@
|
||||
"""Step definitions for TDD issue #2588.
|
||||
|
||||
Verifies that ``agents resource stop`` accepts both ``container-instance``
|
||||
and ``devcontainer-instance`` resource types, and still rejects other types.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from behave import given, then, when
|
||||
from behave.runner import Context
|
||||
from typer.testing import CliRunner
|
||||
|
||||
from cleveragents.cli.commands.resource import _STOPPABLE_TYPES
|
||||
from cleveragents.cli.commands.resource import app as resource_app
|
||||
from cleveragents.domain.models.core.container_lifecycle import (
|
||||
ContainerLifecycleState,
|
||||
ContainerLifecycleTracker,
|
||||
)
|
||||
from cleveragents.resource.handlers.devcontainer import set_lifecycle_tracker
|
||||
|
||||
_CLI_PATCH_SERVICE = "cleveragents.cli.commands.resource._get_registry_service"
|
||||
_CLI_PATCH_STOP = "cleveragents.cli.commands.resource.stop_container"
|
||||
|
||||
|
||||
def _make_mock_resource(
|
||||
*,
|
||||
resource_id: str,
|
||||
name: str,
|
||||
resource_type_name: str,
|
||||
location: str | None = None,
|
||||
) -> MagicMock:
|
||||
res = MagicMock()
|
||||
res.resource_id = resource_id
|
||||
res.name = name
|
||||
res.resource_type_name = resource_type_name
|
||||
res.location = location
|
||||
res.properties = None
|
||||
return res
|
||||
|
||||
|
||||
@given('a running container-instance resource "{name}" with id "{resource_id}"')
|
||||
def step_running_container_instance(
|
||||
context: Context, name: str, resource_id: str
|
||||
) -> None:
|
||||
"""Set up a running container-instance resource for stop tests."""
|
||||
mock_service = MagicMock()
|
||||
mock_resource = _make_mock_resource(
|
||||
resource_id=resource_id,
|
||||
name=name,
|
||||
resource_type_name="container-instance",
|
||||
location="/workspace/project",
|
||||
)
|
||||
mock_service.show_resource.return_value = mock_resource
|
||||
context.tdd2588_mock_service = mock_service
|
||||
context.tdd2588_resource_id = resource_id
|
||||
# Set up lifecycle tracker in running state
|
||||
tracker = ContainerLifecycleTracker(
|
||||
resource_id=resource_id,
|
||||
current_state=ContainerLifecycleState.RUNNING,
|
||||
container_id="ctr-2588-ci",
|
||||
workspace_path="/workspace/project",
|
||||
)
|
||||
set_lifecycle_tracker(tracker)
|
||||
|
||||
|
||||
@given('a running devcontainer-instance resource "{name}" with id "{resource_id}"')
|
||||
def step_running_devcontainer_instance(
|
||||
context: Context, name: str, resource_id: str
|
||||
) -> None:
|
||||
"""Set up a running devcontainer-instance resource for stop tests."""
|
||||
mock_service = MagicMock()
|
||||
mock_resource = _make_mock_resource(
|
||||
resource_id=resource_id,
|
||||
name=name,
|
||||
resource_type_name="devcontainer-instance",
|
||||
location="/workspace/project",
|
||||
)
|
||||
mock_service.show_resource.return_value = mock_resource
|
||||
context.tdd2588_mock_service = mock_service
|
||||
context.tdd2588_resource_id = resource_id
|
||||
# Set up lifecycle tracker in running state
|
||||
tracker = ContainerLifecycleTracker(
|
||||
resource_id=resource_id,
|
||||
current_state=ContainerLifecycleState.RUNNING,
|
||||
container_id="ctr-2588-dc",
|
||||
workspace_path="/workspace/project",
|
||||
)
|
||||
set_lifecycle_tracker(tracker)
|
||||
|
||||
|
||||
@given('a git-checkout resource "{name}" with id "{resource_id}"')
|
||||
def step_git_checkout_resource(context: Context, name: str, resource_id: str) -> None:
|
||||
"""Set up a git-checkout resource (non-stoppable) for stop tests."""
|
||||
mock_service = MagicMock()
|
||||
mock_resource = _make_mock_resource(
|
||||
resource_id=resource_id,
|
||||
name=name,
|
||||
resource_type_name="git-checkout",
|
||||
)
|
||||
mock_service.show_resource.return_value = mock_resource
|
||||
context.tdd2588_mock_service = mock_service
|
||||
context.tdd2588_resource_id = resource_id
|
||||
|
||||
|
||||
@when('I invoke resource stop for "{name}" with --yes')
|
||||
def step_invoke_resource_stop(context: Context, name: str) -> None:
|
||||
"""Invoke the resource stop CLI command with --yes flag."""
|
||||
runner = CliRunner()
|
||||
mock_service = context.tdd2588_mock_service
|
||||
|
||||
with (
|
||||
patch(_CLI_PATCH_SERVICE, return_value=mock_service),
|
||||
patch(_CLI_PATCH_STOP) as mock_stop,
|
||||
):
|
||||
mock_stop.return_value = MagicMock()
|
||||
result = runner.invoke(resource_app, ["stop", name, "--yes"])
|
||||
|
||||
context.tdd2588_result = result
|
||||
context.tdd2588_mock_stop = mock_stop
|
||||
|
||||
|
||||
@when("I inspect the _STOPPABLE_TYPES constant")
|
||||
def step_inspect_stoppable_types(context: Context) -> None:
|
||||
"""Capture the _STOPPABLE_TYPES constant for inspection."""
|
||||
context.tdd2588_stoppable_types = _STOPPABLE_TYPES
|
||||
|
||||
|
||||
@then("the stop exit code should be 0")
|
||||
def step_stop_exit_code_zero(context: Context) -> None:
|
||||
result = context.tdd2588_result
|
||||
assert result.exit_code == 0, (
|
||||
f"Expected exit code 0, got {result.exit_code}. Output:\n{result.output}"
|
||||
)
|
||||
|
||||
|
||||
@then("the stop exit code should be non-zero")
|
||||
def step_stop_exit_code_nonzero(context: Context) -> None:
|
||||
result = context.tdd2588_result
|
||||
assert result.exit_code != 0, (
|
||||
f"Expected non-zero exit code, got {result.exit_code}. Output:\n{result.output}"
|
||||
)
|
||||
|
||||
|
||||
@then('the stop output should contain "{text}"')
|
||||
def step_stop_output_contains(context: Context, text: str) -> None:
|
||||
result = context.tdd2588_result
|
||||
assert text in result.output, f"Expected '{text}' in output, got:\n{result.output}"
|
||||
|
||||
|
||||
@then('_STOPPABLE_TYPES should contain "{resource_type}"')
|
||||
def step_stoppable_types_contains(context: Context, resource_type: str) -> None:
|
||||
stoppable = context.tdd2588_stoppable_types
|
||||
assert resource_type in stoppable, (
|
||||
f"Expected '{resource_type}' in _STOPPABLE_TYPES, got: {stoppable}"
|
||||
)
|
||||
|
||||
|
||||
@then('_STOPPABLE_TYPES should not contain "{resource_type}"')
|
||||
def step_stoppable_types_not_contains(context: Context, resource_type: str) -> None:
|
||||
stoppable = context.tdd2588_stoppable_types
|
||||
assert resource_type not in stoppable, (
|
||||
f"Expected '{resource_type}' NOT in _STOPPABLE_TYPES, got: {stoppable}"
|
||||
)
|
||||
@@ -0,0 +1,35 @@
|
||||
@tdd_issue @tdd_issue_2588
|
||||
Feature: TDD Issue #2588 — agents resource stop rejects container-instance resources
|
||||
As a CleverAgents user
|
||||
I want to stop container-instance resources via agents resource stop
|
||||
So that both container-instance and devcontainer-instance resources
|
||||
can be stopped as required by the specification
|
||||
|
||||
The specification states:
|
||||
"Stop an active devcontainer-instance or container-instance resource.
|
||||
Transitions the container through running -> stopping -> stopped.
|
||||
Only container-typed resources may be stopped."
|
||||
|
||||
Scenario: container-instance is accepted by resource stop
|
||||
Given a running container-instance resource "local/my-ctr" with id "01TDD2588CONTAINERINST001"
|
||||
When I invoke resource stop for "local/my-ctr" with --yes
|
||||
Then the stop exit code should be 0
|
||||
And the stop output should contain "Stopped"
|
||||
|
||||
Scenario: devcontainer-instance is still accepted by resource stop
|
||||
Given a running devcontainer-instance resource "local/my-dc" with id "01TDD2588DEVCONTAINERINST1"
|
||||
When I invoke resource stop for "local/my-dc" with --yes
|
||||
Then the stop exit code should be 0
|
||||
And the stop output should contain "Stopped"
|
||||
|
||||
Scenario: non-container resource type is still rejected by resource stop
|
||||
Given a git-checkout resource "local/my-repo" with id "01TDD2588GITCHECKOUT00001"
|
||||
When I invoke resource stop for "local/my-repo" with --yes
|
||||
Then the stop exit code should be non-zero
|
||||
And the stop output should contain "not a stoppable container type"
|
||||
|
||||
Scenario: _STOPPABLE_TYPES contains both container types
|
||||
When I inspect the _STOPPABLE_TYPES constant
|
||||
Then _STOPPABLE_TYPES should contain "container-instance"
|
||||
And _STOPPABLE_TYPES should contain "devcontainer-instance"
|
||||
And _STOPPABLE_TYPES should not contain "git-checkout"
|
||||
@@ -1349,9 +1349,10 @@ def resource_remove(
|
||||
|
||||
# B2 fix: separate type sets — stop works on any container, but rebuild
|
||||
# requires devcontainer-instance since it invokes ``devcontainer up``.
|
||||
# F19 fix: restricted to devcontainer-instance only — container-instance
|
||||
# has no lifecycle tracker wiring, so stop/rebuild would fail at runtime.
|
||||
_STOPPABLE_TYPES = frozenset({"devcontainer-instance"})
|
||||
# Issue #2588 fix: container-instance is now stoppable — stop_container uses
|
||||
# docker stop with the container_id from the lifecycle tracker, which works
|
||||
# for both container-instance and devcontainer-instance resources.
|
||||
_STOPPABLE_TYPES = frozenset({"devcontainer-instance", "container-instance"})
|
||||
_REBUILDABLE_TYPES = frozenset({"devcontainer-instance"})
|
||||
# F5-r6 fix: moved from inside resource_rebuild() to module level to avoid
|
||||
# per-call allocation (consistent with _STOPPABLE_TYPES / _REBUILDABLE_TYPES).
|
||||
@@ -1372,11 +1373,11 @@ def resource_stop(
|
||||
typer.Option("--yes", "-y", help="Skip confirmation prompt"),
|
||||
] = False,
|
||||
) -> None:
|
||||
"""Stop a running devcontainer-instance resource.
|
||||
"""Stop a running container-instance or devcontainer-instance resource.
|
||||
|
||||
Transitions the container from ``running`` -> ``stopping`` -> ``stopped``.
|
||||
Only devcontainer-instance and container-instance resources
|
||||
may be stopped.
|
||||
Both container-instance and devcontainer-instance resources
|
||||
may be stopped; other resource types produce an error.
|
||||
|
||||
Use ``--yes`` / ``-y`` to skip the confirmation prompt in scripts.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user