Commit Graph

4 Commits

Author SHA1 Message Date
freemo a321fb3b37 fix(plan-lifecycle): add rollback_plan method to PlanLifecycleService
- What was implemented
  - Added PLAN_ROLLED_BACK event type to the EventType enum at src/cleveragents/infrastructure/events/types.py to properly represent successful rollbacks in the domain model.
  - Implemented rollback_plan(plan_id: str, checkpoint_id: str) -> RollbackResult in PlanLifecycleService (src/cleveragents/application/services/plan_lifecycle_service.py) with:
    - Plan state validation: rejects rollback when the plan is in terminal APPLIED or CANCELLED states.
    - Delegation to CheckpointService.selective_rollback() to perform the actual rollback logic and obtain a RollbackResult.
    - Emission of PLAN_ROLLED_BACK as a domain event to reflect the completed rollback.
    - checkpoint_service is accepted as an optional constructor parameter; if not provided, a PlanError is raised to preserve backward compatibility.
  - Updated CLI behavior in src/cleveragents/cli/commands/plan.py so agents plan rollback routes through PlanLifecycleService.rollback_plan() rather than calling CheckpointService.selective_rollback() directly.
  - Updated PlanLifecycleService module docstring to include rollback_plan in the documented API.
  - Added Behave feature file features/plan_lifecycle_rollback.feature with 11 scenarios covering state validation, domain events, and delegation.
  - Added step implementations in features/steps/plan_lifecycle_rollback_steps.py to support the new scenarios.

- Key design decisions
  - rollback_plan returns RollbackResult (the same result type produced by CheckpointService.selective_rollback) so the CLI can display rollback details consistently.
  - Terminal states APPLIED and CANCELLED are disallowed for rollback to prevent inconsistent or invalid state transitions.
  - checkpoint_service is optional in the PlanLifecycleService constructor; when omitted (None), a PlanError is raised to retain backward compatibility while signaling explicit dependency requirements.
  - CLI UI remains powered by CheckpointService for metadata enrichment (e.g., confirmation prompts), but the actual rollback action is performed via PlanLifecycleService to ensure proper domain workflow and event emission.

- Technical implications
  - All rollback logic now flows through the domain service layer (PlanLifecycleService) to preserve invariants and emit domain events, rather than allowing ad-hoc UI routes to bypass service validation.
  - The UI can still retrieve checkpoint metadata for user confirmation, but the operation that modifies state uses the new rollback_plan pathway.
  - Tests and behavior coverage were expanded via the new Behave feature and step implementations to validate state handling, events, and delegation.

- Affected modules/components
  - src/cleveragents/infrastructure/events/types.py
  - src/cleveragents/application/services/plan_lifecycle_service.py
  - src/cleveragents/cli/commands/plan.py
  - PlanLifecycleService module docstring
  - features/plan_lifecycle_rollback.feature
  - features/steps/plan_lifecycle_rollback_steps.py

ISSUES CLOSED: #3677
2026-04-06 13:15:57 +00:00
freemo a05f4c69a7 fix(resources): support multiple named devcontainer configurations in auto-discovery
Extend DevcontainerDiscoveryResult and discover_devcontainers() to scan
.devcontainer/<name>/devcontainer.json patterns (one subdirectory level)
in addition to the existing fixed paths. Each named configuration produces
a distinct result with config_name set to the subdirectory name (e.g.
'api', 'frontend'). Root-level configs retain config_name=None.

- Replace _SCAN_PATHS with _FIXED_SCAN_PATHS for root-level configs
- Add glob-based scan of .devcontainer/<name>/devcontainer.json
- Add config_name: str | None attribute to DevcontainerDiscoveryResult
- Validate config_name (must be non-empty str or None)
- Add 8 new Behave scenarios covering named, multiple, mixed, empty cases
- Add 4 new Robot Framework integration tests for named config discovery
- All existing tests continue to pass (no regression)

ISSUES CLOSED: #2615
2026-04-05 18:08:56 +00:00
CoreRasurae 7ac3f1352c feat(devcontainer): add container-aware tool execution and I/O forwarding
Implement ContainerToolExecutor for delegating tool invocations to
devcontainer environments with full I/O forwarding. Add PathMapper for
bidirectional host/container path translation. Wire container routing
into ToolRunner with graceful fallback when no executor is configured.
Add container_metadata field to ToolInvocation for tracking execution
context.

New modules:
- tool/container_executor.py: ContainerToolExecutor, ContainerConfig,
  ContainerMetadata, ContainerExecutionError, ContainerTimeoutError
- tool/path_mapper.py: PathMapper with host_to_container/container_to_host

Modified:
- tool/runner.py: container execution routing via ExecutionEnvironment
- domain/models/core/change.py: container_metadata on ToolInvocation
- tool/__init__.py: new public exports

Review fixes applied:
- Add Alembic migration m6_004 for container_metadata_json column
- Enforce _MAX_OUTPUT_BYTES (50 MiB) truncation in _run_command()
- Fix path traversal in sync_results_to_host (Path.is_relative_to)
- Allow spaces in _looks_like_path() for valid filesystem paths
- Preserve negative exit codes from signal kills in metadata
- Add default=str to json.dumps(invocation.arguments) safety net
- Log warnings when path mapping recursion depth exceeded
- Warn when devcontainer binary not found on PATH
- Use default allow_nan for host-path JSON validation in runner.py
  (only container path requires RFC 7159 strict mode)
- Reject URL-like patterns in _looks_like_path() to avoid false
  positives on API routes, protocol-relative URIs, and query strings
- Add extract_container_metadata() static helper on
  ContainerToolExecutor as bridge for ToolInvocation wiring
- Use raw_stdout bytes in sync_results_to_host to prevent
  binary file corruption from text-mode decode/re-encode
- Apply posixpath.normpath() in workspace_folder validator and
  reject path components containing '..'
- Check result.timed_out in sync_results_to_host and raise
  ContainerTimeoutError instead of always raising ContainerExecutionError
- Detect overlapping host_root/container_root in PathMapper and
  raise ValueError to prevent corrupt bidirectional mappings
- Wrap host-side I/O in sync_results_to_host with try/except
  OSError to produce ContainerExecutionError on write failure
- Enforce int(timeout) in _build_exec_command to prevent shell
  injection via malicious objects with __str__ methods
- Change ToolResult validator from 'not self.error' to
  'self.error is None' so empty-string errors are accepted
- Iterate required list directly in ToolRunner schema validation
  to detect fields listed in required but absent from properties

Closes #515
2026-03-12 10:31:37 +00:00
freemo 279c6112ae feat(resource): add devcontainer resource type and auto-discovery handler
Added three new built-in resource types: container-instance,
devcontainer-instance, and devcontainer-file. The devcontainer-instance
type inherits from container-instance per ADR-042 and is auto-discovered
when git-checkout or fs-directory resources contain .devcontainer/
directories per ADR-043.

Implementation includes:
- DevcontainerHandler extending BaseResourceHandler with snapshot strategy
- Auto-discovery module scanning .devcontainer/devcontainer.json and
  root .devcontainer.json with JSON validation
- CLI support for devcontainer-instance and container-instance via
  agents resource add with --path and --image flags
- Behave feature with 22 scenarios covering manual registration,
  auto-discovery, invalid JSON handling, and protocol conformance
- Robot integration tests with 10 test cases for CLI round-trip and
  DAG hierarchy validation
- ASV benchmarks measuring discovery throughput with varying subdirectory
  counts, handler resolver cache performance, and result construction
- Reference documentation at docs/reference/devcontainer_resources.md

ISSUES CLOSED: #511
2026-03-02 22:09:37 -05:00