Merge pull request 'Recover valid Today edits from a corrupted offline queue' (#1043) from timmy/1042-recover-valid-today-edits into main

frontend/dashboard.js

@@ -376,11 +376,12 @@
     onStatus: (state, detail = {}) => {
       const status = qs('#today-sync-status');
       status.textContent = state === 'saved' ? 'Today saved to account.' :
-        (state === 'retrying' ? `Today saved on this device · retrying in ${Math.ceil(detail.delayMs / 1000)}s.` :
-          (state === 'pending' ? 'Today saved on this device · sync pending.' :
-            (state === 'full' ? 'Another device filled Today · showing its saved plan.' :
-              (state === 'expired' ? `Today edit expired after 30 days offline${detail.count > 1 ? 's' : ''} · account plan kept.` :
-                'Today sync unavailable · changes stay on this device.'))));
+        (state === 'recovered' ? `Today queue recovered · discarded ${detail.discarded} unreadable device record${detail.discarded === 1 ? '' : 's'}.` :
+          (state === 'retrying' ? `Today saved on this device · retrying in ${Math.ceil(detail.delayMs / 1000)}s.` :
+            (state === 'pending' ? 'Today saved on this device · sync pending.' :
+              (state === 'full' ? 'Another device filled Today · showing its saved plan.' :
+                (state === 'expired' ? `Today edit expired after 30 days offline${detail.count > 1 ? 's' : ''} · account plan kept.` :
+                  'Today sync unavailable · changes stay on this device.')))));
     },
   });
   todaySync.startLifecycle({ window, document });

frontend/today-sync.js

@@ -10,6 +10,8 @@ function createTodaySync({ storage, getLogin, fetchJson, onRemoteIds, onRemotePl
   let retryTimer = null;
   let retryAttempt = 0;
   let expiredCount = 0;
+  let discardedCount = 0;
+  let recoveryNotice = { discarded: 0, until: 0 };
   const knownOperationKeys = new Set();
 
   function cancelRetry() {
@@ -108,10 +110,28 @@ function createTodaySync({ storage, getLogin, fetchJson, onRemoteIds, onRemotePl
         const candidate = storage.key?.(index);
         if (candidate?.startsWith(recordPrefix)) keys.add(candidate);
       }
-      const records = [...keys].map(recordKey => {
-        const record = JSON.parse(storage.getItem(recordKey) || 'null');
-        return record ? { ...record, recordKey } : null;
-      }).filter(record => record?.operation);
+      const records = [];
+      for (const recordKey of keys) {
+        let record;
+        try {
+          record = JSON.parse(storage.getItem(recordKey) || 'null');
+        } catch (_error) {
+          storage.removeItem(recordKey);
+          knownOperationKeys.delete(recordKey);
+          discardedCount += 1;
+          continue;
+        }
+        const operation = record?.operation;
+        const valid = operation && typeof operation.operation_id === 'string' &&
+          ['add', 'remove', 'move', 'configure', 'rollover'].includes(operation.action) &&
+          typeof operation.item_id === 'string' && Number.isFinite(Number(record.queued_at));
+        if (valid) records.push({ ...record, recordKey });
+        else {
+          storage.removeItem(recordKey);
+          knownOperationKeys.delete(recordKey);
+          discardedCount += 1;
+        }
+      }
       const expired = records.filter(record => Number(record.queued_at) >= 1_000_000_000_000 &&
         now() - Number(record.queued_at) > maxOfflineMs);
       expired.forEach(record => {
@@ -272,14 +292,23 @@ function createTodaySync({ storage, getLogin, fetchJson, onRemoteIds, onRemotePl
         operations = pending();
       }
       adopt(plan);
-      onStatus?.(pending().length ? 'pending' : hadConflict ? 'full' :
-        expiredCount ? 'expired' : 'saved', expiredCount ? { count: expiredCount } : {});
+      const stillPending = pending().length;
+      if (discardedCount) recoveryNotice = { discarded: discardedCount, until: now() + 5000 };
+      const recovered = recoveryNotice.until > now() ? recoveryNotice.discarded : 0;
+      onStatus?.(stillPending ? 'pending' : hadConflict ? 'full' :
+        expiredCount ? 'expired' : recovered ? 'recovered' : 'saved',
+      expiredCount ? { count: expiredCount } : recovered ? { discarded: recovered } : {});
+      if (!stillPending) discardedCount = 0;
       retryAttempt = 0;
       cancelRetry();
       return !hadConflict;
     } catch (error) {
-      if (pending().length) scheduleRetry(error, ownerKey);
-      else onStatus?.('error');
+      const stillPending = pending().length;
+      if (discardedCount) recoveryNotice = { discarded: discardedCount, until: now() + 5000 };
+      const recovered = recoveryNotice.until > now() ? recoveryNotice.discarded : 0;
+      if (stillPending) scheduleRetry(error, ownerKey);
+      else onStatus?.(recovered ? 'recovered' : 'error', recovered ? { discarded: recovered } : {});
+      discardedCount = 0;
       return false;
     }
   }

tests/e2e/test_mobile_today_handoff_release.py

@@ -98,8 +98,18 @@ def test_release_artifact_plans_hands_off_and_opens_next_mobile_issue(tmp_path:
             page.locator('[data-today-break-minutes="5"]').click()
             expect(page.locator("#today-break-status")).to_have_text("On break · resume in 5:00")
             expect(page.locator("[data-mobile-today-toggle]")).to_have_text("Resume timer")
+            page.evaluate("""() => {
+              const prefix = 'stackchain.today-sync.v1.timmy.operation.';
+              localStorage.setItem(prefix + 'broken', '{not-json');
+            }""")
             page.reload(wait_until="networkidle")
             expect(page.locator("#my-work-status")).to_contain_text("2")
+            expect(page.locator("#today-sync-status")).to_have_text(
+                "Today queue recovered · discarded 1 unreadable device record."
+            )
+            assert page.evaluate("""() => !Object.keys(localStorage).some(
+              key => key.startsWith('stackchain.today-sync.v1.timmy.operation.')
+            )""")
             expect(page.locator("#today-break-status")).to_contain_text("On break · resume in")
             if page.locator("#issue-sheet").get_attribute("class") == "issue-sheet open":
                 page.locator("#close-issue-sheet").click()
@@ -159,6 +169,7 @@ def test_release_artifact_plans_hands_off_and_opens_next_mobile_issue(tmp_path:
 
             today = page.evaluate("JSON.parse(localStorage.getItem('stackchain.today-work.v1.timmy') || '[]')")
             assert today == ["issue:acme/mobile:41:", "issue:acme/mobile:42:"]
+
             assert page.evaluate("document.documentElement.scrollWidth <= window.innerWidth")
             assert browser_errors == []
             assert failed_responses == []

tests/test_today_sync.py

@@ -5,6 +5,7 @@ from pathlib import Path
 
 TODAY_SYNC = Path(__file__).parents[1] / "frontend" / "today-sync.js"
 COORDINATOR = Path(__file__).parents[1] / "frontend" / "outbox-coordinator.js"
+DASHBOARD = Path(__file__).parents[1] / "frontend" / "dashboard.js"
 
 
 def test_two_tabs_cannot_erase_each_others_today_edits_or_double_drain():
@@ -108,6 +109,44 @@ process.stdout.write(JSON.stringify(sync.pending().map(operation => operation.op
     ).stdout) == ["z-add-first", "a-move-second"]
 
 
+def test_corrupt_queue_record_does_not_hide_or_strand_valid_today_edits():
+    script = f"""
+const createTodaySync = require({json.dumps(str(TODAY_SYNC))});
+const prefix = 'stackchain.today-sync.v1.timmy.operation.';
+const values = new Map([
+  [prefix + 'first', JSON.stringify({{queued_at: 1000, operation: {{operation_id:'first', action:'add', item_id:'issue:r:1:'}}}})],
+  [prefix + 'broken', '{{not-json'],
+  [prefix + 'invalid', JSON.stringify({{queued_at: 1000, operation: {{operation_id:'invalid', action:'erase-everything'}}}})],
+  [prefix + 'second', JSON.stringify({{queued_at: 1001, operation: {{operation_id:'second', action:'move', item_id:'issue:r:2:', direction:'up'}}}})],
+]);
+const storage = {{get length(){{return values.size}},key:i=>[...values.keys()][i]||null,
+  getItem:k=>values.get(k)||null,setItem:(k,v)=>values.set(k,v),removeItem:k=>values.delete(k)}};
+const patches=[]; const statuses=[];
+const sync=createTodaySync({{storage,getLogin:()=> 'timmy',
+ fetchJson:async(_url,options={{}})=>{{const operations=JSON.parse(options.body).operations;patches.push(...operations);return {{revision:1,ids:['issue:r:1:','issue:r:2:'],accepted_operation_ids:operations.map(x=>x.operation_id)}}}},
+ onRemoteIds:()=>{{}},onStatus:(state,detail={{}})=>statuses.push([state,detail.discarded||0])}});
+(async()=>{{const result=await sync.flush();process.stdout.write(JSON.stringify({{
+ result,patches,statuses,remaining:[...values.keys()].filter(key=>key.startsWith(prefix)),pending:sync.pending()
+}}));}})();
+"""
+    result = json.loads(subprocess.run(
+        ["node", "-e", script], check=True, capture_output=True, text=True
+    ).stdout)
+
+    assert result["result"] is True
+    assert [item["operation_id"] for item in result["patches"]] == ["first", "second"]
+    assert result["pending"] == []
+    assert result["remaining"] == []
+    assert result["statuses"][-1] == ["recovered", 2]
+
+
+def test_today_sync_reports_queue_recovery_instead_of_plain_saved_status():
+    source = DASHBOARD.read_text()
+
+    assert "Today queue recovered" in source
+    assert "discarded ${detail.discarded} unreadable device record" in source
+
+
 def test_today_capacity_configuration_syncs_offline_and_adopts_remote_plan():
     script = f"""
 const createTodaySync = require({json.dumps(str(TODAY_SYNC))});