Files
orchestrator/docs/work-items/ORCH-119/12-review.md
claude-bot 5d41ebab34
All checks were successful
CI / test (push) Successful in 1m20s
CI / test (pull_request) Successful in 1m13s
reviewer(ET): auto-commit from reviewer run_id=786
2026-06-17 14:35:31 +03:00

14 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-119 review reviewer approved 2026-06-17 claude-opus-4-8 review ORCH-119 1

Review ORCH-119 — source-backed 00-business-request.md вместо хардкода TBD

Summary

Чистый, узко-скоупленный багфикс. Раздел «Description» файла 00-business-request.md теперь несёт фактический текст запроса из Plane-issue вместо литерала TBD — на обоих путях создания (прямой A + отложенный срез ветки B, ORCH-088, доминирует на self-hosting). Реализация точно зеркалирует прецедент tasks.title, как и предписал ADR-001: аддитивная колонка tasks.description через _ensure_column, записываемая внутри того же атомарного INSERT create_task_atomic (race-safe vs ORCH-053), читаемая в _spawn_materialize_deferred_branch на момент claim (без сети в горячем пути, NFR-4). Тело рендерится чистым unit-тестируемым хелпером _render_business_request с fail-safe fallback-маркером.

Изменение конвейерной машинерии — ноль: ORCH-119-коммит (4390851) трогает только src/db.py, src/webhooks/plane.py, src/agents/launcher.py, CHANGELOG.md и 2 тест-файла. STAGE_TRANSITIONS / QG_CHECKS / check_* / machine-verdict-ключи / базовый CREATE TABLE tasks — байт-в-байт. Инварианты ORCH-088 (анти-stale-base) и ORCH-053 (атомарный анти-dup-claim) сохранены (правки маркированных блоков аддитивны, с дефолтами; семантика (row, created) и момент/условие среза не меняются — трассировка соблюдена).

Все 4 оси (ТЗ, ADR, качество, документация) пройдены. Все 8 AC и 4 FR удовлетворены. 13/13 тестов ORCH-119 + serial-gate зелёные. Единственный красный в полном прогоне — доказанно пред-существующий, вне-скоупный flake чужой задачи (см. P2 ниже), не вносимый ORCH-119.

Соответствие ТЗ / AC (ось 1) — PASS

AC / FR Статус Подтверждение
AC-1 / FR-1 — реальное описание вместо TBD _render_business_request рендерит тело из description; TC-01 (genuine red→green).
AC-2 / FR-2 — прямой путь A start_pipeline передаёт description в _create_initial_docs (plane.py:716); TC-04.
AC-3 / FR-3 — отложенный путь B / персист персист в атомарном INSERT; чтение SELECT ..., description в _spawn → 5-й арг _materialize_deferred_branch; TC-03.
AC-4 / FR-4 — fail-safe fallback _BUSINESS_REQUEST_FALLBACK, try/except → never-raise; TC-02 (""/" "/None).
AC-5 — гейты/схема не тронуты коммит не касается stages.py/qg/checks.py; единственная схема — аддитивная tasks.description; TC-07.
AC-6 — идемпотентность / BC Gitea 422 → return (тело не перезаписывается); _ensure_column no-op; TC-05/TC-06.
AC-7 — регресс red→green + полный прогон ⚠️ TC-01 доказанно red до фикса (функция отсутствовала + TBD хардкод на main), green после; полный прогон — см. P2 (пред-существующий вне-скоупный flake, не ORCH-119).
AC-8 — сопровождение CHANGELOG.md содержит запись ORCH-119.

Соответствие ADR (ось 2) — PASS

  • D1 (аддитивная колонка в атомарном INSERT) — реализовано буквально: _ensure_column(conn,"tasks","description","TEXT") рядом с tasks.title; запись встроена в существующий INSERT create_task_atomic, не отдельным UPDATE (нет race-окна vs ORCH-053).
  • D2 (чистый рендер-хелпер + проброс на обоих путях) — _render_business_request чистый/network-free; проброс A и B.
  • D3 (fail-safe fallback + идемпотентность) — маркер + never-raise + 422-no-op.
  • D4 (инварианты) — ORCH-088 момент/условие среза не меняются (только источник данных), ORCH-053 семантика цела, гейты байт-в-байт, без kill-switch (фикс дефекта).
  • Трассировка (TRACEABILITY): правки блоков с маркерами ORCH-088 (_materialize_deferred_branch) и ORCH-053 (create_task_atomic) сверены — аддитивные, с дефолтами, зафиксированные инварианты не сломаны; комментарии кода корректно ссылаются на оба ADR.
  • Сквозной (global) ADR не требуется — локальное аддитивное решение по прецеденту.

Качество кода (ось 3) — PASS

  • Чистый, network-free, unit-тестируемый хелпер; docstrings на всех новых/изменённых функциях.
  • never-raise контракт соблюдён (try/except в рендере; создание задачи не падает).
  • BC: все добавленные параметры аддитивны с дефолтами; единственный прочий потребитель (serial-gate spy в test_serial_gate_branch.py) синхронно обновлён.
  • Багфикс-регресс (ORCH-019 / BR-4): TC-01 — настоящий фиксатор дефекта (red до фикса: _render_business_request отсутствовал + _create_initial_docs хардкодил TBD на main; green после). Подтверждено git show main:src/webhooks/plane.py.
  • Тесты содержательные: 7 TC покрывают регресс, fallback (4 параметра), оба пути, schema-BC, идемпотентность 422, multiline/спецсимволы, анти-регресс гейтов.

