stackchain/stackchain-dashboard
1
0
Fork 0
Code Issues 9 Pull Requests 1 Actions Packages Projects Releases 713 Wiki Activity

Make screenshot uploads retry-safe #472

Merged
timmy merged 1 commits from timmy/471-idempotent-attachment-uploads into main 2026-08-10 09:56:37 +00:00
Conversation 0 Commits 1 Files Changed 5 +190 -10
5 changed files with 190 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
frontend/dashboard.js
View File

@ -293,7 +293,11 @@
'api/v1/repos/' + repository + '/issues/' + encodeURIComponent(payload.number) + '/attachments',
{
method: 'POST',
headers: { Accept: 'application/json', 'Content-Type': 'application/json' },
headers: {
Accept: 'application/json',
'Content-Type': 'application/json',
'Idempotency-Key': payload.operation_id,
},
body: JSON.stringify({
filename: payload.filename,
content_type: payload.content_type,

10
frontend/issue-attachment.js
View File

@ -11,9 +11,16 @@
function create(options) {
const readDataUrl = options.readDataUrl;
const upload = options.upload;
const createOperationId = options.createOperationId || (() => {
if (typeof globalThis !== 'undefined' && globalThis.crypto?.randomUUID) {
return globalThis.crypto.randomUUID();
}
return Date.now().toString(36) + '-' + Math.random().toString(36).slice(2);
});
let selected = null;
let confirmed = null;
let serialized = null;
let operationId = null;
function select(file) {
if (!file || !IMAGE_TYPES.has(file.type)) {
@ -25,6 +32,7 @@
selected = file;
confirmed = null;
serialized = null;
operationId = createOperationId();
return state();
}
@ -32,6 +40,7 @@
selected = null;
confirmed = null;
serialized = null;
operationId = null;
}
function restore(value) {
@ -80,6 +89,7 @@
filename: attachment.filename,
content_type: attachment.contentType,
data: attachment.data,
operation_id: operationId,
});
if (!confirmed || typeof confirmed.markdown !== 'string' || !confirmed.markdown) {
confirmed = null;

33
src/main.py
View File

@ -1,6 +1,7 @@
import asyncio
import base64
import binascii
import hashlib
import hmac
import math
import os
@ -2776,20 +2777,40 @@ async def attach_to_assigned_issue(
owner: str,
repo: str,
number: int = PathParam(gt=0),
idempotency_key: str | None = Header(default=None, max_length=128),
):
repository = f"{owner}/{repo}"
try:
content = attachment.content()
except ValueError as exc:
raise HTTPException(status_code=422, detail=str(exc)) from exc
async def upload_attachment():
result = await gitea_proxy.upload_assigned_issue_attachment(
repository,
number,
attachment.filename,
attachment.content_type,
content,
)
safe_name = (
result["name"].replace("\\", "\\\\").replace("[", "\\[").replace("]", "\\]")
.replace("\r", " ").replace("\n", " ")
)
safe_url = result["url"].replace("<", "%3C").replace(">", "%3E")
result["markdown"] = f"![{safe_name}](<{safe_url}>)"
return result
try:
result = await asyncio.wait_for(
gitea_proxy.upload_assigned_issue_attachment(
result = await _run_idempotent_authored_action(
upload_attachment(),
idempotency_key=idempotency_key,
fingerprint=(
"issue-attachment",
repository,
number,
attachment.filename,
attachment.content_type,
content,
hashlib.sha256(content).hexdigest(),
),
timeout=ISSUE_ACTION_TIMEOUT_SECONDS,
)
@ -2803,12 +2824,6 @@ async def attach_to_assigned_issue(
status_code=503,
headers={"Retry-After": "1"},
)
safe_name = (
result["name"].replace("\\", "\\\\").replace("[", "\\[").replace("]", "\\]")
.replace("\r", " ").replace("\n", " ")
)
safe_url = result["url"].replace("<", "%3C").replace(">", "%3E")
result["markdown"] = f"![{safe_name}](<{safe_url}>)"
return JSONResponse(result, status_code=201)

29
tests/test_issue_attachment_ui.py
View File

@ -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():

122
tests/test_issue_attachments.py
View File

@ -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))