mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-08 07:29:13 +00:00
claude[bot]
@@ -0,0 +1,184 @@
|
||||
# CODE-REVIEW-700-r1
|
||||
|
||||
Issue: #700 · Предполёт не красит ветку задачи чужими причинами: зеркало workflow и внешние ссылки
|
||||
Этап: code · Заход: r1 · Трек: show · Материал: `89d57e054cdf940d3ead83ec509075164118ee3b` (один коммит поверх `origin/dev` @ `c716bb0f63104f5afb9d660bc058e095256d433d`)
|
||||
|
||||
## Скоуп
|
||||
|
||||
Инфраструктурная задача трека `show`: предполёт `validate.yml` не должен красить
|
||||
ветку задачи двумя внешними по отношению к её диффу причинами —
|
||||
|
||||
1. расхождением зеркала тонких вызывающих workflow (`main` vs `dev`);
|
||||
2. упавшим внешним сайтом при проверке ссылок в документации (`check-docs.mjs`).
|
||||
|
||||
Оба пункта на ветках `issue/*` становятся предупреждением в сводке, не красят
|
||||
вердикт; на push в `dev` (там, где кандидат беты и релиз фактически и
|
||||
проверяются — см. ниже) поведение остаётся прежним, красным. Плюс — на push в
|
||||
`dev` расхождение зеркала автоматически заводит одно owner-issue
|
||||
`[workflow-sync]` (или комментирует уже открытое), по образцу ночного
|
||||
мутационного гейта (#472).
|
||||
|
||||
Файлы: `.github/workflows/validate.yml`, `scripts/check-docs.mjs`,
|
||||
`scripts/mutation-registry.mjs`, `PROCESS.md` §10.4,
|
||||
`test/validate-workflow.test.mjs`, `test/docs-freshness.test.mjs`,
|
||||
`test/classify-changes.test.mjs`. Файлов класса A/D нет — согласен с автором.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Дешёвые гейты (typecheck/test/build/bundle-policy) подтверждены зелёным
|
||||
Validate на этом же SHA (workflow_dispatch, `issue/700-preflight-warnings`,
|
||||
run `36482200722`, `conclusion: success`) — не перегонялись повторно.
|
||||
|
||||
Дополнительно к зелёному Validate прогнал сам:
|
||||
|
||||
| Гейт | Результат |
|
||||
|---|---|
|
||||
| `node --test test/validate-workflow.test.mjs` | 27/27 зелёных |
|
||||
| `node --test test/classify-changes.test.mjs` | 21/21 зелёных |
|
||||
| `node --test test/docs-freshness.test.mjs` | 3/3 зелёных |
|
||||
| `node --test test/mutation-gate.test.mjs` (структура реестра + исполнение guard-команд обоих новых мутантов среди прочих) | 69/69 зелёных |
|
||||
| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | «Исполняемого frontend-диффа нет (`src/**/*.ts` не тронут)» — браузерные смоки нечего выбирать, не пропуск |
|
||||
| Мутация вручную: `advise` → `check` в `.github/workflows/validate.yml`, тест `#700: на ветке задачи…` | падает (`1 fail`), восстановлено |
|
||||
| Мутация вручную: `externalErrors = EXTERNAL_WARN ? warnings : errors` → `= errors` в `scripts/check-docs.mjs`, тест `#700: check-docs…` и `docs-freshness.test.mjs` | оба падают, восстановлено |
|
||||
|
||||
Также прочитал живой лог `run 36482200722`, job «Предполёт»: на этом прогоне
|
||||
`REF=refs/heads/issue/700-preflight-warnings` реально прошёл веткой
|
||||
`--external=warn` (`скриншоты документации: режим warn` →
|
||||
`Documentation checks passed (7 files, 12 external links)`), а шаг «Вердикт
|
||||
предполётных проверок» реально выполнил `task_branch=true` и напечатал
|
||||
`ok тонкие вызывающие workflow в main и dev` через `advise` (все шесть
|
||||
файлов зеркала совпали, поэтому и `advise`, и `check` в этом прогоне дали бы
|
||||
одинаковый «ok» — само разветвление логики живьём не проверено на красном
|
||||
входе, это закрыто юнит-тестами и ручной мутацией выше). Шаг «Расхождение
|
||||
зеркала на dev — issue владельцу» на этом прогоне не выполнялся (условие
|
||||
`push && refs/heads/dev` не совпало) — ожидаемо, не тестировался живьём,
|
||||
только юнит-тестом на структуру шага.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе) — `gh issue list` fail-open вместо fail-safe в новом шаге создания issue
|
||||
|
||||
`.github/workflows/validate.yml`, шаг «Расхождение зеркала на dev — issue
|
||||
владельцу»:
|
||||
|
||||
```bash
|
||||
existing=$(gh issue list --repo "$REPO" --state open --search "\"$marker\" in:title" \
|
||||
--json number,title --jq '[.[] | select(.title | startswith("[workflow-sync]"))][0].number // empty' || true)
|
||||
if [ -n "$existing" ]; then
|
||||
gh issue comment "$existing" ...
|
||||
exit 0
|
||||
fi
|
||||
gh issue create --repo "$REPO" --title "$marker ..." ...
|
||||
```
|
||||
|
||||
`|| true` глушит именно **чтение** списка issue: если `gh issue list` падает
|
||||
(сетевой сбой, рейт-лимит), `existing` остаётся пустой строкой, и скрипт идёт
|
||||
в ветку `gh issue create` — заводит новое `[workflow-sync]` issue, даже если
|
||||
открытое уже есть. Это расходится и с заявленным контрактом задачи («если оно
|
||||
уже открыто, в него добавляется комментарий»/PROCESS.md §10.4 «одно issue»),
|
||||
и с намерением автора («Сбой API — предупреждение: шаг сообщает, а не
|
||||
судит») — фактическое поведение при сбое чтения не «сообщает», а действует
|
||||
(плодит дубликат).
|
||||
|
||||
Сравнение с прецедентом, на который автор явно ссылается (#472,
|
||||
`.github/workflows/_mutation-gate.yml:327`): та же операция там **без**
|
||||
`|| true` — `gh issue list` без защиты, при её падении шаг GitHub Actions
|
||||
(shell по умолчанию `bash -eo pipefail`) прерывается целиком, дубликат не
|
||||
заводится. Новый шаг в #700 сознательно ослабил именно эту защиту.
|
||||
|
||||
**Чем краснеет:** мутация `|| true` → убрать (или заменить на
|
||||
`|| { echo "::warning::не удалось проверить открытые issue — issue не
|
||||
заводится"; exit 0; }`) — тест на этот путь в диффе отсутствует, поэтому
|
||||
проверено чтением, не исполнением; воспроизвести можно, подменив `gh` в PATH
|
||||
шага на скрипт, который для `issue list` возвращает ненулевой код, — тогда
|
||||
следующий `gh issue create` реально уйдёт, хотя нужного issue список не нашёл
|
||||
не потому, что его нет, а потому что сам не смог проверить.
|
||||
|
||||
**Почему Medium, не High:** окно узкое (нужен сбой именно у `gh issue list`,
|
||||
а не у последующих команд), шаг не участвует в вердикте (`check`/`advise` его
|
||||
исход не читают), последствие — лишнее owner-facing issue, а не красный
|
||||
Validate и не пропущенное реальное расхождение. Но это дефект поведения,
|
||||
прямо противоречащий описанному в задаче и в PROCESS.md контракту «одно
|
||||
issue», поэтому не Low: правка тривиальна (одна строка) и находится в этом
|
||||
же диффе, значит чинится в этом же issue (§3 п.8, §2.7), не заводится
|
||||
отдельно.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **Блокирующий периметр не сузился там, где должен остаться красным.**
|
||||
«Кандидат беты» в этом репозитории — не ветка `issue/*`, а коммит с
|
||||
трейлером `Release:` (PROCESS.md, класс D) либо `workflow_dispatch` тега в
|
||||
`release.yml`, оба целятся в `dev`/точный SHA, не в `issue/*`-ветку — значит
|
||||
`task_branch=false`, и `check` (красный) для workflow_sync и `--external`
|
||||
(без `warn`) для внешних ссылок продолжают действовать на этих путях.
|
||||
Отдельно проверил: «кандидат слияния» (`merge-candidate.mjs`) действительно
|
||||
публикуется push’ем в саму ветку задачи (PROCESS.md §… «публикация
|
||||
кандидата в ветку задачи») и там расхождение зеркала будет предупреждением —
|
||||
но финальный гейт слияния всё равно упирается в push **в `dev`** с
|
||||
`--force-with-lease`, а этот push уже не `issue/*`, и там проверка красная
|
||||
по-прежнему. Сквозной дыры в периметре нет.
|
||||
- `validate.yml` не входит в список из шести тонких зеркалируемых файлов
|
||||
(`process.yml`, `mutation-gate.yml`, `process-resume.yml`, `nightly.yml`,
|
||||
`process-reconcile.yml`, `process-metrics.yml`,
|
||||
`test/default-branch-workflows.test.mjs`) — этому диффу зеркалирование в
|
||||
`main` не требуется, сам он не самозеркалируемый файл.
|
||||
- `check-docs.mjs`: `EXTERNAL_WARN` корректно разводит `externalErrors` на
|
||||
`warnings`/`errors`, не создавая двойного пуша в оба стока; выход по
|
||||
`errors.length` не тронут — предупреждения никогда не красят `exitCode`.
|
||||
`transientHosts` (уже существовавший путь всегда-предупреждение) не задет.
|
||||
- Трейлеры коммита: `Issue: #700`, `User-Visible: no` — корректно, поведение
|
||||
для конечного пользователя карточки не меняется (это только CI/процесс),
|
||||
изменений в `docs/USER-GUIDE.ru.md` не требуется и не сделано.
|
||||
- Одно число, один источник (§8): числовых значений, видимых пользователю
|
||||
карточки, дифф не вводит и не дублирует — весь дифф про CI-вердикт.
|
||||
- Тесты умеют падать: обе новые мутации в `scripts/mutation-registry.mjs`
|
||||
(`task-branch-workflow-sync-red-again`, `external-link-warn-mode-ignored`)
|
||||
проверены вручную — соответствующие guard-тесты красные на мутированном
|
||||
коде, зелёные на исходном (см. таблицу гейтов выше).
|
||||
- `PROCESS.md` §10.4 дополнен точно тем, что реализовано: явно назван список
|
||||
из шести файлов, явно — что `performance.yml` в него не входит (не
|
||||
тронуто), явно — что на ветке задачи это предупреждение, а блокирует
|
||||
`dev`/кандидат/релиз.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- `actionlint validate.yml` — заявлен автором, локально бинарник недоступен
|
||||
(`command not found`); не перепроверял. Косвенное подтверждение — сам YAML
|
||||
реально исполнился на CI (run `36482200722`) без ошибки парсинга workflow.
|
||||
- `mutation-gate --check` (полный) и весь `npm run gate:small` — не
|
||||
перегонял: дешёвые гейты уже зелёные на этом SHA (см. ссылку в задаче
|
||||
ревью), а трек `show` не запрашивает мутанты по диффу (#696) — их
|
||||
отсутствие в этом отчёте не находка.
|
||||
- Ветвь `pull_request`/иных не-`issue/*`, не-`dev` рефов (например,
|
||||
гипотетический feature-branch без префикса `issue/`) — в этом репозитории
|
||||
такие пуши процессом не предусмотрены (`pre-push` гейт + прямые пуши,
|
||||
PR-флоу не используется по факту виденных коммитов), поэтому не разбирал
|
||||
отдельно; если он всё же встретится, `task_branch=false` и поведение
|
||||
останется прежним (строгим) — то есть ничего не ослабляется по умолчанию.
|
||||
- Живое воспроизведение красного `workflow_sync` (реальное расхождение
|
||||
`main`/`dev`) и живое исполнение шага создания issue — не запускал (это
|
||||
разрушительное действие: реально завело бы issue в репозитории); логика
|
||||
закрыта юнит-тестами и ручными мутациями, шаг создания issue — только
|
||||
чтением кода плюс находкой выше.
|
||||
- `golden:verify`, `pytest tests_backend`, инварианты модели, performance —
|
||||
не применимо, дифф не трогает `src/**`, рендер, геометрию или Python.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Единственная находка — Medium, в скоупе задачи, с указанной строкой и
|
||||
минимальной правкой. По правилам трека без High это жёлтый вердикт и возврат
|
||||
автору в этом же issue.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/700-preflight-warnings`, коммит `89d57e054cdf` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `945dad8823268ab0bbce926bd9ffe5e71d1f6cb2`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 945dad882326
|
||||
```
|
||||
- Тело issue: `a0c40d40c1401844c3d04ff0695dd347e76172d990004ea2799cc4df38729fce`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user