From 6e7e1afc38cd25a41b65999a1a049906a337518a Mon Sep 17 00:00:00 2001 From: root Date: Fri, 31 Jul 2026 17:26:03 -0500 Subject: [PATCH] fix: keep rejected plan resolves transactional --- .../app/commands/plan/override/resolve_cmd.py | 24 -------- .../plan/test_plan_override_transactions.py | 58 ++++++++++++++++++- .../plan/test_plan_overrides_direct.py | 8 --- .../commands/plan/test_workflow_gates.py | 10 +++- 4 files changed, 64 insertions(+), 36 deletions(-) diff --git a/desloppify/app/commands/plan/override/resolve_cmd.py b/desloppify/app/commands/plan/override/resolve_cmd.py index 6dddeac4b..eea42bc0e 100644 --- a/desloppify/app/commands/plan/override/resolve_cmd.py +++ b/desloppify/app/commands/plan/override/resolve_cmd.py @@ -3,7 +3,6 @@ from __future__ import annotations import argparse -import logging from desloppify.app.commands.helpers.attestation import ( show_attestation_requirement, @@ -14,14 +13,11 @@ from desloppify.app.commands.helpers.command_runtime import command_runtime from desloppify.app.commands.resolve.cmd import cmd_resolve from desloppify.base.exception_sets import PLAN_LOAD_EXCEPTIONS -from desloppify.base.output.fallbacks import log_best_effort_failure from desloppify.base.output.terminal import colorize from desloppify.engine._work_queue.core import ATTEST_EXAMPLE from desloppify.engine.plan_state import ( load_plan, - save_plan, ) -from desloppify.engine.plan_ops import append_log_entry from .resolve_helpers import ( check_cluster_guard, @@ -29,8 +25,6 @@ ) from .resolve_workflow import resolve_workflow_patterns -logger = logging.getLogger(__name__) - def cmd_plan_resolve(args: argparse.Namespace) -> None: """Mark issues as fixed and delegate to resolve command UX.""" @@ -78,24 +72,6 @@ def cmd_plan_resolve(args: argparse.Namespace) -> None: except PLAN_LOAD_EXCEPTIONS: plan = None - try: - if plan is None: - plan = load_plan() - clusters = plan.get("clusters", {}) - cluster_name = next((pattern for pattern in patterns if pattern in clusters), None) - append_log_entry( - plan, - "done", - issue_ids=patterns, - cluster_name=cluster_name, - actor="user", - note=note, - ) - save_plan(plan) - except PLAN_LOAD_EXCEPTIONS as exc: - log_best_effort_failure(logger, "append plan resolve log entry", exc) - print(colorize(f" Note: unable to append plan resolve log entry ({exc}).", "dim")) - resolve_args = argparse.Namespace( status="fixed", patterns=patterns, diff --git a/desloppify/tests/commands/plan/test_plan_override_transactions.py b/desloppify/tests/commands/plan/test_plan_override_transactions.py index c8e7b22c5..944744751 100644 --- a/desloppify/tests/commands/plan/test_plan_override_transactions.py +++ b/desloppify/tests/commands/plan/test_plan_override_transactions.py @@ -7,13 +7,15 @@ import pytest +import desloppify.engine._plan.persistence as plan_persistence from desloppify import state as state_mod from desloppify.app.commands.helpers.command_runtime import CommandRuntime from desloppify.app.commands.plan.override import io as override_io +from desloppify.app.commands.plan.override import resolve_cmd as override_resolve from desloppify.app.commands.plan.override import skip as override_skip from desloppify.base.exception_sets import CommandError -from desloppify.engine.plan_state import empty_plan, load_plan, save_plan from desloppify.engine.plan_ops import skip_items +from desloppify.engine.plan_state import empty_plan, load_plan, save_plan _ATTEST = "I have actually reviewed this and I am not gaming the score." @@ -133,3 +135,57 @@ def _boom(*_args, **_kwargs): assert state_after["issues"][issue_id]["status"] == "open" assert plan_after.get("queue_order", []) == [issue_id] assert plan_after.get("skipped", {}) == {} + + +def test_cmd_plan_resolve_out_of_order_keeps_state_and_plan_unchanged( + tmp_path, + monkeypatch: pytest.MonkeyPatch, + capsys, +) -> None: + state_file = tmp_path / "state.json" + plan_file = tmp_path / "plan.json" + + state, first_id = _seed_state() + second = state_mod.make_issue( + "review", + "src/example.py", + "second-sample-issue", + tier=1, + confidence="high", + summary="second sample", + ) + second_id = second["id"] + state["work_items"][second_id] = second + plan = _seed_plan(first_id) + plan["queue_order"] = [first_id, second_id] + state_mod.save_state(state, state_file) + save_plan(plan, plan_file) + before_plan = load_plan(plan_file) + monkeypatch.setattr(plan_persistence, "PLAN_FILE", plan_file) + + runtime = CommandRuntime( + config={}, + state=copy.deepcopy(state), + state_path=state_file, + ) + override_resolve.cmd_plan_resolve( + argparse.Namespace( + runtime=runtime, + patterns=[second_id], + note="Verified the requested item but it is not at the front of the plan queue", + attest=_ATTEST, + confirm=False, + force_resolve=False, + state=state_file, + lang=None, + path=".", + exclude=None, + ) + ) + + assert "Queue order violation" in capsys.readouterr().out + state_after = state_mod.load_state(state_file) + plan_after = load_plan(plan_file) + assert state_after["work_items"][second_id]["status"] == "open" + assert plan_after["queue_order"] == before_plan["queue_order"] + assert plan_after["execution_log"] == before_plan["execution_log"] diff --git a/desloppify/tests/commands/plan/test_plan_overrides_direct.py b/desloppify/tests/commands/plan/test_plan_overrides_direct.py index d1b5e4c05..9f9cd378e 100644 --- a/desloppify/tests/commands/plan/test_plan_overrides_direct.py +++ b/desloppify/tests/commands/plan/test_plan_overrides_direct.py @@ -101,7 +101,6 @@ def test_override_resolve_cmd_confirm_allows_small_cluster(monkeypatch) -> None: } plan = {"clusters": {"small": {"issue_ids": ["i1", "i2"]}}} delegated: list[argparse.Namespace] = [] - log_entries: list[dict] = [] monkeypatch.setattr( override_resolve_cmd_mod, @@ -109,12 +108,6 @@ def test_override_resolve_cmd_confirm_allows_small_cluster(monkeypatch) -> None: lambda _args: SimpleNamespace(state=state), ) monkeypatch.setattr(override_resolve_cmd_mod, "load_plan", lambda: plan) - monkeypatch.setattr( - override_resolve_cmd_mod, - "append_log_entry", - lambda *_args, **kwargs: log_entries.append(kwargs), - ) - monkeypatch.setattr(override_resolve_cmd_mod, "save_plan", lambda _plan: None) monkeypatch.setattr(override_resolve_cmd_mod, "cmd_resolve", delegated.append) override_resolve_cmd_mod.cmd_plan_resolve( @@ -135,7 +128,6 @@ def test_override_resolve_cmd_confirm_allows_small_cluster(monkeypatch) -> None: assert delegated[0].patterns == ["small"] assert delegated[0].status == "fixed" assert delegated[0].attest.startswith("I have actually resolved the small cluster") - assert log_entries[0]["cluster_name"] == "small" def test_override_resolve_cmd_confirm_requires_note(capsys) -> None: diff --git a/desloppify/tests/commands/plan/test_workflow_gates.py b/desloppify/tests/commands/plan/test_workflow_gates.py index 5184a8c3f..0900e6b35 100644 --- a/desloppify/tests/commands/plan/test_workflow_gates.py +++ b/desloppify/tests/commands/plan/test_workflow_gates.py @@ -16,12 +16,11 @@ import desloppify.app.commands.plan.override.misc as misc_mod import desloppify.app.commands.plan.override.resolve_cmd as resolve_mod import desloppify.app.commands.plan.override.resolve_workflow as resolve_workflow_mod -from desloppify.engine._plan.schema import empty_plan from desloppify.engine._plan.constants import ( WORKFLOW_CREATE_PLAN_ID, WORKFLOW_SCORE_CHECKPOINT_ID, ) - +from desloppify.engine._plan.schema import empty_plan # --------------------------------------------------------------------------- # Helpers @@ -101,7 +100,12 @@ def _mock_plan_io(monkeypatch, plan): monkeypatch.setattr(resolve_workflow_mod, "load_plan", lambda *a, **kw: plan) monkeypatch.setattr(misc_mod, "load_plan", lambda *a, **kw: plan) saved = [] - monkeypatch.setattr(resolve_mod, "save_plan", lambda p, *a, **kw: saved.append(p)) + monkeypatch.setattr( + resolve_mod, + "save_plan", + lambda p, *a, **kw: saved.append(p), + raising=False, + ) monkeypatch.setattr(resolve_workflow_mod, "save_plan", lambda p, *a, **kw: saved.append(p)) monkeypatch.setattr(misc_mod, "save_plan", lambda p, *a, **kw: saved.append(p)) return saved