From c40fb48f9453bb4c10d7e48aea7b749ff57b8169 Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah Date: Wed, 3 Jun 2026 02:32:52 -0700 Subject: [PATCH] feat: add project port health checks --- README.md | 15 +++ cli/python/base_setup/engine.py | 2 + cli/python/base_setup/health.py | 53 ++++++++ cli/python/base_setup/manifest.py | 127 +++++++++++++++++- .../base_setup/tests/test_diagnostics.py | 124 ++++++++++++++++- cli/python/base_setup/tests/test_manifest.py | 116 +++++++++++++++- docs/architecture.md | 12 ++ docs/doctor-findings.md | 1 + 8 files changed, 443 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 07bf4ee..4be74c9 100644 --- a/README.md +++ b/README.md @@ -191,6 +191,14 @@ health: required_env: - DATABASE_URL - REDIS_URL + required_ports: + - name: postgres + host: 127.0.0.1 + port: 5432 + state: listening + - name: app + port: 8000 + state: free activate: source: @@ -227,6 +235,13 @@ the project needs in the local shell. `basectl check ` and non-empty. Base only checks presence; it never reads, prints, or logs the variable values. +The optional top-level `health.required_ports` list declares local TCP ports +the project expects to be either `listening` or `free`. Each entry must include +`port` and `state`; `host` defaults to `127.0.0.1`, and `name` is an optional +display label. Base checks whether a TCP connection succeeds on the declared +endpoint. It does not start or stop services, inspect process ownership, or +perform Docker Compose health checks. + The optional top-level `activate.source` list declares project-root-relative shell scripts to source when `basectl activate ` starts the runtime shell. Base sources those scripts after the Base runtime and project virtual diff --git a/cli/python/base_setup/engine.py b/cli/python/base_setup/engine.py index 47c9472..5b5068f 100644 --- a/cli/python/base_setup/engine.py +++ b/cli/python/base_setup/engine.py @@ -24,6 +24,7 @@ from .delegates import reconcile_mise from .errors import ArtifactError from .health import check_required_env +from .health import check_required_ports from .ide import check_ide_extensions from .ide import check_ide_installs from .ide import check_ide_settings @@ -242,6 +243,7 @@ def manifest_checks(default_manifest: BaseManifest, manifest: BaseManifest) -> t checks.append(check_mise(effective_manifest)) checks.extend(check_required_env(effective_manifest)) + checks.extend(check_required_ports(effective_manifest)) checks.extend(check_demo(effective_manifest)) checks.extend(check_ide_installs(effective_manifest)) checks.extend(check_ide_extensions(effective_manifest)) diff --git a/cli/python/base_setup/health.py b/cli/python/base_setup/health.py index 7717143..2fa5244 100644 --- a/cli/python/base_setup/health.py +++ b/cli/python/base_setup/health.py @@ -1,15 +1,21 @@ from __future__ import annotations import os +import socket from .checks import ArtifactCheck from .manifest import BaseManifest +from .manifest import PortHealthConfig def check_required_env(manifest: BaseManifest) -> list[ArtifactCheck]: return [check_required_env_var(env_name) for env_name in manifest.health.required_env] +def check_required_ports(manifest: BaseManifest) -> list[ArtifactCheck]: + return [check_required_port(port_config) for port_config in manifest.health.required_ports] + + def check_required_env_var(env_name: str) -> ArtifactCheck: if os.environ.get(env_name, ""): return ArtifactCheck( @@ -26,3 +32,50 @@ def check_required_env_var(env_name: str) -> ArtifactCheck: fix=f"Set {env_name} in your shell, .env, or secrets manager.", finding_id="BASE-H001", ) + + +def check_required_port(port_config: PortHealthConfig) -> ArtifactCheck: + label = port_config.name or f"{port_config.host}:{port_config.port}" + endpoint = f"{port_config.host}:{port_config.port}" + listening = tcp_port_is_listening(port_config.host, port_config.port) + + if port_config.state == "listening": + if listening: + return ArtifactCheck( + name=label, + ok=True, + message=f"TCP port '{label}' is listening on {endpoint}.", + fix="", + finding_id="BASE-H002", + ) + return ArtifactCheck( + name=label, + ok=False, + message=f"TCP port '{label}' is not listening on {endpoint}.", + fix=f"Start the service that should listen on {endpoint}.", + finding_id="BASE-H002", + ) + + if not listening: + return ArtifactCheck( + name=label, + ok=True, + message=f"TCP port '{label}' is free on {endpoint}.", + fix="", + finding_id="BASE-H002", + ) + return ArtifactCheck( + name=label, + ok=False, + message=f"TCP port '{label}' is already listening on {endpoint}.", + fix=f"Stop the process using {endpoint} or choose a different project port.", + finding_id="BASE-H002", + ) + + +def tcp_port_is_listening(host: str, port: int, timeout_seconds: float = 0.2) -> bool: + try: + with socket.create_connection((host, port), timeout=timeout_seconds): + return True + except OSError: + return False diff --git a/cli/python/base_setup/manifest.py b/cli/python/base_setup/manifest.py index 0770594..ab20f27 100644 --- a/cli/python/base_setup/manifest.py +++ b/cli/python/base_setup/manifest.py @@ -23,6 +23,7 @@ CURRENT_MANIFEST_SCHEMA_VERSION = 1 ENVIRONMENT_VARIABLE_NAME_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") COMMAND_NAME_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_.:-]*$") +PORT_HEALTH_STATES = {"free", "listening"} class ManifestError(ValueError): @@ -56,9 +57,18 @@ class DemoConfig: description: str | None = None +@dataclass(frozen=True) +class PortHealthConfig: + port: int + state: str + name: str | None = None + host: str = "127.0.0.1" + + @dataclass(frozen=True) class HealthConfig: required_env: tuple[str, ...] = () + required_ports: tuple[PortHealthConfig, ...] = () @dataclass(frozen=True) @@ -267,12 +277,15 @@ def _read_health(path: Path, health_data: Any) -> HealthConfig: if not isinstance(health_data, dict): raise ManifestError(f"{path}: health must be a mapping when provided.") - allowed_keys = {"required_env"} + allowed_keys = {"required_env", "required_ports"} unknown_keys = sorted(set(health_data) - allowed_keys) if unknown_keys: raise ManifestError(f"{path}: health has unsupported keys: {', '.join(unknown_keys)}.") - return HealthConfig(required_env=_read_required_env(path, health_data.get("required_env", []))) + return HealthConfig( + required_env=_read_required_env(path, health_data.get("required_env", [])), + required_ports=_read_required_ports(path, health_data.get("required_ports", [])), + ) def _read_commands(path: Path, commands_data: Any) -> dict[str, str]: @@ -356,6 +369,116 @@ def _read_required_env(path: Path, required_env_data: Any) -> tuple[str, ...]: return tuple(required_env) +def _read_required_ports(path: Path, required_ports_data: Any) -> tuple[PortHealthConfig, ...]: + if required_ports_data is None: + return () + if not isinstance(required_ports_data, list): + raise ManifestError(f"{path}: health.required_ports must be a list when provided.") + + required_ports: list[PortHealthConfig] = [] + seen_endpoints: set[tuple[str, int]] = set() + seen_names: set[str] = set() + for index, port_data in enumerate(required_ports_data, start=1): + required_ports.append( + _read_required_port(path, index, port_data, seen_endpoints, seen_names) + ) + + return tuple(required_ports) + + +def _read_required_port( + path: Path, + index: int, + port_data: Any, + seen_endpoints: set[tuple[str, int]], + seen_names: set[str], +) -> PortHealthConfig: + if not isinstance(port_data, dict): + raise ManifestError(f"{path}: health.required_ports[{index}] must be a mapping.") + + allowed_keys = {"name", "host", "port", "state"} + unknown_keys = sorted(set(port_data) - allowed_keys) + if unknown_keys: + raise ManifestError( + f"{path}: health.required_ports[{index}] has unsupported keys: " + f"{', '.join(unknown_keys)}." + ) + + port = _read_required_port_number(path, index, port_data.get("port")) + state = _read_required_port_state(path, index, port_data.get("state")) + name = _read_required_port_name(path, index, port_data.get("name"), seen_names) + host = _read_required_port_host(path, index, port_data.get("host", "127.0.0.1")) + endpoint = (host, port) + if endpoint in seen_endpoints: + raise ManifestError(f"{path}: health.required_ports[{index}] duplicates '{host}:{port}'.") + seen_endpoints.add(endpoint) + return PortHealthConfig(port=port, state=state, name=name, host=host) + + +def _read_required_port_number(path: Path, index: int, port_data: Any) -> int: + if isinstance(port_data, bool) or not isinstance(port_data, int): + raise ManifestError(f"{path}: health.required_ports[{index}].port must be an integer.") + if port_data < 1 or port_data > 65535: + raise ManifestError( + f"{path}: health.required_ports[{index}].port must be between 1 and 65535." + ) + return port_data + + +def _read_required_port_state(path: Path, index: int, state_data: Any) -> str: + if not isinstance(state_data, str) or not state_data.strip(): + raise ManifestError( + f"{path}: health.required_ports[{index}].state must be a non-empty string." + ) + state = state_data.strip() + if state not in PORT_HEALTH_STATES: + supported_states = ", ".join(sorted(PORT_HEALTH_STATES)) + raise ManifestError( + f"{path}: health.required_ports[{index}].state must be one of: {supported_states}." + ) + return state + + +def _read_required_port_name( + path: Path, + index: int, + name_data: Any, + seen_names: set[str], +) -> str | None: + if name_data is None: + return None + if not isinstance(name_data, str) or not name_data.strip(): + raise ManifestError( + f"{path}: health.required_ports[{index}].name must be a non-empty string." + ) + name = name_data.strip() + if _has_control_line_break(name): + raise ManifestError( + f"{path}: health.required_ports[{index}].name must not contain control line breaks." + ) + if name in seen_names: + raise ManifestError(f"{path}: health.required_ports[{index}].name duplicates '{name}'.") + seen_names.add(name) + return name + + +def _read_required_port_host(path: Path, index: int, host_data: Any) -> str: + if not isinstance(host_data, str) or not host_data.strip(): + raise ManifestError( + f"{path}: health.required_ports[{index}].host must be a non-empty string." + ) + host = host_data.strip() + if _has_control_line_break(host): + raise ManifestError( + f"{path}: health.required_ports[{index}].host must not contain control line breaks." + ) + return host + + +def _has_control_line_break(value: str) -> bool: + return any(separator in value for separator in ("\0", "\n", "\r")) + + def _read_ide_config(path: Path, ide_name: str, config_data: Any) -> IdeConfig: if config_data is None: config_data = {} diff --git a/cli/python/base_setup/tests/test_diagnostics.py b/cli/python/base_setup/tests/test_diagnostics.py index 09b9fb6..8f3e4f2 100644 --- a/cli/python/base_setup/tests/test_diagnostics.py +++ b/cli/python/base_setup/tests/test_diagnostics.py @@ -4,14 +4,20 @@ import io import json import os +import socket import tempfile import unittest from contextlib import redirect_stdout from pathlib import Path from unittest import mock -from base_setup import artifacts, checks as setup_checks, engine, ide -from base_setup.manifest import ArtifactRequest, BaseManifest, HealthConfig, IdeConfig, read_manifest +from base_setup import artifacts, checks as setup_checks, engine, health, ide +from base_setup.manifest import ArtifactRequest +from base_setup.manifest import BaseManifest +from base_setup.manifest import HealthConfig +from base_setup.manifest import IdeConfig +from base_setup.manifest import PortHealthConfig +from base_setup.manifest import read_manifest from base_setup.registry import get_artifact_definition from base_setup.tests.helpers import fake_context, run_engine @@ -171,7 +177,12 @@ def test_check_manifest_reports_required_environment_variables(self) -> None: os.environ.pop("BASE_TEST_REQUIRED_MISSING", None) stdout = io.StringIO() with redirect_stdout(stdout): - status = engine.check_manifest(fake_context(), default_manifest, manifest, output_format="json") + status = engine.check_manifest( + fake_context(), + default_manifest, + manifest, + output_format="json", + ) output = stdout.getvalue() parsed_checks = json.loads(output) @@ -189,6 +200,73 @@ def test_check_manifest_reports_required_environment_variables(self) -> None: self.assertNotIn("super-secret-value", output) + def test_check_manifest_reports_required_ports(self) -> None: + default_manifest = BaseManifest( + path=Path("default_manifest.yaml"), + project_name="base-defaults", + brewfile=None, + artifacts=(), + ) + manifest = BaseManifest( + path=Path("base_manifest.yaml"), + project_name="demo", + brewfile=None, + artifacts=(), + health=HealthConfig( + required_ports=( + PortHealthConfig( + name="postgres", + host="127.0.0.1", + port=5432, + state="listening", + ), + PortHealthConfig(name="app", host="127.0.0.1", port=8000, state="free"), + PortHealthConfig(name="busy-app", host="127.0.0.1", port=9000, state="free"), + ) + ), + ) + + def fake_port_probe(_host: str, port: int) -> bool: + return port in (5432, 9000) + + with mock.patch("base_setup.health.tcp_port_is_listening", side_effect=fake_port_probe): + stdout = io.StringIO() + with redirect_stdout(stdout): + status = engine.check_manifest( + fake_context(), + default_manifest, + manifest, + output_format="json", + ) + + parsed_checks = json.loads(stdout.getvalue()) + self.assertEqual(status, 1) + self.assertEqual( + [check["name"] for check in parsed_checks], + ["postgres", "app", "busy-app"], + ) + self.assertTrue(parsed_checks[0]["ok"]) + self.assertTrue(parsed_checks[1]["ok"]) + self.assertFalse(parsed_checks[2]["ok"]) + self.assertIn("already listening", parsed_checks[2]["message"]) + self.assertEqual( + parsed_checks[2]["fix"], + "Stop the process using 127.0.0.1:9000 or choose a different project port.", + ) + + + def test_tcp_port_probe_reports_listening_socket(self) -> None: + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as listener: + try: + listener.bind(("127.0.0.1", 0)) + except PermissionError as exc: + self.skipTest(f"Loopback socket bind is not permitted: {exc}") + listener.listen(1) + port = listener.getsockname()[1] + + self.assertTrue(health.tcp_port_is_listening("127.0.0.1", port)) + + def test_doctor_manifest_reports_required_environment_variables_without_values(self) -> None: default_manifest = BaseManifest( @@ -226,6 +304,46 @@ def test_doctor_manifest_reports_required_environment_variables_without_values(s self.assertNotIn("another-secret-value", output) + def test_doctor_manifest_reports_required_ports_with_finding_ids(self) -> None: + default_manifest = BaseManifest( + path=Path("default_manifest.yaml"), + project_name="base-defaults", + brewfile=None, + artifacts=(), + ) + manifest = BaseManifest( + path=Path("base_manifest.yaml"), + project_name="demo", + brewfile=None, + artifacts=(), + health=HealthConfig( + required_ports=( + PortHealthConfig(name="postgres", port=5432, state="listening"), + PortHealthConfig(name="app", port=8000, state="free"), + ) + ), + ) + + with mock.patch("base_setup.health.tcp_port_is_listening", return_value=False): + with redirect_stdout(io.StringIO()) as stdout: + status = engine.doctor_manifest(default_manifest, manifest, output_format="json") + + findings = json.loads(stdout.getvalue()) + self.assertEqual(status, 1) + self.assertEqual( + [finding["id"] for finding in findings], + ["BASE-H002", "BASE-H002"], + ) + self.assertEqual( + [finding["status"] for finding in findings], + ["error", "ok"], + ) + self.assertEqual( + findings[0]["fix"], + "Start the service that should listen on 127.0.0.1:5432.", + ) + + class IdeDiagnosticsTests(unittest.TestCase): diff --git a/cli/python/base_setup/tests/test_manifest.py b/cli/python/base_setup/tests/test_manifest.py index 3353d87..bbf7acf 100644 --- a/cli/python/base_setup/tests/test_manifest.py +++ b/cli/python/base_setup/tests/test_manifest.py @@ -155,7 +155,6 @@ def test_reads_manifest_required_environment_variables(self) -> None: self.assertEqual(manifest.health.required_env, ("DATABASE_URL", "REDIS_URL")) - def test_reads_manifest_activation_sources(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: manifest_path = Path(tmpdir) / "base_manifest.yaml" @@ -299,7 +298,6 @@ def test_rejects_invalid_manifest_required_environment_variables(self) -> None: read_manifest(manifest_path) - def test_reads_manifest_commands(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: manifest_path = Path(tmpdir) / "base_manifest.yaml" @@ -650,3 +648,117 @@ def test_rejects_unsupported_auto_ide_setting(self) -> None: with self.assertRaisesRegex(ManifestError, "does not support the special value 'auto'"): read_manifest(manifest_path) + + +class HealthPortManifestParsingTests(unittest.TestCase): + + def test_reads_manifest_required_ports(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + manifest_path = Path(tmpdir) / "base_manifest.yaml" + manifest_path.write_text( + "\n".join( + [ + "project:", + " name: demo", + "", + "health:", + " required_ports:", + " - name: postgres", + " host: 127.0.0.1", + " port: 5432", + " state: listening", + " - port: 8000", + " state: free", + "", + "artifacts: []", + ] + ), + encoding="utf-8", + ) + + manifest = read_manifest(manifest_path) + + self.assertEqual(len(manifest.health.required_ports), 2) + self.assertEqual(manifest.health.required_ports[0].name, "postgres") + self.assertEqual(manifest.health.required_ports[0].host, "127.0.0.1") + self.assertEqual(manifest.health.required_ports[0].port, 5432) + self.assertEqual(manifest.health.required_ports[0].state, "listening") + self.assertEqual(manifest.health.required_ports[1].name, None) + self.assertEqual(manifest.health.required_ports[1].host, "127.0.0.1") + self.assertEqual(manifest.health.required_ports[1].port, 8000) + self.assertEqual(manifest.health.required_ports[1].state, "free") + + + def test_rejects_invalid_manifest_required_ports(self) -> None: + invalid_values = { + "scalar": "health:\n required_ports: 5432", + "integer_entry": "health:\n required_ports:\n - 5432", + "unknown_key": ( + "health:\n" + " required_ports:\n" + " - port: 5432\n" + " state: listening\n" + " protocol: udp" + ), + "missing_port": "health:\n required_ports:\n - state: listening", + "bool_port": "health:\n required_ports:\n - port: true\n state: listening", + "low_port": "health:\n required_ports:\n - port: 0\n state: listening", + "high_port": "health:\n required_ports:\n - port: 65536\n state: listening", + "missing_state": "health:\n required_ports:\n - port: 5432", + "unsupported_state": ( + "health:\n" + " required_ports:\n" + " - port: 5432\n" + " state: occupied" + ), + "empty_name": ( + "health:\n" + " required_ports:\n" + " - name: ''\n" + " port: 5432\n" + " state: listening" + ), + "duplicate_name": ( + "health:\n" + " required_ports:\n" + " - name: db\n" + " port: 5432\n" + " state: listening\n" + " - name: db\n" + " port: 6379\n" + " state: listening" + ), + "empty_host": ( + "health:\n" + " required_ports:\n" + " - host: ''\n" + " port: 5432\n" + " state: listening" + ), + "duplicate_endpoint": ( + "health:\n" + " required_ports:\n" + " - port: 5432\n" + " state: listening\n" + " - port: 5432\n" + " state: free" + ), + } + for name, health_yaml in invalid_values.items(): + with self.subTest(name=name): + with tempfile.TemporaryDirectory() as tmpdir: + manifest_path = Path(tmpdir) / "base_manifest.yaml" + manifest_path.write_text( + "\n".join( + [ + "project:", + " name: demo", + health_yaml, + "artifacts: []", + ] + ), + encoding="utf-8", + ) + + with self.assertRaises(ManifestError): + read_manifest(manifest_path) diff --git a/docs/architecture.md b/docs/architecture.md index 3d8dff7..b08f6f3 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -374,6 +374,14 @@ health: required_env: - DATABASE_URL - REDIS_URL + required_ports: + - name: postgres + host: 127.0.0.1 + port: 5432 + state: listening + - name: app + port: 8000 + state: free activate: source: @@ -416,6 +424,10 @@ orchestration actions. The design rule is delegation-first: top-level `test` contract. - Use `health.required_env` for local environment contracts that `basectl check` and `basectl doctor` should validate without exposing secret values. +- Use `health.required_ports` for local TCP port contracts that should be + explicitly `listening` or `free`. Base checks connection state only; it does + not mutate local services, inspect process ownership, or replace future + Docker Compose health checks. - Use `activate.source` for explicit project activation scripts that need to affect the interactive runtime shell, such as local environment loading, aliases, or functions. Source paths must be relative to the project root and diff --git a/docs/doctor-findings.md b/docs/doctor-findings.md index b0bb827..26de4dd 100644 --- a/docs/doctor-findings.md +++ b/docs/doctor-findings.md @@ -81,3 +81,4 @@ part of the doctor workflow. | ID | Finding | | --- | --- | | `BASE-H001` | Required environment variable presence | +| `BASE-H002` | Required TCP port listening/free state |