Merge pull request 'Keep mobile delivery recovery single-flight until retry settles' (#1017) from timmy/1016-single-flight-delivery-recovery into main

frontend/dashboard.js

@@ -90,10 +90,17 @@
     qs('#my-work').scrollIntoView({block:'start'});
     qs('#my-work').focus();
   }
+  async function recoverMobileDelivery(item) {
+    if (!item?.outbox_id || !activeFlushLogin) throw new Error('Sign in again to retry.');
+    const authored = item.kind === 'authored-outbox';
+    const result = await (authored ? authoredOutbox : issueOutbox).retry(item.outbox_id, activeFlushLogin);
+    (authored ? applyAuthoredOutboxResult : applyOutboxResult)(result);
+  }
   let mobileQueueCounts = {};
   const mobileDeliveryRecovery = createMobileDeliveryRecovery({
     getItems: () => draftInbox.partition(lastDrafts).deliveries,
     getIndex: item => lastDrafts.indexOf(item),
+    activate: recoverMobileDelivery,
     beforeOpen: () => selectMobileQueue('draft'),
     onComplete: () => {
       if (!mobileStartDay.completePhase('delivery')) showMobileQueueCompletion('Delivery', true);

frontend/mobile-delivery-recovery.js

@@ -49,6 +49,7 @@
   };
   let currentId = '';
   let active = false;
+  let inFlight = null;
 
   function currentItem(items) {
     return items.find(item => identity(item) === currentId) || items[0];
@@ -107,36 +108,41 @@
     return 'opened';
   }
 
-  async function defaultActivate(item) {
+  async function attend(item) {
     if (typeof document === 'undefined') return false;
     const index = options.getIndex ? options.getIndex(item) : -1;
-    const selector = item.status === 'authorization' ? '.draft-authorize' :
-      item.checklist_conflict ? '.draft-review-checklist' : item.quarantined ? '.draft-copy' :
-      item.status === 'attention' ? '.draft-resume' : '.draft-send';
+    const selector = item.quarantined ? '.draft-copy' :
+      item.checklist_conflict ? '.draft-review-checklist' : '.draft-resume';
     const action = document.querySelector('#my-work-list ' + selector + '[data-draft-index="' + index + '"]');
-    if (!action) {
-      if (elements.status) elements.status.textContent = 'This delivery changed. Reopen recovery to load its current action.';
-      return false;
-    }
+    if (!action) throw new Error('This delivery changed. Reopen recovery to load its current action.');
     action.click();
-    if (selector === '.draft-resume' || selector === '.draft-copy') elements.dialog.close();
-    await Promise.resolve();
     return true;
   }
 
-  async function activate() {
+  function activate() {
+    if (inFlight) return inFlight;
     const items = ordered();
     const item = currentItem(items);
-    const perform = options.activate || defaultActivate;
-    if (!item) return render();
+    const perform = state(item) === 'attention' ? attend : options.activate;
+    if (!item) return Promise.resolve(render());
     if (elements?.action) elements.action.disabled = true;
     if (elements?.status) elements.status.textContent = 'Working…';
-    try {
-      await perform(item, state(item));
-    } finally {
-      if (elements?.action) elements.action.disabled = false;
-    }
-    return render();
+    inFlight = (async () => {
+      let failure = null;
+      try {
+        await perform(item, state(item));
+      } catch (error) {
+        failure = String(error?.message || 'Delivery could not be completed.').trim().slice(0, 160);
+      } finally {
+        if (elements?.action) elements.action.disabled = false;
+      }
+      const result = render();
+      if (failure && elements?.status) {
+        elements.status.textContent = failure + ' Try again when the connection is ready.';
+      }
+      return result;
+    })().finally(() => { inFlight = null; });
+    return inFlight;
   }
 
   function start() {

tests/e2e/test_mobile_home_bootstrap_release.py

@@ -140,3 +140,63 @@ def test_release_artifact_bootstraps_mobile_home_and_returns_from_insights(
         fake.shutdown()
         fake.server_close()
         fake_thread.join(timeout=5)
+
+
+def test_release_artifact_keeps_mobile_delivery_recovery_single_flight(tmp_path: Path):
+    archives = sorted((ROOT / "dist").glob("stackchain-dashboard-*.tar.gz"))
+    assert len(archives) == 1, "browser job must download exactly one assembled release archive"
+
+    fake = FakeGiteaServer(("127.0.0.1", 0))
+    fake_thread = threading.Thread(target=fake.serve_forever, daemon=True)
+    fake_thread.start()
+    fake_url = f"http://127.0.0.1:{fake.server_port}"
+    browser_errors: list[str] = []
+
+    try:
+        with release_server(archives[0], tmp_path, fake_url) as origin, sync_playwright() as playwright:
+            browser = playwright.chromium.launch(args=["--ignore-certificate-errors"])
+            page = browser.new_page(viewport={"width": 390, "height": 844})
+            page.on("pageerror", lambda error: browser_errors.append(error.stack or str(error)))
+            page.goto(origin + "/", wait_until="networkidle")
+            page.locator('input[name="device_label"]').fill("Delivery recovery release phone")
+            page.locator('input[name="access_token"]').fill(ACCESS_TOKEN)
+            page.locator("#submit-sign-in").click()
+            page.wait_for_url(origin + "/", wait_until="networkidle")
+
+            page.evaluate(
+                """
+                () => {
+                  window.__deliveryAttempts = 0;
+                  window.__deliverySettled = new Promise(resolve => { window.__finishDelivery = resolve; });
+                  window.__releaseRecovery = createMobileDeliveryRecovery({
+                    getItems: () => [{outbox_id:'release-retry', delivery_state:'waiting', title:'Post release note'}],
+                    activate: async () => {
+                      window.__deliveryAttempts += 1;
+                      await window.__deliverySettled;
+                    },
+                  });
+                  window.__releaseRecovery.open();
+                  window.__firstRecovery = window.__releaseRecovery.activate();
+                  window.__secondRecovery = window.__releaseRecovery.activate();
+                }
+                """
+            )
+            action = page.locator("#mobile-delivery-recovery-action")
+            expect(page.locator("#mobile-delivery-recovery")).to_be_visible()
+            expect(action).to_be_disabled()
+            expect(page.locator("#mobile-delivery-recovery-status")).to_have_text("Working…")
+            assert page.evaluate("window.__deliveryAttempts") == 1
+            bounds = action.bounding_box()
+            assert bounds and bounds["height"] >= 44
+            assert page.evaluate("document.documentElement.scrollWidth <= window.innerWidth")
+
+            page.evaluate("window.__finishDelivery()")
+            page.evaluate("Promise.all([window.__firstRecovery, window.__secondRecovery])")
+            expect(action).to_be_enabled()
+            assert page.evaluate("window.__deliveryAttempts") == 1
+            assert browser_errors == []
+            browser.close()
+    finally:
+        fake.shutdown()
+        fake.server_close()
+        fake_thread.join(timeout=5)

tests/test_mobile_delivery_recovery.py

@@ -97,6 +97,83 @@ const createRecovery = require({json.dumps(str(RECOVERY))});
     }
 
 
+def test_recovery_keeps_one_attempt_locked_until_delivery_settles():
+    script = f"""
+const createRecovery = require({json.dumps(str(RECOVERY))});
+(async () => {{
+  let finish;
+  let attempts = 0;
+  const settled = new Promise(resolve => {{finish = resolve}});
+  const element = () => ({{textContent:'', disabled:false}});
+  const elements = {{
+    dialog: {{open:false, showModal(){{this.open=true}}, close(){{this.open=false}}}},
+    title:element(), destination:element(), reason:element(), progress:element(),
+    action:element(), status:element(),
+  }};
+  const recovery = createRecovery({{
+    getItems: () => [{{outbox_id:'retry', delivery_state:'waiting', title:'Post comment'}}],
+    activate: async () => {{attempts += 1; await settled; return true}},
+    elements,
+  }});
+  recovery.open();
+  const first = recovery.activate();
+  const second = recovery.activate();
+  await Promise.resolve();
+  const pending = {{attempts, disabled:elements.action.disabled, status:elements.status.textContent}};
+  finish();
+  await Promise.all([first, second]);
+  process.stdout.write(JSON.stringify({{
+    pending, attempts, disabled:elements.action.disabled,
+  }}));
+}})();
+"""
+
+    assert run_node(script) == {
+        "pending": {"attempts": 1, "disabled": True, "status": "Working…"},
+        "attempts": 1,
+        "disabled": False,
+    }
+
+
+def test_recovery_retains_failed_delivery_with_actionable_status():
+    script = f"""
+const createRecovery = require({json.dumps(str(RECOVERY))});
+(async () => {{
+  const element = () => ({{textContent:'', disabled:false}});
+  const elements = {{
+    dialog: {{open:false, showModal(){{this.open=true}}, close(){{this.open=false}}}},
+    title:element(), destination:element(), reason:element(), progress:element(),
+    action:element(), status:element(),
+  }};
+  const recovery = createRecovery({{
+    getItems: () => [{{outbox_id:'retry', delivery_state:'waiting', title:'Post comment'}}],
+    activate: async () => {{throw new Error('Network unavailable')}},
+    elements,
+  }});
+  recovery.open();
+  const result = await recovery.activate();
+  process.stdout.write(JSON.stringify({{
+    result, disabled:elements.action.disabled, status:elements.status.textContent,
+  }}));
+}})();
+"""
+
+    assert run_node(script) == {
+        "result": {
+            "count": 1,
+            "current": {
+                "id": "retry",
+                "state": "waiting",
+                "primary": "Retry now",
+                "position": 1,
+                "total": 1,
+            },
+        },
+        "disabled": False,
+        "status": "Network unavailable Try again when the connection is ready.",
+    }
+
+
 def test_recovery_renders_delivery_context_and_hands_off_when_cleared():
     script = f"""
 const createRecovery = require({json.dumps(str(RECOVERY))});
@@ -157,6 +234,9 @@ async def test_dashboard_packages_phone_safe_guided_delivery_recovery():
     assert 'id="mobile-delivery-recovery-action"' in html
     assert '<script src="static/mobile-delivery-recovery.js"></script>' in html
     assert "const mobileDeliveryRecovery = createMobileDeliveryRecovery({" in html
+    assert "activate: recoverMobileDelivery," in html
+    assert "await (authored ? authoredOutbox : issueOutbox).retry(item.outbox_id, activeFlushLogin)" in html
+    assert "(authored ? applyAuthoredOutboxResult : applyOutboxResult)(result)" in html
     assert "openDelivery: () => mobileDeliveryRecovery.open()" in html
     assert "mobileStartDay.completePhase('delivery')" in html
     assert "BASE + 'static/mobile-delivery-recovery.js'" in service_worker