From 01ce4817c32556bf690006e77252d47e2fa0f72c Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sun, 13 Sep 2026 07:04:49 +0000 Subject: [PATCH] docs: review document for #562 Issue: #562 User-Visible: no --- docs/reviews/CODE-REVIEW-562-r3.md | 195 +++++++++++++++++++++++++++++ 1 file changed, 195 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-562-r3.md diff --git a/docs/reviews/CODE-REVIEW-562-r3.md b/docs/reviews/CODE-REVIEW-562-r3.md new file mode 100644 index 00000000..ea4262f3 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-562-r3.md @@ -0,0 +1,195 @@ +# CODE-REVIEW-562-r3 + +## Скоуп + +Issue #562 «Process: отвязать роли и инфраструктурный трек от конкретных +агентов», заход r3, блокирующих циклов израсходовано 2/4 до этого раунда. +Прошлый вердикт — жёлтый (Medium в скоупе), документ +`docs/reviews/CODE-REVIEW-562-r2.md`, материал `8f000dd459393cd0d17e9cfa58513a68f7f5ffa1`. + +**Материал этого раунда — ровно `d6180f7f5de6991fc7bc678a2203da820993c544`**, +рабочая копия уже на нём. `git fetch`/`checkout` не выполнялись. + +Ветка приведена конвейером к `dev` между раундами: поверх авторского +`b75130f7` (тот же коммит, что вёл в комментарии автора к r3, локально +эквивалентен `d6180f7f` — см. проверку ниже) легло 2 коммита `dev` +(`744f502b` «ci: include dynamic backend inputs in gates (#542)» и +`f6e5651c` «docs: review document for #542»). После ребейза формально +другой код (§7.2) → **разбор в этом раунде ведётся полностью**, а не по +дельте. + +Проверил цену этого решения предметно, а не только продекларировал: +`git diff b75130f7 d6180f7f --stat` показывает ровно 6 файлов — +`docs/TESTING.md`, `docs/reviews/CODE-REVIEW-542-r1.md`, +`scripts/check-inputs.mjs`, `scripts/mutation-gate.mjs`, +`test/check-inputs.test.mjs`, `test/gate-reuse.test.mjs` — это в точности +содержимое двух указанных коммитов `dev` (#542), никаких иных различий +между до- и после-ребейзным деревом нет. Дополнительно проверил, что три +коммита самой задачи #562 (`process:` / `fix: доказывать infra-трек…` / +`fix: подсказывать infra-вход…`) физически идентичны по содержимому +до и после ребейза: `git diff 8f000dd4 7b58f73e` (те же логические коммиты +пред- и пост-ребейз) отличается ровно на те же 6 файлов #542, то есть сам +diff задачи #562 ребейз не тронул ни байтом. Правки #542 (`check-inputs.mjs`, +`mutation-gate.mjs`, backend-гейт CI) и правки #562 (`task-packet.mjs`, +`process-gate.mjs`, `AGENTS.md`, `PROCESS.md`) не пересекаются ни одним +файлом — интерференции нет. Разбор веду полным по объёму (все AC заново), +но с этим установленным фактом на руках: риск, из-за которого правило §7.2 +существует, здесь предметно не реализовался. + +Полный диапазон `origin/dev(f6e5651c)..HEAD`, 5 содержательных для истории +задачи коммитов (2 — документы предыдущих раундов, не код): + +``` +3934f8d8 process: отвязать роли от конкретных агентов (#562) — материал r1 +f5388259 docs: review document for #562 — документ r1 +7b58f73e fix: доказывать infra-трек по diff (#562) — материал r2 (= 8f000dd4 до ребейза) +a66649a1 docs: review document for #562 — документ r2 +d6180f7f fix: подсказывать infra-вход до появления ветки (#562) — материал r3 (= b75130f7 до ребейза) +``` + +`git diff origin/dev...HEAD --stat`: `AGENTS.md`, `PROCESS.md`, +`scripts/process-gate.mjs`, `scripts/task-packet.mjs`, +`test/process-gate.test.mjs`, `test/task-packet.test.mjs`, плюс два новых +файла `docs/reviews/CODE-REVIEW-562-{r1,r2}.md`. Класс изменений: A не +затронут вовсе (`AGENTS.md`/`PROCESS.md` — класс C, `scripts/**`/`test/**` — +класс B) — задача сама себя корректно относит к инфраструктурному треку. +Трейлеры на всех трёх содержательных коммитах: `Issue: #562`, +`User-Visible: no` — верно, продуктовое поведение не менялось, changelog не +нужен и не тронут. + +## Как проверялось + +| Гейт | Прогнан | Результат | +|---|---|---| +| `npx tsc --noEmit` / `npm test` (весь набор) / `npm run build` + сверка бандлов | нет, переиспользован | Validate на `d6180f7f` green: https://github.com/Matysh/houseplan-card/actions/runs/34743984705 | +| `node --test test/task-packet.test.mjs` | да | 10/10 зелёных | +| `node --test test/process-gate.test.mjs` | да | 35/35 зелёных | +| Тот же `test/task-packet.test.mjs` против кода r2 (`7b58f73e`, через `git worktree`) | да | 8/10 — новый тест `#562: before a branch exists the infra label prompts classification but grants no rights` падает (`AssertionError: /предварительно/` не совпало с `'полный'`), т.е. тест реально ловит регресс r2, а не проходит тавтологически | +| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | да | «Исполняемого frontend-диффа нет… Browser-smoke этим диффом не выбираются» — выбирать нечего | +| `node scripts/check-docs.mjs` | нет | diff не касается `src/**` | +| `golden:verify`, `pytest tests_backend`, `npm run invariants` | нет | diff не меняет рендер/геометрию/Python | +| `git diff`-инспекция интерференции с #542 (соседние коммиты `dev` от ребейза) | да | 0 пересекающихся файлов (см. «Скоуп») | + +## Закрытие раунда r2 + +| Находка r2 | Чем закрыта | Где это видно | +|---|---|---| +| Medium — до появления ветки (`branch === null`) `infrastructure` был всегда `false`, и statusless issue с меткой `infra` получал от `task-packet.mjs` заведомо неверный ответ «вне процесса, ждать S-метку владельца» вместо «инфраструктурный вход, делай сразу» | Введён отдельный флаг `infrastructureHint = branch == null && status == null && labels.includes('infra')`. Track для этого состояния теперь `'инфраструктурный (предварительно; подтвердить путями/diff)'`, а `rightsFor` добавляет явную строку «метка infra — только подсказка, не доказательство и не право: до ветки проверь предполагаемые пути; без class A начинай сразу, при любом class A нужен продуктовый S-flow» и меняет `default`-ветку switch на «предварительный инфраструктурный вход: после проверки отсутствия class A реализовать → push ветки → S7-code-review; diff станет окончательным доказательством». Это ровно вариант, предложенный самой находкой r2 («явно писать неопределённость... маркировать как непроверенный diff'ом, а не как решённый факт»), а не решение «доверять метке безусловно» — поэтому находка r1 (метка = безусловное доказательство трека) этим не реанимируется | `scripts/task-packet.mjs:27,39-51,118-125`; воспроизвёл сам: `buildPacket({issue:{...}, labels:['infra']})` (без `branch`) на `d6180f7f` даёт `track: 'инфраструктурный (предварительно; подтвердить путями/diff)'` и три строки rights, включая подсказку — команда и полный вывод см. «Как проверялось» | +| (методологический риск: не поймать регресс тавтологичным тестом) | Новый тест `test/task-packet.test.mjs:125-136` (`#562: before a branch exists...`) целенаправленно вызывает `buildPacket` **без** `branch` — именно тот путь, который r2 поймал руками, а старые тесты не покрывали | Проверено запуском той же версии теста против кода r2 (`7b58f73e`) через `git worktree`: падает (`AssertionError`), то есть тест не тавтологичен | + +Находка r2 закрыта: сценарий «взять задачу, вызвать пакет до push ветки» теперь +даёт согласованную рекомендацию «после проверки путей — начинай», а не +блокирующее «жди метку владельца», что для инфраструктурного трека +противоречило бы самому issue #562. При этом граница r1 (тематическая +метка НЕ даёт права на класс A и НЕ подменяет механический критерий, когда +ветка уже есть) не нарушена: `infrastructureHint` действует только при +`branch == null && status == null`, тест `#562: the infra label alone never +grants the accelerated track` (метка `infra` + реальный `S6-in-progress`) +остаётся зелёным и продолжает требовать `track === 'полный'`. + +## Унаследовано из r2 / r1 + +Формально это полный разбор (ребейз, §7.2), но там, где текст между +раундами буквально не изменился, вывод предыдущих раундов подтверждён +повторным чтением, а не просто принят на слово: + +- **AC1** (роли не закреплены за Codex/Claude в `AGENTS.md`/`PROCESS.md`; + локальный `CODEX-RUNBOOK.md` вне репозитория и вне материала ревью) — + перечитал `AGENTS.md` («Agent-neutral workflow») и `PROCESS.md` (раздел + ролей) целиком в этом раунде; текст идентичен тому, что проверяли r1/r2 + (дельта `3934f8d8` не тронута ни `7b58f73e`, ни `d6180f7f` — оба диффа + ограничены `scripts/task-packet.mjs`/`test/task-packet.test.mjs`). + Документ первичного вывода: `docs/reviews/CODE-REVIEW-562-r1.md`, SHA + `53f0635e` (= `3934f8d8` после ребейза). +- **AC2** (оба документа одинаково описывают маршруты S1–S8 и + инфра-без-S → S7 ↔ S6 → S8) — то же самое: перечитал диаграмму `PROCESS.md` + §2 и абзац `AGENTS.md` про accelerated entry заново, они согласуются + друг с другом дословно так же, как на r1. +- **AC3** (независимость автора/ревьюера, автоматическое ревью и слияние, + разрешение владельца на выпуск) — механика конвейера (`S7-code-review` + триггерит автоматический ревью, зелёный вердикт мержит и ставит + `S8-merged`) не менялась ни одним из трёх коммитов #562; `process-gate.mjs` + и `test/process-gate.test.mjs` в этой задаче меняют только текст + сообщений/комментариев (см. далее), не логику. +- **AC4** (исторические документы ревью не переписаны) — `docs/reviews/ + CODE-REVIEW-562-r1.md` и `-r2.md` в полном диапазоне `origin/dev...HEAD` + присутствуют только как добавленные файлы (`git diff --stat`: только + insertions, 0 deletions на этих путях) — ни один существующий файл + `docs/reviews/**` не тронут. +- `scripts/process-gate.mjs` / `test/process-gate.test.mjs` — перечитал + диф заново (полный разбор): оба файла в этом раунде меняют только строки + комментариев и текст сообщений (`#118` → `#562`, обновлённая + формулировка), логика `isInfrastructureRange`/`checkIssueStatuses` не + затронута — прогнал `test/process-gate.test.mjs` целиком (35/35) для + проверки, что переименования не сломали ни одной проверки. + +## Проверка AC issue #562 (полная, по коду этого SHA) + +| AC | Проверка | Результат | +|---|---|---| +| AC1: `AGENTS.md`/`PROCESS.md`/локальный `CODEX-RUNBOOK.md` не закрепляют работу за Codex/Claude | Прочитал оба файла целиком в затронутых разделах: `AGENTS.md` «Agent-neutral workflow» — «No task type is reserved for Codex, Claude or any other named model»; `PROCESS.md` — таблица ролей по агентам заменена абзацем «Роли не закреплены за моделями или именами агентов». Таблица `Исполнитель → роли` из старой версии удалена полностью (`git diff` подтверждает: `-\| Исполнитель \| Роли \|` и весь блок). `CODEX-RUNBOOK.md` — не в репозитории (путь `C:\Users\...`), вне материала ревью, как и на r1/r2 | Выполнено для двух документов в репозитории; третий вне доказуемости этим ревью — унаследовано с r1 | +| AC2: оба документа одинаково описывают продуктовый S1–S8 и инфраструктурный «без S → S7 ↔ S6 → S8» | `PROCESS.md` §2: `инфраструктурный трек (§1): без S → S7-code-review ⟲ S6-in-progress → S8-merged`; `AGENTS.md`: «Once the branch is ready and pushed, apply `S7-code-review`... green review... sets `S8-merged`; findings or a failed merge return the issue to `S6-in-progress`». Формулировки эквивалентны машиночитаемо (те же 4 состояния, та же стрелка возврата) | Выполнено | +| AC3: независимость автора/ревьюера, автоматическое ревью, автоматическое слияние, прямое разрешение владельца на выпуск | `AGENTS.md`: «Author and reviewer are independent agents/sessions... the reviewer must start without implementation context and must not be the author grading their own work»; releases остаются «explicitly commanded by the owner». Механизм автослияния (`S7-code-review` → controller → `dev` → `S8-merged`) в `PROCESS.md` не переписан по существу, только убрано закрепление за `Claude`/`Codex` в описании техреализации ревьюера («Текущая техническая реализация независимого ревьюера — `anthropics/claude-code-action`; это деталь автоматизации, а не закрепление роли») | Выполнено | +| AC4: исторические документы ревью не переписываются | `git diff origin/dev...HEAD --stat` показывает `docs/reviews/CODE-REVIEW-562-r1.md`/`-r2.md` только как новые файлы (100% insertions); ни один файл вида `CODE-REVIEW-*-r*.md`/`SPEC-REVIEW-*-r*.md` другой задачи не в диапазоне | Выполнено | + +## Находки + +Нет. High — 0, Medium — 0, Low — 0. Находка r2 закрыта предметно (см. таблицу +выше), новых находок в дельте `7b58f73e..d6180f7f` и в остальном полном +разборе не обнаружено. + +## Что проверено и корректно + +- Регресс r2 (statusless `infra`-issue без ветки получал команду «жди + S-метку») исправлен именно так, как просила находка: неопределённость + явно названа («подсказка, не доказательство и не право»), а не превращена + в новое безусловное доверие метке — граница r1 (метка не даёт прав, когда + ветка/diff уже есть) не нарушена, подтверждено прогоном теста «the infra + label alone never grants the accelerated track». +- Новый тест на регресс r2 не тавтологичен — воспроизвёл падение на + предыдущем SHA через `git worktree`. +- `classify()` для вычисления `branch.infrastructure` переиспользуется из + `scripts/process-gate.mjs` — тот же механический критерий, что у + блокирующего гейта, а не отдельная копия правила. +- Ребейз конвейера не внёс интерференции: файлы, добавленные `dev` в это + же дерево (#542, `check-inputs.mjs`/`mutation-gate.mjs`/`docs/TESTING.md`), + не пересекаются ни с одним файлом, который трогает #562. +- Трейлеры `Issue: #562` / `User-Visible: no` верны на всех трёх + содержательных коммитах; изменение чисто инфраструктурное (класс A не + тронут), changelog не требуется и не тронут. +- `docs/reviews/**` не переписан — только добавлены документы прошлых + раундов задачи. + +## Чего не проверял и почему + +- Полный `npx tsc --noEmit` / `npm test` / `npm run build` не перегонял — + переиспользован зелёный Validate именно на материале ревью (`d6180f7f`, + ссылка выше); прогнал таргетно изменённые тестовые файлы и негативную + пробу на предыдущем SHA вместо этого. +- `node scripts/check-docs.mjs`, browser-смоки, `golden:verify`, + `pytest tests_backend`, `npm run invariants` — не запускал: diff не + касается `src/**`, `demo/**`, Python или геометрии; `smoke-select` + подтвердил отсутствие исполняемого frontend-диффа. +- Локальный `CODEX-RUNBOOK.md` владельца — физически вне репозитория и вне + материала ревью, не проверялся (как и на r1/r2). +- Содержимое коммитов #542, попавших в дерево ребейзом, не ревьюировал + повторно по существу — они уже прошли собственный код-ревью + (`docs/reviews/CODE-REVIEW-542-r1.md`, зелёный вердикт) и не входят в + диапазон `origin/dev...HEAD` этой задачи; проверил только отсутствие + файловой интерференции с #562. + +Вердикт: зелёный · заход r3 · блокирующих циклов 2/4 · High: 0 · Medium: 0 + +--- + + + +## Материал раунда + +- Ветка: `issue/562-agent-neutral-workflow`, коммит `d6180f7f5de6` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `c34dca167de81bea95a3c17b8436bcecb1760dfd` + ``` + git log --all --format='%H %T' | grep c34dca167de8 + ``` +- Тело issue: `520e2e2d0230ee82ee9e86795aa5e61e4172442aa24bea85a9429dd5aa7677ae` +- Вердикт конвейера: `green` · High 0