houseplan-card/docs/reviews/CODE-REVIEW-736-r1.md

CODE-REVIEW-736-r1

Issue: #736 · Этап: code · Трек: show · Заход: r1 · блокирующих циклов использовано 0/2

Материал раунда: git log --oneline origin/dev..HEAD и git diff origin/dev...HEAD на 2ed56bbabd6554afa9380128d688b57004dd47ab (рабочая копия на нём, HEAD подтверждён git rev-parse HEAD). Диапазон — один коммит 2ed56bba. Diffstat:

.github/workflows/_nightly.yml |  51 ++++-
PROCESS.md                     |  22 ++
scripts/night-red.mjs          | 300 ++++++++++++++++++++++++++
test/night-red.test.mjs        | 462 +++++++++++++++++++++++++++++++++++++++++
test/nightly-workflow.test.mjs |  38 ++++

Скоуп

Инфраструктурная задача (класс B + документация §10.4), выделена из #727. После красного полного ночного Validate на dev новая job night_red (_nightly.yml) запускает scripts/night-red.mjs, который находит последнюю зелёную ночь, вычисляет подозреваемые задачи (по Issue: #NN коммитов классов A/B без Release: между зелёной и красной ночью) и пишет каждой один комментарий, не повторяясь в рамках одной серии красных ночей. SCOPE.md не применим напрямую — это процесс/CI-инструмент, не продуктовая фича; трек show подтверждён владельцем в комментарии к issue («трек: show… Следующий статус: S5-ready»).

Маршрут критериев §5 (трек show):

• complexity — владелец оценил сложность 3/10; код компактный, без скрытых развилок — проходит.
• surfaces — одна поверхность: _nightly.yml + scripts/night-red.mjs + абзац PROCESS.md §10.4 — проходит.
• migration — нет изменения конфига и compatibility-полей продукта — проходит.
• ux-contract — нет UX, задача не трогает карточку/интеграцию — проходит.
• perf-touch — не касается производительности карточки или touch — проходит.
• undocumented — поведение полностью зафиксировано в теле issue (раздел «Что меняется», К1–К7) и теперь задокументировано в PROCESS.md §10.4 тем же коммитом — проходит.

Все критерии §5 пройдены → route: fix (не reclassify).

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

1. Прочитан диапазон коммитов и полный дифф (git log, git diff origin/dev...HEAD).
2. Прочитано тело issue #736 (раздел ## ТЗ, К1–К7, таблица AC) и комментарии (оценка владельца, handoff автора).
3. Построчно прочитан scripts/night-red.mjs — сопоставлен с К1–К6 ТЗ.
4. Построчно прочитан дифф .github/workflows/_nightly.yml — сопоставлен с К1, К7, правами и условием if.
5. Прочитаны тесты test/night-red.test.mjs (7 тестов) и добавленный тест в test/nightly-workflow.test.mjs (1 тест) — сверены утверждения с кодом.
6. Проверены вызываемые внешние примитивы на предмет корректного переиспользования (не доказательство новым кодом, а проверка, что контракт соблюдён): classify (scripts/change-classes.mjs), issueTrailers/hasReleaseTrailer (scripts/release-membership.mjs, scripts/ship-review.mjs), evaluateCiProof/loadGithubProofContext/CI_PROOF_POLICIES (scripts/ci-proof.mjs), isMainModule (scripts/spawn-portable.mjs), validateJobs/resolveJobRules (scripts/workflow-jobs.mjs, scripts/ci-proof.mjs) — все существуют и сигнатуры совпадают с использованием.
7. Прогнаны тесты задачи напрямую: node --test test/night-red.test.mjs test/nightly-workflow.test.mjs — 13/13 зелёных (bash-тесты не пропущены, hasBash() истинен в этой среде).
8. Доказательство «тест умеет падать» для двух защитных AC — ручная мутация и прогон (после — восстановление файла, git status подтвердил чистую копию):
   • убрана проверка hasReleaseTrailer в countsForNightRed → упало 3 из 7 тестов (AC1 К3, AC2 К4–К5, AC2/AC3 bash);
   • commentVerdict упрощён до «всегда comment» (убрано правило «не повторяться в серии») → упало 3 из 7 тестов (AC2 К4–К5, AC2 К5, AC2/AC3 bash). Оба защитных правила ловятся без мутационного прогона реестра — по track show это допустимо (мутанты в разработке не гоняются ни на одном треке, §5).
