feat(preflight): replace SUPPORTED_RUNTIMES static list with adapter discovery
Closes task #123 — last piece of #87 cleanup. Pre-fix: workspace/preflight.py:11 hardcoded a tuple of "supported" runtime names (claude-code, codex, ollama, langgraph, etc.). Every new template repo required a code change in molecule-runtime to be recognized — direct violation of the universal-runtime principle (#87) where adapters declare themselves and the runtime stays generic. Post-fix: discovery-based validation via the same ADAPTER_MODULE env var that production load paths already consult (workspace/adapters/__init__.py:get_adapter). Distinguished failure modes so operator messages are concrete: - ADAPTER_MODULE unset → "no adapter installed; set the env var" - ADAPTER_MODULE set but module won't import → import error type + message - module imports but no Adapter class → "convention violation, add `Adapter = YourClass`" - Adapter.name() raises → caught with operator message - Adapter.name() returns non-string → contract violation message - Adapter.name() doesn't match config.runtime → drift WARNING (not fatal; the adapter wins in production, config.yaml is just documentation) The drift case is the one behavioral change worth calling out: the prior static-list path would have hard-failed config.runtime values not in the allowlist. With discovery, an unknown runtime in config.yaml is just a documentation drift — the adapter that's actually installed runs regardless. Operator gets a warning naming both the configured and installed names so they can fix whichever is stale. Tests: - Replaces the obsolete "static list pass/fail" tests with 6 new cases covering each distinguished failure mode, plus a positive test for the adapter-matches-config happy path - Adds an autouse `_default_langgraph_adapter` fixture that pre-installs a fake adapter via sys.modules monkey-patching, so existing tests building default WorkspaceConfig (runtime="langgraph") inherit a valid adapter without each test setting ADAPTER_MODULE - Failure-mode tests opt out of the default fixture via @pytest.mark.no_default_adapter (registered in pytest.ini) - Sentinel pattern (`_UNSET = object()`) for `name_returns` so None is a passable test value (otherwise `is not None` would skip the None branch — exact bug the sentinel avoids) Verification: - 22/22 preflight tests pass (was 16; +6 new failure-path tests) - 1256/1256 workspace pytest pass (was 1251; +5 net) - No production code path other than preflight changed Source: 2026-04-27 #87 cleanup audit after PR #2154 (wedge extraction). This change is independent of the cli_executor.py template moves (task #122) — completes one of the two remaining cleanup items. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Hongming Wang
parent
66b9c04057
commit
7dba700ac3
@ -1,22 +1,82 @@
|
||||
"""Startup preflight checks for workspace runtime configs."""
|
||||
|
||||
import importlib
|
||||
import os
|
||||
from dataclasses import dataclass, field
|
||||
from pathlib import Path
|
||||
|
||||
from config import WorkspaceConfig
|
||||
|
||||
SUPPORTED_RUNTIMES = {
|
||||
"langgraph",
|
||||
"claude-code",
|
||||
"codex",
|
||||
"ollama",
|
||||
"custom",
|
||||
"crewai",
|
||||
"autogen",
|
||||
"deepagents",
|
||||
"openclaw",
|
||||
}
|
||||
|
||||
def _validate_runtime_via_adapter(runtime: str) -> tuple[bool, str]:
|
||||
"""Discover the installed adapter and confirm it matches the
|
||||
config's `runtime` field. Returns (ok, detail) — detail is the
|
||||
operator-actionable failure message when ok is False.
|
||||
|
||||
Replaces the previous hardcoded SUPPORTED_RUNTIMES allowlist
|
||||
(claude-code / codex / ollama / langgraph / etc.). The static list
|
||||
couldn't keep up with new template repos: each new adapter required
|
||||
a code change in molecule-runtime to be 'supported', a violation of
|
||||
the universal-runtime principle (#87).
|
||||
|
||||
Discovery uses the same ADAPTER_MODULE env var that production load
|
||||
paths consult (workspace/adapters/__init__.py:get_adapter). The
|
||||
adapter's static name() string is the source of truth — config.yaml
|
||||
just labels which one the operator expects, and the check warns on
|
||||
drift.
|
||||
|
||||
Failure modes the function distinguishes (each gets a distinct
|
||||
operator-facing message so debugging is concrete):
|
||||
- ADAPTER_MODULE unset → "no adapter installed"
|
||||
- ADAPTER_MODULE set but module won't import → "import failed: …"
|
||||
- module imports but no Adapter class → "Adapter class missing"
|
||||
- Adapter.name() differs from config.runtime → drift warning
|
||||
"""
|
||||
adapter_module = os.environ.get("ADAPTER_MODULE", "").strip()
|
||||
if not adapter_module:
|
||||
return False, (
|
||||
"ADAPTER_MODULE env var is unset — no adapter installed in this "
|
||||
f"image. Workspace declares runtime='{runtime}' but the runtime "
|
||||
"discovery path can't find any. In a template image this is set "
|
||||
"in the Dockerfile (ENV ADAPTER_MODULE=adapter); in dev, set it "
|
||||
"to your local adapter module name."
|
||||
)
|
||||
try:
|
||||
mod = importlib.import_module(adapter_module)
|
||||
except Exception as exc:
|
||||
return False, (
|
||||
f"ADAPTER_MODULE={adapter_module!r} is not importable: "
|
||||
f"{type(exc).__name__}: {exc}. Check the module path + that its "
|
||||
"dependencies installed cleanly."
|
||||
)
|
||||
adapter_cls = getattr(mod, "Adapter", None)
|
||||
if adapter_cls is None:
|
||||
return False, (
|
||||
f"ADAPTER_MODULE={adapter_module!r} imported, but no `Adapter` "
|
||||
"class is exported. Add `Adapter = YourAdapterClass` at module "
|
||||
"scope (convention from BaseAdapter docstring)."
|
||||
)
|
||||
try:
|
||||
adapter_name = adapter_cls.name()
|
||||
except Exception as exc:
|
||||
return False, (
|
||||
f"Adapter.name() raised {type(exc).__name__}: {exc}. The static "
|
||||
"name() classmethod must return the runtime identifier without "
|
||||
"side effects."
|
||||
)
|
||||
if not isinstance(adapter_name, str) or not adapter_name:
|
||||
return False, "Adapter.name() must return a non-empty string."
|
||||
if adapter_name != runtime:
|
||||
# Drift between config.yaml and the installed adapter is unusual
|
||||
# but not fatal — the adapter wins (it's what actually runs).
|
||||
# Operator-facing detail names both so they can fix whichever is
|
||||
# stale.
|
||||
return True, (
|
||||
f"Drift: config.yaml runtime={runtime!r} but installed Adapter "
|
||||
f"reports name={adapter_name!r}. The adapter wins; update "
|
||||
"config.yaml to match if the drift is unintended."
|
||||
)
|
||||
return True, ""
|
||||
|
||||
|
||||
@dataclass
|
||||
@ -42,13 +102,28 @@ def run_preflight(config: WorkspaceConfig, config_path: str) -> PreflightReport:
|
||||
report = PreflightReport()
|
||||
config_dir = Path(config_path)
|
||||
|
||||
if config.runtime not in SUPPORTED_RUNTIMES:
|
||||
runtime_ok, runtime_detail = _validate_runtime_via_adapter(config.runtime)
|
||||
if not runtime_ok:
|
||||
report.failures.append(
|
||||
PreflightIssue(
|
||||
severity="fail",
|
||||
title="Runtime",
|
||||
detail=f"Unsupported runtime '{config.runtime}'",
|
||||
fix="Choose one of the supported runtimes or install the matching adapter.",
|
||||
detail=runtime_detail,
|
||||
fix=(
|
||||
"Install the matching adapter (template repo's Dockerfile "
|
||||
"should set ADAPTER_MODULE) or correct the runtime field in "
|
||||
"config.yaml."
|
||||
),
|
||||
)
|
||||
)
|
||||
elif runtime_detail:
|
||||
# ok=True with a detail = drift warning, not a failure.
|
||||
report.warnings.append(
|
||||
PreflightIssue(
|
||||
severity="warn",
|
||||
title="Runtime",
|
||||
detail=runtime_detail,
|
||||
fix="Update config.yaml runtime to match the installed Adapter.name().",
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
@ -21,6 +21,8 @@ addopts =
|
||||
--cov=.
|
||||
--cov-report=term-missing
|
||||
--cov-fail-under=86
|
||||
markers =
|
||||
no_default_adapter: opt out of preflight tests' autouse fake-adapter fixture (for tests that exercise the no-adapter / broken-adapter failure paths)
|
||||
# Coverage omit / report config lives in workspace/.coveragerc — coverage.py
|
||||
# only reads .coveragerc / setup.cfg / tox.ini / pyproject.toml, NOT
|
||||
# pytest.ini, so [coverage:*] sections here would be silently ignored.
|
||||
|
||||
@ -1,4 +1,8 @@
|
||||
"""Tests for preflight.py — workspace startup checks."""
|
||||
import sys
|
||||
import types
|
||||
|
||||
import pytest
|
||||
|
||||
from config import A2AConfig, RuntimeConfig, WorkspaceConfig
|
||||
from preflight import run_preflight, render_preflight_report, PreflightIssue, PreflightReport
|
||||
@ -19,16 +23,77 @@ def make_config(**overrides):
|
||||
return base
|
||||
|
||||
|
||||
def test_run_preflight_supported_runtime_passes(tmp_path):
|
||||
"""A supported runtime with present files should pass cleanly."""
|
||||
_UNSET = object()
|
||||
|
||||
|
||||
def install_fake_adapter(monkeypatch, name: str = "langgraph", *, raise_on_name: bool = False, no_class: bool = False, name_returns=_UNSET):
|
||||
"""Install a fake adapter module + ADAPTER_MODULE env var so the
|
||||
runtime-discovery path in preflight finds it.
|
||||
|
||||
Args:
|
||||
name: what Adapter.name() returns (default "langgraph" so the
|
||||
base config's runtime field passes the equality check).
|
||||
raise_on_name: if True, Adapter.name() raises (tests the catch path).
|
||||
no_class: if True, the module imports but exports no Adapter symbol.
|
||||
name_returns: override the literal value name() returns. Defaults
|
||||
to a sentinel so that None is a passable test value
|
||||
(else `if name_returns is not None` would skip the
|
||||
None branch — exactly the bug this sentinel avoids).
|
||||
"""
|
||||
# Each call uses a unique module name so monkeypatch's sys.modules
|
||||
# restoration doesn't accidentally reuse a prior test's fake when
|
||||
# the same `name` is requested twice in one test session.
|
||||
module_name = f"_fake_adapter_{name.replace('-', '_')}_{id(monkeypatch)}"
|
||||
fake_mod = types.ModuleType(module_name)
|
||||
|
||||
if not no_class:
|
||||
if raise_on_name:
|
||||
class _Adapter:
|
||||
@staticmethod
|
||||
def name():
|
||||
raise RuntimeError("boom")
|
||||
elif name_returns is not _UNSET:
|
||||
class _Adapter:
|
||||
@staticmethod
|
||||
def name():
|
||||
return name_returns
|
||||
else:
|
||||
class _Adapter:
|
||||
@staticmethod
|
||||
def name():
|
||||
return name
|
||||
fake_mod.Adapter = _Adapter
|
||||
|
||||
monkeypatch.setitem(sys.modules, module_name, fake_mod)
|
||||
monkeypatch.setenv("ADAPTER_MODULE", module_name)
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _default_langgraph_adapter(monkeypatch, request):
|
||||
"""Pre-install a langgraph adapter so existing tests that build a
|
||||
default WorkspaceConfig (runtime="langgraph") pass the discovery
|
||||
check without each test having to set ADAPTER_MODULE manually.
|
||||
|
||||
Tests that need to assert a specific failure mode (no adapter, drift,
|
||||
missing class, etc.) opt out via the `no_default_adapter` marker:
|
||||
|
||||
@pytest.mark.no_default_adapter
|
||||
def test_…(monkeypatch):
|
||||
...
|
||||
"""
|
||||
if "no_default_adapter" in request.keywords:
|
||||
return
|
||||
install_fake_adapter(monkeypatch, name="langgraph")
|
||||
|
||||
|
||||
def test_run_preflight_with_matching_adapter_passes(tmp_path):
|
||||
"""When ADAPTER_MODULE points to a module whose Adapter.name()
|
||||
matches config.runtime, preflight passes cleanly. Default fixture
|
||||
installs a langgraph adapter; the base config also says langgraph."""
|
||||
(tmp_path / "system-prompt.md").write_text("Base prompt.")
|
||||
(tmp_path / "skills").mkdir()
|
||||
|
||||
config = make_config(
|
||||
prompt_files=["system-prompt.md"],
|
||||
skills=[],
|
||||
)
|
||||
|
||||
config = make_config(prompt_files=["system-prompt.md"], skills=[])
|
||||
report = run_preflight(config, str(tmp_path))
|
||||
|
||||
assert report.ok is True
|
||||
@ -36,19 +101,110 @@ def test_run_preflight_supported_runtime_passes(tmp_path):
|
||||
assert report.warnings == []
|
||||
|
||||
|
||||
def test_run_preflight_unsupported_runtime_fails(tmp_path):
|
||||
"""Unsupported runtimes should fail before startup."""
|
||||
def test_run_preflight_unsupported_runtime_warns_about_drift(tmp_path):
|
||||
"""When the runtime requested is not what the installed adapter
|
||||
reports, preflight returns the drift warning (not failure) — the
|
||||
adapter wins in production. The PRIOR static-list behavior would
|
||||
have hard-failed here, but the discovery-based check trusts the
|
||||
adapter and surfaces the mismatch as actionable info."""
|
||||
(tmp_path / "system-prompt.md").write_text("Base prompt.")
|
||||
# Default fixture installs Adapter.name() == "langgraph"; flip the
|
||||
# config to a different name so the drift warning fires.
|
||||
config = make_config(runtime="not-a-runtime", prompt_files=["system-prompt.md"])
|
||||
|
||||
config = make_config(
|
||||
runtime="not-a-runtime",
|
||||
prompt_files=["system-prompt.md"],
|
||||
)
|
||||
report = run_preflight(config, str(tmp_path))
|
||||
|
||||
assert report.ok is True # drift, not fatal
|
||||
assert any(issue.title == "Runtime" and "Drift" in issue.detail for issue in report.warnings)
|
||||
|
||||
|
||||
@pytest.mark.no_default_adapter
|
||||
def test_run_preflight_no_adapter_module_fails(tmp_path, monkeypatch):
|
||||
"""ADAPTER_MODULE unset → no adapter installed → preflight fails
|
||||
with an operator-actionable message naming the env var."""
|
||||
monkeypatch.delenv("ADAPTER_MODULE", raising=False)
|
||||
(tmp_path / "system-prompt.md").write_text("Base prompt.")
|
||||
config = make_config(prompt_files=["system-prompt.md"])
|
||||
|
||||
report = run_preflight(config, str(tmp_path))
|
||||
|
||||
assert report.ok is False
|
||||
assert any(issue.title == "Runtime" for issue in report.failures)
|
||||
runtime_failures = [i for i in report.failures if i.title == "Runtime"]
|
||||
assert len(runtime_failures) == 1
|
||||
assert "ADAPTER_MODULE" in runtime_failures[0].detail
|
||||
assert "unset" in runtime_failures[0].detail
|
||||
|
||||
|
||||
@pytest.mark.no_default_adapter
|
||||
def test_run_preflight_adapter_module_unimportable_fails(tmp_path, monkeypatch):
|
||||
"""ADAPTER_MODULE set to a non-existent module → import error →
|
||||
preflight fails with the underlying exception type + message."""
|
||||
monkeypatch.setenv("ADAPTER_MODULE", "this_module_does_not_exist_for_test")
|
||||
(tmp_path / "system-prompt.md").write_text("Base prompt.")
|
||||
config = make_config(prompt_files=["system-prompt.md"])
|
||||
|
||||
report = run_preflight(config, str(tmp_path))
|
||||
|
||||
assert report.ok is False
|
||||
assert any(
|
||||
i.title == "Runtime" and "not importable" in i.detail
|
||||
for i in report.failures
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.no_default_adapter
|
||||
def test_run_preflight_adapter_module_missing_class_fails(tmp_path, monkeypatch):
|
||||
"""Module imports but doesn't export `Adapter` → fail with the
|
||||
convention reminder. Pin the convention so a future refactor
|
||||
that renames the class doesn't silently bypass discovery."""
|
||||
install_fake_adapter(monkeypatch, name="langgraph", no_class=True)
|
||||
(tmp_path / "system-prompt.md").write_text("Base prompt.")
|
||||
config = make_config(prompt_files=["system-prompt.md"])
|
||||
|
||||
report = run_preflight(config, str(tmp_path))
|
||||
|
||||
assert report.ok is False
|
||||
assert any(
|
||||
i.title == "Runtime" and "no `Adapter` class" in i.detail
|
||||
for i in report.failures
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.no_default_adapter
|
||||
def test_run_preflight_adapter_name_raises_fails(tmp_path, monkeypatch):
|
||||
"""Adapter.name() throwing must be caught — the static method
|
||||
must be side-effect-free per BaseAdapter contract."""
|
||||
install_fake_adapter(monkeypatch, raise_on_name=True)
|
||||
(tmp_path / "system-prompt.md").write_text("Base prompt.")
|
||||
config = make_config(prompt_files=["system-prompt.md"])
|
||||
|
||||
report = run_preflight(config, str(tmp_path))
|
||||
|
||||
assert report.ok is False
|
||||
assert any(
|
||||
i.title == "Runtime" and "name() raised" in i.detail
|
||||
for i in report.failures
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.no_default_adapter
|
||||
def test_run_preflight_adapter_name_non_string_fails(tmp_path, monkeypatch):
|
||||
"""Adapter.name() returning None / int / etc. must fail — the
|
||||
runtime identifier is a string by contract and downstream code
|
||||
assumes that (config matching, log lines, etc.). Use 42 (int) as
|
||||
the returned value so the assertion is unambiguous; None would
|
||||
also work but int is more obviously a contract violation."""
|
||||
install_fake_adapter(monkeypatch, name_returns=42)
|
||||
(tmp_path / "system-prompt.md").write_text("Base prompt.")
|
||||
config = make_config(prompt_files=["system-prompt.md"])
|
||||
|
||||
report = run_preflight(config, str(tmp_path))
|
||||
|
||||
assert report.ok is False
|
||||
assert any(
|
||||
i.title == "Runtime" and "non-empty string" in i.detail
|
||||
for i in report.failures
|
||||
)
|
||||
|
||||
|
||||
# ---------- required_env checks ----------
|
||||
|
||||
Loading…
Reference in New Issue
Block a user