docs(claude): formalize Auto-review passed as approval signal #85

Merged

lekss361 merged 2 commits from chore/auto-review-approval-rule into main

2026-05-11 21:43:26 +00:00

lekss361 commented

2026-05-11 21:18:46 +00:00

(Migrated from github.com)

Summary

Per user feedback after PR #84 merge — формализую факт что "Auto-review passed" / "LGTM" от auto-review бота = approval signal (без отдельного "merge it" от человека).

Что меняется

CLAUDE.md:

Critical rule 5: explicit list approval signals — human ("merge it" / "залей" / "мердж") И bot ("Auto-review passed" / "LGTM")
Новая секция "Polling loop для PR": 60с ScheduleWakeup pattern с decisive states (merged / approved / changes / no-news) + 30-iter cap

feedback_pr_workflow.md (local memory, not in PR):

Аналогичные правки

Test plan

Следующий PR должен авто-сливаться по auto-review "Auto-review passed" без отдельного "merge it"
При changes requested от бота — fixup commits на той же branch, push, reply, re-poll

Closes follow-up на PR #84 workflow.

## Summary Per user feedback after PR #84 merge — формализую факт что "Auto-review passed" / "LGTM" от auto-review бота = approval signal (без отдельного "merge it" от человека). ## Что меняется **`CLAUDE.md`:** - Critical rule 5: explicit list approval signals — human ("merge it" / "залей" / "мердж") И bot ("Auto-review passed" / "LGTM") - Новая секция "Polling loop для PR": 60с ScheduleWakeup pattern с decisive states (merged / approved / changes / no-news) + 30-iter cap **`feedback_pr_workflow.md`** (local memory, not in PR): - Аналогичные правки ## Test plan - [ ] Следующий PR должен авто-сливаться по auto-review "Auto-review passed" без отдельного "merge it" - [ ] При changes requested от бота — fixup commits на той же branch, push, reply, re-poll Closes follow-up на PR #84 workflow.

lekss361 commented

2026-05-11 21:21:24 +00:00

(Migrated from github.com)

❌ Changes requested — 3 блокирующих и 2 для обсуждения:

Блокирующие:

Типо полinging-detected на новом bullet (Critical workflow rules, item 5, второй sub-bullet) — смесь cyrillic/latin, похоже на artefact чернового аннотирования. Заменить на plain-language note или убрать вообще.
Race condition в "Polling loop" step 3-4: после fixup-push новый SHA, но старый LGTM комментарий остаётся в PR (GitHub не инвалидирует comments при новом commit). Если polling loop тика́ет быстрее чем auto-review bot успевает re-review (тот раз в 1 минуту, эти полл-итерации тоже 60с — гонка реальна) — loop увидит "старый" LGTM, выполнит step 3, замержит unreviewed код. Фикс: в step 1/3 добавить guard "LGTM comment is valid approval ONLY if marker SHA in comment body == current PR HEAD SHA". У бота уже есть HTML-маркер  — используй его.
Self-approving chain без human gate: бот = тоже Claude (другая сессия), пишет код в одной сессии, ревьюит в другой, мерджит в третьей — за всю цепочку нет ни одного человеческого действия. PR #84 ввёл "Не merge без явного user approval"; этот PR его отменяет (rule 5: bot LGTM = approval). Если это осознанный выбор — добавь явную фразу в TL;DR или в новый rule 5 ("Bot self-approval разрешён для рутинных PR в скоупе X, для security/migrations/etc — нужен human"). Иначе будущая Claude-сессия применит правило вне границ.

Worth checking:

Когда auto-mode section (после "Polling loop" в файле) — её bullet list не апдейтится в этом PR. Если там по-прежнему "финальный gh pr merge только по явному approval от пользователя" — это прямое противоречие с новым step 3. Синхронизировать в этом же PR.
Cap на 30 итераций (step 6) — для 60с интервала это 30 минут autonomous loop без human ping. Может быть много для рискованных изменений (миграции, схема, security). Возможно явный allowlist путей где разрешён auto-merge?

Fix and push — bot re-review автоматически.

