docs: review document for #517

Issue: #517
User-Visible: no

docs/reviews/SPEC-REVIEW-517-r1.md

@@ -0,0 +1,219 @@
+# SPEC-REVIEW-517-r1
+
+## Скоуп
+
+Issue #517 переносит хранение ТЗ полного трека из `docs/specs/<NN>-*.md` в тело
+issue и переносит доказуемость «вердикт ревью вынесен на этом тексте» с блоба
+файла на `sha256` нормализованного тела issue, записываемый в блок якорей
+документов ревью. Пять поверхностей: (1) `process.yml` — новый шаг снятия
+хеша тела и сравнение с записью последнего зелёного `SPEC-REVIEW`; (2)
+`scripts/review-doc-guard.mjs` — `anchorIssueBodyFrom`/сравнение хешей; (3)
+`scripts/process-gate.mjs` — замена проверки 3 (файл ТЗ) на проверку заголовка
+`## ТЗ`/`AC1` в теле; (4) `scripts/task-packet.mjs` — смена приоритета
+источника AC; (5) `PROCESS.md`/`AGENTS.md`/`docs/specs/README.md` — переписывание
+правил. `User-Visible: no`, продукт и словари не затронуты.
+
+Issue помечен `small`; ТЗ по правилу лёгкого трека действительно лежит в теле
+issue, файла `docs/specs/517-*.md` нет — соответствует ожиданию для этой метки.
+Ветки `issue/517-*` на origin нет: кода ещё не существует, ревью — чисто по
+тексту тела issue и его единственному комментарию (S2-аналитика).
+
+## Как проверялось
+
+- Прочитаны `docs/SCOPE.md`, `PROCESS.md` (целиком), `AGENTS.md`.
+- Прочитано тело issue #517 и комментарий S2 (`gh issue view 517 --json body,comments`).
+- Каждое фактическое утверждение ТЗ и S2-комментария о текущем коде сверено с
+  реальными файлами, а не принято на слово:
+  - `scripts/review-doc-guard.mjs`: `materialAnchorBlock` (строка 256),
+    `anchorTreeFrom`/`anchorVerdictFrom` (442, 451), `reusableGreenVerdict`
+    (480) — сигнатуры и поведение совпадают с описанием в ТЗ (У2, У3).
+  - `scripts/process-gate.mjs`: `checkSpecs` (305), `NO_SPEC_FILE` (76) —
+    совпадает с описанием гейта в плане и S2.
+  - `scripts/task-packet.mjs`: `extractAcceptanceCriteria` (49), `acSource`
+    тернарник (строка ~112: `specs.length ? specs... : issue.body`) —
+    поведение (AC из файла, тело — fallback) совпадает с тем, что план
+    собирается инвертировать (AC5).
+  - `.github/workflows/process.yml`: полностью прочитана структура job
+    `review` — шаги `Перейти на ветку задачи` (безусловный), `Привести ветку к
+    dev`/`rebase` (`if: stage=='code'`), `Зафиксировать SHA материала ревью`/
+    `material` (`if: rebase.conflict != 'true'`, БЕЗ условия по stage),
+    `reuse` (#499, `if: stage=='code'`, сравнивает только дерево через
+    `reusableGreenVerdict`), шаг вызова модели `Review` (`if: gate.proceed &&
+    reuse.reuse != 'true'`), шаг публикации документа (использует
+    `steps.material.outputs.*` без разбора по stage).
+- Проверено отсутствие `docs/specs/517-*.md` (`ls docs/specs/517*` — нет
+  файлов), что подтверждает корректность выбора лёгкого треке ТЗ-в-теле.
+- Проверены точные ссылки, которые ТЗ/S2 называют как подтверждённые фактом:
+  `docs/specs/README.md` (раздел «Обязательные release-артефакты», на который
+  ссылается `PROCESS.md:158`), список секций PROCESS.md (`grep '^## \|^###
+  '`) — секции §2.3, §5, §7.1, §7.3 п.1 существуют и релевантны; секция
+  «§10.5 п.3», которую S2-комментарий называет как место требования файла ТЗ в
+  гейте, **не существует** (максимальный номер подраздела — §10.4, дальше идёт
+  §11) — верная ссылка на этот гейт — §10.2 п.3.
+- Гейты (`typecheck`/`test`/`build`) не прогонялись и не нужны на этом этапе:
+  кода нет, ветки нет, диффа нет — предмет ревью текстовый (тело issue), а не
+  дерево репозитория.
+
+## Находки
+
+### Medium-1 (в скоупе). Гарантия AC2 не держится при повторном применении зелёного вердикта (#499)
+
+**Файл:** тело issue #517, раздел «Уточнения S2», пункт У3, и AC2.
+
+**В чём дефект.** AC2 обещает: правка тела issue после зелёного ревью ТЗ
+обязательно даёт код-ревью строку «ТЗ менялось после зелёного ревью ТЗ» и
+полный разбор. У3 при этом утверждает: «reuse (#499) и страховка #414 не
+затрагиваются» — то есть механизм повторного применения зелёного вердикта без
+вызова модели остаётся как есть. Но именно этот механизм и есть дыра:
+`reusableGreenVerdict` (`scripts/review-doc-guard.mjs:480`) признаёт вердикт
+переиспользуемым по ТРЁМ условиям — записанный вердикт `green`/`High 0`,
+наличие якоря дерева, и **отсутствие отличий git-дерева** от этого якоря
+(`differs(tree)`) — и не знает ничего о теле issue. Шаг `reuse`
+(`process.yml:436-447`) условен только на `stage == 'code'`, а шаг вызова
+модели `Review` (~851) стоит под `if: gate.proceed && reuse.reuse != 'true'`.
+Значит, когда `reuse=true`, модель **не вызывается вовсе** — а именно туда, по
+плану ТЗ, должна попадать «находка, доставляемая ревьюеру в промпт».
+
+**Сценарий воспроизведения.** Код-ревью раунда r_k проходит зелёным (High 0),
+дерево якорится. Слияние не удаётся (ребейз/страж #312) — задача возвращается
+в `S6` и сразу в `S7` (ровно сценарий #437 r4, на который ссылается сам
+`reusableGreenVerdict`). Между r_k и r_k+1 кто-то правит тело issue (уточняет
+AC, меняет формулировку риска) — дерево кода при этом не меняется. На r_k+1
+`reuse` видит: дерево не отличается от якоря вне `docs/reviews/**` → вердикт
+переиспользуется, модель не вызывается, найденная-по-плану строка «ТЗ
+менялось» никогда не печатается. Раздел «Риски» ТЗ утверждает обратное:
+«правка после этого попадёт в следующий заход» — для этого конкретного пути
+это неверно: следующий заход — это переиспользование без чтения вообще.
+
+Важно: на ПЕРВОМ раунде код-ревью (переход `S5→S7`, live-проверка, которую
+называет сам AC2) `reuse` в принципе не может сработать — предыдущего
+`CODE-REVIEW`-документа ещё нет, `reusableGreenVerdict` вернёт `null`, модель
+вызывается штатно. То есть заявленная в AC2 «живая проверка» пройдёт и
+подтвердит AC — но не покроет описанный выше многораундовый путь, и ни один
+названный в ТЗ тест/мутант его не ловит.
+
+**Почему это Medium, а не High.** Основной путь (первое код-ревью после
+зелёного ревью ТЗ) действительно закрыт корректно — уязвим только повторный
+раунд после rebase-only возврата, что само по себе не рядовая, а уже особая
+ситуация (#437-класс). Дефект в скоупе задачи и правится в ней же: либо
+`reusableGreenVerdict`/вызывающий его CLI получает дополнительный параметр
+«хеш тела issue совпадает с якорем», либо в разделе «Риски» ТЗ явно
+фиксируется этот остаточный пробел как принятый (а не отрицается фразой «не
+затрагивается»).
+
+### Medium-2 (в скоупе). Метка `small` не соответствует собственному описанию задачи как полнотрековой
+
+**Файл:** метки issue #517; комментарий S2 («Трек: полный (три поверхности —
+конвейер, гейт, документация процесса)... сегодня формально это трек
+small-стиля, поэтому дополнительно ставлю метку small»).
+
+**В чём дефект.** §5 PROCESS.md требует ОДНОВРЕМЕННО: сложность/риск ≤3, ОДНУ
+поверхность, отсутствие миграции, отсутствие нового UX-контракта, отсутствие
+влияния на touch/perf. Автор сам называет три поверхности (пайплайн, гейт,
+процесс-документация) и явно пишет «трек: полный» прямым текстом — это прямое
+нарушение критерия «одна поверхность», названное самим автором, а не
+предположение ревьюера. Несмотря на это, к issue добавлена метка `small`, и
+заявленная причина — «чтобы `process-gate --issues` не требовал файла ТЗ на
+класс A» — не соответствует коду: `checkSpecs` (`scripts/process-gate.mjs:311-312`,
+`if (!c.classes.has('A') || c.isRelease) continue;`) проверяет только коммиты,
+несущие класс A. У этой задачи класса A нет вовсе (только B: `process.yml`,
+`scripts/**`; и C: `docs/**`) — проверка 3 не сработала бы независимо от
+метки. Причина, которой оправдан выбор трека, ложная.
+
+**Последствие.** Метка `small` снижает бюджет циклов ревью ТЗ с 4 до 2 (§4:
+«Для лёгкого трека лимит ревью ТЗ — 2 цикла») для задачи, которую сам автор
+характеризует как трёхповерхностную и полнотрековую по существу — при том что
+именно эта задача переписывает структурные гарантии всего конвейера ревью.
+Обратное несоответствие (`trivial`/`small` — обоснование не выбора, а отказа
+от него, #338) здесь работает в неверную сторону: критерий нарушен и назван
+самим автором, но метка не снята.
+
+**Рекомендация.** Либо снять `small` (раз задача и без него не требует файла
+ТЗ по факту отсутствия класса A — необходимости в метке нет вообще), либо
+получить явное решение владельца принять урезанный бюджет циклов для этой
+конкретной задачи, зафиксировав его в issue.
+
+### Low-1. У1 содержит неточное утверждение о текущем коде (не блокирует, для протокола)
+
+**Файл:** тело issue #517, «Уточнения S2», пункт У1.
+
+У1 утверждает: «на `spec` шага `material` нет (он под `if: stage ==
+'code'`-веткой ребейза), понадобится снимать хеш в отдельном шаге,
+общем для обоих этапов». Проверка `process.yml` показывает обратное: шаг
+`material` (`id: material`, строка 403) условен ТОЛЬКО на `steps.rebase.outputs.conflict
+!= 'true'`; шаг `rebase`, который единственный несёт условие `stage=='code'`,
+на этапе `spec` просто не выполняется, и его output пуст — а пустая строка `!=
+'true'` истинна. Значит, шаг `material` **уже выполняется и на этапе spec**
+(это же ревью — прямое доказательство: якорь ниже в этом документе получит
+дерево/SHA, снятые этим самым шагом на spec-прогоне). Задача не блокируется
+этим — решение, добавлять ли новый шаг `issue_body` или расширить уже
+существующий `material` дополнительным выводом, относится к «чего пользователь
+не наблюдает» (§7.1) и свободно решается разработчиком. Но фраза «всё, на что
+опирается решение, сверено с кодом» в S2 не вполне точна: там же назван
+несуществующий `§10.5 п.3` (реальный номер — §10.2 п.3, «Что проверяет
+process-gate.mjs») и несуществующая функция `acceptanceFrom` в
+`task-packet.mjs` (реальный механизм — тернарник `acSource` плюс
+`extractAcceptanceCriteria`, поведение при этом описано верно). Снимается
+записью: исправить формулировку У1 и, если сохраняется решение о новом шаге
+`issue_body`, явно обосновать его отдельно от ошибочной посылки «шага нет».
+
+## Что проверено и корректно
+
+- AC1, AC3, AC4, AC5 — однозначны, доказательство названо для каждого
+  (unit/тест на фикстурах/тест на текст), формулировки не оставляют места для
+  трактовки.
+- Обязательные разделы §7.1 присутствуют; неприменимые (UX, модель
+  данных/миграция, i18n, производительность) явно помечены неприменимыми с
+  обоснованием («изменения только в конвейере... продукт и словари не
+  затрагиваются»), а не молча пропущены.
+- Блок принятых предположений оформлен по правилу (У4 — «принято
+  предположительно, менять свободно»), продуктовых открытых вопросов
+  владельцу нет и не должно быть: персона задачи — автор ТЗ/ревьюер, не
+  пользователь продукта (`docs/SCOPE.md` эту поверхность не описывает и не
+  обязан); единственный «технический вопрос» (нормализация тела) автор
+  корректно закрыл сам, не эскалируя владельцу — соответствует правилу «на
+  этапе аналитики технические вопросы не задаются».
+- Обратная совместимость формата якорей (У3: новая строка `Тело issue:`
+  независима от `specs`, старые документы читаются как раньше через
+  `anchorIssueBodyFrom → null`) подтверждена по образцу уже существующих
+  `anchorTreeFrom`/`anchorVerdictFrom`, которые именно так и устроены.
+- Откат описан конкретно (перечислены все файлы), риск лимита тела GitHub
+  (65 536 знаков) сопоставлен с реальным максимумом ТЗ проекта (21 КБ,
+  `docs/specs/162-*.md`) — не голословно.
+- «Не входит» и скоуп/не-скоуп разделены явно, смешанных вопросов владельцу не
+  задано.
+- SCOPE.md намеренно не применяется как рамка приёмки: задача не продуктовая
+  (`User-Visible: no`, персона — внутренний процесс), это соответствует
+  собственному правилу PROCESS.md об инфраструктурных задачах, а не пробел.
+
+## Чего не проверял
+
+- Не прогонялись `typecheck`/`test`/`build`/`process-gate` — кода нет, ветки
+  `issue/517-*` не существует на момент ревью; на этапе ТЗ это гейты
+  следующего этапа.
+- Не проверялась синтаксическая корректность будущих правок YAML
+  (`process.yml`) — их ещё не существует; проверялась только текущая, ДО
+  правки, структура шагов, на которой основаны находки Medium-1/Low-1.
+- Не оценивалось качество мутационных гардов (`review-anchor-drops-issue-body`
+  и т.д.) сверх того, что они названы и привязаны к конкретным чистым функциям
+  — их реализация появится в коде, а не в ТЗ.
+
+## Вердикт
+
+Жёлтый. High: 0, Medium: 2 (оба в скоупе задачи, чинятся в ней же, без
+повторного ревью после исправления по существу — только новый заход того же
+цикла ТЗ). Low: 1, снимается записью в этом документе, отдельного действия не
+требует.
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `dev`, коммит `a824acc1b18a` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `91c557112388667494babbaaa83f5f6a75195628`
+  ```
+  git log --all --format='%H %T' | grep 91c557112388
+  ```
+- Вердикт конвейера: `yellow` · High 0