diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ca15e1f8..5e46a3e1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -517,6 +517,15 @@ _ALL_DATA_COLUMNS + ") " "SELECT " + _ALL_DATA_COLUMNS + " FROM v3_plans"`. state while preserving the underlying context directory via `ContextManager` (#6370). - **Quick Start Guide** (PR #9245): Added `docs/quickstart.md` with an end-to-end quick start guide covering prerequisites, installation, project creation, resource registration, plan/apply workflow, and troubleshooting. Updated `mkdocs.yml` navigation to include the Quick Start page. +- **container-instance --clone-into and devcontainer-instance sandbox strategy** (#7555): + Added `--clone-into` CLI argument to `container-instance` resource type for cloning + a git repository into a running container. Implemented `CloneIntoHandler` with + `clone_repo_into_container()` and `validate_clone_into_url()` helpers. Updated + `devcontainer-instance` to use `snapshot` sandbox strategy (was `none`) to enable + safe plan execution inside containers. Added `container-mount`, `container-exec-env`, + and `container-port` as child types of `devcontainer-instance`. Renamed + `ContainerLifecycleState.DETECTED` to `DISCOVERED` (value: `"discovered"`) to align + with specification terminology. - **Plan checkpoint management CLI commands** (#8683): Added `agents plan checkpoint-list ` and `agents plan checkpoint-delete ` commands. Listing output now highlights checkpoint ID, type, created timestamp, reason, phase, and decision linkage with a concise field summary footer across rich/table/json/yaml formats. Deletion supports batch IDs, interactive confirmation (skip with `--yes`), and structured JSON/YAML responses for automation-friendly scripting. - **Invariant Remove CLI Command** (#8530): Implemented `agents invariant remove ` command that soft-deletes an invariant by ID. The command displays a confirmation prompt before removal (bypassable with `--yes`/ `-y`), outputs the removed invariant ID on success, and shows a clear error message when the invariant ID does not exist. Supports `--format` flag for JSON and YAML output. Full BDD test coverage and Robot Framework integration tests included. diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index cb6362e8d..08cf0f1fb 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -70,3 +70,4 @@ Below are some of the specific details of various contributions. * HAL 9000 has contributed BDD feature file tag coverage improvements (#9124 / pr #9183): added required `@a2a`, `@session`, and `@cli` Gherkin tags to 30 feature files (8 A2A, 7 session, 15 CLI) to enable selective tag-based test filtering via `behave --tags=a2a,session,cli`. * HAL 9000 has contributed the plan tree JSON `decision_id` fix (#9096): updated `step_tree_json_valid` in features/steps/plan_explain_steps.py to correctly handle the {"data": [...]} envelope structure produced by format_output, and removed @tdd_expected_fail from the @tdd_issue_4254 scenario so it runs as a permanent regression guard. * HAL 9000 has contributed the TDD scenario for plan tree correction visual marking (PR #8671 / issue #8576): added a failing BDD scenario proving that corrected nodes (decisions with is_correction=True) are not visually distinguished in the plan tree output, formalizing Spec Requirement #7 as an executable specification. +* HAL 9000 has contributed the `--clone-into` CLI argument for `container-instance`, the `CloneIntoHandler` module, the `devcontainer-instance` snapshot sandbox strategy, and the `ContainerLifecycleState.DISCOVERED` terminology alignment (PR #8304, issue #7555). diff --git a/benchmarks/devcontainer_lifecycle_bench.py b/benchmarks/devcontainer_lifecycle_bench.py index 947196aec..257c1f0a6 100644 --- a/benchmarks/devcontainer_lifecycle_bench.py +++ b/benchmarks/devcontainer_lifecycle_bench.py @@ -80,7 +80,7 @@ class TimeTransitionValidation: """Time valid transition check (1000 iterations).""" for _ in range(1000): validate_transition( - ContainerLifecycleState.DETECTED, + ContainerLifecycleState.DISCOVERED, ContainerLifecycleState.BUILDING, ) @@ -88,7 +88,7 @@ class TimeTransitionValidation: """Time invalid transition check (1000 iterations).""" for _ in range(1000): validate_transition( - ContainerLifecycleState.DETECTED, + ContainerLifecycleState.DISCOVERED, ContainerLifecycleState.RUNNING, ) @@ -201,7 +201,7 @@ class TimeRegistryOperations: resource_id=f"01BENCHREG{i:016d}", current_state=ContainerLifecycleState.RUNNING if i % 2 == 0 - else ContainerLifecycleState.DETECTED, + else ContainerLifecycleState.DISCOVERED, ) set_lifecycle_tracker(tracker) diff --git a/features/container_clone_into.feature b/features/container_clone_into.feature new file mode 100644 index 000000000..c4447e3b8 --- /dev/null +++ b/features/container_clone_into.feature @@ -0,0 +1,92 @@ +@clone-into +Feature: container-instance --clone-into argument + As a CleverAgents user + I want to clone a git repository into a running container + So that I can work with the repository inside the container environment + + # ── URL validation ───────────────────────────────────────── + + Scenario Outline: validate_clone_into_url accepts valid git URLs + When I validate clone-into URL "" + Then the clone-into URL should be valid + + Examples: + | url | + | https://github.com/org/repo.git | + | http://internal.example.com/repo.git | + | git@github.com:org/repo.git | + | ssh://git@bitbucket.org/org/repo.git | + | git://github.com/org/repo.git | + | /local/path/to/repo | + | ./relative/repo | + + Scenario Outline: validate_clone_into_url rejects invalid URLs + When I validate clone-into URL "" + Then the clone-into URL should be invalid + + Examples: + | url | + | not-a-url | + | ftp://example.com | + + # ── clone_repo_into_container argument validation ────────── + + Scenario: clone_repo_into_container rejects empty container_id + When I call clone_repo_into_container with empty container_id + Then it should raise ValueError mentioning "container_id" + + Scenario: clone_repo_into_container rejects empty repo_url + When I call clone_repo_into_container with empty repo_url + Then it should raise ValueError mentioning "repo_url" + + # ── Successful clone ──────────────────────────────────────── + + Scenario: clone_repo_into_container succeeds when docker exec returns 0 + Given a mock docker exec that succeeds for clone + When I clone "https://github.com/org/repo.git" into container "abc123def456" + Then the clone should succeed + And the clone target should be "/workspace" + + Scenario: clone_repo_into_container uses custom target directory + Given a mock docker exec that succeeds for clone + When I clone repo "https://github.com/org/repo.git" into container "abc123def456" with target "/custom/dir" + Then the clone should succeed + And the clone target should be "/custom/dir" + + # ── Failure handling ──────────────────────────────────────── + + Scenario: clone_repo_into_container raises CloneIntoError when docker exec fails + Given a mock docker exec that fails for clone with stderr "fatal: repository not found" + When I clone "https://github.com/org/missing.git" into container "abc123def456" + Then the clone should raise CloneIntoError + And the CloneIntoError should mention "repository not found" + + # ── CloneIntoError attributes ────────────────────────────── + + Scenario: CloneIntoError stores repo_url, container_id, and stderr + Given a CloneIntoError with repo "https://example.com/repo.git" container "ctr-001" stderr "auth failed" + Then the CloneIntoError repo_url should be "https://example.com/repo.git" + And the CloneIntoError container_id should be "ctr-001" + And the CloneIntoError stderr should be "auth failed" + + # ── container-instance resource type has --clone-into arg ── + + Scenario: container-instance resource type has clone-into CLI argument + When I look up the "container-instance" resource type spec + Then the spec should have a CLI argument named "clone-into" + And the clone-into argument should be of type "string" + And the clone-into argument should not be required + + # ── validate_clone_into_url edge cases ───────────────────── + + Scenario: validate_clone_into_url rejects whitespace-only string + When I validate clone-into URL with whitespace only + Then the clone-into URL should be invalid + + # -- End-to-end handler wiring -- + + Scenario: DevcontainerHandler.resolve() calls clone_repo_into_container when clone_into property is set + Given a devcontainer-instance resource with clone_into "https://github.com/org/repo.git" and tracker container_id "abc123def456" + And clone_repo_into_container is mocked to succeed + When DevcontainerHandler.resolve() is called for the resource + Then clone_repo_into_container should have been called with container_id "abc123def456" and url "https://github.com/org/repo.git" diff --git a/features/devcontainer_cleanup.feature b/features/devcontainer_cleanup.feature index 8a455d11b..5c4c469b9 100644 --- a/features/devcontainer_cleanup.feature +++ b/features/devcontainer_cleanup.feature @@ -87,7 +87,7 @@ Feature: Devcontainer Session Cleanup and CLI Commands # ── R8: DevcontainerHandler coverage ─────────────────────── Scenario: DevcontainerHandler has expected class attributes and is instantiable - Then DevcontainerHandler should have _default_strategy "none" + Then DevcontainerHandler should have _default_strategy "snapshot" And DevcontainerHandler should have _type_label "devcontainer" And DevcontainerHandler should be instantiable diff --git a/features/devcontainer_handler.feature b/features/devcontainer_handler.feature index 20d78b6c4..50964cb15 100644 --- a/features/devcontainer_handler.feature +++ b/features/devcontainer_handler.feature @@ -85,7 +85,7 @@ Feature: Devcontainer Resource Handler Scenario: DevcontainerHandler has correct default strategy When I instantiate DevcontainerHandler - Then its default strategy should be "none" + Then its default strategy should be "snapshot" Scenario: DevcontainerHandler has correct type label When I instantiate DevcontainerHandler diff --git a/features/devcontainer_handler_protocol_methods.feature b/features/devcontainer_handler_protocol_methods.feature index 52578bce6..8eaf0330b 100644 --- a/features/devcontainer_handler_protocol_methods.feature +++ b/features/devcontainer_handler_protocol_methods.feature @@ -121,13 +121,13 @@ Feature: DevcontainerHandler missing protocol methods And dcproto the container is in RUNNING state And dcproto a mock sandbox manager that returns a valid sandbox When dcproto I create a sandbox for the resource with plan "plan-001" - Then dcproto the sandbox result should have strategy "none" + Then dcproto the sandbox result should have strategy "snapshot" And dcproto the sandbox result should have a sandbox path - Scenario: dcproto create_sandbox activates container when in DETECTED state + Scenario: dcproto create_sandbox activates container when in DISCOVERED state Given dcproto a devcontainer handler And dcproto a devcontainer-instance resource with location "/ws/project" - And dcproto the container is in DETECTED state + And dcproto the container is in DISCOVERED state And dcproto a mock sandbox manager that returns a valid sandbox And dcproto activate_container is mocked to succeed When dcproto I create a sandbox for the resource with plan "plan-002" diff --git a/features/devcontainer_sandbox_strategy.feature b/features/devcontainer_sandbox_strategy.feature new file mode 100644 index 000000000..fb88f0a59 --- /dev/null +++ b/features/devcontainer_sandbox_strategy.feature @@ -0,0 +1,58 @@ +@devcontainer-sandbox +Feature: devcontainer-instance uses snapshot sandbox strategy + As a CleverAgents developer + I want devcontainer-instance resources to use the snapshot sandbox strategy + So that plan execution inside containers is safely isolated + + # ── DevcontainerHandler default strategy ─────────────────── + + Scenario: DevcontainerHandler default strategy is snapshot + When I inspect DevcontainerHandler class attributes + Then the DevcontainerHandler _default_strategy should be "snapshot" + + # ── devcontainer-instance resource type spec ─────────────── + + Scenario: devcontainer-instance resource type has snapshot sandbox strategy + When I look up the "devcontainer-instance" resource type spec + Then the spec sandbox_strategy should be "snapshot" + + # ── devcontainer-instance child types ────────────────────── + + Scenario: devcontainer-instance child_types includes container-mount + When I look up the "devcontainer-instance" resource type spec + Then the spec child_types should contain "container-mount" + + Scenario: devcontainer-instance child_types includes container-exec-env + When I look up the "devcontainer-instance" resource type spec + Then the spec child_types should contain "container-exec-env" + + Scenario: devcontainer-instance child_types includes container-port + When I look up the "devcontainer-instance" resource type spec + Then the spec child_types should contain "container-port" + + Scenario: devcontainer-instance child_types includes devcontainer-file + When I look up the "devcontainer-instance" resource type spec + Then the spec child_types should contain "devcontainer-file" + + # ── ContainerLifecycleState initial state ────────────────── + + Scenario: ContainerLifecycleTracker default state is discovered + When I create a new ContainerLifecycleTracker for resource "01SANDBOX0000000000000001" + Then the tracker initial state should be "discovered" + + Scenario: ContainerLifecycleState has discovered value + When I inspect ContainerLifecycleState enum values + Then the lifecycle enum values should contain "discovered" + And the lifecycle enum values should not contain "detected" + + # ── BUILTIN_TYPES registration ───────────────────────────── + + Scenario: devcontainer-instance in BUILTIN_TYPES has snapshot strategy + When I look up "devcontainer-instance" in BUILTIN_TYPES + Then the BUILTIN_TYPES entry sandbox_strategy should be "snapshot" + + Scenario: devcontainer-instance in BUILTIN_TYPES has correct child_types + When I look up "devcontainer-instance" in BUILTIN_TYPES + Then the BUILTIN_TYPES entry child_types should include "container-mount" + And the BUILTIN_TYPES entry child_types should include "container-exec-env" + And the BUILTIN_TYPES entry child_types should include "container-port" diff --git a/features/devcontainer_state.feature b/features/devcontainer_state.feature index 859de4093..da7fe64d3 100644 --- a/features/devcontainer_state.feature +++ b/features/devcontainer_state.feature @@ -7,7 +7,7 @@ Feature: Devcontainer Lifecycle State Model Scenario: ContainerLifecycleState has all required values When I inspect ContainerLifecycleState enum values - Then the lifecycle enum should contain "detected" + Then the lifecycle enum should contain "discovered" And the lifecycle enum should contain "building" And the lifecycle enum should contain "running" And the lifecycle enum should contain "stopping" @@ -16,7 +16,7 @@ Feature: Devcontainer Lifecycle State Model Scenario: Valid transitions are accepted Given a lifecycle tracker for resource "01TESTLIFECYCLE0000000001" - When I transition from "detected" to "building" + When I transition from "discovered" to "building" Then the transition should succeed And the tracker state should be "building" @@ -28,7 +28,7 @@ Feature: Devcontainer Lifecycle State Model Scenario: Invalid transition is rejected Given a lifecycle tracker for resource "01TESTLIFECYCLE0000000003" - When I attempt to transition from "detected" to "running" + When I attempt to transition from "discovered" to "running" Then the transition should raise ValueError Scenario: Stopping to error transition is valid @@ -50,14 +50,14 @@ Feature: Devcontainer Lifecycle State Model Scenario: Transition history is recorded Given a lifecycle tracker for resource "01TESTLIFECYCLE0000000007" - When I transition from "detected" to "building" + When I transition from "discovered" to "building" Then the tracker should have 1 transition in history # ── Lifecycle registry ───────────────────────────────────── Scenario: Lifecycle tracker persists state across calls Given a lifecycle tracker for resource "01TESTLIFECYCLE0000000060" - When I transition from "detected" to "building" + When I transition from "discovered" to "building" And I retrieve the tracker for "01TESTLIFECYCLE0000000060" Then the retrieved tracker state should be "building" @@ -132,14 +132,14 @@ Feature: Devcontainer Lifecycle State Model When I attempt to transition from "failed" to "running" Then the transition should raise ValueError - Scenario: Invalid transition detected to stopping is rejected + Scenario: Invalid transition discovered to stopping is rejected Given a lifecycle tracker for resource "01TESTLIFECYCLE0000000244" - When I attempt to transition from "detected" to "stopping" + When I attempt to transition from "discovered" to "stopping" Then the transition should raise ValueError - Scenario: Invalid transition building to detected is rejected + Scenario: Invalid transition building to discovered is rejected Given a lifecycle tracker in "building" state for resource "01TESTLIFECYCLE0000000245" - When I attempt to transition from "building" to "detected" + When I attempt to transition from "building" to "discovered" Then the transition should raise ValueError # ── R4: Multi-line JSON output parsing ───────────────────── @@ -185,7 +185,7 @@ Feature: Devcontainer Lifecycle State Model Scenario: get_lifecycle_tracker auto-creates tracker for new resource When I get tracker for new resource "01TESTLIFECYCLE0000000730" - Then the auto-created tracker state should be "detected" + Then the auto-created tracker state should be "discovered" And the auto-created tracker resource_id should be "01TESTLIFECYCLE0000000730" Scenario: clear_lifecycle_registry stops health check threads diff --git a/features/resource_list_lifecycle_state.feature b/features/resource_list_lifecycle_state.feature index 9eded453d..e8ffdf7c6 100644 --- a/features/resource_list_lifecycle_state.feature +++ b/features/resource_list_lifecycle_state.feature @@ -1,7 +1,7 @@ Feature: agents resource list shows devcontainer lifecycle state column As a CleverAgents user I want the resource list to show the lifecycle state for devcontainer-instance resources - So that I can see whether a devcontainer is detected, running, stopped, or failed + So that I can see whether a devcontainer is discovered, running, stopped, or failed Background: Given a fresh in-memory resource registry @@ -14,10 +14,10 @@ Feature: agents resource list shows devcontainer lifecycle state column And the resource output should contain "Children" And the resource output should contain "Projects" - Scenario: resource list shows "detected (not built)" for a new devcontainer-instance + Scenario: resource list shows "discovered (not built)" for a new devcontainer-instance Given a devcontainer-instance resource is registered When I run resource list with all flag - Then the resource output should contain "detected (not built)" + Then the resource output should contain "discovered (not built)" Scenario: resource list shows "running" state for a running devcontainer Given a devcontainer-instance resource is registered @@ -43,16 +43,16 @@ Feature: agents resource list shows devcontainer lifecycle state column Then the resource output should contain "local/my-repo" And the resource output should contain "git-checkout" - Scenario: resource list shows warning banner when devcontainer is in detected state + Scenario: resource list shows warning banner when devcontainer is in discovered state Given a devcontainer-instance resource is registered When I run resource list with all flag - Then the resource output should contain "Devcontainer detected" + Then the resource output should contain "Devcontainer discovered" Scenario: resource list does not show warning banner when devcontainer is running Given a devcontainer-instance resource is registered And the devcontainer lifecycle state is "running" When I run resource list with all flag - Then the resource output should not contain "Devcontainer detected" + Then the resource output should not contain "Devcontainer discovered" @tdd_issue @tdd_issue_4263 @tdd_expected_fail Scenario: resource list JSON output includes lifecycle_state for devcontainer-instance @@ -70,7 +70,7 @@ Feature: agents resource list shows devcontainer lifecycle state column Scenario: resource list shows lifecycle state for container-instance resources Given a container-instance resource is registered When I run resource list with all flag - Then the resource output should contain "detected (not built)" + Then the resource output should contain "discovered (not built)" Scenario: resource list succeeds gracefully when lifecycle tracker raises an error Given a devcontainer-instance resource is registered diff --git a/features/steps/container_clone_into_steps.py b/features/steps/container_clone_into_steps.py new file mode 100644 index 000000000..bb683e09e --- /dev/null +++ b/features/steps/container_clone_into_steps.py @@ -0,0 +1,404 @@ +"""Step definitions for container_clone_into.feature. + +Tests the CloneIntoHandler module: URL validation, argument validation, +successful clone, failure handling, and container-instance resource type +CLI argument registration. +""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import MagicMock, patch + +from behave import given, then, when # type: ignore[import-untyped] +from behave.runner import Context # type: ignore[import-untyped] + +from cleveragents.application.services._resource_registry_data import ( + BUILTIN_TYPES, +) +from cleveragents.domain.models.core.container_lifecycle import ( + ContainerLifecycleState, + ContainerLifecycleTracker, +) +from cleveragents.resource.handlers.clone_into import ( + CloneIntoError, + clone_repo_into_container, + validate_clone_into_url, +) +from cleveragents.resource.handlers.devcontainer import ( + DevcontainerHandler, + clear_lifecycle_registry, + set_lifecycle_tracker, +) +from cleveragents.domain.models.core.resource import Resource + +# ── When steps ─────────────────────────────────────────────── + + +@when('I validate clone-into URL "{url}"') +def step_validate_url(context: Context, url: str) -> None: + """Validate a clone-into URL.""" + context.clone_url_error = None + try: + validate_clone_into_url(url) + context.clone_url_valid = True + except ValueError as exc: + context.clone_url_valid = False + context.clone_url_error = exc + + +@when("I validate clone-into URL with whitespace only") +def step_validate_whitespace_url(context: Context) -> None: + """Validate a whitespace-only clone-into URL.""" + context.clone_url_error = None + try: + validate_clone_into_url(" ") + context.clone_url_valid = True + except ValueError as exc: + context.clone_url_valid = False + context.clone_url_error = exc + + +@when("I call clone_repo_into_container with empty container_id") +def step_clone_empty_container_id(context: Context) -> None: + """Call clone_repo_into_container with an empty container_id.""" + context.clone_error = None + try: + clone_repo_into_container("", "https://github.com/org/repo.git") + except ValueError as exc: + context.clone_error = exc + + +@when("I call clone_repo_into_container with empty repo_url") +def step_clone_empty_repo_url(context: Context) -> None: + """Call clone_repo_into_container with an empty repo_url.""" + context.clone_error = None + try: + clone_repo_into_container("abc123def456", "") + except ValueError as exc: + context.clone_error = exc + + +@when('I clone "{repo_url}" into container "{container_id}"') +def step_clone_into_container( + context: Context, repo_url: str, container_id: str +) -> None: + """Clone a repository into a container.""" + context.clone_error = None + context.clone_result = None + try: + context.clone_result = clone_repo_into_container(container_id, repo_url) + except (CloneIntoError, ValueError) as exc: + context.clone_error = exc + + +@when( + 'I clone repo "{repo_url}" into container "{container_id}" with target "{target_dir}"' +) +def step_clone_into_container_at( + context: Context, repo_url: str, container_id: str, target_dir: str +) -> None: + """Clone a repository into a container at a specific target directory.""" + context.clone_error = None + context.clone_result = None + try: + context.clone_result = clone_repo_into_container( + container_id, repo_url, target_dir + ) + except (CloneIntoError, ValueError) as exc: + context.clone_error = exc + + +@when('I look up the "{type_name}" resource type spec') +def step_look_up_resource_type_spec(context: Context, type_name: str) -> None: + """Look up a resource type spec from BUILTIN_TYPES.""" + context.resource_type_spec = None + for entry in BUILTIN_TYPES: + if entry.get("name") == type_name: + context.resource_type_spec = entry + break + + +# ── Given steps ────────────────────────────────────────────── + + +@given("a mock docker exec that succeeds for clone") +def step_mock_docker_exec_success(context: Context) -> None: + """Patch subprocess.run to simulate a successful docker exec git clone.""" + mock_result = MagicMock() + mock_result.returncode = 0 + mock_result.stdout = "" + mock_result.stderr = "" + + patcher = patch( + "cleveragents.resource.handlers.clone_into.subprocess.run", + return_value=mock_result, + ) + patcher.start() + context.add_cleanup(patcher.stop) + + +@given('a mock docker exec that fails for clone with stderr "{stderr_msg}"') +def step_mock_docker_exec_failure(context: Context, stderr_msg: str) -> None: + """Patch subprocess.run to simulate a failed docker exec git clone.""" + mock_result = MagicMock() + mock_result.returncode = 1 + mock_result.stdout = "" + mock_result.stderr = stderr_msg + + patcher = patch( + "cleveragents.resource.handlers.clone_into.subprocess.run", + return_value=mock_result, + ) + patcher.start() + context.add_cleanup(patcher.stop) + + +@given( + 'a CloneIntoError with repo "{repo_url}" container "{container_id}" stderr "{stderr}"' +) +def step_create_clone_into_error( + context: Context, repo_url: str, container_id: str, stderr: str +) -> None: + """Create a CloneIntoError instance for attribute testing.""" + context.clone_into_error = CloneIntoError( + repo_url=repo_url, + container_id=container_id, + stderr=stderr, + ) + + +# ── Then steps ─────────────────────────────────────────────── + + +@then("the clone-into URL should be valid") +def step_url_should_be_valid(context: Context) -> None: + assert context.clone_url_valid is True, ( + "Expected URL to be valid but validate_clone_into_url returned False" + ) + + +@then("the clone-into URL should be invalid") +def step_url_should_be_invalid(context: Context) -> None: + assert context.clone_url_valid is False, ( + "Expected URL to be invalid but validate_clone_into_url returned True" + ) + + +@then('it should raise ValueError mentioning "{field}"') +def step_should_raise_value_error(context: Context, field: str) -> None: + assert context.clone_error is not None, ( + "Expected a ValueError but no error was raised" + ) + assert isinstance(context.clone_error, ValueError), ( + f"Expected ValueError, got {type(context.clone_error).__name__}" + ) + assert field.lower() in str(context.clone_error).lower(), ( + f"Expected '{field}' in error message, got: {context.clone_error!r}" + ) + + +@then("the clone should succeed") +def step_clone_should_succeed(context: Context) -> None: + assert context.clone_error is None, ( + f"Expected clone to succeed but got error: {context.clone_error!r}" + ) + assert context.clone_result is not None, "Expected a clone result but got None" + + +@then('the clone target should be "{expected_target}"') +def step_clone_target(context: Context, expected_target: str) -> None: + assert context.clone_result == expected_target, ( + f"Expected clone target '{expected_target}', got '{context.clone_result}'" + ) + + +@then("the clone should raise CloneIntoError") +def step_clone_should_raise(context: Context) -> None: + assert context.clone_error is not None, ( + "Expected CloneIntoError but no error was raised" + ) + assert isinstance(context.clone_error, CloneIntoError), ( + f"Expected CloneIntoError, got {type(context.clone_error).__name__}" + ) + + +@then('the CloneIntoError should mention "{text}"') +def step_clone_error_mentions(context: Context, text: str) -> None: + error_str = str(context.clone_error) + assert text.lower() in error_str.lower(), ( + f"Expected '{text}' in CloneIntoError message, got: {error_str!r}" + ) + + +@then('the CloneIntoError repo_url should be "{expected}"') +def step_clone_error_repo_url(context: Context, expected: str) -> None: + assert context.clone_into_error.repo_url == expected, ( + f"Expected repo_url '{expected}', got '{context.clone_into_error.repo_url}'" + ) + + +@then('the CloneIntoError container_id should be "{expected}"') +def step_clone_error_container_id(context: Context, expected: str) -> None: + assert context.clone_into_error.container_id == expected, ( + f"Expected container_id '{expected}', " + f"got '{context.clone_into_error.container_id}'" + ) + + +@then('the CloneIntoError stderr should be "{expected}"') +def step_clone_error_stderr(context: Context, expected: str) -> None: + assert context.clone_into_error.stderr == expected, ( + f"Expected stderr '{expected}', got '{context.clone_into_error.stderr}'" + ) + + +@then('the spec should have a CLI argument named "{arg_name}"') +def step_spec_has_cli_arg(context: Context, arg_name: str) -> None: + spec = context.resource_type_spec + assert spec is not None, "No resource type spec found" + cli_args: list[dict[str, Any]] = spec.get("cli_args", []) + arg_names = [arg.get("name") for arg in cli_args] + assert arg_name in arg_names, f"Expected CLI argument '{arg_name}' in {arg_names}" + + +@then('the clone-into argument should be of type "{expected_type}"') +def step_clone_into_arg_type(context: Context, expected_type: str) -> None: + spec = context.resource_type_spec + assert spec is not None, "No resource type spec found" + cli_args: list[dict[str, Any]] = spec.get("cli_args", []) + clone_into_arg = next( + (arg for arg in cli_args if arg.get("name") == "clone-into"), None + ) + assert clone_into_arg is not None, "clone-into argument not found" + assert clone_into_arg.get("type") == expected_type, ( + f"Expected type '{expected_type}', got '{clone_into_arg.get('type')}'" + ) + + +@then("the clone-into argument should not be required") +def step_clone_into_not_required(context: Context) -> None: + spec = context.resource_type_spec + assert spec is not None, "No resource type spec found" + cli_args: list[dict[str, Any]] = spec.get("cli_args", []) + clone_into_arg = next( + (arg for arg in cli_args if arg.get("name") == "clone-into"), None + ) + assert clone_into_arg is not None, "clone-into argument not found" + assert clone_into_arg.get("required") is False, ( + f"Expected clone-into to be optional (required=False), " + f"got required={clone_into_arg.get('required')}" + ) + + +# -- End-to-end handler wiring steps -- + + +@given( + 'a devcontainer-instance resource with clone_into "{clone_url}" and tracker container_id "{container_id}"' +) +def step_devcontainer_resource_with_clone_into( + context: Context, clone_url: str, container_id: str +) -> None: + """Set up a devcontainer-instance resource with clone_into property and tracker.""" + # Clear any existing lifecycle state + clear_lifecycle_registry() + + # Create a resource with clone_into in properties + # resource_id must be a valid ULID (Crockford Base32, 26 chars, no I/L/O/U) + # classification must be 'physical' or 'virtual' + context.handler_resource = Resource( + resource_id="01ARZ3NDEKTSV4RRFFQ69G5FAV", + name="test-devcontainer", + resource_type_name="devcontainer-instance", + classification="physical", + description="Test devcontainer for clone-into wiring", + location="/tmp/test-workspace", + properties={"clone_into": clone_url}, + ) + + # Set up the lifecycle tracker in DISCOVERED state with container_id pre-set + # (simulates the state after activate_container() has run and set container_id) + tracker = ContainerLifecycleTracker( + resource_id="01ARZ3NDEKTSV4RRFFQ69G5FAV", + current_state=ContainerLifecycleState.DISCOVERED, + container_id=container_id, + ) + set_lifecycle_tracker(tracker) + + context.handler_clone_url = clone_url + context.handler_container_id = container_id + + # Store cleanup reference + context.add_cleanup(clear_lifecycle_registry) + + +@given("clone_repo_into_container is mocked to succeed") +def step_mock_clone_repo_into_container(context: Context) -> None: + """Mock clone_repo_into_container to capture calls without running docker.""" + mock_clone = MagicMock(return_value="/workspace") + patcher = patch( + "cleveragents.resource.handlers.devcontainer.clone_repo_into_container", + mock_clone, + ) + patcher.start() + context.mock_clone_repo = mock_clone + context.add_cleanup(patcher.stop) + + +@when("DevcontainerHandler.resolve() is called for the resource") +def step_call_devcontainer_handler_resolve(context: Context) -> None: + """Call DevcontainerHandler.resolve() with the test resource.""" + handler = DevcontainerHandler() + mock_sandbox_manager = MagicMock() + mock_bound_resource = MagicMock() + + # Patch activate_container to be a no-op (tracker already has container_id) + # Patch BaseResourceHandler.resolve to return a mock BoundResource + with ( + patch("cleveragents.resource.handlers.devcontainer.activate_container"), + patch( + "cleveragents.resource.handlers._base.BaseResourceHandler.resolve", + return_value=mock_bound_resource, + ), + ): + context.handler_resolve_error = None + context.handler_resolve_result = None + try: + context.handler_resolve_result = handler.resolve( + resource=context.handler_resource, + plan_id="test-plan-001", + slot_name="test-slot", + sandbox_manager=mock_sandbox_manager, + ) + except Exception as exc: + context.handler_resolve_error = exc + + +@then( + 'clone_repo_into_container should have been called with container_id "{expected_container_id}" and url "{expected_url}"' +) +def step_assert_clone_called( + context: Context, expected_container_id: str, expected_url: str +) -> None: + """Assert that clone_repo_into_container was called with the expected arguments.""" + assert context.handler_resolve_error is None, ( + f"Expected resolve() to succeed but got error: {context.handler_resolve_error!r}" + ) + mock_clone = context.mock_clone_repo + assert mock_clone.called, ( + "Expected clone_repo_into_container to have been called, but it was not" + ) + call_args = mock_clone.call_args + actual_container_id = ( + call_args[0][0] if call_args[0] else call_args[1].get("container_id") + ) + actual_url = ( + call_args[0][1] if len(call_args[0]) > 1 else call_args[1].get("repo_url") + ) + assert actual_container_id == expected_container_id, ( + f"Expected container_id '{expected_container_id}', got '{actual_container_id}'" + ) + assert actual_url == expected_url, ( + f"Expected url '{expected_url}', got '{actual_url}'" + ) diff --git a/features/steps/devcontainer_handler_protocol_methods_steps.py b/features/steps/devcontainer_handler_protocol_methods_steps.py index 3122a0594..1e6eebc0d 100644 --- a/features/steps/devcontainer_handler_protocol_methods_steps.py +++ b/features/steps/devcontainer_handler_protocol_methods_steps.py @@ -152,18 +152,18 @@ def step_dcproto_container_running(context: Context) -> None: context.dcproto_container_state = ContainerLifecycleState.RUNNING -@given("dcproto the container is in DETECTED state") -def step_dcproto_container_detected(context: Context) -> None: +@given("dcproto the container is in DISCOVERED state") +def step_dcproto_container_discovered(context: Context) -> None: from cleveragents.resource.handlers._devcontainer_internals import ( set_lifecycle_tracker, ) - # DETECTED is the initial state — no transition needed. + # DISCOVERED is the initial state — no transition needed. tracker = ContainerLifecycleTracker( resource_id=context.dcproto_resource.resource_id, ) set_lifecycle_tracker(tracker) - context.dcproto_container_state = ContainerLifecycleState.DETECTED + context.dcproto_container_state = ContainerLifecycleState.DISCOVERED @given("dcproto the container is in STOPPED state") diff --git a/features/steps/devcontainer_lifecycle_steps.py b/features/steps/devcontainer_lifecycle_steps.py index 7af7a66d8..69f4cdf6e 100644 --- a/features/steps/devcontainer_lifecycle_steps.py +++ b/features/steps/devcontainer_lifecycle_steps.py @@ -164,7 +164,7 @@ def step_validate_transition_bad_to_state(context: Context) -> None: context.type_error = None try: validate_transition( - ContainerLifecycleState.DETECTED, + ContainerLifecycleState.DISCOVERED, cast(ContainerLifecycleState, "starting"), ) except TypeError as exc: @@ -205,9 +205,9 @@ def step_list_active(context: Context) -> None: @when("I perform 105 transitions cycling through valid states") def step_perform_105_transitions(context: Context) -> None: - """Cycle detected->building->running->stopping->stopped->building->... 105 times.""" + """Cycle discovered->building->running->stopping->stopped->building->... 105 times.""" tracker = context.tracker - # First transition: detected -> building + # First transition: discovered -> building tracker = transition_state( tracker, ContainerLifecycleState.BUILDING, reason="t1-init" ) diff --git a/features/steps/devcontainer_sandbox_strategy_steps.py b/features/steps/devcontainer_sandbox_strategy_steps.py new file mode 100644 index 000000000..562283396 --- /dev/null +++ b/features/steps/devcontainer_sandbox_strategy_steps.py @@ -0,0 +1,119 @@ +"""Step definitions for devcontainer_sandbox_strategy.feature. + +Tests that devcontainer-instance uses snapshot sandbox strategy, +has the correct child types, and that ContainerLifecycleState uses +'discovered' as the initial state. +""" + +from __future__ import annotations + +from behave import then, when +from behave.runner import Context + +from cleveragents.application.services._resource_registry_data import ( + BUILTIN_TYPES, +) +from cleveragents.domain.models.core.container_lifecycle import ( + ContainerLifecycleTracker, +) +from cleveragents.resource.handlers.devcontainer import DevcontainerHandler + +# ── When steps ─────────────────────────────────────────────── + + +@when("I inspect DevcontainerHandler class attributes") +def step_inspect_devcontainer_handler(context: Context) -> None: + """Capture DevcontainerHandler class attributes for inspection.""" + context.devcontainer_handler_strategy = DevcontainerHandler._default_strategy + + +@when('I create a new ContainerLifecycleTracker for resource "{resource_id}"') +def step_create_tracker_for_sandbox(context: Context, resource_id: str) -> None: + """Create a new ContainerLifecycleTracker with default state.""" + context.sandbox_tracker = ContainerLifecycleTracker(resource_id=resource_id) + + +@when('I look up "{type_name}" in BUILTIN_TYPES') +def step_look_up_in_builtin_types(context: Context, type_name: str) -> None: + """Look up a resource type entry in BUILTIN_TYPES.""" + context.builtin_type_entry = None + for entry in BUILTIN_TYPES: + if entry.get("name") == type_name: + context.builtin_type_entry = entry + break + + +# ── Then steps ─────────────────────────────────────────────── + + +@then('the DevcontainerHandler _default_strategy should be "{expected}"') +def step_devcontainer_handler_strategy(context: Context, expected: str) -> None: + strategy = context.devcontainer_handler_strategy + strategy_value = strategy.value if hasattr(strategy, "value") else str(strategy) + assert strategy_value == expected, ( + f"Expected DevcontainerHandler._default_strategy to be '{expected}', " + f"got '{strategy_value}'" + ) + + +@then('the spec sandbox_strategy should be "{expected}"') +def step_spec_sandbox_strategy(context: Context, expected: str) -> None: + spec = context.resource_type_spec + assert spec is not None, "No resource type spec found" + actual = spec.get("sandbox_strategy") + assert actual == expected, f"Expected sandbox_strategy '{expected}', got '{actual}'" + + +@then('the spec child_types should contain "{child_type}"') +def step_spec_child_types_contains(context: Context, child_type: str) -> None: + spec = context.resource_type_spec + assert spec is not None, "No resource type spec found" + child_types: list[str] = spec.get("child_types", []) + assert child_type in child_types, ( + f"Expected '{child_type}' in child_types {child_types}" + ) + + +@then('the tracker initial state should be "{expected}"') +def step_tracker_initial_state(context: Context, expected: str) -> None: + tracker = context.sandbox_tracker + actual = tracker.current_state.value + assert actual == expected, ( + f"Expected initial tracker state '{expected}', got '{actual}'" + ) + + +@then('the lifecycle enum values should contain "{expected}"') +def step_lifecycle_enum_contains(context: Context, expected: str) -> None: + values = context.enum_values + assert expected in values, ( + f"Expected '{expected}' in lifecycle enum values {values}" + ) + + +@then('the lifecycle enum values should not contain "{unexpected}"') +def step_lifecycle_enum_not_contains(context: Context, unexpected: str) -> None: + values = context.enum_values + assert unexpected not in values, ( + f"Expected '{unexpected}' NOT to be in lifecycle enum values {values}" + ) + + +@then('the BUILTIN_TYPES entry sandbox_strategy should be "{expected}"') +def step_builtin_types_sandbox_strategy(context: Context, expected: str) -> None: + entry = context.builtin_type_entry + assert entry is not None, "No BUILTIN_TYPES entry found" + actual = entry.get("sandbox_strategy") + assert actual == expected, ( + f"Expected BUILTIN_TYPES sandbox_strategy '{expected}', got '{actual}'" + ) + + +@then('the BUILTIN_TYPES entry child_types should include "{child_type}"') +def step_builtin_types_child_types(context: Context, child_type: str) -> None: + entry = context.builtin_type_entry + assert entry is not None, "No BUILTIN_TYPES entry found" + child_types: list[str] = entry.get("child_types", []) + assert child_type in child_types, ( + f"Expected '{child_type}' in BUILTIN_TYPES child_types {child_types}" + ) diff --git a/features/steps/resource_list_lifecycle_state_steps.py b/features/steps/resource_list_lifecycle_state_steps.py index 5e6dcaddb..8a37dd6f9 100644 --- a/features/steps/resource_list_lifecycle_state_steps.py +++ b/features/steps/resource_list_lifecycle_state_steps.py @@ -102,7 +102,7 @@ def step_register_devcontainer_resource(context: Context) -> None: properties=None, ) context.devcontainer_resource_id = resource.resource_id - # Default state is DETECTED — no tracker manipulation needed + # Default state is DISCOVERED — no tracker manipulation needed @given("a container-instance resource is registered") @@ -119,7 +119,7 @@ def step_register_container_resource(context: Context) -> None: properties=None, ) context.devcontainer_resource_id = resource.resource_id - # Default state is DETECTED — no tracker manipulation needed + # Default state is DISCOVERED — no tracker manipulation needed @given('the devcontainer lifecycle state is "{state}"') diff --git a/robot/helper_devcontainer_handler.py b/robot/helper_devcontainer_handler.py index 7492d62cf..db79c3a52 100644 --- a/robot/helper_devcontainer_handler.py +++ b/robot/helper_devcontainer_handler.py @@ -75,12 +75,11 @@ def cmd_protocol_check() -> None: def cmd_strategy_check() -> None: """Verify DevcontainerHandler default strategy. - F22/F25 fix: handler uses ``none`` because SandboxFactory has not yet - implemented ``snapshot``. The container itself provides isolation. - Known limitation — will switch to ``snapshot`` once implemented. + PR #8304 fix: handler now uses ``snapshot`` strategy for safe plan + execution inside containers (ADR-043 devcontainer integration). """ handler = DevcontainerHandler() - assert handler._default_strategy.value == "none" + assert handler._default_strategy.value == "snapshot" print("strategy-check-ok") diff --git a/robot/helper_devcontainer_lifecycle.py b/robot/helper_devcontainer_lifecycle.py index 604bab7d6..8139fd28d 100644 --- a/robot/helper_devcontainer_lifecycle.py +++ b/robot/helper_devcontainer_lifecycle.py @@ -104,14 +104,14 @@ class _MockRunner: def cmd_enum_values() -> None: """Verify all six lifecycle states exist.""" - expected = {"detected", "building", "running", "stopping", "stopped", "failed"} + expected = {"discovered", "building", "running", "stopping", "stopped", "failed"} actual = {s.value for s in ContainerLifecycleState} assert actual == expected, f"Expected {expected}, got {actual}" print("enum-values-ok") def cmd_transition_valid() -> None: - """Verify detected->building transition.""" + """Verify discovered->building transition.""" clear_lifecycle_registry() tracker = ContainerLifecycleTracker(resource_id="01ROBOTTEST0000000000001") tracker = transition_state(tracker, ContainerLifecycleState.BUILDING, reason="test") diff --git a/src/cleveragents/application/services/_resource_registry_data.py b/src/cleveragents/application/services/_resource_registry_data.py index 714dd61ef..ce5b9fe5b 100644 --- a/src/cleveragents/application/services/_resource_registry_data.py +++ b/src/cleveragents/application/services/_resource_registry_data.py @@ -223,6 +223,17 @@ _GIT_FS_CONTAINER_TYPES: list[dict[str, Any]] = [ ), "default": None, }, + { + "name": "clone-into", + "type": "string", + "required": False, + "description": ( + "Git repository URL to clone into the container after " + "it starts. The repository is cloned into the container " + "workspace using 'git clone '." + ), + "default": None, + }, ], "parent_types": [ "container-runtime", @@ -265,7 +276,12 @@ _GIT_FS_CONTAINER_TYPES: list[dict[str, Any]] = [ }, ], "parent_types": ["git-checkout", "fs-directory"], - "child_types": ["devcontainer-file"], + "child_types": [ + "devcontainer-file", + "container-mount", + "container-exec-env", + "container-port", + ], "auto_discovery": { "trigger_types": ["git-checkout", "fs-directory"], "scan_paths": [ diff --git a/src/cleveragents/cli/commands/resource.py b/src/cleveragents/cli/commands/resource.py index e92d41073..bc4d6a754 100644 --- a/src/cleveragents/cli/commands/resource.py +++ b/src/cleveragents/cli/commands/resource.py @@ -159,7 +159,7 @@ def _get_lifecycle_state_str(resource: Any) -> str | None: resource: A Resource domain object. Returns: - Lifecycle state string (e.g. ``"detected (not built)"``, ``"running"``) + Lifecycle state string (e.g. ``"discovered (not built)"``, ``"running"``) or ``None`` if the resource type has no lifecycle state. """ if resource.resource_type_name not in _CONTAINER_TYPES: @@ -167,8 +167,8 @@ def _get_lifecycle_state_str(resource: Any) -> str | None: try: tracker = get_lifecycle_tracker(resource.resource_id) state = tracker.current_state - if state == ContainerLifecycleState.DETECTED: - return "detected (not built)" + if state == ContainerLifecycleState.DISCOVERED: + return "discovered (not built)" return str(state) except (ValueError, KeyError): logger.warning( @@ -897,7 +897,7 @@ def resource_list( if len(res_id) > 12: res_id = res_id[:12] + "..." lifecycle_state = _get_lifecycle_state_str(res) - if lifecycle_state == "detected (not built)": + if lifecycle_state == "discovered (not built)": detected_devcontainers.append(res) if lifecycle_state: lifecycle_states.append((res, lifecycle_state)) @@ -921,7 +921,7 @@ def resource_list( location = res.location or "(unknown path)" devcontainer_json = f"{location}/.devcontainer/devcontainer.json" console.print( - f"[yellow]⚠ Devcontainer detected at {devcontainer_json}[/yellow]" + f"[yellow]⚠ Devcontainer discovered at {devcontainer_json}[/yellow]" ) console.print(" Container will be built lazily on first access.") console.print( diff --git a/src/cleveragents/domain/models/core/container_lifecycle.py b/src/cleveragents/domain/models/core/container_lifecycle.py index a45ed5d97..819ef4e8f 100644 --- a/src/cleveragents/domain/models/core/container_lifecycle.py +++ b/src/cleveragents/domain/models/core/container_lifecycle.py @@ -7,7 +7,7 @@ validation logic for devcontainer-instance resources. :: - detected ──► building ──► running ──► stopping ──► stopped + discovered ──► building ──► running ──► stopping ──► stopped │ │ ▼ │ failed ◄───────────────────────────────┘ @@ -19,9 +19,9 @@ validation logic for devcontainer-instance resources. Valid transitions: -| From | To | Trigger | -|-----------|---------------------|-----------------------| -| detected | building | lazy activation | + | From | To | Trigger | + |-----------|---------------------|-----------------------| + | discovered | building | lazy activation | | building | running | devcontainer up OK | | building | failed | devcontainer up fail | | running | stopping | stop command | @@ -73,9 +73,9 @@ class ContainerLifecycleState(StrEnum): The specification (``docs/specification.md``) uses the implementation names. Issue #514 should be updated to match. - | Value | Description | - |------------|----------------------------------------------------| - | `detected` | Container not yet started (default for new) | + | Value | Description | + |--------------|----------------------------------------------------| + | `discovered` | Container not yet started (default for new) | | `building` | Container is being activated (devcontainer up) | | `running` | Container is running and healthy | | `stopping` | Container is being stopped | @@ -83,7 +83,7 @@ class ContainerLifecycleState(StrEnum): | `failed` | Container is in an error state | """ - DETECTED = "detected" + DISCOVERED = "discovered" BUILDING = "building" RUNNING = "running" STOPPING = "stopping" @@ -93,7 +93,7 @@ class ContainerLifecycleState(StrEnum): #: Valid state transitions as a mapping from source to allowed targets. VALID_TRANSITIONS: dict[ContainerLifecycleState, frozenset[ContainerLifecycleState]] = { - ContainerLifecycleState.DETECTED: frozenset({ContainerLifecycleState.BUILDING}), + ContainerLifecycleState.DISCOVERED: frozenset({ContainerLifecycleState.BUILDING}), ContainerLifecycleState.BUILDING: frozenset( {ContainerLifecycleState.RUNNING, ContainerLifecycleState.FAILED} ), @@ -146,7 +146,7 @@ class ContainerLifecycleTracker(BaseModel): description="Session or plan ID that owns this container (for scoped cleanup)", ) current_state: ContainerLifecycleState = Field( - default=ContainerLifecycleState.DETECTED, + default=ContainerLifecycleState.DISCOVERED, description="Current lifecycle state", ) container_id: str | None = Field( diff --git a/src/cleveragents/infrastructure/database/repositories.py b/src/cleveragents/infrastructure/database/repositories.py index 95da8038d..6ae44b067 100644 --- a/src/cleveragents/infrastructure/database/repositories.py +++ b/src/cleveragents/infrastructure/database/repositories.py @@ -5922,6 +5922,7 @@ class CheckpointRepository: if pruned_ids: session.flush() + session.commit() return pruned_ids except (OperationalError, SQLAlchemyDatabaseError) as exc: session.rollback() diff --git a/src/cleveragents/resource/handlers/clone_into.py b/src/cleveragents/resource/handlers/clone_into.py new file mode 100644 index 000000000..13980ce50 --- /dev/null +++ b/src/cleveragents/resource/handlers/clone_into.py @@ -0,0 +1,166 @@ +"""CloneIntoHandler: git clone into a running container. + +Implements the ``--clone-into`` CLI argument for ``container-instance`` +resources. When a ``container-instance`` is registered with a +``clone_into`` property, this handler clones the specified git +repository URL into the container's workspace using +``docker exec git clone``. + +Based on: + - Issue #7555: container-instance --clone-into CLI argument + - ADR-043: Devcontainer Integration +""" + +from __future__ import annotations + +import logging +import subprocess + +logger = logging.getLogger(__name__) + +#: Timeout in seconds for the git clone subprocess. +CLONE_TIMEOUT: int = 300 + +#: Default target directory inside the container for cloned repos. +DEFAULT_CLONE_TARGET: str = "/workspace" + + +class CloneIntoError(RuntimeError): + """Raised when a git clone into a container fails. + + Attributes: + repo_url: The repository URL that was being cloned. + container_id: The container ID that was targeted. + stderr: The stderr output from the failed command. + """ + + def __init__( + self, + repo_url: str, + container_id: str, + stderr: str, + ) -> None: + self.repo_url = repo_url + self.container_id = container_id + self.stderr = stderr + super().__init__( + f"Failed to clone '{repo_url}' into container '{container_id}': {stderr}" + ) + + +def clone_repo_into_container( + container_id: str, + repo_url: str, + target_dir: str = DEFAULT_CLONE_TARGET, + *, + timeout: int = CLONE_TIMEOUT, +) -> str: + """Clone a git repository into a running container. + + Executes ``docker exec git clone `` + to clone the repository into the container's filesystem. + + Args: + container_id: The Docker container ID or name to clone into. + repo_url: The git repository URL to clone. + target_dir: The target directory inside the container. + Defaults to ``/workspace``. + timeout: Maximum seconds to wait for the clone to complete. + Defaults to :data:`CLONE_TIMEOUT` (300 seconds). + + Returns: + The target directory path where the repository was cloned. + + Raises: + ValueError: If ``container_id`` or ``repo_url`` is empty. + CloneIntoError: If the git clone command fails. + subprocess.TimeoutExpired: If the clone exceeds ``timeout``. + """ + if not container_id or not container_id.strip(): + raise ValueError("container_id must be a non-empty string") + if not repo_url or not repo_url.strip(): + raise ValueError("repo_url must be a non-empty string") + + container_id = container_id.strip() + repo_url = repo_url.strip() + target_dir = target_dir.strip() if target_dir else DEFAULT_CLONE_TARGET + + logger.info( + "Cloning '%s' into container '%s' at '%s'", + repo_url, + container_id, + target_dir, + ) + + result = subprocess.run( + [ + "docker", + "exec", + container_id, + "git", + "clone", + repo_url, + target_dir, + ], + capture_output=True, + text=True, + timeout=timeout, + check=False, + ) + + if result.returncode != 0: + stderr = result.stderr.strip() + logger.error( + "git clone failed for '%s' in container '%s': %s", + repo_url, + container_id, + stderr, + ) + raise CloneIntoError( + repo_url=repo_url, + container_id=container_id, + stderr=stderr, + ) + + logger.info( + "Successfully cloned '%s' into container '%s' at '%s'", + repo_url, + container_id, + target_dir, + ) + return target_dir + + +def validate_clone_into_url(url: str) -> None: + """Validate that a URL is a plausible git repository URL. + + Accepts: + - HTTPS URLs: ``https://...`` + - SSH URLs: ``git@...`` or ``ssh://...`` + - Local paths: ``/...`` or ``./...`` + - Git protocol: ``git://...`` + + Args: + url: The URL string to validate. + + Raises: + ValueError: If the URL is empty or does not match any valid prefix. + """ + if not url or not url.strip(): + raise ValueError("URL must be a non-empty string") + url = url.strip() + valid_prefixes = ( + "https://", + "http://", + "git@", + "ssh://", + "git://", + "/", + "./", + "../", + ) + if not any(url.startswith(prefix) for prefix in valid_prefixes): + raise ValueError( + f"Invalid git repository URL: {url!r}. " + f"Must start with one of: {', '.join(valid_prefixes)}" + ) diff --git a/src/cleveragents/resource/handlers/devcontainer.py b/src/cleveragents/resource/handlers/devcontainer.py index 1be783f92..20ebaba60 100644 --- a/src/cleveragents/resource/handlers/devcontainer.py +++ b/src/cleveragents/resource/handlers/devcontainer.py @@ -1,8 +1,8 @@ """Devcontainer resource handler for CleverAgents. Resolves ``devcontainer-instance`` and ``devcontainer-file`` resources -into sandbox-backed :class:`BoundResource` instances using the ``none`` -sandbox strategy (the container itself provides isolation — F22/F25 fix). +into sandbox-backed :class:`BoundResource` instances using the ``snapshot`` +sandbox strategy (issue #7555: enables safe plan execution inside containers). The handler: @@ -72,6 +72,10 @@ from cleveragents.resource.handlers._devcontainer_internals import ( list_active_containers, set_lifecycle_tracker, ) +from cleveragents.resource.handlers.clone_into import ( + clone_repo_into_container, + validate_clone_into_url, +) from cleveragents.resource.handlers.devcontainer_cleanup import ( evict_terminal_trackers, list_active_containers_for_session, @@ -119,6 +123,7 @@ __all__ = [ "_stop_health_check", "activate_container", "clear_lifecycle_registry", + "clone_repo_into_container", "evict_terminal_trackers", "get_lifecycle_tracker", "list_active_containers", @@ -128,6 +133,7 @@ __all__ = [ "start_health_check", "stop_all_active_containers", "stop_container", + "validate_clone_into_url", ] @@ -137,22 +143,18 @@ class DevcontainerHandler(BaseResourceHandler): ``devcontainer-instance`` inherits from ``container-instance`` per ADR-042 (resource type inheritance). - Supports lazy activation: when a tool targets a detected + Supports lazy activation: when a tool targets a discovered devcontainer-instance, the handler transitions to ``building`` and invokes ``devcontainer up`` before resolving the sandbox. - F22/F25 fix: uses ``none`` sandbox strategy instead of ``snapshot`` - because the container itself provides isolation and - ``SandboxFactory`` has not yet implemented the ``snapshot`` - strategy. ``devcontainer-file`` resources also use ``none`` - (read-only config file). + Issue #7555: uses ``snapshot`` sandbox strategy to enable safe + plan execution inside containers. ``devcontainer-file`` resources + use ``copy_on_write`` (read-only config file). """ - # F22/F25 fix: SNAPSHOT raises NotImplementedError in SandboxFactory. - # The container IS the sandbox for devcontainer-instance, so NONE is - # semantically correct until a dedicated container-snapshot strategy - # is implemented. - _default_strategy = SandboxStrategy.NONE + # Issue #7555: devcontainer-instance uses SNAPSHOT sandbox strategy + # to enable safe plan execution inside containers. + _default_strategy = SandboxStrategy.SNAPSHOT _type_label = "devcontainer" def content_hash( @@ -301,7 +303,7 @@ class DevcontainerHandler(BaseResourceHandler): _ACTIVATABLE_STATES = frozenset( { - ContainerLifecycleState.DETECTED, + ContainerLifecycleState.DISCOVERED, ContainerLifecycleState.STOPPED, ContainerLifecycleState.FAILED, } @@ -323,6 +325,10 @@ class DevcontainerHandler(BaseResourceHandler): base-class sandbox resolution. For other resource types (e.g. ``devcontainer-file``), it delegates directly. + If the resource has a ``clone_into`` property, the specified git + repository is cloned into the container after activation (issue + #7555). + Args: resource: The resource domain object to resolve. plan_id: The plan requesting the sandbox. @@ -347,6 +353,26 @@ class DevcontainerHandler(BaseResourceHandler): session_id=plan_id, ) + # Issue #7555: wire --clone-into after container is running. + # Read the updated tracker to get the container_id assigned + # during activation, then clone the repository if requested. + if resource.properties: + clone_into_url = resource.properties.get("clone_into") + if clone_into_url and isinstance(clone_into_url, str): + validate_clone_into_url(clone_into_url) + updated_tracker = get_lifecycle_tracker(resource.resource_id) + if updated_tracker.container_id: + logger.info( + "Cloning '%s' into container '%s' (resource %s)", + clone_into_url, + updated_tracker.container_id, + resource.resource_id, + ) + clone_repo_into_container( + updated_tracker.container_id, + clone_into_url, + ) + return super().resolve( resource=resource, plan_id=plan_id, @@ -618,11 +644,6 @@ class DevcontainerHandler(BaseResourceHandler): # We construct a temporary resource pointing at other_location and # call the base-class content_hash directly (bypassing the # devcontainer override which would try to exec into the container). - from cleveragents.resource.handlers._base import ( - EMPTY_CONTENT_HASH, - BaseResourceHandler, - ) - other_resource = resource.__class__( resource_id=resource.resource_id, name=resource.name, @@ -654,13 +675,14 @@ class DevcontainerHandler(BaseResourceHandler): ) -> SandboxResult: """Create an isolated sandbox for a devcontainer resource. - For devcontainer resources the container itself provides - isolation (``none`` sandbox strategy — F22/F25 fix). This - method delegates to the base-class implementation which calls - :meth:`SandboxManager.get_or_create_sandbox` with the ``none`` - strategy. + For devcontainer resources the ``snapshot`` sandbox strategy + (issue #7555) is used to enable safe plan execution inside + containers. This method delegates to the base-class + implementation which calls + :meth:`SandboxManager.get_or_create_sandbox` with the + ``snapshot`` strategy. - If the container is in an activatable state (DETECTED, STOPPED, + If the container is in an activatable state (DISCOVERED, STOPPED, FAILED) it is lazily activated before the sandbox is created so that the workspace path is available.