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

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 пройдены).

---

Материал раунда

• Ветка: issue/775-resume-pending-round, коммит 6bece2517939 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: 5a1dba2cd87b12c5fa82dbc7342a1e8fa0777d47

  git log --all --format='%H %T' | grep 5a1dba2cd87b
  

• Тело issue: bf0fc310f0258fb8df339bab23938f022ff6b529c332fc038bf8698c8c7b6b0f
• Вердикт конвейера: green · High 0 · маршрут fix