Commit Graph

5 Commits

Author SHA1 Message Date
HAL9000 afe89964df chore(tests): remove stale tdd_expected_fail tags from tests now passing on master
CI / push-validation (pull_request) Successful in 26s
CI / helm (pull_request) Successful in 50s
CI / lint (pull_request) Successful in 52s
CI / build (pull_request) Successful in 51s
CI / quality (pull_request) Successful in 52s
CI / security (pull_request) Successful in 1m15s
CI / typecheck (pull_request) Successful in 1m35s
CI / unit_tests (pull_request) Successful in 5m25s
CI / docker (pull_request) Successful in 1m34s
CI / integration_tests (pull_request) Failing after 10m26s
CI / coverage (pull_request) Failing after 18m51s
CI / status-check (pull_request) Has been cancelled
The tdd_expected_fail listener (robot/tdd_expected_fail_listener.py and
features/environment.py) inverts test results: a test tagged
@tdd_expected_fail that PASSES is forced to FAIL with the message
"Bug appears to be fixed. Remove the tdd_expected_fail tag…".

After rebasing onto master, 22 scenarios across 11 files were triggering
that forced-failure path because the underlying bugs (#4199, #4201,
#4202, #4203, #4205, #4206, #4243, #4252, #4301, #4303, #4304) have
been fixed on master but the @tdd_expected_fail tags were never removed.
This commit removes the stale tag (keeping @tdd_issue and
@tdd_issue_<N> for traceability, per the master-side pattern in
robot/tdd_skill_add_regression.robot:9 "tag removed after bug fix").

Files touched (lines: where the stale tag was):
  features/plan_cli_spec_alignment.feature (121, 128, 136)
  robot/a2a_facade.robot (40)
  robot/actor_cli_show.robot (13, 30)
  robot/actor_configuration.robot (8)
  robot/actor_context_export_import.robot (20, 71, 92)
  robot/cli_extensions.robot (61)
  robot/cli_formats.robot (14, 23, 30, 48, 57, 85)
  robot/cli_lifecycle_e2e.robot (77, 87)
  robot/config_project_scope.robot (37)
  robot/config_resolution.robot (13)
  robot/container_tool_exec.robot (137)

Verified locally: targeted unit_tests run on
features/plan_cli_spec_alignment.feature now passes 20/20 (was 17/20
with 3 scenarios forced-failed by the inversion).

ISSUES CLOSED: #3677
2026-06-03 03:22:59 -04:00
freemo 7fb3fc76c8 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-06-03 03:22:59 -04:00
freemo 8ea00f5185 fix: restore CI quality tests to passing state (#4175)
CI / unit_tests (push) Has been cancelled
CI / benchmark-publish (push) Has been cancelled
CI / lint (push) Has been cancelled
CI / typecheck (push) Has been cancelled
CI / security (push) Has been cancelled
CI / quality (push) Has been cancelled
CI / integration_tests (push) Has been cancelled
CI / e2e_tests (push) Has been cancelled
CI / coverage (push) Has been cancelled
CI / benchmark-regression (push) Has been cancelled
CI / build (push) Has been cancelled
CI / push-validation (push) Has been cancelled
CI / status-check (push) Has been cancelled
CI / docker (push) Has been cancelled
CI / helm (push) Has been cancelled
Co-authored-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me>
Co-committed-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me>
2026-04-08 11:02:14 +00:00
brent.edwards 3eecb79003 test(plan): TDD failing tests for checkpoint real rollback (bug #822)
CI / lint (pull_request) Successful in 16s
CI / quality (pull_request) Successful in 46s
CI / typecheck (pull_request) Successful in 56s
CI / security (pull_request) Successful in 56s
CI / benchmark-publish (pull_request) Has been skipped
CI / build (pull_request) Successful in 29s
CI / e2e_tests (pull_request) Successful in 2m7s
CI / unit_tests (pull_request) Successful in 3m8s
CI / docker (pull_request) Successful in 9s
CI / integration_tests (pull_request) Successful in 3m57s
CI / coverage (pull_request) Successful in 7m48s
CI / benchmark-regression (pull_request) Successful in 40m48s
TDD expected-fail tests proving bug #822 exists:
CheckpointService.rollback_to_checkpoint() returns a successful
RollbackResult but does not execute git reset --hard. Files modified
after the checkpoint remain unchanged after rollback.

Also fixes Robot Framework timeout robustness across the entire test
suite: all Run Process calls now use on_timeout=kill (prevents
SIGTERM-induced -15 exit codes under CI load) and timeouts increased
to 120s (prevents premature kills during heavy parallel execution).

ISSUES CLOSED: #839
2026-03-16 01:45:50 +00:00
CoreRasurae 7ac3f1352c feat(devcontainer): add container-aware tool execution and I/O forwarding
CI / benchmark-publish (pull_request) Has been skipped
CI / build (pull_request) Successful in 17s
CI / quality (pull_request) Successful in 20s
CI / lint (pull_request) Successful in 26s
CI / security (pull_request) Successful in 37s
CI / typecheck (pull_request) Successful in 57s
CI / unit_tests (pull_request) Successful in 3m14s
CI / integration_tests (pull_request) Successful in 3m33s
CI / docker (pull_request) Successful in 39s
CI / coverage (pull_request) Successful in 5m29s
CI / lint (push) Successful in 16s
CI / quality (push) Successful in 16s
CI / build (push) Successful in 16s
CI / security (push) Successful in 35s
CI / typecheck (push) Successful in 47s
CI / benchmark-regression (push) Has been skipped
CI / unit_tests (push) Successful in 3m1s
CI / integration_tests (push) Successful in 3m31s
CI / docker (push) Successful in 52s
CI / coverage (push) Successful in 5m47s
CI / benchmark-publish (push) Successful in 19m3s
CI / benchmark-regression (pull_request) Successful in 35m19s
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