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

CODE-REVIEW-703-r1

Issue: #703 · «stable promotion ложно судит уже проверенную бета-линию» Трек: show (владелец задал критерии в теле issue) Материал: 4496c232a0a53186b5db426b584b99a431f7cfbb, ветка issue/703-promotion-range-base поверх origin/dev Заход: r1 · блокирующих циклов израсходовано 0 из 2

Скоуп

Инфраструктурная задача (не класс A, входит сразу в код-ревью, AGENTS.md «Tracks»). Три AC в теле issue:

• AC1 (unit, фикстура git) — промоушен SHA, уже зелёного на dev, даёт пустой диапазон гейтам no-new-any/no-new-private-writes на main; без прогонов dev в окне API граница — последний тег v*, а не прошлый stable.
• AC2 (unit) — новое нарушение после границы по-прежнему красное (hotfix над кандидатом; повтор упавшего HEAD).
• AC3 (контракт workflow + документация) — changes и preflight на main читают прогоны обеих интеграционных веток, preflight больше не падает на event.before, граница описана в PROCESS.md §10.2.

Изменение процессное — docs/SCOPE.md им не ограничивается (AGENTS.md, «infrastructure task … skips analysis and spec»). User-Visible: no в трейлере коммита — верно, поведения продукта дифф не меняет.

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

Прочитаны: AGENTS.md, docs/process/REVIEWER.md, тело issue #703 и оба комментария («Взял», «Сделано»), PROCESS.md §8, §10.2, §11.5, §11.6. Дифф разобран целиком (git diff origin/dev...HEAD, 5 файлов): scripts/classify-base.mjs, .github/workflows/validate.yml, test/promotion-range.test.mjs (новый), test/validate-workflow.test.mjs, PROCESS.md §10.2.

Прочитан scripts/classify-base.mjs целиком, не только дифф — проверено, что pickRangeBase корректно приоритизирует HEAD (только доказанный успехом или тегом), затем предков по объединению judged ∪ tagged, с верной атрибуцией reason (green-ancestor vs release-tag).

Прогнал точечно (полный gate:small/tsc/build не гонял — см. «Чего не проверял»):

