diff --git a/src/ouroboros/config/__init__.py b/src/ouroboros/config/__init__.py index 3f95e1244..670840cd3 100644 --- a/src/ouroboros/config/__init__.py +++ b/src/ouroboros/config/__init__.py @@ -110,6 +110,7 @@ "DriftConfig", "LoggingConfig", "OrchestratorConfig", + "RuntimeProfileConfig", # Loader functions "load_config", "load_credentials", diff --git a/src/ouroboros/config/models.py b/src/ouroboros/config/models.py index fb6ec5b35..e2e3104de 100644 --- a/src/ouroboros/config/models.py +++ b/src/ouroboros/config/models.py @@ -26,6 +26,8 @@ from pydantic import BaseModel, Field, field_validator +from ouroboros.orchestrator_stage import VALID_STAGE_KEYS + class ModelConfig(BaseModel, frozen=True): """Configuration for a single LLM model. @@ -330,16 +332,46 @@ class LoggingConfig(BaseModel, frozen=True): include_reasoning: bool = True +VALID_RUNTIME_BACKENDS = frozenset( + { + "claude", + "claude_code", + "codex", + "codex_cli", + "opencode", + "opencode_cli", + "hermes", + "hermes_cli", + "gemini", + "gemini_cli", + } +) + + class RuntimeProfileConfig(BaseModel, frozen=True): - """Runtime profile configuration shared by backend profiles and stage routing. - - ``backend_profile`` carries backend-native profile names such as PR #505's - Codex ``worker`` mapping. Unknown names are intentionally accepted here so - backend-local resolvers can warn and fall back without making the shared - config object reject future backend slices. ``default`` and ``stages`` - reserve the same object shape used by the stage-routing contract from PR - #538 so the public ``orchestrator.runtime_profile`` key has one stable - table/object form. + """Runtime profile configuration (issue #519 / M4 / S3). + + The Agent OS architecture diagram agreed in #476 lets each pipeline + stage (``interview`` / ``execute`` / ``evaluate`` / ``reflect``) be + served by a different harness. This block exposes that decision as + a configuration surface; the resolution helper in + ``ouroboros.orchestrator.stage`` reads it. + + This object also reserves ``backend_profile`` for backend-native + profile selection (for example PR #505's Codex ``worker`` profile), + so the public ``orchestrator.runtime_profile`` key has one stable + object shape instead of conflicting string-vs-table meanings. + + Attributes: + backend_profile: Optional backend-native profile name. Stage + routing does not interpret it; backend adapters may map it + to their own profile mechanism. + default: Optional runtime backend that serves any stage missing + from ``stages``. ``None`` means "fall through to the + orchestrator's top-level ``runtime_backend``". + stages: Explicit per-stage mapping. Keys must be members of the + closed stage vocabulary; unknown keys raise ``ValueError`` + during Pydantic validation at startup. """ backend_profile: str | None = None @@ -349,7 +381,7 @@ class RuntimeProfileConfig(BaseModel, frozen=True): @field_validator("backend_profile") @classmethod def _validate_backend_profile(cls, value: str | None) -> str | None: - """Normalize backend-native profile names without constraining vocabulary.""" + """Normalize optional backend-native profile names.""" if value is None: return None candidate = value.strip() @@ -357,6 +389,40 @@ def _validate_backend_profile(cls, value: str | None) -> str | None: raise ValueError("runtime_profile.backend_profile must not be empty") return candidate + @field_validator("default") + @classmethod + def _validate_default_backend(cls, value: str | None) -> str | None: + """Reject invalid runtime_profile.default backend names at startup.""" + if value is None: + return None + return _validate_runtime_backend(value, field_name="runtime_profile.default") + + @field_validator("stages") + @classmethod + def _validate_stage_keys(cls, value: dict[str, str]) -> dict[str, str]: + """Reject unknown stage names and invalid backend names at startup.""" + validated: dict[str, str] = {} + for key, backend in value.items(): + if key not in VALID_STAGE_KEYS: + valid_list = ", ".join(sorted(VALID_STAGE_KEYS)) + raise ValueError( + f"Unknown runtime_profile.stages key: {key!r}. Valid keys are: {valid_list}.", + ) + validated[key] = _validate_runtime_backend( + backend, + field_name=f"runtime_profile.stages[{key!r}]", + ) + return validated + + +def _validate_runtime_backend(value: str, *, field_name: str) -> str: + """Validate runtime_profile backend names against orchestrator backends.""" + candidate = value.strip().lower() + if candidate not in VALID_RUNTIME_BACKENDS: + valid_list = ", ".join(sorted(VALID_RUNTIME_BACKENDS)) + raise ValueError(f"{field_name} must be one of: {valid_list}") + return candidate + class OrchestratorConfig(BaseModel, frozen=True): """Orchestrator runtime configuration. diff --git a/src/ouroboros/orchestrator/stage.py b/src/ouroboros/orchestrator/stage.py new file mode 100644 index 000000000..1bd64281d --- /dev/null +++ b/src/ouroboros/orchestrator/stage.py @@ -0,0 +1,22 @@ +"""Compatibility re-export for orchestrator stage routing primitives. + +The canonical definitions live in :mod:`ouroboros.orchestrator_stage` so +configuration validation can import the closed stage vocabulary without +importing the full :mod:`ouroboros.orchestrator` package graph. +""" + +from ouroboros.orchestrator_stage import ( + VALID_STAGE_KEYS, + Stage, + UnknownStageError, + parse_stage, + resolve_runtime_for_stage, +) + +__all__ = [ + "Stage", + "VALID_STAGE_KEYS", + "UnknownStageError", + "parse_stage", + "resolve_runtime_for_stage", +] diff --git a/src/ouroboros/orchestrator_stage.py b/src/ouroboros/orchestrator_stage.py new file mode 100644 index 000000000..115488be8 --- /dev/null +++ b/src/ouroboros/orchestrator_stage.py @@ -0,0 +1,119 @@ +"""``Stage`` — closed enumeration of orchestrator pipeline stages. + +Issue #519 — slice 1 of M4 / S3. The Agent OS architecture diagram +agreed in #476 assigns a different harness per pipeline stage: + +* **interview** — Codex (clarification, ambiguity reduction) +* **execute** — OpenCode / OMX (the running of the AC tree) +* **evaluate** — Claude Code (Stage 1/2/3 verification) +* **reflect** — Hermes (Wonder/Reflect generation) + +This module is the *binding-table primitive* the orchestrator reads to +pick a runtime per stage. The four stages above are the **closed** +initial vocabulary; adding a new stage is an explicit, justified PR +(per the narrow-membership rule the maintainer alignment in #476 Q1 +applied to ``AgentRuntimeContext``). That stops the table from +sprawling into per-handler entries (``qa_judge``, ``unstuck`` …) +which belong inside an :class:`AgentProcess` (#518), not in the +binding table. + +The module deliberately exposes nothing more than the enum and a +small resolution helper. The resolution rule itself is pinned by the +sub-thread: + +:: + + runtime = ( + runtime_profile.stages.get(stage) # explicit per-stage + or runtime_profile.default # opt-in default + or current_orchestrator_runtime_backend # today's behaviour + ) + +When a config has ``runtime_profile=None`` (or omits the block +entirely), :func:`resolve_runtime_for_stage` falls back to the +existing ``orchestrator.runtime_backend`` byte-for-byte — that is the +backwards-compat commitment carried forward from PR #505. +""" + +from __future__ import annotations + +from enum import StrEnum +from typing import Final + + +class Stage(StrEnum): + """Closed enumeration of pipeline stages routed by ``runtime_profile``. + + Adding a member requires (a) a stage name, (b) documentation of + which workflow phase it covers, (c) a justification line in the + PR body explaining why an existing stage cannot host the work. + """ + + INTERVIEW = "interview" + EXECUTE = "execute" + EVALUATE = "evaluate" + REFLECT = "reflect" + + +VALID_STAGE_KEYS: Final[frozenset[str]] = frozenset(stage.value for stage in Stage) + + +class UnknownStageError(ValueError): + """Raised when a runtime_profile.stages key is not a valid stage. + + The error message names the offending key and the valid set so + operators see typos at startup rather than mid-workflow. + """ + + +def parse_stage(value: str) -> Stage: + """Parse a string into a :class:`Stage`, raising on unknown values. + + Used at startup to validate ``runtime_profile.stages`` keys. + Unknown keys raise :class:`UnknownStageError` so a typo in + ``interveiw`` fails fast at config load. + """ + if value not in VALID_STAGE_KEYS: + valid_list = ", ".join(sorted(VALID_STAGE_KEYS)) + raise UnknownStageError( + f"Unknown runtime_profile stage key: {value!r}. Valid keys are: {valid_list}.", + ) + return Stage(value) + + +def resolve_runtime_for_stage( + stage: Stage, + *, + stages: dict[Stage, str] | None, + default: str | None, + fallback: str, +) -> str: + """Return the runtime backend that should serve ``stage``. + + Resolution order locked in the #519 sub-thread: + + 1. ``stages[stage]`` — explicit per-stage mapping wins. + 2. ``default`` — when set, the runtime_profile's own default. + 3. ``fallback`` — today's hard-coded ``orchestrator.runtime_backend``. + + Args: + stage: The pipeline stage being resolved. + stages: Optional explicit stage→runtime mapping. ``None`` means + "no stage block configured". + default: Optional ``runtime_profile.default``. ``None`` means + "no runtime_profile default configured". + fallback: The today-behaviour fallback (the orchestrator's + top-level ``runtime_backend``). Always provided so the + resolution function never returns ``None``. + + Returns: + The runtime backend identifier (e.g. ``"codex"``, ``"opencode"``) + that should serve the given stage. + """ + if stages is not None: + explicit = stages.get(stage) + if explicit: + return explicit + if default: + return default + return fallback diff --git a/tests/unit/orchestrator/test_stage_resolution.py b/tests/unit/orchestrator/test_stage_resolution.py new file mode 100644 index 000000000..22ad7ce30 --- /dev/null +++ b/tests/unit/orchestrator/test_stage_resolution.py @@ -0,0 +1,226 @@ +"""Unit tests for the Stage enum and runtime resolution (slice 1 of #519). + +Coverage: +- Stage enum has exactly the four members agreed in the #519 + sub-thread; ``parse_stage`` rejects unknown values. +- ``resolve_runtime_for_stage`` picks ``stages[stage] → default → + fallback`` in that order. +- ``runtime_profile=None`` (config absence) preserves today's + behaviour byte-for-byte by always returning the orchestrator's + ``runtime_backend``. +- A typo in ``runtime_profile.stages`` (e.g. ``"interveiw"``) raises + a Pydantic validation error at config validation time, not later. +- ``OrchestratorConfig.runtime_profile`` is optional (``None`` + default) so existing configs construct unchanged. +""" + +from __future__ import annotations + +import pytest + +from ouroboros.config import OrchestratorConfig, RuntimeProfileConfig +from ouroboros.orchestrator.stage import ( + VALID_STAGE_KEYS, + Stage, + UnknownStageError, + parse_stage, + resolve_runtime_for_stage, +) + + +class TestStageEnum: + def test_has_four_members(self) -> None: + assert {s.value for s in Stage} == { + "interview", + "execute", + "evaluate", + "reflect", + } + + def test_valid_keys_matches_enum(self) -> None: + assert {s.value for s in Stage} == VALID_STAGE_KEYS + + +class TestParseStage: + def test_valid_string_returns_member(self) -> None: + assert parse_stage("evaluate") is Stage.EVALUATE + + def test_unknown_string_raises_with_helpful_message(self) -> None: + with pytest.raises(UnknownStageError) as info: + parse_stage("interveiw") # typo + msg = str(info.value) + assert "interveiw" in msg + # The error must surface the valid set so operators can fix + # without consulting docs. + for valid in VALID_STAGE_KEYS: + assert valid in msg + + +class TestResolveRuntimeForStage: + def test_explicit_stage_wins(self) -> None: + runtime = resolve_runtime_for_stage( + Stage.EVALUATE, + stages={Stage.EVALUATE: "claude_code"}, + default="opencode", + fallback="claude", + ) + assert runtime == "claude_code" + + def test_default_used_when_stage_missing(self) -> None: + runtime = resolve_runtime_for_stage( + Stage.EVALUATE, + stages={Stage.EXECUTE: "opencode"}, # different stage + default="codex", + fallback="claude", + ) + assert runtime == "codex" + + def test_fallback_when_no_runtime_profile_configured(self) -> None: + runtime = resolve_runtime_for_stage( + Stage.EVALUATE, + stages=None, + default=None, + fallback="claude", + ) + assert runtime == "claude" + + def test_empty_stages_dict_uses_default(self) -> None: + runtime = resolve_runtime_for_stage( + Stage.EVALUATE, + stages={}, + default="opencode", + fallback="claude", + ) + assert runtime == "opencode" + + def test_empty_default_falls_through_to_fallback(self) -> None: + runtime = resolve_runtime_for_stage( + Stage.EVALUATE, + stages={}, + default="", + fallback="claude", + ) + assert runtime == "claude" + + +class TestRuntimeProfileConfig: + def test_none_runtime_profile_is_default_on_orchestrator_config(self) -> None: + config = OrchestratorConfig() + assert config.runtime_profile is None + + def test_construct_with_explicit_runtime_profile(self) -> None: + config = OrchestratorConfig( + runtime_profile=RuntimeProfileConfig( + default="codex", + stages={"evaluate": "claude_code"}, + ) + ) + assert config.runtime_profile is not None + assert config.runtime_profile.backend_profile is None + assert config.runtime_profile.default == "codex" + assert config.runtime_profile.stages == {"evaluate": "claude_code"} + + def test_runtime_profile_object_can_hold_backend_profile_and_stage_routing(self) -> None: + config = OrchestratorConfig( + runtime_profile=RuntimeProfileConfig( + backend_profile="worker", + default="codex", + stages={"execute": "opencode"}, + ) + ) + + assert config.runtime_profile is not None + assert config.runtime_profile.backend_profile == "worker" + assert config.runtime_profile.default == "codex" + assert config.runtime_profile.stages == {"execute": "opencode"} + + def test_runtime_profile_string_is_backend_profile_shorthand(self) -> None: + """Accept PR #505's string shape without stealing stage-routing fields.""" + config = OrchestratorConfig(runtime_profile="worker") + + assert config.runtime_profile is not None + assert config.runtime_profile.backend_profile == "worker" + assert config.runtime_profile.default is None + assert config.runtime_profile.stages == {} + + def test_unknown_stage_key_rejected_at_validation(self) -> None: + # Pydantic wraps validator-raised ValueError in its own + # ``ValidationError`` while preserving the helpful typo message + # and valid stage set. + from pydantic import ValidationError + + with pytest.raises(ValidationError) as info: + RuntimeProfileConfig(stages={"interveiw": "codex"}) + msg = str(info.value) + assert "interveiw" in msg + assert "Unknown runtime_profile.stages key" in msg + for valid in VALID_STAGE_KEYS: + assert valid in msg + + def test_legacy_orchestrator_config_unchanged(self) -> None: + """Configs without ``runtime_profile`` construct exactly as before.""" + # Pin the field set so reviewers see explicit diffs when membership + # grows. + legacy = OrchestratorConfig(runtime_backend="codex") + assert legacy.runtime_backend == "codex" + assert legacy.runtime_profile is None + # The orchestrator's runtime_backend remains the byte-for-byte + # fallback (per #505 commitment). + runtime = resolve_runtime_for_stage( + Stage.EXECUTE, + stages=( + {parse_stage(k): v for k, v in legacy.runtime_profile.stages.items()} + if legacy.runtime_profile + else None + ), + default=legacy.runtime_profile.default if legacy.runtime_profile else None, + fallback=legacy.runtime_backend, + ) + assert runtime == "codex" + + def test_invalid_runtime_profile_default_backend_rejected(self) -> None: + from pydantic import ValidationError + + with pytest.raises(ValidationError) as info: + RuntimeProfileConfig(default="cluade") + msg = str(info.value) + assert "runtime_profile.default" in msg + assert "claude" in msg + assert "codex" in msg + + def test_invalid_runtime_profile_stage_backend_rejected(self) -> None: + from pydantic import ValidationError + + with pytest.raises(ValidationError) as info: + RuntimeProfileConfig(stages={"execute": ""}) + msg = str(info.value) + assert "runtime_profile.stages['execute']" in msg + assert "claude" in msg + assert "codex" in msg + + def test_runtime_profile_accepts_runtime_factory_backend_aliases(self) -> None: + profile = RuntimeProfileConfig( + default="codex_cli", + stages={ + "interview": "claude_code", + "execute": "opencode_cli", + "reflect": "hermes_cli", + "evaluate": "gemini_cli", + }, + ) + + assert profile.default == "codex_cli" + assert profile.stages == { + "interview": "claude_code", + "execute": "opencode_cli", + "reflect": "hermes_cli", + "evaluate": "gemini_cli", + } + + def test_empty_runtime_profile_backend_profile_rejected(self) -> None: + from pydantic import ValidationError + + with pytest.raises(ValidationError) as info: + RuntimeProfileConfig(backend_profile=" ") + + assert "runtime_profile.backend_profile" in str(info.value)