diff --git a/loopx/state_migration.py b/loopx/state_migration.py index cbd9fc2a18..d11c0a44f9 100644 --- a/loopx/state_migration.py +++ b/loopx/state_migration.py @@ -135,6 +135,37 @@ def copy_rewritten_text_file(source: Path, target: Path, *, goal_id_map: dict[st atomic_write_state_text(target, rewrite_text(text, goal_id_map=goal_id_map, path_map=path_map)) +def _planned_state_bytes(source: Path, *, goal_id_map: dict[str, str], path_map: dict[str, str]) -> bytes: + """The exact bytes ``copy_rewritten_text_file`` publishes for ``source``.""" + try: + text = source.read_text(encoding="utf-8") + except UnicodeDecodeError: + return source.read_bytes() + return rewrite_text(text, goal_id_map=goal_id_map, path_map=path_map).encode("utf-8") + + +def _require_no_active_state_clobber( + source_path: Path, + target_path: Path, + *, + goal_id_map: dict[str, str], + path_map: dict[str, str], +) -> None: + """Refuse to replace a different live target state with the legacy copy. + + A repeated migration (for example ``--all-goals`` after more legacy Goals + appear) must not roll back Todos completed or added at the target since the + first copy. Identical content and in-place rewrites remain idempotent. + """ + if not target_path.exists() or source_path.resolve() == target_path.resolve(): + return + if target_path.read_bytes() != _planned_state_bytes(source_path, goal_id_map=goal_id_map, path_map=path_map): + raise ValueError( + f"migrated active state already exists with different content: {target_path}; " + "refusing to overwrite it with the legacy copy" + ) + + def copy_active_state_files( pairs: list[tuple[dict[str, Any], dict[str, Any]]], *, @@ -142,10 +173,23 @@ def copy_active_state_files( path_map: dict[str, str], execute: bool, ) -> list[dict[str, Any]]: + resolved_pairs = [ + ( + source_goal, + target_goal, + resolve_goal_state(source_goal.get("repo"), source_goal.get("state_file")), + resolve_goal_state(target_goal.get("repo"), target_goal.get("state_file")), + ) + for source_goal, target_goal in pairs + ] + # Check every target before the first copy so a conflict leaves no partial write. + for _source_goal, _target_goal, source_path, target_path in resolved_pairs: + if source_path and target_path and source_path.exists(): + _require_no_active_state_clobber( + source_path, target_path, goal_id_map=goal_id_map, path_map=path_map + ) results: list[dict[str, Any]] = [] - for source_goal, target_goal in pairs: - source_path = resolve_goal_state(source_goal.get("repo"), source_goal.get("state_file")) - target_path = resolve_goal_state(target_goal.get("repo"), target_goal.get("state_file")) + for _source_goal, target_goal, source_path, target_path in resolved_pairs: row: dict[str, Any] = { "goal_id": target_goal.get("id"), "source": str(source_path) if source_path else None, diff --git a/tests/test_state_migration.py b/tests/test_state_migration.py index bf46235a7d..4661e08a23 100644 --- a/tests/test_state_migration.py +++ b/tests/test_state_migration.py @@ -76,3 +76,70 @@ def test_migration_rejects_target_goal_id_collisions_before_writing( for target_repo in target_repos.values() ) assert not (target_runtime / "goals" / "goal-b").exists() + + +def _active_state_migration_fixture(tmp_path: Path) -> dict[str, Path]: + repo = tmp_path / "repo" + source_state = repo / ".codex" / "goals" / "goal-a" / "ACTIVE_GOAL_STATE.md" + source_state.parent.mkdir(parents=True) + source_state.write_text("## Agent Todo\n\n- [ ] legacy todo\n", encoding="utf-8") + legacy_registry = tmp_path / "legacy-registry.json" + legacy_registry.write_text( + json.dumps( + { + "schema_version": "0.1", + "goals": [ + { + "id": "goal-a", + "status": "active", + "repo": str(repo), + "state_file": ".codex/goals/goal-a/ACTIVE_GOAL_STATE.md", + } + ], + } + ), + encoding="utf-8", + ) + return { + "repo": repo, + "legacy_registry": legacy_registry, + "target_registry": repo / ".loopx" / "registry.json", + "target_state": repo / ".loopx" / "goals" / "goal-a" / "ACTIVE_GOAL_STATE.md", + "target_runtime": tmp_path / "target-runtime", + } + + +def _migrate_active_state(paths: dict[str, Path], tmp_path: Path, *, execute: bool) -> dict: + return migrate_legacy_state( + legacy_registry_path=paths["legacy_registry"], + target_registry_path=paths["target_registry"], + legacy_runtime_root=tmp_path / "legacy-runtime", + target_runtime_root=paths["target_runtime"], + goal_ids=["goal-a"], + goal_id_map={}, + path_map={".codex/goals": ".loopx/goals"}, + copy_active_state=True, + copy_runtime=False, + execute=execute, + ) + + +def test_repeated_migration_does_not_overwrite_diverged_target_active_state( + tmp_path: Path, +) -> None: + paths = _active_state_migration_fixture(tmp_path) + first = _migrate_active_state(paths, tmp_path, execute=True) + assert first["active_state"][0]["copied"] is True + # An identical rerun is idempotent. + assert _migrate_active_state(paths, tmp_path, execute=True)["ok"] is True + + progressed = "## Agent Todo\n\n- [x] legacy todo\n- [ ] work added after migration\n" + paths["target_state"].write_text(progressed, encoding="utf-8") + registry_before = paths["target_registry"].read_bytes() + + for execute in (False, True): + with pytest.raises(ValueError, match="already exists with different content"): + _migrate_active_state(paths, tmp_path, execute=execute) + + assert paths["target_state"].read_text(encoding="utf-8") == progressed + assert paths["target_registry"].read_bytes() == registry_before