diff --git a/docs/work-items/ORCH-113/12-review.md b/docs/work-items/ORCH-113/12-review.md new file mode 100644 index 0000000..275d840 --- /dev/null +++ b/docs/work-items/ORCH-113/12-review.md @@ -0,0 +1,86 @@ +--- +verdict: APPROVED +work_item: ORCH-113 +stage: review +author_agent: reviewer +status: approved +created_at: 2026-06-15 +model_used: claude-opus-4-8 +type: review +work_item_id: ORCH-113 +version: 1 +--- + +# Review ORCH-113 — BUG: job-reaper не должен повторно запускать финализацию `deploy-staging`, пока жив исходный finalizer + +## Summary + +Фикс инцидента ORCH-111 реализован чисто и полно. Введён процесс-локальный реестр владения +финализацией (`src/finalizer_liveness.py`, never-raise leaf по паттерну `serial_gate`/`coverage_gate`): +монитор `mark()`-ит владение сразу после штампа `exit_code` и `clear()`-ит его в `finally` хвоста +финализации; reaper в Tier-2 при `stage=="deploy-staging"` И активном владении делает **defer** вместо +повторного `advance_stage`, проваливаясь к Tier-3 backstop (который маркер игнорирует → застрявший +finalizer всё равно добивается). + +Проверено по всем 4 осям; блокирующих findings нет. + +- **ТЗ:** FR-1…FR-5 реализованы; AC-1…AC-6 покрыты тестами `tests/test_orch113_reaper_finalizer_liveness.py` + (TC-01…TC-08). Схема БД — **нулевое** изменение (выбран in-memory реестр), что строже допущенной ТЗ §5 + «аддитивная колонка». API §4 (read-only ключи в `GET /queue`) и QG §6 (не трогать) — соблюдены. +- **ADR:** реализация байт-в-байт соответствует ADR-001 / сквозному adr-0043 (D1–D5). Трассировка + сохранена: авторитет Tier-3 (adr-0011/ORCH-065) и сквозной бюджет `reaper_max_running_s (5400) > + Σ(gate-work)+grace` (ORCH-109/110) не нарушены — зафиксировано регресс-тестом TC-07. Ни один + маркированный инвариант не сломан. +- **Качество кода:** хвост `_monitor_agent` вынесен в `_run_monitor_finalization` **дословно** — + подтверждено `git diff -w` (+49/−0, нулевое изменение логики); все переменные, на которые ссылается + извлечённое тело, — параметры/локальные/модульные (нет риска `NameError`, проверено вручную). + never-raise во всех публичных функциях и врезках. Обязательный регресс-тест багфикс-трека (ORCH-019 + BR-4 / coverage ORCH-027) присутствует: TC-05 по построению КРАСНЫЙ до фикса (assert `calls == []`, + который pre-fix reaper нарушил бы вызовом `_try_advance_stage`) и ЗЕЛЁНЫЙ после. +- **Документация:** обновлены в том же PR — `docs/architecture/README.md` (описание Job-reaper + + раздел Tier-2 + список kill-switch + ссылки на ADR), `docs/architecture/internals.md` (детализация + Tier-2), `CHANGELOG.md`, ADR-001 (work-item) и сквозной adr-0043; все номерные доки задачи (00–04, + 06-adr, 07, 08, 10) на месте. + +**Проверка прогона:** `pytest tests/ -q` → **2001 passed**, 0 failures (AC-6); целевой файл — 13 passed. + +## Findings + +### P0 — Blocker +- _нет_ + +### P1 — Must fix +- _нет_ + +### P2 — Should fix +- _нет_ + +### P3 — Nice-to-have (не блокирует приёмку) +- [ ] Frontmatter обоих ADR (`ADR-001` и `adr-0043`) держит `status: proposed`. По мере мержа фикса + статус естественно становится принятым решением — стоит при следующем касании обновить на `accepted` + (косметика трассировки, не влияет на гейт). +- [ ] В врезке `mark()` (`launcher._monitor_agent`, стр. ~884) делается отдельный + `get_task_by_repo_branch(repo, branch)` ради `stage`-контекста, хотя тот же lookup повторяется ниже в + хвосте финализации (стр. ~984). Дублирование на пути, и так делающем БД-работу, обёрнуто never-raise; + `stage` здесь — best-effort контекст для `snapshot()` (reaper резолвит стадию независимо через + `_task_meta`), так что корректность не зависит от него. Можно при желании переиспользовать один lookup. + +## Документация + +**Статус: полностью обновлена в том же PR (golden source соблюдён).** + +| Артефакт | Изменение | Оценка | +|----------|-----------|--------| +| `docs/architecture/README.md` | Job-reaper компонент + раздел Tier-2 + список kill-switch (`ORCH_REAPER_FINALIZER_LIVENESS_ENABLED`) + ссылки на adr-0043 | ✅ | +| `docs/architecture/internals.md` | Детализация Tier-2 deploy-staging defer | ✅ | +| `CHANGELOG.md` | Развёрнутая запись `[Unreleased]` с подпунктами (leaf / эмиссия / консультация / наблюдаемость) | ✅ | +| `docs/work-items/ORCH-113/06-adr/ADR-001-…` | Детальный ADR (D1–D5, альтернативы, последствия) | ✅ | +| `docs/architecture/adr/adr-0043-…` | Сквозной ADR (уточняет adr-0011/0040/0042/0041) | ✅ | +| `docs/work-items/ORCH-113/{00..04,07,08,10}` | Полный пакет номерных доков | ✅ | + +**Обзорные доки / витрина:** правка внутренняя для job-reaper; высокоуровневые описания в +`docs/overview/tech-architecture.md` («job-reaper возвращает в очередь job'ы, чей исполнитель умер») и +`README.md` остаются корректными — обновления не требуют (ORCH-079/ORCH-011 не задеты). Раздел README +«Известные ограничения» не содержит пункта, закрываемого этим PR (баг был инцидентом, не значился +ограничением) — обновление не требуется. Известное ограничение `--workers>1` (TR-3) — системное +пред-допущение, документировано в `10-tech-risks.md` и обоих ADR; вынос в README не обязателен.