diff --git a/docs/runtime-guides/codex.md b/docs/runtime-guides/codex.md index 52b732bf6..d5a32c5c6 100644 --- a/docs/runtime-guides/codex.md +++ b/docs/runtime-guides/codex.md @@ -115,9 +115,39 @@ Under the hood, `CodexCliRuntime` still talks to the local `codex` executable, b - Installs managed Ouroboros skills into `~/.codex/skills/` - Registers the Ouroboros MCP/env hookup in `~/.codex/config.toml` when absent, refreshes setup-managed stdio blocks, and preserves user-managed URL/custom entries by default - Adds missing `profiles.ouroboros-*` Codex profile anchors without overwriting existing profiles +- Registers a managed `[profiles.ouroboros-worker]` section in the same file so Agent OS worker subprocesses can opt out of interactive Codex defaults without losing the MCP/env hookup `~/.codex/config.toml` is not where Ouroboros per-role model overrides belong. Keep `clarification`, `qa`, `semantic`, `consensus`, `llm_profiles`, and `llm_role_profiles` settings in `~/.ouroboros/config.yaml`. If you manage a long-running URL-based Ouroboros MCP server, keep that URL entry in `~/.codex/config.toml`; `ouroboros setup --runtime codex` preserves it by default. Use `--mcp-mode stdio` only when you intentionally want setup to replace the entry with the managed command-spawned server. +### Worker subprocess isolation (Agent OS `runtime_profile`) + +Interactive `codex` sessions and Ouroboros-managed worker subprocesses sometimes want different defaults — for example a different model, sandbox, or notify hook. Set the orchestrator-level runtime profile to `worker` to opt every Ouroboros-spawned `codex exec` invocation into the managed `[profiles.ouroboros-worker]` block: + +```yaml +# ~/.ouroboros/config.yaml +orchestrator: + runtime_backend: codex + runtime_profile: + backend_profile: worker # optional; default unset preserves today's behavior +``` + +Or via the environment for one-off runs: + +```bash +OUROBOROS_RUNTIME_PROFILE=worker ouroboros run workflow --runtime codex seed.yaml +``` + +Customize the worker overrides directly in `~/.codex/config.toml`: + +```toml +[profiles.ouroboros-worker] +model = "o3-mini" +notify = [] +sandbox = "workspace-write" +``` + +When `runtime_profile` is unset (the default), Ouroboros emits `codex exec` exactly as before — no profile flag, full user-config inheritance. This is the Codex-side mapping of the cross-runtime Agent OS profile contract; OpenCode, Hermes, Claude Code, and LiteLLM mappings can add their own backend-local mappings separately. + ### `ooo` Skill Availability on Codex After running `ouroboros setup --runtime codex`, the bundled `ooo` skills are installed into `~/.codex/skills/ouroboros-*` and the routing rules into `~/.codex/rules/`. To refresh only those artifacts after upgrading Ouroboros, run `ouroboros codex refresh`; it does not modify `~/.codex/config.toml` or `~/.ouroboros/config.yaml`. The table below shows each skill and its CLI equivalent for terminal-only workflows. diff --git a/src/ouroboros/cli/commands/setup.py b/src/ouroboros/cli/commands/setup.py index 9cf0969a3..aa0053c9e 100644 --- a/src/ouroboros/cli/commands/setup.py +++ b/src/ouroboros/cli/commands/setup.py @@ -346,6 +346,88 @@ def _is_setup_managed_codex_mcp_entry(entry: dict[str, object]) -> bool: return len(args) >= 3 and args[-3:] == ["ouroboros", "mcp", "serve"] +_CODEX_WORKER_PROFILE_SECTION = """# Ouroboros Agent OS runtime profile for Codex worker subprocesses. +# Activated when ~/.ouroboros/config.yaml sets `orchestrator.runtime_profile.backend_profile: worker` +# (or the OUROBOROS_RUNTIME_PROFILE=worker env var). Add per-worker Codex +# overrides below — for example a different model, sandbox, or notify hook — +# without affecting interactive `codex` sessions that share this config file. + +[profiles.ouroboros-worker] +""" + +_CODEX_WORKER_PROFILE_COMMENT_LINES = ( + "# Ouroboros Agent OS runtime profile for Codex worker subprocesses.", + "# Activated when ~/.ouroboros/config.yaml sets `orchestrator.runtime_profile.backend_profile: worker`", + "# (or the OUROBOROS_RUNTIME_PROFILE=worker env var). Add per-worker Codex", + "# overrides below — for example a different model, sandbox, or notify hook —", + "# without affecting interactive `codex` sessions that share this config file.", +) + + +def _is_codex_ouroboros_worker_profile_header(line: str) -> bool: + """Return True when the line starts the managed Codex worker profile table.""" + return line == "[profiles.ouroboros-worker]" or line.startswith("[profiles.ouroboros-worker.") + + +def _trim_managed_codex_worker_profile_comments(lines: list[str]) -> None: + """Excise every managed worker-profile comment block from ``lines``.""" + expected = list(_CODEX_WORKER_PROFILE_COMMENT_LINES) + block_len = len(expected) + if block_len == 0: + return + + index = 0 + while index <= len(lines) - block_len: + if lines[index : index + block_len] == expected: + end = index + block_len + if end < len(lines) and not lines[end].strip(): + end += 1 + del lines[index:end] + else: + index += 1 + + +def _upsert_codex_worker_profile_section(raw: str) -> tuple[str, bool]: + """Refresh the managed comment block for ``[profiles.ouroboros-worker]``. + + Setup owns only the managed comment/header. Existing user-authored keys + and subtables under ``[profiles.ouroboros-worker]`` are preserved verbatim. + """ + section_lines = _CODEX_WORKER_PROFILE_SECTION.strip("\n").splitlines() + input_lines = raw.splitlines() + output_lines: list[str] = [] + index = 0 + existed_before = False + refreshed = False + + while index < len(input_lines): + line = input_lines[index] + stripped = line.strip() + if stripped == "[profiles.ouroboros-worker]" and not refreshed: + existed_before = True + refreshed = True + _trim_managed_codex_worker_profile_comments(output_lines) + if output_lines and output_lines[-1].strip(): + output_lines.append("") + output_lines.extend(section_lines) + index += 1 + continue + if _is_codex_ouroboros_worker_profile_header(stripped): + existed_before = True + output_lines.append(line) + index += 1 + continue + output_lines.append(line) + index += 1 + + if not refreshed: + if output_lines and output_lines[-1].strip(): + output_lines.append("") + output_lines.extend(section_lines) + + return "\n".join(output_lines).rstrip() + "\n", existed_before + + def _is_codex_ouroboros_table_header(line: str) -> bool: """Return True when the line starts the managed Codex MCP table.""" return line == "[mcp_servers.ouroboros]" or line.startswith("[mcp_servers.ouroboros.") @@ -518,6 +600,44 @@ def _register_codex_default_profiles() -> None: print_success(f"Registered Codex task profiles in {codex_config}: {', '.join(added_profiles)}") +def _register_codex_worker_profile() -> None: + """Register the managed Codex worker profile in ~/.codex/config.toml.""" + import tomllib + + codex_config = Path.home() / ".codex" / "config.toml" + codex_config.parent.mkdir(parents=True, exist_ok=True) + + if codex_config.exists(): + raw = codex_config.read_text(encoding="utf-8") + try: + _existing_codex_profile_names(raw) + except (tomllib.TOMLDecodeError, ValueError) as exc: + print_error(f"Could not parse {codex_config} — skipping worker-profile registration.") + print_info(str(exc)) + return + else: + raw = "" + + updated_raw, existed_before = _upsert_codex_worker_profile_section(raw) + try: + tomllib.loads(updated_raw) + except tomllib.TOMLDecodeError as exc: + print_error( + f"Could not update {codex_config} — worker-profile registration would create invalid TOML." + ) + print_info(str(exc)) + return + if updated_raw == raw: + print_info("Codex worker profile already up to date.") + return + + codex_config.write_text(updated_raw, encoding="utf-8") + if existed_before: + print_success(f"Updated Codex worker profile in {codex_config}") + else: + print_success(f"Registered Codex worker profile in {codex_config}") + + def _ensure_mapping_section(config_dict: dict, key: str) -> dict: """Ensure a top-level YAML section is a mapping before mutating it.""" section = config_dict.get(key) @@ -617,7 +737,7 @@ def _print_codex_config_guidance(config_path: Path) -> None: """Explain where Codex users should configure Ouroboros vs. Codex settings.""" print_info(f"Configure Ouroboros runtime and per-role model overrides in {config_path}.") print_info( - "Use ~/.codex/config.toml only for the Codex MCP/env hookup and Codex profile anchors." + "Use ~/.codex/config.toml for the Codex MCP/env hookup, Codex profile anchors, and [profiles.ouroboros-worker] worker overrides." ) @@ -692,6 +812,7 @@ def _setup_codex(codex_path: str, *, mcp_mode: CodexMcpMode = "auto") -> None: # Register MCP server in Codex config (~/.codex/config.toml) _register_codex_mcp_server(mode=mcp_mode) _register_codex_default_profiles() + _register_codex_worker_profile() _print_codex_config_guidance(config_path) diff --git a/src/ouroboros/codex/runtime_profile.py b/src/ouroboros/codex/runtime_profile.py new file mode 100644 index 000000000..63dcc28a3 --- /dev/null +++ b/src/ouroboros/codex/runtime_profile.py @@ -0,0 +1,76 @@ +"""Codex-side mapping for the orchestrator-level ``runtime_profile``. + +The ``OrchestratorConfig.runtime_profile`` setting names a profile in the +orchestrator's own vocabulary (e.g. ``"worker"``). The Codex backend +translates that name to its own ``--profile`` identifier and applies it +at command-build time. Both the orchestrator runtime +(``ouroboros.orchestrator.codex_cli_runtime``) and the LLM provider +adapter (``ouroboros.providers.codex_cli_adapter``) share this module so +the mapping is single-sourced. + +The module is intentionally Codex-local. Future Agent OS phases that add +OpenCode, Hermes, Claude Code, or LiteLLM mappings should each provide +their own backend-local mapping module rather than expanding this one — +the orchestrator surface only owns the *name*, not the per-backend +translation. + +It also lives outside of ``ouroboros.orchestrator`` to avoid a circular +import: ``ouroboros.orchestrator.__init__`` pulls in the runner, which +pulls in the providers package, which is what imports this module from +the Codex LLM adapter. +""" + +from __future__ import annotations + +from typing import Any + +# Maps the orchestrator-level ``runtime_profile`` value to the Codex-side +# ``--profile`` name. Phase 1 only ships ``worker``; new entries should land +# alongside the matching ``[profiles.]`` section written by setup so +# operators always have a managed home for the per-profile overrides. +RUNTIME_PROFILE_TO_CODEX_PROFILE: dict[str, str] = { + "worker": "ouroboros-worker", +} + + +def resolve_codex_profile( + runtime_profile: str | None, + *, + logger: Any, + log_namespace: str, +) -> str | None: + """Translate an orchestrator runtime_profile to a Codex ``--profile`` name. + + Args: + runtime_profile: The orchestrator-level profile name. ``None`` or an + empty string means "no profile requested" — every Codex code path + then preserves its current default user-config behaviour. + logger: The caller's structured logger (the same object the call site + uses for its own warnings). Passing it through keeps the warning + attributable to that namespace and keeps existing + ``patch("module.log.warning")`` test seams intact. + log_namespace: Caller's structured-log namespace (e.g. + ``"codex_cli_runtime"`` or ``"codex_cli_adapter"``). Used as the + event prefix for the ``runtime_profile_unmapped`` warning so the + event name carries the call site even though the function lives + in a shared module. + + Returns: + The Codex profile name when the orchestrator profile maps to one, + otherwise ``None``. An unmapped non-empty value emits a structured + warning via the caller's logger so the existing fallback path runs + without surprises. + """ + if not runtime_profile: + return None + mapped = RUNTIME_PROFILE_TO_CODEX_PROFILE.get(runtime_profile) + if mapped is None: + logger.warning( + f"{log_namespace}.runtime_profile_unmapped", + runtime_profile=runtime_profile, + hint="No Codex backend mapping; running without --profile.", + ) + return mapped + + +__all__ = ["RUNTIME_PROFILE_TO_CODEX_PROFILE", "resolve_codex_profile"] diff --git a/src/ouroboros/config/__init__.py b/src/ouroboros/config/__init__.py index 486cf544c..3f95e1244 100644 --- a/src/ouroboros/config/__init__.py +++ b/src/ouroboros/config/__init__.py @@ -55,6 +55,7 @@ get_qa_model, get_reflect_model, get_runtime_controls_config, + get_runtime_profile, get_semantic_model, get_usage_limit_pause_seconds, get_wonder_model, @@ -80,6 +81,7 @@ ProviderCredentials, ResilienceConfig, RuntimeControlsConfig, + RuntimeProfileConfig, TierConfig, get_config_dir, get_default_config, @@ -101,6 +103,7 @@ "ExecutionConfig", "ResilienceConfig", "RuntimeControlsConfig", + "RuntimeProfileConfig", "EvaluationConfig", "ConsensusConfig", "PersistenceConfig", @@ -141,6 +144,7 @@ "get_ontology_analysis_model", "get_reflect_model", "get_runtime_controls_config", + "get_runtime_profile", "get_semantic_model", "get_usage_limit_pause_seconds", "get_wonder_model", diff --git a/src/ouroboros/config/loader.py b/src/ouroboros/config/loader.py index d6becb416..14860999f 100644 --- a/src/ouroboros/config/loader.py +++ b/src/ouroboros/config/loader.py @@ -9,6 +9,7 @@ create_default_config: Create default configuration files ensure_config_dir: Ensure ~/.ouroboros/ directory exists get_agent_runtime_backend: Get orchestrator runtime backend from env var or config + get_runtime_profile: Get orchestrator backend profile (e.g. "worker") from env var or config get_agent_permission_mode: Get orchestrator permission mode from env var or config get_llm_backend: Get LLM-only backend from env var or config get_llm_permission_mode: Get LLM-only permission mode from env var or config @@ -800,6 +801,32 @@ def get_max_parallel_workers() -> int: ) +def get_runtime_profile() -> str | None: + """Get the orchestrator backend profile from env var or config file. + + Priority: + 1. OUROBOROS_RUNTIME_PROFILE environment variable + 2. config.yaml orchestrator.runtime_profile.backend_profile + 3. None (no profile — backends keep their default user-config behavior) + + Returns: + The backend profile name (e.g. ``"worker"``) or None. + """ + env_value = os.environ.get("OUROBOROS_RUNTIME_PROFILE", "").strip() + if env_value: + return env_value + + try: + config = load_config() + profile = config.orchestrator.runtime_profile + if profile is not None and profile.backend_profile: + return profile.backend_profile + except ConfigError: + pass + + return None + + def get_codex_cli_path() -> str | None: """Get Codex CLI path from environment variable or config file. diff --git a/src/ouroboros/config/models.py b/src/ouroboros/config/models.py index c4be634e8..fb6ec5b35 100644 --- a/src/ouroboros/config/models.py +++ b/src/ouroboros/config/models.py @@ -22,7 +22,7 @@ """ from pathlib import Path -from typing import Literal +from typing import Any, Literal from pydantic import BaseModel, Field, field_validator @@ -330,11 +330,46 @@ class LoggingConfig(BaseModel, frozen=True): include_reasoning: bool = True +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. + """ + + backend_profile: str | None = None + default: str | None = None + stages: dict[str, str] = Field(default_factory=dict) + + @field_validator("backend_profile") + @classmethod + def _validate_backend_profile(cls, value: str | None) -> str | None: + """Normalize backend-native profile names without constraining vocabulary.""" + if value is None: + return None + candidate = value.strip() + if not candidate: + raise ValueError("runtime_profile.backend_profile must not be empty") + return candidate + + class OrchestratorConfig(BaseModel, frozen=True): """Orchestrator runtime configuration. Attributes: runtime_backend: Agent runtime backend to use for orchestrator execution. + runtime_profile: Optional Agent OS runtime profile object. + ``runtime_profile.backend_profile`` selects backend-native profiles + such as Codex ``--profile ouroboros-worker``. Default ``None`` + preserves the backend's normal user-config behavior. The ``default`` + and ``stages`` fields reserve the same object contract used by the + stage-routing stack so this public key does not split into + incompatible string-vs-table meanings. permission_mode: Default permission mode for local agent runtimes. opencode_permission_mode: Default permission mode for OpenCode agent runtimes. cli_path: Path to Claude CLI binary. Supports: @@ -367,6 +402,16 @@ class OrchestratorConfig(BaseModel, frozen=True): """ runtime_backend: Literal["claude", "codex", "opencode", "hermes", "gemini"] = "claude" + runtime_profile: RuntimeProfileConfig | None = None + + @field_validator("runtime_profile", mode="before") + @classmethod + def _coerce_runtime_profile(cls, value: Any) -> Any: + """Accept the legacy PR #505 string shorthand as backend_profile.""" + if isinstance(value, str): + return {"backend_profile": value} + return value + permission_mode: Literal["default", "acceptEdits", "bypassPermissions"] = "acceptEdits" opencode_permission_mode: Literal["default", "acceptEdits", "bypassPermissions"] = ( "bypassPermissions" diff --git a/src/ouroboros/orchestrator/codex_cli_runtime.py b/src/ouroboros/orchestrator/codex_cli_runtime.py index c14d3fa7f..016799868 100644 --- a/src/ouroboros/orchestrator/codex_cli_runtime.py +++ b/src/ouroboros/orchestrator/codex_cli_runtime.py @@ -23,6 +23,7 @@ build_codex_child_env, resolve_codex_cli_path, ) +from ouroboros.codex.runtime_profile import resolve_codex_profile from ouroboros.codex_permissions import ( build_codex_exec_permission_args, resolve_codex_permission_mode, @@ -101,6 +102,7 @@ def __init__( skills_dir: str | Path | None = None, skill_dispatcher: SkillDispatchHandler | None = None, llm_backend: str | None = None, + runtime_profile: str | None = None, ) -> None: self._cli_path = self._resolve_cli_path(cli_path) self._permission_mode = self._resolve_permission_mode(permission_mode) @@ -109,6 +111,12 @@ def __init__( self._skills_dir = self._resolve_skills_dir(skills_dir) self._skill_dispatcher = skill_dispatcher self._llm_backend = llm_backend or self._default_llm_backend + self._runtime_profile = runtime_profile + self._codex_profile = resolve_codex_profile( + runtime_profile, + logger=log, + log_namespace=self._log_namespace, + ) self._builtin_mcp_handlers: dict[str, Any] | None = None log.info( @@ -117,6 +125,8 @@ def __init__( permission_mode=permission_mode, model=model, cwd=self._cwd, + runtime_profile=runtime_profile, + codex_profile=self._codex_profile, skills_dir=( str(self._skills_dir) if self._skills_dir is not None else self._skills_package_uri ), @@ -625,6 +635,13 @@ def _build_command( """Build the CLI command args. Prompt is fed via stdin separately.""" command = [self._cli_path, "exec"] + # Codex accepts one active --profile. The backend runtime profile is + # the worker-isolation boundary, so it owns that singular flag when + # configured; role/task profile resolution may still contribute a + # model fallback below, but not a second --profile. + if self._codex_profile: + command.extend(["--profile", self._codex_profile]) + command.extend( [ "--json", @@ -641,7 +658,7 @@ def _build_command( command.extend(["--model", normalized_model]) else: runtime_model, runtime_profile = self._resolve_runtime_codex_config(runtime_handle) - if runtime_profile: + if runtime_profile and not self._codex_profile: command.extend(["--profile", runtime_profile]) else: normalized_runtime_model = self._normalize_model(runtime_model) diff --git a/src/ouroboros/orchestrator/runtime_factory.py b/src/ouroboros/orchestrator/runtime_factory.py index b54ffa46c..a25a9c635 100644 --- a/src/ouroboros/orchestrator/runtime_factory.py +++ b/src/ouroboros/orchestrator/runtime_factory.py @@ -11,6 +11,7 @@ get_codex_cli_path, get_hermes_cli_path, get_llm_backend, + get_runtime_profile, ) from ouroboros.orchestrator.adapter import AgentRuntime, ClaudeAgentAdapter from ouroboros.orchestrator.codex_cli_runtime import CodexCliRuntime @@ -84,6 +85,7 @@ def create_agent_runtime( if resolved_backend == "codex": return CodexCliRuntime( cli_path=cli_path or get_codex_cli_path(), + runtime_profile=get_runtime_profile(), **runtime_kwargs, ) diff --git a/src/ouroboros/providers/codex_cli_adapter.py b/src/ouroboros/providers/codex_cli_adapter.py index f90386591..c4dba411c 100644 --- a/src/ouroboros/providers/codex_cli_adapter.py +++ b/src/ouroboros/providers/codex_cli_adapter.py @@ -25,6 +25,7 @@ build_codex_child_env, resolve_codex_cli_path, ) +from ouroboros.codex.runtime_profile import resolve_codex_profile from ouroboros.codex_permissions import ( build_codex_exec_permission_args, resolve_codex_permission_mode, @@ -50,6 +51,8 @@ log = structlog.get_logger() _SAFE_MODEL_NAME_PATTERN = re.compile(r"^[A-Za-z0-9_./:@-]+$") + + _RETRYABLE_ERROR_PATTERNS = ( "rate limit", "temporarily unavailable", @@ -85,6 +88,7 @@ def __init__( max_retries: int = 3, ephemeral: bool = True, timeout: float | None = None, + runtime_profile: str | None = None, ) -> None: self._cli_path = self._resolve_cli_path(cli_path) self._cwd = str(Path(cwd).expanduser()) if cwd is not None else os.getcwd() @@ -95,6 +99,12 @@ def __init__( self._max_retries = max_retries self._ephemeral = ephemeral self._timeout = timeout if timeout and timeout > 0 else None + self._runtime_profile = runtime_profile + self._codex_profile = resolve_codex_profile( + runtime_profile, + logger=log, + log_namespace=self._log_namespace, + ) def _resolve_permission_mode(self, permission_mode: str | None) -> str: """Validate and normalize the adapter permission mode.""" @@ -358,16 +368,22 @@ def _build_command( The prompt is always fed via stdin to avoid ARG_MAX limits. """ - command = [ - self._cli_path, - "exec", - "--json", - "--skip-git-repo-check", - "-C", - self._cwd, - "--output-last-message", - output_last_message_path, - ] + command = [self._cli_path, "exec"] + # Codex has one --profile selector. Runtime-profile worker isolation + # owns that flag when configured; task-profile resolution may still + # contribute a --model fallback, but must not emit a competing profile. + if self._codex_profile: + command.extend(["--profile", self._codex_profile]) + command.extend( + [ + "--json", + "--skip-git-repo-check", + "-C", + self._cwd, + "--output-last-message", + output_last_message_path, + ] + ) command.extend(self._build_permission_args()) @@ -377,7 +393,7 @@ def _build_command( if output_schema_path: command.extend(["--output-schema", output_schema_path]) - if profile: + if profile and not self._codex_profile: command.extend(["--profile", profile]) elif model: command.extend(["--model", model]) @@ -691,7 +707,9 @@ async def _complete_once( effective_config = resolved.config prompt = self._build_prompt(messages, max_turns=effective_config.max_turns) normalized_model = ( - None if resolved.backend_profile else self._normalize_model(effective_config.model) + None + if resolved.backend_profile and not self._codex_profile + else self._normalize_model(effective_config.model) ) output_fd, output_path_str = tempfile.mkstemp(prefix=self._tempfile_prefix, suffix=".txt") os.close(output_fd) diff --git a/src/ouroboros/providers/factory.py b/src/ouroboros/providers/factory.py index 3c06d37fe..960182ca8 100644 --- a/src/ouroboros/providers/factory.py +++ b/src/ouroboros/providers/factory.py @@ -13,6 +13,7 @@ get_gemini_cli_path, get_llm_backend, get_llm_permission_mode, + get_runtime_profile, ) from ouroboros.providers.base import LLMAdapter from ouroboros.providers.claude_code_adapter import ClaudeCodeAdapter @@ -170,6 +171,7 @@ def create_llm_adapter( on_message=on_message, timeout=timeout, max_retries=max_retries, + runtime_profile=get_runtime_profile(), ) if resolved_backend == "gemini": return GeminiCLIAdapter( diff --git a/tests/unit/cli/test_setup.py b/tests/unit/cli/test_setup.py index 80fd758cf..d939767d5 100644 --- a/tests/unit/cli/test_setup.py +++ b/tests/unit/cli/test_setup.py @@ -211,6 +211,181 @@ def test_register_codex_default_profiles_preserves_existing_profile( assert "[profiles.ouroboros-deep]" in contents assert "[profiles.ouroboros-frontier]" in contents + def test_register_codex_worker_profile_writes_section(self, tmp_path: Path) -> None: + """First-time setup creates the [profiles.ouroboros-worker] block.""" + with patch("pathlib.Path.home", return_value=tmp_path): + setup_cmd._register_codex_worker_profile() + + contents = (tmp_path / ".codex" / "config.toml").read_text(encoding="utf-8") + + assert "[profiles.ouroboros-worker]" in contents + assert "Ouroboros Agent OS runtime profile for Codex worker subprocesses." in contents + assert "orchestrator.runtime_profile.backend_profile: worker" in contents + + def test_register_codex_worker_profile_preserves_mcp_and_default_profiles( + self, tmp_path: Path + ) -> None: + """Worker-profile registration must not touch existing MCP/profile anchors.""" + with patch("pathlib.Path.home", return_value=tmp_path): + setup_cmd._register_codex_mcp_server() + setup_cmd._register_codex_default_profiles() + setup_cmd._register_codex_worker_profile() + + contents = (tmp_path / ".codex" / "config.toml").read_text(encoding="utf-8") + + assert contents.count("[mcp_servers.ouroboros]") == 1 + assert contents.count("[mcp_servers.ouroboros.env]") == 1 + assert contents.count("[profiles.ouroboros-fast]") == 1 + assert contents.count("[profiles.ouroboros-worker]") == 1 + assert contents.index("[mcp_servers.ouroboros]") < contents.index( + "[profiles.ouroboros-worker]" + ) + + def test_register_codex_worker_profile_preserves_user_overrides(self, tmp_path: Path) -> None: + """Rerunning setup must not clobber operator-authored worker keys.""" + codex_config = tmp_path / ".codex" / "config.toml" + codex_config.parent.mkdir(parents=True) + codex_config.write_text( + "\n".join( + [ + "# Ouroboros Agent OS runtime profile for Codex worker subprocesses.", + "# Activated when ~/.ouroboros/config.yaml sets " + "`orchestrator.runtime_profile.backend_profile: worker`", + "# (or the OUROBOROS_RUNTIME_PROFILE=worker env var). Add per-worker Codex", + "# overrides below — for example a different model, sandbox, or notify hook —", + "# without affecting interactive `codex` sessions that share this config file.", + "", + "[profiles.ouroboros-worker]", + 'model = "o3-mini"', + "notify = []", + 'sandbox = "workspace-write"', + "", + "[profiles.ouroboros-worker.shell_environment_policy]", + 'inherit = "core"', + "", + ] + ) + + "\n", + encoding="utf-8", + ) + + with patch("pathlib.Path.home", return_value=tmp_path): + setup_cmd._register_codex_worker_profile() + + contents = codex_config.read_text(encoding="utf-8") + + assert contents.count("[profiles.ouroboros-worker]") == 1 + assert 'model = "o3-mini"' in contents + assert "notify = []" in contents + assert 'sandbox = "workspace-write"' in contents + assert "[profiles.ouroboros-worker.shell_environment_policy]" in contents + assert 'inherit = "core"' in contents + assert contents.count("Ouroboros Agent OS runtime profile") == 1 + + def test_register_codex_worker_profile_idempotent_with_user_overrides( + self, tmp_path: Path + ) -> None: + """Multiple reruns must converge without key loss or comment bloat.""" + codex_config = tmp_path / ".codex" / "config.toml" + + with patch("pathlib.Path.home", return_value=tmp_path): + setup_cmd._register_codex_worker_profile() + existing = codex_config.read_text(encoding="utf-8") + codex_config.write_text( + existing.rstrip() + + "\n" + + 'model = "o3-mini"\nnotify = []\nsandbox = "workspace-write"\n', + encoding="utf-8", + ) + + after_user_edit = codex_config.read_text(encoding="utf-8") + setup_cmd._register_codex_worker_profile() + after_second = codex_config.read_text(encoding="utf-8") + setup_cmd._register_codex_worker_profile() + after_third = codex_config.read_text(encoding="utf-8") + + for snapshot in (after_second, after_third): + assert snapshot.count("[profiles.ouroboros-worker]") == 1 + assert snapshot.count("Ouroboros Agent OS runtime profile") == 1 + assert 'model = "o3-mini"' in snapshot + assert "notify = []" in snapshot + assert 'sandbox = "workspace-write"' in snapshot + assert after_second == after_user_edit + assert after_third == after_second + + def test_register_codex_worker_profile_idempotent_when_user_inserts_own_comment( + self, tmp_path: Path + ) -> None: + """Operator comments between managed comments and header must not stack blocks.""" + codex_config = tmp_path / ".codex" / "config.toml" + codex_config.parent.mkdir(parents=True) + codex_config.write_text( + "\n".join( + [ + "# Ouroboros Agent OS runtime profile for Codex worker subprocesses.", + "# Activated when ~/.ouroboros/config.yaml sets " + "`orchestrator.runtime_profile.backend_profile: worker`", + "# (or the OUROBOROS_RUNTIME_PROFILE=worker env var). Add per-worker Codex", + "# overrides below — for example a different model, sandbox, or notify hook —", + "# without affecting interactive `codex` sessions that share this config file.", + "", + "# Operator note: keep this profile aligned with prod-staging.", + "[profiles.ouroboros-worker]", + 'model = "o3-mini"', + "", + ] + ) + + "\n", + encoding="utf-8", + ) + + with patch("pathlib.Path.home", return_value=tmp_path): + setup_cmd._register_codex_worker_profile() + after_first = codex_config.read_text(encoding="utf-8") + setup_cmd._register_codex_worker_profile() + after_second = codex_config.read_text(encoding="utf-8") + + for snapshot in (after_first, after_second): + assert snapshot.count("Ouroboros Agent OS runtime profile") == 1 + assert "# Operator note: keep this profile aligned with prod-staging." in snapshot + assert 'model = "o3-mini"' in snapshot + assert snapshot.count("[profiles.ouroboros-worker]") == 1 + assert after_second == after_first + + def test_register_codex_worker_profile_skips_non_table_profiles_value( + self, tmp_path: Path + ) -> None: + """Valid TOML with scalar profiles must not be corrupted by worker setup.""" + codex_config = tmp_path / ".codex" / "config.toml" + codex_config.parent.mkdir(parents=True) + original = 'profiles = "oops"\n' + codex_config.write_text(original, encoding="utf-8") + + with ( + patch("pathlib.Path.home", return_value=tmp_path), + patch("ouroboros.cli.commands.setup.print_error") as mock_error, + ): + setup_cmd._register_codex_worker_profile() + + mock_error.assert_called_once() + assert codex_config.read_text(encoding="utf-8") == original + + def test_register_codex_worker_profile_skips_invalid_toml(self, tmp_path: Path) -> None: + """Malformed TOML should produce an error message and leave the file alone.""" + codex_config = tmp_path / ".codex" / "config.toml" + codex_config.parent.mkdir(parents=True) + original = "this is = not = valid = toml\n[unterminated" + codex_config.write_text(original, encoding="utf-8") + + with ( + patch("pathlib.Path.home", return_value=tmp_path), + patch("ouroboros.cli.commands.setup.print_error") as mock_error, + ): + setup_cmd._register_codex_worker_profile() + + mock_error.assert_called_once() + assert codex_config.read_text(encoding="utf-8") == original + def test_install_codex_artifacts_installs_rules_and_skills(self, tmp_path: Path) -> None: """Codex setup should install both managed rules and managed skills.""" rules_path = tmp_path / ".codex" / "rules" @@ -245,6 +420,9 @@ def test_setup_codex_updates_config_and_prints_config_split_guidance( patch("ouroboros.cli.commands.setup._install_codex_artifacts") as mock_install, patch("ouroboros.cli.commands.setup._register_codex_mcp_server") as mock_register, patch("ouroboros.cli.commands.setup._register_codex_default_profiles") as mock_profiles, + patch( + "ouroboros.cli.commands.setup._register_codex_worker_profile" + ) as mock_worker_profile, patch("ouroboros.cli.commands.setup.print_info") as mock_info, ): setup_cmd._setup_codex("/usr/local/bin/codex") @@ -275,6 +453,7 @@ def test_setup_codex_updates_config_and_prints_config_split_guidance( mock_install.assert_called_once_with() mock_register.assert_called_once_with(mode="auto") mock_profiles.assert_called_once_with() + mock_worker_profile.assert_called_once_with() info_messages = [call.args[0] for call in mock_info.call_args_list] assert any("Config saved to" in message for message in info_messages) diff --git a/tests/unit/config/test_loader.py b/tests/unit/config/test_loader.py index cd50366ab..227dbcd1d 100644 --- a/tests/unit/config/test_loader.py +++ b/tests/unit/config/test_loader.py @@ -34,6 +34,7 @@ get_qa_model, get_reflect_model, get_runtime_controls_config, + get_runtime_profile, get_semantic_model, get_usage_limit_pause_seconds, get_wonder_model, @@ -51,6 +52,7 @@ OuroborosConfig, ResilienceConfig, RuntimeControlsConfig, + RuntimeProfileConfig, ) from ouroboros.core.errors import ConfigError @@ -1326,3 +1328,52 @@ def test_config_roundtrip_preserves_values(self, tmp_path: Path) -> None: frontier = config.economics.tiers["frontier"] assert frontier.cost_factor == 30 + + +class TestRuntimeProfileConfigAccess: + def test_get_runtime_profile_defaults_to_none(self) -> None: + """No env, no config — runtime_profile resolves to None.""" + config = OuroborosConfig() + with patch("ouroboros.config.loader.load_config", return_value=config): + assert get_runtime_profile() is None + + def test_get_runtime_profile_prefers_env(self, monkeypatch: pytest.MonkeyPatch) -> None: + """Environment variable overrides config for runtime_profile.""" + monkeypatch.setenv("OUROBOROS_RUNTIME_PROFILE", "worker") + config = OuroborosConfig( + orchestrator=OrchestratorConfig( + runtime_profile=RuntimeProfileConfig(backend_profile="future-worker") + ) + ) + with patch("ouroboros.config.loader.load_config", return_value=config): + assert get_runtime_profile() == "worker" + + def test_get_runtime_profile_falls_back_to_config(self) -> None: + config = OuroborosConfig( + orchestrator=OrchestratorConfig( + runtime_profile=RuntimeProfileConfig(backend_profile="worker") + ) + ) + with patch("ouroboros.config.loader.load_config", return_value=config): + assert get_runtime_profile() == "worker" + + def test_get_runtime_profile_accepts_legacy_string_shorthand(self) -> None: + config = OuroborosConfig(orchestrator=OrchestratorConfig(runtime_profile="worker")) + with patch("ouroboros.config.loader.load_config", return_value=config): + assert get_runtime_profile() == "worker" + + def test_get_runtime_profile_accepts_unknown_backend_profile(self) -> None: + config = OuroborosConfig( + orchestrator=OrchestratorConfig( + runtime_profile=RuntimeProfileConfig(backend_profile="future-worker") + ) + ) + with patch("ouroboros.config.loader.load_config", return_value=config): + assert get_runtime_profile() == "future-worker" + + def test_get_runtime_profile_ignores_stage_only_profile(self) -> None: + config = OuroborosConfig( + orchestrator=OrchestratorConfig(runtime_profile=RuntimeProfileConfig(default="codex")) + ) + with patch("ouroboros.config.loader.load_config", return_value=config): + assert get_runtime_profile() is None diff --git a/tests/unit/config/test_models.py b/tests/unit/config/test_models.py index 370120ef2..73eeb64ec 100644 --- a/tests/unit/config/test_models.py +++ b/tests/unit/config/test_models.py @@ -22,6 +22,7 @@ ProviderCredentials, ResilienceConfig, RuntimeControlsConfig, + RuntimeProfileConfig, TierConfig, get_config_dir, get_default_config, @@ -656,3 +657,23 @@ def test_get_config_dir_is_in_home(self) -> None: config_dir = get_config_dir() assert config_dir.parent == Path.home() assert config_dir.name == ".ouroboros" + + +class TestRuntimeProfileConfig: + def test_runtime_profile_backend_profile_accepts_future_values(self) -> None: + profile = RuntimeProfileConfig(backend_profile="future-worker") + assert profile.backend_profile == "future-worker" + + def test_runtime_profile_backend_profile_strips_whitespace(self) -> None: + profile = RuntimeProfileConfig(backend_profile=" worker ") + assert profile.backend_profile == "worker" + + def test_runtime_profile_backend_profile_rejects_empty_string(self) -> None: + with pytest.raises(ValueError) as exc_info: + RuntimeProfileConfig(backend_profile=" ") + assert "runtime_profile.backend_profile" in str(exc_info.value) + + def test_orchestrator_runtime_profile_string_shorthand(self) -> None: + config = OrchestratorConfig(runtime_profile="worker") + assert config.runtime_profile is not None + assert config.runtime_profile.backend_profile == "worker" diff --git a/tests/unit/orchestrator/test_codex_cli_runtime.py b/tests/unit/orchestrator/test_codex_cli_runtime.py index 7ed99037b..5e22c222d 100644 --- a/tests/unit/orchestrator/test_codex_cli_runtime.py +++ b/tests/unit/orchestrator/test_codex_cli_runtime.py @@ -322,6 +322,33 @@ def test_build_command_does_not_double_prefix_prefixed_runtime_handle_kind(self) assert command[command.index("--profile") + 1] == "ouroboros-deep" assert "--model" not in command + def test_runtime_profile_prevents_duplicate_role_profile_flags(self) -> None: + """Worker isolation owns Codex's singular --profile flag when both resolve.""" + runtime = CodexCliRuntime(cli_path="codex", cwd="/tmp/project", runtime_profile="worker") + runtime_handle = RuntimeHandle( + backend="codex_cli", + kind="implementation_session", + metadata={"session_role": "implementation"}, + ) + config = OuroborosConfig( + llm_profiles={ + "standard": { + "providers": {"codex": {"profile": "ouroboros-standard"}}, + }, + }, + llm_role_profiles={"agent_runtime_implementation": "standard"}, + ) + + with patch("ouroboros.providers.profiles.load_config", return_value=config): + command = runtime._build_command( + output_last_message_path="/tmp/out.txt", + runtime_handle=runtime_handle, + ) + + assert command.count("--profile") == 1 + assert command[command.index("--profile") + 1] == "ouroboros-worker" + assert "ouroboros-standard" not in command + def test_build_command_uses_runtime_profile_provider_model_fallback(self) -> None: """Codex runtime profiles without Codex-native profile anchors should use models.""" runtime = CodexCliRuntime(cli_path="codex", cwd="/tmp/project") @@ -444,6 +471,48 @@ def test_build_command_uses_explicit_runtime_profile_model_fallback(self) -> Non assert command[command.index("--model") + 1] == "gpt-5.5" assert "--profile" not in command + def test_build_command_omits_profile_flag_when_runtime_profile_unset(self) -> None: + """Default runtime_profile=None preserves existing command shape (regression).""" + runtime = CodexCliRuntime(cli_path="codex", cwd="/tmp/project") + + command = runtime._build_command(output_last_message_path="/tmp/out.txt") + + assert "--profile" not in command + + def test_build_command_adds_worker_profile_when_configured(self) -> None: + """runtime_profile='worker' maps to Codex `--profile ouroboros-worker`.""" + runtime = CodexCliRuntime( + cli_path="codex", + cwd="/tmp/project", + runtime_profile="worker", + ) + + command = runtime._build_command(output_last_message_path="/tmp/out.txt") + + assert "--profile" in command + profile_index = command.index("--profile") + assert command[profile_index + 1] == "ouroboros-worker" + # Profile must come before the rest of the args so Codex resolves + # the profile-managed defaults before per-flag overrides. + assert profile_index < command.index("--json") + + def test_build_command_skips_unknown_runtime_profile_with_warning(self) -> None: + """Unmapped runtime_profile values fall back to no profile flag and log a warning.""" + with patch("ouroboros.orchestrator.codex_cli_runtime.log.warning") as mock_warning: + runtime = CodexCliRuntime( + cli_path="codex", + cwd="/tmp/project", + runtime_profile="future-tier", + ) + + command = runtime._build_command(output_last_message_path="/tmp/out.txt") + + assert "--profile" not in command + mock_warning.assert_called_once() + warning_args = mock_warning.call_args + assert warning_args.args[0] == "codex_cli_runtime.runtime_profile_unmapped" + assert warning_args.kwargs["runtime_profile"] == "future-tier" + def test_resolve_cli_path_falls_back_from_wrapper(self, tmp_path: Path) -> None: """Runtime should bypass wrappers the same way provider adapters do.""" wrapper = self._write_wrapper(tmp_path / "codex-wrapper") diff --git a/tests/unit/orchestrator/test_runtime_factory.py b/tests/unit/orchestrator/test_runtime_factory.py index fabaa739f..9446d5738 100644 --- a/tests/unit/orchestrator/test_runtime_factory.py +++ b/tests/unit/orchestrator/test_runtime_factory.py @@ -83,6 +83,48 @@ def test_create_codex_runtime_uses_configured_cli_path(self) -> None: assert mock_create_dispatcher.call_args.kwargs["cwd"] == "/tmp/project" assert mock_create_dispatcher.call_args.kwargs["runtime_backend"] == "codex" + def test_create_codex_runtime_propagates_runtime_profile(self) -> None: + """``get_runtime_profile()`` must reach CodexCliRuntime via the factory. + + The runtime is the only place that translates the orchestrator + ``runtime_profile`` into a Codex ``--profile`` argument, so a + regression in the factory wiring would silently disable + worker-subprocess isolation. Lock the path under test. + """ + with ( + patch( + "ouroboros.orchestrator.runtime_factory.get_runtime_profile", + return_value="worker", + ), + patch( + "ouroboros.orchestrator.runtime_factory.create_codex_command_dispatcher", + return_value=object(), + ), + ): + runtime = create_agent_runtime(backend="codex", cwd="/tmp/project") + + assert isinstance(runtime, CodexCliRuntime) + assert runtime._runtime_profile == "worker" + assert runtime._codex_profile == "ouroboros-worker" + + def test_create_codex_runtime_default_profile_is_none(self) -> None: + """Unset profile must remain unset all the way through to the runtime.""" + with ( + patch( + "ouroboros.orchestrator.runtime_factory.get_runtime_profile", + return_value=None, + ), + patch( + "ouroboros.orchestrator.runtime_factory.create_codex_command_dispatcher", + return_value=object(), + ), + ): + runtime = create_agent_runtime(backend="codex", cwd="/tmp/project") + + assert isinstance(runtime, CodexCliRuntime) + assert runtime._runtime_profile is None + assert runtime._codex_profile is None + def test_create_claude_runtime_uses_factory_cwd_and_cli_path(self) -> None: """Claude runtime receives the same construction options as other backends.""" with patch( diff --git a/tests/unit/providers/test_codex_cli_adapter.py b/tests/unit/providers/test_codex_cli_adapter.py index cf8af61be..f86c8579e 100644 --- a/tests/unit/providers/test_codex_cli_adapter.py +++ b/tests/unit/providers/test_codex_cli_adapter.py @@ -313,6 +313,64 @@ def test_build_command_uses_dangerous_bypass_when_requested(self) -> None: assert "--dangerously-bypass-approvals-and-sandbox" in command + def test_build_command_omits_profile_flag_when_runtime_profile_unset(self) -> None: + """Default runtime_profile=None preserves existing command shape (regression).""" + adapter = CodexCliLLMAdapter(cli_path="codex") + + command = adapter._build_command( + output_last_message_path="/tmp/out.txt", + output_schema_path=None, + model=None, + ) + + assert "--profile" not in command + + def test_build_command_adds_worker_profile_when_configured(self) -> None: + """runtime_profile='worker' maps to Codex `--profile ouroboros-worker`.""" + adapter = CodexCliLLMAdapter(cli_path="codex", runtime_profile="worker") + + command = adapter._build_command( + output_last_message_path="/tmp/out.txt", + output_schema_path=None, + model=None, + ) + + assert "--profile" in command + profile_index = command.index("--profile") + assert command[profile_index + 1] == "ouroboros-worker" + assert profile_index < command.index("--json") + + def test_build_command_skips_unknown_runtime_profile_with_warning(self) -> None: + """Unmapped runtime_profile values fall back to no profile flag and log a warning.""" + with patch("ouroboros.providers.codex_cli_adapter.log.warning") as mock_warning: + adapter = CodexCliLLMAdapter(cli_path="codex", runtime_profile="future-tier") + + command = adapter._build_command( + output_last_message_path="/tmp/out.txt", + output_schema_path=None, + model=None, + ) + + assert "--profile" not in command + mock_warning.assert_called_once() + assert mock_warning.call_args.args[0] == "codex_cli_adapter.runtime_profile_unmapped" + assert mock_warning.call_args.kwargs["runtime_profile"] == "future-tier" + + def test_runtime_profile_prevents_duplicate_task_profile_flags(self) -> None: + """Worker isolation owns Codex's singular --profile flag over task profiles.""" + adapter = CodexCliLLMAdapter(cli_path="codex", runtime_profile="worker") + + command = adapter._build_command( + output_last_message_path="/tmp/out.txt", + output_schema_path=None, + model=None, + profile="ouroboros-fast", + ) + + assert command.count("--profile") == 1 + assert command[command.index("--profile") + 1] == "ouroboros-worker" + assert "ouroboros-fast" not in command + @pytest.mark.asyncio async def test_complete_success_reads_output_file(self) -> None: """Successful completions return the CLI output and session id.""" diff --git a/tests/unit/providers/test_factory.py b/tests/unit/providers/test_factory.py index 896962944..2336d2a7e 100644 --- a/tests/unit/providers/test_factory.py +++ b/tests/unit/providers/test_factory.py @@ -127,6 +127,42 @@ def test_creates_codex_adapter(self) -> None: assert isinstance(adapter, CodexCliLLMAdapter) assert adapter._cwd == "/tmp/project" + def test_creates_codex_adapter_propagates_runtime_profile( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """``get_runtime_profile()`` must reach CodexCliLLMAdapter via the factory. + + Same regression-lock as the orchestrator runtime: if the + provider factory ever drops ``runtime_profile=...`` from the + adapter call, worker-subprocess isolation breaks for every LLM + path that runs through Codex (interview, evaluation, …). + """ + monkeypatch.setattr( + "ouroboros.providers.factory.get_runtime_profile", + lambda: "worker", + ) + + adapter = create_llm_adapter(backend="codex", cwd="/tmp/project") + + assert isinstance(adapter, CodexCliLLMAdapter) + assert adapter._runtime_profile == "worker" + assert adapter._codex_profile == "ouroboros-worker" + + def test_creates_codex_adapter_default_profile_is_none( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Unset profile must remain unset on the adapter.""" + monkeypatch.setattr( + "ouroboros.providers.factory.get_runtime_profile", + lambda: None, + ) + + adapter = create_llm_adapter(backend="codex", cwd="/tmp/project") + + assert isinstance(adapter, CodexCliLLMAdapter) + assert adapter._runtime_profile is None + assert adapter._codex_profile is None + def test_creates_codex_adapter_uses_configured_cli_path( self, monkeypatch: pytest.MonkeyPatch ) -> None: