Merge pull request 'Make mobile Update decisions transaction-safe' (#772) from timmy/771-transaction-safe-update-decisions into main

frontend/dashboard.js

@@ -940,6 +940,13 @@
     event.currentTarget.setAttribute('aria-expanded', String(more.open));
     if (more.open) more.scrollIntoView({ behavior:'smooth', block:'end' });
   });
+  const updateDecision = createUpdateDecisionTransaction({
+    controls: [
+      qs('#keep-update-unread'), qs('#mark-update-read-next'),
+      qs('#focus-update-reply'), qs('#toggle-update-more'),
+    ],
+    status: qs('#update-gesture-status'),
+  });
   const notificationReader = createNotificationReader({
     load: fetchNotificationDetail,
     getScope: () => confirmedOwnerLogin,
@@ -5554,13 +5561,17 @@
       button.focus();
     }
   });
-  qs('#keep-update-unread').addEventListener('click', () => {
+  const updateDecisionActions = createUpdateDecisionActions({
-    if (updateTriage.active()) updateTriage.keepUnreadAndNext();
+    transaction:updateDecision, triage:updateTriage, close:()=>closeUpdateSheet(true),
-    else closeUpdateSheet(true);
+    reader:notificationReader, items:()=>lastMyWork, undo:notificationUndo,
+    keepControl:qs('#keep-update-unread'), readControl:qs('#mark-update-read-next'),
   });
+  const keepUpdateUnread = () => updateDecisionActions.keepUnread();
+  const markUpdateRead = () => updateDecisionActions.markRead();
+  qs('#keep-update-unread').addEventListener('click', keepUpdateUnread);
   createUpdateTriageGesture({
     surface: qs('#update-sheet .update-sheet-panel'), enabled: () => selectedUpdateDetail,
-    keepUnread: () => qs('#keep-update-unread').click(), markRead: () => qs('#mark-update-read-next').click(),
+    keepUnread: keepUpdateUnread, markRead: markUpdateRead,
   });
   qs('#update-ownership-action').addEventListener('click', () => updateOwnership.act());
   qs('#update-ownership-start').addEventListener('click', () => updateOwnership.start());
@@ -5647,18 +5658,7 @@
       updateReplyAttachmentController.setBusy(false);
     }
   });
