From 0e55a1e61202f39c2e0c2f789c26e5bf11151653 Mon Sep 17 00:00:00 2001 From: Pascal Date: Fri, 22 May 2026 09:19:00 +0200 Subject: [PATCH 01/11] feat(integration): add doctor diagnostics --- docs/reference/integrations.md | 13 + src/specify_cli/__init__.py | 60 ++++ src/specify_cli/integration_doctor.py | 271 ++++++++++++++++++ .../test_integration_subcommand.py | 113 ++++++++ 4 files changed, 457 insertions(+) create mode 100644 src/specify_cli/integration_doctor.py diff --git a/docs/reference/integrations.md b/docs/reference/integrations.md index ec6c894652..fbbbe3af14 100644 --- a/docs/reference/integrations.md +++ b/docs/reference/integrations.md @@ -123,6 +123,19 @@ specify integration upgrade [] Reinstalls an installed integration with updated templates and commands (e.g., after upgrading Spec Kit). Defaults to the default integration; if a key is provided, it must be one of the installed integrations. Detects locally modified files and blocks the upgrade unless `--force` is used. Stale files from the previous install that are no longer needed are removed automatically. Shared templates stay aligned with the default integration even when upgrading a non-default integration. +## Diagnose Integration State + +```bash +specify integration doctor +specify integration doctor --json +``` + +Inspects the current project's integration state without changing files. The +diagnostic report includes the default integration, installed integrations, +multi-install safety, missing managed files, modified managed files, and any +manifest or state-file problems. The JSON form is intended for CI and coding +agents that need stable machine-readable diagnostics. + ## Integration-Specific Options Some integrations accept additional options via `--integration-options`: diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index c0bdbaabe3..008e32be37 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1615,6 +1615,66 @@ def integration_list( console.print("Install one with: [cyan]specify integration install [/cyan]") +def _print_integration_doctor_report(report: dict[str, Any]) -> None: + status = report["status"] + status_label = { + "ok": "[green]OK[/green]", + "warning": "[yellow]WARNING[/yellow]", + "error": "[red]ERROR[/red]", + }.get(status, status.upper()) + installed = report.get("installed_integrations") or [] + + console.print(f"Integration state: {status_label}") + console.print(f"Default integration: {report.get('default_integration') or 'none'}") + console.print(f"Installed integrations: {', '.join(installed) if installed else 'none'}") + console.print(f"Multi-install safe: {'yes' if report.get('multi_install_safe') else 'no'}") + console.print(f"Shared templates aligned to: {report.get('shared_templates_aligned_to') or 'none'}") + console.print(f"Modified managed files: {report.get('modified_managed_files', 0)}") + console.print(f"Missing managed files: {report.get('missing_managed_files', 0)}") + + findings = report.get("findings") or [] + if not findings: + return + + console.print() + console.print("[bold]Findings:[/bold]") + for item in findings: + severity = item.get("severity", "") + severity_label = { + "error": "[red]error[/red]", + "warning": "[yellow]warning[/yellow]", + }.get(severity, severity) + prefix = f"- {severity_label} {item.get('code', '')}" + if item.get("integration"): + prefix += f" ({item['integration']})" + console.print(f"{prefix}: {item.get('message', '')}", soft_wrap=True) + if item.get("suggestion"): + console.print(f" Suggestion: {item['suggestion']}", soft_wrap=True) + + +@integration_app.command("doctor") +def integration_doctor( + json_output: bool = typer.Option( + False, + "--json", + help="Emit machine-readable integration diagnostics.", + ), +): + """Diagnose the current project's integration state without changing files.""" + from .integration_doctor import diagnose_integration_project + + project_root = _require_specify_project() + report = diagnose_integration_project(project_root) + + if json_output: + console.print(json.dumps(report, indent=2)) + else: + _print_integration_doctor_report(report) + + if report["status"] == "error": + raise typer.Exit(1) + + @integration_app.command("install") def integration_install( key: str = typer.Argument(help="Integration key to install (e.g. claude, copilot)"), diff --git a/src/specify_cli/integration_doctor.py b/src/specify_cli/integration_doctor.py new file mode 100644 index 0000000000..0a6e5a3ef3 --- /dev/null +++ b/src/specify_cli/integration_doctor.py @@ -0,0 +1,271 @@ +"""Read-only diagnostics for project integration state.""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any + +from .integration_state import ( + INTEGRATION_JSON, + IntegrationReadError, + default_integration_key, + installed_integration_keys, + try_read_integration_json, +) +from .integrations import INTEGRATION_REGISTRY +from .integrations.manifest import IntegrationManifest + +_MANIFEST_READ_ERRORS = (ValueError, FileNotFoundError, OSError, UnicodeDecodeError) + + +def _finding( + severity: str, + code: str, + message: str, + *, + integration: str | None = None, + path: str | None = None, + suggestion: str | None = None, +) -> dict[str, str]: + item = { + "severity": severity, + "code": code, + "message": message, + } + if integration: + item["integration"] = integration + if path: + item["path"] = path + if suggestion: + item["suggestion"] = suggestion + return item + + +def _status(findings: list[dict[str, str]]) -> str: + if any(item["severity"] == "error" for item in findings): + return "error" + if findings: + return "warning" + return "ok" + + +def _integration_state_error_message(error: IntegrationReadError) -> str: + if error.kind == "decode": + return f"{INTEGRATION_JSON} contains invalid JSON or is not valid UTF-8." + if error.kind == "os": + return f"Could not read {INTEGRATION_JSON}." + if error.kind == "not_object": + return f"{INTEGRATION_JSON} must contain a JSON object, got {error.detail}." + if error.kind == "schema_too_new": + return ( + f"{INTEGRATION_JSON} uses integration state schema {error.schema}, " + "which is newer than this CLI supports." + ) + return f"Could not inspect {INTEGRATION_JSON}." + + +def _safe_manifest_file(project_root: Path, rel: str) -> Path | None: + rel_path = Path(rel) + if rel_path.is_absolute() or ".." in rel_path.parts: + return None + return project_root / rel_path + + +def _manifest_missing_files(manifest: IntegrationManifest) -> list[str]: + missing: list[str] = [] + for rel in manifest.files: + path = _safe_manifest_file(manifest.project_root, rel) + if path is None: + continue + if not path.exists() and not path.is_symlink(): + missing.append(rel) + return missing + + +def diagnose_integration_project(project_root: Path) -> dict[str, Any]: + """Return a machine-readable integration health report for *project_root*.""" + findings: list[dict[str, str]] = [] + state, error = try_read_integration_json(project_root) + if error is not None: + findings.append( + _finding( + "error", + "integration-state-unreadable", + _integration_state_error_message(error), + path=INTEGRATION_JSON, + suggestion=f"Fix or delete {INTEGRATION_JSON}, then retry.", + ) + ) + return _build_report(None, [], findings, {}, True) + + if state is None: + findings.append( + _finding( + "error", + "integration-state-missing", + f"{INTEGRATION_JSON} is missing.", + path=INTEGRATION_JSON, + suggestion="Run `specify integration install ` to install an integration.", + ) + ) + return _build_report(None, [], findings, {}, True) + + default_key = default_integration_key(state) + installed_keys = installed_integration_keys(state) + if not installed_keys: + findings.append( + _finding( + "warning", + "no-installed-integrations", + "No installed integrations are recorded.", + suggestion="Run `specify integration install ` to install one.", + ) + ) + return _build_report(default_key, installed_keys, findings, {}, True) + + if default_key is None: + findings.append( + _finding( + "error", + "default-integration-missing", + "No default integration is recorded.", + suggestion="Run `specify integration use ` after choosing an installed integration.", + ) + ) + + known_installed = [key for key in installed_keys if key in INTEGRATION_REGISTRY] + for key in installed_keys: + if key not in INTEGRATION_REGISTRY: + findings.append( + _finding( + "error", + "unknown-integration", + f"Integration '{key}' is installed but is not known to this CLI.", + integration=key, + suggestion="Upgrade Spec Kit, or uninstall the stale integration metadata.", + ) + ) + + unsafe = [ + key for key in known_installed + if not getattr(INTEGRATION_REGISTRY[key], "multi_install_safe", False) + ] + if len(installed_keys) > 1 and unsafe: + findings.append( + _finding( + "error", + "unsafe-multi-install", + ( + "Installed integrations are not all declared multi-install safe: " + + ", ".join(sorted(unsafe)) + ), + suggestion="Use `specify integration use ` to change defaults, or `switch` only when replacing integrations.", + ) + ) + + manifest_files_by_path: dict[str, list[str]] = {} + manifest_summaries: dict[str, dict[str, Any]] = {} + for key in installed_keys: + manifest_path = project_root / ".specify" / "integrations" / f"{key}.manifest.json" + if not manifest_path.exists(): + findings.append( + _finding( + "error", + "manifest-missing", + f"Manifest for integration '{key}' is missing.", + integration=key, + path=manifest_path.relative_to(project_root).as_posix(), + suggestion=f"Run `specify integration upgrade {key}` or reinstall the integration.", + ) + ) + manifest_summaries[key] = { + "manifest": manifest_path.relative_to(project_root).as_posix(), + "tracked_files": 0, + "missing_files": [], + "modified_files": [], + } + continue + + try: + manifest = IntegrationManifest.load(key, project_root) + except _MANIFEST_READ_ERRORS as exc: + findings.append( + _finding( + "error", + "manifest-unreadable", + f"Manifest for integration '{key}' is unreadable: {exc}", + integration=key, + path=manifest_path.relative_to(project_root).as_posix(), + suggestion=f"Fix the manifest or reinstall integration '{key}'.", + ) + ) + continue + + missing = _manifest_missing_files(manifest) + modified = manifest.check_modified() + manifest_summaries[key] = { + "manifest": manifest_path.relative_to(project_root).as_posix(), + "tracked_files": len(manifest.files), + "missing_files": missing, + "modified_files": modified, + } + + for rel in manifest.files: + manifest_files_by_path.setdefault(rel, []).append(key) + if missing: + findings.append( + _finding( + "error", + "managed-files-missing", + f"{len(missing)} managed file(s) are missing for integration '{key}'.", + integration=key, + suggestion=f"Run `specify integration upgrade {key}` to regenerate managed files.", + ) + ) + if modified: + findings.append( + _finding( + "warning", + "managed-files-modified", + f"{len(modified)} managed file(s) were modified for integration '{key}'.", + integration=key, + suggestion="Review the changes before running `specify integration upgrade --force`.", + ) + ) + + for rel, keys in sorted(manifest_files_by_path.items()): + if len(keys) > 1: + findings.append( + _finding( + "warning", + "managed-file-collision", + f"Managed file '{rel}' is tracked by multiple integrations: {', '.join(sorted(keys))}.", + path=rel, + suggestion="Review the manifests before uninstalling or upgrading these integrations.", + ) + ) + + multi_install_safe = not (len(installed_keys) > 1 and unsafe) + return _build_report(default_key, installed_keys, findings, manifest_summaries, multi_install_safe) + + +def _build_report( + default_key: str | None, + installed_keys: list[str], + findings: list[dict[str, str]], + manifests: dict[str, dict[str, Any]], + multi_install_safe: bool, +) -> dict[str, Any]: + missing_count = sum(len(item.get("missing_files", [])) for item in manifests.values()) + modified_count = sum(len(item.get("modified_files", [])) for item in manifests.values()) + return { + "status": _status(findings), + "default_integration": default_key, + "installed_integrations": installed_keys, + "multi_install_safe": multi_install_safe, + "shared_templates_aligned_to": default_key, + "missing_managed_files": missing_count, + "modified_managed_files": modified_count, + "manifests": manifests, + "findings": findings, + } diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index abff9a5ee1..4fd24ed41a 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -126,6 +126,119 @@ def test_list_rejects_newer_integration_state_schema(self, tmp_path): assert "only supports schema 1" in normalized +# ── doctor ─────────────────────────────────────────────────────────── + + +class TestIntegrationDoctor: + def test_doctor_requires_speckit_project(self, tmp_path): + old_cwd = os.getcwd() + try: + os.chdir(tmp_path) + result = runner.invoke(app, ["integration", "doctor"]) + finally: + os.chdir(old_cwd) + assert result.exit_code != 0 + assert "Not a spec-kit project" in result.output + + def test_doctor_reports_healthy_project(self, tmp_path): + project = _init_project(tmp_path, "copilot") + result = _run_in_project(project, ["integration", "doctor"]) + + assert result.exit_code == 0 + assert "Integration state: OK" in result.output + assert "Default integration: copilot" in result.output + assert "Installed integrations: copilot" in result.output + assert "Modified managed files: 0" in result.output + assert "Missing managed files: 0" in result.output + + def test_doctor_json_reports_healthy_project(self, tmp_path): + project = _init_project(tmp_path, "copilot") + result = _run_in_project(project, ["integration", "doctor", "--json"]) + + assert result.exit_code == 0 + payload = json.loads(result.output) + assert payload["status"] == "ok" + assert payload["default_integration"] == "copilot" + assert payload["installed_integrations"] == ["copilot"] + assert payload["findings"] == [] + + def test_doctor_reports_invalid_integration_json(self, tmp_path): + project = _init_project(tmp_path, "copilot") + (project / ".specify" / "integration.json").write_text("{", encoding="utf-8") + + result = _run_in_project(project, ["integration", "doctor"]) + + assert result.exit_code != 0 + assert "integration-state-unreadable" in result.output + assert "invalid JSON" in result.output + assert "Traceback" not in result.output + + def test_doctor_reports_missing_integration_json(self, tmp_path): + project = _init_project(tmp_path, "copilot") + (project / ".specify" / "integration.json").unlink() + + result = _run_in_project(project, ["integration", "doctor"]) + + assert result.exit_code != 0 + assert "integration-state-missing" in result.output + assert ".specify/integration.json is missing" in result.output + + def test_doctor_reports_missing_manifest(self, tmp_path): + project = _init_project(tmp_path, "copilot") + (project / ".specify" / "integrations" / "copilot.manifest.json").unlink() + + result = _run_in_project(project, ["integration", "doctor"]) + + assert result.exit_code != 0 + assert "manifest-missing" in result.output + assert "Manifest for integration 'copilot' is missing" in result.output + + def test_doctor_reports_modified_managed_files_without_failing(self, tmp_path): + project = _init_project(tmp_path, "copilot") + manifest_path = project / ".specify" / "integrations" / "copilot.manifest.json" + tracked_files = json.loads(manifest_path.read_text(encoding="utf-8"))["files"] + first_rel = next(iter(tracked_files)) + (project / first_rel).write_text("MODIFIED CONTENT\n", encoding="utf-8") + + result = _run_in_project(project, ["integration", "doctor"]) + + assert result.exit_code == 0 + assert "Integration state: WARNING" in result.output + assert "managed-files-modified" in result.output + assert "Modified managed files: 1" in result.output + + def test_doctor_reports_missing_managed_files(self, tmp_path): + project = _init_project(tmp_path, "copilot") + manifest_path = project / ".specify" / "integrations" / "copilot.manifest.json" + tracked_files = json.loads(manifest_path.read_text(encoding="utf-8"))["files"] + first_rel = next(iter(tracked_files)) + (project / first_rel).unlink() + + result = _run_in_project(project, ["integration", "doctor"]) + + assert result.exit_code != 0 + assert "managed-files-missing" in result.output + assert "Missing managed files: 1" in result.output + + def test_doctor_reports_unsafe_multi_install_combination(self, tmp_path): + from specify_cli.integrations.manifest import IntegrationManifest + + project = _init_project(tmp_path, "copilot") + state_path = project / ".specify" / "integration.json" + state = json.loads(state_path.read_text(encoding="utf-8")) + state["installed_integrations"] = ["copilot", "claude"] + state["default_integration"] = "copilot" + state["integration"] = "copilot" + state_path.write_text(json.dumps(state), encoding="utf-8") + IntegrationManifest("claude", project, version="test").save() + + result = _run_in_project(project, ["integration", "doctor"]) + + assert result.exit_code != 0 + assert "unsafe-multi-install" in result.output + assert "Multi-install safe: no" in result.output + + # ── install ────────────────────────────────────────────────────────── From 9a708269fbc514415aca9f7eae7bdd940164df00 Mon Sep 17 00:00:00 2001 From: Pascal Date: Fri, 22 May 2026 09:56:15 +0200 Subject: [PATCH 02/11] fix(integration): address doctor review feedback --- docs/reference/integrations.md | 7 +- src/specify_cli/__init__.py | 3 +- src/specify_cli/integration_doctor.py | 96 ++++++++++---- .../test_integration_subcommand.py | 118 ++++++++++++++++-- 4 files changed, 191 insertions(+), 33 deletions(-) diff --git a/docs/reference/integrations.md b/docs/reference/integrations.md index fbbbe3af14..bec326c0ed 100644 --- a/docs/reference/integrations.md +++ b/docs/reference/integrations.md @@ -132,9 +132,10 @@ specify integration doctor --json Inspects the current project's integration state without changing files. The diagnostic report includes the default integration, installed integrations, -multi-install safety, missing managed files, modified managed files, and any -manifest or state-file problems. The JSON form is intended for CI and coding -agents that need stable machine-readable diagnostics. +multi-install safety, missing managed files, modified managed files, shared +Spec Kit infrastructure health, unchecked manifests, and any manifest or +state-file problems. The JSON form is intended for CI and coding agents that +need stable machine-readable diagnostics. ## Integration-Specific Options diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 008e32be37..0ca7630f23 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1631,6 +1631,7 @@ def _print_integration_doctor_report(report: dict[str, Any]) -> None: console.print(f"Shared templates aligned to: {report.get('shared_templates_aligned_to') or 'none'}") console.print(f"Modified managed files: {report.get('modified_managed_files', 0)}") console.print(f"Missing managed files: {report.get('missing_managed_files', 0)}") + console.print(f"Unchecked manifests: {report.get('unchecked_manifests', 0)}") findings = report.get("findings") or [] if not findings: @@ -1667,7 +1668,7 @@ def integration_doctor( report = diagnose_integration_project(project_root) if json_output: - console.print(json.dumps(report, indent=2)) + typer.echo(json.dumps(report, indent=2)) else: _print_integration_doctor_report(report) diff --git a/src/specify_cli/integration_doctor.py b/src/specify_cli/integration_doctor.py index 0a6e5a3ef3..41501c1091 100644 --- a/src/specify_cli/integration_doctor.py +++ b/src/specify_cli/integration_doctor.py @@ -16,6 +16,7 @@ from .integrations.manifest import IntegrationManifest _MANIFEST_READ_ERRORS = (ValueError, FileNotFoundError, OSError, UnicodeDecodeError) +_SHARED_MANIFEST_KEY = "speckit" def _finding( @@ -77,11 +78,43 @@ def _manifest_missing_files(manifest: IntegrationManifest) -> list[str]: path = _safe_manifest_file(manifest.project_root, rel) if path is None: continue - if not path.exists() and not path.is_symlink(): + if not path.exists(): missing.append(rel) return missing +def _manifest_summary( + manifest_path: Path, + project_root: Path, + *, + readable: bool, + tracked_files: int = 0, + missing_files: list[str] | None = None, + modified_files: list[str] | None = None, +) -> dict[str, Any]: + return { + "manifest": manifest_path.relative_to(project_root).as_posix(), + "readable": readable, + "tracked_files": tracked_files, + "missing_files": missing_files or [], + "modified_files": modified_files or [], + } + + +def _manifest_owner(key: str) -> str: + if key == _SHARED_MANIFEST_KEY: + return "shared Spec Kit infrastructure" + return f"integration '{key}'" + + +def _manifest_suggestion(key: str, default_key: str | None) -> str: + if key == _SHARED_MANIFEST_KEY: + if default_key and default_key in INTEGRATION_REGISTRY: + return f"Run `specify integration upgrade {default_key}` to regenerate shared managed files." + return "Run `specify init --here --force` to regenerate shared managed files." + return f"Run `specify integration upgrade {key}` or reinstall the integration." + + def diagnose_integration_project(project_root: Path) -> dict[str, Any]: """Return a machine-readable integration health report for *project_root*.""" findings: list[dict[str, str]] = [] @@ -134,8 +167,10 @@ def diagnose_integration_project(project_root: Path) -> dict[str, Any]: ) known_installed = [key for key in installed_keys if key in INTEGRATION_REGISTRY] + unknown_installed: list[str] = [] for key in installed_keys: if key not in INTEGRATION_REGISTRY: + unknown_installed.append(key) findings.append( _finding( "error", @@ -150,6 +185,9 @@ def diagnose_integration_project(project_root: Path) -> dict[str, Any]: key for key in known_installed if not getattr(INTEGRATION_REGISTRY[key], "multi_install_safe", False) ] + if len(installed_keys) > 1: + unsafe.extend(unknown_installed) + if len(installed_keys) > 1 and unsafe: findings.append( _finding( @@ -165,50 +203,62 @@ def diagnose_integration_project(project_root: Path) -> dict[str, Any]: manifest_files_by_path: dict[str, list[str]] = {} manifest_summaries: dict[str, dict[str, Any]] = {} - for key in installed_keys: + manifest_keys = list(installed_keys) + if _SHARED_MANIFEST_KEY not in manifest_keys: + manifest_keys.append(_SHARED_MANIFEST_KEY) + + for key in manifest_keys: + owner = _manifest_owner(key) manifest_path = project_root / ".specify" / "integrations" / f"{key}.manifest.json" if not manifest_path.exists(): findings.append( _finding( "error", "manifest-missing", - f"Manifest for integration '{key}' is missing.", + f"Manifest for {owner} is missing.", integration=key, path=manifest_path.relative_to(project_root).as_posix(), - suggestion=f"Run `specify integration upgrade {key}` or reinstall the integration.", + suggestion=_manifest_suggestion(key, default_key), ) ) - manifest_summaries[key] = { - "manifest": manifest_path.relative_to(project_root).as_posix(), - "tracked_files": 0, - "missing_files": [], - "modified_files": [], - } + manifest_summaries[key] = _manifest_summary( + manifest_path, + project_root, + readable=False, + ) continue try: manifest = IntegrationManifest.load(key, project_root) except _MANIFEST_READ_ERRORS as exc: + manifest_summaries[key] = _manifest_summary( + manifest_path, + project_root, + readable=False, + ) findings.append( _finding( "error", "manifest-unreadable", - f"Manifest for integration '{key}' is unreadable: {exc}", + f"Manifest for {owner} is unreadable: {exc}", integration=key, path=manifest_path.relative_to(project_root).as_posix(), - suggestion=f"Fix the manifest or reinstall integration '{key}'.", + suggestion=_manifest_suggestion(key, default_key), ) ) continue missing = _manifest_missing_files(manifest) - modified = manifest.check_modified() - manifest_summaries[key] = { - "manifest": manifest_path.relative_to(project_root).as_posix(), - "tracked_files": len(manifest.files), - "missing_files": missing, - "modified_files": modified, - } + missing_set = set(missing) + modified = [rel for rel in manifest.check_modified() if rel not in missing_set] + manifest_summaries[key] = _manifest_summary( + manifest_path, + project_root, + readable=True, + tracked_files=len(manifest.files), + missing_files=missing, + modified_files=modified, + ) for rel in manifest.files: manifest_files_by_path.setdefault(rel, []).append(key) @@ -217,9 +267,9 @@ def diagnose_integration_project(project_root: Path) -> dict[str, Any]: _finding( "error", "managed-files-missing", - f"{len(missing)} managed file(s) are missing for integration '{key}'.", + f"{len(missing)} managed file(s) are missing for {owner}.", integration=key, - suggestion=f"Run `specify integration upgrade {key}` to regenerate managed files.", + suggestion=_manifest_suggestion(key, default_key), ) ) if modified: @@ -227,7 +277,7 @@ def diagnose_integration_project(project_root: Path) -> dict[str, Any]: _finding( "warning", "managed-files-modified", - f"{len(modified)} managed file(s) were modified for integration '{key}'.", + f"{len(modified)} managed file(s) were modified for {owner}.", integration=key, suggestion="Review the changes before running `specify integration upgrade --force`.", ) @@ -258,6 +308,7 @@ def _build_report( ) -> dict[str, Any]: missing_count = sum(len(item.get("missing_files", [])) for item in manifests.values()) modified_count = sum(len(item.get("modified_files", [])) for item in manifests.values()) + unchecked_count = sum(1 for item in manifests.values() if not item.get("readable", True)) return { "status": _status(findings), "default_integration": default_key, @@ -266,6 +317,7 @@ def _build_report( "shared_templates_aligned_to": default_key, "missing_managed_files": missing_count, "modified_managed_files": modified_count, + "unchecked_manifests": unchecked_count, "manifests": manifests, "findings": findings, } diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index 4fd24ed41a..7d936a5037 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -130,13 +130,9 @@ def test_list_rejects_newer_integration_state_schema(self, tmp_path): class TestIntegrationDoctor: - def test_doctor_requires_speckit_project(self, tmp_path): - old_cwd = os.getcwd() - try: - os.chdir(tmp_path) - result = runner.invoke(app, ["integration", "doctor"]) - finally: - os.chdir(old_cwd) + def test_doctor_requires_speckit_project(self, tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + result = runner.invoke(app, ["integration", "doctor"]) assert result.exit_code != 0 assert "Not a spec-kit project" in result.output @@ -193,6 +189,19 @@ def test_doctor_reports_missing_manifest(self, tmp_path): assert "manifest-missing" in result.output assert "Manifest for integration 'copilot' is missing" in result.output + def test_doctor_reports_unreadable_manifest_in_json_summary(self, tmp_path): + project = _init_project(tmp_path, "copilot") + _write_invalid_manifest(project, "copilot") + + result = _run_in_project(project, ["integration", "doctor", "--json"]) + + assert result.exit_code != 0 + payload = json.loads(result.output) + assert payload["unchecked_manifests"] == 1 + assert payload["manifests"]["copilot"]["readable"] is False + assert payload["manifests"]["copilot"]["missing_files"] == [] + assert payload["manifests"]["copilot"]["modified_files"] == [] + def test_doctor_reports_modified_managed_files_without_failing(self, tmp_path): project = _init_project(tmp_path, "copilot") manifest_path = project / ".specify" / "integrations" / "copilot.manifest.json" @@ -220,6 +229,38 @@ def test_doctor_reports_missing_managed_files(self, tmp_path): assert "managed-files-missing" in result.output assert "Missing managed files: 1" in result.output + def test_doctor_reports_missing_shared_managed_files(self, tmp_path): + project = _init_project(tmp_path, "copilot") + shared_file = project / ".specify" / "scripts" / "bash" / "common.sh" + assert shared_file.exists() + shared_file.unlink() + + result = _run_in_project(project, ["integration", "doctor"]) + + assert result.exit_code != 0 + assert "managed-files-missing" in result.output + assert "shared Spec Kit infrastructure" in result.output + assert "Missing managed files: 1" in result.output + + def test_doctor_treats_dangling_symlink_as_missing(self, tmp_path): + project = _init_project(tmp_path, "copilot") + manifest_path = project / ".specify" / "integrations" / "copilot.manifest.json" + tracked_files = json.loads(manifest_path.read_text(encoding="utf-8"))["files"] + first_rel = next(iter(tracked_files)) + target = project / first_rel + target.unlink() + try: + target.symlink_to(project / "missing-target") + except OSError as exc: + pytest.skip(f"symlinks unavailable: {exc}") + + result = _run_in_project(project, ["integration", "doctor", "--json"]) + + assert result.exit_code != 0 + payload = json.loads(result.output) + assert first_rel in payload["manifests"]["copilot"]["missing_files"] + assert first_rel not in payload["manifests"]["copilot"]["modified_files"] + def test_doctor_reports_unsafe_multi_install_combination(self, tmp_path): from specify_cli.integrations.manifest import IntegrationManifest @@ -238,6 +279,69 @@ def test_doctor_reports_unsafe_multi_install_combination(self, tmp_path): assert "unsafe-multi-install" in result.output assert "Multi-install safe: no" in result.output + def test_doctor_treats_unknown_multi_install_as_unsafe(self, tmp_path): + from specify_cli.integrations.manifest import IntegrationManifest + + project = _init_project(tmp_path, "claude") + state_path = project / ".specify" / "integration.json" + state = json.loads(state_path.read_text(encoding="utf-8")) + state["installed_integrations"] = ["claude", "mystery"] + state["default_integration"] = "claude" + state["integration"] = "claude" + state_path.write_text(json.dumps(state), encoding="utf-8") + IntegrationManifest("mystery", project, version="test").save() + + result = _run_in_project(project, ["integration", "doctor"]) + + assert result.exit_code != 0 + assert "unknown-integration" in result.output + assert "unsafe-multi-install" in result.output + assert "Multi-install safe: no" in result.output + + def test_doctor_reports_managed_file_collisions(self, tmp_path): + from specify_cli.integrations.manifest import IntegrationManifest + + project = _init_project(tmp_path, "claude") + state_path = project / ".specify" / "integration.json" + state = json.loads(state_path.read_text(encoding="utf-8")) + state["installed_integrations"] = ["claude", "codex"] + state["default_integration"] = "claude" + state["integration"] = "claude" + state_path.write_text(json.dumps(state), encoding="utf-8") + + claude_manifest = project / ".specify" / "integrations" / "claude.manifest.json" + tracked_files = json.loads(claude_manifest.read_text(encoding="utf-8"))["files"] + shared_rel = next(iter(tracked_files)) + codex_manifest = IntegrationManifest("codex", project, version="test") + codex_manifest.record_existing(shared_rel) + codex_manifest.save() + + result = _run_in_project(project, ["integration", "doctor"]) + + assert result.exit_code == 0 + assert "managed-file-collision" in result.output + assert "Integration state: WARNING" in result.output + + def test_doctor_json_is_not_rich_rendered(self, tmp_path, monkeypatch): + project = tmp_path / "proj" + project.mkdir() + (project / ".specify").mkdir() + (project / ".specify" / "integration.json").write_text( + json.dumps({ + "integration": "[red]x[/red]", + "installed_integrations": ["[red]x[/red]"], + }), + encoding="utf-8", + ) + monkeypatch.chdir(project) + + result = runner.invoke(app, ["integration", "doctor", "--json"]) + + assert result.exit_code != 0 + payload = json.loads(result.output) + assert payload["default_integration"] == "[red]x[/red]" + assert payload["installed_integrations"] == ["[red]x[/red]"] + # ── install ────────────────────────────────────────────────────────── From 5d5587aecbe42c55d1dc8daf9753972efbc62fcf Mon Sep 17 00:00:00 2001 From: Pascal Date: Fri, 22 May 2026 10:26:19 +0200 Subject: [PATCH 03/11] fix(integration): harden doctor diagnostics --- docs/reference/integrations.md | 8 +- src/specify_cli/__init__.py | 6 +- src/specify_cli/integration_doctor.py | 85 ++++++++++++++++--- .../test_integration_subcommand.py | 42 +++++++++ 4 files changed, 126 insertions(+), 15 deletions(-) diff --git a/docs/reference/integrations.md b/docs/reference/integrations.md index bec326c0ed..f0f377a6f3 100644 --- a/docs/reference/integrations.md +++ b/docs/reference/integrations.md @@ -132,10 +132,10 @@ specify integration doctor --json Inspects the current project's integration state without changing files. The diagnostic report includes the default integration, installed integrations, -multi-install safety, missing managed files, modified managed files, shared -Spec Kit infrastructure health, unchecked manifests, and any manifest or -state-file problems. The JSON form is intended for CI and coding agents that -need stable machine-readable diagnostics. +multi-install safety, missing managed files, modified managed files, invalid +manifest paths, shared Spec Kit infrastructure health, unchecked manifests, and +the target integration for default-sensitive shared templates. The JSON form is +intended for CI and coding agents that need stable machine-readable diagnostics. ## Integration-Specific Options diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 0ca7630f23..70d72c6ec7 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1628,9 +1628,13 @@ def _print_integration_doctor_report(report: dict[str, Any]) -> None: console.print(f"Default integration: {report.get('default_integration') or 'none'}") console.print(f"Installed integrations: {', '.join(installed) if installed else 'none'}") console.print(f"Multi-install safe: {'yes' if report.get('multi_install_safe') else 'no'}") - console.print(f"Shared templates aligned to: {report.get('shared_templates_aligned_to') or 'none'}") + console.print( + f"Shared templates target alignment: " + f"{report.get('shared_templates_target_alignment') or 'none'}" + ) console.print(f"Modified managed files: {report.get('modified_managed_files', 0)}") console.print(f"Missing managed files: {report.get('missing_managed_files', 0)}") + console.print(f"Invalid manifest paths: {report.get('invalid_manifest_paths', 0)}") console.print(f"Unchecked manifests: {report.get('unchecked_manifests', 0)}") findings = report.get("findings") or [] diff --git a/src/specify_cli/integration_doctor.py b/src/specify_cli/integration_doctor.py index 41501c1091..f2aeb6ed76 100644 --- a/src/specify_cli/integration_doctor.py +++ b/src/specify_cli/integration_doctor.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json from pathlib import Path from typing import Any @@ -13,7 +14,7 @@ try_read_integration_json, ) from .integrations import INTEGRATION_REGISTRY -from .integrations.manifest import IntegrationManifest +from .integrations.manifest import IntegrationManifest, _sha256 _MANIFEST_READ_ERRORS = (ValueError, FileNotFoundError, OSError, UnicodeDecodeError) _SHARED_MANIFEST_KEY = "speckit" @@ -69,18 +70,53 @@ def _safe_manifest_file(project_root: Path, rel: str) -> Path | None: rel_path = Path(rel) if rel_path.is_absolute() or ".." in rel_path.parts: return None - return project_root / rel_path + candidate = project_root / rel_path + try: + candidate.parent.resolve(strict=False).relative_to(project_root.resolve()) + except (OSError, ValueError): + return None + return candidate -def _manifest_missing_files(manifest: IntegrationManifest) -> list[str]: +def _manifest_file_diagnostics( + manifest: IntegrationManifest, +) -> tuple[list[str], list[str], list[str], list[str]]: missing: list[str] = [] - for rel in manifest.files: + modified: list[str] = [] + invalid: list[str] = [] + valid: list[str] = [] + + for rel, expected_hash in manifest.files.items(): path = _safe_manifest_file(manifest.project_root, rel) if path is None: + invalid.append(rel) continue + valid.append(rel) if not path.exists(): missing.append(rel) - return missing + continue + if path.is_symlink() or not path.is_file(): + modified.append(rel) + continue + try: + if _sha256(path) != expected_hash: + modified.append(rel) + except OSError: + modified.append(rel) + + return missing, modified, invalid, valid + + +def _default_not_installed_from_raw_state(project_root: Path) -> str | None: + raw_state = json.loads((project_root / INTEGRATION_JSON).read_text(encoding="utf-8")) + if not isinstance(raw_state.get("installed_integrations"), list): + return None + + raw_default = default_integration_key(raw_state) + raw_installed = installed_integration_keys(raw_state) + if raw_default and raw_default not in raw_installed: + return raw_default + return None def _manifest_summary( @@ -91,6 +127,7 @@ def _manifest_summary( tracked_files: int = 0, missing_files: list[str] | None = None, modified_files: list[str] | None = None, + invalid_files: list[str] | None = None, ) -> dict[str, Any]: return { "manifest": manifest_path.relative_to(project_root).as_posix(), @@ -98,6 +135,7 @@ def _manifest_summary( "tracked_files": tracked_files, "missing_files": missing_files or [], "modified_files": modified_files or [], + "invalid_files": invalid_files or [], } @@ -166,6 +204,21 @@ def diagnose_integration_project(project_root: Path) -> dict[str, Any]: ) ) + raw_default_not_installed = _default_not_installed_from_raw_state(project_root) + if raw_default_not_installed: + findings.append( + _finding( + "error", + "default-integration-not-installed", + ( + f"Default integration '{raw_default_not_installed}' is not listed " + "in installed_integrations." + ), + integration=raw_default_not_installed, + suggestion="Run `specify integration use ` for an installed integration, or reinstall the default integration.", + ) + ) + known_installed = [key for key in installed_keys if key in INTEGRATION_REGISTRY] unknown_installed: list[str] = [] for key in installed_keys: @@ -248,9 +301,7 @@ def diagnose_integration_project(project_root: Path) -> dict[str, Any]: ) continue - missing = _manifest_missing_files(manifest) - missing_set = set(missing) - modified = [rel for rel in manifest.check_modified() if rel not in missing_set] + missing, modified, invalid, valid_files = _manifest_file_diagnostics(manifest) manifest_summaries[key] = _manifest_summary( manifest_path, project_root, @@ -258,10 +309,22 @@ def diagnose_integration_project(project_root: Path) -> dict[str, Any]: tracked_files=len(manifest.files), missing_files=missing, modified_files=modified, + invalid_files=invalid, ) - for rel in manifest.files: + for rel in valid_files: manifest_files_by_path.setdefault(rel, []).append(key) + if invalid: + findings.append( + _finding( + "error", + "manifest-paths-invalid", + f"{len(invalid)} unsafe manifest path(s) are recorded for {owner}.", + integration=key, + path=manifest_path.relative_to(project_root).as_posix(), + suggestion=_manifest_suggestion(key, default_key), + ) + ) if missing: findings.append( _finding( @@ -308,15 +371,17 @@ def _build_report( ) -> dict[str, Any]: missing_count = sum(len(item.get("missing_files", [])) for item in manifests.values()) modified_count = sum(len(item.get("modified_files", [])) for item in manifests.values()) + invalid_count = sum(len(item.get("invalid_files", [])) for item in manifests.values()) unchecked_count = sum(1 for item in manifests.values() if not item.get("readable", True)) return { "status": _status(findings), "default_integration": default_key, "installed_integrations": installed_keys, "multi_install_safe": multi_install_safe, - "shared_templates_aligned_to": default_key, + "shared_templates_target_alignment": default_key, "missing_managed_files": missing_count, "modified_managed_files": modified_count, + "invalid_manifest_paths": invalid_count, "unchecked_manifests": unchecked_count, "manifests": manifests, "findings": findings, diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index 7d936a5037..5a0a9a0ea5 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -144,6 +144,7 @@ def test_doctor_reports_healthy_project(self, tmp_path): assert "Integration state: OK" in result.output assert "Default integration: copilot" in result.output assert "Installed integrations: copilot" in result.output + assert "Shared templates target alignment: copilot" in result.output assert "Modified managed files: 0" in result.output assert "Missing managed files: 0" in result.output @@ -156,6 +157,8 @@ def test_doctor_json_reports_healthy_project(self, tmp_path): assert payload["status"] == "ok" assert payload["default_integration"] == "copilot" assert payload["installed_integrations"] == ["copilot"] + assert payload["shared_templates_target_alignment"] == "copilot" + assert "shared_templates_aligned_to" not in payload assert payload["findings"] == [] def test_doctor_reports_invalid_integration_json(self, tmp_path): @@ -179,6 +182,21 @@ def test_doctor_reports_missing_integration_json(self, tmp_path): assert "integration-state-missing" in result.output assert ".specify/integration.json is missing" in result.output + def test_doctor_reports_default_integration_not_installed(self, tmp_path): + project = _init_project(tmp_path, "claude") + state_path = project / ".specify" / "integration.json" + state = json.loads(state_path.read_text(encoding="utf-8")) + state["default_integration"] = "codex" + state["integration"] = "codex" + state["installed_integrations"] = ["claude"] + state_path.write_text(json.dumps(state), encoding="utf-8") + + result = _run_in_project(project, ["integration", "doctor"]) + + assert result.exit_code != 0 + assert "default-integration-not-installed" in result.output + assert "Default integration 'codex' is not listed" in result.output + def test_doctor_reports_missing_manifest(self, tmp_path): project = _init_project(tmp_path, "copilot") (project / ".specify" / "integrations" / "copilot.manifest.json").unlink() @@ -261,6 +279,30 @@ def test_doctor_treats_dangling_symlink_as_missing(self, tmp_path): assert first_rel in payload["manifests"]["copilot"]["missing_files"] assert first_rel not in payload["manifests"]["copilot"]["modified_files"] + def test_doctor_reports_unsafe_manifest_paths_without_hashing_them(self, tmp_path): + project = _init_project(tmp_path, "copilot") + outside = tmp_path / "outside" + outside.mkdir() + (outside / "secret.txt").write_text("outside project\n", encoding="utf-8") + link = project / "outside-link" + try: + link.symlink_to(outside, target_is_directory=True) + except OSError as exc: + pytest.skip(f"symlinks unavailable: {exc}") + + manifest_path = project / ".specify" / "integrations" / "copilot.manifest.json" + manifest_data = json.loads(manifest_path.read_text(encoding="utf-8")) + manifest_data["files"]["outside-link/secret.txt"] = "wrong" + manifest_path.write_text(json.dumps(manifest_data), encoding="utf-8") + + result = _run_in_project(project, ["integration", "doctor", "--json"]) + + assert result.exit_code != 0 + payload = json.loads(result.output) + assert payload["invalid_manifest_paths"] == 1 + assert "outside-link/secret.txt" in payload["manifests"]["copilot"]["invalid_files"] + assert "outside-link/secret.txt" not in payload["manifests"]["copilot"]["modified_files"] + def test_doctor_reports_unsafe_multi_install_combination(self, tmp_path): from specify_cli.integrations.manifest import IntegrationManifest From d420ce6db50e58275cdd57256d5278bd005a8709 Mon Sep 17 00:00:00 2001 From: Pascal Date: Tue, 26 May 2026 17:47:36 +0200 Subject: [PATCH 04/11] fix(integration): rename doctor diagnostics to status --- docs/reference/integrations.md | 12 +-- src/specify_cli/__init__.py | 18 ++--- ...ration_doctor.py => integration_status.py} | 10 +-- .../test_integration_subcommand.py | 78 +++++++++---------- 4 files changed, 59 insertions(+), 59 deletions(-) rename src/specify_cli/{integration_doctor.py => integration_status.py} (98%) diff --git a/docs/reference/integrations.md b/docs/reference/integrations.md index f0f377a6f3..4f555117b0 100644 --- a/docs/reference/integrations.md +++ b/docs/reference/integrations.md @@ -123,19 +123,19 @@ specify integration upgrade [] Reinstalls an installed integration with updated templates and commands (e.g., after upgrading Spec Kit). Defaults to the default integration; if a key is provided, it must be one of the installed integrations. Detects locally modified files and blocks the upgrade unless `--force` is used. Stale files from the previous install that are no longer needed are removed automatically. Shared templates stay aligned with the default integration even when upgrading a non-default integration. -## Diagnose Integration State +## Report Integration Status ```bash -specify integration doctor -specify integration doctor --json +specify integration status +specify integration status --json ``` -Inspects the current project's integration state without changing files. The -diagnostic report includes the default integration, installed integrations, +Reports the current project's integration status without changing files. The +status report includes the default integration, installed integrations, multi-install safety, missing managed files, modified managed files, invalid manifest paths, shared Spec Kit infrastructure health, unchecked manifests, and the target integration for default-sensitive shared templates. The JSON form is -intended for CI and coding agents that need stable machine-readable diagnostics. +intended for CI and coding agents that need stable machine-readable status data. ## Integration-Specific Options diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 70d72c6ec7..9f5f2f4a9b 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1615,7 +1615,7 @@ def integration_list( console.print("Install one with: [cyan]specify integration install [/cyan]") -def _print_integration_doctor_report(report: dict[str, Any]) -> None: +def _print_integration_status_report(report: dict[str, Any]) -> None: status = report["status"] status_label = { "ok": "[green]OK[/green]", @@ -1624,7 +1624,7 @@ def _print_integration_doctor_report(report: dict[str, Any]) -> None: }.get(status, status.upper()) installed = report.get("installed_integrations") or [] - console.print(f"Integration state: {status_label}") + console.print(f"Integration status: {status_label}") console.print(f"Default integration: {report.get('default_integration') or 'none'}") console.print(f"Installed integrations: {', '.join(installed) if installed else 'none'}") console.print(f"Multi-install safe: {'yes' if report.get('multi_install_safe') else 'no'}") @@ -1657,24 +1657,24 @@ def _print_integration_doctor_report(report: dict[str, Any]) -> None: console.print(f" Suggestion: {item['suggestion']}", soft_wrap=True) -@integration_app.command("doctor") -def integration_doctor( +@integration_app.command("status") +def integration_status( json_output: bool = typer.Option( False, "--json", - help="Emit machine-readable integration diagnostics.", + help="Emit machine-readable integration status.", ), ): - """Diagnose the current project's integration state without changing files.""" - from .integration_doctor import diagnose_integration_project + """Report the current project's integration status without changing files.""" + from .integration_status import build_integration_status_report project_root = _require_specify_project() - report = diagnose_integration_project(project_root) + report = build_integration_status_report(project_root) if json_output: typer.echo(json.dumps(report, indent=2)) else: - _print_integration_doctor_report(report) + _print_integration_status_report(report) if report["status"] == "error": raise typer.Exit(1) diff --git a/src/specify_cli/integration_doctor.py b/src/specify_cli/integration_status.py similarity index 98% rename from src/specify_cli/integration_doctor.py rename to src/specify_cli/integration_status.py index f2aeb6ed76..6b71c6f3e5 100644 --- a/src/specify_cli/integration_doctor.py +++ b/src/specify_cli/integration_status.py @@ -1,4 +1,4 @@ -"""Read-only diagnostics for project integration state.""" +"""Read-only status reporting for project integration state.""" from __future__ import annotations @@ -78,7 +78,7 @@ def _safe_manifest_file(project_root: Path, rel: str) -> Path | None: return candidate -def _manifest_file_diagnostics( +def _manifest_file_status( manifest: IntegrationManifest, ) -> tuple[list[str], list[str], list[str], list[str]]: missing: list[str] = [] @@ -153,8 +153,8 @@ def _manifest_suggestion(key: str, default_key: str | None) -> str: return f"Run `specify integration upgrade {key}` or reinstall the integration." -def diagnose_integration_project(project_root: Path) -> dict[str, Any]: - """Return a machine-readable integration health report for *project_root*.""" +def build_integration_status_report(project_root: Path) -> dict[str, Any]: + """Return a machine-readable integration status report for *project_root*.""" findings: list[dict[str, str]] = [] state, error = try_read_integration_json(project_root) if error is not None: @@ -301,7 +301,7 @@ def diagnose_integration_project(project_root: Path) -> dict[str, Any]: ) continue - missing, modified, invalid, valid_files = _manifest_file_diagnostics(manifest) + missing, modified, invalid, valid_files = _manifest_file_status(manifest) manifest_summaries[key] = _manifest_summary( manifest_path, project_root, diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index 5a0a9a0ea5..42b53aa682 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -126,31 +126,31 @@ def test_list_rejects_newer_integration_state_schema(self, tmp_path): assert "only supports schema 1" in normalized -# ── doctor ─────────────────────────────────────────────────────────── +# ── status ─────────────────────────────────────────────────────────── -class TestIntegrationDoctor: - def test_doctor_requires_speckit_project(self, tmp_path, monkeypatch): +class TestIntegrationStatus: + def test_status_requires_speckit_project(self, tmp_path, monkeypatch): monkeypatch.chdir(tmp_path) - result = runner.invoke(app, ["integration", "doctor"]) + result = runner.invoke(app, ["integration", "status"]) assert result.exit_code != 0 assert "Not a spec-kit project" in result.output - def test_doctor_reports_healthy_project(self, tmp_path): + def test_status_reports_healthy_project(self, tmp_path): project = _init_project(tmp_path, "copilot") - result = _run_in_project(project, ["integration", "doctor"]) + result = _run_in_project(project, ["integration", "status"]) assert result.exit_code == 0 - assert "Integration state: OK" in result.output + assert "Integration status: OK" in result.output assert "Default integration: copilot" in result.output assert "Installed integrations: copilot" in result.output assert "Shared templates target alignment: copilot" in result.output assert "Modified managed files: 0" in result.output assert "Missing managed files: 0" in result.output - def test_doctor_json_reports_healthy_project(self, tmp_path): + def test_status_json_reports_healthy_project(self, tmp_path): project = _init_project(tmp_path, "copilot") - result = _run_in_project(project, ["integration", "doctor", "--json"]) + result = _run_in_project(project, ["integration", "status", "--json"]) assert result.exit_code == 0 payload = json.loads(result.output) @@ -161,28 +161,28 @@ def test_doctor_json_reports_healthy_project(self, tmp_path): assert "shared_templates_aligned_to" not in payload assert payload["findings"] == [] - def test_doctor_reports_invalid_integration_json(self, tmp_path): + def test_status_reports_invalid_integration_json(self, tmp_path): project = _init_project(tmp_path, "copilot") (project / ".specify" / "integration.json").write_text("{", encoding="utf-8") - result = _run_in_project(project, ["integration", "doctor"]) + result = _run_in_project(project, ["integration", "status"]) assert result.exit_code != 0 assert "integration-state-unreadable" in result.output assert "invalid JSON" in result.output assert "Traceback" not in result.output - def test_doctor_reports_missing_integration_json(self, tmp_path): + def test_status_reports_missing_integration_json(self, tmp_path): project = _init_project(tmp_path, "copilot") (project / ".specify" / "integration.json").unlink() - result = _run_in_project(project, ["integration", "doctor"]) + result = _run_in_project(project, ["integration", "status"]) assert result.exit_code != 0 assert "integration-state-missing" in result.output assert ".specify/integration.json is missing" in result.output - def test_doctor_reports_default_integration_not_installed(self, tmp_path): + def test_status_reports_default_integration_not_installed(self, tmp_path): project = _init_project(tmp_path, "claude") state_path = project / ".specify" / "integration.json" state = json.loads(state_path.read_text(encoding="utf-8")) @@ -191,27 +191,27 @@ def test_doctor_reports_default_integration_not_installed(self, tmp_path): state["installed_integrations"] = ["claude"] state_path.write_text(json.dumps(state), encoding="utf-8") - result = _run_in_project(project, ["integration", "doctor"]) + result = _run_in_project(project, ["integration", "status"]) assert result.exit_code != 0 assert "default-integration-not-installed" in result.output assert "Default integration 'codex' is not listed" in result.output - def test_doctor_reports_missing_manifest(self, tmp_path): + def test_status_reports_missing_manifest(self, tmp_path): project = _init_project(tmp_path, "copilot") (project / ".specify" / "integrations" / "copilot.manifest.json").unlink() - result = _run_in_project(project, ["integration", "doctor"]) + result = _run_in_project(project, ["integration", "status"]) assert result.exit_code != 0 assert "manifest-missing" in result.output assert "Manifest for integration 'copilot' is missing" in result.output - def test_doctor_reports_unreadable_manifest_in_json_summary(self, tmp_path): + def test_status_reports_unreadable_manifest_in_json_summary(self, tmp_path): project = _init_project(tmp_path, "copilot") _write_invalid_manifest(project, "copilot") - result = _run_in_project(project, ["integration", "doctor", "--json"]) + result = _run_in_project(project, ["integration", "status", "--json"]) assert result.exit_code != 0 payload = json.loads(result.output) @@ -220,47 +220,47 @@ def test_doctor_reports_unreadable_manifest_in_json_summary(self, tmp_path): assert payload["manifests"]["copilot"]["missing_files"] == [] assert payload["manifests"]["copilot"]["modified_files"] == [] - def test_doctor_reports_modified_managed_files_without_failing(self, tmp_path): + def test_status_reports_modified_managed_files_without_failing(self, tmp_path): project = _init_project(tmp_path, "copilot") manifest_path = project / ".specify" / "integrations" / "copilot.manifest.json" tracked_files = json.loads(manifest_path.read_text(encoding="utf-8"))["files"] first_rel = next(iter(tracked_files)) (project / first_rel).write_text("MODIFIED CONTENT\n", encoding="utf-8") - result = _run_in_project(project, ["integration", "doctor"]) + result = _run_in_project(project, ["integration", "status"]) assert result.exit_code == 0 - assert "Integration state: WARNING" in result.output + assert "Integration status: WARNING" in result.output assert "managed-files-modified" in result.output assert "Modified managed files: 1" in result.output - def test_doctor_reports_missing_managed_files(self, tmp_path): + def test_status_reports_missing_managed_files(self, tmp_path): project = _init_project(tmp_path, "copilot") manifest_path = project / ".specify" / "integrations" / "copilot.manifest.json" tracked_files = json.loads(manifest_path.read_text(encoding="utf-8"))["files"] first_rel = next(iter(tracked_files)) (project / first_rel).unlink() - result = _run_in_project(project, ["integration", "doctor"]) + result = _run_in_project(project, ["integration", "status"]) assert result.exit_code != 0 assert "managed-files-missing" in result.output assert "Missing managed files: 1" in result.output - def test_doctor_reports_missing_shared_managed_files(self, tmp_path): + def test_status_reports_missing_shared_managed_files(self, tmp_path): project = _init_project(tmp_path, "copilot") shared_file = project / ".specify" / "scripts" / "bash" / "common.sh" assert shared_file.exists() shared_file.unlink() - result = _run_in_project(project, ["integration", "doctor"]) + result = _run_in_project(project, ["integration", "status"]) assert result.exit_code != 0 assert "managed-files-missing" in result.output assert "shared Spec Kit infrastructure" in result.output assert "Missing managed files: 1" in result.output - def test_doctor_treats_dangling_symlink_as_missing(self, tmp_path): + def test_status_treats_dangling_symlink_as_missing(self, tmp_path): project = _init_project(tmp_path, "copilot") manifest_path = project / ".specify" / "integrations" / "copilot.manifest.json" tracked_files = json.loads(manifest_path.read_text(encoding="utf-8"))["files"] @@ -272,14 +272,14 @@ def test_doctor_treats_dangling_symlink_as_missing(self, tmp_path): except OSError as exc: pytest.skip(f"symlinks unavailable: {exc}") - result = _run_in_project(project, ["integration", "doctor", "--json"]) + result = _run_in_project(project, ["integration", "status", "--json"]) assert result.exit_code != 0 payload = json.loads(result.output) assert first_rel in payload["manifests"]["copilot"]["missing_files"] assert first_rel not in payload["manifests"]["copilot"]["modified_files"] - def test_doctor_reports_unsafe_manifest_paths_without_hashing_them(self, tmp_path): + def test_status_reports_unsafe_manifest_paths_without_hashing_them(self, tmp_path): project = _init_project(tmp_path, "copilot") outside = tmp_path / "outside" outside.mkdir() @@ -295,7 +295,7 @@ def test_doctor_reports_unsafe_manifest_paths_without_hashing_them(self, tmp_pat manifest_data["files"]["outside-link/secret.txt"] = "wrong" manifest_path.write_text(json.dumps(manifest_data), encoding="utf-8") - result = _run_in_project(project, ["integration", "doctor", "--json"]) + result = _run_in_project(project, ["integration", "status", "--json"]) assert result.exit_code != 0 payload = json.loads(result.output) @@ -303,7 +303,7 @@ def test_doctor_reports_unsafe_manifest_paths_without_hashing_them(self, tmp_pat assert "outside-link/secret.txt" in payload["manifests"]["copilot"]["invalid_files"] assert "outside-link/secret.txt" not in payload["manifests"]["copilot"]["modified_files"] - def test_doctor_reports_unsafe_multi_install_combination(self, tmp_path): + def test_status_reports_unsafe_multi_install_combination(self, tmp_path): from specify_cli.integrations.manifest import IntegrationManifest project = _init_project(tmp_path, "copilot") @@ -315,13 +315,13 @@ def test_doctor_reports_unsafe_multi_install_combination(self, tmp_path): state_path.write_text(json.dumps(state), encoding="utf-8") IntegrationManifest("claude", project, version="test").save() - result = _run_in_project(project, ["integration", "doctor"]) + result = _run_in_project(project, ["integration", "status"]) assert result.exit_code != 0 assert "unsafe-multi-install" in result.output assert "Multi-install safe: no" in result.output - def test_doctor_treats_unknown_multi_install_as_unsafe(self, tmp_path): + def test_status_treats_unknown_multi_install_as_unsafe(self, tmp_path): from specify_cli.integrations.manifest import IntegrationManifest project = _init_project(tmp_path, "claude") @@ -333,14 +333,14 @@ def test_doctor_treats_unknown_multi_install_as_unsafe(self, tmp_path): state_path.write_text(json.dumps(state), encoding="utf-8") IntegrationManifest("mystery", project, version="test").save() - result = _run_in_project(project, ["integration", "doctor"]) + result = _run_in_project(project, ["integration", "status"]) assert result.exit_code != 0 assert "unknown-integration" in result.output assert "unsafe-multi-install" in result.output assert "Multi-install safe: no" in result.output - def test_doctor_reports_managed_file_collisions(self, tmp_path): + def test_status_reports_managed_file_collisions(self, tmp_path): from specify_cli.integrations.manifest import IntegrationManifest project = _init_project(tmp_path, "claude") @@ -358,13 +358,13 @@ def test_doctor_reports_managed_file_collisions(self, tmp_path): codex_manifest.record_existing(shared_rel) codex_manifest.save() - result = _run_in_project(project, ["integration", "doctor"]) + result = _run_in_project(project, ["integration", "status"]) assert result.exit_code == 0 assert "managed-file-collision" in result.output - assert "Integration state: WARNING" in result.output + assert "Integration status: WARNING" in result.output - def test_doctor_json_is_not_rich_rendered(self, tmp_path, monkeypatch): + def test_status_json_is_not_rich_rendered(self, tmp_path, monkeypatch): project = tmp_path / "proj" project.mkdir() (project / ".specify").mkdir() @@ -377,7 +377,7 @@ def test_doctor_json_is_not_rich_rendered(self, tmp_path, monkeypatch): ) monkeypatch.chdir(project) - result = runner.invoke(app, ["integration", "doctor", "--json"]) + result = runner.invoke(app, ["integration", "status", "--json"]) assert result.exit_code != 0 payload = json.loads(result.output) From 81ed372f794b3588b0c27ef220a90bd98cbebf6a Mon Sep 17 00:00:00 2001 From: Pascal Date: Tue, 26 May 2026 17:57:19 +0200 Subject: [PATCH 05/11] fix(integration): address status review feedback --- docs/reference/integrations.md | 2 + src/specify_cli/integration_state.py | 39 +++++++++++---- src/specify_cli/integration_status.py | 48 +++++++++++++------ .../test_integration_subcommand.py | 40 ++++++++++++++++ 4 files changed, 105 insertions(+), 24 deletions(-) diff --git a/docs/reference/integrations.md b/docs/reference/integrations.md index 4f555117b0..a679e834f4 100644 --- a/docs/reference/integrations.md +++ b/docs/reference/integrations.md @@ -136,6 +136,8 @@ multi-install safety, missing managed files, modified managed files, invalid manifest paths, shared Spec Kit infrastructure health, unchecked manifests, and the target integration for default-sensitive shared templates. The JSON form is intended for CI and coding agents that need stable machine-readable status data. +The command exits 0 when the report status is `ok` or `warning`; it exits 1 +only when the report status is `error`. ## Integration-Specific Options diff --git a/src/specify_cli/integration_state.py b/src/specify_cli/integration_state.py index 2b18324351..eaeb10eb1b 100644 --- a/src/specify_cli/integration_state.py +++ b/src/specify_cli/integration_state.py @@ -25,18 +25,9 @@ class IntegrationReadError: schema: int | None = None -def try_read_integration_json( +def _read_integration_json_data( project_root: Path, ) -> tuple[dict[str, Any] | None, IntegrationReadError | None]: - """Parse ``.specify/integration.json`` without raising. - - Returns ``(normalized_state, None)`` on success, ``(None, None)`` when the - file does not exist, or ``(None, error)`` for any parse / validation - failure. This is the single low-level reader; both the CLI's loud - ``_read_integration_json`` and the workflow engine's silent - ``_load_project_integration`` consume it so the schema guard and parse - logic cannot drift between them. - """ path = project_root / INTEGRATION_JSON # Avoid Path.exists() / Path.is_file() as a pre-check: both return False # on some OSErrors (e.g. permission errors during stat), which would @@ -70,9 +61,37 @@ def try_read_integration_json( and schema > INTEGRATION_STATE_SCHEMA ): return None, IntegrationReadError(kind="schema_too_new", schema=schema) + return data, None + + +def try_read_integration_json( + project_root: Path, +) -> tuple[dict[str, Any] | None, IntegrationReadError | None]: + """Parse ``.specify/integration.json`` without raising. + + Returns ``(normalized_state, None)`` on success, ``(None, None)`` when the + file does not exist, or ``(None, error)`` for any parse / validation + failure. This is the single low-level reader; both the CLI's loud + ``_read_integration_json`` and the workflow engine's silent + ``_load_project_integration`` consume it so the schema guard and parse + logic cannot drift between them. + """ + data, error = _read_integration_json_data(project_root) + if data is None: + return None, error return normalize_integration_state(data), None +def try_read_integration_json_with_raw( + project_root: Path, +) -> tuple[dict[str, Any] | None, dict[str, Any] | None, IntegrationReadError | None]: + """Parse ``integration.json`` and return normalized plus raw state.""" + data, error = _read_integration_json_data(project_root) + if data is None: + return None, None, error + return normalize_integration_state(data), data, None + + 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(): diff --git a/src/specify_cli/integration_status.py b/src/specify_cli/integration_status.py index 6b71c6f3e5..bef7a68f0d 100644 --- a/src/specify_cli/integration_status.py +++ b/src/specify_cli/integration_status.py @@ -2,7 +2,7 @@ from __future__ import annotations -import json +import hashlib from pathlib import Path from typing import Any @@ -11,10 +11,10 @@ IntegrationReadError, default_integration_key, installed_integration_keys, - try_read_integration_json, + try_read_integration_json_with_raw, ) from .integrations import INTEGRATION_REGISTRY -from .integrations.manifest import IntegrationManifest, _sha256 +from .integrations.manifest import IntegrationManifest _MANIFEST_READ_ERRORS = (ValueError, FileNotFoundError, OSError, UnicodeDecodeError) _SHARED_MANIFEST_KEY = "speckit" @@ -66,13 +66,25 @@ def _integration_state_error_message(error: IntegrationReadError) -> str: return f"Could not inspect {INTEGRATION_JSON}." -def _safe_manifest_file(project_root: Path, rel: str) -> Path | None: +def _sha256_file(path: Path) -> str: + h = hashlib.sha256() + with open(path, "rb") as fh: + for chunk in iter(lambda: fh.read(8192), b""): + h.update(chunk) + return h.hexdigest() + + +def _safe_manifest_file( + project_root: Path, + project_root_resolved: Path, + rel: str, +) -> Path | None: rel_path = Path(rel) if rel_path.is_absolute() or ".." in rel_path.parts: return None candidate = project_root / rel_path try: - candidate.parent.resolve(strict=False).relative_to(project_root.resolve()) + candidate.parent.resolve(strict=False).relative_to(project_root_resolved) except (OSError, ValueError): return None return candidate @@ -80,6 +92,7 @@ def _safe_manifest_file(project_root: Path, rel: str) -> Path | None: def _manifest_file_status( manifest: IntegrationManifest, + project_root_resolved: Path, ) -> tuple[list[str], list[str], list[str], list[str]]: missing: list[str] = [] modified: list[str] = [] @@ -87,7 +100,7 @@ def _manifest_file_status( valid: list[str] = [] for rel, expected_hash in manifest.files.items(): - path = _safe_manifest_file(manifest.project_root, rel) + path = _safe_manifest_file(manifest.project_root, project_root_resolved, rel) if path is None: invalid.append(rel) continue @@ -99,7 +112,7 @@ def _manifest_file_status( modified.append(rel) continue try: - if _sha256(path) != expected_hash: + if _sha256_file(path) != expected_hash: modified.append(rel) except OSError: modified.append(rel) @@ -107,8 +120,7 @@ def _manifest_file_status( return missing, modified, invalid, valid -def _default_not_installed_from_raw_state(project_root: Path) -> str | None: - raw_state = json.loads((project_root / INTEGRATION_JSON).read_text(encoding="utf-8")) +def _default_not_installed_from_raw_state(raw_state: dict[str, Any]) -> str | None: if not isinstance(raw_state.get("installed_integrations"), list): return None @@ -156,7 +168,8 @@ def _manifest_suggestion(key: str, default_key: str | None) -> str: def build_integration_status_report(project_root: Path) -> dict[str, Any]: """Return a machine-readable integration status report for *project_root*.""" findings: list[dict[str, str]] = [] - state, error = try_read_integration_json(project_root) + project_root_resolved = project_root.resolve() + state, raw_state, error = try_read_integration_json_with_raw(project_root) if error is not None: findings.append( _finding( @@ -181,7 +194,10 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: ) return _build_report(None, [], findings, {}, True) - default_key = default_integration_key(state) + assert raw_state is not None + raw_default_key = default_integration_key(raw_state) + raw_installed_keys = installed_integration_keys(raw_state) + default_key = raw_default_key or default_integration_key(state) installed_keys = installed_integration_keys(state) if not installed_keys: findings.append( @@ -194,7 +210,8 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: ) return _build_report(default_key, installed_keys, findings, {}, True) - if default_key is None: + if raw_installed_keys and raw_default_key is None: + default_key = None findings.append( _finding( "error", @@ -204,7 +221,7 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: ) ) - raw_default_not_installed = _default_not_installed_from_raw_state(project_root) + raw_default_not_installed = _default_not_installed_from_raw_state(raw_state) if raw_default_not_installed: findings.append( _finding( @@ -301,7 +318,10 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: ) continue - missing, modified, invalid, valid_files = _manifest_file_status(manifest) + missing, modified, invalid, valid_files = _manifest_file_status( + manifest, + project_root_resolved, + ) manifest_summaries[key] = _manifest_summary( manifest_path, project_root, diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index 42b53aa682..3cd5c634a6 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -182,6 +182,46 @@ def test_status_reports_missing_integration_json(self, tmp_path): assert "integration-state-missing" in result.output assert ".specify/integration.json is missing" in result.output + def test_status_json_reports_no_installed_integrations_as_warning(self, tmp_path): + project = _init_project(tmp_path, "copilot") + state_path = project / ".specify" / "integration.json" + state_path.write_text( + json.dumps({ + "version": "test", + "integration_state_schema": 1, + "installed_integrations": [], + }), + encoding="utf-8", + ) + + result = _run_in_project(project, ["integration", "status", "--json"]) + + assert result.exit_code == 0 + payload = json.loads(result.output) + assert payload["status"] == "warning" + assert payload["installed_integrations"] == [] + assert payload["findings"][0]["code"] == "no-installed-integrations" + + def test_status_json_reports_missing_default_integration_as_error(self, tmp_path): + project = _init_project(tmp_path, "claude") + state_path = project / ".specify" / "integration.json" + state = json.loads(state_path.read_text(encoding="utf-8")) + state.pop("default_integration", None) + state.pop("integration", None) + state["installed_integrations"] = ["claude"] + state_path.write_text(json.dumps(state), encoding="utf-8") + + result = _run_in_project(project, ["integration", "status", "--json"]) + + assert result.exit_code != 0 + payload = json.loads(result.output) + assert payload["status"] == "error" + assert payload["default_integration"] is None + assert any( + item["code"] == "default-integration-missing" + for item in payload["findings"] + ) + def test_status_reports_default_integration_not_installed(self, tmp_path): project = _init_project(tmp_path, "claude") state_path = project / ".specify" / "integration.json" From 485b143d84bc844352a842ee83269508e737e187 Mon Sep 17 00:00:00 2001 From: Pascal Date: Tue, 26 May 2026 18:12:36 +0200 Subject: [PATCH 06/11] fix(integration): validate status manifest keys --- src/specify_cli/integration_status.py | 22 ++++++++++++++ .../test_integration_subcommand.py | 29 +++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/src/specify_cli/integration_status.py b/src/specify_cli/integration_status.py index bef7a68f0d..06c2d2153c 100644 --- a/src/specify_cli/integration_status.py +++ b/src/specify_cli/integration_status.py @@ -90,6 +90,15 @@ def _safe_manifest_file( return candidate +def _is_safe_manifest_key(key: str) -> bool: + if key in {"", ".", ".."}: + return False + if "/" in key or "\\" in key: + return False + key_path = Path(key) + return not key_path.is_absolute() and key_path.name == key + + def _manifest_file_status( manifest: IntegrationManifest, project_root_resolved: Path, @@ -279,6 +288,19 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: for key in manifest_keys: owner = _manifest_owner(key) + if not _is_safe_manifest_key(key): + findings.append( + _finding( + "error", + "integration-key-invalid", + f"Integration key {key!r} cannot be used as a manifest filename.", + integration=key, + path=INTEGRATION_JSON, + suggestion=f"Fix {INTEGRATION_JSON}, then reinstall the integration.", + ) + ) + continue + manifest_path = project_root / ".specify" / "integrations" / f"{key}.manifest.json" if not manifest_path.exists(): findings.append( diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index 3cd5c634a6..ad354c798c 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -380,6 +380,35 @@ def test_status_treats_unknown_multi_install_as_unsafe(self, tmp_path): assert "unsafe-multi-install" in result.output assert "Multi-install safe: no" in result.output + def test_status_rejects_unsafe_integration_keys_before_manifest_lookup(self, tmp_path): + project = _init_project(tmp_path, "claude") + state_path = project / ".specify" / "integration.json" + unsafe_key = "../../../escape" + state_path.write_text( + json.dumps({ + "integration": unsafe_key, + "default_integration": unsafe_key, + "installed_integrations": [unsafe_key], + }), + encoding="utf-8", + ) + outside_manifest = tmp_path / "escape.manifest.json" + outside_manifest.write_text( + json.dumps({"integration": unsafe_key, "files": {}}), + encoding="utf-8", + ) + + result = _run_in_project(project, ["integration", "status", "--json"]) + + assert result.exit_code != 0 + payload = json.loads(result.output) + assert unsafe_key not in payload["manifests"] + assert any( + item["code"] == "integration-key-invalid" + and item["integration"] == unsafe_key + for item in payload["findings"] + ) + def test_status_reports_managed_file_collisions(self, tmp_path): from specify_cli.integrations.manifest import IntegrationManifest From b3a245b67440d710875fbb04dbaf61c5bb77f850 Mon Sep 17 00:00:00 2001 From: Pascal Date: Tue, 26 May 2026 18:19:04 +0200 Subject: [PATCH 07/11] fix(integration): escape status report output --- src/specify_cli/__init__.py | 24 +++++-- src/specify_cli/integration_status.py | 16 +++++ .../test_integration_subcommand.py | 65 +++++++++++++++++++ 3 files changed, 98 insertions(+), 7 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 9f5f2f4a9b..8f46311257 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -41,6 +41,7 @@ from rich.panel import Panel from rich.live import Live from rich.align import Align +from rich.markup import escape as _rich_escape from rich.table import Table from .integration_runtime import ( invoke_separator_for_integration as _invoke_separator_for_integration, @@ -1623,14 +1624,17 @@ def _print_integration_status_report(report: dict[str, Any]) -> None: "error": "[red]ERROR[/red]", }.get(status, status.upper()) installed = report.get("installed_integrations") or [] + installed_display = ", ".join(_rich_escape(str(item)) for item in installed) console.print(f"Integration status: {status_label}") - console.print(f"Default integration: {report.get('default_integration') or 'none'}") - console.print(f"Installed integrations: {', '.join(installed) if installed else 'none'}") + console.print( + f"Default integration: {_rich_escape(str(report.get('default_integration') or 'none'))}" + ) + console.print(f"Installed integrations: {installed_display if installed else 'none'}") console.print(f"Multi-install safe: {'yes' if report.get('multi_install_safe') else 'no'}") console.print( f"Shared templates target alignment: " - f"{report.get('shared_templates_target_alignment') or 'none'}" + f"{_rich_escape(str(report.get('shared_templates_target_alignment') or 'none'))}" ) console.print(f"Modified managed files: {report.get('modified_managed_files', 0)}") console.print(f"Missing managed files: {report.get('missing_managed_files', 0)}") @@ -1649,12 +1653,18 @@ def _print_integration_status_report(report: dict[str, Any]) -> None: "error": "[red]error[/red]", "warning": "[yellow]warning[/yellow]", }.get(severity, severity) - prefix = f"- {severity_label} {item.get('code', '')}" + prefix = f"- {severity_label} {_rich_escape(str(item.get('code', '')))}" if item.get("integration"): - prefix += f" ({item['integration']})" - console.print(f"{prefix}: {item.get('message', '')}", soft_wrap=True) + prefix += f" ({_rich_escape(str(item['integration']))})" + console.print( + f"{prefix}: {_rich_escape(str(item.get('message', '')))}", + soft_wrap=True, + ) if item.get("suggestion"): - console.print(f" Suggestion: {item['suggestion']}", soft_wrap=True) + console.print( + f" Suggestion: {_rich_escape(str(item['suggestion']))}", + soft_wrap=True, + ) @integration_app.command("status") diff --git a/src/specify_cli/integration_status.py b/src/specify_cli/integration_status.py index 06c2d2153c..965c3afe11 100644 --- a/src/specify_cli/integration_status.py +++ b/src/specify_cli/integration_status.py @@ -3,6 +3,7 @@ from __future__ import annotations import hashlib +import re from pathlib import Path from typing import Any @@ -17,6 +18,15 @@ from .integrations.manifest import IntegrationManifest _MANIFEST_READ_ERRORS = (ValueError, FileNotFoundError, OSError, UnicodeDecodeError) +_MANIFEST_KEY_RE = re.compile(r"^[A-Za-z0-9._-]+$") +_WINDOWS_RESERVED_MANIFEST_BASENAMES = { + "CON", + "PRN", + "AUX", + "NUL", + *(f"COM{i}" for i in range(1, 10)), + *(f"LPT{i}" for i in range(1, 10)), +} _SHARED_MANIFEST_KEY = "speckit" @@ -93,6 +103,12 @@ def _safe_manifest_file( def _is_safe_manifest_key(key: str) -> bool: if key in {"", ".", ".."}: return False + if key.endswith("."): + return False + if _MANIFEST_KEY_RE.fullmatch(key) is None: + return False + if key.split(".", 1)[0].upper() in _WINDOWS_RESERVED_MANIFEST_BASENAMES: + return False if "/" in key or "\\" in key: return False key_path = Path(key) diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index ad354c798c..3d3d489d21 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -409,6 +409,52 @@ def test_status_rejects_unsafe_integration_keys_before_manifest_lookup(self, tmp for item in payload["findings"] ) + def test_status_rejects_filename_invalid_integration_keys(self, tmp_path): + project = _init_project(tmp_path, "claude") + state_path = project / ".specify" / "integration.json" + unsafe_key = "bad:key" + state_path.write_text( + json.dumps({ + "integration": unsafe_key, + "default_integration": unsafe_key, + "installed_integrations": [unsafe_key], + }), + encoding="utf-8", + ) + + result = _run_in_project(project, ["integration", "status", "--json"]) + + assert result.exit_code != 0 + payload = json.loads(result.output) + assert any( + item["code"] == "integration-key-invalid" + and item["integration"] == unsafe_key + for item in payload["findings"] + ) + + def test_status_rejects_windows_reserved_integration_keys(self, tmp_path): + project = _init_project(tmp_path, "claude") + state_path = project / ".specify" / "integration.json" + unsafe_key = "CON" + state_path.write_text( + json.dumps({ + "integration": unsafe_key, + "default_integration": unsafe_key, + "installed_integrations": [unsafe_key], + }), + encoding="utf-8", + ) + + result = _run_in_project(project, ["integration", "status", "--json"]) + + assert result.exit_code != 0 + payload = json.loads(result.output) + assert any( + item["code"] == "integration-key-invalid" + and item["integration"] == unsafe_key + for item in payload["findings"] + ) + def test_status_reports_managed_file_collisions(self, tmp_path): from specify_cli.integrations.manifest import IntegrationManifest @@ -453,6 +499,25 @@ def test_status_json_is_not_rich_rendered(self, tmp_path, monkeypatch): assert payload["default_integration"] == "[red]x[/red]" assert payload["installed_integrations"] == ["[red]x[/red]"] + def test_status_text_escapes_rich_markup_from_project_state(self, tmp_path, monkeypatch): + project = tmp_path / "proj" + project.mkdir() + (project / ".specify").mkdir() + (project / ".specify" / "integration.json").write_text( + json.dumps({ + "integration": "[red]x[/red]", + "installed_integrations": ["[red]x[/red]"], + }), + encoding="utf-8", + ) + monkeypatch.chdir(project) + + result = runner.invoke(app, ["integration", "status"]) + + assert result.exit_code != 0 + assert "Default integration: [red]x[/red]" in result.output + assert "Installed integrations: [red]x[/red]" in result.output + # ── install ────────────────────────────────────────────────────────── From 7d505aaefbf06ca8c42ebe6584676b2876e2448f Mon Sep 17 00:00:00 2001 From: Pascal Date: Wed, 27 May 2026 15:51:07 +0200 Subject: [PATCH 08/11] fix(integration): address status review feedback --- src/specify_cli/integration_status.py | 17 ++++++---- .../test_integration_subcommand.py | 32 +++++++++++++++++-- 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/src/specify_cli/integration_status.py b/src/specify_cli/integration_status.py index 965c3afe11..2aafc06dec 100644 --- a/src/specify_cli/integration_status.py +++ b/src/specify_cli/integration_status.py @@ -9,6 +9,7 @@ from .integration_state import ( INTEGRATION_JSON, + INTEGRATION_STATE_SCHEMA, IntegrationReadError, default_integration_key, installed_integration_keys, @@ -71,7 +72,7 @@ def _integration_state_error_message(error: IntegrationReadError) -> str: if error.kind == "schema_too_new": return ( f"{INTEGRATION_JSON} uses integration state schema {error.schema}, " - "which is newer than this CLI supports." + f"which is newer than this CLI supports; supported schema: {INTEGRATION_STATE_SCHEMA}." ) return f"Could not inspect {INTEGRATION_JSON}." @@ -224,6 +225,8 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: raw_installed_keys = installed_integration_keys(raw_state) default_key = raw_default_key or default_integration_key(state) installed_keys = installed_integration_keys(state) + raw_installed_is_list = isinstance(raw_state.get("installed_integrations"), list) + check_installed_keys = raw_installed_keys if raw_installed_is_list else installed_keys if not installed_keys: findings.append( _finding( @@ -261,9 +264,9 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: ) ) - known_installed = [key for key in installed_keys if key in INTEGRATION_REGISTRY] + known_installed = [key for key in check_installed_keys if key in INTEGRATION_REGISTRY] unknown_installed: list[str] = [] - for key in installed_keys: + for key in check_installed_keys: if key not in INTEGRATION_REGISTRY: unknown_installed.append(key) findings.append( @@ -280,10 +283,10 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: key for key in known_installed if not getattr(INTEGRATION_REGISTRY[key], "multi_install_safe", False) ] - if len(installed_keys) > 1: + if len(check_installed_keys) > 1: unsafe.extend(unknown_installed) - if len(installed_keys) > 1 and unsafe: + if len(check_installed_keys) > 1 and unsafe: findings.append( _finding( "error", @@ -298,7 +301,7 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: manifest_files_by_path: dict[str, list[str]] = {} manifest_summaries: dict[str, dict[str, Any]] = {} - manifest_keys = list(installed_keys) + manifest_keys = list(check_installed_keys) if _SHARED_MANIFEST_KEY not in manifest_keys: manifest_keys.append(_SHARED_MANIFEST_KEY) @@ -416,7 +419,7 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: ) ) - multi_install_safe = not (len(installed_keys) > 1 and unsafe) + multi_install_safe = not (len(check_installed_keys) > 1 and unsafe) return _build_report(default_key, installed_keys, findings, manifest_summaries, multi_install_safe) diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index 3d3d489d21..5e319912d8 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -172,6 +172,21 @@ def test_status_reports_invalid_integration_json(self, tmp_path): assert "invalid JSON" in result.output assert "Traceback" not in result.output + def test_status_reports_supported_schema_for_newer_integration_state(self, tmp_path): + project = _init_project(tmp_path, "copilot") + state_path = project / ".specify" / "integration.json" + state = json.loads(state_path.read_text(encoding="utf-8")) + state["integration_state_schema"] = 99 + state_path.write_text(json.dumps(state), encoding="utf-8") + + result = _run_in_project(project, ["integration", "status", "--json"]) + + assert result.exit_code != 0 + payload = json.loads(result.output) + assert payload["findings"][0]["code"] == "integration-state-unreadable" + assert "schema 99" in payload["findings"][0]["message"] + assert "supported schema: 1" in payload["findings"][0]["message"] + def test_status_reports_missing_integration_json(self, tmp_path): project = _init_project(tmp_path, "copilot") (project / ".specify" / "integration.json").unlink() @@ -231,11 +246,22 @@ def test_status_reports_default_integration_not_installed(self, tmp_path): state["installed_integrations"] = ["claude"] state_path.write_text(json.dumps(state), encoding="utf-8") - result = _run_in_project(project, ["integration", "status"]) + result = _run_in_project(project, ["integration", "status", "--json"]) assert result.exit_code != 0 - assert "default-integration-not-installed" in result.output - assert "Default integration 'codex' is not listed" in result.output + payload = json.loads(result.output) + assert payload["default_integration"] == "codex" + assert payload["installed_integrations"] == ["codex", "claude"] + assert any( + item["code"] == "default-integration-not-installed" + and "Default integration 'codex' is not listed" in item["message"] + for item in payload["findings"] + ) + assert "codex" not in payload["manifests"] + assert not any( + item["code"] == "manifest-missing" and item.get("integration") == "codex" + for item in payload["findings"] + ) def test_status_reports_missing_manifest(self, tmp_path): project = _init_project(tmp_path, "copilot") From 40477a2ff3854c23710c4d4b904b897f87375270 Mon Sep 17 00:00:00 2001 From: Pascal Date: Wed, 27 May 2026 22:22:59 +0200 Subject: [PATCH 09/11] fix(integration): harden status manifest checks --- src/specify_cli/integration_status.py | 8 ++--- .../test_integration_subcommand.py | 32 +++++++++++++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/src/specify_cli/integration_status.py b/src/specify_cli/integration_status.py index 2aafc06dec..6938242bf0 100644 --- a/src/specify_cli/integration_status.py +++ b/src/specify_cli/integration_status.py @@ -236,7 +236,6 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: suggestion="Run `specify integration install ` to install one.", ) ) - return _build_report(default_key, installed_keys, findings, {}, True) if raw_installed_keys and raw_default_key is None: default_key = None @@ -321,7 +320,9 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: continue manifest_path = project_root / ".specify" / "integrations" / f"{key}.manifest.json" - if not manifest_path.exists(): + try: + manifest = IntegrationManifest.load(key, project_root) + except FileNotFoundError: findings.append( _finding( "error", @@ -338,9 +339,6 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: readable=False, ) continue - - try: - manifest = IntegrationManifest.load(key, project_root) except _MANIFEST_READ_ERRORS as exc: manifest_summaries[key] = _manifest_summary( manifest_path, diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index 5e319912d8..a6b30679c3 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -216,6 +216,38 @@ def test_status_json_reports_no_installed_integrations_as_warning(self, tmp_path assert payload["status"] == "warning" assert payload["installed_integrations"] == [] assert payload["findings"][0]["code"] == "no-installed-integrations" + assert "speckit" in payload["manifests"] + assert payload["manifests"]["speckit"]["readable"] is True + + def test_status_checks_shared_manifest_when_no_integrations_installed(self, tmp_path): + project = _init_project(tmp_path, "copilot") + state_path = project / ".specify" / "integration.json" + state_path.write_text( + json.dumps({ + "version": "test", + "integration_state_schema": 1, + "installed_integrations": [], + }), + encoding="utf-8", + ) + (project / ".specify" / "integrations" / "speckit.manifest.json").unlink() + + result = _run_in_project(project, ["integration", "status", "--json"]) + + assert result.exit_code != 0 + payload = json.loads(result.output) + assert payload["status"] == "error" + assert payload["installed_integrations"] == [] + assert payload["unchecked_manifests"] == 1 + assert any( + item["code"] == "no-installed-integrations" + for item in payload["findings"] + ) + assert any( + item["code"] == "manifest-missing" + and item["integration"] == "speckit" + for item in payload["findings"] + ) def test_status_json_reports_missing_default_integration_as_error(self, tmp_path): project = _init_project(tmp_path, "claude") From ee8ddc05464198dfcbdec101295d1dd2ad8619c2 Mon Sep 17 00:00:00 2001 From: Pascal Date: Wed, 27 May 2026 22:52:43 +0200 Subject: [PATCH 10/11] fix(integration): tighten status diagnostics --- src/specify_cli/integration_status.py | 31 ++++++++++-- .../test_integration_subcommand.py | 49 ++++++++++++++++++- 2 files changed, 76 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/integration_status.py b/src/specify_cli/integration_status.py index 6938242bf0..5fd5be5ac4 100644 --- a/src/specify_cli/integration_status.py +++ b/src/specify_cli/integration_status.py @@ -4,6 +4,7 @@ import hashlib import re +import stat from pathlib import Path from typing import Any @@ -131,10 +132,25 @@ def _manifest_file_status( invalid.append(rel) continue valid.append(rel) - if not path.exists(): + try: + path_stat = path.lstat() + except FileNotFoundError: missing.append(rel) continue - if path.is_symlink() or not path.is_file(): + except OSError: + modified.append(rel) + continue + if stat.S_ISLNK(path_stat.st_mode): + try: + path.stat() + except FileNotFoundError: + missing.append(rel) + continue + except OSError: + pass + modified.append(rel) + continue + if not stat.S_ISREG(path_stat.st_mode): modified.append(rel) continue try: @@ -188,6 +204,8 @@ def _manifest_suggestion(key: str, default_key: str | None) -> str: if default_key and default_key in INTEGRATION_REGISTRY: return f"Run `specify integration upgrade {default_key}` to regenerate shared managed files." return "Run `specify init --here --force` to regenerate shared managed files." + if key not in INTEGRATION_REGISTRY: + return f"Upgrade Spec Kit, or remove the stale integration metadata from {INTEGRATION_JSON}." return f"Run `specify integration upgrade {key}` or reinstall the integration." @@ -227,6 +245,7 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: installed_keys = installed_integration_keys(state) raw_installed_is_list = isinstance(raw_state.get("installed_integrations"), list) check_installed_keys = raw_installed_keys if raw_installed_is_list else installed_keys + report_installed_keys = check_installed_keys if not installed_keys: findings.append( _finding( @@ -418,7 +437,13 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: ) multi_install_safe = not (len(check_installed_keys) > 1 and unsafe) - return _build_report(default_key, installed_keys, findings, manifest_summaries, multi_install_safe) + return _build_report( + default_key, + report_installed_keys, + findings, + manifest_summaries, + multi_install_safe, + ) def _build_report( diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index a6b30679c3..e99543816f 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -2,6 +2,7 @@ import json import os +from pathlib import Path import pytest from typer.testing import CliRunner @@ -283,7 +284,7 @@ def test_status_reports_default_integration_not_installed(self, tmp_path): assert result.exit_code != 0 payload = json.loads(result.output) assert payload["default_integration"] == "codex" - assert payload["installed_integrations"] == ["codex", "claude"] + assert payload["installed_integrations"] == ["claude"] assert any( item["code"] == "default-integration-not-installed" and "Default integration 'codex' is not listed" in item["message"] @@ -358,6 +359,32 @@ def test_status_reports_missing_shared_managed_files(self, tmp_path): assert "shared Spec Kit infrastructure" in result.output assert "Missing managed files: 1" in result.output + def test_status_does_not_use_exists_precheck_for_managed_files(self, tmp_path, monkeypatch): + from specify_cli.integration_status import _manifest_file_status + from specify_cli.integrations.manifest import IntegrationManifest + + project = tmp_path / "proj" + project.mkdir() + tracked = project / "tracked.md" + tracked.write_text("content\n", encoding="utf-8") + manifest = IntegrationManifest("test", project, version="test") + manifest.record_existing("tracked.md") + + def fail_exists(self): + raise AssertionError(f"Path.exists() should not be used for {self}") + + monkeypatch.setattr(Path, "exists", fail_exists) + + missing, modified, invalid, valid = _manifest_file_status( + manifest, + project.resolve(), + ) + + assert missing == [] + assert modified == [] + assert invalid == [] + assert valid == ["tracked.md"] + def test_status_treats_dangling_symlink_as_missing(self, tmp_path): project = _init_project(tmp_path, "copilot") manifest_path = project / ".specify" / "integrations" / "copilot.manifest.json" @@ -438,6 +465,26 @@ def test_status_treats_unknown_multi_install_as_unsafe(self, tmp_path): assert "unsafe-multi-install" in result.output assert "Multi-install safe: no" in result.output + def test_status_gives_actionable_suggestion_for_unknown_manifest(self, tmp_path): + project = _init_project(tmp_path, "claude") + state_path = project / ".specify" / "integration.json" + state = json.loads(state_path.read_text(encoding="utf-8")) + state["installed_integrations"] = ["mystery"] + state["default_integration"] = "mystery" + state["integration"] = "mystery" + state_path.write_text(json.dumps(state), encoding="utf-8") + + result = _run_in_project(project, ["integration", "status", "--json"]) + + assert result.exit_code != 0 + payload = json.loads(result.output) + manifest_finding = next( + item for item in payload["findings"] + if item["code"] == "manifest-missing" and item["integration"] == "mystery" + ) + assert "remove the stale integration metadata" in manifest_finding["suggestion"] + assert "integration upgrade mystery" not in manifest_finding["suggestion"] + def test_status_rejects_unsafe_integration_keys_before_manifest_lookup(self, tmp_path): project = _init_project(tmp_path, "claude") state_path = project / ".specify" / "integration.json" From 99db1bbe3b09c942358f0149c870484709098f4d Mon Sep 17 00:00:00 2001 From: Pascal Date: Wed, 27 May 2026 23:09:30 +0200 Subject: [PATCH 11/11] fix(integration): clarify status state sources --- docs/reference/integrations.md | 4 +- src/specify_cli/integration_state.py | 6 + src/specify_cli/integration_status.py | 22 +- .../test_integration_subcommand.py | 229 ++++++++++-------- 4 files changed, 155 insertions(+), 106 deletions(-) diff --git a/docs/reference/integrations.md b/docs/reference/integrations.md index a679e834f4..9e60583145 100644 --- a/docs/reference/integrations.md +++ b/docs/reference/integrations.md @@ -135,7 +135,9 @@ status report includes the default integration, installed integrations, multi-install safety, missing managed files, modified managed files, invalid manifest paths, shared Spec Kit infrastructure health, unchecked manifests, and the target integration for default-sensitive shared templates. The JSON form is -intended for CI and coding agents that need stable machine-readable status data. +intended for CI and coding agents that need stable machine-readable status data; +it also reports the raw recorded integrations and the integration manifests that +were checked when state repair heuristics differ from the recorded file. The command exits 0 when the report status is `ok` or `warning`; it exits 1 only when the report status is `error`. diff --git a/src/specify_cli/integration_state.py b/src/specify_cli/integration_state.py index eaeb10eb1b..ca53dff3a4 100644 --- a/src/specify_cli/integration_state.py +++ b/src/specify_cli/integration_state.py @@ -28,6 +28,12 @@ class IntegrationReadError: def _read_integration_json_data( project_root: Path, ) -> tuple[dict[str, Any] | None, IntegrationReadError | None]: + """Read raw integration state without normalizing or raising. + + Returns ``(data, None)`` when the JSON object is readable and supported, + ``(None, None)`` when the file is absent, and ``(None, error)`` for parse, + schema, encoding, or filesystem failures. + """ path = project_root / INTEGRATION_JSON # Avoid Path.exists() / Path.is_file() as a pre-check: both return False # on some OSErrors (e.g. permission errors during stat), which would diff --git a/src/specify_cli/integration_status.py b/src/specify_cli/integration_status.py index 5fd5be5ac4..dfb9fad76c 100644 --- a/src/specify_cli/integration_status.py +++ b/src/specify_cli/integration_status.py @@ -244,8 +244,12 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: default_key = raw_default_key or default_integration_key(state) installed_keys = installed_integration_keys(state) raw_installed_is_list = isinstance(raw_state.get("installed_integrations"), list) - check_installed_keys = raw_installed_keys if raw_installed_is_list else installed_keys - report_installed_keys = check_installed_keys + raw_default_not_installed = _default_not_installed_from_raw_state(raw_state) + if raw_installed_is_list and raw_default_not_installed and raw_installed_keys: + check_installed_keys = raw_installed_keys + else: + check_installed_keys = installed_keys + recorded_installed_keys = raw_installed_keys if raw_installed_is_list else installed_keys if not installed_keys: findings.append( _finding( @@ -267,7 +271,6 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: ) ) - raw_default_not_installed = _default_not_installed_from_raw_state(raw_state) if raw_default_not_installed: findings.append( _finding( @@ -439,10 +442,12 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: multi_install_safe = not (len(check_installed_keys) > 1 and unsafe) return _build_report( default_key, - report_installed_keys, + installed_keys, findings, manifest_summaries, multi_install_safe, + manifest_checked_keys=check_installed_keys, + recorded_installed_keys=recorded_installed_keys, ) @@ -452,6 +457,9 @@ def _build_report( findings: list[dict[str, str]], manifests: dict[str, dict[str, Any]], multi_install_safe: bool, + *, + manifest_checked_keys: list[str] | None = None, + recorded_installed_keys: list[str] | None = None, ) -> dict[str, Any]: missing_count = sum(len(item.get("missing_files", [])) for item in manifests.values()) modified_count = sum(len(item.get("modified_files", [])) for item in manifests.values()) @@ -461,6 +469,12 @@ def _build_report( "status": _status(findings), "default_integration": default_key, "installed_integrations": installed_keys, + "recorded_installed_integrations": ( + installed_keys if recorded_installed_keys is None else recorded_installed_keys + ), + "manifest_checked_integrations": ( + installed_keys if manifest_checked_keys is None else manifest_checked_keys + ), "multi_install_safe": multi_install_safe, "shared_templates_target_alignment": default_key, "missing_managed_files": missing_count, diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index e99543816f..c9330b1d1a 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -2,6 +2,7 @@ import json import os +import shutil from pathlib import Path import pytest @@ -49,6 +50,32 @@ def _write_invalid_manifest(project, key): return manifest +def _copy_project_template(tmp_path, template): + project = tmp_path / "proj" + shutil.copytree(template, project) + return project + + +@pytest.fixture(scope="module") +def status_copilot_template(tmp_path_factory): + return _init_project(tmp_path_factory.mktemp("status-copilot"), "copilot") + + +@pytest.fixture(scope="module") +def status_claude_template(tmp_path_factory): + return _init_project(tmp_path_factory.mktemp("status-claude"), "claude") + + +@pytest.fixture +def copilot_project(tmp_path, status_copilot_template): + return _copy_project_template(tmp_path, status_copilot_template) + + +@pytest.fixture +def claude_project(tmp_path, status_claude_template): + return _copy_project_template(tmp_path, status_claude_template) + + def _integration_list_row_cells(output: str, key: str) -> list[str]: row = next(line for line in output.splitlines() if line.startswith(f"│ {key}")) return [cell.strip() for cell in row.split("│")[1:-1]] @@ -137,9 +164,8 @@ def test_status_requires_speckit_project(self, tmp_path, monkeypatch): assert result.exit_code != 0 assert "Not a spec-kit project" in result.output - def test_status_reports_healthy_project(self, tmp_path): - project = _init_project(tmp_path, "copilot") - result = _run_in_project(project, ["integration", "status"]) + def test_status_reports_healthy_project(self, copilot_project): + result = _run_in_project(copilot_project, ["integration", "status"]) assert result.exit_code == 0 assert "Integration status: OK" in result.output @@ -149,38 +175,37 @@ def test_status_reports_healthy_project(self, tmp_path): assert "Modified managed files: 0" in result.output assert "Missing managed files: 0" in result.output - def test_status_json_reports_healthy_project(self, tmp_path): - project = _init_project(tmp_path, "copilot") - result = _run_in_project(project, ["integration", "status", "--json"]) + def test_status_json_reports_healthy_project(self, copilot_project): + result = _run_in_project(copilot_project, ["integration", "status", "--json"]) assert result.exit_code == 0 payload = json.loads(result.output) assert payload["status"] == "ok" assert payload["default_integration"] == "copilot" assert payload["installed_integrations"] == ["copilot"] + assert payload["recorded_installed_integrations"] == ["copilot"] + assert payload["manifest_checked_integrations"] == ["copilot"] assert payload["shared_templates_target_alignment"] == "copilot" assert "shared_templates_aligned_to" not in payload assert payload["findings"] == [] - def test_status_reports_invalid_integration_json(self, tmp_path): - project = _init_project(tmp_path, "copilot") - (project / ".specify" / "integration.json").write_text("{", encoding="utf-8") + def test_status_reports_invalid_integration_json(self, copilot_project): + (copilot_project / ".specify" / "integration.json").write_text("{", encoding="utf-8") - result = _run_in_project(project, ["integration", "status"]) + result = _run_in_project(copilot_project, ["integration", "status"]) assert result.exit_code != 0 assert "integration-state-unreadable" in result.output assert "invalid JSON" in result.output assert "Traceback" not in result.output - def test_status_reports_supported_schema_for_newer_integration_state(self, tmp_path): - project = _init_project(tmp_path, "copilot") - state_path = project / ".specify" / "integration.json" + def test_status_reports_supported_schema_for_newer_integration_state(self, copilot_project): + state_path = copilot_project / ".specify" / "integration.json" state = json.loads(state_path.read_text(encoding="utf-8")) state["integration_state_schema"] = 99 state_path.write_text(json.dumps(state), encoding="utf-8") - result = _run_in_project(project, ["integration", "status", "--json"]) + result = _run_in_project(copilot_project, ["integration", "status", "--json"]) assert result.exit_code != 0 payload = json.loads(result.output) @@ -188,19 +213,17 @@ def test_status_reports_supported_schema_for_newer_integration_state(self, tmp_p assert "schema 99" in payload["findings"][0]["message"] assert "supported schema: 1" in payload["findings"][0]["message"] - def test_status_reports_missing_integration_json(self, tmp_path): - project = _init_project(tmp_path, "copilot") - (project / ".specify" / "integration.json").unlink() + def test_status_reports_missing_integration_json(self, copilot_project): + (copilot_project / ".specify" / "integration.json").unlink() - result = _run_in_project(project, ["integration", "status"]) + result = _run_in_project(copilot_project, ["integration", "status"]) assert result.exit_code != 0 assert "integration-state-missing" in result.output assert ".specify/integration.json is missing" in result.output - def test_status_json_reports_no_installed_integrations_as_warning(self, tmp_path): - project = _init_project(tmp_path, "copilot") - state_path = project / ".specify" / "integration.json" + def test_status_json_reports_no_installed_integrations_as_warning(self, copilot_project): + state_path = copilot_project / ".specify" / "integration.json" state_path.write_text( json.dumps({ "version": "test", @@ -210,7 +233,7 @@ def test_status_json_reports_no_installed_integrations_as_warning(self, tmp_path encoding="utf-8", ) - result = _run_in_project(project, ["integration", "status", "--json"]) + result = _run_in_project(copilot_project, ["integration", "status", "--json"]) assert result.exit_code == 0 payload = json.loads(result.output) @@ -220,9 +243,8 @@ def test_status_json_reports_no_installed_integrations_as_warning(self, tmp_path assert "speckit" in payload["manifests"] assert payload["manifests"]["speckit"]["readable"] is True - def test_status_checks_shared_manifest_when_no_integrations_installed(self, tmp_path): - project = _init_project(tmp_path, "copilot") - state_path = project / ".specify" / "integration.json" + def test_status_checks_shared_manifest_when_no_integrations_installed(self, copilot_project): + state_path = copilot_project / ".specify" / "integration.json" state_path.write_text( json.dumps({ "version": "test", @@ -231,9 +253,9 @@ def test_status_checks_shared_manifest_when_no_integrations_installed(self, tmp_ }), encoding="utf-8", ) - (project / ".specify" / "integrations" / "speckit.manifest.json").unlink() + (copilot_project / ".specify" / "integrations" / "speckit.manifest.json").unlink() - result = _run_in_project(project, ["integration", "status", "--json"]) + result = _run_in_project(copilot_project, ["integration", "status", "--json"]) assert result.exit_code != 0 payload = json.loads(result.output) @@ -250,16 +272,15 @@ def test_status_checks_shared_manifest_when_no_integrations_installed(self, tmp_ for item in payload["findings"] ) - def test_status_json_reports_missing_default_integration_as_error(self, tmp_path): - project = _init_project(tmp_path, "claude") - state_path = project / ".specify" / "integration.json" + def test_status_json_reports_missing_default_integration_as_error(self, claude_project): + state_path = claude_project / ".specify" / "integration.json" state = json.loads(state_path.read_text(encoding="utf-8")) state.pop("default_integration", None) state.pop("integration", None) state["installed_integrations"] = ["claude"] state_path.write_text(json.dumps(state), encoding="utf-8") - result = _run_in_project(project, ["integration", "status", "--json"]) + result = _run_in_project(claude_project, ["integration", "status", "--json"]) assert result.exit_code != 0 payload = json.loads(result.output) @@ -270,21 +291,22 @@ def test_status_json_reports_missing_default_integration_as_error(self, tmp_path for item in payload["findings"] ) - def test_status_reports_default_integration_not_installed(self, tmp_path): - project = _init_project(tmp_path, "claude") - state_path = project / ".specify" / "integration.json" + def test_status_reports_default_integration_not_installed(self, claude_project): + state_path = claude_project / ".specify" / "integration.json" state = json.loads(state_path.read_text(encoding="utf-8")) state["default_integration"] = "codex" state["integration"] = "codex" state["installed_integrations"] = ["claude"] state_path.write_text(json.dumps(state), encoding="utf-8") - result = _run_in_project(project, ["integration", "status", "--json"]) + result = _run_in_project(claude_project, ["integration", "status", "--json"]) assert result.exit_code != 0 payload = json.loads(result.output) assert payload["default_integration"] == "codex" - assert payload["installed_integrations"] == ["claude"] + assert payload["installed_integrations"] == ["codex", "claude"] + assert payload["recorded_installed_integrations"] == ["claude"] + assert payload["manifest_checked_integrations"] == ["claude"] assert any( item["code"] == "default-integration-not-installed" and "Default integration 'codex' is not listed" in item["message"] @@ -296,21 +318,38 @@ def test_status_reports_default_integration_not_installed(self, tmp_path): for item in payload["findings"] ) - def test_status_reports_missing_manifest(self, tmp_path): - project = _init_project(tmp_path, "copilot") - (project / ".specify" / "integrations" / "copilot.manifest.json").unlink() + def test_status_checks_effective_default_manifest_when_raw_installed_is_empty(self, claude_project): + state_path = claude_project / ".specify" / "integration.json" + state = json.loads(state_path.read_text(encoding="utf-8")) + state["installed_integrations"] = [] + state_path.write_text(json.dumps(state), encoding="utf-8") + + result = _run_in_project(claude_project, ["integration", "status", "--json"]) + + assert result.exit_code != 0 + payload = json.loads(result.output) + assert payload["installed_integrations"] == ["claude"] + assert payload["recorded_installed_integrations"] == [] + assert payload["manifest_checked_integrations"] == ["claude"] + assert payload["manifests"]["claude"]["readable"] is True + assert any( + item["code"] == "default-integration-not-installed" + for item in payload["findings"] + ) + + def test_status_reports_missing_manifest(self, copilot_project): + (copilot_project / ".specify" / "integrations" / "copilot.manifest.json").unlink() - result = _run_in_project(project, ["integration", "status"]) + result = _run_in_project(copilot_project, ["integration", "status"]) assert result.exit_code != 0 assert "manifest-missing" in result.output assert "Manifest for integration 'copilot' is missing" in result.output - def test_status_reports_unreadable_manifest_in_json_summary(self, tmp_path): - project = _init_project(tmp_path, "copilot") - _write_invalid_manifest(project, "copilot") + def test_status_reports_unreadable_manifest_in_json_summary(self, copilot_project): + _write_invalid_manifest(copilot_project, "copilot") - result = _run_in_project(project, ["integration", "status", "--json"]) + result = _run_in_project(copilot_project, ["integration", "status", "--json"]) assert result.exit_code != 0 payload = json.loads(result.output) @@ -319,40 +358,37 @@ def test_status_reports_unreadable_manifest_in_json_summary(self, tmp_path): assert payload["manifests"]["copilot"]["missing_files"] == [] assert payload["manifests"]["copilot"]["modified_files"] == [] - def test_status_reports_modified_managed_files_without_failing(self, tmp_path): - project = _init_project(tmp_path, "copilot") - manifest_path = project / ".specify" / "integrations" / "copilot.manifest.json" + def test_status_reports_modified_managed_files_without_failing(self, copilot_project): + manifest_path = copilot_project / ".specify" / "integrations" / "copilot.manifest.json" tracked_files = json.loads(manifest_path.read_text(encoding="utf-8"))["files"] first_rel = next(iter(tracked_files)) - (project / first_rel).write_text("MODIFIED CONTENT\n", encoding="utf-8") + (copilot_project / first_rel).write_text("MODIFIED CONTENT\n", encoding="utf-8") - result = _run_in_project(project, ["integration", "status"]) + result = _run_in_project(copilot_project, ["integration", "status"]) assert result.exit_code == 0 assert "Integration status: WARNING" in result.output assert "managed-files-modified" in result.output assert "Modified managed files: 1" in result.output - def test_status_reports_missing_managed_files(self, tmp_path): - project = _init_project(tmp_path, "copilot") - manifest_path = project / ".specify" / "integrations" / "copilot.manifest.json" + def test_status_reports_missing_managed_files(self, copilot_project): + manifest_path = copilot_project / ".specify" / "integrations" / "copilot.manifest.json" tracked_files = json.loads(manifest_path.read_text(encoding="utf-8"))["files"] first_rel = next(iter(tracked_files)) - (project / first_rel).unlink() + (copilot_project / first_rel).unlink() - result = _run_in_project(project, ["integration", "status"]) + result = _run_in_project(copilot_project, ["integration", "status"]) assert result.exit_code != 0 assert "managed-files-missing" in result.output assert "Missing managed files: 1" in result.output - def test_status_reports_missing_shared_managed_files(self, tmp_path): - project = _init_project(tmp_path, "copilot") - shared_file = project / ".specify" / "scripts" / "bash" / "common.sh" + def test_status_reports_missing_shared_managed_files(self, copilot_project): + shared_file = copilot_project / ".specify" / "scripts" / "bash" / "common.sh" assert shared_file.exists() shared_file.unlink() - result = _run_in_project(project, ["integration", "status"]) + result = _run_in_project(copilot_project, ["integration", "status"]) assert result.exit_code != 0 assert "managed-files-missing" in result.output @@ -385,42 +421,40 @@ def fail_exists(self): assert invalid == [] assert valid == ["tracked.md"] - def test_status_treats_dangling_symlink_as_missing(self, tmp_path): - project = _init_project(tmp_path, "copilot") - manifest_path = project / ".specify" / "integrations" / "copilot.manifest.json" + def test_status_treats_dangling_symlink_as_missing(self, copilot_project): + manifest_path = copilot_project / ".specify" / "integrations" / "copilot.manifest.json" tracked_files = json.loads(manifest_path.read_text(encoding="utf-8"))["files"] first_rel = next(iter(tracked_files)) - target = project / first_rel + target = copilot_project / first_rel target.unlink() try: - target.symlink_to(project / "missing-target") + target.symlink_to(copilot_project / "missing-target") except OSError as exc: pytest.skip(f"symlinks unavailable: {exc}") - result = _run_in_project(project, ["integration", "status", "--json"]) + result = _run_in_project(copilot_project, ["integration", "status", "--json"]) assert result.exit_code != 0 payload = json.loads(result.output) assert first_rel in payload["manifests"]["copilot"]["missing_files"] assert first_rel not in payload["manifests"]["copilot"]["modified_files"] - def test_status_reports_unsafe_manifest_paths_without_hashing_them(self, tmp_path): - project = _init_project(tmp_path, "copilot") + def test_status_reports_unsafe_manifest_paths_without_hashing_them(self, tmp_path, copilot_project): outside = tmp_path / "outside" outside.mkdir() (outside / "secret.txt").write_text("outside project\n", encoding="utf-8") - link = project / "outside-link" + link = copilot_project / "outside-link" try: link.symlink_to(outside, target_is_directory=True) except OSError as exc: pytest.skip(f"symlinks unavailable: {exc}") - manifest_path = project / ".specify" / "integrations" / "copilot.manifest.json" + manifest_path = copilot_project / ".specify" / "integrations" / "copilot.manifest.json" manifest_data = json.loads(manifest_path.read_text(encoding="utf-8")) manifest_data["files"]["outside-link/secret.txt"] = "wrong" manifest_path.write_text(json.dumps(manifest_data), encoding="utf-8") - result = _run_in_project(project, ["integration", "status", "--json"]) + result = _run_in_project(copilot_project, ["integration", "status", "--json"]) assert result.exit_code != 0 payload = json.loads(result.output) @@ -428,53 +462,50 @@ def test_status_reports_unsafe_manifest_paths_without_hashing_them(self, tmp_pat assert "outside-link/secret.txt" in payload["manifests"]["copilot"]["invalid_files"] assert "outside-link/secret.txt" not in payload["manifests"]["copilot"]["modified_files"] - def test_status_reports_unsafe_multi_install_combination(self, tmp_path): + def test_status_reports_unsafe_multi_install_combination(self, copilot_project): from specify_cli.integrations.manifest import IntegrationManifest - project = _init_project(tmp_path, "copilot") - state_path = project / ".specify" / "integration.json" + state_path = copilot_project / ".specify" / "integration.json" state = json.loads(state_path.read_text(encoding="utf-8")) state["installed_integrations"] = ["copilot", "claude"] state["default_integration"] = "copilot" state["integration"] = "copilot" state_path.write_text(json.dumps(state), encoding="utf-8") - IntegrationManifest("claude", project, version="test").save() + IntegrationManifest("claude", copilot_project, version="test").save() - result = _run_in_project(project, ["integration", "status"]) + result = _run_in_project(copilot_project, ["integration", "status"]) assert result.exit_code != 0 assert "unsafe-multi-install" in result.output assert "Multi-install safe: no" in result.output - def test_status_treats_unknown_multi_install_as_unsafe(self, tmp_path): + def test_status_treats_unknown_multi_install_as_unsafe(self, claude_project): from specify_cli.integrations.manifest import IntegrationManifest - project = _init_project(tmp_path, "claude") - state_path = project / ".specify" / "integration.json" + state_path = claude_project / ".specify" / "integration.json" state = json.loads(state_path.read_text(encoding="utf-8")) state["installed_integrations"] = ["claude", "mystery"] state["default_integration"] = "claude" state["integration"] = "claude" state_path.write_text(json.dumps(state), encoding="utf-8") - IntegrationManifest("mystery", project, version="test").save() + IntegrationManifest("mystery", claude_project, version="test").save() - result = _run_in_project(project, ["integration", "status"]) + result = _run_in_project(claude_project, ["integration", "status"]) assert result.exit_code != 0 assert "unknown-integration" in result.output assert "unsafe-multi-install" in result.output assert "Multi-install safe: no" in result.output - def test_status_gives_actionable_suggestion_for_unknown_manifest(self, tmp_path): - project = _init_project(tmp_path, "claude") - state_path = project / ".specify" / "integration.json" + def test_status_gives_actionable_suggestion_for_unknown_manifest(self, claude_project): + state_path = claude_project / ".specify" / "integration.json" state = json.loads(state_path.read_text(encoding="utf-8")) state["installed_integrations"] = ["mystery"] state["default_integration"] = "mystery" state["integration"] = "mystery" state_path.write_text(json.dumps(state), encoding="utf-8") - result = _run_in_project(project, ["integration", "status", "--json"]) + result = _run_in_project(claude_project, ["integration", "status", "--json"]) assert result.exit_code != 0 payload = json.loads(result.output) @@ -485,9 +516,8 @@ def test_status_gives_actionable_suggestion_for_unknown_manifest(self, tmp_path) assert "remove the stale integration metadata" in manifest_finding["suggestion"] assert "integration upgrade mystery" not in manifest_finding["suggestion"] - def test_status_rejects_unsafe_integration_keys_before_manifest_lookup(self, tmp_path): - project = _init_project(tmp_path, "claude") - state_path = project / ".specify" / "integration.json" + def test_status_rejects_unsafe_integration_keys_before_manifest_lookup(self, tmp_path, claude_project): + state_path = claude_project / ".specify" / "integration.json" unsafe_key = "../../../escape" state_path.write_text( json.dumps({ @@ -503,7 +533,7 @@ def test_status_rejects_unsafe_integration_keys_before_manifest_lookup(self, tmp encoding="utf-8", ) - result = _run_in_project(project, ["integration", "status", "--json"]) + result = _run_in_project(claude_project, ["integration", "status", "--json"]) assert result.exit_code != 0 payload = json.loads(result.output) @@ -514,9 +544,8 @@ def test_status_rejects_unsafe_integration_keys_before_manifest_lookup(self, tmp for item in payload["findings"] ) - def test_status_rejects_filename_invalid_integration_keys(self, tmp_path): - project = _init_project(tmp_path, "claude") - state_path = project / ".specify" / "integration.json" + def test_status_rejects_filename_invalid_integration_keys(self, claude_project): + state_path = claude_project / ".specify" / "integration.json" unsafe_key = "bad:key" state_path.write_text( json.dumps({ @@ -527,7 +556,7 @@ def test_status_rejects_filename_invalid_integration_keys(self, tmp_path): encoding="utf-8", ) - result = _run_in_project(project, ["integration", "status", "--json"]) + result = _run_in_project(claude_project, ["integration", "status", "--json"]) assert result.exit_code != 0 payload = json.loads(result.output) @@ -537,9 +566,8 @@ def test_status_rejects_filename_invalid_integration_keys(self, tmp_path): for item in payload["findings"] ) - def test_status_rejects_windows_reserved_integration_keys(self, tmp_path): - project = _init_project(tmp_path, "claude") - state_path = project / ".specify" / "integration.json" + def test_status_rejects_windows_reserved_integration_keys(self, claude_project): + state_path = claude_project / ".specify" / "integration.json" unsafe_key = "CON" state_path.write_text( json.dumps({ @@ -550,7 +578,7 @@ def test_status_rejects_windows_reserved_integration_keys(self, tmp_path): encoding="utf-8", ) - result = _run_in_project(project, ["integration", "status", "--json"]) + result = _run_in_project(claude_project, ["integration", "status", "--json"]) assert result.exit_code != 0 payload = json.loads(result.output) @@ -560,25 +588,24 @@ def test_status_rejects_windows_reserved_integration_keys(self, tmp_path): for item in payload["findings"] ) - def test_status_reports_managed_file_collisions(self, tmp_path): + def test_status_reports_managed_file_collisions(self, claude_project): from specify_cli.integrations.manifest import IntegrationManifest - project = _init_project(tmp_path, "claude") - state_path = project / ".specify" / "integration.json" + state_path = claude_project / ".specify" / "integration.json" state = json.loads(state_path.read_text(encoding="utf-8")) state["installed_integrations"] = ["claude", "codex"] state["default_integration"] = "claude" state["integration"] = "claude" state_path.write_text(json.dumps(state), encoding="utf-8") - claude_manifest = project / ".specify" / "integrations" / "claude.manifest.json" + claude_manifest = claude_project / ".specify" / "integrations" / "claude.manifest.json" tracked_files = json.loads(claude_manifest.read_text(encoding="utf-8"))["files"] shared_rel = next(iter(tracked_files)) - codex_manifest = IntegrationManifest("codex", project, version="test") + codex_manifest = IntegrationManifest("codex", claude_project, version="test") codex_manifest.record_existing(shared_rel) codex_manifest.save() - result = _run_in_project(project, ["integration", "status"]) + result = _run_in_project(claude_project, ["integration", "status"]) assert result.exit_code == 0 assert "managed-file-collision" in result.output