mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -0,0 +1,197 @@
|
||||
# CODE-REVIEW-562-r1
|
||||
|
||||
## Скоуп
|
||||
|
||||
Issue #562 «Process: отвязать роли и инфраструктурный трек от конкретных агентов» —
|
||||
инфраструктурная задача (класс A не задет ни одним файлом): `AGENTS.md`,
|
||||
`PROCESS.md`, `scripts/process-gate.mjs`, `scripts/task-packet.mjs`,
|
||||
`test/process-gate.test.mjs`, `test/task-packet.test.mjs`. Материал ревью —
|
||||
ровно `53f0635eddcd66346bca3bc2a92056d333ed1ca3` (единственный коммит впереди
|
||||
`origin/dev`, `merge-base` = `9c08d583`). Трейлеры: `Issue: #562`,
|
||||
`User-Visible: no` — корректны, changelog не требуется и не тронут.
|
||||
|
||||
Заход r1, цикл 0/4.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
| Гейт | Результат | Источник |
|
||||
|---|---|---|
|
||||
| `npx tsc --noEmit` | зелёный | Validate на `53f0635e`, https://github.com/Matysh/houseplan-card/actions/runs/34742978297 (переиспользован по указанию, не перегонялся) |
|
||||
| `npm test` (полный) | зелёный, 2560/2559+1 skip | тот же прогон Validate |
|
||||
| `npm run build` + сверка бандлов | зелёный | тот же прогон Validate |
|
||||
| `node test/task-packet.test.mjs` (таргетно) | зелёный, 8/8 | прогнан в этом ревью |
|
||||
| `node test/process-gate.test.mjs` (таргетно) | зелёный, 35/35 | прогнан в этом ревью |
|
||||
| `node scripts/check-docs.mjs` | не запускался | diff не трогает `src/**` — правило §8 не требует |
|
||||
| смоки (`demo/smoke_*.mjs`) | не запускались | diff не трогает `demo/**`/`src/**`; `smoke-select` по этому диффу тоже неприменим |
|
||||
| `golden:verify` | не запускался | нет визуальных изменений |
|
||||
| `pytest tests_backend` | не запускался | Python не тронут |
|
||||
| `npm run invariants` | не запускался | геометрия не тронута |
|
||||
|
||||
Основная работа этого раунда — чтение диффа против `origin/dev` (`git diff
|
||||
origin/dev...HEAD`), сверка `AGENTS.md`/`PROCESS.md` с телом issue и
|
||||
воспроизведение поведения `scripts/task-packet.mjs` (`rightsFor`) напрямую
|
||||
через `node -e`, а не только чтением.
|
||||
|
||||
## AC → как доказан
|
||||
|
||||
- **AC1** (AGENTS.md/PROCESS.md/локальный CODEX-RUNBOOK.md не закрепляют роли
|
||||
за Codex/Claude) — проверено чтением. `AGENTS.md:204` и `PROCESS.md:516,913`
|
||||
упоминают Codex/Claude только как равноправные примеры «любой агент»;
|
||||
`PROCESS.md:911-912` называет `anthropics/claude-code-action` текущей
|
||||
технической реализацией ревьюера и прямо оговаривает, что это деталь
|
||||
автоматизации, а не закрепление роли. Локальный `CODEX-RUNBOOK.md`
|
||||
(`C:\Users\Admin\...`) физически вне репозитория и вне материала ревью —
|
||||
не проверялся, проверить нечем.
|
||||
- **AC2** (оба документа одинаково описывают маршруты S1–S8 и инфра-без-S →
|
||||
S7 ↔ S6 → S8) — проверено чтением. Формулировка идентична по существу в
|
||||
`AGENTS.md` («Agent-neutral workflow») и `PROCESS.md` (§1, диаграмма §2,
|
||||
§9, блок §14). CODEX-RUNBOOK.md — не проверялся (вне репозитория).
|
||||
- **AC3** (независимость автора/ревьюера, автоматическое ревью, автоматическое
|
||||
слияние, прямое разрешение владельца на выпуск) — проверено чтением. §6 и
|
||||
§10.4 `PROCESS.md`, соответствующий раздел `AGENTS.md` не тронуты по
|
||||
механике, только переформулированы; поведение конвейера (событие от метки,
|
||||
ребейз/слияние, `S8-merged` после push) не изменилось.
|
||||
- **AC4** (исторические документы ревью не переписаны) — доказано `git diff
|
||||
origin/dev...HEAD --stat`: `docs/reviews/**` в диапазоне отсутствует.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium — `task-packet.mjs` определяет инфра-трек и право трогать класс A по тематической метке `infra`, а не по механическому критерию, и это противоречит соседнему модулю в том же диффе
|
||||
|
||||
- **Файл:** `scripts/task-packet.mjs:29-30,36-38,49-51,115-117`
|
||||
- **Что не так:** `rightsFor` и `buildPacket` относят issue к «инфраструктурному»
|
||||
треку и запрещают класс A по одному признаку — наличию метки `infra` в
|
||||
`labels`. Но:
|
||||
1. `PROCESS.md` §9 (эта строка диффом не тронута) прямо называет `infra`
|
||||
**тематической меткой, ортогональной процессу**, наравне с `polish`,
|
||||
`tests`, `docs`, `security`, `vacuum` — то есть меткой, которая ничего
|
||||
не решает и может стоять на любой продуктовой задаче любого трека.
|
||||
2. Механический критерий инфраструктурной задачи, который эта же задача
|
||||
подтверждает и в `AGENTS.md`, и в `PROCESS.md` §1, — «ни одного файла
|
||||
класса A в диапазоне», а не метка.
|
||||
3. `scripts/process-gate.mjs:431-434` (комментарий сохранён этим же диффом,
|
||||
не переписан) объясняет ровно это: «Исключение опирается на diff, а не
|
||||
на метку-разрешение: метку `infra` можно поставить продуктовой задаче и
|
||||
увести продуктовый коммит от проверки статуса». Реальный гейт
|
||||
(`isInfrastructureRange`) действительно смотрит только на классы файлов
|
||||
коммитов, не на метки issue.
|
||||
|
||||
`task-packet.mjs` — единственное место в этом диффе, которое читает метку
|
||||
`infra` как источник правды об «инфраструктурности», и делает это ровно там,
|
||||
где `process-gate.mjs` в том же диффе объясняет, почему так делать нельзя.
|
||||
|
||||
- **Воспроизведение (не только чтением — вызовом функции):**
|
||||
|
||||
```
|
||||
$ node -e "import('./scripts/task-packet.mjs').then(({ rightsFor }) =>
|
||||
console.log(rightsFor('S6-in-progress', ['infra'])))"
|
||||
[
|
||||
'файлы класса A трогать НЕЛЬЗЯ; инфраструктурную реализацию МОЖНО вести сразу по issue (#562)',
|
||||
'следующий шаг: gate:small + смоки по AC → push ветки → метка S7-code-review (метку после push)'
|
||||
]
|
||||
```
|
||||
|
||||
По правилу №1 issue в `S6-in-progress` разрешено трогать класс A — это
|
||||
единственный смысл статуса. Метка `infra`, по документированному §9
|
||||
значению, орthogональна и не должна это отменять (пример: полноценная
|
||||
продуктовая задача с темой «инфраструктура/бэкенд», прошедшая полный
|
||||
S1→S8, помечена и `S6-in-progress`, и тематически `infra` — ничто в
|
||||
процессе такое сочетание не запрещает). Для такой задачи `task-packet.mjs`
|
||||
выдаёт заведомо неверный ответ на прямой вопрос «можно ли трогать класс A»
|
||||
— тот самый вопрос, ради которого инструмент существует
|
||||
(`права выводятся из статусной метки по правилу №1`, комментарий в
|
||||
`test/task-packet.test.mjs:13`).
|
||||
|
||||
Это не гипотетический контрпример: в репозитории уже есть открытые issue с
|
||||
меткой `infra` одновременно со статусной `S*`-меткой (#557 `S8-merged`,
|
||||
#541 `S6-in-progress`) — оба сейчас действительно инфраструктурные по
|
||||
содержанию, так что конкретно на них ответ инструмента случайно верен, но
|
||||
ничто в процессе не гарантирует, что продуктовая задача не получит ту же
|
||||
комбинацию меток.
|
||||
|
||||
- **Почему это не поймали тесты:** новый тест
|
||||
`test/task-packet.test.mjs:19-23` проверяет только `rightsFor(null,
|
||||
['infra'])` (статуса нет вообще) — счастливый путь, совпадающий с
|
||||
ментальной моделью автора. Нет ни одного теста на `infra` вместе с реальной
|
||||
`S*`-меткой, то есть ровно на тот случай, для которого модуль отвечает
|
||||
неверно.
|
||||
- **Серьёзность:** Medium, в скоупе задачи (файл изменён этим же диффом).
|
||||
Не High: `task-packet.mjs` — консультативный инструмент («источник правды
|
||||
остаётся GitHub и git: пакет ничего не пишет и ничего не решает», собственный
|
||||
комментарий модуля), реальный блокирующий гейт (`process-gate.mjs`,
|
||||
pre-push/CI) метку `infra` не читает и остаётся диффо-зависимым, так что
|
||||
ни один реальный push не будет пропущен или отклонён из-за этого дефекта.
|
||||
Риск — недостоверная подсказка агенту/человеку, читающему пакет задачи,
|
||||
вплоть до попытки продуктовой правки под видом инфраструктурной или
|
||||
наоборот отказа от легитимной правки класса A.
|
||||
- **Что чинить:** либо не выводить «инфраструктурность» из метки `infra`
|
||||
вовсе (использовать наличие/отсутствие `S*`-метки как единственный
|
||||
предсказуемый по issue-данным сигнал — без диапазона коммитов `task-packet`
|
||||
и так не может проверить класс файлов, значит стоит явно писать «трек
|
||||
неизвестен без diff» вместо утвердительного «нельзя/можно»), либо
|
||||
однозначно developed заново привязать «инфраструктурный трек» к отсутствию
|
||||
`S*`-метки (что уже и так вычисляется как `status === null`), а не к метке
|
||||
`infra`, и добавить тест на комбинацию `infra` + реальный `S*`.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Единственный коммит в диапазоне, трейлеры `Issue: #562` / `User-Visible: no`
|
||||
верны и соответствуют характеру изменения (документация процесса + два
|
||||
файла класса B, ни одного класса A).
|
||||
- `AGENTS.md` и `PROCESS.md` согласованно описывают продуктовый маршрут
|
||||
`S1`→`S8` и инфраструктурный `без S → S7-code-review ↔ S6-in-progress →
|
||||
S8-merged»; терминология («ускоренный вход», «accelerated entry») совпадает
|
||||
по смыслу в обоих документах.
|
||||
- Ни Codex, ни Claude не закреплены нормативно ни за одной ролью ни в
|
||||
`AGENTS.md`, ни в `PROCESS.md`; единственное оставшееся упоминание
|
||||
`anthropics/claude-code-action` явно помечено как деталь текущей
|
||||
автоматизации, а не правило.
|
||||
- `scripts/process-gate.mjs`: правки — только текст сообщений/комментариев
|
||||
(замена ссылки `#118` → `#562`, уточнение формулировки); логика
|
||||
`isInfrastructureRange`, `checkIssueStatuses`, `statusOptional` не менялась
|
||||
и остаётся диффо-зависимой, как и было. Соответствующие тесты
|
||||
(`test/process-gate.test.mjs`) — тоже только переименования, проходят
|
||||
35/35.
|
||||
- `docs/reviews/**` не тронут — AC4 выполнено буквально.
|
||||
- Изменение не задевает `src/**`, `custom_components/**/*.py`, i18n,
|
||||
манифесты — инфраструктурная классификация задачи верна механически.
|
||||
|
||||
## Чего не проверял и почему
|
||||
|
||||
- Локальный `CODEX-RUNBOOK.md` владельца — физически вне репозитория и вне
|
||||
материала ревью (SHA `53f0635e` его не содержит); проверить существование
|
||||
и содержание невозможно с этой стороны. Это ограничение AC1/AC2 для
|
||||
третьего документа, а не находка.
|
||||
- `node scripts/check-docs.mjs`, браузерные смоки, `golden:verify`, `pytest
|
||||
tests_backend`, `npm run invariants` — не гоняны: diff не касается
|
||||
`src/**`, `demo/**`, Python и геометрии, поэтому по §8 они не обязательны
|
||||
для этой задачи.
|
||||
- Полный `npx tsc --noEmit` / `npm test` / `npm run build` не перегонялись
|
||||
заново — переиспользован зелёный прогон Validate на этом самом SHA
|
||||
(ссылка выше), как разрешено инструкцией по этой задаче; вместо этого
|
||||
таргетно перегнаны оба изменённых test-файла напрямую.
|
||||
- Реальное использование `task-packet.mjs` агентами на живых issue (кроме
|
||||
приведённого воспроизведения `rightsFor`) — инструмент не входит в
|
||||
блокирующий гейт, поэтому эффект находки ограничен качеством подсказки, а
|
||||
не корректностью пайплайна; это отражено в оценке серьёзности как Medium,
|
||||
а не High.
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- SHA материала: `53f0635eddcd66346bca3bc2a92056d333ed1ca3`
|
||||
- Диапазон: `origin/dev(9c08d583)..HEAD`, один коммit
|
||||
- Дерево: рабочая копия на этом SHA во время всего разбора
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/562-agent-neutral-workflow`, коммит `53f0635eddcd` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `5eef2a99a76777df0a8642b5156e6755083e8f8e`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 5eef2a99a767
|
||||
```
|
||||
- Тело issue: `520e2e2d0230ee82ee9e86795aa5e61e4172442aa24bea85a9429dd5aa7677ae`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user