mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-01 12:18:51 +00:00
claude[bot]
@@ -0,0 +1,95 @@
|
||||
# CODE-REVIEW #730 · заход r2
|
||||
|
||||
**Материал**: `3808e6f990694100bc687f09c3f25fdf59f2a91c` (рабочая копия на этом SHA; `git diff origin/dev...HEAD` / `git log --oneline origin/dev..HEAD`).
|
||||
Трек: `show`. Дешёвые гейты (`tsc --noEmit`, `npm test`, `npm run build` со сверкой бандла) подтверждены зелёным Validate на этом SHA: https://github.com/Matysh/houseplan-card/actions/runs/36804205561 — не перегонялись. Ветка не приводилась к `dev` (трек `show`, #696): dev впереди на 20 коммитов, слияние без конфликта.
|
||||
|
||||
## Скоуп
|
||||
|
||||
r1 разобрал AC1–AC3 целиком (два workflow-тела + страж ребейза разбирают отказ push кодом слияния вместо «любой отказ = сдвиг dev») и нашёл одну находку:
|
||||
|
||||
> Medium (в скоупе AC2) — `refusalSummary` пишет «повтор и ребейз не помогут» для ЛЮБОГО не-stale исхода, в том числе `workflow`, где по PROCESS.md ребейз как раз штатное и единственное решение (делает его автор). Страж ребейза теперь (AC2) кладёт эту фразу в сводку шага — и вводит автора в заблуждение именно в момент, когда ему нужно понять, что делать.
|
||||
|
||||
r2 — точечный ответ на эту находку. Делта от материала r1 (`981579aa`) к материалу r2 (`3808e6f9`) — ровно:
|
||||
|
||||
```
|
||||
scripts/merge-candidate.mjs | 7 ++-
|
||||
test/publish-push-refusal.test.mjs | 9 +++
|
||||
```
|
||||
|
||||
(плюс коммит `e0da2224`, добавляющий сам документ r1 в дерево — технический, ревью не касается). Делта локальна: правка внутри одной функции (`refusalSummary`), новый тест рядом с уже существующими тестами той же функции. AC1–AC3 этой делтой не задеты — их материал и доказательства не менялись с r1.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Прочитан полный дифф ветки к `origin/dev` (443 строки) и делта r1→r2 (`git diff 981579aa..3808e6f9`) — отдельно, чтобы увидеть именно то, что изменилось в ответ на находку.
|
||||
2. Прочитано тело issue #730 и все четыре комментария, включая финальный комментарий автора о r2 с указанием SHA `3808e6f9` и перечнем правок.
|
||||
3. Прочитан новый код `refusalSummary` (`scripts/merge-candidate.mjs:538-556`): ветка `nextStep` для `refusal.kind === PUSH_REFUSAL.workflow` против общей фразы для остальных исходов.
|
||||
4. Выполнена дисциплина «тест должен уметь падать»: временно подменил `scripts/merge-candidate.mjs` версией из r1 (`981579aa`) и прогнал `test/publish-push-refusal.test.mjs` — красным стал ровно и только новый тест `#730 r1: при отказе по праву на workflow сводка зовёт автора сделать ребейз, а не отговаривает` (1 fail из 24), остальные 23 прошли. Рабочая копия восстановлена (`cp` обратно сохранённой текущей версии), `git status --short` и `git diff --stat` после восстановления пусты.
|
||||
5. На материале (`3808e6f9`) прогнаны все три задействованных теста-файла: `test/publish-push-refusal.test.mjs` — 24/24, `test/merge-candidate.test.mjs` — 33/33, `test/rebase-generated.test.mjs` — 19/19. Сумма 76/76 совпадает с заявлением автора в комментарии к r2.
|
||||
6. Проверено (`grep`), что фраза «повтор и ребейз не помогут» осталась только как общий случай в `scripts/merge-candidate.mjs`, в тесте, который её же и фиксирует, и в архивном `docs/reviews/CODE-REVIEW-730-r1.md` — нигде больше в кодовой базе (PROCESS.md, комментарии к #705) несогласованного текста не осталось.
|
||||
7. Сверено, что существующий комментарий к задаче при отказе по workflow (`test/rebase-generated.test.mjs:435`, из #705, не менялся) уже говорит «ребейз и push делает автор, либо владелец выдаёт право» — новый текст сводки (`merge-candidate.mjs`) теперь дословно согласован с этой формулировкой, а не придумывает новую.
|
||||
8. Трейлеры коммита `3808e6f9`: `Issue: #730` ✓, `User-Visible: no` ✓ (правка текста CI-сводки, поведение карточки не меняется).
|
||||
9. `node scripts/smoke-select.mjs --base origin/dev --head HEAD` — не перегонял повторно: `src/**` в делте r2 не тронут (как и во всей ветке), вывод r1 («исполняемого frontend-диффа нет») не мог измениться.
|
||||
|
||||
## AC · чем доказан · чем краснеет
|
||||
|
||||
AC1–AC3 не входят в делту r2 — таблица из r1 остаётся в силе без изменений (см. «Унаследовано из r1»). Ниже — доказательство именно правки r2 (находка r1, в скоупе AC2):
|
||||
|
||||
| Что доказывается | Доказательство | Чем краснеет (проверено исполнением) |
|
||||
|---|---|---|
|
||||
| При исходе `workflow` сводка больше не говорит «ребейз не поможет», а называет ребейз автора штатным выходом; при прочих отказах GitHub текст не изменился | `test/publish-push-refusal.test.mjs` — `#730 r1: при отказе по праву на workflow сводка зовёт автора сделать ребейз, а не отговаривает` | На `refusalSummary` из r1 (`981579aa`) — красный (проверено откатом файла и прогоном: 1/24 fail, именно этот тест) |
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка r1 | Чем закрыта | Где это видно |
|
||||
|---|---|---|
|
||||
| Medium (в скоупе AC2): `refusalSummary` пишет «повтор и ребейз не помогут» и для исхода `workflow`, хотя там ребейз — штатное решение автора | `nextStep` в `refusalSummary` теперь различает `PUSH_REFUSAL.workflow` и даёт отдельный текст: «повтор этого шага не поможет… ребейз и push делает автор, либо владелец выдаёт право»; для остальных исходов фраза не изменилась | `scripts/merge-candidate.mjs:541-549` (новая ветка `nextStep`); воспроизведено исполнением — тест `#730 r1` зелёный на `3808e6f9` и красный на `981579aa` (проверено лично, см. «Как проверялось», п.4) |
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Принято без повторной проверки — делта r2 эти участки не касалась:
|
||||
|
||||
- AC1 (разбор push в `_ship-review.yml`/`_beta-derived.yml` через `merge-candidate.mjs --push-refusal`, устаревший lease — прежнее поведение, прочий отказ — стоп без повторов, причина и ответ git без токена в журнал и сводку) — документ `docs/reviews/CODE-REVIEW-730-r1.md`, материал `981579aa`, раздел «AC · чем доказан · чем краснеет», строка AC1; перепроверено свежим прогоном: `test/publish-push-refusal.test.mjs` зелёный на `3808e6f9` (24/24, включая 12 тестов r1 плюс 1 новый).
|
||||
- AC2 (страж ребейза пишет причину отказа в сводку шага) — тот же документ, строка AC2; перепроверено: `test/rebase-generated.test.mjs` зелёный на `3808e6f9` (19/19).
|
||||
- AC3 (нет heredoc в `run:`, тонкие `ship-review.yml`/`beta-derived.yml` не тронуты) — тот же документ, строка AC3; делта r2 эти файлы не меняла (`git diff 981579aa..3808e6f9 --stat` их не перечисляет).
|
||||
- Скрипт `merge-candidate.mjs` берётся из `dev`, не из тонкого вызывающего репозитория — документ r1, п.9 «Как проверялось»; код чекаута (`ref: dev`, `git reset -q --hard origin/dev`) делтой r2 не затронут.
|
||||
- Токен нигде не светится (`noisySecretsGone` во всех новых тестах) — документ r1; новый тест `#730 r1` тоже не вносит текст с токеном.
|
||||
- Трейлеры коммитов r1 (`981579aa`) — документ r1, п.7 «Как проверялось».
|
||||
- Полный `npm test` зелёный на материале r1 (3347/3348, 1 skip) — документ r1, п.5; на r2 полный прогон не повторялся (не требуется: делта — 2 файла, точечно прогнаны все три связанных тест-файла, 76/76, см. выше), наборы дешёвых гейтов подтверждены зелёным Validate на `3808e6f9`.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Находка r1 закрыта содержательно, а не косметически: текст для `workflow`-исхода называет единственно верное действие (ребейз и push автором либо передача права), текст для остальных исходов не поменялся — риск спутать «тупик» с «штатным шагом» снят для того единственного случая, где это было нужно.
|
||||
- Новый текст согласован с уже существующим (немодифицированным, из #705) комментарием к задаче при том же исходе `workflow` — одна и та же мысль не разъезжается в двух разных местах процесса.
|
||||
- Дисциплина «тест должен уметь падать» подтверждена лично (не на слово автора): откат `merge-candidate.mjs` к версии r1 красит ровно новый тест и ничего больше.
|
||||
- Делта минимальна и не трогает AC1/AC3 — файлы `_ship-review.yml`, `_beta-derived.yml`, `_process.yml` в r2 не менялись, риска регрессии в уже проверенной r1 части нет.
|
||||
- Трейлеры `Issue: #730` / `User-Visible: no` на месте в коммите `3808e6f9`; `User-Visible: no` оправдан — правка меняет только текст CI-сводки, не поведение карточки.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- `tsc --noEmit`, `npm run build` + сверка трёх копий бандла — не перегонял: подтверждены зелёным Validate на этом же SHA (`3808e6f9`, https://github.com/Matysh/houseplan-card/actions/runs/36804205561).
|
||||
- Полный `npm test` (3348+ тестов) целиком на `3808e6f9` — не перегонял отдельно: делта r2 касается ровно трёх тест-файлов (`publish-push-refusal`, `merge-candidate`, `rebase-generated`), все три прогнаны поимённо и зелёные (76/76); остальные файлы делтой не затронуты, а Validate на этом SHA уже зелёный целиком.
|
||||
- `actionlint` — делта r2 не трогает YAML workflow-тела (только `scripts/merge-candidate.mjs` и тест), не применимо.
|
||||
- Браузерные смоки — `src/**` не тронут ни в r1, ни в r2; `node scripts/smoke-select.mjs` на материале r1 уже ответил «исполняемого frontend-диффа нет» (документ r1, «Чего не проверял»), делта r2 этот вывод не меняет.
|
||||
- `npm run golden:verify`, `python -m pytest tests_backend -q`, `npm run invariants -- --config …`, performance-профили — не применимо по тем же причинам, что в r1 (нет меток/правок соответствующих областей).
|
||||
- Ручной живой прогон `_ship-review.yml`/`_beta-derived.yml`/`_process.yml` в GitHub Actions — не делал; текст сводки проверен исполнением той же функции (`refusalSummary`), которую исполняет сам workflow-шаг.
|
||||
- Мутанты по диффу — не запрашивались (трек show, #696), не прогонял.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Зелёный. Единственная находка r1 (Medium, в скоупе AC2) закрыта точечно и проверена исполнением: новый тест падает на коде r1 и проходит на коде r2. Делта r2 не задевает AC1/AC3, унаследованные без изменений. Блокирующих находок нет.
|
||||
|
||||
---
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/730-derived-push-refusal`, коммит `3808e6f99069` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `d1b6f2b4200c1166e905a9e9f87cec2be8bd102c`
|
||||
```
|
||||
git log --all --format='%H %T' | grep d1b6f2b4200c
|
||||
```
|
||||
- Тело issue: `3d77fb56ed2e308d5df6d329563f8143ebf6d19f6a36b320a92fdb76ee31d8fd`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user