houseplan-card/docs/reviews/SPEC-REVIEW-727-r1.md

SPEC-REVIEW-727-r1

Issue: #727 · этап: spec · трек: ask · заход: r1 · блокирующих циклов израсходовано (после этого раунда): 1/4

Скоуп

#727 — первая из двух независимых частей, выделенных из #707 п.3: ночное пакетное ship-ревью на dev и переиспользование его результата в гейте беты (ship-review.mjs check). Контракт — К1 (патч-набор задачи по git patch-id --stable), К2 (ночной режим tag=nightly, имя документа SHIP-REVIEW-<база>-dev-<sha12>.md), К3 (shipCoverage: clean/high/ stale/none), К4 (гейт check по покрытию), К5 (ревью беты читает только дельту none/stale), К6 (job в _nightly.yml, dispatch ship-review.yml с tag=nightly), К7 (индекс и архив узнают ночное имя), К8 (комментарий в задачу при High), К9 (канон — PROCESS.md §10.4/§11.7, REVIEWER.md). Красная ночь (комментарий по задачам, слитым после последней зелёной) — отдельный issue E, в #727 не входит. Продуктового кода задача не трогает (класс A файлов нет), User-Visible: no. ТЗ живёт в теле issue под ## ТЗ; комментарий владельца 2026-09-30 передаёт на ревью с треком ask, обоснование которого — первая строка раздела «ТЗ» (сложность/риск >3: четыре поверхности; публичный контракт: правило гейта беты §11.7 меняется с «документ тега покрывает все ship-задачи» на «задачи покрыты документами диапазона с тем же патч-набором») — соответствует критериям §5, трек ask обоснован корректно.

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

Ревью текстовое (этап spec, продуктовой ветки нет). Три слоя:

1. Обязательные разделы §7.1 и однозначность каждого AC — чтением тела issue.
2. Проверка каждого технического утверждения по текущему коду на материале ревью (HEAD рабочей копии 40607aa37f13819c9db37a392a1d99f30382b3c0, совпадает с origin/dev): ТЗ называет конкретные функции, регэкспы, имена файлов, мутанты и исторические коммиты — каждое утверждение либо подтверждается чтением, либо остаётся непроверенной догадкой (находка по §7.1). Список проверенного — ниже.
3. Гейты не прогонялись: задача не меняет ни одного файла репозитория на этом этапе (см. «Чего не проверял»).

Находки

Medium — К7/AC7: архивирование ночного документа с базой-стабильным-тегом не имеет ни одного проверяемого примера, а текущий код его не реализует

Файл: тело issue #727, раздел «Контракт поведения» → К7, второй подпункт («база — стабильный тег → первая архивируемая стабильная линия новее базы»); AC7 в таблице «Критерии приёмки».

Воспроизведение. archivePlan (scripts/reviews-archive.mjs:91-97) сегодня для doc.stage === 'ship' вычисляет единственную целевую линию так:

const line = doc.tag.replace(/-beta\.\d+$/, '');
if (tags.has(line)) moves.push({ ... to: `${ARCHIVE_DIR}/${line}/${name}` ... });
else kept.push({ name, reason: `ревью линии ${line} не входит в архивируемые линии` });

Это находит ТОЧНОЕ совпадение (после снятия -beta.N) между базой документа и одной из архивируемых стабильных линий. Для сегодняшнего единственного формата имени (SHIP-REVIEW-<тег-беты>.md) это корректно: база всегда сама и есть та линия, в которую документ должен уйти.

К7 вводит новый случай — ночной документ, чья база (doc.tag после парсинга нового формата К2/AC7) может быть стабильным тегом (ночь, прошедшая после стабильного релиза и до первого beta.1 следующей линии — это не редкий случай, а обычное окно между двумя циклами). Для него К7 явно требует: «первая архивируемая линия новее базы», то есть не точное совпадение, а поиск следующего по порядку тега. Текущая реализация archivePlan такого поиска не делает вообще: если doc.tag (стабильный) не входит дословно в tags (например, база v1.78.0, а архивируется уже v1.79.0), ветка попадёт в else и документ останется в docs/reviews/ с причиной «линия не входит в архивируемые», а не переедет в v1.79.0, как того хочет К7. Это значит: правило К7 для этой ветки требует новой логики в archivePlan (поиск ближайшего большего тега, не поиск по равенству), а не простого использования существующей функции.

AC7 при этом даёт ровно три конкретных примера — parseDocName(...) на новое имя, renderIndex его перечисляет, archivePlan уносит документ с базой-бетой в v1.79.0 (это старая, уже поддержанная ветка кода). Для ветки «база — стабильный тег» AC7 ограничивается пересказом самого правила («База v1.79.0 → первая архивируемая линия новее базы») без единого конкретного входа и ожидаемого выхода: какая база, какой набор архивируемых тегов, какой результат. Без такого примера реализатор и ревьюер кода не смогут отличить осмысленную имплементацию нового алгоритма от случайной: нет фиксированной пары «вход → выход», по которой можно требовать тест, а «способ доказательства» AC7 называет только файл и раздел теста, не конкретный случай.

