mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -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
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/562-agent-neutral-workflow`, коммит `d6180f7f5de6` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `c34dca167de81bea95a3c17b8436bcecb1760dfd`
|
||||
```
|
||||
git log --all --format='%H %T' | grep c34dca167de8
|
||||
```
|
||||
- Тело issue: `520e2e2d0230ee82ee9e86795aa5e61e4172442aa24bea85a9429dd5aa7677ae`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user