From 0873803faa477592e835f75008e39653b5a581be Mon Sep 17 00:00:00 2001 From: claude-bot Date: Mon, 8 Jun 2026 21:53:09 +0300 Subject: [PATCH] feat(launcher): drop dead frontmatter model + validate model name (never-break) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit G1: remove the dead `model:` line from all 6 .openclaw/agents/*.md prompts — launcher never read it; config (agent_model_*) is the single source of truth. G2: add is_valid_model helper (format check ^claude-…$) applied inside resolve_agent_model's resolution cascade and at the inline --fallback-model read in _spawn. An invalid name is logged and skipped to the next valid level (in the limit: no --model flag), never passed to the CLI, never raises. Format check chosen over an allowlist for forward-compatibility (ADR-001). G3 (routing) and G4 (fallback) intentionally NOT enabled — all agents stay on claude-opus-4-8; agent_fallback_model stays "". Docs (golden source) updated in the same change: README model/effort table + validation, CLAUDE.md, .env.example (ORCH_AGENT_MODEL_*/EFFORT_*/FALLBACK_MODEL), CHANGELOG. Tests: test_agent_frontmatter_no_model.py (G1), extended test_resolve_agent_model.py (G2 never-break). Refs: ORCH-074 Co-Authored-By: Claude Opus 4.7 --- .env.example | 38 +++++++++++ .openclaw/agents/analyst.md | 1 - .openclaw/agents/architect.md | 1 - .openclaw/agents/deployer.md | 1 - .openclaw/agents/developer.md | 1 - .openclaw/agents/reviewer.md | 1 - .openclaw/agents/tester.md | 1 - CHANGELOG.md | 1 + CLAUDE.md | 2 +- docs/architecture/README.md | 16 ++++- src/agents/launcher.py | 87 +++++++++++++++++++++--- tests/test_agent_frontmatter_no_model.py | 68 ++++++++++++++++++ tests/test_resolve_agent_model.py | 87 +++++++++++++++++++++++- 13 files changed, 288 insertions(+), 17 deletions(-) create mode 100644 tests/test_agent_frontmatter_no_model.py diff --git a/.env.example b/.env.example index 6c5b037..980a53f 100644 --- a/.env.example +++ b/.env.example @@ -12,6 +12,44 @@ ORCH_GITEA_WEBHOOK_SECRET= ORCH_CLAUDE_BIN=/usr/bin/claude ORCH_REPOS_DIR=/home/slin/repos ORCH_DB_PATH=/app/data/orchestrator.db + +# ── Agent model / effort / fallback (ORCH-41, validation ORCH-74) ───────────── +# Per-agent LLM model + reasoning effort, resolved by launcher.resolve_agent_*. +# Resolution priority (per agent): project-override (projects_json agent_models/ +# agent_efforts) > ORCH_AGENT_MODEL_ / ORCH_AGENT_EFFORT_ > +# ORCH_AGENT_MODEL_DEFAULT / ORCH_AGENT_EFFORT_DEFAULT > CLI default (no flag). +# The frontmatter `model:` in .openclaw/agents/*.md is DESCRIPTIVE only and is NOT +# read — config below is the single source of truth for the model (ORCH-74 G1). +# +# ORCH-74 (G2): a resolved MODEL name is validated (^claude-…$ format check) before +# it reaches --model. A structurally invalid name (typo, gpt-4, empty) is logged and +# the next valid level is used (in the limit: no --model flag). Forward-compatible: +# a future claude-* version passes without editing any allowlist. EFFORT is validated +# against low|medium|high|xhigh|max (ORCH-41); an invalid effort is dropped. +# +# All 6 agents resolve to claude-opus-4-8 (model-routing G3 NOT enabled). Leave the +# per-agent overrides empty to use the default. Do NOT hardcode the model version +# anywhere except ORCH_AGENT_MODEL_DEFAULT. +ORCH_AGENT_MODEL_DEFAULT=claude-opus-4-8 +ORCH_AGENT_MODEL_ANALYST= +ORCH_AGENT_MODEL_ARCHITECT= +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. +ORCH_AGENT_EFFORT_DEFAULT=high +ORCH_AGENT_EFFORT_ANALYST=high +ORCH_AGENT_EFFORT_ARCHITECT=high +ORCH_AGENT_EFFORT_DEVELOPER=high +ORCH_AGENT_EFFORT_REVIEWER=high +ORCH_AGENT_EFFORT_TESTER=medium +ORCH_AGENT_EFFORT_DEPLOYER=medium +# Optional --fallback-model used when the primary is overloaded. Empty -> no flag +# (G4 NOT enabled, ADR-001 ORCH-74: determinism — all agents stay on opus-4-8). A +# non-empty value is validated by the SAME predicate as the model; a typo is dropped. +ORCH_AGENT_FALLBACK_MODEL= # ORCH-042/ORCH-067: live-tracker mode. bump (DEFAULT since ORCH-067) -> on every # update the old card is deleted and a fresh one is sent silently to the BOTTOM of # the chat (deleteMessage + sendMessage + repoint), so the current status is always diff --git a/.openclaw/agents/analyst.md b/.openclaw/agents/analyst.md index da55498..c82e7e0 100644 --- a/.openclaw/agents/analyst.md +++ b/.openclaw/agents/analyst.md @@ -1,7 +1,6 @@ --- name: analyst description: Бизнес-аналитик. Создаёт пакет документов анализа для work item. -model: claude-sonnet-4-6 tools: - Filesystem (Read везде; Write только docs/work-items//*) - Bash (git log, grep — только для чтения контекста) diff --git a/.openclaw/agents/architect.md b/.openclaw/agents/architect.md index 8670d97..9c95dd6 100644 --- a/.openclaw/agents/architect.md +++ b/.openclaw/agents/architect.md @@ -1,7 +1,6 @@ --- name: architect description: Архитектор системы. Принимает архитектурные решения по ТЗ, фиксирует как ADR. -model: claude-opus-4-7 tools: - Filesystem (Read везде; Write только docs/) - Bash (read-only: grep, git log) diff --git a/.openclaw/agents/deployer.md b/.openclaw/agents/deployer.md index bb0b55b..214ad80 100644 --- a/.openclaw/agents/deployer.md +++ b/.openclaw/agents/deployer.md @@ -1,7 +1,6 @@ --- name: deployer description: DevOps-агент. Запускает staging-проверку и/или прод-деплой. Пишет 15-staging-log.md и 14-deploy-log.md. -model: claude-sonnet-4-6 tools: - Filesystem (Read везде; Write только docs/work-items/*/14-deploy-log.md, docs/work-items/*/15-staging-log.md) - Bash (docker, git, curl, ssh) diff --git a/.openclaw/agents/developer.md b/.openclaw/agents/developer.md index 8f29317..57117c9 100644 --- a/.openclaw/agents/developer.md +++ b/.openclaw/agents/developer.md @@ -1,7 +1,6 @@ --- name: developer description: Senior разработчик. Реализует ТЗ по ADR, пишет тесты, открывает PR. -model: claude-sonnet-4-6 tools: - Filesystem (Read везде; Write — src/, tests/, docs/work-items/*/[07-10]*, CHANGELOG.md) - Git (commit, push; merge запрещён) diff --git a/.openclaw/agents/reviewer.md b/.openclaw/agents/reviewer.md index 9bc24bf..c5d1dbb 100644 --- a/.openclaw/agents/reviewer.md +++ b/.openclaw/agents/reviewer.md @@ -1,7 +1,6 @@ --- name: reviewer description: Senior code reviewer. Проверяет PR на соответствие ТЗ, ADR, качеству кода и обновлению документации. -model: claude-opus-4-7 tools: - Filesystem (Read везде; Write только docs/work-items//12-review.md) - Git (read-only: log, diff, blame) diff --git a/.openclaw/agents/tester.md b/.openclaw/agents/tester.md index 647f4fd..2c5b011 100644 --- a/.openclaw/agents/tester.md +++ b/.openclaw/agents/tester.md @@ -1,7 +1,6 @@ --- name: tester description: QA-инженер. Прогоняет тесты, оформляет отчёт. -model: claude-sonnet-4-6 tools: - Filesystem (Read везде; Write только docs/work-items//13-test-report.md) - Bash (pytest, curl) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b1459c..9e65e6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ Формат: [Keep a Changelog](https://keepachangelog.com/). Записи — на смысловой PR/задачу. ## [Unreleased] +- **Убран мёртвый 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). - **Конфигурируемый верхний лимит длины заголовка QG-0 (`ORCH_QG0_TITLE_MAX`, дефолт 200)** (ORCH-069): хардкод `if len(name) > 80` во входной валидации `_qg0_errors` (`src/webhooks/plane.py`) вынесен в настраиваемый параметр `Settings.qg0_title_max` (env `ORCH_QG0_TITLE_MAX`, дефолт 200). Лимит 80 был гигиеническим, а не структурным (slug режется независимо `[:30]`, `tasks.title TEXT` без ограничения), поэтому валидные заголовки 81–200 символов отклонялись на входе без бизнес-причины. Лимит читается из `settings.qg0_title_max` динамически на каждый вызов (тесты патчат значение), текст ошибки подставляет актуальное число; граница строгая (`len > limit` → FAIL, `len == limit` → PASS). **Graceful-деградация (AC-3, self-hosting safety):** пустое/нечисловое значение env не роняет процесс на старте — `field_validator(mode="before")` `_qg0_title_max_default` в `src/config.py` перехватывает сырое env ДО `int`-парсинга pydantic и при невалидном/пустом входе возвращает дефолт 200 (never-raise), гася `ValidationError`. Чисто аддитивно и обратносовместимо: дефолт 200 > прежних 80 → все ранее проходившие заголовки проходят (AC-7). Инварианты НЕ менялись: `STAGE_TRANSITIONS`, реестр `QG_CHECKS` (QG-0 — inline-валидация входа, не зарегистрированный stage-gate), схема БД, slug-логика `[:30]`, нижние лимиты (`< 5` title, `< 20` description), soft-QG-0 поведение (warning на `work_item.created`), API. ADR `docs/work-items/ORCH-069/06-adr/ADR-001-configurable-qg0-title-limit.md`. Документация: `.env.example`, `.env.staging.example`. Тесты: `tests/test_qg0_title_limit.py`. diff --git a/CLAUDE.md b/CLAUDE.md index dcbdd29..2afcd8e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,7 +6,7 @@ ## Стек - Backend: FastAPI + uvicorn (Python 3.12) - БД: SQLite (`src/db.py`) -- Агенты: Claude CLI (`ORCH_CLAUDE_BIN`), по одному промпту на роль в `.openclaw/agents/` +- Агенты: Claude CLI (`ORCH_CLAUDE_BIN`), по одному промпту на роль в `.openclaw/agents/`. **ORCH-74:** модель/эффорт агента берутся ТОЛЬКО из config (`resolve_agent_model`/`resolve_agent_effort`, ORCH-41) — frontmatter `model:` удалён как мёртвый, frontmatter описательный; имя модели валидируется форматом `^claude-…$` перед `--model` (never-break). - Очередь задач: собственная (SQLite `jobs`, `src/queue_worker.py`, ORCH-1). **ORCH-026:** `claim_next_job` гейтит задачи с незавершёнными зависимостями (`job_deps`, `NOT EXISTS`) без занятия слота `max_concurrency`; декларации/детект циклов — leaf `src/task_deps.py` (kill-switch `ORCH_TASK_DEPS_ENABLED`). Сериализация мержа одного репо — безусловный pre-merge rebase под merge-lease (`ORCH_PREMERGE_REBASE_ALWAYS`). - Контейнеризация: Docker + Compose - CI/CD: Gitea Actions (`.gitea/workflows/`) diff --git a/docs/architecture/README.md b/docs/architecture/README.md index 4d5cf3a..0020af1 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -9,7 +9,7 @@ - **Stage Engine** (`src/stage_engine.py`) — исполнение переходов, диспетчеризация QG (`_run_qg`), откаты, синхронизация с Plane. - **Review/Test Parsers** (`src/review_parse.py`, ORCH-046) — defensive-извлечение дословного must-fix текста из артефактов для встраивания в `task_desc` заворота: `extract_review_findings` (P0/P1 из `12-review.md`), `extract_test_failures` (фрагмент тела `13-test-report.md`). Контракт «never raise»: любая ошибка → `""`. - **Quality Gates** (`src/qg/checks.py`) — проверки выхода со стадии, реестр `QG_CHECKS`. -- **Agent Launcher** (`src/agents/launcher.py`) — запуск Claude CLI агентов в изолированном git worktree, мониторинг, auto-advance. +- **Agent Launcher** (`src/agents/launcher.py`) — запуск Claude CLI агентов в изолированном git worktree, мониторинг, auto-advance. Модель/эффорт каждого агента резолвятся из config (`resolve_agent_model`/`resolve_agent_effort`, ORCH-41), а не из frontmatter промпта. **ORCH-74:** имя модели валидируется форматом `^claude-…$` (`is_valid_model`) перед `--model`; невалидное → лог + откат на следующий уровень/CLI-дефолт (never-break, как `VALID_EFFORTS` для эффорта). Тот же предикат гардит inline-чтение `--fallback-model`. - **Queue** (`src/queue_worker.py`, ORCH-1) — персистентная очередь задач (SQLite `jobs`), atomic claim, max_concurrency, ретраи, restart-safe. **ORCH-026:** `claim_next_job` гейтит задачи с незавершёнными зависимостями (`job_deps`, `NOT EXISTS`) без занятия слота; декларации/циклы — leaf `src/task_deps.py`. - **Job-reaper** (`src/job_reaper.py`, ORCH-065 — [adr-0011](adr/adr-0011-job-reaper-lease-reclaim.md)) — фоновый daemon-поток (каркас `reconciler`), стартует/останавливается в `main.lifespan` (после `reconciler.start()` / перед `worker.stop()`). Детектирует «мёртвый» `running`-job **без рестарта** процесса (Tier-1 мёртвый `jobs.pid` после `reaper_dead_ticks` тиков; Tier-2 `agent_runs.exit_code` записан, а job ещё `running`; Tier-3 backstop `reaper_max_running_s`) и приводит строку к корректному статусу через те же контракты (`_try_advance_stage`/`_finalize_job`, gate-driven; exit≠0/неизвестно → `attempts `ORCH_AGENT_MODEL_`/`ORCH_AGENT_EFFORT_` > `*_default` > CLI-дефолт (без флага)**. frontmatter `model:` в `.openclaw/agents/*.md` **удалён** (ORCH-74 G1) — он был мёртвой/лживой декларацией (launcher его не читает); config — единственный источник правды о модели. Model-routing (G3) НЕ включён — все 6 агентов на `claude-opus-4-8`. + +| Агент | Модель | Эффорт | +|-------|--------|--------| +| analyst | claude-opus-4-8 | high | +| architect | claude-opus-4-8 | high | +| developer | claude-opus-4-8 | high | +| reviewer | claude-opus-4-8 | high | +| tester | claude-opus-4-8 | medium | +| deployer | claude-opus-4-8 | medium | + +**Валидация (ORCH-74 G2, never-break):** резолвенное имя модели проходит формат-чек `is_valid_model` (`^claude-[a-z0-9.-]+$`) перед попаданием в `--model`. Невалидное (опечатка, `gpt-4`, пустое) → `logger.warning` + откат на следующий валидный уровень (в пределе — без `--model`, CLI-дефолт); мусор **никогда** не уезжает в CLI и запуск не падает. Форма — формат-чек, а не статичный allowlist: forward-compatible (будущие `claude-*` проходят без правки кода). Тот же предикат гардит inline-чтение `--fallback-model` (`agent_fallback_model` читается мимо резолва — TRZ §4). Эффорт валидируется множеством `VALID_EFFORTS` (`low|medium|high|xhigh|max`). Fallback (G4) НЕ включён (`agent_fallback_model=""`). Детали — `docs/work-items/ORCH-074/06-adr/ADR-001-model-name-validation.md`. + ### Условный staging-гейт (ORCH-35) `check_staging_status` реален только для self-hosting (`is_self_hosting_repo(repo)` → `orchestrator`); для остальных проектов → no-op `(True, "Staging gate N/A")`. Для orchestrator парсит `staging_status:` из `15-staging-log.md`; FAILED → откат на `development`. Подробнее: [ADR-0003](adr/adr-0003-staging-gate.md). diff --git a/src/agents/launcher.py b/src/agents/launcher.py index d83f6c0..9213b9d 100644 --- a/src/agents/launcher.py +++ b/src/agents/launcher.py @@ -2,6 +2,7 @@ import subprocess import os import json import logging +import re import threading import signal import time @@ -20,6 +21,36 @@ logger = logging.getLogger("orchestrator.launcher") # never passed through to the CLI. VALID_EFFORTS = frozenset({"low", "medium", "high", "xhigh", "max"}) +# ORCH-074 (G2): structural validity check for a Claude CLI model name. We use a +# FORMAT check (^claude-…$), not a static allowlist, on purpose: an allowlist +# recreates the exact rot we kill in G1 — it silently drops a CORRECT newer model +# (e.g. claude-opus-4-9) the day Anthropic ships it (never-break working against +# the operator). The final authority on whether a model exists is the Claude CLI +# itself, not our code; a format check is forward-compatible (new versions pass +# without code edits) while still catching the real failure classes: another +# provider (gpt-4), empty/whitespace, garbage chars, wrong prefix (claud-opus-typo). +# The claude- prefix is hardcoded here because the orchestrator is bound to the +# Claude CLI (CLAUDE_BIN); the canonical model VERSION lives ONLY in +# settings.agent_model_default, never here. See ADR-001 (ORCH-074). +_MODEL_NAME_RE = re.compile(r"^claude-[a-z0-9.-]+$") + + +def is_valid_model(name: str) -> bool: + """ORCH-074 (G2): True iff ``name`` is a structurally valid Claude model name. + + A valid name, after ``strip()``, is non-empty, starts with ``claude-`` and + contains only lowercase letters, digits, dots and dashes. Anything else + (empty/whitespace, another provider like ``gpt-4``, a wrong prefix, illegal + characters) is invalid. This is the single predicate used by BOTH + ``resolve_agent_model`` and the inline ``--fallback-model`` read in ``_spawn`` + so a typo can never reach the CLI (never-break). It is a structural guard, not + a registry of existing models — a structurally valid typo (``claude-opus-typo``) + is left for the CLI to reject. Never raises. + """ + if not name: + return False + return bool(_MODEL_NAME_RE.match(name.strip())) + # ORCH-061: action stages whose success is an ACTION (restart/retag), not a src # edit — so "no changes to commit" is EXPECTED there, not under-delivery (FR-3). _ACTION_STAGES = frozenset({"deploy-staging", "deploy"}) @@ -83,18 +114,48 @@ def _resolve_agent_attr(agent, project_id, project_map_attr, env_attr_prefix, return "" +def _agent_model_candidates(agent: str, project_id: str = None): + """Yield non-empty model candidates in ORCH-41 priority order. + + Same priority as _resolve_agent_attr (project-override > per-agent env > + global default), but as a generator so resolve_agent_model can validate each + level and SKIP an invalid one (ORCH-074 G2) instead of returning the first + non-empty value blindly. Empty levels are simply not yielded. + """ + if project_id: + from ..projects import get_project_by_plane_id + proj = get_project_by_plane_id(project_id) + if proj is not None: + override = getattr(proj, "agent_models", {}).get(agent) + if override: + yield override + per_agent = getattr(settings, f"agent_model_{agent}", "") + if per_agent: + yield per_agent + default = getattr(settings, "agent_model_default", "") + if default: + yield default + + def resolve_agent_model(agent: str, project_id: str = None) -> str: """ORCH-41: resolve the LLM model for an agent (optionally per-project). - Returns "" when no model is configured at any level -> caller omits --model - and the CLI default applies. See _resolve_agent_attr for the priority order. + ORCH-074 (G2): the resolved name is validated with is_valid_model BEFORE it is + returned. An invalid (structurally garbage) value at any level is logged and + SKIPPED — resolution falls through to the next valid level (project-override + invalid -> per-agent env -> default); if no level yields a valid name the + function returns "" so the caller omits --model and the CLI default applies. + The ORCH-41 priority order and signature are unchanged; validation is layered + on top. Never raises and never returns garbage that could reach --model. """ - return _resolve_agent_attr( - agent, project_id, - project_map_attr="agent_models", - env_attr_prefix="agent_model_", - default_attr="agent_model_default", - ) + for value in _agent_model_candidates(agent, project_id): + if is_valid_model(value): + return value + logger.warning( + f"Invalid model name '{value}' for agent '{agent}' " + f"(expected '^claude-…'); skipping to next resolution level / CLI default" + ) + return "" def resolve_agent_effort(agent: str, project_id: str = None) -> str: @@ -371,7 +432,17 @@ class AgentLauncher: effort = resolve_agent_effort(agent, project_id) model_flag = f"--model {model} " if model else "" effort_flag = f"--effort {effort} " if effort else "" + # ORCH-074 (G2): agent_fallback_model is read directly here, bypassing + # resolve_agent_model, so the same validator must guard this point too — + # otherwise a typo in ORCH_AGENT_FALLBACK_MODEL would slip into + # --fallback-model (never-break violation). Empty value -> no flag, exactly + # as before (is_valid_model("") is False but the `if fb` short-circuits). fb = settings.agent_fallback_model + if fb and not is_valid_model(fb): + logger.warning( + f"Invalid fallback model '{fb}'; dropping --fallback-model" + ) + fb = "" fb_flag = f"--fallback-model {fb} " if fb else "" # No git fetch/checkout here: ensure_worktree() already put the worktree on diff --git a/tests/test_agent_frontmatter_no_model.py b/tests/test_agent_frontmatter_no_model.py new file mode 100644 index 0000000..09139ef --- /dev/null +++ b/tests/test_agent_frontmatter_no_model.py @@ -0,0 +1,68 @@ +"""ORCH-074 (G1): the dead `model:` frontmatter is gone from all 6 agent prompts. + +launcher.py never reads frontmatter `model:` — it was a lying/dead declaration +(claude-sonnet-4-6 / claude-opus-4-7) that contradicted the real model resolved +from config (ORCH-41). The mine: if someone "fixed" the launcher to read it, every +agent would silently fall back to a stale model. G1 removes the line entirely so +config (agent_model_*) stays the single source of truth. + +TC-01: no .openclaw/agents/*.md contains a `^model:` line in its frontmatter. +TC-02: each frontmatter is still valid YAML and keeps name/description. +""" +import os + +import pytest + +try: + import yaml # PyYAML + _HAVE_YAML = True +except Exception: # pragma: no cover - yaml is a test/runtime dep + _HAVE_YAML = False + +_AGENTS = ("analyst", "architect", "developer", "reviewer", "tester", "deployer") + +# tests/ is one level under the repo root; .openclaw/agents lives at the root. +_AGENTS_DIR = os.path.join( + os.path.dirname(os.path.dirname(os.path.abspath(__file__))), + ".openclaw", "agents", +) + + +def _frontmatter_block(text: str) -> str: + """Return the YAML between the first two '---' fences (the frontmatter).""" + lines = text.splitlines() + assert lines and lines[0].strip() == "---", "frontmatter must open with '---'" + end = None + for i in range(1, len(lines)): + if lines[i].strip() == "---": + end = i + break + assert end is not None, "frontmatter must close with a second '---'" + return "\n".join(lines[1:end]) + + +@pytest.mark.parametrize("agent", _AGENTS) +def test_no_model_line_in_frontmatter(agent): + """TC-01: no agent prompt declares a `model:` key in its frontmatter.""" + path = os.path.join(_AGENTS_DIR, f"{agent}.md") + with open(path, encoding="utf-8") as f: + block = _frontmatter_block(f.read()) + for line in block.splitlines(): + assert not line.lstrip().startswith("model:"), ( + f"{agent}.md still declares a frontmatter 'model:' line: {line!r}" + ) + + +@pytest.mark.parametrize("agent", _AGENTS) +def test_frontmatter_still_valid_yaml_with_keys(agent): + """TC-02: frontmatter parses as YAML and keeps name/description (no model).""" + path = os.path.join(_AGENTS_DIR, f"{agent}.md") + with open(path, encoding="utf-8") as f: + block = _frontmatter_block(f.read()) + if not _HAVE_YAML: + pytest.skip("PyYAML not available") + data = yaml.safe_load(block) + assert isinstance(data, dict), f"{agent}.md frontmatter is not a YAML mapping" + assert data.get("name") == agent + assert data.get("description"), f"{agent}.md lost its description" + assert "model" not in data, f"{agent}.md frontmatter still has a model key" diff --git a/tests/test_resolve_agent_model.py b/tests/test_resolve_agent_model.py index 029d6f0..b3e170e 100644 --- a/tests/test_resolve_agent_model.py +++ b/tests/test_resolve_agent_model.py @@ -23,7 +23,9 @@ os.environ.setdefault("ORCH_DB_PATH", os.environ.setdefault("ORCH_GITEA_TOKEN", "test-token") os.environ.setdefault("ORCH_PLANE_API_TOKEN", "test-token") -from src.agents.launcher import resolve_agent_model +import logging + +from src.agents.launcher import resolve_agent_model, is_valid_model from src.config import settings from src import projects as P from src.projects import ProjectConfig, reload_projects, _parse_projects_json @@ -154,3 +156,86 @@ def test_parse_projects_json_malformed_override_ignored(): '"agent_models":"oops"}]') parsed = _parse_projects_json(raw) assert parsed is not None and parsed[0].agent_models == {} + + +# ============================================================================= +# ORCH-074 (G2): model-name validation, never-break. is_valid_model is a +# structural format check (^claude-…$), applied on top of the ORCH-41 cascade so +# garbage at any level is logged and skipped, never passed to --model. +# ============================================================================= + +# ---- is_valid_model predicate (the single G2 contract) ---------------------- +def test_is_valid_model_accepts_canonical(): + assert is_valid_model("claude-opus-4-8") is True + assert is_valid_model("claude-sonnet-4-6") is True + # forward-compatible: a future version passes without a code change + assert is_valid_model("claude-opus-4-9") is True + # surrounding whitespace is tolerated (stripped) + assert is_valid_model(" claude-opus-4-8 ") is True + + +def test_is_valid_model_rejects_garbage(): + assert is_valid_model("") is False + assert is_valid_model(" ") is False + assert is_valid_model(None) is False + assert is_valid_model("gpt-4") is False # another provider + assert is_valid_model("claud-opus-typo") is False # wrong prefix + assert is_valid_model("Claude-Opus-4-8") is False # uppercase not allowed + assert is_valid_model("claude-opus 4 8") is False # spaces inside + + +# ---- TC-03: garbage in agent_model_ -> fall back to default ---------- +def test_garbage_per_agent_env_falls_back_to_default(monkeypatch, caplog): + monkeypatch.setattr(settings, "agent_model_developer", "gpt-4") + with caplog.at_level(logging.WARNING): + result = resolve_agent_model("developer") + assert result == "claude-opus-4-8" # dropped garbage, used default + assert any("Invalid model name" in r.message for r in caplog.records) + + +# ---- TC-04: garbage in project-override -> fall back to next valid level ----- +def test_garbage_project_override_falls_back_to_default(monkeypatch, caplog): + _install_registry(monkeypatch, {"developer": "claud-opus-typo"}) + with caplog.at_level(logging.WARNING): + result = resolve_agent_model("developer", ORCH_PLANE_ID) + assert result == "claude-opus-4-8" # override dropped, default used + assert any("Invalid model name" in r.message for r in caplog.records) + + +# ---- TC-05: both override and default invalid -> "" (no --model), no raise --- +def test_all_levels_invalid_returns_empty(monkeypatch, caplog): + monkeypatch.setattr(settings, "agent_model_default", "totally-bogus") + _install_registry(monkeypatch, {"developer": "gpt-4"}) + with caplog.at_level(logging.WARNING): + result = resolve_agent_model("developer", ORCH_PLANE_ID) + assert result == "" # never returns garbage; CLI default applies + # both invalid levels were logged + assert sum("Invalid model name" in r.message for r in caplog.records) >= 2 + + +# ---- TC-06: valid canonical name passes unchanged (ORCH-41 regression) ------- +def test_valid_canonical_unchanged(): + assert resolve_agent_model("developer") == "claude-opus-4-8" + + +# ---- TC-07: all 6 agents resolve to claude-opus-4-8 (routing G3 off) --------- +def test_all_six_agents_resolve_to_opus_4_8(): + for agent in ("analyst", "architect", "developer", "reviewer", "tester", + "deployer"): + assert resolve_agent_model(agent) == "claude-opus-4-8" + + +# ---- TC-08: valid per-project override still passes validation (AC-8) -------- +def test_valid_per_project_override_unchanged(monkeypatch): + _install_registry(monkeypatch, {"reviewer": "claude-sonnet-4-6"}) + assert resolve_agent_model("reviewer", ORCH_PLANE_ID) == "claude-sonnet-4-6" + + +# ---- TC-09 / TC-11: G4 fallback is OFF (ADR-001 decision 3) ------------------ +def test_fallback_model_disabled_by_default(): + # G4 not enabled: agent_fallback_model stays "" -> no --fallback-model flag. + assert settings.agent_fallback_model == "" + # never-break: the SAME predicate guards the inline fallback read in _spawn, + # so a typo there would be rejected exactly like a model name. + assert is_valid_model("claude-bad typo") is False + assert is_valid_model("") is False