From 978f6d50197297fc74972a20d470de0236e5186a Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 23 Oct 2026 17:26:48 -0500 Subject: [PATCH 1/4] feat(cli): add project uninstall with specify destroy Reuse integration teardown to remove generated commands, native event entries, and the last managed dispatcher before cleaning project artifacts. Add explicit constitution/specs preservation options, ownership-aware recovery, declared global-scope handling, and positive and negative lifecycle coverage. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/reference/core.md | 108 +- docs/reference/overview.md | 2 +- src/specify_cli/__init__.py | 2 + src/specify_cli/_operation_destroy.py | 569 +++++++++ src/specify_cli/agents.py | 67 ++ src/specify_cli/command_destroy.py | 176 +++ src/specify_cli/events/__init__.py | 40 + src/specify_cli/integrations/_lifecycle.py | 19 +- .../integrations/_operation_uninstall.py | 324 ++++++ .../integrations/command_uninstall.py | 179 +-- src/specify_cli/mcp_destroy.py | 23 + .../integrations/test_installed_adapters.py | 114 +- .../integrations/test_operation_uninstall.py | 66 ++ tests/specify_cli/test_command_destroy.py | 247 ++++ tests/specify_cli/test_command_init.py | 2 +- tests/specify_cli/test_operation_destroy.py | 1016 +++++++++++++++++ 16 files changed, 2778 insertions(+), 176 deletions(-) create mode 100644 src/specify_cli/_operation_destroy.py create mode 100644 src/specify_cli/command_destroy.py create mode 100644 src/specify_cli/integrations/_operation_uninstall.py create mode 100644 src/specify_cli/mcp_destroy.py create mode 100644 tests/specify_cli/integrations/test_operation_uninstall.py create mode 100644 tests/specify_cli/test_command_destroy.py create mode 100644 tests/specify_cli/test_operation_destroy.py diff --git a/docs/reference/core.md b/docs/reference/core.md index 82f976e3ed..c2d7ff1d98 100644 --- a/docs/reference/core.md +++ b/docs/reference/core.md @@ -1,6 +1,6 @@ # Core Commands -The core `specify` commands handle project initialization, system checks, and version information. +The core `specify` commands handle project initialization, project uninstall, system checks, and version information. ## Initialize a Project @@ -93,6 +93,112 @@ specify init my-project --integration copilot --preset compliance > cannot be symlinks. Unmigrated operations elsewhere in the project retain > their existing symlink behavior. +## Uninstall Spec Kit from a Project + +```bash +specify destroy --dry-run +specify destroy --yes +``` + +`destroy` uninstalls the project's integrations first, then removes its remaining +Spec Kit infrastructure, not the Specify CLI itself. It removes recorded integration +commands/skills, registered extension/preset artifacts, **all `.specify/` contents +including the constitution**, and the **project-local `specs/` directory** by default. +Project source and unrelated agent files remain untouched. Nonempty agent directories +are never removed wholesale. + +Use preservation flags to retain SDD artifacts while removing the tooling: + +```bash +specify destroy --yes --preserve-constitution +specify destroy --yes --preserve-specs +specify destroy --yes --preserve-all +# --preserve-all is equivalent to --preserve-constitution --preserve-specs +``` + +Preserved artifacts keep their original bytes, including when an integration manifest +tracks them. Both individual flags can be combined; combining either with +`--preserve-all` is harmless. `--force` never overrides preservation. + +| Option | Description | +| --- | --- | +| `--project ` | Explicit project root; otherwise use `SPECIFY_INIT_DIR` or the current directory | +| `--dry-run` | Preview file and directory cleanup, managed settings updates, and declared home-scoped outputs without modifying anything | +| `--yes`, `-y` | Explicitly confirm destructive cleanup; required for unattended use | +| `--force` | Also delete modified or unverified whole-owned agent artifacts | +| `--json` | Emit a result on stdout or one structured error on stderr; never prompt | +| `--preserve-constitution` | Keep `.specify/memory/constitution.md` and its parent directories | +| `--preserve-specs` | Keep project-local `specs/` without traversing or modifying it | +| `--preserve-all` | Keep both supported SDD artifact groups above; still remove integrations, tooling, and configuration | + +**Back up specifications and custom content first.** Confirmation authorizes +removing its templates, scripts, installed packages, workflows, caches, and +project-local configuration, including your custom files, constitution, and `specs/` +unless explicitly preserved. Preview the exact file and directory list before confirming. +Only the literal project-local `specs/` directory is removed: feature-directory +overrides, including `SPECIFY_FEATURE_DIRECTORY` and `.specify/feature.json`, do +not expand the deletion scope. A directory without a `.specify` project marker +is a no-op, even if it contains `specs/`. + +Modified hash-tracked generated files outside `.specify` stop cleanup before +deletion unless `--force` is explicit. Contribution cleanup uses the existing +integration lifecycle's registry, provenance-marker, and hash checks. A flat +skill name alone does not establish ownership of same-named skills in other +agents' directories. `--force` does **not** imply confirmation, establish +ownership, override filesystem policy, or authorize deleting shared settings. +For example: + +```bash +specify destroy --project ./my-project --dry-run --json +specify destroy --project ./my-project --yes --force +``` + +Non-default integrations are uninstalled first and the default last, without +switching defaults merely to remove the next integration. Their ordinary teardown +removes Spec Kit's merged native event entries while preserving user entries and +settings. The last installed integration referencing an owned `.specify/events.py` +dispatcher releases and removes it under the existing ownership and force rules. +If teardown preserves the dispatcher or leaves cleanup unresolved, destroy stops +and retains recovery metadata instead of purging the remaining `.specify` tree. + +Missing/corrupt ownership metadata and unsafe paths block cleanup. External +manifest paths and ownership modes must be authorized by the user's existing +local ownership binding; neither confirmation nor force replaces that proof. +Confirmed uninstall uses the normal trusted-adapter loading, teardown, recovery, +and rollback machinery. Dry-run validates metadata without loading adapter code. +Integration-specific uninstall behavior also applies to home-scoped outputs, +such as Hermes skills; those declared directories and registered contribution +files are shown separately in preview (`global_scopes` and `global_files` in +JSON). Only an installed integration's declared home scope can supply these +paths. They are removed by its normal teardown, not by project-tree deletion. +The CLI installation and machine-level trust records are not removed. +Untracked settings not owned by native event cleanup remain in place. + +Persisted symlink policy applies. Destroy never traverses symlinked directories +to identify agent artifacts; allowed leaf links are unlinked without deleting +their targets. An explicitly preserved constitution or `specs/` entry may be a link and is +left untouched. Otherwise the persisted symlink policy applies; targets are +never recursively traversed merely because a link is selected for deletion. +Metadata is always read without following links. + +Preview, rejected confirmation, and validation failures leave files unchanged. +An I/O failure during deletion can leave a partial uninstall and is reported +explicitly. External integration lifecycle transactions retain their normal +rollback behavior, but destroy as a whole is not atomic: earlier integrations +may already have been uninstalled when a later cleanup fails. Remaining +metadata is retained for recovery. Repeating destroy with the same preservation +flags succeeds without further changes. A retained constitution by itself is +a no-op with `--preserve-constitution` or `--preserve-all`; without those flags, +its deletion still requires confirmation. +If teardown raises after deleting its manifest, destroy restores the original +ownership manifest so the recorded integration can be retried after the error +is resolved. This preserves recovery evidence; it does not undo files that +were already removed outside a lifecycle transaction. + +For removing just one integration while retaining Spec Kit tooling, use +[`specify integration uninstall`](../integrations/cli.md#uninstall-an-integration). +`destroy` is currently CLI-only; its MCP inventory records it as unavailable. + ## Naming Features with the Helper Scripts When calling the bundled `create-new-feature` helper scripts directly, generated diff --git a/docs/reference/overview.md b/docs/reference/overview.md index d10ffc3251..1526bf823b 100644 --- a/docs/reference/overview.md +++ b/docs/reference/overview.md @@ -4,7 +4,7 @@ The Specify CLI (`specify`) manages the full lifecycle of Spec-Driven Developmen ## Core Commands -The foundational commands for creating and managing Spec Kit projects. Initialize a new project with the necessary directory structure, templates, and scripts. Verify that your system has the required tools installed. Check version and system information. +The foundational commands for creating and managing Spec Kit projects. Initialize a new project with the necessary directory structure, templates, and scripts. Uninstall with `specify destroy`, removing project-local tooling and SDD artifacts by default, or use `--preserve-all` to retain the constitution and feature specifications. Verify that your system has the required tools installed. Check version and system information. [Core Commands reference →](core.md) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 181b0feec1..4e518f718d 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -391,12 +391,14 @@ def _print_cli_warning( # ===== Root Commands ===== from . import command_check as _command_check # noqa: E402 +from . import command_destroy as _command_destroy from . import command_init as _command_init # noqa: E402 from . import command_mcp as _command_mcp # noqa: E402 from . import command_version as _command_version # noqa: E402 _command_init.register(app) _command_check.register(app) +_command_destroy.register(app) _command_version.register(app) _command_mcp.register(app) diff --git a/src/specify_cli/_operation_destroy.py b/src/specify_cli/_operation_destroy.py new file mode 100644 index 0000000000..b7dceecc41 --- /dev/null +++ b/src/specify_cli/_operation_destroy.py @@ -0,0 +1,569 @@ +"""Project-local uninstall planning and execution, independent of delivery adapters.""" + +from __future__ import annotations + +import errno +import hashlib +import json +import os +import re +import stat +from dataclasses import dataclass +from pathlib import Path, PureWindowsPath +from typing import Any + +from ._file_policy import project_files, read_project_options +from .file_helper import FileHelper, FileHelperError +from .integration_state import ( + default_integration_key, + installed_integration_keys, + normalize_integration_state, +) +from .integrations.manifest import IntegrationManifest + +CONSTITUTION = ".specify/memory/constitution.md" + + +@dataclass(frozen=True) +class DestroyRequest: + project_directory: Path + confirmed: bool = False + force: bool = False + dry_run: bool = False + preserve_constitution: bool = False + preserve_specs: bool = False + preserve_all: bool = False + + @property + def preserved_paths(self) -> tuple[str, ...]: + paths = [] + if self.preserve_constitution or self.preserve_all: + paths.append(CONSTITUTION) + if self.preserve_specs or self.preserve_all: + paths.append("specs") + return tuple(paths) + + +@dataclass(frozen=True) +class DestroyPlan: + project_directory: Path + files: tuple[str, ...] + directories: tuple[str, ...] + preserved: tuple[str, ...] + requires_force: tuple[str, ...] + integrations: tuple[str, ...] = () + updated: tuple[str, ...] = () + contribution_agents: tuple[str, ...] = () + global_scopes: tuple[str, ...] = () + global_files: tuple[str, ...] = () + + +@dataclass(frozen=True) +class DestroyResult: + project_directory: str + dry_run: bool + removed: tuple[str, ...] + preserved: tuple[str, ...] + requires_force: tuple[str, ...] = () + directories: tuple[str, ...] = () + updated: tuple[str, ...] = () + warnings: tuple[str, ...] = () + global_scopes: tuple[str, ...] = () + global_files: tuple[str, ...] = () + + +class DestroyError(ValueError): + """An expected uninstall failure, with any completed mutations made explicit.""" + + def __init__(self, code: str, message: str, **details: Any) -> None: + super().__init__(message) + self.code = code + self.message = message + self.details = details + self.retryable = False + + +def _relative(root: Path, path: Path, *, allow_specs: bool = False) -> str: + try: + relative = path.relative_to(root) + except ValueError as exc: + raise ValueError(f"Destroy cannot remove non-project path: {path}") from exc + text = relative.as_posix() + if ( + text in {"", "."} + or ".." in relative.parts + or "\\" in text + or PureWindowsPath(text).drive + or relative.parts[0].casefold() == ".git" + or ( + relative.parts[0].casefold() == "specs" + and not (allow_specs and relative.parts[0] == "specs") + ) + ): + raise ValueError(f"Unsafe destroy path: {text}") + return text + + +def _read_json(files: FileHelper, relative: str) -> dict[str, Any] | None: + try: + text = files.read_text(relative) + except FileNotFoundError: + return None + value = json.loads(text) + if not isinstance(value, dict): + raise TypeError(f"{relative} must contain a JSON object") + return value + + +def _tree( + files: FileHelper, relative: str, *, preserve_constitution: bool = False, +) -> tuple[set[str], set[str]]: + """Inventory a selected tree without traversing descendant symlinks.""" + leaves: set[str] = set() + directories: set[str] = set() + path = files.root / relative + try: + mode = path.lstat().st_mode + except FileNotFoundError: + return leaves, directories + # Validate all ancestors through the helper before inspecting the leaf. + parent = path.parent + while parent != files.root: + if parent.is_symlink(): + raise FileHelperError(f"Refusing destroy through symlinked directory: {parent}") + parent = parent.parent + + entry = path.lstat() + if ( + getattr(entry, "st_file_attributes", 0) & stat.FILE_ATTRIBUTE_REPARSE_POINT + and not stat.S_ISLNK(mode) + ): + raise FileHelperError(f"Unsupported reparse point: {relative}") + if stat.S_ISLNK(mode): + if preserve_constitution and relative == ".specify/memory": + raise FileHelperError("Cannot preserve constitution through symlinked .specify/memory") + if not files.allow_symlinks: + raise FileHelperError(f"Refusing symlink during destroy: {relative}") + leaves.add(relative) + elif stat.S_ISREG(mode): + files.read_bytes(relative) + leaves.add(relative) + elif stat.S_ISDIR(mode): + with os.scandir(path) as entries: + children = sorted(entry.name for entry in entries) + for name in children: + child = f"{relative}/{name}" + if preserve_constitution and child == CONSTITUTION: + if not (path / name).is_file() and not (path / name).is_symlink(): + raise ValueError(f"{CONSTITUTION} must be a file") + continue + child_files, child_dirs = _tree( + files, child, preserve_constitution=preserve_constitution, + ) + leaves.update(child_files) + directories.update(child_dirs) + directories.add(relative) + else: + raise FileHelperError(f"Unsupported filesystem object: {relative}") + return leaves, directories + + +def _project_artifacts( + files: FileHelper, request: DestroyRequest, +) -> tuple[set[str], set[str], tuple[str, ...]]: + selected = request.preserved_paths + leaves, directories = _tree( + files, ".specify", preserve_constitution=CONSTITUTION in selected, + ) + preserved = tuple( + name for name in selected + if (files.root / name).exists() or (files.root / name).is_symlink() + ) + if CONSTITUTION in preserved: + directories.difference_update({".specify", ".specify/memory"}) + if "specs" not in selected: + specs = files.root / "specs" + if specs.exists() and not specs.is_dir() and not specs.is_symlink(): + raise ValueError("specs must be a directory; refusing to remove a non-directory specs entry") + spec_files, spec_directories = _tree(files, "specs") + leaves.update(spec_files) + directories.update(spec_directories) + return leaves, directories, preserved + + +def _contribution_paths( + root: Path, metadata_files: FileHelper, files: FileHelper, bindings: dict[str, dict | None], + global_scopes: dict[str, Path], +) -> tuple[set[Path], dict[str, set[str]], set[str], set[Path]]: + from .agents import CommandRegistrar + from .integrations import BUILTIN_INTEGRATION_KEYS + + registrar = CommandRegistrar() + paths: set[Path] = set() + hashes: dict[str, set[str]] = {} + agents: set[str] = set() + global_paths: set[Path] = set() + active_generic = read_project_options(root).get("ai") == "generic" + for kind in ("extensions", "presets"): + relative = f".specify/{kind}/.registry" + registry = _read_json(metadata_files, relative) + if registry is None: + package_dir = root / ".specify" / kind + if package_dir.exists() and any( + child.is_dir() and not child.name.startswith(".") + for child in package_dir.iterdir() + ): + raise ValueError(f"Missing {relative}; cannot identify contributed agent artifacts") + continue + records = registry.get(kind) + if not isinstance(records, dict): + raise TypeError(f"{relative}: {kind} must be an object") + for identifier, record in records.items(): + if not isinstance(record, dict): + raise TypeError(f"Invalid {kind} record: {identifier}") + generic_hashes = record.get("generic_artifact_hashes", {}) + if not isinstance(generic_hashes, dict): + raise TypeError(f"Invalid generic_artifact_hashes for {identifier}") + for name, digest in generic_hashes.items(): + if not isinstance(digest, str) or not re.fullmatch(r"[0-9a-fA-F]{64}", digest): + raise ValueError(f"Invalid generic artifact digest for {identifier}: {name}") + relative = _relative(root, root / name) + hashes.setdefault(relative, set()).add(digest.lower()) + paths.add(root / name) + commands = record.get("registered_commands", {}) + skills = record.get("registered_skills", {}) + if not isinstance(commands, dict): + raise TypeError(f"Invalid registered_commands for {identifier}") + if kind == "extensions" and isinstance(skills, dict) and skills: + raise TypeError(f"Invalid registered_skills for extension {identifier}: expected a list") + if isinstance(skills, list): + # Legacy extension skills were mirrored to every detected agent. + names_by_agent = { + agent: skills for agent in BUILTIN_INTEGRATION_KEYS + if agent in registrar.AGENT_CONFIGS + } + for agent, binding in bindings.items(): + if binding and binding["registrar_config"]["extension"] == "/SKILL.md": + names_by_agent[agent] = skills + if "generic" in commands or active_generic: + names_by_agent["generic"] = skills + elif isinstance(skills, dict): + names_by_agent = skills + else: + raise TypeError(f"Invalid registered_skills for {identifier}") + for agent in commands.keys() | names_by_agent.keys(): + if agent == "generic" and generic_hashes: + agents.add(agent) + continue + cmd_names = commands.get(agent, []) + skill_names = names_by_agent.get(agent, []) + if not all( + isinstance(names, list) and all(isinstance(name, str) for name in names) + for names in (cmd_names, skill_names) + ): + raise ValueError(f"Invalid artifact names for {identifier}") + if not cmd_names and not skill_names: + continue + binding = bindings.get(agent) + config = binding["registrar_config"] if binding else None + command_paths = ( + registrar.registered_artifact_paths( + agent, root, commands=cmd_names, skills=[], registrar_config=config, + ) + ) + for path in command_paths: + if path.is_relative_to(root): + paths.add(path) + elif agent in global_scopes and path.is_relative_to(global_scopes[agent]): + global_paths.add(path) + else: + raise ValueError(f"Contribution output is outside its declared uninstall scope: {path}") + if cmd_names: + agents.add(agent) + for skill in skill_names: + candidates = registrar.registered_artifact_paths( + agent, root, commands=[], skills=[skill], registrar_config=config, + ) + for candidate in candidates: + relative = _relative(root, candidate / "SKILL.md") + try: + text = files.read_text(relative) + except FileNotFoundError: + if candidate.exists() and not isinstance(skills, list): + raise ValueError(f"Cannot establish skill ownership at {candidate}") + continue + frontmatter, _ = registrar.parse_frontmatter(text) + metadata = frontmatter.get("metadata", {}) if isinstance(frontmatter, dict) else {} + source = metadata.get("source") if isinstance(metadata, dict) else None + prefix = "extension" if kind == "extensions" else "preset" + if source == f"{prefix}:{identifier}": + paths.add(candidate) + agents.add(agent) + elif not isinstance(skills, list): + raise ValueError(f"Cannot establish skill ownership at {candidate}") + return paths, hashes, agents, global_paths + + +def plan_destroy(request: DestroyRequest) -> DestroyPlan: + """Validate all ownership and paths without changing the project.""" + root = request.project_directory + if not root.is_absolute(): + raise DestroyError("invalid_project_directory", "project_directory must be absolute") + try: + root = root.resolve(strict=True) + if (Path.home().resolve() / ".specify/integration-trust.json").is_relative_to(root): + raise ValueError("Destroy cannot target a project containing machine-level integration trust") + files = project_files(root) + metadata_files = FileHelper(root) + specify = root / ".specify" + if not specify.exists() and not specify.is_symlink(): + return DestroyPlan(root, (), (), (), ()) + if specify.is_symlink() or not specify.is_dir(): + raise ValueError(".specify must be a non-symlinked directory") + + leaves, directories, preserved = _project_artifacts(files, request) + + state = _read_json(metadata_files, ".specify/integration.json") + if state and state.get("integration_state_schema", 1) != 1: + raise ValueError("Unsupported integration state schema") + if state is not None: + if not {"integration", "default_integration", "installed_integrations"} & state.keys(): + raise ValueError("Integration state has no installation inventory") + for field in ("integration", "default_integration"): + if field in state and ( + not isinstance(state[field], str) or not state[field].strip() + ): + raise ValueError(f"Invalid integration state field: {field}") + if "installed_integrations" in state and ( + not isinstance(state["installed_integrations"], list) + or any( + not isinstance(key, str) or not key.strip() + for key in state["installed_integrations"] + ) + ): + raise ValueError("Invalid installed_integrations") + installed = installed_integration_keys(normalize_integration_state(state)) if state else [] + if state is None: + legacy_agent = read_project_options(root).get("ai") + if isinstance(legacy_agent, str) and legacy_agent: + installed = [legacy_agent] + manifests = sorted( + name for name in leaves + if name.startswith(".specify/integrations/") and name.endswith(".manifest.json") + and len(Path(name).parts) == 3 + ) + from .integrations._operation_uninstall import validate_uninstall_ownership + + bindings = {key: validate_uninstall_ownership(root, key) for key in installed} + for key in installed: + if f".specify/integrations/{key}.manifest.json" not in manifests: + raise ValueError(f"Missing manifest for '{key}'; repair or uninstall that integration first") + + expected: dict[str, set[str]] = {} + updated: set[str] = set() + for manifest_path in manifests: + key = Path(manifest_path).name.removesuffix(".manifest.json") + raw = _read_json(metadata_files, manifest_path) + if raw is None or "files" not in raw or raw.get("integration") != key: + raise ValueError(f"Invalid ownership manifest: {manifest_path}") + manifest = IntegrationManifest.load(key, root) + if key != "speckit" and key not in installed: + raise ValueError(f"Integration '{key}' has a manifest but no installed integration state") + for name, digest in manifest.files.items(): + if Path(name).as_posix() != name: + raise ValueError(f"Noncanonical manifest path: {name}") + relative = _relative(root, root / name, allow_specs=True) + if relative == CONSTITUTION or relative == "specs" or relative.startswith("specs/"): + # SDD artifacts are handled only by the final project cleanup, not agent teardown. + continue + if relative == ".specify": + raise ValueError("Manifests must record files, not the .specify root") + if not re.fullmatch(r"[0-9a-fA-F]{64}", digest): + raise ValueError(f"Invalid manifest digest: {name}") + if relative.startswith(".specify/"): + continue + mode = manifest.ownership_modes[name] + if bindings.get(key) is not None: + mode = bindings[key].get("ownership_modes", {}).get(name, "shared") + if mode != "whole": + if (root / name).exists() or (root / name).is_symlink(): + from .integrations import ( + BUILTIN_INTEGRATION_KEYS, + get_integration, + ) + + integration = get_integration(key) if key in BUILTIN_INTEGRATION_KEYS else None + if integration is not None and getattr(integration, "events_config_file", None) != name: + raise ValueError( + f"Partial/shared ownership at {name}; no native event cleanup owns this file", + ) + metadata_files.read_bytes(name) + updated.add(name) + continue + expected.setdefault(relative, set()).add(digest.lower()) + + from .integrations import BUILTIN_INTEGRATION_KEYS, get_integration + from .integrations.installer import safe_project_path + + global_scopes: dict[str, Path] = {} + for key in installed: + integration = get_integration(key) if key in BUILTIN_INTEGRATION_KEYS else None + directory = (integration.registrar_config or {}).get("dir", "") if integration else "" + if directory.startswith("~/"): + global_scopes[key] = safe_project_path(Path.home(), directory[2:]) + contributed, contribution_hashes, contribution_agents, global_paths = _contribution_paths( + root, metadata_files, files, bindings, global_scopes, + ) + global_files: set[str] = set() + if global_paths: + home_files = FileHelper(Path.home(), allow_symlinks=files.allow_symlinks) + for path in sorted(global_paths): + relative = _relative(home_files.root, path) + # Declared global outputs are inspected here, but only their integration tears them down. + candidate_leaves, _ = _tree(home_files, relative) + global_files.update(str(home_files.root / name) for name in candidate_leaves) + for name, digests in contribution_hashes.items(): + expected.setdefault(name, set()).update(digests) + requiring_force: set[str] = set() + for candidate in sorted(contributed | {root / name for name in expected}): + relative = _relative(root, candidate) + if relative == CONSTITUTION or relative.startswith(".specify/"): + continue + candidate_leaves, candidate_dirs = _tree(files, relative) + if candidate_dirs and relative in expected: + raise ValueError(f"Manifest file became a directory: {relative}") + leaves.update(candidate_leaves) + directories.update(candidate_dirs) + for name in candidate_leaves: + path = root / name + digests = expected.get(name, set()) + if digests and ( + path.is_symlink() or hashlib.sha256(files.read_bytes(name)).hexdigest() not in digests + ): + requiring_force.add(name) + + # Only prune empty generated ancestors; never traverse unrelated trees. + for name in tuple(leaves) + tuple(directories): + parent = Path(name).parent + while parent != Path("."): + if parent.as_posix() not in {".specify", ".specify/memory"} or CONSTITUTION not in preserved: + directories.add(parent.as_posix()) + parent = parent.parent + normalized = normalize_integration_state(state or {}) + default = default_integration_key(normalized) + order = tuple(key for key in installed if key != default) + ((default,) if default in installed else ()) + return DestroyPlan( + root, tuple(sorted(leaves)), + tuple(sorted(directories, key=lambda name: (-len(Path(name).parts), name))), + preserved, tuple(sorted(requiring_force)), order, tuple(sorted(updated)), + tuple(sorted(contribution_agents)), + tuple(sorted(str(path) for path in global_scopes.values())), + tuple(sorted(global_files)), + ) + except (OSError, ValueError, TypeError, RuntimeError) as exc: + raise DestroyError("invalid_destroy_state", str(exc), project_directory=str(root)) from exc + + +def destroy_project(request: DestroyRequest) -> DestroyResult: + """Plan and execute a confirmed project-local uninstall.""" + plan = plan_destroy(request) + if request.dry_run: + return DestroyResult( + str(plan.project_directory), True, plan.files, plan.preserved, plan.requires_force, + plan.directories, plan.updated, global_scopes=plan.global_scopes, + global_files=plan.global_files, + ) + if not plan.files and not plan.directories and not plan.integrations and not plan.updated: + return DestroyResult(str(plan.project_directory), False, (), plan.preserved) + if not request.confirmed: + raise DestroyError( + "confirmation_required", + "Destroy deletes project-local Spec Kit files and unpreserved SDD artifacts. " + "Preview with --dry-run; confirm with --yes.", + ) + if plan.requires_force and not request.force: + raise DestroyError( + "modified_files", + "Modified or unverified generated files would be deleted. Back them up or retry with --force.", + paths=list(plan.requires_force), + ) + files = project_files(plan.project_directory) + removed: list[str] = [] + warnings: list[str] = [] + uninstalled: list[str] = [] + try: + from .integrations._operation_uninstall import ( + IntegrationUninstallRequest, + uninstall_integration, + unregister_integration_artifacts, + ) + + for key in plan.integrations: + result = uninstall_integration(IntegrationUninstallRequest( + plan.project_directory, key, request.force, require_complete=True, + preserved_paths=(CONSTITUTION, "specs"), + )) + warnings.extend(result.warnings) + uninstalled.append(key) + for key in set(plan.contribution_agents) - set(plan.integrations): + warnings.extend(unregister_integration_artifacts( + plan.project_directory, key, require_complete=True, + )) + remaining = [ + name for name in plan.files + if not name.startswith((".specify/", "specs/")) and name != "specs" + and ((plan.project_directory / name).exists() or (plan.project_directory / name).is_symlink()) + ] + remaining.extend( + name for name in plan.global_files if Path(name).exists() or Path(name).is_symlink() + ) + if remaining: + raise DestroyError( + "cleanup_incomplete", "Integration uninstall preserved generated artifacts; resolve them before destroy.", + paths=remaining, + ) + if (plan.project_directory / ".specify/events.py").exists(): + raise DestroyError("cleanup_incomplete", "The event lifecycle preserved .specify/events.py; resolve it before destroy.") + # Re-inventory only after teardown has released all integration claims. + leaves, directories, _ = _project_artifacts(files, request) + for name in sorted(leaves, key=lambda name: (name.startswith(".specify/"), name)): + files.delete(name) + removed.append(name) + directories.update(plan.directories) + if CONSTITUTION in plan.preserved: + directories.difference_update({".specify", ".specify/memory"}) + for name in sorted(directories, key=lambda name: (-len(Path(name).parts), name)): + try: + files.delete(name) + except FileNotFoundError: + # Integration teardown already prunes its empty parent directories. + continue + except OSError as exc: + if ( + exc.errno not in {errno.ENOTEMPTY, errno.EEXIST} + or name == ".specify" + or name.startswith(".specify/") + ): + raise + except DestroyError as exc: + exc.details["uninstalled_integrations"] = uninstalled + raise + except (OSError, ValueError, RuntimeError) as exc: + raise DestroyError( + "destroy_failed", + f"Uninstall stopped: {exc}. Some files may already have been removed.", + removed=removed, + uninstalled_integrations=uninstalled, + ) from exc + removed.extend( + name for name in plan.files if name not in removed + and not (plan.project_directory / name).exists() + and not (plan.project_directory / name).is_symlink() + ) + return DestroyResult( + str(plan.project_directory), False, tuple(removed), plan.preserved, + updated=plan.updated, warnings=tuple(warnings), global_scopes=plan.global_scopes, + global_files=tuple( + name for name in plan.global_files if not Path(name).exists() and not Path(name).is_symlink() + ), + ) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 72966998a5..47944113de 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -1369,6 +1369,73 @@ def register_commands_for_non_skill_agents( continue return results + def registered_artifact_paths( + self, + agent_name: str, + project_root: Path, + *, + commands: list[str], + skills: list[str], + registrar_config: dict[str, Any] | None = None, + ) -> set[Path]: + """Enumerate recorded contribution outputs without loading adapter packages. + + The caller validates ownership and filesystem policy before using these + paths. Supplemental skill directories include their copied resources; + native command registrations own only their generated command file. + """ + from . import _get_skills_dir + from .integrations import BUILTIN_INTEGRATION_KEYS + + if agent_name not in BUILTIN_INTEGRATION_KEYS and registrar_config is None: + raise ValueError( + f"Uninstall contributed artifacts for external integration '{agent_name}' " + "before destroy; executable adapter layouts are not loaded during cleanup" + ) + + if registrar_config is not None: + config = registrar_config + elif agent_name == "generic": + from .integrations.generic import registration_directory + + config = { + "dir": str(registration_directory(project_root)), + "extension": ".md", + } + else: + config = self.AGENT_CONFIGS.get(agent_name) + if config is None: + raise ValueError(f"No registration layout for integration '{agent_name}'") + + for name in [*commands, *skills]: + if not name or name in {".", ".."} or not self._is_safe_command_name(name): + raise ValueError(f"Invalid registered artifact name: {name!r}") + + canonical_config = dict(config) + legacy = canonical_config.pop("legacy_dir", None) + destinations = [self._resolve_agent_dir(agent_name, canonical_config, project_root)] + if legacy: + destinations.append(project_root / legacy) + paths: set[Path] = set() + for name in commands: + output_name = self._compute_output_name(agent_name, name, config) + for destination in destinations: + if config["extension"] == "/SKILL.md": + paths.add(destination / output_name / "SKILL.md") + else: + paths.add(destination / f"{output_name}{config['extension']}") + if agent_name == "copilot": + paths.add(project_root / ".github/prompts" / f"{name}.prompt.md") + if skills: + if agent_name not in BUILTIN_INTEGRATION_KEYS: + if config["extension"] != "/SKILL.md": + raise ValueError(f"Cannot establish external supplemental skill ownership for '{agent_name}'") + skills_dir = project_root / config["dir"] + else: + skills_dir = _get_skills_dir(project_root, agent_name) + paths.update(skills_dir / name for name in skills) + return paths + def unregister_commands( self, registered_commands: Dict[str, List[str]], project_root: Path ) -> None: diff --git a/src/specify_cli/command_destroy.py b/src/specify_cli/command_destroy.py new file mode 100644 index 0000000000..fa2283b4f1 --- /dev/null +++ b/src/specify_cli/command_destroy.py @@ -0,0 +1,176 @@ +"""CLI adapter for ``specify destroy``.""" + +from __future__ import annotations + +import json +import logging +import os +import sys +import warnings +from contextlib import redirect_stderr, redirect_stdout +from dataclasses import asdict, replace +from io import StringIO +from pathlib import Path +from typing import Annotated + +import typer +from rich.markup import escape +from typer.core import TyperCommand + +from ._console import console +from ._operation_destroy import ( + DestroyError, + DestroyRequest, + DestroyResult, + destroy_project, +) + +try: + from typer._click.exceptions import UsageError as _UsageError +except ModuleNotFoundError as error: + if error.name != "typer._click": + raise + from click import UsageError as _UsageError + +logger = logging.getLogger(__name__) + + +def _emit_json_error(code: str, message: str, details: dict[str, object]) -> None: + typer.echo(json.dumps({"error": { + "code": code, "message": message, "details": details, + }}), err=True) + + +class _DestroyCommand(TyperCommand): + """Keep argument parsing failures on the destroy JSON contract.""" + + def make_context(self, info_name, args, parent=None, **extra): + json_output = "--json" in args + try: + return super().make_context(info_name, args, parent=parent, **extra) + except _UsageError as exc: + if json_output: + _emit_json_error("invalid_arguments", exc.format_message(), {}) + raise typer.Exit(exc.exit_code) from exc + raise + + +def _stdin_is_interactive() -> bool: + return sys.stdin.isatty() + + +def _invoke_operation(request: DestroyRequest, *, json_output: bool) -> DestroyResult: + if not json_output: + return destroy_project(request) + diagnostics = StringIO() + with ( + redirect_stdout(diagnostics), redirect_stderr(diagnostics), + console.capture() as captured, + warnings.catch_warnings(record=True) as recorded, + ): + warnings.simplefilter("always") + result = destroy_project(request) + messages = tuple(str(warning.message) for warning in recorded) + for text in (captured.get().strip(), diagnostics.getvalue().strip()): + if text: + messages += (text,) + return replace(result, warnings=result.warnings + messages) + + +def destroy( + project: Annotated[Path | None, typer.Option( + "--project", help="Project root (default: cwd or SPECIFY_INIT_DIR).", + )] = None, + dry_run: bool = typer.Option(False, "--dry-run", help="Preview without changing files."), + yes: bool = typer.Option(False, "--yes", "-y", help="Confirm deletion of project-local Spec Kit files."), + force: bool = typer.Option(False, "--force", help="Also remove modified/unverified whole-owned agent files."), + json_output: bool = typer.Option(False, "--json", help="Emit a JSON result or error; never prompt."), + preserve_constitution: bool = typer.Option( + False, "--preserve-constitution", help="Keep .specify/memory/constitution.md unchanged.", + ), + preserve_specs: bool = typer.Option( + False, "--preserve-specs", help="Keep the project-local specs/ directory unchanged.", + ), + preserve_all: bool = typer.Option( + False, "--preserve-all", help="Keep both the constitution and specs; still uninstall tooling.", + ), +) -> None: + """Uninstall Spec Kit and remove .specify/ and specs/ unless explicitly preserved.""" + try: + root = (project or Path(os.environ.get("SPECIFY_INIT_DIR") or Path.cwd())).absolute() + request = DestroyRequest( + root, confirmed=yes, force=force, dry_run=True, + preserve_constitution=preserve_constitution, preserve_specs=preserve_specs, + preserve_all=preserve_all, + ) + preview = _invoke_operation(request, json_output=json_output) + if not json_output: + console.print(f"Project: {escape(preview.project_directory)}") + console.print("Remove unpreserved .specify/ contents, specs/, and recorded agent artifacts:") + if "specs" not in request.preserved_paths and any( + name == "specs" or name.startswith("specs/") + for name in (*preview.removed, *preview.directories) + ): + console.print("[yellow]Warning:[/yellow] Project specifications in specs/ will be deleted.") + for name in preview.removed: + console.print(f" {escape(name)}") + for name in preview.preserved: + console.print(f"Preserve: {escape(name)}") + for name in preview.updated: + console.print(f"Remove managed integration entries from: {escape(name)}") + for name in preview.global_scopes: + console.print(f"Integration uninstall also manages home-scoped outputs in: {escape(name)}") + for name in preview.global_files: + console.print(f" {escape(name)}") + if not preview.removed and preview.directories: + console.print("Remove empty Spec Kit directories.") + if preview.requires_force: + console.print("[yellow]Modified/unverified files require --force:[/yellow]") + for name in preview.requires_force: + console.print(f" {escape(name)}") + if dry_run: + result = preview + else: + confirmed = yes + if ( + (preview.removed or preview.directories or preview.updated) + and not yes and not json_output and _stdin_is_interactive() + ): + confirmed = typer.confirm( + "Delete the planned files and directories? " + "Unpreserved content in .specify/ and specs/ will be lost.", + default=False, + ) + if not confirmed: + console.print("Uninstall cancelled; no files changed.") + return + result = _invoke_operation( + replace(request, confirmed=confirmed, dry_run=False), json_output=json_output, + ) + if json_output: + typer.echo(json.dumps(asdict(result), indent=2)) + elif dry_run: + console.print("Dry run; no files changed.") + else: + for warning in result.warnings: + console.print(f"[yellow]Warning:[/yellow] {escape(warning)}") + console.print("[green]Spec Kit project uninstall complete.[/green]") + except DestroyError as exc: + if json_output: + _emit_json_error(exc.code, exc.message, exc.details) + else: + console.print(f"[red]Error:[/red] {escape(exc.message)}") + raise typer.Exit(1) from exc + except Exception as exc: + logger.debug("Unexpected destroy failure", exc_info=True) + message = "Unable to uninstall project-local Spec Kit." + if json_output: + _emit_json_error("internal_error", message, {}) + else: + console.print(f"[red]Error:[/red] {message}") + raise typer.Exit(1) from exc + + +def register(app: typer.Typer) -> None: + """Register the root uninstall command.""" + app.command(cls=_DestroyCommand)(destroy) diff --git a/src/specify_cli/events/__init__.py b/src/specify_cli/events/__init__.py index d8007b9f76..c1676e6944 100644 --- a/src/specify_cli/events/__init__.py +++ b/src/specify_cli/events/__init__.py @@ -1748,6 +1748,46 @@ def _cleanup_shared_dispatcher( _unlink_file(dispatcher_path, missing_ok=True) +def validate_integration_event_cleanup( + integration: IntegrationBase, project_root: Path, *, require_clean: bool = False, +) -> None: + """Verify that a claimed native config is readable and, after teardown, clean.""" + config_file = getattr(integration, "events_config_file", None) + if not config_file: + return + path = project_root / config_file + if not path.exists(): + return + _ensure_safe_destination(path) + fmt = getattr(integration, "events_format", None) + if fmt in {"toml", "toml-vibe"}: + import tomllib + + data = tomllib.loads(path.read_text(encoding="utf-8")) + else: + data = _load_user_json(path) + if data is None: + raise ValueError(f"Cannot verify native event cleanup for {config_file}") + + def marked(value: Any) -> bool: + if isinstance(value, dict): + return ( + value.get(_SPECKIT_MARKER) is True + or value.get("speckit_marker") is True + or any(marked(child) for child in value.values()) + ) + if isinstance(value, list): + return any(marked(child) for child in value) + return False + + plugin_ref = "./.opencode/plugin/speckit-events.ts" + plugins = data.get("plugin", []) + if require_clean and ( + marked(data) or (fmt == "ts-plugin" and isinstance(plugins, list) and plugin_ref in plugins) + ): + raise ValueError(f"Spec Kit event entries remain in {config_file}") + + def remove_integration_events( integration: IntegrationBase, project_root: Path, manifest: IntegrationManifest, *, diff --git a/src/specify_cli/integrations/_lifecycle.py b/src/specify_cli/integrations/_lifecycle.py index ae26eabc5f..9f2c0f2e8e 100644 --- a/src/specify_cli/integrations/_lifecycle.py +++ b/src/specify_cli/integrations/_lifecycle.py @@ -444,7 +444,7 @@ def invoke(*args, **kwargs): try: installer.load_installed_integrations(root) except installer.IntegrationInstallError: - if operation not in {"upgrade", "uninstall"} or not values.get("force") or key not in records: + if operation != "upgrade" or not values.get("force") or key not in records: raise recovery_token = installer.recovery_exclusion.set((root.resolve(), key)) installer.load_installed_integrations(root) @@ -552,23 +552,6 @@ def invoke(*args, **kwargs): installer.unload_installed_integrations() installer.persist_package(root, key, *candidate) for removed in records.keys() - set(remaining): - # The uninstall command cleans up its own key. A stale - # record's contributions must go here, while its adapter - # is still loaded. - if operation == "uninstall" and removed != key: - from ._helpers import ( - _unregister_extensions_for_agent, - _unregister_presets_for_agent, - ) - - _unregister_extensions_for_agent( - root, removed, - continuing="The adapter was removed, but extension artifacts may need manual cleanup.", - ) - _unregister_presets_for_agent( - root, removed, - continuing="The adapter was removed, but preset artifacts may need manual cleanup.", - ) installer.remove_package(root, removed) if not candidate: installer.load_installed_integrations(root) diff --git a/src/specify_cli/integrations/_operation_uninstall.py b/src/specify_cli/integrations/_operation_uninstall.py new file mode 100644 index 0000000000..a53857f321 --- /dev/null +++ b/src/specify_cli/integrations/_operation_uninstall.py @@ -0,0 +1,324 @@ +"""Shared integration uninstall operation used by CLI and project destruction.""" + +from __future__ import annotations + +from contextlib import contextmanager, nullcontext +from dataclasses import dataclass, replace +from pathlib import Path + +from ..file_helper import FileHelper +from ..integration_state import ( + default_integration_key, + installed_integration_keys, + integration_settings, + try_read_integration_json, +) +from . import BUILTIN_INTEGRATION_KEYS, get_integration, installer +from ._helpers import _write_integration_json +from .manifest import IntegrationManifest + + +@dataclass(frozen=True) +class IntegrationUninstallRequest: + project_directory: Path + key: str | None = None + force: bool = False + require_complete: bool = False + preserved_paths: tuple[str, ...] = () + + +@dataclass(frozen=True) +class IntegrationUninstallResult: + key: str | None + name: str + removed: tuple[Path, ...] = () + skipped: tuple[Path, ...] = () + warnings: tuple[str, ...] = () + missing_manifest: bool = False + + +class IntegrationUninstallError(ValueError): + def __init__(self, code: str, message: str) -> None: + super().__init__(message) + self.code = code + self.message = message + self.details: dict[str, object] = {} + self.retryable = False + + +def validate_uninstall_ownership(root: Path, key: str) -> dict | None: + """External cleanup authority comes from the user's existing local binding.""" + if key in BUILTIN_INTEGRATION_KEYS: + return None + record = installer.read_records(root).get(key) + if record is None: + raise IntegrationUninstallError( + "unproven_ownership", f"No installed package ownership record for integration '{key}'", + ) + binding = installer.recovery_metadata(root, key, record) + if binding is None: + raise IntegrationUninstallError( + "unproven_ownership", f"No trusted local cleanup ownership for integration '{key}'", + ) + return binding + + +def unregister_integration_artifacts(root: Path, key: str, *, require_complete: bool) -> tuple[str, ...]: + from ..extensions import ExtensionManager + from ..presets import PresetManager + + warnings: list[str] = [] + for label, manager in (("extension", ExtensionManager), ("preset", PresetManager)): + try: + manager(root).unregister_agent_artifacts(key) + except Exception as exc: + message = f"Could not clean up {label} artifacts for integration '{key}': {exc}" + if require_complete: + raise IntegrationUninstallError("cleanup_incomplete", message) from exc + warnings.append(message) + return tuple(warnings) + + +@contextmanager +def _retain_recovery_manifest(request: IntegrationUninstallRequest, key: str | None): + """Strict cleanup keeps its original ownership evidence even if teardown raises.""" + if not request.require_complete or key is None: + yield + return + relative = f".specify/integrations/{key}.manifest.json" + files = FileHelper(request.project_directory) + try: + original = files.read_bytes(relative) + except FileNotFoundError: + yield + return + try: + yield + except Exception: + from ._file_changes import changing_file + + with changing_file(request.project_directory / relative): + files.mkdir(".specify/integrations", parents=True, exist_ok=True) + files.write_bytes(relative, original) + raise + + +def _uninstall( + request: IntegrationUninstallRequest, warnings: list[str], +) -> IntegrationUninstallResult: + from ._helpers import ( + _clear_init_options_for_integration, + _register_extensions_for_agent, + _register_presets_for_agent, + _remove_integration_json, + _resolve_integration_options, + _set_default_integration, + ) + + root = request.project_directory + state, error = try_read_integration_json(root) + if error: + raise IntegrationUninstallError("invalid_state", f"Cannot read integration state: {error.detail}") + state = state or {} + default = default_integration_key(state) + installed = installed_integration_keys(state) + key = request.key or default + if key is None: + return IntegrationUninstallResult(None, "") + if key not in installed: + raise IntegrationUninstallError("not_installed", f"Integration '{key}' is not installed.") + integration = get_integration(key) + manifest_path = root / ".specify/integrations" / f"{key}.manifest.json" + missing = not manifest_path.exists() + removed: list[Path] = [] + skipped: list[Path] = [] + manifest = None + recovery_manifest = None + if missing: + if request.require_complete: + raise IntegrationUninstallError("missing_manifest", f"Missing manifest for integration '{key}'") + else: + try: + manifest = IntegrationManifest.load(key, root) + if request.require_complete: + recovery_manifest = IntegrationManifest.load(key, root) + for preserved in request.preserved_paths: + for name in manifest.files: + if Path(name).is_relative_to(preserved): + manifest.remove(name) + except (ValueError, OSError) as exc: + raise IntegrationUninstallError( + "unreadable_manifest", f"Integration manifest for '{key}' is unreadable: {manifest_path}: {exc}", + ) from exc + if integration is None: + warnings.append(f"Integration '{key}' not found in registry. Falling back to manifest-based cleanup.") + removed, skipped = manifest.uninstall(root, force=request.force) + else: + native_config = getattr(integration, "events_config_file", None) + verify_events = request.require_complete and native_config in manifest.files + if verify_events: + from ..events import validate_integration_event_cleanup + + try: + validate_integration_event_cleanup(integration, root) + except (OSError, ValueError) as exc: + raise IntegrationUninstallError("cleanup_incomplete", str(exc)) from exc + removed, skipped = integration.teardown(root, manifest, force=request.force) + + if ( + integration is not None and request.require_complete and recovery_manifest is not None + and getattr(integration, "events_config_file", None) in recovery_manifest.files + ): + from ..events import validate_integration_event_cleanup + + try: + validate_integration_event_cleanup(integration, root, require_clean=True) + except (OSError, ValueError) as exc: + raise IntegrationUninstallError("cleanup_incomplete", str(exc)) from exc + warnings.extend(unregister_integration_artifacts( + root, key, require_complete=request.require_complete, + )) + if request.require_complete and skipped: + raise IntegrationUninstallError( + "cleanup_incomplete", + f"Integration '{key}' preserved files that require attention: " + + ", ".join(str(path) for path in skipped), + ) + if request.require_complete and (root / ".specify/events.py").exists(): + others = [installed_key for installed_key in installed if installed_key != key] + if not others: + raise IntegrationUninstallError( + "cleanup_incomplete", "The event lifecycle preserved .specify/events.py; resolve it before destroy.", + ) + + remaining = [installed_key for installed_key in installed if installed_key != key] + new_default = default if default != key else (remaining[0] if remaining else None) + if remaining: + fallback = get_integration(new_default) if new_default else None + if default == key and fallback is not None: + raw, parsed = _resolve_integration_options(fallback, state, new_default, None) + _set_default_integration( + root, state, new_default, fallback, remaining, raw_options=raw, parsed_options=parsed, + ) + _register_extensions_for_agent( + root, new_default, continuing="The fallback integration was selected; check extension registration.", + ) + _register_presets_for_agent( + root, new_default, continuing="The fallback integration was selected; check preset registration.", + ) + else: + _write_integration_json(root, new_default, remaining, integration_settings(state)) + else: + _remove_integration_json(root) + if default == key: + _clear_init_options_for_integration(root, key) + name = (integration.config or {}).get("name", key) if integration else key + return IntegrationUninstallResult(key, name, tuple(removed), tuple(skipped), missing_manifest=missing) + + +def _uninstall_with_lifecycle(request: IntegrationUninstallRequest) -> IntegrationUninstallResult: + """Run the established teardown, recovery, package, and metadata lifecycle.""" + from ._lifecycle import _transaction + + root = request.project_directory + if not root.is_absolute(): + raise IntegrationUninstallError("invalid_project_directory", "project_directory must be absolute") + from .._file_policy import project_files + + project_files(root) + if not (root / ".specify").is_dir(): + raise IntegrationUninstallError("not_a_spec_kit_project", "Not a Spec Kit project: no .specify directory") + records = installer.read_records(root) + state, error = try_read_integration_json(root) + if error: + raise IntegrationUninstallError("invalid_state", f"Cannot read integration state: {error.detail}") + key = request.key or default_integration_key(state or {}) + warnings: list[str] = [] + token = None + binding = validate_uninstall_ownership(root, key) if request.require_complete and key else None + if key in records and not request.require_complete: + binding = installer.recovery_metadata(root, key, records[key]) + loaded = False + try: + try: + installer.load_installed_integrations(root) + except installer.IntegrationInstallError: + if not request.force or key not in records: + raise + token = installer.recovery_exclusion.set((root.resolve(), key)) + installer.load_installed_integrations(root) + binding = installer.recovery_metadata(root, key, records[key]) + warnings.append( + f"Recovering integration '{key}' from validated ownership metadata " + "without loading its failed implementation.", + ) + loaded = True + transaction = _transaction(root, key, state, records) if records else nullcontext() + with transaction: + if token is not None: + path = root / ".specify/integrations" / f"{key}.manifest.json" + if path.exists(): + manifest = IntegrationManifest.load(key, root) + if binding is None: + manifest = IntegrationManifest(key, root, version=manifest.version) + warnings.append("No local recovery ownership record; preserving old generated files.") + else: + preserved = [] + for name in manifest.files: + mode = binding.get("ownership_modes", {}).get(name, "shared") + manifest.set_ownership(name, mode) + if mode != "whole": + preserved.append(name) + if preserved: + warnings.append( + "Preserving shared, partial, or unproven file ownership: " + + ", ".join(preserved) + "; manual event cleanup may be needed.", + ) + manifest.save() + elif binding is not None: + # Project metadata cannot promote a locally partial/shared claim. + manifest = IntegrationManifest.load(key, root) + for name in manifest.files: + mode = binding.get("ownership_modes", {}).get(name, "shared") + if mode != manifest.ownership_modes[name]: + manifest.set_ownership(name, mode) + manifest.save() + with _retain_recovery_manifest(request, key): + result = _uninstall(request, warnings) + new_state, error = try_read_integration_json(root) + if error: + raise IntegrationUninstallError("invalid_state", f"Cannot read integration state: {error.detail}") + remaining = installed_integration_keys(new_state or {}) + for removed_key in records.keys() - set(remaining): + if removed_key != key: + warnings.extend(unregister_integration_artifacts( + root, removed_key, require_complete=request.require_complete, + )) + installer.remove_package(root, removed_key) + return replace(result, warnings=tuple(warnings)) + except IntegrationUninstallError: + raise + except Exception as exc: + raise IntegrationUninstallError("uninstall_failed", f"Integration uninstall failed: {exc}") from exc + finally: + try: + if loaded: + installer.load_installed_integrations(root) + finally: + if token is not None: + installer.recovery_exclusion.reset(token) + + +@installer.registry_synchronized +def uninstall_integration(request: IntegrationUninstallRequest) -> IntegrationUninstallResult: + """Normalize preparation, execution, and recovery failures at one boundary.""" + try: + if not request.project_directory.is_absolute(): + raise IntegrationUninstallError("invalid_project_directory", "project_directory must be absolute") + return _uninstall_with_lifecycle(replace( + request, project_directory=request.project_directory.resolve(strict=True), + )) + except IntegrationUninstallError: + raise + except Exception as exc: + raise IntegrationUninstallError("uninstall_failed", f"Integration uninstall failed: {exc}") from exc diff --git a/src/specify_cli/integrations/command_uninstall.py b/src/specify_cli/integrations/command_uninstall.py index ffe45c8ae1..3e71e9629c 100644 --- a/src/specify_cli/integrations/command_uninstall.py +++ b/src/specify_cli/integrations/command_uninstall.py @@ -1,172 +1,45 @@ -"""The ``specify integration uninstall`` command.""" -from __future__ import annotations +"""CLI adapter for ``specify integration uninstall``.""" +from __future__ import annotations import typer from rich.markup import escape from .._console import console from .._utils import _display_project_path -from ..integration_state import default_integration_key as _default_integration_key, installed_integration_keys as _installed_integration_keys, integration_settings as _integration_settings from ._commands import integration_app -from ._lifecycle import external_lifecycle, lifecycle_success -from ._helpers import _MANIFEST_READ_ERRORS, _clear_init_options_for_integration, _read_integration_json, _register_extensions_for_agent, _register_presets_for_agent, _remove_integration_json, _resolve_integration_options, _set_default_integration_or_exit, _unregister_extensions_for_agent, _unregister_presets_for_agent, _write_integration_json +from ._operation_uninstall import ( + IntegrationUninstallError, + IntegrationUninstallRequest, + uninstall_integration, +) @integration_app.command("uninstall") -@external_lifecycle("uninstall") def integration_uninstall( key: str = typer.Argument(None, help="Integration key to uninstall (default: current integration)"), force: bool = typer.Option(False, "--force", help="Remove files even if modified"), ): """Uninstall an integration, safely preserving modified files.""" - from . import get_integration - from .manifest import IntegrationManifest from .. import _require_specify_project - project_root = _require_specify_project() - current = _read_integration_json(project_root) - default_key = _default_integration_key(current) - installed_keys = _installed_integration_keys(current) - - if key is None: - if not default_key: - console.print("[yellow]No integration is currently installed.[/yellow]") - raise typer.Exit(0) - key = default_key - - if key not in installed_keys: - console.print(f"[red]Error:[/red] Integration '{key}' is not installed.") - raise typer.Exit(1) - - integration = get_integration(key) - - manifest_path = project_root / ".specify" / "integrations" / f"{key}.manifest.json" - if not manifest_path.exists(): - console.print(f"[yellow]No manifest found for integration '{key}'. Nothing to uninstall.[/yellow]") - _unregister_extensions_for_agent( - project_root, - key, - continuing="Continuing with integration uninstall; extension artifacts may need manual cleanup.", - ) - _unregister_presets_for_agent( - project_root, - key, - continuing="Continuing with integration uninstall; preset artifacts may need manual cleanup.", - ) - remaining = [installed for installed in installed_keys if installed != key] - new_default = default_key if default_key != key else (remaining[0] if remaining else None) - if remaining: - if default_key == key and new_default and (new_integration := get_integration(new_default)): - raw_options, parsed_options = _resolve_integration_options( - new_integration, current, new_default, None - ) - _set_default_integration_or_exit( - project_root, - current, - new_default, - new_integration, - remaining, - raw_options=raw_options, - parsed_options=parsed_options, - ) - _register_extensions_for_agent( - project_root, - new_default, - continuing="The fallback integration was selected, but extensions may need re-registration.", - ) - _register_presets_for_agent( - project_root, - new_default, - continuing="The fallback integration was selected, but presets may need re-registration.", - ) - else: - _write_integration_json( - project_root, new_default, remaining, _integration_settings(current) - ) - else: - _remove_integration_json(project_root) - if default_key == key: - _clear_init_options_for_integration(project_root, key) - raise typer.Exit(0) - + root = _require_specify_project() try: - manifest = IntegrationManifest.load(key, project_root) - except _MANIFEST_READ_ERRORS as exc: - console.print(f"[red]Error:[/red] Integration manifest for '{key}' is unreadable.") - console.print(f"Manifest: {manifest_path}") - console.print( - f"To recover, delete the unreadable manifest, run " - f"[cyan]specify integration uninstall {key}[/cyan] to clear stale metadata, " - f"then run [cyan]specify integration install {key}[/cyan] to regenerate." - ) - console.print(f"[dim]Details:[/dim] {exc}") - raise typer.Exit(1) - - if not integration: - console.print( - f"[yellow]Warning:[/yellow] Integration '{key}' not found " - "in registry. Falling back to manifest-based cleanup." - ) - removed, skipped = manifest.uninstall(project_root, force=force) - else: - removed, skipped = integration.teardown(project_root, manifest, force=force) - - # Extension/preset artifacts are tracked in their own registries, not the - # integration manifest, so teardown leaves them behind. - _unregister_extensions_for_agent( - project_root, - key, - continuing="Continuing with integration uninstall; extension artifacts may need manual cleanup.", - ) - _unregister_presets_for_agent( - project_root, - key, - continuing="Continuing with integration uninstall; preset artifacts may need manual cleanup.", - ) - - remaining = [installed for installed in installed_keys if installed != key] - new_default = default_key if default_key != key else (remaining[0] if remaining else None) - if remaining: - if default_key == key and new_default and (new_integration := get_integration(new_default)): - raw_options, parsed_options = _resolve_integration_options( - new_integration, current, new_default, None - ) - _set_default_integration_or_exit( - project_root, - current, - new_default, - new_integration, - remaining, - raw_options=raw_options, - parsed_options=parsed_options, - ) - _register_extensions_for_agent( - project_root, - new_default, - continuing="The fallback integration was selected, but extensions may need re-registration.", - ) - _register_presets_for_agent( - project_root, - new_default, - continuing="The fallback integration was selected, but presets may need re-registration.", - ) - else: - _write_integration_json( - project_root, new_default, remaining, _integration_settings(current) - ) - else: - _remove_integration_json(project_root) - - if default_key == key: - _clear_init_options_for_integration(project_root, key) - - name = (integration.config or {}).get("name", key) if integration else key - lifecycle_success(f"\n[green]✓[/green] Integration '{escape(str(name))}' uninstalled") - if removed: - console.print(f" Removed {len(removed)} file(s)") - if skipped: - console.print(f"\n[yellow]⚠[/yellow] {len(skipped)} modified file(s) were preserved:") - for path in skipped: - rel = _display_project_path(project_root, path) - console.print(f" {rel}") + result = uninstall_integration(IntegrationUninstallRequest(root, key, force)) + except IntegrationUninstallError as exc: + console.print(f"[red]Error:[/red] {escape(exc.message)}") + raise typer.Exit(1) from exc + if result.key is None: + console.print("[yellow]No integration is currently installed.[/yellow]") + return + for warning in result.warnings: + console.print(f"[yellow]Warning:[/yellow] {escape(warning)}") + if result.missing_manifest: + console.print(f"[yellow]No manifest found for integration '{escape(result.key)}'. Nothing to uninstall.[/yellow]") + console.print(f"\n[green]✓[/green] Integration '{escape(result.name)}' uninstalled") + if result.removed: + console.print(f" Removed {len(result.removed)} file(s)") + if result.skipped: + console.print(f"\n[yellow]⚠[/yellow] {len(result.skipped)} modified file(s) were preserved:") + for path in result.skipped: + console.print(f" {_display_project_path(root, path)}") diff --git a/src/specify_cli/mcp_destroy.py b/src/specify_cli/mcp_destroy.py new file mode 100644 index 0000000000..a3ba6ab18c --- /dev/null +++ b/src/specify_cli/mcp_destroy.py @@ -0,0 +1,23 @@ +"""Explicit MCP inventory disposition for project destruction.""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import Literal + + +@dataclass(frozen=True) +class DestroyToolInventory: + operation_id: Literal["destroy"] = "destroy" + cli_path: Literal["specify destroy"] = "specify destroy" + mcp_tool_name: Literal["specify_destroy"] = "specify_destroy" + contract_version: Literal["1"] = "1" + disposition: Literal["unavailable"] = "unavailable" + disposition_reason: str = ( + "Project destruction is CLI-only until a dedicated MCP destructive-consent adapter is implemented." + ) + capabilities: frozenset[str] = frozenset({"local-read", "project-write", "execution", "self-modifying"}) + network_access: Literal["none"] = "none" + + +DESTROY_TOOL = DestroyToolInventory() diff --git a/tests/specify_cli/integrations/test_installed_adapters.py b/tests/specify_cli/integrations/test_installed_adapters.py index 12484c6f04..c51d5aa2e2 100644 --- a/tests/specify_cli/integrations/test_installed_adapters.py +++ b/tests/specify_cli/integrations/test_installed_adapters.py @@ -179,6 +179,116 @@ def install(project): return result +def test_destroy_uses_trusted_external_lifecycle_and_contribution_cleanup(tmp_path, server): + publish(server) + project = catalog_project(tmp_path, server) + install(project) + added = run(project, ["extension", "add", "git"]) + assert added.exit_code == 0, added.output + source = project / "src/app.py" + source.parent.mkdir() + source.write_bytes(b"application source") + before = snapshot(project) + preview = run(project, ["destroy", "--dry-run", "--json"]) + assert preview.exit_code == 0, preview.output + assert snapshot(project) == before + result = run(project, ["destroy", "--yes", "--force", "--json"]) + assert result.exit_code == 0, result.output + assert result.stderr == "" + assert source.read_bytes() == b"application source" + assert not list((project / ".sample-agent").rglob("SKILL.md")) + assert not (project / ".specify").exists() + + +@pytest.mark.parametrize("force", [False, True]) +def test_destroy_preserves_constitution_recorded_by_a_trusted_adapter(tmp_path, server, force): + body = ''' def setup(self, project_root, manifest, **kwargs): + created = super().setup(project_root, manifest, **kwargs) + created.append(manifest.record_file(".specify/memory/constitution.md", "# Existing principles\\r\\n")) + return created +''' + publish(server, code=implementation(body=body)) + project = catalog_project(tmp_path, server) + install(project) + constitution = project / ".specify/memory/constitution.md" + expected = constitution.read_bytes() + result = run(project, [ + "destroy", "--yes", "--json", "--preserve-constitution", + ] + (["--force"] if force else [])) + assert result.exit_code == 0, result.output + assert result.stderr == "" + assert constitution.read_bytes() == expected + assert json.loads(result.stdout)["preserved"] == [".specify/memory/constitution.md"] + assert not read_records(project) + + +@pytest.mark.parametrize("flags,keep_constitution,keep_specs", [ + ([], False, False), + (["--preserve-specs"], False, True), + (["--preserve-constitution", "--preserve-specs"], True, True), + (["--preserve-all"], True, True), +]) +@pytest.mark.parametrize("force", [False, True]) +def test_destroy_preservation_flags_cover_trusted_adapter_owned_specs( + tmp_path, server, flags, keep_constitution, keep_specs, force, +): + body = ''' def setup(self, project_root, manifest, **kwargs): + created = super().setup(project_root, manifest, **kwargs) + created.append(manifest.record_file(".specify/memory/constitution.md", "# Principles\\r\\n")) + created.append(manifest.record_file("specs/001-feature/spec.md", "# Specification\\r\\n")) + return created +''' + publish(server, code=implementation(body=body)) + project = catalog_project(tmp_path, server) + install(project) + source = project / "app.py" + source.write_bytes(b"unrelated source") + before = snapshot(project) + command = ["destroy", "--json", *flags, *(["--force"] if force else [])] + preview = run(project, [*command, "--dry-run"]) + assert preview.exit_code == 0, preview.output + assert snapshot(project) == before + result = run(project, [*command, "--yes"]) + assert result.exit_code == 0, result.output + assert result.stderr == "" + constitution = project / ".specify/memory/constitution.md" + spec = project / "specs/001-feature/spec.md" + assert constitution.exists() is keep_constitution + assert spec.exists() is keep_specs + if keep_constitution: + assert constitution.read_bytes() == b"# Principles\r\n" + else: + assert not (project / ".specify").exists() + if keep_specs: + assert spec.read_bytes() == b"# Specification\r\n" + else: + assert not (project / "specs").exists() + assert source.read_bytes() == b"unrelated source" + assert not read_records(project) + + +@pytest.mark.parametrize("force", [False, True]) +def test_destroy_cannot_replace_revoked_external_ownership_with_force(tmp_path, server, monkeypatch, force): + publish(server) + project = catalog_project(tmp_path, server) + install(project) + trust_path = Path.home() / ".specify/integration-trust.json" + trust = json.loads(trust_path.read_text()) + trust["grants"] = [] + trust_path.write_text(json.dumps(trust)) + before = snapshot(project) + + def fail(*args, **kwargs): + pytest.fail("An untrusted adapter must not be loaded during destroy") + + monkeypatch.setattr("specify_cli.integrations.installer.load_installed_integrations", fail) + result = run(project, ["destroy", "--yes", "--json"] + (["--force"] if force else [])) + assert result.exit_code == 1 + assert result.stdout == "" + assert "ownership" in json.loads(result.stderr)["error"]["message"] + assert snapshot(project) == before + + def snapshot(project): return { path.relative_to(project).as_posix(): path.read_bytes() @@ -3889,7 +3999,7 @@ def test_check_renders_external_adapter_names_as_literal_text(tmp_path, server, @pytest.mark.parametrize("contents", ["empty", "user", "generated"]) @pytest.mark.parametrize("fail", [False, True]) def test_kimi_legacy_parent_removal_obeys_lifecycle_rollback(tmp_path, server, monkeypatch, operation, contents, fail): - from specify_cli.integrations import command_install, command_uninstall + from specify_cli.integrations import _operation_uninstall, command_install publish(server) project = catalog_project(tmp_path, server) @@ -3916,7 +4026,7 @@ def fail_after_cleanup(*args, **kwargs): assert legacy.exists() == (contents == "user") raise OSError("sample failure after legacy cleanup") - module = command_install if operation == "migrate" else command_uninstall + module = command_install if operation == "migrate" else _operation_uninstall monkeypatch.setattr(module, "_write_integration_json", fail_after_cleanup) arguments = ( ["integration", "install", "kimi", "--force", "--script", "py", "--integration-options=--migrate-legacy"] diff --git a/tests/specify_cli/integrations/test_operation_uninstall.py b/tests/specify_cli/integrations/test_operation_uninstall.py new file mode 100644 index 0000000000..c008436429 --- /dev/null +++ b/tests/specify_cli/integrations/test_operation_uninstall.py @@ -0,0 +1,66 @@ +"""Shared uninstall contracts independent of the CLI adapter.""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from specify_cli.integrations._operation_uninstall import ( + IntegrationUninstallError, + IntegrationUninstallRequest, + uninstall_integration, +) +from tests.specify_cli.integrations._helpers import _init_project + + +def test_explicit_project_uninstall_does_not_change_launch_directory(tmp_path): + project = _init_project(tmp_path, "claude") + launch = Path.cwd() + result = uninstall_integration(IntegrationUninstallRequest(project, "claude")) + assert result.key == "claude" + assert result.removed + assert not result.skipped + assert Path.cwd() == launch + assert not (project / ".specify/integration.json").exists() + assert (project / ".specify/templates").exists() + + +def test_project_with_no_integration_is_a_noop(tmp_path): + (tmp_path / ".specify").mkdir() + result = uninstall_integration(IntegrationUninstallRequest(tmp_path)) + assert result.key is None + assert not result.removed + + +def test_project_marker_is_validated_by_the_shared_operation(tmp_path): + with pytest.raises(IntegrationUninstallError) as caught: + uninstall_integration(IntegrationUninstallRequest(tmp_path)) + assert caught.value.code == "not_a_spec_kit_project" + + +def test_wrong_key_is_a_typed_error_without_mutation(tmp_path): + project = _init_project(tmp_path, "claude") + state = project / ".specify/integration.json" + before = state.read_bytes() + with pytest.raises(IntegrationUninstallError) as caught: + uninstall_integration(IntegrationUninstallRequest(project, "copilot")) + assert caught.value.code == "not_installed" + assert state.read_bytes() == before + + +def test_complete_cleanup_failure_retains_integration_metadata(tmp_path, monkeypatch): + project = _init_project(tmp_path, "claude") + + def fail(*args): + raise OSError("contribution cleanup failed") + + monkeypatch.setattr("specify_cli.extensions.ExtensionManager.unregister_agent_artifacts", fail) + with pytest.raises(IntegrationUninstallError, match="contribution cleanup failed"): + uninstall_integration(IntegrationUninstallRequest( + project, "claude", require_complete=True, + )) + state = json.loads((project / ".specify/integration.json").read_text()) + assert "claude" in state["installed_integrations"] + assert (project / ".specify/integrations/claude.manifest.json").exists() diff --git a/tests/specify_cli/test_command_destroy.py b/tests/specify_cli/test_command_destroy.py new file mode 100644 index 0000000000..0c147d3fba --- /dev/null +++ b/tests/specify_cli/test_command_destroy.py @@ -0,0 +1,247 @@ +"""Public CLI contracts for project-local Spec Kit uninstall.""" + +from __future__ import annotations + +import json + +import pytest +from typer.testing import CliRunner + +from specify_cli import app + +runner = CliRunner() + +PRESERVATION_FLAGS = [ + [], + ["--preserve-constitution"], + ["--preserve-specs"], + ["--preserve-constitution", "--preserve-specs"], + ["--preserve-all"], + ["--preserve-all", "--preserve-constitution"], + ["--preserve-all", "--preserve-specs"], + ["--preserve-all", "--preserve-constitution", "--preserve-specs"], +] + + +@pytest.mark.parametrize("flags", PRESERVATION_FLAGS) +@pytest.mark.parametrize("force", [False, True]) +def test_preservation_flags_control_artifact_cleanup_and_preview(tmp_path, flags, force): + constitution = tmp_path / ".specify/memory/constitution.md" + constitution.parent.mkdir(parents=True) + constitution.write_bytes(b"# Principles\r\n") + config = tmp_path / ".specify/config" + config.write_bytes(b"configuration") + spec = tmp_path / "specs/001-feature/spec.md" + spec.parent.mkdir(parents=True) + spec.write_bytes(b"# Specification\r\n") + source = tmp_path / "app.py" + source.write_bytes(b"source") + original = { + path: path.read_bytes() for path in (constitution, config, spec, source) + } + keep_constitution = "--preserve-all" in flags or "--preserve-constitution" in flags + keep_specs = "--preserve-all" in flags or "--preserve-specs" in flags + args = ["destroy", "--project", str(tmp_path), "--json", *flags, *(["--force"] if force else [])] + preview = runner.invoke(app, [*args, "--dry-run"]) + assert preview.exit_code == 0, preview.output + assert preview.stderr == "" + plan = json.loads(preview.stdout) + assert (".specify/memory/constitution.md" in plan["removed"]) is not keep_constitution + assert ("specs/001-feature/spec.md" in plan["removed"]) is not keep_specs + assert all(path.read_bytes() == content for path, content in original.items()) + result = runner.invoke(app, [*args, "--yes"]) + assert result.exit_code == 0, result.output + assert result.stderr == "" + expected = set() + if keep_constitution: + expected.add(".specify/memory/constitution.md") + assert constitution.read_bytes() == original[constitution] + else: + assert not (tmp_path / ".specify").exists() + if keep_specs: + expected.add("specs") + assert spec.read_bytes() == original[spec] + else: + assert not (tmp_path / "specs").exists() + assert set(json.loads(result.stdout)["preserved"]) == expected + assert source.read_bytes() == original[source] + assert not config.exists() + repeated = runner.invoke(app, [*args, "--yes"]) + assert repeated.exit_code == 0, repeated.output + assert json.loads(repeated.stdout)["removed"] == [] + + +@pytest.mark.parametrize("flags", PRESERVATION_FLAGS) +def test_preservation_flags_never_supply_destructive_consent(tmp_path, monkeypatch, flags): + config = tmp_path / ".specify/config" + config.parent.mkdir() + config.write_bytes(b"configuration") + specs = tmp_path / "specs/feature/spec.md" + specs.parent.mkdir(parents=True) + specs.write_bytes(b"specification") + monkeypatch.setattr("specify_cli.command_destroy._stdin_is_interactive", lambda: False) + result = runner.invoke(app, ["destroy", "--project", str(tmp_path), "--force", "--json", *flags]) + assert result.exit_code == 1, result.output + assert result.stdout == "" + assert json.loads(result.stderr)["error"]["code"] == "confirmation_required" + assert config.read_bytes() == b"configuration" + assert specs.read_bytes() == b"specification" + + +def test_interactive_default_warns_about_specs_and_cancellation_preserves_them(tmp_path, monkeypatch): + (tmp_path / ".specify").mkdir() + specs = tmp_path / "specs/empty" + specs.mkdir(parents=True) + prompts = [] + monkeypatch.setattr("specify_cli.command_destroy._stdin_is_interactive", lambda: True) + + def cancel(message, **kwargs): + prompts.append(message) + return False + + monkeypatch.setattr("typer.confirm", cancel) + result = runner.invoke(app, ["destroy", "--project", str(tmp_path)]) + assert result.exit_code == 0, result.output + assert prompts and "specs/" in prompts[0] + assert "specs/" in result.stdout + assert specs.exists() + assert (tmp_path / ".specify").exists() + + +def test_destroy_removes_configuration_and_preserves_constitution(tmp_path): + memory = tmp_path / ".specify/memory" + memory.mkdir(parents=True) + constitution = memory / "constitution.md" + constitution.write_bytes(b"# Project principles\r\n") + config = tmp_path / ".specify/custom.txt" + config.write_text("disposable", encoding="utf-8") + + result = runner.invoke(app, [ + "destroy", "--project", str(tmp_path), "--yes", "--json", "--preserve-constitution", + ]) + + assert result.exit_code == 0, result.output + assert not config.exists() + assert constitution.read_bytes() == b"# Project principles\r\n" + assert json.loads(result.stdout)["preserved"] == [".specify/memory/constitution.md"] + assert result.stderr == "" + + +@pytest.mark.parametrize("arguments", [[], ["--force"], ["--json"], ["--json", "--force"]]) +def test_unattended_invocations_require_yes(tmp_path, monkeypatch, arguments): + config = tmp_path / ".specify/config" + config.parent.mkdir() + config.write_bytes(b"configuration") + monkeypatch.setattr("specify_cli.command_destroy._stdin_is_interactive", lambda: False) + result = runner.invoke(app, ["destroy", "--project", str(tmp_path), *arguments]) + assert result.exit_code == 1 + assert config.read_bytes() == b"configuration" + if "--json" in arguments: + assert result.stdout == "" + assert json.loads(result.stderr)["error"]["code"] == "confirmation_required" + else: + assert "--yes" in result.output + + +def test_json_preview_does_not_prompt_or_write(tmp_path, monkeypatch): + config = tmp_path / ".specify/config" + config.parent.mkdir() + config.write_bytes(b"configuration") + monkeypatch.setattr("typer.confirm", lambda *a, **kw: pytest.fail("prompted")) + result = runner.invoke(app, ["destroy", "--project", str(tmp_path), "--dry-run", "--json"]) + assert result.exit_code == 0, result.output + assert json.loads(result.stdout)["removed"] == [".specify/config"] + assert result.stderr == "" + assert config.exists() + + +@pytest.mark.parametrize("answer", [True, False]) +def test_interactive_confirmation(tmp_path, monkeypatch, answer): + config = tmp_path / ".specify/config" + config.parent.mkdir() + config.write_bytes(b"configuration") + monkeypatch.setattr("specify_cli.command_destroy._stdin_is_interactive", lambda: True) + monkeypatch.setattr("typer.confirm", lambda *a, **kw: answer) + result = runner.invoke(app, ["destroy", "--project", str(tmp_path)]) + assert result.exit_code == 0, result.output + assert config.exists() is not answer + assert ("cancelled" in result.output) is not answer + + +def test_current_directory_and_project_override(tmp_path, monkeypatch): + target = tmp_path / "target" + target.mkdir() + (target / ".specify").mkdir() + (target / ".specify/config").write_text("config") + monkeypatch.setenv("SPECIFY_INIT_DIR", str(target)) + result = runner.invoke(app, ["destroy", "--yes", "--json"]) + assert result.exit_code == 0, result.output + assert not (target / ".specify").exists() + + +def test_json_error_sanitizes_unexpected_failure(tmp_path, monkeypatch): + def fail(*args, **kwargs): + raise RuntimeError("private detail") + + monkeypatch.setattr("specify_cli.command_destroy.destroy_project", fail) + result = runner.invoke(app, ["destroy", "--project", str(tmp_path), "--yes", "--json"]) + assert result.exit_code == 1 + assert result.stdout == "" + assert json.loads(result.stderr)["error"]["code"] == "internal_error" + assert "private detail" not in result.output + + +def test_mcp_has_explicit_unavailable_disposition(): + from specify_cli.mcp_destroy import DESTROY_TOOL + + assert DESTROY_TOOL.operation_id == "destroy" + assert DESTROY_TOOL.disposition == "unavailable" + assert "project-write" in DESTROY_TOOL.capabilities + assert DESTROY_TOOL.disposition_reason + + +@pytest.mark.parametrize("arguments", [ + ["--unknown"], ["--project"], ["unexpected-positional"], ["--force=invalid"], +]) +def test_json_argument_errors_use_single_envelope(arguments): + result = runner.invoke(app, ["destroy", "--json", *arguments]) + assert result.exit_code == 2 + assert result.stdout == "" + assert json.loads(result.stderr)["error"]["code"] == "invalid_arguments" + + +def test_json_legacy_layout_cleanup_has_no_raw_warnings(tmp_path): + registry = tmp_path / ".specify/extensions/.registry" + registry.parent.mkdir(parents=True) + registry.write_text(json.dumps({ + "extensions": {"sample": { + "registered_commands": {"opencode": ["speckit.sample.test"]}, + }}, + })) + skill = tmp_path / ".opencode/command/speckit.sample.test.md" + skill.parent.mkdir(parents=True) + skill.write_text("generated") + result = runner.invoke(app, [ + "destroy", "--project", str(tmp_path), "--yes", "--force", "--json", + ]) + assert result.exit_code == 0, result.output + assert result.stderr == "" + assert ".opencode/command/speckit.sample.test.md" in json.loads(result.stdout)["removed"] + + +@pytest.mark.parametrize("answer", [True, False]) +def test_directory_only_cleanup_prompts(tmp_path, monkeypatch, answer): + empty = tmp_path / ".specify/empty" + empty.mkdir(parents=True) + prompts = [] + monkeypatch.setattr("specify_cli.command_destroy._stdin_is_interactive", lambda: True) + + def confirm(*args, **kwargs): + prompts.append(args) + return answer + + monkeypatch.setattr("typer.confirm", confirm) + result = runner.invoke(app, ["destroy", "--project", str(tmp_path)]) + assert result.exit_code == 0, result.output + assert len(prompts) == 1 + assert empty.exists() is not answer diff --git a/tests/specify_cli/test_command_init.py b/tests/specify_cli/test_command_init.py index e353445870..cf1d2a9e9f 100644 --- a/tests/specify_cli/test_command_init.py +++ b/tests/specify_cli/test_command_init.py @@ -50,7 +50,7 @@ def test_init_command_registered(): callback_names = [ cmd.callback.__name__ for cmd in app.registered_commands if cmd.callback ] - assert callback_names == ["init", "check", "version", "mcp"] + assert callback_names == ["init", "check", "destroy", "version", "mcp"] def test_init_has_win32_guard(): diff --git a/tests/specify_cli/test_operation_destroy.py b/tests/specify_cli/test_operation_destroy.py new file mode 100644 index 0000000000..1407a5593e --- /dev/null +++ b/tests/specify_cli/test_operation_destroy.py @@ -0,0 +1,1016 @@ +"""Ownership, policy, and lifecycle evidence for project-local uninstall.""" + +from __future__ import annotations + +import json +import os +from pathlib import Path + +import pytest +from typer.testing import CliRunner + +from specify_cli import app +from specify_cli._operation_destroy import ( + CONSTITUTION, + DestroyError, + DestroyRequest, + destroy_project, + plan_destroy, +) +from specify_cli.integrations.manifest import IntegrationManifest + + +def write(root, relative, content=b"generated\n"): + path = root / relative + path.parent.mkdir(parents=True, exist_ok=True) + path.write_bytes(content) + return path + + +def manifest(root, key="copilot", relative=".github/skills/speckit-plan/SKILL.md"): + record = IntegrationManifest(key, root) + record.record_file(relative, b"generated\n") + record.save() + state_path = root / ".specify/integration.json" + state = json.loads(state_path.read_text()) if state_path.exists() else {} + installed = state.get("installed_integrations", []) + default = state.get("integration", key) + if default not in installed: + installed.insert(0, default) + if key not in installed: + installed.append(key) + write(root, ".specify/integration.json", json.dumps({ + "integration": default, "installed_integrations": installed, + }).encode()) + return record + + +def snapshot(root): + return { + path.relative_to(root).as_posix(): path.read_bytes() + for path in root.rglob("*") + if path.is_file() and not path.is_symlink() + } + + +def symlink(path, target, *, directory=False): + try: + path.symlink_to(target, target_is_directory=directory) + except OSError as exc: + if os.name == "nt" and getattr(exc, "winerror", None) == 1314: + pytest.skip("Windows symlink privilege unavailable") + raise + + +@pytest.mark.parametrize("constitution", [False, True]) +def test_uninstall_multiple_manifests_and_preserve_unrelated_files(tmp_path, constitution): + manifest(tmp_path) + manifest(tmp_path, "claude", ".claude/skills/speckit-plan/SKILL.md") + write(tmp_path, ".specify/cache/data") + write(tmp_path, "specs/001-feature/spec.md", b"specification") + write(tmp_path, ".github/skills/user/SKILL.md", b"user skill") + write(tmp_path, "src/main.py", b"source") + if constitution: + write(tmp_path, CONSTITUTION, b"# Principles\r\n\xef\xbb\xbf") + expected = snapshot(tmp_path) + expected = { + path: content for path, content in expected.items() + if (not path.startswith(".specify/") or path == CONSTITUTION) + and "speckit-plan" not in path + } + + result = destroy_project(DestroyRequest(tmp_path, confirmed=True, preserve_all=True)) + + assert snapshot(tmp_path) == expected + assert not (tmp_path / ".claude").exists() + assert (tmp_path / ".specify").exists() is constitution + assert result.preserved == ((CONSTITUTION, "specs") if constitution else ("specs",)) + assert not destroy_project(DestroyRequest(tmp_path, preserve_all=True)).removed + + +@pytest.mark.parametrize("confirmed,force", [(False, False), (False, True), (True, False), (True, True)]) +def test_preview_never_mutates_and_reports_modified_file(tmp_path, confirmed, force): + record = manifest(tmp_path) + relative = next(iter(record.files)) + write(tmp_path, relative, b"customized") + before = snapshot(tmp_path) + + result = destroy_project(DestroyRequest(tmp_path, confirmed=confirmed, force=force, dry_run=True)) + + assert relative in result.removed + assert result.requires_force == (relative,) + assert snapshot(tmp_path) == before + + +@pytest.mark.parametrize("force", [False, True]) +def test_force_never_implies_confirmation(tmp_path, force): + manifest(tmp_path) + before = snapshot(tmp_path) + with pytest.raises(DestroyError, match="confirm with --yes") as caught: + destroy_project(DestroyRequest(tmp_path, force=force)) + assert caught.value.code == "confirmation_required" + assert snapshot(tmp_path) == before + + +def test_modified_files_block_before_any_deletion_and_force_removes_them(tmp_path): + manifest(tmp_path) + changed = write(tmp_path, ".github/skills/speckit-plan/SKILL.md", b"modified") + before = snapshot(tmp_path) + with pytest.raises(DestroyError) as caught: + destroy_project(DestroyRequest(tmp_path, confirmed=True)) + assert caught.value.code == "modified_files" + assert snapshot(tmp_path) == before + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert not changed.exists() + assert not (tmp_path / ".specify").exists() + + +@pytest.mark.parametrize("relative", [ + "../outside", "/outside", ".git/config", "specs/../source.py", + ".GIT/config", "C:\\outside", ".", ".github/../README.md", ".specify", +]) +def test_unsafe_manifest_paths_block_every_deletion(tmp_path, relative): + record = manifest(tmp_path) + path = record.manifest_path + data = json.loads(path.read_text()) + data["files"][relative] = "a" * 64 + path.write_text(json.dumps(data)) + before = snapshot(tmp_path) + with pytest.raises(DestroyError): + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert snapshot(tmp_path) == before + + +@pytest.mark.parametrize("content", [ + b"broken", b"[]", b"\xff", b"{}", b'{"integration":"copilot","files":[]}', + b'{"integration":"copilot","files":{"x":"invalid"}}', + b'{"integration":"copilot","files":{},"ownership_modes":{"x":"partial"}}', +]) +def test_corrupt_manifest_blocks_cleanup(tmp_path, content): + record = manifest(tmp_path) + record.manifest_path.write_bytes(content) + before = snapshot(tmp_path) + with pytest.raises(DestroyError): + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert snapshot(tmp_path) == before + + +@pytest.mark.parametrize("kind", ["extensions", "presets"]) +@pytest.mark.parametrize("content", [b"invalid", b"[]", b"{}", b'{"extensions":[]}']) +def test_corrupt_contribution_registry_blocks_cleanup(tmp_path, kind, content): + manifest(tmp_path) + write(tmp_path, f".specify/{kind}/.registry", content) + before = snapshot(tmp_path) + with pytest.raises(DestroyError): + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert snapshot(tmp_path) == before + + +def test_declared_integration_requires_manifest(tmp_path): + write(tmp_path, ".specify/integration.json", b'{"integration":"copilot"}') + before = snapshot(tmp_path) + with pytest.raises(DestroyError, match="Missing manifest"): + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert snapshot(tmp_path) == before + + +@pytest.mark.parametrize("mode", ["partial", "shared"]) +def test_shared_ownership_never_forced(tmp_path, mode): + record = manifest(tmp_path, relative=".vscode/settings.json") + record.set_ownership(".vscode/settings.json", mode) + record.save() + before = snapshot(tmp_path) + with pytest.raises(DestroyError, match="Partial/shared"): + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert snapshot(tmp_path) == before + + +def test_symlinked_manifest_parent_is_not_traversed(tmp_path): + manifest(tmp_path) + (tmp_path / ".github").rename(tmp_path / "other") + symlink(tmp_path / ".github", tmp_path / "other", directory=True) + before = snapshot(tmp_path) + with pytest.raises(DestroyError, match="symlinked directory"): + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert snapshot(tmp_path) == before + + +@pytest.mark.parametrize("location", [".specify", ".specify/memory", ".specify/integrations"]) +def test_symlinked_configuration_is_rejected(tmp_path, location): + target = tmp_path / "user-directory" + target.mkdir() + write(tmp_path, "user-directory/constitution.md", b"user") + link = tmp_path / location + link.parent.mkdir(parents=True, exist_ok=True) + symlink(link, target, directory=True) + before = snapshot(tmp_path) + with pytest.raises(DestroyError): + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert snapshot(tmp_path) == before + assert link.is_symlink() + + +def test_allowed_link_deletion_never_touches_external_target(tmp_path): + write(tmp_path, ".specify/init-options.json", b'{"allow_symlinks":true}') + external = write(tmp_path, "user-owned", b"must survive") + symlink(tmp_path / ".specify/cache-link", external) + destroy_project(DestroyRequest(tmp_path, confirmed=True)) + assert external.read_bytes() == b"must survive" + assert not (tmp_path / ".specify").exists() + + +def test_denied_link_deletion_stops_before_changes(tmp_path): + write(tmp_path, ".specify/file") + external = write(tmp_path, "user-owned", b"must survive") + symlink(tmp_path / ".specify/cache-link", external) + before = snapshot(tmp_path) + with pytest.raises(DestroyError, match="symlink"): + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert snapshot(tmp_path) == before + + +def test_constitution_link_is_preserved_without_touching_target(tmp_path): + write(tmp_path, ".specify/cache") + external = write(tmp_path, "principles", b"principles") + link = tmp_path / CONSTITUTION + link.parent.mkdir() + symlink(link, external) + destroy_project(DestroyRequest(tmp_path, confirmed=True, preserve_constitution=True)) + assert link.is_symlink() + assert external.read_bytes() == b"principles" + + +def test_io_failure_reports_progress_and_retains_metadata(tmp_path, monkeypatch): + record = manifest(tmp_path) + original_unlink = Path.unlink + + def fail(path, **kwargs): + if ".github" in path.parts: + raise PermissionError("locked by editor") + return original_unlink(path, **kwargs) + + monkeypatch.setattr(Path, "unlink", fail) + with pytest.raises(DestroyError) as caught: + destroy_project(DestroyRequest(tmp_path, confirmed=True)) + assert caught.value.code == "destroy_failed" + assert caught.value.details["removed"] == [] + assert record.manifest_path.exists() + + +def test_empty_project_and_constitution_only_are_noops(tmp_path): + assert not destroy_project(DestroyRequest(tmp_path)).removed + write(tmp_path, CONSTITUTION) + before = snapshot(tmp_path) + assert not destroy_project(DestroyRequest(tmp_path, preserve_constitution=True)).removed + assert snapshot(tmp_path) == before + + +@pytest.mark.parametrize("root", [Path("relative"), Path("missing-root")]) +def test_relative_root_is_rejected(root): + with pytest.raises(DestroyError) as caught: + plan_destroy(DestroyRequest(root)) + assert caught.value.code == "invalid_project_directory" + + +@pytest.mark.parametrize("integration,options", [ + ("copilot", None), + ("claude", None), + ("gemini", None), + ("generic", "--commands-dir .custom/commands"), + ("generic", "--commands-dir .custom/skills --skills"), +]) +def test_real_cli_init_destroy_with_extension_and_preset(tmp_path, monkeypatch, integration, options): + runner = CliRunner() + monkeypatch.chdir(tmp_path) + args = [ + "init", "--here", "--force", "--non-interactive", "--ignore-agent-tools", + "--integration", integration, "--script", "py", "--extension", "git", "--preset", "lean", + ] + if options: + args.append(f"--integration-options={options}") + result = runner.invoke(app, args) + assert result.exit_code == 0, result.output + write(tmp_path, CONSTITUTION, b"real principles\r\n") + write(tmp_path, "specs/001-feature/spec.md", b"feature") + write(tmp_path, ".github/skills/user-owned/SKILL.md", b"custom") + before = snapshot(tmp_path) + preview = destroy_project(DestroyRequest(tmp_path, dry_run=True)) + assert any("git" in path and not path.startswith(".specify") for path in preview.removed) + assert snapshot(tmp_path) == before + + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True, preserve_all=True)) + + assert snapshot(tmp_path) == { + CONSTITUTION: b"real principles\r\n", + "specs/001-feature/spec.md": b"feature", + ".github/skills/user-owned/SKILL.md": b"custom", + } + + +@pytest.mark.parametrize("missing_manifest", [True, False]) +def test_hermes_uses_its_existing_uninstall_lifecycle(tmp_path, monkeypatch, missing_manifest): + home = tmp_path / "home" + home.mkdir() + project = tmp_path / "project" + project.mkdir() + monkeypatch.setattr(Path, "home", lambda: home) + global_skill = write(home, ".hermes/skills/speckit-plan/SKILL.md") + manifest(project, "hermes", ".hermes/skills/speckit-plan/SKILL.md") + if missing_manifest: + (project / ".specify/integrations/hermes.manifest.json").unlink() + before = snapshot(project) + with pytest.raises(DestroyError, match="Missing manifest"): + destroy_project(DestroyRequest(project, confirmed=True)) + assert snapshot(project) == before + assert global_skill.exists() + else: + preview = destroy_project(DestroyRequest(project, dry_run=True)) + assert preview.global_scopes == (str(home / ".hermes/skills"),) + assert global_skill.exists() + destroy_project(DestroyRequest(project, confirmed=True)) + assert not global_skill.exists() + assert not (project / ".specify").exists() + + +def test_machine_level_trust_is_never_removed(tmp_path, monkeypatch): + write(tmp_path, ".specify/integration-trust.json", b'{"grants":[]}') + monkeypatch.setattr(Path, "home", lambda: tmp_path) + before = snapshot(tmp_path) + with pytest.raises(DestroyError, match="machine-level"): + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert snapshot(tmp_path) == before + + +@pytest.mark.parametrize("state", [ + b"{}", + b'{"installed_integrations":"copilot"}', b'{"installed_integrations":[null]}', + b'{"integration":42}', b'{"default_integration":""}', + b'{"integration_state_schema":2}', +]) +def test_invalid_integration_state_blocks_every_deletion(tmp_path, state): + write(tmp_path, ".specify/integration.json", state) + before = snapshot(tmp_path) + with pytest.raises(DestroyError): + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert snapshot(tmp_path) == before + + +def test_legacy_init_options_cannot_hide_missing_manifest(tmp_path): + write(tmp_path, ".specify/init-options.json", b'{"ai":"copilot"}') + before = snapshot(tmp_path) + with pytest.raises(DestroyError, match="Missing manifest"): + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert snapshot(tmp_path) == before + + +@pytest.mark.parametrize("integration,options", [ + ("none", None), + ("claude", None), + ("gemini", None), + ("copilot", "--commands"), + ("generic", "--commands-dir .custom/commands"), + ("generic", "--commands-dir .custom/skills --skills"), +]) +def test_real_init_destroy_layouts(tmp_path, monkeypatch, integration, options): + monkeypatch.chdir(tmp_path) + args = [ + "init", "--here", "--force", "--non-interactive", "--ignore-agent-tools", + "--integration", integration, "--script", "py", + ] + if options: + args.append(f"--integration-options={options}") + result = CliRunner().invoke(app, args) + assert result.exit_code == 0, result.output + destroy_project(DestroyRequest(tmp_path, confirmed=True, preserve_all=True)) + assert snapshot(tmp_path) == {CONSTITUTION: (tmp_path / CONSTITUTION).read_bytes()} + assert not destroy_project(DestroyRequest(tmp_path, preserve_all=True)).removed + + +def test_external_adapter_cleanup_does_not_import_package(tmp_path, monkeypatch): + manifest(tmp_path, "sample-agent", ".sample/commands/speckit.plan.md") + write(tmp_path, ".specify/integration.json", b'{"integration":"sample-agent"}') + write(tmp_path, ".specify/integrations/packages/sample-agent/__init__.py", b"raise RuntimeError()") + + def fail(*args, **kwargs): + pytest.fail("Executable adapter loading must not occur") + + monkeypatch.setattr("specify_cli.integrations.load_installed_integrations", fail) + before = snapshot(tmp_path) + with pytest.raises(DestroyError, match="ownership"): + destroy_project(DestroyRequest(tmp_path, confirmed=True)) + assert snapshot(tmp_path) == before + + +@pytest.mark.parametrize("kind", ["extensions", "presets"]) +def test_inactive_contribution_commands_and_skill_resources_are_included(tmp_path, kind): + write(tmp_path, f".specify/{kind}/.registry", json.dumps({ + kind: {"sample": { + "registered_commands": {"claude": ["speckit.sample.test"]}, + "registered_skills": ( + ["sample-skill"] if kind == "extensions" else {"copilot": ["sample-skill"]} + ), + }}, + }).encode()) + write(tmp_path, ".claude/skills/speckit-sample-test/SKILL.md") + write(tmp_path, ".claude/skills/speckit-sample-test/examples/example.txt") + prefix = "extension" if kind == "extensions" else "preset" + write(tmp_path, ".github/skills/sample-skill/SKILL.md", + f"---\nmetadata:\n source: {prefix}:sample\n---\nSkill\n".encode()) + write(tmp_path, ".github/skills/sample-skill/resources/tool.py") + write(tmp_path, ".github/skills/user-owned/SKILL.md", b"user") + before = snapshot(tmp_path) + preview = destroy_project(DestroyRequest(tmp_path, dry_run=True)) + assert ".github/skills/sample-skill/resources/tool.py" in preview.removed + assert ".claude/skills/speckit-sample-test/SKILL.md" in preview.removed + assert ".claude/skills/speckit-sample-test/examples/example.txt" not in preview.removed + assert snapshot(tmp_path) == before + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert snapshot(tmp_path) == { + ".github/skills/user-owned/SKILL.md": b"user", + ".claude/skills/speckit-sample-test/examples/example.txt": b"generated\n", + } + + +@pytest.mark.parametrize("kind", ["extensions", "presets"]) +def test_missing_contribution_registry_keeps_ownership_metadata(tmp_path, kind): + write(tmp_path, f".specify/{kind}/sample/manifest.yml") + before = snapshot(tmp_path) + with pytest.raises(DestroyError, match="Missing"): + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert snapshot(tmp_path) == before + + +def test_invalid_policy_blocks_cleanup(tmp_path): + write(tmp_path, ".specify/init-options.json", b'{"allow_symlinks":"yes"}') + before = snapshot(tmp_path) + with pytest.raises(DestroyError, match="JSON boolean"): + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert snapshot(tmp_path) == before + + +def test_external_contribution_layout_never_executes_adapter_code(tmp_path, monkeypatch): + from specify_cli.agents import CommandRegistrar + + def unexpected_formatter(name): + pytest.fail("Executed an external adapter") + + monkeypatch.setitem(CommandRegistrar.AGENT_CONFIGS, "sample-agent", { + "dir": ".sample/commands", "extension": ".md", "format_name": unexpected_formatter, + }) + write(tmp_path, ".specify/extensions/.registry", json.dumps({ + "extensions": {"sample": { + "registered_commands": {"sample-agent": ["speckit.sample.test"]}, + }}, + }).encode()) + before = snapshot(tmp_path) + with pytest.raises(DestroyError, match="external integration"): + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert snapshot(tmp_path) == before + + +@pytest.mark.parametrize("hashes", [ + [], {"../outside": "a" * 64}, {".git/config": "a" * 64}, + {".custom/skills/sample/SKILL.md": "invalid"}, +]) +def test_invalid_generic_hash_records_block_cleanup(tmp_path, hashes): + write(tmp_path, ".specify/extensions/.registry", json.dumps({ + "extensions": {"sample": { + "registered_commands": {}, "registered_skills": ["sample"], + "generic_artifact_hashes": hashes, + }}, + }).encode()) + before = snapshot(tmp_path) + with pytest.raises(DestroyError): + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert snapshot(tmp_path) == before + + +def test_flat_skill_records_preserve_another_agents_unrelated_skill(tmp_path): + write(tmp_path, ".specify/extensions/.registry", json.dumps({ + "extensions": {"sample": { + "registered_commands": {}, + "registered_skills": ["speckit-sample-task"], + }}, + }).encode()) + write(tmp_path, ".github/skills/speckit-sample-task/SKILL.md", + b"---\nmetadata:\n source: extension:sample\n---\nOwned\n") + unrelated = write(tmp_path, ".claude/skills/speckit-sample-task/SKILL.md", + b"---\nmetadata:\n source: user:manual\n---\nUnrelated\n") + notes = write(tmp_path, ".claude/skills/speckit-sample-task/notes.txt", b"user notes") + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert unrelated.read_bytes().endswith(b"Unrelated\n") + assert notes.read_bytes() == b"user notes" + assert not (tmp_path / ".github/skills/speckit-sample-task").exists() + + +def test_destroy_runs_event_teardown_nondefault_first_and_preserves_user_settings(tmp_path, monkeypatch): + from specify_cli import events + from specify_cli.integration_state import write_integration_json + from specify_cli.integrations import get_integration + + write(tmp_path, ".specify/init-options.json", b'{"ai":"claude","script":"py"}') + write_integration_json( + tmp_path, version="1.2.3", integration_key="claude", + installed_integrations=["claude", "gemini"], + ) + settings = write(tmp_path, ".claude/settings.json", json.dumps({ + "userSetting": True, + "hooks": {"PreToolUse": [{"matcher": "*", "hooks": [ + {"type": "command", "command": "user-command"}, + ]}]}, + }).encode()) + for key in ("claude", "gemini"): + record = manifest(tmp_path, key, f".{key}/skills/speckit-plan/SKILL.md") + get_integration(key).emit_events( + tmp_path, record, {"pre_tool_use": [{"command": "speckit.test"}]}, + ) + record.save() + dispatcher = tmp_path / ".specify/events.py" + assert dispatcher.is_file() + cleanups = [] + original = events._cleanup_shared_dispatcher + + def observe(integration, root, record, **kwargs): + original(integration, root, record, **kwargs) + cleanups.append((integration.key, dispatcher.exists())) + + monkeypatch.setattr(events, "_cleanup_shared_dispatcher", observe) + destroy_project(DestroyRequest(tmp_path, confirmed=True)) + assert cleanups == [("gemini", True), ("claude", False)] + remaining = json.loads(settings.read_text()) + assert remaining["userSetting"] is True + assert "user-command" in settings.read_text() + assert "_speckit" not in settings.read_text() + assert not (tmp_path / ".specify").exists() + + +def test_destroy_rejects_added_paths_outside_external_ownership_binding(tmp_path, monkeypatch): + from specify_cli.integrations.installer import _grant_trust + + home = tmp_path / "home" + home.mkdir() + root = tmp_path / "project" + root.mkdir() + monkeypatch.setattr(Path, "home", lambda: home) + write(root, ".specify/integration.json", b'{"integration":"sample-agent"}') + record = manifest(root, "sample-agent", ".sample/skills/speckit-plan/SKILL.md") + package = { + "id": "sample-agent", "name": "Sample Agent", "version": "1.0.0", + "description": "sample", "catalog": "local", + "download_url": "https://example.com/sample.zip", "requires": {}, + "files": {"__init__.py": "a" * 64, "integration.yml": "b" * 64}, + "registrar_config": { + "dir": ".sample/skills", "extension": "/SKILL.md", + "format": "markdown", "args": "$ARGUMENTS", "invoke_separator": "-", + }, + } + write(root, ".specify/integrations/packages.json", json.dumps({ + "schema_version": "1.0", "packages": {"sample-agent": package}, + }).encode()) + _grant_trust(root, "sample-agent", package, record_ownership=True) + write(root, "src/app.py", b"application source") + record.record_existing("src/app.py") + record.save() + before = snapshot(root) + with pytest.raises(DestroyError, match="ownership"): + destroy_project(DestroyRequest(root, confirmed=True, force=True)) + assert snapshot(root) == before + + +def test_destroy_preserves_claims_when_native_event_config_is_unreadable(tmp_path): + from specify_cli.integrations import get_integration + + record = manifest(tmp_path, "claude", ".claude/skills/speckit-plan/SKILL.md") + get_integration("claude").emit_events( + tmp_path, record, {"pre_tool_use": [{"command": "speckit.test"}]}, + ) + record.save() + write(tmp_path, ".claude/settings.json", b"{malformed native config") + before = snapshot(tmp_path) + with pytest.raises(DestroyError, match="native event cleanup"): + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert snapshot(tmp_path) == before + + +def test_destroy_does_not_purge_a_dispatcher_preserved_by_event_teardown(tmp_path): + from specify_cli.integrations import get_integration + + record = manifest(tmp_path, "claude", ".claude/skills/speckit-plan/SKILL.md") + get_integration("claude").emit_events( + tmp_path, record, {"pre_tool_use": [{"command": "speckit.test"}]}, + ) + record.save() + dispatcher = write(tmp_path, ".specify/events.py", b"# customized dispatcher") + with pytest.raises(DestroyError, match="preserved .specify/events.py"): + destroy_project(DestroyRequest(tmp_path, confirmed=True)) + assert dispatcher.read_bytes() == b"# customized dispatcher" + retained = IntegrationManifest.load("claude", tmp_path) + assert ".specify/events.py" in retained.files + assert (tmp_path / ".specify/integration.json").exists() + + +@pytest.mark.parametrize("key", ["claude", "gemini", "codex", "vibe", "opencode", "copilot"]) +def test_destroy_cleans_native_event_formats_without_deleting_user_config(tmp_path, key): + from specify_cli.integrations import get_integration + + integration = get_integration(key) + config = integration.events_config_file + is_toml = integration.events_format in {"toml", "toml-vibe"} + write(tmp_path, config, b"user_setting = true\n" if is_toml else b'{"userSetting":true}') + record = manifest(tmp_path, key, f".test/{key}/SKILL.md") + integration.emit_events(tmp_path, record, {"pre_tool_use": [{"command": "speckit.test"}]}) + record.save() + assert config in record.files + result = destroy_project(DestroyRequest(tmp_path, confirmed=True)) + assert config in result.updated + content = (tmp_path / config).read_text() + assert "user_setting" in content or "userSetting" in content + assert "__speckit_event__" not in content + assert "speckit_marker" not in content + assert "speckit-events.ts" not in content + assert not (tmp_path / ".specify").exists() + + +def test_force_can_remove_a_modified_owned_dispatcher_after_native_cleanup(tmp_path): + from specify_cli.integrations import get_integration + + record = manifest(tmp_path, "claude", ".claude/skills/speckit-plan/SKILL.md") + get_integration("claude").emit_events( + tmp_path, record, {"pre_tool_use": [{"command": "speckit.test"}]}, + ) + record.save() + write(tmp_path, ".specify/events.py", b"# customized dispatcher") + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert not (tmp_path / ".specify").exists() + + +def test_later_cleanup_failure_reports_prior_uninstalls_and_retains_remaining_state(tmp_path): + from specify_cli.integration_state import try_read_integration_json + from specify_cli.integrations import get_integration + + for key in ("claude", "gemini"): + record = manifest(tmp_path, key, f".{key}/skills/speckit-plan/SKILL.md") + get_integration(key).emit_events( + tmp_path, record, {"pre_tool_use": [{"command": "speckit.test"}]}, + ) + record.save() + write(tmp_path, ".specify/events.py", b"# customized dispatcher") + with pytest.raises(DestroyError) as caught: + destroy_project(DestroyRequest(tmp_path, confirmed=True)) + assert caught.value.details["uninstalled_integrations"] == ["gemini"] + state, error = try_read_integration_json(tmp_path) + assert error is None + assert state["installed_integrations"] == ["claude"] + assert (tmp_path / ".specify/integrations/claude.manifest.json").exists() + + +@pytest.mark.parametrize("force", [False, True]) +def test_destroy_preserves_constitution_claimed_by_integration_manifest(tmp_path, force): + record = manifest(tmp_path, "claude", ".claude/skills/speckit-plan/SKILL.md") + content = b"# Keep these principles\r\n" + record.record_file(CONSTITUTION, content) + record.save() + preview = destroy_project(DestroyRequest(tmp_path, dry_run=True, preserve_constitution=True)) + assert preview.preserved == (CONSTITUTION,) + result = destroy_project(DestroyRequest( + tmp_path, confirmed=True, force=force, preserve_constitution=True, + )) + assert (tmp_path / CONSTITUTION).read_bytes() == content + assert result.preserved == (CONSTITUTION,) + assert not (tmp_path / ".specify/integration.json").exists() + + +def test_destroy_teardown_failure_retains_manifest_and_allows_retry(tmp_path, monkeypatch): + home = tmp_path / "home" + home.mkdir() + root = tmp_path / "project" + root.mkdir() + monkeypatch.setattr(Path, "home", lambda: home) + record = manifest(root, "hermes", ".hermes/skills/speckit-plan/SKILL.md") + record.record_file(CONSTITUTION, b"# Preserve even across failed teardown\n") + record.save() + global_skill = write(home, ".hermes/skills/speckit-plan/SKILL.md") + original = record.manifest_path.read_bytes() + iterdir = Path.iterdir + + def denied(path): + if path == home / ".hermes/skills": + raise PermissionError("simulated unreadable global skills directory") + return iterdir(path) + + with monkeypatch.context() as failure: + failure.setattr(Path, "iterdir", denied) + with pytest.raises(DestroyError, match="unreadable global skills directory"): + destroy_project(DestroyRequest(root, confirmed=True, preserve_constitution=True)) + assert record.manifest_path.read_bytes() == original + state = json.loads((root / ".specify/integration.json").read_text()) + assert state["installed_integrations"] == ["hermes"] + assert global_skill.exists() + assert (root / CONSTITUTION).read_bytes() == b"# Preserve even across failed teardown\n" + destroy_project(DestroyRequest(root, confirmed=True, preserve_constitution=True)) + assert not global_skill.exists() + assert not record.manifest_path.exists() + assert (root / CONSTITUTION).is_file() + + +@pytest.mark.parametrize("force", [False, True]) +def test_real_hermes_with_extension_previews_and_cleans_declared_home_scope(tmp_path, monkeypatch, force): + home = tmp_path / "home" + home.mkdir() + root = tmp_path / "project" + monkeypatch.setattr(Path, "home", lambda: home) + runner = CliRunner() + initialized = runner.invoke(app, [ + "init", str(root), "--non-interactive", "--ignore-agent-tools", + "--integration", "hermes", "--script", "py", "--extension", "git", + ]) + assert initialized.exit_code == 0, initialized.output + unrelated = write(home, ".hermes/skills/user-skill/SKILL.md", b"unrelated global skill") + before = snapshot(tmp_path) + preview = runner.invoke(app, ["destroy", "--project", str(root), "--dry-run", "--json"]) + assert preview.exit_code == 0, preview.output + assert preview.stderr == "" + assert json.loads(preview.stdout)["global_scopes"] == [str(home / ".hermes/skills")] + global_files = json.loads(preview.stdout)["global_files"] + assert any("speckit-git-" in name for name in global_files) + assert all(Path(name).is_relative_to(home / ".hermes/skills") for name in global_files) + assert snapshot(tmp_path) == before + destroyed = runner.invoke(app, [ + "destroy", "--project", str(root), "--yes", "--json", *(["--force"] if force else []), + ]) + assert destroyed.exit_code == 0, destroyed.output + assert destroyed.stderr == "" + assert not list((home / ".hermes/skills").glob("speckit-*")) + assert unrelated.read_bytes() == b"unrelated global skill" + assert snapshot(root) == {} + + +def test_destroy_preserves_constitution_claimed_by_multiple_integrations(tmp_path): + content = b"# Project principles\r\n" + for key in ("claude", "gemini"): + record = manifest(tmp_path, key, f".{key}/skills/speckit-plan/SKILL.md") + record.record_file(CONSTITUTION, content) + record.save() + destroy_project(DestroyRequest( + tmp_path, confirmed=True, force=True, preserve_constitution=True, + )) + assert snapshot(tmp_path) == {CONSTITUTION: content} + + +def test_global_contribution_scope_requires_an_installed_integration(tmp_path, monkeypatch): + home = tmp_path / "home" + home.mkdir() + root = tmp_path / "project" + root.mkdir() + monkeypatch.setattr(Path, "home", lambda: home) + write(root, ".specify/extensions/.registry", json.dumps({ + "extensions": {"sample": { + "registered_commands": {"hermes": ["speckit.sample.check"]}, + }}, + }).encode()) + outside = write(home, ".hermes/skills/speckit-sample-check/SKILL.md", b"unrelated home skill") + before = snapshot(tmp_path) + with pytest.raises(DestroyError, match="outside its declared uninstall scope"): + destroy_project(DestroyRequest(root, confirmed=True, force=True)) + assert snapshot(tmp_path) == before + assert outside.read_bytes() == b"unrelated home skill" + + +def test_hermes_contribution_preview_rejects_a_symlinked_global_scope(tmp_path, monkeypatch): + home = tmp_path / "home" + home.mkdir() + root = tmp_path / "project" + root.mkdir() + monkeypatch.setattr(Path, "home", lambda: home) + manifest(root, "hermes", ".hermes/skills/speckit-plan/SKILL.md") + write(root, ".specify/extensions/.registry", json.dumps({ + "extensions": {"sample": { + "registered_commands": {"hermes": ["speckit.sample.check"]}, + }}, + }).encode()) + outside = home / "unrelated" + write(home, "unrelated/speckit-sample-check/SKILL.md", b"must survive") + (home / ".hermes").mkdir() + symlink(home / ".hermes/skills", outside, directory=True) + before = snapshot(tmp_path) + with pytest.raises(DestroyError, match="Symlinked"): + destroy_project(DestroyRequest(root, confirmed=True, force=True)) + assert snapshot(tmp_path) == before + + +def test_global_contribution_cleanup_must_complete_before_infrastructure_purge(tmp_path, monkeypatch): + from specify_cli.integrations import get_integration + from specify_cli.integrations.base import IntegrationBase + + home = tmp_path / "home" + home.mkdir() + root = tmp_path / "project" + root.mkdir() + monkeypatch.setattr(Path, "home", lambda: home) + manifest(root, "hermes", ".hermes/skills/speckit-plan/SKILL.md") + write(root, ".specify/extensions/.registry", json.dumps({ + "extensions": {"sample": { + "registered_commands": {"hermes": ["speckit.sample.check"]}, + }}, + }).encode()) + global_skill = write(home, ".hermes/skills/speckit-sample-check/SKILL.md") + integration = get_integration("hermes") + monkeypatch.setattr(integration, "teardown", lambda root, record, **kwargs: IntegrationBase.teardown( + integration, root, record, **kwargs, + )) + monkeypatch.setattr( + "specify_cli.integrations._operation_uninstall.unregister_integration_artifacts", + lambda *args, **kwargs: (), + ) + with pytest.raises(DestroyError, match="preserved generated artifacts") as caught: + destroy_project(DestroyRequest(root, confirmed=True)) + assert str(global_skill) in caught.value.details["paths"] + assert (root / ".specify/extensions/.registry").exists() + assert global_skill.exists() + + +@pytest.mark.parametrize("flags,keep_constitution,keep_specs", [ + ([], False, False), + (["--preserve-constitution"], True, False), + (["--preserve-specs"], False, True), + (["--preserve-constitution", "--preserve-specs"], True, True), + (["--preserve-all"], True, True), +]) +@pytest.mark.parametrize("force", [False, True]) +@pytest.mark.parametrize("integration,options", [ + ("copilot", None), ("claude", None), ("gemini", None), + ("generic", "--commands-dir .custom/commands"), + ("generic", "--commands-dir .custom/skills --skills"), + ("hermes", None), +]) +def test_preservation_matrix_on_initialized_throwaway_projects( + tmp_path, monkeypatch, flags, keep_constitution, keep_specs, force, integration, options, +): + from specify_cli.integrations import get_integration + + home = tmp_path / "home" + home.mkdir() + monkeypatch.setattr(Path, "home", lambda: home) + root = tmp_path / "project" + runner = CliRunner() + args = [ + "init", str(root), "--non-interactive", "--ignore-agent-tools", + "--integration", integration, "--script", "py", "--extension", "git", + ] + if force: + args += ["--preset", "lean"] + if options: + args.append(f"--integration-options={options}") + initialized = runner.invoke(app, args) + assert initialized.exit_code == 0, initialized.output + constitution = write(root, CONSTITUTION, b"# Project principles\r\n") + spec = write(root, "specs/001-feature/spec.md", b"# Specification\r\n") + source = write(root, "src/app.py", b"source\n") + unrelated = write(root, "specs-other/keep.md", b"not part of specs/") + external = write(tmp_path, "outside-specs/spec.md", b"external specification") + monkeypatch.setenv("SPECIFY_FEATURE_DIRECTORY", str(external.parent)) + record = IntegrationManifest.load(integration, root) + # Track both artifacts to prove teardown cannot override the preservation policy. + record.record_existing(CONSTITUTION) + record.record_existing("specs/001-feature/spec.md") + agent = get_integration(integration) + if agent.supports_events(): + agent.emit_events(root, record, {"pre_tool_use": [{"command": "speckit.git.validate"}]}) + record.save() + before = snapshot(tmp_path) + command = [ + "destroy", "--project", str(root), "--json", *flags, *(["--force"] if force else []), + ] + preview = runner.invoke(app, [*command, "--dry-run"]) + assert preview.exit_code == 0, preview.output + assert preview.stderr == "" + assert snapshot(tmp_path) == before + destroyed = runner.invoke(app, [*command, "--yes"]) + assert destroyed.exit_code == 0, destroyed.output + assert destroyed.stderr == "" + assert constitution.exists() is keep_constitution + assert spec.exists() is keep_specs + if keep_constitution: + assert constitution.read_bytes() == b"# Project principles\r\n" + assert sorted(path.relative_to(root / ".specify").as_posix() + for path in (root / ".specify").rglob("*") if path.is_file()) == ["memory/constitution.md"] + else: + assert not (root / ".specify").exists() + if keep_specs: + assert spec.read_bytes() == b"# Specification\r\n" + else: + assert not (root / "specs").exists() + assert source.read_bytes() == b"source\n" + assert unrelated.read_bytes() == b"not part of specs/" + assert external.read_bytes() == b"external specification" + assert not (root / ".specify/events.py").exists() + assert not list((home / ".hermes/skills").glob("speckit-*")) + after = snapshot(tmp_path) + repeated = runner.invoke(app, [*command, "--yes"]) + assert repeated.exit_code == 0, repeated.output + assert json.loads(repeated.stdout)["removed"] == [] + assert snapshot(tmp_path) == after + + +@pytest.mark.parametrize("preserve,allow_symlinks", [(True, False), (False, False), (False, True)]) +def test_specs_symlink_is_never_traversed(tmp_path, preserve, allow_symlinks): + root = tmp_path / "project" + root.mkdir() + write(root, ".specify/init-options.json", json.dumps({"allow_symlinks": allow_symlinks}).encode()) + external = write(tmp_path, "outside/spec.md", b"external specification") + symlink(root / "specs", external.parent, directory=True) + before = snapshot(tmp_path) + request = DestroyRequest(root, confirmed=True, preserve_specs=preserve, force=True) + if not preserve and not allow_symlinks: + with pytest.raises(DestroyError, match="symlink"): + destroy_project(request) + assert snapshot(tmp_path) == before + else: + destroy_project(request) + assert (root / "specs").is_symlink() is preserve + assert external.read_bytes() == b"external specification" + + +def test_default_destroy_does_not_delete_specs_without_project_marker(tmp_path): + spec = write(tmp_path, "specs/001-feature/spec.md", b"not an initialized project") + result = destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert not result.removed + assert spec.read_bytes() == b"not an initialized project" + + +def test_non_directory_specs_entry_is_not_deleted_by_default(tmp_path): + write(tmp_path, ".specify/config") + entry = write(tmp_path, "specs", b"unrelated file") + before = snapshot(tmp_path) + with pytest.raises(DestroyError, match="specs must be a directory"): + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert snapshot(tmp_path) == before + assert entry.read_bytes() == b"unrelated file" + + +def test_specs_deletion_failure_retains_configuration_and_can_be_retried(tmp_path, monkeypatch): + from specify_cli.file_helper import FileHelper + + config = write(tmp_path, ".specify/config") + spec = write(tmp_path, "specs/001-feature/spec.md") + delete = FileHelper.delete + + def fail(self, path, **kwargs): + if path == "specs/001-feature/spec.md": + raise PermissionError("spec is locked") + return delete(self, path, **kwargs) + + with monkeypatch.context() as failure: + failure.setattr(FileHelper, "delete", fail) + with pytest.raises(DestroyError, match="spec is locked"): + destroy_project(DestroyRequest(tmp_path, confirmed=True)) + assert config.exists() + assert spec.exists() + destroy_project(DestroyRequest(tmp_path, confirmed=True)) + assert not (tmp_path / ".specify").exists() + assert not (tmp_path / "specs").exists() + + +@pytest.mark.parametrize("preserve_constitution,preserve_specs,preserve_all", [ + (True, False, False), (False, True, False), (True, True, False), (False, False, True), +]) +def test_preservation_is_independent_of_force_for_modified_integration_files( + tmp_path, preserve_constitution, preserve_specs, preserve_all, +): + record = manifest(tmp_path) + generated = write(tmp_path, next(iter(record.files)), b"user-modified generated command") + constitution = write(tmp_path, CONSTITUTION, b"principles") + specs = write(tmp_path, "specs/001-feature/spec.md", b"specification") + request = DestroyRequest( + tmp_path, confirmed=True, preserve_constitution=preserve_constitution, + preserve_specs=preserve_specs, preserve_all=preserve_all, + ) + before = snapshot(tmp_path) + with pytest.raises(DestroyError) as caught: + destroy_project(request) + assert caught.value.code == "modified_files" + assert snapshot(tmp_path) == before + destroy_project(DestroyRequest( + tmp_path, confirmed=True, force=True, preserve_constitution=preserve_constitution, + preserve_specs=preserve_specs, preserve_all=preserve_all, + )) + assert not generated.exists() + assert constitution.exists() is (preserve_constitution or preserve_all) + assert specs.exists() is (preserve_specs or preserve_all) + + +def test_preserved_specs_are_not_traversed_or_read(tmp_path, monkeypatch): + write(tmp_path, ".specify/config") + spec = write(tmp_path, "specs/001-feature/spec.md", b"private specification") + read_bytes = Path.read_bytes + + def deny_spec_reads(path): + if path == spec: + raise PermissionError("preserved specifications must not be read") + return read_bytes(path) + + with monkeypatch.context() as preservation: + preservation.setattr(Path, "read_bytes", deny_spec_reads) + destroy_project(DestroyRequest(tmp_path, confirmed=True, preserve_specs=True)) + assert spec.read_bytes() == b"private specification" + assert not (tmp_path / ".specify").exists() From b7b7297254c4550fff1eefe3a76242127dc3c6af Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Mon, 26 Oct 2026 08:15:21 -0500 Subject: [PATCH 2/4] fix(destroy): address teardown review and CI lint Preflight all contribution cleanup names and dispatcher ownership/content, preserve symlinked constitution claims lexically, reject incomplete legacy inventories before mutation, retain package registries through purge failures, and allow ordinary external uninstall without a manifest. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/reference/core.md | 10 ++ src/specify_cli/__init__.py | 2 +- src/specify_cli/_operation_destroy.py | 43 +++++- src/specify_cli/agents.py | 14 +- src/specify_cli/events/__init__.py | 14 +- .../integrations/_operation_uninstall.py | 2 +- src/specify_cli/integrations/manifest.py | 13 +- .../integrations/test_installed_adapters.py | 14 ++ tests/specify_cli/test_operation_destroy.py | 129 +++++++++++++++++- 9 files changed, 213 insertions(+), 28 deletions(-) diff --git a/docs/reference/core.md b/docs/reference/core.md index c2d7ff1d98..58e4ccc0af 100644 --- a/docs/reference/core.md +++ b/docs/reference/core.md @@ -158,6 +158,10 @@ switching defaults merely to remove the next integration. Their ordinary teardow removes Spec Kit's merged native event entries while preserving user entries and settings. The last installed integration referencing an owned `.specify/events.py` dispatcher releases and removes it under the existing ownership and force rules. +Preview checks the dispatcher against its ownership records and the lifecycle's +accepted generated content. A modified managed dispatcher requires force before +any integration is uninstalled; missing, partial, or recovered ownership does not +become deletion authority through force. If teardown preserves the dispatcher or leaves cleanup unresolved, destroy stops and retains recovery metadata instead of purging the remaining `.specify` tree. @@ -190,11 +194,17 @@ metadata is retained for recovery. Repeating destroy with the same preservation flags succeeds without further changes. A retained constitution by itself is a no-op with `--preserve-constitution` or `--preserve-all`; without those flags, its deletion still requires confirmation. +Extension and preset registries are retained until their package files and +directories have been removed, so a failed package purge can be retried. If teardown raises after deleting its manifest, destroy restores the original ownership manifest so the recorded integration can be retried after the error is resolved. This preserves recovery evidence; it does not undo files that were already removed outside a lifecycle transaction. +Legacy setups with only an `init-options.json` integration selection and no +`integration.json` inventory are rejected during preview. Re-run `specify init` +for the selected integration to migrate that inventory before using destroy. + For removing just one integration while retaining Spec Kit tooling, use [`specify integration uninstall`](../integrations/cli.md#uninstall-an-integration). `destroy` is currently CLI-only; its MCP inventory records it as unavailable. diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 4e518f718d..0827ea50eb 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -391,7 +391,7 @@ def _print_cli_warning( # ===== Root Commands ===== from . import command_check as _command_check # noqa: E402 -from . import command_destroy as _command_destroy +from . import command_destroy as _command_destroy # noqa: E402 from . import command_init as _command_init # noqa: E402 from . import command_mcp as _command_mcp # noqa: E402 from . import command_version as _command_version # noqa: E402 diff --git a/src/specify_cli/_operation_destroy.py b/src/specify_cli/_operation_destroy.py index b7dceecc41..9233df6228 100644 --- a/src/specify_cli/_operation_destroy.py +++ b/src/specify_cli/_operation_destroy.py @@ -346,7 +346,10 @@ def plan_destroy(request: DestroyRequest) -> DestroyPlan: if state is None: legacy_agent = read_project_options(root).get("ai") if isinstance(legacy_agent, str) and legacy_agent: - installed = [legacy_agent] + raise ValueError( + "Legacy integration inventory has no .specify/integration.json. " + "Re-run specify init for the recorded integration to migrate before destroy.", + ) manifests = sorted( name for name in leaves if name.startswith(".specify/integrations/") and name.endswith(".manifest.json") @@ -360,6 +363,7 @@ def plan_destroy(request: DestroyRequest) -> DestroyPlan: raise ValueError(f"Missing manifest for '{key}'; repair or uninstall that integration first") expected: dict[str, set[str]] = {} + dispatcher_owners: list[tuple[str, bool, str]] = [] updated: set[str] = set() for manifest_path in manifests: key = Path(manifest_path).name.removesuffix(".manifest.json") @@ -380,11 +384,13 @@ def plan_destroy(request: DestroyRequest) -> DestroyPlan: raise ValueError("Manifests must record files, not the .specify root") if not re.fullmatch(r"[0-9a-fA-F]{64}", digest): raise ValueError(f"Invalid manifest digest: {name}") - if relative.startswith(".specify/"): - continue mode = manifest.ownership_modes[name] if bindings.get(key) is not None: mode = bindings[key].get("ownership_modes", {}).get(name, "shared") + if relative == ".specify/events.py": + dispatcher_owners.append((mode, manifest.is_recovered(name), digest)) + if relative.startswith(".specify/"): + continue if mode != "whole": if (root / name).exists() or (root / name).is_symlink(): from .integrations import ( @@ -425,6 +431,19 @@ def plan_destroy(request: DestroyRequest) -> DestroyPlan: for name, digests in contribution_hashes.items(): expected.setdefault(name, set()).update(digests) requiring_force: set[str] = set() + dispatcher = root / ".specify/events.py" + if dispatcher.exists() or dispatcher.is_symlink(): + from .events import dispatcher_content_matches + + if not dispatcher_owners or any( + mode == "partial" or recovered for mode, recovered, _ in dispatcher_owners + ): + raise ValueError("Cannot prove managed dispatcher ownership of .specify/events.py; resolve it before destroy") + if not any( + dispatcher_content_matches(files.read_bytes(".specify/events.py"), digest) + for _, _, digest in dispatcher_owners + ): + requiring_force.add(".specify/events.py") for candidate in sorted(contributed | {root / name for name in expected}): relative = _relative(root, candidate) if relative == CONSTITUTION or relative.startswith(".specify/"): @@ -526,12 +545,28 @@ def destroy_project(request: DestroyRequest) -> DestroyResult: raise DestroyError("cleanup_incomplete", "The event lifecycle preserved .specify/events.py; resolve it before destroy.") # Re-inventory only after teardown has released all integration claims. leaves, directories, _ = _project_artifacts(files, request) - for name in sorted(leaves, key=lambda name: (name.startswith(".specify/"), name)): + registries = {name for name in leaves if Path(name).name == ".registry"} + for name in sorted(leaves - registries, key=lambda name: (name.startswith(".specify/"), name)): files.delete(name) removed.append(name) directories.update(plan.directories) if CONSTITUTION in plan.preserved: directories.difference_update({".specify", ".specify/memory"}) + metadata_parents = { + parent.as_posix() for name in registries for parent in Path(name).parents + } + # Package trees must disappear before their ownership registries are discarded. + for name in sorted(directories - metadata_parents, key=lambda name: (-len(Path(name).parts), name)): + try: + files.delete(name) + except FileNotFoundError: + continue + except OSError as exc: + if exc.errno not in {errno.ENOTEMPTY, errno.EEXIST} or name.startswith(".specify/"): + raise + for name in sorted(registries): + files.delete(name) + removed.append(name) for name in sorted(directories, key=lambda name: (-len(Path(name).parts), name)): try: files.delete(name) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 47944113de..372619e6f9 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -1419,11 +1419,17 @@ def registered_artifact_paths( paths: set[Path] = set() for name in commands: output_name = self._compute_output_name(agent_name, name, config) + names = [output_name] + if output_name != name: + names.append(name) for destination in destinations: - if config["extension"] == "/SKILL.md": - paths.add(destination / output_name / "SKILL.md") - else: - paths.add(destination / f"{output_name}{config['extension']}") + for cleanup_name in names: + if not self._is_safe_command_name(cleanup_name) or cleanup_name in {"", ".", ".."}: + raise ValueError(f"Invalid formatted artifact name: {cleanup_name!r}") + if config["extension"] == "/SKILL.md": + paths.add(destination / cleanup_name / "SKILL.md") + else: + paths.add(destination / f"{cleanup_name}{config['extension']}") if agent_name == "copilot": paths.add(project_root / ".github/prompts" / f"{name}.prompt.md") if skills: diff --git a/src/specify_cli/events/__init__.py b/src/specify_cli/events/__init__.py index c1676e6944..7034c805ff 100644 --- a/src/specify_cli/events/__init__.py +++ b/src/specify_cli/events/__init__.py @@ -1692,6 +1692,14 @@ def _other_event_integrations_reference_dispatcher( return False +def dispatcher_content_matches(content: bytes, expected_hash: str) -> bool: + """Accept the recorded copy or an exact regenerated LF/CRLF dispatcher.""" + return hashlib.sha256(content).hexdigest() == expected_hash or content in ( + _EVENTS_DISPATCHER_TEMPLATE.encode("utf-8"), + _EVENTS_DISPATCHER_TEMPLATE.replace("\n", "\r\n").encode("utf-8"), + ) + + def _cleanup_shared_dispatcher( integration: IntegrationBase, project_root: Path, manifest: IntegrationManifest, *, @@ -1738,11 +1746,7 @@ def _cleanup_shared_dispatcher( return # Another integration may have regenerated the shared file since # this manifest recorded it. Accept only exact generated LF/CRLF. - matches = hashlib.sha256(content).hexdigest() == expected_hash or content in ( - _EVENTS_DISPATCHER_TEMPLATE.encode("utf-8"), - _EVENTS_DISPATCHER_TEMPLATE.replace("\n", "\r\n").encode("utf-8"), - ) - if not matches: + if not dispatcher_content_matches(content, expected_hash): logger.warning("Preserving modified dispatcher: %s", dispatcher_path) return _unlink_file(dispatcher_path, missing_ok=True) diff --git a/src/specify_cli/integrations/_operation_uninstall.py b/src/specify_cli/integrations/_operation_uninstall.py index a53857f321..7523946b7c 100644 --- a/src/specify_cli/integrations/_operation_uninstall.py +++ b/src/specify_cli/integrations/_operation_uninstall.py @@ -275,7 +275,7 @@ def _uninstall_with_lifecycle(request: IntegrationUninstallRequest) -> Integrati + ", ".join(preserved) + "; manual event cleanup may be needed.", ) manifest.save() - elif binding is not None: + elif binding is not None and (root / ".specify/integrations" / f"{key}.manifest.json").exists(): # Project metadata cannot promote a locally partial/shared claim. manifest = IntegrationManifest.load(key, root) for name in manifest.files: diff --git a/src/specify_cli/integrations/manifest.py b/src/specify_cli/integrations/manifest.py index cd1f430c43..f6830dacff 100644 --- a/src/specify_cli/integrations/manifest.py +++ b/src/specify_cli/integrations/manifest.py @@ -272,19 +272,14 @@ def remove(self, rel_path: str | Path) -> bool: Used to keep the manifest consistent after a caller deletes a stale managed file that the current install no longer ships. - Input is normalized through the same lexical pipeline as - ``record_existing`` / ``is_recovered``: absolute paths and paths - containing ``..`` segments are rejected (return ``False``) — such paths - can never be canonical manifest keys, so there is nothing to remove. + Input is matched lexically, without resolving the current filesystem + entry. Absolute paths and paths containing ``..`` segments are rejected + (return ``False``); they cannot be canonical manifest keys. """ rel = Path(rel_path) if rel.is_absolute() or ".." in rel.parts: return False - try: - abs_path = _validate_rel_path(rel, self.project_root) - normalized = abs_path.relative_to(self.project_root).as_posix() - except ValueError: - return False + normalized = rel.as_posix() self._recovered_files.discard(normalized) self._ownership_modes.pop(normalized, None) return self._files.pop(normalized, None) is not None diff --git a/tests/specify_cli/integrations/test_installed_adapters.py b/tests/specify_cli/integrations/test_installed_adapters.py index c51d5aa2e2..fd20e4e7b8 100644 --- a/tests/specify_cli/integrations/test_installed_adapters.py +++ b/tests/specify_cli/integrations/test_installed_adapters.py @@ -200,6 +200,20 @@ def test_destroy_uses_trusted_external_lifecycle_and_contribution_cleanup(tmp_pa assert not (project / ".specify").exists() +def test_healthy_external_uninstall_without_manifest_removes_stale_package(tmp_path, server): + publish(server) + project = catalog_project(tmp_path, server) + install(project) + (project / f".specify/integrations/{KEY}.manifest.json").unlink() + rejected = run(project, ["destroy", "--dry-run", "--json"]) + assert rejected.exit_code == 1 + assert "Missing manifest" in rejected.stderr + result = run(project, ["integration", "uninstall", KEY]) + assert result.exit_code == 0, result.output + assert not read_records(project) + assert not (project / ".specify/integration.json").exists() + + @pytest.mark.parametrize("force", [False, True]) def test_destroy_preserves_constitution_recorded_by_a_trusted_adapter(tmp_path, server, force): body = ''' def setup(self, project_root, manifest, **kwargs): diff --git a/tests/specify_cli/test_operation_destroy.py b/tests/specify_cli/test_operation_destroy.py index 1407a5593e..167eb273e4 100644 --- a/tests/specify_cli/test_operation_destroy.py +++ b/tests/specify_cli/test_operation_destroy.py @@ -358,7 +358,7 @@ def test_invalid_integration_state_blocks_every_deletion(tmp_path, state): def test_legacy_init_options_cannot_hide_missing_manifest(tmp_path): write(tmp_path, ".specify/init-options.json", b'{"ai":"copilot"}') before = snapshot(tmp_path) - with pytest.raises(DestroyError, match="Missing manifest"): + with pytest.raises(DestroyError, match="integration.json"): destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) assert snapshot(tmp_path) == before @@ -601,7 +601,7 @@ def test_destroy_does_not_purge_a_dispatcher_preserved_by_event_teardown(tmp_pat ) record.save() dispatcher = write(tmp_path, ".specify/events.py", b"# customized dispatcher") - with pytest.raises(DestroyError, match="preserved .specify/events.py"): + with pytest.raises(DestroyError, match="retry with --force"): destroy_project(DestroyRequest(tmp_path, confirmed=True)) assert dispatcher.read_bytes() == b"# customized dispatcher" retained = IntegrationManifest.load("claude", tmp_path) @@ -644,7 +644,7 @@ def test_force_can_remove_a_modified_owned_dispatcher_after_native_cleanup(tmp_p assert not (tmp_path / ".specify").exists() -def test_later_cleanup_failure_reports_prior_uninstalls_and_retains_remaining_state(tmp_path): +def test_later_cleanup_failure_reports_prior_uninstalls_and_retains_remaining_state(tmp_path, monkeypatch): from specify_cli.integration_state import try_read_integration_json from specify_cli.integrations import get_integration @@ -654,7 +654,10 @@ def test_later_cleanup_failure_reports_prior_uninstalls_and_retains_remaining_st tmp_path, record, {"pre_tool_use": [{"command": "speckit.test"}]}, ) record.save() - write(tmp_path, ".specify/events.py", b"# customized dispatcher") + def fail(*args, **kwargs): + raise PermissionError("late teardown failure") + + monkeypatch.setattr(get_integration("claude"), "teardown", fail) with pytest.raises(DestroyError) as caught: destroy_project(DestroyRequest(tmp_path, confirmed=True)) assert caught.value.details["uninstalled_integrations"] == ["gemini"] @@ -1014,3 +1017,121 @@ def deny_spec_reads(path): destroy_project(DestroyRequest(tmp_path, confirmed=True, preserve_specs=True)) assert spec.read_bytes() == b"private specification" assert not (tmp_path / ".specify").exists() + + +@pytest.mark.parametrize("force", [False, True]) +def test_tracked_constitution_symlink_preservation(tmp_path, force): + record = manifest(tmp_path, "claude", ".claude/skills/speckit-plan/SKILL.md") + record.record_file(CONSTITUTION, b"original") + record.save() + (tmp_path / CONSTITUTION).unlink() + target = write(tmp_path, "principles.md", b"user principles") + symlink(tmp_path / CONSTITUTION, target) + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=force, preserve_constitution=True)) + assert (tmp_path / CONSTITUTION).is_symlink() + assert target.read_bytes() == b"user principles" + + +def test_dotted_contribution_symlink_is_preflighted_before_uninstall(tmp_path): + manifest(tmp_path, "claude", ".claude/skills/speckit-plan/SKILL.md") + write(tmp_path, ".specify/extensions/.registry", json.dumps({ + "extensions": {"sample": {"registered_commands": {"claude": ["speckit.sample.test"]}}}, + }).encode()) + target = write(tmp_path, "outside/SKILL.md", b"unrelated") + symlink(tmp_path / ".claude/skills/speckit.sample.test", target.parent, directory=True) + before = snapshot(tmp_path) + with pytest.raises(DestroyError, match="symlink"): + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert snapshot(tmp_path) == before + assert target.read_bytes() == b"unrelated" + + +def test_legacy_inventory_is_rejected_during_preview_without_changes(tmp_path): + record = manifest(tmp_path, "claude", ".claude/skills/speckit-plan/SKILL.md") + (tmp_path / ".specify/integration.json").unlink() + write(tmp_path, ".specify/init-options.json", b'{"ai":"claude"}') + before = snapshot(tmp_path) + with pytest.raises(DestroyError, match="integration.json"): + destroy_project(DestroyRequest(tmp_path, dry_run=True)) + assert snapshot(tmp_path) == before + assert record.manifest_path.exists() + + +def test_modified_dispatcher_is_reported_before_any_integration_uninstalls(tmp_path): + from specify_cli.integrations import get_integration + + for key in ("claude", "gemini"): + record = manifest(tmp_path, key, f".{key}/skills/speckit-plan/SKILL.md") + get_integration(key).emit_events(tmp_path, record, {"pre_tool_use": [{"command": "speckit.test"}]}) + record.save() + write(tmp_path, ".specify/events.py", b"# user modified dispatcher") + before = snapshot(tmp_path) + preview = destroy_project(DestroyRequest(tmp_path, dry_run=True)) + assert ".specify/events.py" in preview.requires_force + with pytest.raises(DestroyError) as caught: + destroy_project(DestroyRequest(tmp_path, confirmed=True)) + assert caught.value.code == "modified_files" + assert snapshot(tmp_path) == before + + +@pytest.mark.parametrize("kind", ["extensions", "presets"]) +@pytest.mark.parametrize("failure", ["file", "directory"]) +def test_package_purge_keeps_registry_until_package_is_removed(tmp_path, monkeypatch, kind, failure): + from specify_cli.file_helper import FileHelper + + registry = write(tmp_path, f".specify/{kind}/.registry", json.dumps({kind: {"sample": {}}}).encode()) + package = write(tmp_path, f".specify/{kind}/sample/payload.txt") + delete = FileHelper.delete + blocked = f".specify/{kind}/sample" + ("/payload.txt" if failure == "file" else "") + + def fail(self, path, **kwargs): + if path == blocked: + raise PermissionError("package locked") + return delete(self, path, **kwargs) + + with monkeypatch.context() as fault: + fault.setattr(FileHelper, "delete", fail) + with pytest.raises(DestroyError, match="package locked"): + destroy_project(DestroyRequest(tmp_path, confirmed=True)) + assert registry.exists() + assert package.parent.exists() + destroy_project(DestroyRequest(tmp_path, confirmed=True)) + assert not (tmp_path / ".specify").exists() + + +@pytest.mark.parametrize("newline", [b"\n", b"\r\n"]) +def test_regenerated_dispatcher_with_stale_manifest_hash_needs_no_force(tmp_path, newline): + from specify_cli.integrations import get_integration + + record = manifest(tmp_path, "claude", ".claude/skills/speckit-plan/SKILL.md") + get_integration("claude").emit_events(tmp_path, record, {"pre_tool_use": [{"command": "speckit.test"}]}) + record.save() + path = record.manifest_path + data = json.loads(path.read_text()) + data["files"][".specify/events.py"] = "a" * 64 + path.write_text(json.dumps(data)) + dispatcher = tmp_path / ".specify/events.py" + dispatcher.write_bytes(dispatcher.read_bytes().replace(b"\r\n", b"\n").replace(b"\n", newline)) + preview = destroy_project(DestroyRequest(tmp_path, dry_run=True)) + assert ".specify/events.py" not in preview.requires_force + destroy_project(DestroyRequest(tmp_path, confirmed=True)) + assert not (tmp_path / ".specify").exists() + + +@pytest.mark.parametrize("kind", ["partial", "recovered", "unowned"]) +def test_unproven_dispatcher_cleanup_is_rejected_before_changes(tmp_path, kind): + from specify_cli.integrations import get_integration + + record = manifest(tmp_path, "claude", ".claude/skills/speckit-plan/SKILL.md") + get_integration("claude").emit_events(tmp_path, record, {"pre_tool_use": [{"command": "speckit.test"}]}) + if kind == "partial": + record.set_ownership(".specify/events.py", "partial") + elif kind == "recovered": + record.record_existing(".specify/events.py", recovered=True) + else: + record.remove(".specify/events.py") + record.save() + before = snapshot(tmp_path) + with pytest.raises(DestroyError, match="dispatcher ownership"): + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert snapshot(tmp_path) == before From d5bf78dd7ce3c2cec5f3d71dfa7fa66346ea7a69 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Mon, 26 Oct 2026 09:42:35 -0500 Subject: [PATCH 3/4] fix(destroy): preserve artifact identity and complete owned cleanup Handle case-insensitive artifact preservation without following links, force validated generic contribution cleanup before dropping integration state, resolve stacked preset skill provenance, preflight native settings and the final dispatcher claimant, synchronize hashes after managed preset initialization, finish specs-directory deletion before infrastructure, and expose human recovery details. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/reference/core.md | 14 +- src/specify_cli/_operation_destroy.py | 98 ++++++--- src/specify_cli/_utils.py | 12 ++ src/specify_cli/command_destroy.py | 10 + src/specify_cli/command_init.py | 17 ++ src/specify_cli/extensions/__init__.py | 12 +- .../integrations/_operation_uninstall.py | 36 +++- src/specify_cli/presets/_manager_skills.py | 25 ++- tests/specify_cli/test_command_destroy.py | 18 ++ tests/specify_cli/test_operation_destroy.py | 202 ++++++++++++++++++ 10 files changed, 399 insertions(+), 45 deletions(-) diff --git a/docs/reference/core.md b/docs/reference/core.md index 58e4ccc0af..44d6b8c986 100644 --- a/docs/reference/core.md +++ b/docs/reference/core.md @@ -119,6 +119,8 @@ specify destroy --yes --preserve-all Preserved artifacts keep their original bytes, including when an integration manifest tracks them. Both individual flags can be combined; combining either with `--preserve-all` is harmless. `--force` never overrides preservation. +Preservation recognizes alternate case spellings on case-insensitive filesystems, +including the constitution's parent directories, without following a preserved link. | Option | Description | | --- | --- | @@ -146,6 +148,10 @@ integration lifecycle's registry, provenance-marker, and hash checks. A flat skill name alone does not establish ownership of same-named skills in other agents' directories. `--force` does **not** imply confirmation, establish ownership, override filesystem policy, or authorize deleting shared settings. +For generic integration outputs, force permits removal of modified extension files +only after the recorded command names, output paths, and ownership hashes have +passed validation. Incomplete contribution cleanup retains the integration state +needed to retry. For example: ```bash @@ -159,9 +165,11 @@ removes Spec Kit's merged native event entries while preserving user entries and settings. The last installed integration referencing an owned `.specify/events.py` dispatcher releases and removes it under the existing ownership and force rules. Preview checks the dispatcher against its ownership records and the lifecycle's -accepted generated content. A modified managed dispatcher requires force before +accepted generated content, using the last claimant in uninstall order. A modified managed dispatcher requires force before any integration is uninstalled; missing, partial, or recovered ownership does not become deletion authority through force. +Claimed built-in native settings are also parsed during planning, so malformed +settings stop destruction before another integration is removed. If teardown preserves the dispatcher or leaves cleanup unresolved, destroy stops and retains recovery metadata instead of purging the remaining `.specify` tree. @@ -200,6 +208,10 @@ If teardown raises after deleting its manifest, destroy restores the original ownership manifest so the recorded integration can be retried after the error is resolved. This preserves recovery evidence; it does not undo files that were already removed outside a lifecycle transaction. +Specification files and directories are removed before remaining infrastructure: +a locked `specs/` directory leaves the project marker available for retry. +Human-readable failures list remaining paths and completed file/integration +removals alongside the error; JSON errors expose the same recovery details. Legacy setups with only an `init-options.json` integration selection and no `integration.json` inventory are rejected during preview. Re-run `specify init` diff --git a/src/specify_cli/_operation_destroy.py b/src/specify_cli/_operation_destroy.py index 9233df6228..f7cbd9e7e1 100644 --- a/src/specify_cli/_operation_destroy.py +++ b/src/specify_cli/_operation_destroy.py @@ -13,6 +13,7 @@ from typing import Any from ._file_policy import project_files, read_project_options +from ._utils import same_path_entry from .file_helper import FileHelper, FileHelperError from .integration_state import ( default_integration_key, @@ -140,7 +141,7 @@ def _tree( ): raise FileHelperError(f"Unsupported reparse point: {relative}") if stat.S_ISLNK(mode): - if preserve_constitution and relative == ".specify/memory": + if preserve_constitution and same_path_entry(path, files.root / ".specify/memory"): raise FileHelperError("Cannot preserve constitution through symlinked .specify/memory") if not files.allow_symlinks: raise FileHelperError(f"Refusing symlink during destroy: {relative}") @@ -153,7 +154,7 @@ def _tree( children = sorted(entry.name for entry in entries) for name in children: child = f"{relative}/{name}" - if preserve_constitution and child == CONSTITUTION: + if preserve_constitution and same_path_entry(files.root / child, files.root / CONSTITUTION): if not (path / name).is_file() and not (path / name).is_symlink(): raise ValueError(f"{CONSTITUTION} must be a file") continue @@ -180,7 +181,7 @@ def _project_artifacts( if (files.root / name).exists() or (files.root / name).is_symlink() ) if CONSTITUTION in preserved: - directories.difference_update({".specify", ".specify/memory"}) + directories = _without_constitution_parents(files.root, directories) if "specs" not in selected: specs = files.root / "specs" if specs.exists() and not specs.is_dir() and not specs.is_symlink(): @@ -191,6 +192,13 @@ def _project_artifacts( return leaves, directories, preserved +def _without_constitution_parents(root: Path, directories: set[str]) -> set[str]: + return { + name for name in directories + if not any(same_path_entry(root / name, root / ancestor) for ancestor in (".specify", ".specify/memory")) + } + + def _contribution_paths( root: Path, metadata_files: FileHelper, files: FileHelper, bindings: dict[str, dict | None], global_scopes: dict[str, Path], @@ -203,6 +211,7 @@ def _contribution_paths( hashes: dict[str, set[str]] = {} agents: set[str] = set() global_paths: set[Path] = set() + preset_claims: dict[Path, tuple[set[str], set[str]]] = {} active_generic = read_project_options(root).get("ai") == "generic" for kind in ("extensions", "presets"): relative = f".specify/{kind}/.registry" @@ -221,6 +230,11 @@ def _contribution_paths( for identifier, record in records.items(): if not isinstance(record, dict): raise TypeError(f"Invalid {kind} record: {identifier}") + override_sources: dict[str, str] = {} + if kind == "presets": + from .presets import PresetManager + + override_sources = PresetManager(root).skill_ownership_sources(identifier) generic_hashes = record.get("generic_artifact_hashes", {}) if not isinstance(generic_hashes, dict): raise TypeError(f"Invalid generic_artifact_hashes for {identifier}") @@ -285,6 +299,14 @@ def _contribution_paths( agent, root, commands=[], skills=[skill], registrar_config=config, ) for candidate in candidates: + if kind == "presets": + sources, owners = preset_claims.setdefault(candidate, (set(), set())) + sources.add(f"preset:{identifier}") + override = override_sources.get(skill) + if override: + sources.add(override) + owners.add(agent) + continue relative = _relative(root, candidate / "SKILL.md") try: text = files.read_text(relative) @@ -301,6 +323,21 @@ def _contribution_paths( agents.add(agent) elif not isinstance(skills, list): raise ValueError(f"Cannot establish skill ownership at {candidate}") + for candidate, (sources, owners) in preset_claims.items(): + relative = _relative(root, candidate / "SKILL.md") + try: + content = files.read_text(relative) + except FileNotFoundError: + if candidate.exists(): + raise ValueError(f"Cannot establish skill ownership at {candidate}") + continue + frontmatter, _ = registrar.parse_frontmatter(content) + metadata = frontmatter.get("metadata", {}) if isinstance(frontmatter, dict) else {} + source = metadata.get("source") if isinstance(metadata, dict) else None + if source not in sources: + raise ValueError(f"Cannot establish skill ownership at {candidate}") + paths.add(candidate) + agents.update(owners) return paths, hashes, agents, global_paths @@ -343,6 +380,9 @@ def plan_destroy(request: DestroyRequest) -> DestroyPlan: ): raise ValueError("Invalid installed_integrations") installed = installed_integration_keys(normalize_integration_state(state)) if state else [] + normalized = normalize_integration_state(state or {}) + default = default_integration_key(normalized) + order = tuple(key for key in installed if key != default) + ((default,) if default in installed else ()) if state is None: legacy_agent = read_project_options(root).get("ai") if isinstance(legacy_agent, str) and legacy_agent: @@ -363,7 +403,7 @@ def plan_destroy(request: DestroyRequest) -> DestroyPlan: raise ValueError(f"Missing manifest for '{key}'; repair or uninstall that integration first") expected: dict[str, set[str]] = {} - dispatcher_owners: list[tuple[str, bool, str]] = [] + dispatcher_owners: dict[str, tuple[str, bool, str]] = {} updated: set[str] = set() for manifest_path in manifests: key = Path(manifest_path).name.removesuffix(".manifest.json") @@ -373,6 +413,14 @@ def plan_destroy(request: DestroyRequest) -> DestroyPlan: manifest = IntegrationManifest.load(key, root) if key != "speckit" and key not in installed: raise ValueError(f"Integration '{key}' has a manifest but no installed integration state") + from .integrations import BUILTIN_INTEGRATION_KEYS, get_integration + + integration = get_integration(key) if key in BUILTIN_INTEGRATION_KEYS else None + native_config = getattr(integration, "events_config_file", None) if integration else None + if native_config in manifest.files: + from .events import validate_integration_event_cleanup + + validate_integration_event_cleanup(integration, root) for name, digest in manifest.files.items(): if Path(name).as_posix() != name: raise ValueError(f"Noncanonical manifest path: {name}") @@ -388,17 +436,11 @@ def plan_destroy(request: DestroyRequest) -> DestroyPlan: if bindings.get(key) is not None: mode = bindings[key].get("ownership_modes", {}).get(name, "shared") if relative == ".specify/events.py": - dispatcher_owners.append((mode, manifest.is_recovered(name), digest)) + dispatcher_owners[key] = (mode, manifest.is_recovered(name), digest) if relative.startswith(".specify/"): continue if mode != "whole": if (root / name).exists() or (root / name).is_symlink(): - from .integrations import ( - BUILTIN_INTEGRATION_KEYS, - get_integration, - ) - - integration = get_integration(key) if key in BUILTIN_INTEGRATION_KEYS else None if integration is not None and getattr(integration, "events_config_file", None) != name: raise ValueError( f"Partial/shared ownership at {name}; no native event cleanup owns this file", @@ -435,14 +477,13 @@ def plan_destroy(request: DestroyRequest) -> DestroyPlan: if dispatcher.exists() or dispatcher.is_symlink(): from .events import dispatcher_content_matches - if not dispatcher_owners or any( - mode == "partial" or recovered for mode, recovered, _ in dispatcher_owners - ): + claimants = [key for key in order if key in dispatcher_owners] + if not claimants: raise ValueError("Cannot prove managed dispatcher ownership of .specify/events.py; resolve it before destroy") - if not any( - dispatcher_content_matches(files.read_bytes(".specify/events.py"), digest) - for _, _, digest in dispatcher_owners - ): + mode, recovered, digest = dispatcher_owners[claimants[-1]] + if mode == "partial" or recovered: + raise ValueError("Cannot prove managed dispatcher ownership of .specify/events.py; resolve it before destroy") + if not dispatcher_content_matches(files.read_bytes(".specify/events.py"), digest): requiring_force.add(".specify/events.py") for candidate in sorted(contributed | {root / name for name in expected}): relative = _relative(root, candidate) @@ -465,12 +506,9 @@ def plan_destroy(request: DestroyRequest) -> DestroyPlan: for name in tuple(leaves) + tuple(directories): parent = Path(name).parent while parent != Path("."): - if parent.as_posix() not in {".specify", ".specify/memory"} or CONSTITUTION not in preserved: + if CONSTITUTION not in preserved or _without_constitution_parents(root, {parent.as_posix()}): directories.add(parent.as_posix()) parent = parent.parent - normalized = normalize_integration_state(state or {}) - default = default_integration_key(normalized) - order = tuple(key for key in installed if key != default) + ((default,) if default in installed else ()) return DestroyPlan( root, tuple(sorted(leaves)), tuple(sorted(directories, key=lambda name: (-len(Path(name).parts), name))), @@ -526,7 +564,7 @@ def destroy_project(request: DestroyRequest) -> DestroyResult: uninstalled.append(key) for key in set(plan.contribution_agents) - set(plan.integrations): warnings.extend(unregister_integration_artifacts( - plan.project_directory, key, require_complete=True, + plan.project_directory, key, require_complete=True, force=request.force, )) remaining = [ name for name in plan.files @@ -545,13 +583,23 @@ def destroy_project(request: DestroyRequest) -> DestroyResult: raise DestroyError("cleanup_incomplete", "The event lifecycle preserved .specify/events.py; resolve it before destroy.") # Re-inventory only after teardown has released all integration claims. leaves, directories, _ = _project_artifacts(files, request) + spec_leaves = {name for name in leaves if name == "specs" or name.startswith("specs/")} + spec_directories = {name for name in directories if name == "specs" or name.startswith("specs/")} + for name in sorted(spec_leaves): + files.delete(name) + removed.append(name) + for name in sorted(spec_directories, key=lambda name: (-len(Path(name).parts), name)): + files.delete(name) + leaves.difference_update(spec_leaves) + directories.difference_update(spec_directories) + planned_directories = set(plan.directories) - spec_directories registries = {name for name in leaves if Path(name).name == ".registry"} for name in sorted(leaves - registries, key=lambda name: (name.startswith(".specify/"), name)): files.delete(name) removed.append(name) - directories.update(plan.directories) + directories.update(planned_directories) if CONSTITUTION in plan.preserved: - directories.difference_update({".specify", ".specify/memory"}) + directories = _without_constitution_parents(plan.project_directory, directories) metadata_parents = { parent.as_posix() for name in registries for parent in Path(name).parents } diff --git a/src/specify_cli/_utils.py b/src/specify_cli/_utils.py index 300ca4ff58..87add18c4a 100644 --- a/src/specify_cli/_utils.py +++ b/src/specify_cli/_utils.py @@ -19,6 +19,18 @@ DOCKER_AGENT_CHECK_TIMEOUT = 5 +def same_path_entry(left: Path, right: Path) -> bool: + """Compare alternate case spellings without following leaf symlinks.""" + if left == right: + return True + if str(left).casefold() != str(right).casefold(): + return False + try: + return os.path.samestat(left.lstat(), right.lstat()) + except FileNotFoundError: + return False + + def docker_agent_command(executable: str | None = None) -> list[str] | None: """Return a runnable Docker Agent command, or ``None`` if unavailable. diff --git a/src/specify_cli/command_destroy.py b/src/specify_cli/command_destroy.py index fa2283b4f1..3339d9d6f2 100644 --- a/src/specify_cli/command_destroy.py +++ b/src/specify_cli/command_destroy.py @@ -160,6 +160,16 @@ def destroy( _emit_json_error(exc.code, exc.message, exc.details) else: console.print(f"[red]Error:[/red] {escape(exc.message)}") + for field, label in ( + ("paths", "Paths requiring attention"), + ("removed", "Files already removed"), + ("uninstalled_integrations", "Integrations already uninstalled"), + ): + values = exc.details.get(field) + if isinstance(values, (list, tuple)) and values: + console.print(f"{label}:") + for value in values: + console.print(f" {escape(str(value))}") raise typer.Exit(1) from exc except Exception as exc: logger.debug("Unexpected destroy failure", exc_info=True) diff --git a/src/specify_cli/command_init.py b/src/specify_cli/command_init.py index 7d1536d4ba..138ff33542 100644 --- a/src/specify_cli/command_init.py +++ b/src/specify_cli/command_init.py @@ -2,6 +2,7 @@ from __future__ import annotations +import hashlib import os import shlex import shutil @@ -871,6 +872,11 @@ def init( try: from .presets import PresetCatalog, PresetError, PresetManager + before_preset = { + name: hashlib.sha256((project_path / name).read_bytes()).hexdigest() + for name in manifest.files + if (project_path / name).is_file() and not (project_path / name).is_symlink() + } preset_manager = PresetManager(project_path) speckit_ver = get_speckit_version() @@ -932,6 +938,17 @@ def init( zip_path.unlink(missing_ok=True) except OSError: pass + changed = False + for name, previous in before_preset.items(): + path = project_path / name + if ( + path.is_file() and not path.is_symlink() + and hashlib.sha256(path.read_bytes()).hexdigest() != previous + ): + manifest.record_existing(name) + changed = True + if changed: + manifest.save() except Exception as preset_err: if lifecycle_owns_rollback(): raise diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index c22445ebe0..a6597852d0 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -2099,7 +2099,7 @@ def _complete_generic_refresh( return list(dict.fromkeys(registered + retained)) def _remove_generic_artifact_paths( - self, extension_id: str, metadata: Dict[str, Any], *, skills: bool = True + self, extension_id: str, metadata: Dict[str, Any], *, skills: bool = True, force: bool = False, ) -> None: """Clean recorded generic paths even after the configured directory moves.""" from ..shared_infra import _validate_safe_shared_directory @@ -2129,6 +2129,8 @@ def _remove_generic_artifact_paths( for relative, expected in hashes.items(): if not isinstance(relative, str) or not isinstance(expected, str): continue + if force and not re.fullmatch(r"[0-9a-fA-F]{64}", expected): + continue name = Path(relative) if name.is_absolute() or ".." in name.parts: continue @@ -2151,7 +2153,7 @@ def _remove_generic_artifact_paths( if path.is_symlink() and not path.resolve().is_relative_to(source): continue content = path.read_bytes() - if hashlib.sha256(content).hexdigest() != expected: + if not force and hashlib.sha256(content).hexdigest() != expected: if path.is_symlink(): unlink(path) write_bytes(path, content) @@ -3666,6 +3668,7 @@ def unregister_agent_artifacts( *, enabled_only: bool = False, commands_only: bool = False, + force: bool = False, ) -> None: """Remove extension files registered for a specific agent. @@ -3682,6 +3685,9 @@ def unregister_agent_artifacts( skill artifacts are outside the target agent's lifecycle and must not be touched. + ``force=True`` also removes modified generic output files whose recorded + names, paths, and ownership hashes pass the existing cleanup checks. + Skips cleanup when *agent_name* is not a supported agent to avoid losing registry entries while leaving orphaned files on disk. """ @@ -3705,7 +3711,7 @@ def unregister_agent_artifacts( if agent_name == "generic": self._remove_generic_artifact_paths( - ext_id, metadata, skills=not commands_only + ext_id, metadata, skills=not commands_only, force=force, ) updates: Dict[str, Any] = {} diff --git a/src/specify_cli/integrations/_operation_uninstall.py b/src/specify_cli/integrations/_operation_uninstall.py index 7523946b7c..9fdfd8b5d7 100644 --- a/src/specify_cli/integrations/_operation_uninstall.py +++ b/src/specify_cli/integrations/_operation_uninstall.py @@ -7,6 +7,7 @@ from pathlib import Path from ..file_helper import FileHelper +from .._utils import same_path_entry from ..integration_state import ( default_integration_key, installed_integration_keys, @@ -63,14 +64,36 @@ def validate_uninstall_ownership(root: Path, key: str) -> dict | None: return binding -def unregister_integration_artifacts(root: Path, key: str, *, require_complete: bool) -> tuple[str, ...]: +def unregister_integration_artifacts( + root: Path, key: str, *, require_complete: bool, force: bool = False, +) -> tuple[str, ...]: from ..extensions import ExtensionManager from ..presets import PresetManager warnings: list[str] = [] for label, manager in (("extension", ExtensionManager), ("preset", PresetManager)): try: - manager(root).unregister_agent_artifacts(key) + instance = manager(root) + generic_paths = set() + if require_complete and key == "generic" and label == "extension": + for record in instance.registry.list().values(): + generic_paths.update(record.get("generic_artifact_hashes", {})) + if label == "extension" and force: + instance.unregister_agent_artifacts(key, force=True) + else: + instance.unregister_agent_artifacts(key) + if generic_paths: + from .._file_policy import project_files + + files = project_files(root) + for relative in generic_paths: + try: + files.read_bytes(relative) + except FileNotFoundError: + continue + raise IntegrationUninstallError( + "cleanup_incomplete", f"Generic artifact was preserved: {relative}", + ) except Exception as exc: message = f"Could not clean up {label} artifacts for integration '{key}': {exc}" if require_complete: @@ -144,7 +167,10 @@ def _uninstall( recovery_manifest = IntegrationManifest.load(key, root) for preserved in request.preserved_paths: for name in manifest.files: - if Path(name).is_relative_to(preserved): + if any( + same_path_entry(root / ancestor, root / preserved) + for ancestor in (Path(name), *Path(name).parents) + ): manifest.remove(name) except (ValueError, OSError) as exc: raise IntegrationUninstallError( @@ -176,7 +202,7 @@ def _uninstall( except (OSError, ValueError) as exc: raise IntegrationUninstallError("cleanup_incomplete", str(exc)) from exc warnings.extend(unregister_integration_artifacts( - root, key, require_complete=request.require_complete, + root, key, require_complete=request.require_complete, force=request.force, )) if request.require_complete and skipped: raise IntegrationUninstallError( @@ -292,7 +318,7 @@ def _uninstall_with_lifecycle(request: IntegrationUninstallRequest) -> Integrati for removed_key in records.keys() - set(remaining): if removed_key != key: warnings.extend(unregister_integration_artifacts( - root, removed_key, require_complete=request.require_complete, + root, removed_key, require_complete=request.require_complete, force=request.force, )) installer.remove_package(root, removed_key) return replace(result, warnings=tuple(warnings)) diff --git a/src/specify_cli/presets/_manager_skills.py b/src/specify_cli/presets/_manager_skills.py index 5a4a4fb739..b484093ade 100644 --- a/src/specify_cli/presets/_manager_skills.py +++ b/src/specify_cli/presets/_manager_skills.py @@ -1220,17 +1220,8 @@ def _unregister_skills( else {} ) - def _delete_agent_preset_skills( - self, agent_name: str, skill_names: List[str], pack_id: str - ) -> None: - """Delete still-preset-owned skills when an agent is deactivated.""" - skills_dir = self._safe_skills_dir_for_agent(agent_name) - if skills_dir is None: - return - - - registrar = self._command_registrar() - marker = f"preset:{pack_id}" + def skill_ownership_sources(self, pack_id: str) -> Dict[str, str]: + """Map skill names to validated reconciled override provenance.""" override_sources: Dict[str, str] = {} manifest = PresetResolver(self.project_root)._get_manifest( self.presets_dir / pack_id @@ -1248,6 +1239,18 @@ def _delete_agent_preset_skills( override_sources[skill_name] = ( f"override:{command_name}" ) + return override_sources + + def _delete_agent_preset_skills( + self, agent_name: str, skill_names: List[str], pack_id: str + ) -> None: + """Delete still-preset-owned skills when an agent is deactivated.""" + skills_dir = self._safe_skills_dir_for_agent(agent_name) + if skills_dir is None: + return + registrar = self._command_registrar() + marker = f"preset:{pack_id}" + override_sources = self.skill_ownership_sources(pack_id) for skill_name in skill_names: if not self._is_safe_registry_skill_name(skill_name): continue diff --git a/tests/specify_cli/test_command_destroy.py b/tests/specify_cli/test_command_destroy.py index 0c147d3fba..5db21e85a2 100644 --- a/tests/specify_cli/test_command_destroy.py +++ b/tests/specify_cli/test_command_destroy.py @@ -245,3 +245,21 @@ def confirm(*args, **kwargs): assert result.exit_code == 0, result.output assert len(prompts) == 1 assert empty.exists() is not answer + + +def test_human_errors_show_recovery_paths_and_completed_integrations(tmp_path, monkeypatch): + from specify_cli._operation_destroy import DestroyError + + def fail(*args, **kwargs): + raise DestroyError( + "cleanup_incomplete", "Cleanup needs attention", + paths=[".custom/commands/retained.md"], removed=[".specify/cache"], + uninstalled_integrations=["gemini"], + ) + + monkeypatch.setattr("specify_cli.command_destroy.destroy_project", fail) + result = runner.invoke(app, ["destroy", "--project", str(tmp_path), "--yes"]) + assert result.exit_code == 1 + assert ".custom/commands/retained.md" in result.output + assert ".specify/cache" in result.output + assert "gemini" in result.output diff --git a/tests/specify_cli/test_operation_destroy.py b/tests/specify_cli/test_operation_destroy.py index 167eb273e4..a7e7778c3a 100644 --- a/tests/specify_cli/test_operation_destroy.py +++ b/tests/specify_cli/test_operation_destroy.py @@ -1135,3 +1135,205 @@ def test_unproven_dispatcher_cleanup_is_rejected_before_changes(tmp_path, kind): with pytest.raises(DestroyError, match="dispatcher ownership"): destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) assert snapshot(tmp_path) == before + + +@pytest.mark.parametrize("force", [False, True]) +def test_preserve_constitution_uses_actual_case_insensitive_entry(tmp_path, force): + record = manifest(tmp_path, "claude", ".claude/skills/speckit-plan/SKILL.md") + constitution = write(tmp_path, ".specify/Memory/Constitution.md", b"retain actual spelling") + if not (tmp_path / CONSTITUTION).exists(): + pytest.skip("requires a case-insensitive filesystem") + record.record_existing(".specify/Memory/Constitution.md") + record.save() + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=force, preserve_constitution=True)) + assert constitution.read_bytes() == b"retain actual spelling" + + +@pytest.mark.parametrize("skills", [False, True]) +def test_force_destroy_removes_modified_generic_extension_output(tmp_path, monkeypatch, skills): + monkeypatch.chdir(tmp_path) + options = "--commands-dir .custom/output" + (" --skills" if skills else "") + runner = CliRunner() + result = runner.invoke(app, [ + "init", "--here", "--force", "--non-interactive", "--ignore-agent-tools", + "--integration", "generic", f"--integration-options={options}", "--extension", "git", + ]) + assert result.exit_code == 0, result.output + registry = json.loads((tmp_path / ".specify/extensions/.registry").read_text()) + relative = next(iter(registry["extensions"]["git"]["generic_artifact_hashes"])) + output = write(tmp_path, relative, b"customized extension output") + preview = destroy_project(DestroyRequest(tmp_path, dry_run=True)) + assert relative in preview.requires_force + before = snapshot(tmp_path) + with pytest.raises(DestroyError): + destroy_project(DestroyRequest(tmp_path, confirmed=True)) + assert snapshot(tmp_path) == before + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert not output.exists() + assert not (tmp_path / ".specify").exists() + + +def test_native_config_validation_precedes_all_integration_teardown(tmp_path): + from specify_cli.integrations import get_integration + + for key in ("claude", "gemini"): + record = manifest(tmp_path, key, f".{key}/skills/speckit-plan/SKILL.md") + get_integration(key).emit_events(tmp_path, record, {"pre_tool_use": [{"command": "speckit.test"}]}) + record.save() + write(tmp_path, ".claude/settings.json", b"{malformed") + before = snapshot(tmp_path) + with pytest.raises(DestroyError, match="native event cleanup"): + destroy_project(DestroyRequest(tmp_path, dry_run=True)) + with pytest.raises(DestroyError, match="native event cleanup"): + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert snapshot(tmp_path) == before + + +def test_dispatcher_preflight_uses_last_claimants_hash(tmp_path): + from specify_cli.integrations import get_integration + + for key in ("claude", "gemini"): + record = manifest(tmp_path, key, f".{key}/skills/speckit-plan/SKILL.md") + get_integration(key).emit_events(tmp_path, record, {"pre_tool_use": [{"command": "speckit.test"}]}) + record.save() + write(tmp_path, ".specify/events.py", b"# old dispatcher") + earlier = IntegrationManifest.load("gemini", tmp_path) + earlier.record_existing(".specify/events.py") + earlier.save() + before = snapshot(tmp_path) + preview = destroy_project(DestroyRequest(tmp_path, dry_run=True)) + assert ".specify/events.py" in preview.requires_force + with pytest.raises(DestroyError): + destroy_project(DestroyRequest(tmp_path, confirmed=True)) + assert snapshot(tmp_path) == before + + +@pytest.mark.parametrize("integration", ["copilot", "claude", "gemini"]) +def test_init_preset_output_needs_no_force_but_user_edits_still_do(tmp_path, monkeypatch, integration): + monkeypatch.chdir(tmp_path) + result = CliRunner().invoke(app, [ + "init", "--here", "--force", "--non-interactive", "--ignore-agent-tools", + "--integration", integration, "--preset", "lean", + ]) + assert result.exit_code == 0, result.output + assert not destroy_project(DestroyRequest(tmp_path, dry_run=True)).requires_force + record = IntegrationManifest.load(integration, tmp_path) + relative = next(name for name in record.files if "speckit" in name and not name.startswith(".specify")) + original = (tmp_path / relative).read_bytes() + write(tmp_path, relative, b"user edit") + assert relative in destroy_project(DestroyRequest(tmp_path, dry_run=True)).requires_force + write(tmp_path, relative, original) + destroy_project(DestroyRequest(tmp_path, confirmed=True)) + assert not (tmp_path / ".specify").exists() + + +def test_specs_directory_failure_preserves_project_marker_for_retry(tmp_path, monkeypatch): + from specify_cli.file_helper import FileHelper + + config = write(tmp_path, ".specify/config") + (tmp_path / "specs/empty").mkdir(parents=True) + delete = FileHelper.delete + + def fail(self, path, **kwargs): + if path == "specs": + raise PermissionError("specs directory is locked") + return delete(self, path, **kwargs) + + with monkeypatch.context() as fault: + fault.setattr(FileHelper, "delete", fail) + with pytest.raises(DestroyError, match="specs directory is locked"): + destroy_project(DestroyRequest(tmp_path, confirmed=True)) + assert config.exists() + destroy_project(DestroyRequest(tmp_path, confirmed=True)) + assert not (tmp_path / "specs").exists() + assert not (tmp_path / ".specify").exists() + + +@pytest.mark.parametrize("override", [False, True]) +def test_stacked_preset_skill_ownership_is_resolved_collectively(tmp_path, override): + import yaml + + entries = {} + for identifier in ("lower", "winner"): + entries[identifier] = {"registered_skills": {"copilot": ["speckit-sample"]}} + write(tmp_path, f".specify/presets/{identifier}/preset.yml", yaml.safe_dump({ + "schema_version": "1.0", + "preset": {"id": identifier, "name": identifier, "version": "1.0.0", "description": "test"}, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": {"templates": [ + {"type": "command", "name": "speckit.sample", "file": "commands/sample.md"}, + ]}, + }).encode()) + write(tmp_path, f".specify/presets/{identifier}/commands/sample.md", b"preset command") + write(tmp_path, ".specify/presets/.registry", json.dumps({"presets": entries}).encode()) + source = "override:speckit.sample" if override else "preset:winner" + skill = write(tmp_path, ".github/skills/speckit-sample/SKILL.md", + f"---\nmetadata:\n source: {source}\n---\nManaged skill\n".encode()) + preview = destroy_project(DestroyRequest(tmp_path, dry_run=True)) + assert ".github/skills/speckit-sample/SKILL.md" in preview.removed + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert not skill.exists() + + +@pytest.mark.parametrize("force", [False, True]) +def test_case_insensitive_preserved_constitution_link_is_not_followed(tmp_path, force): + record = manifest(tmp_path, "claude", ".claude/skills/speckit-plan/SKILL.md") + entry = write(tmp_path, ".specify/Memory/Constitution.md", b"original") + if not (tmp_path / CONSTITUTION).exists(): + pytest.skip("requires a case-insensitive filesystem") + record.record_existing(".specify/Memory/Constitution.md") + record.save() + entry.unlink() + target = write(tmp_path, "user-principles.md", b"user data") + symlink(entry, target) + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=force, preserve_all=True)) + assert entry.is_symlink() + assert target.read_bytes() == b"user data" + + +@pytest.mark.parametrize("skills", [False, True]) +def test_failed_forced_generic_cleanup_retains_registration_and_can_retry(tmp_path, monkeypatch, skills): + monkeypatch.chdir(tmp_path) + options = "--commands-dir .custom/output" + (" --skills" if skills else "") + result = CliRunner().invoke(app, [ + "init", "--here", "--force", "--non-interactive", "--ignore-agent-tools", + "--integration", "generic", f"--integration-options={options}", "--extension", "git", + ]) + assert result.exit_code == 0, result.output + registry = json.loads((tmp_path / ".specify/extensions/.registry").read_text()) + relative = next(iter(registry["extensions"]["git"]["generic_artifact_hashes"])) + output = write(tmp_path, relative, b"modified command") + unlink = Path.unlink + + def fail(path, **kwargs): + if path == output: + raise PermissionError("generic output locked") + return unlink(path, **kwargs) + + with monkeypatch.context() as fault: + fault.setattr(Path, "unlink", fail) + with pytest.raises(DestroyError, match="generic output locked"): + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert (tmp_path / ".specify/integration.json").exists() + assert (tmp_path / ".specify/integrations/generic.manifest.json").exists() + assert json.loads((tmp_path / ".specify/init-options.json").read_text())["ai"] == "generic" + assert output.read_bytes() == b"modified command" + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert not output.exists() + assert not (tmp_path / ".specify").exists() + + +def test_dispatcher_earlier_stale_hash_does_not_override_last_owner(tmp_path): + from specify_cli.integrations import get_integration + + for key in ("claude", "gemini"): + record = manifest(tmp_path, key, f".{key}/skills/speckit-plan/SKILL.md") + get_integration(key).emit_events(tmp_path, record, {"pre_tool_use": [{"command": "speckit.test"}]}) + record.save() + write(tmp_path, ".specify/events.py", b"# managed historical dispatcher") + last = IntegrationManifest.load("claude", tmp_path) + last.record_existing(".specify/events.py") + last.save() + assert not destroy_project(DestroyRequest(tmp_path, dry_run=True)).requires_force + destroy_project(DestroyRequest(tmp_path, confirmed=True)) + assert not (tmp_path / ".specify").exists() From 4a6436d653d55649fc930deef8c232d59c003cc6 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 29 Oct 2026 16:28:08 -0500 Subject: [PATCH 4/4] fix(destroy): address cleanup ownership and recovery review Validate preset provenance before reads, inventory real home-scoped outputs, retain strict cleanup registrations and recovery state, and report completed deletions on failures. Refresh tracked hashes only for managed preset writes and normalize teardown exits and project overrides. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/reference/core.md | 21 +- src/specify_cli/_operation_destroy.py | 95 +++++++-- src/specify_cli/command_destroy.py | 9 +- src/specify_cli/command_init.py | 17 -- src/specify_cli/extensions/__init__.py | 38 ++++ src/specify_cli/integrations/_file_changes.py | 4 + .../integrations/_operation_uninstall.py | 71 ++++--- src/specify_cli/integrations/_registration.py | 38 ++++ src/specify_cli/presets/_manager_commands.py | 20 +- src/specify_cli/presets/_manager_skills.py | 14 +- .../integrations/test_installed_adapters.py | 5 +- .../integrations/test_operation_uninstall.py | 100 ++++++++- tests/specify_cli/test_command_destroy.py | 36 ++++ tests/specify_cli/test_operation_destroy.py | 199 ++++++++++++++++++ 14 files changed, 587 insertions(+), 80 deletions(-) diff --git a/docs/reference/core.md b/docs/reference/core.md index 44d6b8c986..a576adca1b 100644 --- a/docs/reference/core.md +++ b/docs/reference/core.md @@ -141,6 +141,8 @@ Only the literal project-local `specs/` directory is removed: feature-directory overrides, including `SPECIFY_FEATURE_DIRECTORY` and `.specify/feature.json`, do not expand the deletion scope. A directory without a `.specify` project marker is a no-op, even if it contains `specs/`. +An invalid `SPECIFY_INIT_DIR` remains an error; an explicit `--project` takes +precedence over that environment override. Modified hash-tracked generated files outside `.specify` stop cleanup before deletion unless `--force` is explicit. Contribution cleanup uses the existing @@ -152,6 +154,9 @@ For generic integration outputs, force permits removal of modified extension fil only after the recorded command names, output paths, and ownership hashes have passed validation. Incomplete contribution cleanup retains the integration state needed to retry. +Managed preset rendering refreshes hashes only for tracked files it writes, +both during `init --preset` and a later `preset add`. Unchanged user edits +are not adopted as generated content. For example: ```bash @@ -191,7 +196,9 @@ to identify agent artifacts; allowed leaf links are unlinked without deleting their targets. An explicitly preserved constitution or `specs/` entry may be a link and is left untouched. Otherwise the persisted symlink policy applies; targets are never recursively traversed merely because a link is selected for deletion. -Metadata is always read without following links. +Metadata is always read without following links. Tree inventory inspects paths +without reading arbitrary caches or specification assets; payload reads are +limited to ownership metadata, provenance checks, and tracked-file hashes. Preview, rejected confirmation, and validation failures leave files unchanged. An I/O failure during deletion can leave a partial uninstall and is reported @@ -204,14 +211,18 @@ a no-op with `--preserve-constitution` or `--preserve-all`; without those flags, its deletion still requires confirmation. Extension and preset registries are retained until their package files and directories have been removed, so a failed package purge can be retried. -If teardown raises after deleting its manifest, destroy restores the original -ownership manifest so the recorded integration can be retried after the error -is resolved. This preserves recovery evidence; it does not undo files that -were already removed outside a lifecycle transaction. +If strict cleanup fails, destroy restores the original ownership manifest, +installation inventory, and init options together so the recorded integration +can be retried after the error is resolved. Skipped contribution cleanup retains +its registration rather than discarding ownership evidence. This preserves +recovery evidence; it does not undo files that were already removed outside a +lifecycle transaction. Specification files and directories are removed before remaining infrastructure: a locked `specs/` directory leaves the project marker available for retry. Human-readable failures list remaining paths and completed file/integration removals alongside the error; JSON errors expose the same recovery details. +Completed home-scoped deletions are reported separately in `global_files`, +including when teardown fails after deleting only some of its outputs. Legacy setups with only an `init-options.json` integration selection and no `integration.json` inventory are rejected during preview. Re-run `specify init` diff --git a/src/specify_cli/_operation_destroy.py b/src/specify_cli/_operation_destroy.py index f7cbd9e7e1..1675117220 100644 --- a/src/specify_cli/_operation_destroy.py +++ b/src/specify_cli/_operation_destroy.py @@ -132,6 +132,8 @@ def _tree( while parent != files.root: if parent.is_symlink(): raise FileHelperError(f"Refusing destroy through symlinked directory: {parent}") + if getattr(parent.lstat(), "st_file_attributes", 0) & stat.FILE_ATTRIBUTE_REPARSE_POINT: + raise FileHelperError(f"Unsupported reparse point: {parent}") parent = parent.parent entry = path.lstat() @@ -147,7 +149,6 @@ def _tree( raise FileHelperError(f"Refusing symlink during destroy: {relative}") leaves.add(relative) elif stat.S_ISREG(mode): - files.read_bytes(relative) leaves.add(relative) elif stat.S_ISDIR(mode): with os.scandir(path) as entries: @@ -211,7 +212,8 @@ def _contribution_paths( hashes: dict[str, set[str]] = {} agents: set[str] = set() global_paths: set[Path] = set() - preset_claims: dict[Path, tuple[set[str], set[str]]] = {} + preset_claims: dict[Path, dict[str, set[str]]] = {} + explicit_preset_claims: set[Path] = set() active_generic = read_project_options(root).get("ai") == "generic" for kind in ("extensions", "presets"): relative = f".specify/{kind}/.registry" @@ -233,7 +235,14 @@ def _contribution_paths( override_sources: dict[str, str] = {} if kind == "presets": from .presets import PresetManager - + from .presets._resolver import PresetResolver + + if not PresetResolver._is_safe_registry_id(identifier): + raise ValueError(f"Invalid preset ID: {identifier!r}") + try: + metadata_files.read_bytes(f".specify/presets/{identifier}/preset.yml") + except FileNotFoundError: + pass override_sources = PresetManager(root).skill_ownership_sources(identifier) generic_hashes = record.get("generic_artifact_hashes", {}) if not isinstance(generic_hashes, dict): @@ -298,14 +307,28 @@ def _contribution_paths( candidates = registrar.registered_artifact_paths( agent, root, commands=[], skills=[skill], registrar_config=config, ) + agent_config = config or registrar.AGENT_CONFIGS.get(agent, {}) + if kind == "presets" and agent_config.get("extension") == "/SKILL.md": + candidates = { + registrar._resolve_agent_dir(agent, agent_config, root) / skill, + } for candidate in candidates: + if not candidate.is_relative_to(root) and not ( + agent in global_scopes and candidate.is_relative_to(global_scopes[agent]) + ): + if kind == "presets" and isinstance(skills, list): + continue + raise ValueError( + f"Contribution output is outside its declared uninstall scope: {candidate}", + ) if kind == "presets": - sources, owners = preset_claims.setdefault(candidate, (set(), set())) - sources.add(f"preset:{identifier}") + sources = preset_claims.setdefault(candidate, {}) + sources.setdefault(f"preset:{identifier}", set()).add(agent) override = override_sources.get(skill) if override: - sources.add(override) - owners.add(agent) + sources.setdefault(override, set()).add(agent) + if not isinstance(skills, list): + explicit_preset_claims.add(candidate) continue relative = _relative(root, candidate / "SKILL.md") try: @@ -323,21 +346,29 @@ def _contribution_paths( agents.add(agent) elif not isinstance(skills, list): raise ValueError(f"Cannot establish skill ownership at {candidate}") - for candidate, (sources, owners) in preset_claims.items(): - relative = _relative(root, candidate / "SKILL.md") + for candidate, sources in preset_claims.items(): + scope_files = files if candidate.is_relative_to(root) else FileHelper( + Path.home(), allow_symlinks=files.allow_symlinks, + ) + relative = _relative(scope_files.root, candidate / "SKILL.md") try: - content = files.read_text(relative) + content = scope_files.read_text(relative) except FileNotFoundError: - if candidate.exists(): + if candidate in explicit_preset_claims and (candidate.exists() or candidate.is_symlink()): raise ValueError(f"Cannot establish skill ownership at {candidate}") continue frontmatter, _ = registrar.parse_frontmatter(content) metadata = frontmatter.get("metadata", {}) if isinstance(frontmatter, dict) else {} source = metadata.get("source") if isinstance(metadata, dict) else None if source not in sources: - raise ValueError(f"Cannot establish skill ownership at {candidate}") - paths.add(candidate) - agents.update(owners) + if candidate in explicit_preset_claims: + raise ValueError(f"Cannot establish skill ownership at {candidate}") + continue + if candidate.is_relative_to(root): + paths.add(candidate) + else: + global_paths.add(candidate) + agents.update(sources[source]) return paths, hashes, agents, global_paths @@ -546,8 +577,20 @@ def destroy_project(request: DestroyRequest) -> DestroyResult: ) files = project_files(plan.project_directory) removed: list[str] = [] + global_removed: set[str] = set() warnings: list[str] = [] uninstalled: list[str] = [] + + def reconcile_removals() -> None: + removed.extend( + name for name in plan.files if name not in removed + and not (plan.project_directory / name).exists() + and not (plan.project_directory / name).is_symlink() + ) + global_removed.update( + name for name in plan.global_files if not Path(name).exists() and not Path(name).is_symlink() + ) + try: from .integrations._operation_uninstall import ( IntegrationUninstallRequest, @@ -560,6 +603,15 @@ def destroy_project(request: DestroyRequest) -> DestroyResult: plan.project_directory, key, request.force, require_complete=True, preserved_paths=(CONSTITUTION, "specs"), )) + for path in result.removed: + if path.exists() or path.is_symlink(): + continue + if path.is_relative_to(plan.project_directory): + relative = path.relative_to(plan.project_directory).as_posix() + if relative not in removed: + removed.append(relative) + else: + global_removed.add(str(path)) warnings.extend(result.warnings) uninstalled.append(key) for key in set(plan.contribution_agents) - set(plan.integrations): @@ -629,24 +681,23 @@ def destroy_project(request: DestroyRequest) -> DestroyResult: ): raise except DestroyError as exc: + reconcile_removals() + exc.details["removed"] = removed + exc.details["global_files"] = sorted(global_removed) exc.details["uninstalled_integrations"] = uninstalled raise except (OSError, ValueError, RuntimeError) as exc: + reconcile_removals() raise DestroyError( "destroy_failed", f"Uninstall stopped: {exc}. Some files may already have been removed.", removed=removed, + global_files=sorted(global_removed), uninstalled_integrations=uninstalled, ) from exc - removed.extend( - name for name in plan.files if name not in removed - and not (plan.project_directory / name).exists() - and not (plan.project_directory / name).is_symlink() - ) + reconcile_removals() return DestroyResult( str(plan.project_directory), False, tuple(removed), plan.preserved, updated=plan.updated, warnings=tuple(warnings), global_scopes=plan.global_scopes, - global_files=tuple( - name for name in plan.global_files if not Path(name).exists() and not Path(name).is_symlink() - ), + global_files=tuple(sorted(global_removed)), ) diff --git a/src/specify_cli/command_destroy.py b/src/specify_cli/command_destroy.py index 3339d9d6f2..e4d15ef8f6 100644 --- a/src/specify_cli/command_destroy.py +++ b/src/specify_cli/command_destroy.py @@ -4,7 +4,6 @@ import json import logging -import os import sys import warnings from contextlib import redirect_stderr, redirect_stdout @@ -18,6 +17,7 @@ from typer.core import TyperCommand from ._console import console +from ._project import ProjectResolutionError, _resolve_init_dir_override_unrendered from ._operation_destroy import ( DestroyError, DestroyRequest, @@ -97,7 +97,11 @@ def destroy( ) -> None: """Uninstall Spec Kit and remove .specify/ and specs/ unless explicitly preserved.""" try: - root = (project or Path(os.environ.get("SPECIFY_INIT_DIR") or Path.cwd())).absolute() + try: + root = project if project is not None else _resolve_init_dir_override_unrendered() or Path.cwd() + root = root.absolute() + except ProjectResolutionError as exc: + raise DestroyError("invalid_project_directory", str(exc)) from exc request = DestroyRequest( root, confirmed=yes, force=force, dry_run=True, preserve_constitution=preserve_constitution, preserve_specs=preserve_specs, @@ -163,6 +167,7 @@ def destroy( for field, label in ( ("paths", "Paths requiring attention"), ("removed", "Files already removed"), + ("global_files", "Home-scoped paths already removed"), ("uninstalled_integrations", "Integrations already uninstalled"), ): values = exc.details.get(field) diff --git a/src/specify_cli/command_init.py b/src/specify_cli/command_init.py index 138ff33542..7d1536d4ba 100644 --- a/src/specify_cli/command_init.py +++ b/src/specify_cli/command_init.py @@ -2,7 +2,6 @@ from __future__ import annotations -import hashlib import os import shlex import shutil @@ -872,11 +871,6 @@ def init( try: from .presets import PresetCatalog, PresetError, PresetManager - before_preset = { - name: hashlib.sha256((project_path / name).read_bytes()).hexdigest() - for name in manifest.files - if (project_path / name).is_file() and not (project_path / name).is_symlink() - } preset_manager = PresetManager(project_path) speckit_ver = get_speckit_version() @@ -938,17 +932,6 @@ def init( zip_path.unlink(missing_ok=True) except OSError: pass - changed = False - for name, previous in before_preset.items(): - path = project_path / name - if ( - path.is_file() and not path.is_symlink() - and hashlib.sha256(path.read_bytes()).hexdigest() != previous - ): - manifest.record_existing(name) - changed = True - if changed: - manifest.save() except Exception as preset_err: if lifecycle_owns_rollback(): raise diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index a6597852d0..93d0551af5 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -3669,6 +3669,7 @@ def unregister_agent_artifacts( enabled_only: bool = False, commands_only: bool = False, force: bool = False, + require_complete: bool = False, ) -> None: """Remove extension files registered for a specific agent. @@ -3688,6 +3689,9 @@ def unregister_agent_artifacts( ``force=True`` also removes modified generic output files whose recorded names, paths, and ownership hashes pass the existing cleanup checks. + ``require_complete`` keeps registration metadata when an owned output + is skipped and raises for the strict uninstall caller. + Skips cleanup when *agent_name* is not a supported agent to avoid losing registry entries while leaving orphaned files on disk. """ @@ -3709,6 +3713,37 @@ def unregister_agent_artifacts( if enabled_only and not metadata.get("enabled", True): continue + pending_paths: set[Path] = set() + if require_complete: + from .._file_policy import project_files + from ..agents import CommandRegistrar as AgentRegistrar + from ..file_helper import FileHelper + + commands = metadata.get("registered_commands", {}) + pending_paths.update(AgentRegistrar(self.project_root).registered_artifact_paths( + agent_name, self.project_root, + commands=self._valid_name_list(commands.get(agent_name)) if isinstance(commands, dict) else [], + skills=[], registrar_config=registrar.AGENT_CONFIGS[agent_name], + )) + if agent_name == "generic": + pending_paths.update( + self.project_root / name for name in metadata.get("generic_artifact_hashes", {}) + ) + if not commands_only: + policy = project_files(self.project_root) + scope = self.project_root if agent_skills_dir.is_relative_to(self.project_root) else Path.home() + files = FileHelper(scope, allow_symlinks=policy.allow_symlinks) + for name in self._valid_name_list(metadata.get("registered_skills")): + path = agent_skills_dir / name + try: + content = files.read_text(path / "SKILL.md") + except FileNotFoundError: + continue + frontmatter, _ = registrar.parse_frontmatter(content) + marker = frontmatter.get("metadata", {}) + if isinstance(marker, dict) and marker.get("source") == f"extension:{ext_id}": + pending_paths.add(path) + if agent_name == "generic": self._remove_generic_artifact_paths( ext_id, metadata, skills=not commands_only, force=force, @@ -3796,6 +3831,9 @@ def unregister_agent_artifacts( if remaining_skills != registered_skills: updates["registered_skills"] = remaining_skills + retained = sorted(path for path in pending_paths if path.exists() or path.is_symlink()) + if retained: + raise ValueError("Extension artifacts were preserved: " + ", ".join(str(path) for path in retained)) if updates: self.registry.update(ext_id, updates) diff --git a/src/specify_cli/integrations/_file_changes.py b/src/specify_cli/integrations/_file_changes.py index 9ed0ebc62f..c2f75277c3 100644 --- a/src/specify_cli/integrations/_file_changes.py +++ b/src/specify_cli/integrations/_file_changes.py @@ -10,6 +10,7 @@ file_change_observer: ContextVar[Callable[[Path, bool, bool], None] | None] = ContextVar( "integration_file_change_observer", default=None ) +registration_changes: ContextVar[set[Path] | None] = ContextVar("registration_changes", default=None) def before_file_change(path: Path, *, removal: bool = False) -> None: @@ -22,6 +23,9 @@ def after_file_change(path: Path) -> None: observer = file_change_observer.get() if observer is not None: observer(path, False, False) + changes = registration_changes.get() + if changes is not None: + changes.add(path) @contextmanager diff --git a/src/specify_cli/integrations/_operation_uninstall.py b/src/specify_cli/integrations/_operation_uninstall.py index 9fdfd8b5d7..2abbfb84ea 100644 --- a/src/specify_cli/integrations/_operation_uninstall.py +++ b/src/specify_cli/integrations/_operation_uninstall.py @@ -78,10 +78,10 @@ def unregister_integration_artifacts( if require_complete and key == "generic" and label == "extension": for record in instance.registry.list().values(): generic_paths.update(record.get("generic_artifact_hashes", {})) + cleanup_options = {"require_complete": True} if require_complete else {} if label == "extension" and force: - instance.unregister_agent_artifacts(key, force=True) - else: - instance.unregister_agent_artifacts(key) + cleanup_options["force"] = True + instance.unregister_agent_artifacts(key, **cleanup_options) if generic_paths: from .._file_policy import project_files @@ -90,7 +90,9 @@ def unregister_integration_artifacts( try: files.read_bytes(relative) except FileNotFoundError: - continue + path = root / relative + if not path.exists() and not path.is_symlink(): + continue raise IntegrationUninstallError( "cleanup_incomplete", f"Generic artifact was preserved: {relative}", ) @@ -103,26 +105,36 @@ def unregister_integration_artifacts( @contextmanager -def _retain_recovery_manifest(request: IntegrationUninstallRequest, key: str | None): - """Strict cleanup keeps its original ownership evidence even if teardown raises.""" +def _retain_recovery_state(request: IntegrationUninstallRequest, key: str | None): + """Keep ownership and installation metadata together until strict cleanup completes.""" if not request.require_complete or key is None: yield return - relative = f".specify/integrations/{key}.manifest.json" files = FileHelper(request.project_directory) - try: - original = files.read_bytes(relative) - except FileNotFoundError: - yield - return + originals: dict[str, bytes | None] = {} + for relative in ( + f".specify/integrations/{key}.manifest.json", + ".specify/integration.json", ".specify/init-options.json", + ): + try: + originals[relative] = files.read_bytes(relative) + except FileNotFoundError: + originals[relative] = None try: yield except Exception: from ._file_changes import changing_file - with changing_file(request.project_directory / relative): - files.mkdir(".specify/integrations", parents=True, exist_ok=True) - files.write_bytes(relative, original) + for relative, original in originals.items(): + with changing_file(request.project_directory / relative, removal=original is None): + if original is None: + try: + files.delete(relative) + except FileNotFoundError: + pass + else: + files.mkdir(Path(relative).parent, parents=True, exist_ok=True) + files.write_bytes(relative, original) raise @@ -189,7 +201,12 @@ def _uninstall( validate_integration_event_cleanup(integration, root) except (OSError, ValueError) as exc: raise IntegrationUninstallError("cleanup_incomplete", str(exc)) from exc - removed, skipped = integration.teardown(root, manifest, force=request.force) + try: + removed, skipped = integration.teardown(root, manifest, force=request.force) + except SystemExit as exc: + raise IntegrationUninstallError( + "uninstall_failed", f"Integration '{key}' teardown exited before cleanup completed (code {exc.code}).", + ) from exc if ( integration is not None and request.require_complete and recovery_manifest is not None @@ -309,18 +326,18 @@ def _uninstall_with_lifecycle(request: IntegrationUninstallRequest) -> Integrati if mode != manifest.ownership_modes[name]: manifest.set_ownership(name, mode) manifest.save() - with _retain_recovery_manifest(request, key): + with _retain_recovery_state(request, key): result = _uninstall(request, warnings) - new_state, error = try_read_integration_json(root) - if error: - raise IntegrationUninstallError("invalid_state", f"Cannot read integration state: {error.detail}") - remaining = installed_integration_keys(new_state or {}) - for removed_key in records.keys() - set(remaining): - if removed_key != key: - warnings.extend(unregister_integration_artifacts( - root, removed_key, require_complete=request.require_complete, force=request.force, - )) - installer.remove_package(root, removed_key) + new_state, error = try_read_integration_json(root) + if error: + raise IntegrationUninstallError("invalid_state", f"Cannot read integration state: {error.detail}") + remaining = installed_integration_keys(new_state or {}) + for removed_key in records.keys() - set(remaining): + if removed_key != key: + warnings.extend(unregister_integration_artifacts( + root, removed_key, require_complete=request.require_complete, force=request.force, + )) + installer.remove_package(root, removed_key) return replace(result, warnings=tuple(warnings)) except IntegrationUninstallError: raise diff --git a/src/specify_cli/integrations/_registration.py b/src/specify_cli/integrations/_registration.py index f1b12253cb..88968fb3dc 100644 --- a/src/specify_cli/integrations/_registration.py +++ b/src/specify_cli/integrations/_registration.py @@ -4,6 +4,7 @@ import inspect from functools import wraps +from pathlib import Path def project_registration(handler): @@ -22,3 +23,40 @@ def invoke(*args, **kwargs): return handler(*args, **kwargs) return invoke + + +def managed_registration(handler): + """Refresh existing manifest claims only for outputs written by registration.""" + @wraps(handler) + def invoke(self, *args, **kwargs): + from ._file_changes import registration_changes + from .manifest import IntegrationManifest + from ..file_helper import FileHelper + from ..shared_infra import _validate_safe_shared_directory + + if registration_changes.get() is not None: + return handler(self, *args, **kwargs) + changed: set[Path] = set() + token = registration_changes.set(changed) + try: + return handler(self, *args, **kwargs) + finally: + registration_changes.reset(token) + if changed: + root = self.project_root + directory = root / ".specify/integrations" + _validate_safe_shared_directory(root, directory) + metadata = FileHelper(root) + for path in sorted(directory.glob("*.manifest.json")): + metadata.read_bytes(path) + manifest = IntegrationManifest.load(path.name.removesuffix(".manifest.json"), root) + refreshed = False + for name in manifest.files: + output = root / name + if output in changed and output.is_file() and not output.is_symlink(): + manifest.record_existing(name) + refreshed = True + if refreshed: + manifest.save() + + return invoke diff --git a/src/specify_cli/presets/_manager_commands.py b/src/specify_cli/presets/_manager_commands.py index d171eb28dd..c9473aec07 100644 --- a/src/specify_cli/presets/_manager_commands.py +++ b/src/specify_cli/presets/_manager_commands.py @@ -16,6 +16,7 @@ ) from ..extensions import ExtensionRegistry from ..integrations._file_changes import write_text as _write_text +from ..integrations._registration import managed_registration from ._manifest import PresetManifest from ._resolver import PresetResolver @@ -92,6 +93,7 @@ def _substitute_core_template( class _PresetCommandMethods: """Command artifact methods shared through PresetManager's lifecycle state.""" + @managed_registration def _register_commands( self, manifest: PresetManifest, @@ -670,7 +672,7 @@ def register_enabled_presets_for_agent(self, agent_name: str) -> None: pack_id, {"registered_skills": merged_skills} ) - def unregister_agent_artifacts(self, agent_name: str) -> None: + def unregister_agent_artifacts(self, agent_name: str, *, require_complete: bool = False) -> None: """Remove ``agent_name``'s tracked preset command/skill artifacts. Mirrors ``ExtensionManager.unregister_agent_artifacts()`` (#2948): @@ -686,6 +688,9 @@ def unregister_agent_artifacts(self, agent_name: str) -> None: tracking, and preset packs themselves are left untouched, and no priority-stack reconciliation runs — this is agent-scoped cleanup only, not preset removal. + + ``require_complete`` retains registration metadata and raises when an + owned output is skipped, so strict uninstall can be retried. """ if not agent_name: return @@ -701,6 +706,7 @@ def unregister_agent_artifacts(self, agent_name: str) -> None: for pack_id, metadata in list(self.registry.list().items()): updates: Dict[str, Any] = {} + pending_paths: set[Path] = set() raw_skills = metadata.get("registered_skills", []) if isinstance(raw_skills, list) and raw_skills: @@ -784,6 +790,12 @@ def unregister_agent_artifacts(self, agent_name: str) -> None: if name not in shared_names ] if command_names_to_unregister: + if require_complete: + pending_paths.update(registrar.registered_artifact_paths( + agent_name, self.project_root, + commands=command_names_to_unregister, skills=[], + registrar_config=agent_config, + )) self._unregister_commands( {agent_name: command_names_to_unregister} ) @@ -799,7 +811,7 @@ def unregister_agent_artifacts(self, agent_name: str) -> None: ): if agent_skill_names: self._delete_agent_preset_skills( - agent_name, agent_skill_names, pack_id + agent_name, agent_skill_names, pack_id, require_complete=require_complete, ) remaining = { other_agent: names @@ -808,6 +820,9 @@ def unregister_agent_artifacts(self, agent_name: str) -> None: } updates["registered_skills"] = remaining + retained = sorted(path for path in pending_paths if path.exists() or path.is_symlink()) + if retained: + raise ValueError("Preset artifacts were preserved: " + ", ".join(str(path) for path in retained)) if updates: self.registry.update(pack_id, updates) @@ -1232,6 +1247,7 @@ def _register_command_from_path( only_agent=only_agent, extra_agents=extra_agents, ) + @managed_registration def _register_for_non_skill_agents( self, registrar: Any, diff --git a/src/specify_cli/presets/_manager_skills.py b/src/specify_cli/presets/_manager_skills.py index b484093ade..b97e72efe3 100644 --- a/src/specify_cli/presets/_manager_skills.py +++ b/src/specify_cli/presets/_manager_skills.py @@ -14,7 +14,7 @@ from .._invocation_style import get_invocation_prefix from .._utils import dump_frontmatter from ..integrations.base import IntegrationBase -from ..integrations._registration import project_registration +from ..integrations._registration import managed_registration, project_registration from ._manager_commands import _substitute_core_template from ._manifest import PresetManifest, PresetValidationError from ._resolver import PresetResolver @@ -619,6 +619,7 @@ def _build_extension_skill_restore_index(self) -> Dict[str, Dict[str, Any]]: return restore_index @project_registration + @managed_registration def _register_skills( self, manifest: "PresetManifest", @@ -1242,11 +1243,14 @@ def skill_ownership_sources(self, pack_id: str) -> Dict[str, str]: return override_sources def _delete_agent_preset_skills( - self, agent_name: str, skill_names: List[str], pack_id: str + self, agent_name: str, skill_names: List[str], pack_id: str, *, require_complete: bool = False, ) -> None: """Delete still-preset-owned skills when an agent is deactivated.""" skills_dir = self._safe_skills_dir_for_agent(agent_name) if skills_dir is None: + candidate = self._resolve_agent_skills_dir(agent_name) + if require_complete and (candidate.exists() or candidate.is_symlink()): + raise ValueError(f"Preset skills directory was preserved: {candidate}") return registrar = self._command_registrar() marker = f"preset:{pack_id}" @@ -1258,13 +1262,19 @@ def _delete_agent_preset_skills( if not self._validate_skill_subdir( skill_subdir, create=False, skills_root=skills_dir ): + if require_complete: + raise ValueError(f"Preset skill was preserved: {skill_subdir}") continue skill_file = skill_subdir / "SKILL.md" if not skill_file.is_file(): + if require_complete and (skill_subdir.exists() or skill_subdir.is_symlink()): + raise ValueError(f"Preset skill was preserved: {skill_subdir}") continue try: content = skill_file.read_text(encoding="utf-8") except (OSError, UnicodeDecodeError): + if require_complete: + raise continue frontmatter, _ = registrar.parse_frontmatter(content) metadata = frontmatter.get("metadata") diff --git a/tests/specify_cli/integrations/test_installed_adapters.py b/tests/specify_cli/integrations/test_installed_adapters.py index fd20e4e7b8..47f9424294 100644 --- a/tests/specify_cli/integrations/test_installed_adapters.py +++ b/tests/specify_cli/integrations/test_installed_adapters.py @@ -179,11 +179,12 @@ def install(project): return result -def test_destroy_uses_trusted_external_lifecycle_and_contribution_cleanup(tmp_path, server): +@pytest.mark.parametrize("contribution", [("extension", "git"), ("preset", "lean")]) +def test_destroy_uses_trusted_external_lifecycle_and_contribution_cleanup(tmp_path, server, contribution): publish(server) project = catalog_project(tmp_path, server) install(project) - added = run(project, ["extension", "add", "git"]) + added = run(project, [contribution[0], "add", contribution[1]]) assert added.exit_code == 0, added.output source = project / "src/app.py" source.parent.mkdir() diff --git a/tests/specify_cli/integrations/test_operation_uninstall.py b/tests/specify_cli/integrations/test_operation_uninstall.py index c008436429..e1de8574e7 100644 --- a/tests/specify_cli/integrations/test_operation_uninstall.py +++ b/tests/specify_cli/integrations/test_operation_uninstall.py @@ -53,7 +53,7 @@ def test_wrong_key_is_a_typed_error_without_mutation(tmp_path): def test_complete_cleanup_failure_retains_integration_metadata(tmp_path, monkeypatch): project = _init_project(tmp_path, "claude") - def fail(*args): + def fail(*args, **kwargs): raise OSError("contribution cleanup failed") monkeypatch.setattr("specify_cli.extensions.ExtensionManager.unregister_agent_artifacts", fail) @@ -64,3 +64,101 @@ def fail(*args): state = json.loads((project / ".specify/integration.json").read_text()) assert "claude" in state["installed_integrations"] assert (project / ".specify/integrations/claude.manifest.json").exists() + + +def test_strict_generic_dangling_output_keeps_recovery_state(tmp_path): + from specify_cli._operation_destroy import DestroyError, DestroyRequest, destroy_project + from tests.specify_cli.test_operation_destroy import symlink, write + + project = _init_project(tmp_path, "generic", "--commands-dir .custom/commands") + write(project, ".specify/init-options.json", + b'{"ai":"generic","integration":"generic","allow_symlinks":true,"commands_dir":".custom/commands"}') + relative = ".custom/commands/speckit.sample.md" + write(project, ".specify/extensions/.registry", json.dumps({ + "extensions": {"sample": { + "registered_commands": {"generic": ["speckit.sample"]}, + "generic_artifact_hashes": {relative: "a" * 64}, + }}, + }).encode()) + output = project / relative + output.parent.mkdir(parents=True, exist_ok=True) + symlink(output, output.parent / "missing") + with pytest.raises(DestroyError): + destroy_project(DestroyRequest(project, confirmed=True, force=True)) + assert output.is_symlink() + assert (project / ".specify/integration.json").exists() + assert (project / ".specify/integrations/generic.manifest.json").exists() + registry = json.loads((project / ".specify/extensions/.registry").read_text()) + assert registry["extensions"]["sample"]["generic_artifact_hashes"] == {relative: "a" * 64} + + +@pytest.mark.parametrize("kind", ["presets", "extensions"]) +def test_strict_skipped_skill_symlink_keeps_registration_and_allows_retry(tmp_path, kind): + from specify_cli._operation_destroy import DestroyError, DestroyRequest, destroy_project + from tests.specify_cli.test_operation_destroy import symlink, write + + project = _init_project(tmp_path, "claude") + write(project, ".specify/init-options.json", b'{"ai":"claude","allow_symlinks":true}') + skills = {"claude": ["speckit-sample"]} if kind == "presets" else ["speckit-sample"] + registry_path = write(project, f".specify/{kind}/.registry", json.dumps({ + kind: {"sample": {"registered_skills": skills}}, + }).encode()) + target = write(project, "retained-skill/SKILL.md", + f"---\nmetadata:\n source: {kind[:-1]}:sample\n---\nOwned\n".encode()).parent + link = project / ".claude/skills/speckit-sample" + symlink(link, target, directory=True) + with pytest.raises(DestroyError, match="preserved"): + destroy_project(DestroyRequest(project, confirmed=True)) + assert (project / ".specify/integration.json").exists() + assert (project / ".specify/integrations/claude.manifest.json").exists() + assert json.loads(registry_path.read_text())[kind]["sample"]["registered_skills"] == skills + link.unlink() + link.mkdir() + (link / "SKILL.md").write_bytes((target / "SKILL.md").read_bytes()) + destroy_project(DestroyRequest(project, confirmed=True)) + assert not (project / ".specify").exists() + + +def test_strict_late_options_failure_restores_state_and_allows_retry(tmp_path, monkeypatch): + from specify_cli._operation_destroy import DestroyRequest, destroy_project + from specify_cli.integrations import _helpers + + project = _init_project(tmp_path, "claude") + paths = [ + project / ".specify/integration.json", project / ".specify/init-options.json", + project / ".specify/integrations/claude.manifest.json", + ] + before = {path: path.read_bytes() for path in paths} + clear = _helpers._clear_init_options_for_integration + + def fail(*args): + clear(*args) + raise PermissionError("late options write failed") + + with monkeypatch.context() as fault: + fault.setattr(_helpers, "_clear_init_options_for_integration", fail) + with pytest.raises(IntegrationUninstallError, match="late options write failed"): + uninstall_integration(IntegrationUninstallRequest(project, "claude", require_complete=True)) + assert {path: path.read_bytes() for path in paths} == before + destroy_project(DestroyRequest(project, confirmed=True)) + assert not (project / ".specify").exists() + + +@pytest.mark.parametrize("exit_code", [0, 3]) +def test_teardown_system_exit_is_a_typed_error_with_recovery_state(tmp_path, monkeypatch, exit_code): + from specify_cli.integrations import get_integration + from specify_cli.integrations.manifest import IntegrationManifest + + project = _init_project(tmp_path, "claude") + record = IntegrationManifest.load("claude", project) + before = record.manifest_path.read_bytes() + + def teardown(root, manifest, **kwargs): + manifest.uninstall(root, **kwargs) + raise SystemExit(exit_code) + + monkeypatch.setattr(get_integration("claude"), "teardown", teardown) + with pytest.raises(IntegrationUninstallError, match="exited"): + uninstall_integration(IntegrationUninstallRequest(project, "claude", require_complete=True)) + assert record.manifest_path.read_bytes() == before + assert (project / ".specify/integration.json").exists() diff --git a/tests/specify_cli/test_command_destroy.py b/tests/specify_cli/test_command_destroy.py index 5db21e85a2..d98b7e93ea 100644 --- a/tests/specify_cli/test_command_destroy.py +++ b/tests/specify_cli/test_command_destroy.py @@ -179,6 +179,42 @@ def test_current_directory_and_project_override(tmp_path, monkeypatch): assert not (target / ".specify").exists() +def test_invalid_environment_override_is_not_a_successful_noop(tmp_path, monkeypatch): + monkeypatch.setenv("SPECIFY_INIT_DIR", str(tmp_path)) + result = runner.invoke(app, ["destroy", "--yes", "--json"]) + assert result.exit_code == 1 + error = json.loads(result.stderr)["error"] + assert error["code"] == "invalid_project_directory" + assert "SPECIFY_INIT_DIR is not a Spec Kit project" in error["message"] + explicit = runner.invoke(app, ["destroy", "--project", str(tmp_path), "--yes", "--json"]) + assert explicit.exit_code == 0, explicit.output + + +@pytest.mark.parametrize("exit_code", [0, 3]) +def test_teardown_system_exit_uses_json_error_contract(tmp_path, monkeypatch, exit_code): + from specify_cli.integrations import get_integration + from tests.specify_cli.integrations._helpers import _init_project + + project = _init_project(tmp_path, "claude") + record = project / ".specify/integrations/claude.manifest.json" + original = record.read_bytes() + + def teardown(root, manifest, **kwargs): + manifest.uninstall(root, **kwargs) + raise SystemExit(exit_code) + + monkeypatch.setattr(get_integration("claude"), "teardown", teardown) + result = runner.invoke(app, ["destroy", "--project", str(project), "--yes", "--json"]) + assert result.exit_code == 1 + assert not result.stdout + error = json.loads(result.stderr)["error"] + assert error["code"] == "destroy_failed" + assert "teardown exited" in error["message"] + assert error["details"]["removed"] + assert record.read_bytes() == original + assert (project / ".specify/integration.json").exists() + + def test_json_error_sanitizes_unexpected_failure(tmp_path, monkeypatch): def fail(*args, **kwargs): raise RuntimeError("private detail") diff --git a/tests/specify_cli/test_operation_destroy.py b/tests/specify_cli/test_operation_destroy.py index a7e7778c3a..b2e7b3d0b0 100644 --- a/tests/specify_cli/test_operation_destroy.py +++ b/tests/specify_cli/test_operation_destroy.py @@ -1337,3 +1337,202 @@ def test_dispatcher_earlier_stale_hash_does_not_override_last_owner(tmp_path): assert not destroy_project(DestroyRequest(tmp_path, dry_run=True)).requires_force destroy_project(DestroyRequest(tmp_path, confirmed=True)) assert not (tmp_path / ".specify").exists() + + +def test_inventory_does_not_read_arbitrary_tree_payloads(tmp_path, monkeypatch): + from specify_cli.file_helper import FileHelper + + write(tmp_path, ".specify/cache/large.bin", b"unreadable but deletable") + write(tmp_path, "specs/001/assets/large.bin", b"unreadable but deletable") + read = FileHelper.read_bytes + + def deny_payload(self, path): + if str(path).endswith(".bin"): + raise PermissionError("payload must not be read") + return read(self, path) + + monkeypatch.setattr(FileHelper, "read_bytes", deny_payload) + assert len(plan_destroy(DestroyRequest(tmp_path)).files) == 2 + destroy_project(DestroyRequest(tmp_path, confirmed=True)) + assert not (tmp_path / ".specify").exists() + assert not (tmp_path / "specs").exists() + + +def test_inventory_still_rejects_reparse_point_ancestors_without_payload_reads(tmp_path, monkeypatch): + import stat + from types import SimpleNamespace + + manifest(tmp_path) + lstat = Path.lstat + + def junction(path, **kwargs): + entry = lstat(path, **kwargs) + if path == tmp_path / ".github": + return SimpleNamespace( + st_mode=entry.st_mode, st_file_attributes=stat.FILE_ATTRIBUTE_REPARSE_POINT, + ) + return entry + + monkeypatch.setattr(Path, "lstat", junction) + with pytest.raises(DestroyError, match="Unsupported reparse point"): + plan_destroy(DestroyRequest(tmp_path)) + + +@pytest.mark.parametrize("identifier", ["../outside", "/outside", "bad\\id", "sample\n"]) +def test_preset_ids_are_validated_before_provenance_reads(tmp_path, monkeypatch, identifier): + from specify_cli.presets import PresetManager + + write(tmp_path, ".specify/presets/.registry", json.dumps({ + "presets": {identifier: {"registered_skills": {}}}, + }).encode()) + + def unexpected(*args): + pytest.fail("unsafe preset ID reached provenance reader") + + monkeypatch.setattr(PresetManager, "skill_ownership_sources", unexpected) + with pytest.raises(DestroyError, match="Invalid preset"): + plan_destroy(DestroyRequest(tmp_path)) + + +def test_preset_manifest_symlink_is_denied_even_when_project_links_are_allowed(tmp_path, monkeypatch): + from specify_cli.presets import PresetManager + + write(tmp_path, ".specify/init-options.json", b'{"allow_symlinks":true}') + write(tmp_path, ".specify/presets/.registry", b'{"presets":{"sample":{}}}') + target = write(tmp_path, "user-preset.yml", b"not metadata") + link = tmp_path / ".specify/presets/sample/preset.yml" + link.parent.mkdir() + symlink(link, target) + + def unexpected(*args): + pytest.fail("linked preset manifest reached provenance reader") + + monkeypatch.setattr(PresetManager, "skill_ownership_sources", unexpected) + with pytest.raises(DestroyError, match="symlink"): + plan_destroy(DestroyRequest(tmp_path)) + + +@pytest.mark.parametrize("legacy", [False, True]) +def test_preset_flat_skill_candidates_do_not_claim_unrelated_agents(tmp_path, legacy): + write(tmp_path, ".specify/presets/.registry", json.dumps({ + "presets": {"sample": { + "registered_skills": ["speckit-sample"] if legacy else { + "copilot": ["speckit-sample"], "claude": ["speckit-sample"], + }, + }}, + }).encode()) + owned = write(tmp_path, ".github/skills/speckit-sample/SKILL.md", + b"---\nmetadata:\n source: preset:sample\n---\nOwned\n") + unrelated = write(tmp_path, ".claude/skills/speckit-sample/SKILL.md", b"user skill") + if not legacy: + with pytest.raises(DestroyError, match="Cannot establish skill ownership"): + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert owned.exists() + else: + destroy_project(DestroyRequest(tmp_path, confirmed=True, force=True)) + assert not owned.exists() + assert not (tmp_path / ".specify").exists() + assert unrelated.read_bytes() == b"user skill" + + +@pytest.mark.parametrize("installed", [False, True]) +def test_hermes_dotted_preset_skill_uses_declared_home_scope(tmp_path, monkeypatch, installed): + home = tmp_path / "home" + home.mkdir() + root = tmp_path / "project" + root.mkdir() + monkeypatch.setattr(Path, "home", lambda: home) + if installed: + manifest(root, "hermes", ".hermes/marker") + write(root, ".specify/presets/.registry", json.dumps({ + "presets": {"sample": {"registered_skills": {"hermes": ["speckit.sample"]}}}, + }).encode()) + skill = write(home, ".hermes/skills/speckit.sample/SKILL.md", + b"---\nmetadata:\n source: preset:sample\n---\nOwned\n") + unrelated = write(home, ".hermes/skills/user/SKILL.md", b"user") + if not installed: + before = snapshot(tmp_path) + with pytest.raises(DestroyError, match="outside its declared uninstall scope"): + destroy_project(DestroyRequest(root, confirmed=True, force=True)) + assert snapshot(tmp_path) == before + return + preview = plan_destroy(DestroyRequest(root)) + assert str(skill) in preview.global_files + destroy_project(DestroyRequest(root, confirmed=True)) + assert not skill.exists() + assert unrelated.read_bytes() == b"user" + assert not (root / ".specify").exists() + + +@pytest.mark.parametrize("failure", ["teardown", "final_cleanup", "incomplete"]) +def test_failures_report_completed_project_and_home_deletions(tmp_path, monkeypatch, failure): + from specify_cli.file_helper import FileHelper + from specify_cli.integrations import get_integration + + home = tmp_path / "home" + home.mkdir() + root = tmp_path / "project" + root.mkdir() + monkeypatch.setattr(Path, "home", lambda: home) + record = manifest(root, "hermes", ".hermes/generated") + project_file = root / ".hermes/generated" + global_file = write(home, ".hermes/skills/speckit-sample-check/SKILL.md") + write(root, ".specify/extensions/.registry", json.dumps({ + "extensions": {"sample": {"registered_commands": {"hermes": ["speckit.sample.check"]}}}, + }).encode()) + blocked = write(root, ".specify/blocked") + teardown = get_integration("hermes").teardown + delete = FileHelper.delete + + def fail_teardown(*args, **kwargs): + result = teardown(*args, **kwargs) + if failure == "teardown": + raise PermissionError("after teardown") + if failure == "incomplete": + write(root, ".specify/events.py", b"unexpected retained dispatcher") + return result + + def fail_delete(self, path, **kwargs): + if self.root / path == blocked: + raise PermissionError("after integration cleanup") + return delete(self, path, **kwargs) + + monkeypatch.setattr(get_integration("hermes"), "teardown", fail_teardown) + if failure == "final_cleanup": + monkeypatch.setattr(FileHelper, "delete", fail_delete) + with pytest.raises(DestroyError) as caught: + destroy_project(DestroyRequest(root, confirmed=True)) + assert ".hermes/generated" in caught.value.details["removed"] + assert str(global_file) in caught.value.details["global_files"] + assert not project_file.exists() + assert not global_file.exists() + assert record.manifest_path.exists() is (failure != "final_cleanup"), str(caught.value) + + +@pytest.mark.parametrize("integration", ["copilot", "claude", "gemini"]) +def test_later_preset_add_refreshes_only_managed_overwrites(tmp_path, monkeypatch, integration): + monkeypatch.chdir(tmp_path) + runner = CliRunner() + result = runner.invoke(app, [ + "init", "--here", "--force", "--non-interactive", "--ignore-agent-tools", + "--integration", integration, + ]) + assert result.exit_code == 0, result.output + record = IntegrationManifest.load(integration, tmp_path) + untouched = ".specify/templates/untouched.md" + record.record_file(untouched, b"original template") + record.save() + write(tmp_path, untouched, b"user customization") + result = runner.invoke(app, ["preset", "add", "lean"]) + assert result.exit_code == 0, result.output + assert plan_destroy(DestroyRequest(tmp_path)).requires_force == () + refreshed = IntegrationManifest.load(integration, tmp_path) + assert untouched in refreshed.check_modified() + relative = next(name for name in refreshed.files if "speckit" in name and not name.startswith(".specify/")) + original = (tmp_path / relative).read_bytes() + write(tmp_path, relative, b"user edit") + assert relative in plan_destroy(DestroyRequest(tmp_path)).requires_force + write(tmp_path, relative, original) + write(tmp_path, untouched, b"original template") + destroy_project(DestroyRequest(tmp_path, confirmed=True)) + assert not (tmp_path / ".specify").exists()