docs: review document for #636

Issue: #636
User-Visible: no

docs/reviews/CODE-REVIEW-636-r1.md

@@ -0,0 +1,205 @@
+# CODE-REVIEW-636-r1
+
+## Скоуп
+
+Issue #636 (инфраструктура, класс B, ускоренный вход по §1 — сразу `S7`, без
+ТЗ/спека). Единственный коммит `351fef43d65ffbc6eeceda7e744bfe142db2236d`
+поверх `dev`@`e29dfdeb`. Ни одного файла класса A — только `scripts/**`,
+`test/**`, `.github/workflows/**`, `PROCESS.md`, `AGENTS.md`.
+
+Задача: стадия `prepare` конвейера ревью (`process.yml`) синхронно ждала
+Validate с мутантами на материале до 45 минут (в среднем 28,1–28,7 мин на
+раунд при 10–12 минутах работы модели), занимая раннер вхолостую. Правка
+заменяет ожидание внутри job на событийное продолжение: `validate-gate.mjs
+--no-wait` диспатчит прогон, убеждается, что тот встал на материал, и выходит
+с кодом 2 (`pending`), не дожидаясь завершения; `process-resume.yml`
+(`workflow_run: completed` на `Проверка (CI)`) переставляет `S7-code-review`,
+когда раунд действительно ждал именно этот прогон; `process-reconcile.mjs`
+служит страховкой на потерянное событие.
+
+AC из тела issue:
+- AC1. Ни одна job конвейера не ждёт другой workflow дольше 3 мин (тест на
+  отсутствие цикла ожидания/`gh run watch` в `process.yml`).
+- AC2. Время от `S7` до начала работы модели не растёт (замер на 5 задачах
+  до/после).
+- AC3. Раунд, чей Validate завершился во время простоя reconcile, стартует не
+  позднее следующего тика reconcile.
+
+Заход r1 — первый цикл, разбор полный.
+
+## Как проверялось
+
+Материал — рабочая копия на `351fef43d65ffbc6eeceda7e744bfe142db2236d`, без
+`git fetch`/`checkout`. Дешёвые гейты (`tsc`, `npm test`, `npm run build` со
+сверкой бандла) не перегонялись: Validate на этом SHA зелёный —
+https://github.com/Matysh/houseplan-card/actions/runs/35824683286 — и по
+условиям задачи это покрывает и мутационный гейт (шесть mutant-jobs на
+ревью). `check-docs.mjs` не требовался: diff не трогает `src/**`. Инварианты
+модели, smoke, golden, `pytest tests_backend`, перф-профили — не требовались:
+diff не трогает геометрию, рендер, Python или чувствительные к перфу пути.
+
+Дополнительно, сверх минимума, сам прогнал точечно:
+- `node --test test/process-resume.test.mjs test/validate-gate.test.mjs
+  test/process-reconcile.test.mjs test/mutation-gate.test.mjs
+  test/review-doc-guard.test.mjs` → 158/158 pass — подтверждает, что новые и
+  задетые тесты зелёные на этом материале, а не только по слову автора.
+- Вручную применил все 4 новых мутанта из `scripts/mutation-registry.mjs`
+  (`gate-no-wait-still-sleeps`, `resume-wakes-round-without-marker`,
+  `resume-ignores-active-run`, `reconcile-wakes-pending-while-validate-active`)
+  по одному и прогнал заявленный `guard`-тест на каждом патче: все четыре
+  красят соответствующий тест (1 упавший тест на патч), рабочая копия
+  восстановлена (`git status --porcelain` пуст после). Мутационный гейт не
+  просто существует — он действительно ловит снятую защиту.
+- Прочитал полный текст `scripts/validate-gate.mjs`, `scripts/process-resume.mjs`,
+  `scripts/process-reconcile.mjs` (диффовые и смежные функции), новый
+  `.github/workflows/process-resume.yml`, изменённые куски `process.yml` и
+  `validate.yml` — целиком, не только по хankам диффа.
+
+## AC · чем доказан · чем краснеет
+
+| AC | Доказательство | Проверка ревьюера |
+|---|---|---|
+| AC1 | `test/validate-gate.test.mjs` — 4 новых теста `#636`: идущий dispatch → `pending` без единого `sleep` (`ops.now()===0`); прогон, который ещё не появился, — гейт диспатчит и ждёт появления (≤3 мин по `VALIDATE_APPEAR_MS`), не завершения; завершённый зелёный/красный — как раньше, сразу. `test/review-doc-guard.test.mjs` проверяет текст `process.yml`/`process-resume.yml` (условие `proceed == 'false'`, `--no-wait`, контракт маркера) | Прогнал тесты (158/158), убил мутант `gate-no-wait-still-sleeps` (1/1). Прочитал `validateGate`: ветка `if (!wait)` стоит строго после проверки `run.status === 'completed'`, поэтому завершённый прогон и с `--no-wait` возвращается как раньше — подтверждено тестом «зелёный/красный принимается сразу» |
+| AC2 | Замер на 5 задачах после слияния — по конструкции задачи это возможно только постфактум, автор явно пометил это как «Чего не прогонял: живой конвейер (только на main после слияния)» | Не переисполнимо на этапе ревью — измерение требует живого прогона в проде после мержа и зеркалирования `process-resume.yml`/`process.yml` в `main`. **Проверено чтением, не исполнением**: событие `workflow_run` физически приходит в момент завершения Validate (тот же момент, когда раньше просыпался poll, ≤20 c реакции), поэтому по конструкции задержка не растёт. Формальное закрытие AC2 остаётся за первым живым раундом после мержа — это ожидаемо для AC такого типа, не находка |
+| AC3 | `test/process-reconcile.test.mjs` — «#636 pending marker»: `pendingValidate==='active'` → `wait`; `completed`/`missing` → `retry` (следующий тик reconcile переставляет `S7` сам); без маркера — прежний `escalate` | Прогнал тест, убил мутант `reconcile-wakes-pending-while-validate-active` (1/1). Прочитал `decideReconciliation`: новая ветка стоит перед general `run.conclusion === 'success' → escalate`, поэтому успешный прогон с маркером не путается со старым «успех без вердикта» |
+
+## Находки
+
+Блокирующих (High) и находок Medium в скоупе — нет.
+
+Одно наблюдение, не блокирующее и не требующее отдельного issue:
+
+- **AC1 в общей формулировке шире, чем то, что фактически исправлено.**
+  `integrate`-job того же `process.yml` (через `scripts/merge-candidate.mjs`,
+  функция `waitValidate`) по-прежнему синхронно поллит `gh run list` до 45
+  минут, когда за время ревью `dev` успел уйти вперёд и кандидата нужно
+  пересобрать и перепроверить. Это тоже «job конвейера, ждущая другой
+  workflow дольше 3 мин», но раздел «Факты» issue называет только стадию
+  `prepare` (28,1–28,7 мин на каждый раунд) и по аналогии — `nightly.yml` и
+  `release-gate.mjs`; про `merge-candidate.mjs`/`integrate` там не сказано ни
+  слова, и сам AC1 в скобках сужает предмет проверки до текста `process.yml`
+  на отсутствие цикла именно там, где раньше был. Не считаю это находкой в
+  скоупе: путь `integrate`-ожидания условный (срабатывает только когда `dev`
+  двигался во время ревью, а не на каждом раунде), для него уже заведён
+  отдельный бюджет 55 минут (#551) заранее, под него, и диффом не тронут —
+  то есть не регрессия. Если владелец сочтёт нужным закрыть и этот случай —
+  это отдельная задача, не расширение текущего скоупа.
+- Аналогично не тронуты `nightly.yml` (`gh run watch`, 3–25 мин) и
+  `release-gate.mjs` (ожидание `performance.yml` до 60 мин), хотя раздел
+  «Правка» issue упоминает их как аналогичные случаи. AC1/AC2/AC3 их не
+  называют, тест AC1 явно ограничен `process.yml` — вне скоупа этой задачи.
+
+## Что проверено и корректно
+
+- `validateGate({ wait: false })`: ветвление корректно — сначала проверяется
+  `run.status === 'completed'` (зелёный/красный возвращаются немедленно
+  независимо от `wait`), и только для незавершённого прогона `wait: false`
+  выходит в `pending` без `sleep`. Появление dispatch (если прогона ещё нет)
+  по-прежнему ограничено `VALIDATE_APPEAR_MS` (3 мин, до двух попыток при
+  гонке ref/SHA, #539) — не новая ветка, поведение унаследовано.
+- `process.yml`: `set +e / code=$? / set -e` корректно перехватывает код
+  выхода 2 до строгого режима; `case` покрывает все три исхода; шаг
+  «Validate идёт» и «Сохранить маркер ожидания» гейтятся именно на
+  `proceed == 'pending'`; шаг «Validate красный» теперь — точно на
+  `proceed == 'false'` (было `!= 'true'`, что раньше ошибочно включало бы и
+  `pending`). Все шаги, зависящие от зелёного гейта («Зелёные гейты на этом
+  SHA», сбор `prepared.json`, публикация артефакта модели), по-прежнему
+  условие `== 'true'`, поэтому `pending` их не задевает — не нужно было
+  трогать эти строки, и их не тронули.
+- `integrate`-job: явная короткая ветка `PROCEED = "pending"` — печатает
+  причину, `proceed=false`, `exit 0`, не долетает до `failure()`-уведомления
+  владельца (это не авария, а штатный промежуточный статус).
+- `process-resume.yml`: триггер `workflow_run` на точное имя workflow
+  (`"Проверка (CI)"`, сверено с `validate.yml`); job-level `if` фильтрует
+  `event == 'workflow_dispatch'` и `head_branch` `issue/*` до checkout —
+  нерелевантные прогоны (push, nightly на `dev`, PR) не тратят раннер вообще.
+  `concurrency: process-resume-<branch>` без `cancel-in-progress` сериализует
+  повторные события на одной ветке. Метка переставляется `HP_PROCESS_TOKEN`,
+  не `GITHUB_TOKEN` — иначе событие `labeled` не запустило бы `process.yml`
+  (тот же инвариант, что и у остального конвейера). Код берётся из `dev`, как
+  у `process-reconcile.yml`.
+- `decideResume`: проверил все ветви руками — не-`workflow_dispatch`,
+  незавершённый прогон, чужой `headSha`, отсутствие `S7`, `blocked`/`review-4`,
+  активный процесс-прогон (в т.ч. НЕ только последний, а любой активный — это
+  на случай гонки со свежим раундом), отсутствие маркера, маркер на другом
+  SHA, устаревший/проигравший маркер (сортировка по `createdAt`+`id`, решает
+  только новейший прогон) — все корректно ведут к `noop`, и только полное
+  совпадение — к `resume`. Специально проверил сценарий «Validate дождался
+  сам merge-candidate во время `integrate`»: в этот момент процесс-прогон,
+  из которого запущен `integrate`, ещё `in_progress` → `mine.some(active)` →
+  `noop`; ложного резюме на постороннем dispatch того же branch не будет.
+- `process-reconcile.mjs`: `pending`/`pendingValidate` гидратируются только
+  для `run.status === 'completed'` (не тратит вызовы API на активные прогоны);
+  новая ветка `run.conclusion === 'success' && run.pending` стоит раньше
+  общей `success → escalate`, поэтому не путает «ждём Validate» с «успех без
+  вердикта»; `validateStateOnMaterial` фильтрует только `workflow_dispatch` и
+  предпочитает `active` над `completed`, что верно (пока хоть один активен —
+  ждать). `loadSealedArtifact` — рефакторинг прежнего `loadPreparedArtifact`
+  без потери проверки чек-суммы; используется одинаково для `prepared.json` и
+  `pending.json`.
+- Дедупликация меток: `retry` в reconcile для pending-случая проходит через
+  тот же `alreadyReported`/`reconciliationKey`, что и остальные ветки —
+  разные `reason` для `active`/`completed`/`missing` дают разные ключи,
+  повторный тик не долбит комментариями бесконечно на неизменном состоянии
+  (комментарий появляется один раз на пару issue+run+reason).
+- `actions/upload-artifact@ea165f8d...# v4` — тот же закреплённый SHA, что уже
+  используется в трёх других местах `process.yml` и в
+  `process-reconcile.yml`; не новый непроверенный pin.
+- `validate.yml` preflight обновлён третьим файлом (`process-resume.yml`) в
+  списке сверки `main`↔`dev` — без этого новый workflow-файл разошёлся бы
+  молча между ветками, как когда-то `process.yml`/`mutation-gate.yml` (#472).
+- Трейлеры коммита: `Issue: #636`, `User-Visible: no` — верно, видимого
+  пользователю поведения нет; changelog не требуется и не тронут.
+- `PROCESS.md`/`AGENTS.md` обновлены в том же коммите, описывают именно то
+  поведение, что в коде (сверил формулировки построчно с реализацией).
+
+## Чего не проверял
+
+- **AC2 не переисполним на этапе ревью** — требует замера на 5 живых задачах
+  после мержа и зеркалирования `process-resume.yml` в `main` (сам автор
+  назвал это в разделе «После слияния — владельцу»). Принято по чтению
+  конструкции (`workflow_run` срабатывает синхронно с завершением Validate),
+  формальная приёмка — задача первого живого раунда после мержа, не этого
+  ревью.
+- Не гонял `npx tsc --noEmit` и полный `npm test`/`npm run build` отдельно —
+  Validate на `351fef43` зелёный, эти гейты и mutant-jobs в нём уже
+  исполнены; перегонять секунды не добавили бы уверенности сверх точечного
+  прогона (158/158) и ручной проверки 4 мутантов, который сделал сам.
+- Не проверял `demo/smoke_*.mjs`, `golden:verify`, `pytest tests_backend`,
+  `model-invariants` — diff не трогает `src/**`, рендер, Python или
+  геометрию; неприменимо по составу diff.
+- Не проверял живое поведение `gh run download`/`gh api` в реальном GitHub
+  Actions окружении (сетевые вызовы `realOps`/`processRuns`/обёртки в
+  `resume()`) — это тонкие обёртки без собственной логики, покрытые
+  fixture-тестами; живая проверка возможна только в реальном прогоне (тот же
+  пробел, что у AC2).
+
+## Материал раунда
+
+```
+material: 351fef43d65ffbc6eeceda7e744bfe142db2236d
+```
+
+## Вердикт
+
+**Зелёный.** Все три AC доказаны (AC1, AC3 — автотестом с проверенной
+способностью падать; AC2 — чтением конструкции с честной пометкой
+«формальная приёмка после мержа»). High и Medium-в-скоупе находок нет.
+Единственное наблюдение (частичное покрытие общей формулировки AC1 —
+`integrate`/`merge-candidate.mjs`, `nightly.yml`, `release-gate.mjs`) вне
+скоупа этой задачи по тексту issue и не является регрессией — оставлено как
+информация владельцу, отдельный issue не завожу.
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `issue/636-review-wait-event`, коммит `351fef43d65f` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `84cbf14af04c3438b6c5ff3e588fc64c3eb5593f`
+  ```
+  git log --all --format='%H %T' | grep 84cbf14af04c
+  ```
+- Тело issue: `6b7ad1bfe1fff446e1ab2c3d089af99b61d2286903f313d2ec0f764dbb6d446e`
+- Вердикт конвейера: `green` · High 0