fix(comfort): harden timer recovery and notification UX after review

docs/family-comfort.md

@@ -21,7 +21,8 @@ account restriction or parental lock. The wearer can return to the game.
 **Documented implementation:** the timer is a single, opt-in Python worker in
 the Frame user's account. Desktop and iPhone share its state. It keeps going
 when the companion disconnects, closes or is suspended. It exits after
-completion or cancellation (normally within five seconds); it is not a boot
+completion or cancellation (normally within five seconds). Cancellation waits
+for any in-flight SteamVR action to finish within its timeout; it is not a boot
 service. A Frame reboot invalidates the session. Suspend counts toward the
 limit, using Linux's boot-time clock. If a warning was delayed by suspend or a
 SteamVR failure, Home waits until at least a full minute after a successful
@@ -30,7 +31,8 @@ shown in the companion. A stale worker is reported as unverified enforcement.
 
 ## Alerts and breaks
 
-During a session:
+During a session, battery, overheating and check-in alerts go to connected
+companions. Break reminders and session warnings also appear on the headset.
 
 - **Low battery:** 15% or below while discharging. One alert until charging or
   recovery to 20%, so values around 15% do not produce repeated notifications.
@@ -109,3 +111,12 @@ success. Successful macOS/Windows/Linux notification display remains unverified.
 one low-battery event during a short session; the test then cancelled the
 session. Overheating alerts use fake sensor samples in tests: the shared
 headset was not deliberately overheated.
+
+**Verified 2026-09-29 on the same Frame:** a fresh one-minute session opened
+Home more than 60 seconds after the successful warning. The test restored the
+previous page and dashboard visibility. Local regression coverage now includes
+slow notification delivery, a total Home-action timeout, failed worker startup,
+unreadable saved state, malformed activity samples and notification UX: 173
+Python tests passed. Desktop and 390-pixel layouts were checked again; system
+notification-denial guidance stayed visible across polls. Initial event history
+did not replay notifications, and only the latest new event was announced.

tests/test_comfort.py

@@ -23,7 +23,7 @@ class SessionTests(unittest.TestCase):
         self.warn, self.home = Mock(), Mock()
 
     def step(self, now, **sample):
-        comfort.tick(self.s, now, sample, self.warn, self.home)
+        comfort.tick(self.s, now, sample, self.warn, self.home, read_clock=lambda: now)
 
     def test_warning_then_home_never_closes_a_game(self):
         self.step(119)
@@ -46,6 +46,16 @@ class SessionTests(unittest.TestCase):
         self.step(460)
         self.home.assert_called_once()
 
+    def test_slow_warning_still_leaves_a_full_minute(self):
+        comfort.tick(self.s, 120, {}, self.warn, self.home, read_clock=lambda: 140)
+        self.assertEqual(self.s['warned'], 140)
+        self.step(180)
+        self.home.assert_not_called()
+        self.step(199)
+        self.home.assert_not_called()
+        self.step(200)
+        self.home.assert_called_once()
+
     def test_failed_warning_never_stops_session(self):
         self.warn.side_effect = RuntimeError('offline')
         with self.assertRaises(RuntimeError):
@@ -144,6 +154,56 @@ class SessionTests(unittest.TestCase):
             spawn.assert_called_once()
             self.assertEqual((Path(tmp) / 'session.json').stat().st_mode & 0o777, 0o600)
 
