From 3e1a1bb2b92fd291a6090f7a28fb5e84ad4f0a9e Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Tue, 6 Oct 2026 15:43:29 -0500 Subject: [PATCH] refactor(version): extract shared operation Move version, runtime, platform, and capability collection into a transport-neutral typed operation while preserving the existing CLI adapter contract and patch points. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/_operation_version.py | 97 +++++++++++++++ src/specify_cli/command_version.py | 97 ++++++++------- tests/specify_cli/test_command_version.py | 42 +++++++ tests/specify_cli/test_operation_version.py | 130 ++++++++++++++++++++ 4 files changed, 324 insertions(+), 42 deletions(-) create mode 100644 src/specify_cli/_operation_version.py create mode 100644 tests/specify_cli/test_operation_version.py diff --git a/src/specify_cli/_operation_version.py b/src/specify_cli/_operation_version.py new file mode 100644 index 0000000000..97318b6dda --- /dev/null +++ b/src/specify_cli/_operation_version.py @@ -0,0 +1,97 @@ +"""Shared application operation for version information collection.""" + +from __future__ import annotations + +import platform +from collections.abc import Callable +from dataclasses import dataclass + +from ._assets import get_speckit_version + + +@dataclass(frozen=True) +class VersionRuntime: + """Runtime versions reported by the version operation.""" + + python: str + openssl: str | None + + +@dataclass(frozen=True) +class VersionSystem: + """Host system information reported by the version operation.""" + + platform: str + architecture: str + os_version: str + + +@dataclass(frozen=True) +class VersionResult: + """Transport-neutral result of the version operation.""" + + cli_version: str + runtime: VersionRuntime | None + system: VersionSystem | None + features: dict[str, bool] + + +def _feature_capabilities() -> dict[str, bool]: + """Return stable local CLI capability flags for humans and agents.""" + return { + "controlled_multi_install_integrations": True, + "integration_use_command": True, + "multi_install_safe_registry_metadata": True, + "integration_upgrade_command": True, + "self_check_command": True, + "workflow_catalog": True, + "bundled_templates": True, + } + + +def _openssl_version() -> str | None: + """Return the loaded OpenSSL version, or None when unavailable.""" + try: + import ssl + except ImportError: + return None + + value = getattr(ssl, "OPENSSL_VERSION", None) + return value if isinstance(value, str) and value else None + + +def collect_version_result( + *, + include_environment: bool = True, + cli_version_getter: Callable[[], str] | None = None, + openssl_version_getter: Callable[[], str | None] | None = None, + feature_capabilities_getter: Callable[[], dict[str, bool]] | None = None, +) -> VersionResult: + """Collect semantic version information for any delivery adapter.""" + get_cli_version = cli_version_getter or get_speckit_version + get_openssl_version = openssl_version_getter or _openssl_version + get_feature_capabilities = feature_capabilities_getter or _feature_capabilities + + cli_version = get_cli_version() + features = get_feature_capabilities() + if not include_environment: + return VersionResult( + cli_version=cli_version, + runtime=None, + system=None, + features=features, + ) + + return VersionResult( + cli_version=cli_version, + runtime=VersionRuntime( + python=platform.python_version(), + openssl=get_openssl_version(), + ), + system=VersionSystem( + platform=platform.system(), + architecture=platform.machine(), + os_version=platform.version(), + ), + features=features, + ) diff --git a/src/specify_cli/command_version.py b/src/specify_cli/command_version.py index c0ae9efba8..976fa785b8 100644 --- a/src/specify_cli/command_version.py +++ b/src/specify_cli/command_version.py @@ -3,55 +3,52 @@ from __future__ import annotations import json -import platform import typer from rich.panel import Panel from rich.table import Table +from . import _operation_version from ._console import console, show_banner +from ._operation_version import ( + VersionResult, + _feature_capabilities, + _openssl_version, + collect_version_result, +) -_INTERNAL_ERROR_MESSAGE = "Unable to collect version information." - +platform = _operation_version.platform -def _feature_capabilities() -> dict[str, bool]: - """Return stable local CLI capability flags for humans and agents.""" - return { - "controlled_multi_install_integrations": True, - "integration_use_command": True, - "multi_install_safe_registry_metadata": True, - "integration_upgrade_command": True, - "self_check_command": True, - "workflow_catalog": True, - "bundled_templates": True, - } +_INTERNAL_ERROR_MESSAGE = "Unable to collect version information." -def _openssl_version() -> str | None: - """Return the loaded OpenSSL version, or None when unavailable.""" - try: - import ssl - except ImportError: - return None +def _json_result(cli_version: str) -> dict[str, object]: + """Collect the complete machine-readable version result.""" + result = collect_version_result( + cli_version_getter=lambda: cli_version, + openssl_version_getter=_openssl_version, + feature_capabilities_getter=_feature_capabilities, + ) + return _result_payload(result) - value = getattr(ssl, "OPENSSL_VERSION", None) - return value if isinstance(value, str) and value else None +def _result_payload(result: VersionResult) -> dict[str, object]: + """Map a complete operation result to the established CLI JSON shape.""" + if result.runtime is None or result.system is None: + raise ValueError("Complete version information is required.") -def _json_result(cli_version: str) -> dict[str, object]: - """Collect the complete machine-readable version result.""" return { - "cli_version": cli_version, + "cli_version": result.cli_version, "runtime": { - "python": platform.python_version(), - "openssl": _openssl_version(), + "python": result.runtime.python, + "openssl": result.runtime.openssl, }, "system": { - "platform": platform.system(), - "architecture": platform.machine(), - "os_version": platform.version(), + "platform": result.system.platform, + "architecture": result.system.architecture, + "os_version": result.system.os_version, }, - "features": _feature_capabilities(), + "features": result.features, } @@ -77,7 +74,12 @@ def version( if json_output: try: - payload = _json_result(get_speckit_version()) + result = collect_version_result( + cli_version_getter=get_speckit_version, + openssl_version_getter=_openssl_version, + feature_capabilities_getter=_feature_capabilities, + ) + payload = _result_payload(result) rendered = _serialize_json(payload) except Exception: # noqa: BLE001 # JSON mode must normalize every unexpected command failure. @@ -94,34 +96,45 @@ def version( typer.echo(rendered) return - cli_version = get_speckit_version() if features: - capabilities = _feature_capabilities() - console.print(f"Spec Kit CLI: {cli_version}") + result = collect_version_result( + include_environment=False, + cli_version_getter=get_speckit_version, + feature_capabilities_getter=_feature_capabilities, + ) + console.print(f"Spec Kit CLI: {result.cli_version}") console.print() console.print("Features:") - for key, enabled in capabilities.items(): + for key, enabled in result.features.items(): label = key.replace("_", " ") console.print(f"- {label}: {'yes' if enabled else 'no'}") return + result = collect_version_result( + cli_version_getter=get_speckit_version, + openssl_version_getter=_openssl_version, + feature_capabilities_getter=_feature_capabilities, + ) + if result.runtime is None or result.system is None: + raise RuntimeError("Complete version information is required.") + show_banner() info_table = Table(show_header=False, box=None, padding=(0, 2)) info_table.add_column("Key", style="cyan", justify="right") info_table.add_column("Value", style="white") - info_table.add_row("CLI Version", cli_version) + info_table.add_row("CLI Version", result.cli_version) info_table.add_row("", "") - info_table.add_row("Python", platform.python_version()) - info_table.add_row("Platform", platform.system()) - info_table.add_row("Architecture", platform.machine()) - info_table.add_row("OS Version", platform.version()) + info_table.add_row("Python", result.runtime.python) + info_table.add_row("Platform", result.system.platform) + info_table.add_row("Architecture", result.system.architecture) + info_table.add_row("OS Version", result.system.os_version) # The OpenSSL runtime the interpreter actually loaded. HTTPS failure # reports (#4433) hinge on which OpenSSL is in play, and on Windows it is # not obvious from the outside, so surface it here. An interpreter built # without the ssl extension skips the row rather than failing the command. - openssl_version = _openssl_version() + openssl_version = result.runtime.openssl if openssl_version: info_table.add_row("OpenSSL", openssl_version) diff --git a/tests/specify_cli/test_command_version.py b/tests/specify_cli/test_command_version.py index d249b3c354..f1e6437bf5 100644 --- a/tests/specify_cli/test_command_version.py +++ b/tests/specify_cli/test_command_version.py @@ -9,6 +9,11 @@ from typer.testing import CliRunner from specify_cli import app +from specify_cli._operation_version import ( + VersionResult, + VersionRuntime, + VersionSystem, +) runner = CliRunner() @@ -83,6 +88,43 @@ def test_version_json_emits_complete_stable_payload(self): assert result.stderr == "" assert json.loads(result.stdout) == expected + def test_version_json_renders_shared_operation_result(self): + """The CLI adapter maps the shared typed result to its JSON contract.""" + operation_result = VersionResult( + cli_version="1.2.3", + runtime=VersionRuntime( + python="3.13.1", + openssl="OpenSSL 3.4.0", + ), + system=VersionSystem( + platform="ExampleOS", + architecture="example64", + os_version="ExampleOS 4.5", + ), + features=EXPECTED_FEATURES, + ) + with patch( + "specify_cli.command_version.collect_version_result", + return_value=operation_result, + ) as collect: + result = runner.invoke(app, ["version", "--json"]) + + assert result.exit_code == 0 + assert json.loads(result.stdout) == { + "cli_version": operation_result.cli_version, + "runtime": { + "python": operation_result.runtime.python, + "openssl": operation_result.runtime.openssl, + }, + "system": { + "platform": operation_result.system.platform, + "architecture": operation_result.system.architecture, + "os_version": operation_result.system.os_version, + }, + "features": operation_result.features, + } + collect.assert_called_once() + def test_version_features_json_is_exact_alias(self): """--features does not filter JSON output or change its bytes.""" with patch("specify_cli.get_speckit_version", return_value="1.2.3"): diff --git a/tests/specify_cli/test_operation_version.py b/tests/specify_cli/test_operation_version.py new file mode 100644 index 0000000000..802d627b8a --- /dev/null +++ b/tests/specify_cli/test_operation_version.py @@ -0,0 +1,130 @@ +"""Tests for the shared version operation.""" + +import sys +from types import SimpleNamespace +from unittest.mock import Mock, patch + +from specify_cli import _operation_version +from specify_cli._operation_version import ( + VersionResult, + VersionRuntime, + VersionSystem, + collect_version_result, +) + +EXPECTED_FEATURES = { + "controlled_multi_install_integrations": True, + "integration_use_command": True, + "multi_install_safe_registry_metadata": True, + "integration_upgrade_command": True, + "self_check_command": True, + "workflow_catalog": True, + "bundled_templates": True, +} + + +def test_collect_version_result_returns_complete_typed_result(): + """The operation owns complete semantic version collection.""" + with ( + patch( + "specify_cli._operation_version.get_speckit_version", + return_value="1.2.3", + ), + patch( + "specify_cli._operation_version.platform.python_version", + return_value="3.13.1", + ), + patch( + "specify_cli._operation_version.platform.system", + return_value="ExampleOS", + ), + patch( + "specify_cli._operation_version.platform.machine", + return_value="example64", + ), + patch( + "specify_cli._operation_version.platform.version", + return_value="ExampleOS 4.5", + ), + patch( + "specify_cli._operation_version._openssl_version", + return_value="OpenSSL 3.4.0", + ), + ): + result = collect_version_result() + + assert result == VersionResult( + cli_version="1.2.3", + runtime=VersionRuntime( + python="3.13.1", + openssl="OpenSSL 3.4.0", + ), + system=VersionSystem( + platform="ExampleOS", + architecture="example64", + os_version="ExampleOS 4.5", + ), + features=EXPECTED_FEATURES, + ) + + +def test_openssl_version_returns_loaded_runtime(monkeypatch): + """The operation reports the OpenSSL runtime loaded by Python.""" + monkeypatch.setitem( + sys.modules, + "ssl", + SimpleNamespace(OPENSSL_VERSION="OpenSSL test"), + ) + + assert _operation_version._openssl_version() == "OpenSSL test" + + +def test_openssl_version_returns_none_when_unavailable(monkeypatch): + """The operation treats a missing ssl module as unavailable.""" + monkeypatch.setitem(sys.modules, "ssl", None) + + assert _operation_version._openssl_version() is None + + +def test_feature_capabilities_are_stable_and_ordered(): + """Feature names and order remain part of the semantic result.""" + result = collect_version_result( + include_environment=False, + cli_version_getter=lambda: "1.2.3", + ) + + assert result.features == EXPECTED_FEATURES + assert list(result.features) == list(EXPECTED_FEATURES) + assert result.runtime is None + assert result.system is None + + +def test_feature_only_collection_skips_environment_probes(): + """Focused capability collection does not touch runtime or system probes.""" + openssl_version_getter = Mock(side_effect=AssertionError("must not run")) + with ( + patch( + "specify_cli._operation_version.platform.python_version", + side_effect=AssertionError("must not run"), + ), + patch( + "specify_cli._operation_version.platform.system", + side_effect=AssertionError("must not run"), + ), + ): + result = collect_version_result( + include_environment=False, + cli_version_getter=lambda: "1.2.3", + openssl_version_getter=openssl_version_getter, + ) + + assert result.cli_version == "1.2.3" + openssl_version_getter.assert_not_called() + + +def test_operation_contract_is_transport_neutral(): + """The operation module exposes semantic data without adapter dependencies.""" + forbidden_names = {"typer", "rich", "mcp"} + + assert forbidden_names.isdisjoint(_operation_version.__dict__) + assert VersionResult.__module__ == "specify_cli._operation_version"