Skip to content

Fix: cross-review PR rotation を light モード (default) / squash モード (opt-in) に分割 (REPORT02) - #5

Merged
takemi-ohama merged 11 commits into
mainfrom
feature/REPORT02-rotate-pr-light-mode
May 23, 2026
Merged

Fix: cross-review PR rotation を light モード (default) / squash モード (opt-in) に分割 (REPORT02)#5
takemi-ohama merged 11 commits into
mainfrom
feature/REPORT02-rotate-pr-light-mode

Conversation

@takemi-ohama

Copy link
Copy Markdown
Contributor

何のために

/ndf:cross-review--rotate-after で発火する scripts/rotate-pr.sh の挙動が、利用者の期待 (「コメント履歴が長くなった PR を AI Agent が読みやすいようにリセットしたい」だけ) と乖離していた。実運用 (PLAN03 PR1, #217 → #221) で以下の副作用が顕在化:

  • 不要な squash でコミット単位レビューが不可能に
  • 時刻 suffix -rHHMMSS のブランチ名が release branch 戦略 / TODO 参照を破壊
  • PR title に (rotated) 内部用語が漏洩
  • 元 PR の body (何のために / 何を / Test plan) が automated body に上書き

詳細: issues/REPORT02.md / issues/REPORT02_rotate-pr-light-mode.md

何を

scripts/rotate-pr.shlight モード (default) / squash モード (opt-in) の 2 モードに分割。/ndf:cross-review 側にも --rotate-mode light|squash を生やす (default=light)。

1. scripts/rotate-pr.sh を 3 段構成に分割

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(subagent_type="general-purpose") を起動して
$TMP_DIR/rotate-pr<STATE_PR>-newtext.json に title/body を書き出す
rotate-pr.sh execute <STATE_PR> --mode light|squash
light : 同ブランチで旧 PR close → 同 head/base で新 PR 作成 (Draft 状態継承)
squash : 既存挙動 (新ブランチ + squash 統合 + (rotated) suffix)

旧 1 引数形式 rotate-pr.sh <STATE_PR> は deprecation warning 付きで squash 相当として残す。

2. ドキュメント更新

  • SKILL.md: 引数表に --rotate-mode 追加、Step 6 の bash 骨組みを 3 段呼び出しに更新、mermaid 図とアンチパターン節を更新
  • docs/02-fix-and-rotation.md: Step 6a/6b/6c に分割、6b の Agent 起動プロンプト例 + newtext.json スキーマを記載
  • scripts/state.py: should-rotate / set-current-pr の docstring に light モード対応を追記 (コード変更なし)

3. light モードで書いてはいけない内部用語 (Agent プロンプトで明示禁止)

  • 「round N で〜」「cross-review で〜」「レビュー指摘で〜」
  • 「(rotated)」のような automated suffix
  • 「fix された問題」の列挙 / レビューサイクルの存在自体への言及

Test plan

cross-review は実 PR を作成するため自動テストは難しい。以下を 手動 E2E で確認する:

静的チェック (本 PR でローカル実施済み)

  • bash -n rotate-pr.sh が通る (syntax OK)
  • rotate-pr.sh / rotate-pr.sh -h で usage が出る (exit=2)
  • rotate-pr.sh prepare (STATE_PR 欠落) で適切なエラー (exit=1)
  • rotate-pr.sh execute 99999 --mode fooinvalid --mode エラー (exit=2)
  • rotate-pr.sh execute 99999 (mode 欠落) で --mode is required エラー (exit=2)
  • rotate-pr.sh prepare 99999 で state.json not found (exit=1)
  • state.py --help / state.py should-rotate --help / state.py set-current-pr --help が正常表示

light モード E2E (テスト用 PR で実施)

  • 検証用ブランチで Draft PR を作り、rotate-pr.sh prepare → Agent (general-purpose) で newtext.json 生成 → 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 が stderr に出る

state.py 整合

  • should-rotaterotate-pr.sh prepare → Agent → rotate-pr.sh execute --mode lightstate.py set-current-pr の一連で state.json.current_pr が正しく更新される

関連

  • 元 issue: REPORT02 (issues/REPORT02.md)
  • 実装プラン: issues/REPORT02_rotate-pr-light-mode.md
  • 実運用での再現事例: PLAN03 PR1 (#217#221)
  • 関連 skill: /ndf:cross-review, /ndf:review, /ndf:fix, /ndf:implementation-plan

takemi-ohamaand others added 2 commits May 22, 2026 07:27
cross-review の --rotate-after で発火する rotate-pr.sh の挙動が、
利用者の期待 (コメント履歴リセットのみ) と乖離しており、
PLAN03 PR1 で release branch 戦略 / TODO 参照を破壊した実害が出たため、
default を light モード (同ブランチで PR を作り直す) に切り替える実装の
issue と実装プランを追加。
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-in) に分割 (REPORT02)
`--rotate-after` で発火する rotation の default を、利用者の期待 (コメント
履歴リセットのみ) に合致する **light モード** に切り替える。squash モードは
backward compat のため opt-in (`--rotate-mode squash`) で残す。
## 変更内容
### `scripts/rotate-pr.sh`
- `prepare <STATE_PR>` / `execute <STATE_PR> --mode light|squash` の 3 段構成に分割
- light モード: 同ブランチで旧 PR を close → 同 head/base で新 PR を作成。
title/body は Agent (general-purpose) が `rotate-pr<STATE_PR>-newtext.json`
に書き出したものを流す。元 PR の `isDraft` をコピー
- squash モード: 既存挙動を完全維持 (`-rHHMMSS` suffix / squash 統合 / `(rotated)` suffix)
- 旧 1 引数形式 `rotate-pr.sh <STATE_PR>` は deprecation warning 付きで squash 相当
- prepare は `gh pr view` + `git log $BASE..HEAD` + `git diff --stat` を JSON dump
### `SKILL.md`
- 引数表に `--rotate-mode light|squash` (default=light) を追加
- 「実行ステップ概要」の Step 6 を `prepare → (light のみ Agent) → execute` の
3 段呼び出しに更新
- mermaid 図の Rotate ノード / アンチパターン節を更新
### `docs/02-fix-and-rotation.md`
- Step 6 を 6a/6b/6c に分割し、6b の Agent 起動例と newtext.json スキーマを記載
- light モードで書いてはいけない内部用語 (round / rotated / cross-review) を明示
### `scripts/state.py`
- `should-rotate` / `set-current-pr` の docstring に「light モードでも整合する
(state.json の key は STATE_PR 不変)」旨を追記。コード変更なし
## Motivation
PLAN03 PR1 で `--rotate-after 5` を発火させたところ、release branch 戦略 /
TODO 参照 / コミット単位レビューを破壊した。利用者の意図は「コメント履歴の
リセット」のみだったため、default を light に切り替え、新規利用者の期待値と
一致させる (Option A 採用、Option B は不採用)。
詳細は issues/REPORT02_rotate-pr-light-mode.md 参照。
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@takemi-ohama
takemi-ohama marked this pull request as ready for review May 22, 2026 08:19

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 1 | gemini | COMMENT

light モードの導入により、ブランチ履歴を汚さずに PR ローテーションが可能になる点は非常に優れています。スクリプトの分割 (prepare / execute) も Agent との連携を考慮した良い設計です。
PR 番号の取得や git 操作の堅牢性を少し高めることで、より安定した運用が可能になると考えられます。

Comment threadplugins/ndf/skills/cross-review/scripts/rotate-pr.sh Outdated
Comment threadplugins/ndf/skills/cross-review/scripts/rotate-pr.sh Outdated
Comment threadplugins/ndf/skills/cross-review/scripts/rotate-pr.sh Outdated

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 1 | codex | REQUEST_CHANGES

light モードが default になっているため、rotation 実行経路で newtext 生成の実行保証と旧 PR close 後の失敗復旧を入れてからマージしてください。

Comment threadplugins/ndf/skills/cross-review/SKILL.md Outdated
Comment threadplugins/ndf/skills/cross-review/scripts/rotate-pr.sh Outdated
Comment threadplugins/ndf/skills/cross-review/scripts/rotate-pr.sh
…ajor)
PR #5 cross-review codex round 1 指摘 (SKILL.md:210) 対応。
light モードのテンプレートが no-op (`:`) のまま Step 6c に進むため、
Agent (general-purpose) が newtext.json を生成し損ねた場合に
rotate-pr.sh execute --mode light が必ず失敗していた。
newtext.json の存在を確認し、未生成なら exit 10 でループを停止して
メインセッションに Agent 起動を促すガードを追加。
PR #5 round 1 で指摘された rotate-pr.sh の 5 件をまとめて対応。
- [codex minor:114] prepare.json 欠落時のエラーメッセージから バッククォートを除去。
バッククォートが command substitution として評価され、
意図せず `rotate-pr.sh prepare ...` を実行してしまう問題を修正。
- [codex major:133] light モードで gh pr close 後に gh pr create が失敗すると
rotation が途中停止して open PR を失う問題。close 前に trap で
reopen_old_pr_on_failure を仕込み、create 成功時のみ trap 解除する形に変更。
- [gemini minor:69] git fetch origin <base> 失敗を `|| true` で握り潰していたため、
origin/<base> が無いケースを検知できなかった。fetch 失敗 + ref 存在確認の
2 段で警告を出すように変更。
- [gemini major:141] light モードでの新 PR 番号取得を URL 文字列の grep から
`gh pr view <url> --json number -q .number` に変更し、より頑健化。
- [gemini major:183] squash モードでも同様に PR 番号取得を --json number 経由へ。

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 2 | gemini | REQUEST_CHANGES

execute_light モードにおける堅牢性と信頼性に関する課題が見つかりました。特に空の PR body を許容しない点や、未 push の変更が反映されない点について修正が必要です。

Comment threadplugins/ndf/skills/cross-review/scripts/rotate-pr.sh Outdated
Comment threadplugins/ndf/skills/cross-review/scripts/rotate-pr.sh
Comment threadplugins/ndf/skills/cross-review/scripts/rotate-pr.sh Outdated

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 2 | codex | REQUEST_CHANGES

修正が必要です。light rotation の再開手順と、eval 前提の出力契約を先に固めてください。

Comment threadplugins/ndf/skills/cross-review/scripts/rotate-pr.sh Outdated
Comment threadplugins/ndf/skills/cross-review/SKILL.md
Comment threadplugins/ndf/skills/cross-review/docs/02-fix-and-rotation.md
…d 2)
- rotate-pr.sh: stdout の KEY=VALUE 出力を printf '%q' で全てシェルエスケープ
(呼び出し側 eval 経由で head/base ブランチ名のメタ文字が実行されないよう防御 / codex)
- rotate-pr.sh: execute_light で空 body を許容 (null のみ拒否 / gemini)
- rotate-pr.sh: execute_light で close 前に git push を実行
(未 push commit がある場合の取りこぼし防止 / gemini)
- rotate-pr.sh: --body "$new_body" を --body-file - + stdin に変更
(長大 body での argv 長制限を回避 / gemini)
- SKILL.md / docs/02: exit 10 を「メイン介入待ち停止」と再定義し、
再開プロトコル (Step 6b Agent → Step 6c execute 直接呼び出し) を明文化 (codex)
- docs/02-fix-and-rotation.md: 外側 python フェンスを 4 連バッククォートに変更
(内側 ```json でブロックが閉じる問題を解消 / codex)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 3 | gemini | APPROVE

シェルスクリプトの安全な変数展開や、PR作成時の引数制限回避など、実用上の堅牢性が向上しています。いくつか軽微な汎用性の向上案をコメントしました。

Comment threadplugins/ndf/skills/cross-review/scripts/rotate-pr.sh Outdated
Comment threadplugins/ndf/skills/cross-review/scripts/rotate-pr.sh

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 3 | codex | REQUEST_CHANGES

修正必須: light モードの rotation で、detached worktree 上の最新修正を head ブランチへ確実に push する必要があります。

Comment threadplugins/ndf/skills/cross-review/scripts/rotate-pr.sh Outdated

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 4 | gemini | APPROVE

今回の変更は、state.py init が worktree を detached HEAD で作成することに起因する、execute_light での git push 失敗を的確に修正しています。HEAD:$head_branch 形式の採用により、現在のコミットが正しく反映されるようになり、堅牢性が向上しました。

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 4 | codex | REQUEST_CHANGES

squash mode と light/default のCLI契約に実行時の不整合があります。下記2点を直してください。

Comment threadplugins/ndf/skills/cross-review/scripts/rotate-pr.sh
Comment threadplugins/ndf/skills/cross-review/scripts/rotate-pr.sh Outdated
…ht (codex round 4)
- execute_squash: state.py init が `git worktree add --detach` で worktree を作る
ため、`git branch --show-current` は空になり new_branch が `-rHHMMSS` だけに
なる問題を修正。フォールバック順を 1) git branch --show-current 2) prepare.json
の head_branch 3) gh pr view --json headRefName とした。
- cmd_execute: SKILL.md / docs / スクリプト冒頭の「light モード (default)」表記に
合わせて --mode 未指定時の default を light に変更。usage 表記も更新。
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 5 | gemini | REQUEST_CHANGES

execute_light で導入された失敗時リカバリ処理が execute_squash に反映されておらず、一貫性と堅牢性に課題があります。また、コマンドライン長制限への対策も全モードで統一することが望ましいです。

Comment threadplugins/ndf/skills/cross-review/scripts/rotate-pr.sh
Comment threadplugins/ndf/skills/cross-review/scripts/rotate-pr.sh Outdated

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 5 | codex | APPROVE

修正必須の指摘はありません。

…gemini round 5)
- execute_squash で gh pr close 直後に trap reopen_old_pr_on_failure を設定
し、新 PR 作成失敗時に旧 PR を reopen するリカバリ処理を追加。
execute_light と同じパターンに揃える (comment_id=3287213842)。
- execute_squash の gh pr create を printf | --body-file - 経由に変更し、
argv 長制限を回避。execute_light と body 渡し方を統一
(comment_id=3287213847)。

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 6 | gemini | REQUEST_CHANGES

PR の堅牢性と効率性を向上させるための修正提案です。gh pr create の失敗時に旧 PR を reopen する仕組みは優れていますが、実装上の重複排除と、通信回数の削減が可能です。

Comment threadplugins/ndf/skills/cross-review/scripts/rotate-pr.sh
Comment threadplugins/ndf/skills/cross-review/scripts/rotate-pr.sh Outdated
Comment threadplugins/ndf/skills/cross-review/scripts/rotate-pr.sh

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 6 | codex | APPROVE

修正必須の新規指摘はありません。

…ini round 6)
- reopen_old_pr_on_failure を file 冒頭で 1 度だけ定義し light/squash 両方で再利用
(関数定義の重複排除, comment 3292611499)
- gh pr view --json number 呼び出しを削減し、create が返す URL の末尾セグメント
(${url##*/}) から PR 番号を抽出 (comment 3292611506)
- gh pr create 成功直後に trap - ERR で reopen rollback を解除し、URL parse 等の
後続処理失敗で新旧 PR が重複 open する事態を回避 (comment 3292611508)
- trap は EXIT ではなく ERR を使い、正常終了時の誤発火を避ける副次効果あり

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 7 | gemini | REQUEST_CHANGES

rotate-pr.shexecute_squash において、PR タイトルに含まれる特殊文字がシェルで誤評価されるリスクと、gh pr view 呼び出しの冗長性に関する修正を提案します。

Comment threadplugins/ndf/skills/cross-review/scripts/rotate-pr.sh Outdated
Comment threadplugins/ndf/skills/cross-review/scripts/rotate-pr.sh Outdated
…ni round 7)
- git commit -m "$(cat <<EOF ... $title ... EOF)" 形式は PR title に
$(...) や `...` が含まれる場合にコマンド置換として実行される脆弱性が
あるため、-m を複数指定で title と注記を分離 (major)
- gh pr view を 3 回呼び出していた箇所を 1 度の --json head/base/title
+ jq に統合し、ネットワーク往復と API 負荷を削減 (minor)
- branch fallback の 3 番目も pr_meta から取り出す形に統一
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@takemi-ohamatakemi-ohama left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 cross-review | round 8 | gemini | COMMENT

execute_squash における API 呼び出しの削減と、PR タイトルの重複付与に関する改善を提案します。

Comment threadplugins/ndf/skills/cross-review/scripts/rotate-pr.sh Outdated
Comment threadplugins/ndf/skills/cross-review/scripts/rotate-pr.sh Outdated
… (gemini round 8)
- execute_squash も execute_light と同様に prepare.json から base / title /
head_branch を優先的に取得し、gh pr view 呼び出しを skip 可能にした
(prepare.json が無い場合のみ gh pr view にフォールバック)。
- 新タイトル生成時、title 末尾に既存の "(rotated)" / "(rotatedN)" が
あれば剥がしてから 1 つだけ "(rotated)" を付与する。
rotation を繰り返しても suffix が重複しない。
@takemi-ohama
takemi-ohama merged commit cee5687 into mainMay 23, 2026
takemi-ohama added a commit that referenced this pull request May 26, 2026
- conftest.py.template: browser_type_launch_args を依存に追加し local モードで
--headed/slow_mo/channel 等が反映されるよう修正 (#1, #6)
- conftest.py.template: 変数名 b → browser に改名 (#2)
- conftest.py.template: CDP 接続時に非 Chromium ブラウザの fail-fast チェック追加 (#5)
- config.py: BrowserConfig.cdp_endpoint の既定値を ws:// → http:// に変更 (#4)
- CHANGELOG.md: v4.10.0 エントリを追加 (#3)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@takemi-ohama
takemi-ohama deleted the feature/REPORT02-rotate-pr-light-mode branch August 14, 2026 03:58
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@takemi-ohama