From cec5b936652b51f77abf9ba58cdb3354552ae8aa Mon Sep 17 00:00:00 2001 From: timmy Date: Thu, 27 Aug 2026 21:42:47 +0000 Subject: [PATCH] fix: make mobile recent work sync generation-safe (Closes #1485) --- frontend/mobile-recent-work.js | 74 +++++++++++++++++--- tests/test_mobile_recent_work.py | 112 +++++++++++++++++++++++++++++++ 2 files changed, 176 insertions(+), 10 deletions(-) diff --git a/frontend/mobile-recent-work.js b/frontend/mobile-recent-work.js index 9d65029..9374c98 100644 --- a/frontend/mobile-recent-work.js +++ b/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; diff --git a/tests/test_mobile_recent_work.py b/tests/test_mobile_recent_work.py index 37e63e2..9b21d49 100644 --- a/tests/test_mobile_recent_work.py +++ b/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))});