Files
orchestrator/docs/work-items/ORCH-065/12-review.md
claude-bot 2ec4bea5bc
All checks were successful
CI / test (push) Successful in 18s
CI / test (pull_request) Successful in 18s
reviewer(ET): auto-commit from reviewer run_id=319
2026-06-07 15:37:13 +00:00

5.8 KiB
Raw Blame History

type, work_item_id, verdict, version
type work_item_id verdict version
review ORCH-065 REQUEST_CHANGES 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/ -q742 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.