fix(notifications): replace Linux progress updates in place (#198)

2026-08-15 13:55:51 -07:00 · 2026-08-14 22:20:33 -07:00
parent d963def1a9
commit 7d729cb60c
3 changed files with 392 additions and 13 deletions
@@ -0,0 +1,4 @@
+type: fixed
+area: notifications
+
+- Character dictionary progress notifications on Linux now update in place instead of flickering off and reappearing on every status change.
@@ -1,6 +1,6 @@
 import assert from 'node:assert/strict';
 import test from 'node:test';
-import { resolveDefaultNotificationIconPath } from './notification';
+import { createNotifySendReplacer, resolveDefaultNotificationIconPath } from './notification';

 test('default notification icon resolves packaged SubMiner asset when no per-notification icon is provided', () => {
  const path = resolveDefaultNotificationIconPath({
@@ -54,6 +54,206 @@ test('default notification icon avoids macOS tray template assets', () => {
  );
 });

+test('notify-send replacer reuses the daemon-assigned id for follow-up updates', () => {
+  const calls: string[][] = [];
+  const replacer = createNotifySendReplacer((args, callback) => {
+    calls.push(args);
+    callback(null, '42\n');
+  });
+
+  replacer('sync', { title: 'SubMiner', body: 'Generating 1/3' }, () => assert.fail('no fallback'));
+  replacer('sync', { title: 'SubMiner', body: 'Generating 2/3' }, () => assert.fail('no fallback'));
+
+  assert.equal(calls.length, 2);
+  assert.equal(calls[0]?.includes('--print-id'), true);
+  assert.equal(
+    calls[0]?.some((arg) => arg.startsWith('--replace-id=')),
+    false,
+  );
+  assert.equal(calls[1]?.includes('--replace-id=42'), true);
+});
+
+test('notify-send replacer collapses a mid-send burst to the latest update', () => {
+  const pending: Array<(error: Error | null, stdout: string) => void> = [];
+  const calls: string[][] = [];
+  const replacer = createNotifySendReplacer((args, callback) => {
+    calls.push(args);
+    pending.push(callback);
+  });
+
+  replacer('sync', { title: 'SubMiner', body: 'first' }, () => assert.fail('no fallback'));
+  replacer('sync', { title: 'SubMiner', body: 'second' }, () => assert.fail('no fallback'));
+  replacer('sync', { title: 'SubMiner', body: 'third' }, () => assert.fail('no fallback'));
+
+  // Only one send is in flight, so the id capture cannot race.
+  assert.equal(calls.length, 1);
+  assert.equal(calls[0]?.at(-1), 'first');
+
+  pending[0]?.(null, '7');
+
+  // 'second' is stale by the time the daemon frees up, so only 'third' is sent.
+  assert.equal(calls.length, 2);
+  assert.equal(calls[1]?.at(-1), 'third');
+  assert.equal(calls[1]?.includes('--replace-id=7'), true);
+
+  pending[1]?.(null, '7');
+  assert.equal(calls.length, 2);
+});
+
+test('notify-send replacer passes the resolved icon path through', () => {
+  const calls: string[][] = [];
+  const replacer = createNotifySendReplacer((args, callback) => {
+    calls.push(args);
+    callback(null, '3');
+  });
+
+  replacer(
+    'sync',
+    { title: 'SubMiner', body: 'Generating', iconPath: '/opt/SubMiner/assets/SubMiner-square.png' },
+    () => assert.fail('no fallback'),
+  );
+
+  assert.equal(calls[0]?.includes('--icon=/opt/SubMiner/assets/SubMiner-square.png'), true);
+});
+
+test('notify-send replacer tracks a separate daemon id per replaceId', () => {
+  const calls: string[][] = [];
+  let nextId = 10;
+  const replacer = createNotifySendReplacer((args, callback) => {
+    calls.push(args);
+    callback(null, String(nextId++));
+  });
+
+  replacer('dictionary', { title: 'SubMiner', body: 'dict 1' }, () => assert.fail('no fallback'));
+  replacer('startup', { title: 'SubMiner', body: 'startup 1' }, () => assert.fail('no fallback'));
+  replacer('dictionary', { title: 'SubMiner', body: 'dict 2' }, () => assert.fail('no fallback'));
+  replacer('startup', { title: 'SubMiner', body: 'startup 2' }, () => assert.fail('no fallback'));
+
+  assert.equal(calls.length, 4);
+  assert.equal(calls[2]?.includes('--replace-id=10'), true);
+  assert.equal(calls[3]?.includes('--replace-id=11'), true);
+});
+
+test('notify-send replacer escapes freedesktop body markup', () => {
+  const calls: string[][] = [];
+  const replacer = createNotifySendReplacer((args, callback) => {
+    calls.push(args);
+    callback(null, '1');
+  });
+
+  replacer('sync', { title: 'SubMiner', body: 'Steins;Gate <0 & more' }, () => {});
+
+  assert.equal(calls[0]?.at(-1), 'Steins;Gate &lt;0 &amp; more');
+});
+
+function spawnError(code: string): Error {
+  return Object.assign(new Error(`spawn notify-send ${code}`), { code });
+}
+
+function daemonError(): Error {
+  // execFile reports a bad exit as a numeric code, unlike a libuv spawn failure.
+  return Object.assign(new Error('notify-send exited with code 1'), { code: 1 });
+}
+
+test('notify-send replacer falls back to Electron notifications permanently when the binary is missing', () => {
+  let execCalls = 0;
+  let fallbacks = 0;
+  const replacer = createNotifySendReplacer((_args, callback) => {
+    execCalls += 1;
+    callback(spawnError('ENOENT'), '');
+  });
+
+  replacer('sync', { title: 'SubMiner', body: 'first' }, () => (fallbacks += 1));
+  replacer('sync', { title: 'SubMiner', body: 'second' }, () => (fallbacks += 1));
+
+  assert.equal(execCalls, 1);
+  assert.equal(fallbacks, 2);
+});
+
+test('notify-send replacer keeps using notify-send after a transient daemon failure', () => {
+  let execCalls = 0;
+  let fallbacks = 0;
+  const replacer = createNotifySendReplacer((_args, callback) => {
+    execCalls += 1;
+    callback(execCalls === 1 ? daemonError() : null, '5');
+  });
+
+  replacer('sync', { title: 'SubMiner', body: 'first' }, () => (fallbacks += 1));
+  replacer('sync', { title: 'SubMiner', body: 'second' }, () => assert.fail('no fallback'));
+
+  // One bad answer only costs that update; a busy daemon must not downgrade the rest of the run.
+  assert.equal(execCalls, 2);
+  assert.equal(fallbacks, 1);
+});
+
+test('notify-send replacer retries after a transient spawn failure', () => {
+  let execCalls = 0;
+  let fallbacks = 0;
+  const replacer = createNotifySendReplacer((_args, callback) => {
+    execCalls += 1;
+    // EMFILE means the process was out of descriptors, not that notify-send is unusable.
+    callback(execCalls === 1 ? spawnError('EMFILE') : null, '5');
+  });
+
+  replacer('sync', { title: 'SubMiner', body: 'first' }, () => (fallbacks += 1));
+  replacer('sync', { title: 'SubMiner', body: 'second' }, () => assert.fail('no fallback'));
+
+  assert.equal(execCalls, 2);
+  assert.equal(fallbacks, 1);
+});
+
+test('notify-send replacer gives up after a run of transient failures', () => {
+  let execCalls = 0;
+  let fallbacks = 0;
+  const replacer = createNotifySendReplacer((_args, callback) => {
+    execCalls += 1;
+    callback(daemonError(), '');
+  });
+
+  for (let update = 0; update < 5; update += 1) {
+    replacer('sync', { title: 'SubMiner', body: `update ${update}` }, () => (fallbacks += 1));
+  }
+
+  // Three strikes, then every later update goes straight to Electron instead of waiting on a spawn.
+  assert.equal(execCalls, 3);
+  assert.equal(fallbacks, 5);
+});
+
+test('notify-send replacer falls back with only the latest update when a send fails mid-flight', () => {
+  let execCalls = 0;
+  const fallbacks: string[] = [];
+  const pending: Array<(error: Error | null, stdout: string) => void> = [];
+  const replacer = createNotifySendReplacer((_args, callback) => {
+    execCalls += 1;
+    pending.push(callback);
+  });
+
+  replacer('sync', { title: 'SubMiner', body: 'first' }, () => fallbacks.push('first'));
+  replacer('sync', { title: 'SubMiner', body: 'second' }, () => fallbacks.push('second'));
+  pending[0]?.(spawnError('ENOENT'), '');
+
+  assert.equal(execCalls, 1);
+  // The queued update still reaches the user, and the superseded one is dropped rather than
+  // flashing a stale message ahead of it.
+  assert.deepEqual(fallbacks, ['second']);
+});
+
+test('notify-send replacer survives a synchronous spawn throw', () => {
+  let execCalls = 0;
+  let fallbacks = 0;
+  const replacer = createNotifySendReplacer(() => {
+    execCalls += 1;
+    throw new Error('spawn threw synchronously');
+  });
+
+  replacer('sync', { title: 'SubMiner', body: 'first' }, () => (fallbacks += 1));
+  // A wedged entry would silently drop every later update, so the fallback must still fire.
+  replacer('sync', { title: 'SubMiner', body: 'second' }, () => (fallbacks += 1));
+
+  assert.equal(execCalls, 1);
+  assert.equal(fallbacks, 2);
+});
+
 test('default notification icon resolves cwd fallback through injected deps', () => {
  const resolvedPath = resolveDefaultNotificationIconPath({
    platform: 'linux',
@@ -1,3 +1,4 @@
+import { execFile } from 'child_process';
 import electron from 'electron';
 import * as fs from 'fs';
 import * as path from 'path';
@@ -53,11 +54,161 @@ function resolveRuntimeDefaultNotificationIconPath(): string | null {
 }

 /**
- * Live notifications keyed by `replaceId`. Electron exposes no native "replace this notification"
- * flag, so a repeated status closes its predecessor instead of stacking a fresh toast per update.
+ * Live Electron notifications keyed by `replaceId`, for platforms without true in-place
+ * replacement (and as the Linux fallback when notify-send is unusable). Electron exposes no native
+ * "replace this notification" flag, so a repeated status closes its predecessor instead of
+ * stacking a fresh toast per update.
 */
 const notificationsByReplaceId = new Map<string, Electron.Notification>();

+/** Untracks first, so the notification's own `close` handler cannot race a replacement into it. */
+function closeTrackedElectronNotification(replaceId: string): void {
+  const tracked = notificationsByReplaceId.get(replaceId);
+  if (!tracked) return;
+  notificationsByReplaceId.delete(replaceId);
+  tracked.close();
+}
+
+/** The freedesktop body is markup; unescaped `&`/`<` in an anime title would corrupt or drop it. */
+function escapeFreedesktopNotificationBody(body: string): string {
+  return body.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;');
+}
+
+type NotifySendExec = (
+  args: string[],
+  callback: (error: Error | null, stdout: string) => void,
+) => void;
+
+type NotifySendNotification = { title: string; body?: string; iconPath?: string };
+
+type NotifySendPending = { notification: NotifySendNotification; fallback: () => void };
+
+type NotifySendEntry = {
+  dbusId: string | null;
+  sending: boolean;
+  pending: NotifySendPending | null;
+};
+
+/**
+ * A sick daemon should not make every later update wait out the spawn timeout first, so a run of
+ * failures still gives up on notify-send even when none of them is a missing binary.
+ */
+const NOTIFY_SEND_MAX_CONSECUTIVE_FAILURES = 3;
+
+/**
+ * A missing or non-executable binary never fixes itself, so notify-send is abandoned on the spot.
+ * Every other failure (a bad exit code, a kill signal, or a transient spawn error like `EMFILE`
+ * under fd pressure) goes through the consecutive-failure threshold instead.
+ */
+const NOTIFY_SEND_PERMANENT_ERROR_CODES = new Set(['ENOENT', 'EACCES', 'EPERM']);
+
+function isNotifySendUnusable(error: unknown): boolean {
+  const code = (error as NodeJS.ErrnoException | null)?.code;
+  return typeof code === 'string' && NOTIFY_SEND_PERMANENT_ERROR_CODES.has(code);
+}
+
+/**
+ * In-place notification replacement for Linux. Electron cannot reuse a freedesktop notification id,
+ * so its close+show fallback makes every progress update flicker off-screen and back. notify-send
+ * `--replace-id` updates the existing popup statically instead. The daemon-assigned id comes from
+ * `--print-id`, so only one send per replaceId is in flight at a time and a fast progress stream
+ * cannot race the id capture. Updates arriving mid-send collapse to the latest one, since a stale
+ * progress message is never worth showing. Any failed update falls back to the Electron path, and
+ * notify-send is abandoned for good once it looks unusable rather than merely unlucky.
+ */
+export function createNotifySendReplacer(
+  execNotifySend: NotifySendExec,
+): (replaceId: string, notification: NotifySendNotification, fallback: () => void) => void {
+  const stateByReplaceId = new Map<string, NotifySendEntry>();
+  let unavailable = false;
+  let consecutiveFailures = 0;
+
+  const flush = (entry: NotifySendEntry): void => {
+    if (entry.sending) return;
+    const next = entry.pending;
+    if (!next) return;
+    entry.pending = null;
+    if (unavailable) {
+      next.fallback();
+      return;
+    }
+
+    const args = ['--app-name=SubMiner', '--print-id'];
+    if (entry.dbusId) {
+      args.push(`--replace-id=${entry.dbusId}`);
+    }
+    if (next.notification.iconPath) {
+      args.push(`--icon=${next.notification.iconPath}`);
+    }
+    args.push('--', next.notification.title);
+    if (next.notification.body) {
+      args.push(escapeFreedesktopNotificationBody(next.notification.body));
+    }
+
+    const finish = (): void => {
+      entry.sending = false;
+      flush(entry);
+    };
+
+    const handleFailure = (error: unknown, unusable: boolean): void => {
+      consecutiveFailures += 1;
+      if (unusable || consecutiveFailures >= NOTIFY_SEND_MAX_CONSECUTIVE_FAILURES) {
+        unavailable = true;
+        logger.warn('notify-send unusable; falling back to Electron notifications', error);
+      } else {
+        logger.warn('notify-send update failed; showing it as an Electron notification', error);
+      }
+      // A queued update has already superseded this one, so flushing it would flash a stale message
+      // before the newer one renders. The pending update falls back on its own turn if needed.
+      if (!entry.pending) {
+        next.fallback();
+      }
+      finish();
+    };
+
+    entry.sending = true;
+    // A synchronous throw would otherwise leave `sending` stuck true and wedge the entry, silently
+    // dropping every later update for this replaceId, so spawn failures are caught here too. It
+    // also means the call itself is malformed, which will not fix itself on the next update.
+    try {
+      execNotifySend(args, (error, stdout) => {
+        if (error) {
+          handleFailure(error, isNotifySendUnusable(error));
+          return;
+        }
+        consecutiveFailures = 0;
+        const id = stdout.trim();
+        if (/^[1-9]\d*$/.test(id)) {
+          entry.dbusId = id;
+        }
+        finish();
+      });
+    } catch (error) {
+      handleFailure(error, true);
+    }
+  };
+
+  return (replaceId, notification, fallback) => {
+    if (unavailable) {
+      fallback();
+      return;
+    }
+    let entry = stateByReplaceId.get(replaceId);
+    if (!entry) {
+      entry = { dbusId: null, sending: false, pending: null };
+      stateByReplaceId.set(replaceId, entry);
+    }
+    entry.pending = { notification, fallback };
+    flush(entry);
+  };
+}
+
+const showLinuxReplaceableNotification = createNotifySendReplacer((args, callback) =>
+  execFile('notify-send', args, { timeout: 5_000 }, (error, stdout) =>
+    callback(error, stdout ?? ''),
+  ),
+);
+
 export function showDesktopNotification(
  title: string,
  options: { body?: string; icon?: string; replaceId?: string },
@@ -103,16 +254,40 @@ export function showDesktopNotification(
    }
  }

-  const notification = new Notification(notificationOptions);
  const replaceId = options.replaceId?.trim();
-  if (replaceId) {
-    notificationsByReplaceId.get(replaceId)?.close();
-    notificationsByReplaceId.set(replaceId, notification);
-    notification.once('close', () => {
-      if (notificationsByReplaceId.get(replaceId) === notification) {
-        notificationsByReplaceId.delete(replaceId);
-      }
-    });
+  const showElectronNotification = (): void => {
+    const notification = new Notification(notificationOptions);
+    if (replaceId) {
+      closeTrackedElectronNotification(replaceId);
+      notificationsByReplaceId.set(replaceId, notification);
+      notification.once('close', () => {
+        if (notificationsByReplaceId.get(replaceId) === notification) {
+          notificationsByReplaceId.delete(replaceId);
+        }
+      });
+    }
+    notification.show();
+  };
+
+  // notify-send takes a path or a theme name, so a base64 icon only exists as an in-memory
+  // NativeImage. Those keep the Electron path and their icon rather than losing it to in-place
+  // replacement.
+  const iconPath =
+    typeof notificationOptions.icon === 'string' ? notificationOptions.icon : undefined;
+  const iconNeedsElectron = notificationOptions.icon !== undefined && iconPath === undefined;
+
+  if (replaceId && process.platform === 'linux' && !iconNeedsElectron) {
+    // An earlier update for this id may have rendered through Electron (a transient notify-send
+    // failure, or a NativeImage icon). That toast is not the one notify-send replaces, so it would
+    // sit on screen next to the updated popup.
+    closeTrackedElectronNotification(replaceId);
+    showLinuxReplaceableNotification(
+      replaceId,
+      { title, body: options.body, iconPath },
+      showElectronNotification,
+    );
+    return;
  }
-  notification.show();
+
+  showElectronNotification();
 }