reviewer(ET): auto-commit from reviewer run_id=195
This commit is contained in:
86
docs/work-items/ORCH-036/12-review.md
Normal file
86
docs/work-items/ORCH-036/12-review.md
Normal file
@@ -0,0 +1,86 @@
|
||||
---
|
||||
type: review
|
||||
work_item_id: ORCH-036
|
||||
verdict: REQUEST_CHANGES
|
||||
version: 1
|
||||
---
|
||||
|
||||
# Review ORCH-036 — Исполняемый самодеплой стадии `deploy` (Вариант B)
|
||||
|
||||
## Summary
|
||||
|
||||
Реализация трёхфазного исполняемого самодеплоя (`src/self_deploy.py` + перехваты
|
||||
Фаз A/B/C в `src/stage_engine.py`, reserved-agent `deploy-finalizer` в
|
||||
`src/agents/launcher.py`, build-once `SOURCE_IMAGE` в хуке) — архитектурно
|
||||
соответствует ADR-001 и закрывает большинство критериев приёмки. Код аккуратный,
|
||||
defensive (never-raise), 566/566 тестов зелёные, контракты `STAGE_TRANSITIONS` /
|
||||
`_parse_deploy_status` / БАГ-8 / merge-gate не тронуты, условность по репо
|
||||
(`self_deploy_applies`) корректна, документация (deployer.md, INFRA.md,
|
||||
DEPLOY_HOOK.md, CHANGELOG, ADR-001 + глобальный adr-0007, README) обновлена
|
||||
содержательно.
|
||||
|
||||
Однако обнаружены **два P1**, блокирующие приёмку: (1) sentinel-маркеры не
|
||||
очищаются на откате → повторный прод-деплой после фикса невозможен (конвейер
|
||||
зависает на `deploy`); (2) не обновлён канонический `.env.example` вопреки прямому
|
||||
требованию ТЗ §2.6 и правилу CLAUDE.md №8.
|
||||
|
||||
## Findings
|
||||
|
||||
### P0 — Blocker
|
||||
- (нет)
|
||||
|
||||
### P1 — Must fix
|
||||
|
||||
- [ ] **Stale deploy-state маркеры ломают re-deploy после отката (`src/stage_engine.py`,
|
||||
`src/self_deploy.py`).** Маркеры `initiated` / `result` / `approve-requested`
|
||||
лежат под `<repos_dir>/.deploy-state-<repo>/<work_item_id>/` и ключуются по
|
||||
**work_item_id**, который стабилен на всю жизнь задачи. БАГ-8 откат
|
||||
(`_handle_qg_failure_rollbacks`, ветка `deployer`+`check_deploy_status`,
|
||||
строки 620–647) НЕ удаляет эти маркеры, и нигде в `src/` нет их очистки
|
||||
(проверено grep'ом). Сценарий: deploy FAILED → откат на development → developer
|
||||
чинит → задача снова доходит до `deploy` → человек ставит Approved → Фаза B
|
||||
(`_handle_self_deploy_phase_b`) видит существующий `initiated` (от первой
|
||||
попытки) → возвращает `self-deploy-already-initiated` **no-op**, detached-хук НЕ
|
||||
запускается, finalizer НЕ ставится. Задача навсегда зависает на `deploy`,
|
||||
ожидая approve, который ничего не делает. Дополнительно: устаревший `result`
|
||||
(FAILED exit от первой попытки) тоже остаётся и был бы прочитан новым
|
||||
finalizer'ом. Это ломает retry-контракт самой стадии для self-hosting-репо —
|
||||
ровно тот класс, под который сделана фича (AC-4 работает только на ПЕРВЫЙ
|
||||
провал; AC-10 «БАГ-8 rollback+retry не сломан» нарушается на втором проходе).
|
||||
Нужно очищать маркеры (`initiated`, `result`, `approve-requested`) на откате
|
||||
`deploy → development` (и/или в начале Фазы B/Фазы A для нового прохода), плюс
|
||||
тест на сценарий FAILED → rollback → повторный deploy.
|
||||
|
||||
- [ ] **`.env.example` не обновлён (`/.env.example`).** ТЗ §2.6 прямо требует:
|
||||
«Прописать дескрипторы в `.env.example`», CLAUDE.md правило №8 объявляет
|
||||
`.env.example` каноном env-конфигурации, а предыдущая задача ORCH-043
|
||||
(merge-gate) задала прецедент — добавила свои `ORCH_MERGE_*` именно туда с
|
||||
описаниями. ORCH-036 ввёл ~14 новых настроек в `src/config.py`
|
||||
(`self_deploy_enabled`, `self_deploy_repos`, `deploy_require_manual_approve`,
|
||||
`deploy_ssh_user/host`, `deploy_hook_script`, `deploy_host_repo_path`,
|
||||
`deploy_prod_*`, `deploy_finalize_delay_s/max_attempts`) и задокументировал их в
|
||||
таблице INFRA.md, но в `.env.example` они отсутствуют полностью. Оператор,
|
||||
поднимающий сервис по канону `.env.example`, не узнает про обязательный
|
||||
`ORCH_DEPLOY_SSH_HOST` (дефолт пустой → detached-деплой не запустится). Добавить
|
||||
блок `ORCH_SELF_DEPLOY_*` / `ORCH_DEPLOY_*` с дескрипторами по образцу
|
||||
merge-gate (значения — плейсхолдеры, секреты не коммитить).
|
||||
|
||||
### P2 — Should fix
|
||||
- [ ] (опц.) INFRA.md обновлён в части env-карты, но пошаговой процедуры
|
||||
«approve → прод-деплой через хук» в INFRA нет (есть в deployer.md и
|
||||
DEPLOY_HOOK.md). Желательно дать короткую сквозную процедуру оператору в одном
|
||||
месте. Не блокирует.
|
||||
|
||||
## Документация
|
||||
|
||||
Обновлена содержательно и в том же PR: `.openclaw/agents/deployer.md` (стадия
|
||||
`deploy` = вызов хука, явный запрет self-restart 8500), `docs/operations/INFRA.md`
|
||||
(env-карта новых настроек), `docs/operations/DEPLOY_HOOK.md` (`SOURCE_IMAGE`
|
||||
build-once + прод-пример), `docs/architecture/README.md` (раздел про исполняемый
|
||||
самодеплой), `CHANGELOG.md` (детальная запись), ADR-001 в `06-adr/` + глобальный
|
||||
`docs/architecture/adr/adr-0007-executable-self-deploy.md`.
|
||||
|
||||
**Пробел (см. P1):** `.env.example` — канонический шаблон env по CLAUDE.md №8 и
|
||||
прямое требование ТЗ §2.6 — НЕ обновлён новыми `ORCH_DEPLOY_*` / `ORCH_SELF_DEPLOY_*`.
|
||||
Это и есть основание держать вердикт `REQUEST_CHANGES` по оси документации, в
|
||||
дополнение к функциональному P1 по очистке маркеров.
|
||||
Reference in New Issue
Block a user