Files
orchestrator/docs/work-items/ORCH-124/12-review.md
claude-bot ec932264db
All checks were successful
CI / test (push) Successful in 1m16s
CI / test (pull_request) Successful in 1m12s
reviewer(ET): auto-commit from reviewer run_id=768
2026-06-16 20:24:55 +03:00

15 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
REQUEST_CHANGES ORCH-124 review reviewer changes-requested 2026-06-16 claude-opus-4-8 review ORCH-124 2

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 точках определения «активной задачи», операторские эндпоинты POST /serial-gate/pause|resume, под независимым под-флагом serial_gate_pause_enabled.

Ядро реализации (код + ТЗ + ADR + тесты) — высокого качества и P0/P1-замечаний по сути фикса не имеет. Реализация 1:1 соответствует ADR-001 (D1D9), закрывает все FR/AC, машинные инварианты не тронуты, полный прогон pytest tests/ зелёный (2178 passed на текущем HEAD; ранее красный order-dependent test_orch123::test_r2… исправлен изоляцией repos_dir в коммите 3a19728).

Но есть один P1 по оси документации: PR меняет описанную в витрине системы (docs/overview/tech-pipeline.md) функциональность serial-gate (маршрут задач внутри репо), но витрину не обновил — нарушение норматива сопровождения docs/overview/README.md и правила агентов §6 (ORCH-011/ORCH-079). Предыдущий ревью (run 766) ошибочно заключил «serial-gate не фигурирует в витрине»; по факту serial-gate описан в 4 файлах витрины, и tech-pipeline.md несёт абсолютное утверждение FIFO, которое ORCH-124 делает неполным. Вердикт: REQUEST_CHANGES (исправляется обновлением витрины в этом же PR).

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

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 буквально по файлам/тестам; прогон зелёный.

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/stages.py, src/qg/, src/frontmatter.py вне диффа — подтверждено по git diff --name-only). ✓

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

  • Чисто, читаемо; docstrings на всех новых публичных функциях; маркеры/ссылки на ADR в коде.
  • never-raise на всех публичных функциях serial_gate/db-хелперах; hot-claim fail-OPEN, freeze fail-CLOSED — направления не инвертированы (TC-13, инъекция ошибки в _pause_layer_enabledbuild_claim_clause()=="" → claim не падает).
  • Helper-сигнатуры выверены по коду: db.get_task_by_work_item_id существует (SELECT *paused_at попадает в dict), notifications.link_for/send_telegram существуют. 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, анти-дрейф 3 точек, offline hot-path, never-raise, 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). Инвариант ORCH-123 R-2 сохранён; правка только теста — корректно. Чуть вне строгой области ORCH-124, но прозрачно задокументирована в CHANGELOG. Не блокирует.

4. Документация (обязательная ось) — ⚠️ обновлена НЕПОЛНО (см. P1)

  • 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 — обновлены качественно.
  • docs/overview/ (витрина системы) НЕ обновлена при изменении описанного в ней маршрута задач serial-gate → P1 (детали ниже). serial-gate присутствует в витрине (grep: tech-pipeline.md, tech-data-model.md, tech-observability.md, tech-quality-security.md).
  • Root README.md «Известные ограничения»: п.3 — про эпик ORCH-088 (пакетный автоном), данным багфиксом не закрывается → обновления не требует.

Findings

P0 — Blocker

  • Нет.

P1 — Must fix

  • Витрина системы docs/overview/ не обновлена при изменении описанной в ней функциональности serial-gate (ORCH-011/ORCH-079, правило агентов §6 + норматив docs/overview/README.md). docs/overview/tech-pipeline.md §«Последовательность внутри репозитория (serial gate)» (стр. 103108) несёт абсолютное утверждение маршрута: «Новая задача репозитория не входит в работу, пока не завершена более ранняя (FIFO)» и уже перечисляет исключение freeze («следующие задачи ждут»). ORCH-124 вводит второе намеренное исключение — оператор может поставить предшественника на паузу, и срочный успешник его обгоняет. После фикса утверждение FIFO неполно/вводит в заблуждение, а норматив сопровождения витрины (docs/overview/README.md: «маршруты задач → tech-pipeline.md») требует синхронного обновления в том же PR. Как исправить: добавить в tech-pipeline.md (раздел serial gate) краткую фразу про ось «пауза без блокировки» (оператор паузит предшественника → срочный успешник обгоняет; пауза ≠ cancel, не обходит freeze/dependency), рядом с уже описанным исключением freeze. Рекомендуется заодно (та же ось, во избежание повторного REQUEST_CHANGES): tech-data-model.md — упомянуть durable-сигнал tasks.paused_at рядом с repo_freeze; tech-observability.md — ключ paused/reason в блоке serial_gate GET /queue и операторские эндпоинты pause|resume. Машинные доковые тесты (test_system_docs.py) сейчас зелёные — это пробел точности прозы, который машинно не ловится.

P2 — Should fix

  • Мусорные хвостовые теги </content> / </invoke> в 4 golden-source доках этого PR. Литерально закоммичены в конце файлов: docs/work-items/ORCH-124/06-adr/ADR-001-serial-gate-pause-without-blocking.md (стр. 299300), docs/architecture/adr/adr-0051-serial-gate-pause-without-blocking.md (стр. 111), docs/work-items/ORCH-124/08-data-requirements.md (стр. 54), docs/work-items/ORCH-124/10-tech-risks.md (стр. 41) — протёкшие в контент закрывающие XML-теги tool-call'а архитектора. Frontmatter цел, машинный парсинг не страдает, но golden-source доки не должны нести этот мусор. (Системный pre-existing паттерн — 52 файла по репо, в т.ч. в уже смерженном ORCH-114 → отдельная гигиеническая зачистка уместна; но 4 файла этого PR следует почистить здесь.)

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). Golden-source инженерные доки (README.md архитектуры, internals.md, CHANGELOG.md, .env.example), оба ADR (work-item + сквозной adr-0051) и 08-data-requirements.md обновлены синхронно и качественно. Однако обзорная витрина docs/overview/ (tech-pipeline.md, обязательно; tech-data-model.md/tech-observability.md, рекомендуется) НЕ обновлена, хотя ORCH-124 меняет описанный в ней маршрут задач serial-gate — это P1 (ORCH-011/ORCH-079). Документация = golden source наравне с кодом; до устранения P1 — REQUEST_CHANGES. После добавления оси «пауза» в витрину (и зачистки P2-тегов) ядро фикса готово к приёмке.