refactor(skills): codex-review передаёт список файлов вместо diff
- Зачем: - 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-команды вместо <diff-path>. - убраны 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) <noreply@anthropic.com>
This commit is contained in:
1 parent
6347252d4e
commit
55079efe46
2 files changed
+69
-36
No files matched your search
@@ -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. Скилл станет значительно проще.
|
||||
Реализовано: Codex получает `--name-only` список файлов и git-команды,
|
||||
сам читает diff. Убраны temp-файлы `claude-diff-*`, preflight, часть cleanup.
|
||||
@@ -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 <plan-path>. Focus on:
|
||||
5. Security — any security concerns?
|
||||
```
|
||||
|
||||
**Промпт для ревью кода:**
|
||||
**Промпт для ревью кода (<= 50 файлов):**
|
||||
```
|
||||
Review the code changes in <diff-path>. 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: <unstaged changes / staged changes / unstaged + staged changes / branch changes vs ${BASE_BRANCH}>.
|
||||
Run <git diff commands> 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 <diff-path>. 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: <unstaged changes / staged changes / ...>.
|
||||
Run <git diff commands> to see changed files and full diffs. Focus on:
|
||||
1. Bugs — ...
|
||||
(те же 5 пунктов)
|
||||
```
|
||||
|
||||
**Промпт для ревью кода против плана:**
|
||||
```
|
||||
Review the code changes in <diff-path> against the implementation plan in <plan-path>.
|
||||
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 <plan-path>.
|
||||
Changed files:
|
||||
|
||||
<список файлов или пусто если > 50>
|
||||
|
||||
Changes include: <тип>.
|
||||
Run <git diff commands> 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-файлы, созданные этим скиллом).
|
||||
|
||||
Reference in new issue
Block a user