This repository was archived by the owner on Aug 26, 2026. It is now read-only.
[Phase1] test/db/のauth.signUp rate limit対策 - #56
Merged
Merged
Conversation
test/db/のcreateTestUser()は呼び出しごとにauth.signUpを実行するが、現在 70件超の呼び出し箇所がありsupabase/config.tomlのauth.rate_limit.sign_in_sign_ups の既定値(5分間に30件)を大きく超えている。 実際に再現した: test:dbをリセットなしで連続実行すると、直近の実行回数の 累積によって間欠的にsignUp失敗でテストが赤くなることを確認した(3/6ファイルが 失敗)。テストが増えるほど顕在化しやすくなる。 このファイルはsupabase start(ローカル/CI専用のDockerコンテナ)にのみ適用され、 本番のSupabaseプロジェクトのダッシュボード側の設定には影響しないため、 sign_in_sign_upsをテスト用途に十分な値まで引き上げた。修正後、 リセットなしで10回連続実行して安定してpassすることを確認済み。 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
レビュー結果CLAUDE.md / docs/ の観点で確認しましたが、問題は見つかりませんでした。承認します。 確認した観点
個別確認事項
指摘事項なし。 |
📝 WalkthroughWalkthroughSupabase Authのサインアップ・サインイン要求上限を、ローカルおよびCI環境で30から1000へ変更します。 ChangesSupabase Authレート制限
Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@supabase/config.toml`:
- Line 210:
コメント内の測定日「2026-08-08」を、実際に「70件超」を測定した日付へ修正してください。実測日を確認できない場合は、日付と件数を削除し、再現可能なテスト結果のみを記載してください。
🪄 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: 5d76fba7-c3ba-40b0-a9f2-e22b0280a3d2
📒 Files selected for processing (1)
supabase/config.toml
| # Number of sign up and sign-in requests that can be made in a 5 minute interval per IP address (excludes anonymous users). | ||
| sign_in_sign_ups = 30 | ||
| # issue #33: test/db/ の各テストが createTestUser() で auth.signUp を呼ぶため、テストスイート全体で | ||
| # 既定値(30)を大きく超える(2026-08-08時点で70件超、テストが増えるほど増加する)。このファイルは |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
計測日を実際の日付に修正してください。
このレビュー時点は 2026年8月7日 です。Line 210 は 2026年8月8日 時点の測定値を記載しているため、将来日付になっています。70件超を測定した実際の日付に修正するか、日付と件数を削除して再現可能なテスト結果を記載してください。将来日付の記録は、後続の設定変更時に誤解を招きます。
🤖 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 `@supabase/config.toml` at line 210,
コメント内の測定日「2026-08-08」を、実際に「70件超」を測定した日付へ修正してください。実測日を確認できない場合は、日付と件数を削除し、再現可能なテスト結果のみを記載してください。
Owner
Author
レビュー指摘の分類
|
There was a problem hiding this comment.
Pull request overview
Supabase Auth の sign_in_sign_ups レート制限が test/db/ の auth.signUp 多用によりテスト実行中に超過し、間欠的な失敗(フレーク)が起きうる問題を、ローカル/CI 向け設定で緩和するPRです。
Changes:
supabase/config.tomlのauth.rate_limit.sign_in_sign_upsを 30 → 1000 に引き上げ- 変更理由・適用範囲(ローカル/CIのみ)をコメントとして追記
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
reitojike
added a commit
that referenced
this pull request
Aug 8, 2026
PR #68 のClaude Reviewの指摘に対応。指摘は正しかった。 誤り: 「300行を超えた5本」は事実と異なり、素の差分では9本だった (#31 307 / #59 365 / #60 380 / #30 441 / #29 617 / #18 679 / #41 698 / #32 1063 / #16 3528)。同じ本文が書いていた「300行以下が65%(17/26)」は 超過9本を含意しており、記述が自己矛盾していた。 原因: 母集団の統計(中央値・65%)は素の差分で数え、外れ値の説明だけに 除外規則を適用していた。数え方を混在させたうえ、超過リストを上位5本で 打ち切って全件確認しなかった。 訂正: 26本すべてを git show --numstat で数え直し、除外の段階ごとに 表で示す。素の差分(中央値135行 / 65% / 超過9本)、パス名で機械的に 判定できる除外まで(中央値128行 / 85% / 超過4本)、除外規則を最後まで 適用(超過2本 = #59・#60)。落ちる7本の内訳も明記した。 あわせて、3段階目の中央値を出さない理由を書いた。supabase/config.toml は #29 では supabase init の出力(416行)、#56 では根拠コメント付きで手で 直した6行で、同じパスでも扱いが逆になる。パス名では決まらないことが、 この節が機械的ゲートになり得ない理由そのものなので、「lintではない」の 段落の根拠もこの実測に差し替えた。 Refs #44
reitojike
added a commit
that referenced
this pull request
Aug 8, 2026
* docs: Issueの粒度とPR差分サイズの目安をCLAUDE.mdに明文化 判断ポイントは1 Issueに3個まで(5個超で分割)、PR差分は300行を目安とする。 記事の実測値をそのまま採らず、main にマージ済みのPR 26本(中央値135行、 300行以下65%)で裏を取ってから採用した。行数の数え方から生成物・ロック ファイル・権限マトリクスを写したテスト表を除外する根拠も、超過した PR #16 / #32 の実態から示した。 機械的ゲートにしない旨と、3回ルール(モデルを上げる) / PO確認(判断を 下せる層に上げる) / 粒度超過(Issueを分ける)の対処の違いを表で整理。 docs/roadmap.md はポインタ1行に留め、根拠は1箇所にだけ置く。 Refs #44 * docs: 判断ポイント数の境界を一本化し、裏取り済みの数値と外部実測を書き分ける PR #68 のClaude Reviewの指摘2件に対応。 指摘1: 「3個まで、5個を超えるなら分割」で4個の扱いが未定義だった。 閾値を「3個まで。4個目が出てきたら分ける」に一本化する。機械的ゲートに しない方針である以上、「検討」と「必ず」の二段構えは実効性のない false precisionになるため、緩衝域を作らず単一の線にした。 指摘2: 裏取り済みの300行と、外部実測のままの3個が同じ文脈に並んでいた。 「2つの数字は裏付けの強さが違う」として段落を分け、300行はこのリポジトリの 実測(PR 26本、中央値135行/300行以下65%)で検証済み、3個は外部実測のみを 根拠とする未検証のヒューリスティックであると明示した。過去Issueの判断数は 記録がなく後から数え直せないため、このリポジトリでの裏取りが今はできない 理由も併記。採用の根拠はコストの非対称性に置いた。 あわせて、外部実測に対応値のない4個/5個を推定して線を引いていないことと、 実績が溜まったら見直す旨を記載した。 Refs #44 * docs: PR実測の集計を数え直し、超過本数の誤りを訂正する PR #68 のClaude Reviewの指摘に対応。指摘は正しかった。 誤り: 「300行を超えた5本」は事実と異なり、素の差分では9本だった (#31 307 / #59 365 / #60 380 / #30 441 / #29 617 / #18 679 / #41 698 / #32 1063 / #16 3528)。同じ本文が書いていた「300行以下が65%(17/26)」は 超過9本を含意しており、記述が自己矛盾していた。 原因: 母集団の統計(中央値・65%)は素の差分で数え、外れ値の説明だけに 除外規則を適用していた。数え方を混在させたうえ、超過リストを上位5本で 打ち切って全件確認しなかった。 訂正: 26本すべてを git show --numstat で数え直し、除外の段階ごとに 表で示す。素の差分(中央値135行 / 65% / 超過9本)、パス名で機械的に 判定できる除外まで(中央値128行 / 85% / 超過4本)、除外規則を最後まで 適用(超過2本 = #59・#60)。落ちる7本の内訳も明記した。 あわせて、3段階目の中央値を出さない理由を書いた。supabase/config.toml は #29 では supabase init の出力(416行)、#56 では根拠コメント付きで手で 直した6行で、同じパスでも扱いが逆になる。パス名では決まらないことが、 この節が機械的ゲートになり得ない理由そのものなので、「lintではない」の 段落の根拠もこの実測に差し替えた。 Refs #44 * docs: PR #29/#32の除外理由が2段階なのに1段階しか書いていなかった記述漏れを修正 Claude Reviewの指摘どおり、#29(617→22)はsupabase/config.toml(416行) だけでなくsupabase/types.ts(179行)も、#32(1063→79)はテスト表(924行) だけでなくyarn.lock(60行)も除外して初めて数字が再現できる。 片方しか書いていなかったため、追試すると数値が合わなかった。 Refs #44
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
背景
PR #32のCopilotレビューで指摘された未対応事項。
test/db/helpers.tsのcreateTestUser()は呼び出しごとにauth.signUpを実行するが、test/db/全体で現在70件超の呼び出し箇所があり、supabase/config.tomlのauth.rate_limit.sign_in_sign_upsの既定値(5分間に30件。issue本文では「30/hour」と記載されていたが、実際のconfig.tomlのコメントは「5分間隔」)を大きく超えている。Closes #33
採用した方針
supabase/config.tomlのsign_in_sign_upsをテスト用途に十分な値まで引き上げる(1000)。このファイルはsupabase startで立ち上がるローカル/CI専用のDockerコンテナにのみ適用され、本番のSupabaseプロジェクト(ダッシュボード側で管理)には影響しないため、安全に引き上げられる。ユーザープーリングによるsignUp回数自体の削減も選択肢にあったが、テストごとに独立したユーザーを作る現在の設計(テスト間の状態汚染を避けるため)を崩さずに済み、リスクが小さいこちらを採用した。
実際に再現した
修正前、
test:dbをリセットなしで連続実行すると、直近の累積呼び出し数によって間欠的にsignUp失敗でテストが赤くなることを確認した(6ファイル中3ファイルが失敗)。テストが増えるほど顕在化しやすくなるという、issueの懸念どおりの挙動だった。確認
yarn lint/yarn typecheck/yarn testすべてpass🤖 Generated with Claude Code
Summary by CodeRabbit