docs: review document for #775

Issue: #775
User-Visible: no

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

@@ -0,0 +1,205 @@
+# CODE-REVIEW-775-r1
+
+Issue: #775 — «Конвейер: "Разбудить раунд" после Validate отвечает noop — задачи в S7 зависают без ревью»
+Трек: show · этап: code · заход: r1 · блокирующих циклов: 0/2
+Материал: `6bece2517939542be56c73bf8ee0703aebb2635c` (= HEAD, = origin/dev + 1 коммит)
+Класс: B (gates/tooling) + C (PROCESS.md) — инфраструктурная задача, без файлов класса A.
+
+## Скоуп
+
+01.10 три задачи (#740, #744, #748) застряли в `S7-code-review`: Validate на
+материале завершился, а `process-resume.mjs` ответил `noop`, ревью не
+запустилось, обход — ручная перестановка метки. Issue называет три гипотезы
+(окно в 200 прогонов `processRuns`, перепутанный «последний» прогон после
+снятия/установки метки в одну секунду, маркер прошлого раунда после ребейза) и
+требует: resume находит именно прогон с маркером, а не просто последний; окно
+не зависит от чужой разметки; красный Validate всегда возвращает задачу в S6 с
+комментарием. Свидетели — юниты `decideResume`/`processRuns` на этих трёх
+историях.
+
+Единственный коммит на ветке правит `scripts/process-reconcile.mjs`,
+`scripts/process-resume.mjs`, `.github/workflows/_process-resume.yml`,
+`scripts/mutation-registry.mjs`, `PROCESS.md` и добавляет
+`test/process-pending-round.test.mjs`; правит `test/process-resume.test.mjs`
+под новую сигнатуру. Продуктовый код (`src/**`, `custom_components/**`) не
+тронут. Трейлеры коммита: `Issue: #775`, `User-Visible: no` — верно: правка не
+меняет ничего, что видит пользователь карточки, changelog не нужен.
+
+**Применимость критериев §5 (route):**
+- complexity — умеренная (переписана выборка истории, идентичность маркера,
+  двойная проверка перед записью), риск смягчён 32 регрессионными тестами,
+  три из них — прямой повтор трёх реальных инцидентов по их идентичностям
+  (id прогонов, SHA, время). ≤3, проходит.
+- surfaces — один модуль: пара resume/reconcile процесса ревью. Проходит.
+- migration — пайплайн уже писал `branch`/`validate_run_id`/`run_attempt`/
+  `stage` в `pending.json` до этой задачи (`.github/workflows/_process.yml:792-811`,
+  файл не менялся в этом диффе); правка лишь ужесточает проверку уже
+  существующих полей. Новых полей конфигурации или совместимости нет. Проходит.
+- ux-contract / perf-touch — не применимо, пайплайн не рендерит карточку и не
+  трогает ввод. Проходит.
+- undocumented — поведение зафиксировано в самом PROCESS.md тем же коммитом
+  (раздел «Поиск ожидающего раунда (#775)», после «весь реестр проверяет
+  ночь…») и в теле issue («Что нужно»). Проходит.
+
+Все критерии §5 пройдены → `route: fix`.
+
+## Как проверялось
+
+Дешёвые гейты на `6bece251` уже зелёные в Validate
+(https://github.com/Matysh/houseplan-card/actions/runs/36892663617) — `npx tsc
+--noEmit`, `npm test`, `npm run build` + сверка бандла не перегонялись
+повторно (#343).
+
+Прогнано самим ревьюером (дополнительно к Validate, т.к. задача про сам
+пайплайн ревью — проверил своими руками, а не только по отчёту автора):
+
+| Гейт | Команда | Результат |
+|---|---|---|
+| Целевые юниты по issue | `node --test test/process-pending-round.test.mjs test/process-resume.test.mjs` | 19/19 зелёных |
+| Юниты reconcile (не изменена сигнатура публичных функций, но логика общая) | `node --test test/process-reconcile.test.mjs` | 13/13 зелёных |
+| Консистентность дайджеста ревьюера с PROCESS.md (правка в этом же файле) | `node --test test/process-digests.test.mjs` | 6/6 зелёных |
+| Статическая целостность реестра мутантов (новые 4 патча накладываются на реальный текст) | `node --test test/mutation-gate.test.mjs` | 69/69 зелёных |
+| Выбор смоков по диффу | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | «Исполняемого frontend-диффа нет» — смоки не выбираются, диф не трогает `src/**` |
+
+Итого 19+13+6+69 = 107 юнит-тестов, все зелёные; независимо подтверждает
+заявленные автором «32 целевых теста» (19 в process-pending-round/process-resume
++ 13 в process-reconcile = 32).
+
+Дополнительно ручной разбор:
+- Прочитан полный `scripts/process-reconcile.mjs` и `scripts/process-resume.mjs`
+  целиком (не только дифф) — проверено чтением, не исполнением, как
+  изменённые функции сочетаются с неизменными (`decideReconciliation`,
+  `applyReconciliationDecision`).
+- Сверено текстом, что четыре новых мутанта в `scripts/mutation-registry.mjs`
+  (`pending-history-latest-200-only`, `pending-revives-previous-request`,
+  `pending-resumes-another-validate`, `pending-no-fresh-snapshot-before-write`)
+  действительно патчат уникальные строки текущего (постдиффового) кода — все
+  четыре `find`-строки найдены ровно один раз в целевых файлах (grep).
+- Прочитан нетронутый этим диффом участок `.github/workflows/_process.yml`
+  (шаги «Validate на материале» / «Validate идёт — раунд продолжит событие» /
+  «Validate красный — вернуть автору без ревью», строки 748–857) — подтверждён
+  заявленный в комментарии автора факт: red-path в S6 с комментарием уже
+  существовал ДО этой задачи и не требовал правки; баг был именно в том, что
+  resume не давал этому пути шанс сработать повторно.
+
+## AC и способ доказательства
+
+Issue не содержит формального раздела `## ТЗ` (инфраструктурная задача, трек
+show, спецификация — текст issue), но называет три проверяемых требования и
+явно требует юнит-свидетелей на трёх реальных историях.
+
+| AC | Чем доказан | Чем краснеет |
+|---|---|---|
+| Resume находит прогон с маркером, не просто «последний» | `test('#775 all three incident markers resume…')` реплицирует точные идентичности (issue, run id, validate id, sha, время, исход) трёх зависших задач и утверждает `action === 'resume'`; разобрано чтением — `reviewRunsForRequest` теперь ограничивает выборку `at(run.createdAt) >= at(request.at)`, что устраняет оба сценария из гипотез issue (события `unlabeled` не порождают `request`, т.к. `latestReviewRequest` фильтрует только `event === 'labeled'`; ребейзный «старый» прогон до переразметки не попадает в окно) | Мутант `pending-revives-previous-request` (сдвигает границу на −120с, реестр `scripts/mutation-registry.mjs`) |
+| Окно истории не зависит от чужой разметки (было: 200 последних прогонов `process.yml`) | `test('#775 history is paged to the request, not capped at 200 or filtered-search 1000 runs')` — 1100 посторонних прогонов для issue #999 не мешают найти нужный; `calls.length === 12` страниц (постранично до `oldest`), `url` без `event=` (без серверного фильтра с потолком 1000) | Мутант `pending-history-latest-200-only` (возвращает предел в 2 страницы) |
+| Красный Validate всегда возвращает задачу в S6 с комментарием | Код шага «Validate красный — вернуть автору без ревью» (`_process.yml:826-857`) не изменён этим диффом и безусловен по `steps.gate.outputs.proceed == 'false'` — проверено чтением, не исполнением. `decideResume` не фильтрует по `conclusion` Validate (только `event`/`status`/`headSha`), и инцидент #744 (`conclusion: 'failure'`) воспроизведён в том же параметризованном тесте с ожиданием `resume` | Явного мутанта на «resume не должен отказывать по conclusion» в реестре нет — проверка идёт через отсутствие фильтра в коде, подтверждена только прочтением; граница ответственности этого диффа — довести до relabel, а не до интерпретации red/green (это не менялось) |
+| Не терять событие при временно неполной выдаче (до 3 снимков, паузы 5с) | `test('#775 transient missing/old/no-marker snapshots recover…')`: `waits = [5000]`; `test('#775 absent evidence gets a bounded grace…')`: `reads = 3`, `waits = [5000, 5000]` — числа точно совпадают с текстом PROCESS.md-правки («не более трёх снимков с паузами по 5 секунд») | Нет отдельного мутанта на счётчик попыток; граница проверена только тестом, не реестром |
+| Перед перестановкой метки состояние перечитывается (TOCTOU) | `test('#775 write path rechecks current request, stop labels, head and runs; duplicates cannot relabel twice')` — 4 варианта изменения состояния между чтениями все дают `applied: false, writes: 0`; второй прогон той же задачи после успешного relabel тоже `applied: false` | Мутант `pending-no-fresh-snapshot-before-write` (вторая проверка подменяется первой) |
+
+Третья строка таблицы (красный Validate → S6) формально не закрыта
+собственным мутантом в реестре этой задачи — это не недоработка диффа: сам
+red-path не входит в его изменяемую поверхность (код не трогался), а то, что
+diff проверяет (`decideResume` не фильтрует по conclusion), доказано чтением
+кода и тестом-инцидентом #744. Не считаю это пустой третьей колонкой в
+смысле §2.7, т.к. объект защиты («resume не блокирует red Validate») — не то
+же самое, что объект мутации («red Validate переводит в S6») — последний не
+менялся и не нуждается в новом мутанте.
+
+## Находки
+
+Нет High. Нет Medium (ни в скоупе, ни вне скоупа).
+
+Два наблюдения Low, не блокируют и не открывают цикл (track show,
+«бухгалтерия» по REVIEWER.md):
+
+1. `decideResume` в ветке `!sha || headSha !== sha` (scripts/process-resume.mjs:61)
+   возвращает терминальный `noop` без `recheck: true`, в отличие от соседних
+   веток «не нашли прогон/маркер». Если чтение текущего SHA ветки через
+   `git/ref/heads/<branch>` окажется на мгновение неконсистентным (крайне
+   маловероятно для простого ref-запроса), событие потеряется до планового
+   `process-reconcile`. Не блокирует: reconcile — штатная страховка именно на
+   этот случай, а ref-API не входит в число источников, для которых авторы
+   уже наблюдали задержку (это были списки прогонов и артефакты).
+2. `processRuns` (scripts/process-reconcile.mjs:240-267) использует offset-
+   пагинацию (`page=N`) без курсора; при параллельном создании новых прогонов
+   `process.yml` во время обхода возможен классический дрейф страниц (пропуск
+   или дублирование элемента на границе). Дубликаты дедуплицируются по
+   `id/attempt`; пропуск по границе проявился бы как «process history
+   pagination made no progress» — громкий отказ, а не тихая потеря. Это не
+   регрессия: офсетная пагинация тем же способом уже использовалась в коде до
+   #775 (`[1,2].flatMap(...)`), просто без ограничения в 2 страницы. Не
+   блокирует.
+
+## Что проверено и корректно
+
+- Корень всех трёх гипотез issue закрыт структурно, а не патчем под частный
+  случай: `latestReviewRequest` берёт только `labeled`-события (гипотеза 2 про
+  `unlabeled`-прогон снята на уровне типа события, не на уровне эвристики);
+  `reviewRunsForRequest` режет историю по времени текущего запроса метки, что
+  одновременно решает «перепутанный latest после ребейза» (гипотеза 3) и
+  убирает нужду в ограничении размера окна, раз оно больше не читает чужие
+  issue вообще (гипотеза 1 снята инженерно, не обходом).
+- `pendingEvidenceError` ужесточает проверку маркера (issue/run_id/run_attempt
+  /stage/branch-префикс/формат sha и validate_run_id) — все поля уже писались
+  пайплайном до этой задачи (`_process.yml:792-811`, не изменён), значит это
+  не миграция формата, а более строгая проверка существующих данных; разрыва
+  совместимости с уже лежащими в Actions старыми `pending.json` быть не
+  должно, т.к. они писались тем же неизменным шагом.
+- `resume()` перед записью перечитывает состояние целиком (`io.state()` +
+  `io.runs()`), сверяет id запроса, id/attempt прогона — honest TOCTOU-защита,
+  а не формальная галочка (тест явно ловит смену запроса/прогона между
+  чтениями).
+- `pendingOf` в реальных `ops` отдельно отказывает, если уже существует
+  `review-result-*` — защита от двойной траты модели даже если бы
+  `decideResume` её не поймал.
+- CLI (`process-resume.mjs` as main) и вызывающий workflow
+  (`_process-resume.yml`) согласованы: новый обязательный `--run-id` появился
+  в обоих местах одним коммитом, тест `test/process-resume.test.mjs` сверяет
+  текст workflow-файла регэкспом на оба изменения.
+- `User-Visible: no` обоснован: диффне меняет ничего, что видит пользователь
+  карточки; «одно число — один источник» (§8) не применимо — в диффе нет
+  пользовательских величин вообще.
+- Трек show подтверждён на issue (`track:show` label), все критерии §5
+  пройдены (см. «Скоуп»), эскалация/reclassify не нужны.
+
+## Чего не проверял
+
+- Не воспроизводил вживую реальное событие `workflow_run` / доставку webhook
+  GitHub — это прямо исключено и самим автором («живую доставку будущего
+  события до слияния этого исправления доказать нельзя»); единственная
+  доступная проверка — юнит-уровень на точных идентичностях инцидентов, она
+  сделана.
+- Не гонял мутационные прогоны (ни старые, ни новые четыре) — на треке show в
+  разработке это не гейт ревью (§2.7, #709); проверил статически, что все
+  четыре `find`-строки патчей существуют и уникальны в целевых файлах, и что
+  `test/mutation-gate.test.mjs` (реестр целостности) зелёный.
+- Не проверял нагрузочно офсетную пагинацию `processRuns` на реальном
+  GitHub API при высокой конкурентной активности (много параллельных меток в
+  репозитории) — см. Low-наблюдение 2; сочтено приемлемым остаточным риском,
+  унаследованным от уже существующего подхода, а не внесённым этой задачей.
+- Golden/визуальные гейты, инварианты модели, pytest backend, performance —
+  не применимы: diff не трогает `src/**`, рендер, геометрию или Python.
+- Не читал историю Actions-запусков трёх реальных инцидентов (#740/#744/#748)
+  напрямую через `gh api` (не было доступа запрашивать живые GitHub Actions
+  логи сверх уже предоставленных в issue ссылок) — доверился идентичностям
+  (run id, validate id, sha, время), процитированным в issue и воспроизведённым
+  в фикстуре теста; они взаимно согласованы (issue-таблица ↔ тест ↔ код).
+
+## Вердикт
+
+Зелёный. High: 0, Medium: 0. Route: fix (все критерии §5 пройдены).
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `issue/775-resume-pending-round`, коммит `6bece2517939` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `5a1dba2cd87b12c5fa82dbc7342a1e8fa0777d47`
+  ```
+  git log --all --format='%H %T' | grep 5a1dba2cd87b
+  ```
+- Тело issue: `bf0fc310f0258fb8df339bab23938f022ff6b529c332fc038bf8698c8c7b6b0f`
+- Вердикт конвейера: `green` · High 0 · маршрут `fix`
+<!-- hp:usage input_tokens=4456 output_tokens=37035 cache_creation_input_tokens=126045 cache_read_input_tokens=2711875 num_turns=37 -->