+    @unittest.skipUnless(os.name == "posix", "on-headset state uses POSIX flock")
+    def test_cancel_clears_stale_worker_error(self):
+        with tempfile.TemporaryDirectory() as tmp, patch.object(comfort, 'ROOT', Path(tmp)), \
+             patch.object(comfort, 'boot', return_value='boot-one'), \
+             patch.object(comfort, 'clock', return_value=200):
+            with comfort.locked():
+                comfort.save(self.s)
+            self.assertIn('not responding', comfort.command({'action': 'status'})['error'])
+            cancelled = comfort.command({'action': 'cancel'})
+            self.assertFalse(cancelled['active'])
+            self.assertIsNone(cancelled['error'])
+            self.assertIsNone(comfort.command({'action': 'status'})['error'])
+
+    @unittest.skipUnless(os.name == "posix", "on-headset state uses POSIX flock")
+    def test_failed_spawn_leaves_session_inactive_and_retryable(self):
+        with tempfile.TemporaryDirectory() as tmp, patch.object(comfort, 'ROOT', Path(tmp)), \
+             patch.object(comfort, 'boot', return_value='boot-one'), \
+             patch.object(comfort, 'clock', return_value=0), patch.object(comfort.subprocess, 'Popen') as spawn:
+            spawn.side_effect = OSError('process limit')
+            with self.assertRaises(OSError):
+                comfort.command(OPTIONS)
+            failed = comfort.command({'action': 'status'})
+            self.assertFalse(failed['active'])
+            self.assertIn('Could not start', failed['error'])
+            spawn.side_effect = None
+            self.assertTrue(comfort.command(OPTIONS)['active'])
+
+    @unittest.skipUnless(os.name == "posix", "on-headset state uses POSIX flock")
+    def test_unreadable_state_is_preserved_and_can_be_replaced(self):
+        for contents in (b'{broken', b'\xff', b'null', b'[]', b'42', b'"x"'):
+            with self.subTest(contents=contents):
+                with tempfile.TemporaryDirectory() as tmp, patch.object(comfort, 'ROOT', Path(tmp)), \
+                     patch.object(comfort, 'boot', return_value='boot-one'), \
+                     patch.object(comfort, 'clock', return_value=0), patch.object(comfort.subprocess, 'Popen'):
+                    (Path(tmp) / 'session.json').write_bytes(contents)
+                    failed = comfort.command({'action': 'status'})
+                    self.assertFalse(failed['active'])
+                    self.assertIn('unreadable', failed['error'])
+                    backups = list(Path(tmp).glob('session-unreadable-*.json'))
+                    self.assertEqual(len(backups), 1)
+                    self.assertEqual(backups[0].read_bytes(), contents)
+                    self.assertTrue(comfort.command(OPTIONS)['active'])
+
+    def test_home_has_total_process_deadline_and_propagates_timeout(self):
+        with patch.object(comfort.subprocess, 'run', side_effect=subprocess.TimeoutExpired('home', 15)) as run:
+            with self.assertRaises(subprocess.TimeoutExpired):
+                comfort.home()
+            self.assertEqual(run.call_args.kwargs['timeout'], 15)
+            self.assertEqual(run.call_args.args[0][-1], '--home')
+
     def test_native_warning_reports_failures_and_quotes_as_one_argument(self):
         with patch.object(comfort.subprocess, 'run') as run:
             run.return_value = subprocess.CompletedProcess([], 0, 'Notification succeeded', '')
@@ -190,5 +250,8 @@ class SensorTests(unittest.TestCase):
             self.assertIsNone(status.thermal_alerts())
         with patch.object(status, 'run', return_value='unavailable'):
             self.assertIsNone(status.activity_level())
+        for malformed in ('{}', '[null, 42, "bad"]'):
+            with patch.object(status, 'run', return_value=malformed):
+                self.assertIsNone(status.activity_level())
         with patch.object(status, 'run', return_value='[{"operation":"status","activity_level":3}]'):
             self.assertEqual(status.activity_level(), 3)

tests/test_comfort_ui.py

@@ -0,0 +1,60 @@
+"""Run the actual shared page's comfort renderer against a minimal DOM/bridge."""
+import pathlib
+import shutil
+import subprocess
+import unittest
+
+ROOT = pathlib.Path(__file__).resolve().parents[1]
+
+
+@unittest.skipUnless(shutil.which('node'), 'Node exercises the shared page JS')
+class ComfortUI(unittest.TestCase):
+    def test_notification_failure_survives_poll_until_success(self):
+        page = (ROOT / 'ui/index.html').read_text()
+        code = page[page.index('let comfortBusy ='):page.index('async function pollComfort()')]
+        setup = r'''
+const assert = require('node:assert/strict');
+const elements = new Map();
+const $ = id => {
+  if (!elements.has(id)) elements.set(id, {textContent:'', hidden:true, disabled:false, type: 'number'});
+  return elements.get(id);
+};
+let denied = 0;
+const window = {frameApp:{notify:async()=>{denied++;throw Error('permission denied');}}};
+const log = ()=>{}, toast = ()=>{};
+'''
+        checks = r'''
+(async()=>{
+  const active = {id:'session-one',active:true,time:100,remaining:120,
+    options:{minutes:2,breakMinutes:1,stillMinutes:1,batteryAlert:true,heatAlert:true},
+    events:[{id:'event-one',kind:'battery',time:99,message:'Low battery'}]};
+  renderComfort(active); // initial history must not replay even a fresh event
+  assert.equal(denied,0);
+  assert.equal($('comfortAnnouncement').textContent,'');
+  active.events.push({id:'event-two',kind:'break',time:100,message:'Take a break'});
+  renderComfort(active);
+  await new Promise(resolve=>setImmediate(resolve));
+  assert.equal(denied,1);
+  assert.equal($('comfortAnnouncement').textContent,'Take a break');
+  assert.equal($('comfortNotificationStatus').hidden,false);
+  assert.match($('comfortNotificationStatus').textContent,/notification settings/);
+  renderComfort({...active,time:105}); // the next normal poll must not erase failure
+  assert.equal($('comfortNotificationStatus').hidden,false);
+  assert.equal($('sessionStart').disabled,true);
+  assert.equal($('sessionMinutes').disabled,true);
+  assert.equal($('sessionCancel').disabled,false);
+  let requests=0;
+  window.frameApp.notify=async()=>{requests++;};
+  renderComfort({...active,time:106});
+  assert.equal($('comfortAnnouncement').textContent,'Take a break');
+  assert.equal(requests,0); // polling does not replay an already-seen event
+  await localNotification('test',true);
+  assert.equal($('comfortNotificationStatus').hidden,true);
+  renderComfort({...active,active:false});
+  assert.equal($('sessionStart').disabled,false);
+  assert.equal($('sessionMinutes').disabled,false);
+  assert.equal($('sessionCancel').disabled,true);
+})().catch(e=>{console.error(e);process.exitCode=1;});
+'''
+        result = subprocess.run(['node', '-e', setup + code + checks], capture_output=True, text=True)
+        self.assertEqual(result.returncode, 0, result.stderr)

