From 56bf30323d31b70a437c7e933caed929368b69bf Mon Sep 17 00:00:00 2001 From: claude-bot Date: Mon, 8 Jun 2026 22:40:39 +0300 Subject: [PATCH] =?UTF-8?q?fix(effort):=20per-role=20floor=20for=20--effor?= =?UTF-8?q?t=20resolution=20+=20developer=E2=86=92xhigh?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolve_agent_effort returned '' for all agents in prod because empty ORCH_AGENT_EFFORT_*= env vars clobber pydantic class-defaults, leaving no non-empty floor to fall back to -> --effort never reached the Claude CLI. Add a level-4 per-role floor in resolve_agent_effort (src/agents/launcher.py): _agent_effort_floor reads the declared class-default of agent_effort_ (model_fields[...].default), which a present-but-empty env cannot override. Floor applies only when levels 1-3 are empty and BEFORE validation, so a typo (non-empty) still drops to '' (never-break ORCH-41) and explicit env/override still wins (priority preserved). config.py: agent_effort_developer high->xhigh (single source of truth; floor follows automatically). Refs: ORCH-081 Co-Authored-By: Claude Opus 4.7 --- .env.example | 9 ++- CHANGELOG.md | 1 + src/agents/launcher.py | 49 ++++++++++++- src/config.py | 10 ++- tests/test_resolve_agent_effort.py | 113 +++++++++++++++++++++++++---- 5 files changed, 156 insertions(+), 26 deletions(-) diff --git a/.env.example b/.env.example index 980a53f..9fb8d2e 100644 --- a/.env.example +++ b/.env.example @@ -37,12 +37,15 @@ ORCH_AGENT_MODEL_DEVELOPER= ORCH_AGENT_MODEL_REVIEWER= ORCH_AGENT_MODEL_TESTER= ORCH_AGENT_MODEL_DEPLOYER= -# Effort split: thinking agents (analyst/architect/developer/reviewer) -> high; -# mechanical agents (tester/deployer) -> medium. +# Effort split (ORCH-081/ORCH-52h): thinking agents (analyst/architect/reviewer) +# -> high; developer -> xhigh (coding/agentic role, Opus 4.8 canon); mechanical +# agents (tester/deployer) -> medium. NB: an empty ORCH_AGENT_EFFORT_*= no longer +# zeroes the effort — the launcher falls back to a per-role floor (= the config.py +# class-default) so each role still runs at its canonical level (ORCH-081). ORCH_AGENT_EFFORT_DEFAULT=high ORCH_AGENT_EFFORT_ANALYST=high ORCH_AGENT_EFFORT_ARCHITECT=high -ORCH_AGENT_EFFORT_DEVELOPER=high +ORCH_AGENT_EFFORT_DEVELOPER=xhigh ORCH_AGENT_EFFORT_REVIEWER=high ORCH_AGENT_EFFORT_TESTER=medium ORCH_AGENT_EFFORT_DEPLOYER=medium diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e65e6c..377131e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ Формат: [Keep a Changelog](https://keepachangelog.com/). Записи — на смысловой PR/задачу. ## [Unreleased] +- **Устойчивость резолва `--effort` к пустому env + developer → `xhigh`** (ORCH-081/ORCH-52h): фикс конфигурационного бага, из-за которого в проде `resolve_agent_effort()` возвращал `''` для всех 6 агентов и `--effort` не передавался в Claude CLI (каждый агент бежал на встроенном CLI-дефолте вместо заявленного уровня — прямой удар по предсказуемости качества всего конвейера, включая enduro-trails из общего инстанса). **Корень:** pydantic Settings трактует ПРИСУТСТВУЮЩУЮ env-переменную, даже пустую (`ORCH_AGENT_EFFORT_*=` без значения), как явное `''` и перебивает class-default; в проде пусты И per-agent, И `agent_effort_default`, поэтому у цепочки резолва (`_resolve_agent_attr`: project-override → per-agent env → default → `''`) не остаётся непустого «пола» для отката. **Фикс (вариант c, ADR-001):** в `resolve_agent_effort` (`src/agents/launcher.py`) добавлен уровень 4 — непустой **per-role floor** ниже `default`: новый чистый helper `_agent_effort_floor(agent)` возвращает декларированный class-default поля `agent_effort_` через `type(settings).model_fields[...].default` (значение, которое пустой env перебить НЕ может). Floor срабатывает ТОЛЬКО когда уровни 1–3 пусты и применяется ДО валидации, поэтому: (а) при пустом прод-`.env` каждая роль получает СВОЙ канонический уровень (developer=`xhigh`, tester/deployer=`medium`, analyst/architect/reviewer=`high`), а не общий default; (б) явная опечатка (`turbo`/`ultra`) непуста → floor НЕ применяется → значение штатно дропается валидацией `VALID_EFFORTS` в `''` (never-break ORCH-41 не регрессирует, floor не маскирует мусор); (в) непустой явный env/project-override/`default` по-прежнему ПОБЕЖДАЕТ floor (приоритет резолва сохранён 1:1). Unknown-agent (имя вне 6 ролей) деградирует на class-default `agent_effort_default` (`high`) — безопасный непустой пол. **`config.py`:** `agent_effort_developer` `high → xhigh` (канон Opus 4.8: coding/agentic роль) — единственное изменение значений; floor подтягивает его автоматически (единый источник правды, ноль риска дрейфа floor-карты). Инварианты НЕ менялись: приоритеты/сигнатуры резолва ORCH-41, `_resolve_agent_attr` (общий с model-резолвом, не тронут), `resolve_agent_model` (ORCH-074), путь проброса `--effort` в `_spawn`, `VALID_EFFORTS`, API, схема БД (без миграций). ADR `docs/work-items/ORCH-081/06-adr/ADR-001-effort-resolution-floor.md`. Документация: `docs/architecture/README.md` (таблица «модель/эффорт по ролям»: developer `xhigh` + ремарка про floor), `.env.example` (`ORCH_AGENT_EFFORT_DEVELOPER=xhigh` + комментарий split/floor). Тесты: `tests/test_resolve_agent_effort.py` (TC-01..08: канон-дефолты, floor при пустом env per-role, floor-не-маскирует-typo, приоритет, `xhigh∈VALID_EFFORTS`, сборка флага `--effort xhigh`/`--effort medium`). - **Убран мёртвый frontmatter `model:` + валидация имени модели (never-break)** (ORCH-074): закрыты два дефекта данных/валидации каркаса выбора модели агентов (ORCH-41), без изменения механизма резолва, API или схемы БД. **G1 — мёртвый frontmatter:** из YAML-frontmatter всех 6 промптов `.openclaw/agents/*.md` удалена строка `model:` (`claude-sonnet-4-6` у analyst/developer/tester/deployer, `claude-opus-4-7` у architect/reviewer). launcher НЕ читал frontmatter `model:` — это была лживая/мёртвая декларация, противоречащая реально используемой модели (config) и принципу «документация = golden source»; мина: если бы кто-то «починил» launcher читать frontmatter, все агенты молча уехали бы на устаревшие модели. config (`agent_model_*`/`agent_model_default`) остаётся единственным источником правды; frontmatter описательный. **G2 — валидация имени модели:** добавлен чистый helper `is_valid_model(name)` + `_MODEL_NAME_RE` (`^claude-[a-z0-9.-]+$`) рядом с `VALID_EFFORTS` в `src/agents/launcher.py`. Резолвенное имя модели валидируется ПЕРЕД попаданием в `--model`: невалидное (опечатка, `gpt-4`, пустое, неверный префикс) → `logger.warning` + откат на следующий валидный уровень каскада ORCH-41 (project-override → env → default), в пределе → `""` (без флага `--model`, CLI-дефолт). Никогда не возвращается мусор и не бросается исключение (never-break, поведенческая аналогия `resolve_agent_effort`/`VALID_EFFORTS`). Выбран **формат-чек, а не allowlist `VALID_MODELS`**: allowlist воссоздаёт ровно ту мину, что убивается в G1 (статичный список врёт при устаревании — молча дропнул бы корректную будущую `claude-opus-4-9`); формат-чек forward-compatible (новые `claude-*` проходят без правки кода), финальный авторитет о существовании модели — сам Claude CLI. Тот же предикат применён к inline-чтению `--fallback-model` (`agent_fallback_model` читается напрямую в `_spawn`, мимо `resolve_agent_model` — TRZ §4), поэтому опечатка в `ORCH_AGENT_FALLBACK_MODEL` тоже дропается с warning; для текущего пустого значения регрессии нет. **G4 (fallback) НЕ включён** (`agent_fallback_model=""`, AC-5 N/A) — ради детерминизма (все агенты на `claude-opus-4-8`); **G3 (routing) НЕ включён** (AC-4 N/A) — осознанное решение стейкхолдера (Слава 08.06). Реализация `resolve_agent_model` рефакторнута на генератор кандидатов `_agent_model_candidates` (тот же приоритет ORCH-41) + валидация-со-скипом. Инварианты НЕ менялись: приоритеты/сигнатуры резолва ORCH-41, структура CLI-команды `_spawn`, `VALID_EFFORTS`-гард эффорта, `STAGE_TRANSITIONS`, реестр `QG_CHECKS`, схема БД (без миграций); enduro per-project override валидные имена проходят без изменения поведения. ADR `docs/work-items/ORCH-074/06-adr/ADR-001-model-name-validation.md`. Документация: `docs/architecture/README.md` (таблица «модель/эффорт по ролям» + валидация), `CLAUDE.md`, `.env.example` (блок `ORCH_AGENT_MODEL_*`/`ORCH_AGENT_EFFORT_*`/`ORCH_AGENT_FALLBACK_MODEL`). Тесты: `tests/test_agent_frontmatter_no_model.py` (G1: TC-01/02), `tests/test_resolve_agent_model.py` (G2 never-break: TC-03..09, TC-11 + is_valid_model). - **Управление зависимостями задач (B ждёт A) + сериализация мержа одного репо** (ORCH-026): два уровня по ADR-001, оба условны (kill-switch + CSV-область, never-raise), без новой стадии и без изменения `STAGE_TRANSITIONS`/реестра `QG_CHECKS`. **Уровень A — сериализация merge/deploy внутри одного репо:** переиспользует существующий merge-lease ORCH-043/065 (никакого нового механизма); единственная новая логика — **безусловный pre-merge rebase**: в `check_branch_mergeable` (`src/qg/checks.py`) под удержанным лизом при флаге `premerge_rebase_always` (дефолт `True`) `auto_rebase_onto_main` вызывается **всегда** (а не только при `branch_is_behind_main`) — детерминированный структурный анти-фантом на ребре планировщика, дополняющий рубежи ORCH-073. На актуальной ветке это no-op (rebase не сдвигает HEAD, `push --force-with-lease` → «Everything up-to-date», CI не триггерится); kill-switch `premerge_rebase_always=False` → прежнее поведение ORCH-043 1:1. Окно сериализации «merge → main-updated» per-repo (для self `done` ⇔ SHA-in-main, ORCH-073): пока A не в `main`, B того же репо получает `merge-lock busy` → defer (не откат); кросс-репо параллелизм сохранён (лиз — per-repo файл). **Уровень B — декларативные зависимости задач:** аддитивная таблица `job_deps(task_id, depends_on_task_id)` (идемпотентный `CREATE TABLE/INDEX IF NOT EXISTS` в `init_db`, без миграции на живой БД); гейт планировщика в `claim_next_job` (`src/db.py`) — `NOT EXISTS (job_deps d JOIN tasks t … WHERE d.task_id=jobs.task_id AND t.stage!='done')` при `task_deps_enabled`: задача с незавершённой зависимостью **не выбирается** и слот `max_concurrency` не занимает; инертно при пустой `job_deps` → нулевая регрессия, kill-switch `task_deps_enabled=False` → запрос 1:1 как ORCH-1. Новый leaf-модуль `src/task_deps.py` (контракт never-raise): `is_task_ready` (fail-open → ready), DFS-детектор циклов (`detect_cycle`/`find_any_cycle`, итеративный WHITE/GREY/BLACK), `handle_cycle` (`set_issue_blocked` по каждой задаче цикла + один Telegram-alert с цепочкой «A → B → A»), `declare_dependency` (вставка + детект цикла), `ingest_plane_relations` (только для `task_deps_source=plane|hybrid`: резолв Plane `blocked-by` UUID → локальный task → запись в `job_deps`; источник истины горячего цикла остаётся БД, дефолт `db` НЕ ходит в сеть на claim), `snapshot` (read-only сводка). Видимость: строка «⏳ ждёт ORCH-NNN» в Telegram-карточке (`src/notifications.py`, never-raise, инвариант «одна карточка на задачу» сохранён); блок `task_deps` в `GET /queue` (`src/main.py`). Совместимость: `reconciler` F-1 пропускает dep-заблокированные задачи (`is_task_ready`, паттерн ORCH-060) + backstop-детект цикла; `job_reaper` сканирует только `running` → dep-блок остаётся `queued`. Зависимости — только intra-repo (v1). Новые настройки: `ORCH_PREMERGE_REBASE_ALWAYS` (true), `ORCH_TASK_DEPS_ENABLED` (true), `ORCH_TASK_DEPS_SOURCE` (db). Инварианты НЕ менялись: `STAGE_TRANSITIONS`, реестр `QG_CHECKS` (гейт зависимостей — врезка в `claim_next_job`, НЕ зарегистрированный QG), схема `tasks`/`jobs`/`agent_runs`, внешние HTTP-эндпоинты; non-self (enduro) — no-op при пустых `job_deps`/области. ADR `docs/work-items/ORCH-026/06-adr/ADR-001-merge-serialization-and-task-deps.md`, глобальный `docs/architecture/adr/adr-0015-task-deps-and-merge-serialization.md`. Документация: `docs/architecture/README.md`, `CLAUDE.md`, `.env.example`. Тесты: `tests/test_orch026_premerge_rebase.py`, `tests/test_orch026_merge_serialize.py`, `tests/test_orch026_conditionality.py`, `tests/test_orch026_task_deps.py`, `tests/test_orch026_dep_cycles.py`, `tests/test_orch026_dep_visibility.py`, `tests/test_orch026_migration.py`, `tests/test_orch026_queue_observability.py`, `tests/test_orch026_serialize_integration.py`, `tests/test_orch026_deps_integration.py`. - **CRIT: системный фикс эрозии `main` — SHA-в-main как единственный критерий merge-verify + регресс-гард + `.gitattributes`** (ORCH-073): устранён корень фантомного merge, из-за которого код задач ORCH-067 (`plane_issue_link`) и ORCH-069 (`qg0_title_max`) дошёл до `done`, но физически отсутствовал в `origin/main` (в `main` попадали только их авто docs-PR). **(FR-1)** `merge_gate.verify_merged_to_main` подтверждает merge **ТОЛЬКО** прямым фактом `git merge-base --is-ancestor origin/main` — OR-ветка `pr_already_merged` удалена (merged PR больше не подтверждает merge); пустой SHA / git-ошибка → `False` (fail-closed, never-raise). **(FR-2)** `pr_already_merged` понижен до idempotency-guard для `merge_pr` и засчитывает PR лишь при `merged & head.ref== & base.ref=="main"` (явный in-loop фильтр вместо ненадёжного query-параметра `head` — исключает авто docs-PR). **(FR-3)** `merge_pr` выбирает open code-PR строго по `head.ref==` И `base.ref=="main"`; merge только через Gitea PR-merge API, никогда push/force-push в `main`. **(FR-5)** новый детерминированный регресс-гард `merge_gate.check_main_regression` в `_handle_merge_verify` ПОСЛЕ подтверждённого SHA-в-main и ДО `done` проверяет, что `origin/main` содержит декларативный append-only набор маркеров ранее-merged задач (`MAIN_REGRESSION_MARKERS`, `git grep -c origin/main -- `); детерминированный `count==0` → alert «main regressed» + HOLD (`set_issue_blocked` + Telegram + Plane, задача НЕ `done`, БЕЗ авто-отката на `development`), git-ошибка самого грепа → fail-OPEN (не блокирует, SHA-в-main остаётся первичным гейтом). Kill-switch `ORCH_REGRESSION_GUARD_ENABLED` (дефолт `true`), область — `merge_verify_applies` (self-hosting / `merge_verify_repos`), non-self → no-op. **(FR-4)** корневой `.gitattributes` с `CHANGELOG.md merge=union` — правки `## [Unreleased]` авто-сливаются при `auto_rebase_onto_main` без конфликта (обе записи сохраняются), ветка не откатывается в `development` и не тащит устаревший код-сосед; `docs/**` под union НЕ ставится. `GET /queue::merge_verify_status` дополнен счётчиком `main_regressed_alerts_total` (read-only). Инварианты НЕ менялись: `STAGE_TRANSITIONS`, реестр `QG_CHECKS` (под-гейт — врезка в `advance_stage`), `check_deploy_status`/`_parse_deploy_status`, merge-gate, image-freshness, схема БД, внешние HTTP-эндпоинты; non-self (enduro) merge/verify/гард — no-op (INV-5); ручной `Confirm Deploy` сохранён. ADR `docs/work-items/ORCH-073/06-adr/ADR-001-merge-verify-sha-truth-and-regression-guard.md` (+ сквозной `adr-0014`). Документация: `docs/architecture/README.md`, `.env.example`. Тесты: `tests/test_orch073_*.py` (TC-01..18). diff --git a/src/agents/launcher.py b/src/agents/launcher.py index 9213b9d..6bc29c9 100644 --- a/src/agents/launcher.py +++ b/src/agents/launcher.py @@ -158,12 +158,50 @@ def resolve_agent_model(agent: str, project_id: str = None) -> str: return "" +def _agent_effort_floor(agent: str) -> str: + """ORCH-081 (ORCH-52h): per-role non-empty floor for --effort resolution. + + Returns the DECLARED class-default of the ``agent_effort_`` field on + Settings (e.g. developer -> ``xhigh``, tester/deployer -> ``medium``, the rest + -> ``high``). This is the value pydantic WOULD have used were it not clobbered + by a spurious empty env var (``ORCH_AGENT_EFFORT_=``): the class-default + is fixed in the class body and a present-but-empty env value cannot override it, + so it is a robust floor even when the host ``.env`` zeroes every effort var. + + config.py is the single source of truth: upgrading developer to ``xhigh`` there + automatically raises the floor here — no second map to keep in sync (ADR-001). + + Unknown agent (a name outside the 6 roles) has no ``agent_effort_`` + field; we degrade to the class-default of ``agent_effort_default`` (``high``), + a safe non-empty floor. Never raises. + """ + fields = type(settings).model_fields + for key in (f"agent_effort_{agent}", "agent_effort_default"): + field = fields.get(key) + if field is not None and field.default: + return field.default + return "" + + def resolve_agent_effort(agent: str, project_id: str = None) -> str: """ORCH-41: resolve the --effort level for an agent (optionally per-project). - Same priority as resolve_agent_model. The resolved value is validated against - VALID_EFFORTS; an invalid value is logged and dropped (returns "") so a typo - in env/projects_json can never pass a bad flag to the CLI. + Same priority as resolve_agent_model, with one extra level below the global + default (ORCH-081 / ADR-001): + 1. project-override (projects_json.agent_efforts[agent]) + 2. per-agent env (settings.agent_effort_) + 3. global default (settings.agent_effort_default) + 4. per-role FLOOR (class-default of agent_effort_) — NEW + + The floor only kicks in when levels 1-3 are all empty (the prod bug: a present + but empty ``ORCH_AGENT_EFFORT_*=`` clobbers every default to ''), guaranteeing + a non-empty target effort for the 6 known roles regardless of host .env state. + + The floor is applied BEFORE validation and ONLY to an empty resolve, so it + never masks a typo: an explicit invalid value (e.g. ``turbo``) is non-empty, + skips the floor, and is logged + dropped to "" exactly as in ORCH-41 (the + resolved value is validated against VALID_EFFORTS; an invalid value can never + pass a bad flag to the CLI). Never raises. """ value = _resolve_agent_attr( agent, project_id, @@ -171,6 +209,11 @@ def resolve_agent_effort(agent: str, project_id: str = None) -> str: env_attr_prefix="agent_effort_", default_attr="agent_effort_default", ) + if not value: + # Levels 1-3 all empty (typically a prod .env with empty ORCH_AGENT_EFFORT_*): + # fall through to the per-role floor (class-default). Applied before + # validation but only here, so a typo (non-empty) never reaches this branch. + value = _agent_effort_floor(agent) if value and value not in VALID_EFFORTS: logger.warning( f"Invalid effort '{value}' for agent '{agent}' " diff --git a/src/config.py b/src/config.py index dc93615..2fc010d 100644 --- a/src/config.py +++ b/src/config.py @@ -97,13 +97,15 @@ class Settings(BaseSettings): agent_model_deployer: str = "" # ORCH-41: per-agent effort / reasoning level: low|medium|high|xhigh|max. - # Empty -> agent_effort_default. Same resolution order as model. Default split: - # thinking agents (analyst/architect/developer/reviewer) -> high; mechanical - # agents (tester/deployer) -> medium. + # Empty -> agent_effort_default. Same resolution order as model. Default split + # (ORCH-081/ORCH-52h): thinking agents (analyst/architect/reviewer) -> high; + # developer -> xhigh (coding/agentic role, Opus 4.8 canon); mechanical agents + # (tester/deployer) -> medium. These class-defaults are ALSO the per-role floor + # used by resolve_agent_effort when the env is empty (single source of truth). agent_effort_default: str = "high" agent_effort_analyst: str = "high" agent_effort_architect: str = "high" - agent_effort_developer: str = "high" + agent_effort_developer: str = "xhigh" agent_effort_reviewer: str = "high" agent_effort_tester: str = "medium" agent_effort_deployer: str = "medium" diff --git a/tests/test_resolve_agent_effort.py b/tests/test_resolve_agent_effort.py index d2718d4..c48a7ba 100644 --- a/tests/test_resolve_agent_effort.py +++ b/tests/test_resolve_agent_effort.py @@ -26,13 +26,22 @@ from src.projects import ProjectConfig, reload_projects ORCH_PLANE_ID = "8da6aa25-a60e-44d6-a1e2-d8ae59aa7d6a" +# ORCH-081/ORCH-52h: canonical effort per role (developer upgraded high -> xhigh). +CANON_EFFORT = { + "analyst": "high", + "architect": "high", + "developer": "xhigh", + "reviewer": "high", + "tester": "medium", + "deployer": "medium", +} + + @pytest.fixture(autouse=True) def _clean_settings(monkeypatch): monkeypatch.setattr(settings, "agent_effort_default", "high") - for a in ("analyst", "architect", "developer", "reviewer"): - monkeypatch.setattr(settings, f"agent_effort_{a}", "high") - for a in ("tester", "deployer"): - monkeypatch.setattr(settings, f"agent_effort_{a}", "medium") + for a, e in CANON_EFFORT.items(): + monkeypatch.setattr(settings, f"agent_effort_{a}", e) monkeypatch.setattr(P.settings, "projects_json", "") reload_projects() yield @@ -50,19 +59,40 @@ def _install_registry(monkeypatch, agent_efforts): monkeypatch.setattr(P, "_BY_REPO", {p.repo: p for p in reg}) -# ---- default split ---------------------------------------------------------- +# ---- TC-01: canonical defaults (AC-1 / FR-4) -------------------------------- def test_default_split(): - assert resolve_agent_effort("developer") == "high" + assert resolve_agent_effort("developer") == "xhigh" assert resolve_agent_effort("architect") == "high" assert resolve_agent_effort("tester") == "medium" assert resolve_agent_effort("deployer") == "medium" -# ---- level 4: nothing -> "" ------------------------------------------------- -def test_no_config_returns_empty(monkeypatch): +@pytest.mark.parametrize("agent,expected", list(CANON_EFFORT.items())) +def test_canonical_effort_all_roles(agent, expected): + assert resolve_agent_effort(agent) == expected + + +# ---- TC-02: empty env -> per-role floor (variant c, AC-2) ------------------- +@pytest.mark.parametrize("agent,expected", list(CANON_EFFORT.items())) +def test_empty_env_falls_back_to_per_role_floor(monkeypatch, agent, expected): + """Models the prod bug: ORCH_AGENT_EFFORT_*= present-but-empty -> every level + resolves to '' on the instance; the per-role floor (config class-default) must + still yield the canonical level (NOT '').""" + monkeypatch.setattr(settings, "agent_effort_default", "") + for a in CANON_EFFORT: + monkeypatch.setattr(settings, f"agent_effort_{a}", "") + result = resolve_agent_effort(agent) + assert result == expected + assert result != "" + + +# ---- unknown agent floor degrades to default (high), never '' --------------- +def test_empty_env_unknown_agent_floor_is_default(monkeypatch): monkeypatch.setattr(settings, "agent_effort_default", "") monkeypatch.setattr(settings, "agent_effort_tester", "") - assert resolve_agent_effort("tester") == "" + # An agent with no agent_effort_ field falls back to the + # agent_effort_default class-default (high), a safe non-empty floor. + assert resolve_agent_effort("nonexistent_role") == "high" # ---- level 2: per-agent env beats default ----------------------------------- @@ -103,6 +133,45 @@ def test_all_valid_efforts_pass(monkeypatch): assert resolve_agent_effort("developer") == e +# ---- TC-03: floor does NOT mask a typo (FR-3 / AC-5) ------------------------ +def test_floor_does_not_mask_typo(monkeypatch): + """An explicit invalid value is non-empty, so the floor is NOT applied: the + value is validated and dropped to '' (never-break ORCH-41), even though the + developer floor (xhigh) exists.""" + monkeypatch.setattr(settings, "agent_effort_default", "") + monkeypatch.setattr(settings, "agent_effort_developer", "turbo") + assert resolve_agent_effort("developer") == "" + + +# ---- TC-04: priority preserved — explicit config beats floor (FR-2) --------- +def test_explicit_env_beats_floor(monkeypatch): + """Operator may deliberately downgrade developer to high; the explicit + non-empty env wins over the xhigh floor.""" + monkeypatch.setattr(settings, "agent_effort_developer", "high") + assert resolve_agent_effort("developer") == "high" + + +def test_default_beats_floor(monkeypatch): + """A non-empty global default wins over the per-role floor (floor is strictly + below default): default=max with empty per-agent -> max, not the xhigh floor.""" + monkeypatch.setattr(settings, "agent_effort_developer", "") + monkeypatch.setattr(settings, "agent_effort_default", "max") + assert resolve_agent_effort("developer") == "max" + + +def test_project_override_beats_floor(monkeypatch): + monkeypatch.setattr(settings, "agent_effort_developer", "") + _install_registry(monkeypatch, {"developer": "high"}) + assert resolve_agent_effort("developer", ORCH_PLANE_ID) == "high" + + +# ---- TC-05: xhigh is a valid effort (FR-5) ---------------------------------- +def test_xhigh_is_valid(): + assert "xhigh" in VALID_EFFORTS + # developer canonical xhigh resolves (is not dropped by validation) + assert resolve_agent_effort("developer") == "xhigh" + + # ---- flag assembly (mirror of launcher cmd construction) -------------------- def _build_flags(model, effort, fb): model_flag = f"--model {model} " if model else "" @@ -111,6 +180,7 @@ def _build_flags(model, effort, fb): return f"{model_flag}{effort_flag}{fb_flag}" +# ---- TC-06: flag assembly (AC-3) -------------------------------------------- def test_flags_present_when_configured(monkeypatch): monkeypatch.setattr(settings, "agent_fallback_model", "claude-sonnet-4-6") model = resolve_agent_model("developer") @@ -118,21 +188,32 @@ def test_flags_present_when_configured(monkeypatch): fb = settings.agent_fallback_model flags = _build_flags(model, effort, fb) assert "--model claude-opus-4-8 " in flags - assert "--effort high " in flags + assert "--effort xhigh " in flags assert "--fallback-model claude-sonnet-4-6 " in flags -def test_flags_absent_when_empty(monkeypatch): +def test_flags_effort_per_role(monkeypatch): + """developer -> --effort xhigh; tester -> --effort medium (mirrors _spawn).""" + assert "--effort xhigh " in _build_flags("", resolve_agent_effort("developer"), "") + assert "--effort medium " in _build_flags("", resolve_agent_effort("tester"), "") + + +def test_flags_absent_when_effort_empty(): + """When the resolved effort is empty, --effort is omitted entirely. Mirrors the + `f"--effort {effort} " if effort else ""` branch in _spawn (AC-3 negative case).""" + flags = _build_flags("", "", "") + assert flags == "" + assert "--effort" not in flags + + +def test_flags_absent_when_model_empty(monkeypatch): monkeypatch.setattr(settings, "agent_model_default", "") monkeypatch.setattr(settings, "agent_model_developer", "") - monkeypatch.setattr(settings, "agent_effort_default", "") - monkeypatch.setattr(settings, "agent_effort_developer", "") monkeypatch.setattr(settings, "agent_fallback_model", "") model = resolve_agent_model("developer") - effort = resolve_agent_effort("developer") fb = settings.agent_fallback_model - flags = _build_flags(model, effort, fb) + flags = _build_flags(model, "", fb) assert flags == "" assert "--model" not in flags - assert "--effort" not in flags + assert "--fallback-model" not in flags assert "--fallback-model" not in flags