Files
2026-09-28 22:39:08 +00:00

19 KiB
Raw Permalink Blame History

CODE-REVIEW-700-r4

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

  • Issue: #700, трек track:show, инфраструктурный маршрут (класс B: .github/workflows/**, scripts/**, test/**; класс C: PROCESS.md, docs/reviews/**). Файлов класса A нет.
  • SHA материала: 1aa52d21071705574c6cccb1f19ccc3a8d5b9083 (рабочая копия уже на нём, git status чист).
  • Диапазон: git log --oneline origin/dev..HEAD — 4 коммита: e45bc87c (задача, r0) → a005aae5 (докс r1) → f6e317d8 (задача, правка Medium из r1) → 1aa52d21 (докс r2).
  • git diff origin/dev...HEAD --stat: validate.yml +66/-12, PROCESS.md +7/-1, scripts/check-docs.mjs +12/-1, scripts/mutation-registry.mjs +34, три тестовых файла, два committed документа ревью (CODE-REVIEW-700-r1.md, -r2.md).
  • Validate на этом SHA: success, https://github.com/Matysh/houseplan-card/actions/runs/36492360526 — дешёвые гейты (tsc --noEmit, npm test, npm run build + сверка бандла) подтверждены этим прогоном, повторно не гонял.

Почему разбор полный, а не по дельте (§2.10)