ui/frame_comfort.py

@@ -71,6 +71,15 @@ def notify(message):
 
 
 def home():
+    # A total process deadline also bounds a CDP peer that keeps sending events
+    # without completing the request. Keep cancellation ordered after this action.
+    r = subprocess.run([sys.executable, str(Path(__file__).resolve()), '--home'],
+                       capture_output=True, text=True, timeout=15)
+    if r.returncode:
+        raise RuntimeError('Steam Home failed: ' + (r.stdout or r.stderr)[-300:])
+
+
+def open_home():
     page = Page()
     try:
         result = page.eval(HOME_JS)
@@ -80,7 +89,7 @@ def home():
         page.sock.close()
 
 
-def tick(s, now, sample, warn=notify, go_home=home):
+def tick(s, now, sample, warn=notify, go_home=home, read_clock=clock):
     """One deterministic step; injected actions/samples also exercise a fake Frame."""
     if not s.get('active'):
         return
@@ -107,7 +116,7 @@ def tick(s, now, sample, warn=notify, go_home=home):
     # Missing samples never count as time worn. No catch-up burst after a disconnect.
     if now >= s['deadline'] - 60 and s['warned'] is None:
         warn('One minute left. Save your progress; Steam Home will open.')
-        s['warned'] = now
+        s['warned'] = max(now, read_clock())
         event(s, 'warning', 'One minute left. Save your progress; Steam Home will open.')
     if s['warned'] is not None and now >= max(s['deadline'], s['warned'] + 60):
         go_home()
@@ -126,7 +135,7 @@ def tick(s, now, sample, warn=notify, go_home=home):
     for kind, enabled, value, message in (
         ('battery', o['batteryAlert'], low if b else None, 'Frame battery is low (15% or less).'),
         ('heat', o['heatAlert'], bool(hot) if hot is not None else None,
-         'Frame reports a hot/critical thermal trip or battery overheat. Take a break.')):
+         'Frame reports a hot/critical thermal trip or battery overheat. Ask the wearer to take a break.')):
         if enabled and value and kind not in s['latched']:
             event(s, kind, message)
             s['latched'].append(kind)
@@ -147,9 +156,17 @@ def locked(name='state.lock', nonblocking=False):
 
 def read_state():
     try:
-        return json.loads((ROOT / 'session.json').read_text())
+        state = json.loads((ROOT / 'session.json').read_text())
+        if not isinstance(state, dict):
+            raise ValueError('Saved session must be an object')
+        return state
     except FileNotFoundError:
         return {'active': False, 'events': []}
+    except (ValueError, UnicodeDecodeError):
+        # Preserve the unreadable state for diagnosis, then allow a new session.
+        (ROOT / 'session.json').replace(ROOT / ('session-unreadable-' + uuid.uuid4().hex + '.json'))
+        return {'active': False, 'events': [],
+                'error': 'Saved session was unreadable. Start a new session.'}
 
 
 def save(s):
@@ -213,13 +230,21 @@ def command(body):
             event(s, 'started', 'Session started. Steam Home opens at the limit; games are not closed.')
         elif body['action'] == 'cancel':
             s['active'] = False
