From f81490c4e9cb70eeb3642b6230b4f659eab0bee3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Apr 2026 19:29:08 +0000 Subject: [PATCH 01/17] Initial plan From 5391d771acc84c55309238d5007c106912c68e8a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Apr 2026 19:53:50 +0000 Subject: [PATCH 02/17] Add specify integration subcommand (list, install, uninstall, switch) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the `specify integration` subcommand group for managing integrations in existing projects after initial setup: - `specify integration list` — shows available integrations and installed status - `specify integration install ` — installs an integration into existing project - `specify integration uninstall [key]` — hash-safe removal preserving modified files - `specify integration switch ` — uninstalls current, installs target Follows the established `specify ` CLI pattern used by extensions and presets. Shared infrastructure (scripts, templates) is preserved during uninstall and switch operations. Agent-Logs-Url: https://github.com/github/spec-kit/sessions/1cca6c84-3e12-465d-88b8-a646d3504f63 Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com> --- src/specify_cli/__init__.py | 362 ++++++++++++++++ .../test_integration_subcommand.py | 397 ++++++++++++++++++ 2 files changed, 759 insertions(+) create mode 100644 tests/integrations/test_integration_subcommand.py diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 28831e6cd2..e8886c9b6a 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1486,6 +1486,368 @@ def get_speckit_version() -> str: return "unknown" +# ===== Integration Commands ===== + +integration_app = typer.Typer( + name="integration", + help="Manage AI agent integrations", + add_completion=False, +) +app.add_typer(integration_app, name="integration") + + +INTEGRATION_JSON = ".specify/integration.json" + + +def _read_integration_json(project_root: Path) -> dict[str, Any]: + """Load ``.specify/integration.json``. Returns ``{}`` on missing/corrupt.""" + path = project_root / INTEGRATION_JSON + if not path.exists(): + return {} + try: + return json.loads(path.read_text(encoding="utf-8")) + except (json.JSONDecodeError, OSError): + return {} + + +def _write_integration_json( + project_root: Path, + integration_key: str, + script_type: str, +) -> None: + """Write ``.specify/integration.json`` for *integration_key*.""" + script_ext = "sh" if script_type == "sh" else "ps1" + dest = project_root / INTEGRATION_JSON + dest.parent.mkdir(parents=True, exist_ok=True) + dest.write_text(json.dumps({ + "integration": integration_key, + "version": get_speckit_version(), + "scripts": { + "update-context": f".specify/integrations/{integration_key}/scripts/update-context.{script_ext}", + }, + }, indent=2) + "\n", encoding="utf-8") + + +def _remove_integration_json(project_root: Path) -> None: + """Remove ``.specify/integration.json`` if it exists.""" + path = project_root / INTEGRATION_JSON + if path.exists(): + path.unlink() + + +def _resolve_script_type(project_root: Path, script_type: str | None) -> str: + """Resolve the script type from the CLI flag or init-options.json.""" + if script_type: + return script_type + opts = load_init_options(project_root) + saved = opts.get("script") + if saved: + return saved + return "ps" if os.name == "nt" else "sh" + + +@integration_app.command("list") +def integration_list(): + """List available integrations and installed status.""" + from .integrations import INTEGRATION_REGISTRY + + project_root = Path.cwd() + + specify_dir = project_root / ".specify" + if not specify_dir.exists(): + console.print("[red]Error:[/red] Not a spec-kit project (no .specify/ directory)") + console.print("Run this command from a spec-kit project root") + raise typer.Exit(1) + + current = _read_integration_json(project_root) + installed_key = current.get("integration") + + table = Table(title="AI Agent Integrations") + table.add_column("Key", style="cyan") + table.add_column("Name") + table.add_column("Status") + table.add_column("CLI Required") + + for key in sorted(INTEGRATION_REGISTRY.keys()): + integration = INTEGRATION_REGISTRY[key] + cfg = integration.config or {} + name = cfg.get("name", key) + requires_cli = cfg.get("requires_cli", False) + + if key == installed_key: + status = "[green]installed[/green]" + else: + status = "" + + cli_req = "yes" if requires_cli else "no (IDE)" + table.add_row(key, name, status, cli_req) + + console.print(table) + + if installed_key: + console.print(f"\n[dim]Current integration:[/dim] [cyan]{installed_key}[/cyan]") + else: + console.print("\n[yellow]No integration currently installed.[/yellow]") + console.print("Install one with: [cyan]specify integration install [/cyan]") + + +@integration_app.command("install") +def integration_install( + key: str = typer.Argument(help="Integration key to install (e.g. claude, copilot)"), + script: str = typer.Option(None, "--script", help="Script type: sh or ps (default: from init-options.json or platform default)"), + integration_options: str = typer.Option(None, "--integration-options", help='Options for the integration (e.g. --integration-options="--commands-dir .myagent/cmds")'), + force: bool = typer.Option(False, "--force", help="Overwrite existing integration without uninstalling first"), +): + """Install an integration into an existing project.""" + from .integrations import INTEGRATION_REGISTRY, get_integration + from .integrations.manifest import IntegrationManifest + + project_root = Path.cwd() + + specify_dir = project_root / ".specify" + if not specify_dir.exists(): + console.print("[red]Error:[/red] Not a spec-kit project (no .specify/ directory)") + console.print("Run this command from a spec-kit project root") + raise typer.Exit(1) + + integration = get_integration(key) + if integration is None: + console.print(f"[red]Error:[/red] Unknown integration '{key}'") + available = ", ".join(sorted(INTEGRATION_REGISTRY.keys())) + console.print(f"Available integrations: {available}") + raise typer.Exit(1) + + current = _read_integration_json(project_root) + installed_key = current.get("integration") + + if installed_key and installed_key == key and not force: + console.print(f"[yellow]Integration '{key}' is already installed.[/yellow]") + console.print("Use [cyan]--force[/cyan] to reinstall, or [cyan]specify integration switch [/cyan] to change.") + raise typer.Exit(0) + + if installed_key and installed_key != key and not force: + console.print(f"[red]Error:[/red] Integration '{installed_key}' is already installed.") + console.print(f"Use [cyan]specify integration switch {key}[/cyan] to switch, or [cyan]--force[/cyan] to overwrite.") + raise typer.Exit(1) + + selected_script = _resolve_script_type(project_root, script) + + manifest = IntegrationManifest( + integration.key, project_root, version=get_speckit_version() + ) + + # Build parsed options from --integration-options + parsed_options: dict[str, Any] | None = None + if integration_options: + parsed_options = _parse_integration_options(integration, integration_options) + + try: + integration.setup( + project_root, manifest, + parsed_options=parsed_options, + script_type=selected_script, + raw_options=integration_options, + ) + manifest.save() + _write_integration_json(project_root, integration.key, selected_script) + + # Update init-options.json to reflect the new integration + opts = load_init_options(project_root) + opts["integration"] = integration.key + opts["ai"] = integration.key + from .integrations.base import SkillsIntegration + if isinstance(integration, SkillsIntegration): + opts["ai_skills"] = True + save_init_options(project_root, opts) + + except Exception as e: + console.print(f"[red]Error:[/red] Failed to install integration: {e}") + raise typer.Exit(1) + + name = (integration.config or {}).get("name", key) + console.print(f"\n[green]✓[/green] Integration '{name}' installed successfully") + + +def _parse_integration_options(integration: Any, raw_options: str) -> dict[str, Any]: + """Parse --integration-options string into a dict matching the integration's declared options.""" + import shlex + parsed: dict[str, Any] = {} + tokens = shlex.split(raw_options) + declared = {opt.name.lstrip("-"): opt for opt in integration.options()} + i = 0 + while i < len(tokens): + token = tokens[i] + name = token.lstrip("-") + opt = declared.get(name) + if opt and opt.is_flag: + parsed[name.replace("-", "_")] = True + i += 1 + elif opt and i + 1 < len(tokens): + parsed[name.replace("-", "_")] = tokens[i + 1] + i += 2 + else: + i += 1 + return parsed or None + + +@integration_app.command("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 .integrations import get_integration + from .integrations.manifest import IntegrationManifest + + project_root = Path.cwd() + + specify_dir = project_root / ".specify" + if not specify_dir.exists(): + console.print("[red]Error:[/red] Not a spec-kit project (no .specify/ directory)") + console.print("Run this command from a spec-kit project root") + raise typer.Exit(1) + + current = _read_integration_json(project_root) + installed_key = current.get("integration") + + if key is None: + if not installed_key: + console.print("[yellow]No integration is currently installed.[/yellow]") + raise typer.Exit(0) + key = installed_key + + if installed_key and installed_key != key: + console.print(f"[red]Error:[/red] Integration '{key}' is not the currently installed integration ('{installed_key}').") + raise typer.Exit(1) + + integration = get_integration(key) + if integration is None: + console.print(f"[red]Error:[/red] Unknown integration '{key}'") + raise typer.Exit(1) + + 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]") + _remove_integration_json(project_root) + raise typer.Exit(0) + + manifest = IntegrationManifest.load(key, project_root) + + removed, skipped = integration.teardown(project_root, manifest, force=force) + + _remove_integration_json(project_root) + + # Update init-options.json to clear the integration + opts = load_init_options(project_root) + if opts.get("integration") == key: + opts.pop("integration", None) + opts.pop("ai", None) + opts.pop("ai_skills", None) + save_init_options(project_root, opts) + + name = (integration.config or {}).get("name", key) + console.print(f"\n[green]✓[/green] Integration '{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 = path.relative_to(project_root) if path.is_absolute() else path + console.print(f" {rel}") + + +@integration_app.command("switch") +def integration_switch( + target: str = typer.Argument(help="Integration key to switch to"), + script: str = typer.Option(None, "--script", help="Script type: sh or ps (default: from init-options.json or platform default)"), + force: bool = typer.Option(False, "--force", help="Force removal of modified files during uninstall"), + integration_options: str = typer.Option(None, "--integration-options", help='Options for the target integration'), +): + """Switch from the current integration to a different one.""" + from .integrations import INTEGRATION_REGISTRY, get_integration + from .integrations.manifest import IntegrationManifest + + project_root = Path.cwd() + + specify_dir = project_root / ".specify" + if not specify_dir.exists(): + console.print("[red]Error:[/red] Not a spec-kit project (no .specify/ directory)") + console.print("Run this command from a spec-kit project root") + raise typer.Exit(1) + + target_integration = get_integration(target) + if target_integration is None: + console.print(f"[red]Error:[/red] Unknown integration '{target}'") + available = ", ".join(sorted(INTEGRATION_REGISTRY.keys())) + console.print(f"Available integrations: {available}") + raise typer.Exit(1) + + current = _read_integration_json(project_root) + installed_key = current.get("integration") + + if installed_key == target: + console.print(f"[yellow]Integration '{target}' is already installed. Nothing to switch.[/yellow]") + raise typer.Exit(0) + + selected_script = _resolve_script_type(project_root, script) + + # Phase 1: Uninstall current integration (if any) + if installed_key: + current_integration = get_integration(installed_key) + manifest_path = project_root / ".specify" / "integrations" / f"{installed_key}.manifest.json" + + if current_integration and manifest_path.exists(): + console.print(f"Uninstalling current integration: [cyan]{installed_key}[/cyan]") + old_manifest = IntegrationManifest.load(installed_key, project_root) + removed, skipped = current_integration.teardown( + project_root, old_manifest, force=force, + ) + if removed: + console.print(f" Removed {len(removed)} file(s)") + if skipped: + console.print(f" [yellow]⚠[/yellow] {len(skipped)} modified file(s) preserved") + else: + console.print(f"[dim]No manifest for '{installed_key}' — skipping uninstall phase[/dim]") + + # Phase 2: Install target integration + console.print(f"Installing integration: [cyan]{target}[/cyan]") + manifest = IntegrationManifest( + target_integration.key, project_root, version=get_speckit_version() + ) + + parsed_options: dict[str, Any] | None = None + if integration_options: + parsed_options = _parse_integration_options(target_integration, integration_options) + + try: + target_integration.setup( + project_root, manifest, + parsed_options=parsed_options, + script_type=selected_script, + raw_options=integration_options, + ) + manifest.save() + _write_integration_json(project_root, target_integration.key, selected_script) + + # Update init-options.json + opts = load_init_options(project_root) + opts["integration"] = target_integration.key + opts["ai"] = target_integration.key + from .integrations.base import SkillsIntegration + if isinstance(target_integration, SkillsIntegration): + opts["ai_skills"] = True + else: + opts.pop("ai_skills", None) + save_init_options(project_root, opts) + + except Exception as e: + console.print(f"[red]Error:[/red] Failed to install integration '{target}': {e}") + raise typer.Exit(1) + + name = (target_integration.config or {}).get("name", target) + console.print(f"\n[green]✓[/green] Switched to integration '{name}'") + + # ===== Preset Commands ===== diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py new file mode 100644 index 0000000000..3e93067f5f --- /dev/null +++ b/tests/integrations/test_integration_subcommand.py @@ -0,0 +1,397 @@ +"""Tests for ``specify integration`` subcommand (list, install, uninstall, switch).""" + +import json +import os + +import pytest +from typer.testing import CliRunner + +from specify_cli import app + + +runner = CliRunner() + + +def _init_project(tmp_path, integration="copilot"): + """Helper: init a spec-kit project with the given integration.""" + project = tmp_path / "proj" + project.mkdir() + old_cwd = os.getcwd() + try: + os.chdir(project) + result = runner.invoke(app, [ + "init", "--here", + "--integration", integration, + "--script", "sh", + "--no-git", + "--ignore-agent-tools", + ], catch_exceptions=False) + finally: + os.chdir(old_cwd) + assert result.exit_code == 0, f"init failed: {result.output}" + return project + + +# ── list ───────────────────────────────────────────────────────────── + + +class TestIntegrationList: + def test_list_requires_speckit_project(self, tmp_path): + old_cwd = os.getcwd() + try: + os.chdir(tmp_path) + result = runner.invoke(app, ["integration", "list"]) + finally: + os.chdir(old_cwd) + assert result.exit_code != 0 + assert "Not a spec-kit project" in result.output + + def test_list_shows_installed(self, tmp_path): + project = _init_project(tmp_path, "copilot") + old_cwd = os.getcwd() + try: + os.chdir(project) + result = runner.invoke(app, ["integration", "list"]) + finally: + os.chdir(old_cwd) + assert result.exit_code == 0 + assert "copilot" in result.output + assert "installed" in result.output + + def test_list_shows_available_integrations(self, tmp_path): + project = _init_project(tmp_path, "copilot") + old_cwd = os.getcwd() + try: + os.chdir(project) + result = runner.invoke(app, ["integration", "list"]) + finally: + os.chdir(old_cwd) + assert result.exit_code == 0 + # Should show multiple integrations + assert "claude" in result.output + assert "gemini" in result.output + + +# ── install ────────────────────────────────────────────────────────── + + +class TestIntegrationInstall: + def test_install_requires_speckit_project(self, tmp_path): + old_cwd = os.getcwd() + try: + os.chdir(tmp_path) + result = runner.invoke(app, ["integration", "install", "claude"]) + finally: + os.chdir(old_cwd) + assert result.exit_code != 0 + assert "Not a spec-kit project" in result.output + + def test_install_unknown_integration(self, tmp_path): + project = _init_project(tmp_path) + old_cwd = os.getcwd() + try: + os.chdir(project) + result = runner.invoke(app, ["integration", "install", "nonexistent"]) + finally: + os.chdir(old_cwd) + assert result.exit_code != 0 + assert "Unknown integration" in result.output + + def test_install_already_installed(self, tmp_path): + project = _init_project(tmp_path, "copilot") + old_cwd = os.getcwd() + try: + os.chdir(project) + result = runner.invoke(app, ["integration", "install", "copilot"]) + finally: + os.chdir(old_cwd) + assert result.exit_code == 0 + assert "already installed" in result.output + + def test_install_different_when_one_exists(self, tmp_path): + project = _init_project(tmp_path, "copilot") + old_cwd = os.getcwd() + try: + os.chdir(project) + result = runner.invoke(app, ["integration", "install", "claude"]) + finally: + os.chdir(old_cwd) + assert result.exit_code != 0 + assert "already installed" in result.output + + def test_install_into_bare_project(self, tmp_path): + """Install into a project with .specify/ but no integration.""" + project = tmp_path / "bare" + project.mkdir() + (project / ".specify").mkdir() + old_cwd = os.getcwd() + try: + os.chdir(project) + result = runner.invoke(app, [ + "integration", "install", "claude", + "--script", "sh", + ], catch_exceptions=False) + finally: + os.chdir(old_cwd) + assert result.exit_code == 0, result.output + assert "installed successfully" in result.output + + # integration.json written + data = json.loads((project / ".specify" / "integration.json").read_text(encoding="utf-8")) + assert data["integration"] == "claude" + + # Manifest created + assert (project / ".specify" / "integrations" / "claude.manifest.json").exists() + + # Claude uses skills directory (not commands) + assert (project / ".claude" / "skills" / "speckit-plan" / "SKILL.md").exists() + + +# ── uninstall ──────────────────────────────────────────────────────── + + +class TestIntegrationUninstall: + def test_uninstall_requires_speckit_project(self, tmp_path): + old_cwd = os.getcwd() + try: + os.chdir(tmp_path) + result = runner.invoke(app, ["integration", "uninstall"]) + finally: + os.chdir(old_cwd) + assert result.exit_code != 0 + assert "Not a spec-kit project" in result.output + + def test_uninstall_no_integration(self, tmp_path): + project = tmp_path / "proj" + project.mkdir() + (project / ".specify").mkdir() + old_cwd = os.getcwd() + try: + os.chdir(project) + result = runner.invoke(app, ["integration", "uninstall"]) + finally: + os.chdir(old_cwd) + assert result.exit_code == 0 + assert "No integration" in result.output + + def test_uninstall_removes_files(self, tmp_path): + project = _init_project(tmp_path, "claude") + # Claude uses skills directory + assert (project / ".claude" / "skills" / "speckit-plan" / "SKILL.md").exists() + assert (project / ".specify" / "integrations" / "claude.manifest.json").exists() + + old_cwd = os.getcwd() + try: + os.chdir(project) + result = runner.invoke(app, ["integration", "uninstall"], catch_exceptions=False) + finally: + os.chdir(old_cwd) + assert result.exit_code == 0 + assert "uninstalled" in result.output + + # Command files removed + assert not (project / ".claude" / "skills" / "speckit-plan" / "SKILL.md").exists() + + # Manifest removed + assert not (project / ".specify" / "integrations" / "claude.manifest.json").exists() + + # integration.json removed + assert not (project / ".specify" / "integration.json").exists() + + def test_uninstall_preserves_modified_files(self, tmp_path): + """Full lifecycle: install → modify → uninstall → modified file kept.""" + project = _init_project(tmp_path, "claude") + plan_file = project / ".claude" / "skills" / "speckit-plan" / "SKILL.md" + assert plan_file.exists() + + # Modify a file + plan_file.write_text("# My custom plan command\n", encoding="utf-8") + + old_cwd = os.getcwd() + try: + os.chdir(project) + result = runner.invoke(app, ["integration", "uninstall"], catch_exceptions=False) + finally: + os.chdir(old_cwd) + assert result.exit_code == 0 + assert "preserved" in result.output + + # Modified file kept + assert plan_file.exists() + assert plan_file.read_text(encoding="utf-8") == "# My custom plan command\n" + + def test_uninstall_wrong_key(self, tmp_path): + project = _init_project(tmp_path, "copilot") + old_cwd = os.getcwd() + try: + os.chdir(project) + result = runner.invoke(app, ["integration", "uninstall", "claude"]) + finally: + os.chdir(old_cwd) + assert result.exit_code != 0 + assert "not the currently installed" in result.output + + def test_uninstall_preserves_shared_infra(self, tmp_path): + """Shared scripts and templates are not removed by integration uninstall.""" + project = _init_project(tmp_path, "claude") + shared_script = project / ".specify" / "scripts" / "bash" / "common.sh" + assert shared_script.exists() + + old_cwd = os.getcwd() + try: + os.chdir(project) + result = runner.invoke(app, ["integration", "uninstall"], catch_exceptions=False) + finally: + os.chdir(old_cwd) + assert result.exit_code == 0 + + # Shared infrastructure preserved + assert shared_script.exists() + assert (project / ".specify" / "templates").is_dir() + + +# ── switch ─────────────────────────────────────────────────────────── + + +class TestIntegrationSwitch: + def test_switch_requires_speckit_project(self, tmp_path): + old_cwd = os.getcwd() + try: + os.chdir(tmp_path) + result = runner.invoke(app, ["integration", "switch", "claude"]) + finally: + os.chdir(old_cwd) + assert result.exit_code != 0 + assert "Not a spec-kit project" in result.output + + def test_switch_unknown_target(self, tmp_path): + project = _init_project(tmp_path) + old_cwd = os.getcwd() + try: + os.chdir(project) + result = runner.invoke(app, ["integration", "switch", "nonexistent"]) + finally: + os.chdir(old_cwd) + assert result.exit_code != 0 + assert "Unknown integration" in result.output + + def test_switch_same_noop(self, tmp_path): + project = _init_project(tmp_path, "copilot") + old_cwd = os.getcwd() + try: + os.chdir(project) + result = runner.invoke(app, ["integration", "switch", "copilot"]) + finally: + os.chdir(old_cwd) + assert result.exit_code == 0 + assert "already installed" in result.output + + def test_switch_between_integrations(self, tmp_path): + project = _init_project(tmp_path, "claude") + # Verify claude files exist (claude uses skills) + assert (project / ".claude" / "skills" / "speckit-plan" / "SKILL.md").exists() + + old_cwd = os.getcwd() + try: + os.chdir(project) + result = runner.invoke(app, [ + "integration", "switch", "copilot", + "--script", "sh", + ], catch_exceptions=False) + finally: + os.chdir(old_cwd) + assert result.exit_code == 0, result.output + assert "Switched to" in result.output + + # Old claude files removed + assert not (project / ".claude" / "skills" / "speckit-plan" / "SKILL.md").exists() + + # New copilot files created + assert (project / ".github" / "agents" / "speckit.plan.agent.md").exists() + + # integration.json updated + data = json.loads((project / ".specify" / "integration.json").read_text(encoding="utf-8")) + assert data["integration"] == "copilot" + + def test_switch_preserves_shared_infra(self, tmp_path): + """Switching preserves shared scripts, templates, and memory.""" + project = _init_project(tmp_path, "claude") + shared_script = project / ".specify" / "scripts" / "bash" / "common.sh" + assert shared_script.exists() + shared_content = shared_script.read_text(encoding="utf-8") + + old_cwd = os.getcwd() + try: + os.chdir(project) + result = runner.invoke(app, [ + "integration", "switch", "copilot", + "--script", "sh", + ], catch_exceptions=False) + finally: + os.chdir(old_cwd) + assert result.exit_code == 0 + + # Shared infra untouched + assert shared_script.exists() + assert shared_script.read_text(encoding="utf-8") == shared_content + + def test_switch_from_nothing(self, tmp_path): + """Switch when no integration is installed should just install the target.""" + project = tmp_path / "bare" + project.mkdir() + (project / ".specify").mkdir() + old_cwd = os.getcwd() + try: + os.chdir(project) + result = runner.invoke(app, [ + "integration", "switch", "claude", + "--script", "sh", + ], catch_exceptions=False) + finally: + os.chdir(old_cwd) + assert result.exit_code == 0 + assert "Switched to" in result.output + + data = json.loads((project / ".specify" / "integration.json").read_text(encoding="utf-8")) + assert data["integration"] == "claude" + + +# ── Full lifecycle ─────────────────────────────────────────────────── + + +class TestIntegrationLifecycle: + def test_install_modify_uninstall_preserves_modified(self, tmp_path): + """Full lifecycle: install → modify file → uninstall → verify modified file kept.""" + project = tmp_path / "lifecycle" + project.mkdir() + (project / ".specify").mkdir() + + old_cwd = os.getcwd() + try: + os.chdir(project) + + # Install + result = runner.invoke(app, [ + "integration", "install", "claude", + "--script", "sh", + ], catch_exceptions=False) + assert result.exit_code == 0 + assert "installed successfully" in result.output + + # Claude uses skills directory + plan_file = project / ".claude" / "skills" / "speckit-plan" / "SKILL.md" + assert plan_file.exists() + + # Modify one file + plan_file.write_text("# user customization\n", encoding="utf-8") + + # Uninstall + result = runner.invoke(app, ["integration", "uninstall"], catch_exceptions=False) + assert result.exit_code == 0 + assert "preserved" in result.output + + # Modified file kept + assert plan_file.exists() + assert plan_file.read_text(encoding="utf-8") == "# user customization\n" + finally: + os.chdir(old_cwd) From a914d8d07ede08ab3f874aa4feabca09d167526f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Apr 2026 19:56:46 +0000 Subject: [PATCH 03/17] Address review feedback: extract helper, fix return type annotation - Extract _update_init_options_for_integration() to deduplicate init-options update logic between install and switch commands - Fix _parse_integration_options return type to dict[str, Any] | None Agent-Logs-Url: https://github.com/github/spec-kit/sessions/1cca6c84-3e12-465d-88b8-a646d3504f63 Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com> --- src/specify_cli/__init__.py | 45 +++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index e8886c9b6a..b7f532ed8c 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1650,15 +1650,7 @@ def integration_install( ) manifest.save() _write_integration_json(project_root, integration.key, selected_script) - - # Update init-options.json to reflect the new integration - opts = load_init_options(project_root) - opts["integration"] = integration.key - opts["ai"] = integration.key - from .integrations.base import SkillsIntegration - if isinstance(integration, SkillsIntegration): - opts["ai_skills"] = True - save_init_options(project_root, opts) + _update_init_options_for_integration(project_root, integration) except Exception as e: console.print(f"[red]Error:[/red] Failed to install integration: {e}") @@ -1668,8 +1660,11 @@ def integration_install( console.print(f"\n[green]✓[/green] Integration '{name}' installed successfully") -def _parse_integration_options(integration: Any, raw_options: str) -> dict[str, Any]: - """Parse --integration-options string into a dict matching the integration's declared options.""" +def _parse_integration_options(integration: Any, raw_options: str) -> dict[str, Any] | None: + """Parse --integration-options string into a dict matching the integration's declared options. + + Returns ``None`` when no recognised options are found. + """ import shlex parsed: dict[str, Any] = {} tokens = shlex.split(raw_options) @@ -1690,6 +1685,22 @@ def _parse_integration_options(integration: Any, raw_options: str) -> dict[str, return parsed or None +def _update_init_options_for_integration( + project_root: Path, + integration: Any, +) -> None: + """Update ``init-options.json`` to reflect *integration* as the active one.""" + from .integrations.base import SkillsIntegration + opts = load_init_options(project_root) + opts["integration"] = integration.key + opts["ai"] = integration.key + if isinstance(integration, SkillsIntegration): + opts["ai_skills"] = True + else: + opts.pop("ai_skills", None) + save_init_options(project_root, opts) + + @integration_app.command("uninstall") def integration_uninstall( key: str = typer.Argument(None, help="Integration key to uninstall (default: current integration)"), @@ -1828,17 +1839,7 @@ def integration_switch( ) manifest.save() _write_integration_json(project_root, target_integration.key, selected_script) - - # Update init-options.json - opts = load_init_options(project_root) - opts["integration"] = target_integration.key - opts["ai"] = target_integration.key - from .integrations.base import SkillsIntegration - if isinstance(target_integration, SkillsIntegration): - opts["ai_skills"] = True - else: - opts.pop("ai_skills", None) - save_init_options(project_root, opts) + _update_init_options_for_integration(project_root, target_integration) except Exception as e: console.print(f"[red]Error:[/red] Failed to install integration '{target}': {e}") From 8a36cd9baa42fa374c100acd78c75e18a3765557 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 3 Apr 2026 14:57:02 -0500 Subject: [PATCH 04/17] Potential fix for pull request finding 'Unused import' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> --- tests/integrations/test_integration_subcommand.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index 3e93067f5f..2ffcf07494 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -3,7 +3,6 @@ import json import os -import pytest from typer.testing import CliRunner from specify_cli import app From f5e975269c99c7cdaddfd65800bc68bfa4baebea Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 3 Apr 2026 15:16:27 -0500 Subject: [PATCH 05/17] Address review feedback: validate script type, handle --flag=value, fix metadata cleanup - Add _normalize_script_type() to validate script type against SCRIPT_TYPE_CHOICES - Handle --name=value syntax in _parse_integration_options() - Clear init-options.json keys in no-manifest uninstall early-return path - Clear stale metadata between switch teardown and install phases - Add 5 tests covering the new edge cases --- src/specify_cli/__init__.py | 40 +++++- .../test_integration_subcommand.py | 122 ++++++++++++++++++ 2 files changed, 159 insertions(+), 3 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index b7f532ed8c..de2f1675f1 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1535,14 +1535,26 @@ def _remove_integration_json(project_root: Path) -> None: path.unlink() +def _normalize_script_type(script_type: str, source: str) -> str: + """Normalize and validate a script type from CLI/config sources.""" + normalized = script_type.strip().lower() + if normalized in SCRIPT_TYPE_CHOICES: + return normalized + console.print( + f"[red]Error:[/red] Invalid script type {script_type!r} from {source}. " + f"Expected one of: {', '.join(sorted(SCRIPT_TYPE_CHOICES.keys()))}." + ) + raise typer.Exit(1) + + def _resolve_script_type(project_root: Path, script_type: str | None) -> str: """Resolve the script type from the CLI flag or init-options.json.""" if script_type: - return script_type + return _normalize_script_type(script_type, "--script") opts = load_init_options(project_root) saved = opts.get("script") - if saved: - return saved + if isinstance(saved, str) and saved.strip(): + return _normalize_script_type(saved, ".specify/init-options.json") return "ps" if os.name == "nt" else "sh" @@ -1673,10 +1685,17 @@ def _parse_integration_options(integration: Any, raw_options: str) -> dict[str, while i < len(tokens): token = tokens[i] name = token.lstrip("-") + value: str | None = None + # Handle --name=value syntax + if "=" in name: + name, value = name.split("=", 1) opt = declared.get(name) if opt and opt.is_flag: parsed[name.replace("-", "_")] = True i += 1 + elif opt and value is not None: + parsed[name.replace("-", "_")] = value + i += 1 elif opt and i + 1 < len(tokens): parsed[name.replace("-", "_")] = tokens[i + 1] i += 2 @@ -1740,6 +1759,13 @@ def integration_uninstall( if not manifest_path.exists(): console.print(f"[yellow]No manifest found for integration '{key}'. Nothing to uninstall.[/yellow]") _remove_integration_json(project_root) + # Clear integration-related keys from init-options.json + opts = load_init_options(project_root) + if opts.get("integration") == key: + opts.pop("integration", None) + opts.pop("ai", None) + opts.pop("ai_skills", None) + save_init_options(project_root, opts) raise typer.Exit(0) manifest = IntegrationManifest.load(key, project_root) @@ -1820,6 +1846,14 @@ def integration_switch( else: console.print(f"[dim]No manifest for '{installed_key}' — skipping uninstall phase[/dim]") + # Clear stale metadata so a failed Phase 2 doesn't reference the removed integration + _remove_integration_json(project_root) + opts = load_init_options(project_root) + opts.pop("integration", None) + opts.pop("ai", None) + opts.pop("ai_skills", None) + save_init_options(project_root, opts) + # Phase 2: Install target integration console.print(f"Installing integration: [cyan]{target}[/cyan]") manifest = IntegrationManifest( diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index 2ffcf07494..8fed05742f 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -394,3 +394,125 @@ def test_install_modify_uninstall_preserves_modified(self, tmp_path): assert plan_file.read_text(encoding="utf-8") == "# user customization\n" finally: os.chdir(old_cwd) + + +# ── Edge-case fixes ───────────────────────────────────────────────── + + +class TestScriptTypeValidation: + def test_invalid_script_type_rejected(self, tmp_path): + """--script with an invalid value should fail with a clear error.""" + project = tmp_path / "proj" + project.mkdir() + (project / ".specify").mkdir() + old_cwd = os.getcwd() + try: + os.chdir(project) + result = runner.invoke(app, [ + "integration", "install", "claude", + "--script", "bash", + ]) + finally: + os.chdir(old_cwd) + assert result.exit_code != 0 + assert "Invalid script type" in result.output + + def test_valid_script_types_accepted(self, tmp_path): + """Both 'sh' and 'ps' should be accepted.""" + project = tmp_path / "proj" + project.mkdir() + (project / ".specify").mkdir() + old_cwd = os.getcwd() + try: + os.chdir(project) + result = runner.invoke(app, [ + "integration", "install", "claude", + "--script", "sh", + ], catch_exceptions=False) + finally: + os.chdir(old_cwd) + assert result.exit_code == 0 + + +class TestParseIntegrationOptionsEqualsForm: + def test_equals_form_parsed(self): + """--commands-dir=./x should be parsed the same as --commands-dir ./x.""" + from specify_cli import _parse_integration_options + from specify_cli.integrations import get_integration + + integration = get_integration("generic") + assert integration is not None + + result_space = _parse_integration_options(integration, "--commands-dir ./mydir") + result_equals = _parse_integration_options(integration, "--commands-dir=./mydir") + assert result_space is not None + assert result_equals is not None + assert result_space["commands_dir"] == "./mydir" + assert result_equals["commands_dir"] == "./mydir" + + +class TestUninstallNoManifestClearsInitOptions: + def test_init_options_cleared_on_no_manifest_uninstall(self, tmp_path): + """When no manifest exists, uninstall should still clear init-options.json.""" + project = tmp_path / "proj" + project.mkdir() + (project / ".specify").mkdir() + + # Write integration.json and init-options.json without a manifest + int_json = project / ".specify" / "integration.json" + int_json.write_text(json.dumps({"integration": "claude"}), encoding="utf-8") + + opts_json = project / ".specify" / "init-options.json" + opts_json.write_text(json.dumps({ + "integration": "claude", + "ai": "claude", + "ai_skills": True, + "script": "sh", + }), encoding="utf-8") + + old_cwd = os.getcwd() + try: + os.chdir(project) + result = runner.invoke(app, ["integration", "uninstall", "claude"]) + finally: + os.chdir(old_cwd) + assert result.exit_code == 0 + + # init-options.json should have integration keys cleared + opts = json.loads(opts_json.read_text(encoding="utf-8")) + assert "integration" not in opts + assert "ai" not in opts + assert "ai_skills" not in opts + # Non-integration keys preserved + assert opts.get("script") == "sh" + + +class TestSwitchClearsMetadataAfterTeardown: + def test_metadata_cleared_between_phases(self, tmp_path): + """If install phase fails during switch, metadata should not reference the removed integration.""" + project = _init_project(tmp_path, "claude") + + # Verify initial state + int_json = project / ".specify" / "integration.json" + assert json.loads(int_json.read_text(encoding="utf-8"))["integration"] == "claude" + + old_cwd = os.getcwd() + try: + os.chdir(project) + # Switch to copilot — should succeed and update metadata + result = runner.invoke(app, [ + "integration", "switch", "copilot", + "--script", "sh", + ], catch_exceptions=False) + finally: + os.chdir(old_cwd) + assert result.exit_code == 0 + + # integration.json should reference copilot, not claude + data = json.loads(int_json.read_text(encoding="utf-8")) + assert data["integration"] == "copilot" + + # init-options.json should reference copilot + opts_json = project / ".specify" / "init-options.json" + opts = json.loads(opts_json.read_text(encoding="utf-8")) + assert opts.get("ai") == "copilot" From ffc1b669df2c74543663a3a705a45897545b6fb6 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 3 Apr 2026 15:26:32 -0500 Subject: [PATCH 06/17] Block --force with different integration, persist script type in init-options - --force on install now rejects overwriting a different integration; users must use 'specify integration switch' instead - _update_init_options_for_integration() now accepts and persists script_type - Fix misleading test docstring for switch metadata test - Add test_force_blocked_with_different_integration --- src/specify_cli/__init__.py | 16 ++++++++++++---- .../test_integration_subcommand.py | 18 +++++++++++++++++- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index de2f1675f1..7a47a33de5 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1637,9 +1637,14 @@ def integration_install( console.print("Use [cyan]--force[/cyan] to reinstall, or [cyan]specify integration switch [/cyan] to change.") raise typer.Exit(0) - if installed_key and installed_key != key and not force: + if installed_key and installed_key != key: console.print(f"[red]Error:[/red] Integration '{installed_key}' is already installed.") - console.print(f"Use [cyan]specify integration switch {key}[/cyan] to switch, or [cyan]--force[/cyan] to overwrite.") + console.print(f"Use [cyan]specify integration switch {key}[/cyan] to switch integrations.") + if force: + console.print( + "[yellow]--force only supports reinstalling the currently installed integration " + "and cannot overwrite a different integration.[/yellow]" + ) raise typer.Exit(1) selected_script = _resolve_script_type(project_root, script) @@ -1662,7 +1667,7 @@ def integration_install( ) manifest.save() _write_integration_json(project_root, integration.key, selected_script) - _update_init_options_for_integration(project_root, integration) + _update_init_options_for_integration(project_root, integration, script_type=selected_script) except Exception as e: console.print(f"[red]Error:[/red] Failed to install integration: {e}") @@ -1707,12 +1712,15 @@ def _parse_integration_options(integration: Any, raw_options: str) -> dict[str, def _update_init_options_for_integration( project_root: Path, integration: Any, + script_type: str | None = None, ) -> None: """Update ``init-options.json`` to reflect *integration* as the active one.""" from .integrations.base import SkillsIntegration opts = load_init_options(project_root) opts["integration"] = integration.key opts["ai"] = integration.key + if script_type: + opts["script"] = script_type if isinstance(integration, SkillsIntegration): opts["ai_skills"] = True else: @@ -1873,7 +1881,7 @@ def integration_switch( ) manifest.save() _write_integration_json(project_root, target_integration.key, selected_script) - _update_init_options_for_integration(project_root, target_integration) + _update_init_options_for_integration(project_root, target_integration, script_type=selected_script) except Exception as e: console.print(f"[red]Error:[/red] Failed to install integration '{target}': {e}") diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index 8fed05742f..b127aa47f9 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -118,6 +118,22 @@ def test_install_different_when_one_exists(self, tmp_path): assert result.exit_code != 0 assert "already installed" in result.output + def test_force_blocked_with_different_integration(self, tmp_path): + """--force must not allow overwriting a different integration.""" + project = _init_project(tmp_path, "copilot") + old_cwd = os.getcwd() + try: + os.chdir(project) + result = runner.invoke(app, [ + "integration", "install", "claude", "--force", + "--script", "sh", + ]) + finally: + os.chdir(old_cwd) + assert result.exit_code != 0 + assert "already installed" in result.output + assert "cannot overwrite a different integration" in result.output + def test_install_into_bare_project(self, tmp_path): """Install into a project with .specify/ but no integration.""" project = tmp_path / "bare" @@ -489,7 +505,7 @@ def test_init_options_cleared_on_no_manifest_uninstall(self, tmp_path): class TestSwitchClearsMetadataAfterTeardown: def test_metadata_cleared_between_phases(self, tmp_path): - """If install phase fails during switch, metadata should not reference the removed integration.""" + """After a successful switch, metadata should reference the new integration.""" project = _init_project(tmp_path, "claude") # Verify initial state From 94a348aebe6f0b100644ddfd1b748229fab40d30 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 3 Apr 2026 15:42:27 -0500 Subject: [PATCH 07/17] Remove --force from integration install, ensure shared infra on install/switch - Remove --force parameter entirely from integration install; users must uninstall before reinstalling to prevent orphaned files - Auto-install shared infrastructure (.specify/scripts/, .specify/templates/) when missing during install or switch - Add test for shared infra creation on bare project install --- src/specify_cli/__init__.py | 24 +++++++----- .../test_integration_subcommand.py | 38 +++++++++++-------- 2 files changed, 37 insertions(+), 25 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 7a47a33de5..608793472c 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1608,7 +1608,6 @@ def integration_install( key: str = typer.Argument(help="Integration key to install (e.g. claude, copilot)"), script: str = typer.Option(None, "--script", help="Script type: sh or ps (default: from init-options.json or platform default)"), integration_options: str = typer.Option(None, "--integration-options", help='Options for the integration (e.g. --integration-options="--commands-dir .myagent/cmds")'), - force: bool = typer.Option(False, "--force", help="Overwrite existing integration without uninstalling first"), ): """Install an integration into an existing project.""" from .integrations import INTEGRATION_REGISTRY, get_integration @@ -1632,23 +1631,24 @@ def integration_install( current = _read_integration_json(project_root) installed_key = current.get("integration") - if installed_key and installed_key == key and not force: + if installed_key and installed_key == key: console.print(f"[yellow]Integration '{key}' is already installed.[/yellow]") - console.print("Use [cyan]--force[/cyan] to reinstall, or [cyan]specify integration switch [/cyan] to change.") + console.print("Run [cyan]specify integration uninstall[/cyan] first, then reinstall.") raise typer.Exit(0) if installed_key and installed_key != key: console.print(f"[red]Error:[/red] Integration '{installed_key}' is already installed.") - console.print(f"Use [cyan]specify integration switch {key}[/cyan] to switch integrations.") - if force: - console.print( - "[yellow]--force only supports reinstalling the currently installed integration " - "and cannot overwrite a different integration.[/yellow]" - ) + console.print(f"Run [cyan]specify integration uninstall[/cyan] first, or use [cyan]specify integration switch {key}[/cyan].") raise typer.Exit(1) selected_script = _resolve_script_type(project_root, script) + # Ensure shared infrastructure exists + if not (project_root / ".specify" / "scripts").exists(): + _install_shared_infra(project_root, selected_script) + if os.name != "nt": + ensure_executable_scripts(project_root) + manifest = IntegrationManifest( integration.key, project_root, version=get_speckit_version() ) @@ -1862,6 +1862,12 @@ def integration_switch( opts.pop("ai_skills", None) save_init_options(project_root, opts) + # Ensure shared infrastructure exists + if not (project_root / ".specify" / "scripts").exists(): + _install_shared_infra(project_root, selected_script) + if os.name != "nt": + ensure_executable_scripts(project_root) + # Phase 2: Install target integration console.print(f"Installing integration: [cyan]{target}[/cyan]") manifest = IntegrationManifest( diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index b127aa47f9..f5322bdf5e 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -106,6 +106,7 @@ def test_install_already_installed(self, tmp_path): os.chdir(old_cwd) assert result.exit_code == 0 assert "already installed" in result.output + assert "uninstall" in result.output def test_install_different_when_one_exists(self, tmp_path): project = _init_project(tmp_path, "copilot") @@ -117,22 +118,7 @@ def test_install_different_when_one_exists(self, tmp_path): os.chdir(old_cwd) assert result.exit_code != 0 assert "already installed" in result.output - - def test_force_blocked_with_different_integration(self, tmp_path): - """--force must not allow overwriting a different integration.""" - project = _init_project(tmp_path, "copilot") - old_cwd = os.getcwd() - try: - os.chdir(project) - result = runner.invoke(app, [ - "integration", "install", "claude", "--force", - "--script", "sh", - ]) - finally: - os.chdir(old_cwd) - assert result.exit_code != 0 - assert "already installed" in result.output - assert "cannot overwrite a different integration" in result.output + assert "uninstall" in result.output def test_install_into_bare_project(self, tmp_path): """Install into a project with .specify/ but no integration.""" @@ -161,6 +147,26 @@ def test_install_into_bare_project(self, tmp_path): # Claude uses skills directory (not commands) assert (project / ".claude" / "skills" / "speckit-plan" / "SKILL.md").exists() + def test_install_bare_project_gets_shared_infra(self, tmp_path): + """Installing into a bare project should create shared scripts and templates.""" + project = tmp_path / "bare" + project.mkdir() + (project / ".specify").mkdir() + old_cwd = os.getcwd() + try: + os.chdir(project) + result = runner.invoke(app, [ + "integration", "install", "claude", + "--script", "sh", + ], catch_exceptions=False) + finally: + os.chdir(old_cwd) + assert result.exit_code == 0, result.output + + # Shared infrastructure should be present + assert (project / ".specify" / "scripts").is_dir() + assert (project / ".specify" / "templates").is_dir() + # ── uninstall ──────────────────────────────────────────────────────── From 42f3c5f8253343552eeb2ff0c258e1137bd3e341 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 3 Apr 2026 15:45:35 -0500 Subject: [PATCH 08/17] Remove redundant installed_key != key check The == key case already returns above, so the != key guard is always true at this point. Simplify to just 'if installed_key:'. --- src/specify_cli/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 608793472c..c4169fb591 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1636,7 +1636,7 @@ def integration_install( console.print("Run [cyan]specify integration uninstall[/cyan] first, then reinstall.") raise typer.Exit(0) - if installed_key and installed_key != key: + if installed_key: console.print(f"[red]Error:[/red] Integration '{installed_key}' is already installed.") console.print(f"Run [cyan]specify integration uninstall[/cyan] first, or use [cyan]specify integration switch {key}[/cyan].") raise typer.Exit(1) From a8318499866e1b0446df2527812e13fcb811a16f Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 3 Apr 2026 15:50:13 -0500 Subject: [PATCH 09/17] Run shared infra unconditionally, defer metadata removal in switch - Call _install_shared_infra() unconditionally on install and switch since it merges without overwriting existing files - Remove premature metadata cleanup between switch phases; metadata is now only updated after successful Phase 2 install --- src/specify_cli/__init__.py | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index c4169fb591..fcd65f56f3 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1643,11 +1643,11 @@ def integration_install( selected_script = _resolve_script_type(project_root, script) - # Ensure shared infrastructure exists - if not (project_root / ".specify" / "scripts").exists(): - _install_shared_infra(project_root, selected_script) - if os.name != "nt": - ensure_executable_scripts(project_root) + # Ensure shared infrastructure is present (safe to run unconditionally; + # _install_shared_infra merges missing files without overwriting). + _install_shared_infra(project_root, selected_script) + if os.name != "nt": + ensure_executable_scripts(project_root) manifest = IntegrationManifest( integration.key, project_root, version=get_speckit_version() @@ -1854,19 +1854,11 @@ def integration_switch( else: console.print(f"[dim]No manifest for '{installed_key}' — skipping uninstall phase[/dim]") - # Clear stale metadata so a failed Phase 2 doesn't reference the removed integration - _remove_integration_json(project_root) - opts = load_init_options(project_root) - opts.pop("integration", None) - opts.pop("ai", None) - opts.pop("ai_skills", None) - save_init_options(project_root, opts) - - # Ensure shared infrastructure exists - if not (project_root / ".specify" / "scripts").exists(): - _install_shared_infra(project_root, selected_script) - if os.name != "nt": - ensure_executable_scripts(project_root) + # Ensure shared infrastructure is present (safe to run unconditionally; + # _install_shared_infra merges missing files without overwriting). + _install_shared_infra(project_root, selected_script) + if os.name != "nt": + ensure_executable_scripts(project_root) # Phase 2: Install target integration console.print(f"Installing integration: [cyan]{target}[/cyan]") From e02a8a0f8669b295c7f4365ad45488e7f440f3d2 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 3 Apr 2026 15:59:09 -0500 Subject: [PATCH 10/17] Add install rollback, graceful manifest errors, clear switch metadata - Attempt teardown rollback on install/switch failure to avoid orphaned files - Catch ValueError/FileNotFoundError on IntegrationManifest.load() in uninstall with user-friendly recovery guidance - Clear metadata immediately after switch teardown so failed Phase 2 doesn't leave stale references to the removed integration --- src/specify_cli/__init__.py | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index fcd65f56f3..b9e29c35cc 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1670,6 +1670,11 @@ def integration_install( _update_init_options_for_integration(project_root, integration, script_type=selected_script) except Exception as e: + # Attempt rollback of any files written by setup + try: + integration.teardown(project_root, manifest, force=True) + except Exception: + pass console.print(f"[red]Error:[/red] Failed to install integration: {e}") raise typer.Exit(1) @@ -1776,7 +1781,14 @@ def integration_uninstall( save_init_options(project_root, opts) raise typer.Exit(0) - manifest = IntegrationManifest.load(key, project_root) + try: + manifest = IntegrationManifest.load(key, project_root) + except (ValueError, FileNotFoundError) as exc: + console.print(f"[red]Error:[/red] Integration manifest for '{key}' is unreadable.") + console.print(f"Manifest: {manifest_path}") + console.print("Delete the manifest file and run [cyan]specify integration install[/cyan] to recover.") + console.print(f"[dim]Details:[/dim] {exc}") + raise typer.Exit(1) removed, skipped = integration.teardown(project_root, manifest, force=force) @@ -1854,6 +1866,14 @@ def integration_switch( else: console.print(f"[dim]No manifest for '{installed_key}' — skipping uninstall phase[/dim]") + # Clear metadata so a failed Phase 2 doesn't leave stale references + _remove_integration_json(project_root) + opts = load_init_options(project_root) + opts.pop("integration", None) + opts.pop("ai", None) + opts.pop("ai_skills", None) + save_init_options(project_root, opts) + # Ensure shared infrastructure is present (safe to run unconditionally; # _install_shared_infra merges missing files without overwriting). _install_shared_infra(project_root, selected_script) @@ -1882,6 +1902,11 @@ def integration_switch( _update_init_options_for_integration(project_root, target_integration, script_type=selected_script) except Exception as e: + # Attempt rollback of any files written by setup + try: + target_integration.teardown(project_root, manifest, force=True) + except Exception: + pass console.print(f"[red]Error:[/red] Failed to install integration '{target}': {e}") raise typer.Exit(1) From f3deae348e78039a7181fc8f968fdd51f8664b48 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 3 Apr 2026 16:02:25 -0500 Subject: [PATCH 11/17] Log rollback failures instead of silently suppressing them --- src/specify_cli/__init__.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index b9e29c35cc..5f58d8c99b 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1673,8 +1673,9 @@ def integration_install( # Attempt rollback of any files written by setup try: integration.teardown(project_root, manifest, force=True) - except Exception: - pass + except Exception as rollback_err: + # Suppress so the original setup error remains the primary failure + console.print(f"[yellow]Warning:[/yellow] Failed to roll back integration changes: {rollback_err}") console.print(f"[red]Error:[/red] Failed to install integration: {e}") raise typer.Exit(1) @@ -1905,8 +1906,9 @@ def integration_switch( # Attempt rollback of any files written by setup try: target_integration.teardown(project_root, manifest, force=True) - except Exception: - pass + except Exception as rollback_err: + # Suppress so the original setup error remains the primary failure + console.print(f"[yellow]Warning:[/yellow] Failed to roll back integration '{target}': {rollback_err}") console.print(f"[red]Error:[/red] Failed to install integration '{target}': {e}") raise typer.Exit(1) From e7f14dec0a01bc8987842acc0de497adfc5ecece Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 3 Apr 2026 16:11:13 -0500 Subject: [PATCH 12/17] Handle corrupt manifest in switch, distinguish unknown vs missing manifest - Wrap IntegrationManifest.load() in switch with ValueError/FileNotFoundError handling, matching the pattern used in uninstall - Split else branch to report 'unknown integration' vs 'no manifest' separately --- src/specify_cli/__init__.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 5f58d8c99b..e805ccba11 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1856,7 +1856,13 @@ def integration_switch( if current_integration and manifest_path.exists(): console.print(f"Uninstalling current integration: [cyan]{installed_key}[/cyan]") - old_manifest = IntegrationManifest.load(installed_key, project_root) + try: + old_manifest = IntegrationManifest.load(installed_key, project_root) + except (ValueError, FileNotFoundError) as exc: + console.print(f"[red]Error:[/red] Could not read integration manifest for '{installed_key}': {manifest_path}") + console.print(f"[dim]{exc}[/dim]") + console.print("Try repairing or removing the manifest file, then rerun the command.") + raise typer.Exit(1) removed, skipped = current_integration.teardown( project_root, old_manifest, force=force, ) @@ -1864,6 +1870,8 @@ def integration_switch( console.print(f" Removed {len(removed)} file(s)") if skipped: console.print(f" [yellow]⚠[/yellow] {len(skipped)} modified file(s) preserved") + elif not current_integration: + console.print(f"[dim]Unknown installed integration '{installed_key}' — skipping uninstall phase[/dim]") else: console.print(f"[dim]No manifest for '{installed_key}' — skipping uninstall phase[/dim]") From c949fe74e3a69984086e6e57b62863a9f80f2dbd Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 3 Apr 2026 16:19:50 -0500 Subject: [PATCH 13/17] Clean up metadata on rollback, broaden init-options match in uninstall - Remove integration.json in install/switch rollback paths so failed installs don't leave stale metadata - Match on both 'integration' and 'ai' keys when clearing init-options.json during uninstall to handle partially-written metadata --- src/specify_cli/__init__.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index e805ccba11..2a2e4ccb9c 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1676,6 +1676,7 @@ def integration_install( except Exception as rollback_err: # Suppress so the original setup error remains the primary failure console.print(f"[yellow]Warning:[/yellow] Failed to roll back integration changes: {rollback_err}") + _remove_integration_json(project_root) console.print(f"[red]Error:[/red] Failed to install integration: {e}") raise typer.Exit(1) @@ -1775,7 +1776,7 @@ def integration_uninstall( _remove_integration_json(project_root) # Clear integration-related keys from init-options.json opts = load_init_options(project_root) - if opts.get("integration") == key: + if opts.get("integration") == key or opts.get("ai") == key: opts.pop("integration", None) opts.pop("ai", None) opts.pop("ai_skills", None) @@ -1797,7 +1798,7 @@ def integration_uninstall( # Update init-options.json to clear the integration opts = load_init_options(project_root) - if opts.get("integration") == key: + if opts.get("integration") == key or opts.get("ai") == key: opts.pop("integration", None) opts.pop("ai", None) opts.pop("ai_skills", None) @@ -1917,6 +1918,7 @@ def integration_switch( except Exception as rollback_err: # Suppress so the original setup error remains the primary failure console.print(f"[yellow]Warning:[/yellow] Failed to roll back integration '{target}': {rollback_err}") + _remove_integration_json(project_root) console.print(f"[red]Error:[/red] Failed to install integration '{target}': {e}") raise typer.Exit(1) From 3fdfa007109d16cc939a6a762f7b1a5f436e66ba Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 3 Apr 2026 16:33:03 -0500 Subject: [PATCH 14/17] Fix recovery guidance for unreadable manifests, fix type annotations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Recovery instructions now guide users through delete manifest → uninstall → reinstall workflow that actually works - Type annotations for optional CLI parameters changed from str to str | None --- src/specify_cli/__init__.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 2a2e4ccb9c..e7b8fcc681 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1606,8 +1606,8 @@ def integration_list(): @integration_app.command("install") def integration_install( key: str = typer.Argument(help="Integration key to install (e.g. claude, copilot)"), - script: str = typer.Option(None, "--script", help="Script type: sh or ps (default: from init-options.json or platform default)"), - integration_options: str = typer.Option(None, "--integration-options", help='Options for the integration (e.g. --integration-options="--commands-dir .myagent/cmds")'), + script: str | None = typer.Option(None, "--script", help="Script type: sh or ps (default: from init-options.json or platform default)"), + integration_options: str | None = typer.Option(None, "--integration-options", help='Options for the integration (e.g. --integration-options="--commands-dir .myagent/cmds")'), ): """Install an integration into an existing project.""" from .integrations import INTEGRATION_REGISTRY, get_integration @@ -1788,7 +1788,11 @@ def integration_uninstall( except (ValueError, FileNotFoundError) as exc: console.print(f"[red]Error:[/red] Integration manifest for '{key}' is unreadable.") console.print(f"Manifest: {manifest_path}") - console.print("Delete the manifest file and run [cyan]specify integration install[/cyan] to recover.") + 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) @@ -1818,9 +1822,9 @@ def integration_uninstall( @integration_app.command("switch") def integration_switch( target: str = typer.Argument(help="Integration key to switch to"), - script: str = typer.Option(None, "--script", help="Script type: sh or ps (default: from init-options.json or platform default)"), + script: str | None = typer.Option(None, "--script", help="Script type: sh or ps (default: from init-options.json or platform default)"), force: bool = typer.Option(False, "--force", help="Force removal of modified files during uninstall"), - integration_options: str = typer.Option(None, "--integration-options", help='Options for the target integration'), + integration_options: str | None = typer.Option(None, "--integration-options", help='Options for the target integration'), ): """Switch from the current integration to a different one.""" from .integrations import INTEGRATION_REGISTRY, get_integration @@ -1862,7 +1866,10 @@ def integration_switch( except (ValueError, FileNotFoundError) as exc: console.print(f"[red]Error:[/red] Could not read integration manifest for '{installed_key}': {manifest_path}") console.print(f"[dim]{exc}[/dim]") - console.print("Try repairing or removing the manifest file, then rerun the command.") + console.print( + f"To recover, delete the unreadable manifest at {manifest_path}, " + f"run [cyan]specify integration uninstall {installed_key}[/cyan], then retry." + ) raise typer.Exit(1) removed, skipped = current_integration.teardown( project_root, old_manifest, force=force, From 2e5bc221a11c41bf997caceaaed467ed722683bc Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 3 Apr 2026 16:41:09 -0500 Subject: [PATCH 15/17] Allow manifest-only uninstall for unknown/removed integrations - Uninstall no longer requires the integration to be in the registry; falls back to manifest.uninstall() directly when get_integration() returns None - Switch Phase 1 similarly uses manifest-only uninstall for unknown integrations instead of skipping teardown, preventing orphaned files --- src/specify_cli/__init__.py | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index e7b8fcc681..fccba4478a 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1766,9 +1766,6 @@ def integration_uninstall( raise typer.Exit(1) integration = get_integration(key) - if integration is None: - console.print(f"[red]Error:[/red] Unknown integration '{key}'") - raise typer.Exit(1) manifest_path = project_root / ".specify" / "integrations" / f"{key}.manifest.json" if not manifest_path.exists(): @@ -1796,7 +1793,7 @@ def integration_uninstall( console.print(f"[dim]Details:[/dim] {exc}") raise typer.Exit(1) - removed, skipped = integration.teardown(project_root, manifest, force=force) + removed, skipped = manifest.uninstall(project_root, force=force) _remove_integration_json(project_root) @@ -1808,7 +1805,7 @@ def integration_uninstall( opts.pop("ai_skills", None) save_init_options(project_root, opts) - name = (integration.config or {}).get("name", key) + name = (integration.config or {}).get("name", key) if integration else key console.print(f"\n[green]✓[/green] Integration '{name}' uninstalled") if removed: console.print(f" Removed {len(removed)} file(s)") @@ -1871,15 +1868,23 @@ def integration_switch( f"run [cyan]specify integration uninstall {installed_key}[/cyan], then retry." ) raise typer.Exit(1) - removed, skipped = current_integration.teardown( - project_root, old_manifest, force=force, - ) + removed, skipped = old_manifest.uninstall(project_root, force=force) if removed: console.print(f" Removed {len(removed)} file(s)") if skipped: console.print(f" [yellow]⚠[/yellow] {len(skipped)} modified file(s) preserved") - elif not current_integration: - console.print(f"[dim]Unknown installed integration '{installed_key}' — skipping uninstall phase[/dim]") + elif not current_integration and manifest_path.exists(): + # Integration removed from registry but manifest exists — use manifest-only uninstall + console.print(f"Uninstalling unknown integration '{installed_key}' via manifest") + try: + old_manifest = IntegrationManifest.load(installed_key, project_root) + removed, skipped = old_manifest.uninstall(project_root, force=force) + if removed: + console.print(f" Removed {len(removed)} file(s)") + if skipped: + console.print(f" [yellow]⚠[/yellow] {len(skipped)} modified file(s) preserved") + except (ValueError, FileNotFoundError) as exc: + console.print(f"[yellow]Warning:[/yellow] Could not read manifest for '{installed_key}': {exc}") else: console.print(f"[dim]No manifest for '{installed_key}' — skipping uninstall phase[/dim]") From 947a8e5eac30f41f3c571f82fa0593d86d3a8e2e Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 3 Apr 2026 16:53:16 -0500 Subject: [PATCH 16/17] Fail fast on corrupt integration.json, validate integration options - _read_integration_json() now exits with an actionable error when integration.json exists but is corrupt/unreadable - _parse_integration_options() rejects unknown options, validates flag usage, and requires values for non-flag options --- src/specify_cli/__init__.py | 49 ++++++++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index fccba4478a..1b160fdf92 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1500,14 +1500,22 @@ def get_speckit_version() -> str: def _read_integration_json(project_root: Path) -> dict[str, Any]: - """Load ``.specify/integration.json``. Returns ``{}`` on missing/corrupt.""" + """Load ``.specify/integration.json``. Returns ``{}`` when missing.""" path = project_root / INTEGRATION_JSON if not path.exists(): return {} try: return json.loads(path.read_text(encoding="utf-8")) - except (json.JSONDecodeError, OSError): - return {} + except json.JSONDecodeError as exc: + console.print(f"[red]Error:[/red] {path} contains invalid JSON.") + console.print(f"Please fix or delete {INTEGRATION_JSON} and retry.") + console.print(f"[dim]Details:[/dim] {exc}") + raise typer.Exit(1) + except OSError as exc: + console.print(f"[red]Error:[/red] Could not read {path}.") + console.print(f"Please fix file permissions or delete {INTEGRATION_JSON} and retry.") + console.print(f"[dim]Details:[/dim] {exc}") + raise typer.Exit(1) def _write_integration_json( @@ -1687,32 +1695,49 @@ def integration_install( def _parse_integration_options(integration: Any, raw_options: str) -> dict[str, Any] | None: """Parse --integration-options string into a dict matching the integration's declared options. - Returns ``None`` when no recognised options are found. + Returns ``None`` when no options are provided. """ import shlex parsed: dict[str, Any] = {} tokens = shlex.split(raw_options) - declared = {opt.name.lstrip("-"): opt for opt in integration.options()} + declared_options = list(integration.options()) + declared = {opt.name.lstrip("-"): opt for opt in declared_options} + allowed = ", ".join(sorted(opt.name for opt in declared_options)) i = 0 while i < len(tokens): token = tokens[i] + if not token.startswith("-"): + console.print(f"[red]Error:[/red] Unexpected integration option value '{token}'.") + if allowed: + console.print(f"Allowed options: {allowed}") + raise typer.Exit(1) name = token.lstrip("-") value: str | None = None # Handle --name=value syntax if "=" in name: name, value = name.split("=", 1) opt = declared.get(name) - if opt and opt.is_flag: - parsed[name.replace("-", "_")] = True + if not opt: + console.print(f"[red]Error:[/red] Unknown integration option '{token}'.") + if allowed: + console.print(f"Allowed options: {allowed}") + raise typer.Exit(1) + key = name.replace("-", "_") + if opt.is_flag: + if value is not None: + console.print(f"[red]Error:[/red] Option '{opt.name}' is a flag and does not accept a value.") + raise typer.Exit(1) + parsed[key] = True i += 1 - elif opt and value is not None: - parsed[name.replace("-", "_")] = value + elif value is not None: + parsed[key] = value i += 1 - elif opt and i + 1 < len(tokens): - parsed[name.replace("-", "_")] = tokens[i + 1] + elif i + 1 < len(tokens) and not tokens[i + 1].startswith("-"): + parsed[key] = tokens[i + 1] i += 2 else: - i += 1 + console.print(f"[red]Error:[/red] Option '{opt.name}' requires a value.") + raise typer.Exit(1) return parsed or None From f6733ba547b90757ff0786cac4069a553fd7a6bb Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 3 Apr 2026 17:15:37 -0500 Subject: [PATCH 17/17] Validate integration.json is a dict, fail fast on missing manifest in switch - _read_integration_json() validates parsed JSON is a dict, not a list/string - Switch fails fast with recovery guidance when manifest is missing instead of silently skipping teardown and risking co-existing integration files --- src/specify_cli/__init__.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 1b160fdf92..455beea38e 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1505,7 +1505,7 @@ def _read_integration_json(project_root: Path) -> dict[str, Any]: if not path.exists(): return {} try: - return json.loads(path.read_text(encoding="utf-8")) + data = json.loads(path.read_text(encoding="utf-8")) except json.JSONDecodeError as exc: console.print(f"[red]Error:[/red] {path} contains invalid JSON.") console.print(f"Please fix or delete {INTEGRATION_JSON} and retry.") @@ -1516,6 +1516,11 @@ def _read_integration_json(project_root: Path) -> dict[str, Any]: console.print(f"Please fix file permissions or delete {INTEGRATION_JSON} and retry.") console.print(f"[dim]Details:[/dim] {exc}") raise typer.Exit(1) + if not isinstance(data, dict): + console.print(f"[red]Error:[/red] {path} must contain a JSON object, got {type(data).__name__}.") + console.print(f"Please fix or delete {INTEGRATION_JSON} and retry.") + raise typer.Exit(1) + return data def _write_integration_json( @@ -1911,7 +1916,12 @@ def integration_switch( except (ValueError, FileNotFoundError) as exc: console.print(f"[yellow]Warning:[/yellow] Could not read manifest for '{installed_key}': {exc}") else: - console.print(f"[dim]No manifest for '{installed_key}' — skipping uninstall phase[/dim]") + console.print(f"[red]Error:[/red] Integration '{installed_key}' is installed but has no manifest.") + console.print( + f"Run [cyan]specify integration uninstall {installed_key}[/cyan] to clear metadata, " + f"then retry [cyan]specify integration switch {target}[/cyan]." + ) + raise typer.Exit(1) # Clear metadata so a failed Phase 2 doesn't leave stale references _remove_integration_json(project_root)