reviewer(ET): auto-commit from reviewer run_id=323

docs/work-items/ORCH-065/12-review.md

@@ -1,8 +1,8 @@
 ---
 type: review
 work_item_id: ORCH-065
-verdict: REQUEST_CHANGES
-version: 2
+verdict: APPROVED
+version: 3
 ---
 
 # Review ORCH-065
@@ -11,24 +11,38 @@ version: 2
 
 Задача закрывает три связанных класса отказов «процесс/поток умер, а ресурс остался
 захваченным навсегда»: zombie jobs (A), залипший merge-lease (B), неидемпотентная
-финализация merge (C). Реализация в целом качественная: новый daemon-поток
-`src/job_reaper.py` по образцу `reconciler` (never-raise, kill-switch, снимок в
-`/queue`), трёхуровневая liveness, атомарный `reap_running_job(... WHERE status='running')`,
-проактивный реклейм lease (`pid_alive` + `reclaim_stale_lease`), идемпотентный guard
-`pr_already_merged`, колонка `jobs.pid` через идемпотентный `_ensure_column`. Инварианты
-сохранены: `STAGE_TRANSITIONS`/`QG_CHECKS`/`check_*`/БАГ-8/exit-коды хука не тронуты
-(коммиты ORCH-065 не касаются `stages.py`/`qg/checks.py`/`stage_engine.py`); реклейм
-lease — только удаление файла, без git-операций. Документация (README, internals, ADR,
-глобальный adr-0011, CHANGELOG, .env.example) обновлена в этом же PR. `pytest tests/ -q`
-— **743 passed**.
+финализация merge (C). Реализация качественная: новый daemon-поток `src/job_reaper.py`
+по образцу `reconciler` (never-raise, kill-switch, снимок в `/queue`), трёхуровневая
+liveness, атомарный `reap_running_job(... WHERE status='running')`, проактивный реклейм
+lease (`pid_alive` + `reclaim_stale_lease`), идемпотентный guard `pr_already_merged`,
+колонка `jobs.pid` через идемпотентный `_ensure_column`.
 
-Прошлый блокер v1 (guard `pr_already_merged` не подключён к пути merge) **устранён**
-коммитом `aa46e5d`: промпт `.openclaw/agents/deployer.md` теперь предписывает консультировать
-`pr_already_merged` ПЕРЕД любым (повторным) merge — AC-11 wiring на месте.
+**Все блокеры предыдущих ревью устранены:**
+- v1 P0 (guard `pr_already_merged` не подключён к merge-пути) — устранён `aa46e5d`:
+  промпт `.openclaw/agents/deployer.md` консультирует `pr_already_merged` ПЕРЕД любым
+  (повторным) merge (AC-11 wiring на месте, подтверждено строками 94–105/152).
+- v2 P1 (Tier-2 реапит живой финализирующий monitor; side-effects ДО атомарного claim,
+  нарушение ADR-001 Р-1) — устранён `3e2eb27` двумя мерами:
+  1. **Tier-2 finalization grace** — новая колонка `finished_age_s` в `get_running_jobs`
+     (`src/db.py:609`) + настройка `reaper_finalize_grace_s` (дефолт 300с); Tier-2
+     реапит только при `finished_age >= grace`, иначе строка не трогается
+     (`src/job_reaper.py:197-209`). Живой финализирующий monitor больше не реапится
+     (FR-1.3/AC-3).
+  2. **claim-before-act** — `_reap_exit0` (`src/job_reaper.py:242-286`) сначала оценивает
+     канонический QG read-only (`_gate_is_green` → `_run_qg`, без побочных эффектов),
+     затем атомарно claim `done` ПЕРВЫМ, и только победитель claim выполняет
+     `_gate_driven_advance`. Проигравший гонку (поздний monitor / стартовый requeue) не
+     делает НИКАКИХ побочных эффектов → нет дубль-advance/дубль-enqueue (FR-1.2/AC-4).
+- v2 P3 (битая ссылка на adr-0011 в CHANGELOG) — исправлена в `3e2eb27`
+  (`adr-0011-job-reaper-lease-reclaim.md`).
 
-Новый блокер: гонка Tier-2 reaper'а с живым monitor-потоком при штатной финализации —
-порядок «side effects → atomic claim» нарушает собственный контракт ADR-001 Р-1 и может
-дать дубль-advance / дубль-enqueue следующей стадии (FR-1.2/FR-1.3/AC-3/AC-4).
+Инварианты сохранены (AC-13): ORCH-065-коммиты (`1a2e881`/`aa46e5d`/`3e2eb27`) НЕ касаются
+`src/stages.py` и `src/qg/checks.py` — `STAGE_TRANSITIONS`/`QG_CHECKS`/`check_*`/БАГ-8/
+exit-коды хука не тронуты; реклейм lease — только удаление файла, без git-операций
+(AC-12). Документация (README, internals, ADR-001, глобальный adr-0011, CHANGELOG,
+.env.example) обновлена в этом же PR (AC-17). Новые тесты покрывают grace-окно,
+lost-claim-no-side-effects, already-advanced-идемпотентность. `pytest tests/ -q` —
+**747 passed**.
 
 ## Findings
 
@@ -36,70 +50,21 @@ lease — только удаление файла, без git-операций.
 - нет
 
 ### P1 — Must fix
