From 2ec4bea5bcb015850d98db165050bd7a6629b2ff Mon Sep 17 00:00:00 2001 From: claude-bot Date: Sun, 7 Jun 2026 15:37:13 +0000 Subject: [PATCH] reviewer(ET): auto-commit from reviewer run_id=319 --- docs/work-items/ORCH-065/12-review.md | 79 +++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 docs/work-items/ORCH-065/12-review.md diff --git a/docs/work-items/ORCH-065/12-review.md b/docs/work-items/ORCH-065/12-review.md new file mode 100644 index 0000000..ee50896 --- /dev/null +++ b/docs/work-items/ORCH-065/12-review.md @@ -0,0 +1,79 @@ +--- +type: review +work_item_id: ORCH-065 +verdict: REQUEST_CHANGES +version: 1 +--- + +# Review ORCH-065 + +## Summary + +Задача закрывает три связанных класса отказов «процесс/поток умер, а ресурс остался +захваченным навсегда»: zombie jobs (A), залипший merge-lease (B), неидемпотентная +финализация merge (C). Реализация качественная: новый daemon-поток `src/job_reaper.py` +по образцу `reconciler` (never-raise, kill-switch, снимок в `/queue`), трёхуровневая +liveness (Tier-1 мёртвый pid + streak, Tier-2 exit_code, Tier-3 backstop), атомарный +reap-claim `reap_running_job(... WHERE status='running')`, gate-driven advance через +неизменный `_try_advance_stage`, проактивный реклейм lease (`pid_alive` + +`reclaim_stale_lease`), колонка `jobs.pid` через идемпотентный `_ensure_column`. +Инварианты соблюдены: `STAGE_TRANSITIONS`/`QG_CHECKS`/`check_*`/БАГ-8/exit-коды хука — +без изменений; реклейм lease — только удаление файла, без git-операций; прод-контейнер +не трогается. Документация (README, internals, ADR, глобальный adr-0011, CHANGELOG, +.env.example) обновлена в этом же PR. `pytest tests/ -q` — **742 passed**. + +Блокер один: дефект Проблемы C (FR-3.2 / AC-11) — детерминированный guard +`pr_already_merged` реализован и юнит-протестирован, но **не подключён ни к одному +реальному пути merge**, при этом ADR и golden-source-документация утверждают обратное. + +## Findings + +### P0 — Blocker +- нет + +### P1 — Must fix +- [ ] **`pr_already_merged` — мёртвый код в проде; ADR/доки утверждают, что он + консультируется (FR-3.2 / AC-11, ADR-001 Р-3).** Функция `merge_gate.pr_already_merged` + определена (`src/merge_gate.py:447`) и покрыта unit-тестами (TC-16/TC-17), но grep по + `src/` показывает **ноль вызовов** из продакшен-кода: ни в `stage_engine.py`, ни в + `webhooks/gitea.py`, ни в `qg/checks.py`, ни в промпте `.openclaw/agents/deployer.md`. + Фактический merge PR в `main` выполняет LLM-агент deployer, и его промпт guard не + упоминает. Тем временем: + - ADR-001 Р-3: «Путь слияния (deployer/merge) **консультируется** с этим guard ПЕРЕД + повторным merge: PR уже слит → no-op (без второго merge и без ошибки)»; + - `docs/architecture/README.md:226`: «добавлен never-raise guard `pr_already_merged` … + уже слит = no-op»; + - CHANGELOG: «`pr_already_merged(...)` — guard **перед повторным merge** при re-drive». + + Следствие: при re-drive (reaper → `queued` → переисполнение стадии `deploy`) с уже + слитым PR гарантия AC-11 «без второго слияния» **не обеспечена детерминированно** — + deployer-агент повторно попытается слить уже слитый PR (Gitea вернёт ошибку + merge → риск ложного БАГ-8 отката). `branch_is_behind_main`-короткое замыкание + (TC-17) делает идемпотентным лишь дорогой rebase+re-test merge-гейта, но НЕ сам акт + слияния. Это прямое расхождение реализации с ADR и golden-source. + + Требуется одно из: + 1. **Подключить guard** к пути merge (инструкция в промпте deployer «перед merge + проверь `pr_already_merged` — уже слит → пометь стадию успешной без повторного + merge», либо детерминированная Python-проверка перед merge), чтобы AC-11 + выполнялся реально; **либо** + 2. если идемпотентность намеренно достигается только gate-driven advance + + `branch_is_behind_main` (а `pr_already_merged` оставлен как helper «на будущее»), + **исправить ADR-001 Р-3, README и CHANGELOG**, убрав утверждение, что путь merge + консультируется с guard, и явно описав, чем именно покрывается «без второго + слияния» из AC-11. + +### P2 — Should fix +- нет + +## Документация + +Обновлена корректно и в этом же PR (AC-17 PASS по составу артефактов): +`docs/architecture/README.md` (раздел про job-reaper + lease-reclaim, таблицы БД и +`/queue`), `docs/architecture/internals.md`, `docs/architecture/adr/adr-0011-*.md` +(+ запись в `adr/README.md`), `docs/work-items/ORCH-065/06-adr/ADR-001-*.md`, +`CHANGELOG.md`, `.env.example` (флаги `ORCH_REAPER_*` / `ORCH_LEASE_RECLAIM_ENABLED`). + +Оговорка: содержание документации про `pr_already_merged` фактически неверно (см. P1) — +guard описан как «консультируемый перед merge», тогда как он нигде не вызывается. +До устранения P1 документация в этой части не является достоверным golden-source.