Skip to content

Fix: PRコメント取得を3ソース対応に拡張し取得漏れを解消 (#13) - #15

Merged
takemi-ohama merged 11 commits into
mainfrom
fix/PLAN13-fetch-pr-comments-3sources
May 25, 2026
Merged

Fix: PRコメント取得を3ソース対応に拡張し取得漏れを解消 (#13)#15
takemi-ohama merged 11 commits into
mainfrom
fix/PLAN13-fetch-pr-comments-3sources

Conversation

@takemi-ohama

Copy link
Copy Markdown
Contributor

Summary

  • GitHub PR のコメント取得がインラインコメントのみだった問題を修正し、レビュー総評 (review body)PR レベルコメント (Conversation タブ) を追加取得する 3 ソース対応を実装
  • 3 ソース一括取得の共有スクリプト fetch-pr-comments.sh を新規作成し、cross-review / fix / review-pr-comments の 3 skill で共用化
  • state.py のインライン gh api 呼び出しをスクリプト呼び出しに差し替え、コメント本文の全文保持・安全なシェル出力 (shlex.quote) ・堅牢なエラーハンドリング (die() による即時中断) を実装
  • SKILL.md / docs の手順を共有スクリプト参照に統一
  • 実装計画ドキュメント (PLAN13) を追加

Changed files

ファイル変更内容
plugins/ndf/skills/fix/scripts/fetch-pr-comments.sh新規 — 3 ソース一括取得スクリプト。各ソース個別の失敗を許容しつつ、全ソース失敗時は非 0 で終了。コメント本文の改行エスケープとバッククォートフェンスの無害化を含む
plugins/ndf/skills/cross-review/scripts/state.pyインライン gh api → 共有スクリプト呼び出しに差し替え。shlex.quote による eval 出力の安全化、Path import 修正、取得失敗時の die() 中断を追加
plugins/ndf/skills/fix/SKILL.md共有スクリプトの使い方セクション追加、パス解決を $CLAUDE_PLUGIN_ROOT に修正
plugins/ndf/skills/review-pr-comments/SKILL.mdコメント取得手順を共有スクリプト参照に変更
plugins/ndf/skills/cross-review/SKILL.md3 ソース対応の説明に更新
plugins/ndf/skills/cross-review/docs/01-state-and-review.mdinit 手順を共有スクリプト参照に更新
plugins/ndf/skills/cross-review/docs/02-fix-and-rotation.mdfix 必須手順を共有スクリプト参照に更新
issues/PLAN13_cross-review-review-body-missing.md新規 — 実装計画ドキュメント

Test plan

  • fetch-pr-comments.sh <owner/repo> <pr_number> を実 PR に対して実行し、3 ソース (インライン / [REVIEW-BODY] / [PR-COMMENT]) の出力が含まれることを確認
  • state.py init <pr> 実行後、existing-comments.txt に 3 ソースの内容が含まれることを確認
  • review body にのみ指摘がある PR で /ndf:cross-review を実行し、指摘が正しく検出されることを確認
  • fetch-pr-comments.sh に不正な引数を渡した場合、適切なエラーメッセージで終了することを確認
  • claude plugin validate が通ることを確認

Closes#13

takemi-ohamaand others added 8 commits May 25, 2026 02:36
GitHub PR の review body と PR レベルコメントが取得されず、
人間レビュアーの指摘が cross-review ループで検出されない問題を修正。
共有スクリプト fetch-pr-comments.sh を fix skill に配置し、
cross-review / fix / review-pr-comments の 3 skill で共用する。
Closes#13
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- state.py: Path → pathlib.Path に修正 (NameError 解消)
- fetch-pr-comments.sh: review body の改行を \n エスケープして全文保持
- fetch-pr-comments.sh: 全ソース取得失敗時のみ非0終了 (0件取得と取得失敗の区別)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PRレベルコメント・インラインコメントも review body と同様に
gsub("\n"; "\\n") で全文を1行に保持するよう統一。
split("\n")[0] による先頭行切り詰めを排除し、
fix/review-pr-comments が複数行の指摘を見落とさないようにする。
fetch-pr-comments.shが全ソース失敗で非0を返した場合、
空スナップショットで黙って継続せずdie()でinitを失敗させる。
重複検出が無効な状態でレビューを進めないようにする。
- PLAN13 サンプルコードを実装済みの gsub + 失敗カウント方式に更新 (codex minor)
- args.worktree を .resolve() で絶対パスに変換 (gemini major)
- フォーク PR の fetch 失敗時に gh pr checkout --detach でフォールバック (gemini major)
- KEY=VALUE 出力のパス・ブランチ名をダブルクォートで囲む (gemini major)
- worktree ベースディレクトリの書き込み権限チェック追加 (gemini minor)
- read_text() / write_text() 全箇所に encoding="utf-8" を明示指定 (gemini minor)
…minor 2件)
- state.py: gh pr checkout --detach の戻り値チェック追加、失敗時 die() で中断
- state.py: _default_worktree_base の os.access 失敗時に die() せずフォールバック継続
- fetch-pr-comments.sh: 引数 (REPO, PR) の存在チェック追加
- fetch-pr-comments.sh: 各 gh api 失敗時に stderr へ個別警告メッセージ出力
Bash tool 実行時に $0 はシェル自体を指すため、SKILL.md からの
相対パス解決に使えない。Claude Code プラグインが提供する
$CLAUDE_PLUGIN_ROOT 環境変数を使用するよう修正。
対象:
- plugins/ndf/skills/fix/SKILL.md
- plugins/ndf/skills/review-pr-comments/SKILL.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- state.py: eval "$(state.py init ...)" で取り込む KEY=VALUE 出力のパス・文字列値を
shlex.quote() で囲み、パスに特殊文字 ($, ", 改行等) が含まれる場合のコマンド注入・
構文破壊を防止 (WORKTREE, TMP_DIR, REPO, HEAD_BRANCH, BASE_BRANCH)
- fetch-pr-comments.sh: コメント本文中の ``` (3連バッククォート) を ` ` ` に置換し、
launch-gemini.sh の Markdown フェンスが本文中のコードブロックで閉じられる問題を防止
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 6 | codex | APPROVE

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

@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.

Test review

Comment threadplugins/ndf/skills/cross-review/scripts/state.py

@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.

Test review 2

Comment threadplugins/ndf/skills/fix/scripts/fetch-pr-comments.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 6 | gemini | REQUEST_CHANGES

PR #15 の修正を確認しました。既存の NameError や body 切り詰めは解消されていますが、状態の永続性と再開時の動作に関して数点改善案があります。

その他の指摘

  • [minor / 信頼性] state.pycmd_merge_fix 等の各サブコマンドで _tmp_dir() を再計算していますが、これはカレントディレクトリや環境変数に依存するため、init 時と異なるパスを返すリスクがあります。init 時に st["tmp_dir"] に保存した値を優先的に使用するように変更し、パス解決の整合性を担保してください。

Comment threadplugins/ndf/skills/cross-review/scripts/state.py
Comment threadplugins/ndf/skills/fix/scripts/fetch-pr-comments.sh Outdated
1. 再開時の変数出力不足: RESUMED=1 パスで REPO, HEAD_BRANCH,
BASE_BRANCH, IS_OWN_PR, EVENT_DOWNGRADE を state.json から
読み出して stdout に出力するよう修正
2. gh api --jq の raw output 保証: --jq を -q + jq -r パイプに
変更し、環境差による引用符混入リスクを排除
3. _tmp_dir() の再計算リスク: _resolve_tmp_dir(pr) を導入し、
state.json に保存済みの tmp_dir を優先使用。各コマンドの
パス解決を init 時確定値に統一
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 1 | codex | REQUEST_CHANGES

fetch-pr-comments.sh が現状のままだと全ソース取得に失敗し、state.py init が既存コメントスナップショット作成で中断します。gh api 呼び出しの -q を削除するか、各コマンドで有効な --jq 引数を渡してください。

Comment threadplugins/ndf/skills/fix/scripts/fetch-pr-comments.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 | gemini | REQUEST_CHANGES

PR #15 において、中断状態からの再開時に必要な変数の出力不足の修正、および fetch-pr-comments.sh での raw string 出力対応が行われたことを確認しました。これらは前回の指摘を適切に反映しています。

一方で、新たに追加された _resolve_tmp_dir のパス解決ロジックに不整合があり、state.json の読み込み元と参照先が乖離する懸念があります。また、パス解決関数内でのディレクトリ作成(副作用)や、_tmp_dir における workspace 引数の優先順位(環境変数が優先される点)など、いくつか修正・改善が望ましい点が見つかりました。

Comment threadplugins/ndf/skills/cross-review/scripts/state.py
Comment threadplugins/ndf/skills/cross-review/scripts/state.py Outdated
Comment threadplugins/ndf/skills/cross-review/scripts/state.py Outdated
1. [major] fetch-pr-comments.sh: gh api の -q フラグ(引数必須)を削除し、
raw JSON を jq -r へ直接パイプする形に修正
2. [major] state.py: _state_path() が _resolve_tmp_dir() を使うと
state.json の読み込み元と参照先が乖離する問題を修正。
state ファイル自体のパス解決には _tmp_dir() を直接使用
3. [minor] state.py: _resolve_tmp_dir() 内の p.mkdir() 副作用を削除。
パス解決関数でディレクトリ作成しないよう修正
4. [minor] state.py: st.get("repo", "") 等を st.get("repo") or "" に変更し、
値が None の場合に "None" 文字列になる問題を防止
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 2 | codex | APPROVE

修正が必要な追加指摘はありません。

@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

CROSS_REVIEW_TMP_DIR を使用した際の再開検知漏れおよび内部関数の副作用に関する修正を推奨します。

Comment threadplugins/ndf/skills/cross-review/scripts/state.py
Comment threadplugins/ndf/skills/cross-review/scripts/state.py
…キュメント修正
- cmd_initの再開チェックでCROSS_REVIEW_TMP_DIR環境変数が設定されている場合に
そちらを参照するよう修正。カスタムディレクトリ使用時にstateファイルを
見落として上書きするリスクを解消。
- _resolve_tmp_dirのdocstringを実際の挙動に合わせ、_tmp_dir()経由で
mkdirの副作用が発生する可能性があることを明記。
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 #15 の変更内容を確認しました。fetch-pr-comments.sh によるコメント取得ソースの拡張と、state.py における再開時の状態復元および worktree 作成の堅牢性向上が適切に実装されています。設計方針(PLAN13)に沿った一貫性のある修正であり、技術的な懸念点も解消されているため、承認します。

@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 | APPROVE

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

@takemi-ohama
takemi-ohama merged commit bf576a7 into mainMay 25, 2026
@takemi-ohama
takemi-ohama deleted the fix/PLAN13-fetch-pr-comments-3sources branch August 14, 2026 04:00
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.

cross-review: PR review body の指摘を見落とす

1 participant

@takemi-ohama