Contact email: don't strand a change saved as a send finishes; time reports exactly

Third review follow-ups:
- A sender that found nothing waiting checks again after letting go of the
  send lock, so a change saved in that moment is sent, not left for a retrier.
- A report is compared with a removal using its full-precision start time, so
  a report sent after the address was removed is logged as sent.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

tests/test_contact.py

@@ -169,9 +169,8 @@ class Contact(Base):
         with mock.patch.object(tm, "post", side_effect=remove_meanwhile):
             fr.send({**REPORT, "contact": "me@example.com", "contactFollowup": True})
         self.assertNotIn("me@example.com", tm.SENT.read_text())
-        fc._removed.clear()
-        fr.send({**REPORT, "contact": "other@example.com", "contactFollowup": True})
-        self.assertIn("other@example.com", tm.SENT.read_text())  # other reports are logged as sent
+        fr.send({**REPORT, "contact": "me@example.com", "contactFollowup": True})
+        self.assertIn("me@example.com", tm.SENT.read_text())  # sent again after removal: logged as sent
 
     def test_saving_during_a_slow_send_returns_at_once(self):
         busy = fc._send_lock
@@ -185,6 +184,28 @@ class Contact(Base):
         self.assertTrue(fc._send_pending())
         self.assertEqual(len(self.got), 1)
 
+    def test_a_change_saved_as_a_send_finishes_is_not_left_behind(self):
+        real = fc._send_lock
+
+        class Lock:  # a Save lands after the sender found nothing waiting, before it lets go
+            saved = False
+
+            def acquire(self, blocking=True):
+                return real.acquire(blocking)
+
+            def release(self):
+                if not Lock.saved:
+                    Lock.saved = True
+                    s = threading.Thread(target=fc.save, args=({"email": "me@example.com", "updates": True},))
+                    s.start()
+                    s.join(5)
+                real.release()
+
+        with mock.patch.object(fc, "_send_lock", Lock()):
+            self.assertTrue(fc._send_pending())
+        self.assertEqual([e["properties"]["email"] for e in self.events()], ["me@example.com"])
+        self.assertFalse(fc.state()["waiting"])
+
     # ---- the one-time prompt
 
     def test_the_prompt_waits_for_a_working_setup_then_stays_dismissed(self):

ui/frame_contact.py

@@ -18,7 +18,6 @@ A change that can't be sent (offline) waits in the state file and is retried in
 background, so a withdrawal is never lost. The page's one-time prompt is remembered here
 too: once it has been shown or dismissed it never comes back.
 """
-import calendar
 import json
 import os
 import re
@@ -100,7 +99,7 @@ def _send_pending(block=True):
             with _lock:
                 event = load()['pending']
             if event is None:
-                return True
+                break
             try:
                 frame_telemetry.post([event], timeout=30)
             except frame_telemetry.SendError:
@@ -108,6 +107,10 @@ def _send_pending(block=True):
             _sent(event)
     finally:
         _send_lock.release()
+    # A change saved just as this finished found the lock still held and left it to us.
+    with _lock:
+        left = load()['pending'] is not None
+    return _send_pending(block=False) if left else True
 
 
 def _sent(event):
@@ -139,18 +142,14 @@ def _forget_locally(email):
             frame_telemetry._write_lines(frame_telemetry.SENT, rows)
 
 
-def redact_removed(event):
-    """Before logging a report sent while its address was being removed: take the address out.
-    Call with frame_telemetry._lock held, so a removal can't slip between this and the log."""
+def redact_removed(event, started):
+    """Before logging a report (started at time.time() `started`) whose address was removed
+    while it was being sent: take the address out. Call with frame_telemetry._lock held, so a
+    removal can't slip between this and the log."""
     p = event.get('properties') or {}
     removed_at = _removed.get(str(p.get('contact') or '').strip().lower())
-    if removed_at is not None:
-        try:
-            started = calendar.timegm(time.strptime(event['timestamp'], '%Y-%m-%dT%H:%M:%SZ'))
-        except (KeyError, ValueError):
-            started = 0
-        if started <= removed_at:
-            p['contact'] = '<removed>'
+    if removed_at is not None and started <= removed_at:
+        p['contact'] = '<removed>'
 
 
 def save(body):

ui/frame_report.py

@@ -123,13 +123,14 @@ def send(body):
     # Its own random id: a report can carry contact details, so it isn't linked to this copy's analytics.
     event = {'event': 'problem_report', 'distinct_id': str(uuid.uuid4()), 'uuid': str(uuid.uuid4()),
              'timestamp': time.strftime('%Y-%m-%dT%H:%M:%SZ', time.gmtime()), 'properties': props}
+    started = time.time()
     try:
         frame_telemetry.post([event], timeout=30)
     except frame_telemetry.SendError as e:
         raise ReportError(str(e))
     try:
         with frame_telemetry._lock:  # the lock a removal holds while wiping its address
-            frame_contact.redact_removed(event)
+            frame_contact.redact_removed(event, started)
             frame_telemetry.record_sent([event])
     except OSError:
         pass  # it was sent; failing to log it here mustn't make the person send it again