Skip to content

Commit cc61b4a

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 cc61b4a

File tree

3 files changed

+55
-18
lines changed

3 files changed

+55
-18
lines changed

src/specify_cli/__init__.py

Lines changed: 31 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]")
@@ -2649,6 +2649,9 @@ def extension_update(
26492649
for ext_id in extensions_to_update:
26502650
# Get installed version
26512651
metadata = manager.registry.get(ext_id)
2652+
if metadata is None or "version" not in metadata:
2653+
console.print(f"⚠ {ext_id}: Registry entry corrupted or missing (skipping)")
2654+
continue
26522655
installed_version = pkg_version.Version(metadata["version"])
26532656

26542657
# Get catalog info
@@ -2803,7 +2806,14 @@ def extension_update(
28032806

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

28082818
# Preserve the original installation timestamp
28092819
if "installed_at" in backup_registry_entry:
@@ -2813,7 +2823,9 @@ def extension_update(
28132823
if not backup_registry_entry.get("enabled", True):
28142824
new_metadata["enabled"] = False
28152825

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

28182830
# Also disable hooks in extensions.yml if extension was disabled
28192831
if not backup_registry_entry.get("enabled", True):
@@ -2860,7 +2872,10 @@ def extension_update(
28602872
# (files that weren't in the original backup)
28612873
try:
28622874
new_registry_entry = manager.registry.get(extension_id)
2863-
new_registered_commands = new_registry_entry.get("registered_commands", {})
2875+
if new_registry_entry is None:
2876+
new_registered_commands = {}
2877+
else:
2878+
new_registered_commands = new_registry_entry.get("registered_commands", {})
28642879
for agent_name, cmd_names in new_registered_commands.items():
28652880
if agent_name not in registrar.AGENT_CONFIGS:
28662881
continue
@@ -2926,7 +2941,7 @@ def extension_update(
29262941
if backup_registry_entry:
29272942
manager.registry.restore(extension_id, backup_registry_entry)
29282943

2929-
console.print(f" [green]✓[/green] Rollback successful")
2944+
console.print(" [green]✓[/green] Rollback successful")
29302945
# Clean up backup directory only on successful rollback
29312946
if backup_base.exists():
29322947
shutil.rmtree(backup_base)
@@ -2974,6 +2989,10 @@ def extension_enable(
29742989

29752990
# Update registry
29762991
metadata = manager.registry.get(extension_id)
2992+
if metadata is None:
2993+
console.print(f"[red]Error:[/red] Extension '{extension_id}' not found in registry (corrupted state)")
2994+
raise typer.Exit(1)
2995+
29772996
if metadata.get("enabled", True):
29782997
console.print(f"[yellow]Extension '{display_name}' is already enabled[/yellow]")
29792998
raise typer.Exit(0)
@@ -3018,6 +3037,10 @@ def extension_disable(
30183037

30193038
# Update registry
30203039
metadata = manager.registry.get(extension_id)
3040+
if metadata is None:
3041+
console.print(f"[red]Error:[/red] Extension '{extension_id}' not found in registry (corrupted state)")
3042+
raise typer.Exit(1)
3043+
30213044
if not metadata.get("enabled", True):
30223045
console.print(f"[yellow]Extension '{display_name}' is already disabled[/yellow]")
30233046
raise typer.Exit(0)

src/specify_cli/extensions.py

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -250,12 +250,15 @@ def update(self, extension_id: str, metadata: dict):
250250
raise KeyError(f"Extension '{extension_id}' is not installed")
251251
# Merge new metadata with existing, preserving original installed_at
252252
existing = self.data["extensions"][extension_id]
253-
original_installed_at = existing.get("installed_at")
254253
# Merge: existing fields preserved, new fields override
255254
merged = {**existing, **metadata}
256-
# Always preserve original installed_at
257-
if original_installed_at:
258-
merged["installed_at"] = original_installed_at
255+
# Always preserve original installed_at based on key existence, not truthiness,
256+
# to handle cases where the field exists but may be falsy (legacy/corruption)
257+
if "installed_at" in existing:
258+
merged["installed_at"] = existing["installed_at"]
259+
else:
260+
# If not present in existing, explicitly remove from merged if caller provided it
261+
merged.pop("installed_at", None)
259262
self.data["extensions"][extension_id] = merged
260263
self._save()
261264

@@ -270,7 +273,7 @@ def restore(self, extension_id: str, metadata: dict):
270273
extension_id: Extension ID
271274
metadata: Complete extension metadata including installed_at
272275
"""
273-
self.data["extensions"][extension_id] = metadata
276+
self.data["extensions"][extension_id] = dict(metadata)
274277
self._save()
275278

276279
def remove(self, extension_id: str):
@@ -286,21 +289,28 @@ def remove(self, extension_id: str):
286289
def get(self, extension_id: str) -> Optional[dict]:
287290
"""Get extension metadata from registry.
288291
292+
Returns a shallow copy to prevent callers from accidentally mutating
293+
internal registry state without going through the write path.
294+
289295
Args:
290296
extension_id: Extension ID
291297
292298
Returns:
293-
Extension metadata or None if not found
299+
Shallow copy of extension metadata, or None if not found
294300
"""
295-
return self.data["extensions"].get(extension_id)
301+
entry = self.data["extensions"].get(extension_id)
302+
return dict(entry) if entry is not None else None
296303

297304
def list(self) -> Dict[str, dict]:
298305
"""Get all installed extensions.
299306
307+
Returns a shallow copy of the extensions mapping to prevent callers
308+
from accidentally mutating internal registry state.
309+
300310
Returns:
301-
Dictionary of extension_id -> metadata
311+
Dictionary of extension_id -> metadata (shallow copies)
302312
"""
303-
return self.data["extensions"]
313+
return {k: dict(v) for k, v in self.data["extensions"].items()}
304314

305315
def is_installed(self, extension_id: str) -> bool:
306316
"""Check if extension is installed.
@@ -645,7 +655,7 @@ def list_installed(self) -> List[Dict[str, Any]]:
645655
result.append({
646656
"id": ext_id,
647657
"name": manifest.name,
648-
"version": metadata["version"],
658+
"version": metadata.get("version", "unknown"),
649659
"description": manifest.description,
650660
"enabled": metadata.get("enabled", True),
651661
"installed_at": metadata.get("installed_at"),

tests/test_extensions.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2104,6 +2104,10 @@ def mock_download(extension_id):
21042104
catch_exceptions=True,
21052105
)
21062106

2107+
assert result.exit_code != 0, (
2108+
f"Expected non-zero exit code since mock download raises, got {result.exit_code}"
2109+
)
2110+
21072111
# Verify download_extension was called with the resolved ID, not the display name
21082112
assert len(download_called_with) == 1
21092113
assert download_called_with[0] == "acme-jira-integration", (

0 commit comments

Comments
 (0)