From 95fad30676b7578f2129808a779ed8a24e8e6d25 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 09:47:43 +0100 Subject: [PATCH 01/39] feat: add preset update command Implement staged atomic preset updates with bulk and dry-run support, preserving registry state and reconciling manifests and constitutions safely. Assisted-by: GitHub Copilot (model: GPT-5.6 Luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/reference/presets.md | 24 +++ src/specify_cli/presets/__init__.py | 276 +++++++++++++++++++++++++++ src/specify_cli/presets/_commands.py | 199 +++++++++++++++++++ tests/test_presets.py | 87 +++++++++ 4 files changed, 586 insertions(+) diff --git a/docs/reference/presets.md b/docs/reference/presets.md index 1098abfb42..a8d236ff0a 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -39,6 +39,30 @@ specify preset remove Removes an installed preset and cleans up its registered commands. +## Update a Preset + +```bash +specify preset update [] +``` + +Updates one installed preset, or all installed presets when no ID is given. +Catalogue updates are resolved by preset ID and only use catalogues that allow +installation. A preset installed from a local directory or an archive URL must +be updated with the same `--dev ` or `--from ` source. + +| Option | Description | +| ---------------- | ------------------------------------------------ | +| `--from ` | Update from a `.zip`, `.tar.gz`, or `.tgz` URL | +| `--dev ` | Update from a local directory | +| `--priority ` | Set a new resolution priority | +| `--dry-run` | Show the manifest diff without changing anything | + +Single-preset updates do not prompt for confirmation. Bulk updates ask once, +report each preset independently, preserve priority and enabled state unless +explicitly changed, and skip compatible presets that are already current. +Updates stage and validate the new preset before atomically replacing the +installed directory. + ## List Installed Presets ```bash diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index abc63299c2..ebb8d5c3a6 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -156,6 +156,9 @@ def _materialize_constitution_template( content = composed_content.encode("utf-8") result = "composed" + if memory_constitution.exists() and memory_constitution.read_bytes() == content: + return "unchanged" + _ensure_safe_shared_directory(project_root, memory_constitution.parent) _write_shared_bytes(project_root, memory_constitution, content) provenance = memory_constitution.parent / _CONSTITUTION_PROVENANCE_FILE @@ -680,6 +683,43 @@ def get_hash(self) -> str: return f"sha256:{h.hexdigest()}" +def diff_preset_manifests( + old_manifest: PresetManifest, new_manifest: PresetManifest +) -> Dict[str, List[Dict[str, Any]]]: + """Diff preset templates by their stable ``(name, type)`` identity.""" + old_entries = { + (item["name"], item["type"]): item for item in old_manifest.templates + } + new_entries = { + (item["name"], item["type"]): item for item in new_manifest.templates + } + added = [] + removed = [] + changed = [] + unchanged = [] + for identity in sorted(new_entries.keys() - old_entries.keys()): + added.append({"identity": identity, "new": new_entries[identity]}) + for identity in sorted(old_entries.keys() - new_entries.keys()): + removed.append({"identity": identity, "old": old_entries[identity]}) + for identity in sorted(old_entries.keys() & new_entries.keys()): + old_item = old_entries[identity] + new_item = new_entries[identity] + if old_item == new_item: + unchanged.append( + {"identity": identity, "old": old_item, "new": new_item} + ) + else: + changed.append( + {"identity": identity, "old": old_item, "new": new_item} + ) + return { + "added": added, + "removed": removed, + "changed": changed, + "unchanged": unchanged, + } + + class PresetRegistry: """Manages the registry of installed presets.""" @@ -4010,6 +4050,242 @@ def _reconcile_constitution(self, *, create_if_missing: bool = False) -> None: return _materialize_constitution_template(self.project_root, memory_constitution) + def _validate_update_source( + self, + source_dir: Path, + pack_id: str, + speckit_version: str, + ) -> tuple[PresetManifest, PresetManifest, Dict[str, List[Dict[str, Any]]]]: + """Validate an update without touching the installed preset.""" + current_dir = self.presets_dir / pack_id + try: + old_manifest = PresetManifest(current_dir / "preset.yml") + except PresetValidationError as exc: + raise PresetValidationError( + f"Installed preset '{pack_id}' has a corrupt manifest; " + "remove and add the preset again" + ) from exc + + try: + new_manifest = PresetManifest(source_dir / "preset.yml") + except PresetValidationError as exc: + raise PresetValidationError( + f"Incoming preset '{pack_id}' has an unparseable or corrupt " + "manifest; remove and add the preset again" + ) from exc + if new_manifest.id != pack_id: + raise PresetValidationError( + f"Preset ID mismatch: installed '{pack_id}', " + f"incoming '{new_manifest.id}'. Remove and add the preset again." + ) + self.check_compatibility(new_manifest, speckit_version) + for template in new_manifest.templates: + referenced = source_dir / template["file"] + if not referenced.is_file(): + raise PresetValidationError( + f"Preset '{pack_id}' is missing referenced file " + f"'{template['file']}'. Remove and add the preset again." + ) + return old_manifest, new_manifest, diff_preset_manifests( + old_manifest, new_manifest + ) + + def update_from_directory( + self, + source_dir: Path, + speckit_version: str, + *, + pack_id: Optional[str] = None, + priority: Optional[int] = None, + dry_run: bool = False, + ) -> tuple[PresetManifest, Dict[str, List[Dict[str, Any]]]]: + """Update an installed preset from a validated directory.""" + if priority is not None and priority < 1: + raise PresetValidationError( + "Priority must be a positive integer (1 or higher)" + ) + try: + incoming = PresetManifest(source_dir / "preset.yml") + except PresetValidationError as exc: + raise PresetValidationError( + "Incoming preset has an unparseable or corrupt manifest; " + "remove and add the preset again" + ) from exc + target_id = pack_id or incoming.id + if not self.registry.is_installed(target_id): + raise PresetError(f"Preset '{target_id}' is not installed") + old_manifest, new_manifest, diff = self._validate_update_source( + source_dir, target_id, speckit_version + ) + if dry_run: + return new_manifest, diff + + metadata = self.registry.get(target_id) + if metadata is None: + raise PresetError(f"Preset '{target_id}' has no valid registry entry") + preserved_priority = normalize_priority( + priority if priority is not None else metadata.get("priority", 10) + ) + enabled = metadata.get("enabled", True) + staging_dir = self.presets_dir / f"{target_id}.staging" + backup_dir = self.presets_dir / f"{target_id}.bak" + if staging_dir.exists() or staging_dir.is_symlink(): + shutil.rmtree(staging_dir) + if backup_dir.exists() or backup_dir.is_symlink(): + shutil.rmtree(backup_dir) + shutil.copytree(source_dir, staging_dir) + current_dir = self.presets_dir / target_id + registry_before = copy.deepcopy(metadata) + try: + os.replace(current_dir, backup_dir) + try: + os.replace(staging_dir, current_dir) + self.registry.update( + target_id, + { + "version": new_manifest.version, + "manifest_hash": new_manifest.get_hash(), + "priority": preserved_priority, + "enabled": enabled, + }, + ) + except Exception: + if current_dir.exists(): + shutil.rmtree(current_dir) + if backup_dir.exists(): + os.replace(backup_dir, current_dir) + self.registry.restore(target_id, registry_before) + raise + except Exception: + if staging_dir.exists(): + shutil.rmtree(staging_dir) + raise + + command_names = { + item["identity"][0] + for item in diff["added"] + diff["removed"] + diff["changed"] + if item["identity"][1] == "command" + } + command_names.update( + item["name"] + for item in old_manifest.templates + new_manifest.templates + if item.get("type") == "command" + ) + command_names.update( + alias + for item in old_manifest.templates + new_manifest.templates + if item.get("type") == "command" + for alias in item.get("aliases", []) + if isinstance(alias, str) + ) + try: + old_command_names = { + item["name"] + for item in old_manifest.templates + if item.get("type") == "command" + } + new_command_names = { + item["name"] + for item in new_manifest.templates + if item.get("type") == "command" + } + old_command_names.update( + alias + for item in old_manifest.templates + if item.get("type") == "command" + for alias in item.get("aliases", []) + if isinstance(alias, str) + ) + new_command_names.update( + alias + for item in new_manifest.templates + if item.get("type") == "command" + for alias in item.get("aliases", []) + if isinstance(alias, str) + ) + removed_command_names = old_command_names - new_command_names + registered_commands_before = metadata.get("registered_commands", {}) + if removed_command_names and isinstance( + registered_commands_before, dict + ): + stale_commands = { + agent: [ + name + for name in names + if name in removed_command_names + ] + for agent, names in registered_commands_before.items() + if isinstance(names, list) + } + stale_commands = { + agent: names + for agent, names in stale_commands.items() + if names + } + if stale_commands: + self._unregister_commands(stale_commands) + registered_commands = self._register_commands(new_manifest, current_dir) + self.registry.update( + target_id, {"registered_commands": registered_commands} + ) + registered_skills = self._register_skills(new_manifest, current_dir) + self.registry.update(target_id, {"registered_skills": registered_skills}) + if command_names: + self._reconcile_composed_commands(sorted(command_names)) + self._reconcile_skills(sorted(command_names)) + has_constitution_layer = any( + item.get("type") == "template" + and item.get("name") == "constitution-template" + for item in old_manifest.templates + new_manifest.templates + ) + if has_constitution_layer: + self.reconcile_constitution( + f"Failed to reconcile constitution after updating {target_id}", + create_if_missing=True, + ) + except Exception as exc: + import warnings + + warnings.warn( + f"Preset '{target_id}' was swapped, but post-update " + f"reconciliation failed: {exc}", + stacklevel=2, + ) + finally: + if backup_dir.exists(): + shutil.rmtree(backup_dir) + if staging_dir.exists(): + shutil.rmtree(staging_dir) + return new_manifest, diff + + def update_from_archive( + self, + archive_path: Path, + speckit_version: str, + *, + pack_id: str, + priority: Optional[int] = None, + dry_run: bool = False, + ) -> tuple[PresetManifest, Dict[str, List[Dict[str, Any]]]]: + """Update an installed preset from a supported archive.""" + with tempfile.TemporaryDirectory() as tmpdir: + extracted = Path(tmpdir) + safe_extract_archive( + archive_path, extracted, error_type=PresetValidationError + ) + roots = [extracted] + if not (extracted / "preset.yml").exists(): + roots = [item for item in extracted.iterdir() if item.is_dir()] + if len(roots) != 1 or not (roots[0] / "preset.yml").is_file(): + raise PresetValidationError("No preset.yml found in archive") + return self.update_from_directory( + roots[0], + speckit_version, + pack_id=pack_id, + priority=priority, + dry_run=dry_run, + ) + def install_from_archive( self, archive_path: Path, diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index ab74a8e029..7fab6aa9e4 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -10,6 +10,7 @@ import os import re +import tempfile from pathlib import Path import typer @@ -428,6 +429,204 @@ def preset_remove( raise typer.Exit(1) +@preset_app.command("update") +def preset_update( + preset_id: str = typer.Argument(None, help="Preset ID to update (or all)"), + from_url: str = typer.Option(None, "--from", help="Update from an archive URL"), + dev: str = typer.Option(None, "--dev", help="Update from a local directory"), + priority: int = typer.Option(None, "--priority", help="New priority"), + dry_run: bool = typer.Option(False, "--dry-run", help="Show changes without writing"), +): + """Update one preset, or all installed presets.""" + from .. import _require_specify_project, get_speckit_version + from . import ( + PresetCatalog, + PresetCompatibilityError, + PresetError, + PresetManager, + PresetValidationError, + ) + from packaging import version as pkg_version + + project_root = _require_specify_project() + manager = PresetManager(project_root) + if priority is not None and priority < 1: + console.print("[red]Error:[/red] Priority must be a positive integer (1 or higher)") + raise typer.Exit(1) + if from_url and dev: + console.print("[red]Error:[/red] Use only one of --from or --dev") + raise typer.Exit(1) + if (from_url or dev) and not preset_id: + console.print("[red]Error:[/red] --from and --dev require a preset ID") + raise typer.Exit(1) + + installed = manager.list_installed() + ids = [preset_id] if preset_id else [item["id"] for item in installed] + if not ids: + console.print("[yellow]No presets installed.[/yellow]") + return + if not preset_id: + if not typer.confirm("Update all installed presets?"): + console.print("Cancelled") + return + + catalog = PresetCatalog(project_root) + speckit_version = get_speckit_version() + outcomes = [] + + def download_explicit(url: str) -> Path: + import urllib.error + from urllib.parse import urlparse + from specify_cli.authentication.http import open_url + + try: + parsed = urlparse(url) + parsed.port + except ValueError as exc: + raise PresetError(f"Invalid URL: {url}") from exc + if not is_https_or_localhost_http(url): + raise PresetError("URL must use HTTPS (HTTP is only allowed for localhost)") + def validate_redirect(old_url, new_url): + if not is_safe_download_redirect(old_url, new_url): + raise PresetError( + "redirect target must use HTTPS or remain on localhost" + ) + + try: + with open_url( + url, timeout=60, redirect_validator=validate_redirect + ) as response: + final_url = response.geturl() if hasattr(response, "geturl") else url + if not is_https_or_localhost_http(final_url): + raise PresetError("redirect target uses a disallowed URL") + data = read_response_limited( + response, error_type=PresetError, label=f"preset {url}" + ) + content_type = ( + response.getheader("Content-Type") + if hasattr(response, "getheader") + else None + ) + except urllib.error.URLError as exc: + raise PresetError(f"Failed to download preset: {exc}") from exc + declared_format = archive_format_from_name(url) + suffix = archive_suffix(declared_format) if declared_format else ".archive" + fd, name = tempfile.mkstemp(prefix="speckit-preset-update-", suffix=suffix) + os.close(fd) + path = Path(name) + path.write_bytes(data) + detected = detect_archive_format( + path, + source_name=url, + content_type=content_type, + error_type=PresetError, + ) + if declared_format is None: + detected_path = path.with_suffix(archive_suffix(detected)) + os.replace(path, detected_path) + path = detected_path + return path + + for item_id in ids: + safe_id = _escape_markup(str(item_id)) + source_kind = "catalog" + archive_path = None + try: + metadata = manager.registry.get(item_id) + if metadata is None or "version" not in metadata: + raise PresetError("registry entry is missing or corrupt") + installed_version = pkg_version.Version(str(metadata["version"])) + source_path = None + pack_info = None + if dev: + source_kind = "dev" + source_path = Path(dev).resolve() + if not source_path.is_dir(): + raise PresetError(f"Directory not found: {dev}") + elif from_url: + source_kind = "url" + archive_path = download_explicit(from_url) + else: + pack_info = catalog.get_pack_info(item_id) + if not pack_info: + raise PresetError( + "source not re-resolvable — supply --from/--dev explicitly" + ) + if not pack_info.get("_install_allowed", True): + raise PresetError( + f"updates are not allowed from " + f"'{pack_info.get('_catalog_name', 'catalog')}'" + ) + catalog_version = pkg_version.Version(str(pack_info["version"])) + if catalog_version <= installed_version: + console.print( + f"[dim]• {safe_id}: already latest, skipped " + f"(v{installed_version})[/dim]" + ) + outcomes.append("skipped") + continue + archive_path = catalog.download_pack(item_id) + + if source_kind == "dev": + manifest, diff = manager.update_from_directory( + source_path, + speckit_version, + pack_id=item_id, + priority=priority, + dry_run=dry_run, + ) + else: + manifest, diff = manager.update_from_archive( + archive_path, + speckit_version, + pack_id=item_id, + priority=priority, + dry_run=dry_run, + ) + action = "would update" if dry_run else "updated" + added_commands = sum( + entry["identity"][1] == "command" for entry in diff["added"] + ) + removed_commands = sum( + entry["identity"][1] == "command" for entry in diff["removed"] + ) + constitution_unchanged = any( + entry["identity"] + == ("constitution-template", "template") + for entry in diff["unchanged"] + ) + console.print( + f"[green]✓[/green] {safe_id}: {action} to v{manifest.version} " + f"(+{added_commands} commands, -{removed_commands} commands, " + f"{'constitution unchanged' if constitution_unchanged else 'constitution reconciled'})" + ) + if dry_run: + for category in ("added", "removed", "changed", "unchanged"): + identities = [ + f"{name} ({template_type})" + for (name, template_type) in ( + entry["identity"] for entry in diff[category] + ) + ] + if identities: + console.print( + f" {category}: {', '.join(identities)}" + ) + console.print(" planned actions: stage, validate, atomically swap, reconcile") + outcomes.append("updated") + except (PresetCompatibilityError, PresetValidationError, PresetError) as exc: + detail = _escape_markup(str(exc).replace("\n", " ")) + prefix = "skipped" if isinstance(exc, PresetCompatibilityError) else "failed" + console.print(f"[yellow]•[/yellow] {safe_id}: {prefix} — {detail}") + outcomes.append(prefix) + finally: + if archive_path is not None and source_kind == "url": + archive_path.unlink(missing_ok=True) + + if any(outcome == "failed" for outcome in outcomes): + raise typer.Exit(1) + + @preset_app.command("search") def preset_search( query: str = typer.Argument(None, help="Search query"), diff --git a/tests/test_presets.py b/tests/test_presets.py index 57a70b4192..c6119e4dfd 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -14651,3 +14651,90 @@ def test_resolved_content_embeds_core_and_sync_pass(self, project_dir): assert "## Constitution Template Sync" in content assert "supersedes the \"Scope Guard\" above" in content assert "plan-template.md" in content + + +class TestPresetUpdate: + def _updated_source(self, pack_dir, version="2.0.0"): + source = pack_dir.parent / "updated-pack" + shutil.copytree(pack_dir, source) + manifest_path = source / "preset.yml" + data = yaml.safe_load(manifest_path.read_text()) + data["preset"]["version"] = version + data["provides"]["templates"].append( + { + "type": "command", + "name": "new-command", + "file": "commands/new-command.md", + } + ) + (source / "commands").mkdir() + (source / "commands" / "new-command.md").write_text("# New\n") + manifest_path.write_text(yaml.safe_dump(data)) + return source + + def test_manifest_diff_uses_name_and_type_identity(self, pack_dir): + source = self._updated_source(pack_dir) + old = PresetManifest(pack_dir / "preset.yml") + new = PresetManifest(source / "preset.yml") + from specify_cli.presets import diff_preset_manifests + + diff = diff_preset_manifests(old, new) + assert [item["identity"] for item in diff["added"]] == [ + ("new-command", "command") + ] + assert diff["changed"] == [] + assert diff["unchanged"][0]["identity"] == ("spec-template", "template") + + def test_update_preserves_priority_and_enabled_state( + self, project_dir, pack_dir + ): + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.0", priority=3) + manager.registry.update("test-pack", {"enabled": False}) + source = self._updated_source(pack_dir) + + manifest, diff = manager.update_from_directory( + source, "0.1.0", pack_id="test-pack" + ) + + assert manifest.version == "2.0.0" + assert diff["added"] + metadata = manager.registry.get("test-pack") + assert metadata["priority"] == 3 + assert metadata["enabled"] is False + assert (project_dir / ".specify/presets/test-pack/preset.yml").exists() + + def test_dry_run_does_not_modify_installation(self, project_dir, pack_dir): + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.0", priority=4) + before = (project_dir / ".specify/presets/test-pack/preset.yml").read_bytes() + source = self._updated_source(pack_dir) + + manifest, diff = manager.update_from_directory( + source, "0.1.0", pack_id="test-pack", dry_run=True + ) + + assert manifest.version == "2.0.0" + assert diff["added"] + assert ( + project_dir / ".specify/presets/test-pack/preset.yml" + ).read_bytes() == before + assert manager.registry.get("test-pack")["version"] == "1.0.0" + + def test_missing_referenced_file_leaves_installation_untouched( + self, project_dir, pack_dir + ): + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.0") + source = self._updated_source(pack_dir) + (source / "commands" / "new-command.md").unlink() + installed = ( + project_dir / ".specify/presets/test-pack/preset.yml" + ).read_bytes() + + with pytest.raises(PresetValidationError, match="[Rr]emove and add"): + manager.update_from_directory(source, "0.1.0", pack_id="test-pack") + + assert ( + project_dir / ".specify/presets/test-pack/preset.yml" + ).read_bytes() == installed From be01b85eaabb556e6521192a064f38cfbddc0b87 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 09:49:56 +0100 Subject: [PATCH 02/39] fix: tighten preset update failure handling Keep the installed preset untouched and clean up partial staging when update validation or staging fails, and make missing preset errors direct users to preset add. Assisted-by: GitHub Copilot (model: GPT-5.6 Luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/presets/__init__.py | 7 ++++++- src/specify_cli/presets/_commands.py | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index ebb8d5c3a6..740894d083 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -4133,7 +4133,12 @@ def update_from_directory( shutil.rmtree(staging_dir) if backup_dir.exists() or backup_dir.is_symlink(): shutil.rmtree(backup_dir) - shutil.copytree(source_dir, staging_dir) + try: + shutil.copytree(source_dir, staging_dir) + except Exception: + if staging_dir.exists(): + shutil.rmtree(staging_dir) + raise current_dir = self.presets_dir / target_id registry_before = copy.deepcopy(metadata) try: diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index 7fab6aa9e4..62e4c55401 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -533,6 +533,10 @@ def validate_redirect(old_url, new_url): archive_path = None try: metadata = manager.registry.get(item_id) + if not manager.registry.is_installed(item_id): + raise PresetError( + f"Preset '{item_id}' is not installed; use 'preset add' instead" + ) if metadata is None or "version" not in metadata: raise PresetError("registry entry is missing or corrupt") installed_version = pkg_version.Version(str(metadata["version"])) @@ -598,7 +602,8 @@ def validate_redirect(old_url, new_url): console.print( f"[green]✓[/green] {safe_id}: {action} to v{manifest.version} " f"(+{added_commands} commands, -{removed_commands} commands, " - f"{'constitution unchanged' if constitution_unchanged else 'constitution reconciled'})" + f"{'constitution unchanged' if constitution_unchanged else 'constitution reconciled'}, " + f"priority {'kept at ' + str(metadata.get('priority', 10)) if priority is None else 'set to ' + str(priority)})" ) if dry_run: for category in ("added", "removed", "changed", "unchanged"): From ed021f2b239e4ecec2b83938f0e23ee923e00dcf Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 09:50:51 +0100 Subject: [PATCH 03/39] fix: support explicit bulk preset updates Add the --all alias for bulk updates while preserving source-option validation and single-update semantics. Assisted-by: GitHub Copilot (model: GPT-5.6 Luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/presets/_commands.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index 62e4c55401..3236a2db20 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -436,6 +436,7 @@ def preset_update( dev: str = typer.Option(None, "--dev", help="Update from a local directory"), priority: int = typer.Option(None, "--priority", help="New priority"), dry_run: bool = typer.Option(False, "--dry-run", help="Show changes without writing"), + all_presets: bool = typer.Option(False, "--all", help="Update all installed presets"), ): """Update one preset, or all installed presets.""" from .. import _require_specify_project, get_speckit_version @@ -456,16 +457,20 @@ def preset_update( if from_url and dev: console.print("[red]Error:[/red] Use only one of --from or --dev") raise typer.Exit(1) - if (from_url or dev) and not preset_id: + if preset_id and all_presets: + console.print("[red]Error:[/red] Use either a preset ID or --all, not both") + raise typer.Exit(1) + if (from_url or dev) and (not preset_id or all_presets): console.print("[red]Error:[/red] --from and --dev require a preset ID") raise typer.Exit(1) installed = manager.list_installed() - ids = [preset_id] if preset_id else [item["id"] for item in installed] + bulk = all_presets or not preset_id + ids = [preset_id] if not bulk else [item["id"] for item in installed] if not ids: console.print("[yellow]No presets installed.[/yellow]") return - if not preset_id: + if bulk: if not typer.confirm("Update all installed presets?"): console.print("Cancelled") return From af489078fbb83116076fa9925be6c6d45b3f7975 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 10:03:38 +0100 Subject: [PATCH 04/39] fix: reconcile only changed preset commands Scope command and skill registration to added, removed, and changed manifest identities, preserve unaffected registry tracking, and report constitution status from the resolved content hash. Assisted-by: GitHub Copilot (model: GPT-5.6 Luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/presets/__init__.py | 80 ++++++++++++++++++++++------ src/specify_cli/presets/_commands.py | 21 ++++++-- 2 files changed, 80 insertions(+), 21 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 740894d083..b2a5a569d6 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -1163,7 +1163,9 @@ def find_unmet_extension_dependencies( def _register_commands( self, manifest: PresetManifest, - preset_dir: Path + preset_dir: Path, + *, + command_names: Optional[set[str]] = None, ) -> Dict[str, List[str]]: """Register preset command overrides with all detected AI agents. @@ -1183,7 +1185,13 @@ def _register_commands( Dictionary mapping agent names to lists of registered command names """ command_templates = [ - t for t in manifest.templates if t.get("type") == "command" + t for t in manifest.templates + if t.get("type") == "command" + and ( + command_names is None + or t["name"] in command_names + or any(alias in command_names for alias in t.get("aliases", [])) + ) ] if not command_templates: return {} @@ -2951,6 +2959,7 @@ def _register_skills( *, target_dir: Optional[Path] = None, target_agent: Optional[str] = None, + command_names: Optional[set[str]] = None, ) -> Dict[str, List[str]]: """Generate SKILL.md files for preset command overrides. @@ -2984,7 +2993,13 @@ def _register_skills( two can be tracked/restored consistently (#2948). """ command_templates = [ - t for t in manifest.templates if t.get("type") == "command" + t for t in manifest.templates + if t.get("type") == "command" + and ( + command_names is None + or t["name"] in command_names + or any(alias in command_names for alias in t.get("aliases", [])) + ) ] if not command_templates: return {} @@ -4171,18 +4186,14 @@ def update_from_directory( for item in diff["added"] + diff["removed"] + diff["changed"] if item["identity"][1] == "command" } - command_names.update( - item["name"] - for item in old_manifest.templates + new_manifest.templates - if item.get("type") == "command" - ) - command_names.update( - alias - for item in old_manifest.templates + new_manifest.templates - if item.get("type") == "command" - for alias in item.get("aliases", []) - if isinstance(alias, str) - ) + for item in diff["added"] + diff["removed"] + diff["changed"]: + template = item.get("new") or item.get("old") + if template and template.get("type") == "command": + command_names.update( + alias + for alias in template.get("aliases", []) + if isinstance(alias, str) + ) try: old_command_names = { item["name"] @@ -4229,11 +4240,46 @@ def update_from_directory( } if stale_commands: self._unregister_commands(stale_commands) - registered_commands = self._register_commands(new_manifest, current_dir) + registered_commands = self._register_commands( + new_manifest, current_dir, command_names=command_names + ) + merged_commands: Dict[str, List[str]] = {} + for agent, names in registered_commands_before.items(): + if isinstance(names, list): + retained = [name for name in names if name not in command_names] + if retained: + merged_commands[agent] = retained + for agent, names in registered_commands.items(): + merged_commands.setdefault(agent, []).extend( + name for name in names if name not in merged_commands.get(agent, []) + ) + registered_commands = merged_commands self.registry.update( target_id, {"registered_commands": registered_commands} ) - registered_skills = self._register_skills(new_manifest, current_dir) + affected_skill_names = set() + for name in command_names: + modern, legacy = self._skill_names_for_command(name) + affected_skill_names.update((modern, legacy)) + registered_skills_before = metadata.get("registered_skills", {}) + registered_skills = self._register_skills( + new_manifest, current_dir, command_names=command_names + ) + if isinstance(registered_skills_before, dict): + merged_skills: Dict[str, List[str]] = {} + for agent, names in registered_skills_before.items(): + if isinstance(names, list): + retained = [ + name for name in names if name not in affected_skill_names + ] + if retained: + merged_skills[agent] = retained + for agent, names in registered_skills.items(): + merged_skills.setdefault(agent, []).extend( + name for name in names + if name not in merged_skills.get(agent, []) + ) + registered_skills = merged_skills self.registry.update(target_id, {"registered_skills": registered_skills}) if command_names: self._reconcile_composed_commands(sorted(command_names)) diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index 3236a2db20..ce1488c77b 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -576,6 +576,12 @@ def validate_redirect(old_url, new_url): continue archive_path = catalog.download_pack(item_id) + constitution_path = ( + project_root / ".specify" / "memory" / "constitution.md" + ) + constitution_before = ( + constitution_path.read_bytes() if constitution_path.exists() else None + ) if source_kind == "dev": manifest, diff = manager.update_from_directory( source_path, @@ -599,10 +605,17 @@ def validate_redirect(old_url, new_url): removed_commands = sum( entry["identity"][1] == "command" for entry in diff["removed"] ) - constitution_unchanged = any( - entry["identity"] - == ("constitution-template", "template") - for entry in diff["unchanged"] + constitution_after = ( + constitution_path.read_bytes() if constitution_path.exists() else None + ) + constitution_unchanged = ( + any( + entry["identity"] + == ("constitution-template", "template") + for entry in diff["unchanged"] + ) + if dry_run + else constitution_before == constitution_after ) console.print( f"[green]✓[/green] {safe_id}: {action} to v{manifest.version} " From b06ca6e4b2428f8eb0dcba2c85b3f7de97e017b1 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 10:13:07 +0100 Subject: [PATCH 05/39] test: cover preset content reconciliation Detect referenced template content changes in update diffs and isolate malformed catalog versions and archive cleanup. Add regression coverage for content-only changes. Assisted-by: GitHub Copilot (model: GPT-5.6 Luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/presets/__init__.py | 47 ++++++++++++++++++++++++++-- src/specify_cli/presets/_commands.py | 16 ++++++++-- tests/test_presets.py | 17 ++++++++++ 3 files changed, 75 insertions(+), 5 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index b2a5a569d6..a0169361a9 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -720,6 +720,48 @@ def diff_preset_manifests( } +def _diff_preset_template_files( + diff: Dict[str, List[Dict[str, Any]]], + old_manifest: PresetManifest, + new_manifest: PresetManifest, + old_dir: Path, + new_dir: Path, +) -> Dict[str, List[Dict[str, Any]]]: + """Include referenced-file content changes in the manifest diff.""" + old_by_identity = { + (item["name"], item["type"]): item for item in old_manifest.templates + } + new_by_identity = { + (item["name"], item["type"]): item for item in new_manifest.templates + } + unchanged = [] + changed = list(diff["changed"]) + for entry in diff["unchanged"]: + identity = entry["identity"] + old_path = old_dir / old_by_identity[identity]["file"] + new_path = new_dir / new_by_identity[identity]["file"] + if ( + not old_path.is_file() + or not new_path.is_file() + or old_path.read_bytes() != new_path.read_bytes() + ): + changed.append( + { + "identity": identity, + "old": old_by_identity[identity], + "new": new_by_identity[identity], + } + ) + else: + unchanged.append(entry) + return { + "added": diff["added"], + "removed": diff["removed"], + "changed": changed, + "unchanged": unchanged, + } + + class PresetRegistry: """Manages the registry of installed presets.""" @@ -4101,8 +4143,9 @@ def _validate_update_source( f"Preset '{pack_id}' is missing referenced file " f"'{template['file']}'. Remove and add the preset again." ) - return old_manifest, new_manifest, diff_preset_manifests( - old_manifest, new_manifest + diff = diff_preset_manifests(old_manifest, new_manifest) + return old_manifest, new_manifest, _diff_preset_template_files( + diff, old_manifest, new_manifest, current_dir, source_dir ) def update_from_directory( diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index ce1488c77b..b45d671670 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -544,7 +544,12 @@ def validate_redirect(old_url, new_url): ) if metadata is None or "version" not in metadata: raise PresetError("registry entry is missing or corrupt") - installed_version = pkg_version.Version(str(metadata["version"])) + try: + installed_version = pkg_version.Version(str(metadata["version"])) + except (KeyError, TypeError, ValueError) as exc: + raise PresetError( + f"invalid installed version for preset '{item_id}'" + ) from exc source_path = None pack_info = None if dev: @@ -566,7 +571,12 @@ def validate_redirect(old_url, new_url): f"updates are not allowed from " f"'{pack_info.get('_catalog_name', 'catalog')}'" ) - catalog_version = pkg_version.Version(str(pack_info["version"])) + try: + catalog_version = pkg_version.Version(str(pack_info["version"])) + except (KeyError, TypeError, ValueError) as exc: + raise PresetError( + f"catalog entry for preset '{item_id}' has an invalid version" + ) from exc if catalog_version <= installed_version: console.print( f"[dim]• {safe_id}: already latest, skipped " @@ -643,7 +653,7 @@ def validate_redirect(old_url, new_url): console.print(f"[yellow]•[/yellow] {safe_id}: {prefix} — {detail}") outcomes.append(prefix) finally: - if archive_path is not None and source_kind == "url": + if archive_path is not None: archive_path.unlink(missing_ok=True) if any(outcome == "failed" for outcome in outcomes): diff --git a/tests/test_presets.py b/tests/test_presets.py index c6119e4dfd..4a703c2fde 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -14738,3 +14738,20 @@ def test_missing_referenced_file_leaves_installation_untouched( assert ( project_dir / ".specify/presets/test-pack/preset.yml" ).read_bytes() == installed + + def test_template_file_content_change_is_reconciled( + self, project_dir, pack_dir + ): + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.0") + source = pack_dir.parent / "content-updated-pack" + shutil.copytree(pack_dir, source) + source_file = source / "templates" / "spec-template.md" + source_file.write_text(source_file.read_text() + "\nUpdated.\n") + + _, _, diff = manager._validate_update_source( + source, "test-pack", "0.1.0" + ) + + assert diff["changed"][0]["identity"] == ("spec-template", "template") + assert diff["unchanged"] == [] From 078b790f38fd1bf02b218d0b5a72a0cee516e67d Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 10:22:47 +0100 Subject: [PATCH 06/39] test: cover preset update acceptance paths Add regression coverage for manifest rejection, type changes, rollback, missing IDs, and single-update dry runs. Assisted-by: GitHub Copilot (model: GPT-5.6 Luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/test_presets.py | 112 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/tests/test_presets.py b/tests/test_presets.py index 4a703c2fde..ebd9b4f1ef 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -14755,3 +14755,115 @@ def test_template_file_content_change_is_reconciled( assert diff["changed"][0]["identity"] == ("spec-template", "template") assert diff["unchanged"] == [] + + def test_id_mismatch_is_rejected_before_swap(self, project_dir, pack_dir): + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.0") + source = self._updated_source(pack_dir) + data = yaml.safe_load((source / "preset.yml").read_text()) + data["preset"]["id"] = "renamed-pack" + (source / "preset.yml").write_text(yaml.safe_dump(data)) + + with pytest.raises(PresetValidationError, match="Remove and add"): + manager.update_from_directory(source, "0.1.0", pack_id="test-pack") + + assert manager.registry.get("test-pack")["version"] == "1.0.0" + + def test_corrupt_manifest_is_rejected_before_swap(self, project_dir, pack_dir): + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.0") + source = self._updated_source(pack_dir) + (source / "preset.yml").write_text("not: [valid") + + with pytest.raises(PresetValidationError, match="corrupt"): + manager.update_from_directory(source, "0.1.0", pack_id="test-pack") + + assert manager.registry.get("test-pack")["version"] == "1.0.0" + + def test_type_change_remains_reconcilable(self, project_dir, pack_dir): + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.0") + source = self._updated_source(pack_dir) + data = yaml.safe_load((source / "preset.yml").read_text()) + data["provides"]["templates"][0]["type"] = "script" + data["provides"]["templates"][0]["name"] = "spec-template-script" + (source / "preset.yml").write_text(yaml.safe_dump(data)) + (source / "templates" / "spec-template.md").write_text("# Script\n") + + _, diff = manager.update_from_directory( + source, "0.1.0", pack_id="test-pack" + ) + + assert ("spec-template", "template") in [ + entry["identity"] for entry in diff["removed"] + ] + assert ("spec-template-script", "script") in [ + entry["identity"] for entry in diff["added"] + ] + + def test_swap_failure_restores_directory_and_registry( + self, project_dir, pack_dir, monkeypatch + ): + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.0") + source = self._updated_source(pack_dir) + installed_manifest = ( + project_dir / ".specify" / "presets" / "test-pack" / "preset.yml" + ).read_bytes() + original_update = manager.registry.update + + def fail_update(pack_id, updates): + if updates.get("version") == "2.0.0": + raise OSError("simulated registry failure") + return original_update(pack_id, updates) + + monkeypatch.setattr(manager.registry, "update", fail_update) + with pytest.raises(OSError, match="simulated registry failure"): + manager.update_from_directory(source, "0.1.0", pack_id="test-pack") + + assert ( + project_dir / ".specify" / "presets" / "test-pack" / "preset.yml" + ).read_bytes() == installed_manifest + assert manager.registry.get("test-pack")["version"] == "1.0.0" + + def test_cli_missing_id_directs_user_to_add(self, project_dir): + from typer.testing import CliRunner + from specify_cli import app + + result = CliRunner().invoke( + app, + ["preset", "update", "missing-pack"], + obj={"project_root": project_dir}, + ) + + assert result.exit_code == 1 + assert "preset" in result.output and "add" in result.output + + def test_cli_single_dry_run_does_not_prompt( + self, project_dir, pack_dir, monkeypatch + ): + from typer.testing import CliRunner + from specify_cli import app + + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.0") + source = self._updated_source(pack_dir) + monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir) + monkeypatch.setattr("specify_cli.get_speckit_version", lambda: "0.1.0") + + result = CliRunner().invoke( + app, + [ + "preset", + "update", + "test-pack", + "--dev", + str(source), + "--dry-run", + ], + ) + + assert result.exit_code == 0, result.output + assert "would update" in result.output + assert "planned actions" in result.output + assert "Update all installed presets?" not in result.output From 8b07f099d4366f7508d1a95c7cbc394fb3b6baf1 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 10:23:56 +0100 Subject: [PATCH 07/39] fix: validate staged preset updates Revalidate the copied staging directory before the atomic swap and cover rollback when staged validation fails. Assisted-by: GitHub Copilot (model: GPT-5.6 Luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/presets/__init__.py | 3 +++ tests/test_presets.py | 30 +++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index a0169361a9..09b85dd29b 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -4193,6 +4193,9 @@ def update_from_directory( shutil.rmtree(backup_dir) try: shutil.copytree(source_dir, staging_dir) + self._validate_update_source( + staging_dir, target_id, speckit_version + ) except Exception: if staging_dir.exists(): shutil.rmtree(staging_dir) diff --git a/tests/test_presets.py b/tests/test_presets.py index ebd9b4f1ef..bd297c76f3 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -14826,6 +14826,36 @@ def fail_update(pack_id, updates): ).read_bytes() == installed_manifest assert manager.registry.get("test-pack")["version"] == "1.0.0" + def test_staged_validation_failure_leaves_live_install_untouched( + self, project_dir, pack_dir, monkeypatch + ): + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.0") + source = self._updated_source(pack_dir) + installed_manifest = ( + project_dir / ".specify" / "presets" / "test-pack" / "preset.yml" + ).read_bytes() + original_validate = manager._validate_update_source + calls = 0 + + def fail_staged(source_dir, pack_id, speckit_version): + nonlocal calls + calls += 1 + if calls == 2: + raise PresetValidationError("staged validation failed") + return original_validate(source_dir, pack_id, speckit_version) + + monkeypatch.setattr(manager, "_validate_update_source", fail_staged) + with pytest.raises(PresetValidationError, match="staged validation"): + manager.update_from_directory(source, "0.1.0", pack_id="test-pack") + + assert ( + project_dir / ".specify" / "presets" / "test-pack" / "preset.yml" + ).read_bytes() == installed_manifest + assert not ( + project_dir / ".specify" / "presets" / "test-pack.staging" + ).exists() + def test_cli_missing_id_directs_user_to_add(self, project_dir): from typer.testing import CliRunner from specify_cli import app From eb15a18ec857630841769b0bcae8beba5afe6506 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 10:38:28 +0100 Subject: [PATCH 08/39] fix: preserve preset registrations across agents Reconcile historical command and skill targets during updates and reuse release-asset download handling for explicit sources. Assisted-by: GitHub Copilot (model: GPT-5.6 Luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/presets/__init__.py | 43 ++++++++++++++++++++++++---- src/specify_cli/presets/_commands.py | 20 +++++++++++-- 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 09b85dd29b..47da6fcc16 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -4289,10 +4289,15 @@ def update_from_directory( registered_commands = self._register_commands( new_manifest, current_dir, command_names=command_names ) + active_agent = resolve_active_agent_for_registration(self.project_root) merged_commands: Dict[str, List[str]] = {} for agent, names in registered_commands_before.items(): if isinstance(names, list): - retained = [name for name in names if name not in command_names] + retained = ( + [name for name in names if name not in command_names] + if agent == active_agent + else list(names) + ) if retained: merged_commands[agent] = retained for agent, names in registered_commands.items(): @@ -4315,9 +4320,15 @@ def update_from_directory( merged_skills: Dict[str, List[str]] = {} for agent, names in registered_skills_before.items(): if isinstance(names, list): - retained = [ - name for name in names if name not in affected_skill_names - ] + retained = ( + [ + name + for name in names + if name not in affected_skill_names + ] + if agent == active_agent + else list(names) + ) if retained: merged_skills[agent] = retained for agent, names in registered_skills.items(): @@ -4328,8 +4339,28 @@ def update_from_directory( registered_skills = merged_skills self.registry.update(target_id, {"registered_skills": registered_skills}) if command_names: - self._reconcile_composed_commands(sorted(command_names)) - self._reconcile_skills(sorted(command_names)) + historical_agents = { + agent + for agent in registered_commands_before + if agent != active_agent + } + self._reconcile_composed_commands( + sorted(command_names), extra_agents=historical_agents + ) + extra_skill_dirs = {} + if isinstance(registered_skills_before, dict): + for agent in registered_skills_before: + if agent == active_agent: + continue + skill_dir = self._resolve_agent_skills_dir(agent) + extra_skill_dirs[skill_dir] = ( + agent, + sorted(affected_skill_names), + ) + self._reconcile_skills( + sorted(command_names), + extra_skills_dirs=extra_skill_dirs or None, + ) has_constitution_layer = any( item.get("type") == "template" and item.get("name") == "constitution-template" diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index b45d671670..a390f31509 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -483,6 +483,8 @@ def download_explicit(url: str) -> Path: import urllib.error from urllib.parse import urlparse from specify_cli.authentication.http import open_url + from specify_cli.authentication.http import github_provider_hosts + from specify_cli._github_http import resolve_github_release_asset_api_url try: parsed = urlparse(url) @@ -498,10 +500,24 @@ def validate_redirect(old_url, new_url): ) try: + resolved_url = resolve_github_release_asset_api_url( + url, open_url, github_hosts=github_provider_hosts() + ) + download_url = resolved_url or url + extra_headers = ( + {"Accept": "application/octet-stream"} if resolved_url else None + ) with open_url( - url, timeout=60, redirect_validator=validate_redirect + download_url, + timeout=60, + extra_headers=extra_headers, + redirect_validator=validate_redirect, ) as response: - final_url = response.geturl() if hasattr(response, "geturl") else url + final_url = ( + response.geturl() + if hasattr(response, "geturl") + else download_url + ) if not is_https_or_localhost_http(final_url): raise PresetError("redirect target uses a disallowed URL") data = read_response_limited( From f560fc12b0826bfbc91be1573d280788d5e1071e Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 10:51:10 +0100 Subject: [PATCH 09/39] fix: align preset update status wording Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/_commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index a390f31509..ac1626be97 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -595,7 +595,7 @@ def validate_redirect(old_url, new_url): ) from exc if catalog_version <= installed_version: console.print( - f"[dim]• {safe_id}: already latest, skipped " + f"[dim]• {safe_id}: Up to date, skipped " f"(v{installed_version})[/dim]" ) outcomes.append("skipped") From 921ab54a79f4c382ae40ab6d2a5ade3e33d73c05 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 10:53:18 +0100 Subject: [PATCH 10/39] feat: preflight preset bulk updates Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/_commands.py | 60 +++++++++++++++++++++++++--- 1 file changed, 55 insertions(+), 5 deletions(-) diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index ac1626be97..cbf5b8654d 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -470,14 +470,64 @@ def preset_update( if not ids: console.print("[yellow]No presets installed.[/yellow]") return - if bulk: - if not typer.confirm("Update all installed presets?"): - console.print("Cancelled") - return catalog = PresetCatalog(project_root) speckit_version = get_speckit_version() outcomes = [] + catalog_candidates = {} + + if bulk: + actionable_ids = [] + for item_id in ids: + safe_id = _escape_markup(str(item_id)) + metadata = manager.registry.get(item_id) + try: + if not manager.registry.is_installed(item_id): + raise PresetError( + f"Preset '{item_id}' is not installed; use 'preset add' instead" + ) + if metadata is None or "version" not in metadata: + raise PresetError("registry entry is missing or corrupt") + installed_version = pkg_version.Version(str(metadata["version"])) + pack_info = catalog.get_pack_info(item_id) + if not pack_info: + raise PresetError( + "source not re-resolvable — supply --from/--dev explicitly" + ) + if not pack_info.get("_install_allowed", True): + raise PresetError( + f"updates are not allowed from " + f"'{pack_info.get('_catalog_name', 'catalog')}'" + ) + catalog_version = pkg_version.Version(str(pack_info["version"])) + if catalog_version <= installed_version: + console.print( + f"[dim]• {safe_id}: Up to date, skipped " + f"(v{installed_version})[/dim]" + ) + outcomes.append("skipped") + continue + catalog_candidates[item_id] = pack_info + actionable_ids.append(item_id) + except (KeyError, TypeError, ValueError) as exc: + detail = _escape_markup( + f"invalid version metadata for preset '{item_id}'" + ) + console.print(f"[yellow]•[/yellow] {safe_id}: failed — {detail}") + outcomes.append("failed") + except (PresetCompatibilityError, PresetValidationError, PresetError) as exc: + detail = _escape_markup(str(exc).replace("\n", " ")) + console.print(f"[yellow]•[/yellow] {safe_id}: failed — {detail}") + outcomes.append("failed") + + ids = actionable_ids + if not ids: + if any(outcome == "failed" for outcome in outcomes): + raise typer.Exit(1) + return + if not typer.confirm("Update all installed presets?"): + console.print("Cancelled") + return def download_explicit(url: str) -> Path: import urllib.error @@ -577,7 +627,7 @@ def validate_redirect(old_url, new_url): source_kind = "url" archive_path = download_explicit(from_url) else: - pack_info = catalog.get_pack_info(item_id) + pack_info = catalog_candidates.get(item_id) or catalog.get_pack_info(item_id) if not pack_info: raise PresetError( "source not re-resolvable — supply --from/--dev explicitly" From cda4ed0980a9ed8263f9cdb7244c84556c020c1c Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 10:55:20 +0100 Subject: [PATCH 11/39] fix: preflight preset compatibility Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/_commands.py | 136 ++++++++++++++++----------- 1 file changed, 81 insertions(+), 55 deletions(-) diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index cbf5b8654d..6ca72139d5 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -476,59 +476,6 @@ def preset_update( outcomes = [] catalog_candidates = {} - if bulk: - actionable_ids = [] - for item_id in ids: - safe_id = _escape_markup(str(item_id)) - metadata = manager.registry.get(item_id) - try: - if not manager.registry.is_installed(item_id): - raise PresetError( - f"Preset '{item_id}' is not installed; use 'preset add' instead" - ) - if metadata is None or "version" not in metadata: - raise PresetError("registry entry is missing or corrupt") - installed_version = pkg_version.Version(str(metadata["version"])) - pack_info = catalog.get_pack_info(item_id) - if not pack_info: - raise PresetError( - "source not re-resolvable — supply --from/--dev explicitly" - ) - if not pack_info.get("_install_allowed", True): - raise PresetError( - f"updates are not allowed from " - f"'{pack_info.get('_catalog_name', 'catalog')}'" - ) - catalog_version = pkg_version.Version(str(pack_info["version"])) - if catalog_version <= installed_version: - console.print( - f"[dim]• {safe_id}: Up to date, skipped " - f"(v{installed_version})[/dim]" - ) - outcomes.append("skipped") - continue - catalog_candidates[item_id] = pack_info - actionable_ids.append(item_id) - except (KeyError, TypeError, ValueError) as exc: - detail = _escape_markup( - f"invalid version metadata for preset '{item_id}'" - ) - console.print(f"[yellow]•[/yellow] {safe_id}: failed — {detail}") - outcomes.append("failed") - except (PresetCompatibilityError, PresetValidationError, PresetError) as exc: - detail = _escape_markup(str(exc).replace("\n", " ")) - console.print(f"[yellow]•[/yellow] {safe_id}: failed — {detail}") - outcomes.append("failed") - - ids = actionable_ids - if not ids: - if any(outcome == "failed" for outcome in outcomes): - raise typer.Exit(1) - return - if not typer.confirm("Update all installed presets?"): - console.print("Cancelled") - return - def download_explicit(url: str) -> Path: import urllib.error from urllib.parse import urlparse @@ -598,10 +545,88 @@ def validate_redirect(old_url, new_url): path = detected_path return path + if bulk: + actionable_ids = [] + catalog_archives = {} + for item_id in ids: + safe_id = _escape_markup(str(item_id)) + metadata = manager.registry.get(item_id) + archive_path = None + try: + if not manager.registry.is_installed(item_id): + raise PresetError( + f"Preset '{item_id}' is not installed; use 'preset add' instead" + ) + if metadata is None or "version" not in metadata: + raise PresetError("registry entry is missing or corrupt") + installed_version = pkg_version.Version(str(metadata["version"])) + pack_info = catalog.get_pack_info(item_id) + if not pack_info: + raise PresetError( + "source not re-resolvable — supply --from/--dev explicitly" + ) + if not pack_info.get("_install_allowed", True): + raise PresetError( + f"updates are not allowed from " + f"'{pack_info.get('_catalog_name', 'catalog')}'" + ) + catalog_version = pkg_version.Version(str(pack_info["version"])) + if catalog_version <= installed_version: + console.print( + f"[dim]• {safe_id}: Up to date, skipped " + f"(v{installed_version})[/dim]" + ) + outcomes.append("skipped") + continue + archive_path = catalog.download_pack(item_id) + manager.update_from_archive( + archive_path, + speckit_version, + pack_id=item_id, + priority=priority, + dry_run=True, + ) + catalog_candidates[item_id] = pack_info + catalog_archives[item_id] = archive_path + actionable_ids.append(item_id) + except PresetCompatibilityError as exc: + detail = _escape_markup(str(exc).replace("\n", " ")) + console.print( + f"[yellow]•[/yellow] {safe_id}: skipped — {detail}" + ) + outcomes.append("skipped") + if archive_path is not None: + archive_path.unlink(missing_ok=True) + except (KeyError, TypeError, ValueError) as exc: + detail = _escape_markup( + f"invalid version metadata for preset '{item_id}'" + ) + console.print(f"[yellow]•[/yellow] {safe_id}: failed — {detail}") + outcomes.append("failed") + if archive_path is not None: + archive_path.unlink(missing_ok=True) + except (PresetValidationError, PresetError) as exc: + detail = _escape_markup(str(exc).replace("\n", " ")) + console.print(f"[yellow]•[/yellow] {safe_id}: failed — {detail}") + outcomes.append("failed") + if archive_path is not None: + archive_path.unlink(missing_ok=True) + + ids = actionable_ids + if not ids: + if any(outcome == "failed" for outcome in outcomes): + raise typer.Exit(1) + return + if not typer.confirm("Update all installed presets?"): + for archive_path in catalog_archives.values(): + archive_path.unlink(missing_ok=True) + console.print("Cancelled") + return + for item_id in ids: safe_id = _escape_markup(str(item_id)) source_kind = "catalog" - archive_path = None + archive_path = catalog_archives.get(item_id) if bulk else None try: metadata = manager.registry.get(item_id) if not manager.registry.is_installed(item_id): @@ -650,7 +675,8 @@ def validate_redirect(old_url, new_url): ) outcomes.append("skipped") continue - archive_path = catalog.download_pack(item_id) + if archive_path is None: + archive_path = catalog.download_pack(item_id) constitution_path = ( project_root / ".specify" / "memory" / "constitution.md" From d8e87360e52116519e4bec5d36a36e8f53bfe812 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 10:59:42 +0100 Subject: [PATCH 12/39] fix: preserve priority during bulk preset updates Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- docs/reference/presets.md | 11 +++++++---- src/specify_cli/presets/_commands.py | 16 +++++++++++----- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/docs/reference/presets.md b/docs/reference/presets.md index a8d236ff0a..4bba88960b 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -54,12 +54,15 @@ be updated with the same `--dev ` or `--from ` source. | ---------------- | ------------------------------------------------ | | `--from ` | Update from a `.zip`, `.tar.gz`, or `.tgz` URL | | `--dev ` | Update from a local directory | -| `--priority ` | Set a new resolution priority | +| `--priority ` | Set a new resolution priority for a single update | | `--dry-run` | Show the manifest diff without changing anything | -Single-preset updates do not prompt for confirmation. Bulk updates ask once, -report each preset independently, preserve priority and enabled state unless -explicitly changed, and skip compatible presets that are already current. +Single-preset updates do not prompt for confirmation and may use `--priority` +to reprioritize the preset. Bulk updates ask once, report each preset +independently, preserve priority and enabled state, and skip compatible +presets that are already current. If `--priority` is supplied to a bulk +update, it is ignored and a note is printed after pre-flight checks to prevent +unintended priority collisions. Updates stage and validate the new preset before atomically replacing the installed directory. diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index 6ca72139d5..bbceae1f20 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -451,7 +451,7 @@ def preset_update( project_root = _require_specify_project() manager = PresetManager(project_root) - if priority is not None and priority < 1: + if priority is not None and not (all_presets or not preset_id) and priority < 1: console.print("[red]Error:[/red] Priority must be a positive integer (1 or higher)") raise typer.Exit(1) if from_url and dev: @@ -466,6 +466,7 @@ def preset_update( installed = manager.list_installed() bulk = all_presets or not preset_id + effective_priority = None if bulk else priority ids = [preset_id] if not bulk else [item["id"] for item in installed] if not ids: console.print("[yellow]No presets installed.[/yellow]") @@ -583,7 +584,7 @@ def validate_redirect(old_url, new_url): archive_path, speckit_version, pack_id=item_id, - priority=priority, + priority=effective_priority, dry_run=True, ) catalog_candidates[item_id] = pack_info @@ -617,6 +618,11 @@ def validate_redirect(old_url, new_url): if any(outcome == "failed" for outcome in outcomes): raise typer.Exit(1) return + if priority is not None: + console.print( + "[yellow]Note:[/yellow] --priority is ignored for bulk updates; " + "existing preset priorities will be preserved." + ) if not typer.confirm("Update all installed presets?"): for archive_path in catalog_archives.values(): archive_path.unlink(missing_ok=True) @@ -689,7 +695,7 @@ def validate_redirect(old_url, new_url): source_path, speckit_version, pack_id=item_id, - priority=priority, + priority=effective_priority, dry_run=dry_run, ) else: @@ -697,7 +703,7 @@ def validate_redirect(old_url, new_url): archive_path, speckit_version, pack_id=item_id, - priority=priority, + priority=effective_priority, dry_run=dry_run, ) action = "would update" if dry_run else "updated" @@ -723,7 +729,7 @@ def validate_redirect(old_url, new_url): f"[green]✓[/green] {safe_id}: {action} to v{manifest.version} " f"(+{added_commands} commands, -{removed_commands} commands, " f"{'constitution unchanged' if constitution_unchanged else 'constitution reconciled'}, " - f"priority {'kept at ' + str(metadata.get('priority', 10)) if priority is None else 'set to ' + str(priority)})" + f"priority {'kept at ' + str(metadata.get('priority', 10)) if effective_priority is None else 'set to ' + str(effective_priority)})" ) if dry_run: for category in ("added", "removed", "changed", "unchanged"): From b599ddc2b4c5eceeb20bd8ccb6f04d121c0f9d98 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 11:02:23 +0100 Subject: [PATCH 13/39] refactor: split preset archive download helpers Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/_commands.py | 158 +++++++++++++++------------ 1 file changed, 88 insertions(+), 70 deletions(-) diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index bbceae1f20..80e051fbb4 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -41,6 +41,93 @@ preset_app.add_typer(preset_catalog_app, name="catalog") +def _fetch_preset_archive_data( + url: str, + error_type: type[Exception], +) -> tuple[bytes, str | None]: + """Fetch bounded archive bytes after validating URL and redirects.""" + import urllib.error + from urllib.parse import urlparse + + from specify_cli.authentication.http import github_provider_hosts, open_url + from specify_cli._github_http import resolve_github_release_asset_api_url + + try: + parsed = urlparse(url) + parsed.port + except ValueError as exc: + raise error_type(f"Invalid URL: {url}") from exc + if not is_https_or_localhost_http(url): + raise error_type("URL must use HTTPS (HTTP is only allowed for localhost)") + + def validate_redirect(old_url, new_url): + if not is_safe_download_redirect(old_url, new_url): + raise error_type( + "redirect target must use HTTPS or remain on localhost" + ) + + try: + resolved_url = resolve_github_release_asset_api_url( + url, open_url, github_hosts=github_provider_hosts() + ) + download_url = resolved_url or url + extra_headers = {"Accept": "application/octet-stream"} if resolved_url else None + with open_url( + download_url, + timeout=60, + extra_headers=extra_headers, + redirect_validator=validate_redirect, + ) as response: + final_url = ( + response.geturl() if hasattr(response, "geturl") else download_url + ) + if not is_https_or_localhost_http(final_url): + raise error_type("redirect target uses a disallowed URL") + data = read_response_limited( + response, error_type=error_type, label=f"preset {url}" + ) + content_type = ( + response.getheader("Content-Type") + if hasattr(response, "getheader") + else None + ) + except urllib.error.URLError as exc: + raise error_type(f"Failed to download preset: {exc}") from exc + return data, content_type + + +def _write_preset_archive( + data: bytes, + url: str, + content_type: str | None, + error_type: type[Exception], +) -> Path: + """Write downloaded bytes to a temporary archive with a detected suffix.""" + declared_format = archive_format_from_name(url) + suffix = archive_suffix(declared_format) if declared_format else ".archive" + fd, name = tempfile.mkstemp(prefix="speckit-preset-update-", suffix=suffix) + os.close(fd) + path = Path(name) + path.write_bytes(data) + detected = detect_archive_format( + path, + source_name=url, + content_type=content_type, + error_type=error_type, + ) + if declared_format is None: + detected_path = path.with_suffix(archive_suffix(detected)) + os.replace(path, detected_path) + path = detected_path + return path + + +def _download_preset_archive(url: str, error_type: type[Exception]) -> Path: + """Download and classify a preset archive into a temporary file.""" + data, content_type = _fetch_preset_archive_data(url, error_type) + return _write_preset_archive(data, url, content_type, error_type) + + def _warn_unmet_extension_dependencies(manager, manifest) -> None: """Warn when a preset's declared extension dependencies are unsatisfied. @@ -477,75 +564,6 @@ def preset_update( outcomes = [] catalog_candidates = {} - def download_explicit(url: str) -> Path: - import urllib.error - from urllib.parse import urlparse - from specify_cli.authentication.http import open_url - from specify_cli.authentication.http import github_provider_hosts - from specify_cli._github_http import resolve_github_release_asset_api_url - - try: - parsed = urlparse(url) - parsed.port - except ValueError as exc: - raise PresetError(f"Invalid URL: {url}") from exc - if not is_https_or_localhost_http(url): - raise PresetError("URL must use HTTPS (HTTP is only allowed for localhost)") - def validate_redirect(old_url, new_url): - if not is_safe_download_redirect(old_url, new_url): - raise PresetError( - "redirect target must use HTTPS or remain on localhost" - ) - - try: - resolved_url = resolve_github_release_asset_api_url( - url, open_url, github_hosts=github_provider_hosts() - ) - download_url = resolved_url or url - extra_headers = ( - {"Accept": "application/octet-stream"} if resolved_url else None - ) - with open_url( - download_url, - timeout=60, - extra_headers=extra_headers, - redirect_validator=validate_redirect, - ) as response: - final_url = ( - response.geturl() - if hasattr(response, "geturl") - else download_url - ) - if not is_https_or_localhost_http(final_url): - raise PresetError("redirect target uses a disallowed URL") - data = read_response_limited( - response, error_type=PresetError, label=f"preset {url}" - ) - content_type = ( - response.getheader("Content-Type") - if hasattr(response, "getheader") - else None - ) - except urllib.error.URLError as exc: - raise PresetError(f"Failed to download preset: {exc}") from exc - declared_format = archive_format_from_name(url) - suffix = archive_suffix(declared_format) if declared_format else ".archive" - fd, name = tempfile.mkstemp(prefix="speckit-preset-update-", suffix=suffix) - os.close(fd) - path = Path(name) - path.write_bytes(data) - detected = detect_archive_format( - path, - source_name=url, - content_type=content_type, - error_type=PresetError, - ) - if declared_format is None: - detected_path = path.with_suffix(archive_suffix(detected)) - os.replace(path, detected_path) - path = detected_path - return path - if bulk: actionable_ids = [] catalog_archives = {} @@ -656,7 +674,7 @@ def validate_redirect(old_url, new_url): raise PresetError(f"Directory not found: {dev}") elif from_url: source_kind = "url" - archive_path = download_explicit(from_url) + archive_path = _download_preset_archive(from_url, PresetError) else: pack_info = catalog_candidates.get(item_id) or catalog.get_pack_info(item_id) if not pack_info: From d24612cea1a7c2a510358a10ae55d0ac542f2136 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 11:03:37 +0100 Subject: [PATCH 14/39] refactor: reuse preset archive download flow Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/_commands.py | 95 ++++++---------------------- 1 file changed, 18 insertions(+), 77 deletions(-) diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index 80e051fbb4..9377f0018e 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -82,7 +82,11 @@ def validate_redirect(old_url, new_url): response.geturl() if hasattr(response, "geturl") else download_url ) if not is_https_or_localhost_http(final_url): - raise error_type("redirect target uses a disallowed URL") + raise error_type( + "Preset URL redirected to a disallowed URL: " + f"{final_url}. Redirect targets must use HTTPS with a hostname, " + "or HTTP for localhost (127.0.0.1, ::1)." + ) data = read_response_limited( response, error_type=error_type, label=f"preset {url}" ) @@ -332,15 +336,6 @@ def preset_add( console.print(f"[red]Error:[/red] Invalid URL: {_escape_markup(from_url)}") raise typer.Exit(1) - def _validate_download_redirect(old_url, new_url): - if not is_safe_download_redirect(old_url, new_url): - import urllib.error - - raise urllib.error.URLError( - "redirect target must use HTTPS without entering a local " - "target, or stay within loopback over HTTP" - ) - if not is_https_or_localhost_http(from_url): console.print( "[red]Error:[/red] URL must use HTTPS with a hostname and be " @@ -350,78 +345,24 @@ def _validate_download_redirect(old_url, new_url): raise typer.Exit(1) console.print(f"Installing preset from [cyan]{_escape_markup(from_url)}[/cyan]...") - import urllib.error - import tempfile - - with tempfile.TemporaryDirectory() as tmpdir: - archive_path = Path(tmpdir) / "preset.archive" - try: - from specify_cli.authentication.http import open_url as _open_url - from specify_cli.authentication.http import github_provider_hosts - from specify_cli._github_http import resolve_github_release_asset_api_url - - _preset_extra_headers = None - _resolved_from_url = resolve_github_release_asset_api_url( - from_url, _open_url, github_hosts=github_provider_hosts() - ) - if _resolved_from_url: - from_url = _resolved_from_url - _preset_extra_headers = {"Accept": "application/octet-stream"} - - with _open_url( - from_url, - timeout=60, - extra_headers=_preset_extra_headers, - redirect_validator=_validate_download_redirect, - ) as response: - final_url = response.geturl() if hasattr(response, "geturl") else from_url - if not is_https_or_localhost_http(final_url): - console.print( - "[red]Error:[/red] Preset URL redirected to a disallowed URL: " - f"{final_url}. Redirect targets must use HTTPS with a hostname, " - "or HTTP for localhost (127.0.0.1, ::1)." - ) - raise typer.Exit(1) - archive_data = read_response_limited( - response, - error_type=PresetError, - label=f"preset {from_url}", - ) - content_type = ( - response.getheader("Content-Type") - if hasattr(response, "getheader") - else None - ) - archive_path.write_bytes(archive_data) - format_source = ( - final_url - if archive_format_from_name(final_url) is not None - else from_url - ) - archive_format = detect_archive_format( - archive_path, - source_name=format_source, - content_type=content_type, - error_type=PresetError, - ) - detected_path = archive_path.with_suffix( - archive_suffix(archive_format) - ) - os.replace(archive_path, detected_path) - archive_path = detected_path - except (urllib.error.URLError, PresetError) as e: - console.print( - f"[red]Error:[/red] Failed to download: " - f"{_escape_markup(str(e))}" - ) - raise typer.Exit(1) - + archive_path = None + try: + archive_path = _download_preset_archive(from_url, PresetError) + except PresetError as e: + console.print( + f"[red]Error:[/red] Failed to download: " + f"{_escape_markup(str(e))}" + ) + raise typer.Exit(1) + try: manifest = manager.install_from_zip( archive_path, speckit_version, priority, ) - + finally: + if archive_path is not None and archive_path.exists(): + archive_path.unlink() console.print(f"[green]✓[/green] Preset '{manifest.name}' v{manifest.version} installed (priority {priority})") elif preset_id: From 912b7d88e772aa5f76f97e02c454559f880ac4a9 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 11:14:28 +0100 Subject: [PATCH 15/39] fix: satisfy preset command lint Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/_commands.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index 9377f0018e..5a9b97cbb3 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -49,12 +49,12 @@ def _fetch_preset_archive_data( import urllib.error from urllib.parse import urlparse - from specify_cli.authentication.http import github_provider_hosts, open_url from specify_cli._github_http import resolve_github_release_asset_api_url + from specify_cli.authentication.http import github_provider_hosts, open_url try: parsed = urlparse(url) - parsed.port + _ = parsed.port except ValueError as exc: raise error_type(f"Invalid URL: {url}") from exc if not is_https_or_localhost_http(url): @@ -298,11 +298,11 @@ def preset_add( """Install a preset.""" from .. import _locate_bundled_preset, _require_specify_project, get_speckit_version from . import ( - PresetManager, PresetCatalog, + PresetCompatibilityError, PresetError, + PresetManager, PresetValidationError, - PresetCompatibilityError, ) project_root = _require_specify_project() @@ -331,7 +331,7 @@ def preset_add( try: _parsed = _urlparse(from_url) - _parsed.port + _ = _parsed.port except ValueError: console.print(f"[red]Error:[/red] Invalid URL: {_escape_markup(from_url)}") raise typer.Exit(1) @@ -467,6 +467,8 @@ def preset_update( all_presets: bool = typer.Option(False, "--all", help="Update all installed presets"), ): """Update one preset, or all installed presets.""" + from packaging import version as pkg_version + from .. import _require_specify_project, get_speckit_version from . import ( PresetCatalog, @@ -475,7 +477,6 @@ def preset_update( PresetManager, PresetValidationError, ) - from packaging import version as pkg_version project_root = _require_specify_project() manager = PresetManager(project_root) @@ -557,7 +558,7 @@ def preset_update( outcomes.append("skipped") if archive_path is not None: archive_path.unlink(missing_ok=True) - except (KeyError, TypeError, ValueError) as exc: + except (KeyError, TypeError, ValueError): detail = _escape_markup( f"invalid version metadata for preset '{item_id}'" ) @@ -866,7 +867,7 @@ def preset_info( """Show detailed information about a preset.""" from .. import _require_specify_project from ..extensions import normalize_priority - from . import PresetCatalog, PresetManager, PresetError + from . import PresetCatalog, PresetError, PresetManager project_root = _require_specify_project() safe_preset_id = _escape_markup(str(preset_id)) From ef8fd1a3bf545db0abc6c3a8cd31297229d23558 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 11:31:40 +0100 Subject: [PATCH 16/39] Fix bundled preset updates Use the installed bundled preset directory when catalogue metadata has no download URL. Preserve archive handling for externally downloaded presets. Assisted-by: GitHub Copilot (model: GPT-5.6 Luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/_commands.py | 52 ++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 11 deletions(-) diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index 5a9b97cbb3..5a5c74c05a 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -469,7 +469,11 @@ def preset_update( """Update one preset, or all installed presets.""" from packaging import version as pkg_version - from .. import _require_specify_project, get_speckit_version + from .. import ( + _locate_bundled_preset, + _require_specify_project, + get_speckit_version, + ) from . import ( PresetCatalog, PresetCompatibilityError, @@ -505,6 +509,7 @@ def preset_update( speckit_version = get_speckit_version() outcomes = [] catalog_candidates = {} + catalog_sources = {} if bulk: actionable_ids = [] @@ -539,14 +544,30 @@ def preset_update( ) outcomes.append("skipped") continue - archive_path = catalog.download_pack(item_id) - manager.update_from_archive( - archive_path, - speckit_version, - pack_id=item_id, - priority=effective_priority, - dry_run=True, - ) + if pack_info.get("bundled") and not pack_info.get("download_url"): + source_path = _locate_bundled_preset(item_id) + if source_path is None: + raise PresetError( + f"Preset '{item_id}' is bundled with spec-kit but " + "could not be found in the installed package" + ) + manager.update_from_directory( + source_path, + speckit_version, + pack_id=item_id, + priority=effective_priority, + dry_run=True, + ) + catalog_sources[item_id] = source_path + else: + archive_path = catalog.download_pack(item_id) + manager.update_from_archive( + archive_path, + speckit_version, + pack_id=item_id, + priority=effective_priority, + dry_run=True, + ) catalog_candidates[item_id] = pack_info catalog_archives[item_id] = archive_path actionable_ids.append(item_id) @@ -641,7 +662,16 @@ def preset_update( ) outcomes.append("skipped") continue - if archive_path is None: + if pack_info.get("bundled") and not pack_info.get("download_url"): + source_path = catalog_sources.get(item_id) or _locate_bundled_preset( + item_id + ) + if source_path is None: + raise PresetError( + f"Preset '{item_id}' is bundled with spec-kit but " + "could not be found in the installed package" + ) + elif archive_path is None: archive_path = catalog.download_pack(item_id) constitution_path = ( @@ -650,7 +680,7 @@ def preset_update( constitution_before = ( constitution_path.read_bytes() if constitution_path.exists() else None ) - if source_kind == "dev": + if source_kind == "dev" or source_path is not None: manifest, diff = manager.update_from_directory( source_path, speckit_version, From 02e21ef90a0871f60b463054f438bdb19b34489c Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 11:51:12 +0100 Subject: [PATCH 17/39] Clean up removed preset skills during updates Preserve recorded skill ownership until removed command skills are unregistered during preset reconciliation. Assisted-by: GitHub Copilot (model: GPT-5.6 Luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/__init__.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 47da6fcc16..99c583b0d5 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -4313,6 +4313,32 @@ def update_from_directory( modern, legacy = self._skill_names_for_command(name) affected_skill_names.update((modern, legacy)) registered_skills_before = metadata.get("registered_skills", {}) + if isinstance(registered_skills_before, dict): + removed_skill_names = { + skill_name + for command_name in removed_command_names + for skill_name in self._skill_names_for_command(command_name) + } + stale_skills = { + agent: [ + name + for name in names + if name in removed_skill_names + ] + for agent, names in registered_skills_before.items() + if isinstance(names, list) + } + stale_skills = { + agent: names + for agent, names in stale_skills.items() + if names + } + if stale_skills: + self._unregister_skills( + stale_skills, + current_dir, + restore_from_bundled_core=True, + ) registered_skills = self._register_skills( new_manifest, current_dir, command_names=command_names ) From b87a37a8235af8fa0852f1bf9e11bcea2fb04106 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 12:50:11 +0100 Subject: [PATCH 18/39] fix: address preset update review findings Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- docs/reference/presets.md | 28 +++++++++ src/specify_cli/presets/__init__.py | 90 +++++++++++++++++++--------- src/specify_cli/presets/_commands.py | 37 +++++++----- tests/test_presets.py | 82 ++++++++++++++++++++++++- 4 files changed, 194 insertions(+), 43 deletions(-) diff --git a/docs/reference/presets.md b/docs/reference/presets.md index 4bba88960b..758330e70a 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -66,6 +66,34 @@ unintended priority collisions. Updates stage and validate the new preset before atomically replacing the installed directory. +### Recovering from an interrupted update + +An interrupted update can leave `.bak` (the pre-update directory) +or `.staging` (the validated candidate) under +`.specify/presets/`. These are crash-recovery artefacts, not additional +installed presets. Do not edit the registry to point at either directory. + +Inspect both manifests before taking action: + +```bash +find .specify/presets -maxdepth 1 \ + \( -name '' -o -name '.bak' -o -name '.staging' \) \ + -print +sed -n '1,120p' .specify/presets//preset.yml +sed -n '1,120p' .specify/presets/.bak/preset.yml +sed -n '1,120p' .specify/presets/.staging/preset.yml +``` + +If the live `` directory is missing and the backup manifest is the +version recorded in `.specify/presets/.registry`, restore the backup by +renaming it to ``. Leave a staging directory untouched until its +contents have been inspected, then remove it only after confirming that the +live directory and registry agree. If the state is ambiguous, copy the +affected directories aside for inspection and use the supported update flow +with `--from ` or `--dev `, or remove and re-add the preset. There +is deliberately no repair command, because catalogue re-resolution is by +preset ID alone and durable source provenance is not maintained. + ## List Installed Presets ```bash diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 99c583b0d5..fb1b43a88a 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -1458,6 +1458,11 @@ def register_enabled_presets_for_agent(self, agent_name: str) -> None: name = tmpl.get("name") if tmpl.get("type") == "command" and isinstance(name, str): affected_cmd_names.add(name) + affected_cmd_names.update( + alias + for alias in tmpl.get("aliases", []) + if isinstance(alias, str) + ) # Isolate per-preset failures: one preset that fails to register # must not abort registration of the remaining enabled presets. @@ -4024,9 +4029,17 @@ def install_from_directory( # Reconcile all affected commands from the full priority stack so that # install order doesn't determine the winning command file. cmd_names = [ - t["name"] + name for t in manifest.templates if t.get("type") == "command" + for name in ( + [t["name"]] + + [ + alias + for alias in t.get("aliases", []) + if isinstance(alias, str) + ] + ) ] if cmd_names: try: @@ -4193,9 +4206,16 @@ def update_from_directory( shutil.rmtree(backup_dir) try: shutil.copytree(source_dir, staging_dir) - self._validate_update_source( + _, staged_manifest, staged_diff = self._validate_update_source( staging_dir, target_id, speckit_version ) + # The source directory may be edited while it is being copied + # (notably during --dev updates). Once staged, the copied tree is + # the immutable input for this update, so its validation result + # must drive both the registry metadata and reconciliation. + new_manifest = staged_manifest + diff = staged_diff + staged_manifest_hash = new_manifest.get_hash() except Exception: if staging_dir.exists(): shutil.rmtree(staging_dir) @@ -4206,11 +4226,12 @@ def update_from_directory( os.replace(current_dir, backup_dir) try: os.replace(staging_dir, current_dir) + new_manifest.path = current_dir / "preset.yml" self.registry.update( target_id, { "version": new_manifest.version, - "manifest_hash": new_manifest.get_hash(), + "manifest_hash": staged_manifest_hash, "priority": preserved_priority, "enabled": enabled, }, @@ -4233,13 +4254,13 @@ def update_from_directory( if item["identity"][1] == "command" } for item in diff["added"] + diff["removed"] + diff["changed"]: - template = item.get("new") or item.get("old") - if template and template.get("type") == "command": - command_names.update( - alias - for alias in template.get("aliases", []) - if isinstance(alias, str) - ) + for template in (item.get("old"), item.get("new")): + if template and template.get("type") == "command": + command_names.update( + alias + for alias in template.get("aliases", []) + if isinstance(alias, str) + ) try: old_command_names = { item["name"] @@ -4293,11 +4314,10 @@ def update_from_directory( merged_commands: Dict[str, List[str]] = {} for agent, names in registered_commands_before.items(): if isinstance(names, list): - retained = ( - [name for name in names if name not in command_names] - if agent == active_agent - else list(names) + names_to_remove = removed_command_names | ( + command_names if agent == active_agent else set() ) + retained = [name for name in names if name not in names_to_remove] if retained: merged_commands[agent] = retained for agent, names in registered_commands.items(): @@ -4346,15 +4366,10 @@ def update_from_directory( merged_skills: Dict[str, List[str]] = {} for agent, names in registered_skills_before.items(): if isinstance(names, list): - retained = ( - [ - name - for name in names - if name not in affected_skill_names - ] - if agent == active_agent - else list(names) + names_to_remove = removed_skill_names | ( + affected_skill_names if agent == active_agent else set() ) + retained = [name for name in names if name not in names_to_remove] if retained: merged_skills[agent] = retained for agent, names in registered_skills.items(): @@ -4387,11 +4402,27 @@ def update_from_directory( sorted(command_names), extra_skills_dirs=extra_skill_dirs or None, ) - has_constitution_layer = any( - item.get("type") == "template" - and item.get("name") == "constitution-template" - for item in old_manifest.templates + new_manifest.templates - ) + def _preset_has_constitution_layer( + manifest: PresetManifest, preset_dir: Path + ) -> bool: + return ( + any( + item.get("type") == "template" + and item.get("name") == "constitution-template" + for item in manifest.templates + ) + or any( + (preset_dir / relative_path).is_file() + for relative_path in ( + "templates/constitution-template.md", + "constitution-template.md", + ) + ) + ) + + has_constitution_layer = _preset_has_constitution_layer( + old_manifest, backup_dir + ) or _preset_has_constitution_layer(new_manifest, current_dir) if has_constitution_layer: self.reconcile_constitution( f"Failed to reconcile constitution after updating {target_id}", @@ -4402,7 +4433,10 @@ def update_from_directory( warnings.warn( f"Preset '{target_id}' was swapped, but post-update " - f"reconciliation failed: {exc}", + f"reconciliation failed: {exc}. Generated command, skill, or " + "constitution files may be stale. Re-run the update with " + f"--from or --dev , or remove and re-add " + f"preset '{target_id}' to refresh them.", stacklevel=2, ) finally: diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index 5a5c74c05a..00db46306d 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -110,20 +110,24 @@ def _write_preset_archive( declared_format = archive_format_from_name(url) suffix = archive_suffix(declared_format) if declared_format else ".archive" fd, name = tempfile.mkstemp(prefix="speckit-preset-update-", suffix=suffix) - os.close(fd) path = Path(name) - path.write_bytes(data) - detected = detect_archive_format( - path, - source_name=url, - content_type=content_type, - error_type=error_type, - ) - if declared_format is None: - detected_path = path.with_suffix(archive_suffix(detected)) - os.replace(path, detected_path) - path = detected_path - return path + try: + os.close(fd) + path.write_bytes(data) + detected = detect_archive_format( + path, + source_name=url, + content_type=content_type, + error_type=error_type, + ) + if declared_format is None: + detected_path = path.with_suffix(archive_suffix(detected)) + os.replace(path, detected_path) + path = detected_path + return path + except Exception: + path.unlink(missing_ok=True) + raise def _download_preset_archive(url: str, error_type: type[Exception]) -> Path: @@ -606,7 +610,8 @@ def preset_update( ) if not typer.confirm("Update all installed presets?"): for archive_path in catalog_archives.values(): - archive_path.unlink(missing_ok=True) + if archive_path is not None: + archive_path.unlink(missing_ok=True) console.print("Cancelled") return @@ -735,6 +740,10 @@ def preset_update( ) console.print(" planned actions: stage, validate, atomically swap, reconcile") outcomes.append("updated") + except OSError as exc: + detail = _escape_markup(str(exc).replace("\n", " ")) + console.print(f"[yellow]•[/yellow] {safe_id}: failed — {detail}") + outcomes.append("failed") except (PresetCompatibilityError, PresetValidationError, PresetError) as exc: detail = _escape_markup(str(exc).replace("\n", " ")) prefix = "skipped" if isinstance(exc, PresetCompatibilityError) else "failed" diff --git a/tests/test_presets.py b/tests/test_presets.py index bd297c76f3..eab6853abc 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -41,7 +41,10 @@ ) from specify_cli.extensions import ExtensionRegistry from specify_cli._console import console -from specify_cli.presets._commands import _warn_unmet_extension_dependencies +from specify_cli.presets._commands import ( + _warn_unmet_extension_dependencies, + _write_preset_archive, +) # ===== Fixtures ===== @@ -14856,6 +14859,83 @@ def fail_staged(source_dir, pack_id, speckit_version): project_dir / ".specify" / "presets" / "test-pack.staging" ).exists() + def test_staged_validation_result_is_authoritative( + self, project_dir, pack_dir, monkeypatch + ): + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.0") + source = self._updated_source(pack_dir) + original_copytree = shutil.copytree + + def copy_then_mutate(source_dir, destination_dir, *args, **kwargs): + result = original_copytree(source_dir, destination_dir, *args, **kwargs) + if isinstance(source_dir, Path) and source_dir == source: + data = yaml.safe_load((source_dir / "preset.yml").read_text()) + data["preset"]["version"] = "3.0.0" + (source_dir / "preset.yml").write_text(yaml.safe_dump(data)) + return result + + monkeypatch.setattr(shutil, "copytree", copy_then_mutate) + + manifest, _ = manager.update_from_directory( + source, "0.1.0", pack_id="test-pack" + ) + + assert manifest.version == "2.0.0" + assert manager.registry.get("test-pack")["version"] == "2.0.0" + + def test_convention_only_constitution_update_reconciles( + self, project_dir, temp_dir + ): + manager = PresetManager(project_dir) + manager.install_from_directory(CONSTITUTION_SYNC_PRESET_DIR, "0.15.0") + source = _make_convention_constitution_preset(temp_dir) + manager.install_from_directory(source, "0.1.5") + updated = temp_dir / "updated-convention-constitution" + shutil.copytree(source, updated) + data = yaml.safe_load((updated / "preset.yml").read_text()) + data["preset"]["version"] = "2.0.0" + (updated / "preset.yml").write_text(yaml.safe_dump(data)) + (updated / "templates" / "constitution-template.md").write_text( + "# Updated Convention Constitution\n" + ) + + manager.update_from_directory( + updated, "0.1.5", pack_id="convention-constitution" + ) + + memory = project_dir / ".specify" / "memory" / "constitution.md" + assert memory.read_text() == "# Updated Convention Constitution\n" + + def test_archive_write_cleans_temp_file_on_detection_failure( + self, monkeypatch + ): + created = {} + original_mkstemp = tempfile.mkstemp + + def record_mkstemp(*args, **kwargs): + fd, name = original_mkstemp(*args, **kwargs) + created["path"] = Path(name) + return fd, name + + monkeypatch.setattr(tempfile, "mkstemp", record_mkstemp) + monkeypatch.setattr( + "specify_cli.presets._commands.detect_archive_format", + lambda *args, **kwargs: (_ for _ in ()).throw( + PresetValidationError("invalid archive") + ), + ) + + with pytest.raises(PresetValidationError, match="invalid archive"): + _write_preset_archive( + b"not an archive", + "https://example.com/preset", + None, + PresetValidationError, + ) + + assert not created["path"].exists() + def test_cli_missing_id_directs_user_to_add(self, project_dir): from typer.testing import CliRunner from specify_cli import app From 963db02b6aa3b652fa49271e9767a652c1514de8 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 13:13:39 +0100 Subject: [PATCH 19/39] fix: harden preset update review paths Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/__init__.py | 56 +++++++++++++++++----------- src/specify_cli/presets/_commands.py | 36 +++++++++++++++--- 2 files changed, 65 insertions(+), 27 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index fb1b43a88a..ca215a8781 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -1458,11 +1458,6 @@ def register_enabled_presets_for_agent(self, agent_name: str) -> None: name = tmpl.get("name") if tmpl.get("type") == "command" and isinstance(name, str): affected_cmd_names.add(name) - affected_cmd_names.update( - alias - for alias in tmpl.get("aliases", []) - if isinstance(alias, str) - ) # Isolate per-preset failures: one preset that fails to register # must not abort registration of the remaining enabled presets. @@ -4169,6 +4164,7 @@ def update_from_directory( pack_id: Optional[str] = None, priority: Optional[int] = None, dry_run: bool = False, + expected_version: Optional[str] = None, ) -> tuple[PresetManifest, Dict[str, List[Dict[str, Any]]]]: """Update an installed preset from a validated directory.""" if priority is not None and priority < 1: @@ -4200,15 +4196,25 @@ def update_from_directory( enabled = metadata.get("enabled", True) staging_dir = self.presets_dir / f"{target_id}.staging" backup_dir = self.presets_dir / f"{target_id}.bak" - if staging_dir.exists() or staging_dir.is_symlink(): - shutil.rmtree(staging_dir) - if backup_dir.exists() or backup_dir.is_symlink(): - shutil.rmtree(backup_dir) + def remove_swap_path(path: Path) -> None: + if path.is_symlink(): + path.unlink() + elif path.exists(): + shutil.rmtree(path) + + remove_swap_path(staging_dir) + remove_swap_path(backup_dir) try: shutil.copytree(source_dir, staging_dir) _, staged_manifest, staged_diff = self._validate_update_source( staging_dir, target_id, speckit_version ) + if expected_version is not None and staged_manifest.version != expected_version: + raise PresetValidationError( + f"Preset '{target_id}' manifest version " + f"{staged_manifest.version} does not match catalogue version " + f"{expected_version}" + ) # The source directory may be edited while it is being copied # (notably during --dev updates). Once staged, the copied tree is # the immutable input for this update, so its validation result @@ -4217,8 +4223,7 @@ def update_from_directory( diff = staged_diff staged_manifest_hash = new_manifest.get_hash() except Exception: - if staging_dir.exists(): - shutil.rmtree(staging_dir) + remove_swap_path(staging_dir) raise current_dir = self.presets_dir / target_id registry_before = copy.deepcopy(metadata) @@ -4237,15 +4242,13 @@ def update_from_directory( }, ) except Exception: - if current_dir.exists(): - shutil.rmtree(current_dir) + remove_swap_path(current_dir) if backup_dir.exists(): os.replace(backup_dir, current_dir) self.registry.restore(target_id, registry_before) raise except Exception: - if staging_dir.exists(): - shutil.rmtree(staging_dir) + remove_swap_path(staging_dir) raise command_names = { @@ -4332,7 +4335,16 @@ def update_from_directory( for name in command_names: modern, legacy = self._skill_names_for_command(name) affected_skill_names.update((modern, legacy)) - registered_skills_before = metadata.get("registered_skills", {}) + raw_registered_skills_before = metadata.get("registered_skills", {}) + if isinstance(raw_registered_skills_before, list): + registered_skills_before = self._infer_legacy_skill_provenance( + [name for name in raw_registered_skills_before if isinstance(name, str)], + target_id, + ) + elif isinstance(raw_registered_skills_before, dict): + registered_skills_before = copy.deepcopy(raw_registered_skills_before) + else: + registered_skills_before = {} if isinstance(registered_skills_before, dict): removed_skill_names = { skill_name @@ -4440,10 +4452,8 @@ def _preset_has_constitution_layer( stacklevel=2, ) finally: - if backup_dir.exists(): - shutil.rmtree(backup_dir) - if staging_dir.exists(): - shutil.rmtree(staging_dir) + remove_swap_path(backup_dir) + remove_swap_path(staging_dir) return new_manifest, diff def update_from_archive( @@ -4454,6 +4464,7 @@ def update_from_archive( pack_id: str, priority: Optional[int] = None, dry_run: bool = False, + expected_version: Optional[str] = None, ) -> tuple[PresetManifest, Dict[str, List[Dict[str, Any]]]]: """Update an installed preset from a supported archive.""" with tempfile.TemporaryDirectory() as tmpdir: @@ -4472,6 +4483,7 @@ def update_from_archive( pack_id=pack_id, priority=priority, dry_run=dry_run, + expected_version=expected_version, ) def install_from_archive( @@ -4882,7 +4894,7 @@ class PresetCatalog: COMMUNITY_CATALOG_URL = "https://raw.githubusercontent.com/github/spec-kit/main/presets/catalog.community.json" CACHE_DURATION = 3600 # 1 hour in seconds - def __init__(self, project_root: Path): + def __init__(self, project_root: Path, cache_dir: Optional[Path] = None): """Initialize preset catalog manager. Args: @@ -4890,7 +4902,7 @@ def __init__(self, project_root: Path): """ self.project_root = project_root self.presets_dir = project_root / ".specify" / "presets" - self.cache_dir = self.presets_dir / ".cache" + self.cache_dir = cache_dir or self.presets_dir / ".cache" self.cache_file = self.cache_dir / "catalog.json" self.cache_metadata_file = self.cache_dir / "catalog-metadata.json" diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index 00db46306d..63e3fc2066 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -10,6 +10,7 @@ import os import re +import shutil import tempfile from pathlib import Path @@ -509,7 +510,12 @@ def preset_update( console.print("[yellow]No presets installed.[/yellow]") return - catalog = PresetCatalog(project_root) + dry_run_cache = ( + Path(tempfile.mkdtemp(prefix="speckit-preset-dry-run-")) + if dry_run + else None + ) + catalog = PresetCatalog(project_root, cache_dir=dry_run_cache) speckit_version = get_speckit_version() outcomes = [] catalog_candidates = {} @@ -547,6 +553,8 @@ def preset_update( f"(v{installed_version})[/dim]" ) outcomes.append("skipped") + if dry_run_cache is not None: + shutil.rmtree(dry_run_cache, ignore_errors=True) continue if pack_info.get("bundled") and not pack_info.get("download_url"): source_path = _locate_bundled_preset(item_id) @@ -561,6 +569,7 @@ def preset_update( pack_id=item_id, priority=effective_priority, dry_run=True, + expected_version=str(pack_info["version"]), ) catalog_sources[item_id] = source_path else: @@ -571,6 +580,7 @@ def preset_update( pack_id=item_id, priority=effective_priority, dry_run=True, + expected_version=str(pack_info["version"]), ) catalog_candidates[item_id] = pack_info catalog_archives[item_id] = archive_path @@ -601,7 +611,11 @@ def preset_update( ids = actionable_ids if not ids: if any(outcome == "failed" for outcome in outcomes): + if dry_run_cache is not None: + shutil.rmtree(dry_run_cache, ignore_errors=True) raise typer.Exit(1) + if dry_run_cache is not None: + shutil.rmtree(dry_run_cache, ignore_errors=True) return if priority is not None: console.print( @@ -613,6 +627,8 @@ def preset_update( if archive_path is not None: archive_path.unlink(missing_ok=True) console.print("Cancelled") + if dry_run_cache is not None: + shutil.rmtree(dry_run_cache, ignore_errors=True) return for item_id in ids: @@ -692,6 +708,9 @@ def preset_update( pack_id=item_id, priority=effective_priority, dry_run=dry_run, + expected_version=( + str(pack_info["version"]) if pack_info is not None else None + ), ) else: manifest, diff = manager.update_from_archive( @@ -700,6 +719,9 @@ def preset_update( pack_id=item_id, priority=effective_priority, dry_run=dry_run, + expected_version=( + str(pack_info["version"]) if pack_info is not None else None + ), ) action = "would update" if dry_run else "updated" added_commands = sum( @@ -712,10 +734,10 @@ def preset_update( constitution_path.read_bytes() if constitution_path.exists() else None ) constitution_unchanged = ( - any( - entry["identity"] - == ("constitution-template", "template") - for entry in diff["unchanged"] + not any( + entry["identity"] == ("constitution-template", "template") + for category in ("added", "removed", "changed") + for entry in diff[category] ) if dry_run else constitution_before == constitution_after @@ -754,7 +776,11 @@ def preset_update( archive_path.unlink(missing_ok=True) if any(outcome == "failed" for outcome in outcomes): + if dry_run_cache is not None: + shutil.rmtree(dry_run_cache, ignore_errors=True) raise typer.Exit(1) + if dry_run_cache is not None: + shutil.rmtree(dry_run_cache, ignore_errors=True) @preset_app.command("search") From fa22dc9aceac8b58083ef57b7b9918d19fccc6f3 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 14:19:31 +0100 Subject: [PATCH 20/39] fix: complete preset update review fixes Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/__init__.py | 58 ++++++++++++++++++++++++---- src/specify_cli/presets/_commands.py | 2 - tests/test_presets.py | 33 ++++++++++++++++ 3 files changed, 84 insertions(+), 9 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index ca215a8781..3301cf7726 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -2022,6 +2022,7 @@ def _reconcile_composed_commands( command_names: List[str], extra_agents: Optional[Set[str]] = None, target_agent: Optional[str] = None, + include_disabled_id: Optional[str] = None, ) -> Set[str]: """Re-resolve and re-register composed commands from the full stack. @@ -2070,7 +2071,9 @@ def _reconcile_composed_commands( except ImportError: return set() - resolver = PresetResolver(self.project_root) + resolver = PresetResolver( + self.project_root, include_disabled_id=include_disabled_id + ) registrar = CommandRegistrar() reconciled_commands: set[str] = set() @@ -2483,6 +2486,7 @@ def _reconcile_skills( Dict[Path, tuple[Optional[str], List[str]]] ] = None, target_agent: Optional[str] = None, + include_disabled_id: Optional[str] = None, ) -> Set[str]: """Re-register skills for commands whose winning layer changed. @@ -2510,7 +2514,9 @@ def _reconcile_skills( # command renders its skill whether or not a like-named extension is # installed. The per-name loop below skips anything that doesn't # resolve to a managed skill directory. - resolver = PresetResolver(self.project_root) + resolver = PresetResolver( + self.project_root, include_disabled_id=include_disabled_id + ) active_skills_dir = self._get_skills_dir() from .. import load_init_options @@ -4184,6 +4190,11 @@ def update_from_directory( old_manifest, new_manifest, diff = self._validate_update_source( source_dir, target_id, speckit_version ) + if expected_version is not None and new_manifest.version != expected_version: + raise PresetValidationError( + f"Preset '{target_id}' manifest version {new_manifest.version} " + f"does not match catalogue version {expected_version}" + ) if dry_run: return new_manifest, diff @@ -4251,11 +4262,35 @@ def remove_swap_path(path: Path) -> None: remove_swap_path(staging_dir) raise - command_names = { + primary_command_names = { item["identity"][0] for item in diff["added"] + diff["removed"] + diff["changed"] if item["identity"][1] == "command" } + command_names = set(primary_command_names) + command_names.update( + alias + for manifest in (old_manifest, new_manifest) + for item in manifest.templates + if item.get("type") == "command" + for alias in item.get("aliases", []) + if isinstance(alias, str) + ) + if priority is not None: + original_priority = normalize_priority(metadata.get("priority", 10)) + else: + original_priority = preserved_priority + if preserved_priority != original_priority: + primary_command_names.update( + item["name"] + for item in new_manifest.templates + if item.get("type") == "command" and isinstance(item.get("name"), str) + ) + primary_command_names.update( + item["name"] + for item in old_manifest.templates + if item.get("type") == "command" and isinstance(item.get("name"), str) + ) for item in diff["added"] + diff["removed"] + diff["changed"]: for template in (item.get("old"), item.get("new")): if template and template.get("type") == "command": @@ -4311,7 +4346,7 @@ def remove_swap_path(path: Path) -> None: if stale_commands: self._unregister_commands(stale_commands) registered_commands = self._register_commands( - new_manifest, current_dir, command_names=command_names + new_manifest, current_dir, command_names=primary_command_names ) active_agent = resolve_active_agent_for_registration(self.project_root) merged_commands: Dict[str, List[str]] = {} @@ -4398,7 +4433,9 @@ def remove_swap_path(path: Path) -> None: if agent != active_agent } self._reconcile_composed_commands( - sorted(command_names), extra_agents=historical_agents + sorted(primary_command_names), + extra_agents=historical_agents, + include_disabled_id=target_id if not enabled else None, ) extra_skill_dirs = {} if isinstance(registered_skills_before, dict): @@ -4413,6 +4450,7 @@ def remove_swap_path(path: Path) -> None: self._reconcile_skills( sorted(command_names), extra_skills_dirs=extra_skill_dirs or None, + include_disabled_id=target_id if not enabled else None, ) def _preset_has_constitution_layer( manifest: PresetManifest, preset_dir: Path @@ -5774,7 +5812,9 @@ class PresetResolver: 4. .specify/templates/ - Core templates (shipped with Spec Kit) """ - def __init__(self, project_root: Path): + def __init__( + self, project_root: Path, include_disabled_id: Optional[str] = None + ): """Initialize preset resolver. Args: @@ -5785,6 +5825,7 @@ def __init__(self, project_root: Path): self.presets_dir = project_root / ".specify" / "presets" self.overrides_dir = self.templates_dir / "overrides" self.extensions_dir = project_root / ".specify" / "extensions" + self.include_disabled_id = include_disabled_id self._manifest_cache: Dict[str, Optional["PresetManifest"]] = {} def _get_manifest(self, pack_dir: Path) -> Optional["PresetManifest"]: @@ -5809,7 +5850,10 @@ def _get_all_presets_by_priority(self) -> List[tuple[str, dict]]: registry = PresetRegistry(self.presets_dir) return [ (pack_id, metadata) - for pack_id, metadata in registry.list_by_priority() + for pack_id, metadata in registry.list_by_priority( + include_disabled=self.include_disabled_id is not None + ) + if metadata.get("enabled", True) or pack_id == self.include_disabled_id if self._is_safe_registry_id(pack_id) ] diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index 63e3fc2066..4082046402 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -553,8 +553,6 @@ def preset_update( f"(v{installed_version})[/dim]" ) outcomes.append("skipped") - if dry_run_cache is not None: - shutil.rmtree(dry_run_cache, ignore_errors=True) continue if pack_info.get("bundled") and not pack_info.get("download_url"): source_path = _locate_bundled_preset(item_id) diff --git a/tests/test_presets.py b/tests/test_presets.py index eab6853abc..aa57578bf9 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -14724,6 +14724,39 @@ def test_dry_run_does_not_modify_installation(self, project_dir, pack_dir): ).read_bytes() == before assert manager.registry.get("test-pack")["version"] == "1.0.0" + def test_dry_run_validates_catalogue_expected_version( + self, project_dir, pack_dir + ): + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.0") + source = self._updated_source(pack_dir) + + with pytest.raises(PresetValidationError, match="does not match catalogue"): + manager.update_from_directory( + source, + "0.1.0", + pack_id="test-pack", + dry_run=True, + expected_version="9.9.9", + ) + + assert manager.registry.get("test-pack")["version"] == "1.0.0" + + def test_disabled_preset_is_included_when_reconciling_its_update( + self, project_dir, pack_dir + ): + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.0") + manager.registry.update("test-pack", {"enabled": False}) + + resolver = PresetResolver(project_dir, include_disabled_id="test-pack") + layers = resolver.collect_all_layers("spec-template", "template") + + assert layers + assert layers[0]["path"] == ( + project_dir / ".specify" / "presets" / "test-pack" / "templates" / "spec-template.md" + ) + def test_missing_referenced_file_leaves_installation_untouched( self, project_dir, pack_dir ): From 6c0b134adfaaf84a1ea0c306a78dcf875f63da9a Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 14:49:53 +0100 Subject: [PATCH 21/39] fix: tighten disabled preset reconciliation Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/__init__.py | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 3301cf7726..bfd66a0250 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -2122,7 +2122,13 @@ def record_written(written: Dict[str, List[str]]) -> None: # Cache registry and manifests outside the loop to avoid # repeated filesystem reads for each command name. - presets_by_priority = list(self.registry.list_by_priority()) + presets_by_priority = [ + (pack_id, metadata) + for pack_id, metadata in self.registry.list_by_priority( + include_disabled=include_disabled_id is not None + ) + if metadata.get("enabled", True) or pack_id == include_disabled_id + ] for cmd_name in command_names: layers = resolver.collect_all_layers(cmd_name, "command") @@ -2527,7 +2533,13 @@ def _reconcile_skills( active_ai = None # Cache registry once to avoid repeated filesystem reads - presets_by_priority = list(self.registry.list_by_priority()) + presets_by_priority = [ + (pack_id, metadata) + for pack_id, metadata in self.registry.list_by_priority( + include_disabled=include_disabled_id is not None + ) + if metadata.get("enabled", True) or pack_id == include_disabled_id + ] # Group command names by winning preset to batch _register_skills calls # while only registering skills for the specific commands being @@ -4291,6 +4303,7 @@ def remove_swap_path(path: Path) -> None: for item in old_manifest.templates if item.get("type") == "command" and isinstance(item.get("name"), str) ) + reconcile_command_names = set(primary_command_names) for item in diff["added"] + diff["removed"] + diff["changed"]: for template in (item.get("old"), item.get("new")): if template and template.get("type") == "command": @@ -4407,7 +4420,7 @@ def remove_swap_path(path: Path) -> None: restore_from_bundled_core=True, ) registered_skills = self._register_skills( - new_manifest, current_dir, command_names=command_names + new_manifest, current_dir, command_names=reconcile_command_names ) if isinstance(registered_skills_before, dict): merged_skills: Dict[str, List[str]] = {} @@ -4426,14 +4439,14 @@ def remove_swap_path(path: Path) -> None: ) registered_skills = merged_skills self.registry.update(target_id, {"registered_skills": registered_skills}) - if command_names: + if reconcile_command_names: historical_agents = { agent for agent in registered_commands_before if agent != active_agent } self._reconcile_composed_commands( - sorted(primary_command_names), + sorted(reconcile_command_names), extra_agents=historical_agents, include_disabled_id=target_id if not enabled else None, ) @@ -4448,7 +4461,7 @@ def remove_swap_path(path: Path) -> None: sorted(affected_skill_names), ) self._reconcile_skills( - sorted(command_names), + sorted(reconcile_command_names), extra_skills_dirs=extra_skill_dirs or None, include_disabled_id=target_id if not enabled else None, ) From 7217c4b411c810a51365ae572f9f89c844725b3c Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 15:00:56 +0100 Subject: [PATCH 22/39] fix: harden preset update validation Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/__init__.py | 29 ++++++++++++++++++++++++++-- src/specify_cli/presets/_commands.py | 13 ++++++++++--- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index bfd66a0250..f86d9e4a10 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -457,6 +457,16 @@ def _validate(self): f"Invalid template {field}: expected a string, " f"got {type(tmpl[field]).__name__}" ) + aliases = tmpl.get("aliases", []) + if aliases is None: + aliases = [] + tmpl["aliases"] = aliases + if not isinstance(aliases, list) or not all( + isinstance(alias, str) for alias in aliases + ): + raise PresetValidationError( + "Invalid template aliases: expected a list of strings" + ) if tmpl["type"] not in VALID_PRESET_TEMPLATE_TYPES: raise PresetValidationError( @@ -4170,9 +4180,24 @@ def _validate_update_source( f"'{template['file']}'. Remove and add the preset again." ) diff = diff_preset_manifests(old_manifest, new_manifest) - return old_manifest, new_manifest, _diff_preset_template_files( + diff = _diff_preset_template_files( diff, old_manifest, new_manifest, current_dir, source_dir ) + def convention_constitution(base_dir: Path) -> Optional[bytes]: + for relative in ( + "templates/constitution-template.md", + "constitution-template.md", + ): + candidate = base_dir / relative + if candidate.is_file(): + return candidate.read_bytes() + return None + + diff["_constitution_changed"] = ( + convention_constitution(current_dir) + != convention_constitution(source_dir) + ) + return old_manifest, new_manifest, diff def update_from_directory( self, @@ -4303,7 +4328,6 @@ def remove_swap_path(path: Path) -> None: for item in old_manifest.templates if item.get("type") == "command" and isinstance(item.get("name"), str) ) - reconcile_command_names = set(primary_command_names) for item in diff["added"] + diff["removed"] + diff["changed"]: for template in (item.get("old"), item.get("new")): if template and template.get("type") == "command": @@ -4312,6 +4336,7 @@ def remove_swap_path(path: Path) -> None: for alias in template.get("aliases", []) if isinstance(alias, str) ) + reconcile_command_names = set(primary_command_names) try: old_command_names = { item["name"] diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index 4082046402..7f971ad728 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -599,7 +599,7 @@ def preset_update( outcomes.append("failed") if archive_path is not None: archive_path.unlink(missing_ok=True) - except (PresetValidationError, PresetError) as exc: + except (OSError, PresetValidationError, PresetError) as exc: detail = _escape_markup(str(exc).replace("\n", " ")) console.print(f"[yellow]•[/yellow] {safe_id}: failed — {detail}") outcomes.append("failed") @@ -732,7 +732,8 @@ def preset_update( constitution_path.read_bytes() if constitution_path.exists() else None ) constitution_unchanged = ( - not any( + not diff.get("_constitution_changed", False) + and not any( entry["identity"] == ("constitution-template", "template") for category in ("added", "removed", "changed") for entry in diff[category] @@ -759,6 +760,8 @@ def preset_update( f" {category}: {', '.join(identities)}" ) console.print(" planned actions: stage, validate, atomically swap, reconcile") + else: + _warn_unmet_extension_dependencies(manager, manifest) outcomes.append("updated") except OSError as exc: detail = _escape_markup(str(exc).replace("\n", " ")) @@ -766,7 +769,11 @@ def preset_update( outcomes.append("failed") except (PresetCompatibilityError, PresetValidationError, PresetError) as exc: detail = _escape_markup(str(exc).replace("\n", " ")) - prefix = "skipped" if isinstance(exc, PresetCompatibilityError) else "failed" + prefix = ( + "skipped" + if bulk and isinstance(exc, PresetCompatibilityError) + else "failed" + ) console.print(f"[yellow]•[/yellow] {safe_id}: {prefix} — {detail}") outcomes.append(prefix) finally: From e7965a0ae443dcfcdbcb05baee61e953921238bf Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 15:02:17 +0100 Subject: [PATCH 23/39] docs: clarify preset update source resolution Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- docs/reference/presets.md | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/docs/reference/presets.md b/docs/reference/presets.md index 758330e70a..38da1d6649 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -46,9 +46,19 @@ specify preset update [] ``` Updates one installed preset, or all installed presets when no ID is given. -Catalogue updates are resolved by preset ID and only use catalogues that allow -installation. A preset installed from a local directory or an archive URL must -be updated with the same `--dev ` or `--from ` source. +Catalogue updates are resolved by preset ID alone. Spec Kit searches the active +catalogues for a matching entry and only uses entries from catalogues that +allow installation. It does not retain or replay the original URL or local +directory used to install a preset. If the installed preset ID cannot be +resolved through an installation-enabled catalogue, provide an explicit +`--dev ` or `--from ` source. + +This means a preset installed from a catalogue can later be updated from the +catalogue entry currently associated with its ID, while a development or +one-off archive installation remains updateable only when an explicit source +is supplied or when that ID is otherwise available from an installation-enabled +catalogue. The incoming manifest must still use the installed preset ID and +pass normal compatibility and file validation. | Option | Description | | ---------------- | ------------------------------------------------ | @@ -91,8 +101,9 @@ contents have been inspected, then remove it only after confirming that the live directory and registry agree. If the state is ambiguous, copy the affected directories aside for inspection and use the supported update flow with `--from ` or `--dev `, or remove and re-add the preset. There -is deliberately no repair command, because catalogue re-resolution is by -preset ID alone and durable source provenance is not maintained. +is deliberately no repair command. Recovery uses the normal update, remove, and +add flows, and catalogue re-resolution is deliberately based on preset ID +rather than stored source provenance. ## List Installed Presets From c4f60c56eb87e9316b67dcabc7c2fbacc7d150fe Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 15:35:45 +0100 Subject: [PATCH 24/39] fix: harden preset alias reconciliation Validate aliases as safe relative identifiers and reconcile only aliases affected by the manifest diff, while including removed aliases in final stack reconciliation. Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/__init__.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index f86d9e4a10..6e9bd631ec 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -45,7 +45,11 @@ ) from .._invocation_style import get_invocation_prefix from ..integrations.base import IntegrationBase -from .._utils import dump_frontmatter, version_satisfies +from .._utils import ( + dump_frontmatter, + relative_extension_path_violation, + version_satisfies, +) from ..shared_infra import ( _ensure_safe_shared_destination, _ensure_safe_shared_directory, @@ -467,6 +471,12 @@ def _validate(self): raise PresetValidationError( "Invalid template aliases: expected a list of strings" ) + for alias in aliases: + reason = relative_extension_path_violation(alias) + if reason: + raise PresetValidationError( + f"Invalid template alias '{alias}': {reason}" + ) if tmpl["type"] not in VALID_PRESET_TEMPLATE_TYPES: raise PresetValidationError( @@ -4305,14 +4315,6 @@ def remove_swap_path(path: Path) -> None: if item["identity"][1] == "command" } command_names = set(primary_command_names) - command_names.update( - alias - for manifest in (old_manifest, new_manifest) - for item in manifest.templates - if item.get("type") == "command" - for alias in item.get("aliases", []) - if isinstance(alias, str) - ) if priority is not None: original_priority = normalize_priority(metadata.get("priority", 10)) else: @@ -4337,6 +4339,7 @@ def remove_swap_path(path: Path) -> None: if isinstance(alias, str) ) reconcile_command_names = set(primary_command_names) + reconcile_command_names.update(command_names) try: old_command_names = { item["name"] @@ -4363,6 +4366,7 @@ def remove_swap_path(path: Path) -> None: if isinstance(alias, str) ) removed_command_names = old_command_names - new_command_names + reconcile_command_names.update(removed_command_names) registered_commands_before = metadata.get("registered_commands", {}) if removed_command_names and isinstance( registered_commands_before, dict @@ -4405,7 +4409,7 @@ def remove_swap_path(path: Path) -> None: target_id, {"registered_commands": registered_commands} ) affected_skill_names = set() - for name in command_names: + for name in reconcile_command_names: modern, legacy = self._skill_names_for_command(name) affected_skill_names.update((modern, legacy)) raw_registered_skills_before = metadata.get("registered_skills", {}) From eb42603b2b47a5d2da7111d0976e6a9307551811 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 15:42:56 +0100 Subject: [PATCH 25/39] fix: isolate concurrent preset updates Use unique per-update staging and backup paths, discard copied composition artefacts before validation, and document the explicit bulk update option. Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- docs/reference/presets.md | 1 + src/specify_cli/presets/__init__.py | 15 +++++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/docs/reference/presets.md b/docs/reference/presets.md index 38da1d6649..8505796fd2 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -65,6 +65,7 @@ pass normal compatibility and file validation. | `--from ` | Update from a `.zip`, `.tar.gz`, or `.tgz` URL | | `--dev ` | Update from a local directory | | `--priority ` | Set a new resolution priority for a single update | +| `--all` | Update all installed presets | | `--dry-run` | Show the manifest diff without changing anything | Single-preset updates do not prompt for confirmation and may use `--priority` diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 6e9bd631ec..00a115617a 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -4252,18 +4252,25 @@ def update_from_directory( priority if priority is not None else metadata.get("priority", 10) ) enabled = metadata.get("enabled", True) - staging_dir = self.presets_dir / f"{target_id}.staging" - backup_dir = self.presets_dir / f"{target_id}.bak" + swap_token = tempfile.mkdtemp( + prefix=f".{target_id}.update-", dir=self.presets_dir + ) + os.rmdir(swap_token) + staging_dir = Path(f"{swap_token}.staging") + backup_dir = Path(f"{swap_token}.bak") def remove_swap_path(path: Path) -> None: if path.is_symlink(): path.unlink() elif path.exists(): shutil.rmtree(path) - remove_swap_path(staging_dir) - remove_swap_path(backup_dir) try: shutil.copytree(source_dir, staging_dir) + generated_composition = staging_dir / ".composed" + if generated_composition.is_symlink(): + generated_composition.unlink() + elif generated_composition.is_dir(): + shutil.rmtree(generated_composition) _, staged_manifest, staged_diff = self._validate_update_source( staging_dir, target_id, speckit_version ) From 5eab13f6792ceac9ab3ac6c3a6883a80982523c5 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 15:44:24 +0100 Subject: [PATCH 26/39] fix: improve preset update previews Include aliases during priority reconciliation and report dry-run constitution status conservatively when the projected stack may differ. Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/__init__.py | 8 ++++++++ src/specify_cli/presets/_commands.py | 24 +++++++++++++++--------- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 00a115617a..3db1d2721f 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -4337,6 +4337,14 @@ def remove_swap_path(path: Path) -> None: for item in old_manifest.templates if item.get("type") == "command" and isinstance(item.get("name"), str) ) + for manifest in (old_manifest, new_manifest): + for item in manifest.templates: + if item.get("type") == "command": + command_names.update( + alias + for alias in item.get("aliases", []) + if isinstance(alias, str) + ) for item in diff["added"] + diff["removed"] + diff["changed"]: for template in (item.get("old"), item.get("new")): if template and template.get("type") == "command": diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index 7f971ad728..00acffd7ac 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -731,20 +731,26 @@ def preset_update( constitution_after = ( constitution_path.read_bytes() if constitution_path.exists() else None ) - constitution_unchanged = ( - not diff.get("_constitution_changed", False) - and not any( - entry["identity"] == ("constitution-template", "template") - for category in ("added", "removed", "changed") - for entry in diff[category] - ) + constitution_diff = diff.get("_constitution_changed", False) or any( + entry["identity"] == ("constitution-template", "template") + for category in ("added", "removed", "changed") + for entry in diff[category] + ) + constitution_status = ( + "constitution reconciliation pending" + if dry_run and priority is not None and not constitution_diff + else "constitution change planned" + if dry_run and constitution_diff + else "constitution unchanged" if dry_run - else constitution_before == constitution_after + else "constitution unchanged" + if constitution_before == constitution_after + else "constitution reconciled" ) console.print( f"[green]✓[/green] {safe_id}: {action} to v{manifest.version} " f"(+{added_commands} commands, -{removed_commands} commands, " - f"{'constitution unchanged' if constitution_unchanged else 'constitution reconciled'}, " + f"{constitution_status}, " f"priority {'kept at ' + str(metadata.get('priority', 10)) if effective_priority is None else 'set to ' + str(effective_priority)})" ) if dry_run: From d05d6c31ec3fb23208bac28bcf6cb4ad74737a6c Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 15:54:42 +0100 Subject: [PATCH 27/39] fix: harden preset update reporting Use PEP 440 version equality, isolate temporary archive cleanup failures, report changed commands, and document unique recovery artefact names. Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- docs/reference/presets.md | 10 +++++----- src/specify_cli/presets/__init__.py | 16 +++++++++++++-- src/specify_cli/presets/_commands.py | 30 +++++++++++++++++++++------- 3 files changed, 42 insertions(+), 14 deletions(-) diff --git a/docs/reference/presets.md b/docs/reference/presets.md index 8505796fd2..094505ac8b 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -79,8 +79,8 @@ installed directory. ### Recovering from an interrupted update -An interrupted update can leave `.bak` (the pre-update directory) -or `.staging` (the validated candidate) under +An interrupted update can leave `..update-*.bak` (the pre-update +directory) or `..update-*.staging` (the validated candidate) under `.specify/presets/`. These are crash-recovery artefacts, not additional installed presets. Do not edit the registry to point at either directory. @@ -88,11 +88,11 @@ Inspect both manifests before taking action: ```bash find .specify/presets -maxdepth 1 \ - \( -name '' -o -name '.bak' -o -name '.staging' \) \ + \( -name '' -o -name '.*.update-*.bak' -o -name '.*.update-*.staging' \) \ -print sed -n '1,120p' .specify/presets//preset.yml -sed -n '1,120p' .specify/presets/.bak/preset.yml -sed -n '1,120p' .specify/presets/.staging/preset.yml +sed -n '1,120p' .specify/presets/..update-*.bak/preset.yml +sed -n '1,120p' .specify/presets/..update-*.staging/preset.yml ``` If the live `` directory is missing and the backup manifest is the diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 3db1d2721f..4daa68ff0c 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -81,6 +81,14 @@ def _is_comparable_version(value: str) -> bool: return True +def _versions_equal(left: str, right: str) -> bool: + """Compare versions using PEP 440 semantics where possible.""" + try: + return pkg_version.Version(left) == pkg_version.Version(right) + except pkg_version.InvalidVersion: + return left == right + + def _constitution_is_generated( project_root: Path, memory_constitution: Path, @@ -4237,7 +4245,9 @@ def update_from_directory( old_manifest, new_manifest, diff = self._validate_update_source( source_dir, target_id, speckit_version ) - if expected_version is not None and new_manifest.version != expected_version: + if expected_version is not None and not _versions_equal( + new_manifest.version, expected_version + ): raise PresetValidationError( f"Preset '{target_id}' manifest version {new_manifest.version} " f"does not match catalogue version {expected_version}" @@ -4274,7 +4284,9 @@ def remove_swap_path(path: Path) -> None: _, staged_manifest, staged_diff = self._validate_update_source( staging_dir, target_id, speckit_version ) - if expected_version is not None and staged_manifest.version != expected_version: + if expected_version is not None and not _versions_equal( + staged_manifest.version, expected_version + ): raise PresetValidationError( f"Preset '{target_id}' manifest version " f"{staged_manifest.version} does not match catalogue version " diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index 00acffd7ac..5827cf094f 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -137,6 +137,18 @@ def _download_preset_archive(url: str, error_type: type[Exception]) -> Path: return _write_preset_archive(data, url, content_type, error_type) +def _cleanup_archive(path: Path | None) -> None: + if path is None: + return + try: + path.unlink(missing_ok=True) + except OSError as exc: + console.print( + f"[yellow]Warning:[/yellow] Could not remove temporary archive " + f"'{path}': {exc}" + ) + + def _warn_unmet_extension_dependencies(manager, manifest) -> None: """Warn when a preset's declared extension dependencies are unsatisfied. @@ -367,7 +379,7 @@ def preset_add( ) finally: if archive_path is not None and archive_path.exists(): - archive_path.unlink() + _cleanup_archive(archive_path) console.print(f"[green]✓[/green] Preset '{manifest.name}' v{manifest.version} installed (priority {priority})") elif preset_id: @@ -418,7 +430,7 @@ def preset_add( console.print(f"[green]✓[/green] Preset '{manifest.name}' v{manifest.version} installed (priority {priority})") finally: if 'archive_path' in locals() and archive_path.exists(): - archive_path.unlink(missing_ok=True) + _cleanup_archive(archive_path) else: console.print("[red]Error:[/red] Specify a preset ID, --from URL, or --dev path") raise typer.Exit(1) @@ -590,7 +602,7 @@ def preset_update( ) outcomes.append("skipped") if archive_path is not None: - archive_path.unlink(missing_ok=True) + _cleanup_archive(archive_path) except (KeyError, TypeError, ValueError): detail = _escape_markup( f"invalid version metadata for preset '{item_id}'" @@ -598,13 +610,13 @@ def preset_update( console.print(f"[yellow]•[/yellow] {safe_id}: failed — {detail}") outcomes.append("failed") if archive_path is not None: - archive_path.unlink(missing_ok=True) + _cleanup_archive(archive_path) except (OSError, PresetValidationError, PresetError) as exc: detail = _escape_markup(str(exc).replace("\n", " ")) console.print(f"[yellow]•[/yellow] {safe_id}: failed — {detail}") outcomes.append("failed") if archive_path is not None: - archive_path.unlink(missing_ok=True) + _cleanup_archive(archive_path) ids = actionable_ids if not ids: @@ -623,7 +635,7 @@ def preset_update( if not typer.confirm("Update all installed presets?"): for archive_path in catalog_archives.values(): if archive_path is not None: - archive_path.unlink(missing_ok=True) + _cleanup_archive(archive_path) console.print("Cancelled") if dry_run_cache is not None: shutil.rmtree(dry_run_cache, ignore_errors=True) @@ -728,6 +740,9 @@ def preset_update( removed_commands = sum( entry["identity"][1] == "command" for entry in diff["removed"] ) + changed_commands = sum( + entry["identity"][1] == "command" for entry in diff["changed"] + ) constitution_after = ( constitution_path.read_bytes() if constitution_path.exists() else None ) @@ -750,6 +765,7 @@ def preset_update( console.print( f"[green]✓[/green] {safe_id}: {action} to v{manifest.version} " f"(+{added_commands} commands, -{removed_commands} commands, " + f"~{changed_commands} commands, " f"{constitution_status}, " f"priority {'kept at ' + str(metadata.get('priority', 10)) if effective_priority is None else 'set to ' + str(effective_priority)})" ) @@ -784,7 +800,7 @@ def preset_update( outcomes.append(prefix) finally: if archive_path is not None: - archive_path.unlink(missing_ok=True) + _cleanup_archive(archive_path) if any(outcome == "failed" for outcome in outcomes): if dry_run_cache is not None: From d2c328e3b5aadd8794b3ac86bd1cd168b5e6434d Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 15:57:50 +0100 Subject: [PATCH 28/39] fix: remove file-form composition artefacts Discard a source-supplied regular .composed path before staged preset validation, alongside directories and symlinks. Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 4daa68ff0c..2d9649f30f 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -4281,6 +4281,8 @@ def remove_swap_path(path: Path) -> None: generated_composition.unlink() elif generated_composition.is_dir(): shutil.rmtree(generated_composition) + elif generated_composition.exists(): + generated_composition.unlink() _, staged_manifest, staged_diff = self._validate_update_source( staging_dir, target_id, speckit_version ) From facb93f525850c36fe6a93c5edc22dca49922caf Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 16:19:49 +0100 Subject: [PATCH 29/39] address remaining preset update review threads Record constitution provenance for legacy generated files on the unchanged path, gather inactive agents stack-wide during priority-only reconciliation, fix the stale staging-path assertion, and add bulk CLI coverage for failure isolation, skips, and cancellation. Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/__init__.py | 39 ++++++++- tests/test_presets.py | 124 +++++++++++++++++++++++++++- 2 files changed, 159 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 2d9649f30f..82f713fd46 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -168,12 +168,32 @@ def _materialize_constitution_template( content = composed_content.encode("utf-8") result = "composed" + provenance = memory_constitution.parent / _CONSTITUTION_PROVENANCE_FILE if memory_constitution.exists() and memory_constitution.read_bytes() == content: + # A legacy generated constitution has no sidecar, and without one it is + # only recognised while it still byte-matches the bundled core + # template. Record provenance now so a later core upgrade cannot + # misclassify it as hand-edited. + if not provenance.exists(): + _ensure_safe_shared_directory( + project_root, memory_constitution.parent + ) + _write_shared_text( + project_root, + provenance, + json.dumps( + { + "sha256": _content_sha256(content), + "source": top_layer["source"], + }, + indent=2, + ) + + "\n", + ) return "unchanged" _ensure_safe_shared_directory(project_root, memory_constitution.parent) _write_shared_bytes(project_root, memory_constitution, content) - provenance = memory_constitution.parent / _CONSTITUTION_PROVENANCE_FILE _write_shared_text( project_root, provenance, @@ -4503,6 +4523,23 @@ def remove_swap_path(path: Path) -> None: for agent in registered_commands_before if agent != active_agent } + # A priority change can promote this preset above a provider + # whose ownership lives only on another preset's registry + # entry, so inactive agents must be gathered stack-wide rather + # than from this preset's own history alone. + for other_id, other_meta in self.registry.list_by_priority( + include_disabled=True + ): + if other_id == target_id or not isinstance(other_meta, dict): + continue + other_commands = other_meta.get("registered_commands", {}) + if not isinstance(other_commands, dict): + continue + for agent, names in other_commands.items(): + if agent == active_agent or not isinstance(names, list): + continue + if reconcile_command_names.intersection(names): + historical_agents.add(agent) self._reconcile_composed_commands( sorted(reconcile_command_names), extra_agents=historical_agents, diff --git a/tests/test_presets.py b/tests/test_presets.py index aa57578bf9..2c93fcec08 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -14888,9 +14888,9 @@ def fail_staged(source_dir, pack_id, speckit_version): assert ( project_dir / ".specify" / "presets" / "test-pack" / "preset.yml" ).read_bytes() == installed_manifest - assert not ( - project_dir / ".specify" / "presets" / "test-pack.staging" - ).exists() + assert not list( + (project_dir / ".specify" / "presets").glob(".test-pack.update-*") + ) def test_staged_validation_result_is_authoritative( self, project_dir, pack_dir, monkeypatch @@ -15010,3 +15010,121 @@ def test_cli_single_dry_run_does_not_prompt( assert "would update" in result.output assert "planned actions" in result.output assert "Update all installed presets?" not in result.output + + def _second_pack(self, pack_dir, pack_id, version="1.0.0"): + source = pack_dir.parent / pack_id + shutil.copytree(pack_dir, source) + manifest_path = source / "preset.yml" + data = yaml.safe_load(manifest_path.read_text()) + data["preset"]["id"] = pack_id + data["preset"]["name"] = pack_id + data["preset"]["version"] = version + manifest_path.write_text(yaml.safe_dump(data)) + return source + + def _bulk_cli_env(self, project_dir, monkeypatch, pack_infos, sources): + import specify_cli + from specify_cli.presets import PresetCatalog + + monkeypatch.setattr( + "specify_cli._require_specify_project", lambda: project_dir + ) + monkeypatch.setattr("specify_cli.get_speckit_version", lambda: "0.1.0") + monkeypatch.setattr( + PresetCatalog, + "get_pack_info", + lambda self, pack_id: pack_infos.get(pack_id), + ) + monkeypatch.setattr( + specify_cli, + "_locate_bundled_preset", + lambda pack_id: sources.get(pack_id), + ) + + def test_cli_bulk_isolates_failures_and_preserves_state( + self, project_dir, pack_dir, monkeypatch + ): + from typer.testing import CliRunner + from specify_cli import app + + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.0", priority=7) + second_source = self._second_pack(pack_dir, "second-pack") + manager.install_from_directory(second_source, "0.1.0", priority=5) + manager.registry.update("second-pack", {"enabled": False}) + + broken = pack_dir.parent / "broken-update" + shutil.copytree(pack_dir, broken) + (broken / "preset.yml").write_text("preset: [unbalanced\n") + good = self._second_pack(pack_dir, "second-pack-v2", version="2.0.0") + data = yaml.safe_load((good / "preset.yml").read_text()) + data["preset"]["id"] = "second-pack" + data["preset"]["name"] = "second-pack" + (good / "preset.yml").write_text(yaml.safe_dump(data)) + + self._bulk_cli_env( + project_dir, + monkeypatch, + { + "test-pack": {"version": "2.0.0", "bundled": True}, + "second-pack": {"version": "2.0.0", "bundled": True}, + }, + {"test-pack": broken, "second-pack": good}, + ) + + result = CliRunner().invoke(app, ["preset", "update"], input="y\n") + + assert result.exit_code == 1, result.output + assert "Update all installed presets?" in result.output + assert "test-pack" in result.output + registry = PresetManager(project_dir).registry + second = registry.get("second-pack") + assert second["version"] == "2.0.0" + assert second["enabled"] is False + assert second["priority"] == 5 + assert registry.get("test-pack")["version"] == "1.0.0" + + def test_cli_bulk_skips_already_current_presets( + self, project_dir, pack_dir, monkeypatch + ): + from typer.testing import CliRunner + from specify_cli import app + + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.0") + + self._bulk_cli_env( + project_dir, + monkeypatch, + {"test-pack": {"version": "1.0.0", "bundled": True}}, + {"test-pack": pack_dir}, + ) + + result = CliRunner().invoke(app, ["preset", "update"], input="y\n") + + assert result.exit_code == 0, result.output + assert "Up to date, skipped" in result.output + assert PresetManager(project_dir).registry.get("test-pack")["version"] == "1.0.0" + + def test_cli_bulk_cancellation_leaves_installations_untouched( + self, project_dir, pack_dir, monkeypatch + ): + from typer.testing import CliRunner + from specify_cli import app + + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.0") + source = self._updated_source(pack_dir) + + self._bulk_cli_env( + project_dir, + monkeypatch, + {"test-pack": {"version": "2.0.0", "bundled": True}}, + {"test-pack": source}, + ) + + result = CliRunner().invoke(app, ["preset", "update"], input="n\n") + + assert result.exit_code == 0, result.output + assert "Cancelled" in result.output + assert PresetManager(project_dir).registry.get("test-pack")["version"] == "1.0.0" From 1384ed31ba3eedd74ed3d2533fc9464a72c86fa7 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 16:38:38 +0100 Subject: [PATCH 30/39] keep unchanged constitutions free of file writes Revert the provenance sidecar write on the unchanged path. An unchanged resolved constitution must produce zero file changes, so provenance is recorded only when constitution content is actually materialized. Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/__init__.py | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 82f713fd46..08fa63e09f 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -170,26 +170,9 @@ def _materialize_constitution_template( provenance = memory_constitution.parent / _CONSTITUTION_PROVENANCE_FILE if memory_constitution.exists() and memory_constitution.read_bytes() == content: - # A legacy generated constitution has no sidecar, and without one it is - # only recognised while it still byte-matches the bundled core - # template. Record provenance now so a later core upgrade cannot - # misclassify it as hand-edited. - if not provenance.exists(): - _ensure_safe_shared_directory( - project_root, memory_constitution.parent - ) - _write_shared_text( - project_root, - provenance, - json.dumps( - { - "sha256": _content_sha256(content), - "source": top_layer["source"], - }, - indent=2, - ) - + "\n", - ) + # An unchanged constitution must produce zero file changes, so no + # provenance sidecar is written here either. Provenance is recorded + # only when constitution content is actually materialized below. return "unchanged" _ensure_safe_shared_directory(project_root, memory_constitution.parent) From 7ff2cd8d395c814e65f6713f130b3c6465afd38d Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 16:53:34 +0100 Subject: [PATCH 31/39] report a clear error when a preset vanishes mid-swap A concurrent update that wins the rename race left the losing process surfacing a raw errno for the backup rename. Translate that into a readable message stating no changes were made. Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/__init__.py | 8 +++++- tests/test_presets.py | 40 +++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 08fa63e09f..7ac6e8aeb6 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -4310,7 +4310,13 @@ def remove_swap_path(path: Path) -> None: current_dir = self.presets_dir / target_id registry_before = copy.deepcopy(metadata) try: - os.replace(current_dir, backup_dir) + try: + os.replace(current_dir, backup_dir) + except FileNotFoundError as exc: + raise PresetError( + f"Preset '{target_id}' changed on disk during the update " + "(another update may be running); no changes were made" + ) from exc try: os.replace(staging_dir, current_dir) new_manifest.path = current_dir / "preset.yml" diff --git a/tests/test_presets.py b/tests/test_presets.py index 2c93fcec08..2cd2a10b8a 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -15128,3 +15128,43 @@ def test_cli_bulk_cancellation_leaves_installations_untouched( assert result.exit_code == 0, result.output assert "Cancelled" in result.output assert PresetManager(project_dir).registry.get("test-pack")["version"] == "1.0.0" + + +class TestPresetUpdateConcurrency: + def _updated_source(self, pack_dir, version="2.0.0"): + source = pack_dir.parent / "concurrent-updated-pack" + shutil.copytree(pack_dir, source) + manifest_path = source / "preset.yml" + data = yaml.safe_load(manifest_path.read_text()) + data["preset"]["version"] = version + manifest_path.write_text(yaml.safe_dump(data)) + return source + + def test_vanished_install_during_swap_reports_clear_error( + self, project_dir, pack_dir, monkeypatch + ): + """A racing update that removes the live directory mid-swap must yield a + readable message rather than a raw errno.""" + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.0") + source = self._updated_source(pack_dir) + import os + + real_replace = os.replace + current = project_dir / ".specify" / "presets" / "test-pack" + + def racing_replace(src, dst): + if Path(src) == current: + shutil.rmtree(current) + return real_replace(src, dst) + + monkeypatch.setattr( + "specify_cli.presets.os.replace", racing_replace + ) + + with pytest.raises(PresetError, match="changed on disk during the update"): + manager.update_from_directory(source, "0.1.0", pack_id="test-pack") + + assert not list( + (project_dir / ".specify" / "presets").glob(".test-pack.update-*") + ) From 981b5f0daba5ce1d73922f0209677f8c68fa52e9 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 17:20:37 +0100 Subject: [PATCH 32/39] cover bulk preset update execution behaviour Add CLI tests for the bulk update path that were previously only exercised at preflight: post-confirmation failure isolation, one-time confirmation across multiple presets, incompatible-as-skipped, the discovery-only catalogue gate, newer-than-catalogue skip, unresolvable sources, and the explicit --all spelling. Each new assertion is mutation-verified: disabling the corresponding guard in _commands.py makes the matching test fail. Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- tests/test_presets.py | 196 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 194 insertions(+), 2 deletions(-) diff --git a/tests/test_presets.py b/tests/test_presets.py index 2cd2a10b8a..4d32bb701a 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -15075,8 +15075,8 @@ def test_cli_bulk_isolates_failures_and_preserves_state( result = CliRunner().invoke(app, ["preset", "update"], input="y\n") assert result.exit_code == 1, result.output - assert "Update all installed presets?" in result.output - assert "test-pack" in result.output + assert result.output.count("Update all installed presets?") == 1 + assert "test-pack: failed" in result.output registry = PresetManager(project_dir).registry second = registry.get("second-pack") assert second["version"] == "2.0.0" @@ -15129,6 +15129,198 @@ def test_cli_bulk_cancellation_leaves_installations_untouched( assert "Cancelled" in result.output assert PresetManager(project_dir).registry.get("test-pack")["version"] == "1.0.0" + def _two_actionable_packs(self, project_dir, pack_dir, monkeypatch): + """Install two presets that both clear preflight, so the main update + loop is genuinely exercised rather than short-circuited early.""" + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.0", priority=7) + manager.install_from_directory( + self._second_pack(pack_dir, "second-pack"), "0.1.0", priority=5 + ) + + def _renamed(tmp_id, target_id): + src = self._second_pack(pack_dir, tmp_id, version="2.0.0") + data = yaml.safe_load((src / "preset.yml").read_text()) + data["preset"]["id"] = target_id + data["preset"]["name"] = target_id + (src / "preset.yml").write_text(yaml.safe_dump(data)) + return src + + self._bulk_cli_env( + project_dir, + monkeypatch, + { + "test-pack": {"version": "2.0.0", "bundled": True}, + "second-pack": {"version": "2.0.0", "bundled": True}, + }, + { + "test-pack": _renamed("first-new", "test-pack"), + "second-pack": _renamed("second-new", "second-pack"), + }, + ) + + def test_cli_bulk_confirms_once_for_multiple_presets( + self, project_dir, pack_dir, monkeypatch + ): + """Two actionable presets must produce exactly one confirmation.""" + from typer.testing import CliRunner + from specify_cli import app + + self._two_actionable_packs(project_dir, pack_dir, monkeypatch) + + result = CliRunner().invoke(app, ["preset", "update"], input="y\n") + + assert result.exit_code == 0, result.output + assert result.output.count("Update all installed presets?") == 1 + registry = PresetManager(project_dir).registry + assert registry.get("test-pack")["version"] == "2.0.0" + assert registry.get("second-pack")["version"] == "2.0.0" + + def test_cli_all_flag_matches_bare_bulk_invocation( + self, project_dir, pack_dir, monkeypatch + ): + """The explicit --all spelling drives the same bulk path.""" + from typer.testing import CliRunner + from specify_cli import app + + self._two_actionable_packs(project_dir, pack_dir, monkeypatch) + + result = CliRunner().invoke(app, ["preset", "update", "--all"], input="y\n") + + assert result.exit_code == 0, result.output + assert result.output.count("Update all installed presets?") == 1 + registry = PresetManager(project_dir).registry + assert registry.get("test-pack")["version"] == "2.0.0" + assert registry.get("second-pack")["version"] == "2.0.0" + + def test_cli_bulk_execution_failure_does_not_abort_later_presets( + self, project_dir, pack_dir, monkeypatch + ): + """A failure raised inside the main update loop, after preflight and + confirmation both succeeded, must not stop a later preset updating.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.presets import PresetManager as RealManager + + self._two_actionable_packs(project_dir, pack_dir, monkeypatch) + real_update = RealManager.update_from_directory + + def failing_update(self, source, speckit_version, **kwargs): + # Only fail the real update, never the preflight dry run, so the + # failure is guaranteed to occur in the main loop. + if kwargs.get("pack_id") == "test-pack" and not kwargs.get("dry_run"): + raise PresetError("simulated execution failure") + return real_update(self, source, speckit_version, **kwargs) + + monkeypatch.setattr(RealManager, "update_from_directory", failing_update) + + result = CliRunner().invoke(app, ["preset", "update"], input="y\n") + + assert result.exit_code == 1, result.output + assert "test-pack: failed — simulated execution failure" in result.output + registry = PresetManager(project_dir).registry + assert registry.get("test-pack")["version"] == "1.0.0" + assert registry.get("second-pack")["version"] == "2.0.0" + + def test_cli_bulk_reports_incompatible_as_skipped_not_failed( + self, project_dir, pack_dir, monkeypatch + ): + """An incompatible preset surfacing during the real update is a true + no-op: reported as skipped, and it must not force a nonzero exit.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.presets import PresetCompatibilityError + from specify_cli.presets import PresetManager as RealManager + + self._two_actionable_packs(project_dir, pack_dir, monkeypatch) + real_update = RealManager.update_from_directory + + def incompatible(self, source, speckit_version, **kwargs): + if kwargs.get("pack_id") == "test-pack" and not kwargs.get("dry_run"): + raise PresetCompatibilityError("requires speckit >=9.0.0") + return real_update(self, source, speckit_version, **kwargs) + + monkeypatch.setattr(RealManager, "update_from_directory", incompatible) + + result = CliRunner().invoke(app, ["preset", "update"], input="y\n") + + assert result.exit_code == 0, result.output + assert "test-pack: skipped" in result.output + assert "failed" not in result.output + registry = PresetManager(project_dir).registry + assert registry.get("test-pack")["version"] == "1.0.0" + assert registry.get("second-pack")["version"] == "2.0.0" + + def test_cli_bulk_rejects_discovery_only_catalogue( + self, project_dir, pack_dir, monkeypatch + ): + """The _install_allowed discovery-only gate applies to updates.""" + from typer.testing import CliRunner + from specify_cli import app + + PresetManager(project_dir).install_from_directory(pack_dir, "0.1.0") + self._bulk_cli_env( + project_dir, + monkeypatch, + { + "test-pack": { + "version": "2.0.0", + "bundled": True, + "_install_allowed": False, + "_catalog_name": "community", + } + }, + {"test-pack": self._updated_source(pack_dir)}, + ) + + result = CliRunner().invoke(app, ["preset", "update"], input="y\n") + + assert result.exit_code == 1, result.output + assert "not allowed from" in result.output + assert "community" in result.output + assert PresetManager(project_dir).registry.get("test-pack")["version"] == "1.0.0" + + def test_cli_bulk_skips_when_installed_is_newer_than_catalogue( + self, project_dir, pack_dir, monkeypatch + ): + """A catalogue entry older than the installed version is a skip, never + a silent downgrade.""" + from typer.testing import CliRunner + from specify_cli import app + + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.0") + manager.registry.update("test-pack", {"version": "5.0.0"}) + + self._bulk_cli_env( + project_dir, + monkeypatch, + {"test-pack": {"version": "2.0.0", "bundled": True}}, + {"test-pack": self._updated_source(pack_dir)}, + ) + + result = CliRunner().invoke(app, ["preset", "update"], input="y\n") + + assert result.exit_code == 0, result.output + assert "Up to date, skipped" in result.output + assert PresetManager(project_dir).registry.get("test-pack")["version"] == "5.0.0" + + def test_cli_bulk_unresolvable_source_reports_clearly( + self, project_dir, pack_dir, monkeypatch + ): + """A preset with no catalogue entry cannot be re-resolved by id.""" + from typer.testing import CliRunner + from specify_cli import app + + PresetManager(project_dir).install_from_directory(pack_dir, "0.1.0") + self._bulk_cli_env(project_dir, monkeypatch, {}, {}) + + result = CliRunner().invoke(app, ["preset", "update"], input="y\n") + + assert result.exit_code == 1, result.output + assert "not re-resolvable" in result.output + assert "--from/--dev" in result.output + class TestPresetUpdateConcurrency: def _updated_source(self, pack_dir, version="2.0.0"): From 6cec91f1ec415e30415a456ce19379e53637d9e3 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 17:43:03 +0100 Subject: [PATCH 33/39] fix redirected preset archive classification Preserve the final URL returned after redirects and classify downloaded archives from their detected bytes using the canonical suffix. This prevents a .zip request that redirects to a .tgz asset from being validated with the wrong filename format. Add a regression test covering the redirected archive case. Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/_commands.py | 24 ++++++++++------------- tests/test_presets.py | 29 ++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index 5827cf094f..4d8637ccb0 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -20,7 +20,6 @@ from .._console import console from .._download_security import ( - archive_format_from_name, archive_suffix, detect_archive_format, is_https_or_localhost_http, @@ -45,7 +44,7 @@ def _fetch_preset_archive_data( url: str, error_type: type[Exception], -) -> tuple[bytes, str | None]: +) -> tuple[bytes, str | None, str]: """Fetch bounded archive bytes after validating URL and redirects.""" import urllib.error from urllib.parse import urlparse @@ -98,33 +97,30 @@ def validate_redirect(old_url, new_url): ) except urllib.error.URLError as exc: raise error_type(f"Failed to download preset: {exc}") from exc - return data, content_type + return data, content_type, final_url def _write_preset_archive( data: bytes, - url: str, + source_url: str, content_type: str | None, error_type: type[Exception], ) -> Path: """Write downloaded bytes to a temporary archive with a detected suffix.""" - declared_format = archive_format_from_name(url) - suffix = archive_suffix(declared_format) if declared_format else ".archive" - fd, name = tempfile.mkstemp(prefix="speckit-preset-update-", suffix=suffix) + fd, name = tempfile.mkstemp(prefix="speckit-preset-update-", suffix=".archive") path = Path(name) try: os.close(fd) path.write_bytes(data) detected = detect_archive_format( path, - source_name=url, + source_name=source_url, content_type=content_type, error_type=error_type, ) - if declared_format is None: - detected_path = path.with_suffix(archive_suffix(detected)) - os.replace(path, detected_path) - path = detected_path + detected_path = path.with_suffix(archive_suffix(detected)) + os.replace(path, detected_path) + path = detected_path return path except Exception: path.unlink(missing_ok=True) @@ -133,8 +129,8 @@ def _write_preset_archive( def _download_preset_archive(url: str, error_type: type[Exception]) -> Path: """Download and classify a preset archive into a temporary file.""" - data, content_type = _fetch_preset_archive_data(url, error_type) - return _write_preset_archive(data, url, content_type, error_type) + data, content_type, final_url = _fetch_preset_archive_data(url, error_type) + return _write_preset_archive(data, final_url, content_type, error_type) def _cleanup_archive(path: Path | None) -> None: diff --git a/tests/test_presets.py b/tests/test_presets.py index 4d32bb701a..20a7d4703b 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -42,6 +42,7 @@ from specify_cli.extensions import ExtensionRegistry from specify_cli._console import console from specify_cli.presets._commands import ( + _download_preset_archive, _warn_unmet_extension_dependencies, _write_preset_archive, ) @@ -14969,6 +14970,34 @@ def record_mkstemp(*args, **kwargs): assert not created["path"].exists() + def test_archive_download_uses_final_redirect_url_and_canonical_suffix( + self, monkeypatch + ): + """A .zip request redirected to a tarball must retain the final format.""" + archive = io.BytesIO() + with tarfile.open(fileobj=archive, mode="w:gz") as tar: + info = tarfile.TarInfo("preset.yml") + payload = b"schema_version: '1.0'\n" + info.size = len(payload) + tar.addfile(info, io.BytesIO(payload)) + + monkeypatch.setattr( + "specify_cli.presets._commands._fetch_preset_archive_data", + lambda url, error_type: ( + archive.getvalue(), + "application/gzip", + "https://cdn.example.test/preset.tgz", + ), + ) + + path = _download_preset_archive( + "https://example.test/preset.zip", PresetValidationError + ) + try: + assert path.suffixes[-2:] == [".tar", ".gz"] + finally: + path.unlink(missing_ok=True) + def test_cli_missing_id_directs_user_to_add(self, project_dir): from typer.testing import CliRunner from specify_cli import app From f7cb866f9dca31b01ed00e3686f9bf2985d93a3c Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 17:54:21 +0100 Subject: [PATCH 34/39] resolve remaining preset review findings Serialize preset updates with the existing project transaction lock, make resolver lookups alias-aware, preserve per-agent historical skill ownership during reconciliation, and make dry-run constitution output respect the real sync and hand-edit guards. Add regression coverage for alias resolution and command-only priority changes during dry-run updates. Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/__init__.py | 56 ++++++++++++++++++++++++++-- src/specify_cli/presets/_commands.py | 38 +++++++++++++------ tests/test_presets.py | 49 ++++++++++++++++++++++++ 3 files changed, 129 insertions(+), 14 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 7ac6e8aeb6..87fda9bec0 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -4204,6 +4204,7 @@ def _validate_update_source( diff = _diff_preset_template_files( diff, old_manifest, new_manifest, current_dir, source_dir ) + def convention_constitution(base_dir: Path) -> Optional[bytes]: for relative in ( "templates/constitution-template.md", @@ -4218,6 +4219,16 @@ def convention_constitution(base_dir: Path) -> Optional[bytes]: convention_constitution(current_dir) != convention_constitution(source_dir) ) + diff["_constitution_layer"] = ( + new_manifest.id == _CONSTITUTION_SYNC_PRESET_ID + or any( + item.get("type") == "template" + and item.get("name") == "constitution-template" + for item in old_manifest.templates + new_manifest.templates + ) + or convention_constitution(current_dir) is not None + or convention_constitution(source_dir) is not None + ) return old_manifest, new_manifest, diff def update_from_directory( @@ -4229,6 +4240,29 @@ def update_from_directory( priority: Optional[int] = None, dry_run: bool = False, expected_version: Optional[str] = None, + ) -> tuple[PresetManifest, Dict[str, List[Dict[str, Any]]]]: + """Update an installed preset while serializing its transaction.""" + from ..workflows._commands import _workflow_install_transaction + + with _workflow_install_transaction(self.project_root): + return self._update_from_directory_locked( + source_dir, + speckit_version, + pack_id=pack_id, + priority=priority, + dry_run=dry_run, + expected_version=expected_version, + ) + + def _update_from_directory_locked( + self, + source_dir: Path, + speckit_version: str, + *, + pack_id: Optional[str] = None, + priority: Optional[int] = None, + dry_run: bool = False, + expected_version: Optional[str] = None, ) -> tuple[PresetManifest, Dict[str, List[Dict[str, Any]]]]: """Update an installed preset from a validated directory.""" if priority is not None and priority < 1: @@ -4536,13 +4570,19 @@ def remove_swap_path(path: Path) -> None: ) extra_skill_dirs = {} if isinstance(registered_skills_before, dict): - for agent in registered_skills_before: + for agent, names in registered_skills_before.items(): if agent == active_agent: continue + if not isinstance(names, list): + continue skill_dir = self._resolve_agent_skills_dir(agent) extra_skill_dirs[skill_dir] = ( agent, - sorted(affected_skill_names), + sorted( + name + for name in names + if isinstance(name, str) + ), ) self._reconcile_skills( sorted(reconcile_command_names), @@ -5977,7 +6017,17 @@ def _manifest_declared_template( if not manifest: return None, None for tmpl in manifest.templates: - if tmpl.get("name") == template_name and tmpl.get("type") == template_type: + aliases = tmpl.get("aliases", []) + if ( + tmpl.get("type") == template_type + and ( + tmpl.get("name") == template_name + or ( + isinstance(aliases, list) + and template_name in aliases + ) + ) + ): file_path = tmpl.get("file") if file_path: manifest_candidate = pack_dir / file_path diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index 4d8637ccb0..e08a239fdb 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -315,7 +315,9 @@ def preset_add( PresetCompatibilityError, PresetError, PresetManager, + PresetResolver, PresetValidationError, + _constitution_is_generated, ) project_root = _require_specify_project() @@ -747,17 +749,31 @@ def preset_update( for category in ("added", "removed", "changed") for entry in diff[category] ) - constitution_status = ( - "constitution reconciliation pending" - if dry_run and priority is not None and not constitution_diff - else "constitution change planned" - if dry_run and constitution_diff - else "constitution unchanged" - if dry_run - else "constitution unchanged" - if constitution_before == constitution_after - else "constitution reconciled" - ) + if dry_run: + sync_metadata = manager.registry.get("constitution-sync") + constitution_can_reconcile = ( + bool(diff.get("_constitution_layer")) + and isinstance(sync_metadata, dict) + and sync_metadata.get("enabled", True) + and constitution_path.exists() + and _constitution_is_generated( + project_root, + constitution_path, + PresetResolver(project_root), + ) + ) + constitution_status = ( + "constitution change planned" + if constitution_can_reconcile + and (constitution_diff or priority is not None) + else "constitution unchanged" + ) + else: + constitution_status = ( + "constitution unchanged" + if constitution_before == constitution_after + else "constitution reconciled" + ) console.print( f"[green]✓[/green] {safe_id}: {action} to v{manifest.version} " f"(+{added_commands} commands, -{removed_commands} commands, " diff --git a/tests/test_presets.py b/tests/test_presets.py index 20a7d4703b..943e5d8739 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -5485,6 +5485,24 @@ def _create_multi_command_preset_with_aliases(self, temp_dir, preset_id, command yaml.dump(manifest_data, f) return preset_dir + def test_resolver_matches_manifest_aliases(self, project_dir, temp_dir): + preset_dir = self._create_multi_command_preset_with_aliases( + temp_dir, "alias-resolver-preset", [("speckit.primary", ["speckit.alias"])] + ) + manager = PresetManager(project_dir) + manager.install_from_directory(preset_dir, "0.1.0") + + resolver = PresetResolver(project_dir) + entry, candidate = resolver._manifest_declared_template( + manager.presets_dir / "alias-resolver-preset", + "speckit.alias", + "command", + ) + + assert entry is not None + assert candidate is not None + assert candidate.name == "speckit.primary.md" + def test_skill_overridden_on_preset_install(self, project_dir, temp_dir): """When skills mode was used, a preset command override should update the skill.""" # Simulate skills mode having been used: write init-options + create skill @@ -15040,6 +15058,37 @@ def test_cli_single_dry_run_does_not_prompt( assert "planned actions" in result.output assert "Update all installed presets?" not in result.output + def test_cli_dry_run_does_not_plan_constitution_for_command_only_priority_change( + self, project_dir, pack_dir, monkeypatch + ): + from typer.testing import CliRunner + from specify_cli import app + + manager = PresetManager(project_dir) + manager.install_from_directory(CONSTITUTION_SYNC_PRESET_DIR, "0.15.0") + manager.install_from_directory(pack_dir, "0.1.0") + source = self._updated_source(pack_dir) + monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir) + monkeypatch.setattr("specify_cli.get_speckit_version", lambda: "0.1.0") + + result = CliRunner().invoke( + app, + [ + "preset", + "update", + "test-pack", + "--dev", + str(source), + "--priority", + "5", + "--dry-run", + ], + ) + + assert result.exit_code == 0, result.output + assert "constitution unchanged" in result.output + assert "constitution reconciliation pending" not in result.output + def _second_pack(self, pack_dir, pack_id, version="1.0.0"): source = pack_dir.parent / pack_id shutil.copytree(pack_dir, source) From e63f25f776cee7c4e11975c270d0e23feb881725 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 18:17:57 +0100 Subject: [PATCH 35/39] Fix remaining preset update review findings Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/__init__.py | 15 +++++---- src/specify_cli/presets/_commands.py | 6 ++-- tests/test_presets.py | 48 ++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 9 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 87fda9bec0..b483d2285f 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -501,13 +501,14 @@ def _validate(self): # counted by PresetManifest.templates. Reject at validation time # instead, mirroring the sibling fix for ExtensionManifest's # provides.templates/scripts (#4016). - name_type = (tmpl["name"], tmpl["type"]) - if name_type in seen_name_types: - raise PresetValidationError( - f"Duplicate template name '{tmpl['name']}' of type " - f"'{tmpl['type']}' in 'provides.templates'" - ) - seen_name_types.add(name_type) + for declared_name in (tmpl["name"], *aliases): + name_type = (declared_name, tmpl["type"]) + if name_type in seen_name_types: + raise PresetValidationError( + f"Duplicate template name or alias '{declared_name}' " + f"of type '{tmpl['type']}' in 'provides.templates'" + ) + seen_name_types.add(name_type) # Validate file path safety: must be relative, no parent traversal file_path = tmpl["file"] diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index e08a239fdb..70911df820 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -557,7 +557,7 @@ def preset_update( f"'{pack_info.get('_catalog_name', 'catalog')}'" ) catalog_version = pkg_version.Version(str(pack_info["version"])) - if catalog_version <= installed_version: + if catalog_version <= installed_version and priority is None: console.print( f"[dim]• {safe_id}: Up to date, skipped " f"(v{installed_version})[/dim]" @@ -684,7 +684,9 @@ def preset_update( raise PresetError( f"catalog entry for preset '{item_id}' has an invalid version" ) from exc - if catalog_version <= installed_version: + if catalog_version < installed_version or ( + catalog_version == installed_version and priority is None + ): console.print( f"[dim]• {safe_id}: Up to date, skipped " f"(v{installed_version})[/dim]" diff --git a/tests/test_presets.py b/tests/test_presets.py index 943e5d8739..55b8a10ecf 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -530,6 +530,30 @@ def test_duplicate_template_name_and_type_raises_validation_error( with pytest.raises(PresetValidationError, match="Duplicate template name"): PresetManifest(manifest_path) + def test_duplicate_template_alias_and_type_raises_validation_error( + self, temp_dir, valid_pack_data + ): + """Aliases must not shadow another template's primary identity.""" + valid_pack_data["provides"]["templates"] = [ + { + "type": "command", + "name": "specify", + "file": "commands/specify.md", + "aliases": ["spec"], + }, + { + "type": "command", + "name": "other", + "file": "commands/other.md", + "aliases": ["spec"], + }, + ] + manifest_path = temp_dir / "preset.yml" + with open(manifest_path, "w") as f: + yaml.dump(valid_pack_data, f) + with pytest.raises(PresetValidationError, match="Duplicate template name or alias"): + PresetManifest(manifest_path) + def test_same_name_different_type_templates_allowed( self, temp_dir, valid_pack_data ): @@ -15383,6 +15407,30 @@ def test_cli_bulk_skips_when_installed_is_newer_than_catalogue( assert "Up to date, skipped" in result.output assert PresetManager(project_dir).registry.get("test-pack")["version"] == "5.0.0" + def test_cli_single_priority_update_runs_at_equal_catalogue_version( + self, project_dir, pack_dir, monkeypatch + ): + """An explicit single-item priority change is actionable at equal version.""" + from typer.testing import CliRunner + from specify_cli import app + + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.0", priority=7) + self._bulk_cli_env( + project_dir, + monkeypatch, + {"test-pack": {"version": "1.0.0", "bundled": True}}, + {"test-pack": pack_dir}, + ) + + result = CliRunner().invoke( + app, ["preset", "update", "test-pack", "--priority", "3"] + ) + + assert result.exit_code == 0, result.output + assert "updated" in result.output + assert PresetManager(project_dir).registry.get("test-pack")["priority"] == 3 + def test_cli_bulk_unresolvable_source_reports_clearly( self, project_dir, pack_dir, monkeypatch ): From d2d633e23c76eed2e78ff11336ddc273ed72ccf5 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 18:20:06 +0100 Subject: [PATCH 36/39] Fix preset update imports Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/_commands.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index 70911df820..c4406457c8 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -315,9 +315,7 @@ def preset_add( PresetCompatibilityError, PresetError, PresetManager, - PresetResolver, PresetValidationError, - _constitution_is_generated, ) project_root = _require_specify_project() @@ -494,7 +492,9 @@ def preset_update( PresetCompatibilityError, PresetError, PresetManager, + PresetResolver, PresetValidationError, + _constitution_is_generated, ) project_root = _require_specify_project() From 12f7219b11e9035ae508e7ba57ef50f025364ffb Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 18:27:04 +0100 Subject: [PATCH 37/39] Fix bulk preset update version skipping Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/_commands.py | 4 ++-- tests/test_presets.py | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index c4406457c8..af57a71dc9 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -557,7 +557,7 @@ def preset_update( f"'{pack_info.get('_catalog_name', 'catalog')}'" ) catalog_version = pkg_version.Version(str(pack_info["version"])) - if catalog_version <= installed_version and priority is None: + if catalog_version <= installed_version and effective_priority is None: console.print( f"[dim]• {safe_id}: Up to date, skipped " f"(v{installed_version})[/dim]" @@ -685,7 +685,7 @@ def preset_update( f"catalog entry for preset '{item_id}' has an invalid version" ) from exc if catalog_version < installed_version or ( - catalog_version == installed_version and priority is None + catalog_version == installed_version and effective_priority is None ): console.print( f"[dim]• {safe_id}: Up to date, skipped " diff --git a/tests/test_presets.py b/tests/test_presets.py index 55b8a10ecf..4b0f9197b7 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -15407,6 +15407,30 @@ def test_cli_bulk_skips_when_installed_is_newer_than_catalogue( assert "Up to date, skipped" in result.output assert PresetManager(project_dir).registry.get("test-pack")["version"] == "5.0.0" + def test_cli_bulk_priority_is_ignored_for_equal_catalogue_version( + self, project_dir, pack_dir, monkeypatch + ): + """Bulk priority remains ignored when the catalogue is already current.""" + from typer.testing import CliRunner + from specify_cli import app + + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.0", priority=7) + self._bulk_cli_env( + project_dir, + monkeypatch, + {"test-pack": {"version": "1.0.0", "bundled": True}}, + {"test-pack": pack_dir}, + ) + + result = CliRunner().invoke( + app, ["preset", "update", "--all", "--priority", "3"], input="y\n" + ) + + assert result.exit_code == 0, result.output + assert "Up to date, skipped" in result.output + assert PresetManager(project_dir).registry.get("test-pack")["priority"] == 7 + def test_cli_single_priority_update_runs_at_equal_catalogue_version( self, project_dir, pack_dir, monkeypatch ): From 9591066b7f167a9428fe2c328f1727dbe41f8711 Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 18:31:13 +0100 Subject: [PATCH 38/39] Harden preset update dry runs and registry refresh Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/__init__.py | 13 +++++++++++++ tests/test_presets.py | 16 ++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index b483d2285f..4f3c2f8718 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -4245,7 +4245,20 @@ def update_from_directory( """Update an installed preset while serializing its transaction.""" from ..workflows._commands import _workflow_install_transaction + if dry_run: + return self._update_from_directory_locked( + source_dir, + speckit_version, + pack_id=pack_id, + priority=priority, + dry_run=True, + expected_version=expected_version, + ) + with _workflow_install_transaction(self.project_root): + # Refresh after waiting for the lock so concurrent updates cannot + # overwrite newer registry metadata with this manager's snapshot. + self.registry.data = PresetRegistry(self.presets_dir).data return self._update_from_directory_locked( source_dir, speckit_version, diff --git a/tests/test_presets.py b/tests/test_presets.py index 4b0f9197b7..2f7cf7e16c 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -15113,6 +15113,22 @@ def test_cli_dry_run_does_not_plan_constitution_for_command_only_priority_change assert "constitution unchanged" in result.output assert "constitution reconciliation pending" not in result.output + def test_dry_run_does_not_create_transaction_lock( + self, project_dir, pack_dir + ): + """Dry-run validation must not leave a project lock behind.""" + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.0") + source = self._updated_source(pack_dir) + lock_path = project_dir / ".specify" / ".workflow-install.lock" + assert not lock_path.exists() + + manager.update_from_directory( + source, "0.1.0", pack_id="test-pack", dry_run=True + ) + + assert not lock_path.exists() + def _second_pack(self, pack_dir, pack_id, version="1.0.0"): source = pack_dir.parent / pack_id shutil.copytree(pack_dir, source) From e8a7950d57f998ff6e6f421a9a30d4c1791ed33b Mon Sep 17 00:00:00 2001 From: "Damien Hardy (digimangos)" <49808393+digimangos@users.noreply.github.com> Date: Fri, 4 Sep 2026 18:43:27 +0100 Subject: [PATCH 39/39] Recheck catalogue version under update lock Assisted-by: GitHub Copilot (model: gpt-5.6-luna, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a8e35d-943a-45f9-8afd-b94968b66064 --- src/specify_cli/presets/__init__.py | 17 +++++++++++++ tests/test_presets.py | 37 +++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 4f3c2f8718..fabdf44d1f 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -4259,6 +4259,23 @@ def update_from_directory( # Refresh after waiting for the lock so concurrent updates cannot # overwrite newer registry metadata with this manager's snapshot. self.registry.data = PresetRegistry(self.presets_dir).data + if expected_version is not None: + metadata = self.registry.get(pack_id or "") + installed_version = ( + metadata.get("version") if isinstance(metadata, dict) else None + ) + if isinstance(installed_version, str): + try: + installed = pkg_version.Version(installed_version) + catalogue = pkg_version.Version(expected_version) + except pkg_version.InvalidVersion: + pass + else: + if installed > catalogue and priority is None: + raise PresetCompatibilityError( + f"installed preset version {installed_version} is " + f"newer than catalogue version {expected_version}" + ) return self._update_from_directory_locked( source_dir, speckit_version, diff --git a/tests/test_presets.py b/tests/test_presets.py index 2f7cf7e16c..fb7c8d87d0 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -15526,3 +15526,40 @@ def racing_replace(src, dst): assert not list( (project_dir / ".specify" / "presets").glob(".test-pack.update-*") ) + + def test_catalogue_version_is_rechecked_after_lock( + self, project_dir, pack_dir, monkeypatch + ): + """A newer concurrent install must prevent a stale catalogue update.""" + from contextlib import contextmanager + + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.0") + source = self._updated_source(pack_dir, version="2.0.0") + + @contextmanager + def racing_transaction(_project_root): + concurrent = PresetManager(project_dir) + concurrent.registry.update("test-pack", {"version": "3.0.0"}) + yield + + monkeypatch.setattr( + "specify_cli.workflows._commands._workflow_install_transaction", + racing_transaction, + ) + + with pytest.raises(PresetCompatibilityError, match="newer than catalogue"): + manager.update_from_directory( + source, + "0.1.0", + pack_id="test-pack", + expected_version="2.0.0", + ) + + assert PresetManager(project_dir).registry.get("test-pack")["version"] == "3.0.0" + assert ( + PresetManifest( + project_dir / ".specify" / "presets" / "test-pack" / "preset.yml" + ).version + == "1.0.0" + )