Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
162 changes: 162 additions & 0 deletions issues/PLAN13_cross-review-review-body-missing.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
# PLAN13: cross-review — PR review body の指摘見落とし修正

- 起票日: 2026-05-25
- 対象 plugin: `ndf` v4.7.5
- 対象 skill: `ndf:cross-review`, `ndf:fix`, `ndf:review-pr-comments`
- 関連 issue: [GitHub Issue #13](https://github.com/devbasex/ai-plugins/issues/13)
- 報告者: takemi-ohama
- 実際のケース: [carmo-system-console PR #14137](https://github.com/volareinc/carmo-system-console/pull/14137) で人間レビュアーの `CHANGES_REQUESTED` review body 指摘が cross-review 6 ラウンド通じて検出されなかった

## 背景・課題

### 現状

PR コメント取得時に **インラインコメント (`pulls/{pr}/comments`)** のみを取得している。GitHub の PR コメントは 3 つのソースに分かれるが、うち 2 つが見落とされている:

| ソース | API | 取得状況 | 内容 |
|---|---|---|---|
| インラインコメント | `pulls/{pr}/comments` | ✅ 取得済み | diff の特定行に紐づくコメント |
| レビュー body | `pulls/{pr}/reviews` の `body` フィールド | ❌ **未取得** | レビュー投稿時の総評テキスト |
| PR レベルコメント | `issues/{pr}/comments` | ❌ **未取得** | Conversation タブの通常コメント |

### 影響

- 人間レビュアーが review body にのみ指摘を書いた場合、cross-review ループ全体で検出されない
- `/ndf:fix` が review body の指摘を修正対象として認識しない
- `/ndf:review-pr-comments` が review body/PR レベルコメントを分類対象に含めない

### 影響箇所

| ファイル | 修正内容 |
|---|---|
| `plugins/ndf/skills/fix/scripts/fetch-pr-comments.sh` | **新規作成** — 3 ソース一括取得の共有スクリプト |
| `plugins/ndf/skills/cross-review/scripts/state.py` (L306-324) | 既存の `gh api` 直接呼び出しを `fetch-pr-comments.sh` 呼び出しに差し替え |
| `plugins/ndf/skills/fix/SKILL.md` (L184) | スクリプト参照と使い方を追記 |
| `plugins/ndf/skills/review-pr-comments/SKILL.md` (L49) | コメント取得を共有スクリプト参照に変更 |
| `plugins/ndf/skills/cross-review/SKILL.md` (L87) | Step 4 の説明を共有スクリプト経由に更新 |
| `plugins/ndf/skills/cross-review/docs/01-state-and-review.md` (L98) | 既存コメント差分の説明を共有スクリプト参照に更新 |
| `plugins/ndf/skills/cross-review/docs/02-fix-and-rotation.md` (L76) | fix prompt 内のコメント取得手順を共有スクリプト参照に更新 |

## 修正方針

### 設計方針: コメント取得の共有スクリプト化

3 つのスキル (cross-review, fix, review-pr-comments) が同じ 3 ソースの `gh api` 呼び出しを必要とするため、共有スクリプトに切り出す。

**配置場所**: `plugins/ndf/skills/fix/scripts/fetch-pr-comments.sh`

cross-review は既に fix をサブエージェント経由で呼ぶ依存関係にあるため、fix 側にスクリプトを置けば依存方向が一致する。review-pr-comments も fix の前段(分類→修正)の関係。

### 1. `fix/scripts/fetch-pr-comments.sh` — 共有スクリプト新規作成 (コア)

3 ソースを一括取得し、タグ付き行単位で stdout に出力するシェルスクリプト。

```bash
#!/usr/bin/env bash
# Usage: fetch-pr-comments.sh <owner/repo> <pr_number>
set -uo pipefail # -e は意図的に外す (0件ソースで後続が止まるのを防止)

REPO="$1"
PR="$2"

# 1. インラインコメント (diff の特定行に紐づく)
# 本文全体を保持。改行は \n エスケープして 1 行に収める。
if ! gh api "repos/${REPO}/pulls/${PR}/comments" --paginate --jq \
'.[] | "\(.path // "?"):\(.line // .original_line // "?") [\(.user.login)] \(.body // "" | gsub("\n"; "\\n"))"'; then
(( FAIL_COUNT += 1 )) || true
fi

# 2. レビュー body (CHANGES_REQUESTED / COMMENTED 等の総評)
# 本文全体を保持。改行は \n エスケープして 1 行に収める。
if ! gh api "repos/${REPO}/pulls/${PR}/reviews" --paginate --jq \
'.[] | select(.body != null and .body != "") | "[REVIEW-BODY] [\(.user.login)] state=\(.state) \(.body | gsub("\n"; "\\n"))"'; then
(( FAIL_COUNT += 1 )) || true
fi

# 3. PR レベルコメント (Conversation タブの通常コメント)
# 本文全体を保持。改行は \n エスケープして 1 行に収める。
if ! gh api "repos/${REPO}/issues/${PR}/comments" --paginate --jq \
'.[] | "[PR-COMMENT] [\(.user.login)] \(.body // "" | gsub("\n"; "\\n"))"'; then
(( FAIL_COUNT += 1 )) || true
fi

# 全ソース失敗時のみ非 0 で終了(認証切れ等の検出)
if (( FAIL_COUNT >= 3 )); then
echo "ERROR: 全 3 ソースの取得に失敗しました" >&2
exit 1
fi
```

出力フォーマット:
- インラインコメント: `path:line [user] body`
- review body: `[REVIEW-BODY] [user] state=CHANGES_REQUESTED body`
- PR コメント: `[PR-COMMENT] [user] body`

### 2. `state.py` — 既存コメント収集を共有スクリプト呼び出しに差し替え

`init()` 内の L306-324 (インラインコメント取得 + ファイル書き出し) を `fetch-pr-comments.sh` の呼び出しに置き換え:

```python
fetch_script = Path(__file__).resolve().parent.parent.parent / "fix" / "scripts" / "fetch-pr-comments.sh"
r = subprocess.run(
[str(fetch_script), repo, str(pr)],
capture_output=True, text=True,
)
existing_path = tmp_dir / f"cross-review-pr{pr}-existing-comments.txt"
if r.returncode == 0:
existing_path.write_text(r.stdout)
else:
info(f"⚠ 既存コメント取得失敗: {r.stderr.strip()[:200]}")
existing_path.write_text("")
```

既存の `jq_filter` 変数と `subprocess.run(["gh", "api", ...])` ブロックは削除。

### 3. `fix/SKILL.md` — スクリプト参照とコマンド例の更新

gh コマンド例セクションに `fetch-pr-comments.sh` の使い方を追記:

```markdown
### PR コメント一括取得 (3 ソース)

```bash
# 共有スクリプトで全ソース一括取得
"$(dirname "$0")/scripts/fetch-pr-comments.sh" <owner/repo> <pr_number>
```

### 4. `review-pr-comments/SKILL.md` — コメント取得セクション更新

Step 2 のコメント取得で `fix/scripts/fetch-pr-comments.sh` を参照:

```markdown
### 2. PRコメント取得

fix skill の共有スクリプトで 3 ソース (インラインコメント / レビュー body / PR レベルコメント) を一括取得:

```bash
PLUGIN_DIR="$(cd "$(dirname "$0")/../.." && pwd)"
"$PLUGIN_DIR/skills/fix/scripts/fetch-pr-comments.sh" "$REPO" "$PR_NUMBER"
```

### 5. ドキュメント整合

| ファイル | 更新内容 |
|---|---|
| `cross-review/SKILL.md` (L87) | Step 4 の説明を「3 ソース (fetch-pr-comments.sh 経由)」に更新 |
| `cross-review/docs/01-state-and-review.md` (L98) | 既存コメント差分の説明を共有スクリプト参照に更新 |
| `cross-review/docs/02-fix-and-rotation.md` (L76) | fix prompt 内のコメント取得手順を共有スクリプト参照に更新 |

## 単一 PR 判定

- 変更ファイル: 7 ファイル (新規 1 + 既存 6)
- 差分: 推定 80-120 行 (共有スクリプト化で各ファイルの変更量は減少)
- すべて同一目的 (コメント取得ソースの拡張 + 共有スクリプト化) で結合度が高い
- 依存関係のある複数タスクなし

→ **単一 PR で対応**。release ブランチ不要。

## テスト計画

- [ ] `state.py` の変更後、実 PR に対して `state.py init` を実行し、`existing-comments.txt` に 3 ソースの内容が含まれることを確認
- [ ] review body にのみ指摘がある PR で `/ndf:cross-review` を実行し、指摘が検出されることを確認
- [ ] `claude plugin validate` が通ることを確認
2 changes: 1 addition & 1 deletion plugins/ndf/skills/cross-review/SKILL.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -84,7 +84,7 @@ state.json の読み書きや AI launcher 起動・完了待ちは全て委譲
| 1 | 自分の PR 判定(422 回避) | `gh api user` と `gh pr view --json author` を比較し `is_own_pr` / `event_downgrade` を state.json に書く |
| 2 | worktree 分離 | `git worktree add <worktree-base>/pr<PR> <head>` を冪等実行(`<worktree-base>` は `NDF_WORKTREE_BASE` env > `/work/worktrees` > `$HOME/work/worktrees` の優先順で解決) |
| 3 | gemini trusted directory | `launch-gemini.sh` が `GEMINI_CLI_TRUST_WORKSPACE=true` + `--skip-trust` を必ず併用。**tmp dir は `<worktree>/.cross_review/`** を採用し、gemini の workspace 制約 (workspace 外の `write_file` がブロックされる) を根本回避 |
| 4 | 既存コメント差分 | `gh api .../comments --paginate` を `$TMP_DIR/cross-review-pr<PR>-existing-comments.txt` に保存し、gemini プロンプトには **内容をインライン埋め込み**、codex プロンプトには path を渡す |
| 4 | 既存コメント差分 | `fix/scripts/fetch-pr-comments.sh` で 3 ソース (インラインコメント / レビュー body / PR レベルコメント) を一括取得し `$TMP_DIR/cross-review-pr<PR>-existing-comments.txt` に保存。gemini プロンプトには **内容をインライン埋め込み**、codex プロンプトには path を渡す |

### `<worktree-base>` の解決順

Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -95,7 +95,7 @@ cd "$WORKTREE"
1. 既存 state.json があり `final == null` なら再開
2. 自分の PR 判定(`gh api user` と `gh pr view --json author` を比較)
3. worktree 作成(`<worktree-base>/pr<PR>`。`<worktree-base>` は `NDF_WORKTREE_BASE` env > `/work/worktrees` > `$HOME/work/worktrees` の優先順で解決。実 path は state.json の `worktree_path` を参照)
4. 既存コメントスナップショット → `$TMP_DIR/cross-review-pr<PR>-existing-comments.txt`
4. 既存コメントスナップショット (`fix/scripts/fetch-pr-comments.sh` で 3 ソース一括取得) → `$TMP_DIR/cross-review-pr<PR>-existing-comments.txt`
5. state.json 書き出し

**重要**: 以降の全ステップで `cd $WORKTREE` を強制。
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -73,7 +73,7 @@ worktree 外を触ると競合します。

## 必須実行手順(順序厳守)

1. PR コメント取得: `gh api "repos/{OWNER_REPO}/pulls/{PR}/comments" --paginate`
1. PR コメント取得 (3 ソース): `fix/scripts/fetch-pr-comments.sh {OWNER_REPO}{PR}` でインラインコメント / レビュー body / PR レベルコメントを一括取得
2. 重要度を独自再判定(AI agent のラベルは参考値)
3. CI 状態スナップショット: `gh pr checks {PR} --json name,state` (**完了待ちはしない**、PENDING は無視して FAILURE のみ修正対象に取り込む)
4. critical/major + 該当 minor/nit の修正コミット(worktree 内のみ)
Expand Down
86 changes: 50 additions & 36 deletions plugins/ndf/skills/cross-review/scripts/state.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -29,6 +29,7 @@
import json
import os
import pathlib
import shlex
import subprocess
import sys
from typing import Any
Expand All@@ -50,8 +51,11 @@ def _default_worktree_base() -> pathlib.Path:
legacy = pathlib.Path("/work/worktrees")
try:
legacy.mkdir(parents=True, exist_ok=True)
# mkdir 成功 = 書き込み可能 → 既存環境互換でこちらを使う
return legacy
if not os.access(legacy, os.W_OK):
info(f"⚠ worktree ベースディレクトリに書き込み権限がありません: {legacy} — フォールバック")
else:
# mkdir 成功 + 書き込み可能 → 既存環境互換でこちらを使う
return legacy
except OSError:
pass
return pathlib.Path.home() / "work" / "worktrees"
Expand DownExpand Up@@ -223,13 +227,13 @@ def _load(pr: int) -> dict[str, Any]:
p = _state_path(pr)
if not p.exists():
die(f"state.json not found: {p}")
return json.loads(p.read_text())
return json.loads(p.read_text(encoding="utf-8"))


def _save(pr: int, state: dict[str, Any]) -> None:
p = _state_path(pr)
tmp = p.with_suffix(".json.tmp")
tmp.write_text(json.dumps(state, indent=2, ensure_ascii=False))
tmp.write_text(json.dumps(state, indent=2, ensure_ascii=False), encoding="utf-8")
tmp.replace(p)


Expand All@@ -256,7 +260,7 @@ def cmd_init(args: argparse.Namespace) -> None:
pr = args.pr
# worktree path を先に解決してから tmp_dir を決定する。
# tmp_dir は <worktree>/.cross_review/ に配置し、gemini の workspace 制約を根本回避。
worktree = args.worktree or str(_default_worktree_base() / f"pr{pr}")
worktree = str(pathlib.Path(args.worktree).resolve()) if args.worktree else str(_default_worktree_base() / f"pr{pr}")

# worktree 存在チェック用: _tmp_dir() は mkdir するため、先に呼ぶと
# worktree ディレクトリが副作用で作成され exists() が常に true になる。
Expand All@@ -265,14 +269,14 @@ def cmd_init(args: argparse.Namespace) -> None:
# 再開チェック: state ファイルの存在確認は _tmp_dir() を使わず直接パスを組む
resume_state_file = pathlib.Path(worktree) / ".cross_review" / f"cross-review-pr{pr}-state.json"
if resume_state_file.exists():
st = json.loads(resume_state_file.read_text())
st = json.loads(resume_state_file.read_text(encoding="utf-8"))
if st.get("final") is None:
tmp_dir = _tmp_dir(worktree)
wt = st.get("worktree_path") or ""
info(f"↻ 前回中断 state から再開(round={len(st.get('rounds', []))})")
print(f"PR={st['current_pr']}")
print(f"WORKTREE={wt}")
print(f"TMP_DIR={tmp_dir}")
print(f'PR={st["current_pr"]}')
print(f'WORKTREE={shlex.quote(str(wt))}')
print(f'TMP_DIR={shlex.quote(str(tmp_dir))}')
print(f"RESUMED=1")
return

Expand All@@ -288,14 +292,29 @@ def cmd_init(args: argparse.Namespace) -> None:
head_branch = _sh(["gh", "pr", "view", str(pr), "--json", "headRefName", "--jq", ".headRefName"])
base_branch = _sh(["gh", "pr", "view", str(pr), "--json", "baseRefName", "--jq", ".baseRefName"])
if not pathlib.Path(worktree).exists():
_sh(["git", "fetch", "origin", head_branch])
# head branch が既に別の worktree (例: 現在の作業ディレクトリ) で checkout されている
# 場合、`git worktree add <path> <branch>` は
# `fatal: '<branch>' is already used by worktree at '<other>'`
# で落ちる。これを避けるため、`origin/<head_branch>` を **detached** で展開する。
# cross-review はファイル参照しかしないので detached HEAD で全く問題ない。
_sh(["git", "worktree", "add", "--detach", worktree, f"origin/{head_branch}"])
info(f"✅ worktree 作成 (detached @ origin/{head_branch}): {worktree}")
# フォーク PR の場合 origin に head_branch がないことがある。
# fetch 失敗時は gh pr checkout --detach でフォールバックする。
fetch_result = subprocess.run(
["git", "fetch", "origin", head_branch],
capture_output=True, text=True,
)
if fetch_result.returncode == 0:
# head branch が既に別の worktree で checkout されている場合を避けるため
# detached で展開する。cross-review はファイル参照しかしないので問題ない。
_sh(["git", "worktree", "add", "--detach", worktree, f"origin/{head_branch}"])
info(f"✅ worktree 作成 (detached @ origin/{head_branch}): {worktree}")
else:
info(f"⚠ git fetch origin {head_branch} 失敗 (フォーク PR の可能性) — gh pr checkout でフォールバック")
_sh(["git", "worktree", "add", "--detach", worktree, "HEAD"])
# worktree 内で gh pr checkout を実行して正しいコミットに切り替え
checkout_result = subprocess.run(
["gh", "pr", "checkout", str(pr), "--detach"],
capture_output=True, text=True,
cwd=worktree,
)
if checkout_result.returncode != 0:
die(f"gh pr checkout --detach #{pr} 失敗: {checkout_result.stderr.strip()}")
info(f"✅ worktree 作成 (gh pr checkout --detach #{pr}): {worktree}")
else:
info(f"↻ 既存 worktree 流用: {worktree}")

Expand All@@ -304,24 +323,19 @@ def cmd_init(args: argparse.Namespace) -> None:
state_file = tmp_dir / f"cross-review-pr{pr}-state.json"

# 既存コメントスナップショット(重複指摘防止)。
# NOTE: `gh api --paginate` は REST のページごとに **JSON 配列が連続して** stdout に出る
# ため、`json.loads(r.stdout)` は複数ページで JSONDecodeError になり、コメントが空に
# 落ちる。`--jq '.[] | ...'` で gh CLI 側に整形させ、行単位で素直に書き出す。
# 3 ソース (インラインコメント / レビュー body / PR レベルコメント) を
# fix skill の共有スクリプトで一括取得する。
repo = _sh(["gh", "repo", "view", "--json", "nameWithOwner", "-q", ".nameWithOwner"])
jq_filter = (
r'.[] | "\(.path // "?"):\(.line // .original_line // "?") '
r'[\(.user.login)] \(.body // "" | split("\n")[0])"'
)
fetch_script = pathlib.Path(__file__).resolve().parent.parent.parent / "fix" / "scripts" / "fetch-pr-comments.sh"
Comment thread
takemi-ohama marked this conversation as resolved.
r = subprocess.run(
["gh", "api", f"repos/{repo}/pulls/{pr}/comments", "--paginate", "--jq", jq_filter],
[str(fetch_script), repo, str(pr)],
capture_output=True, text=True,
)
existing_path = tmp_dir / f"cross-review-pr{pr}-existing-comments.txt"
if r.returncode == 0:
existing_path.write_text(r.stdout)
existing_path.write_text(r.stdout, encoding="utf-8")
else:
info(f"⚠ 既存コメント取得失敗: {r.stderr.strip()[:200]}")
existing_path.write_text("")
die(f"既存コメント取得失敗 (重複検出無効のため中断): {r.stderr.strip()[:200]}")

state = {
"started_at": _now(),
Expand All@@ -342,14 +356,14 @@ def cmd_init(args: argparse.Namespace) -> None:
"deferred_nits": [],
"final": None,
}
state_file.write_text(json.dumps(state, indent=2, ensure_ascii=False))
state_file.write_text(json.dumps(state, indent=2, ensure_ascii=False), encoding="utf-8")
info(f"✅ state 初期化: {state_file}")
print(f"PR={pr}")
print(f"WORKTREE={worktree}")
print(f"TMP_DIR={tmp_dir}")
print(f"REPO={repo}")
print(f"HEAD_BRANCH={head_branch}")
print(f"BASE_BRANCH={base_branch}")
print(f'WORKTREE={shlex.quote(str(worktree))}')
print(f'TMP_DIR={shlex.quote(str(tmp_dir))}')
print(f'REPO={shlex.quote(str(repo))}')
print(f'HEAD_BRANCH={shlex.quote(str(head_branch))}')
print(f'BASE_BRANCH={shlex.quote(str(base_branch))}')
print(f"IS_OWN_PR={'1' if is_own else '0'}")
print(f"EVENT_DOWNGRADE={'1' if event_downgrade else '0'}")
print("RESUMED=0")
Expand DownExpand Up@@ -395,7 +409,7 @@ def cmd_read_result(args: argparse.Namespace) -> None:
die(f"{agent}: result 未生成 ({rfile})")

try:
r = json.loads(rfile.read_text())
r = json.loads(rfile.read_text(encoding="utf-8"))
except json.JSONDecodeError as exc:
die(f"{agent}: result.json の parse に失敗 ({rfile}): {exc}", code=3)

Expand DownExpand Up@@ -504,7 +518,7 @@ def collect_keys(round_no: int) -> set[str]:
if not p.exists():
continue
try:
payload = json.loads(p.read_text())
payload = json.loads(p.read_text(encoding="utf-8"))
except json.JSONDecodeError:
continue
# gemini round 4 指摘: payload は本来 dict (comments: [...]) だが、
Expand Down
Loading