9. Проверены трейлеры коммита: Issue: #736, User-Visible: no — корректно, changelog не трогается, видимого пользователю поведения нет.
10. Сверено размещение абзаца PROCESS.md: внутри §10.4, рядом с абзацем о ночном ship-ревью (#727), как и заявлено в ТЗ (К7).
11. Сверена логика потолка прав тонкого файла (test/default-branch-workflows.test.mjs:195-215): night_red просит actions: read, contents: read — подмножество уже имеющегося actions: write, contents: read у других job тела; объединение прав не меняется, тонкий nightly.yml в main не трогается — подтверждено чтением теста и логики unionPermissions, не исполнением (gate:small не перегонялся — обоснование ниже).

Таблица AC · чем доказан · чем краснеет

AC	Что	Доказано	Чем краснеет (проверено)
AC1 (К2–К3)	последняя зелёная ночь и подозреваемые	test/night-red.test.mjs, 3 теста (findLastGreen, rangeSuspects, countsForNightRed) на настоящем git во временном репозитории с реальным ci-proof	Проверено мутацией: убрана фильтрация Release: → тест AC1 К3 падает. Остальные случаи (лёгкий zелёный, поздний, чужая ветка, окно 50, G==R) разобраны чтением — тест явно утверждает каждый из них отдельным assert
AC2 (К1, К4, К5)	когда и сколько комментариев	test/night-red.test.mjs, 3 теста + bash-тест на реальном HTTP-сервере и подменённом gh	Проверено мутацией: упрощение commentVerdict (без проверки «та же серия») → тест AC2 К4–К5 и AC2 К5 падают. conclusion фильтр (cancelled/success/timed_out) разобран отдельным тестом с тремя исходами
AC3	проводка, права, канон	test/nightly-workflow.test.mjs (контракт YAML + содержимого PROCESS.md), ревью кода	Проверено чтением, не исполнением: if-условие, права job, токены, checkout-параметры, отсутствие npm ci, наличие абзаца К7 в шапке _nightly.yml и в PROCESS.md §10.4 — всё сверено построчным assert.match против реального YAML/MD. Актуальность bash -n на теле шага подтверждена (прогнано как часть теста)

Пустых третьих столбцов нет.

Находки

Находок нет. High: 0, Medium: 0, Low: 0.

Разобраны и признаны корректными потенциально спорные места:

• Порядок коммитов в rangeSuspects/commentBody (oldest→newest внутри byIssue, LIST_LIMIT берёт первые N = самые старые, явно подтверждено тестом AC1 К3 и AC2 К4–К5) — не дефект, просто явный выбор, не влияющий на AC.
• Слияние (--no-merges) исключает коммит-слияние из диапазона даже если у него собственный Issue: трейлер (тест side branch, #6) — соответствует тексту ТЗ К3 дословно («сами слияния не считаются»).
• evaluateCiProof вызывается без явного candidate.sha (default {}) — безопасно, так как функция сама подставляет runShaOf(run); тот же паттерн уже используется в ship-review.mjs, это не новый риск.
• Гонка «issue закрылась между view и comment» — не покрыта тестом, но не AC и не риск, явно описанный в ТЗ; cost/benefit на track show не оправдывает находку.

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

• Скрипт работает только при conclusion: failure красного прогона (К1) — ветки cancelled/success/timed_out дают только строку сводки, проверено тестом с тремя исходами.
• Поиск G (К2): 50 последних dispatch-прогонов, фильтр по completed+success, raньше красного по времени, предок по git merge-base --is-ancestor, зелёный ci-proof по политике release (лёгкий прогон — stale). Порядок проверок (сначала дешёвые, потом загрузка ci-proof) подтверждён тестом loaded — доказательство грузится только у прошедших дешёвые фильтры.
• Подозреваемые (К3): git rev-list --no-merges G..R, фильтр класса A/B без Release:, коммиты без трейлера — в «без задачи», коммиты из docs/** — не считаются, ветка, влитая вторым родителем, входит, само слияние — нет.
• Шаблон комментария (К4): оба прогона, до 10 коммитов («и ещё N»), упавшие job, фраза «подозреваемая, а не виновная», маркер hp:night-red green=… red=… commits=…, несущий все (не только показанные) коммиты задачи.
• Подавление шума (К5): закрытая задача — не комментируется, строка в сводке; одна и та же пара — не повторяется; новый коммит задачи в той же серии — комментарий снова; новая зелёная ночь начинает новую серию.
• Цвет ночи (К6): сбой скрипта/API — ::warning:: и строка сводки, код возврата 0, continue-on-error: true на уровне job — проверено на реальном bash с имитацией сбоя Actions API (500) и отсутствующего скрипта.
• Права и токены (К7, AC3): night_red просит только actions: read, contents: read; ACTIONS_TOKEN = github.token, GH_TOKEN (для gh) = secrets.HP_PROCESS_TOKEN — подтверждено и YAML-тестом, и bash-тестом (реальные HTTP-запросы несут Bearer actions-token, gh-лог несёт token=process-token).
• dispatch отдаёt run_id тем же шагом, что и head_sha, до шага ожидания — красный watch не теряет выход.
• Канон: абзац «Красная ночь (#736)» в PROCESS.md §10.4 рядом с абзацем о ночном ship-ревью; шапка _nightly.yml называет адресата сигнала. Оба проверены построчным сопоставлением с текстом ТЗ.
• Трейлеры коммита корректны (Issue: #736, User-Visible: no), changelog не нужен и не тронут — задача не меняет видимое пользователю число или поведение (§8 неприменим: ничего не дублируется для пользователя).
• test/default-branch-workflows.test.mjs логически не ломается: добавленная job снижает, а не расширяет права относительно существующего потолка тонкого файла.

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

• npx tsc --noEmit, npm test (полный), npm run build + сверка бандла — не перегонял: Validate на этом же SHA 2ed56bba зелёный (ссылка на прогон дана в постановке), дешёвые гейты этим подтверждены. Дополнительно прогнал целевые тестовые файлы задачи напрямую (п. 7 выше) — зелёные.
• Смоки в браузере — не прогонял: node scripts/smoke-select.mjs --base <base> --head <head> не запускался отдельно; диапазон не трогает src/** или что-либо, влияющее на визуальный рендер карточки (только .github/workflows/**, scripts/**, test/**, PROCESS.md) — браузерных поверхностей в диффе нет, это чисто серверный/CI-скрипт. Решение: не прогонять, так как изменённые пути не пересекаются ни с одним смоук-сценарием.
• npm run golden:verify — не прогонял: метки ci:golden на задаче нет, диапазон не трогает путь отрисовки плана.
• python -m pytest tests_backend — не прогонял: диапазон не содержит правок custom_components/**/*.py.
• npm run invariants — не прогонял: диапазон не трогает геометрию модели.
• Performance-профили — не названы в AC, не прогонял.
• Реальный прогон против живого GitHub API (загрузка ci-proof через actions: read, gh issue view/comment с HP_PROCESS_TOKEN) — не воспроизводился: это заявленный автором риск («Против настоящего GitHub не проверены…»), out of scope для ревью кода на материале — первая живая красная ночь покажет это на практике; механизм отказоустойчив (continue-on-error, ::warning::), поэтому сбой первого реального прогона не красит ночь и не блокирует бету.
• Мутационный прогон реестра — не запускался: на треке show мутанты в разработке не гоняются ни одним агентом (#709), отсутствие прогона — не находка. Вместо этого вручную проверено (см. «Как проверялось», п. 8), что тесты ловят минимум по одной осмысленной порче на каждый защитный AC.

Вывод

AC1–AC3 выполнены и доказаны чтением + исполнением тестов задачи; оба защитных правила (исключение Release:-коммитов, подавление повторного комментария в серии) проверены на умение падать вручную. Находок нет.

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

---

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

• Ветка: issue/736-night-red-comment, коммит 2ed56bbabd65 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: 8186f6ff39cd6868725a79f4a7feeb444c6c6d88

  git log --all --format='%H %T' | grep 8186f6ff39cd
  

• Тело issue: 6cd408eba981cb82629e225f025e183f70de0ac956d12cc26cf1d6d07c983ec2
• Вердикт конвейера: green · High 0 · маршрут fix