Почему это находка, а не мелочь. Правило К7 в этой части меняет публичный контракт архивирования (куда физически переедет документ, на который будущие ревью ссылаются как на «унаследовано из r», §2.10) — ошибка здесь тихая: archivePlan возвращает план, который применяется git-мультивом в --apply, и неверный путь обнаружится не тестом, а человеком, читающим архив postfactum. Ветка реально достижима (первое же окно между релизом и следующей бетой), не теоретический край.

Что нужно от автора. Добавить в AC7 (или в само К7) один конкретный пример для ветки «база — стабильный тег»: например, «архивируемые линии [v1.78.0, v1.79.0], документ с базой v1.78.0 (стабильный) → переезжает в v1.79.0» и отрицательный случай «архивируемых линий новее базы ещё нет → документ остаётся (kept), как сегодня для тега, которого нет в tags». Это дёшево (одна строка в таблице AC) и снимает риск, что «первая линия новее базы» останется нереализованной или реализованной с ошибкой в граничном случае (нет более новой линии, несколько линий новее одновременно).

Без High в задаче это жёлтый вердикт, правка ТЗ и повторный цикл (§2.4, §4).

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

ТЗ необычно тщательно сверено с текущим кодом и историей; кроме находки выше, все проверенные утверждения подтвердились дословно:

• Существование и сигнатуры readCandidateHistory, issueTrailers (scripts/release-membership.mjs, импортируются в scripts/ship-review.mjs:28), shipReviewDocPath, isShipIssue, specSection, shipIssuesInRange, renderShipBrief, anchorBlock, parseAnchorBlock, shipReviewProblems, readShipDoc в scripts/ship-review.mjs — все существуют как названо.
• RELEASE_TAG_RE (scripts/ship-review.mjs:33) и shipReviewDocPath (падает на нерелизном теге) — подтверждают, что литеральное tag=nightly не может случайно совпасть с реальным тегом релиза (риск «неявный вход», который ТЗ сам называет и откладывает).
• Текущий машинный блок документа (docs/reviews/SHIP-REVIEW-v1.79.0-beta.1.md, якорь <!-- hp-ship-review-anchors -->, поля tag/candidate/base/issues/ high/medium/low/run) совпадает построчно с тем, что генерирует anchorBlock — К2's «прежние строки блока не меняются», «дополняется mode/patches» корректно описывает расширение, а не переписывание существующего формата.
• Существующий тег v1.79.0-beta.1 и документ SHIP-REVIEW-v1.79.0-beta.1.md реальны (нет фантомных примеров); пример имени ночного документа в К2/AC7 (SHIP-REVIEW-v1.79.0-beta.1-dev-108427dc1234.md) корректно использует реальный тег как базу.
• Коммит dca0fd28 («Release v1.79.0-beta.1 candidate») реально несёт трейлеры Issue: #661/#692/#693/#711/#713 и Release: v1.79.0-beta.1 — обоснование исключения Release:-коммитов из патч-набора (К1) фактически точное, не гипотетическое.
• SHIP_DOC_NAME в scripts/reviews-index.mjs:32 и parseDocName (scripts/reviews-index.mjs:45-58) сегодня действительно узнают только SHIP-REVIEW-<тег>.md без суффикса — К7's требование расширить регэксп под -dev-<sha12> с nightly: true корректно описывает необходимое изменение, а не несуществующий пробел.
• parseDocName/archivePlan в scripts/reviews-archive.mjs импортируются и используются именно так, как описывает К7 (кроме разобранного пробела).
• Существующий тест test/nightly-workflow.test.mjs (29 строк) уже содержит test('nightly ждёт запущенный Validate и падает вместе с ним (#492 §7)', …) и тест про русское имя job — ровно те тесты, которые АC6 требует оставить зелёными; файл не новый, «Затронутые файлы» его называет верно.
• Текущий _nightly.yml — одна job dispatch, делает gh workflow run validate.yml --repo "$REPO" --ref dev -f full=true токеном GH_TOKEN: ${{ github.token }}, ждёт появления прогона до 3 минут (18×10с) — это ровно прецедент, на который К6 ссылается («как у dispatch Validate», «до трёх минут, как у Validate»); новая job для ship-review по аналогии технически реализуема без смены токена или прав.
• ship-review.yml (тонкий, main) сегодня действительно объявляет входы tag (required), candidate (optional, default ""), force (optional boolean) и права contents: read / job dev: contents: read, issues: read — К6's «тонкие файлы не меняются: входы... и права... уже есть в main» подтверждается дословно; tag=nightly не требует нового входа.
• HP_PROCESS_TOKEN — существующий секрет, уже используется для публикации комментариев/меток конвейером (_process.yml, _process-resume.yml, _beta-derived.yml и др.) — К8's «Токен — HP_PROCESS_TOKEN, как у публикации» ссылается на реальный, а не придуманный прецедент; в _ship-review.yml сегодня комментариев в issue нет — это действительно новая часть работы, и ТЗ её как новую не маскирует.
• Мутант ship-review-ignores-merge-marker существует в scripts/mutation-registry.mjs:13732 (AC10).
• node scripts/entry-cost.mjs --check — реальная команда (scripts/entry-cost.mjs:12,75), AC9 её не выдумывает.
• test/process-digests.test.mjs, test/ship-review.test.mjs существуют — план автотестов ссылается на реальные файлы, не на планируемые с нуля (кроме test/nightly-workflow.test.mjs, который тоже существует).
• Обязательные разделы §7.1 (сценарий, что человек увидит, проблема, скоуп и не-скоуп, контракт поведения, UX/данные/миграция/i18n/perf/touch, критерии приёмки AC1–AC10 со способом доказательства, план автотестов, риски, откат, release-артефакты) — присутствуют, в правильном порядке.
• «Принято предположительно» — 6 пунктов, все — реальные технические развилки (способ dispatch, гранулярность severity по документу vs по задаче, критерий «последний документ», имя файла), ни один не маскирует продуктовый вопрос: гранулярность «High снимает покрытие со всех задач документа» совпадает с уже действующим сегодня правилом гейта §11.7 («документ обязан покрывать их все и не нести High» — это уже документ-уровневый, а не задаче-уровневый гейт), так что это не новый прецедент, а перенос старого.
• Факты сверены автором с origin/dev 108427dc — коммиты между этим SHA и материалом ревью (git diff 108427dc..40607aa3 --stat) не затрагивают ни один из файлов, которые называет ТЗ (scripts/ship-review.mjs, scripts/reviews-index.mjs, scripts/reviews-archive.mjs, .github/workflows/_nightly.yml, .github/workflows/_ship-review.yml, .github/workflows/ship-review.yml) — материал не устарел за эти 4 коммита.
• Продуктовых вопросов владельцу в ТЗ нет; единственный открытый пункт, который я бы мог счесть техническим спором (гранулярность severity), оказался уже действующим прецедентом — оспаривать нечего.

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

