fix: restructure PR creation to POST-first with diagnose-on-failure

Move preflight checks from an authoritative gate (before POST) to a
diagnostic run after POST failure. This avoids false-positive failures
when a just-pushed head branch is momentarily invisible to GitHub ref
endpoints, and eliminates 2-3 extra serial API round-trips on the happy
path.

Also drop unused _PR_CREATED_FALSE indirection and add a docstring to
pr_creation_guard acknowledging the fail-open detection design.

agent/middleware/pr_creation_guard.py

@@ -179,6 +179,18 @@ def _contains_direct_pull_create(tokens: list[str]) -> bool:
 
 
 def is_pr_creation_fallback_command(command: str) -> bool:
+    """Return True when *command* is a known PR-creation shell fallback.
+
+    Detection is literal-token matching — it catches the primary vectors
+    (``gh pr create``, ``gh api repos/.../pulls -X POST``, ``curl`` to
+    ``/pulls``) but is intentionally fail-open: shell aliases, ``gh``
+    aliases (``gh prc`` set via ``gh alias set``), flags between ``pr`` and
+    ``create`` (``gh pr --repo x create``), and non-curl HTTP clients
+    (``python -c …``, ``wget``, etc.) will not be blocked. This is
+    acceptable because the threat model is an honest agent papering over an
+    ``open_pull_request`` failure, not an adversary trying to bypass the
+    guardrail.
+    """
     tokens = _shell_tokens(command)
     return (
         _contains_gh_pr_create(tokens)

agent/tools/open_pull_request.py

@@ -25,7 +25,6 @@ _REFERENCES_HEADING = "## References"
 _ACCESS_FAILURE_CODE = "github_app_access_missing_or_repo_not_found"
 _BRANCH_FAILURE_CODE = "github_pr_branch_not_visible"
 _PREFLIGHT_FAILURE_CODE = "github_pr_preflight_failed"
-_PR_CREATED_FALSE = False
 
 
 async def _resolve_pr_author_token() -> tuple[str | None, str]:
@@ -140,7 +139,7 @@ def _failure_payload(
         "token_kind": token_kind,
         "http_status": http_status,
         "branch_pushed": branch_pushed,
-        "pr_created": _PR_CREATED_FALSE,
+        "pr_created": False,
         "failed_step": failed_step,
         "likely_cause": likely_cause,
         "suggested_action": suggested_action,
@@ -281,6 +280,12 @@ async def _preflight_pr_access(
     head: str,
     base: str,
 ) -> dict[str, Any] | None:
+    """Diagnose *why* a PR creation POST failed — not an authoritative gate.
+
+    This runs only after the POST returns a non-201/non-422 status so it
+    doesn't add latency on the happy path and avoids false positives from
+    read-after-write inconsistency on just-pushed head branches.
+    """
     repo_resp = await _github_get(client, token, f"/repos/{owner}/{repo}")
     if repo_resp.status_code in {403, 404}:
         return _access_failure_payload(
@@ -637,17 +642,6 @@ async def _open_pull_request(
         )
 
     async with httpx.AsyncClient(timeout=30.0) as client:
-        preflight_failure = await _preflight_pr_access(
-            client=client,
-            token=token,
-            token_kind=kind,
-            owner=owner,
-            repo=repo,
-            head=head,
-            base=base,
-        )
-        if preflight_failure is not None:
-            return preflight_failure
         body = await _maybe_append_references(client, token, owner, repo, body)
         payload = {"title": title, "head": head, "base": base, "body": body, "draft": draft}
         resp = await client.post(
@@ -699,6 +693,25 @@ async def _open_pull_request(
                     "token_kind": kind,
                 }
 
+        # POST failed — run preflight diagnostics to understand why, so the
+        # agent gets an actionable diagnosis instead of a raw HTTP status.
+        # Preflight runs *after* the POST so a just-pushed branch that is
+        # momentarily invisible to GitHub's ref endpoints does not cause a
+        # false-positive preflight failure on what would have been a successful
+        # PR creation.
+        diagnostic = await _preflight_pr_access(
+            client=client,
+            token=token,
+            token_kind=kind,
+            owner=owner,
+            repo=repo,
+            head=head,
+            base=base,
+        )
+        if diagnostic is not None:
+            return diagnostic
+
+        # Preflight found nothing — surface the raw POST failure.
         if resp.status_code == 404:
             return _access_failure_payload(
                 owner=owner,

tests/test_open_pull_request.py

@@ -322,10 +322,13 @@ def test_404_create_returns_actionable_access_diagnostic(
 def test_preflight_head_branch_404_reports_branch_not_pushed(
     monkeypatch: pytest.MonkeyPatch,
 ) -> None:
+    """Preflight runs as a diagnostic after the POST fails — the POST is attempted
+    first so a just-pushed branch that momentarily 404s from GitHub's ref
+    endpoints doesn't cause a false-positive preflight failure."""
     _set_config(monkeypatch, {"source": "slack", "github_login": "johannes117"})
     _stub_token(monkeypatch)
     client = _RoutingClient(
-        post=_FakeResponse(201, {"html_url": "u", "number": 1, "user": {}}),
+        post=_FakeResponse(422, {"message": "Validation failed"}),
         get_routes={
             "/repos/langchain-ai/open-swe/branches/main": _FakeResponse(200, {"name": "main"}),
             "/repos/langchain-ai/open-swe/branches/open-swe%2Ffeature": _FakeResponse(
@@ -343,7 +346,7 @@ def test_preflight_head_branch_404_reports_branch_not_pushed(
     assert result["branch_pushed"] is False
     assert result["head_branch_visible"] is False
     assert result["failed_step"] == "preflight_head_branch"
-    assert client.post_calls == []
+    assert len(client.post_calls) == 1
 
 
 async def _coro(value: Any) -> Any:
@@ -617,3 +620,44 @@ def test_derive_pr_state_draft() -> None:
 
 def test_derive_pr_state_open() -> None:
     assert opr.derive_pr_state(state="open", merged=False, draft=False) == "open"
+
+
+def test_happy_path_skips_preflight_branch_gets(monkeypatch: pytest.MonkeyPatch) -> None:
+    """POST-first: when the PR is created successfully no preflight GETs
+    are made to /branches/... endpoints — only reference / plan checks."""
+    _set_config(monkeypatch, {"source": "slack", "github_login": "johannes117"})
+    monkeypatch.setattr(opr, "_resolve_pr_author_token", lambda: _coro(("tok", "user")))
+
+    client = _RoutingClient(
+        post=_FakeResponse(201, {"html_url": "u", "number": 1, "user": {}}),
+        get_routes={"/repos/langchain-ai/open-swe": _FakeResponse(200, {"private": False})},
+    )
+    _install_client(monkeypatch, client)
+
+    result = _open()
+
+    assert result["success"] is True
+    branch_gets = [c for c in client.get_calls if "/branches/" in c["url"]]
+    assert branch_gets == []
+
+
+def test_post_failure_diagnoses_via_preflight(monkeypatch: pytest.MonkeyPatch) -> None:
+    """When the POST fails with a non-201/non-422 status, the preflight runs
+    as a diagnostic and its result is returned (not the raw POST failure)."""
+    _set_config(monkeypatch, {"source": "slack", "github_login": "johannes117"})
+    _stub_token(monkeypatch)
+
+    client = _RoutingClient(
+        post=_FakeResponse(403, {"message": "Resource not accessible"}),
+        get_routes={
+            "/repos/langchain-ai/open-swe": _FakeResponse(403, {"message": "Not Found"}),
+        },
+    )
+    _install_client(monkeypatch, client)
+
+    result = _open()
+
+    assert result["success"] is False
+    assert result["code"] == "github_app_access_missing_or_repo_not_found"
+    assert result["failed_step"] == "preflight_repo"
+    assert result["repo_visible"] is False