diff --git a/agent/prompt.py b/agent/prompt.py index 5a9efd8a..ce916119 100644 --- a/agent/prompt.py +++ b/agent/prompt.py @@ -141,10 +141,10 @@ Make HTTP requests (GET, POST, PUT, DELETE, etc.) to APIs. Use this for API call Do not use this tool to create or update the pull request for completed code changes. Use `commit_and_open_pr` for that workflow so commits are pushed and GitHub authentication is handled correctly. For other PR-related actions, use the dedicated GitHub PR tools when available. #### `commit_and_open_pr` -Commits all changes, pushes to a branch, and opens a **draft** GitHub PR. If a PR already exists for the branch, it is reused and its existing title is preserved; the PR body may be refreshed to reflect the latest work. +Commits all changes, pushes to a branch, and opens a **draft** GitHub PR. If a PR already exists for the branch, it is reused and its existing title and description are preserved. Use this for submitting code changes, not for editing existing PR metadata. #### `edit_pull_request` -Edits the title and/or body of an existing GitHub Pull Request. Use this to deliberately update a PR title when the overall PR contents have changed enough that the existing title is stale or misleading, or to update a PR description after creation. Requires `pr_number` and at least one of `title` or `body`. +Edits the title and/or body of an existing GitHub Pull Request. Use this only when you deliberately need to change PR metadata after creation: pass `title` only to retitle, `body` only to rewrite the description, or both when both should change. Do not call this as part of the normal submit flow unless the current PR title or description is stale or misleading. Requires `pr_number` and at least one of `title` or `body`. #### `linear_comment` Posts a comment to a Linear ticket given a `ticket_id`. Call this **after** `commit_and_open_pr` to notify stakeholders that the work is done and include the PR link. You can tag Linear users with `@username` (their Linear display name). Example: "I've completed the implementation and opened a PR: . Hey @username, let me know if you have any feedback!". @@ -284,7 +284,7 @@ When you have completed your implementation, follow these steps in order: 2. **Review your changes**: Review the diff to ensure correctness. Verify no regressions or unintended modifications. 3. **Submit via `commit_and_open_pr` tool**: Call this tool as the final step. - If a PR already exists for the branch, `commit_and_open_pr` preserves the existing PR title. Do not use repeated `commit_and_open_pr` calls to retitle an existing PR; use `edit_pull_request` with `title` only when the current title is stale or misleading for the overall PR contents. + If a PR already exists for the branch, `commit_and_open_pr` preserves the existing PR title and description. Do not use repeated `commit_and_open_pr` calls to edit existing PR metadata; use `edit_pull_request` only when the current title or description is stale or misleading for the overall PR contents. **PR Title** (under 70 characters): ``` diff --git a/agent/tools/commit_and_open_pr.py b/agent/tools/commit_and_open_pr.py index aa74895f..5c9eb0bf 100644 --- a/agent/tools/commit_and_open_pr.py +++ b/agent/tools/commit_and_open_pr.py @@ -53,6 +53,9 @@ def commit_and_open_pr( You MUST call this tool when you have completed your work and want to submit your changes for review. This is the final step in your workflow. + If a PR already exists for the branch, this tool reuses it and preserves + the existing PR title and description. To intentionally edit existing PR + metadata, use `edit_pull_request` after this succeeds. Before calling this tool, ensure you have: 1. Reviewed your changes for correctness diff --git a/agent/tools/edit_pull_request.py b/agent/tools/edit_pull_request.py index fc33062e..968bfd71 100644 --- a/agent/tools/edit_pull_request.py +++ b/agent/tools/edit_pull_request.py @@ -18,7 +18,9 @@ def edit_pull_request( ) -> dict[str, Any]: """Edit the title and/or body of an existing GitHub Pull Request. - Use this tool to update a PR's title or description after it has been created. + Use this tool only when you intentionally need to update a PR's title or + description after it has been created. Normal code submission should use + `commit_and_open_pr`, which preserves existing PR metadata when reusing a PR. At least one of `title` or `body` must be provided. Args: diff --git a/agent/utils/github.py b/agent/utils/github.py index 7649b621..4593c5bb 100644 --- a/agent/utils/github.py +++ b/agent/utils/github.py @@ -233,7 +233,6 @@ async def create_github_pr( repo_name=repo_name, github_token=token, pr_number=pr_number, - body=body, ) if not updated: if token != tokens_to_try[-1]: @@ -425,22 +424,27 @@ async def _update_github_pr( repo_name: str, github_token: str, pr_number: int | None, - body: str, title: str | None = None, + body: str | None = None, ) -> bool: """Update an existing PR via PATCH.""" if pr_number is None: logger.warning("Cannot update PR: pr_number is None") return False + payload: dict[str, str] = {} + if title is not None: + payload["title"] = title + if body is not None: + payload["body"] = body + if not payload: + logger.info("No existing PR fields to update for PR #%s", pr_number) + return True headers = { "Authorization": f"Bearer {github_token}", "Accept": "application/vnd.github+json", "X-GitHub-Api-Version": "2022-11-28", } try: - payload = {"body": body} - if title is not None: - payload["title"] = title response = await http_client.patch( f"https://api.github.com/repos/{repo_owner}/{repo_name}/pulls/{pr_number}", headers=headers, diff --git a/tests/test_github_create_pr.py b/tests/test_github_create_pr.py index c75a4a21..47620eac 100644 --- a/tests/test_github_create_pr.py +++ b/tests/test_github_create_pr.py @@ -10,6 +10,45 @@ import pytest from agent.utils.github import create_github_pr +@pytest.mark.asyncio +async def test_create_github_pr_existing_pr_preserves_description(): + """Existing PRs should be reused without overwriting their title or body.""" + existing_pr_url = "https://github.com/owner/repo/pull/42" + existing_pr_number = 42 + create_response = MagicMock() + create_response.status_code = 422 + create_response.json.return_value = {"message": "Validation Failed"} + + mock_client = MagicMock() + mock_client.post = AsyncMock(return_value=create_response) + mock_client.patch = AsyncMock() + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock(return_value=False) + + with ( + patch("httpx.AsyncClient", return_value=mock_client), + patch( + "agent.utils.github._find_existing_pr", + AsyncMock(return_value=(existing_pr_url, existing_pr_number)), + ), + patch("agent.utils.github._add_label", AsyncMock()), + ): + pr_url, pr_number, pr_existing = await create_github_pr( + repo_owner="owner", + repo_name="repo", + github_token="token", + title="Test PR", + head_branch="feature/test", + base_branch="main", + body="new generated body", + ) + + assert pr_url == existing_pr_url + assert pr_number == existing_pr_number + assert pr_existing is True + mock_client.patch.assert_not_awaited() + + @pytest.mark.asyncio async def test_create_github_pr_http_error_falls_back_to_existing_pr(): """When httpx.HTTPError is raised during PR creation, the function should diff --git a/tests/test_github_pr_label.py b/tests/test_github_pr_label.py index 065b7fe8..88092366 100644 --- a/tests/test_github_pr_label.py +++ b/tests/test_github_pr_label.py @@ -167,7 +167,6 @@ def test_create_pr_adds_label_on_existing_pr(monkeypatch: pytest.MonkeyPatch) -> responses = [ _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)) @@ -187,18 +186,14 @@ def test_create_pr_adds_label_on_existing_pr(monkeypatch: pytest.MonkeyPatch) -> assert result == ("https://github.com/o/r/pull/7", 7, True) assert calls[2] == ( - "PATCH", - "https://api.github.com/repos/o/r/pulls/7", - {"body": "body"}, - ) - assert calls[3] == ( "POST", "https://api.github.com/repos/o/r/issues/7/labels", {"labels": ["OpenSWE"]}, ) + assert [call[0] for call in calls] == ["POST", "GET", "POST"] -def test_create_pr_returns_failure_when_existing_pr_update_fails( +def test_create_pr_returns_existing_pr_when_existing_pr_label_fails( monkeypatch: pytest.MonkeyPatch, ) -> None: calls: list[tuple[str, str, dict | None]] = [] @@ -221,7 +216,7 @@ def test_create_pr_returns_failure_when_existing_pr_update_fails( ) ) - assert result == (None, None, False) + assert result == ("https://github.com/o/r/pull/7", 7, True) assert calls == [ ( "POST", @@ -240,24 +235,20 @@ def test_create_pr_returns_failure_when_existing_pr_update_fails( {"head": "o:feature", "state": "open", "per_page": 1}, ), ( - "PATCH", - "https://api.github.com/repos/o/r/pulls/7", - {"body": "body"}, + "POST", + "https://api.github.com/repos/o/r/issues/7/labels", + {"labels": ["OpenSWE"]}, ), ] -def test_create_pr_retries_existing_pr_update_with_installation_token( +def test_create_pr_preserves_existing_pr_metadata_without_token_retry( 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)) @@ -276,7 +267,7 @@ def test_create_pr_retries_existing_pr_update_with_installation_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"] + assert [call[0] for call in calls] == ["POST", "GET", "POST"] def test_create_pr_succeeds_when_label_fails(monkeypatch: pytest.MonkeyPatch) -> None: