Harden preserved-config rescue against divergence and long names

Address three review findings on the reinstall config-rescue path:

- A complete .rescue-complete marker proves only that staging finished,
  not that dest_dir was modified. A crash after staging sync but before
  the rmtree leaves the live kept config intact; if the user edits it
  before retrying, preferring the staged bytes silently overwrote the
  newer config. The two copies are indistinguishable in provenance from
  disk, so detect divergence between a complete staging copy and the live
  config and abort (preserving both) instead of unconditionally choosing
  staging.
- The staging directory embedded the full extension ID in one path
  component. Extension IDs are length-unbounded, so a valid long ID could
  install at dest_dir yet fail every reinstall-after-keep-config with
  ENAMETOOLONG. Derive the staging component from a fixed-length hash via
  a new _rescue_staging_dir() helper.
- The stranded-config restore used the full config filename as a
  NamedTemporaryFile prefix; a name already near the component limit plus
  the random suffix raised ENAMETOOLONG. Use a short fixed prefix.

Updates the retry regression test to the new divergence semantics and
adds conflict-abort, long-ID, and fixed-prefix coverage.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Author: mnriem

src/specify_cli/extensions/__init__.py

@@ -746,6 +746,20 @@ 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-<id>`` 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 _collect_manifest_command_names(manifest: ExtensionManifest) -> Dict[str, str]:
         """Collect command and alias names declared by a manifest.
@@ -1470,7 +1484,7 @@ def install_from_directory(
         # staging dir is removed only after every config has been successfully
         # restored.
         stranded_configs: dict[str, tuple[bytes, int]] = {}
-        rescue_staging_dir = self.extensions_dir / f".rescue-staging-{manifest.id}"
+        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
@@ -1492,16 +1506,49 @@ def install_from_directory(
             # 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.
+            conflicting: list[str] = []
             for staged_file in sorted(rescue_staging_dir.iterdir()):
                 if (
                     staged_file.is_file()
                     and not staged_file.is_symlink()
                     and staged_file.name.endswith(("-config.yml", "-config.local.yml"))
                 ):
+                    staged_bytes = staged_file.read_bytes()
+                    live_file = dest_dir / staged_file.name
+                    if live_file.is_file() and not live_file.is_symlink():
+                        try:
+                            live_bytes = live_file.read_bytes()
+                        except OSError:
+                            live_bytes = None
+                        if live_bytes is not None and live_bytes != staged_bytes:
+                            conflicting.append(staged_file.name)
                     stranded_configs[staged_file.name] = (
-                        staged_file.read_bytes(),
+                        staged_bytes,
                         staged_file.stat().st_mode,
                     )
+            if conflicting:
+                names = ", ".join(sorted(conflicting))
+                raise ValidationError(
+                    "Preserved extension config conflict for "
+                    f"'{manifest.id}': the current config file(s) ({names}) in "
+                    f"{dest_dir} differ from the rescued backup left by an "
+                    f"interrupted install in {rescue_staging_dir}. Both copies "
+                    "have been preserved. Resolve manually — keep the current "
+                    f"file and delete {rescue_staging_dir}, or restore the "
+                    "backup over the current file — then reinstall."
+                )
         elif dest_dir.exists() and not self.registry.is_installed(manifest.id):
             for cfg_file in (
                 list(dest_dir.glob("*-config.yml"))
@@ -1599,10 +1646,16 @@ def _restore_stranded_config_file(
         ) -> 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=f".{target.name}.",
+                    prefix=".cfg-restore.",
                     delete=False,
                 ) as tmp:
                     tmp_path = Path(tmp.name)

tests/test_extensions.py

@@ -1421,7 +1421,12 @@ def test_extensionignore_load_failure_preserves_kept_config(
     def test_retry_after_staging_backup_restores_stranded_config(
         self, extension_dir, project_dir, monkeypatch
     ):
-        """A retry after an interrupted install should restore the rescued config from staging."""
+        """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)
@@ -1446,7 +1451,7 @@ def test_retry_after_staging_backup_restores_stranded_config(
         assert not manager.registry.is_installed("test-ext")
         assert config_file.exists()
 
-        staging_dir = manager.extensions_dir / ".rescue-staging-test-ext"
+        staging_dir = manager._rescue_staging_dir("test-ext")
         assert not staging_dir.exists()
 
         original_copytree = shutil.copytree
@@ -1473,11 +1478,9 @@ def flaky_copytree(*args, **kwargs):
         assert (staging_dir / ".rescue-complete").exists()
         assert (staging_dir / "test-ext-config.yml").exists()
 
-        # Corrupt the rollback-restored copy to prove the next retry must rely
-        # on the durable staging backup, not whatever was written to dest_dir
-        # by the earlier failed install attempt.
-        config_file.write_text("model: wrong-model\n", encoding="utf-8")
-        assert config_file.read_bytes() != original_bytes
+        # 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
@@ -1493,6 +1496,95 @@ def flaky_copytree(*args, **kwargs):
             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")
+
+    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_install_force_without_existing(self, extension_dir, project_dir):
         """Test force-install when extension is NOT already installed (works normally)."""
         manager = ExtensionManager(project_dir)