orchestrator/docs/work-items/ORCH-060/12-review.md

type, work_item_id, verdict, version

type	work_item_id	verdict	version
review	ORCH-060	APPROVED	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: код и доку обновлены синхронно. Нарушений нет.