Skip to content

feat(preflight): replace SUPPORTED_RUNTIMES static list with adapter discovery - #2155

Merged
HongmingWang-Rabbit merged 1 commit into
stagingfrom
feat/preflight-runtime-discovery
Apr 27, 2026
Merged

feat(preflight): replace SUPPORTED_RUNTIMES static list with adapter discovery#2155
HongmingWang-Rabbit merged 1 commit into
stagingfrom
feat/preflight-runtime-discovery

Conversation

@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor

Summary

Closes task #123 — last piece of #87 cleanup independent of the cli_executor.py template moves.

Pre-fix: workspace/preflight.py:11 hardcoded a tuple of "supported" runtime names. Every new template repo required a code change in molecule-runtime to be recognized — direct violation of the universal-runtime principle (#87).

Post-fix: discovery-based validation via the same `ADAPTER_MODULE` env var that production load paths already use (`workspace/adapters/init.py:get_adapter`). Distinguished failure modes so operator messages are concrete:

Mode Detail
`ADAPTER_MODULE` unset "no adapter installed; set the env var"
Module won't import Underlying ImportError type + message
No `Adapter` class "Add Adapter = YourClass per BaseAdapter convention"
`Adapter.name()` raises "name() raised : " — must be side-effect-free
`Adapter.name()` returns non-string "must return a non-empty string"
`Adapter.name() != config.runtime` WARNING (not fail) — adapter wins, config drift surfaced

Behavioral change worth calling out

The drift case is a softening: prior static-list path hard-failed config.runtime values not in the allowlist; now drift is a warning so the workspace still boots while operator gets actionable info naming both the configured + installed runtime names. Reflects production reality (the adapter wins regardless).

Tests (6 new + autouse fixture)

  • Replaces obsolete "static list pass/fail" tests with 6 cases covering each distinguished failure mode + a positive happy-path test
  • Autouse `_default_langgraph_adapter` fixture pre-installs a fake adapter via `sys.modules` monkey-patching, so existing tests building default WorkspaceConfig (`runtime="langgraph"`) inherit a valid adapter
  • Failure-mode tests opt out via `@pytest.mark.no_default_adapter` (registered in pytest.ini)
  • Sentinel pattern (`_UNSET = object()`) for `name_returns` so None / int / etc. are passable test values

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

🤖 Generated with Claude Code

…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>
@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor Author

Code review (5-axis) — needs cross-account approval

Note: posting as a comment because gh's active account is also the PR author. The user's other account will need to formally approve.

Correctness:

  • Each failure mode (unset / unimportable / no Adapter class / name() raises / non-string name() / drift) gets a distinguished operator-facing message — debugging is concrete, not a single generic "unsupported"
  • Drift case correctly returns (True, detail) so the dispatch in run_preflight puts it in warnings instead of failures
  • Empty-string detail on success skips the warning branch — clean signal/no-noise

Tests:

  • 6 new failure-path tests, each pinned to the exact substring of the operator message — a future refactor that drops the actionable detail will fail loudly
  • _UNSET = object() sentinel lets None be a passable test value for name_returns — exactly the bug a if name_returns is not None check would have caused
  • Autouse fake-adapter fixture + @pytest.mark.no_default_adapter opt-out keeps existing tests untouched while exercising failure paths cleanly

Architecture:

Security:

  • importlib.import_module(env_var) is arbitrary-module-loading by design, but ADAPTER_MODULE is image-build-time config (Dockerfile ENV), not user input
  • Same trust boundary as the existing production load path

Performance: ✓ — preflight runs once at startup; one module import is negligible


Nit: install_fake_adapter uses id(monkeypatch) in the module name to avoid sys.modules collisions across tests, but pytest's monkeypatch already restores sys.modules at teardown. The unique-name suffix is defensive overkill — could just be f"_fake_adapter_{name.replace('-', '_')}". Not worth blocking on; current code is correct, just slightly more machinery than needed.

Verdict: would approve. Auto-merge already armed; will fire once CI greens + an approving review lands.

@HongmingWang-Rabbit
HongmingWang-Rabbit added this pull request to the merge queue Apr 27, 2026
Merged via the queue into staging with commit 2fbf6b6 Apr 27, 2026
14 checks passed
@molecule-ai
molecule-ai Bot deleted the feat/preflight-runtime-discovery branch May 20, 2026 06:21
HongmingWang-Rabbit pushed a commit that referenced this pull request Jun 12, 2026
… redis online-status key/TTL coverage (#2150)

Refile of the WIP at origin/regression/2150-migration-replay-from-scratch-real-pg
(stalled 2026-06-03, never advanced past DRAFT, base on a 200-commit-stale main
that would have undone my PR #2449's guard widening + the mc#1982 mask removal
+ the #2149 scheduler trigger if merged directly).

This is the #2150 implementation (close-supersedes the WIP PR #2155):

  - workspace-server/internal/db/postgres_replay_integration_test.go (286 lines)
      Real-PG integration tests for db.RunMigrations (forward chain replay-from-
      scratch via the production entrypoint, hard-fail; double-apply for the
      045 crash-loop class) and db.InitPostgres (ping + bad-DSN).
  - workspace-server/internal/db/redis_test.go (291 lines)
      Unit tests for redis.go (was untested fleet-wide): SetOnline / IsOnline /
      RefreshTTL on ws:<id>, CacheURL / GetCachedURL on ws:<id>:url, internal
      namespace pin, LivenessTTL >= 5x heartbeat, real TTL expiry via miniredis.
  - .gitea/workflows/handlers-postgres-integration.yml (+27)
      New 'Migration replay-from-scratch gate (#2150)' step, runs the integration
      suite against a SEPARATE 'molecule_replay' database on the same sibling
      Postgres (so the destructive DROP SCHEMA never touches the handlers
      molecule DB). Inserted AFTER the scheduler (#2149) step; does NOT undo
      any of: the mc#1982 mask removal, the preflight INTEGRATION_DB_URL
      check, or the table-presence guard widening (PR #2449).
  - .gitea/scripts/detect-changes.py (+5)
      'handlers-postgres' profile now also matches internal/db/ (additive,
      preserves the scheduler trigger from #2149) so a change to redis.go or
      postgres.go runs the gate.

Refs #2150. Supersedes the WIP PR #2155 (DRAFT, 6 days stalled, branched from
a 200-commit-stale main).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant