docs: レビューフローの見直し(Draft先行 + CodeRabbit導入) - #35
Conversation
PR #18〜#32の実績分析(Claude/Copilotの指摘重複率、Copilotのクレジット 消費)を踏まえた運用変更。 - PRはまずDraftで作成し、Claude/Codex/CodeRabbit(無料)と反復してから Ready化してCopilot(有料)の最終レビューを受ける運用をCLAUDE.mdに明記 (Copilotのrulesetは既にreview_draft_pull_requests: falseのため ruleset自体の変更は不要だった) - .coderabbit.yamlを追加。デフォルトでdraftをスキップする設定を drafts: trueで上書きし、CLAUDE.mdの絶対ルール・RLS・test/dbの 観点をpath_instructionsで明示 - docs/roadmap.mdの「保留: 外部アカウント待ち」にCodeRabbit導入を追記 - docs/permissions.mdに、INSERT/UPDATE/DELETEのRETURNINGがSELECTポリシー で空になるだけでは「弾かれた」証明にならない、という#26で見つかった 罠を追記
📝 WalkthroughWalkthroughCodeRabbitの自動レビュー設定とPR運用を更新した。RLS検証では、 Changesレビュー運用とRLS検証
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
レビュー総評コード変更はなく、 良い点
指摘(インラインコメント参照)
見送った/機械的に指摘しなかった点
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.coderabbit.yaml:
- Around line 5-10: Update the auto-review policy in .coderabbit.yaml under
reviews.auto_review by adding auto_pause_after_reviewed_commits set to 0 if you
want CodeRabbit to keep reviewing every push after Ready, or otherwise align the
documented workflow in CLAUDE.md under the “Ready後はpushのたびに再レビューが走る” guidance so
it explicitly mentions the pause after 5 reviewed commits and the need to run
`@coderabbitai` review after that; use the existing auto_review and Ready後
workflow symbols as the place to make the policy consistent.
- Around line 30-32: Update the RLS negative-test guidance in the configuration
to require operation-specific assertions using an authenticated user client, not
service_role: verify UPDATE preserves the target row’s old values, DELETE leaves
the target row present, and INSERT does not create a new row, all from a role
that can view the target rows.
- Around line 23-25: RLSポリシー内の自己参照サブクエリをSECURITY
DEFINER関数でラップする場合の安全条件を指示に追加し、非公開スキーマへの配置、SET search_path =
''または安全な固定値の指定、すべての参照でのスキーマ修飾を必須にする。さらにPUBLICおよび不要なロールからEXECUTEをREVOKEし、必要なロールのみにGRANTするよう明記する。
- Around line 7-10: Align the CodeRabbit draft-review policy with the documented
workflow: update .coderabbit.yaml#7-10 to match the intended behavior for Draft
PRs, and update CLAUDE.md#52-56 so the “Ready化後にCodeRabbitが最終レビュー” guidance
matches that same setting. Use the existing draft-review configuration symbol
`drafts` as the source of truth, and keep both documents consistent about
whether Draft PRs are reviewed or only Ready PRs are reviewed.
In `@docs/permissions.md`:
- Line 92: Update the code fence at line 92 in the permissions documentation to
specify the TypeScript language identifier, changing the opening fence to
```typescript so the MD040 markdownlint warning is resolved.
- Around line 86-90: docs/permissions.md の `RETURNING` と RLS の説明を修正し、USING
句が壊れていても更新・削除できるという説明を削除してください。空の結果だけでは WHERE 条件不一致と SELECT または USING
ポリシーによる抑止を区別できないことを明記してください。対象行を閲覧できるユーザー視点で、更新後の値または削除後の存在を確認する手順は維持してください。
In `@docs/roadmap.md`:
- Line 89: CodeRabbit無料プランのレート制限の記載を実際の仕様に更新してください。docs/roadmap.md:89
のCodeRabbit導入項目はPRを1レビュー/時/開発者、IDE/CLIを3レビュー/時/開発者と記載し、.coderabbit.yaml:7-10
の「無料枠」コメントも同じ制限に修正してください。Draftレビューは契約プランのレート制限対象であることを反映してください。
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: db5af572-c7ad-4ad2-90cc-f07ccca6bd01
📒 Files selected for processing (4)
.coderabbit.yamlCLAUDE.mddocs/permissions.mddocs/roadmap.md
- auto_pause_after_reviewed_commits: 0 を追加(既定5コミットで自動レビューが 止まる設定は反復push運用と相性が悪い) - CLAUDE.mdの記述を.coderabbit.yaml(drafts: true)と一致させる。 「Copilot(導入後はCodeRabbitも)の最終レビュー」という誤った表現を修正し、 CodeRabbitもDraft中から反復レビューする旨を明記 - CodeRabbit Freeプランの実際のレート制限(GitHub連携PRレビューは1回/時/開発者) をCLAUDE.md・docs/roadmap.md・.coderabbit.yamlに反映。前回調査した 「200ファイル/時・4レビュー/時」は誤りだった(CodeRabbit自身のレビュー コメントがPostgreSQL公式ドキュメントとCodeRabbit公式docsをその場で クロールして訂正) - docs/permissions.mdのRETURNING説明をPostgreSQL公式ドキュメントに基づき 修正。UPDATEはSELECTポリシー不一致だと実際はエラーになり(空配列ではない)、 DELETEはSELECTポリシーを満たさない行がそもそも削除候補から除外される、 というのが正確な挙動 - markdownlint MD040(コードフェンスの言語識別子)を修正 - .coderabbit.yamlのpath_instructionsを強化(security definerの安全条件、 操作別のRLS否定側テスト観点)
レビュー結果(ドキュメント・設定ファイルのみの変更)対象は 確認した観点と結果
確認できなかった点(参考情報)
総評ブロッキングな指摘なし。ドキュメントの技術的主張は公式ドキュメントで裏取りが取れており、既存のテストコード( |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.coderabbit.yaml (1)
26-26: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win型の導出対象を限定してください。
型を手書きせずは、すべてのTypeScript型をsupabase/types.tsまたはz.inferから作る指示です。これはCLAUDE.mdの対象範囲より広く、ドメイン型やUI型まで生成型に結合させます。DB行型と外部入力型に限定してください。修正例
- - 型を手書きせず、生成型(supabase/types.ts)かZodスキーマの z.infer から導出しているか + - DBの行型は生成型(supabase/types.ts)から、外部入力型はZodスキーマの z.infer から導出しているか🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.coderabbit.yaml at line 26, Update the review rule in the type-derivation entry to limit its scope to database row types and external input types. Require Supabase generated types or z.infer only for those categories, rather than implying that all TypeScript, domain, and UI types must be derived from them.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLAUDE.md`:
- Around line 53-56:
CLAUDE.mdの該当するレビュー実行条件を、各ツールが常にpushごとに実行されるという断定から条件付きの記述へ更新してください。CodexはOPENAI_API_KEYなど必要なシークレットが設定されている場合のみ実行され、CodeRabbitはレート制限によりスキップされる可能性があることを明記し、ClaudeとCopilotの既存条件およびdraft時の挙動は維持してください。
In `@docs/permissions.md`:
- Around line 78-84: 標準の Supabase client の戻り値契約に合わせ、asUserA と asUserB の
expenses.select() 結果から data 配列を取り出して行数を検証するよう例を更新してください。両者が data-only wrapper
である場合は、戻り値が配列である契約を例内で明記し、応答オブジェクト自体に toHaveLength を適用しないでください。
- Around line 86-95: Revise the UPDATE/DELETE guidance in the documented policy
behavior to limit SELECT-policy requirements to operations that read from the
target table, specifically when WHERE or RETURNING references target-table
columns. Preserve the existing distinction between no matching rows and rows
filtered by USING/visibility, without implying that every UPDATE/DELETE
automatically requires SELECT visibility.
---
Outside diff comments:
In @.coderabbit.yaml:
- Line 26: Update the review rule in the type-derivation entry to limit its
scope to database row types and external input types. Require Supabase generated
types or z.infer only for those categories, rather than implying that all
TypeScript, domain, and UI types must be derived from them.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5526f209-3ff7-4103-a90d-d7d2e539856e
📒 Files selected for processing (4)
.coderabbit.yamlCLAUDE.mddocs/permissions.mddocs/roadmap.md
- CLAUDE.md: レビュー実行条件をツールごとに正確化(Codexはシークレット設定時、CodeRabbitはFreeプランのレート制限対象) - docs/permissions.md: select()のコード例をdata/errorの分解に修正、UPDATE/DELETEのSELECTポリシー要求条件を限定
CodeRabbit 2回目レビューの指摘分類 (commit fe445e8)前回コミット(4af910b)の7件は全てCodeRabbitが✅自動確認済み。新たに3件の指摘があり、いずれも本物の修正として反映しました。
3件ともPostgreSQL公式ドキュメントの参照込みで裏付けがあり、いずれも妥当と判断したため修正しました。 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLAUDE.md`:
- Around line 53-55:
CLAUDE_CODE_OAUTH_TOKENの設定が必要であることを明記し、claude-review.ymlのClaudeレビューはトークン設定時にdraftのpushごとに実行され、未設定時はスキップされる説明へ更新してください。CodexとCodeRabbitの既存説明は変更しないでください。
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 271ff108-878d-437d-9fc2-468ff767a53c
📒 Files selected for processing (2)
CLAUDE.mddocs/permissions.md
CLAUDE_CODE_OAUTH_TOKEN未設定時はClaudeレビューがスキップされる旨が 記述から抜けており、実際のワークフロー(claude-review.yml)の挙動と 矛盾していた。Codex/CodeRabbitと同様に条件を明記する。 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CodeRabbit 4回目レビューの指摘分類 (commit 36212ed)
併せて、Claude自身の初回レビュー指摘( |
|
レビュー総評 このPRは
良い点
指摘(インラインコメント参照)
いずれも運用ドキュメントとしての正確性に関する指摘で、ブロッカーではありません。実地確認(Ready化後の挙動)を待ってから roadmap.md の記述を確定させることを推奨します。 |
roadmap.mdがCopilotのdraft/ready挙動を「確認済み」と断定していたが、 このPR自体のTest planは未検証のままだった。CodeRabbitのDraft動作の 確認と、Copilotの挙動(CLAUDE.mdの既存記載を踏襲・未検証)を書き分けた。 .coderabbit.yamlのauto_pause_after_reviewed_commits: 0も、5コミットを 超えた継続の実地確認がまだであることをコメントに追記した。 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.coderabbit.yaml (1)
38-46: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winINSERTの否定テストに、対象行を参照できる認証済みテスト主体を指定してください。
否定側クライアントの空の
RETURNINGやSELECTだけでは、RLSによる拒否とSELECTポリシーによる非表示を区別できません。対象行を参照できる主体で、新しい行が存在しないことを確認してください。service_roleは使用しないでください。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.coderabbit.yaml around lines 38 - 46, Update the INSERT negative-test guidance under the test/db/** configuration to require an authenticated test subject that can read the target rows, then verify from that subject that no new row was created. Do not rely only on the rejecting client’s empty RETURNING or SELECT result, and explicitly prohibit service_role credentials.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/roadmap.md`:
- Line 89: The CodeRabbit plan assumptions are inconsistent across the roadmap
and configuration. In docs/roadmap.md lines 89-89, replace “無料の3人目” with the
verified plan name and document behavior after the trial ends; in
.coderabbit.yaml lines 17-19, update the explanation for
auto_pause_after_reviewed_commits: 0 to reflect the actual plan that permits
GitHub PR reviews and its applicable limits.
---
Outside diff comments:
In @.coderabbit.yaml:
- Around line 38-46: Update the INSERT negative-test guidance under the
test/db/** configuration to require an authenticated test subject that can read
the target rows, then verify from that subject that no new row was created. Do
not rely only on the rejecting client’s empty RETURNING or SELECT result, and
explicitly prohibit service_role credentials.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b896b716-50f0-48ae-a6c1-993e56f9d0a1
📒 Files selected for processing (2)
.coderabbit.yamldocs/roadmap.md
レビュー結果このPRはコード変更を含まない、ドキュメント/レビューボット設定のみの変更(
重点的に検証した点
これらはすべて公式ドキュメントの記述と一致しており、正確です。「RETURNINGやSELECTが空=弾かれた証明にはならない」という結論も妥当で、 軽微な所見(nitpick、対応不要判断でも問題なし)PR本文のTest planのチェックボックスが3つとも未チェックですが、実際のレビュー履歴( 総評ドキュメントの内容・技術的な正確性ともに問題なし。既に4回の自己修正ラウンドを経ており、記述の精度は高い状態です。Approve相当と判断します。 |
## CodeRabbit 5回目レビューの指摘分類 (commit 4ef0ec8) - CodeRabbitのレビュー実行結果が毎回`Plan: Pro Plus`と表示されており、 CLAUDE.md/roadmap.mdが前提としていた「Freeプラン・1回/時のレート制限」 と矛盾していた。GitHub App導入直後のトライアル期間中の可能性が高いため、 本物の修正として注記を追加した(トライアル終了後にFreeプランの制限が 実際に効き始めたら更新する)。 - .coderabbit.yamlのtest/db/**向けpath_instructionsで、INSERTの否定側 テストが「対象行を見られる側の視点で」新規行が作られていないことを 確認する、という条件が抜けていた(UPDATE/DELETEは明記済み)。妥当な nitpickとして、docs/permissions.mdの既存記述と揃える形で修正した。 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CodeRabbit 5回目レビューの指摘分類 (commit c68ebb4)
|
レビュー総評ドキュメント/設定ファイルのみのPR( 全体として、CodeRabbitの5回の指摘往復を経て「未検証の事実には未検証と明記する」という自己規律が丁寧に守られている良いPRです。特に 本物の修正(インラインコメント参照)
妥当なnitpick(インラインコメント参照)
確認できた点
🤖 Generated with Claude Code |
## Claude 4回目レビューの指摘分類 (commit c68ebb4) - docs/permissions.md:88-99が自己矛盾していた。「UPDATE...RETURNINGは SELECTポリシー不一致でエラーになる」と書きながら、直後で同じ状況を 「エラーも出ない」と説明し、コード例もtry/catchなしでdata=[]を期待して いた。実際のexpenses_update_ownポリシー(using (user_id = auth.uid())) で検証すると、他人の行を更新しようとするケースはUSING句で静かに除外 される(エラーなし・0件)。「更新後の行がSELECTポリシーを満たさない とエラー」は別の仕組みで、UPDATE/SELECTポリシーの条件が食い違う場合 にのみ起こる、より狭いケース。本物の修正として2つの仕組みを分けて 説明し直した - docs/roadmap.mdの「Copilotのdraft/ready挙動はCLAUDE.mdの既存記載を 踏襲」という記述が誤り(base branch時点のCLAUDE.mdにCopilotへの言及は なく、このPRで新規に持ち込んだ主張だった)。妥当な指摘として、 Rulesetの設定自体は既知の事実である一方、挙動の実地確認はまだして いない、という位置づけに書き直した Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude 4回目レビューの指摘分類 (commit 2ec236b)
CodeRabbitの「Freeプラン前提の記述をPlan名に合わせて統一してほしい」という重複指摘は見送った。トライアル期間中の可能性が高いという判断で、既に注記を追加済み(commit c68ebb4)。実際のプラン契約状況はこちらでは確認できないため、確定的な書き換えはユーザー確認待ちとする。 |
レビュー総評(独立検証)コード変更なし。 過去の指摘を鵜呑みにせず、現在のHEAD( 技術的正確性(独立検証)
この一次情報と、現在の記述(USING句による静かな除外とUPDATE/INSERTのRETURNINGがSELECTポリシー違反でエラーになるケースを分けて説明している箇所)は一致しており、以前指摘されていた自己矛盾(「エラーになる」と書きながら直後で「エラーも出ない」とするコード例が並んでいた版)はcommit また、 気になった点(いずれもブロッカーではなく、過去レビューで既出)
総評ブロッキングな指摘なし。技術的主張はPostgreSQL公式ドキュメントで裏取りでき、既存の |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (2)
docs/permissions.md:112
- UPDATEの例でも、
data ?? []やbefore?.amountの形だと、クエリがエラーでdata: null/before: nullになった場合に意図せずテストが成立しうるため、errorがnullであることと、取得できていることを明示的に確認する例の方が安全です。
const { data: before } = await asUserA.from("expenses").select("amount").eq("id", id).single();
// これだけでは何も検証していない(WHERE不一致で0件なのか、正しく弾かれて0件なのか区別できない)
const { data } = await asUserB.from("expenses").update({ amount: 1 }).eq("id", id).select();
expect(data ?? []).toHaveLength(0);
docs/permissions.md:85
- 否定側の例で
data ?? []にフォールバックすると、クエリ自体がエラーでdata: nullになっても「0件」としてテストが通ってしまい、RLSではなく別の失敗(例: 権限/スキーマ/ネットワーク)を見逃します。errorがnullであることも併せて検証する例にした方が、意図(USING句で静かに0件になる)が伝わります。
This issue also appears on line 108 of the same file.
expect(dataA ?? []).toHaveLength(3);
// これが本体
const { data: dataB } = await asUserB.from("expenses").select();
expect(dataB ?? []).toHaveLength(0);
レビュー総評このPRはコード変更なし( 確認した内容
指摘
権限判定・削除分岐・公開設定の既定値に関わるコード変更は今回なく、テストの要否について指摘する点はありませんでした。 |
## Copilot 2回目レビューの指摘分類 (commit 40bf55d) - docs/permissions.md:81/85/108,112 | 本物の修正 | 前回追加した`data ?? []` というnull安全化が、実はerrorが発生してdata:nullになったケースを 「0件」として静かに握りつぶしてしまう、というCopilotの指摘は的確だった。 「否定側テストが何も検証していない」ことへの警戒を説くドキュメント自身が 同じ落とし穴を持っていたことになる。`?? []`を外し、`error`がnullである ことを明示的に検証する形に修正した Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
レビュー指摘分類 (commit 9447f05)
前回のClaudeレビュー(commit 40bf55d)で挙がった、docs/roadmap.mdのCopilot挙動記述とCLAUDE.mdのquota失敗モード記述は、既にcommit 40bf55dで反映済みのため該当スレッドをresolveします。 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (2)
docs/permissions.md:119
- 同様に、更新後の確認クエリでも
single()のerrorをチェックしていないため、取得に失敗してunchangedが null の場合でも、比較が成立してしまう可能性があります。意図通り「値が変化していない」ことを保証するため、errorの検証を追加した方が安全です。
// これが本体。本人(対象行を見られる側)の視点で値が実際に変化していないことを確認する
const { data: unchanged } = await asUserA.from("expenses").select("amount").eq("id", id).single();
expect(unchanged?.amount).toBe(before?.amount);
docs/permissions.md:110
- 後半の例(更新前の値を控える部分)だけ
single()のerrorを確認していないため、前段で強調している「errorがnullであることを確認する」方針と不整合になります。読者がそのままコピペすると、取得失敗時にbeforeが null のままテストが意図せず通る可能性があります。
This issue also appears on line 117 of the same file.
```typescript
// 更新前に本人視点で元の値を控えておく
const { data: before } = await asUserA.from("expenses").select("amount").eq("id", id).single();
レビュー総評このPRはdocs/設定ファイルのみの変更(
代わりに、今後のテストの書き方を左右する docs/permissions.md の技術的検証追加された「UPDATE/DELETEの否定側テストは対象行を見られる側の視点まで確認する」「RETURNINGはSELECTポリシー不一致だとエラーになる(inserted or updated rows to be returned are never silently ignored)」という記述を、PostgreSQL公式ドキュメント(CREATE POLICY)と突き合わせて確認しました。
RETURNING/USING句の説明が過去のレビューラウンドで一度自己矛盾していた点(4回目のClaudeレビューで指摘・修正済み)も、最終状態では正しく書き分けられていることを確認しました。 コメントした点
その他確認した点
全体として、過去のレビューラウンドで自己矛盾やnullフォールバックの問題が丁寧に潰されており、最終状態は正確です。ブロッキングな指摘はありません。 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CLAUDE.md (1)
59-62: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCopilotレビューの実行条件を条件付きで記載してください。
quotaに余裕がある場合に限り、Ready化時のCopilotレビューが実行されます。「1回受ける」ではなく「1回依頼する」と記載してください。Ready化後のpushごとの再レビューは、
copilot_code_reviewRulesetの設定により試行されます。設定確認の要求は削除してください。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CLAUDE.md` around lines 59 - 62, CLAUDE.mdのCopilotレビュー運用記述を更新し、quotaに余裕がある場合に限ってReady化時にCopilotレビューを1回依頼する条件を明記してください。「1回受ける」ではなく「1回依頼する」とし、Ready化後のpushごとの再レビューはcopilot_code_review Rulesetにより試行される旨を記載してください。設定確認を求める記述は削除してください。
♻️ Duplicate comments (1)
docs/permissions.md (1)
90-106: 🔒 Security & Privacy | 🟠 Major
SELECTポリシーによる空結果も説明してください。Line 103-104 は、空の結果の原因を
WHERE不一致とUSING除外だけに限定しています。UPDATEまたはDELETEがWHEREまたはRETURNINGで対象列を読む場合、SELECTポリシーも既存行を絞り込みます。そのため、dataが空でerrorがnullでも、SELECTポリシーによる抑止を除外できません。Line 96-101 は
UPDATE ... RETURNINGのエラーだけを説明しています。DELETEでも同じ条件でSELECTポリシーが適用されます。空結果の判定にこの経路を追加してください。(postgresql.org)修正例
-つまり「RETURNINGが空で、エラーも出ない」という結果だけでは、(a) `WHERE`条件に一致する行が -最初から無かったのか、(b) USING句によって正しく弾かれたのか、を区別できない。 +つまり「RETURNINGが空で、エラーも出ない」という結果だけでは、(a) `WHERE`条件に一致する行が +最初から無かったのか、(b) USING句によって弾かれたのか、(c) 対象列の読み取りに伴う +SELECTポリシーによって既存行が絞り込まれたのか、を区別できない。 + +DELETEでも、`WHERE`または`RETURNING`が対象列を読む場合はSELECTポリシーが適用される。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/permissions.md` around lines 90 - 106, Update the documentation around the empty-result explanation in the UPDATE/DELETE section to include SELECT-policy filtering as an additional possible cause when data is empty and error is null, including visibility needed by WHERE or RETURNING. Clarify that DELETE is also subject to SELECT-policy behavior, while retaining the existing UPDATE ... RETURNING error explanation and instructing tests to verify the unchanged or remaining row from an authorized perspective.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/permissions.md`:
- Around line 109-119: Update both asUserA single-row reads in the before and
unchanged checks to retain their error results and assert each error is null and
each data object is present before comparing amount; then compare the verified
amount values.
---
Outside diff comments:
In `@CLAUDE.md`:
- Around line 59-62:
CLAUDE.mdのCopilotレビュー運用記述を更新し、quotaに余裕がある場合に限ってReady化時にCopilotレビューを1回依頼する条件を明記してください。「1回受ける」ではなく「1回依頼する」とし、Ready化後のpushごとの再レビューはcopilot_code_review
Rulesetにより試行される旨を記載してください。設定確認を求める記述は削除してください。
---
Duplicate comments:
In `@docs/permissions.md`:
- Around line 90-106: Update the documentation around the empty-result
explanation in the UPDATE/DELETE section to include SELECT-policy filtering as
an additional possible cause when data is empty and error is null, including
visibility needed by WHERE or RETURNING. Clarify that DELETE is also subject to
SELECT-policy behavior, while retaining the existing UPDATE ... RETURNING error
explanation and instructing tests to verify the unchanged or remaining row from
an authorized perspective.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d388b009-02b6-48cf-b995-5e2d61fd7816
📒 Files selected for processing (4)
.coderabbit.yamlCLAUDE.mddocs/permissions.mddocs/roadmap.md
## レビュー指摘分類 (commit 9447f05) - docs/permissions.md:110/119 | 本物の修正(Copilot) | 前回`error`検証を 追加したUPDATE否定側テスト例で、同じブロック内の`before`/`unchanged` 取得(`.single()`)にだけerror検証が抜けていた。一貫性のため全クエリに error検証を追加した - docs/roadmap.md:89 | 妥当なnitpick(Claude) | 「プレミアムリクエスト13回 相当」という数値に出典がなかった。公式の固定値ではなく実測値である旨を 明記した Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
レビュー指摘分類 (commit a9c1811)
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
CLAUDE.md:73
- インラインコード(
...)が改行をまたいでおり(72-73行)、Markdownとして正しくレンダリングされません。コマンドは箇条書き内の fenced code block にすると崩れずに読めます。
- Copilotの最終レビューは「プレミアムリクエストのquota上限に達したため実行できなかった」
という形で失敗することがある(PR #35で発生)。この場合レビューコメントは投稿されるが
中身のないもので、コードは実際にはレビューされていない。quotaを追加してから
`gh api repos/{owner}/{repo}/pulls/{number}/requested_reviewers -X POST
-f 'reviewers[]=copilot-pull-request-reviewer[bot]'` で再リクエストする
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/permissions.md (1)
90-106:⚠️ Potential issue | 🟠 Major
SELECTポリシーによる除外を判定に追加してください。Line 91 の「まずコマンド自身の
USING句で絞り込まれる」は、この例では不正確です。WHEREまたはRETURNINGが対象テーブルの列を読むUPDATE/DELETEでは、SELECTポリシーも適用されます。USING句は、読み取り可能な行のうち、更新または削除を許可する行を制限します。(postgresql.org)そのため、Lines 103-104 の分類に
SELECTポリシーによる除外を追加してください。data=[]かつerror=nullは、(a)WHERE条件不一致、(b)SELECTポリシーによる非可視化、(c) command-specificUSING句による除外のいずれでも発生します。Line 96-101 のUPDATE ... RETURNINGのエラー説明は維持できます。修正例
- PostgreSQLのRLSでは、UPDATE/DELETEの対象行はまずコマンド自身のUSING句で絞り込まれる。 + 対象テーブルの列をWHERE、RETURNING、または更新式で読むUPDATE/DELETEでは、 + SELECTポリシーとコマンド自身のUSING句の両方が対象行を制限する。 - (a) WHERE条件に一致する行が最初から無かったのか、(b) USING句によって正しく弾かれたのか + (a) WHERE条件に一致する行が最初から無かったのか、(b) SELECTポリシーで見えないのか、 + (c) USING句によって正しく弾かれたのか#!/usr/bin/env bash set -euo pipefail rg -n -C 4 \ 'expenses_update_own|CREATE POLICY|SELECT|UPDATE|DELETE|RETURNING|USING|\.update\(|\.delete\(' \ docs/permissions.md supabase test 2>/dev/null || true🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/permissions.md` around lines 90 - 106, Update the explanation around the UPDATE/DELETE result classification in docs/permissions.md to include SELECT-policy invisibility as a third cause of data=[] with error=null, alongside WHERE mismatch and command-specific USING exclusion. Keep the existing UPDATE ... RETURNING error explanation intact, and revise the USING description so it accurately distinguishes SELECT visibility from command authorization.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@docs/permissions.md`:
- Around line 90-106: Update the explanation around the UPDATE/DELETE result
classification in docs/permissions.md to include SELECT-policy invisibility as a
third cause of data=[] with error=null, alongside WHERE mismatch and
command-specific USING exclusion. Keep the existing UPDATE ... RETURNING error
explanation intact, and revise the USING description so it accurately
distinguishes SELECT visibility from command authorization.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 44c50e96-0cdf-49be-a364-c161ec980a77
📒 Files selected for processing (2)
docs/permissions.mddocs/roadmap.md
| - Ready化後にCopilot/CodeRabbitの指摘で追加修正が発生した場合は、都度pushしてよい | ||
| (Ready後はpushのたびに再レビューが走る。ただしCodeRabbitは上記レート制限の対象) |
There was a problem hiding this comment.
妥当なnitpick: このDraft先行フローの狙いは「Claude/Codex/CodeRabbitとの反復ではプレミアムリクエストを消費せず、Copilotの最終レビューだけ1回受ける」ことのはず(52-62行目)。しかしこの箇条書きは「Ready化後の追加修正は都度pushしてよい」としており、Ready後の再push1回ごとにCopilotのレビューが再度走ってプレミアムリクエストを消費する(すぐ上の行が明記している通り)。指摘が多く複数回の追加pushが必要になった場合、このPR自体がまさにその状態(Ready化後もレビュー往復が発生)であり、結局Copilotのクレジットを何度も消費してしまう可能性がある。「Ready後に指摘が複数回にわたりそうな場合はDraftに戻す」のような回避策や、そもそも許容している旨を一言添えておくと、この運用が解決しようとしている問題(Copilotのクレジット消費)を自ら再現してしまう抜けを塞げる。
レビュー総評このPRは 良い点
指摘インラインコメントを1件投稿しました( その他、 |
レビュー指摘分類 (commit a9c1811時点、最終)
|
* feat: markdownlintを導入し、静的解析強化の継続運用を確立する (#40) CodeRabbitがPR #35〜#39で繰り返し指摘したMD040/MD031相当の指摘は、本来 静的解析で無料かつ即座に拾えるはずだった。markdownlint-cli2をyarn lintに 統合し、docs/**/*.md・.claude/skills/**/*.md・ルート直下*.mdをerrorで ゲートする。既存の236件の違反(見出し/リスト/テーブル前後の空行、テーブル のパイプ間隔など)は--fixで機械的に解消し、自動修正できなかった言語識別子 欠落(MD040)と引用ブロック内の空行(MD028)は手で直した。 併せて、レビュー指摘を起点に静的解析を強化していく運用をdocs/lint-policy.md に明文化し、pr-review-flow skillの指摘分類に「静的解析で拾えたはずか」の 軸を追加した。 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: yarn lintの説明をmarkdownlint統合後の実態に合わせる CodeRabbitの指摘(PR #41): CLAUDE.md/docs/prd.mdの「yarn lint = ESLint」 という記述がpackage.jsonの実際のスクリプト(eslint && markdownlint-cli2) と食い違っていた。 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: yarn lint説明のツール名をmarkdownlint-cli2に統一 Copilotの指摘(PR #41): package.jsonの実際のパッケージ名(markdownlint-cli2) と表記を揃える。 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
概要
PR #18〜#32のレビュー実績(Claude/Copilotの指摘重複率分析、Copilotのクレジット消費)を踏まえた運用変更。ドキュメント・設定ファイルのみの変更。
変更内容
CLAUDE.md— PRをDraftで作成し、Claude/Codex/CodeRabbit(無料)との反復後にReady化してCopilot(有料)の最終レビューを受ける運用を明記.coderabbit.yaml(新規) — CodeRabbit導入。デフォルトのdraftスキップをdrafts: trueで無効化し、CLAUDE.mdの絶対ルール・RLS・test/dbの観点をpath_instructionsに明示docs/roadmap.md— 「保留: 外部アカウント待ち」にCodeRabbit導入(完了)を追記docs/permissions.md— Issue #26で見つかった「RETURNINGだけでは弾かれたことの証明にならない」罠を追記このPR自体の位置づけ
新しいDraft先行フローの動作確認を兼ねる。Draft状態でClaude/Codex/CodeRabbitのレビューが走り、Copilotは走らないことを確認してからReady化する。
Test plan
OPENAI_API_KEY未設定のため意図通りスキップ、Copilotは走らないことを確認(Draft中の7回のpushでCopilotのレビューは一度も付かなかった)gh pr ready実行後にcopilot-pull-request-reviewerが起動。1回目はプレミアムリクエストのquota上限で失敗、quota追加後の再リクエストで2回成功。詳細はCLAUDE.md/docs/roadmap.md参照)Summary by CodeRabbit
ドキュメント
開発プロセス