Skip to content

fix: PRコメント取得を3ソース対応に拡張 (#13) - #14

Closed
takemi-ohama wants to merge 8 commits into
mainfrom
fix/PLAN13-fetch-pr-comments-3sources
Closed

fix: PRコメント取得を3ソース対応に拡張 (#13)#14
takemi-ohama wants to merge 8 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ソース一括取得の共有スクリプト fix/scripts/fetch-pr-comments.sh を新規作成し、cross-review / fix / review-pr-comments の3 skill で共用
  • state.py のインライン gh api 呼び出しをスクリプト呼び出しに差し替え

背景

carmo-system-console PR #14137 で人間レビュアーが review body にのみ書いた CHANGES_REQUESTED 指摘が cross-review 6ラウンド通じて検出されなかった。

変更ファイル

ファイル変更内容
fix/scripts/fetch-pr-comments.sh新規 — 3ソース一括取得スクリプト (`
cross-review/scripts/state.pyインライン gh api → スクリプト呼び出しに差し替え
fix/SKILL.mdスクリプト使い方セクション追加
review-pr-comments/SKILL.mdStep 2 を共有スクリプト参照に変更
cross-review/SKILL.mdStep 4 の説明を3ソース対応に更新
cross-review/docs/01-state-and-review.mdinit 手順 4 を共有スクリプト参照に更新
cross-review/docs/02-fix-and-rotation.mdfix 必須手順 1 を共有スクリプト参照に更新

Test plan

  • fetch-pr-comments.sh を実PRに対して実行し、3ソースの出力 (インライン / [REVIEW-BODY] / [PR-COMMENT]) が含まれることを確認
  • state.py init 実行後、existing-comments.txt に3ソースの内容が含まれることを確認
  • review body にのみ指摘がある PR で /ndf:cross-review を実行し、指摘が検出されることを確認
  • claude plugin validate が通ることを確認 ✅

Closes#13

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>

@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

設計レベルでは、3ソース取得を共有化する方向は妥当ですが、現在の実装では初期化処理が実行時例外で止まる点と、review body の本文を取りこぼす点を修正する必要があります。

Comment threadplugins/ndf/skills/cross-review/scripts/state.py Outdated
Comment threadplugins/ndf/skills/fix/scripts/fetch-pr-comments.sh Outdated
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 コメントの取得ソースを拡張し、指摘見落としを防止する改善を確認しました。共有スクリプト化による設計も適切です。state.py で Path が直接参照されている箇所があり、NameError が発生するため修正が必要です。

Comment threadplugins/ndf/skills/cross-review/scripts/state.py Outdated
- 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>
@takemi-ohama

Copy link
Copy Markdown
ContributorAuthor

🔧 /ndf:fix サマリ (round 1)

対応件数: critical=1 / major=2 / minor=0 (合計 3 件、うち重複 1 件で実質 3 指摘)
deferred: 0 件 / rejected: 0 件
commit: 90025cd
CI: NONE (CI チェック未設定)

詳細

#severitypath対応thread
1criticalstate.py:310Path(...)pathlib.Path(...) に修正 (NameError 解消)Resolved
2majorfetch-pr-comments.sh:17review body の jq を split("\n")[0]gsub("\n"; "\\n") に変更し全文保持Resolved
3majorfetch-pr-comments.sh:13|| true 撤去、失敗カウントで全ソース失敗時のみ非0終了Resolved
4critical (重複)state.py:310#1 と同一。修正済みResolved

@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

PR レベルコメントの本文切り詰めにより、Conversation タブに書かれた複数行の指摘を引き続き見落とすため、3 ソース取得の出力粒度を揃えてください。

Comment threadplugins/ndf/skills/fix/scripts/fetch-pr-comments.sh Outdated
PRレベルコメント・インラインコメントも review body と同様に
gsub("\n"; "\\n") で全文を1行に保持するよう統一。
split("\n")[0] による先頭行切り詰めを排除し、
fix/review-pr-comments が複数行の指摘を見落とさないようにする。
@takemi-ohama

Copy link
Copy Markdown
ContributorAuthor

Fix Summary (Round 2/5)

対応結果

重要度件数対応
critical0-
major1修正済み
minor0-
nit0-

修正内容

commit 52a4685: 全 3 ソースのコメント本文を全文保持に統一

  • fetch-pr-comments.sh の PR レベルコメント (L28) とインラインコメント (L15) で split("\n")[0]gsub("\n"; "\\n") に変更
  • review body (L22) では前回修正で gsub 済みだったが、他 2 ソースに同じ修正が漏れていた
  • これにより /ndf:fix/ndf:review-pr-comments が複数行の指摘を見落とさなくなる

CI 状態

CI チェックなし(リポジトリに CI ワークフロー未設定)

Deferred / Rejected

なし

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

全 3 ソース (インライン / レビュー body / PR レベルコメント) からのコメント取得と全文保持が正しく実装されており、既存の指摘事項にも全て対応されています。

@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

既存コメント取得の全ソース失敗時は、空スナップショットで継続せず init を失敗させる必要があります。そうしないと重複指摘防止が無効化されたまま cross-review が進みます。

Comment threadplugins/ndf/skills/cross-review/scripts/state.py
fetch-pr-comments.shが全ソース失敗で非0を返した場合、
空スナップショットで黙って継続せずdie()でinitを失敗させる。
重複検出が無効な状態でレビューを進めないようにする。

@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

追加されたPLAN文書内の実装例が最終実装と矛盾しているため、後続作業で同じ不具合を再導入しないよう修正してください。

Comment threadissues/PLAN13_cross-review-review-body-missing.md 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 | gemini | REQUEST_CHANGES

Round 2 レビューです。既存の指摘事項は修正されていますが、自動化ツールとしての堅牢性を高めるため、以下の点について再考をお願いします。

修正が必要な点

  • [major / 正確性] worktree パス解決のポータビリティ: state.py init において args.worktree が相対パスで指定された場合、保存された state を別ディレクトリから利用するとパスが壊れます。.resolve() で絶対パスに変換して保持してください。
  • [major / 正確性] フォーク PR への対応: 現在 origin に head ブランチがあることを前提としていますが、外部からの PR (fork) の場合 origin にはブランチが存在せず fetch が失敗します。gh pr view から得られる headRepository 情報を利用して適切なリモートから取得するようにしてください。
  • [major / 信頼性] シェル出力のエスケープ: PR=WORKTREE= 等の出力は eval される想定ですが、パス等にスペースが含まれるとシェルで構文エラーになります。値をダブルクォートで囲むか、エスケープして出力してください。
  • [minor / 信頼性] 書き込み権限チェックの厳密化: _default_worktree_basemkdir(exist_ok=True) を使っていますが、ディレクトリが既に存在する場合、現在のユーザーに書き込み権限があるか保証されません。os.access(..., os.W_OK) 等でのチェック追加を検討してください。
  • [minor / 信頼性] ファイル保存時のエンコーディング: _save 等での write_text において、encoding="utf-8" を明示的に指定することを推奨します。

- 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)

@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

フォーク PR フォールバックで checkout 失敗を検出せずに誤った worktree でレビューが進む経路があります。

Comment threadplugins/ndf/skills/cross-review/scripts/state.py 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 3 | gemini | REQUEST_CHANGES

共有スクリプト化によりコメント取得が強化されましたが、state.py のフォールバックロジックにバグがあり、特定の権限構成下で動作が停止する可能性があります。また、新設されたシェルの堅牢性とエラー報告について改善を推奨します。

Comment threadplugins/ndf/skills/cross-review/scripts/state.py Outdated
Comment threadplugins/ndf/skills/fix/scripts/fetch-pr-comments.sh
Comment threadplugins/ndf/skills/fix/scripts/fetch-pr-comments.sh Outdated
…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 へ個別警告メッセージ出力

@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

指摘事項はすべて解消されました。状態管理スクリプトの堅牢性向上およびコメント取得スクリプトの改善を確認しました。

@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

SKILL.md 内の共有スクリプト呼び出し例が $0SKILL.md の場所として扱っており、実際の Bash 実行コンテキストでは存在しないパスを生成します。2 箇所とも repo root 基準など実行環境に依存しないパス解決へ揃えてください。

Comment threadplugins/ndf/skills/fix/SKILL.md Outdated
Comment threadplugins/ndf/skills/review-pr-comments/SKILL.md Outdated
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>

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

3ソース(インラインコメント、レビューbody、PRレベルコメント)の一括取得への移行と、それによる指摘見落としの防止が適切に実装されています。

  • fetch-pr-comments.sh による共通スクリプト化により、保守性が向上しています。
  • gsub("\n"; "\\n") を用いた本文全体の保持により、箇条書きなどの重要な指摘の見落としが解消されています。
  • state.py における pathlib のインポート修正や UTF-8 指定、フォーク PR への対応など、堅牢性が向上しています。

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

修正が必要な箇所を2件コメントしました。

Comment threadplugins/ndf/skills/cross-review/scripts/state.py Outdated
Comment threadplugins/ndf/skills/fix/scripts/fetch-pr-comments.sh Outdated
- 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-ohama

Copy link
Copy Markdown
ContributorAuthor

ℹ️ レビューコメント履歴整理のため本 PR を一度 close し、同じブランチ fix/PLAN13-fetch-pr-comments-3sources で新 PR を作り直します。ブランチの内容・base は変えません。

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