From 0e55a1e61202f39c2e0c2f789c26e5bf11151653 Mon Sep 17 00:00:00 2001 From: Pascal Date: Fri, 22 May 2026 09:19:00 +0200 Subject: [PATCH 01/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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 From 9dedcf43ecb45000a2f9b60039411718dc873807 Mon Sep 17 00:00:00 2001 From: Pascal Date: Thu, 28 May 2026 16:26:01 +0200 Subject: [PATCH 12/21] fix(integration): report unknown multi-install safety --- docs/reference/integrations.md | 4 ++- src/specify_cli/__init__.py | 7 +++- src/specify_cli/integration_status.py | 6 ++-- .../test_integration_subcommand.py | 33 +++++++++++++++++++ 4 files changed, 45 insertions(+), 5 deletions(-) diff --git a/docs/reference/integrations.md b/docs/reference/integrations.md index 9e60583145..978a91bd3c 100644 --- a/docs/reference/integrations.md +++ b/docs/reference/integrations.md @@ -139,7 +139,9 @@ 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`. +only when the report status is `error`. In JSON output, `multi_install_safe` +is `null` when the integration state is missing or unreadable because safety +could not be evaluated. ## Integration-Specific Options diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 8f46311257..c09470ce53 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1631,7 +1631,12 @@ def _print_integration_status_report(report: dict[str, Any]) -> None: 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'}") + multi_install_safe = report.get("multi_install_safe") + if multi_install_safe is None: + multi_install_safe_display = "unknown" + else: + multi_install_safe_display = "yes" if multi_install_safe else "no" + console.print(f"Multi-install safe: {multi_install_safe_display}") console.print( f"Shared templates target alignment: " f"{_rich_escape(str(report.get('shared_templates_target_alignment') or 'none'))}" diff --git a/src/specify_cli/integration_status.py b/src/specify_cli/integration_status.py index dfb9fad76c..cd796f95fb 100644 --- a/src/specify_cli/integration_status.py +++ b/src/specify_cli/integration_status.py @@ -224,7 +224,7 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: suggestion=f"Fix or delete {INTEGRATION_JSON}, then retry.", ) ) - return _build_report(None, [], findings, {}, True) + return _build_report(None, [], findings, {}, None) if state is None: findings.append( @@ -236,7 +236,7 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: suggestion="Run `specify integration install ` to install an integration.", ) ) - return _build_report(None, [], findings, {}, True) + return _build_report(None, [], findings, {}, None) assert raw_state is not None raw_default_key = default_integration_key(raw_state) @@ -456,7 +456,7 @@ def _build_report( installed_keys: list[str], findings: list[dict[str, str]], manifests: dict[str, dict[str, Any]], - multi_install_safe: bool, + multi_install_safe: bool | None, *, manifest_checked_keys: list[str] | None = None, recorded_installed_keys: list[str] | None = None, diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index c9330b1d1a..61c2767ecd 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -185,6 +185,7 @@ def test_status_json_reports_healthy_project(self, copilot_project): assert payload["installed_integrations"] == ["copilot"] assert payload["recorded_installed_integrations"] == ["copilot"] assert payload["manifest_checked_integrations"] == ["copilot"] + assert payload["multi_install_safe"] is True assert payload["shared_templates_target_alignment"] == "copilot" assert "shared_templates_aligned_to" not in payload assert payload["findings"] == [] @@ -197,8 +198,24 @@ def test_status_reports_invalid_integration_json(self, copilot_project): assert result.exit_code != 0 assert "integration-state-unreadable" in result.output assert "invalid JSON" in result.output + assert "Multi-install safe: unknown" in result.output assert "Traceback" not in result.output + def test_status_json_reports_unknown_multi_install_safety_when_state_unreadable( + self, + copilot_project, + ): + (copilot_project / ".specify" / "integration.json").write_text("{", encoding="utf-8") + + result = _run_in_project(copilot_project, ["integration", "status", "--json"]) + + assert result.exit_code != 0 + payload = json.loads(result.output) + assert payload["status"] == "error" + assert payload["multi_install_safe"] is None + assert payload["manifest_checked_integrations"] == [] + assert payload["findings"][0]["code"] == "integration-state-unreadable" + 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")) @@ -221,6 +238,22 @@ def test_status_reports_missing_integration_json(self, copilot_project): assert result.exit_code != 0 assert "integration-state-missing" in result.output assert ".specify/integration.json is missing" in result.output + assert "Multi-install safe: unknown" in result.output + + def test_status_json_reports_unknown_multi_install_safety_when_state_missing( + self, + copilot_project, + ): + (copilot_project / ".specify" / "integration.json").unlink() + + result = _run_in_project(copilot_project, ["integration", "status", "--json"]) + + assert result.exit_code != 0 + payload = json.loads(result.output) + assert payload["status"] == "error" + assert payload["multi_install_safe"] is None + assert payload["manifest_checked_integrations"] == [] + assert payload["findings"][0]["code"] == "integration-state-missing" def test_status_json_reports_no_installed_integrations_as_warning(self, copilot_project): state_path = copilot_project / ".specify" / "integration.json" From 1596fe2cb4921136b92eb9cab32e39d52249a0df Mon Sep 17 00:00:00 2001 From: Pascal Date: Thu, 28 May 2026 16:34:24 +0200 Subject: [PATCH 13/21] fix(integration): mark empty safety as unknown --- docs/reference/integrations.md | 4 ++-- src/specify_cli/integration_status.py | 2 +- tests/integrations/test_integration_subcommand.py | 1 + 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/reference/integrations.md b/docs/reference/integrations.md index 978a91bd3c..3773bd536a 100644 --- a/docs/reference/integrations.md +++ b/docs/reference/integrations.md @@ -140,8 +140,8 @@ 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`. In JSON output, `multi_install_safe` -is `null` when the integration state is missing or unreadable because safety -could not be evaluated. +is `null` when no installed integration set can be evaluated, such as when the +integration state is missing, unreadable, or records no installed integrations. ## Integration-Specific Options diff --git a/src/specify_cli/integration_status.py b/src/specify_cli/integration_status.py index cd796f95fb..cfe7b6b5ce 100644 --- a/src/specify_cli/integration_status.py +++ b/src/specify_cli/integration_status.py @@ -439,7 +439,7 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: ) ) - multi_install_safe = not (len(check_installed_keys) > 1 and unsafe) + multi_install_safe = None if not check_installed_keys else not (len(check_installed_keys) > 1 and unsafe) return _build_report( default_key, installed_keys, diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index 61c2767ecd..35a10c2c20 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -272,6 +272,7 @@ def test_status_json_reports_no_installed_integrations_as_warning(self, copilot_ payload = json.loads(result.output) assert payload["status"] == "warning" assert payload["installed_integrations"] == [] + assert payload["multi_install_safe"] is None assert payload["findings"][0]["code"] == "no-installed-integrations" assert "speckit" in payload["manifests"] assert payload["manifests"]["speckit"]["readable"] is True From 934b15279ec89669562e5e814d6ff35f3340e293 Mon Sep 17 00:00:00 2001 From: Pascal Date: Thu, 28 May 2026 16:40:40 +0200 Subject: [PATCH 14/21] fix(integration): ignore invalid raw installed list --- src/specify_cli/integration_state.py | 7 +++---- src/specify_cli/integration_status.py | 10 ++++++--- .../test_integration_subcommand.py | 21 +++++++++++++++++++ 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/specify_cli/integration_state.py b/src/specify_cli/integration_state.py index ca53dff3a4..74dc6aa7de 100644 --- a/src/specify_cli/integration_state.py +++ b/src/specify_cli/integration_state.py @@ -77,10 +77,9 @@ def try_read_integration_json( 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. + failure. This helper delegates file I/O and raw JSON validation to + ``_read_integration_json_data`` so callers that need raw state can share + the same low-level reader instead of duplicating parse logic. """ data, error = _read_integration_json_data(project_root) if data is None: diff --git a/src/specify_cli/integration_status.py b/src/specify_cli/integration_status.py index cfe7b6b5ce..1bc075071f 100644 --- a/src/specify_cli/integration_status.py +++ b/src/specify_cli/integration_status.py @@ -19,7 +19,7 @@ from .integrations import INTEGRATION_REGISTRY from .integrations.manifest import IntegrationManifest -_MANIFEST_READ_ERRORS = (ValueError, FileNotFoundError, OSError, UnicodeDecodeError) +_MANIFEST_READ_ERRORS = (ValueError, OSError, UnicodeDecodeError) _MANIFEST_KEY_RE = re.compile(r"^[A-Za-z0-9._-]+$") _WINDOWS_RESERVED_MANIFEST_BASENAMES = { "CON", @@ -240,10 +240,14 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: assert raw_state is not None raw_default_key = default_integration_key(raw_state) - raw_installed_keys = installed_integration_keys(raw_state) + raw_installed_is_list = isinstance(raw_state.get("installed_integrations"), list) + raw_installed_keys = ( + installed_integration_keys(raw_state) + if raw_installed_is_list + else [] + ) 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) 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 diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index 35a10c2c20..b3d221729b 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -325,6 +325,27 @@ def test_status_json_reports_missing_default_integration_as_error(self, claude_p for item in payload["findings"] ) + def test_status_ignores_non_list_raw_installed_integrations(self, copilot_project): + state_path = copilot_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"] = "copilot" + state_path.write_text(json.dumps(state), encoding="utf-8") + + result = _run_in_project(copilot_project, ["integration", "status", "--json"]) + + assert result.exit_code == 0 + payload = json.loads(result.output) + assert payload["status"] == "warning" + assert payload["installed_integrations"] == [] + assert payload["recorded_installed_integrations"] == [] + assert payload["manifest_checked_integrations"] == [] + assert payload["multi_install_safe"] is None + assert [item["code"] for item in payload["findings"]] == [ + "no-installed-integrations", + ] + 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")) From c8d9faac8c3c204c0d2eb9ef89f538e977237ebb Mon Sep 17 00:00:00 2001 From: Pascal Date: Thu, 28 May 2026 18:19:59 +0200 Subject: [PATCH 15/21] fix(integration): report actual manifest checks --- src/specify_cli/integration_status.py | 17 ++++++++++++++--- .../integrations/test_integration_subcommand.py | 13 +++++++++---- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/specify_cli/integration_status.py b/src/specify_cli/integration_status.py index 1bc075071f..50d3638565 100644 --- a/src/specify_cli/integration_status.py +++ b/src/specify_cli/integration_status.py @@ -63,11 +63,20 @@ def _status(findings: list[dict[str, str]]) -> str: return "ok" +def _with_error_detail(message: str, error: IntegrationReadError) -> str: + if error.detail: + return f"{message} Detail: {error.detail}" + return message + + 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." + return _with_error_detail( + f"{INTEGRATION_JSON} contains invalid JSON or is not valid UTF-8.", + error, + ) if error.kind == "os": - return f"Could not read {INTEGRATION_JSON}." + return _with_error_detail(f"Could not read {INTEGRATION_JSON}.", error) if error.kind == "not_object": return f"{INTEGRATION_JSON} must contain a JSON object, got {error.detail}." if error.kind == "schema_too_new": @@ -326,6 +335,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]] = {} + attempted_manifest_keys: list[str] = [] manifest_keys = list(check_installed_keys) if _SHARED_MANIFEST_KEY not in manifest_keys: manifest_keys.append(_SHARED_MANIFEST_KEY) @@ -345,6 +355,7 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: ) continue + attempted_manifest_keys.append(key) manifest_path = project_root / ".specify" / "integrations" / f"{key}.manifest.json" try: manifest = IntegrationManifest.load(key, project_root) @@ -450,7 +461,7 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: findings, manifest_summaries, multi_install_safe, - manifest_checked_keys=check_installed_keys, + manifest_checked_keys=attempted_manifest_keys, recorded_installed_keys=recorded_installed_keys, ) diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index b3d221729b..ed4ffe1ad1 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -184,7 +184,7 @@ def test_status_json_reports_healthy_project(self, copilot_project): assert payload["default_integration"] == "copilot" assert payload["installed_integrations"] == ["copilot"] assert payload["recorded_installed_integrations"] == ["copilot"] - assert payload["manifest_checked_integrations"] == ["copilot"] + assert payload["manifest_checked_integrations"] == ["copilot", "speckit"] assert payload["multi_install_safe"] is True assert payload["shared_templates_target_alignment"] == "copilot" assert "shared_templates_aligned_to" not in payload @@ -198,6 +198,7 @@ def test_status_reports_invalid_integration_json(self, copilot_project): assert result.exit_code != 0 assert "integration-state-unreadable" in result.output assert "invalid JSON" in result.output + assert "Expecting property name" in result.output assert "Multi-install safe: unknown" in result.output assert "Traceback" not in result.output @@ -215,6 +216,7 @@ def test_status_json_reports_unknown_multi_install_safety_when_state_unreadable( assert payload["multi_install_safe"] is None assert payload["manifest_checked_integrations"] == [] assert payload["findings"][0]["code"] == "integration-state-unreadable" + assert "Expecting property name" in payload["findings"][0]["message"] def test_status_reports_supported_schema_for_newer_integration_state(self, copilot_project): state_path = copilot_project / ".specify" / "integration.json" @@ -273,6 +275,7 @@ def test_status_json_reports_no_installed_integrations_as_warning(self, copilot_ assert payload["status"] == "warning" assert payload["installed_integrations"] == [] assert payload["multi_install_safe"] is None + assert payload["manifest_checked_integrations"] == ["speckit"] assert payload["findings"][0]["code"] == "no-installed-integrations" assert "speckit" in payload["manifests"] assert payload["manifests"]["speckit"]["readable"] is True @@ -295,6 +298,7 @@ def test_status_checks_shared_manifest_when_no_integrations_installed(self, copi payload = json.loads(result.output) assert payload["status"] == "error" assert payload["installed_integrations"] == [] + assert payload["manifest_checked_integrations"] == ["speckit"] assert payload["unchecked_manifests"] == 1 assert any( item["code"] == "no-installed-integrations" @@ -340,7 +344,7 @@ def test_status_ignores_non_list_raw_installed_integrations(self, copilot_projec assert payload["status"] == "warning" assert payload["installed_integrations"] == [] assert payload["recorded_installed_integrations"] == [] - assert payload["manifest_checked_integrations"] == [] + assert payload["manifest_checked_integrations"] == ["speckit"] assert payload["multi_install_safe"] is None assert [item["code"] for item in payload["findings"]] == [ "no-installed-integrations", @@ -361,7 +365,7 @@ def test_status_reports_default_integration_not_installed(self, claude_project): assert payload["default_integration"] == "codex" assert payload["installed_integrations"] == ["codex", "claude"] assert payload["recorded_installed_integrations"] == ["claude"] - assert payload["manifest_checked_integrations"] == ["claude"] + assert payload["manifest_checked_integrations"] == ["claude", "speckit"] assert any( item["code"] == "default-integration-not-installed" and "Default integration 'codex' is not listed" in item["message"] @@ -385,7 +389,7 @@ def test_status_checks_effective_default_manifest_when_raw_installed_is_empty(se payload = json.loads(result.output) assert payload["installed_integrations"] == ["claude"] assert payload["recorded_installed_integrations"] == [] - assert payload["manifest_checked_integrations"] == ["claude"] + assert payload["manifest_checked_integrations"] == ["claude", "speckit"] assert payload["manifests"]["claude"]["readable"] is True assert any( item["code"] == "default-integration-not-installed" @@ -593,6 +597,7 @@ def test_status_rejects_unsafe_integration_keys_before_manifest_lookup(self, tmp assert result.exit_code != 0 payload = json.loads(result.output) assert unsafe_key not in payload["manifests"] + assert payload["manifest_checked_integrations"] == ["speckit"] assert any( item["code"] == "integration-key-invalid" and item["integration"] == unsafe_key From 646ab9d462d8ad794649496ba6673ba5db07a98d Mon Sep 17 00:00:00 2001 From: Pascal Date: Thu, 28 May 2026 18:26:09 +0200 Subject: [PATCH 16/21] fix(integration): tighten status contract invariants --- docs/reference/integrations.md | 3 ++- src/specify_cli/integration_status.py | 25 ++++++++++++++--- .../test_integration_subcommand.py | 27 +++++++++++++++++-- 3 files changed, 48 insertions(+), 7 deletions(-) diff --git a/docs/reference/integrations.md b/docs/reference/integrations.md index 3773bd536a..9638856dc0 100644 --- a/docs/reference/integrations.md +++ b/docs/reference/integrations.md @@ -141,7 +141,8 @@ 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`. In JSON output, `multi_install_safe` is `null` when no installed integration set can be evaluated, such as when the -integration state is missing, unreadable, or records no installed integrations. +integration state is missing, unreadable, lacks a valid recorded integration +list, or records no installed integrations. ## Integration-Specific Options diff --git a/src/specify_cli/integration_status.py b/src/specify_cli/integration_status.py index 50d3638565..6ce990aebd 100644 --- a/src/specify_cli/integration_status.py +++ b/src/specify_cli/integration_status.py @@ -19,7 +19,7 @@ from .integrations import INTEGRATION_REGISTRY from .integrations.manifest import IntegrationManifest -_MANIFEST_READ_ERRORS = (ValueError, OSError, UnicodeDecodeError) +_MANIFEST_READ_ERRORS = (ValueError, OSError) _MANIFEST_KEY_RE = re.compile(r"^[A-Za-z0-9._-]+$") _WINDOWS_RESERVED_MANIFEST_BASENAMES = { "CON", @@ -249,7 +249,8 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: assert raw_state is not None raw_default_key = default_integration_key(raw_state) - raw_installed_is_list = isinstance(raw_state.get("installed_integrations"), list) + raw_installed_value = raw_state.get("installed_integrations") + raw_installed_is_list = isinstance(raw_installed_value, list) raw_installed_keys = ( installed_integration_keys(raw_state) if raw_installed_is_list @@ -262,7 +263,20 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: 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 + recorded_installed_keys = raw_installed_keys + if "installed_integrations" in raw_state and not raw_installed_is_list: + findings.append( + _finding( + "warning", + "installed-integrations-invalid", + ( + "installed_integrations must be a list, " + f"got {type(raw_installed_value).__name__}." + ), + path=INTEGRATION_JSON, + suggestion=f"Fix {INTEGRATION_JSON}, then retry.", + ) + ) if not installed_keys: findings.append( _finding( @@ -454,7 +468,10 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: ) ) - multi_install_safe = None if not check_installed_keys else not (len(check_installed_keys) > 1 and unsafe) + if not raw_installed_is_list or not raw_installed_keys: + multi_install_safe = None + else: + multi_install_safe = not (len(check_installed_keys) > 1 and unsafe) return _build_report( default_key, installed_keys, diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index ed4ffe1ad1..f08f86bc42 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -198,7 +198,7 @@ def test_status_reports_invalid_integration_json(self, copilot_project): assert result.exit_code != 0 assert "integration-state-unreadable" in result.output assert "invalid JSON" in result.output - assert "Expecting property name" in result.output + assert "Detail:" in result.output assert "Multi-install safe: unknown" in result.output assert "Traceback" not in result.output @@ -216,7 +216,7 @@ def test_status_json_reports_unknown_multi_install_safety_when_state_unreadable( assert payload["multi_install_safe"] is None assert payload["manifest_checked_integrations"] == [] assert payload["findings"][0]["code"] == "integration-state-unreadable" - assert "Expecting property name" in payload["findings"][0]["message"] + assert "Detail:" in payload["findings"][0]["message"] def test_status_reports_supported_schema_for_newer_integration_state(self, copilot_project): state_path = copilot_project / ".specify" / "integration.json" @@ -347,9 +347,31 @@ def test_status_ignores_non_list_raw_installed_integrations(self, copilot_projec assert payload["manifest_checked_integrations"] == ["speckit"] assert payload["multi_install_safe"] is None assert [item["code"] for item in payload["findings"]] == [ + "installed-integrations-invalid", "no-installed-integrations", ] + def test_status_reports_non_list_raw_installed_integrations_with_default(self, copilot_project): + state_path = copilot_project / ".specify" / "integration.json" + state = json.loads(state_path.read_text(encoding="utf-8")) + state["default_integration"] = "copilot" + state["integration"] = "copilot" + state["installed_integrations"] = "copilot" + state_path.write_text(json.dumps(state), encoding="utf-8") + + result = _run_in_project(copilot_project, ["integration", "status", "--json"]) + + assert result.exit_code == 0 + payload = json.loads(result.output) + assert payload["status"] == "warning" + assert payload["installed_integrations"] == ["copilot"] + assert payload["recorded_installed_integrations"] == [] + assert payload["manifest_checked_integrations"] == ["copilot", "speckit"] + assert payload["multi_install_safe"] is None + assert [item["code"] for item in payload["findings"]] == [ + "installed-integrations-invalid", + ] + 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")) @@ -390,6 +412,7 @@ def test_status_checks_effective_default_manifest_when_raw_installed_is_empty(se assert payload["installed_integrations"] == ["claude"] assert payload["recorded_installed_integrations"] == [] assert payload["manifest_checked_integrations"] == ["claude", "speckit"] + assert payload["multi_install_safe"] is None assert payload["manifests"]["claude"]["readable"] is True assert any( item["code"] == "default-integration-not-installed" From 982e8a62fd067e471e0aff2cc6345fffe507212d Mon Sep 17 00:00:00 2001 From: Pascal Date: Thu, 28 May 2026 18:40:18 +0200 Subject: [PATCH 17/21] fix(integration): harden manifest status paths --- src/specify_cli/integration_status.py | 34 +++++++++-- .../test_integration_subcommand.py | 57 ++++++++++++++++++- 2 files changed, 86 insertions(+), 5 deletions(-) diff --git a/src/specify_cli/integration_status.py b/src/specify_cli/integration_status.py index 6ce990aebd..0eb32c7161 100644 --- a/src/specify_cli/integration_status.py +++ b/src/specify_cli/integration_status.py @@ -106,11 +106,25 @@ def _safe_manifest_file( candidate = project_root / rel_path try: candidate.parent.resolve(strict=False).relative_to(project_root_resolved) - except (OSError, ValueError): + except (OSError, RuntimeError, ValueError): return None return candidate +def _safe_symlink_target(path: Path, project_root_resolved: Path) -> bool: + try: + target = path.readlink() + except OSError: + return False + + target_path = target if target.is_absolute() else path.parent / target + try: + target_path.resolve(strict=False).relative_to(project_root_resolved) + except (OSError, RuntimeError, ValueError): + return False + return True + + def _is_safe_manifest_key(key: str) -> bool: if key in {"", ".", ".."}: return False @@ -140,16 +154,21 @@ def _manifest_file_status( if path is None: invalid.append(rel) continue - valid.append(rel) try: path_stat = path.lstat() except FileNotFoundError: + valid.append(rel) missing.append(rel) continue except OSError: + valid.append(rel) modified.append(rel) continue if stat.S_ISLNK(path_stat.st_mode): + if not _safe_symlink_target(path, project_root_resolved): + invalid.append(rel) + continue + valid.append(rel) try: path.stat() except FileNotFoundError: @@ -159,6 +178,7 @@ def _manifest_file_status( pass modified.append(rel) continue + valid.append(rel) if not stat.S_ISREG(path_stat.st_mode): modified.append(rel) continue @@ -214,7 +234,10 @@ def _manifest_suggestion(key: str, default_key: str | None) -> str: 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 ( + "Upgrade Spec Kit, reinstall with a supported CLI version, " + f"or remove the stale integration entry from {INTEGRATION_JSON}." + ) return f"Run `specify integration upgrade {key}` or reinstall the integration." @@ -323,7 +346,10 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: "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.", + suggestion=( + "Upgrade Spec Kit, reinstall with a supported CLI version, " + f"or remove the stale integration entry from {INTEGRATION_JSON}." + ), ) ) diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index f08f86bc42..5605c6501d 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -503,6 +503,36 @@ def fail_exists(self): assert invalid == [] assert valid == ["tracked.md"] + def test_status_treats_resolve_runtime_error_as_invalid_path(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") + project_root_resolved = project.resolve() + original_resolve = Path.resolve + + def fail_project_parent_resolve(self, *args, **kwargs): + if self == project: + raise RuntimeError("symlink loop") + return original_resolve(self, *args, **kwargs) + + monkeypatch.setattr(Path, "resolve", fail_project_parent_resolve) + + missing, modified, invalid, valid = _manifest_file_status( + manifest, + project_root_resolved, + ) + + assert missing == [] + assert modified == [] + assert invalid == ["tracked.md"] + assert valid == [] + 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"] @@ -544,6 +574,30 @@ 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_tracked_symlink_target_escape_as_invalid(self, tmp_path, copilot_project): + outside = tmp_path / "outside" + outside.mkdir() + outside_file = outside / "secret.txt" + outside_file.write_text("outside project\n", encoding="utf-8") + + 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)) + tracked_path = copilot_project / first_rel + tracked_path.unlink() + try: + tracked_path.symlink_to(outside_file) + except OSError as exc: + pytest.skip(f"symlinks unavailable: {exc}") + + result = _run_in_project(copilot_project, ["integration", "status", "--json"]) + + assert result.exit_code != 0 + payload = json.loads(result.output) + assert payload["invalid_manifest_paths"] == 1 + assert first_rel in payload["manifests"]["copilot"]["invalid_files"] + assert first_rel not in payload["manifests"]["copilot"]["modified_files"] + def test_status_reports_unsafe_multi_install_combination(self, copilot_project): from specify_cli.integrations.manifest import IntegrationManifest @@ -577,6 +631,7 @@ def test_status_treats_unknown_multi_install_as_unsafe(self, claude_project): assert result.exit_code != 0 assert "unknown-integration" in result.output assert "unsafe-multi-install" in result.output + assert "remove the stale integration entry" in result.output assert "Multi-install safe: no" in result.output def test_status_gives_actionable_suggestion_for_unknown_manifest(self, claude_project): @@ -595,7 +650,7 @@ def test_status_gives_actionable_suggestion_for_unknown_manifest(self, claude_pr 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 "remove the stale integration entry" 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, claude_project): From 4b40ef2207260189748167ae55c988458d2ae86a Mon Sep 17 00:00:00 2001 From: Pascal Date: Thu, 28 May 2026 18:41:46 +0200 Subject: [PATCH 18/21] fix(integration): avoid symlink target stat --- src/specify_cli/integration_status.py | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/specify_cli/integration_status.py b/src/specify_cli/integration_status.py index 0eb32c7161..aa184cf6af 100644 --- a/src/specify_cli/integration_status.py +++ b/src/specify_cli/integration_status.py @@ -111,18 +111,26 @@ def _safe_manifest_file( return candidate -def _safe_symlink_target(path: Path, project_root_resolved: Path) -> bool: +def _symlink_target_status(path: Path, project_root_resolved: Path) -> str: try: target = path.readlink() except OSError: - return False + return "modified" target_path = target if target.is_absolute() else path.parent / target try: - target_path.resolve(strict=False).relative_to(project_root_resolved) + resolved_target = target_path.resolve(strict=False) + resolved_target.relative_to(project_root_resolved) except (OSError, RuntimeError, ValueError): - return False - return True + return "invalid" + + try: + resolved_target.lstat() + except FileNotFoundError: + return "missing" + except OSError: + return "modified" + return "modified" def _is_safe_manifest_key(key: str) -> bool: @@ -165,17 +173,14 @@ def _manifest_file_status( modified.append(rel) continue if stat.S_ISLNK(path_stat.st_mode): - if not _safe_symlink_target(path, project_root_resolved): + symlink_status = _symlink_target_status(path, project_root_resolved) + if symlink_status == "invalid": invalid.append(rel) continue valid.append(rel) - try: - path.stat() - except FileNotFoundError: + if symlink_status == "missing": missing.append(rel) continue - except OSError: - pass modified.append(rel) continue valid.append(rel) From 6d5383e3c7ce8fb85f9a8c9a841b6e2d1a086a67 Mon Sep 17 00:00:00 2001 From: Pascal Date: Thu, 28 May 2026 19:02:44 +0200 Subject: [PATCH 19/21] fix(integration): clarify status edge cases --- src/specify_cli/integration_status.py | 37 ++++++++++++++++--- .../test_integration_subcommand.py | 20 ++++++++++ 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/src/specify_cli/integration_status.py b/src/specify_cli/integration_status.py index aa184cf6af..3f457d54ea 100644 --- a/src/specify_cli/integration_status.py +++ b/src/specify_cli/integration_status.py @@ -19,7 +19,7 @@ from .integrations import INTEGRATION_REGISTRY from .integrations.manifest import IntegrationManifest -_MANIFEST_READ_ERRORS = (ValueError, OSError) +_MANIFEST_READ_ERRORS = (RuntimeError, ValueError, OSError) _MANIFEST_KEY_RE = re.compile(r"^[A-Za-z0-9._-]+$") _WINDOWS_RESERVED_MANIFEST_BASENAMES = { "CON", @@ -111,7 +111,13 @@ def _safe_manifest_file( return candidate -def _symlink_target_status(path: Path, project_root_resolved: Path) -> str: +def _tracked_symlink_manifest_status(path: Path, project_root_resolved: Path) -> str: + """Classify a tracked symlink without following it outside the project. + + Manifests store content hashes for regular files, so an existing in-project + symlink is still reported as modified. Escaping targets are invalid, and + dangling in-project targets are missing. + """ try: target = path.readlink() except OSError: @@ -133,6 +139,24 @@ def _symlink_target_status(path: Path, project_root_resolved: Path) -> str: return "modified" +def _resolve_project_root_for_status( + project_root: Path, + findings: list[dict[str, str]], +) -> Path: + try: + return project_root.resolve() + except (OSError, RuntimeError) as exc: + findings.append( + _finding( + "warning", + "project-root-unresolved", + f"Could not fully resolve project root: {exc}", + suggestion="Check project path permissions and symlinks before relying on manifest path checks.", + ) + ) + return project_root.absolute() + + def _is_safe_manifest_key(key: str) -> bool: if key in {"", ".", ".."}: return False @@ -173,7 +197,7 @@ def _manifest_file_status( modified.append(rel) continue if stat.S_ISLNK(path_stat.st_mode): - symlink_status = _symlink_target_status(path, project_root_resolved) + symlink_status = _tracked_symlink_manifest_status(path, project_root_resolved) if symlink_status == "invalid": invalid.append(rel) continue @@ -249,7 +273,7 @@ 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]] = [] - project_root_resolved = project_root.resolve() + project_root_resolved = _resolve_project_root_for_status(project_root, findings) state, raw_state, error = try_read_integration_json_with_raw(project_root) if error is not None: findings.append( @@ -374,7 +398,10 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: "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.", + suggestion=( + "Use `specify integration use ` to change defaults, " + "or `specify integration switch ` only when replacing integrations." + ), ) ) diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index 5605c6501d..15c9df4cb0 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -503,6 +503,25 @@ def fail_exists(self): assert invalid == [] assert valid == ["tracked.md"] + def test_status_reports_unresolved_project_root_without_crashing(self, copilot_project, monkeypatch): + original_resolve = Path.resolve + failed = {"done": False} + + def fail_first_project_root_resolve(self, *args, **kwargs): + if self == copilot_project and not failed["done"]: + failed["done"] = True + raise RuntimeError("symlink loop") + return original_resolve(self, *args, **kwargs) + + monkeypatch.setattr(Path, "resolve", fail_first_project_root_resolve) + + result = _run_in_project(copilot_project, ["integration", "status", "--json"]) + + assert result.exit_code == 0 + payload = json.loads(result.output) + assert payload["status"] == "warning" + assert any(item["code"] == "project-root-unresolved" for item in payload["findings"]) + def test_status_treats_resolve_runtime_error_as_invalid_path(self, tmp_path, monkeypatch): from specify_cli.integration_status import _manifest_file_status from specify_cli.integrations.manifest import IntegrationManifest @@ -614,6 +633,7 @@ def test_status_reports_unsafe_multi_install_combination(self, copilot_project): assert result.exit_code != 0 assert "unsafe-multi-install" in result.output assert "Multi-install safe: no" in result.output + assert "specify integration switch " in result.output def test_status_treats_unknown_multi_install_as_unsafe(self, claude_project): from specify_cli.integrations.manifest import IntegrationManifest From 67d28d4590eda546ef95da4de68b4a3376eb776d Mon Sep 17 00:00:00 2001 From: Pascal Date: Thu, 28 May 2026 19:04:53 +0200 Subject: [PATCH 20/21] test(integration): pin status filesystem guards --- .../test_integration_subcommand.py | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index 15c9df4cb0..1ccb38057f 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -503,6 +503,19 @@ def fail_exists(self): assert invalid == [] assert valid == ["tracked.md"] + def test_status_does_not_use_exists_precheck_for_manifest_load(self, copilot_project, monkeypatch): + def fail_exists(self): + raise AssertionError(f"Path.exists() should not be used for {self}") + + monkeypatch.setattr(Path, "exists", fail_exists) + + 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["manifests"]["copilot"]["readable"] is True + def test_status_reports_unresolved_project_root_without_crashing(self, copilot_project, monkeypatch): original_resolve = Path.resolve failed = {"done": False} @@ -593,7 +606,7 @@ 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_tracked_symlink_target_escape_as_invalid(self, tmp_path, copilot_project): + def test_status_reports_tracked_symlink_target_escape_as_invalid(self, tmp_path, copilot_project, monkeypatch): outside = tmp_path / "outside" outside.mkdir() outside_file = outside / "secret.txt" @@ -609,6 +622,16 @@ def test_status_reports_tracked_symlink_target_escape_as_invalid(self, tmp_path, except OSError as exc: pytest.skip(f"symlinks unavailable: {exc}") + original_stat = Path.stat + + def fail_tracked_symlink_stat(self, *args, **kwargs): + follows_symlinks = kwargs.get("follow_symlinks", True) + if self == tracked_path and follows_symlinks: + raise AssertionError("Path.stat() should not follow tracked symlinks") + return original_stat(self, *args, **kwargs) + + monkeypatch.setattr(Path, "stat", fail_tracked_symlink_stat) + result = _run_in_project(copilot_project, ["integration", "status", "--json"]) assert result.exit_code != 0 From 928d89d744c0f806dbac3e78be20a41e296810e1 Mon Sep 17 00:00:00 2001 From: Pascal Date: Thu, 28 May 2026 19:17:11 +0200 Subject: [PATCH 21/21] fix(integration): tighten status path checks --- src/specify_cli/integration_state.py | 7 ++- src/specify_cli/integration_status.py | 55 +++++++++++++++---- .../test_integration_subcommand.py | 40 ++++++++++++++ 3 files changed, 89 insertions(+), 13 deletions(-) diff --git a/src/specify_cli/integration_state.py b/src/specify_cli/integration_state.py index 74dc6aa7de..26e1c24b68 100644 --- a/src/specify_cli/integration_state.py +++ b/src/specify_cli/integration_state.py @@ -90,7 +90,12 @@ def try_read_integration_json( 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.""" + """Parse ``integration.json`` and return normalized plus raw state. + + Returns ``(normalized_state, raw_state, None)`` when the file is readable, + ``(None, None, None)`` when it is absent, and ``(None, None, error)`` for + parse, schema, encoding, or filesystem failures. + """ data, error = _read_integration_json_data(project_root) if data is None: return None, None, error diff --git a/src/specify_cli/integration_status.py b/src/specify_cli/integration_status.py index 3f457d54ea..4f6f5280ff 100644 --- a/src/specify_cli/integration_status.py +++ b/src/specify_cli/integration_status.py @@ -19,7 +19,7 @@ from .integrations import INTEGRATION_REGISTRY from .integrations.manifest import IntegrationManifest -_MANIFEST_READ_ERRORS = (RuntimeError, ValueError, OSError) +_MANIFEST_READ_ERRORS = (ValueError, OSError) _MANIFEST_KEY_RE = re.compile(r"^[A-Za-z0-9._-]+$") _WINDOWS_RESERVED_MANIFEST_BASENAMES = { "CON", @@ -99,19 +99,31 @@ def _safe_manifest_file( project_root: Path, project_root_resolved: Path, rel: str, + *, + project_root_is_resolved: bool = True, ) -> 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_resolved) + candidate_parent = ( + candidate.parent.resolve(strict=False) + if project_root_is_resolved + else candidate.parent.absolute() + ) + candidate_parent.relative_to(project_root_resolved) except (OSError, RuntimeError, ValueError): return None return candidate -def _tracked_symlink_manifest_status(path: Path, project_root_resolved: Path) -> str: +def _tracked_symlink_manifest_status( + path: Path, + project_root_resolved: Path, + *, + project_root_is_resolved: bool = True, +) -> str: """Classify a tracked symlink without following it outside the project. Manifests store content hashes for regular files, so an existing in-project @@ -125,13 +137,17 @@ def _tracked_symlink_manifest_status(path: Path, project_root_resolved: Path) -> target_path = target if target.is_absolute() else path.parent / target try: - resolved_target = target_path.resolve(strict=False) - resolved_target.relative_to(project_root_resolved) + contained_target = ( + target_path.resolve(strict=False) + if project_root_is_resolved + else target_path.absolute() + ) + contained_target.relative_to(project_root_resolved) except (OSError, RuntimeError, ValueError): return "invalid" try: - resolved_target.lstat() + contained_target.lstat() except FileNotFoundError: return "missing" except OSError: @@ -142,9 +158,9 @@ def _tracked_symlink_manifest_status(path: Path, project_root_resolved: Path) -> def _resolve_project_root_for_status( project_root: Path, findings: list[dict[str, str]], -) -> Path: +) -> tuple[Path, bool]: try: - return project_root.resolve() + return project_root.resolve(), True except (OSError, RuntimeError) as exc: findings.append( _finding( @@ -154,7 +170,7 @@ def _resolve_project_root_for_status( suggestion="Check project path permissions and symlinks before relying on manifest path checks.", ) ) - return project_root.absolute() + return project_root.absolute(), False def _is_safe_manifest_key(key: str) -> bool: @@ -175,6 +191,8 @@ def _is_safe_manifest_key(key: str) -> bool: def _manifest_file_status( manifest: IntegrationManifest, project_root_resolved: Path, + *, + project_root_is_resolved: bool = True, ) -> tuple[list[str], list[str], list[str], list[str]]: missing: list[str] = [] modified: list[str] = [] @@ -182,7 +200,12 @@ def _manifest_file_status( valid: list[str] = [] for rel, expected_hash in manifest.files.items(): - path = _safe_manifest_file(manifest.project_root, project_root_resolved, rel) + path = _safe_manifest_file( + manifest.project_root, + project_root_resolved, + rel, + project_root_is_resolved=project_root_is_resolved, + ) if path is None: invalid.append(rel) continue @@ -197,7 +220,11 @@ def _manifest_file_status( modified.append(rel) continue if stat.S_ISLNK(path_stat.st_mode): - symlink_status = _tracked_symlink_manifest_status(path, project_root_resolved) + symlink_status = _tracked_symlink_manifest_status( + path, + project_root_resolved, + project_root_is_resolved=project_root_is_resolved, + ) if symlink_status == "invalid": invalid.append(rel) continue @@ -273,7 +300,10 @@ 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]] = [] - project_root_resolved = _resolve_project_root_for_status(project_root, findings) + project_root_resolved, project_root_is_resolved = _resolve_project_root_for_status( + project_root, + findings, + ) state, raw_state, error = try_read_integration_json_with_raw(project_root) if error is not None: findings.append( @@ -469,6 +499,7 @@ def build_integration_status_report(project_root: Path) -> dict[str, Any]: missing, modified, invalid, valid_files = _manifest_file_status( manifest, project_root_resolved, + project_root_is_resolved=project_root_is_resolved, ) manifest_summaries[key] = _manifest_summary( manifest_path, diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index 1ccb38057f..d4dec0367d 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -535,6 +535,35 @@ def fail_first_project_root_resolve(self, *args, **kwargs): assert payload["status"] == "warning" assert any(item["code"] == "project-root-unresolved" for item in payload["findings"]) + def test_status_uses_lexical_manifest_paths_when_project_root_resolution_falls_back(self, tmp_path): + from specify_cli.integration_status import _manifest_file_status + from specify_cli.integrations.manifest import IntegrationManifest + + real_project = tmp_path / "real-project" + real_project.mkdir() + tracked = real_project / "tracked.md" + tracked.write_text("content\n", encoding="utf-8") + symlinked_project = tmp_path / "symlinked-project" + try: + symlinked_project.symlink_to(real_project, target_is_directory=True) + except OSError as exc: + pytest.skip(f"symlinks unavailable: {exc}") + + manifest = IntegrationManifest("test", real_project, version="test") + manifest.record_existing("tracked.md") + manifest.project_root = symlinked_project.absolute() + + missing, modified, invalid, valid = _manifest_file_status( + manifest, + symlinked_project.absolute(), + project_root_is_resolved=False, + ) + + assert missing == [] + assert modified == [] + assert invalid == [] + assert valid == ["tracked.md"] + def test_status_treats_resolve_runtime_error_as_invalid_path(self, tmp_path, monkeypatch): from specify_cli.integration_status import _manifest_file_status from specify_cli.integrations.manifest import IntegrationManifest @@ -565,6 +594,17 @@ def fail_project_parent_resolve(self, *args, **kwargs): assert invalid == ["tracked.md"] assert valid == [] + def test_status_does_not_mask_runtime_errors_from_manifest_load(self, copilot_project, monkeypatch): + from specify_cli import integration_status as status_module + + def fail_load(key, project_root): + raise RuntimeError(f"unexpected manifest loader bug for {key}") + + monkeypatch.setattr(status_module.IntegrationManifest, "load", fail_load) + + with pytest.raises(RuntimeError, match="unexpected manifest loader bug"): + status_module.build_integration_status_report(copilot_project) + 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"]