Skip to content

Commit 769f3ef

Browse files
iamaeroplaneclaude
andcommitted
fix(lint): address ruff linting errors and registry.update() semantics
- Remove unused import ExtensionError in extension_info - Remove extraneous f-prefix from strings without placeholders - Use registry.restore() instead of registry.update() for installed_at preservation (update() always preserves existing installed_at, ignoring our override) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 9f63a9c commit 769f3ef

3 files changed

Lines changed: 305 additions & 20 deletions

File tree

src/specify_cli/__init__.py

Lines changed: 66 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1847,7 +1847,7 @@ def _resolve_installed_extension(
18471847
for ext in name_matches:
18481848
table.add_row(ext.get("id", ""), ext.get("name", ""), str(ext.get("version", "")))
18491849
console.print(table)
1850-
console.print(f"\nPlease rerun using the extension ID:")
1850+
console.print("\nPlease rerun using the extension ID:")
18511851
console.print(f" [bold]specify extension {command_name} <extension-id>[/bold]")
18521852
raise typer.Exit(1)
18531853
else:
@@ -1910,7 +1910,7 @@ def _resolve_catalog_extension(
19101910
ext.get("_catalog_name", ""),
19111911
)
19121912
console.print(table)
1913-
console.print(f"\nPlease rerun using the extension ID:")
1913+
console.print("\nPlease rerun using the extension ID:")
19141914
console.print(f" [bold]specify extension {command_name} <extension-id>[/bold]")
19151915
raise typer.Exit(1)
19161916

@@ -2426,7 +2426,7 @@ def extension_info(
24262426
extension: str = typer.Argument(help="Extension ID or name"),
24272427
):
24282428
"""Show detailed information about an extension."""
2429-
from .extensions import ExtensionCatalog, ExtensionManager, ExtensionError
2429+
from .extensions import ExtensionCatalog, ExtensionManager
24302430

24312431
project_root = Path.cwd()
24322432

@@ -2488,7 +2488,7 @@ def extension_info(
24882488
console.print(f"[yellow]Catalog unavailable:[/yellow] {catalog_error}")
24892489
console.print("[dim]Note: Using locally installed extension; catalog info could not be verified.[/dim]")
24902490
else:
2491-
console.print(f"[yellow]Note:[/yellow] Not found in catalog (custom/local extension)")
2491+
console.print("[yellow]Note:[/yellow] Not found in catalog (custom/local extension)")
24922492

24932493
console.print()
24942494
console.print("[green]✓ Installed[/green]")
@@ -2608,6 +2608,7 @@ def extension_update(
26082608
ExtensionManager,
26092609
ExtensionCatalog,
26102610
ExtensionError,
2611+
ValidationError,
26112612
CommandRegistrar,
26122613
HookExecutor,
26132614
)
@@ -2649,7 +2650,16 @@ def extension_update(
26492650
for ext_id in extensions_to_update:
26502651
# Get installed version
26512652
metadata = manager.registry.get(ext_id)
2652-
installed_version = pkg_version.Version(metadata["version"])
2653+
if metadata is None or "version" not in metadata:
2654+
console.print(f"⚠ {ext_id}: Registry entry corrupted or missing (skipping)")
2655+
continue
2656+
try:
2657+
installed_version = pkg_version.Version(metadata["version"])
2658+
except pkg_version.InvalidVersion:
2659+
console.print(
2660+
f"⚠ {ext_id}: Invalid installed version '{metadata.get('version')}' in registry (skipping)"
2661+
)
2662+
continue
26532663

26542664
# Get catalog info
26552665
ext_info = catalog.get_extension_info(ext_id)
@@ -2662,7 +2672,13 @@ def extension_update(
26622672
console.print(f"⚠ {ext_id}: Updates not allowed from '{ext_info.get('_catalog_name', 'catalog')}' (skipping)")
26632673
continue
26642674

2665-
catalog_version = pkg_version.Version(ext_info["version"])
2675+
try:
2676+
catalog_version = pkg_version.Version(ext_info["version"])
2677+
except pkg_version.InvalidVersion:
2678+
console.print(
2679+
f"⚠ {ext_id}: Invalid catalog version '{ext_info.get('version')}' (skipping)"
2680+
)
2681+
continue
26662682

26672683
if catalog_version > installed_version:
26682684
updates_available.append(
@@ -2710,6 +2726,7 @@ def extension_update(
27102726
backup_base = manager.extensions_dir / ".backup" / f"{extension_id}-update"
27112727
backup_ext_dir = backup_base / "extension"
27122728
backup_commands_dir = backup_base / "commands"
2729+
backup_config_dir = backup_base / "config"
27132730

27142731
# Store backup state
27152732
backup_registry_entry = None
@@ -2728,6 +2745,15 @@ def extension_update(
27282745
shutil.rmtree(backup_ext_dir)
27292746
shutil.copytree(extension_dir, backup_ext_dir)
27302747

2748+
# Backup config files separately so they can be restored
2749+
# after a successful install (install_from_directory clears dest dir).
2750+
config_files = list(extension_dir.glob("*-config.yml")) + list(
2751+
extension_dir.glob("*-config.local.yml")
2752+
)
2753+
for cfg_file in config_files:
2754+
backup_config_dir.mkdir(parents=True, exist_ok=True)
2755+
shutil.copy2(cfg_file, backup_config_dir / cfg_file.name)
2756+
27312757
# 3. Backup command files for all agents
27322758
registered_commands = backup_registry_entry.get("registered_commands", {})
27332759
for agent_name, cmd_names in registered_commands.items():
@@ -2801,9 +2827,23 @@ def extension_update(
28012827
# 8. Install new version
28022828
_ = manager.install_from_zip(zip_path, speckit_version)
28032829

2830+
# Restore user config files from backup after successful install.
2831+
new_extension_dir = manager.extensions_dir / extension_id
2832+
if backup_config_dir.exists() and new_extension_dir.exists():
2833+
for cfg_file in backup_config_dir.iterdir():
2834+
if cfg_file.is_file():
2835+
shutil.copy2(cfg_file, new_extension_dir / cfg_file.name)
2836+
28042837
# 9. Restore metadata from backup (installed_at, enabled state)
28052838
if backup_registry_entry:
2806-
new_metadata = manager.registry.get(extension_id)
2839+
# Copy current registry entry to avoid mutating internal
2840+
# registry state before explicit restore().
2841+
current_metadata = manager.registry.get(extension_id)
2842+
if current_metadata is None:
2843+
raise RuntimeError(
2844+
f"Registry entry for '{extension_id}' missing after install — update incomplete"
2845+
)
2846+
new_metadata = dict(current_metadata)
28072847

28082848
# Preserve the original installation timestamp
28092849
if "installed_at" in backup_registry_entry:
@@ -2813,7 +2853,9 @@ def extension_update(
28132853
if not backup_registry_entry.get("enabled", True):
28142854
new_metadata["enabled"] = False
28152855

2816-
manager.registry.update(extension_id, new_metadata)
2856+
# Use restore() instead of update() because update() always
2857+
# preserves the existing installed_at, ignoring our override
2858+
manager.registry.restore(extension_id, new_metadata)
28172859

28182860
# Also disable hooks in extensions.yml if extension was disabled
28192861
if not backup_registry_entry.get("enabled", True):
@@ -2860,7 +2902,10 @@ def extension_update(
28602902
# (files that weren't in the original backup)
28612903
try:
28622904
new_registry_entry = manager.registry.get(extension_id)
2863-
new_registered_commands = new_registry_entry.get("registered_commands", {})
2905+
if new_registry_entry is None:
2906+
new_registered_commands = {}
2907+
else:
2908+
new_registered_commands = new_registry_entry.get("registered_commands", {})
28642909
for agent_name, cmd_names in new_registered_commands.items():
28652910
if agent_name not in registrar.AGENT_CONFIGS:
28662911
continue
@@ -2926,7 +2971,7 @@ def extension_update(
29262971
if backup_registry_entry:
29272972
manager.registry.restore(extension_id, backup_registry_entry)
29282973

2929-
console.print(f" [green]✓[/green] Rollback successful")
2974+
console.print(" [green]✓[/green] Rollback successful")
29302975
# Clean up backup directory only on successful rollback
29312976
if backup_base.exists():
29322977
shutil.rmtree(backup_base)
@@ -2944,6 +2989,9 @@ def extension_update(
29442989
console.print(f" • {ext_name}: {error}")
29452990
raise typer.Exit(1)
29462991

2992+
except ValidationError as e:
2993+
console.print(f"\n[red]Validation Error:[/red] {e}")
2994+
raise typer.Exit(1)
29472995
except ExtensionError as e:
29482996
console.print(f"\n[red]Error:[/red] {e}")
29492997
raise typer.Exit(1)
@@ -2974,6 +3022,10 @@ def extension_enable(
29743022

29753023
# Update registry
29763024
metadata = manager.registry.get(extension_id)
3025+
if metadata is None:
3026+
console.print(f"[red]Error:[/red] Extension '{extension_id}' not found in registry (corrupted state)")
3027+
raise typer.Exit(1)
3028+
29773029
if metadata.get("enabled", True):
29783030
console.print(f"[yellow]Extension '{display_name}' is already enabled[/yellow]")
29793031
raise typer.Exit(0)
@@ -3018,6 +3070,10 @@ def extension_disable(
30183070

30193071
# Update registry
30203072
metadata = manager.registry.get(extension_id)
3073+
if metadata is None:
3074+
console.print(f"[red]Error:[/red] Extension '{extension_id}' not found in registry (corrupted state)")
3075+
raise typer.Exit(1)
3076+
30213077
if not metadata.get("enabled", True):
30223078
console.print(f"[yellow]Extension '{display_name}' is already disabled[/yellow]")
30233079
raise typer.Exit(0)

src/specify_cli/extensions.py

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import tempfile
1313
import zipfile
1414
import shutil
15+
import copy
1516
from dataclasses import dataclass
1617
from pathlib import Path
1718
from typing import Optional, Dict, List, Any, Callable, Set
@@ -250,12 +251,15 @@ def update(self, extension_id: str, metadata: dict):
250251
raise KeyError(f"Extension '{extension_id}' is not installed")
251252
# Merge new metadata with existing, preserving original installed_at
252253
existing = self.data["extensions"][extension_id]
253-
original_installed_at = existing.get("installed_at")
254254
# Merge: existing fields preserved, new fields override
255255
merged = {**existing, **metadata}
256-
# Always preserve original installed_at
257-
if original_installed_at:
258-
merged["installed_at"] = original_installed_at
256+
# Always preserve original installed_at based on key existence, not truthiness,
257+
# to handle cases where the field exists but may be falsy (legacy/corruption)
258+
if "installed_at" in existing:
259+
merged["installed_at"] = existing["installed_at"]
260+
else:
261+
# If not present in existing, explicitly remove from merged if caller provided it
262+
merged.pop("installed_at", None)
259263
self.data["extensions"][extension_id] = merged
260264
self._save()
261265

@@ -270,7 +274,7 @@ def restore(self, extension_id: str, metadata: dict):
270274
extension_id: Extension ID
271275
metadata: Complete extension metadata including installed_at
272276
"""
273-
self.data["extensions"][extension_id] = metadata
277+
self.data["extensions"][extension_id] = dict(metadata)
274278
self._save()
275279

276280
def remove(self, extension_id: str):
@@ -286,21 +290,28 @@ def remove(self, extension_id: str):
286290
def get(self, extension_id: str) -> Optional[dict]:
287291
"""Get extension metadata from registry.
288292
293+
Returns a deep copy to prevent callers from accidentally mutating
294+
nested internal registry state without going through the write path.
295+
289296
Args:
290297
extension_id: Extension ID
291298
292299
Returns:
293-
Extension metadata or None if not found
300+
Deep copy of extension metadata, or None if not found
294301
"""
295-
return self.data["extensions"].get(extension_id)
302+
entry = self.data["extensions"].get(extension_id)
303+
return copy.deepcopy(entry) if entry is not None else None
296304

297305
def list(self) -> Dict[str, dict]:
298306
"""Get all installed extensions.
299307
308+
Returns a deep copy of the extensions mapping to prevent callers
309+
from accidentally mutating nested internal registry state.
310+
300311
Returns:
301-
Dictionary of extension_id -> metadata
312+
Dictionary of extension_id -> metadata (deep copies)
302313
"""
303-
return self.data["extensions"]
314+
return copy.deepcopy(self.data["extensions"])
304315

305316
def is_installed(self, extension_id: str) -> bool:
306317
"""Check if extension is installed.
@@ -645,7 +656,7 @@ def list_installed(self) -> List[Dict[str, Any]]:
645656
result.append({
646657
"id": ext_id,
647658
"name": manifest.name,
648-
"version": metadata["version"],
659+
"version": metadata.get("version", "unknown"),
649660
"description": manifest.description,
650661
"enabled": metadata.get("enabled", True),
651662
"installed_at": metadata.get("installed_at"),

0 commit comments

Comments
 (0)