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

CODE-REVIEW-632-r1

Issue: #632 · этап: код-ревью · заход r1 · блокирующих циклов израсходовано 0 из 4 Трек: инфраструктурный (ускоренный вход, AGENTS.md §1 / PROCESS.md §1) — весь дифф класса B, ни одного файла класса A. Материал: git log --oneline origin/dev..HEAD = один коммит a775080a («task-packet: track follows issue status and history, not a class-A-free diff»), рабочая копия уже на нём (detached HEAD). git diff origin/dev...HEAD: scripts/task-packet.mjs, test/task-packet.test.mjs, scripts/mutation-registry.mjs — 3 файла, +122/-6.

Скоуп

Баг: scripts/task-packet.mjs классифицировал ветку продуктовой S6-задачи как «инфраструктурную» и печатал «файлы класса A трогать НЕЛЬЗЯ», если текущий diff к origin/dev ещё не содержал ни одного файла класса A — реальный кейс #607, чья ветка в момент воспроизведения несла только docs/reviews/SPEC-REVIEW-607-r1.md. Ожидаемое поведение по телу issue: «продуктовый S-flow и статус S5/S6/S7 должны сохранять право менять class A до появления первого такого файла».

Правка вводит branchIsInfrastructure() (документы docs/reviews/** больше не считаются материалом ветки) и productFlowEvidence() (статус S1–S5, раздел ## ТЗ в теле issue, файл docs/specs/NN-*, документ или вердикт ревью ТЗ) — при любом из этих признаков эвристика «diff без класса A → инфраструктура» не применяется.

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

Дешёвые гейты подтверждены зелёным Validate на этом же SHA a775080a (https://github.com/Matysh/houseplan-card/actions/runs/35941025607) — по инструкции раунда не перегонялись повторно: npx tsc --noEmit, npm test, npm run build + сверка бандла. Diff не касается src/**, геометрии, open_spans, marker.space → check-docs.mjs и model-invariants.mjs не применимы. Смоки/golden/pytest/perf — diff их не касается (чистый scripts/**+test/**, src/** не менялся).

Что прогнал сам, целенаправленно по AC и по защитным свойствам:

Команда	Результат
node --test test/task-packet.test.mjs	13/13 ok
node scripts/mutation-gate.mjs --check	task-packet-product-flow-overrides-diff ok, task-packet-review-docs-not-material ok, task-packet-s6-s7-alone-not-product-flow ok (анкоры валидны)
node scripts/mutation-gate.mjs --id=task-packet-product-flow-overrides-diff	поймано 1 из 1
node scripts/mutation-gate.mjs --id=task-packet-review-docs-not-material	поймано 1 из 1
node scripts/mutation-gate.mjs --id=task-packet-s6-s7-alone-not-product-flow	поймано 1 из 1

Три мутанта реально красят ровно заявленный тест и не красят посторонние — защитные AC1–AC3 доказаны по правилу «чем краснеет» (§2.7), не только заявлением автора.

Дополнительно (ручной разбор по коду, не мутацией — проверка гипотезы о непокрытой ветке трека): вызвал buildPacket() напрямую с входами, имитирующими реальный trivial-трек (§5.1) продуктовой задачи —

• синтетический кейс: labels: ['bug','P2','S6-in-progress','trivial'], branch.infrastructure: true, тело issue с обычными AC1./AC2. без заголовка ## ТЗ (trivial-трек ТЗ не пишет вовсе, §5.1) → track остаётся 'инфраструктурный', rights содержит «файлы класса A трогать НЕЛЬЗЯ»;
• то же самое воспроизвёл на реальном закрытом trivial-issue #612 (метки bug, P2, trivial, тело — ## Дефект 1 / ## Дефект 2 / ## AC, без ## ТЗ, без docs/specs файла, без SPEC-REVIEW документа/вердикта) — buildPacket с тем же телом и S6-in-progress даёт тот же ложный инфраструктурный запрет.

Находки

Medium (в скоупе задачи) — trivial-трек не входит в признаки продуктового потока

Файл: scripts/task-packet.mjs:127-143 (PRE_CODE_STATUSES, productFlowEvidence)

Симптом: productFlowEvidence() ищет ровно четыре признака: статус S1–S5, заголовок ## ТЗ в теле issue, файл docs/specs/NN-*, документ или вердикт ревью ТЗ. Все четыре предполагают, что задача писала спецификацию. Короткий трек (trivial, PROCESS.md §5.1) сознательно эту стадию пропускает целиком: «Маршрут: S1-new → S2-analysis → S5-ready → S6-in-progress → …, минуя S3-spec и S4-spec-review» и «AC пишет автор в теле issue при переводе в S5-ready» — без раздела ## ТЗ, без файла в docs/specs, без документа ревью ТЗ. Для такой задачи, как только она проходит S5-ready (то есть как раз в S6-in-progress/S7-code-review, где и предполагается вызов task-packet), productFlowEvidence возвращает пустой список тем же образом, что и для настоящей ускоренной инфраструктурной задачи — их неотличить. Итог: branchIsInfrastructure(...)===true (диффа пока нет файлов класса A — обычная фаза «тесты/фикстуры раньше src») снова печатает «файлы класса A трогать НЕЛЬЗЯ» для реальной продуктовой trivial-задачи — это в точности симптом #607, воспроизведённый для целого документированного трека, который в тестах задачи (AC1…AC4 из хендоффа) не участвует ни разу: AC3 намеренно проверяет случай «инфраструктурная задача без ТЗ» теми же метками ['infra', 'S6-in-progress'] / ['infra', 'S7-code-review'] — без метки trivial, поэтому конфликт между «настоящая инфраструктурная задача без ТЗ» и «настоящая trivial-задача без ТЗ» тестами не различается и не покрыт.

Воспроизведение (проверено чтением + прямым вызовом buildPacket, не исполнением всего скрипта — gh в песочнице недоступен, как и у автора):

const packet = buildPacket({
  issue: { number: 612, title: '…', state: 'CLOSED', url: 'u',
    body: '## Дефект 1\n…\n## Дефект 2\n…\n## AC\n- AC1. …\n- AC2. …\n- AC3. …' },
  labels: ['bug', 'P2', 'trivial', 'S6-in-progress'],
  branch: { name: 'issue/612-x', tip: 'e'.repeat(40), base: 'f'.repeat(40),
    ahead: 1, behind: 0, treeWithoutReviews: null, infrastructure: true },
});
// packet.track === 'инфраструктурный'
// packet.rights содержит 'файлы класса A трогать НЕЛЬЗЯ; …'

#612 — реальный, закрытый trivial-баг (не синтетика): его тело подтверждает, что trivial-задачи не несут ## ТЗ.

Почему не High: task-packet.mjs — советующий инструмент («пакет ничего не пишет и ничего не решает», комментарий в шапке файла), а не гейт CI; ошибочная строка вводит в заблуждение читающего агента, но не блокирует и не портит продуктовый код напрямую. Окно срабатывания уже (нужен push trivial-ветки, чей текущий diff ещё не содержит класса A) из-за WIP-лимита 1 и «push один раз по готовности», но метки trivial подтверждено существуют и используются на практике (нашёл 10 issue с меткой trivial, #612 — предметный пример) и не являются нишевым — это второй из двух регулярных продуктовых треков.

Почему в скоупе задачи, а не отдельный issue: тот же файл, та же функция (productFlowEvidence), тот же класс дефекта, который #632 и должен закрыть — «продуктовый S-flow… должен сохранять право менять class A» из тела issue не содержит оговорки «кроме trivial». Решение владельца 2026-08-19 (#202): Medium в скоупе чинится в этой же задаче.

Предлагаемое (не обязывающее автора) направление: добавить labels.includes('trivial') (по аналогии — стоит проверить и не помешает ли это тому, что trivial+infra тематическая метка одновременно встречается на реальных issue вроде #538/#467-470 — там trivial явно описывает продуктовый short-track, а не ускоренный infra-вход, что я проверил чтением PROCESS.md §1 и §5.1: ускоренный infra-вход не имеет понятия трека trivial/small вовсе, это метки исключительно продуктового S-flow) в список признаков productFlowEvidence, плюс тест на кейс ['trivial', 'S6-in-progress'] и мутант, ловящий откат.

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

• AC1 (продуктовая S6-задача с ТЗ/ревью ТЗ сохраняет право класса A) — доказано test/task-packet.test.mjs › #632: product S6 issue keeps class A rights…, мутант task-packet-product-flow-overrides-diff красит именно этот тест (проверено прогоном).
• AC2 (docs/reviews/** не в материале ветки) — доказано #632: review documents never classify a branch as infrastructure, мутант task-packet-review-docs-not-material (проверено прогоном). Отдельно проверил чтением: branchIsInfrastructure фильтрует по префиксу docs/reviews/ до вызова classify, поэтому вложенные пути (docs/reviews/legacy/…, если появятся) тоже безопасны. По исходному репро #607 (ветка содержит только SPEC-REVIEW-607-r1.md) branchIsInfrastructure возвращает false уже на уровне material.length === 0 — сам факт, что #632 закрывается даже без productFlowEvidence для этого конкретного случая; productFlowEvidence расширяет защиту на менее тривиальные ситуации (например, ветка уже содержит один файл класса B — тест/фикстуру — раньше первого файла класса A).
• AC3 (настоящая инфраструктурная задача без ТЗ, включая возврат в S6 и стояние на S7, сохраняет запрет) — доказано #632: statusless or returned infra issue without spec keeps the class A ban, мутант task-packet-s6-s7-alone-not-product-flow (проверено прогоном). Но покрывает только ['bug','infra','process'] / ['infra','S6-in-progress'] / ['infra','S7-code-review'] — не trivial, см. находку выше.
• AC4 (каждый признак самодостаточен; ## ТЗшка не считается разделом ТЗ) — проверено чтением: (?![\p{L}\p{N}_]) корректно отсекает ТЗшка, поскольку \b в JS не различает кириллицу как «словесный» символ (голый \b посчитал бы границу сразу после З и ложно совпал бы) — логика верна, подтверждено прогоном теста #632: product S6 issue keeps class A rights…, который явно проверяет и позитивный, и негативный случаи заголовка.
• Реальный кейс #607 (единственный файл на ветке — SPEC-REVIEW-607-r1.md) и обратный кейс (единственный файл — scripts/task-packet.mjs, собственная ветка #632) оба воспроизведены тестами и не про регрессируют друг друга.
• Трейлеры коммита: Issue: #632, User-Visible: no — корректно, продуктового поведения нет, changelog не требуется.
• Класс файлов: scripts/task-packet.mjs, test/task-packet.test.mjs, scripts/mutation-registry.mjs — весь дифф класса B, ни одного файла класса A; трек «инфраструктурный (ускоренный вход)» issue #632 применён корректно (метки: bug, infra, S7-code-review, process, без предыдущих S1…S6).
• Формат нового мутанта в scripts/mutation-registry.mjs соответствует соседним записям (id, guard, because, patches[].find/replace); find-строки совпадают с реальными строками файла посимвольно (иначе --check не прошёл бы анкоры — прогнано и подтверждено).

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

• Живой node scripts/task-packet.mjs --issue 607 с реальным gh — не запускал по той же причине, что и автор (gh/api.github.com недоступны из песочницы ревьюера). Проверено на уровне buildPacket напрямую, что эквивалентно проверке решающей логики — сборка входов (collectInputs) в этой правке не менялась содержательно (только вызов вынесенной branchIsInfrastructure). check-inputs.mjs --coverage, no-new-any.mjs — не перегонял, diff не добавляет новых скриптов/тестов вне уже показанных и не содержит TS-типов (any неприменим к .mjs); полагаюсь на зелёный Validate на этом SHA.
• process-gate.mjs целиком — не перегонял; трейлеры проверил вручную (git log -1 --format=full), формат совпадает с §1 таблицей классов.

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

tree 7cd2f2bf4d1c876b39a41d53e489ec1bbe1019cf
blob e033c1014d283942b33fe4819a7def6ec710172d scripts/task-packet.mjs
blob 5cbca549226a4905c093671f6c55b394c17c0168 test/task-packet.test.mjs
blob a0ff104344554102d33db3e18a06cfa9d2932ec7 scripts/mutation-registry.mjs
SHA материала ревью: a775080ae231bf0e1524ab13fda85ee303234280

Вердикт

Жёлтый. AC1/AC2/AC3/AC4 из хендоффа доказаны и воспроизведены (мутанты красят заявленные тесты, тесты падать умеют). Один Medium в скоупе: productFlowEvidence не покрывает trivial-трек продуктовых задач (§5.1), из-за чего реальный продуктовый bug fix на этом треке в S6-in-progress/ S7-code-review может получить тот же ложный «класс A запрещён», что и исходный баг #607 — просто по другому репро. High нет. Возврат автору для правки в этом же issue (решение владельца 2026-08-19, #202) — отдельный issue не заводится.

---

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

• Ветка: issue/632-task-packet-track, коммит a775080ae231 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: ea023197549dc674c02fae226d9324246cf522fe

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

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