feat(web+macos): wire deep-link consumers — composer pre-fill, thread nav, window activation (LUM-2068)

apps/macos/src/main/deep-links.test.ts

@@ -4,6 +4,28 @@ import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test";
 // from `app.on` so tests can fire them. `setAsDefaultProtocolClient`
 // is also captured to verify scheme registration.
 type Listener = (...args: unknown[]) => void;
+
+// Synthetic WebContents stub for the subscriber-tracking tests.
+// `once("destroyed", …)` captures the cleanup handler so tests can
+// fire it to simulate a renderer crash / window close.
+const makeSender = (): {
+  sender: { once: (event: string, handler: () => void) => void };
+  fireDestroyed: () => void;
+} => {
+  let destroyedHandler: (() => void) | null = null;
+  return {
+    sender: {
+      once: (event, handler) => {
+        if (event === "destroyed") destroyedHandler = handler;
+      },
+    },
+    fireDestroyed: () => destroyedHandler?.(),
+  };
+};
+const subscribeWith = (s: ReturnType<typeof makeSender>) =>
+  ipcOnListeners.get("vellum:deepLinks:subscribe")?.({ sender: s.sender });
+const unsubscribeWith = (s: ReturnType<typeof makeSender>) =>
+  ipcOnListeners.get("vellum:deepLinks:unsubscribe")?.({ sender: s.sender });
 const appListeners = new Map<string, Listener>();
 const appOnMock = mock((event: string, listener: Listener) => {
   appListeners.set(event, listener);
@@ -21,15 +43,26 @@ let windows: Array<{
   webContents: { send: ReturnType<typeof mock> };
 }> = [];
 
+let appIsReady = true;
 mock.module("electron", () => ({
   app: {
     on: appOnMock,
     setAsDefaultProtocolClient: setAsDefaultProtocolClientMock,
+    isReady: () => appIsReady,
   },
   ipcMain: { handle: ipcHandleMock, on: ipcOnMock },
   BrowserWindow: { getAllWindows: () => windows },
 }));
 
+// `./main-window` is called from `handleDeepLink` to bring the main
+// window forward for actionable kinds. Stub so we can assert on the
+// call without standing up the full lifecycle module (which
+// transitively imports electron-store).
+const ensureMainWindowVisibleMock = mock(async () => undefined);
+mock.module("./main-window", () => ({
+  ensureVisible: ensureMainWindowVisibleMock,
+}));
+
 const {
   __resetForTesting,
   extractDeepLinkFromArgv,
@@ -51,7 +84,9 @@ beforeEach(() => {
   setAsDefaultProtocolClientMock.mockClear();
   ipcHandleMock.mockClear();
   ipcOnMock.mockClear();
+  ensureMainWindowVisibleMock.mockClear();
   windows = [];
+  appIsReady = true;
 });
 
 afterEach(() => {
@@ -232,16 +267,18 @@ describe("installDeepLinks", () => {
     handleDeepLink("vellum://send?message=backlog");
 
     // Renderer mounts: subscribes, drains.
-    ipcOnListeners.get("vellum:deepLinks:subscribe")?.();
+    const s1 = makeSender();
+    subscribeWith(s1);
     expect(drainHandler()).toEqual([{ kind: "send", message: "backlog" }]);
 
     // Live link arrives while subscribed — broadcasts only.
     handleDeepLink("vellum://thread/live");
 
     // Renderer hard-navigates: unsubscribe, then a new renderer
     // mounts and drains. The live link must NOT be replayed.
-    ipcOnListeners.get("vellum:deepLinks:unsubscribe")?.();
-    ipcOnListeners.get("vellum:deepLinks:subscribe")?.();
+    unsubscribeWith(s1);
+    const s2 = makeSender();
+    subscribeWith(s2);
     expect(drainHandler()).toEqual([]);
   });
 
@@ -251,33 +288,30 @@ describe("installDeepLinks", () => {
       (c) => c[0] === "vellum:deepLinks:drain",
     )![1] as () => unknown[];
 
-    // Renderer mounted and drained the initial empty backlog.
-    ipcOnListeners.get("vellum:deepLinks:subscribe")?.();
+    const s1 = makeSender();
+    subscribeWith(s1);
     expect(drainHandler()).toEqual([]);
 
-    // User logs out — renderer unmounts.
-    ipcOnListeners.get("vellum:deepLinks:unsubscribe")?.();
+    unsubscribeWith(s1);
 
-    // A deep link arrives during the auth flow. No subscribers,
-    // so it must be buffered.
     handleDeepLink("vellum://thread/post-logout");
 
-    // User logs in — renderer mounts again, subscribes, drains.
-    ipcOnListeners.get("vellum:deepLinks:subscribe")?.();
+    const s2 = makeSender();
+    subscribeWith(s2);
     expect(drainHandler()).toEqual([
       { kind: "openThread", threadId: "post-logout" },
     ]);
   });
 
-  test("subscribe/unsubscribe IPC accounting is reference-counted and never goes negative", () => {
+  test("unsubscribe with no matching subscriber is a no-op (idempotent delete)", () => {
     installDeepLinks();
     const drainHandler = ipcHandleMock.mock.calls.find(
       (c) => c[0] === "vellum:deepLinks:drain",
     )![1] as () => unknown[];
 
-    // Unsubscribe before any subscribe — should clamp to 0, not -1.
-    ipcOnListeners.get("vellum:deepLinks:unsubscribe")?.();
-    ipcOnListeners.get("vellum:deepLinks:unsubscribe")?.();
+    const s = makeSender();
+    unsubscribeWith(s);
+    unsubscribeWith(s);
 
     handleDeepLink("vellum://send?message=should-buffer");
     expect(drainHandler()).toEqual([
@@ -287,7 +321,8 @@ describe("installDeepLinks", () => {
 
   test("post-drain live links still broadcast (live subscribers still get them)", () => {
     installDeepLinks();
-    ipcOnListeners.get("vellum:deepLinks:subscribe")?.();
+    const s = makeSender();
+    subscribeWith(s);
 
     const w = makeWindow();
     windows = [w];
@@ -298,6 +333,32 @@ describe("installDeepLinks", () => {
       message: "live",
     });
   });
+
+  test("destroyed webContents auto-clears its subscription (no leak when React cleanup misses)", () => {
+    // The real bug this guards against: window close on Darwin
+    // can tear down the JS context before React effect cleanups
+    // flush, so `vellum:deepLinks:unsubscribe` never fires.
+    // The `destroyed` listener cleans up regardless, so future
+    // links buffer correctly.
+    installDeepLinks();
+    const drainHandler = ipcHandleMock.mock.calls.find(
+      (c) => c[0] === "vellum:deepLinks:drain",
+    )![1] as () => unknown[];
+
+    const s = makeSender();
+    subscribeWith(s);
+    expect(drainHandler()).toEqual([]);
+
+    // Simulate window close without React cleanup running — only
+    // the webContents `destroyed` event fires.
+    s.fireDestroyed();
+
+    // No subscribers now → next link is buffered.
+    handleDeepLink("vellum://send?message=after-crash");
+    expect(drainHandler()).toEqual([
+      { kind: "send", message: "after-crash" },
+    ]);
+  });
 });
 
 describe("handleDeepLink — broadcast", () => {
@@ -342,3 +403,72 @@ describe("handleDeepLink — broadcast", () => {
     });
   });
 });
+
+describe("handleDeepLink — window activation", () => {
+  test("brings the main window forward for `send` (covers the no-renderer case on Darwin)", () => {
+    handleDeepLink("vellum://send?message=hi");
+    expect(ensureMainWindowVisibleMock).toHaveBeenCalledTimes(1);
+  });
+
+  test("brings the main window forward for `openThread`", () => {
+    handleDeepLink("vellum://thread/abc");
+    expect(ensureMainWindowVisibleMock).toHaveBeenCalledTimes(1);
+  });
+
+  test("does NOT activate the window for unknown kinds (no UI side effect for foreign schemes)", () => {
+    handleDeepLink("javascript:alert(1)");
+    handleDeepLink("file:///etc/passwd");
+    handleDeepLink("not a url");
+
+    expect(ensureMainWindowVisibleMock).not.toHaveBeenCalled();
+  });
+
+  test("defers activation when app is not yet ready (cold-launch via vellum://)", () => {
+    // Cold launch path: `will-finish-launching` → `open-url` fires
+    // BEFORE `app.whenReady()`. `new BrowserWindow()` pre-ready
+    // would race Electron init; the link is buffered above and the
+    // initial `installMainWindow` in the whenReady chain creates
+    // the window which drains it on mount.
+    appIsReady = false;
+    handleDeepLink("vellum://send?message=cold-launch");
+
+    expect(ensureMainWindowVisibleMock).not.toHaveBeenCalled();
+  });
+
+  test("activates after app becomes ready (warm path: subsequent links)", () => {
+    appIsReady = false;
+    handleDeepLink("vellum://send?message=cold");
+    expect(ensureMainWindowVisibleMock).not.toHaveBeenCalled();
+
+    // Simulate whenReady having fired.
+    appIsReady = true;
+    handleDeepLink("vellum://thread/warm");
+    expect(ensureMainWindowVisibleMock).toHaveBeenCalledTimes(1);
+  });
+
+  test("buffers the link AND activates so the renderer-on-mount drain still delivers it", () => {
+    // Simulating the macOS path: app alive, main window closed,
+    // user clicks vellum://send → main handles it. The link must
+    // both (a) be parked in the buffer for the freshly-created
+    // renderer to drain, and (b) trigger window creation so the
+    // renderer actually mounts.
+    handleDeepLink("vellum://send?message=delivered");
+
+    // Activation fired.
+    expect(ensureMainWindowVisibleMock).toHaveBeenCalledTimes(1);
+    // Link buffered (no subscribers yet — the new window hasn't
+    // mounted).
+    const drainHandler = ipcHandleMock.mock.calls.find(
+      (c) => c[0] === "vellum:deepLinks:drain",
+    );
+    // installDeepLinks hasn't run in this test, so register the
+    // handler via a fresh install before draining.
+    if (!drainHandler) {
+      installDeepLinks();
+    }
+    const drain = ipcHandleMock.mock.calls.find(
+      (c) => c[0] === "vellum:deepLinks:drain",
+    )![1] as () => unknown[];
+    expect(drain()).toEqual([{ kind: "send", message: "delivered" }]);
+  });
+});

apps/macos/src/main/deep-links.ts

@@ -1,4 +1,11 @@
-import { BrowserWindow, app, ipcMain } from "electron";
+import {
+  BrowserWindow,
+  app,
+  ipcMain,
+  type WebContents,
+} from "electron";
+
+import { ensureVisible as ensureMainWindowVisible } from "./main-window";
 
 /**
  * Inbound deep links — `vellum://` and `vellum-assistant://` URL
@@ -102,26 +109,27 @@ export const extractDeepLinkFromArgv = (argv: readonly string[]): string | null
 
 const pending: DeepLink[] = [];
 
-// Active renderer subscribers. Renderer calls `vellum:deepLinks:subscribe`
-// when its `onLink` handler is registered and `vellum:deepLinks:unsubscribe`
-// on cleanup. Buffer when count is zero (no subscribers to receive the
-// broadcast); broadcast-only when count > 0.
+// Active renderer subscribers tracked by their `WebContents` rather
+// than a counter. Renderer calls `vellum:deepLinks:subscribe` when
+// its `onLink` handler registers; we add the `event.sender` and
+// listen for that webContents's `destroyed` event so cleanup runs
+// even when React effect teardown doesn't fire (window-close kills
+// the JS context before `useEffect` cleanups flush — a leaked
+// counter would flip buffering off and silently drop later links).
+// `vellum:deepLinks:unsubscribe` covers the common mount/unmount
+// path; the `destroyed` listener is the defense-in-depth.
 //
-// This is what closes both the Codex P2 (live-link replay on renderer
-// reload — broadcast doesn't enter the buffer when a subscriber is
-// listening) AND the logout-relogin gap (after the renderer unmounts,
-// links arriving during the auth flip land in the buffer and the next
-// renderer drains them on mount). A "drained once, never buffer
-// again" flag is wrong because it conflates "has ever drained" with
-// "is subscribed right now."
+// Buffer when the set is empty; broadcast-only when non-empty. This
+// keeps both the live-link-replay defense AND the
+// renderer-down-link-buffers behavior the consumer relies on.
 //
 // Residual race (sub-microsecond, not realistically triggerable by
 // user action): a link arriving between the renderer's
 // `ipcRenderer.on` registration and main's processing of the
 // `subscribe` IPC could be buffered AND broadcast. A single
 // renderer-side dedup would catch this if it ever bit; today the
 // timing makes it theoretical.
-let subscriberCount = 0;
+const subscribers = new Set<WebContents>();
 
 const broadcast = (link: DeepLink): void => {
   for (const win of BrowserWindow.getAllWindows()) {
@@ -131,14 +139,40 @@ const broadcast = (link: DeepLink): void => {
 };
 
 /**
- * Main entry — parse, buffer-if-no-subscribers, broadcast. Internal
- * to this module; exposed via the `open-url` / `second-instance`
+ * Main entry — parse, buffer-if-no-subscribers, broadcast, and
+ * bring the main window forward for actionable kinds. Internal to
+ * this module; exposed via the `open-url` / `second-instance`
  * event handlers and exported for tests.
+ *
+ * Window activation lives HERE (not only in the renderer-side
+ * consumer) because on macOS the app keeps running after the main
+ * window closes (`window-all-closed` doesn't quit on Darwin). In
+ * that state the renderer doesn't exist, so a renderer-only
+ * `ensureMainWindowVisible()` would never fire; the buffered link
+ * would sit forever. `unknown` kinds skip activation: an attacker
+ * who could induce the OS to route a `javascript:` URL to us
+ * shouldn't get a UI side effect.
+ *
+ * Main owns the cold path (no-renderer activation), renderer owns
+ * the hot path (window minimized / behind another window — see
+ * `useGlobalDeepLinkConsumer`). The duplicated call when both fire
+ * is intentional defense-in-depth — `ensureVisible` short-circuits
+ * on an already-visible main window.
  */
 export const handleDeepLink = (input: string): void => {
   const link = parseVellumUrl(input);
-  if (subscriberCount === 0) pending.push(link);
+  if (subscribers.size === 0) pending.push(link);
   broadcast(link);
+  // Activation is gated on `app.isReady()`. On cold launch, the
+  // `will-finish-launching` → `open-url` path fires BEFORE
+  // `app.whenReady()`, and `new BrowserWindow()` pre-ready races
+  // Electron's init. The link is already buffered above; the
+  // initial `installMainWindow()` in the `whenReady` chain in
+  // `index.ts` creates the first window, which drains the link
+  // on mount.
+  if (link.kind !== "unknown" && app.isReady()) {
+    void ensureMainWindowVisible();
+  }
 };
 
 let installed = false;
@@ -175,22 +209,29 @@ export const installDeepLinks = (): void => {
     return pending.splice(0, pending.length);
   });
 
-  // Subscriber tracking — see the `subscriberCount` comment above
-  // for the model. `ipcMain.on` (fire-and-forget) is sufficient —
-  // these are accounting messages, no return value expected. The
-  // preload sends them inside `onLink` registration / cleanup.
-  ipcMain.on("vellum:deepLinks:subscribe", () => {
-    subscriberCount++;
+  // Subscriber tracking — see the `subscribers` comment above for
+  // the model. `ipcMain.on` (fire-and-forget) is sufficient — these
+  // are accounting messages, no return value expected. The preload
+  // sends them inside `onLink` registration / cleanup; the
+  // `destroyed` listener is the defense-in-depth for the cases
+  // where the React effect cleanup doesn't run before the
+  // webContents is torn down.
+  ipcMain.on("vellum:deepLinks:subscribe", (event) => {
+    if (subscribers.has(event.sender)) return;
+    subscribers.add(event.sender);
+    event.sender.once("destroyed", () => {
+      subscribers.delete(event.sender);
+    });
   });
-  ipcMain.on("vellum:deepLinks:unsubscribe", () => {
-    subscriberCount = Math.max(0, subscriberCount - 1);
+  ipcMain.on("vellum:deepLinks:unsubscribe", (event) => {
+    subscribers.delete(event.sender);
   });
 };
 
 // Test seam — exported only for unit-test setup. Production code
 // uses `installDeepLinks` instead.
 export const __resetForTesting = (): void => {
   installed = false;
-  subscriberCount = 0;
+  subscribers.clear();
   pending.length = 0;
 };