Документация (ось 4) — PASS

Изменён src/ → проверка обязательна. Документация обновлена в том же PR:

  • CHANGELOG.md — подробная запись ORCH-119 (AC-8).
  • ADR-001 (06-adr/ADR-001-source-backed-business-request-doc.md) + 08-data-requirements.md — присутствуют, исчерпывающие.
  • Центрального schema-дока колонок tasks нет — конвенция проекта: per-task скалярные колонки документируются в ADR задачи + data-requirements + CHANGELOG (так же оформлены tasks.title / tasks.track / tasks.cancelled_at). ORCH-119 ей следует.
  • Витрина docs/overview/ (ORCH-011): tech-agents.md по-прежнему верно указывает 00-business-request.md как вход аналитика; tech-data-model.md описывает tasks концептуально (без перечня колонок) — не устаревает добавлением колонки. Обновление не требуется.
  • README.md «Известные ограничения» (ORCH-079): TBD-баг 00-business-request.md не числится среди открытых ограничений (Telegram-48h / intra-repo deps / batch-автоном) — закрывать/снимать нечего, обновление README не требуется.
  • PIPELINE_DOCS.md / HANDOFF_PROTOCOL.md: описывают артефакт по владельцу/гейт-статусу (_create_initial_docs, информационный, не парсится) — продюсер и статус не изменились, не устаревают.

Findings

P0 — Blocker

  • Нет.

P1 — Must fix

  • Нет.

P2 — Should fix (вне скоупа ORCH-119 — НЕ блокер; рекомендация отдельной задачи)

  • Полный pytest tests/ -q красный: 1 failtests/test_orch123_staging_runner_exec.py::test_r2_held_deploy_staging_not_rolled_back (2172 passed, детерминированно). Номинально это задевает букву AC-7 («полный прогон зелёный»), но провал доказанно пред-существующий и полностью независимый от ORCH-119:
    • Не ORCH-119: ORCH-119-коммит 4390851 не трогает stage_engine / staging_runner / qg/checks / 15-staging-log / конфиг, читаемый check_staging_status. Тест-файл ORCH-119 не загрязняет ORCH-123 (вместе 11/11 зелёных); провал воспроизводится при полностью деселектнутом ORCH-119.
    • Корень — isolation-дефект чужого (уже смерженного) теста ORCH-123: тест переиспользует work_item_id="ORCH-123", а в дереве лежит закоммиченный артефакт docs/work-items/ORCH-123/15-staging-log.md (приехал при мерже ORCH-123). При полном импорте сьюта (module-level env/CWD-мутации множества тест-файлов) check_staging_status резолвит этот закоммиченный лог → гейт «зеленеет» → advance вызывается → assert result is None падает. Файл ORCH-123 изолированно зелёный (26/26).
    • Не блокирует пайплайн в реальной среде: ORCH-116 смержен в main после ORCH-123 и прошёл весь конвейер (включая testing/детерминированный test-runner ORCH-116) при уже присутствующем 15-staging-log.md ORCH-123 → flake не воспроизводится в реальном test-runner/CI, это артефакт локального порядка импорта полного tests/.
    • Действие: завести отдельный bug на test-hygiene ORCH-123 (тест не должен переиспользовать живой work_item_id с закоммиченными артефактами и зависеть от глобального CWD/конфига). Править здесь нельзя — чужой артефакт стадии, вне скоупа ORCH-119 (правило «не править артефакты других этапов»).

P3 — Nice-to-have

  • Тест-файл tests/test_orch119_business_request.py мутирует os.environ (ORCH_DB_PATH/ORCH_REPOS_DIR) на уровне модуля (импорт) — добавляет к общей import-pollution-поверхности сьюта. Соответствует существующей конвенции многих тест-файлов проекта (не новый грех), на ORCH-119-провалы не влияет; на будущее — скоупить через фикстуру.

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

Статус: обновлена в том же PR — достаточно. CHANGELOG.md (AC-8) + ADR-001 + 08-data-requirements.md — присутствуют и исчерпывающи. Витрина docs/overview/, обзорный README.md («Известные ограничения»), PIPELINE_DOCS.md/HANDOFF_PROTOCOL.md проверены явно — не устаревают (артефакт остаётся информационным, продюсер/владелец/гейт-статус не изменились; TBD-баг не числился среди открытых ограничений). Центрального schema-дока колонок tasks нет — аддитивная колонка документирована по конвенции (ADR + data-requirements + CHANGELOG, как tasks.title/track/cancelled_at). Необновлённой обязательной документации не найдено.

Вердикт

APPROVED. Нет P0/P1. Все AC/FR/ADR удовлетворены; реализация — точное зеркало одобренного прецедента с нулевым касанием конвейерной машинерии и сохранёнными инвариантами ORCH-088/ORCH-053. Единственный красный полного прогона — доказанно пред-существующий, детерминированный, вне-скоупный flake чужой смерженной задачи (ORCH-123), который ORCH-119 не вносил и не может починить в своём скоупе; дух AC-7 (отсутствие внесённых регрессий, собственные тесты зелёные) соблюдён.