From 55079efe46e0707e25a62c8c6cfe4263df566028 Mon Sep 17 00:00:00 2001 From: Dmitry Dementev Date: Thu, 2 Apr 2026 00:07:57 +0300 Subject: [PATCH] =?UTF-8?q?refactor(skills):=20codex-review=20=D0=BF=D0=B5?= =?UTF-8?q?=D1=80=D0=B5=D0=B4=D0=B0=D1=91=D1=82=20=D1=81=D0=BF=D0=B8=D1=81?= =?UTF-8?q?=D0=BE=D0=BA=20=D1=84=D0=B0=D0=B9=D0=BB=D0=BE=D0=B2=20=D0=B2?= =?UTF-8?q?=D0=BC=D0=B5=D1=81=D1=82=D0=BE=20diff?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Зачем: - Codex имеет доступ к репо в read-only sandbox и может сам читать diff — сбор в temp-файл избыточен. - Что: - шаг 1: автодетект дополнен проверкой branch-изменений (BASE_BRANCH...HEAD). - шаг 2: добавлено определение base branch через origin/HEAD + fallback main/master. - шаг 3: diff заменён на --name-only списки (unstaged+staged, branch как fallback). - шаг 4: промпты передают список файлов и git-команды вместо . - убраны temp-файлы claude-diff-*, preflight --shortstat, часть cleanup. - TODO.md: задача отмечена как выполненная. - Проверка: - /codex-review code — одобрено Codex за 3 раунда, 0 permission prompts. Co-Authored-By: Claude Opus 4.6 (1M context) --- TODO.md | 10 +-- agents/.agents/skills/codex-review/SKILL.md | 95 ++++++++++++++------- 2 files changed, 69 insertions(+), 36 deletions(-) diff --git a/TODO.md b/TODO.md index daa7197..0ed8441 100644 --- a/TODO.md +++ b/TODO.md @@ -16,11 +16,7 @@ Реализовано: шаги 2, 3, 7 переписаны на встроенные инструменты (Write tool, stdout capture). См. коммит с `fix(skills): убраны permission prompts из codex-review`. -## codex-review: убрать сбор diff, передавать только список файлов +## ~~codex-review: передавать список файлов вместо diff~~ DONE -Codex работает в `read-only` sandbox с доступом ко всему репо. Ему не нужен -готовый diff в temp-файле — достаточно `git diff --name-only` и контекста -("unstaged changes" / "staged" / "vs main"). - -Это уберёт: шаг 3 (сбор diff), preflight `--shortstat`, temp-файлы `claude-diff-*`, -часть cleanup. Скилл станет значительно проще. \ No newline at end of file +Реализовано: Codex получает `--name-only` список файлов и git-команды, +сам читает diff. Убраны temp-файлы `claude-diff-*`, preflight, часть cleanup. \ No newline at end of file diff --git a/agents/.agents/skills/codex-review/SKILL.md b/agents/.agents/skills/codex-review/SKILL.md index e6cd7e2..94aba63 100644 --- a/agents/.agents/skills/codex-review/SKILL.md +++ b/agents/.agents/skills/codex-review/SKILL.md @@ -21,7 +21,7 @@ user_invocable: true ## Инструкции -> **Плейсхолдеры:** `${REVIEW_ID}` и `${CODEX_SESSION_ID}` в шагах ниже — это шаблонные плейсхолдеры, НЕ shell-переменные. Подставляй литеральные значения напрямую в каждый tool call. +> **Плейсхолдеры:** `${REVIEW_ID}`, `${CODEX_SESSION_ID}` и `${BASE_BRANCH}` в шагах ниже — это шаблонные плейсхолдеры, НЕ shell-переменные. Подставляй литеральные значения напрямую в каждый tool call. ### Шаг 1: Определить режим ревью @@ -29,7 +29,10 @@ user_invocable: true **Автодетект (без явного аргумента):** -1. Проверь `git diff --stat` и `git diff --cached --stat` на наличие изменений кода. +1. Проверь наличие изменений кода (любой непустой — значит есть): + - `git diff --name-only` — unstaged + - `git diff --cached --name-only` — staged + - `git diff --name-only ${BASE_BRANCH}...HEAD` — коммиты ветки 2. Проверь, есть ли план в текущем контексте разговора (из plan mode, задач или обсуждения). | Изменения кода? | План в контексте? | Режим | @@ -46,6 +49,20 @@ user_invocable: true Сгенерируй уникальный `REVIEW_ID` самостоятельно, формат: `{unix_timestamp}-{случайное_4значное_число}`. Пример: `1711872000-4821`. **НЕ используй bash** — подставляй значение напрямую в команды следующих шагов. +Определи base branch репозитория: + +```bash +git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's|refs/remotes/origin/||' +``` + +Если команда вернула пустой результат (remote HEAD не настроен), используй fallback: + +```bash +git rev-parse --verify main 2>/dev/null && echo main || echo master +``` + +Сохрани результат как `BASE_BRANCH` — используется в `git diff ${BASE_BRANCH}...HEAD` далее. + ### Шаг 3: Подготовить материал для ревью **Ревью плана:** @@ -57,32 +74,27 @@ user_invocable: true **Ревью кода:** -Собрать diff — выполняй по одной команде, используй первый непустой результат: +Собери список изменённых файлов: -1. `git diff` — unstaged changes -2. Если stdout пуст — `git diff --cached` — staged changes -3. Если пуст — `git diff main...HEAD` — все коммиты ветки +1. `git diff --name-only` — unstaged changes +2. `git diff --cached --name-only` — staged changes -Каждую команду запускай отдельным Bash tool call (без перенаправлений). -Результат (stdout первой непустой команды) сохрани через **Write tool** в `/tmp/claude-diff-${REVIEW_ID}.diff`. +Объедини unstaged + staged (уникальные пути). Если оба пусты: -Если все три пусты — нет изменений для ревью, сообщи пользователю. +3. `git diff --name-only ${BASE_BRANCH}...HEAD` — коммиты ветки (fallback) -**Большие diff-ы (best-effort preflight):** перед захватом полного diff проверь размер -через `--shortstat` в том же порядке приоритета, что и основной diff: +Branch берётся ТОЛЬКО когда нет локальных изменений — иначе контекст раздувается. +Для branch в промпте указывай команду `git diff ${BASE_BRANCH}...HEAD` (полный diff). -1. `git diff --shortstat` — если непусто, оценивай по нему -2. Если пусто — `git diff --cached --shortstat` -3. Если пусто — `git diff main...HEAD --shortstat` +Codex имеет доступ к репо и сам прочитает полный diff и файлы. +В промпт (шаг 4) передай список файлов и какие git diff команды запускать. -Вывод вида `3 files changed, 4200 insertions(+), 100 deletions(-)`. -Если insertions + deletions > 1500 — **останови сбор diff**, сообщи пользователю -что полный diff может быть обрезан Bash tool, и потребуй использовать `/codex-review <путь>` для конкретных файлов. -**НЕ продолжай** с потенциально неполным diff — частичное ревью хуже, чем отсутствие ревью. -Это best-effort проверка: `--shortstat` считает строки, а не байты. Diff с малым числом -очень длинных строк (минифицированный JSON, lockfile) может пройти порог, но быть огромным по размеру. +**Много файлов (> 50):** если объединённый список превышает 50 путей, +передай в промпт только git-команды без списка файлов — Codex разберётся сам. -**Ревью кода против плана:** подготовить и путь к плану, И diff. +Если все источники пусты — нет изменений для ревью, сообщи пользователю. + +**Ревью кода против плана:** подготовить путь к плану И собрать список изменённых файлов (как выше). ### Шаг 4: Сформировать промпт и запустить первый раунд @@ -98,9 +110,14 @@ Review the implementation plan in . Focus on: 5. Security — any security concerns? ``` -**Промпт для ревью кода:** +**Промпт для ревью кода (<= 50 файлов):** ``` -Review the code changes in . You can also read the full files in the repo for context. Focus on: +Review the code changes in this repo. Changed files: + +<список файлов из --name-only> + +Changes include: . +Run to see the full diffs. Focus on: 1. Bugs — logic errors, off-by-one, null handling, race conditions 2. Edge cases — what inputs or states could break this? 3. Style — does the code follow existing project conventions? @@ -108,10 +125,24 @@ Review the code changes in . You can also read the full files in the 5. Tests — is the change adequately tested? ``` +**Промпт для ревью кода (> 50 файлов):** +``` +Review the code changes in this repo. +Changes include: . +Run to see changed files and full diffs. Focus on: +1. Bugs — ... +(те же 5 пунктов) +``` + **Промпт для ревью кода против плана:** ``` -Review the code changes in against the implementation plan in . -You can also read the full files in the repo for context. Focus on: +Review the code changes in this repo against the implementation plan in . +Changed files: + +<список файлов или пусто если > 50> + +Changes include: <тип>. +Run to see the full diffs. Focus on: 1. Completeness — does the implementation cover all plan steps? 2. Deviations — where does the code differ from the plan? Are deviations justified? 3. Bugs — logic errors, edge cases, null handling @@ -150,7 +181,7 @@ timeout 600 codex exec \ - Команда **синхронная**: когда она вернулась, файл `-o` уже готов. **НЕ** используй poll-loop (`while/sleep`). - Если exit code = 124 (таймаут) — сообщи пользователю и предложи повторить. -**Пример для режима code** (diff в temp-файле): +**Пример для режима code** (unstaged изменения в двух файлах): ```bash timeout 600 codex exec \ @@ -158,7 +189,13 @@ timeout 600 codex exec \ -c model_reasoning_effort=high \ -s read-only \ -o /tmp/codex-review-${REVIEW_ID}.md \ - "Review the code changes in /tmp/claude-diff-${REVIEW_ID}.diff. You can also read the full files in the repo for context. Focus on: + "Review the code changes in this repo. Changed files: + +agents/.agents/skills/codex-review/SKILL.md +TODO.md + +Changes include: unstaged changes. +Run git diff to see the full diffs. Focus on: 1. Bugs — logic errors, off-by-one, null handling, race conditions ... If changes are needed, end with exactly: VERDICT: REVISE" @@ -236,7 +273,7 @@ Please re-review. End with VERDICT: APPROVED or VERDICT: REVISE" **Fallback** — если `resume` не сработал (сессия истекла, session ID не захвачен): -1. Пересобрать diff (аналогично Шагу 3). +1. Собрать список изменённых файлов (аналогично Шагу 3). 2. Запустить свежий `codex exec -o` с описанием предыдущих раундов в промпте. Вернуться к **Шагу 5**. @@ -271,7 +308,7 @@ Please re-review. End with VERDICT: APPROVED or VERDICT: REVISE" ### Шаг 9: Очистка ```bash -rm -f /tmp/claude-plan-${REVIEW_ID}.md /tmp/claude-diff-${REVIEW_ID}.diff /tmp/codex-review-${REVIEW_ID}.md +rm -f /tmp/claude-plan-${REVIEW_ID}.md /tmp/codex-review-${REVIEW_ID}.md ``` НЕ удалять файлы планов, которые существовали до ревью (только temp-файлы, созданные этим скиллом).