reviewer(ET): auto-commit from reviewer run_id=438
This commit is contained in:
85
docs/work-items/ORCH-088/12-review.md
Normal file
85
docs/work-items/ORCH-088/12-review.md
Normal file
@@ -0,0 +1,85 @@
|
||||
---
|
||||
type: review
|
||||
work_item_id: ORCH-088
|
||||
verdict: APPROVED
|
||||
version: 1
|
||||
---
|
||||
|
||||
# Review ORCH-088 — Per-repo serial gate (Этап 1, serial e2e)
|
||||
|
||||
## Summary
|
||||
PR реализует per-repo serial gate (FR-1…FR-5) тремя согласованными механизмами в полном
|
||||
соответствии с ТЗ и ADR-001: gate-в-claim (`db.claim_next_job`), отложенный срез ветки
|
||||
(`start_pipeline` → `launcher._materialize_deferred_branch`) и durable rollback-freeze
|
||||
(`repo_freeze` + `POST /serial-gate/unfreeze`). Чистая логика вынесена в leaf-модуль
|
||||
`src/serial_gate.py` (never-raise). Полный прогон `pytest tests/ -q` — **1114 passed**;
|
||||
профильные сюиты (`test_serial_gate*`, `test_queue_endpoint`, `test_plane_webhook`,
|
||||
`test_status_trigger`) — 33 passed. Документация обновлена в том же PR. Блокеров нет.
|
||||
|
||||
## Оси проверки
|
||||
|
||||
### 1. Соответствие ТЗ / AC
|
||||
- FR-1 (gate на входе в анализ) — gate-фрагмент в `claim_next_job`, только `jobs.agent='analyst'`,
|
||||
только локальная БД (NFR-2). AC-1 ✓
|
||||
- FR-2 (очередь e2e, FIFO) — реализация уточняет псевдо-SQL ADR `t2.id != jobs.task_id` на
|
||||
`t2.id < jobs.task_id`. Уточнение **корректно и обосновано** (при `!=` пакет одновременно
|
||||
созданных задач взаимно блокируется → дедлок); задокументировано в коде, CHANGELOG и README.
|
||||
AC-2 ✓
|
||||
- FR-3 (per-repo) — все выборки фильтруются `t2.repo = jobs.repo`; cross-repo параллелизм
|
||||
сохранён. AC-4 ✓
|
||||
- FR-4 (restart-safe) — активная задача из `tasks`, freeze в `repo_freeze`; in-memory состояния
|
||||
нет. AC-3 ✓
|
||||
- FR-5 (rollback-freeze) — `set_repo_freeze` в DEGRADED-ветке `run_post_deploy_monitor` +
|
||||
Telegram-алерт; ручное снятие `POST /serial-gate/unfreeze`. AC-5 ✓
|
||||
- AC-6 (анти-stale-base) — закрыт **структурно**: ветка не создаётся до открытия gate
|
||||
(deferred cut в `_materialize_deferred_branch` от свежего `origin/main`). ✓
|
||||
- AC-7 (kill-switch/нулевая регрессия), AC-8 (fail-OPEN claim), AC-9 (fail-CLOSED freeze),
|
||||
AC-10 (`/queue` блок), AC-11 (инварианты) — все подтверждены кодом и тестами.
|
||||
|
||||
### 2. Соответствие ADR
|
||||
- D1–D10 реализованы как описано. Единственное отклонение — FIFO-условие `<` вместо `!=`
|
||||
(D1) — улучшает ADR, устраняет дедлок, явно задокументировано. Глобальный ADR
|
||||
`adr-0017-serial-gate.md` заведён и зарегистрирован.
|
||||
- `STAGE_TRANSITIONS` / `QG_CHECKS` / `check_*` / merge-gate / merge-verify / image-freshness /
|
||||
post-deploy / exit-коды хука — без изменений (AC-11). ✓
|
||||
|
||||
### 3. Качество кода
|
||||
- Leaf-модуль `src/serial_gate.py` — строгий never-raise; корректно разнесены направления
|
||||
отказа: claim — fail-OPEN (`build_claim_clause` → `""`), freeze — fail-CLOSED
|
||||
(`is_repo_frozen` → `True`). Санитизация repo-токенов `^[A-Za-z0-9._-]+$` перед встраиванием
|
||||
в SQL `IN (...)`.
|
||||
- Миграция `repo_freeze` аддитивна и идемпотентна (`CREATE TABLE/INDEX IF NOT EXISTS`).
|
||||
- `_materialize_deferred_branch` исполняется в worker-потоке (нет running loop) → `asyncio.run`
|
||||
безопасен; Gitea-вызовы идемпотентны (409/422 → no-op) → реклейм/рестарт безопасны; transient
|
||||
Gitea-ошибка пробрасывается → job переочередь (нет half-cut состояния).
|
||||
- Docstrings содержательны на всех публичных функциях.
|
||||
|
||||
### 4. Качество тестов
|
||||
Содержательные сюиты покрывают gate (claim), deferred branch, e2e, freeze, `/queue`-snapshot,
|
||||
webhook и status-trigger. Тесты не тривиальны (проверяют поведение, а не факт вызова).
|
||||
|
||||
## Findings
|
||||
|
||||
### P0 — Blocker
|
||||
- (нет)
|
||||
|
||||
### P1 — Must fix
|
||||
- (нет)
|
||||
|
||||
### P2 — Should fix
|
||||
- (нет — отмечено лишь как наблюдение) `_materialize_deferred_branch` делает два отдельных
|
||||
`asyncio.run` подряд; функционально корректно, можно объединить в один loop при будущем
|
||||
рефакторинге. Не блокирует.
|
||||
|
||||
## Документация
|
||||
Обновлена в том же PR — правило golden-source (CLAUDE.md §2) выполнено:
|
||||
- `docs/architecture/README.md` — новый раздел «Per-repo serial gate (ORCH-088)», обновлены
|
||||
таблица API (`GET /queue` + новый `POST /serial-gate/unfreeze`), раздел БД (`repo_freeze`),
|
||||
строка статуса доработок.
|
||||
- `CLAUDE.md` — абзац о serial-режиме в разделе «Очередь задач».
|
||||
- `CHANGELOG.md` — запись `feat:` (ORCH-088).
|
||||
- `.env.example` — три новых флага с описанием.
|
||||
- `docs/work-items/ORCH-088/06-adr/ADR-001-serial-gate.md` + сквозной
|
||||
`docs/architecture/adr/adr-0017-serial-gate.md` + `08-data-requirements.md`.
|
||||
|
||||
Изменения `src/` полностью отражены в документации → требование Reviewer §4 удовлетворено.
|
||||
Reference in New Issue
Block a user