Skip to content

Fix: 投稿できていないレビューを承認として扱わない(v8.5.3) - #134

Merged
takemi-ohama merged 2 commits into
mainfrom
fix/cross-refactoring-review-post-verification
Aug 22, 2026
Merged

Fix: 投稿できていないレビューを承認として扱わない(v8.5.3)#134
takemi-ohama merged 2 commits into
mainfrom
fix/cross-refactoring-review-post-verification

Conversation

@takemi-ohama

Copy link
Copy Markdown
Contributor

何のために

5 回目の実機試行(#132)で残した不具合 19 と 20 を直す。

投稿は AI 自身が gh api で行うため、失敗しても結果ファイルの判定だけは残る。そのまま
採ると、実装担当が読むべき指摘が Pull Request に無いまま収束する。実測では、ラウンド 1 の
承認 2 件が GitHub 上に痕跡を持たないまま採用の確定に使われていた。

$ gh api repos/devbasex/ai-plugins/pulls/131/reviews \
--jq '.[] | "\(.user.login) \(.state) \(.submitted_at)"'
takemi-ohama COMMENTED 2026-08-21T04:15:29Z
takemi-ohama COMMENTED 2026-08-21T04:18:52Z
takemi-ohama COMMENTED 2026-08-21T04:19:04Z

3 件はいずれもラウンド 2 のもので、ラウンド 1 のレビューは 1 件も残っていない。

もう一方は、投稿に失敗したレビュー担当が結果ファイルを書かずに終わる点である。プロンプトの
gh api が失敗したら即座に終了する」が結果ファイルの書き出しより優先されるため、進行側
からは「レビュー担当が動かなかった」と区別が付かない。

何を

対象変更
scripts/refactor.pyjudge()review_url を必須とし、post_error を持つ結果を差し戻す。_unposted_reviewers() が URL の識別子から GitHub 側の存在を確かめる。上限に達したときは投稿できなかった担当も中断の対象へ含める
prompts/review.md投稿に失敗したときも post_error 付きの結果ファイルを書かせる。HTTP 422 のときの投稿し直しの手順も添えた
docs/02-apply-and-review.md投稿の確認の仕様を追加
SKILL.md設計方針に「投稿の確認」を追加、アンチパターンに 1 件追加

投稿の確認

結果ファイルの申告GitHub 側扱い
post_error あり見に行かない投稿できていない。差し戻す
review_url なし見に行かない投稿できていない。差し戻す
review_url ありある採用する
review_url あり無い差し戻す
review_url あり取得できない申告を採用し、確認できなかったことを出力へ残す

「無い」と「取得できない」を区別する。 取得の失敗で止めると、GitHub 側の一時的な不調で
進行が進まなくなる。cross-review が v8.5.0 で入れた突き合わせと同じ考え方である。

Test plan

  • plugins/ndf-shared/skills/cross-refactoring/tests/test_judge_review.py に 6 件追加
    • 投稿されていない判定を差し戻す
    • 投稿の失敗を申告した判定を差し戻す
    • 投稿できないまま上限に達したら中断する
    • GitHub 側に無いレビューを差し戻す
    • GitHub 側を確かめられないときは申告を採る
    • レビュー URL から識別子を取り出す
  • uv run --with pytest python -m pytest plugins/ndf-shared/skills/cross-refactoring/tests plugins/ndf-shared/skills/cross-review/tests -q476 passed(追加前 470)
  • claude plugin validate が marketplace / ndf-claude とも通る
  • python3 scripts/check-skill-frontmatter.py がエラー 0 件 / 警告 0 件

投稿は AI 自身が gh api で行うため、失敗しても結果ファイルの判定だけは残る。
そのまま採ると、実装担当が読むべき指摘が Pull Request に無いまま収束する。
5 回目の実機試行では、ラウンド 1 の承認 2 件が GitHub 上に痕跡を持たないまま
採用の確定に使われていた。
judge-review が結果ファイルの review_url を必須とし、URL の識別子から
repos/<repo>/pulls/<PR>/reviews/<id> の存在を確かめる。無ければ差し戻し、
取得できないときは申告を採用して確認できなかったことを出力へ残す。「無い」と
「取得できない」を区別しないと、GitHub 側の一時的な不調で進行が止まる。
あわせてレビュープロンプトが、投稿に失敗したときも post_error 付きの結果
ファイルを書かせる。書かずに終わると、進行側からは「レビュー担当が動かな
かった」と区別が付かない。上限に達したときは投稿できなかった担当も中断の
対象へ含める。
Co-Authored-By: Claude Opus 5 <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

投稿確認が一時失敗した結果を再利用する経路に修正が必要です。

Comment threadplugins/ndf-shared/skills/cross-refactoring/scripts/refactor.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 1 | gemini | APPROVE

設計と実装の両面において、レビューの投稿漏れを確実に防ぐ堅牢な仕組みが追加されていることを確認しました。
特に、GitHub API の一時的な不調等による情報取得の失敗時にも進行を止めないよう、申告ベースへのフォールバックが設けられている点は優れた設計です。
テストも網羅的であり、ロジックに懸念点はありません。

投稿の有無は結果ファイルの内容では決まらず GitHub 側の状態で決まる。
鍵に含めずに判定を再生すると、投稿が見えるようになった後で同じコマンドを
叩き直しても差し戻し(exit 3)を返し続け、`invalid_reviews` も進まないため
中断の上限にも到達せず進行が止まる。
`_unposted_reviewers()` を判定済みの照合より先に呼び、投稿できていない担当の
一覧を鍵へ混ぜる。GitHub 側の状態が変われば鍵も変わり、判定をやり直す。
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UjJqoXkDs3LhhK4aFhh84p
@takemi-ohama

Copy link
Copy Markdown
ContributorAuthor

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

対応件数: critical=0 / major=1 / minor=0 / nit=0 (合計 1 件)
deferred: 0 件 / rejected: 0 件
commit: b45ef9b
CI: SUCCESS(修正前スナップショット。再実行の完了は待っていません)

詳細

#指摘元ファイル重要度(再判定)対応
1codexplugins/ndf-shared/skills/cross-refactoring/scripts/refactor.pymajor修正済み・Resolve 済み

指摘: 投稿の確認で差し戻した(exit 3)あと、同じ結果ファイルで叩き直すと review_key の再生で GitHub を再確認せず exit 3 を返し続ける。

再判定: major のまま採用しました。再生の経路では invalid_reviews が増えないため、中断の上限(MAX_INVALID_REVIEWS)にも到達しません。GitHub 側に反映された後も進行が止まり続ける経路です。

修正: _unposted_reviewers() を判定済みの照合より先に呼び、投稿できていない担当の一覧を digest へ混ぜました。GitHub 側の状態が変われば鍵も変わり、判定をやり直します。

テスト: 先に失敗するテストを追加(test_posting_check_is_redone_when_github_state_changes)。_posted_review_stateFalseTrue に変わったとき、2 回目で承認まで進むことを確認します。

検証

  • uv run --with pytest python -m pytest .../cross-refactoring/tests .../cross-review/tests -q477 passed(従来 476 + 新規 1)
  • bash scripts/build-runtime-plugins.sh → 3 ランタイムへ同期済み(同一コミットに同梱)
  • python3 scripts/check-skill-frontmatter.py → frontmatter 合計 11112 / 11200 文字
  • claude plugin validate . / claude plugin validate plugins/ndf-claude → いずれも passed

gemini は APPROVE(指摘 0 件)でした。

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

投稿の確実な反映と失敗時の停止処理について、変更の意図通りに実装されており問題ありません。懸念点はなく、追加の修正提案はありません。

@takemi-ohama
takemi-ohama merged commit 2c986d5 into mainAug 22, 2026
7 checks passed
@takemi-ohama
takemi-ohama deleted the fix/cross-refactoring-review-post-verification branch August 22, 2026 02:29
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