From 5d4ef9369ee83dc20dfeb6b40ac0e7a9387012cd Mon Sep 17 00:00:00 2001 From: claude-bot Date: Wed, 10 Jun 2026 02:57:31 +0300 Subject: [PATCH] reviewer(ET): auto-commit from reviewer run_id=554 --- docs/work-items/ORCH-057/12-review.md | 105 ++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 docs/work-items/ORCH-057/12-review.md diff --git a/docs/work-items/ORCH-057/12-review.md b/docs/work-items/ORCH-057/12-review.md new file mode 100644 index 0000000..71476a6 --- /dev/null +++ b/docs/work-items/ORCH-057/12-review.md @@ -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`, `/.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 +выполнено.