From 28864417f81b0c6117b2aba11349670774264db3 Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Sat, 2 May 2026 08:03:47 +0500 Subject: [PATCH 1/5] fix(workflow): support integration: auto to follow project's initialized AI Closes #2406 (squashed) --- src/specify_cli/__init__.py | 5 + src/specify_cli/workflows/engine.py | 85 +++++++++- tests/test_workflows.py | 235 ++++++++++++++++++++++++++++ workflows/speckit/workflow.yml | 21 ++- 4 files changed, 341 insertions(+), 5 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index ccd670d20e..adaea6336a 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1936,6 +1936,11 @@ def _read_integration_json(project_root: Path) -> dict[str, Any]: console.print(f"Please fix or delete {INTEGRATION_JSON} and retry.") console.print(f"[dim]Details:[/dim] {exc}") raise typer.Exit(1) + except UnicodeDecodeError as exc: + console.print(f"[red]Error:[/red] {path} is not valid UTF-8.") + console.print(f"Please fix or delete {INTEGRATION_JSON} and retry.") + console.print(f"[dim]Details:[/dim] {exc}") + raise typer.Exit(1) except OSError as exc: console.print(f"[red]Error:[/red] Could not read {path}.") console.print(f"Please fix file permissions or delete {INTEGRATION_JSON} and retry.") diff --git a/src/specify_cli/workflows/engine.py b/src/specify_cli/workflows/engine.py index d6a73bbeb0..9db3b12725 100644 --- a/src/specify_cli/workflows/engine.py +++ b/src/specify_cli/workflows/engine.py @@ -19,6 +19,7 @@ import yaml +from ..integration_state import INTEGRATION_JSON from .base import RunStatus, StepContext, StepResult, StepStatus @@ -143,6 +144,35 @@ def validate_workflow(definition: WorkflowDefinition) -> list[str]: f"Must be 'string', 'number', or 'boolean'." ) + # Validate the default eagerly so authoring mistakes (e.g. a + # default not in the declared enum, or a non-numeric default for + # a number input) surface at install/validation time instead of + # at workflow-execution time. ``"auto"`` for the integration + # input is a runtime-resolved sentinel, so only the + # enum-membership check is exempted for that exact case — the + # declared type is still enforced (e.g. ``type: number`` paired + # with ``default: "auto"`` is still rejected). + if "default" in input_def: + default_value = input_def["default"] + is_auto_integration = ( + input_name == "integration" and default_value == "auto" + ) + validation_input_def: dict[str, Any] = input_def + if is_auto_integration and "enum" in input_def: + validation_input_def = { + key: value + for key, value in input_def.items() + if key != "enum" + } + try: + WorkflowEngine._coerce_input( + input_name, default_value, validation_input_def + ) + except ValueError as exc: + errors.append( + f"Input {input_name!r} has invalid default: {exc}" + ) + # -- Steps ------------------------------------------------------------ if not isinstance(definition.steps, list): errors.append("'steps' must be a list.") @@ -715,12 +745,65 @@ def _resolve_inputs( name, provided[name], input_def ) elif "default" in input_def: - resolved[name] = input_def["default"] + default_value = self._resolve_default(name, input_def["default"]) + # If the integration default could not be resolved against + # project state and falls back to the literal ``"auto"`` + # sentinel, exempt it from enum-membership coercion so a + # workflow that lists specific integrations in ``enum`` does + # not crash at runtime — declared type is still enforced. + coerce_input_def = input_def + if ( + name == "integration" + and default_value == "auto" + and "enum" in input_def + ): + coerce_input_def = { + key: value + for key, value in input_def.items() + if key != "enum" + } + resolved[name] = self._coerce_input( + name, default_value, coerce_input_def + ) elif input_def.get("required", False): msg = f"Required input {name!r} not provided." raise ValueError(msg) return resolved + def _resolve_default(self, name: str, default: Any) -> Any: + """Resolve special default sentinels against project state. + + For the ``integration`` input, ``"auto"`` resolves to the integration + recorded in ``.specify/integration.json`` so workflows dispatch to the + AI the project was actually initialized with, instead of a hardcoded + value baked into the workflow YAML. + """ + if name == "integration" and default == "auto": + resolved = self._load_project_integration() + if resolved is not None: + return resolved + return default + + def _load_project_integration(self) -> str | None: + """Read the active integration key from ``.specify/integration.json``. + + Returns None when the file is missing or malformed; callers are + expected to fall back to a literal default. + """ + path = self.project_root / INTEGRATION_JSON + if not path.is_file(): + return None + try: + data = json.loads(path.read_text(encoding="utf-8")) + except (json.JSONDecodeError, OSError, UnicodeDecodeError): + return None + if not isinstance(data, dict): + return None + value = data.get("integration") + if isinstance(value, str) and value: + return value + return None + @staticmethod def _coerce_input( name: str, value: Any, input_def: dict[str, Any] diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 4c042fc7d5..804ec709be 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -1495,6 +1495,241 @@ def test_execute_missing_required_input(self, project_dir): with pytest.raises(ValueError, match="Required input"): engine.execute(definition, {}) + def test_integration_auto_default_uses_project_integration(self, project_dir): + """`integration: auto` should resolve to .specify/integration.json's integration.""" + from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition + + specify_dir = project_dir / ".specify" + specify_dir.mkdir(parents=True, exist_ok=True) + (specify_dir / "integration.json").write_text( + json.dumps({"integration": "opencode", "version": "0.7.4"}), + encoding="utf-8", + ) + + definition = WorkflowDefinition.from_string(""" +schema_version: "1.0" +workflow: + id: "auto-default" + name: "Auto Default" + version: "1.0.0" +inputs: + integration: + type: string + default: "auto" +""") + engine = WorkflowEngine(project_dir) + resolved = engine._resolve_inputs(definition, {}) + assert resolved["integration"] == "opencode" + + def test_integration_auto_default_falls_back_when_no_integration_json(self, project_dir): + """`integration: auto` should keep the literal "auto" when project state is missing. + + The engine itself must not invent an integration when + ``.specify/integration.json`` is absent; any later validation or + command resolution will handle an unresolved ``"auto"`` value. + """ + from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition + + definition = WorkflowDefinition.from_string(""" +schema_version: "1.0" +workflow: + id: "auto-fallback" + name: "Auto Fallback" + version: "1.0.0" +inputs: + integration: + type: string + default: "auto" +""") + engine = WorkflowEngine(project_dir) + resolved = engine._resolve_inputs(definition, {}) + assert resolved["integration"] == "auto" + + def test_integration_explicit_input_overrides_auto(self, project_dir): + """An explicit --input integration=X must win over `auto` even when integration.json exists.""" + from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition + + specify_dir = project_dir / ".specify" + specify_dir.mkdir(parents=True, exist_ok=True) + (specify_dir / "integration.json").write_text( + json.dumps({"integration": "opencode"}), + encoding="utf-8", + ) + + definition = WorkflowDefinition.from_string(""" +schema_version: "1.0" +workflow: + id: "explicit-wins" + name: "Explicit Wins" + version: "1.0.0" +inputs: + integration: + type: string + default: "auto" +""") + engine = WorkflowEngine(project_dir) + resolved = engine._resolve_inputs(definition, {"integration": "claude"}) + assert resolved["integration"] == "claude" + + def test_integration_auto_ignores_malformed_integration_json(self, project_dir): + """A malformed integration.json must not crash — fall back to the literal default.""" + from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition + + specify_dir = project_dir / ".specify" + specify_dir.mkdir(parents=True, exist_ok=True) + (specify_dir / "integration.json").write_text("{not json", encoding="utf-8") + + definition = WorkflowDefinition.from_string(""" +schema_version: "1.0" +workflow: + id: "auto-malformed" + name: "Auto Malformed" + version: "1.0.0" +inputs: + integration: + type: string + default: "auto" +""") + engine = WorkflowEngine(project_dir) + resolved = engine._resolve_inputs(definition, {}) + assert resolved["integration"] == "auto" + + def test_integration_auto_ignores_non_utf8_integration_json(self, project_dir): + """A non-UTF8 integration.json must not crash — fall back to the literal default.""" + from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition + + specify_dir = project_dir / ".specify" + specify_dir.mkdir(parents=True, exist_ok=True) + # 0xFF is invalid as the leading byte of a UTF-8 sequence, so + # ``Path.read_text(encoding="utf-8")`` raises UnicodeDecodeError. + (specify_dir / "integration.json").write_bytes(b"\xff\xfe\x00\x00") + + definition = WorkflowDefinition.from_string(""" +schema_version: "1.0" +workflow: + id: "auto-non-utf8" + name: "Auto Non UTF-8" + version: "1.0.0" +inputs: + integration: + type: string + default: "auto" +""") + engine = WorkflowEngine(project_dir) + resolved = engine._resolve_inputs(definition, {}) + assert resolved["integration"] == "auto" + + def test_default_value_is_validated_against_enum(self, project_dir): + """Defaults must run through the same coercion/enum check as provided inputs.""" + from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition + + definition = WorkflowDefinition.from_string(""" +schema_version: "1.0" +workflow: + id: "default-enum" + name: "Default Enum" + version: "1.0.0" +inputs: + scope: + type: string + default: "not-in-enum" + enum: ["full", "backend-only", "frontend-only"] +""") + engine = WorkflowEngine(project_dir) + with pytest.raises(ValueError, match="not in allowed values"): + engine._resolve_inputs(definition, {}) + + def test_default_value_is_coerced_to_declared_type(self, project_dir): + """A numeric default declared as a string should still be coerced like a provided input.""" + from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition + + definition = WorkflowDefinition.from_string(""" +schema_version: "1.0" +workflow: + id: "default-coerce" + name: "Default Coerce" + version: "1.0.0" +inputs: + retries: + type: number + default: "3" +""") + engine = WorkflowEngine(project_dir) + resolved = engine._resolve_inputs(definition, {}) + assert resolved["retries"] == 3 + assert isinstance(resolved["retries"], int) + + def test_validate_workflow_rejects_invalid_default(self): + """Authoring-time validation should reject defaults that violate enum.""" + from specify_cli.workflows.engine import WorkflowDefinition, validate_workflow + + definition = WorkflowDefinition.from_string(""" +schema_version: "1.0" +workflow: + id: "bad-default" + name: "Bad Default" + version: "1.0.0" +inputs: + scope: + type: string + default: "not-in-enum" + enum: ["full", "backend-only", "frontend-only"] +steps: + - id: noop + type: gate + message: "noop" + options: [approve] +""") + errors = validate_workflow(definition) + assert any("invalid default" in e for e in errors), errors + + def test_validate_workflow_exempts_integration_auto_sentinel(self): + """``integration: auto`` is a runtime-resolved sentinel and must not fail validation.""" + from specify_cli.workflows.engine import WorkflowDefinition, validate_workflow + + definition = WorkflowDefinition.from_string(""" +schema_version: "1.0" +workflow: + id: "auto-ok" + name: "Auto OK" + version: "1.0.0" +inputs: + integration: + type: string + default: "auto" + enum: ["copilot", "claude", "gemini"] +steps: + - id: noop + type: gate + message: "noop" + options: [approve] +""") + errors = validate_workflow(definition) + assert not any("invalid default" in e for e in errors), errors + + def test_validate_workflow_still_checks_type_for_auto_sentinel(self): + """The ``auto`` exemption only skips enum-membership; declared type is still enforced.""" + from specify_cli.workflows.engine import WorkflowDefinition, validate_workflow + + definition = WorkflowDefinition.from_string(""" +schema_version: "1.0" +workflow: + id: "auto-bad-type" + name: "Auto Bad Type" + version: "1.0.0" +inputs: + integration: + type: number + default: "auto" +steps: + - id: noop + type: gate + message: "noop" + options: [approve] +""") + errors = validate_workflow(definition) + assert any("invalid default" in e for e in errors), errors + # ===== State Persistence Tests ===== diff --git a/workflows/speckit/workflow.yml b/workflows/speckit/workflow.yml index bf18451029..4acf5a915d 100644 --- a/workflows/speckit/workflow.yml +++ b/workflows/speckit/workflow.yml @@ -7,9 +7,22 @@ workflow: description: "Runs specify → plan → tasks → implement with review gates" requires: - speckit_version: ">=0.7.2" + # 0.8.3 is the first release with engine-side resolution of the + # ``integration: "auto"`` default. Older versions would treat "auto" + # as a literal integration key and fail at dispatch. + speckit_version: ">=0.8.5" integrations: - any: ["copilot", "claude", "gemini"] + # The four commands below (specify, plan, tasks, implement) are core + # spec-kit commands provided by every integration. The list here is an + # explicit non-exhaustive compatibility hint following the documented + # ``any: [...]`` schema; the workflow is intended to run against any + # integration the project was initialized with, including ones not + # listed below. + any: + - "claude" + - "copilot" + - "gemini" + - "opencode" inputs: spec: @@ -18,8 +31,8 @@ inputs: prompt: "Describe what you want to build" integration: type: string - default: "copilot" - prompt: "Integration to use (e.g. claude, copilot, gemini)" + default: "auto" + prompt: "Integration to use (e.g. claude, copilot, gemini; 'auto' uses the project's initialized integration)" scope: type: string default: "full" From 8c4488acbf2a259f52d3562314a84270086a854e Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Tue, 5 May 2026 13:16:13 +0500 Subject: [PATCH 2/5] fix(workflow): combine JSONDecodeError and UnicodeDecodeError handling Address Copilot feedback: UnicodeDecodeError can be raised by both read_text() and json.loads(), so combining the handlers ensures both cases produce a consistent, clear error message. --- src/specify_cli/__init__.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index adaea6336a..abbda1247c 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1931,13 +1931,8 @@ def _read_integration_json(project_root: Path) -> dict[str, Any]: return {} try: data = json.loads(path.read_text(encoding="utf-8")) - except json.JSONDecodeError as exc: - console.print(f"[red]Error:[/red] {path} contains invalid JSON.") - console.print(f"Please fix or delete {INTEGRATION_JSON} and retry.") - console.print(f"[dim]Details:[/dim] {exc}") - raise typer.Exit(1) - except UnicodeDecodeError as exc: - console.print(f"[red]Error:[/red] {path} is not valid UTF-8.") + except (json.JSONDecodeError, UnicodeDecodeError) as exc: + console.print(f"[red]Error:[/red] {path} contains invalid JSON or is not valid UTF-8.") console.print(f"Please fix or delete {INTEGRATION_JSON} and retry.") console.print(f"[dim]Details:[/dim] {exc}") raise typer.Exit(1) From f58ca8c7b8a6cad2f74d576e07dea836d886f31b Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Sat, 9 May 2026 03:06:15 +0500 Subject: [PATCH 3/5] fix(workflows): honor integration_state schema guard and modern state in 'integration: auto' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three Copilot follow-ups on PR #2421: 1. engine.py:799 — `_load_project_integration` was bypassing the same schema guard `_read_integration_json` enforces. It now reads the schema field directly, returns None on a future schema (so the workflow falls back to the literal 'auto' default rather than guessing), and routes through `normalize_integration_state` / `default_integration_key` so modern installs that record `default_integration` / `installed_integrations` (without the legacy top-level `integration` field) resolve correctly. 2. test_workflows.py — added two regression cases: - `integration: auto` resolves a modern normalized state file - `integration: auto` falls back when the state file declares a newer `integration_state_schema` than this CLI supports 3. test_cli.py — added a CLI-level regression for the `UnicodeDecodeError` branch in `_read_integration_json` to match the existing malformed-JSON coverage. --- src/specify_cli/workflows/engine.py | 30 ++++++++---- tests/integrations/test_cli.py | 24 ++++++++++ tests/test_workflows.py | 73 +++++++++++++++++++++++++++++ 3 files changed, 119 insertions(+), 8 deletions(-) diff --git a/src/specify_cli/workflows/engine.py b/src/specify_cli/workflows/engine.py index 9db3b12725..25de351ed4 100644 --- a/src/specify_cli/workflows/engine.py +++ b/src/specify_cli/workflows/engine.py @@ -19,7 +19,12 @@ import yaml -from ..integration_state import INTEGRATION_JSON +from ..integration_state import ( + INTEGRATION_JSON, + INTEGRATION_STATE_SCHEMA, + default_integration_key, + normalize_integration_state, +) from .base import RunStatus, StepContext, StepResult, StepStatus @@ -785,9 +790,14 @@ def _resolve_default(self, name: str, default: Any) -> Any: return default def _load_project_integration(self) -> str | None: - """Read the active integration key from ``.specify/integration.json``. - - Returns None when the file is missing or malformed; callers are + """Read the default integration key from ``.specify/integration.json``. + + Honors the same schema guard as ``_read_integration_json`` (rejects + files whose ``integration_state_schema`` is newer than this CLI + supports) and reads the canonical normalized state, so modern + installs that record ``default_integration`` / ``installed_integrations`` + resolve correctly under ``integration: auto``. Returns None when the + file is missing, malformed, or written by a newer CLI; callers are expected to fall back to a literal default. """ path = self.project_root / INTEGRATION_JSON @@ -799,10 +809,14 @@ def _load_project_integration(self) -> str | None: return None if not isinstance(data, dict): return None - value = data.get("integration") - if isinstance(value, str) and value: - return value - return None + schema = data.get("integration_state_schema") + if ( + isinstance(schema, int) + and not isinstance(schema, bool) + and schema > INTEGRATION_STATE_SCHEMA + ): + return None + return default_integration_key(normalize_integration_state(data)) @staticmethod def _coerce_input( diff --git a/tests/integrations/test_cli.py b/tests/integrations/test_cli.py index 7732d57300..562afd488d 100644 --- a/tests/integrations/test_cli.py +++ b/tests/integrations/test_cli.py @@ -1191,6 +1191,30 @@ def fail_search(self, **kwargs): assert "contains invalid JSON" in normalized_output assert "integration.json" in normalized_output + def test_search_rejects_non_utf8_integration_json_before_catalog_lookup( + self, tmp_path, monkeypatch + ): + """A non-UTF8 ``integration.json`` must surface a clear error and + avoid falling through to the catalog lookup, mirroring the malformed-JSON + case but for the ``UnicodeDecodeError`` branch in ``_read_integration_json``.""" + project = self._make_project(tmp_path) + # 0xFF is invalid as the leading byte of any UTF-8 sequence, so + # ``Path.read_text(encoding="utf-8")`` raises ``UnicodeDecodeError``. + (project / ".specify" / "integration.json").write_bytes(b"\xff\xfe\x00\x00") + + from specify_cli.integrations.catalog import IntegrationCatalog + + def fail_search(self, **kwargs): + raise AssertionError("catalog search should not be called") + + monkeypatch.setattr(IntegrationCatalog, "search", fail_search) + + result = self._invoke(["integration", "search"], project) + normalized_output = _normalize_cli_output(result.output) + assert result.exit_code == 1 + assert "not valid UTF-8" in normalized_output + assert "integration.json" in normalized_output + def test_search_filters_by_tag(self, tmp_path, monkeypatch): project = self._make_project(tmp_path) self._patch_catalog(monkeypatch) diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 804ec709be..653b3e3ca2 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -1619,6 +1619,79 @@ def test_integration_auto_ignores_non_utf8_integration_json(self, project_dir): resolved = engine._resolve_inputs(definition, {}) assert resolved["integration"] == "auto" + def test_integration_auto_resolves_modern_normalized_state(self, project_dir): + """`integration: auto` must resolve modern state files that record + ``default_integration`` / ``installed_integrations`` and omit the + legacy ``integration`` field.""" + from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition + + specify_dir = project_dir / ".specify" + specify_dir.mkdir(parents=True, exist_ok=True) + (specify_dir / "integration.json").write_text( + json.dumps( + { + "version": "0.8.3", + "integration_state_schema": 1, + "default_integration": "claude", + "installed_integrations": ["claude", "copilot"], + "integration_settings": {}, + } + ), + encoding="utf-8", + ) + + definition = WorkflowDefinition.from_string(""" +schema_version: "1.0" +workflow: + id: "auto-modern" + name: "Auto Modern" + version: "1.0.0" +inputs: + integration: + type: string + default: "auto" +""") + engine = WorkflowEngine(project_dir) + resolved = engine._resolve_inputs(definition, {}) + assert resolved["integration"] == "claude" + + def test_integration_auto_rejects_future_state_schema(self, project_dir): + """`integration: auto` must not silently use a state file written by a newer + CLI (``integration_state_schema`` greater than the current supported value); + the resolver falls back to the literal default rather than guessing.""" + from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition + from specify_cli.integration_state import INTEGRATION_STATE_SCHEMA + + specify_dir = project_dir / ".specify" + specify_dir.mkdir(parents=True, exist_ok=True) + (specify_dir / "integration.json").write_text( + json.dumps( + { + "version": "99.0.0", + "integration_state_schema": INTEGRATION_STATE_SCHEMA + 1, + "default_integration": "claude", + "installed_integrations": ["claude"], + "integration_settings": {}, + } + ), + encoding="utf-8", + ) + + definition = WorkflowDefinition.from_string(""" +schema_version: "1.0" +workflow: + id: "auto-future-schema" + name: "Auto Future Schema" + version: "1.0.0" +inputs: + integration: + type: string + default: "auto" +""") + engine = WorkflowEngine(project_dir) + resolved = engine._resolve_inputs(definition, {}) + assert resolved["integration"] == "auto" + def test_default_value_is_validated_against_enum(self, project_dir): """Defaults must run through the same coercion/enum check as provided inputs.""" from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition From 9a0e045e5958dc83d599c26b1ccfee071570664a Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Wed, 13 May 2026 00:43:58 +0500 Subject: [PATCH 4/5] refactor(integration): extract shared try_read_integration_json helper Address Copilot review on PR #2421: Both `_read_integration_json` (CLI) and `_load_project_integration` (workflow engine) were parsing `.specify/integration.json` independently, duplicating the schema guard and risking drift between the two readers. Extract the parse + schema validation into a single low-level helper `try_read_integration_json` in `integration_state.py` that returns either the normalized state or a structured `IntegrationReadError`. Both callers now delegate to this helper: - CLI keeps its loud-fail UX: each error kind ("decode", "os", "not_object", "schema_too_new") is translated into the existing console message + typer.Exit(1). - Engine keeps its silent fallback: any error simply returns None so `integration: auto` falls back to the workflow's literal default. This eliminates the divergence Copilot flagged without changing observable behavior for either caller. --- src/specify_cli/__init__.py | 41 +++++++++++----------- src/specify_cli/integration_state.py | 51 ++++++++++++++++++++++++++++ src/specify_cli/workflows/engine.py | 35 +++++-------------- 3 files changed, 82 insertions(+), 45 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 06d2c0b3df..8d2053f14e 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -68,6 +68,7 @@ integration_setting as _integration_setting, integration_settings as _integration_settings, normalize_integration_state as _normalize_integration_state, + try_read_integration_json as _try_read_integration_json, write_integration_json as _write_integration_json_file, ) from .shared_infra import ( @@ -1952,35 +1953,37 @@ def get_speckit_version() -> str: def _read_integration_json(project_root: Path) -> dict[str, Any]: - """Load ``.specify/integration.json``. Returns normalized state when present.""" + """Load ``.specify/integration.json``. Returns normalized state when present. + + Delegates the parse / schema-guard logic to the shared + :func:`_try_read_integration_json` helper so the CLI and workflow engine + cannot drift on validation rules. Each error variant is translated into + the existing loud-fail UX (console message + ``typer.Exit(1)``). + """ path = project_root / INTEGRATION_JSON - if not path.exists(): - return {} - try: - data = json.loads(path.read_text(encoding="utf-8")) - except (json.JSONDecodeError, UnicodeDecodeError) as exc: + state, error = _try_read_integration_json(project_root) + if error is None: + return state or {} + if error.kind == "decode": console.print(f"[red]Error:[/red] {path} contains invalid JSON or is not valid UTF-8.") console.print(f"Please fix or delete {INTEGRATION_JSON} and retry.") - console.print(f"[dim]Details:[/dim] {exc}") - raise typer.Exit(1) - except OSError as exc: + console.print(f"[dim]Details:[/dim] {error.detail}") + elif error.kind == "os": console.print(f"[red]Error:[/red] Could not read {path}.") console.print(f"Please fix file permissions or delete {INTEGRATION_JSON} and retry.") - console.print(f"[dim]Details:[/dim] {exc}") - raise typer.Exit(1) - if not isinstance(data, dict): - console.print(f"[red]Error:[/red] {path} must contain a JSON object, got {type(data).__name__}.") + console.print(f"[dim]Details:[/dim] {error.detail}") + elif error.kind == "not_object": + console.print( + f"[red]Error:[/red] {path} must contain a JSON object, got {error.detail}." + ) console.print(f"Please fix or delete {INTEGRATION_JSON} and retry.") - raise typer.Exit(1) - schema = data.get("integration_state_schema") - if isinstance(schema, int) and not isinstance(schema, bool) and schema > INTEGRATION_STATE_SCHEMA: + elif error.kind == "schema_too_new": console.print( - f"[red]Error:[/red] {path} uses integration state schema {schema}, " + f"[red]Error:[/red] {path} uses integration state schema {error.schema}, " f"but this CLI only supports schema {INTEGRATION_STATE_SCHEMA}." ) console.print("Please upgrade Spec Kit before modifying integrations.") - raise typer.Exit(1) - return _normalize_integration_state(data) + raise typer.Exit(1) def _write_integration_json( diff --git a/src/specify_cli/integration_state.py b/src/specify_cli/integration_state.py index ac892dfbf6..c51be47749 100644 --- a/src/specify_cli/integration_state.py +++ b/src/specify_cli/integration_state.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +from dataclasses import dataclass from pathlib import Path from typing import Any @@ -11,6 +12,56 @@ INTEGRATION_STATE_SCHEMA = 1 +@dataclass(frozen=True) +class IntegrationReadError: + """Structured failure from :func:`try_read_integration_json`. + + Callers map ``kind`` to whatever surface they need (loud CLI error, + silent fallback, etc.) without re-implementing the parse/validation logic. + """ + + kind: str # "decode", "os", "not_object", "schema_too_new" + detail: str = "" + schema: int | None = 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. + """ + path = project_root / INTEGRATION_JSON + if not path.is_file(): + return None, None + try: + raw = path.read_text(encoding="utf-8") + except UnicodeDecodeError as exc: + return None, IntegrationReadError(kind="decode", detail=str(exc)) + except OSError as exc: + return None, IntegrationReadError(kind="os", detail=str(exc)) + try: + data = json.loads(raw) + except json.JSONDecodeError as exc: + return None, IntegrationReadError(kind="decode", detail=str(exc)) + if not isinstance(data, dict): + return None, IntegrationReadError(kind="not_object", detail=type(data).__name__) + schema = data.get("integration_state_schema") + if ( + isinstance(schema, int) + and not isinstance(schema, bool) + and schema > INTEGRATION_STATE_SCHEMA + ): + return None, IntegrationReadError(kind="schema_too_new", schema=schema) + return normalize_integration_state(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/workflows/engine.py b/src/specify_cli/workflows/engine.py index 25de351ed4..4aa1036086 100644 --- a/src/specify_cli/workflows/engine.py +++ b/src/specify_cli/workflows/engine.py @@ -20,10 +20,8 @@ import yaml from ..integration_state import ( - INTEGRATION_JSON, - INTEGRATION_STATE_SCHEMA, default_integration_key, - normalize_integration_state, + try_read_integration_json, ) from .base import RunStatus, StepContext, StepResult, StepStatus @@ -792,31 +790,16 @@ def _resolve_default(self, name: str, default: Any) -> Any: def _load_project_integration(self) -> str | None: """Read the default integration key from ``.specify/integration.json``. - Honors the same schema guard as ``_read_integration_json`` (rejects - files whose ``integration_state_schema`` is newer than this CLI - supports) and reads the canonical normalized state, so modern - installs that record ``default_integration`` / ``installed_integrations`` - resolve correctly under ``integration: auto``. Returns None when the - file is missing, malformed, or written by a newer CLI; callers are - expected to fall back to a literal default. + Delegates parsing and schema validation to + :func:`try_read_integration_json` — the same low-level helper used by + the CLI — so the engine cannot drift from CLI behavior on the parse + path. Returns ``None`` when the file is missing, malformed, or + written by a newer CLI; callers fall back to the literal default. """ - path = self.project_root / INTEGRATION_JSON - if not path.is_file(): + state, error = try_read_integration_json(self.project_root) + if state is None or error is not None: return None - try: - data = json.loads(path.read_text(encoding="utf-8")) - except (json.JSONDecodeError, OSError, UnicodeDecodeError): - return None - if not isinstance(data, dict): - return None - schema = data.get("integration_state_schema") - if ( - isinstance(schema, int) - and not isinstance(schema, bool) - and schema > INTEGRATION_STATE_SCHEMA - ): - return None - return default_integration_key(normalize_integration_state(data)) + return default_integration_key(state) @staticmethod def _coerce_input( From f136b7b5cdea5ea99ae821c7558916650bbad7df Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Wed, 13 May 2026 07:42:21 +0500 Subject: [PATCH 5/5] fix(integration): distinguish missing file from non-regular path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address Copilot review on PR #2421: `try_read_integration_json` was collapsing two distinct cases into a single `(None, None)` return: 1. `.specify/integration.json` truly missing — silent fallback is correct. 2. Path exists but is a directory, socket, or other non-regular file — this is a misconfiguration the CLI should surface loudly. Split the check: `exists()` falsey returns `(None, None)`; existing-but- not-a-regular-file returns `(None, IntegrationReadError(kind="os", ...))` so the CLI's loud-fail path produces an actionable error while the engine still treats it as a fallback to the workflow's literal default. --- src/specify_cli/integration_state.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/integration_state.py b/src/specify_cli/integration_state.py index c51be47749..c95caf20a6 100644 --- a/src/specify_cli/integration_state.py +++ b/src/specify_cli/integration_state.py @@ -38,8 +38,13 @@ def try_read_integration_json( logic cannot drift between them. """ path = project_root / INTEGRATION_JSON - if not path.is_file(): + if not path.exists(): return None, None + if not path.is_file(): + return None, IntegrationReadError( + kind="os", + detail=f"{path} exists but is not a regular file", + ) try: raw = path.read_text(encoding="utf-8") except UnicodeDecodeError as exc: