diff --git a/docs/work-items/ORCH-053/12-review.md b/docs/work-items/ORCH-053/12-review.md new file mode 100644 index 0000000..6b5ac96 --- /dev/null +++ b/docs/work-items/ORCH-053/12-review.md @@ -0,0 +1,88 @@ +--- +type: review +work_item_id: ORCH-053 +verdict: APPROVED +version: 1 +--- + +# Review ORCH-053 — Sweeper потерянных webhook (реконсиляция застрявших стадий) + +## Summary +PR реализует фоновый reconciler застрявших стадий ровно в объёме ТЗ (`02-trz.md`) и +ADR (`06-adr/ADR-001`, глобальный `adr-0007`). Все 17 acceptance-criteria покрыты +кодом и тестами; полный прогон `pytest` — **563 passed**. Реализация строго следует +ключевым инвариантам: продвижение только через неизменный `advance_stage(..., +finished_agent=None)`, никакой дублирующей advance/rollback-логики в `reconciler.py`, +структурная невозможность спама нотификаций, never-raise на единицу работы, +restart-safe daemon-поток, kill-switch'и. Схема БД и реестры `STAGE_TRANSITIONS` / +`QG_CHECKS` не тронуты. Документация обновлена в этом же PR. Рекомендация: **APPROVED**. + +## Соответствие ТЗ +- `src/reconciler.py` (НОВЫЙ): F-1 `reconcile_gate_once` + F-2 `reconcile_plane_once`, класс + `Reconciler` + module-singleton по образцу `queue_worker`. ✓ +- `src/config.py`: все 6 `reconcile_*` настроек с дефолтами по таблице §5. ✓ +- `src/main.py`: старт после `worker.start()`, стоп перед `worker.stop()`, блок `reconcile` + в `GET /queue`. ✓ +- `src/stage_engine.py`: тонкий `advance_if_gate_passed` — read-only пред-оценка гейта, + advance только через `advance_stage`, на красном гейте `advance_stage` не вызывается + вовсе (подавление спама без изменения общего критпути). ✓ +- `src/plane_sync.py`: `list_issues_by_state` с курсорной пагинацией и never-raise → `[]`. ✓ +- `src/webhooks/gitea.py`: F-3 БД-fallback `sha→branch` (`_resolve_branch_via_db`), + однозначность обязательна, `debug→info`. ✓ +- `src/webhooks/plane.py` + `src/db.py`: F-2 переиспользует `handle_status_start` / + `handle_verdict` без дублирования; анти-дубль `create_task_atomic` под process-wide Lock, + `start_pipeline` рефакторен на atomic-claim первым DB-действием. ✓ +- Схема БД и реестры не менялись (§6/§8 ТЗ). ✓ + +## Соответствие ADR +- §2 (источник истины — гейт; продвижение только через `advance_stage`): соблюдено — + в `reconciler.py` нет собственного `update_task_stage`/`enqueue_job` для advance (AC-2). +- §3 (async-обработчики из sync-потока через `asyncio.run`): реализовано в `_dispatch`. +- §4 (atomic-claim под `threading.Lock`, без миграции): `db.create_task_atomic`. +- §6 (F-1 не трогает `analysis`): ранний возврат в `advance_if_gate_passed` и в + `_reconcile_gate_task` (AC-16). +- §7 (grace «потерян, а не задержан»): F-1 по `tasks.updated_at` (SQL `age_s`), F-2 по + `issue.updated_at` (`_age_seconds_iso`). +- Нарушений глобальных ADR нет; `adr-0007` заведён и внесён в `docs/architecture/adr/README.md`. + +## Качество кода +- Контракт never-raise выдержан на всех уровнях: outer loop, per-task, per-project, per-issue, + `_parse_grace_overrides`, `list_issues_by_state`, `_resolve_branch_via_db`, телеграм-нотификация. +- Идемпотентность: active-job guard в F-1 и F-2; самозатухание через обновление `updated_at` + после advance; `max_concurrency=1`. Подтверждено анализом — F-2 на approved/rejected всегда + меняет состояние (analysis approved-via-status всегда проходит; rollback всегда срабатывает), + поэтому петли спама нотификаций структурно не возникает. +- Защита от ложного матча в F-3 (только при единственной development-задаче repo). +- Docstrings содержательные на всех публичных функциях; тесты не тривиальные (мапятся на + TC-01…TC-21 из `04-test-plan.yaml`). + +## Документация +Обновлена в этом же PR (AC-17 выполнен): +- `docs/architecture/README.md` — компонент Reconciler, раздел resilience, строка в таблице API + (`/queue` … + reconcile), footer-пометка. ✓ +- `docs/work-items/ORCH-053/06-adr/ADR-001-stuck-task-reconciler.md` — заведён. ✓ +- `docs/architecture/adr/adr-0007-reconciler.md` + строка в `adr/README.md`. ✓ +- `CHANGELOG.md` — запись в `[Unreleased]/Added`. ✓ +- `docs/operations/INFRA.md` — kill-switch'и и env-карта (self-hosting). ✓ +- `README.md` и `.env.example` — env-таблица `ORCH_RECONCILE_*`. ✓ + +## Findings + +### P0 — Blocker +- Нет. + +### P1 — Must fix +- Нет. + +### P2 — Should fix +- Нет. + +### P3 — Nice-to-have +- Несоответствие статуса ADR: `06-adr/ADR-001` помечен `Статус: Proposed`, тогда как + `docs/architecture/adr/README.md` указывает `adr-0007` как `accepted`. Косметика — + привести к одному значению при следующем касании. +- `get_project_states(pid)` теоретически может вернуть словарь без ключей + `approved`/`rejected` при частичном резолве состояний проекта → `KeyError` в + `_reconcile_plane_project`. Сейчас изолировано per-project `try/except` (never-raise + держится, эффект — пропуск F-2 для проекта). Можно усилить `.get(...)`-доступом ради + явности; не блокер.