Files
2026-09-30 20:30:34 +00:00

13 KiB
Raw Permalink Blame History

CODE-REVIEW #704 · заход r1

Материал: dd61036b5af84e5dd190803d1c3dce74fa51b5a0 (git log --oneline origin/dev..HEAD — один коммит; git diff origin/dev...HEAD — 5 файлов, +231/-5). Трек: show. Блокирующих циклов использовано: 0 из 2.

Скоуп

Issue #704: release.yml ставил независимое ревью stable-линии в очередь токеном GITHUB_TOKEN, прогон стартовал от github-actions[bot], и claude-code-action отказывал боту без allowed_bots. Родительский workflow при этом не отличал «dispatch принят» от «ревью реально стартовало».

ТЗ — три AC:

  • AC1 — allowed_bots разрешает ровно ожидаемого бота, не '*'.
  • AC2 — родительский workflow находит прогон и пишет в сводку ссылку и статус («запущено» / «не стартовало за N минут»); выпуск не блокируется.
  • AC3 — unit-тест конфигурации, который ломается при удалении строки.

Изменённые файлы: .github/workflows/release-review.yml, .github/workflows/release.yml, PROCESS.md, test/release-review.test.mjs, test/release-workflow.test.mjs. Продуктовый код (src/**) не тронут — User-Visible: no в трейлере коммита корректен, правка обоих CHANGELOG не требовалась и не делалась.

Как проверялось

Дешёвые гейты этого SHA уже подтверждены зелёным Validate (https://github.com/Matysh/houseplan-card/actions/runs/36771523717) — npx tsc --noEmit, npm test, npm run build повторно не гонялись.

Прогнано в этом раунде:

  1. Реальное исполнение новых тестов. node --test test/release-review.test.mjs test/release-workflow.test.mjs — 20/20 зелёных (0 пропущено: hasTools() подтвердил наличие bash+jq, значит три новых AC2-теста реально исполнили bash-скрипт шага, а не пропустили себя).
  2. Мутационная проверка «тест умеет падать» (вручную, с восстановлением файлов из копии и сверкой git status/git diff --stat = пусто после):
    • удалена строка allowed_bots: "github-actions[bot]" из release-review.yml → test/release-review.test.mjs покраснел ровно на ожидаемой проверке («allowed_bots на месте…»), остальные 8 тестов файла прошли. AC1/AC3 доказаны исполняемым тестом, не только чтением.
    • в release.yml started=true расширено на queued (симулирован дефект: шаг посчитал бы очередь «стартом») → 2 из 3 новых AC2-тестов в test/release-workflow.test.mjs покраснели («прогон найден и стартовал» и «прогон в очереди — предупреждение»). AC2 доказан исполняемым тестом, не только чтением.
  3. Проверка ключевого технического утверждения AC1 (не декларация автора, а сверка с источником). Автор утверждает: isAllowedBot в claude-code-action на пиннутом SHA 9cdae7f0d995e3ba7c33f226087fdf82a59cd520 сравнивает регистронезависимо и без суффикса [bot]. Получен исходник src/github/validation/actor.ts и action.yml с этого SHA напрямую из GitHub (gh api repos/anthropics/claude-code-action/contents/...?ref=9cdae7f0...):
    • isAllowedBot(actor, allowedBots): trimmed === '*' → разрешить всех; иначе список allowedBots.split(',').map(s => s.trim().toLowerCase().replace(/\[bot\]$/, '')), сравнение с actor.toLowerCase().replace(/\[bot\]$/, ''). Подтверждено дословно.
    • checkHumanActor: allowed_bots консультируется только когда actorType !== 'User' (человек список не проходит вообще, как и заявлено в ТЗ/issue-комментарии).
    • action.yml: вход allowed_bots существует, тип string, default "". Вывод: значение "github-actions[bot]" в диффе корректно совпадает с актёром github-actions[bot] (оба нормализуются к github-actions), любой другой бот по-прежнему отклоняется, человек список не консультирует. AC1 верен по факту, не только по описанию автора.
  4. node scripts/smoke-select.mjs --base origin/dev --head HEAD → «Исполняемого frontend-диффа нет… Browser-smoke этим диффом не выбираются — выбирать нечего». Подтверждено: src/**/*.ts не тронут.
  5. Прочитаны неизменённые куски контекста: PROCESS.md §11.5 (описание пайплайна ревью линии, дифф синхронен с кодом), §10.4 (список из шести файлов, требующих немедленного зеркала в main; release.yml/release-review.yml в этот список не входят — их собственные комментарии в файле объясняют, почему зеркало не нужно: release-review.yml дергается workflow_dispatch --ref dev, release.yml на событии release читается с коммита тега, а тег для стабильного всегда ставится с вершины main, куда дойдёт обычным промоушеном dev→main перед разрезкой тега — не специальный ручной шаг).

Что проверено и корректно

  • AC1. allowed_bots: "github-actions[bot]" — не '*', сравнение подтверждено по исходнику экшена (см. выше). Комментарий в release-review.yml (7 строк) объясняет контекст и ссылается на #704 и наблюдавшуюся ошибку v1.78.0.
  • AC2. Логика independent-review в release.yml переписана: dispatch → опрос gh run list (окно 180 с, шаг 15 с, что укладывается в timeout-minutes: 5 job — проверено тестом #704 AC2: ожидание прогона укладывается в бюджет job) → четыре различимых исхода, каждый со своей строкой в $GITHUB_STEP_SUMMARY и, где уместно, ::warning::: dispatch отклонён (exit 1, как раньше) / прогон не появился за N мин (exit 0) / прогон завис в очереди весь таймаут (exit 0, ссылка в предупреждении) / прогон стартовал (exit 0, ссылка и статус, доп. warning при conclusion: failure). Фильтр по displayTitle == "Release review $TAG" и createdAt >= since (с запасом в 60 с до dispatch) корректно отсекает старый прогон того же тега — проверено тестом (older с createdAt: iso(-3600) не попадает в сводку) и логически совпадает с run-name: "Release review ${{ inputs.tag }}" в release-review.yml.
  • AC3. Два новых теста в test/release-review.test.mjs: точное значение и отсутствие '*' для шага Review, плюс общий инвариант «ни один workflow не пускает allowed_bots: '*'» по всем файлам .github/workflows/*.yml. Оба упали при целевой мутации (см. выше).
  • Документация. PROCESS.md §11.5 получил абзац, синхронный с кодом (токен dispatch, allowlist, диагностика в сводке, окно в три минуты, ссылка на #704).
  • Обратная совместимость с #638. Существующий тест #638 AC2: ревью линии ставится в очередь параллельно гейтам и ни один job выпуска его не ждёт по-прежнему зелёный — новая логика не добавила needs на independent-review и не изменила это свойство.
  • Трейлеры. Issue: #704, User-Visible: no — оба корректны, продуктовый код и видимое поведение карточки не затронуты.

Находки

Нет. High — 0, Medium — 0, Low — 0.

Отдельно проверена и отклонена как находка гипотеза «правка release.yml не подействует на следующий stable-релиз без ручного зеркала в main»: §10.4 ограничивает обязательное немедленное зеркалирование шестью конкретными файлами для событий без привязанного ref (issues, schedule, workflow_run); release.yml/release-review.yml в этот список не входят, и release-событие по документированному (и ранее воспроизведённому на v1.78.0 — сам факт, что independent-review вообще выполнился и дошёл до dispatch, подтверждает рабочий путь) поведению читается с коммита тега, который для стабильного релиза всегда ставится с вершины main — то есть обычный промоушен dev → main перед разрезкой тега донесёт этот коммит без отдельного действия.

Чего не проверял

  • Живой прогон claude-code-action от настоящего github-actions[bot] на реальном GitHub — недоказуемо локально; автор указывает это сам как риск. Компенсировано сверкой с исходником экшена на пиннутом SHA (п. 3 выше), что закрывает основной риск «а вдруг сравнение работает иначе».
  • Полный npm run gate:small / tsc / npm test (весь набор) / npm run build — не перегонялись, дешёвые гейты уже зелёные на этом SHA (Validate run 36771523717); прогнаны точечно только два новых тестовых файла.
  • npm run golden:verify, python -m pytest tests_backend, npm run invariants — неприменимы: диффом не затронуты ни рендер, ни custom_components/**/*.py, ни геометрия/ссылки на неё.
  • Поведение при реальном force=true повторном дергании на тот же тег во время активного concurrency-окна release-review-${{ inputs.tag }} (когда прогон реально застревает в queued из-за конкурентности, а не из-за отказа) — логически покрыто веткой «в очереди весь таймаут» и тестом на status: queued, но не воспроизведено на реальном раннере.

Вердикт

Зелёный. AC1–AC3 выполнены и доказаны исполняемыми тестами с подтверждённой способностью падать; ключевое техническое утверждение автора (поведение isAllowedBot) проверено по первоисточнику, а не принято на слово. Находок нет.


Материал раунда

  • Ветка: issue/704-stable-review-bot, коммит dd61036b5af8 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: 888e656fc22851a25640bde167f450fc8b8934d3
    git log --all --format='%H %T' | grep 888e656fc228
    
  • Тело issue: c7051f4ce2ca96216b857b0aeb6871cfb32669ec3def1e3b7dcacee82f8979d9
  • Вердикт конвейера: green · High 0