diff --git a/frontend/dashboard.js b/frontend/dashboard.js index 9e0749d..1f4be91 100644 --- a/frontend/dashboard.js +++ b/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 }); diff --git a/frontend/today-sync.js b/frontend/today-sync.js index b3532c0..a6bf255 100644 --- a/frontend/today-sync.js +++ b/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; } } diff --git a/tests/e2e/test_mobile_today_handoff_release.py b/tests/e2e/test_mobile_today_handoff_release.py index 122442f..d999354 100644 --- a/tests/e2e/test_mobile_today_handoff_release.py +++ b/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 == [] diff --git a/tests/test_today_sync.py b/tests/test_today_sync.py index a6f0ee6..41db9ab 100644 --- a/tests/test_today_sync.py +++ b/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))});