diff --git a/issues/REPORT02.md b/issues/REPORT02.md new file mode 100644 index 00000000..b6c444ec --- /dev/null +++ b/issues/REPORT02.md @@ -0,0 +1,165 @@ +# REPORT06: ndf:cross-review の PR ローテーション仕様 修正依頼 + +## 何のために + +`ndf:cross-review` の `--rotate-after` で発火する PR ローテーション (`scripts/rotate-pr.sh`) の挙動が、利用者の意図と乖離している。**「コメント履歴が長くなった PR を AI Agent が読みやすいようにリセットしたい」だけ** のケースで、不要な squash と新ブランチ生成が走り、release branch 戦略 / TODO 参照 / レビューの粒度を壊してしまう。実運用で問題が出た (PLAN03 PR1) ため、ndf 側に修正を依頼する。 + +## 何を + +`scripts/rotate-pr.sh` を **同ブランチで PR を作り直す軽量モード (default)** と、**squash + 新ブランチを作る重量モード (opt-in)** の 2 モードに分割する。または既存の `--rotate-after` を軽量化し、重量モードは別フラグに退避する。 + +## 発生事象 + +### 現状の `rotate-pr.sh` の挙動 + +```bash +# 既存ブランチ feature/PLAN03-infra-gamma-docs-prep を起点に +git checkout -b "${BRANCH}-r$(date +%H%M%S)" # -rHHMMSS suffix を付与 +git reset --soft "origin/$BASE" # squash +git commit -m " (cross-review rotation: PR #<OLD> を squash 統合)" +git push -u origin "$NEW_BRANCH" + +# 旧 PR を close +gh pr close "$OLD_PR" + +# 新 PR を作成 +gh pr create --title "$TITLE (rotated)" --body "...automated body..." +``` + +### 問題点 + +| # | 観点 | 内容 | +|---|---|---| +| 1 | **不要な squash** | コミット単位レビューがやり辛くなる。元 PR の修正履歴 (round 1〜6 で何を直したか) が 1 commit に潰れる | +| 2 | **時刻 suffix のブランチ名** | `feature/...-r014230` のような可読性低い名前。**release branch 戦略 / TODO.md / 他 PR の参照を破壊** する | +| 3 | **PR title 末尾の `(rotated)`** | サイクル運用の内部用語が PR title に漏れる | +| 4 | **PR body の上書き** | 元 PR で丁寧に書いた「何のために / 何を / Test plan」が automated body に置き換わる | +| 5 | **モード固定** | 利用者が「コメント履歴だけリセットしたい」場合の選択肢がない | + +### 実運用での再現 + +PLAN03 PR1 (`/ndf:cross-review 217 --max-rounds 6 --rotate-after 5`) で、round 5 終了後 `should-rotate` が true を返した。 + +- release PR (#216) → PR1 (#217) という release branch 戦略 を採用していた +- TODO.md / `08-rollout.md` で PR 番号と branch 名を多数の箇所から参照 +- 単純に「履歴が読みづらいから PR を作り直したい」だけだった + +このため `rotate-pr.sh` を実行せず、手動で以下を行うことで問題回避した: + +```bash +# 同ブランチで close → 新規 PR 作成 (title / body も維持) +gh pr comment 217 --body "ℹ️ レビューコメント履歴が長くなったため..." +gh pr close 217 +gh pr create --base "release/PLAN03-docs-import" \ + --head "feature/PLAN03-infra-gamma-docs-prep" \ + --title "$ORIGINAL_TITLE" --body "$ORIGINAL_BODY" +gh pr ready 221 --undo # Draft 維持 +``` + +## 修正提案 + +### Option A: モード分離 (推奨) + +`rotate-pr.sh` に `--mode` を追加し、default を軽量化: + +``` +--mode light (default): 同ブランチで close → 新規 PR 作成。 + title / body は **現状の差分・実装状態を反映して書き直す**。 + ただし review-fix サイクルが回ったこと自体や round 番号などの + 内部用語は出さない (PR を読む人は cross-review を意識しない)。 +--mode squash : 既存挙動 (squash + 新ブランチ + (rotated) suffix) +``` + +`/ndf:cross-review` 側にも `--rotate-mode light|squash` を生やす。 + +#### light モードの title / body 生成方針 + +新 PR の title / body は以下の素材から「PR の最終形」を表現する文書として書き直す: + +- `git log $BASE..HEAD` (最終的に含まれる commit メッセージ) +- `git diff $BASE..HEAD --stat` + 主要変更ファイルの内容 +- 元 PR の **背景セクション** (「何のために」「Test plan」など、設計意図の部分) は再利用してよい + +書いて **よい** こと: +- 何のために (背景・動機) — 元 PR から継承可 +- 何を (変更内容) — 現在のブランチの実態を反映 +- Test plan — 元 PR から継承可 + +書いて **はいけない** こと: +- 「round N で〜」「cross-review で〜」「レビュー指摘で〜」など内部運用の文言 +- 「(rotated)」のような automated suffix +- 「fix された問題」の列挙 (PR の読者には不要なノイズ) + +### Option B: light モードのみに変更 (Breaking change) + +squash モードは利用ケースが稀なので default を light に切り替え、squash は廃止 or 別 skill に切り出す。 + +### Option C: 設定で抑止する逃げ道だけ用意 + +`--no-rotate` フラグで rotation 自体をスキップできるようにする (現状 should-rotate が true で必ず実行されるため、ユーザが介入できない)。最低限これだけでも欲しい。 + +## 修正対象ファイル + +- `skills/cross-review/scripts/rotate-pr.sh` +- `skills/cross-review/SKILL.md` (rotation セクションの説明更新) +- `skills/cross-review/docs/02-fix-and-rotation.md` (Step 6 の説明更新) +- `skills/cross-review/scripts/state.py` (`should-rotate` / `set-current-pr` の挙動が light モードでも整合するかの確認) + +## 期待する default 挙動 (light モード) + +``` +旧 PR #217 を close → 新 PR #221 を作成 + - branch: feature/PLAN03-infra-gamma-docs-prep (変更なし) + - title: 現状の差分を反映して書き直す + (例: scope が広がっていれば title もそれに合わせる) + - body: 現状の差分・最終実装に合わせて書き直す + - 何のために / 何を / Test plan を再生成 + - 元 PR の背景セクションは再利用してよい + - review-fix が発生したこと / その内容は書かない + - draft 状態: 元 PR と同じ (元が Draft なら新 PR も Draft) + - 旧 PR への close コメント: 「コメント履歴整理のため新 PR に巻き直し」のような短い説明 +``` + +これで以下が成立する: + +- release branch 戦略 / TODO.md / 他 PR の参照が壊れない +- PR の commit 履歴が squash されず、レビュー粒度を維持 +- automated な (rotated) suffix が title に付かない +- 最終的な PR の title / body が **現状の実装** を正しく説明している (元 PR から実装が変わっている場合に古い説明が残らない) +- PR を読む人 (将来のレビュアー / 後続 PR を作る人) は cross-review の存在を意識しなくて済む + +## 参考: 実運用で書いたフォールバック手順 + +```bash +# 1. 旧 PR の情報を保存 (base / head / 元 title・body は再生成の素材として保持) +gh pr view "$OLD" --json title,body,headRefName,baseRefName,isDraft > /tmp/pr-info.json + +# 2. close 通知 + close +gh pr comment "$OLD" --body "ℹ️ レビューコメント履歴が長くなったため、AI Agent の可読性向上のために本 PR を一度 close し、同じブランチ \`$HEAD\` で新 PR を作り直します。ブランチの内容・base は変えません。" +gh pr close "$OLD" + +# 3. title / body は git log $BASE..HEAD と git diff から再生成して使う +# (cross-review の round や fix 内容は書かない) +gh pr create \ + --base "$(jq -r .baseRefName /tmp/pr-info.json)" \ + --head "$(jq -r .headRefName /tmp/pr-info.json)" \ + --title "<最終実装に合わせて書き直す>" \ + --body "<何のために / 何を / Test plan を再生成>" + +# 4. 元が Draft だった場合 Draft 化 +gh pr ready "$NEW" --undo +``` + +**注**: 今回 PLAN03 PR1 の手動運用では時間都合で元の title/body をそのままコピーしたが、 +本来は **現状の最終実装** を反映した title/body に書き直すのが望ましい。 +ndf 側で light モードを実装する際はこの再生成を必須の挙動とする。 + +## 優先度 + +**Medium**。現状 squash モードでも `--rotate-after` を非常に大きな値にして発火させない運用は可能だが、defaults が「履歴リセット」用途と乖離しているのは UX 問題。 + +## 関連 + +- PLAN03 PR1 (`#217` → `#221`) の cross-review 実運用 — 旧 PR は close、新 PR で継続 +- `issues/PLAN03/TODO.md` (PR1 行 / PR1 実装セクション / 進捗チェックリスト) +- ndf プラグイン: `skills/cross-review/` diff --git a/issues/REPORT02_rotate-pr-light-mode.md b/issues/REPORT02_rotate-pr-light-mode.md new file mode 100644 index 00000000..46ddb576 --- /dev/null +++ b/issues/REPORT02_rotate-pr-light-mode.md @@ -0,0 +1,137 @@ +# REPORT02 実装プラン: cross-review PR ローテーション light モード対応 + +**Issue**: [issues/REPORT02.md](REPORT02.md) +**作成日**: 2026-05-21 +**ステータス**: Draft + +## 1. 何のために + +`ndf:cross-review` の `--rotate-after` で発火する `scripts/rotate-pr.sh` の挙動が、利用者の期待 (「コメント履歴が長くなった PR を AI Agent が読みやすいようにリセットしたい」だけ) と乖離している。実運用 (PLAN03 PR1) で以下の副作用が顕在化したため、default を **同ブランチで PR を作り直す light モード** に切り替える。 + +問題点 (issue より引用): + +| # | 観点 | 内容 | +|---|---|---| +| 1 | 不要な squash | コミット単位レビューがやり辛くなる | +| 2 | 時刻 suffix のブランチ名 | `feature/...-r014230` で release branch 戦略 / TODO 参照を破壊 | +| 3 | PR title `(rotated)` suffix | 内部用語が PR title に漏れる | +| 4 | PR body の自動上書き | 元 PR の「何のために / 何を / Test plan」が消失 | +| 5 | モード固定 | 利用者が選べない | + +## 2. 何を + +`scripts/rotate-pr.sh` を `--mode light` (default) / `--mode squash` (opt-in) の 2 モードに分割する。Option A の方針に従い、`--rotate-after` 利用者の **新 default = light**。 + +### 設計判断 (ユーザ確認済) + +| 項目 | 採用 | 理由 | +|---|---|---| +| Default mode | **light** | 新規利用者の期待値に合致。Breaking だが影響範囲は `--rotate-after` 利用者のみで小さい。squash を残すことで救済可能 | +| `--no-rotate` フラグ (Option C) | **実装しない** | light が default なら rotation の害が最小化されるため不要 | +| light モードの title/body 再生成 | **Claude (general-purpose サブエージェント) で生成** | shell template では「現状の差分・実装状態を反映」が困難。codex/gemini は cross-review 内で既に占有されているため Claude (general-purpose) が最適 | +| 新 PR の Draft 状態 | **元 PR の isDraft をコピー** | issue line 119 の期待挙動と一致 | + +## 3. light モードの挙動仕様 + +``` +旧 PR #N を close → 新 PR #M を作成 + - branch: 元と同じ (新ブランチ作らない) + - base: 元と同じ + - title: Claude が現状の git log/diff から書き直す + - body: Claude が「何のために / 何を / Test plan」を再生成 + ・元 PR の背景セクションは再利用してよい + ・round N / cross-review / 「rotated」等の内部用語は禁止 + - draft 状態: 元 PR の isDraft をコピー (元が Draft なら新 PR も Draft) + - 旧 PR への close コメント: 「コメント履歴整理のため新 PR に巻き直し」 +``` + +squash モードは既存挙動を完全維持 (`(rotated)` suffix / 新ブランチ / squash commit / 自動 body)。 + +## 4. 実装方針 + +### 責務分割 + +rotate-pr.sh だけで Claude を呼べないため、Step 6 (rotation) を **3 段階に分割**: + +``` +Step 6a: rotate-pr.sh prepare <STATE_PR> + → 元 PR の title/body/isDraft + git log $BASE..HEAD + git diff --stat を + $TMP_DIR/rotate-pr<STATE_PR>-prepare.json に dump + +Step 6b: メインセッションが Agent(subagent_type="general-purpose") を起動して + title/body を生成 → $TMP_DIR/rotate-pr<STATE_PR>-newtext.json に書き出す + (squash モードでは Step 6b をスキップ) + +Step 6c: rotate-pr.sh execute <STATE_PR> --mode light|squash + → mode に応じて旧 PR close + 新 PR 作成。 + light は newtext.json を読んで title/body に流す。 + squash は既存ロジック。 + → stdout に NEW_PR / NEW_PR_URL / NEW_BRANCH を吐く (既存契約維持) +``` + +互換: 既存呼び出し `rotate-pr.sh <STATE_PR>` (引数 1 つ) は **squash モード相当のショートカット** として残し、deprecate warning を stderr に出す (cross-review SKILL.md からの呼び出しは新形式に書き換える)。 + +### Step 6b のサブエージェントプロンプト方針 + +書いて **よい** こと (issue より): +- 何のために (背景・動機) — 元 PR から継承可 +- 何を (変更内容) — 現在のブランチの実態を反映 +- Test plan — 元 PR から継承可 + +書いて **はいけない** こと: +- 「round N で〜」「cross-review で〜」「レビュー指摘で〜」等の内部運用文言 +- 「(rotated)」のような automated suffix +- 「fix された問題」の列挙 + +## 5. 修正対象ファイル + +| ファイル | 変更内容 | +|---|---| +| `plugins/ndf/skills/cross-review/scripts/rotate-pr.sh` | `prepare` / `execute --mode light\|squash` サブコマンド化。light モードロジック新規追加 | +| `plugins/ndf/skills/cross-review/SKILL.md` | 引数表に `--rotate-mode light\|squash` 追加 (default=light)。Step 6 の bash 骨組みを 3 段階呼び出しに更新。アンチパターン節も併せて更新 | +| `plugins/ndf/skills/cross-review/docs/02-fix-and-rotation.md` | Step 6 の手順説明を更新。light/squash の違いと Step 6b の Agent 起動例を追加 | +| `plugins/ndf/skills/cross-review/scripts/state.py` | `should-rotate` / `set-current-pr` は light でもそのまま機能するため、コード変更は不要。ドキュメント (docstring) のみ light モードを明記 | + +## 6. テスト計画 + +`cross-review` は GitHub PR を実際に作成するため自動テストは難しい。**手動検証** を中心に置く: + +- [ ] **dry-run スクリプト**: `rotate-pr.sh prepare` 単体で実行し、`prepare.json` が想定の構造になることを確認 (PR は触らない) +- [ ] **シェルチェック**: `shellcheck rotate-pr.sh` でエラー 0 +- [ ] **light モード E2E (テスト用 PR)**: 検証用ブランチで Draft PR を作り、`rotate-pr.sh execute --mode light` を実行。以下を確認: + - 旧 PR が close され「コメント履歴整理のため」コメントが付く + - 新 PR が **同じブランチ・同じ base** で作成される + - 新 PR の title / body に内部用語 (round / rotated / cross-review) が含まれない + - 元が Draft なら新 PR も Draft + - stdout に `NEW_PR=` / `NEW_PR_URL=` / `NEW_BRANCH=` が KEY=VALUE 形式で出力される +- [ ] **squash モード回帰**: `rotate-pr.sh execute --mode squash` で既存挙動が壊れていないことを確認 (新ブランチに `-rHHMMSS` suffix / `(rotated)` suffix / squash commit) +- [ ] **後方互換**: `rotate-pr.sh <STATE_PR>` (旧形式 1 引数) が squash モード相当で動作し、deprecation warning が出る +- [ ] **state.py 整合**: `should-rotate` → `rotate-pr.sh prepare` → Agent → `rotate-pr.sh execute --mode light` → `state.py set-current-pr` の一連で `state.json.current_pr` が正しく更新される + +## 7. PR 分割計画 + +**単一 PR で十分**。理由: + +- 変更ファイルが 4 個と少なく、いずれも `skills/cross-review/` 配下に閉じている +- `rotate-pr.sh` の挙動変更 → SKILL.md / docs の追従は不可分。分割すると中間状態でドキュメントと実装が乖離する +- 差分は ~300 行程度に収まる見込み + +→ `/ndf:implementation-plan` + `/ndf:pr` の通常フロー。release branch 戦略は採用しない。 + +ブランチ: `feature/REPORT02-rotate-pr-light-mode` +base: `main` + +## 8. アンチパターン (実装時に避けること) + +- ❌ `rotate-pr.sh` 内で `claude` CLI を直接呼ぶ — 環境依存・コスト管理外。メインセッションの Agent tool で行う +- ❌ light モードで `gh pr edit` で旧 PR の title/body を流用するだけにする — issue line 153-155 の通り「最終実装を反映して書き直す」が必須 +- ❌ Step 6a/6b/6c を 1 つの shell コマンドに無理に押し込む — Agent 呼び出しは bash から不可 +- ❌ squash モードを廃止する — backward compat のために残す (Option A 採用、Option B は不採用) +- ❌ 内部用語 (round / rotated / cross-review) を新 PR の title/body に漏らす — Agent プロンプトで明示的に禁止 + +## 9. 関連 + +- 元 issue: [issues/REPORT02.md](REPORT02.md) +- 対象 skill: `plugins/ndf/skills/cross-review/` +- 実運用での再現事例: PLAN03 PR1 (`#217` → `#221`) +- 関連 skill: `/ndf:cross-review`, `/ndf:review`, `/ndf:fix`, `/ndf:implementation-plan`, `/ndf:pr` diff --git a/plugins/ndf/skills/cross-review/SKILL.md b/plugins/ndf/skills/cross-review/SKILL.md index 1c2141fa..009e3dd6 100644 --- a/plugins/ndf/skills/cross-review/SKILL.md +++ b/plugins/ndf/skills/cross-review/SKILL.md @@ -1,7 +1,7 @@ --- name: cross-review description: "PR を codex / gemini 両方にレビューさせ、両方 APPROVE まで /ndf:review → /ndf:fix を自動ループ。サブエージェント分離・PR ローテーション・nit 集約でメイン context 消費を最小化" -argument-hint: "[PR番号] [--max-rounds N] [--rotate-after K] [--only codex|gemini]" +argument-hint: "[PR番号] [--max-rounds N] [--rotate-after K] [--rotate-mode light|squash] [--only codex|gemini]" disable-model-invocation: true allowed-tools: - Bash @@ -40,7 +40,7 @@ state.json の読み書きや AI launcher 起動・完了待ちは全て委譲 | 修正 | **必ずサブエージェント (`general-purpose`) で実行**。メイン context に diff は載せない | | ユーザ問い合わせ | 自動判断を最大化(`critical`/`major`/`minor` は自動修正、`nit` は最後にまとめて 1 回だけ問い合わせ) | | 状態の永続化 | `/tmp/cross-review-pr<番号>-state.json` に集約。中断・再開可能 | -| 長尺PR対策 | **`--rotate-after` ラウンドで PR をローテーション**(squash + 新ブランチ + 新 PR) | +| 長尺PR対策 | **`--rotate-after` ラウンドで PR をローテーション**(default=light: 同ブランチで PR 巻き直し / squash: 新ブランチ + squash 統合) | | 振動検知 | 同じ指摘が 2 round で 50%以上重複したら中断 | ## 引数 @@ -50,6 +50,7 @@ state.json の読み書きや AI launcher 起動・完了待ちは全て委譲 | `[PR番号]` | 対象 PR(省略時は直前 PR / 現在ブランチ) | — | | `--max-rounds N` | 全体最大ラウンド数(PR ローテーションを含む通算) | `6` | | `--rotate-after K` | この round 数で未収束なら PR ローテーション | `5` | +| `--rotate-mode light\|squash` | ローテーション方式。`light`: 同ブランチで旧 PR を close → 新 PR (title/body は現状の差分・実装から再生成)。`squash`: 既存挙動 (squash 統合 + 新ブランチ + `(rotated)` suffix) | `light` | | `--only codex` / `--only gemini` | 片方だけで回す(デバッグ用) | 両方 | 例: @@ -57,9 +58,15 @@ state.json の読み書きや AI launcher 起動・完了待ちは全て委譲 ``` /ndf:cross-review 123 /ndf:cross-review 123 --max-rounds 4 --rotate-after 2 +/ndf:cross-review 123 --rotate-mode squash # 既存挙動が欲しい場合のみ /ndf:cross-review 123 --only codex ``` +### `--rotate-mode` の選び方 + +- **`light` (default)**: PR を読む人 (将来のレビュアー / 後続 PR を作る人) が cross-review の存在を意識せずに済む。release branch 戦略・TODO 参照・コミット単位レビューを破壊しない。**通常はこちらを使う** +- **`squash`**: 巨大 PR を 1 commit に潰したい / `(rotated)` suffix で rotation 履歴を PR title に残したい場合のみ。release branch 戦略を使う運用とは併用しない + ## 前提 - `/ndf:review` が **AI 直接投稿**(外部 AI 自身が `gh api` で投稿)に対応 @@ -126,7 +133,7 @@ flowchart TD Check -->|振動検知 50% 重複| Osc([final = oscillation]):::stop Check -->|CI failure code-related| Err([final = error]):::stop Check -->|"CI failure meta-only (Assignees 等)"| Round - Check -->|round_in_pr >= rotate-after| Rotate["PR rotation<br/>squash + 新ブランチ + 新 PR"] + Check -->|round_in_pr >= rotate-after| Rotate["PR rotation<br/>light (default): 同ブランチで巻き直し<br/>squash (opt-in): 新ブランチ + squash 統合<br/>light は Agent が title/body 再生成"] Check -->|それ以外| Round Rotate --> Round @@ -152,6 +159,10 @@ SCRIPTS="$CLAUDE_PLUGIN_ROOT/skills/cross-review/scripts" # STATE_PR を渡す。「現在レビュー中の PR」は state.json の current_pr を内部参照する。 STATE_PR=$INITIAL_PR +# ROTATE_MODE は引数 --rotate-mode (default=light) から取得。light なら Step 6b で +# Agent (general-purpose) を起動して新 PR の title/body を生成する。 +ROTATE_MODE=${ROTATE_MODE:-light} + # Step 0: state 初期化 / 再開 eval "$("$SCRIPTS/state.py" init "$STATE_PR" \ --max-rounds "$MAX_ROUNDS" --rotate-after "$ROTATE_AFTER" \ @@ -187,7 +198,36 @@ while :; do # Step 6: PR ローテーション判定 (0=rotate/2=keep)。state.json の current_pr を内部更新。 if "$SCRIPTS/state.py" should-rotate "$STATE_PR"; then - eval "$("$SCRIPTS/rotate-pr.sh" "$STATE_PR")" # NEW_PR を eval で取り込む + # Step 6a: 旧 PR の素材 (title/body/isDraft + git log/diff stat) を dump + eval "$("$SCRIPTS/rotate-pr.sh" prepare "$STATE_PR")" + + # Step 6b: light モードのみ。**メインセッション側で Agent(subagent_type="general-purpose") を起動して** + # prepare.json を読ませ、現状の差分・実装を反映した title/body を + # $TMP_DIR/rotate-pr<STATE_PR>-newtext.json に書き出させる。 + # 詳細プロンプトは docs/02-fix-and-rotation.md Step 6b 参照。 + # squash モードでは Step 6b 不要。 + # + # ⚠ Bash 単体では Agent ツールを呼べない。下記のフローは「メイン会話セッション側で」 + # 実行される前提なので、bash の while ループそのものは pseudo-code として読み、 + # 実際には以下の 3 段階を **メインが順に駆動する** こと: + # (1) bash 側で `rotate-pr.sh prepare $STATE_PR` を実行 (これは普通の bash) + # (2) メインが Agent(...) を起動して newtext.json を書かせる (bash の外) + # (3) bash 側で `rotate-pr.sh execute $STATE_PR --mode light` を実行 + # 下記の if exit 10 は **誤ってメイン介在なしで Step 6c に進むことを防ぐガード** であり、 + # exit 10 を観測したらメインは Step 6b の Agent を起動し、newtext.json が + # 生成されてから **同じ STATE_PR で Step 6c (execute) を直接呼び直す**。 + # state.json は完全に再開可能 (prepare.json はそのまま再利用される)。 + NEWTEXT_JSON="$TMP_DIR/rotate-pr$STATE_PR-newtext.json" + if [ "$ROTATE_MODE" = "light" ] && [ ! -s "$NEWTEXT_JSON" ]; then + echo "⏸ light モード: メインセッションで Agent(general-purpose) を起動し" >&2 + echo " $NEWTEXT_JSON を生成してから rotate-pr.sh execute $STATE_PR --mode light を実行してください" >&2 + echo " (docs/02-fix-and-rotation.md Step 6b 参照 / 再開プロトコルは下記 '## 再開プロトコル')" >&2 + exit 10 + fi + + # Step 6c: 実行。NEW_PR / NEW_PR_URL / NEW_BRANCH を eval で取り込む。 + eval "$("$SCRIPTS/rotate-pr.sh" execute "$STATE_PR" --mode "$ROTATE_MODE")" + "$SCRIPTS/state.py" set-current-pr "$STATE_PR" "$NEW_PR" # NOTE: STATE_PR は変えない。次ループの scripts も $STATE_PR を渡す。 fi @@ -202,6 +242,30 @@ done - Step 0〜4 — [docs/01-state-and-review.md](docs/01-state-and-review.md) - Step 5〜8 — [docs/02-fix-and-rotation.md](docs/02-fix-and-rotation.md) +## light モード rotation の再開プロトコル (exit 10 を観測した時) + +bash ループは Agent tool を呼べないため、light モードでは Step 6b の介入が必須。 +メインセッションはループ全体を 1 回の bash で完結させず、以下のように駆動する: + +1. **通常のループ実行** — Step 0〜6a まで bash で進めると、newtext.json が未生成の + ため `exit 10` で停止する。state.json には prepare.json までの状態が + 永続化されているので **そのまま再開可能**。 +2. **Step 6b (Agent 起動)** — メインが Agent(subagent_type=`general-purpose`) を + 起動し、prepare.json を読ませて `$TMP_DIR/rotate-pr$STATE_PR-newtext.json` を + 書き出させる (プロンプトテンプレートは docs/02-fix-and-rotation.md Step 6b)。 +3. **Step 6c (execute) を直接呼ぶ** — メインが bash で以下を実行: + + ```bash + eval "$("$SCRIPTS/rotate-pr.sh" execute "$STATE_PR" --mode light)" + "$SCRIPTS/state.py" set-current-pr "$STATE_PR" "$NEW_PR" + ``` + +4. **ループ再開** — Step 7 (次ラウンド) からループ全体を再開する。`STATE_PR` は + 不変なので、`start-round` 以降は通常通り進む。 + +> ⚠ exit 10 はエラーではなく **メイン介入待ちの一時停止シグナル**。final ステータスには +> 反映しない (中断扱いではない)。次回 round カウントにも影響しない。 + ## レビュー出力の制約 **目的**: PR 上に Resolve 義務を伴うインラインコメントを増やさない。 @@ -274,6 +338,10 @@ pint / larastan / test / build などは **中断** を原則とする。 - ❌ **nit を都度ユーザに問う** — 必ずバッチ集約して最後に 1 回 - ❌ **`max-rounds` なしで回す** — 無限ループの温床 - ❌ **PR ローテーションを忘れる** — 100+ コメントの巨大 PR になる +- ❌ **light モードで Agent (general-purpose) 呼び出しを省略する** — newtext.json が無いと `rotate-pr.sh execute --mode light` はエラーで止まる。prepare → Agent → execute の 3 段は不可分 +- ❌ **light モードで新 PR の title/body に内部用語を漏らす** — 「round N」「rotated」「cross-review」「レビュー指摘で〜」等は禁止 (Agent プロンプトで明示禁止) +- ❌ **newtext.json に旧 PR の title/body をそのままコピーする** — 「現状の差分・実装を反映」が必須。古い説明が残ると後続 PR / 将来のレビュアーが混乱 +- ❌ **`rotate-pr.sh` 内から `claude` CLI を呼んで title/body を生成する** — 環境依存・コスト管理外。Agent tool でメイン側から呼ぶ - ❌ **CI 失敗を一律で中断** — コード関連/メタチェックを分類(上記参照) - ❌ **自分の PR に `REQUEST_CHANGES` で投稿** — 必ず 422。事前判定 + COMMENT ダウングレード - ❌ **`gemini --yolo` だけで起動** — trusted directory で YOLO 無効化。`--skip-trust` 併用 diff --git a/plugins/ndf/skills/cross-review/docs/02-fix-and-rotation.md b/plugins/ndf/skills/cross-review/docs/02-fix-and-rotation.md index 34dcd5e0..f7585031 100644 --- a/plugins/ndf/skills/cross-review/docs/02-fix-and-rotation.md +++ b/plugins/ndf/skills/cross-review/docs/02-fix-and-rotation.md @@ -7,7 +7,9 @@ | (Agent) | Step 5 — 修正サブエージェント起動(メインからの責務) | | `scripts/state.py merge-fix` | Step 5 後段 — fix 戻り値マージ + CI 分類 | | `scripts/state.py should-rotate` | Step 6 — rotate 要否判定 | -| `scripts/rotate-pr.sh` | Step 6 — PR rotation 実行 | +| `scripts/rotate-pr.sh prepare` | Step 6a — 旧 PR の素材を `rotate-pr<STATE_PR>-prepare.json` に dump | +| (Agent) | Step 6b — light モードのみ。新 PR の title/body を再生成して `rotate-pr<STATE_PR>-newtext.json` に書き出し | +| `scripts/rotate-pr.sh execute` | Step 6c — 旧 PR close + 新 PR 作成 (light は同ブランチ / squash は新ブランチ) | | `scripts/state.py set-current-pr` | Step 6 — rotation 後の state 更新 | | `scripts/state.py report` | Step 8 — deferred nit + ラウンドサマリ | @@ -164,11 +166,24 @@ fi **例**: `check_pr_requirements`(Assignees 未設定)はループ継続、 `laravel/pint` や `phpstan` の失敗は即中断してユーザ判断。 -## Step 6: PR ローテーション判定 +## Step 6: PR ローテーション (prepare → Agent → execute の 3 段) + +`rotate-pr.sh` は **light モード (default) と squash モード (opt-in)** を持つ。 +両者ともメインからは `prepare → (light のみ Agent) → execute` の 3 段で呼ぶ。 ```bash if "$SCRIPTS/state.py" should-rotate "$STATE_PR"; then - eval "$("$SCRIPTS/rotate-pr.sh" "$STATE_PR")" # NEW_PR / NEW_PR_URL / NEW_BRANCH を取り込む + # Step 6a: 旧 PR の素材 dump (title / body / isDraft / git log / git diff --stat) + eval "$("$SCRIPTS/rotate-pr.sh" prepare "$STATE_PR")" + + # Step 6b: light モードのみ。Agent(subagent_type="general-purpose") で + # 現状の差分・実装を反映した新 title/body を生成し、 + # $TMP_DIR/rotate-pr<STATE_PR>-newtext.json に書き出させる。 + # (squash モードでは Step 6b は不要) + + # Step 6c: 実行 (NEW_PR / NEW_PR_URL / NEW_BRANCH を取り込む) + eval "$("$SCRIPTS/rotate-pr.sh" execute "$STATE_PR" --mode "$ROTATE_MODE")" + "$SCRIPTS/state.py" set-current-pr "$STATE_PR" "$NEW_PR" # NOTE: STATE_PR は **絶対に変えない**。次ループの scripts も $STATE_PR で呼ぶ。 fi @@ -177,15 +192,103 @@ fi `should-rotate` は `round_in_pr >= rotate_after && total_rounds < max_rounds` で exit 0 を返す(rotate 要)。それ以外は exit 2(keep)。 -`rotate-pr.sh` が内部で行う処理: +### Step 6a: `rotate-pr.sh prepare <STATE_PR>` + +state.json から旧 PR / worktree を解決し、以下の素材を 1 つの JSON に dump する: + +```json +{ + "state_pr": 217, + "old_pr": 217, + "old_pr_url": "https://github.com/.../pull/217", + "worktree_path": "/work/worktrees/pr217", + "head_branch": "feature/...", + "base_branch": "release/...", + "is_draft": true, + "round_in_pr": 5, + "old_title": "...", + "old_body": "...", + "git_log": "abc1234 メッセージ\n...", + "git_diff_stat": " path/to/file | 12 +-\n ..." +} +``` + +ファイル: `$TMP_DIR/rotate-pr<STATE_PR>-prepare.json`。 +stdout にも `OLD_PR=` / `HEAD_BRANCH=` / `BASE_BRANCH=` / `IS_DRAFT=` / `PREPARE_JSON=` を出すので +`eval` で取り込める。 + +### Step 6b: Agent (general-purpose) で新 title/body を生成 (light モードのみ) + +メインセッションから以下のように Agent を起動する。プロンプトは +**書いて良いこと / 禁止事項** を必ず明示する +(外側の prompt フェンスは内側に ```json を含むため 4 連バッククォートで囲む): + +````python +Agent( + subagent_type="general-purpose", + description=f"Generate light-rotation PR text for PR #{OLD_PR}", + prompt=f""" +PR rotation の light モードで作成する新 PR の title / body を生成してください。 + +## 素材 +- prepare.json: $TMP_DIR/rotate-pr{STATE_PR}-prepare.json + - 元 PR の title / body / git log $BASE..HEAD / git diff --stat +- 必要なら worktree 内のファイルを直接読んで実装内容を確認してよい + (worktree: {WORKTREE_PATH}) + +## 出力ファイル +$TMP_DIR/rotate-pr{STATE_PR}-newtext.json に JSON で書き出してください: + +```json +{{ + "title": "新 PR の title (元 PR の title をそのままコピーしない。現状の実装を反映)", + "body": "新 PR の body (Markdown)。以下のセクションを含む:\\n## 何のために\\n## 何を\\n## Test plan" +}} +``` + +## 書いて **良い** こと +- 何のために (背景・動機) — 元 PR の背景セクションは再利用可 +- 何を (変更内容) — 現在のブランチの実態を git log / git diff から反映 +- Test plan — 元 PR から継承可 + +## 書いて **はいけない** こと (内部用語の漏洩防止) +- 「round N で〜」「cross-review で〜」「レビュー指摘で〜」 +- 「(rotated)」のような automated suffix +- 「fix された問題」の列挙 / レビューサイクルの存在自体への言及 +- 「旧 PR」「巻き直し」等の rotation 内部用語 + +PR を読む人は cross-review の存在を意識しないため、最終 PR を初めて見る読者向けに +書く。元 title / body をそのままコピーするのは **禁止** (現状の実装を反映)。 +""", +) +```` + +> ⚠ `rotate-pr.sh` 内部から `claude` / `codex` / `gemini` CLI を直接呼んで生成 +> させてはならない (環境依存・コスト管理外)。**メイン側の Agent tool で行う**。 + +### Step 6c: `rotate-pr.sh execute <STATE_PR> --mode light|squash` + +`--mode` で実際の rotation を実行する: + +| mode | 振る舞い | +|---|---| +| `light` (default) | prepare.json と newtext.json を読み、**同ブランチ・同 base** で旧 PR を close → 新 PR を作成。`is_draft=true` なら新 PR も Draft で作る。title/body は newtext から流す | +| `squash` (opt-in) | 既存挙動。`<branch>-rHHMMSS` の新ブランチを作って `git reset --soft origin/$BASE` で squash 統合 → 旧 PR close → 新 PR (`(rotated)` suffix + automated body) | + +stdout には両モードとも以下を KEY=VALUE で出す: + +- `NEW_PR=<number>` +- `NEW_PR_URL=<url>` +- `NEW_BRANCH=<branch>` (light モードでは元ブランチと同じ) + +`state.py set-current-pr` が `state.json` の `current_pr` / `pr_history` を更新する。 +state.json の **キーは元 PR 番号 (STATE_PR) のまま** なので、light/squash どちらでも +後続スクリプトへの第 1 引数は `$STATE_PR` を渡し続ければよい。 -1. state.json から `current_pr` (= 旧 PR) と `worktree_path` を読む -2. 既存ブランチを **squash 統合** した新ブランチ作成 -3. 旧 PR に「ローテーションのため close」コメント + close -4. 新 PR 作成(タイトル末尾に `(rotated)` 付与) -5. 新 PR 番号 / URL / ブランチ名を stdout に KEY=VALUE で吐く +### 後方互換: 旧 1 引数形式 -`state.py set-current-pr` が `state.json` の `current_pr` / `pr_history` を更新。 +`rotate-pr.sh <STATE_PR>` (引数 1 つ) は `execute --mode squash` 相当として動くが、 +stderr に deprecation warning を出す。新規呼び出しは必ず prepare → execute 形式へ移行。 > ⚠ **重要**: state.json のファイル名は **最初に init した PR 番号** がキー > (`$STATE_PR`)。rotation 後も全 scripts の **第 1 引数には常に `$STATE_PR`** を渡す。 diff --git a/plugins/ndf/skills/cross-review/scripts/rotate-pr.sh b/plugins/ndf/skills/cross-review/scripts/rotate-pr.sh index f576261a..28f3e8c5 100755 --- a/plugins/ndf/skills/cross-review/scripts/rotate-pr.sh +++ b/plugins/ndf/skills/cross-review/scripts/rotate-pr.sh @@ -1,58 +1,276 @@ #!/usr/bin/env bash -# PR rotation — squash + 新ブランチ + 新 PR. +# PR rotation — light モード (default) / squash モード # -# Usage: rotate-pr.sh <STATE_PR> +# Usage: +# rotate-pr.sh prepare <STATE_PR> +# 旧 PR の title/body/isDraft + git log $BASE..HEAD + git diff --stat を +# $TMP_DIR/rotate-pr<STATE_PR>-prepare.json に dump する。 +# light モードでは メインセッションの Agent が prepare.json を読み、 +# 現状の差分・実装を反映した新 title/body を生成して +# $TMP_DIR/rotate-pr<STATE_PR>-newtext.json に書き出すこと。 # -# 引数 STATE_PR は state.json の key (= 最初に init した PR 番号)。 -# 閉じる「現在の PR」は state.json の `current_pr` を読む。 +# rotate-pr.sh execute <STATE_PR> [--mode light|squash] (default: light) +# light : 同ブランチで旧 PR を close → 同 head/base で新 PR を作成。 +# title/body は newtext.json から流す。元 PR の isDraft をコピー。 +# PR title に内部用語 (rotated/round/cross-review) は付与しない。 +# squash : 既存挙動。squash 統合した新ブランチ (-rHHMMSS suffix) + 新 PR。 +# title 末尾に "(rotated)"、body は automated text。 +# いずれも stdout に NEW_PR= / NEW_PR_URL= / NEW_BRANCH= を出力。 # -# 既存ブランチを squash した新ブランチを作り、旧 PR (=current_pr) を close、新 PR を作成する。 -# 新 PR 番号を stdout に "NEW_PR=<番号>" / "NEW_PR_URL=<url>" 形式で出力。 +# rotate-pr.sh <STATE_PR> (deprecated) +# 旧 1 引数形式。`execute --mode squash` 相当として動くが、stderr に +# deprecation warning を出す。新規呼び出しは prepare → execute 形式へ移行。 # +# 引数 STATE_PR は state.json の key (= 最初に init した PR 番号)。 +# 閉じる「現在の PR」は state.json の `current_pr` を読む。 # state.json の current_pr / pr_history 更新は `state.py set-current-pr` で別途行う。 set -euo pipefail -STATE_PR=${1:?STATE_PR required} - SCRIPT_DIR=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) # shellcheck source=_tmpdir.sh . "$SCRIPT_DIR/_tmpdir.sh" TMP_DIR=$(tmpdir) -STATE=$TMP_DIR/cross-review-pr$STATE_PR-state.json -[ -s "$STATE" ] || { echo "state.json not found: $STATE" >&2; exit 1; } +usage() { + cat >&2 <<'USAGE' +Usage: + rotate-pr.sh prepare <STATE_PR> + rotate-pr.sh execute <STATE_PR> [--mode light|squash] (default: light) + rotate-pr.sh <STATE_PR> (deprecated, = execute --mode squash) +USAGE +} -WORKTREE=$(jq -r '.worktree_path' "$STATE") -OLD_PR=$(jq -r '.current_pr' "$STATE") -ROUND_IN_PR=$(jq --argjson p "$OLD_PR" '[.rounds[] | select(.pr == $p)] | length' "$STATE") +# close 後に新 PR create が失敗した場合の rollback hook (light/squash 共通)。 +# OLD_PR はグローバル (load_state で set される) を参照する。 +# 両モードから `trap reopen_old_pr_on_failure ERR` で登録し、create 成功直後に +# `trap - ERR` で解除する (gemini round 6 指摘: 関数定義の重複排除 + EXIT ではなく +# ERR で hook して PR 作成後の処理失敗による誤発火を避ける)。 +reopen_old_pr_on_failure() { + local exit_code=$? + echo "⚠ 新 PR 作成系処理に失敗 (exit=$exit_code) — 旧 PR #${OLD_PR:-?} を reopen します" >&2 + if [ -n "${OLD_PR:-}" ]; then + gh pr reopen "$OLD_PR" >&2 || echo "⚠ 旧 PR #$OLD_PR の reopen にも失敗しました。手動で確認してください。" >&2 + fi +} -cd "$WORKTREE" +# state.json から共通情報を読み出して shell 変数にセットする。 +# 呼び出し後: STATE_FILE / WORKTREE / OLD_PR / ROUND_IN_PR が使える。 +load_state() { + local state_pr=$1 + STATE_FILE=$TMP_DIR/cross-review-pr$state_pr-state.json + [ -s "$STATE_FILE" ] || { echo "state.json not found: $STATE_FILE" >&2; exit 1; } + WORKTREE=$(jq -r '.worktree_path' "$STATE_FILE") + OLD_PR=$(jq -r '.current_pr' "$STATE_FILE") + ROUND_IN_PR=$(jq --argjson p "$OLD_PR" '[.rounds[] | select(.pr == $p)] | length' "$STATE_FILE") +} -BRANCH=$(git branch --show-current) -BASE=$(gh pr view "$OLD_PR" --json baseRefName -q .baseRefName) -TITLE=$(gh pr view "$OLD_PR" --json title -q .title) -NEW_BRANCH="${BRANCH}-r$(date +%H%M%S)" +cmd_prepare() { + local state_pr=${1:?STATE_PR required} + load_state "$state_pr" -echo "🔄 PR #$OLD_PR rotation: $BRANCH → $NEW_BRANCH (base=$BASE)" >&2 + cd "$WORKTREE" -# 1. 既存ブランチを squash して新ブランチに -git checkout -b "$NEW_BRANCH" -git reset --soft "origin/$BASE" -git commit -m "$(cat <<EOF -$TITLE + local pr_json + pr_json=$(gh pr view "$OLD_PR" \ + --json number,url,title,body,headRefName,baseRefName,isDraft) + local head_branch base_branch + head_branch=$(jq -r '.headRefName' <<<"$pr_json") + base_branch=$(jq -r '.baseRefName' <<<"$pr_json") -(cross-review rotation: PR #$OLD_PR を squash 統合) -EOF -)" -git push -u origin "$NEW_BRANCH" + # base が origin にあることを保証 (git log/diff のため) + if ! git fetch --quiet origin "$base_branch"; then + echo "⚠ git fetch origin $base_branch に失敗しました。ローカル参照のみで継続します。" >&2 + fi + if ! git rev-parse --verify --quiet "origin/$base_branch" >/dev/null; then + echo "⚠ origin/$base_branch が見つかりません。git_log / git_diff_stat は空になります。" >&2 + fi + + local range="origin/$base_branch..HEAD" + local git_log git_diff_stat + git_log=$(git log --pretty=format:'%h %s' "$range" 2>/dev/null || echo "") + git_diff_stat=$(git diff --stat "$range" 2>/dev/null || echo "") + + local out=$TMP_DIR/rotate-pr$state_pr-prepare.json + jq -n \ + --argjson state_pr "$state_pr" \ + --argjson pr "$pr_json" \ + --argjson round_in_pr "$ROUND_IN_PR" \ + --arg worktree "$WORKTREE" \ + --arg git_log "$git_log" \ + --arg git_diff_stat "$git_diff_stat" \ + '{ + state_pr: $state_pr, + old_pr: $pr.number, + old_pr_url: $pr.url, + worktree_path: $worktree, + head_branch: $pr.headRefName, + base_branch: $pr.baseRefName, + is_draft: $pr.isDraft, + round_in_pr: $round_in_pr, + old_title: $pr.title, + old_body: ($pr.body // ""), + git_log: $git_log, + git_diff_stat: $git_diff_stat + }' > "$out" + + echo "✅ prepare.json 書き出し: $out" >&2 + # 呼び出し側 (SKILL.md / docs) は eval で stdout を取り込む契約。 + # PR の head/base 由来の値は shell メタ文字を含み得るため必ず printf '%q' で + # シェルエスケープしてから出力する (例: ブランチ名に "; rm -rf / 等が来ても安全)。 + printf 'PREPARE_JSON=%q\n' "$out" + printf 'OLD_PR=%q\n' "$OLD_PR" + printf 'HEAD_BRANCH=%q\n' "$head_branch" + printf 'BASE_BRANCH=%q\n' "$base_branch" + printf 'IS_DRAFT=%q\n' "$(jq -r '.isDraft' <<<"$pr_json")" +} + +# light モード本体: 同ブランチで旧 PR を close → 同 head/base で新 PR 作成。 +execute_light() { + local state_pr=$1 + load_state "$state_pr" + + local prep=$TMP_DIR/rotate-pr$state_pr-prepare.json + local newtext=$TMP_DIR/rotate-pr$state_pr-newtext.json + [ -s "$prep" ] || { echo "prepare.json not found: $prep — 先に rotate-pr.sh prepare $state_pr を実行してください" >&2; exit 1; } + [ -s "$newtext" ] || { echo "newtext.json not found: $newtext — Agent (general-purpose) で title/body を生成して書き出してください" >&2; exit 1; } + + local head_branch base_branch is_draft new_title new_body + head_branch=$(jq -r '.head_branch' "$prep") + base_branch=$(jq -r '.base_branch' "$prep") + is_draft=$(jq -r '.is_draft' "$prep") + new_title=$(jq -r '.title' "$newtext") + new_body=$(jq -r '.body' "$newtext") + + [ -n "$new_title" ] && [ "$new_title" != "null" ] || { echo "newtext.json に .title がない" >&2; exit 1; } + # body は空文字列を許容 (GitHub は空 body を許容)。null のみ拒否。 + [ "$new_body" != "null" ] || { echo "newtext.json に .body がない (null)" >&2; exit 1; } + + cd "$WORKTREE" + + echo "🔄 PR #$OLD_PR rotation (light): 同ブランチ $head_branch で巻き直し (base=$base_branch)" >&2 + + # 1. 作業ディレクトリに未 push のコミットがある可能性に備え、close 前に push する。 + # push しないと新 PR に最新コミットが反映されないケースがあるため必須 (gemini 指摘)。 + # state.py init は worktree を detached HEAD で作るため、ブランチ名のみで push すると + # detached HEAD 上の修正コミットが push されない。HEAD:<branch> 形式で現在の HEAD を + # 明示する (codex 指摘)。--force / --no-verify は禁止。 + echo "🔼 git push origin HEAD:$head_branch (未 push commit が無ければ no-op)" >&2 + git push origin HEAD:"$head_branch" + + # 2. 旧 PR を close (コメント残し) + gh pr comment "$OLD_PR" --body "ℹ️ レビューコメント履歴整理のため本 PR を一度 close し、同じブランチ \`$head_branch\` で新 PR を作り直します。ブランチの内容・base は変えません。" + gh pr close "$OLD_PR" + + # close 後に create が失敗した場合は旧 PR を reopen して rotation の途中停止を回避する + # (関数定義は file 冒頭で共通化, gemini round 6 指摘) + trap reopen_old_pr_on_failure ERR -# 2. 旧 PR を close(コメント残し) -gh pr comment "$OLD_PR" --body "🔄 cross-review ループ進行中のため、本 PR を close し新規 PR に巻き直します。 round_in_pr=$ROUND_IN_PR で長尺化を回避。" -gh pr close "$OLD_PR" + # 3. 新 PR を同 head/base で作成 (Draft 状態は元 PR から継承)。 + # body は --body-file - 経由で stdin から渡し、argv 長制限を回避する (gemini 指摘)。 + local create_args=(--base "$base_branch" --head "$head_branch" --title "$new_title" --body-file -) + if [ "$is_draft" = "true" ]; then + create_args+=(--draft) + fi + local new_pr_url + new_pr_url=$(printf '%s' "$new_body" | gh pr create "${create_args[@]}") -# 3. 新 PR 作成 -NEW_PR_URL=$(gh pr create --base "$BASE" --title "$TITLE (rotated)" --body "$(cat <<EOF + # gh pr create 成功直後に trap を解除し、後続の URL parse / echo 等が失敗しても + # 新旧 PR が重複して開く事態を避ける (gemini round 6 指摘)。 + trap - ERR + + # PR 番号は create 出力 URL の末尾セグメントから抽出 (gh pr view 追加呼び出しを削減, + # gemini round 6 指摘)。URL 形式: https://github.com/<owner>/<repo>/pull/<number> + local new_pr=${new_pr_url##*/} + + echo "✅ 新 PR #$new_pr: $new_pr_url" >&2 + # eval される契約。head_branch / URL に shell メタ文字が混ざっても安全なよう %q で escape + printf 'NEW_PR=%q\n' "$new_pr" + printf 'NEW_PR_URL=%q\n' "$new_pr_url" + printf 'NEW_BRANCH=%q\n' "$head_branch" +} + +# squash モード本体: 既存挙動を完全維持。 +execute_squash() { + local state_pr=$1 + load_state "$state_pr" + + cd "$WORKTREE" + + local branch base title new_branch pr_meta prep + prep=$TMP_DIR/rotate-pr$state_pr-prepare.json + + # PR メタ情報 (base / title / head) は、まず prepare.json があればそこから読み出し、 + # 無い場合のみ gh pr view にフォールバックする (execute_light と同じ方針で + # 不要な API 呼び出しを排除, gemini round 8 指摘)。 + if [ -s "$prep" ]; then + base=$(jq -r '.base_branch // empty' "$prep") + title=$(jq -r '.old_title // empty' "$prep") + fi + if [ -z "${base:-}" ] || [ -z "${title:-}" ]; then + pr_meta=$(gh pr view "$OLD_PR" --json headRefName,baseRefName,title) + [ -n "${base:-}" ] || base=$(printf '%s' "$pr_meta" | jq -r '.baseRefName') + [ -n "${title:-}" ] || title=$(printf '%s' "$pr_meta" | jq -r '.title') + fi + + # state.py init は worktree を `git worktree add --detach origin/<head>` で作るため、 + # `git branch --show-current` は空文字を返す。空のまま new_branch を生成すると + # `-rHHMMSS` だけのブランチ名になってしまうので、フォールバック順を以下に固定する: + # 1. git branch --show-current (通常 worktree なら使える) + # 2. prepare.json の head_branch (prepare 済みなら最も信頼できる) + # 3. gh pr view --json headRefName (prepare 未実行でも復元可能) + # (codex round 4 指摘) + branch=$(git branch --show-current) + if [ -z "$branch" ] && [ -s "$prep" ]; then + branch=$(jq -r '.head_branch // empty' "$prep") + fi + if [ -z "$branch" ]; then + pr_meta=${pr_meta:-$(gh pr view "$OLD_PR" --json headRefName,baseRefName,title)} + branch=$(printf '%s' "$pr_meta" | jq -r '.headRefName') + fi + [ -n "$branch" ] || { echo "head branch を復元できませんでした (detached worktree かつ prepare.json / gh pr view から取得失敗)" >&2; exit 1; } + new_branch="${branch}-r$(date +%H%M%S)" + + # 既に title 末尾に "(rotated)" / "(rotated2)" 等が付いている場合は除去してから + # "(rotated)" を 1 つだけ付与し、ローテーションのたびに suffix が重複しないようにする + # (gemini round 8 指摘)。 + # 例: + # "Fix foo" → "Fix foo (rotated)" + # "Fix foo (rotated)" → "Fix foo (rotated)" + # "Fix foo (rotated2)" → "Fix foo (rotated)" + # "Fix foo (rotated)(rotated)" → "Fix foo (rotated)" + local title_stripped=$title + while [[ $title_stripped =~ [[:space:]]*\(rotated[0-9]*\)$ ]]; do + title_stripped=${title_stripped%"${BASH_REMATCH[0]}"} + done + local new_title="$title_stripped (rotated)" + + echo "🔄 PR #$OLD_PR rotation (squash): $branch → $new_branch (base=$base)" >&2 + + # 1. 既存ブランチを squash して新ブランチに + git checkout -b "$new_branch" + git reset --soft "origin/$base" + # commit message は -m を複数指定で分割して渡す。$(cat <<EOF ... $title ... EOF) 形式は + # PR title に $(...) や `...` が含まれる場合にコマンド置換として実行される脆弱性がある + # ため使わない (gemini round 7 指摘)。 + git commit \ + -m "$title" \ + -m "(cross-review rotation: PR #$OLD_PR を squash 統合)" + git push -u origin "$new_branch" + + # 2. 旧 PR を close (コメント残し) + gh pr comment "$OLD_PR" --body "🔄 cross-review ループ進行中のため、本 PR を close し新規 PR に巻き直します。 round_in_pr=$ROUND_IN_PR で長尺化を回避。" + gh pr close "$OLD_PR" + + # close 後に create が失敗した場合は旧 PR を reopen して rotation の途中停止を回避する + # (関数定義は file 冒頭で共通化, gemini round 6 指摘) + trap reopen_old_pr_on_failure ERR + + # 3. 新 PR 作成 + # body は --body-file - 経由で stdin から渡し、argv 長制限を回避する + # (execute_light と統一, gemini round 5 指摘) + local new_body + new_body=$(cat <<EOF ## Summary 旧 PR #$OLD_PR をベースに、cross-review クロスレビューループの継続。 旧 PR は round_in_pr=$ROUND_IN_PR で巻き直しのため close 済み。 @@ -60,11 +278,85 @@ NEW_PR_URL=$(gh pr create --base "$BASE" --title "$TITLE (rotated)" --body "$(ca <!-- I want to review in Japanese. --> EOF -)") +) + local new_pr_url + new_pr_url=$(printf '%s' "$new_body" | gh pr create --base "$base" --title "$new_title" --body-file -) + + # gh pr create 成功直後に trap を解除し、後続の URL parse / echo 等が失敗しても + # 新旧 PR が重複して開く事態を避ける (gemini round 6 指摘)。 + trap - ERR + + # PR 番号は create 出力 URL の末尾セグメントから抽出 (gh pr view 追加呼び出しを削減, + # gemini round 6 指摘)。URL 形式: https://github.com/<owner>/<repo>/pull/<number> + local new_pr=${new_pr_url##*/} + + echo "✅ 新 PR #$new_pr: $new_pr_url" >&2 + # eval される契約。new_branch / URL に shell メタ文字が混ざっても安全なよう %q で escape + printf 'NEW_PR=%q\n' "$new_pr" + printf 'NEW_PR_URL=%q\n' "$new_pr_url" + printf 'NEW_BRANCH=%q\n' "$new_branch" +} + +cmd_execute() { + local state_pr=${1:?STATE_PR required} + shift + # --mode 未指定時は light を default (SKILL.md / 02-fix-and-rotation.md / スクリプト + # 冒頭コメントの「light モード (default)」表記に CLI 契約を揃える, codex round 4 指摘) + local mode="light" + while [ $# -gt 0 ]; do + case $1 in + --mode) + mode=${2:?--mode requires light|squash} + shift 2 + ;; + --mode=*) + mode=${1#--mode=} + shift + ;; + *) + echo "unknown arg: $1" >&2 + usage + exit 2 + ;; + esac + done + case $mode in + light) execute_light "$state_pr" ;; + squash) execute_squash "$state_pr" ;; + *) echo "invalid --mode: $mode (light|squash)" >&2; exit 2 ;; + esac +} + +# ---- entrypoint ---- -NEW_PR=$(echo "$NEW_PR_URL" | grep -oE '/pull/[0-9]+' | grep -oE '[0-9]+') +if [ $# -eq 0 ]; then + usage + exit 2 +fi -echo "✅ 新 PR #$NEW_PR: $NEW_PR_URL" >&2 -echo "NEW_PR=$NEW_PR" -echo "NEW_PR_URL=$NEW_PR_URL" -echo "NEW_BRANCH=$NEW_BRANCH" +case $1 in + prepare) + shift + cmd_prepare "$@" + ;; + execute) + shift + cmd_execute "$@" + ;; + -h|--help) + usage + ;; + *) + # 旧形式: rotate-pr.sh <STATE_PR> → squash 相当 + if [ $# -eq 1 ] && [[ $1 =~ ^[0-9]+$ ]]; then + echo "⚠ DEPRECATED: rotate-pr.sh <STATE_PR> 形式は廃止予定です。新形式に移行してください:" >&2 + echo " rotate-pr.sh prepare $1" >&2 + echo " rotate-pr.sh execute $1 --mode light|squash" >&2 + echo " (本実行は --mode squash 相当で継続します)" >&2 + execute_squash "$1" + else + usage + exit 2 + fi + ;; +esac diff --git a/plugins/ndf/skills/cross-review/scripts/state.py b/plugins/ndf/skills/cross-review/scripts/state.py index 45305b73..0fa9bbe4 100755 --- a/plugins/ndf/skills/cross-review/scripts/state.py +++ b/plugins/ndf/skills/cross-review/scripts/state.py @@ -460,7 +460,13 @@ def cmd_merge_fix(args: argparse.Namespace) -> None: def cmd_should_rotate(args: argparse.Namespace) -> None: - """Step 6 — PR ローテーション要否。Exit 0=rotate, 2=keep.""" + """Step 6 — PR ローテーション要否。Exit 0=rotate, 2=keep. + + 判定は ``round_in_pr >= rotate_after && total < max_rounds`` のみで、 + rotate-pr.sh の ``--mode light|squash`` どちらでも同じ条件を使う。 + state.json の key は ``STATE_PR`` (最初に init した PR 番号) で固定なので、 + light モードで head_branch が変わらない場合でも整合する。 + """ pr = args.pr st = _load(pr) current_pr = st["current_pr"] @@ -477,7 +483,13 @@ def cmd_should_rotate(args: argparse.Namespace) -> None: def cmd_set_current_pr(args: argparse.Namespace) -> None: - """PR ローテーション完了後の state 更新。""" + """PR ローテーション完了後の state 更新。 + + rotate-pr.sh の light / squash どちらでも、新 PR 番号を受け取って + ``current_pr`` を切り替え、``pr_history`` に新 PR エントリを追加する。 + state.json のファイル名は ``STATE_PR`` (= ``args.pr``) ベースで不変なので、 + light モードで head_branch が変わらないケースでも問題なく追跡できる。 + """ pr = args.pr # 旧 PR (state file の key) new_pr = args.new_pr st = _load(pr)