Merge pull request 'Keep offline screenshots outside localStorage quota' (#474) from timmy/473-indexeddb-screenshot-outbox into main

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

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 };

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;

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))});

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))});