b46a0e6102
CI / status-check (push) Blocked by required conditions
CI / build (push) Successful in 18s
CI / helm (push) Successful in 31s
CI / lint (push) Successful in 3m29s
CI / typecheck (push) Successful in 3m56s
CI / benchmark-regression (push) Has been skipped
CI / quality (push) Successful in 3m41s
CI / unit_tests (push) Successful in 4m11s
CI / integration_tests (push) Failing after 4m12s
CI / security (push) Successful in 4m38s
CI / docker (push) Successful in 1m32s
CI / coverage (push) Successful in 11m33s
CI / e2e_tests (push) Failing after 23m17s
CI / benchmark-publish (push) Failing after 38m50s
## Summary Implements the full revert-mode re-execution pipeline from the specification (§ Correction Flow, Revert Mode), enabling `agents plan correct --mode=revert` to truly re-execute from the targeted decision point rather than only logically marking decisions as superseded. ### Changes - **Resource rollback**: `CorrectionService.execute_revert` now delegates to `CheckpointService.rollback_to_checkpoint` when a decision-aligned checkpoint exists, performing real `git reset --hard` in the sandbox. Gracefully degrades when checkpointing is unavailable. - **Reasoning rollback**: Extracts `actor_state_ref` from the target decision's `ContextSnapshot` for downstream LangGraph actor restoration. - **Guidance injection**: Generates a `user_intervention` decision ID for callers to record correction guidance in the decision tree. - **Phase transition**: Signals that the plan should re-enter Strategize via `phase_transition_target="strategize"` on the result. - **Model extension**: Added `checkpoint_restored`, `actor_state_ref`, `user_intervention_decision_id`, and `phase_transition_target` fields to `CorrectionResult` with backward-compatible defaults. - **Dispatch update**: `execute_correction` now accepts and forwards a `decisions` parameter. ### Design Approach Uses a signal-based architecture: rather than having `CorrectionService` directly manipulate plan state or LangGraph actors, the enhanced `execute_revert` returns signal fields on `CorrectionResult` that downstream consumers use for the full pipeline. This preserves separation of concerns. ### Tests - **Behave**: 16 new BDD scenarios in `features/revert_re_execution.feature` - **Robot**: 7 integration tests in `robot/revert_re_execution.robot` - **Coverage**: 98% (above 97% threshold) ### Quality Gates | Gate | Status | |------|--------| | `nox -s lint` | Pass | | `nox -s typecheck` | Pass (0 errors) | | `nox -s unit_tests` | Pass (12376 scenarios, 0 failed) | | `nox -s integration_tests` | Pass (1631 passed, 3 pre-existing failures #647) | | `nox -s e2e_tests` | Pass (37 passed) | | `nox -s coverage_report` | Pass (98%) | Closes #844 Reviewed-on: #1153 Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com> Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com>
385 lines
14 KiB
Python
385 lines
14 KiB
Python
"""Step definitions for revert-mode re-execution from decision point.
|
|
|
|
Covers checkpoint restoration, actor state recovery, phase transition
|
|
signalling, user intervention decision creation, and subtree selectivity.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import re
|
|
from typing import Any
|
|
from unittest.mock import MagicMock
|
|
|
|
from behave import given, then, when # type: ignore[import-untyped]
|
|
from behave.runner import Context # type: ignore[import-untyped]
|
|
|
|
from cleveragents.application.services.checkpoint_service import CheckpointService
|
|
from cleveragents.application.services.correction_service import CorrectionService
|
|
from cleveragents.domain.models.core.checkpoint import Checkpoint, CheckpointMetadata
|
|
from cleveragents.domain.models.core.correction import (
|
|
CorrectionMode,
|
|
CorrectionResult,
|
|
CorrectionStatus,
|
|
)
|
|
from cleveragents.domain.models.core.decision import (
|
|
ContextSnapshot,
|
|
Decision,
|
|
DecisionType,
|
|
)
|
|
from cleveragents.domain.models.core.plan import ULID_PATTERN
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Helpers
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
_ULID_RE = re.compile(ULID_PATTERN)
|
|
|
|
|
|
def _ensure_ulid(value: str) -> str:
|
|
"""Return ``value`` when already ULID-like, otherwise generate one."""
|
|
if _ULID_RE.match(value):
|
|
return value
|
|
from ulid import ULID
|
|
|
|
return str(ULID())
|
|
|
|
|
|
def _make_decision(
|
|
decision_id: str,
|
|
plan_id: str,
|
|
actor_state_ref: str = "",
|
|
) -> Decision:
|
|
"""Create a minimal Decision for testing actor state extraction."""
|
|
from ulid import ULID
|
|
|
|
return Decision(
|
|
decision_id=str(ULID()),
|
|
plan_id=str(ULID()),
|
|
sequence_number=0,
|
|
decision_type=DecisionType.STRATEGY_CHOICE,
|
|
question="Test question?",
|
|
chosen_option="Option A",
|
|
context_snapshot=ContextSnapshot(actor_state_ref=actor_state_ref),
|
|
)
|
|
|
|
|
|
def _make_checkpoint(
|
|
plan_id: str,
|
|
decision_id: str,
|
|
sandbox_ref: str,
|
|
) -> Checkpoint:
|
|
"""Create a Checkpoint aligned to a decision."""
|
|
from ulid import ULID
|
|
|
|
return Checkpoint(
|
|
checkpoint_id=str(ULID()),
|
|
plan_id=_ensure_ulid(plan_id),
|
|
sandbox_ref=sandbox_ref,
|
|
decision_id=decision_id,
|
|
checkpoint_type="pre_write",
|
|
metadata=CheckpointMetadata(reason="decision-aligned"),
|
|
)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Given steps
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("revexec a correction service with a mock checkpoint service")
|
|
def step_given_correction_service_with_checkpoint(context: Context) -> None:
|
|
mock_cp_svc = MagicMock(spec=CheckpointService)
|
|
mock_cp_svc.list_checkpoints.return_value = []
|
|
mock_cp_svc.rollback_to_checkpoint.return_value = MagicMock(
|
|
restored_files_count=3,
|
|
changed_paths=["a.py", "b.py", "c.py"],
|
|
from_checkpoint_id="cp1",
|
|
)
|
|
context.revexec_checkpoint_service = mock_cp_svc
|
|
context.revexec_service = CorrectionService(
|
|
checkpoint_service=mock_cp_svc,
|
|
)
|
|
context.revexec_decisions = {}
|
|
context.revexec_tree = {}
|
|
|
|
|
|
@given("revexec a correction service without checkpoint service")
|
|
def step_given_correction_service_no_checkpoint(context: Context) -> None:
|
|
context.revexec_service = CorrectionService()
|
|
context.revexec_decisions = {}
|
|
context.revexec_tree = {}
|
|
|
|
|
|
@given("revexec a correction service with a failing checkpoint list")
|
|
def step_given_correction_service_failing_list(context: Context) -> None:
|
|
mock_cp_svc = MagicMock(spec=CheckpointService)
|
|
mock_cp_svc.list_checkpoints.side_effect = RuntimeError("list failed")
|
|
context.revexec_checkpoint_service = mock_cp_svc
|
|
context.revexec_service = CorrectionService(
|
|
checkpoint_service=mock_cp_svc,
|
|
)
|
|
context.revexec_decisions = {}
|
|
context.revexec_tree = {}
|
|
|
|
|
|
@given("revexec a correction service with a failing checkpoint rollback")
|
|
def step_given_correction_service_failing_rollback(context: Context) -> None:
|
|
mock_cp_svc = MagicMock(spec=CheckpointService)
|
|
cp = _make_checkpoint("P1", "D1", "ref-fail")
|
|
mock_cp_svc.list_checkpoints.return_value = [cp]
|
|
mock_cp_svc.rollback_to_checkpoint.side_effect = RuntimeError("rollback failed")
|
|
context.revexec_checkpoint_service = mock_cp_svc
|
|
context.revexec_service = CorrectionService(
|
|
checkpoint_service=mock_cp_svc,
|
|
)
|
|
context.revexec_decisions = {}
|
|
context.revexec_tree = {}
|
|
|
|
|
|
@given(
|
|
'revexec a checkpoint exists for plan "{plan_id}" decision "{decision_id}" with ref "{ref}"'
|
|
)
|
|
def step_given_checkpoint_exists(
|
|
context: Context, plan_id: str, decision_id: str, ref: str
|
|
) -> None:
|
|
cp = _make_checkpoint(plan_id, decision_id, ref)
|
|
context.revexec_checkpoint_service.list_checkpoints.return_value = [cp]
|
|
|
|
|
|
@given('revexec no checkpoint exists for plan "{plan_id}" decision "{decision_id}"')
|
|
def step_given_no_checkpoint(context: Context, plan_id: str, decision_id: str) -> None:
|
|
context.revexec_checkpoint_service.list_checkpoints.return_value = []
|
|
|
|
|
|
@given('revexec a decision "{decision_id}" with actor_state_ref "{ref}"')
|
|
def step_given_decision_with_actor_state(
|
|
context: Context, decision_id: str, ref: str
|
|
) -> None:
|
|
decision = _make_decision(decision_id, "P1", actor_state_ref=ref)
|
|
context.revexec_decisions[decision_id] = decision
|
|
|
|
|
|
@given(
|
|
'revexec a correction request targeting decision "{decision_id}" in plan "{plan_id}" for revert'
|
|
)
|
|
def step_given_correction_request(
|
|
context: Context, decision_id: str, plan_id: str
|
|
) -> None:
|
|
svc: CorrectionService = context.revexec_service
|
|
context.revexec_request = svc.request_correction(
|
|
plan_id,
|
|
decision_id,
|
|
CorrectionMode.REVERT,
|
|
)
|
|
|
|
|
|
@given(
|
|
'revexec a correction request targeting decision "{decision_id}" in plan "{plan_id}" for revert with decisions'
|
|
)
|
|
def step_given_correction_request_with_decisions(
|
|
context: Context, decision_id: str, plan_id: str
|
|
) -> None:
|
|
svc: CorrectionService = context.revexec_service
|
|
context.revexec_request = svc.request_correction(
|
|
plan_id,
|
|
decision_id,
|
|
CorrectionMode.REVERT,
|
|
)
|
|
|
|
|
|
@given(
|
|
'revexec a correction request targeting decision "{decision_id}" in plan "{plan_id}" for revert with guidance "{guidance}"'
|
|
)
|
|
def step_given_correction_request_guidance(
|
|
context: Context, decision_id: str, plan_id: str, guidance: str
|
|
) -> None:
|
|
svc: CorrectionService = context.revexec_service
|
|
context.revexec_request = svc.request_correction(
|
|
plan_id,
|
|
decision_id,
|
|
CorrectionMode.REVERT,
|
|
guidance=guidance,
|
|
)
|
|
|
|
|
|
@given('revexec a decision tree where "{parent}" has children "{children}"')
|
|
def step_given_decision_tree_simple(
|
|
context: Context, parent: str, children: str
|
|
) -> None:
|
|
child_list = [c.strip() for c in children.split(",")]
|
|
context.revexec_tree[parent] = child_list
|
|
|
|
|
|
@given(
|
|
'revexec a two-level tree "{parent}" children "{children}" then "{parent2}" children "{children2}"'
|
|
)
|
|
def step_given_decision_tree_two_level(
|
|
context: Context, parent: str, children: str, parent2: str, children2: str
|
|
) -> None:
|
|
context.revexec_tree[parent] = [c.strip() for c in children.split(",")]
|
|
context.revexec_tree[parent2] = [c.strip() for c in children2.split(",")]
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# When steps
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@when("revexec I execute the revert correction")
|
|
def step_when_execute_revert(context: Context) -> None:
|
|
svc: CorrectionService = context.revexec_service
|
|
tree: dict[str, list[str]] = getattr(context, "revexec_tree", {})
|
|
context.revexec_result = svc.execute_revert(
|
|
context.revexec_request.correction_id,
|
|
decision_tree=tree if tree else None,
|
|
)
|
|
|
|
|
|
@when("revexec I execute the revert correction with decisions")
|
|
def step_when_execute_revert_with_decisions(context: Context) -> None:
|
|
svc: CorrectionService = context.revexec_service
|
|
context.revexec_result = svc.execute_revert(
|
|
context.revexec_request.correction_id,
|
|
decisions=context.revexec_decisions,
|
|
)
|
|
|
|
|
|
@when("revexec I execute the revert correction with decisions and tree")
|
|
def step_when_execute_revert_full(context: Context) -> None:
|
|
svc: CorrectionService = context.revexec_service
|
|
context.revexec_result = svc.execute_revert(
|
|
context.revexec_request.correction_id,
|
|
decision_tree=context.revexec_tree,
|
|
decisions=context.revexec_decisions,
|
|
)
|
|
|
|
|
|
@when("revexec I execute the revert correction with tree")
|
|
def step_when_execute_revert_with_tree(context: Context) -> None:
|
|
svc: CorrectionService = context.revexec_service
|
|
context.revexec_result = svc.execute_revert(
|
|
context.revexec_request.correction_id,
|
|
decision_tree=context.revexec_tree,
|
|
)
|
|
|
|
|
|
@when("revexec I dispatch execute_correction with decisions")
|
|
def step_when_dispatch_with_decisions(context: Context) -> None:
|
|
svc: CorrectionService = context.revexec_service
|
|
context.revexec_result = svc.execute_correction(
|
|
context.revexec_request.correction_id,
|
|
decisions=context.revexec_decisions,
|
|
)
|
|
|
|
|
|
@when("revexec I create a CorrectionResult with only correction_id and status")
|
|
def step_when_create_default_result(context: Context) -> None:
|
|
context.revexec_result = CorrectionResult(
|
|
correction_id="test-corr-1",
|
|
status=CorrectionStatus.APPLIED,
|
|
)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Then steps
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@then("revexec the result checkpoint_restored should be {expected}")
|
|
def step_then_checkpoint_restored(context: Context, expected: str) -> None:
|
|
result: CorrectionResult = context.revexec_result
|
|
expected_bool = expected.lower() == "true"
|
|
assert result.checkpoint_restored == expected_bool, (
|
|
f"Expected checkpoint_restored={expected_bool}, "
|
|
f"got {result.checkpoint_restored}"
|
|
)
|
|
|
|
|
|
@then('revexec the result status should be "{expected}"')
|
|
def step_then_result_status(context: Context, expected: str) -> None:
|
|
result: CorrectionResult = context.revexec_result
|
|
assert str(result.status) == expected, (
|
|
f"Expected status={expected}, got {result.status}"
|
|
)
|
|
|
|
|
|
@then('revexec the result actor_state_ref should be "{expected}"')
|
|
def step_then_actor_state_ref(context: Context, expected: str) -> None:
|
|
result: CorrectionResult = context.revexec_result
|
|
assert result.actor_state_ref == expected, (
|
|
f"Expected actor_state_ref={expected!r}, got {result.actor_state_ref!r}"
|
|
)
|
|
|
|
|
|
@then('revexec the result actor_state_ref should be ""')
|
|
def step_then_actor_state_ref_empty(context: Context) -> None:
|
|
result: CorrectionResult = context.revexec_result
|
|
assert result.actor_state_ref == "", (
|
|
f"Expected actor_state_ref to be empty, got {result.actor_state_ref!r}"
|
|
)
|
|
|
|
|
|
@then('revexec the result phase_transition_target should be "{expected}"')
|
|
def step_then_phase_transition(context: Context, expected: str) -> None:
|
|
result: CorrectionResult = context.revexec_result
|
|
assert result.phase_transition_target == expected, (
|
|
f"Expected phase_transition_target={expected!r}, "
|
|
f"got {result.phase_transition_target!r}"
|
|
)
|
|
|
|
|
|
@then('revexec the result phase_transition_target should be ""')
|
|
def step_then_phase_transition_empty(context: Context) -> None:
|
|
result: CorrectionResult = context.revexec_result
|
|
assert result.phase_transition_target == "", (
|
|
"Expected phase_transition_target to be empty, "
|
|
f"got {result.phase_transition_target!r}"
|
|
)
|
|
|
|
|
|
@then("revexec the result user_intervention_decision_id should not be empty")
|
|
def step_then_intervention_not_empty(context: Context) -> None:
|
|
result: CorrectionResult = context.revexec_result
|
|
assert result.user_intervention_decision_id, (
|
|
"Expected non-empty user_intervention_decision_id"
|
|
)
|
|
|
|
|
|
@then('revexec the result user_intervention_decision_id should be ""')
|
|
def step_then_intervention_empty(context: Context) -> None:
|
|
result: CorrectionResult = context.revexec_result
|
|
assert result.user_intervention_decision_id == "", (
|
|
f"Expected empty user_intervention_decision_id, "
|
|
f"got {result.user_intervention_decision_id!r}"
|
|
)
|
|
|
|
|
|
@then('revexec the reverted decisions should contain "{decision_id}"')
|
|
def step_then_reverted_contains(context: Context, decision_id: str) -> None:
|
|
result: CorrectionResult = context.revexec_result
|
|
assert decision_id in result.reverted_decisions, (
|
|
f"Expected {decision_id} in reverted_decisions, got {result.reverted_decisions}"
|
|
)
|
|
|
|
|
|
@then('revexec the reverted decisions should not contain "{decision_id}"')
|
|
def step_then_reverted_not_contains(context: Context, decision_id: str) -> None:
|
|
result: CorrectionResult = context.revexec_result
|
|
assert decision_id not in result.reverted_decisions, (
|
|
f"Expected {decision_id} NOT in reverted_decisions, "
|
|
f"got {result.reverted_decisions}"
|
|
)
|
|
|
|
|
|
@then("revexec the attempt details should contain phase_transition_target")
|
|
def step_then_attempt_has_phase(context: Context) -> None:
|
|
svc: CorrectionService = context.revexec_service
|
|
attempts = svc.list_attempts(context.revexec_request.correction_id)
|
|
assert len(attempts) > 0, "Expected at least one attempt"
|
|
details: dict[str, Any] = attempts[0].details
|
|
assert "phase_transition_target" in details, (
|
|
f"Expected phase_transition_target in attempt details, "
|
|
f"got keys: {list(details.keys())}"
|
|
)
|