106 lines
7.3 KiB
Markdown
106 lines
7.3 KiB
Markdown
---
|
||
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 D1–D6 (включая сознательный отказ от блокирующего 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» (D1–D3, условность, наблюдаемость) ✓
|
||
- `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
|
||
выполнено.
|