From 6842b69eed1a6530d7475f4427132b4a1b501e49 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Wed, 9 Sep 2026 14:20:48 +0000 Subject: [PATCH] docs: review document for #511 Issue: #511 User-Visible: no --- docs/reviews/SPEC-REVIEW-511-r1.md | 166 +++++++++++++++++++++++++++++ 1 file changed, 166 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-511-r1.md diff --git a/docs/reviews/SPEC-REVIEW-511-r1.md b/docs/reviews/SPEC-REVIEW-511-r1.md new file mode 100644 index 00000000..9c618915 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-511-r1.md @@ -0,0 +1,166 @@ +# SPEC-REVIEW-511-r1 + +- Issue: #511 — «release-gate: судить по последнему завершённому прогону на SHA, отменённые не блокируют» +- Этап: ТЗ на ревью (PROCESS.md §2.4), лёгкий трек (`small`) +- Заход: r1 · блокирующих циклов израсходовано 1 из 2 (этот раунд — жёлтый, бюджет тратится) +- Вердикт: **жёлтый** + +## Скоуп + +Issue пуре-инфраструктурная (класс B по AGENTS.md: `scripts/release-gate.mjs`, +`test/release-gate.test.mjs`), продуктового кода не касается, `docs/SCOPE.md` +неприменим напрямую — задача не закрывает и не ломает ни один Core user job, +это правка гейта публикации релиза. Автор (Codex) уже прогнал её через +`S2-analysis` по нормальному циклу (не через инфраструктурный обход Claude), +трек `small` выбран корректно: одна поверхность, риск 2/10, нет +конфига/i18n/перфа/touch — критерии §5 совпадают. + +ТЗ живёт в теле issue (верно для `small`): «Проблема» → «Решение владельца» → +AC1–AC3. + +## Как проверялось + +Кода ещё нет (этап ТЗ) — проверка велась чтением текущего репозитория и +сверкой утверждений ТЗ с реальным деревом: + +- прочитан `scripts/release-gate.mjs` — текущая `classifyValidateRuns` + подтверждает описанный в «Проблеме» баг: любой `completed` прогон с + `conclusion !== 'success'` (включая `cancelled`) переводит гейт в `fail` + независимо от порядка по времени; +- прочитан `test/release-gate.test.mjs` — существующий тест + `release gate fails closed for red, cancelled and skipped runs` прямо + проверяет, что `cancelled` даёт `fail`; новый AC1 этот тест переворачивает + для `cancelled` — ожидаемо, будет переписан в реализации, самой ТЗ это не + вредит; +- прочитан `.github/workflows/release.yml` (строки 43, 52) — оба вызова + `release-gate.mjs` (Validate и `performance.yml`/Full Performance) идут + через один и тот же `classifyValidateRuns`, так что AC2 («release.yml не + меняется») структурно верен; +- **AC3 сверен с деревом `docs/`**: `find docs -iname "*performance*"` — + файла `docs/performance/README.md` **не существует** ни в каком виде + (есть только `docs/specs/*performance*.md` — черновики специфичных задач, + не общая документация гейта); +- `grep -n -i "gate\|exact-sha"` по `docs/TESTING.md` — документ существует, + но не содержит ни одной фразы о семантике «последний завершённый прогон» + сейчас; это чек-лист AC по фичам, не общее описание релизного гейта; +- прочитан `docs/DEVELOPMENT.md:353-370` — вот где живёт релевантный текст + сегодня: «*A missing, failed, cancelled or one-hour-timed-out Validate + withholds the asset*» (строка 360). После реализации AC1 это утверждение + станет **фактически неверным** для `cancelled` — отменённый прогон + перестанет удерживать ассет, если рядом есть более поздний зелёный. + +Продуктовых команд (`typecheck`/`test`/`build`) не гонял: на этапе ТЗ кода +нет, гонять нечего. + +## Находки + +### Medium (в скоупе задачи) — AC3 указывает не туда, реальный источник искажения не назван + +**Файл:** тело issue #511, раздел AC, пункт AC3. + +**В чём дефект.** AC3 требует добавить фразу о новой семантике в +`docs/performance/README.md` **или** `docs/TESTING.md`. Первого файла не +существует (`find docs -iname "*performance*"` его не находит) — ТЗ ссылается +на документ, которого нет в дереве. Второй существует, но по грепу не +содержит сейчас никакого описания семантики релизного гейта и структурно +устроен как чек-лист AC по фичам, а не как общее описание процесса релиза — +топически не то место. + +При этом реальный текст, который **станет неверным** после реализации AC1, +уже есть и назван неверно: `docs/DEVELOPMENT.md:360` прямо говорит «cancelled +… Validate withholds the asset» — а по новому контракту (AC1) отменённый +прогон удержания не даёт, если рядом есть более поздний завершённый. Этот +файл в AC3 не упомянут вообще. + +**Сценарий отказа.** Исполнитель читает AC3 буквально: пытается открыть +`docs/performance/README.md` — файла нет, значит остаётся только +`docs/TESTING.md`, куда добавляется одна фраза не по месту. AC3 формально +закрыт (фраза добавлена, гейт `check-docs` не тронут — `docs/TESTING.md` не +входит в скриншотный отпечаток). Но `docs/DEVELOPMENT.md:360` остаётся +описывать старую семантику («cancelled withholds the asset»), которая с этого +момента прямо противоречит поведению кода. Канонический документ процесса +релиза лжёт следующему читателю (агенту или владельцу), причём именно там, +где документация должна была объяснить свежую про-cancelled логику — то есть +AC3 не достигает своей цели, будучи формально выполненным. + +**Почему в скоупе, не Medium вне-скоупа.** Правка одной фразы в правильном +файле — часть этой же задачи (документация семантики гейта — прямое +следствие AC1), не соседнее поведение. + +**Что чинить.** Переформулировать AC3: заменить или дополнить список целевых +файлов на `docs/DEVELOPMENT.md` (минимум — исправить строку 360, которая +после реализации станет ложной), убрать несуществующий +`docs/performance/README.md`. `docs/TESTING.md` может остаться вторым +вариантом, если там действительно логично держать пояснение, но он не +обязан быть единственным кандидатом. + +## Низкие находки (сняты ревьюером с записью, не блокируют) + +1. **Раздел «откат» отсутствует в теле issue.** Шаблон лёгкого трека (§5) + требует «проблема · контракт · AC1…ACn с доказательством · **откат**» в + самом теле issue. Слово «откат» встречается только в комментарии + `S2-analysis` («откат = revert»), не в теле. Снимаю: контент тривиален и + однозначен для чистой правки скрипта + теста без состояния/миграции — + `git revert` коммита закрывает откат полностью, спорить тут не о чем. + Рекомендую автору добавить явную строку «Откат: revert коммита, + состояния/миграции нет» в тело при следующей правке (заодно с AC3), но + отдельного цикла ради этого не требую. +2. **AC2 и AC3 не называют явно способ доказательства** (unit/backend/ + smoke/golden/«ревью кода»), как того требует чек-лист DoR (§2.5). По + смыслу оба доказываются диффом при код-ревью («ревью кода»), это + единственное разумное прочтение для «файл не менялся» и «фраза в + документации», неоднозначности здесь не возникает. Снимаю с той же + рекомендацией — сформулировать явно заодно с правкой AC3. + +## Что проверено и корректно + +- AC1 полностью и однозначно описывает алгоритм: `cancelled` исключаются + целиком, из оставшихся берётся хронологически последний + (`run_started_at`/`created_at`), решение зависит только от него. + Все шесть перечисленных пар вход/выход взаимно непротиворечивы и + детерминированно выводятся из этого правила — включая пограничные случаи + (`[cancelled(newer), green(older)]` → `success`, где исключение `cancelled` + и выбор «последнего среди оставшихся» дают однозначный результат без + дополнительных допущений). Доказательство названо (`тест release-gate.test`). +- Продуктовая рамка соблюдена: изменение не расширяет скоуп, не трогает + `src/**`, не создаёт новый UX-контракт, не требует i18n/changelog + (`User-Visible: no` подразумевается корректно — правка невидима + пользователю продукта). +- AC2 проверяем и структурно подтверждён чтением `release.yml`: оба вызова + `release-gate.mjs` используют общую функцию, третьего места с отдельной + логикой в workflow нет. +- Отсутствие догадок, выданных за решение: «Решение владельца» в теле issue — + прямая цитата ретроспективы 09.09, п.5, а не домысел автора спецификации; + алгоритм AC1 полностью выводится из неё, ничего не додумано сверх текста + решения. +- Продуктовых вопросов владельцу в этой ТЗ не требуется — весь материал + технический (где хранится порядок сортировки прогонов, как называется + функция и т.п.), решается автором/ревьюером без эскалации. + +## Чего не проверял + +- Не гонял `npx tsc --noEmit`, `npm test`, `npm run build` — кода к задаче + ещё нет, гонять нечего на этапе ТЗ. +- Не проверял историческую точность инцидента 09.09 (отменённый дубль + 34354468354 и красный dispatch vs v1.72.0) — это утверждение о факте + прошлого прогона CI, не проверяемое чтением репозитория; для ревью ТЗ + достаточно того, что описанный баг воспроизводится логикой текущего + `classifyValidateRuns`. +- Не рассматривал раздел `docs/CHANGELOG.md`/`docs/CHANGELOG.ru.md` — задача + корректно `User-Visible: no`, оба changelog не требуются. + +## Итог + +Единственная содержательная находка (Medium, AC3) в скоупе задачи и чинится +без нового issue — правкой AC3 в теле #511. High-находок нет. Вердикт — +жёлтый; возврат автору на правку ТЗ, следующий цикл — по дельте. + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Якоря снять не удалось: ветки задачи нет, материал читался по `dev`. +- Вердикт конвейера: `yellow` · High 0