From 0e55a1e61202f39c2e0c2f789c26e5bf11151653 Mon Sep 17 00:00:00 2001 From: Pascal Date: Fri, 22 May 2026 09:19:00 +0200 Subject: [PATCH 1/3] 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 2/3] 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 3/3] 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