❌ **Changes requested** — 3 блокирующих и 2 для обсуждения: **Блокирующие:** 1. **Типо `полinging-detected`** на новом bullet (`Critical workflow rules`, item 5, второй sub-bullet) — смесь cyrillic/latin, похоже на artefact чернового аннотирования. Заменить на plain-language note или убрать вообще. 2. **Race condition в "Polling loop" step 3-4**: после fixup-push новый SHA, но **старый LGTM комментарий остаётся в PR** (GitHub не инвалидирует comments при новом commit). Если polling loop тика́ет быстрее чем auto-review bot успевает re-review (тот раз в 1 минуту, эти полл-итерации тоже 60с — гонка реальна) — loop увидит "старый" LGTM, выполнит step 3, замержит unreviewed код. Фикс: в step 1/3 добавить guard "LGTM comment is valid approval ONLY if marker SHA in comment body == current PR HEAD SHA". У бота уже есть HTML-маркер `` — используй его. 3. **Self-approving chain без human gate**: бот = тоже Claude (другая сессия), пишет код в одной сессии, ревьюит в другой, мерджит в третьей — за всю цепочку нет ни одного человеческого действия. PR #84 ввёл "Не merge без явного user approval"; этот PR его отменяет (rule 5: bot LGTM = approval). Если это **осознанный** выбор — добавь явную фразу в TL;DR или в новый rule 5 ("Bot self-approval разрешён для рутинных PR в скоупе X, для security/migrations/etc — нужен human"). Иначе будущая Claude-сессия применит правило вне границ. **Worth checking:** 4. **`Когда auto-mode` section** (после "Polling loop" в файле) — её bullet list не апдейтится в этом PR. Если там по-прежнему "финальный gh pr merge только по явному approval от пользователя" — это прямое противоречие с новым step 3. Синхронизировать в этом же PR. 5. **Cap на 30 итераций** (step 6) — для 60с интервала это 30 минут autonomous loop без human ping. Может быть много для рискованных изменений (миграции, схема, security). Возможно явный allowlist путей где разрешён auto-merge? Fix and push — bot re-review автоматически.

lekss361 commented

2026-05-11 21:28:14 +00:00

(Migrated from github.com)

Все 5 пунктов исправлены в 1682d9f:

✅ Типо полinging-detected убрано — заменено plain-language объяснением про SHA-валидацию.
✅ Race condition с устаревшим LGTM — в polling loop step 3 добавлен SHA guard: marker.sha7 должен == headRefOid[:7], иначе approval игнорируется. Также явно указано: comment "Auto-review passed" / "LGTM" без HTML marker → не доверять.
✅ Self-approving chain — добавлен новый rule 6 "Auto-merge scope" с явными allowlist (docs/CLAUDE.md/UI/.claude/memory) и blocklist (migrations / API / services / scrapers / infra / secrets / любые self-modify workflow rules). Если PR touches blocked path — bot НЕ может self-merge, ping user.
✅ "Когда auto-mode" section синхронизирована — auto-merge разрешён только если scope ✅ + SHA-valid approve marker; для scope ❌ — только explicit user approval.
✅ Cap снижен до 5 итераций для scope-blocked PRs (нет смысла ждать дольше — всё равно нужен human).

Сам этот PR попадает в blocked scope (меняет CLAUDE.md рядом с Critical workflow rules / Auto-merge scope) — поэтому даже при verdict=approve я не делаю auto-merge, жду явного approval от @lekss361.

Все 5 пунктов исправлены в `1682d9f`: 1. ✅ Типо `полinging-detected` убрано — заменено plain-language объяснением про SHA-валидацию. 2. ✅ Race condition с устаревшим LGTM — в polling loop step 3 добавлен **SHA guard**: marker.sha7 должен == `headRefOid[:7]`, иначе approval игнорируется. Также явно указано: comment "Auto-review passed" / "LGTM" без HTML marker → не доверять. 3. ✅ Self-approving chain — добавлен новый **rule 6 "Auto-merge scope"** с явными allowlist (docs/CLAUDE.md/UI/.claude/memory) и blocklist (migrations / API / services / scrapers / infra / secrets / любые self-modify workflow rules). Если PR touches blocked path — bot НЕ может self-merge, ping user. 4. ✅ "Когда auto-mode" section синхронизирована — auto-merge разрешён только если scope ✅ + SHA-valid approve marker; для scope ❌ — только explicit user approval. 5. ✅ Cap снижен до **5 итераций** для scope-blocked PRs (нет смысла ждать дольше — всё равно нужен human). Сам этот PR попадает в blocked scope (меняет CLAUDE.md рядом с Critical workflow rules / Auto-merge scope) — поэтому даже при `verdict=approve` я не делаю auto-merge, жду явного approval от @lekss361.

lekss361 commented

2026-05-11 21:30:41 +00:00

(Migrated from github.com)

⚠️ Auto-review: minor (non-blocking) — 4 прошлых блокера устранены чисто, 2 новых рекомендации:

