From 7dba700ac335417c134671e3f34f3ab06fd1c58c Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Mon, 27 Apr 2026 00:44:51 -0700 Subject: [PATCH] feat(preflight): replace SUPPORTED_RUNTIMES static list with adapter discovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- workspace/preflight.py | 103 ++++++++++++++--- workspace/pytest.ini | 2 + workspace/tests/test_preflight.py | 184 +++++++++++++++++++++++++++--- 3 files changed, 261 insertions(+), 28 deletions(-) diff --git a/workspace/preflight.py b/workspace/preflight.py index 672d3f21..1e6aaad2 100644 --- a/workspace/preflight.py +++ b/workspace/preflight.py @@ -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().", ) ) diff --git a/workspace/pytest.ini b/workspace/pytest.ini index 22fee4eb..6692a7fe 100644 --- a/workspace/pytest.ini +++ b/workspace/pytest.ini @@ -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. diff --git a/workspace/tests/test_preflight.py b/workspace/tests/test_preflight.py index dcec602e..3bb4a793 100644 --- a/workspace/tests/test_preflight.py +++ b/workspace/tests/test_preflight.py @@ -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 ----------