reviewer(ET): auto-commit from reviewer run_id=312
This commit is contained in:
@@ -2,41 +2,64 @@
|
||||
type: review
|
||||
work_item_id: ORCH-021
|
||||
verdict: APPROVED
|
||||
version: 1
|
||||
version: 2
|
||||
---
|
||||
|
||||
# Review ORCH-021 — Post-deploy мониторинг прода + реакция на деградацию
|
||||
|
||||
## Summary
|
||||
Реализация продлевает ответственность конвейера ЗА `deploy → done` через
|
||||
reserved-agent job `post-deploy-monitor` (вариант B из ADR-001, калька
|
||||
`deploy-finalizer`). Новый leaf-модуль `src/post_deploy.py` (never-raise), арм в
|
||||
terminal-блоке `advance_stage`, перехват тика в `launcher.launch_job` ДО `_spawn`,
|
||||
исполнение в `stage_engine.run_post_deploy_monitor`, блок `post_deploy` в `GET /queue`.
|
||||
Соответствует ТЗ, ADR и критериям приёмки. Все **700 тестов зелёные**
|
||||
(`pytest tests/ -q`), включая 30 новых (`test_post_deploy.py`,
|
||||
`test_post_deploy_integration.py`). Документация обновлена в том же PR. P0/P1 нет.
|
||||
Реализация продлевает ответственность конвейера ЗА терминальный переход
|
||||
`deploy → done`, закрывая класс инцидентов «зелёный деплой, красный прод» (ET-8).
|
||||
Механизм — детерминированный reserved-agent job `post-deploy-monitor` (вариант B
|
||||
из ADR-001, точная калька `deploy-finalizer`): арм в `stage_engine.advance_stage`
|
||||
(блок `next_stage == "done"`), один тик = один job (перехват в
|
||||
`launcher.launch_job` ДО `_spawn` → `stage_engine.run_post_deploy_monitor`),
|
||||
чистая логика в новом leaf-модуле `src/post_deploy.py` (never-raise).
|
||||
|
||||
## Соответствие ТЗ / ADR
|
||||
- §2.1 `src/post_deploy.py` — leaf-модуль, never-raise, `post_deploy_applies`,
|
||||
`probe_signals`, `classify`, `decide_action`, sentinel-state, артефакт, rollback-команда. ✔
|
||||
- §2.2 механизм — reserved-agent job, restart-safe (sentinel `armed`/`series`/`done`
|
||||
+ jobs-очередь), идемпотентный арм. ✔ (соответствует выбору архитектора в ADR §1)
|
||||
- §2.3 реакция — self-hosting ВСЕГДА `ALERT_ONLY` (структурно: `decide_action` →
|
||||
`run_rollback` недостижим для self), не-self+auto → `--rollback`, exit 1/2 → эскалация. ✔
|
||||
- §2.4 конфигурация — все `post_deploy_*` параметры с безопасными дефолтами; откат
|
||||
переиспользует `deploy_prod_*`. ✔
|
||||
- §2.5 артефакт `16-post-deploy-log.md` — валидный YAML-frontmatter, best-effort. ✔
|
||||
- §2.6 `GET /queue` блок `post_deploy`. ✔
|
||||
- §3 инварианты — `STAGE_TRANSITIONS`, `QG_CHECKS`, `check_deploy_status`, terminal-sync,
|
||||
merge-gate, exit-коды хука, схема БД — НЕ изменены (AC-12; подтверждено зелёным
|
||||
полным прогоном). ✔
|
||||
Проверены все четыре оси. Реализация соответствует ТЗ (`02-trz.md`), ADR-001 и
|
||||
глобальному adr-0010, удовлетворяет всем критериям приёмки AC-1…AC-18.
|
||||
Документация (golden-source) обновлена в том же PR. Регрессов нет.
|
||||
|
||||
## Критерии приёмки
|
||||
AC-1…AC-18 покрыты тестами TC01–TC20 (classify HEALTHY/DEGRADED по обоим порогам,
|
||||
устойчивость к одиночному глюку, kill-switch, условность репо, idempotent арм,
|
||||
self-hosting ALERT_ONLY, non-self rollback + эскалация, never-raise, артефакт,
|
||||
`/queue`). AC-18 (документация) — выполнен (см. ниже).
|
||||
## Соответствие ТЗ
|
||||
- §2.1 `src/post_deploy.py` (leaf, never-raise): `post_deploy_applies`,
|
||||
`probe_signals`, `classify`, `decide_action`, sentinel-state, артефакт,
|
||||
`build_rollback_command` — все на месте. ✅
|
||||
- §2.2 Оркестрация: арм в terminal-блоке + reserved-agent тик с
|
||||
само-перепостановкой через `available_at_delay_s`; restart-safe (sentinel
|
||||
`armed`/`series`/`done` + jobs-очередь). ✅
|
||||
- §2.3 Реакция: non-self+auto → хук `--rollback` (синхронно, целевой ≠ orch);
|
||||
self-hosting → ВСЕГДА `ALERT_ONLY`. ✅
|
||||
- §2.4 Конфигурация: все `post_deploy_*` в `src/config.py`, дефолты безопасны
|
||||
(kill-switch on, auto-rollback off), параметры отката переиспользуют
|
||||
`deploy_prod_*`. ✅
|
||||
- §2.5 Артефакт `16-post-deploy-log.md` с машиночитаемым frontmatter,
|
||||
best-effort. ✅
|
||||
- §2.6 Блок `post_deploy` в `GET /queue`. ✅
|
||||
- §2.7/§2.8/§3 Инварианты: `STAGE_TRANSITIONS`, `QG_CHECKS`,
|
||||
`check_deploy_status`, terminal-sync, merge-gate, exit-code-контракт хука,
|
||||
схема БД — не тронуты (подтверждено зелёным полным прогоном). ✅
|
||||
|
||||
## Соответствие ADR
|
||||
Реализация 1:1 повторяет ADR-001: механизм (reserved-agent, не стадия/не daemon),
|
||||
точки интеграции, пороги BR-3, политика реакции BR-5 (self never auto-rollback —
|
||||
структурный инвариант в `decide_action` + отсутствие вызова `run_rollback` на
|
||||
ALERT_ONLY). Нарушений глобальных ADR не выявлено.
|
||||
|
||||
## Качество кода
|
||||
- Контракт never-raise выдержан во всех публичных функциях и в каждой ветке
|
||||
`run_post_deploy_monitor`; launcher оборачивает тик в доп. guard (AC-16).
|
||||
- `classify` fail-safe → HEALTHY на мусорном входе (ложный DEGRADED опаснее).
|
||||
- Docstrings содержательные, со ссылками на AC/BR.
|
||||
- Условность раската по образцу ORCH-35/36/43/58 (флаг + CSV-репо).
|
||||
|
||||
## Тесты
|
||||
30 тестов ORCH-021 (`tests/test_post_deploy.py`,
|
||||
`tests/test_post_deploy_integration.py`) — содержательные, покрывают
|
||||
классификацию (AC-3..6), self-hosting safety (TC-19 явно проверяет, что хук
|
||||
`--rollback` НЕ вызывается для self — AC-8), idempotency двойного арма (AC-15),
|
||||
kill-switch/условность (AC-2/10/11), exit-code маппинг (AC-9), frontmatter
|
||||
артефакта (AC-13), never-raise (AC-16), `/queue` (AC-14). Полный прогон
|
||||
`pytest tests/` — **701 passed** (регрессов нет, AC-12).
|
||||
|
||||
## Findings
|
||||
|
||||
@@ -49,25 +72,28 @@ self-hosting ALERT_ONLY, non-self rollback + эскалация, never-raise, а
|
||||
### P2 — Should fix
|
||||
- нет
|
||||
|
||||
### P3 — Nice-to-have (не блокируют)
|
||||
- `build_rollback_command(repo)` принимает `repo`, но не использует его (симметрия с
|
||||
`build_deploy_command`); можно убрать или задокументировать.
|
||||
- `status()` в `/queue` формирует `active` по `os.path.basename(d)` (только work_item_id,
|
||||
без repo) — для разных репо с одинаковым wi возможна косметическая коллизия в выводе.
|
||||
- Теоретическое раздвоение цепочки тиков при дубле job (как у `deploy-finalizer`);
|
||||
на практике гасится `max_concurrency=1` + `done`-маркером. Принятый паттерн, не регресс.
|
||||
### P3 — Nice to have
|
||||
- [ ] `run_post_deploy_monitor`: в ветке `ALERT_ONLY` для **не-self** репо при
|
||||
`post_deploy_auto_rollback=false` текст алерта упоминает «авто-rollback для
|
||||
self-hosting запрещён (BR-5)», что для не-self случая формулировка не совсем
|
||||
точна (косметика сообщения; на поведение не влияет).
|
||||
- [ ] `write_post_deploy_log` коммитит/пушит артефакт в ветку задачи, которая к
|
||||
моменту наблюдения уже слита/может быть удалена — артефакт может не попасть в
|
||||
`main`. Контракт best-effort соблюдён (never-raise, ничего не роняет); как
|
||||
улучшение наблюдаемости — рассмотреть запись лог-артефакта отдельным путём.
|
||||
|
||||
## Документация
|
||||
Обновлена в том же PR (golden-source, AC-18 PASS):
|
||||
- `CLAUDE.md` — `16-post-deploy-log.md` добавлен в перечень артефактов.
|
||||
- `docs/architecture/README.md` — раздел «Post-deploy наблюдение…», блок `/queue`,
|
||||
заметка об обновлении.
|
||||
- `CHANGELOG.md` — запись в `[Unreleased] / Added`.
|
||||
- `.env.example` — переменные `ORCH_POST_DEPLOY_*`.
|
||||
- ADR work-item `docs/work-items/ORCH-021/06-adr/ADR-001-post-deploy-monitor.md`
|
||||
+ сквозной `docs/architecture/adr/adr-0010-post-deploy-monitor.md`.
|
||||
Обновлено в том же PR (golden-source, AC-18 — PASS):
|
||||
- `CLAUDE.md` — `16-post-deploy-log.md` добавлен в перечень артефактов;
|
||||
- `docs/architecture/README.md` — раздел «Post-deploy наблюдение прода» + блок
|
||||
`post_deploy` в таблице API `/queue`;
|
||||
- `docs/architecture/adr/adr-0010-post-deploy-monitor.md` — новый сквозной ADR;
|
||||
- `docs/work-items/ORCH-021/06-adr/ADR-001-post-deploy-monitor.md` — детальный ADR;
|
||||
- `CHANGELOG.md` — запись в `Added` (+ fix Dockerfile `COPY data/`);
|
||||
- `README.md` / `.env.example` — все `ORCH_POST_DEPLOY_*` env задокументированы.
|
||||
|
||||
## Заметка о diff
|
||||
`git diff main...HEAD` содержит файлы ORCH-060/ORCH-061/`staging_verdict.py`/`reconciler.py`
|
||||
из-за устаревшей merge-base ветки (эти изменения уже прошли свои PR в `main`).
|
||||
Предмет ревью — единственный коммит ORCH-021 `c5b516b` (12 файлов).
|
||||
Изменение `src/` сопровождено обновлением документации — правило CLAUDE.md №2/№6
|
||||
выполнено.
|
||||
|
||||
## Вердикт
|
||||
Только P3 (nice-to-have) findings, блокеров и must-fix нет → **APPROVED**.
|
||||
|
||||
Reference in New Issue
Block a user