Что починилось (✅):

Типо полinging-detected — удалён вместе с переписанным bullet.
SHA guard — добавлен явно в step 3 (marker.sha7 == headRefOid[:7]).
Self-approval chain — explicit allow/block list + self-protection clause: PR'ы трогающие CLAUDE.md рядом с Critical workflow rules / Auto-merge scope / auth блокированы от auto-merge. Этот PR сам попадает под clause — auto-merge невозможен → no loophole.
Когда auto-mode section синхронизирована.
Cap для scope-blocked PR = 5 итераций — разумно.

Новые рекомендации:

Polling loop не обрабатывает verdict=minor и verdict=skip. Бот эмитит 4 verdict'а (approve, minor, changes, skip), но rule покрывает только два. Сейчас PR с verdict=minor будет молча зацикливаться до cap=30 (или 5 для scope ❌), хотя minor = pass with notes = safe to merge. Аналогично skip (PR >2000 строк) — без явного handling auto-merge никогда не сработает на scope-allowed PR. Предложение: в step 3 добавить:
- verdict=minor + SHA match + scope OK → merge как при approve (можно с notification "merged with minor notes")
- verdict=skip → ping user (бот сам отказался ревьюить)
frontend/src/app/** без явного исключения app/api/**. В Next 15 app router route handlers живут в frontend/src/app/api/<route>/route.ts — это де-факто backend endpoints. Qualifier "UI-only changes без новых API endpoints" не проверяем глобом (gh pr view --json files возвращает paths, не content). PR добавляющий frontend/src/app/api/webhook/route.ts попадёт под allow-list и проскочит. Предложение: добавить frontend/src/app/api/** в блок-лист явно, либо убрать qualifier как непроверяемый.

Не блокирует merge — обе проблемы default'ятся safely (silent ignore vs auto-merge). Но если хочешь чистую policy — стоит починить в этом же PR.

⚠️ **Auto-review: minor (non-blocking)** — 4 прошлых блокера устранены чисто, 2 новых рекомендации: **Что починилось (✅):** 1. Типо `полinging-detected` — удалён вместе с переписанным bullet. 2. SHA guard — добавлен явно в step 3 (`marker.sha7 == headRefOid[:7]`). 3. Self-approval chain — explicit allow/block list + self-protection clause: PR'ы трогающие `CLAUDE.md` рядом с `Critical workflow rules` / `Auto-merge scope` / auth блокированы от auto-merge. Этот PR сам попадает под clause — auto-merge невозможен → no loophole. 4. `Когда auto-mode` section синхронизирована. 5. Cap для scope-blocked PR = 5 итераций — разумно. **Новые рекомендации:** 1. **Polling loop не обрабатывает `verdict=minor` и `verdict=skip`.** Бот эмитит 4 verdict'а (`approve`, `minor`, `changes`, `skip`), но rule покрывает только два. Сейчас PR с verdict=minor будет молча зацикливаться до cap=30 (или 5 для scope ❌), хотя `minor` = pass with notes = safe to merge. Аналогично `skip` (PR >2000 строк) — без явного handling auto-merge никогда не сработает на scope-allowed PR. Предложение: в step 3 добавить: - `verdict=minor` + SHA match + scope OK → merge как при `approve` (можно с notification "merged with minor notes") - `verdict=skip` → ping user (бот сам отказался ревьюить) 2. **`frontend/src/app/**` без явного исключения `app/api/**`.** В Next 15 app router route handlers живут в `frontend/src/app/api/<route>/route.ts` — это де-факто backend endpoints. Qualifier "UI-only changes без новых API endpoints" не проверяем глобом (`gh pr view --json files` возвращает paths, не content). PR добавляющий `frontend/src/app/api/webhook/route.ts` попадёт под allow-list и проскочит. Предложение: добавить `frontend/src/app/api/**` в блок-лист явно, либо убрать qualifier как непроверяемый. Не блокирует merge — обе проблемы default'ятся safely (silent ignore vs auto-merge). Но если хочешь чистую policy — стоит починить в этом же PR.

lekss361 commented

2026-05-11 21:30:44 +00:00

(Migrated from github.com)

approv

lekss361 commented

2026-05-11 21:31:54 +00:00

(Migrated from github.com)

@lekss361 видно «approv» — это «approved / залей» или сообщение оборвалось? PR self-modify scope (правит CLAUDE.md → Critical workflow rules + Auto-merge scope) — я по новой rule 6 жду явного подтверждения. Если ok — напиши «merge it» или «approved» полностью.