Address review feedback: validate script type, handle --flag=value, f…

…ix 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
github · mnriem · Apr 3, 2026 · Apr 3, 2026 · Apr 3, 2026 · Apr 3, 2026
commit f5e975269c99c7cdaddfd65800bc68bfa4baebea
@@ -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(

@@ -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"