mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-10-07 10:29:05 +00:00
fix: update existing PR title and body when create returns 422 (#1163)
- Root cause: create_github_pr found an existing PR on 422 but never
PATCHed it, so callers like commit_and_open_pr could not update the
PR body (e.g. adding "Closes AB-1159") on subsequent invocations.
- Change: after _find_existing_pr succeeds, call new _update_github_pr
helper which PATCHes /repos/{owner}/{repo}/pulls/{number} with the
requested title and body before returning pr_existing=True.
- Verified: self-evident API call addition; proof in production traces.
Co-authored-by: LangSmith Forge <forge-agent@langsmith.ai>
Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
This commit is contained in:
parent
c6ea869df8
commit
8095f93364
2 changed files with 151 additions and 4 deletions
|
|
@ -222,16 +222,31 @@ async def create_github_pr(
|
||||||
github_token=token,
|
github_token=token,
|
||||||
head_branch=head_branch,
|
head_branch=head_branch,
|
||||||
)
|
)
|
||||||
if existing:
|
pr_url, pr_number = existing
|
||||||
|
if pr_url:
|
||||||
|
logger.info("Using existing PR for head branch: %s", pr_url)
|
||||||
|
updated = await _update_github_pr(
|
||||||
|
http_client=http_client,
|
||||||
|
repo_owner=repo_owner,
|
||||||
|
repo_name=repo_name,
|
||||||
|
github_token=token,
|
||||||
|
pr_number=pr_number,
|
||||||
|
title=title,
|
||||||
|
body=body,
|
||||||
|
)
|
||||||
|
if not updated:
|
||||||
|
if token != tokens_to_try[-1]:
|
||||||
|
logger.info("Retrying existing PR update with installation token")
|
||||||
|
continue
|
||||||
|
return None, None, False
|
||||||
await _add_label(
|
await _add_label(
|
||||||
http_client,
|
http_client,
|
||||||
repo_owner,
|
repo_owner,
|
||||||
repo_name,
|
repo_name,
|
||||||
label_tok,
|
label_tok,
|
||||||
existing[1],
|
pr_number,
|
||||||
)
|
)
|
||||||
logger.info("Using existing PR for head branch: %s", existing[0])
|
return pr_url, pr_number, True
|
||||||
return existing[0], existing[1], True
|
|
||||||
else:
|
else:
|
||||||
logger.debug(
|
logger.debug(
|
||||||
"Could not find existing PR with current token, will retry"
|
"Could not find existing PR with current token, will retry"
|
||||||
|
|
@ -353,6 +368,45 @@ async def _find_existing_pr(
|
||||||
return None, None
|
return None, None
|
||||||
|
|
||||||
|
|
||||||
|
async def _update_github_pr(
|
||||||
|
http_client: httpx.AsyncClient,
|
||||||
|
repo_owner: str,
|
||||||
|
repo_name: str,
|
||||||
|
github_token: str,
|
||||||
|
pr_number: int | None,
|
||||||
|
title: str,
|
||||||
|
body: str,
|
||||||
|
) -> bool:
|
||||||
|
"""Update an existing PR's title and body via PATCH."""
|
||||||
|
if pr_number is None:
|
||||||
|
logger.warning("Cannot update PR: pr_number is None")
|
||||||
|
return False
|
||||||
|
headers = {
|
||||||
|
"Authorization": f"Bearer {github_token}",
|
||||||
|
"Accept": "application/vnd.github+json",
|
||||||
|
"X-GitHub-Api-Version": "2022-11-28",
|
||||||
|
}
|
||||||
|
try:
|
||||||
|
response = await http_client.patch(
|
||||||
|
f"https://api.github.com/repos/{repo_owner}/{repo_name}/pulls/{pr_number}",
|
||||||
|
headers=headers,
|
||||||
|
json={"title": title, "body": body},
|
||||||
|
)
|
||||||
|
except httpx.HTTPError:
|
||||||
|
logger.warning("Failed to update PR #%s", pr_number, exc_info=True)
|
||||||
|
return False
|
||||||
|
if response.status_code == 200: # noqa: PLR2004
|
||||||
|
logger.info("Updated existing PR #%s with new title and body", pr_number)
|
||||||
|
return True
|
||||||
|
logger.warning(
|
||||||
|
"Failed to update PR #%s (%s): %s",
|
||||||
|
pr_number,
|
||||||
|
response.status_code,
|
||||||
|
response.json().get("message"),
|
||||||
|
)
|
||||||
|
return False
|
||||||
|
|
||||||
|
|
||||||
async def get_github_default_branch(
|
async def get_github_default_branch(
|
||||||
repo_owner: str,
|
repo_owner: str,
|
||||||
repo_name: str,
|
repo_name: str,
|
||||||
|
|
|
||||||
|
|
@ -46,6 +46,12 @@ class _FakeAsyncClient:
|
||||||
self._calls.append(("GET", url, params))
|
self._calls.append(("GET", url, params))
|
||||||
return self._responses.pop(0)
|
return self._responses.pop(0)
|
||||||
|
|
||||||
|
async def patch(
|
||||||
|
self, url: str, *, headers: dict[str, str], json: dict | None = None
|
||||||
|
) -> _FakeResponse:
|
||||||
|
self._calls.append(("PATCH", url, json))
|
||||||
|
return self._responses.pop(0)
|
||||||
|
|
||||||
|
|
||||||
class _RaiseOnLabelPostClient(_FakeAsyncClient):
|
class _RaiseOnLabelPostClient(_FakeAsyncClient):
|
||||||
"""Raises on the second POST (the label call) to simulate network failure."""
|
"""Raises on the second POST (the label call) to simulate network failure."""
|
||||||
|
|
@ -161,6 +167,7 @@ def test_create_pr_adds_label_on_existing_pr(monkeypatch: pytest.MonkeyPatch) ->
|
||||||
responses = [
|
responses = [
|
||||||
_FakeResponse(422, {"message": "A pull request already exists"}),
|
_FakeResponse(422, {"message": "A pull request already exists"}),
|
||||||
_FakeResponse(200, [{"html_url": "https://github.com/o/r/pull/7", "number": 7}]),
|
_FakeResponse(200, [{"html_url": "https://github.com/o/r/pull/7", "number": 7}]),
|
||||||
|
_FakeResponse(200, {"html_url": "https://github.com/o/r/pull/7", "number": 7}),
|
||||||
_FakeResponse(200, [{"name": "OpenSWE"}]),
|
_FakeResponse(200, [{"name": "OpenSWE"}]),
|
||||||
]
|
]
|
||||||
monkeypatch.setattr(github.httpx, "AsyncClient", lambda: _FakeAsyncClient(responses, calls))
|
monkeypatch.setattr(github.httpx, "AsyncClient", lambda: _FakeAsyncClient(responses, calls))
|
||||||
|
|
@ -180,12 +187,98 @@ def test_create_pr_adds_label_on_existing_pr(monkeypatch: pytest.MonkeyPatch) ->
|
||||||
|
|
||||||
assert result == ("https://github.com/o/r/pull/7", 7, True)
|
assert result == ("https://github.com/o/r/pull/7", 7, True)
|
||||||
assert calls[2] == (
|
assert calls[2] == (
|
||||||
|
"PATCH",
|
||||||
|
"https://api.github.com/repos/o/r/pulls/7",
|
||||||
|
{"title": "feat: test", "body": "body"},
|
||||||
|
)
|
||||||
|
assert calls[3] == (
|
||||||
"POST",
|
"POST",
|
||||||
"https://api.github.com/repos/o/r/issues/7/labels",
|
"https://api.github.com/repos/o/r/issues/7/labels",
|
||||||
{"labels": ["OpenSWE"]},
|
{"labels": ["OpenSWE"]},
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_create_pr_returns_failure_when_existing_pr_update_fails(
|
||||||
|
monkeypatch: pytest.MonkeyPatch,
|
||||||
|
) -> None:
|
||||||
|
calls: list[tuple[str, str, dict | None]] = []
|
||||||
|
responses = [
|
||||||
|
_FakeResponse(422, {"message": "A pull request already exists"}),
|
||||||
|
_FakeResponse(200, [{"html_url": "https://github.com/o/r/pull/7", "number": 7}]),
|
||||||
|
_FakeResponse(403, {"message": "Resource not accessible by integration"}),
|
||||||
|
]
|
||||||
|
monkeypatch.setattr(github.httpx, "AsyncClient", lambda: _FakeAsyncClient(responses, calls))
|
||||||
|
|
||||||
|
result = asyncio.run(
|
||||||
|
github.create_github_pr(
|
||||||
|
repo_owner="o",
|
||||||
|
repo_name="r",
|
||||||
|
github_token="token",
|
||||||
|
title="feat: test",
|
||||||
|
head_branch="feature",
|
||||||
|
base_branch="main",
|
||||||
|
body="body",
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
assert result == (None, None, False)
|
||||||
|
assert calls == [
|
||||||
|
(
|
||||||
|
"POST",
|
||||||
|
"https://api.github.com/repos/o/r/pulls",
|
||||||
|
{
|
||||||
|
"title": "feat: test",
|
||||||
|
"head": "feature",
|
||||||
|
"base": "main",
|
||||||
|
"body": "body",
|
||||||
|
"draft": True,
|
||||||
|
},
|
||||||
|
),
|
||||||
|
(
|
||||||
|
"GET",
|
||||||
|
"https://api.github.com/repos/o/r/pulls",
|
||||||
|
{"head": "o:feature", "state": "open", "per_page": 1},
|
||||||
|
),
|
||||||
|
(
|
||||||
|
"PATCH",
|
||||||
|
"https://api.github.com/repos/o/r/pulls/7",
|
||||||
|
{"title": "feat: test", "body": "body"},
|
||||||
|
),
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
def test_create_pr_retries_existing_pr_update_with_installation_token(
|
||||||
|
monkeypatch: pytest.MonkeyPatch,
|
||||||
|
) -> None:
|
||||||
|
calls: list[tuple[str, str, dict | None]] = []
|
||||||
|
responses = [
|
||||||
|
_FakeResponse(422, {"message": "A pull request already exists"}),
|
||||||
|
_FakeResponse(200, [{"html_url": "https://github.com/o/r/pull/7", "number": 7}]),
|
||||||
|
_FakeResponse(403, {"message": "Resource not accessible by integration"}),
|
||||||
|
_FakeResponse(422, {"message": "A pull request already exists"}),
|
||||||
|
_FakeResponse(200, [{"html_url": "https://github.com/o/r/pull/7", "number": 7}]),
|
||||||
|
_FakeResponse(200, {"html_url": "https://github.com/o/r/pull/7", "number": 7}),
|
||||||
|
_FakeResponse(200, [{"name": "OpenSWE"}]),
|
||||||
|
]
|
||||||
|
monkeypatch.setattr(github.httpx, "AsyncClient", lambda: _FakeAsyncClient(responses, calls))
|
||||||
|
|
||||||
|
result = asyncio.run(
|
||||||
|
github.create_github_pr(
|
||||||
|
repo_owner="o",
|
||||||
|
repo_name="r",
|
||||||
|
github_token="user-token",
|
||||||
|
title="feat: test",
|
||||||
|
head_branch="feature",
|
||||||
|
base_branch="main",
|
||||||
|
body="body",
|
||||||
|
installation_token="install-token",
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
assert result == ("https://github.com/o/r/pull/7", 7, True)
|
||||||
|
assert [call[0] for call in calls] == ["POST", "GET", "PATCH", "POST", "GET", "PATCH", "POST"]
|
||||||
|
|
||||||
|
|
||||||
def test_create_pr_succeeds_when_label_fails(monkeypatch: pytest.MonkeyPatch) -> None:
|
def test_create_pr_succeeds_when_label_fails(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||||
"""PR creation should succeed even if labeling fails."""
|
"""PR creation should succeed even if labeling fails."""
|
||||||
calls: list[tuple[str, str, dict | None]] = []
|
calls: list[tuple[str, str, dict | None]] = []
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue