reviewer(ET): auto-commit from reviewer run_id=486
This commit is contained in:
99
docs/work-items/ORCH-063/12-review.md
Normal file
99
docs/work-items/ORCH-063/12-review.md
Normal file
@@ -0,0 +1,99 @@
|
||||
---
|
||||
verdict: APPROVED
|
||||
work_item: ORCH-063
|
||||
stage: review
|
||||
author_agent: reviewer
|
||||
status: approved
|
||||
created_at: 2026-06-09
|
||||
model_used: claude-opus-4-8
|
||||
type: review
|
||||
work_item_id: ORCH-063
|
||||
version: 1
|
||||
---
|
||||
|
||||
# Review ORCH-063 — INFRA: disk-watchdog мониторинг диска mva154 + алерт при ≥85%
|
||||
|
||||
> Машинный вердикт читается ТОЛЬКО из `verdict:` во frontmatter. `APPROVED` → дальше по конвейеру.
|
||||
|
||||
## Summary
|
||||
|
||||
PR реализует disk-watchdog — фоновый daemon-поток `src/disk_watchdog.py` по канону
|
||||
`reconciler`/`job_reaper`, точно по ТЗ `02-trz.md` и ADR-001/`adr-0024`. Все 9 критериев приёмки
|
||||
(`03-acceptance-criteria.md` AC-1..AC-9) выполнены и покрыты содержательными тестами
|
||||
(`tests/test_disk_watchdog.py`, TC-01..TC-12, 18 кейсов). Полный регресс зелёный: **1296 passed**.
|
||||
Инварианты соблюдены: `STAGE_TRANSITIONS`/`QG_CHECKS`/`check_*`/схема БД — **не тронуты** (проверено
|
||||
`git diff` — `src/stages.py`/`src/stage_engine.py`/`src/qg/` без изменений), миграций нет. Документация
|
||||
обновлена как golden source в том же work-item. **Блокеров (P0/P1) нет → APPROVED.**
|
||||
|
||||
## Оси проверки
|
||||
|
||||
### 1. Соответствие ТЗ / Acceptance Criteria
|
||||
- **AC-1 (heartbeat-демон):** `DiskWatchdog(threading.Thread(daemon=True) + threading.Event)`,
|
||||
`_stop.wait(interval)` (чистый stop, без блокирующего `time.sleep`), контракт
|
||||
`start()`/`stop(timeout)`/`status()`. Старт в `main.lifespan` **после** `reaper.start()`, стоп
|
||||
**первым** в `finally` (reverse) — `src/main.py`. ✓
|
||||
- **AC-2 (алерт ≥ порога):** `format_alert_message` несёт host/путь/`used_pct`/свободно (ГБ+%)/порог;
|
||||
отправка `send_telegram(..., disable_notification=False)` — notifying. Подтверждено TC-08. ✓
|
||||
- **AC-3 (анти-спам):** чистая `decide_action(used_pct, threshold, prev, now, realert_s)`, cooldown
|
||||
`disk_monitor_realert_s`, время инъецируется `now_provider`. TC-02/TC-03 + e2e. ✓
|
||||
- **AC-4 (recovery):** переход «выше→ниже» → ровно одно recovery-сообщение + сброс `alerting`; ниже
|
||||
порога молчит. TC-04 + e2e (`test_tick_antispam_then_realert_then_recovery`). ✓
|
||||
- **AC-5 (config + kill-switch):** 5 флагов `disk_monitor_*` (env `ORCH_DISK_MONITOR_*`, `env_prefix=ORCH_`)
|
||||
+ defensive-валидаторы (порог 1..100, интервалы > 0 → дефолт + warning). `enabled=False` → `start()`
|
||||
no-op (TC-09), `.env.example` обновлён. ✓
|
||||
- **AC-6 (never-raise):** три уровня — per-path (`_measure_one`), per-tick (`_run` outer try/except),
|
||||
per-send (`_send`). TC-07 (битый путь / падение `send_telegram`). ✓
|
||||
- **AC-7 (наблюдаемость):** аддитивный блок `disk_monitor` в `GET /queue`; `status()` never-raise
|
||||
(минимум `{"enabled": …}` при ошибке). TC-11 проверяет сохранность всех существующих ключей. ✓
|
||||
- **AC-8 (источник = хост-ФС):** дефолт `/repos,/app/data` через `shutil.disk_usage`, не overlay `/`,
|
||||
не субпроцесс `df`; дедуп по `st_dev`. TC-06. ✓
|
||||
- **AC-9 (документация):** см. секцию «Документация». ✓
|
||||
|
||||
### 2. Соответствие ADR / инвариантам
|
||||
- Реализация 1:1 с ADR-001 D1–D8: stdlib-замер (D1), дедуп `st_dev` fail-open (D2), pure
|
||||
`decide_action` + in-memory state (D3), прямой `send_telegram` без helper (D4), один порог 85% (D5),
|
||||
lifecycle/врезки (D6), config (D7), инварианты (D8). Сквозной `adr-0024` зарегистрирован в ряду
|
||||
`reconciler`/`job_reaper`.
|
||||
- **Трассировка:** врезки в `main.lifespan` и `@app.get("/queue")` — строго **аддитивные** (новый
|
||||
импорт + один вызов `disk_watchdog.start()/stop()` + ключ `"disk_monitor"`); зафиксированные
|
||||
инварианты соседних маркеров не сломаны. `STAGE_TRANSITIONS`/`QG_CHECKS` не затронуты — подтверждено.
|
||||
|
||||
### 3. Качество кода
|
||||
- Docstrings на всех публичных функциях/классе; чистая leaf-логика отделена от потока (тестируемо).
|
||||
- Defensive-граничные случаи покрыты: `total == 0` → `0.0`, пустой CSV → дефолт, `os.stat` fail → fail-open.
|
||||
- Тесты содержательные (не тривиальные): юнит-решения, измерение/дедуп, e2e цикл alert→silent→realert→
|
||||
recovery, интеграция `/queue`. Полный suite зелёный (1296).
|
||||
|
||||
### 4. Документация (golden source)
|
||||
- `docs/architecture/README.md` — компонент «Disk-watchdog» + описание блока `/queue`. ✓
|
||||
- `docs/operations/INFRA.md` — что мониторится / порог / как отключить / реакция на алерт. ✓
|
||||
- `.env.example` — 5 дескрипторов `ORCH_DISK_MONITOR_*`. ✓
|
||||
- `CHANGELOG.md` — запись `feat:`. ✓
|
||||
- `docs/work-items/ORCH-063/06-adr/ADR-001-disk-watchdog.md` + сквозной `adr-0024`. ✓
|
||||
- `src/` изменён → документация обновлена в том же work-item. Ось пройдена.
|
||||
|
||||
## Findings
|
||||
|
||||
### P0 — Blocker
|
||||
- (нет)
|
||||
|
||||
### P1 — Must fix
|
||||
- (нет)
|
||||
|
||||
### P2 — Should fix
|
||||
- (нет)
|
||||
|
||||
### P3 — Nice-to-have
|
||||
- [ ] Косметика: хвостовые артефакты тул-обёртки `</content>` / `</invoke>`, протёкшие в текст
|
||||
golden-source доков, авторизованных на стадии architecture (НЕ в developer-коммите):
|
||||
`06-adr/ADR-001-disk-watchdog.md` (строки 195–196), `docs/architecture/adr/adr-0024-disk-watchdog.md`
|
||||
(стр. 59), `07-infra-requirements.md` (стр. 63), `10-tech-risks.md` (стр. 39). На парсинг
|
||||
frontmatter/QG не влияют (находятся в конце файла), функциональность не затрагивают — поэтому P3.
|
||||
Рекомендуется зачистить при следующем касании этих доков (правка чужой стадии — по согласованию,
|
||||
CLAUDE.md §3). Не блокирует вердикт.
|
||||
|
||||
## Документация
|
||||
Обновлена полностью в том же work-item: `architecture/README.md` (компонент + блок `/queue`),
|
||||
`operations/INFRA.md` (мониторинг/порог/отключение/реакция), `.env.example` (новые `ORCH_DISK_*`),
|
||||
`CHANGELOG.md` (`feat:`), задачный ADR-001 + сквозной `adr-0024`. Обзорная витрина (README «Известные
|
||||
ограничения») этим PR не затрагивается. Ось документации пройдена — оснований для `REQUEST_CHANGES` нет.
|
||||
Reference in New Issue
Block a user