reviewer(ET): auto-commit from reviewer run_id=705
This commit is contained in:
123
docs/work-items/ORCH-112/12-review.md
Normal file
123
docs/work-items/ORCH-112/12-review.md
Normal file
@@ -0,0 +1,123 @@
|
||||
---
|
||||
verdict: APPROVED # APPROVED | REQUEST_CHANGES — строго одно из двух, UPPERCASE
|
||||
work_item: ORCH-112
|
||||
stage: review
|
||||
author_agent: reviewer
|
||||
status: approved
|
||||
created_at: 2026-06-15
|
||||
model_used: claude-opus-4-8
|
||||
type: review
|
||||
work_item_id: ORCH-112
|
||||
version: 1
|
||||
---
|
||||
|
||||
# Review ORCH-112 — deploy-base checkout-hygiene (resilient-pull)
|
||||
|
||||
## Summary
|
||||
|
||||
Багфикс инцидента **ORCH-111** (bug → escalate full-cycle): прод-self-deploy падал на голом
|
||||
`git pull origin main` хост-хука из-за грязного shared main checkout (остатки ORCH-104 от ORCH-104).
|
||||
Реализован **resilient-pull в хуке** (`--deploy`): перед pull при обнаружении грязи база сводится к
|
||||
чистому `origin/main` (`git fetch` + `git reset --hard origin/main` + скоупленный `git clean -fd`),
|
||||
под kill-switch, never-raise, скоуп self-hosting.
|
||||
|
||||
Проверены все 4 оси. Реализация **точно соответствует** ADR-001 (D1–D7) и сквозному adr-0044, все
|
||||
10 критериев приёмки (AC-1…AC-10) покрыты содержательными тестами, документация (golden source)
|
||||
обновлена в том же PR, инварианты конвейера/БД/exit-code-контракт хука — байт-в-байт не тронуты.
|
||||
|
||||
**Вердикт: APPROVED.** P0/P1/P2 findings отсутствуют.
|
||||
|
||||
### Что сверено (доказательная база)
|
||||
|
||||
**Ось 1 — соответствие ТЗ (02-trz) / критериям (03-acceptance-criteria):**
|
||||
- FR-1 (устойчивый pull) / AC-1 — ✅ хук-блок «2a. Resilient pull» + регресс **TC-01** (зелёный после
|
||||
фикса) и **TC-01b** (тот же грязный base без гигиены → `would be overwritten by merge`, репро инцидента).
|
||||
- FR-1 / AC-2 (untracked WIP) — ✅ **TC-02** (остатки сняты, не протекают в деплой).
|
||||
- NFR-2 / AC-3 (сохранность) — ✅ **TC-03** (`.deploy-prev-image-*`, `deploy-hook.log`, gitignored
|
||||
`.env`/`data/*.db`, sibling `.deploy-state-*`/`.merge-lease-*.json`, `.git/worktrees/*` — на месте) +
|
||||
**TC-05** статический контракт (`git clean -fd`, **никогда `-x`**, явные excludes).
|
||||
- BR-5 / AC-4 (happy-path) — ✅ **TC-04** (чистая база → no-op + fast-forward, exit-коды байт-в-байт).
|
||||
- NFR-1 / AC-5 (self-hosting safety) — ✅ скоуп `$REPO`, `reset --hard origin/main` (не локальная
|
||||
догадка), нет push/force-push (TC-05 ассерт).
|
||||
- FR-5 / AC-6 (kill-switch + обратимость) — ✅ **TC-06** (off → инертно; пустой CSV → self-hosting only;
|
||||
enduro не затронут).
|
||||
- FR-2 / AC-7 (сходимость после cancel/failed) — ✅ **TC-07** (deploy-time self-heal; `cancel_task`
|
||||
корректно НЕ расширён — D4).
|
||||
- FR-4 / AC-8 (наблюдаемость) — ✅ **TC-08** (`read_report`/`alert_dirty` never-raise) + врезка в
|
||||
`run_deploy_finalizer` (sentinel → Telegram, best-effort).
|
||||
- NFR-5 / AC-9 (инвариант конвейера/БД) — ✅ **TC-09** + проверка дифа: `STAGE_TRANSITIONS`/`QG_CHECKS`/
|
||||
`check_*`/machine-verdict/схема БД/exit-code-контракт хука (0/1/2) не тронуты.
|
||||
- BR-3 / AC-10 (документация) — ✅ см. ось 4.
|
||||
|
||||
**Ось 2 — соответствие ADR:**
|
||||
- ADR-001 D1–D7 реализованы дословно: resilient-pull в хуке (не janitor/не container-side, D1),
|
||||
NEVER `-x` + excludes (D2), leaf `checkout_hygiene.py` + инжекция env в `build_deploy_command` (D3),
|
||||
`cancel_task` не расширяется / janitor не вводится (D4), sentinel → finalizer-alert (D5), docs (D6),
|
||||
только `--deploy` не `--build-staging` (D7, подтверждено размещением блока между шагами 1 и 2 пути
|
||||
`--deploy`).
|
||||
- Трассировка (ORCH-078): правка `build_deploy_command` (маркеры ORCH-101/ORCH-058) — чисто аддитивна
|
||||
(одно env-присваивание после `EXPECTED_REVISION`), инвариант image-freshness не сломан; ORCH-036
|
||||
exit-code-контракт и ORCH-090 cancel-каскад не нарушены; INV-4 (никогда push/force-push `main`)
|
||||
соблюдён.
|
||||
|
||||
**Ось 3 — качество кода:**
|
||||
- Leaf чистый, never-raise, ленивые импорты (`self_deploy`/`qg.checks`/`notifications`) — leaf-инвариант
|
||||
доказан **TC-05** (`leaf_is_a_pure_leaf`). Docstrings на всех публичных функциях. `shlex.quote` на
|
||||
инжектируемом пути. Env-проводка консистентна с существующим паттерном `result`-sentinel
|
||||
(`initiate_deploy` пред-создаёт `container_state_dir` → запись `hygiene` гарантированно проходит).
|
||||
- **Багфикс-трек регресс-тест (ORCH-019 / BR-4):** присутствует — **TC-01** (фиксатор дефекта,
|
||||
зелёный после фикса) в паре с **TC-01b** (репродукция инцидента: голый pull аборт без гигиены).
|
||||
- Все ссылки на API существуют (`notifications.link_for`/`send_telegram`, `self_deploy.host_state_dir`/
|
||||
`container_state_dir`, `qg.checks.is_self_hosting_repo`). `repo`/`work_item_id`/`task_id` в скоупе
|
||||
финализатора.
|
||||
- Тесты содержательные: 17 TC (шелл-симуляция реального хука в герметичном git-репо без сети/прода/ssh
|
||||
+ unit). Прогон: **17/17 зелёные**; смежные deploy/config/stage_engine/frontmatter — **200/200 зелёные**;
|
||||
docs/hardcode/canon — **101/101 зелёные**.
|
||||
|
||||
**Ось 4 — документация (golden source):**
|
||||
- `src/` изменён → документация обновлена **в том же PR**: `docs/operations/INFRA.md` (инвариант
|
||||
deploy-база ≠ workspace), `docs/architecture/README.md` (раздел ORCH-112 design), `CHANGELOG.md`,
|
||||
`CLAUDE.md` (паспортный блок), `.env.example` (новые ключи), ADR-001 + сквозной
|
||||
`docs/architecture/adr/adr-0044`. Консистентны между собой и с кодом.
|
||||
- Обзорные доки (ORCH-079): открытые пункты `README.md` «Известные ограничения» (Telegram 48h /
|
||||
intra-repo deps / batch-автоном) этим PR **не закрываются** → обновление не требуется. ✅
|
||||
- Витрина системы (ORCH-011): фикс — внутренняя устойчивость deploy-пути, **не** новая
|
||||
стадия/гейт/агент/интеграция/способность → витрина `docs/overview/` не затронута;
|
||||
`tests/test_system_docs.py` зелёный. ✅
|
||||
|
||||
## Findings
|
||||
|
||||
### P0 — Blocker
|
||||
- Нет.
|
||||
|
||||
### P1 — Must fix
|
||||
- Нет.
|
||||
|
||||
### P2 — Should fix
|
||||
- Нет.
|
||||
|
||||
### P3 — Nice-to-have (не блокирует, на усмотрение)
|
||||
- `tests/test_deploy_checkout_hygiene.py::test_tc05_hook_clean_is_never_destructive` ассертит
|
||||
`assert "-x" not in code` по **всем** исполняемым строкам хука. Текущий хук токена `-x` не содержит
|
||||
(тест зелёный), но будущая легитимная конструкция (`set -x`, `[ -x file ]`, `chmod +x`) ложно уронит
|
||||
ассерт. Можно сузить проверку до строки(ок) `git clean` — но это страховка критичного инварианта
|
||||
INV-HYGIENE-1, поэтому строгость намеренна и допустима. Не блокирует.
|
||||
|
||||
## Документация
|
||||
|
||||
**Статус: обновлена полностью, в том же PR (golden source соблюдён).**
|
||||
|
||||
| Документ | Статус |
|
||||
|----------|--------|
|
||||
| `docs/work-items/ORCH-112/06-adr/ADR-001-deploy-base-checkout-hygiene.md` | ✅ заведён (architecture, после escalate full-cycle) |
|
||||
| `docs/architecture/adr/adr-0044-deploy-base-checkout-hygiene.md` | ✅ сквозной ADR заведён |
|
||||
| `docs/operations/INFRA.md` | ✅ инвариант deploy-база ≠ workspace + страховка resilient-pull |
|
||||
| `docs/architecture/README.md` | ✅ раздел ORCH-112 (design) |
|
||||
| `CHANGELOG.md` | ✅ запись [Unreleased] |
|
||||
| `CLAUDE.md` | ✅ паспортный блок |
|
||||
| `.env.example` | ✅ `ORCH_CHECKOUT_HYGIENE_ENABLED` / `_REPOS` |
|
||||
| `docs/overview/` (витрина, ORCH-011) | ➖ не требуется (внутренний deploy-fix, не новая способность) |
|
||||
| `README.md` «Известные ограничения» (ORCH-079) | ➖ не требуется (открытые пункты не закрываются) |
|
||||
|
||||
Необновлённой документации при изменённом `src/` **нет** → ось 4 пройдена, P0 по документации
|
||||
отсутствует.
|
||||
Reference in New Issue
Block a user