mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-03 13:18:58 +00:00
@@ -0,0 +1,148 @@
|
||||
# CODE-REVIEW-700-r2
|
||||
|
||||
Issue: #700 · Предполёт не красит ветку задачи чужими причинами: зеркало workflow и внешние ссылки
|
||||
Этап: code · Заход: r2 · Трек: show · Материал: `ed3d63e20db24fe9172c83f7ea2e824c90444dcc`
|
||||
(коммит `ed3d63e2` поверх материала r1 `89d57e054cdf940d3ead83ec509075164118ee3b`,
|
||||
который сам лежит поверх `origin/dev` @ `c716bb0f63104f5afb9d660bc058e095256d433d`;
|
||||
на момент этого раунда `origin/dev` ушёл на 8 коммитов вперёд, слияние без
|
||||
конфликта — по треку `show` ветка к `dev` не приводится, материал — ветка как есть).
|
||||
|
||||
## Скоуп раунда
|
||||
|
||||
Единственная цель r2 — закрыть единственную находку r1 (Medium, в скоупе):
|
||||
`gh issue list … || true` в шаге «Расхождение зеркала на dev — issue
|
||||
владельцу» (`.github/workflows/validate.yml`) глушил сбой чтения списка
|
||||
открытых issue в пустую строку и проваливался в `gh issue create`, заводя
|
||||
дубликат `[workflow-sync]` даже когда нужное issue уже открыто, но недоступно
|
||||
для чтения (сеть, рейт-лимит).
|
||||
|
||||
Дельта r1→r2 — ровно один коммит `ed3d63e2`, три файла:
|
||||
|
||||
```
|
||||
.github/workflows/validate.yml | 9 +++++++--
|
||||
scripts/mutation-registry.mjs | 11 +++++++++++
|
||||
test/validate-workflow.test.mjs | 3 +++
|
||||
```
|
||||
|
||||
Дельта локальна и пропорциональна находке (одна правка на одну строку внутри
|
||||
уже одобренного шага): AC, которые она задевает, — только тот же самый AC про
|
||||
«одно issue, не дубликат» из тела #700 и контракта PROCESS.md §10.4. Остальной
|
||||
диапазон (`check-docs.mjs`, `PROCESS.md`, `test/classify-changes.test.mjs`,
|
||||
`test/docs-freshness.test.mjs`) не тронут этим коммитом — соответствующая
|
||||
часть r1 наследуется без повторной проверки (раздел ниже).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Дешёвые гейты подтверждены зелёным Validate на этом же SHA `ed3d63e2`
|
||||
(workflow_dispatch, run `36484635253`, `conclusion: success`, ссылка дана в
|
||||
задаче ревью) — `npx tsc --noEmit`, `npm test`, `npm run build` +
|
||||
`bundle-policy --verify` повторно не гонял.
|
||||
|
||||
Дополнительно к зелёному Validate прогнал сам:
|
||||
|
||||
| Гейт | Результат |
|
||||
|---|---|
|
||||
| `node --test test/validate-workflow.test.mjs` | 27/27 зелёных, включая изменённый `#700: на ветке задачи…` |
|
||||
| `node --test test/classify-changes.test.mjs` | 21/21 зелёных (не тронут этим коммитом, для очистки дельты) |
|
||||
| `node --test test/mutation-gate.test.mjs` | 69/69 зелёных, структура нового мутанта `workflow-sync-issue-duplicated-on-read-failure` валидна |
|
||||
| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | «Исполняемого frontend-диффа нет» — src/**/*.ts не тронут, браузерные смоки нечего выбирать |
|
||||
| Мутация вручную из реестра: `exit 0` → `:` в новом `if`-блоке (patch `workflow-sync-issue-duplicated-on-read-failure`), тест `--test-name-pattern="#700: на ветке задачи"` | падает (1 fail, `assert.doesNotMatch` на `|| true)` и/или `assert.match` на новый `if !`-блок), файл восстановлен из бэкапа, `git status` чист |
|
||||
|
||||
Живое воспроизведение реального сбоя `gh issue list` на CI не запускал (это
|
||||
живой internal API-вызов на push в `dev`, вне доступа из этой сессии, и это
|
||||
разрушительное действие, если сорвётся не так, как задумано) — как и в r1,
|
||||
логика закрыта юнит-тестом плюс ручной мутацией guard-команды выше.
|
||||
|
||||
## Находки
|
||||
|
||||
Нет. Правка r2 точна: устраняет ровно описанный в r1 путь, не расширяет и не
|
||||
сужает ничего другого в шаге.
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка r1 | Чем закрыта | Где это видно |
|
||||
|---|---|---|
|
||||
| Medium: `gh issue list … \|\| true` — сбой чтения списка открытых issue проваливается в `gh issue create`, заводя дубликат `[workflow-sync]` вместо предупреждения | `\|\| true` убран; чтение обёрнуто в `if ! existing=$(gh issue list …); then echo "::warning::…"; exit 0; fi` — при ненулевом коде `gh issue list` шаг предупреждает и выходит, не доходя ни до `gh issue comment`, ни до `gh issue create` | `.github/workflows/validate.yml` (коммит `ed3d63e2`, блок шага «Расхождение зеркала на dev — issue владельцу»); поведение зафиксировано тестом `test/validate-workflow.test.mjs:628-633` (два новых `assert`: на форму `if !…then…exit 0…fi` и на отсутствие `\|\| true)` до `gh issue create`) и мутантом `workflow-sync-issue-duplicated-on-read-failure` в `scripts/mutation-registry.mjs`, который я применил вручную — тест краснеет на мутированном коде и зеленеет на исходном (таблица гейтов выше) |
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Принято без повторной проверки в этом раунде — документ и материал:
|
||||
`docs/reviews/CODE-REVIEW-700-r1.md` (в дереве `545be665`), материал
|
||||
`89d57e054cdf940d3ead83ec509075164118ee3b`, дерево материала `945dad882326…`:
|
||||
|
||||
- Периметр `task_branch=true/false` не сузился: `check`/`advise` для
|
||||
workflow_sync и `--external`/без `warn` для внешних ссылок остаются красными
|
||||
для push в `dev` и для кандидата беты/релиза; предупреждением становится
|
||||
только ветка задачи.
|
||||
- `validate.yml` не входит в список из шести тонких зеркалируемых файлов —
|
||||
этому диффу зеркалирование в `main` не требуется.
|
||||
- `check-docs.mjs`: `EXTERNAL_WARN` корректно разводит `warnings`/`errors`,
|
||||
выход по `errors.length` не тронут.
|
||||
- Трейлеры коммита r1 (`Issue: #700`, `User-Visible: no`) — корректны, диффу
|
||||
класса A/D нет.
|
||||
- `PROCESS.md` §10.4 дополнен точно тем, что реализовано в r1.
|
||||
- Оба мутанта r1 (`task-branch-workflow-sync-red-again`,
|
||||
`external-link-warn-mode-ignored`) — краснеют на мутации, зеленеют на
|
||||
исходнике (проверено в r1 вручную).
|
||||
- `actionlint` не перепроверялся (недоступен локально и в r1, и сейчас);
|
||||
косвенно — YAML реально исполнился на CI (run r1 `36482200722` и run r2
|
||||
`36484635253`) без ошибки парсинга.
|
||||
|
||||
## Что проверено и корректно (r2, сверх наследования)
|
||||
|
||||
- Новый `if`-блок синтаксически и семантически корректен для `bash -eo
|
||||
pipefail` (шелл GitHub Actions по умолчанию): присваивание внутри условия
|
||||
`if !` не триггерит `set -e`, а код возврата, который проверяется, — код
|
||||
возврата `gh issue list` (json/jq считает сам `gh`, не отдельный процесс в
|
||||
пайпе), так что сетевой сбой или рейт-лимит действительно попадает в ветку
|
||||
`if`, а не проскакивает мимо неё.
|
||||
- Ветка «не прочитано → предупреждение и выход» физически предшествует и
|
||||
`gh issue comment`, и `gh issue create` — при сбое чтения оба этих вызова
|
||||
теперь не выполняются вообще, то есть дубликат исключён, а не просто
|
||||
переименован в другую ошибку.
|
||||
- Комментарий в коде (`# Список не прочитан — …`) объясняет неочевидную
|
||||
причину (почему это не «оставить || true»), а не пересказывает код — по
|
||||
стилю совпадает с уже принятыми в r1 комментариями этого же шага.
|
||||
- Новый мутант в реестре (`workflow-sync-issue-duplicated-on-read-failure`)
|
||||
минимален и целится ровно в закрытый путь (`exit 0` → `:`, единственная
|
||||
правка, из-за которой шаг снова провалился бы в `if [ -n "$existing" ]`
|
||||
с пустым `$existing`).
|
||||
- Изменение не расширяет и не меняет скоуп задачи: файлов класса A/D нет,
|
||||
видимое поведение карточки не меняется, `docs/USER-GUIDE.ru.md` не
|
||||
требует правок.
|
||||
- Одно число, один источник (§8): числовых значений, видимых пользователю
|
||||
карточки, дифф r2 не вводит.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- `mutation-gate --check` (полный прогон, а не структура) и весь `npm run
|
||||
gate:small` — дешёвые гейты уже подтверждены зелёным Validate на этом SHA,
|
||||
трек `show` не запрашивает мутанты по диффу целиком (#696).
|
||||
- Живое исполнение шага «Расхождение зеркала на dev — issue владельцу» с
|
||||
реально упавшим `gh issue list` на push в `dev` — не воспроизводил (нужен
|
||||
реальный сбой GitHub API или подмена `gh` внутри Actions-раннера, недоступно
|
||||
из этой сессии); логика закрыта юнит-тестом плюс ручной мутацией, как в r1.
|
||||
- `golden:verify`, `pytest tests_backend`, инварианты модели, performance —
|
||||
неприменимо, дифф всего диапазона не трогает `src/**`, рендер, геометрию или
|
||||
Python (подтверждено ещё в r1, r2 диапазон это не меняет).
|
||||
|
||||
## Вердикт
|
||||
|
||||
Единственная находка r1 закрыта точной правкой с тестом и проверенным
|
||||
мутантом; новых находок нет.
|
||||
|
||||
---
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/700-preflight-warnings`, коммит `ed3d63e20db2` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `9a8052d18a3baa6cadd0907021a3a01dfc11d935`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 9a8052d18a3b
|
||||
```
|
||||
- Тело issue: `a0c40d40c1401844c3d04ff0695dd347e76172d990004ea2799cc4df38729fce`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user