test: add TDD bug-capture test for #1025 — plan correct auto-resolve (#1172)
CI / quality (push) Successful in 36s
CI / build (push) Successful in 20s
CI / helm (push) Successful in 22s
CI / lint (push) Successful in 3m21s
CI / typecheck (push) Successful in 3m57s
CI / benchmark-regression (push) Has been skipped
CI / security (push) Successful in 4m7s
CI / unit_tests (push) Successful in 9m18s
CI / docker (push) Successful in 1m18s
CI / coverage (push) Successful in 12m28s
CI / e2e_tests (push) Failing after 19m0s
CI / integration_tests (push) Successful in 24m45s
CI / status-check (push) Failing after 1s
CI / benchmark-publish (push) Successful in 28m39s
CI / quality (push) Successful in 36s
CI / build (push) Successful in 20s
CI / helm (push) Successful in 22s
CI / lint (push) Successful in 3m21s
CI / typecheck (push) Successful in 3m57s
CI / benchmark-regression (push) Has been skipped
CI / security (push) Successful in 4m7s
CI / unit_tests (push) Successful in 9m18s
CI / docker (push) Successful in 1m18s
CI / coverage (push) Successful in 12m28s
CI / e2e_tests (push) Failing after 19m0s
CI / integration_tests (push) Successful in 24m45s
CI / status-check (push) Failing after 1s
CI / benchmark-publish (push) Successful in 28m39s
## Summary - add Behave `@tdd_expected_fail` bug-capture scenarios for `plan correct` auto-resolve without `--plan` - add shared mock fixtures and Robot integration coverage for isolated-container divergence reproducing bug #1025 - add ASV benchmark coverage for active-plan filtering and document the new TDD capture in changelog ## Testing - nox -s unit_tests -- features/tdd_plan_correct_auto_resolve.feature - nox -s integration_tests -- --include tdd_bug_1025 - nox -s lint - nox -s typecheck - nox -s coverage_report - nox Closes #1035 Reviewed-on: #1172 Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com> Co-authored-by: Brent Edwards <brent.edwards@cleverthis.com> Co-committed-by: Brent Edwards <brent.edwards@cleverthis.com>
This commit was merged in pull request #1172.
This commit is contained in:
@@ -2,6 +2,14 @@
|
||||
|
||||
## Unreleased
|
||||
|
||||
- Added TDD bug-capture tests for bug #1025 — ``plan correct`` auto-resolve
|
||||
fails in isolated E2E environments. Two Behave BDD scenarios
|
||||
(``@tdd_bug @tdd_bug_1025 @tdd_expected_fail``) verify that
|
||||
``_resolve_active_plan_id()`` finds an ``Execute/COMPLETE`` plan when
|
||||
``--plan`` is omitted. Two Robot Framework integration tests exercise
|
||||
the same path in a subprocess context. Tests simulate the divergent-
|
||||
container condition (fresh ``CLEVERAGENTS_HOME`` with empty database).
|
||||
ASV benchmark measures active-plan filtering overhead. (#1035)
|
||||
- Added missing `LspServerConfig` model fields per specification:
|
||||
`description` (max 1000 chars), `transport` (`LspTransport` enum with
|
||||
`stdio`/`tcp`, default `stdio`), `initialization` (dict for LSP
|
||||
|
||||
@@ -0,0 +1,100 @@
|
||||
"""ASV benchmarks for plan correct auto-resolve (issue #1025).
|
||||
|
||||
Measures overhead of the ``_resolve_active_plan_id()`` path when the
|
||||
``plan correct`` command is invoked without a ``--plan`` flag. Benchmarks
|
||||
the plan listing and filtering that determines the active plan.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
from datetime import datetime
|
||||
from pathlib import Path
|
||||
from typing import ClassVar
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
try:
|
||||
from cleveragents.domain.models.core.plan import (
|
||||
NamespacedName,
|
||||
Plan,
|
||||
PlanIdentity,
|
||||
PlanPhase,
|
||||
PlanTimestamps,
|
||||
ProcessingState,
|
||||
ProjectLink,
|
||||
)
|
||||
except ModuleNotFoundError:
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "src"))
|
||||
from cleveragents.domain.models.core.plan import (
|
||||
NamespacedName,
|
||||
Plan,
|
||||
PlanIdentity,
|
||||
PlanPhase,
|
||||
PlanTimestamps,
|
||||
ProcessingState,
|
||||
ProjectLink,
|
||||
)
|
||||
|
||||
from ulid import ULID
|
||||
|
||||
|
||||
def _make_plan(phase: PlanPhase, state: ProcessingState) -> Plan:
|
||||
"""Build a plan with the given phase and state."""
|
||||
return Plan(
|
||||
identity=PlanIdentity(plan_id=str(ULID())),
|
||||
namespaced_name=NamespacedName(
|
||||
server=None, namespace="local", name="bench-plan"
|
||||
),
|
||||
action_name="local/bench-action",
|
||||
description="Benchmark plan",
|
||||
definition_of_done=None,
|
||||
phase=phase,
|
||||
processing_state=state,
|
||||
project_links=[ProjectLink(project_name="proj-1")],
|
||||
strategy_actor="openai/gpt-4",
|
||||
execution_actor="openai/gpt-4",
|
||||
created_by=None,
|
||||
reusable=False,
|
||||
read_only=False,
|
||||
timestamps=PlanTimestamps(created_at=datetime.now(), updated_at=datetime.now()),
|
||||
)
|
||||
|
||||
|
||||
def _build_plan_list(count: int) -> list[Plan]:
|
||||
"""Build a mixed list of plans with various states.
|
||||
|
||||
Approximately half the plans are terminal (APPLIED/CANCELLED),
|
||||
and half are active (Execute/COMPLETE or Strategize/PROCESSING).
|
||||
"""
|
||||
plans: list[Plan] = []
|
||||
for i in range(count):
|
||||
if i % 2 == 0:
|
||||
plans.append(_make_plan(PlanPhase.EXECUTE, ProcessingState.COMPLETE))
|
||||
else:
|
||||
plans.append(_make_plan(PlanPhase.APPLY, ProcessingState.APPLIED))
|
||||
return plans
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Benchmarks
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class ResolveActivePlanSuite:
|
||||
"""Benchmark the active plan filtering logic from _resolve_active_plan_id."""
|
||||
|
||||
params: ClassVar[list[int]] = [10, 100, 500]
|
||||
param_names: ClassVar[list[str]] = ["plan_count"]
|
||||
|
||||
def setup(self, plan_count: int) -> None:
|
||||
"""Prepare plan lists of varying size."""
|
||||
self.plans = _build_plan_list(plan_count)
|
||||
self.mock_service = MagicMock()
|
||||
self.mock_service.list_plans.return_value = self.plans
|
||||
|
||||
def time_filter_active_plans(self, plan_count: int) -> None:
|
||||
"""Time the non-terminal filtering done by _resolve_active_plan_id."""
|
||||
plans = self.mock_service.list_plans()
|
||||
active = [p for p in plans if not p.is_terminal]
|
||||
if active:
|
||||
_ = active[0].identity.plan_id
|
||||
@@ -0,0 +1,231 @@
|
||||
"""Shared mock fixtures for TDD plan-correct auto-resolve tests.
|
||||
|
||||
Provides constants, mock builders, and CLI argument helpers used by both
|
||||
the Behave step definitions
|
||||
(``features/steps/tdd_plan_correct_auto_resolve_steps.py``) and the Robot
|
||||
Framework integration test helper
|
||||
(``robot/helper_tdd_plan_correct_auto_resolve.py``).
|
||||
|
||||
Centralising the mock builders eliminates duplication and ensures both
|
||||
test suites exercise the ``plan correct`` CLI auto-resolve path with
|
||||
identically-shaped mock objects.
|
||||
|
||||
Bug: https://git.cleverthis.com/cleveragents/cleveragents-core/issues/1025
|
||||
TDD: https://git.cleverthis.com/cleveragents/cleveragents-core/issues/1035
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from datetime import datetime
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from cleveragents.core.exceptions import ResourceNotFoundError
|
||||
from cleveragents.domain.models.core.plan import (
|
||||
NamespacedName,
|
||||
Plan,
|
||||
PlanIdentity,
|
||||
PlanPhase,
|
||||
PlanTimestamps,
|
||||
ProcessingState,
|
||||
ProjectLink,
|
||||
)
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Patch targets
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
PATCH_CONTAINER: str = "cleveragents.application.container.get_container"
|
||||
PATCH_CORRECTION_SVC: str = (
|
||||
"cleveragents.application.services.correction_service.CorrectionService"
|
||||
)
|
||||
# We intentionally do NOT patch ``_resolve_active_plan_id`` because
|
||||
# this test exercises the auto-resolve path itself.
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fixed identifiers for deterministic assertions
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
# The decision_id passed as the positional argument (not a plan_id).
|
||||
DECISION_ID: str = "DEC-1025-TARGET"
|
||||
# Root decision in the plan's tree.
|
||||
ROOT_DECISION_ID: str = "DEC-1025-ROOT"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Mock builders
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def make_decision_ns(
|
||||
decision_id: str,
|
||||
parent_decision_id: str | None,
|
||||
) -> SimpleNamespace:
|
||||
"""Create a minimal decision-like namespace for list_decisions."""
|
||||
return SimpleNamespace(
|
||||
decision_id=decision_id,
|
||||
parent_decision_id=parent_decision_id,
|
||||
)
|
||||
|
||||
|
||||
def _make_plan() -> Plan:
|
||||
"""Build a real ``Plan`` in Execute/COMPLETE state.
|
||||
|
||||
This simulates a plan that has completed the Execute phase — the
|
||||
exact state described in bug #1025. A fresh ULID is used for the
|
||||
identity to satisfy the 26-char ULID validation constraint.
|
||||
"""
|
||||
from ulid import ULID
|
||||
|
||||
return Plan(
|
||||
identity=PlanIdentity(plan_id=str(ULID())),
|
||||
namespaced_name=NamespacedName(
|
||||
server=None, namespace="local", name="tdd-1025-plan"
|
||||
),
|
||||
action_name="local/tdd-1025-action",
|
||||
description="TDD plan for bug #1025 — Execute/COMPLETE state",
|
||||
definition_of_done=None,
|
||||
phase=PlanPhase.EXECUTE,
|
||||
processing_state=ProcessingState.COMPLETE,
|
||||
project_links=[ProjectLink(project_name="proj-1")],
|
||||
strategy_actor="openai/gpt-4",
|
||||
execution_actor="openai/gpt-4",
|
||||
created_by=None,
|
||||
reusable=False,
|
||||
read_only=False,
|
||||
timestamps=PlanTimestamps(created_at=datetime.now(), updated_at=datetime.now()),
|
||||
)
|
||||
|
||||
|
||||
def make_full_container() -> MagicMock:
|
||||
"""Build a mock DI container that has the plan and decision tree.
|
||||
|
||||
This container simulates the "main" context where the plan exists
|
||||
in the database. Both ``plan_lifecycle_service`` and
|
||||
``decision_service`` return valid data.
|
||||
"""
|
||||
plan = _make_plan()
|
||||
|
||||
mock_plan_svc = MagicMock()
|
||||
# get_plan raises RNF for the decision_id (it's not a plan_id),
|
||||
# ensuring the code falls through to the decision_id path.
|
||||
mock_plan_svc.get_plan.side_effect = ResourceNotFoundError(
|
||||
resource_type="Plan",
|
||||
resource_id=DECISION_ID,
|
||||
)
|
||||
# list_plans returns the plan in Execute/COMPLETE — auto-resolve
|
||||
# should find this.
|
||||
mock_plan_svc.list_plans.return_value = [plan]
|
||||
|
||||
decisions = [
|
||||
make_decision_ns(ROOT_DECISION_ID, None),
|
||||
make_decision_ns(DECISION_ID, ROOT_DECISION_ID),
|
||||
]
|
||||
|
||||
mock_decision_svc = MagicMock()
|
||||
mock_decision_svc.list_decisions.return_value = decisions
|
||||
mock_decision_svc.get_influence_edges.return_value = {}
|
||||
|
||||
mock_container = MagicMock()
|
||||
mock_container.plan_lifecycle_service.return_value = mock_plan_svc
|
||||
mock_container.decision_service.return_value = mock_decision_svc
|
||||
return mock_container
|
||||
|
||||
|
||||
def make_isolated_container() -> MagicMock:
|
||||
"""Build a mock DI container simulating an isolated E2E environment.
|
||||
|
||||
In an isolated E2E subprocess environment (where
|
||||
``CLEVERAGENTS_HOME`` points to a fresh temp directory), the
|
||||
``plan_lifecycle_service`` resolves to a fresh database that has
|
||||
**no plans**. This container simulates that condition.
|
||||
"""
|
||||
mock_plan_svc = MagicMock()
|
||||
mock_plan_svc.list_plans.return_value = [] # No plans in isolated DB
|
||||
|
||||
mock_container = MagicMock()
|
||||
mock_container.plan_lifecycle_service.return_value = mock_plan_svc
|
||||
return mock_container
|
||||
|
||||
|
||||
def make_container_side_effects() -> list[MagicMock]:
|
||||
"""Build the side_effect list for ``get_container`` calls.
|
||||
|
||||
The ``correct_decision`` function calls ``get_container()`` once
|
||||
for its own use, then ``_resolve_active_plan_id`` calls
|
||||
``_get_lifecycle_service`` → ``get_container()`` a second time.
|
||||
|
||||
In isolated E2E environments, the second call resolves to a fresh
|
||||
container with an empty database. This side-effect list simulates
|
||||
that divergence:
|
||||
|
||||
- Call 1: full container (has plans, decisions, services)
|
||||
- Call 2: isolated container (empty plan list)
|
||||
|
||||
Returns:
|
||||
A two-element list suitable for ``MagicMock.side_effect``.
|
||||
"""
|
||||
return [make_full_container(), make_isolated_container()]
|
||||
|
||||
|
||||
def make_correction_svc(
|
||||
target_decision_id: str,
|
||||
mode: str = "revert",
|
||||
) -> MagicMock:
|
||||
"""Build a mock CorrectionService that succeeds for a given target.
|
||||
|
||||
Args:
|
||||
target_decision_id: The decision ID expected in the correction.
|
||||
mode: The correction mode (``"revert"`` or ``"append"``).
|
||||
"""
|
||||
svc = MagicMock()
|
||||
svc.request_correction.return_value = SimpleNamespace(
|
||||
correction_id="CORR-TDD-1025",
|
||||
mode=SimpleNamespace(value=mode),
|
||||
target_decision_id=target_decision_id,
|
||||
guidance="Recompute decision subtree",
|
||||
)
|
||||
svc.execute_correction.return_value = SimpleNamespace(
|
||||
correction_id="CORR-TDD-1025",
|
||||
status=SimpleNamespace(value="applied"),
|
||||
reverted_decisions=[target_decision_id],
|
||||
new_decisions=[],
|
||||
)
|
||||
return svc
|
||||
|
||||
|
||||
def build_cli_args(decision_id: str, mode: str = "revert") -> list[str]:
|
||||
"""Build the CLI argument list for ``plan correct <decision_id>``.
|
||||
|
||||
The ``--plan`` flag is intentionally **omitted** so that the code
|
||||
must auto-resolve the active plan via ``_resolve_active_plan_id()``.
|
||||
|
||||
Args:
|
||||
decision_id: The decision ID to pass as the first positional arg.
|
||||
mode: The correction mode (``"revert"`` or ``"append"``).
|
||||
"""
|
||||
return [
|
||||
"correct",
|
||||
decision_id,
|
||||
"--mode",
|
||||
mode,
|
||||
"--guidance",
|
||||
"Recompute decision subtree",
|
||||
"--yes",
|
||||
"--format",
|
||||
"plain",
|
||||
]
|
||||
|
||||
|
||||
__all__: list[str] = [
|
||||
"DECISION_ID",
|
||||
"PATCH_CONTAINER",
|
||||
"PATCH_CORRECTION_SVC",
|
||||
"ROOT_DECISION_ID",
|
||||
"build_cli_args",
|
||||
"make_container_side_effects",
|
||||
"make_correction_svc",
|
||||
"make_decision_ns",
|
||||
"make_full_container",
|
||||
"make_isolated_container",
|
||||
]
|
||||
@@ -0,0 +1,176 @@
|
||||
"""Step definitions for tdd_plan_correct_auto_resolve.feature.
|
||||
|
||||
Captures bug #1025: the ``plan correct`` CLI command's
|
||||
``_resolve_active_plan_id()`` fails to find plans in isolated E2E
|
||||
environments (where ``CLEVERAGENTS_HOME`` is a temp directory) even
|
||||
when a plan in ``Execute/COMPLETE`` state exists in the database.
|
||||
|
||||
The test simulates this by providing divergent container instances:
|
||||
the main ``correct_decision`` function gets a container that has
|
||||
plans, but the ``_resolve_active_plan_id`` path (via
|
||||
``_get_lifecycle_service``) gets a container whose
|
||||
``list_plans()`` returns nothing — mimicking the condition where
|
||||
a fresh ``CLEVERAGENTS_HOME`` resolves to an empty database.
|
||||
|
||||
The assertions verify the *expected* (correct) behaviour: the
|
||||
command should auto-resolve the plan and succeed. They will
|
||||
**fail** on the current codebase, proving the bug exists. The
|
||||
``@tdd_expected_fail`` tag inverts the result so CI passes.
|
||||
|
||||
All step text uses the ``tpcar`` prefix to avoid collisions with
|
||||
other step files.
|
||||
|
||||
Bug: https://git.cleverthis.com/cleveragents/cleveragents-core/issues/1025
|
||||
TDD: https://git.cleverthis.com/cleveragents/cleveragents-core/issues/1035
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import patch
|
||||
|
||||
from behave import given, then, when
|
||||
from behave.runner import Context
|
||||
from typer.testing import CliRunner
|
||||
|
||||
from cleveragents.cli.commands.plan import app as plan_app
|
||||
from features.mocks.tdd_plan_correct_auto_resolve_fixtures import (
|
||||
DECISION_ID,
|
||||
PATCH_CONTAINER,
|
||||
PATCH_CORRECTION_SVC,
|
||||
build_cli_args,
|
||||
make_container_side_effects,
|
||||
make_correction_svc,
|
||||
)
|
||||
|
||||
runner = CliRunner()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# GIVEN steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@given(
|
||||
"tpcar a container with a plan in Execute/COMPLETE"
|
||||
" and an isolated auto-resolve path"
|
||||
)
|
||||
def step_tpcar_container_with_isolated_resolve(context: Context) -> None:
|
||||
"""Set up divergent containers to simulate the isolated-env bug.
|
||||
|
||||
The first ``get_container()`` call (from ``correct_decision``)
|
||||
returns a full container with a plan in ``Execute/COMPLETE``.
|
||||
The second call (from ``_resolve_active_plan_id`` via
|
||||
``_get_lifecycle_service``) returns an isolated container whose
|
||||
``list_plans()`` returns no plans — reproducing the E2E failure.
|
||||
"""
|
||||
context.tpcar_container_side_effects = make_container_side_effects()
|
||||
|
||||
|
||||
@given("tpcar a CorrectionService that succeeds for the target decision in revert mode")
|
||||
def step_tpcar_correction_svc_revert(context: Context) -> None:
|
||||
"""Set up mock CorrectionService for revert mode."""
|
||||
context.tpcar_correction_svc = make_correction_svc(DECISION_ID, mode="revert")
|
||||
|
||||
|
||||
@given("tpcar a CorrectionService that succeeds for the target decision in append mode")
|
||||
def step_tpcar_correction_svc_append(context: Context) -> None:
|
||||
"""Set up mock CorrectionService for append mode."""
|
||||
context.tpcar_correction_svc = make_correction_svc(DECISION_ID, mode="append")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# WHEN steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@when(
|
||||
"tpcar I invoke plan correct with a decision_id and no --plan flag in revert mode"
|
||||
)
|
||||
def step_tpcar_invoke_revert(context: Context) -> None:
|
||||
"""Invoke ``plan correct <decision_id> --mode revert`` without --plan.
|
||||
|
||||
The ``--plan`` flag is intentionally omitted so the code must call
|
||||
``_resolve_active_plan_id()`` to find the active plan. The
|
||||
divergent container side-effects simulate the isolated-env
|
||||
condition where auto-resolve fails.
|
||||
"""
|
||||
args = build_cli_args(DECISION_ID, mode="revert")
|
||||
|
||||
with (
|
||||
patch(
|
||||
PATCH_CORRECTION_SVC,
|
||||
return_value=context.tpcar_correction_svc,
|
||||
),
|
||||
patch(
|
||||
PATCH_CONTAINER,
|
||||
side_effect=context.tpcar_container_side_effects,
|
||||
),
|
||||
):
|
||||
context.tpcar_result = runner.invoke(plan_app, args)
|
||||
|
||||
|
||||
@when(
|
||||
"tpcar I invoke plan correct with a decision_id and no --plan flag in append mode"
|
||||
)
|
||||
def step_tpcar_invoke_append(context: Context) -> None:
|
||||
"""Invoke ``plan correct <decision_id> --mode append`` without --plan.
|
||||
|
||||
Same auto-resolve path as revert mode — the divergent container
|
||||
side-effects exercise the same ``_resolve_active_plan_id`` bug.
|
||||
"""
|
||||
args = build_cli_args(DECISION_ID, mode="append")
|
||||
|
||||
with (
|
||||
patch(
|
||||
PATCH_CORRECTION_SVC,
|
||||
return_value=context.tpcar_correction_svc,
|
||||
),
|
||||
patch(
|
||||
PATCH_CONTAINER,
|
||||
side_effect=context.tpcar_container_side_effects,
|
||||
),
|
||||
):
|
||||
context.tpcar_result = runner.invoke(plan_app, args)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# THEN steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@then("tpcar the command should exit successfully")
|
||||
def step_tpcar_exit_ok(context: Context) -> None:
|
||||
"""Assert plan correct exits with code 0.
|
||||
|
||||
This assertion will FAIL on the current codebase because the
|
||||
divergent containers cause ``_resolve_active_plan_id`` to see
|
||||
an empty plan list and raise ``typer.Abort`` — proving bug #1025.
|
||||
"""
|
||||
result = context.tpcar_result
|
||||
assert result.exit_code == 0, (
|
||||
f"Bug #1025: plan correct exited with code {result.exit_code} "
|
||||
f"instead of 0 when --plan was omitted. The auto-resolve "
|
||||
f"path failed to find the active plan in the isolated "
|
||||
f"environment. Output: {result.output}"
|
||||
)
|
||||
|
||||
|
||||
@then("tpcar the correction should have used the auto-resolved plan")
|
||||
def step_tpcar_correction_used_resolved_plan(context: Context) -> None:
|
||||
"""Assert request_correction was called (auto-resolve succeeded).
|
||||
|
||||
When auto-resolve works correctly, the correction service receives
|
||||
the plan_id from the auto-resolved plan and the decision_id from
|
||||
the positional argument.
|
||||
"""
|
||||
svc = context.tpcar_correction_svc
|
||||
svc.request_correction.assert_called_once()
|
||||
call_record = svc.request_correction.call_args
|
||||
kw = call_record.kwargs
|
||||
|
||||
actual_target = kw.get("target_decision_id")
|
||||
assert actual_target == DECISION_ID, (
|
||||
f"Bug #1025: request_correction was called with "
|
||||
f"target_decision_id={actual_target!r} but expected "
|
||||
f"{DECISION_ID!r}."
|
||||
)
|
||||
@@ -0,0 +1,45 @@
|
||||
@tdd_expected_fail @tdd_issue @tdd_issue_1025
|
||||
Feature: TDD Bug #1025 — plan correct auto-resolve fails in isolated environments
|
||||
As a developer
|
||||
I want plan correct to auto-resolve the active plan when invoked
|
||||
with a decision_id and no --plan flag
|
||||
So that the CLI works without manually specifying the plan ID
|
||||
|
||||
The plan correct CLI command uses _resolve_active_plan_id() to find
|
||||
the current plan when --plan is omitted. In isolated E2E environments
|
||||
(where CLEVERAGENTS_HOME is a temp directory), this resolution fails
|
||||
even when a plan in Execute/COMPLETE state exists in the database.
|
||||
|
||||
The expected behavior is that _resolve_active_plan_id() finds the
|
||||
plan in Execute/COMPLETE state and auto-resolves it, allowing the
|
||||
correction to proceed without the explicit --plan flag.
|
||||
|
||||
Bug #1025 reports that running:
|
||||
plan correct <decision_id> --mode revert --guidance "..."
|
||||
fails with "No active plan found" in isolated subprocess environments.
|
||||
|
||||
These tests simulate the isolated-environment condition by providing
|
||||
divergent container instances: the main correct_decision context gets
|
||||
a container with plans, but the _resolve_active_plan_id path gets a
|
||||
container with an empty database (mimicking a fresh CLEVERAGENTS_HOME).
|
||||
|
||||
The scenarios assert the expected (correct) behavior and will FAIL
|
||||
until the bug is fixed. The @tdd_expected_fail tag inverts the
|
||||
result so CI passes.
|
||||
|
||||
# This test captures bug #1025 and uses @tdd_expected_fail until the
|
||||
# fix is merged.
|
||||
|
||||
Scenario: plan correct auto-resolves active plan in revert mode
|
||||
Given tpcar a container with a plan in Execute/COMPLETE and an isolated auto-resolve path
|
||||
And tpcar a CorrectionService that succeeds for the target decision in revert mode
|
||||
When tpcar I invoke plan correct with a decision_id and no --plan flag in revert mode
|
||||
Then tpcar the command should exit successfully
|
||||
And tpcar the correction should have used the auto-resolved plan
|
||||
|
||||
Scenario: plan correct auto-resolves active plan in append mode
|
||||
Given tpcar a container with a plan in Execute/COMPLETE and an isolated auto-resolve path
|
||||
And tpcar a CorrectionService that succeeds for the target decision in append mode
|
||||
When tpcar I invoke plan correct with a decision_id and no --plan flag in append mode
|
||||
Then tpcar the command should exit successfully
|
||||
And tpcar the correction should have used the auto-resolved plan
|
||||
@@ -0,0 +1,127 @@
|
||||
"""Helper script for tdd_plan_correct_auto_resolve.robot integration tests.
|
||||
|
||||
Each subcommand exercises the ``plan correct`` CLI command with a
|
||||
decision_id as the first positional argument and **no** ``--plan``
|
||||
flag, forcing the code to auto-resolve the active plan via
|
||||
``_resolve_active_plan_id()``.
|
||||
|
||||
The helper simulates the isolated-environment condition described in
|
||||
bug #1025: the first ``get_container()`` call returns a container
|
||||
with plans, but the second call (from ``_resolve_active_plan_id``)
|
||||
returns a container with an empty plan list.
|
||||
|
||||
The helper reports the **real** outcome: it exits 0 and prints the
|
||||
sentinel when the operation succeeds (bug is fixed), and exits 1
|
||||
when the bug is still present. The ``tdd_expected_fail_listener``
|
||||
on the Robot side handles pass/fail inversion while the bug remains
|
||||
open.
|
||||
|
||||
Bug: https://git.cleverthis.com/cleveragents/cleveragents-core/issues/1025
|
||||
TDD: https://git.cleverthis.com/cleveragents/cleveragents-core/issues/1035
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
from collections.abc import Callable
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
# Ensure local source tree is importable
|
||||
_SRC = str(Path(__file__).resolve().parents[1] / "src")
|
||||
if _SRC not in sys.path:
|
||||
sys.path.insert(0, _SRC)
|
||||
|
||||
# Ensure features package is importable for shared fixtures
|
||||
_FEATURES = str(Path(__file__).resolve().parents[1])
|
||||
if _FEATURES not in sys.path:
|
||||
sys.path.insert(0, _FEATURES)
|
||||
|
||||
from features.mocks.tdd_plan_correct_auto_resolve_fixtures import ( # noqa: E402
|
||||
DECISION_ID,
|
||||
PATCH_CONTAINER,
|
||||
PATCH_CORRECTION_SVC,
|
||||
build_cli_args,
|
||||
make_container_side_effects,
|
||||
make_correction_svc,
|
||||
)
|
||||
from typer.testing import CliRunner # noqa: E402
|
||||
|
||||
from cleveragents.cli.commands.plan import app as plan_app # noqa: E402
|
||||
|
||||
runner = CliRunner()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Subcommands
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _run_plan_correct_auto_resolve(mode: str, sentinel: str) -> None:
|
||||
"""Invoke ``plan correct <decision_id>`` without ``--plan``.
|
||||
|
||||
Simulates the isolated-environment condition: the first
|
||||
``get_container`` call returns a full container, the second
|
||||
returns an empty one.
|
||||
|
||||
Exits 0 with *sentinel* when the command succeeds (bug fixed).
|
||||
Exits 1 when the bug is still present.
|
||||
|
||||
Args:
|
||||
mode: The correction mode (``"revert"`` or ``"append"``).
|
||||
sentinel: The sentinel string to print on success.
|
||||
"""
|
||||
correction_svc = make_correction_svc(DECISION_ID, mode=mode)
|
||||
side_effects = make_container_side_effects()
|
||||
|
||||
args = build_cli_args(DECISION_ID, mode=mode)
|
||||
|
||||
with (
|
||||
patch(PATCH_CORRECTION_SVC, return_value=correction_svc),
|
||||
patch(PATCH_CONTAINER, side_effect=side_effects),
|
||||
):
|
||||
result = runner.invoke(plan_app, args)
|
||||
|
||||
if result.exit_code != 0:
|
||||
print(
|
||||
f"Bug #1025: plan correct failed with exit code "
|
||||
f"{result.exit_code} when --plan was omitted. "
|
||||
f"Auto-resolve failed in isolated environment. "
|
||||
f"Output: {result.output}",
|
||||
file=sys.stderr,
|
||||
)
|
||||
sys.exit(1)
|
||||
|
||||
# Verify the correction service was called
|
||||
correction_svc.request_correction.assert_called_once()
|
||||
print(sentinel)
|
||||
|
||||
|
||||
def plan_correct_auto_resolve_revert() -> None:
|
||||
"""Auto-resolve test with --mode revert."""
|
||||
_run_plan_correct_auto_resolve("revert", "tdd-plan-correct-auto-resolve-revert-ok")
|
||||
|
||||
|
||||
def plan_correct_auto_resolve_append() -> None:
|
||||
"""Auto-resolve test with --mode append."""
|
||||
_run_plan_correct_auto_resolve("append", "tdd-plan-correct-auto-resolve-append-ok")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Dispatcher
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
_COMMANDS: dict[str, Callable[[], None]] = {
|
||||
"auto-resolve-revert": plan_correct_auto_resolve_revert,
|
||||
"auto-resolve-append": plan_correct_auto_resolve_append,
|
||||
}
|
||||
|
||||
if __name__ == "__main__":
|
||||
if len(sys.argv) < 2 or sys.argv[1] not in _COMMANDS:
|
||||
print(
|
||||
f"Usage: {sys.argv[0]} <{'|'.join(_COMMANDS)}>",
|
||||
file=sys.stderr,
|
||||
)
|
||||
sys.exit(1)
|
||||
cmd = _COMMANDS[sys.argv[1]]
|
||||
cmd()
|
||||
@@ -0,0 +1,43 @@
|
||||
*** Settings ***
|
||||
Documentation TDD Bug #1025 — plan correct auto-resolve fails in isolated environments.
|
||||
... Integration test verifying that ``plan correct <decision_id>``
|
||||
... auto-resolves the active plan when ``--plan`` is omitted, even
|
||||
... in isolated E2E subprocess environments where CLEVERAGENTS_HOME
|
||||
... is a temp directory.
|
||||
...
|
||||
... Tests are tagged tdd_expected_fail so CI passes via result inversion
|
||||
... while the bug remains unfixed.
|
||||
...
|
||||
... Bug: https://git.cleverthis.com/cleveragents/cleveragents-core/issues/1025
|
||||
... TDD: https://git.cleverthis.com/cleveragents/cleveragents-core/issues/1035
|
||||
Resource ${CURDIR}/common.resource
|
||||
Suite Setup Setup Test Environment
|
||||
Suite Teardown Cleanup Test Environment
|
||||
|
||||
*** Variables ***
|
||||
${HELPER} ${CURDIR}/helper_tdd_plan_correct_auto_resolve.py
|
||||
|
||||
*** Test Cases ***
|
||||
TDD Plan Correct Auto Resolve Revert Mode
|
||||
[Documentation] Verify that plan correct auto-resolves the active plan
|
||||
... when invoked with a decision_id and no --plan flag in
|
||||
... revert mode. Bug #1025: auto-resolve fails in isolated
|
||||
... E2E environments.
|
||||
[Tags] tdd_expected_fail tdd_issue tdd_issue_1025
|
||||
${result}= Run Process ${PYTHON} ${HELPER} auto-resolve-revert cwd=${WORKSPACE} timeout=30s on_timeout=kill
|
||||
Log ${result.stdout}
|
||||
Log ${result.stderr}
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} tdd-plan-correct-auto-resolve-revert-ok
|
||||
|
||||
TDD Plan Correct Auto Resolve Append Mode
|
||||
[Documentation] Verify that plan correct auto-resolves the active plan
|
||||
... when invoked with a decision_id and no --plan flag in
|
||||
... append mode. Bug #1025: auto-resolve fails in isolated
|
||||
... E2E environments.
|
||||
[Tags] tdd_expected_fail tdd_issue tdd_issue_1025
|
||||
${result}= Run Process ${PYTHON} ${HELPER} auto-resolve-append cwd=${WORKSPACE} timeout=30s on_timeout=kill
|
||||
Log ${result.stdout}
|
||||
Log ${result.stderr}
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} tdd-plan-correct-auto-resolve-append-ok
|
||||
Reference in New Issue
Block a user