Merge pull request 'Make mobile Recent Work sync generation-safe and self-healing' (#1486) from timmy/1485-mobile-recent-work-sync into main

frontend/mobile-recent-work.js

@@ -15,9 +15,42 @@
   const setTimer = options.setTimeout || setTimeout;
   const clearTimer = options.clearTimeout || clearTimeout;
   const debounceMs = Number.isFinite(options.debounceMs) ? Math.max(0, options.debounceMs) : 150;
+  const retryMs = Number.isFinite(options.retryMs) ? Math.max(1, options.retryMs) : 1000;
+  const retryMaxMs = Number.isFinite(options.retryMaxMs) ? Math.max(retryMs, options.retryMaxMs) : 30000;
   let syncFlight = null;
   let syncAccount = '';
   let debounceTimer = null;
+  let retryTimer = null;
+  let retryAccount = '';
+  let retryAttempt = 0;
+  let operationSequence = 0;
+
+  function operationId() {
+    operationSequence += 1;
+    return Date.now().toString(36) + '-' + operationSequence.toString(36);
+  }
+
+  function clearRetry(resetAttempt = false) {
+    if (retryTimer) clearTimer(retryTimer);
+    retryTimer = null;
+    retryAccount = '';
+    if (resetAttempt) retryAttempt = 0;
+  }
+
+  function scheduleRetry(accountKey) {
+    if (retryTimer || key() !== accountKey || !hasPending(read())) return false;
+    retryAccount = accountKey;
+    const delay = Math.min(retryMaxMs, retryMs * (2 ** retryAttempt));
+    retryAttempt += 1;
+    retryTimer = setTimer(() => {
+      const timer = retryTimer;
+      retryTimer = null;
+      retryAccount = '';
+      if (timer) clearTimer(timer);
+      if (key() === accountKey && hasPending(read())) void sync();
+    }, delay);
+    return true;
+  }
 
   function login() {
     return String(getLogin?.() || '').trim().toLowerCase();
@@ -66,13 +99,28 @@
       const route = action === 'pin' ? item?.route : String(candidate?.route || '');
       if ((action !== 'pin' && action !== 'unpin') || !route || (action === 'pin' && !item)) continue;
       if (!unique.some(existing => (existing.item?.route || existing.route) === route)) {
-        unique.push(action === 'pin' ? {action, item} : {action, route});
+        const normalized = action === 'pin' ? {action, item} : {action, route};
+        if (typeof candidate.operationId === 'string' && candidate.operationId) normalized.operationId = candidate.operationId;
+        unique.push(normalized);
       }
       if (unique.length === pinnedLimit) break;
     }
     return unique;
   }
 
+  function normalizePending(value) {
+    if (!Array.isArray(value)) return [];
+    const unique = [];
+    for (const candidate of value) {
+      const item = normalize(candidate);
+      if (!item || unique.some(existing => existing.route === item.route)) continue;
+      if (typeof candidate.operationId === 'string' && candidate.operationId) item.operationId = candidate.operationId;
+      unique.push(item);
+      if (unique.length === limit) break;
+    }
+    return unique;
+  }
+
   function empty() {
     return {items:[], pinned:[], pending:[], pinOps:[]};
   }
