Files
orchestrator/docs/work-items/ORCH-124/12-review.md
claude-bot be8ddfcd57
All checks were successful
CI / test (push) Successful in 1m18s
CI / test (pull_request) Successful in 1m13s
reviewer(ET): auto-commit from reviewer run_id=772
2026-06-16 22:31:49 +03:00

12 KiB
Raw Blame History

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-124 review reviewer approved 2026-06-16 claude-opus-4-8 review ORCH-124 3

Review ORCH-124 — serial-gate «пауза без блокировки» (per-task park-сигнал)

Summary

Багфикс (метка Bug, эскалирован в full-cycle) инцидента ORCH-116/ORCH-123: serial-gate считал «активной» любую задачу со стадией вне {done,cancelled}, а Plane-статусы Backlog/Blocked/Needs-Input (слой B) не меняют tasks.stage (слой A) ⇒ приостановленный предшественник держал FIFO-гейт закрытым против срочного успешника. Решение — ортогональная ось планировщика «пауза»: аддитивная колонка tasks.paused_at TEXT + терм AND paused_at IS NULL во всех 3 точках определения «активной задачи» (build_claim_clause/repo_has_active_task/_per_repo_snapshot), операторские эндпоинты POST /serial-gate/pause|resume, под независимым под-флагом serial_gate_pause_enabled.

Вердикт: APPROVED. Оба замечания предыдущего ревью (run v2) устранены коммитом 58e5dfe:

  • P1 (витрина docs/overview/) → исправлено: tech-pipeline.md (ось «пауза без блокировки» рядом с исключением freeze), tech-data-model.md (durable-сигнал tasks.paused_at), tech-observability.md (paused/reason в блоке serial_gate GET /queue + эндпоинты pause|resume) — синхронно в этом PR.
  • P2 (хвостовые tool-call-теги) → исправлено: все 4 golden-source дока этого PR (06-adr/ADR-001, adr-0051, 08-data-requirements.md, 10-tech-risks.md) очищены (проверено tail + grep по изменённым .md; оставшиеся совпадения — лишь описательная проза в CHANGELOG.md/ этом ревью).

Ядро фикса (код + ТЗ + ADR + тесты) — высокого качества, P0/P1-замечаний по сути нет. Реализация 1:1 соответствует ADR-001 (D1D9), закрывает все FR/AC, машинные инварианты не тронуты. Целевой прогон tests/test_orch124_serial_gate_pause.py + tests/test_orch123_staging_runner_exec.py + tests/test_system_docs.py зелёный (70 passed).

Проверка по осям

1. Соответствие ТЗ (02-trz.md / 03-acceptance-criteria.md) —

  • FR-1 (пауза исключает из горячего SQL): build_claim_clause — терм AND t2.paused_at IS NULL внутри существующего EXISTS, offline, fail-OPEN сохранён (внутри того же try/except). ✓
  • FR-2 (зеркало + снапшот согласованы): один предикат во всех 3 точках; анти-дрейф — TC-11. ✓
  • FR-3 (явное durable DB-намерение): колонка paused_at + set/clear/is_task_paused + эндпоинты; durable через рестарт — TC-06. ✓
  • FR-4 (анти-stale-base при resume): через существующие механизмы (D8), без новой rebase-машинерии. ✓
  • FR-5 (причина ожидания): _waiting_reason (freezedependencyactive-tasknull) + ключ pausedTC-10. ✓
  • FR-6 (явные блоки сохранены): пауза НЕ обходит repo_freeze/task_depsTC-08/TC-09 (подтверждено: task_deps читает только терминал {done,cancelled}, не paused_at). ✓
  • FR-7 (условность/нейтральность): под-флаг + scope serial_gate_repos; kill-switch off байт-в-байт — TC-14. ✓
  • AC-1…AC-10 покрыты TC-01…TC-15 буквально по файлам/тестам; целевой прогон зелёный (70 passed).

2. Соответствие ADR (06-adr/ADR-001, сквозной adr-0051) —

