Skip to content

Commit 82cceae

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 82cceae

File tree

3 files changed

+265
-18
lines changed

3 files changed

+265
-18
lines changed

src/specify_cli/__init__.py

Lines changed: 35 additions & 8 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,6 +2650,9 @@ def extension_update(
26492650
for ext_id in extensions_to_update:
26502651
# Get installed version
26512652
metadata = manager.registry.get(ext_id)
2653+
if metadata is None or "version" not in metadata:
2654+
console.print(f"⚠ {ext_id}: Registry entry corrupted or missing (skipping)")
2655+
continue
26522656
installed_version = pkg_version.Version(metadata["version"])
26532657

26542658
# Get catalog info
@@ -2803,7 +2807,14 @@ def extension_update(
28032807

28042808
# 9. Restore metadata from backup (installed_at, enabled state)
28052809
if backup_registry_entry:
2806-
new_metadata = manager.registry.get(extension_id)
2810+
# Copy current registry entry to avoid mutating internal
2811+
# registry state before explicit restore().
2812+
current_metadata = manager.registry.get(extension_id)
2813+
if current_metadata is None:
2814+
raise RuntimeError(
2815+
f"Registry entry for '{extension_id}' missing after install — update incomplete"
2816+
)
2817+
new_metadata = dict(current_metadata)
28072818

28082819
# Preserve the original installation timestamp
28092820
if "installed_at" in backup_registry_entry:
@@ -2813,7 +2824,9 @@ def extension_update(
28132824
if not backup_registry_entry.get("enabled", True):
28142825
new_metadata["enabled"] = False
28152826

2816-
manager.registry.update(extension_id, new_metadata)
2827+
# Use restore() instead of update() because update() always
2828+
# preserves the existing installed_at, ignoring our override
2829+
manager.registry.restore(extension_id, new_metadata)
28172830

28182831
# Also disable hooks in extensions.yml if extension was disabled
28192832
if not backup_registry_entry.get("enabled", True):
@@ -2860,7 +2873,10 @@ def extension_update(
28602873
# (files that weren't in the original backup)
28612874
try:
28622875
new_registry_entry = manager.registry.get(extension_id)
2863-
new_registered_commands = new_registry_entry.get("registered_commands", {})
2876+
if new_registry_entry is None:
2877+
new_registered_commands = {}
2878+
else:
2879+
new_registered_commands = new_registry_entry.get("registered_commands", {})
28642880
for agent_name, cmd_names in new_registered_commands.items():
28652881
if agent_name not in registrar.AGENT_CONFIGS:
28662882
continue
@@ -2926,7 +2942,7 @@ def extension_update(
29262942
if backup_registry_entry:
29272943
manager.registry.restore(extension_id, backup_registry_entry)
29282944

2929-
console.print(f" [green]✓[/green] Rollback successful")
2945+
console.print(" [green]✓[/green] Rollback successful")
29302946
# Clean up backup directory only on successful rollback
29312947
if backup_base.exists():
29322948
shutil.rmtree(backup_base)
@@ -2944,6 +2960,9 @@ def extension_update(
29442960
console.print(f" • {ext_name}: {error}")
29452961
raise typer.Exit(1)
29462962

2963+
except ValidationError as e:
2964+
console.print(f"\n[red]Validation Error:[/red] {e}")
2965+
raise typer.Exit(1)
29472966
except ExtensionError as e:
29482967
console.print(f"\n[red]Error:[/red] {e}")
29492968
raise typer.Exit(1)
@@ -2974,6 +2993,10 @@ def extension_enable(
29742993

29752994
# Update registry
29762995
metadata = manager.registry.get(extension_id)
2996+
if metadata is None:
2997+
console.print(f"[red]Error:[/red] Extension '{extension_id}' not found in registry (corrupted state)")
2998+
raise typer.Exit(1)
2999+
29773000
if metadata.get("enabled", True):
29783001
console.print(f"[yellow]Extension '{display_name}' is already enabled[/yellow]")
29793002
raise typer.Exit(0)
@@ -3018,6 +3041,10 @@ def extension_disable(
30183041

30193042
# Update registry
30203043
metadata = manager.registry.get(extension_id)
3044+
if metadata is None:
3045+
console.print(f"[red]Error:[/red] Extension '{extension_id}' not found in registry (corrupted state)")
3046+
raise typer.Exit(1)
3047+
30213048
if not metadata.get("enabled", True):
30223049
console.print(f"[yellow]Extension '{display_name}' is already disabled[/yellow]")
30233050
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)