-- [ ] **Tier-2 реапит живой, штатно финализирующийся job; side-effects идут ДО
-  атомарного claim (нарушение ADR-001 Р-1, риск дубль-advance/дубль-enqueue).**
-  В `JobReaper._reap_job` (`src/job_reaper.py:177`) ветка Tier-2 срабатывает на ЛЮБОЙ
-  `running`-job, у которого в `agent_runs` уже записан `exit_code` — **без grace и без
-  какого-либо признака смерти monitor'а**. Но именно это состояние («exit_code записан,
-  job ещё running») — нормальное окно финализации живого monitor-потока: `_monitor_agent`
-  пишет `exit_code`, затем выполняет git commit/push (+PR), БАГ-8-проверку, **сетевые
-  usage-комментарии в Plane** (секунды-десятки секунд), и лишь потом `_try_advance_stage`
-  → `_finalize_job`. pid агента (`jobs.pid`) к этому моменту уже мёртв (процесс завершён
-  до записи exit_code), поэтому отличить «monitor умер» от «monitor жив и финализирует»
-  по pid невозможно, а reaper тикает каждые `reaper_interval_s` (60с). Доступный для
-  grace `finished_at_run` запрашивается в `get_running_jobs`, но **не используется**
-  (помечен «debug only», `src/db.py`).
-
-  Усугубляет дефект порядок в `_reap_known_outcome` (`src/job_reaper.py:206`): для exit0
-  сначала вызывается `_gate_driven_advance(job)` (побочные эффекты — `_try_advance_stage`
-  → `advance_stage` → `enqueue_job` следующей стадии), и **только потом** атомарный
-  `reap_running_job(..., "done")`. Это прямо противоречит ADR-001 Р-1: «Атомарный
-  reap-claim. **Перед любым действием с побочными эффектами** reaper атомарно
-  «застолбляет» строку тем же приёмом, что `claim_next_job`». Поскольку claim стоит
-  ПОСЛЕ side-effects, его guard `WHERE status='running'` не сериализует advance: даже
-  проигравший гонку reaper (rowcount==0) уже успел выполнить `advance_stage`.
-
-  Последствия в окне гонки (reaper-тик попал в финализацию живого monitor'а):
-  - `enqueue_job` (`src/db.py:419`) — обычный INSERT **без дедупликации**, поэтому
-    параллельные `advance_stage` от reaper и monitor дают **две `queued`-строки
-    следующего агента** → дублирующий запуск следующей стадии (двойной commit/PR/
-    комментарии, лишние токены);
-  - либо (если гейт следующего ребра ещё красный, напр. CI не позеленел) reaper уходит
-    в `_reap_unknown_outcome` → `reap_running_job(..., "queued")` и **спихивает в queued
-    job, который вот-вот успешно завершится** monitor'ом; при `max_concurrency=1` воркер
-    может повторно заклеймить и **перезапустить тот же агент**.
-
-  Это противоречит FR-1.3/AC-3 (живой агент НЕ должен реапиться) и FR-1.2/AC-4
-  (никакого дубль-advance). Требуется одно из:
-  1. ввести **grace для Tier-2** — реапить только если `finished_at_run` старше
-     заведомо-большего, чем максимальное окно финализации (использовать уже
-     запрашиваемый `finished_at_run`), и/или
-  2. **claim-before-act**: атомарно «застолбить» строку (как требует ADR Р-1) ДО любого
-     `advance_stage`/`enqueue_job`, чтобы проигравший гонку reaper не выполнял побочных
-     эффектов; advance — только после выигранного claim.
-
-  (Tier-1 этим не страдает: streak `reaper_dead_ticks` + мёртвый pid; ветка
-  `_reap_unknown_outcome` без exit0 тоже безопасна — единственное действие там и есть
-  атомарный flip.)
+- нет
 
 ### P2 — Should fix
 - нет
 
 ### P3 — Nice to have
-- [ ] **Битая ссылка в CHANGELOG на глобальный ADR.** В записи ORCH-065 указан
-  `docs/architecture/adr/adr-0011-job-reaper-and-lease-reclaim.md`, фактический файл —
-  `adr-0011-job-reaper-lease-reclaim.md` (без `-and-`). README и `src/job_reaper.py`
-  ссылаются корректно. Поправить путь в `CHANGELOG.md`.
+- нет
 
 ## Документация
 
-Обновлена корректно и в этом же PR (AC-17 PASS по составу): `docs/architecture/README.md`
-(раздел про job-reaper + lease-reclaim, таблицы БД и `/queue`), `docs/architecture/internals.md`,
-`docs/architecture/adr/adr-0011-job-reaper-lease-reclaim.md` (+ запись в `adr/README.md`),
-`docs/work-items/ORCH-065/06-adr/ADR-001-job-reaper-and-lease-reclaim.md`, `CHANGELOG.md`,
-`.env.example` (флаги `ORCH_REAPER_*` / `ORCH_LEASE_RECLAIM_ENABLED`).
-
-Оговорка: ADR-001 Р-1 описывает «claim перед побочными эффектами», но реализация exit0-пути
-этому не следует (см. P1) — при исправлении кода либо привести код в соответствие с ADR,
-либо синхронизировать формулировку ADR с фактическим порядком. Битая ссылка на adr-0011 в
-CHANGELOG — P3.
+Обновлена корректно и в этом же PR (AC-17 PASS): `docs/architecture/README.md`
+(раздел про job-reaper + lease-reclaim, таблицы БД и `/queue`),
+`docs/architecture/internals.md`, `docs/architecture/adr/adr-0011-job-reaper-lease-reclaim.md`
+(+ запись в `adr/README.md`),
+`docs/work-items/ORCH-065/06-adr/ADR-001-job-reaper-and-lease-reclaim.md`, `CHANGELOG.md`
+(ссылка на adr-0011 исправлена), `.env.example` (флаги `ORCH_REAPER_*` /
+`ORCH_REAPER_FINALIZE_GRACE_S` / `ORCH_LEASE_RECLAIM_ENABLED`). ADR-001 Р-1 и реализация
+exit0-пути теперь согласованы (claim-before-act).