Merge pull request 'Continue mobile checklist progress across offline changes' (#910) from timmy/909-offline-checklist-coalesce into main

frontend/authored-outbox.js

@@ -61,6 +61,21 @@ function createAuthoredOutbox({ storage, fetchJson, coordinator, backgroundSync,
         throw new Error('A review for this saved head is already queued. Open Drafts to inspect or discard it first.');
       }
     }
+    if (message.kind === 'issue-content') {
+      const queuedContent = items.find(item => item.kind === 'issue-content' && item.status === 'queued' &&
+        item.ownerLogin === ownerLogin && item.repository === String(message.repository || '') &&
+        item.number === Number(message.number || 0));
+      if (queuedContent) {
+        const replacement = {
+          ...queuedContent,
+          operationId: String(requestedOperationId || makeId()).slice(0, 128),
+          title: String(message.title || ''),
+          body: String(message.body || ''),
+        };
+        write(items.map(item => item.id === queuedContent.id ? replacement : item), mirror);
+        return replacement;
+      }
+    }
     if (items.length >= maxItems) throw new Error('Message outbox is full. Send or discard a queued message first.');
     const id = String(requestedOperationId || makeId()).slice(0, 128);
     const item = {
@@ -107,6 +122,7 @@ function createAuthoredOutbox({ storage, fetchJson, coordinator, backgroundSync,
   }
 
   async function enqueueDurably(message) {
+    const previousItems = read();
     const item = enqueue(message, false);
     if (message.attachment && ['update-reply', 'update-reply-read'].includes(message.kind) &&
         (!backgroundSync?.reconcile || !backgroundSync?.requestSync)) {
@@ -128,7 +144,7 @@ function createAuthoredOutbox({ storage, fetchJson, coordinator, backgroundSync,
     try {
       await backgroundSync.reconcile(durableItems, 'authored');
     } catch (error) {
-      write(read().filter(candidate => candidate.id !== item.id), false);
+      write(previousItems, false);
       throw error;
     }
     try {

frontend/background-issue-sync.js

@@ -105,6 +105,11 @@ function createIssueSyncStore({
           };
           incoming.set(current.id, replacement);
         }
+        if (current.kind === 'issue-content' && replacement.kind === 'issue-content' &&
+            current.operationId !== replacement.operationId && current.status === 'sending' &&
+            Number(current.claimUntil) > Number(now())) {
+          throw new Error('Checklist sync is already delivering. Retry this change.');
+        }
         if ((current.status === 'sending' && Number(current.claimUntil) > Number(now())) ||
             (['attention', 'authorization'].includes(current.status) &&
               replacement.status === current.status) ||

frontend/issue-sheet.js

@@ -200,7 +200,7 @@ function createIssueSheet({ fetchJson, storage, renderMarkdown = globalThis.rend
       container.classList.toggle('checklist-pending', detail.checklist_pending === true);
       container.innerHTML = renderMarkdown(
         detail.body || 'No description provided.', {
-          interactiveTasks: Boolean(interactive) && detail.checklist_pending !== true,
+          interactiveTasks: Boolean(interactive),
         }
       );
     },

tests/test_authored_outbox.py

@@ -113,6 +113,69 @@ outbox.flush('timmy').then(result => process.stdout.write(JSON.stringify({{queue
     assert output["remaining"] == []
 
 
+def test_durable_issue_content_admission_coalesces_latest_body_on_original_revision():
+    script = f"""
+const createAuthoredOutbox = require({json.dumps(str(OUTBOX))});
+const values = new Map(); const mirrors = []; const ids = ['check-1', 'check-2'];
+const storage = {{getItem:k=>values.get(k)||null,setItem:(k,v)=>values.set(k,v)}};
+const outbox = createAuthoredOutbox({{
+  storage, getOwnerLogin:()=> 'timmy', createOperationId:()=>ids.shift(),
+  backgroundSync:{{
+    reconcile:async items=>mirrors.push(items.map(item=>({{...item}}))),
+    requestSync:async()=>{{}},
+  }},
+}});
+(async()=>{{
+  await outbox.enqueueDurably({{kind:'issue-content',repository:'stackchain/dashboard',number:17,
+    title:'Ship',body:'- [x] Build\\n- [ ] Test',expectedUpdatedAt:'server-revision'}});
+  const second = await outbox.enqueueDurably({{kind:'issue-content',repository:'stackchain/dashboard',number:17,
+    title:'Ship',body:'- [x] Build\\n- [x] Test',expectedUpdatedAt:'pending-local-revision'}});
+  process.stdout.write(JSON.stringify({{items:outbox.list(),second,mirrors}}));
+}})();
+"""
+    output = run_node(script)
+
+    assert len(output["items"]) == 1
+    assert output["items"][0]["id"] == "check-1"
+    assert output["items"][0]["operationId"] == "check-2"
+    assert output["items"][0]["body"] == "- [x] Build\n- [x] Test"
+    assert output["items"][0]["expectedUpdatedAt"] == "server-revision"
+    assert output["second"]["item"] == output["items"][0]
+    assert len(output["mirrors"][1]) == 1
+    assert output["mirrors"][1][0]["operationId"] == "check-2"
+
+
+def test_failed_durable_issue_content_replacement_restores_previous_intent():
+    script = f"""
+const createAuthoredOutbox = require({json.dumps(str(OUTBOX))});
+const values = new Map(); const ids = ['check-1', 'check-2']; let admissions = 0;
+const storage = {{getItem:k=>values.get(k)||null,setItem:(k,v)=>values.set(k,v)}};
+const outbox = createAuthoredOutbox({{
+  storage, getOwnerLogin:()=> 'timmy', createOperationId:()=>ids.shift(),
+  backgroundSync:{{
+    reconcile:async()=>{{if (++admissions === 2) throw new Error('IndexedDB unavailable');}},
+    requestSync:async()=>{{}},
+  }},
+}});
+(async()=>{{
+  await outbox.enqueueDurably({{kind:'issue-content',repository:'o/r',number:9,
+    title:'Ship',body:'- [x] Build\\n- [ ] Test',expectedUpdatedAt:'server-revision'}});
+  let error = '';
+  try {{
+    await outbox.enqueueDurably({{kind:'issue-content',repository:'o/r',number:9,
+      title:'Ship',body:'- [x] Build\\n- [x] Test',expectedUpdatedAt:'server-revision'}});
+  }} catch (caught) {{ error = caught.message; }}
+  process.stdout.write(JSON.stringify({{error,items:outbox.list()}}));
+}})();
+"""
+    output = run_node(script)
+
+    assert output["error"] == "IndexedDB unavailable"
+    assert len(output["items"]) == 1
+    assert output["items"][0]["operationId"] == "check-1"
+    assert output["items"][0]["body"] == "- [x] Build\n- [ ] Test"
+
+
 def test_authored_outbox_persists_and_delivers_desired_blocker_state():
     script = f"""
 const createAuthoredOutbox = require({json.dumps(str(OUTBOX))});

tests/test_background_issue_sync.py

@@ -131,6 +131,34 @@ const fetchJson=async(url,options={{}})=>{{
     assert output["result"]["confirmed"][0]["body"] == "- [x] Test"
 
 
+def test_active_issue_content_delivery_rejects_a_false_durable_replacement():
+    script = f"""
+const createBackgroundIssueSync = require({json.dumps(str(SYNC))});
+const records = new Map([['check-1', {{
+  id:'check-1',operationId:'old-op',kind:'issue-content',outboxLane:'authored',
+  ownerLogin:'timmy',status:'sending',claimUntil:200,body:'- [x] Build\\n- [ ] Test'
+}}]]); 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,now:()=>100}});
+  let error='';
+  try {{ await store.reconcile([{{
+    id:'check-1',operationId:'new-op',kind:'issue-content',ownerLogin:'timmy',status:'queued',
+    body:'- [x] Build\\n- [x] Test'
+  }}], 'authored'); }} catch (caught) {{ error=caught.message; }}
+  process.stdout.write(JSON.stringify({{error,snapshot:await store.snapshot()}}));
+}})();
+"""
+    output = run_node(script)
+
+    assert output["error"] == "Checklist sync is already delivering. Retry this change."
+    assert output["snapshot"][0]["operationId"] == "old-op"
+    assert output["snapshot"][0]["body"] == "- [x] Build\n- [ ] Test"
+
+
 def test_closed_app_sync_delivers_desired_blocker_state():
     script = f"""
 const createBackgroundIssueSync = require({json.dumps(str(SYNC))});

tests/test_gitea_transport.py

@@ -234,7 +234,7 @@ async def test_streaming_reads_share_the_same_bounded_read_capacity():
     try:
         await asyncio.wait_for(saturated.wait(), timeout=0.2)
         with pytest.raises(gitea_proxy.GiteaOverloadedError):
-            await asyncio.wait_for(gitea_proxy.fetch("third"), timeout=0.1)
+            await asyncio.wait_for(gitea_proxy.fetch("third"), timeout=1.0)
     finally:
         release.set()
         await asyncio.gather(*streams)

tests/test_my_work.py

@@ -2287,6 +2287,30 @@ process.stdout.write(JSON.stringify({{
     }
 
 
+def test_pending_offline_checklist_remains_interactive_for_the_next_change():
+    script = f"""
+const createIssueSheet = require({json.dumps(str(ISSUE_SHEET))});
+let options = null; const classes = [];
+const container = {{
+  classList:{{toggle:(name,present)=>classes.push([name,present])}},
+  innerHTML:'',
+}};
+const controller = createIssueSheet({{
+  storage:null,
+  renderMarkdown:(body, received)=>{{options=received;return '<label><input class="task-list-toggle"></label>';}}
+}});
+controller.renderTasks(container, {{body:'- [x] Build\\n- [ ] Test',checklist_pending:true}}, true);
+process.stdout.write(JSON.stringify({{options,classes,html:container.innerHTML}}));
+"""
+    output = json.loads(subprocess.run(
+        ["node", "-e", script], check=True, capture_output=True, text=True
+    ).stdout)
+
+    assert output["options"] == {"interactiveTasks": True}
+    assert output["classes"] == [["checklist-pending", True]]
+    assert "task-list-toggle" in output["html"]
+
+
 def test_issue_checklist_toggle_submits_exact_revision_checked_body_once():
     script = f"""
 const createIssueSheet = require({json.dumps(str(ISSUE_SHEET))});