fix: keep rejected plan resolves transactional

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,23 +13,18 @@
 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,
     split_synthetic_patterns,
 )
 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,

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"]

desloppify/tests/commands/plan/test_plan_overrides_direct.py

@@ -101,20 +101,13 @@ 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,
         "command_runtime",
         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:

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