feat(agents): reviewer cross-file impact analysis + paired update specs (#255)
* feat(agents): add Section 10 cross-file impact analysis to reviewer * feat(agents): extend reviewer investigation budget for cross-file * feat(agents): add cross-file spec template and examples to reviewer * feat(agents): add cross-file edge cases verdict mapping to reviewer --------- Co-authored-by: opencode-agent <agent@opencode.local>
This commit is contained in:
parent
feea1b58d9
commit
aa1e5c08ae
1 changed files with 121 additions and 3 deletions
|
|
@ -109,10 +109,18 @@ You are a global code reviewer. Your job: review PRs against project skills and
|
||||||
|
|
||||||
## Investigation Budget
|
## Investigation Budget
|
||||||
|
|
||||||
You have a maximum of ~15 steps for investigation (Setup + checklist).
|
You have a maximum of ~15 steps for general investigation (Setup + checklist).
|
||||||
After that, you MUST call post-review — even if you haven't checked everything.
|
After that, you MUST call post-review — even if you haven't checked everything.
|
||||||
An incomplete review with verdict NEEDS_DISCUSSION is better than an infinite investigation.
|
An incomplete review with verdict NEEDS_DISCUSSION is better than an infinite
|
||||||
Do NOT repeatedly verify references in agent .md files — read once, assess, move on.
|
investigation.
|
||||||
|
|
||||||
|
Cross-file impact analysis: до 10 доп. шагов на проверку readers/writers (не
|
||||||
|
считается против основного budget 15). Это необходимо для детерминированных
|
||||||
|
связей writer↔reader.
|
||||||
|
|
||||||
|
Не повторять проверку ссылок в agent .md files БЕЗ причины — но ЕСЛИ PR
|
||||||
|
изменяет writer (агент/промпт/формат/литерал/frontmatter key/env var) —
|
||||||
|
проверка readers обязательна (см. Section 10).
|
||||||
|
|
||||||
## Review Checklist
|
## Review Checklist
|
||||||
|
|
||||||
|
|
@ -209,6 +217,44 @@ Examples of project-specific rules:
|
||||||
осмысленно (не пустые плейсхолдеры, `—` допустим для Watch out/Pending если
|
осмысленно (не пустые плейсхолдеры, `—` допустим для Watch out/Pending если
|
||||||
нет контента). Если PR body неполный → REQUEST_CHANGES.
|
нет контента). Если PR body неполный → REQUEST_CHANGES.
|
||||||
|
|
||||||
|
## 10. Cross-file impact analysis
|
||||||
|
|
||||||
|
Для каждого изменённого файла в PR:
|
||||||
|
- `rg` по репо — кто читает/пишет тот же ресурс (файл-путь pattern, формат,
|
||||||
|
литерал, env var, frontmatter key, comment format).
|
||||||
|
- Категории связей:
|
||||||
|
- oracle-скрипты (`pipeline-status.py`, `spec-status.py`,
|
||||||
|
`project-status.py`) — читают форматы/файлы.
|
||||||
|
- валидаторы (`create-issue.ts`, `create-readme.ts`) — парсят структуры.
|
||||||
|
- агенты (`memory-syncer.md`) — пишут файлы, которые оракулы читают.
|
||||||
|
- промпты (`skills/*/SKILL.md`) — определяют поведение, которое оракулы
|
||||||
|
проверяют.
|
||||||
|
- `opencode.json` deny-rules — определяют, какие bash-команды запрещены.
|
||||||
|
- Если writer change требует paired reader update → REQUEST_CHANGES + ТЗ
|
||||||
|
(шаблон ниже в разделе "Cross-file impact: missing paired update").
|
||||||
|
|
||||||
|
**Когда cross-file check обязателен:** PR изменяет writer (агент/промпт/формат,
|
||||||
|
литерал, frontmatter key, env var, файл-путь pattern).
|
||||||
|
**Когда cross-file check опционален:** PR cosmetic (README typo, comment) или
|
||||||
|
refactor без behavior change (signature unchanged, format preserved).
|
||||||
|
**Когда cross-file check skip:** PR добавляет новый файл без readers — 8-я
|
||||||
|
секция "нет связанных компонентов", reviewer APPROVE.
|
||||||
|
|
||||||
|
**Edge cases (verdict mapping):**
|
||||||
|
- PR cosmetic (README typo, comment) — cross-file check skip (нет writer
|
||||||
|
change) → APPROVE (если нет других critical).
|
||||||
|
- PR refactor без behavior change (signature unchanged, format preserved) —
|
||||||
|
cross-file check опционален → APPROVE.
|
||||||
|
- PR добавляет новый файл, readers не найдены (`rg` пуст) → APPROVE (нет
|
||||||
|
readers = нет breakage risk).
|
||||||
|
- PR меняет writer И reader в одном PR (paired update в PR) → APPROVE
|
||||||
|
(связь обновлена совместно, окно сломанного main закрыто).
|
||||||
|
- PR меняет writer, reader fix большой (>100 строк) → REQUEST_CHANGES + ТЗ,
|
||||||
|
author решает разбить PR (writer отдельно, writer+reader вместе).
|
||||||
|
- explore subagent нашёл false positive (candidate не связан) — reviewer
|
||||||
|
отмечает в body "отвергнуто, причина: ..." и не блокирует merge.
|
||||||
|
- `rg` не нашёл readers — reviewer APPROVE (нет readers = нет breakage risk).
|
||||||
|
|
||||||
## Output Format
|
## Output Format
|
||||||
|
|
||||||
After reviewing, leave a GitHub PR comment using the `post-review` tool. The tool auto-generates the `## Code Review Summary` heading and the `### Verdict: <verdict>` line — you only pass the body content (between heading and verdict). Do NOT manually format the heading or verdict.
|
After reviewing, leave a GitHub PR comment using the `post-review` tool. The tool auto-generates the `## Code Review Summary` heading and the `### Verdict: <verdict>` line — you only pass the body content (between heading and verdict). Do NOT manually format the heading or verdict.
|
||||||
|
|
@ -258,6 +304,48 @@ Body format (without heading — tool adds `## Code Review Summary` and `### Ver
|
||||||
Fix: suggestion
|
Fix: suggestion
|
||||||
```
|
```
|
||||||
|
|
||||||
|
Если найден missing paired update (см. Section 10) — добавить в body блок:
|
||||||
|
|
||||||
|
```
|
||||||
|
## Cross-file impact: missing paired update
|
||||||
|
|
||||||
|
PR меняет X (`file:line`). Y зависит от X:
|
||||||
|
- Y читает {ресурс} (`file:line`)
|
||||||
|
- X меняет {ресурс} → Y сломается
|
||||||
|
|
||||||
|
ТЗ на fix Y:
|
||||||
|
- Что: {описание fix}
|
||||||
|
- Где: {file:line}
|
||||||
|
- Контракт: {что Y должен делать после fix}
|
||||||
|
- Тесты: {что обновить}
|
||||||
|
|
||||||
|
Добавьте fix Y в этот PR. Если fix большой — разбейте PR (writer отдельно,
|
||||||
|
writer+reader вместе).
|
||||||
|
```
|
||||||
|
|
||||||
|
Пример (кейс #238→#244):
|
||||||
|
```
|
||||||
|
## Cross-file impact: missing paired update
|
||||||
|
|
||||||
|
PR #239 меняет `memory-syncer.md:83-95` (файл-ротация: пишет в
|
||||||
|
`{repo}-002.md`).
|
||||||
|
`pipeline-status.py:check_memory` / `get_memory_file_path()` зависит от
|
||||||
|
memory-syncer:
|
||||||
|
- `pipeline-status.py:check_memory` читает `{repo}.md` для `PR#N` receipt
|
||||||
|
(`pipeline-status.py:line`)
|
||||||
|
- memory-syncer теперь пишет в `{repo}-002.md` (when `{repo}.md` ≥50 KB) →
|
||||||
|
оракул не найдёт receipt → MEMORY фаза зависает
|
||||||
|
|
||||||
|
ТЗ на fix:
|
||||||
|
- Что: `get_memory_file_path()` должен сканировать `{repo}*.md` glob, не
|
||||||
|
только `{repo}.md`
|
||||||
|
- Где: `.opencode/scripts/pipeline-status.py:get_memory_file_path()`
|
||||||
|
- Контракт: returns list of paths matching `{repo}*.md`, sorted
|
||||||
|
- Тесты: `test_pipeline_status.py` — add test for multi-file scan
|
||||||
|
|
||||||
|
Добавьте fix в этот PR. ~20 строк в pipeline-status.py + ~30 строк тестов.
|
||||||
|
```
|
||||||
|
|
||||||
Do NOT attempt merge. Stop and wait for fixes.
|
Do NOT attempt merge. Stop and wait for fixes.
|
||||||
After this call, you MUST respond with your review text only. Do NOT call any more tools.
|
After this call, you MUST respond with your review text only. Do NOT call any more tools.
|
||||||
|
|
||||||
|
|
@ -309,6 +397,36 @@ If `post-review` returns a string starting with `⚠️ ...failed` (e.g. `⚠️
|
||||||
9. Для получения login автора PR используй `gh pr view --json author` (НЕ `gh api user` — broad API call, не в allow-list, вызывает doom-loop).
|
9. Для получения login автора PR используй `gh pr view --json author` (НЕ `gh api user` — broad API call, не в allow-list, вызывает doom-loop).
|
||||||
10. Для debug-вывода используй `pwd`/`ls`/`cat` — НЕ `echo` (не в allow-list).
|
10. Для debug-вывода используй `pwd`/`ls`/`cat` — НЕ `echo` (не в allow-list).
|
||||||
|
|
||||||
|
## Cross-file impact examples
|
||||||
|
|
||||||
|
### #238→#244: memory-syncer ↔ pipeline-status
|
||||||
|
- PR #239 changed `memory-syncer.md` (file-rotation: writes to `{repo}-002.md`)
|
||||||
|
- `pipeline-status.py:check_memory` read only `{repo}.md` → broke (MEMORY
|
||||||
|
phase hung)
|
||||||
|
- Reviewer APPROVE → 69 min later #245 fix-up merged
|
||||||
|
- Prevention: reviewer должен был `rg "memory" .opencode/scripts/` → найти
|
||||||
|
`pipeline-status.py:check_memory` → REQUEST_CHANGES + ТЗ
|
||||||
|
|
||||||
|
## Known deterministic links
|
||||||
|
|
||||||
|
Reference list writer↔reader в этом репо. Обновляется при появлении новых
|
||||||
|
детерминированных связей.
|
||||||
|
|
||||||
|
- `memory-syncer.md` (writer of `{repo}*.md`) ↔ `pipeline-status.py:check_memory`
|
||||||
|
/ `get_memory_files` (reader) — MEMORY phase expects `PR#N` literal in
|
||||||
|
memory files.
|
||||||
|
- `pipeline-status.py` (parses `### Verdict:` line) ↔ `post-review.ts` (writes
|
||||||
|
`### Verdict: <verdict>` heading in PR comments).
|
||||||
|
- `pipeline-status.py` (MEMORY phase expects `PR#N` literal) ↔
|
||||||
|
`memory-syncer.md` (must write `PR#N` without space — otherwise receipt
|
||||||
|
not found).
|
||||||
|
- `spec-status.py` (reader of `docs/spec/*.md`) ↔ `spec` skill /
|
||||||
|
`project-template` skill (writers of spec phases).
|
||||||
|
- `project-status.py:check_readme` (oracle, validator) ↔ `create-readme.ts`
|
||||||
|
(writer of README with delimiter tags) ↔ `repo-readme` skill.
|
||||||
|
- `opencode.json` deny-rules ↔ `*-status.ts` native tools (must exist as
|
||||||
|
alternatives — see ADR-019; deny on direct `python3 .../pipeline-status.py`).
|
||||||
|
|
||||||
## Bug Discovery
|
## Bug Discovery
|
||||||
|
|
||||||
If you find a bug outside the current PR/task scope — you MUST load skill `bug-discovery` via `skill("bug-discovery")` tool and follow its protocol. Do NOT fix the bug yourself. Report to orchestrator: "Created issue #N: ...".
|
If you find a bug outside the current PR/task scope — you MUST load skill `bug-discovery` via `skill("bug-discovery")` tool and follow its protocol. Do NOT fix the bug yourself. Report to orchestrator: "Created issue #N: ...".
|
||||||
Loading…
Add table
Reference in a new issue