Merge pull request 'Reconcile ambiguous merge outcomes after upstream timeouts' (#278) from timmy/277-reconcile-merge-timeouts into main

frontend/pull-sheet.js

@@ -139,7 +139,11 @@ function createPullSheet({ fetchJson, storage, createConversationPager = globalT
         headers: { Accept: 'application/json', 'Content-Type': 'application/json' },
         body: JSON.stringify({ expected_head_sha: expectedHeadSha }),
       }).then(result => {
-        if (!result?.merged) throw new Error('Pull request merge was not confirmed.');
+        if (!result?.merged) {
+          throw new Error(result?.confirmation_pending && result?.error
+            ? result.error
+            : 'Pull request merge was not confirmed.');
+        }
         return result;
       }).finally(() => { mergeRequest = null; });
       return mergeRequest;

src/gitea_proxy.py

@@ -1470,6 +1470,21 @@ async def pull_completion_review(repository: str, number: int) -> dict:
     }
 
 
+async def is_pull_merged_at_head(
+    repository: str, number: int, expected_head_sha: str
+) -> bool:
+    pull = await fetch(f"repos/{repository}/pulls/{number}")
+    if not isinstance(pull, dict):
+        return False
+    head_value = pull.get("head")
+    head = head_value if isinstance(head_value, dict) else {}
+    return (
+        pull.get("merged") is True
+        and pull.get("state") == "closed"
+        and head.get("sha") == expected_head_sha
+    )
+
+
 async def merge_assigned_pull(
     repository: str, number: int, expected_head_sha: str
 ) -> dict:

src/main.py

@@ -2068,9 +2068,26 @@ async def merge_assigned_pull(
             status_code=409,
         )
     except Exception:
+        try:
+            merged = await asyncio.wait_for(
+                gitea_proxy.is_pull_merged_at_head(
+                    repository, number, submission.expected_head_sha
+                ),
+                timeout=ISSUE_ACTION_TIMEOUT_SECONDS,
+            )
+        except Exception:
+            merged = False
+        if merged:
+            return {"number": number, "merged": True, "state": "closed"}
         return JSONResponse(
-            {"error": "The pull request could not be merged. It remains in My Work; please retry."},
+            {
-            status_code=503,
+                "number": number,
+                "merged": False,
+                "state": "unknown",
+                "confirmation_pending": True,
+                "error": "Merge confirmation is pending. Check its status before retrying.",
+            },
+            status_code=202,
             headers={"Retry-After": "1"},
         )
 

tests/test_my_work.py

