From 1bf70bafe63a2bf8839329f7d844031d21a1cfbf Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 10 Jul 2026 12:34:11 +0000 Subject: [PATCH] Fix reinstall-overwrites-kept-config: preserve config on plain reinstall after --keep-config Apply the remediation from the bug assessment on issue #3427. Before the unconditional shutil.rmtree(dest_dir), scan dest_dir for any *-config.yml and *-config.local.yml files and hold their contents in memory. After shutil.copytree succeeds, write them back so user-customized values always win over the packaged defaults. This mirrors the existing backup/restore logic for the --force reinstall path but handles the case where remove --keep-config left config files behind in an unregistered extension directory. Refs #3427 Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/extensions/__init__.py | 20 +++++++++ tests/test_extensions.py | 58 +++++++++++++++++++++++++- 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index b84b836545..6f179cc53a 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -1397,6 +1397,22 @@ def install_from_directory( backup_config_dir.unlink() did_remove = self.remove(manifest.id) + # 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 now and write them back after copytree so the user's values + # always win over the packaged defaults. + stranded_configs: dict[str, bytes] = {} + if dest_dir.exists() and not self.registry.is_installed(manifest.id): + for cfg_file in ( + list(dest_dir.glob("*-config.yml")) + + list(dest_dir.glob("*-config.local.yml")) + ): + if cfg_file.is_file() and not cfg_file.is_symlink(): + stranded_configs[cfg_file.name] = cfg_file.read_bytes() + # Install extension (dest_dir computed above during self-install guard) if dest_dir.exists(): shutil.rmtree(dest_dir) @@ -1427,6 +1443,10 @@ def install_from_directory( hook_executor = HookExecutor(self.project_root) hook_executor.register_hooks(manifest) + # Restore stranded configs rescued before the rmtree above. + for filename, content in stranded_configs.items(): + (dest_dir / filename).write_bytes(content) + # Restore config files from backup when --force triggered a removal. # Only restore *.yml config files to match what remove() backs up, # so unexpected artifacts in .backup/ are not resurrected. diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 2885180360..73c1b138cf 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -1226,7 +1226,63 @@ def test_install_force_config_preserved(self, extension_dir, project_dir): assert new_config.exists() assert new_config.read_text() == "test: config" - def test_install_force_without_existing(self, extension_dir, project_dir): + 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) + + # Install once + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + # Customize the config file + 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) + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + # Preserved config must survive the reinstall + assert config_file.exists() + assert "custom-model" in config_file.read_text() + assert "99" 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() + + """Test force-install when extension is NOT already installed (works normally).""" manager = ExtensionManager(project_dir)