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

verdict, work_item, stage, author_agent, status, created_at, model_used, type, work_item_id, version

verdict	work_item	stage	author_agent	status	created_at	model_used	type	work_item_id	version
APPROVED	ORCH-090	review	reviewer	approved	2026-06-09	claude-opus-4-8	review	ORCH-090	2

Review ORCH-090 — Механизм отмены задачи: статус STOP (re-review, attempt 2)

Summary

Повторный review после фикса блокирующего P1 из предыдущей итерации (12-review.md v1). Реализация STOP-отмены аккуратна и канонична (leaf src/cancel.py never-raise, kill-switch stop_status_enabled, fail-closed маршрутизация по образцу confirm_deploy/ORCH-059, аддитивные идемпотентные миграции). Кросс-каттинг {done} → {done, cancelled} проведён исчерпывающе и консистентно (serial_gate / task_deps / stages / db / job_reaper / queue_worker), в точности по adr-0026.

Оба ранее блокировавших/важных дефекта закрыты и покрыты содержательными тестами:

• P1 (был blocker) — ИСПРАВЛЕН. cancel.in_critical_window сужен: удержание merge-lease без бегущего актора (_task_has_running_actor) на стадии deploy в ожидании Confirm Deploy теперь НЕ считается критическим окном → немедленный полный сброс, который сам отпускает lease (шаг 3c). Тесты test_d7_lease_held_idle_parking_is_not_critical, test_d7_lease_held_with_running_actor_still_critical, test_d7_stop_on_deploy_awaiting_confirm_full_resets (последний прямо проверяет stage='cancelled' + удалённую ветку + current_lease_holder is None). Сверено по коду src/cancel.py::in_critical_window (стр. 100–158) и stage_engine.cancel_task — wedge self-hosting-репо устранён.
• P2 (был should-fix) — ИСПРАВЛЕН. Deferred-ветка cancel_task шлёт алерт только при первом переходе (first = set_task_cancel_requested(...), далее if first:); повторный STOP в критическом окне даёт deferred-already-pending без повторного уведомления. Тест test_d7_repeated_stop_in_critical_window_no_duplicate_notify (ровно 1 notify).

Полный регресс pytest tests/ зелёный (1349 passed); tests/test_stop_status.py — 30 кейсов (TC-01…TC-14 + D7), покрывают AC-1…AC-10 и оба фикса.

Оси проверки: ✅ ТЗ/AC (AC-1…AC-10, включая ранее проваленный AC-7) · ✅ ADR (соответствие adr-0026/ADR-001; см. P2-нит ниже) · ✅ качество кода · ✅ документация.

Findings

P0 — Blocker

• (нет)

P1 — Must fix

• (нет)

P2 — Should fix

• Work-item ADR-001 §D7 не синхронизирован с фиксом P1 (running-actor-уточнение). docs/work-items/ORCH-090/06-adr/ADR-001-stop-cancel-task.md §D7 (стр. 189–201) по-прежнему определяет критическое окно как «задача держит merge-lease … / merge в процессе» — без оговорки «И активно бегущий актор», которую фактически реализует код (cancel.in_critical_window + _task_has_running_actor) после фикса P1. Авторитетные golden-source доки уже синхронизированы (CLAUDE.md — абзац «Уточнение P1 (ORCH-090 review)»; docs/architecture/README.md стр. 316–317 «P1-уточнение»; CHANGELOG.md — буллет «Фикс P1»), поэтому витрина проекта корректна и это не P0 «src изменён, доки не обновлены». Но per «documentation = golden source» работа-айтемный ADR (запись именно этого архитектурного решения) должен честно отражать итоговую семантику — как это уже сделано для уточнения D4. Предложение: добавить в §D7 строку-уточнение «merge-lease критичен ТОЛЬКО при бегущем акторе; припаркованное ожидание Confirm Deploy обратимо → немедленный сброс» (ссылка на review P1). Не блокирует.

P3 — Nice to have

• «Завис» cancel_requested_at на успешно задеплоенной задаче → вечный pending в GET /queue (перенесено из v1, не адресовано). При SUCCESS-деплое run_deploy_finalizer вызывает cancel_task(force=True), который видит stage='done' → «already-terminal» no-op и не очищает cancel_requested_at; db.cancelled_tasks_snapshot считает pending = cancel_requested_at IS NOT NULL AND stage != 'cancelled' → done-задача с бывшим deferred-STOP навсегда показывается «pending». Чисто наблюдаемость; предложение — очищать cancel_requested_at при честном no-op после завершения.
• adr-0026 п.6 (post-deploy monitor «не тикает по отменённой задаче») в коде не реализован (перенесено из v1). Фактически безвреден и недостижим: post-deploy наблюдение идёт только ПОСЛЕ done, а STOP на done — no-op. Рекомендация: снять пункт из adr-0026 как нерелевантный либо добавить дешёвый терминал-гард для строгого соответствия ADR.
• Косметика: «рваная» строковая склейка комментария relaunch-hole в src/webhooks/plane.py (стр. 345–351) — собрать в одну строку для читаемости.

Документация

Обновлена полностью и качественно — отдельных blocking-findings нет. Проверено пофайльно:

• README.md — таблица env (ORCH_STOP_STATUS_ENABLED/ORCH_STOP_STATUS_REPOS), раздел «Отмена задачи: статус STOP (ORCH-090)», обновлён список job-статусов (cancelled), инфра-предусловие.
• docs/architecture/README.md — раздел STOP со статусом «реализовано», блок stop в /queue, раздел «База данных» (колонки/тумбстон/статусы) и P1-уточнение (стр. 316–317).
• docs/architecture/internals.md — STAGE_TRANSITIONS (сток cancelled), терминал-предикат {done,cancelled}, job-статусы.
• CHANGELOG.md (feat: + отдельный буллет «Фикс P1»), CLAUDE.md (раздел «Отмена задачи: статус STOP (ORCH-090)» с абзацем «Уточнение P1»), .env.example — согласованы.
• ADR: локальный 06-adr/ADR-001-stop-cancel-task.md + сквозной docs/architecture/adr/adr-0026-stop-cancel-task.md; уточнение D4 (тумбстон plane_issue_id) отражено в коде и доках. Единственный gap — §D7 локального ADR не дотянут до running-actor-фикса (P2 выше).
• Раздела README «Известные ограничения», который ORCH-090 закрывал бы (ORCH-079), нет — обзорная витрина не рассинхронена.

Трассировка маркеров (TRACEABILITY.md): правки маркированных инвариантов serial_gate/ORCH-088 и task_deps/ORCH-026 сверены с их ADR — расширение терминал-набора до {done,cancelled} сохраняет FIFO-семантику (t2.id < jobs.task_id) и dep-готовность (терминальный предшественник), инварианты не сломаны. STAGE_TRANSITIONS exit-гейты / QG_CHECKS / check_* — не тронуты (подтверждено анти-регресс-снапшотами, зелёные).

Вердикт

APPROVED — оба ранее найденных дефекта (P1 wedge при STOP в ожидании Confirm Deploy; P2 дубль-уведомления в deferred-ветке) исправлены и покрыты содержательными тестами; полный регресс зелёный (1349 passed). Остаются только P2 (синхронизация §D7 локального ADR) и P3 (наблюдаемость/ косметика) — не блокируют приёмку, желательны к устранению попутно.