From efd3d92f8ed9e2011679e789ee727324898faefd Mon Sep 17 00:00:00 2001 From: gaoguobin <31329849+gaoguobin@users.noreply.github.com> Date: Wed, 20 May 2026 14:51:38 +0800 Subject: [PATCH 1/2] Fix backup discovery ordering across default and fallback roots --- README.md | 2 +- .../claude-code-environment-backup/SKILL.md | 3 + skills/codex-environment-backup/SKILL.md | 3 + src/agent_environment_backup/core.py | 63 +++++++++++----- tests/test_core.py | 74 ++++++++++++++++++- 5 files changed, 124 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index e5f30cb..b119b17 100644 --- a/README.md +++ b/README.md @@ -224,7 +224,7 @@ Backups are written under `~/Documents/CodexBackups` by default: ~/Documents/CodexBackups/codex-backup-YYYYMMDD-HHMMSS.tar.gz.sha256 ``` -If `~/Documents` already exists but is not a writable directory, the CLI falls +If `~/Documents` already exists but is not a directory, the CLI falls back to `~/CodexBackups`. For Claude Code, the default home is `~/.claude` and backups are written under diff --git a/skills/claude-code-environment-backup/SKILL.md b/skills/claude-code-environment-backup/SKILL.md index c31e2c5..ed2bb92 100644 --- a/skills/claude-code-environment-backup/SKILL.md +++ b/skills/claude-code-environment-backup/SKILL.md @@ -141,3 +141,6 @@ Initial versions are manual-trigger only. If the user asks for periodic backups, - Prefer exact paths from JSON output. - Do not infer that restore succeeded from natural language alone; use the restore JSON, restored file count, errors list, and post-restore structural doctor report. - For install/update/uninstall, report the executed instruction source and the final structural doctor/status result. + + +> Validation note: when you are testing across sandbox or permission boundaries, pass the same explicit `--backup-root` to `backup`, `list-backups`, and `restore` so discovery stays deterministic. diff --git a/skills/codex-environment-backup/SKILL.md b/skills/codex-environment-backup/SKILL.md index 13896e9..ace137e 100644 --- a/skills/codex-environment-backup/SKILL.md +++ b/skills/codex-environment-backup/SKILL.md @@ -142,3 +142,6 @@ Initial versions are manual-trigger only. If the user asks for periodic backups, - Prefer exact paths from JSON output. - Do not infer that restore succeeded from natural language alone; use the restore JSON, restored file count, errors list, and post-restore structural doctor report. - For install/update/uninstall, report the executed instruction source and the final structural doctor/status result. + + +> Validation note: when you are testing across sandbox or permission boundaries, pass the same explicit `--backup-root` to `backup`, `list-backups`, and `restore` so discovery stays deterministic. diff --git a/src/agent_environment_backup/core.py b/src/agent_environment_backup/core.py index 7f4326b..cbacca0 100644 --- a/src/agent_environment_backup/core.py +++ b/src/agent_environment_backup/core.py @@ -104,11 +104,21 @@ def default_backup_root(profile: EnvironmentProfile | None = None) -> Path: profile = CODEX_PROFILE home = Path.home() documents = home / "Documents" - if documents.exists() and (not documents.is_dir() or not os.access(documents, os.W_OK)): + if documents.exists() and not documents.is_dir(): return (home / profile.default_backup_subdir).resolve() return (documents / profile.default_backup_subdir).resolve() + +def default_backup_roots_for_discovery(profile: EnvironmentProfile | None = None) -> list[Path]: + if profile is None: + profile = CODEX_PROFILE + primary = default_backup_root(profile) + fallback = (Path.home() / profile.default_backup_subdir).resolve() + if fallback == primary: + return [primary] + return [primary, fallback] + def is_relative_to(child: Path, parent: Path) -> bool: try: child.resolve().relative_to(parent.resolve()) @@ -1933,21 +1943,40 @@ def list_backups( ) -> dict[str, Any]: if profile is None: profile = CODEX_PROFILE - root = Path(backup_root).expanduser().resolve() if backup_root else default_backup_root(profile) + roots = [Path(backup_root).expanduser().resolve()] if backup_root else default_backup_roots_for_discovery(profile) + root = roots[0] items: list[dict[str, Any]] = [] - if not root.exists(): - return {"ok": True, "backup_root": str(root), "backups": items} - for manifest in sorted(root.rglob("manifest.json")): - try: - data = json.loads(manifest.read_text(encoding="utf-8")) - except Exception as exc: - items.append( - { - "backup_dir": str(manifest.parent), - "status": "unreadable", - "error": str(exc), - } - ) + seen: set[Path] = set() + for candidate_root in roots: + if not candidate_root.exists(): continue - items.append(backup_list_item(manifest, data)) - return {"ok": True, "backup_root": str(root), "backups": items} + for manifest in sorted(candidate_root.rglob("manifest.json")): + resolved_manifest = manifest.resolve() + if resolved_manifest in seen: + continue + seen.add(resolved_manifest) + try: + data = json.loads(manifest.read_text(encoding="utf-8")) + except Exception as exc: + items.append( + { + "backup_dir": str(manifest.parent), + "status": "unreadable", + "error": str(exc), + } + ) + continue + items.append(backup_list_item(manifest, data)) + + items.sort( + key=lambda item: ( + item.get("created_at") or "", + item.get("backup_dir") or "", + ), + reverse=True, + ) + + result: dict[str, Any] = {"ok": True, "backup_root": str(root), "backups": items} + if not backup_root and len(roots) > 1: + result["discovery_roots"] = [str(candidate) for candidate in roots] + return result diff --git a/tests/test_core.py b/tests/test_core.py index bc28281..d0ce155 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -682,10 +682,10 @@ def test_default_backup_root_falls_back_when_documents_is_not_usable(self) -> No self.assertEqual(codex_root, (fake_home / "CodexBackups").resolve()) self.assertEqual(claude_root, (fake_home / "ClaudeCodeBackups").resolve()) - def test_default_backup_root_falls_back_when_documents_is_not_writable(self) -> None: + def test_default_backup_root_ignores_writability_checks(self) -> None: from agent_environment_backup.core import default_backup_root, CODEX_PROFILE with self.temp_root() as temp_dir: - fake_home = Path(temp_dir) / "home-with-unwritable-documents" + fake_home = Path(temp_dir) / "home-with-documents" documents = fake_home / "Documents" documents.mkdir(parents=True) with ( @@ -693,7 +693,75 @@ def test_default_backup_root_falls_back_when_documents_is_not_writable(self) -> mock.patch.object(core_module.os, "access", return_value=False), ): result = default_backup_root(CODEX_PROFILE) - self.assertEqual(result, (fake_home / "CodexBackups").resolve()) + self.assertEqual(result, (fake_home / "Documents" / "CodexBackups").resolve()) + + + def test_list_backups_discovers_legacy_home_fallback_root(self) -> None: + from agent_environment_backup.core import list_backups, CODEX_PROFILE + with self.temp_root() as temp_dir: + fake_home = Path(temp_dir) / "home-for-discovery" + docs_root = fake_home / "Documents" / "CodexBackups" + fallback_root = fake_home / "CodexBackups" + docs_root.mkdir(parents=True) + backup_dir = fallback_root / "codex-backup-20260520-141125" + backup_dir.mkdir(parents=True) + (backup_dir / "manifest.json").write_text( + json.dumps( + { + "schema_version": 1, + "created_at": "2026-05-20T14:11:25Z", + "source_home": str(fake_home / ".codex"), + "backup_label": "codex-backup-20260520-141125", + "profile": "codex", + } + ), + encoding="utf-8", + ) + with mock.patch.object(core_module.Path, "home", return_value=fake_home): + listing = list_backups(profile=CODEX_PROFILE) + self.assertTrue(listing["ok"]) + self.assertEqual(listing["backup_root"], str(docs_root.resolve())) + self.assertIn(str(fallback_root.resolve()), listing.get("discovery_roots", [])) + self.assertEqual(len(listing["backups"]), 1) + self.assertTrue(listing["backups"][0]["backup_dir"].endswith("codex-backup-20260520-141125")) + + + def test_list_backups_default_discovery_is_sorted_by_created_at_desc(self) -> None: + from agent_environment_backup.core import list_backups, CODEX_PROFILE + with self.temp_root() as temp_dir: + fake_home = Path(temp_dir) / "home-for-sort" + docs_root = fake_home / "Documents" / "CodexBackups" + fallback_root = fake_home / "CodexBackups" + first = docs_root / "codex-backup-older" + second = fallback_root / "codex-backup-newer" + first.mkdir(parents=True) + second.mkdir(parents=True) + (first / "manifest.json").write_text( + json.dumps( + { + "schema_version": 1, + "created_at": "2026-05-20T14:00:00Z", + "source_home": str(fake_home / ".codex"), + "profile": "codex", + } + ), + encoding="utf-8", + ) + (second / "manifest.json").write_text( + json.dumps( + { + "schema_version": 1, + "created_at": "2026-05-20T14:11:25Z", + "source_home": str(fake_home / ".codex"), + "profile": "codex", + } + ), + encoding="utf-8", + ) + with mock.patch.object(core_module.Path, "home", return_value=fake_home): + listing = list_backups(profile=CODEX_PROFILE) + self.assertEqual(len(listing["backups"]), 2) + self.assertTrue(listing["backups"][0]["backup_dir"].endswith("codex-backup-newer")) def test_inspect_claude_code_config(self) -> None: from agent_environment_backup.core import inspect_claude_code_config From caeec86b068376ed4da53f2664bfd6d457dff648 Mon Sep 17 00:00:00 2001 From: gaoguobin <31329849+gaoguobin@users.noreply.github.com> Date: Wed, 20 May 2026 14:59:53 +0800 Subject: [PATCH 2/2] Fix list_backups sorting for non-string created_at values --- src/agent_environment_backup/core.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/agent_environment_backup/core.py b/src/agent_environment_backup/core.py index cbacca0..2803cb2 100644 --- a/src/agent_environment_backup/core.py +++ b/src/agent_environment_backup/core.py @@ -1968,13 +1968,14 @@ def list_backups( continue items.append(backup_list_item(manifest, data)) - items.sort( - key=lambda item: ( - item.get("created_at") or "", - item.get("backup_dir") or "", - ), - reverse=True, - ) + def _sort_key(item: dict[str, Any]) -> tuple[str, str]: + created_at = item.get("created_at") + created_key = created_at if isinstance(created_at, str) else ("" if created_at is None else str(created_at)) + backup_dir = item.get("backup_dir") + backup_dir_key = backup_dir if isinstance(backup_dir, str) else ("" if backup_dir is None else str(backup_dir)) + return (created_key, backup_dir_key) + + items.sort(key=_sort_key, reverse=True) result: dict[str, Any] = {"ok": True, "backup_root": str(root), "backups": items} if not backup_root and len(roots) > 1: