diff --git a/README.md b/README.md index 3d0d439..170773d 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,11 @@ 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** sheet accepts the same image formats and stores the screenshot with its account-bound -outbox capture. Delivery creates the issue exactly once, then uploads and comments with +outbox capture. Durable admission writes the complete screenshot capture to IndexedDB +before confirmation; localStorage keeps only bounded attachment metadata, avoiding base64 +quota pressure and synchronous multi-megabyte writes. Existing queued screenshot payloads +migrate to the IndexedDB-backed form on the next durable admission or worker reconciliation. +Delivery creates the issue exactly once, then uploads and comments with the image; after a partial failure, retry resumes with the confirmed issue instead of creating a duplicate. Pull-request replies and mobile My Work issue and PR comments use Gitea's diff --git a/frontend/background-issue-sync.js b/frontend/background-issue-sync.js index 20ce8b1..73c36b6 100644 --- a/frontend/background-issue-sync.js +++ b/frontend/background-issue-sync.js @@ -65,13 +65,18 @@ function createIssueSyncStore({ transaction, indexedDB = globalThis.indexedDB, n if (current.recordType === 'receipt-preference') continue; const currentLane = current.outboxLane || 'issue'; if (currentLane !== outboxLane) continue; - const replacement = incoming.get(current.id); + let replacement = incoming.get(current.id); if (!replacement) { if (current.status !== 'sending' || Number(current.claimUntil) <= Number(now())) { await records.delete(current.id); } continue; } + if (current.operationId === replacement.operationId && current.attachment?.data && + replacement.attachment?.stored && !replacement.attachment.data) { + replacement = { ...replacement, attachment: current.attachment }; + incoming.set(current.id, replacement); + } if ((current.status === 'sending' && Number(current.claimUntil) > Number(now())) || (current.status === 'attention' && replacement.status === 'attention') || current.status === 'sent') { @@ -141,8 +146,10 @@ function createIssueSyncStore({ transaction, indexedDB = globalThis.indexedDB, n if (current && ((current.status === 'sending' && Number(current.claimUntil) > Number(now())) || (current.status === 'attention' && item.status === 'attention') || current.status === 'sent')) return current; + const preservedAttachment = current?.attachment?.data && item?.attachment?.stored && !item.attachment.data + ? { attachment: current.attachment } : {}; const next = current && current.operationId === item.operationId ? { - ...item, + ...item, ...preservedAttachment, ...(current.deliveredIssue ? { deliveredIssue: current.deliveredIssue } : {}), ...(current.attachmentMarkdown ? { attachmentMarkdown: current.attachmentMarkdown } : {}), } : { ...item }; diff --git a/frontend/issue-outbox.js b/frontend/issue-outbox.js index 14a8aeb..4d1cb8c 100644 --- a/frontend/issue-outbox.js +++ b/frontend/issue-outbox.js @@ -9,7 +9,9 @@ function createIssueOutbox({ storage, fetchJson, coordinator, backgroundSync, ge const contentType = String(value?.contentType || ''); const filename = String(value?.filename || '').slice(0, 255); const data = String(value?.data || ''); - if (!filename || !['image/png', 'image/jpeg', 'image/webp'].includes(contentType) || !data) return undefined; + if (!filename || !['image/png', 'image/jpeg', 'image/webp'].includes(contentType)) return undefined; + if (!data && value?.stored === true) return { filename, contentType, stored: true }; + if (!data) return undefined; return { filename, contentType, data }; } @@ -31,7 +33,7 @@ function createIssueOutbox({ storage, fetchJson, coordinator, backgroundSync, ge } } - function enqueue(draft, mirror = true) { + function prepareItem(draft) { const ownerLogin = String(getOwnerLogin() || '').trim(); if (!ownerLogin) throw new Error('Confirm your Gitea account before queueing an issue.'); const items = read(); @@ -55,28 +57,52 @@ function createIssueOutbox({ storage, fetchJson, coordinator, backgroundSync, ge item.milestoneId = Number(draft.milestoneId); } if (/^\d{4}-\d{2}-\d{2}$/.test(String(draft?.dueDate || ''))) item.dueDate = String(draft.dueDate); + return item; + } + + function localIndexItem(item) { + if (!item?.attachment?.data) return item; + return { + ...item, + attachment: { + filename: item.attachment.filename, + contentType: item.attachment.contentType, + stored: true, + }, + }; + } + + function enqueue(draft, mirror = true) { + const items = read(); + const item = prepareItem(draft); items.push(item); write(items, mirror); return item; } async function enqueueDurably(draft) { - const item = enqueue(draft, false); if (!backgroundSync?.reconcile || !backgroundSync?.requestSync) { + const item = enqueue(draft, false); return { item, background: false, durability: 'foreground-only' }; } + const item = prepareItem(draft); + const current = read(); try { - await backgroundSync.reconcile(read()); + await backgroundSync.reconcile([...current, item]); + const localItem = localIndexItem(item); + write([...current, item].map(localIndexItem), false); await backgroundSync.requestSync(); - return { item, background: true, durability: 'background' }; + return { item: localItem, background: true, durability: 'background' }; } catch (error) { - return { item, background: false, durability: 'foreground-only', error }; + const persisted = read().find(candidate => candidate.id === item.id); + if (!persisted) throw error; + return { item: persisted, background: false, durability: 'foreground-only', error }; } } - function update(id, draft, mirror = true) { + function prepareUpdate(id, draft) { let updated = null; - write(read().map(item => { + const items = read().map(item => { if (item.id !== id) return item; const nextRepository = String(draft?.repository || ''); const nextTitle = String(draft?.title || ''); @@ -109,21 +135,33 @@ function createIssueOutbox({ storage, fetchJson, coordinator, backgroundSync, ge delete updated.error; delete updated.deliveryState; return updated; - }), mirror); + }); + return { items, updated }; + } + + function update(id, draft, mirror = true) { + const { items, updated } = prepareUpdate(id, draft); + write(items, mirror); return updated; } async function updateDurably(id, draft) { - const item = update(id, draft, false); - if (!item || !backgroundSync?.reconcile || !backgroundSync?.requestSync) { + if (!backgroundSync?.reconcile || !backgroundSync?.requestSync) { + const item = update(id, draft, false); return { item, background: false, durability: 'foreground-only' }; } + const { items, updated: item } = prepareUpdate(id, draft); + if (!item) return { item, background: false, durability: 'foreground-only' }; try { - await backgroundSync.reconcile(read()); + await backgroundSync.reconcile(items); + const localItems = items.map(localIndexItem); + write(localItems, false); await backgroundSync.requestSync(); - return { item, background: true, durability: 'background' }; + return { item: localIndexItem(item), background: true, durability: 'background' }; } catch (error) { - return { item, background: false, durability: 'foreground-only', error }; + const persisted = read().find(candidate => candidate.id === id); + if (!persisted || persisted.operationId !== item.operationId) throw error; + return { item: persisted, background: false, durability: 'foreground-only', error }; } } @@ -318,10 +356,10 @@ function createIssueOutbox({ storage, fetchJson, coordinator, backgroundSync, ge return []; } if (background?.status === 'attention') return [{ - ...item, ...deliveryStage, status: 'attention', error: String(background.error || 'Issue needs attention').slice(0, 240), + ...localIndexItem(item), ...deliveryStage, status: 'attention', error: String(background.error || 'Issue needs attention').slice(0, 240), ...(background.deliveryState ? { deliveryState: background.deliveryState } : {}), }]; - return [{ ...item, ...deliveryStage }]; + return [{ ...localIndexItem(item), ...deliveryStage }]; }); write(items); return items; diff --git a/tests/test_background_issue_sync.py b/tests/test_background_issue_sync.py index 0bf1f30..215071d 100644 --- a/tests/test_background_issue_sync.py +++ b/tests/test_background_issue_sync.py @@ -354,6 +354,35 @@ const transaction=work=>{{const run=tail.then(()=>work({{ ] +def test_reconciling_a_lightweight_attachment_reference_preserves_indexeddb_bytes(): + script = f""" +const createBackgroundIssueSync = require({json.dumps(str(SYNC))}); +const records=new Map();let tail=Promise.resolve(); +const transaction=work=>{{const run=tail.then(()=>work({{ + getAll:async()=>[...records.values()].map(value=>({{...value}})), + put:async value=>records.set(value.id,{{...value}}),delete:async id=>records.delete(id), +}}));tail=run.catch(()=>{{}});return run;}}; +(async()=>{{ + const store=createBackgroundIssueSync.createIssueSyncStore({{transaction}}); + await store.reconcile([{{ + id:'first',operationId:'first',ownerLogin:'timmy',status:'queued', + attachment:{{filename:'one.png',contentType:'image/png',data:'first-image-bytes'}}, + }}]); + await store.reconcile([ + {{id:'first',operationId:'first',ownerLogin:'timmy',status:'queued', + attachment:{{filename:'one.png',contentType:'image/png',stored:true}}}}, + {{id:'second',operationId:'second',ownerLogin:'timmy',status:'queued', + attachment:{{filename:'two.png',contentType:'image/png',data:'second-image-bytes'}}}}, + ]); + process.stdout.write(JSON.stringify(await store.snapshot())); +}})(); +""" + output = run_node(script) + + assert output[0]["attachment"]["data"] == "first-image-bytes" + assert output[1]["attachment"]["data"] == "second-image-bytes" + + def test_stale_foreground_upsert_preserves_confirmed_attachment_delivery_stages(): script = f""" const createBackgroundIssueSync=require({json.dumps(str(SYNC))}); diff --git a/tests/test_issue_outbox.py b/tests/test_issue_outbox.py index 167d071..c9cec38 100644 --- a/tests/test_issue_outbox.py +++ b/tests/test_issue_outbox.py @@ -403,6 +403,117 @@ Promise.resolve().then(async () => {{ assert output["items"][0]["title"] == "Keep this" +def test_durable_screenshot_is_mirrored_before_a_payload_free_local_index_is_committed(): + script = f""" +const createIssueOutbox = require({json.dumps(str(OUTBOX))}); +const values = new Map(); const events = []; const snapshots = []; +const storage = {{ + getItem:key => values.get(key) || null, + setItem:(key,value) => {{ + if (value.includes('base64-screenshot-bytes')) throw new Error('screenshot leaked into localStorage'); + events.push('local-index'); values.set(key,value); + }}, +}}; +const outbox = createIssueOutbox({{ + storage, getOwnerLogin:()=> 'timmy', createOperationId:()=> 'quota-safe-1', + backgroundSync: {{ + reconcile: async items => {{ events.push('indexeddb'); snapshots.push(items); }}, + requestSync: async () => {{ events.push('sync'); }}, + }}, +}}); +(async () => {{ + const result = await outbox.enqueueDurably({{ + repository:'stackchain/dashboard', title:'Mobile layout', + attachment:{{filename:'phone.webp',contentType:'image/webp',data:'base64-screenshot-bytes'}}, + }}); + process.stdout.write(JSON.stringify({{ + events, mirrored:snapshots[0][0], local:outbox.list()[0], result:result.item, + raw:values.get('stackchain.issue-outbox.v1'), + }})); +}})(); +""" + output = run_node(script) + + assert output["events"] == ["indexeddb", "local-index", "sync"] + assert output["mirrored"]["attachment"]["data"] == "base64-screenshot-bytes" + assert output["local"]["attachment"] == { + "filename": "phone.webp", + "contentType": "image/webp", + "stored": True, + } + assert "base64-screenshot-bytes" not in output["raw"] + assert output["result"]["attachment"] == output["local"]["attachment"] + + +def test_replacing_a_durable_screenshot_updates_indexeddb_before_the_local_reference(): + script = f""" +const createIssueOutbox = require({json.dumps(str(OUTBOX))}); +const values = new Map(); const events = []; const snapshots = []; +const storage = {{ + getItem:key => values.get(key) || null, + setItem:(key,value) => {{ + if (value.includes('new-image-bytes')) throw new Error('replacement leaked into localStorage'); + events.push('local-index'); values.set(key,value); + }}, +}}; +const backgroundSync = {{ + reconcile:async items => {{ events.push('indexeddb'); snapshots.push(items); }}, + requestSync:async () => {{ events.push('sync'); }}, +}}; +const outbox = createIssueOutbox({{ + storage, backgroundSync, getOwnerLogin:()=> 'timmy', + createOperationId:(() => {{ let sequence=0; return () => 'replace-' + (++sequence); }})(), +}}); +(async () => {{ + const admitted = await outbox.enqueueDurably({{repository:'o/r',title:'Visual'}}); + events.length = 0; + const result = await outbox.updateDurably(admitted.item.id, {{ + ...admitted.item, + attachment:{{filename:'new.png',contentType:'image/png',data:'new-image-bytes'}}, + }}); + process.stdout.write(JSON.stringify({{ + events, mirrored:snapshots.at(-1)[0], local:outbox.list()[0], result:result.item, + }})); +}})(); +""" + output = run_node(script) + + assert output["events"] == ["indexeddb", "local-index", "sync"] + assert output["mirrored"]["attachment"]["data"] == "new-image-bytes" + assert output["local"]["attachment"]["stored"] is True + assert "data" not in output["local"]["attachment"] + assert output["result"] == output["local"] + + +def test_next_durable_admission_migrates_legacy_screenshot_payloads_out_of_localstorage(): + script = f""" +const createIssueOutbox = require({json.dumps(str(OUTBOX))}); +const values = new Map([['stackchain.issue-outbox.v1', JSON.stringify({{version:3,items:[{{ + id:'legacy',operationId:'legacy',repository:'o/r',title:'Queued before upgrade',body:'', + labelIds:[],ownerLogin:'timmy',status:'queued',queuedAt:1, + attachment:{{filename:'old.png',contentType:'image/png',data:'legacy-image-bytes'}}, +}}]}})]]); +const mirrored = []; +const storage = {{getItem:k=>values.get(k)||null,setItem:(k,v)=>values.set(k,v)}}; +const outbox = createIssueOutbox({{ + storage, getOwnerLogin:()=> 'timmy', createOperationId:()=> 'new-item', + backgroundSync:{{reconcile:async items=>mirrored.push(items),requestSync:async()=>{{}}}}, +}}); +(async()=>{{ + await outbox.enqueueDurably({{repository:'o/r',title:'New queue item'}}); + process.stdout.write(JSON.stringify({{ + mirrored:mirrored[0],local:outbox.list(),raw:values.get('stackchain.issue-outbox.v1'), + }})); +}})(); +""" + output = run_node(script) + + assert output["mirrored"][0]["attachment"]["data"] == "legacy-image-bytes" + assert output["local"][0]["attachment"]["stored"] is True + assert "data" not in output["local"][0]["attachment"] + assert "legacy-image-bytes" not in output["raw"] + + def test_issue_outbox_reports_degraded_admission_without_losing_foreground_item(): script = f""" const createIssueOutbox = require({json.dumps(str(OUTBOX))});