From e6f250ebecbb58c0f4f73c7e31fc6637c778b8a8 Mon Sep 17 00:00:00 2001
From: Tristan Manchester <tmanchester96@gmail.com>
Date: Fri, 17 Apr 2026 09:00:29 +0200
Subject: [PATCH] Fix plan key migration data loss

---
 desloppify/engine/_plan/schema/__init__.py        |  2 +-
 desloppify/engine/_plan/schema/normalize.py       |  2 +-
 .../tests/plan/test_persistence_runtime_paths.py  | 14 ++++++++++++++
 desloppify/tests/plan/test_schema_migrations.py   | 15 +++++++++++++++
 4 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/desloppify/engine/_plan/schema/__init__.py b/desloppify/engine/_plan/schema/__init__.py
index 900198495..146afaa92 100644
--- a/desloppify/engine/_plan/schema/__init__.py
+++ b/desloppify/engine/_plan/schema/__init__.py
@@ -289,10 +289,10 @@ def ensure_plan_defaults(plan: dict[str, Any]) -> None:
 
     Runtime contract is v8-only. Legacy payloads are upgraded in-place once.
     """
+    _upgrade_plan_to_v8(plan)
     defaults = empty_plan()
     for key, value in defaults.items():
         plan.setdefault(key, value)
-    _upgrade_plan_to_v8(plan)
     subjective_defer_meta = plan.get("subjective_defer_meta")
     if isinstance(subjective_defer_meta, dict):
         subjective_defer_meta.pop("force_visible_ids", None)
diff --git a/desloppify/engine/_plan/schema/normalize.py b/desloppify/engine/_plan/schema/normalize.py
index 820b38a60..72fa83505 100644
--- a/desloppify/engine/_plan/schema/normalize.py
+++ b/desloppify/engine/_plan/schema/normalize.py
@@ -13,7 +13,7 @@
 def _rename_key(d: dict, old: str, new: str) -> bool:
     if old not in d:
         return False
-    d.setdefault(new, d.pop(old))
+    d[new] = d.pop(old)
     return True
 
 
diff --git a/desloppify/tests/plan/test_persistence_runtime_paths.py b/desloppify/tests/plan/test_persistence_runtime_paths.py
index 72370aafa..63aa42081 100644
--- a/desloppify/tests/plan/test_persistence_runtime_paths.py
+++ b/desloppify/tests/plan/test_persistence_runtime_paths.py
@@ -82,3 +82,17 @@ def test_resolve_plan_load_status_migrates_legacy_lifecycle_in_memory_only(tmp_p
     assert json.loads(plan_file.read_text(encoding="utf-8"))["refresh_state"][
         "lifecycle_phase"
     ] == "workflow"
+
+
+def test_resolve_plan_load_status_preserves_legacy_uncommitted_findings(tmp_path):
+    plan_file = tmp_path / "plan.json"
+    plan_file.write_text(
+        '{"version": 7, "created": "2026-01-01T00:00:00+00:00", "updated": "2026-01-01T00:00:00+00:00", "queue_order": [], "deferred": [], "skipped": {}, "active_cluster": null, "overrides": {}, "clusters": {}, "superseded": {}, "promoted_ids": [], "plan_start_scores": {}, "refresh_state": {}, "execution_log": [], "epic_triage_meta": {}, "commit_log": [], "uncommitted_findings": ["review::a.py::issue-1"], "uncommitted_issues": [], "commit_tracking_branch": null}\n',
+        encoding="utf-8",
+    )
+
+    status = persistence_mod.resolve_plan_load_status(plan_file)
+
+    assert status.plan is not None
+    assert status.plan["uncommitted_issues"] == ["review::a.py::issue-1"]
+    assert "uncommitted_findings" not in status.plan
diff --git a/desloppify/tests/plan/test_schema_migrations.py b/desloppify/tests/plan/test_schema_migrations.py
index b6e344159..2df5be0db 100644
--- a/desloppify/tests/plan/test_schema_migrations.py
+++ b/desloppify/tests/plan/test_schema_migrations.py
@@ -28,6 +28,21 @@ def test_ensure_container_types_sets_defaults_and_renames_keys() -> None:
     assert plan["commit_tracking_branch"] is None
 
 
+def test_ensure_container_types_rename_overwrites_existing_target_key() -> None:
+    plan = {
+        "epic_triage_meta": {"finding_snapshot_hash": "abc", "issue_snapshot_hash": ""},
+        "uncommitted_findings": ["x"],
+        "uncommitted_issues": [],
+    }
+
+    migrations.ensure_container_types(plan)
+
+    assert plan["epic_triage_meta"]["issue_snapshot_hash"] == "abc"
+    assert "finding_snapshot_hash" not in plan["epic_triage_meta"]
+    assert plan["uncommitted_issues"] == ["x"]
+    assert "uncommitted_findings" not in plan
+
+
 def test_migrate_synthesis_to_triage_renames_ids_meta_and_cluster_fields() -> None:
     plan = {
         "queue_order": ["synthesis::a", "other"],