fix: reconcile concurrent Draft saves (Closes #857)

2026-08-14 23:28:12 +00:00 · 2026-08-14 23:28:12 +00:00 · 8a3316a5c4
commit 8a3316a5c4
parent 631eb2e584
3 changed files with 192 additions and 19 deletions
--- a/frontend/index.html
+++ b/frontend/index.html
@ -252,6 +252,7 @@
    <div class="small" id="notification-page-status" aria-live="polite"></div>
    <button class="load-more-notifications" id="load-more-notifications" type="button" hidden>Load older updates</button>
    <div class="small" id="my-work-action-status" aria-live="assertive"></div>
+    <button class="retry-work-route" id="retry-unfiled-draft-sync" type="button" hidden>Retry Draft sync</button>
    <div class="small" id="today-sync-status" aria-live="polite">Today is saved on this device.</div>
    <div class="small" id="later-sync-status" aria-live="polite">Later is saved on this device.</div>
    <button class="retry-work-route" id="retry-work-route" type="button" hidden>Retry shared work item</button>
--- a/frontend/unfiled-draft-sync.js
+++ b/frontend/unfiled-draft-sync.js
@ -6,12 +6,13 @@
  function createUnfiledDraftSync(options, mountedFetch) {
    if (mountedFetch) return createUnfiledDraftSync.mount(options, mountedFetch);
    const {
-      captures, fetchJson, storage, getLogin = () => '', onState = () => {},
+      captures, fetchJson, storage, getLogin = () => '', onState = () => {}, maxConflictRetries = 2,
      createConflictId = () => 'conflict-' + (globalThis.crypto?.randomUUID?.() || Date.now().toString(36)),
    } = options;
    const key = 'stackchain.unfiled-draft-sync.v1';
    let inFlight = null;
    let rerun = false;
+    let queuedOwner = '';

    function read(login) {
      try {
@ -33,8 +34,8 @@
      storage?.setItem(key, JSON.stringify(state.all));
    }

-    function publish(status, message) {
-      onState({status, ...(message ? {message} : {})});
+    function publish(status, message, details = {}) {
+      onState({status, ...(message ? {message} : {}), ...details});
      const visibleStatus = globalThis.document?.querySelector?.('#my-work-action-status');
      if (message && visibleStatus) visibleStatus.textContent = message;
    }
@ -131,9 +132,7 @@
          : 'Drafts synced across devices.');
        return true;
      } catch (error) {
-        if (error?.status === 409) {
-          publish('conflict', 'Drafts changed on another device. Sync again to combine them.');
-        } else {
+        if (error?.status !== 409) {
          publish('pending', 'Saved on this device · sync pending');
        }
        throw error;
@ -145,13 +144,32 @@
      if (!owner) return Promise.resolve(false);
      if (inFlight) {
        rerun = true;
+        queuedOwner = owner;
        return inFlight;
      }
      inFlight = (async () => {
        let result = false;
+        let conflictRetries = 0;
+        let activeOwner = owner;
        do {
          rerun = false;
-          result = await runSync(owner);
+          try {
+            result = await runSync(activeOwner);
+          } catch (error) {
+            if (error?.status !== 409) throw error;
+            if (conflictRetries >= maxConflictRetries) {
+              publish('conflict', 'Draft sync needs attention. Retry when your devices finish saving.', {retryable:true});
+              throw error;
+            }
+            conflictRetries += 1;
+            rerun = true;
+          }
+          if (queuedOwner) {
+            if (queuedOwner !== activeOwner) conflictRetries = 0;
+            activeOwner = queuedOwner;
+            queuedOwner = '';
+            rerun = true;
+          }
        } while (rerun);
        return result;
      })().finally(() => { inFlight = null; });
@ -168,16 +186,29 @@
      return sync(owner);
    }

+    function retry() {
+      return sync(getLogin());
+    }
+
    captures.subscribe?.((id, removed) => {
      const login = getLogin();
      if (login) void (removed ? remove(id, login) : sync(login)).catch(() => {});
    });

-    return {sync, remove};
+    return {sync, remove, retry};
  }
  createUnfiledDraftSync.mount = (captures, fetchJson) => {
    const getLogin = captures.currentLogin;
-    const controller = createUnfiledDraftSync({captures, fetchJson, storage:globalThis.localStorage, getLogin});
+    const retryButton = globalThis.document?.querySelector?.('#retry-unfiled-draft-sync');
+    const controller = createUnfiledDraftSync({
+      captures, fetchJson, storage:globalThis.localStorage, getLogin,
+      onState:state => { if (retryButton) retryButton.hidden = !state.retryable; },
+    });
+    retryButton?.addEventListener?.('click', async () => {
+      retryButton.disabled = true;
+      try { await controller.retry(); } catch (_error) { /* state remains actionable */ }
+      finally { retryButton.disabled = false; }
+    });
    const connect = () => {
      const login = getLogin();
      if (login) void controller.sync(login).catch(() => {});
--- a/tests/test_unfiled_draft_sync.py
+++ b/tests/test_unfiled_draft_sync.py
@ -4,6 +4,7 @@ from pathlib import Path


 SYNC = Path(__file__).parents[1] / "frontend" / "unfiled-draft-sync.js"
+HTML = Path(__file__).parents[1] / "frontend" / "index.html"


 def run_node(script: str):
@ -58,25 +59,138 @@ function device(initial=[]) {{
    assert output["desktopStates"][-1] == "ready"


-def test_unfiled_draft_sync_surfaces_revision_conflict_without_losing_local_draft():
+def test_unfiled_draft_sync_rebases_and_retries_a_revision_race_without_user_action():
    script = f"""
 const createSync=require({json.dumps(str(SYNC))});
-let calls=0;const states=[];let drafts=[{{id:'local',title:'Local',body:'',saved_at:1}}];
-const conflict={{revision:2,drafts:[{{id:'remote',title:'Remote',body:'',saved_at:2}}]}};
+let calls=0, puts=0;const states=[];
+let drafts=[{{id:'local',title:'Local',body:'',saved_at:1}}];
+let remote={{revision:1,drafts:[]}};
 const sync=createSync({{
-  captures:{{exportOwned:async()=>drafts,mergeRemote:async()=>0,discard:async()=>true}},
+  captures:{{
+    exportOwned:async()=>drafts,
+    reconcileRemote:async items=>{{drafts=JSON.parse(JSON.stringify(items))}},
+    discard:async()=>true,
+  }},
  storage:{{getItem:()=>null,setItem:()=>{{}}}},
-  fetchJson:async(_url,init)=>{{calls++;if(!init)return {{revision:1,drafts:[]}};const error=new Error('conflict');error.status=409;error.payload={{detail:{{snapshot:conflict}}}};throw error}},
+  fetchJson:async(_url,init)=>{{
+    calls++;
+    if(!init)return JSON.parse(JSON.stringify(remote));
+    puts++;
+    const body=JSON.parse(init.body);
+    if(puts===1){{
+      remote={{revision:2,drafts:[{{id:'remote',title:'Remote',body:'',saved_at:2}}]}};
+      const error=new Error('conflict');error.status=409;
+      error.payload={{detail:{{snapshot:JSON.parse(JSON.stringify(remote))}}}};
+      throw error;
+    }}
+    if(body.revision!==remote.revision)throw new Error('stale retry');
+    remote={{revision:remote.revision+1,drafts:body.drafts}};
+    return JSON.parse(JSON.stringify(remote));
+  }},
  onState:state=>states.push(state),
 }});
-(async()=>{{let message='';try{{await sync.sync('timmy')}}catch(error){{message=error.message}}process.stdout.write(JSON.stringify({{calls,states,message,drafts}}))}})();
+(async()=>{{
+  const result=await sync.sync('timmy');
+  process.stdout.write(JSON.stringify({{calls,puts,states,result,drafts,remote}}));
+}})().catch(error=>{{console.error(error);process.exit(1)}});
 """
    output = run_node(script)

-    assert output["calls"] == 2
-    assert output["states"][-1]["status"] == "conflict"
-    assert output["states"][-1]["message"] == "Drafts changed on another device. Sync again to combine them."
-    assert output["drafts"][0]["id"] == "local"
+    assert output["result"] is True
+    assert output["puts"] == 2
+    assert output["calls"] == 4
+    assert [item["id"] for item in output["remote"]["drafts"]] == ["remote", "local"]
+    assert output["drafts"] == output["remote"]["drafts"]
+    assert output["states"][-1]["status"] == "ready"
+
+
+def test_unfiled_draft_sync_bounds_conflicts_and_exposes_a_retry_path():
+    script = f"""
+const createSync=require({json.dumps(str(SYNC))});
+let contended=true, puts=0;
+let drafts=[{{id:'local',title:'Local',body:'kept',saved_at:1}}];
+let remote={{revision:1,drafts:[]}};const states=[];
+const controller=createSync({{
+  captures:{{
+    exportOwned:async()=>JSON.parse(JSON.stringify(drafts)),
+    reconcileRemote:async items=>{{drafts=JSON.parse(JSON.stringify(items))}},
+    discard:async()=>true,
+  }},
+  storage:{{getItem:()=>null,setItem:()=>{{}}}},getLogin:()=> 'timmy',maxConflictRetries:1,
+  fetchJson:async(_url,init)=>{{
+    if(!init)return JSON.parse(JSON.stringify(remote));
+    puts++;const body=JSON.parse(init.body);
+    if(contended){{
+      remote={{revision:remote.revision+1,drafts:remote.drafts}};
+      const error=new Error('conflict');error.status=409;throw error;
+    }}
+    remote={{revision:remote.revision+1,drafts:body.drafts}};
+    return JSON.parse(JSON.stringify(remote));
+  }},
+  onState:state=>states.push(state),
+}});
+(async()=>{{
+  let rejected=false;
+  try{{await controller.sync('timmy')}}catch(error){{rejected=error.status===409}}
+  const afterExhaustion={{puts,rejected,state:states.at(-1),drafts:JSON.parse(JSON.stringify(drafts))}};
+  contended=false;
+  const retried=await controller.retry();
+  process.stdout.write(JSON.stringify({{afterExhaustion,retried,puts,state:states.at(-1),drafts,remote}}));
+}})().catch(error=>{{console.error(error);process.exit(1)}});
+"""
+    output = run_node(script)
+
+    assert output["afterExhaustion"]["puts"] == 2
+    assert output["afterExhaustion"]["rejected"] is True
+    assert output["afterExhaustion"]["state"] == {
+        "status": "conflict",
+        "message": "Draft sync needs attention. Retry when your devices finish saving.",
+        "retryable": True,
+    }
+    assert output["afterExhaustion"]["drafts"][0]["body"] == "kept"
+    assert output["retried"] is True
+    assert output["state"]["status"] == "ready"
+    assert output["drafts"] == output["remote"]["drafts"]
+
+
+def test_mounted_unfiled_draft_sync_shows_a_working_retry_action_after_exhaustion():
+    assert 'id="retry-unfiled-draft-sync"' in HTML.read_text()
+    script = f"""
+let clickHandler;const status={{textContent:''}};
+const button={{hidden:true,disabled:false,textContent:'Retry Draft sync',addEventListener:(_name,handler)=>clickHandler=handler}};
+global.document={{querySelector:selector=>selector==='#retry-unfiled-draft-sync'?button:(selector==='#my-work-action-status'?status:null)}};
+global.localStorage={{getItem:()=>null,setItem:()=>{{}}}};
+global.setTimeout=()=>0;global.addEventListener=()=>{{}};
+const createSync=require({json.dumps(str(SYNC))});
+let contended=true;let remote={{revision:0,drafts:[]}};
+const captures={{
+  currentLogin:()=> 'timmy',exportOwned:async()=>[{{id:'local',title:'Local',body:'',saved_at:1}}],
+  reconcileRemote:async()=>{{}},discard:async()=>true,
+}};
+const api=async(_url,init)=>{{
+  if(!init)return remote;
+  if(contended){{remote={{revision:remote.revision+1,drafts:[]}};const error=new Error('conflict');error.status=409;throw error}}
+  const body=JSON.parse(init.body);remote={{revision:remote.revision+1,drafts:body.drafts}};return remote;
+}};
+(async()=>{{
+  const controller=createSync.mount(captures,api);
+  try{{await controller.sync('timmy')}}catch(_error){{}}
+  const exhausted={{hidden:button.hidden,label:button.textContent,status:status.textContent}};
+  contended=false;
+  await clickHandler();
+  process.stdout.write(JSON.stringify({{exhausted,after:{{hidden:button.hidden,disabled:button.disabled,status:status.textContent}},remote}}));
+}})().catch(error=>{{console.error(error);process.exit(1)}});
+"""
+    output = run_node(script)
+
+    assert output["exhausted"] == {
+        "hidden": False,
+        "label": "Retry Draft sync",
+        "status": "Draft sync needs attention. Retry when your devices finish saving.",
+    }
+    assert output["after"]["hidden"] is True
+    assert output["after"]["disabled"] is False
+    assert [draft["id"] for draft in output["remote"]["drafts"]] == ["local"]


 def test_unfiled_draft_sync_coalesces_overlapping_reconnects_without_self_conflict():
@ -104,6 +218,33 @@ const sync=createSync({{
    assert output["results"] == [True, True]


+def test_unfiled_draft_sync_does_not_drop_an_account_transition_during_an_active_sync():
+    script = f"""
+const createSync=require({json.dumps(str(SYNC))});
+let remote={{revision:0,drafts:[]}}, release;
+const gate=new Promise(resolve=>release=resolve);let firstGet=true;const exported=[];
+const api=async(_url,init)=>{{
+  if(!init){{if(firstGet){{firstGet=false;await gate}}return JSON.parse(JSON.stringify(remote))}}
+  const body=JSON.parse(init.body);remote={{revision:remote.revision+1,drafts:body.drafts}};return JSON.parse(JSON.stringify(remote));
+}};
+const captures={{
+  exportOwned:async owner=>{{exported.push(owner);return [{{id:owner,title:owner,body:'',saved_at:owner==='alice'?1:2}}]}},
+  reconcileRemote:async()=>{{}},discard:async()=>true,
+}};
+const sync=createSync({{captures,fetchJson:api,storage:{{getItem:()=>null,setItem:()=>{{}}}}}});
+(async()=>{{
+  const alice=sync.sync('alice');const bob=sync.sync('bob');release();
+  const results=await Promise.all([alice,bob]);
+  process.stdout.write(JSON.stringify({{exported,results,remote}}));
+}})().catch(error=>{{console.error(error);process.exit(1)}});
+"""
+    output = run_node(script)
+
+    assert output["exported"] == ["alice", "bob"]
+    assert output["results"] == [True, True]
+    assert [draft["id"] for draft in output["remote"]["drafts"]] == ["bob", "alice"]
+
+
 def test_unfiled_draft_sync_preserves_divergent_same_draft_edits_as_a_conflict_copy():
    script = f"""
 const createSync=require({json.dumps(str(SYNC))});