@@ -2528,6 +2528,31 @@ Promise.all([first, duplicate]).then(async comments => {{
     assert len(output["comments"]) == 2 and len(output["merges"]) == 2
 
 
+def test_pull_sheet_surfaces_pending_merge_confirmation_guidance():
+    script = f"""
+const createPullSheet = require({json.dumps(str(PULL_SHEET))});
+const controller = createPullSheet({{
+  storage: null,
+  fetchJson: () => Promise.resolve({{
+    number: 7,
+    merged: false,
+    state: 'unknown',
+    confirmation_pending: true,
+    error: 'Merge confirmation is pending. Check its status before retrying.',
+  }}),
+}});
+controller.merge({{repository:'stackchain/api', number:7}}, 'abc123')
+  .then(() => process.stdout.write(JSON.stringify({{resolved:true}})))
+  .catch(error => process.stdout.write(JSON.stringify({{resolved:false, message:error.message}})));
+"""
+    result = subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True)
+
+    assert json.loads(result.stdout) == {
+        "resolved": False,
+        "message": "Merge confirmation is pending. Check its status before retrying.",
+    }
+
+
 def test_pull_sheet_enables_merge_only_for_current_safe_state():
     script = f"""
 const createPullSheet = require({json.dumps(str(PULL_SHEET))});

tests/test_pull_api.py

@@ -1,3 +1,5 @@
+import asyncio
+
 import httpx
 import pytest
 
@@ -247,6 +249,108 @@ async def test_assigned_pull_merge_requires_current_eligible_head(monkeypatch):
     assert calls == [("stackchain/api", 7, "abc123")]
 
 
+@pytest.mark.anyio
+async def test_assigned_pull_merge_reconciles_acceptance_before_timeout(monkeypatch):
+    calls = []
+    merged = False
+
+    async def assigned(repository, number):
+        return True
+
+    async def merge(repository, number, expected_head_sha):
+        nonlocal merged
+        calls.append(("merge", repository, number, expected_head_sha))
+        merged = True
+        await asyncio.sleep(0.05)
+        return {"number": number, "merged": True, "state": "closed"}
+
+    async def confirm(repository, number, expected_head_sha):
+        calls.append(("confirm", repository, number, expected_head_sha))
+        return merged
+
+    monkeypatch.setattr(main, "ISSUE_ACTION_TIMEOUT_SECONDS", 0.01)
+    monkeypatch.setattr(main.gitea_proxy, "is_assigned_pull", assigned)
+    monkeypatch.setattr(main.gitea_proxy, "merge_assigned_pull", merge)
+    monkeypatch.setattr(main.gitea_proxy, "is_pull_merged_at_head", confirm, raising=False)
+    transport = httpx.ASGITransport(app=main.app)
+    async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
+        response = await client.post(
+            "/api/v1/repos/stackchain/api/pulls/7/merge",
+            json={"expected_head_sha": "abc123"},
+        )
+
+    assert response.status_code == 200
+    assert response.json() == {"number": 7, "merged": True, "state": "closed"}
+    assert calls == [
+        ("merge", "stackchain/api", 7, "abc123"),
+        ("confirm", "stackchain/api", 7, "abc123"),
+    ]
+
+
+@pytest.mark.anyio
+async def test_gitea_merge_confirmation_requires_merged_expected_head():
+    requests = []
+
+    async def handler(request):
+        requests.append((request.method, request.url.path))
+        return httpx.Response(200, json={
+            "number": 7,
+            "state": "closed",
+            "merged": True,
+            "head": {"sha": "abc123"},
+        })
+
+    gitea_proxy.start_client(transport=httpx.MockTransport(handler))
+    try:
+        confirmed = await gitea_proxy.is_pull_merged_at_head(
+            "stackchain/api", 7, "abc123"
+        )
+    finally:
+        await gitea_proxy.stop_client()
+
+    assert confirmed is True
+    assert requests == [("GET", "/api/v1/repos/stackchain/api/pulls/7")]
+
+
+@pytest.mark.anyio
+async def test_assigned_pull_merge_reports_unresolved_confirmation_without_false_state(monkeypatch):
+    calls = []
+
+    async def assigned(repository, number):
+        return True
+
+    async def merge(repository, number, expected_head_sha):
+        calls.append(("merge", repository, number, expected_head_sha))
+        raise httpx.ReadTimeout("upstream response was lost")
+
+    async def confirm(repository, number, expected_head_sha):
+        calls.append(("confirm", repository, number, expected_head_sha))
+        return False
+
+    monkeypatch.setattr(main.gitea_proxy, "is_assigned_pull", assigned)
+    monkeypatch.setattr(main.gitea_proxy, "merge_assigned_pull", merge)
+    monkeypatch.setattr(main.gitea_proxy, "is_pull_merged_at_head", confirm)
+    transport = httpx.ASGITransport(app=main.app)
+    async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
+        response = await client.post(
+            "/api/v1/repos/stackchain/api/pulls/7/merge",
+            json={"expected_head_sha": "abc123"},
+        )
+
+    assert response.status_code == 202
+    assert response.json() == {
+        "number": 7,
+        "merged": False,
+        "state": "unknown",
+        "confirmation_pending": True,
+        "error": "Merge confirmation is pending. Check its status before retrying.",
+    }
+    assert calls == [
+        ("merge", "stackchain/api", 7, "abc123"),
+        ("confirm", "stackchain/api", 7, "abc123"),
+    ]
+
+
 @pytest.mark.anyio
 async def test_assigned_pull_merge_returns_conflict_without_mutating_stale_head(monkeypatch):
     async def assigned(repository, number):