From 7ec3330175a0d9285a02bd1dac2678a8bdee8128 Mon Sep 17 00:00:00 2001 From: timmy Date: Mon, 10 Aug 2026 10:56:02 +0000 Subject: [PATCH] feat: queue screenshot comments offline (Closes #477) --- README.md | 7 ++-- frontend/authored-outbox.js | 22 +++++++++++- frontend/background-issue-sync.js | 46 ++++++++++++++++++++++-- frontend/comment-next.js | 34 +++++++++++------- frontend/dashboard.js | 56 +++++++++++++++++++++++++++-- tests/test_authored_outbox.py | 49 +++++++++++++++++++++++++ tests/test_background_issue_sync.py | 37 +++++++++++++++++++ tests/test_comment_next.py | 24 +++++++++++++ tests/test_issue_attachment_ui.py | 10 ++++++ 9 files changed, 265 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 170773d..9672492 100644 --- a/README.md +++ b/README.md @@ -20,8 +20,11 @@ list repository labels and open milestones, set or clear due dates on assigned i 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 up to 2 MB. -The screenshot uploads before the comment is posted; validation or upload failures keep -both the typed comment and removable preview available for retry. The mobile **New issue** +For online delivery, the screenshot uploads before the comment is posted; 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 +either stage. The mobile **New issue** sheet accepts the same image formats and stores the screenshot with its account-bound outbox capture. Durable admission writes the complete screenshot capture to IndexedDB before confirmation; localStorage keeps only bounded attachment metadata, avoiding base64 diff --git a/frontend/authored-outbox.js b/frontend/authored-outbox.js index 9abe813..c2a56ce 100644 --- a/frontend/authored-outbox.js +++ b/frontend/authored-outbox.js @@ -74,6 +74,13 @@ function createAuthoredOutbox({ storage, fetchJson, coordinator, backgroundSync, ownerLogin, status: 'queued', queuedAt: Number(now()), + ...(message.kind === 'issue-comment' && message.attachment ? { + attachment: { + filename: String(message.attachment.filename || ''), + contentType: String(message.attachment.contentType || ''), + stored: true, + }, + } : {}), ...(message.kind === 'pull-review' ? { decision: String(message.decision || 'comment'), expectedHeadSha: String(message.expectedHeadSha || ''), @@ -94,8 +101,21 @@ function createAuthoredOutbox({ storage, fetchJson, coordinator, backgroundSync, if (!backgroundSync?.reconcile || !backgroundSync?.requestSync) { return { item, background: false, durability: 'foreground-only' }; } + const durableItems = read().map(candidate => candidate.id === item.id && message.attachment ? { + ...candidate, + attachment: { + filename: String(message.attachment.filename || ''), + contentType: String(message.attachment.contentType || ''), + data: String(message.attachment.data || ''), + }, + } : candidate); + try { + await backgroundSync.reconcile(durableItems, 'authored'); + } catch (error) { + write(read().filter(candidate => candidate.id !== item.id), false); + throw error; + } try { - await backgroundSync.reconcile(read(), 'authored'); await backgroundSync.requestSync(); return { item, background: true, durability: 'background' }; } catch (error) { diff --git a/frontend/background-issue-sync.js b/frontend/background-issue-sync.js index 862c7d9..9709cbc 100644 --- a/frontend/background-issue-sync.js +++ b/frontend/background-issue-sync.js @@ -410,11 +410,53 @@ function createBackgroundIssueSync({ return deliveredIssue; } + async function deliverScreenshotComment(item) { + const repository = String(item.repository || '').split('/').map(encodeURIComponent).join('/'); + let attachmentMarkdown = item.attachmentMarkdown; + if (!attachmentMarkdown) { + const uploaded = await requestJson( + base + 'api/v1/repos/' + repository + '/issues/' + encodeURIComponent(item.number) + '/attachments', + { + method: 'POST', + headers: { + Accept: 'application/json', 'Content-Type': 'application/json', + 'Idempotency-Key': stageOperationId(item.operationId, 'attachment'), + }, + body: JSON.stringify({ + filename: item.attachment.filename, + content_type: item.attachment.contentType, + data: item.attachment.data, + }), + }, + ); + attachmentMarkdown = String(uploaded?.markdown || ''); + if (!attachmentMarkdown) { + const error = new Error('The server did not confirm the screenshot upload.'); + error.status = 422; + throw error; + } + await store.update?.(item.id, current => ({ ...current, attachmentMarkdown })); + } + const text = String(item.body || '').trim(); + return requestJson( + base + 'api/v1/repos/' + repository + '/issues/' + encodeURIComponent(item.number) + '/comments', + { + method: 'POST', + headers: { + Accept: 'application/json', 'Content-Type': 'application/json', + 'Idempotency-Key': stageOperationId(item.operationId, 'comment'), + }, + body: JSON.stringify({ body: text ? text + '\n\n' + attachmentMarkdown : attachmentMarkdown }), + }, + ); + } + async function deliver(item) { const request = deliveryRequest(item); try { - const delivered = item.attachment && !item.kind ? - await deliverIssueCapture(item) : await requestJson(request.url, request.options); + const delivered = item.attachment && item.kind === 'issue-comment' ? + await deliverScreenshotComment(item) : item.attachment && !item.kind ? + await deliverIssueCapture(item) : await requestJson(request.url, request.options); if (item.kind === 'issue-close' && delivered?.state !== 'closed') { const error = new Error('Issue closure was not confirmed.'); error.status = 422; diff --git a/frontend/comment-next.js b/frontend/comment-next.js index 86ca019..d5dc272 100644 --- a/frontend/comment-next.js +++ b/frontend/comment-next.js @@ -1,6 +1,20 @@ function createCommentNext({ post, queue, queueKind = '', canQueue, accept = () => undefined, complete }) { let inFlight = null; + async function admitQueued(item, message) { + const admission = await queue(message); + if (!admission || (!admission.item && admission.durable !== true)) { + throw new Error('Comment was not saved for delivery.'); + } + accept(item, { delivery: admission.background ? 'queued' : 'saved' }); + return { + accepted: true, + delivery: admission.background ? 'queued' : 'saved', + background: Boolean(admission.background), + completed: Boolean(complete(item)), + }; + } + function submit(item, body, operationId = '') { if (inFlight) return inFlight; inFlight = (async () => { @@ -16,20 +30,10 @@ function createCommentNext({ post, queue, queueKind = '', canQueue, accept = () kind: item.kind === 'pull' ? 'pull-comment' : 'issue-comment', repository: item.repository, number: item.number, }; - const admission = await queue({ + return admitQueued(item, { ...identity, body, operationId: typeof operationId === 'function' ? operationId() : operationId, }); - if (!admission || (!admission.item && admission.durable !== true)) { - throw new Error('Comment was not saved for delivery.'); - } - accept(item, { delivery: admission.background ? 'queued' : 'saved' }); - return { - accepted: true, - delivery: admission.background ? 'queued' : 'saved', - background: Boolean(admission.background), - completed: Boolean(complete(item)), - }; } accept(item, { delivery: 'posted', comment }); return { @@ -45,7 +49,13 @@ function createCommentNext({ post, queue, queueKind = '', canQueue, accept = () return inFlight; } - return { submit, busy: () => Boolean(inFlight) }; + function admit(item, message) { + if (inFlight) return inFlight; + inFlight = admitQueued(item, message).finally(() => { inFlight = null; }); + return inFlight; + } + + return { submit, admit, busy: () => Boolean(inFlight) }; } if (typeof module !== 'undefined' && module.exports) module.exports = createCommentNext; diff --git a/frontend/dashboard.js b/frontend/dashboard.js index c632083..fc6f544 100644 --- a/frontend/dashboard.js +++ b/frontend/dashboard.js @@ -3795,6 +3795,19 @@ qs('#issue-milestone').disabled = false; } }); + async function queueIssueScreenshotComment(item, body, operationId, advance = false) { + const message = { + kind: 'issue-comment', repository: item.repository, number: item.number, body, + operationId: operationId || globalThis.crypto?.randomUUID?.() || String(Date.now()), + attachment: await issueAttachmentController.serialize(), + }; + if (advance) return await issueCommentNext.admit(item, message); + const admission = await authoredOutbox.enqueueDurably(message); + issueController.saveDraft(item, ''); + if (selectedIssue === item) qs('#issue-comment').value = ''; + return admission; + } + async function submitCommentAndNext(kind) { const item = kind === 'issue' ? selectedIssue : selectedPull; if (!item || !workSession.checkpointed(item)) return; @@ -3816,9 +3829,20 @@ status.textContent = kind === 'issue' && issueAttachmentController.state() ? 'Uploading screenshot before opening next…' : 'Posting comment and opening next…'; try { - const preparedBody = kind === 'issue' ? - await issueAttachmentController.prepareComment(item, body) : body; - const result = await controller.submit(item, preparedBody, operationId); + let result; + if (kind === 'issue' && issueAttachmentController.state() && navigator.onLine === false) { + result = await queueIssueScreenshotComment(item, body, operationId(), true); + } else { + let preparedBody; + try { + preparedBody = kind === 'issue' ? + await issueAttachmentController.prepareComment(item, body) : body; + } catch (error) { + if (kind !== 'issue' || !issueAttachmentController.state() || !canQueueMessage(error)) throw error; + result = await queueIssueScreenshotComment(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(); @@ -3850,8 +3874,34 @@ 'Uploading screenshot…' : 'Posting comment…'; let preparedBody; try { + if (issueAttachmentController.state() && navigator.onLine === false) { + const operationId = localStorage.getItem('stackchain.issue-comment.v1:' + selectedIssue.repository + '#' + selectedIssue.number + ':operation'); + const admission = await queueIssueScreenshotComment(selectedIssue, body, operationId); + refreshMyWorkView(); + issueAttachmentController.clear(); + qs('#issue-comment-status').textContent = admission.background ? + 'Queued with screenshot for sync when the connection returns.' : + 'Saved with screenshot for next launch; background delivery unavailable.'; + button.disabled = false; + return; + } preparedBody = await issueAttachmentController.prepareComment(selectedIssue, body); } catch (error) { + if (issueAttachmentController.state() && canQueueMessage(error)) { + try { + const operationId = localStorage.getItem('stackchain.issue-comment.v1:' + selectedIssue.repository + '#' + selectedIssue.number + ':operation'); + const admission = await queueIssueScreenshotComment(selectedIssue, body, operationId); + refreshMyWorkView(); + issueAttachmentController.clear(); + qs('#issue-comment-status').textContent = admission.background ? + 'Queued with screenshot for sync when the connection returns.' : + 'Saved with screenshot for next launch; background delivery unavailable.'; + button.disabled = false; + return; + } catch (admissionError) { + error = admissionError; + } + } qs('#issue-comment-status').textContent = error.message + ' Your comment and screenshot are safe; retry.'; qs('#issue-comment').focus(); button.disabled = false; diff --git a/tests/test_authored_outbox.py b/tests/test_authored_outbox.py index 91fd1f0..3671bc3 100644 --- a/tests/test_authored_outbox.py +++ b/tests/test_authored_outbox.py @@ -425,3 +425,52 @@ async def test_mobile_dashboard_loads_and_operates_authored_message_outbox(): assert "if (result?.queued)" in html assert "backgroundSync: backgroundIssueSync" in html assert "authoredOutbox.reconcileBackground(records)" in html + + +def test_screenshot_comment_admits_bytes_to_indexeddb_without_putting_them_in_localstorage(): + 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),removeItem:k=>values.delete(k)}}; +const outbox=createAuthoredOutbox({{ + storage,getOwnerLogin:()=>'timmy', + backgroundSync:{{reconcile:async(items,lane)=>mirrors.push({{items,lane}}),requestSync:async()=>{{}}}}, +}}); +(async()=>{{ + const admission=await outbox.enqueueDurably({{ + kind:'issue-comment',repository:'stackchain/dashboard',number:477, + body:'Broken at 320px',operationId:'comment-image-477', + attachment:{{filename:'phone.png',contentType:'image/png',data:'PRIVATE-IMAGE-BYTES'}}, + }}); + process.stdout.write(JSON.stringify({{admission,local:values.get('stackchain.authored-outbox.v1'),mirrored:mirrors[0]}})); +}})(); +""" + output = run_node(script) + + assert "PRIVATE-IMAGE-BYTES" not in output["local"] + local_item = json.loads(output["local"])["items"][0] + assert local_item["attachment"] == { + "filename": "phone.png", "contentType": "image/png", "stored": True + } + assert output["mirrored"]["lane"] == "authored" + assert output["mirrored"]["items"][0]["attachment"]["data"] == "PRIVATE-IMAGE-BYTES" + assert output["admission"]["durability"] == "background" + + +def test_screenshot_comment_failed_indexeddb_admission_is_not_accepted_locally(): + script = f""" +const createAuthoredOutbox=require({json.dumps(str(OUTBOX))}); +const values=new Map(); +const storage={{getItem:k=>values.get(k)||null,setItem:(k,v)=>values.set(k,v),removeItem:k=>values.delete(k)}}; +const outbox=createAuthoredOutbox({{storage,getOwnerLogin:()=>'timmy',backgroundSync:{{ + reconcile:async()=>{{throw new Error('IndexedDB unavailable');}},requestSync:async()=>{{}}, +}}}}); +(async()=>{{ + let error='';try{{await outbox.enqueueDurably({{kind:'issue-comment',repository:'o/r',number:1, + body:'Keep me',operationId:'image-op',attachment:{{filename:'a.png',contentType:'image/png',data:'abc'}}}});}} + catch(caught){{error=caught.message;}} + process.stdout.write(JSON.stringify({{error,items:outbox.list()}})); +}})(); +""" + output = run_node(script) + assert output == {"error": "IndexedDB unavailable", "items": []} diff --git a/tests/test_background_issue_sync.py b/tests/test_background_issue_sync.py index b5a73de..313cc38 100644 --- a/tests/test_background_issue_sync.py +++ b/tests/test_background_issue_sync.py @@ -129,6 +129,43 @@ createBackgroundIssueSync({{store,fetchJson}}).flush().then(()=>process.stdout.w assert all(len(key) <= 128 for key in keys) +def test_screenshot_comment_retry_reuses_upload_checkpoint_and_posts_combined_body_once(): + script = f""" +const createBackgroundIssueSync=require({json.dumps(str(SYNC))}); +let item={{id:'message-image',operationId:'message-image',ownerLogin:'timmy',status:'queued', + kind:'issue-comment',repository:'stackchain/dashboard',number:477,body:'Broken at 320px', + attachment:{{filename:'phone.png',contentType:'image/png',data:'iVBORw0KGgo='}}}}; +const state={{calls:[],released:0,completed:0}};let commentAttempts=0; +const store={{claimNext:async()=>item?{{...item}}:null,update:async(_id,transform)=>{{item=transform(item);}}, + complete:async()=>{{state.completed++;item=null;}},release:async()=>{{state.released++;item={{...item,status:'queued'}};}}, + fail:async()=>{{}},countBlocked:async()=>0}}; +const fetchJson=async(url,options={{}})=>{{ + if(url==='api/v1/background-identity')return{{login:'timmy'}}; + state.calls.push({{url,key:options.headers?.['Idempotency-Key'],body:options.body&&JSON.parse(options.body)}}); + if(url.endsWith('/attachments'))return{{markdown:'![phone.png](https://forge.example/phone.png)'}}; + if(commentAttempts++===0){{const error=new Error('Comment unavailable');error.status=503;throw error;}} + return{{id:91}}; +}}; +(async()=>{{const sync=createBackgroundIssueSync({{store,fetchJson}});let firstError=''; + try{{await sync.flush();}}catch(error){{firstError=error.message;}} + const checkpoint={{...item}};const second=await sync.flush(); + process.stdout.write(JSON.stringify({{state,firstError,checkpoint,second}})); +}})(); +""" + output = run_node(script) + + assert output["firstError"] == "Comment unavailable" + assert output["checkpoint"]["attachmentMarkdown"].startswith("![phone.png]") + assert sum(call["url"].endswith("/attachments") for call in output["state"]["calls"]) == 1 + comments = [call for call in output["state"]["calls"] if call["url"].endswith("/comments")] + assert [call["key"] for call in comments] == ["message-image:comment"] * 2 + assert comments[-1]["body"] == { + "body": "Broken at 320px\n\n![phone.png](https://forge.example/phone.png)" + } + assert output["state"]["completed"] == 1 + assert output["second"]["confirmed"] == [{"id": 91}] + + 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_comment_next.py b/tests/test_comment_next.py index b1953fb..ddcbf5d 100644 --- a/tests/test_comment_next.py +++ b/tests/test_comment_next.py @@ -254,6 +254,30 @@ process.stdout.write(JSON.stringify({{ } +def test_comment_and_next_can_admit_a_complete_screenshot_message_before_advancing(): + script = f""" +const createCommentNext=require({json.dumps(str(COMMENT_NEXT))}); +const calls=[]; +const controller=createCommentNext({{ + post:()=>{{throw new Error('must not post');}},canQueue:()=>true, + queue:async message=>{{calls.push({{queue:message}});return{{item:{{id:'queued'}},background:true}};}}, + accept:item=>calls.push({{accept:item.number}}), + complete:item=>{{calls.push({{complete:item.number}});return true;}}, +}}); +(async()=>{{const item={{kind:'issue',repository:'stackchain/dashboard',number:477}}; + const result=await controller.admit(item,{{kind:'issue-comment',repository:item.repository,number:item.number, + body:'Mobile evidence',operationId:'image-op',attachment:{{filename:'phone.png',contentType:'image/png',data:'abc'}}}}); + process.stdout.write(JSON.stringify({{result,calls}})); +}})(); +""" + output = json.loads(run_node(script)) + assert output["result"] == { + "accepted": True, "delivery": "queued", "background": True, "completed": True + } + assert output["calls"][0]["queue"]["attachment"]["data"] == "abc" + assert output["calls"][1:] == [{"accept": 477}, {"complete": 477}] + + @pytest.mark.anyio async def test_mobile_composers_offer_comment_and_next_only_for_today_checkpoint(): html = await dashboard() diff --git a/tests/test_issue_attachment_ui.py b/tests/test_issue_attachment_ui.py index f92c97f..1389a65 100644 --- a/tests/test_issue_attachment_ui.py +++ b/tests/test_issue_attachment_ui.py @@ -252,3 +252,13 @@ def test_readme_documents_mobile_screenshot_limits_and_delivery_order(): assert "uploads before the comment is posted" in readme assert "New issue" in readme assert "retry resumes with the confirmed issue" in readme + + +def test_issue_screenshot_comments_queue_serialized_bytes_before_clearing_or_advancing(): + source = DASHBOARD.read_text() + + assert "async function queueIssueScreenshotComment" in source + assert "attachment: await issueAttachmentController.serialize()" in source + assert "await authoredOutbox.enqueueDurably(message)" in source + assert "await issueCommentNext.admit(item, message)" in source + assert "navigator.onLine === false" in source