• Гейты (npx tsc --noEmit, npm test, npm run build) не прогонял — задача не меняет ни одного файла репозитория на этом этапе; они станут обязательны на S7-code-review к реальному диффу.
• Не проверял golden/смоки/бэкенд-pytest/инварианты модели — задача класса A не содержит, User-Visible: no, геометрии и визуала не касается (сама ТЗ это явно фиксирует в разделе «UX · данные · i18n · миграция · perf · touch» и «Release-артефакты»).
• Не проверял корректность git patch-id --stable на реальных cherry-pick сценариях (АС1 «тот же дифф в другом коммите даёт тот же patch-id») — это задокументированное поведение самой команды git, не специфика этого кода; проверка на встроенном временном git-репозитории — задача автора теста, не спецификации.
• Не оценивал реальную стоимость job prepare ship-ревью в ночи («около минуты, без npm ci») — измеримо только на первом живом прогоне; ТЗ сама называет первый живой прогон «наблюдением, не AC» (раздел «Риски»).
• Не проверял, действительно ли GitHub допускает workflow_dispatch через gh workflow run с GITHUB_TOKEN для другого workflow-файла (ship-review.yml) из job с правом actions: write, отличным от той же операции для validate.yml в существующем коде, — по документированному поведению GitHub Actions разницы по целевому workflow нет (workflow_dispatch API не различает вызываемый файл), и это тот же механизм, что уже работает в _nightly.yml; отдельно на реальном runner не проверял.

Вердикт

Один Medium в скоупе задачи (К7/AC7: правило архивирования ночного документа с базой-стабильным-тегом не реализуемо существующим archivePlan и не имеет ни одного конкретного примера входа/выхода в AC) — без High это жёлтый вердикт с возвратом автору по §2.4. Все прочие технические утверждения ТЗ — имена функций, регэкспы, номера строк, реальные теги и коммиты — сверены построчно с dev и подтвердились; трек ask обоснован верно; продуктовых вопросов владельцу нет.

Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 1 → в задаче

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

• Этап: spec. Материал — тело issue #727 на момент комментария Matysh 2026-09-30T23:25:21Z («Оценка и ТЗ: трек ask… Передаю на ревью ТЗ»).
• Кода/ветки продукта не существует (инфраструктурная задача до реализации); факты сверены с рабочей копией на HEAD = origin/dev = 40607aa37f13819c9db37a392a1d99f30382b3c0.

---

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

• Ветка: dev, коммит 40607aa37f13 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: 901e6cbd1964ef2dcd9a0e08e8ec01a5c47dcac7

  git log --all --format='%H %T' | grep 901e6cbd1964
  

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