diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index e00b9c247a..f33c1578b1 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -9,11 +9,13 @@ from __future__ import annotations import copy +import errno import hashlib import json import os import re import shutil +import stat import tempfile import zipfile from dataclasses import dataclass @@ -101,6 +103,51 @@ def _load_core_command_names() -> frozenset[str]: CORE_COMMAND_NAMES = _load_core_command_names() +def _fsync_fd(fd: int) -> None: + """Sync a file descriptor, raising on real storage errors.""" + try: + os.fsync(fd) + except AttributeError: + return + except NotImplementedError: + return + except OSError as exc: + if exc.errno in {errno.ENOTSUP, errno.EOPNOTSUPP, errno.EINVAL, errno.EBADF}: + return + raise + + +def _fsync_directory(path: Path) -> None: + """Sync a directory when the platform supports it.""" + if not path.exists(): + return + if os.name == "nt": + return + try: + dir_fd = os.open(str(path), os.O_RDONLY | getattr(os, "O_DIRECTORY", 0)) + except (AttributeError, NotImplementedError): + return + except OSError as exc: + if exc.errno in {errno.ENOTSUP, errno.EOPNOTSUPP, errno.EINVAL, errno.EBADF}: + return + try: + dir_fd = os.open(str(path), os.O_RDONLY) + except (AttributeError, NotImplementedError): + return + except OSError as exc2: + if exc2.errno in {errno.ENOTSUP, errno.EOPNOTSUPP, errno.EINVAL, errno.EBADF}: + return + raise + try: + _fsync_fd(dir_fd) + finally: + try: + os.close(dir_fd) + except OSError: + # Cleanup after an fsync failure should not mask the original error. + pass + + class ExtensionError(Exception): """Base exception for extension-related errors.""" @@ -699,6 +746,32 @@ def __init__(self, project_root: Path): self.extensions_dir = project_root / ".specify" / "extensions" self.registry = ExtensionRegistry(self.extensions_dir) + def _rescue_staging_dir(self, extension_id: str) -> Path: + """Fixed-length staging directory path for a preserved-config rescue. + + The extension ID can be arbitrarily long (manifest validation caps only + the character set, not the length), so embedding it verbatim in a single + path component could push the ``.rescue-staging-`` directory past a + filesystem's per-component byte limit and make every reinstall after + ``--keep-config`` fail with ``ENAMETOOLONG`` even though the extension + installs fine at ``dest_dir``. Hash the ID to a fixed-length suffix so + the component length is bounded regardless of ID length. + """ + digest = hashlib.sha256(extension_id.encode("utf-8")).hexdigest()[:16] + return self.extensions_dir / f".rescue-staging-{digest}" + + @staticmethod + def _has_keep_config_marker(directory: Path) -> bool: + """Return True when *directory* contains a valid ``.keep-config`` marker. + + The marker is a regular (non-symlink) file written by + ``remove(..., keep_config=True)`` to record explicit provenance. Its + content is intentionally empty — only presence matters, not content. + The symlink guard prevents a crafted symlink from fooling the check. + """ + marker = directory / ".keep-config" + return marker.is_file() and not marker.is_symlink() + @staticmethod def _collect_manifest_command_names(manifest: ExtensionManifest) -> Dict[str, str]: """Collect command and alias names declared by a manifest. @@ -1402,12 +1475,384 @@ def install_from_directory( backup_config_dir.unlink() did_remove = self.remove(manifest.id) + # Load and validate .extensionignore BEFORE reading/creating the rescue + # staging directory (and thus before deleting dest_dir). The loader can + # raise ValidationError (invalid UTF-8) or OSError; doing it first means + # such a failure aborts while the kept config is still authoritative in + # its documented location, rather than leaving a freshly published + # staging copy that a later retry (after the user edits the kept config) + # would reload and use to overwrite the newer bytes. Any staging left by + # an earlier destructive attempt is intentionally left intact here. + ignore_fn = self._load_extensionignore(source_dir) + + # Rescue any config files left behind by a prior `remove --keep-config`. + # When an extension is removed with --keep-config, it is no longer in + # the registry but its config files remain in dest_dir. A subsequent + # plain (non-force) install would delete that directory unconditionally, + # silently discarding the preserved config. We read those files into + # memory and also write a durable staging copy outside dest_dir so + # that a partial rmtree, failed copytree, or partial restore cannot + # permanently discard the user's original bytes on a retry. The + # staging dir is removed only after every config has been successfully + # restored. + stranded_configs: dict[str, tuple[bytes, int]] = {} + rescue_staging_dir = self._rescue_staging_dir(manifest.id) + # A staging directory is trusted only when this completion marker is + # present. The marker is written after every staged file is complete + # and removed before the non-atomic cleanup, so a crash mid-staging or + # mid-cleanup can never leave a partial directory that a retry mistakes + # for a complete durable backup. + rescue_complete_marker = rescue_staging_dir / ".rescue-complete" + staging_is_complete = ( + rescue_staging_dir.is_dir() + and not rescue_staging_dir.is_symlink() + and rescue_complete_marker.is_file() + and not rescue_complete_marker.is_symlink() + ) + + if staging_is_complete and not self.registry.is_installed(manifest.id): + # A previous install attempt staged the configs but never + # completed cleanly. Reload from the durable backup so the + # original bytes are used on retry rather than whatever + # mixture of packaged defaults and partial restores remains + # on disk. Only load non-symlinked files whose names match + # the two recognised config suffixes so a tampered staging + # directory cannot inject arbitrary files. + # + # A complete staging directory proves only that staging finished, + # not that dest_dir was ever modified: a crash after staging was + # synced but before the rmtree below leaves the live kept config + # intact. If the user then edits that live config before retrying, + # blindly preferring the staged bytes would silently overwrite the + # newer config. The staged and live copies are indistinguishable + # in provenance from disk alone (a genuine post-crash edit vs. a + # packaged default written by a partially-completed copytree), so + # when a live config disagrees with its staged copy we must not + # silently pick either — preserve both and abort, letting the user + # resolve it. dest_dir is still untouched here, so raising is safe. + def _recognized_config_names( + directory: Path, *, follow_symlinks: bool = True + ) -> set[str]: + names: set[str] = set() + if not directory.is_dir(): + return names + for entry in directory.iterdir(): + if not entry.name.endswith( + ("-config.yml", "-config.local.yml") + ): + continue + if follow_symlinks: + if entry.is_file() and not entry.is_symlink(): + names.add(entry.name) + else: + # Include symlinks without following them so that + # live-only symlinked configs are detected and + # preserved rather than silently deleted. + if entry.is_file() or entry.is_symlink(): + names.add(entry.name) + return names + + conflicting: set[str] = set() + staged_names = _recognized_config_names(rescue_staging_dir) + live_names = _recognized_config_names( + dest_dir, follow_symlinks=False + ) + # A live-only config created after the interrupted attempt is not + # enumerated by staging, so without this it would be silently + # deleted by the rmtree below and its bytes lost. Treat it as a + # conflict so both locations are preserved and the user resolves it. + live_only = live_names - staged_names + conflicting.update(live_only) + # Load original permission bits from the sidecar JSON written by + # the staging step. Staged files are kept at mode 0o600 so that + # rmtree always succeeds on Windows, so staged_stat.st_mode would + # always be 0o600 and must not be used for mode comparisons or + # restoration; the sidecar records the true original mode. + rescue_modes_file = rescue_staging_dir / ".rescue-modes.json" + _staged_modes: dict[str, int] = {} + if rescue_modes_file.is_file() and not rescue_modes_file.is_symlink(): + try: + _staged_modes = json.loads(rescue_modes_file.read_bytes()) + except (OSError, ValueError): + # Ignore unreadable/invalid sidecar metadata and fall back + # to each staged file's mode for compatibility. + pass + for staged_name in sorted(staged_names): + staged_file = rescue_staging_dir / staged_name + staged_stat = staged_file.stat() + staged_bytes = staged_file.read_bytes() + # Prefer the sidecar-recorded mode; fall back to the staged + # file's own mode for backwards-compat with staging dirs + # written before the sidecar was introduced. + staged_mode = _staged_modes.get( + staged_name, stat.S_IMODE(staged_stat.st_mode) + ) + live_file = dest_dir / staged_name + if live_file.is_symlink(): + # A user may have replaced the live config with a symlink + # after the interrupted attempt. It cannot be compared by + # bytes/mode against the staged copy, and the rmtree below + # would silently delete this newer choice and restore the + # older staged file. Treat any live symlink as a conflict so + # both are preserved and the user resolves it. + conflicting.add(staged_name) + elif live_file.is_file(): + # A live config that cannot be read or stat'ed must not be + # treated as non-conflicting: the rmtree below would delete + # it and restore the stale staged copy. Abort while dest_dir + # is untouched so no newer or permission-restricted config is + # lost. Divergence also includes permission-only edits (for + # example tightening a secret-bearing config from 0644 to + # 0600), which byte equality alone would miss and then revert. + try: + live_stat = live_file.stat() + live_bytes = live_file.read_bytes() + except OSError: + conflicting.add(staged_name) + else: + if live_bytes != staged_bytes or stat.S_IMODE( + live_stat.st_mode + ) != staged_mode: + conflicting.add(staged_name) + stranded_configs[staged_name] = (staged_bytes, staged_mode) + if conflicting: + # Split into two cases for accurate user guidance: files that + # exist in both locations but have diverged, and files that + # exist only in the live directory with no rescue-backup copy. + both_diverged = conflicting - live_only + live_only_conflict = conflicting & live_only + msg_parts: list[str] = [ + f"Preserved extension config conflict for '{manifest.id}':" + ] + if both_diverged: + names = ", ".join(sorted(both_diverged)) + msg_parts.append( + f"The current config(s) ({names}) in {dest_dir} differ" + f" from their rescued backup in {rescue_staging_dir}." + " Both copies have been preserved." + ) + if live_only_conflict: + names = ", ".join(sorted(live_only_conflict)) + msg_parts.append( + f"The config(s) ({names}) exist only in {dest_dir}" + f" with no counterpart in the rescued backup at" + f" {rescue_staging_dir}." + ) + msg_parts.append( + f"Reconcile {dest_dir} and {rescue_staging_dir} to the" + f" desired final state, delete {rescue_staging_dir}," + " then reinstall." + ) + raise ValidationError(" ".join(msg_parts)) + elif ( + dest_dir.exists() + and not self.registry.is_installed(manifest.id) + and self._has_keep_config_marker(dest_dir) + ): + for cfg_file in ( + list(dest_dir.glob("*-config.yml")) + + list(dest_dir.glob("*-config.local.yml")) + ): + if cfg_file.is_symlink(): + # `remove --keep-config` preserves a symlinked config + # because Path.is_file() follows symlinks. Its bytes cannot + # be safely rescued (the target may live outside dest_dir), + # and the rmtree below would delete the link and silently + # discard the kept configuration. Reject the reinstall while + # dest_dir is untouched so the user resolves it rather than + # losing the linked config. + raise ValidationError( + "Preserved extension config for " + f"'{manifest.id}' is a symlink ({cfg_file.name}) in " + f"{dest_dir}, which cannot be safely rescued during " + "reinstall. Resolve manually — replace the symlink with " + "a regular file or remove it — then reinstall." + ) + if cfg_file.is_file(): + stranded_configs[cfg_file.name] = ( + cfg_file.read_bytes(), + cfg_file.stat().st_mode, + ) + + if stranded_configs and not staging_is_complete: + # Write a durable backup outside dest_dir before any + # destructive operation so the original bytes survive a + # crash or partial failure at any later step. The staging + # dir is cleaned up only after every restore succeeds. + # + # Any pre-existing staging dir here lacks the completion marker + # (staging_is_complete is False), so it is a stale partial from an + # interrupted attempt — remove it first for a clean write. + if rescue_staging_dir.is_symlink(): + rescue_staging_dir.unlink() + elif rescue_staging_dir.is_dir(): + shutil.rmtree(rescue_staging_dir) + elif rescue_staging_dir.exists(): + rescue_staging_dir.unlink() + try: + rescue_staging_dir.mkdir(parents=True, exist_ok=True) + for filename, (content, mode) in stranded_configs.items(): + staged = rescue_staging_dir / filename + # Create the staging file with mode 0600 before writing so + # the preserved bytes are never transiently readable by other + # local users, even on a umask that would produce 0644. + # O_BINARY (0 on POSIX) is required so Windows does not open + # the descriptor in text mode and translate the preserved + # bytes' "\n" into "\r\n" as they are written. + fd = os.open( + str(staged), + os.O_WRONLY | os.O_CREAT | os.O_EXCL | getattr(os, "O_BINARY", 0), + 0o600, + ) + try: + # os.write() may write fewer bytes than requested, so + # loop until the whole buffer is on disk — a truncated + # "durable" backup would be trusted over the intact + # config on a retry and cause silent data loss. + view = memoryview(content) + written = 0 + while written < len(view): + written += os.write(fd, view[written:]) + # Do NOT chmod the staged file: setting a read-only + # mode (e.g. 0o444) makes the file undeletable on + # Windows and causes shutil.rmtree to fail during + # cleanup. Original modes are recorded separately in + # .rescue-modes.json so they can be reapplied when the + # config is actually restored. + _fsync_fd(fd) + finally: + os.close(fd) + # Persist the original permission bits in a sidecar JSON file + # so a retry can correctly reapply them even though the staged + # files themselves are kept at their creation mode (0o600). + rescue_modes_file = rescue_staging_dir / ".rescue-modes.json" + modes_payload = json.dumps( + { + filename: stat.S_IMODE(mode) + for filename, (_, mode) in stranded_configs.items() + }, + sort_keys=True, + ).encode() + modes_fd = os.open( + str(rescue_modes_file), + os.O_WRONLY | os.O_CREAT | os.O_EXCL | getattr(os, "O_BINARY", 0), + 0o600, + ) + try: + view = memoryview(modes_payload) + written = 0 + while written < len(view): + written += os.write(modes_fd, view[written:]) + _fsync_fd(modes_fd) + finally: + os.close(modes_fd) + # Flush the staging directory metadata before publishing the + # completion marker so a crash cannot leave a visible marker with + # only a subset of staged files. + _fsync_directory(rescue_staging_dir) + # Write the completion marker only after every staged file is + # fully written so a retry trusts staging only when it is whole. + marker_fd = os.open( + str(rescue_complete_marker), + os.O_WRONLY | os.O_CREAT | os.O_EXCL, + 0o600, + ) + try: + _fsync_fd(marker_fd) + finally: + os.close(marker_fd) + _fsync_directory(rescue_staging_dir) + _fsync_directory(rescue_staging_dir.parent) + except BaseException: + # Durable staging failed (or was interrupted). Continuing with + # only the in-memory copy would reintroduce the permanent-loss + # path this staging exists to close: the rmtree below could + # delete the originals and a later restore failure would leave + # no on-disk copy. dest_dir is still untouched here, so clean + # up the partial staging dir and abort the install instead of + # proceeding destructively. + shutil.rmtree(rescue_staging_dir, ignore_errors=True) + raise + # Install extension (dest_dir computed above during self-install guard) if dest_dir.exists(): shutil.rmtree(dest_dir) - ignore_fn = self._load_extensionignore(source_dir) - shutil.copytree(source_dir, dest_dir, ignore=ignore_fn) + def _restore_stranded_config_file( + target: Path, content: bytes, preserved_mode: int + ) -> None: + tmp_path: Path | None = None + try: + # A short fixed prefix, not f".{target.name}.": the preserved + # config filename may itself already be near the filesystem's + # per-component byte limit, and NamedTemporaryFile appends a + # random suffix to the prefix — reusing the full name would push + # the temp file past the limit and raise ENAMETOOLONG on every + # retry. tempfile already guarantees collision avoidance. + with tempfile.NamedTemporaryFile( + mode="wb", + dir=target.parent, + prefix=".cfg-restore.", + delete=False, + ) as tmp: + tmp_path = Path(tmp.name) + tmp.write(content) + tmp.flush() + _fsync_fd(tmp.fileno()) + try: + tmp_path.chmod(stat.S_IMODE(preserved_mode)) + except (NotImplementedError, OSError): + pass # Best-effort; chmod may not be supported on all platforms. + os.replace(tmp_path, target) + try: + target_fd = os.open(str(target), os.O_RDONLY) + except (AttributeError, OSError, NotImplementedError): + target_fd = None + try: + if target_fd is not None: + _fsync_fd(target_fd) + finally: + if target_fd is not None: + try: + os.close(target_fd) + except OSError: + pass # best-effort close during cleanup; ignore errors + _fsync_directory(target.parent) + except BaseException: + if tmp_path is not None and tmp_path.exists(): + tmp_path.unlink() + raise + + try: + shutil.copytree(source_dir, dest_dir, ignore=ignore_fn) + except BaseException: + # copytree failed — dest_dir may be absent or only partially + # created. Write the rescued configs back now so they are not + # permanently lost even though the install did not complete. + if stranded_configs: + dest_dir.mkdir(parents=True, exist_ok=True) + for filename, (content, mode) in stranded_configs.items(): + target = dest_dir / filename + _restore_stranded_config_file(target, content, mode) + raise + + # Restore stranded configs rescued before the rmtree above. + for filename, (content, mode) in stranded_configs.items(): + target = dest_dir / filename + _restore_stranded_config_file(target, content, mode) + + # Every stranded config has been restored successfully — the + # durable staging backup is no longer needed. Raise on failure so + # the install is not reported as successful while a stale backup + # that could be misread on the next retry remains on disk. + if rescue_staging_dir.is_dir() and not rescue_staging_dir.is_symlink(): + # Remove the completion marker before the non-atomic rmtree so a + # crash mid-cleanup cannot leave a staging dir that a retry would + # wrongly trust as a complete durable backup. + rescue_complete_marker.unlink(missing_ok=True) + _fsync_directory(rescue_staging_dir) + shutil.rmtree(rescue_staging_dir) + _fsync_directory(rescue_staging_dir.parent) # Register commands with AI agents registered_commands = {} @@ -1586,6 +2031,12 @@ def remove(self, extension_id: str, keep_config: bool = False) -> bool: shutil.rmtree(child) else: child.unlink() + # Write a provenance marker so install_from_directory can + # distinguish this --keep-config leftover from a directory left + # by a partially-failed install (which must not have its + # packaged default configs treated as user-preserved data). + # Content is intentionally empty — only presence matters. + (extension_dir / ".keep-config").write_text("") else: # Backup config files before deleting if extension_dir.exists(): diff --git a/src/specify_cli/workflows/_commands.py b/src/specify_cli/workflows/_commands.py index 9ab199c023..080c39b3a2 100644 --- a/src/specify_cli/workflows/_commands.py +++ b/src/specify_cli/workflows/_commands.py @@ -1666,8 +1666,8 @@ def _validate_and_install_local( except OSError as cleanup_exc: console.print( "[yellow]Warning:[/yellow] Could not remove temporary " - f"download file {_escape_markup(str(tmp_path))}: " - f"{_escape_markup(str(cleanup_exc))}" + f"workflow download file: {_escape_markup(str(cleanup_exc))} " + f"(path: {_escape_markup(str(tmp_path))})" ) console.print(f"[red]Error:[/red] Failed to download workflow: {_escape_markup(str(exc))}") raise typer.Exit(1) @@ -1691,8 +1691,8 @@ def _validate_and_install_local( except OSError as exc: console.print( "[yellow]Warning:[/yellow] Could not remove temporary " - f"download file {_escape_markup(str(tmp_path))}: " - f"{_escape_markup(str(exc))}" + f"workflow download file: {_escape_markup(str(exc))} " + f"(path: {_escape_markup(str(tmp_path))})" ) return diff --git a/tests/extensions/test_update_agent_context_feature_json.py b/tests/extensions/test_update_agent_context_feature_json.py index 957415708c..25b5ff9457 100644 --- a/tests/extensions/test_update_agent_context_feature_json.py +++ b/tests/extensions/test_update_agent_context_feature_json.py @@ -11,7 +11,6 @@ from tests.conftest import requires_bash from tests.extensions.test_extension_agent_context import ( - BASH, POWERSHELL, _bash_posix_path, _run_bash_agent_context_script, diff --git a/tests/test_extensions.py b/tests/test_extensions.py index d2f4af8264..e2baee6414 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -22,6 +22,7 @@ from unittest.mock import MagicMock from tests.conftest import strip_ansi +from specify_cli import extensions as _ext_module from specify_cli.extensions import ( CatalogEntry, CORE_COMMAND_NAMES, @@ -1226,6 +1227,690 @@ def test_install_force_config_preserved(self, extension_dir, project_dir): assert new_config.exists() assert new_config.read_text() == "test: config" + def test_reinstall_after_keep_config_preserves_config( + self, extension_dir, project_dir + ): + """Reinstalling after `remove --keep-config` must not overwrite preserved config.""" + manager = ExtensionManager(project_dir) + + # Add a packaged default config so the reinstall has a file to overwrite. + # Without the fix, the packaged default would silently win on reinstall. + packaged_config = extension_dir / "test-ext-config.yml" + packaged_config.write_text("model: default-model\nmax_iterations: 1\n") + + # Install once (packaged default is copied into the installed directory) + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + # Overwrite the installed config with user-customized values + ext_dir = project_dir / ".specify" / "extensions" / "test-ext" + config_file = ext_dir / "test-ext-config.yml" + config_file.write_text("model: custom-model\nmax_iterations: 99\n") + + # Remove while preserving config + manager.remove("test-ext", keep_config=True) + assert not manager.registry.is_installed("test-ext") + assert config_file.exists() + assert "custom-model" in config_file.read_text() + + # Plain reinstall (no --force) — packaged default is still present in + # extension_dir, so a naive implementation would overwrite the custom values. + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + # Preserved config must survive the reinstall and beat the packaged default + assert config_file.exists() + assert "custom-model" in config_file.read_text() + assert "99" in config_file.read_text() + assert "default-model" not in config_file.read_text() + + def test_reinstall_after_keep_config_preserves_local_config( + self, extension_dir, project_dir + ): + """Local config override files (*-config.local.yml) are also rescued on reinstall.""" + manager = ExtensionManager(project_dir) + + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + ext_dir = project_dir / ".specify" / "extensions" / "test-ext" + local_cfg = ext_dir / "test-ext-config.local.yml" + local_cfg.write_text("local_override: true\n") + + manager.remove("test-ext", keep_config=True) + assert local_cfg.exists() + + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + assert local_cfg.exists() + assert "local_override: true" in local_cfg.read_text() + + def test_reinstall_with_symlinked_config_rejects_install( + self, extension_dir, project_dir + ): + """A preserved symlinked config must abort reinstall, not be deleted.""" + ext_dir = project_dir / ".specify" / "extensions" / "test-ext" + if not can_create_symlink(ext_dir.parent if ext_dir.parent.exists() else project_dir): + pytest.skip("Current platform/user cannot create symlinks") + + manager = ExtensionManager(project_dir) + packaged_config = extension_dir / "test-ext-config.yml" + packaged_config.write_text("model: default-model\n") + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + # Replace the installed config with a symlink to a file outside dest_dir. + config_file = ext_dir / "test-ext-config.yml" + external_target = project_dir / "external-config.yml" + external_target.write_text("model: linked-model\n") + config_file.unlink() + os.symlink(external_target, config_file) + assert config_file.is_symlink() + + # `remove --keep-config` follows the symlink via is_file() and keeps it. + manager.remove("test-ext", keep_config=True) + assert not manager.registry.is_installed("test-ext") + assert config_file.is_symlink() + + # Plain reinstall must reject rather than silently delete the link. + with pytest.raises(ValidationError, match="is a symlink"): + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + # The symlink and its target survive; nothing was silently discarded. + assert config_file.is_symlink() + assert external_target.read_text() == "model: linked-model\n" + assert not manager.registry.is_installed("test-ext") + + def test_retry_with_symlinked_live_config_aborts_and_preserves_both( + self, extension_dir, project_dir, monkeypatch + ): + """A live config replaced by a symlink on retry is a conflict, not overwritten.""" + ext_dir = project_dir / ".specify" / "extensions" / "test-ext" + if not can_create_symlink(project_dir): + pytest.skip("Current platform/user cannot create symlinks") + + manager = ExtensionManager(project_dir) + + packaged_config = extension_dir / "test-ext-config.yml" + packaged_config.write_text("model: default-model\n") + + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + config_file = ext_dir / "test-ext-config.yml" + config_file.write_text("model: custom-model\nmax_iterations: 99\n") + staged_bytes = config_file.read_bytes() + + manager.remove("test-ext", keep_config=True) + assert not manager.registry.is_installed("test-ext") + + staging_dir = manager._rescue_staging_dir("test-ext") + + original_copytree = shutil.copytree + copytree_calls = 0 + + def flaky_copytree(*args, **kwargs): + nonlocal copytree_calls + copytree_calls += 1 + if copytree_calls == 1: + dst = args[1] + Path(dst).mkdir(parents=True, exist_ok=True) + (Path(dst) / "_partial.txt").write_text("partial") + raise OSError("simulated disk full") + return original_copytree(*args, **kwargs) + + monkeypatch.setattr(_ext_module.shutil, "copytree", flaky_copytree) + + with pytest.raises(OSError, match="simulated disk full"): + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + assert staging_dir.exists() + assert (staging_dir / ".rescue-complete").exists() + + # Simulate the user replacing the live config with a symlink before retry. + external_target = project_dir / "external-config.yml" + external_target.write_text("model: newer-linked-model\n") + config_file.unlink() + os.symlink(external_target, config_file) + assert config_file.is_symlink() + + with pytest.raises(ValidationError, match="Preserved extension config conflict"): + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + # Both copies survive: the live symlink choice and the staged backup. + assert config_file.is_symlink() + assert external_target.read_text() == "model: newer-linked-model\n" + assert staging_dir.exists() + assert (staging_dir / "test-ext-config.yml").read_bytes() == staged_bytes + assert not manager.registry.is_installed("test-ext") + + def test_copytree_failure_restores_stranded_config( + self, extension_dir, project_dir, monkeypatch + ): + """A copytree failure must not permanently lose a preserved config. + + When copytree raises after the existing directory has been removed, the + rollback path must write the rescued bytes back to dest_dir and restore + the original file mode, while leaving the extension unregistered. + """ + import stat + + manager = ExtensionManager(project_dir) + + # Add a packaged default config so copytree would overwrite it on success. + packaged_config = extension_dir / "test-ext-config.yml" + packaged_config.write_text("model: default-model\n") + + # Install once so the extension is on disk. + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + ext_dir = project_dir / ".specify" / "extensions" / "test-ext" + config_file = ext_dir / "test-ext-config.yml" + config_file.write_text("model: custom-model\nmax_iterations: 99\n") + + # Set a known, non-default mode so we can assert it survives the rollback. + if platform.system() != "Windows": + config_file.chmod(0o640) + original_bytes = config_file.read_bytes() + original_mode = config_file.stat().st_mode + + # Remove while preserving the config — it is now a stranded file. + manager.remove("test-ext", keep_config=True) + assert not manager.registry.is_installed("test-ext") + assert config_file.exists() + + # Make copytree create a partial destination then raise so the rollback + # path is exercised. + + def failing_copytree(src, dst, **kwargs): + Path(dst).mkdir(parents=True, exist_ok=True) + (Path(dst) / "_partial.txt").write_text("partial") + raise OSError("simulated disk full") + + monkeypatch.setattr(_ext_module.shutil, "copytree", failing_copytree) + + with pytest.raises(OSError, match="simulated disk full"): + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + # The preserved config must have been written back by the rollback path. + assert config_file.exists(), "rollback must recreate the config file" + assert config_file.read_bytes() == original_bytes + + # On POSIX, the original file mode must be faithfully restored. + if platform.system() != "Windows": + restored_mode = config_file.stat().st_mode + assert stat.S_IMODE(restored_mode) == stat.S_IMODE(original_mode) + + # The extension must remain unregistered after the failed install. + assert not manager.registry.is_installed("test-ext") + + def test_extensionignore_load_failure_preserves_kept_config( + self, extension_dir, project_dir + ): + """An .extensionignore load failure must not lose a preserved config. + + `.extensionignore` is loaded/validated before the rescue staging + directory is read or created (and thus before dest_dir is deleted), so a + ValidationError raised for invalid UTF-8 must abort the reinstall while + leaving the kept config authoritative in its documented location. It must + NOT publish a stale staging copy that a later retry — after the user + fixes the ignore file and edits the kept config — would reload and use to + silently overwrite the newer bytes. + """ + manager = ExtensionManager(project_dir) + + packaged_config = extension_dir / "test-ext-config.yml" + packaged_config.write_text("model: default-model\n") + + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + ext_dir = project_dir / ".specify" / "extensions" / "test-ext" + config_file = ext_dir / "test-ext-config.yml" + config_file.write_text("model: custom-model\nmax_iterations: 99\n") + original_bytes = config_file.read_bytes() + + manager.remove("test-ext", keep_config=True) + assert not manager.registry.is_installed("test-ext") + assert config_file.exists() + + # Author an .extensionignore that is not valid UTF-8 so the loader + # raises before rescue staging is read or created. + (extension_dir / ".extensionignore").write_bytes(b"\xff\xfe invalid\n") + + with pytest.raises(ValidationError, match="not valid UTF-8"): + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + # The kept config must remain in its documented location, untouched. + assert config_file.exists(), "config must survive the ignore-load failure" + assert config_file.read_bytes() == original_bytes + assert not manager.registry.is_installed("test-ext") + + # No rescue staging may have been published, so a later retry reads the + # live (possibly newly edited) config rather than stale staged bytes. + staging_dir = manager._rescue_staging_dir("test-ext") + assert not staging_dir.exists() + + # Simulate the user fixing the ignore file and editing the kept config, + # then retrying: the retry must adopt the newer bytes, never a stale + # staged copy. + (extension_dir / ".extensionignore").write_text("*.log\n") + config_file.write_text("model: newer-model\n") + newer_bytes = config_file.read_bytes() + + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + assert manager.registry.is_installed("test-ext") + assert config_file.read_bytes() == newer_bytes + + def test_retry_after_staging_backup_restores_stranded_config( + self, extension_dir, project_dir, monkeypatch + ): + """A retry after an interrupted install restores the rescued config. + + When the live config is unchanged since the interrupted attempt (it + still matches the staged backup), the retry proceeds and yields the + preserved bytes, and the staging directory is cleaned up on success. + """ + import stat + + manager = ExtensionManager(project_dir) + + packaged_config = extension_dir / "test-ext-config.yml" + packaged_config.write_text("model: default-model\n") + + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + ext_dir = project_dir / ".specify" / "extensions" / "test-ext" + config_file = ext_dir / "test-ext-config.yml" + config_file.write_text("model: custom-model\nmax_iterations: 99\n") + + if platform.system() != "Windows": + config_file.chmod(0o640) + original_bytes = config_file.read_bytes() + original_mode = config_file.stat().st_mode + + manager.remove("test-ext", keep_config=True) + assert not manager.registry.is_installed("test-ext") + assert config_file.exists() + + staging_dir = manager._rescue_staging_dir("test-ext") + assert not staging_dir.exists() + + original_copytree = shutil.copytree + copytree_calls = 0 + + def flaky_copytree(*args, **kwargs): + nonlocal copytree_calls + copytree_calls += 1 + if copytree_calls == 1: + dst = args[1] + Path(dst).mkdir(parents=True, exist_ok=True) + (Path(dst) / "_partial.txt").write_text("partial") + raise OSError("simulated disk full") + return original_copytree(*args, **kwargs) + + monkeypatch.setattr(_ext_module.shutil, "copytree", flaky_copytree) + + with pytest.raises(OSError, match="simulated disk full"): + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + assert staging_dir.exists() + assert (staging_dir / ".rescue-complete").exists() + assert (staging_dir / "test-ext-config.yml").exists() + + # The rollback restored the preserved bytes to the live config, so it + # still agrees with the staged backup — the retry proceeds normally. + assert config_file.read_bytes() == original_bytes + + manifest = manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + assert manifest.id == "test-ext" + assert manager.registry.is_installed("test-ext") + assert config_file.read_bytes() == original_bytes + assert not (ext_dir / "_partial.txt").exists() + assert not staging_dir.exists() + + if platform.system() != "Windows": + restored_mode = config_file.stat().st_mode + assert stat.S_IMODE(restored_mode) == stat.S_IMODE(original_mode) + + def test_retry_restores_config_from_staging_when_live_absent( + self, extension_dir, project_dir, monkeypatch + ): + """Retry succeeds using only staging when the live config is absent. + + After a copytree failure, staging is complete and the rollback writes + the config back to dest_dir. If a power loss interrupts that rollback + the config may be absent on the next attempt. The retry-from-staging + branch (``if staging_is_complete``) must restore the config from staging + alone so the original bytes and mode are recovered even when no live copy + is present. This is the critical path that distinguishes the staging + branch from the live-dir fallback. + """ + import stat + + manager = ExtensionManager(project_dir) + + packaged_config = extension_dir / "test-ext-config.yml" + packaged_config.write_text("model: default-model\n") + + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + ext_dir = project_dir / ".specify" / "extensions" / "test-ext" + config_file = ext_dir / "test-ext-config.yml" + config_file.write_text("model: custom-model\nmax_iterations: 99\n") + + if platform.system() != "Windows": + config_file.chmod(0o640) + original_bytes = config_file.read_bytes() + original_mode = config_file.stat().st_mode + + manager.remove("test-ext", keep_config=True) + assert not manager.registry.is_installed("test-ext") + assert config_file.exists() + + staging_dir = manager._rescue_staging_dir("test-ext") + assert not staging_dir.exists() + + original_copytree = shutil.copytree + copytree_calls = 0 + + def flaky_copytree(*args, **kwargs): + nonlocal copytree_calls + copytree_calls += 1 + if copytree_calls == 1: + dst = args[1] + Path(dst).mkdir(parents=True, exist_ok=True) + (Path(dst) / "_partial.txt").write_text("partial") + raise OSError("simulated disk full") + return original_copytree(*args, **kwargs) + + monkeypatch.setattr(_ext_module.shutil, "copytree", flaky_copytree) + + with pytest.raises(OSError, match="simulated disk full"): + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + # Staging is complete after the first failure. + assert staging_dir.exists() + assert (staging_dir / ".rescue-complete").exists() + assert (staging_dir / "test-ext-config.yml").exists() + + # Simulate a power loss that prevented the rollback from writing the + # config back: delete the live copy so the retry must rely on staging. + config_file.unlink() + assert not config_file.exists() + + manifest = manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + # The staging branch must restore the original bytes even though no live + # copy was present — proving staging (not the live-dir fallback) was used. + assert manifest.id == "test-ext" + assert manager.registry.is_installed("test-ext") + assert config_file.read_bytes() == original_bytes + assert not (ext_dir / "_partial.txt").exists() + assert not staging_dir.exists() + + if platform.system() != "Windows": + restored_mode = config_file.stat().st_mode + assert stat.S_IMODE(restored_mode) == stat.S_IMODE(original_mode) + + def test_retry_with_edited_live_config_aborts_and_preserves_both( + self, extension_dir, project_dir, monkeypatch + ): + """A retry must not silently overwrite a config edited after a crash. + + A complete staging directory proves only that staging finished, not + that dest_dir was modified. If the user edits the live kept config + before retrying, the retry must detect the divergence, preserve both + copies, and abort rather than blindly restoring the older staged bytes. + """ + manager = ExtensionManager(project_dir) + + packaged_config = extension_dir / "test-ext-config.yml" + packaged_config.write_text("model: default-model\n") + + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + ext_dir = project_dir / ".specify" / "extensions" / "test-ext" + config_file = ext_dir / "test-ext-config.yml" + config_file.write_text("model: custom-model\nmax_iterations: 99\n") + staged_bytes = config_file.read_bytes() + + manager.remove("test-ext", keep_config=True) + assert not manager.registry.is_installed("test-ext") + + staging_dir = manager._rescue_staging_dir("test-ext") + + original_copytree = shutil.copytree + copytree_calls = 0 + + def flaky_copytree(*args, **kwargs): + nonlocal copytree_calls + copytree_calls += 1 + if copytree_calls == 1: + dst = args[1] + Path(dst).mkdir(parents=True, exist_ok=True) + (Path(dst) / "_partial.txt").write_text("partial") + raise OSError("simulated disk full") + return original_copytree(*args, **kwargs) + + monkeypatch.setattr(_ext_module.shutil, "copytree", flaky_copytree) + + with pytest.raises(OSError, match="simulated disk full"): + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + assert staging_dir.exists() + assert (staging_dir / ".rescue-complete").exists() + + # Simulate the user editing the live config before retrying so it now + # diverges from the staged backup. + config_file.write_text("model: newer-edited-model\n") + edited_bytes = config_file.read_bytes() + assert edited_bytes != staged_bytes + + with pytest.raises(ValidationError, match="Preserved extension config conflict"): + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + # Both copies must survive: the edited live config and the staged backup. + assert config_file.read_bytes() == edited_bytes + assert staging_dir.exists() + assert (staging_dir / "test-ext-config.yml").read_bytes() == staged_bytes + assert not manager.registry.is_installed("test-ext") + + @pytest.mark.parametrize( + "failure_mode", + [ + pytest.param("mkdir", id="mkdir"), + pytest.param("os_open", id="os_open"), + pytest.param("fsync", id="fsync"), + ], + ) + def test_staging_failure_aborts_before_dest_dir_removal( + self, extension_dir, project_dir, monkeypatch, failure_mode + ): + """Staging failures abort the install before dest_dir is removed. + + When mkdir, os.open, or fsync fails while publishing rescue staging, + the install must abort before removing dest_dir so the preserved + config bytes remain authoritative, any partial staging is cleaned up + rather than trusted on retry, and the extension stays unregistered. + """ + import errno as _errno + + manager = ExtensionManager(project_dir) + + packaged_config = extension_dir / "test-ext-config.yml" + packaged_config.write_text("model: default-model\n") + + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + ext_dir = project_dir / ".specify" / "extensions" / "test-ext" + config_file = ext_dir / "test-ext-config.yml" + config_file.write_text("model: custom-model\nmax_iterations: 99\n") + original_bytes = config_file.read_bytes() + + manager.remove("test-ext", keep_config=True) + assert not manager.registry.is_installed("test-ext") + assert config_file.exists() + + staging_dir = manager._rescue_staging_dir("test-ext") + assert not staging_dir.exists() + + if failure_mode == "mkdir": + original_mkdir = Path.mkdir + + def failing_mkdir(self_path, *args, **kwargs): + if self_path == staging_dir: + raise OSError("staging mkdir failed") + return original_mkdir(self_path, *args, **kwargs) + + monkeypatch.setattr(Path, "mkdir", failing_mkdir) + + elif failure_mode == "os_open": + original_os_open = _ext_module.os.open + + def failing_os_open(path, flags, mode=0o777, *args, **kwargs): + # Fail only for file creation (O_CREAT) inside the staging + # directory so other os.open calls (e.g. directory fsync) are + # unaffected. + if str(staging_dir) in str(path) and (flags & os.O_CREAT): + raise OSError(_errno.ENOSPC, "No space left on device") + return original_os_open(path, flags, mode, *args, **kwargs) + + monkeypatch.setattr(_ext_module.os, "open", failing_os_open) + + else: # "fsync" + def failing_fsync_fd(fd: int) -> None: + # Simulate a real storage error (EIO) that the helper propagates. + raise OSError(_errno.EIO, "Input/output error") + + monkeypatch.setattr(_ext_module, "_fsync_fd", failing_fsync_fd) + + with pytest.raises(OSError): + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + # dest_dir must still exist — the install aborted before rmtree. + assert ext_dir.exists(), "dest_dir must survive a staging failure" + assert config_file.read_bytes() == original_bytes, ( + "preserved config must remain authoritative" + ) + # Partial staging must have been cleaned up and not left as complete. + assert not staging_dir.exists() or not ( + staging_dir / ".rescue-complete" + ).exists(), "incomplete staging must not be trusted" + assert not manager.registry.is_installed("test-ext") + + def test_rescue_staging_dir_is_fixed_length_for_long_ids(self, project_dir): + """The rescue staging component length must not grow with the ID length. + + Manifest validation caps the ID character set but not its length, so a + very long (but valid) ID must not lengthen the single staging path + component past a filesystem's per-component byte limit. + """ + manager = ExtensionManager(project_dir) + + short_dir = manager._rescue_staging_dir("a") + long_id = "a" * 250 + long_dir = manager._rescue_staging_dir(long_id) + + # Same fixed component length regardless of ID length. + assert len(short_dir.name) == len(long_dir.name) + # Comfortably within the common 255-byte component limit. + assert len(long_dir.name.encode("utf-8")) <= 255 + # Distinct IDs still map to distinct staging directories. + assert manager._rescue_staging_dir("b") != short_dir + + def test_failed_install_without_keep_config_does_not_rescue_defaults( + self, extension_dir, project_dir, monkeypatch + ): + """A dir left by a partially-failed install must not trigger the rescue path. + + Any install that copies files but then fails during command, skill, or + hook registration also leaves a complete dest_dir with no registry entry. + On a later retry from an updated package this branch must not treat the + previous package's default config as user-preserved data and restore it + over the new defaults. Only directories explicitly left by + ``remove --keep-config`` (which writes a ``.keep-config`` marker) should + trigger the rescue path. + """ + manager = ExtensionManager(project_dir) + + packaged_config = extension_dir / "test-ext-config.yml" + packaged_config.write_text("model: default-model\n") + + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + ext_dir = project_dir / ".specify" / "extensions" / "test-ext" + config_file = ext_dir / "test-ext-config.yml" + + # Simulate a partially-failed install: the extension directory is present + # with the packaged default config but there is no .keep-config marker and + # the extension is not in the registry. This matches what happens when + # copytree succeeds but command/hook registration raises afterwards. + manager.registry.remove("test-ext") + assert not manager.registry.is_installed("test-ext") + assert ext_dir.exists() + assert not (ext_dir / ".keep-config").exists() + + # Update the packaged config so a retry with the new package would use + # different defaults — the old defaults must NOT be rescued. + packaged_config.write_text("model: updated-default-model\n") + + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + assert manager.registry.is_installed("test-ext") + # The new packaged default must win; the old default was not user data. + assert config_file.read_text() == "model: updated-default-model\n" + def test_install_force_without_existing(self, extension_dir, project_dir): """Test force-install when extension is NOT already installed (works normally).""" manager = ExtensionManager(project_dir)