feat: attach screenshots to pull comments (Closes #515)

tests/test_authored_outbox.py

@@ -578,3 +578,29 @@ const outbox=createAuthoredOutbox({{storage,getOwnerLogin:()=>'timmy',background
 """
     output = run_node(script)
     assert output == {"error": "IndexedDB unavailable", "items": []}
+
+
+def test_pull_screenshot_comment_persists_blob_only_in_indexeddb():
+    script = f"""
+const createAuthoredOutbox=require({json.dumps(str(OUTBOX))});
+const values=new Map();const mirrors=[];
+const storage={{getItem:k=>values.get(k)||null,setItem:(k,v)=>values.set(k,v)}};
+const outbox=createAuthoredOutbox({{storage,getOwnerLogin:()=>'timmy',backgroundSync:{{
+  reconcile:async items=>mirrors.push(items),requestSync:async()=>{{}},
+}}}});
+(async()=>{{const blob=new Blob(['PULL-PRIVATE-BYTES'],{{type:'image/webp'}});
+  await outbox.enqueueDurably({{kind:'pull-comment',repository:'stackchain/web',number:31,
+    body:'Mobile proof',operationId:'pull-image-31',
+    attachment:{{filename:'proof.webp',contentType:'image/webp',blob}}}});
+  const local=values.get('stackchain.authored-outbox.v1');const durable=mirrors[0][0].attachment;
+  process.stdout.write(JSON.stringify({{local,metadata:JSON.parse(local).items[0].attachment,
+    durable:{{isBlob:durable.blob instanceof Blob,text:await durable.blob.text()}}}}));
+}})();
+"""
+    output = run_node(script)
+
+    assert "PULL-PRIVATE-BYTES" not in output["local"]
+    assert output["metadata"] == {
+        "filename": "proof.webp", "contentType": "image/webp", "stored": True
+    }
+    assert output["durable"] == {"isBlob": True, "text": "PULL-PRIVATE-BYTES"}

tests/test_background_issue_sync.py

@@ -237,6 +237,71 @@ const fetchJson=async(url,options={{}})=>{{
     assert output["second"]["confirmed"] == [{"id": 91}]
 
 
+def test_pull_screenshot_retry_uploads_to_pull_and_reuses_checkpoint_before_one_comment():
+    script = f"""
+const createBackgroundIssueSync=require({json.dumps(str(SYNC))});
+let item={{id:'pull-image',operationId:'pull-image',ownerLogin:'timmy',status:'queued',
+  kind:'pull-comment',repository:'stackchain/web',number:31,body:'Mobile proof',
+  attachment:{{filename:'proof.png',contentType:'image/png',blob:new Blob(['proof'],{{type:'image/png'}})}}}};
+const calls=[];let commentAttempts=0;
+const store={{claimNext:async()=>item?{{...item}}:null,update:async(_id,fn)=>{{item=fn(item);}},
+  complete:async()=>{{item=null;}},release:async()=>{{item={{...item,status:'queued'}};}},fail:async()=>{{}},countBlocked:async()=>0}};
+const fetchJson=async(url,options={{}})=>{{if(url==='api/v1/background-identity')return{{login:'timmy'}};
+  calls.push({{url,key:options.headers?.['Idempotency-Key'],body:options.body instanceof FormData?'multipart':JSON.parse(options.body)}});
+  if(url.endsWith('/attachments'))return{{markdown:'![proof.png](https://forge.example/proof.png)'}};
+  if(commentAttempts++===0){{const error=new Error('offline');error.status=503;throw error;}}return{{id:77}};
+}};
+(async()=>{{const sync=createBackgroundIssueSync({{store,fetchJson}});try{{await sync.flush();}}catch(_error){{}}
+  const checkpoint=item.attachmentMarkdown;const result=await sync.flush();
+  process.stdout.write(JSON.stringify({{calls,checkpoint,result}}));}})();
+"""
+    output = run_node(script)
+
+    assert output["checkpoint"].startswith("![proof.png]")
+    assert [call["url"] for call in output["calls"]] == [
+        "api/v1/repos/stackchain/web/pulls/31/attachments",
+        "api/v1/repos/stackchain/web/pulls/31/comments",
+        "api/v1/repos/stackchain/web/pulls/31/comments",
+    ]
+    assert [call["key"] for call in output["calls"]] == [
+        "pull-image:attachment", "pull-image:comment", "pull-image:comment"
+    ]
+    assert output["calls"][-1]["body"] == {
+        "body": "Mobile proof\n\n![proof.png](https://forge.example/proof.png)"
+    }
+    assert output["result"]["confirmed"] == [{"id": 77}]
+
+
+def test_pull_screenshot_comment_timeout_replays_one_upload_with_durable_stage_identity():
+    script = f"""
+const createBackgroundIssueSync=require({json.dumps(str(SYNC))});
+let item={{id:'pull-timeout',operationId:'pull-timeout',ownerLogin:'timmy',status:'queued',
+  kind:'pull-comment',repository:'stackchain/web',number:31,body:'Mobile proof',
+  attachment:{{filename:'proof.png',contentType:'image/png',blob:new Blob(['proof'],{{type:'image/png'}})}}}};
+const calls=[];let commentAttempts=0;
+const store={{claimNext:async()=>item?{{...item}}:null,update:async(_id,fn)=>{{item=fn(item);}},
+  complete:async()=>{{item=null;}},release:async()=>{{item={{...item,status:'queued'}};}},fail:async()=>{{}},countBlocked:async()=>0}};
+const fetchJson=async(url,options={{}})=>{{if(url==='api/v1/background-identity')return{{login:'timmy'}};
+  calls.push({{url,key:options.headers?.['Idempotency-Key']}});
+  if(url.endsWith('/attachments'))return{{markdown:'![proof.png](https://forge.example/proof.png)'}};
+  if(commentAttempts++===0)return new Promise(()=>{{}});return{{id:78}};
+}};
+(async()=>{{const sync=createBackgroundIssueSync({{store,fetchJson,requestTimeoutMs:5}});
+  let timedOut='';try{{await sync.flush();}}catch(error){{timedOut=error.message;}}
+  const checkpoint=item.attachmentMarkdown;const replay=await sync.flush();
+  process.stdout.write(JSON.stringify({{calls,timedOut,checkpoint,replay}}));}})();
+"""
+    output = run_node(script)
+
+    assert output["timedOut"] == "Background request timed out."
+    assert output["checkpoint"].startswith("![proof.png]")
+    assert [call["key"] for call in output["calls"]] == [
+        "pull-timeout:attachment", "pull-timeout:comment", "pull-timeout:comment"
+    ]
+    assert sum(call["url"].endswith("/attachments") for call in output["calls"]) == 1
+    assert output["replay"]["confirmed"] == [{"id": 78}]
+
+
 def test_closed_app_sync_retains_created_issue_for_create_and_start_recovery():
     script = f"""
 const createBackgroundIssueSync = require({json.dumps(str(SYNC))});

tests/test_issue_attachment_ui.py

@@ -406,6 +406,28 @@ Promise.resolve(pending).then(async()=>process.stdout.write(JSON.stringify({{
     }
 
 
+def test_pending_comment_admission_locks_attachment_replacement_and_remove_controls():
+    script = f"""
+const attachment=require({json.dumps(str(ATTACHMENT))});
+class Element {{
+  constructor() {{ this.listeners={{}};this.hidden=true;this.value='';this.files=[];this.textContent='';this.src='';this.disabled=false; }}
+  addEventListener(type,fn) {{ this.listeners[type]=fn; }}
+}}
+const input=new Element(),preview=new Element(),image=new Element(),meta=new Element(),remove=new Element(),status=new Element();
+const controller=attachment.mount({{input,preview,image,meta,remove,status,
+  createObjectURL:()=> 'blob:preview',revokeObjectURL:()=>{{}},upload:async()=>{{}}}});
+controller.setBusy(true);
+const pending={{input:input.disabled,remove:remove.disabled}};
+controller.setBusy(false);
+process.stdout.write(JSON.stringify({{pending,released:{{input:input.disabled,remove:remove.disabled}}}}));
+"""
+
+    assert json.loads(run_node(script)) == {
+        "pending": {"input": True, "remove": True},
+        "released": {"input": False, "remove": False},
+    }
+
+
 def test_issue_comment_actions_upload_binary_multipart_before_posting_and_clear_after_acceptance():
     source = DASHBOARD.read_text()
 
@@ -450,3 +472,50 @@ def test_issue_screenshot_comments_queue_serialized_bytes_before_clearing_or_adv
     assert "await authoredOutbox.enqueueDurably(message)" in source
     assert "await issueCommentNext.admit(item, message)" in source
     assert "navigator.onLine === false" in source
+
+
+def test_assigned_pull_composer_offers_screenshot_preview_remove_and_reuses_optimizer():
+    html = INDEX.read_text()
+    source = DASHBOARD.read_text()
+
+    assert 'id="pull-attachment"' in html
+    assert 'accept="image/png,image/jpeg,image/webp"' in html
+    assert 'for="pull-attachment"' in html
+    assert 'id="pull-attachment-preview"' in html
+    assert 'id="pull-attachment-image"' in html
+    assert 'id="remove-pull-attachment"' in html
+    assert "const pullAttachmentController = issueAttachment.mount({" in source
+    assert "pullAttachmentController.clear();" in source
+
+
+def test_retrying_same_pull_load_preserves_screenshot_but_target_change_clears_it():
+    source = DASHBOARD.read_text()
+    open_body = source.split("async function openPullSheet(item, trigger, offlineDetail = null) {", 1)[1].split(
+        "\n  function closePullSheet", 1
+    )[0]
+
+    assert "if (!sameWorkTarget(selectedPull, item)) pullAttachmentController.clear();" in open_body
+
+
+def test_pull_screenshot_comment_uploads_or_durably_admits_before_clearing_draft():
+    source = DASHBOARD.read_text()
+
+    assert "async function queuePullScreenshotComment" in source
+    assert "attachment: await pullAttachmentController.serialize()" in source
+    assert "await authoredOutbox.enqueueDurably(message)" in source
+    assert "navigator.onLine === false" in source
+    assert "Your comment and screenshot are safe; retry." in source
+
+
+def test_pull_screenshot_foreground_and_replay_share_the_durable_operation_pipeline():
+    source = DASHBOARD.read_text()
+    standard = source.split(
+        "qs('#send-pull-comment').addEventListener('click', async () => {", 1
+    )[1].split("\n  });", 1)[0]
+    comment_next = source.split("async function submitCommentAndNext(kind) {", 1)[1].split(
+        "\n  }\n  qs('#send-issue-comment-next')", 1
+    )[0]
+
+    assert "await queuePullScreenshotComment(item, body, operationId, false, true)" in standard
+    assert "pullAttachmentController.prepareComment" not in standard
+    assert "attachmentController.state() && (kind === 'pull' || navigator.onLine === false)" in comment_next

tests/test_issue_attachments.py

@@ -402,3 +402,73 @@ async def test_attachment_endpoint_rejects_unsafe_or_mismatched_filename(monkeyp
 
     assert [traversal.status_code, mismatch.status_code] == [422, 422]
     assert called is False
+
+
+@pytest.mark.anyio
+async def test_pull_attachment_endpoint_uploads_binary_to_exact_assigned_pull(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/pull.png",
+            "size": len(content),
+        }
+
+    monkeypatch.setattr(
+        main.gitea_proxy, "upload_assigned_pull_attachment", upload, 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/web/pulls/31/attachments",
+            files={"file": ("pull.png", PNG_BYTES, "image/png")},
+            headers={"Idempotency-Key": "pull-image-31:attachment"},
+        )
+        replay = await client.post(
+            "/api/v1/repos/stackchain/web/pulls/31/attachments",
+            files={"file": ("pull.png", PNG_BYTES, "image/png")},
+            headers={"Idempotency-Key": "pull-image-31:attachment"},
+        )
+
+    assert response.status_code == replay.status_code == 201
+    assert replay.json() == response.json()
+    assert response.json()["markdown"] == (
+        "![pull.png](<https://forge.example/attachments/pull.png>)"
+    )
+    assert calls == [("stackchain/web", 31, "pull.png", "image/png", PNG_BYTES)]
+
+
+@pytest.mark.anyio
+async def test_gitea_pull_attachment_revalidates_assignment_before_exact_pull_upload():
+    requests = []
+
+    async def handler(request):
+        requests.append(request)
+        if request.url.path == "/api/v1/user":
+            return httpx.Response(200, json={"login": "timmy"})
+        if request.method == "GET":
+            return httpx.Response(200, json={
+                "state": "open", "assignees": [{"login": "timmy"}],
+            })
+        return httpx.Response(201, json={
+            "name": "pull.png", "size": len(PNG_BYTES),
+            "browser_download_url": "https://forge.example/attachments/pull.png",
+        })
+
+    gitea_proxy.start_client(transport=httpx.MockTransport(handler))
+    try:
+        result = await gitea_proxy.upload_assigned_pull_attachment(
+            "stackchain/web", 31, "pull.png", "image/png", PNG_BYTES
+        )
+    finally:
+        await gitea_proxy.stop_client()
+
+    assert [(request.method, request.url.path) for request in requests] == [
+        ("GET", "/api/v1/user"),
+        ("GET", "/api/v1/repos/stackchain/web/pulls/31"),
+        ("POST", "/api/v1/repos/stackchain/web/issues/31/assets"),
+    ]
+    assert PNG_BYTES in requests[-1].content
+    assert result["url"] == "https://forge.example/attachments/pull.png"

tests/test_request_boundary.py

@@ -122,6 +122,15 @@ def test_request_limits_are_route_specific_and_cover_api_mutations():
     assert main.request_body_limit("POST", "/unrelated") is None
 
 
+def test_pull_screenshot_upload_uses_binary_attachment_boundary():
+    assert (
+        main.request_body_limit(
+            "POST", "/api/v1/repos/stackchain/project/pulls/31/attachments"
+        )
+        == 2 * 1024 * 1024 + 64 * 1024
+    )
+
+
 @pytest.mark.anyio
 async def test_request_limit_uses_application_path_under_domain_subpath():
     downstream_called = False