Реализация = ADR 1:1: D1 (явный per-task флаг), D2 (tasks.paused_at через _ensure_column, паттерн cancelled_at/track), D3 (ортогональная ось — терминал {done,cancelled} байт-в-байт), D4 (3 точки согласованно), D5 (наблюдаемость), D6 (serial_gate_pause_enabled), D7 (эндпоинты), D8 (анти-stale-base реюзом существующих механизмов), D9 (never-raise/fail-directions).

  • Трассировка (ORCH-078/TRACEABILITY): правка горячего SQL с маркерами ORCH-088/ORCH-090 — инвариант FIFO (t2.id < jobs.task_id, R-7) и терминал-множество {done,cancelled} (adr-0026) не сломаны (структурный TC-15 + сверка: src/task_deps.py/src/stages.py вне диффа, paused_at не читают). Маркер ORCH-124 размещён рядом; сводный сквозной adr-0051 заведён. ✓
  • Нет нарушений глобальных ADR. STAGE_TRANSITIONS/QG_CHECKS/check_*/machine-verdict/схемы существующих таблиц — не тронуты (дифф src/** = только config.py/db.py/main.py/serial_gate.py; stages.py/qg//frontmatter.py/task_deps.py вне диффа — подтверждено git diff --name-only). ✓

3. Качество кода —

  • Чисто, читаемо; docstrings на всех новых публичных функциях; маркеры/ссылки на ADR в коде.
  • never-raise на всех публичных функциях serial_gate/db-хелперах; hot-claim fail-OPEN (pause-терм внутри try/except → ошибка _pause_layer_enabled даёт build_claim_clause()==""), freeze fail-CLOSED — направления не инвертированы (TC-13). ✓
  • Helper-сигнатуры выверены по коду: db.get_task_by_work_item_id существует и возвращает dict (SELECT *paused_at попадает в результат), notifications.link_for/send_telegram существуют (паттерн from .notifications import send_telegram, link_for уже используют coverage_gate/ staging_runner). resume-эндпоинт сознательно НЕ гейтится _pause_layer_enabled() (позволяет снять «залипшую» паузу после выключения слоя) — корректное решение, не дефект.
  • Багфикс-трек (ORCH-019 BR-4) — выполнен: обязательный регресс-тест-фиксатор TC-01 воспроизводит инцидент (красный до фикса — опирается на отсутствовавшие set_task_paused/paused_at; зелёный после); «до-фикса»-поведение бага дополнительно зафиксировано в TC-14 (kill-switch off → паузнутая задача снова блокирует). ✓
  • Тесты содержательные (15 TC, не тривиальные): регресс инцидента, анти-регресс ORCH-088, durable/restart, resume, сохранность freeze/dependency, снапшот-reason, анти-дрейф 3 точек, offline hot-path, never-raise/fail-OPEN, kill-switch-нейтральность, структурный анти-регресс реестров/схем.
  • Тест-гигиена (коммит 3a19728): изоляция settings.repos_dir в фикстуре tests/test_orch123_staging_runner_exec.py устранила order-dependent FAIL test_r2… (фолбэк check_staging_status на реальный <repos_dir>/orchestrator/.../15-staging-log.md после мержа ORCH-123). cfg импортирован в файле (стр. 39); инвариант ORCH-123 R-2 сохранён; правка только теста; прозрачно задокументирована в CHANGELOG. Не блокирует.

4. Документация (обязательная ось) — обновлена полно

src/ изменён → документация обновлена синхронно в том же PR:

  • docs/architecture/README.md (раздел serial-gate + ось «пауза», таблица БД paused_at, таблица API /serial-gate/pause|resume), docs/architecture/internals.md (ось «пауза» ⊥ «терминальность»), CHANGELOG.md (развёрнуто), .env.example (ORCH_SERIAL_GATE_PAUSE_ENABLED).
  • ADR 06-adr/ADR-001 + сквозной adr-0051, 08-data-requirements.md, 10-tech-risks.md.
  • Витрина системы docs/overview/ (ORCH-011) — обновлена (исправление P1 предыдущего ревью): tech-pipeline.md (исключение FIFO «пауза без блокировки»), tech-data-model.md (tasks.paused_at), tech-observability.md (paused/reason + эндпоинты). tests/test_system_docs.py — зелёный.
  • Root README.md «Известные ограничения»: п. про эпик ORCH-088 (пакетный автоном) данным багфиксом не закрывается → обновления не требует.

Findings

P0 — Blocker

  • Нет.

P1 — Must fix

  • Нет. (Оба замечания предыдущего ревью — P1 витрина + P2 теги — устранены коммитом 58e5dfe.)

P2 — Should fix

  • Нет открытых. (Ранее P2 «хвостовые tool-call-теги в 4 доках» устранён.)

P3 — Nice-to-have (не блокирует)

  • HTTP-уровневых тестов для POST /serial-gate/pause|resume нет (терминальный no-op / под-флаг-off no-op / success+telegram). Не блокирует: тонкая обёртка над полностью покрытыми db.set/clear/is_task_paused, а SQL-предикат гейта протестирован исчерпывающе; соответствует конвенции репо (соседние операторские эндпоинты /serial-gate/unfreeze, /transition-lease/release тоже без HTTP-тестов).
  • .task-dev.md (dev-скретч, обновлён метаданными ORCH-124) — pre-existing трекаемый паттерн, вне области ORCH-124; кандидат на .gitignore отдельной задачей.

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

src/ изменён (config/db/main/serial_gate). Все классы документации обновлены синхронно и качественно: инженерные доки (README.md архитектуры, internals.md, CHANGELOG.md, .env.example), оба ADR (work-item 06-adr/ADR-001 + сквозной adr-0051), 08-data-requirements.md, 10-tech-risks.md, и обзорная витрина docs/overview/ (tech-pipeline.md/tech-data-model.md/tech-observability.md — маршрут задач serial-gate с осью «пауза»). Машинные доковые инварианты целы (test_system_docs.py зелёный). Документация = golden source наравне с кодом — требование выполнено. Ось документации закрыта, блокирующих находок нет → APPROVED.