From 55e5e968ae74d98f6c66ae7c9a39f2e247a189b4 Mon Sep 17 00:00:00 2001 From: claude-bot Date: Sun, 7 Jun 2026 11:53:34 +0000 Subject: [PATCH] reviewer(ET): auto-commit from reviewer run_id=291 --- docs/work-items/ORCH-060/12-review.md | 63 +++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 docs/work-items/ORCH-060/12-review.md diff --git a/docs/work-items/ORCH-060/12-review.md b/docs/work-items/ORCH-060/12-review.md new file mode 100644 index 0000000..a5ba1de --- /dev/null +++ b/docs/work-items/ORCH-060/12-review.md @@ -0,0 +1,63 @@ +--- +type: review +work_item_id: ORCH-060 +verdict: APPROVED +version: 1 +--- + +# Review ORCH-060 + +## Summary +Reviewer-проверка PR `feature/ORCH-060-reconciler-escalated-max-retri` (commit `4db8276`, +`fix(reconciler): skip escalated / Blocked / Needs-Input tasks in F-1`). + +Задача — устранить инцидент ET-013 (бесконечная разблокировка escalated-задачи F-1-реконсайлером). +Реализованы два пред-гарда в `Reconciler._reconcile_gate_task` строго ПОСЛЕ существующих гардов +(`analysis` carve-out → нет гейта → активный job → grace) и ДО `advance_if_gate_passed`: +- **Guard 1** (детерминированный, без сети, проверяется первым): `developer_retry_count(task_id) >= MAX_DEVELOPER_RETRIES`; +- **Guard 2** (Вариант A — Plane API, never-raise → консервативный skip): `_is_blocked_or_needs_input(task)`. + +Реализация **полностью соответствует** ТЗ (`02-trz.md`), критериям приёмки (`03-acceptance-criteria.md`) +и ADR-001. Все 13 AC покрыты тестами (TC-01…TC-11 + sub-flag + F-2-регресс). `pytest tests/ -q` — +**644 passed, 0 регрессий**; `tests/test_reconciler.py` — 27 passed. + +## Соответствие ТЗ / ADR +- **Guard 1** — точка вставки, граница `>=`, источник счётчика (`agent_runs`) совпадают с ТЗ §9 и ADR §«Гард 1». ✓ +- Промоут `stage_engine._developer_retry_count` → публичный `developer_retry_count`, приватный алиас сохранён, все 4 внутренних call-site (`stage_engine.py:565/613/874/950`) работают через алиас — единый источник истины, SQL не дублируется. ✓ +- `MAX_DEVELOPER_RETRIES` импортируется из `stage_engine`, **хардкода `3` в `reconciler.py` нет** (grep подтверждает). ✓ (AC-11) +- **Guard 2 — Вариант A** без миграции БД: новый never-raise `plane_sync.fetch_issue_state` (тот же endpoint/headers, что `fetch_issue_sequence_id`), консервативный фоллбэк (`True`→skip) при любой ошибке/`None`/нерезолвленном проекте. Соответствует ADR §«Гард 2» и обоснованию выбора A над B. ✓ +- Под-флаг `reconcile_skip_blocked_enabled` (default `true`) гасит ТОЛЬКО сетевой Guard 2; Guard 1 всегда активен. ✓ +- Инварианты ORCH-053 сохранены: схема БД / `STAGE_TRANSITIONS` / `QG_CHECKS` не тронуты; never-raise на единицу работы (`reconcile_gate_once` per-task `try/except` + `_is_blocked_or_needs_input` внутренний `try/except`); тишина при пропуске (ранний `return` до `advance`, без `unblocked_total++`/лога/Telegram); `analysis` carve-out и kill-switch'и не изменены. ✓ +- API не изменён (`GET /queue` без изменений по содержимому) — соответствует ТЗ §4. ✓ + +## Качество кода +- Docstrings на новых публичных/значимых функциях (`fetch_issue_state`, `developer_retry_count`, `_is_blocked_or_needs_input`) — содержательные, объясняют контракт never-raise и мотивацию. ✓ +- Обработка Plane-формата `state` (bare uuid и `{"id": ...}`-вложение) — defensive. ✓ +- Тесты содержательные (не тривиальные): граница ровно на лимите (TC-04), изоляция исключения с проверкой соседа (TC-10), отсутствие сетевого вызова гейта на escalated (TC-08), регресс F-2 (TC-09). ✓ +- Self-hosting: чистая логика sweeper'а, прод-контейнер не рестартится/не роняется. ✓ + +## Findings + +### P0 — Blocker +- нет + +### P1 — Must fix +- нет + +### P2 — Should fix +- нет + +> Замечание (P3 / информационно, не блокирует): Guard 2 делает per-candidate сетевой вызов Plane +> для ВСЕХ репо (включая не-self-hosting), а не только для `orchestrator`. Это осознанное решение +> Варианта A, явно зафиксировано в ADR §«Последствия» (митигировано: кандидатов после grace мало, +> `get_project_states` кэшируется, Guard 1 отсекает escalated до сети). Соответствует ADR — не finding. + +## Документация +Обновлено в этом же PR (AC-12 — PASS): +- `docs/work-items/ORCH-060/06-adr/ADR-001-reconciler-skip-escalated.md` — создан, Accepted, полное обоснование A vs B. ✓ +- `docs/architecture/README.md` — описание F-1 дополнено skip escalated/Blocked/Needs-Input; footer ORCH-060 переведён в статус «реализовано» с деталями. ✓ +- `CHANGELOG.md` — запись в `### Fixed` (`fix(reconciler): ...`). ✓ +- `README.md` — таблица env дополнена `ORCH_RECONCILE_SKIP_BLOCKED_ENABLED`. ✓ +- `.env.example` — канонический ключ + дескриптор добавлены (правило CLAUDE.md №8). ✓ + +Документация = golden source: код и доку обновлены синхронно. Нарушений нет.