Skip to content

feat: add --dev flag to extension update command #2006

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Testimonial wants to merge 1 commit into
base: main
Choose a base branch
from
+106 −1
Open

feat: add --dev flag to extension update command #2006

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 106 additions & 1 deletion src/specify_cli/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4024,6 +4024,7 @@ def _print_extension_info(ext_info: dict, manager):
@extension_app.command("update")
def extension_update(
extension: str = typer.Argument(None, help="Extension ID or name to update (or all)"),
dev: bool = typer.Option(False, "--dev", help="Update from local directory (re-copies source, preserves config)"),
):
"""Update extension(s) to latest version."""
from .extensions import (
Expand All @@ -4048,9 +4049,113 @@ def extension_update(
raise typer.Exit(1)

manager = ExtensionManager(project_root)
catalog = ExtensionCatalog(project_root)
speckit_version = get_speckit_version()

# ── Dev mode: update from local directory ──────────────────────────
if dev:
if not extension:
console.print("[red]Error:[/red] --dev requires extension path argument")
console.print("Usage: specify extension update --dev /path/to/extension")
raise typer.Exit(1)
Comment on lines +4054 to +4059
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are CLI integration tests for specify extension update already (see TestExtensionUpdateCLI), but this new --dev code path is untested. Adding tests for --dev update (installed and not-installed cases, config preservation, and disabled extension hook behavior) would help prevent regressions.

Copilot uses AI. Check for mistakes.

source_path = Path(extension).expanduser().resolve()
if not source_path.exists():
console.print(f"[red]Error:[/red] Directory not found: {source_path}")
raise typer.Exit(1)
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--dev accepts a local directory, but the code only checks exists(). If a user passes a file path, the error message becomes “No extension.yml found” rather than a clear “must be a directory”. Consider validating source_path.is_dir() and exiting with an explicit message when it isn’t a directory.

Suggested change
raise typer.Exit(1)
raise typer.Exit(1)
elif not source_path.is_dir():
console.print(f"[red]Error:[/red] --dev path must be a directory: {source_path}")
raise typer.Exit(1)

Copilot uses AI. Check for mistakes.

manifest_path = source_path / "extension.yml"
if not manifest_path.exists():
console.print(f"[red]Error:[/red] No extension.yml found in {source_path}")
raise typer.Exit(1)

# Read extension ID from source manifest
import yaml
with open(manifest_path) as f:
manifest_data = yaml.safe_load(f) or {}
Comment on lines +4072 to +4074
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading/parsing extension.yml here is outside the surrounding try/except and can raise (YAML syntax error, permission error, etc.), resulting in an uncaught exception/traceback. Consider using the existing ExtensionManifest loader/validator (from specify_cli.extensions) or wrapping the file read + yaml.safe_load in error handling and returning a clean Typer error message.

Suggested change
import yaml
with open(manifest_path) as f:
manifest_data = yaml.safe_load(f) or {}
try:
with open(manifest_path) as f:
manifest_data = yaml.safe_load(f) or {}
except (OSError, yaml.YAMLError) as e:
console.print(f"[red]Error:[/red] Failed to read or parse {manifest_path}: {e}")
raise typer.Exit(1)

Copilot uses AI. Check for mistakes.
extension_id = manifest_data.get("extension", {}).get("id")
if not extension_id:
console.print("[red]Error:[/red] extension.yml missing extension.id")
raise typer.Exit(1)

new_version = manifest_data.get("extension", {}).get("version", "unknown")

# Check if installed
installed = manager.list_installed()
installed_ids = {ext["id"] for ext in installed}

if extension_id not in installed_ids:
Comment on lines +4082 to +4086
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Installed detection uses manager.list_installed() to build installed_ids. list_installed() is based on registry.list() which filters out corrupted/non-dict entries, so an extension can still be “installed” per registry.is_installed() yet be missing from installed_ids—leading --dev to attempt a fresh install and fail with “already installed”. Consider checking installation via manager.registry.is_installed(extension_id) (or manager.registry.keys()) instead of relying on list_installed().

Suggested change
# Check if installed
installed = manager.list_installed()
installed_ids = {ext["id"] for ext in installed}
if extension_id not in installed_ids:
# Check if installed using registry to handle corrupted entries gracefully
try:
is_installed = manager.registry.is_installed(extension_id)
except AttributeError:
# Fallback for environments without registry.is_installed
installed = manager.list_installed()
installed_ids = {ext["id"] for ext in installed}
is_installed = extension_id in installed_ids
if not is_installed:

Copilot uses AI. Check for mistakes.
console.print(f"[yellow]Extension '{extension_id}' not installed — installing fresh[/yellow]")
try:
manifest = manager.install_from_directory(source_path, speckit_version)
console.print(f"\n[green]✓[/green] Installed {extension_id} v{manifest.version} from {source_path}")
except Exception as e:
console.print(f"[red]Error:[/red] {e}")
raise typer.Exit(1)
raise typer.Exit(0)

# Get current metadata to preserve
backup_registry_entry = manager.registry.get(extension_id)
current_version = backup_registry_entry.get("version", "unknown") if isinstance(backup_registry_entry, dict) else "unknown"

console.print(f"🔄 Updating {extension_id} from local directory...")
console.print(f" Source: {source_path}")
console.print(f" Version: {current_version} → {new_version}")

# Backup config files before removal
extension_dir = manager.extensions_dir / extension_id
backup_config_dir = manager.extensions_dir / ".backup" / f"{extension_id}-dev-update" / "config"
if extension_dir.exists():
config_files = list(extension_dir.glob("*-config.yml")) + list(
extension_dir.glob("*-config.local.yml")
) + list(extension_dir.glob("local-config.yml"))
if config_files:
backup_config_dir.mkdir(parents=True, exist_ok=True)
for cfg_file in config_files:
shutil.copy2(cfg_file, backup_config_dir / cfg_file.name)

try:
# Remove old version (keeps hooks backup internally)
manager.remove(extension_id, keep_config=True)

# Install from local directory
manifest = manager.install_from_directory(source_path, speckit_version)

# Restore config files
new_extension_dir = manager.extensions_dir / extension_id
if backup_config_dir.exists() and new_extension_dir.exists():
for cfg_file in backup_config_dir.iterdir():
if cfg_file.is_file():
shutil.copy2(cfg_file, new_extension_dir / cfg_file.name)

# Restore preserved metadata (installed_at, priority, enabled state)
if backup_registry_entry and isinstance(backup_registry_entry, dict):
current_metadata = manager.registry.get(extension_id)
if current_metadata and isinstance(current_metadata, dict):
new_metadata = dict(current_metadata)
if "installed_at" in backup_registry_entry:
new_metadata["installed_at"] = backup_registry_entry["installed_at"]
if "priority" in backup_registry_entry:
new_metadata["priority"] = normalize_priority(backup_registry_entry["priority"])
if not backup_registry_entry.get("enabled", True):
new_metadata["enabled"] = False
new_metadata["source"] = "local"
manager.registry.restore(extension_id, new_metadata)
Comment on lines +4130 to +4142
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the extension was disabled before the update, this code restores enabled=False in the registry, but it doesn’t re-disable hooks in .specify/extensions.yml. The catalog update path explicitly disables those hooks to keep behavior consistent. Consider mirroring that logic here so disabled extensions don’t end up executing hooks after a dev update.

Copilot uses AI. Check for mistakes.

console.print(f"\n[green]✓[/green] Updated {extension_id} to v{new_version} from {source_path}")
except Exception as e:
console.print(f"\n[red]✗[/red] Update failed: {e}")
raise typer.Exit(1)
finally:
# Clean up backup
backup_base = manager.extensions_dir / ".backup" / f"{extension_id}-dev-update"
if backup_base.exists():
shutil.rmtree(backup_base)

Comment on lines +4144 to +4153
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failure handling here can cause irreversible loss: after remove() succeeds, if install_from_directory() or config restore fails, the old extension is gone and the config backup is deleted in finally. Consider implementing rollback (similar to the existing catalog update path) or, at minimum, only deleting the backup directory after a successful update and leaving it in place on failure. Also consider guarding shutil.rmtree in the cleanup path so a cleanup error doesn’t mask the original update failure.

Copilot uses AI. Check for mistakes.
raise typer.Exit(0)

# ── Catalog mode: update from catalog (existing behavior) ─────────
catalog = ExtensionCatalog(project_root)

try:
# Get list of extensions to update
installed = manager.list_installed()
Expand Down
Loading