diff --git a/README.md b/README.md index dcb1700..f268f7e 100644 --- a/README.md +++ b/README.md @@ -19,8 +19,8 @@ threads, create and self-assign issues, discover, claim, and release issue assig list repository labels and open milestones, set or clear due dates on assigned issues, create issue comments, close assigned issues, inspect/comment on assigned pull requests, merge assigned pull requests, and submit pull-request reviews. -Assigned-issue comments can include one PNG, JPEG, or WebP screenshot; the mobile composer automatically optimizes oversized screenshots on-device to fit the 2 MB upload boundary while leaving already-valid files unchanged. -For online delivery, the screenshot uploads before the comment is posted; validation or upload +Assigned-issue and assigned-pull-request comments can include one PNG, JPEG, or WebP screenshot; each mobile composer automatically optimizes oversized screenshots on-device to fit the 2 MB upload boundary while leaving already-valid files unchanged. +For online delivery, the screenshot uploads before the comment is posted—to the exact assigned issue or pull request—and produces one Markdown comment; validation or upload failures keep both the typed comment and removable preview available for retry. Offline screenshot comments admit their text and image bytes to IndexedDB before confirmation, keep only bounded metadata in localStorage, and use checkpointed upload/comment identities so reconnect retries cannot duplicate diff --git a/frontend/authored-outbox.js b/frontend/authored-outbox.js index b48c27f..68f1e28 100644 --- a/frontend/authored-outbox.js +++ b/frontend/authored-outbox.js @@ -75,7 +75,7 @@ function createAuthoredOutbox({ storage, fetchJson, coordinator, backgroundSync, status: 'queued', queuedAt: Number(now()), ...(message.kind === 'update-reply-read' ? { replyConfirmed: message.replyConfirmed === true } : {}), - ...(message.kind === 'issue-comment' && message.attachment ? { + ...(['issue-comment', 'pull-comment'].includes(message.kind) && message.attachment ? { attachment: { filename: String(message.attachment.filename || ''), contentType: String(message.attachment.contentType || ''), diff --git a/frontend/background-issue-sync.js b/frontend/background-issue-sync.js index 1f552fb..24c2213 100644 --- a/frontend/background-issue-sync.js +++ b/frontend/background-issue-sync.js @@ -510,11 +510,12 @@ function createBackgroundIssueSync({ async function deliverScreenshotComment(item) { const repository = String(item.repository || '').split('/').map(encodeURIComponent).join('/'); + const resource = item.kind === 'pull-comment' ? 'pulls' : 'issues'; let attachmentMarkdown = item.attachmentMarkdown; if (!attachmentMarkdown) { const uploaded = await requestStage( item, - base + 'api/v1/repos/' + repository + '/issues/' + encodeURIComponent(item.number) + '/attachments', + base + 'api/v1/repos/' + repository + '/' + resource + '/' + encodeURIComponent(item.number) + '/attachments', { method: 'POST', headers: { @@ -535,7 +536,7 @@ function createBackgroundIssueSync({ const text = String(item.body || '').trim(); return requestStage( item, - base + 'api/v1/repos/' + repository + '/issues/' + encodeURIComponent(item.number) + '/comments', + base + 'api/v1/repos/' + repository + '/' + resource + '/' + encodeURIComponent(item.number) + '/comments', { method: 'POST', headers: { @@ -551,7 +552,7 @@ function createBackgroundIssueSync({ const request = deliveryRequest(item); try { const delivered = item.kind === 'update-reply-read' ? await deliverReplyRead(item) : - item.attachment && item.kind === 'issue-comment' ? + item.attachment && ['issue-comment', 'pull-comment'].includes(item.kind) ? await deliverScreenshotComment(item) : item.attachment && !item.kind ? await deliverIssueCapture(item) : await requestStage(item, request.url, request.options); if (item.kind === 'issue-close' && delivered?.state !== 'closed') { diff --git a/frontend/dashboard.js b/frontend/dashboard.js index a5a6f94..c5d1ab4 100644 --- a/frontend/dashboard.js +++ b/frontend/dashboard.js @@ -303,6 +303,27 @@ ); }, }); + const pullAttachmentController = issueAttachment.mount({ + input: qs('#pull-attachment'), + preview: qs('#pull-attachment-preview'), + image: qs('#pull-attachment-image'), + meta: qs('#pull-attachment-meta'), + remove: qs('#remove-pull-attachment'), + status: qs('#pull-comment-status'), + createObjectURL: file => URL.createObjectURL(file), + revokeObjectURL: url => URL.revokeObjectURL(url), + upload: payload => { + const repository = payload.repository.split('/').map(encodeURIComponent).join('/'); + return fetchReviewJson( + 'api/v1/repos/' + repository + '/pulls/' + encodeURIComponent(payload.number) + '/attachments', + { + method: 'POST', + headers: { Accept:'application/json', 'Idempotency-Key':payload.operation_id }, + body: issueAttachment.multipart(payload), + }, + ); + }, + }); const createIssueAttachmentController = issueAttachment.mount({ input: qs('#create-issue-attachment'), preview: qs('#create-issue-attachment-preview'), @@ -2343,8 +2364,14 @@ } } + function sameWorkTarget(left, right) { + return Boolean(left && right && left.repository === right.repository && + Number(left.number) === Number(right.number)); + } + async function openPullSheet(item, trigger, offlineDetail = null) { if (!item) return; + if (!sameWorkTarget(selectedPull, item)) pullAttachmentController.clear(); qs('#pull-review').inert = false; selectedPull = item; pullMentions.dismiss(); @@ -2407,6 +2434,7 @@ return; } mobileComposerViewport.close(qs('#pull-sheet .pull-sheet-panel')); + pullAttachmentController.clear(); qs('#pull-sheet').classList.remove('open'); selectedPull = null; selectedPullDetail = null; @@ -3929,13 +3957,32 @@ return admission; } + async function queuePullScreenshotComment(item, body, operationId, advance = false, deliver = false) { + const message = { + kind: 'pull-comment', repository: item.repository, number: item.number, body, + operationId: operationId || globalThis.crypto?.randomUUID?.() || String(Date.now()), + attachment: await pullAttachmentController.serialize(), + }; + if (advance) return await pullCommentNext.admit(item, message); + const admission = await authoredOutbox.enqueueDurably(message); + if (deliver) { + const delivery = await authoredOutbox.retry(admission.item.id, activeFlushLogin); + return { ...admission, delivered: delivery.confirmed?.[0] || null }; + } + pullController.saveDraft(item, ''); + if (selectedPull === item) qs('#pull-comment').value = ''; + return admission; + } + async function submitCommentAndNext(kind) { const item = kind === 'issue' ? selectedIssue : selectedPull; if (!item || !workSession.checkpointed(item)) return; const textarea = qs('#' + kind + '-comment'); const status = qs('#' + kind + '-comment-status'); const body = textarea.value.trim(); - if (!body && (kind !== 'issue' || !issueAttachmentController.state())) { + const attachmentController = kind === 'issue' ? issueAttachmentController : pullAttachmentController; + const queueScreenshot = kind === 'issue' ? queueIssueScreenshotComment : queuePullScreenshotComment; + if (!body && !attachmentController.state()) { status.textContent = 'Write a comment before posting.'; textarea.focus(); return; @@ -3947,26 +3994,28 @@ item.repository + '#' + item.number + ':operation'); postButton.disabled = true; nextButton.disabled = true; - status.textContent = kind === 'issue' && issueAttachmentController.state() ? + attachmentController.setBusy(true); + status.textContent = attachmentController.state() ? 'Uploading screenshot before opening next…' : 'Posting comment and opening next…'; try { let result; - if (kind === 'issue' && issueAttachmentController.state() && navigator.onLine === false) { - result = await queueIssueScreenshotComment(item, body, operationId(), true); + if (attachmentController.state() && (kind === 'pull' || navigator.onLine === false)) { + result = await queueScreenshot(item, body, operationId(), true); } else { let preparedBody; try { preparedBody = kind === 'issue' ? - await issueAttachmentController.prepareComment(item, body) : body; + await issueAttachmentController.prepareComment(item, body) : + await pullAttachmentController.prepareComment(item, body); } catch (error) { - if (kind !== 'issue' || !issueAttachmentController.state() || !canQueueMessage(error)) throw error; - result = await queueIssueScreenshotComment(item, body, operationId(), true); + if (!attachmentController.state() || !canQueueMessage(error)) throw error; + result = await queueScreenshot(item, body, operationId(), true); } if (!result) result = await controller.submit(item, preparedBody, operationId); } const stillOpen = kind === 'issue' ? selectedIssue === item : selectedPull === item; if (!stillOpen) return; - if (kind === 'issue') issueAttachmentController.clear(); + attachmentController.clear(); if (!result.completed) status.textContent = 'Comment saved, but Today still needs completion.'; else if (result.delivery === 'posted') status.textContent = 'Comment posted.'; else if (result.background) status.textContent = 'Queued for sync when the connection returns.'; @@ -3975,6 +4024,7 @@ status.textContent = error.message + ' Your draft, screenshot, and Today position are safe; retry.'; textarea.focus(); } finally { + attachmentController.setBusy(false); postButton.disabled = false; nextButton.disabled = false; } @@ -4234,39 +4284,68 @@ }); qs('#send-pull-comment').addEventListener('click', async () => { if (!selectedPull) return; + const item = selectedPull; const body = qs('#pull-comment').value.trim(); - if (!body) { + if (!body && !pullAttachmentController.state()) { qs('#pull-comment-status').textContent = 'Write a comment before posting.'; qs('#pull-comment').focus(); return; } const button = qs('#send-pull-comment'); button.disabled = true; - qs('#pull-comment-status').textContent = 'Posting comment…'; + pullAttachmentController.setBusy(true); + qs('#pull-comment-status').textContent = pullAttachmentController.state() ? + 'Uploading screenshot…' : 'Posting comment…'; + const operationId = localStorage.getItem('stackchain.pull-comment.v1:' + + item.repository + '#' + item.number + ':operation'); + let preparedBody; try { - const comment = await pullController.comment(selectedPull, body); - if (pullConversation) renderPullConversation(pullConversation.append(comment)); - qs('#pull-comment').value = ''; + if (pullAttachmentController.state()) { + const admission = await queuePullScreenshotComment(item, body, operationId, false, true); + refreshMyWorkView(); + pullController.saveDraft(item, ''); + if (selectedPull === item) { + if (admission.delivered && pullConversation) { + renderPullConversation(pullConversation.append(admission.delivered)); + } + qs('#pull-comment').value = ''; + pullAttachmentController.clear(); + qs('#pull-comment-status').textContent = admission.delivered ? 'Comment posted.' : + (admission.background ? 'Queued with screenshot for sync when the connection returns.' : + 'Saved with screenshot for next launch; background delivery unavailable.'); + } + return; + } + preparedBody = body; + const comment = await pullController.comment(item, preparedBody); + if (selectedPull === item && pullConversation) renderPullConversation(pullConversation.append(comment)); + pullController.saveDraft(item, ''); + if (selectedPull === item) qs('#pull-comment').value = ''; + pullAttachmentController.clear(); qs('#pull-comment-status').textContent = 'Comment posted.'; } catch (error) { - if (canQueueMessage(error)) { - const operationId = localStorage.getItem('stackchain.pull-comment.v1:' + selectedPull.repository + '#' + selectedPull.number + ':operation'); - qs('#pull-comment-status').textContent = 'Saving for background delivery…'; - const admission = await authoredOutbox.enqueueDurably({ kind:'pull-comment', repository:selectedPull.repository, - number:selectedPull.number, body, operationId }); - refreshMyWorkView(); - if (admission.background) { - qs('#pull-comment').value = ''; - qs('#pull-comment-status').textContent = 'Queued for sync when the connection returns.'; - } else { - qs('#pull-comment-status').textContent = 'Saved for next launch; background delivery unavailable.'; - qs('#pull-comment').focus(); + if (canQueueMessage(error) && !pullAttachmentController.state()) { + try { + qs('#pull-comment-status').textContent = 'Saving for background delivery…'; + const admission = await authoredOutbox.enqueueDurably({ + kind:'pull-comment', repository:item.repository, number:item.number, + body:preparedBody ?? body, operationId, + }); + pullController.saveDraft(item, ''); + if (selectedPull === item) qs('#pull-comment').value = ''; + refreshMyWorkView(); + qs('#pull-comment-status').textContent = admission.background ? + 'Queued for sync when the connection returns.' : + 'Saved for next launch; background delivery unavailable.'; + return; + } catch (admissionError) { + error = admissionError; } - } else { - qs('#pull-comment-status').textContent = error.message + ' Your draft is safe; retry.'; - qs('#pull-comment').focus(); } + qs('#pull-comment-status').textContent = error.message + ' Your comment and screenshot are safe; retry.'; + qs('#pull-comment').focus(); } finally { + pullAttachmentController.setBusy(false); button.disabled = false; } }); diff --git a/frontend/index.html b/frontend/index.html index c40e687..4163646 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -558,6 +558,15 @@
+
+ + +
+
diff --git a/frontend/issue-attachment.js b/frontend/issue-attachment.js index f97b2fb..4b98e2a 100644 --- a/frontend/issue-attachment.js +++ b/frontend/issue-attachment.js @@ -181,6 +181,11 @@ let previewUrl = ''; let selectionSequence = 0; + function setBusy(busy) { + options.input.disabled = Boolean(busy); + options.remove.disabled = Boolean(busy); + } + function clearPreview() { selectionSequence += 1; if (previewUrl) options.revokeObjectURL(previewUrl); @@ -247,7 +252,7 @@ return restored; } - return Object.assign(controller, { clear: clearPreview, restore: restorePreview }); + return Object.assign(controller, { clear: clearPreview, restore: restorePreview, setBusy }); } return { create, mount, multipart, optimizeImage, MAX_BYTES }; diff --git a/src/gitea_proxy.py b/src/gitea_proxy.py index e5d6a94..c77433d 100644 --- a/src/gitea_proxy.py +++ b/src/gitea_proxy.py @@ -920,6 +920,33 @@ async def upload_assigned_issue_attachment( return {"name": name, "url": url, "size": size} +async def upload_assigned_pull_attachment( + repository: str, + number: int, + filename: str, + content_type: str, + content: bytes, +) -> dict: + if not await is_assigned_pull(repository, number): + raise IssueNotAvailableError("Assigned pull request not found") + response = await _get_client().post( + f"/api/v1/repos/{repository}/issues/{number}/assets", + headers=_auth(), + params={"name": filename}, + files={"attachment": (filename, content, content_type)}, + ) + response.raise_for_status() + attachment = response.json() + if not isinstance(attachment, dict): + raise ValueError("Gitea attachment response was not an object") + name = attachment.get("name") + url = _safe_web_url(attachment.get("browser_download_url")) + size = attachment.get("size") + if not isinstance(name, str) or not name or not url or not isinstance(size, int): + raise ValueError("Gitea did not confirm the attachment") + return {"name": name, "url": url, "size": size} + + async def repo_labels(repository: str) -> list[dict]: response = await _get_client().get( f"/api/v1/repos/{repository}/labels", diff --git a/src/main.py b/src/main.py index 171dfd3..22da575 100644 --- a/src/main.py +++ b/src/main.py @@ -3468,6 +3468,67 @@ async def attach_to_assigned_issue( return JSONResponse(result, status_code=201) +@app.post("/api/v1/repos/{owner}/{repo}/pulls/{number}/attachments", status_code=201) +async def attach_to_assigned_pull( + request: Request, + owner: str, + repo: str, + number: int = PathParam(gt=0), + idempotency_key: str | None = Header(default=None, max_length=128), +): + repository = f"{owner}/{repo}" + try: + if request.headers.get("content-type", "").lower().startswith("multipart/form-data"): + form = await request.form() + uploaded = form.get("file") + if not isinstance(uploaded, UploadFile): + raise ValueError("screenshot file is required") + filename = str(uploaded.filename or "") + content_type = str(uploaded.content_type or "") + content = _validate_binary_attachment(filename, content_type, await uploaded.read()) + else: + attachment = IssueAttachment.model_validate(await request.json()) + filename = attachment.filename + content_type = attachment.content_type + content = attachment.content() + except (ValueError, ValidationError) as exc: + raise HTTPException(status_code=422, detail=str(exc)) from exc + + async def upload_attachment(): + result = await gitea_proxy.upload_assigned_pull_attachment( + repository, number, filename, 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 _run_idempotent_authored_action( + upload_attachment(), + idempotency_key=idempotency_key, + fingerprint=( + "pull-attachment", repository, number, filename, content_type, + hashlib.sha256(content).hexdigest(), + ), + timeout=ISSUE_ACTION_TIMEOUT_SECONDS, + ) + except gitea_proxy.IssueNotAvailableError as exc: + raise HTTPException(status_code=404, detail="Assigned pull request not found") from exc + except HTTPException: + raise + except Exception: + return JSONResponse( + {"error": "The screenshot could not be uploaded. Your draft is safe; please retry."}, + status_code=503, + headers={"Retry-After": "1"}, + ) + return JSONResponse(result, status_code=201) + + @app.patch("/api/v1/repos/{owner}/{repo}/issues/{number}/close") async def close_assigned_issue( request: Request, diff --git a/src/request_boundary.py b/src/request_boundary.py index 69c8737..20020b2 100644 --- a/src/request_boundary.py +++ b/src/request_boundary.py @@ -18,7 +18,7 @@ def request_body_limit(method: str, path: str) -> int | None: if ( normalized_method == "POST" and path.startswith("/api/v1/repos/") - and "/issues/" in path + and ("/issues/" in path or "/pulls/" in path) and path.endswith("/attachments") ): return ISSUE_ATTACHMENT_BODY_LIMIT diff --git a/tests/test_authored_outbox.py b/tests/test_authored_outbox.py index 3df1173..85d0b9a 100644 --- a/tests/test_authored_outbox.py +++ b/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"} diff --git a/tests/test_background_issue_sync.py b/tests/test_background_issue_sync.py index 984b4b1..2296d26 100644 --- a/tests/test_background_issue_sync.py +++ b/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))}); diff --git a/tests/test_issue_attachment_ui.py b/tests/test_issue_attachment_ui.py index b65fff9..b5cd144 100644 --- a/tests/test_issue_attachment_ui.py +++ b/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 diff --git a/tests/test_issue_attachments.py b/tests/test_issue_attachments.py index d518952..56301d1 100644 --- a/tests/test_issue_attachments.py +++ b/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]()" + ) + 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" diff --git a/tests/test_request_boundary.py b/tests/test_request_boundary.py index 1144a85..d2a3d7c 100644 --- a/tests/test_request_boundary.py +++ b/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