From a48410547b271c6bb1abc438a0ed3d3852d5f10e Mon Sep 17 00:00:00 2001 From: "Brent E. Edwards" Date: Sun, 29 Mar 2026 00:25:56 +0000 Subject: [PATCH 1/2] fix(cli): plan correct active-plan resolution in isolated environments Add a CLEVERAGENTS_HOME fallback in _resolve_active_plan_id() so isolated subprocess invocations can resolve the active non-terminal plan even when CWD differs from persistent storage. This keeps the existing primary lookup path intact, only falling back when needed, and adds a Robot regression helper/suite that reproduces and verifies the isolated environment behavior without requiring --plan. ISSUES CLOSED: #1025 --- features/consolidated_plan_misc.feature | 12 +- features/steps/plan_cli_legacy_r2_steps.py | 107 +++++++++++++++ robot/helper_plan_correct_isolated_resolve.py | 128 ++++++++++++++++++ robot/plan_correct_isolated_resolve.robot | 19 +++ src/cleveragents/cli/commands/plan.py | 51 +++++++ 5 files changed, 316 insertions(+), 1 deletion(-) create mode 100644 robot/helper_plan_correct_isolated_resolve.py create mode 100644 robot/plan_correct_isolated_resolve.robot diff --git a/features/consolidated_plan_misc.feature b/features/consolidated_plan_misc.feature index 174d11217..00548d102 100644 --- a/features/consolidated_plan_misc.feature +++ b/features/consolidated_plan_misc.feature @@ -520,6 +520,17 @@ Feature: Consolidated Plan Misc Then r2plan-a typer Abort should be raised + Scenario: r2plan resolve_active_plan_id skips home fallback when DB env override is set + When r2plan-I call _resolve_active_plan_id with explicit DB env override and no active plans + Then r2plan-a typer Abort should be raised + And r2plan-the home DB fallback should not be attempted + + + Scenario: r2plan resolve_active_plan_id surfaces unexpected fallback errors + When r2plan-I call _resolve_active_plan_id with unexpected fallback error + Then r2plan-a RuntimeError should be raised + + # ============================================================ # Originally from: plan_cli_spec_print_r2.feature # Feature: Plan CLI spec dict and print branch coverage (round 2) @@ -1037,4 +1048,3 @@ Feature: Consolidated Plan Misc And a PlanExecutor without execution_context When I attempt to run execute with an empty plan_id Then the plan execution context should raise a ValidationError containing "plan_id" - diff --git a/features/steps/plan_cli_legacy_r2_steps.py b/features/steps/plan_cli_legacy_r2_steps.py index 8e9750029..fb9845c8b 100644 --- a/features/steps/plan_cli_legacy_r2_steps.py +++ b/features/steps/plan_cli_legacy_r2_steps.py @@ -14,6 +14,8 @@ All step text uses the ``r2plan-`` prefix to avoid collisions. from __future__ import annotations +import os +import tempfile import warnings from datetime import datetime from types import SimpleNamespace @@ -403,6 +405,97 @@ def step_resolve_service_error(context: Any) -> None: context.r2_error = typer.Abort() +@when( + "r2plan-I call _resolve_active_plan_id with explicit DB env override and no active plans" +) +def step_resolve_skips_home_fallback_when_db_override(context: Any) -> None: + mock_svc = MagicMock() + terminal_plan = _make_plan( + phase=PlanPhase.APPLY, + processing_state=ProcessingState.APPLIED, + ) + mock_svc.list_plans.return_value = [terminal_plan] + + with tempfile.TemporaryDirectory(prefix="r2plan-db-override-") as tmp: + home_dir = os.path.join(tmp, "home") + os.makedirs(home_dir, exist_ok=True) + + mock_uow = MagicMock() + with ( + patch( + "cleveragents.cli.commands.plan._get_lifecycle_service", + return_value=mock_svc, + ), + patch( + "cleveragents.infrastructure.database.unit_of_work.UnitOfWork", + return_value=mock_uow, + ), + patch.dict( + os.environ, + { + "CLEVERAGENTS_HOME": home_dir, + "CLEVERAGENTS_DATABASE_URL": "sqlite:////tmp/explicit-test.db", + }, + clear=False, + ), + ): + try: + _resolve_active_plan_id() + context.r2_error = None + except typer.Abort: + context.r2_error = typer.Abort() + + context.r2_home_fallback_attempted = mock_uow.called + + +@when("r2plan-I call _resolve_active_plan_id with unexpected fallback error") +def step_resolve_unexpected_fallback_error(context: Any) -> None: + mock_svc = MagicMock() + terminal_plan = _make_plan( + phase=PlanPhase.APPLY, + processing_state=ProcessingState.APPLIED, + ) + mock_svc.list_plans.return_value = [terminal_plan] + + with tempfile.TemporaryDirectory(prefix="r2plan-fallback-error-") as tmp: + home_dir = os.path.join(tmp, "home") + workspace_dir = os.path.join(tmp, "workspace") + os.makedirs(home_dir, exist_ok=True) + os.makedirs(workspace_dir, exist_ok=True) + + original_cwd = os.getcwd() + os.chdir(workspace_dir) + try: + with ( + patch( + "cleveragents.cli.commands.plan._get_lifecycle_service", + return_value=mock_svc, + ), + patch( + "cleveragents.infrastructure.database.unit_of_work.UnitOfWork", + side_effect=RuntimeError("unexpected fallback crash"), + ), + patch.dict( + os.environ, + { + "CLEVERAGENTS_HOME": home_dir, + "CLEVERAGENTS_DATABASE_URL": "", + "CLEVERAGENTS_TEST_DATABASE_URL": "", + }, + clear=False, + ), + ): + try: + _resolve_active_plan_id() + context.r2_error = None + except typer.Abort: + context.r2_error = typer.Abort() + except RuntimeError as exc: + context.r2_error = exc + finally: + os.chdir(original_cwd) + + # --------------------------------------------------------------------------- # Then steps - legacy wrapper assertions # --------------------------------------------------------------------------- @@ -440,3 +533,17 @@ def step_typer_abort(context: Any) -> None: assert isinstance(context.r2_error, typer.Abort), ( f"Expected typer.Abort, got {type(context.r2_error).__name__}" ) + + +@then("r2plan-the home DB fallback should not be attempted") +def step_home_fallback_not_attempted(context: Any) -> None: + attempted = getattr(context, "r2_home_fallback_attempted", None) + assert attempted is False, "Expected fallback UnitOfWork not to be called" + + +@then("r2plan-a RuntimeError should be raised") +def step_runtime_error(context: Any) -> None: + assert context.r2_error is not None, "Expected RuntimeError but none raised" + assert isinstance(context.r2_error, RuntimeError), ( + f"Expected RuntimeError, got {type(context.r2_error).__name__}" + ) diff --git a/robot/helper_plan_correct_isolated_resolve.py b/robot/helper_plan_correct_isolated_resolve.py new file mode 100644 index 000000000..7a770cb77 --- /dev/null +++ b/robot/helper_plan_correct_isolated_resolve.py @@ -0,0 +1,128 @@ +"""Regression helper for issue #1025 (plan correct auto-resolve). + +This helper simulates isolated CLI invocations that share +``CLEVERAGENTS_HOME`` but run from different working directories. + +It verifies that ``agents plan correct `` can auto-resolve +the active plan (without ``--plan``) after process-global state is reset. +""" + +from __future__ import annotations + +import os +import tempfile +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +from typer.testing import CliRunner +from ulid import ULID + +from cleveragents.application.container import get_container, reset_container +from cleveragents.cli.commands.plan import app as plan_app +from cleveragents.config.settings import Settings +from cleveragents.domain.models.core.plan import PlanPhase, ProcessingState + + +def _reset_process_state() -> None: + """Reset singleton process state to emulate a fresh subprocess.""" + Settings.reset() + reset_container() + + +def _seed_active_plan(home_dir: Path) -> str: + """Create and persist an Execute/COMPLETE plan under ``home_dir`` DB.""" + os.chdir(home_dir) + _reset_process_state() + + container = get_container() + lifecycle = container.plan_lifecycle_service() + + action_name = f"local/issue-1025-{ULID()}" + lifecycle.create_action( + name=action_name, + description="Issue #1025 regression action", + definition_of_done="Do the thing", + strategy_actor="local/strategy-actor", + execution_actor="local/execution-actor", + ) + plan = lifecycle.use_action(action_name=action_name) + plan.phase = PlanPhase.EXECUTE + plan.processing_state = ProcessingState.COMPLETE + lifecycle.save_plan(plan) + + return plan.identity.plan_id + + +def run_regression() -> int: + """Run the regression scenario and return process exit status.""" + with tempfile.TemporaryDirectory(prefix="ca_issue_1025_") as tmp: + root = Path(tmp) + home_dir = root / "home" + workspace_dir = root / "workspace" + home_dir.mkdir(parents=True, exist_ok=True) + workspace_dir.mkdir(parents=True, exist_ok=True) + + os.environ["CLEVERAGENTS_HOME"] = str(home_dir) + os.environ["CLEVERAGENTS_AUTO_APPLY_MIGRATIONS"] = "true" + os.environ["CLEVERAGENTS_TESTING_USE_MOCK_AI"] = "true" + os.environ.pop("CLEVERAGENTS_DATABASE_URL", None) + os.environ.pop("CLEVERAGENTS_TEST_DATABASE_URL", None) + + plan_id = _seed_active_plan(home_dir) + + # Simulate a new isolated invocation from a *different* CWD. + os.chdir(workspace_dir) + _reset_process_state() + + fake_svc = MagicMock() + fake_svc.request_correction.return_value = SimpleNamespace( + correction_id=str(ULID()), + mode=SimpleNamespace(value="revert"), + target_decision_id=str(ULID()), + guidance="regression guidance", + ) + fake_svc.execute_correction.return_value = SimpleNamespace( + correction_id=str(ULID()), + status=SimpleNamespace(value="applied"), + new_decisions=[], + reverted_decisions=[], + ) + + runner = CliRunner() + with patch( + "cleveragents.application.services.correction_service.CorrectionService", + return_value=fake_svc, + ): + result = runner.invoke( + plan_app, + [ + "correct", + str(ULID()), + "--mode", + "revert", + "--guidance", + "regression guidance", + "--yes", + ], + ) + + if result.exit_code != 0: + print(result.output) + return 1 + + request_call = fake_svc.request_correction.call_args + if request_call is None: + print("request_correction was not invoked") + return 1 + resolved_plan_id = request_call.kwargs.get("plan_id") + if resolved_plan_id != plan_id: + print(f"Resolved plan mismatch: expected {plan_id}, got {resolved_plan_id}") + return 1 + + print("issue-1025-plan-correct-isolated-resolve-ok") + return 0 + + +if __name__ == "__main__": + raise SystemExit(run_regression()) diff --git a/robot/plan_correct_isolated_resolve.robot b/robot/plan_correct_isolated_resolve.robot new file mode 100644 index 000000000..67ded38f4 --- /dev/null +++ b/robot/plan_correct_isolated_resolve.robot @@ -0,0 +1,19 @@ +*** Settings *** +Documentation Regression test for issue #1025 (plan correct auto-resolve) +Resource ${CURDIR}/common.resource +Suite Setup Setup Test Environment +Suite Teardown Cleanup Test Environment + +*** Variables *** +${HELPER} ${CURDIR}/helper_plan_correct_isolated_resolve.py + +*** Test Cases *** +Plan Correct Auto Resolves Active Plan In Isolated Environment + [Documentation] Verify ``plan correct`` resolves active plan without ``--plan`` + ... when subprocess-like invocations share CLEVERAGENTS_HOME but + ... run from different working directories. + ${result}= Run Process ${PYTHON} ${HELPER} cwd=${WORKSPACE} timeout=120s on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} issue-1025-plan-correct-isolated-resolve-ok diff --git a/src/cleveragents/cli/commands/plan.py b/src/cleveragents/cli/commands/plan.py index fcc836dba..de3317ec8 100644 --- a/src/cleveragents/cli/commands/plan.py +++ b/src/cleveragents/cli/commands/plan.py @@ -35,6 +35,7 @@ import os import re import warnings from contextlib import suppress +from pathlib import Path from typing import TYPE_CHECKING, Annotated, Any import typer @@ -42,6 +43,7 @@ from rich.console import Console from rich.panel import Panel from rich.progress import Progress, SpinnerColumn, TextColumn from rich.table import Table +from sqlalchemy.exc import SQLAlchemyError from cleveragents.a2a.models import A2aRequest from cleveragents.cli.formatting import OutputFormat, format_output @@ -3114,11 +3116,60 @@ def _resolve_active_plan_id() -> str: Raises: typer.Abort: If no suitable plan is found. """ + + def _resolve_from_cleveragents_home() -> str | None: + """Best-effort fallback lookup using ``CLEVERAGENTS_HOME`` storage. + + In isolated subprocess environments some commands run from a project + workspace while persistent state lives under ``CLEVERAGENTS_HOME``. + When the default DB lookup returns no active plans, this fallback + attempts to read lifecycle plans from ``$CLEVERAGENTS_HOME``. + """ + + explicit_db_override = any( + os.environ.get(key, "").strip() + for key in ( + "CLEVERAGENTS_DATABASE_URL", + "CLEVERAGENTS_TEST_DATABASE_URL", + ) + ) + if explicit_db_override: + return None + + home_raw = os.environ.get("CLEVERAGENTS_HOME", "").strip() + if not home_raw: + return None + + home_db = (Path(home_raw).expanduser() / ".cleveragents" / "db.sqlite").resolve( + strict=False + ) + current_db = (Path.cwd() / ".cleveragents" / "db.sqlite").resolve(strict=False) + if home_db == current_db: + return None + + try: + from cleveragents.infrastructure.database.unit_of_work import UnitOfWork + + home_url = f"sqlite:///{home_db}" + uow = UnitOfWork(home_url, require_confirmation=False) + with uow.transaction() as ctx: + plans = ctx.lifecycle_plans.list_all() + + active = [p for p in plans if not p.is_terminal] + if not active: + return None + return active[0].identity.plan_id + except (CleverAgentsError, OSError, SQLAlchemyError, ValueError): + return None + try: service = _get_lifecycle_service() plans = service.list_plans() active = [p for p in plans if not p.is_terminal] if not active: + fallback_plan_id = _resolve_from_cleveragents_home() + if fallback_plan_id: + return fallback_plan_id console.print( "[red]Error:[/red] No active plan found. " "Specify --plan explicitly." -- 2.52.0 From 9dae67737ee284113afeaec8e565d4da890b3f40 Mon Sep 17 00:00:00 2001 From: "Brent E. Edwards" Date: Tue, 31 Mar 2026 01:28:15 +0000 Subject: [PATCH 2/2] fix(cli): use container DI override for correction service mock in integration test The plan_correct_isolated_resolve robot helper patched CorrectionService at the module level, but correct_decision now resolves the service via container.correction_service(). The module-level patch had no effect on the container's already-imported reference, causing the mock to never intercept the call and the test to fail with exit code 1. Replace unittest.mock.patch with container.correction_service.override() which is the idiomatic dependency-injector mechanism and correctly intercepts the singleton provider. ISSUES CLOSED: #1025 --- robot/helper_plan_correct_isolated_resolve.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/robot/helper_plan_correct_isolated_resolve.py b/robot/helper_plan_correct_isolated_resolve.py index 7a770cb77..13b15f71f 100644 --- a/robot/helper_plan_correct_isolated_resolve.py +++ b/robot/helper_plan_correct_isolated_resolve.py @@ -13,7 +13,7 @@ import os import tempfile from pathlib import Path from types import SimpleNamespace -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock from typer.testing import CliRunner from ulid import ULID @@ -90,10 +90,8 @@ def run_regression() -> int: ) runner = CliRunner() - with patch( - "cleveragents.application.services.correction_service.CorrectionService", - return_value=fake_svc, - ): + container = get_container() + with container.correction_service.override(fake_svc): result = runner.invoke( plan_app, [ -- 2.52.0