mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -0,0 +1,208 @@
|
||||
# SPEC-REVIEW-472-r1
|
||||
|
||||
Этап: spec (PROCESS.md §2.4) · Трек: лёгкий (`small`) · Заход r1 · блокирующих циклов 0/2
|
||||
SHA на момент ревью: `615045cb1882fa36164a55d544dc6520f2f58618` (dev)
|
||||
Ревьюер ≠ автор ТЗ.
|
||||
|
||||
## Скоуп
|
||||
|
||||
Issue #472: еженедельный полный мутационный прогон (`mutation-gate.yml`) может
|
||||
падать/отменяться без адресата — нет прав на issue, нет шага на отказ, одна
|
||||
concurrency-группа разрешает ручному прогону убить расписание. ТЗ в теле issue
|
||||
(лёгкий трек), контракт из 6 пунктов, AC1–AC8, два мутанта на
|
||||
`scripts/mutation-gate.mjs`. Продукт (карточка, фронтенд, бэкенд-интеграция) не
|
||||
затронут — чистая CI/process-задача.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
- Прочитаны: тело issue #472, оба комментария (`S2 — разбор`, `ТЗ готово`).
|
||||
- Сверено с фактическим состоянием репозитория на `615045cb`:
|
||||
`.github/workflows/mutation-gate.yml`, `.github/workflows/validate.yml`
|
||||
(шаг `workflow_sync`), `.github/workflows/announce.yml` (образец
|
||||
Telegram-шага), `scripts/mutation-gate.mjs` (реестр, `runMutant`,
|
||||
`runCleanGuards`, `main()` — ветки `--shard`/`--check`/`--build-only`),
|
||||
`test/mutation-gate.test.mjs` (существующее покрытие), стиль контрактных
|
||||
тестов на YAML в `test/gate-reuse.test.mjs` (прецедент: строковые
|
||||
проверки, без YAML-парсера — в `package.json` его и нет).
|
||||
- Проверена модель прав GitHub Actions (`permissions:` на уровне job
|
||||
заменяет, а не дополняет уровень workflow) против прецедентов в самом
|
||||
репозитории (`validate.yml:38,40` — `actions: read` явно объявлен для job,
|
||||
которой нужен список прогонов; job `smoke`/`frontend`, качающие артефакты
|
||||
через `download-artifact`, не имеют собственного `permissions:` и потому
|
||||
наследуют **неограниченный** дефолт `validate.yml`, где верхнего
|
||||
`permissions:` нет вовсе — в отличие от `mutation-gate.yml`, где верхний
|
||||
`permissions: contents: read` уже ограничивает всё, что не названо).
|
||||
- docs/SCOPE.md, docs/USER-GUIDE.ru.md, канонические документы подсистем
|
||||
(SUN/LIGHT/CANVAS/WALL-THICKNESS/UX-MODES/CONFIG-COMPATIBILITY/
|
||||
TOUCH-SUPPORT) — не применимы, задача не трогает продукт и видимое
|
||||
пользователю поведение. Первым вопросом («какую строку Core user jobs
|
||||
закрывает задача») можно пренебречь: это не продуктовая, а процессная/
|
||||
инфраструктурная задача, и §5 лёгкого трека такие явно допускает
|
||||
(«одна поверхность» — сам workflow и его репортёр).
|
||||
- Файла в `docs/specs/` не создано — верно для `small` (§2.3).
|
||||
|
||||
## Лёгкий трек: критерии §5 — все выполнены
|
||||
|
||||
Подтверждаю разбор автора в S2: сложность/риск низкие (один workflow + один
|
||||
новый чистый скрипт + контрактные тесты), одна поверхность, нет миграции
|
||||
конфига, нет нового UX-контракта (пользователь продукта ничего не видит), нет
|
||||
влияния на perf/touch. Отказа от лёгкого трека не требуется.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе задачи) — 2
|
||||
|
||||
**M1. AC3 не отличает «сбежавший мутант» от «чистый прогон уже красный» —
|
||||
риск ложного/потерянного сигнала в самом отчёте, который эта задача обязана
|
||||
делать надёжным.**
|
||||
|
||||
- Файл: `scripts/mutation-gate.mjs`, функция `runMutant` (строка 6759) и
|
||||
`runCleanGuards` (строки 6771–6793); контракт — тело issue #472, раздел
|
||||
«Контракт», пункт 3, и AC3.
|
||||
- Воспроизведение (по коду, не по прогону — «проверено чтением»): шаг
|
||||
«Каждый тест ловит свою поломку» вызывает `node scripts/mutation-gate.mjs
|
||||
--shard=N/4`. Внутри `main()` (строка 6897) **до** цикла по мутантам
|
||||
всегда вызывается `runCleanGuards(selected)` — прогон каждого уникального
|
||||
`guard` без мутации. Если хоть один guard уже красный без мутанта (флеки,
|
||||
сломанное окружение, забытый мердж-конфликт в тесте), печатается
|
||||
```
|
||||
FAIL чистый прогон: <guard-команда> красный без мутанта
|
||||
```
|
||||
и функция возвращает `false` раньше, чем напечатается хоть одна строка
|
||||
`FAIL <id>: тест остался зелёным на сломанном коде` — то есть эта форма
|
||||
реально попадает в тот же лог, что улетит артефактом (AC2), и относится к
|
||||
той же самой невыполненной задаче: «шард отказал, назови почему».
|
||||
Контракт AC3 описывает парсинг как «отсортированный список id из
|
||||
`FAIL`-строк всех шардов» — этого недостаточно, чтобы отличить
|
||||
`FAIL чистый прогон: …` от `FAIL <mutant-id>: …`: если репортёр наивно
|
||||
берёт первый токен после `FAIL `, он получит фиктивный «id» вроде
|
||||
`чистый` (или `чистый прогон`, в зависимости от реализации), которого нет
|
||||
в реестре — а значит нет и `guard`-а для таблицы, и команда «что делать»
|
||||
`node scripts/mutation-gate.mjs --id=чистый` в письме владельцу будет
|
||||
синтаксически некорректной (реестр ответит «мутант «чистый» не объявлен»).
|
||||
Если же вместо этого такую строку молча отбросить без специального
|
||||
правила — реальная поломка (тест уже красный без всякого мутанта) исчезнет
|
||||
из отчёта так же тихо, как исчезал изначальный сигнал, из-за которого
|
||||
заведена вся задача.
|
||||
- Все реальные id мутантов — строго `[a-z0-9-]+` (проверено:
|
||||
`grep -c "id: '" scripts/mutation-gate.mjs` → 509 совпадений, ни один не
|
||||
содержит кириллицу/пробел), так что различение технически дёшево — но ТЗ
|
||||
не называет правило, и заявленные фикстуры юнита («три лога») не
|
||||
гарантируют, что этот случай будет вообще замечен реализацией.
|
||||
- Почему это не «додумать самому за автора»: правило разбора лога — решение
|
||||
не продуктовое (агенты решают такие сами, §7.1), но оно прямо определяет,
|
||||
выполняется ли AC3 и AC4 корректно, поэтому это находка ревью, а не
|
||||
тихое согласие.
|
||||
- Что чинит: одна фраза в контракте — например, «строки `FAIL чистый прогон:
|
||||
…` не считаются сбежавшим мутантом; шард получает статус `failed` без
|
||||
записи в `escaped`, само сообщение уходит в отдельное поле/строку отчёта
|
||||
(broken guard)» — плюс лог-фикстура на этот случай в AC3.
|
||||
|
||||
**M2. AC5 называет для job `report` только `permissions: issues: write` —
|
||||
как буквально описано, job не сможет скачать артефакты шардов.**
|
||||
|
||||
- Файл: тело issue #472, раздел «Контракт», пункт 4 («Job `report`»), и
|
||||
таблица AC, строка AC5.
|
||||
- В GitHub Actions `permissions:`, объявленный на уровне job, **заменяет**
|
||||
права workflow целиком для этой job, а не дополняет их (документированное
|
||||
поведение; в этом же репозитории есть подтверждающий прецедент:
|
||||
`validate.yml:38-40` явно прописывает `actions: read` для job `changes`,
|
||||
которой нужен список прогонов, тогда как job `smoke`/`frontend`, которые
|
||||
тоже вызывают `actions/download-artifact`, никакого собственного
|
||||
`permissions:` не имеют и получают безлимитный дефолт `validate.yml` —
|
||||
в этом файле верхнего `permissions:` нет вовсе). У `mutation-gate.yml`,
|
||||
наоборот, верхний `permissions: contents: read` уже есть (строки 31–32),
|
||||
так что job `report` с одним `issues: write` получит **только** это право
|
||||
и ничего сверх.
|
||||
- Job `report` по контракту «скачивает артефакты всех шардов» —
|
||||
`actions/download-artifact` обращается к Actions REST API, которая требует
|
||||
`actions: read`; без него шаг вернёт 403 независимо от того, что репозиторий
|
||||
публичный (это разграничение GitHub API, а не git-протокола). Ей также
|
||||
нужен код репозитория, чтобы выполнить `node
|
||||
scripts/mutation-gate-report.mjs` и прочитать реестр мутантов из
|
||||
`scripts/mutation-gate.mjs` — то есть checkout, для которого обычно
|
||||
указывают `contents: read` (на публичном репозитории он, возможно,
|
||||
отработает и без явного права, но называть это гарантией не стоит).
|
||||
- Последствие: реализация «по букве AC5» ломает job `report` на первом же
|
||||
реальном отказе по расписанию — то есть именно тогда, когда его сигнал
|
||||
нужен. Это новый вариант того самого «у отказа нет адресата», ради
|
||||
которого заведена вся задача, просто на другом шаге.
|
||||
- Что чинит: расширить AC5 — `permissions: { contents: read, actions: read,
|
||||
issues: write }` (или явно объяснить, почему `contents` не нужен, если
|
||||
автор проверит, что публичный checkout работает без него).
|
||||
|
||||
### Low — 1 (не блокирует, на усмотрение автора)
|
||||
|
||||
**L1. Неверный номер строки в разделе «Проблема».** Текст «checkout берёт
|
||||
`ref: dev` (`mutation-gate.yml:33`)» — на `615045cb` эта строка (шаблон
|
||||
`${{ github.event_name == 'workflow_dispatch' && inputs.ref || 'dev' }}`)
|
||||
находится на строке 52, а не 33 (33–36 — блок `concurrency`). Утверждение по
|
||||
существу верное (checkout действительно форсирует `dev` вне
|
||||
`workflow_dispatch`), путаница только в ссылке — не влияет ни на один AC,
|
||||
править по усмотрению автора.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Все три «конструктивные причины» из «Проблема» подтверждены построчно:
|
||||
`permissions: contents: read` — строки 31–32; отсутствие шага на отказ —
|
||||
в файле действительно нет `if: failure()`/уведомлений; одна
|
||||
concurrency-группа `mutation-gate` с `cancel-in-progress: true` — строки
|
||||
34–36.
|
||||
- AC1 (раздельные concurrency-группы по `github.event_name`) — реализуемо
|
||||
контрактным тестом на строку `group:`, других триггеров у workflow нет
|
||||
(только `workflow_dispatch` и `schedule`), двузначности нет.
|
||||
- AC8 и пункт 5 контракта — сверено с `validate.yml`: шаг `workflow_sync`
|
||||
(строки 61-77) сегодня действительно сверяет только `process.yml` через
|
||||
`git show origin/main:… origin/dev:…`; расширение на второй файл тем же
|
||||
diff'ом тривиально и не пересекается с M1/M2.
|
||||
- AC7 (Telegram необязателен) — паттерн уже есть в `announce.yml` (тот же
|
||||
`curl`, секреты через `env`, `set -euo pipefail`); контракт прямо
|
||||
ссылается на этот файл, реализуемо без новых допущений.
|
||||
- Формат `FAIL <id>: тест остался зелёным на сломанном коде` в AC3/AC4
|
||||
дословно совпадает с реальной строкой в коде (`mutation-gate.mjs:6759`) —
|
||||
сам факт этой конкретной строки не выдуман, выдумано (точнее, недосказано)
|
||||
только то, что она не единственная форма `FAIL` в том же логе (см. M1).
|
||||
- Два новых мутанта (`mutation-report-drops-missing-shard`,
|
||||
`mutation-report-duplicates-escaped`) целятся именно в AC3 и укладываются
|
||||
в существующий формат реестра `scripts/mutation-gate.mjs`.
|
||||
- Откат — одним коммитом, без миграции данных; согласуется с «Что не
|
||||
меняется» (реестр, шардирование, `--changed`, права `mutants`).
|
||||
- Критерии лёгкого трека §5 — все пять выполнены одновременно, отказа от
|
||||
трека не требуется (см. выше).
|
||||
- Файл в `docs/specs/` не создан, соответствует правилу `small` (§2.3).
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не проверял, действительно ли `actions/checkout` на публичном репозитории
|
||||
отработает без явного `contents: read` — сослался на общую практику GitHub
|
||||
(github-token без прав всё ещё может анонимно клонировать публичный git,
|
||||
но это не то же самое, что «гарантированно работает»), это дополнение к
|
||||
M2, а не отдельная находка, автор может закрыть её явной проверкой в коде.
|
||||
- Не прогонял `npx tsc --noEmit`/`npm test`/`npm run build` — Validate уже
|
||||
зелёный на этом же SHA `615045cb` (упомянутый в контексте прогон), а
|
||||
задача ещё в `S4-spec-review`: кода для этих гейтов пока нет, реализации
|
||||
не существует. Это ревью ТЗ, а не код-ревью.
|
||||
- Не запускал `node scripts/mutation-gate.mjs` живьём и не эмулировал
|
||||
`--shard` с искусственно сломанным guard — вывод для M1 получен чтением
|
||||
функций `runMutant`/`runCleanGuards`/`main`, а не исполнением; поведение
|
||||
однозначно следует из порядка вызовов в коде (`runCleanGuards` выполняется
|
||||
и может завершить `main` раньше цикла мутантов).
|
||||
- Не проверял golden/смоки/инварианты модели/performance-профили — diff не
|
||||
существует (ТЗ, не код), ни один из этих гейтов неприменим на этом этапе.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Обе находки — Medium **в скоупе задачи** (AC3 и AC5 самого текущего
|
||||
контракта), High нет. По правилу §2.4/§5 это жёлтый вердикт: автор правит
|
||||
текст ТЗ (уточняет разбор `FAIL`-строк в контракте+AC3, расширяет
|
||||
`permissions` в контракте+AC5), фикс проходит второй заход в пределах
|
||||
лимита 2 циклов лёгкого трека.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Якоря снять не удалось: ветки задачи нет, материал читался по `dev`.
|
||||
Reference in New Issue
Block a user