mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-10-06 06:32:12 +00:00
fix: guard reviewer GraphQL against null repository (#1426)
GitHub returns repository: null when the token can't read the repo (SAML,
expired token, private/deleted). dict.get(k, {}) doesn't coalesce explicit
null, so fetch_pr_review_threads crashed with AttributeError and publish_review
could never post a review. Guard with isinstance checks and return collected
threads on null repository; sweep the same pattern in resolve_review_thread.
Co-authored-by: open-swe[bot] <215916821+open-swe[bot]@users.noreply.github.com>
This commit is contained in:
parent
bbe449244a
commit
6a9fd85295
2 changed files with 37 additions and 8 deletions
|
|
@ -503,12 +503,21 @@ async def fetch_pr_review_threads(
|
||||||
)
|
)
|
||||||
return out
|
return out
|
||||||
data = response.json()
|
data = response.json()
|
||||||
threads = (
|
data_root = data.get("data") if isinstance(data, dict) else None
|
||||||
data.get("data", {})
|
repository = data_root.get("repository") if isinstance(data_root, dict) else None
|
||||||
.get("repository", {})
|
if not isinstance(repository, dict):
|
||||||
.get("pullRequest", {})
|
logger.warning(
|
||||||
.get("reviewThreads", {})
|
"Null repository in review-threads response for %s/%s#%s "
|
||||||
)
|
"(token likely lacks access: SAML, expired token, or private/deleted repo)",
|
||||||
|
owner,
|
||||||
|
repo,
|
||||||
|
pr_number,
|
||||||
|
)
|
||||||
|
return out
|
||||||
|
pull_request = repository.get("pullRequest")
|
||||||
|
threads = pull_request.get("reviewThreads") if isinstance(pull_request, dict) else None
|
||||||
|
if not isinstance(threads, dict):
|
||||||
|
return out
|
||||||
for thread in threads.get("nodes", []) or []:
|
for thread in threads.get("nodes", []) or []:
|
||||||
if not isinstance(thread, dict):
|
if not isinstance(thread, dict):
|
||||||
continue
|
continue
|
||||||
|
|
@ -668,8 +677,10 @@ async def resolve_review_thread(*, thread_node_id: str, token: str) -> bool:
|
||||||
if data.get("errors"):
|
if data.get("errors"):
|
||||||
logger.warning("resolveReviewThread errors: %s", data["errors"])
|
logger.warning("resolveReviewThread errors: %s", data["errors"])
|
||||||
return False
|
return False
|
||||||
thread = data.get("data", {}).get("resolveReviewThread", {}).get("thread", {})
|
data_root = data.get("data") if isinstance(data, dict) else None
|
||||||
return bool(thread.get("isResolved"))
|
resolved = data_root.get("resolveReviewThread") if isinstance(data_root, dict) else None
|
||||||
|
thread = resolved.get("thread") if isinstance(resolved, dict) else None
|
||||||
|
return bool(thread.get("isResolved")) if isinstance(thread, dict) else False
|
||||||
|
|
||||||
|
|
||||||
async def reply_to_review_comment(
|
async def reply_to_review_comment(
|
||||||
|
|
|
||||||
|
|
@ -356,6 +356,24 @@ async def test_resolve_review_thread_returns_true_on_success() -> None:
|
||||||
assert ok is True
|
assert ok is True
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_fetch_pr_review_threads_handles_null_repository() -> None:
|
||||||
|
"""GitHub returns ``repository: null`` when the token can't read the repo
|
||||||
|
(SAML, expired token, private/deleted). ``dict.get(k, {})`` does not coalesce
|
||||||
|
explicit null, so the fetch must guard against it and return collected threads."""
|
||||||
|
response = MagicMock()
|
||||||
|
response.json.return_value = {"data": {"repository": None}}
|
||||||
|
response.raise_for_status.return_value = None
|
||||||
|
|
||||||
|
client_cm = AsyncMock()
|
||||||
|
client_cm.__aenter__.return_value = client_cm
|
||||||
|
client_cm.post = AsyncMock(return_value=response)
|
||||||
|
|
||||||
|
with patch("agent.reviewer_publish.httpx.AsyncClient", return_value=client_cm):
|
||||||
|
threads = await fetch_pr_review_threads(owner="o", repo="r", pr_number=1, token="t")
|
||||||
|
assert threads == []
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_post_pull_request_review_non_dict_body_surfaces_status_and_excerpt() -> None:
|
async def test_post_pull_request_review_non_dict_body_surfaces_status_and_excerpt() -> None:
|
||||||
"""A non-dict GitHub response body must surface status code + body excerpt
|
"""A non-dict GitHub response body must surface status code + body excerpt
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue