docs: review document for #562

Issue: #562
User-Visible: no
This commit is contained in:
claude[bot]
2026-09-13 06:56:20 +00:00
parent 7b58f73e57
commit a66649a13b
+200
View File
@@ -0,0 +1,200 @@
# CODE-REVIEW-562-r2
## Скоуп
Issue #562 «Process: отвязать роли и инфраструктурный трек от конкретных
агентов», заход r2, блокирующих циклов израсходовано 1/4 до этого раунда.
Материал ревью — ровно `8f000dd459393cd0d17e9cfa58513a68f7f5ffa1` (рабочая
копия уже на нём). Полный диапазон против `dev`: `origin/dev(9c08d583)..HEAD`
(два содержательных коммита — `53f0635e` разобран и получил жёлтый вердикт в
r1, `8f000dd4` — правка этого раунда; `00ac0875` — только документ r1).
Раунд не первый и предыдущий вердикт был жёлтым (Medium в скоупе), поэтому по
PROCESS.md §2.9 разбор ведётся по дельте: находка r1 плюс всё, до чего дельта
дотягивается, а не полный повторный прогон. Дельта раунда — ровно
`53f0635e..8f000dd4`:
```
scripts/task-packet.mjs | 16 +++++++++++-----
test/task-packet.test.mjs | 17 +++++++++++++++--
2 files changed, 26 insertions(+), 7 deletions(-)
```
Это не ребейз (родитель тот же `dev`-merge-base `9c08d583`, что и в r1),
контракт поведения затронут ровно там же, где была находка r1, новая
подсистема не задета — сокращение объёма законно.
Трейлеры коммита `8f000dd4`: `Issue: #562`, `User-Visible: no` — верны
(правка внутри `scripts/**` и `test/**`, класс A не тронут, changelog не
нужен и не тронут).
## Как проверялось
| Гейт | Результат | Источник |
|---|---|---|
| `npx tsc --noEmit` / `npm test` (полный) / `npm run build` + сверка бандлов | зелёный | Validate на `8f000dd4`, https://github.com/Matysh/houseplan-card/actions/runs/34743589488 (переиспользован по инструкции, не перегонялся) |
| `node --test test/task-packet.test.mjs` (таргетно, по дельте) | зелёный, 9/9 | прогнан в этом ревью |
| Тест на старом коде (`53f0635e`) с тем же файлом теста | красный, 2/9 | прогнан в этом ревью через `git worktree` — доказывает, что новый негативный тест умеет падать (см. находки закрытия r1 ниже) |
| `node scripts/smoke-select.mjs --base 53f0635e --head 8f000dd4` | «Исполняемого frontend-диффа нет... Тронуто файлов: 3» | прогнан; смоки не выбираются — диффу нечего проверять браузером |
| `node scripts/check-docs.mjs` | не запускался | дельта не трогает `src/**` |
| `golden:verify`, `pytest tests_backend`, `npm run invariants` | не запускались | нет визуальных/Python/геометрических изменений |
## Закрытие раунда r1
| Находка r1 | Чем закрыта | Где это видно |
|---|---|---|
| Medium — `task-packet.mjs` определял «инфраструктурный» трек и запрет класса A по тематической метке `infra`, а не по механическому критерию из `process-gate.mjs`, из-за чего продуктовая задача с меткой `infra` + `S6-in-progress` получала неверный ответ «класс A трогать нельзя» | `rightsFor` больше не читает `infra` из `labels` вообще — трек передаётся явным параметром `{ infrastructure }`; `buildPacket` вычисляет его из `branch.infrastructure`, а тот в `collectInputs` — из реального diff `base..ref` через переиспользованный `classify()` из `process-gate.mjs` (тот же критерий, что у блокирующего гейта) | `scripts/task-packet.mjs:22,27,30,110-121,209-210`; воспроизведение — `node -e "rightsFor('S6-in-progress', ['infra'])"` теперь возвращает `'продуктовый код трогать МОЖНО'`, а не запрет |
| (следствие) отсутствие теста на комбинацию `infra` + реальная `S*`-метка | новый тест `test/task-packet.test.mjs:112-120` явно бьёт `labels: ['infra', 'S6-in-progress']` и требует `track === 'полный'` + разрешение на класс A | проверено запуском теста против кода r1 (`53f0635e`) — падает с `AssertionError: expected 'полный', actual 'инфраструктурный'`, то есть тест действительно ловит старый баг, а не проходит тавтологически |
Находка r1 закрыта полностью для того сценария, для которого была написана
(ложное «нельзя» для помеченной `infra` продуктовой задачи). Дельта, однако,
меняет ещё одну ветку того же кода (как трек вычисляется, когда ветки ещё
нет) — это и есть новая находка этого раунда, см. ниже.
## Унаследовано из r1
Приняты без повторной проверки (дельта этого раунда их не касается):
- **AC1** (роли не закреплены за Codex/Claude в `AGENTS.md`/`PROCESS.md`;
локальный `CODEX-RUNBOOK.md` вне материала) и **AC2** (оба документа
одинаково описывают маршруты S1–S8 и инфра-без-S → S7 ↔ S6 → S8) —
`AGENTS.md` и `PROCESS.md` дельтой `8f000dd4` не тронуты вовсе (диф
ограничен `scripts/task-packet.mjs` и `test/task-packet.test.mjs`).
Документ: `docs/reviews/CODE-REVIEW-562-r1.md`, SHA `53f0635e`.
- **AC3** (независимость автора/ревьюера, автоматическое ревью и слияние,
разрешение владельца на выпуск) — механика конвейера этой дельтой не
затронута. Документ: тот же, SHA `53f0635e`.
- **AC4** (исторические документы ревью не переписаны) — дельта `8f000dd4`
не касается `docs/reviews/**` (см. `git show --stat 8f000dd4` в скоупе
выше — только `scripts/` и `test/`). Документ: тот же, SHA `53f0635e`.
- `scripts/process-gate.mjs` и `test/process-gate.test.mjs` — не изменены
этой дельтой (последний раз правились в `53f0635e`, только переименования
сообщений/ссылки на issue); логика `isInfrastructureRange` не тронута.
Документ: тот же, SHA `53f0635e`.
## Находки
### Medium (в скоупе) — после фикса статусless infra-issue без опубликованной ветки получает заведомо неверную подсказку «вне процесса, ждать S-метку владельца»
- **Файл:** `scripts/task-packet.mjs:118` (`const infrastructure = branch?.infrastructure === true;`) и `scripts/task-packet.mjs:39-51` (`rightsFor`, ветка `default`).
- **Что не так:** новый критерий полностью корректен, когда ветка уже
опубликована и `collectInputs` успел посчитать diff. Но пока ветки на
`origin/issue/NN-*` ещё нет (`refs[0]` пуст в `collectInputs`), `branch`
остаётся `null`, и `infrastructure` теперь **всегда** `false` — старый
путь через `labels.includes('infra')`, который умел ответить верно даже
без ветки, убран целиком, а альтернативы для случая «ветки ещё нет» не
появилось.
Это ровно момент, для которого сама задача #562 вводит ускоренный трек:
«Инфраструктурная задача выполняется сразу... без стадий S1–S6 до первого
хендоффа. По готовности ветка публикуется» (тело issue). И это ровно
прописанный сценарий использования инструмента: `AGENTS.md:378` — «Start a
task from its packet... node scripts/task-packet.mjs --issue NN» — команда,
которую в первую очередь запускают при взятии задачи, то есть до того, как
какая-либо ветка опубликована.
- **Воспроизведение:**
```
$ node -e "import('./scripts/task-packet.mjs').then(({ buildPacket }) =>
console.log(buildPacket({
issue: { number: 999, title: 'infra no branch yet', state: 'OPEN', url: 'u', body: '' },
labels: ['infra'],
})))"
track: полный
rights: [
'продуктовый код трогать НЕЛЬЗЯ: статус не S5/S6/S7 (правило №1)',
'статусной метки нет — продуктовая задача вне процесса; вход — первая S*-метка владельца'
]
```
На коде r1 (`53f0635e`) тот же вызов давал верный ответ:
```
track: инфраструктурный
rights: [
'файлы класса A трогать НЕЛЬЗЯ; инфраструктурную реализацию МОЖНО вести сразу по issue (#562)',
'инфраструктурный вход: реализовать и проверить → push ветки → S7-code-review; ТЗ и S1–S6 не нужны (#562)'
]
```
Это регресс, внесённый именно правкой этого раунда, а не унаследованная
проблема: до `8f000dd4` тот же вход обрабатывался верно.
- **Почему это не поймал новый тест:** оба новых/изменённых теста этого
раунда (`test/task-packet.test.mjs:20-24,112-120`) передают `branch`
явным объектом с уже посчитанным `infrastructure`. Ни один тест не
вызывает `buildPacket`/`rightsFor` для statusless `infra`-issue **без**
`branch` — то есть без ветки, что и есть штатное состояние в момент
«взять задачу и вызвать пакет».
- **Серьёзность:** Medium, в скоупе (файл и тест изменены этим же диффом).
Не High: `task-packet.mjs` — консультативный инструмент, ничего не решает
и не блокирует ни один реальный push; блокирующий гейт (`process-gate.mjs`)
метку не читает и этой правкой не затронут. Риск — неверная подсказка
ровно в момент, когда агент решает, можно ли начинать инфраструктурную
реализацию без S-метки: вместо разрешения «делай сразу» пакет теперь
говорит «жди S-метку от владельца», что для инфра-трека прямо противоречит
тексту issue #562 и `PROCESS.md`.
- **Что чинить:** для случая без ветки не возвращать уверенное «вне
процесса» — либо явно писать неопределённость («трек не подтверждён без
diff опубликованной ветки; по метке `infra` похоже на инфраструктурный»),
либо (эквивалент предложения r1, вариант «привязать трек к metки без
diff», применённый только к before-branch состоянию) при отсутствии ветки
временно доверять `labels.includes('infra')` как единственному
доступному сигналу, но маркировать его в выводе как непроверенный
diff'ом, а не как решённый факт.
## Что проверено и корректно
- Находка r1 закрыта именно так, как и была сформулирована: `task-packet.mjs`
для мисклассифицированной продуктовой задачи (`infra` + реальная
`S6-in-progress`) больше не запрещает класс A — трек и права теперь
выводятся из того же механического критерия (`classify()` из
`process-gate.mjs`), что и у блокирующего гейта, а не из темы issue.
- Новый тест `#562: the infra label alone never grants the accelerated
track` — не тавтологичен: проверено падением на снапшоте `53f0635e`
(`AssertionError`, 2 упавших теста из 9).
- Тесты дельты: 9/9 зелёных на `8f000dd4`.
- Трейлеры `Issue: #562` / `User-Visible: no` верны; класс A не тронут;
changelog не требуется и не тронут — соответствует характеру правки.
- `docs/reviews/**` этой дельтой не тронут — AC4 остаётся выполненным.
- `smoke-select` подтверждает: diff не задевает `src/**`/`demo/**`, браузерные
смоки этому раунду не нужны.
## Чего не проверял и почему
- Полный `npx tsc --noEmit` / `npm test` / `npm run build` не перегонялись —
переиспользован зелёный Validate на этом самом SHA `8f000dd4` (ссылка
выше); вместо этого таргетно перегнан изменённый тестовый файл и его же
версия на предыдущем SHA (для проверки «тест умеет падать»).
- `AGENTS.md`, `PROCESS.md`, локальный `CODEX-RUNBOOK.md` — не перечитывались
заново: дельта раунда их не трогает (см. «Унаследовано из r1»).
- `check-docs`, browser-смоки, `golden:verify`, `pytest tests_backend`,
`npm run invariants` — не запускались: дельта ограничена `scripts/**` и
`test/**`, ни `src/**`, ни `demo/**`, ни Python, ни геометрия не тронуты.
- Реальное поведение `scripts/process-gate.mjs` (блокирующий гейт) — не
перепроверялось: этой дельтой файл не изменён, вывод r1 (диффо-зависимый,
метку `infra` не читает) остаётся в силе.
## Материал раунда
- SHA материала: `8f000dd459393cd0d17e9cfa58513a68f7f5ffa1`
- Дельта раунда: `53f0635eddcd66346bca3bc2a92056d333ed1ca3..8f000dd459393cd0d17e9cfa58513a68f7f5ffa1` (2 файла, 26 строк)
- Полный диапазон против `dev`: `origin/dev(9c08d583)..HEAD`
- Дерево: рабочая копия на этом SHA во время всего разбора
---
<!-- material-anchors: сгенерировано конвейером (#414) -->
## Материал раунда
- Ветка: `issue/562-agent-neutral-workflow`, коммит `8f000dd45939` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
- Дерево материала: `36ef122f5778df5144077d79a3584a54a421ee1e`
```
git log --all --format='%H %T' | grep 36ef122f5778
```
- Тело issue: `520e2e2d0230ee82ee9e86795aa5e61e4172442aa24bea85a9429dd5aa7677ae`
- Вердикт конвейера: `yellow` · High 0