diff --git a/devlaunch/worktree/models.py b/devlaunch/worktree/models.py index 43857674..5dce57a4 100644 --- a/devlaunch/worktree/models.py +++ b/devlaunch/worktree/models.py @@ -1,9 +1,31 @@ """Data models for worktree backend.""" -from dataclasses import asdict, dataclass, field +from dataclasses import asdict, dataclass, field, fields from datetime import datetime from pathlib import Path -from typing import Dict, List, Optional +from typing import Any, Dict, List, Optional + + +def unknown_fields(model: Any, data: Dict) -> List[str]: + """Return the keys in ``data`` that ``model`` declares no field for. + + A newer devlaunch writing an extra field is the expected source. Callers use + this to report what a rewrite would drop; ``from_dict`` ignores those keys. + """ + known = {f.name for f in fields(model)} + return sorted(key for key in data if key not in known) + + +def _drop_unknown(model: Any, data: Dict) -> Dict: + """Copy ``data`` keeping only the fields ``model`` declares. + + Ignoring an unrecognized field keeps the entry loadable. Rebuilding it with + ``cls(**data)`` instead would raise TypeError, and since one stored field is + shaped like every other, a single field added by a newer build would make + every entry unreadable at once. + """ + known = {f.name for f in fields(model)} + return {key: value for key, value in data.items() if key in known} @dataclass @@ -27,8 +49,8 @@ def to_dict(self) -> Dict: @classmethod def from_dict(cls, data: Dict) -> "BaseRepository": - """Create from dictionary.""" - data = data.copy() + """Create from dictionary, ignoring fields this build does not declare.""" + data = _drop_unknown(cls, data) data["local_path"] = Path(data["local_path"]) if data.get("last_fetched"): data["last_fetched"] = datetime.fromisoformat(data["last_fetched"]) @@ -58,8 +80,8 @@ def to_dict(self) -> Dict: @classmethod def from_dict(cls, data: Dict) -> "WorktreeInfo": - """Create from dictionary.""" - data = data.copy() + """Create from dictionary, ignoring fields this build does not declare.""" + data = _drop_unknown(cls, data) data["local_path"] = Path(data["local_path"]) data["created_at"] = datetime.fromisoformat(data["created_at"]) data["last_used"] = datetime.fromisoformat(data["last_used"]) diff --git a/devlaunch/worktree/storage.py b/devlaunch/worktree/storage.py index 2deb1f6c..c25f60be 100644 --- a/devlaunch/worktree/storage.py +++ b/devlaunch/worktree/storage.py @@ -1,11 +1,28 @@ """Storage utilities for worktree metadata.""" +import contextlib import json import os +import shutil +import stat +import sys +import tempfile from pathlib import Path -from typing import Dict, List, Optional +from typing import Any, Dict, List, Optional, Tuple -from .models import BaseRepository, WorktreeInfo +from .models import BaseRepository, WorktreeInfo, unknown_fields + +# Version of the on-disk metadata.json format. A file without a "version" key +# predates versioning and is treated as version 1. +SCHEMA_VERSION = 1 + +# Top-level keys this build writes, and therefore the only ones a rewrite keeps. +_KNOWN_SECTIONS = frozenset({"version", "repositories", "worktrees"}) + +# Errors raised when a single stored entry cannot be rebuilt into a model: +# KeyError for a missing field, TypeError for an unknown/bad-typed field +# (from_dict does cls(**data)), ValueError for an unparseable timestamp. +_ENTRY_ERRORS = (KeyError, TypeError, ValueError) def _get_default_metadata_path() -> Path: @@ -16,6 +33,33 @@ def _get_default_metadata_path() -> Path: return Path.home() / ".cache" / "devlaunch" / "metadata.json" +def _warn(message: str) -> None: + """Emit a single warning line on stderr (stdout is parsed by completions).""" + print(f"dl: {message}", file=sys.stderr) + + +def _file_mode(path: Path) -> Optional[int]: + """Return the permission bits of ``path``, or None if it does not exist.""" + try: + return stat.S_IMODE(path.stat().st_mode) + except OSError: + return None + + +def _resolve_link(path: Path) -> Path: + """Return the real file behind ``path``, following it if it is a symlink. + + Only the final component is resolved. Writing atomically means renaming a + temp file over the target, which would replace a symlink with a regular + file; anyone who points metadata.json at a synced directory would silently + lose the link and every later write. Resolving once up front keeps every + file operation (write, quarantine, backup) on the real file. + """ + if path.is_symlink(): + return Path(os.path.realpath(path)) + return path + + class MetadataStorage: """Handles persistent storage of worktree metadata.""" @@ -25,36 +69,209 @@ def __init__(self, metadata_path: Optional[Path] = None): metadata_path = _get_default_metadata_path() self.metadata_path = metadata_path self.metadata_path.parent.mkdir(parents=True, exist_ok=True) + # Every file operation targets the real file, not a symlink pointing at it. + self._file_path = _resolve_link(self.metadata_path) self._load() - def _load(self) -> None: - """Load metadata from disk.""" - if self.metadata_path.exists(): - with open(self.metadata_path, "r", encoding="utf-8") as f: + def _quarantine(self, reason: str) -> None: + """Move an unusable metadata file aside so the data stays inspectable. + + A single quarantine slot is used, overwritten on repeat corruption. + """ + corrupt_path = self._file_path.with_name(self._file_path.name + ".corrupt") + try: + self._file_path.replace(corrupt_path) + except OSError as exc: + _warn( + f"{reason}; could not move it aside to {corrupt_path} ({exc}); " + "starting with empty metadata" + ) + else: + _warn(f"{reason}; moved it to {corrupt_path} and started with empty metadata") + + def _read_file(self) -> Optional[Dict[str, Any]]: + """Read and sanity-check the metadata file, quarantining it if unusable.""" + if not self._file_path.exists(): + return None + try: + with open(self._file_path, "r", encoding="utf-8") as f: data = json.load(f) + # ValueError covers both json.JSONDecodeError (a ValueError subclass) and + # the UnicodeDecodeError that non-UTF-8 bytes raise from inside json.load. + except (OSError, ValueError) as exc: + self._quarantine(f"could not read metadata file {self._file_path} ({exc})") + return None + if not isinstance(data, dict): + self._quarantine( + f"metadata file {self._file_path} is not a JSON object " + f"(found {type(data).__name__})" + ) + return None + return data + + def _backup(self) -> None: + """Copy the on-disk file aside before a lossy rewrite can overwrite it. + + This runs at load time, while the original bytes are still on disk: the + next mutation rewrites the file from what was loaded, so anything _load + could not round-trip is gone by then. A single backup slot is used, + overwritten on repeat, kept separate from the quarantine slot so the two + recovery cases cannot clobber each other. + """ + backup_path = self._file_path.with_name(self._file_path.name + ".bak") + reason = ( + f"rewriting {self._file_path} in this build's format will drop " + "information it currently holds" + ) + try: + shutil.copy2(self._file_path, backup_path) + except OSError as exc: + _warn(f"{reason}; could not preserve the original at {backup_path} ({exc})") + else: + _warn(f"{reason}; preserved the original at {backup_path}") + + def _load_section( + self, data: Dict[str, Any], section: str, model: Any + ) -> Tuple[Dict[str, Any], bool]: + """Rebuild one section, skipping (not discarding) individually broken entries. + + Returns the loaded entries and whether anything stored was left behind -- + an entry that could not be rebuilt at all, or one carrying a field this + build does not declare and so would drop on the next write. + """ + entries = data.get(section, {}) + if not isinstance(entries, dict): + _warn( + f'ignoring the "{section}" section of {self._file_path}: ' + f"expected an object, found {type(entries).__name__}" + ) + return {}, True + + loaded: Dict[str, Any] = {} + lossy = False + for key, entry in entries.items(): + if not isinstance(entry, dict): + lossy = True + _warn( + f"skipping malformed {section} entry {key!r} in {self._file_path}: " + f"expected an object, found {type(entry).__name__}" + ) + continue + try: + loaded[key] = model.from_dict(entry) + except _ENTRY_ERRORS as exc: + lossy = True + _warn(f"skipping malformed {section} entry {key!r} in {self._file_path}: {exc!r}") + continue + # The entry loaded, but a field only a newer build knows about is not + # carried into the rebuilt model and disappears on the next write. + extra = unknown_fields(model, entry) + if extra: + lossy = True + _warn( + f"{section} entry {key!r} in {self._file_path} has field(s) this build " + f"does not understand ({', '.join(extra)}); they are dropped when it " + "is rewritten" + ) + return loaded, lossy + + def _load_version(self, data: Dict[str, Any]) -> Tuple[int, bool]: + """Interpret the version header, returning the version and whether it is lossy.""" + if "version" not in data: + # An absent version means a legacy pre-versioning file: same shape as v1. + return SCHEMA_VERSION, False + + raw = data["version"] + # JSON has a single number type, so tools freely normalize 1 to 1.0; an + # integral number is that version. bool is an int subclass in Python, but + # a true/false header is nonsense rather than version 1. + if isinstance(raw, int) and not isinstance(raw, bool): + version = raw + elif isinstance(raw, float) and raw.is_integer(): + version = int(raw) else: - data = {"repositories": {}, "worktrees": {}} + # The entries do not depend on the header, so never discard them over + # it: warn, read the file as legacy v1, and preserve the original + # because the rewritten header will not match what is there now. + _warn( + f'metadata file {self._file_path} has an invalid "version" header ' + f"({raw!r}); reading it as schema version {SCHEMA_VERSION}" + ) + return SCHEMA_VERSION, True + + if version > SCHEMA_VERSION: + _warn( + f"{self._file_path} was written by a newer devlaunch (schema version " + f"{version}, this build understands {SCHEMA_VERSION}); its entries are " + f"loaded as-is, and the next change rewrites the whole file as schema " + f"version {SCHEMA_VERSION}" + ) + return version, True + # A version below SCHEMA_VERSION is an older shape, upgraded on the next + # write; the value is exposed unchanged so a migration can branch on it. + return version, False + + def _load(self) -> None: + """Load metadata from disk, never raising on damaged input.""" self.repositories: Dict[str, BaseRepository] = {} self.worktrees: Dict[str, WorktreeInfo] = {} + self.schema_version: int = SCHEMA_VERSION - # Load repositories - for key, repo_data in data.get("repositories", {}).items(): - self.repositories[key] = BaseRepository.from_dict(repo_data) + data = self._read_file() + if data is None: + return - # Load worktrees - for key, worktree_data in data.get("worktrees", {}).items(): - self.worktrees[key] = WorktreeInfo.from_dict(worktree_data) + self.schema_version, version_is_lossy = self._load_version(data) + self.repositories, repos_skipped = self._load_section(data, "repositories", BaseRepository) + self.worktrees, worktrees_skipped = self._load_section(data, "worktrees", WorktreeInfo) + + unknown = sorted(set(data) - _KNOWN_SECTIONS) + if unknown: + _warn( + f"{self._file_path} has top-level key(s) this build does not understand " + f"({', '.join(unknown)}); they are dropped when it is rewritten" + ) + + if version_is_lossy or repos_skipped or worktrees_skipped or unknown: + self._backup() def save(self) -> None: - """Save metadata to disk.""" + """Save metadata to disk atomically. + + Writes to a fresh temp file, fsyncs it, then renames it over the real + path, so an interrupted write can never leave a truncated metadata.json + behind. Write failures are deliberately not swallowed: silently losing + workspace metadata is worse than an error. + """ data = { + "version": SCHEMA_VERSION, "repositories": {key: repo.to_dict() for key, repo in self.repositories.items()}, "worktrees": {key: worktree.to_dict() for key, worktree in self.worktrees.items()}, } - with open(self.metadata_path, "w", encoding="utf-8") as f: - json.dump(data, f, indent=2) + # mkstemp gives a name no other writer can be holding, in the same + # directory so the rename stays atomic, created 0600 so the contents are + # never briefly world-readable. + fd, temp_name = tempfile.mkstemp( + dir=self._file_path.parent, prefix=f"{self._file_path.name}.", suffix=".tmp" + ) + temp_path = Path(temp_name) + try: + with os.fdopen(fd, "w", encoding="utf-8") as f: + json.dump(data, f, indent=2) + f.flush() + os.fsync(f.fileno()) + # Renaming a fresh file would otherwise reset the mode to the umask + # default, silently widening a metadata.json the user locked down. + mode = _file_mode(self._file_path) + if mode is not None: + os.chmod(temp_path, mode) + temp_path.replace(self._file_path) + finally: + # No-op after a successful rename; cleans up a failed write. + with contextlib.suppress(OSError): + temp_path.unlink(missing_ok=True) def add_repository(self, repo: BaseRepository) -> None: """Add or update a repository.""" @@ -83,11 +300,11 @@ def add_worktree(self, worktree: WorktreeInfo) -> None: key = f"{worktree.owner}/{worktree.repo}/{worktree.branch}" self.worktrees[key] = worktree - # Update repository's worktree list + # Update repository's worktree list in memory, then write once. repo = self.get_repository(worktree.owner, worktree.repo) if repo and worktree.branch not in repo.worktrees: repo.worktrees.append(worktree.branch) - self.add_repository(repo) + self.repositories[f"{worktree.owner}/{worktree.repo}"] = repo self.save() @@ -122,10 +339,10 @@ def remove_worktree(self, owner: str, repo: str, branch: str) -> None: if key in self.worktrees: del self.worktrees[key] - # Update repository's worktree list + # Update repository's worktree list in memory, then write once. repo_obj = self.get_repository(owner, repo) if repo_obj and branch in repo_obj.worktrees: repo_obj.worktrees.remove(branch) - self.add_repository(repo_obj) + self.repositories[f"{owner}/{repo}"] = repo_obj self.save() diff --git a/test/test_worktree_storage.py b/test/test_worktree_storage.py index 13d7d757..07ea0c68 100644 --- a/test/test_worktree_storage.py +++ b/test/test_worktree_storage.py @@ -2,14 +2,17 @@ # pylint: disable=redefined-outer-name import json +import os +import stat import tempfile from datetime import datetime from pathlib import Path import pytest +from devlaunch.worktree import storage as storage_module from devlaunch.worktree.models import BaseRepository, WorktreeInfo -from devlaunch.worktree.storage import MetadataStorage +from devlaunch.worktree.storage import SCHEMA_VERSION, MetadataStorage @pytest.fixture @@ -21,6 +24,55 @@ def temp_storage(): yield storage +def _repo_entry(owner="owner1", repo="repo1", **overrides): + """Build a valid on-disk repository entry.""" + entry = { + "owner": owner, + "repo": repo, + "remote_url": f"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/{owner}/{repo}.git", + "local_path": f"/tmp/repos/{owner}/{repo}", + "default_branch": "main", + "last_fetched": None, + "worktrees": [], + } + entry.update(overrides) + return entry + + +def _worktree_entry(owner="owner1", repo="repo1", branch="branch1", **overrides): + """Build a valid on-disk worktree entry.""" + entry = { + "owner": owner, + "repo": repo, + "branch": branch, + "local_path": f"/tmp/worktrees/{owner}/{repo}/{branch}", + "workspace_id": branch, + "created_at": "2024-01-01T10:00:00", + "last_used": "2024-01-01T12:00:00", + "devpod_workspace_id": None, + } + entry.update(overrides) + return entry + + +def _write_metadata(path: Path, text: str) -> None: + """Write raw metadata file content.""" + path.write_text(text, encoding="utf-8") + + +def _record_replacements(monkeypatch) -> list: + """Record the source path of every Path.replace() performed from now on.""" + seen: list = [] + real_replace = Path.replace + + def recording_replace(self, target): + seen.append(Path(self)) + return real_replace(self, target) + + monkeypatch.setattr(Path, "replace", recording_replace) + return seen + + class TestMetadataStorage: """Tests for MetadataStorage class.""" @@ -346,3 +398,724 @@ def test_save_creates_valid_json(self, temp_storage): assert "repositories" in data assert "worktrees" in data assert "test-owner/test-repo" in data["repositories"] + + +class TestCorruptMetadataFile: + """A corrupt metadata.json must never crash the CLI.""" + + @pytest.mark.parametrize( + "content", + [ + pytest.param("{not json", id="invalid-json"), + pytest.param('{"repositories": {"a": {"owner": "a"', id="truncated"), + pytest.param("", id="empty-file"), + pytest.param("[]", id="toplevel-list"), + pytest.param('"x"', id="toplevel-string"), + ], + ) + def test_corrupt_file_loads_empty_and_is_quarantined(self, tmp_path, capsys, content): + """A corrupt file yields empty state, is quarantined, and warns on stderr only.""" + metadata_path = tmp_path / "metadata.json" + _write_metadata(metadata_path, content) + + storage = MetadataStorage(metadata_path) + + assert storage.repositories == {} + assert storage.worktrees == {} + corrupt_path = tmp_path / "metadata.json.corrupt" + assert corrupt_path.exists() + assert corrupt_path.read_text(encoding="utf-8") == content + assert not metadata_path.exists() + + captured = capsys.readouterr() + assert captured.out == "" + assert len(captured.err.strip().splitlines()) == 1 + assert str(corrupt_path) in captured.err + + @pytest.mark.parametrize( + "content", + [ + pytest.param(b'{"a": \x82\xff}', id="invalid-utf8"), + pytest.param('{"repositories": {}, "worktrees": {}}'.encode("utf-16"), id="utf16"), + ], + ) + def test_undecodable_file_loads_empty_and_is_quarantined(self, tmp_path, capsys, content): + """Bytes that are not valid UTF-8 are corruption, not a crash.""" + metadata_path = tmp_path / "metadata.json" + metadata_path.write_bytes(content) + + storage = MetadataStorage(metadata_path) + + assert storage.repositories == {} + assert storage.worktrees == {} + corrupt_path = tmp_path / "metadata.json.corrupt" + assert corrupt_path.read_bytes() == content + assert not metadata_path.exists() + + captured = capsys.readouterr() + assert captured.out == "" + assert len(captured.err.strip().splitlines()) == 1 + assert str(corrupt_path) in captured.err + + def test_quarantine_uses_single_slot(self, tmp_path, capsys): + """Repeated corruption overwrites the single .corrupt slot.""" + metadata_path = tmp_path / "metadata.json" + _write_metadata(metadata_path, "first-corruption") + MetadataStorage(metadata_path) + _write_metadata(metadata_path, "second-corruption") + MetadataStorage(metadata_path) + capsys.readouterr() + + corrupt_files = sorted(p.name for p in tmp_path.iterdir()) + assert corrupt_files == ["metadata.json.corrupt"] + assert (tmp_path / "metadata.json.corrupt").read_text( + encoding="utf-8" + ) == "second-corruption" + + def test_storage_is_usable_after_quarantine(self, tmp_path, capsys): + """After recovering from corruption the storage can still save and reload.""" + metadata_path = tmp_path / "metadata.json" + _write_metadata(metadata_path, "{broken") + storage = MetadataStorage(metadata_path) + capsys.readouterr() + + storage.add_repository( + BaseRepository( + owner="owner1", + repo="repo1", + remote_url="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/owner1/repo1.git", + local_path=Path("/tmp/repos/owner1/repo1"), + ) + ) + + assert MetadataStorage(metadata_path).get_repository("owner1", "repo1") is not None + + +class TestMalformedEntries: + """One bad entry must not cost the user the whole file.""" + + @pytest.mark.parametrize( + "bad_entry", + [ + pytest.param( + {k: v for k, v in _worktree_entry().items() if k != "local_path"}, + id="missing-local-path", + ), + pytest.param( + _worktree_entry(created_at="not-a-timestamp"), id="unparseable-created-at" + ), + pytest.param(_worktree_entry(local_path=None), id="null-local-path"), + pytest.param("not-an-object", id="non-dict-entry"), + ], + ) + def test_bad_worktree_entry_is_skipped(self, tmp_path, capsys, bad_entry): + """Good entries load, only the bad one is skipped, and the file is not quarantined.""" + metadata_path = tmp_path / "metadata.json" + _write_metadata( + metadata_path, + json.dumps( + { + "version": SCHEMA_VERSION, + "repositories": {"owner1/repo1": _repo_entry()}, + "worktrees": { + "owner1/repo1/good": _worktree_entry(branch="good"), + "owner1/repo1/bad": bad_entry, + }, + } + ), + ) + + storage = MetadataStorage(metadata_path) + + assert list(storage.worktrees) == ["owner1/repo1/good"] + assert "owner1/repo1" in storage.repositories + assert not (tmp_path / "metadata.json.corrupt").exists() + assert metadata_path.exists() + + captured = capsys.readouterr() + assert captured.out == "" + # One line naming the skipped entry, one naming the backup of the original. + assert len(captured.err.strip().splitlines()) == 2 + assert "owner1/repo1/bad" in captured.err + assert str(tmp_path / "metadata.json.bak") in captured.err + + def test_skipped_entry_survives_the_next_write(self, tmp_path, capsys): + """A rewrite drops the skipped entry, so the original must be preserved first.""" + metadata_path = tmp_path / "metadata.json" + _write_metadata( + metadata_path, + json.dumps( + { + "version": SCHEMA_VERSION, + "repositories": {"owner1/repo1": _repo_entry()}, + "worktrees": { + "owner1/repo1/good": _worktree_entry(branch="good"), + "owner1/repo1/bad": _worktree_entry( + branch="bad", created_at="not-a-timestamp" + ), + }, + } + ), + ) + original_bytes = metadata_path.read_bytes() + + storage = MetadataStorage(metadata_path) + backup_path = tmp_path / "metadata.json.bak" + # The backup exists before anything can overwrite the file. + assert backup_path.read_bytes() == original_bytes + assert str(backup_path) in capsys.readouterr().err + + storage.add_worktree( + WorktreeInfo( + owner="owner1", + repo="repo1", + branch="new", + local_path=Path("/tmp/worktrees/owner1/repo1/new"), + workspace_id="new", + ) + ) + + on_disk = json.loads(metadata_path.read_text(encoding="utf-8")) + assert sorted(on_disk["worktrees"]) == ["owner1/repo1/good", "owner1/repo1/new"] + assert backup_path.read_bytes() == original_bytes + assert "not-a-timestamp" in backup_path.read_text(encoding="utf-8") + + def test_a_newer_field_does_not_cost_the_entry(self, tmp_path, capsys): + """A field only a newer build knows about is dropped, not the whole entry. + + Every stored entry has the same shape, so treating an unrecognized field + as corruption would make one field added by a newer devlaunch wipe the + entire worktree list at once. The entry loads; the original is preserved. + """ + metadata_path = tmp_path / "metadata.json" + _write_metadata( + metadata_path, + json.dumps( + { + "version": SCHEMA_VERSION, + "repositories": {"owner1/repo1": _repo_entry(future_repo_field=1)}, + "worktrees": { + "owner1/repo1/branch1": _worktree_entry(pinned_by_newer_build=True) + }, + } + ), + ) + original_bytes = metadata_path.read_bytes() + + storage = MetadataStorage(metadata_path) + + assert list(storage.worktrees) == ["owner1/repo1/branch1"] + assert list(storage.repositories) == ["owner1/repo1"] + assert storage.worktrees["owner1/repo1/branch1"].branch == "branch1" + + captured = capsys.readouterr() + assert captured.out == "" + # One line per entry naming the dropped field, one naming the backup. + assert len(captured.err.strip().splitlines()) == 3 + assert "pinned_by_newer_build" in captured.err + assert "future_repo_field" in captured.err + assert not (tmp_path / "metadata.json.corrupt").exists() + assert (tmp_path / "metadata.json.bak").read_bytes() == original_bytes + + storage.save() + + on_disk = json.loads(metadata_path.read_text(encoding="utf-8")) + assert sorted(on_disk["worktrees"]) == ["owner1/repo1/branch1"] + assert "pinned_by_newer_build" not in on_disk["worktrees"]["owner1/repo1/branch1"] + assert (tmp_path / "metadata.json.bak").read_bytes() == original_bytes + + def test_unknown_top_level_section_survives_the_next_write(self, tmp_path, capsys): + """A section this build cannot round-trip is preserved before it is dropped.""" + metadata_path = tmp_path / "metadata.json" + _write_metadata( + metadata_path, + json.dumps( + { + "version": SCHEMA_VERSION, + "repositories": {"owner1/repo1": _repo_entry()}, + "worktrees": {}, + "pinned_workspaces": {"owner1/repo1": True}, + } + ), + ) + original_bytes = metadata_path.read_bytes() + + storage = MetadataStorage(metadata_path) + + assert list(storage.repositories) == ["owner1/repo1"] + captured = capsys.readouterr() + assert captured.out == "" + assert len(captured.err.strip().splitlines()) == 2 + assert "pinned_workspaces" in captured.err + backup_path = tmp_path / "metadata.json.bak" + assert str(backup_path) in captured.err + assert backup_path.read_bytes() == original_bytes + + storage.save() + + assert "pinned_workspaces" not in json.loads(metadata_path.read_text(encoding="utf-8")) + assert backup_path.read_bytes() == original_bytes + + def test_backup_uses_a_single_slot_distinct_from_quarantine(self, tmp_path, capsys): + """Repeated lossy loads overwrite one .bak slot and never touch .corrupt.""" + metadata_path = tmp_path / "metadata.json" + for marker in ("first", "second"): + _write_metadata( + metadata_path, + json.dumps( + { + "version": SCHEMA_VERSION, + "repositories": {}, + "worktrees": {f"owner1/repo1/{marker}": "not-an-object"}, + } + ), + ) + MetadataStorage(metadata_path) + capsys.readouterr() + + assert sorted(p.name for p in tmp_path.iterdir()) == [ + "metadata.json", + "metadata.json.bak", + ] + assert "second" in (tmp_path / "metadata.json.bak").read_text(encoding="utf-8") + + def test_bad_repository_entry_is_skipped(self, tmp_path, capsys): + """A malformed repository entry is skipped while good repositories load.""" + metadata_path = tmp_path / "metadata.json" + _write_metadata( + metadata_path, + json.dumps( + { + "version": SCHEMA_VERSION, + "repositories": { + "owner1/good": _repo_entry(repo="good"), + "owner1/bad": _repo_entry(repo="bad", local_path=None), + }, + "worktrees": {}, + } + ), + ) + + storage = MetadataStorage(metadata_path) + + assert list(storage.repositories) == ["owner1/good"] + assert not (tmp_path / "metadata.json.corrupt").exists() + captured = capsys.readouterr() + assert captured.out == "" + assert "owner1/bad" in captured.err + assert (tmp_path / "metadata.json.bak").exists() + + def test_non_dict_section_is_skipped(self, tmp_path, capsys): + """A section that is not an object is skipped without losing the other section.""" + metadata_path = tmp_path / "metadata.json" + _write_metadata( + metadata_path, + json.dumps( + { + "version": SCHEMA_VERSION, + "repositories": ["not", "an", "object"], + "worktrees": {"owner1/repo1/branch1": _worktree_entry()}, + } + ), + ) + + storage = MetadataStorage(metadata_path) + + assert storage.repositories == {} + assert list(storage.worktrees) == ["owner1/repo1/branch1"] + assert not (tmp_path / "metadata.json.corrupt").exists() + captured = capsys.readouterr() + assert captured.out == "" + assert "repositories" in captured.err + assert (tmp_path / "metadata.json.bak").exists() + + +class TestAtomicSave: + """save() must never leave a half-written metadata.json.""" + + def test_save_writes_via_a_temp_file(self, temp_storage, monkeypatch): + """save() renames a temp file into place rather than writing in place.""" + seen = _record_replacements(monkeypatch) + temp_storage.save() + + assert len(seen) == 1 + assert seen[0].name != temp_storage.metadata_path.name + assert seen[0].name.endswith(".tmp") + + def test_instances_in_one_process_do_not_share_a_temp_path(self, tmp_path, monkeypatch): + """Two storage objects must not write through the same temp file.""" + metadata_path = tmp_path / "metadata.json" + first = MetadataStorage(metadata_path) + second = MetadataStorage(metadata_path) + seen = _record_replacements(monkeypatch) + + first.save() + second.save() + + assert len(seen) == 2 + assert seen[0] != seen[1] + + def test_interrupted_save_leaves_original_intact(self, temp_storage, monkeypatch): + """A truncated temp file is discarded and the previous metadata stays readable.""" + temp_storage.add_repository( + BaseRepository( + owner="owner1", + repo="repo1", + remote_url="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/owner1/repo1.git", + local_path=Path("/tmp/repos/owner1/repo1"), + ) + ) + original = temp_storage.metadata_path.read_text(encoding="utf-8") + parent = temp_storage.metadata_path.parent + partial = {} + + def exploding_dump(_data, fp, **_kwargs): + # Write real bytes first so a genuinely truncated temp file exists on + # disk at the moment the write fails. + fp.write('{\n "version": 1,\n "repositories": {"owner2/repo2"') + fp.flush() + partial.update( + {p: p.read_text(encoding="utf-8") for p in parent.glob("*.tmp")}, + ) + raise OSError("disk full") + + monkeypatch.setattr(storage_module.json, "dump", exploding_dump) + # save() must not swallow the write failure + with pytest.raises(OSError): + temp_storage.add_repository( + BaseRepository( + owner="owner2", + repo="repo2", + remote_url="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/owner2/repo2.git", + local_path=Path("/tmp/repos/owner2/repo2"), + ) + ) + + monkeypatch.undo() + # A single truncated temp file really existed mid-write. + assert len(partial) == 1 + truncated = next(iter(partial.values())) + assert truncated.endswith('{"owner2/repo2"') + with pytest.raises(json.JSONDecodeError): + json.loads(truncated) + + assert temp_storage.metadata_path.read_text(encoding="utf-8") == original + reloaded = MetadataStorage(temp_storage.metadata_path) + assert list(reloaded.repositories) == ["owner1/repo1"] + leftovers = [ + p.name + for p in temp_storage.metadata_path.parent.iterdir() + if p.name != temp_storage.metadata_path.name + ] + assert leftovers == [] + + +class TestFilePermissions: + """metadata.json holds repo owners and local paths: keep it private.""" + + @staticmethod + def _mode(path: Path) -> int: + return stat.S_IMODE(path.stat().st_mode) + + def test_existing_mode_survives_the_atomic_replace(self, tmp_path): + """A 0600 metadata.json is still 0600 after save().""" + metadata_path = tmp_path / "metadata.json" + _write_metadata( + metadata_path, + json.dumps({"version": SCHEMA_VERSION, "repositories": {}, "worktrees": {}}), + ) + metadata_path.chmod(0o600) + storage = MetadataStorage(metadata_path) + + previous_umask = os.umask(0o002) + try: + storage.save() + finally: + os.umask(previous_umask) + + assert self._mode(metadata_path) == 0o600 + + def test_unusual_existing_mode_is_preserved(self, tmp_path): + """save() copies whatever mode the user set, not just 0600.""" + metadata_path = tmp_path / "metadata.json" + _write_metadata( + metadata_path, + json.dumps({"version": SCHEMA_VERSION, "repositories": {}, "worktrees": {}}), + ) + metadata_path.chmod(0o640) + MetadataStorage(metadata_path).save() + + assert self._mode(metadata_path) == 0o640 + + def test_new_file_is_created_private(self, tmp_path): + """A metadata.json created from scratch is 0600 regardless of umask.""" + metadata_path = tmp_path / "metadata.json" + storage = MetadataStorage(metadata_path) + + previous_umask = os.umask(0o002) + try: + storage.save() + finally: + os.umask(previous_umask) + + assert self._mode(metadata_path) == 0o600 + + +class TestSymlinkedMetadataFile: + """A metadata.json symlinked elsewhere must keep working through the link.""" + + @staticmethod + def _linked(tmp_path): + real_path = tmp_path / "synced" / "metadata.json" + real_path.parent.mkdir() + link_path = tmp_path / "metadata.json" + link_path.symlink_to(real_path) + return real_path, link_path + + def test_save_writes_through_the_symlink(self, tmp_path): + """save() updates the link target and leaves the symlink itself in place.""" + real_path, link_path = self._linked(tmp_path) + _write_metadata( + real_path, + json.dumps({"version": SCHEMA_VERSION, "repositories": {}, "worktrees": {}}), + ) + + storage = MetadataStorage(link_path) + storage.add_repository( + BaseRepository( + owner="owner1", + repo="repo1", + remote_url="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/owner1/repo1.git", + local_path=Path("/tmp/repos/owner1/repo1"), + ) + ) + + assert link_path.is_symlink() + assert os.readlink(link_path) == str(real_path) + on_disk = json.loads(real_path.read_text(encoding="utf-8")) + assert "owner1/repo1" in on_disk["repositories"] + assert list(MetadataStorage(link_path).repositories) == ["owner1/repo1"] + + def test_quarantine_moves_the_link_target(self, tmp_path, capsys): + """Corruption behind a symlink quarantines the real file, not the link.""" + real_path, link_path = self._linked(tmp_path) + _write_metadata(real_path, "{broken") + + MetadataStorage(link_path) + + assert link_path.is_symlink() + assert not real_path.exists() + corrupt_path = real_path.with_name("metadata.json.corrupt") + assert corrupt_path.read_text(encoding="utf-8") == "{broken" + assert str(corrupt_path) in capsys.readouterr().err + + +class TestSingleWrite: + """Mutating helpers must write metadata.json exactly once.""" + + @staticmethod + def _count_saves(storage, monkeypatch): + calls = [] + real_save = storage.save + + def counting_save(): + calls.append(1) + real_save() + + monkeypatch.setattr(storage, "save", counting_save) + return calls + + def _populated(self, storage): + storage.add_repository( + BaseRepository( + owner="owner1", + repo="repo1", + remote_url="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/owner1/repo1.git", + local_path=Path("/tmp/repos/owner1/repo1"), + worktrees=[], + ) + ) + return WorktreeInfo( + owner="owner1", + repo="repo1", + branch="branch1", + local_path=Path("/tmp/worktrees/owner1/repo1/branch1"), + workspace_id="branch1", + ) + + def test_add_worktree_saves_once(self, temp_storage, monkeypatch): + """add_worktree writes once even when it updates the repository's branch list.""" + worktree = self._populated(temp_storage) + calls = self._count_saves(temp_storage, monkeypatch) + + temp_storage.add_worktree(worktree) + + assert len(calls) == 1 + on_disk = json.loads(temp_storage.metadata_path.read_text(encoding="utf-8")) + assert on_disk["worktrees"]["owner1/repo1/branch1"]["branch"] == "branch1" + assert on_disk["repositories"]["owner1/repo1"]["worktrees"] == ["branch1"] + + def test_remove_worktree_saves_once(self, temp_storage, monkeypatch): + """remove_worktree writes once even when it updates the repository's branch list.""" + worktree = self._populated(temp_storage) + temp_storage.add_worktree(worktree) + calls = self._count_saves(temp_storage, monkeypatch) + + temp_storage.remove_worktree("owner1", "repo1", "branch1") + + assert len(calls) == 1 + on_disk = json.loads(temp_storage.metadata_path.read_text(encoding="utf-8")) + assert on_disk["worktrees"] == {} + assert on_disk["repositories"]["owner1/repo1"]["worktrees"] == [] + + +class TestSchemaVersion: + """The top-level version key gives future migrations a deterministic trigger.""" + + def test_save_writes_current_version(self, temp_storage): + """A round-trip writes and reads back version 1.""" + temp_storage.save() + on_disk = json.loads(temp_storage.metadata_path.read_text(encoding="utf-8")) + + assert SCHEMA_VERSION == 1 + assert on_disk["version"] == 1 + assert MetadataStorage(temp_storage.metadata_path).schema_version == 1 + + def test_legacy_file_without_version_loads_silently(self, tmp_path, capsys): + """A pre-versioning file is treated as version 1 with no warning.""" + metadata_path = tmp_path / "metadata.json" + _write_metadata( + metadata_path, + json.dumps( + { + "repositories": {"owner1/repo1": _repo_entry()}, + "worktrees": {"owner1/repo1/branch1": _worktree_entry()}, + } + ), + ) + + storage = MetadataStorage(metadata_path) + + assert storage.schema_version == 1 + assert list(storage.repositories) == ["owner1/repo1"] + assert list(storage.worktrees) == ["owner1/repo1/branch1"] + captured = capsys.readouterr() + assert captured.out == "" + assert captured.err == "" + assert not (tmp_path / "metadata.json.corrupt").exists() + + def test_newer_version_warns_and_preserves_the_original(self, tmp_path, capsys): + """A file from a newer devlaunch loads, and the original survives the rewrite.""" + metadata_path = tmp_path / "metadata.json" + _write_metadata( + metadata_path, + json.dumps( + { + "version": 99, + "repositories": {"owner1/repo1": _repo_entry()}, + "worktrees": {"owner1/repo1/branch1": _worktree_entry()}, + } + ), + ) + original_bytes = metadata_path.read_bytes() + + storage = MetadataStorage(metadata_path) + + assert storage.schema_version == 99 + assert list(storage.repositories) == ["owner1/repo1"] + assert list(storage.worktrees) == ["owner1/repo1/branch1"] + assert metadata_path.exists() + assert not (tmp_path / "metadata.json.corrupt").exists() + captured = capsys.readouterr() + assert captured.out == "" + assert len(captured.err.strip().splitlines()) == 2 + assert "newer" in captured.err + backup_path = tmp_path / "metadata.json.bak" + assert str(backup_path) in captured.err + assert backup_path.read_bytes() == original_bytes + + storage.save() + + # The promise is not immutability: the file is rewritten in this format. + assert json.loads(metadata_path.read_text(encoding="utf-8"))["version"] == SCHEMA_VERSION + assert backup_path.read_bytes() == original_bytes + + def test_older_version_loads_and_is_upgraded_on_write(self, tmp_path, capsys): + """A lower version is an older shape: load it, report it, upgrade on write.""" + metadata_path = tmp_path / "metadata.json" + _write_metadata( + metadata_path, + json.dumps( + { + "version": 0, + "repositories": {"owner1/repo1": _repo_entry()}, + "worktrees": {"owner1/repo1/branch1": _worktree_entry()}, + } + ), + ) + + storage = MetadataStorage(metadata_path) + + assert storage.schema_version == 0 + assert list(storage.repositories) == ["owner1/repo1"] + assert list(storage.worktrees) == ["owner1/repo1/branch1"] + captured = capsys.readouterr() + assert captured.out == "" + assert captured.err == "" + assert not (tmp_path / "metadata.json.corrupt").exists() + assert not (tmp_path / "metadata.json.bak").exists() + + storage.save() + + assert json.loads(metadata_path.read_text(encoding="utf-8"))["version"] == SCHEMA_VERSION + + @pytest.mark.parametrize( + "version, normalized", + [ + pytest.param("1.0", 1, id="integral-float"), + pytest.param("2.0", 2, id="integral-float-newer"), + pytest.param("true", None, id="bool-true"), + pytest.param("null", None, id="null"), + pytest.param('"1"', None, id="string"), + pytest.param("1.5", None, id="non-integral-float"), + pytest.param("[1]", None, id="list"), + ], + ) + def test_odd_version_header_never_costs_the_entries( + self, tmp_path, capsys, version, normalized + ): + """A cosmetically odd version header is not file-level corruption.""" + metadata_path = tmp_path / "metadata.json" + _write_metadata( + metadata_path, + '{"version": ' + + version + + ', "repositories": {"owner1/repo1": ' + + json.dumps(_repo_entry()) + + '}, "worktrees": {"owner1/repo1/branch1": ' + + json.dumps(_worktree_entry()) + + "}}", + ) + original_bytes = metadata_path.read_bytes() + + storage = MetadataStorage(metadata_path) + + assert list(storage.repositories) == ["owner1/repo1"] + assert list(storage.worktrees) == ["owner1/repo1/branch1"] + assert metadata_path.exists() + assert not (tmp_path / "metadata.json.corrupt").exists() + backup_path = tmp_path / "metadata.json.bak" + captured = capsys.readouterr() + assert captured.out == "" + + if normalized is not None: + # An integral number is that version; JSON has one number type. + assert storage.schema_version == normalized + if normalized <= SCHEMA_VERSION: + assert captured.err == "" + assert not backup_path.exists() + else: + assert storage.schema_version == SCHEMA_VERSION + assert len(captured.err.strip().splitlines()) == 2 + assert str(backup_path) in captured.err + assert backup_path.read_bytes() == original_bytes