Формально с r2 (доказанно зелёный, docs/reviews/CODE-REVIEW-700-r2.md) код задачи не менялся ни байтом: r3 применил тот же вердикт повторно без вызова модели, потому что дерево a6670a47 совпадало с проверенным. Но между r3 и r4 ветку пришлось перебазировать на ушедший на 11 коммитов вперёд dev (e1700757) — push кандидата рвался токеном конвейера без права workflow (#705), и автор сам пометил: «дифф против dev сменил контекст validate.yml … ожидаю новый заход ревью, а не повтор вердикта r2». Это прямо описанное в задании исключение — «ребейз на ушедший вперёд dev» — где разбор остаётся полным. Сделал полный разбор: перечитал итоговый preflight job целиком (не только хунки диффа), проверил, как правка #700 сочетается с соседними правками dev (#696 — цена захода по треку/мутанты по диффу; #697 — режим скриншотов на ветке задачи), и заново прогнал целевые тесты и три зарегистрированных мутанта.

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

  1. Прочитан весь .github/workflows/validate.yml job preflight в его текущем виде (после ребейза), не только диф-хунки — искал конфликт правки #700 с окружающим контекстом от #696/#697.
  2. Прочитан scripts/check-docs.mjs целиком вокруг флага --external/--external=warn и путь errors/warnings/process.exitCode.
  3. Прочитаны все три тестовых диффа (test/validate-workflow.test.mjs, test/docs-freshness.test.mjs, test/classify-changes.test.mjs) и правка PROCESS.md.
  4. Прогнаны целевые юниты: node --test test/validate-workflow.test.mjs test/docs-freshness.test.mjs test/classify-changes.test.mjs — 52/52 зелёных.
  5. Для каждого из трёх мутантов #700 в scripts/mutation-registry.mjs (task-branch-workflow-sync-red-again, workflow-sync-issue-duplicated-on-read-failure, external-link-warn-mode-ignored) применил патч руками, перезапустил соответствующий --test-name-pattern, убедился, что тест краснеет, откатил файл (git status после — чист). Это не требуется на track:show (мутанты по диффу не запрашиваются), но дёшево и напрямую проверяет дисциплину «тест умеет падать» для тестов, которые я использую как доказательство.
  6. node scripts/smoke-select.mjs --base origin/dev --head HEAD → «Исполняемого frontend-диффа нет (src/**/*.ts не тронут)» — прямое совпадение «нечего выбирать», не решение пропустить.
  7. Сверил трейлеры всех 4 коммитов (git log -1 --format=%B): Issue: #700 на каждом; User-Visible: no на задачных — корректно, изменение не видно продукту; на докс-коммитах трейлеры избыточны, но не вредят.
  8. Сверил, что лейблы infra/process, использованные в новом шаге создания issue, существуют в репозитории (gh label list).

Гейты: что прогнал, что нет, почему

Гейт Статус Почему
npx tsc --noEmit, npm test, npm run build + сверка 3 копий бандла не гонял повторно зелёный Validate на этом же SHA (run 36492360526) уже подтвердил — цена та же, повторный прогон ничего нового не даст (#343)
node --test test/validate-workflow.test.mjs test/docs-freshness.test.mjs test/classify-changes.test.mjs прогнал сам целевые тесты по диффу; 52/52
3 мутанта #700 из mutation-registry.mjs вручную прогнал сам не требуется на show, но дёшево и это единственное прямое доказательство «тест краснеет» на защитных AC
actionlint validate.yml не гонял (инструмент не установлен в среде) синтаксис YAML фактически подтверждён тем, что Validate на этом SHA успешно исполнил job — невалидный YAML GitHub Actions не запустил бы вовсе
node scripts/smoke-select.mjs --base origin/dev --head HEAD прогнал требование прогона по диффу; результат — «нечего выбирать» (нет src/**)
npm run golden:verify не гонял нет метки ci:golden, golden-файлы не тронуты
python -m pytest tests_backend -q не гонял custom_components/**/*.py не тронут
npm run invariants -- --config <export> не гонял геометрия и ссылки на неё не тронуты
performance-профили не гонял не названы в AC
Поведение на push в main / тег релиза (не dev, не issue/*) проверено чтением, не исполнением в среде ревью нет доступа поднять реальный push-событие на main; case "$REF" in refs/heads/issue/*) — единственная новая ветка условия, любой другой ref (включая main/теги) идёт по прежнему строгому пути check()/--external без изменений в этой ветке кода

AC (тело issue, раздел «Предложение», 3 пункта)

AC Чем доказан Чем краснеет
1. На issue/* расхождение зеркала и упавшая внешняя ссылка — предупреждение в сводку, без красного job Автотест test/validate-workflow.test.mjs («#700: на ветке задачи…») читает advise()/check()-ветвление и --external=warn; я применил мутант task-branch-workflow-sync-red-again (advise→check) — тест упал; применил external-link-warn-mode-ignored — тест упал Оба мутанта воспроизведены вручную и убиты; откат подтверждён git status
2. Блокируют только push в dev, кандидат беты, релиз Прочитано по коду: case "$REF" in refs/heads/issue/*) — единственное исключение; check-docs.mjs то же самое условие в validate.yml. Кандидат беты — коммит с Release: на dev (тот же ref, тот же строгий путь), не отдельная ветка Проверено чтением, не исполнением (нет доступа поднять push-событие на main/тег в среде ревью)
3. Расхождение зеркала на dev заводит одно issue, если такого ещё нет (как ночной #472) Шаг «Расхождение зеркала на dev — issue владельцу», if: push && ref==dev && workflow_sync.outcome=='failure'; мутант workflow-sync-issue-duplicated-on-read-failure (снимает фикс r1: exit 0→:) убит — тест #700: на ветке задачи… красный на мутанте, зелёный на исходнике Мутант воспроизведён вручную и убит; логика идентична принятому прецеденту _mutation-gate.yml:317-333 (одно issue, комментарий к открытому)

Все три AC — track show, до трёх штук в теле issue, лимит соблюдён.

Рассмотрено и отклонено как находка

Гонка/ложное срабатывание при сбое git fetch внутри шага workflow_sync. Шаг начинается с git fetch --quiet origin main dev; в GitHub Actions шаги run: по умолчанию исполняются с bash -eo pipefail, поэтому сбой сети на этой строке уронит шаг до цикла diff, и steps.workflow_sync.outcome станет failure без реального расхождения зеркала. После #700 это не просто красит один прогон (как было раньше) — на push в dev это теперь автоматически заводит владельцу постоянный GitHub issue [workflow-sync], который придётся закрывать вручную, хотя расхождения нет.

Не поднимаю как Medium: это тот же дизайн, который уже принят и явно задокументирован в прецеденте, на который ссылается сама задача — _mutation-gate.yml:275 открытым текстом говорит «любой другой пропуск шардов (упал material) по-прежнему заводит issue», то есть владелец уже согласился не различать «содержательный отказ» и «инфраструктурный сбой» ради простоты. Задача #700 не меняет и не ухудшает это поведение — она копирует уже принятый паттерн один в один. Расширять скоуп до различения причин отказа шага — не работа этой задачи.

Гонка двух параллельных push в dev, оба находят issue не открытым и оба создают дубликат. Технически возможно между gh issue list и gh issue create в двух разных прогонах, но concurrency: group: validate-...${{ github.ref }} с cancel-in-progress: true отменяет предыдущий прогон на том же ref при новом push — два прогона preflight для dev одновременно не живут в штатном случае. Не нахожу воспроизводимого сценария в рамках обычного использования; Low, не блокирует.

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

  • advise() не устанавливает fail=1 — предупреждение не красит итоговый вердикт job (проверено чтением и мутантом).
  • check-docs.mjs: при --external=warn отказы внешних ссылок уходят в warnings, а не в errors, значит process.exitCode не выставляется этой причиной — шаг docs не падает от чужого сайта на ветке задачи. Остальные проверки документации (пропущенный alt, отсутствующий якорь, скриншот-манифест и т. д.) остаются в errors независимо от ветки — AC не про них, и они по-прежнему красят.
  • permissions: issues: write добавлено на уровне job, а не workflow — оценил риск: validate.yml не входит в список из шести «тонких» файлов, зеркалируемых в main (process.yml, mutation-gate.yml, process-resume.yml, nightly.yml, process-reconcile.yml, process-metrics.yml), значит новое право не нужно синхронизировать в main — согласуется с тем, что сам список этот файл не включает.
  • Фикс r1 (gh issue list … || true → if ! existing=$(…); then …; exit 0; fi) закрыт и не регрессировал при ребейзе: воспроизведён мутант, тест падает.
  • Трейлеры Issue:/User-Visible: на месте на обоих коммитах класса B; User-Visible: no корректен — изменение невидимо продукту, оба changelog не требуются.
  • Одно число — один источник (§8): в диффе нет пользовательски видимых чисел (внутренний CI-процесс), пункт неприменим.
  • Лейблы infra, process, используемые в gh issue create, существуют в репозитории.

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

  • actionlint сам не гонял (инструмент недоступен в среде ревью) — полагаюсь на успешный прогон Validate на этом SHA как косвенное доказательство валидности YAML.
  • Реальное поведение на push в main и на релизном теге не воспроизводил исполнением — только чтением кода (нет средства поднять такое событие в среде ревью); риск минимален, так как единственная новая ветка условия — issue/*, остальное не тронуто.
  • Полные наборы (golden, HA pytest, инварианты, performance) не гонял — ни один AC их не требует, дифф их не касается.
  • Мутанты по диффу в mutation-gate.yml-смысле («на этом SHA») не запрашивал — не требуется на track:show; три мутанта из реестра, которые сам автор завёл под #700, проверил вручную вместо этого (не обязательное, но дешёвое усиление).

Закрытие раунда r3

r3 не вносил новых находок: он применил зелёный вердикт r2 повторно без вызова модели, потому что дерево a6670a47 было побайтово тем же, что проверялось в r2. Закрывать нечего.

Унаследовано из r3 / переподтверждено в r4

Код задачи (validate.yml, check-docs.mjs, три тестовых файла, mutation-registry.mjs, PROCESS.md) текстуально идентичен тому, что получило зелёный вердикт в docs/reviews/CODE-REVIEW-700-r2.md (SHA материала r2 — ed3d63e2/a6670a47). Несмотря на это, не переносил вердикт по правилу «дерево не изменилось», а провёл полный разбор заново: рабочая копия ребейзнута на 11 коммитов вперёд dev (§2.10, «ребейз на ушедший вперёд dev» — явное исключение из сокращённого объёма), и сам автор указал, что итоговый контекст validate.yml внутри preflight сменился из-за параллельных правок #696/#697. Результат независимого полного разбора совпал с r2: 0 High, 0 Medium.

Итог

High: 0. Medium (в скоупе): 0. Medium (вне скоупа): 0. Low: 1 (гонка двух параллельных push в dev, отклонена как невоспроизводимая при текущем concurrency, не правится и не заводится отдельно). Все три AC доказаны — два исполнением с падающим мутантом, один чтением с явной пометкой «проверено чтением, не исполнением» из-за ограничений среды ревью, а не из-за отсутствия автотеста.

Вердикт: зелёный.


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

  • Ветка: issue/700-preflight-warnings, коммит 1aa52d210717 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: 72952e142f2868e6a1cf473135996b79e5eacb5e
    git log --all --format='%H %T' | grep 72952e142f28
    
  • Тело issue: a0c40d40c1401844c3d04ff0695dd347e76172d990004ea2799cc4df38729fce
  • Вердикт конвейера: green · High 0