• node --test test/promotion-range.test.mjs → pass 6.
• node --test test/validate-workflow.test.mjs → pass 28.
• Мутация: временно заменил if (head && (headGreen.has(head) || tagged.has(head))) на if (false) в scripts/classify-base.mjs, перезапустил promotion-range.test.mjs → pass 4, fail 2 (AC1-тесты красятся), откатил файл (cp бэкапа), git status --short после отката пуст — эксперимент следов не оставил. Тест умеет падать.
• Независимо, вне тестовой фикстуры, воспроизвёл парсинг git for-each-ref --format='%(refname:short) %(objectname) %(*objectname)' на настоящем репозитории с annotated и lightweight тегами — поле «коммит» (peeled || object) извлекается верно для обоих видов.
• node scripts/smoke-select.mjs --base origin/dev --head HEAD → «Исполняемого frontend-диффа нет»; дифф не трогает src/**, браузерные смоки не относятся.

Главная проверка этого раунда — воспроизвести производственное поведение границы по тегам, а не только unit-фикстуру. AC1 второй сценарий («без прогонов dev … граница — последний тег») в test/promotion-range.test.mjs создаёт теги прямо в свежесозданном git-репозитории и вызывает classify-base.mjs там же — это НЕ то, как коммит попадает в рабочую копию джобов preflight/changes в реальном Validate: там дерево кладёт actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 (# v7, validate.yml:71,342) с fetch-depth: 0, filter: 'blob:none' и БЕЗ fetch-tags: true. Я проверил исходники именно этого запиненного коммита (curl тела action.yml и dist/index.js с того же SHA):

• action.yml: вход fetch-tags — default: false;
• dist/index.js, метод fetch(): args.push('--no-tags') безусловно, с комментарием «Always use --no-tags for explicit control over tag fetching»; refspec +refs/tags/*:refs/tags/* добавляется только когда settings.fetchTags === true (getRefSpec).

Затем эмпирически воспроизвёл именно эту последовательность: git init + remote add + git -c protocol.version=2 fetch --no-tags --prune --no-recurse-submodules … на репозитории с тегом, затем — как и делает шаг changes/preflight дальше по файлу (git fetch -q origin dev, validate.yml:371,726,841) — простой git fetch БЕЗ явного --no-tags. В обоих случаях git for-each-ref refs/tags после фетча пуст: явный одиночный refspec (origin dev) не включает git-механизм auto-follow тегов, а --no-tags на первом fetch не оставляет вообще ни одного tracking-объекта для тега. Убедился отдельно, что это не проблема моей среды ревью: в РАБОЧЕЙ КОПИИ этой сессии (клонированной иначе, не через actions/checkout) теги есть — несовпадение подтверждает, что дело в конкретном пути checkout-экшена, а не в git вообще.

Находки

High — release-tag граница (AC1, вторая часть) не работает в реальном Validate: checkout никогда не тянет теги

scripts/classify-base.mjs:88-99 (releaseTaggedShas) и вызов на scripts/classify-base.mjs:292-294 читают git for-each-ref … refs/tags локально. Это верно работает в test/promotion-range.test.mjs, потому что фикстура создаёт теги прямо в том же репозитории, где выполняется CLI. Но ни один шаг preflight/changes в .github/workflows/validate.yml не устанавливает fetch-tags: true на checkout (grep -n fetch-tags .github/workflows/validate.yml — пусто), а сам actions/checkout@… (v7, пин 3d3c42e5aac5ba805825da76410c181273ba90b1) при fetch-tags: false (дефолт) передаёт git fetch --no-tags безусловно и не добавляет refspec тегов — подтверждено чтением dist/index.js этого экшена и независимо эмпирическим повтором той же fetch-команды (см. «Как проверялось»). Последующие явные git fetch -q origin dev (validate.yml:371,726,841) тоже не подтягивают теги: явный одиночный refspec не запускает auto-follow.

Воспроизведение (без CI, тем же способом, что и для встроенного unit-теста, но с реальной последовательностью git-команд checkout-экшена):

git init /tmp/src && cd /tmp/src && ... commit ... && git tag -a v1.0.0 -m rel
cd /tmp/dst && git init && git remote add origin /tmp/src
git -c protocol.version=2 fetch --no-tags --prune --no-recurse-submodules \
  origin +refs/heads/main*:refs/remotes/origin/main* +refs/heads/main:refs/remotes/origin/main
git for-each-ref refs/tags   # пусто
git fetch -q origin dev     # как later-шаги validate.yml
git for-each-ref refs/tags   # по-прежнему пусто

Следствие. Основной путь AC1 (SHA, зелёный на dev в пределах окна API в 100 прогонов) не затронут — он работает через ответ GitHub API, а не через git tag. Но именно вторая часть AC1 — «без прогонов dev в списке граница — последний тег v*, а не прошлый stable» — заявлена как страховка НА СЛУЧАЙ, когда SHA кандидата выпал из стодневного окна API (этот риск прямо назван автором в комментарии «Сделано»: «Если SHA кандидата выпал из окна, границей станет последний тег. Это шире, чем нужно, но не прошлый stable»). В реальности это неверно: при выпадении из окна tagged пуст, pickRangeBase падает мимо release-tag сразу в fallback (event.before), т.е. воспроизводит ИМЕННО тот инцидент, ради которого заведено #703 — просто при более редком триггере (окно API, а не разница dev/main). Ни один существующий гейт это не ловит: test/promotion-range.test.mjs и test/validate-workflow.test.mjs проверяют CLI и текст workflow-файла по отдельности, ни один не проверяет фактическое поведение actions/checkout с реальными его флагами.

Чем закрыть (для автора, не для этого ревью): добавить fetch-tags: true к обоим checkout (validate.yml:71 в preflight, :342 в changes) — они и так тянут fetch-depth: 0, полную историю, лишних данных это не добавит; и дополнить test/validate-workflow.test.mjs (или новый тест) assertion на fetch-tags: true рядом с уже добавленной проверкой fetch-depth: 0 (test/validate-workflow.test.mjs, новый тест «#703 AC3», последняя строка), чтобы дальнейшая правка checkout не могла тихо снять флаг.

Low — тест AC2 не различает «hotfix на main» и «пуш в dev» на практике

test/promotion-range.test.mjs:121-134, тест «#703 AC2: новое нарушение после границы красное — hotfix на main и пуш в dev» вызывает rangeBase(fx, hotfix, { dev, main }, fx.candidate) дважды с ИДЕНТИЧНЫМИ аргументами (onMain и onDev) — разницы между «событие на main» и «событие на dev» в самом вызове нет, оба используют один и тот же merged payload. Это осмысленно отражает инвариант «граница одна независимо от ветки», но имя и комментарий теста обещают проверку двух разных путей, а по факту второй ассерт (onDev) — дубликат первого, не отдельный сценарий. Не блокирует: собственно инвариант («граница одна») проверен по построению (mergeRunPayloads тестируется отдельно), а не через эту дублирующую проверку. Можно оставить как есть — записываю, находка Low, не открывает цикл (трек show, REVIEWER.md «Трек show»: бухгалтерия — Low).

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

• AC1, первая часть — исполнением: test/promotion-range.test.mjs «#703 AC1: промоушен проверенного SHA в main — пустой диапазон и ноль нарушений» зелёный, мутация на строку HEAD-проверки красит его (см. «Как проверялось»). CLI действительно возвращает base = head, диапазон пуст, no-new-private-writes находит 0 вместо 2 старых записей. Работает через API-ответ (headGreen), не зависит от найденного дефекта с тегами.
• AC2 — оба сценария (hotfix над кандидатом, повтор упавшего HEAD) доказаны исполнением, зелёные, мутация на firstGreen/headGreen логику не ставилась отдельно, но покрывающий её путь (проверка «упавший HEAD не граница») в тесте #703: HEAD засчитывает только успех или тег… (test/promotion-range.test.mjs:148-168) явно различает упавший и доказанный HEAD через прямой вызов pickRangeBase — читал построчно, логика корректна.
• AC3, контракт workflow — подтверждено чтением validate.yml и исполнением test/validate-workflow.test.mjs (строковые ассерты на обновлённый if: preflight, цикл for branch in dev main, изменение changes/base-шага с other=dev/main). Preflight действительно больше не падает на event.before для main: шаг «База диапазона» теперь выполняется и для refs/heads/main, BEFORE_SHA у provenance/process_gate переключается на steps.range.outputs.base.
• PROCESS.md §10.2 — текст соответствует коду один в один (три условия границы, HEAD только по успеху/тегу, ссылка на реализацию и тест); добавлен в верное место — рядом с уже существующей оговоркой «При продвижении в main не перепроверяются коммиты, уже достижимые из prerelease-тега».
• Трейлеры коммита — Issue: #703, User-Visible: no — верны, никакого видимого пользователю поведения дифф не меняет; changelog не тронут и не должен быть.
• «Одно число — один источник» (§8) — неприменимо, дифф не вводит величин, видимых пользователю.
• releaseTaggedShas корректно разбирает и annotated, и lightweight теги (проверено отдельным прогоном git for-each-ref на реальном репозитории — см. «Как проверялось»), фильтр RELEASE_TAG отсекает beta.0 и немаркированные теги верно (юнит-тест плюс мой ручной прогон совпали).
• Изменение не трогает геометрию модели, Python-бэкенд, golden-эталоны — соответствующие гейты неприменимы.

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

• Полный npx tsc --noEmit / npm test / npm run build с сверкой трёх копий бандла не гонял заново: Validate на этом SHA (4496c232) зелёный (https://github.com/Matysh/houseplan-card/actions/runs/36745546331), §8/AGENTS.md разрешают на него сослаться. Прогнал точечно только два изменённых тестовых файла (см. «Как проверялось») — это не замена полного набора.
• Браузерные смоки — smoke-select сообщает «исполняемого frontend-диффа нет», тело issue не называет ни одного смока в AC; Chromium не поднимал.
• npm run golden:verify — метки ci:golden нет, demo/golden/** не тронут.
• python -m pytest tests_backend — custom_components/**/*.py не тронут.
• npm run invariants — дифф не касается геометрии модели плана.
• Performance-профили — не названы в AC, дифф не трогает рендер/performance код.
• Мутанты по диффу — трек show: мутанты в разработке не гоняются ни на каком треке (REVIEWER.md, «Трек show», #709); в этом диффе и нет продуктового кода, защита — чистые юниты с отрицательным случаем в самом тесте (см. таблицу ниже), реестр мутантов не применим.
• Живой промоушен dev → main на реальном GitHub Actions не воспроизводил — автор прямо пишет «НЕ сделано: живой промоушен в main этим не проверить до следующего stable»; это ожидаемо и не находка само по себе, но именно поэтому расхождение checkout-экшена с ожиданием теста осталось незамеченным до этого ревью.

Таблица «AC · чем доказан · чем краснеет» (защитные AC)

AC	чем доказан	чем краснеет
AC1 (пустой диапазон на доказанном SHA)	test/promotion-range.test.mjs тест «#703 AC1: промоушен …»	мутация if (false) на HEAD-проверку — падает (проверено в этом раунде)
AC1 (тег как граница без прогонов dev)	тот же файл, тест «#703 AC1: без прогонов dev …»	не красит в производственном пайплайне — см. находку High: releaseTaggedShas в реальном Validate всегда получает пустой набор тегов, юнит-тест этого не ловит, т.к. не проходит через actions/checkout
AC2 (новое нарушение красное)	тесты «#703 AC2: …» ×2	отрицательный случай внутри самого теста (упавший HEAD vs доказанный)
AC3 (workflow-контракт)	test/validate-workflow.test.mjs, новый тест	строковые ассерты на regex — точечно, но соразмерно (контракт YAML, не поведение)

Вердикт

Красный. Один High: вторая часть AC1 (release-tag как граница, когда SHA кандидата выпал из окна API в 100 прогонов) реализована в scripts/classify-base.mjs корректно, но неработоспособна в реальном Validate — ни один из задействованных шагов actions/checkout не получает fetch-tags: true, и пин v7 (3d3c42e5aac5ba805825da76410c181273ba90b1) безусловно передаёт --no-tags. Это тихо воспроизводит именно тот класс инцидента, ради которого заведено #703, при более редком, но реальном триггере. Основной путь AC1 (доказанный SHA в пределах окна API) и оба сценария AC2 работают верно. Один Low — тестовая избыточность, не блокирует.

---

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

• Ветка: issue/703-promotion-range-base, коммит 4496c232a0a5 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: 68cd755f1339d9f4cedc1a28b173b15519540bed

  git log --all --format='%H %T' | grep 68cd755f1339
  

• Тело issue: d3b8c4a60a2666a458283fb684284710ad5982fe267e8a362f852f3dedaa5761
• Вердикт конвейера: red · High 1