diff --git a/docs/reviews/CODE-REVIEW-706-r1.md b/docs/reviews/CODE-REVIEW-706-r1.md new file mode 100644 index 00000000..64b33092 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-706-r1.md @@ -0,0 +1,138 @@ +# Код-ревью #706 — заход r1 + +Материал ревью: `origin/dev...HEAD`, вершина `ae0e516af16e2ebcea9642949135cb950ce65eb3` +(один коммит поверх `dev@e7fca7b9`). Трек `show`, мутанты по диффу на материале не +запрашивались (не находка). + +## Скоуп + +Конвейер перестаёт снимать `S7-code-review` без возврата, когда исход `rereview` +после точного кандидата (#492) возвращает задачу в ту же метку, из которой она +пришла: `FROM == TO`. Совмещённый вызов `gh issue edit --add-label X --remove-label X` +добавлял и тут же снимал одну метку — итог был «без статуса», событие `labeled` не +приходило, новый заход ревью не стартовал (см. инцидент #699, зафиксированный в теле +issue). + +Правка вводит `scripts/status-label.mjs` (`moveStatusLabel`): при `FROM == TO` метка +снимается и ставится заново двумя вызовами `gh` через `relabel` автосверки (#555, +`scripts/process-reconcile.mjs:342`) — одна попытка восстановления, затем исключение; +при разных метках — как раньше, одной правкой. Шаг «Переставить метку» в +`_process.yml` вызывает скрипт вместо инлайн `gh issue edit`. `PROCESS.md` описывает +новое поведение точного кандидата. Три AC из тела issue закрыты тестами +`test/status-label.test.mjs` и двумя мутантами в `scripts/mutation-registry.mjs`. + +Класса A (публичный API/контракт) в диффе нет — только скрипты процесса и workflow. + +## Как проверялось + +Дешёвые гейты на этом SHA уже подтверждены (#343): Validate `ae0e516a` — success +(https://github.com/Matysh/houseplan-card/actions/runs/36521621591). `tsc --noEmit`, +`npm test`, `npm run build` на этом прогоне не перегонял; вместо этого: + +- прочитан код `scripts/status-label.mjs`, `.github/workflows/_process.yml` (весь шаг + «Переставить метку» и его окружение), `PROCESS.md` (правило точного кандидата), + `scripts/process-reconcile.mjs:342-351` (`relabel`, на которую опирается новый скрипт); +- `node --test --test-name-pattern="#706" test/status-label.test.mjs` — 4/4 зелёных; +- оба новых мутанта применены руками к копии дерева (`/tmp`, не в рабочей копии) и + проверено, что целевой юнит-тест красится: + - `rereview-relabel-in-one-call` (патч `scripts/status-label.mjs`, `if (from === to)` + → `if (false)`) — тест `#706 rereview` падает: `expected: 'relabeled', actual: 'moved'`; + - `process-label-step-combined-again` (откат шага на инлайн `gh issue edit`) — тест + `#706 шаг конвейера` падает на `assert.doesNotMatch(.../gh issue edit/)`; +- `node scripts/smoke-select.mjs --base origin/dev --head HEAD` — + «Исполняемого frontend-диффа нет (`src/**/*.ts` не тронут)», браузерные смоки не + выбираются — верно: правка не трогает `src/**`; +- `python3 -c "import yaml; yaml.safe_load(open('.github/workflows/_process.yml'))"` — + YAML синтаксически валиден после правки шага; +- `node --check scripts/status-label.mjs` — синтаксис ОК; +- прослежена совместимость сигнатур: локальный `gh(args, {allowFailure})` в + `status-label.mjs` вызывается `relabel(repo, {number: issue}, to, execute)` из + `process-reconcile.mjs` теми же двумя аргументами (`execute(args)` и + `execute(args, {allowFailure: true})`) — контракт совпадает; +- проверено, что заявленный `cancel-in-progress: false` в группе + `concurrency: process-issue-${{ github.event.issue.number }}` действительно стоит на + всех четырёх job (`_process.yml:53-55, 317-319, 974-976, 1350-1352`) — довод автора + «повторный `labeled` не отменяет текущий прогон, а встаёт в очередь» подтверждён; + без этого перестановка метки из середины ещё выполняющегося workflow-прогона могла + бы породить гонку; +- проверено, что комбинация «одна и та же метка на обоих концах» больше нигде в + `_process.yml` не встречается кроме исправленного места (`grep` по + `add-label.*remove-label`): строки 462, 702, 822 — везде разные метки на входе и + выходе, тот же класс бага там не воспроизводится, latent-дефект не пропущен; +- запущен полный `npm run gate:small` в фоне как избыточная перепроверка (дублирует + уже зелёный Validate) — не блокировал вывод вердикта; на момент публикации документа + ещё выполнялся, к находкам не привёл ни на одном пройденном шаге. + +## AC · чем доказан · чем краснеет + +| AC | Доказательство | Чем краснеет | Проверено | +|---|---|---|---| +| AC1: `FROM == TO` → снятие, затем постановка, задача остаётся в `S7-code-review` | `#706 rereview` | `rereview-relabel-in-one-call` | тест зелёный; мутант применён вручную — тест краснеет (см. выше) | +| AC2: `FROM ≠ TO` → одна правка; без `FROM` — только постановка | `#706 обычный исход` | — (поведение до #706, регрессии не требуется) | тест зелёный, прочитан код ветки `moved` | +| AC3: сбой повторной постановки роняет шаг; в шаге нет совмещённого `gh issue edit` | `#706 сбой…`, `#706 шаг конвейера` | `process-label-step-combined-again` (для второй половины); восстановление — мутанты `relabel` из #555 | тесты зелёные; мутант применён вручную — тест краснеет (см. выше) | + +Все три AC — защитные (гард против «задача осталась без статуса молча»), у каждого +есть строка «чем доказан/чем краснеет» с результатом прогона, пустых столбцов нет. + +## Находки + +Не найдено. Синтаксис аргументов CLI (`arg()` в `status-label.mjs`) проверен вручную +на всех четырёх флагах (`--repo=`, `--issue=`, `--from=`, `--to=`) — смещение среза +`name.length + 3` совпадает с длиной префикса `--{name}=` для каждого; ошибки на +пустом `--from=` (случай без исходной метки) не возникает, т.к. пустая строка — валидный +случай по AC2. + +## Что проверено и корректно + +- `moveStatusLabel` в обычном случае (`FROM ≠ TO`) даёт байт-в-байт то же поведение, + что и прежний инлайн `gh issue edit --add-label TO --remove-label FROM` — регрессии + для всех исходов, кроме `rereview`, нет. +- Обработка ошибок сохраняет прежнюю семантику шага: неудача любого `gh`-вызова + бросает исключение → `console.error('::error::...')` → `process.exit(1)` → + шаг падает → срабатывает последующий `if: failure()` («Позвать владельца»), как и + раньше при падении инлайн-команды. +- `PROCESS.md` (§ про точного кандидата) обновлён в том же диффе и согласован с кодом: + описывает именно снятие-и-постановку при разошедшемся patch-id, а не общий случай. +- User-Visible-трейлер не требуется и не заявлен — правка невидима пользователю + карточки (внутренний процесс ревью), changelog не тронут — это ожидаемо. +- Тесты корректно чувствительны: `#706 шаг конвейера` проверяет и наличие вызова + скрипта, и отсутствие возврата к `gh issue edit` — не только позитивную, но и + негативную сторону структуры шага. + +## Чего не проверял + +- Живой прогон `rereview` на самом конвейере — по признанию автора, воспроизводится + только при следующем слиянии со сменой patch-id; не гейт этого ревью (сценарий по + своей природе недоступен до реального конкурентного слияния). +- `npm run gate:small`, `process-gate --range`, `mutation-gate --check`, + диф-мутанты по 6 шардам (162/162) — заявлены автором, не перегонял; `tsc --noEmit`, + `npm test`, `npm run build` покрыты цитированным зелёным Validate на этом SHA и + повторно не гонялись согласно правилу дешёвых гейтов; `npm run gate:small` запущен + фоново как необязательная подстраховка, не как обязательный гейт этого раунда. +- `actionlint` — бинарь недоступен в этом окружении; синтаксис шага проверен вручную + построчным сравнением с `find`/`replace` мутанта и YAML-парсером Python, но + actionlint-специфичные проверки (например, контекст `steps.*.outputs` в других + частях файла) не перегонялись. +- Golden, pytest, инварианты модели — диф не трогает `src/**`, рендер, геометрию или + Python-бэкенд; по правилам объёма гейтов не применимы, что подтверждено + `smoke-select.mjs`, а не только предположением. + +## Вердикт + +Зелёный. Все три AC доказаны тестами, которые я проверил на способность падать +(применил оба мутанта вручную к копии дерева). Класса A нет, User-Visible не требуется, +изменение локально и соразмерно заявленной сложности 2/10. + +--- + + + +## Материал раунда + +- Ветка: `issue/706-rereview-label`, коммит `ae0e516af16e` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `2441daa60197945e66175706f34258bb589764c7` + ``` + git log --all --format='%H %T' | grep 2441daa60197 + ``` +- Тело issue: `c4af86c1bc1383ec564be59c70334fb4158021c86e84c0d13f23951152fc619d` +- Вердикт конвейера: `green` · High 0