fix(resource): wire discover_devcontainers() into git-checkout and fs-directory handlers #10907

Merged
HAL9000 merged 2 commits from bugfix/m6-devcontainer-autodiscovery-wiring into master 2026-05-08 00:11:51 +00:00
6 changed files with 491 additions and 31 deletions
+9
View File
1
@@ -67,6 +67,15 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
infinite recursion at runtime. Added Behave regression tests
(`features/actor_subgraph_cycle_detection.feature`) and a Robot Framework
integration test (`robot/actor_compiler.robot`) to prevent regressions.
- **Devcontainer auto-discovery wired into `git-checkout`/`fs-directory` handlers** (#4740):
`GitCheckoutHandler.discover_children()` and `FsDirectoryHandler.discover_children()` now
call `discover_devcontainers()` after scanning for `fs-directory` children. Any
`.devcontainer/devcontainer.json` or root-level `.devcontainer.json` found at the resource
location is registered as a `devcontainer-instance` child resource with
`provisioning_state: discovered`. Named configurations (`.devcontainer/<name>/devcontainer.json`)
are also discovered and carry the configuration name in the `config_name` property.
This wires the previously-isolated `discover_devcontainers()` function into the production
code path, enabling the spec's zero-configuration devcontainer experience.
### Changed
+16 -19
View File
@@ -57,35 +57,32 @@ inherits. Represents a generic container execution environment.
| Child types | (none) |
| Handler | `DevcontainerHandler` |
## Auto-Discovery (Planned)
## Auto-Discovery
> **Not yet wired (F31/F23):** The discovery module
> (`discover_devcontainers()`) exists and is tested in isolation, but
> is **not** invoked during `project link-resource` or any other
> production code path. Auto-discovery will be wired in a follow-up PR.
> For now, devcontainer-instance resources must be added manually via
> `agents resource add`.
Auto-discovery is wired into `GitCheckoutHandler.discover_children()` and
`FsDirectoryHandler.discover_children()` (issue #4740). When either handler
scans a resource location, it calls `discover_devcontainers()` after
discovering `fs-directory` children.
When wired, linking a `git-checkout` or `fs-directory` resource to a
project will trigger an auto-discovery hook that scans for devcontainer
configurations in the following locations (relative to the resource
root):
Devcontainer configurations are detected at the following locations
(relative to the resource root):
1. `.devcontainer/devcontainer.json`
2. `.devcontainer.json` (root-level)
3. `.devcontainer/<name>/devcontainer.json` (named configurations)
### Discovery Process (Planned)
### Discovery Process
1. **Trigger**: Linking a `git-checkout` or `fs-directory` resource.
2. **Scan**: The discovery module checks for configuration files at the
well-known paths listed above.
1. **Trigger**: `discover_children()` is called on a `git-checkout` or
`fs-directory` resource.
2. **Scan**: `discover_devcontainers()` checks for configuration files at
the well-known paths listed above.
3. **Validate**: Each discovered file is parsed as JSON. Invalid files
are skipped with a warning.
4. **Register**: For each valid configuration:
- A `devcontainer-instance` child resource is created under the
parent resource.
- A `devcontainer-file` child resource is created under the
devcontainer instance, pointing to the JSON file.
parent resource with `provisioning_state: discovered`.
- Named configurations carry the subdirectory name as `config_name`.
### Lazy Activation
@@ -250,7 +247,7 @@ confirmation unless `--yes` (`-y`) is passed.
| Registry eviction | Terminal-state trackers (stopped/failed) are evicted when count exceeds 200 (`_MAX_TERMINAL_TRACKERS`). Eviction runs automatically after bulk cleanup via `stop_all_active_containers`. Long-running processes with many container cycles may lose historical tracker data. | Acceptable for MVP; persisted state in M7+ eliminates this. |
| Sandbox strategy (F22/F25) | Specification uses `container_snapshot`; `SandboxFactory` does not yet implement `snapshot`. Handler now uses `SandboxStrategy.NONE` — the container itself provides isolation. | Implement `container_snapshot` in `SandboxFactory` and switch handler back to it. |
| Hardcoded `docker stop` (F21) | `stop_container()` calls `docker stop` directly. Devcontainer CLI can target Podman or other engines; the devcontainer CLI does not yet offer a `stop` subcommand. | Detect the container engine from resource properties or devcontainer CLI config and dispatch to the appropriate stop command. |
| Auto-discovery not wired (F23) | Documentation describes auto-discovery on `project link-resource`, but no code path invokes `discover_devcontainers()` during resource linking. | Wire the discovery hook into the resource linking flow. |
| Auto-discovery wired (F23 resolved) | `discover_devcontainers()` is now called from `GitCheckoutHandler.discover_children()` and `FsDirectoryHandler.discover_children()`. Devcontainer configs are auto-detected when `discover_children()` is invoked. | Completed in issue #4740. |
| Container execution stubbed (F24) | `ToolRunner` returns an error for `ExecutionEnvironment.CONTAINER`, so lazy activation cannot trigger via tool use in production. Lazy activation currently works through `DevcontainerHandler.resolve()` on plan sandbox resolution. | Full container execution support is scoped for a follow-up PR (#616). |
## DevcontainerHandler Protocol Methods
@@ -0,0 +1,62 @@
Feature: Devcontainer auto-discovery wired into git-checkout and fs-directory handlers
As a CleverAgents user
I want devcontainer configurations to be automatically discovered
When I register a git-checkout or fs-directory resource
So that devcontainer-instance child resources are created without manual intervention
Scenario: git-checkout discover_children finds root devcontainer config
Given dcwire a git repo with a ".devcontainer/devcontainer.json" file
When dcwire I call discover_children on the git-checkout resource
Then dcwire the children include a "devcontainer-instance" resource named "devcontainer-default"
And dcwire the devcontainer child has provisioning_state "discovered"
And dcwire the devcontainer child has devcontainer_json_path set
Scenario: git-checkout discover_children finds named devcontainer config
Given dcwire a git repo with a ".devcontainer/api/devcontainer.json" named config
When dcwire I call discover_children on the git-checkout resource
Then dcwire the children include a "devcontainer-instance" resource named "devcontainer-api"
And dcwire the devcontainer child has config_name "api"
Scenario: git-checkout discover_children with no devcontainer returns only directories
Given dcwire a git repo with no devcontainer configuration
When dcwire I call discover_children on the git-checkout resource
Then dcwire no devcontainer-instance children are present
Scenario: git-checkout discover_children includes both fs-directory and devcontainer children
Given dcwire a git repo with a subdirectory "src" and a ".devcontainer/devcontainer.json" file
When dcwire I call discover_children on the git-checkout resource
Then dcwire the children include a "fs-directory" resource named "src"
And dcwire the children include a "devcontainer-instance" resource named "devcontainer-default"
Scenario: fs-directory discover_children finds root devcontainer config
Given dcwire a filesystem directory with a ".devcontainer/devcontainer.json" file
When dcwire I call discover_children on the fs-directory resource
Then dcwire the children include a "devcontainer-instance" resource named "devcontainer-default"
And dcwire the devcontainer child has provisioning_state "discovered"
Scenario: fs-directory discover_children finds named devcontainer config
Given dcwire a filesystem directory with a ".devcontainer/frontend/devcontainer.json" named config
When dcwire I call discover_children on the fs-directory resource
Then dcwire the children include a "devcontainer-instance" resource named "devcontainer-frontend"
And dcwire the devcontainer child has config_name "frontend"
Scenario: fs-directory discover_children with no devcontainer returns only directories
Given dcwire a filesystem directory with no devcontainer configuration
When dcwire I call discover_children on the fs-directory resource
Then dcwire no devcontainer-instance children are present
Scenario: fs-directory discover_children includes both fs-directory and devcontainer children
Given dcwire a filesystem directory with a subdirectory "lib" and a ".devcontainer/devcontainer.json" file
When dcwire I call discover_children on the fs-directory resource
Then dcwire the children include a "fs-directory" resource named "lib"
And dcwire the children include a "devcontainer-instance" resource named "devcontainer-default"
Scenario: git-checkout discover_children finds root-level .devcontainer.json
Given dcwire a git repo with a root-level ".devcontainer.json" file
When dcwire I call discover_children on the git-checkout resource
Then dcwire the children include a "devcontainer-instance" resource named "devcontainer-default"
Scenario: fs-directory discover_children finds root-level .devcontainer.json
Given dcwire a filesystem directory with a root-level ".devcontainer.json" file
When dcwire I call discover_children on the fs-directory resource
Then dcwire the children include a "devcontainer-instance" resource named "devcontainer-default"
@@ -0,0 +1,339 @@
"""Step definitions for devcontainer_autodiscovery_wiring.feature.
Tests that discover_devcontainers() is correctly wired into
GitCheckoutHandler.discover_children() and FsDirectoryHandler.discover_children().
All steps use the ``dcwire`` prefix to avoid Behave AmbiguousStep errors.
"""
from __future__ import annotations
import json
import subprocess
import tempfile
from pathlib import Path
from behave import given, then, when
from cleveragents.domain.models.core.resource import PhysVirt, Resource
from cleveragents.resource.handlers.fs_directory import FsDirectoryHandler
from cleveragents.resource.handlers.git_checkout import GitCheckoutHandler
_VALID_ULID = "01JQDVHN5X5QJKBMZ3AP8Y4G7K"
_DEVCONTAINER_JSON = json.dumps({"name": "Test Dev Container", "image": "ubuntu:22.04"})
# ---------------------------------------------------------------------------
# Helpers
# ---------------------------------------------------------------------------
def _make_git_resource(location: str) -> Resource:
"""Create a minimal git-checkout Resource."""
return Resource(
resource_id=_VALID_ULID,
name="test-repo",
resource_type_name="git-checkout",
classification=PhysVirt.PHYSICAL,
description="Test git checkout resource",
location=location,
)
def _make_fs_resource(location: str) -> Resource:
"""Create a minimal fs-directory Resource."""
return Resource(
resource_id=_VALID_ULID,
name="test-dir",
resource_type_name="fs-directory",
classification=PhysVirt.PHYSICAL,
description="Test filesystem directory resource",
location=location,
)
def _init_git_repo(tmpdir: str, extra_files: dict[str, str] | None = None) -> str:
"""Initialise a bare-minimum git repo with an initial commit."""
subprocess.run(["git", "init"], cwd=tmpdir, capture_output=True, check=True)
subprocess.run(
["git", "config", "user.email", "test@dcwire.dev"],
cwd=tmpdir,
capture_output=True,
check=True,
)
subprocess.run(
["git", "config", "user.name", "DCWire Test"],
cwd=tmpdir,
capture_output=True,
check=True,
)
subprocess.run(
["git", "config", "commit.gpgSign", "false"],
cwd=tmpdir,
capture_output=True,
check=True,
)
# Always create a README so there is at least one tracked file
readme = Path(tmpdir) / "README.md"
readme.write_text("# dcwire test\n")
if extra_files:
for rel_path, file_content in extra_files.items():
full = Path(tmpdir) / rel_path
full.parent.mkdir(parents=True, exist_ok=True)
full.write_text(file_content)
subprocess.run(["git", "add", "."], cwd=tmpdir, capture_output=True, check=True)
subprocess.run(
["git", "commit", "-m", "init"],
cwd=tmpdir,
capture_output=True,
check=True,
)
return tmpdir
def _create_devcontainer_dir(base: str, rel_path: str) -> None:
"""Create a devcontainer.json at the given relative path inside base."""
full = Path(base) / rel_path
full.parent.mkdir(parents=True, exist_ok=True)
full.write_text(_DEVCONTAINER_JSON)
# ---------------------------------------------------------------------------
# GIVEN steps — git-checkout
# ---------------------------------------------------------------------------
@given('dcwire a git repo with a ".devcontainer/devcontainer.json" file')
def step_dcwire_git_with_root_devcontainer(ctx):
tmpdir = tempfile.mkdtemp(prefix="dcwire-git-")
_create_devcontainer_dir(tmpdir, ".devcontainer/devcontainer.json")
_init_git_repo(tmpdir, {".devcontainer/devcontainer.json": _DEVCONTAINER_JSON})
ctx.dcwire_handler = GitCheckoutHandler()
ctx.dcwire_resource = _make_git_resource(tmpdir)
ctx.dcwire_result = None
ctx.dcwire_error = None
ctx.dcwire_last_dc_child = None
@given('dcwire a git repo with a ".devcontainer/api/devcontainer.json" named config')
def step_dcwire_git_with_named_devcontainer(ctx):
tmpdir = tempfile.mkdtemp(prefix="dcwire-git-named-")
_create_devcontainer_dir(tmpdir, ".devcontainer/api/devcontainer.json")
_init_git_repo(tmpdir, {".devcontainer/api/devcontainer.json": _DEVCONTAINER_JSON})
ctx.dcwire_handler = GitCheckoutHandler()
ctx.dcwire_resource = _make_git_resource(tmpdir)
ctx.dcwire_result = None
ctx.dcwire_error = None
ctx.dcwire_last_dc_child = None
@given("dcwire a git repo with no devcontainer configuration")
def step_dcwire_git_no_devcontainer(ctx):
tmpdir = tempfile.mkdtemp(prefix="dcwire-git-nodc-")
_init_git_repo(tmpdir)
ctx.dcwire_handler = GitCheckoutHandler()
ctx.dcwire_resource = _make_git_resource(tmpdir)
ctx.dcwire_result = None
ctx.dcwire_error = None
ctx.dcwire_last_dc_child = None
@given(
'dcwire a git repo with a subdirectory "src" and a ".devcontainer/devcontainer.json" file'
)
def step_dcwire_git_with_src_and_devcontainer(ctx):
tmpdir = tempfile.mkdtemp(prefix="dcwire-git-src-")
_create_devcontainer_dir(tmpdir, ".devcontainer/devcontainer.json")
_init_git_repo(
tmpdir,
{
"src/main.py": "print('hello')\n",
".devcontainer/devcontainer.json": _DEVCONTAINER_JSON,
},
)
ctx.dcwire_handler = GitCheckoutHandler()
ctx.dcwire_resource = _make_git_resource(tmpdir)
ctx.dcwire_result = None
ctx.dcwire_error = None
ctx.dcwire_last_dc_child = None
@given('dcwire a git repo with a root-level ".devcontainer.json" file')
def step_dcwire_git_with_root_level_devcontainer_json(ctx):
tmpdir = tempfile.mkdtemp(prefix="dcwire-git-rootdc-")
_create_devcontainer_dir(tmpdir, ".devcontainer.json")
_init_git_repo(tmpdir, {".devcontainer.json": _DEVCONTAINER_JSON})
ctx.dcwire_handler = GitCheckoutHandler()
ctx.dcwire_resource = _make_git_resource(tmpdir)
ctx.dcwire_result = None
ctx.dcwire_error = None
ctx.dcwire_last_dc_child = None
# ---------------------------------------------------------------------------
# GIVEN steps — fs-directory
# ---------------------------------------------------------------------------
@given('dcwire a filesystem directory with a ".devcontainer/devcontainer.json" file')
def step_dcwire_fs_with_root_devcontainer(ctx):
tmpdir = tempfile.mkdtemp(prefix="dcwire-fs-")
_create_devcontainer_dir(tmpdir, ".devcontainer/devcontainer.json")
ctx.dcwire_handler = FsDirectoryHandler()
ctx.dcwire_resource = _make_fs_resource(tmpdir)
ctx.dcwire_result = None
ctx.dcwire_error = None
ctx.dcwire_last_dc_child = None
@given(
'dcwire a filesystem directory with a ".devcontainer/frontend/devcontainer.json" named config'
)
def step_dcwire_fs_with_named_devcontainer(ctx):
tmpdir = tempfile.mkdtemp(prefix="dcwire-fs-named-")
_create_devcontainer_dir(tmpdir, ".devcontainer/frontend/devcontainer.json")
ctx.dcwire_handler = FsDirectoryHandler()
ctx.dcwire_resource = _make_fs_resource(tmpdir)
ctx.dcwire_result = None
ctx.dcwire_error = None
ctx.dcwire_last_dc_child = None
@given("dcwire a filesystem directory with no devcontainer configuration")
def step_dcwire_fs_no_devcontainer(ctx):
tmpdir = tempfile.mkdtemp(prefix="dcwire-fs-nodc-")
ctx.dcwire_handler = FsDirectoryHandler()
ctx.dcwire_resource = _make_fs_resource(tmpdir)
ctx.dcwire_result = None
ctx.dcwire_error = None
ctx.dcwire_last_dc_child = None
@given(
'dcwire a filesystem directory with a subdirectory "lib" and a ".devcontainer/devcontainer.json" file'
)
def step_dcwire_fs_with_lib_and_devcontainer(ctx):
tmpdir = tempfile.mkdtemp(prefix="dcwire-fs-lib-")
lib_dir = Path(tmpdir) / "lib"
lib_dir.mkdir()
_create_devcontainer_dir(tmpdir, ".devcontainer/devcontainer.json")
ctx.dcwire_handler = FsDirectoryHandler()
ctx.dcwire_resource = _make_fs_resource(tmpdir)
ctx.dcwire_result = None
ctx.dcwire_error = None
ctx.dcwire_last_dc_child = None
@given('dcwire a filesystem directory with a root-level ".devcontainer.json" file')
def step_dcwire_fs_with_root_level_devcontainer_json(ctx):
tmpdir = tempfile.mkdtemp(prefix="dcwire-fs-rootdc-")
_create_devcontainer_dir(tmpdir, ".devcontainer.json")
ctx.dcwire_handler = FsDirectoryHandler()
ctx.dcwire_resource = _make_fs_resource(tmpdir)
ctx.dcwire_result = None
ctx.dcwire_error = None
ctx.dcwire_last_dc_child = None
# ---------------------------------------------------------------------------
# WHEN steps
Outdated
Review

Suggestion: The two WHEN steps (step_dcwire_discover_git and step_dcwire_discover_fs) have identical bodies. Consider extracting a single shared WHEN step pattern like dcwire I call discover_children on the resource to reduce duplication. Not required for merge.

Suggestion: The two WHEN steps (step_dcwire_discover_git and step_dcwire_discover_fs) have identical bodies. Consider extracting a single shared WHEN step pattern like dcwire I call discover_children on the resource to reduce duplication. Not required for merge.
# ---------------------------------------------------------------------------
@when("dcwire I call discover_children on the git-checkout resource")
def step_dcwire_discover_git(ctx):
try:
ctx.dcwire_result = ctx.dcwire_handler.discover_children(
resource=ctx.dcwire_resource
)
except Exception as exc:
ctx.dcwire_error = exc
@when("dcwire I call discover_children on the fs-directory resource")
def step_dcwire_discover_fs(ctx):
try:
ctx.dcwire_result = ctx.dcwire_handler.discover_children(
resource=ctx.dcwire_resource
)
except Exception as exc:
ctx.dcwire_error = exc
# ---------------------------------------------------------------------------
# THEN steps
# ---------------------------------------------------------------------------
@then('dcwire the children include a "devcontainer-instance" resource named "{name}"')
def step_dcwire_has_devcontainer_child(ctx, name):
assert ctx.dcwire_error is None, f"Unexpected error: {ctx.dcwire_error}"
assert ctx.dcwire_result is not None, "Expected a list of children"
dc_children = [
r
for r in ctx.dcwire_result
if r.resource_type_name == "devcontainer-instance" and r.name == name
]
assert len(dc_children) == 1, (
f"Expected exactly one devcontainer-instance named '{name}', "
f"got {[r.name for r in ctx.dcwire_result if r.resource_type_name == 'devcontainer-instance']}"
)
ctx.dcwire_last_dc_child = dc_children[0]
@then('dcwire the children include a "fs-directory" resource named "{name}"')
def step_dcwire_has_fs_child(ctx, name):
assert ctx.dcwire_error is None, f"Unexpected error: {ctx.dcwire_error}"
assert ctx.dcwire_result is not None, "Expected a list of children"
fs_children = [
r
for r in ctx.dcwire_result
if r.resource_type_name == "fs-directory" and r.name == name
]
assert len(fs_children) == 1, (
f"Expected exactly one fs-directory named '{name}', "
f"got {[r.name for r in ctx.dcwire_result if r.resource_type_name == 'fs-directory']}"
)
@then('dcwire the devcontainer child has provisioning_state "{state}"')
def step_dcwire_has_provisioning_state(ctx, state):
assert ctx.dcwire_last_dc_child is not None, "No devcontainer child captured"
props = ctx.dcwire_last_dc_child.properties or {}
actual = props.get("provisioning_state")
assert actual == state, f"Expected provisioning_state='{state}', got '{actual}'"
@then("dcwire the devcontainer child has devcontainer_json_path set")
def step_dcwire_has_json_path(ctx):
assert ctx.dcwire_last_dc_child is not None, "No devcontainer child captured"
props = ctx.dcwire_last_dc_child.properties or {}
path_val = props.get("devcontainer_json_path")
assert path_val, f"Expected devcontainer_json_path to be set, got {path_val!r}"
assert "devcontainer.json" in path_val, (
f"Expected devcontainer_json_path to contain 'devcontainer.json', got {path_val!r}"
)
@then('dcwire the devcontainer child has config_name "{config_name}"')
def step_dcwire_has_config_name(ctx, config_name):
assert ctx.dcwire_last_dc_child is not None, "No devcontainer child captured"
props = ctx.dcwire_last_dc_child.properties or {}
actual = props.get("config_name")
assert actual == config_name, (
f"Expected config_name='{config_name}', got '{actual}'"
)
@then("dcwire no devcontainer-instance children are present")
def step_dcwire_no_devcontainer_children(ctx):
assert ctx.dcwire_error is None, f"Unexpected error: {ctx.dcwire_error}"
assert ctx.dcwire_result is not None, "Expected a list of children"
dc_children = [
r for r in ctx.dcwire_result if r.resource_type_name == "devcontainer-instance"
]
assert len(dc_children) == 0, (
f"Expected no devcontainer-instance children, got {[r.name for r in dc_children]}"
)
@@ -37,6 +37,7 @@ from cleveragents.resource.handlers._base import (
EMPTY_CONTENT_HASH,
BaseResourceHandler,
)
from cleveragents.resource.handlers.discovery import discover_devcontainers
from cleveragents.resource.handlers.protocol import (
CheckpointResult,
Content,
@@ -233,10 +234,13 @@ class FsDirectoryHandler(BaseResourceHandler):
)
def discover_children(self, *, resource: Resource) -> list[Resource]:
"""Discover subdirectories as child resources.
"""Discover subdirectories and devcontainer instances as child resources.
Each immediate subdirectory becomes a child ``fs-directory``
resource.
resource. Additionally, any ``.devcontainer/`` configurations
found at the resource location are registered as
``devcontainer-instance`` child resources with
``provisioning_state: discovered``.
Args:
resource: The parent fs-directory resource.
@@ -261,6 +265,28 @@ class FsDirectoryHandler(BaseResourceHandler):
)
children.append(child)
# Wire devcontainer auto-discovery (issue #4740)
Outdated
Review

Suggestion: Same deduplication question as in git_checkout.py (see inline comment there). If a shared helper is added later, the devcontainer wiring here could become:

dc_results = discover_and_register_devcontainers(self, resource)

Non-blocking — acceptable to leave as-is for now.

Suggestion: Same deduplication question as in git_checkout.py (see inline comment there). If a shared helper is added later, the devcontainer wiring here could become: ``` dc_results = discover_and_register_devcontainers(self, resource) ``` Non-blocking — acceptable to leave as-is for now.
dc_results = discover_devcontainers(location, "fs-directory")
for dc_result in dc_results:
config_name = dc_result.config_name or "default"
dc_child = Resource(
resource_id=self._derive_child_id(
resource.resource_id, f"devcontainer-{config_name}"
),
name=f"devcontainer-{config_name}",
resource_type_name="devcontainer-instance",
classification=resource.classification,
description=f"Devcontainer at {dc_result.config_path}",
location=location,
parents=[resource.resource_id],
properties={
"devcontainer_json_path": str(dc_result.config_path),
"config_name": config_name,
"provisioning_state": "discovered",
},
)
children.append(dc_child)
return children
# -- Checkpoint and rollback (issue #836) ------------------------------
@@ -6,17 +6,18 @@ strategy (with fallback to ``copy_on_write``).
Content CRUD operations (issue #827):
- ``read`` ``git show HEAD:<path>``
- ``write`` atomic file write inside the checkout
- ``delete`` ``os.remove`` + ``git rm --cached``
- ``list_children`` ``git ls-tree -r --name-only HEAD``
- ``diff`` ``git diff --no-index``
- ``discover_children`` ``git ls-tree --name-only HEAD``
- ``read`` -- ``git show HEAD:<path>``
Outdated
Review

Suggestion: Consider using single backticks for inline code in the module-level docstring (lines 9-14). They use double backticks which is non-standard in Python reST where inline code uses escaped single ticks. Minor style nit only.

Suggestion: Consider using single backticks for inline code in the module-level docstring (lines 9-14). They use double backticks which is non-standard in Python reST where ``inline code`` uses escaped single ticks. Minor style nit only.
- ``write`` -- atomic file write inside the checkout
- ``delete`` -- ``os.remove`` + ``git rm --cached``
- ``list_children`` -- ``git ls-tree -r --name-only HEAD``
- ``diff`` -- ``git diff --no-index``
- ``discover_children`` -- ``git ls-tree --name-only HEAD`` + devcontainer discovery
Based on:
- implementation_plan.md group M1.resource-handlers (L2254-L2271)
- Built-in type definition in resource_registry_service.py L62-98
- Issue #827 ResourceHandler CRUD and discovery methods
- Issue #827 -- ResourceHandler CRUD and discovery methods
- Issue #4740 -- Wire discover_devcontainers() into git-checkout handler
"""
from __future__ import annotations
@@ -36,6 +37,7 @@ from cleveragents.resource.handlers._base import (
EMPTY_CONTENT_HASH,
BaseResourceHandler,
)
from cleveragents.resource.handlers.discovery import discover_devcontainers
Outdated
Review

Suggestion: The import from cleveragents.resource.handlers.discovery import discover_devcontainers is placed between _base and protocol imports. Consider alphabetically ordering this import for consistency with module-level sorting conventions - minor style nit.

Suggestion: The import from cleveragents.resource.handlers.discovery import discover_devcontainers is placed between _base and protocol imports. Consider alphabetically ordering this import for consistency with module-level sorting conventions - minor style nit.
from cleveragents.resource.handlers.protocol import (
CheckpointResult,
Content,
@@ -303,17 +305,20 @@ class GitCheckoutHandler(BaseResourceHandler):
)
def discover_children(self, *, resource: Resource) -> list[Resource]:
"""Discover child resources via ``git ls-tree``.
"""Discover child resources via ``git ls-tree`` and devcontainer scan.
Each top-level directory in the repo becomes a child resource
of type ``fs-directory``.
of type ``fs-directory``. Additionally, any ``.devcontainer/``
configurations found at the resource location are registered as
``devcontainer-instance`` child resources with
``provisioning_state: discovered``.
Args:
resource: The parent git-checkout resource.
Returns:
List of child :class:`Resource` objects for top-level
directories.
directories and discovered devcontainer instances.
"""
location = self._require_location(resource)
@@ -346,6 +351,28 @@ class GitCheckoutHandler(BaseResourceHandler):
)
children.append(child)
# Wire devcontainer auto-discovery (issue #4740)
Outdated
Review

Question: The devcontainer wiring block here (lines 354-374) is nearly identical to the same block in fs_directory.py (lines 268-288). Both create a Resource with the same structure, differ only by the string literal passed to discover_devcontainers(). Consider whether this deduplication is worth extracting into a shared helper method for future handler types, or leave as-is given it matches existing patterns. This is non-blocking — acceptable to leave as-is for this PR.

Question: The devcontainer wiring block here (lines 354-374) is nearly identical to the same block in `fs_directory.py` (lines 268-288). Both create a Resource with the same structure, differ only by the string literal passed to `discover_devcontainers()`. Consider whether this deduplication is worth extracting into a shared helper method for future handler types, or leave as-is given it matches existing patterns. This is non-blocking — acceptable to leave as-is for this PR.
dc_results = discover_devcontainers(location, "git-checkout")
for dc_result in dc_results:
config_name = dc_result.config_name or "default"
dc_child = Resource(
resource_id=self._derive_child_id(
resource.resource_id, f"devcontainer-{config_name}"
),
name=f"devcontainer-{config_name}",
resource_type_name="devcontainer-instance",
classification=resource.classification,
description=f"Devcontainer at {dc_result.config_path}",
location=location,
parents=[resource.resource_id],
properties={
"devcontainer_json_path": str(dc_result.config_path),
"config_name": config_name,
"provisioning_state": "discovered",
},
)
children.append(dc_child)
return children
# -- Checkpoint and rollback (issue #836) ------------------------------