fix: make screenshot uploads retry-safe (#471)

tests/test_issue_attachment_ui.py

@@ -24,6 +24,7 @@ const attachment = require({json.dumps(str(ATTACHMENT))});
 const calls = [];
 const file = {{name:'checkout.png', type:'image/png', size:8}};
 const controller = attachment.create({{
+  createOperationId: () => 'attachment-comment-471',
   readDataUrl: async selected => {{ calls.push('read:' + selected.name); return 'data:image/png;base64,iVBORw0KGgo='; }},
   upload: async payload => {{ calls.push(payload); return {{markdown:'![checkout.png](https://forge.example/a.png)'}}; }},
 }});
@@ -47,6 +48,7 @@ controller.select(file);
             "filename": "checkout.png",
             "content_type": "image/png",
             "data": "iVBORw0KGgo=",
+            "operation_id": "attachment-comment-471",
         },
     ]
     assert output["state"] == {
@@ -54,6 +56,32 @@ controller.select(file);
     }
 
 
+def test_mobile_attachment_retry_reuses_operation_key_until_file_changes():
+    script = f"""
+const attachment = require({json.dumps(str(ATTACHMENT))});
+const keys=[];
+let sequence=0;
+let fail=true;
+const controller=attachment.create({{
+  createOperationId:()=> 'attachment-' + (++sequence),
+  readDataUrl:async file=>'data:'+file.type+';base64,iVBORw0KGgo=',
+  upload:async payload=>{{keys.push(payload.operation_id);if(fail){{fail=false;throw new Error('offline');}}return {{markdown:'![ok](https://forge.example/a)'}};}},
+}});
+controller.select({{name:'first.png',type:'image/png',size:8}});
+(async()=>{{
+  try{{await controller.prepareComment({{repository:'stackchain/api',number:17}},'Evidence');}}catch(_error){{}}
+  await controller.prepareComment({{repository:'stackchain/api',number:17}},'Evidence');
+  controller.select({{name:'replacement.png',type:'image/png',size:8}});
+  await controller.prepareComment({{repository:'stackchain/api',number:17}},'Replacement');
+  process.stdout.write(JSON.stringify(keys));
+}})().catch(error=>{{console.error(error);process.exit(1);}});
+"""
+
+    assert json.loads(run_node(script)) == [
+        "attachment-1", "attachment-1", "attachment-2"
+    ]
+
+
 def test_selected_screenshot_serializes_for_durable_issue_capture_without_uploading():
     script = f"""
 const attachment = require({json.dumps(str(ATTACHMENT))});
@@ -165,6 +193,7 @@ def test_issue_comment_actions_upload_before_posting_and_clear_after_acceptance(
     assert "issueAttachmentController.prepareComment(item, body)" in source
     assert "issueAttachmentController.prepareComment(selectedIssue, body)" in source
     assert source.count("issueAttachmentController.clear();") >= 2
+    assert "'Idempotency-Key': payload.operation_id" in source
 
 
 def test_closing_issue_sheet_cannot_carry_a_screenshot_to_another_issue():

tests/test_issue_attachments.py

@@ -1,3 +1,4 @@
+import asyncio
 import base64
 
 import httpx
@@ -9,6 +10,15 @@ from src import gitea_proxy, main
 PNG_BYTES = b"\x89PNG\r\n\x1a\n" + b"mobile screenshot"
 
 
+@pytest.fixture(autouse=True)
+def clear_attachment_idempotency():
+    main._authored_action_operations.clear()
+    main._idempotency_ledger.clear()
+    yield
+    main._authored_action_operations.clear()
+    main._idempotency_ledger.clear()
+
+
 @pytest.mark.anyio
 async def test_attachment_endpoint_uploads_valid_screenshot_to_assigned_issue(monkeypatch):
     calls = []
@@ -48,6 +58,118 @@ async def test_attachment_endpoint_uploads_valid_screenshot_to_assigned_issue(mo
     ]
 
 
+@pytest.mark.anyio
+async def test_attachment_endpoint_replays_confirmed_upload_for_same_key(monkeypatch):
+    calls = []
+
+    async def upload(repository, number, filename, content_type, content):
+        calls.append((repository, number, filename, content_type, content))
+        return {
+            "name": filename,
+            "url": "https://forge.example/attachments/checkout.png",
+            "size": len(content),
+        }
+
+    monkeypatch.setattr(main.gitea_proxy, "upload_assigned_issue_attachment", upload)
+    payload = {
+        "filename": "checkout.png",
+        "content_type": "image/png",
+        "data": base64.b64encode(PNG_BYTES).decode("ascii"),
+    }
+    transport = httpx.ASGITransport(app=main.app)
+    async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
+        first = await client.post(
+            "/api/v1/repos/stackchain/api/issues/17/attachments",
+            json=payload,
+            headers={"Idempotency-Key": "attachment-retry-471"},
+        )
+        replay = await client.post(
+            "/api/v1/repos/stackchain/api/issues/17/attachments",
+            json=payload,
+            headers={"Idempotency-Key": "attachment-retry-471"},
+        )
+
+    assert first.status_code == replay.status_code == 201
+    assert replay.json() == first.json()
+    assert len(calls) == 1
+
+
+@pytest.mark.anyio
+async def test_attachment_endpoint_coalesces_concurrent_upload_retries(monkeypatch):
+    calls = 0
+
+    async def upload(_repository, _number, filename, _content_type, content):
+        nonlocal calls
+        calls += 1
+        await asyncio.sleep(0.05)
+        return {
+            "name": filename,
+            "url": "https://forge.example/attachments/checkout.png",
+            "size": len(content),
+        }
+
+    monkeypatch.setattr(main.gitea_proxy, "upload_assigned_issue_attachment", upload)
+    payload = {
+        "filename": "checkout.png",
+        "content_type": "image/png",
+        "data": base64.b64encode(PNG_BYTES).decode("ascii"),
+    }
+    transport = httpx.ASGITransport(app=main.app)
+    async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
+        first, retry = await asyncio.gather(*[
+            client.post(
+                "/api/v1/repos/stackchain/api/issues/17/attachments",
+                json=payload,
+                headers={"Idempotency-Key": "attachment-concurrent-471"},
+            )
+            for _ in range(2)
+        ])
+
+    assert first.status_code == retry.status_code == 201
+    assert first.json() == retry.json()
+    assert calls == 1
+
+
+@pytest.mark.anyio
+async def test_attachment_endpoint_rejects_changed_upload_for_used_key(monkeypatch):
+    calls = 0
+
+    async def upload(_repository, _number, filename, _content_type, content):
+        nonlocal calls
+        calls += 1
+        return {
+            "name": filename,
+            "url": "https://forge.example/attachments/checkout.png",
+            "size": len(content),
+        }
+
+    monkeypatch.setattr(main.gitea_proxy, "upload_assigned_issue_attachment", upload)
+    transport = httpx.ASGITransport(app=main.app)
+    async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
+        accepted = await client.post(
+            "/api/v1/repos/stackchain/api/issues/17/attachments",
+            json={
+                "filename": "checkout.png",
+                "content_type": "image/png",
+                "data": base64.b64encode(PNG_BYTES).decode("ascii"),
+            },
+            headers={"Idempotency-Key": "attachment-conflict-471"},
+        )
+        conflict = await client.post(
+            "/api/v1/repos/stackchain/api/issues/17/attachments",
+            json={
+                "filename": "checkout.png",
+                "content_type": "image/png",
+                "data": base64.b64encode(PNG_BYTES + b" changed").decode("ascii"),
+            },
+            headers={"Idempotency-Key": "attachment-conflict-471"},
+        )
+
+    assert accepted.status_code == 201
+    assert conflict.status_code == 409
+    assert calls == 1
+
+
 @pytest.mark.anyio
 async def test_attachment_endpoint_admits_a_normal_phone_screenshot(monkeypatch):
     screenshot = b"\x89PNG\r\n\x1a\n" + (b"x" * (100 * 1024))