diff --git a/docs/work-items/ORCH-115/12-review.md b/docs/work-items/ORCH-115/12-review.md new file mode 100644 index 0000000..5e0e606 --- /dev/null +++ b/docs/work-items/ORCH-115/12-review.md @@ -0,0 +1,134 @@ +--- +verdict: APPROVED +work_item: ORCH-115 +stage: review +author_agent: reviewer +status: approved +created_at: 2026-06-16 +model_used: claude-opus-4-8 +type: review +work_item_id: ORCH-115 +version: 1 +--- + +# Review ORCH-115 — детерминированный staging-раннер вместо LLM-деплойера + +## Summary + +Замена LLM-агента `deployer` на стадии `deploy-staging` (self-hosting `orchestrator`) +детерминированным leaf `src/staging_runner.py`, перехватываемым в `launch_job` **до** `_spawn` +(прецедент `deploy-finalizer`/`post-deploy-monitor`). Реализация строго соблюдает корневой +инвариант NFR-1 — это замена **продюсера** артефакта `15-staging-log.md`, а **не** гейта: +`STAGE_TRANSITIONS`, `QG_CHECKS`, `check_staging_status`/`_parse_staging_status`, machine-key +`staging_status:` и схема БД — байт-в-байт не тронуты. Все FR-1…FR-7 реализованы, все +AC-1…AC-12 выполнены, документация обновлена в том же PR на всех требуемых поверхностях, +полный регресс зелёный (**2105 passed**), новый сьют (24 теста) зелёный, анти-дрейф LLM-карты +зелёный. + +**Вердикт: APPROVED** — P0/P1 findings отсутствуют. + +## Проверка по осям + +### 1. Соответствие ТЗ (`02-trz.md`) / критериям (`03-acceptance-criteria.md`) +- **FR-1 / AC-1** — перехват в `launch_job` до `_spawn` при `agent=="deployer"` + + `should_intercept` (стадия `deploy-staging` + `applies`); возвращает `None`, `_spawn` не + вызывается, `agent_runs` не создаётся, `jobs`-строка ведётся `mark_job` через + `_run_staging_runner_job` (зеркало `_run_deploy_finalizer_job`). ✅ (TC-05) +- **FR-2 / AC-9 / AC-8** — та же каноническая staging-сюита через `proc_group.run_in_process_group` + (tree-kill, таймаут). Команда из config (без host-хардкодов). ✅ (TC-11/TC-12) +- **FR-3 / AC-3** — exit-код → `staging_status:` через **единый** контракт + `self_deploy.map_exit_code_to_status` (`0→SUCCESS`, иначе/None/нечисло→`FAILED`), второй маппинг + не введён. ✅ (TC-02) +- **FR-4 / AC-2** — `15-staging-log.md` с `staging_status: SUCCESS|FAILED` (UPPERCASE) + полной + 52c-схемой (`work_item`/`stage: deploy-staging`/`author_agent: staging-runner`/`status`/ + `created_at`/`model_used: n/a`); INFRA-WAIVED копируется в тело; best-effort commit/push в + **фичеветку** (не в `main`), гейт читает worktree первым (`check_staging_status` lookup order + подтверждён в `src/qg/checks.py:627`). ✅ (TC-03/TC-04) +- **FR-5 / AC-4** — после вердикта вызывается существующий + `advance_stage(current_stage="deploy-staging", finished_agent="deployer")` — параметры в точности + соответствуют сигнатуре `stage_engine.advance_stage:191` и зеркалят `run_deploy_finalizer:2092`; + SUCCESS → под-гейты + Phase A; FAILED → существующий откат `deploy-staging → development` + (`stage_engine.py:932`, ветка `agent=="deployer" and qg=="check_staging_status"`). Новой ветви + маршрутизации нет. ✅ (TC-07/TC-10) +- **FR-6 / AC-6** — kill-switch `staging_runner_enabled` + скоуп `staging_runner_repos` (пусто → + self-hosting only через `is_self_hosting_repo`); `applies` проверяется первым, при выключенном + флаге `_spawn` LLM-deployer'а байт-в-байт. ✅ (TC-01/TC-08) +- **FR-7 / AC-10** — read-only блок `staging_runner` в `GET /queue` + один структурный лог-вердикт + на прогон, различающий `code-pass`/`code-fail`/`tool-error`. ✅ (TC-13) +- **AC-7 (never-raise)** — все публичные функции изолированы; tool-error (spawn-error/таймаут/ + `returncode None`) даёт bounded DEFER, затем fail-closed FAILED; никогда тихий advance/ложный + green. ✅ (TC-10) +- **AC-12** — `pytest tests/ -q` зелёный (2105 passed); новый `tests/test_orch115_staging_runner.py` + зелёный (24 теста, покрытие leaf 83%; непокрытые строки — исключительно defensive `except`-ветви + never-raise). ✅ + +### 2. Соответствие ADR (`06-adr/ADR-001` + сквозной `adr-0048`) +- D1–D11 реализованы как зафиксировано: точка диспетчеризации (перехват до `_spawn`, дискриминатор + по **стадии задачи**, не по имени роли — защита от перехвата прод-`deploy` deployer'а, TC-06), + чистота leaf'а (на импорте только `config`/`proc_group`; `db`/`git_worktree`/`stage_engine`/ + `qg.checks`/`self_deploy`/`notifications` — лениво), переиспользование `proc_group` и единого + маппинга, **двухуровневый исход D5 (анти-ORCH-110)** — инфра-сбой ≠ код-фейл, restart-safe + маркер-счётчик в `task_content` без правки схемы БД, push только в фичеветку, инициация + существующего гейта, бюджет времени без правки `reaper_max_running_s`, наблюдаемость. +- **Инвариант NFR-1 / AC-5 соблюдён байт-в-байт** — анти-дрейф TC-09 подтверждает неизменность + `STAGE_TRANSITIONS["deploy-staging"]`, наличие `check_staging_status` в `QG_CHECKS`, отсутствие + новых таблиц/колонок, неизменность machine-key `staging_status:`. +- **Граница O1 (трассировка маркеров)** — код ORCH-114 (`transition_lease`) и ORCH-112 + (`checkout_hygiene`) не модифицируется; lease берётся **внутри** `advance_stage`, раннер его не + трогает — зафиксированный инвариант цепочки ORCH-110/111/112/113/114 не сломан. + +### 3. Качество кода +- Docstrings на всех публичных функциях; следование установленным паттернам (`self_deploy`/ + `proc_group`/`merge_gate`); классификация `suite_ran = returncode is not None and not timed_out` + корректно трактует timeout-kill (returncode может быть `-9`, но `timed_out=True`) как tool-error, + а не код-фейл. +- Сверены все интеграционные сигнатуры: `ProcResult`, `run_in_process_group(grace_s/tree_kill)`, + `enqueue_job(available_at_delay_s)`, `mark_job`, `get_worktree_path`, `link_for`/`send_telegram`, + `is_self_hosting_repo`/`SELF_HOSTING_REPO` — все совпадают. +- **Self-hosting safety (AC-8/BR-7)** — в командной строке нет рестарта 8500 / `docker compose up` + / `--build` / force-push / правок `.env`; git push строго в фичеветку, никогда в `main`. ✅ (TC-12) +- Багфикс-трек (BR-4) неприменим: ORCH-115 — полный цикл (метка не `Bug`, прошёл `architecture`), + регресс-тест-фиксатор не требуется; покрытие обеспечено новым сьютом. + +### 4. Документация (обязательная проверка) +`src/` изменён → документация обновлена **в том же PR** на всех требуемых поверхностях: +- `CLAUDE.md` (раздел-паспорт ORCH-115), `CHANGELOG.md` (`[Unreleased]`). +- `docs/architecture/README.md` (компонент Staging-runner + adr-link), `internals.md` + (детерминированные перехваты `launch_job`). +- Норматив сопровождения ORCH-118: `llm-call-sites.md` (A6 — срез реализован, машинный + `ORCH-118-INVENTORY-BLOCK` сохраняет deployer `avoidable=yes`/`axis=C` как fallback), + `llm-determinization-roadmap.md` (rank 1 — ✅, инвариант «ровно один `first_slice`» цел), + `llm-usage-policy.md` (§5 — единственный транспорт S0 не нарушен). Анти-дрейф + `tests/test_llm_call_site_inventory.py` / `test_llm_determinization_docs.py` зелёные (TC-14). +- **Витрина системы `docs/overview/` (ORCH-011)** — обновлены `tech-pipeline.md`, `tech-agents.md`, + `tech-quality-security.md`; `tests/test_system_docs.py` зелёный. +- `.openclaw/agents/deployer.md` (LLM-ветвь `deploy-staging` — fallback), `.env.example` (5 ключей), + ADR work-item + сквозной `adr-0048`. +- **README «Известные ограничения» (ORCH-079)** — корректно не тронут: ни один из трёх открытых + пунктов (Telegram 48h / intra-repo deps / batch-autonomy) не закрывается ORCH-115; ложного + «решённое за открытое» нет. + +## Findings + +### P0 — Blocker +- Нет. + +### P1 — Must fix +- Нет. + +### P2 — Should fix +- Нет. + +### P3 — Nice-to-have +- [ ] `run_staging_gate` при отсутствии `work_item_id`/`branch` (повреждённая строка `tasks` — + задача на `deploy-staging` без branch) делает ранний `return`; обёртка `_run_staging_runner_job` + затем помечает джоб `done`, оставляя задачу «висеть» на `deploy-staging` до ре-драйва + reconciler/reaper. Это крайний кейс (любая задача на `deploy-staging` имеет branch), логируется + как ERROR и согласуется с never-raise-контрактом, но опционально можно сделать defensive-defer + вместо `done`-без-эффекта. Не блокирует приёмку. + +## Документация +Обновлена полностью и в том же PR. Изменение `src/` сопровождено синхронным обновлением паспорта, +чейнджлога, архитектурных доков, LLM-карты/roadmap/политики (+ зелёные анти-дрейф тесты), витрины +`docs/overview/` (ORCH-011), промпта `deployer.md`, `.env.example` и обоих ADR. Обзорная витрина +README не требует правок (нет закрытого ORCH-115 пункта). Документационная ось — **PASS**.