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

CODE-REVIEW-705-r1

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

• Issue: #705 · трек show (инфраструктура, вход сразу в S7-code-review)
• SHA материала: c17c05341bc503f1cf2aa85684a23c0f6afeaeeb (рабочая копия уже на нём)
• git log --oneline origin/dev..HEAD: один коммит — c17c0534 fix(process): a GitHub push refusal is not a stale lease (#705)
• Ветка к dev не приводилась (трек show, #696): dev впереди на 10 коммитов, слияние чистое — это подтверждено конвейером и не находка.
• Validate на этом SHA зелёный: https://github.com/Matysh/houseplan-card/actions/runs/36771211279 (npx tsc --noEmit, npm test, npm run build + сверка копий бандла на этом прогоне не перегонялись — приняты по ссылке).

Скоуп

Задача чинит инцидент #700: pushWithLease/deleteBranch в scripts/merge-candidate.mjs считали устаревшим lease любой push-отказ, содержащий слово rejected, включая ! [remote rejected] — отказ самого GitHub (например, кандидату, меняющему .github/workflows/, от токена без права workflow). Диффу соответствуют три файла кода (scripts/merge-candidate.mjs, .github/workflows/_process.yml, scripts/mutation-registry.mjs) и два тестовых (test/merge-candidate.test.mjs, test/rebase-generated.test.mjs), плюс абзац в PROCESS.md §4.2. Работа инфраструктурная (класс B), под J-строку docs/SCOPE.md не подпадает — верно, инфраструктура конвейера ревью в скоуп продукта не входит и рецензируется по AGENTS.md/PROCESS.md, а не по docs/SCOPE.md.

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

Прочитан целиком дифф git diff origin/dev...HEAD (все 6 файлов), тело issue #705 и единственный комментарий автора («Взял и сделал»). Код разобран построчно: classifyPushRefusal, redactSecrets, PushRefusal, decideMerge с новым полем s.refused, обёртка mergeCandidate → mergeAttempts, изменения в realOps.pushWithLease/deleteBranch, новый CLI-режим --push-refusal= (describePushRefusal/pushRefusalMain), правки .github/workflows/_process.yml (шаг «Привести ветку к dev» и новый шаг возврата автору), запись в scripts/mutation-registry.mjs, абзац PROCESS.md §4.2.

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

Гейт	Команда	Результат
Целевые unit-тесты диффа	node --test test/merge-candidate.test.mjs test/rebase-generated.test.mjs	52/52 зелёные
Регресс канона ревью	node --test test/process-digests.test.mjs	5/5 зелёные
Реестр мутантов синтаксически цел	node scripts/mutation-registry-check.mjs	exit 0, без вывода
Отбор браузерных смоков	node scripts/smoke-select.mjs --base origin/dev --head HEAD	«Исполняемого frontend-диффа нет … смоки не выбираются» — не гейт этой задачи
Защитный AC3 — мутант push-refusal-kinds-glued	вручную применил патч мутанта из реестра к scripts/merge-candidate.mjs, прогнал node --test --test-name-pattern="#705 AC1" test/merge-candidate.test.mjs, затем восстановил файл из git diff/резервной копии	красный — 2 из 3 тестов паттерна #705 AC1 падают (git push dev отклонён — stale: вместо stale (stale info):), после восстановления файла — снова 52/52 зелёные

Не прогонялось и почему: npx tsc --noEmit, npm test (полный), npm run build

• сверка бандла — уже зелёные на этом SHA в Validate (ссылка выше), диффу они не нужны повторно (#343). golden:verify — нет метки ci:golden и диффа в src/**/demo/**. pytest tests_backend — Python не тронут. Инварианты модели (npm run invariants) — геометрия не тронута. Performance-профили — в AC не названы. Полный mutation-gate --check (весь реестр) — не гейт ревью, мутанты в разработке не гоняются (#709), ночной прогон отдельно; проверен только точечно мутант этой задачи, как того требует защитный AC.

§8: число, видимое пользователю

Диффу не принадлежит ни одна продуктовая/пользовательская величина — правка инфраструктурная, User-Visible: no (см. ниже). В коде есть два совпадающих магических литерала 1500: новый PUSH_STDERR_LIMIT (обрезка stderr для классификации отказа) и уже существовавшая до #705 .slice(0, 1500) в generic-обработчике #492 (сбой шага слияния) — они относятся к разным путям и разным текстам, случайное совпадение значения, не общий источник факта. Это не дублирование пользовательского числа в смысле §8 (не то же самое значение, показанное дважды из двух вычислений), поэтому не отмечаю находкой — но называю прямо, раз диффом введён второй такой литерал.

Находки

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

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

• AC1. classifyPushRefusal (scripts/merge-candidate.mjs:102-122) различает три исхода по тексту stderr:
  • устаревший lease — ! [rejected] … (stale info|fetch first|non-fast-forward) и гонка lease на сервере (cannot lock ref … but expected, incorrect old value provided) — трактуется как тот же stale;
  • отказ по праву на workflow — регэксп WORKFLOW_REFUSAL покрывает classic PAT, fine-grained PAT, OAuth App, GitHub App/GITHUB_TOKEN, прежние формы bot/integration — проверено тестом на каждую формулировку (test/merge-candidate.test.mjs, блок WORKFLOW_REFUSALS);
  • прочий [remote rejected] — pre-receive/protected-branch хуки и подобное. redactSecrets вырезает userinfo в URL, ghp_/gho_/ghu_/ghs_/ghr_/github_pat_, заголовок Authorization и явно переданные секреты (split/join до regex — защита от новых форматов токенов) — проверено чтением и тестом с комбинацией всех типов секретов в одном тексте, включая частичное совпадение внутри URL. stderr печатается в журнал (log() внутри refusedPush) — раньше не печатался вовсе (сам баг #700).
• AC2. decideMerge получил s.refused и отображает его в push-refused-workflow/push-refused до проверки conflict — корректно: отказ GitHub — не «ветка изменилась» (#312) и не конфликт ребейза. mergeCandidate — новая тонкая обёртка над переименованным mergeAttempts: ловит PushRefusal, брошенный из realOps.pushWithLease на любом из трёх мест, где он вызывается (fast-forward в dev, публикация кандидата в ветку задачи, финальный push кандидата в dev), и вместо падения шага комментирует issue и возвращает S6-in-progress, не выполняя deleteBranch — веткa не помечается влитой, потому что не влита. Прочертил все три места вызова pushWithLease в mergeAttempts (строки ~440, ~456, ~483) — исключение из любого перехватывается одним и тем же catch. Комментарий push-refused-workflow содержит дословную фразу ТЗ («кандидат меняет workflow-файл, токен конвейера не может его опубликовать: ребейз и push делает автор, либо владелец выдаёт право»), имя файла и ответ GitHub — подтверждено тестами AC2 построчным assert.match. Страж ребейза в .github/workflows/_process.yml (шаг «Привести ветку к dev») разбирает stderr push тем же кодом — node scripts/merge-candidate.mjs --push-refusal=… — а не своим regex; новый шаг «Push ребейза отклонён по праву на workflow» переводит задачу в S6-in-progress с комментарием и не расходует цикл ревью (код не читался). Шаги «Зафиксировать SHA материала», «Зелёный вердикт применим без ревью» и «Validate на материале» корректно пропускаются доп. условием steps.rebase.outputs.refused == '' — проверено и юнит-тестом (test/rebase-generated.test.mjs, разбор текста workflow), и bash-исполнением реального фрагмента шага (runStepPush) с фейковым git, отвечающим нужным stderr — воспроизводит фактическое поведение раннера, а не только текст скрипта.
• AC3. Мутант push-refusal-kinds-glued (реестр, scripts/mutation-registry.mjs) возвращает старую склеенную регулярку /stale info|rejected|fetch first|lease/i перед веткой workflow — я применил этот патч руками к рабочей копии, прогнал guard-тест (--test-name-pattern="#705 AC1") и получил красный результат (2 из 3 тестов падают, ассерты видят kind=stale там, где ожидался workflow), затем восстановил файл. Мутация действительно ловится названным guard'ом — «умеет падать» подтверждено исполнением, не заявлением автора.
• Регресс на существующий разбор: удаление влитой ветки (deleteBranch) по-прежнему возвращает false только на устаревшем lease; отказ GitHub при удалении ветки (PUSH_REFUSAL.remote/workflow) теперь бросает ошибку с текстом причины вместо молчаливого false — проверено тестом (assert.throws(...).../remote-rejected/).
• Тест на настоящем git с pre-receive-хуком (без сети, bare-репозиторий в tmp) подтверждает, что классификация работает не только на сконструированных строках stderr, а на реальном ответе git/сервера, и что dev не трогается при отказе (ls-remote после отказа = base).
• Трейлеры коммита: Issue: #705, User-Visible: no — верно, правка внутренняя, оба CHANGELOG не нужны.
• test/process-digests.test.mjs (сверка REVIEWER.md с PROCESS.md) зелёный — новый абзац PROCESS.md §4.2 не ломает сверку выжимки.

Риски, названные автором и подтверждённые как честные

Автор прямо пишет: «точный текст отказа именно от HP_PROCESS_TOKEN не виден — логи #700 закрыты. Тексты взяты из публичных отчётов GitHub». Согласен, что это не устранимо в рамках этой задачи: реальный текст отказа для конкретно этого приложения/токена нельзя воспроизвести без доступа к уже закрытым логам. Деградация безопасна: нераспознанный текст отказа падает в unknown (сбой шага, как раньше, ничего не скрывается) или в generic remote-rejected (комментарий с причиной, без ложного «ветка изменилась»). Не нахожу это блокирующим — риск честно раскрыт, а поведение при ошибке классификации не хуже прежнего.

Отдельно: сама эта задача меняет .github/workflows/_process.yml, то есть её собственное слияние в dev — ровно тот случай, который она чинит. Если владелец ещё не выдал HP_PROCESS_TOKEN право на workflow (пункт 1 предложения в ТЗ, явно вынесенный за скоуп кода), слияние этого кандидата само продемонстрирует новый путь push-refused-workflow вместо ложного #312 — это ожидаемое поведение, а не дефект, и не входит в код-ревью.

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

• Полный npm run gate:small/CI-раннер заново — принят по зелёному Validate на этом SHA (ссылка выше, #343).
• Реальный, боевой отказ GitHub с токеном HP_PROCESS_TOKEN — недоступен (логи #700 закрыты); классификация проверена на текстах из публичной документации GitHub, а не на воспроизведённом инциденте.
• Полный ночной прогон реестра мутантов — не гейт ревью на show (#709), прогнан точечно только мутант этой задачи.
• Golden/perf/HA pytest/invariants — диффу не соответствуют (обоснование в таблице гейтов выше).

Вердикт

Зелёный. AC1–AC3 выполнены и доказаны исполнением (unit-тесты, реальный git с pre-receive-хуком, ручная проверка мутанта на красный/зелёный), находок нет.

---

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

• Ветка: issue/705-push-refusal-kinds, коммит c17c05341bc5 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: 43220b1aa1940537b1d101e7a3e377989273a6ad

  git log --all --format='%H %T' | grep 43220b1aa194
  

• Тело issue: 4e6ce37d10900c768172f1d382518d679447b0d0207891176c66cd833c344ae7
• Вердикт конвейера: green · High 0