fix: address follow-up copilot feedback

Author: PascalThuet

src/specify_cli/integration_state.py

@@ -11,14 +11,21 @@
 INTEGRATION_STATE_SCHEMA = 1
 
 
+def clean_integration_key(key: Any) -> str | None:
+    """Return a stripped integration key, or None for empty/non-string values."""
+    if not isinstance(key, str) or not key.strip():
+        return None
+    return key.strip()
+
+
 def dedupe_integration_keys(keys: list[Any]) -> list[str]:
     """Return a de-duplicated list of non-empty integration keys."""
     seen: set[str] = set()
     deduped: list[str] = []
     for key in keys:
-        if not isinstance(key, str) or not key.strip():
+        clean = clean_integration_key(key)
+        if clean is None:
             continue
-        clean = key.strip()
         if clean in seen:
             continue
         seen.add(clean)
@@ -61,10 +68,8 @@ def normalize_integration_settings(settings: Any) -> dict[str, dict[str, Any]]:
 
 def normalize_integration_state(data: dict[str, Any]) -> dict[str, Any]:
     """Normalize legacy and multi-install integration metadata."""
-    legacy_key = data.get("integration")
-    default_key = data.get("default_integration")
-    if not isinstance(default_key, str) or not default_key.strip():
-        default_key = legacy_key if isinstance(legacy_key, str) else None
+    legacy_key = clean_integration_key(data.get("integration"))
+    default_key = clean_integration_key(data.get("default_integration")) or legacy_key
 
     installed = data.get("installed_integrations")
     installed_keys = dedupe_integration_keys(installed if isinstance(installed, list) else [])
@@ -93,7 +98,7 @@ def normalize_integration_state(data: dict[str, Any]) -> dict[str, Any]:
 def default_integration_key(state: dict[str, Any]) -> str | None:
     """Return the default integration key from normalized state."""
     key = state.get("default_integration") or state.get("integration")
-    return key if isinstance(key, str) and key else None
+    return clean_integration_key(key)
 
 
 def installed_integration_keys(state: dict[str, Any]) -> list[str]:
@@ -123,6 +128,7 @@ def write_integration_json(
     dest = project_root / INTEGRATION_JSON
     dest.parent.mkdir(parents=True, exist_ok=True)
 
+    integration_key = clean_integration_key(integration_key)
     installed = dedupe_integration_keys(installed_integrations or [])
     if integration_key and integration_key not in installed:
         installed.insert(0, integration_key)

tests/integrations/test_integration_state.py

@@ -0,0 +1,56 @@
+"""Tests for integration state normalization helpers."""
+
+import json
+
+from specify_cli.integration_state import INTEGRATION_JSON
+from specify_cli.integration_state import (
+    default_integration_key,
+    normalize_integration_state,
+    write_integration_json,
+)
+
+
+def test_normalize_integration_state_strips_default_key_without_duplicates():
+    state = normalize_integration_state(
+        {
+            "default_integration": " claude ",
+            "integration": " claude ",
+            "installed_integrations": ["claude"],
+        }
+    )
+
+    assert state["integration"] == "claude"
+    assert state["default_integration"] == "claude"
+    assert state["installed_integrations"] == ["claude"]
+
+
+def test_normalize_integration_state_strips_legacy_key_fallback():
+    state = normalize_integration_state(
+        {
+            "integration": " codex ",
+            "installed_integrations": [],
+        }
+    )
+
+    assert state["integration"] == "codex"
+    assert state["default_integration"] == "codex"
+    assert state["installed_integrations"] == ["codex"]
+
+
+def test_default_integration_key_strips_raw_state_values():
+    assert default_integration_key({"default_integration": " claude "}) == "claude"
+    assert default_integration_key({"integration": " codex "}) == "codex"
+
+
+def test_write_integration_json_strips_integration_key(tmp_path):
+    write_integration_json(
+        tmp_path,
+        version="1.2.3",
+        integration_key=" claude ",
+        installed_integrations=["claude"],
+    )
+
+    state = json.loads((tmp_path / INTEGRATION_JSON).read_text(encoding="utf-8"))
+    assert state["integration"] == "claude"
+    assert state["default_integration"] == "claude"
+    assert state["installed_integrations"] == ["claude"]

tests/integrations/test_integration_subcommand.py

@@ -81,8 +81,15 @@ def test_list_shows_multi_install_safe_status(self, tmp_path):
         assert result.exit_code == 0
         assert "Multi-install" in result.output
         assert "Safe" in result.output
-        assert "yes" in result.output
-        assert "no" in result.output
+        rows = {
+            cells[0]: cells
+            for line in result.output.splitlines()
+            if line.startswith("│")
+            for cells in ([cell.strip() for cell in line.split("│")[1:-1]],)
+            if cells and cells[0]
+        }
+        assert rows["claude"][4] == "yes"
+        assert rows["copilot"][4] == "no"
 
 
 # ── install ──────────────────────────────────────────────────────────

tests/integrations/test_registry.py

@@ -66,15 +66,17 @@ def _posix_path(value: str | None) -> str | None:
 
 def _integration_root_dir(key: str) -> str | None:
     integration = INTEGRATION_REGISTRY[key]
-    return _posix_path(integration.config.get("folder"))
+    cfg = integration.config if isinstance(integration.config, dict) else {}
+    return _posix_path(cfg.get("folder"))
 
 
 def _integration_commands_dir(key: str) -> str | None:
     integration = INTEGRATION_REGISTRY[key]
-    folder = integration.config.get("folder")
+    cfg = integration.config if isinstance(integration.config, dict) else {}
+    folder = cfg.get("folder")
     if not folder:
         return None
-    subdir = integration.config.get("commands_subdir", "commands")
+    subdir = cfg.get("commands_subdir", "commands")
     return (PurePosixPath(folder) / subdir).as_posix()