diff --git a/issues/issue-35-issue-plan-cross-review-required.md b/issues/issue-35-issue-plan-cross-review-required.md new file mode 100644 index 0000000..92a1582 --- /dev/null +++ b/issues/issue-35-issue-plan-cross-review-required.md @@ -0,0 +1,94 @@ +# Issue 35: issue-plan-strategy 個別 PR cross-review 必須化 + +## 関連リンク + +- GitHub Issue: https://github.com/devbasex/ai-plugins/issues/35 +- 関連 Skill: `plugins/ndf-shared/skills/issue-plan-strategy/SKILL.md` +- 関連 Skill: `plugins/ndf-shared/skills/cross-review/SKILL.md` + +## 概要 + +`ndf:issue-plan-strategy` の multi-PR ワークフローで、個別 PR のレビューが軽量レビューだけで済まされ、重大バグが release 統合後まで残る運用を防ぐ。 + +Step 6 を「個別 PR は原則 `/ndf:cross-review` 必須」と読める内容に変更し、release PR Ready 前の前提条件とアンチパターンを明文化する。 + +## 問題・背景 + +現行の Step 6 は `/ndf:review-branch`、`/ndf:review`、`/ndf:cross-review` を選択肢として並べているため、個別 PR を Claude Code の code-reviewer や単発レビューだけで release ブランチへ merge できるように読める。 + +その運用では、release PR 側の cross-review が個別 PR 範囲の重大バグをまとめて検出する形になり、ワークフロー自身が禁止している「release PR で個別 PR 範囲の指摘を解決する」状態に近づく。 + +## 修正対象 + +- `plugins/ndf-shared/skills/issue-plan-strategy/SKILL.md` +- `plugins/ndf-shared/skills/cross-review/SKILL.md` +- `plugins/ndf-claude/skills/issue-plan-strategy/SKILL.md` +- `plugins/ndf-codex/skills/issue-plan-strategy/SKILL.md` +- `plugins/ndf-kiro/skills/issue-plan-strategy/SKILL.md` +- 必要に応じて `docs/ndf-plugin-reference.md` + +runtime 別配布物は `plugins/ndf-shared` を正とし、`scripts/build-runtime-plugins.sh` で同期する。 + +## タスク分解 + +### Task 1: Step 6 のレビュー方針を強化 + +- **対象ファイル:** `plugins/ndf-shared/skills/issue-plan-strategy/SKILL.md` +- **変更内容:** 個別 PR は原則 `/ndf:cross-review ` を実行し、codex + gemini の収束を確認してから release ブランチへ merge する、と明記する。 + +### Task 2: 軽量レビューの位置づけを限定 + +- **対象ファイル:** `plugins/ndf-shared/skills/issue-plan-strategy/SKILL.md` +- **変更内容:** `/ndf:review-branch` は PR 作成前のセルフレビュー、`/ndf:review` は例外的な単発確認に限定する。Claude Code の code-reviewer 単発レビューを cross-review の代替にしない方針を明記する。 + +### Task 3: release PR Ready 前チェックを追加 + +- **対象ファイル:** `plugins/ndf-shared/skills/issue-plan-strategy/SKILL.md` +- **変更内容:** Step 8 の body 最終化 / Ready for review 前チェックに「全個別 PR が cross-review approved 済み」を追加する。省略した場合のフォールバックは個別 PR の状態別に明記する: **open なら** 当該個別 PR で `/ndf:cross-review` を回す（release PR には回さない。ループ内 `/ndf:fix` が release PR を修正し原則が崩れるため）、**既に release へ merge 済みなら** 元の差分は release ブランチにあり新規 PR には乗らないため release PR で `/ndf:cross-review` を回して追認する。いずれも後追い対応で手戻りが増える点も記載する。 + +### Task 4: アンチパターン追記 + +- **対象ファイル:** `plugins/ndf-shared/skills/issue-plan-strategy/SKILL.md` +- **変更内容:** 「個別 PR を cross-review せず、Claude Code の code-reviewer / 単発レビューだけで release へ merge する」をアンチパターン表に追加する。 + +### Task 5: cross-review 側との整合確認 + +- **対象ファイル:** `plugins/ndf-shared/skills/cross-review/SKILL.md` +- **変更内容:** issue-plan-strategy から見た cross-review の役割と矛盾がないか確認する。必要なら関連リンクまたは利用場面の説明を補足する。 + +### Task 6: runtime 配布物同期 + +- **対象ファイル:** `plugins/ndf-claude/`, `plugins/ndf-codex/`, `plugins/ndf-kiro/` +- **変更内容:** `bash scripts/build-runtime-plugins.sh` を実行し、shared の変更を runtime 別配布物へ反映する。 + +### Task 7: cross-review skill を model 起動可能化（実装中に追加） + +- **対象ファイル:** `plugins/ndf-shared/skills/cross-review/SKILL.md` +- **背景:** 本 issue の実装中、cross-review を毎回スラッシュコマンドで手入力する必要があった（`disable-model-invocation: true` によりモデルから起動不可のため）ことから、追加要望として対応した。 +- **変更内容:** `disable-model-invocation: true` を削除し、メインセッションから Skill tool 経由で起動可能にする。あわせて `when_to_use` を追加し、通常の単発レビュー依頼は `/ndf:review`、本 skill は収束ループを明示したときのみという責務分担を明文化する（重い codex + gemini 収束ループが単発レビュー依頼で自動選択されるのを防ぐ）。 + +## PR 分割計画 + +単一 PR で進める。変更は主に skill 文書の運用ルール強化で、コード変更や複数機能の段階的 merge は不要。 + +| PR # | branch 名 | 概要 | 依存 | 並行可否 | +|---|---|---|---|---| +| 1 | `docs/issue-35-require-cross-review-per-pr` | issue-plan-strategy の個別 PR cross-review 必須化と runtime 同期 | なし | - | + +release branch: なし +base branch: `main` + +## 影響範囲 + +- `ndf:issue-plan-strategy` の multi-PR 実行手順 +- 個別 PR と release PR のレビュー責務分担 +- release PR Ready 前のチェックリスト +- runtime 別 NDF plugin 配布物 +- `ndf:cross-review` skill の起動方式（model 起動可能化 + `when_to_use` 追加。Task 7） + +## テスト計画 + +- [ ] `bash scripts/build-runtime-plugins.sh --check` +- [ ] `bash scripts/validate-runtime-plugins.sh` +- [ ] Markdown link check が通ることを確認する +- [ ] `plugins/ndf-shared/skills/issue-plan-strategy/SKILL.md` と runtime 別コピーに drift がないことを確認する diff --git a/plugins/ndf-claude/skills/cross-review/SKILL.md b/plugins/ndf-claude/skills/cross-review/SKILL.md index 5a63867..034dae4 100644 --- a/plugins/ndf-claude/skills/cross-review/SKILL.md +++ b/plugins/ndf-claude/skills/cross-review/SKILL.md @@ -1,8 +1,8 @@ --- name: cross-review description: "Run iterative Codex and Gemini PR reviews." +when_to_use: "PR を codex + gemini 両方でレビューし、両者 APPROVE まで自動収束させたいときに限定して使う。明示トリガ: 'cross-review', 'クロスレビュー', '両AIレビュー', '収束レビュー', 'codex と gemini でレビュー'。通常の単発 PR レビュー依頼 (第二意見が 1 回欲しい等) は本 skill を選ばず /ndf:review を使う。重い収束ループ (codex+gemini を複数ラウンド起動) のため、単発レビューと責務を明確に分ける。" argument-hint: "[PR番号] [--max-rounds N] [--rotate-after K] [--rotate-mode light|squash] [--only codex|gemini] [--focus TEXT] [--extra-instructions-file PATH]" -disable-model-invocation: true allowed-tools: - Bash - Read @@ -472,4 +472,7 @@ pint / larastan / test / build などは **中断** を原則とする。 - `/ndf:codex` — codex CLI 呼び出し手順 - `/ndf:gemini` — gemini CLI 呼び出し手順 - `/ndf:resolve-pr-comments` — Resolve Conversation の詳細 +- `/ndf:issue-plan-strategy` — multi-PR ワークフローでは **個別 PR ごとに本 cross-review が原則必須**。 + `/ndf:review` 単発や Claude Code の `code-reviewer` は代替にせず、release ブランチへ merge する前に + codex + gemini の APPROVE 収束を確認する (Step 6) - `general-purpose` エージェント — fix 実行用サブエージェント diff --git a/plugins/ndf-claude/skills/issue-plan-strategy/SKILL.md b/plugins/ndf-claude/skills/issue-plan-strategy/SKILL.md index 4275a37..fdae1ec 100644 --- a/plugins/ndf-claude/skills/issue-plan-strategy/SKILL.md +++ b/plugins/ndf-claude/skills/issue-plan-strategy/SKILL.md @@ -64,7 +64,7 @@ issue 取得 ─┤ │ 既存 plan ─▶│ Step 3: release branch 作成 + Draft release PR │ │ Step 4: 個別 PR ブランチ作成 + 各 Draft PR (release base) │ Step 5: git worktree で並行開発 (依存関係を考慮) │ - │ Step 6: 個別 PR ごとに /ndf:review or /ndf:cross-review + │ Step 6: 個別 PR ごとに /ndf:cross-review (原則必須) │ │ → /ndf:fix → merge into release │ │ Step 7: release ブランチで結合テスト相当のレビュー │ │ Step 8: release PR body 最終化 → Ready & merge │ @@ -125,7 +125,7 @@ git push -u origin release/ ### レビュアー視点の原則 (release PR body の大前提) -個別 PR はセルフレビュー (`/ndf:cross-review` 等) で merge される。**人間のレビュアーが見るのは release PR だけ**であり、個別 PR の存在をレビュアーに意識させてはならない。したがって: +個別 PR は AI による収束レビュー (`/ndf:cross-review` 等) で merge される。**人間のレビュアーが見るのは release PR だけ**であり、個別 PR の存在をレビュアーに意識させてはならない。したがって: - release PR の body は **self-contained 必須**: 「何のために」(背景・解決したい課題) と「何を」(release ブランチ全体としての変更内容) を、**個別 PR を一切参照せずに**理解できる粒度で書く - 個別 PR リンクの列挙を body の本文にしない。開発中の進捗管理に使う場合は `
` 折りたたみ内の補足情報に格下げする @@ -225,16 +225,22 @@ git worktree add ../--ui feature/-ui ## Step 6: 個別 PR のレビュー -**レビューは原則個別 PR 単位**で行う: +**個別 PR は原則 `/ndf:cross-review ` を必須**とする。codex + gemini の両者が +`APPROVE` に収束したことを確認してから Draft を解除し、release ブランチへ merge する。 +個別 PR で重大バグを取りこぼすと、release PR 側の cross-review がまとめて検出する形に +なり、本 skill が禁止する「release PR で個別 PR 範囲の指摘を解決する」状態に陥る。 -| 用途 | コマンド | -|---|---| -| PR 作成前のセルフレビュー | `/ndf:review-branch` | -| GitHub 上の単体レビュー | `/ndf:review ` | -| codex + gemini 両方の収束ループ | `/ndf:cross-review ` | -| 指摘の修正 | `/ndf:fix ` | +| 用途 | コマンド | 位置づけ | +|---|---|---| +| PR 作成前のセルフレビュー | `/ndf:review-branch` | push / PR 化の前段。cross-review の代替にはしない | +| 個別 PR の収束レビュー (原則必須) | `/ndf:cross-review ` | codex + gemini 両方の APPROVE 収束を確認する本線 | +| GitHub 上の例外的な単発確認 | `/ndf:review ` | ごく軽微な差分の単発確認に限定。cross-review の代替にはしない | +| 指摘の修正 | `/ndf:fix ` | cross-review ループ内・後で自動起動される | -個別 PR が APPROVE → Draft 解除 → release ブランチへ merge (squash 推奨)。 +- Claude Code の `code-reviewer` などの単発レビュアーや `/ndf:review` の単発レビューを + **cross-review の代替にしない**。単発レビューは片側 AI の一発判定にとどまり、収束ループを + 回さないため取りこぼしが残る。 +- 個別 PR が cross-review で APPROVE → Draft 解除 → release ブランチへ merge (squash 推奨)。 ## Step 7: release ブランチのレビュー (結合テスト相当のみ) @@ -246,7 +252,7 @@ release ブランチへの merge が一通り進んだ段階で: - 設定値の重複・矛盾 - migration の順序依存 - E2E シナリオ (`/ndf:playwright-scenario-test` の活用) -- ここで個別 PR 範囲のバグが見つかった場合は、**release PR にコメントせず**、該当の個別 PR (既に merge 済みなら新しい修正 PR を release 配下に作成) 側に指摘を書き込み、修正ループを回す +- ここで **新たに** 個別 PR 範囲のバグが見つかった場合は、**release PR にコメントせず**、該当の個別 PR (既に merge 済みなら修正差分を載せた新しい修正 PR を release 配下に作成) 側に指摘を書き込み、修正ループを回す。この場合レビュー対象は **修正差分** であり新規 PR でレビューできる（元の差分がそもそも cross-review 未実施だったケースは扱いが異なるため Step 8 のフォールバック参照） - release PR には integration 観点の指摘のみ残す ## Step 8: release PR body の最終化と release → default の merge @@ -264,11 +270,27 @@ gh pr edit --title "..." --body "..." 最終化のチェック観点 (Step 3 のレビュアー視点の原則を満たすこと): +- [ ] **全個別 PR が `/ndf:cross-review` で APPROVE 収束済み** (Step 6 の前提。未実施の PR が残っていないこと) - [ ] 「何のために」「何を」が個別 PR や plan ファイルを辿らずに理解できる - [ ] 実装中の方針変更・スコープ増減が body に反映されている - [ ] 個別 PR への参照が本文に残っていない (`
` 内の開発用情報は残してよい) - [ ] 内部用語 (round、rotated 等) が漏れていない +> **cross-review を省略した個別 PR が残っている場合のフォールバック**: Ready for review の前に +> 未 cross-review の個別 PR を特定し、その **状態に応じて** 対応する: +> +> - **個別 PR がまだ open**: その個別 PR に対して `/ndf:cross-review <個別PR番号>` を回して APPROVE +> 収束させてから release へ merge する。**release PR に対して直接は回さない** — ループ内の +> `/ndf:fix` が release PR を修正・Resolve してしまい、「個別 PR 範囲の指摘は個別 PR 側で解決する」 +> 原則 (Step 7) が崩れるため。 +> - **個別 PR が既に release へ merge 済み** (元の差分が release ブランチに取り込まれ、新規 PR には +> 乗らない): release PR に対して `/ndf:cross-review ` を回し、**release PR 全体を +> 改めてレビューする**（当該差分もその中に含まれる。個別差分だけを抽出しての再レビューにはならず、 +> release PR 全体が対象になるぶん手戻りが大きい）。この場合ループ内の `/ndf:fix` は release ブランチを +> 直接修正する **追認的な対応** になる（個別 PR 単位のレビューは既に取り返せないため）。 +> +> いずれも後追い対応で手戻りが増えるので、原則は Step 6 で各個別 PR を cross-review 済みにしておくこと。 + ### Draft 解除と merge release PR が APPROVE されたら: @@ -319,6 +341,7 @@ git checkout release/ | release ブランチを作らず巨大な 1 PR で出す | レビュー困難・revert 困難・並行開発不可 | | 個別 PR の base を default にする | release で統合する意味が失われ、partial merge が default を汚染する | | 個別 PR Draft 作成を実装後に回す | PR 番号が未確定でクロス参照や CI 待機の段取りが組めない | +| 個別 PR を cross-review せず、Claude Code の code-reviewer 等の単発レビューだけで release へ merge する | 片側 AI の一発判定で収束ループを回さないため重大バグを取りこぼし、release PR 側でまとめて検出され手戻りが増える (Step 6) | | release PR で個別 PR 範囲の指摘を解決しようとする | 該当 PR が既に閉じている場合、コミット意図がずれる | | release PR の body を個別 PR リンクの列挙だけにする | レビュアーは release PR 単体で変更を把握できず、個別 PR や plan を辿ることになる。body は self-contained 必須 (Step 3 / Step 8) | | body 最終化せずに Ready for review にする | Draft 作成時の plan ベースの暫定 body のままだと実装の最終形と乖離する | diff --git a/plugins/ndf-codex/skills/cross-review/SKILL.md b/plugins/ndf-codex/skills/cross-review/SKILL.md index 208437b..473b07f 100644 --- a/plugins/ndf-codex/skills/cross-review/SKILL.md +++ b/plugins/ndf-codex/skills/cross-review/SKILL.md @@ -1,8 +1,8 @@ --- name: cross-review description: "Run iterative Codex and Gemini PR reviews." +when_to_use: "PR を codex + gemini 両方でレビューし、両者 APPROVE まで自動収束させたいときに限定して使う。明示トリガ: 'cross-review', 'クロスレビュー', '両AIレビュー', '収束レビュー', 'codex と gemini でレビュー'。通常の単発 PR レビュー依頼 (第二意見が 1 回欲しい等) は本 skill を選ばず /ndf:review を使う。重い収束ループ (codex+gemini を複数ラウンド起動) のため、単発レビューと責務を明確に分ける。" argument-hint: "[PR番号] [--max-rounds N] [--rotate-after K] [--rotate-mode light|squash] [--only codex|gemini] [--focus TEXT] [--extra-instructions-file PATH]" -disable-model-invocation: true allowed-tools: - Bash - Read @@ -472,4 +472,7 @@ pint / larastan / test / build などは **中断** を原則とする。 - `/ndf:codex` — codex CLI 呼び出し手順 - `/ndf:gemini` — gemini CLI 呼び出し手順 - `/ndf:resolve-pr-comments` — Resolve Conversation の詳細 +- `/ndf:issue-plan-strategy` — multi-PR ワークフローでは **個別 PR ごとに本 cross-review が原則必須**。 + `/ndf:review` 単発や Claude Code の `code-reviewer` は代替にせず、release ブランチへ merge する前に + codex + gemini の APPROVE 収束を確認する (Step 6) - `general-purpose` エージェント — fix 実行用サブエージェント diff --git a/plugins/ndf-codex/skills/issue-plan-strategy/SKILL.md b/plugins/ndf-codex/skills/issue-plan-strategy/SKILL.md index 4275a37..fdae1ec 100644 --- a/plugins/ndf-codex/skills/issue-plan-strategy/SKILL.md +++ b/plugins/ndf-codex/skills/issue-plan-strategy/SKILL.md @@ -64,7 +64,7 @@ issue 取得 ─┤ │ 既存 plan ─▶│ Step 3: release branch 作成 + Draft release PR │ │ Step 4: 個別 PR ブランチ作成 + 各 Draft PR (release base) │ Step 5: git worktree で並行開発 (依存関係を考慮) │ - │ Step 6: 個別 PR ごとに /ndf:review or /ndf:cross-review + │ Step 6: 個別 PR ごとに /ndf:cross-review (原則必須) │ │ → /ndf:fix → merge into release │ │ Step 7: release ブランチで結合テスト相当のレビュー │ │ Step 8: release PR body 最終化 → Ready & merge │ @@ -125,7 +125,7 @@ git push -u origin release/ ### レビュアー視点の原則 (release PR body の大前提) -個別 PR はセルフレビュー (`/ndf:cross-review` 等) で merge される。**人間のレビュアーが見るのは release PR だけ**であり、個別 PR の存在をレビュアーに意識させてはならない。したがって: +個別 PR は AI による収束レビュー (`/ndf:cross-review` 等) で merge される。**人間のレビュアーが見るのは release PR だけ**であり、個別 PR の存在をレビュアーに意識させてはならない。したがって: - release PR の body は **self-contained 必須**: 「何のために」(背景・解決したい課題) と「何を」(release ブランチ全体としての変更内容) を、**個別 PR を一切参照せずに**理解できる粒度で書く - 個別 PR リンクの列挙を body の本文にしない。開発中の進捗管理に使う場合は `
` 折りたたみ内の補足情報に格下げする @@ -225,16 +225,22 @@ git worktree add ../--ui feature/-ui ## Step 6: 個別 PR のレビュー -**レビューは原則個別 PR 単位**で行う: +**個別 PR は原則 `/ndf:cross-review ` を必須**とする。codex + gemini の両者が +`APPROVE` に収束したことを確認してから Draft を解除し、release ブランチへ merge する。 +個別 PR で重大バグを取りこぼすと、release PR 側の cross-review がまとめて検出する形に +なり、本 skill が禁止する「release PR で個別 PR 範囲の指摘を解決する」状態に陥る。 -| 用途 | コマンド | -|---|---| -| PR 作成前のセルフレビュー | `/ndf:review-branch` | -| GitHub 上の単体レビュー | `/ndf:review ` | -| codex + gemini 両方の収束ループ | `/ndf:cross-review ` | -| 指摘の修正 | `/ndf:fix ` | +| 用途 | コマンド | 位置づけ | +|---|---|---| +| PR 作成前のセルフレビュー | `/ndf:review-branch` | push / PR 化の前段。cross-review の代替にはしない | +| 個別 PR の収束レビュー (原則必須) | `/ndf:cross-review ` | codex + gemini 両方の APPROVE 収束を確認する本線 | +| GitHub 上の例外的な単発確認 | `/ndf:review ` | ごく軽微な差分の単発確認に限定。cross-review の代替にはしない | +| 指摘の修正 | `/ndf:fix ` | cross-review ループ内・後で自動起動される | -個別 PR が APPROVE → Draft 解除 → release ブランチへ merge (squash 推奨)。 +- Claude Code の `code-reviewer` などの単発レビュアーや `/ndf:review` の単発レビューを + **cross-review の代替にしない**。単発レビューは片側 AI の一発判定にとどまり、収束ループを + 回さないため取りこぼしが残る。 +- 個別 PR が cross-review で APPROVE → Draft 解除 → release ブランチへ merge (squash 推奨)。 ## Step 7: release ブランチのレビュー (結合テスト相当のみ) @@ -246,7 +252,7 @@ release ブランチへの merge が一通り進んだ段階で: - 設定値の重複・矛盾 - migration の順序依存 - E2E シナリオ (`/ndf:playwright-scenario-test` の活用) -- ここで個別 PR 範囲のバグが見つかった場合は、**release PR にコメントせず**、該当の個別 PR (既に merge 済みなら新しい修正 PR を release 配下に作成) 側に指摘を書き込み、修正ループを回す +- ここで **新たに** 個別 PR 範囲のバグが見つかった場合は、**release PR にコメントせず**、該当の個別 PR (既に merge 済みなら修正差分を載せた新しい修正 PR を release 配下に作成) 側に指摘を書き込み、修正ループを回す。この場合レビュー対象は **修正差分** であり新規 PR でレビューできる（元の差分がそもそも cross-review 未実施だったケースは扱いが異なるため Step 8 のフォールバック参照） - release PR には integration 観点の指摘のみ残す ## Step 8: release PR body の最終化と release → default の merge @@ -264,11 +270,27 @@ gh pr edit --title "..." --body "..." 最終化のチェック観点 (Step 3 のレビュアー視点の原則を満たすこと): +- [ ] **全個別 PR が `/ndf:cross-review` で APPROVE 収束済み** (Step 6 の前提。未実施の PR が残っていないこと) - [ ] 「何のために」「何を」が個別 PR や plan ファイルを辿らずに理解できる - [ ] 実装中の方針変更・スコープ増減が body に反映されている - [ ] 個別 PR への参照が本文に残っていない (`
` 内の開発用情報は残してよい) - [ ] 内部用語 (round、rotated 等) が漏れていない +> **cross-review を省略した個別 PR が残っている場合のフォールバック**: Ready for review の前に +> 未 cross-review の個別 PR を特定し、その **状態に応じて** 対応する: +> +> - **個別 PR がまだ open**: その個別 PR に対して `/ndf:cross-review <個別PR番号>` を回して APPROVE +> 収束させてから release へ merge する。**release PR に対して直接は回さない** — ループ内の +> `/ndf:fix` が release PR を修正・Resolve してしまい、「個別 PR 範囲の指摘は個別 PR 側で解決する」 +> 原則 (Step 7) が崩れるため。 +> - **個別 PR が既に release へ merge 済み** (元の差分が release ブランチに取り込まれ、新規 PR には +> 乗らない): release PR に対して `/ndf:cross-review ` を回し、**release PR 全体を +> 改めてレビューする**（当該差分もその中に含まれる。個別差分だけを抽出しての再レビューにはならず、 +> release PR 全体が対象になるぶん手戻りが大きい）。この場合ループ内の `/ndf:fix` は release ブランチを +> 直接修正する **追認的な対応** になる（個別 PR 単位のレビューは既に取り返せないため）。 +> +> いずれも後追い対応で手戻りが増えるので、原則は Step 6 で各個別 PR を cross-review 済みにしておくこと。 + ### Draft 解除と merge release PR が APPROVE されたら: @@ -319,6 +341,7 @@ git checkout release/ | release ブランチを作らず巨大な 1 PR で出す | レビュー困難・revert 困難・並行開発不可 | | 個別 PR の base を default にする | release で統合する意味が失われ、partial merge が default を汚染する | | 個別 PR Draft 作成を実装後に回す | PR 番号が未確定でクロス参照や CI 待機の段取りが組めない | +| 個別 PR を cross-review せず、Claude Code の code-reviewer 等の単発レビューだけで release へ merge する | 片側 AI の一発判定で収束ループを回さないため重大バグを取りこぼし、release PR 側でまとめて検出され手戻りが増える (Step 6) | | release PR で個別 PR 範囲の指摘を解決しようとする | 該当 PR が既に閉じている場合、コミット意図がずれる | | release PR の body を個別 PR リンクの列挙だけにする | レビュアーは release PR 単体で変更を把握できず、個別 PR や plan を辿ることになる。body は self-contained 必須 (Step 3 / Step 8) | | body 最終化せずに Ready for review にする | Draft 作成時の plan ベースの暫定 body のままだと実装の最終形と乖離する | diff --git a/plugins/ndf-kiro/skills/cross-review/SKILL.md b/plugins/ndf-kiro/skills/cross-review/SKILL.md index 709c281..77e1384 100644 --- a/plugins/ndf-kiro/skills/cross-review/SKILL.md +++ b/plugins/ndf-kiro/skills/cross-review/SKILL.md @@ -1,8 +1,8 @@ --- name: cross-review description: "Run iterative Codex and Gemini PR reviews." +when_to_use: "PR を codex + gemini 両方でレビューし、両者 APPROVE まで自動収束させたいときに限定して使う。明示トリガ: 'cross-review', 'クロスレビュー', '両AIレビュー', '収束レビュー', 'codex と gemini でレビュー'。通常の単発 PR レビュー依頼 (第二意見が 1 回欲しい等) は本 skill を選ばず /ndf:review を使う。重い収束ループ (codex+gemini を複数ラウンド起動) のため、単発レビューと責務を明確に分ける。" argument-hint: "[PR番号] [--max-rounds N] [--rotate-after K] [--rotate-mode light|squash] [--only codex|gemini] [--focus TEXT] [--extra-instructions-file PATH]" -disable-model-invocation: true allowed-tools: - Bash - Read @@ -472,4 +472,7 @@ pint / larastan / test / build などは **中断** を原則とする。 - `/ndf:codex` — codex CLI 呼び出し手順 - `/ndf:gemini` — gemini CLI 呼び出し手順 - `/ndf:resolve-pr-comments` — Resolve Conversation の詳細 +- `/ndf:issue-plan-strategy` — multi-PR ワークフローでは **個別 PR ごとに本 cross-review が原則必須**。 + `/ndf:review` 単発や Claude Code の `code-reviewer` は代替にせず、release ブランチへ merge する前に + codex + gemini の APPROVE 収束を確認する (Step 6) - `general-purpose` エージェント — fix 実行用サブエージェント diff --git a/plugins/ndf-kiro/skills/issue-plan-strategy/SKILL.md b/plugins/ndf-kiro/skills/issue-plan-strategy/SKILL.md index 4275a37..fdae1ec 100644 --- a/plugins/ndf-kiro/skills/issue-plan-strategy/SKILL.md +++ b/plugins/ndf-kiro/skills/issue-plan-strategy/SKILL.md @@ -64,7 +64,7 @@ issue 取得 ─┤ │ 既存 plan ─▶│ Step 3: release branch 作成 + Draft release PR │ │ Step 4: 個別 PR ブランチ作成 + 各 Draft PR (release base) │ Step 5: git worktree で並行開発 (依存関係を考慮) │ - │ Step 6: 個別 PR ごとに /ndf:review or /ndf:cross-review + │ Step 6: 個別 PR ごとに /ndf:cross-review (原則必須) │ │ → /ndf:fix → merge into release │ │ Step 7: release ブランチで結合テスト相当のレビュー │ │ Step 8: release PR body 最終化 → Ready & merge │ @@ -125,7 +125,7 @@ git push -u origin release/ ### レビュアー視点の原則 (release PR body の大前提) -個別 PR はセルフレビュー (`/ndf:cross-review` 等) で merge される。**人間のレビュアーが見るのは release PR だけ**であり、個別 PR の存在をレビュアーに意識させてはならない。したがって: +個別 PR は AI による収束レビュー (`/ndf:cross-review` 等) で merge される。**人間のレビュアーが見るのは release PR だけ**であり、個別 PR の存在をレビュアーに意識させてはならない。したがって: - release PR の body は **self-contained 必須**: 「何のために」(背景・解決したい課題) と「何を」(release ブランチ全体としての変更内容) を、**個別 PR を一切参照せずに**理解できる粒度で書く - 個別 PR リンクの列挙を body の本文にしない。開発中の進捗管理に使う場合は `
` 折りたたみ内の補足情報に格下げする @@ -225,16 +225,22 @@ git worktree add ../--ui feature/-ui ## Step 6: 個別 PR のレビュー -**レビューは原則個別 PR 単位**で行う: +**個別 PR は原則 `/ndf:cross-review ` を必須**とする。codex + gemini の両者が +`APPROVE` に収束したことを確認してから Draft を解除し、release ブランチへ merge する。 +個別 PR で重大バグを取りこぼすと、release PR 側の cross-review がまとめて検出する形に +なり、本 skill が禁止する「release PR で個別 PR 範囲の指摘を解決する」状態に陥る。 -| 用途 | コマンド | -|---|---| -| PR 作成前のセルフレビュー | `/ndf:review-branch` | -| GitHub 上の単体レビュー | `/ndf:review ` | -| codex + gemini 両方の収束ループ | `/ndf:cross-review ` | -| 指摘の修正 | `/ndf:fix ` | +| 用途 | コマンド | 位置づけ | +|---|---|---| +| PR 作成前のセルフレビュー | `/ndf:review-branch` | push / PR 化の前段。cross-review の代替にはしない | +| 個別 PR の収束レビュー (原則必須) | `/ndf:cross-review ` | codex + gemini 両方の APPROVE 収束を確認する本線 | +| GitHub 上の例外的な単発確認 | `/ndf:review ` | ごく軽微な差分の単発確認に限定。cross-review の代替にはしない | +| 指摘の修正 | `/ndf:fix ` | cross-review ループ内・後で自動起動される | -個別 PR が APPROVE → Draft 解除 → release ブランチへ merge (squash 推奨)。 +- Claude Code の `code-reviewer` などの単発レビュアーや `/ndf:review` の単発レビューを + **cross-review の代替にしない**。単発レビューは片側 AI の一発判定にとどまり、収束ループを + 回さないため取りこぼしが残る。 +- 個別 PR が cross-review で APPROVE → Draft 解除 → release ブランチへ merge (squash 推奨)。 ## Step 7: release ブランチのレビュー (結合テスト相当のみ) @@ -246,7 +252,7 @@ release ブランチへの merge が一通り進んだ段階で: - 設定値の重複・矛盾 - migration の順序依存 - E2E シナリオ (`/ndf:playwright-scenario-test` の活用) -- ここで個別 PR 範囲のバグが見つかった場合は、**release PR にコメントせず**、該当の個別 PR (既に merge 済みなら新しい修正 PR を release 配下に作成) 側に指摘を書き込み、修正ループを回す +- ここで **新たに** 個別 PR 範囲のバグが見つかった場合は、**release PR にコメントせず**、該当の個別 PR (既に merge 済みなら修正差分を載せた新しい修正 PR を release 配下に作成) 側に指摘を書き込み、修正ループを回す。この場合レビュー対象は **修正差分** であり新規 PR でレビューできる（元の差分がそもそも cross-review 未実施だったケースは扱いが異なるため Step 8 のフォールバック参照） - release PR には integration 観点の指摘のみ残す ## Step 8: release PR body の最終化と release → default の merge @@ -264,11 +270,27 @@ gh pr edit --title "..." --body "..." 最終化のチェック観点 (Step 3 のレビュアー視点の原則を満たすこと): +- [ ] **全個別 PR が `/ndf:cross-review` で APPROVE 収束済み** (Step 6 の前提。未実施の PR が残っていないこと) - [ ] 「何のために」「何を」が個別 PR や plan ファイルを辿らずに理解できる - [ ] 実装中の方針変更・スコープ増減が body に反映されている - [ ] 個別 PR への参照が本文に残っていない (`
` 内の開発用情報は残してよい) - [ ] 内部用語 (round、rotated 等) が漏れていない +> **cross-review を省略した個別 PR が残っている場合のフォールバック**: Ready for review の前に +> 未 cross-review の個別 PR を特定し、その **状態に応じて** 対応する: +> +> - **個別 PR がまだ open**: その個別 PR に対して `/ndf:cross-review <個別PR番号>` を回して APPROVE +> 収束させてから release へ merge する。**release PR に対して直接は回さない** — ループ内の +> `/ndf:fix` が release PR を修正・Resolve してしまい、「個別 PR 範囲の指摘は個別 PR 側で解決する」 +> 原則 (Step 7) が崩れるため。 +> - **個別 PR が既に release へ merge 済み** (元の差分が release ブランチに取り込まれ、新規 PR には +> 乗らない): release PR に対して `/ndf:cross-review ` を回し、**release PR 全体を +> 改めてレビューする**（当該差分もその中に含まれる。個別差分だけを抽出しての再レビューにはならず、 +> release PR 全体が対象になるぶん手戻りが大きい）。この場合ループ内の `/ndf:fix` は release ブランチを +> 直接修正する **追認的な対応** になる（個別 PR 単位のレビューは既に取り返せないため）。 +> +> いずれも後追い対応で手戻りが増えるので、原則は Step 6 で各個別 PR を cross-review 済みにしておくこと。 + ### Draft 解除と merge release PR が APPROVE されたら: @@ -319,6 +341,7 @@ git checkout release/ | release ブランチを作らず巨大な 1 PR で出す | レビュー困難・revert 困難・並行開発不可 | | 個別 PR の base を default にする | release で統合する意味が失われ、partial merge が default を汚染する | | 個別 PR Draft 作成を実装後に回す | PR 番号が未確定でクロス参照や CI 待機の段取りが組めない | +| 個別 PR を cross-review せず、Claude Code の code-reviewer 等の単発レビューだけで release へ merge する | 片側 AI の一発判定で収束ループを回さないため重大バグを取りこぼし、release PR 側でまとめて検出され手戻りが増える (Step 6) | | release PR で個別 PR 範囲の指摘を解決しようとする | 該当 PR が既に閉じている場合、コミット意図がずれる | | release PR の body を個別 PR リンクの列挙だけにする | レビュアーは release PR 単体で変更を把握できず、個別 PR や plan を辿ることになる。body は self-contained 必須 (Step 3 / Step 8) | | body 最終化せずに Ready for review にする | Draft 作成時の plan ベースの暫定 body のままだと実装の最終形と乖離する | diff --git a/plugins/ndf-shared/skills/cross-review/SKILL.md b/plugins/ndf-shared/skills/cross-review/SKILL.md index 5a63867..034dae4 100644 --- a/plugins/ndf-shared/skills/cross-review/SKILL.md +++ b/plugins/ndf-shared/skills/cross-review/SKILL.md @@ -1,8 +1,8 @@ --- name: cross-review description: "Run iterative Codex and Gemini PR reviews." +when_to_use: "PR を codex + gemini 両方でレビューし、両者 APPROVE まで自動収束させたいときに限定して使う。明示トリガ: 'cross-review', 'クロスレビュー', '両AIレビュー', '収束レビュー', 'codex と gemini でレビュー'。通常の単発 PR レビュー依頼 (第二意見が 1 回欲しい等) は本 skill を選ばず /ndf:review を使う。重い収束ループ (codex+gemini を複数ラウンド起動) のため、単発レビューと責務を明確に分ける。" argument-hint: "[PR番号] [--max-rounds N] [--rotate-after K] [--rotate-mode light|squash] [--only codex|gemini] [--focus TEXT] [--extra-instructions-file PATH]" -disable-model-invocation: true allowed-tools: - Bash - Read @@ -472,4 +472,7 @@ pint / larastan / test / build などは **中断** を原則とする。 - `/ndf:codex` — codex CLI 呼び出し手順 - `/ndf:gemini` — gemini CLI 呼び出し手順 - `/ndf:resolve-pr-comments` — Resolve Conversation の詳細 +- `/ndf:issue-plan-strategy` — multi-PR ワークフローでは **個別 PR ごとに本 cross-review が原則必須**。 + `/ndf:review` 単発や Claude Code の `code-reviewer` は代替にせず、release ブランチへ merge する前に + codex + gemini の APPROVE 収束を確認する (Step 6) - `general-purpose` エージェント — fix 実行用サブエージェント diff --git a/plugins/ndf-shared/skills/issue-plan-strategy/SKILL.md b/plugins/ndf-shared/skills/issue-plan-strategy/SKILL.md index 4275a37..fdae1ec 100644 --- a/plugins/ndf-shared/skills/issue-plan-strategy/SKILL.md +++ b/plugins/ndf-shared/skills/issue-plan-strategy/SKILL.md @@ -64,7 +64,7 @@ issue 取得 ─┤ │ 既存 plan ─▶│ Step 3: release branch 作成 + Draft release PR │ │ Step 4: 個別 PR ブランチ作成 + 各 Draft PR (release base) │ Step 5: git worktree で並行開発 (依存関係を考慮) │ - │ Step 6: 個別 PR ごとに /ndf:review or /ndf:cross-review + │ Step 6: 個別 PR ごとに /ndf:cross-review (原則必須) │ │ → /ndf:fix → merge into release │ │ Step 7: release ブランチで結合テスト相当のレビュー │ │ Step 8: release PR body 最終化 → Ready & merge │ @@ -125,7 +125,7 @@ git push -u origin release/ ### レビュアー視点の原則 (release PR body の大前提) -個別 PR はセルフレビュー (`/ndf:cross-review` 等) で merge される。**人間のレビュアーが見るのは release PR だけ**であり、個別 PR の存在をレビュアーに意識させてはならない。したがって: +個別 PR は AI による収束レビュー (`/ndf:cross-review` 等) で merge される。**人間のレビュアーが見るのは release PR だけ**であり、個別 PR の存在をレビュアーに意識させてはならない。したがって: - release PR の body は **self-contained 必須**: 「何のために」(背景・解決したい課題) と「何を」(release ブランチ全体としての変更内容) を、**個別 PR を一切参照せずに**理解できる粒度で書く - 個別 PR リンクの列挙を body の本文にしない。開発中の進捗管理に使う場合は `
` 折りたたみ内の補足情報に格下げする @@ -225,16 +225,22 @@ git worktree add ../--ui feature/-ui ## Step 6: 個別 PR のレビュー -**レビューは原則個別 PR 単位**で行う: +**個別 PR は原則 `/ndf:cross-review ` を必須**とする。codex + gemini の両者が +`APPROVE` に収束したことを確認してから Draft を解除し、release ブランチへ merge する。 +個別 PR で重大バグを取りこぼすと、release PR 側の cross-review がまとめて検出する形に +なり、本 skill が禁止する「release PR で個別 PR 範囲の指摘を解決する」状態に陥る。 -| 用途 | コマンド | -|---|---| -| PR 作成前のセルフレビュー | `/ndf:review-branch` | -| GitHub 上の単体レビュー | `/ndf:review ` | -| codex + gemini 両方の収束ループ | `/ndf:cross-review ` | -| 指摘の修正 | `/ndf:fix ` | +| 用途 | コマンド | 位置づけ | +|---|---|---| +| PR 作成前のセルフレビュー | `/ndf:review-branch` | push / PR 化の前段。cross-review の代替にはしない | +| 個別 PR の収束レビュー (原則必須) | `/ndf:cross-review ` | codex + gemini 両方の APPROVE 収束を確認する本線 | +| GitHub 上の例外的な単発確認 | `/ndf:review ` | ごく軽微な差分の単発確認に限定。cross-review の代替にはしない | +| 指摘の修正 | `/ndf:fix ` | cross-review ループ内・後で自動起動される | -個別 PR が APPROVE → Draft 解除 → release ブランチへ merge (squash 推奨)。 +- Claude Code の `code-reviewer` などの単発レビュアーや `/ndf:review` の単発レビューを + **cross-review の代替にしない**。単発レビューは片側 AI の一発判定にとどまり、収束ループを + 回さないため取りこぼしが残る。 +- 個別 PR が cross-review で APPROVE → Draft 解除 → release ブランチへ merge (squash 推奨)。 ## Step 7: release ブランチのレビュー (結合テスト相当のみ) @@ -246,7 +252,7 @@ release ブランチへの merge が一通り進んだ段階で: - 設定値の重複・矛盾 - migration の順序依存 - E2E シナリオ (`/ndf:playwright-scenario-test` の活用) -- ここで個別 PR 範囲のバグが見つかった場合は、**release PR にコメントせず**、該当の個別 PR (既に merge 済みなら新しい修正 PR を release 配下に作成) 側に指摘を書き込み、修正ループを回す +- ここで **新たに** 個別 PR 範囲のバグが見つかった場合は、**release PR にコメントせず**、該当の個別 PR (既に merge 済みなら修正差分を載せた新しい修正 PR を release 配下に作成) 側に指摘を書き込み、修正ループを回す。この場合レビュー対象は **修正差分** であり新規 PR でレビューできる（元の差分がそもそも cross-review 未実施だったケースは扱いが異なるため Step 8 のフォールバック参照） - release PR には integration 観点の指摘のみ残す ## Step 8: release PR body の最終化と release → default の merge @@ -264,11 +270,27 @@ gh pr edit --title "..." --body "..." 最終化のチェック観点 (Step 3 のレビュアー視点の原則を満たすこと): +- [ ] **全個別 PR が `/ndf:cross-review` で APPROVE 収束済み** (Step 6 の前提。未実施の PR が残っていないこと) - [ ] 「何のために」「何を」が個別 PR や plan ファイルを辿らずに理解できる - [ ] 実装中の方針変更・スコープ増減が body に反映されている - [ ] 個別 PR への参照が本文に残っていない (`
` 内の開発用情報は残してよい) - [ ] 内部用語 (round、rotated 等) が漏れていない +> **cross-review を省略した個別 PR が残っている場合のフォールバック**: Ready for review の前に +> 未 cross-review の個別 PR を特定し、その **状態に応じて** 対応する: +> +> - **個別 PR がまだ open**: その個別 PR に対して `/ndf:cross-review <個別PR番号>` を回して APPROVE +> 収束させてから release へ merge する。**release PR に対して直接は回さない** — ループ内の +> `/ndf:fix` が release PR を修正・Resolve してしまい、「個別 PR 範囲の指摘は個別 PR 側で解決する」 +> 原則 (Step 7) が崩れるため。 +> - **個別 PR が既に release へ merge 済み** (元の差分が release ブランチに取り込まれ、新規 PR には +> 乗らない): release PR に対して `/ndf:cross-review ` を回し、**release PR 全体を +> 改めてレビューする**（当該差分もその中に含まれる。個別差分だけを抽出しての再レビューにはならず、 +> release PR 全体が対象になるぶん手戻りが大きい）。この場合ループ内の `/ndf:fix` は release ブランチを +> 直接修正する **追認的な対応** になる（個別 PR 単位のレビューは既に取り返せないため）。 +> +> いずれも後追い対応で手戻りが増えるので、原則は Step 6 で各個別 PR を cross-review 済みにしておくこと。 + ### Draft 解除と merge release PR が APPROVE されたら: @@ -319,6 +341,7 @@ git checkout release/ | release ブランチを作らず巨大な 1 PR で出す | レビュー困難・revert 困難・並行開発不可 | | 個別 PR の base を default にする | release で統合する意味が失われ、partial merge が default を汚染する | | 個別 PR Draft 作成を実装後に回す | PR 番号が未確定でクロス参照や CI 待機の段取りが組めない | +| 個別 PR を cross-review せず、Claude Code の code-reviewer 等の単発レビューだけで release へ merge する | 片側 AI の一発判定で収束ループを回さないため重大バグを取りこぼし、release PR 側でまとめて検出され手戻りが増える (Step 6) | | release PR で個別 PR 範囲の指摘を解決しようとする | 該当 PR が既に閉じている場合、コミット意図がずれる | | release PR の body を個別 PR リンクの列挙だけにする | レビュアーは release PR 単体で変更を把握できず、個別 PR や plan を辿ることになる。body は self-contained 必須 (Step 3 / Step 8) | | body 最終化せずに Ready for review にする | Draft 作成時の plan ベースの暫定 body のままだと実装の最終形と乖離する |