-  qs('#mark-update-read-next').addEventListener('click', async () => {
+  qs('#mark-update-read-next').addEventListener('click', markUpdateRead);
-    qs('#mark-update-read-next').disabled = true;
-    try {
-      const result = await notificationReader.markReadAndNext(lastMyWork);
-      if (result) {
-        notificationUndo.offer(result.item, result.items);
-        if (updateTriage.active()) updateTriage.acceptCompleted();
-      }
-    } finally {
-      qs('#mark-update-read-next').disabled = false;
-    }
-  });
   qs('#acknowledge-update-next').addEventListener('click', async () => {
     const button = qs('#acknowledge-update-next');
     button.disabled = true;

frontend/index.html

@@ -1091,6 +1091,7 @@
 <script src="static/update-read-position.js"></script>
 <script src="static/update-triage-launcher.js"></script>
 <script src="static/update-triage-gesture.js"></script>
+<script src="static/update-decision-transaction.js"></script>
 <script src="static/agenda-session-launcher.js"></script>
 <script src="static/mobile-launch.js"></script>
 <script src="static/mobile-app-shortcuts.js"></script>

frontend/service-worker.js

@@ -80,6 +80,7 @@ const SHELL = [
   BASE + 'static/update-read-position.js',
   BASE + 'static/update-triage-launcher.js',
   BASE + 'static/update-triage-gesture.js',
+  BASE + 'static/update-decision-transaction.js',
   BASE + 'static/agenda-session-launcher.js',
   BASE + 'static/mobile-launch.js',
   BASE + 'static/mobile-app-shortcuts.js',

frontend/update-decision-transaction.js

@@ -0,0 +1,38 @@
+(function(r){
+  function transaction({controls=[],status}) {
+    let busy=false, disabled=[];
+    async function run(message, action, {origin,failure}={}) {
+      if (busy) return false;
+      busy=true;
+      disabled=controls.map(control=>control.disabled);
+      controls.forEach(control=>{ control.disabled=true; });
+      status.textContent=message;
+      let completed=false;
+      try { completed=Boolean(await action()); }
+      catch (error) { failure=failure || error.message || 'Could not complete decision. Retry.'; }
+      if (!completed && failure) status.textContent=failure;
+      busy=false;
+      controls.forEach((control,index)=>{ control.disabled=disabled[index]; });
+      if (!completed) origin?.focus?.();
+      return completed;
+    }
+    return {run,busy:()=>busy};
+  }
+  function actions(o) {
+    return {
+      keepUnread:()=>o.transaction.run('Keeping unread…',()=>{
+        if(o.triage.active()) o.triage.keepUnreadAndNext(); else o.close();
+        return true;
+      },{origin:o.keepControl,failure:'Could not keep this update unread. Retry.'}),
+      markRead:()=>o.transaction.run('Marking read…',async()=>{
+        const result=await o.reader.markReadAndNext(o.items());
+        if(!result) return false;
+        o.undo.offer(result.item,result.items);
+        if(o.triage.active()) o.triage.acceptCompleted();
+        return true;
+      },{origin:o.readControl,failure:'Could not mark update read. Retry.'}),
+    };
+  }
+  if(typeof module==='object'&&module.exports){module.exports=transaction;module.exports.createActions=actions;}
+  else {r.createUpdateDecisionTransaction=transaction;r.createUpdateDecisionActions=actions;}
+})(typeof self!=='undefined'?self:this);

tests/test_frontend_bundle.py

@@ -68,8 +68,8 @@ def test_product_workflows_are_stable_lazy_feature_chunks(tmp_path):
     assert b"function attachSecurityCenter" in security_center.runtime_bytes
     assert b"gitea_time_logged" not in first.runtime_bytes
     assert b"gitea_time_logged" in security_center.runtime_bytes
-    # One-ahead Updates prefetch stays in the core reader so transitions can reuse its in-flight request.
+    # Core mobile workflows stay below 98 KiB gzip, including transaction-safe Update decisions.
-    assert len(first.runtime_gzip_bytes) <= 97 * 1024
+    assert len(first.runtime_gzip_bytes) <= 98 * 1024
     assert f'name="stackchain-feature-issue-capture" content="{capture.runtime_name}"' in first.dashboard_html
     assert f'name="stackchain-feature-pull-workflow" content="{pull_workflow.runtime_name}"' in first.dashboard_html
     assert f"BASE + '{capture.runtime_name}'" in first.service_worker_source

tests/test_my_work.py

@@ -6339,7 +6339,7 @@ async def test_mobile_update_reader_is_in_app_safe_area_aware_and_actionable():
     assert '.update-sheet-actions button, .update-sheet-actions a { min-height:44px;' in html
     assert "createNotificationReader" in html
     assert "api/v1/notifications/" in html
-    assert "notificationReader.markReadAndNext(lastMyWork)" in html
+    assert "reader:notificationReader, items:()=>lastMyWork" in html
 
 
 def test_notification_replier_preserves_failed_draft_and_clears_only_after_success():

tests/test_service_worker.py

@@ -850,6 +850,7 @@ def test_install_precaches_complete_subpath_scoped_app_shell():
         "/dashboard/static/update-read-position.js",
         "/dashboard/static/update-triage-launcher.js",
         "/dashboard/static/update-triage-gesture.js",
+        "/dashboard/static/update-decision-transaction.js",
         "/dashboard/static/agenda-session-launcher.js",
         "/dashboard/static/mobile-launch.js",
         "/dashboard/static/mobile-app-shortcuts.js",

tests/test_update_triage_gesture.py

@@ -8,6 +8,7 @@ from tests.dashboard_bundle import dashboard
 
 
 GESTURE = Path(__file__).resolve().parents[1] / "frontend" / "update-triage-gesture.js"
+TRANSACTION = Path(__file__).resolve().parents[1] / "frontend" / "update-decision-transaction.js"
 
 
 def run_gesture(script):
@@ -17,6 +18,71 @@ def run_gesture(script):
     return json.loads(result.stdout)
 
 
+def run_transaction(script):
+    source = f"const createTransaction = require({json.dumps(str(TRANSACTION))});\n" + script
+    result = subprocess.run(["node", "-e", source], capture_output=True, text=True)
+    assert result.returncode == 0, result.stderr
+    return json.loads(result.stdout)
+
+
+def test_update_decision_transaction_locks_every_action_until_success():
+    result = run_transaction(r"""
+const controls = ['keep', 'read', 'reply', 'more'].map(name => ({name, disabled:false, focus(){}}));
+const status = {textContent:'Ready'};
+let resolveAction;
+let calls = 0;
+const transaction = createTransaction({controls, status});
+const pending = transaction.run('Marking read…', () => new Promise(resolve => {
+  calls += 1;
+  resolveAction = resolve;
+}));
+const locked = controls.every(control => control.disabled) && transaction.busy();
+const rejected = transaction.run('Keeping unread…', () => { calls += 1; });
+resolveAction(true);
+Promise.all([pending, rejected]).then(([completed, second]) => process.stdout.write(JSON.stringify({
+  locked, pendingStatus: 'Marking read…', calls, completed, second,
+  unlocked: controls.every(control => !control.disabled) && !transaction.busy(),
+  finalStatus: status.textContent,
+})));
+""")
+
+    assert result == {
+        "locked": True,
+        "pendingStatus": "Marking read…",
+        "calls": 1,
+        "completed": True,
+        "second": False,
+        "unlocked": True,
+        "finalStatus": "Marking read…",
+    }
+
+
+def test_update_decision_transaction_restores_focus_and_announces_failure():
+    result = run_transaction(r"""
+let focused = 0;
+let focusedWhileEnabled = false;
+const origin = {disabled:false, focus(){ focused += 1; focusedWhileEnabled = !this.disabled; }};
+const other = {disabled:false, focus(){}};
+const status = {textContent:''};
+const transaction = createTransaction({controls:[origin, other], status});
+transaction.run('Marking read…', async () => false, {
+  origin,
+  failure:'Could not mark update read. Retry.',
+}).then(completed => process.stdout.write(JSON.stringify({
+  completed, focused, focusedWhileEnabled, status:status.textContent,
+  unlocked:!origin.disabled && !other.disabled && !transaction.busy(),
+})));
+""")
+
+    assert result == {
+        "completed": False,
+        "focused": 1,
+        "focusedWhileEnabled": True,
+        "status": "Could not mark update read. Retry.",
+        "unlocked": True,
+    }
+
+
 def test_committed_horizontal_swipes_decide_once_and_lock_while_pending():
     result = run_gesture(r"""
 class Surface {
@@ -102,9 +168,11 @@ async def test_dashboard_wires_accessible_phone_only_update_swipe_triage():
     html = await dashboard()
 
     assert '<script src="static/update-triage-gesture.js"></script>' in html
+    assert '<script src="static/update-decision-transaction.js"></script>' in html
     assert 'id="update-gesture-status"' in html
     assert "createUpdateTriageGesture({" in html
-    assert "qs('#keep-update-unread').click()" in html
+    assert "createUpdateDecisionActions({" in html
-    assert "qs('#mark-update-read-next').click()" in html
+    assert "keepUnread: keepUpdateUnread" in html
+    assert "markRead: markUpdateRead" in html
     assert "data-triage-gesture" in html
     assert "prefers-reduced-motion: reduce" in html

tests/test_update_triage_session.py

@@ -158,7 +158,7 @@ async def test_dashboard_wires_resumable_updates_triage_mobile_flow():
     assert 'id="keep-update-unread" type="button" aria-label="Keep unread and open next update">Keep unread</button>' in html
     assert "openUpdates: openUpdateTriage" in html
     assert "updateTriage.acceptCompleted()" in html
-    assert "updateTriage.keepUnreadAndNext()" in html
+    assert "triage:updateTriage" in html
     assert "updateTriage.reconcile()" in html
     assert 'id="mobile-update-outcome"' in html
     assert 'id="review-kept-updates"' in html