reviewer(ET): auto-commit from reviewer run_id=554

This commit is contained in:
2026-06-10 02:57:31 +03:00
committed by orchestrator-deployer
parent a98d605477
commit 5d4ef9369e

View File

@@ -0,0 +1,105 @@
---
verdict: APPROVED
work_item: ORCH-057
stage: review
author_agent: reviewer
status: approved
created_at: 2026-06-10
model_used: claude-opus-4-8
type: review
work_item_id: ORCH-057
version: 1
---
# Review ORCH-057
## Summary
Follow-up ORCH-040: закрыт недоделанный AC по legacy `root:root` файлам, ломавшим создание
worktree под uid 1000. Реализованы три аддитивных, обратимых kill-switch'ем слоя ровно по ADR-001:
**D1** actionable-ошибка в `ensure_worktree`, **D2** детект-леаф `src/fs_normalize.py`
(never-raise, TTL-кэш, scope-aware), **D3** best-effort startup-наблюдаемость в `main.lifespan`
(WARNING + Telegram, claim не блокируется), плюс `GET /queue` блок `fs_ownership` и
`POST /fs-normalize/check`. Документация (INFRA.md процедура, architecture/README.md, сквозной
adr-0031, CHANGELOG, .env.example) обновлена в том же PR.
Проверено по 4 осям; все 7 AC выполнены, P0/P1 findings нет. Регресс `pytest tests/ -q`
**1507 passed**; целевые модули (`test_fs_normalize`, `test_fs_normalize_startup`,
`test_git_worktree_perm`, `test_api_queue`) — **25 passed**, покрывают TC-01…TC-12.
## Соответствие ТЗ (02-trz) и AC (03-acceptance-criteria)
- **FR-1 / AC-1, AC-2** ✓ — `git_worktree._raise_if_permission` + `fs_normalize.is_permission_failure`/
`build_worktree_help`: класс «нет прав» (`Permission denied`/`could not create leading directories`/
`insufficient permission`/`PermissionError`/`EACCES`/`EPERM`) → actionable `RuntimeError` с причиной,
`chown`-командой и ссылкой на INFRA.md. Обе точки сбоя обёрнуты (`os.makedirs` + оба `worktree add`).
Не-прав-ошибки сохраняют прежний raw-контракт (TC-02 PASS). Под kill-switch — no-op, контракт 1:1.
- **FR-2 / AC-3** ✓ — `scan_ownership` обходит `/repos/_wt`, `<repo>/.git/{objects,worktrees}`,
`data/runs`; ранний выход на первом `lstat.st_uid != target_uid`; чистая среда → `mismatch=False`
идемпотентно; never-raise → консервативный `mismatch=False` (TC-03/04/05).
- **FR-3 / AC-4** ✓ — startup-хук never-fatal: WARNING + Telegram при mismatch; claim не блокируется
(D3, преднамеренно — внятный ранний отказ даёт D1, знающий repo). Read-only блок `fs_ownership` в
`GET /queue` (TC-10/TC-12).
- **FR-4** ✓ — `normalize()` chown только при `_is_privileged()` (geteuid==0); под uid 1000 — no-op +
честный лог, НЕ ошибка; gated `fs_normalize_auto` (дефолт False) (TC-08).
- **FR-5 / AC-7** ✓ — INFRA.md: блокер P-5 + подраздел «Миграция uid: обязательная нормализация»
со всеми корнями; work-item ADR + сквозной adr-0031.
- **§7 совместимость / AC-5** ✓ — `applies(repo)` first (kill-switch + scope; пустой CSV →
self-hosting only через `is_self_hosting_repo`); enduro-trails не сканируется при дефолте.
TTL-кэш (`fs_scan_cache_ttl_s`). Регресс зелёный (1507 passed).
## Соответствие ADR
- Реализация совпадает с ADR-001 D1D6 (включая сознательный отказ от блокирующего claim-гейта и
init-контейнера — обоснование в «Альтернативах»). Сквозная регистрация adr-0031 присутствует и
отражена в architecture/README.md.
- **Трассировка (AC-6 / TRACEABILITY):** инварианты конвейера не тронуты — commit `9852871` НЕ
затрагивает `src/stages.py`, `src/qg/checks.py`, `src/db.py`, `src/stage_engine.py`. Маркеры
ORCH-040/088 в `git_worktree`/`main` читаются, зафиксированные инварианты (never-fatal startup,
отложенный срез ветки) не сломаны. `STAGE_TRANSITIONS`/`QG_CHECKS`/`check_*`/machine-verdict/схема
БД — байт-в-байт прежние.
## Качество кода
- `src/fs_normalize.py` — чистый leaf (импортирует только `config`/`logging`/`os`/`time`,
лениво `qg.checks`/`notifications`); строгий never-raise на каждой публичной функции; docstrings
на всех публичных символах; `os.lstat` (не `stat`) для честной оценки симлинков. Зависимость
односторонняя (`git_worktree``fs_normalize`).
- Узкий `_PERM_MARKERS` сознательно не реклассифицирует не-прав-ошибки (защита AC-2).
- Тесты содержательны (214/136/139/68 строк), используют `tmp_path`/monkeypatch, без реального
chown и записи в `/repos`; покрывают обе ветки классификатора, идемпотентность, scope, kill-switch,
TTL-кэш, startup-never-fatal.
- Утечек/секретов/security-дыр не выявлено; chown физически возможен только под root (`_is_privileged`).
## Findings
### P0 — Blocker
- Нет.
### P1 — Must fix
- Нет.
### P2 — Should fix
- Нет.
### P3 — Nice-to-have (не блокирует)
- [ ] `snapshot()` в `GET /queue` на холодном кэше инициирует реальный обход `.git/objects` синхронно
в обработчике запроса. На практике кэш прогрет startup-хуком и TTL=300s, обход только для
self-hosting — латентность пренебрежима, паттерн зеркалит `coverage_gate`. Можно при желании
отдавать в `/queue` только кэш без форс-скана. Информационно.
## Документация
Обновлена в том же PR — golden source синхронен с кодом:
- `docs/operations/INFRA.md` — P-5 (блокер миграции uid) + подраздел процедуры со всеми корнями ✓
- `docs/architecture/README.md` — компонент «FS ownership detect» (D1D3, условность, наблюдаемость) ✓
- `docs/architecture/adr/adr-0031-legacy-ownership-normalization.md` — сквозной ADR (tracked) ✓
- `docs/work-items/ORCH-057/06-adr/ADR-001-…md` — work-item ADR ✓
- `CHANGELOG.md` — запись ORCH-057 ✓
- `.env.example` — 6 флагов `ORCH_FS_*`
`README.md` «Известные ограничения» (ORCH-079): пункт про legacy-ownership/uid-миграцию там
отсутствует — закрывать/снимать нечего, обзорная витрина в обновлении не нуждается.
**Вывод:** изменение `src/` сопровождено обновлением документации → требование правила 6
выполнено.