diff --git a/agent/middleware/open_pr.py b/agent/middleware/open_pr.py index 28c786ea..19443b80 100644 --- a/agent/middleware/open_pr.py +++ b/agent/middleware/open_pr.py @@ -36,6 +36,7 @@ from ..utils.github import ( git_has_uncommitted_changes, git_has_unpushed_commits, git_push, + is_permanent_github_push_failure, ) from ..utils.github_app import get_github_app_installation_token from ..utils.github_token import get_github_token @@ -87,7 +88,11 @@ async def open_pr_if_needed( return None if pr_payload.get("success"): - # Tool already handled commit/push/PR creation + return None + + error = pr_payload.get("error") + if isinstance(error, str) and is_permanent_github_push_failure(error): + logger.info("Skipping PR safety net after permanent push failure") return None pr_title = pr_payload.get("title", "feat: Open SWE PR") diff --git a/agent/prompt.py b/agent/prompt.py index 5a8937ac..fc78bd1c 100644 --- a/agent/prompt.py +++ b/agent/prompt.py @@ -305,6 +305,8 @@ When you have completed your implementation, follow these steps in order: **IMPORTANT: Never claim a PR was created or updated unless `commit_and_open_pr` returned `success` and a PR link. If it returns "No changes detected" or any error, report that instead.** +**IMPORTANT: If `commit_and_open_pr` returns an error containing "403", "Permission denied", or "PERMANENT_FAILURE", this is a permanent authorization failure — the token does not have write access to the repository. Do NOT retry. Report the error to the user immediately and stop.** + 4. **Notify the source** immediately after `commit_and_open_pr` succeeds. Include a brief summary and the PR link: - Linear-triggered: use `linear_comment` with an `@mention` of the user who triggered the task - Slack-triggered: use `slack_thread_reply` diff --git a/agent/tools/commit_and_open_pr.py b/agent/tools/commit_and_open_pr.py index cd8e277e..ece7c02a 100644 --- a/agent/tools/commit_and_open_pr.py +++ b/agent/tools/commit_and_open_pr.py @@ -24,6 +24,7 @@ from ..utils.github import ( git_has_uncommitted_changes, git_has_unpushed_commits, git_push, + is_permanent_github_push_failure, ) from ..utils.github_app import get_github_app_installation_token from ..utils.github_token import get_github_token @@ -200,9 +201,20 @@ def commit_and_open_pr( push_result = git_push(sandbox_backend, repo_dir, target_branch) if push_result.exit_code != 0: + push_output = push_result.output.strip() + if is_permanent_github_push_failure(push_output): + return { + "success": False, + "error": ( + f"PERMANENT_FAILURE: do not retry. Git push was rejected with a 403 " + f"permission denied error — the token does not have write access to this " + f"repository. Report this to the user and stop. Details: {push_output}" + ), + "pr_url": None, + } return { "success": False, - "error": f"Git push failed: {push_result.output.strip()}", + "error": f"Git push failed: {push_output}", "pr_url": None, } diff --git a/agent/utils/github.py b/agent/utils/github.py index 643883c5..7c41bb6e 100644 --- a/agent/utils/github.py +++ b/agent/utils/github.py @@ -15,6 +15,17 @@ HTTP_CREATED = 201 HTTP_UNPROCESSABLE_ENTITY = 422 +def is_permanent_github_push_failure(output: str) -> bool: + """Return whether git push output indicates a permanent auth failure.""" + normalized_output = output.lower() + return ( + "permanent_failure" in normalized_output + or "403" in normalized_output + or "permission" in normalized_output + or "denied" in normalized_output + ) + + def _run_git( sandbox_backend: SandboxBackendProtocol, repo_dir: str, command: str ) -> ExecuteResponse: diff --git a/tests/test_commit_and_open_pr_403.py b/tests/test_commit_and_open_pr_403.py new file mode 100644 index 00000000..4fd5381f --- /dev/null +++ b/tests/test_commit_and_open_pr_403.py @@ -0,0 +1,128 @@ +"""Tests that commit_and_open_pr returns a PERMANENT_FAILURE message on 403 push errors.""" + +from unittest.mock import AsyncMock, MagicMock, patch + +from deepagents.backends.protocol import ExecuteResponse + + +def _make_push_result(exit_code: int, output: str) -> ExecuteResponse: + return ExecuteResponse(output=output, exit_code=exit_code, truncated=False) + + +def _make_config(thread_id: str = "test-thread") -> dict: + return { + "configurable": { + "thread_id": thread_id, + "repo": {"owner": "langchain-ai", "name": "docs"}, + }, + "metadata": {}, + } + + +@patch("agent.tools.commit_and_open_pr.get_config") +@patch("agent.tools.commit_and_open_pr.get_sandbox_backend_sync") +@patch("agent.tools.commit_and_open_pr.resolve_repo_dir", return_value="/repo/docs") +@patch("agent.tools.commit_and_open_pr.git_has_uncommitted_changes", return_value=False) +@patch("agent.tools.commit_and_open_pr.git_fetch_origin") +@patch("agent.tools.commit_and_open_pr.git_has_unpushed_commits", return_value=True) +@patch("agent.tools.commit_and_open_pr.git_current_branch", return_value="open-swe/test-thread") +@patch("agent.tools.commit_and_open_pr.git_checkout_branch", return_value=True) +@patch("agent.tools.commit_and_open_pr.git_config_user") +@patch("agent.tools.commit_and_open_pr.git_add_all") +@patch("agent.tools.commit_and_open_pr.get_github_token", return_value="ghp_token") +@patch( + "agent.tools.commit_and_open_pr.get_github_app_installation_token", + new_callable=AsyncMock, + return_value="ghs_token", +) +@patch("agent.tools.commit_and_open_pr.git_push") +def test_403_push_returns_permanent_failure( + mock_git_push, + mock_get_installation_token, + mock_get_token, + mock_git_add_all, + mock_git_config_user, + mock_git_checkout_branch, + mock_git_current_branch, + mock_git_has_unpushed, + mock_git_fetch_origin, + mock_git_has_uncommitted, + mock_resolve_repo_dir, + mock_get_sandbox, + mock_get_config, +) -> None: + from agent.tools.commit_and_open_pr import commit_and_open_pr + + mock_get_config.return_value = _make_config() + mock_get_sandbox.return_value = MagicMock() + mock_git_push.return_value = _make_push_result( + exit_code=128, + output=( + "remote: Permission to langchain-ai/docs.git denied to hinthornw.\n" + "fatal: unable to access 'https://github.com/langchain-ai/docs/': " + "The requested URL returned error: 403" + ), + ) + + result = commit_and_open_pr( + title="fix: something", body="## Description\nfoo\n\n## Test Plan\n- [ ] check" + ) + + assert result["success"] is False + assert result["pr_url"] is None + error = result["error"] + assert "PERMANENT_FAILURE" in error + assert "do not retry" in error + assert "403" in error + + +@patch("agent.tools.commit_and_open_pr.get_config") +@patch("agent.tools.commit_and_open_pr.get_sandbox_backend_sync") +@patch("agent.tools.commit_and_open_pr.resolve_repo_dir", return_value="/repo/docs") +@patch("agent.tools.commit_and_open_pr.git_has_uncommitted_changes", return_value=False) +@patch("agent.tools.commit_and_open_pr.git_fetch_origin") +@patch("agent.tools.commit_and_open_pr.git_has_unpushed_commits", return_value=True) +@patch("agent.tools.commit_and_open_pr.git_current_branch", return_value="open-swe/test-thread") +@patch("agent.tools.commit_and_open_pr.git_checkout_branch", return_value=True) +@patch("agent.tools.commit_and_open_pr.git_config_user") +@patch("agent.tools.commit_and_open_pr.git_add_all") +@patch("agent.tools.commit_and_open_pr.get_github_token", return_value="ghp_token") +@patch( + "agent.tools.commit_and_open_pr.get_github_app_installation_token", + new_callable=AsyncMock, + return_value="ghs_token", +) +@patch("agent.tools.commit_and_open_pr.git_push") +def test_non_403_push_failure_returns_regular_error( + mock_git_push, + mock_get_installation_token, + mock_get_token, + mock_git_add_all, + mock_git_config_user, + mock_git_checkout_branch, + mock_git_current_branch, + mock_git_has_unpushed, + mock_git_fetch_origin, + mock_git_has_uncommitted, + mock_resolve_repo_dir, + mock_get_sandbox, + mock_get_config, +) -> None: + from agent.tools.commit_and_open_pr import commit_and_open_pr + + mock_get_config.return_value = _make_config() + mock_get_sandbox.return_value = MagicMock() + mock_git_push.return_value = _make_push_result( + exit_code=1, + output="error: failed to push some refs to 'origin'", + ) + + result = commit_and_open_pr( + title="fix: something", body="## Description\nfoo\n\n## Test Plan\n- [ ] check" + ) + + assert result["success"] is False + assert result["pr_url"] is None + error = result["error"] + assert "PERMANENT_FAILURE" not in error + assert error.startswith("Git push failed:") diff --git a/tests/test_open_pr_middleware.py b/tests/test_open_pr_middleware.py index 8ade5ddb..d92e5e57 100644 --- a/tests/test_open_pr_middleware.py +++ b/tests/test_open_pr_middleware.py @@ -115,6 +115,44 @@ class TestOpenPrIfNeededMiddleware: assert result is None + @pytest.mark.asyncio + async def test_skips_when_commit_and_open_pr_failed_permanently(self) -> None: + payload = { + "success": False, + "error": ( + "PERMANENT_FAILURE: do not retry. Git push was rejected with a 403 " + "permission denied error." + ), + "pr_url": None, + } + state = self._make_state( + [ + ToolMessage( + content=json.dumps(payload), + tool_call_id="1", + name="commit_and_open_pr", + ) + ] + ) + + with ( + patch( + "agent.middleware.open_pr.get_config", + return_value={ + "configurable": { + "thread_id": "thread-permanent", + "repo": {"owner": "org", "name": "repo"}, + } + }, + ), + patch( + "agent.middleware.open_pr.get_sandbox_backend", new_callable=AsyncMock + ) as mock_sandbox, + ): + await open_pr_if_needed.aafter_agent(state, self._make_runtime()) + + mock_sandbox.assert_not_called() + @pytest.mark.asyncio async def test_proceeds_when_commit_and_open_pr_failed_git_push(self) -> None: """When success=False due to git push failure, safety net should attempt PR creation.""" @@ -244,7 +282,7 @@ class TestOpenPrIfNeededMiddleware: """ payload = { "success": False, - "error": "Git push failed: remote rejected (permission denied)", + "error": "Git push failed: remote contains work", "pr_url": None, } state = self._make_state(