@@ -86,7 +134,7 @@
       return {
         items:normalizeList(parsed?.items),
         pinned:normalizeList(parsed?.pinned, pinnedLimit),
-        pending:normalizeList(parsed?.pending),
+        pending:normalizePending(parsed?.pending),
         pinOps:normalizePinOps(parsed?.pinOps),
       };
     } catch (_) {
@@ -100,7 +148,7 @@
       storage.setItem(accountKey, JSON.stringify({
         items:normalizeList(value.items),
         pinned:normalizeList(value.pinned, pinnedLimit),
-        pending:normalizeList(value.pending),
+        pending:normalizePending(value.pending),
         pinOps:normalizePinOps(value.pinOps),
       }));
       return true;
@@ -149,7 +197,7 @@
     const current = read();
     current.items = [normalized, ...current.items.filter(existing => existing.route !== normalized.route)].slice(0, limit);
     current.pinned = current.pinned.map(existing => existing.route === normalized.route ? normalized : existing);
-    current.pending = [normalized, ...current.pending.filter(existing => existing.route !== normalized.route)].slice(0, limit);
+    current.pending = [{...normalized, operationId:operationId()}, ...current.pending.filter(existing => existing.route !== normalized.route)].slice(0, limit);
     if (!persist(current, accountKey)) return false;
     announce(current);
     render();
@@ -159,7 +207,7 @@
 
   function queuePinOp(current, operation) {
     const route = operation.item?.route || operation.route;
-    current.pinOps = [operation, ...current.pinOps.filter(existing => (existing.item?.route || existing.route) !== route)];
+    current.pinOps = [{...operation, operationId:operationId()}, ...current.pinOps.filter(existing => (existing.item?.route || existing.route) !== route)];
   }
 
   function pin(item) {
@@ -206,7 +254,7 @@
     if (key() !== accountKey || !snapshot || !Array.isArray(snapshot.items)) return false;
     const remote = normalizeList(snapshot.items);
     const remotePinned = normalizeList(snapshot.pinned, pinnedLimit);
-    const unsent = normalizeList(pending);
+    const unsent = normalizePending(pending);
     const unsentPinOps = normalizePinOps(pinOps);
     const value = {
       items:normalizeList([...unsent, ...remote]),
@@ -231,7 +279,7 @@
         let snapshot;
         if (sending) {
           snapshot = await fetchJson('api/v1/recent-work', {
-            method:'POST', headers:{'Content-Type':'application/json'}, body:JSON.stringify(sending),
+            method:'POST', headers:{'Content-Type':'application/json'}, body:JSON.stringify(normalize(sending)),
           });
         } else {
           const isPin = pinOperation.action === 'pin';
@@ -244,17 +292,21 @@
         if (key() !== accountKey) return read();
         const latest = read();
         const pending = sending
-          ? latest.pending.filter(item => item.route !== sending.route)
+          ? latest.pending.filter(item => item.operationId !== sending.operationId)
           : latest.pending;
         const pinOps = pinOperation
           ? latest.pinOps.filter(operation => {
             const sameRoute = (operation.item?.route || operation.route) === (pinOperation.item?.route || pinOperation.route);
-            return !sameRoute || operation.action !== pinOperation.action;
+            return !sameRoute || operation.action !== pinOperation.action || operation.operationId !== pinOperation.operationId;
           })
           : latest.pinOps;
         if (!adopt(snapshot, accountKey, pending, pinOps)) throw new Error('Recent work response is invalid.');
+        retryAttempt = 0;
       } catch (_error) {
-        if (key() === accountKey) announce(read());
+        if (key() === accountKey) {
+          announce(read());
+          scheduleRetry(accountKey);
+        }
         return read();
       }
     }
@@ -264,6 +316,8 @@
   function sync() {
     if (debounceTimer) { clearTimer(debounceTimer); debounceTimer = null; }
     const accountKey = key();
+    if (retryTimer && retryAccount !== accountKey) clearRetry(true);
+    else if (retryTimer) clearRetry(false);
     if (!fetchJson || !accountKey || !hasPending(read())) return Promise.resolve(read());
     if (syncFlight && syncAccount === accountKey) return syncFlight;
     syncAccount = accountKey;

tests/test_mobile_recent_work.py

@@ -159,6 +159,118 @@ process.stdout.write(JSON.stringify({{immediate,settled:recent.items(),status:st
     assert payload["calls"] == [["api/v1/recent-work", "POST"]]
 
 
+def test_recent_work_drains_a_newer_same_route_generation_after_an_inflight_response():
+    script = f"""
+const createRecentWork = require({json.dumps(str(RECENT_WORK))});
+(async()=>{{
+const values = new Map(); const calls=[];
+const storage={{getItem:key=>values.get(key)||null,setItem:(key,value)=>values.set(key,value)}};
+const oldItem={{kind:'issue',repository:'stackchain/dashboard',number:7,title:'Old title'}};
+const newItem={{kind:'issue',repository:'stackchain/dashboard',number:7,title:'New title'}};
+let releaseFirst;
+const firstResponse=new Promise(resolve=>{{releaseFirst=resolve;}});
+const recent=createRecentWork({{
+  storage,getLogin:()=>'alice',debounceMs:99999,
+  fetchJson:async (_url, options)=>{{
+    const sent=JSON.parse(options.body);
+    calls.push(sent.title);
+    if (calls.length === 1) return firstResponse;
+    return {{items:[{{...newItem,route:'#/my-work/issue/stackchain/dashboard/7'}}],pinned:[]}};
+  }},
+}});
+recent.record(oldItem);
+const syncing=recent.sync();
+await Promise.resolve();
+recent.record(newItem);
+releaseFirst({{items:[{{...oldItem,route:'#/my-work/issue/stackchain/dashboard/7'}}],pinned:[]}});
+await syncing;
+process.stdout.write(JSON.stringify({{calls,items:recent.items(),state:recent.state()}}));
+process.exit(0);
+}})().catch(error=>{{console.error(error);process.exit(1);}});
+"""
+    payload = run_node(script)
+
+    assert payload["calls"] == ["Old title", "New title"]
+    assert payload["items"][0]["title"] == "New title"
+    assert payload["state"] == {"pending": False, "pendingCount": 0}
+
+
+def test_recent_work_retries_a_transient_sync_failure_without_a_lifecycle_event():
+    script = f"""
+const createRecentWork = require({json.dumps(str(RECENT_WORK))});
+(async()=>{{
+const values=new Map(); const timers=[]; let calls=0;
+const recent=createRecentWork({{
+  storage:{{getItem:key=>values.get(key)||null,setItem:(key,value)=>values.set(key,value)}},
+  getLogin:()=>'alice',debounceMs:99999,retryMs:25,
+  setTimeout:(callback,delay)=>{{const timer={{callback,delay,cleared:false}};timers.push(timer);return timer;}},
+  clearTimeout:timer=>{{timer.cleared=true;}},
+  fetchJson:async (_url,options)=>{{
+    calls += 1;
+    if (calls === 1) throw new Error('temporary outage');
+    return {{items:[JSON.parse(options.body)],pinned:[]}};
+  }},
+}});
+recent.record({{kind:'issue',repository:'stackchain/dashboard',number:7,title:'Keep me'}});
+await recent.sync();
+const afterFailure={{calls,state:recent.state(),active:timers.filter(timer=>!timer.cleared).map(timer=>timer.delay)}};
+const retryTimer=timers.find(timer=>!timer.cleared);
+retryTimer?.callback();
+await new Promise(resolve=>setImmediate(resolve));
+const settled={{calls,state:recent.state(),active:timers.filter(timer=>!timer.cleared).length}};
+process.stdout.write(JSON.stringify({{afterFailure,settled}}));
+}})().catch(error=>{{console.error(error);process.exit(1);}});
+"""
+    payload = run_node(script)
+
+    assert payload["afterFailure"] == {
+        "calls": 1,
+        "state": {"pending": True, "pendingCount": 1},
+        "active": [25],
+    }
+    assert payload["settled"] == {
+        "calls": 2,
+        "state": {"pending": False, "pendingCount": 0},
+        "active": 0,
+    }
+
+
+def test_recent_work_drains_a_newer_pin_generation_after_an_inflight_response():
+    script = f"""
+const createRecentWork = require({json.dumps(str(RECENT_WORK))});
+(async()=>{{
+const values=new Map(); const calls=[];
+const oldItem={{kind:'issue',repository:'stackchain/dashboard',number:7,title:'Old pin'}};
+const newItem={{kind:'issue',repository:'stackchain/dashboard',number:7,title:'New pin'}};
+const route='#/my-work/issue/stackchain/dashboard/7';
+let releaseFirst;
+const firstResponse=new Promise(resolve=>{{releaseFirst=resolve;}});
+const recent=createRecentWork({{
+  storage:{{getItem:key=>values.get(key)||null,setItem:(key,value)=>values.set(key,value)}},
+  getLogin:()=>'alice',debounceMs:99999,
+  fetchJson:async (_url,options)=>{{
+    const sent=JSON.parse(options.body); calls.push(sent.title);
+    if (calls.length === 1) return firstResponse;
+    return {{items:[],pinned:[{{...newItem,route}}]}};
+  }},
+}});
+recent.pin(oldItem);
+const syncing=recent.sync();
+await Promise.resolve();
+recent.pin(newItem);
+releaseFirst({{items:[],pinned:[{{...oldItem,route}}]}});
+await syncing;
+process.stdout.write(JSON.stringify({{calls,pinned:recent.pinned(),state:recent.state()}}));
+process.exit(0);
+}})().catch(error=>{{console.error(error);process.exit(1);}});
+"""
+    payload = run_node(script)
+
+    assert payload["calls"] == ["Old pin", "New pin"]
+    assert payload["pinned"][0]["title"] == "New pin"
+    assert payload["state"] == {"pending": False, "pendingCount": 0}
+
+
 def test_recent_work_pins_offline_first_syncs_and_renders_separate_touch_actions():
     script = f"""
 const createRecentWork = require({json.dumps(str(RECENT_WORK))});