From 0531a29eb43e08546164ef0fee7ec4b7a277f210 Mon Sep 17 00:00:00 2001 From: Wada Yusuke Date: Thu, 9 Jul 2026 20:11:10 +0900 Subject: [PATCH 1/2] =?UTF-8?q?fix(backend):=20GitHub=20API=20=E3=83=AC?= =?UTF-8?q?=E3=83=BC=E3=83=88=E5=88=B6=E9=99=90=E3=81=AE=20403=20=E3=82=92?= =?UTF-8?q?=E3=83=AA=E3=83=88=E3=83=A9=E3=82=A4=E3=81=9B=E3=81=9A=E6=8F=A1?= =?UTF-8?q?=E3=82=8A=E8=BE=BC=E3=82=80=E4=B8=8D=E5=85=B7=E5=90=88=E3=82=92?= =?UTF-8?q?=E4=BF=AE=E6=AD=A3=EF=BC=88#485=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fetch_repo_tree が rate-limit の 403 を partial 走査として黙って握り込み、 リトライされずスキル証跡が欠けていた問題を修正。fetch_repos_raw の判別ロジックを 共通ヘルパ（_raise_if_rate_limited）に抽出し、fetch_repo_tree / fetch_languages / fetch_repo_file の GitHub API 呼び出し全経路で RetryableError に統一した。 Co-Authored-By: Claude Sonnet 5 --- .../intelligence/github/api_client.py | 69 ++++++++++++++----- backend/tests/test_github_api_client.py | 56 ++++++++++++++- 2 files changed, 106 insertions(+), 19 deletions(-) diff --git a/backend/app/services/intelligence/github/api_client.py b/backend/app/services/intelligence/github/api_client.py index e56e9030..4adc2310 100644 --- a/backend/app/services/intelligence/github/api_client.py +++ b/backend/app/services/intelligence/github/api_client.py @@ -90,22 +90,11 @@ async def fetch_repos_raw( ) if resp.status_code == 404: raise GitHubUserNotFoundError(username) + # レート制限（403 + 残量 0 / 429）は共通ヘルパで RetryableError に変換する + _raise_if_rate_limited(resp) if resp.status_code == 403: - # GitHub は rate limit でも 403 を返すため、ヘッダで判別する - if _is_rate_limited(resp): - retry_after = _retry_after_from_github(resp) - logger.warning( - "GitHub API rate limit hit (retry_after=%s)", retry_after, - ) - raise RetryableError( - "GitHub API rate limit", retry_after=retry_after, - ) + # レート制限でない 403 は権限エラー等の恒久障害 raise NonRetryableError(f"GitHub API 403 Forbidden: {resp.text[:200]}") - if resp.status_code == 429: - retry_after = _retry_after_from_github(resp) - raise RetryableError( - "GitHub API 429 Too Many Requests", retry_after=retry_after, - ) if resp.status_code in _RETRYABLE_STATUS_CODES: raise RetryableError(f"GitHub API {resp.status_code}") if 400 <= resp.status_code < 500: @@ -121,9 +110,35 @@ async def fetch_repos_raw( def _is_rate_limited(response: httpx.Response) -> bool: - """GitHub のレスポンスがレート制限起因かを判定する。""" - remaining = response.headers.get("x-ratelimit-remaining") - return remaining == "0" + """GitHub のレスポンスがレート制限起因かを判定する。 + + GitHub は API レート制限を超過すると **403** を返す（残量ヘッダで判別する）。 + 二次的な abuse 検出などで **429** を返すこともあり、こちらは残量に依らずレート制限扱い。 + """ + if response.status_code == 429: + return True + if response.status_code == 403: + return response.headers.get("x-ratelimit-remaining") == "0" + return False + + +def _raise_if_rate_limited(response: httpx.Response) -> None: + """レスポンスがレート制限（403 + 残量 0 / 429）なら ``RetryableError`` を raise する。 + + GitHub の全 fetch 経路で共通のレート制限ハンドリング。レート制限は特定リポの問題では + なくトークン単位のグローバルなスロットリングのため、``([], partial)`` として黙って + 握り込むと後続リポも同様に空収集になり、証跡が静かに欠ける。``retry_after`` 付きで + raise し直し、リセット窓を待ってタスク全体を再試行させる（``fetch_repos_raw`` と同方針）。 + レート制限でなければ何もしない（呼び出し側が他ステータスを処理する）。 + """ + if _is_rate_limited(response): + retry_after = _retry_after_from_github(response) + logger.warning( + "GitHub API rate limit hit (status=%s, retry_after=%s)", + response.status_code, + retry_after, + ) + raise RetryableError("GitHub API rate limit", retry_after=retry_after) def _retry_after_from_github(response: httpx.Response) -> float | None: @@ -155,8 +170,11 @@ async def fetch_languages( return {} try: resp = await client.get(f"/repos/{owner}/{repo}/languages") + # レート制限は握り込まず RetryableError で連携全体をリトライさせる（#485） + _raise_if_rate_limited(resp) if resp.status_code == 403: - logger.warning("Rate limit on languages for %s/%s", owner, repo) + # レート制限でない 403（ブロック等）は言語情報を欠いたまま best-effort 継続 + logger.warning("Languages fetch forbidden for %s/%s", owner, repo) return {} resp.raise_for_status() return resp.json() @@ -183,6 +201,10 @@ async def fetch_repo_tree( 加え、tree 取得自体が失敗した場合（非200 / 不正レスポンス / ``httpx.HTTPError``）も 「依存ゼロ」と「走査不能」を区別するため ``True`` を返す（D9(d)）。不正 owner/repo は 実在リポではなく走査対象ですらないため ``([], False)`` とする。 + + ただしレート制限（403 + 残量 0 / 429）は 1 リポの部分走査ではなくトークン単位の + グローバルなスロットリングのため、partial として握り込まず ``RetryableError`` を raise し、 + リセット窓を待って連携タスク全体を再試行させる（#485。``fetch_repos_raw`` と同方針）。 """ if not _is_valid_owner_repo(owner, repo): return [], False @@ -192,7 +214,16 @@ async def fetch_repo_tree( f"/repos/{owner}/{repo}/git/trees/{branch}", params={"recursive": "1"}, ) + # レート制限は「1 リポの部分走査」ではなくトークン単位のスロットリングのため、 + # partial として握り込まず RetryableError で連携全体をリトライさせる（#485）。 + _raise_if_rate_limited(resp) if resp.status_code != 200: + logger.warning( + "Git tree fetch returned %s for %s/%s (partial)", + resp.status_code, + owner, + repo, + ) return [], True data = resp.json() if not isinstance(data, dict): @@ -228,6 +259,8 @@ async def fetch_repo_file( f"/repos/{owner}/{repo}/contents/{path}", headers={"Accept": "application/vnd.github.raw+json"}, ) + # レート制限は握り込まず RetryableError で連携全体をリトライさせる（#485） + _raise_if_rate_limited(resp) if resp.status_code != 200: return None return resp.text diff --git a/backend/tests/test_github_api_client.py b/backend/tests/test_github_api_client.py index 9161bbd4..7ee83592 100644 --- a/backend/tests/test_github_api_client.py +++ b/backend/tests/test_github_api_client.py @@ -9,7 +9,9 @@ from unittest.mock import AsyncMock, MagicMock import httpx -from app.services.intelligence.github.api_client import fetch_repo_tree +import pytest +from app.services.intelligence.github.api_client import fetch_languages, fetch_repo_tree +from app.services.tasks.exceptions import RetryableError def _run(coro): @@ -23,12 +25,24 @@ def _run(coro): def _client_with_tree(tree, truncated=False, status_code=200): """Trees API レスポンスを返す AsyncClient モックを生成する。""" resp = MagicMock(status_code=status_code) + resp.headers = {} resp.json = MagicMock(return_value={"tree": tree, "truncated": truncated}) client = MagicMock() client.get = AsyncMock(return_value=resp) return client +def _client_with_status(status_code, headers=None): + """任意ステータス・ヘッダのレスポンスを返す AsyncClient モックを生成する。""" + resp = MagicMock(status_code=status_code) + resp.headers = headers or {} + resp.json = MagicMock(return_value={}) + resp.text = "" + client = MagicMock() + client.get = AsyncMock(return_value=resp) + return client + + def test_returns_all_blob_paths(): """blob の相対パスをすべて返し、ディレクトリ（tree）は除外すること。""" tree = [ @@ -74,3 +88,43 @@ def test_invalid_owner_repo_returns_empty(): result = _run(fetch_repo_tree(client, "../evil", "repo", "main")) assert result == ([], False) client.get.assert_not_called() + + +def test_rate_limited_403_raises_retryable(): + """レート制限の 403（X-RateLimit-Remaining:0）は partial 握り込みではなく + RetryableError を raise し、連携全体をリトライ経路へ乗せること（#485）。""" + client = _client_with_status( + 403, headers={"x-ratelimit-remaining": "0", "retry-after": "42"} + ) + with pytest.raises(RetryableError) as exc: + _run(fetch_repo_tree(client, "u", "repo", "main")) + assert exc.value.retry_after == 42 + + +def test_429_raises_retryable(): + """429 Too Many Requests も RetryableError（retry_after 付き）で raise すること（#485）。""" + client = _client_with_status(429, headers={"retry-after": "30"}) + with pytest.raises(RetryableError) as exc: + _run(fetch_repo_tree(client, "u", "repo", "main")) + assert exc.value.retry_after == 30 + + +def test_genuine_403_returns_partial(): + """レート制限でない 403（残量あり = 権限エラー等）はリトライさせず + 従来どおり partial 扱い（[], True）で返すこと（#485）。""" + client = _client_with_status(403, headers={"x-ratelimit-remaining": "57"}) + assert _run(fetch_repo_tree(client, "u", "repo", "main")) == ([], True) + + +def test_languages_rate_limited_raises_retryable(): + """fetch_languages もレート制限 403 を {} で握り込まず RetryableError を raise すること + （同一ホットパスの兄弟 fetch も #485 で統一）。""" + client = _client_with_status(403, headers={"x-ratelimit-remaining": "0"}) + with pytest.raises(RetryableError): + _run(fetch_languages(client, "u", "repo")) + + +def test_languages_genuine_403_returns_empty(): + """レート制限でない 403 は言語情報を欠いたまま {} で best-effort 継続すること。""" + client = _client_with_status(403, headers={"x-ratelimit-remaining": "57"}) + assert _run(fetch_languages(client, "u", "repo")) == {} From c05c7fef9867a5b779f08714fa857534592e3587 Mon Sep 17 00:00:00 2001 From: Wada Yusuke Date: Thu, 9 Jul 2026 20:23:27 +0900 Subject: [PATCH 2/2] =?UTF-8?q?test(backend):=20fetch=5Frepo=5Ffile=20?= =?UTF-8?q?=E3=81=AE=E3=83=AC=E3=83=BC=E3=83=88=E5=88=B6=E9=99=90=E3=83=AA?= =?UTF-8?q?=E3=83=88=E3=83=A9=E3=82=A4=E5=9B=9E=E5=B8=B0=E3=83=86=E3=82=B9?= =?UTF-8?q?=E3=83=88=E3=82=92=E8=BF=BD=E5=8A=A0=EF=BC=88#485=20=E3=83=AC?= =?UTF-8?q?=E3=83=93=E3=83=A5=E3=83=BC=E5=8F=8D=E6=98=A0=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fetch_repo_file も _raise_if_rate_limited を呼ぶが未テストだったため、 fetch_repo_tree / fetch_languages と同じスタイルで - レート制限 403 → RetryableError - 429 → RetryableError - 権限 403（残量あり）→ None（当該 manifest スキップ） の 3 ケースを追加。 Co-Authored-By: Claude Opus 4.8 --- backend/tests/test_github_api_client.py | 32 ++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/backend/tests/test_github_api_client.py b/backend/tests/test_github_api_client.py index 7ee83592..610cdd1b 100644 --- a/backend/tests/test_github_api_client.py +++ b/backend/tests/test_github_api_client.py @@ -10,7 +10,11 @@ import httpx import pytest -from app.services.intelligence.github.api_client import fetch_languages, fetch_repo_tree +from app.services.intelligence.github.api_client import ( + fetch_languages, + fetch_repo_file, + fetch_repo_tree, +) from app.services.tasks.exceptions import RetryableError @@ -128,3 +132,29 @@ def test_languages_genuine_403_returns_empty(): """レート制限でない 403 は言語情報を欠いたまま {} で best-effort 継続すること。""" client = _client_with_status(403, headers={"x-ratelimit-remaining": "57"}) assert _run(fetch_languages(client, "u", "repo")) == {} + + +def test_repo_file_rate_limited_403_raises_retryable(): + """fetch_repo_file もレート制限 403 を None で握り込まず RetryableError を raise すること + （同一ホットパスの兄弟 fetch も #485 で統一）。""" + client = _client_with_status( + 403, headers={"x-ratelimit-remaining": "0", "retry-after": "42"} + ) + with pytest.raises(RetryableError) as exc: + _run(fetch_repo_file(client, "u", "repo", "requirements.txt")) + assert exc.value.retry_after == 42 + + +def test_repo_file_429_raises_retryable(): + """fetch_repo_file の 429 も RetryableError（retry_after 付き）で raise すること（#485）。""" + client = _client_with_status(429, headers={"retry-after": "30"}) + with pytest.raises(RetryableError) as exc: + _run(fetch_repo_file(client, "u", "repo", "requirements.txt")) + assert exc.value.retry_after == 30 + + +def test_repo_file_genuine_403_returns_none(): + """レート制限でない 403（残量あり = 権限エラー等）はリトライさせず + 従来どおり None（当該 manifest をスキップ）で best-effort 継続すること（#485）。""" + client = _client_with_status(403, headers={"x-ratelimit-remaining": "57"}) + assert _run(fetch_repo_file(client, "u", "repo", "requirements.txt")) is None