diff --git a/loopx/capabilities/machine_configuration/builtins.py b/loopx/capabilities/machine_configuration/builtins.py index 368b829221..613c74e4a5 100644 --- a/loopx/capabilities/machine_configuration/builtins.py +++ b/loopx/capabilities/machine_configuration/builtins.py @@ -1,51 +1,83 @@ from __future__ import annotations -from collections.abc import Mapping +from collections.abc import Callable, Mapping +from importlib import import_module from typing import Any -from .contract import MachineConfigurationRegistry +from .contract import MachineConfigurationNamespace, MachineConfigurationRegistry + +# Namespace id, owner module and factory. Owners stay at the composition +# boundary: the generic contract does not depend on any consumer capability, +# and a reader loads only the namespaces it resolves. +_BUILTIN_NAMESPACES = ( + ( + "context_delivery", + "loopx.capabilities.machine_configuration.context_delivery", + "context_delivery_machine_configuration_namespace", + ), + ( + "explore_harness", + "loopx.capabilities.explore.machine_defaults", + "explore_machine_configuration_namespace", + ), + ( + "codex_chat", + "loopx.capabilities.native_chat.machine_defaults", + "codex_chat_machine_configuration_namespace", + ), + ( + "goal_storage", + "loopx.capabilities.machine_configuration.goal_storage", + "goal_storage_machine_configuration_namespace", + ), + ( + "manager_runtime", + "loopx.capabilities.manager_runtime.machine_profile", + "manager_runtime_machine_configuration_namespace", + ), + ( + "periodic_report", + "loopx.capabilities.periodic_report.machine_defaults", + "periodic_report_machine_configuration_namespace", + ), + ( + "todo_replan_cadence", + "loopx.capabilities.todo_replan_cadence.machine_defaults", + "todo_replan_cadence_machine_configuration_namespace", + ), + ( + "change_quality_qualification", + "loopx.capabilities.change_quality.machine_defaults", + "change_quality_machine_configuration_namespace", + ), + ( + "pull_request_review", + "loopx.capabilities.pr_review_queue.machine_defaults", + "pull_request_review_machine_configuration_namespace", + ), + ( + "steward_executor", + "loopx.capabilities.steward_executor.machine_defaults", + "steward_executor_machine_configuration_namespace", + ), +) + + +def _owner(module: str, factory: str) -> Callable[[], MachineConfigurationNamespace]: + def load() -> MachineConfigurationNamespace: + contract: MachineConfigurationNamespace = getattr( + import_module(module), factory + )() + return contract + + return load def build_builtin_machine_configuration_registry() -> MachineConfigurationRegistry: - # Imports stay at the composition boundary: the generic contract does not - # depend on any consumer capability. - from ..change_quality.machine_defaults import ( - change_quality_machine_configuration_namespace, - ) - from ..periodic_report.machine_defaults import ( - periodic_report_machine_configuration_namespace, - ) - from ..manager_runtime.machine_profile import ( - manager_runtime_machine_configuration_namespace, - ) - from ..pr_review_queue.machine_defaults import ( - pull_request_review_machine_configuration_namespace, - ) - from ..steward_executor.machine_defaults import ( - steward_executor_machine_configuration_namespace, - ) - from ..todo_replan_cadence.machine_defaults import ( - todo_replan_cadence_machine_configuration_namespace, - ) - - from ..explore.machine_defaults import explore_machine_configuration_namespace - from .goal_storage import goal_storage_machine_configuration_namespace - from ..native_chat.machine_defaults import codex_chat_machine_configuration_namespace - from .context_delivery import context_delivery_machine_configuration_namespace - - return ( - MachineConfigurationRegistry() - .register(context_delivery_machine_configuration_namespace()) - .register(explore_machine_configuration_namespace()) - .register(codex_chat_machine_configuration_namespace()) - .register(goal_storage_machine_configuration_namespace()) - .register(manager_runtime_machine_configuration_namespace()) - .register(periodic_report_machine_configuration_namespace()) - .register(todo_replan_cadence_machine_configuration_namespace()) - .register(change_quality_machine_configuration_namespace()) - .register(pull_request_review_machine_configuration_namespace()) - .register(steward_executor_machine_configuration_namespace()) - ) + registry = MachineConfigurationRegistry() + for namespace, module, factory in _BUILTIN_NAMESPACES: + registry.register_deferred(namespace, _owner(module, factory)) + return registry def project_goal_with_builtin_machine_configuration( diff --git a/loopx/capabilities/machine_configuration/contract.py b/loopx/capabilities/machine_configuration/contract.py index ed4aa4918d..b5e1082b44 100644 --- a/loopx/capabilities/machine_configuration/contract.py +++ b/loopx/capabilities/machine_configuration/contract.py @@ -89,38 +89,69 @@ class MachineConfigurationRegistry: def __init__(self) -> None: self._namespaces: dict[str, MachineConfigurationNamespace] = {} + self._deferred: dict[str, Callable[[], MachineConfigurationNamespace]] = {} + + def _reserve(self, namespace: str) -> None: + if namespace in self._namespaces or namespace in self._deferred: + raise ValueError( + "machine-configuration namespace is already registered: " + namespace + ) def register( self, contract: MachineConfigurationNamespace ) -> MachineConfigurationRegistry: - if contract.namespace in self._namespaces: - raise ValueError( - "machine-configuration namespace is already registered: " - + contract.namespace - ) + self._reserve(contract.namespace) # Validate public catalog metadata before this registry can own effects. # A bad provider template must fail before preview/apply/rollback runs. contract.public_descriptor() self._namespaces[contract.namespace] = contract return self + def register_deferred( + self, namespace: str, load: Callable[[], MachineConfigurationNamespace] + ) -> MachineConfigurationRegistry: + """Register an owner that is loaded when its namespace is first resolved. + + Readers then import only the namespaces they touch. Resolution applies + the same template validation as ``register`` before the owner is + returned, so a bad template still fails before that namespace can own a + preview, apply or rollback effect. + """ + if not _NAMESPACE_RE.fullmatch(namespace): + raise ValueError("machine-configuration namespace is invalid") + self._reserve(namespace) + self._deferred[namespace] = load + return self + def resolve(self, namespace: str) -> MachineConfigurationNamespace: - try: - return self._namespaces[namespace] - except KeyError as exc: + contract = self._namespaces.get(namespace) + if contract is not None: + return contract + load = self._deferred.get(namespace) + if load is None: raise ValueError( f"unsupported machine-configuration namespace: {namespace}" - ) from exc + ) + contract = load() + if contract.namespace != namespace: + raise ValueError( + "machine-configuration namespace owner declares another id: " + + namespace + ) + contract.public_descriptor() + del self._deferred[namespace] + self._namespaces[namespace] = contract + return contract @property def namespace_ids(self) -> tuple[str, ...]: - return tuple(sorted(self._namespaces)) + return tuple(sorted({*self._namespaces, *self._deferred})) def public_catalog(self) -> dict[str, Any]: return { "schema_version": MACHINE_CONFIGURATION_CATALOG_SCHEMA, "namespaces": [ - self._namespaces[namespace].public_descriptor() + self.resolve(namespace).public_descriptor() for namespace in self.namespace_ids ], } diff --git a/tests/capabilities/test_machine_configuration_contract.py b/tests/capabilities/test_machine_configuration_contract.py index 95640c08b5..ac0912f595 100644 --- a/tests/capabilities/test_machine_configuration_contract.py +++ b/tests/capabilities/test_machine_configuration_contract.py @@ -1,10 +1,17 @@ from __future__ import annotations from collections.abc import Mapping +from pathlib import Path +import subprocess +import sys from typing import Any import pytest +from loopx.capabilities.machine_configuration.builtins import ( + build_builtin_machine_configuration_registry, +) + from loopx.capabilities.machine_configuration.contract import ( MachineConfigurationNamespace, MachineConfigurationRegistry, @@ -42,25 +49,28 @@ def _apply_example_public_update( return {**dict(current or {}), **dict(update)} +def _example_namespace(**overrides: Any) -> MachineConfigurationNamespace: + fields: dict[str, Any] = { + "namespace": "example", + "schema_versions": frozenset({"example_v0"}), + "normalize": _normalize_example, + "project_public": lambda value: { + key: item for key, item in value.items() if key != "secret" + }, + "apply_public_update": _apply_example_public_update, + "title": "Example defaults", + "description": "Example machine-level defaults.", + "default_configuration": { + "schema_version": "example_v0", + "enabled": False, + "secret": "hidden", + }, + } + return MachineConfigurationNamespace(**{**fields, **overrides}) + + def _registry() -> MachineConfigurationRegistry: - return MachineConfigurationRegistry().register( - MachineConfigurationNamespace( - namespace="example", - schema_versions=frozenset({"example_v0"}), - normalize=_normalize_example, - project_public=lambda value: { - key: item for key, item in value.items() if key != "secret" - }, - apply_public_update=_apply_example_public_update, - title="Example defaults", - description="Example machine-level defaults.", - default_configuration={ - "schema_version": "example_v0", - "enabled": False, - "secret": "hidden", - }, - ) - ) + return MachineConfigurationRegistry().register(_example_namespace()) def _config() -> dict[str, Any]: @@ -228,3 +238,144 @@ def test_namespace_merge_rejects_private_fields_in_a_public_update() -> None: }, registry=_registry(), ) + + +def test_deferred_owner_loads_once_on_first_resolution() -> None: + loads: list[str] = [] + + def load() -> MachineConfigurationNamespace: + loads.append("example") + return _example_namespace() + + registry = MachineConfigurationRegistry().register_deferred("example", load) + + assert registry.namespace_ids == ("example",) + assert loads == [] + assert normalize_machine_configuration(_config(), registry=registry) == _config() + assert registry.public_catalog() == _registry().public_catalog() + assert loads == ["example"] + + +def test_deferred_owner_template_is_validated_before_it_can_be_used() -> None: + registry = MachineConfigurationRegistry().register_deferred( + "example", + lambda: _example_namespace(default_configuration={"schema_version": "example_v0"}), + ) + + for _attempt in range(2): + with pytest.raises(TypeError, match="example.enabled must be a boolean"): + normalize_machine_configuration(_config(), registry=registry) + with pytest.raises(TypeError, match="example.enabled must be a boolean"): + registry.public_catalog() + + +def test_deferred_owner_must_declare_its_registered_namespace() -> None: + registry = MachineConfigurationRegistry().register_deferred( + "other", _example_namespace + ) + + with pytest.raises(ValueError, match="owner declares another id: other"): + registry.resolve("other") + with pytest.raises(ValueError, match="unsupported machine-configuration namespace"): + registry.resolve("example") + + +def test_deferred_and_eager_owners_share_one_namespace_reservation() -> None: + with pytest.raises(ValueError, match="already registered: example"): + _registry().register_deferred("example", _example_namespace) + with pytest.raises(ValueError, match="already registered: example"): + MachineConfigurationRegistry().register_deferred( + "example", _example_namespace + ).register(_example_namespace()) + with pytest.raises(ValueError, match="namespace is invalid"): + MachineConfigurationRegistry().register_deferred("Example", _example_namespace) + + +def test_builtin_catalog_lists_every_owner_in_namespace_order() -> None: + registry = build_builtin_machine_configuration_registry() + + assert registry.namespace_ids == ( + "change_quality_qualification", + "codex_chat", + "context_delivery", + "explore_harness", + "goal_storage", + "manager_runtime", + "periodic_report", + "pull_request_review", + "steward_executor", + "todo_replan_cadence", + ) + assert [ + descriptor["namespace"] + for descriptor in registry.public_catalog()["namespaces"] + ] == list(registry.namespace_ids) + + +def test_builtin_registry_still_fails_closed_on_an_invalid_stored_sibling() -> None: + configuration = { + "schema_version": "loopx_machine_configuration_v0", + "namespaces": { + "context_delivery": { + "schema_version": "context_delivery_machine_defaults_v0", + "mode": "on", + }, + "periodic_report": {"schema_version": "unknown_v0"}, + }, + } + + with pytest.raises(ValueError, match="unsupported schema_version"): + normalize_machine_configuration( + configuration, registry=build_builtin_machine_configuration_registry() + ) + + +def test_builtin_registry_loads_only_the_owners_a_reader_resolves() -> None: + script = """ +import sys + +from loopx.capabilities.machine_configuration.builtins import ( + build_builtin_machine_configuration_registry, +) +from loopx.capabilities.machine_configuration.contract import ( + normalize_machine_configuration, +) + +normalize_machine_configuration( + { + "schema_version": "loopx_machine_configuration_v0", + "namespaces": { + "context_delivery": { + "schema_version": "context_delivery_machine_defaults_v0", + "mode": "on", + } + }, + }, + registry=build_builtin_machine_configuration_registry(), +) +unrelated = sorted( + module + for module in ( + "loopx.capabilities.change_quality.machine_defaults", + "loopx.capabilities.explore.machine_defaults", + "loopx.capabilities.manager_runtime.machine_profile", + "loopx.capabilities.native_chat.machine_defaults", + "loopx.capabilities.periodic_report.machine_defaults", + "loopx.capabilities.pr_review_queue.machine_defaults", + "loopx.capabilities.steward_executor.machine_defaults", + "loopx.capabilities.todo_replan_cadence.machine_defaults", + ) + if module in sys.modules +) +assert unrelated == [], unrelated +assert "loopx.capabilities.machine_configuration.context_delivery" in sys.modules +""" + completed = subprocess.run( + [sys.executable, "-c", script], + cwd=Path(__file__).resolve().parents[2], + text=True, + capture_output=True, + check=False, + ) + + assert completed.returncode == 0, completed.stderr