diff --git a/zenith/src/zenith_harness/acp_runner.py b/zenith/src/zenith_harness/acp_runner.py index 8525488..a7a71a1 100644 --- a/zenith/src/zenith_harness/acp_runner.py +++ b/zenith/src/zenith_harness/acp_runner.py @@ -89,22 +89,30 @@ class ACPError(Exception): pass -def _augment_acp_command(command: str, provider) -> str: +def _augment_acp_command( + command: str, provider, reasoning_effort: str | None = None +) -> str: """Append provider-specific config flags to the ACP launch command. For codex-acp this is the no-ask, no-sandbox combo — equivalent to `codex --dangerously-bypass-approvals-and-sandbox`, which codex-acp does not expose as a flag but accepts via `-c` overrides. + `reasoning_effort` is the per-role override from + ZENITH__REASONING_EFFORT (validated against + `config.VALID_REASONING_EFFORTS` at discovery); None keeps the + historical "xhigh" default. + For hermes the command is passed through unchanged. """ name = getattr(provider, "name", None) if name == "codex": + effort = reasoning_effort or "xhigh" return ( command + ' -c sandbox_mode="danger-full-access"' + ' -c approval_policy="never"' - + ' -c model_reasoning_effort="xhigh"' + + f' -c model_reasoning_effort="{effort}"' ) # hermes: no-op return command @@ -567,7 +575,11 @@ async def run_node( raise RuntimeError( f"No ACP command for role={role}. Set ZENITH_{role.upper()}_ACP_COMMAND." ) - acp_command = _augment_acp_command(acp_command, role_config.worker_provider) + acp_command = _augment_acp_command( + acp_command, + role_config.worker_provider, + role_config.worker_reasoning_effort, + ) workspace_dir = str(Path(cwd).expanduser().resolve() if cwd else store.workspace_dir(project_id)) project_bucket = str(store.zenith_dir(project_id)) @@ -733,7 +745,11 @@ async def run_terminal_review( "No ACP command for terminal reviewer. " "Set ZENITH_TERMINAL_REVIEWER_ACP_COMMAND." ) - acp_command = _augment_acp_command(acp_command, role_config.worker_provider) + acp_command = _augment_acp_command( + acp_command, + role_config.worker_provider, + role_config.worker_reasoning_effort, + ) workspace_dir = str(store.workspace_dir(project_id)) project_bucket = str(store.zenith_dir(project_id)) diff --git a/zenith/src/zenith_harness/cli.py b/zenith/src/zenith_harness/cli.py index f4cd9f4..095918e 100644 --- a/zenith/src/zenith_harness/cli.py +++ b/zenith/src/zenith_harness/cli.py @@ -8,7 +8,7 @@ import click from .assets import AssetLoader, iter_skill_directories -from .config import HarnessConfig +from .config import VALID_REASONING_EFFORTS, HarnessConfig from .envelope import render_task_list from .providers import ( ProviderDefinition, @@ -19,7 +19,7 @@ ) from .storage import ProjectStore -MCP_ENV_FORWARD_ALLOWLIST = ( +RUNTIME_ENV_FORWARD_ALLOWLIST = ( "ANTHROPIC_API_KEY", "ANTHROPIC_AUTH_TOKEN", "ANTHROPIC_BASE_URL", @@ -34,6 +34,9 @@ "GLM_API_KEY", "GLM_BASE_URL", "MAX_THINKING_TOKENS", + "ZENITH_WORKER_REASONING_EFFORT", + "ZENITH_VALIDATOR_REASONING_EFFORT", + "ZENITH_TERMINAL_REVIEWER_REASONING_EFFORT", "ZAI_API_KEY", "ZAI_BASE_URL", ) @@ -71,6 +74,9 @@ def cli() -> None: @click.option("--validator-acp-command", default=None) @click.option("--terminal-reviewer-provider", type=click.Choice(provider_names_for_role("worker")), default=None) @click.option("--terminal-reviewer-acp-command", default=None) +@click.option("--worker-reasoning-effort", type=click.Choice(VALID_REASONING_EFFORTS), default=None) +@click.option("--validator-reasoning-effort", type=click.Choice(VALID_REASONING_EFFORTS), default=None) +@click.option("--terminal-reviewer-reasoning-effort", type=click.Choice(VALID_REASONING_EFFORTS), default=None) @click.option("--zenith-home", type=click.Path(), default=None) @click.option("--workspace-dir", "workspace_dir", type=click.Path(exists=True), default=".") def init( @@ -82,6 +88,9 @@ def init( validator_acp_command: str | None, terminal_reviewer_provider: str | None, terminal_reviewer_acp_command: str | None, + worker_reasoning_effort: str | None, + validator_reasoning_effort: str | None, + terminal_reviewer_reasoning_effort: str | None, zenith_home: str | None, workspace_dir: str, ) -> None: @@ -109,7 +118,21 @@ def init( # 1) MCP / Codex config storage_env = _storage_env(zenith_home=zenith_home, workspace=workspace, selection=selection) - _write_bootstrap_config(workspace, selection, storage_env) + # Flags are sugar for the ZENITH_*_REASONING_EFFORT env vars and win over + # valid inherited shell settings. An invalid value already in the + # environment still fails fast at discover() above — flags override + # settings, they don't mask broken ones (the same validation would raise + # at server launch anyway). + effort_env = { + var: value + for var, value in ( + ("ZENITH_WORKER_REASONING_EFFORT", worker_reasoning_effort), + ("ZENITH_VALIDATOR_REASONING_EFFORT", validator_reasoning_effort), + ("ZENITH_TERMINAL_REVIEWER_REASONING_EFFORT", terminal_reviewer_reasoning_effort), + ) + if value + } + _write_bootstrap_config(workspace, selection, storage_env, effort_env) # 2) Per-provider agents + orchestrator prompt for provider in selection.providers(): @@ -311,10 +334,10 @@ def _storage_env( return env -def _forwarded_mcp_env() -> dict[str, str]: +def _forwarded_runtime_env() -> dict[str, str]: return { key: value - for key in MCP_ENV_FORWARD_ALLOWLIST + for key in RUNTIME_ENV_FORWARD_ALLOWLIST if (value := os.environ.get(key)) } @@ -353,12 +376,12 @@ def _write_bootstrap_config( workspace: Path, selection: ProviderSelection, storage_env: dict[str, str], + cli_env: dict[str, str], ) -> None: fmt = selection.orchestrator.config_format - env = {**selection.env(), **storage_env} + env = {**selection.env(), **storage_env, **_forwarded_runtime_env(), **cli_env} server_args = _mcp_server_args() if fmt == "mcp_json": - env = {**env, **_forwarded_mcp_env()} path = workspace / ".mcp.json" existing = ( json.loads(path.read_text(encoding="utf-8")) if path.exists() else {} diff --git a/zenith/src/zenith_harness/config.py b/zenith/src/zenith_harness/config.py index 0508f74..22adcc0 100644 --- a/zenith/src/zenith_harness/config.py +++ b/zenith/src/zenith_harness/config.py @@ -14,6 +14,14 @@ DEFAULT_MAX_PARALLEL_NODES = 4 +# codex-acp `model_reasoning_effort` values. Also a safety allowlist: the +# resolved value is spliced into a shell command line by acp_runner. Codex's +# "ultra" is deliberately excluded: it is not a reasoning tier (codex +# downgrades the request to "max" on the wire) but a switch to proactive +# multi-agent mode — a lane spawning its own agent swarm inside a harness +# that already orchestrates and validates per-lane work. +VALID_REASONING_EFFORTS = ("minimal", "low", "medium", "high", "xhigh", "max") + def _bundled_dir() -> Path: return (Path(__file__).resolve().parent / "bundled").resolve() @@ -35,6 +43,20 @@ def _resolve_max_parallel(value: str | None) -> int: return max(1, parsed) +def _resolve_reasoning_effort(value: str | None, *, env_var: str) -> str | None: + """None passes through (provider default); anything else must be on the + allowlist — a typo silently ignored would spend xhigh the user thought + they had dialed down.""" + if not value: + return None + if value not in VALID_REASONING_EFFORTS: + raise ValueError( + f"{env_var}={value!r} is not a valid reasoning effort; " + f"choose one of: {', '.join(VALID_REASONING_EFFORTS)}" + ) + return value + + @dataclass(frozen=True) class HarnessConfig: """Static configuration loaded from env. Per-call overrides allowed via `with_*`.""" @@ -50,6 +72,11 @@ class HarnessConfig: terminal_reviewer_provider_name: str | None terminal_reviewer_acp_command: str | None max_parallel_nodes: int = DEFAULT_MAX_PARALLEL_NODES + # Per-role reasoning effort for providers whose ACP command accepts one + # (codex today). None means the provider default ("xhigh" for codex). + worker_reasoning_effort: str | None = None + validator_reasoning_effort: str | None = None + terminal_reviewer_reasoning_effort: str | None = None @classmethod def discover(cls) -> HarnessConfig: @@ -96,6 +123,18 @@ def discover(cls) -> HarnessConfig: max_parallel_nodes=_resolve_max_parallel( os.environ.get("ZENITH_MAX_PARALLEL_NODES") ), + worker_reasoning_effort=_resolve_reasoning_effort( + os.environ.get("ZENITH_WORKER_REASONING_EFFORT"), + env_var="ZENITH_WORKER_REASONING_EFFORT", + ), + validator_reasoning_effort=_resolve_reasoning_effort( + os.environ.get("ZENITH_VALIDATOR_REASONING_EFFORT"), + env_var="ZENITH_VALIDATOR_REASONING_EFFORT", + ), + terminal_reviewer_reasoning_effort=_resolve_reasoning_effort( + os.environ.get("ZENITH_TERMINAL_REVIEWER_REASONING_EFFORT"), + env_var="ZENITH_TERMINAL_REVIEWER_REASONING_EFFORT", + ), ) # ------------------------------------------------------------------ @@ -197,6 +236,9 @@ def for_role( self.validator_provider_name or self.worker_provider_name ), worker_acp_command=self.resolved_validator_acp_command, + worker_reasoning_effort=( + self.validator_reasoning_effort or self.worker_reasoning_effort + ), ) if role == "terminal_reviewer": return replace( @@ -207,5 +249,10 @@ def for_role( or self.worker_provider_name ), worker_acp_command=self.resolved_terminal_reviewer_acp_command, + worker_reasoning_effort=( + self.terminal_reviewer_reasoning_effort + or self.validator_reasoning_effort + or self.worker_reasoning_effort + ), ) raise ValueError(f"unknown role: {role}") diff --git a/zenith/tests/test_acp_runner.py b/zenith/tests/test_acp_runner.py index d0349ae..412ea69 100644 --- a/zenith/tests/test_acp_runner.py +++ b/zenith/tests/test_acp_runner.py @@ -155,8 +155,21 @@ def test_augment_acp_command_codex_appends_bypass_flags(): assert out.startswith("codex-acp ") +def test_augment_acp_command_codex_reasoning_effort_override(): + out = _augment_acp_command("codex-acp", PROVIDERS["codex"], reasoning_effort="medium") + assert 'model_reasoning_effort="medium"' in out + assert "xhigh" not in out + # The bypass flags are effort-independent. + assert 'sandbox_mode="danger-full-access"' in out + assert 'approval_policy="never"' in out + + def test_augment_acp_command_claude_untouched(): assert _augment_acp_command("claude-agent-acp", PROVIDERS["claude"]) == "claude-agent-acp" + assert ( + _augment_acp_command("claude-agent-acp", PROVIDERS["claude"], reasoning_effort="low") + == "claude-agent-acp" + ) def test_codex_acp_env_preserves_node_path_when_bwrap_is_present( diff --git a/zenith/tests/test_cli.py b/zenith/tests/test_cli.py index e5f186d..87e640e 100644 --- a/zenith/tests/test_cli.py +++ b/zenith/tests/test_cli.py @@ -99,6 +99,108 @@ def test_codex_writes_codex_config( "then use Zenith to run this mission." in r.output ) + def test_claude_init_writes_reasoning_effort_env( + self, + runner: CliRunner, + workspace: Path, + env: dict[str, str], + monkeypatch: pytest.MonkeyPatch, + ) -> None: + monkeypatch.setenv("ZENITH_WORKER_REASONING_EFFORT", "high") + monkeypatch.setenv("ZENITH_VALIDATOR_REASONING_EFFORT", "medium") + monkeypatch.setenv("ZENITH_TERMINAL_REVIEWER_REASONING_EFFORT", "low") + + r = runner.invoke(cli, ["init", "--workspace-dir", str(workspace), "--agent", "claude"]) + assert r.exit_code == 0, r.output + + mcp = json.loads((workspace / ".mcp.json").read_text(encoding="utf-8")) + server_env = mcp["mcpServers"]["zenith"]["env"] + assert server_env["ZENITH_WORKER_REASONING_EFFORT"] == "high" + assert server_env["ZENITH_VALIDATOR_REASONING_EFFORT"] == "medium" + assert server_env["ZENITH_TERMINAL_REVIEWER_REASONING_EFFORT"] == "low" + + def test_codex_init_writes_reasoning_effort_env( + self, + runner: CliRunner, + workspace: Path, + env: dict[str, str], + monkeypatch: pytest.MonkeyPatch, + ) -> None: + monkeypatch.setenv("ZENITH_WORKER_REASONING_EFFORT", "high") + monkeypatch.setenv("ZENITH_VALIDATOR_REASONING_EFFORT", "medium") + monkeypatch.setenv("ZENITH_TERMINAL_REVIEWER_REASONING_EFFORT", "low") + + r = runner.invoke(cli, ["init", "--workspace-dir", str(workspace), "--agent", "codex"]) + assert r.exit_code == 0, r.output + + config = tomllib.loads( + (workspace / ".codex" / "config.toml").read_text(encoding="utf-8") + ) + server_env = config["mcp_servers"]["zenith"]["env"] + assert server_env["ZENITH_WORKER_REASONING_EFFORT"] == "high" + assert server_env["ZENITH_VALIDATOR_REASONING_EFFORT"] == "medium" + assert server_env["ZENITH_TERMINAL_REVIEWER_REASONING_EFFORT"] == "low" + + def test_init_reasoning_effort_flags_override_env( + self, + runner: CliRunner, + workspace: Path, + env: dict[str, str], + monkeypatch: pytest.MonkeyPatch, + ) -> None: + monkeypatch.setenv("ZENITH_WORKER_REASONING_EFFORT", "xhigh") + + r = runner.invoke( + cli, + [ + "init", + "--workspace-dir", + str(workspace), + "--agent", + "claude", + "--worker-reasoning-effort", + "max", + "--validator-reasoning-effort", + "medium", + ], + ) + assert r.exit_code == 0, r.output + + mcp = json.loads((workspace / ".mcp.json").read_text(encoding="utf-8")) + server_env = mcp["mcpServers"]["zenith"]["env"] + # Flag beats the inherited shell env. + assert server_env["ZENITH_WORKER_REASONING_EFFORT"] == "max" + assert server_env["ZENITH_VALIDATOR_REASONING_EFFORT"] == "medium" + assert "ZENITH_TERMINAL_REVIEWER_REASONING_EFFORT" not in server_env + + def test_init_invalid_inherited_effort_env_fails_despite_flag( + self, + runner: CliRunner, + workspace: Path, + env: dict[str, str], + monkeypatch: pytest.MonkeyPatch, + ) -> None: + # Flags override valid inherited settings; a broken env var is still a + # hard error — the same validation would raise at server launch, so + # masking it at init would only defer the failure. + monkeypatch.setenv("ZENITH_WORKER_REASONING_EFFORT", "turbo") + + r = runner.invoke( + cli, + [ + "init", + "--workspace-dir", + str(workspace), + "--agent", + "claude", + "--worker-reasoning-effort", + "max", + ], + ) + assert r.exit_code != 0 + assert isinstance(r.exception, ValueError) + assert "ZENITH_WORKER_REASONING_EFFORT" in str(r.exception) + def test_claude_init_writes_runtime_validator_env_names( self, runner: CliRunner, workspace: Path, env: dict[str, str] ) -> None: diff --git a/zenith/tests/test_config.py b/zenith/tests/test_config.py index 55d64b9..83c9c49 100644 --- a/zenith/tests/test_config.py +++ b/zenith/tests/test_config.py @@ -3,8 +3,21 @@ from pathlib import Path +import pytest + from zenith_harness.config import HarnessConfig +_EFFORT_ENV_VARS = ( + "ZENITH_WORKER_REASONING_EFFORT", + "ZENITH_VALIDATOR_REASONING_EFFORT", + "ZENITH_TERMINAL_REVIEWER_REASONING_EFFORT", +) + + +def _clear_effort_env(monkeypatch) -> None: + for var in _EFFORT_ENV_VARS: + monkeypatch.delenv(var, raising=False) + def test_discover_defaults_to_four_parallel_nodes( monkeypatch, @@ -44,3 +57,86 @@ def test_discover_invalid_parallelism_falls_back_to_default( config = HarnessConfig.discover() assert config.max_parallel_nodes == 4 + + +def test_discover_reasoning_effort_defaults_to_none( + monkeypatch, + harness_home: Path, +) -> None: + monkeypatch.setenv("ZENITH_HOME", str(harness_home)) + monkeypatch.delenv("ZENITH_PROJECT_BUCKET_DIR", raising=False) + _clear_effort_env(monkeypatch) + + config = HarnessConfig.discover() + + assert config.worker_reasoning_effort is None + assert config.validator_reasoning_effort is None + assert config.terminal_reviewer_reasoning_effort is None + + +def test_discover_reasoning_effort_per_role( + monkeypatch, + harness_home: Path, +) -> None: + monkeypatch.setenv("ZENITH_HOME", str(harness_home)) + monkeypatch.delenv("ZENITH_PROJECT_BUCKET_DIR", raising=False) + monkeypatch.setenv("ZENITH_WORKER_REASONING_EFFORT", "high") + monkeypatch.setenv("ZENITH_VALIDATOR_REASONING_EFFORT", "medium") + monkeypatch.setenv("ZENITH_TERMINAL_REVIEWER_REASONING_EFFORT", "max") + + config = HarnessConfig.discover() + + assert config.worker_reasoning_effort == "high" + assert config.validator_reasoning_effort == "medium" + assert config.terminal_reviewer_reasoning_effort == "max" + + +def test_discover_invalid_reasoning_effort_rejected( + monkeypatch, + harness_home: Path, +) -> None: + monkeypatch.setenv("ZENITH_HOME", str(harness_home)) + monkeypatch.delenv("ZENITH_PROJECT_BUCKET_DIR", raising=False) + _clear_effort_env(monkeypatch) + # Not silently ignored: the value lands in a shell command line, and a + # typo'd downgrade would silently keep spending xhigh. + monkeypatch.setenv("ZENITH_VALIDATOR_REASONING_EFFORT", "extra-high") + + with pytest.raises(ValueError, match="ZENITH_VALIDATOR_REASONING_EFFORT"): + HarnessConfig.discover() + + +def test_for_role_reasoning_effort_cascade( + monkeypatch, + harness_home: Path, +) -> None: + monkeypatch.setenv("ZENITH_HOME", str(harness_home)) + monkeypatch.delenv("ZENITH_PROJECT_BUCKET_DIR", raising=False) + _clear_effort_env(monkeypatch) + monkeypatch.setenv("ZENITH_WORKER_REASONING_EFFORT", "medium") + + config = HarnessConfig.discover() + + # Unset roles inherit down the same chain as providers/commands: + # terminal_reviewer -> validator -> worker. + assert config.for_role("worker").worker_reasoning_effort == "medium" + assert config.for_role("validator").worker_reasoning_effort == "medium" + assert config.for_role("terminal_reviewer").worker_reasoning_effort == "medium" + + +def test_for_role_reasoning_effort_explicit_override_wins( + monkeypatch, + harness_home: Path, +) -> None: + monkeypatch.setenv("ZENITH_HOME", str(harness_home)) + monkeypatch.delenv("ZENITH_PROJECT_BUCKET_DIR", raising=False) + _clear_effort_env(monkeypatch) + monkeypatch.setenv("ZENITH_WORKER_REASONING_EFFORT", "xhigh") + monkeypatch.setenv("ZENITH_VALIDATOR_REASONING_EFFORT", "low") + + config = HarnessConfig.discover() + + assert config.for_role("worker").worker_reasoning_effort == "xhigh" + assert config.for_role("validator").worker_reasoning_effort == "low" + # terminal_reviewer falls back to the validator setting first. + assert config.for_role("terminal_reviewer").worker_reasoning_effort == "low"