Skip to content

Commit 08c7e70

Browse files
jawwad-aliclaude
andcommitted
fix(extensions): preserve keep-config leftover on reinstall (data loss)
remove(keep_config=True) intentionally leaves the top-level *-config.yml / *-config.local.yml in place and drops the registry entry. A subsequent install_from_directory is not a --force removal (did_remove is False), so the unconditional 'if dest_dir.exists(): shutil.rmtree(dest_dir)' wiped the preserved config and the backup-restore path never ran — silently destroying user configuration the feature promised to keep. Capture the top-level *-config.yml/*-config.local.yml from an existing dest_dir before the rmtree and write them back after the fresh copytree, mirroring the *-config filter the --force restore path already uses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 983a87f commit 08c7e70

2 files changed

Lines changed: 49 additions & 0 deletions

File tree

src/specify_cli/extensions/__init__.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1397,13 +1397,39 @@ def install_from_directory(
13971397
backup_config_dir.unlink()
13981398
did_remove = self.remove(manifest.id)
13991399

1400+
# A prior `remove(keep_config=True)` leaves the top-level
1401+
# *-config.yml / *-config.local.yml in place while dropping the
1402+
# registry entry. A later reinstall is not a --force removal
1403+
# (did_remove is False), so the rmtree below would wipe those
1404+
# preserved configs and the backup-restore path never runs. Capture
1405+
# them here and write them back after the fresh copytree, mirroring
1406+
# the *-config filter the --force restore path uses.
1407+
preserved_configs: dict[str, bytes] = {}
1408+
if dest_dir.exists():
1409+
for cfg_file in dest_dir.iterdir():
1410+
if (
1411+
cfg_file.is_file()
1412+
and not cfg_file.is_symlink()
1413+
and (
1414+
cfg_file.name.endswith("-config.yml")
1415+
or cfg_file.name.endswith("-config.local.yml")
1416+
)
1417+
):
1418+
preserved_configs[cfg_file.name] = cfg_file.read_bytes()
1419+
14001420
# Install extension (dest_dir computed above during self-install guard)
14011421
if dest_dir.exists():
14021422
shutil.rmtree(dest_dir)
14031423

14041424
ignore_fn = self._load_extensionignore(source_dir)
14051425
shutil.copytree(source_dir, dest_dir, ignore=ignore_fn)
14061426

1427+
# Restore configs preserved from a keep-config leftover (see above).
1428+
# The --force backup-restore below (did_remove) still takes precedence
1429+
# for that path, which uses .backup/ rather than an in-place leftover.
1430+
for name, data in preserved_configs.items():
1431+
(dest_dir / name).write_bytes(data)
1432+
14071433
# Register commands with AI agents
14081434
registered_commands = {}
14091435
if register_commands:

tests/test_extensions.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1557,6 +1557,29 @@ def test_config_backup_on_remove(self, extension_dir, project_dir):
15571557
assert backup_file.exists()
15581558
assert backup_file.read_text() == "test: config"
15591559

1560+
def test_reinstall_preserves_keep_config_leftover(self, extension_dir, project_dir):
1561+
"""A reinstall after `remove(keep_config=True)` must not wipe the
1562+
preserved config. remove(keep_config=True) leaves the *-config.yml in
1563+
place and drops the registry entry; the subsequent reinstall is not a
1564+
--force removal, so the unconditional rmtree used to destroy it."""
1565+
manager = ExtensionManager(project_dir)
1566+
manager.install_from_directory(extension_dir, "0.1.0", register_commands=False)
1567+
1568+
ext_dir = project_dir / ".specify" / "extensions" / "test-ext"
1569+
config_file = ext_dir / "test-ext-config.yml"
1570+
config_file.write_text("api_key: MY-CUSTOMIZED-VALUE")
1571+
1572+
# keep-config removal: registry entry dropped, config left in place.
1573+
assert manager.remove("test-ext", keep_config=True) is True
1574+
assert config_file.exists()
1575+
1576+
# Reinstall (no --force; registry now reports not-installed).
1577+
manager.install_from_directory(extension_dir, "0.1.0", register_commands=False)
1578+
1579+
# The user's config must survive the reinstall (was silently wiped).
1580+
assert config_file.exists()
1581+
assert "MY-CUSTOMIZED-VALUE" in config_file.read_text()
1582+
15601583

15611584
# ===== CommandRegistrar Tests =====
15621585

0 commit comments

Comments
 (0)