48 lines
5.8 KiB
Markdown
48 lines
5.8 KiB
Markdown
---
|
||
type: review
|
||
work_item_id: ORCH-026
|
||
verdict: APPROVED
|
||
version: 1
|
||
---
|
||
|
||
# Review ORCH-026
|
||
|
||
## Summary
|
||
ORCH-026 реализует два уровня по ADR-001: **Уровень A** — сериализация merge/deploy внутри одного репо (переиспользует merge-lease ORCH-043/065 + единственная новая логика — безусловный pre-merge rebase под флагом `premerge_rebase_always`) и **Уровень B** — декларативные зависимости задач (аддитивная таблица `job_deps`, гейт `NOT EXISTS` в `claim_next_job`, leaf-модуль `src/task_deps.py`). Реализация минимально-инвазивна, строго соответствует ТЗ и ADR, обе фичи условны (kill-switch) и инертны без данных. Все 16 критериев приёмки выполнены. Полный прогон `pytest tests/ -q` — **991 passed**, из них 50 новых ORCH-026-тестов зелёные. Документация обновлена в том же PR. **APPROVED.**
|
||
|
||
## Findings
|
||
|
||
### P0 — Blocker
|
||
- (нет)
|
||
|
||
### P1 — Must fix
|
||
- (нет)
|
||
|
||
### P2 — Should fix
|
||
- (нет)
|
||
|
||
### P3 — Nice to have
|
||
- [ ] PR-ветка несёт коммиты ORCH-073 (`main` ещё не получил merge #77, merge-base = `77abfb3`). Это ожидаемо по топологии (ORCH-026 (B) построен поверх уже отревьюенного предшественника ORCH-073 (A): у ORCH-073 есть собственные `12-review.md`/`13-test-report.md`/`14-deploy-log.md`) и фактически демонстрирует саму фичу A (rebase B на код A). Не блокирует; при merge в `main` приедут оба набора изменений — это корректно.
|
||
|
||
## Соответствие ТЗ и ADR
|
||
- **Уровень A (AC-A1…A7):** окно сериализации обеспечено существующим merge-lease без нового механизма (ADR §A-1/A-3/A-4). A-2 — `check_branch_mergeable` (`src/qg/checks.py`) под лизом при `premerge_rebase_always=True` всегда вызывает `auto_rebase_onto_main`, снимая short-circuit `branch_is_behind_main`; kill-switch off → поведение ORCH-043 1:1. `STAGE_TRANSITIONS`/`QG_CHECKS`/`Confirm Deploy` не тронуты — соответствует инвариантам §9. Никаких push/force в `main` (только `--force-with-lease` ветки).
|
||
- **Уровень B (AC-B1…B5):** гейт `NOT EXISTS (job_deps JOIN tasks WHERE stage!='done')` в `claim_next_job` (`src/db.py`) — job не выбирается, слот `max_concurrency` не занимается; при выключенном флаге / пустой таблице clause не добавляется (нулевая регрессия). `task_deps.py` — чистый leaf: `is_task_ready` (fail-open), итеративный WHITE/GREY/BLACK DFS-детектор циклов (защита от recursion-limit на проде), `handle_cycle` (Blocked+alert), `declare_dependency`, `ingest_plane_relations` (только `plane|hybrid`, дефолт `db` не ходит в сеть на горячем пути). reconciler F-1 получил Guard 3 (skip dep-заблокированных + backstop детект цикла); reaper не тронут (сканирует `running`).
|
||
- **Общие (AC-G1…G5):** контракт never-raise выдержан во всех новых функциях (try/except, консервативная деградация). Миграция строго аддитивна — `CREATE TABLE/INDEX IF NOT EXISTS`, без `REFERENCES`, схема `tasks`/`jobs` не изменена (AC-G4 OK на живой общей БД). Наблюдаемость — read-only блок `task_deps` в `GET /queue`. Реализация в точности по местам, указанным в ADR §«Места реализации».
|
||
|
||
## Качество кода
|
||
- Docstrings на всех публичных функциях, явно документирован контракт fail-open/fail-closed.
|
||
- SQL-гейт безопасен: `dep_gate` — константная строка (нет инъекции), таблица `job_deps` гарантированно создана в `init_db`.
|
||
- Переменные `plane_id`/`plane_project_id`/`task_id` в `start_pipeline` — в области видимости (проверено).
|
||
- Тесты содержательные: миграция, conditionality (kill-switch), циклы, видимость, observability, интеграция сериализации и зависимостей.
|
||
|
||
## Документация — обновлена (golden source)
|
||
Проверено: код в `src/` изменён → документация обновлена В ТОМ ЖЕ PR (разнесена по pipeline-коммитам ветки, что нормально):
|
||
- `docs/architecture/README.md` — разделы про очередь (`claim_next_job`-гейт), pre-merge rebase, «Зависимости задач: B ждёт A», `job_deps`, наблюдаемость (architect-коммит `f8ec1c2`). ✓
|
||
- `docs/work-items/ORCH-026/06-adr/ADR-001-merge-serialization-and-task-deps.md` + глобальный `docs/architecture/adr/adr-0015-task-deps-and-merge-serialization.md`. ✓
|
||
- `CLAUDE.md` — паспорт (очередь/сериализация). ✓
|
||
- `CHANGELOG.md` — запись `## [Unreleased]`. ✓
|
||
- `.env.example` — `ORCH_PREMERGE_REBASE_ALWAYS`/`ORCH_TASK_DEPS_ENABLED`/`ORCH_TASK_DEPS_SOURCE`. ✓
|
||
- `08-data-requirements.md` — таблица `job_deps`. ✓
|
||
|
||
Документация = golden source: требование выполнено.
|