+            s['error'] = None
             if s.get('id'):
                 event(s, 'cancelled', 'Session timer and monitoring cancelled.')
         save(s)
         if body['action'] == 'start':
-            subprocess.Popen([sys.executable, str(Path(__file__).resolve()), '--watch'],
-                             stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL,
-                             start_new_session=True, close_fds=True)
+            try:
+                subprocess.Popen([sys.executable, str(Path(__file__).resolve()), '--watch'],
+                                 stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL,
+                                 start_new_session=True, close_fds=True)
+            except OSError as e:
+                s['active'] = False
+                s['error'] = 'Could not start session worker: ' + str(e)
+                event(s, 'error', s['error'])
+                save(s)
+                raise
         return current(s, clock())
 
 
@@ -228,7 +253,11 @@ if __name__ == '__main__':
         watch()
     else:
         try:
-            print(json.dumps(command(json.loads(sys.argv[1]))))
+            if sys.argv[1:] == ['--home']:
+                open_home()
+                print(json.dumps({'home': True}))
+            else:
+                print(json.dumps(command(json.loads(sys.argv[1]))))
         except Exception as e:
             print(json.dumps({'error': str(e)}))
             sys.exit(1)

ui/frame_status.py

@@ -175,7 +175,10 @@ def thermal_alerts():
 def activity_level():
     try:
         rows = json.loads(run("/opt/steamvr/bin/linuxarm64/vrcmd", "--stats"))
-        return next((r.get("activity_level") for r in rows if r.get("operation") == "status"), None)
+        if not isinstance(rows, list):
+            return None
+        return next((r.get("activity_level") for r in rows
+                     if isinstance(r, dict) and r.get("operation") == "status"), None)
     except (ValueError, TypeError):
         return None
 

ui/index.html

@@ -101,6 +101,7 @@
   .shelf-head .count { color: var(--muted); font-size: 13px; }
   .shelf-head .spacer { flex: 1; }
   .sub { color: var(--muted); font-size: 12.5px; }
+  .sr-only { position: absolute; width: 1px; height: 1px; overflow: hidden; clip-path: inset(50%); white-space: nowrap; }
   .hint { color: var(--muted); font-size: 12.5px; margin-top: 11px; line-height: 1.5; }
 
   /* ---- buttons ---- */
@@ -483,20 +484,22 @@
         <label>Check in after <input id="stillMinutes" type="number" min="0" max="240" value="30" required style="width:75px"> active minutes</label>
       </div>
       <div class="row" style="flex-wrap:wrap;margin:14px 0;gap:14px">
-        <label><input id="batteryAlert" type="checkbox" checked> Low battery</label>
-        <label><input id="heatAlert" type="checkbox" checked> Overheating</label>
+        <label><input id="batteryAlert" type="checkbox" checked> Alert on low battery</label>
+        <label><input id="heatAlert" type="checkbox" checked> Alert on overheating</label>
         <button class="action small" id="sessionStart" type="submit">Start session</button>
         <button class="small" id="sessionCancel" type="button" disabled>Cancel session</button>
         <button class="small" id="testNotification" type="button">Enable / test notifications</button>
       </div>
     </form>
     <p id="comfortStatus" role="status">Checking session…</p>
+    <p id="comfortNotificationStatus" class="hint" role="status" hidden></p>
     <p class="hint">A one-minute warning, then Steam Home. Games stay running: save and pause first. Keep this app open and connected for notifications.</p>
     <details class="hint"><summary>How sessions and alerts work</summary>
       <p>This is a reminder, not a parental lock. The timer continues on the Frame if you disconnect; a headset restart cancels it. Cancel before changing settings. Set break/check-in to 0 to turn them off.</p>
-      <p>Alerts run during a session. iOS may suspend phone notifications in the background. Breaks and check-ins count SteamVR activity, not confirmed wear time.</p>
+      <p>Battery, heat and check-in alerts appear on connected companions during a session. Headset warnings and break reminders continue without a companion. iOS may suspend phone notifications in the background. Breaks and check-ins count SteamVR activity, not confirmed wear time.</p>
     </details>
-    <div id="comfortEvents" class="hint" aria-live="polite"></div>
+    <div id="comfortEvents" class="hint"></div>
+    <span id="comfortAnnouncement" class="sr-only" aria-live="polite"></span>
   </section>
 
   <section id="shots">
@@ -926,15 +929,27 @@ function refresh() {
 }
 
 // The Frame owns the clock. Poll independently of status and the selected tab.
-let comfortBusy = false, comfortSeen = new Set(), comfortSession = null;
+let comfortBusy = false, comfortSeen = new Set(), comfortSession = null, comfortHydrated = false;
 async function localNotification(message, request = false) {
-  if (window.frameApp?.notify) return window.frameApp.notify(message, request);
-  if (!("Notification" in window)) throw new Error("This browser has no notifications; use the Frame Control app.");
-  const permission = request ? await Notification.requestPermission() : Notification.permission;
-  if (permission !== "granted") throw new Error("Notifications are off. Enable them in system settings.");
-  new Notification("Frame Control", {body: message});
+  try {
+    if (window.frameApp?.notify) await window.frameApp.notify(message, request);
+    else {
+      if (!("Notification" in window)) throw new Error("This browser has no notifications; use the Frame Control app.");
+      const permission = request ? await Notification.requestPermission() : Notification.permission;
+      if (permission !== "granted") throw new Error("Notifications are off. Enable them in system settings.");
+      new Notification("Frame Control", {body: message});
+    }
+    $("comfortNotificationStatus").hidden = true;
+  } catch (e) {
+    $("comfortNotificationStatus").hidden = false;
+    $("comfortNotificationStatus").textContent = "Notifications aren't available on this device. Check system notification settings, then use Enable / test notifications. Headset reminders continue during an active session.";
+    throw e;
+  }
 }
+
 function renderComfort(s) {
+  const firstSnapshot = !comfortHydrated;
+  comfortHydrated = true;
   $("sessionStart").disabled = !!s.active;
   for (const id of ["sessionMinutes", "breakMinutes", "stillMinutes", "batteryAlert", "heatAlert"])
     $(id).disabled = !!s.active;
@@ -944,30 +959,41 @@ function renderComfort(s) {
     comfortSeen.clear();
     if (s.options) for (const [key, value] of Object.entries(s.options)) {
       const el = $(key === "minutes" ? "sessionMinutes" : key);
+      if (!el) continue;
       if (el.type === "checkbox") el.checked = value; else el.value = value;
     }
   }
-  $("comfortStatus").textContent = s.error || (s.active
+  let statusText = s.error || (s.active
     ? `${Math.ceil(s.remaining / 60)} min until Steam Home · ${s.activity == null ? "activity unknown" : s.activity === 3 ? "headset in standby" : "monitoring"}`
     : "No session running.");
   if (s.active && s.unavailable?.length)
-    $("comfortStatus").textContent += " · No readings: " + s.unavailable.join(", ");
+    statusText += " · No readings: " + s.unavailable.join(", ");
+  if ($("comfortStatus").textContent !== statusText) $("comfortStatus").textContent = statusText;
+  let latest = null;
   for (const e of s.events || []) {
     if (comfortSeen.has(e.id)) continue;
     comfortSeen.add(e.id);
     // Historical events remain visible but never produce a burst on reconnect.
-    if (s.time - e.time >= 0 && s.time - e.time < 30 && !["started", "cancelled"].includes(e.kind)) {
+    if (!firstSnapshot && s.time - e.time >= 0 && s.time - e.time < 30 && !["started", "cancelled"].includes(e.kind)) {
+      latest = e.message;
       log(e.message); toast(e.message, e.kind === "error");
-      localNotification(e.message).catch(err => { $("comfortStatus").textContent += " · " + err.message; });
+      localNotification(e.message).catch(() => {});  // the notification status keeps the failure visible
     }
   }
-  $("comfortEvents").textContent = (s.events || []).slice(-3).map(e => e.message).join(" · ");
+  // Keep only the server's bounded history; a new page seeds it without replay.
+  comfortSeen = new Set((s.events || []).map(e => e.id));
+  if (latest !== null) $("comfortAnnouncement").textContent = latest;
+  const history = (s.events || []).slice(-3).map(e => e.message).join(" · ");
+  if ($("comfortEvents").textContent !== history) $("comfortEvents").textContent = history;
 }
 async function pollComfort() {
   if (!comfortBusy) {
     comfortBusy = true;
     try { renderComfort(await api("/api/comfort", {action: "status"})); }
-    catch (e) { $("comfortStatus").textContent = "Session status unavailable: " + e.message; }
+    catch (e) {
+      const message = "Session status unavailable: " + e.message;
+      if ($("comfortStatus").textContent !== message) $("comfortStatus").textContent = message;
+    }
     finally { comfortBusy = false; }
   }
   setTimeout(pollComfort, 5000);